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

[Matthew Auld <matthew.auld@intel.com>]

On 19/08/2026 21:37, Gustavo Sousa wrote:
> Matthew Auld <matthew.auld@intel.com> writes:
> 
>> On 24/07/2026 22:44, Gustavo Sousa wrote:
>>> Wa_14026539277 is a temporary workaround that targets only A*
>>> steppings of graphics version 35.10 and requires that we avoid using
>>> 2-way coherency for device-cacheable memory accesses.  For
>>> driver-internal usage, we convert those configurations to be
>>> device-uncached and 1-way-coherent; on the UAPI side, we reject
>>> VM_BIND/MADVISE calls that are 2-way-coherent and leave it up to
>>> userspace to select the alternative option that is most appropriate to
>>> their usage.
>>>
>>> v2:
>>>     - Add missing bits to convert the PAT value to device-uncacheable in
>>>       wa_14026539277_fixup_pat_value(). (Sashiko)
>>>     - Also validate the PAT index in the DRM_XE_VM_MADVISE
>>>       ioctl. (Sashiko)
>>>     - Match against the graphics IP instead of the platform.
>>>
>>> Signed-off-by: Gustavo Sousa <gustavo.sousa@intel.com>
>>> ---
>>>    drivers/gpu/drm/xe/xe_pat.h        | 10 +++++++
>>>    drivers/gpu/drm/xe/xe_device.c     | 16 ++++++----
>>>    drivers/gpu/drm/xe/xe_pat.c        | 61 ++++++++++++++++++++++++++++++++++----
>>>    drivers/gpu/drm/xe/xe_vm.c         |  5 ++++
>>>    drivers/gpu/drm/xe/xe_vm_madvise.c |  3 ++
>>>    drivers/gpu/drm/xe/xe_wa_oob.rules |  1 +
>>>    6 files changed, 84 insertions(+), 12 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/xe/xe_pat.h b/drivers/gpu/drm/xe/xe_pat.h
>>> index 7060f66e1d63..10374022f1d2 100644
>>> --- a/drivers/gpu/drm/xe/xe_pat.h
>>> +++ b/drivers/gpu/drm/xe/xe_pat.h
>>> @@ -82,6 +82,16 @@ bool xe_pat_index_get_comp_en(struct xe_device *xe, u16 pat_index);
>>>     */
>>>    u16 xe_pat_index_get_l3_policy(struct xe_device *xe, u16 pat_index);
>>>    
>>> +/**
>>> + * xe_pat_wa_14026539277_reserved - Is this PAT index reserved from
>>> + * use due to Wa_14026539277?
>>> + * @xe: xe device
>>> + * @pat_index: The pat_index to query
>>> + *
>>> + * Return: a boolean indicating whether the PAT index is reserved or not.
>>> + */
>>> +bool xe_pat_wa_14026539277_reserved(struct xe_device *xe, u16 pat_index);
>>> +
>>>    #define xe_cache_pat_idx(xe, cache_mode) ({					\
>>>    	const struct xe_device *__xedev = (xe);					\
>>>    	enum xe_cache_level __mode = (cache_mode);				\
>>> diff --git a/drivers/gpu/drm/xe/xe_device.c b/drivers/gpu/drm/xe/xe_device.c
>>> index 5189419a0593..c4b8023cb8a2 100644
>>> --- a/drivers/gpu/drm/xe/xe_device.c
>>> +++ b/drivers/gpu/drm/xe/xe_device.c
>>> @@ -836,14 +836,18 @@ static void override_has_cached_pt(struct xe_device *xe)
>>>    	struct xe_gt *gt;
>>>    	u8 id;
>>>    
>>> -	/*
>>> -	 * Wa_16029380221: The affected GT will always use non-coherent
>>> -	 * access to page tables, so we must do uncached writes from the
>>> -	 * CPU.
>>> -	 */
>>> -	for_each_gt(gt, xe, id)
>>> +	for_each_gt(gt, xe, id) {
>>> +		/*
>>> +		 * Wa_16029380221: The affected GT will always use
>>> +		 * non-coherent access to page tables, so we must do
>>> +		 * uncached writes from the CPU.
>>> +		 */
>>>    		if (XE_GT_WA(gt, 16029380221))
>>>    			xe->info.has_cached_pt = false;
>>> +
>>> +		if (XE_GT_WA(gt, 14026539277))
>>> +			xe->info.has_cached_pt = false;
>>> +	}
>>>    }
>>>    
>>>    static int probe_has_flat_ccs(struct xe_device *xe)
>>> diff --git a/drivers/gpu/drm/xe/xe_pat.c b/drivers/gpu/drm/xe/xe_pat.c
>>> index a5fe1beec652..a9cde12eba70 100644
>>> --- a/drivers/gpu/drm/xe/xe_pat.c
>>> +++ b/drivers/gpu/drm/xe/xe_pat.c
>>> @@ -313,6 +313,37 @@ u16 xe_pat_index_get_l3_policy(struct xe_device *xe, u16 pat_index)
>>>    	return REG_FIELD_GET(XE2_L3_POLICY, xe->pat.table[pat_index].value);
>>>    }
>>>    
>>> +bool xe_pat_wa_14026539277_reserved(struct xe_device *xe, u16 pat_index)
>>> +{
>>> +	struct xe_gt *gt;
>>> +	u8 id;
>>> +	bool has_wa = false;
>>> +
>>> +	for_each_gt(gt, xe, id) {
>>> +		if (XE_GT_WA(gt, 14026539277)) {
>>> +			has_wa = true;
>>> +			break;
>>> +		}
>>> +	}
>>> +
>>> +	return has_wa && xe_pat_index_get_l3_policy(xe, pat_index) != XE_L3_POLICY_UC &&
>>
>> Just to make sure I understand. Why do we bother to check the l3 policy
>> here? Do we not just want to ban all 2WAY from uapi pov? The l3/l4
>> policy can often be promoted via the per MOCS setting, so even if UC
>> here it can be made WB via MOCS. Is this OK?
> 
> That's a good point.
> 
> The workaround instructions were to avoid using GT-cacheable
> 2-way-coherent configurations, that's why I was allowing 2-way coherency
> for XE_L3_POLICY_UC, but I did not think of the scenario of the cache
> policy being overridden by MOCS.  Thanks for pointing it out!
> 
> Yeah, it seems we should use a bigger hammer and disallow any 2-way
> coherency then.  I'll change this on the next version.

Feel free to add:
Reviewed-by: Matthew Auld <matthew.auld@intel.com>

> 
> --
> Gustavo Sousa
> 
>>
>>> +	       xe_pat_index_get_coh_mode(xe, pat_index) == XE_COH_2WAY;
>>> +}
>>> +
>>> +static u32 wa_14026539277_fixup_pat_value(struct xe_gt *gt, u32 value)
>>> +{
>>> +	if (XE_GT_WA(gt, 14026539277)) {
>>> +		if (REG_FIELD_GET(XE2_L3_POLICY, value) != XE_L3_POLICY_UC &&
>>> +		    REG_FIELD_GET(XE2_COH_MODE, value) == XE_COH_2WAY) {
>>> +			value &= ~(XE2_L3_POLICY | XE2_COH_MODE);
>>> +			value |= REG_FIELD_PREP(XE2_L3_POLICY, XE_L3_POLICY_UC) |
>>> +				 REG_FIELD_PREP(XE2_COH_MODE, XE_COH_1WAY);
>>> +		}
>>> +	}
>>> +
>>> +	return value;
>>> +}
>>> +
>>>    static const struct xe_pat_table_entry *gt_pta_entry(struct xe_gt *gt)
>>>    {
>>>    	struct xe_device *xe = gt_to_xe(gt);
>>> @@ -373,18 +404,22 @@ static void program_pat_mcr(struct xe_gt *gt, const struct xe_pat_table_entry ta
>>>    
>>>    	for (int i = 0; i < n_entries; i++) {
>>>    		struct xe_reg_mcr reg_mcr = XE_REG_MCR(_PAT_INDEX(i));
>>> +		u32 pat = wa_14026539277_fixup_pat_value(gt, table[i].value);
>>>    
>>> -		xe_gt_mcr_multicast_write(gt, reg_mcr, table[i].value);
>>> +		xe_gt_mcr_multicast_write(gt, reg_mcr, pat);
>>>    	}
>>>    
>>>    	if (xe->pat.pat_ats)
>>> -		xe_gt_mcr_multicast_write(gt, XE_REG_MCR(_PAT_ATS), xe->pat.pat_ats->value);
>>> +		xe_gt_mcr_multicast_write(gt, XE_REG_MCR(_PAT_ATS),
>>> +					  wa_14026539277_fixup_pat_value(gt, xe->pat.pat_ats->value));
>>>    
>>>    	if (pta_entry)
>>> -		xe_gt_mcr_multicast_write(gt, XE_REG_MCR(_PAT_PTA), pta_entry->value);
>>> +		xe_gt_mcr_multicast_write(gt, XE_REG_MCR(_PAT_PTA),
>>> +					  wa_14026539277_fixup_pat_value(gt, pta_entry->value));
>>>    
>>>    	if (tr_pta_entry)
>>> -		xe_gt_mcr_multicast_write(gt, XE_REG_MCR(_PAT_TR_PTA), tr_pta_entry->value);
>>> +		xe_gt_mcr_multicast_write(gt, XE_REG_MCR(_PAT_TR_PTA),
>>> +					  wa_14026539277_fixup_pat_value(gt, tr_pta_entry->value));
>>>    }
>>>    
>>>    static int xelp_dump(struct xe_gt *gt, struct drm_printer *p)
>>> @@ -534,13 +569,16 @@ static int xe2_dump(struct xe_gt *gt, struct drm_printer *p)
>>>    	drm_printf(p, "PAT table: (* = reserved entry)\n");
>>>    
>>>    	for (i = 0; i < xe->pat.n_entries; i++) {
>>> +		bool rsvd = !xe->pat.table[i].valid ||
>>> +			    xe_pat_wa_14026539277_reserved(xe, i);
>>> +
>>>    		if (xe_gt_is_media_type(gt))
>>>    			pat = xe_mmio_read32(&gt->mmio, XE_REG(_PAT_INDEX(i)));
>>>    		else
>>>    			pat = xe_gt_mcr_unicast_read_any(gt, XE_REG_MCR(_PAT_INDEX(i)));
>>>    
>>>    		xe_pat_index_label(label, sizeof(label), i);
>>> -		xe->pat.ops->entry_dump(p, label, pat, !xe->pat.table[i].valid);
>>> +		xe->pat.ops->entry_dump(p, label, pat, rsvd);
>>>    	}
>>>    
>>>    	/*
>>> @@ -747,9 +785,14 @@ int xe_pat_dump_sw_config(struct xe_gt *gt, struct drm_printer *p)
>>>    	for (u32 i = 0; i < xe->pat.n_entries; i++) {
>>>    		u32 pat = xe->pat.table[i].value;
>>>    
>>> +		pat = wa_14026539277_fixup_pat_value(gt, pat);
>>> +
>>>    		if (GRAPHICS_VER(xe) >= 20) {
>>> +			bool rsvd = !xe->pat.table[i].valid ||
>>> +				    xe_pat_wa_14026539277_reserved(xe, i);
>>> +
>>>    			xe_pat_index_label(label, sizeof(label), i);
>>> -			xe->pat.ops->entry_dump(p, label, pat, !xe->pat.table[i].valid);
>>> +			xe->pat.ops->entry_dump(p, label, pat, rsvd);
>>>    		} else if (xe->info.platform == XE_METEORLAKE) {
>>>    			xelpg_pat_entry_dump(p, i, pat);
>>>    		} else if (xe->info.platform == XE_PVC) {
>>> @@ -764,6 +807,8 @@ int xe_pat_dump_sw_config(struct xe_gt *gt, struct drm_printer *p)
>>>    	if (pta_entry) {
>>>    		u32 pat = pta_entry->value;
>>>    
>>> +		pat = wa_14026539277_fixup_pat_value(gt, pat);
>>> +
>>>    		drm_printf(p, "Page Table Access:\n");
>>>    		xe->pat.ops->entry_dump(p, "PTA_MODE", pat, false);
>>>    	}
>>> @@ -771,6 +816,8 @@ int xe_pat_dump_sw_config(struct xe_gt *gt, struct drm_printer *p)
>>>    	if (tr_pta_entry) {
>>>    		u32 pat = tr_pta_entry->value;
>>>    
>>> +		pat = wa_14026539277_fixup_pat_value(gt, pat);
>>> +
>>>    		drm_printf(p, "TRTT Page Table Access:\n");
>>>    		xe->pat.ops->entry_dump(p, "TR_PTA_MODE", pat, false);
>>>    	}
>>> @@ -778,6 +825,8 @@ int xe_pat_dump_sw_config(struct xe_gt *gt, struct drm_printer *p)
>>>    	if (xe->pat.pat_ats) {
>>>    		u32 pat = xe->pat.pat_ats->value;
>>>    
>>> +		pat = wa_14026539277_fixup_pat_value(gt, pat);
>>> +
>>>    		drm_printf(p, "PCIe ATS/PASID:\n");
>>>    		xe->pat.ops->entry_dump(p, "PAT_ATS ", pat, false);
>>>    	}
>>> diff --git a/drivers/gpu/drm/xe/xe_vm.c b/drivers/gpu/drm/xe/xe_vm.c
>>> index 9e0176861cb6..64c03c669630 100644
>>> --- a/drivers/gpu/drm/xe/xe_vm.c
>>> +++ b/drivers/gpu/drm/xe/xe_vm.c
>>> @@ -3697,6 +3697,11 @@ static int vm_bind_ioctl_check_args(struct xe_device *xe, struct xe_vm *vm,
>>>    			goto free_bind_ops;
>>>    		}
>>>    
>>> +		if (XE_IOCTL_DBG(xe, xe_pat_wa_14026539277_reserved(xe, pat_index))) {
>>> +			err = -EINVAL;
>>> +			goto free_bind_ops;
>>> +		}
>>> +
>>>    		if (XE_IOCTL_DBG(xe, op > DRM_XE_VM_BIND_OP_PREFETCH) ||
>>>    		    XE_IOCTL_DBG(xe, flags & ~SUPPORTED_FLAGS) ||
>>>    		    XE_IOCTL_DBG(xe, obj && (is_null || is_cpu_addr_mirror)) ||
>>> diff --git a/drivers/gpu/drm/xe/xe_vm_madvise.c b/drivers/gpu/drm/xe/xe_vm_madvise.c
>>> index 0474768a38aa..265273294756 100644
>>> --- a/drivers/gpu/drm/xe/xe_vm_madvise.c
>>> +++ b/drivers/gpu/drm/xe/xe_vm_madvise.c
>>> @@ -500,6 +500,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;
>>> +
>>>    	/*
>>>    	 * Using coh_none with CPU cached buffers is not allowed on iGPU.
>>>    	 * On iGPU the GPU shares the LLC with the CPU, so with coh_none
>>> diff --git a/drivers/gpu/drm/xe/xe_wa_oob.rules b/drivers/gpu/drm/xe/xe_wa_oob.rules
>>> index f02ac9bf7424..d53e7f75814b 100644
>>> --- a/drivers/gpu/drm/xe/xe_wa_oob.rules
>>> +++ b/drivers/gpu/drm/xe/xe_wa_oob.rules
>>> @@ -71,4 +71,5 @@
>>>    		GRAPHICS_VERSION(3511)
>>>    16029897822	MEDIA_VERSION(3500)
>>>    		GRAPHICS_VERSION(3510)
>>> +14026539277	GRAPHICS_VERSION(3510), GRAPHICS_STEP(A0, B0)
>>>    14027054324	GRAPHICS_VERSION(3511)
>>>