Re: [PATCH v3 11/23] drm/xe/log: Extend components list with hardware items

On 8/12/2026 8:55 AM, Tauro, Riana wrote:
> 
> On 04-08-2026 11:35, Mallesh, Koujalagi wrote:
>>
>> On 30-07-2026 08:51 pm, Michal Wajdeczko wrote:
>>> Update our super macro with new top level hardware components.
>>>
>>> Signed-off-by: Michal Wajdeczko <[email protected]>
>> LGTM,
>> Reviewed-by: Mallesh Koujalagi <[email protected]>
>>> Cc: Rodrigo Vivi <[email protected]>
>>> Cc: Aravind Iddamsetty <[email protected]>
>>> ---
>>> v2: follow enum xe_ras_component values (Aravind)
>>> ---
>>>   drivers/gpu/drm/xe/abi/xe_log_abi.h | 12 ++++++++++++
>>>   1 file changed, 12 insertions(+)
>>>
>>> diff --git a/drivers/gpu/drm/xe/abi/xe_log_abi.h b/drivers/gpu/drm/xe/abi/xe_log_abi.h
>>> index 4861a5b58c10..b3373d4b29d5 100644
>>> --- a/drivers/gpu/drm/xe/abi/xe_log_abi.h
>>> +++ b/drivers/gpu/drm/xe/abi/xe_log_abi.h
>>> @@ -129,6 +129,10 @@ enum xe_log_location_bits {
>>>    *     @SIGID is the default xe_sigid for the component (without the XE_SIGID_ prefix)
>>>    */
>>>   #define DEFINE_XE_LOG_COMPONENTS(define) \
>>> +    DEFINE_XE_LOG_SOFTWARE_COMPONENTS(define) \
>>> +    DEFINE_XE_LOG_HARDWARE_COMPONENTS(define)
>>> +
>>> +#define DEFINE_XE_LOG_SOFTWARE_COMPONENTS(define) \
>>>       /* */                                    \
>>>       define(SYSTEM, 1, PCI, SW, "Linux PCI Subsystem")            \
>>>       define(SYSTEM, 2, DRM, SW, "DRM")                    \
>>> @@ -156,6 +160,14 @@ enum xe_log_location_bits {
>>>       define(DRIVER_FIRMWARE, 3, GSC, RUNTIME_FW, "GSC")            \
>>>       define(DRIVER_FIRMWARE, 16, PCODE, DEVICE_FW, "PCode")            \
>>>       define(DRIVER_FIRMWARE, 17, SYSCTRL, DEVICE_FW, "System Controller")    \
>>> +
>>> +#define DEFINE_XE_LOG_HARDWARE_COMPONENTS(define) \
>>> +    define(HARDWARE, 1, DEVMEM, DEVICE_MEMORY, "Device Memory")        \
> 
> DEVICE_MEMORY

do you mean s/DEVMEM/DEVICE_MEMORY ?

> 
>>> +    define(HARDWARE, 2, HWCORE, CORE_COMPUTE, "Core Compute")        \
> 
> Core_compute

and here s/HWCORE/CORE_COMPUTE ?

> 
>>> +    /*     HARDWARE, 3, RESERVED */                        \
>>> +    define(HARDWARE, 4, PCIE, PCIE, "PCIe Interface")            \
>>> +    define(HARDWARE, 5, FABRIC, FABRIC, "Fabric")                \
>>> +    define(HARDWARE, 6, SOC, SOC_INTERNAL, "SoC Internal")            \
> SOC_INTERNAL

and here s/SOC/SOC_INTERNAL ?

(btw, is there a plan for SOC_EXTERNAL ?
if no, why do we want to keep _INTERNAL suffix ?
> 
> Let's retain the names same as UAPI to avoid unnecessary confusion while collecting CPER logs.

but those listed here are our local component names
and just today there is 1:1 relation between those
HW components and HW SIGIDs, but maybe one day there
will be more components that would map into existing
SIGIDs (compare COMP_GUC/HUC --> SIGID_RUNTIME_FW)

the SIGID enum names are intact
	XE_SIGID_DEVICE_MEMORY
	XE_SIGID_SOC_INTERNAL
	..
and btw I'm not sure that we ever convert SIGID enum values
(202, 205) into any user facing friendly names

and the idea for having a shorter component names was that
we are doing dmesg decoration and print friendly component
and/or location name (except SIGID, which is a raw integer)

	[drm] ERROR SIGID=104 (-EPROTO) Tile0: GT0: GUC: blah
	[drm] ERROR SIGID=202 (123456789012) Tile0: DEVMEM: blah
	[drm] ERROR SIGID=205 (123456789012) Tile1: SOC: blah

having those lengthy names does not look nice:

	[drm] ERROR SIGID=202 (123456789012) Tile0: DEVICE_MEMORY: blah
	[drm] ERROR SIGID=205 (123456789012) Tile1: SOC_INTERNAL: blah

besides, in dmesg only "SIGID=%u" chunk is part of our contract

in CPER, all SIGID and component and location will be stored
as plain numbers and how those numbers will be decoded is beyond us

btw, maybe component decoration should be before location?

	[drm] ERROR SIGID=104 (-EPROTO) GUC: Tile0: GT0: blah
	[drm] ERROR SIGID=202 (123456789012) DEVICE_MEMORY: Tile1: blah
	[drm] ERROR SIGID=205 (123456789012) SOC_INTERNAL: Tile1: blah

(see patch 4/23)

> 
> These names were asked to be retained similar to actual hardware error classification even for uapi.
> 
> ++@Aravind
> 
> Thanks
> Riana
> 
> 
>>>       /* eod */
>>>     /**