Re: [PATCH 10/19] drm/xe/log: Extend components list with hardware items

On 7/28/2026 4:47 PM, Aravind Iddamsetty wrote:
> 
> On 24-07-2026 20:41, Michal Wajdeczko wrote:
>> On 7/24/2026 1:49 PM, Aravind Iddamsetty wrote:
>>> On 24-07-2026 02:07, Michal Wajdeczko wrote:
>>>> Update our super macro with new top level hardware components.
>>>>
>>>> Signed-off-by: Michal Wajdeczko <[email protected]>
>>>> Cc: Rodrigo Vivi <[email protected]>
>>>> ---
>>>>  drivers/gpu/drm/xe/abi/xe_log_abi.h | 11 +++++++++++
>>>>  1 file changed, 11 insertions(+)
>>>>
>>>> diff --git a/drivers/gpu/drm/xe/abi/xe_log_abi.h b/drivers/gpu/drm/xe/abi/xe_log_abi.h
>>>> index cde86c6c08f2..edf4dc13377d 100644
>>>> --- a/drivers/gpu/drm/xe/abi/xe_log_abi.h
>>>> +++ b/drivers/gpu/drm/xe/abi/xe_log_abi.h
>>>> @@ -124,6 +124,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")					\
>>>> @@ -151,6 +155,13 @@ 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, PCIE, PCIE, "PCIe Interface")			\
>>>> +	define(HARDWARE, 2, HWCORE, CORE_COMPUTE, "Core Compute")		\
>>>> +	define(HARDWARE, 3, DEVMEM, DEVICE_MEMORY, "Device Memory")		\
>>>> +	define(HARDWARE, 4, FABRIC, FABRIC, "Fabric")				\
>>>> +	define(HARDWARE, 5, SOC, SOC_INTERNAL, "SoC Internal")			\
>>>>  	/* eod */
>>>>  
>>> For HW errors FW already reports the components, we already have
>>> definition of component "enum xe_ras_component" that matches how FW
>>> reports the errors we shall reuse it not redefine again.
>> from what I can see in [1] this enum was defined just as local definition and there was no mention that they are fixed HW originated values and wasn't placed in regs/ (like we do with all HW related stuff)
> 
> check this https://gitlab.com/freedesktop-mirror/drm-tip/-/blob/drm-tip/drivers/gpu/drm/xe/xe_ras.c?ref_type=heads#L379

sorry, missed your response and I've already posted v2

> 
> where error details is retrieved from FW and component details are extracted into structxe_ras_error_class <https://elixir.bootlin.com/linux/v7.2-rc4/C/ident/xe_ras_error_class>
> 
>> OTOH, these LOG components definitions are supposed to be part of the log ABI, so shall be defined in more exposed place (like our abi/ folder)
>>
>> but there is still a solution for this.
>>
>> we can define HW LOG components in this LOG abi/ as:
>>
>> +	define(HARDWARE, 1, DEVMEM, DEVICE_MEMORY, "Device Memory")		\
>> +	define(HARDWARE, 2, HWCORE, CORE_COMPUTE, "Core Compute")		\
>> +	define(HARDWARE, 4, PCIE, PCIE, "PCIe Interface")			\
>> +	define(HARDWARE, 5, FABRIC, FABRIC, "Fabric")				\
>> +	define(HARDWARE, 6, SOC, SOC_INTERNAL, "SoC Internal")			\
> 
> As the component needs to go to CPER in case of HW errors

I guess in case of SW errors we will also want/need to encode component identifier,
so we can't assume that only identifiers defined by the particular FW will be ever used

IMO the CPER/LOG identifiers are just different namespace compared to FW IDs

> it will derived from structxe_ras_error_class <https://elixir.bootlin.com/linux/v7.2-rc4/C/ident/xe_ras_error_class> given by FW which will go into CPER. We shall use whatever the FW has given.
> 

but with asserts defined in [2] you should be able to easily convert those FW provided identifiers to the LOG component identifiers (and back)
also, you still have an option to keep all these HW internals ids in the data blob that LOG infra does not rely on

Michal

[2] https://patchwork.freedesktop.org/patch/742837/?series=171022&rev=2

> Thanks,
> 
> Aravind.
> 
>> as this will include TYPE(HARDWARE) definition for each component identifier
>> so the XE_LOG_COMPONENT_SOC is still defined as 0x0608, not as a plain 6
>>
>> and have separate HW components definitions as reported by the FW in another abi/ as:
>>
>> 	SYSCTRL_COMP_DEVICE_MEMORY = 1,
>> 	SYSCTRL_COMP_CORE_COMPUTE = 2,
>> 	SYSCTRL_COMP_PCIE = 4,
>> 	SYSCTRL_COMP_FABRIC = 5,
>> 	SYSCTRL_COMP_SOC_INTERNAL = 6,
>>
>> and finally somewhere in the code we can make sure we don't change that accidentally:
>>
>> 	BUILD_BUG_ON(SYSCTRL_COMP_DEVICE_MEMORY !=
>> 		     FIELD_GET(XE_LOG_LOCATION_ID_MASK,
>> 			       XE_LOG_COMPONENT_DEVICE_MEMORY));
>> 	...
>> 	BUILD_BUG_ON(SYSCTRL_COMP_SOC_INTERNAL !=	// aka 6
>> 		     FIELD_GET(XE_LOG_LOCATION_ID_MASK,	// aka 0xFF00
>> 			       XE_LOG_COMPONENT_SOC));  // aka 0x0608
>>
>>
>> [1] https://elixir.bootlin.com/linux/v7.2-rc4/source/drivers/gpu/drm/xe/xe_ras.c#L23
>>
>>
>>> Thanks,
>>> Aravind.
>>>>  /**