On Mon, Sep 28, 2015 at 02:38:31PM +1000, Paul Mackerras wrote:
> commit e297c939b745e420ef0b9dc989cb87bda617b399 upstream.
>
> This fixes a race which can result in the same virtual IRQ number
> being assigned to two different MSI interrupts. The most visible
> consequence of that is usually a warning and stack trace from the
> sysfs code about an attempt to create a duplicate entry in sysfs.
>
> The race happens when one CPU (say CPU 0) is disposing of an MSI
> while another CPU (say CPU 1) is setting up an MSI. CPU 0 calls
> (for example) pnv_teardown_msi_irqs(), which calls
> msi_bitmap_free_hwirqs() to indicate that the MSI (i.e. its
> hardware IRQ number) is no longer in use. Then, before CPU 0 gets
> to calling irq_dispose_mapping() to free up the virtal IRQ number,
> CPU 1 comes in and calls msi_bitmap_alloc_hwirqs() to allocate an
> MSI, and gets the same hardware IRQ number that CPU 0 just freed.
> CPU 1 then calls irq_create_mapping() to get a virtual IRQ number,
> which sees that there is currently a mapping for that hardware IRQ
> number and returns the corresponding virtual IRQ number (which is
> the same virtual IRQ number that CPU 0 was using). CPU 0 then
> calls irq_dispose_mapping() and frees that virtual IRQ number.
> Now, if another CPU comes along and calls irq_create_mapping(), it
> is likely to get the virtual IRQ number that was just freed,
> resulting in the same virtual IRQ number apparently being used for
> two different hardware interrupts.
>
> To fix this race, we just move the call to msi_bitmap_free_hwirqs()
> to after the call to irq_dispose_mapping(). Since virq_to_hw()
> doesn't work for the virtual IRQ number after irq_dispose_mapping()
> has been called, we need to call it before irq_dispose_mapping() and
> remember the result for the msi_bitmap_free_hwirqs() call.
>
> The pattern of calling msi_bitmap_free_hwirqs() before
> irq_dispose_mapping() appears in 5 places under arch/powerpc, and
> appears to have originated in commit 05af7bd2d75e ("[POWERPC] MPIC
> U3/U4 MSI backend") from 2007.
>
> Fixes: 05af7bd2d75e ("[POWERPC] MPIC U3/U4 MSI backend")
> Cc: stable@xxxxxxxxxxxxxxx # v2.6.22 - v4.1
Thanks, I'm applying it to the 3.16 kernel.
Cheers,
--
Luís
> Reported-by: Alexey Kardashevskiy <aik@xxxxxxxxx>
> Signed-off-by: Paul Mackerras <paulus@xxxxxxxxx>
> Signed-off-by: Michael Ellerman <mpe@xxxxxxxxxxxxxx>
> Signed-off-by: Paul Mackerras <paulus@xxxxxxxxx>
> ---
> arch/powerpc/platforms/powernv/pci.c | 5 +++--
> arch/powerpc/sysdev/fsl_msi.c | 5 +++--
> arch/powerpc/sysdev/mpic_pasemi_msi.c | 6 ++++--
> arch/powerpc/sysdev/mpic_u3msi.c | 5 +++--
> arch/powerpc/sysdev/ppc4xx_msi.c | 5 +++--
> 5 files changed, 16 insertions(+), 10 deletions(-)
>
> diff --git a/arch/powerpc/platforms/powernv/pci.c b/arch/powerpc/platforms/powernv/pci.c
> index bca2aeb..3ff29cf 100644
> --- a/arch/powerpc/platforms/powernv/pci.c
> +++ b/arch/powerpc/platforms/powernv/pci.c
> @@ -99,6 +99,7 @@ static void pnv_teardown_msi_irqs(struct pci_dev *pdev)
> struct pci_controller *hose = pci_bus_to_host(pdev->bus);
> struct pnv_phb *phb = hose->private_data;
> struct msi_desc *entry;
> + irq_hw_number_t hwirq;
>
> if (WARN_ON(!phb))
> return;
> @@ -106,10 +107,10 @@ static void pnv_teardown_msi_irqs(struct pci_dev *pdev)
> list_for_each_entry(entry, &pdev->msi_list, list) {
> if (entry->irq == NO_IRQ)
> continue;
> + hwirq = virq_to_hw(entry->irq);
> irq_set_msi_desc(entry->irq, NULL);
> - msi_bitmap_free_hwirqs(&phb->msi_bmp,
> - virq_to_hw(entry->irq) - phb->msi_base, 1);
> irq_dispose_mapping(entry->irq);
> + msi_bitmap_free_hwirqs(&phb->msi_bmp, hwirq - phb->msi_base, 1);
> }
> }
> #endif /* CONFIG_PCI_MSI */
> diff --git a/arch/powerpc/sysdev/fsl_msi.c b/arch/powerpc/sysdev/fsl_msi.c
> index f086c6f..fd16cb5 100644
> --- a/arch/powerpc/sysdev/fsl_msi.c
> +++ b/arch/powerpc/sysdev/fsl_msi.c
> @@ -128,15 +128,16 @@ static void fsl_teardown_msi_irqs(struct pci_dev *pdev)
> {
> struct msi_desc *entry;
> struct fsl_msi *msi_data;
> + irq_hw_number_t hwirq;
>
> list_for_each_entry(entry, &pdev->msi_list, list) {
> if (entry->irq == NO_IRQ)
> continue;
> + hwirq = virq_to_hw(entry->irq);
> msi_data = irq_get_chip_data(entry->irq);
> irq_set_msi_desc(entry->irq, NULL);
> - msi_bitmap_free_hwirqs(&msi_data->bitmap,
> - virq_to_hw(entry->irq), 1);
> irq_dispose_mapping(entry->irq);
> + msi_bitmap_free_hwirqs(&msi_data->bitmap, hwirq, 1);
> }
>
> return;
> diff --git a/arch/powerpc/sysdev/mpic_pasemi_msi.c b/arch/powerpc/sysdev/mpic_pasemi_msi.c
> index a3f660e..89496cf 100644
> --- a/arch/powerpc/sysdev/mpic_pasemi_msi.c
> +++ b/arch/powerpc/sysdev/mpic_pasemi_msi.c
> @@ -65,6 +65,7 @@ static struct irq_chip mpic_pasemi_msi_chip = {
> static void pasemi_msi_teardown_msi_irqs(struct pci_dev *pdev)
> {
> struct msi_desc *entry;
> + irq_hw_number_t hwirq;
>
> pr_debug("pasemi_msi_teardown_msi_irqs, pdev %p\n", pdev);
>
> @@ -72,10 +73,11 @@ static void pasemi_msi_teardown_msi_irqs(struct pci_dev *pdev)
> if (entry->irq == NO_IRQ)
> continue;
>
> + hwirq = virq_to_hw(entry->irq);
> irq_set_msi_desc(entry->irq, NULL);
> - msi_bitmap_free_hwirqs(&msi_mpic->msi_bitmap,
> - virq_to_hw(entry->irq), ALLOC_CHUNK);
> irq_dispose_mapping(entry->irq);
> + msi_bitmap_free_hwirqs(&msi_mpic->msi_bitmap,
> + hwirq, ALLOC_CHUNK);
> }
>
> return;
> diff --git a/arch/powerpc/sysdev/mpic_u3msi.c b/arch/powerpc/sysdev/mpic_u3msi.c
> index b2cef18..13a34b2 100644
> --- a/arch/powerpc/sysdev/mpic_u3msi.c
> +++ b/arch/powerpc/sysdev/mpic_u3msi.c
> @@ -107,15 +107,16 @@ static u64 find_u4_magic_addr(struct pci_dev *pdev, unsigned int hwirq)
> static void u3msi_teardown_msi_irqs(struct pci_dev *pdev)
> {
> struct msi_desc *entry;
> + irq_hw_number_t hwirq;
>
> list_for_each_entry(entry, &pdev->msi_list, list) {
> if (entry->irq == NO_IRQ)
> continue;
>
> + hwirq = virq_to_hw(entry->irq);
> irq_set_msi_desc(entry->irq, NULL);
> - msi_bitmap_free_hwirqs(&msi_mpic->msi_bitmap,
> - virq_to_hw(entry->irq), 1);
> irq_dispose_mapping(entry->irq);
> + msi_bitmap_free_hwirqs(&msi_mpic->msi_bitmap, hwirq, 1);
> }
>
> return;
> diff --git a/arch/powerpc/sysdev/ppc4xx_msi.c b/arch/powerpc/sysdev/ppc4xx_msi.c
> index 6e2e6aa..02a137d 100644
> --- a/arch/powerpc/sysdev/ppc4xx_msi.c
> +++ b/arch/powerpc/sysdev/ppc4xx_msi.c
> @@ -124,16 +124,17 @@ void ppc4xx_teardown_msi_irqs(struct pci_dev *dev)
> {
> struct msi_desc *entry;
> struct ppc4xx_msi *msi_data = &ppc4xx_msi;
> + irq_hw_number_t hwirq;
>
> dev_dbg(&dev->dev, "PCIE-MSI: tearing down msi irqs\n");
>
> list_for_each_entry(entry, &dev->msi_list, list) {
> if (entry->irq == NO_IRQ)
> continue;
> + hwirq = virq_to_hw(entry->irq);
> irq_set_msi_desc(entry->irq, NULL);
> - msi_bitmap_free_hwirqs(&msi_data->bitmap,
> - virq_to_hw(entry->irq), 1);
> irq_dispose_mapping(entry->irq);
> + msi_bitmap_free_hwirqs(&msi_data->bitmap, hwirq, 1);
> }
> }
>
> --
> 2.1.4
>
> --
> To unsubscribe from this list: send the line "unsubscribe stable" in
> the body of a message to majordomo@xxxxxxxxxxxxxxx
> More majordomo info at http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe stable" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html