| Message ID | 20230314164948.539135-23-dbarboza@ventanamicro.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | target/riscv: rework CPU extensions validation | expand |
On 2023/3/15 00:49, Daniel Henrique Barboza wrote: > We have one last case where we're changing env->misa_ext* during > validation. riscv_cpu_disable_priv_spec_isa_exts(), at the end of > riscv_cpu_validate_set_extensions(), will disable cpu->cfg.ext_h and > cpu->cfg.ext_v if priv_ver check fails. > > This check can be done in riscv_cpu_validate_misa_ext(). The difference > here is that we're not silently disable it: we'll error out. Silently > disabling a MISA extension after all the validation is completed can > can cause inconsistencies that we don't have to deal with. Verify ealier > and fail faster. > > Note that we're ignoring RVV priv_ver validation since its minimal priv > is also the minimal value we support. RVH will error out if enabled > under priv_ver under 1_12_0. > > As a bonus, we're guaranteeing that all env->misa_ext* changes will > happen up until riscv_set_G_virt_ext(). We don't have to worry about > keeping env->misa_ext in sync with cpu->cfg. > > Signed-off-by: Daniel Henrique Barboza <dbarboza@ventanamicro.com> > --- > target/riscv/cpu.c | 28 +++++++++++++++++++--------- > 1 file changed, 19 insertions(+), 9 deletions(-) > > diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c > index f8f416d6dd..1f72e1b8ce 100644 > --- a/target/riscv/cpu.c > +++ b/target/riscv/cpu.c > @@ -1067,6 +1067,20 @@ static void riscv_cpu_validate_misa_ext(CPURISCVState *env, > return; > } > > + /* > + * Check for priv spec version. RVH is 1_12_0, RVV is 1_10_0. > + * We don't support anything under 1_10_0 so skip checking > + * priv for RVV. > + * > + * We're hardcoding it here to avoid looping into the > + * 50+ entries of isa_edata_arr[] just to check the RVH > + * entry. > + */ > + if (misa_ext & RVH && env->priv_ver < PRIV_VERSION_1_12_0) { > + error_setg(errp, "H extension requires priv spec 1.12.0"); > + return; > + } > + > if (misa_ext & RVV) { > /* > * V depends on Zve64d, which requires D. It also > @@ -1117,14 +1131,10 @@ static void riscv_cpu_validate_misa_mxl(RISCVCPU *cpu, Error **errp) > > /* > * Check consistency between chosen extensions while setting > - * cpu->cfg accordingly, setting env->misa_ext and > - * misa_ext_mask in the end. > + * cpu->cfg accordingly. > */ > static void riscv_cpu_validate_set_extensions(RISCVCPU *cpu, Error **errp) > { > - CPURISCVState *env = &cpu->env; > - uint32_t ext = 0; > - > if (cpu->cfg.epmp && !cpu->cfg.pmp) { > /* > * Enhanced PMP should only be available > @@ -1241,10 +1251,6 @@ static void riscv_cpu_validate_set_extensions(RISCVCPU *cpu, Error **errp) > * validated and set everything we need. > */ > riscv_cpu_disable_priv_spec_isa_exts(cpu); > - > - ext = riscv_get_misa_ext_with_cpucfg(&cpu->cfg); > - > - env->misa_ext_mask = env->misa_ext = ext; This can be removed in the patch 17. Regards, Weiwei Li > } > > #ifndef CONFIG_USER_ONLY > @@ -1355,6 +1361,10 @@ static void riscv_cpu_realize(DeviceState *dev, Error **errp) > return; > } > > + /* > + * This is the last point where env->misa_ext* can > + * be changed. > + */ > if (cpu->cfg.ext_g) { > riscv_set_G_virt_ext(cpu); > }
diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c index f8f416d6dd..1f72e1b8ce 100644 --- a/target/riscv/cpu.c +++ b/target/riscv/cpu.c @@ -1067,6 +1067,20 @@ static void riscv_cpu_validate_misa_ext(CPURISCVState *env, return; } + /* + * Check for priv spec version. RVH is 1_12_0, RVV is 1_10_0. + * We don't support anything under 1_10_0 so skip checking + * priv for RVV. + * + * We're hardcoding it here to avoid looping into the + * 50+ entries of isa_edata_arr[] just to check the RVH + * entry. + */ + if (misa_ext & RVH && env->priv_ver < PRIV_VERSION_1_12_0) { + error_setg(errp, "H extension requires priv spec 1.12.0"); + return; + } + if (misa_ext & RVV) { /* * V depends on Zve64d, which requires D. It also @@ -1117,14 +1131,10 @@ static void riscv_cpu_validate_misa_mxl(RISCVCPU *cpu, Error **errp) /* * Check consistency between chosen extensions while setting - * cpu->cfg accordingly, setting env->misa_ext and - * misa_ext_mask in the end. + * cpu->cfg accordingly. */ static void riscv_cpu_validate_set_extensions(RISCVCPU *cpu, Error **errp) { - CPURISCVState *env = &cpu->env; - uint32_t ext = 0; - if (cpu->cfg.epmp && !cpu->cfg.pmp) { /* * Enhanced PMP should only be available @@ -1241,10 +1251,6 @@ static void riscv_cpu_validate_set_extensions(RISCVCPU *cpu, Error **errp) * validated and set everything we need. */ riscv_cpu_disable_priv_spec_isa_exts(cpu); - - ext = riscv_get_misa_ext_with_cpucfg(&cpu->cfg); - - env->misa_ext_mask = env->misa_ext = ext; } #ifndef CONFIG_USER_ONLY @@ -1355,6 +1361,10 @@ static void riscv_cpu_realize(DeviceState *dev, Error **errp) return; } + /* + * This is the last point where env->misa_ext* can + * be changed. + */ if (cpu->cfg.ext_g) { riscv_set_G_virt_ext(cpu); }
We have one last case where we're changing env->misa_ext* during validation. riscv_cpu_disable_priv_spec_isa_exts(), at the end of riscv_cpu_validate_set_extensions(), will disable cpu->cfg.ext_h and cpu->cfg.ext_v if priv_ver check fails. This check can be done in riscv_cpu_validate_misa_ext(). The difference here is that we're not silently disable it: we'll error out. Silently disabling a MISA extension after all the validation is completed can can cause inconsistencies that we don't have to deal with. Verify ealier and fail faster. Note that we're ignoring RVV priv_ver validation since its minimal priv is also the minimal value we support. RVH will error out if enabled under priv_ver under 1_12_0. As a bonus, we're guaranteeing that all env->misa_ext* changes will happen up until riscv_set_G_virt_ext(). We don't have to worry about keeping env->misa_ext in sync with cpu->cfg. Signed-off-by: Daniel Henrique Barboza <dbarboza@ventanamicro.com> --- target/riscv/cpu.c | 28 +++++++++++++++++++--------- 1 file changed, 19 insertions(+), 9 deletions(-)