[PATCH 1/2] pci/aer: Add bus flag to skip source id matching

[PATCH 1/2] pci/aer: Add bus flag to skip source id matching

[Jon Derrick]

Allow root port buses to choose to skip source id matching when finding
the faulting device. Certain root port devices may return an incorrect
source id and recommend to scan child device registers for aer
notifications.

Signed-off-by: Jon Derrick <jonathan.derrick@intel.com>
---
 drivers/pci/pcie/aer/aerdrv_core.c | 7 +++++--
 include/linux/pci.h                | 5 +++--
 2 files changed, 8 insertions(+), 4 deletions(-)

diff --git a/drivers/pci/pcie/aer/aerdrv_core.c b/drivers/pci/pcie/aer/aerdrv_core.c
index 521e39c..3a0d839 100644
--- a/drivers/pci/pcie/aer/aerdrv_core.c
+++ b/drivers/pci/pcie/aer/aerdrv_core.c
@@ -132,7 +132,9 @@ static bool is_error_source(struct pci_dev *dev, struct aer_err_info *e_info)
 	 * When bus id is equal to 0, it might be a bad id
 	 * reported by root port.
 	 */
-	if (!nosourceid && (PCI_BUS_NUM(e_info->id) != 0)) {
+	if (!nosourceid &&
+	    (PCI_BUS_NUM(e_info->id) != 0) &&
+	    !(dev->bus->bus_flags & PCI_BUS_FLAGS_NO_AERSID)) {
 		/* Device ID match? */
 		if (e_info->id == ((dev->bus->number << 8) | dev->devfn))
 			return true;
@@ -147,7 +149,8 @@ static bool is_error_source(struct pci_dev *dev, struct aer_err_info *e_info)
 	 *      1) nosourceid==y;
 	 *      2) bus id is equal to 0. Some ports might lose the bus
 	 *              id of error source id;
-	 *      3) There are multiple errors and prior id comparing fails;
+	 *      3) bus flag PCI_BUS_FLAGS_NO_AERSID is set
+	 *      4) There are multiple errors and prior id comparing fails;
 	 * We check AER status registers to find possible reporter.
 	 */
 	if (atomic_read(&dev->enable_cnt) == 0)
diff --git a/include/linux/pci.h b/include/linux/pci.h
index b7d3aa2..5c08137 100644
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -183,8 +183,9 @@ enum pci_irq_reroute_variant {
 
 typedef unsigned short __bitwise pci_bus_flags_t;
 enum pci_bus_flags {
-	PCI_BUS_FLAGS_NO_MSI   = (__force pci_bus_flags_t) 1,
-	PCI_BUS_FLAGS_NO_MMRBC = (__force pci_bus_flags_t) 2,
+	PCI_BUS_FLAGS_NO_MSI	= (__force pci_bus_flags_t) 1,
+	PCI_BUS_FLAGS_NO_MMRBC	= (__force pci_bus_flags_t) 2,
+	PCI_BUS_FLAGS_NO_AERSID	= (__force pci_bus_flags_t) 4,
 };
 
 /* These values come from the PCI Express Spec */
-- 
1.8.3.1

[PATCH 2/2] pci/vmd: Add quirk for aer to ignore source id

[Jon Derrick]

VMD root ports change all source ids to the VMD device id. To find the
sender of the aer notification, we need to scan all child devices for
the aer sender, rather than relying on the source id from the message.

Signed-off-by: Jon Derrick <jonathan.derrick@intel.com>
---
 drivers/pci/quirks.c | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)

diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c
index 0359e0b..a10ce81 100644
--- a/drivers/pci/quirks.c
+++ b/drivers/pci/quirks.c
@@ -4438,3 +4438,20 @@ DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_INTEL,      0x2032, quirk_hsbp);
 DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_INTEL,      0x2033, quirk_hsbp);
 DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_PMC_Sierra, 0x8531, quirk_hsbp);
 DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_PMC_Sierra, 0x8533, quirk_hsbp);
+
+/*
+ * VMD enabled root ports will change the source id for all messages
+ * to the VMD device. Rather than doing device matching with the source
+ * id, the AER driver should traverse the child device tree, reading
+ * AER registers to find the faulting device.
+ */
+static void quirk_no_aersid(struct pci_dev *pdev)
+{
+	/* VMD Domain */
+	if (pdev->bus->sysdata && pci_domain_nr(pdev->bus) >= 0x10000)
+		pdev->bus->bus_flags |= PCI_BUS_FLAGS_NO_AERSID;
+}
+DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_INTEL,      0x2030, quirk_no_aersid);
+DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_INTEL,      0x2031, quirk_no_aersid);
+DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_INTEL,      0x2032, quirk_no_aersid);
+DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_INTEL,      0x2033, quirk_no_aersid);
-- 
1.8.3.1

Re: [PATCH 1/2] pci/aer: Add bus flag to skip source id matching

[Bjorn Helgaas]

On Thu, Aug 25, 2016 at 05:26:10PM -0600, Jon Derrick wrote:
> Allow root port buses to choose to skip source id matching when finding
> the faulting device. Certain root port devices may return an incorrect
> source id and recommend to scan child device registers for aer
> notifications.
> 
> Signed-off-by: Jon Derrick <jonathan.derrick@intel.com>

Applied both of these to pci/host-vmd for v4.9, thanks!

I think I might even go farther and remove the "nosourceid" module
parameter.  Per Documentation/PCI/pcieaer-howto.txt, it's to work
around broken hardware, but it's not otherwise documented.  With the
flag you're adding, we could add quirks for that broken hardware,
which would be far better.  I doubt anybody's actually using that
module parameter.

> ---
>  drivers/pci/pcie/aer/aerdrv_core.c | 7 +++++--
>  include/linux/pci.h                | 5 +++--
>  2 files changed, 8 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/pci/pcie/aer/aerdrv_core.c b/drivers/pci/pcie/aer/aerdrv_core.c
> index 521e39c..3a0d839 100644
> --- a/drivers/pci/pcie/aer/aerdrv_core.c
> +++ b/drivers/pci/pcie/aer/aerdrv_core.c
> @@ -132,7 +132,9 @@ static bool is_error_source(struct pci_dev *dev, struct aer_err_info *e_info)
>  	 * When bus id is equal to 0, it might be a bad id
>  	 * reported by root port.
>  	 */
> -	if (!nosourceid && (PCI_BUS_NUM(e_info->id) != 0)) {
> +	if (!nosourceid &&
> +	    (PCI_BUS_NUM(e_info->id) != 0) &&
> +	    !(dev->bus->bus_flags & PCI_BUS_FLAGS_NO_AERSID)) {
>  		/* Device ID match? */
>  		if (e_info->id == ((dev->bus->number << 8) | dev->devfn))
>  			return true;
> @@ -147,7 +149,8 @@ static bool is_error_source(struct pci_dev *dev, struct aer_err_info *e_info)
>  	 *      1) nosourceid==y;
>  	 *      2) bus id is equal to 0. Some ports might lose the bus
>  	 *              id of error source id;
> -	 *      3) There are multiple errors and prior id comparing fails;
> +	 *      3) bus flag PCI_BUS_FLAGS_NO_AERSID is set
> +	 *      4) There are multiple errors and prior id comparing fails;
>  	 * We check AER status registers to find possible reporter.
>  	 */
>  	if (atomic_read(&dev->enable_cnt) == 0)
> diff --git a/include/linux/pci.h b/include/linux/pci.h
> index b7d3aa2..5c08137 100644
> --- a/include/linux/pci.h
> +++ b/include/linux/pci.h
> @@ -183,8 +183,9 @@ enum pci_irq_reroute_variant {
>  
>  typedef unsigned short __bitwise pci_bus_flags_t;
>  enum pci_bus_flags {
> -	PCI_BUS_FLAGS_NO_MSI   = (__force pci_bus_flags_t) 1,
> -	PCI_BUS_FLAGS_NO_MMRBC = (__force pci_bus_flags_t) 2,
> +	PCI_BUS_FLAGS_NO_MSI	= (__force pci_bus_flags_t) 1,
> +	PCI_BUS_FLAGS_NO_MMRBC	= (__force pci_bus_flags_t) 2,
> +	PCI_BUS_FLAGS_NO_AERSID	= (__force pci_bus_flags_t) 4,
>  };
>  
>  /* These values come from the PCI Express Spec */
> -- 
> 1.8.3.1
> 

Re: [PATCH 1/2] pci/aer: Add bus flag to skip source id matching

[Jon Derrick]

On Tue, Sep 06, 2016 at 02:23:44PM -0500, Bjorn Helgaas wrote:
> On Thu, Aug 25, 2016 at 05:26:10PM -0600, Jon Derrick wrote:
> > Allow root port buses to choose to skip source id matching when finding
> > the faulting device. Certain root port devices may return an incorrect
> > source id and recommend to scan child device registers for aer
> > notifications.
> > 
> > Signed-off-by: Jon Derrick <jonathan.derrick@intel.com>
> 
> Applied both of these to pci/host-vmd for v4.9, thanks!
Thanks!

> 
> I think I might even go farther and remove the "nosourceid" module
> parameter.  Per Documentation/PCI/pcieaer-howto.txt, it's to work
> around broken hardware, but it's not otherwise documented.  With the
> flag you're adding, we could add quirks for that broken hardware,
> which would be far better.  I doubt anybody's actually using that
> module parameter.
Yes I'll agree that it makes more sense to call out the nonconforming hardware and have their quirk/fix inbox