[PATCH] drm/amdgpu: Set VM_L2_CNTL.PDE_FAULT_CLASSIFICATION to 0

[Christian König]

> Don't we want PDE faults to be treated the same way as page faults?
Yeah, I think that this was actually the original intention. Problem is 
that this is actually the wrong approach for this.

> Or is the idea that we should never see a PDE fault unless something goes wrong
Exactly, yes. What you really want to do is to set the P bit for PDEs 
which can be allocated dynamically.

This way the walker stops at the current processed entry and sends your 
a standard page fault for a leave page.

Regards,
Christian.

Am 26.02.19 um 04:50 schrieb Bridgman, John:
> Or is the idea that we should never see a PDE fault unless something goes wrong, and that we would set up an entry corresponding to an unmapped subtree as an invalid PTE for a very large page rather than an invalid PDE?
>
> Thanks,
> John
>    Original Message
> From: Bridgman, John
> Sent: Monday, February 25, 2019 22:46
> To: Alex Deucher; Zhao, Yong
> Cc: amd-gfx list
> Subject: Re: [PATCH] drm/amdgpu: Set VM_L2_CNTL.PDE_FAULT_CLASSIFICATION to 0
>
>
> Don't we want PDE faults to be treated the same way as page faults? Or am I misinterpreting the commit message?
>
> Thanks,
> John
>    Original Message
> From: Alex Deucher
> Sent: Monday, February 25, 2019 21:53
> To: Zhao, Yong
> Cc: amd-gfx at lists.freedesktop.org
> Subject: Re: [PATCH] drm/amdgpu: Set VM_L2_CNTL.PDE_FAULT_CLASSIFICATION to 0
>
>
> On Mon, Feb 25, 2019 at 6:03 PM Zhao, Yong <Yong.Zhao at amd.com> wrote:
>> This is recommended by HW designers. Previously when it was set to 1,
>> the PDE walk error in VM fault will be treated as
>> PERMISSION_OR_INVALID_PAGE_FAULT rather than usually expected OTHER_FAULT.
>> As a result, the retry control in VM_CONTEXT*_CNTL will change accordingly.
>>
>> The above behavior is kind of abnormal. Furthermore, the
>> PDE_FAULT_CLASSIFICATION == 1 feature was targeted for very old ASICs
>> and it never made it way to production. Therefore, we should set it to 0.
>>
>> Signed-off-by: Yong Zhao <Yong.Zhao at amd.com>
> Acked-by: Alex Deucher <alexander.deucher at amd.com>
>
>> ---
>>   drivers/gpu/drm/amd/amdgpu/gfxhub_v1_0.c | 2 +-
>>   drivers/gpu/drm/amd/amdgpu/mmhub_v1_0.c  | 2 +-
>>   2 files changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/amd/amdgpu/gfxhub_v1_0.c b/drivers/gpu/drm/amd/amdgpu/gfxhub_v1_0.c
>> index f5edddf3b29d..c10ed568ca6c 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/gfxhub_v1_0.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/gfxhub_v1_0.c
>> @@ -143,7 +143,7 @@ static void gfxhub_v1_0_init_cache_regs(struct amdgpu_device *adev)
>>          /* XXX for emulation, Refer to closed source code.*/
>>          tmp = REG_SET_FIELD(tmp, VM_L2_CNTL, L2_PDE0_CACHE_TAG_GENERATION_MODE,
>>                              0);
>> -       tmp = REG_SET_FIELD(tmp, VM_L2_CNTL, PDE_FAULT_CLASSIFICATION, 1);
>> +       tmp = REG_SET_FIELD(tmp, VM_L2_CNTL, PDE_FAULT_CLASSIFICATION, 0);
>>          tmp = REG_SET_FIELD(tmp, VM_L2_CNTL, CONTEXT1_IDENTITY_ACCESS_MODE, 1);
>>          tmp = REG_SET_FIELD(tmp, VM_L2_CNTL, IDENTITY_MODE_FRAGMENT_SIZE, 0);
>>          WREG32_SOC15(GC, 0, mmVM_L2_CNTL, tmp);
>> diff --git a/drivers/gpu/drm/amd/amdgpu/mmhub_v1_0.c b/drivers/gpu/drm/amd/amdgpu/mmhub_v1_0.c
>> index d0d966d6080a..2a039946a549 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/mmhub_v1_0.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/mmhub_v1_0.c
>> @@ -163,7 +163,7 @@ static void mmhub_v1_0_init_cache_regs(struct amdgpu_device *adev)
>>          /* XXX for emulation, Refer to closed source code.*/
>>          tmp = REG_SET_FIELD(tmp, VM_L2_CNTL, L2_PDE0_CACHE_TAG_GENERATION_MODE,
>>                              0);
>> -       tmp = REG_SET_FIELD(tmp, VM_L2_CNTL, PDE_FAULT_CLASSIFICATION, 1);
>> +       tmp = REG_SET_FIELD(tmp, VM_L2_CNTL, PDE_FAULT_CLASSIFICATION, 0);
>>          tmp = REG_SET_FIELD(tmp, VM_L2_CNTL, CONTEXT1_IDENTITY_ACCESS_MODE, 1);
>>          tmp = REG_SET_FIELD(tmp, VM_L2_CNTL, IDENTITY_MODE_FRAGMENT_SIZE, 0);
>>          WREG32_SOC15(MMHUB, 0, mmVM_L2_CNTL, tmp);
>> --
>> 2.17.1
>>
>> _______________________________________________
>> amd-gfx mailing list
>> amd-gfx at lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
> _______________________________________________
> amd-gfx mailing list
> amd-gfx at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
> _______________________________________________
> amd-gfx mailing list
> amd-gfx at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx