Re: [PATCH v2] RISC-V: Clean up the Zicbom block size probing

[Andrew Jones <ajones@ventanamicro.com>]

On Fri, Sep 02, 2022 at 09:55:27AM +0000, Conor.Dooley@microchip.com wrote:
> On 01/09/2022 16:57, Heiko Stübner wrote:
> > EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
> > 
> > Am Freitag, 12. August 2022, 17:40:10 CEST schrieb Palmer Dabbelt:
> >> This fixes two issues: I truncated the warning's hart ID when porting to
> >> the 64-bit hart ID code, and the original code's warning handling could
> >> fire on an uninitialized hart ID.
> >>
> >> The biggest change here is that riscv_cbom_block_size is no longer
> >> initialized, as IMO the default isn't sane: there's nothing in the ISA
> >> that mandates any specific cache block size, so falling back to one will
> >> just silently produce the wrong answer on some systems.  This also
> >> changes the probing order so the cache block size is known before
> >> enabling Zicbom support.
> >>
> >> Fixes: 3aefb2ee5bdd ("riscv: implement Zicbom-based CMO instructions + the t-head variant")
> >> Fixes: 1631ba1259d6 ("riscv: Add support for non-coherent devices using zicbom extension")
> >> Reported-by: kernel test robot <lkp@intel.com>
> >> Signed-off-by: Palmer Dabbelt <palmer@rivosinc.com>
> > 
> > With Conor's fixes folded in, this compiles and breaks the T-Head CMOs
> > as they rely on that default value :-) .
> > 
> > Can we do the following:
> > 
> > (1) pick Anup's patch moving the block-size init over to cacheflush [0]
> > (2) apply this patch (with Conor's fixes and adapted to the changed
> >      location) and add this one additional line:
> > 
> > diff --git a/arch/riscv/errata/thead/errata.c b/arch/riscv/errata/thead/errata.c
> > index d4b1526538ad..67fa078f303f 100644
> > --- a/arch/riscv/errata/thead/errata.c
> > +++ b/arch/riscv/errata/thead/errata.c
> > @@ -42,6 +42,7 @@ static bool errata_probe_cmo(unsigned int stage,
> >          if (stage == RISCV_ALTERNATIVES_EARLY_BOOT)
> >                  return false;
> > 
> > +       riscv_cbom_block_size = L1_CACHE_BYTES;
> >          riscv_noncoherent_supported();
> >          return true;
> >   }
> > 
> > 
> > With that done everything works (again) and looks great, so would be
> > Reviewed-by: Heiko Stuebner <heiko@sntech.de>
> > Tested-by: Heiko Stuebner <heiko@sntech.de>
> 
> With all of the above, it'd also be:
> Reviewed-by: Conor Dooley <conor.dooley@microchip.com>

I needed these fixes for a patch series enabling KVM guests to use Zicbom
so I've gone ahead and made Conor's and Heiko's changes [1]. I rebased
Anup's moving patch on top of that [2]. I'll go ahead and post this patch
separately in case that makes things easier.

[1] https://github.com/jones-drew/linux/commit/af361283ec3129846307f787a3ebb19bd4a9c421
[2] https://github.com/jones-drew/linux/commit/61c404299c63c8706c129d2a67071f5aae94594f

(I forgot Conor's r-b on this patch in the branch, but I'll pick it up now
while posting.)

Thanks,
drew

> 
> > 
> > 
> > Heiko
> > 
> > [0] https://lore.kernel.org/r/20220830044642.566769-3-apatel@ventanamicro.com
> > 
> > 
> 
> _______________________________________________
> linux-riscv mailing list
> linux-riscv@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-riscv

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv