Re: [PATCH v2 2/2] drm/xe/ras: Enable SGUnit PCIe error reporting

[Bjorn Helgaas]

On Tue, Aug 25, 2026 at 11:12:51AM +0530, Raag Jadav wrote:
> SGUnit as a PCI endpoint is not AER capable and requires explicit
> programming of error reporting bits in DevCtl register of it's config
> space for upstream error propagation. Hack it.

s/it's/its/

> While at it, add native AER sanity check for upstream port before
> programming SGUnit register.
> 
> This is as per pci_enable_pcie_error_reporting() logic implemented
> in PCIe AER driver.
> 
> Signed-off-by: Raag Jadav <raag.jadav@xxxxxxxxx>
> Reviewed-by: Lukas Wunner <lukas@xxxxxxxxx>
> ---
>  drivers/gpu/drm/xe/xe_ras.c | 14 ++++++++++++++
>  1 file changed, 14 insertions(+)
> 
> diff --git a/drivers/gpu/drm/xe/xe_ras.c b/drivers/gpu/drm/xe/xe_ras.c
> index de4cb9ef7355..c4b5c0b5eb0d 100644
> --- a/drivers/gpu/drm/xe/xe_ras.c
> +++ b/drivers/gpu/drm/xe/xe_ras.c
> @@ -3,6 +3,8 @@
>   * Copyright © 2026 Intel Corporation
>   */
>  
> +#include <linux/aer.h>
> +
>  #include "xe_debugfs.h"
>  #include "xe_device.h"
>  #include "xe_drm_ras.h"
> @@ -234,6 +236,9 @@ static struct pci_dev *find_usp_dev(struct pci_dev *pdev)
>  	return pci_upstream_bridge(vsp);
>  }
>  
> +#define PCI_EXP_AER_FLAGS	(PCI_EXP_DEVCTL_CERE | PCI_EXP_DEVCTL_NFERE | \
> +				 PCI_EXP_DEVCTL_FERE | PCI_EXP_DEVCTL_URRE)

Ugh :)  Duplicate of what's in drivers/pci/pci.h.

>  static void ras_usp_aer_init(struct xe_device *xe)
>  {
>  	struct pci_dev *pdev = to_pci_dev(xe->drm.dev);
> @@ -251,6 +256,14 @@ static void ras_usp_aer_init(struct xe_device *xe)
>  		return;
>  	}
>  
> +	if (!pcie_aer_is_native(usp)) {
> +		dev_warn(&usp->dev, "No native AER support\n");
> +		return;
> +	}
> +
> +	pcie_capability_set_word(pdev, PCI_EXP_DEVCTL, PCI_EXP_AER_FLAGS);

The PCI core calls pci_aer_init() for every device, which only calls
pci_enable_pcie_error_reporting() to set PCI_EXP_AER_FLAGS when the
device itself has an AER Capability.

Setting PCI_EXP_AER_FLAGS enables the device to send ERR_* messages
upstream.  *Maybe* enabling those has something to do with whether the
Root Port (the ultimate consumer of those ERR_* messages) supports
AER, but I don't think it is related to whether the device *itself*
has an AER Capability.

It seems to me like the PCI core should do
pci_enable_pcie_error_reporting() independent of whether the device
has an AER Capability.

> +	pci_save_state(pdev);
> +
>  	/*
>  	 * Clear any stale Uncorrectable Internal Error Status event in Uncorrectable Error
>  	 * Status Register.
> @@ -279,6 +292,7 @@ static void ras_usp_aer_init(struct xe_device *xe)
>  	pci_save_state(usp);
>  	dev_dbg(&usp->dev, "Uncorrectable Internal Errors downgraded and unmasked\n");
>  }
> +MODULE_IMPORT_NS("AER");
>  
>  static void ras_send_error_event(struct xe_device *xe, u8 severity, u8 component)
>  {
> -- 
> 2.43.0
>