| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <20250402150812.0595465a@collabora.com>
Date: Wed, 2 Apr 2025 15:08:12 +0200
From: Boris Brezillon <boris.brezillon@...labora.com>
To: Adrián Larumbe <adrian.larumbe@...labora.com>
Cc: Steven Price <steven.price@....com>, Liviu Dudau <liviu.dudau@....com>,
Maarten Lankhorst <maarten.lankhorst@...ux.intel.com>, Maxime Ripard
<mripard@...nel.org>, Thomas Zimmermann <tzimmermann@...e.de>, David Airlie
<airlied@...il.com>, Simona Vetter <simona@...ll.ch>, Sumit Semwal
<sumit.semwal@...aro.org>, Christian König
<christian.koenig@....com>, kernel@...labora.com,
dri-devel@...ts.freedesktop.org, linux-kernel@...r.kernel.org,
linux-media@...r.kernel.org, linaro-mm-sig@...ts.linaro.org
Subject: Re: [PATCH v4 3/4] drm/panthor: Label all kernel BO's
On Wed, 2 Apr 2025 12:54:28 +0100
Adrián Larumbe <adrian.larumbe@...labora.com> 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
> 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.
>
> Signed-off-by: Adrián Larumbe <adrian.larumbe@...labora.com>
> ---
> drivers/gpu/drm/panthor/panthor_fw.c | 8 +++++---
> drivers/gpu/drm/panthor/panthor_gem.c | 3 ++-
> 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, 18 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/gpu/drm/panthor/panthor_fw.c b/drivers/gpu/drm/panthor/panthor_fw.c
> index 4a9c4afa9ad7..36e60bb2dcc5 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");
nit: Let's try to use the caps consistently in the names we assign to
kernel BOs, like, cap on the first word only, or cap on each word, I
don't mind, but pick one and try to stick to it.
> 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 7d017f9d1d52..44d027e6d664 100644
> --- a/drivers/gpu/drm/panthor/panthor_gem.c
> +++ b/drivers/gpu/drm/panthor/panthor_gem.c
> @@ -81,7 +81,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;
> @@ -105,6 +105,7 @@ 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);
nit: can we add a blank line here, and remove the one that's after the
bo->flags assignment?
> /* 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 e18fbc093abd..49daa5088a0d 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 db0285ce5812..ad122bd37ac2 100644
> --- a/drivers/gpu/drm/panthor/panthor_heap.c
> +++ b/drivers/gpu/drm/panthor/panthor_heap.c
> @@ -147,7 +147,8 @@ static int panthor_alloc_heap_chunk(struct panthor_device *ptdev,
> chunk->bo = panthor_kernel_bo_create(ptdev, 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;
> @@ -550,7 +551,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 1a276db095ff..a0b8f1ba4ea8 100644
> --- a/drivers/gpu/drm/panthor/panthor_sched.c
> +++ b/drivers/gpu/drm/panthor/panthor_sched.c
> @@ -3334,7 +3334,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,
> + "Ring buffer");
> if (IS_ERR(queue->ringbuf)) {
> ret = PTR_ERR(queue->ringbuf);
> goto err_free_queue;
> @@ -3364,7 +3365,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,
> + "fdinfo slots");
I think I'd prefer "Group job stats", just in case we end up dumping
other stuff that's not exposed through fdinfo there.
>
> if (IS_ERR(queue->profiling.slots)) {
> ret = PTR_ERR(queue->profiling.slots);
> @@ -3495,7 +3497,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;
Powered by blists - more mailing lists