Re: [RFC PATCH v4 5/5] drm/xe: Replace critical drm_err log with xe_ras_log_probe

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

On 6/30/2026 1:55 PM, Mallesh Koujalagi wrote:
> From: Dnyaneshwar Bhadane <dnyaneshwar.bhadane@intel.com>
> 
> Replace the drm_err() call with xe_ras_log_probe
> starting from the xe_pci_probe considering the only considering the
> init paths for the object such mmio, pcode, pci etc. are fall under
> the xe_ras_log_probe() category.
> 
> Signed-off-by: Dnyaneshwar Bhadane <dnyaneshwar.bhadane@intel.com>
> Signed-off-by: Mallesh Koujalagi <mallesh.koujalagi@intel.com>
> ---
>  drivers/gpu/drm/xe/xe_device.c | 10 +++++-----
>  drivers/gpu/drm/xe/xe_hwmon.c  |  4 +++-
>  drivers/gpu/drm/xe/xe_irq.c    |  5 +++--
>  drivers/gpu/drm/xe/xe_mmio.c   |  5 +++--
>  drivers/gpu/drm/xe/xe_pat.c    |  6 ++++--
>  drivers/gpu/drm/xe/xe_pci.c    | 14 +++++++++-----
>  drivers/gpu/drm/xe/xe_pcode.c  |  5 +++--
>  drivers/gpu/drm/xe/xe_vram.c   |  5 +++--
>  8 files changed, 33 insertions(+), 21 deletions(-)
> 
> diff --git a/drivers/gpu/drm/xe/xe_device.c b/drivers/gpu/drm/xe/xe_device.c
> index 7538ac7c7dfb..208dc6e378ba 100644
> --- a/drivers/gpu/drm/xe/xe_device.c
> +++ b/drivers/gpu/drm/xe/xe_device.c
> @@ -589,7 +589,7 @@ int xe_device_init_early(struct xe_device *xe)
>  		 * Cleanup done in xe_device_destroy via
>  		 * drmm_add_action_or_reset register above
>  		 */
> -		drm_err(&xe->drm, "Failed to allocate xe workqueues\n");
> +		xe_ras_log_probe(xe, -ENOMEM, "Failed to allocate xe workqueues\n");
>  		return -ENOMEM;
>  	}
>  
> @@ -649,7 +649,7 @@ static void __xe_driver_flr(struct xe_device *xe)
>  	 */
>  	ret = xe_mmio_wait32(mmio, GU_CNTL, DRIVERFLR, 0, flr_timeout, NULL, false);
>  	if (ret) {
> -		drm_err(&xe->drm, "Driver-FLR-prepare wait for ready failed! %d\n", ret);
> +		xe_ras_log_probe(xe, -ret, "Driver-FLR-prepare wait for ready failed! %d\n", ret);

why passing -ret to the log macro?
why printing errno twice ?

the __xe_driver_flr() is called from

 - xe_device_shutdown()
 - xe_driver_flr()
   - xe_driver_flr_fini
      - devm_add_action_or_reset
      - devm teardown

so this error could be seen during the probe only we fail to add new cleanup action
are you sure it should be tagged as PROBE error?

as the plan is to have consistent and stable error reporting,
maybe each promotion from regular error to RAS-class error should
be done in a separate patch to make sure it is properly reviewed?

>  		return;
>  	}
>  	xe_mmio_write32(mmio, GU_DEBUG, DRIVERFLR_STATUS);
> @@ -660,7 +660,7 @@ static void __xe_driver_flr(struct xe_device *xe)
>  	/* Wait for hardware teardown to complete */
>  	ret = xe_mmio_wait32(mmio, GU_CNTL, DRIVERFLR, 0, flr_timeout, NULL, false);
>  	if (ret) {
> -		drm_err(&xe->drm, "Driver-FLR-teardown wait completion failed! %d\n", ret);
> +		xe_ras_log_probe(xe, -ret, "Driver-FLR-teardown wait completion failed! %d\n", ret);
>  		return;
>  	}
>  
> @@ -668,7 +668,7 @@ static void __xe_driver_flr(struct xe_device *xe)
>  	ret = xe_mmio_wait32(mmio, GU_DEBUG, DRIVERFLR_STATUS, DRIVERFLR_STATUS,
>  			     flr_timeout, NULL, false);
>  	if (ret) {
> -		drm_err(&xe->drm, "Driver-FLR-reinit wait completion failed! %d\n", ret);
> +		xe_ras_log_probe(xe, -ret, "Driver-FLR-reinit wait completion failed! %d\n", ret);
>  		return;
>  	}
>  
> @@ -720,7 +720,7 @@ static int xe_set_dma_info(struct xe_device *xe)
>  	return 0;
>  
>  mask_err:
> -	drm_err(&xe->drm, "Can't set DMA mask/consistent mask (%d)\n", err);
> +	xe_ras_log_probe(xe, -err, "Can't set DMA mask/consistent mask (%d)\n", err);
>  	return err;
>  }
>  
> diff --git a/drivers/gpu/drm/xe/xe_hwmon.c b/drivers/gpu/drm/xe/xe_hwmon.c
> index de3f2aeffc3f..efca13c2a897 100644
> --- a/drivers/gpu/drm/xe/xe_hwmon.c
> +++ b/drivers/gpu/drm/xe/xe_hwmon.c
> @@ -18,6 +18,7 @@
>  #include "xe_mmio.h"
>  #include "xe_pcode.h"
>  #include "xe_pcode_api.h"
> +#include "xe_ras_log.h"
>  #include "xe_sriov.h"
>  #include "xe_pm.h"
>  #include "xe_vsec.h"
> @@ -1575,7 +1576,8 @@ int xe_hwmon_register(struct xe_device *xe)
>  								&hwmon_chip_info,
>  								hwmon_groups);
>  	if (IS_ERR(hwmon->hwmon_dev)) {
> -		drm_err(&xe->drm, "Failed to register xe hwmon (%pe)\n", hwmon->hwmon_dev);
> +		xe_ras_log_probe(xe, -PTR_ERR(hwmon->hwmon_dev),
> +				 "Failed to register xe hwmon (%pe)\n", hwmon->hwmon_dev);
>  		xe->hwmon = NULL;
>  		return PTR_ERR(hwmon->hwmon_dev);
>  	}
> diff --git a/drivers/gpu/drm/xe/xe_irq.c b/drivers/gpu/drm/xe/xe_irq.c
> index 9e49e2241da4..8453681956c4 100644
> --- a/drivers/gpu/drm/xe/xe_irq.c
> +++ b/drivers/gpu/drm/xe/xe_irq.c
> @@ -23,6 +23,7 @@
>  #include "xe_mert.h"
>  #include "xe_mmio.h"
>  #include "xe_pxp.h"
> +#include "xe_ras_log.h"
>  #include "xe_sriov.h"
>  #include "xe_sysctrl.h"
>  #include "xe_tile.h"
> @@ -822,7 +823,7 @@ int xe_irq_install(struct xe_device *xe)
>  
>  	err = pci_alloc_irq_vectors(pdev, nvec, nvec, irq_flags);
>  	if (err < 0) {
> -		drm_err(&xe->drm, "Failed to allocate IRQ vectors: %d\n", err);
> +		xe_ras_log_probe(xe, -EINVAL, "Failed to allocate IRQ vectors: %d\n", err);

there is err available, why using -EINVAL?

>  		return err;
>  	}
>  
> @@ -891,7 +892,7 @@ static int xe_irq_msix_init(struct xe_device *xe)
>  		return 0;  /* MSI */
>  
>  	if (nvec < 0) {
> -		drm_err(&xe->drm, "Failed getting MSI-X vectors count: %d\n", nvec);
> +		xe_ras_log_probe(xe, -nvec, "Failed getting MSI-X vectors count: %d\n", nvec);

you are reporting error twice

>  		return nvec;
>  	}
>  
> diff --git a/drivers/gpu/drm/xe/xe_mmio.c b/drivers/gpu/drm/xe/xe_mmio.c
> index 7fa18dfcb5a2..d10ba886dcda 100644
> --- a/drivers/gpu/drm/xe/xe_mmio.c
> +++ b/drivers/gpu/drm/xe/xe_mmio.c
> @@ -16,6 +16,7 @@
>  #include "xe_device.h"
>  #include "xe_gt_sriov_vf.h"
>  #include "xe_printk.h"
> +#include "xe_ras_log.h"
>  #include "xe_sriov.h"
>  #include "xe_tile_printk.h"
>  #include "xe_trace.h"
> @@ -105,13 +106,13 @@ int xe_mmio_probe_early(struct xe_device *xe)
>  
>  	xe->mmio.regs = pcim_iomap(pdev, GTTMMADR_BAR, 0);
>  	if (!xe->mmio.regs) {
> -		xe_err(xe, "Failed to map GTTMMADR_BAR\n");
> +		xe_ras_log_probe(xe, -EIO, "Failed to map GTTMMADR_BAR\n");
>  		return -EIO;
>  	}
>  
>  	xe->mmio.size = pci_resource_len(pdev, GTTMMADR_BAR);
>  	if (xe->mmio.size < SZ_16M) {
> -		xe_err(xe, "GTTMMADR_BAR is too small: %zu\n", xe->mmio.size);
> +		xe_ras_log_probe(xe, -EIO, "GTTMMADR_BAR is too small: %zu\n", xe->mmio.size);

while this error is during the probe, it actually shows some HW error
are we sure we want to log that as software error?

>  		return -EIO;
>  	}
>  
> diff --git a/drivers/gpu/drm/xe/xe_pat.c b/drivers/gpu/drm/xe/xe_pat.c
> index fad5b5a5ed4a..49d2e81090d3 100644
> --- a/drivers/gpu/drm/xe/xe_pat.c
> +++ b/drivers/gpu/drm/xe/xe_pat.c
> @@ -17,6 +17,7 @@
>  #include "xe_gt.h"
>  #include "xe_gt_mcr.h"
>  #include "xe_mmio.h"
> +#include "xe_ras_log.h"
>  #include "xe_sriov.h"
>  #include "xe_wa.h"
>  
> @@ -650,8 +651,9 @@ void xe_pat_init_early(struct xe_device *xe)
>  		 * raise an error rather than trying to silently inherit the
>  		 * most recent platform's behavior.
>  		 */
> -		drm_err(&xe->drm, "Missing PAT table for platform with graphics version %d.%02d!\n",
> -			GRAPHICS_VER(xe), GRAPHICS_VERx100(xe) % 100);
> +		xe_ras_log_probe(xe, -ENODEV,
> +				 "Missing PAT table for platform with graphics version %d.%02d!\n",
> +				 GRAPHICS_VER(xe), GRAPHICS_VERx100(xe) % 100);

are you sure it is fatal?
it looks that driver probe continues (even after below asserts) as error code is not propagated

maybe we should fix the original code first?

>  	}
>  
>  	xe_assert(xe, xe->pat.ops->dump);
> diff --git a/drivers/gpu/drm/xe/xe_pci.c b/drivers/gpu/drm/xe/xe_pci.c
> index 9c249454cc95..58442d9eb8a0 100644
> --- a/drivers/gpu/drm/xe/xe_pci.c
> +++ b/drivers/gpu/drm/xe/xe_pci.c
> @@ -31,6 +31,7 @@
>  #include "xe_pci_types.h"
>  #include "xe_pm.h"
>  #include "xe_printk.h"
> +#include "xe_ras_log.h"
>  #include "xe_sriov.h"
>  #include "xe_step.h"
>  #include "xe_survivability_mode.h"
> @@ -708,8 +709,9 @@ static int handle_gmdid(struct xe_device *xe,
>  
>  	*graphics_ip = find_graphics_ip(ver);
>  	if (!*graphics_ip) {
> -		drm_err(&xe->drm, "Hardware reports unknown graphics version %u.%02u\n",
> -			ver / 100, ver % 100);
> +		xe_ras_log_probe(xe, -ENODEV,
> +				 "Hardware reports unknown graphics version %u.%02u\n",
> +				 ver / 100, ver % 100);
>  	}
>  
>  	ret = read_gmdid(xe, GMDID_MEDIA, &ver, media_revid);
> @@ -722,8 +724,9 @@ static int handle_gmdid(struct xe_device *xe,
>  
>  	*media_ip = find_media_ip(ver);
>  	if (!*media_ip) {
> -		drm_err(&xe->drm, "Hardware reports unknown media version %u.%02u\n",
> -			ver / 100, ver % 100);
> +		xe_ras_log_probe(xe, -ENODEV,
> +				 "Hardware reports unknown media version %u.%02u\n",
> +				 ver / 100, ver % 100);

IIRC we can continue without media GT, so is this FATAL?

>  	}
>  
>  	return 0;
> @@ -1022,7 +1025,8 @@ static int xe_info_init(struct xe_device *xe,
>  		 * required for VRAM management).
>  		 */
>  		if (!tile->primary_gt) {
> -			drm_err(&xe->drm, "Cannot probe device with without a primary GT\n");
> +			xe_ras_log_probe(xe, -ENODEV,
> +					 "Cannot probe device with without a primary GT\n");
>  			return -ENODEV;
>  		}
>  
> diff --git a/drivers/gpu/drm/xe/xe_pcode.c b/drivers/gpu/drm/xe/xe_pcode.c
> index 866986694d9c..046336c7b378 100644
> --- a/drivers/gpu/drm/xe/xe_pcode.c
> +++ b/drivers/gpu/drm/xe/xe_pcode.c
> @@ -15,6 +15,7 @@
>  #include "xe_device.h"
>  #include "xe_mmio.h"
>  #include "xe_pcode_api.h"
> +#include "xe_ras_log.h"
>  
>  /**
>   * DOC: PCODE
> @@ -316,8 +317,8 @@ int xe_pcode_ready(struct xe_device *xe, bool locked)
>  		mutex_unlock(&tile->pcode.lock);
>  
>  	if (ret)
> -		drm_err(&xe->drm,
> -			"PCODE initialization timedout after: 3 min\n");
> +		xe_ras_log_probe(xe, -ETIMEDOUT,
> +				 "PCODE initialization timedout after: 3 min\n");

this function is also called from resume paths,
are you sure it should be tagged as PROBE/FATAL error?

>  
>  	return ret;
>  }
> diff --git a/drivers/gpu/drm/xe/xe_vram.c b/drivers/gpu/drm/xe/xe_vram.c
> index 23eb7edbdd57..e81d92239934 100644
> --- a/drivers/gpu/drm/xe/xe_vram.c
> +++ b/drivers/gpu/drm/xe/xe_vram.c
> @@ -18,6 +18,7 @@
>  #include "xe_force_wake.h"
>  #include "xe_gt_mcr.h"
>  #include "xe_mmio.h"
> +#include "xe_ras_log.h"
>  #include "xe_sriov.h"
>  #include "xe_tile_sriov_vf.h"
>  #include "xe_ttm_vram_mgr.h"
> @@ -43,7 +44,7 @@ static int determine_lmem_bar_size(struct xe_device *xe, struct xe_vram_region *
>  	struct pci_dev *pdev = to_pci_dev(xe->drm.dev);
>  
>  	if (!resource_is_valid(pdev, LMEM_BAR)) {
> -		drm_err(&xe->drm, "pci resource is not valid\n");
> +		xe_ras_log_probe(xe, -ENXIO, "pci resource is not valid\n");
>  		return -ENXIO;
>  	}
>  
> @@ -237,7 +238,7 @@ static int vram_region_init(struct xe_device *xe, struct xe_vram_region *vram,
>  	vram->io_size = min_t(u64, usable_size, remain_io_size);
>  
>  	if (!vram->io_size) {
> -		drm_err(&xe->drm, "Tile without any CPU visible VRAM. Aborting.\n");
> +		xe_ras_log_probe(xe, -ENODEV, "Tile without any CPU visible VRAM. Aborting.\n");
>  		return -ENODEV;
>  	}
>