Re: [PATCH V3 3/4] drm/xe/xe3p_lpg: Enable L2 flush optimization feature

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

On Fri, 2026-02-20 at 12:06 +0000, Matthew Auld wrote:
> On 20/02/2026 11:50, Thomas Hellström wrote:
> > On Fri, 2026-02-20 at 11:46 +0000, Matthew Auld wrote:
> > > On 20/02/2026 10:16, Tejas Upadhyay wrote:
> > > > When set, the L2 flush optimization feature will control
> > > > whether L2 is in Persistent or Transient mode through
> > > > monitoring of media activity.
> > > > 
> > > > To enable L2 flush optimization include new feature flag
> > > > GUC_CTL_ENABLE_L2FLUSH_OPT for Novalake platforms when
> > > > media type is detected.
> > > > 
> > > > Also, restrict userptr, svm and dmabuf mappings to be
> > > > either 2WAY or XA+1WAY
> > > > 
> > > > V2(MattA): validate dma-buf bos and madvise pat-index
> > 
> > Question: Assuming that we on *faulting* VMs always perform a TLB
> > flush
> > on unbind. Can we eliminate the PAT restrictions on those? That
> > would
> > actually then include all SVM maps.
> 
> With unbind do you mean notifier invalidation flow on svm side? Or 
> actual vma unbind?

Both actually, or to rephrase, before we release any memory back to
system we have removed its GPU mappings and flushed TLB?

/Thomas



> 
> > 
> > /Thomas
> >   
> > 
> > > > 
> > > > Signed-off-by: Tejas Upadhyay <tejas.upadhyay@intel.com>
> > > > ---
> > > >    drivers/gpu/drm/xe/xe_guc.c        |  3 +++
> > > >    drivers/gpu/drm/xe/xe_guc_fwif.h   |  1 +
> > > >    drivers/gpu/drm/xe/xe_vm.c         |  9 +++++++++
> > > >    drivers/gpu/drm/xe/xe_vm_madvise.c | 18 ++++++++++++++++++
> > > >    4 files changed, 31 insertions(+)
> > > > 
> > > > diff --git a/drivers/gpu/drm/xe/xe_guc.c
> > > > b/drivers/gpu/drm/xe/xe_guc.c
> > > > index cbbb4d665b8f..97c33c3dd520 100644
> > > > --- a/drivers/gpu/drm/xe/xe_guc.c
> > > > +++ b/drivers/gpu/drm/xe/xe_guc.c
> > > > @@ -98,6 +98,9 @@ static u32 guc_ctl_feature_flags(struct
> > > > xe_guc
> > > > *guc)
> > > >    	if (xe_guc_using_main_gamctrl_queues(guc))
> > > >    		flags |= GUC_CTL_MAIN_GAMCTRL_QUEUES;
> > > >    
> > > > +	if (GRAPHICS_VER(xe) >= 35 && !IS_DGFX(xe) &&
> > > > xe_gt_is_media_type(guc_to_gt(guc)))
> > > > +		flags |= GUC_CTL_ENABLE_L2FLUSH_OPT;
> > > > +
> > > >    	return flags;
> > > >    }
> > > >    
> > > > diff --git a/drivers/gpu/drm/xe/xe_guc_fwif.h
> > > > b/drivers/gpu/drm/xe/xe_guc_fwif.h
> > > > index a33ea288b907..39ff7b3e960b 100644
> > > > --- a/drivers/gpu/drm/xe/xe_guc_fwif.h
> > > > +++ b/drivers/gpu/drm/xe/xe_guc_fwif.h
> > > > @@ -67,6 +67,7 @@ struct guc_update_exec_queue_policy {
> > > >    #define   GUC_CTL_ENABLE_PSMI_LOGGING	BIT(7)
> > > >    #define   GUC_CTL_MAIN_GAMCTRL_QUEUES	BIT(9)
> > > >    #define   GUC_CTL_DISABLE_SCHEDULER	BIT(14)
> > > > +#define   GUC_CTL_ENABLE_L2FLUSH_OPT	BIT(15)
> > > >    
> > > >    #define GUC_CTL_DEBUG			3
> > > >    #define   GUC_LOG_VERBOSITY		REG_GENMASK(1, 0)
> > > > diff --git a/drivers/gpu/drm/xe/xe_vm.c
> > > > b/drivers/gpu/drm/xe/xe_vm.c
> > > > index c06fd250e037..e2e4c9648d05 100644
> > > > --- a/drivers/gpu/drm/xe/xe_vm.c
> > > > +++ b/drivers/gpu/drm/xe/xe_vm.c
> > > > @@ -3474,6 +3474,11 @@ static int
> > > > vm_bind_ioctl_check_args(struct
> > > > xe_device *xe, struct xe_vm *vm,
> > > >    				 op ==
> > > > DRM_XE_VM_BIND_OP_MAP_USERPTR) ||
> > > >    		    XE_IOCTL_DBG(xe, coh_mode == XE_COH_NONE
> > > > &&
> > > >    				 op ==
> > > > DRM_XE_VM_BIND_OP_MAP_USERPTR) ||
> > > > +		    XE_IOCTL_DBG(xe,
> > > > xe_device_is_l2_flush_optimized(xe) &&
> > > > +				 (op ==
> > > > DRM_XE_VM_BIND_OP_MAP_USERPTR ||
> > > > +				  /* svm */
> > > > +				  op == (DRM_XE_VM_BIND_OP_MAP
> > > > &&
> > > > is_cpu_addr_mirror)) &&
> > > 
> > > op == (DRM_XE_VM_BIND_OP_MAP && is_cpu_addr_mirror)
> > > 
> > > I think you meant (op == DRM_XE_VM_BIND_OP_MAP) && is_cpu ?
> > > 
> > > But maybe just drop the op check. Having the check being
> > > consistent
> > > for
> > > bind/unbind matches existing uapi behaviour?
> > > 
> > > > +				 (pat_index != 19 || coh_mode
> > > > !=
> > > > XE_COH_2WAY)) ||
> > > >    		    XE_IOCTL_DBG(xe, comp_en &&
> > > >    				 op ==
> > > > DRM_XE_VM_BIND_OP_MAP_USERPTR) ||
> > > >    		    XE_IOCTL_DBG(xe, op ==
> > > > DRM_XE_VM_BIND_OP_MAP_USERPTR &&
> > > > @@ -3608,6 +3613,10 @@ static int
> > > > xe_vm_bind_ioctl_validate_bo(struct xe_device *xe, struct xe_bo
> > > > *bo,
> > > >    	if (XE_IOCTL_DBG(xe, bo->ttm.base.import_attach &&
> > > > comp_en))
> > > >    		return -EINVAL;
> > > >    
> > > > +	if (XE_IOCTL_DBG(xe, bo->ttm.base.import_attach &&
> > > > xe_device_is_l2_flush_optimized(xe) &&
> > > > +			 (pat_index != 19 || coh_mode !=
> > > > XE_COH_2WAY)))
> > > > +		return -EINVAL;
> > > > +
> > > >    	/* If a BO is protected it can only be mapped if the
> > > > key
> > > > is still valid */
> > > >    	if ((bind_flags & DRM_XE_VM_BIND_FLAG_CHECK_PXP) &&
> > > > xe_bo_is_protected(bo) &&
> > > >    	    op != DRM_XE_VM_BIND_OP_UNMAP && op !=
> > > > DRM_XE_VM_BIND_OP_UNMAP_ALL)
> > > > diff --git a/drivers/gpu/drm/xe/xe_vm_madvise.c
> > > > b/drivers/gpu/drm/xe/xe_vm_madvise.c
> > > > index 1a1ad8c07d49..2a35dbeba2d8 100644
> > > > --- a/drivers/gpu/drm/xe/xe_vm_madvise.c
> > > > +++ b/drivers/gpu/drm/xe/xe_vm_madvise.c
> > > > @@ -411,6 +411,7 @@ int xe_vm_madvise_ioctl(struct drm_device
> > > > *dev,
> > > > void *data, struct drm_file *fil
> > > >    	struct xe_vmas_in_madvise_range madvise_range = {.addr
> > > > =
> > > > args->start,
> > > >    							
> > > > .range =
> > > > args->range, };
> > > >    	struct xe_madvise_details details;
> > > > +	u16 pat_index, coh_mode;
> > > >    	struct xe_vm *vm;
> > > >    	struct drm_exec exec;
> > > >    	int err, attr_type;
> > > > @@ -447,6 +448,15 @@ int xe_vm_madvise_ioctl(struct drm_device
> > > > *dev, void *data, struct drm_file *fil
> > > >    	if (err || !madvise_range.num_vmas)
> > > >    		goto madv_fini;
> > > >    
> > > > +	pat_index = array_index_nospec(args->pat_index.val,
> > > > xe-
> > > > > pat.n_entries);
> > > 
> > > This needs to be conditional on DRM_XE_MEM_RANGE_ATTR_PAT. This
> > > is a
> > > union underneath so pat_index.val is not actually a pat_index for
> > > the
> > > other madv types, but just some other random data.
> > > 
> > > > +	coh_mode = xe_pat_index_get_coh_mode(xe, pat_index);
> > > > +	if (XE_IOCTL_DBG(xe,
> > > > madvise_range.has_svm_userptr_vmas &&
> > > > +			 xe_device_is_l2_flush_optimized(xe)
> > > > &&
> > > > +			 (pat_index != 19 || coh_mode !=
> > > > XE_COH_2WAY))) {
> > > > +		err = -EINVAL;
> > > > +		goto madv_fini;
> > > > +	}
> > > > +
> > > >    	if (madvise_range.has_bo_vmas) {
> > > >    		if (args->type ==
> > > > DRM_XE_MEM_RANGE_ATTR_ATOMIC) {
> > > >    			if (!check_bo_args_are_sane(vm,
> > > > madvise_range.vmas,
> > > > @@ -464,6 +474,14 @@ int xe_vm_madvise_ioctl(struct drm_device
> > > > *dev, void *data, struct drm_file *fil
> > > >    
> > > >    				if (!bo)
> > > >    					continue;
> > > > +
> > > > +				if (XE_IOCTL_DBG(xe, bo-
> > > > > ttm.base.import_attach &&
> > > > +						
> > > > xe_device_is_l2_flush_optimized(xe) &&
> > > > +						 (pat_index !=
> > > > 19
> > > > > > coh_mode != XE_COH_2WAY))) {
> > > > +					err = -EINVAL;
> > > > +					goto err_fini;
> > > > +				}
> > > > +
> > > >    				err = drm_exec_lock_obj(&exec,
> > > > &bo->ttm.base);
> > > >    				drm_exec_retry_on_contention(&
> > > > exec
> > > > );
> > > >    				if (err)