[PATCH v3] PCI: Sanitise firmware BAR assignments behind a PCI-PCI bridge

* [PATCH v3] PCI: Sanitise firmware BAR assignments behind a PCI-PCI bridge
@ 2022-09-21 19:49 Maciej W. Rozycki
  2022-09-21 22:53 ` Bjorn Helgaas
  0 siblings, 1 reply; 3+ messages in thread
From: Maciej W. Rozycki @ 2022-09-21 19:49 UTC (permalink / raw)
  To: Bjorn Helgaas; +Cc: linux-pci, linux-kernel

Fix an issue with the Tyan Tomcat IV S1564D system, the BIOS of which 
does not assign PCI buses beyond #2, where our resource reallocation 
code preserves the reset default of an I/O BAR assignment outside its 
upstream PCI-to-PCI bridge's I/O forwarding range:

pci 0000:06:08.0: BAR 4: no space for [io  size 0x0020]
pci 0000:06:08.0: BAR 4: trying firmware assignment [io  0xfce0-0xfcff]
pci 0000:06:08.0: BAR 4: assigned [io  0xfce0-0xfcff]
[...]
pci_bus 0000:06: resource 0 [io  0x2000-0x2fff]

Consequently when the device driver tries to access 06:08.0 according to 
its designated address range it pokes at an unassigned I/O location, 
likely subtractively decoded by the southbridge and forwarded to ISA, 
causing the driver to become confused and bail out:

uhci_hcd 0000:06:08.0: host system error, PCI problems?
uhci_hcd 0000:06:08.0: host controller process error, something bad happened!
uhci_hcd 0000:06:08.0: host controller halted, very bad!
uhci_hcd 0000:06:08.0: HCRESET not completed yet!
uhci_hcd 0000:06:08.0: HC died; cleaning up

if good luck happens or if bad luck does, an infinite flood of messages:

uhci_hcd 0000:06:08.0: host system error, PCI problems?
uhci_hcd 0000:06:08.0: host controller process error, something bad happened!

making the system virtually unusable.

This is because we try to retain any BAR assignment the firmware may 
have made here, which may be necessary for devices on the root bus with 
some systems, but cannot work for devices that are behind a PCI-to-PCI 
bridge where the BAR assignment is outside the upstream bridge's 
forwarding range.

Make sure then for a device behind a PCI-to-PCI bridge that any firmware 
assignment is within the bridge's relevant forwarding window or do not 
restore the assignment, fixing the system concerned as follows:

pci 0000:06:08.0: BAR 4: no space for [io  size 0x0020]
pci 0000:06:08.0: BAR 4: failed to assign [io  0xfce0-0xfcff]
[...]
pci 0000:06:08.0: BAR 4: assigned [io  0x2000-0x201f]

and making device 06:08.0 work correctly.

Cf. <https://bugzilla.kernel.org/show_bug.cgi?id=16263>

Signed-off-by: Maciej W. Rozycki <macro@orcam.me.uk>
Link: https://lore.kernel.org/r/alpine.DEB.2.21.2203012338460.46819@angie.orcam.me.uk
Fixes: 58c84eda0756 ("PCI: fall back to original BIOS BAR addresses")
Cc: stable@vger.kernel.org # v2.6.35+
---
Hi Bjorn,

 I have trimmed the change description down as you requested and left the 
change proper unmodified, as discussed in my earlier response.

 Let me know if you have any other concerns with this fix.

  Maciej

Changes from v2:

- Change description trimmed and rephrased, link to a full bootstrap log 
  earlier on in discussion added.

Changes from v1:

- Do restore firmware BAR assignments behind a PCI-PCI bridge, but only if 
  within the bridge's forwarding window.

- Update the change description and heading accordingly (was: PCI: Do not 
  restore firmware BAR assignments behind a PCI-PCI bridge).
---
 drivers/pci/setup-res.c |   12 +++++++++++-
 1 file changed, 11 insertions(+), 1 deletion(-)

linux-pci-setup-res-fw-address-nobridge.diff
Index: linux-macro/drivers/pci/setup-res.c
===================================================================

--- linux-macro.orig/drivers/pci/setup-res.c
+++ linux-macro/drivers/pci/setup-res.c
@@ -212,9 +212,19 @@ static int pci_revert_fw_address(struct
 	res->end = res->start + size - 1;
 	res->flags &= ~IORESOURCE_UNSET;
 
+	/*
+	 * If we're behind a P2P or CardBus bridge, make sure we're
+	 * inside the relevant forwarding window, or otherwise the
+	 * assignment must have been bogus and accesses intended for
+	 * the range assigned would not reach the device anyway.
+	 * On the root bus accept anything under the assumption the
+	 * host bridge will let it through.
+	 */
 	root = pci_find_parent_resource(dev, res);
 	if (!root) {
-		if (res->flags & IORESOURCE_IO)
+		if (dev->bus->parent)
+			return -ENXIO;
+		else if (res->flags & IORESOURCE_IO)
 			root = &ioport_resource;
 		else
 			root = &iomem_resource;

^ permalink raw reply	[flat|nested] 3+ messages in thread