linux-kernel - Re: [PATCH 1/2] drm/panthor: Add PANTHOR_GROUP_PRIORITY

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <8f4329e3-613a-49b1-9d01-0e8451bea54e@arm.com>
Date: Thu, 5 Sep 2024 17:22:10 +0100
From: Mihail Atanassov <mihail.atanassov@....com>
To: Steven Price <steven.price@....com>,
 Mary Guillemard <mary.guillemard@...labora.com>, linux-kernel@...r.kernel.org
Cc: dri-devel@...ts.freedesktop.org,
 Boris Brezillon <boris.brezillon@...labora.com>,
 Christopher Healy <healych@...zon.com>, kernel@...labora.com,
 Liviu Dudau <liviu.dudau@....com>,
 Maarten Lankhorst <maarten.lankhorst@...ux.intel.com>,
 Maxime Ripard <mripard@...nel.org>, Thomas Zimmermann <tzimmermann@...e.de>,
 David Airlie <airlied@...il.com>, Daniel Vetter <daniel@...ll.ch>,
 nd <nd@....com>
Subject: Re: [PATCH 1/2] drm/panthor: Add PANTHOR_GROUP_PRIORITY_REALTIME
 group priority



On 05/09/2024 16:41, Steven Price wrote:
> Hi Mihail,
> 
> On 05/09/2024 14:54, Mihail Atanassov wrote:
>> Hi Mary,
>>
>> On 05/09/2024 12:13, Mary Guillemard wrote:
>>> This adds a new value to drm_panthor_group_priority exposing the
>>> realtime priority to userspace.
>>>
>>> This is required to implement NV_context_priority_realtime in Mesa.
>>>
>>> Signed-off-by: Mary Guillemard <mary.guillemard@...labora.com>
>>> ---
>>>    drivers/gpu/drm/panthor/panthor_drv.c   | 2 +-
>>>    drivers/gpu/drm/panthor/panthor_sched.c | 2 --
>>>    include/uapi/drm/panthor_drm.h          | 7 +++++++
>>>    3 files changed, 8 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/panthor/panthor_drv.c
>>> b/drivers/gpu/drm/panthor/panthor_drv.c
>>> index 0caf9e9a8c45..7b1db2adcb4c 100644
>>> --- a/drivers/gpu/drm/panthor/panthor_drv.c
>>> +++ b/drivers/gpu/drm/panthor/panthor_drv.c
>>> @@ -1041,7 +1041,7 @@ static int group_priority_permit(struct drm_file
>>> *file,
>>>                     u8 priority)
>>>    {
>>>        /* Ensure that priority is valid */
>>> -    if (priority > PANTHOR_GROUP_PRIORITY_HIGH)
>>> +    if (priority > PANTHOR_GROUP_PRIORITY_REALTIME)
>>>            return -EINVAL;
>>>          /* Medium priority and below are always allowed */
>>> diff --git a/drivers/gpu/drm/panthor/panthor_sched.c
>>> b/drivers/gpu/drm/panthor/panthor_sched.c
>>> index 91a31b70c037..86908ada7335 100644
>>> --- a/drivers/gpu/drm/panthor/panthor_sched.c
>>> +++ b/drivers/gpu/drm/panthor/panthor_sched.c
>>> @@ -137,8 +137,6 @@ enum panthor_csg_priority {
>>>         * non-real-time groups. When such a group becomes executable,
>>>         * it will evict the group with the lowest non-rt priority if
>>>         * there's no free group slot available.
>>> -     *
>>> -     * Currently not exposed to userspace.
>>>         */
>>>        PANTHOR_CSG_PRIORITY_RT,
>>>    diff --git a/include/uapi/drm/panthor_drm.h
>>> b/include/uapi/drm/panthor_drm.h
>>> index 1fd8473548ac..011a555e4674 100644
>>> --- a/include/uapi/drm/panthor_drm.h
>>> +++ b/include/uapi/drm/panthor_drm.h
>>> @@ -720,6 +720,13 @@ enum drm_panthor_group_priority {
>>>         * Requires CAP_SYS_NICE or DRM_MASTER.
>>>         */
>>>        PANTHOR_GROUP_PRIORITY_HIGH,
>>> +
>>> +    /**
>>> +     * @PANTHOR_GROUP_PRIORITY_REALTIME: Realtime priority group.
>>> +     *
>>> +     * Requires CAP_SYS_NICE or DRM_MASTER.
>>> +     */
>>> +    PANTHOR_GROUP_PRIORITY_REALTIME,
>>
>> This is a uapi change, but doesn't have a corresponding driver version
>> bump in the same patch. You also document the addition of this enum
>> value in the next patch, which also is a tad wonky. If you reversed the
>> order of the patches, they'd make more sense IMO.
> 
> You can't reverse the order because then the version bump would be
> before all the features were present. Generally we put the version bump
> at the end of a patch series because it's indicating to user space that
> the new features can be used. This way round if a bisect lands in the
> middle of the series then the new priority is there but won't be used
> because user space won't know about it (which is fine).
> 

Ack.

> Steve
> 
>>>    };
>>>      /**
>>
> 

-- 
Mihail Atanassov <mihail.atanassov@....com>