Linaro-mm-sig

linaro-mm-sig@lists.linaro.org

16 participants
7076 discussions

Re: [PATCH v3 2/3] dma-buf: heaps: Parameterize heap name in __add_cma_heap()

by Maxime Ripard

On Thu, 22 May 2025 12:14:17 -0700, Jared Kangas wrote: > Prepare for the introduction of a fixed-name CMA heap by replacing the > unused void pointer parameter in __add_cma_heap() with the heap name. > > Signed-off-by: Jared Kangas <jkangas(a)redhat.com> Reviewed-by: Maxime Ripard <mripard(a)kernel.org> Thanks! Maxime

1 year, 1 month

Re: [PATCH v3 1/3] Documentation: dma-buf: heaps: Fix code markup

by Maxime Ripard

On Thu, 22 May 2025 12:14:16 -0700, Jared Kangas wrote: > Code snippets should be wrapped in double backticks to follow > reStructuredText semantics; the use of single backticks uses the > :title-reference: role by default, which isn't quite what we want. > Add double backticks to code snippets to fix this. > > > [ ... ] Reviewed-by: Maxime Ripard <mripard(a)kernel.org> Thanks! Maxime

1 year, 1 month

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

by Jens Wiklander

On Mon, May 26, 2025 at 9:22 AM Sumit Garg <sumit.garg(a)kernel.org> wrote: > > On Tue, May 20, 2025 at 05:16:49PM +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 | 74 ++++++++++++++++++++++++++++++++++++++-- > > include/linux/tee_core.h | 5 +++ > > 2 files changed, 77 insertions(+), 2 deletions(-) > > > > diff --git a/drivers/tee/tee_shm.c b/drivers/tee/tee_shm.c > > index e1ed52ee0a16..92a6a35e1a1e 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; > > @@ -49,7 +56,14 @@ static void tee_shm_release(struct tee_device *teedev, struct tee_shm *shm) > > struct tee_shm *parent_shm = NULL; > > void *p = shm; > > > > - if (shm->flags & TEE_SHM_DMA_BUF) { > > + if (shm->flags & TEE_SHM_DMA_MEM) { > > + 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); > > Although the kernel bot already found a randconfig issue, it looks like > we need to add Kconfig dependencies like HAS_DMA, DMA_CMA etc. > > Also, I was thinking if we should rather add a new TEE subsystem > specific Kconfig option like: TEE_DMABUF_HEAPS which can then be used to > select whatever dependency is needed as well as act as a gating Kconfig > for relevant features. You mean something like this? --- a/drivers/tee/Kconfig +++ b/drivers/tee/Kconfig @@ -13,6 +13,14 @@ menuconfig TEE if TEE +config TEE_DMABUF_HEAPS + bool + depends on HAS_DMA && DMABUF_HEAPS + +config TEE_STATIC_PROTMEM_POOL + bool + depends on HAS_IOMEM && TEE_DMABUF_HEAPS + Cheers, Jens > > -Sumit > > > + } else if (shm->flags & TEE_SHM_DMA_BUF) { > > struct tee_shm_dmabuf_ref *ref; > > > > ref = container_of(shm, struct tee_shm_dmabuf_ref, shm); > > @@ -306,6 +320,62 @@ struct tee_shm *tee_shm_alloc_priv_buf(struct tee_context *ctx, size_t size) > > } > > EXPORT_SYMBOL_GPL(tee_shm_alloc_priv_buf); > > > > +/** > > + * 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); > > + > > 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 02c07f661349..925690e1020b 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 > >

1 year, 1 month

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

by Jens Wiklander

On Fri, May 23, 2025 at 3:31 PM Sumit Garg <sumit.garg(a)kernel.org> wrote: > > On Tue, May 20, 2025 at 05:16:48PM +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 | 111 ++++++++++++++++++++++++++++++++++++-- > > include/linux/tee_core.h | 1 + > > include/linux/tee_drv.h | 10 ++++ > > include/uapi/linux/tee.h | 31 +++++++++++ > > 6 files changed, 221 insertions(+), 5 deletions(-) > > With below minor comments fixed, feel free to add: > > Reviewed-by: Sumit Garg <sumit.garg(a)oss.qualcomm.com> > > > > > 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; > > Is this an issue being detected now? If yes then shouldn't it be a > separate fix eligible for backport? No, this is to add an eventual offset for a TEE_SHM_DMA_BUF, introduced above. > > > 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..e1ed52ee0a16 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,23 @@ 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) { > > + struct tee_shm *parent_shm = NULL; > > + 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); > > + parent_shm = ref->parent_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); > > @@ -57,9 +74,10 @@ static void tee_shm_release(struct tee_device *teedev, struct tee_shm *shm) > > release_registered_pages(shm); > > } > > > > - teedev_ctx_put(shm->ctx); > > + if (shm->ctx) > > + teedev_ctx_put(shm->ctx); > > redundant change? Yes, I'll remove it. > > > > > - kfree(shm); > > + kfree(p); > > > > tee_device_put(teedev); > > } > > @@ -169,7 +187,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 +197,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; > > + 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 b8b99c97e00c..02c07f661349 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..8ec5f46fbfbe 100644 > > --- a/include/uapi/linux/tee.h > > +++ b/include/uapi/linux/tee.h > > @@ -118,6 +118,37 @@ struct tee_ioctl_shm_alloc_data { > > #define TEE_IOC_SHM_ALLOC _IOWR(TEE_IOC_MAGIC, TEE_IOC_BASE + 1, \ > > struct tee_ioctl_shm_alloc_data) > > > > +/** > > + * 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) > > Please add this IOCTL in correct sequence order. OK, I'll fix it. Thanks, Jens > > -Sumit > > > + > > /** > > * struct tee_ioctl_buf_data - Variable sized buffer > > * @buf_ptr: [in] A __user pointer to a buffer > > -- > > 2.43.0 > >

1 year, 1 month

Re: [PATCH v9 3/9] tee: implement protected DMA-heap

by Jens Wiklander

On Fri, May 23, 2025 at 3:03 PM Sumit Garg <sumit.garg(a)kernel.org> wrote: > > + Robin > > Jens, > > I suppose you missed to add Robin who has earlier reviewed this patch. Yes, you're right, sorry. Thanks for catching this. > > On Tue, May 20, 2025 at 05:16:46PM +0200, Jens Wiklander wrote: > > Implement DMA heap for protected DMA-buf allocation in the TEE > > subsystem. > > > > Restricted memory refers to memory buffers behind a hardware enforced > > s/Restricted/Protected/ > > > firewall. It is not accessible to the kernel during normal circumstances > > but rather only accessible to certain hardware IPs or CPUs executing in > > higher or differently privileged mode than the kernel itself. This > > interface allows to allocate and manage such protected memory buffers > > via interaction with a TEE implementation. > > > > The protected memory is allocated for a specific use-case, like Secure > > Video Playback, Trusted UI, or Secure Video Recording where certain > > hardware devices can access the memory. > > > > The DMA-heaps are enabled explicitly by the TEE backend driver. The TEE > > backend drivers needs to implement protected memory pool to manage the > > protected memory. > > > > Signed-off-by: Jens Wiklander <jens.wiklander(a)linaro.org> > > --- > > drivers/tee/Makefile | 1 + > > drivers/tee/tee_heap.c | 487 ++++++++++++++++++++++++++++++++++++++ > > drivers/tee/tee_private.h | 6 + > > include/linux/tee_core.h | 65 +++++ > > 4 files changed, 559 insertions(+) > > create mode 100644 drivers/tee/tee_heap.c > > > > diff --git a/drivers/tee/Makefile b/drivers/tee/Makefile > > index 5488cba30bd2..949a6a79fb06 100644 > > --- a/drivers/tee/Makefile > > +++ b/drivers/tee/Makefile > > @@ -1,6 +1,7 @@ > > # SPDX-License-Identifier: GPL-2.0 > > obj-$(CONFIG_TEE) += tee.o > > tee-objs += tee_core.o > > +tee-objs += tee_heap.o > > tee-objs += tee_shm.o > > tee-objs += tee_shm_pool.o > > obj-$(CONFIG_OPTEE) += optee/ > > diff --git a/drivers/tee/tee_heap.c b/drivers/tee/tee_heap.c > > new file mode 100644 > > index 000000000000..a332805f9f26 > > --- /dev/null > > +++ b/drivers/tee/tee_heap.c > > @@ -0,0 +1,487 @@ > > +// SPDX-License-Identifier: GPL-2.0-only > > +/* > > + * Copyright (c) 2025, Linaro Limited > > + */ > > + > > +#include <linux/dma-buf.h> > > +#include <linux/dma-heap.h> > > +#include <linux/genalloc.h> > > +#include <linux/module.h> > > +#include <linux/scatterlist.h> > > +#include <linux/slab.h> > > +#include <linux/tee_core.h> > > +#include <linux/xarray.h> > > + > > +#include "tee_private.h" > > + > > +struct tee_dma_heap { > > + struct dma_heap *heap; > > + enum tee_dma_heap_id id; > > + struct tee_protmem_pool *pool; > > + struct tee_device *teedev; > > + /* Protects pool and teedev above */ > > + struct mutex mu; > > +}; > > + > > +struct tee_heap_buffer { > > + struct tee_protmem_pool *pool; > > + struct tee_device *teedev; > > + size_t size; > > + size_t offs; > > + struct sg_table table; > > +}; > > + > > +struct tee_heap_attachment { > > + struct sg_table table; > > + struct device *dev; > > +}; > > + > > +struct tee_protmem_static_pool { > > + struct tee_protmem_pool pool; > > + struct gen_pool *gen_pool; > > + phys_addr_t pa_base; > > + void *base; > > +}; > > + > > +#if IS_ENABLED(CONFIG_DMABUF_HEAPS) > > +static DEFINE_XARRAY_ALLOC(tee_dma_heap); > > + > > +static int copy_sg_table(struct sg_table *dst, struct sg_table *src) > > +{ > > + struct scatterlist *dst_sg; > > + struct scatterlist *src_sg; > > + int ret; > > + int i; > > + > > + ret = sg_alloc_table(dst, src->orig_nents, GFP_KERNEL); > > + if (ret) > > + return ret; > > + > > + dst_sg = dst->sgl; > > + for_each_sgtable_sg(src, src_sg, i) { > > + sg_set_page(dst_sg, sg_page(src_sg), src_sg->length, > > + src_sg->offset); > > + dst_sg = sg_next(dst_sg); > > + } > > + > > + return 0; > > +} > > + > > +static int tee_heap_attach(struct dma_buf *dmabuf, > > + struct dma_buf_attachment *attachment) > > +{ > > + struct tee_heap_buffer *buf = dmabuf->priv; > > + struct tee_heap_attachment *a; > > + int ret; > > + > > + a = kzalloc(sizeof(*a), GFP_KERNEL); > > + if (!a) > > + return -ENOMEM; > > + > > + ret = copy_sg_table(&a->table, &buf->table); > > + if (ret) { > > + kfree(a); > > + return ret; > > + } > > + > > + a->dev = attachment->dev; > > + attachment->priv = a; > > + > > + return 0; > > +} > > + > > +static void tee_heap_detach(struct dma_buf *dmabuf, > > + struct dma_buf_attachment *attachment) > > +{ > > + struct tee_heap_attachment *a = attachment->priv; > > + > > + sg_free_table(&a->table); > > + kfree(a); > > +} > > + > > +static struct sg_table * > > +tee_heap_map_dma_buf(struct dma_buf_attachment *attachment, > > + enum dma_data_direction direction) > > +{ > > + struct tee_heap_attachment *a = attachment->priv; > > + int ret; > > + > > + ret = dma_map_sgtable(attachment->dev, &a->table, direction, > > + DMA_ATTR_SKIP_CPU_SYNC); > > + if (ret) > > + return ERR_PTR(ret); > > + > > + return &a->table; > > +} > > + > > +static void tee_heap_unmap_dma_buf(struct dma_buf_attachment *attachment, > > + struct sg_table *table, > > + enum dma_data_direction direction) > > +{ > > + struct tee_heap_attachment *a = attachment->priv; > > + > > + WARN_ON(&a->table != table); > > + > > + dma_unmap_sgtable(attachment->dev, table, direction, > > + DMA_ATTR_SKIP_CPU_SYNC); > > +} > > + > > +static void tee_heap_buf_free(struct dma_buf *dmabuf) > > +{ > > + struct tee_heap_buffer *buf = dmabuf->priv; > > + struct tee_device *teedev = buf->teedev; > > + > > + buf->pool->ops->free(buf->pool, &buf->table); > > + tee_device_put(teedev); > > +} > > + > > +static const struct dma_buf_ops tee_heap_buf_ops = { > > + .attach = tee_heap_attach, > > + .detach = tee_heap_detach, > > + .map_dma_buf = tee_heap_map_dma_buf, > > + .unmap_dma_buf = tee_heap_unmap_dma_buf, > > + .release = tee_heap_buf_free, > > +}; > > + > > +static struct dma_buf *tee_dma_heap_alloc(struct dma_heap *heap, > > + unsigned long len, u32 fd_flags, > > + u64 heap_flags) > > +{ > > + struct tee_dma_heap *h = dma_heap_get_drvdata(heap); > > + DEFINE_DMA_BUF_EXPORT_INFO(exp_info); > > + struct tee_device *teedev = NULL; > > + struct tee_heap_buffer *buf; > > + struct tee_protmem_pool *pool; > > + struct dma_buf *dmabuf; > > + int rc; > > + > > + mutex_lock(&h->mu); > > + if (tee_device_get(h->teedev)) { > > + teedev = h->teedev; > > + pool = h->pool; > > + } > > + mutex_unlock(&h->mu); > > + > > + if (!teedev) > > + return ERR_PTR(-EINVAL); > > + > > + buf = kzalloc(sizeof(*buf), GFP_KERNEL); > > + if (!buf) { > > + dmabuf = ERR_PTR(-ENOMEM); > > + goto err; > > + } > > + buf->size = len; > > + buf->pool = pool; > > + buf->teedev = teedev; > > + > > + rc = pool->ops->alloc(pool, &buf->table, len, &buf->offs); > > + if (rc) { > > + dmabuf = ERR_PTR(rc); > > + goto err_kfree; > > + } > > + > > + exp_info.ops = &tee_heap_buf_ops; > > + exp_info.size = len; > > + exp_info.priv = buf; > > + exp_info.flags = fd_flags; > > + dmabuf = dma_buf_export(&exp_info); > > + if (IS_ERR(dmabuf)) > > + goto err_protmem_free; > > + > > + return dmabuf; > > + > > +err_protmem_free: > > + pool->ops->free(pool, &buf->table); > > +err_kfree: > > + kfree(buf); > > +err: > > + tee_device_put(h->teedev); > > + return dmabuf; > > +} > > + > > +static const struct dma_heap_ops tee_dma_heap_ops = { > > + .allocate = tee_dma_heap_alloc, > > +}; > > + > > +static const char *heap_id_2_name(enum tee_dma_heap_id id) > > +{ > > + switch (id) { > > + case TEE_DMA_HEAP_SECURE_VIDEO_PLAY: > > + return "protected,secure-video"; > > + case TEE_DMA_HEAP_TRUSTED_UI: > > + return "protected,trusted-ui"; > > + case TEE_DMA_HEAP_SECURE_VIDEO_RECORD: > > + return "protected,secure-video-record"; > > + default: > > + return NULL; > > + } > > +} > > + > > +static int alloc_dma_heap(struct tee_device *teedev, enum tee_dma_heap_id id, > > + struct tee_protmem_pool *pool) > > +{ > > + struct dma_heap_export_info exp_info = { > > + .ops = &tee_dma_heap_ops, > > + .name = heap_id_2_name(id), > > + }; > > + struct tee_dma_heap *h; > > + int rc; > > + > > + if (!exp_info.name) > > + return -EINVAL; > > + > > + if (xa_reserve(&tee_dma_heap, id, GFP_KERNEL)) { > > + if (!xa_load(&tee_dma_heap, id)) > > + return -EEXIST; > > + return -ENOMEM; > > + } > > + > > + h = kzalloc(sizeof(*h), GFP_KERNEL); > > + if (!h) > > + return -ENOMEM; > > + h->id = id; > > + h->teedev = teedev; > > + h->pool = pool; > > + mutex_init(&h->mu); > > + > > + exp_info.priv = h; > > + h->heap = dma_heap_add(&exp_info); > > + if (IS_ERR(h->heap)) { > > + rc = PTR_ERR(h->heap); > > + kfree(h); > > + > > + return rc; > > + } > > + > > + /* "can't fail" due to the call to xa_reserve() above */ > > + return WARN(xa_store(&tee_dma_heap, id, h, GFP_KERNEL), > > + "xa_store() failed"); > > I think this can rather be simplified to: > > return WARN_ON(xa_is_err(xa_store(&tee_dma_heap, id, h, GFP_KERNEL))); OK > > > +} > > + > > +int tee_device_register_dma_heap(struct tee_device *teedev, > > + enum tee_dma_heap_id id, > > + struct tee_protmem_pool *pool) > > +{ > > + struct tee_dma_heap *h; > > + int rc; > > + > > + h = xa_load(&tee_dma_heap, id); > > + if (h) { > > + mutex_lock(&h->mu); > > + if (h->teedev) { > > + rc = -EBUSY; > > + } else { > > + h->teedev = teedev; > > + h->pool = pool; > > + rc = 0; > > + } > > + mutex_unlock(&h->mu); > > + } else { > > + rc = alloc_dma_heap(teedev, id, pool); > > + } > > + > > + if (rc) > > + dev_err(&teedev->dev, "can't register DMA heap id %d (%s)\n", > > + id, heap_id_2_name(id)); > > + > > + return rc; > > +} > > +EXPORT_SYMBOL_GPL(tee_device_register_dma_heap); > > + > > +void tee_device_unregister_all_dma_heaps(struct tee_device *teedev) > > +{ > > + struct tee_protmem_pool *pool; > > + struct tee_dma_heap *h; > > + u_long i; > > + > > + xa_for_each(&tee_dma_heap, i, h) { > > + if (h) { > > + pool = NULL; > > + mutex_lock(&h->mu); > > + if (h->teedev == teedev) { > > + pool = h->pool; > > + h->teedev = NULL; > > + h->pool = NULL; > > + } > > + mutex_unlock(&h->mu); > > + if (pool) > > + pool->ops->destroy_pool(pool); > > + } > > + } > > +} > > +EXPORT_SYMBOL_GPL(tee_device_unregister_all_dma_heaps); > > + > > +int tee_heap_update_from_dma_buf(struct tee_device *teedev, > > + struct dma_buf *dmabuf, size_t *offset, > > + struct tee_shm *shm, > > + struct tee_shm **parent_shm) > > +{ > > + struct tee_heap_buffer *buf; > > + int rc; > > + > > + /* The DMA-buf must be from our heap */ > > + if (dmabuf->ops != &tee_heap_buf_ops) > > + return -EINVAL; > > + > > + buf = dmabuf->priv; > > + /* The buffer must be from the same teedev */ > > + if (buf->teedev != teedev) > > + return -EINVAL; > > + > > + shm->size = buf->size; > > + > > + rc = buf->pool->ops->update_shm(buf->pool, &buf->table, buf->offs, shm, > > + parent_shm); > > + if (!rc && *parent_shm) > > + *offset = buf->offs; > > + > > + return rc; > > +} > > +#else > > +int tee_device_register_dma_heap(struct tee_device *teedev __always_unused, > > + enum tee_dma_heap_id id __always_unused, > > + struct tee_protmem_pool *pool __always_unused) > > +{ > > + return -EINVAL; > > +} > > +EXPORT_SYMBOL_GPL(tee_device_register_dma_heap); > > + > > +void > > +tee_device_unregister_all_dma_heaps(struct tee_device *teedev __always_unused) > > +{ > > +} > > +EXPORT_SYMBOL_GPL(tee_device_unregister_all_dma_heaps); > > + > > +int tee_heap_update_from_dma_buf(struct tee_device *teedev __always_unused, > > + struct dma_buf *dmabuf __always_unused, > > + size_t *offset __always_unused, > > + struct tee_shm *shm __always_unused, > > + struct tee_shm **parent_shm __always_unused) > > +{ > > + return -EINVAL; > > +} > > +#endif > > + > > +static struct tee_protmem_static_pool * > > +to_protmem_static_pool(struct tee_protmem_pool *pool) > > +{ > > + return container_of(pool, struct tee_protmem_static_pool, pool); > > +} > > + > > +static int protmem_pool_op_static_alloc(struct tee_protmem_pool *pool, > > + struct sg_table *sgt, size_t size, > > + size_t *offs) > > +{ > > + struct tee_protmem_static_pool *stp = to_protmem_static_pool(pool); > > + phys_addr_t pa; > > + int ret; > > + > > + pa = gen_pool_alloc(stp->gen_pool, size); > > + if (!pa) > > + return -ENOMEM; > > + > > + ret = sg_alloc_table(sgt, 1, GFP_KERNEL); > > + if (ret) { > > + gen_pool_free(stp->gen_pool, pa, size); > > + return ret; > > + } > > + > > + sg_set_page(sgt->sgl, phys_to_page(pa), size, 0); > > Did you missed pfn_valid() check from prior v8 review comments? No, I tried it, but pfn_valid() didn't like the address, so I had to find a way to fix it. tee_protmem_static_pool_alloc(), below, calls memremap() on the range, and that should make pfn_valid() redundant. > > > + *offs = pa - stp->pa_base; > > + > > + return 0; > > +} > > + > > +static void protmem_pool_op_static_free(struct tee_protmem_pool *pool, > > + struct sg_table *sgt) > > +{ > > + struct tee_protmem_static_pool *stp = to_protmem_static_pool(pool); > > + struct scatterlist *sg; > > + int i; > > + > > + for_each_sgtable_sg(sgt, sg, i) > > + gen_pool_free(stp->gen_pool, sg_phys(sg), sg->length); > > + sg_free_table(sgt); > > +} > > + > > +static int protmem_pool_op_static_update_shm(struct tee_protmem_pool *pool, > > + struct sg_table *sgt, size_t offs, > > + struct tee_shm *shm, > > + struct tee_shm **parent_shm) > > +{ > > + struct tee_protmem_static_pool *stp = to_protmem_static_pool(pool); > > + > > + shm->paddr = stp->pa_base + offs; > > + *parent_shm = NULL; > > + > > + return 0; > > +} > > + > > +static void protmem_pool_op_static_destroy_pool(struct tee_protmem_pool *pool) > > +{ > > + struct tee_protmem_static_pool *stp = to_protmem_static_pool(pool); > > + > > + gen_pool_destroy(stp->gen_pool); > > + memunmap(stp->base); > > + kfree(stp); > > +} > > + > > +static struct tee_protmem_pool_ops protmem_pool_ops_static = { > > + .alloc = protmem_pool_op_static_alloc, > > + .free = protmem_pool_op_static_free, > > + .update_shm = protmem_pool_op_static_update_shm, > > + .destroy_pool = protmem_pool_op_static_destroy_pool, > > +}; > > + > > +struct tee_protmem_pool *tee_protmem_static_pool_alloc(phys_addr_t paddr, > > + size_t size) > > +{ > > + const size_t page_mask = PAGE_SIZE - 1; > > + struct tee_protmem_static_pool *stp; > > + int rc; > > + > > + /* Check it's page aligned */ > > + if ((paddr | size) & page_mask) > > + return ERR_PTR(-EINVAL); > > + > > + stp = kzalloc(sizeof(*stp), GFP_KERNEL); > > + if (!stp) > > + return ERR_PTR(-ENOMEM); > > + > > + /* > > + * Map the memory as uncached to make sure the kernel can work with > > + * __pfn_to_page() and friends since that's needed when passing the > > + * protected DMA-buf to a device. The memory should otherwise not > > + * be touched by the kernel since it's likely to cause an external > > + * abort due to the protection status. > > + */ > > + stp->base = memremap(paddr, size, MEMREMAP_WC); > > + if (!stp->base) { > > + rc = -EINVAL; > > + goto err_free; > > + } > > + > > + stp->gen_pool = gen_pool_create(PAGE_SHIFT, -1); > > + if (!stp->gen_pool) { > > + rc = -ENOMEM; > > + goto err_unmap; > > + } > > + > > + rc = gen_pool_add(stp->gen_pool, paddr, size, -1); > > + if (rc) > > + goto err_free_pool; > > + > > + stp->pool.ops = &protmem_pool_ops_static; > > + stp->pa_base = paddr; > > + return &stp->pool; > > + > > +err_free_pool: > > + gen_pool_destroy(stp->gen_pool); > > +err_unmap: > > + memunmap(stp->base); > > +err_free: > > + kfree(stp); > > + > > + return ERR_PTR(rc); > > +} > > +EXPORT_SYMBOL_GPL(tee_protmem_static_pool_alloc); > > diff --git a/drivers/tee/tee_private.h b/drivers/tee/tee_private.h > > index 9bc50605227c..6c6ff5d5eed2 100644 > > --- a/drivers/tee/tee_private.h > > +++ b/drivers/tee/tee_private.h > > @@ -8,6 +8,7 @@ > > #include <linux/cdev.h> > > #include <linux/completion.h> > > #include <linux/device.h> > > +#include <linux/dma-buf.h> > > #include <linux/kref.h> > > #include <linux/mutex.h> > > #include <linux/types.h> > > @@ -24,4 +25,9 @@ struct tee_shm *tee_shm_alloc_user_buf(struct tee_context *ctx, size_t size); > > struct tee_shm *tee_shm_register_user_buf(struct tee_context *ctx, > > unsigned long addr, size_t length); > > > > +int tee_heap_update_from_dma_buf(struct tee_device *teedev, > > + struct dma_buf *dmabuf, size_t *offset, > > + struct tee_shm *shm, > > + struct tee_shm **parent_shm); > > + > > #endif /*TEE_PRIVATE_H*/ > > diff --git a/include/linux/tee_core.h b/include/linux/tee_core.h > > index a38494d6b5f4..b8b99c97e00c 100644 > > --- a/include/linux/tee_core.h > > +++ b/include/linux/tee_core.h > > @@ -8,9 +8,11 @@ > > > > #include <linux/cdev.h> > > #include <linux/device.h> > > +#include <linux/dma-buf.h> > > #include <linux/idr.h> > > #include <linux/kref.h> > > #include <linux/list.h> > > +#include <linux/scatterlist.h> > > #include <linux/tee.h> > > #include <linux/tee_drv.h> > > #include <linux/types.h> > > @@ -30,6 +32,12 @@ > > #define TEE_DEVICE_FLAG_REGISTERED 0x1 > > #define TEE_MAX_DEV_NAME_LEN 32 > > > > +enum tee_dma_heap_id { > > + TEE_DMA_HEAP_SECURE_VIDEO_PLAY = 1, > > + TEE_DMA_HEAP_TRUSTED_UI, > > + TEE_DMA_HEAP_SECURE_VIDEO_RECORD, > > +}; > > + > > /** > > * struct tee_device - TEE Device representation > > * @name: name of device > > @@ -116,6 +124,36 @@ struct tee_desc { > > u32 flags; > > }; > > > > +/** > > + * struct tee_protmem_pool - protected memory pool > > + * @ops: operations > > + * > > + * This is an abstract interface where this struct is expected to be > > + * embedded in another struct specific to the implementation. > > + */ > > +struct tee_protmem_pool { > > + const struct tee_protmem_pool_ops *ops; > > +}; > > + > > +/** > > + * struct tee_protmem_pool_ops - protected memory pool operations > > + * @alloc: called when allocating protected memory > > + * @free: called when freeing protected memory > > + * @update_shm: called when registering a dma-buf to update the @shm > > + * with physical address of the buffer or to return the > > + * @parent_shm of the memory pool > > + * @destroy_pool: called when destroying the pool > > + */ > > +struct tee_protmem_pool_ops { > > + int (*alloc)(struct tee_protmem_pool *pool, struct sg_table *sgt, > > + size_t size, size_t *offs); > > + void (*free)(struct tee_protmem_pool *pool, struct sg_table *sgt); > > + int (*update_shm)(struct tee_protmem_pool *pool, struct sg_table *sgt, > > + size_t offs, struct tee_shm *shm, > > + struct tee_shm **parent_shm); > > + void (*destroy_pool)(struct tee_protmem_pool *pool); > > +}; > > + > > /** > > * tee_device_alloc() - Allocate a new struct tee_device instance > > * @teedesc: Descriptor for this driver > > @@ -154,6 +192,11 @@ int tee_device_register(struct tee_device *teedev); > > */ > > void tee_device_unregister(struct tee_device *teedev); > > > > +int tee_device_register_dma_heap(struct tee_device *teedev, > > + enum tee_dma_heap_id id, > > + struct tee_protmem_pool *pool); > > +void tee_device_unregister_all_dma_heaps(struct tee_device *teedev); > > + > > /** > > * tee_device_set_dev_groups() - Set device attribute groups > > * @teedev: Device to register > > @@ -229,6 +272,28 @@ static inline void tee_shm_pool_free(struct tee_shm_pool *pool) > > pool->ops->destroy_pool(pool); > > } > > > > +/** > > + * tee_protmem_static_pool_alloc() - Create a protected memory manager > > + * @paddr: Physical address of start of pool > > + * @size: Size in bytes of the pool > > + * > > + * @returns pointer to a 'struct tee_shm_pool' or an ERR_PTR on failure. > > s/tee_shm_pool/tee_protmem_pool/ I'll fix it. > > Rest looks fine to me. Thanks, Jens > > -Sumit > > > + */ > > +struct tee_protmem_pool *tee_protmem_static_pool_alloc(phys_addr_t paddr, > > + size_t size); > > + > > +/** > > + * tee_protmem_pool_free() - Free a protected memory pool > > + * @pool: The protected memory pool to free > > + * > > + * There must be no remaining protected memory allocated from this pool > > + * when this function is called. > > + */ > > +static inline void tee_protmem_pool_free(struct tee_protmem_pool *pool) > > +{ > > + pool->ops->destroy_pool(pool); > > +} > > + > > /** > > * tee_get_drvdata() - Return driver_data pointer > > * @returns the driver_data pointer supplied to tee_register(). > > -- > > 2.43.0 > >

1 year, 1 month

Re: [PATCH v3 01/10] dt-bindings: npu: rockchip,rknn: Add bindings

by Rob Herring (Arm)

On Fri, 16 May 2025 18:53:15 +0200, Tomeu Vizoso wrote: > Add the bindings for the Neural Processing Unit IP from Rockchip. > > v2: > - Adapt to new node structure (one node per core, each with its own > IOMMU) > - Several misc. fixes from Sebastian Reichel > > v3: > - Split register block in its constituent subblocks, and only require > the ones that the kernel would ever use (Nicolas Frattaroli) > - Group supplies (Rob Herring) > - Explain the way in which the top core is special (Rob Herring) > > Signed-off-by: Tomeu Vizoso <tomeu(a)tomeuvizoso.net> > Signed-off-by: Sebastian Reichel <sebastian.reichel(a)collabora.com> > --- > .../bindings/npu/rockchip,rknn-core.yaml | 162 +++++++++++++++++++++ > 1 file changed, 162 insertions(+) > My bot found errors running 'make dt_binding_check' on your patch: yamllint warnings/errors: dtschema/dtc warnings/errors: /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/npu/rockchip,rknn-core.yaml: properties:reg-names: 'oneOf' conditional failed, one must be fixed: [{'const': 'pc'}, {'const': 'cna'}, {'const': 'core'}] is too long [{'const': 'pc'}, {'const': 'cna'}, {'const': 'core'}] is too short False schema does not allow 3 1 was expected 3 is greater than the maximum of 2 hint: "minItems" is only needed if less than the "items" list length from schema $id: http://devicetree.org/meta-schemas/items.yaml# /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/npu/rockchip,rknn-core.example.dtb: npu-core@fdab0000 (rockchip,rk3588-rknn-core-top): compatible: 'oneOf' conditional failed, one must be fixed: ['rockchip,rk3588-rknn-core-top', 'rockchip,rknn-core-top'] is too long 'rockchip,rk3588-rknn-core-top' is not one of ['rockchip,rk3588-rknn-core'] from schema $id: http://devicetree.org/schemas/npu/rockchip,rknn-core.yaml# /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/npu/rockchip,rknn-core.example.dtb: npu-core@fdab0000 (rockchip,rk3588-rknn-core-top): reg: [[0, 4255842304, 0, 36864]] is too short from schema $id: http://devicetree.org/schemas/npu/rockchip,rknn-core.yaml# /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/npu/rockchip,rknn-core.example.dtb: npu-core@fdac0000 (rockchip,rk3588-rknn-core): compatible: 'oneOf' conditional failed, one must be fixed: ['rockchip,rk3588-rknn-core', 'rockchip,rknn-core'] is too long 'rockchip,rk3588-rknn-core' is not one of ['rockchip,rk3588-rknn-core-top'] from schema $id: http://devicetree.org/schemas/npu/rockchip,rknn-core.yaml# /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/npu/rockchip,rknn-core.example.dtb: npu-core@fdac0000 (rockchip,rk3588-rknn-core): reg: [[0, 4255907840, 0, 36864]] is too short from schema $id: http://devicetree.org/schemas/npu/rockchip,rknn-core.yaml# doc reference errors (make refcheckdocs): See https://patchwork.ozlabs.org/project/devicetree-bindings/patch/20250516-6-1… The base for the series is generally the latest rc1. A different dependency should be noted in *this* patch. If you already ran 'make dt_binding_check' and didn't see the above error(s), then make sure 'yamllint' is installed and dt-schema is up to date: pip3 install dtschema --upgrade Please check and re-submit after running the above command yourself. Note that DT_SCHEMA_FILES can be set to your schema file to speed up checking your schema. However, it must be unset to test all examples with your schema.

1 year, 1 month

[PATCH v9 0/9] TEE subsystem for protected dma-buf allocations

by Jens Wiklander

Hi, This patch set allocates the protected DMA-bufs from a DMA-heap instantiated from the TEE subsystem. The TEE subsystem handles the DMA-buf allocations since it is the TEE (OP-TEE, AMD-TEE, TS-TEE, or perhaps a future QTEE) which sets up the protection for the memory used for the DMA-bufs. The DMA-heap uses a protected memory pool provided by the backend TEE driver, allowing it to choose how to allocate the protected physical memory. The allocated DMA-bufs must be imported with a new TEE_IOC_SHM_REGISTER_FD before they can be passed as arguments when requesting services from the secure world. Three use-cases (Secure Video Playback, Trusted UI, and Secure Video Recording) have been identified so far to serve as examples of what can be expected. The use-cases have predefined DMA-heap names, "protected,secure-video", "protected,trusted-ui", and "protected,secure-video-record". The backend driver registers protected memory pools for the use-cases it supports. Each use-case has its own protected memory pool since different use-cases require isolation from different parts of the system. A protected memory pool can be based on a static carveout instantiated while probing the TEE backend driver, or dynamically allocated from CMA (dma_alloc_pages()) and made protected as needed by the TEE. This can be tested on a RockPi 4B+ with the following steps: repo init -u https://github.com/jenswi-linaro/manifest.git -m rockpi4.xml \ -b prototype/sdp-v9 repo sync -j8 cd build make toolchains -j$(nproc) make all -j$(nproc) # Copy ../out/rockpi4.img to an SD card and boot the RockPi from that # Connect a monitor to the RockPi # login and at the prompt: gst-launch-1.0 videotestsrc ! \ aesenc key=1f9423681beb9a79215820f6bda73d0f \ iv=e9aa8e834d8d70b7e0d254ff670dd718 serialize-iv=true ! \ aesdec key=1f9423681beb9a79215820f6bda73d0f ! \ kmssink The aesdec module has been hacked to use an OP-TEE TA to decrypt the stream into protected DMA-bufs which are consumed by the kmssink. The primitive QEMU tests from previous patch sets can be tested on RockPi in the same way using: xtest --sdp-basic The primitive tests are tested on QEMU with the following steps: repo init -u https://github.com/jenswi-linaro/manifest.git -m qemu_v8.xml \ -b prototype/sdp-v9 repo sync -j8 cd build make toolchains -j$(nproc) make SPMC_AT_EL=1 all -j$(nproc) make SPMC_AT_EL=1 run-only # login and at the prompt: xtest --sdp-basic The SPMC_AT_EL=1 parameter configures the build with FF-A and an SPMC at S-EL1 inside OP-TEE. The parameter can be changed to SPMC_AT_EL=n to test without FF-A using the original SMC ABI instead. Please remember to do %make arm-tf-clean for TF-A to be rebuilt properly using the new configuration. https://optee.readthedocs.io/en/latest/building/prerequisites.html list dependencies required to build the above. The primitive tests are pretty basic, mostly checking that a Trusted Application in the secure world can access and manipulate the memory. There are also some negative tests for out of bounds buffers, etc. Thanks, Jens Changes since V8: * Using dma_alloc_pages() instead of cma_alloc() so the direct dependency on CMA can be removed together with the patches "cma: export cma_alloc() and cma_release()" and "dma-contiguous: export dma_contiguous_default_area". The patch * Renaming the patch "tee: add tee_shm_alloc_cma_phys_mem()" to "tee: add tee_shm_alloc_dma_mem()" * Setting DMA mask for the OP-TEE TEE device based on input from the secure world instead of relying on the parent device so following patches are removed: "tee: tee_device_alloc(): copy dma_mask from parent device" and "optee: pass parent device to tee_device_alloc()". * Adding Sumit Garg's R-B to "tee: refactor params_from_user()" * In the patch "tee: implement protected DMA-heap", map the physical memory passed to tee_protmem_static_pool_alloc(). Changes since V7: * Adding "dma-buf: dma-heap: export declared functions", "cma: export cma_alloc() and cma_release()", and "dma-contiguous: export dma_contiguous_default_area" to export the symbols needed to keep the TEE subsystem as a load module. * Removing CONFIG_TEE_DMABUF_HEAP and CONFIG_TEE_CMA since they aren't needed any longer. * Addressing review comments in "optee: sync secure world ABI headers" * Better align protected memory pool initialization between the smc-abi and ffa-abi parts of the optee driver. * Removing the patch "optee: account for direction while converting parameters" Changes since V6: * Restricted memory is now known as protected memory since to use the same term as https://docs.vulkan.org/guide/latest/protected.html. Update all patches to consistently use protected memory. * In "tee: implement protected DMA-heap" add the hidden config option TEE_DMABUF_HEAP to tell if the DMABUF_HEAPS functions are available for the TEE subsystem * Adding "tee: refactor params_from_user()", broken out from the patch "tee: new ioctl to a register tee_shm from a dmabuf file descriptor" * For "tee: new ioctl to a register tee_shm from a dmabuf file descriptor": - Update commit message to mention protected memory - Remove and open code tee_shm_get_parent_shm() in param_from_user_memref() * In "tee: add tee_shm_alloc_cma_phys_mem" add the hidden config option TEE_CMA to tell if the CMA functions are available for the TEE subsystem * For "tee: tee_device_alloc(): copy dma_mask from parent device" and "optee: pass parent device to tee_device_alloc", added Reviewed-by: Sumit Garg <sumit.garg(a)kernel.org> Changes since V5: * Removing "tee: add restricted memory allocation" and "tee: add TEE_IOC_RSTMEM_FD_INFO" * Adding "tee: implement restricted DMA-heap", "tee: new ioctl to a register tee_shm from a dmabuf file descriptor", "tee: add tee_shm_alloc_cma_phys_mem()", "optee: pass parent device to tee_device_alloc()", and "tee: tee_device_alloc(): copy dma_mask from parent device" * The two TEE driver OPs "rstmem_alloc()" and "rstmem_free()" are replaced with a struct tee_rstmem_pool abstraction. * Replaced the the TEE_IOC_RSTMEM_ALLOC user space API with the DMA-heap API Changes since V4: * Adding the patch "tee: add TEE_IOC_RSTMEM_FD_INFO" needed by the GStreamer demo * Removing the dummy CPU access and mmap functions from the dma_buf_ops * Fixing a compile error in "optee: FF-A: dynamic restricted memory allocation" reported by kernel test robot <lkp(a)intel.com> Changes since V3: * Make the use_case and flags field in struct tee_shm u32's instead of u16's * Add more description for TEE_IOC_RSTMEM_ALLOC in the header file * Import namespace DMA_BUF in module tee, reported by lkp(a)intel.com * Added a note in the commit message for "optee: account for direction while converting parameters" why it's needed * Factor out dynamic restricted memory allocation from "optee: support restricted memory allocation" into two new commits "optee: FF-A: dynamic restricted memory allocation" and "optee: smc abi: dynamic restricted memory allocation" * Guard CMA usage with #ifdef CONFIG_CMA, effectively disabling dynamic restricted memory allocate if CMA isn't configured Changes since the V2 RFC: * Based on v6.12 * Replaced the flags for SVP and Trusted UID memory with a u32 field with unique id for each use case * Added dynamic allocation of restricted memory pools * Added OP-TEE ABI both with and without FF-A for dynamic restricted memory * Added support for FF-A with FFA_LEND Changes since the V1 RFC: * Based on v6.11 * Complete rewrite, replacing the restricted heap with TEE_IOC_RSTMEM_ALLOC Changes since Olivier's post [2]: * Based on Yong Wu's post [1] where much of dma-buf handling is done in the generic restricted heap * Simplifications and cleanup * New commit message for "dma-buf: heaps: add Linaro restricted dmabuf heap support" * Replaced the word "secure" with "restricted" where applicable Etienne Carriere (1): tee: new ioctl to a register tee_shm from a dmabuf file descriptor Jens Wiklander (8): optee: sync secure world ABI headers dma-buf: dma-heap: export declared functions tee: implement protected DMA-heap tee: refactor params_from_user() tee: add tee_shm_alloc_dma_mem() optee: support protected memory allocation optee: FF-A: dynamic protected memory allocation optee: smc abi: dynamic protected memory allocation drivers/dma-buf/dma-heap.c | 3 + drivers/tee/Makefile | 1 + drivers/tee/optee/Makefile | 1 + drivers/tee/optee/core.c | 10 + drivers/tee/optee/ffa_abi.c | 147 ++++++++- drivers/tee/optee/optee_ffa.h | 27 +- drivers/tee/optee/optee_msg.h | 84 +++++- drivers/tee/optee/optee_private.h | 15 +- drivers/tee/optee/optee_smc.h | 37 ++- drivers/tee/optee/protmem.c | 332 ++++++++++++++++++++ drivers/tee/optee/smc_abi.c | 113 ++++++- drivers/tee/tee_core.c | 155 +++++++--- drivers/tee/tee_heap.c | 487 ++++++++++++++++++++++++++++++ drivers/tee/tee_private.h | 16 + drivers/tee/tee_shm.c | 183 ++++++++++- include/linux/tee_core.h | 71 +++++ include/linux/tee_drv.h | 10 + include/uapi/linux/tee.h | 31 ++ 18 files changed, 1655 insertions(+), 68 deletions(-) create mode 100644 drivers/tee/optee/protmem.c create mode 100644 drivers/tee/tee_heap.c base-commit: b4432656b36e5cc1d50a1f2dc15357543add530e -- 2.43.0

1 year, 1 month

Re: [PATCH 2/2] dmabuf/heaps: implement DMA_BUF_IOCTL_RW_FILE for system_heap

by Christian König

On 5/22/25 10:02, wangtao wrote: >> -----Original Message----- >> From: Christian König <christian.koenig(a)amd.com> >> Sent: Wednesday, May 21, 2025 7:57 PM >> To: wangtao <tao.wangtao(a)honor.com>; T.J. Mercier >> <tjmercier(a)google.com> >> Cc: sumit.semwal(a)linaro.org; benjamin.gaignard(a)collabora.com; >> Brian.Starkey(a)arm.com; jstultz(a)google.com; linux-media(a)vger.kernel.org; >> dri-devel(a)lists.freedesktop.org; linaro-mm-sig(a)lists.linaro.org; linux- >> kernel(a)vger.kernel.org; wangbintian(BintianWang) >> <bintian.wang(a)honor.com>; yipengxiang <yipengxiang(a)honor.com>; liulu >> 00013167 <liulu.liu(a)honor.com>; hanfeng 00012985 <feng.han(a)honor.com>; >> amir73il(a)gmail.com >> Subject: Re: [PATCH 2/2] dmabuf/heaps: implement >> DMA_BUF_IOCTL_RW_FILE for system_heap >> >> On 5/21/25 12:25, wangtao wrote: >>> [wangtao] I previously explained that >>> read/sendfile/splice/copy_file_range >>> syscalls can't achieve dmabuf direct IO zero-copy. >> >> And why can't you work on improving those syscalls instead of creating a new >> IOCTL? >> > [wangtao] As I mentioned in previous emails, these syscalls cannot > achieve dmabuf zero-copy due to technical constraints. Yeah, and why can't you work on removing those technical constrains? What is blocking you from improving the sendfile system call or proposing a patch to remove the copy_file_range restrictions? Regards, Christian. Could you > specify the technical points, code, or principles that need > optimization? > > Let me explain again why these syscalls can't work: > 1. read() syscall > - dmabuf fops lacks read callback implementation. Even if implemented, > file_fd info cannot be transferred > - read(file_fd, dmabuf_ptr, len) with remap_pfn_range-based mmap > cannot access dmabuf_buf pages, forcing buffer-mode reads > > 2. sendfile() syscall > - Requires CPU copy from page cache to memory file(tmpfs/shmem): > [DISK] --DMA--> [page cache] --CPU copy--> [MEMORY file] > - CPU overhead (both buffer/direct modes involve copies): > 55.08% do_sendfile > |- 55.08% do_splice_direct > |-|- 55.08% splice_direct_to_actor > |-|-|- 22.51% copy_splice_read > |-|-|-|- 16.57% f2fs_file_read_iter > |-|-|-|-|- 15.12% __iomap_dio_rw > |-|-|- 32.33% direct_splice_actor > |-|-|-|- 32.11% iter_file_splice_write > |-|-|-|-|- 28.42% vfs_iter_write > |-|-|-|-|-|- 28.42% do_iter_write > |-|-|-|-|-|-|- 28.39% shmem_file_write_iter > |-|-|-|-|-|-|-|- 24.62% generic_perform_write > |-|-|-|-|-|-|-|-|- 18.75% __pi_memmove > > 3. splice() requires one end to be a pipe, incompatible with regular files or dmabuf. > > 4. copy_file_range() > - Blocked by cross-FS restrictions (Amir's commit 868f9f2f8e00) > - Even without this restriction, Even without restrictions, implementing > the copy_file_range callback in dmabuf fops would only allow dmabuf read > from regular files. This is because copy_file_range relies on > file_out->f_op->copy_file_range, which cannot support dmabuf write > operations to regular files. > > Test results confirm these limitations: > T.J. Mercier's 1G from ext4 on 6.12.20 | read/sendfile (ms) w/ 3 > drop_caches > ------------------------|------------------- > udmabuf buffer read | 1210 > udmabuf direct read | 671 > udmabuf buffer sendfile | 1096 > udmabuf direct sendfile | 2340 > > My 3GHz CPU tests (cache cleared): > Method | alloc | read | vs. (%) > ----------------------------------------------- > udmabuf buffer read | 135 | 546 | 180% > udmabuf direct read | 159 | 300 | 99% > udmabuf buffer sendfile | 134 | 303 | 100% > udmabuf direct sendfile | 141 | 912 | 301% > dmabuf buffer read | 22 | 362 | 119% > my patch direct read | 29 | 265 | 87% > > My 1GHz CPU tests (cache cleared): > Method | alloc | read | vs. (%) > ----------------------------------------------- > udmabuf buffer read | 552 | 2067 | 198% > udmabuf direct read | 540 | 627 | 60% > udmabuf buffer sendfile | 497 | 1045 | 100% > udmabuf direct sendfile | 527 | 2330 | 223% > dmabuf buffer read | 40 | 1111 | 106% > patch direct read | 44 | 310 | 30% > > Test observations align with expectations: > 1. dmabuf buffer read requires slow CPU copies > 2. udmabuf direct read achieves zero-copy but has page retrieval > latency from vaddr > 3. udmabuf buffer sendfile suffers CPU copy overhead > 4. udmabuf direct sendfile combines CPU copies with frequent DMA > operations due to small pipe buffers > 5. dmabuf buffer read also requires CPU copies > 6. My direct read patch enables zero-copy with better performance > on low-power CPUs > 7. udmabuf creation time remains problematic (as you’ve noted). > >>> My focus is enabling dmabuf direct I/O for [regular file] <--DMA--> >>> [dmabuf] zero-copy. >> >> Yeah and that focus is wrong. You need to work on a general solution to the >> issue and not specific to your problem. >> >>> Any API achieving this would work. Are there other uAPIs you think >>> could help? Could you recommend experts who might offer suggestions? >> >> Well once more: Either work on sendfile or copy_file_range or eventually >> splice to make it what you want to do. >> >> When that is done we can discuss with the VFS people if that approach is >> feasible. >> >> But just bypassing the VFS review by implementing a DMA-buf specific IOCTL >> is a NO-GO. That is clearly not something you can do in any way. > [wangtao] The issue is that only dmabuf lacks Direct I/O zero-copy support. Tmpfs/shmem > already work with Direct I/O zero-copy. As explained, existing syscalls or > generic methods can't enable dmabuf direct I/O zero-copy, which is why I > propose adding an IOCTL command. > > I respect your perspective. Could you clarify specific technical aspects, > code requirements, or implementation principles for modifying sendfile() > or copy_file_range()? This would help advance our discussion. > > Thank you for engaging in this dialogue. > >> >> Regards, >> Christian.

1 year, 1 month

[PATCH v2 00/12] Rust abstractions for shmem-backed GEM objects

by Lyude Paul

This is the next version of the shmem backed GEM objects series originally from Asahi, previously posted by Daniel Almeida. Along with bindings for shmem backed GEM objects, it also adds a few features that various users like Tyr and Asahi are interested in: * The ability to pass custom arguments to new GEM objects (needed by Tyr) * OpaqueObject (to enable the use of custom private GEM objects, which I believe asahi wanted) And replaces some of the hand-rolled API bindings (sg_table mainly) with some of the WIP patch series for adding kernel-wide bindings. It also addresses the comments from the code review of the last version of this patch series. Currently doesn't apply on an upstream branch, but should very soon as all of the dependencies in this series are on a mailing list already. The current branch this can be applied on top of is here: https://gitlab.freedesktop.org/lyudess/linux/-/commits/rust%2Fgem-shmem-base Which is based on top of nova/nova-next with the following patch series applied: * My (hopefully final) gem bindings cleanup: https://lkml.org/lkml/2025/5/20/1541 * Benno's derive Zeroable series: https://lkml.org/lkml/2025/5/20/1446 * Abdiel's sg_table series: https://lwn.net/Articles/1020986/ Also, there is one FIXES patch on top of Abdiel's work to fix some iterator bugs. These fixes have already been mentioned on the mailing list and should not be needed for their V2 version Asahi Lina (3): rust: helpers: Add bindings/wrappers for dma_resv_lock rust: drm: gem: shmem: Add DRM shmem helper abstraction rust: drm: gem: shmem: Add share_dma_resv to ObjectConfig Lyude Paul (9): rust: drm: gem: Add raw_dma_resv() function drm/gem/shmem: Extract drm_gem_shmem_init() from drm_gem_shmem_create() drm/gem/shmem: Extract drm_gem_shmem_release() from drm_gem_shmem_free() rust: gem: Introduce BaseDriverObject::Args rust: drm: gem: Add OpaqueObject rust: drm: gem: Introduce OwnedSGTable rust: Add dma_buf stub bindings rust: drm: gem: Add export() callback rust: drm: gem: Add BaseObject::prime_export() drivers/gpu/drm/drm_gem_shmem_helper.c | 98 +++++-- drivers/gpu/drm/nova/gem.rs | 6 +- include/drm/drm_gem_shmem_helper.h | 2 + rust/bindings/bindings_helper.h | 4 + rust/helpers/dma-resv.c | 13 + rust/helpers/drm.c | 48 +++- rust/helpers/helpers.c | 1 + rust/kernel/dma_buf.rs | 39 +++ rust/kernel/drm/gem/mod.rs | 187 ++++++++++++- rust/kernel/drm/gem/shmem.rs | 370 +++++++++++++++++++++++++ rust/kernel/lib.rs | 1 + 11 files changed, 727 insertions(+), 42 deletions(-) create mode 100644 rust/helpers/dma-resv.c create mode 100644 rust/kernel/dma_buf.rs create mode 100644 rust/kernel/drm/gem/shmem.rs -- 2.49.0

1 year, 1 month

Re: [PATCH v3 4/5] drm/panfrost: show device-wide list of DRM GEM objects over DebugFS

by Steven Price

On 20/05/2025 18:44, Adrián Larumbe wrote: > This change is essentially a Panfrost port of commit a3707f53eb3f > ("drm/panthor: show device-wide list of DRM GEM objects over DebugFS"). > > The DebugFS file is almost the same as in Panthor, minus the GEM object > usage flags, since Panfrost has no kernel-only BO's. > > Two additional GEM state flags which are displayed but aren't relevant > to Panthor are 'Purged' and 'Purgeable', since Panfrost implements an > explicit shrinker and a madvise ioctl to flag objects as reclaimable. > > Signed-off-by: Adrián Larumbe <adrian.larumbe(a)collabora.com> Reviewed-by: Steven Price <steven.price(a)arm.com> > --- > drivers/gpu/drm/panfrost/panfrost_device.c | 5 + > drivers/gpu/drm/panfrost/panfrost_device.h | 15 +++ > drivers/gpu/drm/panfrost/panfrost_drv.c | 35 ++++++ > drivers/gpu/drm/panfrost/panfrost_gem.c | 134 +++++++++++++++++++++ > drivers/gpu/drm/panfrost/panfrost_gem.h | 47 ++++++++ > 5 files changed, 236 insertions(+) > > diff --git a/drivers/gpu/drm/panfrost/panfrost_device.c b/drivers/gpu/drm/panfrost/panfrost_device.c > index 5d35076b2e6d..04bec27449cb 100644 > --- a/drivers/gpu/drm/panfrost/panfrost_device.c > +++ b/drivers/gpu/drm/panfrost/panfrost_device.c > @@ -209,6 +209,11 @@ int panfrost_device_init(struct panfrost_device *pfdev) > > spin_lock_init(&pfdev->cycle_counter.lock); > > +#ifdef CONFIG_DEBUG_FS > + mutex_init(&pfdev->debugfs.gems_lock); > + INIT_LIST_HEAD(&pfdev->debugfs.gems_list); > +#endif > + > err = panfrost_pm_domain_init(pfdev); > if (err) > return err; > diff --git a/drivers/gpu/drm/panfrost/panfrost_device.h b/drivers/gpu/drm/panfrost/panfrost_device.h > index dcff70f905cd..077525a3ad68 100644 > --- a/drivers/gpu/drm/panfrost/panfrost_device.h > +++ b/drivers/gpu/drm/panfrost/panfrost_device.h > @@ -111,6 +111,17 @@ struct panfrost_compatible { > u8 gpu_quirks; > }; > > +/** > + * struct panfrost_device_debugfs - Device-wide DebugFS tracking structures > + */ > +struct panfrost_device_debugfs { > + /** @gems_list: Device-wide list of GEM objects owned by at least one file. */ > + struct list_head gems_list; > + > + /** @gems_lock: Serializes access to the device-wide list of GEM objects. */ > + struct mutex gems_lock; > +}; > + > struct panfrost_device { > struct device *dev; > struct drm_device *ddev; > @@ -164,6 +175,10 @@ struct panfrost_device { > atomic_t use_count; > spinlock_t lock; > } cycle_counter; > + > +#ifdef CONFIG_DEBUG_FS > + struct panfrost_device_debugfs debugfs; > +#endif > }; > > struct panfrost_mmu { > diff --git a/drivers/gpu/drm/panfrost/panfrost_drv.c b/drivers/gpu/drm/panfrost/panfrost_drv.c > index 179fbaa1cd0c..f576cb215898 100644 > --- a/drivers/gpu/drm/panfrost/panfrost_drv.c > +++ b/drivers/gpu/drm/panfrost/panfrost_drv.c > @@ -13,6 +13,7 @@ > #include <linux/platform_device.h> > #include <linux/pm_runtime.h> > #include <drm/panfrost_drm.h> > +#include <drm/drm_debugfs.h> > #include <drm/drm_drv.h> > #include <drm/drm_ioctl.h> > #include <drm/drm_syncobj.h> > @@ -659,6 +660,37 @@ static const struct file_operations panfrost_drm_driver_fops = { > .show_fdinfo = drm_show_fdinfo, > }; > > +#ifdef CONFIG_DEBUG_FS > +static int panthor_gems_show(struct seq_file *m, void *data) > +{ > + struct drm_info_node *node = m->private; > + struct drm_device *dev = node->minor->dev; > + struct panfrost_device *pfdev = dev->dev_private; > + > + panfrost_gem_debugfs_print_bos(pfdev, m); > + > + return 0; > +} > + > +static struct drm_info_list panthor_debugfs_list[] = { > + {"gems", panthor_gems_show, 0, NULL}, > +}; > + > +static int panthor_gems_debugfs_init(struct drm_minor *minor) > +{ > + drm_debugfs_create_files(panthor_debugfs_list, > + ARRAY_SIZE(panthor_debugfs_list), > + minor->debugfs_root, minor); > + > + return 0; > +} > + > +static void panfrost_debugfs_init(struct drm_minor *minor) > +{ > + panthor_gems_debugfs_init(minor); > +} > +#endif > + > /* > * Panfrost driver version: > * - 1.0 - initial interface > @@ -683,6 +715,9 @@ static const struct drm_driver panfrost_drm_driver = { > > .gem_create_object = panfrost_gem_create_object, > .gem_prime_import_sg_table = panfrost_gem_prime_import_sg_table, > +#ifdef CONFIG_DEBUG_FS > + .debugfs_init = panfrost_debugfs_init, > +#endif > }; > > static int panfrost_probe(struct platform_device *pdev) > diff --git a/drivers/gpu/drm/panfrost/panfrost_gem.c b/drivers/gpu/drm/panfrost/panfrost_gem.c > index 04483d5fb45d..bb73f2a68a12 100644 > --- a/drivers/gpu/drm/panfrost/panfrost_gem.c > +++ b/drivers/gpu/drm/panfrost/panfrost_gem.c > @@ -12,6 +12,36 @@ > #include "panfrost_gem.h" > #include "panfrost_mmu.h" > > +#ifdef CONFIG_DEBUG_FS > +static void panfrost_gem_debugfs_bo_add(struct panfrost_device *pfdev, > + struct panfrost_gem_object *bo) > +{ > + bo->debugfs.creator.tgid = current->group_leader->pid; > + get_task_comm(bo->debugfs.creator.process_name, current->group_leader); > + > + mutex_lock(&pfdev->debugfs.gems_lock); > + list_add_tail(&bo->debugfs.node, &pfdev->debugfs.gems_list); > + mutex_unlock(&pfdev->debugfs.gems_lock); > +} > + > +static void panfrost_gem_debugfs_bo_rm(struct panfrost_gem_object *bo) > +{ > + struct panfrost_device *pfdev = bo->base.base.dev->dev_private; > + > + if (list_empty(&bo->debugfs.node)) > + return; > + > + mutex_lock(&pfdev->debugfs.gems_lock); > + list_del_init(&bo->debugfs.node); > + mutex_unlock(&pfdev->debugfs.gems_lock); > +} > +#else > +static void panfrost_gem_debugfs_bo_add(struct panfrost_device *pfdev, > + struct panfrost_gem_object *bo) > +{} > +static void panfrost_gem_debugfs_bo_rm(struct panfrost_gem_object *bo) {} > +#endif > + > /* Called DRM core on the last userspace/kernel unreference of the > * BO. > */ > @@ -37,6 +67,7 @@ static void panfrost_gem_free_object(struct drm_gem_object *obj) > WARN_ON_ONCE(!list_empty(&bo->mappings.list)); > > kfree_const(bo->label.str); > + panfrost_gem_debugfs_bo_rm(bo); > mutex_destroy(&bo->label.lock); > > if (bo->sgts) { > @@ -266,6 +297,8 @@ struct drm_gem_object *panfrost_gem_create_object(struct drm_device *dev, size_t > obj->base.map_wc = !pfdev->coherent; > mutex_init(&obj->label.lock); > > + panfrost_gem_debugfs_bo_add(pfdev, obj); > + > return &obj->base.base; > } > > @@ -354,3 +387,104 @@ panfrost_gem_internal_set_label(struct drm_gem_object *obj, const char *label) > > panfrost_gem_set_label(obj, str); > } > + > +#ifdef CONFIG_DEBUG_FS > +struct gem_size_totals { > + size_t size; > + size_t resident; > + size_t reclaimable; > +}; > + > +struct flag_def { > + u32 flag; > + const char *name; > +}; > + > +static void panfrost_gem_debugfs_print_flag_names(struct seq_file *m) > +{ > + int len; > + int i; > + > + static const struct flag_def gem_state_flags_names[] = { > + {PANFROST_DEBUGFS_GEM_STATE_FLAG_IMPORTED, "imported"}, > + {PANFROST_DEBUGFS_GEM_STATE_FLAG_EXPORTED, "exported"}, > + {PANFROST_DEBUGFS_GEM_STATE_FLAG_PURGED, "purged"}, > + {PANFROST_DEBUGFS_GEM_STATE_FLAG_PURGEABLE, "purgeable"}, > + }; > + > + seq_puts(m, "GEM state flags: "); > + for (i = 0, len = ARRAY_SIZE(gem_state_flags_names); i < len; i++) { > + seq_printf(m, "%s (0x%x)%s", gem_state_flags_names[i].name, > + gem_state_flags_names[i].flag, (i < len - 1) ? ", " : "\n\n"); > + } > +} > + > +static void panfrost_gem_debugfs_bo_print(struct panfrost_gem_object *bo, > + struct seq_file *m, > + struct gem_size_totals *totals) > +{ > + unsigned int refcount = kref_read(&bo->base.base.refcount); > + char creator_info[32] = {}; > + size_t resident_size; > + u32 gem_state_flags = 0; > + > + /* Skip BOs being destroyed. */ > + if (!refcount) > + return; > + > + resident_size = bo->base.pages ? bo->base.base.size : 0; > + > + snprintf(creator_info, sizeof(creator_info), > + "%s/%d", bo->debugfs.creator.process_name, bo->debugfs.creator.tgid); > + seq_printf(m, "%-32s%-16d%-16d%-16zd%-16zd0x%-16lx", > + creator_info, > + bo->base.base.name, > + refcount, > + bo->base.base.size, > + resident_size, > + drm_vma_node_start(&bo->base.base.vma_node)); > + > + if (bo->base.base.import_attach) > + gem_state_flags |= PANFROST_DEBUGFS_GEM_STATE_FLAG_IMPORTED; > + if (bo->base.base.dma_buf) > + gem_state_flags |= PANFROST_DEBUGFS_GEM_STATE_FLAG_EXPORTED; > + > + if (bo->base.madv < 0) > + gem_state_flags |= PANFROST_DEBUGFS_GEM_STATE_FLAG_PURGED; > + else if (bo->base.madv > 0) > + gem_state_flags |= PANFROST_DEBUGFS_GEM_STATE_FLAG_PURGEABLE; > + > + seq_printf(m, "0x%-10x", gem_state_flags); > + > + scoped_guard(mutex, &bo->label.lock) { > + seq_printf(m, "%s\n", bo->label.str ? : ""); > + } > + > + totals->size += bo->base.base.size; > + totals->resident += resident_size; > + if (bo->base.madv > 0) > + totals->reclaimable += resident_size; > +} > + > +void panfrost_gem_debugfs_print_bos(struct panfrost_device *pfdev, > + struct seq_file *m) > +{ > + struct gem_size_totals totals = {0}; > + struct panfrost_gem_object *bo; > + > + panfrost_gem_debugfs_print_flag_names(m); > + > + seq_puts(m, "created-by global-name refcount size resident-size file-offset state label\n"); > + seq_puts(m, "-----------------------------------------------------------------------------------------------------------------------------------\n"); > + > + scoped_guard(mutex, &pfdev->debugfs.gems_lock) { > + list_for_each_entry(bo, &pfdev->debugfs.gems_list, debugfs.node) { > + panfrost_gem_debugfs_bo_print(bo, m, &totals); > + } > + } > + > + seq_puts(m, "===================================================================================================================================\n"); > + seq_printf(m, "Total size: %zd, Total resident: %zd, Total reclaimable: %zd\n", > + totals.size, totals.resident, totals.reclaimable); > +} > +#endif > diff --git a/drivers/gpu/drm/panfrost/panfrost_gem.h b/drivers/gpu/drm/panfrost/panfrost_gem.h > index 3d87c41ad09d..8de3e76f2717 100644 > --- a/drivers/gpu/drm/panfrost/panfrost_gem.h > +++ b/drivers/gpu/drm/panfrost/panfrost_gem.h > @@ -8,9 +8,47 @@ > #include <drm/drm_mm.h> > > struct panfrost_mmu; > +struct panfrost_device; > > #define PANFROST_BO_LABEL_MAXLEN 4096 > > +enum panfrost_debugfs_gem_state_flags { > + /** @PANFROST_DEBUGFS_GEM_STATE_FLAG_IMPORTED: GEM BO is PRIME imported. */ > + PANFROST_DEBUGFS_GEM_STATE_FLAG_IMPORTED = BIT(0), > + > + /** @PANFROST_DEBUGFS_GEM_STATE_FLAG_EXPORTED: GEM BO is PRIME exported. */ > + PANFROST_DEBUGFS_GEM_STATE_FLAG_EXPORTED = BIT(1), > + > + /** @PANFROST_DEBUGFS_GEM_STATE_FLAG_PURGED: GEM BO was reclaimed by the shrinker. */ > + PANFROST_DEBUGFS_GEM_STATE_FLAG_PURGED = BIT(2), > + > + /** > + * @PANFROST_DEBUGFS_GEM_STATE_FLAG_PURGEABLE: GEM BO pages were marked as no longer > + * needed by UM and can be reclaimed by the shrinker. > + */ > + PANFROST_DEBUGFS_GEM_STATE_FLAG_PURGEABLE = BIT(3), > +}; > + > +/** > + * struct panfrost_gem_debugfs - GEM object's DebugFS list information > + */ > +struct panfrost_gem_debugfs { > + /** > + * @node: Node used to insert the object in the device-wide list of > + * GEM objects, to display information about it through a DebugFS file. > + */ > + struct list_head node; > + > + /** @creator: Information about the UM process which created the GEM. */ > + struct { > + /** @creator.process_name: Group leader name in owning thread's process */ > + char process_name[TASK_COMM_LEN]; > + > + /** @creator.tgid: PID of the thread's group leader within its process */ > + pid_t tgid; > + } creator; > +}; > + > struct panfrost_gem_object { > struct drm_gem_shmem_object base; > struct sg_table *sgts; > @@ -59,6 +97,10 @@ struct panfrost_gem_object { > > bool noexec :1; > bool is_heap :1; > + > +#ifdef CONFIG_DEBUG_FS > + struct panfrost_gem_debugfs debugfs; > +#endif > }; > > struct panfrost_gem_mapping { > @@ -108,4 +150,9 @@ void panfrost_gem_shrinker_cleanup(struct drm_device *dev); > void panfrost_gem_set_label(struct drm_gem_object *obj, const char *label); > void panfrost_gem_internal_set_label(struct drm_gem_object *obj, const char *label); > > +#ifdef CONFIG_DEBUG_FS > +void panfrost_gem_debugfs_print_bos(struct panfrost_device *pfdev, > + struct seq_file *m); > +#endif > + > #endif /* __PANFROST_GEM_H__ */

1 year, 1 month

Jump to page:

2026

2025

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

2012

2011

Linaro-mm-sig