diff mbox series

[2/2] target/riscv/cpu.c: do not skip misa logic in riscv_cpu_realize()

Message ID 20230110201405.247785-3-dbarboza@ventanamicro.com
State New
Headers show
Series target/riscv/cpu: fix sifive_u 32/64bits boot in riscv-to-apply.next | expand

Commit Message

Daniel Henrique Barboza Jan. 10, 2023, 8:14 p.m. UTC
All RISCV CPUs are setting cpu->cfg during their cpu_init() functions,
meaning that there's no reason to skip all the misa validation and setup
if misa_ext was set beforehand - especially since we're setting an
updated value in set_misa() in the end.

Put this code chunk into a new riscv_cpu_validate_set_extensions()
helper and always execute it regardless of what the board set in
env->misa_ext.

This will put more responsibility in how each board is going to init
their attributes and extensions if they're not using the defaults.
It'll also allow realize() to do its job looking only at the extensions
enabled per se, not corner cases that some CPUs might have, and we won't
have to change multiple code paths to fix or change how extensions work.

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

Comments

Alistair Francis Jan. 10, 2023, 10:53 p.m. UTC | #1
On Wed, Jan 11, 2023 at 6:17 AM Daniel Henrique Barboza
<dbarboza@ventanamicro.com> wrote:
>
> All RISCV CPUs are setting cpu->cfg during their cpu_init() functions,
> meaning that there's no reason to skip all the misa validation and setup
> if misa_ext was set beforehand - especially since we're setting an
> updated value in set_misa() in the end.
>
> Put this code chunk into a new riscv_cpu_validate_set_extensions()
> helper and always execute it regardless of what the board set in
> env->misa_ext.
>
> This will put more responsibility in how each board is going to init
> their attributes and extensions if they're not using the defaults.
> It'll also allow realize() to do its job looking only at the extensions
> enabled per se, not corner cases that some CPUs might have, and we won't
> have to change multiple code paths to fix or change how extensions work.
>
> Signed-off-by: Daniel Henrique Barboza <dbarboza@ventanamicro.com>

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

Alistair

> ---
>  target/riscv/cpu.c | 485 +++++++++++++++++++++++----------------------
>  1 file changed, 248 insertions(+), 237 deletions(-)
>
> diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
> index b8c1edb7c2..33ed59a1b6 100644
> --- a/target/riscv/cpu.c
> +++ b/target/riscv/cpu.c
> @@ -631,6 +631,250 @@ static void riscv_cpu_disas_set_info(CPUState *s, disassemble_info *info)
>      }
>  }
>
> +/*
> + * Check consistency between chosen extensions while setting
> + * cpu->cfg accordingly, doing a set_misa() in the end.
> + */
> +static void riscv_cpu_validate_set_extensions(RISCVCPU *cpu, Error **errp)
> +{
> +    CPURISCVState *env = &cpu->env;
> +    uint32_t ext = 0;
> +
> +    /* Do some ISA extension error checking */
> +    if (cpu->cfg.ext_g && !(cpu->cfg.ext_i && cpu->cfg.ext_m &&
> +                            cpu->cfg.ext_a && cpu->cfg.ext_f &&
> +                            cpu->cfg.ext_d &&
> +                            cpu->cfg.ext_icsr && cpu->cfg.ext_ifencei)) {
> +        warn_report("Setting G will also set IMAFD_Zicsr_Zifencei");
> +        cpu->cfg.ext_i = true;
> +        cpu->cfg.ext_m = true;
> +        cpu->cfg.ext_a = true;
> +        cpu->cfg.ext_f = true;
> +        cpu->cfg.ext_d = true;
> +        cpu->cfg.ext_icsr = true;
> +        cpu->cfg.ext_ifencei = true;
> +    }
> +
> +    if (cpu->cfg.ext_i && cpu->cfg.ext_e) {
> +        error_setg(errp,
> +                   "I and E extensions are incompatible");
> +        return;
> +    }
> +
> +    if (!cpu->cfg.ext_i && !cpu->cfg.ext_e) {
> +        error_setg(errp,
> +                   "Either I or E extension must be set");
> +        return;
> +    }
> +
> +    if (cpu->cfg.ext_s && !cpu->cfg.ext_u) {
> +        error_setg(errp,
> +                   "Setting S extension without U extension is illegal");
> +        return;
> +    }
> +
> +    if (cpu->cfg.ext_h && !cpu->cfg.ext_i) {
> +        error_setg(errp,
> +                   "H depends on an I base integer ISA with 32 x registers");
> +        return;
> +    }
> +
> +    if (cpu->cfg.ext_h && !cpu->cfg.ext_s) {
> +        error_setg(errp, "H extension implicitly requires S-mode");
> +        return;
> +    }
> +
> +    if (cpu->cfg.ext_f && !cpu->cfg.ext_icsr) {
> +        error_setg(errp, "F extension requires Zicsr");
> +        return;
> +    }
> +
> +    if ((cpu->cfg.ext_zawrs) && !cpu->cfg.ext_a) {
> +        error_setg(errp, "Zawrs extension requires A extension");
> +        return;
> +    }
> +
> +    if ((cpu->cfg.ext_zfh || cpu->cfg.ext_zfhmin) && !cpu->cfg.ext_f) {
> +        error_setg(errp, "Zfh/Zfhmin extensions require F extension");
> +        return;
> +    }
> +
> +    if (cpu->cfg.ext_d && !cpu->cfg.ext_f) {
> +        error_setg(errp, "D extension requires F extension");
> +        return;
> +    }
> +
> +    if (cpu->cfg.ext_v && !cpu->cfg.ext_d) {
> +        error_setg(errp, "V extension requires D extension");
> +        return;
> +    }
> +
> +    if ((cpu->cfg.ext_zve32f || cpu->cfg.ext_zve64f) && !cpu->cfg.ext_f) {
> +        error_setg(errp, "Zve32f/Zve64f extensions require F extension");
> +        return;
> +    }
> +
> +    /* Set the ISA extensions, checks should have happened above */
> +    if (cpu->cfg.ext_zdinx || cpu->cfg.ext_zhinx ||
> +        cpu->cfg.ext_zhinxmin) {
> +        cpu->cfg.ext_zfinx = true;
> +    }
> +
> +    if (cpu->cfg.ext_zfinx) {
> +        if (!cpu->cfg.ext_icsr) {
> +            error_setg(errp, "Zfinx extension requires Zicsr");
> +            return;
> +        }
> +        if (cpu->cfg.ext_f) {
> +            error_setg(errp,
> +                "Zfinx cannot be supported together with F extension");
> +            return;
> +        }
> +    }
> +
> +    if (cpu->cfg.ext_c) {
> +        cpu->cfg.ext_zca = true;
> +        if (cpu->cfg.ext_f && env->misa_mxl_max == MXL_RV32) {
> +            cpu->cfg.ext_zcf = true;
> +        }
> +        if (cpu->cfg.ext_d) {
> +            cpu->cfg.ext_zcd = true;
> +        }
> +    }
> +
> +    if (env->misa_mxl_max != MXL_RV32 && cpu->cfg.ext_zcf) {
> +        error_setg(errp, "Zcf extension is only relevant to RV32");
> +        return;
> +    }
> +
> +    if (!cpu->cfg.ext_f && cpu->cfg.ext_zcf) {
> +        error_setg(errp, "Zcf extension requires F extension");
> +        return;
> +    }
> +
> +    if (!cpu->cfg.ext_d && cpu->cfg.ext_zcd) {
> +        error_setg(errp, "Zcd extension requires D extension");
> +        return;
> +    }
> +
> +    if ((cpu->cfg.ext_zcf || cpu->cfg.ext_zcd || cpu->cfg.ext_zcb ||
> +         cpu->cfg.ext_zcmp || cpu->cfg.ext_zcmt) && !cpu->cfg.ext_zca) {
> +        error_setg(errp, "Zcf/Zcd/Zcb/Zcmp/Zcmt extensions require Zca "
> +                         "extension");
> +        return;
> +    }
> +
> +    if (cpu->cfg.ext_zcd && (cpu->cfg.ext_zcmp || cpu->cfg.ext_zcmt)) {
> +        error_setg(errp, "Zcmp/Zcmt extensions are incompatible with "
> +                         "Zcd extension");
> +        return;
> +    }
> +
> +    if (cpu->cfg.ext_zcmt && !cpu->cfg.ext_icsr) {
> +        error_setg(errp, "Zcmt extension requires Zicsr extension");
> +        return;
> +    }
> +
> +    if (cpu->cfg.ext_zk) {
> +        cpu->cfg.ext_zkn = true;
> +        cpu->cfg.ext_zkr = true;
> +        cpu->cfg.ext_zkt = true;
> +    }
> +
> +    if (cpu->cfg.ext_zkn) {
> +        cpu->cfg.ext_zbkb = true;
> +        cpu->cfg.ext_zbkc = true;
> +        cpu->cfg.ext_zbkx = true;
> +        cpu->cfg.ext_zkne = true;
> +        cpu->cfg.ext_zknd = true;
> +        cpu->cfg.ext_zknh = true;
> +    }
> +
> +    if (cpu->cfg.ext_zks) {
> +        cpu->cfg.ext_zbkb = true;
> +        cpu->cfg.ext_zbkc = true;
> +        cpu->cfg.ext_zbkx = true;
> +        cpu->cfg.ext_zksed = true;
> +        cpu->cfg.ext_zksh = true;
> +    }
> +
> +    if (cpu->cfg.ext_i) {
> +        ext |= RVI;
> +    }
> +    if (cpu->cfg.ext_e) {
> +        ext |= RVE;
> +    }
> +    if (cpu->cfg.ext_m) {
> +        ext |= RVM;
> +    }
> +    if (cpu->cfg.ext_a) {
> +        ext |= RVA;
> +    }
> +    if (cpu->cfg.ext_f) {
> +        ext |= RVF;
> +    }
> +    if (cpu->cfg.ext_d) {
> +        ext |= RVD;
> +    }
> +    if (cpu->cfg.ext_c) {
> +        ext |= RVC;
> +    }
> +    if (cpu->cfg.ext_s) {
> +        ext |= RVS;
> +    }
> +    if (cpu->cfg.ext_u) {
> +        ext |= RVU;
> +    }
> +    if (cpu->cfg.ext_h) {
> +        ext |= RVH;
> +    }
> +    if (cpu->cfg.ext_v) {
> +        int vext_version = VEXT_VERSION_1_00_0;
> +        ext |= RVV;
> +        if (!is_power_of_2(cpu->cfg.vlen)) {
> +            error_setg(errp,
> +                    "Vector extension VLEN must be power of 2");
> +            return;
> +        }
> +        if (cpu->cfg.vlen > RV_VLEN_MAX || cpu->cfg.vlen < 128) {
> +            error_setg(errp,
> +                    "Vector extension implementation only supports VLEN "
> +                    "in the range [128, %d]", RV_VLEN_MAX);
> +            return;
> +        }
> +        if (!is_power_of_2(cpu->cfg.elen)) {
> +            error_setg(errp,
> +                    "Vector extension ELEN must be power of 2");
> +            return;
> +        }
> +    if (cpu->cfg.elen > 64 || cpu->cfg.elen < 8) {
> +        error_setg(errp,
> +                "Vector extension implementation only supports ELEN "
> +                "in the range [8, 64]");
> +        return;
> +    }
> +    if (cpu->cfg.vext_spec) {
> +        if (!g_strcmp0(cpu->cfg.vext_spec, "v1.0")) {
> +            vext_version = VEXT_VERSION_1_00_0;
> +        } else {
> +            error_setg(errp,
> +                   "Unsupported vector spec version '%s'",
> +                   cpu->cfg.vext_spec);
> +            return;
> +        }
> +    } else {
> +        qemu_log("vector version is not specified, "
> +                 "use the default value v1.0\n");
> +    }
> +    set_vext_version(env, vext_version);
> +    }
> +    if (cpu->cfg.ext_j) {
> +        ext |= RVJ;
> +    }
> +
> +    set_misa(env, env->misa_mxl, ext);
> +}
> +
>  static void riscv_cpu_realize(DeviceState *dev, Error **errp)
>  {
>      CPUState *cs = CPU(dev);
> @@ -726,243 +970,10 @@ static void riscv_cpu_realize(DeviceState *dev, Error **errp)
>      }
>      assert(env->misa_mxl_max == env->misa_mxl);
>
> -    /* If only MISA_EXT is unset for misa, then set it from properties */
> -    if (env->misa_ext == 0) {
> -        uint32_t ext = 0;
> -
> -        /* Do some ISA extension error checking */
> -        if (cpu->cfg.ext_g && !(cpu->cfg.ext_i && cpu->cfg.ext_m &&
> -                                cpu->cfg.ext_a && cpu->cfg.ext_f &&
> -                                cpu->cfg.ext_d &&
> -                                cpu->cfg.ext_icsr && cpu->cfg.ext_ifencei)) {
> -            warn_report("Setting G will also set IMAFD_Zicsr_Zifencei");
> -            cpu->cfg.ext_i = true;
> -            cpu->cfg.ext_m = true;
> -            cpu->cfg.ext_a = true;
> -            cpu->cfg.ext_f = true;
> -            cpu->cfg.ext_d = true;
> -            cpu->cfg.ext_icsr = true;
> -            cpu->cfg.ext_ifencei = true;
> -        }
> -
> -        if (cpu->cfg.ext_i && cpu->cfg.ext_e) {
> -            error_setg(errp,
> -                       "I and E extensions are incompatible");
> -            return;
> -        }
> -
> -        if (!cpu->cfg.ext_i && !cpu->cfg.ext_e) {
> -            error_setg(errp,
> -                       "Either I or E extension must be set");
> -            return;
> -        }
> -
> -        if (cpu->cfg.ext_s && !cpu->cfg.ext_u) {
> -            error_setg(errp,
> -                       "Setting S extension without U extension is illegal");
> -            return;
> -        }
> -
> -        if (cpu->cfg.ext_h && !cpu->cfg.ext_i) {
> -            error_setg(errp,
> -                       "H depends on an I base integer ISA with 32 x registers");
> -            return;
> -        }
> -
> -        if (cpu->cfg.ext_h && !cpu->cfg.ext_s) {
> -            error_setg(errp, "H extension implicitly requires S-mode");
> -            return;
> -        }
> -
> -        if (cpu->cfg.ext_f && !cpu->cfg.ext_icsr) {
> -            error_setg(errp, "F extension requires Zicsr");
> -            return;
> -        }
> -
> -        if ((cpu->cfg.ext_zawrs) && !cpu->cfg.ext_a) {
> -            error_setg(errp, "Zawrs extension requires A extension");
> -            return;
> -        }
> -
> -        if ((cpu->cfg.ext_zfh || cpu->cfg.ext_zfhmin) && !cpu->cfg.ext_f) {
> -            error_setg(errp, "Zfh/Zfhmin extensions require F extension");
> -            return;
> -        }
> -
> -        if (cpu->cfg.ext_d && !cpu->cfg.ext_f) {
> -            error_setg(errp, "D extension requires F extension");
> -            return;
> -        }
> -
> -        if (cpu->cfg.ext_v && !cpu->cfg.ext_d) {
> -            error_setg(errp, "V extension requires D extension");
> -            return;
> -        }
> -
> -        if ((cpu->cfg.ext_zve32f || cpu->cfg.ext_zve64f) && !cpu->cfg.ext_f) {
> -            error_setg(errp, "Zve32f/Zve64f extensions require F extension");
> -            return;
> -        }
> -
> -        /* Set the ISA extensions, checks should have happened above */
> -        if (cpu->cfg.ext_zdinx || cpu->cfg.ext_zhinx ||
> -            cpu->cfg.ext_zhinxmin) {
> -            cpu->cfg.ext_zfinx = true;
> -        }
> -
> -        if (cpu->cfg.ext_zfinx) {
> -            if (!cpu->cfg.ext_icsr) {
> -                error_setg(errp, "Zfinx extension requires Zicsr");
> -                return;
> -            }
> -            if (cpu->cfg.ext_f) {
> -                error_setg(errp,
> -                    "Zfinx cannot be supported together with F extension");
> -                return;
> -            }
> -        }
> -
> -        if (cpu->cfg.ext_c) {
> -            cpu->cfg.ext_zca = true;
> -            if (cpu->cfg.ext_f && env->misa_mxl_max == MXL_RV32) {
> -                cpu->cfg.ext_zcf = true;
> -            }
> -            if (cpu->cfg.ext_d) {
> -                cpu->cfg.ext_zcd = true;
> -            }
> -        }
> -
> -        if (env->misa_mxl_max != MXL_RV32 && cpu->cfg.ext_zcf) {
> -            error_setg(errp, "Zcf extension is only relevant to RV32");
> -            return;
> -        }
> -
> -        if (!cpu->cfg.ext_f && cpu->cfg.ext_zcf) {
> -            error_setg(errp, "Zcf extension requires F extension");
> -            return;
> -        }
> -
> -        if (!cpu->cfg.ext_d && cpu->cfg.ext_zcd) {
> -            error_setg(errp, "Zcd extension requires D extension");
> -            return;
> -        }
> -
> -        if ((cpu->cfg.ext_zcf || cpu->cfg.ext_zcd || cpu->cfg.ext_zcb ||
> -             cpu->cfg.ext_zcmp || cpu->cfg.ext_zcmt) && !cpu->cfg.ext_zca) {
> -            error_setg(errp, "Zcf/Zcd/Zcb/Zcmp/Zcmt extensions require Zca "
> -                             "extension");
> -            return;
> -        }
> -
> -        if (cpu->cfg.ext_zcd && (cpu->cfg.ext_zcmp || cpu->cfg.ext_zcmt)) {
> -            error_setg(errp, "Zcmp/Zcmt extensions are incompatible with "
> -                             "Zcd extension");
> -            return;
> -        }
> -
> -        if (cpu->cfg.ext_zcmt && !cpu->cfg.ext_icsr) {
> -            error_setg(errp, "Zcmt extension requires Zicsr extension");
> -            return;
> -        }
> -
> -        if (cpu->cfg.ext_zk) {
> -            cpu->cfg.ext_zkn = true;
> -            cpu->cfg.ext_zkr = true;
> -            cpu->cfg.ext_zkt = true;
> -        }
> -
> -        if (cpu->cfg.ext_zkn) {
> -            cpu->cfg.ext_zbkb = true;
> -            cpu->cfg.ext_zbkc = true;
> -            cpu->cfg.ext_zbkx = true;
> -            cpu->cfg.ext_zkne = true;
> -            cpu->cfg.ext_zknd = true;
> -            cpu->cfg.ext_zknh = true;
> -        }
> -
> -        if (cpu->cfg.ext_zks) {
> -            cpu->cfg.ext_zbkb = true;
> -            cpu->cfg.ext_zbkc = true;
> -            cpu->cfg.ext_zbkx = true;
> -            cpu->cfg.ext_zksed = true;
> -            cpu->cfg.ext_zksh = true;
> -        }
> -
> -        if (cpu->cfg.ext_i) {
> -            ext |= RVI;
> -        }
> -        if (cpu->cfg.ext_e) {
> -            ext |= RVE;
> -        }
> -        if (cpu->cfg.ext_m) {
> -            ext |= RVM;
> -        }
> -        if (cpu->cfg.ext_a) {
> -            ext |= RVA;
> -        }
> -        if (cpu->cfg.ext_f) {
> -            ext |= RVF;
> -        }
> -        if (cpu->cfg.ext_d) {
> -            ext |= RVD;
> -        }
> -        if (cpu->cfg.ext_c) {
> -            ext |= RVC;
> -        }
> -        if (cpu->cfg.ext_s) {
> -            ext |= RVS;
> -        }
> -        if (cpu->cfg.ext_u) {
> -            ext |= RVU;
> -        }
> -        if (cpu->cfg.ext_h) {
> -            ext |= RVH;
> -        }
> -        if (cpu->cfg.ext_v) {
> -            int vext_version = VEXT_VERSION_1_00_0;
> -            ext |= RVV;
> -            if (!is_power_of_2(cpu->cfg.vlen)) {
> -                error_setg(errp,
> -                        "Vector extension VLEN must be power of 2");
> -                return;
> -            }
> -            if (cpu->cfg.vlen > RV_VLEN_MAX || cpu->cfg.vlen < 128) {
> -                error_setg(errp,
> -                        "Vector extension implementation only supports VLEN "
> -                        "in the range [128, %d]", RV_VLEN_MAX);
> -                return;
> -            }
> -            if (!is_power_of_2(cpu->cfg.elen)) {
> -                error_setg(errp,
> -                        "Vector extension ELEN must be power of 2");
> -                return;
> -            }
> -            if (cpu->cfg.elen > 64 || cpu->cfg.elen < 8) {
> -                error_setg(errp,
> -                        "Vector extension implementation only supports ELEN "
> -                        "in the range [8, 64]");
> -                return;
> -            }
> -            if (cpu->cfg.vext_spec) {
> -                if (!g_strcmp0(cpu->cfg.vext_spec, "v1.0")) {
> -                    vext_version = VEXT_VERSION_1_00_0;
> -                } else {
> -                    error_setg(errp,
> -                           "Unsupported vector spec version '%s'",
> -                           cpu->cfg.vext_spec);
> -                    return;
> -                }
> -            } else {
> -                qemu_log("vector version is not specified, "
> -                         "use the default value v1.0\n");
> -            }
> -            set_vext_version(env, vext_version);
> -        }
> -        if (cpu->cfg.ext_j) {
> -            ext |= RVJ;
> -        }
> -
> -        set_misa(env, env->misa_mxl, ext);
> +    riscv_cpu_validate_set_extensions(cpu, &local_err);
> +    if (local_err != NULL) {
> +        error_propagate(errp, local_err);
> +        return;
>      }
>
>  #ifndef CONFIG_USER_ONLY
> --
> 2.39.0
>
>
Bin Meng Jan. 11, 2023, 5:56 a.m. UTC | #2
On Wed, Jan 11, 2023 at 4:17 AM Daniel Henrique Barboza
<dbarboza@ventanamicro.com> wrote:
>
> All RISCV CPUs are setting cpu->cfg during their cpu_init() functions,
> meaning that there's no reason to skip all the misa validation and setup
> if misa_ext was set beforehand - especially since we're setting an
> updated value in set_misa() in the end.
>
> Put this code chunk into a new riscv_cpu_validate_set_extensions()
> helper and always execute it regardless of what the board set in
> env->misa_ext.
>
> This will put more responsibility in how each board is going to init
> their attributes and extensions if they're not using the defaults.
> It'll also allow realize() to do its job looking only at the extensions
> enabled per se, not corner cases that some CPUs might have, and we won't
> have to change multiple code paths to fix or change how extensions work.
>
> Signed-off-by: Daniel Henrique Barboza <dbarboza@ventanamicro.com>
> ---
>  target/riscv/cpu.c | 485 +++++++++++++++++++++++----------------------
>  1 file changed, 248 insertions(+), 237 deletions(-)
>

Reviewed-by: Bin Meng <bmeng@tinylab.org>
diff mbox series

Patch

diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
index b8c1edb7c2..33ed59a1b6 100644
--- a/target/riscv/cpu.c
+++ b/target/riscv/cpu.c
@@ -631,6 +631,250 @@  static void riscv_cpu_disas_set_info(CPUState *s, disassemble_info *info)
     }
 }
 
+/*
+ * Check consistency between chosen extensions while setting
+ * cpu->cfg accordingly, doing a set_misa() in the end.
+ */
+static void riscv_cpu_validate_set_extensions(RISCVCPU *cpu, Error **errp)
+{
+    CPURISCVState *env = &cpu->env;
+    uint32_t ext = 0;
+
+    /* Do some ISA extension error checking */
+    if (cpu->cfg.ext_g && !(cpu->cfg.ext_i && cpu->cfg.ext_m &&
+                            cpu->cfg.ext_a && cpu->cfg.ext_f &&
+                            cpu->cfg.ext_d &&
+                            cpu->cfg.ext_icsr && cpu->cfg.ext_ifencei)) {
+        warn_report("Setting G will also set IMAFD_Zicsr_Zifencei");
+        cpu->cfg.ext_i = true;
+        cpu->cfg.ext_m = true;
+        cpu->cfg.ext_a = true;
+        cpu->cfg.ext_f = true;
+        cpu->cfg.ext_d = true;
+        cpu->cfg.ext_icsr = true;
+        cpu->cfg.ext_ifencei = true;
+    }
+
+    if (cpu->cfg.ext_i && cpu->cfg.ext_e) {
+        error_setg(errp,
+                   "I and E extensions are incompatible");
+        return;
+    }
+
+    if (!cpu->cfg.ext_i && !cpu->cfg.ext_e) {
+        error_setg(errp,
+                   "Either I or E extension must be set");
+        return;
+    }
+
+    if (cpu->cfg.ext_s && !cpu->cfg.ext_u) {
+        error_setg(errp,
+                   "Setting S extension without U extension is illegal");
+        return;
+    }
+
+    if (cpu->cfg.ext_h && !cpu->cfg.ext_i) {
+        error_setg(errp,
+                   "H depends on an I base integer ISA with 32 x registers");
+        return;
+    }
+
+    if (cpu->cfg.ext_h && !cpu->cfg.ext_s) {
+        error_setg(errp, "H extension implicitly requires S-mode");
+        return;
+    }
+
+    if (cpu->cfg.ext_f && !cpu->cfg.ext_icsr) {
+        error_setg(errp, "F extension requires Zicsr");
+        return;
+    }
+
+    if ((cpu->cfg.ext_zawrs) && !cpu->cfg.ext_a) {
+        error_setg(errp, "Zawrs extension requires A extension");
+        return;
+    }
+
+    if ((cpu->cfg.ext_zfh || cpu->cfg.ext_zfhmin) && !cpu->cfg.ext_f) {
+        error_setg(errp, "Zfh/Zfhmin extensions require F extension");
+        return;
+    }
+
+    if (cpu->cfg.ext_d && !cpu->cfg.ext_f) {
+        error_setg(errp, "D extension requires F extension");
+        return;
+    }
+
+    if (cpu->cfg.ext_v && !cpu->cfg.ext_d) {
+        error_setg(errp, "V extension requires D extension");
+        return;
+    }
+
+    if ((cpu->cfg.ext_zve32f || cpu->cfg.ext_zve64f) && !cpu->cfg.ext_f) {
+        error_setg(errp, "Zve32f/Zve64f extensions require F extension");
+        return;
+    }
+
+    /* Set the ISA extensions, checks should have happened above */
+    if (cpu->cfg.ext_zdinx || cpu->cfg.ext_zhinx ||
+        cpu->cfg.ext_zhinxmin) {
+        cpu->cfg.ext_zfinx = true;
+    }
+
+    if (cpu->cfg.ext_zfinx) {
+        if (!cpu->cfg.ext_icsr) {
+            error_setg(errp, "Zfinx extension requires Zicsr");
+            return;
+        }
+        if (cpu->cfg.ext_f) {
+            error_setg(errp,
+                "Zfinx cannot be supported together with F extension");
+            return;
+        }
+    }
+
+    if (cpu->cfg.ext_c) {
+        cpu->cfg.ext_zca = true;
+        if (cpu->cfg.ext_f && env->misa_mxl_max == MXL_RV32) {
+            cpu->cfg.ext_zcf = true;
+        }
+        if (cpu->cfg.ext_d) {
+            cpu->cfg.ext_zcd = true;
+        }
+    }
+
+    if (env->misa_mxl_max != MXL_RV32 && cpu->cfg.ext_zcf) {
+        error_setg(errp, "Zcf extension is only relevant to RV32");
+        return;
+    }
+
+    if (!cpu->cfg.ext_f && cpu->cfg.ext_zcf) {
+        error_setg(errp, "Zcf extension requires F extension");
+        return;
+    }
+
+    if (!cpu->cfg.ext_d && cpu->cfg.ext_zcd) {
+        error_setg(errp, "Zcd extension requires D extension");
+        return;
+    }
+
+    if ((cpu->cfg.ext_zcf || cpu->cfg.ext_zcd || cpu->cfg.ext_zcb ||
+         cpu->cfg.ext_zcmp || cpu->cfg.ext_zcmt) && !cpu->cfg.ext_zca) {
+        error_setg(errp, "Zcf/Zcd/Zcb/Zcmp/Zcmt extensions require Zca "
+                         "extension");
+        return;
+    }
+
+    if (cpu->cfg.ext_zcd && (cpu->cfg.ext_zcmp || cpu->cfg.ext_zcmt)) {
+        error_setg(errp, "Zcmp/Zcmt extensions are incompatible with "
+                         "Zcd extension");
+        return;
+    }
+
+    if (cpu->cfg.ext_zcmt && !cpu->cfg.ext_icsr) {
+        error_setg(errp, "Zcmt extension requires Zicsr extension");
+        return;
+    }
+
+    if (cpu->cfg.ext_zk) {
+        cpu->cfg.ext_zkn = true;
+        cpu->cfg.ext_zkr = true;
+        cpu->cfg.ext_zkt = true;
+    }
+
+    if (cpu->cfg.ext_zkn) {
+        cpu->cfg.ext_zbkb = true;
+        cpu->cfg.ext_zbkc = true;
+        cpu->cfg.ext_zbkx = true;
+        cpu->cfg.ext_zkne = true;
+        cpu->cfg.ext_zknd = true;
+        cpu->cfg.ext_zknh = true;
+    }
+
+    if (cpu->cfg.ext_zks) {
+        cpu->cfg.ext_zbkb = true;
+        cpu->cfg.ext_zbkc = true;
+        cpu->cfg.ext_zbkx = true;
+        cpu->cfg.ext_zksed = true;
+        cpu->cfg.ext_zksh = true;
+    }
+
+    if (cpu->cfg.ext_i) {
+        ext |= RVI;
+    }
+    if (cpu->cfg.ext_e) {
+        ext |= RVE;
+    }
+    if (cpu->cfg.ext_m) {
+        ext |= RVM;
+    }
+    if (cpu->cfg.ext_a) {
+        ext |= RVA;
+    }
+    if (cpu->cfg.ext_f) {
+        ext |= RVF;
+    }
+    if (cpu->cfg.ext_d) {
+        ext |= RVD;
+    }
+    if (cpu->cfg.ext_c) {
+        ext |= RVC;
+    }
+    if (cpu->cfg.ext_s) {
+        ext |= RVS;
+    }
+    if (cpu->cfg.ext_u) {
+        ext |= RVU;
+    }
+    if (cpu->cfg.ext_h) {
+        ext |= RVH;
+    }
+    if (cpu->cfg.ext_v) {
+        int vext_version = VEXT_VERSION_1_00_0;
+        ext |= RVV;
+        if (!is_power_of_2(cpu->cfg.vlen)) {
+            error_setg(errp,
+                    "Vector extension VLEN must be power of 2");
+            return;
+        }
+        if (cpu->cfg.vlen > RV_VLEN_MAX || cpu->cfg.vlen < 128) {
+            error_setg(errp,
+                    "Vector extension implementation only supports VLEN "
+                    "in the range [128, %d]", RV_VLEN_MAX);
+            return;
+        }
+        if (!is_power_of_2(cpu->cfg.elen)) {
+            error_setg(errp,
+                    "Vector extension ELEN must be power of 2");
+            return;
+        }
+    if (cpu->cfg.elen > 64 || cpu->cfg.elen < 8) {
+        error_setg(errp,
+                "Vector extension implementation only supports ELEN "
+                "in the range [8, 64]");
+        return;
+    }
+    if (cpu->cfg.vext_spec) {
+        if (!g_strcmp0(cpu->cfg.vext_spec, "v1.0")) {
+            vext_version = VEXT_VERSION_1_00_0;
+        } else {
+            error_setg(errp,
+                   "Unsupported vector spec version '%s'",
+                   cpu->cfg.vext_spec);
+            return;
+        }
+    } else {
+        qemu_log("vector version is not specified, "
+                 "use the default value v1.0\n");
+    }
+    set_vext_version(env, vext_version);
+    }
+    if (cpu->cfg.ext_j) {
+        ext |= RVJ;
+    }
+
+    set_misa(env, env->misa_mxl, ext);
+}
+
 static void riscv_cpu_realize(DeviceState *dev, Error **errp)
 {
     CPUState *cs = CPU(dev);
@@ -726,243 +970,10 @@  static void riscv_cpu_realize(DeviceState *dev, Error **errp)
     }
     assert(env->misa_mxl_max == env->misa_mxl);
 
-    /* If only MISA_EXT is unset for misa, then set it from properties */
-    if (env->misa_ext == 0) {
-        uint32_t ext = 0;
-
-        /* Do some ISA extension error checking */
-        if (cpu->cfg.ext_g && !(cpu->cfg.ext_i && cpu->cfg.ext_m &&
-                                cpu->cfg.ext_a && cpu->cfg.ext_f &&
-                                cpu->cfg.ext_d &&
-                                cpu->cfg.ext_icsr && cpu->cfg.ext_ifencei)) {
-            warn_report("Setting G will also set IMAFD_Zicsr_Zifencei");
-            cpu->cfg.ext_i = true;
-            cpu->cfg.ext_m = true;
-            cpu->cfg.ext_a = true;
-            cpu->cfg.ext_f = true;
-            cpu->cfg.ext_d = true;
-            cpu->cfg.ext_icsr = true;
-            cpu->cfg.ext_ifencei = true;
-        }
-
-        if (cpu->cfg.ext_i && cpu->cfg.ext_e) {
-            error_setg(errp,
-                       "I and E extensions are incompatible");
-            return;
-        }
-
-        if (!cpu->cfg.ext_i && !cpu->cfg.ext_e) {
-            error_setg(errp,
-                       "Either I or E extension must be set");
-            return;
-        }
-
-        if (cpu->cfg.ext_s && !cpu->cfg.ext_u) {
-            error_setg(errp,
-                       "Setting S extension without U extension is illegal");
-            return;
-        }
-
-        if (cpu->cfg.ext_h && !cpu->cfg.ext_i) {
-            error_setg(errp,
-                       "H depends on an I base integer ISA with 32 x registers");
-            return;
-        }
-
-        if (cpu->cfg.ext_h && !cpu->cfg.ext_s) {
-            error_setg(errp, "H extension implicitly requires S-mode");
-            return;
-        }
-
-        if (cpu->cfg.ext_f && !cpu->cfg.ext_icsr) {
-            error_setg(errp, "F extension requires Zicsr");
-            return;
-        }
-
-        if ((cpu->cfg.ext_zawrs) && !cpu->cfg.ext_a) {
-            error_setg(errp, "Zawrs extension requires A extension");
-            return;
-        }
-
-        if ((cpu->cfg.ext_zfh || cpu->cfg.ext_zfhmin) && !cpu->cfg.ext_f) {
-            error_setg(errp, "Zfh/Zfhmin extensions require F extension");
-            return;
-        }
-
-        if (cpu->cfg.ext_d && !cpu->cfg.ext_f) {
-            error_setg(errp, "D extension requires F extension");
-            return;
-        }
-
-        if (cpu->cfg.ext_v && !cpu->cfg.ext_d) {
-            error_setg(errp, "V extension requires D extension");
-            return;
-        }
-
-        if ((cpu->cfg.ext_zve32f || cpu->cfg.ext_zve64f) && !cpu->cfg.ext_f) {
-            error_setg(errp, "Zve32f/Zve64f extensions require F extension");
-            return;
-        }
-
-        /* Set the ISA extensions, checks should have happened above */
-        if (cpu->cfg.ext_zdinx || cpu->cfg.ext_zhinx ||
-            cpu->cfg.ext_zhinxmin) {
-            cpu->cfg.ext_zfinx = true;
-        }
-
-        if (cpu->cfg.ext_zfinx) {
-            if (!cpu->cfg.ext_icsr) {
-                error_setg(errp, "Zfinx extension requires Zicsr");
-                return;
-            }
-            if (cpu->cfg.ext_f) {
-                error_setg(errp,
-                    "Zfinx cannot be supported together with F extension");
-                return;
-            }
-        }
-
-        if (cpu->cfg.ext_c) {
-            cpu->cfg.ext_zca = true;
-            if (cpu->cfg.ext_f && env->misa_mxl_max == MXL_RV32) {
-                cpu->cfg.ext_zcf = true;
-            }
-            if (cpu->cfg.ext_d) {
-                cpu->cfg.ext_zcd = true;
-            }
-        }
-
-        if (env->misa_mxl_max != MXL_RV32 && cpu->cfg.ext_zcf) {
-            error_setg(errp, "Zcf extension is only relevant to RV32");
-            return;
-        }
-
-        if (!cpu->cfg.ext_f && cpu->cfg.ext_zcf) {
-            error_setg(errp, "Zcf extension requires F extension");
-            return;
-        }
-
-        if (!cpu->cfg.ext_d && cpu->cfg.ext_zcd) {
-            error_setg(errp, "Zcd extension requires D extension");
-            return;
-        }
-
-        if ((cpu->cfg.ext_zcf || cpu->cfg.ext_zcd || cpu->cfg.ext_zcb ||
-             cpu->cfg.ext_zcmp || cpu->cfg.ext_zcmt) && !cpu->cfg.ext_zca) {
-            error_setg(errp, "Zcf/Zcd/Zcb/Zcmp/Zcmt extensions require Zca "
-                             "extension");
-            return;
-        }
-
-        if (cpu->cfg.ext_zcd && (cpu->cfg.ext_zcmp || cpu->cfg.ext_zcmt)) {
-            error_setg(errp, "Zcmp/Zcmt extensions are incompatible with "
-                             "Zcd extension");
-            return;
-        }
-
-        if (cpu->cfg.ext_zcmt && !cpu->cfg.ext_icsr) {
-            error_setg(errp, "Zcmt extension requires Zicsr extension");
-            return;
-        }
-
-        if (cpu->cfg.ext_zk) {
-            cpu->cfg.ext_zkn = true;
-            cpu->cfg.ext_zkr = true;
-            cpu->cfg.ext_zkt = true;
-        }
-
-        if (cpu->cfg.ext_zkn) {
-            cpu->cfg.ext_zbkb = true;
-            cpu->cfg.ext_zbkc = true;
-            cpu->cfg.ext_zbkx = true;
-            cpu->cfg.ext_zkne = true;
-            cpu->cfg.ext_zknd = true;
-            cpu->cfg.ext_zknh = true;
-        }
-
-        if (cpu->cfg.ext_zks) {
-            cpu->cfg.ext_zbkb = true;
-            cpu->cfg.ext_zbkc = true;
-            cpu->cfg.ext_zbkx = true;
-            cpu->cfg.ext_zksed = true;
-            cpu->cfg.ext_zksh = true;
-        }
-
-        if (cpu->cfg.ext_i) {
-            ext |= RVI;
-        }
-        if (cpu->cfg.ext_e) {
-            ext |= RVE;
-        }
-        if (cpu->cfg.ext_m) {
-            ext |= RVM;
-        }
-        if (cpu->cfg.ext_a) {
-            ext |= RVA;
-        }
-        if (cpu->cfg.ext_f) {
-            ext |= RVF;
-        }
-        if (cpu->cfg.ext_d) {
-            ext |= RVD;
-        }
-        if (cpu->cfg.ext_c) {
-            ext |= RVC;
-        }
-        if (cpu->cfg.ext_s) {
-            ext |= RVS;
-        }
-        if (cpu->cfg.ext_u) {
-            ext |= RVU;
-        }
-        if (cpu->cfg.ext_h) {
-            ext |= RVH;
-        }
-        if (cpu->cfg.ext_v) {
-            int vext_version = VEXT_VERSION_1_00_0;
-            ext |= RVV;
-            if (!is_power_of_2(cpu->cfg.vlen)) {
-                error_setg(errp,
-                        "Vector extension VLEN must be power of 2");
-                return;
-            }
-            if (cpu->cfg.vlen > RV_VLEN_MAX || cpu->cfg.vlen < 128) {
-                error_setg(errp,
-                        "Vector extension implementation only supports VLEN "
-                        "in the range [128, %d]", RV_VLEN_MAX);
-                return;
-            }
-            if (!is_power_of_2(cpu->cfg.elen)) {
-                error_setg(errp,
-                        "Vector extension ELEN must be power of 2");
-                return;
-            }
-            if (cpu->cfg.elen > 64 || cpu->cfg.elen < 8) {
-                error_setg(errp,
-                        "Vector extension implementation only supports ELEN "
-                        "in the range [8, 64]");
-                return;
-            }
-            if (cpu->cfg.vext_spec) {
-                if (!g_strcmp0(cpu->cfg.vext_spec, "v1.0")) {
-                    vext_version = VEXT_VERSION_1_00_0;
-                } else {
-                    error_setg(errp,
-                           "Unsupported vector spec version '%s'",
-                           cpu->cfg.vext_spec);
-                    return;
-                }
-            } else {
-                qemu_log("vector version is not specified, "
-                         "use the default value v1.0\n");
-            }
-            set_vext_version(env, vext_version);
-        }
-        if (cpu->cfg.ext_j) {
-            ext |= RVJ;
-        }
-
-        set_misa(env, env->misa_mxl, ext);
+    riscv_cpu_validate_set_extensions(cpu, &local_err);
+    if (local_err != NULL) {
+        error_propagate(errp, local_err);
+        return;
     }
 
 #ifndef CONFIG_USER_ONLY