lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Date:   Wed, 25 Oct 2017 01:56:13 +0000
From:   "Levin, Alexander (Sasha Levin)" <alexander.levin@...izon.com>
To:     "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "stable@...r.kernel.org" <stable@...r.kernel.org>
Cc:     Jason Gunthorpe <jgunthorpe@...idianresearch.com>,
        Bjorn Helgaas <bhelgaas@...gle.com>,
        "Levin, Alexander (Sasha Levin)" <alexander.levin@...izon.com>
Subject: [PATCH AUTOSEL for 4.4 03/25] PCI: mvebu: Handle changes to the
 bridge windows while enabled

From: Jason Gunthorpe <jgunthorpe@...idianresearch.com>

[ Upstream commit d9bf28e2650fe3eeefed7e34841aea07d10c6543 ]

The PCI core will write to the bridge window config multiple times while
they are enabled.  This can lead to mbus failures like this:

 mvebu_mbus: cannot add window '4:e8', conflicts with another window
 mvebu-pcie mbus:pex@...00000: Could not create MBus window at [mem 0xe0000000-0xe00fffff]: -22

For me this is happening during a hotplug cycle.  The PCI core is not
changing the values, just writing them twice while active.

The patch addresses the general case of any change to an active window, but
not atomically.  The code is slightly refactored so io and mem can share
more of the window logic.

Signed-off-by: Jason Gunthorpe <jgunthorpe@...idianresearch.com>
Signed-off-by: Bjorn Helgaas <bhelgaas@...gle.com>
Acked-by: Jason Cooper <jason@...edaemon.net>
Signed-off-by: Sasha Levin <alexander.levin@...izon.com>
---
 drivers/pci/host/pci-mvebu.c | 101 +++++++++++++++++++++++++------------------
 1 file changed, 60 insertions(+), 41 deletions(-)

diff --git a/drivers/pci/host/pci-mvebu.c b/drivers/pci/host/pci-mvebu.c
index 53b79c5f0559..379d08f76146 100644
--- a/drivers/pci/host/pci-mvebu.c
+++ b/drivers/pci/host/pci-mvebu.c
@@ -131,6 +131,12 @@ struct mvebu_pcie {
 	int nports;
 };
 
+struct mvebu_pcie_window {
+	phys_addr_t base;
+	phys_addr_t remap;
+	size_t size;
+};
+
 /* Structure representing one PCIe interface */
 struct mvebu_pcie_port {
 	char *name;
@@ -148,10 +154,8 @@ struct mvebu_pcie_port {
 	struct mvebu_sw_pci_bridge bridge;
 	struct device_node *dn;
 	struct mvebu_pcie *pcie;
-	phys_addr_t memwin_base;
-	size_t memwin_size;
-	phys_addr_t iowin_base;
-	size_t iowin_size;
+	struct mvebu_pcie_window memwin;
+	struct mvebu_pcie_window iowin;
 	u32 saved_pcie_stat;
 };
 
@@ -377,23 +381,45 @@ static void mvebu_pcie_add_windows(struct mvebu_pcie_port *port,
 	}
 }
 
+static void mvebu_pcie_set_window(struct mvebu_pcie_port *port,
+				  unsigned int target, unsigned int attribute,
+				  const struct mvebu_pcie_window *desired,
+				  struct mvebu_pcie_window *cur)
+{
+	if (desired->base == cur->base && desired->remap == cur->remap &&
+	    desired->size == cur->size)
+		return;
+
+	if (cur->size != 0) {
+		mvebu_pcie_del_windows(port, cur->base, cur->size);
+		cur->size = 0;
+		cur->base = 0;
+
+		/*
+		 * If something tries to change the window while it is enabled
+		 * the change will not be done atomically. That would be
+		 * difficult to do in the general case.
+		 */
+	}
+
+	if (desired->size == 0)
+		return;
+
+	mvebu_pcie_add_windows(port, target, attribute, desired->base,
+			       desired->size, desired->remap);
+	*cur = *desired;
+}
+
 static void mvebu_pcie_handle_iobase_change(struct mvebu_pcie_port *port)
 {
-	phys_addr_t iobase;
+	struct mvebu_pcie_window desired = {};
 
 	/* Are the new iobase/iolimit values invalid? */
 	if (port->bridge.iolimit < port->bridge.iobase ||
 	    port->bridge.iolimitupper < port->bridge.iobaseupper ||
 	    !(port->bridge.command & PCI_COMMAND_IO)) {
-
-		/* If a window was configured, remove it */
-		if (port->iowin_base) {
-			mvebu_pcie_del_windows(port, port->iowin_base,
-					       port->iowin_size);
-			port->iowin_base = 0;
-			port->iowin_size = 0;
-		}
-
+		mvebu_pcie_set_window(port, port->io_target, port->io_attr,
+				      &desired, &port->iowin);
 		return;
 	}
 
@@ -410,32 +436,27 @@ static void mvebu_pcie_handle_iobase_change(struct mvebu_pcie_port *port)
 	 * specifications. iobase is the bus address, port->iowin_base
 	 * is the CPU address.
 	 */
-	iobase = ((port->bridge.iobase & 0xF0) << 8) |
-		(port->bridge.iobaseupper << 16);
-	port->iowin_base = port->pcie->io.start + iobase;
-	port->iowin_size = ((0xFFF | ((port->bridge.iolimit & 0xF0) << 8) |
-			    (port->bridge.iolimitupper << 16)) -
-			    iobase) + 1;
-
-	mvebu_pcie_add_windows(port, port->io_target, port->io_attr,
-			       port->iowin_base, port->iowin_size,
-			       iobase);
+	desired.remap = ((port->bridge.iobase & 0xF0) << 8) |
+			(port->bridge.iobaseupper << 16);
+	desired.base = port->pcie->io.start + desired.remap;
+	desired.size = ((0xFFF | ((port->bridge.iolimit & 0xF0) << 8) |
+			 (port->bridge.iolimitupper << 16)) -
+			desired.remap) +
+		       1;
+
+	mvebu_pcie_set_window(port, port->io_target, port->io_attr, &desired,
+			      &port->iowin);
 }
 
 static void mvebu_pcie_handle_membase_change(struct mvebu_pcie_port *port)
 {
+	struct mvebu_pcie_window desired = {.remap = MVEBU_MBUS_NO_REMAP};
+
 	/* Are the new membase/memlimit values invalid? */
 	if (port->bridge.memlimit < port->bridge.membase ||
 	    !(port->bridge.command & PCI_COMMAND_MEMORY)) {
-
-		/* If a window was configured, remove it */
-		if (port->memwin_base) {
-			mvebu_pcie_del_windows(port, port->memwin_base,
-					       port->memwin_size);
-			port->memwin_base = 0;
-			port->memwin_size = 0;
-		}
-
+		mvebu_pcie_set_window(port, port->mem_target, port->mem_attr,
+				      &desired, &port->memwin);
 		return;
 	}
 
@@ -445,14 +466,12 @@ static void mvebu_pcie_handle_membase_change(struct mvebu_pcie_port *port)
 	 * window to setup, according to the PCI-to-PCI bridge
 	 * specifications.
 	 */
-	port->memwin_base  = ((port->bridge.membase & 0xFFF0) << 16);
-	port->memwin_size  =
-		(((port->bridge.memlimit & 0xFFF0) << 16) | 0xFFFFF) -
-		port->memwin_base + 1;
-
-	mvebu_pcie_add_windows(port, port->mem_target, port->mem_attr,
-			       port->memwin_base, port->memwin_size,
-			       MVEBU_MBUS_NO_REMAP);
+	desired.base = ((port->bridge.membase & 0xFFF0) << 16);
+	desired.size = (((port->bridge.memlimit & 0xFFF0) << 16) | 0xFFFFF) -
+		       desired.base + 1;
+
+	mvebu_pcie_set_window(port, port->mem_target, port->mem_attr, &desired,
+			      &port->memwin);
 }
 
 /*
-- 
2.11.0

Powered by blists - more mailing lists