On Thu, Nov 24 2022 at 13:04, Marc Zyngier wrote: > On Mon, 21 Nov 2022 14:39:36 +0000, >> static void pci_mask_msi(struct irq_data *data) >> { >> struct msi_desc *desc = irq_data_get_msi_desc(data); >> >> pci_msi_mask(desc, BIT(data->irq - desc->irq)); >> + cond_mask_parent(data); > > I find this a bit odd. If anything, I'd rather drop the masking at the > PCI level and keep it local to the interrupt controller, because this > is likely to be more universal than the equivalent PCI operation > (think multi-MSI, for example, which cannot masks individual MSIs). > > Another thing is that the static key is a global state. Nothing says > that masking one way or the other is a universal thing, specially when > you have multiple interrupt controllers dealing with MSIs in different > ways. For example, GICv3 can use both the ITS and the GICv3-MBI frame > at the same time for different PCI RC. OK, they happen to deal with > MSIs in the same way, but you hopefully get my point. I'm fine with dropping that. I did this because basically all of the various ARM PCI/MSI domain implementation have a copy of the same functions. Some of them have pointlessly the wrong order because copy & pasta is so wonderful.... So the alternative solution is to provide _ONE_ set of correct callbacks and let the domain initialization code override the irq chip callbacks of the default PCI/MSI template. Thanks, tglx