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