All of lore.kernel.org
 help / color / mirror / Atom feed
From: Gilles Buloz <Gilles.Buloz@kontron.com>
To: Bjorn Helgaas <helgaas@kernel.org>
Cc: Bjorn Helgaas <bhelgaas@google.com>,
	linux-pci <linux-pci@vger.kernel.org>,
	"Minghuan.Lian@nxp.com" <Minghuan.Lian@nxp.com>,
	"linux-arm-kernel@lists.infradead.org"
	<linux-arm-kernel@lists.infradead.org>,
	Ard Biesheuvel <ard.biesheuvel@linaro.org>
Subject: Re: LS1043A : "synchronous abort" at boot due to PCI config read
Date: Thu, 3 May 2018 12:40:27 +0000	[thread overview]
Message-ID: <5AEB033A.4060407@kontron.com> (raw)
In-Reply-To: <20180502172341.GA123831@bhelgaas-glaptop.roam.corp.google.com>

Subject:    [PATCH] For exception at PCI probe due to bridge reporting UR

Even if a device supports extended config access, no such access must be
done to this device If there's a bridge not supporting that in the path
to this device. Doing such access with UR reporting enabled on the root
bridge leads to an exception.

This is the case on a LS1043A CPU (NXP QorIQ Layerscape) platform with
the following bus topology :
  LS1043 PCIe root
    -> PEX8112 PCIe-to-PCI bridge (not supporting ext cfg on PCI side)
      -> PMC slot connector (for legacy PMC modules)
With a PMC module topology as follows :
  PMC connector
    -> PCI-to-PCIe bridge
      -> PCIe switch (4 ports)
        -> 4 PCIe devices (one on each port)
In this case all devices behind the PEX8112 are supporting extended config
access but this is prohibited by the PEX8112. Without this patch, an
exception (synchronous abort) occurs in pci_cfg_space_size_ext().

This patch checks the parent bridge of each allocated child bus to know if
extended config access is supported on the child bus, and sets a flag in
child->bus_flags if not supported. This  flag is inherited by all children
buses of this child bus and then is checked to avoid this unsupported
accesses to every device on these buses.

Thanks

=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D
Gilles BULOZ
Senior software engineer
Kontron France
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D


Signed-off-by: Gilles Buloz <gilles.buloz@kontron.com>

--- linux-4.17-rc1/include/linux/pci.h.orig=092018-04-16 01:24:20.000000000=
 +0000
+++ linux-4.17-rc1/include/linux/pci.h=092018-05-03 09:53:03.270000000 +000=
0
@@ -217,6 +217,7 @@ enum pci_bus_flags {
  =09PCI_BUS_FLAGS_NO_MSI=09=3D (__force pci_bus_flags_t) 1,
  =09PCI_BUS_FLAGS_NO_MMRBC=09=3D (__force pci_bus_flags_t) 2,
  =09PCI_BUS_FLAGS_NO_AERSID=09=3D (__force pci_bus_flags_t) 4,
+=09PCI_BUS_FLAGS_NO_EXTCFG=09=3D (__force pci_bus_flags_t) 8,
  };
 =20
  /* Values from Link Status register, PCIe r3.1, sec 7.8.8 */
--- linux-4.17-rc1/drivers/pci/probe.c.orig=092018-05-03 09:45:21.110000000=
 +0000
+++ linux-4.17-rc1/drivers/pci/probe.c=092018-05-03 09:46:50.550000000 +000=
0
@@ -882,6 +882,24 @@ free:
  =09return err;
  }
 =20
+static bool pci_bridge_child_bus_ext_cfg_accessible(struct pci_dev *bridge=
)
+{
+=09int pos;
+=09u32 status;
+
+=09if (pci_is_pcie(bridge) &&
+=09    (pci_pcie_type(bridge) !=3D PCI_EXP_TYPE_PCIE_BRIDGE) &&
+=09    (pci_pcie_type(bridge) !=3D PCI_EXP_TYPE_PCI_BRIDGE))
+=09=09return true;
+
+=09/* PCI/PCI, or PCIe/PCI (forward), or PCI/PCIe (reverse) bridge */
+=09pos =3D pci_find_capability(bridge, PCI_CAP_ID_PCIX);
+=09if (pos)
+=09=09pci_read_config_dword(bridge, pos + PCI_X_STATUS, &status);
+
+=09return pos && (status & (PCI_X_STATUS_266MHZ | PCI_X_STATUS_533MHZ));
+}
+
  static struct pci_bus *pci_alloc_child_bus(struct pci_bus *parent,
  =09=09=09=09=09   struct pci_dev *bridge, int busnr)
  {
@@ -930,6 +948,20 @@ static struct pci_bus *pci_alloc_child_b
  =09}
  =09bridge->subordinate =3D child;
 =20
+=09/*
+=09 * if bus_flags inherited from parent bus do not already report lack of
+=09 * extended config space support, check if supported by child bus by
+=09 * checking its parent bridge
+=09 */
+=09if (child->bus_flags & PCI_BUS_FLAGS_NO_EXTCFG) {
+=09=09pci_info(child, "extended config space not accessible due to parent =
bus\n");
+=09} else {
+=09=09if (!pci_bridge_child_bus_ext_cfg_accessible(bridge)) {
+=09=09=09child->bus_flags |=3D PCI_BUS_FLAGS_NO_EXTCFG;
+=09=09=09pci_info(child, "extended config space not accessible due to pare=
nt bridge\n");
+=09=09}
+=09}
+
  add_dev:
  =09pci_set_bus_msi_domain(child);
  =09ret =3D device_register(&child->dev);
@@ -1393,6 +1425,9 @@ int pci_cfg_space_size(struct pci_dev *d
  =09u32 status;
  =09u16 class;
 =20
+=09if (dev->bus->bus_flags & PCI_BUS_FLAGS_NO_EXTCFG)
+=09=09return PCI_CFG_SPACE_SIZE;
+
  =09class =3D dev->class >> 8;
  =09if (class =3D=3D PCI_CLASS_BRIDGE_HOST)
  =09=09return pci_cfg_space_size_ext(dev);

WARNING: multiple messages have this Message-ID (diff)
From: Gilles.Buloz@kontron.com (Gilles Buloz)
To: linux-arm-kernel@lists.infradead.org
Subject: LS1043A : "synchronous abort" at boot due to PCI config read
Date: Thu, 3 May 2018 12:40:27 +0000	[thread overview]
Message-ID: <5AEB033A.4060407@kontron.com> (raw)
In-Reply-To: <20180502172341.GA123831@bhelgaas-glaptop.roam.corp.google.com>

Subject:    [PATCH] For exception at PCI probe due to bridge reporting UR

Even if a device supports extended config access, no such access must be
done to this device If there's a bridge not supporting that in the path
to this device. Doing such access with UR reporting enabled on the root
bridge leads to an exception.

This is the case on a LS1043A CPU (NXP QorIQ Layerscape) platform with
the following bus topology :
  LS1043 PCIe root
    -> PEX8112 PCIe-to-PCI bridge (not supporting ext cfg on PCI side)
      -> PMC slot connector (for legacy PMC modules)
With a PMC module topology as follows :
  PMC connector
    -> PCI-to-PCIe bridge
      -> PCIe switch (4 ports)
        -> 4 PCIe devices (one on each port)
In this case all devices behind the PEX8112 are supporting extended config
access but this is prohibited by the PEX8112. Without this patch, an
exception (synchronous abort) occurs in pci_cfg_space_size_ext().

This patch checks the parent bridge of each allocated child bus to know if
extended config access is supported on the child bus, and sets a flag in
child->bus_flags if not supported. This  flag is inherited by all children
buses of this child bus and then is checked to avoid this unsupported
accesses to every device on these buses.

Thanks

=====================
Gilles BULOZ
Senior software engineer
Kontron France
=====================


Signed-off-by: Gilles Buloz <gilles.buloz@kontron.com>

--- linux-4.17-rc1/include/linux/pci.h.orig	2018-04-16 01:24:20.000000000 +0000
+++ linux-4.17-rc1/include/linux/pci.h	2018-05-03 09:53:03.270000000 +0000
@@ -217,6 +217,7 @@ 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_AERSID	= (__force pci_bus_flags_t) 4,
+	PCI_BUS_FLAGS_NO_EXTCFG	= (__force pci_bus_flags_t) 8,
  };
  
  /* Values from Link Status register, PCIe r3.1, sec 7.8.8 */
--- linux-4.17-rc1/drivers/pci/probe.c.orig	2018-05-03 09:45:21.110000000 +0000
+++ linux-4.17-rc1/drivers/pci/probe.c	2018-05-03 09:46:50.550000000 +0000
@@ -882,6 +882,24 @@ free:
  	return err;
  }
  
+static bool pci_bridge_child_bus_ext_cfg_accessible(struct pci_dev *bridge)
+{
+	int pos;
+	u32 status;
+
+	if (pci_is_pcie(bridge) &&
+	    (pci_pcie_type(bridge) != PCI_EXP_TYPE_PCIE_BRIDGE) &&
+	    (pci_pcie_type(bridge) != PCI_EXP_TYPE_PCI_BRIDGE))
+		return true;
+
+	/* PCI/PCI, or PCIe/PCI (forward), or PCI/PCIe (reverse) bridge */
+	pos = pci_find_capability(bridge, PCI_CAP_ID_PCIX);
+	if (pos)
+		pci_read_config_dword(bridge, pos + PCI_X_STATUS, &status);
+
+	return pos && (status & (PCI_X_STATUS_266MHZ | PCI_X_STATUS_533MHZ));
+}
+
  static struct pci_bus *pci_alloc_child_bus(struct pci_bus *parent,
  					   struct pci_dev *bridge, int busnr)
  {
@@ -930,6 +948,20 @@ static struct pci_bus *pci_alloc_child_b
  	}
  	bridge->subordinate = child;
  
+	/*
+	 * if bus_flags inherited from parent bus do not already report lack of
+	 * extended config space support, check if supported by child bus by
+	 * checking its parent bridge
+	 */
+	if (child->bus_flags & PCI_BUS_FLAGS_NO_EXTCFG) {
+		pci_info(child, "extended config space not accessible due to parent bus\n");
+	} else {
+		if (!pci_bridge_child_bus_ext_cfg_accessible(bridge)) {
+			child->bus_flags |= PCI_BUS_FLAGS_NO_EXTCFG;
+			pci_info(child, "extended config space not accessible due to parent bridge\n");
+		}
+	}
+
  add_dev:
  	pci_set_bus_msi_domain(child);
  	ret = device_register(&child->dev);
@@ -1393,6 +1425,9 @@ int pci_cfg_space_size(struct pci_dev *d
  	u32 status;
  	u16 class;
  
+	if (dev->bus->bus_flags & PCI_BUS_FLAGS_NO_EXTCFG)
+		return PCI_CFG_SPACE_SIZE;
+
  	class = dev->class >> 8;
  	if (class == PCI_CLASS_BRIDGE_HOST)
  		return pci_cfg_space_size_ext(dev);

  reply	other threads:[~2018-05-03 12:40 UTC|newest]

Thread overview: 31+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-04-13 17:32 LS1043A : "synchronous abort" at boot due to PCI config read Gilles Buloz
2018-04-27  8:43 ` Ard Biesheuvel
2018-04-27  8:43   ` Ard Biesheuvel
2018-04-27 12:29   ` Gilles Buloz
2018-04-27 12:29     ` Gilles Buloz
2018-04-27 16:56     ` Bjorn Helgaas
2018-04-27 16:56       ` Bjorn Helgaas
2018-04-30  8:46       ` Gilles Buloz
2018-04-30  8:46         ` Gilles Buloz
2018-04-30 13:36         ` Gilles Buloz
2018-04-30 13:36           ` Gilles Buloz
2018-04-30 17:04           ` Bjorn Helgaas
2018-04-30 17:04             ` Bjorn Helgaas
2018-04-30 17:53             ` Gilles Buloz
2018-04-30 17:53               ` Gilles Buloz
2018-05-02 12:57               ` Gilles Buloz
2018-05-02 12:57                 ` Gilles Buloz
2018-05-02 13:26                 ` Bjorn Helgaas
2018-05-02 13:26                   ` Bjorn Helgaas
2018-05-02 13:48                   ` Gilles Buloz
2018-05-02 13:48                     ` Gilles Buloz
2018-05-02 17:23                     ` Bjorn Helgaas
2018-05-02 17:23                       ` Bjorn Helgaas
2018-05-03 12:40                       ` Gilles Buloz [this message]
2018-05-03 12:40                         ` Gilles Buloz
2018-05-03 22:31                         ` [PATCH] PCI: Check whether bridges allow access to extended config space Bjorn Helgaas
2018-05-03 22:31                           ` Bjorn Helgaas
2018-05-03 22:31                           ` Bjorn Helgaas
2018-05-04 15:45                           ` Gilles Buloz
2018-05-04 15:45                             ` Gilles Buloz
2018-05-04 15:45                             ` Gilles Buloz

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=5AEB033A.4060407@kontron.com \
    --to=gilles.buloz@kontron.com \
    --cc=Minghuan.Lian@nxp.com \
    --cc=ard.biesheuvel@linaro.org \
    --cc=bhelgaas@google.com \
    --cc=helgaas@kernel.org \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-pci@vger.kernel.org \
    /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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.