[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <8520D5D51A55D047800579B094147198258D33AB@XAP-PVEXMBX01.xlnx.xilinx.com>
Date: Fri, 2 Sep 2016 14:48:13 +0000
From: Bharat Kumar Gogada <bharat.kumar.gogada@...inx.com>
To: "tglx@...utronix.de" <tglx@...utronix.de>,
"forst@...stwoof.ru" <forst@...stwoof.ru>,
"mingo@...nel.org" <mingo@...nel.org>,
"jason.taylor@...plivity.com" <jason.taylor@...plivity.com>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
"linux@...thiasprager.de" <linux@...thiasprager.de>,
"marc.zyngier@....com" <marc.zyngier@....com>,
"hpa@...or.com" <hpa@...or.com>,
Bharat Kumar Gogada <bharatku@...inx.com>,
"bhelgaas@...gle.com" <bhelgaas@...gle.com>,
"linux-tip-commits@...r.kernel.org"
<linux-tip-commits@...r.kernel.org>
Subject: RE: [tip:irq/urgent] genirq/msi: Make sure PCI MSIs are activated
early
Thanks Marc and Thomas for addressing the issue.
> -----Original Message-----
> From: tip tree robot [mailto:tipbot@...or.com]
> Sent: Tuesday, August 09, 2016 12:59 PM
> To: linux-tip-commits@...r.kernel.org
> Cc: Bharat Kumar Gogada <bharatku@...inx.com>; bhelgaas@...gle.com;
> hpa@...or.com; linux@...thiasprager.de; marc.zyngier@....com;
> jason.taylor@...plivity.com; linux-kernel@...r.kernel.org;
> forst@...stwoof.ru; mingo@...nel.org; tglx@...utronix.de
> Subject: [tip:irq/urgent] genirq/msi: Make sure PCI MSIs are activated early
>
> Commit-ID: f3b0946d629c8bfbd3e5f038e30cb9c711a35f10
> Gitweb:
> http://git.kernel.org/tip/f3b0946d629c8bfbd3e5f038e30cb9c711a35f10
> Author: Marc Zyngier <marc.zyngier@....com>
> AuthorDate: Wed, 13 Jul 2016 17:18:33 +0100
> Committer: Thomas Gleixner <tglx@...utronix.de>
> CommitDate: Tue, 9 Aug 2016 09:19:32 +0200
>
> genirq/msi: Make sure PCI MSIs are activated early
>
> Bharat Kumar Gogada reported issues with the generic MSI code, where the
> end-point ended up with garbage in its MSI configuration (both for the vector
> and the message).
>
> It turns out that the two MSI paths in the kernel are doing slightly different
> things:
>
> generic MSI: disable MSI -> allocate MSI -> enable MSI -> setup EP
> PCI MSI: disable MSI -> allocate MSI -> setup EP -> enable MSI
>
> And it turns out that end-points are allowed to latch the content of the MSI
> configuration registers as soon as MSIs are enabled. In Bharat's case, the
> end-point ends up using whatever was there already, which is not what you
> want.
>
> In order to make things converge, we introduce a new MSI domain flag
> (MSI_FLAG_ACTIVATE_EARLY) that is unconditionally set for PCI/MSI. When set,
> this flag forces the programming of the end-point as soon as the MSIs are
> allocated.
>
> A consequence of this is that we have an extra activate in irq_startup, but
> that should be without much consequence.
>
> tglx:
>
> - Several people reported a VMWare regression with PCI/MSI-X passthrough. It
> turns out that the patch also cures that issue.
>
> - We need to have a look at the MSI disable interrupt path, where we write
> the msg to all zeros without disabling MSI in the PCI device. Is that
> correct?
>
> Fixes: 52f518a3a7c2 "x86/MSI: Use hierarchical irqdomains to manage MSI
> interrupts"
> Reported-and-tested-by: Bharat Kumar Gogada
> <bharat.kumar.gogada@...inx.com>
> Reported-and-tested-by: Foster Snowhill <forst@...stwoof.ru>
> Reported-by: Matthias Prager <linux@...thiasprager.de>
> Reported-by: Jason Taylor <jason.taylor@...plivity.com>
> Signed-off-by: Marc Zyngier <marc.zyngier@....com>
> Acked-by: Bjorn Helgaas <bhelgaas@...gle.com>
> Cc: linux-pci@...r.kernel.org
> Cc: stable@...r.kernel.org
> Link: http://lkml.kernel.org/r/1468426713-31431-1-git-send-email-
> marc.zyngier@....com
> Signed-off-by: Thomas Gleixner <tglx@...utronix.de>
>
> ---
> drivers/pci/msi.c | 2 ++
> include/linux/msi.h | 2 ++
> kernel/irq/msi.c | 11 +++++++++++
> 3 files changed, 15 insertions(+)
>
> diff --git a/drivers/pci/msi.c b/drivers/pci/msi.c
> index a02981e..eafa613 100644
> --- a/drivers/pci/msi.c
> +++ b/drivers/pci/msi.c
> @@ -1411,6 +1411,8 @@ struct irq_domain *pci_msi_create_irq_domain(struct
> fwnode_handle *fwnode,
> if (info->flags & MSI_FLAG_USE_DEF_CHIP_OPS)
> pci_msi_domain_update_chip_ops(info);
>
> + info->flags |= MSI_FLAG_ACTIVATE_EARLY;
> +
> domain = msi_create_irq_domain(fwnode, info, parent);
> if (!domain)
> return NULL;
> diff --git a/include/linux/msi.h b/include/linux/msi.h
> index 4f0bfe5..e8c81fb 100644
> --- a/include/linux/msi.h
> +++ b/include/linux/msi.h
> @@ -270,6 +270,8 @@ enum {
> MSI_FLAG_MULTI_PCI_MSI = (1 << 2),
> /* Support PCI MSIX interrupts */
> MSI_FLAG_PCI_MSIX = (1 << 3),
> + /* Needs early activate, required for PCI */
> + MSI_FLAG_ACTIVATE_EARLY = (1 << 4),
> };
>
> int msi_domain_set_affinity(struct irq_data *data, const struct cpumask *mask,
> diff --git a/kernel/irq/msi.c b/kernel/irq/msi.c
> index 5499935..19e9dfb 100644
> --- a/kernel/irq/msi.c
> +++ b/kernel/irq/msi.c
> @@ -359,6 +359,17 @@ int msi_domain_alloc_irqs(struct irq_domain *domain,
> struct device *dev,
> else
> dev_dbg(dev, "irq [%d-%d] for MSI\n",
> virq, virq + desc->nvec_used - 1);
> + /*
> + * This flag is set by the PCI layer as we need to activate
> + * the MSI entries before the PCI layer enables MSI in the
> + * card. Otherwise the card latches a random msi message.
> + */
> + if (info->flags & MSI_FLAG_ACTIVATE_EARLY) {
> + struct irq_data *irq_data;
> +
> + irq_data = irq_domain_get_irq_data(domain, desc-
> >irq);
> + irq_domain_activate_irq(irq_data);
> + }
> }
>
> return 0;
Powered by blists - more mailing lists