Linaro-mm-sig

linaro-mm-sig@lists.linaro.org

12 participants
3138 discussions

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

by Jens Wiklander

Hi Amir, On Fri, May 30, 2025 at 4:13 AM Amirreza Zarrabi <amirreza.zarrabi(a)oss.qualcomm.com> wrote: > > Hi Jens, > > On 5/21/2025 1:16 AM, 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 > > 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; > > +}; > > + > > Isn't using an xarray excessive for just three entries, given static, limited IDs? The interface is nice and scales well. Do you think it adds too much overhead? What should we use instead? > > > +#if IS_ENABLED(CONFIG_DMABUF_HEAPS) > > +static DEFINE_XARRAY_ALLOC(tee_dma_heap); > > + > > Why are we even considering sgl in the first place, given that tee_shm_register_fd() > does not accept any dma_buf with more than one entry? Wouldn't it make sense to > ensure sgl has a single entry after calling alloc()? I didn't want to close that door completely, even if we can currently only have a single entry. > > > +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"); > > +} > > + > > +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); > > + > > (Please first see comment below for update_shm()) > > Other than calling update_shm(), nothing significant happens in this function. > If we remove update_shm(), these operations can be moved to the register function. Yes, but at the expense of making that function more complicated. > > However, I also argue that since we have a well-designed generic function like > tee_ioctl_shm_register_fd(), why should we restrict ourselves to checking the type of > dma_buf? Any dma_buf should be acceptable to register as tee_shm (thougn it should fail > if it is used in protected operation). Otherwise, TEE_IOC_SHM_REGISTER_FD is > not an accurate name it should be TEE_IOC_SHM_REGISTER_PROTECTED_FD (or something similar). tee_shm_register_fd() can handle both DMA-bufs from a registered TEE heap and DMA-bufs allocated by other means. The backend driver may have further restrictions. The OP-TEE FF-A driver can only accept DMA-bufs allocated from a registered TEE heap since it needs the shared memory handle of the underlying pool to pass the buffer as an argument to the secure world. > > > +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); > > + *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; > > + > > Should be a loop? > > > + 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); > > + > > I understand your reasoning for this, but considering that paddr comes from > the backend, isn’t it the backend’s responsibility to ensure that pfn_valid() > passes? For example, it might want to call devm_memremap_pages() itself. > So, should we simply ensure that the pages are valid , and fail on return, > rather than attempting to fix them? You have a point. I'll update accordingly. > > > + /* > > + * 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); > > Why do we need update_shm()? Currently, it seems to do nothing beyond setting > parent_shm, simply indicating that this shm is part of a larger shm. > > What if the backend wants to handle the buffer using something other than > tee_shm - for example, if it doesn’t want to use tee_shm_alloc_dma_mem()? > Would removing update_shm() and replacing it with shm_release(), while also > getting rid of tee_heap_update_from_dma_buf(), be a more streamlined approach?" > > This way, tee_shm for dma_buf would be treated like any other tee_shm - using > refcounting based on tee_shm rather than its parent - and we would simply call > shm_release() upon release. > > With this change, we can even accept any dm_buf making the TEE_IOC_SHM_REGISTER_FD generic. The parent_shm is a must for the dynamic allocation of protected memory for the OP-TEE backend driver. Cheers, Jens > > Best Regards, > Amir > > > + 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. > > + */ > > +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(). >

5 months, 2 weeks

[PATCH 6.6 391/444] drm/gem: Internally test import_attach for imported objects

by Greg Kroah-Hartman

6.6-stable review patch. If anyone has any objections, please let me know. ------------------ From: Thomas Zimmermann <tzimmermann(a)suse.de> commit 8260731ccad0451207b45844bb66eb161a209218 upstream. Test struct drm_gem_object.import_attach to detect imported objects. During object clenanup, the dma_buf field might be NULL. Testing it in an object's free callback then incorrectly does a cleanup as for native objects. Happens for calls to drm_mode_destroy_dumb_ioctl() that clears the dma_buf field in drm_gem_object_exported_dma_buf_free(). v3: - only test for import_attach (Boris) v2: - use import_attach.dmabuf instead of dma_buf (Christian) Signed-off-by: Thomas Zimmermann <tzimmermann(a)suse.de> Fixes: b57aa47d39e9 ("drm/gem: Test for imported GEM buffers with helper") Reported-by: Andy Yan <andyshrk(a)163.com> Closes: https://lore.kernel.org/dri-devel/38d09d34.4354.196379aa560.Coremail.andysh… Tested-by: Andy Yan <andyshrk(a)163.com> Cc: Thomas Zimmermann <tzimmermann(a)suse.de> Cc: Anusha Srivatsa <asrivats(a)redhat.com> Cc: Christian König <christian.koenig(a)amd.com> Cc: Maarten Lankhorst <maarten.lankhorst(a)linux.intel.com> Cc: Maxime Ripard <mripard(a)kernel.org> Cc: David Airlie <airlied(a)gmail.com> Cc: Simona Vetter <simona(a)ffwll.ch> Cc: Sumit Semwal <sumit.semwal(a)linaro.org> Cc: "Christian König" <christian.koenig(a)amd.com> Cc: dri-devel(a)lists.freedesktop.org Cc: linux-media(a)vger.kernel.org Cc: linaro-mm-sig(a)lists.linaro.org Reviewed-by: Boris Brezillon <boris.brezillon(a)collabora.com> Reviewed-by: Simona Vetter <simona.vetter(a)ffwll.ch> Link: https://lore.kernel.org/r/20250416065820.26076-1-tzimmermann@suse.de Signed-off-by: Greg Kroah-Hartman <gregkh(a)linuxfoundation.org> --- include/drm/drm_gem.h | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) --- a/include/drm/drm_gem.h +++ b/include/drm/drm_gem.h @@ -567,8 +567,7 @@ static inline bool drm_gem_object_is_sha */ static inline bool drm_gem_is_imported(const struct drm_gem_object *obj) { - /* The dma-buf's priv field points to the original GEM object. */ - return obj->dma_buf && (obj->dma_buf->priv != obj); + return !!obj->import_attach; } #ifdef CONFIG_LOCKDEP

5 months, 2 weeks

Re: [RFC PATCH 00/30] Host side (KVM/VFIO/IOMMUFD) support for TDISP using TSM

by Jason Gunthorpe

On Thu, May 29, 2025 at 01:34:43PM +0800, Xu Yilun wrote: > This series has 3 sections: I really think this is too big to try to progress, even in RFC form. > Patch 1 - 11 deal with the private MMIO mapping in KVM MMU via DMABUF. > Leverage Jason & Vivek's latest VFIO dmabuf series [3], see Patch 2 - 4. > The concern for get_pfn() kAPI [4] is not addressed so are marked as > HACK, will investigate later. I would probably split this out entirely into its own topic. It doesn't seem directly related to TSM as KVM can use DMABUF for good reasons independently . > Patch 12 - 22 is about TSM Bind/Unbind/Guest request management in VFIO > & IOMMUFD. Picks some of Shameer's patch in [5], see Patch 12 & 14. This is some reasonable topic on its own after Dan's series > Patch 23 - 30 is a solution to meet the TDX specific sequence > enforcement on various device Unbind cases, including converting device > back to shared, hot unplug, TD destroy. Start with a tdx_tsm driver > prototype and finally implement the Unbind enforcement inside the > driver. To be honest it is still awkward to me, but I need help. Then you have a series or two to implement TDX using the infrastructure. Jason

5 months, 2 weeks

Re: [RFC PATCH 10/30] vfio/pci: Export vfio dma-buf specific info for importers

by Jason Gunthorpe

On Thu, May 29, 2025 at 01:34:53PM +0800, Xu Yilun wrote: > Export vfio dma-buf specific info by attaching vfio_dma_buf_data in > struct dma_buf::priv. Provide a helper vfio_dma_buf_get_data() for > importers to fetch these data. Exporters identify VFIO dma-buf by > successfully getting these data. > > VFIO dma-buf supports disabling host access to these exported MMIO > regions when the device is converted to private. Exporters like KVM > need to identify this type of dma-buf to decide if it is good to use. > KVM only allows host unaccessible MMIO regions been mapped in private > roots. > > Export struct kvm * handler attached to the vfio device. This > allows KVM to do another sanity check. MMIO should only be assigned to > a CoCo VM if its owner device is already assigned to the same VM. This doesn't seem right, it should be encapsulated into the standard DMABUF API in some way. Jason

5 months, 2 weeks

Re: [PATCH v3 3/4] udmabuf: Implement udmabuf rw_file callback

by kernel test robot

Hi wangtao, kernel test robot noticed the following build errors: [auto build test ERROR on brauner-vfs/vfs.all] [also build test ERROR on next-20250530] [cannot apply to linus/master v6.15] [If your patch is applied to the wrong git tree, kindly drop us a note. And when submitting patch, we suggest to use '--base' as documented in https://git-scm.com/docs/git-format-patch#_base_tree_information] url: https://github.com/intel-lab-lkp/linux/commits/wangtao/fs-allow-cross-FS-co… base: https://git.kernel.org/pub/scm/linux/kernel/git/vfs/vfs.git vfs.all patch link: https://lore.kernel.org/r/20250530103941.11092-4-tao.wangtao%40honor.com patch subject: [PATCH v3 3/4] udmabuf: Implement udmabuf rw_file callback config: sparc64-randconfig-002-20250530 (https://download.01.org/0day-ci/archive/20250530/202505302235.mDzENMSm-lkp@…) compiler: sparc64-linux-gcc (GCC) 15.1.0 reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20250530/202505302235.mDzENMSm-lkp@…) If you fix the issue in a separate patch/commit (i.e. not just a new version of the same patch/commit), kindly add following tags | Reported-by: kernel test robot <lkp(a)intel.com> | Closes: https://lore.kernel.org/oe-kbuild-all/202505302235.mDzENMSm-lkp@intel.com/ All error/warnings (new ones prefixed by >>): drivers/dma-buf/udmabuf.c: In function 'udmabuf_rw_file': >> drivers/dma-buf/udmabuf.c:298:25: error: storage size of 'iter' isn't known 298 | struct iov_iter iter; | ^~~~ >> drivers/dma-buf/udmabuf.c:299:45: error: 'ITER_SOURCE' undeclared (first use in this function) 299 | unsigned int direction = is_write ? ITER_SOURCE : ITER_DEST; | ^~~~~~~~~~~ drivers/dma-buf/udmabuf.c:299:45: note: each undeclared identifier is reported only once for each function it appears in >> drivers/dma-buf/udmabuf.c:299:59: error: 'ITER_DEST' undeclared (first use in this function) 299 | unsigned int direction = is_write ? ITER_SOURCE : ITER_DEST; | ^~~~~~~~~ >> drivers/dma-buf/udmabuf.c:327:17: error: implicit declaration of function 'iov_iter_bvec'; did you mean 'bvec_iter_bvec'? [-Wimplicit-function-declaration] 327 | iov_iter_bvec(&iter, direction, bvec, bv_idx, bv_total); | ^~~~~~~~~~~~~ | bvec_iter_bvec >> drivers/dma-buf/udmabuf.c:298:25: warning: unused variable 'iter' [-Wunused-variable] 298 | struct iov_iter iter; | ^~~~ vim +298 drivers/dma-buf/udmabuf.c 286 287 static ssize_t udmabuf_rw_file(struct dma_buf *dmabuf, loff_t my_pos, 288 struct file *other, loff_t pos, 289 size_t count, bool is_write) 290 { 291 struct udmabuf *ubuf = dmabuf->priv; 292 loff_t my_end = my_pos + count, bv_beg, bv_end = 0; 293 pgoff_t pg_idx = my_pos / PAGE_SIZE; 294 pgoff_t pg_end = DIV_ROUND_UP(my_end, PAGE_SIZE); 295 size_t i, bv_off, bv_len, bv_num, bv_idx = 0, bv_total = 0; 296 struct bio_vec *bvec; 297 struct kiocb kiocb; > 298 struct iov_iter iter; > 299 unsigned int direction = is_write ? ITER_SOURCE : ITER_DEST; 300 ssize_t ret = 0, rw_total = 0; 301 struct folio *folio; 302 303 bv_num = min_t(size_t, pg_end - pg_idx + 1, 1024); 304 bvec = kvcalloc(bv_num, sizeof(*bvec), GFP_KERNEL); 305 if (!bvec) 306 return -ENOMEM; 307 308 init_sync_kiocb(&kiocb, other); 309 kiocb.ki_pos = pos; 310 311 for (i = 0; i < ubuf->nr_pinned && my_pos < my_end; i++) { 312 folio = ubuf->pinned_folios[i]; 313 bv_beg = bv_end; 314 bv_end += folio_size(folio); 315 if (bv_end <= my_pos) 316 continue; 317 318 bv_len = min(bv_end, my_end) - my_pos; 319 bv_off = my_pos - bv_beg; 320 my_pos += bv_len; 321 bv_total += bv_len; 322 bvec_set_page(&bvec[bv_idx], &folio->page, bv_len, bv_off); 323 if (++bv_idx < bv_num && my_pos < my_end) 324 continue; 325 326 /* start R/W if bvec is full or count reaches zero. */ > 327 iov_iter_bvec(&iter, direction, bvec, bv_idx, bv_total); 328 if (is_write) 329 ret = other->f_op->write_iter(&kiocb, &iter); 330 else 331 ret = other->f_op->read_iter(&kiocb, &iter); 332 if (ret <= 0) 333 break; 334 rw_total += ret; 335 if (ret < bv_total || fatal_signal_pending(current)) 336 break; 337 338 bv_idx = bv_total = 0; 339 } 340 kvfree(bvec); 341 342 return rw_total > 0 ? rw_total : ret; 343 } 344 -- 0-DAY CI Kernel Test Service https://github.com/intel/lkp-tests/wiki

5 months, 2 weeks

Re: [RFC PATCH 00/12] Private MMIO support for private assigned dev

by Jason Gunthorpe

On Thu, May 29, 2025 at 10:41:15PM +0800, Xu Yilun wrote: > > On AMD, the host can "revoke" at any time, at worst it'll see RMP > > events from IOMMU. Thanks, > > Is the RMP event firstly detected by host or guest? If by host, > host could fool guest by just suppress the event. Guest thought the > DMA writting is successful but it is not and may cause security issue. Is that in scope of the threat model though? Host must not be able to change DMAs or target them to different memory, but the host can stop DMA and loose it, surely? Host controls the PCI memory enable bit, doesn't it? Jason

5 months, 2 weeks

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

by Rob Herring

On Tue, May 20, 2025 at 5:27 AM Tomeu Vizoso <tomeu(a)tomeuvizoso.net> 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) > > v4: > - Change required node name to npu@ (Rob Herring and Krzysztof Kozlowski) > - Remove unneeded items: (Krzysztof Kozlowski) > - Fix use of minItems/maxItems (Krzysztof Kozlowski) > - Add reg-names to list of required properties (Krzysztof Kozlowski) > - Fix example (Krzysztof Kozlowski) > > v5: > - Rename file to rockchip,rk3588-rknn-core.yaml (Krzysztof Kozlowski) > - Streamline compatible property (Krzysztof Kozlowski) > > Signed-off-by: Sebastian Reichel <sebastian.reichel(a)collabora.com> > Signed-off-by: Tomeu Vizoso <tomeu(a)tomeuvizoso.net> > --- > .../bindings/npu/rockchip,rk3588-rknn-core.yaml | 147 +++++++++++++++++++++ > 1 file changed, 147 insertions(+) > > diff --git a/Documentation/devicetree/bindings/npu/rockchip,rk3588-rknn-core.yaml b/Documentation/devicetree/bindings/npu/rockchip,rk3588-rknn-core.yaml > new file mode 100644 > index 0000000000000000000000000000000000000000..9eb426367afcbc03c387d43c4b8250cdd1b9ee86 > --- /dev/null > +++ b/Documentation/devicetree/bindings/npu/rockchip,rk3588-rknn-core.yaml > @@ -0,0 +1,147 @@ > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) > +%YAML 1.2 > +--- > +$id: http://devicetree.org/schemas/npu/rockchip,rk3588-rknn-core.yaml# > +$schema: http://devicetree.org/meta-schemas/core.yaml# > + > +title: Neural Processing Unit IP from Rockchip > + > +maintainers: > + - Tomeu Vizoso <tomeu(a)tomeuvizoso.net> > + > +description: > + Rockchip IP for accelerating inference of neural networks, based on NVIDIA's > + open source NVDLA IP. > + > + There is to be a node per each core in the NPU. In Rockchip's design there > + will be one core that is special and needs to be powered on before any of the > + other cores can be used. This special core is called the top core and should > + have the compatible string that corresponds to top cores. Is this really a distinction in the h/w? If you change which core is the top one in the DT, does it still work? > + > +properties: > + $nodename: > + pattern: '^npu@[a-f0-9]+$' > + > + compatible: > + enum: > + - rockchip,rk3588-rknn-core-top > + - rockchip,rk3588-rknn-core > + > + reg: > + maxItems: 3 > + > + reg-names: > + items: > + - const: pc > + - const: cna > + - const: core > + > + clocks: > + minItems: 2 > + maxItems: 4 > + > + clock-names: > + items: > + - const: aclk > + - const: hclk > + - const: npu > + - const: pclk > + minItems: 2 It is odd that the non-top cores only have bus clocks and no module clock. But based on the clock names, I'm guessing the aclk/hclk are not shared, but the npu and pclk are shared. Since you make the top core probe first, then it will enable the shared clocks and the non-top cores don't have to worry about them. If so, that is wrong as it is letting the software design define the bindings. Rob

5 months, 2 weeks

Re: [PATCH v5 09/12] tee: add Qualcomm TEE driver

by Dan Carpenter

Hi Amirreza, kernel test robot noticed the following build warnings: url: https://github.com/intel-lab-lkp/linux/commits/Amirreza-Zarrabi/tee-allow-a… base: 3be1a7a31fbda82f3604b6c31e4f390110de1b46 patch link: https://lore.kernel.org/r/20250526-qcom-tee-using-tee-ss-without-mem-obj-v5… patch subject: [PATCH v5 09/12] tee: add Qualcomm TEE driver config: x86_64-randconfig-161-20250528 (https://download.01.org/0day-ci/archive/20250528/202505280653.Y79JKqDd-lkp@…) compiler: gcc-12 (Debian 12.2.0-14) 12.2.0 If you fix the issue in a separate patch/commit (i.e. not just a new version of the same patch/commit), kindly add following tags | Reported-by: kernel test robot <lkp(a)intel.com> | Reported-by: Dan Carpenter <dan.carpenter(a)linaro.org> | Closes: https://lore.kernel.org/r/202505280653.Y79JKqDd-lkp@intel.com/ smatch warnings: drivers/tee/qcomtee/call.c:748 qcomtee_probe() warn: missing error code 'err' vim +/err +748 drivers/tee/qcomtee/call.c accd33ce59c3367 Amirreza Zarrabi 2025-05-26 711 static int qcomtee_probe(struct platform_device *pdev) accd33ce59c3367 Amirreza Zarrabi 2025-05-26 712 { accd33ce59c3367 Amirreza Zarrabi 2025-05-26 713 struct workqueue_struct *async_wq; accd33ce59c3367 Amirreza Zarrabi 2025-05-26 714 struct tee_device *teedev; accd33ce59c3367 Amirreza Zarrabi 2025-05-26 715 struct tee_shm_pool *pool; accd33ce59c3367 Amirreza Zarrabi 2025-05-26 716 struct tee_context *ctx; accd33ce59c3367 Amirreza Zarrabi 2025-05-26 717 struct qcomtee *qcomtee; accd33ce59c3367 Amirreza Zarrabi 2025-05-26 718 int err; accd33ce59c3367 Amirreza Zarrabi 2025-05-26 719 accd33ce59c3367 Amirreza Zarrabi 2025-05-26 720 qcomtee = kzalloc(sizeof(*qcomtee), GFP_KERNEL); accd33ce59c3367 Amirreza Zarrabi 2025-05-26 721 if (!qcomtee) accd33ce59c3367 Amirreza Zarrabi 2025-05-26 722 return -ENOMEM; accd33ce59c3367 Amirreza Zarrabi 2025-05-26 723 accd33ce59c3367 Amirreza Zarrabi 2025-05-26 724 pool = qcomtee_shm_pool_alloc(); accd33ce59c3367 Amirreza Zarrabi 2025-05-26 725 if (IS_ERR(pool)) { accd33ce59c3367 Amirreza Zarrabi 2025-05-26 726 err = PTR_ERR(pool); accd33ce59c3367 Amirreza Zarrabi 2025-05-26 727 accd33ce59c3367 Amirreza Zarrabi 2025-05-26 728 goto err_free_qcomtee; accd33ce59c3367 Amirreza Zarrabi 2025-05-26 729 } accd33ce59c3367 Amirreza Zarrabi 2025-05-26 730 accd33ce59c3367 Amirreza Zarrabi 2025-05-26 731 teedev = tee_device_alloc(&qcomtee_desc, NULL, pool, qcomtee); accd33ce59c3367 Amirreza Zarrabi 2025-05-26 732 if (IS_ERR(teedev)) { accd33ce59c3367 Amirreza Zarrabi 2025-05-26 733 err = PTR_ERR(teedev); accd33ce59c3367 Amirreza Zarrabi 2025-05-26 734 accd33ce59c3367 Amirreza Zarrabi 2025-05-26 735 goto err_pool_destroy; accd33ce59c3367 Amirreza Zarrabi 2025-05-26 736 } accd33ce59c3367 Amirreza Zarrabi 2025-05-26 737 accd33ce59c3367 Amirreza Zarrabi 2025-05-26 738 qcomtee->teedev = teedev; accd33ce59c3367 Amirreza Zarrabi 2025-05-26 739 qcomtee->pool = pool; accd33ce59c3367 Amirreza Zarrabi 2025-05-26 740 err = tee_device_register(qcomtee->teedev); accd33ce59c3367 Amirreza Zarrabi 2025-05-26 741 if (err) accd33ce59c3367 Amirreza Zarrabi 2025-05-26 742 goto err_unreg_teedev; accd33ce59c3367 Amirreza Zarrabi 2025-05-26 743 accd33ce59c3367 Amirreza Zarrabi 2025-05-26 744 platform_set_drvdata(pdev, qcomtee); accd33ce59c3367 Amirreza Zarrabi 2025-05-26 745 /* Start async wq. */ accd33ce59c3367 Amirreza Zarrabi 2025-05-26 746 async_wq = alloc_ordered_workqueue("qcomtee_wq", 0); accd33ce59c3367 Amirreza Zarrabi 2025-05-26 747 if (!async_wq) accd33ce59c3367 Amirreza Zarrabi 2025-05-26 @748 goto err_unreg_teedev; err = -ENOMEM; accd33ce59c3367 Amirreza Zarrabi 2025-05-26 749 accd33ce59c3367 Amirreza Zarrabi 2025-05-26 750 qcomtee->wq = async_wq; accd33ce59c3367 Amirreza Zarrabi 2025-05-26 751 /* Driver context used for async operations of teedev. */ accd33ce59c3367 Amirreza Zarrabi 2025-05-26 752 ctx = teedev_open(qcomtee->teedev); accd33ce59c3367 Amirreza Zarrabi 2025-05-26 753 if (IS_ERR(ctx)) { accd33ce59c3367 Amirreza Zarrabi 2025-05-26 754 err = PTR_ERR(ctx); accd33ce59c3367 Amirreza Zarrabi 2025-05-26 755 accd33ce59c3367 Amirreza Zarrabi 2025-05-26 756 goto err_dest_wq; accd33ce59c3367 Amirreza Zarrabi 2025-05-26 757 } accd33ce59c3367 Amirreza Zarrabi 2025-05-26 758 accd33ce59c3367 Amirreza Zarrabi 2025-05-26 759 qcomtee->ctx = ctx; accd33ce59c3367 Amirreza Zarrabi 2025-05-26 760 /* Init Object table. */ accd33ce59c3367 Amirreza Zarrabi 2025-05-26 761 qcomtee->xa_last_id = 0; accd33ce59c3367 Amirreza Zarrabi 2025-05-26 762 xa_init_flags(&qcomtee->xa_local_objects, XA_FLAGS_ALLOC); accd33ce59c3367 Amirreza Zarrabi 2025-05-26 763 /* Get QTEE verion. */ accd33ce59c3367 Amirreza Zarrabi 2025-05-26 764 qcomtee_get_qtee_feature_list(qcomtee->ctx, accd33ce59c3367 Amirreza Zarrabi 2025-05-26 765 QCOMTEE_FEATURE_VER_OP_GET_QTEE_ID, accd33ce59c3367 Amirreza Zarrabi 2025-05-26 766 &qtee_version); accd33ce59c3367 Amirreza Zarrabi 2025-05-26 767 accd33ce59c3367 Amirreza Zarrabi 2025-05-26 768 pr_info("QTEE version %u.%u.%u\n", accd33ce59c3367 Amirreza Zarrabi 2025-05-26 769 QTEE_VERSION_GET_MAJOR(qtee_version), accd33ce59c3367 Amirreza Zarrabi 2025-05-26 770 QTEE_VERSION_GET_MINOR(qtee_version), accd33ce59c3367 Amirreza Zarrabi 2025-05-26 771 QTEE_VERSION_GET_PATCH(qtee_version)); accd33ce59c3367 Amirreza Zarrabi 2025-05-26 772 accd33ce59c3367 Amirreza Zarrabi 2025-05-26 773 return 0; accd33ce59c3367 Amirreza Zarrabi 2025-05-26 774 accd33ce59c3367 Amirreza Zarrabi 2025-05-26 775 err_dest_wq: accd33ce59c3367 Amirreza Zarrabi 2025-05-26 776 destroy_workqueue(qcomtee->wq); accd33ce59c3367 Amirreza Zarrabi 2025-05-26 777 err_unreg_teedev: accd33ce59c3367 Amirreza Zarrabi 2025-05-26 778 tee_device_unregister(qcomtee->teedev); accd33ce59c3367 Amirreza Zarrabi 2025-05-26 779 err_pool_destroy: accd33ce59c3367 Amirreza Zarrabi 2025-05-26 780 tee_shm_pool_free(pool); accd33ce59c3367 Amirreza Zarrabi 2025-05-26 781 err_free_qcomtee: accd33ce59c3367 Amirreza Zarrabi 2025-05-26 782 kfree(qcomtee); accd33ce59c3367 Amirreza Zarrabi 2025-05-26 783 accd33ce59c3367 Amirreza Zarrabi 2025-05-26 784 return err; accd33ce59c3367 Amirreza Zarrabi 2025-05-26 785 } -- 0-DAY CI Kernel Test Service https://github.com/intel/lkp-tests/wiki

5 months, 2 weeks

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

by kernel test robot

Hi Amirreza, kernel test robot noticed the following build warnings: [auto build test WARNING on 3be1a7a31fbda82f3604b6c31e4f390110de1b46] url: https://github.com/intel-lab-lkp/linux/commits/Amirreza-Zarrabi/tee-allow-a… base: 3be1a7a31fbda82f3604b6c31e4f390110de1b46 patch link: https://lore.kernel.org/r/20250526-qcom-tee-using-tee-ss-without-mem-obj-v5… patch subject: [PATCH v5 03/12] tee: add TEE_IOCTL_PARAM_ATTR_TYPE_UBUF config: arm64-randconfig-r121-20250527 (https://download.01.org/0day-ci/archive/20250528/202505280721.abBn0GaE-lkp@…) compiler: aarch64-linux-gcc (GCC) 8.5.0 reproduce: (https://download.01.org/0day-ci/archive/20250528/202505280721.abBn0GaE-lkp@…) If you fix the issue in a separate patch/commit (i.e. not just a new version of the same patch/commit), kindly add following tags | Reported-by: kernel test robot <lkp(a)intel.com> | Closes: https://lore.kernel.org/oe-kbuild-all/202505280721.abBn0GaE-lkp@intel.com/ sparse warnings: (new ones prefixed by >>) drivers/tee/tee_core.c:393:48: sparse: sparse: incorrect type in assignment (different address spaces) @@ expected void *[noderef] uaddr @@ got void [noderef] __user * @@ drivers/tee/tee_core.c:393:48: sparse: expected void *[noderef] uaddr drivers/tee/tee_core.c:393:48: sparse: got void [noderef] __user * >> drivers/tee/tee_core.c:396:56: sparse: sparse: incorrect type in argument 1 (different address spaces) @@ expected void const [noderef] __user *addr @@ got void *[noderef] uaddr @@ drivers/tee/tee_core.c:396:56: sparse: expected void const [noderef] __user *addr drivers/tee/tee_core.c:396:56: sparse: got void *[noderef] uaddr drivers/tee/tee_core.c:785:41: sparse: sparse: incorrect type in assignment (different address spaces) @@ expected void *[noderef] uaddr @@ got void [noderef] __user * @@ drivers/tee/tee_core.c:785:41: sparse: expected void *[noderef] uaddr drivers/tee/tee_core.c:785:41: sparse: got void [noderef] __user * drivers/tee/tee_core.c:788:56: sparse: sparse: incorrect type in argument 1 (different address spaces) @@ expected void const [noderef] __user *addr @@ got void *[noderef] uaddr @@ drivers/tee/tee_core.c:788:56: sparse: expected void const [noderef] __user *addr drivers/tee/tee_core.c:788:56: sparse: got void *[noderef] uaddr drivers/tee/tee_core.c:396:46: sparse: sparse: dereference of noderef expression drivers/tee/tee_core.c:396:46: sparse: sparse: dereference of noderef expression drivers/tee/tee_core.c:677:37: sparse: sparse: dereference of noderef expression drivers/tee/tee_core.c:788:46: sparse: sparse: dereference of noderef expression drivers/tee/tee_core.c:788:46: sparse: sparse: dereference of noderef expression vim +396 drivers/tee/tee_core.c 361 362 static int params_from_user(struct tee_context *ctx, struct tee_param *params, 363 size_t num_params, 364 struct tee_ioctl_param __user *uparams) 365 { 366 size_t n; 367 368 for (n = 0; n < num_params; n++) { 369 struct tee_shm *shm; 370 struct tee_ioctl_param ip; 371 372 if (copy_from_user(&ip, uparams + n, sizeof(ip))) 373 return -EFAULT; 374 375 /* All unused attribute bits has to be zero */ 376 if (ip.attr & ~TEE_IOCTL_PARAM_ATTR_MASK) 377 return -EINVAL; 378 379 params[n].attr = ip.attr; 380 switch (ip.attr & TEE_IOCTL_PARAM_ATTR_TYPE_MASK) { 381 case TEE_IOCTL_PARAM_ATTR_TYPE_NONE: 382 case TEE_IOCTL_PARAM_ATTR_TYPE_VALUE_OUTPUT: 383 break; 384 case TEE_IOCTL_PARAM_ATTR_TYPE_VALUE_INPUT: 385 case TEE_IOCTL_PARAM_ATTR_TYPE_VALUE_INOUT: 386 params[n].u.value.a = ip.a; 387 params[n].u.value.b = ip.b; 388 params[n].u.value.c = ip.c; 389 break; 390 case TEE_IOCTL_PARAM_ATTR_TYPE_UBUF_INPUT: 391 case TEE_IOCTL_PARAM_ATTR_TYPE_UBUF_OUTPUT: 392 case TEE_IOCTL_PARAM_ATTR_TYPE_UBUF_INOUT: 393 params[n].u.ubuf.uaddr = u64_to_user_ptr(ip.a); 394 params[n].u.ubuf.size = ip.b; 395 > 396 if (!access_ok(params[n].u.ubuf.uaddr, 397 params[n].u.ubuf.size)) 398 return -EFAULT; 399 400 break; 401 case TEE_IOCTL_PARAM_ATTR_TYPE_MEMREF_INPUT: 402 case TEE_IOCTL_PARAM_ATTR_TYPE_MEMREF_OUTPUT: 403 case TEE_IOCTL_PARAM_ATTR_TYPE_MEMREF_INOUT: 404 /* 405 * If a NULL pointer is passed to a TA in the TEE, 406 * the ip.c IOCTL parameters is set to TEE_MEMREF_NULL 407 * indicating a NULL memory reference. 408 */ 409 if (ip.c != TEE_MEMREF_NULL) { 410 /* 411 * If we fail to get a pointer to a shared 412 * memory object (and increase the ref count) 413 * from an identifier we return an error. All 414 * pointers that has been added in params have 415 * an increased ref count. It's the callers 416 * responibility to do tee_shm_put() on all 417 * resolved pointers. 418 */ 419 shm = tee_shm_get_from_id(ctx, ip.c); 420 if (IS_ERR(shm)) 421 return PTR_ERR(shm); 422 423 /* 424 * Ensure offset + size does not overflow 425 * offset and does not overflow the size of 426 * the referred shared memory object. 427 */ 428 if ((ip.a + ip.b) < ip.a || 429 (ip.a + ip.b) > shm->size) { 430 tee_shm_put(shm); 431 return -EINVAL; 432 } 433 } else if (ctx->cap_memref_null) { 434 /* Pass NULL pointer to OP-TEE */ 435 shm = NULL; 436 } else { 437 return -EINVAL; 438 } 439 440 params[n].u.memref.shm_offs = ip.a; 441 params[n].u.memref.size = ip.b; 442 params[n].u.memref.shm = shm; 443 break; 444 default: 445 /* Unknown attribute */ 446 return -EINVAL; 447 } 448 } 449 return 0; 450 } 451 -- 0-DAY CI Kernel Test Service https://github.com/intel/lkp-tests/wiki

5 months, 2 weeks

Re: [PATCH v5 09/12] tee: add Qualcomm TEE driver

by kernel test robot

5 months, 2 weeks

Jump to page:

2025

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

2012

2011

Linaro-mm-sig