[PATCH i-g-t 2/5] lib/xe: Added xe_vm_madvise ioctl support
Gurram, Pravalika
pravalika.gurram at intel.com
Fri Aug 29 04:51:14 UTC 2025
> -----Original Message-----
> From: igt-dev <igt-dev-bounces at lists.freedesktop.org> On Behalf Of Gurram,
> Pravalika
> Sent: Friday, August 29, 2025 10:09 AM
> To: Sharma, Nishit <nishit.sharma at intel.com>; igt-dev at lists.freedesktop.org;
> Ghimiray, Himal Prasad <himal.prasad.ghimiray at intel.com>; Brost, Matthew
> <matthew.brost at intel.com>
> Cc: Siddiqui, Ayaz A <ayaz.siddiqui at intel.com>
> Subject: RE: [PATCH i-g-t 2/5] lib/xe: Added xe_vm_madvise ioctl support
>
>
>
> > -----Original Message-----
> > From: Sharma, Nishit <nishit.sharma at intel.com>
> > Sent: Thursday, August 28, 2025 10:18 PM
> > To: Gurram, Pravalika <pravalika.gurram at intel.com>; igt-
> > dev at lists.freedesktop.org; Ghimiray, Himal Prasad
> > <himal.prasad.ghimiray at intel.com>; Brost, Matthew
> > <matthew.brost at intel.com>
> > Cc: Siddiqui, Ayaz A <ayaz.siddiqui at intel.com>
> > Subject: RE: [PATCH i-g-t 2/5] lib/xe: Added xe_vm_madvise ioctl
> > support
> >
> >
> >
> > -----Original Message-----
> > From: Gurram, Pravalika <pravalika.gurram at intel.com>
> > Sent: Thursday, August 28, 2025 8:23 PM
> > To: Sharma, Nishit <nishit.sharma at intel.com>;
> > igt-dev at lists.freedesktop.org; Ghimiray, Himal Prasad
> > <himal.prasad.ghimiray at intel.com>; Brost, Matthew
> > <matthew.brost at intel.com>; Sharma, Nishit <nishit.sharma at intel.com>
> > Cc: Siddiqui, Ayaz A <ayaz.siddiqui at intel.com>
> > Subject: RE: [PATCH i-g-t 2/5] lib/xe: Added xe_vm_madvise ioctl
> > support
> >
> >
> >
> > > -----Original Message-----
> > > From: igt-dev <igt-dev-bounces at lists.freedesktop.org> On Behalf Of
> > > nishit.sharma at intel.com
> > > Sent: Tuesday, August 26, 2025 2:12 PM
> > > To: igt-dev at lists.freedesktop.org; Ghimiray, Himal Prasad
> > > <himal.prasad.ghimiray at intel.com>; Brost, Matthew
> > > <matthew.brost at intel.com>; Sharma, Nishit <nishit.sharma at intel.com>
> > > Subject: [PATCH i-g-t 2/5] lib/xe: Added xe_vm_madvise ioctl support
> > >
> > Use ] lib/xe: Add xe_vm_madvise ioctl
> >
> > > From: Nishit Sharma <nishit.sharma at intel.com>
> > >
> > > Added xe_vm_madvise() which issues madvise ioctl
> > > DRM_IOCTL_XE_MADVISE for VM region advising the driver about
> > > expected usage or memory policy for specified address range. MADVISE
> > > ioctl requires pointer to drm_xe_madvise structure as one of the
> > > inputs. Depending upon type of madvise operation like Atomic,
> > > Preferred LOC or PAT required members of drm_xe_madvise structure
> > > are initialized and passed in MADVISE ioctl.
> > >
> > > Signed-off-by: Nishit Sharma <nishit.sharma at intel.com>
> > > ---
> > > lib/xe/xe_ioctl.c | 56
> > > +++++++++++++++++++++++++++++++++++++++++++++++
> > > lib/xe/xe_ioctl.h | 5 ++++-
> > > 2 files changed, 60 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/lib/xe/xe_ioctl.c b/lib/xe/xe_ioctl.c index
> > > 1e95af409..43bad8452
> > > 100644
> > > --- a/lib/xe/xe_ioctl.c
> > > +++ b/lib/xe/xe_ioctl.c
> > > @@ -585,3 +585,59 @@ int64_t xe_wait_ufence(int fd, uint64_t *addr,
> > > uint64_t value,
> > > igt_assert_eq(__xe_wait_ufence(fd, addr, value, exec_queue,
> > > &timeout), 0);
> > > return timeout;
> > > }
> > > +
> > > +int __xe_vm_madvise(int fd, uint32_t vm, uint64_t addr, uint64_t range,
> > > + uint64_t ext, uint32_t type, uint32_t op_val, uint16_t
> > > policy) {
> > > + struct drm_xe_madvise madvise = {};
> > > +
> > > + madvise.extensions = ext;
> > > + madvise.vm_id = vm;
> > > + madvise.start = addr;
> > > + madvise.range = range;
> > > +
> > > + if (type == DRM_XE_MEM_RANGE_ATTR_ATOMIC) {
> > > + madvise.type = DRM_XE_MEM_RANGE_ATTR_ATOMIC;
> > > + madvise.atomic.val = op_val;
> > > + } else if (type == DRM_XE_MEM_RANGE_ATTR_PREFERRED_LOC) {
> > > + madvise.type =
> > > DRM_XE_MEM_RANGE_ATTR_PREFERRED_LOC;
> > > + madvise.preferred_mem_loc.devmem_fd = op_val;
> > > + madvise.preferred_mem_loc.migration_policy = policy;
> > > + igt_debug("madvise.preferred_mem_loc.devmem_fd =
> > > %d\n",
> > > + madvise.preferred_mem_loc.devmem_fd);
> > > + } else if (type == DRM_XE_MEM_RANGE_ATTR_PAT) {
> > > + madvise.type = DRM_XE_MEM_RANGE_ATTR_PAT;
> > > + madvise.pat_index.val = op_val;
> > > + } else {
> > > + igt_warn("Unknown attribute\n");
> > > + return -EINVAL;
> > > + }
> > > +
> > Uses a switch for clarity and extensibility Initialize the struct with
> > designated initializers. "Please refer ___xe_vm_bind"
> > __xe_vm_bind initializes struct drm_xe_vm_bind and struct
> > drm_xe_vm_bind doesn't deal with different type of attributes The
> > struct drm_xe_madvise have different type of attributes like preferred
> > loc, atomic hence have different members. So structure Member
> > initialization is different in this case
>
> Please use space on replay to differentiate
>
> We can assign the remaining members like below, and type can be maintained in
> switch case.
> can you please check if this works
> ----------------------------------------------------------------------------------------
> struct drm_xe_madvise madvise = {
> .extensions = ext,
> .vm_id = vm,
> .start = addr,
> .range = range,
> };
> switch (type) {
> case DRM_XE_MEM_RANGE_ATTR_ATOMIC:
> madvise.type = DRM_XE_MEM_RANGE_ATTR_ATOMIC;
> madvise.atomic.val = op_val;
> break;
> ............. add here rest of the cases -----------
> --------------------------------------------------------------------------------------------
> Pravalika
>
> > > + if (igt_ioctl(fd, DRM_IOCTL_XE_MADVISE, &madvise))
> > > + return -errno;
> > > +
> > > + return 0;
> > > +}
> > > +
> > > +/**
> > > + * xe_vm_madvise:
> > > + * @fd: xe device fd
> > > + * @vm: vm_id of the virtual range
> > > + * @addr: start of the virtual address range
> > > + * @range: size of the virtual address range
> > > + * @ext: Pointer to the first extension struct, if any
> > > + * @type: type of attribute
> > > + * @op_val: fd/atomic value/pat index, depending upon type of
> > > +operation
> > > + * @policy: Page migration policy
> > > + *
> > > + * Function initializes different members of struct drm_xe_madvise
> > > +and calls
> > > + * MADVISE IOCTL .
> > > + *
> > > + * Returns 0 if success and asserts otherwise.
> > > + */
> > > +int xe_vm_madvise(int fd, uint32_t vm, uint64_t addr, uint64_t range,
> > > + uint64_t ext, uint32_t type, uint32_t op_val, uint16_t policy)
> > > {
> > > + igt_assert_eq(__xe_vm_madvise(fd, vm, addr, range, ext, type,
> > > op_val, policy), 0);
> > > + return 0;
> > > +}
> > > diff --git a/lib/xe/xe_ioctl.h b/lib/xe/xe_ioctl.h index
> > > 6302d1a7d..a5996cf65
> > > 100644
> > > --- a/lib/xe/xe_ioctl.h
> > > +++ b/lib/xe/xe_ioctl.h
> > > @@ -99,5 +99,8 @@ int __xe_wait_ufence(int fd, uint64_t *addr,
> > > uint64_t value,
> > > uint32_t exec_queue, int64_t *timeout); int64_t
> > > xe_wait_ufence(int fd, uint64_t *addr, uint64_t value,
> > > uint32_t exec_queue, int64_t timeout);
> > > -
> > > +int __xe_vm_madvise(int fd, uint32_t vm, uint64_t addr, uint64_t
> > > +range,
> > > uint64_t ext,
> > > + uint32_t type, uint32_t op_val, uint16_t policy);
> >
> > Both functions currently use the same parameters. Are there plans to
> > modify __xe_vm_madvise in the future?
> > If not, is there a reason to maintain two separate functions with same
> > arguments?
> > Idea was to follow calling conventions as other function. E.g
> > xe_vm_bind_async() and also ioctl is called from different Function
> > for better readability.
> >
> > --Pravalika
Having two functions with the same signature (like __xe_vm_madvise and xe_vm_madvise) is usually done for below reason
The version with a leading underscore (__xe_vm_madvise) is used as a internal helper,
and the non-underscored version (xe_vm_madvise) is the public API exposed to other modules.
in vm_bind also we are adding extra arg, but i think we can maintain two here in future if needed we can change underscore one which is internal helper
e.g :
-----------
void xe_vm_bind_async(int fd, uint32_t vm, uint32_t exec_queue, uint32_t bo, uint64_t offset, uint64_t addr, uint64_t size, struct drm_xe_sync *sync, uint32_t num_syncs)
void __xe_vm_bind_assert(int fd, uint32_t vm, uint32_t exec_queue, uint32_t bo, uint64_t offset, uint64_t addr, uint64_t size, uint32_t op, uint32_t flags, struct drm_xe_sync *sync, uint32_t num_syncs, uint32_t prefetch_region, uint64_t ext)
-------------
--Pravalika
> > int
> > > +xe_vm_madvise(int fd, uint32_t vm, uint64_t addr, uint64_t range,
> > > +uint64_t
> > > ext,
> > > + uint32_t type, uint32_t op_val, uint16_t policy);
> > > #endif /* XE_IOCTL_H */
> > > --
> > > 2.43.0
More information about the igt-dev
mailing list