Linaro-mm-sig

linaro-mm-sig@lists.linaro.org

15 participants
3025 discussions

[PATCH AUTOSEL 6.1 02/17] udmabuf: fix a buf size overflow issue during udmabuf creation

by Sasha Levin

From: Xiaogang Chen <xiaogang.chen(a)amd.com> [ Upstream commit 021ba7f1babd029e714d13a6bf2571b08af96d0f ] by casting size_limit_mb to u64 when calculate pglimit. Signed-off-by: Xiaogang Chen<Xiaogang.Chen(a)amd.com> Link: https://patchwork.freedesktop.org/patch/msgid/20250321164126.329638-1-xiaog… Signed-off-by: Christian König <christian.koenig(a)amd.com> Signed-off-by: Sasha Levin <sashal(a)kernel.org> --- drivers/dma-buf/udmabuf.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/dma-buf/udmabuf.c b/drivers/dma-buf/udmabuf.c index ef99174d81ced..546bba502fbc1 100644 --- a/drivers/dma-buf/udmabuf.c +++ b/drivers/dma-buf/udmabuf.c @@ -186,7 +186,7 @@ static long udmabuf_create(struct miscdevice *device, if (!ubuf) return -ENOMEM; - pglimit = (size_limit_mb * 1024 * 1024) >> PAGE_SHIFT; + pglimit = ((u64)size_limit_mb * 1024 * 1024) >> PAGE_SHIFT; for (i = 0; i < head->count; i++) { if (!IS_ALIGNED(list[i].offset, PAGE_SIZE)) goto err; -- 2.39.5

5 months

[PATCH AUTOSEL 6.6 03/24] udmabuf: fix a buf size overflow issue during udmabuf creation

by Sasha Levin

From: Xiaogang Chen <xiaogang.chen(a)amd.com> [ Upstream commit 021ba7f1babd029e714d13a6bf2571b08af96d0f ] by casting size_limit_mb to u64 when calculate pglimit. Signed-off-by: Xiaogang Chen<Xiaogang.Chen(a)amd.com> Link: https://patchwork.freedesktop.org/patch/msgid/20250321164126.329638-1-xiaog… Signed-off-by: Christian König <christian.koenig(a)amd.com> Signed-off-by: Sasha Levin <sashal(a)kernel.org> --- drivers/dma-buf/udmabuf.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/dma-buf/udmabuf.c b/drivers/dma-buf/udmabuf.c index d1fcdd1f9aaed..373282beeb606 100644 --- a/drivers/dma-buf/udmabuf.c +++ b/drivers/dma-buf/udmabuf.c @@ -214,7 +214,7 @@ static long udmabuf_create(struct miscdevice *device, if (!ubuf) return -ENOMEM; - pglimit = (size_limit_mb * 1024 * 1024) >> PAGE_SHIFT; + pglimit = ((u64)size_limit_mb * 1024 * 1024) >> PAGE_SHIFT; for (i = 0; i < head->count; i++) { if (!IS_ALIGNED(list[i].offset, PAGE_SIZE)) goto err; -- 2.39.5

5 months

[PATCH AUTOSEL 6.13 03/34] udmabuf: fix a buf size overflow issue during udmabuf creation

by Sasha Levin

From: Xiaogang Chen <xiaogang.chen(a)amd.com> [ Upstream commit 021ba7f1babd029e714d13a6bf2571b08af96d0f ] by casting size_limit_mb to u64 when calculate pglimit. Signed-off-by: Xiaogang Chen<Xiaogang.Chen(a)amd.com> Link: https://patchwork.freedesktop.org/patch/msgid/20250321164126.329638-1-xiaog… Signed-off-by: Christian König <christian.koenig(a)amd.com> Signed-off-by: Sasha Levin <sashal(a)kernel.org> --- drivers/dma-buf/udmabuf.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/dma-buf/udmabuf.c b/drivers/dma-buf/udmabuf.c index cc7398cc17d67..e74e36a8ecda2 100644 --- a/drivers/dma-buf/udmabuf.c +++ b/drivers/dma-buf/udmabuf.c @@ -393,7 +393,7 @@ static long udmabuf_create(struct miscdevice *device, if (!ubuf) return -ENOMEM; - pglimit = (size_limit_mb * 1024 * 1024) >> PAGE_SHIFT; + pglimit = ((u64)size_limit_mb * 1024 * 1024) >> PAGE_SHIFT; for (i = 0; i < head->count; i++) { pgoff_t subpgcnt; -- 2.39.5

5 months

[PATCH AUTOSEL 6.14 03/34] udmabuf: fix a buf size overflow issue during udmabuf creation

by Sasha Levin

From: Xiaogang Chen <xiaogang.chen(a)amd.com> [ Upstream commit 021ba7f1babd029e714d13a6bf2571b08af96d0f ] by casting size_limit_mb to u64 when calculate pglimit. Signed-off-by: Xiaogang Chen<Xiaogang.Chen(a)amd.com> Link: https://patchwork.freedesktop.org/patch/msgid/20250321164126.329638-1-xiaog… Signed-off-by: Christian König <christian.koenig(a)amd.com> Signed-off-by: Sasha Levin <sashal(a)kernel.org> --- drivers/dma-buf/udmabuf.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/dma-buf/udmabuf.c b/drivers/dma-buf/udmabuf.c index cc7398cc17d67..e74e36a8ecda2 100644 --- a/drivers/dma-buf/udmabuf.c +++ b/drivers/dma-buf/udmabuf.c @@ -393,7 +393,7 @@ static long udmabuf_create(struct miscdevice *device, if (!ubuf) return -ENOMEM; - pglimit = (size_limit_mb * 1024 * 1024) >> PAGE_SHIFT; + pglimit = ((u64)size_limit_mb * 1024 * 1024) >> PAGE_SHIFT; for (i = 0; i < head->count; i++) { pgoff_t subpgcnt; -- 2.39.5

5 months

Re: [PATCH 0/3] uio/dma-buf: Give UIO users access to DMA addresses.

by Christoph Hellwig

On Mon, Apr 14, 2025 at 10:24:55AM +0200, Thomas Petazzoni wrote: > What this patch series is about is to add new user-space interface to > extend the existing UIO subsystem. Which as I explained to you is fundamentally broken and unsafe. If you need to do DMA from userspae you need to use vfio/iommufd. > I am not sure how this can work in our use-case. We have a very simple > set of IP blocks implemented in a FPGA, some of those IP blocks are > able to perform DMA operations. The register of those IP blocks are > mapped into a user-space application using the existing, accepted > upstream, UIO subsystem. Some of those registers allow to program DMA > transfers. So far, we can do all what we need, except program those DMA > transfers. Lots of people are having the same issue, and zillions of > ugly out-of-tree solutions flourish all over, and we're trying to see > if we can constructively find a solution that would be acceptable > upstream to resolve this use-case. Our platform is an old PowerPC with > no IOMMU. Then your driver design can't work and you need to replace it with a proper in-kernel driver.

5 months

Re: [PATCH v7 3/4] drm/panthor: Label all kernel BO's

by Steven Price

On 11/04/2025 16:03, Adrián Larumbe wrote: > Kernel BO's aren't exposed to UM, so labelling them is the responsibility > of the driver itself. This kind of tagging will prove useful in further > commits when want to expose these objects through DebugFS. > > Expand panthor_kernel_bo_create() interface to take a NULL-terminated NIT: s/NULL/NUL/ > string. No bounds checking is done because all label strings are given > as statically-allocated literals, but if a more complex kernel BO naming > scheme with explicit memory allocation and formatting was desired in the > future, this would have to change. I'm not sure I follow why bounds-checking would be an issue for strings from the kernel. But as you state these are all literals so definitely not a problem. > > Signed-off-by: Adrián Larumbe <adrian.larumbe(a)collabora.com> > Reviewed-by: Boris Brezillon <boris.brezillon(a)collabora.com> > Reviewed-by: Liviu Dudau <liviu.dudau(a)arm.com> Reviewed-by: Steven Price <steven.price(a)arm.com> > --- > drivers/gpu/drm/panthor/panthor_fw.c | 8 +++++--- > drivers/gpu/drm/panthor/panthor_gem.c | 4 +++- > drivers/gpu/drm/panthor/panthor_gem.h | 2 +- > drivers/gpu/drm/panthor/panthor_heap.c | 6 ++++-- > drivers/gpu/drm/panthor/panthor_sched.c | 9 ++++++--- > 5 files changed, 19 insertions(+), 10 deletions(-) > > diff --git a/drivers/gpu/drm/panthor/panthor_fw.c b/drivers/gpu/drm/panthor/panthor_fw.c > index 0f52766a3120..a7fdc4d8020d 100644 > --- a/drivers/gpu/drm/panthor/panthor_fw.c > +++ b/drivers/gpu/drm/panthor/panthor_fw.c > @@ -449,7 +449,8 @@ panthor_fw_alloc_queue_iface_mem(struct panthor_device *ptdev, > DRM_PANTHOR_BO_NO_MMAP, > DRM_PANTHOR_VM_BIND_OP_MAP_NOEXEC | > DRM_PANTHOR_VM_BIND_OP_MAP_UNCACHED, > - PANTHOR_VM_KERNEL_AUTO_VA); > + PANTHOR_VM_KERNEL_AUTO_VA, > + "Queue FW interface"); > if (IS_ERR(mem)) > return mem; > > @@ -481,7 +482,8 @@ panthor_fw_alloc_suspend_buf_mem(struct panthor_device *ptdev, size_t size) > return panthor_kernel_bo_create(ptdev, panthor_fw_vm(ptdev), size, > DRM_PANTHOR_BO_NO_MMAP, > DRM_PANTHOR_VM_BIND_OP_MAP_NOEXEC, > - PANTHOR_VM_KERNEL_AUTO_VA); > + PANTHOR_VM_KERNEL_AUTO_VA, > + "FW suspend buffer"); > } > > static int panthor_fw_load_section_entry(struct panthor_device *ptdev, > @@ -601,7 +603,7 @@ static int panthor_fw_load_section_entry(struct panthor_device *ptdev, > section->mem = panthor_kernel_bo_create(ptdev, panthor_fw_vm(ptdev), > section_size, > DRM_PANTHOR_BO_NO_MMAP, > - vm_map_flags, va); > + vm_map_flags, va, "FW section"); > if (IS_ERR(section->mem)) > return PTR_ERR(section->mem); > > diff --git a/drivers/gpu/drm/panthor/panthor_gem.c b/drivers/gpu/drm/panthor/panthor_gem.c > index af0ac17f357f..3c5fc854356e 100644 > --- a/drivers/gpu/drm/panthor/panthor_gem.c > +++ b/drivers/gpu/drm/panthor/panthor_gem.c > @@ -82,7 +82,7 @@ void panthor_kernel_bo_destroy(struct panthor_kernel_bo *bo) > struct panthor_kernel_bo * > panthor_kernel_bo_create(struct panthor_device *ptdev, struct panthor_vm *vm, > size_t size, u32 bo_flags, u32 vm_map_flags, > - u64 gpu_va) > + u64 gpu_va, const char *name) > { > struct drm_gem_shmem_object *obj; > struct panthor_kernel_bo *kbo; > @@ -106,6 +106,8 @@ panthor_kernel_bo_create(struct panthor_device *ptdev, struct panthor_vm *vm, > kbo->obj = &obj->base; > bo->flags = bo_flags; > > + panthor_gem_kernel_bo_set_label(kbo, name); > + > /* The system and GPU MMU page size might differ, which becomes a > * problem for FW sections that need to be mapped at explicit address > * since our PAGE_SIZE alignment might cover a VA range that's > diff --git a/drivers/gpu/drm/panthor/panthor_gem.h b/drivers/gpu/drm/panthor/panthor_gem.h > index beba066b4974..62aea06dbc6d 100644 > --- a/drivers/gpu/drm/panthor/panthor_gem.h > +++ b/drivers/gpu/drm/panthor/panthor_gem.h > @@ -153,7 +153,7 @@ panthor_kernel_bo_vunmap(struct panthor_kernel_bo *bo) > struct panthor_kernel_bo * > panthor_kernel_bo_create(struct panthor_device *ptdev, struct panthor_vm *vm, > size_t size, u32 bo_flags, u32 vm_map_flags, > - u64 gpu_va); > + u64 gpu_va, const char *name); > > void panthor_kernel_bo_destroy(struct panthor_kernel_bo *bo); > > diff --git a/drivers/gpu/drm/panthor/panthor_heap.c b/drivers/gpu/drm/panthor/panthor_heap.c > index 3bdf61c14264..d236e9ceade4 100644 > --- a/drivers/gpu/drm/panthor/panthor_heap.c > +++ b/drivers/gpu/drm/panthor/panthor_heap.c > @@ -151,7 +151,8 @@ static int panthor_alloc_heap_chunk(struct panthor_heap_pool *pool, > chunk->bo = panthor_kernel_bo_create(pool->ptdev, pool->vm, heap->chunk_size, > DRM_PANTHOR_BO_NO_MMAP, > DRM_PANTHOR_VM_BIND_OP_MAP_NOEXEC, > - PANTHOR_VM_KERNEL_AUTO_VA); > + PANTHOR_VM_KERNEL_AUTO_VA, > + "Tiler heap chunk"); > if (IS_ERR(chunk->bo)) { > ret = PTR_ERR(chunk->bo); > goto err_free_chunk; > @@ -555,7 +556,8 @@ panthor_heap_pool_create(struct panthor_device *ptdev, struct panthor_vm *vm) > pool->gpu_contexts = panthor_kernel_bo_create(ptdev, vm, bosize, > DRM_PANTHOR_BO_NO_MMAP, > DRM_PANTHOR_VM_BIND_OP_MAP_NOEXEC, > - PANTHOR_VM_KERNEL_AUTO_VA); > + PANTHOR_VM_KERNEL_AUTO_VA, > + "Heap pool"); > if (IS_ERR(pool->gpu_contexts)) { > ret = PTR_ERR(pool->gpu_contexts); > goto err_destroy_pool; > diff --git a/drivers/gpu/drm/panthor/panthor_sched.c b/drivers/gpu/drm/panthor/panthor_sched.c > index 446ec780eb4a..43ee57728de5 100644 > --- a/drivers/gpu/drm/panthor/panthor_sched.c > +++ b/drivers/gpu/drm/panthor/panthor_sched.c > @@ -3332,7 +3332,8 @@ group_create_queue(struct panthor_group *group, > DRM_PANTHOR_BO_NO_MMAP, > DRM_PANTHOR_VM_BIND_OP_MAP_NOEXEC | > DRM_PANTHOR_VM_BIND_OP_MAP_UNCACHED, > - PANTHOR_VM_KERNEL_AUTO_VA); > + PANTHOR_VM_KERNEL_AUTO_VA, > + "CS ring buffer"); > if (IS_ERR(queue->ringbuf)) { > ret = PTR_ERR(queue->ringbuf); > goto err_free_queue; > @@ -3362,7 +3363,8 @@ group_create_queue(struct panthor_group *group, > DRM_PANTHOR_BO_NO_MMAP, > DRM_PANTHOR_VM_BIND_OP_MAP_NOEXEC | > DRM_PANTHOR_VM_BIND_OP_MAP_UNCACHED, > - PANTHOR_VM_KERNEL_AUTO_VA); > + PANTHOR_VM_KERNEL_AUTO_VA, > + "Group job stats"); > > if (IS_ERR(queue->profiling.slots)) { > ret = PTR_ERR(queue->profiling.slots); > @@ -3493,7 +3495,8 @@ int panthor_group_create(struct panthor_file *pfile, > DRM_PANTHOR_BO_NO_MMAP, > DRM_PANTHOR_VM_BIND_OP_MAP_NOEXEC | > DRM_PANTHOR_VM_BIND_OP_MAP_UNCACHED, > - PANTHOR_VM_KERNEL_AUTO_VA); > + PANTHOR_VM_KERNEL_AUTO_VA, > + "Group sync objects"); > if (IS_ERR(group->syncobjs)) { > ret = PTR_ERR(group->syncobjs); > goto err_put_group;

5 months

Re: [PATCH v7 2/4] drm/panthor: Add driver IOCTL for setting BO labels

by Steven Price

On 11/04/2025 16:03, Adrián Larumbe wrote: > Allow UM to label a BO for which it possesses a DRM handle. > > Signed-off-by: Adrián Larumbe <adrian.larumbe(a)collabora.com> > Reviewed-by: Liviu Dudau <liviu.dudau(a)arm.com> > Reviewed-by: Boris Brezillon <boris.brezillon(a)collabora.com> Reviewed-by: Steven Price <steven.price(a)arm.com> Although very minor NITs below which you can consider. > --- > drivers/gpu/drm/panthor/panthor_drv.c | 42 ++++++++++++++++++++++++++- > drivers/gpu/drm/panthor/panthor_gem.h | 2 ++ > include/uapi/drm/panthor_drm.h | 23 +++++++++++++++ > 3 files changed, 66 insertions(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/panthor/panthor_drv.c b/drivers/gpu/drm/panthor/panthor_drv.c > index 06fe46e32073..983b24f1236c 100644 > --- a/drivers/gpu/drm/panthor/panthor_drv.c > +++ b/drivers/gpu/drm/panthor/panthor_drv.c > @@ -1331,6 +1331,44 @@ static int panthor_ioctl_vm_get_state(struct drm_device *ddev, void *data, > return 0; > } > > +static int panthor_ioctl_bo_set_label(struct drm_device *ddev, void *data, > + struct drm_file *file) > +{ > + struct drm_panthor_bo_set_label *args = data; > + struct drm_gem_object *obj; > + const char *label; > + int ret = 0; > + > + obj = drm_gem_object_lookup(file, args->handle); > + if (!obj) > + return -ENOENT; > + > + if (args->size && args->label) { > + if (args->size > PANTHOR_BO_LABEL_MAXLEN) { > + ret = -E2BIG; > + goto err_label; > + } > + > + label = strndup_user(u64_to_user_ptr(args->label), args->size); > + if (IS_ERR(label)) { > + ret = PTR_ERR(label); > + goto err_label; > + } > + } else if (args->size && !args->label) { > + ret = -EINVAL; > + goto err_label; > + } else { > + label = NULL; > + } > + > + panthor_gem_bo_set_label(obj, label); > + > +err_label: > + drm_gem_object_put(obj); > + > + return ret; > +} > + > static int > panthor_open(struct drm_device *ddev, struct drm_file *file) > { > @@ -1400,6 +1438,7 @@ static const struct drm_ioctl_desc panthor_drm_driver_ioctls[] = { > PANTHOR_IOCTL(TILER_HEAP_CREATE, tiler_heap_create, DRM_RENDER_ALLOW), > PANTHOR_IOCTL(TILER_HEAP_DESTROY, tiler_heap_destroy, DRM_RENDER_ALLOW), > PANTHOR_IOCTL(GROUP_SUBMIT, group_submit, DRM_RENDER_ALLOW), > + PANTHOR_IOCTL(BO_SET_LABEL, bo_set_label, DRM_RENDER_ALLOW), > }; > > static int panthor_mmap(struct file *filp, struct vm_area_struct *vma) > @@ -1509,6 +1548,7 @@ static void panthor_debugfs_init(struct drm_minor *minor) > * - 1.2 - adds DEV_QUERY_GROUP_PRIORITIES_INFO query > * - adds PANTHOR_GROUP_PRIORITY_REALTIME priority > * - 1.3 - adds DRM_PANTHOR_GROUP_STATE_INNOCENT flag > + * - 1.4 - adds DRM_IOCTL_PANTHOR_BO_SET_LABEL ioctl > */ > static const struct drm_driver panthor_drm_driver = { > .driver_features = DRIVER_RENDER | DRIVER_GEM | DRIVER_SYNCOBJ | > @@ -1522,7 +1562,7 @@ static const struct drm_driver panthor_drm_driver = { > .name = "panthor", > .desc = "Panthor DRM driver", > .major = 1, > - .minor = 3, > + .minor = 4, > > .gem_create_object = panthor_gem_create_object, > .gem_prime_import_sg_table = drm_gem_shmem_prime_import_sg_table, > diff --git a/drivers/gpu/drm/panthor/panthor_gem.h b/drivers/gpu/drm/panthor/panthor_gem.h > index af0d77338860..beba066b4974 100644 > --- a/drivers/gpu/drm/panthor/panthor_gem.h > +++ b/drivers/gpu/drm/panthor/panthor_gem.h > @@ -13,6 +13,8 @@ > > struct panthor_vm; > > +#define PANTHOR_BO_LABEL_MAXLEN PAGE_SIZE > + > /** > * struct panthor_gem_object - Driver specific GEM object. > */ > diff --git a/include/uapi/drm/panthor_drm.h b/include/uapi/drm/panthor_drm.h > index 97e2c4510e69..12b1994499a9 100644 > --- a/include/uapi/drm/panthor_drm.h > +++ b/include/uapi/drm/panthor_drm.h > @@ -127,6 +127,9 @@ enum drm_panthor_ioctl_id { > > /** @DRM_PANTHOR_TILER_HEAP_DESTROY: Destroy a tiler heap. */ > DRM_PANTHOR_TILER_HEAP_DESTROY, > + > + /** @DRM_PANTHOR_BO_SET_LABEL: Label a BO. */ > + DRM_PANTHOR_BO_SET_LABEL, > }; > > /** > @@ -977,6 +980,24 @@ struct drm_panthor_tiler_heap_destroy { > __u32 pad; > }; > > +/** > + * struct drm_panthor_bo_set_label - Arguments passed to DRM_IOCTL_PANTHOR_BO_SET_LABEL > + */ > +struct drm_panthor_bo_set_label { > + /** @handle: Handle of the buffer object to label. */ > + __u32 handle; > + > + /** > + * @size: Length of the label, including the NULL terminator. > + * > + * Cannot be greater than the OS page size. > + */ > + __u32 size; > + > + /** @label: User pointer to a NULL-terminated string */ > + __u64 label; > +}; First very minor NIT: * NULL is a pointer, i.e. (void*)0 * NUL is the ASCII code point '\0'. So it's a NUL-terminated string. Second NIT: We don't actually need 'size' - since the string is NUL-terminated we can just strndup_user(__user_pointer__, PAGE_SIZE). As things stand we validate that strlen(label) < size <= PAGE_SIZE - which is a little odd (user space might as well just pass PAGE_SIZE rather than calculate the actual length). Thanks, Steve > + > /** > * DRM_IOCTL_PANTHOR() - Build a Panthor IOCTL number > * @__access: Access type. Must be R, W or RW. > @@ -1019,6 +1040,8 @@ enum { > DRM_IOCTL_PANTHOR(WR, TILER_HEAP_CREATE, tiler_heap_create), > DRM_IOCTL_PANTHOR_TILER_HEAP_DESTROY = > DRM_IOCTL_PANTHOR(WR, TILER_HEAP_DESTROY, tiler_heap_destroy), > + DRM_IOCTL_PANTHOR_BO_SET_LABEL = > + DRM_IOCTL_PANTHOR(WR, BO_SET_LABEL, bo_set_label), > }; > > #if defined(__cplusplus)

5 months

Re: [PATCH v7 1/4] drm/panthor: Introduce BO labeling

by Steven Price

Hi Adrián, On 11/04/2025 16:03, Adrián Larumbe wrote: > Add a new character string Panthor BO field, and a function that allows > setting it from within the driver. > > Driver takes care of freeing the string when it's replaced or no longer > needed at object destruction time, but allocating it is the responsibility > of callers. > > Signed-off-by: Adrián Larumbe <adrian.larumbe(a)collabora.com> > Reviewed-by: Boris Brezillon <boris.brezillon(a)collabora.com> > --- > drivers/gpu/drm/panthor/panthor_gem.c | 39 +++++++++++++++++++++++++++ > drivers/gpu/drm/panthor/panthor_gem.h | 17 ++++++++++++ > 2 files changed, 56 insertions(+) > > diff --git a/drivers/gpu/drm/panthor/panthor_gem.c b/drivers/gpu/drm/panthor/panthor_gem.c > index 8244a4e6c2a2..af0ac17f357f 100644 > --- a/drivers/gpu/drm/panthor/panthor_gem.c > +++ b/drivers/gpu/drm/panthor/panthor_gem.c > @@ -2,6 +2,7 @@ > /* Copyright 2019 Linaro, Ltd, Rob Herring <robh(a)kernel.org> */ > /* Copyright 2023 Collabora ltd. */ > > +#include <linux/cleanup.h> > #include <linux/dma-buf.h> > #include <linux/dma-mapping.h> > #include <linux/err.h> > @@ -18,6 +19,14 @@ static void panthor_gem_free_object(struct drm_gem_object *obj) > struct panthor_gem_object *bo = to_panthor_bo(obj); > struct drm_gem_object *vm_root_gem = bo->exclusive_vm_root_gem; > > + /* > + * Label might have been allocated with kstrdup_const(), > + * we need to take that into account when freeing the memory > + */ > + kfree_const(bo->label.str); > + > + mutex_destroy(&bo->label.lock); > + > drm_gem_free_mmap_offset(&bo->base.base); > mutex_destroy(&bo->gpuva_list_lock); > drm_gem_shmem_free(&bo->base); > @@ -196,6 +205,7 @@ struct drm_gem_object *panthor_gem_create_object(struct drm_device *ddev, size_t > obj->base.map_wc = !ptdev->coherent; > mutex_init(&obj->gpuva_list_lock); > drm_gem_gpuva_set_lock(&obj->base.base, &obj->gpuva_list_lock); > + mutex_init(&obj->label.lock); > > return &obj->base.base; > } > @@ -247,3 +257,32 @@ panthor_gem_create_with_handle(struct drm_file *file, > > return ret; > } > + > +void > +panthor_gem_bo_set_label(struct drm_gem_object *obj, const char *label) > +{ > + struct panthor_gem_object *bo = to_panthor_bo(obj); > + const char *old_label; > + > + scoped_guard(mutex, &bo->label.lock) { > + old_label = bo->label.str; > + bo->label.str = label; > + } > + > + kfree(old_label); Shouldn't this be kfree_const()? I suspect as things stand we can't trigger this bug but it's inconsistent. > +} > + > +void > +panthor_gem_kernel_bo_set_label(struct panthor_kernel_bo *bo, const char *label) > +{ > + const char *str; > + > + str = kstrdup_const(label, GFP_KERNEL); > + if (!str) { In the next patch you permit user space to clear the label (args->size==0) which means label==NULL and AFAICT kstrdup_const() will return NULL in this case triggering this warning. Thanks, Steve > + /* Failing to allocate memory for a label isn't a fatal condition */ > + drm_warn(bo->obj->dev, "Not enough memory to allocate BO label"); > + return; > + } > + > + panthor_gem_bo_set_label(bo->obj, str); > +} > diff --git a/drivers/gpu/drm/panthor/panthor_gem.h b/drivers/gpu/drm/panthor/panthor_gem.h > index 1a363bb814f4..af0d77338860 100644 > --- a/drivers/gpu/drm/panthor/panthor_gem.h > +++ b/drivers/gpu/drm/panthor/panthor_gem.h > @@ -46,6 +46,20 @@ struct panthor_gem_object { > > /** @flags: Combination of drm_panthor_bo_flags flags. */ > u32 flags; > + > + /** > + * @label: BO tagging fields. The label can be assigned within the > + * driver itself or through a specific IOCTL. > + */ > + struct { > + /** > + * @label.str: Pointer to NULL-terminated string, > + */ > + const char *str; > + > + /** @lock.str: Protects access to the @label.str field. */ > + struct mutex lock; > + } label; > }; > > /** > @@ -91,6 +105,9 @@ panthor_gem_create_with_handle(struct drm_file *file, > struct panthor_vm *exclusive_vm, > u64 *size, u32 flags, uint32_t *handle); > > +void panthor_gem_bo_set_label(struct drm_gem_object *obj, const char *label); > +void panthor_gem_kernel_bo_set_label(struct panthor_kernel_bo *bo, const char *label); > + > static inline u64 > panthor_kernel_bo_gpuva(struct panthor_kernel_bo *bo) > {

5 months

Re: [PATCH 0/3] uio/dma-buf: Give UIO users access to DMA addresses.

by Christian König

Am 14.04.25 um 10:24 schrieb Thomas Petazzoni: > Hello Christoph, > > On Sun, 13 Apr 2025 22:55:02 -0700 > Christoph Hellwig <hch(a)infradead.org> wrote: > >> On Thu, Apr 10, 2025 at 04:53:17PM +0200, Bastien Curutchet wrote: >>> Hi all, >>> >>> Many UIO users performing DMA from their UIO device need to access the >>> DMA addresses of the allocated buffers. There are out-of-tree drivers >>> that allow to do it but nothing in the mainline. >> In which case it does not matter at all for mainline. > It is impressive how "out-of-tree" triggers kernel maintainers, and > then end up not even reading what the patch series is all about (but I > forgive you, because it triggers me in the same way). > > This patch series is NOT about adding new kernel APIs meant to be used > by out-of-tree drivers, which of course would be bad, and we would have > never proposed. > > What this patch series is about is to add new user-space interface to > extend the existing UIO subsystem. What my colleague Bastien was > mentioning about out-of-tree drivers is that there are a LOT of > out-of-tree drivers extending UIO to allow user-space applications to > do DMA, and our proposal is that considering how many people need that > and implement ugly out-of-tree drivers to solve this issue, we suggest > the mainline kernel should have a built in solution. > > Please re-read again, and realize that we are NOT adding new kernel > APIs for out-of-tree drivers. > >> FYI the proper and safe way to do DMA from userspace is to use >> vfio/iommufd. > I am not sure how this can work in our use-case. We have a very simple > set of IP blocks implemented in a FPGA, some of those IP blocks are > able to perform DMA operations. The register of those IP blocks are > mapped into a user-space application using the existing, accepted > upstream, UIO subsystem. Some of those registers allow to program DMA > transfers. So far, we can do all what we need, except program those DMA > transfers. Lots of people are having the same issue, and zillions of > ugly out-of-tree solutions flourish all over, and we're trying to see > if we can constructively find a solution that would be acceptable > upstream to resolve this use-case. Our platform is an old PowerPC with > no IOMMU. Maybe I should try to better explain the concern here. The question is "Where is the source code of your FPGA driver?". I mean that you are trying to replace the out-of-tree solution is rather welcomed, but the out-of-tree solutions are out-of-tree because they don't fit with requirements to be added to the core Linux tree. And one of those requirements is that you need to provide the source code of the userspace user of this interface, in this case here that is your FPGA driver. An MIT/X11 license is usually sufficient, GPL is of course seen as better and it must not be a toy application, but rather the real thing. And that is what people usually don't want and that's why no in-tree solution exists for this. Regards, Christian. > > Note that the safety argument doesn't hold: UIO already allows to map > registers into user-space applications, which can already program DMA > transfers to arbitrary locations in memory. The safety comes from the > fact that such UIO devices are associated with permissions that the > system administrator controls, to decide which applications are safe > enough to be granted access to those UIO devices. Therefore, providing > access through UIO of the physical address of well-defined DMA buffers > properly allocated through a sane API is not reducing the safety of > anything compared to what UIO already allows today. > > Best regards, > > Thomas

5 months

Re: [PATCH 0/3] uio/dma-buf: Give UIO users access to DMA addresses.

by Christian König

Am 14.04.25 um 10:17 schrieb Thomas Petazzoni: > Hello Christian, > > On Fri, 11 Apr 2025 14:41:56 +0200 > Christian König <christian.koenig(a)amd.com> wrote: > >> But anyway please note that when you want to create new UAPI you need >> to provide an open source user of it. E.g. link to a repository or >> something similar in the covert letter should do it. > Could you clarify what is the "reference" user-space user of UIO that > exists today? I honestly don't know. I have never looked into UIO before this patch set arrived. What I mean is that the general rule to justify adding UAPI to the Linux kernel is that you need to have an open source user of that UAPI. In other words for UIO this means that you *can't* do things like exposing some kernel functionality like DMA-buf through UIO and then write a binary only driver on top of it. Regards, Christian. > > Thomas

5 months

Jump to page:

2025

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

2012

2011

Linaro-mm-sig