Re: [PATCH v3 18/23] drm/xe: Report 'Survivability Mode' errors using SIGID

["Mallesh, Koujalagi" <mallesh.koujalagi@intel.com>]

[-- Attachment #1: Type: text/plain, Size: 6968 bytes --]


On 07-08-2026 05:44 pm, Michal Wajdeczko wrote:
>
> On 8/7/2026 1:18 PM, Mallesh, Koujalagi wrote:
>> On 30-07-2026 08:51 pm, Michal Wajdeczko wrote:
>>> Report various 'Survivability Mode' errors using xe_log() helpers.
>>>
>>> Signed-off-by: Michal Wajdeczko<michal.wajdeczko@intel.com>
>>> Cc: Rodrigo Vivi<rodrigo.vivi@intel.com>
>>> Cc: Riana Tauro<riana.tauro@intel.com>
>>> Cc: Aravind Iddamsetty<aravind.iddamsetty@intel.com>
>>> Cc: Mallesh Koujalagi<mallesh.koujalagi@intel.com>
>>> ---
>>>   drivers/gpu/drm/xe/xe_survivability_mode.c | 24 +++++++++++++---------
>>>   1 file changed, 14 insertions(+), 10 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/xe/xe_survivability_mode.c b/drivers/gpu/drm/xe/xe_survivability_mode.c
>>> index 4c506027fa94..788b7e8137a9 100644
>>> --- a/drivers/gpu/drm/xe/xe_survivability_mode.c
>>> +++ b/drivers/gpu/drm/xe/xe_survivability_mode.c
>>> @@ -14,9 +14,11 @@
>>>   #include "xe_device.h"
>>>   #include "xe_heci_gsc.h"
>>>   #include "xe_i2c.h"
>>> +#include "xe_log.h"
>>>   #include "xe_mmio.h"
>>>   #include "xe_nvm.h"
>>>   #include "xe_pcode_api.h"
>>> +#include "xe_printk.h"
>>>   #include "xe_vsec.h"
>>>   
>>>   /**
>>> @@ -179,11 +181,11 @@ static void log_survivability_info(struct pci_dev *pdev)
>>>   	u32 *info = survivability->info;
>>>   	int id;
>>>   
>>> -	dev_info(&pdev->dev, "Survivability Boot Status : Critical Failure (%d)\n",
>>> -		 survivability->boot_status);
>>> +	xe_log_info(xe, SURVIVABILITY, "Boot Status : Critical Failure (%d)\n",
>>> +		    survivability->boot_status);
> btw, is it OK that we use INFO level for "critical failure" ?

Good catch! We log the message when the device has a critical boot 
failure and the survivability is too old to handle if (version < 2)

so the driver is about to abort with -ENXIO. Using xe_log_info for that 
situation is wrong one. Using

xe_log_err_fatal(xe, SURVIVABILITY, -ENXIO, ...); we can fix it.

>
>>>   	for (id = 0; id < MAX_SCRATCH_REG; id++) {
>>>   		if (info[id])
>>> -			dev_info(&pdev->dev, "%s: 0x%x\n", reg_map[id], info[id]);
>>> +			xe_log_info(xe, SURVIVABILITY, "%s: 0x%x\n", reg_map[id], info[id]);
>>>   	}
>>>   }
>>>   
>>> @@ -316,7 +318,6 @@ static int create_survivability_sysfs(struct pci_dev *pdev)
>>>   
>>>   static int enable_boot_survivability_mode(struct pci_dev *pdev)
>>>   {
>>> -	struct device *dev = &pdev->dev;
>>>   	struct xe_device *xe = pdev_to_xe_device(pdev);
>>>   	struct xe_survivability *survivability = &xe->survivability;
>>>   	int ret = 0;
>>> @@ -342,12 +343,12 @@ static int enable_boot_survivability_mode(struct pci_dev *pdev)
>>>   	if (ret)
>>>   		goto err;
>>>   
>>> -	dev_err(dev, "In Survivability Mode\n");
>>> -
>>> +	xe_log_emit(pdev, check_boot_failure(xe) ? CPER_SEV_FATAL : CPER_SEV_INFORMATIONAL,
>>> +		    XE_SIGID_SURVIVABILITY, 0, 0, 0, 0, "In Survivability Boot Mode\n");
>> Please make it cleaner and simpler.
> sure
>
> it was one of the earliest examples of the new xe_log API,
> and that's why it was using the base xe_log function
>> if(check_boot_failure(xe))
>>
>>      xe_log_err_fatal(xe, SURVIVABILITY, .. );
>>
>> else
>>
>>       xe_log_info(xe, SURVIVABILITY, .. );
>>
>>
>> OR
>>
>> xe_log_emit(xe_any_to_pdev(xe),
> we do have pdev already, no need to cast back to xe
Agreed!
>
>>              check_boot_failure(xe) ? CPER_SEV_FATAL : CPER_SEV_INFORMATIONAL,
>>              XE_SIGID_SURVIVABILITY, XE_LOG_COMPONENT_SURVIVABILITY,
>>              xe_log_location(xe),
>>              &survivability->boot_status, sizeof(survivability->boot_status),
> cool, but isn't this already printed in log_survivability_info() ?
it's not print both, either it print log_survivability_info() (based on 
condition and return) or enable_boot_survivability_mode.
>
>>              "In Survivability Boot Mode\n");
> btw, as we use SURVIVABILITY component, the dmesg will already
> have "SURVIVABILITY: " decoration, so maybe this msg should be:
>
> 	"Boot mode enabled!\n"
>
> with dmesg:
>
> 	<3> [drm] ERROR SIGID=103 FATAL (04) SURVIVABILITY: Boot mode enabled!
> or
> 	<6> [drm] SIGID=103 SURVIVABILITY: Boot mode enabled!
Agreed!
>
>>>   	return 0;
>>>   
>>>   err:
>>> -	dev_err(dev, "Failed to enable Survivability Mode\n");
>>> +	xe_log_err_fatal(xe, SURVIVABILITY, ret, "Failed to enable Survivability Mode\n");
> and here:
>
> 	"Failed to enter Boot mode!\n"
>
> with dmesg:
>
> 	<3> [drm] ERROR SIGID=103 FATAL (-ENOMEM) SURVIVABILITY: Failed to enter Boot mode!
>
Make sense.
>>>   	survivability->mode = false;
>>>   	return ret;
>>>   }
>>> @@ -412,7 +413,7 @@ void xe_survivability_mode_runtime_enable(struct xe_device *xe)
>>>   	struct pci_dev *pdev = to_pci_dev(xe->drm.dev);
>>>   
>>>   	if (!IS_DGFX(xe) || IS_SRIOV_VF(xe) || xe->info.platform < XE_BATTLEMAGE) {
>>> -		dev_err(&pdev->dev, "Runtime Survivability Mode not supported\n");
>>> +		xe_log_info(xe, SURVIVABILITY, "Runtime Mode not supported!\n");
>> We can add xe_log_err(xe, SURVIVABILITY, -EOPNOTSUPP, ...);
> hmm, actually I was wondering if this dev_err() was correct
> maybe it should be just xe_dbg() as we are not doing anything
> related to SURVIVABILITY ?

That function is called in runtime survivability, so debugger will get 
the context easily and figure it out

what cause it.

>
>>>   		return;
>>>   	}
>>>   
>>> @@ -422,11 +423,14 @@ void xe_survivability_mode_runtime_enable(struct xe_device *xe)
>>>   		dev_err(&pdev->dev, "Failed to create survivability sysfs\n");
>> need to use xe_log_err(xe, SURVIVABILITY. -EIO, ... );
>>>   
>>>   	survivability->type = XE_SURVIVABILITY_TYPE_RUNTIME;
>>> -	dev_err(&pdev->dev, "Runtime Survivability mode enabled\n");
>>> +	xe_log_err_fatal(xe, SURVIVABILITY, 0, "Runtime Mode enabled!\n");
>> hmm, Logging error as fatal, however passing err=0 (Success). is it right? or simply we can log as xe_log_err or xe_log_info ? any thoughts.
Already I checked with Arch team, better to provide as xe_log_info 
rather than fatal. Just indicate to user it's Runtime survivability mode 
in such case.
> passing 0 instead of errno to xe_log_err() helpers will just omit
> printing anything in ( ), no %pe nor %phN
>
> whether this should be info/fatal/recoverable it's not me to answer
> your initial documentation [1] was saying that all XE_SIG_SURVIVABILITY
> should have CPER_SEV_FATAL
>
> [1]https://patchwork.freedesktop.org/patch/732271/?series=168333&rev=1
>
>>>   
>>>   	xe_device_set_wedged_method(xe, DRM_WEDGE_RECOVERY_VENDOR);
>>>   	xe_device_declare_wedged(xe);
>>> -	dev_err(&pdev->dev, "Firmware flash required, Please refer to the userspace documentation for more details!\n");
>>> +
>>> +	xe_log_err_fatal(xe, SURVIVABILITY, 0, "Firmware flash required!\n");
>> ditto
> ditto ;)
ditto ;)
>>> +	xe_info(xe, "Please refer to the userspace documentation for more details how to flash the firmware on %s!\n",
>>> +		xe->info.platform_name);
>>>   }
>>>   
>>>   /**

[-- Attachment #2: Type: text/html, Size: 11877 bytes --]