On Thu, Apr 11, 2019 at 5:18 AM Igor Mammedov <imamm...@redhat.com> wrote: > > On Wed, 10 Apr 2019 23:10:25 +0000 > Alistair Francis <alistair.fran...@wdc.com> wrote: > > > If a user specifies a CPU that we don't understand then we want to fall > > back to a CPU generated from the ISA string. > It might look like a nice thing to do at the beginning, but > fallbacks become a source of pain in future and get in the way > of refactorings if there is a promise to maintain defaults (fallbacks) > stable. > > I suggest do not fallback to anything, just fail cleanly > with informative error telling users what is wrong and let user > fix their invalid CLI in the first place.
Maybe fall back isn't the right word then, as this is the goal of this series. Here is what I want to happen: User runs QEMU RISC-V without -cpu option: The user gets a CPU that supports "standard" ISA extensions, as what happens now User runs QEMU RISC-V with a CPU model that is hard coded in cpu.h (such as TYPE_RISCV_CPU_SIFIVE_U54): The user gets a CPU that matches that hard coded CPU string/init User runs QEMU RISC-V with a RISC-V ISA string that isn't hardcoded (for example rv64gcsuh): QEMU will dynamically create a CPU based on the specified bit length (rv32/rv64) and the ISA extensions supported The goal of this is to allow new extensions to be enabled and disabled as required. For example when we add a new extension maybe we don't want it on by default but we want an option to enable it. Or maybe a user wants to test their code on a model without compressed instruction (c extension) support. Alistair > > > > At the moment the generated CPU is assumed to be a privledge spec > > version 1.10 CPU with an MMU. This can be changed in the future. > > > > Signed-off-by: Alistair Francis <alistair.fran...@wdc.com> > > --- > > v3: > > - Ensure a minimal length so we don't run off the end of the string. > > - Don't parse the rv32/rv64 in the loop > > target/riscv/cpu.c | 101 ++++++++++++++++++++++++++++++++++++++++++++- > > target/riscv/cpu.h | 2 + > > 2 files changed, 102 insertions(+), 1 deletion(-) > > > > diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c > > index d61bce6d55..27be9e412a 100644 > > --- a/target/riscv/cpu.c > > +++ b/target/riscv/cpu.c > > @@ -19,6 +19,7 @@ > > > > #include "qemu/osdep.h" > > #include "qemu/log.h" > > +#include "qemu/error-report.h" > > #include "cpu.h" > > #include "exec/exec-all.h" > > #include "qapi/error.h" > > @@ -103,6 +104,99 @@ static void set_resetvec(CPURISCVState *env, int > > resetvec) > > #endif > > } > > > > +static void riscv_generate_cpu_init(Object *obj) > > +{ > > + RISCVCPU *cpu = RISCV_CPU(obj); > > + CPURISCVState *env = &cpu->env; > > + RISCVCPUClass *mcc = RISCV_CPU_GET_CLASS(cpu); > > + const char *riscv_cpu = mcc->isa_str; > > + target_ulong target_misa = 0; > > + target_ulong rvxlen = 0; > > + int i; > > + bool valid = false; > > + > > + /* > > + * We need at least 5 charecters for the string to be valid. Check that > > + * now so we can be lazier later. > > + */ > > + if (strlen(riscv_cpu) < 5) { > > + error_report("'%s' does not appear to be a valid RISC-V ISA > > string", > > + riscv_cpu); > > + exit(1); > > + } > > + > > + if (riscv_cpu[0] == 'r' && riscv_cpu[1] == 'v') { > > + /* Starts with "rv" */ > > + if (riscv_cpu[2] == '3' && riscv_cpu[3] == '2') { > > + valid = true; > > + rvxlen = RV32; > > + } > > + if (riscv_cpu[2] == '6' && riscv_cpu[3] == '4') { > > + valid = true; > > + rvxlen = RV64; > > + } > > + } > > + > > + if (!valid) { > > + error_report("'%s' does not appear to be a valid RISC-V CPU", > > + riscv_cpu); > > + exit(1); > > + } > > + > > + for (i = 4; i < strlen(riscv_cpu); i++) { > > + switch (riscv_cpu[i]) { > > + case 'i': > > + if (target_misa & RVE) { > > + error_report("I and E extensions are incompatible"); > > + exit(1); > > + } > > + target_misa |= RVI; > > + continue; > > + case 'e': > > + if (target_misa & RVI) { > > + error_report("I and E extensions are incompatible"); > > + exit(1); > > + } > > + target_misa |= RVE; > > + continue; > > + case 'g': > > + target_misa |= RVI | RVM | RVA | RVF | RVD; > > + continue; > > + case 'm': > > + target_misa |= RVM; > > + continue; > > + case 'a': > > + target_misa |= RVA; > > + continue; > > + case 'f': > > + target_misa |= RVF; > > + continue; > > + case 'd': > > + target_misa |= RVD; > > + continue; > > + case 'c': > > + target_misa |= RVC; > > + continue; > > + case 's': > > + target_misa |= RVS; > > + continue; > > + case 'u': > > + target_misa |= RVU; > > + continue; > > + default: > > + warn_report("QEMU does not support the %c extension", > > + riscv_cpu[i]); > > + continue; > > + } > > + } > > + > > + set_misa(env, rvxlen | target_misa); > > + set_versions(env, USER_VERSION_2_02_0, PRIV_VERSION_1_10_0); > > + set_resetvec(env, DEFAULT_RSTVEC); > > + set_feature(env, RISCV_FEATURE_MMU); > > + set_feature(env, RISCV_FEATURE_PMP); > > +} > > + > > static void riscv_any_cpu_init(Object *obj) > > { > > CPURISCVState *env = &RISCV_CPU(obj)->env; > > @@ -178,6 +272,7 @@ static void rv64imacu_nommu_cpu_init(Object *obj) > > static ObjectClass *riscv_cpu_class_by_name(const char *cpu_model) > > { > > ObjectClass *oc; > > + RISCVCPUClass *mcc; > > char *typename; > > char **cpuname; > > > > @@ -188,7 +283,10 @@ static ObjectClass *riscv_cpu_class_by_name(const char > > *cpu_model) > > g_free(typename); > > if (!oc || !object_class_dynamic_cast(oc, TYPE_RISCV_CPU) || > > object_class_is_abstract(oc)) { > > - return NULL; > > + /* No CPU found, try the generic CPU and pass in the ISA string */ > > + oc = object_class_by_name(TYPE_RISCV_CPU_GEN); > > + mcc = RISCV_CPU_CLASS(oc); > > + mcc->isa_str = g_strdup(cpu_model); > > } > > return oc; > > } > > @@ -440,6 +538,7 @@ static const TypeInfo riscv_cpu_type_infos[] = { > > .class_init = riscv_cpu_class_init, > > }, > > DEFINE_CPU(TYPE_RISCV_CPU_ANY, riscv_any_cpu_init), > > + DEFINE_CPU(TYPE_RISCV_CPU_GEN, riscv_generate_cpu_init), > > #if defined(TARGET_RISCV32) > > DEFINE_CPU(TYPE_RISCV_CPU_RV32GCSU_V1_09_1, > > rv32gcsu_priv1_09_1_cpu_init), > > DEFINE_CPU(TYPE_RISCV_CPU_RV32GCSU_V1_10_0, > > rv32gcsu_priv1_10_0_cpu_init), > > diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h > > index 20bce8742e..453108a855 100644 > > --- a/target/riscv/cpu.h > > +++ b/target/riscv/cpu.h > > @@ -48,6 +48,7 @@ > > #define CPU_RESOLVING_TYPE TYPE_RISCV_CPU > > > > #define TYPE_RISCV_CPU_ANY RISCV_CPU_TYPE_NAME("any") > > +#define TYPE_RISCV_CPU_GEN RISCV_CPU_TYPE_NAME("rv*") > > #define TYPE_RISCV_CPU_RV32GCSU_V1_09_1 > > RISCV_CPU_TYPE_NAME("rv32gcsu-v1.9.1") > > #define TYPE_RISCV_CPU_RV32GCSU_V1_10_0 > > RISCV_CPU_TYPE_NAME("rv32gcsu-v1.10.0") > > #define TYPE_RISCV_CPU_RV32IMACU_NOMMU > > RISCV_CPU_TYPE_NAME("rv32imacu-nommu") > > @@ -211,6 +212,7 @@ typedef struct RISCVCPUClass { > > /*< public >*/ > > DeviceRealize parent_realize; > > void (*parent_reset)(CPUState *cpu); > > + const char *isa_str; > > } RISCVCPUClass; > > > > /** >