[PATCH] drm/amdgpu: skip huge page for PRT mapping

[PATCH] drm/amdgpu: skip huge page for PRT mapping

[Junwei Zhang]

PRT mapping doesn't support huge page, since it's per PTE basis.

Signed-off-by: Junwei Zhang <Jerry.Zhang@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
index 850cd66..4ce8bb0 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
@@ -1111,7 +1111,8 @@ static void amdgpu_vm_handle_huge_pages(struct amdgpu_pte_update_params *p,
 
 	/* In the case of a mixed PT the PDE must point to it*/
 	if (p->adev->asic_type >= CHIP_VEGA10 && !p->src &&
-	    nptes == AMDGPU_VM_PTE_COUNT(p->adev)) {
+	    nptes == AMDGPU_VM_PTE_COUNT(p->adev) &&
+	    !(flags & AMDGPU_PTE_PRT)) {
 		/* Set the huge page flag to stop scanning at this PDE */
 		flags |= AMDGPU_PDE_PTE;
 	}
-- 
1.9.1

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

RE: [PATCH] drm/amdgpu: skip huge page for PRT mapping

[Zhou, David(ChunMing)]

Good catch, Reviewed-by: Chunming  Zhou <david1.zhou@amd.com>

-----Original Message-----
From: amd-gfx [mailto:amd-gfx-bounces@lists.freedesktop.org] On Behalf Of Junwei Zhang
Sent: Monday, June 04, 2018 10:04 AM
To: amd-gfx@lists.freedesktop.org
Cc: Zhang, Jerry <Jerry.Zhang@amd.com>
Subject: [PATCH] drm/amdgpu: skip huge page for PRT mapping

PRT mapping doesn't support huge page, since it's per PTE basis.

Signed-off-by: Junwei Zhang <Jerry.Zhang@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
index 850cd66..4ce8bb0 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
@@ -1111,7 +1111,8 @@ static void amdgpu_vm_handle_huge_pages(struct amdgpu_pte_update_params *p,
 
 	/* In the case of a mixed PT the PDE must point to it*/
 	if (p->adev->asic_type >= CHIP_VEGA10 && !p->src &&
-	    nptes == AMDGPU_VM_PTE_COUNT(p->adev)) {
+	    nptes == AMDGPU_VM_PTE_COUNT(p->adev) &&
+	    !(flags & AMDGPU_PTE_PRT)) {
 		/* Set the huge page flag to stop scanning at this PDE */
 		flags |= AMDGPU_PDE_PTE;
 	}
-- 
1.9.1

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH] drm/amdgpu: skip huge page for PRT mapping

[Christian König]

Actually that is not correct. According to the documentation the PRT 
flag should work for huge pages as well.

Christian.

Am 04.06.2018 um 07:59 schrieb Zhou, David(ChunMing):
> Good catch, Reviewed-by: Chunming  Zhou <david1.zhou@amd.com>
>
> -----Original Message-----
> From: amd-gfx [mailto:amd-gfx-bounces@lists.freedesktop.org] On Behalf Of Junwei Zhang
> Sent: Monday, June 04, 2018 10:04 AM
> To: amd-gfx@lists.freedesktop.org
> Cc: Zhang, Jerry <Jerry.Zhang@amd.com>
> Subject: [PATCH] drm/amdgpu: skip huge page for PRT mapping
>
> PRT mapping doesn't support huge page, since it's per PTE basis.
>
> Signed-off-by: Junwei Zhang <Jerry.Zhang@amd.com>
> ---
>   drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 3 ++-
>   1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> index 850cd66..4ce8bb0 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> @@ -1111,7 +1111,8 @@ static void amdgpu_vm_handle_huge_pages(struct amdgpu_pte_update_params *p,
>   
>   	/* In the case of a mixed PT the PDE must point to it*/
>   	if (p->adev->asic_type >= CHIP_VEGA10 && !p->src &&
> -	    nptes == AMDGPU_VM_PTE_COUNT(p->adev)) {
> +	    nptes == AMDGPU_VM_PTE_COUNT(p->adev) &&
> +	    !(flags & AMDGPU_PTE_PRT)) {
>   		/* Set the huge page flag to stop scanning at this PDE */
>   		flags |= AMDGPU_PDE_PTE;
>   	}

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH] drm/amdgpu: skip huge page for PRT mapping

[Zhang, Jerry (Junwei)]

On 06/04/2018 02:43 PM, Christian König wrote:
> Actually that is not correct. According to the documentation the PRT flag should
> work for huge pages as well.

Mmm, I checked the doc earlier, didn't find the PRT flag for PDE.

In CTS PRT test, the reserved PRT mapping introduces huge page mapping, so later 
tiled bo mapping cannot make sure the corresponding PTE is set as PRT.
Then following access triggers VM fault.

Jerry

>
> Christian.
>
> Am 04.06.2018 um 07:59 schrieb Zhou, David(ChunMing):
>> Good catch, Reviewed-by: Chunming  Zhou <david1.zhou@amd.com>
>>
>> -----Original Message-----
>> From: amd-gfx [mailto:amd-gfx-bounces@lists.freedesktop.org] On Behalf Of
>> Junwei Zhang
>> Sent: Monday, June 04, 2018 10:04 AM
>> To: amd-gfx@lists.freedesktop.org
>> Cc: Zhang, Jerry <Jerry.Zhang@amd.com>
>> Subject: [PATCH] drm/amdgpu: skip huge page for PRT mapping
>>
>> PRT mapping doesn't support huge page, since it's per PTE basis.
>>
>> Signed-off-by: Junwei Zhang <Jerry.Zhang@amd.com>
>> ---
>>   drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 3 ++-
>>   1 file changed, 2 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
>> index 850cd66..4ce8bb0 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
>> @@ -1111,7 +1111,8 @@ static void amdgpu_vm_handle_huge_pages(struct
>> amdgpu_pte_update_params *p,
>>       /* In the case of a mixed PT the PDE must point to it*/
>>       if (p->adev->asic_type >= CHIP_VEGA10 && !p->src &&
>> -        nptes == AMDGPU_VM_PTE_COUNT(p->adev)) {
>> +        nptes == AMDGPU_VM_PTE_COUNT(p->adev) &&
>> +        !(flags & AMDGPU_PTE_PRT)) {
>>           /* Set the huge page flag to stop scanning at this PDE */
>>           flags |= AMDGPU_PDE_PTE;
>>       }
>
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH] drm/amdgpu: skip huge page for PRT mapping

[Christian König]

Am 04.06.2018 um 09:02 schrieb Zhang, Jerry (Junwei):
> On 06/04/2018 02:43 PM, Christian König wrote:
>> Actually that is not correct. According to the documentation the PRT 
>> flag should
>> work for huge pages as well.
>
> Mmm, I checked the doc earlier, didn't find the PRT flag for PDE.

The PDE indeed doesn't have the PRT flag, but it also doesn't have the 
fragment and MTYPE fields and those still work normally when you enable 
huge page handling.

The trick is that when you set the huge page flag then the PDE is 
handled like a PTE and so all the extra fields (PRT, fragment size, 
MTYPE etc...) should now be handled correctly.

Could be that there is a hardware bug related to PRT handling and the 
huge page flag, but at least in theory it should work fine.

>
> In CTS PRT test, the reserved PRT mapping introduces huge page 
> mapping, so later tiled bo mapping cannot make sure the corresponding 
> PTE is set as PRT.
> Then following access triggers VM fault.

Interesting, but that rather sounds like a bug in the handling instead 
of a hardware problem.

Can you narrow down the CTS test further into a libdrm unit test? Or in 
other words what exactly does the CTS test do?

Christian.

>
> Jerry
>
>>
>> Christian.
>>
>> Am 04.06.2018 um 07:59 schrieb Zhou, David(ChunMing):
>>> Good catch, Reviewed-by: Chunming  Zhou <david1.zhou@amd.com>
>>>
>>> -----Original Message-----
>>> From: amd-gfx [mailto:amd-gfx-bounces@lists.freedesktop.org] On 
>>> Behalf Of
>>> Junwei Zhang
>>> Sent: Monday, June 04, 2018 10:04 AM
>>> To: amd-gfx@lists.freedesktop.org
>>> Cc: Zhang, Jerry <Jerry.Zhang@amd.com>
>>> Subject: [PATCH] drm/amdgpu: skip huge page for PRT mapping
>>>
>>> PRT mapping doesn't support huge page, since it's per PTE basis.
>>>
>>> Signed-off-by: Junwei Zhang <Jerry.Zhang@amd.com>
>>> ---
>>>   drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 3 ++-
>>>   1 file changed, 2 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
>>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
>>> index 850cd66..4ce8bb0 100644
>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
>>> @@ -1111,7 +1111,8 @@ static void amdgpu_vm_handle_huge_pages(struct
>>> amdgpu_pte_update_params *p,
>>>       /* In the case of a mixed PT the PDE must point to it*/
>>>       if (p->adev->asic_type >= CHIP_VEGA10 && !p->src &&
>>> -        nptes == AMDGPU_VM_PTE_COUNT(p->adev)) {
>>> +        nptes == AMDGPU_VM_PTE_COUNT(p->adev) &&
>>> +        !(flags & AMDGPU_PTE_PRT)) {
>>>           /* Set the huge page flag to stop scanning at this PDE */
>>>           flags |= AMDGPU_PDE_PTE;
>>>       }
>>

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH] drm/amdgpu: skip huge page for PRT mapping

[Zhang, Jerry (Junwei)]

On 06/04/2018 03:48 PM, Christian König wrote:
> Am 04.06.2018 um 09:02 schrieb Zhang, Jerry (Junwei):
>> On 06/04/2018 02:43 PM, Christian König wrote:
>>> Actually that is not correct. According to the documentation the PRT flag should
>>> work for huge pages as well.
>>
>> Mmm, I checked the doc earlier, didn't find the PRT flag for PDE.
>
> The PDE indeed doesn't have the PRT flag, but it also doesn't have the fragment
> and MTYPE fields and those still work normally when you enable huge page handling.
>
> The trick is that when you set the huge page flag then the PDE is handled like a
> PTE and so all the extra fields (PRT, fragment size, MTYPE etc...) should now be
> handled correctly.
>
> Could be that there is a hardware bug related to PRT handling and the huge page
> flag, but at least in theory it should work fine.

The doc just skips these fields in PDE, if those fields really works expectedly, 
even if it doesn't describe the details(but we could refer to PTE fields), we 
may have to confirm that with HW guys.

In my view in current stage, PDE doesn't support PRT and it may not make much 
sense to support PRT either.
The huge page always happens when reserving PRT range, but later UMD is likely 
to bind a/some tiled bo(s) inside this range, that will break the huge page 
mapping and split into several pieces mappings, representing in PTE instead of 
huge page.
In this case, just skipping the huge page for PRT may be an acceptable way.

>
>>
>> In CTS PRT test, the reserved PRT mapping introduces huge page mapping, so
>> later tiled bo mapping cannot make sure the corresponding PTE is set as PRT.
>> Then following access triggers VM fault.
>
> Interesting, but that rather sounds like a bug in the handling instead of a
> hardware problem.

On the 2nd thinking, we may handle that for huge page.
e.g.when binding tiled bo in PRT range, we could split the huge page into pieces 
PTE mappings.
However, it may just make the code more complex and get the same results as 
current fix.

>
> Can you narrow down the CTS test further into a libdrm unit test? Or in other
> words what exactly does the CTS test do?

The issue could be reproduced by below command:
{{{
deqp-vk -n dEQP-VK.sparse_resources.buffer.ssbo.sparse_residency.buffer_size_2_24
}}}

In KMD view, the main process(with amdgpu mainline driver) is like below:
1) reserve PRT range [0x300400000, 0x300400000 + 0x1000000)
2) bind tiled bos each other page
    0x300400000 ~ 0x300401000,
    0x300402000 ~ 0x300403000,
    0x300404000 ~ 0x300405000,
    ...
3) access them all, VM fault at 0x300401000

Jerry

>
> Christian.
>
>>
>> Jerry
>>
>>>
>>> Christian.
>>>
>>> Am 04.06.2018 um 07:59 schrieb Zhou, David(ChunMing):
>>>> Good catch, Reviewed-by: Chunming  Zhou <david1.zhou@amd.com>
>>>>
>>>> -----Original Message-----
>>>> From: amd-gfx [mailto:amd-gfx-bounces@lists.freedesktop.org] On Behalf Of
>>>> Junwei Zhang
>>>> Sent: Monday, June 04, 2018 10:04 AM
>>>> To: amd-gfx@lists.freedesktop.org
>>>> Cc: Zhang, Jerry <Jerry.Zhang@amd.com>
>>>> Subject: [PATCH] drm/amdgpu: skip huge page for PRT mapping
>>>>
>>>> PRT mapping doesn't support huge page, since it's per PTE basis.
>>>>
>>>> Signed-off-by: Junwei Zhang <Jerry.Zhang@amd.com>
>>>> ---
>>>>   drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 3 ++-
>>>>   1 file changed, 2 insertions(+), 1 deletion(-)
>>>>
>>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
>>>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
>>>> index 850cd66..4ce8bb0 100644
>>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
>>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
>>>> @@ -1111,7 +1111,8 @@ static void amdgpu_vm_handle_huge_pages(struct
>>>> amdgpu_pte_update_params *p,
>>>>       /* In the case of a mixed PT the PDE must point to it*/
>>>>       if (p->adev->asic_type >= CHIP_VEGA10 && !p->src &&
>>>> -        nptes == AMDGPU_VM_PTE_COUNT(p->adev)) {
>>>> +        nptes == AMDGPU_VM_PTE_COUNT(p->adev) &&
>>>> +        !(flags & AMDGPU_PTE_PRT)) {
>>>>           /* Set the huge page flag to stop scanning at this PDE */
>>>>           flags |= AMDGPU_PDE_PTE;
>>>>       }
>>>
>
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH] drm/amdgpu: skip huge page for PRT mapping

[Christian König]

Am 04.06.2018 um 10:19 schrieb Zhang, Jerry (Junwei):
> On 06/04/2018 03:48 PM, Christian König wrote:
>> Am 04.06.2018 um 09:02 schrieb Zhang, Jerry (Junwei):
>>> On 06/04/2018 02:43 PM, Christian König wrote:
>>>> Actually that is not correct. According to the documentation the 
>>>> PRT flag should
>>>> work for huge pages as well.
>>>
>>> Mmm, I checked the doc earlier, didn't find the PRT flag for PDE.
>>
>> The PDE indeed doesn't have the PRT flag, but it also doesn't have 
>> the fragment
>> and MTYPE fields and those still work normally when you enable huge 
>> page handling.
>>
>> The trick is that when you set the huge page flag then the PDE is 
>> handled like a
>> PTE and so all the extra fields (PRT, fragment size, MTYPE etc...) 
>> should now be
>> handled correctly.
>>
>> Could be that there is a hardware bug related to PRT handling and the 
>> huge page
>> flag, but at least in theory it should work fine.
>
> The doc just skips these fields in PDE, if those fields really works 
> expectedly, even if it doesn't describe the details(but we could refer 
> to PTE fields), we may have to confirm that with HW guys.

I've just confirmed that huge pages and PRTs should work together. We 
even have an unit test for that.

I mean we could ping Wade as well, but I'm pretty sure that he will tell 
you just the same.

I think by disabling huge pages for PRTs we actually hide the real issue 
somehow, probably some problem with updating PRTs after they are split up.

Can you check if the problem also vanishes when you disable the 
following optimization in amdgpu_vm_update_ptes?
>                 /* We don't need to update PTEs for huge pages */
>                 if (entry->huge)
>                         continue;
Just comment this out for a test.

>
> In my view in current stage, PDE doesn't support PRT and it may not 
> make much sense to support PRT either.
> The huge page always happens when reserving PRT range, but later UMD 
> is likely to bind a/some tiled bo(s) inside this range, that will 
> break the huge page mapping and split into several pieces mappings, 
> representing in PTE instead of huge page.
> In this case, just skipping the huge page for PRT may be an acceptable 
> way.

Well disabling huge pages for PRTs can definitely cause a huge 
performance problem because it increases the TLB misses for PRTs by a 
factor of 512.

And since PRTs are usually used for large only sparse allocated textures 
that is really really important here.

>
>>
>>>
>>> In CTS PRT test, the reserved PRT mapping introduces huge page 
>>> mapping, so
>>> later tiled bo mapping cannot make sure the corresponding PTE is set 
>>> as PRT.
>>> Then following access triggers VM fault.
>>
>> Interesting, but that rather sounds like a bug in the handling 
>> instead of a
>> hardware problem.
>
> On the 2nd thinking, we may handle that for huge page.
> e.g.when binding tiled bo in PRT range, we could split the huge page 
> into pieces PTE mappings.
> However, it may just make the code more complex and get the same 
> results as current fix.

That should already be the case when everything works as expected.

>
>>
>> Can you narrow down the CTS test further into a libdrm unit test? Or 
>> in other
>> words what exactly does the CTS test do?
>
> The issue could be reproduced by below command:
> {{{
> deqp-vk -n 
> dEQP-VK.sparse_resources.buffer.ssbo.sparse_residency.buffer_size_2_24
> }}}
>
> In KMD view, the main process(with amdgpu mainline driver) is like below:
> 1) reserve PRT range [0x300400000, 0x300400000 + 0x1000000)
> 2) bind tiled bos each other page
>    0x300400000 ~ 0x300401000,
>    0x300402000 ~ 0x300403000,
>    0x300404000 ~ 0x300405000,
>    ...
> 3) access them all, VM fault at 0x300401000

That sounds like something I should be able to reproduce when your quick 
test doesn't bring the desired result.

Christian.

>
> Jerry
>
>>
>> Christian.
>>
>>>
>>> Jerry
>>>
>>>>
>>>> Christian.
>>>>
>>>> Am 04.06.2018 um 07:59 schrieb Zhou, David(ChunMing):
>>>>> Good catch, Reviewed-by: Chunming Zhou <david1.zhou@amd.com>
>>>>>
>>>>> -----Original Message-----
>>>>> From: amd-gfx [mailto:amd-gfx-bounces@lists.freedesktop.org] On 
>>>>> Behalf Of
>>>>> Junwei Zhang
>>>>> Sent: Monday, June 04, 2018 10:04 AM
>>>>> To: amd-gfx@lists.freedesktop.org
>>>>> Cc: Zhang, Jerry <Jerry.Zhang@amd.com>
>>>>> Subject: [PATCH] drm/amdgpu: skip huge page for PRT mapping
>>>>>
>>>>> PRT mapping doesn't support huge page, since it's per PTE basis.
>>>>>
>>>>> Signed-off-by: Junwei Zhang <Jerry.Zhang@amd.com>
>>>>> ---
>>>>>   drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 3 ++-
>>>>>   1 file changed, 2 insertions(+), 1 deletion(-)
>>>>>
>>>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
>>>>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
>>>>> index 850cd66..4ce8bb0 100644
>>>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
>>>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
>>>>> @@ -1111,7 +1111,8 @@ static void amdgpu_vm_handle_huge_pages(struct
>>>>> amdgpu_pte_update_params *p,
>>>>>       /* In the case of a mixed PT the PDE must point to it*/
>>>>>       if (p->adev->asic_type >= CHIP_VEGA10 && !p->src &&
>>>>> -        nptes == AMDGPU_VM_PTE_COUNT(p->adev)) {
>>>>> +        nptes == AMDGPU_VM_PTE_COUNT(p->adev) &&
>>>>> +        !(flags & AMDGPU_PTE_PRT)) {
>>>>>           /* Set the huge page flag to stop scanning at this PDE */
>>>>>           flags |= AMDGPU_PDE_PTE;
>>>>>       }
>>>>
>>

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH] drm/amdgpu: skip huge page for PRT mapping

[Christian König]

I've figured out what's going wrong here.

Your analysis of the problem is correct, but the proposed fix just 
doesn't handle everything.

The issue is that I assumed in the replace operation that the existing 
lose end of the mapping can stay as they are, but that assumption is 
actually not correct because of huge page handling.

This not only affects PRTs, but all mappings in general, so that's a 
rather severe bug we need to fix.

Just send a only compile tested patch with subject "drm/amdgpu: fix 
clear_all and replace handling in the VM" to the list, please give that 
one a try and with your change.

Thanks,
Christian.

Am 04.06.2018 um 11:51 schrieb Christian König:
> Am 04.06.2018 um 10:19 schrieb Zhang, Jerry (Junwei):
>> On 06/04/2018 03:48 PM, Christian König wrote:
>>> Am 04.06.2018 um 09:02 schrieb Zhang, Jerry (Junwei):
>>>> On 06/04/2018 02:43 PM, Christian König wrote:
>>>>> Actually that is not correct. According to the documentation the 
>>>>> PRT flag should
>>>>> work for huge pages as well.
>>>>
>>>> Mmm, I checked the doc earlier, didn't find the PRT flag for PDE.
>>>
>>> The PDE indeed doesn't have the PRT flag, but it also doesn't have 
>>> the fragment
>>> and MTYPE fields and those still work normally when you enable huge 
>>> page handling.
>>>
>>> The trick is that when you set the huge page flag then the PDE is 
>>> handled like a
>>> PTE and so all the extra fields (PRT, fragment size, MTYPE etc...) 
>>> should now be
>>> handled correctly.
>>>
>>> Could be that there is a hardware bug related to PRT handling and 
>>> the huge page
>>> flag, but at least in theory it should work fine.
>>
>> The doc just skips these fields in PDE, if those fields really works 
>> expectedly, even if it doesn't describe the details(but we could 
>> refer to PTE fields), we may have to confirm that with HW guys.
>
> I've just confirmed that huge pages and PRTs should work together. We 
> even have an unit test for that.
>
> I mean we could ping Wade as well, but I'm pretty sure that he will 
> tell you just the same.
>
> I think by disabling huge pages for PRTs we actually hide the real 
> issue somehow, probably some problem with updating PRTs after they are 
> split up.
>
> Can you check if the problem also vanishes when you disable the 
> following optimization in amdgpu_vm_update_ptes?
>>                 /* We don't need to update PTEs for huge pages */
>>                 if (entry->huge)
>>                         continue;
> Just comment this out for a test.
>
>>
>> In my view in current stage, PDE doesn't support PRT and it may not 
>> make much sense to support PRT either.
>> The huge page always happens when reserving PRT range, but later UMD 
>> is likely to bind a/some tiled bo(s) inside this range, that will 
>> break the huge page mapping and split into several pieces mappings, 
>> representing in PTE instead of huge page.
>> In this case, just skipping the huge page for PRT may be an 
>> acceptable way.
>
> Well disabling huge pages for PRTs can definitely cause a huge 
> performance problem because it increases the TLB misses for PRTs by a 
> factor of 512.
>
> And since PRTs are usually used for large only sparse allocated 
> textures that is really really important here.
>
>>
>>>
>>>>
>>>> In CTS PRT test, the reserved PRT mapping introduces huge page 
>>>> mapping, so
>>>> later tiled bo mapping cannot make sure the corresponding PTE is 
>>>> set as PRT.
>>>> Then following access triggers VM fault.
>>>
>>> Interesting, but that rather sounds like a bug in the handling 
>>> instead of a
>>> hardware problem.
>>
>> On the 2nd thinking, we may handle that for huge page.
>> e.g.when binding tiled bo in PRT range, we could split the huge page 
>> into pieces PTE mappings.
>> However, it may just make the code more complex and get the same 
>> results as current fix.
>
> That should already be the case when everything works as expected.
>
>>
>>>
>>> Can you narrow down the CTS test further into a libdrm unit test? Or 
>>> in other
>>> words what exactly does the CTS test do?
>>
>> The issue could be reproduced by below command:
>> {{{
>> deqp-vk -n 
>> dEQP-VK.sparse_resources.buffer.ssbo.sparse_residency.buffer_size_2_24
>> }}}
>>
>> In KMD view, the main process(with amdgpu mainline driver) is like 
>> below:
>> 1) reserve PRT range [0x300400000, 0x300400000 + 0x1000000)
>> 2) bind tiled bos each other page
>>    0x300400000 ~ 0x300401000,
>>    0x300402000 ~ 0x300403000,
>>    0x300404000 ~ 0x300405000,
>>    ...
>> 3) access them all, VM fault at 0x300401000
>
> That sounds like something I should be able to reproduce when your 
> quick test doesn't bring the desired result.
>
> Christian.
>
>>
>> Jerry
>>
>>>
>>> Christian.
>>>
>>>>
>>>> Jerry
>>>>
>>>>>
>>>>> Christian.
>>>>>
>>>>> Am 04.06.2018 um 07:59 schrieb Zhou, David(ChunMing):
>>>>>> Good catch, Reviewed-by: Chunming Zhou <david1.zhou@amd.com>
>>>>>>
>>>>>> -----Original Message-----
>>>>>> From: amd-gfx [mailto:amd-gfx-bounces@lists.freedesktop.org] On 
>>>>>> Behalf Of
>>>>>> Junwei Zhang
>>>>>> Sent: Monday, June 04, 2018 10:04 AM
>>>>>> To: amd-gfx@lists.freedesktop.org
>>>>>> Cc: Zhang, Jerry <Jerry.Zhang@amd.com>
>>>>>> Subject: [PATCH] drm/amdgpu: skip huge page for PRT mapping
>>>>>>
>>>>>> PRT mapping doesn't support huge page, since it's per PTE basis.
>>>>>>
>>>>>> Signed-off-by: Junwei Zhang <Jerry.Zhang@amd.com>
>>>>>> ---
>>>>>>   drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 3 ++-
>>>>>>   1 file changed, 2 insertions(+), 1 deletion(-)
>>>>>>
>>>>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
>>>>>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
>>>>>> index 850cd66..4ce8bb0 100644
>>>>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
>>>>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
>>>>>> @@ -1111,7 +1111,8 @@ static void amdgpu_vm_handle_huge_pages(struct
>>>>>> amdgpu_pte_update_params *p,
>>>>>>       /* In the case of a mixed PT the PDE must point to it*/
>>>>>>       if (p->adev->asic_type >= CHIP_VEGA10 && !p->src &&
>>>>>> -        nptes == AMDGPU_VM_PTE_COUNT(p->adev)) {
>>>>>> +        nptes == AMDGPU_VM_PTE_COUNT(p->adev) &&
>>>>>> +        !(flags & AMDGPU_PTE_PRT)) {
>>>>>>           /* Set the huge page flag to stop scanning at this PDE */
>>>>>>           flags |= AMDGPU_PDE_PTE;
>>>>>>       }
>>>>>
>>>
>

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH] drm/amdgpu: skip huge page for PRT mapping

[Zhang, Jerry (Junwei)]

On 06/04/2018 07:01 PM, Christian König wrote:
> I've figured out what's going wrong here.
>
> Your analysis of the problem is correct, but the proposed fix just doesn't
> handle everything.
>
> The issue is that I assumed in the replace operation that the existing lose end
> of the mapping can stay as they are, but that assumption is actually not correct
> because of huge page handling.
>
> This not only affects PRTs, but all mappings in general, so that's a rather
> severe bug we need to fix.
>
> Just send a only compile tested patch with subject "drm/amdgpu: fix clear_all
> and replace handling in the VM" to the list, please give that one a try and with
> your change.

Sorry, I didn't find the patch in amdgfx or internal group.
If I missed any, please send the patch to me again.

I may get your meaning to handle the huge page in amdgpu_vm_bo_clear_mappings().
If the replaced mapping is huge page, then it has to do mapping for else part.
rather than map the target one only.
(in this case, the "after" part has to do mapping, inheriting the flags from the 
replaced one who has PRT flag)

Try to achieve that locally, found it's a bit tricky to do mapping for else 
part, since the else part(reserved PRT range) bo_va is different from the target 
one(tiled bo). so has to do bo update for else bo_va.
(under debugging...)

>
> Thanks,
> Christian.
>
> Am 04.06.2018 um 11:51 schrieb Christian König:
>> Am 04.06.2018 um 10:19 schrieb Zhang, Jerry (Junwei):
>>> On 06/04/2018 03:48 PM, Christian König wrote:
>>>> Am 04.06.2018 um 09:02 schrieb Zhang, Jerry (Junwei):
>>>>> On 06/04/2018 02:43 PM, Christian König wrote:
>>>>>> Actually that is not correct. According to the documentation the PRT flag
>>>>>> should
>>>>>> work for huge pages as well.
>>>>>
>>>>> Mmm, I checked the doc earlier, didn't find the PRT flag for PDE.
>>>>
>>>> The PDE indeed doesn't have the PRT flag, but it also doesn't have the fragment
>>>> and MTYPE fields and those still work normally when you enable huge page
>>>> handling.
>>>>
>>>> The trick is that when you set the huge page flag then the PDE is handled
>>>> like a
>>>> PTE and so all the extra fields (PRT, fragment size, MTYPE etc...) should
>>>> now be
>>>> handled correctly.
>>>>
>>>> Could be that there is a hardware bug related to PRT handling and the huge page
>>>> flag, but at least in theory it should work fine.
>>>
>>> The doc just skips these fields in PDE, if those fields really works
>>> expectedly, even if it doesn't describe the details(but we could refer to PTE
>>> fields), we may have to confirm that with HW guys.
>>
>> I've just confirmed that huge pages and PRTs should work together. We even
>> have an unit test for that.
>>
>> I mean we could ping Wade as well, but I'm pretty sure that he will tell you
>> just the same.
>>
>> I think by disabling huge pages for PRTs we actually hide the real issue
>> somehow, probably some problem with updating PRTs after they are split up.
>>
>> Can you check if the problem also vanishes when you disable the following
>> optimization in amdgpu_vm_update_ptes?
>>>                 /* We don't need to update PTEs for huge pages */
>>>                 if (entry->huge)
>>>                         continue;
>> Just comment this out for a test.
>>
>>>
>>> In my view in current stage, PDE doesn't support PRT and it may not make much
>>> sense to support PRT either.
>>> The huge page always happens when reserving PRT range, but later UMD is
>>> likely to bind a/some tiled bo(s) inside this range, that will break the huge
>>> page mapping and split into several pieces mappings, representing in PTE
>>> instead of huge page.
>>> In this case, just skipping the huge page for PRT may be an acceptable way.
>>
>> Well disabling huge pages for PRTs can definitely cause a huge performance
>> problem because it increases the TLB misses for PRTs by a factor of 512.
>>
>> And since PRTs are usually used for large only sparse allocated textures that
>> is really really important here.
>>
>>>
>>>>
>>>>>
>>>>> In CTS PRT test, the reserved PRT mapping introduces huge page mapping, so
>>>>> later tiled bo mapping cannot make sure the corresponding PTE is set as PRT.
>>>>> Then following access triggers VM fault.
>>>>
>>>> Interesting, but that rather sounds like a bug in the handling instead of a
>>>> hardware problem.
>>>
>>> On the 2nd thinking, we may handle that for huge page.
>>> e.g.when binding tiled bo in PRT range, we could split the huge page into
>>> pieces PTE mappings.
>>> However, it may just make the code more complex and get the same results as
>>> current fix.
>>
>> That should already be the case when everything works as expected.
>>
>>>
>>>>
>>>> Can you narrow down the CTS test further into a libdrm unit test? Or in other
>>>> words what exactly does the CTS test do?
>>>
>>> The issue could be reproduced by below command:
>>> {{{
>>> deqp-vk -n
>>> dEQP-VK.sparse_resources.buffer.ssbo.sparse_residency.buffer_size_2_24
>>> }}}
>>>
>>> In KMD view, the main process(with amdgpu mainline driver) is like below:
>>> 1) reserve PRT range [0x300400000, 0x300400000 + 0x1000000)
>>> 2) bind tiled bos each other page
>>>    0x300400000 ~ 0x300401000,
>>>    0x300402000 ~ 0x300403000,
>>>    0x300404000 ~ 0x300405000,
>>>    ...
>>> 3) access them all, VM fault at 0x300401000
>>
>> That sounds like something I should be able to reproduce when your quick test
>> doesn't bring the desired result.
>>
>> Christian.
>>
>>>
>>> Jerry
>>>
>>>>
>>>> Christian.
>>>>
>>>>>
>>>>> Jerry
>>>>>
>>>>>>
>>>>>> Christian.
>>>>>>
>>>>>> Am 04.06.2018 um 07:59 schrieb Zhou, David(ChunMing):
>>>>>>> Good catch, Reviewed-by: Chunming Zhou <david1.zhou@amd.com>
>>>>>>>
>>>>>>> -----Original Message-----
>>>>>>> From: amd-gfx [mailto:amd-gfx-bounces@lists.freedesktop.org] On Behalf Of
>>>>>>> Junwei Zhang
>>>>>>> Sent: Monday, June 04, 2018 10:04 AM
>>>>>>> To: amd-gfx@lists.freedesktop.org
>>>>>>> Cc: Zhang, Jerry <Jerry.Zhang@amd.com>
>>>>>>> Subject: [PATCH] drm/amdgpu: skip huge page for PRT mapping
>>>>>>>
>>>>>>> PRT mapping doesn't support huge page, since it's per PTE basis.
>>>>>>>
>>>>>>> Signed-off-by: Junwei Zhang <Jerry.Zhang@amd.com>
>>>>>>> ---
>>>>>>>   drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 3 ++-
>>>>>>>   1 file changed, 2 insertions(+), 1 deletion(-)
>>>>>>>
>>>>>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
>>>>>>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
>>>>>>> index 850cd66..4ce8bb0 100644
>>>>>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
>>>>>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
>>>>>>> @@ -1111,7 +1111,8 @@ static void amdgpu_vm_handle_huge_pages(struct
>>>>>>> amdgpu_pte_update_params *p,
>>>>>>>       /* In the case of a mixed PT the PDE must point to it*/
>>>>>>>       if (p->adev->asic_type >= CHIP_VEGA10 && !p->src &&
>>>>>>> -        nptes == AMDGPU_VM_PTE_COUNT(p->adev)) {
>>>>>>> +        nptes == AMDGPU_VM_PTE_COUNT(p->adev) &&
>>>>>>> +        !(flags & AMDGPU_PTE_PRT)) {
>>>>>>>           /* Set the huge page flag to stop scanning at this PDE */
>>>>>>>           flags |= AMDGPU_PDE_PTE;
>>>>>>>       }
>>>>>>
>>>>
>>
>
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH] drm/amdgpu: skip huge page for PRT mapping

[Zhang, Jerry (Junwei)]

On 06/04/2018 05:51 PM, Christian König wrote:
> Am 04.06.2018 um 10:19 schrieb Zhang, Jerry (Junwei):
>> On 06/04/2018 03:48 PM, Christian König wrote:
>>> Am 04.06.2018 um 09:02 schrieb Zhang, Jerry (Junwei):
>>>> On 06/04/2018 02:43 PM, Christian König wrote:
>>>>> Actually that is not correct. According to the documentation the PRT flag
>>>>> should
>>>>> work for huge pages as well.
>>>>
>>>> Mmm, I checked the doc earlier, didn't find the PRT flag for PDE.
>>>
>>> The PDE indeed doesn't have the PRT flag, but it also doesn't have the fragment
>>> and MTYPE fields and those still work normally when you enable huge page
>>> handling.
>>>
>>> The trick is that when you set the huge page flag then the PDE is handled like a
>>> PTE and so all the extra fields (PRT, fragment size, MTYPE etc...) should now be
>>> handled correctly.
>>>
>>> Could be that there is a hardware bug related to PRT handling and the huge page
>>> flag, but at least in theory it should work fine.
>>
>> The doc just skips these fields in PDE, if those fields really works
>> expectedly, even if it doesn't describe the details(but we could refer to PTE
>> fields), we may have to confirm that with HW guys.
>
> I've just confirmed that huge pages and PRTs should work together. We even have
> an unit test for that.
>
> I mean we could ping Wade as well, but I'm pretty sure that he will tell you
> just the same.
>
> I think by disabling huge pages for PRTs we actually hide the real issue
> somehow, probably some problem with updating PRTs after they are split up.

Thanks to confirm that.
Let's figure it out to support PRT with huge pages.

>
> Can you check if the problem also vanishes when you disable the following
> optimization in amdgpu_vm_update_ptes?
>>                 /* We don't need to update PTEs for huge pages */
>>                 if (entry->huge)
>>                         continue;
> Just comment this out for a test.

Thanks to point it out.
That indeed works well too.

The issue happened by missing to update PTE for reserved PRT range, if huge page 
is introduced.

Not sure if below change is expected to save the huge page for PRT support.
That updates the huge page and fills the correct PTE for PRT as well, different 
from current fix patch.
{{{
                 /* We don't need to update PTEs for huge pages */
-               if (entry->huge)
+               if (entry->huge && !(flags & AMDGPU_PTE_PRT))
                         continue;

}}}


Jerry

>
>>
>> In my view in current stage, PDE doesn't support PRT and it may not make much
>> sense to support PRT either.
>> The huge page always happens when reserving PRT range, but later UMD is likely
>> to bind a/some tiled bo(s) inside this range, that will break the huge page
>> mapping and split into several pieces mappings, representing in PTE instead of
>> huge page.
>> In this case, just skipping the huge page for PRT may be an acceptable way.
>
> Well disabling huge pages for PRTs can definitely cause a huge performance
> problem because it increases the TLB misses for PRTs by a factor of 512.
>
> And since PRTs are usually used for large only sparse allocated textures that is
> really really important here.
>
>>
>>>
>>>>
>>>> In CTS PRT test, the reserved PRT mapping introduces huge page mapping, so
>>>> later tiled bo mapping cannot make sure the corresponding PTE is set as PRT.
>>>> Then following access triggers VM fault.
>>>
>>> Interesting, but that rather sounds like a bug in the handling instead of a
>>> hardware problem.
>>
>> On the 2nd thinking, we may handle that for huge page.
>> e.g.when binding tiled bo in PRT range, we could split the huge page into
>> pieces PTE mappings.
>> However, it may just make the code more complex and get the same results as
>> current fix.
>
> That should already be the case when everything works as expected.
>
>>
>>>
>>> Can you narrow down the CTS test further into a libdrm unit test? Or in other
>>> words what exactly does the CTS test do?
>>
>> The issue could be reproduced by below command:
>> {{{
>> deqp-vk -n dEQP-VK.sparse_resources.buffer.ssbo.sparse_residency.buffer_size_2_24
>> }}}
>>
>> In KMD view, the main process(with amdgpu mainline driver) is like below:
>> 1) reserve PRT range [0x300400000, 0x300400000 + 0x1000000)
>> 2) bind tiled bos each other page
>>    0x300400000 ~ 0x300401000,
>>    0x300402000 ~ 0x300403000,
>>    0x300404000 ~ 0x300405000,
>>    ...
>> 3) access them all, VM fault at 0x300401000
>
> That sounds like something I should be able to reproduce when your quick test
> doesn't bring the desired result.
>
> Christian.
>
>>
>> Jerry
>>
>>>
>>> Christian.
>>>
>>>>
>>>> Jerry
>>>>
>>>>>
>>>>> Christian.
>>>>>
>>>>> Am 04.06.2018 um 07:59 schrieb Zhou, David(ChunMing):
>>>>>> Good catch, Reviewed-by: Chunming Zhou <david1.zhou@amd.com>
>>>>>>
>>>>>> -----Original Message-----
>>>>>> From: amd-gfx [mailto:amd-gfx-bounces@lists.freedesktop.org] On Behalf Of
>>>>>> Junwei Zhang
>>>>>> Sent: Monday, June 04, 2018 10:04 AM
>>>>>> To: amd-gfx@lists.freedesktop.org
>>>>>> Cc: Zhang, Jerry <Jerry.Zhang@amd.com>
>>>>>> Subject: [PATCH] drm/amdgpu: skip huge page for PRT mapping
>>>>>>
>>>>>> PRT mapping doesn't support huge page, since it's per PTE basis.
>>>>>>
>>>>>> Signed-off-by: Junwei Zhang <Jerry.Zhang@amd.com>
>>>>>> ---
>>>>>>   drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 3 ++-
>>>>>>   1 file changed, 2 insertions(+), 1 deletion(-)
>>>>>>
>>>>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
>>>>>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
>>>>>> index 850cd66..4ce8bb0 100644
>>>>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
>>>>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
>>>>>> @@ -1111,7 +1111,8 @@ static void amdgpu_vm_handle_huge_pages(struct
>>>>>> amdgpu_pte_update_params *p,
>>>>>>       /* In the case of a mixed PT the PDE must point to it*/
>>>>>>       if (p->adev->asic_type >= CHIP_VEGA10 && !p->src &&
>>>>>> -        nptes == AMDGPU_VM_PTE_COUNT(p->adev)) {
>>>>>> +        nptes == AMDGPU_VM_PTE_COUNT(p->adev) &&
>>>>>> +        !(flags & AMDGPU_PTE_PRT)) {
>>>>>>           /* Set the huge page flag to stop scanning at this PDE */
>>>>>>           flags |= AMDGPU_PDE_PTE;
>>>>>>       }
>>>>>
>>>
>
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH] drm/amdgpu: skip huge page for PRT mapping

[Christian König]

Hi Jerry,

Am 05.06.2018 um 03:50 schrieb Zhang, Jerry (Junwei):
> [SNIP]
>>
>> Can you check if the problem also vanishes when you disable the 
>> following
>> optimization in amdgpu_vm_update_ptes?
>>>                 /* We don't need to update PTEs for huge pages */
>>>                 if (entry->huge)
>>>                         continue;
>> Just comment this out for a test.
>
> Thanks to point it out.
> That indeed works well too.
>
> The issue happened by missing to update PTE for reserved PRT range, if 
> huge page is introduced.
>
> Not sure if below change is expected to save the huge page for PRT 
> support.
> That updates the huge page and fills the correct PTE for PRT as well, 
> different from current fix patch.
> {{{
>                 /* We don't need to update PTEs for huge pages */
> -               if (entry->huge)
> +               if (entry->huge && !(flags & AMDGPU_PTE_PRT))
>                         continue;
>
> }}}

problem is that not only affects PRT, but all mappings.

See the patch here if you currently can't receive mails from amd-gfx: 
https://patchwork.freedesktop.org/patch/227336/

Essentially I made a wrong assumption in the clear all code path. Please 
also note that I hadn't time to fully test this patch yet.

Christian.

>
>
> Jerry

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH] drm/amdgpu: skip huge page for PRT mapping

[Zhang, Jerry (Junwei)]

On 06/05/2018 02:20 PM, Christian König wrote:
> Hi Jerry,
>
> Am 05.06.2018 um 03:50 schrieb Zhang, Jerry (Junwei):
>> [SNIP]
>>>
>>> Can you check if the problem also vanishes when you disable the following
>>> optimization in amdgpu_vm_update_ptes?
>>>>                 /* We don't need to update PTEs for huge pages */
>>>>                 if (entry->huge)
>>>>                         continue;
>>> Just comment this out for a test.
>>
>> Thanks to point it out.
>> That indeed works well too.
>>
>> The issue happened by missing to update PTE for reserved PRT range, if huge
>> page is introduced.
>>
>> Not sure if below change is expected to save the huge page for PRT support.
>> That updates the huge page and fills the correct PTE for PRT as well,
>> different from current fix patch.
>> {{{
>>                 /* We don't need to update PTEs for huge pages */
>> -               if (entry->huge)
>> +               if (entry->huge && !(flags & AMDGPU_PTE_PRT))
>>                         continue;
>>
>> }}}
>
> problem is that not only affects PRT, but all mappings.
>
> See the patch here if you currently can't receive mails from amd-gfx:
> https://patchwork.freedesktop.org/patch/227336/

Thanks for your info.
We may also need to add bo_va info, since the next replace(with clear mapping) 
will use that.
{{{
@@ -2172,6 +2172,7 @@ int amdgpu_vm_bo_clear_mappings(struct amdgpu_device *adev,
                         before->last = saddr - 1;
                         before->offset = tmp->offset;
                         before->flags = tmp->flags;
+                       before->bo_va = tmp->bo_va;
                         list_add(&before->list, &tmp->bo_va->invalids);
                 }

@@ -2182,6 +2183,7 @@ int amdgpu_vm_bo_clear_mappings(struct amdgpu_device *adev,
                         after->offset = tmp->offset;
                         after->offset += after->start - tmp->start;
                         after->flags = tmp->flags;
+                       after->bo_va = tmp->bo_va;
                         list_add(&after->list, &tmp->bo_va->invalids);

}}}

>
> Essentially I made a wrong assumption in the clear all code path. Please also
> note that I hadn't time to fully test this patch yet.

Yes, i will test it more.
And it could fix PRT issue as well.(without my patch)

Jerry

>
> Christian.
>
>>
>>
>> Jerry
>
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx