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

["Nilawar, Badal" <badal.nilawar@intel.com>]

On 14-08-2026 00:30, Rodrigo Vivi wrote:
> On Thu, Aug 13, 2026 at 07:12:41PM +0530, Nilawar, Badal wrote:
>> Hi Michal,
>>
>> Couple of comments, on rate-limiting and XE_SIGID_WEDGED, from v3 https://patchwork.freedesktop.org/patch/743308/?series=171022&rev=3#comment_1373861
>>
>> are not answered/addressed.
> Hi Badal, could you please be more specific about the questions and repeat them here?
> I couldn't find from the provided url...
>
> But as we chatted, this series doesn't change the behavior of that and
> the comment of rate-limit is exactly to ensure this is documented. To
> ensure it is clear dmesg is not an ABI. It should not be reliable in
> any way or shape. It is simpler a helper.
Ok.
>
> On the wedge, the resolution buckets were removed, but I'm not sure
> if there were more questions around that.

My concern is about statement "Pick per *report site*, not per 
incident", XE_SIGID_WEDGED is not site specific SIGID. It can be 
reported from multiple sites.

>
>>> + * Examples::
>>> + *
>>> + *   <3> xe 0000:03:00.0: [drm] *ERROR* SIGID=104 FATAL (-EPROTO) Invalid GuC reply
>>> + *   <3> xe 0000:03:00.0: [drm] *ERROR* SIGID=106 (-ETIMEDOUT) Engine 'rcs0' hung
>>> + *   <6> xe 0000:03:00.0: [drm] SIGID=103 In survivability mode
>>> + */
>>> +void xe_log_emit(struct pci_dev *pdev, int cper_sev, enum xe_sigid sigid,
>>> +		 u32 component, u32 location, const void *data, size_t len,
>>> +		 const char *fmt, ...)
>>> +{
>>> +	struct va_format vaf;
>>> +	va_list args;
>>> +
>>> +	va_start(args, fmt);
>>> +	vaf.fmt = fmt;
>>> +	vaf.va = &args;
>>> +
>>> +	log_emit_dmesg(pdev, cper_sev, sigid, component, location, data, len, &vaf);
>>> +	log_emit_cper(pdev, cper_sev, sigid, component, location, data, len, &vaf);
>>> +
>>  From kunit example I got this output for hardware errors.
>> drm-kunit-mock-device demo_dmesg.drm-kunit-mock-device: [drm] *ERROR*
>> SIGID=204 (0102030405060708090a0b0c) [Hardware Error]: testing HARDWARE
>> signature
>> drm-kunit-mock-device demo_dmesg.drm-kunit-mock-device: [drm] *ERROR*
>> SIGID=202 CORRECTED (0102030405060708090a0b0c) [Hardware Error]: Tile1:
>> testing HARDWARE signature
>>
>> SIGIDs 202 and 204 correspond to the XE_RAS_COMP_DEVICE_MEMORY and
>> XE_RAS_COMP_FABRIC components returned by firmware via xe_ras_error_class.
>> If we want the component name to be included in the error message, what
>> should be passed to the logging helper? The current KUnit test uses
>> XE_LOG_COMPONENT_NONE, so no component information is being emitted.
> Our design goal here was to minimize the SIGIDs and make it per site
> due to maintenability concerns.
>
> There will be a field in the actual CPER with location and that can
> also be printed in the msg if needed. In a matter of fact, this
> is what we currently do anyway.
>
> Increasing this to a per case id is a big no!

Agreed.

Thanks,
Badal

>
> Thanks,
> Rodrigo.
>
>> Thanks,
>> Badal
>>
>>> +	va_end(args);
>>> +}
>>> diff --git a/drivers/gpu/drm/xe/xe_log.h b/drivers/gpu/drm/xe/xe_log.h
>>> new file mode 100644
>>> index 000000000000..d475e816ee0b
>>> --- /dev/null
>>> +++ b/drivers/gpu/drm/xe/xe_log.h
>>> @@ -0,0 +1,20 @@
>>> +/* SPDX-License-Identifier: MIT */
>>> +/*
>>> + * Copyright © 2026 Intel Corporation
>>> + */
>>> +
>>> +#ifndef _XE_LOG_H_
>>> +#define _XE_LOG_H_
>>> +
>>> +#include <linux/cper.h>
>>> +
>>> +#include "abi/xe_sigid_abi.h"
>>> +
>>> +struct pci_dev;
>>> +
>>> +__printf(8, 9)
>>> +void xe_log_emit(struct pci_dev *pdev, int cper_sev, enum xe_sigid sigid,
>>> +		 u32 component, u32 location, const void *data, size_t len,
>>> +		 const char *fmt, ...);
>>> +
>>> +#endif