From 1237c2d6942709cf82b999b6f6e8624b86ac495f Mon Sep 17 00:00:00 2001 From: Bin Meng Date: Thu, 29 Dec 2022 17:18:21 +0800 Subject: hw/char: riscv_htif: Move registers from CPUArchState to HTIFState At present for some unknown reason the HTIF registers (fromhost & tohost) are defined in the RISC-V CPUArchState. It should really be put in the HTIFState struct as it is only meaningful to HTIF. Signed-off-by: Bin Meng Reviewed-by: Daniel Henrique Barboza Reviewed-by: Alistair Francis Message-Id: <20221229091828.1945072-6-bmeng@tinylab.org> Signed-off-by: Alistair Francis --- target/riscv/cpu.h | 4 ---- target/riscv/machine.c | 6 ++---- 2 files changed, 2 insertions(+), 8 deletions(-) (limited to 'target') diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h index f5609b62a2..61a9a40958 100644 --- a/target/riscv/cpu.h +++ b/target/riscv/cpu.h @@ -309,10 +309,6 @@ struct CPUArchState { target_ulong sscratch; target_ulong mscratch; - /* temporary htif regs */ - uint64_t mfromhost; - uint64_t mtohost; - /* Sstc CSRs */ uint64_t stimecmp; diff --git a/target/riscv/machine.c b/target/riscv/machine.c index 65a8549ec2..c6ce318cce 100644 --- a/target/riscv/machine.c +++ b/target/riscv/machine.c @@ -333,8 +333,8 @@ static const VMStateDescription vmstate_pmu_ctr_state = { const VMStateDescription vmstate_riscv_cpu = { .name = "cpu", - .version_id = 5, - .minimum_version_id = 5, + .version_id = 6, + .minimum_version_id = 6, .post_load = riscv_cpu_post_load, .fields = (VMStateField[]) { VMSTATE_UINTTL_ARRAY(env.gpr, RISCVCPU, 32), @@ -384,8 +384,6 @@ const VMStateDescription vmstate_riscv_cpu = { VMSTATE_UINTTL_ARRAY(env.mhpmeventh_val, RISCVCPU, RV_MAX_MHPMEVENTS), VMSTATE_UINTTL(env.sscratch, RISCVCPU), VMSTATE_UINTTL(env.mscratch, RISCVCPU), - VMSTATE_UINT64(env.mfromhost, RISCVCPU), - VMSTATE_UINT64(env.mtohost, RISCVCPU), VMSTATE_UINT64(env.stimecmp, RISCVCPU), VMSTATE_END_OF_LIST() -- cgit v1.2.3 From 44e7372b213bad4e4589d765f011b25c897c8ab1 Mon Sep 17 00:00:00 2001 From: Dongxue Zhang Date: Thu, 15 Dec 2022 16:27:14 +0800 Subject: target/riscv/cpu.c: Fix elen check The elen check should be cpu->cfg.elen in range [8, 64]. Signed-off-by: Dongxue Zhang Reviewed-by: LIU Zhiwei Reviewed-by: Frank Chang Reviewed-by: Alistair Francis Message-Id: <167236721596.15277.2653405273227256289-0@git.sr.ht> [ Changes by AF: - Tidy up commit message ] Signed-off-by: Alistair Francis --- target/riscv/cpu.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'target') diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c index cc75ca7667..a2e6238bd7 100644 --- a/target/riscv/cpu.c +++ b/target/riscv/cpu.c @@ -882,7 +882,7 @@ static void riscv_cpu_realize(DeviceState *dev, Error **errp) "Vector extension ELEN must be power of 2"); return; } - if (cpu->cfg.elen > 64 || cpu->cfg.vlen < 8) { + if (cpu->cfg.elen > 64 || cpu->cfg.elen < 8) { error_setg(errp, "Vector extension implementation only supports ELEN " "in the range [8, 64]"); -- cgit v1.2.3 From c66ffcd5358ba88e93e1ffb15ae42ca52dab12a8 Mon Sep 17 00:00:00 2001 From: Daniel Henrique Barboza Date: Fri, 13 Jan 2023 14:52:29 -0300 Subject: target/riscv/cpu: set cpu->cfg in register_cpu_props() There is an informal contract between the cpu_init() functions and riscv_cpu_realize(): if cpu->env.misa_ext is zero, assume that the default settings were loaded via register_cpu_props() and do validations to set env.misa_ext. If it's not zero, skip this whole process and assume that the board somehow did everything. At this moment, all SiFive CPUs are setting a non-zero misa_ext during their cpu_init() and skipping a good chunk of riscv_cpu_realize(). This causes problems when the code being skipped in riscv_cpu_realize() contains fixes or assumptions that affects all CPUs, meaning that SiFive CPUs are missing out. To allow this code to not be skipped anymore, all the cpu->cfg.ext_* attributes needs to be set during cpu_init() time. At this moment this is being done in register_cpu_props(). The SiFive boards are setting their own extensions during cpu_init() though, meaning that they don't want all the defaults from register_cpu_props(). Let's move the contract between *_cpu_init() and riscv_cpu_realize() to register_cpu_props(). Inside this function we'll check if cpu->env.misa_ext was set and, if that's the case, set all relevant cpu->cfg.ext_* attributes, and only that. Leave the 'misa_ext' = 0 case as is today, i.e. loading all the defaults from riscv_cpu_extensions[]. register_cpu_props() can then be called by all the cpu_init() functions, including the SiFive ones. This will make all CPUs behave more in line with what riscv_cpu_realize() expects. This will also make the cpu_init() functions even more alike, but at this moment we would need some design changes in how we're initializing extensions/attributes (e.g. some CPUs are setting cfg options after register_cpu_props(), so we can't simply add the function to a common post_init() hook) to make a common cpu_init() code across all CPUs. Signed-off-by: Daniel Henrique Barboza Reviewed-by: Alistair Francis Message-Id: <20230113175230.473975-2-dbarboza@ventanamicro.com> Signed-off-by: Alistair Francis --- target/riscv/cpu.c | 40 ++++++++++++++++++++++++++++++++++++++++ target/riscv/cpu.h | 4 ++++ 2 files changed, 44 insertions(+) (limited to 'target') diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c index a2e6238bd7..e682102c2a 100644 --- a/target/riscv/cpu.c +++ b/target/riscv/cpu.c @@ -256,6 +256,7 @@ static void rv64_sifive_u_cpu_init(Object *obj) { CPURISCVState *env = &RISCV_CPU(obj)->env; set_misa(env, MXL_RV64, RVI | RVM | RVA | RVF | RVD | RVC | RVS | RVU); + register_cpu_props(DEVICE(obj)); set_priv_version(env, PRIV_VERSION_1_10_0); } @@ -265,6 +266,7 @@ static void rv64_sifive_e_cpu_init(Object *obj) RISCVCPU *cpu = RISCV_CPU(obj); set_misa(env, MXL_RV64, RVI | RVM | RVA | RVC | RVU); + register_cpu_props(DEVICE(obj)); set_priv_version(env, PRIV_VERSION_1_10_0); cpu->cfg.mmu = false; } @@ -299,6 +301,7 @@ static void rv32_sifive_u_cpu_init(Object *obj) { CPURISCVState *env = &RISCV_CPU(obj)->env; set_misa(env, MXL_RV32, RVI | RVM | RVA | RVF | RVD | RVC | RVS | RVU); + register_cpu_props(DEVICE(obj)); set_priv_version(env, PRIV_VERSION_1_10_0); } @@ -308,6 +311,7 @@ static void rv32_sifive_e_cpu_init(Object *obj) RISCVCPU *cpu = RISCV_CPU(obj); set_misa(env, MXL_RV32, RVI | RVM | RVA | RVC | RVU); + register_cpu_props(DEVICE(obj)); set_priv_version(env, PRIV_VERSION_1_10_0); cpu->cfg.mmu = false; } @@ -318,6 +322,7 @@ static void rv32_ibex_cpu_init(Object *obj) RISCVCPU *cpu = RISCV_CPU(obj); set_misa(env, MXL_RV32, RVI | RVM | RVC | RVU); + register_cpu_props(DEVICE(obj)); set_priv_version(env, PRIV_VERSION_1_11_0); cpu->cfg.mmu = false; cpu->cfg.epmp = true; @@ -329,6 +334,7 @@ static void rv32_imafcu_nommu_cpu_init(Object *obj) RISCVCPU *cpu = RISCV_CPU(obj); set_misa(env, MXL_RV32, RVI | RVM | RVA | RVF | RVC | RVU); + register_cpu_props(DEVICE(obj)); set_priv_version(env, PRIV_VERSION_1_10_0); cpu->cfg.mmu = false; } @@ -1083,10 +1089,44 @@ static Property riscv_cpu_extensions[] = { DEFINE_PROP_END_OF_LIST(), }; +/* + * Register CPU props based on env.misa_ext. If a non-zero + * value was set, register only the required cpu->cfg.ext_* + * properties and leave. env.misa_ext = 0 means that we want + * all the default properties to be registered. + */ static void register_cpu_props(DeviceState *dev) { + RISCVCPU *cpu = RISCV_CPU(OBJECT(dev)); + uint32_t misa_ext = cpu->env.misa_ext; Property *prop; + /* + * If misa_ext is not zero, set cfg properties now to + * allow them to be read during riscv_cpu_realize() + * later on. + */ + if (cpu->env.misa_ext != 0) { + cpu->cfg.ext_i = misa_ext & RVI; + cpu->cfg.ext_e = misa_ext & RVE; + cpu->cfg.ext_m = misa_ext & RVM; + cpu->cfg.ext_a = misa_ext & RVA; + cpu->cfg.ext_f = misa_ext & RVF; + cpu->cfg.ext_d = misa_ext & RVD; + cpu->cfg.ext_v = misa_ext & RVV; + cpu->cfg.ext_c = misa_ext & RVC; + cpu->cfg.ext_s = misa_ext & RVS; + cpu->cfg.ext_u = misa_ext & RVU; + cpu->cfg.ext_h = misa_ext & RVH; + cpu->cfg.ext_j = misa_ext & RVJ; + + /* + * We don't want to set the default riscv_cpu_extensions + * in this case. + */ + return; + } + for (prop = riscv_cpu_extensions; prop && prop->name; prop++) { qdev_property_add_static(dev, prop); } diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h index 61a9a40958..bcf0826753 100644 --- a/target/riscv/cpu.h +++ b/target/riscv/cpu.h @@ -63,6 +63,10 @@ #define RV(x) ((target_ulong)1 << (x - 'A')) +/* + * Consider updating register_cpu_props() when adding + * new MISA bits here. + */ #define RVI RV('I') #define RVE RV('E') /* E and I are mutually exclusive */ #define RVM RV('M') -- cgit v1.2.3 From 5ab1095213318effd9bb4667f7f52da21f81acc6 Mon Sep 17 00:00:00 2001 From: Daniel Henrique Barboza Date: Fri, 13 Jan 2023 14:52:30 -0300 Subject: target/riscv/cpu.c: do not skip misa logic in riscv_cpu_realize() 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 Reviewed-by: Alistair Francis Reviewed-by: Bin Meng Message-Id: <20230113175230.473975-3-dbarboza@ventanamicro.com> [ Changes by AF: - Rebase ] Signed-off-by: Alistair Francis --- target/riscv/cpu.c | 399 +++++++++++++++++++++++++++-------------------------- 1 file changed, 205 insertions(+), 194 deletions(-) (limited to 'target') diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c index e682102c2a..c192d96a94 100644 --- a/target/riscv/cpu.c +++ b/target/riscv/cpu.c @@ -625,6 +625,207 @@ 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_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); @@ -720,200 +921,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_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 -- cgit v1.2.3 From 877a3a3732dcd45b09b96a6ff9655f6a2e19540f Mon Sep 17 00:00:00 2001 From: Bin Meng Date: Mon, 9 Jan 2023 23:26:55 +0800 Subject: target/riscv: Use TARGET_FMT_lx for env->mhartid env->mhartid is currently casted to long before printed, which drops the high 32-bit for rv64 on 32-bit host. Use TARGET_FMT_lx instead. Signed-off-by: Bin Meng Reviewed-by: Alistair Francis Message-Id: <20230109152655.340114-1-bmeng@tinylab.org> Signed-off-by: Alistair Francis --- target/riscv/cpu.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) (limited to 'target') diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c index c192d96a94..14a7027095 100644 --- a/target/riscv/cpu.c +++ b/target/riscv/cpu.c @@ -867,9 +867,9 @@ static void riscv_cpu_realize(DeviceState *dev, Error **errp) (env->priv_ver < isa_edata_arr[i].min_version)) { isa_ext_update_enabled(cpu, &isa_edata_arr[i], false); #ifndef CONFIG_USER_ONLY - warn_report("disabling %s extension for hart 0x%lx because " - "privilege spec version does not match", - isa_edata_arr[i].name, (unsigned long)env->mhartid); + warn_report("disabling %s extension for hart 0x" TARGET_FMT_lx + " because privilege spec version does not match", + isa_edata_arr[i].name, env->mhartid); #else warn_report("disabling %s extension because " "privilege spec version does not match", -- cgit v1.2.3 From 06d85c24c28f42a57680dc21955e343f58d93089 Mon Sep 17 00:00:00 2001 From: Andrew Bresticker Date: Thu, 15 Dec 2022 17:45:40 -0500 Subject: target/riscv: Fix up masking of vsip/vsie accesses The current logic attempts to shift the VS-level bits into their correct position in mip while leaving the remaining bits in-tact. This is both pointless and likely incorrect since one would expect that any new, future VS-level interrupts will get their own position in mip rather than sharing with their (H)S-level equivalent. Fix this, and make the logic more readable, by just making off the VS-level bits and shifting them into position. This also fixes reads of vsip, which would only ever report vsip.VSSIP since the non-writable bits got masked off as well. Fixes: d028ac7512f1 ("arget/riscv: Implement AIA CSRs for 64 local interrupts on RV32") Signed-off-by: Andrew Bresticker Reviewed-by: Alistair Francis Message-Id: <20221215224541.1423431-1-abrestic@rivosinc.com> Signed-off-by: Alistair Francis --- target/riscv/csr.c | 35 +++++++++++------------------------ 1 file changed, 11 insertions(+), 24 deletions(-) (limited to 'target') diff --git a/target/riscv/csr.c b/target/riscv/csr.c index 0db2c233e5..270de7b1a8 100644 --- a/target/riscv/csr.c +++ b/target/riscv/csr.c @@ -2305,22 +2305,15 @@ static RISCVException rmw_vsie64(CPURISCVState *env, int csrno, uint64_t new_val, uint64_t wr_mask) { RISCVException ret; - uint64_t rval, vsbits, mask = env->hideleg & VS_MODE_INTERRUPTS; + uint64_t rval, mask = env->hideleg & VS_MODE_INTERRUPTS; /* Bring VS-level bits to correct position */ - vsbits = new_val & (VS_MODE_INTERRUPTS >> 1); - new_val &= ~(VS_MODE_INTERRUPTS >> 1); - new_val |= vsbits << 1; - vsbits = wr_mask & (VS_MODE_INTERRUPTS >> 1); - wr_mask &= ~(VS_MODE_INTERRUPTS >> 1); - wr_mask |= vsbits << 1; + new_val = (new_val & (VS_MODE_INTERRUPTS >> 1)) << 1; + wr_mask = (wr_mask & (VS_MODE_INTERRUPTS >> 1)) << 1; ret = rmw_mie64(env, csrno, &rval, new_val, wr_mask & mask); if (ret_val) { - rval &= mask; - vsbits = rval & VS_MODE_INTERRUPTS; - rval &= ~VS_MODE_INTERRUPTS; - *ret_val = rval | (vsbits >> 1); + *ret_val = (rval & mask) >> 1; } return ret; @@ -2521,22 +2514,16 @@ static RISCVException rmw_vsip64(CPURISCVState *env, int csrno, uint64_t new_val, uint64_t wr_mask) { RISCVException ret; - uint64_t rval, vsbits, mask = env->hideleg & vsip_writable_mask; + uint64_t rval, mask = env->hideleg & VS_MODE_INTERRUPTS; /* Bring VS-level bits to correct position */ - vsbits = new_val & (VS_MODE_INTERRUPTS >> 1); - new_val &= ~(VS_MODE_INTERRUPTS >> 1); - new_val |= vsbits << 1; - vsbits = wr_mask & (VS_MODE_INTERRUPTS >> 1); - wr_mask &= ~(VS_MODE_INTERRUPTS >> 1); - wr_mask |= vsbits << 1; - - ret = rmw_mip64(env, csrno, &rval, new_val, wr_mask & mask); + new_val = (new_val & (VS_MODE_INTERRUPTS >> 1)) << 1; + wr_mask = (wr_mask & (VS_MODE_INTERRUPTS >> 1)) << 1; + + ret = rmw_mip64(env, csrno, &rval, new_val, + wr_mask & mask & vsip_writable_mask); if (ret_val) { - rval &= mask; - vsbits = rval & VS_MODE_INTERRUPTS; - rval &= ~VS_MODE_INTERRUPTS; - *ret_val = rval | (vsbits >> 1); + *ret_val = (rval & mask) >> 1; } return ret; -- cgit v1.2.3 From e471a8c9850f1af0c1bc5768ca28285348cdd6c5 Mon Sep 17 00:00:00 2001 From: Andrew Bresticker Date: Thu, 15 Dec 2022 17:45:41 -0500 Subject: target/riscv: Trap on writes to stimecmp from VS when hvictl.VTI=1 Per the AIA specification, writes to stimecmp from VS level should trap when hvictl.VTI is set since the write may cause vsip.STIP to become unset. Fixes: 3ec0fe18a31f ("target/riscv: Add vstimecmp support") Signed-off-by: Andrew Bresticker Reviewed-by: Alistair Francis Message-Id: <20221215224541.1423431-2-abrestic@rivosinc.com> Signed-off-by: Alistair Francis --- target/riscv/csr.c | 6 ++++++ 1 file changed, 6 insertions(+) (limited to 'target') diff --git a/target/riscv/csr.c b/target/riscv/csr.c index 270de7b1a8..62e6c4acbd 100644 --- a/target/riscv/csr.c +++ b/target/riscv/csr.c @@ -1037,6 +1037,9 @@ static RISCVException write_stimecmp(CPURISCVState *env, int csrno, RISCVCPU *cpu = env_archcpu(env); if (riscv_cpu_virt_enabled(env)) { + if (env->hvictl & HVICTL_VTI) { + return RISCV_EXCP_VIRT_INSTRUCTION_FAULT; + } return write_vstimecmp(env, csrno, val); } @@ -1057,6 +1060,9 @@ static RISCVException write_stimecmph(CPURISCVState *env, int csrno, RISCVCPU *cpu = env_archcpu(env); if (riscv_cpu_virt_enabled(env)) { + if (env->hvictl & HVICTL_VTI) { + return RISCV_EXCP_VIRT_INSTRUCTION_FAULT; + } return write_vstimecmph(env, csrno, val); } -- cgit v1.2.3 From 3ceeb19a53e51c0c6310d760d26dca08145797c5 Mon Sep 17 00:00:00 2001 From: Richard Henderson Date: Sun, 15 Jan 2023 06:06:56 -1000 Subject: target/riscv: Introduce helper_set_rounding_mode_chkfrm The new helper always validates the contents of FRM, even if the new rounding mode is not DYN. This is required by the vector unit. Track whether we've validated FRM separately from whether we've updated fp_status with a given rounding mode, so that we can elide calls correctly. This partially reverts d6c4d3f2a69 which attempted the to do the same thing, but with two calls to gen_set_rm(), which is both inefficient and tickles an assertion in decode_save_opc. Resolves: https://gitlab.com/qemu-project/qemu/-/issues/1441 Signed-off-by: Richard Henderson Reviewed-by: Daniel Henrique Barboza Acked-by: Alistair Francis Message-Id: <20230115160657.3169274-2-richard.henderson@linaro.org> Signed-off-by: Alistair Francis --- target/riscv/fpu_helper.c | 37 +++++++++++++++++++++++++++++++++ target/riscv/helper.h | 1 + target/riscv/insn_trans/trans_rvv.c.inc | 24 ++++----------------- target/riscv/translate.c | 19 +++++++++++++++++ 4 files changed, 61 insertions(+), 20 deletions(-) (limited to 'target') diff --git a/target/riscv/fpu_helper.c b/target/riscv/fpu_helper.c index 5699c9517f..96817df8ef 100644 --- a/target/riscv/fpu_helper.c +++ b/target/riscv/fpu_helper.c @@ -81,6 +81,43 @@ void helper_set_rounding_mode(CPURISCVState *env, uint32_t rm) set_float_rounding_mode(softrm, &env->fp_status); } +void helper_set_rounding_mode_chkfrm(CPURISCVState *env, uint32_t rm) +{ + int softrm; + + /* Always validate frm, even if rm != DYN. */ + if (unlikely(env->frm >= 5)) { + riscv_raise_exception(env, RISCV_EXCP_ILLEGAL_INST, GETPC()); + } + if (rm == RISCV_FRM_DYN) { + rm = env->frm; + } + switch (rm) { + case RISCV_FRM_RNE: + softrm = float_round_nearest_even; + break; + case RISCV_FRM_RTZ: + softrm = float_round_to_zero; + break; + case RISCV_FRM_RDN: + softrm = float_round_down; + break; + case RISCV_FRM_RUP: + softrm = float_round_up; + break; + case RISCV_FRM_RMM: + softrm = float_round_ties_away; + break; + case RISCV_FRM_ROD: + softrm = float_round_to_odd; + break; + default: + g_assert_not_reached(); + } + + set_float_rounding_mode(softrm, &env->fp_status); +} + void helper_set_rod_rounding_mode(CPURISCVState *env) { set_float_rounding_mode(float_round_to_odd, &env->fp_status); diff --git a/target/riscv/helper.h b/target/riscv/helper.h index 227c7122ef..9792ab5086 100644 --- a/target/riscv/helper.h +++ b/target/riscv/helper.h @@ -3,6 +3,7 @@ DEF_HELPER_2(raise_exception, noreturn, env, i32) /* Floating Point - rounding mode */ DEF_HELPER_FLAGS_2(set_rounding_mode, TCG_CALL_NO_WG, void, env, i32) +DEF_HELPER_FLAGS_2(set_rounding_mode_chkfrm, TCG_CALL_NO_WG, void, env, i32) DEF_HELPER_FLAGS_1(set_rod_rounding_mode, TCG_CALL_NO_WG, void, env) /* Floating Point - fused */ diff --git a/target/riscv/insn_trans/trans_rvv.c.inc b/target/riscv/insn_trans/trans_rvv.c.inc index d455acedbf..bbb5c3a7b5 100644 --- a/target/riscv/insn_trans/trans_rvv.c.inc +++ b/target/riscv/insn_trans/trans_rvv.c.inc @@ -2679,13 +2679,9 @@ static bool do_opfv(DisasContext *s, arg_rmr *a, int rm) { if (checkfn(s, a)) { - if (rm != RISCV_FRM_DYN) { - gen_set_rm(s, RISCV_FRM_DYN); - } - uint32_t data = 0; TCGLabel *over = gen_new_label(); - gen_set_rm(s, rm); + gen_set_rm_chkfrm(s, rm); tcg_gen_brcondi_tl(TCG_COND_EQ, cpu_vl, 0, over); tcg_gen_brcond_tl(TCG_COND_GEU, cpu_vstart, cpu_vl, over); @@ -2882,17 +2878,13 @@ static bool opffv_widen_check(DisasContext *s, arg_rmr *a) static bool trans_##NAME(DisasContext *s, arg_rmr *a) \ { \ if (CHECK(s, a)) { \ - if (FRM != RISCV_FRM_DYN) { \ - gen_set_rm(s, RISCV_FRM_DYN); \ - } \ - \ uint32_t data = 0; \ static gen_helper_gvec_3_ptr * const fns[2] = { \ gen_helper_##HELPER##_h, \ gen_helper_##HELPER##_w, \ }; \ TCGLabel *over = gen_new_label(); \ - gen_set_rm(s, FRM); \ + gen_set_rm_chkfrm(s, FRM); \ tcg_gen_brcondi_tl(TCG_COND_EQ, cpu_vl, 0, over); \ tcg_gen_brcond_tl(TCG_COND_GEU, cpu_vstart, cpu_vl, over); \ \ @@ -3005,17 +2997,13 @@ static bool opffv_narrow_check(DisasContext *s, arg_rmr *a) static bool trans_##NAME(DisasContext *s, arg_rmr *a) \ { \ if (CHECK(s, a)) { \ - if (FRM != RISCV_FRM_DYN) { \ - gen_set_rm(s, RISCV_FRM_DYN); \ - } \ - \ uint32_t data = 0; \ static gen_helper_gvec_3_ptr * const fns[2] = { \ gen_helper_##HELPER##_h, \ gen_helper_##HELPER##_w, \ }; \ TCGLabel *over = gen_new_label(); \ - gen_set_rm(s, FRM); \ + gen_set_rm_chkfrm(s, FRM); \ tcg_gen_brcondi_tl(TCG_COND_EQ, cpu_vl, 0, over); \ tcg_gen_brcond_tl(TCG_COND_GEU, cpu_vstart, cpu_vl, over); \ \ @@ -3060,10 +3048,6 @@ static bool opxfv_narrow_check(DisasContext *s, arg_rmr *a) static bool trans_##NAME(DisasContext *s, arg_rmr *a) \ { \ if (opxfv_narrow_check(s, a)) { \ - if (FRM != RISCV_FRM_DYN) { \ - gen_set_rm(s, RISCV_FRM_DYN); \ - } \ - \ uint32_t data = 0; \ static gen_helper_gvec_3_ptr * const fns[3] = { \ gen_helper_##HELPER##_b, \ @@ -3071,7 +3055,7 @@ static bool trans_##NAME(DisasContext *s, arg_rmr *a) \ gen_helper_##HELPER##_w, \ }; \ TCGLabel *over = gen_new_label(); \ - gen_set_rm(s, FRM); \ + gen_set_rm_chkfrm(s, FRM); \ tcg_gen_brcondi_tl(TCG_COND_EQ, cpu_vl, 0, over); \ tcg_gen_brcond_tl(TCG_COND_GEU, cpu_vstart, cpu_vl, over); \ \ diff --git a/target/riscv/translate.c b/target/riscv/translate.c index df38db7553..493c3815e1 100644 --- a/target/riscv/translate.c +++ b/target/riscv/translate.c @@ -114,6 +114,8 @@ typedef struct DisasContext { bool pm_base_enabled; /* Use icount trigger for native debug */ bool itrigger; + /* FRM is known to contain a valid value. */ + bool frm_valid; /* TCG of the current insn_start */ TCGOp *insn_start; } DisasContext; @@ -674,12 +676,29 @@ static void gen_set_rm(DisasContext *ctx, int rm) gen_helper_set_rod_rounding_mode(cpu_env); return; } + if (rm == RISCV_FRM_DYN) { + /* The helper will return only if frm valid. */ + ctx->frm_valid = true; + } /* The helper may raise ILLEGAL_INSN -- record binv for unwind. */ decode_save_opc(ctx); gen_helper_set_rounding_mode(cpu_env, tcg_constant_i32(rm)); } +static void gen_set_rm_chkfrm(DisasContext *ctx, int rm) +{ + if (ctx->frm == rm && ctx->frm_valid) { + return; + } + ctx->frm = rm; + ctx->frm_valid = true; + + /* The helper may raise ILLEGAL_INSN -- record binv for unwind. */ + decode_save_opc(ctx); + gen_helper_set_rounding_mode_chkfrm(cpu_env, tcg_constant_i32(rm)); +} + static int ex_plus_1(DisasContext *ctx, int nf) { return nf + 1; -- cgit v1.2.3 From f251c01a623e0c998a2127f8648d4d02cd04e702 Mon Sep 17 00:00:00 2001 From: Richard Henderson Date: Sun, 15 Jan 2023 06:06:57 -1000 Subject: target/riscv: Remove helper_set_rod_rounding_mode The only setting of RISCV_FRM_ROD is from the vector unit, and now handled by helper_set_rounding_mode_chkfrm. This helper is now unused. Signed-off-by: Richard Henderson Reviewed-by: Daniel Henrique Barboza Reviewed-by: Alistair Francis Message-Id: <20230115160657.3169274-3-richard.henderson@linaro.org> Signed-off-by: Alistair Francis --- target/riscv/fpu_helper.c | 5 ----- target/riscv/helper.h | 1 - target/riscv/translate.c | 4 ---- 3 files changed, 10 deletions(-) (limited to 'target') diff --git a/target/riscv/fpu_helper.c b/target/riscv/fpu_helper.c index 96817df8ef..449d236df6 100644 --- a/target/riscv/fpu_helper.c +++ b/target/riscv/fpu_helper.c @@ -118,11 +118,6 @@ void helper_set_rounding_mode_chkfrm(CPURISCVState *env, uint32_t rm) set_float_rounding_mode(softrm, &env->fp_status); } -void helper_set_rod_rounding_mode(CPURISCVState *env) -{ - set_float_rounding_mode(float_round_to_odd, &env->fp_status); -} - static uint64_t do_fmadd_h(CPURISCVState *env, uint64_t rs1, uint64_t rs2, uint64_t rs3, int flags) { diff --git a/target/riscv/helper.h b/target/riscv/helper.h index 9792ab5086..58a30f03d6 100644 --- a/target/riscv/helper.h +++ b/target/riscv/helper.h @@ -4,7 +4,6 @@ DEF_HELPER_2(raise_exception, noreturn, env, i32) /* Floating Point - rounding mode */ DEF_HELPER_FLAGS_2(set_rounding_mode, TCG_CALL_NO_WG, void, env, i32) DEF_HELPER_FLAGS_2(set_rounding_mode_chkfrm, TCG_CALL_NO_WG, void, env, i32) -DEF_HELPER_FLAGS_1(set_rod_rounding_mode, TCG_CALL_NO_WG, void, env) /* Floating Point - fused */ DEF_HELPER_FLAGS_4(fmadd_s, TCG_CALL_NO_RWG, i64, env, i64, i64, i64) diff --git a/target/riscv/translate.c b/target/riscv/translate.c index 493c3815e1..01cc30a365 100644 --- a/target/riscv/translate.c +++ b/target/riscv/translate.c @@ -672,10 +672,6 @@ static void gen_set_rm(DisasContext *ctx, int rm) } ctx->frm = rm; - if (rm == RISCV_FRM_ROD) { - gen_helper_set_rod_rounding_mode(cpu_env); - return; - } if (rm == RISCV_FRM_DYN) { /* The helper will return only if frm valid. */ ctx->frm_valid = true; -- cgit v1.2.3