Re: [PATCH v4 03/20] drm/xe/vm: Add attributes struct as member of vma

["Thomas Hellström" <thomas.hellstrom@linux.intel.com>]

On Sun, 2025-06-22 at 21:18 -0700, Matthew Brost wrote:
> On Fri, Jun 13, 2025 at 06:25:41PM +0530, Himal Prasad Ghimiray
> wrote:
> > The attribute of xe_vma will determine the migration policy and the
> > encoding of the page table entries (PTEs) for that vma.
> > This attribute helps manage how memory pages are moved and how
> > their
> > addresses are translated. It will be used by madvise to set the
> > behavior of the vma.
> > 
> > v2 (Matthew Brost)
> > - Add docs
> > 
> > Cc: Matthew Brost <matthew.brost@intel.com>
> > Signed-off-by: Himal Prasad Ghimiray
> > <himal.prasad.ghimiray@intel.com>
> > ---
> >  drivers/gpu/drm/xe/xe_vm_types.h | 28 ++++++++++++++++++++++++++++
> >  1 file changed, 28 insertions(+)
> > 
> > diff --git a/drivers/gpu/drm/xe/xe_vm_types.h
> > b/drivers/gpu/drm/xe/xe_vm_types.h
> > index bed6088e1bb3..c7663058a7e3 100644
> > --- a/drivers/gpu/drm/xe/xe_vm_types.h
> > +++ b/drivers/gpu/drm/xe/xe_vm_types.h
> > @@ -77,6 +77,27 @@ struct xe_userptr {
> >  #endif
> >  };
> >  
> > +/**
> > + * struct xe_vma_mem_attr - memory attributes associated with vma
> > + */
> > +struct xe_vma_mem_attr {
> > +	/** @preferred_loc: perferred memory_location*/
> > +	struct {
> > +		/** @preferred_loc.migration_policy: Policy for
> > migration of pages */
> > +		u32 migration_policy;
> > +
> > +		/**
> > +		 * @preferred_loc.devmem_fd: used for determining
> > pagemap_fd requested by user
> > +		 * DRM_XE_PREFERRED_LOC_DEFAULT_SYSTEM and
> > DRM_XE_PREFERRED_LOC_DEFAULT_DEVICE mean
> > +		 * system memory or closest device memory
> > respectively.
> 
> Nit, I generally prefer 80 character line wraps. I know checkpatch
> allows 100 but in general I think Xe generally wraps at 80 unless it
> is
> an odd case with nested code and an 80 character wrap makes the code
> unreadable.
> 
> 80 generally allows 2 windows split vertically in vi on a typical
> laptop
> which makes navigating the code a bit easier.
> 
> > +		 */
> > +		u32 devmem_fd;
> > +	} preferred_loc;
> > +
> > +	/** @atomic_access: The atomic access type for the vma */
> 
> I'd reference the uAPI here. e.g., DRM_XE_VMA_ATOMIC_*

+1.

> 
> With the minor nits fixed:
> Reviewed-by: Matthew Brost <matthew.brost@intel.com>

Also with those fixes, feel free to add
Reviewed-by: Thomas Hellström <thomas.hellstrom@linux.intel.com>

> 
> Matt
> 
> > +	u32 atomic_access;
> > +};
> > +
> >  struct xe_vma {
> >  	/** @gpuva: Base GPUVA object */
> >  	struct drm_gpuva gpuva;
> > @@ -135,6 +156,13 @@ struct xe_vma {
> >  	 * Needs to be signalled before UNMAP can be processed.
> >  	 */
> >  	struct xe_user_fence *ufence;
> > +
> > +	/**
> > +	 * @attr: The attributes of vma which determines the
> > migration policy
> > +	 * and encoding of the PTEs for this vma.
> > +	 */
> > +	struct xe_vma_mem_attr attr;
> > +
> >  };
> >  
> >  /**
> > -- 
> > 2.34.1
> >