Re: [PATCH 11/20] drm/xe/eudebug: Introduce vm bind and vm bind debug data events

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential
issue to consider:
- [Medium] drm/xe/eudebug: missed bind events in xe_vm_bind_ioctl() on
  prefetch -ENODATA
--

commit cd195b3b0e2ac13dc8ba630c0ab3fbbd171bde93
Author: Christoph Manszewski <c.manszewski@gmail.com>

drm/xe/eudebug: Introduce vm bind and vm bind debug data events

This commit adds eudebug VM bind and debug data operation events to notify
an attached debugger when VM bind debug data operations are executed or
discovered.

> diff --git a/drivers/gpu/drm/xe/xe_vm.c b/drivers/gpu/drm/xe/xe_vm.c
> index bfd5a76e953fb..0b6645c86e9a8 100644
> --- a/drivers/gpu/drm/xe/xe_vm.c
> +++ b/drivers/gpu/drm/xe/xe_vm.c
> @@ -3810,6 +3810,10 @@ static struct dma_fence *vm_bind_ioctl_ops_execute(struct xe_vm *vm,
>  		xe_vm_set_validation_exec(vm, &exec);
>  		fence = ops_execute(vm, vops);
>  		xe_vm_set_validation_exec(vm, NULL);
> +
> +		if (!IS_ERR(fence) || PTR_ERR(fence) == -ENODATA)
> +			xe_eudebug_vm_bind_execute(vm, vops);
> +
>  		if (IS_ERR(fence)) {
>  			if (PTR_ERR(fence) == -ENODATA)
>  				vm_bind_ioctl_ops_fini(vm, vops, NULL);

[Severity: Medium]
Can a multi-operation VM bind skip xe_eudebug_vm_bind_execute() while still
leaving debug data changes committed?

In xe_vm_bind_ioctl(), debug data add and remove operations are committed to
vm->debug_data.list during vm_bind_ioctl_process_user_extensions().  If the
same bind array also includes an SVM prefetch operation and
vm_bind_ioctl_ops_prefetch_ranges() returns -ENODATA (for example, when
prefetch_thread_func() hits a VRAM allocation failure or
xe_svm_range_get_pages() returns -EFAULT), xe_vm_bind_ioctl() jumps to
unwind_ops without calling vm_bind_ioctl_ops_execute():

drivers/gpu/drm/xe/xe_vm.c:xe_vm_bind_ioctl() {
    ...
	err = vm_bind_ioctl_ops_prefetch_ranges(vm, &vops);
	if (err)
		goto unwind_ops;

	fence = vm_bind_ioctl_ops_execute(vm, &vops);
    ...
unwind_ops:
	if (err && err != -ENODATA)
		vm_bind_ioctl_ops_unwind(vm, ops, args->num_binds);
	xe_vma_ops_fini(&vops);
    ...
free_syncs:
	if (err == -ENODATA)
		err = vm_bind_ioctl_signal_fences(vm, q, syncs, num_syncs);
    ...
}

Because -ENODATA skips vm_bind_ioctl_ops_unwind(), signals the out-fences,
and returns 0 to userspace, the debug data changes stay on
vm->debug_data.list without delivering the bind and debug data events to the
debugger.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261109135549.4128280-1-mika.kuoppala@linux.intel.com?part=11