Linaro-mm-sig

linaro-mm-sig@lists.linaro.org

2 participants
3305 discussions

by Christian König

Hi everyone, dma_fences have ever lived under the tyranny dictated by the module lifetime of their issuer, leading to crashes should anybody still holding a reference to a dma_fence when the module of the issuer was unloaded. The basic problem is that when buffer are shared between drivers dma_fence objects can leak into external drivers and stay there even after they are signaled. The dma_resv object for example only lazy releases dma_fences. So what happens is that when the module who originally created the dma_fence unloads the dma_fence_ops function table becomes unavailable as well and so any attempt to release the fence crashes the system. Previously various approaches have been discussed, including changing the locking semantics of the dma_fence callbacks (by me) as well as using the drm scheduler as intermediate layer (by Sima) to disconnect dma_fences from their actual users, but none of them are actually solving all problems. Tvrtko did some really nice prerequisite work by protecting the returned strings of the dma_fence_ops by RCU. This way dma_fence creators where able to just wait for an RCU grace period after fence signaling before they could be save to free those data structures. Now this patch set here goes a step further and protects the whole dma_fence_ops structure by RCU, so that after the fence signals the pointer to the dma_fence_ops is set to NULL when there is no wait nor release callback given. All functionality which use the dma_fence_ops reference are put inside an RCU critical section, except for the deprecated issuer specific wait and of course the optional release callback. Additional to the RCU changes the lock protecting the dma_fence state previously had to be allocated external. This set here now changes the functionality to make that external lock optional and allows dma_fences to use an inline lock and be self contained. v4: Rebases the whole set on upstream changes, especially the cleanup from Philip in patch "drm/amdgpu: independence for the amdkfd_fence!". Adding two patches which brings the DMA-fence self tests up to date. The first selftest changes removes the mock_wait and so actually starts testing the default behavior instead of some hacky implementation in the test. This one got upstreamed independent of this set. The second drops the mock_fence as well and tests the new RCU and inline spinlock functionality. v5: Rebase on top of drm-misc-next instead of drm-tip, leave out all driver changes for now since those should go through the driver specific paths anyway. Address a few more review comments, especially some rebase mess and typos. And finally fix one more bug found by AMDs CI system. Especially the first patch still needs a Reviewed-by, apart from that I think I've addressed all review comments and problems. Please review and comment, Christian.

2 weeks, 4 days

Re: [PATCH 0/5] New DRM accel driver for Texas Instruments' C7x DSPs

by Christian König

On 1/13/26 18:44, Tomeu Vizoso wrote: > This series adds a new DRM/Accel driver that supports the C7x DSPs > inside some Texas Instruments SoCs such as the J722S. These can be used > as accelerators for various workloads, including machine learning > inference. > > This driver controls the power state of the hardware via remoteproc and > communicates with the firmware running on the DSP via rpmsg_virtio. The > kernel driver itself allocates buffers, manages contexts, and submits > jobs to the DSP firmware. Buffers are mapped by the DSP itself using its > MMU, providing memory isolation among different clients. > > The source code for the firmware running on the DSP is available at: > https://gitlab.freedesktop.org/tomeu/thames_firmware/. > > Everything else is done in userspace, as a Gallium driver (also called > thames) that is part of the Mesa3D project: https://docs.mesa3d.org/teflon.html > > If there is more than one core that advertises the same rpmsg_virtio > service name, the driver will load balance jobs between them with > drm-gpu-scheduler. I only took 5 minutes to skim over it, so no full review. You have the classic mistake of allocating memory in the run_job callback of the scheduler, but that is trivial to fix. Apart from that looks pretty solid to me. Regards, Christian. > > Userspace portion of the driver: https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/39298 > > Signed-off-by: Tomeu Vizoso <tomeu(a)tomeuvizoso.net> > --- > Tomeu Vizoso (5): > arm64: dts: ti: k3-j722s-ti-ipc-firmware: Add memory pool for DSP i/o buffers > accel/thames: Add driver for the C7x DSPs in TI SoCs > accel/thames: Add IOCTLs for BO creation and mapping > accel/thames: Add IOCTL for job submission > accel/thames: Add IOCTL for memory synchronization > > Documentation/accel/thames/index.rst | 28 ++ > MAINTAINERS | 9 + > .../boot/dts/ti/k3-j722s-ti-ipc-firmware.dtsi | 11 +- > drivers/accel/Kconfig | 1 + > drivers/accel/Makefile | 3 +- > drivers/accel/thames/Kconfig | 26 ++ > drivers/accel/thames/Makefile | 11 + > drivers/accel/thames/thames_core.c | 161 +++++++ > drivers/accel/thames/thames_core.h | 53 +++ > drivers/accel/thames/thames_device.c | 93 +++++ > drivers/accel/thames/thames_device.h | 46 ++ > drivers/accel/thames/thames_drv.c | 180 ++++++++ > drivers/accel/thames/thames_drv.h | 21 + > drivers/accel/thames/thames_gem.c | 407 ++++++++++++++++++ > drivers/accel/thames/thames_gem.h | 45 ++ > drivers/accel/thames/thames_ipc.h | 204 +++++++++ > drivers/accel/thames/thames_job.c | 463 +++++++++++++++++++++ > drivers/accel/thames/thames_job.h | 51 +++ > drivers/accel/thames/thames_rpmsg.c | 276 ++++++++++++ > drivers/accel/thames/thames_rpmsg.h | 27 ++ > 20 files changed, 2113 insertions(+), 3 deletions(-) > --- > base-commit: 27927a79b3c6aebd18f38507a8160294243763dc > change-id: 20260113-thames-334127a2d91d > > Best regards,

2 weeks, 5 days

Re: [PATCH 4/5] accel/thames: Add IOCTL for job submission

by Christian König

On 1/13/26 18:44, Tomeu Vizoso wrote: > Using the DRM GPU scheduler infrastructure, with a scheduler for each > core. > > Contexts are created in all cores, and buffers mapped to all of them as > well, so all cores are ready to execute any job. > > The job submission code was initially based on Panfrost. > > Signed-off-by: Tomeu Vizoso <tomeu(a)tomeuvizoso.net> > --- > drivers/accel/thames/Makefile | 1 + > drivers/accel/thames/thames_core.c | 6 + > drivers/accel/thames/thames_drv.c | 19 ++ > drivers/accel/thames/thames_job.c | 463 ++++++++++++++++++++++++++++++++++++ > drivers/accel/thames/thames_job.h | 51 ++++ > drivers/accel/thames/thames_rpmsg.c | 52 ++++ > 6 files changed, 592 insertions(+) > > diff --git a/drivers/accel/thames/Makefile b/drivers/accel/thames/Makefile > index 0051e319f2e4966de72bc342d5b6e40b2890c006..b6c4516f8250e3d442f22e80d609cb1be2970128 100644 > --- a/drivers/accel/thames/Makefile > +++ b/drivers/accel/thames/Makefile > @@ -7,4 +7,5 @@ thames-y := \ > thames_device.o \ > thames_drv.o \ > thames_gem.o \ > + thames_job.o \ > thames_rpmsg.o > diff --git a/drivers/accel/thames/thames_core.c b/drivers/accel/thames/thames_core.c > index 92af1d68063116bcfa28a33960cbe829029fc1bf..5b96b25d287096803e034fcd4261d51795871543 100644 > --- a/drivers/accel/thames/thames_core.c > +++ b/drivers/accel/thames/thames_core.c > @@ -13,6 +13,7 @@ > > #include "thames_core.h" > #include "thames_device.h" > +#include "thames_job.h" > #include "thames_rpmsg.h" > > /* Shift to convert bytes to megabytes (divide by 1048576) */ > @@ -115,11 +116,16 @@ int thames_core_init(struct thames_core *core) > if (err) > return err; > > + err = thames_job_init(core); > + if (err) > + return err; > + > return 0; > } > > void thames_core_fini(struct thames_core *core) > { > + thames_job_fini(core); > thames_rpmsg_fini(core); > } > > diff --git a/drivers/accel/thames/thames_drv.c b/drivers/accel/thames/thames_drv.c > index 29a80b9747ae84778b09f5dbd5b8d6d596f1947a..bf7355832241d5a671e196f465d891effaa4a8fb 100644 > --- a/drivers/accel/thames/thames_drv.c > +++ b/drivers/accel/thames/thames_drv.c > @@ -14,6 +14,7 @@ > #include "thames_drv.h" > #include "thames_core.h" > #include "thames_gem.h" > +#include "thames_job.h" > #include "thames_ipc.h" > > static struct platform_device *drm_dev; > @@ -38,8 +39,22 @@ static int thames_open(struct drm_device *dev, struct drm_file *file) > > file->driver_priv = thames_priv; > > + ret = thames_job_open(thames_priv); > + if (ret) > + goto err_free; > + > + ret = thames_context_create(thames_priv); > + if (ret) { > + dev_err(dev->dev, "Failed to create context for client: %d", ret); > + goto err_close_job; > + } > + > return 0; > > +err_close_job: > + thames_job_close(thames_priv); > +err_free: > + kfree(thames_priv); > err_put_mod: > module_put(THIS_MODULE); > return ret; > @@ -49,6 +64,9 @@ static void thames_postclose(struct drm_device *dev, struct drm_file *file) > { > struct thames_file_priv *thames_priv = file->driver_priv; > > + thames_context_destroy(thames_priv); > + > + thames_job_close(thames_priv); > kfree(thames_priv); > module_put(THIS_MODULE); > } > @@ -57,6 +75,7 @@ static const struct drm_ioctl_desc thames_drm_driver_ioctls[] = { > #define THAMES_IOCTL(n, func) DRM_IOCTL_DEF_DRV(THAMES_##n, thames_ioctl_##func, 0) > THAMES_IOCTL(BO_CREATE, bo_create), > THAMES_IOCTL(BO_MMAP_OFFSET, bo_mmap_offset), > + THAMES_IOCTL(SUBMIT, submit), > }; > > DEFINE_DRM_ACCEL_FOPS(thames_accel_driver_fops); > diff --git a/drivers/accel/thames/thames_job.c b/drivers/accel/thames/thames_job.c > new file mode 100644 > index 0000000000000000000000000000000000000000..bd8f8fa1783cf10c5e71c8f2ce5fcc880a9b150b > --- /dev/null > +++ b/drivers/accel/thames/thames_job.c > @@ -0,0 +1,463 @@ > +// SPDX-License-Identifier: GPL-2.0-only > +/* Copyright 2019 Linaro, Ltd, Rob Herring <robh(a)kernel.org> */ > +/* Copyright 2019 Collabora ltd. */ > +/* Copyright 2024-2025 Tomeu Vizoso <tomeu(a)tomeuvizoso.net> */ > +/* Copyright 2026 Texas Instruments Incorporated - https://www.ti.com/ */ > + > +#include "linux/dev_printk.h" > +#include <drm/drm_file.h> > +#include <drm/drm_gem.h> > +#include <drm/drm_print.h> > +#include <drm/thames_accel.h> > +#include <linux/platform_device.h> > + > +#include "thames_core.h" > +#include "thames_device.h" > +#include "thames_drv.h" > +#include "thames_gem.h" > +#include "thames_job.h" > +#include "thames_rpmsg.h" > + > +#define JOB_TIMEOUT_MS 500 > + > +static struct thames_job *to_thames_job(struct drm_sched_job *sched_job) > +{ > + return container_of(sched_job, struct thames_job, base); > +} > + > +static const char *thames_fence_get_driver_name(struct dma_fence *fence) > +{ > + return "thames"; > +} > + > +static const char *thames_fence_get_timeline_name(struct dma_fence *fence) > +{ > + return "thames"; > +} > + > +static const struct dma_fence_ops thames_fence_ops = { > + .get_driver_name = thames_fence_get_driver_name, > + .get_timeline_name = thames_fence_get_timeline_name, > +}; > + > +static struct dma_fence *thames_fence_create(struct thames_core *core) > +{ > + struct dma_fence *fence; > + > + fence = kzalloc(sizeof(*fence), GFP_KERNEL); Oh yeah, that is the classic mistake :) You should not allocate any memory in the schedulers job_run() callback! The best solution is to split your kzalloc() from the dma_fence_init() and allocate memory when creating the job. The alternative is to use GFP_ATOMIC, but that is not really nice since it digs into the atomic reserve without a good reason. For things like core dumps and debugging output you can also use GFP_NOWAIT, but keep in mind that this can trivially fail so it is not good for things fences. Regards, Christian. > + if (!fence) > + return ERR_PTR(-ENOMEM); > + > + dma_fence_init(fence, &thames_fence_ops, &core->fence_lock, core->fence_context, > + ++core->emit_seqno); > + > + return fence; > +} > + > +static void thames_job_hw_submit(struct thames_core *core, struct thames_job *job) > +{ > + int ret; > + > + /* Don't queue the job if a reset is in progress */ > + if (atomic_read(&core->reset.pending)) > + return; > + > + ret = thames_rpmsg_send_submit_job(core, job->file_priv->context_id, job->job_id, > + to_thames_bo(job->kernel)->iova, job->kernel_size, > + to_thames_bo(job->params)->iova, job->params_size, > + &job->ipc_sequence); > + > + if (ret) { > + dev_err(core->dev, "Failed to submit kernel to DSP core %d\n", core->index); > + return; > + } > +} > + > +static int thames_acquire_object_fences(struct drm_gem_object **bos, int bo_count, > + struct drm_sched_job *job, bool is_write) > +{ > + int i, ret; > + > + for (i = 0; i < bo_count; i++) { > + ret = dma_resv_reserve_fences(bos[i]->resv, 1); > + if (ret) > + return ret; > + > + ret = drm_sched_job_add_implicit_dependencies(job, bos[i], is_write); > + if (ret) > + return ret; > + } > + > + return 0; > +} > + > +static void thames_attach_object_fences(struct drm_gem_object **bos, int bo_count, > + struct dma_fence *fence) > +{ > + int i; > + > + for (i = 0; i < bo_count; i++) > + dma_resv_add_fence(bos[i]->resv, fence, DMA_RESV_USAGE_WRITE); > +} > + > +static int thames_job_push(struct thames_job *job) > +{ > + struct thames_device *tdev = job->tdev; > + struct drm_gem_object **bos; > + struct ww_acquire_ctx acquire_ctx; > + int ret = 0; > + > + dev_dbg(tdev->ddev.dev, "Pushing job with %u in BOs and %u out BOs\n", job->in_bo_count, > + job->out_bo_count); > + bos = kvmalloc_array(job->in_bo_count + job->out_bo_count, sizeof(void *), GFP_KERNEL); > + memcpy(bos, job->in_bos, job->in_bo_count * sizeof(void *)); > + memcpy(&bos[job->in_bo_count], job->out_bos, job->out_bo_count * sizeof(void *)); > + > + ret = drm_gem_lock_reservations(bos, job->in_bo_count + job->out_bo_count, &acquire_ctx); > + if (ret) > + goto err; > + > + scoped_guard(mutex, &tdev->sched_lock) > + { > + drm_sched_job_arm(&job->base); > + > + job->inference_done_fence = dma_fence_get(&job->base.s_fence->finished); > + > + ret = thames_acquire_object_fences(job->in_bos, job->in_bo_count, &job->base, > + false); > + if (ret) > + goto err_unlock; > + > + ret = thames_acquire_object_fences(job->out_bos, job->out_bo_count, &job->base, > + true); > + if (ret) > + goto err_unlock; > + > + kref_get(&job->refcount); /* put by scheduler job completion */ > + > + drm_sched_entity_push_job(&job->base); > + } > + > + thames_attach_object_fences(job->out_bos, job->out_bo_count, job->inference_done_fence); > + > +err_unlock: > + drm_gem_unlock_reservations(bos, job->in_bo_count + job->out_bo_count, &acquire_ctx); > +err: > + kvfree(bos); > + > + return ret; > +} > + > +static void thames_job_cleanup(struct kref *ref) > +{ > + struct thames_job *job = container_of(ref, struct thames_job, refcount); > + struct thames_device *tdev = job->tdev; > + unsigned int i; > + > + dma_fence_put(job->done_fence); > + dma_fence_put(job->inference_done_fence); > + > + ida_free(&tdev->job_ida, job->job_id); > + > + if (job->kernel) > + drm_gem_object_put(job->kernel); > + > + if (job->params) > + drm_gem_object_put(job->params); > + > + if (job->in_bos) { > + for (i = 0; i < job->in_bo_count; i++) > + drm_gem_object_put(job->in_bos[i]); > + > + kvfree(job->in_bos); > + } > + > + if (job->out_bos) { > + for (i = 0; i < job->out_bo_count; i++) > + drm_gem_object_put(job->out_bos[i]); > + > + kvfree(job->out_bos); > + } > + > + kfree(job); > +} > + > +static void thames_job_put(struct thames_job *job) > +{ > + kref_put(&job->refcount, thames_job_cleanup); > +} > + > +static void thames_job_free(struct drm_sched_job *sched_job) > +{ > + struct thames_job *job = to_thames_job(sched_job); > + > + drm_sched_job_cleanup(sched_job); > + > + thames_job_put(job); > +} > + > +static struct thames_core *sched_to_core(struct thames_device *tdev, > + struct drm_gpu_scheduler *sched) > +{ > + unsigned int core; > + > + for (core = 0; core < tdev->num_cores; core++) { > + if (&tdev->cores[core].sched == sched) > + return &tdev->cores[core]; > + } > + > + return NULL; > +} > + > +static struct dma_fence *thames_job_run(struct drm_sched_job *sched_job) > +{ > + struct thames_job *job = to_thames_job(sched_job); > + struct thames_device *tdev = job->tdev; > + struct thames_core *core = sched_to_core(tdev, sched_job->sched); > + struct dma_fence *fence = NULL; > + > + if (unlikely(job->base.s_fence->finished.error)) > + return NULL; > + > + fence = thames_fence_create(core); > + if (IS_ERR(fence)) > + return fence; > + > + if (job->done_fence) > + dma_fence_put(job->done_fence); > + job->done_fence = dma_fence_get(fence); > + > + scoped_guard(mutex, &core->job_lock) > + { > + core->in_flight_job = job; > + thames_job_hw_submit(core, job); > + } > + > + return fence; > +} > + > +static void thames_reset(struct thames_core *core, struct drm_sched_job *bad) > +{ > + if (!atomic_read(&core->reset.pending)) > + return; > + > + drm_sched_stop(&core->sched, bad); > + scoped_guard(mutex, &core->job_lock) core->in_flight_job = NULL; > + thames_core_reset(core); > + atomic_set(&core->reset.pending, 0); > + drm_sched_start(&core->sched, 0); > +} > + > +static enum drm_gpu_sched_stat thames_job_timedout(struct drm_sched_job *sched_job) > +{ > + struct thames_job *job = to_thames_job(sched_job); > + struct thames_device *tdev = job->tdev; > + struct thames_core *core = sched_to_core(tdev, sched_job->sched); > + > + if (!core) { > + dev_err(tdev->ddev.dev, "Failed to find core for timed out job\n"); > + return DRM_GPU_SCHED_STAT_NONE; > + } > + > + dev_err(core->dev, "Job %u timed out on DSP core %d\n", job->job_id, core->index); > + > + atomic_set(&core->reset.pending, 1); > + thames_reset(core, sched_job); > + > + return DRM_GPU_SCHED_STAT_RESET; > +} > + > +static void thames_reset_work(struct work_struct *work) > +{ > + struct thames_core *core; > + > + core = container_of(work, struct thames_core, reset.work); > + thames_reset(core, NULL); > +} > + > +static const struct drm_sched_backend_ops thames_sched_ops = { .run_job = thames_job_run, > + .timedout_job = thames_job_timedout, > + .free_job = thames_job_free }; > + > +int thames_job_init(struct thames_core *core) > +{ > + struct drm_sched_init_args args = { > + .ops = &thames_sched_ops, > + .num_rqs = DRM_SCHED_PRIORITY_COUNT, > + .credit_limit = 1, > + .timeout = msecs_to_jiffies(JOB_TIMEOUT_MS), > + .name = dev_name(core->dev), > + .dev = core->dev, > + }; > + int ret; > + > + INIT_WORK(&core->reset.work, thames_reset_work); > + spin_lock_init(&core->fence_lock); > + mutex_init(&core->job_lock); > + > + core->reset.wq = alloc_ordered_workqueue("thames-reset-%d", 0, core->index); > + if (!core->reset.wq) > + return -ENOMEM; > + > + core->fence_context = dma_fence_context_alloc(1); > + > + args.timeout_wq = core->reset.wq; > + ret = drm_sched_init(&core->sched, &args); > + if (ret) { > + dev_err(core->dev, "Failed to create scheduler: %d.", ret); > + destroy_workqueue(core->reset.wq); > + return ret; > + } > + > + return 0; > +} > + > +void thames_job_fini(struct thames_core *core) > +{ > + drm_sched_fini(&core->sched); > + > + cancel_work_sync(&core->reset.work); > + destroy_workqueue(core->reset.wq); > +} > + > +int thames_job_open(struct thames_file_priv *thames_priv) > +{ > + struct thames_device *tdev = thames_priv->tdev; > + struct drm_gpu_scheduler **scheds = > + kmalloc_array(tdev->num_cores, sizeof(*scheds), GFP_KERNEL); > + unsigned int core; > + int ret; > + > + for (core = 0; core < tdev->num_cores; core++) > + scheds[core] = &tdev->cores[core].sched; > + > + ret = drm_sched_entity_init(&thames_priv->sched_entity, DRM_SCHED_PRIORITY_NORMAL, scheds, > + tdev->num_cores, NULL); > + if (WARN_ON(ret)) > + return ret; > + > + return 0; > +} > + > +void thames_job_close(struct thames_file_priv *thames_priv) > +{ > + struct drm_sched_entity *entity = &thames_priv->sched_entity; > + > + kfree(entity->sched_list); > + drm_sched_entity_destroy(entity); > +} > + > +static int thames_ioctl_submit_job(struct drm_device *dev, struct drm_file *file, > + struct drm_thames_job *job) > +{ > + struct thames_device *tdev = to_thames_device(dev); > + struct thames_file_priv *file_priv = file->driver_priv; > + struct thames_job *tjob = NULL; > + int ret = 0; > + > + tjob = kzalloc(sizeof(*tjob), GFP_KERNEL); > + if (!tjob) > + return -ENOMEM; > + > + kref_init(&tjob->refcount); > + > + tjob->tdev = tdev; > + tjob->file_priv = file_priv; > + > + tjob->job_id = ida_alloc_min(&tdev->job_ida, 1, GFP_KERNEL); > + if (tjob->job_id < 0) > + goto out_put_job; > + > + ret = drm_sched_job_init(&tjob->base, &file_priv->sched_entity, 1, NULL, file->client_id); > + if (ret) > + goto out_put_job; > + > + tjob->kernel = drm_gem_object_lookup(file, job->kernel); > + if (!tjob->kernel) { > + ret = -ENOENT; > + goto out_cleanup_job; > + } > + > + tjob->kernel_size = job->kernel_size; > + > + if (job->params) { > + tjob->params = drm_gem_object_lookup(file, job->params); > + if (!tjob->params) { > + ret = -ENOENT; > + goto out_cleanup_job; > + } > + tjob->params_size = job->params_size; > + } > + > + ret = drm_gem_objects_lookup(file, u64_to_user_ptr(job->in_bo_handles), > + job->in_bo_handle_count, &tjob->in_bos); > + if (ret) > + goto out_cleanup_job; > + > + tjob->in_bo_count = job->in_bo_handle_count; > + > + ret = drm_gem_objects_lookup(file, u64_to_user_ptr(job->out_bo_handles), > + job->out_bo_handle_count, &tjob->out_bos); > + if (ret) > + goto out_cleanup_job; > + > + tjob->out_bo_count = job->out_bo_handle_count; > + > + ret = thames_job_push(tjob); > + > +out_cleanup_job: > + if (ret) > + drm_sched_job_cleanup(&tjob->base); > +out_put_job: > + thames_job_put(tjob); > + > + return ret; > +} > + > +#define THAMES_MAX_JOBS_PER_SUBMIT 256 > + > +int thames_ioctl_submit(struct drm_device *dev, void *data, struct drm_file *file) > +{ > + struct drm_thames_submit *args = data; > + struct drm_thames_job *jobs; > + size_t jobs_size; > + int ret = 0; > + unsigned int i = 0; > + > + if (args->pad) > + return -EINVAL; > + > + if (args->job_count == 0) > + return -EINVAL; > + > + if (args->job_count > THAMES_MAX_JOBS_PER_SUBMIT) { > + dev_err(dev->dev, "Job count %u exceeds maximum %u\n", args->job_count, > + THAMES_MAX_JOBS_PER_SUBMIT); > + return -EINVAL; > + } > + > + jobs_size = array_size(args->job_count, sizeof(*jobs)); > + if (jobs_size == SIZE_MAX) > + return -EINVAL; > + > + jobs = kvmalloc_array(args->job_count, sizeof(*jobs), GFP_KERNEL); > + if (!jobs) > + return -ENOMEM; > + > + if (copy_from_user(jobs, u64_to_user_ptr(args->jobs), jobs_size)) { > + ret = -EFAULT; > + drm_dbg(dev, "Failed to copy incoming job array\n"); > + goto exit; > + } > + > + for (i = 0; i < args->job_count; i++) { > + ret = thames_ioctl_submit_job(dev, file, &jobs[i]); > + if (ret) > + break; > + } > + > +exit: > + kvfree(jobs); > + > + return ret; > +} > diff --git a/drivers/accel/thames/thames_job.h b/drivers/accel/thames/thames_job.h > new file mode 100644 > index 0000000000000000000000000000000000000000..3bfd2c779d9b783624a25e6d06368f3e1daf569e > --- /dev/null > +++ b/drivers/accel/thames/thames_job.h > @@ -0,0 +1,51 @@ > +/* SPDX-License-Identifier: GPL-2.0-only */ > +/* Copyright 2024-2025 Tomeu Vizoso <tomeu(a)tomeuvizoso.net> */ > +/* Copyright 2026 Texas Instruments Incorporated - https://www.ti.com/ */ > + > +#ifndef __THAMES_JOB_H__ > +#define __THAMES_JOB_H__ > + > +#include <drm/drm_drv.h> > +#include <drm/gpu_scheduler.h> > + > +#include "thames_core.h" > +#include "thames_drv.h" > + > +struct thames_job { > + struct drm_sched_job base; > + > + struct thames_device *tdev; > + struct thames_file_priv *file_priv; > + > + u32 job_id; > + u32 ipc_sequence; > + > + struct drm_gem_object *kernel; > + size_t kernel_size; > + > + struct drm_gem_object *params; > + size_t params_size; > + > + struct drm_gem_object **in_bos; > + u32 in_bo_count; > + > + struct drm_gem_object **out_bos; > + u32 out_bo_count; > + > + /* Fence to be signaled by drm-sched once its done with the job */ > + struct dma_fence *inference_done_fence; > + > + /* Fence to be signaled by rpmsg handler when the job is complete. */ > + struct dma_fence *done_fence; > + > + struct kref refcount; > +}; > + > +int thames_ioctl_submit(struct drm_device *dev, void *data, struct drm_file *file); > + > +int thames_job_init(struct thames_core *core); > +void thames_job_fini(struct thames_core *core); > +int thames_job_open(struct thames_file_priv *thames_priv); > +void thames_job_close(struct thames_file_priv *thames_priv); > + > +#endif > diff --git a/drivers/accel/thames/thames_rpmsg.c b/drivers/accel/thames/thames_rpmsg.c > index a25465295a177877c5ca2b3c93f52d8288863797..9747690e0f84fe00d605ad0e708d597da2240d97 100644 > --- a/drivers/accel/thames/thames_rpmsg.c > +++ b/drivers/accel/thames/thames_rpmsg.c > @@ -11,6 +11,7 @@ > #include "thames_core.h" > #include "thames_device.h" > #include "thames_ipc.h" > +#include "thames_job.h" > > #define THAMES_PING_TEST_PATTERN 0xDEADBEEF > #define THAMES_PING_TIMEOUT_MS 5000 > @@ -71,6 +72,36 @@ static int thames_rpmsg_callback(struct rpmsg_device *rpdev, void *data, int len > ida_free(&core->tdev->ipc_seq_ida, hdr->seq); > break; > > + case THAMES_MSG_SUBMIT_JOB_RESPONSE: { > + struct thames_job *job; > + > + scoped_guard(mutex, &core->job_lock) > + { > + job = core->in_flight_job; > + if (!job) { > + dev_err(&rpdev->dev, > + "Received job response but no job in flight\n"); > + ida_free(&core->tdev->ipc_seq_ida, hdr->seq); > + return -EINVAL; > + } > + > + if (hdr->seq != job->ipc_sequence) { > + dev_err(&rpdev->dev, > + "Job response sequence mismatch: got %u, expected %u\n", > + hdr->seq, job->ipc_sequence); > + ida_free(&core->tdev->ipc_seq_ida, hdr->seq); > + return -EINVAL; > + } > + > + dma_fence_signal(job->done_fence); > + core->in_flight_job = NULL; > + } > + > + ida_free(&core->tdev->ipc_seq_ida, hdr->seq); > + > + break; > + } > + > default: > dev_warn(&rpdev->dev, "Unknown message type: %u\n", hdr->type); > break; > @@ -191,6 +222,27 @@ int thames_rpmsg_send_unmap_bo(struct thames_core *core, u32 context_id, u32 bo_ > return thames_rpmsg_send_raw(core, &msg, sizeof(msg)); > } > > +int thames_rpmsg_send_submit_job(struct thames_core *core, u32 context_id, u32 job_id, > + u64 kernel_iova, u64 kernel_size, u64 args_iova, u64 args_size, > + u32 *sequence) > +{ > + struct thames_msg_submit_job msg = {}; > + > + msg.hdr.type = THAMES_MSG_SUBMIT_JOB; > + msg.hdr.seq = ida_alloc(&core->tdev->ipc_seq_ida, GFP_KERNEL); > + msg.hdr.len = sizeof(msg); > + msg.context_id = context_id; > + msg.job_id = job_id; > + msg.kernel_iova = kernel_iova; > + msg.kernel_size = kernel_size; > + msg.args_iova = args_iova; > + msg.args_size = args_size; > + > + *sequence = msg.hdr.seq; > + > + return thames_rpmsg_send_raw(core, &msg, sizeof(msg)); > +} > + > int thames_rpmsg_ping_test(struct thames_core *core) > { > const u32 test_data = THAMES_PING_TEST_PATTERN; >

2 weeks, 5 days

Re: [PATCH 2/5] accel/thames: Add driver for the C7x DSPs in TI SoCs

by Robert Nelson

On Tue, Jan 13, 2026 at 11:45 AM Tomeu Vizoso <tomeu(a)tomeuvizoso.net> wrote: > > Some SoCs from Texas Instruments contain DSPs that can be used for > general compute tasks. > > This driver provides a drm/accel UABI to userspace for submitting jobs > to the DSP cores and managing the input, output and intermediate memory. > > Signed-off-by: Tomeu Vizoso <tomeu(a)tomeuvizoso.net> > --- > Documentation/accel/thames/index.rst | 28 +++++ > MAINTAINERS | 9 ++ > drivers/accel/Kconfig | 1 + > drivers/accel/Makefile | 3 +- > drivers/accel/thames/Kconfig | 26 +++++ > drivers/accel/thames/Makefile | 9 ++ > drivers/accel/thames/thames_core.c | 155 ++++++++++++++++++++++++++ > drivers/accel/thames/thames_core.h | 53 +++++++++ > drivers/accel/thames/thames_device.c | 93 ++++++++++++++++ > drivers/accel/thames/thames_device.h | 46 ++++++++ > drivers/accel/thames/thames_drv.c | 156 +++++++++++++++++++++++++++ > drivers/accel/thames/thames_drv.h | 21 ++++ > drivers/accel/thames/thames_ipc.h | 204 +++++++++++++++++++++++++++++++++++ > drivers/accel/thames/thames_rpmsg.c | 155 ++++++++++++++++++++++++++ > drivers/accel/thames/thames_rpmsg.h | 27 +++++ > 15 files changed, 985 insertions(+), 1 deletion(-) > > diff --git a/Documentation/accel/thames/index.rst b/Documentation/accel/thames/index.rst > new file mode 100644 > index 0000000000000000000000000000000000000000..ca8391031f226f7ef1dc210a356c86acbe126c6f > --- /dev/null > +++ b/Documentation/accel/thames/index.rst > @@ -0,0 +1,28 @@ > +.. SPDX-License-Identifier: GPL-2.0-only > + > +============================================================ > + accel/thames Driver for the C7x DSPs from Texas Instruments > +============================================================ > + > +The accel/thames driver supports the C7x DSPs inside some Texas Instruments SoCs > +such as the J722S. These can be used as accelerators for various workloads, > +including machine learning inference. > + > +This driver controls the power state of the hardware via :doc:`remoteproc </staging/remoteproc>` > +and communicates with the firmware running on the DSP via :doc:`rpmsg_virtio </staging/rpmsg_virtio>`. > +The kernel driver itself allocates buffers, manages contexts, and submits jobs > +to the DSP firmware. Buffers are mapped by the DSP itself using its MMU, > +providing memory isolation among different clients. > + > +The source code for the firmware running on the DSP is available at: > +https://gitlab.freedesktop.org/tomeu/thames_firmware/. > + > +Everything else is done in userspace, as a Gallium driver (also called thames) > +that is part of the Mesa3D project: https://docs.mesa3d.org/teflon.html > + > +If there is more than one core that advertises the same rpmsg_virtio service > +name, the driver will load balance jobs between them with drm-gpu-scheduler. > + > +Hardware currently supported: > + > +* J722S > diff --git a/MAINTAINERS b/MAINTAINERS > index dc731d37c8feeff25613c59fe9c929927dadaa7e..a3fc809c797269d0792dfe5202cc1b49f6ff57e9 100644 > --- a/MAINTAINERS > +++ b/MAINTAINERS > @@ -7731,6 +7731,15 @@ F: Documentation/devicetree/bindings/npu/rockchip,rk3588-rknn-core.yaml > F: drivers/accel/rocket/ > F: include/uapi/drm/rocket_accel.h > > +DRM ACCEL DRIVER FOR TI C7x DSPS > +M: Tomeu Vizoso <tomeu(a)tomeuvizoso.net> > +L: dri-devel(a)lists.freedesktop.org > +S: Supported > +T: git https://gitlab.freedesktop.org/drm/misc/kernel.git > +F: Documentation/accel/thames/ > +F: drivers/accel/thames/ > +F: include/uapi/drm/thames_accel.h Oh where is this "thames_accel.h" ? ;) 2026-01-13T18:16:11.881084Z 01E drivers/accel/thames/thames_drv.c:8:10: fatal error: drm/thames_accel.h: No such file or directory 2026-01-13T18:16:11.881086Z 01E 8 | #include <drm/thames_accel.h> 2026-01-13T18:16:11.881087Z 01E | ^~~~~~~~~~~~~~~~~~~~ 2026-01-13T18:16:11.881115Z 01E compilation terminated. 2026-01-13T18:16:11.884552Z 01E make[8]: *** [scripts/Makefile.build:287: drivers/accel/thames/thames_drv.o] Error 1 2026-01-13T18:16:11.884694Z 01E make[7]: *** [scripts/Makefile.build:544: drivers/accel/thames] Error 2 2026-01-13T18:16:11.884926Z 01E make[6]: *** [scripts/Makefile.build:544: drivers/accel] Error 2 2026-01-13T18:16:11.884976Z 01E make[6]: *** Waiting for unfinished jobs.... $ find . | grep thames_accel.h $ grep -R "thames_accel.h" ./* ./drivers/accel/thames/Kconfig: include/uapi/drm/thames_accel.h and is used by the Thames userspace ./drivers/accel/thames/thames_job.c:#include <drm/thames_accel.h> ./drivers/accel/thames/thames_drv.c:#include <drm/thames_accel.h> ./drivers/accel/thames/thames_gem.c:#include <drm/thames_accel.h> ./MAINTAINERS:F: include/uapi/drm/thames_accel.h Regards, -- Robert Nelson https://rcn-ee.com/

2 weeks, 5 days

[PATCH 0/4] dma-buf: add revoke mechanism to invalidate shared buffers

by Leon Romanovsky

This series implements a dma-buf “revoke” mechanism: to allow a dma-buf exporter to explicitly invalidate (“kill”) a shared buffer after it has been distributed to importers, so that further CPU and device access is prevented and importers reliably observe failure. Today, dma-buf effectively provides “if you have the fd, you can keep using the memory indefinitely.” That assumption breaks down when an exporter must reclaim, reset, evict, or otherwise retire backing memory after it has been shared. Concrete cases include GPU reset and recovery where old allocations become unsafe to access, memory eviction/overcommit where backing storage must be withdrawn, and security or isolation situations where continued access must be prevented. While drivers can sometimes approximate this with exporter-specific fencing and policy, there is no core dma-buf state transition that communicates “this buffer is no longer valid; fail access” across all access paths. The change in this series is to introduce a core “revoked” state on the dma-buf object and a corresponding exporter-triggered revoke operation. Once a dma-buf is revoked, new access paths are blocked so that attempts to DMA-map, vmap, or mmap the buffer fail in a consistent way. In addition, the series aims to invalidate existing access as much as the kernel allows: device mappings are torn down where possible so devices and IOMMUs cannot continue DMA. The semantics are intentionally simple: revoke is a one-way, permanent transition for the lifetime of that dma-buf instance. From a compatibility perspective, users that never invoke revoke are unaffected, and exporters that adopt it gain a core-supported enforcement mechanism rather than relying on ad hoc driver behavior. The intent is to keep the interface minimal and avoid imposing policy; the series provides the mechanism to terminate access, with policy remaining in the exporter and higher-level components. BTW, see this megathread [1] for additional context. Ironically, it was posted exactly one year ago. [1] https://lore.kernel.org/all/20250107142719.179636-2-yilun.xu@linux.intel.co… Thanks Cc: linux-rdma(a)vger.kernel.org Cc: linux-kernel(a)vger.kernel.org Cc: linux-media(a)vger.kernel.org Cc: dri-devel(a)lists.freedesktop.org Cc: linaro-mm-sig(a)lists.linaro.org Cc: kvm(a)vger.kernel.org Cc: iommu(a)lists.linux.dev To: Jason Gunthorpe <jgg(a)ziepe.ca> To: Leon Romanovsky <leon(a)kernel.org> To: Sumit Semwal <sumit.semwal(a)linaro.org> To: Christian König <christian.koenig(a)amd.com> To: Alex Williamson <alex(a)shazbot.org> To: Kevin Tian <kevin.tian(a)intel.com> To: Joerg Roedel <joro(a)8bytes.org> To: Will Deacon <will(a)kernel.org> To: Robin Murphy <robin.murphy(a)arm.com> Signed-off-by: Leon Romanovsky <leonro(a)nvidia.com> --- Leon Romanovsky (4): dma-buf: Introduce revoke semantics vfio: Use dma-buf revoke semantics iommufd: Require DMABUF revoke semantics iommufd/selftest: Reuse dma-buf revoke semantics drivers/dma-buf/dma-buf.c | 36 ++++++++++++++++++++++++++++++++---- drivers/iommu/iommufd/pages.c | 2 +- drivers/iommu/iommufd/selftest.c | 12 ++++-------- drivers/vfio/pci/vfio_pci_dmabuf.c | 27 ++++++--------------------- include/linux/dma-buf.h | 31 +++++++++++++++++++++++++++++++ 5 files changed, 74 insertions(+), 34 deletions(-) --- base-commit: 9ace4753a5202b02191d54e9fdf7f9e3d02b85eb change-id: 20251221-dmabuf-revoke-b90ef16e4236 Best regards, -- Leon Romanovsky <leonro(a)nvidia.com>

2 weeks, 6 days

Re: [RFC v2 01/11] file: add callback for pre-mapping dmabuf

by Christoph Hellwig

On Fri, Jan 09, 2026 at 10:10:57AM +0800, Ming Lei wrote: > On Thu, Jan 08, 2026 at 11:17:03AM +0100, Christoph Hellwig wrote: > > On Thu, Jan 08, 2026 at 10:19:18AM +0800, Ming Lei wrote: > > > > The feature is in no way nvme specific. nvme is just the initial > > > > underlying driver. It makes total sense to support this for any high > > > > performance block device, and to pass it through file systems. > > > > > > But why does FS care the dma buffer attachment? Since high performance > > > host controller is exactly the dma buffer attachment point. > > > > I can't parse what you're trying to say here. > > dma buffer attachment is simply none of FS's business. The file systems should indeed never do a dma buffer attachment itself, but that's not the point. > > But even when not stacking, the registration still needs to go > > through the file system even for a single device, never mind multiple > > controlled by the file system. > > dma_buf can have multiple importers, so why does it have to go through FS for > single device only? > > If the registered buffer is attached to single device before going > through FS, it can not support stacking block device, and it can't or not > easily to use for multiple block device, no matter if they are behind same > host controller or multiple. Because the file system, or the file_operations instance to be more specific, is the only entity that known what block device(s) or other DMA capable device(s) like (R)NIC a file maps to.

3 weeks, 3 days

Re: [RFC v2 01/11] file: add callback for pre-mapping dmabuf

by Christoph Hellwig

On Thu, Jan 08, 2026 at 10:19:18AM +0800, Ming Lei wrote: > > The feature is in no way nvme specific. nvme is just the initial > > underlying driver. It makes total sense to support this for any high > > performance block device, and to pass it through file systems. > > But why does FS care the dma buffer attachment? Since high performance > host controller is exactly the dma buffer attachment point. I can't parse what you're trying to say here. > If the callback is added in `struct file_operations` for wiring dma buffer > and the importer(host contrller), you will see it is hard to let it cross device > mapper/raid or other stackable block devices. Why? But even when not stacking, the registration still needs to go through the file system even for a single device, never mind multiple controlled by the file system.

3 weeks, 3 days

Re: [RFC v2 01/11] file: add callback for pre-mapping dmabuf

by Christian König

On 1/4/26 02:42, Ming Lei wrote: > On Thu, Dec 04, 2025 at 02:10:25PM +0100, Christoph Hellwig wrote: >> On Thu, Dec 04, 2025 at 12:09:46PM +0100, Christian König wrote: >>>> I find the naming pretty confusing a well. But what this does is to >>>> tell the file system/driver that it should expect a future >>>> read_iter/write_iter operation that takes data from / puts data into >>>> the dmabuf passed to this operation. >>> >>> That explanation makes much more sense. >>> >>> The remaining question is why does the underlying file system / driver >>> needs to know that it will get addresses from a DMA-buf? >> >> This eventually ends up calling dma_buf_dynamic_attach and provides >> a way to find the dma_buf_attachment later in the I/O path. > > Maybe it can be named as ->dma_buf_attach()? For wiring dma-buf and the > importer side(nvme). Yeah that would make it much more cleaner. Also some higher level documentation would certainly help. > But I am wondering why not make it as one subsystem interface, such as nvme > ioctl, then the whole implementation can be simplified a lot. It is reasonable > because subsystem is exactly the side for consuming/importing the dma-buf. Yeah that it might be better if it's more nvme specific came to me as well. Regards, Christian. > > > Thanks, > Ming >

3 weeks, 4 days

Re: [PATCH] dma-buf: system_heap: account for system heap allocation in memcg

by Christian König

On 12/19/25 16:58, Maxime Ripard wrote: > On Fri, Dec 19, 2025 at 02:50:50PM +0100, Christian König wrote: >> On 12/19/25 11:25, Maxime Ripard wrote: >>> On Mon, Dec 15, 2025 at 03:53:22PM +0100, Christian König wrote: >>>> On 12/15/25 14:59, Maxime Ripard wrote: >> ... >>>>>>> The shared ownership is indeed broken, but it's not more or less broken >>>>>>> than, say, memfd + udmabuf, and I'm sure plenty of others. >>>>>>> >>>>>>> So we really improve the common case, but only make the "advanced" >>>>>>> slightly more broken than it already is. >>>>>>> >>>>>>> Would you disagree? >>>>>> >>>>>> I strongly disagree. As far as I can see there is a huge chance we >>>>>> break existing use cases with that. >>>>> >>>>> Which ones? And what about the ones that are already broken? >>>> >>>> Well everybody that expects that driver resources are *not* accounted to memcg. >>> >>> Which is a thing only because these buffers have never been accounted >>> for in the first place. >> >> Yeah, completely agree. By not accounting it for such a long time we >> ended up with people depending on this behavior. >> >> Not nice, but that's what it is. >> >>> So I guess the conclusion is that we shouldn't >>> even try to do memory accounting, because someone somewhere might not >>> expect that one of its application would take too much RAM in the >>> system? >> >> Well we do need some kind of solution to the problem. Either having >> some setting where you say "This memcg limit is inclusive/exclusive >> device driver allocated memory" or have a completely separate limit >> for device driver allocated memory. > > A device driver memory specific limit sounds like a good idea because it > would make it easier to bridge the gap with dmem. Completely agree, but that approach was rejected by the cgroups people. I mean we can already use udmabuf to allocate memcg accounted system memory which then can be imported into device drivers. So I don't see much reason why we should account dma-buf heaps and driver interfaces to memcg as well, we just need some way to limit them. Regards, Christian. > > Happy holidays, > Maxime

3 weeks, 4 days

Re: [RFC v2 01/11] file: add callback for pre-mapping dmabuf

by Christoph Hellwig

On Tue, Jan 06, 2026 at 07:51:12PM +0000, Pavel Begunkov wrote: >> But I am wondering why not make it as one subsystem interface, such as nvme >> ioctl, then the whole implementation can be simplified a lot. It is reasonable >> because subsystem is exactly the side for consuming/importing the dma-buf. > > It's not an nvme specific interface, and so a file op was much more > convenient. It is the much better abstraction. Also the nvme subsystems is not an actor, and registering things to the subsystems does not work. The nvme controller is the entity that does the dma mapping, and this interface works very well for that.

3 weeks, 5 days

← Newer
1
2
3
4
5
6
7
8
9
...
331
Older →

Jump to page:

2026

2025

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

2012

2011

Linaro-mm-sig