On Fri, Mar 31, 2023 at 11:45 AM Arnd Bergmann <arnd@xxxxxxxx> wrote: > > On Fri, Mar 31, 2023, at 12:37, Lad, Prabhakar wrote: > > On Thu, Mar 30, 2023 at 10:34 PM Arnd Bergmann <arnd@xxxxxxxx> 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
