Re: [PATCH 15/18] irqchip/apple-aic: Add support for the Apple Interrupt Controller

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 08/02/2021 20.36, Marc Zyngier wrote:
On Mon, 08 Feb 2021 10:29:23 +0000,
Arnd Bergmann <arnd@xxxxxxxxxx> wrote:

On Mon, Feb 8, 2021 at 10:25 AM Marc Zyngier <maz@xxxxxxxxxx> wrote:
On Thu, 04 Feb 2021 20:39:48 +0000, Hector Martin <marcan@xxxxxxxxx> 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@xxxxxxxxx)
Public Key: https://mrcn.st/pub



[Index of Archives]     [Device Tree Compilter]     [Device Tree Spec]     [Linux Driver Backports]     [Video for Linux]     [Linux USB Devel]     [Linux PCI Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Yosemite Backpacking]


  Powered by Linux