Re: [PATCH v2] drm/xe/vm: Fix BO prefetch with CONSULT_MEM_ADVISE_PREF_LOC

[Matthew Brost <matthew.brost@intel.com>]

On Wed, Jul 01, 2026 at 02:51:39PM +0530, Ghimiray, Himal Prasad wrote:
> 
> 
> On 25-06-2026 02:38, Matthew Brost wrote:
> > On Wed, Jun 24, 2026 at 02:07:13PM -0700, Matthew Brost wrote:
> > > On Wed, Jun 24, 2026 at 11:19:44PM +0530, Himal Prasad Ghimiray wrote:
> > > > When prefetch region is DRM_XE_CONSULT_MEM_ADVISE_PREF_LOC for a BO VMA,
> > > > the code used it as an index into region_to_mem_type[], causing an
> > > > out-of-bounds access since the value is -1.
> > > > 
> > > > Resolve the preferred location for BO VMAs directly: local VRAM on dGFX
> > > > (using the BO's tile placement) or system memory on iGPU.
> > > > 
> > > > Discovered using AI-assisted static analysis confirmed by Intel Product
> > > > Security.
> > > > 
> > > > v2:
> > > > -Fix null dereference
> > > > 
> > > > Reported-by: Martin Hodo <martin.hodo@intel.com>
> > > > Fixes: c1bb69a2e8e2 ("drm/xe/svm: Consult madvise preferred location in prefetch")
> > > > Cc: Matthew Brost <matthew.brost@intel.com>
> > > > Signed-off-by: Himal Prasad Ghimiray<himal.prasad.ghimiray@intel.com>
> > > > ---
> > > >   drivers/gpu/drm/xe/xe_vm.c | 25 ++++++++++++++++++++-----
> > > >   1 file changed, 20 insertions(+), 5 deletions(-)
> > > > 
> > > > diff --git a/drivers/gpu/drm/xe/xe_vm.c b/drivers/gpu/drm/xe/xe_vm.c
> > > > index 080c2fff0e95..32ded13491ca 100644
> > > > --- a/drivers/gpu/drm/xe/xe_vm.c
> > > > +++ b/drivers/gpu/drm/xe/xe_vm.c
> > > > @@ -3255,11 +3255,26 @@ static int op_lock_and_prep(struct drm_exec *exec, struct xe_vm *vm,
> > > >   						    .request_decompress = false,
> > > >   						    .check_purged = true,
> > > >   					    });
> > > > -		if (!err && !xe_vma_has_no_bo(vma))
> > > > -			err = xe_bo_migrate(xe_vma_bo(vma),
> > > > -					    region_to_mem_type[region],
> > > > -					    NULL,
> > > > -					    exec);
> > > > +		if (!err && !xe_vma_has_no_bo(vma)) {
> > > > +			struct xe_bo *bo = xe_vma_bo(vma);
> > > > +			u32 mem_type;
> > > > +
> > > > +			if (region == DRM_XE_CONSULT_MEM_ADVISE_PREF_LOC) {
> > > 
> > > Shouldn't logic more or less look like xe_vma_resolve_pagemap which
> > > picks the region based on vma->attr.preferred_loc.devmem_fd?
> > > 
> > > So roughly:
> > > 
> > > mem_type = mem_type = bo->placements[i].mem_type;
> > > 
> > 
> > typo: s/bo->placements[i].mem_type/bo->placements[0].mem_type/;
> 
> 
> Thanks, Matt for the review. For a BO VMA the preferred location always
> stays DEFAULT_DEVICE (madvise skips BO VMAs), so devmem_fd is never
> DEFAULT_SYSTEM and both approaches pick the same
> placement. Extending madvise to BO VMAs would also need a design call
> first, since preferred_loc is per-VMA  and a BO can be mapped by multiple
> VMAs, so preferences could conflict. I'd
> prefer to keep this patch minimal, but happy to switch if you feel strongly.
> 

Ah, yes - I forgot madvise isn't implemented with BO VMAs.

So LGTM:
Reviewed-by: Matthew Brost <matthew.brost@intel.com>

> > 
> > Matt
> > 
> > > for (i = 0; i < bo->placement.num_placement; i++) {
> > > 	if(mem_type_is_vram(bo->placements[i].mem_type) &&
> > > 	   vma->attr.preferred_loc.devmem_fd == DRM_XE_PREFERRED_LOC_DEFAULT_DEVICE) {
> > > 		mem_type = bo->placements[i].mem_type;
> > > 		break;
> > > 	} else if (!mem_type_is_vram(bo->placements[i].mem_type &&
> > > 		   vma->attr.preferred_loc.devmem_fd == DRM_XE_PREFERRED_LOC_DEFAULT_SYSTEM) {
> > > 		mem_type = bo->placements[i].mem_type;
> > > 		break;
> > > 	}
> > > }
> > > 
> > > Matt
> > > 
> > > > +				unsigned int i;
> > > > +
> > > > +				mem_type = XE_PL_TT;
> > > > +				for (i = 0; i < bo->placement.num_placement; i++) {
> > > > +					if (mem_type_is_vram(bo->placements[i].mem_type)) {
> > > > +						mem_type = bo->placements[i].mem_type;
> > > > +						break;
> > > > +					}
> > > > +				}
> > > > +			} else {
> > > > +				mem_type = region_to_mem_type[region];
> > > > +			}
> > > > +
> > > > +			err = xe_bo_migrate(bo, mem_type, NULL, exec);
> > > > +		}
> > > >   		break;
> > > >   	}
> > > >   	default:
> > > > -- 
> > > > 2.43.0
> > > > 
>