| Message ID | 20230327224934.363314-3-dbarboza@ventanamicro.com |
|---|---|
| State | New |
| Headers | show |
| Series | remove MISA ext_N flags from cpu->cfg | expand |
On 2023/3/28 06:49, Daniel Henrique Barboza wrote: > The code that disables extensions if there's a priv version mismatch > uses cpu->cfg.ext_N properties to do its job. > > We're aiming to not rely on cpu->cfg.ext_N props for MISA bits. Split > the MISA related verifications in a new function, removing it from > isa_edata_arr[]. > > We're also erroring it out instead of disabling, making the cpu_init() > function responsible for running an adequate priv spec for the MISA > extensions it wants to use. > > Note that the RVV verification is being ignored since we're always have > at least PRIV_VERSION_1_10_0. > > Signed-off-by: Daniel Henrique Barboza <dbarboza@ventanamicro.com> > --- > target/riscv/cpu.c | 19 +++++++++++++++++-- > 1 file changed, 17 insertions(+), 2 deletions(-) > > diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c > index 2711d80e16..21c0c637e4 100644 > --- a/target/riscv/cpu.c > +++ b/target/riscv/cpu.c > @@ -72,10 +72,11 @@ struct isa_ext_data { > * 4. Non-standard extensions (starts with 'X') must be listed after all > * standard extensions. They must be separated from other multi-letter > * extensions by an underscore. > + * > + * Single letter extensions are checked in riscv_cpu_validate_misa_priv() > + * instead. > */ > static const struct isa_ext_data isa_edata_arr[] = { > - ISA_EXT_DATA_ENTRY(h, false, PRIV_VERSION_1_12_0, ext_h), > - ISA_EXT_DATA_ENTRY(v, false, PRIV_VERSION_1_10_0, ext_v), If misa properties are removed from here, the multi_letter field in isa_edata_arr will be redundant. We can just remove it. Otherwise, the all patchset is LGTM. Reviewed-by: Weiwei Li <liweiwei@iscas.ac.cn> Weiwei Li > ISA_EXT_DATA_ENTRY(zicbom, true, PRIV_VERSION_1_12_0, ext_icbom), > ISA_EXT_DATA_ENTRY(zicboz, true, PRIV_VERSION_1_12_0, ext_icboz), > ISA_EXT_DATA_ENTRY(zicond, true, PRIV_VERSION_1_12_0, ext_zicond), > @@ -1131,6 +1132,14 @@ static void riscv_cpu_sync_misa_cfg(CPURISCVState *env) > env->misa_ext = env->misa_ext_mask = ext; > } > > +static void riscv_cpu_validate_misa_priv(CPURISCVState *env, Error **errp) > +{ > + if (riscv_has_ext(env, RVH) && env->priv_ver < PRIV_VERSION_1_12_0) { > + error_setg(errp, "H extension requires priv spec 1.12.0"); > + return; > + } > +} > + > static void riscv_cpu_realize(DeviceState *dev, Error **errp) > { > CPUState *cs = CPU(dev); > @@ -1174,6 +1183,12 @@ static void riscv_cpu_realize(DeviceState *dev, Error **errp) > */ > riscv_cpu_sync_misa_cfg(env); > > + riscv_cpu_validate_misa_priv(env, &local_err); > + if (local_err != NULL) { > + error_propagate(errp, local_err); > + return; > + } > + > /* Force disable extensions if priv spec version does not match */ > for (i = 0; i < ARRAY_SIZE(isa_edata_arr); i++) { > if (isa_ext_is_enabled(cpu, &isa_edata_arr[i]) &&
On 3/29/23 05:32, liweiwei wrote: > > On 2023/3/28 06:49, Daniel Henrique Barboza wrote: >> The code that disables extensions if there's a priv version mismatch >> uses cpu->cfg.ext_N properties to do its job. >> >> We're aiming to not rely on cpu->cfg.ext_N props for MISA bits. Split >> the MISA related verifications in a new function, removing it from >> isa_edata_arr[]. >> >> We're also erroring it out instead of disabling, making the cpu_init() >> function responsible for running an adequate priv spec for the MISA >> extensions it wants to use. >> >> Note that the RVV verification is being ignored since we're always have >> at least PRIV_VERSION_1_10_0. >> >> Signed-off-by: Daniel Henrique Barboza <dbarboza@ventanamicro.com> >> --- >> target/riscv/cpu.c | 19 +++++++++++++++++-- >> 1 file changed, 17 insertions(+), 2 deletions(-) >> >> diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c >> index 2711d80e16..21c0c637e4 100644 >> --- a/target/riscv/cpu.c >> +++ b/target/riscv/cpu.c >> @@ -72,10 +72,11 @@ struct isa_ext_data { >> * 4. Non-standard extensions (starts with 'X') must be listed after all >> * standard extensions. They must be separated from other multi-letter >> * extensions by an underscore. >> + * >> + * Single letter extensions are checked in riscv_cpu_validate_misa_priv() >> + * instead. >> */ >> static const struct isa_ext_data isa_edata_arr[] = { >> - ISA_EXT_DATA_ENTRY(h, false, PRIV_VERSION_1_12_0, ext_h), >> - ISA_EXT_DATA_ENTRY(v, false, PRIV_VERSION_1_10_0, ext_v), > > If misa properties are removed from here, the multi_letter field in isa_edata_arr will be redundant. > > We can just remove it. Otherwise, the all patchset is LGTM. Good point. I'll remove it. > > Reviewed-by: Weiwei Li <liweiwei@iscas.ac.cn> Thanks! Daniel > > Weiwei Li > >> ISA_EXT_DATA_ENTRY(zicbom, true, PRIV_VERSION_1_12_0, ext_icbom), >> ISA_EXT_DATA_ENTRY(zicboz, true, PRIV_VERSION_1_12_0, ext_icboz), >> ISA_EXT_DATA_ENTRY(zicond, true, PRIV_VERSION_1_12_0, ext_zicond), >> @@ -1131,6 +1132,14 @@ static void riscv_cpu_sync_misa_cfg(CPURISCVState *env) >> env->misa_ext = env->misa_ext_mask = ext; >> } >> +static void riscv_cpu_validate_misa_priv(CPURISCVState *env, Error **errp) >> +{ >> + if (riscv_has_ext(env, RVH) && env->priv_ver < PRIV_VERSION_1_12_0) { >> + error_setg(errp, "H extension requires priv spec 1.12.0"); >> + return; >> + } >> +} >> + >> static void riscv_cpu_realize(DeviceState *dev, Error **errp) >> { >> CPUState *cs = CPU(dev); >> @@ -1174,6 +1183,12 @@ static void riscv_cpu_realize(DeviceState *dev, Error **errp) >> */ >> riscv_cpu_sync_misa_cfg(env); >> + riscv_cpu_validate_misa_priv(env, &local_err); >> + if (local_err != NULL) { >> + error_propagate(errp, local_err); >> + return; >> + } >> + >> /* Force disable extensions if priv spec version does not match */ >> for (i = 0; i < ARRAY_SIZE(isa_edata_arr); i++) { >> if (isa_ext_is_enabled(cpu, &isa_edata_arr[i]) && >
diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c index 2711d80e16..21c0c637e4 100644 --- a/target/riscv/cpu.c +++ b/target/riscv/cpu.c @@ -72,10 +72,11 @@ struct isa_ext_data { * 4. Non-standard extensions (starts with 'X') must be listed after all * standard extensions. They must be separated from other multi-letter * extensions by an underscore. + * + * Single letter extensions are checked in riscv_cpu_validate_misa_priv() + * instead. */ static const struct isa_ext_data isa_edata_arr[] = { - ISA_EXT_DATA_ENTRY(h, false, PRIV_VERSION_1_12_0, ext_h), - ISA_EXT_DATA_ENTRY(v, false, PRIV_VERSION_1_10_0, ext_v), ISA_EXT_DATA_ENTRY(zicbom, true, PRIV_VERSION_1_12_0, ext_icbom), ISA_EXT_DATA_ENTRY(zicboz, true, PRIV_VERSION_1_12_0, ext_icboz), ISA_EXT_DATA_ENTRY(zicond, true, PRIV_VERSION_1_12_0, ext_zicond), @@ -1131,6 +1132,14 @@ static void riscv_cpu_sync_misa_cfg(CPURISCVState *env) env->misa_ext = env->misa_ext_mask = ext; } +static void riscv_cpu_validate_misa_priv(CPURISCVState *env, Error **errp) +{ + if (riscv_has_ext(env, RVH) && env->priv_ver < PRIV_VERSION_1_12_0) { + error_setg(errp, "H extension requires priv spec 1.12.0"); + return; + } +} + static void riscv_cpu_realize(DeviceState *dev, Error **errp) { CPUState *cs = CPU(dev); @@ -1174,6 +1183,12 @@ static void riscv_cpu_realize(DeviceState *dev, Error **errp) */ riscv_cpu_sync_misa_cfg(env); + riscv_cpu_validate_misa_priv(env, &local_err); + if (local_err != NULL) { + error_propagate(errp, local_err); + return; + } + /* Force disable extensions if priv spec version does not match */ for (i = 0; i < ARRAY_SIZE(isa_edata_arr); i++) { if (isa_ext_is_enabled(cpu, &isa_edata_arr[i]) &&
The code that disables extensions if there's a priv version mismatch uses cpu->cfg.ext_N properties to do its job. We're aiming to not rely on cpu->cfg.ext_N props for MISA bits. Split the MISA related verifications in a new function, removing it from isa_edata_arr[]. We're also erroring it out instead of disabling, making the cpu_init() function responsible for running an adequate priv spec for the MISA extensions it wants to use. Note that the RVV verification is being ignored since we're always have at least PRIV_VERSION_1_10_0. Signed-off-by: Daniel Henrique Barboza <dbarboza@ventanamicro.com> --- target/riscv/cpu.c | 19 +++++++++++++++++-- 1 file changed, 17 insertions(+), 2 deletions(-)