linux-kernel - Re: [patch V2 06/40] PCI/MSI: Provide static key for parent mask/unmask

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] [day] [month] [year] [list]

Message-ID: <861qiwdhl4.wl-maz@kernel.org>
Date:   Wed, 31 May 2023 09:35:03 +0100
From:   Marc Zyngier <maz@...nel.org>
To:     Thomas Gleixner <tglx@...utronix.de>
Cc:     LKML <linux-kernel@...r.kernel.org>, Will Deacon <will@...nel.org>,
        linux-pci@...r.kernel.org, Bjorn Helgaas <bhelgaas@...gle.com>,
        Lorenzo Pieralisi <lorenzo.pieralisi@....com>,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        Jason Gunthorpe <jgg@...lanox.com>,
        Andrew Lunn <andrew@...n.ch>,
        Gregory Clement <gregory.clement@...tlin.com>,
        Sebastian Hesselbarth <sebastian.hesselbarth@...il.com>,
        Ammar Faizi <ammarfaizi2@...weeb.org>,
        Robin Murphy <robin.murphy@....com>,
        Lorenzo Pieralisi <lpieralisi@...nel.org>,
        Nishanth Menon <nm@...com>, Tero Kristo <kristo@...nel.org>,
        Santosh Shilimkar <ssantosh@...nel.org>,
        linux-arm-kernel@...ts.infradead.org,
        Vinod Koul <vkoul@...nel.org>, Sinan Kaya <okaya@...nel.org>,
        Andy Gross <agross@...nel.org>,
        Bjorn Andersson <andersson@...nel.org>,
        Mark Rutland <mark.rutland@....com>,
        Shameerali Kolothum Thodi 
        <shameerali.kolothum.thodi@...wei.com>,
        Zenghui Yu <yuzenghui@...wei.com>,
        Shawn Guo <shawnguo@...nel.org>,
        Sascha Hauer <s.hauer@...gutronix.de>,
        Fabio Estevam <festevam@...il.com>,
        Anna-Maria Behnsen <anna-maria.behnsen@...utronix.de>
Subject: Re: [patch V2 06/40] PCI/MSI: Provide static key for parent mask/unmask

On Tue, 23 May 2023 14:05:56 +0100,
Thomas Gleixner <tglx@...utronix.de> wrote:
> 
> On Tue, May 23 2023 at 11:25, Marc Zyngier wrote:
> > On Mon, 22 May 2023 15:19:39 +0100,
> > Thomas Gleixner <tglx@...utronix.de> wrote:
> >> On the other hand for PCI/MSI[x] the mask/unmask operations are not in
> >> the hot path as PCI/MSI[x] are strictly edge. Mask/unmask is only
> >> happening on startup, shutdown and when an interrupt arrives after
> >> disable_irq() incremented the lazy disable counter.
> >> 
> >> For regular interrupt handling mask/unmask is not involved.
> >> 
> >> So to avoid that global key we can let the parent domain set a new flag,
> >> e.g. MSI_FLAG_PCI_MSI_MASK_PARENT, in msi_parent_ops::supported_flags
> >> and let the PCI/MSI core code query that information when the per device
> >> domain is created and select the appropriate template or fixup the
> >> callbacks after the domain is created.
> >> 
> >> Does that address your concerns?
> >
> > It does to a certain extent.
> >
> > But what I'd really like is that in the most common case where the
> > interrupt controller is capable of masking MSIs, the PCI/MSI
> > *enabling* becomes the responsibility of the PCI core code and not the
> > IRQ code.
> >
> > The IRQ code should ideally only be concerned with the masking of the
> > interrupt at the irqchip level, and not beyond that. And that'd solve
> > the Xen problem by merely ignoring it.
> >
> > If we have HW out there that cannot mask MSIs at the interrupt
> > controller level, then we'd have to fallback to device-side masking,
> > which doesn't really work in general (MultiMSI being my favourite
> > example). My gut feeling is that this is rare, but I'm pretty sure it
> > exists.
> 
> Sure. There are 3 parts involved:
> 
>       [Device]--->[PCI/MSI]---->[GIC]
>                    irqchip      irqchip
> 
> Controlling the interrupt machinery in the device happens at the device
> driver level and is conceptually independent of the interrupt
> manangement code. The device driver has no access to the PCI/MSI irqchip
> and all it can do is to enable/disable the source of the interrupt in
> the device.
> 
> For the interrupt management code the job is to ensure that an interrupt
> can be prevented from disrupting the OS operation independent of the
> device driver correctness.
> 
> As a matter of fact we know that PCI/MSI masking ranges from not
> possible over flaky to properly working. So we can't reliably prevent
> that a rougue device spams the PCIe bus with messages.
> 
> Which means that we should utilize the fact that the next interrupt chip
> in the hierarchy can mask reliably. I wish I could disable individual
> vectors at the local APIC level on x86...
> 
> Now the question is whether we want to make this conditional depending
> on what the PCI/MSI[X] hardware advertises or just keep it simple and do
> it unconditionally.

I think this should be unconditional if the root irqchip (the GIC in
this instance) is capable of it.

So a suggestion where the root irqchip exposes its masking capability,
which upon detection by the upper layer (whateverbusyouwant/MSI) makes
it stop playing with its own device-level mask has my full support
(and now breathe normally).

Thanks,

	M.

-- 
Without deviation from the norm, progress is not possible.