|
| From: | Daniel Henrique Barboza |
| Subject: | Re: [PATCH v8 06/19] target/riscv: add rv64i CPU |
| Date: | Thu, 2 Nov 2023 11:23:04 -0300 |
| User-agent: | Mozilla Thunderbird |
On 11/2/23 06:59, Andrew Jones wrote:
On Wed, Nov 01, 2023 at 05:41:51PM -0300, Daniel Henrique Barboza wrote:We don't have any form of a 'bare bones' CPU. rv64, our default CPUs, comes with a lot of defaults. This is fine for most regular uses but it's not suitable when more control of what is actually loaded in the CPU is required. A bare-bones CPU would be annoying to deal with if not by profile support, a way to load a multitude of extensions with a single flag. Profile support is going to be implemented shortly, so let's add a CPU for it. The new 'rv64i' CPU will have only RVI loaded. It is inspired in the profile specification that dictates, for RVA22U64 [1]: "RVA22U64 Mandatory Base RV64I is the mandatory base ISA for RVA22U64" And so it seems that RV64I is the mandatory base ISA for all profiles listed in [1], making it an ideal CPU to use with profile support. rv64i is a CPU of type TYPE_RISCV_BARE_CPU. It has a mix of features from pre-existent CPUs: - it allows extensions to be enabled, like generic CPUs; - it will not inherit extension defaults, like vendor CPUs. This is the minimum extension set to boot OpenSBI and buildroot using rv64i: ./build/qemu-system-riscv64 -nographic -M virt \ -cpu rv64i,sv39=true,g=true,c=true,s=true,u=true Our minimal riscv,isa in this case will be: # cat /proc/device-tree/cpus/cpu@0/riscv,isa rv64imafdc_zicntr_zicsr_zifencei_zihpm_zca_zcd# [1] https://github.com/riscv/riscv-profiles/blob/main/profiles.adoc Signed-off-by: Daniel Henrique Barboza <dbarboza@ventanamicro.com> --- target/riscv/cpu-qom.h | 2 ++ target/riscv/cpu.c | 23 +++++++++++++++++++++++ 2 files changed, 25 insertions(+) diff --git a/target/riscv/cpu-qom.h b/target/riscv/cpu-qom.h index 7831e86d37..ea9a752280 100644 --- a/target/riscv/cpu-qom.h +++ b/target/riscv/cpu-qom.h @@ -25,6 +25,7 @@ #define TYPE_RISCV_CPU "riscv-cpu" #define TYPE_RISCV_DYNAMIC_CPU "riscv-dynamic-cpu" #define TYPE_RISCV_VENDOR_CPU "riscv-vendor-cpu" +#define TYPE_RISCV_BARE_CPU "riscv-bare-cpu"#define RISCV_CPU_TYPE_SUFFIX "-" TYPE_RISCV_CPU#define RISCV_CPU_TYPE_NAME(name) (name RISCV_CPU_TYPE_SUFFIX) @@ -35,6 +36,7 @@ #define TYPE_RISCV_CPU_BASE32 RISCV_CPU_TYPE_NAME("rv32") #define TYPE_RISCV_CPU_BASE64 RISCV_CPU_TYPE_NAME("rv64") #define TYPE_RISCV_CPU_BASE128 RISCV_CPU_TYPE_NAME("x-rv128") +#define TYPE_RISCV_CPU_RV64I RISCV_CPU_TYPE_NAME("rv64i") #define TYPE_RISCV_CPU_IBEX RISCV_CPU_TYPE_NAME("lowrisc-ibex") #define TYPE_RISCV_CPU_SHAKTI_C RISCV_CPU_TYPE_NAME("shakti-c") #define TYPE_RISCV_CPU_SIFIVE_E31 RISCV_CPU_TYPE_NAME("sifive-e31") diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c index f7c1989d14..4a6e544eaf 100644 --- a/target/riscv/cpu.c +++ b/target/riscv/cpu.c @@ -544,6 +544,16 @@ static void rv128_base_cpu_init(Object *obj) set_satp_mode_max_supported(RISCV_CPU(obj), VM_1_10_SV57); #endif } + +static void rv64i_bare_cpu_init(Object *obj) +{ + CPURISCVState *env = &RISCV_CPU(obj)->env; + riscv_cpu_set_misa(env, MXL_RV64, RVI); + + /* Remove the defaults from the parent class */ + RISCV_CPU(obj)->cfg.ext_zicntr = false; + RISCV_CPU(obj)->cfg.ext_zihpm = false;Good catch since v1, but having to do this is a bit gross. I'd prefer the parent class not enable any extensions. Each CPU type that needs these can just set them themselves.
It's less work to disable it for bare CPUs than to enable these 2 extensions for every other existing CPUs. These 2 extensions need to be enabled by default to preserve existing behavior. For the future we can, for example, create a new parent device (e.g. TYPE_RISCV_CPU_LEGACY) that enables these extensions. Current CPUs can inherit from it. TYPE_RISCV_CPU can then remain pristine, no default extensions. Thanks, Daniel
+} #else static void rv32_base_cpu_init(Object *obj) { @@ -1753,6 +1763,13 @@ void riscv_cpu_list(void) .instance_init = initfn \ }+#define DEFINE_BARE_CPU(type_name, initfn) \+ { \ + .name = type_name, \ + .parent = TYPE_RISCV_BARE_CPU, \ + .instance_init = initfn \ + } + static const TypeInfo riscv_cpu_type_infos[] = { { .name = TYPE_RISCV_CPU, @@ -1775,6 +1792,11 @@ static const TypeInfo riscv_cpu_type_infos[] = { .parent = TYPE_RISCV_CPU, .abstract = true, }, + { + .name = TYPE_RISCV_BARE_CPU, + .parent = TYPE_RISCV_CPU, + .abstract = true, + }, DEFINE_DYNAMIC_CPU(TYPE_RISCV_CPU_ANY, riscv_any_cpu_init), DEFINE_DYNAMIC_CPU(TYPE_RISCV_CPU_MAX, riscv_max_cpu_init), #if defined(TARGET_RISCV32) @@ -1791,6 +1813,7 @@ static const TypeInfo riscv_cpu_type_infos[] = { DEFINE_VENDOR_CPU(TYPE_RISCV_CPU_THEAD_C906, rv64_thead_c906_cpu_init), DEFINE_VENDOR_CPU(TYPE_RISCV_CPU_VEYRON_V1, rv64_veyron_v1_cpu_init), DEFINE_DYNAMIC_CPU(TYPE_RISCV_CPU_BASE128, rv128_base_cpu_init), + DEFINE_BARE_CPU(TYPE_RISCV_CPU_RV64I, rv64i_bare_cpu_init), #endif };--2.41.0We'll probably need to bring back the satp supported mode setting, as suggested on a previous patch, but otherwise Reviewed-by: Andrew Jones <ajones@ventanamicro.com> Thanks, drew
| [Prev in Thread] | Current Thread | [Next in Thread] |