On Thu, Nov 02, 2023 at 07:44:44PM -0300, Daniel Henrique Barboza wrote:
> This CPU was suggested by Alistair [1] and others during the profile
> design discussions. It consists of the bare 'rv64i' CPU with rva22u64
> enabled by default, like an alias of '-cpu rv64i,rva22u64=true'.
>
> Users now have an even easier way of consuming this user-mode profile by
> doing '-cpu rva22u64'. Extensions can be enabled/disabled at will on top
> of it.
>
> We can boot Linux with this "user-mode" CPU by doing:
>
> -cpu rva22u64,sv39=true,s=true,zifencei=true
>
> [1] https://lore.kernel.org/qemu-riscv/CAKmqyKP7xzZ9Sx=-Lbx2Ob0qCfB7Z+JO944FQ2TQ+49mqo0q_Q@mail.gmail.com/
>
> Signed-off-by: Daniel Henrique Barboza <dbarboza@ventanamicro.com>
> ---
> target/riscv/cpu-qom.h | 1 +
> target/riscv/cpu.c | 10 ++++++++++
> target/riscv/tcg/tcg-cpu.c | 9 +++++++++
> 3 files changed, 20 insertions(+)
>
> diff --git a/target/riscv/cpu-qom.h b/target/riscv/cpu-qom.h
> index ea9a752280..ac38ffc6cf 100644
> --- a/target/riscv/cpu-qom.h
> +++ b/target/riscv/cpu-qom.h
> @@ -37,6 +37,7 @@
> #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_RVA22U64 RISCV_CPU_TYPE_NAME("rva22u64")
> #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 d24ffbf3f8..1f2932031a 100644
> --- a/target/riscv/cpu.c
> +++ b/target/riscv/cpu.c
> @@ -1542,6 +1542,15 @@ static Property riscv_cpu_properties[] = {
> DEFINE_PROP_END_OF_LIST(),
> };
>
> +#if defined(TARGET_RISCV64)
> +static void rva22u64_bare_cpu_init(Object *obj)
The "rva22u64_bare" name is a bit weird, indicating it's both an rva22u64
type and a bare type, which isn't possible. Why not just
rva22u64_cpu_init()?
> +{
> + rv64i_bare_cpu_init(obj);
> +
> + RVA22U64.enabled = true;
> +}
> +#endif
> +
> static const gchar *riscv_gdb_arch_name(CPUState *cs)
> {
> RISCVCPU *cpu = RISCV_CPU(cs);
> @@ -1876,6 +1885,7 @@ static const TypeInfo riscv_cpu_type_infos[] = {
> 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),
> + DEFINE_BARE_CPU(TYPE_RISCV_CPU_RVA22U64, rva22u64_bare_cpu_init),
Oh, I see. Because we want to use DEFINE_BARE_CPU() here we wanted bare in
the init function name. Maybe, for self-documentation / less confusion
purposes, we should have a DEFINE_PROFILE_CPU() macro even if it's just an
alias for DEFINE_BARE_CPU().
> #endif
> };
>
> diff --git a/target/riscv/tcg/tcg-cpu.c b/target/riscv/tcg/tcg-cpu.c
> index ef43264cb3..553fb337e7 100644
> --- a/target/riscv/tcg/tcg-cpu.c
> +++ b/target/riscv/tcg/tcg-cpu.c
> @@ -1068,6 +1068,15 @@ static void riscv_cpu_add_profiles(Object *cpu_obj)
> object_property_add(cpu_obj, profile->name, "bool",
> cpu_get_profile, cpu_set_profile,
> NULL, (void *)profile);
> +
> + /*
> + * CPUs might enable a profile right from the start.
> + * Enable its mandatory extensions right away in this
> + * case.
> + */
> + if (profile->enabled) {
> + object_property_set_bool(cpu_obj, profile->name, true, NULL);
> + }
> }
> }
>
> --
> 2.41.0
>
Other than the naming nits.
Reviewed-by: Andrew Jones <ajones@ventanamicro.com>
Thanks,
drew