| From | Hector Martin 'marcan' <> | Subject | Re: [PATCH 15/18] irqchip/apple-aic: Add support for the Apple Interrupt Controller | Date | Mon, 8 Feb 2021 20:13:22 +0900 |
| |
On 08/02/2021 19.29, Arnd Bergmann wrote: > On Mon, Feb 8, 2021 at 10:25 AM Marc Zyngier <maz@kernel.org> wrote: >> On Thu, 04 Feb 2021 20:39:48 +0000, Hector Martin <marcan@marcan.st> wrote: > >>> +{ >>> + 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).
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...
-- Hector Martin "marcan" (marcan@marcan.st) Public Key: https://mrcn.st/pub
|