[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:38:58 UTC 2025



> -----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
> 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