[PATCH 14/27] drm/xe/eudebug: userptr vm access pread/pwrite

[PATCH 14/27] drm/xe/eudebug: userptr vm access pread/pwrite

[Mika Kuoppala]

Implement debugger vm access for userptrs.

When userptr bind is done, mmu notifier is added by core xe.
Later when debugger wants to access the target memory, this
notifier can be used as it carries the struct mm of target.

Implement userptr vm access, for debugger pread/pwrite
using notifier mm passed to access_remote_vm().

This is based on suggestions from Thomas, Joonas and Simona.

v2: need to add offset into vma (Dominik)
v3: use exported access_remote_vm (Thomas)

Cc: Matthew Brost <matthew.brost@intel.com>
Cc: Andrzej Hajda <andrzej.hajda@intel.com>
Cc: Thomas Hellström <thomas.hellstrom@linux.intel.com>
Cc: Dominik Grzegorzek <dominik.grzegorzek@intel.com>
Cc: Christian König <christian.koenig@amd.com>
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Cc: Simona Vetter <simona@ffwll.ch>
Signed-off-by: Mika Kuoppala <mika.kuoppala@linux.intel.com>
---
 drivers/gpu/drm/xe/xe_eudebug.c | 21 +++++++++++++++++++++
 1 file changed, 21 insertions(+)

diff --git a/drivers/gpu/drm/xe/xe_eudebug.c b/drivers/gpu/drm/xe/xe_eudebug.c
index 210d9eeab1a7..25f18aa5447b 100644
--- a/drivers/gpu/drm/xe/xe_eudebug.c
+++ b/drivers/gpu/drm/xe/xe_eudebug.c
@@ -3077,6 +3077,27 @@ static int xe_eudebug_vma_access(struct xe_vma *vma,
 
 		xe_bo_put(bo);
 
+		return ret;
+	} else if (xe_vma_is_userptr(vma)) {
+		struct xe_userptr *userptr = &to_userptr_vma(vma)->userptr;
+		struct xe_vm *vm = xe_vma_vm(vma);
+		struct mm_struct *mm = NULL;
+		int ret;
+
+		down_read(&vm->userptr.notifier_lock);
+		if (mmget_not_zero(userptr->notifier.mm))
+			mm = userptr->notifier.mm;
+		up_read(&vm->userptr.notifier_lock);
+
+		if (!mm)
+			return -EFAULT;
+
+		ret = access_remote_vm(mm,
+				       xe_vma_userptr(vma) + offset_in_vma,
+				       buf, bytes,
+				       write ? FOLL_WRITE : 0);
+		mmput(mm);
+
 		return ret;
 	}
 
-- 
2.43.0

Re: [PATCH 13/26] RFC drm/xe/eudebug: userptr vm pread/pwrite

[Thomas Hellström]

On Fri, 2024-12-20 at 13:31 +0200, Mika Kuoppala wrote:
> Implement debugger vm access for userptrs.
> 
> When bind is done, take ref to current task so that
> we know from which vm the address was bound. Then during
> debugger pread/pwrite we use this target task as
> parameter to access the debuggee vm with access_process_vm().
> 
> This is based on suggestions from Thomas, Joonas and Simona.
> 
> v2: need to add offset into vma (Dominik)
> 
> Cc: Matthew Brost <matthew.brost@intel.com>
> Cc: Andrzej Hajda <andrzej.hajda@intel.com>
> Cc: Thomas Hellström <thomas.hellstrom@linux.intel.com>
> Cc: Dominik Grzegorzek <dominik.grzegorzek@intel.com>
> Cc: Christian König <christian.koenig@amd.com>
> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> Cc: Simona Vetter <simona@ffwll.ch>
> Signed-off-by: Mika Kuoppala <mika.kuoppala@linux.intel.com>
> ---
>  drivers/gpu/drm/xe/xe_eudebug.c  | 13 +++++++++++++
>  drivers/gpu/drm/xe/xe_vm.c       |  4 ++++
>  drivers/gpu/drm/xe/xe_vm.h       | 28 +++++++++++++++++++++++++++-
>  drivers/gpu/drm/xe/xe_vm_types.h |  6 ++++++
>  4 files changed, 50 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/xe/xe_eudebug.c
> b/drivers/gpu/drm/xe/xe_eudebug.c
> index 9d87df75348b..8b29192ab110 100644
> --- a/drivers/gpu/drm/xe/xe_eudebug.c
> +++ b/drivers/gpu/drm/xe/xe_eudebug.c
> @@ -3074,6 +3074,19 @@ static int xe_eudebug_vma_access(struct xe_vma
> *vma, u64 offset_in_vma,

AFAICT all across the core mm code, unsigned long is used for mm
offsets, rather than u64, which we use for gpu- and physical offsets.


>  		xe_bo_put(bo);
>  
>  		return ret;
> +	} else if (xe_vma_is_userptr(vma)) {
> +		struct xe_userptr *userptr = &to_userptr_vma(vma)-
> >userptr;
> +
> +		/*
> +		 * XXX: access_remote_vm() would fit as userptr
> notifier has
> +		 * mm ref so we would not need to carry task ref at
> all.
> +		 * But access_remote_vm is not exported.
> access_process_vm()
> +		 * is exported so use it instead.
> +		 */

Could we add a follow-up patch that exports access_remote_vm() and
changes this code to use access_remote_vm() instead?



> +		return access_process_vm(userptr->eudebug.task,
> +					 xe_vma_userptr(vma) +
> offset_in_vma,
> +					 buf, bytes,
> +					 write ? FOLL_WRITE : 0);
>  	}
>  
>  	return -EINVAL;
> diff --git a/drivers/gpu/drm/xe/xe_vm.c b/drivers/gpu/drm/xe/xe_vm.c
> index 1cb21325d8dd..235ae2db5188 100644
> --- a/drivers/gpu/drm/xe/xe_vm.c
> +++ b/drivers/gpu/drm/xe/xe_vm.c
> @@ -999,6 +999,8 @@ static struct xe_vma *xe_vma_create(struct xe_vm
> *vm,
>  			}
>  
>  			userptr->notifier_seq = LONG_MAX;
> +
> +			xe_eudebug_track_userptr_task(userptr);
>  		}
>  
>  		xe_vm_get(vm);
> @@ -1023,6 +1025,8 @@ static void xe_vma_destroy_late(struct xe_vma
> *vma)
>  		if (userptr->sg)
>  			xe_hmm_userptr_free_sg(uvma);
>  
> +		xe_eudebug_untrack_userptr_task(userptr);
> +
>  		/*
>  		 * Since userptr pages are not pinned, we can't
> remove
>  		 * the notifer until we're sure the GPU is not
> accessing
> diff --git a/drivers/gpu/drm/xe/xe_vm.h b/drivers/gpu/drm/xe/xe_vm.h
> index 23adb7442881..4334cf2b0d9d 100644
> --- a/drivers/gpu/drm/xe/xe_vm.h
> +++ b/drivers/gpu/drm/xe/xe_vm.h
> @@ -274,9 +274,35 @@ static inline void vm_dbg(const struct
> drm_device *dev,
>  			  const char *format, ...)
>  { /* noop */ }
>  #endif
> -#endif
>  
>  struct xe_vm_snapshot *xe_vm_snapshot_capture(struct xe_vm *vm);
>  void xe_vm_snapshot_capture_delayed(struct xe_vm_snapshot *snap);
>  void xe_vm_snapshot_print(struct xe_vm_snapshot *snap, struct
> drm_printer *p);
>  void xe_vm_snapshot_free(struct xe_vm_snapshot *snap);
> +
> +#if IS_ENABLED(CONFIG_DRM_XE_EUDEBUG)
> +static inline void xe_eudebug_track_userptr_task(struct xe_userptr
> *userptr)
> +{
> +	/*
> +	 * We could use the mm which is on notifier. But
> +	 * the access_remote_vm() is not exported. Thus
> +	 * we get reference to task for access_process_vm()
> +	 */
> +	userptr->eudebug.task = get_task_struct(current);
> +}
> +
> +static inline void xe_eudebug_untrack_userptr_task(struct xe_userptr
> *userptr)
> +{
> +	put_task_struct(userptr->eudebug.task);
> +}
> +#else
> +static inline void xe_eudebug_track_userptr_task(struct xe_userptr
> *userptr)
> +{
> +}
> +
> +static inline void xe_eudebug_untrack_userptr_task(struct xe_userptr
> *userptr)
> +{
> +}
> +#endif /* CONFIG_DRM_XE_EUDEBUG */
> +
> +#endif
> diff --git a/drivers/gpu/drm/xe/xe_vm_types.h
> b/drivers/gpu/drm/xe/xe_vm_types.h
> index 557b047ebdd7..26176ccbcbbc 100644
> --- a/drivers/gpu/drm/xe/xe_vm_types.h
> +++ b/drivers/gpu/drm/xe/xe_vm_types.h
> @@ -68,6 +68,12 @@ struct xe_userptr {
>  #if IS_ENABLED(CONFIG_DRM_XE_USERPTR_INVAL_INJECT)
>  	u32 divisor;
>  #endif
> +
> +#if IS_ENABLED(CONFIG_DRM_XE_EUDEBUG)
> +	struct {
> +		struct task_struct *task;
> +	} eudebug;
> +#endif
>  };
>  
>  struct xe_vma {

Otherwise LGTM.
Thanks,
Thomas

[PATCH 14/27] drm/xe/eudebug: userptr vm access pread/pwrite

[Mika Kuoppala]

Implement debugger vm access for userptrs.

When userptr bind is done, mmu notifier is added by core xe.
Later when debugger wants to access the target memory, this
notifier can be used as it carries the struct mm of target.

Implement userptr vm access, for debugger pread/pwrite
using notifier mm passed to access_remote_vm().

This is based on suggestions from Thomas, Joonas and Simona.

v2: need to add offset into vma (Dominik)
v3: use exported access_remote_vm (Thomas)

Cc: Matthew Brost <matthew.brost@intel.com>
Cc: Andrzej Hajda <andrzej.hajda@intel.com>
Cc: Thomas Hellström <thomas.hellstrom@linux.intel.com>
Cc: Dominik Grzegorzek <dominik.grzegorzek@intel.com>
Cc: Christian König <christian.koenig@amd.com>
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Cc: Simona Vetter <simona@ffwll.ch>
Signed-off-by: Mika Kuoppala <mika.kuoppala@linux.intel.com>
---
 drivers/gpu/drm/xe/xe_eudebug.c | 21 +++++++++++++++++++++
 1 file changed, 21 insertions(+)

diff --git a/drivers/gpu/drm/xe/xe_eudebug.c b/drivers/gpu/drm/xe/xe_eudebug.c
index 210d9eeab1a7..25f18aa5447b 100644
--- a/drivers/gpu/drm/xe/xe_eudebug.c
+++ b/drivers/gpu/drm/xe/xe_eudebug.c
@@ -3077,6 +3077,27 @@ static int xe_eudebug_vma_access(struct xe_vma *vma,
 
 		xe_bo_put(bo);
 
+		return ret;
+	} else if (xe_vma_is_userptr(vma)) {
+		struct xe_userptr *userptr = &to_userptr_vma(vma)->userptr;
+		struct xe_vm *vm = xe_vma_vm(vma);
+		struct mm_struct *mm = NULL;
+		int ret;
+
+		down_read(&vm->userptr.notifier_lock);
+		if (mmget_not_zero(userptr->notifier.mm))
+			mm = userptr->notifier.mm;
+		up_read(&vm->userptr.notifier_lock);
+
+		if (!mm)
+			return -EFAULT;
+
+		ret = access_remote_vm(mm,
+				       xe_vma_userptr(vma) + offset_in_vma,
+				       buf, bytes,
+				       write ? FOLL_WRITE : 0);
+		mmput(mm);
+
 		return ret;
 	}
 
-- 
2.43.0