[PATCH 0/7] Fixes for Armada 370/XP PCIe

[PATCH 0/7] Fixes for Armada 370/XP PCIe

[Thomas Petazzoni]

Hello,

This set of commits fixes a number of problems in the PCIe support of
the Armada 370 and Armada XP SoCs, allowing to use PCIe devices that
were not properly supported until now.

Due to the interaction of PCIe with other subsystems, the fixes are
not limited to drivers/pci, but also touch drivers/bus and
drivers/irqchip.

Here are the details of the patches:

 * The first three patches are fixes in the MSI handling. They fix
   problems with PCIe device drivers trying to use MSI-X (which we
   don't support), and incorrect freeing of MSIs causing kernel panics
   when PCIe device drivers try to allocate/free MSIs several times.

   They touch drivers/irqchip/ only, and they are independent from the
   rest of the series, both from a build and a runtime point of view.

   These bugs exist since the MSI support was added, in v3.13. The
   commits carry the necessary Fixes and Cc to stable informations.

 * The fourth patch fixes an off-by-one in the computed size of MBus
   windows. This only worked because the mvebu-mbus driver was
   silently accepting invalid sizes. I've marked it for stable because
   it really is bug, but even though it's not visible by itself, it is
   needed for other patches in the series.

   This patch touches drivers/pci/host, and should probably be taken
   by the mvebu maintainers, with the Ack of the PCI maintainer, as it
   is a runtime dependency for the next patch.

   This patch is marked for stable all the way to when the PCI driver
   was introduced.

 * The fifth patch make the mvebu-mbus driver check (and loudly
   complain) if it is asked to create invalid regions (whose base
   address or size are not compatible with the documented MBus
   requirements).

   This patch touches drivers/bus, and should probably be taken by the
   mvebu maintainers. As explained above, it is mandatory to have
   PATCH 4 applied before PATCH 5: there is a runtime dependency
   between the two (but no build time dependency).

   This patch is not marked for stable, as it only adds some
   additional debugging information, which while very useful, is not
   technically stable material.

 * The sixth patch relaxes a check in mvebu-mbus driver about
   conflicting windows, which is necessary

   This patch touches drivers/bus, and should probably be taken by the
   mvebu maintainers. It is not a fix by itself, but it is required
   for the last patch, which is a fix. Therefore, it is marked for
   stable, all the way to when the mvebu-mbus driver was introduced.

 * The seventh patch improves the pci-mvebu driver to allow the
   creation of multiple MBus windows when the PCI BAR calculated by
   the Linux PCI core has a non power-of-two size. This is needed
   because MBus windows can only have power-of-two sizes. Until now,
   we were configuring invalid MBus windows, which with certain PCIe
   devices, was either causing kernel panics at boot time or
   misfunction of the device.

These patches have been tested by Neil Greatorex
<neil@fatboyfat.co.uk> who initially reported the issues, have also
been tested by Willy Tarreau and Matthew Winter (even though they
didn't give their formal Tested-by [1] [2]), and have been heavily
discussed with Jason Gunthorpe.

Note that there are still two known issues in the mvebu PCIe handling:

 1/ When earlyprintk is disabled, we have a timing / PHY reset issue
    which leads to the PCIe device connected on the first interface to
    not be detected. We already have patches floating around, but
    nothing that we are all happy with yet.

 2/ When a PCI IGB card is plugged with many other PCI devices in a
    system, the IGB card initialization doesn't work ('igb
    0000:01:00.0: The NVM Checksum Is Not Valid'). We are still
    working on this issue.

These two issues will be handled separately.

Thanks,

Thomas

[1] http://www.spinics.net/lists/linux-pci/msg30406.html
[2] http://www.spinics.net/lists/linux-pci/msg30444.html

Jason Gunthorpe (1):
  bus: mvebu-mbus: Avoid setting an undefined window size

Neil Greatorex (1):
  irqchip: armada-370-xp: Fix releasing of MSIs

Thomas Petazzoni (4):
  irqchip: armada-370-xp: fix invalid cast of signed value into unsigned
    variable
  irqchip: armada-370-xp: implement the ->check_device() msi_chip
    operation
  bus: mvebu-mbus: allow several windows with the same target/attribute
  pci: pci-mvebu: split PCIe BARs into multiple MBus windows when needed

Willy Tarreau (1):
  pci: mvebu: fix off-by-one in the computed size of the mbus windows

 drivers/bus/mvebu-mbus.c            | 22 ++++++---
 drivers/irqchip/irq-armada-370-xp.c | 17 +++++--
 drivers/pci/host/pci-mvebu.c        | 92 ++++++++++++++++++++++++++++++-------
 3 files changed, 106 insertions(+), 25 deletions(-)

-- 
1.9.2

[PATCH 1/7] irqchip: armada-370-xp: fix invalid cast of signed value into unsigned variable

[Thomas Petazzoni]

The armada_370_xp_alloc_msi() function returns a signed int, which is
negative on error. However, we store the return value into an
irq_hw_number_t, which is unsigned. Therefore, we actually never test
if armada_370_xp_alloc_msi() returns an error or not, which may lead
us to use hwirq numbers of as 0xffffffe4 (when
armada_370_xp_alloc_msi() returns -ENOSPC).

This commit fixes that by storing the return value of
armada_370_xp_alloc_msi() in a signed variable.

Fixes: 31f614edb726fcc4d5aa0f2895fbdec9b04a3ca4 ('irqchip: armada-370-xp: implement MSI support')
Cc: <stable@vger.kernel.org> # v3.13+
Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
Tested-by: Neil Greatorex <neil@fatboyfat.co.uk>
---
 drivers/irqchip/irq-armada-370-xp.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/irqchip/irq-armada-370-xp.c b/drivers/irqchip/irq-armada-370-xp.c
index 41be897..3c8d89b 100644
--- a/drivers/irqchip/irq-armada-370-xp.c
+++ b/drivers/irqchip/irq-armada-370-xp.c
@@ -132,8 +132,7 @@ static int armada_370_xp_setup_msi_irq(struct msi_chip *chip,
 				       struct msi_desc *desc)
 {
 	struct msi_msg msg;
-	irq_hw_number_t hwirq;
-	int virq;
+	int virq, hwirq;
 
 	hwirq = armada_370_xp_alloc_msi();
 	if (hwirq < 0)
-- 
1.9.2

[PATCH 2/7] irqchip: armada-370-xp: implement the ->check_device() msi_chip operation

[Thomas Petazzoni]

Until now, we were leaving the ->check_device() msi_chip operation
empty, which leads the PCI core to believe that we support both MSI
and MSI-X. In fact, we do not support MSI-X, so we have to tell this
to the PCI core by providing an implementation of this operation.

Fixes: 31f614edb726fcc4d5aa0f2895fbdec9b04a3ca4 ('irqchip: armada-370-xp: implement MSI support')
Cc: <stable@vger.kernel.org> # v3.13+
Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
Tested-by: Neil Greatorex <neil@fatboyfat.co.uk>
---
 drivers/irqchip/irq-armada-370-xp.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/drivers/irqchip/irq-armada-370-xp.c b/drivers/irqchip/irq-armada-370-xp.c
index 3c8d89b..78b0ac9 100644
--- a/drivers/irqchip/irq-armada-370-xp.c
+++ b/drivers/irqchip/irq-armada-370-xp.c
@@ -162,6 +162,15 @@ static void armada_370_xp_teardown_msi_irq(struct msi_chip *chip,
 	armada_370_xp_free_msi(d->hwirq);
 }
 
+static int armada_370_xp_check_msi_device(struct msi_chip *chip, struct pci_dev *dev,
+					  int nvec, int type)
+{
+	/* We support MSI, but not MSI-X */
+	if (type == PCI_CAP_ID_MSI)
+		return 0;
+	return -EINVAL;
+}
+
 static struct irq_chip armada_370_xp_msi_irq_chip = {
 	.name = "armada_370_xp_msi_irq",
 	.irq_enable = unmask_msi_irq,
@@ -200,6 +209,7 @@ static int armada_370_xp_msi_init(struct device_node *node,
 
 	msi_chip->setup_irq = armada_370_xp_setup_msi_irq;
 	msi_chip->teardown_irq = armada_370_xp_teardown_msi_irq;
+	msi_chip->check_device = armada_370_xp_check_msi_device;
 	msi_chip->of_node = node;
 
 	armada_370_xp_msi_domain =
-- 
1.9.2

[PATCH 3/7] irqchip: armada-370-xp: Fix releasing of MSIs

[Thomas Petazzoni]

From: Neil Greatorex <neil@fatboyfat.co.uk>

Store the value of d->hwirq in a local variable as the real value is wiped out
by calling irq_dispose_mapping. Without this patch, the armada_370_xp_free_msi
function would always free MSI#0, no matter what was passed to it.

Fixes: 31f614edb726fcc4d5aa0f2895fbdec9b04a3ca4 ('irqchip: armada-370-xp: implement MSI support')
Cc: <stable@vger.kernel.org> # v3.13+
Signed-off-by: Neil Greatorex <neil@fatboyfat.co.uk>
Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
---
 drivers/irqchip/irq-armada-370-xp.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/irqchip/irq-armada-370-xp.c b/drivers/irqchip/irq-armada-370-xp.c
index 78b0ac9..868d11b 100644
--- a/drivers/irqchip/irq-armada-370-xp.c
+++ b/drivers/irqchip/irq-armada-370-xp.c
@@ -158,8 +158,10 @@ static void armada_370_xp_teardown_msi_irq(struct msi_chip *chip,
 					   unsigned int irq)
 {
 	struct irq_data *d = irq_get_irq_data(irq);
+	unsigned long hwirq = d->hwirq;
+
 	irq_dispose_mapping(irq);
-	armada_370_xp_free_msi(d->hwirq);
+	armada_370_xp_free_msi(hwirq);
 }
 
 static int armada_370_xp_check_msi_device(struct msi_chip *chip, struct pci_dev *dev,
-- 
1.9.2

[PATCH 4/7] pci: mvebu: fix off-by-one in the computed size of the mbus windows

[Thomas Petazzoni]

From: Willy Tarreau <w@1wt.eu>

mvebu_pcie_handle_membase_change() and
mvebu_pcie_handle_iobase_change() do not correctly compute the window
size. PCI uses an inclusive start/end address pair, which requires a
+1 when converting to size.

This only worked because a bug in the mbus driver allowed it to
silently accept and round up bogus sizes.

Fix this by adding one to the computed size.

Fixes: 45361a4fe4464180815157654aabbd2afb4848ad ('PCIe driver for Marvell Armada 370/XP systems')
Cc: <stable@vger.kernel.org> # v3.11+
Signed-off-by: Willy Tarreau <w@1wt.eu>
Reviewed-By: Jason Gunthorpe <jgunthorpe@obsidianresearch.com>
Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
Tested-by: Neil Greatorex <neil@fatboyfat.co.uk>
---
 drivers/pci/host/pci-mvebu.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/pci/host/pci-mvebu.c b/drivers/pci/host/pci-mvebu.c
index d3d1cfd..4829921 100644
--- a/drivers/pci/host/pci-mvebu.c
+++ b/drivers/pci/host/pci-mvebu.c
@@ -331,7 +331,7 @@ static void mvebu_pcie_handle_iobase_change(struct mvebu_pcie_port *port)
 	port->iowin_base = port->pcie->io.start + iobase;
 	port->iowin_size = ((0xFFF | ((port->bridge.iolimit & 0xF0) << 8) |
 			    (port->bridge.iolimitupper << 16)) -
-			    iobase);
+			    iobase) + 1;
 
 	mvebu_mbus_add_window_remap_by_id(port->io_target, port->io_attr,
 					  port->iowin_base, port->iowin_size,
@@ -364,7 +364,7 @@ static void mvebu_pcie_handle_membase_change(struct mvebu_pcie_port *port)
 	port->memwin_base  = ((port->bridge.membase & 0xFFF0) << 16);
 	port->memwin_size  =
 		(((port->bridge.memlimit & 0xFFF0) << 16) | 0xFFFFF) -
-		port->memwin_base;
+		port->memwin_base + 1;
 
 	mvebu_mbus_add_window_by_id(port->mem_target, port->mem_attr,
 				    port->memwin_base, port->memwin_size);
-- 
1.9.2

[PATCH 5/7] bus: mvebu-mbus: Avoid setting an undefined window size

[Thomas Petazzoni]

From: Jason Gunthorpe <jgunthorpe@obsidianresearch.com>

The mbus hardware requires a power of two size, and size aligned base.
Currently, if a non-power of two is passed in to the low level routines
they configure the register in a way that results in undefined behaviour.

Call WARN and return EINVAL instead.

Also, update the debugfs routines to show a message if there is an
invalid register setting.

All together this makes the recent problems with silent failure
of PCI very obvious, noisy and debuggable.

Signed-off-by: Jason Gunthorpe <jgunthorpe@obsidianresearch.com>
Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
---
 drivers/bus/mvebu-mbus.c | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

diff --git a/drivers/bus/mvebu-mbus.c b/drivers/bus/mvebu-mbus.c
index 293e2e0..afee0f7 100644
--- a/drivers/bus/mvebu-mbus.c
+++ b/drivers/bus/mvebu-mbus.c
@@ -56,6 +56,7 @@
 #include <linux/of.h>
 #include <linux/of_address.h>
 #include <linux/debugfs.h>
+#include <linux/log2.h>
 
 /*
  * DDR target is the same on all platforms.
@@ -266,6 +267,17 @@ static int mvebu_mbus_setup_window(struct mvebu_mbus_state *mbus,
 		mbus->soc->win_cfg_offset(win);
 	u32 ctrl, remap_addr;
 
+	if (!is_power_of_2(size)) {
+		WARN(true, "Invalid MBus window size: 0x%zx\n", size);
+		return -EINVAL;
+	}
+
+	if ((base & (phys_addr_t)(size - 1)) != 0) {
+		WARN(true, "Invalid MBus base/size: %pa len 0x%zx\n", &base,
+		     size);
+		return -EINVAL;
+	}
+
 	ctrl = ((size - 1) & WIN_CTRL_SIZE_MASK) |
 		(attr << WIN_CTRL_ATTR_SHIFT)    |
 		(target << WIN_CTRL_TGT_SHIFT)   |
@@ -413,6 +425,10 @@ static int mvebu_devs_debug_show(struct seq_file *seq, void *v)
 			   win, (unsigned long long)wbase,
 			   (unsigned long long)(wbase + wsize), wtarget, wattr);
 
+		if (!is_power_of_2(wsize) ||
+		    ((wbase & (u64)(wsize - 1)) != 0))
+			seq_puts(seq, " (Invalid base/size!!)");
+
 		if (win < mbus->soc->num_remappable_wins) {
 			seq_printf(seq, " (remap %016llx)\n",
 				   (unsigned long long)wremap);
-- 
1.9.2

[PATCH 6/7] bus: mvebu-mbus: allow several windows with the same target/attribute

[Thomas Petazzoni]

Having multiple windows with the same target and attribute is actually
legal, and can be useful for PCIe windows, when PCIe BARs have a size
that isn't a power of two, and we therefore need to create several
MBus windows to cover the PCIe BAR for a given PCIe interface.

Fixes: fddddb52a6c4e2438f4514ed979183653ca0732a ('bus: introduce an Marvell EBU MBus driver')
Cc: <stable@vger.kernel.org> # v3.10+
Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
Tested-by: Neil Greatorex <neil@fatboyfat.co.uk>
---
 drivers/bus/mvebu-mbus.c | 6 ------
 1 file changed, 6 deletions(-)

diff --git a/drivers/bus/mvebu-mbus.c b/drivers/bus/mvebu-mbus.c
index afee0f7..00b7344 100644
--- a/drivers/bus/mvebu-mbus.c
+++ b/drivers/bus/mvebu-mbus.c
@@ -223,12 +223,6 @@ static int mvebu_mbus_window_conflicts(struct mvebu_mbus_state *mbus,
 		 */
 		if ((u64)base < wend && end > wbase)
 			return 0;
-
-		/*
-		 * Check if target/attribute conflicts
-		 */
-		if (target == wtarget && attr == wattr)
-			return 0;
 	}
 
 	return 1;
-- 
1.9.2

[PATCH 7/7] pci: pci-mvebu: split PCIe BARs into multiple MBus windows when needed

[Thomas Petazzoni]

MBus windows are used on Marvell platforms to map certain peripherals
in the physical address space. In the PCIe context, MBus windows are
needed to map PCIe I/O and memory regions in the physical address.

However, those MBus windows can only have power of two sizes, while
PCIe BAR do not necessarily guarantee this. For this reason, the
current pci-mvebu breaks on platforms where PCIe devices have BARs
that don't sum up to a power of two size at the emulated bridge level.

This commit fixes this by allowing the pci-mvebu driver to create
multiple contiguous MBus windows (each having a power of two size) to
cover a given PCIe BAR.

To achieve this, two functions are added: mvebu_pcie_add_windows() and
mvebu_pcie_del_windows() to respectively add and remove all the MBus
windows that are needed to map the provided PCIe region base and
size. The emulated PCI bridge code now calls those functions, instead
of directly calling the mvebu-mbus driver functions.

Fixes: 45361a4fe4464180815157654aabbd2afb4848ad ('pci: PCIe driver for Marvell Armada 370/XP systems')
Cc: <stable@vger.kernel.org> # v3.11+
Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
Tested-by: Neil Greatorex <neil@fatboyfat.co.uk>
---
 drivers/pci/host/pci-mvebu.c | 88 +++++++++++++++++++++++++++++++++++++-------
 1 file changed, 74 insertions(+), 14 deletions(-)

diff --git a/drivers/pci/host/pci-mvebu.c b/drivers/pci/host/pci-mvebu.c
index 4829921..e384e25 100644
--- a/drivers/pci/host/pci-mvebu.c
+++ b/drivers/pci/host/pci-mvebu.c
@@ -293,6 +293,58 @@ static int mvebu_pcie_hw_wr_conf(struct mvebu_pcie_port *port,
 	return PCIBIOS_SUCCESSFUL;
 }
 
+/*
+ * Remove windows, starting from the largest ones to the smallest
+ * ones.
+ */
+static void mvebu_pcie_del_windows(struct mvebu_pcie_port *port,
+				   phys_addr_t base, size_t size)
+{
+	while (size) {
+		size_t sz = 1 << (fls(size) - 1);
+
+		mvebu_mbus_del_window(base, sz);
+		base += sz;
+		size -= sz;
+	}
+}
+
+/*
+ * MBus windows can only have a power of two size, but PCI BARs do not
+ * have this constraint. Therefore, we have to split the PCI BAR into
+ * areas each having a power of two size. We start from the largest
+ * one (i.e highest order bit set in the size).
+ */
+static void mvebu_pcie_add_windows(struct mvebu_pcie_port *port,
+				   unsigned int target, unsigned int attribute,
+				   phys_addr_t base, size_t size,
+				   phys_addr_t remap)
+{
+	size_t size_mapped = 0;
+
+	while (size) {
+		size_t sz = 1 << (fls(size) - 1);
+		int ret;
+
+		ret = mvebu_mbus_add_window_remap_by_id(target, attribute, base,
+							sz, remap);
+		if (ret) {
+			dev_err(&port->pcie->pdev->dev,
+				"Could not create MBus window at 0x%x, size 0x%x: %d\n",
+				base, sz, ret);
+			mvebu_pcie_del_windows(port, base - size_mapped,
+					       size_mapped);
+			return;
+		}
+
+		size -= sz;
+		size_mapped += sz;
+		base += sz;
+		if (remap != MVEBU_MBUS_NO_REMAP)
+			remap += sz;
+	}
+}
+
 static void mvebu_pcie_handle_iobase_change(struct mvebu_pcie_port *port)
 {
 	phys_addr_t iobase;
@@ -304,8 +356,8 @@ static void mvebu_pcie_handle_iobase_change(struct mvebu_pcie_port *port)
 
 		/* If a window was configured, remove it */
 		if (port->iowin_base) {
-			mvebu_mbus_del_window(port->iowin_base,
-					      port->iowin_size);
+			mvebu_pcie_del_windows(port, port->iowin_base,
+					       port->iowin_size);
 			port->iowin_base = 0;
 			port->iowin_size = 0;
 		}
@@ -333,9 +385,9 @@ static void mvebu_pcie_handle_iobase_change(struct mvebu_pcie_port *port)
 			    (port->bridge.iolimitupper << 16)) -
 			    iobase) + 1;
 
-	mvebu_mbus_add_window_remap_by_id(port->io_target, port->io_attr,
-					  port->iowin_base, port->iowin_size,
-					  iobase);
+	mvebu_pcie_add_windows(port, port->io_target, port->io_attr,
+			       port->iowin_base, port->iowin_size,
+			       iobase);
 }
 
 static void mvebu_pcie_handle_membase_change(struct mvebu_pcie_port *port)
@@ -346,8 +398,8 @@ static void mvebu_pcie_handle_membase_change(struct mvebu_pcie_port *port)
 
 		/* If a window was configured, remove it */
 		if (port->memwin_base) {
-			mvebu_mbus_del_window(port->memwin_base,
-					      port->memwin_size);
+			mvebu_pcie_del_windows(port, port->memwin_base,
+					       port->memwin_size);
 			port->memwin_base = 0;
 			port->memwin_size = 0;
 		}
@@ -366,8 +418,9 @@ static void mvebu_pcie_handle_membase_change(struct mvebu_pcie_port *port)
 		(((port->bridge.memlimit & 0xFFF0) << 16) | 0xFFFFF) -
 		port->memwin_base + 1;
 
-	mvebu_mbus_add_window_by_id(port->mem_target, port->mem_attr,
-				    port->memwin_base, port->memwin_size);
+	mvebu_pcie_add_windows(port, port->mem_target, port->mem_attr,
+			       port->memwin_base, port->memwin_size,
+			       MVEBU_MBUS_NO_REMAP);
 }
 
 /*
@@ -743,14 +796,21 @@ static resource_size_t mvebu_pcie_align_resource(struct pci_dev *dev,
 
 	/*
 	 * On the PCI-to-PCI bridge side, the I/O windows must have at
-	 * least a 64 KB size and be aligned on their size, and the
-	 * memory windows must have at least a 1 MB size and be
-	 * aligned on their size
+	 * least a 64 KB size and the memory windows must have at
+	 * least a 1 MB size. Moreover, MBus windows need to have a
+	 * base address aligned on their size, and their size must be
+	 * a power of two. This means that if the BAR doesn't have a
+	 * power of two size, several MBus windows will actually be
+	 * created. We need to ensure that the biggest MBus window
+	 * (which will be the first one) is aligned on its size, which
+	 * explains the rounddown_pow_of_two() being done here.
 	 */
 	if (res->flags & IORESOURCE_IO)
-		return round_up(start, max_t(resource_size_t, SZ_64K, size));
+		return round_up(start, max_t(resource_size_t, SZ_64K,
+					     rounddown_pow_of_two(size)));
 	else if (res->flags & IORESOURCE_MEM)
-		return round_up(start, max_t(resource_size_t, SZ_1M, size));
+		return round_up(start, max_t(resource_size_t, SZ_1M,
+					     rounddown_pow_of_two(size)));
 	else
 		return start;
 }
-- 
1.9.2

[PATCH 0/7] Fixes for Armada 370/XP PCIe

[Jason Cooper]

Bjorn,

On Fri, Apr 18, 2014 at 02:19:46PM +0200, Thomas Petazzoni wrote:
> Hello,
> 
> This set of commits fixes a number of problems in the PCIe support of
> the Armada 370 and Armada XP SoCs, allowing to use PCIe devices that
> were not properly supported until now.
> 
> Due to the interaction of PCIe with other subsystems, the fixes are
> not limited to drivers/pci, but also touch drivers/bus and
> drivers/irqchip.

There is no official maintainer for drivers/bus, so we usually take
those throught the arm-soc tree.  Thomas (tglx) takes pull requests of
branches mvebu sets up for him, so the irqchip stuff is handled.

Would you mind Acking #4 and #7, particularly #7 for us?  It would sure
simplify getting this set of fixes into mainline.

> Here are the details of the patches:
> 
>  * The first three patches are fixes in the MSI handling. They fix
>    problems with PCIe device drivers trying to use MSI-X (which we
>    don't support), and incorrect freeing of MSIs causing kernel panics
>    when PCIe device drivers try to allocate/free MSIs several times.
> 
>    They touch drivers/irqchip/ only, and they are independent from the
>    rest of the series, both from a build and a runtime point of view.
> 
>    These bugs exist since the MSI support was added, in v3.13. The
>    commits carry the necessary Fixes and Cc to stable informations.
> 
>  * The fourth patch fixes an off-by-one in the computed size of MBus
>    windows. This only worked because the mvebu-mbus driver was
>    silently accepting invalid sizes. I've marked it for stable because
>    it really is bug, but even though it's not visible by itself, it is
>    needed for other patches in the series.
> 
>    This patch touches drivers/pci/host, and should probably be taken
>    by the mvebu maintainers, with the Ack of the PCI maintainer, as it
>    is a runtime dependency for the next patch.
> 
>    This patch is marked for stable all the way to when the PCI driver
>    was introduced.
> 
>  * The fifth patch make the mvebu-mbus driver check (and loudly
>    complain) if it is asked to create invalid regions (whose base
>    address or size are not compatible with the documented MBus
>    requirements).
> 
>    This patch touches drivers/bus, and should probably be taken by the
>    mvebu maintainers. As explained above, it is mandatory to have
>    PATCH 4 applied before PATCH 5: there is a runtime dependency
>    between the two (but no build time dependency).
> 
>    This patch is not marked for stable, as it only adds some
>    additional debugging information, which while very useful, is not
>    technically stable material.
> 
>  * The sixth patch relaxes a check in mvebu-mbus driver about
>    conflicting windows, which is necessary
> 
>    This patch touches drivers/bus, and should probably be taken by the
>    mvebu maintainers. It is not a fix by itself, but it is required
>    for the last patch, which is a fix. Therefore, it is marked for
>    stable, all the way to when the mvebu-mbus driver was introduced.
> 
>  * The seventh patch improves the pci-mvebu driver to allow the
>    creation of multiple MBus windows when the PCI BAR calculated by
>    the Linux PCI core has a non power-of-two size. This is needed
>    because MBus windows can only have power-of-two sizes. Until now,
>    we were configuring invalid MBus windows, which with certain PCIe
>    devices, was either causing kernel panics at boot time or
>    misfunction of the device.
...
> Jason Gunthorpe (1):
>   bus: mvebu-mbus: Avoid setting an undefined window size
> 
> Neil Greatorex (1):
>   irqchip: armada-370-xp: Fix releasing of MSIs
> 
> Thomas Petazzoni (4):
>   irqchip: armada-370-xp: fix invalid cast of signed value into unsigned
>     variable
>   irqchip: armada-370-xp: implement the ->check_device() msi_chip
>     operation
>   bus: mvebu-mbus: allow several windows with the same target/attribute
>   pci: pci-mvebu: split PCIe BARs into multiple MBus windows when needed
> 
> Willy Tarreau (1):
>   pci: mvebu: fix off-by-one in the computed size of the mbus windows
> 
>  drivers/bus/mvebu-mbus.c            | 22 ++++++---
>  drivers/irqchip/irq-armada-370-xp.c | 17 +++++--
>  drivers/pci/host/pci-mvebu.c        | 92 ++++++++++++++++++++++++++++++-------
>  3 files changed, 106 insertions(+), 25 deletions(-)

thx,

Jason.

[PATCH 0/7] Fixes for Armada 370/XP PCIe

[Jason Cooper]

Thomas,

On Fri, Apr 18, 2014 at 02:19:46PM +0200, Thomas Petazzoni wrote:
> Hello,
> 
> This set of commits fixes a number of problems in the PCIe support of
> the Armada 370 and Armada XP SoCs, allowing to use PCIe devices that
> were not properly supported until now.
> 
> Due to the interaction of PCIe with other subsystems, the fixes are
> not limited to drivers/pci, but also touch drivers/bus and
> drivers/irqchip.

I've tentatively pulled this series as follows:

  patches 1-3 into mvebu/drivers-irqchip-fixes

  patches 4-7 into mvebu/drivers-mbus_pci-fixes

with the latter depending on the former.  I'm going to hold them in
-next for a bit.  At least until we get an Ack from Bjorn.  Then I'll
send them their appropriate ways.

thx,

Jason.

[PATCH 0/7] Fixes for Armada 370/XP PCIe

[Thomas Petazzoni]

Dear Jason Cooper,

On Sun, 20 Apr 2014 16:04:09 -0400, Jason Cooper wrote:

> I've tentatively pulled this series as follows:
> 
>   patches 1-3 into mvebu/drivers-irqchip-fixes
> 
>   patches 4-7 into mvebu/drivers-mbus_pci-fixes

Thanks!

> with the latter depending on the former.  I'm going to hold them in
> -next for a bit.  At least until we get an Ack from Bjorn.  Then I'll
> send them their appropriate ways.

There is actually no need for a dependency between the two. Neither
from a build point of view, or a runtime point of view. 1-3 fix purely
MSI related things, while 4-7 fix purely MBus/PCI related things. I
don't if not having a dependency will make things easier for you, but I
thought it was worth mentioning. The only reason why all those patches
were together is that, as a whole, they allow to fix a number of
problems with certain PCIe devices.

Thanks!

Thomas
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

[PATCH 0/7] Fixes for Armada 370/XP PCIe

[Jason Cooper]

Thomas,

On Sun, Apr 20, 2014 at 11:08:27PM +0200, Thomas Petazzoni wrote:
> On Sun, 20 Apr 2014 16:04:09 -0400, Jason Cooper wrote:
> 
> > I've tentatively pulled this series as follows:
> > 
> >   patches 1-3 into mvebu/drivers-irqchip-fixes
> > 
> >   patches 4-7 into mvebu/drivers-mbus_pci-fixes
> 
> Thanks!
> 
> > with the latter depending on the former.  I'm going to hold them in
> > -next for a bit.  At least until we get an Ack from Bjorn.  Then I'll
> > send them their appropriate ways.
> 
> There is actually no need for a dependency between the two. Neither
> from a build point of view, or a runtime point of view. 1-3 fix purely
> MSI related things, while 4-7 fix purely MBus/PCI related things. I
> don't if not having a dependency will make things easier for you, but I
> thought it was worth mentioning. The only reason why all those patches
> were together is that, as a whole, they allow to fix a number of
> problems with certain PCIe devices.

Ok, I'll make them two separate branches then.  Thanks for clarifying
(which I'm sure you already did somewhere, but it's been a few days).

thx,

Jason.

[PATCH 4/7] pci: mvebu: fix off-by-one in the computed size of the mbus windows

[Bjorn Helgaas]

On Fri, Apr 18, 2014 at 02:19:50PM +0200, Thomas Petazzoni wrote:
> From: Willy Tarreau <w@1wt.eu>
> 
> mvebu_pcie_handle_membase_change() and
> mvebu_pcie_handle_iobase_change() do not correctly compute the window
> size. PCI uses an inclusive start/end address pair, which requires a
> +1 when converting to size.
> 
> This only worked because a bug in the mbus driver allowed it to
> silently accept and round up bogus sizes.
> 
> Fix this by adding one to the computed size.
> 
> Fixes: 45361a4fe4464180815157654aabbd2afb4848ad ('PCIe driver for Marvell Armada 370/XP systems')
> Cc: <stable@vger.kernel.org> # v3.11+
> Signed-off-by: Willy Tarreau <w@1wt.eu>
> Reviewed-By: Jason Gunthorpe <jgunthorpe@obsidianresearch.com>
> Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
> Tested-by: Neil Greatorex <neil@fatboyfat.co.uk>

Acked-by: Bjorn Helgaas <bhelgaas@google.com>

If I were merging via my tree, I would tweak the subject capitalization
so "git log --oneline drivers/pci/host/pci-mvebu.c" looks consistent.

> ---
>  drivers/pci/host/pci-mvebu.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/pci/host/pci-mvebu.c b/drivers/pci/host/pci-mvebu.c
> index d3d1cfd..4829921 100644
> --- a/drivers/pci/host/pci-mvebu.c
> +++ b/drivers/pci/host/pci-mvebu.c
> @@ -331,7 +331,7 @@ static void mvebu_pcie_handle_iobase_change(struct mvebu_pcie_port *port)
>  	port->iowin_base = port->pcie->io.start + iobase;
>  	port->iowin_size = ((0xFFF | ((port->bridge.iolimit & 0xF0) << 8) |
>  			    (port->bridge.iolimitupper << 16)) -
> -			    iobase);
> +			    iobase) + 1;
>  
>  	mvebu_mbus_add_window_remap_by_id(port->io_target, port->io_attr,
>  					  port->iowin_base, port->iowin_size,
> @@ -364,7 +364,7 @@ static void mvebu_pcie_handle_membase_change(struct mvebu_pcie_port *port)
>  	port->memwin_base  = ((port->bridge.membase & 0xFFF0) << 16);
>  	port->memwin_size  =
>  		(((port->bridge.memlimit & 0xFFF0) << 16) | 0xFFFFF) -
> -		port->memwin_base;
> +		port->memwin_base + 1;
>  
>  	mvebu_mbus_add_window_by_id(port->mem_target, port->mem_attr,
>  				    port->memwin_base, port->memwin_size);
> -- 
> 1.9.2
> 

[PATCH 7/7] pci: pci-mvebu: split PCIe BARs into multiple MBus windows when needed

[Bjorn Helgaas]

On Fri, Apr 18, 2014 at 02:19:53PM +0200, Thomas Petazzoni wrote:
> MBus windows are used on Marvell platforms to map certain peripherals
> in the physical address space. In the PCIe context, MBus windows are
> needed to map PCIe I/O and memory regions in the physical address.
> 
> However, those MBus windows can only have power of two sizes, while
> PCIe BAR do not necessarily guarantee this. For this reason, the
> current pci-mvebu breaks on platforms where PCIe devices have BARs
> that don't sum up to a power of two size at the emulated bridge level.
> 
> This commit fixes this by allowing the pci-mvebu driver to create
> multiple contiguous MBus windows (each having a power of two size) to
> cover a given PCIe BAR.
> 
> To achieve this, two functions are added: mvebu_pcie_add_windows() and
> mvebu_pcie_del_windows() to respectively add and remove all the MBus
> windows that are needed to map the provided PCIe region base and
> size. The emulated PCI bridge code now calls those functions, instead
> of directly calling the mvebu-mbus driver functions.
> 
> Fixes: 45361a4fe4464180815157654aabbd2afb4848ad ('pci: PCIe driver for Marvell Armada 370/XP systems')
> Cc: <stable@vger.kernel.org> # v3.11+
> Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
> Tested-by: Neil Greatorex <neil@fatboyfat.co.uk>

Acked-by: Bjorn Helgaas <bhelgaas@google.com>

> ---
>  drivers/pci/host/pci-mvebu.c | 88 +++++++++++++++++++++++++++++++++++++-------
>  1 file changed, 74 insertions(+), 14 deletions(-)
> 
> diff --git a/drivers/pci/host/pci-mvebu.c b/drivers/pci/host/pci-mvebu.c
> index 4829921..e384e25 100644
> --- a/drivers/pci/host/pci-mvebu.c
> +++ b/drivers/pci/host/pci-mvebu.c
> @@ -293,6 +293,58 @@ static int mvebu_pcie_hw_wr_conf(struct mvebu_pcie_port *port,
>  	return PCIBIOS_SUCCESSFUL;
>  }
>  
> +/*
> + * Remove windows, starting from the largest ones to the smallest
> + * ones.
> + */
> +static void mvebu_pcie_del_windows(struct mvebu_pcie_port *port,
> +				   phys_addr_t base, size_t size)
> +{
> +	while (size) {
> +		size_t sz = 1 << (fls(size) - 1);
> +
> +		mvebu_mbus_del_window(base, sz);
> +		base += sz;
> +		size -= sz;
> +	}
> +}
> +
> +/*
> + * MBus windows can only have a power of two size, but PCI BARs do not
> + * have this constraint. Therefore, we have to split the PCI BAR into
> + * areas each having a power of two size. We start from the largest
> + * one (i.e highest order bit set in the size).
> + */
> +static void mvebu_pcie_add_windows(struct mvebu_pcie_port *port,
> +				   unsigned int target, unsigned int attribute,
> +				   phys_addr_t base, size_t size,
> +				   phys_addr_t remap)
> +{
> +	size_t size_mapped = 0;
> +
> +	while (size) {
> +		size_t sz = 1 << (fls(size) - 1);
> +		int ret;
> +
> +		ret = mvebu_mbus_add_window_remap_by_id(target, attribute, base,
> +							sz, remap);
> +		if (ret) {
> +			dev_err(&port->pcie->pdev->dev,
> +				"Could not create MBus window at 0x%x, size 0x%x: %d\n",
> +				base, sz, ret);
> +			mvebu_pcie_del_windows(port, base - size_mapped,
> +					       size_mapped);
> +			return;
> +		}
> +
> +		size -= sz;
> +		size_mapped += sz;
> +		base += sz;
> +		if (remap != MVEBU_MBUS_NO_REMAP)
> +			remap += sz;
> +	}
> +}
> +
>  static void mvebu_pcie_handle_iobase_change(struct mvebu_pcie_port *port)
>  {
>  	phys_addr_t iobase;
> @@ -304,8 +356,8 @@ static void mvebu_pcie_handle_iobase_change(struct mvebu_pcie_port *port)
>  
>  		/* If a window was configured, remove it */
>  		if (port->iowin_base) {
> -			mvebu_mbus_del_window(port->iowin_base,
> -					      port->iowin_size);
> +			mvebu_pcie_del_windows(port, port->iowin_base,
> +					       port->iowin_size);
>  			port->iowin_base = 0;
>  			port->iowin_size = 0;
>  		}
> @@ -333,9 +385,9 @@ static void mvebu_pcie_handle_iobase_change(struct mvebu_pcie_port *port)
>  			    (port->bridge.iolimitupper << 16)) -
>  			    iobase) + 1;
>  
> -	mvebu_mbus_add_window_remap_by_id(port->io_target, port->io_attr,
> -					  port->iowin_base, port->iowin_size,
> -					  iobase);
> +	mvebu_pcie_add_windows(port, port->io_target, port->io_attr,
> +			       port->iowin_base, port->iowin_size,
> +			       iobase);
>  }
>  
>  static void mvebu_pcie_handle_membase_change(struct mvebu_pcie_port *port)
> @@ -346,8 +398,8 @@ static void mvebu_pcie_handle_membase_change(struct mvebu_pcie_port *port)
>  
>  		/* If a window was configured, remove it */
>  		if (port->memwin_base) {
> -			mvebu_mbus_del_window(port->memwin_base,
> -					      port->memwin_size);
> +			mvebu_pcie_del_windows(port, port->memwin_base,
> +					       port->memwin_size);
>  			port->memwin_base = 0;
>  			port->memwin_size = 0;
>  		}
> @@ -366,8 +418,9 @@ static void mvebu_pcie_handle_membase_change(struct mvebu_pcie_port *port)
>  		(((port->bridge.memlimit & 0xFFF0) << 16) | 0xFFFFF) -
>  		port->memwin_base + 1;
>  
> -	mvebu_mbus_add_window_by_id(port->mem_target, port->mem_attr,
> -				    port->memwin_base, port->memwin_size);
> +	mvebu_pcie_add_windows(port, port->mem_target, port->mem_attr,
> +			       port->memwin_base, port->memwin_size,
> +			       MVEBU_MBUS_NO_REMAP);
>  }
>  
>  /*
> @@ -743,14 +796,21 @@ static resource_size_t mvebu_pcie_align_resource(struct pci_dev *dev,
>  
>  	/*
>  	 * On the PCI-to-PCI bridge side, the I/O windows must have at
> -	 * least a 64 KB size and be aligned on their size, and the
> -	 * memory windows must have at least a 1 MB size and be
> -	 * aligned on their size
> +	 * least a 64 KB size and the memory windows must have at
> +	 * least a 1 MB size. Moreover, MBus windows need to have a
> +	 * base address aligned on their size, and their size must be
> +	 * a power of two. This means that if the BAR doesn't have a
> +	 * power of two size, several MBus windows will actually be
> +	 * created. We need to ensure that the biggest MBus window
> +	 * (which will be the first one) is aligned on its size, which
> +	 * explains the rounddown_pow_of_two() being done here.
>  	 */
>  	if (res->flags & IORESOURCE_IO)
> -		return round_up(start, max_t(resource_size_t, SZ_64K, size));
> +		return round_up(start, max_t(resource_size_t, SZ_64K,
> +					     rounddown_pow_of_two(size)));
>  	else if (res->flags & IORESOURCE_MEM)
> -		return round_up(start, max_t(resource_size_t, SZ_1M, size));
> +		return round_up(start, max_t(resource_size_t, SZ_1M,
> +					     rounddown_pow_of_two(size)));
>  	else
>  		return start;
>  }
> -- 
> 1.9.2
> 

[PATCH 4/7] pci: mvebu: fix off-by-one in the computed size of the mbus windows

[Jason Cooper]

On Mon, Apr 21, 2014 at 10:47:54AM -0600, Bjorn Helgaas wrote:
> On Fri, Apr 18, 2014 at 02:19:50PM +0200, Thomas Petazzoni wrote:
> > From: Willy Tarreau <w@1wt.eu>
> > 
> > mvebu_pcie_handle_membase_change() and
> > mvebu_pcie_handle_iobase_change() do not correctly compute the window
> > size. PCI uses an inclusive start/end address pair, which requires a
> > +1 when converting to size.
> > 
> > This only worked because a bug in the mbus driver allowed it to
> > silently accept and round up bogus sizes.
> > 
> > Fix this by adding one to the computed size.
> > 
> > Fixes: 45361a4fe4464180815157654aabbd2afb4848ad ('PCIe driver for Marvell Armada 370/XP systems')
> > Cc: <stable@vger.kernel.org> # v3.11+
> > Signed-off-by: Willy Tarreau <w@1wt.eu>
> > Reviewed-By: Jason Gunthorpe <jgunthorpe@obsidianresearch.com>
> > Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
> > Tested-by: Neil Greatorex <neil@fatboyfat.co.uk>
> 
> Acked-by: Bjorn Helgaas <bhelgaas@google.com>
> 
> If I were merging via my tree, I would tweak the subject capitalization
> so "git log --oneline drivers/pci/host/pci-mvebu.c" looks consistent.

Will do, thanks for the reminder.

thx,

Jason.