Thanks Marc and Thomas for addressing the issue.
> -----Original Message----- > From: tip tree robot [mailto:tip...@zytor.com] > Sent: Tuesday, August 09, 2016 12:59 PM > To: linux-tip-comm...@vger.kernel.org > Cc: Bharat Kumar Gogada <bhara...@xilinx.com>; bhelg...@google.com; > h...@zytor.com; li...@matthiasprager.de; marc.zyng...@arm.com; > jason.tay...@simplivity.com; linux-kernel@vger.kernel.org; > fo...@forstwoof.ru; mi...@kernel.org; t...@linutronix.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.zyng...@arm.com> > AuthorDate: Wed, 13 Jul 2016 17:18:33 +0100 > Committer: Thomas Gleixner <t...@linutronix.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.gog...@xilinx.com> > Reported-and-tested-by: Foster Snowhill <fo...@forstwoof.ru> > Reported-by: Matthias Prager <li...@matthiasprager.de> > Reported-by: Jason Taylor <jason.tay...@simplivity.com> > Signed-off-by: Marc Zyngier <marc.zyng...@arm.com> > Acked-by: Bjorn Helgaas <bhelg...@google.com> > Cc: linux-...@vger.kernel.org > Cc: sta...@vger.kernel.org > Link: http://lkml.kernel.org/r/1468426713-31431-1-git-send-email- > marc.zyng...@arm.com > Signed-off-by: Thomas Gleixner <t...@linutronix.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;
