[v2 19/31] drm/xe/svm: Determine a vma is backed by device memory

[Zeng, Oak]

> -----Original Message-----
> From: Brost, Matthew <matthew.brost at intel.com>
> Sent: Wednesday, April 10, 2024 5:57 PM
> To: Zeng, Oak <oak.zeng at intel.com>
> Cc: intel-xe at lists.freedesktop.org; Ghimiray, Himal Prasad
> <himal.prasad.ghimiray at intel.com>; Bommu, Krishnaiah
> <krishnaiah.bommu at intel.com>; Thomas.Hellstrom at linux.intel.com; Welty,
> Brian <brian.welty at intel.com>
> Subject: Re: [v2 19/31] drm/xe/svm: Determine a vma is backed by device
> memory
> 
> On Tue, Apr 09, 2024 at 04:17:30PM -0400, Oak Zeng wrote:
> > With system allocator, a userptr can now be back by device
> > memory also. Introduce a helper function xe_vma_is_devmem
> > to determine whether a vma is backed by device memory.
> >
> > Signed-off-by: Oak Zeng <oak.zeng at intel.com>
> > ---
> >  drivers/gpu/drm/xe/xe_pt.c | 14 ++++++++++++--
> >  1 file changed, 12 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/xe/xe_pt.c b/drivers/gpu/drm/xe/xe_pt.c
> > index 846e896edcb5..525092111be9 100644
> > --- a/drivers/gpu/drm/xe/xe_pt.c
> > +++ b/drivers/gpu/drm/xe/xe_pt.c
> > @@ -577,6 +577,17 @@ static const struct xe_pt_walk_ops
> xe_pt_stage_bind_ops = {
> >  	.pt_entry = xe_pt_stage_bind_entry,
> >  };
> >
> > +static bool xe_vma_is_devmem(struct xe_vma *vma)
> 
> At some point we probably want to scrub the driver as we intermix
> devmem, vram, and lmem nomenclature. I think in case we mean the same
> thing too. Anwyays that is a little out of scope here.

Agree we are mix using those terms.

Devmem is still good to me in this case. I will keep it for now.

> 
> > +{
> > +	if (xe_vma_is_userptr(vma)) {
> > +		struct xe_userptr_vma *uvma = to_userptr_vma(vma);
> > +		return uvma->userptr.is_device_pages;
> 
> Helper itself LGTM. Maybe promote to xe_vm.c/xe_vm.h?
> 
> Also consider other options rather than userptr.is_device_pages flag
> here (e.g. look for buddy blocks, check a gpuvm flags, etc...). Can live
> with a flag but we can do without it, great.

For now this helper is only used in this file. I will keep it local static in xe_pt.c for now. We can consider a move if it is used in other file in the future.

In v3, I introduced range based page table update. Instead of update page table for the whole vma, we can now update only a sub-range of a vma. 

For system allocator, the backing store of a vma can now be a mixture placement of system memory and device memory. I will update this helper to reflect such change.

Oak

> 
> Matt
> 
> > +	} else {
> > +		struct xe_bo *bo = xe_vma_bo(vma);
> > +		return bo && (xe_bo_is_vram(bo) ||
> xe_bo_is_stolen_devmem(bo));
> > +	}
> > +}
> > +
> >  /**
> >   * xe_pt_stage_bind() - Build a disconnected page-table tree for a given
> address
> >   * range.
> > @@ -601,8 +612,7 @@ xe_pt_stage_bind(struct xe_tile *tile, struct
> xe_vma *vma,
> >  {
> >  	struct xe_device *xe = tile_to_xe(tile);
> >  	struct xe_bo *bo = xe_vma_bo(vma);
> > -	bool is_devmem = !xe_vma_is_userptr(vma) && bo &&
> > -		(xe_bo_is_vram(bo) || xe_bo_is_stolen_devmem(bo));
> > +	bool is_devmem = xe_vma_is_devmem(vma);
> >  	struct xe_res_cursor curs;
> >  	struct xe_pt_stage_bind_walk xe_walk = {
> >  		.base = {
> > --
> > 2.26.3
> >