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

[Anup Patel]

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.

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