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

[Alistair Francis]

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

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