Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Wed, 31 May 2023 09:35:03 +0100
Message-ID: <861qiwdhl4.wl-maz@kernel.org>
From:   Marc Zyngier <maz@kernel.org>
To:     Thomas Gleixner <tglx@linutronix.de>
Cc:     LKML <linux-kernel@vger.kernel.org>, Will Deacon <will@kernel.org>,
        linux-pci@vger.kernel.org, Bjorn Helgaas <bhelgaas@google.com>,
        Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>,
        Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
        Jason Gunthorpe <jgg@mellanox.com>,
        Andrew Lunn <andrew@lunn.ch>,
        Gregory Clement <gregory.clement@bootlin.com>,
        Sebastian Hesselbarth <sebastian.hesselbarth@gmail.com>,
        Ammar Faizi <ammarfaizi2@gnuweeb.org>,
        Robin Murphy <robin.murphy@arm.com>,
        Lorenzo Pieralisi <lpieralisi@kernel.org>,
        Nishanth Menon <nm@ti.com>, Tero Kristo <kristo@kernel.org>,
        Santosh Shilimkar <ssantosh@kernel.org>,
        linux-arm-kernel@lists.infradead.org,
        Vinod Koul <vkoul@kernel.org>, Sinan Kaya <okaya@kernel.org>,
        Andy Gross <agross@kernel.org>,
        Bjorn Andersson <andersson@kernel.org>,
        Mark Rutland <mark.rutland@arm.com>,
        Shameerali Kolothum Thodi 
        <shameerali.kolothum.thodi@huawei.com>,
        Zenghui Yu <yuzenghui@huawei.com>,
        Shawn Guo <shawnguo@kernel.org>,
        Sascha Hauer <s.hauer@pengutronix.de>,
        Fabio Estevam <festevam@gmail.com>,
        Anna-Maria Behnsen <anna-maria.behnsen@linutronix.de>
Subject: Re: [patch V2 06/40] PCI/MSI: Provide static key for parent mask/unmask
In-Reply-To: <87lehfurij.ffs@tglx>
References: <20221121135653.208611233@linutronix.de>
        <20221121140048.659849460@linutronix.de>
        <8635a8o65q.wl-maz@kernel.org>
        <87bkowcx0z.ffs@tglx>
        <86zgcgmpzl.wl-maz@kernel.org>
        <87v8n3c2qy.ffs@tglx>
        <87ttw4wiro.ffs@tglx>
        <86r0r7cpks.wl-maz@kernel.org>
        <87lehfurij.ffs@tglx>
User-Agent: Wanderlust/2.15.9 (Almost Unreal) SEMI-EPG/1.14.7 (Harue)
 FLIM-LB/1.14.9 (=?UTF-8?B?R29qxY0=?=) APEL-LB/10.8 EasyPG/1.0.0 Emacs/28.2
 (aarch64-unknown-linux-gnu) MULE/6.0 (HANACHIRUSATO)
MIME-Version: 1.0 (generated by SEMI-EPG 1.14.7 - "Harue")
Content-Type: text/plain; charset=US-ASCII
Precedence: bulk

On Tue, 23 May 2023 14:05:56 +0100,
Thomas Gleixner <tglx@linutronix.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@linutronix.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.