Re: [PATCH 02/14] drm/amdgpu/gfxhub: Enable retry fault interrupts when needed

["Christian König" <christian.koenig@amd.com>]

On 7/16/26 00:51, Timur Kristóf wrote:
> On Thursday, July 2, 2026 2:02:50 PM Central European Summer Time Christian 
> König wrote:
>> On 7/2/26 11:14, Timur Kristóf wrote:
>>> On 2026. július 2., csütörtök 10:10:10 közép-európai nyári idő Christian
>>> König> 
>>> wrote:
>>>> On 7/1/26 18:17, Timur Kristóf wrote:
>>>>> Enable retry fault interrupts when initializing the GFXHUB
>>>>> system aperture registers according to whether retrying
>>>>> page faults is enabled in amdgpu (ie. amdgpu.noretry=0).
>>>>>
>>>>> Needs to be done for each GFXHUB version at once,
>>>>> because none of them actually enabled this interrupt.
>>>>
>>>> Thinking more about it we are clearly missing something here. The retry
>>>> fault interrupt itself should be enabled all the time.
>>>
>>> Why would it be enabled all the time?
>>> I haven't seen any retry faults on neither Navi 3 nor Navi 4 without
>>> enabling the ENABLE_RETRY_FAULT_INTERRUPT bit.
>>
>> As far as I understand it the GCVM_L2_PROTECTION_FAULT_CNTL2 is actually a
>> debug register for the fault behavior of the GC block.
> 
> After our last conversation I tested this again and I can confirm that I do NOT 
> get any retry fault interrupts on Navi 3 if I remove the code that enables 
> this bit. That makes me think this is not enabled by default.
> 
>> According to the register spec for Navi 44 the ENABLE_RETRY_FAULT_INTERRUPT
>> should be default 1 and always be set under normal cricumstances.
> 
> This is definitely not the case on Navi 31 and also on Strix Halo.
> I haven't checked on Navi 4x but judging by the fact that the gfxhub_v12_1 
> code has to enable this bit explicitly, I doubt it would be enabled on Navi 4x 
> either.

My guess is that this is either some bug in the golden register settings or the VBIOS so that isn't correctly initialized by default, or that this bit is controlled by FW to enable/disable the functionality depending on which queue is currently processing.

I need to double check that when I'm back from vacation.

>>>> IIRC only the RETRY_PERMISSION_OR_INVALID_PAGE_FAULT bit in the
>>>> VM_CONTEXT0_CNTL register should be set or cleared by the kernel driver
>>>> or
>>>> firmware to control if the HW retries the access or not.
>>>
>>> That is clearly not the case on GFX12.1 and I haven't seen any indication
>>> that it would be different on GFX11.x and 12.0 either.
>>
>> Mhm, then either the FW or golden register settings for that register is not
>> correct.
>>
>> That we set ACTIVE_PAGE_MIGRATION_PTE_READ_RETRY to one is rather strange as
>> well since we don't use that feature in the Linux driver.
>>
>> My suggestion is to always set the ENABLE_RETRY_FAULT_INTERRUPT in
>> GCVM_L2_PROTECTION_FAULT_CNTL2 no matter what the noretry flag says, but
>> I'm going to ask Alex and our ROCm people about that again when I have
>> time.
> 
> I don't see why we should enable this bit when we don't want to enable retry 
> faults.

Because according to the HW documentation it should always be on and never disabled. I'm pretty sure we shouldn't touch it from the kernel driver.

For now just always set it to one during initialization, if the retry fault interrupt is enabled or not is gated through the bit in the VM_CONTEXT1_CNTL register for the kernel driver.

Regards,
Christian.

> 
> Thanks & best regards,
> Timur
> 
> 
> 
>