On Thu, Jan 19, 2023 at 02:00:27PM +0100, Alexandre Ghiti wrote:
> Hi Alistair, Andrew,
>
> On Thu, Jan 19, 2023 at 1:25 AM Alistair Francis <alistai...@gmail.com> wrote:
> >
> > On Wed, Jan 18, 2023 at 10:19 PM Andrew Jones <ajo...@ventanamicro.com>
> > wrote:
> > >
> > > On Wed, Jan 18, 2023 at 10:28:46AM +1000, Alistair Francis wrote:
> > > > On Wed, Jan 18, 2023 at 2:32 AM Andrew Jones <ajo...@ventanamicro.com>
> > > > wrote:
> > > > >
> > > > > On Fri, Jan 13, 2023 at 11:34:53AM +0100, Alexandre Ghiti wrote:
> > > ...
> > > > > > +
> > > > > > + /* Get rid of 32-bit/64-bit incompatibility */
> > > > > > + for (int i = 0; i < 16; ++i) {
> > > > > > + if ((cpu->cfg.satp_mode.map & (1 << i)) && !valid_vm[i]) {
> > > > >
> > > > > If we ever define mode=1 for rv64, then 'sv32=on' will be incorrectly
> > > > > accepted as an alias. I think we should simply not define the sv32
> > > > > property for rv64 nor the rv64-only modes for rv32. So, down in
> > > > > riscv_add_satp_mode_properties() we can add some
> > > > >
> > > > > #if defined(TARGET_RISCV32)
> > > > > ...
> > > > > #elif defined(TARGET_RISCV64)
> > > > > ...
> > > > > #endif
> > > >
> > > > Do not add any #if defined(TARGET_RISCV32) to QEMU.
> > > >
> > > > We are aiming for the riscv64-softmmu to be able to emulate 32-bit
> > > > CPUs and compile time macros are the wrong solution here. Instead you
> > > > can get the xlen of the hart and use that.
> > > >
> > >
> > > Does this mean we want to be able to do the following?
> > >
> > > qemu-system-riscv64 -cpu rv32,sv32=on ...
> >
> > That's the plan
> >
> > >
> > > If so, then can we move the object_property_add() for sv32 to
> > > rv32_base_cpu_init() and the rest to rv64_base_cpu_init()?
> > > Currently, that would be doing the same thing as proposed above,
> > > since those functions are under TARGET_RISCV* defines, but I guess
> > > the object_property_add()'s would then be in more or less the right
> > > places for when the 32-bit emulation support work is started.
> >
> > Sounds like a good idea :)
>
> What about riscv_any_cpu_init and riscv_host_cpu_init?
riscv_host_cpu_init depends on KVM support, so we actually don't need to
add the properties in this patch at all. That's later work. I'm not real
clear as to what riscv_any_cpu_init is. It looks like a cpu type that
tries to enable all supported standard extensions. Maybe we need a patch
like below first and then add the sv* properties in the same way we will
for the rv*_base_cpu_init functions.
Thanks,
drew
diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
index cc75ca76677f..a2987205991e 100644
--- a/target/riscv/cpu.c
+++ b/target/riscv/cpu.c
@@ -229,19 +229,15 @@ static void set_vext_version(CPURISCVState *env, int
vext_ver)
env->vext_ver = vext_ver;
}
-static void riscv_any_cpu_init(Object *obj)
-{
- CPURISCVState *env = &RISCV_CPU(obj)->env;
-#if defined(TARGET_RISCV32)
- set_misa(env, MXL_RV32, RVI | RVM | RVA | RVF | RVD | RVC | RVU);
-#elif defined(TARGET_RISCV64)
- set_misa(env, MXL_RV64, RVI | RVM | RVA | RVF | RVD | RVC | RVU);
-#endif
- set_priv_version(env, PRIV_VERSION_1_12_0);
- register_cpu_props(DEVICE(obj));
-}
-
#if defined(TARGET_RISCV64)
+static void rv64_any_cpu_init(Object *obj)
+{
+ CPURISCVState *env = &RISCV_CPU(obj)->env;
+ set_misa(env, MXL_RV64, RVI | RVM | RVA | RVF | RVD | RVC | RVU);
+ set_priv_version(env, PRIV_VERSION_1_12_0);
+ register_cpu_props(DEVICE(obj));
+}
+
static void rv64_base_cpu_init(Object *obj)
{
CPURISCVState *env = &RISCV_CPU(obj)->env;
@@ -285,6 +281,14 @@ static void rv128_base_cpu_init(Object *obj)
set_priv_version(env, PRIV_VERSION_1_12_0);
}
#else
+static void rv32_any_cpu_init(Object *obj)
+{
+ CPURISCVState *env = &RISCV_CPU(obj)->env;
+ set_misa(env, MXL_RV32, RVI | RVM | RVA | RVF | RVD | RVC | RVU);
+ set_priv_version(env, PRIV_VERSION_1_12_0);
+ register_cpu_props(DEVICE(obj));
+}
+
static void rv32_base_cpu_init(Object *obj)
{
CPURISCVState *env = &RISCV_CPU(obj)->env;
@@ -1285,17 +1289,18 @@ static const TypeInfo riscv_cpu_type_infos[] = {
.class_size = sizeof(RISCVCPUClass),
.class_init = riscv_cpu_class_init,
},
- DEFINE_CPU(TYPE_RISCV_CPU_ANY, riscv_any_cpu_init),
#if defined(CONFIG_KVM)
DEFINE_CPU(TYPE_RISCV_CPU_HOST, riscv_host_cpu_init),
#endif
#if defined(TARGET_RISCV32)
+ DEFINE_CPU(TYPE_RISCV_CPU_ANY, rv32_any_cpu_init),
DEFINE_CPU(TYPE_RISCV_CPU_BASE32, rv32_base_cpu_init),
DEFINE_CPU(TYPE_RISCV_CPU_IBEX, rv32_ibex_cpu_init),
DEFINE_CPU(TYPE_RISCV_CPU_SIFIVE_E31, rv32_sifive_e_cpu_init),
DEFINE_CPU(TYPE_RISCV_CPU_SIFIVE_E34, rv32_imafcu_nommu_cpu_init),
DEFINE_CPU(TYPE_RISCV_CPU_SIFIVE_U34, rv32_sifive_u_cpu_init),
#elif defined(TARGET_RISCV64)
+ DEFINE_CPU(TYPE_RISCV_CPU_ANY, rv64_any_cpu_init),
DEFINE_CPU(TYPE_RISCV_CPU_BASE64, rv64_base_cpu_init),
DEFINE_CPU(TYPE_RISCV_CPU_SIFIVE_E51, rv64_sifive_e_cpu_init),
DEFINE_CPU(TYPE_RISCV_CPU_SIFIVE_U54, rv64_sifive_u_cpu_init),
