Re: [PATCH v2 3/3] drm/xe/pat: always check is_valid() when querying an index

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

On 22/10/2026 08:02, Upadhyay, Tejas wrote:
> 
> 
>> -----Original Message-----
>> From: Auld, Matthew <matthew.auld@intel.com>
>> Sent: 15 October 2026 21:16
>> To: intel-xe@lists.freedesktop.org
>> Cc: Teres Alexis, Alan Previn <alan.previn.teres.alexis@intel.com>; Upadhyay,
>> Tejas <tejas.upadhyay@intel.com>
>> Subject: [PATCH v2 3/3] drm/xe/pat: always check is_valid() when querying an
>> index
>>
>> Make the bevaviour consistent. If trying to extract comp or l3 on an unknown
>> index, just treat as a programmer error, like we already do for coh_mode
>> (implicit).
>>
>> Signed-off-by: Matthew Auld <matthew.auld@intel.com>
>> Cc: Alan Previn <alan.previn.teres.alexis@intel.com>
>> Cc: Tejas Upadhyay <tejas.upadhyay@intel.com>
>> ---
>>   drivers/gpu/drm/xe/xe_pat.c | 6 +++---
>>   drivers/gpu/drm/xe/xe_pat.h | 3 ++-
>>   2 files changed, 5 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/xe/xe_pat.c b/drivers/gpu/drm/xe/xe_pat.c index
>> d239693b803b..1189d7255c12 100644
>> --- a/drivers/gpu/drm/xe/xe_pat.c
>> +++ b/drivers/gpu/drm/xe/xe_pat.c
>> @@ -303,7 +303,7 @@ bool xe_pat_index_is_valid(struct xe_device *xe, u16
>> pat_index)
>>
>>   u16 xe_pat_index_get_coh_mode(struct xe_device *xe, u16 pat_index)  {
>> -	WARN_ON(pat_index >= xe->pat.n_entries);
>> +	WARN_ON(!xe_pat_index_is_valid(xe, pat_index));
> 
> Multiple WARN_ON!
> Should we change API:
> bool xe_pat_index_is_valid(struct xe_device *xe, u16 pat_index)
> {
>      if (WARN_ON(pat_index >= xe->pat.n_entries))
>          return false;
>      return xe->pat.table[pat_index].valid;
> }

Yeah, good point.

> 
> Tejas
> 
>>   	WARN_ON(!xe->pat.table[pat_index].coh_mode ||
>>   		xe->pat.table[pat_index].coh_mode > XE_COH_2WAY);
>>   	return xe->pat.table[pat_index].coh_mode;
>> @@ -311,13 +311,13 @@ u16 xe_pat_index_get_coh_mode(struct xe_device
>> *xe, u16 pat_index)
>>
>>   bool xe_pat_index_get_comp_en(struct xe_device *xe, u16 pat_index)  {
>> -	WARN_ON(pat_index >= xe->pat.n_entries);
>> +	WARN_ON(!xe_pat_index_is_valid(xe, pat_index));
>>   	return !!(xe->pat.table[pat_index].value & XE2_COMP_EN);  }
>>
>>   u16 xe_pat_index_get_l3_policy(struct xe_device *xe, u16 pat_index)  {
>> -	WARN_ON(pat_index >= xe->pat.n_entries);
>> +	WARN_ON(!xe_pat_index_is_valid(xe, pat_index));
>>
>>   	return REG_FIELD_GET(XE2_L3_POLICY, xe-
>>> pat.table[pat_index].value);  } diff --git a/drivers/gpu/drm/xe/xe_pat.h
>> b/drivers/gpu/drm/xe/xe_pat.h index f6b0fac2000a..de4d57ef9a4e 100644
>> --- a/drivers/gpu/drm/xe/xe_pat.h
>> +++ b/drivers/gpu/drm/xe/xe_pat.h
>> @@ -75,7 +75,7 @@ u16 xe_pat_index_get_coh_mode(struct xe_device *xe,
>> u16 pat_index);
>>
>>   /**
>>    * xe_pat_index_get_comp_en - Extract the compression enable flag for
>> - * the given pat_index.
>> + * the given pat_index. Index must be valid for the hw.
>>    * @xe: xe device
>>    * @pat_index: The pat_index to query
>>    *
>> @@ -88,6 +88,7 @@ bool xe_pat_index_get_comp_en(struct xe_device *xe,
>> u16 pat_index);
>>   #define XE_L3_POLICY_UC		3 /* Uncached */
>>   /**
>>    * xe_pat_index_get_l3_policy - Extract the L3 policy for the given pat_index.
>> + * Index must be valid for the hw.
>>    * @xe: xe device
>>    * @pat_index: The pat_index to query
>>    */
>> --
>> 2.56.0
>