lkml.org 
[lkml]   [2021]   [Feb]   [8]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
From
SubjectRe: [PATCH 15/18] irqchip/apple-aic: Add support for the Apple Interrupt Controller
Date
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

\
 
 \ /
  Last update: 2021-02-08 12:31    [W:0.598 / U:0.168 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site