From be3f48cb21c1ca4907a0822eea406c8dd4a73ddb Mon Sep 17 00:00:00 2001 From: Bjørn Erik Nilsen Date: Fri, 29 Nov 2013 14:35:24 +0100 Subject: PCI: designware: Fix crash in dw_msi_teardown_irq() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 904d0e788993 ("PCI: designware: Add irq_create_mapping()") resulted in pre-allocated irq descs. Problem was that in assign_irq() these descs were explicitly allocated and hence also freed, resulting in a crash. We also need to clear the entire irq range in teardown. With this commit the teardown basically does exactly the opposite of what was done in setup. The crash this fixes looks like: Unable to handle kernel NULL pointer dereference at virtual address 00000020 PC is at dw_msi_teardown_irq+0x40/0x118 LR is at trace_hardirqs_on_caller+0xf4/0x1c0 Backtrace: [<802c401c>] (dw_msi_teardown_irq+0x0/0x118) from [<802c1844>] (arch_teardown_msi_irq+0x3c/0x40) [<802c1808>] (arch_teardown_msi_irq+0x0/0x40) from [<802c1a08>] (default_teardown_msi_irqs+0x68/0x84) [<802c19a0>] (default_teardown_msi_irqs+0x0/0x84) from [<802c1a34>] (arch_teardown_msi_irqs+0x10/0x14) [<802c1a24>] (arch_teardown_msi_irqs+0x0/0x14) from [<802c1ad0>] (free_msi_irqs+0x98/0x144) [<802c1a38>] (free_msi_irqs+0x0/0x144) from [<802c2570>] (pci_disable_msi+0x48/0x60) [<802c2528>] (pci_disable_msi+0x0/0x60) from [<7f0057d4>] (sxdma_irq_free+0x44/0x48 [sxdma]) [bhelgaas: add crash info] Tested-by: Mohit Kumar Signed-off-by: Bjørn Erik Nilsen Signed-off-by: Bjorn Helgaas Acked-by: Marek Vasut Acked-by: Jingoo Han --- drivers/pci/host/pcie-designware.c | 49 ++++++++++++++++++++++++++++---------- 1 file changed, 37 insertions(+), 12 deletions(-) diff --git a/drivers/pci/host/pcie-designware.c b/drivers/pci/host/pcie-designware.c index e33b68be0391..61345a18a6d1 100644 --- a/drivers/pci/host/pcie-designware.c +++ b/drivers/pci/host/pcie-designware.c @@ -209,6 +209,25 @@ static int find_valid_pos0(struct pcie_port *pp, int msgvec, int pos, int *pos0) return 0; } +static void clear_irq_range(struct pcie_port *pp, unsigned int irq_base, + unsigned int nvec, unsigned int pos) +{ + unsigned int i, res, bit, val; + + i = 0; + while (i < nvec) { + irq_set_msi_desc_off(irq_base, i, NULL); + clear_bit(pos + i, pp->msi_irq_in_use); + /* Disable corresponding interrupt on MSI interrupt controller */ + res = ((pos + i) / 32) * 12; + bit = (pos + i) % 32; + dw_pcie_rd_own_conf(pp, PCIE_MSI_INTR0_ENABLE + res, 4, &val); + val &= ~(1 << bit); + dw_pcie_wr_own_conf(pp, PCIE_MSI_INTR0_ENABLE + res, 4, val); + ++i; + } +} + static int assign_irq(int no_irqs, struct msi_desc *desc, int *pos) { int res, bit, irq, pos0, pos1, i; @@ -242,11 +261,20 @@ static int assign_irq(int no_irqs, struct msi_desc *desc, int *pos) if (!irq) goto no_valid_irq; + /* + * irq_create_mapping (called from dw_pcie_host_init) pre-allocates + * descs so there is no need to allocate descs here. We can therefore + * assume that if irq_find_mapping above returns non-zero, then the + * descs are also successfully allocated. + */ + i = 0; while (i < no_irqs) { + if (irq_set_msi_desc_off(irq, i, desc) != 0) { + clear_irq_range(pp, irq, i, pos0); + goto no_valid_irq; + } set_bit(pos0 + i, pp->msi_irq_in_use); - irq_alloc_descs((irq + i), (irq + i), 1, 0); - irq_set_msi_desc(irq + i, desc); /*Enable corresponding interrupt in MSI interrupt controller */ res = ((pos0 + i) / 32) * 12; bit = (pos0 + i) % 32; @@ -266,7 +294,7 @@ no_valid_irq: static void clear_irq(unsigned int irq) { - int res, bit, val, pos; + unsigned int pos, nvec; struct irq_desc *desc; struct msi_desc *msi; struct pcie_port *pp; @@ -281,18 +309,15 @@ static void clear_irq(unsigned int irq) return; } + /* undo what was done in assign_irq */ pos = data->hwirq; + nvec = 1 << msi->msi_attrib.multiple; - irq_free_desc(irq); - - clear_bit(pos, pp->msi_irq_in_use); + clear_irq_range(pp, irq, nvec, pos); - /* Disable corresponding interrupt on MSI interrupt controller */ - res = (pos / 32) * 12; - bit = pos % 32; - dw_pcie_rd_own_conf(pp, PCIE_MSI_INTR0_ENABLE + res, 4, &val); - val &= ~(1 << bit); - dw_pcie_wr_own_conf(pp, PCIE_MSI_INTR0_ENABLE + res, 4, val); + /* all irqs cleared; reset attributes */ + msi->irq = 0; + msi->msi_attrib.multiple = 0; } static int dw_msi_setup_irq(struct msi_chip *chip, struct pci_dev *pdev, -- cgit From 64989e7399f09b72689e25fb40f2d0d5e073b13a Mon Sep 17 00:00:00 2001 From: Bjørn Erik Nilsen Date: Fri, 29 Nov 2013 14:35:25 +0100 Subject: PCI: designware: Remove redundant call to pci_write_config_word() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit write_msi_msg() does exactly the same so there is no need to explicitly call pci_write_config_word() and do the same twice. Tested-by: Mohit Kumar Signed-off-by: Bjørn Erik Nilsen Signed-off-by: Bjorn Helgaas Acked-by: Marek Vasut Acked-by: Jingoo Han --- drivers/pci/host/pcie-designware.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/drivers/pci/host/pcie-designware.c b/drivers/pci/host/pcie-designware.c index 61345a18a6d1..5274085ecd4b 100644 --- a/drivers/pci/host/pcie-designware.c +++ b/drivers/pci/host/pcie-designware.c @@ -345,10 +345,10 @@ static int dw_msi_setup_irq(struct msi_chip *chip, struct pci_dev *pdev, if (irq < 0) return irq; - msg_ctr &= ~PCI_MSI_FLAGS_QSIZE; - msg_ctr |= msgvec << 4; - pci_write_config_word(pdev, desc->msi_attrib.pos + PCI_MSI_FLAGS, - msg_ctr); + /* + * write_msi_msg() will update PCI_MSI_FLAGS so there is + * no need to explicitly call pci_write_config_word(). + */ desc->msi_attrib.multiple = msgvec; msg.address_lo = virt_to_phys((void *)pp->msi_data); -- cgit From 0b8cfb6aa3aabc96177b1e68ef13d2eb5c686606 Mon Sep 17 00:00:00 2001 From: Bjorn Helgaas Date: Mon, 9 Dec 2013 15:11:25 -0700 Subject: PCI: designware: Use typical "for" loop idiom It's conventional to use "for" rather than "while" for simple iteration. No functional change. Signed-off-by: Bjorn Helgaas --- drivers/pci/host/pcie-designware.c | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/drivers/pci/host/pcie-designware.c b/drivers/pci/host/pcie-designware.c index 5274085ecd4b..1c92833a4ed3 100644 --- a/drivers/pci/host/pcie-designware.c +++ b/drivers/pci/host/pcie-designware.c @@ -214,8 +214,7 @@ static void clear_irq_range(struct pcie_port *pp, unsigned int irq_base, { unsigned int i, res, bit, val; - i = 0; - while (i < nvec) { + for (i = 0; i < nvec; i++) { irq_set_msi_desc_off(irq_base, i, NULL); clear_bit(pos + i, pp->msi_irq_in_use); /* Disable corresponding interrupt on MSI interrupt controller */ @@ -224,7 +223,6 @@ static void clear_irq_range(struct pcie_port *pp, unsigned int irq_base, dw_pcie_rd_own_conf(pp, PCIE_MSI_INTR0_ENABLE + res, 4, &val); val &= ~(1 << bit); dw_pcie_wr_own_conf(pp, PCIE_MSI_INTR0_ENABLE + res, 4, val); - ++i; } } @@ -268,8 +266,7 @@ static int assign_irq(int no_irqs, struct msi_desc *desc, int *pos) * descs are also successfully allocated. */ - i = 0; - while (i < no_irqs) { + for (i = 0; i < no_irqs; i++) { if (irq_set_msi_desc_off(irq, i, desc) != 0) { clear_irq_range(pp, irq, i, pos0); goto no_valid_irq; @@ -281,7 +278,6 @@ static int assign_irq(int no_irqs, struct msi_desc *desc, int *pos) dw_pcie_rd_own_conf(pp, PCIE_MSI_INTR0_ENABLE + res, 4, &val); val |= 1 << bit; dw_pcie_wr_own_conf(pp, PCIE_MSI_INTR0_ENABLE + res, 4, val); - i++; } *pos = pos0; -- cgit