Re: [PATCH v3 08/13] drm/xe/pat: Return error from xe_pat_init_early() on unknown platform

[Dnyaneshwar Bhadane <dnyaneshwar.bhadane@Intel.com>]

On 09-Sep-26 12:07 AM, Michal Wajdeczko wrote:
> 
> 
> On 9/8/2026 12:45 PM, Dnyaneshwar Bhadane wrote:
>> xe_pat_init_early() currently only logs an error when the platform's
>> graphics version doesn't match any known PAT table, but continues
>> probing with an uninitialized pat.ops. This can lead to a NULL
>> pointer dereference later when pat.ops is used.
>>
>> Change xe_pat_init_early() to return an int and propagate -ENODATA
>> to xe_device_probe() so the driver fails to load cleanly instead of
>> continuing in a broken state.
>>
> 
> Fixes: ??
Hi, For the missing pat we should not process the further as we need 
proper IP specific xe->pat.ops assigned before moving from this 
function. so early exit from the driver probe would limit the pat->ops 
null pointer in xe_pat_init access.?
> 
>> Signed-off-by: Dnyaneshwar Bhadane <dnyaneshwar.bhadane@intel.com>
>> ---
>>   drivers/gpu/drm/xe/xe_device.c |  4 +++-
>>   drivers/gpu/drm/xe/xe_pat.c    | 14 +++++---------
>>   drivers/gpu/drm/xe/xe_pat.h    |  2 +-
>>   3 files changed, 9 insertions(+), 11 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/xe/xe_device.c b/drivers/gpu/drm/xe/xe_device.c
>> index 8583b2e9ecf4..b7f78d45dee1 100644
>> --- a/drivers/gpu/drm/xe/xe_device.c
>> +++ b/drivers/gpu/drm/xe/xe_device.c
>> @@ -960,7 +960,9 @@ int xe_device_probe(struct xe_device *xe)
>>   	int err;
>>   	u8 id;
>>   
>> -	xe_pat_init_early(xe);
>> +	err = xe_pat_init_early(xe);
>> +	if (err)
>> +		return err;
>>   
>>   	err = xe_sriov_init(xe);
>>   	if (err)
>> diff --git a/drivers/gpu/drm/xe/xe_pat.c b/drivers/gpu/drm/xe/xe_pat.c
>> index a5fe1beec652..8af92e3c4c17 100644
>> --- a/drivers/gpu/drm/xe/xe_pat.c
>> +++ b/drivers/gpu/drm/xe/xe_pat.c
>> @@ -590,7 +590,7 @@ static const struct xe_pat_ops xe3p_xpc_pat_ops = {
>>   	.entry_dump = xe3p_xpc_pat_entry_dump,
>>   };
>>   
> 
> now it's time to add kernel-doc
Yes, I will add this in document.>> -void xe_pat_init_early(struct 
xe_device *xe)
>> +int xe_pat_init_early(struct xe_device *xe)
>>   {
>>   	xe->pat.idx[XE_CACHE_WB_COMPRESSION] = XE_PAT_INVALID_IDX;
>>   	xe->pat.idx[XE_CACHE_NONE_COMPRESSION] = XE_PAT_INVALID_IDX;
>> @@ -678,22 +678,18 @@ void xe_pat_init_early(struct xe_device *xe)
>>   		xe->pat.idx[XE_CACHE_WT] = 2;
>>   		xe->pat.idx[XE_CACHE_WB] = 0;
>>   	} else {
>> -		/*
>> -		 * Going forward we expect to need new PAT settings for most
>> -		 * new platforms; failure to provide a new table can easily
>> -		 * lead to subtle, hard-to-debug problems.  If none of the
>> -		 * conditions above match the platform we're running on we'll
>> -		 * raise an error rather than trying to silently inherit the
>> -		 * most recent platform's behavior.
>> -		 */
> 
> can't we leave this comment as-is?
Noted. Thank you Michal

Dnyaneshwar, >
>>   		drm_err(&xe->drm, "Missing PAT table for platform with graphics version %d.%02d!\n",
>>   			GRAPHICS_VER(xe), GRAPHICS_VERx100(xe) % 100);
>> +		return -ENODATA;
>> +
>>   	}
>>   
>> +	xe_assert(xe, xe->pat.ops);
>>   	xe_assert(xe, xe->pat.ops->dump);
>>   	xe_assert(xe, xe->pat.ops->program_graphics);
>>   	xe_assert(xe, MEDIA_VER(xe) < 13 || xe->pat.ops->program_media);
>>   	xe_assert(xe, GRAPHICS_VER(xe) < 20 || xe->pat.ops->entry_dump);
>> +	return 0;
>>   }
>>   
>>   void xe_pat_init(struct xe_gt *gt)
>> diff --git a/drivers/gpu/drm/xe/xe_pat.h b/drivers/gpu/drm/xe/xe_pat.h
>> index 7060f66e1d63..66e335fb3b0c 100644
>> --- a/drivers/gpu/drm/xe/xe_pat.h
>> +++ b/drivers/gpu/drm/xe/xe_pat.h
>> @@ -43,7 +43,7 @@ struct xe_pat_table_entry {
>>    * xe_pat_init_early - SW initialization, setting up data based on device
>>    * @xe: xe device
>>    */
>> -void xe_pat_init_early(struct xe_device *xe);
>> +int xe_pat_init_early(struct xe_device *xe);
>>   
>>   /**
>>    * xe_pat_init - Program HW PAT table
>