Re: [PATCH] drm/xe: always keep track of remap prev/next

[Matthew Brost <matthew.brost@intel.com>]

On Thu, Mar 12, 2026 at 01:46:49PM +0000, Matthew Auld wrote:
> During 3D workload, user is reporting hitting:
> 
> [  413.361679] WARNING: drivers/gpu/drm/xe/xe_vm.c:1217 at vm_bind_ioctl_ops_unwind+0x1e2/0x2e0 [xe], CPU#7: vkd3d_queue/9925
> [  413.361944] CPU: 7 UID: 1000 PID: 9925 Comm: vkd3d_queue Kdump: loaded Not tainted 7.0.0-070000rc3-generic #202603090038 PREEMPT(lazy)
> [  413.361949] RIP: 0010:vm_bind_ioctl_ops_unwind+0x1e2/0x2e0 [xe]
> [  413.362074] RSP: 0018:ffffd4c25c3df930 EFLAGS: 00010282
> [  413.362077] RAX: 0000000000000000 RBX: ffff8f3ee817ed10 RCX: 0000000000000000
> [  413.362078] RDX: 0000000000000000 RSI: 0000000000000000 RDI: 0000000000000000
> [  413.362079] RBP: ffffd4c25c3df980 R08: 0000000000000000 R09: 0000000000000000
> [  413.362081] R10: 0000000000000000 R11: 0000000000000000 R12: ffff8f41fbf99380
> [  413.362082] R13: ffff8f3ee817e968 R14: 00000000ffffffef R15: ffff8f43d00bd380
> [  413.362083] FS:  00000001040ff6c0(0000) GS:ffff8f4696d89000(0000) knlGS:00000000330b0000
> [  413.362085] CS:  0010 DS: 002b ES: 002b CR0: 0000000080050033
> [  413.362086] CR2: 00007ddfc4747000 CR3: 00000002e6262005 CR4: 0000000000f72ef0
> [  413.362088] PKRU: 55555554
> [  413.362089] Call Trace:
> [  413.362092]  <TASK>
> [  413.362096]  xe_vm_bind_ioctl+0xa9a/0xc60 [xe]
> 
> Which seems to hint that the vma we are re-inserting for the ops unwind
> is either invalid or overlapping with something already inserted in the
> vm. It shouldn't be invalid since this is a re-insertion, so must have
> worked before. Leaving the likely culprit as something already placed
> where we want to insert the vma.
> 
> Following from that, for the case where we do something like a rebind in
> the middle of a vma, and one or both mapped ends are already compatible,
> we skip doing the rebind of those vma and set next/prev to NULL.
> However, if we need to do an ops unwind later, we currently need to know
> the next/prev when removing the two end vma, before then re-inserting
> the original vma we tried to split. If we don't remove the prev/end vma,
> then the re-insertion could fail since the two end vma are overlapping
> the range we want to re-insert. This could explain the user bug, and
> does seem to fit.
> 
> With that keep prev/next intact, and rely on skip prev/next.
> 
> Link: https://gitlab.freedesktop.org/drm/xe/kernel/-/issues/7602
> Signed-off-by: Matthew Auld <matthew.auld@intel.com>
> Cc: Matthew Brost <matthew.brost@intel.com>

I'll need a bit to wrap my head around this - but quick question can we
recreate this bug in an IGT and get it added to our testing?

Matt

> ---
>  drivers/gpu/drm/xe/xe_pt.c | 12 ++++++------
>  drivers/gpu/drm/xe/xe_vm.c |  2 --
>  2 files changed, 6 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/gpu/drm/xe/xe_pt.c b/drivers/gpu/drm/xe/xe_pt.c
> index 13b355fadd58..a5bc553f8456 100644
> --- a/drivers/gpu/drm/xe/xe_pt.c
> +++ b/drivers/gpu/drm/xe/xe_pt.c
> @@ -1442,9 +1442,9 @@ static int op_check_svm_userptr(struct xe_vm *vm, struct xe_vma_op *op,
>  		err = vma_check_userptr(vm, op->map.vma, pt_update);
>  		break;
>  	case DRM_GPUVA_OP_REMAP:
> -		if (op->remap.prev)
> +		if (op->remap.prev && !op->remap.skip_prev)
>  			err = vma_check_userptr(vm, op->remap.prev, pt_update);
> -		if (!err && op->remap.next)
> +		if (!err && op->remap.next && !op->remap.skip_next)
>  			err = vma_check_userptr(vm, op->remap.next, pt_update);
>  		break;
>  	case DRM_GPUVA_OP_UNMAP:
> @@ -2178,12 +2178,12 @@ static int op_prepare(struct xe_vm *vm,
>  
>  		err = unbind_op_prepare(tile, pt_update_ops, old);
>  
> -		if (!err && op->remap.prev) {
> +		if (!err && op->remap.prev && !op->remap.skip_prev) {
>  			err = bind_op_prepare(vm, tile, pt_update_ops,
>  					      op->remap.prev, false);
>  			pt_update_ops->wait_vm_bookkeep = true;
>  		}
> -		if (!err && op->remap.next) {
> +		if (!err && op->remap.next && !op->remap.skip_next) {
>  			err = bind_op_prepare(vm, tile, pt_update_ops,
>  					      op->remap.next, false);
>  			pt_update_ops->wait_vm_bookkeep = true;
> @@ -2408,10 +2408,10 @@ static void op_commit(struct xe_vm *vm,
>  
>  		unbind_op_commit(vm, tile, pt_update_ops, old, fence, fence2);
>  
> -		if (op->remap.prev)
> +		if (op->remap.prev && !op->remap.skip_prev)
>  			bind_op_commit(vm, tile, pt_update_ops, op->remap.prev,
>  				       fence, fence2, false);
> -		if (op->remap.next)
> +		if (op->remap.next && !op->remap.skip_next)
>  			bind_op_commit(vm, tile, pt_update_ops, op->remap.next,
>  				       fence, fence2, false);
>  		break;
> diff --git a/drivers/gpu/drm/xe/xe_vm.c b/drivers/gpu/drm/xe/xe_vm.c
> index 5572e12c2a7e..9a897388ff5f 100644
> --- a/drivers/gpu/drm/xe/xe_vm.c
> +++ b/drivers/gpu/drm/xe/xe_vm.c
> @@ -2584,7 +2584,6 @@ static int xe_vma_op_commit(struct xe_vm *vm, struct xe_vma_op *op)
>  			if (!err && op->remap.skip_prev) {
>  				op->remap.prev->tile_present =
>  					tile_present;
> -				op->remap.prev = NULL;
>  			}
>  		}
>  		if (op->remap.next) {
> @@ -2594,7 +2593,6 @@ static int xe_vma_op_commit(struct xe_vm *vm, struct xe_vma_op *op)
>  			if (!err && op->remap.skip_next) {
>  				op->remap.next->tile_present =
>  					tile_present;
> -				op->remap.next = NULL;
>  			}
>  		}
>  
> -- 
> 2.53.0
>