On 29/05/2019 11:08, Miquel Raynal wrote: > Hi Marc & Raymond, > > Marc Zyngier <marc.zyngier@xxxxxxx> wrote on Thu, 23 May 2019 10:26:01 > +0100: > >> On 23/05/2019 04:11, raymond pang wrote: >>> Hi Miquel, >>> >>> This patch adds clearing GHC.IS into hot path, could you explain how >>> irq storm is generated? thanks >>> According to AHCI Spec, HBA should not refer to GHC.IS to generate >>> MSI when applying multiple MSIs. >> >> Well spotted. >> >> I have the ugly feeling that this is because the Marvell AHCI >> implementation is not using MSIs at all, but instead a pair of wired >> interrupts (which are level triggered instead of edge, hence the >> screaming interrupts). >> >> The changes in the following patches abuse the rest of the driver by >> pretending this is a a multi-MSI setup, while it clearly doesn't match >> the expectation of the AHCI spec for MSIs. >> >> It looks like this shouldn't be imposed on other unsuspecting >> implementations which correctly use edge-triggered MSIs and do not >> require such an MMIO access. > > I understand your concern, let me add a AHCI_HFLAG_LEVEL_MSI in > hpriv->flags which will be used by the mvebu_ahci.c driver to request > for this MMIO access. This way, the hot path remains the same. I'm not convinced that's a good idea, if only because from the PoV of the AHCI device itself, these are not MSIs at all, but wired interrupts. The fact that there is some glue logic in the middle that turns it into a message (and then back into a wire) is a regrettable implementation detail. I'd rather you stick to the normal interrupt handler, or provide your own, which would solve most problems. Thanks, M. -- Jazz is not dead. It just smells funny...
