[PATCH v3 04/16] target/riscv: move 'mmu' to riscv_cpu_properties[]

Daniel Henrique Barboza posted 16 patches 10 months, 4 weeks ago
There is a newer version of this series
[PATCH v3 04/16] target/riscv: move 'mmu' to riscv_cpu_properties[]
Posted by Daniel Henrique Barboza 10 months, 4 weeks ago
Commit 7f0bdfb5bfc ("target/riscv/cpu.c: remove cfg setup from
riscv_cpu_init()") already did some of the work by making some
cpu_init() functions to explictly enable their own 'mmu' default.

The generic CPUs didn't get update by that commit, so they are still
relying on the defaults set by the 'mmu' option. But having 'mmu' and
'pmp' being default=true will force CPUs that doesn't implement these
options to set them to 'false' in their cpu_init(), which isn't ideal.

We'll move 'mmu' to riscv_cpu_properties[] without any defaults, i.e.
the default will be 'false'. Compensate it by manually setting 'mmu =
true' to the generic CPUs that requires it.

Implement a setter for it to forbid the 'mmu' setting to be changed for
vendor CPUs. This will allow the option to exist for all CPUs and, at
the same time, protect vendor CPUs from undesired changes:

$ ./build/qemu-system-riscv64 -M virt -cpu sifive-e51,mmu=true
qemu-system-riscv64: can't apply global sifive-e51-riscv-cpu.mmu=true:
   CPU 'sifive-e51' does not allow changing the value of 'mmu'

Signed-off-by: Daniel Henrique Barboza <dbarboza@ventanamicro.com>
---
 target/riscv/cpu.c | 55 ++++++++++++++++++++++++++++++++++++++++++----
 1 file changed, 51 insertions(+), 4 deletions(-)

diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
index e90b70c0a7..9f1407b73f 100644
--- a/target/riscv/cpu.c
+++ b/target/riscv/cpu.c
@@ -419,6 +419,8 @@ static void riscv_max_cpu_init(Object *obj)
     CPURISCVState *env = &cpu->env;
     RISCVMXL mlx = MXL_RV64;
 
+    cpu->cfg.mmu = true;
+
 #ifdef TARGET_RISCV32
     mlx = MXL_RV32;
 #endif
@@ -433,7 +435,11 @@ static void riscv_max_cpu_init(Object *obj)
 #if defined(TARGET_RISCV64)
 static void rv64_base_cpu_init(Object *obj)
 {
-    CPURISCVState *env = &RISCV_CPU(obj)->env;
+    RISCVCPU *cpu = RISCV_CPU(obj);
+    CPURISCVState *env = &cpu->env;
+
+    cpu->cfg.mmu = true;
+
     /* We set this in the realise function */
     riscv_cpu_set_misa(env, MXL_RV64, 0);
     /* Set latest version of privileged specification */
@@ -551,13 +557,18 @@ static void rv64_veyron_v1_cpu_init(Object *obj)
 
 static void rv128_base_cpu_init(Object *obj)
 {
+    RISCVCPU *cpu = RISCV_CPU(obj);
+    CPURISCVState *env = &cpu->env;
+
     if (qemu_tcg_mttcg_enabled()) {
         /* Missing 128-bit aligned atomics */
         error_report("128-bit RISC-V currently does not work with Multi "
                      "Threaded TCG. Please use: -accel tcg,thread=single");
         exit(EXIT_FAILURE);
     }
-    CPURISCVState *env = &RISCV_CPU(obj)->env;
+
+    cpu->cfg.mmu = true;
+
     /* We set this in the realise function */
     riscv_cpu_set_misa(env, MXL_RV128, 0);
     /* Set latest version of privileged specification */
@@ -569,7 +580,11 @@ static void rv128_base_cpu_init(Object *obj)
 #else
 static void rv32_base_cpu_init(Object *obj)
 {
-    CPURISCVState *env = &RISCV_CPU(obj)->env;
+    RISCVCPU *cpu = RISCV_CPU(obj);
+    CPURISCVState *env = &cpu->env;
+
+    cpu->cfg.mmu = true;
+
     /* We set this in the realise function */
     riscv_cpu_set_misa(env, MXL_RV32, 0);
     /* Set latest version of privileged specification */
@@ -1550,8 +1565,38 @@ static const PropertyInfo prop_pmu_mask = {
     .set = prop_pmu_mask_set,
 };
 
+static void prop_mmu_set(Object *obj, Visitor *v, const char *name,
+                         void *opaque, Error **errp)
+{
+    RISCVCPU *cpu = RISCV_CPU(obj);
+    bool value;
+
+    visit_type_bool(v, name, &value, errp);
+
+    if (cpu->cfg.mmu != value && riscv_cpu_is_vendor(obj)) {
+        cpu_set_prop_err(cpu, "mmu", errp);
+        return;
+    }
+
+    cpu_option_add_user_setting(name, value);
+    cpu->cfg.mmu = value;
+}
+
+static void prop_mmu_get(Object *obj, Visitor *v, const char *name,
+                         void *opaque, Error **errp)
+{
+    bool value = RISCV_CPU(obj)->cfg.mmu;
+
+    visit_type_bool(v, name, &value, errp);
+}
+
+static const PropertyInfo prop_mmu = {
+    .name = "mmu",
+    .get = prop_mmu_get,
+    .set = prop_mmu_set,
+};
+
 Property riscv_cpu_options[] = {
-    DEFINE_PROP_BOOL("mmu", RISCVCPU, cfg.mmu, true),
     DEFINE_PROP_BOOL("pmp", RISCVCPU, cfg.pmp, true),
 
     DEFINE_PROP_STRING("priv_spec", RISCVCPU, cfg.priv_spec),
@@ -1572,6 +1617,8 @@ static Property riscv_cpu_properties[] = {
     {.name = "pmu-mask", .info = &prop_pmu_mask},
     {.name = "pmu-num", .info = &prop_pmu_num}, /* Deprecated */
 
+    {.name = "mmu", .info = &prop_mmu},
+
 #ifndef CONFIG_USER_ONLY
     DEFINE_PROP_UINT64("resetvec", RISCVCPU, env.resetvec, DEFAULT_RSTVEC),
 #endif
-- 
2.43.0
Re: [PATCH v3 04/16] target/riscv: move 'mmu' to riscv_cpu_properties[]
Posted by Alistair Francis 10 months, 3 weeks ago
On Thu, Jan 4, 2024 at 3:43 AM Daniel Henrique Barboza
<dbarboza@ventanamicro.com> wrote:
>
> Commit 7f0bdfb5bfc ("target/riscv/cpu.c: remove cfg setup from
> riscv_cpu_init()") already did some of the work by making some
> cpu_init() functions to explictly enable their own 'mmu' default.
>
> The generic CPUs didn't get update by that commit, so they are still
> relying on the defaults set by the 'mmu' option. But having 'mmu' and
> 'pmp' being default=true will force CPUs that doesn't implement these
> options to set them to 'false' in their cpu_init(), which isn't ideal.
>
> We'll move 'mmu' to riscv_cpu_properties[] without any defaults, i.e.
> the default will be 'false'. Compensate it by manually setting 'mmu =
> true' to the generic CPUs that requires it.
>
> Implement a setter for it to forbid the 'mmu' setting to be changed for
> vendor CPUs. This will allow the option to exist for all CPUs and, at
> the same time, protect vendor CPUs from undesired changes:
>
> $ ./build/qemu-system-riscv64 -M virt -cpu sifive-e51,mmu=true
> qemu-system-riscv64: can't apply global sifive-e51-riscv-cpu.mmu=true:
>    CPU 'sifive-e51' does not allow changing the value of 'mmu'
>
> Signed-off-by: Daniel Henrique Barboza <dbarboza@ventanamicro.com>

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

Alistair

> ---
>  target/riscv/cpu.c | 55 ++++++++++++++++++++++++++++++++++++++++++----
>  1 file changed, 51 insertions(+), 4 deletions(-)
>
> diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
> index e90b70c0a7..9f1407b73f 100644
> --- a/target/riscv/cpu.c
> +++ b/target/riscv/cpu.c
> @@ -419,6 +419,8 @@ static void riscv_max_cpu_init(Object *obj)
>      CPURISCVState *env = &cpu->env;
>      RISCVMXL mlx = MXL_RV64;
>
> +    cpu->cfg.mmu = true;
> +
>  #ifdef TARGET_RISCV32
>      mlx = MXL_RV32;
>  #endif
> @@ -433,7 +435,11 @@ static void riscv_max_cpu_init(Object *obj)
>  #if defined(TARGET_RISCV64)
>  static void rv64_base_cpu_init(Object *obj)
>  {
> -    CPURISCVState *env = &RISCV_CPU(obj)->env;
> +    RISCVCPU *cpu = RISCV_CPU(obj);
> +    CPURISCVState *env = &cpu->env;
> +
> +    cpu->cfg.mmu = true;
> +
>      /* We set this in the realise function */
>      riscv_cpu_set_misa(env, MXL_RV64, 0);
>      /* Set latest version of privileged specification */
> @@ -551,13 +557,18 @@ static void rv64_veyron_v1_cpu_init(Object *obj)
>
>  static void rv128_base_cpu_init(Object *obj)
>  {
> +    RISCVCPU *cpu = RISCV_CPU(obj);
> +    CPURISCVState *env = &cpu->env;
> +
>      if (qemu_tcg_mttcg_enabled()) {
>          /* Missing 128-bit aligned atomics */
>          error_report("128-bit RISC-V currently does not work with Multi "
>                       "Threaded TCG. Please use: -accel tcg,thread=single");
>          exit(EXIT_FAILURE);
>      }
> -    CPURISCVState *env = &RISCV_CPU(obj)->env;
> +
> +    cpu->cfg.mmu = true;
> +
>      /* We set this in the realise function */
>      riscv_cpu_set_misa(env, MXL_RV128, 0);
>      /* Set latest version of privileged specification */
> @@ -569,7 +580,11 @@ static void rv128_base_cpu_init(Object *obj)
>  #else
>  static void rv32_base_cpu_init(Object *obj)
>  {
> -    CPURISCVState *env = &RISCV_CPU(obj)->env;
> +    RISCVCPU *cpu = RISCV_CPU(obj);
> +    CPURISCVState *env = &cpu->env;
> +
> +    cpu->cfg.mmu = true;
> +
>      /* We set this in the realise function */
>      riscv_cpu_set_misa(env, MXL_RV32, 0);
>      /* Set latest version of privileged specification */
> @@ -1550,8 +1565,38 @@ static const PropertyInfo prop_pmu_mask = {
>      .set = prop_pmu_mask_set,
>  };
>
> +static void prop_mmu_set(Object *obj, Visitor *v, const char *name,
> +                         void *opaque, Error **errp)
> +{
> +    RISCVCPU *cpu = RISCV_CPU(obj);
> +    bool value;
> +
> +    visit_type_bool(v, name, &value, errp);
> +
> +    if (cpu->cfg.mmu != value && riscv_cpu_is_vendor(obj)) {
> +        cpu_set_prop_err(cpu, "mmu", errp);
> +        return;
> +    }
> +
> +    cpu_option_add_user_setting(name, value);
> +    cpu->cfg.mmu = value;
> +}
> +
> +static void prop_mmu_get(Object *obj, Visitor *v, const char *name,
> +                         void *opaque, Error **errp)
> +{
> +    bool value = RISCV_CPU(obj)->cfg.mmu;
> +
> +    visit_type_bool(v, name, &value, errp);
> +}
> +
> +static const PropertyInfo prop_mmu = {
> +    .name = "mmu",
> +    .get = prop_mmu_get,
> +    .set = prop_mmu_set,
> +};
> +
>  Property riscv_cpu_options[] = {
> -    DEFINE_PROP_BOOL("mmu", RISCVCPU, cfg.mmu, true),
>      DEFINE_PROP_BOOL("pmp", RISCVCPU, cfg.pmp, true),
>
>      DEFINE_PROP_STRING("priv_spec", RISCVCPU, cfg.priv_spec),
> @@ -1572,6 +1617,8 @@ static Property riscv_cpu_properties[] = {
>      {.name = "pmu-mask", .info = &prop_pmu_mask},
>      {.name = "pmu-num", .info = &prop_pmu_num}, /* Deprecated */
>
> +    {.name = "mmu", .info = &prop_mmu},
> +
>  #ifndef CONFIG_USER_ONLY
>      DEFINE_PROP_UINT64("resetvec", RISCVCPU, env.resetvec, DEFAULT_RSTVEC),
>  #endif
> --
> 2.43.0
>
>