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

[Thomas Gleixner]

On Tue, May 23 2023 at 11:25, Marc Zyngier wrote:
> On Mon, 22 May 2023 15:19:39 +0100,
> Thomas Gleixner <tglx@xxxxxxxxxxxxx> 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.

Thanks,

        tglx