Linaro-mm-sig

linaro-mm-sig@lists.linaro.org

11 participants
2933 discussions

Re: [PATCH v10 6/9] tee: add tee_shm_alloc_dma_mem()

by Jens Wiklander

On Tue, Jun 17, 2025 at 1:32 PM Sumit Garg <sumit.garg(a)kernel.org> wrote: > > On Tue, Jun 10, 2025 at 03:13:50PM +0200, Jens Wiklander wrote: > > Add tee_shm_alloc_dma_mem() to allocate DMA memory. The memory is > > represented by a tee_shm object using the new flag TEE_SHM_DMA_MEM to > > identify it as DMA memory. The allocated memory will later be lent to > > the TEE to be used as protected memory. > > > > Signed-off-by: Jens Wiklander <jens.wiklander(a)linaro.org> > > --- > > drivers/tee/tee_shm.c | 85 +++++++++++++++++++++++++++++++++++++++- > > include/linux/tee_core.h | 5 +++ > > 2 files changed, 88 insertions(+), 2 deletions(-) > > > > diff --git a/drivers/tee/tee_shm.c b/drivers/tee/tee_shm.c > > index e63095e84644..60b0f3932cee 100644 > > --- a/drivers/tee/tee_shm.c > > +++ b/drivers/tee/tee_shm.c > > @@ -5,6 +5,8 @@ > > #include <linux/anon_inodes.h> > > #include <linux/device.h> > > #include <linux/dma-buf.h> > > +#include <linux/dma-mapping.h> > > +#include <linux/highmem.h> > > #include <linux/idr.h> > > #include <linux/io.h> > > #include <linux/mm.h> > > @@ -13,9 +15,14 @@ > > #include <linux/tee_core.h> > > #include <linux/uaccess.h> > > #include <linux/uio.h> > > -#include <linux/highmem.h> > > #include "tee_private.h" > > > > +struct tee_shm_dma_mem { > > + struct tee_shm shm; > > + dma_addr_t dma_addr; > > + struct page *page; > > +}; > > + > > static void shm_put_kernel_pages(struct page **pages, size_t page_count) > > { > > size_t n; > > @@ -48,7 +55,16 @@ static void tee_shm_release(struct tee_device *teedev, struct tee_shm *shm) > > { > > void *p = shm; > > > > - if (shm->flags & TEE_SHM_DMA_BUF) { > > + if (shm->flags & TEE_SHM_DMA_MEM) { > > +#if IS_ENABLED(CONFIG_TEE_DMABUF_HEAPS) > > nit: this config check can be merged into the above if check. No, because dma_free_pages() is only defined if CONFIG_TEE_DMABUF_HEAPS is enabled. > > > + struct tee_shm_dma_mem *dma_mem; > > + > > + dma_mem = container_of(shm, struct tee_shm_dma_mem, shm); > > + p = dma_mem; > > + dma_free_pages(&teedev->dev, shm->size, dma_mem->page, > > + dma_mem->dma_addr, DMA_BIDIRECTIONAL); > > +#endif > > + } else if (shm->flags & TEE_SHM_DMA_BUF) { > > Do we need a similar config check for this flag too? No, because DMA_SHARED_BUFFER is selected, so the dma_buf functions are defined. Cheers, Jens > > With these addressed, feel free to add: > > Reviewed-by: Sumit Garg <sumit.garg(a)oss.qualcomm.com> > > -Sumit > > > struct tee_shm_dmabuf_ref *ref; > > > > ref = container_of(shm, struct tee_shm_dmabuf_ref, shm); > > @@ -303,6 +319,71 @@ struct tee_shm *tee_shm_alloc_priv_buf(struct tee_context *ctx, size_t size) > > } > > EXPORT_SYMBOL_GPL(tee_shm_alloc_priv_buf); > > > > +#if IS_ENABLED(CONFIG_TEE_DMABUF_HEAPS) > > +/** > > + * tee_shm_alloc_dma_mem() - Allocate DMA memory as shared memory object > > + * @ctx: Context that allocates the shared memory > > + * @page_count: Number of pages > > + * > > + * The allocated memory is expected to be lent (made inaccessible to the > > + * kernel) to the TEE while it's used and returned (accessible to the > > + * kernel again) before it's freed. > > + * > > + * This function should normally only be used internally in the TEE > > + * drivers. > > + * > > + * @returns a pointer to 'struct tee_shm' > > + */ > > +struct tee_shm *tee_shm_alloc_dma_mem(struct tee_context *ctx, > > + size_t page_count) > > +{ > > + struct tee_device *teedev = ctx->teedev; > > + struct tee_shm_dma_mem *dma_mem; > > + dma_addr_t dma_addr; > > + struct page *page; > > + > > + if (!tee_device_get(teedev)) > > + return ERR_PTR(-EINVAL); > > + > > + page = dma_alloc_pages(&teedev->dev, page_count * PAGE_SIZE, > > + &dma_addr, DMA_BIDIRECTIONAL, GFP_KERNEL); > > + if (!page) > > + goto err_put_teedev; > > + > > + dma_mem = kzalloc(sizeof(*dma_mem), GFP_KERNEL); > > + if (!dma_mem) > > + goto err_free_pages; > > + > > + refcount_set(&dma_mem->shm.refcount, 1); > > + dma_mem->shm.ctx = ctx; > > + dma_mem->shm.paddr = page_to_phys(page); > > + dma_mem->dma_addr = dma_addr; > > + dma_mem->page = page; > > + dma_mem->shm.size = page_count * PAGE_SIZE; > > + dma_mem->shm.flags = TEE_SHM_DMA_MEM; > > + > > + teedev_ctx_get(ctx); > > + > > + return &dma_mem->shm; > > + > > +err_free_pages: > > + dma_free_pages(&teedev->dev, page_count * PAGE_SIZE, page, dma_addr, > > + DMA_BIDIRECTIONAL); > > +err_put_teedev: > > + tee_device_put(teedev); > > + > > + return ERR_PTR(-ENOMEM); > > +} > > +EXPORT_SYMBOL_GPL(tee_shm_alloc_dma_mem); > > +#else > > +struct tee_shm *tee_shm_alloc_dma_mem(struct tee_context *ctx, > > + size_t page_count) > > +{ > > + return ERR_PTR(-EINVAL); > > +} > > +EXPORT_SYMBOL_GPL(tee_shm_alloc_dma_mem); > > +#endif > > + > > int tee_dyn_shm_alloc_helper(struct tee_shm *shm, size_t size, size_t align, > > int (*shm_register)(struct tee_context *ctx, > > struct tee_shm *shm, > > diff --git a/include/linux/tee_core.h b/include/linux/tee_core.h > > index f17710196c4c..e46a53e753af 100644 > > --- a/include/linux/tee_core.h > > +++ b/include/linux/tee_core.h > > @@ -29,6 +29,8 @@ > > #define TEE_SHM_POOL BIT(2) /* Memory allocated from pool */ > > #define TEE_SHM_PRIV BIT(3) /* Memory private to TEE driver */ > > #define TEE_SHM_DMA_BUF BIT(4) /* Memory with dma-buf handle */ > > +#define TEE_SHM_DMA_MEM BIT(5) /* Memory allocated with */ > > + /* dma_alloc_pages() */ > > > > #define TEE_DEVICE_FLAG_REGISTERED 0x1 > > #define TEE_MAX_DEV_NAME_LEN 32 > > @@ -310,6 +312,9 @@ void *tee_get_drvdata(struct tee_device *teedev); > > */ > > struct tee_shm *tee_shm_alloc_priv_buf(struct tee_context *ctx, size_t size); > > > > +struct tee_shm *tee_shm_alloc_dma_mem(struct tee_context *ctx, > > + size_t page_count); > > + > > int tee_dyn_shm_alloc_helper(struct tee_shm *shm, size_t size, size_t align, > > int (*shm_register)(struct tee_context *ctx, > > struct tee_shm *shm, > > -- > > 2.43.0 > >

2 weeks, 6 days

Re: [PATCH v10 5/9] tee: new ioctl to a register tee_shm from a dmabuf file descriptor

by Jens Wiklander

On Tue, Jun 17, 2025 at 12:48 PM Sumit Garg <sumit.garg(a)kernel.org> wrote: > > On Tue, Jun 10, 2025 at 03:13:49PM +0200, Jens Wiklander wrote: > > From: Etienne Carriere <etienne.carriere(a)foss.st.com> > > > > Add a userspace API to create a tee_shm object that refers to a dmabuf > > reference. > > > > Userspace registers the dmabuf file descriptor as in a tee_shm object. > > The registration is completed with a tee_shm returned file descriptor. > > > > Userspace is free to close the dmabuf file descriptor after it has been > > registered since all the resources are now held via the new tee_shm > > object. > > > > Closing the tee_shm file descriptor will eventually release all > > resources used by the tee_shm object when all references are released. > > > > The new IOCTL, TEE_IOC_SHM_REGISTER_FD, supports dmabuf references to > > physically contiguous memory buffers. Dmabuf references acquired from > > the TEE DMA-heap can be used as protected memory for Secure Video Path > > and such use cases. It depends on the TEE and the TEE driver if dmabuf > > references acquired by other means can be used. > > > > A new tee_shm flag is added to identify tee_shm objects built from a > > registered dmabuf, TEE_SHM_DMA_BUF. > > > > Signed-off-by: Etienne Carriere <etienne.carriere(a)foss.st.com> > > Signed-off-by: Olivier Masse <olivier.masse(a)nxp.com> > > Signed-off-by: Jens Wiklander <jens.wiklander(a)linaro.org> > > --- > > drivers/tee/tee_core.c | 63 +++++++++++++++++++++- > > drivers/tee/tee_private.h | 10 ++++ > > drivers/tee/tee_shm.c | 106 ++++++++++++++++++++++++++++++++++++-- > > include/linux/tee_core.h | 1 + > > include/linux/tee_drv.h | 10 ++++ > > include/uapi/linux/tee.h | 31 +++++++++++ > > 6 files changed, 217 insertions(+), 4 deletions(-) > > > > diff --git a/drivers/tee/tee_core.c b/drivers/tee/tee_core.c > > index 5259b8223c27..0e9d9e5872a4 100644 > > --- a/drivers/tee/tee_core.c > > +++ b/drivers/tee/tee_core.c > > @@ -353,11 +353,49 @@ tee_ioctl_shm_register(struct tee_context *ctx, > > return ret; > > } > > > > +static int > > +tee_ioctl_shm_register_fd(struct tee_context *ctx, > > + struct tee_ioctl_shm_register_fd_data __user *udata) > > +{ > > + struct tee_ioctl_shm_register_fd_data data; > > + struct tee_shm *shm; > > + long ret; > > + > > + if (copy_from_user(&data, udata, sizeof(data))) > > + return -EFAULT; > > + > > + /* Currently no input flags are supported */ > > + if (data.flags) > > + return -EINVAL; > > + > > + shm = tee_shm_register_fd(ctx, data.fd); > > + if (IS_ERR(shm)) > > + return -EINVAL; > > + > > + data.id = shm->id; > > + data.flags = shm->flags; > > + data.size = shm->size; > > + > > + if (copy_to_user(udata, &data, sizeof(data))) > > + ret = -EFAULT; > > + else > > + ret = tee_shm_get_fd(shm); > > + > > + /* > > + * When user space closes the file descriptor the shared memory > > + * should be freed or if tee_shm_get_fd() failed then it will > > + * be freed immediately. > > + */ > > + tee_shm_put(shm); > > + return ret; > > +} > > + > > static int param_from_user_memref(struct tee_context *ctx, > > struct tee_param_memref *memref, > > struct tee_ioctl_param *ip) > > { > > struct tee_shm *shm; > > + size_t offs = 0; > > > > /* > > * If a NULL pointer is passed to a TA in the TEE, > > @@ -388,6 +426,26 @@ static int param_from_user_memref(struct tee_context *ctx, > > tee_shm_put(shm); > > return -EINVAL; > > } > > + > > + if (shm->flags & TEE_SHM_DMA_BUF) { > > + struct tee_shm_dmabuf_ref *ref; > > + > > + ref = container_of(shm, struct tee_shm_dmabuf_ref, shm); > > + if (ref->parent_shm) { > > + /* > > + * The shm already has one reference to > > + * ref->parent_shm so we are clear of 0. > > + * We're getting another reference since > > + * this shm will be used in the parameter > > + * list instead of the shm we got with > > + * tee_shm_get_from_id() above. > > + */ > > + refcount_inc(&ref->parent_shm->refcount); > > + tee_shm_put(shm); > > + shm = ref->parent_shm; > > + offs = ref->offset; > > + } > > + } > > } else if (ctx->cap_memref_null) { > > /* Pass NULL pointer to OP-TEE */ > > shm = NULL; > > @@ -395,7 +453,7 @@ static int param_from_user_memref(struct tee_context *ctx, > > return -EINVAL; > > } > > > > - memref->shm_offs = ip->a; > > + memref->shm_offs = ip->a + offs; > > memref->size = ip->b; > > memref->shm = shm; > > > > @@ -841,6 +899,8 @@ static long tee_ioctl(struct file *filp, unsigned int cmd, unsigned long arg) > > return tee_ioctl_shm_alloc(ctx, uarg); > > case TEE_IOC_SHM_REGISTER: > > return tee_ioctl_shm_register(ctx, uarg); > > + case TEE_IOC_SHM_REGISTER_FD: > > + return tee_ioctl_shm_register_fd(ctx, uarg); > > case TEE_IOC_OPEN_SESSION: > > return tee_ioctl_open_session(ctx, uarg); > > case TEE_IOC_INVOKE: > > @@ -1300,3 +1360,4 @@ MODULE_AUTHOR("Linaro"); > > MODULE_DESCRIPTION("TEE Driver"); > > MODULE_VERSION("1.0"); > > MODULE_LICENSE("GPL v2"); > > +MODULE_IMPORT_NS("DMA_BUF"); > > diff --git a/drivers/tee/tee_private.h b/drivers/tee/tee_private.h > > index 6c6ff5d5eed2..308467705da6 100644 > > --- a/drivers/tee/tee_private.h > > +++ b/drivers/tee/tee_private.h > > @@ -13,6 +13,16 @@ > > #include <linux/mutex.h> > > #include <linux/types.h> > > > > +/* extra references appended to shm object for registered shared memory */ > > +struct tee_shm_dmabuf_ref { > > + struct tee_shm shm; > > + size_t offset; > > + struct dma_buf *dmabuf; > > + struct dma_buf_attachment *attach; > > + struct sg_table *sgt; > > + struct tee_shm *parent_shm; > > +}; > > + > > int tee_shm_get_fd(struct tee_shm *shm); > > > > bool tee_device_get(struct tee_device *teedev); > > diff --git a/drivers/tee/tee_shm.c b/drivers/tee/tee_shm.c > > index daf6e5cfd59a..e63095e84644 100644 > > --- a/drivers/tee/tee_shm.c > > +++ b/drivers/tee/tee_shm.c > > @@ -4,6 +4,7 @@ > > */ > > #include <linux/anon_inodes.h> > > #include <linux/device.h> > > +#include <linux/dma-buf.h> > > #include <linux/idr.h> > > #include <linux/io.h> > > #include <linux/mm.h> > > @@ -45,7 +46,21 @@ static void release_registered_pages(struct tee_shm *shm) > > > > static void tee_shm_release(struct tee_device *teedev, struct tee_shm *shm) > > { > > - if (shm->flags & TEE_SHM_POOL) { > > + void *p = shm; > > + > > + if (shm->flags & TEE_SHM_DMA_BUF) { > > + struct tee_shm_dmabuf_ref *ref; > > + > > + ref = container_of(shm, struct tee_shm_dmabuf_ref, shm); > > + p = ref; > > + if (ref->attach) { > > + dma_buf_unmap_attachment(ref->attach, ref->sgt, > > + DMA_BIDIRECTIONAL); > > + > > + dma_buf_detach(ref->dmabuf, ref->attach); > > + } > > + dma_buf_put(ref->dmabuf); > > + } else if (shm->flags & TEE_SHM_POOL) { > > teedev->pool->ops->free(teedev->pool, shm); > > } else if (shm->flags & TEE_SHM_DYNAMIC) { > > int rc = teedev->desc->ops->shm_unregister(shm->ctx, shm); > > @@ -59,7 +74,7 @@ static void tee_shm_release(struct tee_device *teedev, struct tee_shm *shm) > > > > teedev_ctx_put(shm->ctx); > > > > - kfree(shm); > > + kfree(p); > > > > tee_device_put(teedev); > > } > > @@ -169,7 +184,7 @@ struct tee_shm *tee_shm_alloc_user_buf(struct tee_context *ctx, size_t size) > > * tee_client_invoke_func(). The memory allocated is later freed with a > > * call to tee_shm_free(). > > * > > - * @returns a pointer to 'struct tee_shm' > > + * @returns a pointer to 'struct tee_shm' on success, and ERR_PTR on failure > > */ > > struct tee_shm *tee_shm_alloc_kernel_buf(struct tee_context *ctx, size_t size) > > { > > @@ -179,6 +194,91 @@ struct tee_shm *tee_shm_alloc_kernel_buf(struct tee_context *ctx, size_t size) > > } > > EXPORT_SYMBOL_GPL(tee_shm_alloc_kernel_buf); > > > > +struct tee_shm *tee_shm_register_fd(struct tee_context *ctx, int fd) > > +{ > > + struct tee_shm_dmabuf_ref *ref; > > + int rc; > > + > > + if (!tee_device_get(ctx->teedev)) > > + return ERR_PTR(-EINVAL); > > + > > + teedev_ctx_get(ctx); > > + > > + ref = kzalloc(sizeof(*ref), GFP_KERNEL); > > + if (!ref) { > > + rc = -ENOMEM; > > + goto err_put_tee; > > + } > > + > > + refcount_set(&ref->shm.refcount, 1); > > + ref->shm.ctx = ctx; > > + ref->shm.id = -1; > > + ref->shm.flags = TEE_SHM_DMA_BUF; > > + > > + ref->dmabuf = dma_buf_get(fd); > > + if (IS_ERR(ref->dmabuf)) { > > + rc = PTR_ERR(ref->dmabuf); > > + goto err_kfree_ref; > > + } > > + > > + rc = tee_heap_update_from_dma_buf(ctx->teedev, ref->dmabuf, > > + &ref->offset, &ref->shm, > > + &ref->parent_shm); > > + if (!rc) > > + goto out; > > One odd thing I find here, why do we bail out on success case here? > Don't we need the DMA buffer attach and map APIs to be invoked on > success case here? No, because if tee_heap_update_from_dma_buf() succeeds, we know everything we need about the buffer. Note that we're returning a valid pointer below to indicate success. Cheers, Jens > > -Sumit > > > + if (rc != -EINVAL) > > + goto err_put_dmabuf; > > + > > + ref->attach = dma_buf_attach(ref->dmabuf, &ctx->teedev->dev); > > + if (IS_ERR(ref->attach)) { > > + rc = PTR_ERR(ref->attach); > > + goto err_put_dmabuf; > > + } > > + > > + ref->sgt = dma_buf_map_attachment(ref->attach, DMA_BIDIRECTIONAL); > > + if (IS_ERR(ref->sgt)) { > > + rc = PTR_ERR(ref->sgt); > > + goto err_detach; > > + } > > + > > + if (sg_nents(ref->sgt->sgl) != 1) { > > + rc = -EINVAL; > > + goto err_unmap_attachement; > > + } > > + > > + ref->shm.paddr = page_to_phys(sg_page(ref->sgt->sgl)); > > + ref->shm.size = ref->sgt->sgl->length; > > + > > +out: > > + mutex_lock(&ref->shm.ctx->teedev->mutex); > > + ref->shm.id = idr_alloc(&ref->shm.ctx->teedev->idr, &ref->shm, > > + 1, 0, GFP_KERNEL); > > + mutex_unlock(&ref->shm.ctx->teedev->mutex); > > + if (ref->shm.id < 0) { > > + rc = ref->shm.id; > > + if (ref->attach) > > + goto err_unmap_attachement; > > + goto err_put_dmabuf; > > + } > > + > > + return &ref->shm; > > + > > +err_unmap_attachement: > > + dma_buf_unmap_attachment(ref->attach, ref->sgt, DMA_BIDIRECTIONAL); > > +err_detach: > > + dma_buf_detach(ref->dmabuf, ref->attach); > > +err_put_dmabuf: > > + dma_buf_put(ref->dmabuf); > > +err_kfree_ref: > > + kfree(ref); > > +err_put_tee: > > + teedev_ctx_put(ctx); > > + tee_device_put(ctx->teedev); > > + > > + return ERR_PTR(rc); > > +} > > +EXPORT_SYMBOL_GPL(tee_shm_register_fd); > > + > > /** > > * tee_shm_alloc_priv_buf() - Allocate shared memory for a privately shared > > * kernel buffer > > diff --git a/include/linux/tee_core.h b/include/linux/tee_core.h > > index 22e03d897dc3..f17710196c4c 100644 > > --- a/include/linux/tee_core.h > > +++ b/include/linux/tee_core.h > > @@ -28,6 +28,7 @@ > > #define TEE_SHM_USER_MAPPED BIT(1) /* Memory mapped in user space */ > > #define TEE_SHM_POOL BIT(2) /* Memory allocated from pool */ > > #define TEE_SHM_PRIV BIT(3) /* Memory private to TEE driver */ > > +#define TEE_SHM_DMA_BUF BIT(4) /* Memory with dma-buf handle */ > > > > #define TEE_DEVICE_FLAG_REGISTERED 0x1 > > #define TEE_MAX_DEV_NAME_LEN 32 > > diff --git a/include/linux/tee_drv.h b/include/linux/tee_drv.h > > index a54c203000ed..824f1251de60 100644 > > --- a/include/linux/tee_drv.h > > +++ b/include/linux/tee_drv.h > > @@ -116,6 +116,16 @@ struct tee_shm *tee_shm_alloc_kernel_buf(struct tee_context *ctx, size_t size); > > struct tee_shm *tee_shm_register_kernel_buf(struct tee_context *ctx, > > void *addr, size_t length); > > > > +/** > > + * tee_shm_register_fd() - Register shared memory from file descriptor > > + * > > + * @ctx: Context that allocates the shared memory > > + * @fd: Shared memory file descriptor reference > > + * > > + * @returns a pointer to 'struct tee_shm' on success, and ERR_PTR on failure > > + */ > > +struct tee_shm *tee_shm_register_fd(struct tee_context *ctx, int fd); > > + > > /** > > * tee_shm_free() - Free shared memory > > * @shm: Handle to shared memory to free > > diff --git a/include/uapi/linux/tee.h b/include/uapi/linux/tee.h > > index d0430bee8292..d843cf980d98 100644 > > --- a/include/uapi/linux/tee.h > > +++ b/include/uapi/linux/tee.h > > @@ -378,6 +378,37 @@ struct tee_ioctl_shm_register_data { > > __s32 id; > > }; > > > > +/** > > + * struct tee_ioctl_shm_register_fd_data - Shared memory registering argument > > + * @fd: [in] File descriptor identifying dmabuf reference > > + * @size: [out] Size of referenced memory > > + * @flags: [in] Flags to/from allocation. > > + * @id: [out] Identifier of the shared memory > > + * > > + * The flags field should currently be zero as input. Updated by the call > > + * with actual flags as defined by TEE_IOCTL_SHM_* above. > > + * This structure is used as argument for TEE_IOC_SHM_REGISTER_FD below. > > + */ > > +struct tee_ioctl_shm_register_fd_data { > > + __s64 fd; > > + __u64 size; > > + __u32 flags; > > + __s32 id; > > +}; > > + > > +/** > > + * TEE_IOC_SHM_REGISTER_FD - register a shared memory from a file descriptor > > + * > > + * Returns a file descriptor on success or < 0 on failure > > + * > > + * The returned file descriptor refers to the shared memory object in the > > + * kernel. The supplied file deccriptor can be closed if it's not needed > > + * for other purposes. The shared memory is freed when the descriptor is > > + * closed. > > + */ > > +#define TEE_IOC_SHM_REGISTER_FD _IOWR(TEE_IOC_MAGIC, TEE_IOC_BASE + 8, \ > > + struct tee_ioctl_shm_register_fd_data) > > + > > /** > > * TEE_IOC_SHM_REGISTER - Register shared memory argument > > * > > -- > > 2.43.0 > >

2 weeks, 6 days

[PATCH v2] Documentation: dma-buf: heaps: Add naming guidelines

by Maxime Ripard

We've discussed a number of times of how some heap names are bad, but not really what makes a good heap name. Let's document what we expect the heap names to look like. Reviewed-by: Bagas Sanjaya <bagasdotme(a)gmail.com> Signed-off-by: Maxime Ripard <mripard(a)kernel.org> --- Changes in v2: - Added justifications for each requirement / suggestions - Added a mention and example of buffer attributes - Link to v1: https://lore.kernel.org/r/20250520-dma-buf-heap-names-doc-v1-1-ab31f74809ee… --- Documentation/userspace-api/dma-buf-heaps.rst | 38 +++++++++++++++++++++++++++ 1 file changed, 38 insertions(+) diff --git a/Documentation/userspace-api/dma-buf-heaps.rst b/Documentation/userspace-api/dma-buf-heaps.rst index 535f49047ce6450796bf4380c989e109355efc05..835ad1c3a65bc07b6f41d387d85c57162909e859 100644 --- a/Documentation/userspace-api/dma-buf-heaps.rst +++ b/Documentation/userspace-api/dma-buf-heaps.rst @@ -21,5 +21,43 @@ following heaps: usually created either through the kernel commandline through the `cma` parameter, a memory region Device-Tree node with the `linux,cma-default` property set, or through the `CMA_SIZE_MBYTES` or `CMA_SIZE_PERCENTAGE` Kconfig options. Depending on the platform, it might be called ``reserved``, ``linux,cma``, or ``default-pool``. + +Naming Convention +================= + +``dma-buf`` heaps name should meet a number of constraints: + +- That name must be stable, and must not change from one version to the + other. Userspace identifies heaps by their name, so if the names ever + changes, we would be likely to introduce regressions. + +- That name must describe the memory region the heap will allocate from, + and must uniquely identify it in a given platform. Since userspace + applications use the heap name as the discriminant, it must be able to + tell which heap it wants to use reliably if there's multiple heaps. + +- That name must not mention implementation details, such as the + allocator. The heap driver will change over time, and implementation + details when it was introduced might not be relevant in the future. + +- The name should describe properties of the buffers that would be + allocated. Doing so will make heap identification easier for + userspace. Such properties are: + + - ``cacheable`` / ``uncacheable`` for buffers with CPU caches enabled + or disabled; + + - ``contiguous`` for physically contiguous buffers; + + - ``protected`` for encrypted buffers not accessible the OS; + +- The name may describe intended usage. Doing so will make heap + identification easier for userspace applications and users. + +For example, assuming a platform with a reserved memory region located +at the RAM address 0x42000000, intended to allocate video framebuffers, +physically contiguous, and backed by the CMA kernel allocator. Good +names would be ``memory@42000000-cacheable-contiguous`` or +``video@42000000``, but ``cma-video`` wouldn't. --- base-commit: 19272b37aa4f83ca52bdf9c16d5d81bdd1354494 change-id: 20250520-dma-buf-heap-names-doc-31261aa0cfe6 Best regards, -- Maxime Ripard <mripard(a)kernel.org>

3 weeks, 1 day

[PATCH] Documentation: dma-buf: heaps: Add naming guidelines

by Maxime Ripard

We've discussed a number of times of how some heap names are bad, but not really what makes a good heap name. Let's document what we expect the heap names to look like. Signed-off-by: Maxime Ripard <mripard(a)kernel.org> --- Documentation/userspace-api/dma-buf-heaps.rst | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/Documentation/userspace-api/dma-buf-heaps.rst b/Documentation/userspace-api/dma-buf-heaps.rst index 535f49047ce6450796bf4380c989e109355efc05..b24618e360a9a9ba0bd85135d8c1760776f1a37f 100644 --- a/Documentation/userspace-api/dma-buf-heaps.rst +++ b/Documentation/userspace-api/dma-buf-heaps.rst @@ -21,5 +21,24 @@ following heaps: usually created either through the kernel commandline through the `cma` parameter, a memory region Device-Tree node with the `linux,cma-default` property set, or through the `CMA_SIZE_MBYTES` or `CMA_SIZE_PERCENTAGE` Kconfig options. Depending on the platform, it might be called ``reserved``, ``linux,cma``, or ``default-pool``. + +Naming Convention +================= + +A good heap name is a name that: + +- Is stable, and won't change from one version to the other; + +- Describes the memory region the heap will allocate from, and will + uniquely identify it in a given platform; + +- Doesn't use implementation details, such as the allocator; + +- Can describe intended usage. + +For example, assuming a platform with a reserved memory region located +at the RAM address 0x42000000, intended to allocate video framebuffers, +and backed by the CMA kernel allocator. Good names would be +`memory@42000000` or `video@42000000`, but `cma-video` wouldn't. --- base-commit: 92a09c47464d040866cf2b4cd052bc60555185fb change-id: 20250520-dma-buf-heap-names-doc-31261aa0cfe6 Best regards, -- Maxime Ripard <mripard(a)kernel.org>

3 weeks, 1 day

Re: [PATCH v4 0/4] Implement dmabuf direct I/O via copy_file_range

by Christoph Hellwig

On Fri, Jun 13, 2025 at 09:33:55AM +0000, wangtao wrote: > > > > > On Mon, Jun 09, 2025 at 09:32:20AM +0000, wangtao wrote: > > > Are you suggesting adding an ITER_DMABUF type to iov_iter, > > > > Yes. > > May I clarify: Do all disk operations require data to pass through > memory (reading into memory or writing from memory)? In the block layer, > the bio structure uses bio_iov_iter_get_pages to convert iter_type > objects into memory-backed bio_vec representations. > However, some dmabufs are not memory-based, making page-to-bio_vec > conversion impossible. This suggests adding a callback function in > dma_buf_ops to handle dmabuf- to-bio_vec conversion. bios do support PCI P2P tranfers. This could be fairly easily extended to other peer to peer transfers if we manage to come up with a coherent model for them. No need for a callback.

3 weeks, 1 day

Re: [PATCH v4 0/4] Implement dmabuf direct I/O via copy_file_range

by Christoph Hellwig

On Fri, Jun 13, 2025 at 09:43:08AM +0000, wangtao wrote: > Here's my analysis on Linux 6.6 with F2FS/iomap. Linux 6.6 is almost two years old and completely irrelevant. Please provide numbers on 6.16 or current Linus' tree.

3 weeks, 1 day

Re: [PATCH v5 03/12] tee: add TEE_IOCTL_PARAM_ATTR_TYPE_UBUF

by Andrew Davis

On 5/27/25 1:56 AM, Amirreza Zarrabi wrote: > For drivers that can transfer data to the TEE without using shared > memory from client, it is necessary to receive the user address > directly, bypassing any processing by the TEE subsystem. Introduce > TEE_IOCTL_PARAM_ATTR_TYPE_UBUF_INPUT/OUTPUT/INOUT to represent > userspace buffers. > Could you expand on this, what is the issue with normal MEMREF? Andrew > Reviewed-by: Sumit Garg <sumit.garg(a)oss.qualcomm.com> > Tested-by: Neil Armstrong <neil.armstrong(a)linaro.org> > Signed-off-by: Amirreza Zarrabi <amirreza.zarrabi(a)oss.qualcomm.com> > --- > drivers/tee/tee_core.c | 33 +++++++++++++++++++++++++++++++++ > include/linux/tee_drv.h | 6 ++++++ > include/uapi/linux/tee.h | 22 ++++++++++++++++------ > 3 files changed, 55 insertions(+), 6 deletions(-) > > diff --git a/drivers/tee/tee_core.c b/drivers/tee/tee_core.c > index b9ea5a85278c..74e40ed83fa7 100644 > --- a/drivers/tee/tee_core.c > +++ b/drivers/tee/tee_core.c > @@ -387,6 +387,17 @@ static int params_from_user(struct tee_context *ctx, struct tee_param *params, > params[n].u.value.b = ip.b; > params[n].u.value.c = ip.c; > break; > + case TEE_IOCTL_PARAM_ATTR_TYPE_UBUF_INPUT: > + case TEE_IOCTL_PARAM_ATTR_TYPE_UBUF_OUTPUT: > + case TEE_IOCTL_PARAM_ATTR_TYPE_UBUF_INOUT: > + params[n].u.ubuf.uaddr = u64_to_user_ptr(ip.a); > + params[n].u.ubuf.size = ip.b; > + > + if (!access_ok(params[n].u.ubuf.uaddr, > + params[n].u.ubuf.size)) > + return -EFAULT; > + > + break; > case TEE_IOCTL_PARAM_ATTR_TYPE_MEMREF_INPUT: > case TEE_IOCTL_PARAM_ATTR_TYPE_MEMREF_OUTPUT: > case TEE_IOCTL_PARAM_ATTR_TYPE_MEMREF_INOUT: > @@ -455,6 +466,11 @@ static int params_to_user(struct tee_ioctl_param __user *uparams, > put_user(p->u.value.c, &up->c)) > return -EFAULT; > break; > + case TEE_IOCTL_PARAM_ATTR_TYPE_UBUF_OUTPUT: > + case TEE_IOCTL_PARAM_ATTR_TYPE_UBUF_INOUT: > + if (put_user((u64)p->u.ubuf.size, &up->b)) > + return -EFAULT; > + break; > case TEE_IOCTL_PARAM_ATTR_TYPE_MEMREF_OUTPUT: > case TEE_IOCTL_PARAM_ATTR_TYPE_MEMREF_INOUT: > if (put_user((u64)p->u.memref.size, &up->b)) > @@ -655,6 +671,13 @@ static int params_to_supp(struct tee_context *ctx, > ip.b = p->u.value.b; > ip.c = p->u.value.c; > break; > + case TEE_IOCTL_PARAM_ATTR_TYPE_UBUF_INPUT: > + case TEE_IOCTL_PARAM_ATTR_TYPE_UBUF_OUTPUT: > + case TEE_IOCTL_PARAM_ATTR_TYPE_UBUF_INOUT: > + ip.a = (u64)p->u.ubuf.uaddr; > + ip.b = p->u.ubuf.size; > + ip.c = 0; > + break; > case TEE_IOCTL_PARAM_ATTR_TYPE_MEMREF_INPUT: > case TEE_IOCTL_PARAM_ATTR_TYPE_MEMREF_OUTPUT: > case TEE_IOCTL_PARAM_ATTR_TYPE_MEMREF_INOUT: > @@ -757,6 +780,16 @@ static int params_from_supp(struct tee_param *params, size_t num_params, > p->u.value.b = ip.b; > p->u.value.c = ip.c; > break; > + case TEE_IOCTL_PARAM_ATTR_TYPE_UBUF_OUTPUT: > + case TEE_IOCTL_PARAM_ATTR_TYPE_UBUF_INOUT: > + p->u.ubuf.uaddr = u64_to_user_ptr(ip.a); > + p->u.ubuf.size = ip.b; > + > + if (!access_ok(params[n].u.ubuf.uaddr, > + params[n].u.ubuf.size)) > + return -EFAULT; > + > + break; > case TEE_IOCTL_PARAM_ATTR_TYPE_MEMREF_OUTPUT: > case TEE_IOCTL_PARAM_ATTR_TYPE_MEMREF_INOUT: > /* > diff --git a/include/linux/tee_drv.h b/include/linux/tee_drv.h > index a54c203000ed..78bbf12f02f0 100644 > --- a/include/linux/tee_drv.h > +++ b/include/linux/tee_drv.h > @@ -82,6 +82,11 @@ struct tee_param_memref { > struct tee_shm *shm; > }; > > +struct tee_param_ubuf { > + void * __user uaddr; > + size_t size; > +}; > + > struct tee_param_value { > u64 a; > u64 b; > @@ -92,6 +97,7 @@ struct tee_param { > u64 attr; > union { > struct tee_param_memref memref; > + struct tee_param_ubuf ubuf; > struct tee_param_value value; > } u; > }; > diff --git a/include/uapi/linux/tee.h b/include/uapi/linux/tee.h > index d0430bee8292..3e9b1ec5dfde 100644 > --- a/include/uapi/linux/tee.h > +++ b/include/uapi/linux/tee.h > @@ -151,6 +151,13 @@ struct tee_ioctl_buf_data { > #define TEE_IOCTL_PARAM_ATTR_TYPE_MEMREF_OUTPUT 6 > #define TEE_IOCTL_PARAM_ATTR_TYPE_MEMREF_INOUT 7 /* input and output */ > > +/* > + * These defines userspace buffer parameters. > + */ > +#define TEE_IOCTL_PARAM_ATTR_TYPE_UBUF_INPUT 8 > +#define TEE_IOCTL_PARAM_ATTR_TYPE_UBUF_OUTPUT 9 > +#define TEE_IOCTL_PARAM_ATTR_TYPE_UBUF_INOUT 10 /* input and output */ > + > /* > * Mask for the type part of the attribute, leaves room for more types > */ > @@ -186,14 +193,17 @@ struct tee_ioctl_buf_data { > /** > * struct tee_ioctl_param - parameter > * @attr: attributes > - * @a: if a memref, offset into the shared memory object, else a value parameter > - * @b: if a memref, size of the buffer, else a value parameter > + * @a: if a memref, offset into the shared memory object, > + * else if a ubuf, address of the user buffer, > + * else a value parameter > + * @b: if a memref or ubuf, size of the buffer, else a value parameter > * @c: if a memref, shared memory identifier, else a value parameter > * > - * @attr & TEE_PARAM_ATTR_TYPE_MASK indicates if memref or value is used in > - * the union. TEE_PARAM_ATTR_TYPE_VALUE_* indicates value and > - * TEE_PARAM_ATTR_TYPE_MEMREF_* indicates memref. TEE_PARAM_ATTR_TYPE_NONE > - * indicates that none of the members are used. > + * @attr & TEE_PARAM_ATTR_TYPE_MASK indicates if memref, ubuf, or value is > + * used in the union. TEE_PARAM_ATTR_TYPE_VALUE_* indicates value, > + * TEE_PARAM_ATTR_TYPE_MEMREF_* indicates memref, and TEE_PARAM_ATTR_TYPE_UBUF_* > + * indicates ubuf. TEE_PARAM_ATTR_TYPE_NONE indicates that none of the members > + * are used. > * > * Shared memory is allocated with TEE_IOC_SHM_ALLOC which returns an > * identifier representing the shared memory object. A memref can reference >

3 weeks, 5 days

Re: [PATCH v6 1/4] sync_file: Protect access to driver and timeline name

by Christian König

On 6/10/25 18:42, Tvrtko Ursulin wrote: > Protect the access to driver and timeline name which otherwise could be > freed as dma-fence exported is signalling fences. > > This prepares the code for incoming dma-fence API changes which will start > asserting these accesses are done from a RCU locked section. > > Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin(a)igalia.com> Reviewed-by: Christian König <christian.koenig(a)amd.com> > --- > drivers/dma-buf/sync_file.c | 24 ++++++++++++++++++++---- > 1 file changed, 20 insertions(+), 4 deletions(-) > > diff --git a/drivers/dma-buf/sync_file.c b/drivers/dma-buf/sync_file.c > index 212df4b849fe..747e377fb954 100644 > --- a/drivers/dma-buf/sync_file.c > +++ b/drivers/dma-buf/sync_file.c > @@ -135,12 +135,18 @@ char *sync_file_get_name(struct sync_file *sync_file, char *buf, int len) > strscpy(buf, sync_file->user_name, len); > } else { > struct dma_fence *fence = sync_file->fence; > + const char __rcu *timeline; > + const char __rcu *driver; > > + rcu_read_lock(); > + driver = dma_fence_driver_name(fence); > + timeline = dma_fence_timeline_name(fence); > snprintf(buf, len, "%s-%s%llu-%lld", > - dma_fence_driver_name(fence), > - dma_fence_timeline_name(fence), > + rcu_dereference(driver), > + rcu_dereference(timeline), > fence->context, > fence->seqno); > + rcu_read_unlock(); > } > > return buf; > @@ -262,9 +268,17 @@ static long sync_file_ioctl_merge(struct sync_file *sync_file, > static int sync_fill_fence_info(struct dma_fence *fence, > struct sync_fence_info *info) > { > - strscpy(info->obj_name, dma_fence_timeline_name(fence), > + const char __rcu *timeline; > + const char __rcu *driver; > + > + rcu_read_lock(); > + > + driver = dma_fence_driver_name(fence); > + timeline = dma_fence_timeline_name(fence); > + > + strscpy(info->obj_name, rcu_dereference(timeline), > sizeof(info->obj_name)); > - strscpy(info->driver_name, dma_fence_driver_name(fence), > + strscpy(info->driver_name, rcu_dereference(driver), > sizeof(info->driver_name)); > > info->status = dma_fence_get_status(fence); > @@ -273,6 +287,8 @@ static int sync_fill_fence_info(struct dma_fence *fence, > ktime_to_ns(dma_fence_timestamp(fence)) : > ktime_set(0, 0); > > + rcu_read_unlock(); > + > return info->status; > } >

3 weeks, 6 days

Re: [PATCH v6 3/4] dma-fence: Add safe access helpers and document the rules

by Christian König

On 6/10/25 18:42, Tvrtko Ursulin wrote: > Dma-fence objects currently suffer from a potential use after free problem > where fences exported to userspace and other drivers can outlive the > exporting driver, or the associated data structures. > > The discussion on how to address this concluded that adding reference > counting to all the involved objects is not desirable, since it would need > to be very wide reaching and could cause unloadable drivers if another > entity would be holding onto a signaled fence reference potentially > indefinitely. > > This patch enables the safe access by introducing and documenting a > contract between fence exporters and users. It documents a set of > contraints and adds helpers which a) drivers with potential to suffer from > the use after free must use and b) users of the dma-fence API must use as > well. > > Premise of the design has multiple sides: > > 1. Drivers (fence exporters) MUST ensure a RCU grace period between > signalling a fence and freeing the driver private data associated with it. > > The grace period does not have to follow the signalling immediately but > HAS to happen before data is freed. > > 2. Users of the dma-fence API marked with such requirement MUST contain > the complete access to the data within a single code block guarded by > rcu_read_lock() and rcu_read_unlock(). > > The combination of the two ensures that whoever sees the > DMA_FENCE_FLAG_SIGNALED_BIT not set is guaranteed to have access to a > valid fence->lock and valid data potentially accessed by the fence->ops > virtual functions, until the call to rcu_read_unlock(). > > 3. Module unload (fence->ops) disappearing is for now explicitly not > handled. That would required a more complex protection, possibly needing > SRCU instead of RCU to handle callers such as dma_fence_release() and > dma_fence_wait_timeout(), where race between > dma_fence_enable_sw_signaling, signalling, and dereference of > fence->ops->wait() would need a sleeping SRCU context. > > Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin(a)igalia.com> Reviewed-by: Christian König <christian.koenig(a)amd.com> > --- > drivers/dma-buf/dma-fence.c | 111 ++++++++++++++++++++++++++++--- > include/linux/dma-fence.h | 31 ++++++--- > include/trace/events/dma_fence.h | 38 +++++++++-- > 3 files changed, 157 insertions(+), 23 deletions(-) > > diff --git a/drivers/dma-buf/dma-fence.c b/drivers/dma-buf/dma-fence.c > index 74f9e4b665e3..3f78c56b58dc 100644 > --- a/drivers/dma-buf/dma-fence.c > +++ b/drivers/dma-buf/dma-fence.c > @@ -511,12 +511,20 @@ dma_fence_wait_timeout(struct dma_fence *fence, bool intr, signed long timeout) > > dma_fence_enable_sw_signaling(fence); > > - trace_dma_fence_wait_start(fence); > + if (trace_dma_fence_wait_start_enabled()) { > + rcu_read_lock(); > + trace_dma_fence_wait_start(fence); > + rcu_read_unlock(); > + } > if (fence->ops->wait) > ret = fence->ops->wait(fence, intr, timeout); > else > ret = dma_fence_default_wait(fence, intr, timeout); > - trace_dma_fence_wait_end(fence); > + if (trace_dma_fence_wait_end_enabled()) { > + rcu_read_lock(); > + trace_dma_fence_wait_end(fence); > + rcu_read_unlock(); > + } > return ret; > } > EXPORT_SYMBOL(dma_fence_wait_timeout); > @@ -533,16 +541,23 @@ void dma_fence_release(struct kref *kref) > struct dma_fence *fence = > container_of(kref, struct dma_fence, refcount); > > + rcu_read_lock(); > trace_dma_fence_destroy(fence); > > - if (WARN(!list_empty(&fence->cb_list) && > - !test_bit(DMA_FENCE_FLAG_SIGNALED_BIT, &fence->flags), > - "Fence %s:%s:%llx:%llx released with pending signals!\n", > - dma_fence_driver_name(fence), > - dma_fence_timeline_name(fence), > - fence->context, fence->seqno)) { > + if (!list_empty(&fence->cb_list) && > + !test_bit(DMA_FENCE_FLAG_SIGNALED_BIT, &fence->flags)) { > + const char __rcu *timeline; > + const char __rcu *driver; > unsigned long flags; > > + driver = dma_fence_driver_name(fence); > + timeline = dma_fence_timeline_name(fence); > + > + WARN(1, > + "Fence %s:%s:%llx:%llx released with pending signals!\n", > + rcu_dereference(driver), rcu_dereference(timeline), > + fence->context, fence->seqno); > + > /* > * Failed to signal before release, likely a refcounting issue. > * > @@ -556,6 +571,8 @@ void dma_fence_release(struct kref *kref) > spin_unlock_irqrestore(fence->lock, flags); > } > > + rcu_read_unlock(); > + > if (fence->ops->release) > fence->ops->release(fence); > else > @@ -982,11 +999,21 @@ EXPORT_SYMBOL(dma_fence_set_deadline); > */ > void dma_fence_describe(struct dma_fence *fence, struct seq_file *seq) > { > + const char __rcu *timeline; > + const char __rcu *driver; > + > + rcu_read_lock(); > + > + timeline = dma_fence_timeline_name(fence); > + driver = dma_fence_driver_name(fence); > + > seq_printf(seq, "%s %s seq %llu %ssignalled\n", > - dma_fence_driver_name(fence), > - dma_fence_timeline_name(fence), > + rcu_dereference(driver), > + rcu_dereference(timeline), > fence->seqno, > dma_fence_is_signaled(fence) ? "" : "un"); > + > + rcu_read_unlock(); > } > EXPORT_SYMBOL(dma_fence_describe); > > @@ -1055,3 +1082,67 @@ dma_fence_init64(struct dma_fence *fence, const struct dma_fence_ops *ops, > BIT(DMA_FENCE_FLAG_SEQNO64_BIT)); > } > EXPORT_SYMBOL(dma_fence_init64); > + > +/** > + * dma_fence_driver_name - Access the driver name > + * @fence: the fence to query > + * > + * Returns a driver name backing the dma-fence implementation. > + * > + * IMPORTANT CONSIDERATION: > + * Dma-fence contract stipulates that access to driver provided data (data not > + * directly embedded into the object itself), such as the &dma_fence.lock and > + * memory potentially accessed by the &dma_fence.ops functions, is forbidden > + * after the fence has been signalled. Drivers are allowed to free that data, > + * and some do. > + * > + * To allow safe access drivers are mandated to guarantee a RCU grace period > + * between signalling the fence and freeing said data. > + * > + * As such access to the driver name is only valid inside a RCU locked section. > + * The pointer MUST be both queried and USED ONLY WITHIN a SINGLE block guarded > + * by the &rcu_read_lock and &rcu_read_unlock pair. > + */ > +const char __rcu *dma_fence_driver_name(struct dma_fence *fence) > +{ > + RCU_LOCKDEP_WARN(!rcu_read_lock_held(), > + "RCU protection is required for safe access to returned string"); > + > + if (!test_bit(DMA_FENCE_FLAG_SIGNALED_BIT, &fence->flags)) > + return fence->ops->get_driver_name(fence); > + else > + return "detached-driver"; > +} > +EXPORT_SYMBOL(dma_fence_driver_name); > + > +/** > + * dma_fence_timeline_name - Access the timeline name > + * @fence: the fence to query > + * > + * Returns a timeline name provided by the dma-fence implementation. > + * > + * IMPORTANT CONSIDERATION: > + * Dma-fence contract stipulates that access to driver provided data (data not > + * directly embedded into the object itself), such as the &dma_fence.lock and > + * memory potentially accessed by the &dma_fence.ops functions, is forbidden > + * after the fence has been signalled. Drivers are allowed to free that data, > + * and some do. > + * > + * To allow safe access drivers are mandated to guarantee a RCU grace period > + * between signalling the fence and freeing said data. > + * > + * As such access to the driver name is only valid inside a RCU locked section. > + * The pointer MUST be both queried and USED ONLY WITHIN a SINGLE block guarded > + * by the &rcu_read_lock and &rcu_read_unlock pair. > + */ > +const char __rcu *dma_fence_timeline_name(struct dma_fence *fence) > +{ > + RCU_LOCKDEP_WARN(!rcu_read_lock_held(), > + "RCU protection is required for safe access to returned string"); > + > + if (!test_bit(DMA_FENCE_FLAG_SIGNALED_BIT, &fence->flags)) > + return fence->ops->get_driver_name(fence); > + else > + return "signaled-timeline"; > +} > +EXPORT_SYMBOL(dma_fence_timeline_name); > diff --git a/include/linux/dma-fence.h b/include/linux/dma-fence.h > index 10a849cb2d3f..64639e104110 100644 > --- a/include/linux/dma-fence.h > +++ b/include/linux/dma-fence.h > @@ -378,15 +378,28 @@ bool dma_fence_remove_callback(struct dma_fence *fence, > struct dma_fence_cb *cb); > void dma_fence_enable_sw_signaling(struct dma_fence *fence); > > -static inline const char *dma_fence_driver_name(struct dma_fence *fence) > -{ > - return fence->ops->get_driver_name(fence); > -} > - > -static inline const char *dma_fence_timeline_name(struct dma_fence *fence) > -{ > - return fence->ops->get_timeline_name(fence); > -} > +/** > + * DOC: Safe external access to driver provided object members > + * > + * All data not stored directly in the dma-fence object, such as the > + * &dma_fence.lock and memory potentially accessed by functions in the > + * &dma_fence.ops table, MUST NOT be accessed after the fence has been signalled > + * because after that point drivers are allowed to free it. > + * > + * All code accessing that data via the dma-fence API (or directly, which is > + * discouraged), MUST make sure to contain the complete access within a > + * &rcu_read_lock and &rcu_read_unlock pair. > + * > + * Some dma-fence API handles this automatically, while other, as for example > + * &dma_fence_driver_name and &dma_fence_timeline_name, leave that > + * responsibility to the caller. > + * > + * To enable this scheme to work drivers MUST ensure a RCU grace period elapses > + * between signalling the fence and freeing the said data. > + * > + */ > +const char __rcu *dma_fence_driver_name(struct dma_fence *fence); > +const char __rcu *dma_fence_timeline_name(struct dma_fence *fence); > > /** > * dma_fence_is_signaled_locked - Return an indication if the fence > diff --git a/include/trace/events/dma_fence.h b/include/trace/events/dma_fence.h > index 84c83074ee81..4814a65b68dc 100644 > --- a/include/trace/events/dma_fence.h > +++ b/include/trace/events/dma_fence.h > @@ -34,14 +34,44 @@ DECLARE_EVENT_CLASS(dma_fence, > __entry->seqno) > ); > > -DEFINE_EVENT(dma_fence, dma_fence_emit, > +/* > + * Safe only for call sites which are guaranteed to not race with fence > + * signaling,holding the fence->lock and having checked for not signaled, or the > + * signaling path itself. > + */ > +DECLARE_EVENT_CLASS(dma_fence_unsignaled, > + > + TP_PROTO(struct dma_fence *fence), > + > + TP_ARGS(fence), > + > + TP_STRUCT__entry( > + __string(driver, fence->ops->get_driver_name(fence)) > + __string(timeline, fence->ops->get_timeline_name(fence)) > + __field(unsigned int, context) > + __field(unsigned int, seqno) > + ), > + > + TP_fast_assign( > + __assign_str(driver); > + __assign_str(timeline); > + __entry->context = fence->context; > + __entry->seqno = fence->seqno; > + ), > + > + TP_printk("driver=%s timeline=%s context=%u seqno=%u", > + __get_str(driver), __get_str(timeline), __entry->context, > + __entry->seqno) > +); > + > +DEFINE_EVENT(dma_fence_unsignaled, dma_fence_emit, > > TP_PROTO(struct dma_fence *fence), > > TP_ARGS(fence) > ); > > -DEFINE_EVENT(dma_fence, dma_fence_init, > +DEFINE_EVENT(dma_fence_unsignaled, dma_fence_init, > > TP_PROTO(struct dma_fence *fence), > > @@ -55,14 +85,14 @@ DEFINE_EVENT(dma_fence, dma_fence_destroy, > TP_ARGS(fence) > ); > > -DEFINE_EVENT(dma_fence, dma_fence_enable_signal, > +DEFINE_EVENT(dma_fence_unsignaled, dma_fence_enable_signal, > > TP_PROTO(struct dma_fence *fence), > > TP_ARGS(fence) > ); > > -DEFINE_EVENT(dma_fence, dma_fence_signaled, > +DEFINE_EVENT(dma_fence_unsignaled, dma_fence_signaled, > > TP_PROTO(struct dma_fence *fence), >

3 weeks, 6 days

[PATCH v3 0/3] media: fix incorrect use of dma_sync_sg_*() calls

by Marek Szyprowski

Dear All, This patchset fixes the incorrect use of dma_sync_sg_*() calls in media and related drivers. They are replaced with much safer dma_sync_sgtable_*() variants, which take care of passing the proper number of elements for the sync operation. Best regards Marek Szyprowski, PhD Samsung R&D Institute Poland Change log: v3: added cc: stable to tags v2: fixes typos and added cc: stable Patch summary: Marek Szyprowski (3): media: videobuf2: use sgtable-based scatterlist wrappers udmabuf: use sgtable-based scatterlist wrappers media: omap3isp: use sgtable-based scatterlist wrappers drivers/dma-buf/udmabuf.c | 5 ++--- drivers/media/common/videobuf2/videobuf2-dma-sg.c | 4 ++-- drivers/media/platform/ti/omap3isp/ispccdc.c | 8 ++++---- drivers/media/platform/ti/omap3isp/ispstat.c | 6 ++---- 4 files changed, 10 insertions(+), 13 deletions(-) -- 2.34.1

3 weeks, 6 days

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

Jump to page:

2025

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

2012

2011

Linaro-mm-sig