Need to support mixed memory mappings with HMM

[Felix Kuehling]

Am 2021-03-25 um 12:22 p.m. schrieb Christian König:
>>>>>> My idea is to change the amdgpu_vm_update_mapping interface to use
>>>>>> some
>>>>>> high-bit in the pages_addr array to indicate the page type. For the
>>>>>> default page type (0) nothing really changes for the callers. The
>>>>>> "flags" parameter needs to become a pointer to an array that gets
>>>>>> indexed by the high bits from the pages_addr array. For existing
>>>>>> callers
>>>>>> it's as easy as changing flags to &flags (array of size 1). For
>>>>>> HMM we
>>>>>> would pass a pointer to a real array.
>>>>> Yeah, I've thought about stuff like that as well for a while.
>>>>>
>>>>> Problem is that this won't work that easily. We assume at many places
>>>>> that the flags doesn't change for the range in question.
>>>> I think some lower level functions assume that the flags stay the same
>>>> for physically contiguous ranges. But if you use the high-bits to
>>>> encode
>>>> the page type, the ranges won't be contiguous any more. So you can
>>>> change page flags for different contiguous ranges.
>>> Yeah, but then you also get absolutely zero THP and fragment flags
>>> support.
>> As long as you have a contiguous 2MB page with the same page type, I
>> think you can still get a THP mapping in the GPU page table. But if one
>> page in the middle of a 2MB page has a different page type, that will
>> break the THP mapping, as it should.
>
> Yeah, but currently we detect that before we call down into the
> functions to update the tables.
>
> When you give a mixed list the chance for that is just completely gone.

Currently the detection of contiguous ranges is in amdgpu_vm_update_mapping:

> if (num_entries > AMDGPU_GPU_PAGES_IN_CPU_PAGE) { uint64_t pfn =
> cursor.start >> PAGE_SHIFT; uint64_t count; contiguous =
> pages_addr[pfn + 1] == pages_addr[pfn] + PAGE_SIZE; tmp = num_entries
> / AMDGPU_GPU_PAGES_IN_CPU_PAGE; for (count = 2; count < tmp; ++count)
> { uint64_t idx = pfn + count; if (contiguous != (pages_addr[idx] ==
> pages_addr[idx - 1] + PAGE_SIZE)) break; } num_entries = count *
> AMDGPU_GPU_PAGES_IN_CPU_PAGE; } 

If a page type changes from one page to the next, it will end a
contiguous range and call into the lower level (amdgpu_vm_update_ptes).
I don't think anything needs to change in amdgpu_vm_update_ptes, because
all pages contiguous passed to it use the same page type, so the same
flags. And the existing code in amdgpu_vm_update_mapping already does
the right thing. I honestly don't see the problem.

Regards,
  Felix


>
> Regards,
> Christian.