| Message ID | 20230322222004.357013-19-dbarboza@ventanamicro.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | target/riscv: rework CPU extensions validation | expand |
On 2023/3/23 06:19, Daniel Henrique Barboza wrote: > 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. > > Signed-off-by: Daniel Henrique Barboza <dbarboza@ventanamicro.com> > --- > target/riscv/cpu.c | 14 ++++++++++++++ > 1 file changed, 14 insertions(+) > > diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c > index 399f63b42f..d2eb2b3ba1 100644 > --- a/target/riscv/cpu.c > +++ b/target/riscv/cpu.c > @@ -1055,6 +1055,20 @@ static void riscv_cpu_validate_misa_ext(RISCVCPU *cpu, Error **errp) > 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 (cpu->cfg.ext_h && env->priv_ver < PRIV_VERSION_1_12_0) { > + error_setg(errp, "H extension requires priv spec 1.12.0"); > + return; > + } The other multi-letter extensions are directly disabled for lower priv version with warning message. Whether we should do the similar action here? Regards, Weiwei Li > + > if (cpu->cfg.ext_v) { > riscv_cpu_validate_v(env, &cpu->cfg, &local_err); > if (local_err != NULL) {
On 3/24/23 11:56, liweiwei wrote: > > On 2023/3/23 06:19, Daniel Henrique Barboza wrote: >> 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. >> >> Signed-off-by: Daniel Henrique Barboza <dbarboza@ventanamicro.com> >> --- >> target/riscv/cpu.c | 14 ++++++++++++++ >> 1 file changed, 14 insertions(+) >> >> diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c >> index 399f63b42f..d2eb2b3ba1 100644 >> --- a/target/riscv/cpu.c >> +++ b/target/riscv/cpu.c >> @@ -1055,6 +1055,20 @@ static void riscv_cpu_validate_misa_ext(RISCVCPU *cpu, Error **errp) >> 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 (cpu->cfg.ext_h && env->priv_ver < PRIV_VERSION_1_12_0) { >> + error_setg(errp, "H extension requires priv spec 1.12.0"); >> + return; >> + } > The other multi-letter extensions are directly disabled for lower priv version with warning message. > > Whether we should do the similar action here? I'd rather error out in all cases, to be honest. cpu_init() functions should be responsible for choosing an adequate priv spec level for the extensions it wants to use. Thanks, Daniel > > Regards, > > Weiwei Li > >> + >> if (cpu->cfg.ext_v) { >> riscv_cpu_validate_v(env, &cpu->cfg, &local_err); >> if (local_err != NULL) { >
diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c index 399f63b42f..d2eb2b3ba1 100644 --- a/target/riscv/cpu.c +++ b/target/riscv/cpu.c @@ -1055,6 +1055,20 @@ static void riscv_cpu_validate_misa_ext(RISCVCPU *cpu, Error **errp) 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 (cpu->cfg.ext_h && env->priv_ver < PRIV_VERSION_1_12_0) { + error_setg(errp, "H extension requires priv spec 1.12.0"); + return; + } + if (cpu->cfg.ext_v) { riscv_cpu_validate_v(env, &cpu->cfg, &local_err); if (local_err != NULL) {
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. Signed-off-by: Daniel Henrique Barboza <dbarboza@ventanamicro.com> --- target/riscv/cpu.c | 14 ++++++++++++++ 1 file changed, 14 insertions(+)