| Message ID | 20250310171834.78299-5-jonathan.cavitt@intel.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | drm/xe/xe_vm: Implement xe_vm_get_faults_ioctl | expand |
On Mon, Mar 10, 2025 at 05:18:32PM +0000, Jonathan Cavitt wrote: > Add initial declarations for the drm_xe_vm_get_faults ioctl. > > Signed-off-by: Jonathan Cavitt <jonathan.cavitt@intel.com> > --- > include/uapi/drm/xe_drm.h | 49 +++++++++++++++++++++++++++++++++++++++ > 1 file changed, 49 insertions(+) > > diff --git a/include/uapi/drm/xe_drm.h b/include/uapi/drm/xe_drm.h > index 616916985e3f..90c2fcdbd5c1 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_FAULTS This should be a generic "get VM property" IOCTL, not a specific IOCTL that only retrieves faults. This allows for future expansion of the uAPI. > */ > > /* > @@ -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_FAULTS 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_FAULTS DRM_IOWR(DRM_COMMAND_BASE + DRM_XE_VM_GET_FAULTS, struct drm_xe_vm_get_faults) > > /** > * DOC: Xe IOCTL Extensions > @@ -1189,6 +1192,52 @@ struct drm_xe_vm_bind { > __u64 reserved[2]; > }; > > +struct xe_vm_fault { > + /** @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_faults - Input of &DRM_IOCTL_XE_VM_GET_FAULTS > + * > + * The user provides a VM ID, and the ioctl will > + * > + */ > +struct drm_xe_vm_get_faults { > + /** @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; > + > + /** @size: Size to allocate for @ptr */ > + __u32 size; > + > + /** @fault_count: Number of faults to be returned */ > + __u32 fault_count; > + fault_count is implied by size. Matt > + /** @pad: MBZ */ > + __u32 pad; > + > + /** @reserved: MBZ */ > + __u64 reserved[2]; > + > + /** @faults: Pointer to user-defined array of xe_vm_fault of flexible size */ > + __u64 faults; > +}; > + > /** > * struct drm_xe_exec_queue_create - Input of &DRM_IOCTL_XE_EXEC_QUEUE_CREATE > * > -- > 2.43.0 >
-----Original Message----- From: Brost, Matthew <matthew.brost@intel.com> Sent: Monday, March 10, 2025 9:51 PM To: Cavitt, Jonathan <jonathan.cavitt@intel.com> Cc: intel-xe@lists.freedesktop.org; Gupta, saurabhg <saurabhg.gupta@intel.com>; Zuo, Alex <alex.zuo@intel.com>; joonas.lahtinen@linux.intel.com; Zhang, Jianxun <jianxun.zhang@intel.com>; Lin, Shuicheng <shuicheng.lin@intel.com>; dri-devel@lists.freedesktop.org Subject: Re: [PATCH v7 4/6] drm/xe/uapi: Define drm_xe_vm_get_faults > > On Mon, Mar 10, 2025 at 05:18:32PM +0000, Jonathan Cavitt wrote: > > Add initial declarations for the drm_xe_vm_get_faults ioctl. > > > > Signed-off-by: Jonathan Cavitt <jonathan.cavitt@intel.com> > > --- > > include/uapi/drm/xe_drm.h | 49 +++++++++++++++++++++++++++++++++++++++ > > 1 file changed, 49 insertions(+) > > > > diff --git a/include/uapi/drm/xe_drm.h b/include/uapi/drm/xe_drm.h > > index 616916985e3f..90c2fcdbd5c1 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_FAULTS > > This should be a generic "get VM property" IOCTL, not a specific IOCTL > that only retrieves faults. This allows for future expansion of the > uAPI. Question from @Zhang, Jianxun : """ But first of all, should we regard faults as a property? """ -Jonathan Cavitt > > > */ > > > > /* > > @@ -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_FAULTS 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_FAULTS DRM_IOWR(DRM_COMMAND_BASE + DRM_XE_VM_GET_FAULTS, struct drm_xe_vm_get_faults) > > > > /** > > * DOC: Xe IOCTL Extensions > > @@ -1189,6 +1192,52 @@ struct drm_xe_vm_bind { > > __u64 reserved[2]; > > }; > > > > +struct xe_vm_fault { > > + /** @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_faults - Input of &DRM_IOCTL_XE_VM_GET_FAULTS > > + * > > + * The user provides a VM ID, and the ioctl will > > + * > > + */ > > +struct drm_xe_vm_get_faults { > > + /** @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; > > + > > + /** @size: Size to allocate for @ptr */ > > + __u32 size; > > + > > + /** @fault_count: Number of faults to be returned */ > > + __u32 fault_count; > > + > > fault_count is implied by size. > > Matt > > > + /** @pad: MBZ */ > > + __u32 pad; > > + > > + /** @reserved: MBZ */ > > + __u64 reserved[2]; > > + > > + /** @faults: Pointer to user-defined array of xe_vm_fault of flexible size */ > > + __u64 faults; > > +}; > > + > > /** > > * struct drm_xe_exec_queue_create - Input of &DRM_IOCTL_XE_EXEC_QUEUE_CREATE > > * > > -- > > 2.43.0 > > >
Sorry if my reply is still in a wrong format. I have set plain text in the web-based outlook this time but don't know if it really works. Refer to my inline comment, thanks.
-----Original Message----- From: Zhang, Jianxun <jianxun.zhang@intel.com> Sent: Wednesday, March 12, 2025 9:28 AM To: Cavitt, Jonathan <jonathan.cavitt@intel.com>; Brost, Matthew <matthew.brost@intel.com> Cc: intel-xe@lists.freedesktop.org; Gupta, saurabhg <saurabhg.gupta@intel.com>; Zuo, Alex <alex.zuo@intel.com>; joonas.lahtinen@linux.intel.com; Lin, Shuicheng <shuicheng.lin@intel.com>; dri-devel@lists.freedesktop.org Subject: Re: [PATCH v7 4/6] drm/xe/uapi: Define drm_xe_vm_get_faults > > Sorry if my reply is still in a wrong format. I have set plain text in the web-based outlook this time but don't know if it really works. > > Refer to my inline comment, thanks. > ________________________________________ > From: Cavitt, Jonathan <jonathan.cavitt@intel.com> > Sent: Tuesday, March 11, 2025 7:11 AM > To: Brost, Matthew; Zhang, Jianxun > Cc: intel-xe@lists.freedesktop.org; Gupta, saurabhg; Zuo, Alex; joonas.lahtinen@linux.intel.com; Lin, Shuicheng; dri-devel@lists.freedesktop.org; Cavitt, Jonathan > Subject: RE: [PATCH v7 4/6] drm/xe/uapi: Define drm_xe_vm_get_faults > > -----Original Message----- > From: Brost, Matthew <matthew.brost@intel.com> > Sent: Monday, March 10, 2025 9:51 PM > To: Cavitt, Jonathan <jonathan.cavitt@intel.com> > Cc: intel-xe@lists.freedesktop.org; Gupta, saurabhg <saurabhg.gupta@intel.com>; Zuo, Alex <alex.zuo@intel.com>; joonas.lahtinen@linux.intel.com; Zhang, Jianxun <jianxun.zhang@intel.com>; Lin, Shuicheng <shuicheng.lin@intel.com>; dri-devel@lists.freedesktop.org > Subject: Re: [PATCH v7 4/6] drm/xe/uapi: Define drm_xe_vm_get_faults > > > > On Mon, Mar 10, 2025 at 05:18:32PM +0000, Jonathan Cavitt wrote: > > > Add initial declarations for the drm_xe_vm_get_faults ioctl. > > > > > > Signed-off-by: Jonathan Cavitt <jonathan.cavitt@intel.com> > > > --- > > > include/uapi/drm/xe_drm.h | 49 +++++++++++++++++++++++++++++++++++++++ > > > 1 file changed, 49 insertions(+) > > > > > > diff --git a/include/uapi/drm/xe_drm.h b/include/uapi/drm/xe_drm.h > > > index 616916985e3f..90c2fcdbd5c1 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_FAULTS > > > > This should be a generic "get VM property" IOCTL, not a specific IOCTL > > that only retrieves faults. This allows for future expansion of the > > uAPI. > > Question from @Zhang, Jianxun : > """ > But first of all, should we regard faults as a property? > """ > -Jonathan Cavitt > > Thanks for Jonathan relaying my question here. I just feel it is a little strange to treat "faults" as "property". Perhaps it is just a naming thing. But I get KMD wants to take an extensible and consolidated approach. Either/any way works for me in UMD. I got in touch with Matt Brost, and this was his explanation: """ I can see that PoV, kinda a bikeshed but in general for query things we make them generic and extensible to avoid having a ton of IOCTLs for very specific things """ I guess I'll prioritize reverting it back into a property get ioctl. -Jonathan Cavitt > > Thanks! > > > > > > */ > > > > > > /* > > > @@ -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_FAULTS 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_FAULTS DRM_IOWR(DRM_COMMAND_BASE + DRM_XE_VM_GET_FAULTS, struct drm_xe_vm_get_faults) > > > > > > /** > > > * DOC: Xe IOCTL Extensions > > > @@ -1189,6 +1192,52 @@ struct drm_xe_vm_bind { > > > __u64 reserved[2]; > > > }; > > > > > > +struct xe_vm_fault { > > > + /** @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_faults - Input of &DRM_IOCTL_XE_VM_GET_FAULTS > > > + * > > > + * The user provides a VM ID, and the ioctl will > > > + * > > > + */ > > > +struct drm_xe_vm_get_faults { > > > + /** @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; > > > + > > > + /** @size: Size to allocate for @ptr */ > > > + __u32 size; > > > + > > > + /** @fault_count: Number of faults to be returned */ > > > + __u32 fault_count; > > > + > > > > fault_count is implied by size. > > > > Matt > > > > > + /** @pad: MBZ */ > > > + __u32 pad; > > > + > > > + /** @reserved: MBZ */ > > > + __u64 reserved[2]; > > > + > > > + /** @faults: Pointer to user-defined array of xe_vm_fault of flexible size */ > > > + __u64 faults; > > > +}; > > > + > > > /** > > > * struct drm_xe_exec_queue_create - Input of &DRM_IOCTL_XE_EXEC_QUEUE_CREATE > > > * > > > -- > > > 2.43.0 > > > > > >
diff --git a/include/uapi/drm/xe_drm.h b/include/uapi/drm/xe_drm.h index 616916985e3f..90c2fcdbd5c1 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_FAULTS */ /* @@ -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_FAULTS 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_FAULTS DRM_IOWR(DRM_COMMAND_BASE + DRM_XE_VM_GET_FAULTS, struct drm_xe_vm_get_faults) /** * DOC: Xe IOCTL Extensions @@ -1189,6 +1192,52 @@ struct drm_xe_vm_bind { __u64 reserved[2]; }; +struct xe_vm_fault { + /** @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_faults - Input of &DRM_IOCTL_XE_VM_GET_FAULTS + * + * The user provides a VM ID, and the ioctl will + * + */ +struct drm_xe_vm_get_faults { + /** @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; + + /** @size: Size to allocate for @ptr */ + __u32 size; + + /** @fault_count: Number of faults to be returned */ + __u32 fault_count; + + /** @pad: MBZ */ + __u32 pad; + + /** @reserved: MBZ */ + __u64 reserved[2]; + + /** @faults: Pointer to user-defined array of xe_vm_fault of flexible size */ + __u64 faults; +}; + /** * struct drm_xe_exec_queue_create - Input of &DRM_IOCTL_XE_EXEC_QUEUE_CREATE *
Add initial declarations for the drm_xe_vm_get_faults ioctl. Signed-off-by: Jonathan Cavitt <jonathan.cavitt@intel.com> --- include/uapi/drm/xe_drm.h | 49 +++++++++++++++++++++++++++++++++++++++ 1 file changed, 49 insertions(+)