Re: [PATCH for-8.1 v2 22/26] target/riscv: error out on priv failure for RVH

2023-03-14 Thread liweiwei



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 
---
  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 = >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(>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);
  }





[PATCH for-8.1 v2 22/26] target/riscv: error out on priv failure for RVH

2023-03-14 Thread Daniel Henrique Barboza
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 
---
 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 = >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(>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);
 }
-- 
2.39.2