On Wed, Sep 27, 2023 at 4:32 AM Daniel Henrique Barboza
<dbarb...@ventanamicro.com> wrote:
>
> At this moment we do not expose extension properties for vendor CPUs
> because that would allow users to change them via command line. The
> drawback is that if we were to add an API that shows all CPU properties,
> e.g. qmp-query-cpu-model-expansion, we won't be able to show extensions
> state of vendor CPUs.
>
> We have the required machinery to create extension properties for vendor
> CPUs while not allowing users to enable extensions. Disabling existing
> extensions is allowed since it can be useful for debugging.
>
> Change the set() callback cpu_set_multi_ext_cfg() to allow enabling
> extensions only for generic CPUs. In cpu_add_multi_ext_prop() let's not
> set the default values for the properties if we're not dealing with
> generic CPUs, otherwise the values set in cpu_init() of vendor CPUs will
> be overwritten. And finally, in tcg_cpu_instance_init(), add cpu user
> properties for all CPUs.
>
> For the veyron-v1 CPU, we're now able to disable existing extensions
> like smstateen:
>
> $ ./build/qemu-system-riscv64 --nographic -M virt \
> -cpu veyron-v1,smstateen=false
>
> But setting extensions that the CPU didn't set during cpu_init(), like
> V, is not allowed:
>
> $ ./build/qemu-system-riscv64 --nographic -M virt \
> -cpu veyron-v1,v=true
> qemu-system-riscv64: can't apply global veyron-v1-riscv-cpu.v=true:
> 'veyron-v1' CPU does not allow enabling extensions
>
> Signed-off-by: Daniel Henrique Barboza <dbarb...@ventanamicro.com>
Reviewed-by: Alistair Francis <alistair.fran...@wdc.com>
Alistair
> ---
> target/riscv/tcg/tcg-cpu.c | 64 +++++++++++++++++++++++++++++---------
> 1 file changed, 50 insertions(+), 14 deletions(-)
>
> diff --git a/target/riscv/tcg/tcg-cpu.c b/target/riscv/tcg/tcg-cpu.c
> index f31aa9bcc4..a90ee63b06 100644
> --- a/target/riscv/tcg/tcg-cpu.c
> +++ b/target/riscv/tcg/tcg-cpu.c
> @@ -549,6 +549,11 @@ void riscv_cpu_validate_set_extensions(RISCVCPU *cpu,
> Error **errp)
> riscv_cpu_disable_priv_spec_isa_exts(cpu);
> }
>
> +static bool riscv_cpu_is_generic(Object *cpu_obj)
> +{
> + return object_dynamic_cast(cpu_obj, TYPE_RISCV_DYNAMIC_CPU) != NULL;
> +}
> +
> /*
> * We'll get here via the following path:
> *
> @@ -632,13 +637,27 @@ static void cpu_set_misa_ext_cfg(Object *obj, Visitor
> *v, const char *name,
> target_ulong misa_bit = misa_ext_cfg->misa_bit;
> RISCVCPU *cpu = RISCV_CPU(obj);
> CPURISCVState *env = &cpu->env;
> - bool value;
> + bool generic_cpu = riscv_cpu_is_generic(obj);
> + bool prev_val, value;
>
> if (!visit_type_bool(v, name, &value, errp)) {
> return;
> }
>
> + prev_val = env->misa_ext & misa_bit;
> +
> + if (value == prev_val) {
> + return;
> + }
> +
> if (value) {
> + if (!generic_cpu) {
> + g_autofree char *cpuname = riscv_cpu_get_name(cpu);
> + error_setg(errp, "'%s' CPU does not allow enabling extensions",
> + cpuname);
> + return;
> + }
> +
> env->misa_ext |= misa_bit;
> env->misa_ext_mask |= misa_bit;
> } else {
> @@ -688,6 +707,7 @@ static const RISCVCPUMisaExtConfig misa_ext_cfgs[] = {
> */
> static void riscv_cpu_add_misa_properties(Object *cpu_obj)
> {
> + bool use_def_vals = riscv_cpu_is_generic(cpu_obj);
> int i;
>
> for (i = 0; i < ARRAY_SIZE(misa_ext_cfgs); i++) {
> @@ -706,7 +726,9 @@ static void riscv_cpu_add_misa_properties(Object *cpu_obj)
> cpu_set_misa_ext_cfg,
> NULL, (void *)misa_cfg);
> object_property_set_description(cpu_obj, name, desc);
> - object_property_set_bool(cpu_obj, name, misa_cfg->enabled, NULL);
> + if (use_def_vals) {
> + object_property_set_bool(cpu_obj, name, misa_cfg->enabled, NULL);
> + }
> }
> }
>
> @@ -714,17 +736,32 @@ static void cpu_set_multi_ext_cfg(Object *obj, Visitor
> *v, const char *name,
> void *opaque, Error **errp)
> {
> const RISCVCPUMultiExtConfig *multi_ext_cfg = opaque;
> - bool value;
> + RISCVCPU *cpu = RISCV_CPU(obj);
> + bool generic_cpu = riscv_cpu_is_generic(obj);
> + bool prev_val, value;
>
> if (!visit_type_bool(v, name, &value, errp)) {
> return;
> }
>
> - isa_ext_update_enabled(RISCV_CPU(obj), multi_ext_cfg->offset, value);
> -
> g_hash_table_insert(multi_ext_user_opts,
> GUINT_TO_POINTER(multi_ext_cfg->offset),
> (gpointer)value);
> +
> + prev_val = isa_ext_is_enabled(cpu, multi_ext_cfg->offset);
> +
> + if (value == prev_val) {
> + return;
> + }
> +
> + if (value && !generic_cpu) {
> + g_autofree char *cpuname = riscv_cpu_get_name(cpu);
> + error_setg(errp, "'%s' CPU does not allow enabling extensions",
> + cpuname);
> + return;
> + }
> +
> + isa_ext_update_enabled(cpu, multi_ext_cfg->offset, value);
> }
>
> static void cpu_get_multi_ext_cfg(Object *obj, Visitor *v, const char *name,
> @@ -739,11 +776,17 @@ static void cpu_get_multi_ext_cfg(Object *obj, Visitor
> *v, const char *name,
> static void cpu_add_multi_ext_prop(Object *cpu_obj,
> const RISCVCPUMultiExtConfig *multi_cfg)
> {
> + bool generic_cpu = riscv_cpu_is_generic(cpu_obj);
> +
> object_property_add(cpu_obj, multi_cfg->name, "bool",
> cpu_get_multi_ext_cfg,
> cpu_set_multi_ext_cfg,
> NULL, (void *)multi_cfg);
>
> + if (!generic_cpu) {
> + return;
> + }
> +
> /*
> * Set def val directly instead of using
> * object_property_set_bool() to save the set()
> @@ -828,20 +871,13 @@ static bool riscv_cpu_has_max_extensions(Object
> *cpu_obj)
> return object_dynamic_cast(cpu_obj, TYPE_RISCV_CPU_MAX) != NULL;
> }
>
> -static bool riscv_cpu_has_user_properties(Object *cpu_obj)
> -{
> - return object_dynamic_cast(cpu_obj, TYPE_RISCV_DYNAMIC_CPU) != NULL;
> -}
> -
> static void tcg_cpu_instance_init(CPUState *cs)
> {
> RISCVCPU *cpu = RISCV_CPU(cs);
> Object *obj = OBJECT(cpu);
>
> - if (riscv_cpu_has_user_properties(obj)) {
> - multi_ext_user_opts = g_hash_table_new(NULL, g_direct_equal);
> - riscv_cpu_add_user_properties(obj);
> - }
> + multi_ext_user_opts = g_hash_table_new(NULL, g_direct_equal);
> + riscv_cpu_add_user_properties(obj);
>
> if (riscv_cpu_has_max_extensions(obj)) {
> riscv_init_max_cpu_extensions(obj);
> --
> 2.41.0
>
>
