Re: [PATCH v3 08/15] riscv: Add Zic64b to cpufeature and hwprobe

[Conor Dooley <conor@kernel.org>]

[-- Attachment #1: Type: text/plain, Size: 2496 bytes --]

On Thu, Jun 04, 2026 at 02:42:22PM +0800, Guodong Xu wrote:
> On Wed, Jun 3, 2026 at 7:28 AM <sashiko-bot@kernel.org> wrote:
> >
> > > @@ -154,6 +154,23 @@ static int riscv_ext_zicbop_validate(const struct riscv_isa_ext_data *data,
> > >       return 0;
> > >  }
> > >
> > > +static int riscv_ext_zic64b_validate(const struct riscv_isa_ext_data *data,
> > > +                                  const unsigned long *isa_bitmap)
> > > +{
> > > +     /*
> > > +      * Zic64b mandates 64-byte naturally aligned cache blocks; cross-check the
> > > +      * cbom/cbop/cboz block-size device-tree properties to avoid inconsistency.
> > > +      */
> > > +     if (riscv_cbom_block_size != 64 ||
> > > +         riscv_cbop_block_size != 64 ||
> > > +         riscv_cboz_block_size != 64) {
> >
> > [Severity: Medium]
> > Will this check erroneously disable Zic64b if a platform does not implement
> > all three CBO extensions?
> 
> Zic64b is mandatory in RVA22/RVA23, which also mandate Zicbom,
> Zicbop and Zicboz, so all three block sizes must be present on any Zic64b
> platform.

Where do you get that conclusion from? That's not how we treat any other
extension defined in the profiles spec. My understanding was that even
if things are defined in the profiles spec that they're fair game for
independent use.

> 
> And architecturally, all the block sizes are one quantity: under Zic64b
> that value is 64, so the three must all read 64.
> 
> Anything else is a misconfigured DT, and the kernel should not trust
> the zic64b it claims.
> 
> BR,
> Guodong
> 
> >
> > If a hardware platform implements Zic64b but omits an optional extension
> > like Zicbop, the riscv,cbop-block-size property might be absent from the
> > device tree, leaving riscv_cbop_block_size initialized at 0.
> >
> > This unconditional check evaluates 0 != 64 as true, which would spuriously
> > trigger this error path and disable the valid Zic64b extension.
> >
> > Should this validation only enforce the 64-byte size if the corresponding
> > block size variable is non-zero, indicating the extension is actually present?
> >
> > > +             pr_err("Zic64b detected in ISA string, disabling as the cache block size is not 64 bytes\n");
> > > +             return -EINVAL;
> > > +     }
> > > +
> > > +     return 0;
> > > +}
> >
> > --
> > Sashiko AI review · https://sashiko.dev/#/patchset/20260603-rva23u64-hwprobe-v2-v3-0-5529a7b28384@gmail.com?part=8

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]