Am 29.03.2017 um 17:48 schrieb Felix Kuehling:
Fix the start/end address calculation for address ranges that span
multiple page directories in amdgpu_vm_alloc_levels.
Add WARN_ONs if page tables aren't found. Otherwise the page table
update would just fail silently.
v2:
* Change WARN_ON to WARN_ON_ONCE
* Move masking of high address bits to caller
* Add range-check for "from" and "to"
Signed-off-by: Felix Kuehling <felix.kuehl...@amd.com>
---
drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 20 +++++++++++++++-----
1 file changed, 15 insertions(+), 5 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
index 818747f..033852b 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
@@ -275,13 +275,18 @@ static int amdgpu_vm_alloc_levels(struct amdgpu_device
*adev,
memset(parent->entries, 0 , sizeof(struct amdgpu_vm_pt));
}
- from = (saddr >> shift) % amdgpu_vm_num_entries(adev, level);
- to = (eaddr >> shift) % amdgpu_vm_num_entries(adev, level);
+ from = saddr >> shift;
+ to = eaddr >> shift;
+ if (from >= amdgpu_vm_num_entries(adev, level) ||
+ to >= amdgpu_vm_num_entries(adev, level))
+ return -EINVAL;
if (to > parent->last_entry_used)
parent->last_entry_used = to;
++level;
+ saddr = saddr & ((1 << shift) - 1);
+ eaddr = eaddr & ((1 << shift) - 1);
Why do you need this part? The modulo above should have handled that.
/* walk over the address space and allocate the page tables */
for (pt_idx = from; pt_idx <= to; ++pt_idx) {
@@ -312,8 +317,11 @@ static int amdgpu_vm_alloc_levels(struct amdgpu_device
*adev,
}
if (level < adev->vm_manager.num_level) {
- r = amdgpu_vm_alloc_levels(adev, vm, entry, saddr,
- eaddr, level);
+ uint64_t sub_saddr = (pt_idx == from) ? saddr : 0;
+ uint64_t sub_eaddr = (pt_idx == to) ? eaddr :
+ ((1 << shift) - 1);
+ r = amdgpu_vm_alloc_levels(adev, vm, entry, sub_saddr,
+ sub_eaddr, level);
if (r)
return r;
}
@@ -957,9 +965,11 @@ static struct amdgpu_bo *amdgpu_vm_get_pt(struct
amdgpu_pte_update_params *p,
entry = &entry->entries[idx];
}
- if (level)
+ if (WARN_ON_ONCE(level))
return NULL;
+ WARN_ON_ONCE(!entry->bo);
+
Please drop those warnings. It is perfectly valid not to have the PT
allocated here.
Rather add a warning to the caller if it needs the BO.
return entry->bo;
}
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
