target/riscv: store max SATP mode as a single integer in RISCVCPUConfig

[PATCH 4/7] target/riscv: cpu: store max SATP mode as a single integer

Posted by Paolo Bonzini 11 months, 3 weeks ago

The maximum available SATP mode implies all the shorter virtual address sizes.
Store it in RISCVCPUConfig and avoid recomputing it via satp_mode_max_from_map.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 target/riscv/cpu_cfg.h     |  1 +
 target/riscv/cpu.c         | 11 +++++------
 target/riscv/tcg/tcg-cpu.c |  3 ++-
 3 files changed, 8 insertions(+), 7 deletions(-)

diff --git a/target/riscv/cpu_cfg.h b/target/riscv/cpu_cfg.h
index b410b1e6038..28d8de978fa 100644
--- a/target/riscv/cpu_cfg.h
+++ b/target/riscv/cpu_cfg.h
@@ -192,6 +192,7 @@ struct RISCVCPUConfig {
     bool short_isa_string;
 
 #ifndef CONFIG_USER_ONLY
+    int8_t max_satp_mode;
     RISCVSATPMap satp_mode;
 #endif
 };
diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
index 7950b6447f8..2d06543217a 100644
--- a/target/riscv/cpu.c
+++ b/target/riscv/cpu.c
@@ -444,6 +444,7 @@ static void set_satp_mode_max_supported(RISCVCPU *cpu,
     }
 
     assert(cpu->cfg.satp_mode.supported & (1 << satp_mode));
+    cpu->cfg.max_satp_mode = satp_mode;
 }
 
 /* Set the satp mode to the max supported */
@@ -1177,16 +1178,13 @@ static void riscv_cpu_disas_set_info(CPUState *s, disassemble_info *info)
 static void riscv_cpu_satp_mode_finalize(RISCVCPU *cpu, Error **errp)
 {
     bool rv32 = riscv_cpu_is_32bit(cpu);
-    uint8_t satp_mode_map_max, satp_mode_supported_max;
+    uint8_t satp_mode_map_max;
 
     /* The CPU wants the OS to decide which satp mode to use */
     if (cpu->cfg.satp_mode.supported == 0) {
         return;
     }
 
-    satp_mode_supported_max =
-                    satp_mode_max_from_map(cpu->cfg.satp_mode.supported);
-
     if (cpu->cfg.satp_mode.map == 0) {
         if (cpu->cfg.satp_mode.init == 0) {
             /* If unset by the user, we fallback to the default satp mode. */
@@ -1215,10 +1213,10 @@ static void riscv_cpu_satp_mode_finalize(RISCVCPU *cpu, Error **errp)
     satp_mode_map_max = satp_mode_max_from_map(cpu->cfg.satp_mode.map);
 
     /* Make sure the user asked for a supported configuration (HW and qemu) */
-    if (satp_mode_map_max > satp_mode_supported_max) {
+    if (satp_mode_map_max > cpu->cfg.max_satp_mode) {
         error_setg(errp, "satp_mode %s is higher than hw max capability %s",
                    satp_mode_str(satp_mode_map_max, rv32),
-                   satp_mode_str(satp_mode_supported_max, rv32));
+                   satp_mode_str(cpu->cfg.max_satp_mode, rv32));
         return;
     }
 
@@ -1477,6 +1475,7 @@ static void riscv_cpu_init(Object *obj)
     cpu->cfg.cbom_blocksize = 64;
     cpu->cfg.cbop_blocksize = 64;
     cpu->cfg.cboz_blocksize = 64;
+    cpu->cfg.max_satp_mode = -1;
     cpu->env.vext_ver = VEXT_VERSION_1_00_0;
 }
 
diff --git a/target/riscv/tcg/tcg-cpu.c b/target/riscv/tcg/tcg-cpu.c
index 0a137281de1..a9f59a67e00 100644
--- a/target/riscv/tcg/tcg-cpu.c
+++ b/target/riscv/tcg/tcg-cpu.c
@@ -693,8 +693,9 @@ static bool riscv_cpu_validate_profile_satp(RISCVCPU *cpu,
                                             RISCVCPUProfile *profile,
                                             bool send_warn)
 {
-    int satp_max = satp_mode_max_from_map(cpu->cfg.satp_mode.supported);
+    int satp_max = cpu->cfg.max_satp_mode;
 
+    assert(satp_max >= 0);
     if (profile->satp_mode > satp_max) {
         if (send_warn) {
             bool is_32bit = riscv_cpu_is_32bit(cpu);
-- 
2.48.1

Re: [PATCH 4/7] target/riscv: cpu: store max SATP mode as a single integer

Posted by Alistair Francis 11 months, 1 week ago

On Wed, Feb 19, 2025 at 3:00 AM Paolo Bonzini <pbonzini@redhat.com> wrote:
>
> The maximum available SATP mode implies all the shorter virtual address sizes.
> Store it in RISCVCPUConfig and avoid recomputing it via satp_mode_max_from_map.
>
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>

This fails to build on the latest riscv-to-apply.next
(https://github.com/alistair23/qemu/tree/riscv-to-apply.next)

../target/riscv/cpu.c: In function ‘riscv_cpu_init’:
../target/riscv/cpu.c:1481:13: error: ‘RISCVCPUConfig’ has no member
named ‘max_satp_mode’
 1481 |     cpu->cfg.max_satp_mode = -1;
      |             ^

Do you mind rebasing?

Alistair

> ---
>  target/riscv/cpu_cfg.h     |  1 +
>  target/riscv/cpu.c         | 11 +++++------
>  target/riscv/tcg/tcg-cpu.c |  3 ++-
>  3 files changed, 8 insertions(+), 7 deletions(-)
>
> diff --git a/target/riscv/cpu_cfg.h b/target/riscv/cpu_cfg.h
> index b410b1e6038..28d8de978fa 100644
> --- a/target/riscv/cpu_cfg.h
> +++ b/target/riscv/cpu_cfg.h
> @@ -192,6 +192,7 @@ struct RISCVCPUConfig {
>      bool short_isa_string;
>
>  #ifndef CONFIG_USER_ONLY
> +    int8_t max_satp_mode;
>      RISCVSATPMap satp_mode;
>  #endif
>  };
> diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
> index 7950b6447f8..2d06543217a 100644
> --- a/target/riscv/cpu.c
> +++ b/target/riscv/cpu.c
> @@ -444,6 +444,7 @@ static void set_satp_mode_max_supported(RISCVCPU *cpu,
>      }
>
>      assert(cpu->cfg.satp_mode.supported & (1 << satp_mode));
> +    cpu->cfg.max_satp_mode = satp_mode;
>  }
>
>  /* Set the satp mode to the max supported */
> @@ -1177,16 +1178,13 @@ static void riscv_cpu_disas_set_info(CPUState *s, disassemble_info *info)
>  static void riscv_cpu_satp_mode_finalize(RISCVCPU *cpu, Error **errp)
>  {
>      bool rv32 = riscv_cpu_is_32bit(cpu);
> -    uint8_t satp_mode_map_max, satp_mode_supported_max;
> +    uint8_t satp_mode_map_max;
>
>      /* The CPU wants the OS to decide which satp mode to use */
>      if (cpu->cfg.satp_mode.supported == 0) {
>          return;
>      }
>
> -    satp_mode_supported_max =
> -                    satp_mode_max_from_map(cpu->cfg.satp_mode.supported);
> -
>      if (cpu->cfg.satp_mode.map == 0) {
>          if (cpu->cfg.satp_mode.init == 0) {
>              /* If unset by the user, we fallback to the default satp mode. */
> @@ -1215,10 +1213,10 @@ static void riscv_cpu_satp_mode_finalize(RISCVCPU *cpu, Error **errp)
>      satp_mode_map_max = satp_mode_max_from_map(cpu->cfg.satp_mode.map);
>
>      /* Make sure the user asked for a supported configuration (HW and qemu) */
> -    if (satp_mode_map_max > satp_mode_supported_max) {
> +    if (satp_mode_map_max > cpu->cfg.max_satp_mode) {
>          error_setg(errp, "satp_mode %s is higher than hw max capability %s",
>                     satp_mode_str(satp_mode_map_max, rv32),
> -                   satp_mode_str(satp_mode_supported_max, rv32));
> +                   satp_mode_str(cpu->cfg.max_satp_mode, rv32));
>          return;
>      }
>
> @@ -1477,6 +1475,7 @@ static void riscv_cpu_init(Object *obj)
>      cpu->cfg.cbom_blocksize = 64;
>      cpu->cfg.cbop_blocksize = 64;
>      cpu->cfg.cboz_blocksize = 64;
> +    cpu->cfg.max_satp_mode = -1;
>      cpu->env.vext_ver = VEXT_VERSION_1_00_0;
>  }
>
> diff --git a/target/riscv/tcg/tcg-cpu.c b/target/riscv/tcg/tcg-cpu.c
> index 0a137281de1..a9f59a67e00 100644
> --- a/target/riscv/tcg/tcg-cpu.c
> +++ b/target/riscv/tcg/tcg-cpu.c
> @@ -693,8 +693,9 @@ static bool riscv_cpu_validate_profile_satp(RISCVCPU *cpu,
>                                              RISCVCPUProfile *profile,
>                                              bool send_warn)
>  {
> -    int satp_max = satp_mode_max_from_map(cpu->cfg.satp_mode.supported);
> +    int satp_max = cpu->cfg.max_satp_mode;
>
> +    assert(satp_max >= 0);
>      if (profile->satp_mode > satp_max) {
>          if (send_warn) {
>              bool is_32bit = riscv_cpu_is_32bit(cpu);
> --
> 2.48.1
>
>

Re: [PATCH 4/7] target/riscv: cpu: store max SATP mode as a single integer

Posted by Alistair Francis 11 months, 1 week ago

On Wed, Feb 19, 2025 at 3:00 AM Paolo Bonzini <pbonzini@redhat.com> wrote:
>
> The maximum available SATP mode implies all the shorter virtual address sizes.
> Store it in RISCVCPUConfig and avoid recomputing it via satp_mode_max_from_map.
>
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>

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

Alistair

> ---
>  target/riscv/cpu_cfg.h     |  1 +
>  target/riscv/cpu.c         | 11 +++++------
>  target/riscv/tcg/tcg-cpu.c |  3 ++-
>  3 files changed, 8 insertions(+), 7 deletions(-)
>
> diff --git a/target/riscv/cpu_cfg.h b/target/riscv/cpu_cfg.h
> index b410b1e6038..28d8de978fa 100644
> --- a/target/riscv/cpu_cfg.h
> +++ b/target/riscv/cpu_cfg.h
> @@ -192,6 +192,7 @@ struct RISCVCPUConfig {
>      bool short_isa_string;
>
>  #ifndef CONFIG_USER_ONLY
> +    int8_t max_satp_mode;
>      RISCVSATPMap satp_mode;
>  #endif
>  };
> diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
> index 7950b6447f8..2d06543217a 100644
> --- a/target/riscv/cpu.c
> +++ b/target/riscv/cpu.c
> @@ -444,6 +444,7 @@ static void set_satp_mode_max_supported(RISCVCPU *cpu,
>      }
>
>      assert(cpu->cfg.satp_mode.supported & (1 << satp_mode));
> +    cpu->cfg.max_satp_mode = satp_mode;
>  }
>
>  /* Set the satp mode to the max supported */
> @@ -1177,16 +1178,13 @@ static void riscv_cpu_disas_set_info(CPUState *s, disassemble_info *info)
>  static void riscv_cpu_satp_mode_finalize(RISCVCPU *cpu, Error **errp)
>  {
>      bool rv32 = riscv_cpu_is_32bit(cpu);
> -    uint8_t satp_mode_map_max, satp_mode_supported_max;
> +    uint8_t satp_mode_map_max;
>
>      /* The CPU wants the OS to decide which satp mode to use */
>      if (cpu->cfg.satp_mode.supported == 0) {
>          return;
>      }
>
> -    satp_mode_supported_max =
> -                    satp_mode_max_from_map(cpu->cfg.satp_mode.supported);
> -
>      if (cpu->cfg.satp_mode.map == 0) {
>          if (cpu->cfg.satp_mode.init == 0) {
>              /* If unset by the user, we fallback to the default satp mode. */
> @@ -1215,10 +1213,10 @@ static void riscv_cpu_satp_mode_finalize(RISCVCPU *cpu, Error **errp)
>      satp_mode_map_max = satp_mode_max_from_map(cpu->cfg.satp_mode.map);
>
>      /* Make sure the user asked for a supported configuration (HW and qemu) */
> -    if (satp_mode_map_max > satp_mode_supported_max) {
> +    if (satp_mode_map_max > cpu->cfg.max_satp_mode) {
>          error_setg(errp, "satp_mode %s is higher than hw max capability %s",
>                     satp_mode_str(satp_mode_map_max, rv32),
> -                   satp_mode_str(satp_mode_supported_max, rv32));
> +                   satp_mode_str(cpu->cfg.max_satp_mode, rv32));
>          return;
>      }
>
> @@ -1477,6 +1475,7 @@ static void riscv_cpu_init(Object *obj)
>      cpu->cfg.cbom_blocksize = 64;
>      cpu->cfg.cbop_blocksize = 64;
>      cpu->cfg.cboz_blocksize = 64;
> +    cpu->cfg.max_satp_mode = -1;
>      cpu->env.vext_ver = VEXT_VERSION_1_00_0;
>  }
>
> diff --git a/target/riscv/tcg/tcg-cpu.c b/target/riscv/tcg/tcg-cpu.c
> index 0a137281de1..a9f59a67e00 100644
> --- a/target/riscv/tcg/tcg-cpu.c
> +++ b/target/riscv/tcg/tcg-cpu.c
> @@ -693,8 +693,9 @@ static bool riscv_cpu_validate_profile_satp(RISCVCPU *cpu,
>                                              RISCVCPUProfile *profile,
>                                              bool send_warn)
>  {
> -    int satp_max = satp_mode_max_from_map(cpu->cfg.satp_mode.supported);
> +    int satp_max = cpu->cfg.max_satp_mode;
>
> +    assert(satp_max >= 0);
>      if (profile->satp_mode > satp_max) {
>          if (send_warn) {
>              bool is_32bit = riscv_cpu_is_32bit(cpu);
> --
> 2.48.1
>
>

[PATCH 1/7] hw/riscv: acpi: only create RHCT MMU entry for supported types
[PATCH 2/7] target/riscv: env->misa_mxl is a constant
[PATCH 3/7] target/riscv: assert argument to set_satp_mode_max_supported is valid
[PATCH 4/7] target/riscv: cpu: store max SATP mode as a single integer
[PATCH 5/7] target/riscv: update max_satp_mode based on QOM properties
[PATCH 6/7] target/riscv: remove supported from RISCVSATPMap
[PATCH 7/7] target/riscv: move satp_mode.{map,init} out of CPUConfig