Re: [PATCH v3 6/6] drm/panthor: Expose protected rendering features

[Boris Brezillon <boris.brezillon@collabora.com>]

On Fri, 11 Sep 2026 13:40:14 +0200
Ketil Johnsen <ketil.johnsen@arm.com> wrote:

> From: Boris Brezillon <boris.brezillon@collabora.com>
> 
> Extensions to Panthor uAPI:
> - New IOCTL for user space to provide protected FW memory.
> - New query for checking protected rendering availability/status
>   and requirements.
> - Extends group creation to allow user space to provide a protected
>   suspend buffer.
> 
> 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.
> 
> Processes who want to execute in protected mode must also ensure they
> pass a protected suspend buffer during group creation.
> 
> Added panthor_kernel_bo_import() to allow user provided buffers.
> Refactor panthor_kernel_bo_create() to allow shared code with the
> new import variant.

There's just two many things happening here, so I'd suggest splitting
this patch into:

- Add the section_vm_map_flags() helper
- Add size/VA to panthor_fw_section
- s/panthor_gem_debugfs_set_usage_flags/panthor_gem_debugfs_add_usage_flags/
- support creating kernel BOs from a pre-existing GEM object
- add support for FW PROTM init (with the new ioctl)
- add support for PROTM group init
- bump the driver version to expose the new ioctls

>  /**
> - * panthor_kernel_bo_create() - Create and map a GEM object to a VM
> + * panthor_kernel_bo_import() - Create a kernel BO from an 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_import(struct panthor_device *ptdev, struct panthor_vm *vm,

Not sure I like the name, because the BO we pass is not necessarily
imported. I think I prefer panthor_kernel_bo_create_{with,from}_bo() or
_{with,from}_gem().

> +			 struct panthor_gem_object *bo, u32 vm_map_flags,
> +			 u64 gpu_va, u32 vm_map_size)
>  {

[..]

> diff --git a/drivers/gpu/drm/panthor/panthor_sched.c b/drivers/gpu/drm/panthor/panthor_sched.c
> index 1fe77e5c41995..d73fc2f6633a6 100644
> --- a/drivers/gpu/drm/panthor/panthor_sched.c
> +++ b/drivers/gpu/drm/panthor/panthor_sched.c
> @@ -1429,10 +1429,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 (!(ptdev->protm.info.state & DRM_PANTHOR_PROTM_INITIALIZED))

Should we instead check that the group is initialized for PROTM
support, and then have a check in group init to reject group PROTM init
if the device itself is not PROTM-initialized.

> +		atomic_or(BIT(cs_id), &group->fatal_queues);
> +	else
> +		atomic_or(BIT(cs_id), &group->protm_pending_queues);
>  
>  	sched_queue_delayed_work(sched, tick, 0);
>  }