linux-kernel - Re: [PATCH v7 1/6] riscv: mm: dma-noncoherent: Switch using function pointers for cache management

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>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CA+V-a8t_kozAobT028-RN39uoXcd7-0oJAhAR24MEzmoLVKX8Q@mail.gmail.com>
Date:   Fri, 31 Mar 2023 13:11:49 +0100
From:   "Lad, Prabhakar" <prabhakar.csengg@...il.com>
To:     Arnd Bergmann <arnd@...db.de>
Cc:     "Conor.Dooley" <conor.dooley@...rochip.com>,
        Geert Uytterhoeven <geert+renesas@...der.be>,
        Heiko Stübner <heiko@...ech.de>,
        guoren <guoren@...nel.org>,
        Andrew Jones <ajones@...tanamicro.com>,
        Paul Walmsley <paul.walmsley@...ive.com>,
        Palmer Dabbelt <palmer@...belt.com>,
        Albert Ou <aou@...s.berkeley.edu>,
        Samuel Holland <samuel@...lland.org>,
        linux-riscv@...ts.infradead.org, Rob Herring <robh+dt@...nel.org>,
        Krzysztof Kozlowski <krzysztof.kozlowski+dt@...aro.org>,
        devicetree@...r.kernel.org, linux-kernel@...r.kernel.org,
        Linux-Renesas <linux-renesas-soc@...r.kernel.org>,
        Biju Das <biju.das.jz@...renesas.com>,
        "Lad, Prabhakar" <prabhakar.mahadev-lad.rj@...renesas.com>
Subject: Re: [PATCH v7 1/6] riscv: mm: dma-noncoherent: Switch using function
 pointers for cache management

On Fri, Mar 31, 2023 at 11:45 AM Arnd Bergmann <arnd@...db.de> wrote:
>
> On Fri, Mar 31, 2023, at 12:37, Lad, Prabhakar wrote:
> > On Thu, Mar 30, 2023 at 10:34 PM Arnd Bergmann <arnd@...db.de> wrote:
> >
> >> It also seems wrong to have the fallback be to do nothing
> >> when the pointer is NULL, since that cannot actually work
> >> when a device is not cache coherent.
> >>
> > If the device is non cache coherent and if it doesn't support ZICBOM
> > ISA extension the device won't work anyway. So non-cache coherent
> > devices until they have their CMO config enabled won't work anyway. So
> > I didn't see any benefit in enabling ZICBOM by default. Please let me
> > know if I am misunderstanding.
>
> Two things:
>
> - Having a broken machine crash with in invalid instruction
>   exception is better than having it run into silent data
>   corruption.
>
> - a correctly predicted branch is typically faster than an
>   indirect function call, so the fallback to zicbom makes the
>   expected (at least in the future) case the fast one.
>
Ok, thank you for the clarification. I'll default to zicbom.

> > @@ -465,7 +466,6 @@ config RISCV_ISA_ZICBOM
> >         depends on MMU
> >         depends on RISCV_ALTERNATIVE
> >         default y
> > -       select RISCV_DMA_NONCOHERENT
> >         help
> >            Adds support to dynamically detect the presence of the ZICBOM
> >            extension (Cache Block Management Operations) and enable its
> >
> > But what if the platform doesn't have the ZICBOM ISA extension?
>
> Then it needs to register its cache operations before the first
> DMA, which is something that it should do anyway. With your
> current code, it may work by accident depending on the state of
> the cache, but with the version I suggested, it will either work
> correctly all the time or crash in an obvious way when misconfigured.
>
Okay, agreed.

Cheers,
Prabhakar