Re: [PATCH] accel/ivpu: Add support for getting and setting command queue priority

[Andrzej Kacprowski <andrzej.kacprowski@linux.intel.com>]

On 31-Aug-26 2:33 PM, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> 
> Pre-existing issues:
> - [High] Missing CAP_SYS_NICE or DRM_MASTER check for elevated job priorities (REALTIME / FOCUS), allowing unprivileged users to monopolize the NPU.
> --
> 
> commit 57cf0d7005a7761ad6193ea568802164b7fac6db
> Author: Andrzej Kacprowski <andrzej.kacprowski@linux.intel.com>
> 
> accel/ivpu: Add support for getting and setting command queue priority
> 
> This commit adds a new DRM_IVPU_PARAM_CMDQ_PRIORITY parameter to query and
> update the scheduling priority of an existing command queue. The priority
> can be changed on a queue that already has pending jobs, propagating the
> change to firmware in hardware scheduling mode.
> 
>> diff --git a/drivers/accel/ivpu/ivpu_job.c b/drivers/accel/ivpu/ivpu_job.c
>> index ebb2c865b09a0..b3de5dd29d1e8 100644
>> --- a/drivers/accel/ivpu/ivpu_job.c
>> +++ b/drivers/accel/ivpu/ivpu_job.c
> 
> [ ... ]
> 
>> @@ -1127,6 +1132,86 @@ int ivpu_cmdq_destroy_ioctl(struct drm_device *dev, void *data, struct drm_file
>>   	return ret;
>>   }
>>   
>> +static int ivpu_cmdq_priority_args_check(struct ivpu_device *vdev, u32 cmdq_id, u64 priority)
>> +{
>> +	if (!ivpu_is_capable(vdev, DRM_IVPU_CAP_CMDQ_SET_PRIORITY)) {
>> +		ivpu_dbg(vdev, IOCTL, "Command queue priority not supported\n");
>> +		return -ENODEV;
>> +	}
>> +
>> +	if (cmdq_id < IVPU_CMDQ_MIN_ID || cmdq_id > IVPU_CMDQ_MAX_ID) {
>> +		ivpu_dbg(vdev, IOCTL, "Invalid command queue ID %u\n", cmdq_id);
>> +		return -EINVAL;
>> +	}
>> +
>> +	if (priority > DRM_IVPU_JOB_PRIORITY_REALTIME)
>> +		return -EINVAL;
> 
> [Severity: High]
> This is a pre-existing issue, but is there a missing capability check for
> elevated priorities here?
> 
> An unprivileged user who opens the NPU device can call the IVPU_SET_PARAM
> ioctl with DRM_IVPU_PARAM_CMDQ_PRIORITY and set the priority to
> DRM_IVPU_JOB_PRIORITY_REALTIME. Without a CAP_SYS_NICE or DRM_MASTER check,
> could a malicious or buggy application elevate its command queue priority to
> monopolize the hardware and starve other processes?
> 
> I notice this same behavior appears to be pre-existing in
> ivpu_cmdq_create_ioctl() as well, where queues can be created with realtime
> priority by any user. Should these priority levels be restricted?
> 

The Intel NPU is not a display/graphics device and has no notion of 
DRM_MASTER which is typically the display server process.

Using CAP_SYS_NICE has a bit more merit however:
- strict CAP_SYS_NICE check in KMD will break existing use cases
- no other accel drivers use CAP_SYS_NICE

Current strategy is to allow OS level AI runtime to control NPU workload 
priority - dynamic priority change was added to
support that use case.

I will take it into consideration for future driver updates, but it
cannot be immediately solved in this patch.

- Andrzej
>> +
>> +	return 0;
>> +}
>> +
>> +int ivpu_cmdq_get_priority(struct ivpu_file_priv *file_priv, u32 cmdq_id, u64 *priority)
>> +{
> 
> [ ... ]
>