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

[Michal Wajdeczko <michal.wajdeczko@intel.com>]

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" ?

>>  	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

>             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() ?

>             "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!

> 
>>  	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!


>>  	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 ?

>>  		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.

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 ;)

>> +	xe_info(xe, "Please refer to the userspace documentation for more details how to flash the firmware on %s!\n",
>> +		xe->info.platform_name);
>>  }
>>  
>>  /**