Messages in this thread |  | | From | Hector Martin <> | Subject | Re: [PATCH 15/18] irqchip/apple-aic: Add support for the Apple Interrupt Controller | Date | Tue, 9 Feb 2021 00:31:18 +0900 |
| |
On 08/02/2021 20.36, Marc Zyngier wrote: > On Mon, 08 Feb 2021 10:29:23 +0000, > Arnd Bergmann <arnd@kernel.org> 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. > > If there is a sync with memory required, it should happen at the point > where it is Acked, not when masked/unmasked or anything else. And > given that you want to sync with an external agent (the DMA producer), > the DMB generated by readl won't save you, as it only orders CPU > accesses AFAICT.
Found an doc that talks about this, but then... how does the current Linux code work anyway for normal use cases?
https://elinux.org/images/7/73/Deacon-weak-to-weedy.pdf
That says dmb is not enough for DMA-control to DMA-data dependencies due to speculation, which is what we have here and the situation I described (with an IRQ along the way, but that's irrelevant). But that's what readl does: a read followed by a dmb(oshld) followed by a control dependency (but that needs an isb to take effect). How does this not break drivers that read DMA-accessed memory after a readl of a status register? I thought that was the point of the non-relaxed functions.
-- Hector Martin (marcan@marcan.st) Public Key: https://mrcn.st/pub
|  |