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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [day] [month] [year] [list]

Message-ID: <c699ab0c-39a0-6b0a-9409-db506ac6f824@marcan.st>
Date:   Tue, 9 Feb 2021 00:31:18 +0900
From:   Hector Martin <marcan@...can.st>
To:     Marc Zyngier <maz@...nel.org>, Arnd Bergmann <arnd@...nel.org>
Cc:     SoC Team <soc@...nel.org>,
        Linux ARM <linux-arm-kernel@...ts.infradead.org>,
        Rob Herring <robh+dt@...nel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        DTML <devicetree@...r.kernel.org>,
        Olof Johansson <olof@...om.net>
Subject: Re: [PATCH 15/18] irqchip/apple-aic: Add support for the Apple
 Interrupt Controller

On 08/02/2021 20.36, Marc Zyngier wrote:
> On Mon, 08 Feb 2021 10:29:23 +0000,
> Arnd Bergmann <arnd@...nel.org> wrote:
>>
>> On Mon, Feb 8, 2021 at 10:25 AM Marc Zyngier <maz@...nel.org> wrote:
>>> On Thu, 04 Feb 2021 20:39:48 +0000, Hector Martin <marcan@...can.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@...can.st)
Public Key: https://mrcn.st/pub