* [PATCH v4 01/12] drm/panthor: De-duplicate FW memory section sync
2026-10-12 12:59 [PATCH v4 00/12] drm/panthor: Protected mode support for Mali CSF GPUs Ketil Johnsen
@ 2026-10-12 12:59 ` Ketil Johnsen
2026-10-12 12:59 ` [PATCH v4 02/12] drm/panthor: Minor scheduler refactoring Ketil Johnsen
` (10 subsequent siblings)
11 siblings, 0 replies; 18+ messages in thread
From: Ketil Johnsen @ 2026-10-12 12:59 UTC (permalink / raw)
To: Boris Brezillon, Steven Price, Liviu Dudau, Maarten Lankhorst,
Maxime Ripard, Thomas Zimmermann, David Airlie, Simona Vetter,
Daniel Almeida, Alice Ryhl
Cc: dri-devel, linux-kernel, Ketil Johnsen
Handle the sync to device of FW memory sections inside
panthor_fw_init_section_mem() so that the callers do not have to.
This small improvement is also critical for protected FW sections,
so we avoid issuing memory transactions to protected memory from
CPU running in normal mode.
Signed-off-by: Ketil Johnsen <ketil.johnsen@arm.com>
Reviewed-by: Boris Brezillon <boris.brezillon@collabora.com>
Reviewed-by: Liviu Dudau <liviu.dudau@arm.com>
Reviewed-by: Steven Price <steven.price@arm.com>
---
drivers/gpu/drm/panthor/panthor_fw.c | 23 +++++++----------------
1 file changed, 7 insertions(+), 16 deletions(-)
diff --git a/drivers/gpu/drm/panthor/panthor_fw.c b/drivers/gpu/drm/panthor/panthor_fw.c
index 4f1fab66a13bf..5f9f7a92c56a8 100644
--- a/drivers/gpu/drm/panthor/panthor_fw.c
+++ b/drivers/gpu/drm/panthor/panthor_fw.c
@@ -449,6 +449,7 @@ static void panthor_fw_init_section_mem(struct panthor_device *ptdev,
struct panthor_fw_section *section)
{
bool was_mapped = !!section->mem->kmap;
+ struct sg_table *sgt;
int ret;
if (!section->data.size &&
@@ -467,6 +468,12 @@ static void panthor_fw_init_section_mem(struct panthor_device *ptdev,
if (!was_mapped)
panthor_kernel_bo_vunmap(section->mem);
+
+ /* An sgt should have been requested when the kernel BO was GPU-mapped. */
+ sgt = to_panthor_bo(section->mem->obj)->dmap.sgt;
+ if (!drm_WARN_ON_ONCE(&ptdev->base, !sgt))
+ dma_sync_sgtable_for_device(ptdev->base.dev, sgt,
+ DMA_TO_DEVICE);
}
/**
@@ -639,7 +646,6 @@ static int panthor_fw_load_section_entry(struct panthor_device *ptdev,
if (section_size) {
u32 cache_mode = hdr.flags & CSF_FW_BINARY_IFACE_ENTRY_CACHE_MODE_MASK;
- struct panthor_gem_object *bo;
u32 vm_map_flags = 0;
u64 va = hdr.va.start;
@@ -676,14 +682,6 @@ static int panthor_fw_load_section_entry(struct panthor_device *ptdev,
}
panthor_fw_init_section_mem(ptdev, section);
-
- bo = to_panthor_bo(section->mem->obj);
-
- /* An sgt should have been requested when the kernel BO was GPU-mapped. */
- if (drm_WARN_ON_ONCE(&ptdev->base, !bo->dmap.sgt))
- return -EINVAL;
-
- dma_sync_sgtable_for_device(ptdev->base.dev, bo->dmap.sgt, DMA_TO_DEVICE);
}
if (hdr.va.start == CSF_MCU_SHARED_REGION_START)
@@ -738,17 +736,10 @@ panthor_reload_fw_sections(struct panthor_device *ptdev, bool full_reload)
struct panthor_fw_section *section;
list_for_each_entry(section, &ptdev->fw->sections, node) {
- struct sg_table *sgt;
-
if (!full_reload && !(section->flags & CSF_FW_BINARY_IFACE_ENTRY_WR))
continue;
panthor_fw_init_section_mem(ptdev, section);
-
- /* An sgt should have been requested when the kernel BO was GPU-mapped. */
- sgt = to_panthor_bo(section->mem->obj)->dmap.sgt;
- if (!drm_WARN_ON_ONCE(&ptdev->base, !sgt))
- dma_sync_sgtable_for_device(ptdev->base.dev, sgt, DMA_TO_DEVICE);
}
}
--
2.43.0
^ permalink raw reply related [flat|nested] 18+ messages in thread* [PATCH v4 02/12] drm/panthor: Minor scheduler refactoring
2026-10-12 12:59 [PATCH v4 00/12] drm/panthor: Protected mode support for Mali CSF GPUs Ketil Johnsen
2026-10-12 12:59 ` [PATCH v4 01/12] drm/panthor: De-duplicate FW memory section sync Ketil Johnsen
@ 2026-10-12 12:59 ` Ketil Johnsen
2026-10-12 13:05 ` sashiko-bot
2026-10-12 12:59 ` [PATCH v4 03/12] drm/panthor: Pass drm_file instead of panthor_file Ketil Johnsen
` (9 subsequent siblings)
11 siblings, 1 reply; 18+ messages in thread
From: Ketil Johnsen @ 2026-10-12 12:59 UTC (permalink / raw)
To: Boris Brezillon, Steven Price, Liviu Dudau, Maarten Lankhorst,
Maxime Ripard, Thomas Zimmermann, David Airlie, Simona Vetter,
Daniel Almeida, Alice Ryhl
Cc: dri-devel, linux-kernel, Florent Tomasin, Ketil Johnsen
From: Florent Tomasin <florent.tomasin@arm.com>
Refactor parts of the group scheduling logic into new helper functions.
This will simplify addition of the protected mode feature.
Remove redundant assignments of csg_slot.
Reviewed-by: Boris Brezillon <boris.brezillon@collabora.com>
Signed-off-by: Florent Tomasin <florent.tomasin@arm.com>
Co-developed-by: Ketil Johnsen <ketil.johnsen@arm.com>
Signed-off-by: Ketil Johnsen <ketil.johnsen@arm.com>
---
v4:
- Handle error from csg_slot_prog_locked()
v3:
- Use panthor_fw_csg_endpoint_req_update() (was rebase mistake)
- Function rename to tick_ctx_update_group_prio()
v2:
- Moved option to ding only the CSG doorbell to later patch
---
drivers/gpu/drm/panthor/panthor_sched.c | 135 +++++++++++++++---------
1 file changed, 83 insertions(+), 52 deletions(-)
diff --git a/drivers/gpu/drm/panthor/panthor_sched.c b/drivers/gpu/drm/panthor/panthor_sched.c
index a343c5180e01e..023abaf41890c 100644
--- a/drivers/gpu/drm/panthor/panthor_sched.c
+++ b/drivers/gpu/drm/panthor/panthor_sched.c
@@ -2345,12 +2345,85 @@ tick_ctx_cleanup(struct panthor_scheduler *sched,
}
}
+static void
+tick_ctx_evict_group(struct panthor_scheduler *sched,
+ struct panthor_csg_slots_upd_ctx *upd_ctx,
+ struct panthor_group *group)
+{
+ struct panthor_device *ptdev = sched->ptdev;
+
+ if (drm_WARN_ON(&ptdev->base, group->csg_id < 0))
+ return;
+
+ csgs_upd_ctx_queue_reqs(ptdev, upd_ctx, group->csg_id,
+ group_can_run(group) ?
+ CSG_STATE_SUSPEND : CSG_STATE_TERMINATE,
+ CSG_STATE_MASK);
+}
+
+static void
+tick_ctx_update_group_prio(struct panthor_scheduler *sched,
+ struct panthor_csg_slots_upd_ctx *upd_ctx,
+ struct panthor_group *group,
+ int new_csg_prio)
+{
+ struct panthor_device *ptdev = sched->ptdev;
+ struct panthor_fw_csg_iface *csg_iface;
+ struct panthor_csg_slot *csg_slot;
+
+ if (group->csg_id < 0)
+ return;
+
+ csg_iface = panthor_fw_get_csg_iface(ptdev, group->csg_id);
+ csg_slot = &sched->csg_slots[group->csg_id];
+
+ if (csg_slot->priority != new_csg_prio) {
+ panthor_fw_csg_endpoint_req_update(ptdev, csg_iface,
+ CSG_EP_REQ_PRIORITY(new_csg_prio),
+ CSG_EP_REQ_PRIORITY_MASK);
+ csgs_upd_ctx_queue_reqs(ptdev, upd_ctx, group->csg_id,
+ csg_iface->output->ack ^ CSG_ENDPOINT_CONFIG,
+ CSG_ENDPOINT_CONFIG);
+ }
+}
+
+static int
+tick_ctx_schedule_group(struct panthor_scheduler *sched,
+ struct panthor_csg_slots_upd_ctx *upd_ctx,
+ struct panthor_group *group,
+ int csg_id, int csg_prio)
+{
+ struct panthor_device *ptdev = sched->ptdev;
+ struct panthor_fw_csg_iface *csg_iface =
+ panthor_fw_get_csg_iface(ptdev, csg_id);
+ int ret;
+
+ ret = group_bind_locked(group, csg_id);
+ if (ret)
+ return ret;
+
+ csg_slot_prog_locked(ptdev, csg_id, csg_prio);
+ if (ret) {
+ group_unbind_locked(group);
+ return ret;
+ }
+
+ csgs_upd_ctx_queue_reqs(ptdev, upd_ctx, csg_id,
+ group->state == PANTHOR_CS_GROUP_SUSPENDED ?
+ CSG_STATE_RESUME : CSG_STATE_START,
+ CSG_STATE_MASK);
+ csgs_upd_ctx_queue_reqs(ptdev, upd_ctx, csg_id,
+ csg_iface->output->ack ^ CSG_ENDPOINT_CONFIG,
+ CSG_ENDPOINT_CONFIG);
+
+ return 0;
+}
+
static void
tick_ctx_apply(struct panthor_scheduler *sched, struct panthor_sched_tick_ctx *ctx)
{
struct panthor_group *group, *tmp;
struct panthor_device *ptdev = sched->ptdev;
- struct panthor_csg_slot *csg_slot;
int prio, new_csg_prio = MAX_CSG_PRIO, i;
u32 free_csg_slots = 0;
struct panthor_csg_slots_upd_ctx upd_ctx;
@@ -2360,44 +2433,13 @@ tick_ctx_apply(struct panthor_scheduler *sched, struct panthor_sched_tick_ctx *c
for (prio = PANTHOR_CSG_PRIORITY_COUNT - 1; prio >= 0; prio--) {
/* Suspend or terminate evicted groups. */
- list_for_each_entry(group, &ctx->old_groups[prio], run_node) {
- bool term = !group_can_run(group);
- int csg_id = group->csg_id;
-
- if (drm_WARN_ON(&ptdev->base, csg_id < 0))
- continue;
-
- csg_slot = &sched->csg_slots[csg_id];
- csgs_upd_ctx_queue_reqs(ptdev, &upd_ctx, csg_id,
- term ? CSG_STATE_TERMINATE : CSG_STATE_SUSPEND,
- CSG_STATE_MASK);
- }
+ list_for_each_entry(group, &ctx->old_groups[prio], run_node)
+ tick_ctx_evict_group(sched, &upd_ctx, group);
/* Update priorities on already running groups. */
- list_for_each_entry(group, &ctx->groups[prio], run_node) {
- struct panthor_fw_csg_iface *csg_iface;
- int csg_id = group->csg_id;
-
- if (csg_id < 0) {
- new_csg_prio--;
- continue;
- }
-
- csg_slot = &sched->csg_slots[csg_id];
- csg_iface = panthor_fw_get_csg_iface(ptdev, csg_id);
- if (csg_slot->priority == new_csg_prio) {
- new_csg_prio--;
- continue;
- }
-
- panthor_fw_csg_endpoint_req_update(ptdev, csg_iface,
- CSG_EP_REQ_PRIORITY(new_csg_prio),
- CSG_EP_REQ_PRIORITY_MASK);
- csgs_upd_ctx_queue_reqs(ptdev, &upd_ctx, csg_id,
- csg_iface->output->ack ^ CSG_ENDPOINT_CONFIG,
- CSG_ENDPOINT_CONFIG);
- new_csg_prio--;
- }
+ list_for_each_entry(group, &ctx->groups[prio], run_node)
+ tick_ctx_update_group_prio(sched, &upd_ctx, group,
+ new_csg_prio--);
}
ret = csgs_upd_ctx_apply_locked(ptdev, &upd_ctx);
@@ -2425,34 +2467,23 @@ tick_ctx_apply(struct panthor_scheduler *sched, struct panthor_sched_tick_ctx *c
for (prio = PANTHOR_CSG_PRIORITY_COUNT - 1; prio >= 0; prio--) {
list_for_each_entry(group, &ctx->groups[prio], run_node) {
int csg_id = group->csg_id;
- struct panthor_fw_csg_iface *csg_iface;
+ int csg_prio = new_csg_prio--;
- if (csg_id >= 0) {
- new_csg_prio--;
+ if (csg_id >= 0)
continue;
- }
csg_id = ffs(free_csg_slots) - 1;
if (drm_WARN_ON(&ptdev->base, csg_id < 0))
break;
- csg_iface = panthor_fw_get_csg_iface(ptdev, csg_id);
- csg_slot = &sched->csg_slots[csg_id];
- ret = group_bind_locked(group, csg_id);
+ ret = tick_ctx_schedule_group(sched, &upd_ctx, group,
+ csg_id, csg_prio);
if (ret) {
panthor_device_schedule_reset(ptdev);
ctx->csg_upd_failed_mask |= BIT(csg_id);
return;
}
- csg_slot_prog_locked(ptdev, csg_id, new_csg_prio--);
- csgs_upd_ctx_queue_reqs(ptdev, &upd_ctx, csg_id,
- group->state == PANTHOR_CS_GROUP_SUSPENDED ?
- CSG_STATE_RESUME : CSG_STATE_START,
- CSG_STATE_MASK);
- csgs_upd_ctx_queue_reqs(ptdev, &upd_ctx, csg_id,
- csg_iface->output->ack ^ CSG_ENDPOINT_CONFIG,
- CSG_ENDPOINT_CONFIG);
free_csg_slots &= ~BIT(csg_id);
}
}
--
2.43.0
^ permalink raw reply related [flat|nested] 18+ messages in thread* Re: [PATCH v4 02/12] drm/panthor: Minor scheduler refactoring
2026-10-12 12:59 ` [PATCH v4 02/12] drm/panthor: Minor scheduler refactoring Ketil Johnsen
@ 2026-10-12 13:05 ` sashiko-bot
0 siblings, 0 replies; 18+ messages in thread
From: sashiko-bot @ 2026-10-12 13:05 UTC (permalink / raw)
To: Ketil Johnsen; +Cc: dri-devel
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] Unassigned return value from csg_slot_prog_locked() causes dead error path
--
commit 7eff0da835de3ef4c810c3a0eb36aa6ce221f295
Author: Florent Tomasin <florent.tomasin@arm.com>
drm/panthor: Minor scheduler refactoring
This commit refactors parts of the group scheduling logic into new helper
functions. This change simplifies the addition of the protected mode feature
and removes redundant assignments of csg_slot.
> diff --git a/drivers/gpu/drm/panthor/panthor_sched.c b/drivers/gpu/drm/panthor/panthor_sched.c
> index 0493858e55d76..68425fe561d1a 100644
> --- a/drivers/gpu/drm/panthor/panthor_sched.c
> +++ b/drivers/gpu/drm/panthor/panthor_sched.c
> @@ -2345,12 +2345,85 @@ tick_ctx_cleanup(struct panthor_scheduler *sched,
[ ... ]
> +static int
> +tick_ctx_schedule_group(struct panthor_scheduler *sched,
> + struct panthor_csg_slots_upd_ctx *upd_ctx,
> + struct panthor_group *group,
> + int csg_id, int csg_prio)
> +{
> + struct panthor_device *ptdev = sched->ptdev;
> + struct panthor_fw_csg_iface *csg_iface =
> + panthor_fw_get_csg_iface(ptdev, csg_id);
> + int ret;
> +
> + ret = group_bind_locked(group, csg_id);
> + if (ret)
> + return ret;
> +
> + csg_slot_prog_locked(ptdev, csg_id, csg_prio);
> + if (ret) {
> + group_unbind_locked(group);
> + return ret;
> + }
[Severity: Medium]
Is the return value from csg_slot_prog_locked() intentionally ignored here?
Because `ret` was successfully checked after group_bind_locked(), it is
guaranteed to be 0 at this point. As a result, the `if (ret)` check will
always evaluate to false, making the error handling block dead code.
If csg_slot_prog_locked() fails, the failure is silently ignored and the
function proceeds to queue CSG_STATE_START hardware requests for a slot that
hasn't been successfully programmed, which can lead to hardware timeouts or
undefined behavior.
Should this be `ret = csg_slot_prog_locked(ptdev, csg_id, csg_prio);` instead,
matching the patch's stated intention in the v4 changelog to handle the error
from this function?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20261012125940.3291951-1-ketil.johnsen@arm.com?part=2
^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH v4 03/12] drm/panthor: Pass drm_file instead of panthor_file
2026-10-12 12:59 [PATCH v4 00/12] drm/panthor: Protected mode support for Mali CSF GPUs Ketil Johnsen
2026-10-12 12:59 ` [PATCH v4 01/12] drm/panthor: De-duplicate FW memory section sync Ketil Johnsen
2026-10-12 12:59 ` [PATCH v4 02/12] drm/panthor: Minor scheduler refactoring Ketil Johnsen
@ 2026-10-12 12:59 ` Ketil Johnsen
2026-10-12 12:59 ` [PATCH v4 04/12] drm/panthor: Don't allocate protm_suspend_buf Ketil Johnsen
` (8 subsequent siblings)
11 siblings, 0 replies; 18+ messages in thread
From: Ketil Johnsen @ 2026-10-12 12:59 UTC (permalink / raw)
To: Boris Brezillon, Steven Price, Liviu Dudau, Maarten Lankhorst,
Maxime Ripard, Thomas Zimmermann, David Airlie, Simona Vetter,
Daniel Almeida, Alice Ryhl
Cc: dri-devel, linux-kernel, Ketil Johnsen
From: Boris Brezillon <boris.brezillon@collabora.com>
Some sched helpers need info that are part of drm_file, and we will
soon need to call drm_gem_object_lookup() from panthor_group_create().
Let's prepare for that by passing a drm_file instead of panthor_file to
all current helpers taking a panthor_file, so we keep things consistent.
Signed-off-by: Boris Brezillon <boris.brezillon@collabora.com>
Co-developed-by: Ketil Johnsen <ketil.johnsen@arm.com>
Signed-off-by: Ketil Johnsen <ketil.johnsen@arm.com>
---
v3:
- Fixed incorrect parameter passed to panthor_gpu_show_fdinfo()
v2:
- First introduction of this patch in this patch set
---
drivers/gpu/drm/panthor/panthor_drv.c | 38 +++++++++++--------------
drivers/gpu/drm/panthor/panthor_mmu.c | 11 ++++---
drivers/gpu/drm/panthor/panthor_mmu.h | 6 ++--
drivers/gpu/drm/panthor/panthor_sched.c | 37 ++++++++++++++----------
drivers/gpu/drm/panthor/panthor_sched.h | 23 +++++++--------
5 files changed, 59 insertions(+), 56 deletions(-)
diff --git a/drivers/gpu/drm/panthor/panthor_drv.c b/drivers/gpu/drm/panthor/panthor_drv.c
index 1ca8aedd83f46..51a2e5358cd51 100644
--- a/drivers/gpu/drm/panthor/panthor_drv.c
+++ b/drivers/gpu/drm/panthor/panthor_drv.c
@@ -1110,7 +1110,6 @@ static int panthor_ioctl_bo_mmap_offset(struct drm_device *ddev, void *data,
static int panthor_ioctl_group_submit(struct drm_device *ddev, void *data,
struct drm_file *file)
{
- struct panthor_file *pfile = file->driver_priv;
struct drm_panthor_group_submit *args = data;
struct drm_panthor_queue_submit *jobs_args;
struct panthor_submit_ctx ctx;
@@ -1135,8 +1134,7 @@ static int panthor_ioctl_group_submit(struct drm_device *ddev, void *data,
const struct drm_panthor_queue_submit *qsubmit = &jobs_args[i];
struct drm_sched_job *job;
- job = panthor_job_create(pfile, args->group_handle, qsubmit,
- file->client_id);
+ job = panthor_job_create(file, args->group_handle, qsubmit);
if (IS_ERR(job)) {
ret = PTR_ERR(job);
goto out_cleanup_submit_ctx;
@@ -1216,19 +1214,17 @@ static int panthor_ioctl_group_submit(struct drm_device *ddev, void *data,
static int panthor_ioctl_group_destroy(struct drm_device *ddev, void *data,
struct drm_file *file)
{
- struct panthor_file *pfile = file->driver_priv;
struct drm_panthor_group_destroy *args = data;
if (args->pad)
return -EINVAL;
- return panthor_group_destroy(pfile, args->group_handle);
+ return panthor_group_destroy(file, args->group_handle);
}
static int panthor_ioctl_group_create(struct drm_device *ddev, void *data,
struct drm_file *file)
{
- struct panthor_file *pfile = file->driver_priv;
struct drm_panthor_group_create *args = data;
struct drm_panthor_queue_create *queue_args;
int ret;
@@ -1244,7 +1240,7 @@ static int panthor_ioctl_group_create(struct drm_device *ddev, void *data,
if (ret)
goto out;
- ret = panthor_group_create(pfile, args, queue_args, file->client_id);
+ ret = panthor_group_create(file, args, queue_args);
if (ret < 0)
goto out;
args->group_handle = ret;
@@ -1258,10 +1254,9 @@ static int panthor_ioctl_group_create(struct drm_device *ddev, void *data,
static int panthor_ioctl_group_get_state(struct drm_device *ddev, void *data,
struct drm_file *file)
{
- struct panthor_file *pfile = file->driver_priv;
struct drm_panthor_group_get_state *args = data;
- return panthor_group_get_state(pfile, args);
+ return panthor_group_get_state(file, args);
}
static int panthor_ioctl_tiler_heap_create(struct drm_device *ddev, void *data,
@@ -1604,6 +1599,7 @@ panthor_open(struct drm_device *ddev, struct drm_file *file)
if (!pfile)
return -ENOMEM;
+ file->driver_priv = pfile;
pfile->ptdev = ptdev;
pfile->user_mmio.offset = DRM_PANTHOR_USER_MMIO_OFFSET;
@@ -1618,19 +1614,18 @@ panthor_open(struct drm_device *ddev, struct drm_file *file)
#endif
- ret = panthor_vm_pool_create(pfile);
+ ret = panthor_vm_pool_create(file);
if (ret)
goto err_free_file;
- ret = panthor_group_pool_create(pfile);
+ ret = panthor_group_pool_create(file);
if (ret)
goto err_destroy_vm_pool;
- file->driver_priv = pfile;
return 0;
err_destroy_vm_pool:
- panthor_vm_pool_destroy(pfile);
+ panthor_vm_pool_destroy(file);
err_free_file:
kfree(pfile);
@@ -1642,8 +1637,8 @@ panthor_postclose(struct drm_device *ddev, struct drm_file *file)
{
struct panthor_file *pfile = file->driver_priv;
- panthor_group_pool_destroy(pfile);
- panthor_vm_pool_destroy(pfile);
+ panthor_group_pool_destroy(file);
+ panthor_vm_pool_destroy(file);
kfree(pfile);
}
@@ -1703,11 +1698,13 @@ static int panthor_mmap(struct file *filp, struct vm_area_struct *vma)
}
static void panthor_gpu_show_fdinfo(struct panthor_device *ptdev,
- struct panthor_file *pfile,
+ struct drm_file *file,
struct drm_printer *p)
{
+ struct panthor_file *pfile = file->driver_priv;
+
if (ptdev->profile_mask & PANTHOR_DEVICE_PROFILING_ALL)
- panthor_fdinfo_gather_group_samples(pfile);
+ panthor_fdinfo_gather_group_samples(file);
if (ptdev->profile_mask & PANTHOR_DEVICE_PROFILING_TIMESTAMP) {
#ifdef CONFIG_ARM_ARCH_TIMER
@@ -1727,11 +1724,10 @@ static void panthor_gpu_show_fdinfo(struct panthor_device *ptdev,
static void panthor_show_internal_memory_stats(struct drm_printer *p, struct drm_file *file)
{
char *drv_name = file->minor->dev->driver->name;
- struct panthor_file *pfile = file->driver_priv;
struct drm_memory_stats stats = {0};
- panthor_fdinfo_gather_group_mem_info(pfile, &stats);
- panthor_vm_heaps_sizes(pfile, &stats);
+ panthor_fdinfo_gather_group_mem_info(file, &stats);
+ panthor_vm_heaps_sizes(file, &stats);
drm_fdinfo_print_size(p, drv_name, "resident", "memory", stats.resident);
drm_fdinfo_print_size(p, drv_name, "active", "memory", stats.active);
@@ -1742,7 +1738,7 @@ static void panthor_show_fdinfo(struct drm_printer *p, struct drm_file *file)
struct drm_device *dev = file->minor->dev;
struct panthor_device *ptdev = container_of(dev, struct panthor_device, base);
- panthor_gpu_show_fdinfo(ptdev, file->driver_priv, p);
+ panthor_gpu_show_fdinfo(ptdev, file, p);
panthor_show_internal_memory_stats(p, file);
drm_show_memory_stats(p, file);
diff --git a/drivers/gpu/drm/panthor/panthor_mmu.c b/drivers/gpu/drm/panthor/panthor_mmu.c
index d75d575473da4..3e5f20768d545 100644
--- a/drivers/gpu/drm/panthor/panthor_mmu.c
+++ b/drivers/gpu/drm/panthor/panthor_mmu.c
@@ -1692,8 +1692,9 @@ panthor_vm_pool_get_vm(struct panthor_vm_pool *pool, u32 handle)
* Note that VMs can outlive the pool they were created from if other
* objects hold a reference to there VMs.
*/
-void panthor_vm_pool_destroy(struct panthor_file *pfile)
+void panthor_vm_pool_destroy(struct drm_file *file)
{
+ struct panthor_file *pfile = file->driver_priv;
struct panthor_vm *vm;
unsigned long i;
@@ -1715,8 +1716,9 @@ void panthor_vm_pool_destroy(struct panthor_file *pfile)
*
* Return: 0 on success, a negative error code otherwise.
*/
-int panthor_vm_pool_create(struct panthor_file *pfile)
+int panthor_vm_pool_create(struct drm_file *file)
{
+ struct panthor_file *pfile = file->driver_priv;
struct panthor_gem_object *dummy;
int ret;
@@ -1737,7 +1739,7 @@ int panthor_vm_pool_create(struct panthor_file *pfile)
return 0;
err_destroy_vm_pool:
- panthor_vm_pool_destroy(pfile);
+ panthor_vm_pool_destroy(file);
return ret;
}
@@ -2157,8 +2159,9 @@ struct panthor_heap_pool *panthor_vm_get_heap_pool(struct panthor_vm *vm, bool c
* Calculate all heap chunk sizes in all heap pools bound to a VM. If the VM
* is active, record the size as active as well.
*/
-void panthor_vm_heaps_sizes(struct panthor_file *pfile, struct drm_memory_stats *stats)
+void panthor_vm_heaps_sizes(struct drm_file *file, struct drm_memory_stats *stats)
{
+ struct panthor_file *pfile = file->driver_priv;
struct panthor_vm *vm;
unsigned long i;
diff --git a/drivers/gpu/drm/panthor/panthor_mmu.h b/drivers/gpu/drm/panthor/panthor_mmu.h
index abc36e7204be0..e262fe2303f6c 100644
--- a/drivers/gpu/drm/panthor/panthor_mmu.h
+++ b/drivers/gpu/drm/panthor/panthor_mmu.h
@@ -38,7 +38,7 @@ int panthor_vm_as(struct panthor_vm *vm);
struct panthor_heap_pool *
panthor_vm_get_heap_pool(struct panthor_vm *vm, bool create);
-void panthor_vm_heaps_sizes(struct panthor_file *pfile, struct drm_memory_stats *stats);
+void panthor_vm_heaps_sizes(struct drm_file *file, struct drm_memory_stats *stats);
struct panthor_vm *panthor_vm_get(struct panthor_vm *vm);
void panthor_vm_put(struct panthor_vm *vm);
@@ -64,8 +64,8 @@ void panthor_vm_add_job_fence_to_bos_resvs(struct panthor_vm *vm,
struct dma_resv *panthor_vm_resv(struct panthor_vm *vm);
struct drm_gem_object *panthor_vm_root_gem(struct panthor_vm *vm);
-void panthor_vm_pool_destroy(struct panthor_file *pfile);
-int panthor_vm_pool_create(struct panthor_file *pfile);
+void panthor_vm_pool_destroy(struct drm_file *file);
+int panthor_vm_pool_create(struct drm_file *file);
int panthor_vm_pool_create_vm(struct panthor_device *ptdev,
struct panthor_vm_pool *pool,
struct drm_panthor_vm_create *args);
diff --git a/drivers/gpu/drm/panthor/panthor_sched.c b/drivers/gpu/drm/panthor/panthor_sched.c
index 023abaf41890c..3bd6be9407097 100644
--- a/drivers/gpu/drm/panthor/panthor_sched.c
+++ b/drivers/gpu/drm/panthor/panthor_sched.c
@@ -3117,8 +3117,9 @@ void panthor_sched_post_reset(struct panthor_device *ptdev, bool reset_failed)
}
}
-void panthor_fdinfo_gather_group_samples(struct panthor_file *pfile)
+void panthor_fdinfo_gather_group_samples(struct drm_file *file)
{
+ struct panthor_file *pfile = file->driver_priv;
struct panthor_group_pool *gpool = pfile->groups;
struct panthor_group *group;
unsigned long i;
@@ -3654,11 +3655,11 @@ static void add_group_kbo_sizes(struct panthor_device *ptdev,
#define MAX_GROUPS_PER_POOL 128
-int panthor_group_create(struct panthor_file *pfile,
+int panthor_group_create(struct drm_file *file,
const struct drm_panthor_group_create *group_args,
- const struct drm_panthor_queue_create *queue_args,
- u64 drm_client_id)
+ const struct drm_panthor_queue_create *queue_args)
{
+ struct panthor_file *pfile = file->driver_priv;
struct panthor_device *ptdev = pfile->ptdev;
struct panthor_group_pool *gpool = pfile->groups;
struct panthor_scheduler *sched = ptdev->scheduler;
@@ -3756,7 +3757,8 @@ int panthor_group_create(struct panthor_file *pfile,
goto err_put_group;
for (i = 0; i < group_args->queues.count; i++) {
- group->queues[i] = group_create_queue(group, &queue_args[i], drm_client_id, gid, i);
+ group->queues[i] = group_create_queue(group, &queue_args[i],
+ file->client_id, gid, i);
if (IS_ERR(group->queues[i])) {
ret = PTR_ERR(group->queues[i]);
group->queues[i] = NULL;
@@ -3796,8 +3798,9 @@ int panthor_group_create(struct panthor_file *pfile,
return ret;
}
-int panthor_group_destroy(struct panthor_file *pfile, u32 group_handle)
+int panthor_group_destroy(struct drm_file *file, u32 group_handle)
{
+ struct panthor_file *pfile = file->driver_priv;
struct panthor_group_pool *gpool = pfile->groups;
struct panthor_device *ptdev = pfile->ptdev;
struct panthor_scheduler *sched = ptdev->scheduler;
@@ -3848,9 +3851,10 @@ static struct panthor_group *group_from_handle(struct panthor_group_pool *pool,
return group;
}
-int panthor_group_get_state(struct panthor_file *pfile,
+int panthor_group_get_state(struct drm_file *file,
struct drm_panthor_group_get_state *get_state)
{
+ struct panthor_file *pfile = file->driver_priv;
struct panthor_group_pool *gpool = pfile->groups;
struct panthor_device *ptdev = pfile->ptdev;
struct panthor_scheduler *sched = ptdev->scheduler;
@@ -3881,8 +3885,9 @@ int panthor_group_get_state(struct panthor_file *pfile,
return 0;
}
-int panthor_group_pool_create(struct panthor_file *pfile)
+int panthor_group_pool_create(struct drm_file *file)
{
+ struct panthor_file *pfile = file->driver_priv;
struct panthor_group_pool *gpool;
gpool = kzalloc_obj(*gpool);
@@ -3894,8 +3899,9 @@ int panthor_group_pool_create(struct panthor_file *pfile)
return 0;
}
-void panthor_group_pool_destroy(struct panthor_file *pfile)
+void panthor_group_pool_destroy(struct drm_file *file)
{
+ struct panthor_file *pfile = file->driver_priv;
struct panthor_group_pool *gpool = pfile->groups;
struct panthor_group *group;
unsigned long i;
@@ -3904,7 +3910,7 @@ void panthor_group_pool_destroy(struct panthor_file *pfile)
return;
xa_for_each(&gpool->xa, i, group)
- panthor_group_destroy(pfile, i);
+ panthor_group_destroy(file, i);
xa_destroy(&gpool->xa);
kfree(gpool);
@@ -3919,9 +3925,10 @@ void panthor_group_pool_destroy(struct panthor_file *pfile)
*
*/
void
-panthor_fdinfo_gather_group_mem_info(struct panthor_file *pfile,
+panthor_fdinfo_gather_group_mem_info(struct drm_file *file,
struct drm_memory_stats *stats)
{
+ struct panthor_file *pfile = file->driver_priv;
struct panthor_group_pool *gpool = pfile->groups;
struct panthor_group *group;
unsigned long i;
@@ -3984,11 +3991,11 @@ struct panthor_vm *panthor_job_vm(struct drm_sched_job *sched_job)
}
struct drm_sched_job *
-panthor_job_create(struct panthor_file *pfile,
+panthor_job_create(struct drm_file *file,
u16 group_handle,
- const struct drm_panthor_queue_submit *qsubmit,
- u64 drm_client_id)
+ const struct drm_panthor_queue_submit *qsubmit)
{
+ struct panthor_file *pfile = file->driver_priv;
struct panthor_group_pool *gpool = pfile->groups;
struct panthor_job *job;
u32 credits;
@@ -4059,7 +4066,7 @@ panthor_job_create(struct panthor_file *pfile,
ret = drm_sched_job_init(&job->base,
&job->group->queues[job->queue_idx]->entity,
- credits, job->group, drm_client_id);
+ credits, job->group, file->client_id);
if (ret)
goto err_put_job;
diff --git a/drivers/gpu/drm/panthor/panthor_sched.h b/drivers/gpu/drm/panthor/panthor_sched.h
index d87cbdba4d40b..c509fcb5a0c9c 100644
--- a/drivers/gpu/drm/panthor/panthor_sched.h
+++ b/drivers/gpu/drm/panthor/panthor_sched.h
@@ -16,31 +16,28 @@ struct drm_panthor_queue_create;
struct drm_panthor_group_get_state;
struct drm_panthor_queue_submit;
struct panthor_device;
-struct panthor_file;
struct panthor_group_pool;
struct panthor_job;
-int panthor_group_create(struct panthor_file *pfile,
+int panthor_group_create(struct drm_file *file,
const struct drm_panthor_group_create *group_args,
- const struct drm_panthor_queue_create *queue_args,
- u64 drm_client_id);
-int panthor_group_destroy(struct panthor_file *pfile, u32 group_handle);
-int panthor_group_get_state(struct panthor_file *pfile,
+ const struct drm_panthor_queue_create *queue_args);
+int panthor_group_destroy(struct drm_file *file, u32 group_handle);
+int panthor_group_get_state(struct drm_file *file,
struct drm_panthor_group_get_state *get_state);
struct drm_sched_job *
-panthor_job_create(struct panthor_file *pfile,
+panthor_job_create(struct drm_file *file,
u16 group_handle,
- const struct drm_panthor_queue_submit *qsubmit,
- u64 drm_client_id);
+ const struct drm_panthor_queue_submit *qsubmit);
struct drm_sched_job *panthor_job_get(struct drm_sched_job *job);
struct panthor_vm *panthor_job_vm(struct drm_sched_job *sched_job);
void panthor_job_put(struct drm_sched_job *job);
void panthor_job_update_resvs(struct drm_exec *exec, struct drm_sched_job *job);
-int panthor_group_pool_create(struct panthor_file *pfile);
-void panthor_group_pool_destroy(struct panthor_file *pfile);
-void panthor_fdinfo_gather_group_mem_info(struct panthor_file *pfile,
+int panthor_group_pool_create(struct drm_file *file);
+void panthor_group_pool_destroy(struct drm_file *file);
+void panthor_fdinfo_gather_group_mem_info(struct drm_file *pfile,
struct drm_memory_stats *stats);
int panthor_sched_init(struct panthor_device *ptdev);
@@ -54,7 +51,7 @@ void panthor_sched_report_mmu_fault(struct panthor_device *ptdev);
void panthor_sched_prepare_for_vm_destruction(struct panthor_device *ptdev);
void panthor_sched_report_fw_events(struct panthor_device *ptdev, u32 events);
-void panthor_fdinfo_gather_group_samples(struct panthor_file *pfile);
+void panthor_fdinfo_gather_group_samples(struct drm_file *file);
#ifdef CONFIG_DEBUG_FS
void panthor_sched_debugfs_init(struct drm_minor *minor);
--
2.43.0
^ permalink raw reply related [flat|nested] 18+ messages in thread* [PATCH v4 04/12] drm/panthor: Don't allocate protm_suspend_buf
2026-10-12 12:59 [PATCH v4 00/12] drm/panthor: Protected mode support for Mali CSF GPUs Ketil Johnsen
` (2 preceding siblings ...)
2026-10-12 12:59 ` [PATCH v4 03/12] drm/panthor: Pass drm_file instead of panthor_file Ketil Johnsen
@ 2026-10-12 12:59 ` Ketil Johnsen
2026-10-12 12:59 ` [PATCH v4 05/12] drm/panthor: Add support for entering and exiting protected mode Ketil Johnsen
` (7 subsequent siblings)
11 siblings, 0 replies; 18+ messages in thread
From: Ketil Johnsen @ 2026-10-12 12:59 UTC (permalink / raw)
To: Boris Brezillon, Steven Price, Liviu Dudau, Maarten Lankhorst,
Maxime Ripard, Thomas Zimmermann, David Airlie, Simona Vetter,
Daniel Almeida, Alice Ryhl
Cc: dri-devel, linux-kernel, Ketil Johnsen
The PROTM suspend buffer is only needed if the group is going to
use PROTM in the first place, so let's not assume we need one until
we're being asked to.
Reviewed-by: Boris Brezillon <boris.brezillon@collabora.com>
Reviewed-by: Steven Price <steven.price@arm.com>
Reviewed-by: Liviu Dudau <liviu.dudau@arm.com>
Signed-off-by: Ketil Johnsen <ketil.johnsen@arm.com>
---
drivers/gpu/drm/panthor/panthor_sched.c | 14 ++++----------
1 file changed, 4 insertions(+), 10 deletions(-)
diff --git a/drivers/gpu/drm/panthor/panthor_sched.c b/drivers/gpu/drm/panthor/panthor_sched.c
index 3bd6be9407097..db801705f0d27 100644
--- a/drivers/gpu/drm/panthor/panthor_sched.c
+++ b/drivers/gpu/drm/panthor/panthor_sched.c
@@ -3641,9 +3641,11 @@ static void add_group_kbo_sizes(struct panthor_device *ptdev,
if (drm_WARN_ON(&ptdev->base, ptdev != group->ptdev))
return;
- group->fdinfo.kbo_sizes += group->suspend_buf->obj->size;
- group->fdinfo.kbo_sizes += group->protm_suspend_buf->obj->size;
group->fdinfo.kbo_sizes += group->syncobjs->obj->size;
+ group->fdinfo.kbo_sizes += group->suspend_buf->obj->size;
+
+ if (group->protm_suspend_buf)
+ group->fdinfo.kbo_sizes += group->protm_suspend_buf->obj->size;
for (i = 0; i < group->queue_count; i++) {
queue = group->queues[i];
@@ -3724,14 +3726,6 @@ int panthor_group_create(struct drm_file *file,
goto err_put_group;
}
- suspend_size = csg_iface->control->protm_suspend_size;
- group->protm_suspend_buf = panthor_fw_alloc_suspend_buf_mem(ptdev, suspend_size);
- if (IS_ERR(group->protm_suspend_buf)) {
- ret = PTR_ERR(group->protm_suspend_buf);
- group->protm_suspend_buf = NULL;
- goto err_put_group;
- }
-
group->syncobjs = panthor_kernel_bo_create(ptdev, group->vm,
group_args->queues.count *
sizeof(struct panthor_syncobj_64b),
--
2.43.0
^ permalink raw reply related [flat|nested] 18+ messages in thread* [PATCH v4 05/12] drm/panthor: Add support for entering and exiting protected mode
2026-10-12 12:59 [PATCH v4 00/12] drm/panthor: Protected mode support for Mali CSF GPUs Ketil Johnsen
` (3 preceding siblings ...)
2026-10-12 12:59 ` [PATCH v4 04/12] drm/panthor: Don't allocate protm_suspend_buf Ketil Johnsen
@ 2026-10-12 12:59 ` Ketil Johnsen
2026-10-12 13:17 ` sashiko-bot
2026-10-12 12:59 ` [PATCH v4 06/12] drm/panthor: New function for FW section map flags Ketil Johnsen
` (6 subsequent siblings)
11 siblings, 1 reply; 18+ messages in thread
From: Ketil Johnsen @ 2026-10-12 12:59 UTC (permalink / raw)
To: Boris Brezillon, Steven Price, Liviu Dudau, Maarten Lankhorst,
Maxime Ripard, Thomas Zimmermann, David Airlie, Simona Vetter,
Daniel Almeida, Alice Ryhl
Cc: dri-devel, linux-kernel, Florent Tomasin, Paul Toadere,
Samuel Percival, Ketil Johnsen
From: Florent Tomasin <florent.tomasin@arm.com>
This patch modifies the Panthor driver code to allow handling
of the GPU HW protected mode enter and exit.
The logic added by this patch includes:
- the mechanisms needed for entering and exiting protected mode.
- the handling of protected mode IRQs and FW interactions.
- the scheduler changes needed to decide when to enter
protected mode based on CSG scheduling.
- GPU fault handling during protected mode execution.
Note that the submission of protected mode jobs are done
from the user space.
The following is a summary of how protected mode is entered
and exited:
- When the GPU detects a protected mode job needs to be
executed, an IRQ is sent to the CPU to notify the kernel
driver that the job is blocked until the GPU has entered
protected mode. The entering of protected mode is controlled
by the kernel driver.
- The Mali Panthor CSF driver will schedule a tick and evaluate
which CS in the CSG to schedule on slot needs protected mode.
If the priority of the CSG is not sufficiently high, the
protected mode job will not progress until the CSG is
scheduled at top priority.
- The Panthor scheduler notifies the GPU that the blocked
protected jobs will soon be able to progress.
- Once all CSG and CS slots are updated, the scheduler
requests the GPU to enter protected mode and waits for
it to be acknowledged.
- If successful, all protected mode jobs will resume execution
while normal mode jobs block until the GPU exits
protected mode, or the kernel driver rotates the CSGs
and forces the GPU to exit protected mode.
- If unsuccessful, the scheduler will request a GPU reset.
- Faults during protected mode are reported GPU wide, and not as
CSG/CS errors. We allow only one CSG to run in protected mode at a
time so we know which CSG to blame for the fault.
- All faults during protected mode are handled with a GPU reset.
This sequence will continue so long the user space is
submitting protected mode jobs.
Signed-off-by: Florent Tomasin <florent.tomasin@arm.com>
Co-developed-by: Paul Toadere <paul.toadere@arm.com>
Signed-off-by: Paul Toadere <paul.toadere@arm.com>
Co-developed-by: Samuel Percival <samuel.percival@arm.com>
Signed-off-by: Samuel Percival <samuel.percival@arm.com>
Co-developed-by: Ketil Johnsen <ketil.johnsen@arm.com>
Signed-off-by: Ketil Johnsen <ketil.johnsen@arm.com>
---
v4:
- Do not pick any more groups if first group requests protm.
Prevous versions allowed lower pri groups to stay on slot.
- Moved the rwsem for protm sync into scheduler.
- Added protm exit handling to start of scheduler tick.
- Scheduler will notify MMU code when it enters and exits protm.
- When in protected mode, MMU code will do "delayed updates" of VMs
which are not "running". This avoids unnecessary preemptions and
blocking during unrelated VM_BIND ops.
- Page table updates to FW VM and currently running protm VM can
not proceed before protm is exited (preempted).
- Ensure ticking during protm (so we can evaluate preemption).
- Added special protm preempt code with preempt_protm_group().
- Poll for protm enter and exit without FW event wait queue.
Protm enter simply use read_poll_timeout().
Protm exit use a custom loop as this needs to be interruptible (long waits).
- Reversed check order in panthor_gpu_protm_fault_pending()
- Do not print error message in panthor_fw_protm_exit(), leave that to caller
since it can some times be expected.
- Reintroduce disabling of GPU_IRQ_PROTM_FAULT while handling them.
- Reset protm_fault and re-enable GPU_IRQ_PROTM_FAULT after FW have re-started.
- Tweaks to tick_ctx_handle_protm_group(), take events_lock and ack everything!
- Use "protm block" with DEFINE_GUARD helpers (MMU code).
- Minor spelling/wording and naming changes.
v3:
- Rebase
- Required changes due to new IRQ handling (events_lock).
- Required changes due to memory reclaim (more places to sync with protm).
- Count number of enter and exits to and from protected mode.
- Added helper function wait_protm_enter() to check protm enter condition, using
the counters mentioned above.
- Tweaks to timeouts.
- Removed disable/enable of GPU_IRQ_PROTM_FAULT. No longer recall what issue it
was supposed to fix.
- protm_fault changed to atomic.
- Moved sync with protm out from panthor_vm_lock_region() and
panthor_vm_unlock_region().
- Avoids special dealing with as.slots_lock and GPU reset.
- Block and unblock of protm in same function, so easier to see.
- We no longer need to add panthor_vm_expand_locked_region() (different patch).
- The downside is that we might block protm mode more than necessary.
- Added panthor_sched_protm_try_block() used by memory reclaim case.
- protm_pending_queues made atomic (following the same change for fatal_queues).
- Only clear bits from protm_pending_queues when we ACK CS_PROTM_PENDING.
- Clear any pending CS_PROTM_PENDING on slot reset.
- Ack all CS_PROTM_PENDING in tick_ctx_handle_protm_group(), no matter the value
of protm_pending_queues.
- Inlined helper function panthor_sched_protm_enter().
v2:
- Heavily reworked, although conceptually similar to v1.
---
drivers/gpu/drm/panthor/panthor_device.h | 8 +
drivers/gpu/drm/panthor/panthor_fw.c | 94 ++++-
drivers/gpu/drm/panthor/panthor_fw.h | 4 +
drivers/gpu/drm/panthor/panthor_gpu.c | 49 ++-
drivers/gpu/drm/panthor/panthor_gpu.h | 4 +
drivers/gpu/drm/panthor/panthor_mmu.c | 283 +++++++++-----
drivers/gpu/drm/panthor/panthor_mmu.h | 6 +
drivers/gpu/drm/panthor/panthor_sched.c | 467 +++++++++++++++++++++--
drivers/gpu/drm/panthor/panthor_sched.h | 5 +
9 files changed, 806 insertions(+), 114 deletions(-)
diff --git a/drivers/gpu/drm/panthor/panthor_device.h b/drivers/gpu/drm/panthor/panthor_device.h
index b55a3f9edd414..4362c55bcd7e4 100644
--- a/drivers/gpu/drm/panthor/panthor_device.h
+++ b/drivers/gpu/drm/panthor/panthor_device.h
@@ -336,6 +336,14 @@ struct panthor_device {
struct list_head node;
} gems;
#endif
+ /** @protm: Protected mode related data. */
+ struct {
+ /** @protm_enter_count: Number of times we entered protm. */
+ atomic64_t protm_enter_count;
+
+ /** @protm_exit_count: Number of times we exited protm. */
+ atomic64_t protm_exit_count;
+ } protm;
};
struct panthor_gpu_usage {
diff --git a/drivers/gpu/drm/panthor/panthor_fw.c b/drivers/gpu/drm/panthor/panthor_fw.c
index 5f9f7a92c56a8..e639ffeac7a32 100644
--- a/drivers/gpu/drm/panthor/panthor_fw.c
+++ b/drivers/gpu/drm/panthor/panthor_fw.c
@@ -1067,7 +1067,9 @@ static void panthor_fw_init_global_iface(struct panthor_device *ptdev)
GLB_CFG_PROGRESS_TIMER |
GLB_CFG_POWEROFF_TIMER |
GLB_IDLE_EN |
- GLB_IDLE;
+ GLB_IDLE |
+ GLB_PROTM_ENTER |
+ GLB_PROTM_EXIT;
if (panthor_fw_has_glb_state(ptdev))
glb_iface->input->ack_irq_mask |= GLB_STATE_MASK;
@@ -1281,6 +1283,9 @@ int panthor_fw_post_reset(struct panthor_device *ptdev)
return ret;
}
+ atomic64_set(&ptdev->protm.protm_enter_count, 0);
+ atomic64_set(&ptdev->protm.protm_exit_count, 0);
+
/* We must re-initialize the global interface even on fast-reset. */
panthor_fw_init_global_iface(ptdev);
return 0;
@@ -1476,6 +1481,93 @@ static void panthor_fw_ping_work(struct work_struct *work)
}
}
+static bool wait_protm_enter(struct panthor_device *ptdev,
+ s64 protm_session_seqno)
+{
+ return (panthor_gpu_status(ptdev) & GPU_STATUS_PROTM_ACTIVE) ||
+ (atomic64_read(&ptdev->protm.protm_exit_count) >=
+ protm_session_seqno);
+}
+
+int panthor_fw_protm_enter(struct panthor_device *ptdev)
+{
+ struct panthor_fw_global_iface *glb_iface =
+ panthor_fw_get_glb_iface(ptdev);
+ u32 acked;
+ int ret;
+ long long enter_count;
+ bool status;
+
+ /* Restart the watchdog timer, so it doesn't hit immediately
+ * after entering protected mode, since this will cause GPU
+ * to exit protected mode to respond to the ping request.
+ */
+ mod_delayed_work(ptdev->reset.wq, &ptdev->fw->watchdog.ping_work,
+ msecs_to_jiffies(PING_INTERVAL_MS));
+
+ panthor_fw_toggle_reqs(glb_iface, req, ack, GLB_PROTM_ENTER);
+ panthor_fw_ring_doorbell(ptdev, CSF_GLB_DOORBELL_ID);
+
+ ret = panthor_fw_glb_wait_acks(ptdev, GLB_PROTM_ENTER, &acked, 250);
+ if (ret) {
+ drm_err(&ptdev->base,
+ "Wait for FW protected mode acknowledge timed out");
+ return ret;
+ }
+
+ enter_count = atomic64_inc_return(&ptdev->protm.protm_enter_count);
+
+ /* Poll for the entry of protected mode.
+ * It is possible that GPU_STATUS_PROTM_ACTIVE is set and cleared
+ * before we check it below, so we must also check for GLB_PROTM_EXIT.
+ * GLB_PROTM_EXIT can not be checked directly, because this could also
+ * be handled and clear before we check below. We count number of
+ * protm enters and exits to safely handle that case.
+ */
+ ret = read_poll_timeout(wait_protm_enter, status, status, 100, 500000,
+ false, ptdev, enter_count);
+
+ if (ret) {
+ drm_err(&ptdev->base,
+ "Wait for GPU protected mode enter timed out");
+ return -ETIMEDOUT;
+ }
+
+ return 0;
+}
+
+int panthor_fw_protm_exit_wait(struct panthor_device *ptdev, u32 timeout_ms)
+{
+ unsigned long deadline = jiffies + msecs_to_jiffies(timeout_ms);
+
+ do {
+ if (!(panthor_gpu_status(ptdev) & GPU_STATUS_PROTM_ACTIVE))
+ return 0;
+
+ if (signal_pending(current))
+ return -ERESTARTSYS;
+
+ usleep_range_state(100, 200, TASK_INTERRUPTIBLE);
+ } while (!time_after_eq(jiffies, deadline));
+
+ if (!(panthor_gpu_status(ptdev) & GPU_STATUS_PROTM_ACTIVE))
+ return 0;
+
+ return -ETIMEDOUT;
+}
+
+int panthor_fw_protm_exit(struct panthor_device *ptdev, u32 timeout_ms)
+{
+ struct panthor_fw_global_iface *glb_iface =
+ panthor_fw_get_glb_iface(ptdev);
+
+ /* Send PING request to force an exit */
+ panthor_fw_toggle_reqs(glb_iface, req, ack, GLB_PING);
+ panthor_fw_ring_doorbell(ptdev, CSF_GLB_DOORBELL_ID);
+
+ return panthor_fw_protm_exit_wait(ptdev, timeout_ms);
+}
+
/**
* panthor_fw_init() - Initialize FW related data.
* @ptdev: Device.
diff --git a/drivers/gpu/drm/panthor/panthor_fw.h b/drivers/gpu/drm/panthor/panthor_fw.h
index a99a9b6f4825c..4eda8f8e714c1 100644
--- a/drivers/gpu/drm/panthor/panthor_fw.h
+++ b/drivers/gpu/drm/panthor/panthor_fw.h
@@ -529,4 +529,8 @@ static inline int panthor_fw_resume(struct panthor_device *ptdev)
int panthor_fw_init(struct panthor_device *ptdev);
void panthor_fw_unplug(struct panthor_device *ptdev);
+int panthor_fw_protm_enter(struct panthor_device *ptdev);
+int panthor_fw_protm_exit(struct panthor_device *ptdev, u32 timeout_ms);
+int panthor_fw_protm_exit_wait(struct panthor_device *ptdev, u32 timeout_ms);
+
#endif
diff --git a/drivers/gpu/drm/panthor/panthor_gpu.c b/drivers/gpu/drm/panthor/panthor_gpu.c
index a383b04f101ed..319282a22f669 100644
--- a/drivers/gpu/drm/panthor/panthor_gpu.c
+++ b/drivers/gpu/drm/panthor/panthor_gpu.c
@@ -46,6 +46,9 @@ struct panthor_gpu {
/** @cache_flush_lock: Lock to serialize cache flushes */
struct mutex cache_flush_lock;
+
+ /** @protm_fault: True if a GPU_IRQ_PROTM_FAULT has been raised */
+ atomic_t protm_fault;
};
#define GPU_INTERRUPTS_MASK \
@@ -91,6 +94,35 @@ static void panthor_gpu_irq_handler(struct panthor_irq *pirq, u32 status)
struct panthor_device *ptdev = pirq->ptdev;
struct panthor_gpu *gpu = ptdev->gpu;
+ if (status & GPU_IRQ_PROTM_FAULT) {
+ /* Make a note of this fault before we clear the interrupt.
+ * This ensures panthor_gpu_protm_fault_pending() can always
+ * give an accurate answer.
+ *
+ * There is a race we need to handle between two interrupts,
+ * this GPU_IRQ_PROTM_FAULT and JOB_INT_GLOBAL_IF with the
+ * GLB_PROTM_EXIT event.
+ *
+ * Although GPU_IRQ_PROTM_FAULT is always raised first,
+ * processing of GLB_PROTM_EXIT could still execute first.
+ * The handling of GLB_PROTM_EXIT MUST know if a
+ * GPU_IRQ_PROTM_FAULT has been raised or not, otherwise it
+ * could incorrectly think everything is fine and resume
+ * with normal scheduling to early.
+ *
+ * We still need to do fault handling (reset) here as well,
+ * because some failures during protected mode do not
+ * automatically exit protected mode (no GLB_PROTM_EXIT).
+ * This means there is a slim chance we do two GPU resets
+ * instead of just one. This is not ideal, but should be safe.
+ */
+ atomic_set(&gpu->protm_fault, 1);
+ panthor_irq_disable_events(&ptdev->gpu->irq,
+ GPU_IRQ_PROTM_FAULT);
+ drm_warn(&ptdev->base, "GPU Fault in protected mode");
+ panthor_device_schedule_reset(ptdev);
+ }
+
gpu_write(gpu->irq.iomem, INT_CLEAR, status);
if (tracepoint_enabled(gpu_power_status) && (status & GPU_POWER_INTERRUPTS_MASK))
@@ -107,8 +139,6 @@ static void panthor_gpu_irq_handler(struct panthor_irq *pirq, u32 status)
fault_status, panthor_exception_name(ptdev, fault_status & 0xFF),
address);
}
- if (status & GPU_IRQ_PROTM_FAULT)
- drm_warn(&ptdev->base, "GPU Fault in protected mode\n");
spin_lock(&ptdev->gpu->reqs_lock);
if (status & ptdev->gpu->pending_reqs) {
@@ -123,6 +153,13 @@ static irqreturn_t panthor_gpu_irq_threaded_handler(int irq, void *data)
return panthor_irq_default_threaded_handler(data, panthor_gpu_irq_handler);
}
+bool panthor_gpu_protm_fault_pending(struct panthor_device *ptdev)
+{
+ return gpu_read(ptdev->gpu->irq.iomem, INT_RAWSTAT) &
+ GPU_IRQ_PROTM_FAULT ||
+ atomic_read(&ptdev->gpu->protm_fault);
+}
+
/**
* panthor_gpu_unplug() - Called when the GPU is unplugged.
* @ptdev: Device to unplug.
@@ -430,6 +467,9 @@ int panthor_gpu_soft_reset(struct panthor_device *ptdev)
return -ETIMEDOUT;
}
+ atomic_set(&ptdev->gpu->protm_fault, 0);
+ panthor_irq_enable_events(&ptdev->gpu->irq, GPU_IRQ_PROTM_FAULT);
+
scoped_guard(spinlock, &ptdev->gpu->reqs_lock) {
ptdev->gpu->pending_reqs = 0;
}
@@ -508,3 +548,8 @@ int panthor_gpu_coherency_init(struct panthor_device *ptdev)
drm_err(&ptdev->base, "Coherency not supported by the device");
return -ENOTSUPP;
}
+
+u32 panthor_gpu_status(struct panthor_device *ptdev)
+{
+ return gpu_read(ptdev->gpu->iomem, GPU_STATUS);
+}
diff --git a/drivers/gpu/drm/panthor/panthor_gpu.h b/drivers/gpu/drm/panthor/panthor_gpu.h
index f615feb056094..00ec9a8c5d7fd 100644
--- a/drivers/gpu/drm/panthor/panthor_gpu.h
+++ b/drivers/gpu/drm/panthor/panthor_gpu.h
@@ -60,4 +60,8 @@ u64 panthor_gpu_get_cycle_count(struct panthor_device *ptdev);
int panthor_gpu_coherency_init(struct panthor_device *ptdev);
+u32 panthor_gpu_status(struct panthor_device *ptdev);
+
+bool panthor_gpu_protm_fault_pending(struct panthor_device *ptdev);
+
#endif
diff --git a/drivers/gpu/drm/panthor/panthor_mmu.c b/drivers/gpu/drm/panthor/panthor_mmu.c
index 3e5f20768d545..38e9986416dd9 100644
--- a/drivers/gpu/drm/panthor/panthor_mmu.c
+++ b/drivers/gpu/drm/panthor/panthor_mmu.c
@@ -77,6 +77,12 @@ struct panthor_mmu {
/** @as.faulty_mask: Bitmask encoding the faulty slots. */
unsigned long faulty_mask;
+ /** @as.delay_update_mask: Bitmask encoding slots which must use delayed updates */
+ u32 delay_update_mask;
+
+ /** @as.delayed_update_mask: Bitmask encoding slots with pending delayed updates */
+ u32 delayed_update_mask;
+
/** @as.slots: VMs currently bound to the AS slots. */
struct panthor_as_slot slots[MAX_AS_SLOTS];
@@ -450,6 +456,26 @@ struct panthor_vm_bind_job {
*/
static struct kmem_cache *pt_cache;
+static bool protm_block_timeout(struct panthor_vm *vm)
+{
+ return panthor_sched_protm_block_timeout(vm->ptdev, vm);
+}
+
+static bool protm_try_block(struct panthor_vm *vm)
+{
+ return panthor_sched_protm_try_block(vm->ptdev);
+}
+
+static void protm_unblock(struct panthor_vm *vm)
+{
+ panthor_sched_protm_unblock(vm->ptdev);
+}
+
+/* Only cond variants can be used, hence the use of WARN_ON() here */
+DEFINE_GUARD(protm_block, struct panthor_vm*, WARN_ON(_T), protm_unblock(_T));
+DEFINE_GUARD_COND(protm_block, _timeout, protm_block_timeout(_T));
+DEFINE_GUARD_COND(protm_block, _try, protm_try_block(_T));
+
/**
* alloc_pt() - Custom page table allocator
* @cookie: Cookie passed at page table allocation time.
@@ -1809,12 +1835,17 @@ static int panthor_vm_lock_region(struct panthor_vm *vm, u64 start, u64 size)
mutex_lock(&ptdev->mmu->as.slots_lock);
if (vm->as.id >= 0 && size) {
- /* Lock the region that needs to be updated */
- gpu_write64(ptdev->mmu->iomem, AS_LOCKADDR(vm->as.id),
- pack_region_range(ptdev, &start, &size));
+ if (ptdev->mmu->as.delay_update_mask & BIT(vm->as.id)) {
+ ptdev->mmu->as.delayed_update_mask |= BIT(vm->as.id);
+ } else {
+ /* Lock the region that needs to be updated */
+ gpu_write64(ptdev->mmu->iomem, AS_LOCKADDR(vm->as.id),
+ pack_region_range(ptdev, &start, &size));
- /* If the lock succeeded, update the locked_region info. */
- ret = as_send_cmd_and_wait(ptdev, vm->as.id, AS_COMMAND_LOCK);
+ /* If the lock succeeded, update the locked_region info. */
+ ret = as_send_cmd_and_wait(ptdev, vm->as.id,
+ AS_COMMAND_LOCK);
+ }
}
if (!ret) {
@@ -1832,25 +1863,31 @@ static void panthor_vm_unlock_region(struct panthor_vm *vm)
mutex_lock(&ptdev->mmu->as.slots_lock);
if (vm->as.id >= 0) {
- int ret;
+ if (ptdev->mmu->as.delay_update_mask & BIT(vm->as.id)) {
+ ptdev->mmu->as.delayed_update_mask |= BIT(vm->as.id);
+ } else {
+ int ret;
- /* flush+invalidate RW caches and invalidate RO ones.
- * TODO: See if we can use FLUSH_PA_RANGE when the physical
- * range is narrow enough and the HW supports it.
- */
- ret = panthor_gpu_flush_caches(ptdev, CACHE_CLEAN | CACHE_INV,
- CACHE_CLEAN | CACHE_INV,
- CACHE_INV);
+ /* flush+invalidate RW caches and invalidate RO ones.
+ * TODO: See if we can use FLUSH_PA_RANGE when the physical
+ * range is narrow enough and the HW supports it.
+ */
+ ret = panthor_gpu_flush_caches(ptdev,
+ CACHE_CLEAN | CACHE_INV,
+ CACHE_CLEAN | CACHE_INV,
+ CACHE_INV);
- /* Unlock the region if the flush is effective. */
- if (!ret)
- ret = as_send_cmd_and_wait(ptdev, vm->as.id, AS_COMMAND_UNLOCK);
+ /* Unlock the region if the flush is effective. */
+ if (!ret)
+ ret = as_send_cmd_and_wait(ptdev, vm->as.id,
+ AS_COMMAND_UNLOCK);
- /* If we fail to flush or unlock the region, schedule a GPU reset
- * to unblock the situation.
- */
- if (ret)
- panthor_device_schedule_reset(ptdev);
+ /* If we fail to flush or unlock the region, schedule a
+ * GPU reset to unblock the situation.
+ */
+ if (ret)
+ panthor_device_schedule_reset(ptdev);
+ }
}
vm->locked_region.start = 0;
vm->locked_region.size = 0;
@@ -2014,6 +2051,8 @@ void panthor_mmu_post_reset(struct panthor_device *ptdev)
*/
ptdev->mmu->as.alloc_mask = 0;
ptdev->mmu->as.faulty_mask = 0;
+ ptdev->mmu->as.delay_update_mask = 0;
+ ptdev->mmu->as.delayed_update_mask = 0;
for (u32 i = 0; i < ARRAY_SIZE(ptdev->mmu->as.slots); i++) {
struct panthor_vm *vm = ptdev->mmu->as.slots[i].vm;
@@ -2493,6 +2532,52 @@ void panthor_vm_update_bo_reclaim_lru_locked(struct panthor_gem_object *bo)
}
}
+static int evict_vm_bo_locked(struct drm_gpuvm_bo *vm_bo)
+{
+ struct drm_gpuva *va;
+
+ /* It can be that the vm_bo was already evicted but a new
+ * mapping pointing to this BO got created in the meantime,
+ * thus turning the vm_bo in partially evicted state. In that case
+ * we don't call drm_gpuvm_bo_evict() again because this would
+ * mess up with the internal gpuvm lists, but we do walk the
+ * VAs on this vm_bo to make sure the non-evicted ones are
+ * torn down.
+ */
+ if (!vm_bo->evicted)
+ drm_gpuvm_bo_evict(vm_bo, true);
+
+ drm_gpuvm_bo_for_each_va(va, vm_bo) {
+ struct panthor_vm *vm =
+ container_of(vm_bo->vm, struct panthor_vm, base);
+ struct panthor_vma *vma =
+ container_of(va, struct panthor_vma, base);
+ int ret;
+
+ if (vma->evicted)
+ continue;
+
+ /* If something fail in the middle of a VM_BO eviction, the
+ * VM_BO is considered fully evicted, but some of its VMAs might
+ * still be active. That's okay because the pages won't be
+ * released if this function returns an error.
+ *
+ * On the next job targeting this VM, the partially evicted
+ * VM_BO will be validated, causing all its evicted VMAs to be
+ * repopulated before the job runs. So no GPU fault expected.
+ */
+ ret = panthor_vm_lock_region(vm, va->va.addr, va->va.range);
+ if (ret)
+ return ret;
+
+ panthor_vm_unmap_pages(vm, va->va.addr, va->va.range);
+ panthor_vm_unlock_region(vm);
+ vma->evicted = true;
+ }
+
+ return 0;
+}
+
int panthor_vm_evict_bo_mappings_locked(struct panthor_gem_object *bo)
{
struct drm_gpuvm_bo *vm_bo;
@@ -2500,48 +2585,13 @@ int panthor_vm_evict_bo_mappings_locked(struct panthor_gem_object *bo)
drm_gem_for_each_gpuvm_bo(vm_bo, &bo->base) {
struct panthor_vm *vm = container_of(vm_bo->vm, struct panthor_vm, base);
- struct drm_gpuva *va;
-
- if (!mutex_trylock(&vm->op_lock))
- return -EDEADLK;
-
- /* It can be that the vm_bo was already evicted but a new
- * mapping pointing to this BO got created in the meantime,
- * thus turning the vm_bo in partially evicted state. In that case
- * we don't call drm_gpuvm_bo_evict() again because this would
- * mess up with the internal gpuvm lists, but we do walk the
- * VAs on this vm_bo to make sure the non-evicted ones are
- * torn down.
- */
- if (!vm_bo->evicted)
- drm_gpuvm_bo_evict(vm_bo, true);
-
- drm_gpuvm_bo_for_each_va(va, vm_bo) {
- struct panthor_vma *vma = container_of(va, struct panthor_vma, base);
-
- if (vma->evicted)
- continue;
-
- /* If something fail in the middle of a VM_BO eviction, the VM_BO
- * is considered fully evicted, but some of its VMAs might still be
- * active. That's okay because the pages won't be released if this
- * function returns an error.
- *
- * On the next job targeting this VM, the partially evicted VM_BO
- * will be validated, causing all its evicted VMAs to be repopulated
- * before the job runs. So no GPU fault expected.
- */
- ret = panthor_vm_lock_region(vm, va->va.addr, va->va.range);
- if (ret)
- break;
- panthor_vm_unmap_pages(vm, va->va.addr, va->va.range);
- panthor_vm_unlock_region(vm);
- vma->evicted = true;
+ scoped_cond_guard(mutex_try, ret = -EDEADLK, &vm->op_lock) {
+ scoped_cond_guard(protm_block_try, ret = -EDEADLK, vm) {
+ ret = evict_vm_bo_locked(vm_bo);
+ }
}
- mutex_unlock(&vm->op_lock);
-
if (ret)
break;
}
@@ -2611,8 +2661,12 @@ static int remap_evicted_vma(struct drm_gpuvm_bo *vm_bo,
break;
}
- if (found) {
+ if (!found)
+ goto out_unlock;
+
+ scoped_cond_guard(protm_block_timeout, ret = -ETIMEDOUT, vm) {
vm->op_ctx = op_ctx;
+
ret = panthor_vm_lock_region(vm, evicted_vma->base.va.addr,
evicted_vma->base.va.range);
if (!ret) {
@@ -2636,6 +2690,7 @@ static int remap_evicted_vma(struct drm_gpuvm_bo *vm_bo,
vm->op_ctx = NULL;
}
+out_unlock:
mutex_unlock(&vm->op_lock);
out_cleanup:
@@ -2726,39 +2781,42 @@ panthor_vm_exec_op(struct panthor_vm *vm, struct panthor_vm_op_ctx *op,
mutex_lock(&vm->op_lock);
vm->op_ctx = op;
- ret = panthor_vm_lock_region(vm, op->va.addr, op->va.range);
- if (ret)
- goto out;
-
- switch (op_type) {
- case DRM_PANTHOR_VM_BIND_OP_TYPE_MAP: {
- const struct drm_gpuvm_map_req map_req = {
- .map.va.addr = op->va.addr,
- .map.va.range = op->va.range,
- .map.gem.obj = op->map.vm_bo->obj,
- .map.gem.offset = op->map.bo_offset,
- };
-
- if (vm->unusable) {
- ret = -EINVAL;
+ scoped_cond_guard(protm_block_timeout, ret = -ETIMEDOUT, vm) {
+ ret = panthor_vm_lock_region(vm, op->va.addr, op->va.range);
+ if (ret)
+ goto out;
+
+ switch (op_type) {
+ case DRM_PANTHOR_VM_BIND_OP_TYPE_MAP: {
+ const struct drm_gpuvm_map_req map_req = {
+ .map.va.addr = op->va.addr,
+ .map.va.range = op->va.range,
+ .map.gem.obj = op->map.vm_bo->obj,
+ .map.gem.offset = op->map.bo_offset,
+ };
+
+ if (vm->unusable) {
+ ret = -EINVAL;
+ break;
+ }
+
+ ret = drm_gpuvm_sm_map(&vm->base, vm, &map_req);
break;
}
- ret = drm_gpuvm_sm_map(&vm->base, vm, &map_req);
- break;
- }
+ case DRM_PANTHOR_VM_BIND_OP_TYPE_UNMAP:
+ ret = drm_gpuvm_sm_unmap(&vm->base, vm, op->va.addr,
+ op->va.range);
+ break;
- case DRM_PANTHOR_VM_BIND_OP_TYPE_UNMAP:
- ret = drm_gpuvm_sm_unmap(&vm->base, vm, op->va.addr, op->va.range);
- break;
+ default:
+ ret = -EINVAL;
+ break;
+ }
- default:
- ret = -EINVAL;
- break;
+ panthor_vm_unlock_region(vm);
}
- panthor_vm_unlock_region(vm);
-
out:
if (ret && flag_vm_unusable_on_failure)
panthor_vm_declare_unusable(vm);
@@ -3517,3 +3575,54 @@ void panthor_mmu_pt_cache_fini(void)
{
kmem_cache_destroy(pt_cache);
}
+
+bool panthor_vm_for_mcu(struct panthor_vm *vm)
+{
+ return vm->for_mcu;
+}
+
+void panthor_mmu_protm_enter_event(struct panthor_device *ptdev,
+ struct panthor_vm *vm)
+{
+ /* Scheduler is entering protm mode with specified VM (AS).
+ * Scheduler now promises it will not run any work using any other VM
+ * than the provided vm and the FW VM until further notice.
+ * This means we can do page table updates for all the other VMs, even
+ * in protected mode, as long as we do the necessary cache operations
+ * before normal scheduling is resumed. This is done on protm exit,
+ * see panthor_mmu_protm_exit_event().
+ */
+ if (drm_WARN_ON(&ptdev->base, !vm))
+ return;
+
+ if (drm_WARN_ON(&ptdev->base, vm->as.id < 0))
+ return;
+
+ guard(mutex)(&ptdev->mmu->as.slots_lock);
+
+ drm_WARN_ON(&ptdev->base, ptdev->mmu->as.delay_update_mask);
+
+ ptdev->mmu->as.delay_update_mask = ~(BIT(0) | BIT(vm->as.id));
+}
+
+void panthor_mmu_protm_exit_event(struct panthor_device *ptdev)
+{
+ /* We left protected mode and scheduler is about to resume normal
+ * scheduling. Do the necessary cache operation so any page table
+ * updates we might have done during protm is correctly visible
+ * to the GPU.
+ */
+ int ret;
+
+ guard(mutex)(&ptdev->mmu->as.slots_lock);
+
+ ptdev->mmu->as.delay_update_mask = 0;
+
+ if (!ptdev->mmu->as.delayed_update_mask)
+ return;
+
+ ret = panthor_gpu_flush_caches(ptdev, CACHE_CLEAN | CACHE_INV,
+ CACHE_CLEAN | CACHE_INV, CACHE_INV);
+ if (ret)
+ panthor_device_schedule_reset(ptdev);
+}
diff --git a/drivers/gpu/drm/panthor/panthor_mmu.h b/drivers/gpu/drm/panthor/panthor_mmu.h
index e262fe2303f6c..9f9d690953ead 100644
--- a/drivers/gpu/drm/panthor/panthor_mmu.h
+++ b/drivers/gpu/drm/panthor/panthor_mmu.h
@@ -106,6 +106,12 @@ void panthor_vm_update_resvs(struct panthor_vm *vm, struct drm_exec *exec,
int panthor_mmu_pt_cache_init(void);
void panthor_mmu_pt_cache_fini(void);
+bool panthor_vm_for_mcu(struct panthor_vm *vm);
+
+void panthor_mmu_protm_enter_event(struct panthor_device *ptdev,
+ struct panthor_vm *vm);
+void panthor_mmu_protm_exit_event(struct panthor_device *ptdev);
+
#ifdef CONFIG_DEBUG_FS
void panthor_mmu_debugfs_init(struct drm_minor *minor);
#endif
diff --git a/drivers/gpu/drm/panthor/panthor_sched.c b/drivers/gpu/drm/panthor/panthor_sched.c
index db801705f0d27..fff805fd794a5 100644
--- a/drivers/gpu/drm/panthor/panthor_sched.c
+++ b/drivers/gpu/drm/panthor/panthor_sched.c
@@ -307,6 +307,50 @@ struct panthor_scheduler {
*/
struct list_head stopped_groups;
} reset;
+
+ /** @protm: Protected mode related fields. */
+ struct {
+ /**
+ * @lock: Lock for the protected mode state (active_group)
+ *
+ * This lock is used to manage current protected mode state,
+ * as represented by the member active_group.
+ *
+ * Knowing this state is critical for MMU operations, because:
+ * - The MMU will silently ignore commands issued when the GPU
+ * is in protected mode.
+ * - There can not be any pending MMU LOCKS in place when
+ * entering protected mode.
+ *
+ * Code which interacts with the MMU, typically by calling
+ * panthor_vm_lock_region(), should therefore ensure the
+ * scheduler is not in and will not enter protected mode first.
+ * This is done by calling either
+ * - panthor_sched_protm_block_timeout(), or
+ * - panthor_sched_protm_try_block()
+ *
+ * Once the MMU operations have completed, call
+ * panthor_sched_protm_unblock() to tell the scheduler that
+ * it is safe to enter protected mode again.
+ */
+ struct rw_semaphore lock;
+
+ /**
+ * @active_group: The active protected group.
+ *
+ * We only allow one protected group to run at the same time,
+ * as it makes it easier to handle faults in protected mode.
+ *
+ * If the active_group member is set, then the scheduler
+ * believes the GPU is in protected mode. If not set, then the
+ * GPU is for sure not in normal mode.
+
+ * Changes are done with both panthor_scheduler::lock and
+ * panthor_scheduler::protm::lock held as writer.
+ * This means you only need to hold one of these locks for read.
+ */
+ struct panthor_group *active_group;
+ } protm;
};
/**
@@ -571,6 +615,16 @@ struct panthor_group {
*/
atomic_t fatal_queues;
+ /**
+ * @protm_pending_queues: Bitmask reflecting the queues that have raised
+ * a CS_PROTM_PENDING.
+ *
+ * The GPU will set the bit associated to the queue pending protected
+ * mode when a PROT_REGION command is executing or when trying to resume
+ * previously suspended protected mode jobs.
+ */
+ atomic_t protm_pending_queues;
+
/** @tiler_oom: Mask of queues that have a tiler OOM event to process. */
atomic_t tiler_oom;
@@ -1154,8 +1208,9 @@ cs_slot_reset_locked(struct panthor_device *ptdev, u32 csg_id, u32 cs_id)
lockdep_assert_held(&ptdev->scheduler->lock);
panthor_fw_update_reqs(cs_iface, req,
- CS_STATE_STOP,
- CS_STATE_MASK);
+ CS_STATE_STOP | (cs_iface->output->ack &
+ CS_PROTM_PENDING),
+ CS_STATE_MASK | CS_PROTM_PENDING);
queue_suspend_timeout(queue);
@@ -1394,6 +1449,27 @@ csg_slot_prog_locked(struct panthor_device *ptdev, u32 csg_id, u32 priority)
return 0;
}
+static void
+cs_slot_process_protm_pending_event_locked(struct panthor_device *ptdev,
+ u32 csg_id, u32 cs_id)
+{
+ struct panthor_scheduler *sched = ptdev->scheduler;
+ struct panthor_csg_slot *csg_slot = &sched->csg_slots[csg_id];
+ struct panthor_group *group = csg_slot->group;
+
+ lockdep_assert_held(&sched->events_lock);
+
+ if (!group)
+ return;
+
+ /* Do not allow user space work to switch into protected mode, as we
+ * do not fully support this quite yet.
+ */
+ atomic_or(BIT(cs_id), &group->fatal_queues);
+
+ sched_queue_delayed_work(sched, tick, 0);
+}
+
static void
cs_slot_process_fatal_event_locked(struct panthor_device *ptdev,
u32 csg_id, u32 cs_id)
@@ -1642,6 +1718,10 @@ static bool cs_slot_process_irq_locked(struct panthor_device *ptdev,
if (events & CS_TILER_OOM)
cs_slot_process_tiler_oom_event_locked(ptdev, csg_id, cs_id);
+ if (events & CS_PROTM_PENDING)
+ cs_slot_process_protm_pending_event_locked(ptdev, csg_id,
+ cs_id);
+
/* We don't acknowledge the TILER_OOM event since its handling is
* deferred to a separate work.
*/
@@ -1856,6 +1936,30 @@ static void sched_process_idle_event_locked(struct panthor_device *ptdev)
sched_queue_delayed_work(ptdev->scheduler, tick, 0);
}
+static void sched_process_protm_exit_event_locked(struct panthor_device *ptdev)
+{
+ struct panthor_fw_global_iface *glb_iface =
+ panthor_fw_get_glb_iface(ptdev);
+ struct panthor_scheduler *sched = ptdev->scheduler;
+
+ lockdep_assert_held(&sched->events_lock);
+
+ atomic64_inc(&ptdev->protm.protm_exit_count);
+
+ /* Acknowledge the protm exit */
+ panthor_fw_update_reqs(glb_iface, req, glb_iface->output->ack,
+ GLB_PROTM_EXIT);
+
+ /* If there are pending fault from protected mode execution, then early
+ * out here. The GPU_IRQ_PROTM_FAULT handling will trigger the propper
+ * error recovery via a GPU reset.
+ */
+ if (panthor_gpu_protm_fault_pending(ptdev))
+ return;
+
+ sched_queue_delayed_work(sched, tick, 0);
+}
+
/**
* sched_process_global_irq_locked() - Process the scheduling part of a global IRQ
* @ptdev: Device.
@@ -1871,6 +1975,9 @@ static void sched_process_global_irq_locked(struct panthor_device *ptdev)
ack = READ_ONCE(glb_iface->output->ack);
evts = (req ^ ack) & GLB_EVT_MASK;
+ if (evts & GLB_PROTM_EXIT)
+ sched_process_protm_exit_event_locked(ptdev);
+
if (evts & GLB_IDLE)
sched_process_idle_event_locked(ptdev);
}
@@ -1882,22 +1989,22 @@ static void sched_process_global_irq_locked(struct panthor_device *ptdev)
*/
void panthor_sched_report_fw_events(struct panthor_device *ptdev, u32 events)
{
+ u32 csg_events = events & ~JOB_INT_GLOBAL_IF;
+
if (!ptdev->scheduler)
return;
guard(spinlock)(&ptdev->scheduler->events_lock);
- if (events & JOB_INT_GLOBAL_IF) {
- sched_process_global_irq_locked(ptdev);
- events &= ~JOB_INT_GLOBAL_IF;
- }
-
- while (events) {
- u32 csg_id = ffs(events) - 1;
+ while (csg_events) {
+ u32 csg_id = ffs(csg_events) - 1;
sched_process_csg_irq_locked(ptdev, csg_id);
- events &= ~BIT(csg_id);
+ csg_events &= ~BIT(csg_id);
}
+
+ if (events & JOB_INT_GLOBAL_IF)
+ sched_process_global_irq_locked(ptdev);
}
/**
@@ -1983,6 +2090,69 @@ group_unbind_locked(struct panthor_group *group)
return 0;
}
+static void handle_protm_fault(struct panthor_device *ptdev)
+{
+ struct panthor_scheduler *sched = ptdev->scheduler;
+ u32 csg_id;
+ struct panthor_group *protm_group;
+
+ guard(mutex)(&sched->lock);
+
+ protm_group = sched->protm.active_group;
+
+ if (!protm_group || !panthor_gpu_protm_fault_pending(ptdev))
+ return;
+
+ atomic_set(&protm_group->fatal_queues,
+ GENMASK(protm_group->queue_count - 1, 0));
+
+ /* Different kinds of faults during protected mode can give different
+ * behavior/state.
+ * Case 1) The fault keeps the GPU in protected mode.
+ * In this case, the request to exit protected mode below will
+ * fail and we need to take some further action.
+ * Case 2) The fault do not keep the GPU in protected mode.
+ * In this case, the request to exit protected
+ * mode below will succeed, and we don't need to take any
+ * further action right here.
+ */
+ if (!panthor_fw_protm_exit(ptdev, 500))
+ return;
+
+ /* GPU failed to exit protected mode.
+ * Mark all CSGs as suspended and unbind them, so that they are
+ * unaffected by the GPU reset itself.
+ * We can not suspend the groups in this case, because we are stuck
+ * in protected mode. That is also the reason it is safe to unbind
+ * without suspending first (the groups are already "suspended").
+ * The failing protected group will be scheduled for termination.
+ */
+
+ for (csg_id = 0; csg_id < sched->csg_slot_count; csg_id++) {
+ struct panthor_group *group = sched->csg_slots[csg_id].group;
+
+ if (!group)
+ continue;
+
+ group_get(group);
+
+ group->state = PANTHOR_CS_GROUP_SUSPENDED;
+ group_unbind_locked(group);
+
+ drm_WARN_ON(&group->ptdev->base, !list_empty(&group->run_node));
+
+ if (group_can_run(group)) {
+ list_add(&group->run_node,
+ &sched->groups.idle[group->priority]);
+ } else {
+ list_del_init(&group->wait_node);
+ group_queue_work(group, term);
+ }
+
+ group_put(group);
+ }
+}
+
static const char *fence_get_driver_name(struct dma_fence *fence)
{
return "panthor";
@@ -2012,6 +2182,12 @@ static void csgs_upd_ctx_init(struct panthor_csg_slots_upd_ctx *ctx)
memset(ctx, 0, sizeof(*ctx));
}
+static void csgs_upd_ctx_ring_doorbell(struct panthor_csg_slots_upd_ctx *ctx,
+ u32 csg_id)
+{
+ ctx->update_mask |= BIT(csg_id);
+}
+
static void csgs_upd_ctx_queue_reqs(struct panthor_device *ptdev,
struct panthor_csg_slots_upd_ctx *ctx,
u32 csg_id, u32 value, u32 mask)
@@ -2022,7 +2198,8 @@ static void csgs_upd_ctx_queue_reqs(struct panthor_device *ptdev,
ctx->requests[csg_id].value = (ctx->requests[csg_id].value & ~mask) | (value & mask);
ctx->requests[csg_id].mask |= mask;
- ctx->update_mask |= BIT(csg_id);
+
+ csgs_upd_ctx_ring_doorbell(ctx, csg_id);
}
static int csgs_upd_ctx_apply_locked(struct panthor_device *ptdev,
@@ -2039,8 +2216,12 @@ static int csgs_upd_ctx_apply_locked(struct panthor_device *ptdev,
while (update_slots) {
struct panthor_fw_csg_iface *csg_iface;
u32 csg_id = ffs(update_slots) - 1;
+ u32 req_mask = ctx->requests[csg_id].mask;
update_slots &= ~BIT(csg_id);
+ if (!req_mask)
+ continue;
+
csg_iface = panthor_fw_get_csg_iface(ptdev, csg_id);
panthor_fw_update_reqs(csg_iface, req,
ctx->requests[csg_id].value,
@@ -2057,6 +2238,9 @@ static int csgs_upd_ctx_apply_locked(struct panthor_device *ptdev,
int ret;
update_slots &= ~BIT(csg_id);
+ if (!req_mask)
+ continue;
+
csg_iface = panthor_fw_get_csg_iface(ptdev, csg_id);
ret = panthor_fw_csg_wait_acks(ptdev, csg_id, req_mask, &acked, 100);
@@ -2093,13 +2277,14 @@ struct panthor_sched_tick_ctx {
bool immediate_tick;
bool stop_tick;
u32 csg_upd_failed_mask;
+ struct panthor_group *protm_group;
};
static bool
tick_ctx_is_full(const struct panthor_scheduler *sched,
const struct panthor_sched_tick_ctx *ctx)
{
- return ctx->group_count == sched->csg_slot_count;
+ return (ctx->group_count == sched->csg_slot_count) || ctx->protm_group;
}
static void
@@ -2134,6 +2319,13 @@ tick_ctx_pick_groups_from_list(const struct panthor_scheduler *sched,
if (!owned_by_tick_ctx)
group_get(group);
+ /* Only the first pick is allowed to request switch to protm */
+ if (ctx->group_count == 0 &&
+ atomic_read(&group->protm_pending_queues)) {
+ ctx->protm_group = group;
+ ctx->stop_tick = false;
+ }
+
ctx->group_count++;
/* If we have more than one active group with the same priority,
@@ -2292,6 +2484,79 @@ static void group_term_work(struct work_struct *work)
group_put(group);
}
+bool panthor_sched_protm_block_timeout(struct panthor_device *ptdev,
+ struct panthor_vm *vm)
+{
+ struct panthor_scheduler *sched = ptdev->scheduler;
+ int ret;
+
+ if (!sched)
+ return true;
+
+ down_read(&sched->protm.lock);
+
+ if (!sched->protm.active_group)
+ return true; /* All good, we are in normal mode */
+
+ /* We will only exit protected mode for the passed vm and the MCU VM.
+ * Other VMs have no active CSG work, so MMU updates can proceed,
+ * although with special caution (delayed updates)!
+ */
+ if (sched->protm.active_group->vm != vm && !panthor_vm_for_mcu(vm))
+ return true; /* Specified VM can proceed, with caution. */
+
+ /* Give GPU a chance to finish ongoing protm work by itself */
+ ret = panthor_fw_protm_exit_wait(ptdev, 5);
+ if (!ret)
+ return true; /* All good, we are back to normal mode */
+
+ /* We do not want this function to fail, because that can lead to
+ * innocent processes failing. That is why we wait up to 2x the job
+ * timeout for GPU to exit protm here. This shall cover the worst case
+ * where protm work hangs and we must rely on job timeout to recover.
+ */
+ ret = panthor_fw_protm_exit(ptdev, JOB_TIMEOUT_MS << 1);
+ if (ret)
+ up_read(&sched->protm.lock);
+
+ return ret ? false : true;
+}
+
+bool panthor_sched_protm_try_block(struct panthor_device *ptdev)
+{
+ struct panthor_scheduler *sched = ptdev->scheduler;
+ int ret;
+
+ if (!sched)
+ return false;
+
+ ret = down_read_trylock(&sched->protm.lock);
+ if (ret) {
+ /* Unlike panthor_sched_protm_block_timeout(), we don't bother
+ * checking against a particular vm. It is not that important
+ * to succeed in this use case.
+ */
+ if (sched->protm.active_group) {
+ up_read(&sched->protm.lock);
+ return false;
+ }
+
+ return true;
+ }
+
+ return false;
+}
+
+void panthor_sched_protm_unblock(struct panthor_device *ptdev)
+{
+ struct panthor_scheduler *sched = ptdev->scheduler;
+
+ if (!sched)
+ return;
+
+ up_read(&sched->protm.lock);
+}
+
static void
tick_ctx_cleanup(struct panthor_scheduler *sched,
struct panthor_sched_tick_ctx *ctx)
@@ -2419,6 +2684,41 @@ tick_ctx_schedule_group(struct panthor_scheduler *sched,
return 0;
}
+static void
+tick_ctx_handle_protm_group(struct panthor_scheduler *sched,
+ struct panthor_csg_slots_upd_ctx *upd_ctx,
+ struct panthor_group *group)
+{
+ struct panthor_device *ptdev = sched->ptdev;
+ struct panthor_fw_csg_iface *csg_iface =
+ panthor_fw_get_csg_iface(ptdev, group->csg_id);
+ u32 q;
+
+ if (drm_WARN_ON(&ptdev->base, group->csg_id < 0))
+ return;
+
+ guard(spinlock)(&sched->events_lock);
+
+ for (q = 0; q < group->queue_count; q++) {
+ struct panthor_fw_cs_iface *cs_iface =
+ panthor_fw_get_cs_iface(ptdev, group->csg_id, q);
+
+ /* Ack any pending CS_PROTM_PENDING so it can run in protm */
+ if ((cs_iface->output->ack ^ cs_iface->input->req) &
+ CS_PROTM_PENDING)
+ panthor_fw_update_reqs(cs_iface, req,
+ cs_iface->output->ack,
+ CS_PROTM_PENDING);
+ }
+
+ panthor_fw_toggle_reqs(csg_iface, doorbell_req, doorbell_ack,
+ GENMASK(group->queue_count - 1, 0));
+
+ csgs_upd_ctx_ring_doorbell(upd_ctx, group->csg_id);
+
+ atomic_set(&group->protm_pending_queues, 0);
+}
+
static void
tick_ctx_apply(struct panthor_scheduler *sched, struct panthor_sched_tick_ctx *ctx)
{
@@ -2488,6 +2788,9 @@ tick_ctx_apply(struct panthor_scheduler *sched, struct panthor_sched_tick_ctx *c
}
}
+ if (ctx->protm_group)
+ tick_ctx_handle_protm_group(sched, &upd_ctx, ctx->protm_group);
+
ret = csgs_upd_ctx_apply_locked(ptdev, &upd_ctx);
if (ret) {
panthor_device_schedule_reset(ptdev);
@@ -2495,6 +2798,24 @@ tick_ctx_apply(struct panthor_scheduler *sched, struct panthor_sched_tick_ctx *c
return;
}
+ if (ctx->protm_group) {
+ down_write(&sched->protm.lock);
+
+ sched->protm.active_group = ctx->protm_group;
+ group_get(sched->protm.active_group);
+
+ /* Tell MMU we are entering protm with this VM (AS) */
+ panthor_mmu_protm_enter_event(ptdev, ctx->protm_group->vm);
+
+ ret = panthor_fw_protm_enter(ptdev);
+ if (ret) {
+ panthor_device_schedule_reset(ptdev);
+ ctx->csg_upd_failed_mask = U32_MAX;
+ }
+
+ up_write(&sched->protm.lock);
+ }
+
for (prio = PANTHOR_CSG_PRIORITY_COUNT - 1; prio >= 0; prio--) {
list_for_each_entry_safe(group, tmp, &ctx->groups[prio], run_node) {
list_del_init(&group->run_node);
@@ -2531,12 +2852,12 @@ tick_ctx_apply(struct panthor_scheduler *sched, struct panthor_sched_tick_ctx *c
}
static u64
-tick_ctx_update_resched_target(struct panthor_scheduler *sched,
- const struct panthor_sched_tick_ctx *ctx)
+tick_update_resched_target(struct panthor_scheduler *sched,
+ bool stop_tick)
{
u64 resched_target;
- if (ctx->stop_tick)
+ if (stop_tick)
goto no_tick;
resched_target = sched->last_tick + sched->tick_period;
@@ -2552,6 +2873,38 @@ tick_ctx_update_resched_target(struct panthor_scheduler *sched,
return U64_MAX;
}
+static bool preempt_protm_group(struct panthor_group *group, bool full_tick)
+{
+ struct panthor_device *ptdev = group->ptdev;
+ struct panthor_scheduler *sched = ptdev->scheduler;
+ bool groups_waiting;
+
+ /* If the protm group can not run, then preempt! */
+ if (!group_can_run(group))
+ return true;
+
+ /* If we have RT groups waiting and we are not RT, then preempt! */
+ groups_waiting =
+ !list_empty(&sched->groups.runnable[PANTHOR_CSG_PRIORITY_RT]);
+ if (groups_waiting && group->priority < PANTHOR_CSG_PRIORITY_RT)
+ return true;
+
+ /* The below conditions for preempting are only valid for full ticks */
+ if (!full_tick)
+ return false;
+
+ for (int pri = PANTHOR_CSG_PRIORITY_RT - 1;
+ pri >= sched->protm.active_group->priority; pri--) {
+ if (!list_empty(&sched->groups.runnable[pri])) {
+ groups_waiting = true;
+ break;
+ }
+ }
+
+ return groups_waiting;
+
+}
+
static void tick_work(struct work_struct *work)
{
struct panthor_scheduler *sched = container_of(work, struct panthor_scheduler,
@@ -2584,6 +2937,58 @@ static void tick_work(struct work_struct *work)
if (panthor_device_reset_is_pending(sched->ptdev))
goto out_unlock;
+ /* If we exited protm since last tick, then deal with that first */
+ if (sched->protm.active_group) {
+ bool protm = atomic64_read(&ptdev->protm.protm_exit_count) !=
+ atomic64_read(&ptdev->protm.protm_enter_count);
+ if (!protm) {
+ down_write(&sched->protm.lock);
+
+ /* No longer in protected mode, clear active_group */
+ group_put(sched->protm.active_group);
+ sched->protm.active_group = NULL;
+
+ /* Allow MMU ops on all ASes again */
+ panthor_mmu_protm_exit_event(ptdev);
+
+ up_write(&sched->protm.lock);
+ }
+ }
+
+ if (sched->protm.active_group) {
+ /* A normal tick will issue CSG_STATUS_UPDATE requests in
+ * tick_ctx_init(). If we do this while running in protm, then
+ * the FW will preempt and exit protm in order to fullfill the
+ * request. That is why we do a "SW only" check for preemption
+ * in this case, so we avoid unnecessary preemption.
+ */
+ if (preempt_protm_group(sched->protm.active_group, full_tick)) {
+ /* Ensure we are fully out of protected mode first.
+ * We rely on the GLB_PROTM_EXIT (or error recovery)
+ * to get a new tick.
+ */
+ if (panthor_fw_protm_exit(ptdev, 500)) {
+ drm_warn(&ptdev->base,
+ "Failed to preempt protm");
+ panthor_device_schedule_reset(ptdev);
+ }
+ } else {
+ sched->last_tick = now;
+
+ /* We can stop ticking if this was a full tick.
+ * New work which can preempt this protm work will
+ * generate new ticks.
+ */
+ resched_delay =
+ tick_update_resched_target(sched, full_tick);
+ if (resched_delay != U64_MAX)
+ sched_queue_delayed_work(sched, tick,
+ resched_delay);
+ }
+
+ goto out_unlock;
+ }
+
tick_ctx_init(sched, &ctx);
if (ctx.csg_upd_failed_mask)
goto out_cleanup_ctx;
@@ -2635,14 +3040,20 @@ static void tick_work(struct work_struct *work)
}
}
- /* If we have free CSG slots left, pick idle groups */
- for (prio = PANTHOR_CSG_PRIORITY_COUNT - 1;
- prio >= 0 && !tick_ctx_is_full(sched, &ctx);
- prio--) {
- /* Check the old_group queue first to avoid reprogramming the slots */
- tick_ctx_pick_groups_from_list(sched, &ctx, &ctx.old_groups[prio], false, true);
- tick_ctx_pick_groups_from_list(sched, &ctx, &sched->groups.idle[prio],
- false, false);
+ /* No need to pick any more groups if we are going to run in protm */
+ if (!ctx.protm_group) {
+ for (prio = PANTHOR_CSG_PRIORITY_COUNT - 1;
+ prio >= 0 && !tick_ctx_is_full(sched, &ctx); prio--) {
+ /* Check the old_group queue first to avoid
+ * reprogramming the slots
+ */
+ tick_ctx_pick_groups_from_list(sched, &ctx,
+ &ctx.old_groups[prio],
+ false, true);
+ tick_ctx_pick_groups_from_list(sched, &ctx,
+ &sched->groups.idle[prio],
+ false, false);
+ }
}
tick_ctx_apply(sched, &ctx);
@@ -2664,7 +3075,7 @@ static void tick_work(struct work_struct *work)
}
sched->last_tick = now;
- resched_delay = tick_ctx_update_resched_target(sched, &ctx);
+ resched_delay = tick_update_resched_target(sched, ctx.stop_tick);
if (ctx.immediate_tick)
resched_delay = 0;
@@ -3069,6 +3480,8 @@ void panthor_sched_pre_reset(struct panthor_device *ptdev)
cancel_work_sync(&sched->sync_upd_work);
cancel_delayed_work_sync(&sched->tick_work);
+ handle_protm_fault(ptdev);
+
panthor_sched_suspend(ptdev);
/* Stop all groups that might still accept jobs, so we don't get passed
@@ -3094,6 +3507,11 @@ void panthor_sched_post_reset(struct panthor_device *ptdev, bool reset_failed)
mutex_lock(&sched->reset.lock);
+ if (sched->protm.active_group) {
+ group_put(sched->protm.active_group);
+ sched->protm.active_group = NULL;
+ }
+
list_for_each_entry_safe(group, group_tmp, &sched->reset.stopped_groups, run_node) {
/* Consider all previously running group as terminated if the
* reset failed.
@@ -4170,6 +4588,7 @@ int panthor_sched_init(struct panthor_device *ptdev)
INIT_WORK(&sched->sync_upd_work, sync_upd_work);
spin_lock_init(&sched->events_lock);
+ init_rwsem(&sched->protm.lock);
ret = drmm_mutex_init(&ptdev->base, &sched->lock);
if (ret)
diff --git a/drivers/gpu/drm/panthor/panthor_sched.h b/drivers/gpu/drm/panthor/panthor_sched.h
index c509fcb5a0c9c..64d583de80c42 100644
--- a/drivers/gpu/drm/panthor/panthor_sched.h
+++ b/drivers/gpu/drm/panthor/panthor_sched.h
@@ -53,6 +53,11 @@ void panthor_sched_report_fw_events(struct panthor_device *ptdev, u32 events);
void panthor_fdinfo_gather_group_samples(struct drm_file *file);
+bool panthor_sched_protm_block_timeout(struct panthor_device *ptdev,
+ struct panthor_vm *vm);
+bool panthor_sched_protm_try_block(struct panthor_device *ptdev);
+void panthor_sched_protm_unblock(struct panthor_device *ptdev);
+
#ifdef CONFIG_DEBUG_FS
void panthor_sched_debugfs_init(struct drm_minor *minor);
#endif
--
2.43.0
^ permalink raw reply related [flat|nested] 18+ messages in thread* Re: [PATCH v4 05/12] drm/panthor: Add support for entering and exiting protected mode
2026-10-12 12:59 ` [PATCH v4 05/12] drm/panthor: Add support for entering and exiting protected mode Ketil Johnsen
@ 2026-10-12 13:17 ` sashiko-bot
0 siblings, 0 replies; 18+ messages in thread
From: sashiko-bot @ 2026-10-12 13:17 UTC (permalink / raw)
To: Ketil Johnsen; +Cc: dri-devel
Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider:
- [High] sched: data race and UAF on sched->protm.active_group during reset
- [Medium] mmu: delayed_update_mask is never cleared, causing permanent cache flushes
- [Medium] drm/panthor: UAPI loses -ERESTARTSYS on signal interruption during protm exit
- [Low] cleanup: mixing goto-based error handling with scoped_cond_guard()
--
commit d3563f471c349ec35f609214f5443133a617a0b3
Author: Florent Tomasin <florent.tomasin@arm.com>
drm/panthor: Add support for entering and exiting protected mode
This patch modifies the Panthor driver code to allow handling of the GPU HW
protected mode enter and exit. It introduces the mechanisms needed for
entering and exiting protected mode, handling of protected mode IRQs,
scheduler changes, and GPU fault handling.
> diff --git a/drivers/gpu/drm/panthor/panthor_mmu.c b/drivers/gpu/drm/panthor/panthor_mmu.c
> --- a/drivers/gpu/drm/panthor/panthor_mmu.c
> +++ b/drivers/gpu/drm/panthor/panthor_mmu.c
> @@ -2626,6 +2690,7 @@ static int remap_evicted_vma(struct drm_gpuvm_bo *vm_bo,
> break;
> }
>
> - if (found) {
> + if (!found)
> + goto out_unlock;
> +
> + scoped_cond_guard(protm_block_timeout, ret = -ETIMEDOUT, vm) {
[Severity: Low]
Does this mix goto-based error handling with scoped guards?
The function uses both goto labels like out_unlock and scoped_cond_guard. The
kernel guidelines advise against mixing goto-based cleanup and scope-based
cleanup helpers in the same function.
[Severity: Medium]
Does this lose the -ERESTARTSYS return value?
The wait function panthor_fw_protm_exit_wait() correctly returns -ERESTARTSYS
if interrupted by a signal, but panthor_sched_protm_block_timeout() converts
this error to a boolean. The guard macro here then hardcodes ret = -ETIMEDOUT.
If the userspace thread is interrupted, it incorrectly receives a timeout error
instead of standard signal interruption semantics.
> vm->op_ctx = op_ctx;
> +
> ret = panthor_vm_lock_region(vm, evicted_vma->base.va.addr,
[ ... ]
> @@ -2726,39 +2781,42 @@ panthor_vm_exec_op(struct panthor_vm *vm, struct panthor_vm_op_ctx *op,
> mutex_lock(&vm->op_lock);
> vm->op_ctx = op;
>
> - ret = panthor_vm_lock_region(vm, op->va.addr, op->va.range);
> - if (ret)
> - goto out;
> -
> - switch (op_type) {
[ ... ]
> + scoped_cond_guard(protm_block_timeout, ret = -ETIMEDOUT, vm) {
> + ret = panthor_vm_lock_region(vm, op->va.addr, op->va.range);
> + if (ret)
> + goto out;
[Severity: Low]
Is it safe to use a goto inside a scoped guard block when the function relies
on a mix of scope-based and goto-based cleanup?
[Severity: Medium]
Will signal interruptions incorrectly return -ETIMEDOUT here?
The boolean guard panthor_sched_protm_block_timeout discards the specific
-ERESTARTSYS error from panthor_fw_protm_exit_wait(), and this macro hardcodes
a timeout error on failure.
[ ... ]
> @@ -3517,3 +3575,54 @@ void panthor_mmu_pt_cache_fini(void)
[ ... ]
> +void panthor_mmu_protm_exit_event(struct panthor_device *ptdev)
> +{
[ ... ]
> + guard(mutex)(&ptdev->mmu->as.slots_lock);
> +
> + ptdev->mmu->as.delay_update_mask = 0;
> +
> + if (!ptdev->mmu->as.delayed_update_mask)
> + return;
> +
> + ret = panthor_gpu_flush_caches(ptdev, CACHE_CLEAN | CACHE_INV,
[Severity: Medium]
Is it possible that delayed_update_mask is never cleared back to 0?
The code checks delayed_update_mask to decide whether to flush the cache,
but it only clears delay_update_mask. Leaving delayed_update_mask non-zero
could trigger a full GPU cache flush on every exit from protected mode.
> diff --git a/drivers/gpu/drm/panthor/panthor_sched.c b/drivers/gpu/drm/panthor/panthor_sched.c
> --- a/drivers/gpu/drm/panthor/panthor_sched.c
> +++ b/drivers/gpu/drm/panthor/panthor_sched.c
> @@ -2484,6 +2484,79 @@ static void group_term_work(struct work_struct *work)
[ ... ]
> +bool panthor_sched_protm_block_timeout(struct panthor_device *ptdev,
> + struct panthor_vm *vm)
> +{
[ ... ]
> + /* We do not want this function to fail, because that can lead to
> + * innocent processes failing. That is why we wait up to 2x the job
> + * timeout for GPU to exit protm here. This shall cover the worst case
> + * where protm work hangs and we must rely on job timeout to recover.
> + */
> + ret = panthor_fw_protm_exit(ptdev, JOB_TIMEOUT_MS << 1);
> + if (ret)
> + up_read(&sched->protm.lock);
> +
> + return ret ? false : true;
> +}
[Severity: Medium]
Does returning a boolean here discard the specific error code from
panthor_fw_protm_exit?
If panthor_fw_protm_exit_wait() returns -ERESTARTSYS, the boolean return type
prevents propagating it to userspace, which breaks standard syscall restart
semantics.
[ ... ]
> @@ -3094,6 +3507,11 @@ void panthor_sched_post_reset(struct panthor_device *ptdev, bool reset_failed)
>
> mutex_lock(&sched->reset.lock);
>
> + if (sched->protm.active_group) {
> + group_put(sched->protm.active_group);
> + sched->protm.active_group = NULL;
> + }
> +
[Severity: High]
Does this modification of active_group without holding the proper
rw_semaphore introduce a data race?
The reader side panthor_sched_protm_block_timeout() checks and dereferences
sched->protm.active_group->vm while holding down_read(&sched->protm.lock).
Because panthor_sched_post_reset() sets active_group to NULL and drops its
reference without acquiring down_write(&sched->protm.lock), the group could
be freed out from under the ioctl thread causing a use-after-free.
--
Sashiko AI review · https://sashiko.dev/#/patchset/20261012125940.3291951-1-ketil.johnsen@arm.com?part=5
^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH v4 06/12] drm/panthor: New function for FW section map flags
2026-10-12 12:59 [PATCH v4 00/12] drm/panthor: Protected mode support for Mali CSF GPUs Ketil Johnsen
` (4 preceding siblings ...)
2026-10-12 12:59 ` [PATCH v4 05/12] drm/panthor: Add support for entering and exiting protected mode Ketil Johnsen
@ 2026-10-12 12:59 ` Ketil Johnsen
2026-10-12 12:59 ` [PATCH v4 07/12] drm/panthor: Keep size and VA in FW section obj Ketil Johnsen
` (5 subsequent siblings)
11 siblings, 0 replies; 18+ messages in thread
From: Ketil Johnsen @ 2026-10-12 12:59 UTC (permalink / raw)
To: Boris Brezillon, Steven Price, Liviu Dudau, Maarten Lankhorst,
Maxime Ripard, Thomas Zimmermann, David Airlie, Simona Vetter,
Daniel Almeida, Alice Ryhl
Cc: dri-devel, linux-kernel, Ketil Johnsen
From: Boris Brezillon <boris.brezillon@collabora.com>
This refactors the VM map flags for FW sections into a new function
called section_vm_map_flags().
This comes in handly for future handling of protected FW sections
(type CSF_FW_BINARY_IFACE_ENTRY_PROT).
Signed-off-by: Boris Brezillon <boris.brezillon@collabora.com>
Co-developed-by: Ketil Johnsen <ketil.johnsen@arm.com>
Signed-off-by: Ketil Johnsen <ketil.johnsen@arm.com>
---
v4:
- First version of this change as a separate commit
---
drivers/gpu/drm/panthor/panthor_fw.c | 44 ++++++++++++++++------------
1 file changed, 26 insertions(+), 18 deletions(-)
diff --git a/drivers/gpu/drm/panthor/panthor_fw.c b/drivers/gpu/drm/panthor/panthor_fw.c
index e639ffeac7a32..7b8c90b19f4fe 100644
--- a/drivers/gpu/drm/panthor/panthor_fw.c
+++ b/drivers/gpu/drm/panthor/panthor_fw.c
@@ -543,6 +543,31 @@ panthor_fw_alloc_suspend_buf_mem(struct panthor_device *ptdev, size_t size)
"FW suspend buffer");
}
+static u32 section_vm_map_flags(const struct panthor_fw_section *section)
+{
+ u32 cache_mode = section->flags &
+ CSF_FW_BINARY_IFACE_ENTRY_CACHE_MODE_MASK;
+ u32 vm_map_flags = 0;
+
+ if (!(section->flags & CSF_FW_BINARY_IFACE_ENTRY_WR))
+ vm_map_flags |= DRM_PANTHOR_VM_BIND_OP_MAP_READONLY;
+
+ if (!(section->flags & CSF_FW_BINARY_IFACE_ENTRY_EX))
+ vm_map_flags |= DRM_PANTHOR_VM_BIND_OP_MAP_NOEXEC;
+
+ /* TODO: CSF_FW_BINARY_IFACE_ENTRY_CACHE_MODE_*_COHERENT are mapped to
+ * non-cacheable for now. We might want to introduce a new
+ * IOMMU_xxx flag (or abuse IOMMU_MMIO, which maps to device
+ * memory and is currently not used by our driver) for
+ * AS_MEMATTR_AARCH64_SHARED memory, so we can take benefit
+ * of IO-coherent systems.
+ */
+ if (cache_mode != CSF_FW_BINARY_IFACE_ENTRY_CACHE_MODE_CACHED)
+ vm_map_flags |= DRM_PANTHOR_VM_BIND_OP_MAP_UNCACHED;
+
+ return vm_map_flags;
+}
+
static int panthor_fw_load_section_entry(struct panthor_device *ptdev,
const struct firmware *fw,
struct panthor_fw_binary_iter *iter,
@@ -645,26 +670,9 @@ static int panthor_fw_load_section_entry(struct panthor_device *ptdev,
}
if (section_size) {
- u32 cache_mode = hdr.flags & CSF_FW_BINARY_IFACE_ENTRY_CACHE_MODE_MASK;
- u32 vm_map_flags = 0;
+ u32 vm_map_flags = section_vm_map_flags(section);
u64 va = hdr.va.start;
- if (!(hdr.flags & CSF_FW_BINARY_IFACE_ENTRY_WR))
- vm_map_flags |= DRM_PANTHOR_VM_BIND_OP_MAP_READONLY;
-
- if (!(hdr.flags & CSF_FW_BINARY_IFACE_ENTRY_EX))
- vm_map_flags |= DRM_PANTHOR_VM_BIND_OP_MAP_NOEXEC;
-
- /* TODO: CSF_FW_BINARY_IFACE_ENTRY_CACHE_MODE_*_COHERENT are mapped to
- * non-cacheable for now. We might want to introduce a new
- * IOMMU_xxx flag (or abuse IOMMU_MMIO, which maps to device
- * memory and is currently not used by our driver) for
- * AS_MEMATTR_AARCH64_SHARED memory, so we can take benefit
- * of IO-coherent systems.
- */
- if (cache_mode != CSF_FW_BINARY_IFACE_ENTRY_CACHE_MODE_CACHED)
- vm_map_flags |= DRM_PANTHOR_VM_BIND_OP_MAP_UNCACHED;
-
section->mem = panthor_kernel_bo_create(ptdev, panthor_fw_vm(ptdev),
section_size,
DRM_PANTHOR_BO_NO_MMAP,
--
2.43.0
^ permalink raw reply related [flat|nested] 18+ messages in thread* [PATCH v4 07/12] drm/panthor: Keep size and VA in FW section obj
2026-10-12 12:59 [PATCH v4 00/12] drm/panthor: Protected mode support for Mali CSF GPUs Ketil Johnsen
` (5 preceding siblings ...)
2026-10-12 12:59 ` [PATCH v4 06/12] drm/panthor: New function for FW section map flags Ketil Johnsen
@ 2026-10-12 12:59 ` Ketil Johnsen
2026-10-12 12:59 ` [PATCH v4 08/12] drm/panthor: Refactor GEM usage flags assignment Ketil Johnsen
` (4 subsequent siblings)
11 siblings, 0 replies; 18+ messages in thread
From: Ketil Johnsen @ 2026-10-12 12:59 UTC (permalink / raw)
To: Boris Brezillon, Steven Price, Liviu Dudau, Maarten Lankhorst,
Maxime Ripard, Thomas Zimmermann, David Airlie, Simona Vetter,
Daniel Almeida, Alice Ryhl
Cc: dri-devel, linux-kernel, Ketil Johnsen
From: Boris Brezillon <boris.brezillon@collabora.com>
Keep a copy of the FW section size and VA in the
panthor_fw_section object. We need this in the future when handling
late init of protected memory for FW sections of type
CSF_FW_BINARY_IFACE_ENTRY_PROT.
Signed-off-by: Boris Brezillon <boris.brezillon@collabora.com>
Co-developed-by: Ketil Johnsen <ketil.johnsen@arm.com>
Signed-off-by: Ketil Johnsen <ketil.johnsen@arm.com>
---
v4:
- First version of this change as a separate commit
---
drivers/gpu/drm/panthor/panthor_fw.c | 21 +++++++++++++++------
1 file changed, 15 insertions(+), 6 deletions(-)
diff --git a/drivers/gpu/drm/panthor/panthor_fw.c b/drivers/gpu/drm/panthor/panthor_fw.c
index 7b8c90b19f4fe..fc07796fc9ef7 100644
--- a/drivers/gpu/drm/panthor/panthor_fw.c
+++ b/drivers/gpu/drm/panthor/panthor_fw.c
@@ -198,6 +198,12 @@ struct panthor_fw_section {
/** @size: Size of @buf in bytes. */
size_t size;
} data;
+
+ /** @size: Section size. */
+ size_t size;
+
+ /** @va: Section VA. */
+ u32 va;
};
#define CSF_MCU_SHARED_REGION_START 0x04000000ULL
@@ -669,18 +675,21 @@ static int panthor_fw_load_section_entry(struct panthor_device *ptdev,
section->name = name;
}
+ section->size = section_size;
+ section->va = hdr.va.start;
+
if (section_size) {
u32 vm_map_flags = section_vm_map_flags(section);
- u64 va = hdr.va.start;
- section->mem = panthor_kernel_bo_create(ptdev, panthor_fw_vm(ptdev),
- section_size,
- DRM_PANTHOR_BO_NO_MMAP,
- vm_map_flags, va, "FW section");
+ section->mem = panthor_kernel_bo_create(
+ ptdev, panthor_fw_vm(ptdev), section->size,
+ DRM_PANTHOR_BO_NO_MMAP, vm_map_flags, section->va,
+ "FW section");
if (IS_ERR(section->mem))
return PTR_ERR(section->mem);
- if (drm_WARN_ON(&ptdev->base, section->mem->va_node.start != hdr.va.start))
+ if (drm_WARN_ON(&ptdev->base,
+ section->mem->va_node.start != section->va))
return -EINVAL;
if (section->flags & CSF_FW_BINARY_IFACE_ENTRY_SHARED) {
--
2.43.0
^ permalink raw reply related [flat|nested] 18+ messages in thread* [PATCH v4 08/12] drm/panthor: Refactor GEM usage flags assignment
2026-10-12 12:59 [PATCH v4 00/12] drm/panthor: Protected mode support for Mali CSF GPUs Ketil Johnsen
` (6 preceding siblings ...)
2026-10-12 12:59 ` [PATCH v4 07/12] drm/panthor: Keep size and VA in FW section obj Ketil Johnsen
@ 2026-10-12 12:59 ` Ketil Johnsen
2026-10-12 12:59 ` [PATCH v4 09/12] drm/panthor: Create kernel BO from existing GEM Ketil Johnsen
` (3 subsequent siblings)
11 siblings, 0 replies; 18+ messages in thread
From: Ketil Johnsen @ 2026-10-12 12:59 UTC (permalink / raw)
To: Boris Brezillon, Steven Price, Liviu Dudau, Maarten Lankhorst,
Maxime Ripard, Thomas Zimmermann, David Airlie, Simona Vetter,
Daniel Almeida, Alice Ryhl
Cc: dri-devel, linux-kernel, Ketil Johnsen
From: Boris Brezillon <boris.brezillon@collabora.com>
Changes panthor_gem_debugfs_set_usage_flags() to
panthor_gem_debugfs_add_usage_flags(). This will come in handy in a
later change when we split panthor_kernel_bo_create() to also handle
protected memory.
Signed-off-by: Boris Brezillon <boris.brezillon@collabora.com>
Co-developed-by: Ketil Johnsen <ketil.johnsen@arm.com>
Signed-off-by: Ketil Johnsen <ketil.johnsen@arm.com>
---
v4:
- First version of this change as a separate commit
---
drivers/gpu/drm/panthor/panthor_gem.c | 15 +++++++++------
drivers/gpu/drm/panthor/panthor_gem.h | 2 +-
2 files changed, 10 insertions(+), 7 deletions(-)
diff --git a/drivers/gpu/drm/panthor/panthor_gem.c b/drivers/gpu/drm/panthor/panthor_gem.c
index c07a44057e426..2e39a32fcd673 100644
--- a/drivers/gpu/drm/panthor/panthor_gem.c
+++ b/drivers/gpu/drm/panthor/panthor_gem.c
@@ -74,14 +74,16 @@ static void panthor_gem_debugfs_bo_rm(struct panthor_gem_object *bo)
mutex_unlock(&ptdev->gems.lock);
}
-static void panthor_gem_debugfs_set_usage_flags(struct panthor_gem_object *bo, u32 usage_flags)
+static void panthor_gem_debugfs_add_usage_flags(struct panthor_gem_object *bo,
+ u32 usage_flags)
{
- bo->debugfs.flags = usage_flags;
- panthor_gem_debugfs_bo_add(bo);
+ atomic_or(usage_flags, &bo->debugfs.flags);
}
#else
+static void panthor_gem_debugfs_bo_add(struct panthor_gem_object *bo) {}
static void panthor_gem_debugfs_bo_rm(struct panthor_gem_object *bo) {}
-static void panthor_gem_debugfs_set_usage_flags(struct panthor_gem_object *bo, u32 usage_flags) {}
+static void panthor_gem_debugfs_add_usage_flags(struct panthor_gem_object *bo,
+ u32 usage_flags) {}
static void panthor_gem_debugfs_bo_init(struct panthor_gem_object *bo) {}
#endif
@@ -1031,7 +1033,8 @@ panthor_gem_create(struct drm_device *dev, size_t size, uint32_t flags,
bo->base.resv = bo->exclusive_vm_root_gem->resv;
}
- panthor_gem_debugfs_set_usage_flags(bo, usage_flags);
+ panthor_gem_debugfs_add_usage_flags(bo, usage_flags);
+ panthor_gem_debugfs_bo_add(bo);
return bo;
err_put:
@@ -1644,9 +1647,9 @@ static void panthor_gem_debugfs_bo_print(struct panthor_gem_object *bo,
enum panthor_gem_reclaim_state reclaim_state = bo->reclaim_state;
unsigned int refcount = kref_read(&bo->base.refcount);
int reclaimed_count = atomic_read(&bo->reclaimed_count);
+ u32 gem_usage_flags = atomic_read(&bo->debugfs.flags);
char creator_info[32] = {};
size_t resident_size;
- u32 gem_usage_flags = bo->debugfs.flags;
u32 gem_state_flags = 0;
/* Skip BOs being destroyed. */
diff --git a/drivers/gpu/drm/panthor/panthor_gem.h b/drivers/gpu/drm/panthor/panthor_gem.h
index 5ae37d0d3646f..4da3f59693ce4 100644
--- a/drivers/gpu/drm/panthor/panthor_gem.h
+++ b/drivers/gpu/drm/panthor/panthor_gem.h
@@ -62,7 +62,7 @@ struct panthor_gem_debugfs {
} creator;
/** @flags: Combination of panthor_debugfs_gem_usage_flags flags */
- u32 flags;
+ atomic_t flags;
};
/**
--
2.43.0
^ permalink raw reply related [flat|nested] 18+ messages in thread* [PATCH v4 09/12] drm/panthor: Create kernel BO from existing GEM
2026-10-12 12:59 [PATCH v4 00/12] drm/panthor: Protected mode support for Mali CSF GPUs Ketil Johnsen
` (7 preceding siblings ...)
2026-10-12 12:59 ` [PATCH v4 08/12] drm/panthor: Refactor GEM usage flags assignment Ketil Johnsen
@ 2026-10-12 12:59 ` Ketil Johnsen
2026-10-12 13:12 ` sashiko-bot
2026-10-12 12:59 ` [PATCH v4 10/12] drm/panthor: Support FW init of protm Ketil Johnsen
` (2 subsequent siblings)
11 siblings, 1 reply; 18+ messages in thread
From: Ketil Johnsen @ 2026-10-12 12:59 UTC (permalink / raw)
To: Boris Brezillon, Steven Price, Liviu Dudau, Maarten Lankhorst,
Maxime Ripard, Thomas Zimmermann, David Airlie, Simona Vetter,
Daniel Almeida, Alice Ryhl
Cc: dri-devel, linux-kernel, Ketil Johnsen
From: Boris Brezillon <boris.brezillon@collabora.com>
Refactors/splits existing panthor_kernel_bo_create().
Adds panthor_kernel_bo_create_from_gem() so we can create kernel BOs
from imported protected memory in future change.
Signed-off-by: Boris Brezillon <boris.brezillon@collabora.com>
Co-developed-by: Ketil Johnsen <ketil.johnsen@arm.com>
Signed-off-by: Ketil Johnsen <ketil.johnsen@arm.com>
---
v4:
- First version of this change as a separate commit
---
drivers/gpu/drm/panthor/panthor_gem.c | 94 ++++++++++++++++++---------
drivers/gpu/drm/panthor/panthor_gem.h | 7 ++
2 files changed, 71 insertions(+), 30 deletions(-)
diff --git a/drivers/gpu/drm/panthor/panthor_gem.c b/drivers/gpu/drm/panthor/panthor_gem.c
index 2e39a32fcd673..f1939b4aafed1 100644
--- a/drivers/gpu/drm/panthor/panthor_gem.c
+++ b/drivers/gpu/drm/panthor/panthor_gem.c
@@ -1033,7 +1033,6 @@ panthor_gem_create(struct drm_device *dev, size_t size, uint32_t flags,
bo->base.resv = bo->exclusive_vm_root_gem->resv;
}
- panthor_gem_debugfs_add_usage_flags(bo, usage_flags);
panthor_gem_debugfs_bo_add(bo);
return bo;
@@ -1260,7 +1259,9 @@ void panthor_kernel_bo_destroy(struct panthor_kernel_bo *bo)
panthor_kernel_bo_vunmap(bo);
drm_WARN_ON(bo->obj->dev,
- to_panthor_bo(bo->obj)->exclusive_vm_root_gem != panthor_vm_root_gem(vm));
+ to_panthor_bo(bo->obj)->exclusive_vm_root_gem &&
+ (to_panthor_bo(bo->obj)->exclusive_vm_root_gem !=
+ panthor_vm_root_gem(vm)));
panthor_vm_unmap_range(vm, bo->va_node.start, bo->va_node.size);
panthor_vm_free_va(vm, &bo->va_node);
if (vm == panthor_fw_vm(ptdev))
@@ -1271,46 +1272,33 @@ void panthor_kernel_bo_destroy(struct panthor_kernel_bo *bo)
}
/**
- * panthor_kernel_bo_create() - Create and map a GEM object to a VM
+ * panthor_kernel_bo_create_from_gem() - Create kernel BO from existing GEM object
* @ptdev: Device.
* @vm: VM to map the GEM to.
- * @size: Size of the buffer object.
- * @bo_flags: Combination of drm_panthor_bo_flags flags.
+ * @bo: BO to use for our kernel BO.
* @vm_map_flags: Combination of drm_panthor_vm_bind_op_flags (only those
* that are related to map operations).
* @gpu_va: GPU address assigned when mapping to the VM.
* If gpu_va == PANTHOR_VM_KERNEL_AUTO_VA, the virtual address will be
* automatically allocated.
- * @name: Descriptive label of the BO's contents
+ * @vm_map_size: Size of the BO to map to the VM.
*
* Return: A valid pointer in case of success, an ERR_PTR() otherwise.
*/
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, const char *name)
+panthor_kernel_bo_create_from_gem(struct panthor_device *ptdev,
+ struct panthor_vm *vm,
+ struct panthor_gem_object *bo,
+ u32 vm_map_flags, u64 gpu_va, u32 vm_map_size)
{
struct panthor_kernel_bo *kbo;
- struct panthor_gem_object *bo;
- u32 debug_flags = PANTHOR_DEBUGFS_GEM_USAGE_FLAG_KERNEL;
int ret;
- if (drm_WARN_ON(&ptdev->base, !vm))
- return ERR_PTR(-EINVAL);
-
kbo = kzalloc_obj(*kbo);
if (!kbo)
return ERR_PTR(-ENOMEM);
- if (vm == panthor_fw_vm(ptdev))
- debug_flags |= PANTHOR_DEBUGFS_GEM_USAGE_FLAG_FW_MAPPED;
-
- bo = panthor_gem_create(&ptdev->base, size, bo_flags, vm, debug_flags);
- if (IS_ERR(bo)) {
- ret = PTR_ERR(bo);
- goto err_free_kbo;
- }
-
+ drm_gem_object_get(&bo->base);
kbo->obj = &bo->base;
if (vm == panthor_fw_vm(ptdev)) {
@@ -1319,24 +1307,27 @@ panthor_kernel_bo_create(struct panthor_device *ptdev, struct panthor_vm *vm,
goto err_put_obj;
}
- 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
* expected to be used for another section.
* Make sure we never map more than we need.
*/
- size = ALIGN(size, panthor_vm_page_size(vm));
- ret = panthor_vm_alloc_va(vm, gpu_va, size, &kbo->va_node);
+ vm_map_size = ALIGN(vm_map_size, panthor_vm_page_size(vm));
+ ret = panthor_vm_alloc_va(vm, gpu_va, vm_map_size, &kbo->va_node);
if (ret)
goto err_unpin;
- ret = panthor_vm_map_bo_range(vm, bo, 0, size, kbo->va_node.start, vm_map_flags);
+ ret = panthor_vm_map_bo_range(vm, bo, 0, vm_map_size,
+ kbo->va_node.start, vm_map_flags);
if (ret)
goto err_free_va;
kbo->vm = panthor_vm_get(vm);
+ if (vm == panthor_fw_vm(ptdev))
+ panthor_gem_debugfs_add_usage_flags(
+ bo, PANTHOR_DEBUGFS_GEM_USAGE_FLAG_FW_MAPPED);
+
return kbo;
err_free_va:
@@ -1348,12 +1339,55 @@ panthor_kernel_bo_create(struct panthor_device *ptdev, struct panthor_vm *vm,
err_put_obj:
drm_gem_object_put(&bo->base);
-
-err_free_kbo:
kfree(kbo);
return ERR_PTR(ret);
}
+/**
+ * panthor_kernel_bo_create() - Create and map a GEM object to a VM
+ * @ptdev: Device.
+ * @vm: VM to map the GEM to.
+ * @size: Size of the buffer object.
+ * @bo_flags: Combination of drm_panthor_bo_flags flags.
+ * @vm_map_flags: Combination of drm_panthor_vm_bind_op_flags (only those
+ * that are related to map operations).
+ * @gpu_va: GPU address assigned when mapping to the VM.
+ * If gpu_va == PANTHOR_VM_KERNEL_AUTO_VA, the virtual address will be
+ * automatically allocated.
+ * @name: Descriptive label of the BO's contents
+ *
+ * Return: A valid pointer in case of success, an ERR_PTR() otherwise.
+ */
+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, const char *name)
+{
+ struct panthor_kernel_bo *kbo;
+ struct panthor_gem_object *bo;
+
+ if (drm_WARN_ON(&ptdev->base, !vm))
+ return ERR_PTR(-EINVAL);
+
+ bo = panthor_gem_create(&ptdev->base, size, bo_flags, vm, 0);
+ if (IS_ERR(bo))
+ return ERR_CAST(bo);
+
+ kbo = panthor_kernel_bo_create_from_gem(ptdev, vm, bo, vm_map_flags,
+ gpu_va, size);
+ if (!IS_ERR(kbo)) {
+ panthor_gem_debugfs_add_usage_flags(
+ bo, PANTHOR_DEBUGFS_GEM_USAGE_FLAG_KERNEL);
+ panthor_gem_kernel_bo_set_label(kbo, name);
+ }
+
+ /* panthor_kernel_bo_create_from_gem() acquires a GEM ref if the import
+ * succeeds, so we can release it unconditionally here.
+ */
+ drm_gem_object_put(&bo->base);
+ return kbo;
+}
+
/**
* panthor_dummy_bo_create() - Create a Panthor BO meant to back sparse bindings.
* @ptdev: Device.
diff --git a/drivers/gpu/drm/panthor/panthor_gem.h b/drivers/gpu/drm/panthor/panthor_gem.h
index 4da3f59693ce4..03dd3927b5d16 100644
--- a/drivers/gpu/drm/panthor/panthor_gem.h
+++ b/drivers/gpu/drm/panthor/panthor_gem.h
@@ -318,6 +318,13 @@ panthor_kernel_bo_vunmap(struct panthor_kernel_bo *bo)
}
}
+struct panthor_kernel_bo *
+panthor_kernel_bo_create_from_gem(struct panthor_device *ptdev,
+ struct panthor_vm *vm,
+ struct panthor_gem_object *bo,
+ u32 vm_map_flags, u64 gpu_va,
+ u32 vm_map_size);
+
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,
--
2.43.0
^ permalink raw reply related [flat|nested] 18+ messages in thread* Re: [PATCH v4 09/12] drm/panthor: Create kernel BO from existing GEM
2026-10-12 12:59 ` [PATCH v4 09/12] drm/panthor: Create kernel BO from existing GEM Ketil Johnsen
@ 2026-10-12 13:12 ` sashiko-bot
0 siblings, 0 replies; 18+ messages in thread
From: sashiko-bot @ 2026-10-12 13:12 UTC (permalink / raw)
To: Ketil Johnsen; +Cc: dri-devel
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Medium] drm/panthor: integer truncation of size_t to u32 in panthor_kernel_bo_create_from_gem()
- [Low] drm/panthor: unused usage_flags parameter in panthor_gem_create()
--
commit e696a47bb56852d031f4d4fbdb9474523f19a3f1
Author: Boris Brezillon <boris.brezillon@collabora.com>
drm/panthor: Create kernel BO from existing GEM
Refactors/splits existing panthor_kernel_bo_create(). Adds
panthor_kernel_bo_create_from_gem() so we can create kernel BOs
from imported protected memory in future change.
> diff --git a/drivers/gpu/drm/panthor/panthor_gem.c b/drivers/gpu/drm/panthor/panthor_gem.c
> index 0af4ae47780be..ec6eba8e2ac3a 100644
> --- a/drivers/gpu/drm/panthor/panthor_gem.c
> +++ b/drivers/gpu/drm/panthor/panthor_gem.c
[ ... ]
> @@ -1036,7 +1036,6 @@ panthor_gem_create(struct drm_device *dev, size_t size, uint32_t flags,
> bo->base.resv = bo->exclusive_vm_root_gem->resv;
> }
>
> - panthor_gem_debugfs_add_usage_flags(bo, usage_flags);
> panthor_gem_debugfs_bo_add(bo);
> return bo;
[Severity: Low]
Since the call to panthor_gem_debugfs_add_usage_flags() is removed here, does
this leave the usage_flags parameter entirely unused in panthor_gem_create()?
Should the parameter be removed from the function signature as well to avoid
dead code?
[ ... ]
> @@ -1274,46 +1275,33 @@ void panthor_kernel_bo_destroy(struct panthor_kernel_bo *bo)
> }
>
> /**
> - * panthor_kernel_bo_create() - Create and map a GEM object to a VM
> + * panthor_kernel_bo_create_from_gem() - Create kernel BO from existing GEM object
[ ... ]
> 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, const char *name)
> +panthor_kernel_bo_create_from_gem(struct panthor_device *ptdev,
> + struct panthor_vm *vm,
> + struct panthor_gem_object *bo,
> + u32 vm_map_flags, u64 gpu_va, u32 vm_map_size)
[Severity: Medium]
Does this cause an implicit integer truncation on 64-bit platforms?
The panthor_kernel_bo_create() function takes size as a size_t (which is 64
bits wide), but passes it to vm_map_size here, which is declared as a u32:
panthor_kernel_bo_create(..., size_t size, ...)
{
...
kbo = panthor_kernel_bo_create_from_gem(ptdev, vm, bo, vm_map_flags,
gpu_va, size);
...
}
Could this lead to silent partial allocations or out-of-bounds accesses if a
kernel BO size ever exceeds 4GB?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20261012125940.3291951-1-ketil.johnsen@arm.com?part=9
^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH v4 10/12] drm/panthor: Support FW init of protm
2026-10-12 12:59 [PATCH v4 00/12] drm/panthor: Protected mode support for Mali CSF GPUs Ketil Johnsen
` (8 preceding siblings ...)
2026-10-12 12:59 ` [PATCH v4 09/12] drm/panthor: Create kernel BO from existing GEM Ketil Johnsen
@ 2026-10-12 12:59 ` Ketil Johnsen
2026-10-12 13:15 ` sashiko-bot
2026-10-12 12:59 ` [PATCH v4 11/12] drm/panthor: Protm support for groups Ketil Johnsen
2026-10-12 12:59 ` [PATCH v4 12/12] drm/panthor: Bump version to expose protm feature Ketil Johnsen
11 siblings, 1 reply; 18+ messages in thread
From: Ketil Johnsen @ 2026-10-12 12:59 UTC (permalink / raw)
To: Boris Brezillon, Steven Price, Liviu Dudau, Maarten Lankhorst,
Maxime Ripard, Thomas Zimmermann, David Airlie, Simona Vetter,
Daniel Almeida, Alice Ryhl
Cc: dri-devel, linux-kernel, Ketil Johnsen
From: Boris Brezillon <boris.brezillon@collabora.com>
The Mali GPU FW needs some protected memory when executing in protected
mode. This FW memory section is assigned a VA during device init.
A user space process with the needed privileges (CAP_SYS_MODULE) must
provide a suitable memory buffer before the Mali GPU is capable of
executing in protected mode.
Adds user query of FW protm state, min FW section size and min protm
suspend buffer size.
Adds user initiated init of FW protm section.
Signed-off-by: Boris Brezillon <boris.brezillon@collabora.com>
Co-developed-by: Ketil Johnsen <ketil.johnsen@arm.com>
Signed-off-by: Ketil Johnsen <ketil.johnsen@arm.com>
---
v4:
- First version of this change as a separate commit
---
drivers/gpu/drm/panthor/panthor_device.h | 3 +
drivers/gpu/drm/panthor/panthor_drv.c | 18 +++-
drivers/gpu/drm/panthor/panthor_fw.c | 107 +++++++++++++++++++++--
drivers/gpu/drm/panthor/panthor_fw.h | 2 +
include/uapi/drm/panthor_drm.h | 75 ++++++++++++++++
5 files changed, 197 insertions(+), 8 deletions(-)
diff --git a/drivers/gpu/drm/panthor/panthor_device.h b/drivers/gpu/drm/panthor/panthor_device.h
index 4362c55bcd7e4..99e5174cf0e50 100644
--- a/drivers/gpu/drm/panthor/panthor_device.h
+++ b/drivers/gpu/drm/panthor/panthor_device.h
@@ -343,6 +343,9 @@ struct panthor_device {
/** @protm_exit_count: Number of times we exited protm. */
atomic64_t protm_exit_count;
+
+ /** @info: Protected mode info. */
+ struct drm_panthor_protm_info info;
} protm;
};
diff --git a/drivers/gpu/drm/panthor/panthor_drv.c b/drivers/gpu/drm/panthor/panthor_drv.c
index 51a2e5358cd51..cc1c050ea3806 100644
--- a/drivers/gpu/drm/panthor/panthor_drv.c
+++ b/drivers/gpu/drm/panthor/panthor_drv.c
@@ -177,11 +177,13 @@ panthor_get_uobj_array(const struct drm_panthor_obj_array *in, u32 min_stride,
PANTHOR_UOBJ_DECL(struct drm_panthor_mmu_info, page_size_bitmap), \
PANTHOR_UOBJ_DECL(struct drm_panthor_timestamp_info, current_timestamp), \
PANTHOR_UOBJ_DECL(struct drm_panthor_group_priorities_info, pad), \
+ PANTHOR_UOBJ_DECL(struct drm_panthor_protm_info, pad), \
PANTHOR_UOBJ_DECL(struct drm_panthor_sync_op, timeline_value), \
PANTHOR_UOBJ_DECL(struct drm_panthor_queue_submit, syncs), \
PANTHOR_UOBJ_DECL(struct drm_panthor_queue_create, ringbuf_size), \
PANTHOR_UOBJ_DECL(struct drm_panthor_vm_bind_op, syncs), \
- PANTHOR_UOBJ_DECL(struct drm_panthor_bo_sync_op, size))
+ PANTHOR_UOBJ_DECL(struct drm_panthor_bo_sync_op, size), \
+ PANTHOR_UOBJ_DECL(struct drm_panthor_protm_init, pad))
/**
* PANTHOR_UOBJ_SET() - Copy a kernel object to a user object.
@@ -958,6 +960,10 @@ static int panthor_ioctl_dev_query(struct drm_device *ddev, void *data, struct d
args->size = sizeof(ptdev->mmu_info);
return 0;
+ case DRM_PANTHOR_DEV_QUERY_PROTM_INFO:
+ args->size = sizeof(ptdev->protm.info);
+ return 0;
+
default:
return -EINVAL;
}
@@ -991,6 +997,9 @@ static int panthor_ioctl_dev_query(struct drm_device *ddev, void *data, struct d
case DRM_PANTHOR_DEV_QUERY_MMU_INFO:
return PANTHOR_UOBJ_SET(args->pointer, args->size, ptdev->mmu_info);
+ case DRM_PANTHOR_DEV_QUERY_PROTM_INFO:
+ return PANTHOR_UOBJ_SET(args->pointer, args->size, ptdev->protm.info);
+
default:
return -EINVAL;
}
@@ -1588,6 +1597,12 @@ static int panthor_ioctl_bo_query_info(struct drm_device *ddev, void *data,
return 0;
}
+static int panthor_ioctl_protm_init(struct drm_device *ddev, void *data,
+ struct drm_file *file)
+{
+ return panthor_fw_protm_init(file, data);
+}
+
static int
panthor_open(struct drm_device *ddev, struct drm_file *file)
{
@@ -1664,6 +1679,7 @@ static const struct drm_ioctl_desc panthor_drm_driver_ioctls[] = {
PANTHOR_IOCTL(SET_USER_MMIO_OFFSET, set_user_mmio_offset, DRM_RENDER_ALLOW),
PANTHOR_IOCTL(BO_SYNC, bo_sync, DRM_RENDER_ALLOW),
PANTHOR_IOCTL(BO_QUERY_INFO, bo_query_info, DRM_RENDER_ALLOW),
+ PANTHOR_IOCTL(PROTM_INIT, protm_init, DRM_RENDER_ALLOW),
};
static int panthor_mmap(struct file *filp, struct vm_area_struct *vma)
diff --git a/drivers/gpu/drm/panthor/panthor_fw.c b/drivers/gpu/drm/panthor/panthor_fw.c
index fc07796fc9ef7..0af6addfd55a5 100644
--- a/drivers/gpu/drm/panthor/panthor_fw.c
+++ b/drivers/gpu/drm/panthor/panthor_fw.c
@@ -13,8 +13,10 @@
#include <linux/mutex.h>
#include <linux/platform_device.h>
#include <linux/pm_runtime.h>
+#include <linux/capability.h>
#include <drm/drm_drv.h>
+#include <drm/drm_file.h>
#include <drm/drm_managed.h>
#include <drm/drm_print.h>
@@ -252,6 +254,9 @@ struct panthor_fw {
/** @shared_section: The section containing the FW interfaces. */
struct panthor_fw_section *shared_section;
+ /** @protm_section: The protected mode section. */
+ struct panthor_fw_section *protm_section;
+
/** @iface: FW interfaces. */
struct panthor_fw_iface iface;
@@ -261,6 +266,9 @@ struct panthor_fw {
struct delayed_work ping_work;
} watchdog;
+ /** @protm_init_lock: Used to serialize protm initialization. */
+ struct mutex protm_init_lock;
+
/**
* @req_waitqueue: FW request waitqueue.
*
@@ -462,6 +470,9 @@ static void panthor_fw_init_section_mem(struct panthor_device *ptdev,
!(section->flags & CSF_FW_BINARY_IFACE_ENTRY_ZERO))
return;
+ if (!section->mem)
+ return;
+
ret = panthor_kernel_bo_vmap(section->mem);
if (drm_WARN_ON(&ptdev->base, ret))
return;
@@ -621,12 +632,6 @@ static int panthor_fw_load_section_entry(struct panthor_device *ptdev,
return -EINVAL;
}
- if (hdr.flags & CSF_FW_BINARY_IFACE_ENTRY_PROT) {
- drm_warn(&ptdev->base,
- "Firmware protected mode entry is not supported, ignoring");
- return 0;
- }
-
if (hdr.va.start == CSF_MCU_SHARED_REGION_START &&
!(hdr.flags & CSF_FW_BINARY_IFACE_ENTRY_SHARED)) {
drm_err(&ptdev->base,
@@ -678,7 +683,22 @@ static int panthor_fw_load_section_entry(struct panthor_device *ptdev,
section->size = section_size;
section->va = hdr.va.start;
- if (section_size) {
+ if (hdr.flags & CSF_FW_BINARY_IFACE_ENTRY_PROT) {
+ if (ptdev->fw->protm_section) {
+ drm_err(&ptdev->base,
+ "Only one protected section supported\n");
+ return -EINVAL;
+ }
+
+ ptdev->protm.info.fw_protected_sections_size =
+ ALIGN(section->size, vm_pgsz);
+ ptdev->fw->protm_section = section;
+ }
+
+ /* Defer the section->mem creation if this is a protected entry.
+ * This will be populated when DRM_IOCTL_PANTHOR_PROTM_INIT is called.
+ */
+ if (section->size && !(hdr.flags & CSF_FW_BINARY_IFACE_ENTRY_PROT)) {
u32 vm_map_flags = section_vm_map_flags(section);
section->mem = panthor_kernel_bo_create(
@@ -985,6 +1005,11 @@ static int panthor_init_csg_iface(struct panthor_device *ptdev,
return -EINVAL;
}
+ if (!csg_idx) {
+ ptdev->protm.info.group_protected_suspend_buf_size =
+ csg_iface->control->protm_suspend_size;
+ }
+
if (csg_idx > 0) {
struct panthor_fw_csg_iface *first_csg_iface =
panthor_fw_get_csg_iface(ptdev, 0);
@@ -1585,6 +1610,70 @@ int panthor_fw_protm_exit(struct panthor_device *ptdev, u32 timeout_ms)
return panthor_fw_protm_exit_wait(ptdev, timeout_ms);
}
+int panthor_fw_protm_init(struct drm_file *file,
+ struct drm_panthor_protm_init *args)
+{
+ struct panthor_file *pfile = file->driver_priv;
+ struct panthor_device *ptdev = pfile->ptdev;
+ struct panthor_fw_section *protm_section = ptdev->fw->protm_section;
+ struct drm_gem_object *obj;
+ u32 vm_map_flags;
+ int cookie, ret = 0;
+
+ if (!capable(CAP_SYS_MODULE))
+ return -EPERM;
+
+ if (args->pad)
+ return -EINVAL;
+
+ if (!protm_section || !protm_section->size)
+ return -EINVAL;
+
+ guard(mutex)(&ptdev->fw->protm_init_lock);
+
+ /* Thread safe access so other readers do not need protm_init_lock */
+ if (smp_load_acquire(&ptdev->protm.info.state) &
+ DRM_PANTHOR_PROTM_INITIALIZED)
+ return 0;
+
+ if (!drm_dev_enter(&ptdev->base, &cookie))
+ return -ENODEV;
+
+ obj = drm_gem_object_lookup(file,
+ args->fw_protected_sections_bo_handle);
+ if (!obj) {
+ ret = -ENOENT;
+ goto out_dev_exit;
+ }
+
+ if (obj->size < ptdev->protm.info.fw_protected_sections_size) {
+ ret = -EINVAL;
+ goto out_gem_put;
+ }
+
+ vm_map_flags = section_vm_map_flags(protm_section);
+
+ protm_section->mem = panthor_kernel_bo_create_from_gem(
+ ptdev, panthor_fw_vm(ptdev), to_panthor_bo(obj), vm_map_flags,
+ protm_section->va, protm_section->size);
+ if (IS_ERR(protm_section->mem)) {
+ ret = PTR_ERR(protm_section->mem);
+ protm_section->mem = NULL;
+ goto out_gem_put;
+ }
+
+ /* Make sure all above is completed before we update state */
+ smp_store_release(&ptdev->protm.info.state,
+ DRM_PANTHOR_PROTM_INITIALIZED);
+
+out_gem_put:
+ drm_gem_object_put(obj);
+
+out_dev_exit:
+ drm_dev_exit(cookie);
+ return ret;
+}
+
/**
* panthor_fw_init() - Initialize FW related data.
* @ptdev: Device.
@@ -1606,6 +1695,10 @@ int panthor_fw_init(struct panthor_device *ptdev)
INIT_LIST_HEAD(&fw->sections);
INIT_DELAYED_WORK(&fw->watchdog.ping_work, panthor_fw_ping_work);
+ ret = drmm_mutex_init(&ptdev->base, &fw->protm_init_lock);
+ if (ret)
+ return ret;
+
irq = platform_get_irq_byname(to_platform_device(ptdev->base.dev), "job");
if (irq <= 0)
return -ENODEV;
diff --git a/drivers/gpu/drm/panthor/panthor_fw.h b/drivers/gpu/drm/panthor/panthor_fw.h
index 4eda8f8e714c1..34bc9dc82314c 100644
--- a/drivers/gpu/drm/panthor/panthor_fw.h
+++ b/drivers/gpu/drm/panthor/panthor_fw.h
@@ -529,6 +529,8 @@ static inline int panthor_fw_resume(struct panthor_device *ptdev)
int panthor_fw_init(struct panthor_device *ptdev);
void panthor_fw_unplug(struct panthor_device *ptdev);
+int panthor_fw_protm_init(struct drm_file *file,
+ struct drm_panthor_protm_init *args);
int panthor_fw_protm_enter(struct panthor_device *ptdev);
int panthor_fw_protm_exit(struct panthor_device *ptdev, u32 timeout_ms);
int panthor_fw_protm_exit_wait(struct panthor_device *ptdev, u32 timeout_ms);
diff --git a/include/uapi/drm/panthor_drm.h b/include/uapi/drm/panthor_drm.h
index a2ff0f4ec6915..d2d21e3e6bb39 100644
--- a/include/uapi/drm/panthor_drm.h
+++ b/include/uapi/drm/panthor_drm.h
@@ -154,6 +154,11 @@ enum drm_panthor_ioctl_id {
* This is useful for imported BOs.
*/
DRM_PANTHOR_BO_QUERY_INFO,
+
+ /**
+ * @DRM_PANTHOR_PROTM_INIT: Device-wide initialize of protected mode.
+ */
+ DRM_PANTHOR_PROTM_INIT,
};
/**
@@ -256,6 +261,11 @@ enum drm_panthor_dev_query_type {
/** @DRM_PANTHOR_DEV_QUERY_MMU_INFO: Query MMU information. */
DRM_PANTHOR_DEV_QUERY_MMU_INFO,
+
+ /**
+ * @DRM_PANTHOR_DEV_QUERY_PROTM_INFO: Query supported protected rendering information.
+ */
+ DRM_PANTHOR_DEV_QUERY_PROTM_INFO,
};
/**
@@ -517,6 +527,51 @@ struct drm_panthor_group_priorities_info {
__u8 pad[3];
};
+/**
+ * enum drm_panthor_protm_state_flags - Describes the state of the protected mode feature.
+ *
+ * List of GPU states which can be used by the GPU to access protected memory.
+ */
+enum drm_panthor_protm_state_flags {
+ /**
+ * @DRM_PANTHOR_PROTM_INITIALIZED: Device-wide initialization of the
+ * protected mode feature is done.
+ */
+ DRM_PANTHOR_PROTM_INITIALIZED = 1 << 0,
+};
+
+/**
+ * struct drm_panthor_protm_info - Protected mode info.
+ *
+ * Structure grouping all queryable information relating to protected mode.
+ */
+struct drm_panthor_protm_info {
+ /**
+ * @state: Combination of enum drm_panthor_protm_state_flags flags.
+ */
+ __u32 state;
+
+ /**
+ * @fw_protected_sections_size: Size of all the protected FW sections.
+ *
+ * Size of the protected buffer to pass through
+ * DRM_IOCTL_PANTHOR_PROTM_INIT.
+ */
+ __u32 fw_protected_sections_size;
+
+ /**
+ * @group_protected_suspend_buf_size: Size of the group suspend buffer.
+ *
+ * This must be used to allocate a protected BO that's big enough to use
+ * as a protected suspend buffer when a group supports protected
+ * rendering.
+ */
+ __u32 group_protected_suspend_buf_size;
+
+ /** @pad: MBZ. */
+ __u32 pad;
+};
+
/**
* struct drm_panthor_dev_query - Arguments passed to DRM_PANTHOR_IOCTL_DEV_QUERY
*/
@@ -1270,6 +1325,24 @@ struct drm_panthor_bo_query_info {
__u32 pad;
};
+/**
+ * struct drm_panthor_protm_init - Protected mode initialization arguments.
+ */
+struct drm_panthor_protm_init {
+ /**
+ * @fw_protected_sections_bo_handle: Handle of the BO to use for the FW protected
+ * sections.
+ *
+ * This BO must have been allocated from a protected DMA-BUF heap and
+ * imported in panthor. It's size must be at least
+ * drm_panthor_protm_info::fw_protected_sections_size.
+ */
+ __u32 fw_protected_sections_bo_handle;
+
+ /** @pad: MBZ. */
+ __u32 pad;
+};
+
/**
* DRM_IOCTL_PANTHOR() - Build a Panthor IOCTL number
* @__access: Access type. Must be R, W or RW.
@@ -1320,6 +1393,8 @@ enum {
DRM_IOCTL_PANTHOR(WR, BO_SYNC, bo_sync),
DRM_IOCTL_PANTHOR_BO_QUERY_INFO =
DRM_IOCTL_PANTHOR(WR, BO_QUERY_INFO, bo_query_info),
+ DRM_IOCTL_PANTHOR_PROTM_INIT =
+ DRM_IOCTL_PANTHOR(WR, PROTM_INIT, protm_init),
};
#if defined(__cplusplus)
--
2.43.0
^ permalink raw reply related [flat|nested] 18+ messages in thread* Re: [PATCH v4 10/12] drm/panthor: Support FW init of protm
2026-10-12 12:59 ` [PATCH v4 10/12] drm/panthor: Support FW init of protm Ketil Johnsen
@ 2026-10-12 13:15 ` sashiko-bot
0 siblings, 0 replies; 18+ messages in thread
From: sashiko-bot @ 2026-10-12 13:15 UTC (permalink / raw)
To: Ketil Johnsen; +Cc: dri-devel
Thank you for your contribution! Sashiko AI review found 5 potential issue(s) to consider:
- [Critical] drm: kernel crash on FW reload due to mapping protected PROT section
- [High] drm: data race on protm_section->mem causes ERR_PTR dereference
- [Medium] drm: mixing goto-based cleanup and guard() in panthor_fw_protm_init
- [Medium] drm/panthor: missing VA reservation for PROT section allows auto-VA clash
- [Low] drm: data race in panthor_ioctl_dev_query copying PROTM_INFO
--
commit 6a74417abb2feb49f589b33202b41b8e8a067ce4
Author: Boris Brezillon <boris.brezillon@collabora.com>
Subject: drm/panthor: Support FW init of protm
This commit adds support for initializing the Mali GPU firmware's protected
mode (protm) memory section. It introduces user queries for protm state and a
new ioctl for user-initiated protected mode initialization.
> diff --git a/drivers/gpu/drm/panthor/panthor_drv.c b/drivers/gpu/drm/panthor/panthor_drv.c
> index 51a2e5358cd51..cc1c050ea3806 100644
> --- a/drivers/gpu/drm/panthor/panthor_drv.c
> +++ b/drivers/gpu/drm/panthor/panthor_drv.c
[ ... ]
> @@ -991,6 +997,9 @@ static int panthor_ioctl_dev_query(struct drm_device *ddev, void *data, struct d
> case DRM_PANTHOR_DEV_QUERY_MMU_INFO:
> return PANTHOR_UOBJ_SET(args->pointer, args->size, ptdev->mmu_info);
>
> + case DRM_PANTHOR_DEV_QUERY_PROTM_INFO:
> + return PANTHOR_UOBJ_SET(args->pointer, args->size, ptdev->protm.info);
[Severity: Low]
Is it safe to directly pass ptdev->protm.info to PANTHOR_UOBJ_SET() here?
Since panthor_fw_protm_init() updates the state field concurrently using
smp_store_release(), this byte/word copy in copy_to_user() might race
and copy torn state to userspace.
Should this create a local copy using smp_load_acquire() before passing it
to copy_to_user(), similar to how timestamp_info is handled?
> +
> default:
> return -EINVAL;
> }
> diff --git a/drivers/gpu/drm/panthor/panthor_fw.c b/drivers/gpu/drm/panthor/panthor_fw.c
> index fc07796fc9ef7..0af6addfd55a5 100644
> --- a/drivers/gpu/drm/panthor/panthor_fw.c
> +++ b/drivers/gpu/drm/panthor/panthor_fw.c
[ ... ]
> @@ -462,6 +470,9 @@ static void panthor_fw_init_section_mem(struct panthor_device *ptdev,
> !(section->flags & CSF_FW_BINARY_IFACE_ENTRY_ZERO))
> return;
>
> + if (!section->mem)
> + return;
[Severity: High]
Does this check properly handle the error pointer state?
If panthor_fw_protm_init() is currently executing concurrently,
section->mem might briefly contain an ERR_PTR if
panthor_kernel_bo_create_from_gem() fails.
Since the reset worker reads section->mem without holding the
protm_init_lock, could it observe the intermediate ERR_PTR, bypass this
check, and crash when panthor_kernel_bo_vmap() dereferences it?
> +
> ret = panthor_kernel_bo_vmap(section->mem);
> if (drm_WARN_ON(&ptdev->base, ret))
> return;
>
> memcpy(section->mem->kmap, section->data.buf, section->data.size);
[Severity: Critical]
Will this cause a secure fault (SError) or kernel panic during a GPU reset?
If PROTM_INIT was successfully called by userspace, section->mem now
holds protected memory. During a slow reset, panthor_reload_fw_sections()
iterates over all firmware sections and unconditionally calls
panthor_fw_init_section_mem().
If it processes the PROT section, it will attempt to vmap and
memset/memcpy the protected memory from the CPU non-secure world. Should
this function or the reload loop explicitly skip the
CSF_FW_BINARY_IFACE_ENTRY_PROT section to avoid a secure fault?
[ ... ]
> @@ -678,7 +683,22 @@ static int panthor_fw_load_section_entry(struct panthor_device *ptdev,
> section->size = section_size;
> section->va = hdr.va.start;
>
> - if (section_size) {
> + if (hdr.flags & CSF_FW_BINARY_IFACE_ENTRY_PROT) {
> + if (ptdev->fw->protm_section) {
> + drm_err(&ptdev->base,
> + "Only one protected section supported\n");
> + return -EINVAL;
> + }
> +
> + ptdev->protm.info.fw_protected_sections_size =
> + ALIGN(section->size, vm_pgsz);
> + ptdev->fw->protm_section = section;
> + }
> +
> + /* Defer the section->mem creation if this is a protected entry.
> + * This will be populated when DRM_IOCTL_PANTHOR_PROTM_INIT is called.
> + */
> + if (section->size && !(hdr.flags & CSF_FW_BINARY_IFACE_ENTRY_PROT)) {
[Severity: Medium]
Does skipping the memory allocation here also unintentionally skip reserving
the virtual address in the firmware VM?
If the firmware binary places the protected mode (PROT) section within the
kernel_auto_va range, and we skip panthor_kernel_bo_create() (which
allocates the VA), could an unprivileged user allocating queues or suspend
buffers steal this unreserved VA?
This would later cause panthor_fw_protm_init() to fail with -ENOSPC when
it attempts to reserve the exact VA. Should the VA still be reserved early,
even if memory allocation is deferred?
> u32 vm_map_flags = section_vm_map_flags(section);
>
> section->mem = panthor_kernel_bo_create(
[ ... ]
> @@ -1585,6 +1610,70 @@ int panthor_fw_protm_exit(struct panthor_device *ptdev, u32 timeout_ms)
> return panthor_fw_protm_exit_wait(ptdev, timeout_ms);
> }
>
> +int panthor_fw_protm_init(struct drm_file *file,
> + struct drm_panthor_protm_init *args)
> +{
> + struct panthor_file *pfile = file->driver_priv;
> + struct panthor_device *ptdev = pfile->ptdev;
> + struct panthor_fw_section *protm_section = ptdev->fw->protm_section;
> + struct drm_gem_object *obj;
> + u32 vm_map_flags;
> + int cookie, ret = 0;
> +
> + if (!capable(CAP_SYS_MODULE))
> + return -EPERM;
> +
> + if (args->pad)
> + return -EINVAL;
> +
> + if (!protm_section || !protm_section->size)
> + return -EINVAL;
> +
> + guard(mutex)(&ptdev->fw->protm_init_lock);
[Severity: Medium]
This isn't a bug, but this function mixes scope-based cleanup
(guard(mutex)) with traditional goto cleanup labels (like
goto out_gem_put).
According to the cleanup helpers documentation, goto and scope-based
cleanup should not be mixed in the same function. Could all resources in
this function be converted to use scope-based cleanup to avoid ownership
confusion?
> +
> + /* Thread safe access so other readers do not need protm_init_lock */
> + if (smp_load_acquire(&ptdev->protm.info.state) &
> + DRM_PANTHOR_PROTM_INITIALIZED)
> + return 0;
> +
> + if (!drm_dev_enter(&ptdev->base, &cookie))
> + return -ENODEV;
> +
> + obj = drm_gem_object_lookup(file,
> + args->fw_protected_sections_bo_handle);
> + if (!obj) {
> + ret = -ENOENT;
> + goto out_dev_exit;
> + }
> +
> + if (obj->size < ptdev->protm.info.fw_protected_sections_size) {
> + ret = -EINVAL;
> + goto out_gem_put;
> + }
> +
> + vm_map_flags = section_vm_map_flags(protm_section);
> +
> + protm_section->mem = panthor_kernel_bo_create_from_gem(
> + ptdev, panthor_fw_vm(ptdev), to_panthor_bo(obj), vm_map_flags,
> + protm_section->va, protm_section->size);
> + if (IS_ERR(protm_section->mem)) {
> + ret = PTR_ERR(protm_section->mem);
> + protm_section->mem = NULL;
[Severity: High]
Is it safe to assign directly to the globally visible protm_section->mem
before checking for errors?
If this allocation fails, protm_section->mem briefly holds an ERR_PTR
until the error check clears it. A concurrent GPU reset worker, which reads
section->mem without holding protm_init_lock, could observe this
intermediate state and crash when trying to vmap it.
Should the result be stored in a local variable and only assigned to
protm_section->mem upon success?
> + goto out_gem_put;
> + }
> +
> + /* Make sure all above is completed before we update state */
> + smp_store_release(&ptdev->protm.info.state,
> + DRM_PANTHOR_PROTM_INITIALIZED);
> +
> +out_gem_put:
> + drm_gem_object_put(obj);
> +
> +out_dev_exit:
> + drm_dev_exit(cookie);
> + return ret;
> +}
--
Sashiko AI review · https://sashiko.dev/#/patchset/20261012125940.3291951-1-ketil.johnsen@arm.com?part=10
^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH v4 11/12] drm/panthor: Protm support for groups
2026-10-12 12:59 [PATCH v4 00/12] drm/panthor: Protected mode support for Mali CSF GPUs Ketil Johnsen
` (9 preceding siblings ...)
2026-10-12 12:59 ` [PATCH v4 10/12] drm/panthor: Support FW init of protm Ketil Johnsen
@ 2026-10-12 12:59 ` Ketil Johnsen
2026-10-12 13:15 ` sashiko-bot
2026-10-12 12:59 ` [PATCH v4 12/12] drm/panthor: Bump version to expose protm feature Ketil Johnsen
11 siblings, 1 reply; 18+ messages in thread
From: Ketil Johnsen @ 2026-10-12 12:59 UTC (permalink / raw)
To: Boris Brezillon, Steven Price, Liviu Dudau, Maarten Lankhorst,
Maxime Ripard, Thomas Zimmermann, David Airlie, Simona Vetter,
Daniel Almeida, Alice Ryhl
Cc: dri-devel, linux-kernel, Ketil Johnsen
From: Boris Brezillon <boris.brezillon@collabora.com>
Extends struct drm_panthor_group_create to allow user provided protm
suspend buffer. This is needed for protm execution with this group.
Make use of the user supplied suspend buffer (import).
Allow PROT_REGION instruction for groups which provided protm
suspend buffer.
Store the suspend buffer sizes (normal and protected) provided by FW
after FW start. Unsafe to read directly from FW as these will return
zero between FW load and FW start.
Signed-off-by: Boris Brezillon <boris.brezillon@collabora.com>
Co-developed-by: Ketil Johnsen <ketil.johnsen@arm.com>
Signed-off-by: Ketil Johnsen <ketil.johnsen@arm.com>
---
v4:
- First version of this change as a separate commit
---
drivers/gpu/drm/panthor/panthor_fw.c | 24 ++++++++++
drivers/gpu/drm/panthor/panthor_fw.h | 2 +
drivers/gpu/drm/panthor/panthor_sched.c | 59 ++++++++++++++++++++-----
include/uapi/drm/panthor_drm.h | 10 ++++-
4 files changed, 83 insertions(+), 12 deletions(-)
diff --git a/drivers/gpu/drm/panthor/panthor_fw.c b/drivers/gpu/drm/panthor/panthor_fw.c
index 0af6addfd55a5..11c370ef86aad 100644
--- a/drivers/gpu/drm/panthor/panthor_fw.c
+++ b/drivers/gpu/drm/panthor/panthor_fw.c
@@ -269,6 +269,12 @@ struct panthor_fw {
/** @protm_init_lock: Used to serialize protm initialization. */
struct mutex protm_init_lock;
+ /** @suspend_size: Size of regular suspend buffer */
+ u32 suspend_size;
+
+ /** @protm_suspend_size: Size of protected suspend buffer size */
+ u32 protm_suspend_size;
+
/**
* @req_waitqueue: FW request waitqueue.
*
@@ -1006,6 +1012,14 @@ static int panthor_init_csg_iface(struct panthor_device *ptdev,
}
if (!csg_idx) {
+ /* We make a copy of these sizes, as csg_iface can read these
+ * as zero during full GPU reset, between FW sections being
+ * reloaded and FW fully booted.
+ */
+ ptdev->fw->suspend_size = csg_iface->control->suspend_size;
+ ptdev->fw->protm_suspend_size =
+ csg_iface->control->protm_suspend_size;
+
ptdev->protm.info.group_protected_suspend_buf_size =
csg_iface->control->protm_suspend_size;
}
@@ -1523,6 +1537,16 @@ static void panthor_fw_ping_work(struct work_struct *work)
}
}
+u32 panthor_fw_suspend_size(struct panthor_device *ptdev)
+{
+ return ptdev->fw->suspend_size;
+}
+
+u32 panthor_fw_protm_suspend_size(struct panthor_device *ptdev)
+{
+ return ptdev->fw->protm_suspend_size;
+}
+
static bool wait_protm_enter(struct panthor_device *ptdev,
s64 protm_session_seqno)
{
diff --git a/drivers/gpu/drm/panthor/panthor_fw.h b/drivers/gpu/drm/panthor/panthor_fw.h
index 34bc9dc82314c..98576114cc803 100644
--- a/drivers/gpu/drm/panthor/panthor_fw.h
+++ b/drivers/gpu/drm/panthor/panthor_fw.h
@@ -531,6 +531,8 @@ void panthor_fw_unplug(struct panthor_device *ptdev);
int panthor_fw_protm_init(struct drm_file *file,
struct drm_panthor_protm_init *args);
+u32 panthor_fw_suspend_size(struct panthor_device *ptdev);
+u32 panthor_fw_protm_suspend_size(struct panthor_device *ptdev);
int panthor_fw_protm_enter(struct panthor_device *ptdev);
int panthor_fw_protm_exit(struct panthor_device *ptdev, u32 timeout_ms);
int panthor_fw_protm_exit_wait(struct panthor_device *ptdev, u32 timeout_ms);
diff --git a/drivers/gpu/drm/panthor/panthor_sched.c b/drivers/gpu/drm/panthor/panthor_sched.c
index fff805fd794a5..d1178be178a48 100644
--- a/drivers/gpu/drm/panthor/panthor_sched.c
+++ b/drivers/gpu/drm/panthor/panthor_sched.c
@@ -1462,10 +1462,15 @@ cs_slot_process_protm_pending_event_locked(struct panthor_device *ptdev,
if (!group)
return;
- /* Do not allow user space work to switch into protected mode, as we
- * do not fully support this quite yet.
+ /* Do not allow user space work to switch into protected mode if we
+ * do not support protected mode on this device.
+ * User space should query (and init) this support before attempting
+ * to use such GPU instructions.
*/
- atomic_or(BIT(cs_id), &group->fatal_queues);
+ if (group->protm_suspend_buf)
+ atomic_or(BIT(cs_id), &group->protm_pending_queues);
+ else
+ atomic_or(BIT(cs_id), &group->fatal_queues);
sched_queue_delayed_work(sched, tick, 0);
}
@@ -4074,6 +4079,7 @@ static void add_group_kbo_sizes(struct panthor_device *ptdev,
}
#define MAX_GROUPS_PER_POOL 128
+#define GROUP_CREATE_FLAGS DRM_PANTHOR_GROUP_CREATE_PROTECTED
int panthor_group_create(struct drm_file *file,
const struct drm_panthor_group_create *group_args,
@@ -4083,14 +4089,10 @@ int panthor_group_create(struct drm_file *file,
struct panthor_device *ptdev = pfile->ptdev;
struct panthor_group_pool *gpool = pfile->groups;
struct panthor_scheduler *sched = ptdev->scheduler;
- struct panthor_fw_csg_iface *csg_iface = panthor_fw_get_csg_iface(ptdev, 0);
struct panthor_group *group = NULL;
- u32 gid, i, suspend_size;
+ u32 gid, i;
int ret;
- if (group_args->pad)
- return -EINVAL;
-
if (group_args->priority >= PANTHOR_CSG_PRIORITY_COUNT)
return -EINVAL;
@@ -4136,14 +4138,51 @@ int panthor_group_create(struct drm_file *file,
goto err_put_group;
}
- suspend_size = csg_iface->control->suspend_size;
- group->suspend_buf = panthor_fw_alloc_suspend_buf_mem(ptdev, suspend_size);
+ group->suspend_buf = panthor_fw_alloc_suspend_buf_mem(
+ ptdev, panthor_fw_suspend_size(ptdev));
if (IS_ERR(group->suspend_buf)) {
ret = PTR_ERR(group->suspend_buf);
group->suspend_buf = NULL;
goto err_put_group;
}
+ if (group_args->protected_suspend_bo_handle) {
+ struct drm_gem_object *obj;
+
+ /* Read a correct snapshot of the state */
+ if (!(smp_load_acquire(&ptdev->protm.info.state) &
+ DRM_PANTHOR_PROTM_INITIALIZED)) {
+ ret = -EINVAL;
+ goto err_put_group;
+ }
+
+ obj = drm_gem_object_lookup(
+ file, group_args->protected_suspend_bo_handle);
+ if (!obj) {
+ ret = -ENOENT;
+ goto err_put_group;
+ }
+
+ if (obj->size <
+ ptdev->protm.info.group_protected_suspend_buf_size) {
+ drm_gem_object_put(obj);
+ ret = -EINVAL;
+ goto err_put_group;
+ }
+
+ group->protm_suspend_buf = panthor_kernel_bo_create_from_gem(
+ ptdev, panthor_fw_vm(ptdev), to_panthor_bo(obj),
+ DRM_PANTHOR_VM_BIND_OP_MAP_NOEXEC,
+ PANTHOR_VM_KERNEL_AUTO_VA,
+ panthor_fw_protm_suspend_size(ptdev));
+ drm_gem_object_put(obj);
+ if (IS_ERR(group->protm_suspend_buf)) {
+ ret = PTR_ERR(group->protm_suspend_buf);
+ group->protm_suspend_buf = NULL;
+ goto err_put_group;
+ }
+ }
+
group->syncobjs = panthor_kernel_bo_create(ptdev, group->vm,
group_args->queues.count *
sizeof(struct panthor_syncobj_64b),
diff --git a/include/uapi/drm/panthor_drm.h b/include/uapi/drm/panthor_drm.h
index d2d21e3e6bb39..a0ed16041e178 100644
--- a/include/uapi/drm/panthor_drm.h
+++ b/include/uapi/drm/panthor_drm.h
@@ -956,8 +956,14 @@ struct drm_panthor_group_create {
/** @priority: Group priority (see enum drm_panthor_group_priority). */
__u8 priority;
- /** @pad: Padding field, MBZ. */
- __u32 pad;
+ /**
+ * @protected_suspend_bo_handle: BO to use as a protected suspend buffer.
+ *
+ * This BO must have been allocated from a protected DMA-BUF heap and
+ * imported in panthor. It's size must be at least
+ * drm_panthor_protm_info::group_protected_suspend_buf_size.
+ */
+ __u32 protected_suspend_bo_handle;
/**
* @compute_core_mask: Mask encoding cores that can be used for compute jobs.
--
2.43.0
^ permalink raw reply related [flat|nested] 18+ messages in thread* Re: [PATCH v4 11/12] drm/panthor: Protm support for groups
2026-10-12 12:59 ` [PATCH v4 11/12] drm/panthor: Protm support for groups Ketil Johnsen
@ 2026-10-12 13:15 ` sashiko-bot
0 siblings, 0 replies; 18+ messages in thread
From: sashiko-bot @ 2026-10-12 13:15 UTC (permalink / raw)
To: Ketil Johnsen; +Cc: dri-devel
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] drm/panthor: missing validation of protected memory for PROTM suspend buffers
- [Low] drm: dead macro GROUP_CREATE_FLAGS referencing undefined identifier
--
commit a26512a2f35a2a18c407c42a57bf8210da106e39
Author: Boris Brezillon <boris.brezillon@collabora.com>
drm/panthor: Protm support for groups
This commit extends struct drm_panthor_group_create to allow a user-provided
protm suspend buffer. It imports the user-supplied suspend buffer and allows
the PROT_REGION instruction for groups that provide it. It also stores the
suspend buffer sizes provided by the FW.
> diff --git a/drivers/gpu/drm/panthor/panthor_sched.c b/drivers/gpu/drm/panthor/panthor_sched.c
> index 13c12a24e2f45..bfdabd7ee7beb 100644
> --- a/drivers/gpu/drm/panthor/panthor_sched.c
> +++ b/drivers/gpu/drm/panthor/panthor_sched.c
[ ... ]
> @@ -4074,6 +4079,7 @@ static void add_group_kbo_sizes(struct panthor_device *ptdev,
> }
>
> #define MAX_GROUPS_PER_POOL 128
> +#define GROUP_CREATE_FLAGS DRM_PANTHOR_GROUP_CREATE_PROTECTED
[Severity: Low]
Is this macro needed? The identifier DRM_PANTHOR_GROUP_CREATE_PROTECTED is not
defined anywhere in the codebase, and GROUP_CREATE_FLAGS is never used.
While it has no runtime effect, it is a compilation hazard if the macro is ever
evaluated and constitutes dead code.
>
> int panthor_group_create(struct drm_file *file,
> const struct drm_panthor_group_create *group_args,
[ ... ]
> @@ -4136,14 +4138,51 @@ int panthor_group_create(struct drm_file *file,
> goto err_put_group;
> }
>
> - suspend_size = csg_iface->control->suspend_size;
> - group->suspend_buf = panthor_fw_alloc_suspend_buf_mem(ptdev, suspend_size);
> + group->suspend_buf = panthor_fw_alloc_suspend_buf_mem(
> + ptdev, panthor_fw_suspend_size(ptdev));
> if (IS_ERR(group->suspend_buf)) {
> ret = PTR_ERR(group->suspend_buf);
> group->suspend_buf = NULL;
> goto err_put_group;
> }
>
> + if (group_args->protected_suspend_bo_handle) {
> + struct drm_gem_object *obj;
> +
> + /* Read a correct snapshot of the state */
> + if (!(smp_load_acquire(&ptdev->protm.info.state) &
> + DRM_PANTHOR_PROTM_INITIALIZED)) {
> + ret = -EINVAL;
> + goto err_put_group;
> + }
> +
> + obj = drm_gem_object_lookup(
> + file, group_args->protected_suspend_bo_handle);
> + if (!obj) {
> + ret = -ENOENT;
> + goto err_put_group;
> + }
> +
> + if (obj->size <
> + ptdev->protm.info.group_protected_suspend_buf_size) {
> + drm_gem_object_put(obj);
> + ret = -EINVAL;
> + goto err_put_group;
> + }
> +
> + group->protm_suspend_buf = panthor_kernel_bo_create_from_gem(
> + ptdev, panthor_fw_vm(ptdev), to_panthor_bo(obj),
> + DRM_PANTHOR_VM_BIND_OP_MAP_NOEXEC,
> + PANTHOR_VM_KERNEL_AUTO_VA,
> + panthor_fw_protm_suspend_size(ptdev));
[Severity: High]
Does this code map the user-provided BO into the firmware VM without validating
that it actually corresponds to a protected DMA-BUF heap?
The uAPI comments state this BO must be allocated from a protected DMA-BUF
heap, but there doesn't appear to be any verification of its security properties
or whether it's actually backed by protected memory.
If the Mali firmware writes protected state into this suspend buffer during
context switches, could a malicious userspace application supply a standard
shmem-backed BO to read the protected state from normal RAM?
Even if the GPU hardware traps non-secure accesses to secure physical addresses,
should the kernel verify the user-provided memory handle is actually secure
before using it for firmware structures?
(Note that a similar missing validation seems to exist for the firmware
protected sections initialization in panthor_fw_protm_init(), though that path
is protected by CAP_SYS_MODULE).
> + drm_gem_object_put(obj);
> + if (IS_ERR(group->protm_suspend_buf)) {
> + ret = PTR_ERR(group->protm_suspend_buf);
> + group->protm_suspend_buf = NULL;
> + goto err_put_group;
> + }
> + }
> +
> group->syncobjs = panthor_kernel_bo_create(ptdev, group->vm,
> group_args->queues.count *
> sizeof(struct panthor_syncobj_64b),
--
Sashiko AI review · https://sashiko.dev/#/patchset/20261012125940.3291951-1-ketil.johnsen@arm.com?part=11
^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH v4 12/12] drm/panthor: Bump version to expose protm feature
2026-10-12 12:59 [PATCH v4 00/12] drm/panthor: Protected mode support for Mali CSF GPUs Ketil Johnsen
` (10 preceding siblings ...)
2026-10-12 12:59 ` [PATCH v4 11/12] drm/panthor: Protm support for groups Ketil Johnsen
@ 2026-10-12 12:59 ` Ketil Johnsen
11 siblings, 0 replies; 18+ messages in thread
From: Ketil Johnsen @ 2026-10-12 12:59 UTC (permalink / raw)
To: Boris Brezillon, Steven Price, Liviu Dudau, Maarten Lankhorst,
Maxime Ripard, Thomas Zimmermann, David Airlie, Simona Vetter,
Daniel Almeida, Alice Ryhl
Cc: dri-devel, linux-kernel, Ketil Johnsen
Bumps version to expose previously added:
- DRM_IOCTL_PANTHOR_PROTM_INIT ioctl
- DRM_PANTHOR_DEV_QUERY_PROTM_INFO query
- drm_panthor_group_create::protected_suspend_bo_handle
Signed-off-by: Ketil Johnsen <ketil.johnsen@arm.com>
---
v4:
- First version of this change as a separate commit
---
drivers/gpu/drm/panthor/panthor_drv.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/panthor/panthor_drv.c b/drivers/gpu/drm/panthor/panthor_drv.c
index cc1c050ea3806..2da7bdb9a6bef 100644
--- a/drivers/gpu/drm/panthor/panthor_drv.c
+++ b/drivers/gpu/drm/panthor/panthor_drv.c
@@ -1801,6 +1801,9 @@ static void panthor_debugfs_init(struct drm_minor *minor)
* - 1.8 - extends DEV_QUERY_TIMESTAMP_INFO with flags
* - 1.9 - adds DRM_PANTHOR_DEV_QUERY_MMU_INFO query
* - adds DRM_PANTHOR_VM_BIND_OP_MAP_SPARSE flag
+ * - 1.10 - adds DRM_IOCTL_PANTHOR_PROTM_INIT ioctl
+ * - adds DRM_PANTHOR_DEV_QUERY_PROTM_INFO query
+ * - adds drm_panthor_group_create::protected_suspend_bo_handle
*/
static const struct drm_driver panthor_drm_driver = {
.driver_features = DRIVER_RENDER | DRIVER_GEM | DRIVER_SYNCOBJ |
@@ -1814,7 +1817,7 @@ static const struct drm_driver panthor_drm_driver = {
.name = "panthor",
.desc = "Panthor DRM driver",
.major = 1,
- .minor = 9,
+ .minor = 10,
.gem_prime_import_sg_table = panthor_gem_prime_import_sg_table,
.gem_prime_import = panthor_gem_prime_import,
--
2.43.0
^ permalink raw reply related [flat|nested] 18+ messages in thread