Re: [PATCH v4 01/10] ata: libahci: Ensure the host interrupt status bits are cleared

From: Marc Zyngier <marc.zyngier@arm.com>
To: Miquel Raynal <miquel.raynal@bootlin.com>
Cc: Mark Rutland <mark.rutland@arm.com>, Andrew Lunn <andrew@lunn.ch>,
	raymond pang <raymondpangxd@gmail.com>,
	Jason Cooper <jason@lakedaemon.net>,
	Nadav Haklai <nadavh@marvell.com>,
	devicetree@vger.kernel.org,
	Antoine Tenart <antoine.tenart@bootlin.com>,
	Gregory Clement <gregory.clement@bootlin.com>,
	Baruch Siach <baruch@tkos.co.il>,
	Maxime Chevallier <maxime.chevallier@bootlin.com>,
	linux-ide@vger.kernel.org, Hans de Goede <hdegoede@redhat.com>,
	Rob Herring <robh+dt@kernel.org>, Jens Axboe <axboe@kernel.dk>,
	Thomas Petazzoni <thomas.petazzoni@bootlin.com>,
	Thomas Gleixner <tglx@linutronix.de>,
	linux-arm-kernel@lists.infradead.org,
	Sebastian Hesselbarth <sebastian.hesselbarth@gmail.com>
Subject: Re: [PATCH v4 01/10] ata: libahci: Ensure the host interrupt status bits are cleared
Date: Wed, 29 May 2019 11:37:58 +0100	[thread overview]
Message-ID: <409ea2c5-c31a-fb6a-22c6-98b45e767809@arm.com> (raw)
In-Reply-To: <20190529120833.29334c70@xps13>

On 29/05/2019 11:08, Miquel Raynal wrote:
> Hi Marc & Raymond,
> 
> Marc Zyngier <marc.zyngier@arm.com> 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...

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel