Re: [PATCH 6/6] drm/xe/log: Relax location ID recognition

[Michal Wajdeczko <michal.wajdeczko@intel.com>]

On 9/9/2026 11:00 AM, Mallesh, Koujalagi wrote:
> 
> On 04-09-2026 10:35 pm, Michal Wajdeczko wrote:
>> It turned out that during early probe phase, VFs use detached from
>> the xe_device, temporary xe_gt objects, which when used as location
>> in xe_log() macros, will be treated by the dmesg decoration code as
>> bogus, possibly triggering a WARN, and the output will look like:
>>
>>  [drm] *ERROR* SIGID=104 (-ETIMEDOUT) LOC3.0? GUC: MMIO request ...
                                         ^^^^^^^
[here] ----------------------------------^^^^^^^

>>
>> instead of expected:
>>
>>  [drm] *ERROR* SIGID=104 (-ETIMEDOUT) Tile0: GT0: GUC: MMIO request ...
>>
>> Relax the tile/GT id validation and instead of looking for the real
>> objects, only check if encoded id is within the range of possible
>> tiles or GTs on the current xe device, using data from the device
>> descriptor rather then the object list.
> 
> nit: 'rather than'
> 
> Please mention invalid id case, how it's going to render like 
> 
> Warn + Tile3?  GT9? now.

invalid ID it is shown [here] and in the cover letter

regarding a WARN, it will not show up in production builds,
but it would look like:

 [ ] xe 0000:00:02.1: LOG: unrecognized location 3.0
 [ ] WARNING: drivers/gpu/drm/xe/xe_log.c:97 at __xe_log_emit+0x3ba/0xaa0 [xe], CPU#9: sh/5268
 [ ] Tainted: [W]=WARN
 [ ] RIP: 0010:__xe_log_emit+0x3ce/0xaa0 [xe]
 [ ] Call Trace:
 [ ]  <TASK>
 [ ]  ? mark_held_locks+0x46/0x90
 [ ]  ? __xe_mmio_wait32+0x87/0x170 [xe]
 [ ]  xe_guc_mmio_send_recv+0x354/0xc10 [xe]
 [ ]  ? dev_printk_emit+0xa0/0xe0
 [ ]  xe_guc_mmio_send+0x10/0x20 [xe]
 [ ]  vf_reset_guc_state+0x47/0x100 [xe]
 [ ]  ? rcu_read_unlock+0x1c/0x80
 [ ]  xe_gt_sriov_vf_bootstrap+0x2f/0xcb0 [xe]
 [ ]  ? xe_gt_mmio_init+0x30/0x1e0 [xe]
 [ ]  ? __drm_dev_dbg+0xa9/0xe0
 [ ]  ? __kmalloc_large_noprof+0x115/0x130
 [ ]  read_gmdid+0xf3/0x310 [xe]
 [ ]  ? xe_device_probe_early+0x143/0x710 [xe]
 [ ]  ? xe_pci_rebar_resize+0x17e/0x2e0 [xe]
 [ ]  ? __pci_set_master+0x61/0x100
 [ ]  xe_pci_probe+0x554/0xac0 [xe]
 [ ]  ? mark_held_locks+0x46/0x90
 [ ]  ? __pm_runtime_resume+0x5b/0x90
 [ ]  ? _raw_spin_unlock_irqrestore+0x67/0x90


> 
>> Fixes: 1151b9f6f465 ("drm/xe/log: Add component/location decorations to dmesg")
>> Signed-off-by: Michal Wajdeczko <michal.wajdeczko@intel.com>
>> ---
>> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
>> Cc: Mallesh Koujalagi <mallesh.koujalagi@intel.com>
>> ---
>>  drivers/gpu/drm/xe/xe_log.c | 40 ++++++++++++++++++++++++-------------
>>  1 file changed, 26 insertions(+), 14 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/xe/xe_log.c b/drivers/gpu/drm/xe/xe_log.c
>> index 5549ef6966fd..29eb16db3320 100644
>> --- a/drivers/gpu/drm/xe/xe_log.c
>> +++ b/drivers/gpu/drm/xe/xe_log.c
>> @@ -10,6 +10,7 @@
>>  
>>  #include "xe_device.h"
>>  #include "xe_log.h"
>> +#include "xe_pci_types.h"
>>  #include "xe_printk.h"
>>  
>>  static void log_emit_cper(struct pci_dev *pdev, int cper_sev, enum xe_sigid sigid,
>> @@ -52,18 +53,24 @@ static const char *log_component_prefix(u32 component)
>>  	return component ? log_unknown_component_prefix(component) : "";
>>  }
>>  
>> -static struct xe_gt *get_gt_safe(struct pci_dev *pdev, u8 id)
>> +static bool allowed_tile_id(struct xe_device *xe, u8 tile_id)
>>  {
>> -	struct xe_device *xe = pdev_to_xe_device(pdev);
>> +	return tile_id < 1 + xe->desc->max_remote_tiles;
>> +}
>>  
>> -	return xe ? xe_device_get_gt(xe, id) : NULL;
>> +static bool allowed_gt_id(struct xe_device *xe, u8 gt_id)
>> +{
>> +	return gt_id < (1 + xe->desc->max_remote_tiles) * xe->desc->max_gt_per_tile;
>>  }
>>  
>> -static struct xe_tile *get_tile_safe(struct pci_dev *pdev, u8 id)
>> +static u8 gt_id_to_tile_id(struct xe_device *xe, u8 gt_id)
>>  {
>> -	struct xe_device *xe = pdev_to_xe_device(pdev);
>> +	return gt_id / xe->desc->max_gt_per_tile;
> 
> hmm, we need to check max_gt_per_tile should not be zero to avoid hard Oops. I know in real scenario, max_gt_per_tile
> 
> value is greater than 0 and less than equal to XE_MAX_GT_PER_TILE, however rogue user can write kunit test case and manipulate it, to avoid such scenario, we need to handle it.

desc->max_gt_per_tile are const and are defined in xe_pci.c, so ordinary user can't change that

we also have a test check_platform_desc [2] to make sure no developer will define max_gt_per_tile as 0

and we can't really control rouge user that will prepare and use invalid descriptor for kunit,
but even then, it will crash just that kunit, so who cares?

[2] https://elixir.bootlin.com/linux/v7.3-rc1/source/drivers/gpu/drm/xe/tests/xe_pci_test.c#L63

> 
> Thanks,
> 
> -/Mallesh
> 
>> +}
>>  
>> -	return xe && id < xe->info.tile_count ? &xe->tiles[id] : NULL;
>> +static const char *location_suffix(bool valid)
>> +{
>> +	return valid ? ":" : "?";
>>  }
>>  
>>  static const char *log_location_prefix(struct pci_dev *pdev, u32 location, char *buf, size_t size)
>> @@ -76,17 +83,22 @@ static const char *log_location_prefix(struct pci_dev *pdev, u32 location, char
>>  			goto unrecognized;
>>  		strscpy(buf, "", size);
>>  	} else if (type == XE_LOG_LOCATION_TYPE_TILE) {
>> -		struct xe_tile *tile = get_tile_safe(pdev, id);
>> +		struct xe_device *xe = xe_any_to_xe(pdev);
>> +		bool valid = xe ? allowed_tile_id(xe, id) : false;
>> +		const char *pad = location_suffix(valid);
>>  
>> -		if (!tile)
>> -			goto unrecognized;
>> -		snprintf(buf, size, "Tile%u: ", id);
>> +		pci_WARN(pdev, !valid && IS_ENABLED(CONFIG_DRM_XE_DEBUG),
>> +			 "LOG: invalid tile identifier: %u\n", id);
>> +		snprintf(buf, size, "Tile%u%s ", id, pad);
>>  	} else if (type == XE_LOG_LOCATION_TYPE_GT) {
>> -		struct xe_gt *gt = get_gt_safe(pdev, id);
>> +		struct xe_device *xe = xe_any_to_xe(pdev);
>> +		bool valid = xe ? allowed_gt_id(xe, id) : false;
>> +		const char *pad = location_suffix(valid);
>> +		u8 tile_id = xe ? gt_id_to_tile_id(xe, id) : 0;
>>  
>> -		if (!gt)
>> -			goto unrecognized;
>> -		snprintf(buf, size, "Tile%u: GT%u: ", gt->tile->id, id);
>> +		pci_WARN(pdev, !valid && IS_ENABLED(CONFIG_DRM_XE_DEBUG),
>> +			 "LOG: invalid GT identifier: %u\n", id);
>> +		snprintf(buf, size, "Tile%u%s GT%u%s ", tile_id, pad, id, pad);
>>  	} else {
>>  		goto unrecognized;
>>  	}