linux-kernel - Re: [PATCH v2 1/1] drm/amdkfd: Fix unaligned doorbell absolute offset for gfx8

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <8cd0539d-5d5e-e670-9577-7db72393b784@amd.com>
Date:   Thu, 28 Sep 2023 11:30:23 -0400
From:   Felix Kuehling <felix.kuehling@....com>
To:     "Joshi, Mukul" <Mukul.Joshi@....com>,
        "Yadav, Arvind" <Arvind.Yadav@....com>,
        "Koenig, Christian" <Christian.Koenig@....com>,
        "Deucher, Alexander" <Alexander.Deucher@....com>,
        "Sharma, Shashank" <Shashank.Sharma@....com>,
        "Pan, Xinhui" <Xinhui.Pan@....com>,
        "airlied@...il.com" <airlied@...il.com>,
        "daniel@...ll.ch" <daniel@...ll.ch>
Cc:     "amd-gfx@...ts.freedesktop.org" <amd-gfx@...ts.freedesktop.org>,
        "dri-devel@...ts.freedesktop.org" <dri-devel@...ts.freedesktop.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v2 1/1] drm/amdkfd: Fix unaligned doorbell absolute offset
 for gfx8

On 2023-09-28 10:30, Joshi, Mukul wrote:
> [AMD Official Use Only - General]
>
>> -----Original Message-----
>> From: Yadav, Arvind <Arvind.Yadav@....com>
>> Sent: Thursday, September 28, 2023 5:54 AM
>> To: Koenig, Christian <Christian.Koenig@....com>; Deucher, Alexander
>> <Alexander.Deucher@....com>; Sharma, Shashank
>> <Shashank.Sharma@....com>; Kuehling, Felix <Felix.Kuehling@....com>;
>> Joshi, Mukul <Mukul.Joshi@....com>; Pan, Xinhui <Xinhui.Pan@....com>;
>> airlied@...il.com; daniel@...ll.ch
>> Cc: amd-gfx@...ts.freedesktop.org; dri-devel@...ts.freedesktop.org; linux-
>> kernel@...r.kernel.org; Yadav, Arvind <Arvind.Yadav@....com>; Koenig,
>> Christian <Christian.Koenig@....com>
>> Subject: [PATCH v2 1/1] drm/amdkfd: Fix unaligned doorbell absolute offset
>> for gfx8
>>
>> This patch is to adjust the absolute doorbell offset against the doorbell id
>> considering the doorbell size of 32/64 bit.
>>
>> v2:
>> - Addressed the review comment from Felix.
>>
>> Cc: Christian Koenig <christian.koenig@....com>
>> Cc: Alex Deucher <alexander.deucher@....com>
>> Signed-off-by: Shashank Sharma <shashank.sharma@....com>
>> Signed-off-by: Arvind Yadav <Arvind.Yadav@....com>
>> ---
>>   drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c | 9 ++++++++-
>>   1 file changed, 8 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c
>> b/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c
>> index 0d3d538b64eb..c54c4392d26e 100644
>> --- a/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c
>> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c
>> @@ -407,7 +407,14 @@ static int allocate_doorbell(struct
>> qcm_process_device *qpd,
>>
>>        q->properties.doorbell_off = amdgpu_doorbell_index_on_bar(dev-
>>> adev,
>>                                                                  qpd-
>>> proc_doorbells,
>> -                                                               q-
>>> doorbell_id);
>> +                                                               0);
>> +
> It looks like amdgpu_doorbell_index_on_bar() works only for 64-bit doorbells.
> Shouldn't it work for both 32-bit and 64-bit doorbells considering this is common
> doorbell manager code?

I could see this argument going either way. KFD is the only one that 
cares about managing doorbells for user mode queues on GFXv8 GPUs. This 
is not a use case that amdgpu cares about. So I'm OK with KFD doing its 
own address calculations to make sure doorbells continue to work on GFXv8.

It may not be worth adding complexity to the common doorbell manager 
code to support legacy GPUs with 32-bit doorbells.

Regards,
   Felix


>
> Thanks,
> Mukul
>
>> +     /* Adjust the absolute doorbell offset against the doorbell id
>> considering
>> +      * the doorbell size of 32/64 bit.
>> +      */
>> +     q->properties.doorbell_off += q->doorbell_id *
>> +                                   dev->kfd->device_info.doorbell_size / 4;
>> +
>>        return 0;
>>   }
>>
>> --
>> 2.34.1