* [PATCH v3 1/6] drm/xe/xe_gt_pagefault: Disallow writes to read-only VMAs
2025-02-28 18:21 [PATCH v3 0/6] drm/xe/xe_vm: Implement xe_vm_get_property_ioctl Jonathan Cavitt
@ 2025-02-28 18:21 ` Jonathan Cavitt
2025-02-28 18:21 ` [PATCH v3 2/6] drm/xe/xe_gt_pagefault: Migrate pagefault struct to header Jonathan Cavitt
` (4 subsequent siblings)
5 siblings, 0 replies; 10+ messages in thread
From: Jonathan Cavitt @ 2025-02-28 18:21 UTC (permalink / raw)
To: intel-xe
Cc: saurabhg.gupta, alex.zuo, jonathan.cavitt, joonas.lahtinen,
matthew.brost, jianxun.zhang, dri-devel
The page fault handler should reject write/atomic access to read only
VMAs. Add code to handle this in handle_pagefault after the VMA lookup.
Fixes: 3d420e9fa848 ("drm/xe: Rework GPU page fault handling")
Signed-off-by: Jonathan Cavitt <jonathan.cavitt@intel.com>
Suggested-by: Matthew Brost <matthew.brost@intel.com>
---
drivers/gpu/drm/xe/xe_gt_pagefault.c | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/drivers/gpu/drm/xe/xe_gt_pagefault.c b/drivers/gpu/drm/xe/xe_gt_pagefault.c
index 17d69039b866..f608a765fa7c 100644
--- a/drivers/gpu/drm/xe/xe_gt_pagefault.c
+++ b/drivers/gpu/drm/xe/xe_gt_pagefault.c
@@ -235,6 +235,11 @@ static int handle_pagefault(struct xe_gt *gt, struct pagefault *pf)
goto unlock_vm;
}
+ if (xe_vma_read_only(vma) && pf->access_type != ACCESS_TYPE_READ) {
+ err = -EPERM;
+ goto unlock_vm;
+ }
+
err = handle_vma_pagefault(gt, pf, vma);
unlock_vm:
--
2.43.0
^ permalink raw reply related [flat|nested] 10+ messages in thread* [PATCH v3 2/6] drm/xe/xe_gt_pagefault: Migrate pagefault struct to header
2025-02-28 18:21 [PATCH v3 0/6] drm/xe/xe_vm: Implement xe_vm_get_property_ioctl Jonathan Cavitt
2025-02-28 18:21 ` [PATCH v3 1/6] drm/xe/xe_gt_pagefault: Disallow writes to read-only VMAs Jonathan Cavitt
@ 2025-02-28 18:21 ` Jonathan Cavitt
2025-02-28 18:21 ` [PATCH v3 3/6] drm/xe/xe_vm: Add per VM pagefault info Jonathan Cavitt
` (3 subsequent siblings)
5 siblings, 0 replies; 10+ messages in thread
From: Jonathan Cavitt @ 2025-02-28 18:21 UTC (permalink / raw)
To: intel-xe
Cc: saurabhg.gupta, alex.zuo, jonathan.cavitt, joonas.lahtinen,
matthew.brost, jianxun.zhang, dri-devel
Migrate the pagefault struct from xe_gt_pagefault.c to the
xe_gt_pagefault.h header file, along with the associated enum values.
v2: Normalize names for common header (Matt Brost)
Signed-off-by: Jonathan Cavitt <jonathan.cavitt@intel.com>
---
drivers/gpu/drm/xe/xe_gt_pagefault.c | 43 ++++++----------------------
drivers/gpu/drm/xe/xe_gt_pagefault.h | 28 ++++++++++++++++++
2 files changed, 36 insertions(+), 35 deletions(-)
diff --git a/drivers/gpu/drm/xe/xe_gt_pagefault.c b/drivers/gpu/drm/xe/xe_gt_pagefault.c
index f608a765fa7c..07b52d3c1a60 100644
--- a/drivers/gpu/drm/xe/xe_gt_pagefault.c
+++ b/drivers/gpu/drm/xe/xe_gt_pagefault.c
@@ -22,33 +22,6 @@
#include "xe_trace_bo.h"
#include "xe_vm.h"
-struct pagefault {
- u64 page_addr;
- u32 asid;
- u16 pdata;
- u8 vfid;
- u8 access_type;
- u8 fault_type;
- u8 fault_level;
- u8 engine_class;
- u8 engine_instance;
- u8 fault_unsuccessful;
- bool trva_fault;
-};
-
-enum access_type {
- ACCESS_TYPE_READ = 0,
- ACCESS_TYPE_WRITE = 1,
- ACCESS_TYPE_ATOMIC = 2,
- ACCESS_TYPE_RESERVED = 3,
-};
-
-enum fault_type {
- NOT_PRESENT = 0,
- WRITE_ACCESS_VIOLATION = 1,
- ATOMIC_ACCESS_VIOLATION = 2,
-};
-
struct acc {
u64 va_range_base;
u32 asid;
@@ -60,9 +33,9 @@ struct acc {
u8 engine_instance;
};
-static bool access_is_atomic(enum access_type access_type)
+static bool access_is_atomic(enum xe_pagefault_access_type access_type)
{
- return access_type == ACCESS_TYPE_ATOMIC;
+ return access_type == XE_PAGEFAULT_ACCESS_TYPE_ATOMIC;
}
static bool vma_is_valid(struct xe_tile *tile, struct xe_vma *vma)
@@ -125,7 +98,7 @@ static int xe_pf_begin(struct drm_exec *exec, struct xe_vma *vma,
return 0;
}
-static int handle_vma_pagefault(struct xe_gt *gt, struct pagefault *pf,
+static int handle_vma_pagefault(struct xe_gt *gt, struct xe_pagefault *pf,
struct xe_vma *vma)
{
struct xe_vm *vm = xe_vma_vm(vma);
@@ -204,7 +177,7 @@ static struct xe_vm *asid_to_vm(struct xe_device *xe, u32 asid)
return vm;
}
-static int handle_pagefault(struct xe_gt *gt, struct pagefault *pf)
+static int handle_pagefault(struct xe_gt *gt, struct xe_pagefault *pf)
{
struct xe_device *xe = gt_to_xe(gt);
struct xe_vm *vm;
@@ -235,7 +208,7 @@ static int handle_pagefault(struct xe_gt *gt, struct pagefault *pf)
goto unlock_vm;
}
- if (xe_vma_read_only(vma) && pf->access_type != ACCESS_TYPE_READ) {
+ if (xe_vma_read_only(vma) && pf->access_type != XE_PAGEFAULT_ACCESS_TYPE_READ) {
err = -EPERM;
goto unlock_vm;
}
@@ -263,7 +236,7 @@ static int send_pagefault_reply(struct xe_guc *guc,
return xe_guc_ct_send(&guc->ct, action, ARRAY_SIZE(action), 0, 0);
}
-static void print_pagefault(struct xe_device *xe, struct pagefault *pf)
+static void print_pagefault(struct xe_device *xe, struct xe_pagefault *pf)
{
drm_dbg(&xe->drm, "\n\tASID: %d\n"
"\tVFID: %d\n"
@@ -283,7 +256,7 @@ static void print_pagefault(struct xe_device *xe, struct pagefault *pf)
#define PF_MSG_LEN_DW 4
-static bool get_pagefault(struct pf_queue *pf_queue, struct pagefault *pf)
+static bool get_pagefault(struct pf_queue *pf_queue, struct xe_pagefault *pf)
{
const struct xe_guc_pagefault_desc *desc;
bool ret = false;
@@ -370,7 +343,7 @@ static void pf_queue_work_func(struct work_struct *w)
struct xe_gt *gt = pf_queue->gt;
struct xe_device *xe = gt_to_xe(gt);
struct xe_guc_pagefault_reply reply = {};
- struct pagefault pf = {};
+ struct xe_pagefault pf = {};
unsigned long threshold;
int ret;
diff --git a/drivers/gpu/drm/xe/xe_gt_pagefault.h b/drivers/gpu/drm/xe/xe_gt_pagefault.h
index 839c065a5e4c..33616043d17a 100644
--- a/drivers/gpu/drm/xe/xe_gt_pagefault.h
+++ b/drivers/gpu/drm/xe/xe_gt_pagefault.h
@@ -11,6 +11,34 @@
struct xe_gt;
struct xe_guc;
+struct xe_pagefault {
+ u64 page_addr;
+ u32 asid;
+ u16 pdata;
+ u8 vfid;
+ u8 access_type;
+ u8 fault_type;
+ u8 fault_level;
+ u8 engine_class;
+ u8 engine_instance;
+ u8 fault_unsuccessful;
+ bool prefetch;
+ bool trva_fault;
+};
+
+enum xe_pagefault_access_type {
+ XE_PAGEFAULT_ACCESS_TYPE_READ = 0,
+ XE_PAGEFAULT_ACCESS_TYPE_WRITE = 1,
+ XE_PAGEFAULT_ACCESS_TYPE_ATOMIC = 2,
+ XE_PAGEFAULT_ACCESS_TYPE_RESERVED = 3,
+};
+
+enum xe_pagefault_type {
+ XE_PAGEFAULT_TYPE_NOT_PRESENT = 0,
+ XE_PAGEFAULT_TYPE_WRITE_ACCESS_VIOLATION = 1,
+ XE_PAGEFAULT_TYPE_ATOMIC_ACCESS_VIOLATION = 2,
+};
+
int xe_gt_pagefault_init(struct xe_gt *gt);
void xe_gt_pagefault_reset(struct xe_gt *gt);
int xe_guc_pagefault_handler(struct xe_guc *guc, u32 *msg, u32 len);
--
2.43.0
^ permalink raw reply related [flat|nested] 10+ messages in thread* [PATCH v3 3/6] drm/xe/xe_vm: Add per VM pagefault info
2025-02-28 18:21 [PATCH v3 0/6] drm/xe/xe_vm: Implement xe_vm_get_property_ioctl Jonathan Cavitt
2025-02-28 18:21 ` [PATCH v3 1/6] drm/xe/xe_gt_pagefault: Disallow writes to read-only VMAs Jonathan Cavitt
2025-02-28 18:21 ` [PATCH v3 2/6] drm/xe/xe_gt_pagefault: Migrate pagefault struct to header Jonathan Cavitt
@ 2025-02-28 18:21 ` Jonathan Cavitt
2025-03-05 5:45 ` Zhang, Jianxun
2025-02-28 18:21 ` [PATCH v3 4/6] drm/xe/uapi: Define drm_xe_vm_get_property Jonathan Cavitt
` (2 subsequent siblings)
5 siblings, 1 reply; 10+ messages in thread
From: Jonathan Cavitt @ 2025-02-28 18:21 UTC (permalink / raw)
To: intel-xe
Cc: saurabhg.gupta, alex.zuo, jonathan.cavitt, joonas.lahtinen,
matthew.brost, jianxun.zhang, dri-devel
Add additional information to each VM so they can report up to the last
50 seen pagefaults. Only failed pagefaults are saved this way, as
successful pagefaults should recover and not need to be reported to
userspace.
Signed-off-by: Jonathan Cavitt <jonathan.cavitt@intel.com>
Suggested-by: Matthew Brost <matthew.brost@intel.com>
---
drivers/gpu/drm/xe/xe_gt_pagefault.c | 17 +++++++++++
drivers/gpu/drm/xe/xe_vm.c | 45 ++++++++++++++++++++++++++++
drivers/gpu/drm/xe/xe_vm.h | 6 ++++
drivers/gpu/drm/xe/xe_vm_types.h | 20 +++++++++++++
4 files changed, 88 insertions(+)
diff --git a/drivers/gpu/drm/xe/xe_gt_pagefault.c b/drivers/gpu/drm/xe/xe_gt_pagefault.c
index 07b52d3c1a60..84907fb4295e 100644
--- a/drivers/gpu/drm/xe/xe_gt_pagefault.c
+++ b/drivers/gpu/drm/xe/xe_gt_pagefault.c
@@ -335,6 +335,22 @@ int xe_guc_pagefault_handler(struct xe_guc *guc, u32 *msg, u32 len)
return full ? -ENOSPC : 0;
}
+static void save_pagefault_to_vm(struct xe_device *xe, struct xe_pagefault *pf)
+{
+ struct xe_vm *vm;
+ struct xe_pagefault *store;
+
+ vm = asid_to_vm(xe, pf->asid);
+ if (IS_ERR(vm))
+ return;
+
+ spin_lock(&vm->pfs.lock);
+ store = kzalloc(sizeof(*pf), GFP_KERNEL);
+ memcpy(store, pf, sizeof(*pf));
+ xe_vm_add_pf_entry(vm, store);
+ spin_unlock(&vm->pfs.lock);
+}
+
#define USM_QUEUE_MAX_RUNTIME_MS 20
static void pf_queue_work_func(struct work_struct *w)
@@ -353,6 +369,7 @@ static void pf_queue_work_func(struct work_struct *w)
ret = handle_pagefault(gt, &pf);
if (unlikely(ret)) {
print_pagefault(xe, &pf);
+ save_pagefault_to_vm(xe, &pf);
pf.fault_unsuccessful = 1;
drm_dbg(&xe->drm, "Fault response: Unsuccessful %d\n", ret);
}
diff --git a/drivers/gpu/drm/xe/xe_vm.c b/drivers/gpu/drm/xe/xe_vm.c
index 996000f2424e..6211b971bbbd 100644
--- a/drivers/gpu/drm/xe/xe_vm.c
+++ b/drivers/gpu/drm/xe/xe_vm.c
@@ -746,6 +746,46 @@ int xe_vm_userptr_check_repin(struct xe_vm *vm)
list_empty_careful(&vm->userptr.invalidated)) ? 0 : -EAGAIN;
}
+static void free_pf_entry(struct xe_vm *vm, struct xe_vm_pf_entry *e)
+{
+ list_del(&e->list);
+ kfree(e->pf);
+ kfree(e);
+ vm->pfs.len--;
+}
+
+void xe_vm_add_pf_entry(struct xe_vm *vm, struct xe_pagefault *pf)
+{
+ struct xe_vm_pf_entry *e = NULL;
+
+ e = kzalloc(sizeof(*e), GFP_KERNEL);
+ xe_assert(vm->xe, e);
+
+ spin_lock(&vm->pfs.lock);
+ list_add_tail(&e->list, &vm->pfs.list);
+ vm->pfs.len++;
+ /**
+ * Limit the number of pfs in the pf list to prevent memory overuse.
+ */
+ if (vm->pfs.len > MAX_PFS) {
+ struct xe_vm_pf_entry *rem =
+ list_first_entry(&vm->pfs.list, struct xe_vm_pf_entry, list);
+
+ free_pf_entry(vm, rem);
+ }
+ spin_unlock(&vm->pfs.lock);
+}
+
+void xe_vm_remove_pf_entries(struct xe_vm *vm)
+{
+ struct xe_vm_pf_entry *e, *tmp;
+
+ spin_lock(&vm->pfs.lock);
+ list_for_each_entry_safe(e, tmp, &vm->pfs.list, list)
+ free_pf_entry(vm, e);
+ spin_unlock(&vm->pfs.lock);
+}
+
static int xe_vma_ops_alloc(struct xe_vma_ops *vops, bool array_of_binds)
{
int i;
@@ -1448,6 +1488,9 @@ struct xe_vm *xe_vm_create(struct xe_device *xe, u32 flags)
init_rwsem(&vm->userptr.notifier_lock);
spin_lock_init(&vm->userptr.invalidated_lock);
+ INIT_LIST_HEAD(&vm->pfs.list);
+ spin_lock_init(&vm->pfs.lock);
+
ttm_lru_bulk_move_init(&vm->lru_bulk_move);
INIT_WORK(&vm->destroy_work, vm_destroy_work_func);
@@ -1672,6 +1715,8 @@ void xe_vm_close_and_put(struct xe_vm *vm)
}
up_write(&xe->usm.lock);
+ xe_vm_remove_pf_entries(vm);
+
for_each_tile(tile, xe, id)
xe_range_fence_tree_fini(&vm->rftree[id]);
diff --git a/drivers/gpu/drm/xe/xe_vm.h b/drivers/gpu/drm/xe/xe_vm.h
index f66075f8a6fe..4d94ab5c8ea4 100644
--- a/drivers/gpu/drm/xe/xe_vm.h
+++ b/drivers/gpu/drm/xe/xe_vm.h
@@ -12,6 +12,8 @@
#include "xe_map.h"
#include "xe_vm_types.h"
+#define MAX_PFS 50
+
struct drm_device;
struct drm_printer;
struct drm_file;
@@ -244,6 +246,10 @@ int xe_vma_userptr_pin_pages(struct xe_userptr_vma *uvma);
int xe_vma_userptr_check_repin(struct xe_userptr_vma *uvma);
+void xe_vm_add_pf_entry(struct xe_vm *vm, struct xe_pagefault *pf);
+
+void xe_vm_remove_pf_entries(struct xe_vm *vm);
+
bool xe_vm_validate_should_retry(struct drm_exec *exec, int err, ktime_t *end);
int xe_vm_lock_vma(struct drm_exec *exec, struct xe_vma *vma);
diff --git a/drivers/gpu/drm/xe/xe_vm_types.h b/drivers/gpu/drm/xe/xe_vm_types.h
index 52467b9b5348..10b0952db56c 100644
--- a/drivers/gpu/drm/xe/xe_vm_types.h
+++ b/drivers/gpu/drm/xe/xe_vm_types.h
@@ -18,6 +18,7 @@
#include "xe_range_fence.h"
struct xe_bo;
+struct xe_pagefault;
struct xe_sync_entry;
struct xe_user_fence;
struct xe_vm;
@@ -135,6 +136,13 @@ struct xe_userptr_vma {
struct xe_device;
+struct xe_vm_pf_entry {
+ /** @pf: observed pagefault */
+ struct xe_pagefault *pf;
+ /** @list: link into @xe_vm.pfs.list */
+ struct list_head list;
+};
+
struct xe_vm {
/** @gpuvm: base GPUVM used to track VMAs */
struct drm_gpuvm gpuvm;
@@ -274,6 +282,18 @@ struct xe_vm {
bool capture_once;
} error_capture;
+ /**
+ * @pfs: List of all pagefaults associated with this VM
+ */
+ struct {
+ /** @lock: lock protecting @bans.list */
+ spinlock_t lock;
+ /** @list: list of xe_exec_queue_ban_entry entries */
+ struct list_head list;
+ /** @len: length of @bans.list */
+ unsigned int len;
+ } pfs;
+
/**
* @tlb_flush_seqno: Required TLB flush seqno for the next exec.
* protected by the vm resv.
--
2.43.0
^ permalink raw reply related [flat|nested] 10+ messages in thread* Re: [PATCH v3 3/6] drm/xe/xe_vm: Add per VM pagefault info
2025-02-28 18:21 ` [PATCH v3 3/6] drm/xe/xe_vm: Add per VM pagefault info Jonathan Cavitt
@ 2025-03-05 5:45 ` Zhang, Jianxun
0 siblings, 0 replies; 10+ messages in thread
From: Zhang, Jianxun @ 2025-03-05 5:45 UTC (permalink / raw)
To: Cavitt, Jonathan, intel-xe@lists.freedesktop.org
Cc: Gupta, saurabhg, Zuo, Alex, joonas.lahtinen@linux.intel.com,
Brost, Matthew, dri-devel@lists.freedesktop.org
[-- Attachment #1: Type: text/plain, Size: 7202 bytes --]
________________________________
From: Cavitt, Jonathan <jonathan.cavitt@intel.com>
Sent: Friday, February 28, 2025 10:21 AM
To: intel-xe@lists.freedesktop.org <intel-xe@lists.freedesktop.org>
Cc: Gupta, saurabhg <saurabhg.gupta@intel.com>; Zuo, Alex <alex.zuo@intel.com>; Cavitt, Jonathan <jonathan.cavitt@intel.com>; joonas.lahtinen@linux.intel.com <joonas.lahtinen@linux.intel.com>; Brost, Matthew <matthew.brost@intel.com>; Zhang, Jianxun <jianxun.zhang@intel.com>; dri-devel@lists.freedesktop.org <dri-devel@lists.freedesktop.org>
Subject: [PATCH v3 3/6] drm/xe/xe_vm: Add per VM pagefault info
Add additional information to each VM so they can report up to the last
50 seen pagefaults. Only failed pagefaults are saved this way, as
successful pagefaults should recover and not need to be reported to
userspace.
Signed-off-by: Jonathan Cavitt <jonathan.cavitt@intel.com>
Suggested-by: Matthew Brost <matthew.brost@intel.com>
---
drivers/gpu/drm/xe/xe_gt_pagefault.c | 17 +++++++++++
drivers/gpu/drm/xe/xe_vm.c | 45 ++++++++++++++++++++++++++++
drivers/gpu/drm/xe/xe_vm.h | 6 ++++
drivers/gpu/drm/xe/xe_vm_types.h | 20 +++++++++++++
4 files changed, 88 insertions(+)
diff --git a/drivers/gpu/drm/xe/xe_gt_pagefault.c b/drivers/gpu/drm/xe/xe_gt_pagefault.c
index 07b52d3c1a60..84907fb4295e 100644
--- a/drivers/gpu/drm/xe/xe_gt_pagefault.c
+++ b/drivers/gpu/drm/xe/xe_gt_pagefault.c
@@ -335,6 +335,22 @@ int xe_guc_pagefault_handler(struct xe_guc *guc, u32 *msg, u32 len)
return full ? -ENOSPC : 0;
}
+static void save_pagefault_to_vm(struct xe_device *xe, struct xe_pagefault *pf)
+{
+ struct xe_vm *vm;
+ struct xe_pagefault *store;
+
+ vm = asid_to_vm(xe, pf->asid);
+ if (IS_ERR(vm))
+ return;
+
+ spin_lock(&vm->pfs.lock);
+ store = kzalloc(sizeof(*pf), GFP_KERNEL);
+ memcpy(store, pf, sizeof(*pf));
+ xe_vm_add_pf_entry(vm, store);
+ spin_unlock(&vm->pfs.lock);
+}
+
#define USM_QUEUE_MAX_RUNTIME_MS 20
static void pf_queue_work_func(struct work_struct *w)
@@ -353,6 +369,7 @@ static void pf_queue_work_func(struct work_struct *w)
ret = handle_pagefault(gt, &pf);
if (unlikely(ret)) {
print_pagefault(xe, &pf);
+ save_pagefault_to_vm(xe, &pf);
pf.fault_unsuccessful = 1;
drm_dbg(&xe->drm, "Fault response: Unsuccessful %d\n", ret);
}
diff --git a/drivers/gpu/drm/xe/xe_vm.c b/drivers/gpu/drm/xe/xe_vm.c
index 996000f2424e..6211b971bbbd 100644
--- a/drivers/gpu/drm/xe/xe_vm.c
+++ b/drivers/gpu/drm/xe/xe_vm.c
@@ -746,6 +746,46 @@ int xe_vm_userptr_check_repin(struct xe_vm *vm)
list_empty_careful(&vm->userptr.invalidated)) ? 0 : -EAGAIN;
}
+static void free_pf_entry(struct xe_vm *vm, struct xe_vm_pf_entry *e)
+{
+ list_del(&e->list);
+ kfree(e->pf);
+ kfree(e);
+ vm->pfs.len--;
+}
+
+void xe_vm_add_pf_entry(struct xe_vm *vm, struct xe_pagefault *pf)
+{
+ struct xe_vm_pf_entry *e = NULL;
+
+ e = kzalloc(sizeof(*e), GFP_KERNEL);
+ xe_assert(vm->xe, e);
+
+ spin_lock(&vm->pfs.lock);
+ list_add_tail(&e->list, &vm->pfs.list);
+ vm->pfs.len++;
+ /**
+ * Limit the number of pfs in the pf list to prevent memory overuse.
+ */
+ if (vm->pfs.len > MAX_PFS) {
+ struct xe_vm_pf_entry *rem =
+ list_first_entry(&vm->pfs.list, struct xe_vm_pf_entry, list);
+
I think the first page fault could be more valuable than the following in actual debug work though I cannot provide a concrete case. Maybe we should just stop adding new page faults once the list is full? 50 faults perphaps is enough for a developer to work out...
+ free_pf_entry(vm, rem);
+ }
+ spin_unlock(&vm->pfs.lock);
+}
+
+void xe_vm_remove_pf_entries(struct xe_vm *vm)
+{
+ struct xe_vm_pf_entry *e, *tmp;
+
+ spin_lock(&vm->pfs.lock);
+ list_for_each_entry_safe(e, tmp, &vm->pfs.list, list)
+ free_pf_entry(vm, e);
+ spin_unlock(&vm->pfs.lock);
+}
+
static int xe_vma_ops_alloc(struct xe_vma_ops *vops, bool array_of_binds)
{
int i;
@@ -1448,6 +1488,9 @@ struct xe_vm *xe_vm_create(struct xe_device *xe, u32 flags)
init_rwsem(&vm->userptr.notifier_lock);
spin_lock_init(&vm->userptr.invalidated_lock);
+ INIT_LIST_HEAD(&vm->pfs.list);
+ spin_lock_init(&vm->pfs.lock);
+
ttm_lru_bulk_move_init(&vm->lru_bulk_move);
INIT_WORK(&vm->destroy_work, vm_destroy_work_func);
@@ -1672,6 +1715,8 @@ void xe_vm_close_and_put(struct xe_vm *vm)
}
up_write(&xe->usm.lock);
+ xe_vm_remove_pf_entries(vm);
+
for_each_tile(tile, xe, id)
xe_range_fence_tree_fini(&vm->rftree[id]);
diff --git a/drivers/gpu/drm/xe/xe_vm.h b/drivers/gpu/drm/xe/xe_vm.h
index f66075f8a6fe..4d94ab5c8ea4 100644
--- a/drivers/gpu/drm/xe/xe_vm.h
+++ b/drivers/gpu/drm/xe/xe_vm.h
@@ -12,6 +12,8 @@
#include "xe_map.h"
#include "xe_vm_types.h"
+#define MAX_PFS 50
+
struct drm_device;
struct drm_printer;
struct drm_file;
@@ -244,6 +246,10 @@ int xe_vma_userptr_pin_pages(struct xe_userptr_vma *uvma);
int xe_vma_userptr_check_repin(struct xe_userptr_vma *uvma);
+void xe_vm_add_pf_entry(struct xe_vm *vm, struct xe_pagefault *pf);
+
+void xe_vm_remove_pf_entries(struct xe_vm *vm);
+
bool xe_vm_validate_should_retry(struct drm_exec *exec, int err, ktime_t *end);
int xe_vm_lock_vma(struct drm_exec *exec, struct xe_vma *vma);
diff --git a/drivers/gpu/drm/xe/xe_vm_types.h b/drivers/gpu/drm/xe/xe_vm_types.h
index 52467b9b5348..10b0952db56c 100644
--- a/drivers/gpu/drm/xe/xe_vm_types.h
+++ b/drivers/gpu/drm/xe/xe_vm_types.h
@@ -18,6 +18,7 @@
#include "xe_range_fence.h"
struct xe_bo;
+struct xe_pagefault;
struct xe_sync_entry;
struct xe_user_fence;
struct xe_vm;
@@ -135,6 +136,13 @@ struct xe_userptr_vma {
struct xe_device;
+struct xe_vm_pf_entry {
+ /** @pf: observed pagefault */
+ struct xe_pagefault *pf;
+ /** @list: link into @xe_vm.pfs.list */
+ struct list_head list;
+};
+
struct xe_vm {
/** @gpuvm: base GPUVM used to track VMAs */
struct drm_gpuvm gpuvm;
@@ -274,6 +282,18 @@ struct xe_vm {
bool capture_once;
} error_capture;
+ /**
+ * @pfs: List of all pagefaults associated with this VM
+ */
+ struct {
+ /** @lock: lock protecting @bans.list */
+ spinlock_t lock;
+ /** @list: list of xe_exec_queue_ban_entry entries */
+ struct list_head list;
+ /** @len: length of @bans.list */
+ unsigned int len;
+ } pfs;
+
/**
* @tlb_flush_seqno: Required TLB flush seqno for the next exec.
* protected by the vm resv.
--
2.43.0
[-- Attachment #2: Type: text/html, Size: 13354 bytes --]
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH v3 4/6] drm/xe/uapi: Define drm_xe_vm_get_property
2025-02-28 18:21 [PATCH v3 0/6] drm/xe/xe_vm: Implement xe_vm_get_property_ioctl Jonathan Cavitt
` (2 preceding siblings ...)
2025-02-28 18:21 ` [PATCH v3 3/6] drm/xe/xe_vm: Add per VM pagefault info Jonathan Cavitt
@ 2025-02-28 18:21 ` Jonathan Cavitt
2025-02-28 18:21 ` [PATCH v3 5/6] drm/xe/xe_gt_pagefault: Add address_type field to pagefaults Jonathan Cavitt
2025-02-28 18:21 ` [PATCH v3 6/6] drm/xe/xe_vm: Implement xe_vm_get_property_ioctl Jonathan Cavitt
5 siblings, 0 replies; 10+ messages in thread
From: Jonathan Cavitt @ 2025-02-28 18:21 UTC (permalink / raw)
To: intel-xe
Cc: saurabhg.gupta, alex.zuo, jonathan.cavitt, joonas.lahtinen,
matthew.brost, jianxun.zhang, dri-devel
Add initial declarations for the drm_xe_vm_get_property ioctl.
Signed-off-by: Jonathan Cavitt <jonathan.cavitt@intel.com>
---
include/uapi/drm/xe_drm.h | 63 +++++++++++++++++++++++++++++++++++++++
1 file changed, 63 insertions(+)
diff --git a/include/uapi/drm/xe_drm.h b/include/uapi/drm/xe_drm.h
index 76a462fae05f..38f29600e127 100644
--- a/include/uapi/drm/xe_drm.h
+++ b/include/uapi/drm/xe_drm.h
@@ -81,6 +81,7 @@ extern "C" {
* - &DRM_IOCTL_XE_EXEC
* - &DRM_IOCTL_XE_WAIT_USER_FENCE
* - &DRM_IOCTL_XE_OBSERVATION
+ * - %DRM_IOCTL_XE_VM_GET_PROPERTY
*/
/*
@@ -102,6 +103,7 @@ extern "C" {
#define DRM_XE_EXEC 0x09
#define DRM_XE_WAIT_USER_FENCE 0x0a
#define DRM_XE_OBSERVATION 0x0b
+#define DRM_XE_VM_GET_PROPERTY 0x0c
/* Must be kept compact -- no holes */
@@ -117,6 +119,7 @@ extern "C" {
#define DRM_IOCTL_XE_EXEC DRM_IOW(DRM_COMMAND_BASE + DRM_XE_EXEC, struct drm_xe_exec)
#define DRM_IOCTL_XE_WAIT_USER_FENCE DRM_IOWR(DRM_COMMAND_BASE + DRM_XE_WAIT_USER_FENCE, struct drm_xe_wait_user_fence)
#define DRM_IOCTL_XE_OBSERVATION DRM_IOW(DRM_COMMAND_BASE + DRM_XE_OBSERVATION, struct drm_xe_observation_param)
+#define DRM_IOCTL_XE_VM_GET_PROPERTY DRM_IOWR(DRM_COMMAND_BASE + DRM_XE_VM_GET_PROPERTY, struct drm_xe_vm_get_property)
/**
* DOC: Xe IOCTL Extensions
@@ -1166,6 +1169,66 @@ struct drm_xe_vm_bind {
__u64 reserved[2];
};
+struct drm_xe_pf {
+ /** @address: Address of the fault, if relevant */
+ __u64 address;
+#define DRM_XE_FAULT_ADDRESS_TYPE_NONE_EXT 0
+#define DRM_XE_FAULT_ADDRESS_TYPE_READ_INVALID_EXT 1
+#define DRM_XE_FAULT_ADDRESS_TYPE_WRITE_INVALID_EXT 2
+ /** @address_type: , if relevant */
+ __u32 address_type;
+ /**
+ * @address_precision: Precision of faulted address, if relevant.
+ * Currently only SZ_4K.
+ */
+ __u32 address_precision;
+ /** @reserved: MBZ */
+ __u64 reserved[3];
+};
+
+/**
+ * struct drm_xe_vm_get_property - Input of &DRM_IOCTL_XE_VM_GET_PROPERTY
+ *
+ * The user provides a VM ID and a property to query to this ioctl,
+ * and the ioctl returns the size of the return value. Calling the
+ * ioctl again with memory reserved in @data will save the
+ * requested property data to the pointer saved at @data.
+ *
+ * In the future, some properties may simply be scalar values. In
+ * such cases, the size field will remain zero, and the value of the
+ * scalar property will be saved to @value.
+ *
+ * The valid properties are:
+ * - %DRM_XE_VM_GET_PROPERTY_FAULTS : List of all failed pagefaults seen by VM
+ */
+struct drm_xe_vm_get_property {
+ /** @extensions: Pointer to the first extension struct, if any */
+ __u64 extensions;
+
+ /** @vm_id: The ID of the VM to query the properties of */
+ __u32 vm_id;
+
+#define DRM_XE_VM_GET_PROPERTY_FAULTS 0
+ /** @property: The property to get */
+ __u32 property;
+
+ /** @size: Size of returned property @data */
+ __u32 size;
+
+ /** @pad: MBZ */
+ __u32 pad;
+
+ union {
+ /** @value: Return for scalar data values */
+ __u64 value;
+ /** @ptr: Pointer to user structs when required */
+ __u64 ptr;
+ };
+
+ /** @reserved: MBZ */
+ __u64 reserved[2];
+};
+
/**
* struct drm_xe_exec_queue_create - Input of &DRM_IOCTL_XE_EXEC_QUEUE_CREATE
*
--
2.43.0
^ permalink raw reply related [flat|nested] 10+ messages in thread* [PATCH v3 5/6] drm/xe/xe_gt_pagefault: Add address_type field to pagefaults
2025-02-28 18:21 [PATCH v3 0/6] drm/xe/xe_vm: Implement xe_vm_get_property_ioctl Jonathan Cavitt
` (3 preceding siblings ...)
2025-02-28 18:21 ` [PATCH v3 4/6] drm/xe/uapi: Define drm_xe_vm_get_property Jonathan Cavitt
@ 2025-02-28 18:21 ` Jonathan Cavitt
2025-02-28 18:21 ` [PATCH v3 6/6] drm/xe/xe_vm: Implement xe_vm_get_property_ioctl Jonathan Cavitt
5 siblings, 0 replies; 10+ messages in thread
From: Jonathan Cavitt @ 2025-02-28 18:21 UTC (permalink / raw)
To: intel-xe
Cc: saurabhg.gupta, alex.zuo, jonathan.cavitt, joonas.lahtinen,
matthew.brost, jianxun.zhang, dri-devel
Add a new field to the xe_pagefault struct, address_type, that tracks
the type of fault the pagefault incurred.
Signed-off-by: Jonathan Cavitt <jonathan.cavitt@intel.com>
---
drivers/gpu/drm/xe/xe_gt_pagefault.c | 3 +++
drivers/gpu/drm/xe/xe_gt_pagefault.h | 1 +
2 files changed, 4 insertions(+)
diff --git a/drivers/gpu/drm/xe/xe_gt_pagefault.c b/drivers/gpu/drm/xe/xe_gt_pagefault.c
index 84907fb4295e..ecf9f76bd423 100644
--- a/drivers/gpu/drm/xe/xe_gt_pagefault.c
+++ b/drivers/gpu/drm/xe/xe_gt_pagefault.c
@@ -204,11 +204,13 @@ static int handle_pagefault(struct xe_gt *gt, struct xe_pagefault *pf)
vma = lookup_vma(vm, pf->page_addr);
if (!vma) {
+ pf->address_type = DRM_XE_FAULT_ADDRESS_TYPE_NONE_EXT;
err = -EINVAL;
goto unlock_vm;
}
if (xe_vma_read_only(vma) && pf->access_type != XE_PAGEFAULT_ACCESS_TYPE_READ) {
+ pf->address_type = DRM_XE_FAULT_ADDRESS_TYPE_WRITE_INVALID_EXT;
err = -EPERM;
goto unlock_vm;
}
@@ -276,6 +278,7 @@ static bool get_pagefault(struct pf_queue *pf_queue, struct xe_pagefault *pf)
pf->asid = FIELD_GET(PFD_ASID, desc->dw1);
pf->vfid = FIELD_GET(PFD_VFID, desc->dw2);
pf->access_type = FIELD_GET(PFD_ACCESS_TYPE, desc->dw2);
+ pf->address_type = 0;
pf->fault_type = FIELD_GET(PFD_FAULT_TYPE, desc->dw2);
pf->page_addr = (u64)(FIELD_GET(PFD_VIRTUAL_ADDR_HI, desc->dw3)) <<
PFD_VIRTUAL_ADDR_HI_SHIFT;
diff --git a/drivers/gpu/drm/xe/xe_gt_pagefault.h b/drivers/gpu/drm/xe/xe_gt_pagefault.h
index 33616043d17a..969f7b458d3f 100644
--- a/drivers/gpu/drm/xe/xe_gt_pagefault.h
+++ b/drivers/gpu/drm/xe/xe_gt_pagefault.h
@@ -17,6 +17,7 @@ struct xe_pagefault {
u16 pdata;
u8 vfid;
u8 access_type;
+ u8 address_type;
u8 fault_type;
u8 fault_level;
u8 engine_class;
--
2.43.0
^ permalink raw reply related [flat|nested] 10+ messages in thread* [PATCH v3 6/6] drm/xe/xe_vm: Implement xe_vm_get_property_ioctl
2025-02-28 18:21 [PATCH v3 0/6] drm/xe/xe_vm: Implement xe_vm_get_property_ioctl Jonathan Cavitt
` (4 preceding siblings ...)
2025-02-28 18:21 ` [PATCH v3 5/6] drm/xe/xe_gt_pagefault: Add address_type field to pagefaults Jonathan Cavitt
@ 2025-02-28 18:21 ` Jonathan Cavitt
2025-03-03 20:22 ` Zhang, Jianxun
5 siblings, 1 reply; 10+ messages in thread
From: Jonathan Cavitt @ 2025-02-28 18:21 UTC (permalink / raw)
To: intel-xe
Cc: saurabhg.gupta, alex.zuo, jonathan.cavitt, joonas.lahtinen,
matthew.brost, jianxun.zhang, dri-devel
Add support for userspace to request a list of observed failed
pagefaults from a specified VM.
v2:
- Only allow querying of failed pagefaults (Matt Brost)
Signed-off-by: Jonathan Cavitt <jonathan.cavitt@intel.com>
Suggested-by: Matthew Brost <matthew.brost@intel.com>
---
drivers/gpu/drm/xe/xe_device.c | 3 ++
drivers/gpu/drm/xe/xe_vm.c | 79 ++++++++++++++++++++++++++++++++++
drivers/gpu/drm/xe/xe_vm.h | 2 +
3 files changed, 84 insertions(+)
diff --git a/drivers/gpu/drm/xe/xe_device.c b/drivers/gpu/drm/xe/xe_device.c
index 9454b51f7ad8..43accae152ff 100644
--- a/drivers/gpu/drm/xe/xe_device.c
+++ b/drivers/gpu/drm/xe/xe_device.c
@@ -193,6 +193,9 @@ static const struct drm_ioctl_desc xe_ioctls[] = {
DRM_IOCTL_DEF_DRV(XE_WAIT_USER_FENCE, xe_wait_user_fence_ioctl,
DRM_RENDER_ALLOW),
DRM_IOCTL_DEF_DRV(XE_OBSERVATION, xe_observation_ioctl, DRM_RENDER_ALLOW),
+ DRM_IOCTL_DEF_DRV(XE_VM_GET_PROPERTY, xe_vm_get_property_ioctl,
+ DRM_RENDER_ALLOW),
+
};
static long xe_drm_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
diff --git a/drivers/gpu/drm/xe/xe_vm.c b/drivers/gpu/drm/xe/xe_vm.c
index 6211b971bbbd..00d2c62ccf53 100644
--- a/drivers/gpu/drm/xe/xe_vm.c
+++ b/drivers/gpu/drm/xe/xe_vm.c
@@ -3234,6 +3234,85 @@ int xe_vm_bind_ioctl(struct drm_device *dev, void *data, struct drm_file *file)
return err;
}
+static u32 xe_vm_get_property_size(struct xe_vm *vm, u32 property)
+{
+ u32 size = 0;
+
+ switch (property) {
+ case DRM_XE_VM_GET_PROPERTY_FAULTS:
+ spin_lock(&vm->pfs.lock);
+ size = vm->pfs.len * sizeof(struct drm_xe_pf);
+ spin_unlock(&vm->pfs.lock);
+ return size;
+ default:
+ return -EINVAL;
+ }
+}
+
+static int fill_property_pfs(struct xe_vm *vm,
+ struct drm_xe_vm_get_property *args,
+ u32 size)
+{
+ struct drm_xe_pf __user *usr_ptr = u64_to_user_ptr(args->ptr);
+ struct drm_xe_pf *fault_list;
+ struct drm_xe_pf *fault;
+ struct xe_vm_pf_entry *entry;
+ int i = 0;
+
+ if (copy_from_user(&fault_list, usr_ptr, size))
+ return -EFAULT;
+
+ spin_lock(&vm->pfs.lock);
+ list_for_each_entry(entry, &vm->pfs.list, list) {
+ struct xe_pagefault *pf = entry->pf;
+
+ fault = &fault_list[i++];
+ fault->address = pf->page_addr;
+ fault->address_type = pf->address_type;
+ fault->address_precision = SZ_4K;
+ }
+ spin_unlock(&vm->pfs.lock);
+
+ if (copy_to_user(usr_ptr, &fault_list, size))
+ return -EFAULT;
+
+ return 0;
+}
+
+int xe_vm_get_property_ioctl(struct drm_device *drm, void *data,
+ struct drm_file *file)
+{
+ struct xe_device *xe = to_xe_device(drm);
+ struct xe_file *xef = to_xe_file(file);
+ struct drm_xe_vm_get_property *args = data;
+ struct xe_vm *vm;
+ u32 size;
+
+ if (XE_IOCTL_DBG(xe, args->reserved[0] || args->reserved[1]))
+ return -EINVAL;
+
+ vm = xe_vm_lookup(xef, args->vm_id);
+ if (XE_IOCTL_DBG(xe, !vm))
+ return -ENOENT;
+
+ size = xe_vm_get_property_size(vm, args->property);
+ if (size < 0) {
+ return size;
+ } else if (args->size != size) {
+ if (args->size)
+ return -EINVAL;
+ args->size = size;
+ return 0;
+ }
+
+ switch (args->property) {
+ case DRM_XE_VM_GET_PROPERTY_FAULTS:
+ return fill_property_pfs(vm, args, size);
+ default:
+ return -EINVAL;
+ }
+}
+
/**
* xe_vm_bind_kernel_bo - bind a kernel BO to a VM
* @vm: VM to bind the BO to
diff --git a/drivers/gpu/drm/xe/xe_vm.h b/drivers/gpu/drm/xe/xe_vm.h
index 4d94ab5c8ea4..bf6604465aa3 100644
--- a/drivers/gpu/drm/xe/xe_vm.h
+++ b/drivers/gpu/drm/xe/xe_vm.h
@@ -184,6 +184,8 @@ int xe_vm_destroy_ioctl(struct drm_device *dev, void *data,
struct drm_file *file);
int xe_vm_bind_ioctl(struct drm_device *dev, void *data,
struct drm_file *file);
+int xe_vm_get_property_ioctl(struct drm_device *dev, void *data,
+ struct drm_file *file);
void xe_vm_close_and_put(struct xe_vm *vm);
--
2.43.0
^ permalink raw reply related [flat|nested] 10+ messages in thread* Re: [PATCH v3 6/6] drm/xe/xe_vm: Implement xe_vm_get_property_ioctl
2025-02-28 18:21 ` [PATCH v3 6/6] drm/xe/xe_vm: Implement xe_vm_get_property_ioctl Jonathan Cavitt
@ 2025-03-03 20:22 ` Zhang, Jianxun
2025-03-03 20:53 ` Cavitt, Jonathan
0 siblings, 1 reply; 10+ messages in thread
From: Zhang, Jianxun @ 2025-03-03 20:22 UTC (permalink / raw)
To: Cavitt, Jonathan, intel-xe@lists.freedesktop.org
Cc: Gupta, saurabhg, Zuo, Alex, joonas.lahtinen@linux.intel.com,
Brost, Matthew, dri-devel@lists.freedesktop.org
[-- Attachment #1: Type: text/plain, Size: 5727 bytes --]
________________________________
From: Cavitt, Jonathan <jonathan.cavitt@intel.com>
Sent: Friday, February 28, 2025 10:21 AM
To: intel-xe@lists.freedesktop.org <intel-xe@lists.freedesktop.org>
Cc: Gupta, saurabhg <saurabhg.gupta@intel.com>; Zuo, Alex <alex.zuo@intel.com>; Cavitt, Jonathan <jonathan.cavitt@intel.com>; joonas.lahtinen@linux.intel.com <joonas.lahtinen@linux.intel.com>; Brost, Matthew <matthew.brost@intel.com>; Zhang, Jianxun <jianxun.zhang@intel.com>; dri-devel@lists.freedesktop.org <dri-devel@lists.freedesktop.org>
Subject: [PATCH v3 6/6] drm/xe/xe_vm: Implement xe_vm_get_property_ioctl
Add support for userspace to request a list of observed failed
pagefaults from a specified VM.
v2:
- Only allow querying of failed pagefaults (Matt Brost)
Signed-off-by: Jonathan Cavitt <jonathan.cavitt@intel.com>
Suggested-by: Matthew Brost <matthew.brost@intel.com>
---
drivers/gpu/drm/xe/xe_device.c | 3 ++
drivers/gpu/drm/xe/xe_vm.c | 79 ++++++++++++++++++++++++++++++++++
drivers/gpu/drm/xe/xe_vm.h | 2 +
3 files changed, 84 insertions(+)
diff --git a/drivers/gpu/drm/xe/xe_device.c b/drivers/gpu/drm/xe/xe_device.c
index 9454b51f7ad8..43accae152ff 100644
--- a/drivers/gpu/drm/xe/xe_device.c
+++ b/drivers/gpu/drm/xe/xe_device.c
@@ -193,6 +193,9 @@ static const struct drm_ioctl_desc xe_ioctls[] = {
DRM_IOCTL_DEF_DRV(XE_WAIT_USER_FENCE, xe_wait_user_fence_ioctl,
DRM_RENDER_ALLOW),
DRM_IOCTL_DEF_DRV(XE_OBSERVATION, xe_observation_ioctl, DRM_RENDER_ALLOW),
+ DRM_IOCTL_DEF_DRV(XE_VM_GET_PROPERTY, xe_vm_get_property_ioctl,
+ DRM_RENDER_ALLOW),
+
};
static long xe_drm_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
diff --git a/drivers/gpu/drm/xe/xe_vm.c b/drivers/gpu/drm/xe/xe_vm.c
index 6211b971bbbd..00d2c62ccf53 100644
--- a/drivers/gpu/drm/xe/xe_vm.c
+++ b/drivers/gpu/drm/xe/xe_vm.c
@@ -3234,6 +3234,85 @@ int xe_vm_bind_ioctl(struct drm_device *dev, void *data, struct drm_file *file)
return err;
}
+static u32 xe_vm_get_property_size(struct xe_vm *vm, u32 property)
+{
+ u32 size = 0;
+
+ switch (property) {
+ case DRM_XE_VM_GET_PROPERTY_FAULTS:
+ spin_lock(&vm->pfs.lock);
+ size = vm->pfs.len * sizeof(struct drm_xe_pf);
+ spin_unlock(&vm->pfs.lock);
+ return size;
+ default:
+ return -EINVAL;
+ }
+}
+
+static int fill_property_pfs(struct xe_vm *vm,
+ struct drm_xe_vm_get_property *args,
+ u32 size)
+{
+ struct drm_xe_pf __user *usr_ptr = u64_to_user_ptr(args->ptr);
+ struct drm_xe_pf *fault_list;
+ struct drm_xe_pf *fault;
+ struct xe_vm_pf_entry *entry;
+ int i = 0;
+
+ if (copy_from_user(&fault_list, usr_ptr, size))
+ return -EFAULT;
Why is copying from user space memory needed in this query ioctl?
Does copy_from_user() automatically allocate kernel memory for fault_list?
+
+ spin_lock(&vm->pfs.lock);
+ list_for_each_entry(entry, &vm->pfs.list, list) {
+ struct xe_pagefault *pf = entry->pf;
+
+ fault = &fault_list[i++];
+ fault->address = pf->page_addr;
+ fault->address_type = pf->address_type;
+ fault->address_precision = SZ_4K;
If we can get the exact address, the precision should be 1, right? (https://registry.khronos.org/vulkan/specs/latest/man/html/VkDeviceFaultAddressInfoEXT.html)
+ }
+ spin_unlock(&vm->pfs.lock);
+
+ if (copy_to_user(usr_ptr, &fault_list, size))
+ return -EFAULT;
+
+ return 0;
+}
+
+int xe_vm_get_property_ioctl(struct drm_device *drm, void *data,
+ struct drm_file *file)
+{
+ struct xe_device *xe = to_xe_device(drm);
+ struct xe_file *xef = to_xe_file(file);
+ struct drm_xe_vm_get_property *args = data;
+ struct xe_vm *vm;
+ u32 size;
+
+ if (XE_IOCTL_DBG(xe, args->reserved[0] || args->reserved[1]))
+ return -EINVAL;
+
+ vm = xe_vm_lookup(xef, args->vm_id);
+ if (XE_IOCTL_DBG(xe, !vm))
+ return -ENOENT;
+
+ size = xe_vm_get_property_size(vm, args->property);
+ if (size < 0) {
+ return size;
+ } else if (args->size != size) {
+ if (args->size)
+ return -EINVAL;
If there is a change in the size of property between the first and the second calls, say, more faults added in this case, the 2nd call will fail unintendedly.
+ args->size = size;
+ return 0;
+ }
+
+ switch (args->property) {
+ case DRM_XE_VM_GET_PROPERTY_FAULTS:
+ return fill_property_pfs(vm, args, size);
+ default:
+ return -EINVAL;
+ }
+}
+
/**
* xe_vm_bind_kernel_bo - bind a kernel BO to a VM
* @vm: VM to bind the BO to
diff --git a/drivers/gpu/drm/xe/xe_vm.h b/drivers/gpu/drm/xe/xe_vm.h
index 4d94ab5c8ea4..bf6604465aa3 100644
--- a/drivers/gpu/drm/xe/xe_vm.h
+++ b/drivers/gpu/drm/xe/xe_vm.h
@@ -184,6 +184,8 @@ int xe_vm_destroy_ioctl(struct drm_device *dev, void *data,
struct drm_file *file);
int xe_vm_bind_ioctl(struct drm_device *dev, void *data,
struct drm_file *file);
+int xe_vm_get_property_ioctl(struct drm_device *dev, void *data,
+ struct drm_file *file);
void xe_vm_close_and_put(struct xe_vm *vm);
--
2.43.0
[-- Attachment #2: Type: text/html, Size: 12387 bytes --]
^ permalink raw reply related [flat|nested] 10+ messages in thread* RE: [PATCH v3 6/6] drm/xe/xe_vm: Implement xe_vm_get_property_ioctl
2025-03-03 20:22 ` Zhang, Jianxun
@ 2025-03-03 20:53 ` Cavitt, Jonathan
0 siblings, 0 replies; 10+ messages in thread
From: Cavitt, Jonathan @ 2025-03-03 20:53 UTC (permalink / raw)
To: Zhang, Jianxun, intel-xe@lists.freedesktop.org
Cc: Gupta, saurabhg, Zuo, Alex, joonas.lahtinen@linux.intel.com,
Brost, Matthew, dri-devel@lists.freedesktop.org, Cavitt, Jonathan
[-- Attachment #1: Type: text/plain, Size: 7422 bytes --]
From: Zhang, Jianxun <jianxun.zhang@intel.com>
Sent: Monday, March 3, 2025 12:23 PM
To: Cavitt, Jonathan <jonathan.cavitt@intel.com>; intel-xe@lists.freedesktop.org
Cc: Gupta, saurabhg <saurabhg.gupta@intel.com>; Zuo, Alex <alex.zuo@intel.com>; joonas.lahtinen@linux.intel.com; Brost, Matthew <matthew.brost@intel.com>; dri-devel@lists.freedesktop.org
Subject: Re: [PATCH v3 6/6] drm/xe/xe_vm: Implement xe_vm_get_property_ioctl
________________________________
From: Cavitt, Jonathan <jonathan.cavitt@intel.com<mailto:jonathan.cavitt@intel.com>>
Sent: Friday, February 28, 2025 10:21 AM
To: intel-xe@lists.freedesktop.org<mailto:intel-xe@lists.freedesktop.org> <intel-xe@lists.freedesktop.org<mailto:intel-xe@lists.freedesktop.org>>
Cc: Gupta, saurabhg <saurabhg.gupta@intel.com<mailto:saurabhg.gupta@intel.com>>; Zuo, Alex <alex.zuo@intel.com<mailto:alex.zuo@intel.com>>; Cavitt, Jonathan <jonathan.cavitt@intel.com<mailto:jonathan.cavitt@intel.com>>; joonas.lahtinen@linux.intel.com<mailto:joonas.lahtinen@linux.intel.com> <joonas.lahtinen@linux.intel.com<mailto:joonas.lahtinen@linux.intel.com>>; Brost, Matthew <matthew.brost@intel.com<mailto:matthew.brost@intel.com>>; Zhang, Jianxun <jianxun.zhang@intel.com<mailto:jianxun.zhang@intel.com>>; dri-devel@lists.freedesktop.org<mailto:dri-devel@lists.freedesktop.org> <dri-devel@lists.freedesktop.org<mailto:dri-devel@lists.freedesktop.org>>
Subject: [PATCH v3 6/6] drm/xe/xe_vm: Implement xe_vm_get_property_ioctl
Add support for userspace to request a list of observed failed
pagefaults from a specified VM.
v2:
- Only allow querying of failed pagefaults (Matt Brost)
Signed-off-by: Jonathan Cavitt <jonathan.cavitt@intel.com<mailto:jonathan.cavitt@intel.com>>
Suggested-by: Matthew Brost <matthew.brost@intel.com<mailto:matthew.brost@intel.com>>
---
drivers/gpu/drm/xe/xe_device.c | 3 ++
drivers/gpu/drm/xe/xe_vm.c | 79 ++++++++++++++++++++++++++++++++++
drivers/gpu/drm/xe/xe_vm.h | 2 +
3 files changed, 84 insertions(+)
diff --git a/drivers/gpu/drm/xe/xe_device.c b/drivers/gpu/drm/xe/xe_device.c
index 9454b51f7ad8..43accae152ff 100644
--- a/drivers/gpu/drm/xe/xe_device.c
+++ b/drivers/gpu/drm/xe/xe_device.c
@@ -193,6 +193,9 @@ static const struct drm_ioctl_desc xe_ioctls[] = {
DRM_IOCTL_DEF_DRV(XE_WAIT_USER_FENCE, xe_wait_user_fence_ioctl,
DRM_RENDER_ALLOW),
DRM_IOCTL_DEF_DRV(XE_OBSERVATION, xe_observation_ioctl, DRM_RENDER_ALLOW),
+ DRM_IOCTL_DEF_DRV(XE_VM_GET_PROPERTY, xe_vm_get_property_ioctl,
+ DRM_RENDER_ALLOW),
+
};
static long xe_drm_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
diff --git a/drivers/gpu/drm/xe/xe_vm.c b/drivers/gpu/drm/xe/xe_vm.c
index 6211b971bbbd..00d2c62ccf53 100644
--- a/drivers/gpu/drm/xe/xe_vm.c
+++ b/drivers/gpu/drm/xe/xe_vm.c
@@ -3234,6 +3234,85 @@ int xe_vm_bind_ioctl(struct drm_device *dev, void *data, struct drm_file *file)
return err;
}
+static u32 xe_vm_get_property_size(struct xe_vm *vm, u32 property)
+{
+ u32 size = 0;
+
+ switch (property) {
+ case DRM_XE_VM_GET_PROPERTY_FAULTS:
+ spin_lock(&vm->pfs.lock);
+ size = vm->pfs.len * sizeof(struct drm_xe_pf);
+ spin_unlock(&vm->pfs.lock);
+ return size;
+ default:
+ return -EINVAL;
+ }
+}
+
+static int fill_property_pfs(struct xe_vm *vm,
+ struct drm_xe_vm_get_property *args,
+ u32 size)
+{
+ struct drm_xe_pf __user *usr_ptr = u64_to_user_ptr(args->ptr);
+ struct drm_xe_pf *fault_list;
+ struct drm_xe_pf *fault;
+ struct xe_vm_pf_entry *entry;
+ int i = 0;
+
+ if (copy_from_user(&fault_list, usr_ptr, size))
+ return -EFAULT;
> Why is copying from user space memory needed in this query ioctl?
> Does copy_from_user() automatically allocate kernel memory for fault_list?
Hrmm… No, you have a point. It probably isn’t necessary to copy the memory
from userspace if we don’t need to read anything from it. Though I’m fairly certain
it does automatically allocate kernel memory for the fault_list, as we do similar
things in xe_query.c without allocating memory for first pointer.
+
+ spin_lock(&vm->pfs.lock);
+ list_for_each_entry(entry, &vm->pfs.list, list) {
+ struct xe_pagefault *pf = entry->pf;
+
+ fault = &fault_list[i++];
+ fault->address = pf->page_addr;
+ fault->address_type = pf->address_type;
+ fault->address_precision = SZ_4K;
> If we can get the exact address, the precision should be 1, right?
> (https://registry.khronos.org/vulkan/specs/latest/man/html/VkDeviceFaultAddressInfoEXT.html)
I… don’t know, actually. That question would be better answered by Matthew Brost, I think.
I’ll change it to 1 for v3.
+ }
+ spin_unlock(&vm->pfs.lock);
+
+ if (copy_to_user(usr_ptr, &fault_list, size))
+ return -EFAULT;
+
+ return 0;
+}
+
+int xe_vm_get_property_ioctl(struct drm_device *drm, void *data,
+ struct drm_file *file)
+{
+ struct xe_device *xe = to_xe_device(drm);
+ struct xe_file *xef = to_xe_file(file);
+ struct drm_xe_vm_get_property *args = data;
+ struct xe_vm *vm;
+ u32 size;
+
+ if (XE_IOCTL_DBG(xe, args->reserved[0] || args->reserved[1]))
+ return -EINVAL;
+
+ vm = xe_vm_lookup(xef, args->vm_id);
+ if (XE_IOCTL_DBG(xe, !vm))
+ return -ENOENT;
+
+ size = xe_vm_get_property_size(vm, args->property);
+ if (size < 0) {
+ return size;
+ } else if (args->size != size) {
+ if (args->size)
+ return -EINVAL;
> If there is a change in the size of property between the first and the second calls, say, more faults added in this case, the 2nd call will fail unintendedly.
I suppose the alternative would be to ask userspace to allocate memory for the maximum
possible number of pagefaults, then return the number of pagefaults in “value” while saving
the pagefaults to “*ptr”.
Hmm… Give me a moment to see what that would look like…
-Jonathan Cavitt
+ args->size = size;
+ return 0;
+ }
+
+ switch (args->property) {
+ case DRM_XE_VM_GET_PROPERTY_FAULTS:
+ return fill_property_pfs(vm, args, size);
+ default:
+ return -EINVAL;
+ }
+}
+
/**
* xe_vm_bind_kernel_bo - bind a kernel BO to a VM
* @vm: VM to bind the BO to
diff --git a/drivers/gpu/drm/xe/xe_vm.h b/drivers/gpu/drm/xe/xe_vm.h
index 4d94ab5c8ea4..bf6604465aa3 100644
--- a/drivers/gpu/drm/xe/xe_vm.h
+++ b/drivers/gpu/drm/xe/xe_vm.h
@@ -184,6 +184,8 @@ int xe_vm_destroy_ioctl(struct drm_device *dev, void *data,
struct drm_file *file);
int xe_vm_bind_ioctl(struct drm_device *dev, void *data,
struct drm_file *file);
+int xe_vm_get_property_ioctl(struct drm_device *dev, void *data,
+ struct drm_file *file);
void xe_vm_close_and_put(struct xe_vm *vm);
--
2.43.0
[-- Attachment #2: Type: text/html, Size: 19998 bytes --]
^ permalink raw reply related [flat|nested] 10+ messages in thread