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

["Zhang, Jerry (Junwei)" <Jerry.Zhang-5C7GfCeVMHo@public.gmane.org>]

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