Re: Intel I350 mini-PCIe card (igb) on Mirabox (mvebu / Armada 370)

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hello all,

On Tue, 8 Apr 2014 17:13:09 +0200, Thomas Petazzoni wrote:

> Unfortunately here your patch is not sufficient to solve the problem
> apparently. I've fixed another problem where the return value of
> armada_370_xp_alloc_msi() (which is signed) is casted into an unsigned
> irq_hw_number_t in armada_370_xp_setup_msi_irq(), but I'm still seeing
> many MSIs allocated, then some freed, and finally a kernel panic.
> 
>  * Log: https://gist.github.com/tpetazzoni/10140012
> 
>  * Diff against v3.14: https://gist.github.com/tpetazzoni/10140121
> 
> Ideas? If some of you are interested in discussing this live, I'm on
> #mvlinux on Freenode.

Please find attached the 3 patches I'm currently using on the
irq-armada-370-xp. They make the situation a bit better (the igb driver
no longer believes we support MSI-X), but it still crashes in the igb
driver (see https://gist.github.com/tpetazzoni/10144886).

Thomas
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
>From e85d22477eab2f2e3d7a0de0d32ea1bc3b6f4412 Mon Sep 17 00:00:00 2001
From: Thomas Petazzoni <thomas.petazzoni@xxxxxxxxxxxxxxxxxx>
Date: Tue, 8 Apr 2014 17:33:31 +0200
Subject: [PATCH 1/3] irqchip: armada-370-xp: fix invalid cast of signed value
 into unsigned variable

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.

Signed-off-by: Thomas Petazzoni <thomas.petazzoni@xxxxxxxxxxxxxxxxxx>
---
 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 5409564..7858729 100644
--- a/drivers/irqchip/irq-armada-370-xp.c
+++ b/drivers/irqchip/irq-armada-370-xp.c
@@ -130,8 +130,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.8.3.2

>From c9e4a6512fb7e41b13ab7f038732777d268ceee6 Mon Sep 17 00:00:00 2001
From: Thomas Petazzoni <thomas.petazzoni@xxxxxxxxxxxxxxxxxx>
Date: Tue, 8 Apr 2014 17:35:23 +0200
Subject: [PATCH 2/3] irqchip: armada-370-xp: implement the ->check_device()
 msi_chip operation

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.

Signed-off-by: Thomas Petazzoni <thomas.petazzoni@xxxxxxxxxxxxxxxxxx>
---
 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 7858729..5d925af 100644
--- a/drivers/irqchip/irq-armada-370-xp.c
+++ b/drivers/irqchip/irq-armada-370-xp.c
@@ -160,6 +160,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,
@@ -198,6 +207,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.8.3.2

>From 9752c7d275f3647dde7ac52d081e88ccc82f1bd2 Mon Sep 17 00:00:00 2001
From: Neil Greatorex <neil@xxxxxxxxxxxxxxx>
Date: Sun, 6 Apr 2014 16:10:43 +0100
Subject: [PATCH 3/3] irqchip: armada-370-xp: Fix releasing of MSIs

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.

Signed-off-by: Neil Greatorex <neil@xxxxxxxxxxxxxxx>
Signed-off-by: Thomas Petazzoni <thomas.petazzoni@xxxxxxxxxxxxxxxxxx>
---
 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 5d925af..939eb0d 100644
--- a/drivers/irqchip/irq-armada-370-xp.c
+++ b/drivers/irqchip/irq-armada-370-xp.c
@@ -156,8 +156,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.8.3.2

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@xxxxxxxxxxxxxxxxxxx
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[Index of Archives]     [Linux Kernel]     [Linux ARM (vger)]     [Linux ARM MSM]     [Linux Omap]     [CentOS ARM]     [Linux Arm]     [Linux Tegra]     [Fedora ARM]     [Linux for Samsung SOC]     [eCos]     [Linux Fastboot]     [Gcc Help]     [Git]     [DCCP]     [IETF Announce]     [Security]     [Linux MIPS]     [Yosemite Campsites]     [Photos]