Re: [PATCH] irqchip/msi-lib: Honor the MSI_FLAG_PCI_MSI_MASK_PARENT flag

[Marc Zyngier <maz@kernel.org>]

On Sat, 17 May 2025 20:59:10 +0100,
Thomas Gleixner <tglx@linutronix.de> wrote:
> 
> On Sat, May 17 2025 at 11:30, Marc Zyngier wrote:
> > +	/*
> > +	 * If the parent domain insists on being in charge of masking, obey
> > +	 * blindly. The default mask/unmask become the shutdown/enable
> > +	 * callbacks, ensuring that we correctly start/stop the interrupt.
> > + 	 * We make a point in not using the irq_disable() in order to
> > +	 * preserve the "lazy disable" behaviour.
> > +	 */
> > +	if (info->flags & MSI_FLAG_PCI_MSI_MASK_PARENT) {
> > +		chip->irq_shutdown	= chip->irq_mask;
> > +		chip->irq_enable	= chip->irq_unmask;
> 
> This is only correct, when the chip does not have dedicated
> irq_shutdown/enable callbacks.

The chip structure provided by the PCI MSI code doesn't provide such
callback, meaning that they are unused for the whole hierarchy.

> And I really hate the asymmetry of this.

So do I, but that's how the lazy disable thing currently works. Drop
the bizarre asymmetry on irq_disable, and we can make this nicely
symmetric as well.

> 
> > +		chip->irq_mask		= irq_chip_mask_parent;
> > +		chip->irq_unmask	= irq_chip_unmask_parent;
> > +	}
> 
> I'm still trying to understand, what's the actual problem is you are
> trying to solve.

I'm trying to remove some overhead from machines that don't need to
suffer from this nonsense double masking. Specially in VMs when
masking/unmasking requires *two* extremely costly exits (write +
synchronising read-back). This change reduces the overhead
significantly by only masking where it actually matters.

> MSIs are edge type interrupts, so the interrupt handling hotpath usually
> does not mask at all. The only time masking happens is when it's lazy
> disabled or during affinity changes, which is not the end of the world.

And that's part of the problem. The lazy disable ends up being way
more costly than it should when the interrupt fires during the
"disabled but not quite" phase, and in turn makes the critical section
delineated by disable_irq()/enable_irq() more expensive.

So while, as you put it, it's "not the end of the world", this seems
to me like a valuable optimisation.

Another possible improvement would be to teach the PCI code it can
still rely on masking even when the endpoint is not capable of masking
individual MSIs.

Thanks,

	M.

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