Re: [PATCH v4 3/3] drm/xe: Add up-to-date implementation for Wa_14026539277

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] drm/xe: misplaced Wa_14026539277 PAT check in xe_vm_madvise_ioctl()
--

commit f0309ee40a85276908319b3fa40c88f56ae1d6c4
Author: Gustavo Sousa <gustavo.sousa@intel.com>

drm/xe: Add up-to-date implementation for Wa_14026539277

This commit implements Wa_14026539277 for A* steppings of graphics version
35.10 by converting internal 2-way-coherent PAT entries to 1-way-coherent and
device-uncached, disabling cached page tables, and rejecting 2-way-coherent
PAT indices in the VM_BIND and VM_MADVISE ioctls.

[ ... ]

> diff --git a/drivers/gpu/drm/xe/xe_vm_madvise.c b/drivers/gpu/drm/xe/xe_vm_madvise.c
> index 875c00563f910..5e3699d133d44 100644
> --- a/drivers/gpu/drm/xe/xe_vm_madvise.c
> +++ b/drivers/gpu/drm/xe/xe_vm_madvise.c
> @@ -496,6 +496,9 @@ static bool check_pat_args_are_sane(struct xe_device *xe,
>  	u16 coh_mode = xe_pat_index_get_coh_mode(xe, pat_index);
>  	int i;
>  
> +	if (XE_IOCTL_DBG(xe, xe_pat_wa_14026539277_reserved(xe, pat_index)))
> +		return false;

[Severity: Low]
Should this xe_pat_wa_14026539277_reserved() check be placed in
madvise_args_are_sane() alongside the existing xe_pat_index_is_valid() check
instead of in check_pat_args_are_sane()?

In xe_vm_madvise_ioctl(), madvise_args_are_sane() validates VMA-independent
arguments upfront:

	case DRM_XE_MEM_RANGE_ATTR_PAT:
	{
		u16 pat_index;

		if (XE_IOCTL_DBG(xe, args->pat_index.val >= xe->pat.n_entries))
			return false;

		pat_index = array_index_nospec(args->pat_index.val, xe->pat.n_entries);
		if (XE_IOCTL_DBG(xe, !xe_pat_index_is_valid(xe, pat_index)))
			return false;

whereas check_pat_args_are_sane() is only called after xe_svm_flush(),
xe_vm_alloc_madvise_vma(), and get_vmas():

	err = xe_vm_alloc_madvise_vma(vm, madvise_range.addr, args->range);
	if (err)
		goto madv_fini;

	err = get_vmas(vm, &madvise_range);
	if (err || !madvise_range.num_vmas)
		goto madv_fini;

	if (args->type == DRM_XE_MEM_RANGE_ATTR_PAT) {
		...
		if (!check_pat_args_are_sane(xe, &madvise_range, pat_index)) {
			err = -EINVAL;
			goto free_vmas;
		}
	}

When userspace passes a PAT index reserved by Wa_14026539277, will
xe_vm_madvise_ioctl() return 0 instead of -EINVAL if the target range has no
VMAs, or split CPU_ADDR_MIRROR VMAs in xe_vm_alloc_madvise_vma() before
rejecting the PAT index?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261026-wa_14026539277-v4-0-54e40d3e92aa@intel.com?part=3