linux-kernel - Re: [PATCH] genirq/msi: Make sure PCI MSIs are activated early

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <alpine.DEB.2.11.1607250928370.5629@nanos>
Date:	Mon, 25 Jul 2016 09:45:13 +0200 (CEST)
From:	Thomas Gleixner <tglx@...utronix.de>
To:	Bjorn Helgaas <helgaas@...nel.org>
cc:	Marc Zyngier <marc.zyngier@....com>,
	Bjorn Helgaas <bhelgaas@...gle.com>,
	Bharat Kumar Gogada <bharat.kumar.gogada@...inx.com>,
	linux-pci@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] genirq/msi: Make sure PCI MSIs are activated early

On Fri, 22 Jul 2016, Bjorn Helgaas wrote:
> On Wed, Jul 13, 2016 at 05:18:33PM +0100, Marc Zyngier wrote:
> > 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.
> > 
> > Reported-by: Bharat Kumar Gogada <bharat.kumar.gogada@...inx.com>
> > Tested-by: Bharat Kumar Gogada <bharat.kumar.gogada@...inx.com>
> > Signed-off-by: Marc Zyngier <marc.zyngier@....com>
> 
> Acked-by: Bjorn Helgaas <bhelgaas@...gle.com>
> 
> Thomas, let me know if you'd like me to take this.  It looks like the
> real smarts here are in kernel/irq, so I assume you'll take it unless
> I hear otherwise.

I'll take it. Though I have second thoughts about the whole issue.

We deliberately made the allocation sequence of interrupts in a way that we
can easily rollback in case of failure.

We achieved that by activating the interrupts only at request time and not
somewhere in the middle of the allocation sequence. That makes the whole
hierarchical allocation more robust and avoids complex rollbacks.

Now that new flag is basically torpedoing that approach.

What I really wonder is why that is only an issue with that particular xilinx
hardware/IP block. I'm aware that up to PCI 2.3 the mask bit for MSI
interrupts is optional or in really old versions not even specified. So only
if that mask bit is missing the above described issue can happen.

If not, then we might have a general issue that we don't mask the entry before
we call pci_msi_set_enable().

Thoughts?

	tglx