On Mon, Feb 8, 2021 at 12:13 PM Hector Martin 'marcan' [off-list ref] wrote:

On 08/02/2021 19.29, Arnd Bergmann wrote:

quoted

On Mon, Feb 8, 2021 at 10:25 AM Marc Zyngier [off-list ref] wrote:

quoted

On Thu, 04 Feb 2021 20:39:48 +0000, Hector Martin [off-list ref] wrote:

quoted

quoted

+{
+     return readl(ic->base + reg);

Please consider using the _relaxed accessors, as I don't think any of
these interacts with memory (apart from IPIs, of course).

MSI interrupts require serializing with DMA, so at the minimum I think there
needs to be something that ensures that DMA from device into memory
has completed before delivering the completion interrupt to a driver. This
may already be implied when the AIC is entered, but this is hard to know
without actual hardware specs.

I don't think this can be implied in any case, because if IRQ A fires
and then the CPU speculates its way through AIC into the IRQ B handler,
which reads DMA'd memory, then IRQ B fires and it has higher priority
and *that* is what ends up getting returned from the event register
first, the execution will commit with an ordering violation.

I'm pretty sure we need *some* level of explicit synchronization between
reading the event register and actually delivering IRQs downstream.
Using _relaxed might be okay, but we'd still need something where the
isb() currently is in aic_handle_irq (though I admit I don't have a
perfect picture of the memory ordering subtleties involved here yet).

The idea of the barriers in readl/writel is to avoid having to guess these
subtleties, so maybe it's easier to start with the normal readl() and no
other barrier for an initial merge, as that should be sufficient, and then
do any optimizations in a later patch after everyone agrees what the
specific requirements are and much much time can be saved this way.

Incidentally, just from the races and problems I've run into with
trivial tests in m1n1, these CPUs seem to be *very* eager to speculate
and I suspect they will help uncover race conditions in Linux...

Yes, that definitely helps.

       Arnd

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