Re: [PATCH v4 02/32] drm/xe/log: Add structured SIGID error logging infrastructure

On 8/14/2026 3:32 PM, Nilawar, Badal wrote:
> Adding a few more review comments.
> 
> On 13-08-2026 00:44, Michal Wajdeczko wrote:
>> From: Mallesh Koujalagi <[email protected]>
>>

...

>> +
>> +static void log_emit_cper(struct pci_dev *pdev, int cper_sev, enum xe_sigid sigid,
>> +			  u32 component, u32 location, const void *data, size_t len,
>> +			  struct va_format *vaf)
>> +{
>> +	/* TODO */
> 
> Just for information, CPER log construction for hardware errors involves a system controller mailbox interaction, with the error_class passed via the data payload.
> 
>> +}
>> +
>> +static bool is_hw_sigid(enum xe_sigid sigid)
>> +{
>> +	return (int)sigid >= INTEL_SIGID_GPU_XE_HARDWARE_START;
>> +}
>> +
>> +static bool is_sev_error(int cper_sev)
>> +{
>> +	return cper_sev != CPER_SEV_INFORMATIONAL;
>> +}
>> +
>> +static const char *log_hwe_prefix(int cper_sev, enum xe_sigid sigid)
>> +{
>> +	return is_sev_error(cper_sev) && is_hw_sigid(sigid) ? HW_ERR : "";
>> +}
>> +
>> +static const char *log_sev_prefix(int cper_sev)
>> +{
>> +	switch (cper_sev) {
>> +	case CPER_SEV_FATAL:
>> +		return "FATAL ";
>> +	case CPER_SEV_RECOVERABLE:
>> +		return "";
> 
> Can we print "RECOVERABLE" here? In my earlier comment I mistakenly used "CORRECTABLE"; I meant "RECOVERABLE".

but this is to some extend redundant information:
if it isn't a FATAL or CORRECTED, then it must be RECOVERABLE, no?

the same as we don't add "INFORMATIONAL" since dmesg
is already using KERN_INFO level

> 
> Thanks,
> Badal
> 
>> +	case CPER_SEV_CORRECTED:
>> +		return "CORRECTED ";
>> +	case CPER_SEV_INFORMATIONAL:
>> +		return "";
>> +	default:
>> +		WARN(IS_ENABLED(CONFIG_DRM_XE_DEBUG), "LOG: unknown severity %d\n", cper_sev);
>> +		return "";
>> +	}
>> +}
>> +