Re: [PATCH V5 3/4] drm/xe/xe3p_lpg: Restrict UAPI to enable L2 flush optimization

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

Hi, Tejas,

On Tue, 2026-03-03 at 11:54 +0530, Tejas Upadhyay wrote:
> When set, starting xe3p_lpg, 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.
> 
> Tighten UAPI validation to restrict userptr, svm and
> dmabuf mappings to be either 2WAY or XA+1WAY
> 
> V4(MattA): Modify uapi doc and commit
> V3(MattA): check valid op and pat_index value
> V2(MattA): validate dma-buf bos and madvise pat-index
> 
> Acked-by: Michal Mrozek <michal.mrozek@intel.com>
> 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         |  8 ++++++++
>  drivers/gpu/drm/xe/xe_vm_madvise.c | 23 +++++++++++++++++++++++
>  include/uapi/drm/xe_drm.h          |  4 +++-
>  5 files changed, 38 insertions(+), 1 deletion(-)
> 

Claude has some suggestions on this patch:

"
commit 77271d8c323486ebe5dc6f1264fc318a43a29084
Author: Tejas Upadhyay <tejas.upadhyay@intel.com>

drm/xe/xe3p_lpg: Restrict UAPI to enable L2 flush optimization

This commit adds UAPI validation to restrict userptr, svm, and dmabuf
mappings on L2 flush optimized platforms (Xe3p+ non-DGFX) to use either
2WAY coherency or XA with 1WAY coherency (pat_index 19).

> diff --git a/drivers/gpu/drm/xe/xe_vm.c b/drivers/gpu/drm/xe/xe_vm.c
> index 99fa36123f58..42356d73ca65 100644
> --- a/drivers/gpu/drm/xe/xe_vm.c
> +++ b/drivers/gpu/drm/xe/xe_vm.c

[ ... ]

> @@ -3480,6 +3480,10 @@ 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 ||
> +			  is_cpu_addr_mirror) &&
> +			 (pat_index != 19 || coh_mode != XE_COH_2WAY))
||

The condition uses || but should this use && instead?

The commit message says mappings should be allowed if they use "either
2WAY or XA+1WAY". The UAPI documentation also says "the kernel expects
either 2WAY or 1WAY and XA @pat_index".

Looking at xe3p_lpg_pat_table in xe_pat.c:

    [19] = XE2_PAT( 1, 0, 0, 2, 3, 2 )

The last parameter is coh_mode=2, which maps to XE_COH_1WAY based on
the XE2_PAT macro:

    .coh_mode = __coh_mode ? __coh_mode : XE_COH_NONE

So pat_index 19 has coh_mode=XE_COH_1WAY, not XE_COH_2WAY.

With the current || condition, for pat_index 19:
  - pat_index != 19 is false
  - coh_mode != XE_COH_2WAY is true (since coh_mode is XE_COH_1WAY)
  - false || true = true, validation fails

This means userptr/svm operations will always fail validation on L2
flush optimized platforms, even when using the correct pat_index 19.

Should the condition be:

    (pat_index != 19 && coh_mode != XE_COH_2WAY)

This would fail only if the mapping is neither pat_index 19 (XA+1WAY)
nor using 2WAY coherency, matching the commit message intent.

The same issue appears in xe_vm_bind_ioctl_validate_bo() and
xe_vm_madvise_ioctl() at the similar validation points.

"

Thanks,
Thomas