[PATCH v2] target/riscv: Don't wrongly override isa version

LIU Zhiwei posted 1 patch 2 years, 9 months ago
Test checkpatch passed
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20210811144612.68674-1-zhiwei_liu@c-sky.com
Maintainers: Alistair Francis <alistair.francis@wdc.com>, Bin Meng <bin.meng@windriver.com>, Palmer Dabbelt <palmer@dabbelt.com>
target/riscv/cpu.c | 14 ++++++++------
1 file changed, 8 insertions(+), 6 deletions(-)
[PATCH v2] target/riscv: Don't wrongly override isa version
Posted by LIU Zhiwei 2 years, 9 months ago
For some cpu, the isa version has already been set in cpu init function.
Thus only override the isa version when isa version is not set, or
users set different isa version explicitly by cpu parameters.

Signed-off-by: LIU Zhiwei <zhiwei_liu@c-sky.com>
---
 target/riscv/cpu.c | 14 ++++++++------
 1 file changed, 8 insertions(+), 6 deletions(-)

diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
index 991a6bb760..1a2b03d579 100644
--- a/target/riscv/cpu.c
+++ b/target/riscv/cpu.c
@@ -392,9 +392,7 @@ static void riscv_cpu_realize(DeviceState *dev, Error **errp)
     RISCVCPU *cpu = RISCV_CPU(dev);
     CPURISCVState *env = &cpu->env;
     RISCVCPUClass *mcc = RISCV_CPU_GET_CLASS(dev);
-    int priv_version = PRIV_VERSION_1_11_0;
-    int bext_version = BEXT_VERSION_0_93_0;
-    int vext_version = VEXT_VERSION_0_07_1;
+    int priv_version = 0;
     target_ulong target_misa = env->misa;
     Error *local_err = NULL;
 
@@ -417,9 +415,11 @@ static void riscv_cpu_realize(DeviceState *dev, Error **errp)
         }
     }
 
-    set_priv_version(env, priv_version);
-    set_bext_version(env, bext_version);
-    set_vext_version(env, vext_version);
+    if (priv_version) {
+        set_priv_version(env, priv_version);
+    } else if (!env->priv_ver) {
+        set_priv_version(env, PRIV_VERSION_1_11_0);
+    }
 
     if (cpu->cfg.mmu) {
         set_feature(env, RISCV_FEATURE_MMU);
@@ -497,6 +497,7 @@ static void riscv_cpu_realize(DeviceState *dev, Error **errp)
             target_misa |= RVH;
         }
         if (cpu->cfg.ext_b) {
+            int bext_version = BEXT_VERSION_0_93_0;
             target_misa |= RVB;
 
             if (cpu->cfg.bext_spec) {
@@ -515,6 +516,7 @@ static void riscv_cpu_realize(DeviceState *dev, Error **errp)
             set_bext_version(env, bext_version);
         }
         if (cpu->cfg.ext_v) {
+            int vext_version = VEXT_VERSION_0_07_1;
             target_misa |= RVV;
             if (!is_power_of_2(cpu->cfg.vlen)) {
                 error_setg(errp,
-- 
2.25.1


Re: [PATCH v2] target/riscv: Don't wrongly override isa version
Posted by Bin Meng 2 years, 9 months ago
On Wed, Aug 11, 2021 at 10:46 PM LIU Zhiwei <zhiwei_liu@c-sky.com> wrote:
>
> For some cpu, the isa version has already been set in cpu init function.
> Thus only override the isa version when isa version is not set, or
> users set different isa version explicitly by cpu parameters.
>
> Signed-off-by: LIU Zhiwei <zhiwei_liu@c-sky.com>
> ---
>  target/riscv/cpu.c | 14 ++++++++------
>  1 file changed, 8 insertions(+), 6 deletions(-)

Please include a changelog in the newer version in the future.

>
> diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
> index 991a6bb760..1a2b03d579 100644
> --- a/target/riscv/cpu.c
> +++ b/target/riscv/cpu.c
> @@ -392,9 +392,7 @@ static void riscv_cpu_realize(DeviceState *dev, Error **errp)
>      RISCVCPU *cpu = RISCV_CPU(dev);
>      CPURISCVState *env = &cpu->env;
>      RISCVCPUClass *mcc = RISCV_CPU_GET_CLASS(dev);
> -    int priv_version = PRIV_VERSION_1_11_0;
> -    int bext_version = BEXT_VERSION_0_93_0;
> -    int vext_version = VEXT_VERSION_0_07_1;
> +    int priv_version = 0;
>      target_ulong target_misa = env->misa;
>      Error *local_err = NULL;
>
> @@ -417,9 +415,11 @@ static void riscv_cpu_realize(DeviceState *dev, Error **errp)
>          }
>      }
>
> -    set_priv_version(env, priv_version);
> -    set_bext_version(env, bext_version);
> -    set_vext_version(env, vext_version);
> +    if (priv_version) {
> +        set_priv_version(env, priv_version);
> +    } else if (!env->priv_ver) {
> +        set_priv_version(env, PRIV_VERSION_1_11_0);
> +    }
>
>      if (cpu->cfg.mmu) {
>          set_feature(env, RISCV_FEATURE_MMU);
> @@ -497,6 +497,7 @@ static void riscv_cpu_realize(DeviceState *dev, Error **errp)
>              target_misa |= RVH;
>          }
>          if (cpu->cfg.ext_b) {
> +            int bext_version = BEXT_VERSION_0_93_0;
>              target_misa |= RVB;
>
>              if (cpu->cfg.bext_spec) {
> @@ -515,6 +516,7 @@ static void riscv_cpu_realize(DeviceState *dev, Error **errp)
>              set_bext_version(env, bext_version);
>          }
>          if (cpu->cfg.ext_v) {
> +            int vext_version = VEXT_VERSION_0_07_1;
>              target_misa |= RVV;
>              if (!is_power_of_2(cpu->cfg.vlen)) {
>                  error_setg(errp,
> --

Reviewed-by: Bin Meng <bmeng.cn@gmail.com>

Re: [PATCH v2] target/riscv: Don't wrongly override isa version
Posted by Alistair Francis 2 years, 8 months ago
On Thu, Aug 12, 2021 at 12:46 AM LIU Zhiwei <zhiwei_liu@c-sky.com> wrote:
>
> For some cpu, the isa version has already been set in cpu init function.
> Thus only override the isa version when isa version is not set, or
> users set different isa version explicitly by cpu parameters.
>
> Signed-off-by: LIU Zhiwei <zhiwei_liu@c-sky.com>

Reviewed-by: Alistair Francis <alistair.francis@wdc.com>

Alistair

> ---
>  target/riscv/cpu.c | 14 ++++++++------
>  1 file changed, 8 insertions(+), 6 deletions(-)
>
> diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
> index 991a6bb760..1a2b03d579 100644
> --- a/target/riscv/cpu.c
> +++ b/target/riscv/cpu.c
> @@ -392,9 +392,7 @@ static void riscv_cpu_realize(DeviceState *dev, Error **errp)
>      RISCVCPU *cpu = RISCV_CPU(dev);
>      CPURISCVState *env = &cpu->env;
>      RISCVCPUClass *mcc = RISCV_CPU_GET_CLASS(dev);
> -    int priv_version = PRIV_VERSION_1_11_0;
> -    int bext_version = BEXT_VERSION_0_93_0;
> -    int vext_version = VEXT_VERSION_0_07_1;
> +    int priv_version = 0;
>      target_ulong target_misa = env->misa;
>      Error *local_err = NULL;
>
> @@ -417,9 +415,11 @@ static void riscv_cpu_realize(DeviceState *dev, Error **errp)
>          }
>      }
>
> -    set_priv_version(env, priv_version);
> -    set_bext_version(env, bext_version);
> -    set_vext_version(env, vext_version);
> +    if (priv_version) {
> +        set_priv_version(env, priv_version);
> +    } else if (!env->priv_ver) {
> +        set_priv_version(env, PRIV_VERSION_1_11_0);
> +    }
>
>      if (cpu->cfg.mmu) {
>          set_feature(env, RISCV_FEATURE_MMU);
> @@ -497,6 +497,7 @@ static void riscv_cpu_realize(DeviceState *dev, Error **errp)
>              target_misa |= RVH;
>          }
>          if (cpu->cfg.ext_b) {
> +            int bext_version = BEXT_VERSION_0_93_0;
>              target_misa |= RVB;
>
>              if (cpu->cfg.bext_spec) {
> @@ -515,6 +516,7 @@ static void riscv_cpu_realize(DeviceState *dev, Error **errp)
>              set_bext_version(env, bext_version);
>          }
>          if (cpu->cfg.ext_v) {
> +            int vext_version = VEXT_VERSION_0_07_1;
>              target_misa |= RVV;
>              if (!is_power_of_2(cpu->cfg.vlen)) {
>                  error_setg(errp,
> --
> 2.25.1
>
>