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: <1c441d20-951d-407b-90ba-4cda3b0505b2@app.fastmail.com>
Date:   Fri, 31 Mar 2023 12:44:54 +0200
From:   "Arnd Bergmann" <arnd@...db.de>
To:     Prabhakar <prabhakar.csengg@...il.com>
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 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.

> @@ -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.

     Arnd