Re: [PATCH v2 7/8] target/riscv: Force disable extensions if priv spec version does not match

[Anup Patel]

On Tue, May 24, 2022 at 3:22 AM Alistair Francis <alistair23@gmail.com> wrote:
>
> On Fri, May 20, 2022 at 1:07 AM Anup Patel <anup@brainfault.org> wrote:
> >
> > On Tue, May 17, 2022 at 5:46 AM Alistair Francis <alistair23@gmail.com> 
> > wrote:
> > >
> > > On Thu, May 12, 2022 at 12:52 AM Anup Patel <apatel@ventanamicro.com> 
> > > wrote:
> > > >
> > > > We should disable extensions in riscv_cpu_realize() if minimum required
> > > > priv spec version is not satisfied. This also ensures that machines with
> > > > priv spec v1.11 (or lower) cannot enable H, V, and various multi-letter
> > > > extensions.
> > > >
> > > > Fixes: a775398be2e ("target/riscv: Add isa extenstion strings to the
> > > > device tree")
> > > > Signed-off-by: Anup Patel <apatel@ventanamicro.com>
> > >
> > > This will potentially confuse users as we just disable the extension
> > > without telling them.
> > >
> > > Could we not just leave this as is and let users specify the
> > > extensions they want? Then it's up to them to specify the correct
> > > combinations
> >
> > The ISA extensions are not independent of the Priv spec version.
> >
> > For example, we have bits for Sstc, Svpbmt, and Zicbo[m|p|z] extensions
> > in xenvcfg CSRs which are only available for Priv v1.12 spec.
> >
> > We can't allow users to enable extensions which don't meet
> > the Priv spec version requirements.
>
> Fair point. Ok we should at least report a warning if any of these are
> set though

Okay, I will update this patch accordingly.

Regards,
Anup

>
> Alistair
>
> >
> > Regards,
> > Anup
> >
> > >
> > > Alistair
> > >
> > > > ---
> > > >  target/riscv/cpu.c | 34 ++++++++++++++++++++++++++++++++++
> > > >  1 file changed, 34 insertions(+)
> > > >
> > > > diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
> > > > index f3b61dfd63..25a4ba3e22 100644
> > > > --- a/target/riscv/cpu.c
> > > > +++ b/target/riscv/cpu.c
> > > > @@ -541,6 +541,40 @@ static void riscv_cpu_realize(DeviceState *dev, 
> > > > Error **errp)
> > > >          set_priv_version(env, priv_version);
> > > >      }
> > > >
> > > > +    /* Force disable extensions if priv spec version does not match */
> > > > +    if (env->priv_ver < PRIV_VERSION_1_12_0) {
> > > > +        cpu->cfg.ext_h = false;
> > > > +        cpu->cfg.ext_v = false;
> > > > +        cpu->cfg.ext_zfh = false;
> > > > +        cpu->cfg.ext_zfhmin = false;
> > > > +        cpu->cfg.ext_zfinx = false;
> > > > +        cpu->cfg.ext_zhinx = false;
> > > > +        cpu->cfg.ext_zhinxmin = false;
> > > > +        cpu->cfg.ext_zdinx = false;
> > > > +        cpu->cfg.ext_zba = false;
> > > > +        cpu->cfg.ext_zbb = false;
> > > > +        cpu->cfg.ext_zbc = false;
> > > > +        cpu->cfg.ext_zbkb = false;
> > > > +        cpu->cfg.ext_zbkc = false;
> > > > +        cpu->cfg.ext_zbkx = false;
> > > > +        cpu->cfg.ext_zbs = false;
> > > > +        cpu->cfg.ext_zk = false;
> > > > +        cpu->cfg.ext_zkn = false;
> > > > +        cpu->cfg.ext_zknd = false;
> > > > +        cpu->cfg.ext_zkne = false;
> > > > +        cpu->cfg.ext_zknh = false;
> > > > +        cpu->cfg.ext_zkr = false;
> > > > +        cpu->cfg.ext_zks = false;
> > > > +        cpu->cfg.ext_zksed = false;
> > > > +        cpu->cfg.ext_zksh = false;
> > > > +        cpu->cfg.ext_zkt = false;
> > > > +        cpu->cfg.ext_zve32f = false;
> > > > +        cpu->cfg.ext_zve64f = false;
> > > > +        cpu->cfg.ext_svinval = false;
> > > > +        cpu->cfg.ext_svnapot = false;
> > > > +        cpu->cfg.ext_svpbmt = false;
> > > > +    }
> > > > +
> > > >      if (cpu->cfg.mmu) {
> > > >          riscv_set_feature(env, RISCV_FEATURE_MMU);
> > > >      }
> > > > --
> > > > 2.34.1
> > > >
> > > >