On 2017å¹´12æ??08æ?¥ 22:00, Christian König wrote: > What is wrong with the old approach? When the num entries are less than one page(512), the old approach will result in error. e.g. when we enable translate further feature, the ptb will only have 32 entries with 64KB---16 * 4KB selection. > > I would rather say that the address should be limited by the level > shift instead. This way we avoid the modulo altogether. already in your patch set. Regards, David Zhou > > Christian. > > Am 08.12.2017 um 11:56 schrieb Chunming Zhou: >> Change-Id: I40ecf31ad4b51022a2c0c076ae45188b6e9d63de >> Signed-off-by: Chunming Zhou <david1.zhou at amd.com> >> --- >>  drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 6 ++++-- >>  1 file changed, 4 insertions(+), 2 deletions(-) >> >> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c >> b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c >> index 8904ccf78fc9..affe64e42cef 100644 >> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c >> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c >> @@ -1335,11 +1335,13 @@ void amdgpu_vm_get_entry(struct >> amdgpu_pte_update_params *p, uint64_t addr, >>      *parent = NULL; >>      *entry = &p->vm->root; >>      while ((*entry)->entries) { >> -       unsigned idx = addr >> amdgpu_vm_level_shift(p->adev, level--); >> +       unsigned idx = addr >> amdgpu_vm_level_shift(p->adev, level); >>  -       idx %= amdgpu_bo_size((*entry)->base.bo) / 8; >> +       idx %= amdgpu_vm_num_entries(p->adev, level); >>          *parent = *entry; >>          *entry = &(*entry)->entries[idx]; >> +       if (level) >> +           level--; >>      } >>       if (level != 0) >