LinuxPPC-Dev Archive on lore.kernel.org
 help / color / mirror / Atom feed
From: Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@linux.intel.com>
To: Lukas Wunner <lukas@wunner.de>,
	Bjorn Helgaas <helgaas@kernel.org>,
	linux-pci@vger.kernel.org
Cc: Sourabh Jain <sourabhjain@linux.ibm.com>,
	Oliver OHalloran <oohall@gmail.com>,
	linuxppc-dev@lists.ozlabs.org,
	Alex Deucher <alexander.deucher@amd.com>,
	Keith Busch <kbusch@kernel.org>,
	Maciej Grochowski <Maciej.Grochowski@sony.com>
Subject: Re: [PATCH 2/2] PCI/AER: Check validity of Error Source ID register
Date: Tue, 3 Nov 2026 08:15:07 -0800	[thread overview]
Message-ID: <5fe32a78-4048-4624-8c1a-b50e3ecbb093@linux.intel.com> (raw)
In-Reply-To: <27a3876874161204e7a37ab63e7b27ee9783261f.1793697480.git.lukas@wunner.de>



On 11/3/2026 1:45 AM, Lukas Wunner wrote:
> Certain Intel and AMD Root Ports are known to populate the Error Source ID
> register with an invalid value.  The AER driver seeks to recognize such
> buggy silicon by assuming invalidity if the bus number is zero.  It then
> searches for the error-signaling device in the hierarchy below the Root
> Port.  This was introduced by commit 28eb27cf0839 ("PCI AER: support
> invalid error source IDs").
> 
> Alex has provided dmesg output which shows an invalid Error Source ID with
> a non-zero bus number.  The AER driver's heuristic fails to recognize its
> invalidity:
> 
>   pcieport 0000:00:07.1: AER: Correctable Error message received from 0000:01:00.0 (no details found)
> 
> The Root Port at 0000:00:07.1 has subordinate bus 04, so an error source
> on bus 01 is obviously invalid.  The range of subordinate buses is cached
> and checking against it is cheap.
> 
> Do that instead of relying on a heuristic.  Emit a message when an invalid
> Error Source ID is detected to alert silicon validation engineers.
> 
> Link: https://bugzilla.kernel.org/show_bug.cgi?id=222095
> Tested-by: Alex Deucher <alexander.deucher@amd.com>
> Signed-off-by: Lukas Wunner <lukas@wunner.de>
> ---
>  drivers/pci/pci.h       |  7 ++++++-
>  drivers/pci/pcie/aer.c  | 33 +++++++++++++++++++++++++--------
>  drivers/pci/pcie/rcec.c | 14 ++++++++++++++
>  3 files changed, 45 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/pci/pci.h b/drivers/pci/pci.h
> index 77efa4c5c960..aa392bb6a482 100644
> --- a/drivers/pci/pci.h
> +++ b/drivers/pci/pci.h
> @@ -871,6 +871,9 @@ static inline bool pci_dev_binding_disallowed(struct pci_dev *dev)
>   * @id: Value from register PCI_ERR_ROOT_ERR_SRC
>   * @severity: AER severity, 0-UNCOR Non-fatal, 1-UNCOR fatal, 2-COR
>   * @root_ratelimit_print: Flag to log or not log the root's error. 0=NotLog/1=Log
> + * @id_valid: Whether @id is a valid value.  Some Root Ports report an invalid
> + *	Error Source ID, necessitating a search for the error-signaling device
> + *	in the hierarchy below the Root Port.
>   * @multi_error_valid: If multiple errors are reported
>   * @first_error: First reported error
>   * @__pad2: Padding for alignment
> @@ -893,7 +896,8 @@ struct aer_err_info {
>  
>  	unsigned int severity:2;
>  	unsigned int root_ratelimit_print:1;
> -	unsigned int __pad1:4;
> +	unsigned int __pad1:3;
> +	unsigned int id_valid:1;
>  	unsigned int multi_error_valid:1;
>  
>  	unsigned int first_error:5;
> @@ -955,6 +959,7 @@ void pcie_link_rcec(struct pci_dev *rcec);
>  void pcie_walk_rcec(struct pci_dev *rcec,
>  		    int (*cb)(struct pci_dev *, void *),
>  		    void *userdata);
> +bool pci_rcec_associated(struct pci_dev *rcec, u16 id);
>  #else
>  static inline void pci_rcec_init(struct pci_dev *dev) { }
>  static inline void pci_rcec_exit(struct pci_dev *dev) { }
> diff --git a/drivers/pci/pcie/aer.c b/drivers/pci/pcie/aer.c
> index d492c7ce01e1..9957c36bf215 100644
> --- a/drivers/pci/pcie/aer.c
> +++ b/drivers/pci/pcie/aer.c
> @@ -951,12 +951,13 @@ static void aer_print_source(struct pci_dev *dev, struct aer_err_info *info,
>  {
>  	u16 source = info->id;
>  
> -	pci_info(dev, "%s%s Error message%s from %04x:%02x:%02x.%d%s\n",
> +	pci_info(dev, "%s%s Error message%s from %04x:%02x:%02x.%d%s%s\n",
>  		 info->multi_error_valid ? "Multiple " : "",
>  		 aer_error_severity_string[info->severity],
>  		 info->multi_error_valid ? "s received, first one" : " received",
>  		 pci_domain_nr(dev->bus), PCI_BUS_NUM(source),
>  		 PCI_SLOT(source), PCI_FUNC(source),
> +		 info->id_valid ? "" : " (invalid source)",
>  		 found ? "" : " (no details found)");
>  }
>  
> @@ -1146,6 +1147,24 @@ static int add_error_device(struct aer_err_info *e_info, struct pci_dev *dev)
>  	return 0;
>  }
>  
> +static bool source_id_valid(struct pci_dev *root, u16 id)
> +{
> +	/* The Root Port or RCEC can itself be a valid Error Source */
> +	if (pci_dev_id(root) == id)
> +		return true;
> +
> +	/* For RCECs, @id shall be in range of its associated buses/devices */
> +	if (pci_pcie_type(root) == PCI_EXP_TYPE_RC_EC)
> +		return pci_rcec_associated(root, id);
> +
> +	/* For Root Ports, @id shall be in range of its subordinate buses */
> +	if (!root->subordinate)
> +		return false;
> +
> +	return root->subordinate->busn_res.start <= PCI_BUS_NUM(id) &&
> +	       root->subordinate->busn_res.end   >= PCI_BUS_NUM(id);
> +}
> +
>  /**
>   * is_error_source - check whether the device is source of reported error
>   * @dev: pointer to pci_dev to be checked
> @@ -1157,11 +1176,8 @@ static bool is_error_source(struct pci_dev *dev, struct aer_err_info *e_info)
>  	u32 status, mask;
>  	u16 reg16;
>  
> -	/*
> -	 * When bus ID is equal to 0, it might be a bad ID
> -	 * reported by Root Port.
> -	 */
> -	if ((PCI_BUS_NUM(e_info->id) != 0) &&
> +	/* Some Root Ports report an invalid Error Source ID */
> +	if (e_info->id_valid &&
>  	    !(dev->bus->bus_flags & PCI_BUS_FLAGS_NO_AERSID)) {
>  		/* Device ID match? */
>  		if (e_info->id == pci_dev_id(dev))
> @@ -1174,8 +1190,7 @@ static bool is_error_source(struct pci_dev *dev, struct aer_err_info *e_info)
>  
>  	/*
>  	 * When either
> -	 *      1) bus ID is equal to 0. Some ports might lose the bus
> -	 *              ID of error source id;
> +	 *      1) Error Source ID register is invalid;
>  	 *      2) bus flag PCI_BUS_FLAGS_NO_AERSID is set
>  	 *      3) There are multiple errors and prior ID comparing fails;
>  	 * We check AER status registers to find possible reporter.
> @@ -1566,6 +1581,8 @@ static void aer_isr_one_error_type(struct pci_dev *root,
>  {
>  	bool found;
>  
> +	info->id_valid = source_id_valid(root, info->id);
> +
>  	found = find_source_device(root, info);

nit: Updated the kernel-doc help of find_source_device() to include id_valid.

>  
>  	/*
> diff --git a/drivers/pci/pcie/rcec.c b/drivers/pci/pcie/rcec.c
> index 2bf6d9d549ba..609d656300fa 100644
> --- a/drivers/pci/pcie/rcec.c
> +++ b/drivers/pci/pcie/rcec.c
> @@ -21,6 +21,20 @@ struct walk_rcec_data {
>  	void *user_data;
>  };
>  
> +bool pci_rcec_associated(struct pci_dev *rcec, u16 id)
> +{
> +	if (!rcec->rcec_ea)
> +		return false;
> +
> +	if (rcec->bus->number == PCI_BUS_NUM(id)) {
> +		unsigned long bitmap = rcec->rcec_ea->bitmap;
> +		return test_bit(PCI_SLOT(id), &bitmap);
> +	}
> +
> +	return rcec->rcec_ea->nextbusn <= PCI_BUS_NUM(id) &&
> +	       rcec->rcec_ea->lastbusn >= PCI_BUS_NUM(id);
> +}
> +
>  static bool rcec_assoc_rciep(struct pci_dev *rcec, struct pci_dev *rciep)
>  {
>  	unsigned long bitmap = rcec->rcec_ea->bitmap;

Reviewed-by: Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@linux.intel.com>


-- 
Sathyanarayanan Kuppuswamy
Linux Kernel Developer



      parent reply	other threads:[~2026-11-03 16:15 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-11-03  9:45 [PATCH 0/2] PCI/AER: Improve Error Source handling Lukas Wunner
2026-11-03  9:45 ` [PATCH 1/2] PCI/AER: Avoid NULL pointer deref on search for Error Source Lukas Wunner
2026-11-03 12:39   ` Ilpo Järvinen
2026-11-03 16:08   ` Kuppuswamy Sathyanarayanan
2026-11-03  9:45 ` [PATCH 2/2] PCI/AER: Check validity of Error Source ID register Lukas Wunner
2026-11-03 12:57   ` Ilpo Järvinen
2026-11-03 16:15   ` Kuppuswamy Sathyanarayanan [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=5fe32a78-4048-4624-8c1a-b50e3ecbb093@linux.intel.com \
    --to=sathyanarayanan.kuppuswamy@linux.intel.com \
    --cc=Maciej.Grochowski@sony.com \
    --cc=alexander.deucher@amd.com \
    --cc=helgaas@kernel.org \
    --cc=kbusch@kernel.org \
    --cc=linux-pci@vger.kernel.org \
    --cc=linuxppc-dev@lists.ozlabs.org \
    --cc=lukas@wunner.de \
    --cc=oohall@gmail.com \
    --cc=sourabhjain@linux.ibm.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox