Skip to content

Commit ed2bdf3

Browse files
author
Thomas Hellström
committed
drm/xe/vm: Subclass userptr vmas
The construct allocating only parts of the vma structure when the userptr part is not needed is very fragile. A developer could add additional fields below the userptr part, and the code could easily attempt to access the userptr part even if its not persent. So introduce xe_userptr_vma which subclasses struct xe_vma the proper way, and accordingly modify a couple of interfaces. This should also help if adding userptr helpers to drm_gpuvm. v2: - Fix documentation of to_userptr_vma() (Matthew Brost) - Fix allocation and freeing of vmas to clearer distinguish between the types. Closes: https://lore.kernel.org/intel-xe/0c4cc1a7-f409-4597-b110-81f9e45d1ffe@embeddedor.com/T/#u Fixes: a4cc60a ("drm/xe: Only alloc userptr part of xe_vma for userptrs") Cc: Rodrigo Vivi <rodrigo.vivi@intel.com> Cc: Matthew Brost <matthew.brost@intel.com> Cc: Lucas De Marchi <lucas.demarchi@intel.com> Signed-off-by: Thomas Hellström <thomas.hellstrom@linux.intel.com> Reviewed-by: Matthew Brost <matthew.brost@intel.com> Link: https://patchwork.freedesktop.org/patch/msgid/20240131091628.12318-1-thomas.hellstrom@linux.intel.com (cherry picked from commit 5bd24e7) Signed-off-by: Thomas Hellström <thomas.hellstrom@linux.intel.com>
1 parent 89642db commit ed2bdf3

File tree

5 files changed

+137
-93
lines changed

5 files changed

+137
-93
lines changed

drivers/gpu/drm/xe/xe_gt_pagefault.c

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -165,7 +165,8 @@ static int handle_pagefault(struct xe_gt *gt, struct pagefault *pf)
165165
goto unlock_vm;
166166
}
167167

168-
if (!xe_vma_is_userptr(vma) || !xe_vma_userptr_check_repin(vma)) {
168+
if (!xe_vma_is_userptr(vma) ||
169+
!xe_vma_userptr_check_repin(to_userptr_vma(vma))) {
169170
downgrade_write(&vm->lock);
170171
write_locked = false;
171172
}
@@ -181,11 +182,13 @@ static int handle_pagefault(struct xe_gt *gt, struct pagefault *pf)
181182
/* TODO: Validate fault */
182183

183184
if (xe_vma_is_userptr(vma) && write_locked) {
185+
struct xe_userptr_vma *uvma = to_userptr_vma(vma);
186+
184187
spin_lock(&vm->userptr.invalidated_lock);
185-
list_del_init(&vma->userptr.invalidate_link);
188+
list_del_init(&uvma->userptr.invalidate_link);
186189
spin_unlock(&vm->userptr.invalidated_lock);
187190

188-
ret = xe_vma_userptr_pin_pages(vma);
191+
ret = xe_vma_userptr_pin_pages(uvma);
189192
if (ret)
190193
goto unlock_vm;
191194

@@ -220,7 +223,7 @@ static int handle_pagefault(struct xe_gt *gt, struct pagefault *pf)
220223
dma_fence_put(fence);
221224

222225
if (xe_vma_is_userptr(vma))
223-
ret = xe_vma_userptr_check_repin(vma);
226+
ret = xe_vma_userptr_check_repin(to_userptr_vma(vma));
224227
vma->usm.tile_invalidated &= ~BIT(tile->id);
225228

226229
unlock_dma_resv:

drivers/gpu/drm/xe/xe_pt.c

Lines changed: 16 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -618,8 +618,8 @@ xe_pt_stage_bind(struct xe_tile *tile, struct xe_vma *vma,
618618

619619
if (!xe_vma_is_null(vma)) {
620620
if (xe_vma_is_userptr(vma))
621-
xe_res_first_sg(vma->userptr.sg, 0, xe_vma_size(vma),
622-
&curs);
621+
xe_res_first_sg(to_userptr_vma(vma)->userptr.sg, 0,
622+
xe_vma_size(vma), &curs);
623623
else if (xe_bo_is_vram(bo) || xe_bo_is_stolen(bo))
624624
xe_res_first(bo->ttm.resource, xe_vma_bo_offset(vma),
625625
xe_vma_size(vma), &curs);
@@ -906,17 +906,17 @@ static void xe_vm_dbg_print_entries(struct xe_device *xe,
906906

907907
#ifdef CONFIG_DRM_XE_USERPTR_INVAL_INJECT
908908

909-
static int xe_pt_userptr_inject_eagain(struct xe_vma *vma)
909+
static int xe_pt_userptr_inject_eagain(struct xe_userptr_vma *uvma)
910910
{
911-
u32 divisor = vma->userptr.divisor ? vma->userptr.divisor : 2;
911+
u32 divisor = uvma->userptr.divisor ? uvma->userptr.divisor : 2;
912912
static u32 count;
913913

914914
if (count++ % divisor == divisor - 1) {
915-
struct xe_vm *vm = xe_vma_vm(vma);
915+
struct xe_vm *vm = xe_vma_vm(&uvma->vma);
916916

917-
vma->userptr.divisor = divisor << 1;
917+
uvma->userptr.divisor = divisor << 1;
918918
spin_lock(&vm->userptr.invalidated_lock);
919-
list_move_tail(&vma->userptr.invalidate_link,
919+
list_move_tail(&uvma->userptr.invalidate_link,
920920
&vm->userptr.invalidated);
921921
spin_unlock(&vm->userptr.invalidated_lock);
922922
return true;
@@ -927,7 +927,7 @@ static int xe_pt_userptr_inject_eagain(struct xe_vma *vma)
927927

928928
#else
929929

930-
static bool xe_pt_userptr_inject_eagain(struct xe_vma *vma)
930+
static bool xe_pt_userptr_inject_eagain(struct xe_userptr_vma *uvma)
931931
{
932932
return false;
933933
}
@@ -1000,9 +1000,9 @@ static int xe_pt_userptr_pre_commit(struct xe_migrate_pt_update *pt_update)
10001000
{
10011001
struct xe_pt_migrate_pt_update *userptr_update =
10021002
container_of(pt_update, typeof(*userptr_update), base);
1003-
struct xe_vma *vma = pt_update->vma;
1004-
unsigned long notifier_seq = vma->userptr.notifier_seq;
1005-
struct xe_vm *vm = xe_vma_vm(vma);
1003+
struct xe_userptr_vma *uvma = to_userptr_vma(pt_update->vma);
1004+
unsigned long notifier_seq = uvma->userptr.notifier_seq;
1005+
struct xe_vm *vm = xe_vma_vm(&uvma->vma);
10061006
int err = xe_pt_vm_dependencies(pt_update->job,
10071007
&vm->rftree[pt_update->tile_id],
10081008
pt_update->start,
@@ -1023,7 +1023,7 @@ static int xe_pt_userptr_pre_commit(struct xe_migrate_pt_update *pt_update)
10231023
*/
10241024
do {
10251025
down_read(&vm->userptr.notifier_lock);
1026-
if (!mmu_interval_read_retry(&vma->userptr.notifier,
1026+
if (!mmu_interval_read_retry(&uvma->userptr.notifier,
10271027
notifier_seq))
10281028
break;
10291029

@@ -1032,11 +1032,11 @@ static int xe_pt_userptr_pre_commit(struct xe_migrate_pt_update *pt_update)
10321032
if (userptr_update->bind)
10331033
return -EAGAIN;
10341034

1035-
notifier_seq = mmu_interval_read_begin(&vma->userptr.notifier);
1035+
notifier_seq = mmu_interval_read_begin(&uvma->userptr.notifier);
10361036
} while (true);
10371037

10381038
/* Inject errors to test_whether they are handled correctly */
1039-
if (userptr_update->bind && xe_pt_userptr_inject_eagain(vma)) {
1039+
if (userptr_update->bind && xe_pt_userptr_inject_eagain(uvma)) {
10401040
up_read(&vm->userptr.notifier_lock);
10411041
return -EAGAIN;
10421042
}
@@ -1297,7 +1297,7 @@ __xe_pt_bind_vma(struct xe_tile *tile, struct xe_vma *vma, struct xe_exec_queue
12971297
vma->tile_present |= BIT(tile->id);
12981298

12991299
if (bind_pt_update.locked) {
1300-
vma->userptr.initial_bind = true;
1300+
to_userptr_vma(vma)->userptr.initial_bind = true;
13011301
up_read(&vm->userptr.notifier_lock);
13021302
xe_bo_put_commit(&deferred);
13031303
}
@@ -1642,7 +1642,7 @@ __xe_pt_unbind_vma(struct xe_tile *tile, struct xe_vma *vma, struct xe_exec_queu
16421642

16431643
if (!vma->tile_present) {
16441644
spin_lock(&vm->userptr.invalidated_lock);
1645-
list_del_init(&vma->userptr.invalidate_link);
1645+
list_del_init(&to_userptr_vma(vma)->userptr.invalidate_link);
16461646
spin_unlock(&vm->userptr.invalidated_lock);
16471647
}
16481648
up_read(&vm->userptr.notifier_lock);

0 commit comments

Comments
 (0)