[PATCH 6/8] drm/amdgpu: meld together VM fragment and huge page handling
Samuel Pitoiset
samuel.pitoiset at gmail.com
Thu Nov 8 19:35:08 UTC 2018
On 11/8/18 5:50 PM, Christian König wrote:
> Hi Samuel,
>
> yeah, that is a known issue and I was already looking into that all day
> today. Problem is that I can't reproduce it reliable.
>
> Do you have any hint how to trigger that? E.g. memory exhaustion maybe?
Hi Christian,
Good to hear that you are already working on.
The GPU hang with Rise of Tomb Raider is reliable on my side. It always
hangs while loading the benchmark. I use the Ultra High preset.
Do you have access to that game? As mentioned, F1 2017 is also affected
but I used RoTR during my bisect.
Thanks.
>
> Thanks,
> Christian.
>
> Am 08.11.18 um 17:17 schrieb Samuel Pitoiset:
>> Hi,
>>
>> This change introduces GPU hangs with F1 2017 and Rise of Tomb Raider
>> on RADV/GFX9. I bisected the issue.
>>
>> Can you have a look?
>>
>> Thanks!
>>
>> On 9/12/18 10:54 AM, Christian König wrote:
>>> This optimizes the generating of PTEs by walking the hierarchy only once
>>> for a range and making changes as necessary.
>>>
>>> It allows for both huge (2MB) as well giant (1GB) pages to be used on
>>> Vega and Raven.
>>>
>>> Signed-off-by: Christian König <christian.koenig at amd.com>
>>> Reviewed-by: Felix Kuehling <Felix.Kuehling at amd.com>
>>> Acked-by: Junwei Zhang <Jerry.Zhang at amd.com>
>>> ---
>>> drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 267
>>> ++++++++++++++++++---------------
>>> 1 file changed, 147 insertions(+), 120 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
>>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
>>> index 747b6ff6fea7..328325324a1d 100644
>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
>>> @@ -1488,46 +1488,76 @@ int amdgpu_vm_update_directories(struct
>>> amdgpu_device *adev,
>>> }
>>> /**
>>> - * amdgpu_vm_handle_huge_pages - handle updating the PD with huge pages
>>> + * amdgpu_vm_update_huge - figure out parameters for PTE updates
>>> *
>>> - * @p: see amdgpu_pte_update_params definition
>>> - * @entry: vm_pt entry to check
>>> - * @parent: parent entry
>>> - * @nptes: number of PTEs updated with this operation
>>> - * @dst: destination address where the PTEs should point to
>>> - * @flags: access flags fro the PTEs
>>> - *
>>> - * Check if we can update the PD with a huge page.
>>> + * Make sure to set the right flags for the PTEs at the desired level.
>>> */
>>> -static void amdgpu_vm_handle_huge_pages(struct
>>> amdgpu_pte_update_params *p,
>>> - struct amdgpu_vm_pt *entry,
>>> - struct amdgpu_vm_pt *parent,
>>> - unsigned nptes, uint64_t dst,
>>> - uint64_t flags)
>>> -{
>>> - uint64_t pde;
>>> +static void amdgpu_vm_update_huge(struct amdgpu_pte_update_params
>>> *params,
>>> + struct amdgpu_bo *bo, unsigned level,
>>> + uint64_t pe, uint64_t addr,
>>> + unsigned count, uint32_t incr,
>>> + uint64_t flags)
>>> - /* 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)) {
>>> - /* Set the huge page flag to stop scanning at this PDE */
>>> +{
>>> + if (level != AMDGPU_VM_PTB) {
>>> flags |= AMDGPU_PDE_PTE;
>>> + amdgpu_gmc_get_vm_pde(params->adev, level, &addr, &flags);
>>> }
>>> - if (!(flags & AMDGPU_PDE_PTE)) {
>>> - if (entry->huge) {
>>> - /* Add the entry to the relocated list to update it. */
>>> - entry->huge = false;
>>> - amdgpu_vm_bo_relocated(&entry->base);
>>> - }
>>> + amdgpu_vm_update_func(params, bo, pe, addr, count, incr, flags);
>>> +}
>>> +
>>> +/**
>>> + * amdgpu_vm_fragment - get fragment for PTEs
>>> + *
>>> + * @params: see amdgpu_pte_update_params definition
>>> + * @start: first PTE to handle
>>> + * @end: last PTE to handle
>>> + * @flags: hw mapping flags
>>> + * @frag: resulting fragment size
>>> + * @frag_end: end of this fragment
>>> + *
>>> + * Returns the first possible fragment for the start and end address.
>>> + */
>>> +static void amdgpu_vm_fragment(struct amdgpu_pte_update_params *params,
>>> + uint64_t start, uint64_t end, uint64_t flags,
>>> + unsigned int *frag, uint64_t *frag_end)
>>> +{
>>> + /**
>>> + * The MC L1 TLB supports variable sized pages, based on a fragment
>>> + * field in the PTE. When this field is set to a non-zero value,
>>> page
>>> + * granularity is increased from 4KB to (1 << (12 + frag)). The PTE
>>> + * flags are considered valid for all PTEs within the fragment
>>> range
>>> + * and corresponding mappings are assumed to be physically
>>> contiguous.
>>> + *
>>> + * The L1 TLB can store a single PTE for the whole fragment,
>>> + * significantly increasing the space available for translation
>>> + * caching. This leads to large improvements in throughput when the
>>> + * TLB is under pressure.
>>> + *
>>> + * The L2 TLB distributes small and large fragments into two
>>> + * asymmetric partitions. The large fragment cache is significantly
>>> + * larger. Thus, we try to use large fragments wherever possible.
>>> + * Userspace can support this by aligning virtual base address and
>>> + * allocation size to the fragment size.
>>> + */
>>> + unsigned max_frag = params->adev->vm_manager.fragment_size;
>>> +
>>> + /* system pages are non continuously */
>>> + if (params->src || !(flags & AMDGPU_PTE_VALID)) {
>>> + *frag = 0;
>>> + *frag_end = end;
>>> return;
>>> }
>>> - entry->huge = true;
>>> - amdgpu_gmc_get_vm_pde(p->adev, AMDGPU_VM_PDB0, &dst, &flags);
>>> -
>>> - pde = (entry - parent->entries) * 8;
>>> - amdgpu_vm_update_func(p, parent->base.bo, pde, dst, 1, 0, flags);
>>> + /* This intentionally wraps around if no bit is set */
>>> + *frag = min((unsigned)ffs(start) - 1, (unsigned)fls64(end -
>>> start) - 1);
>>> + if (*frag >= max_frag) {
>>> + *frag = max_frag;
>>> + *frag_end = end & ~((1ULL << max_frag) - 1);
>>> + } else {
>>> + *frag_end = start + (1 << *frag);
>>> + }
>>> }
>>> /**
>>> @@ -1545,108 +1575,105 @@ static void
>>> amdgpu_vm_handle_huge_pages(struct amdgpu_pte_update_params *p,
>>> * 0 for success, -EINVAL for failure.
>>> */
>>> static int amdgpu_vm_update_ptes(struct amdgpu_pte_update_params
>>> *params,
>>> - uint64_t start, uint64_t end,
>>> - uint64_t dst, uint64_t flags)
>>> + uint64_t start, uint64_t end,
>>> + uint64_t dst, uint64_t flags)
>>> {
>>> struct amdgpu_device *adev = params->adev;
>>> - const uint64_t mask = AMDGPU_VM_PTE_COUNT(adev) - 1;
>>> struct amdgpu_vm_pt_cursor cursor;
>>> + uint64_t frag_start = start, frag_end;
>>> + unsigned int frag;
>>> - /* walk over the address space and update the page tables */
>>> - for_each_amdgpu_vm_pt_leaf(adev, params->vm, start, end - 1,
>>> cursor) {
>>> + /* figure out the initial fragment */
>>> + amdgpu_vm_fragment(params, frag_start, end, flags, &frag,
>>> &frag_end);
>>> +
>>> + /* walk over the address space and update the PTs */
>>> + amdgpu_vm_pt_start(adev, params->vm, start, &cursor);
>>> + while (cursor.pfn < end) {
>>> struct amdgpu_bo *pt = cursor.entry->base.bo;
>>> - uint64_t pe_start;
>>> - unsigned nptes;
>>> + unsigned shift, parent_shift, num_entries;
>>> + uint64_t incr, entry_end, pe_start;
>>> - if (!pt || cursor.level != AMDGPU_VM_PTB)
>>> + if (!pt)
>>> return -ENOENT;
>>> - if ((cursor.pfn & ~mask) == (end & ~mask))
>>> - nptes = end - cursor.pfn;
>>> - else
>>> - nptes = AMDGPU_VM_PTE_COUNT(adev) - (cursor.pfn & mask);
>>> -
>>> - amdgpu_vm_handle_huge_pages(params, cursor.entry,
>>> cursor.parent,
>>> - nptes, dst, flags);
>>> - /* We don't need to update PTEs for huge pages */
>>> - if (cursor.entry->huge) {
>>> - dst += nptes * AMDGPU_GPU_PAGE_SIZE;
>>> + /* The root level can't be a huge page */
>>> + if (cursor.level == adev->vm_manager.root_level) {
>>> + if (!amdgpu_vm_pt_descendant(adev, &cursor))
>>> + return -ENOENT;
>>> continue;
>>> }
>>> - pe_start = (cursor.pfn & mask) * 8;
>>> - amdgpu_vm_update_func(params, pt, pe_start, dst, nptes,
>>> - AMDGPU_GPU_PAGE_SIZE, flags);
>>> - dst += nptes * AMDGPU_GPU_PAGE_SIZE;
>>> - }
>>> -
>>> - return 0;
>>> -}
>>> + /* First check if the entry is already handled */
>>> + if (cursor.pfn < frag_start) {
>>> + cursor.entry->huge = true;
>>> + amdgpu_vm_pt_next(adev, &cursor);
>>> + continue;
>>> + }
>>> -/*
>>> - * amdgpu_vm_frag_ptes - add fragment information to PTEs
>>> - *
>>> - * @params: see amdgpu_pte_update_params definition
>>> - * @vm: requested vm
>>> - * @start: first PTE to handle
>>> - * @end: last PTE to handle
>>> - * @dst: addr those PTEs should point to
>>> - * @flags: hw mapping flags
>>> - *
>>> - * Returns:
>>> - * 0 for success, -EINVAL for failure.
>>> - */
>>> -static int amdgpu_vm_frag_ptes(struct amdgpu_pte_update_params *params,
>>> - uint64_t start, uint64_t end,
>>> - uint64_t dst, uint64_t flags)
>>> -{
>>> - /**
>>> - * The MC L1 TLB supports variable sized pages, based on a fragment
>>> - * field in the PTE. When this field is set to a non-zero value,
>>> page
>>> - * granularity is increased from 4KB to (1 << (12 + frag)). The PTE
>>> - * flags are considered valid for all PTEs within the fragment
>>> range
>>> - * and corresponding mappings are assumed to be physically
>>> contiguous.
>>> - *
>>> - * The L1 TLB can store a single PTE for the whole fragment,
>>> - * significantly increasing the space available for translation
>>> - * caching. This leads to large improvements in throughput when the
>>> - * TLB is under pressure.
>>> - *
>>> - * The L2 TLB distributes small and large fragments into two
>>> - * asymmetric partitions. The large fragment cache is significantly
>>> - * larger. Thus, we try to use large fragments wherever possible.
>>> - * Userspace can support this by aligning virtual base address and
>>> - * allocation size to the fragment size.
>>> - */
>>> - unsigned max_frag = params->adev->vm_manager.fragment_size;
>>> - int r;
>>> + /* If it isn't already handled it can't be a huge page */
>>> + if (cursor.entry->huge) {
>>> + /* Add the entry to the relocated list to update it. */
>>> + cursor.entry->huge = false;
>>> + amdgpu_vm_bo_relocated(&cursor.entry->base);
>>> + }
>>> - /* system pages are non continuously */
>>> - if (params->src || !(flags & AMDGPU_PTE_VALID))
>>> - return amdgpu_vm_update_ptes(params, start, end, dst, flags);
>>> -
>>> - while (start != end) {
>>> - uint64_t frag_flags, frag_end;
>>> - unsigned frag;
>>> -
>>> - /* This intentionally wraps around if no bit is set */
>>> - frag = min((unsigned)ffs(start) - 1,
>>> - (unsigned)fls64(end - start) - 1);
>>> - if (frag >= max_frag) {
>>> - frag_flags = AMDGPU_PTE_FRAG(max_frag);
>>> - frag_end = end & ~((1ULL << max_frag) - 1);
>>> - } else {
>>> - frag_flags = AMDGPU_PTE_FRAG(frag);
>>> - frag_end = start + (1 << frag);
>>> + shift = amdgpu_vm_level_shift(adev, cursor.level);
>>> + parent_shift = amdgpu_vm_level_shift(adev, cursor.level - 1);
>>> + if (adev->asic_type < CHIP_VEGA10) {
>>> + /* No huge page support before GMC v9 */
>>> + if (cursor.level != AMDGPU_VM_PTB) {
>>> + if (!amdgpu_vm_pt_descendant(adev, &cursor))
>>> + return -ENOENT;
>>> + continue;
>>> + }
>>> + } else if (frag < shift) {
>>> + /* We can't use this level when the fragment size is
>>> + * smaller than the address shift. Go to the next
>>> + * child entry and try again.
>>> + */
>>> + if (!amdgpu_vm_pt_descendant(adev, &cursor))
>>> + return -ENOENT;
>>> + continue;
>>> + } else if (frag >= parent_shift) {
>>> + /* If the fragment size is even larger than the parent
>>> + * shift we should go up one level and check it again.
>>> + */
>>> + if (!amdgpu_vm_pt_ancestor(&cursor))
>>> + return -ENOENT;
>>> + continue;
>>> }
>>> - r = amdgpu_vm_update_ptes(params, start, frag_end, dst,
>>> - flags | frag_flags);
>>> - if (r)
>>> - return r;
>>> + /* Looks good so far, calculate parameters for the update */
>>> + incr = AMDGPU_GPU_PAGE_SIZE << shift;
>>> + num_entries = amdgpu_vm_num_entries(adev, cursor.level);
>>> + pe_start = ((cursor.pfn >> shift) & (num_entries - 1)) * 8;
>>> + entry_end = num_entries << shift;
>>> + entry_end += cursor.pfn & ~(entry_end - 1);
>>> + entry_end = min(entry_end, end);
>>> +
>>> + do {
>>> + uint64_t upd_end = min(entry_end, frag_end);
>>> + unsigned nptes = (upd_end - frag_start) >> shift;
>>> +
>>> + amdgpu_vm_update_huge(params, pt, cursor.level,
>>> + pe_start, dst, nptes, incr,
>>> + flags | AMDGPU_PTE_FRAG(frag));
>>> +
>>> + pe_start += nptes * 8;
>>> + dst += nptes * AMDGPU_GPU_PAGE_SIZE << shift;
>>> +
>>> + frag_start = upd_end;
>>> + if (frag_start >= frag_end) {
>>> + /* figure out the next fragment */
>>> + amdgpu_vm_fragment(params, frag_start, end,
>>> + flags, &frag, &frag_end);
>>> + if (frag < shift)
>>> + break;
>>> + }
>>> + } while (frag_start < entry_end);
>>> - dst += (frag_end - start) * AMDGPU_GPU_PAGE_SIZE;
>>> - start = frag_end;
>>> + if (frag >= shift)
>>> + amdgpu_vm_pt_next(adev, &cursor);
>>> }
>>> return 0;
>>> @@ -1708,8 +1735,8 @@ static int amdgpu_vm_bo_update_mapping(struct
>>> amdgpu_device *adev,
>>> params.func = amdgpu_vm_cpu_set_ptes;
>>> params.pages_addr = pages_addr;
>>> - return amdgpu_vm_frag_ptes(¶ms, start, last + 1,
>>> - addr, flags);
>>> + return amdgpu_vm_update_ptes(¶ms, start, last + 1,
>>> + addr, flags);
>>> }
>>> ring = container_of(vm->entity.rq->sched, struct amdgpu_ring,
>>> sched);
>>> @@ -1788,7 +1815,7 @@ static int amdgpu_vm_bo_update_mapping(struct
>>> amdgpu_device *adev,
>>> if (r)
>>> goto error_free;
>>> - r = amdgpu_vm_frag_ptes(¶ms, start, last + 1, addr, flags);
>>> + r = amdgpu_vm_update_ptes(¶ms, start, last + 1, addr, flags);
>>> if (r)
>>> goto error_free;
>>>
>
More information about the amd-gfx
mailing list