Re: [PATCH v6 03/12] riscv-qmp-cmds.c: expose named features in cpu_model_expansion

[Andrew Jones]

On Sat, Oct 28, 2023 at 05:54:18AM -0300, Daniel Henrique Barboza wrote:
> Named features (zic64b the sole example at this moment) aren't expose to
> users, thus we need another way to expose them.
> 
> Go through each named feature, get its boolean value, do the needed
> conversions (bool to qbool, qbool to QObject) and add it to output dict.
> 
> Another adjustment is needed: named features are evaluated during
> finalize(), so riscv_cpu_finalize_features() needs to be mandatory
> regardless of whether we have an input dict or not. Otherwise zic64b
> will always return 'false', which is incorrect: the default values of
> cache blocksizes (cbom_blocksize and cboz_blocksize) are set to 64,

and cbop_blocksize

> satisfying the conditions for zic64b.
> 
> Here's an API usage example after this patch:
> 
>  $ ./build/qemu-system-riscv64 -S -M virt -display none
>     -qmp tcp:localhost:1234,server,wait=off
> 
>  $ ./scripts/qmp/qmp-shell localhost:1234
> Welcome to the QMP low-level shell!
> Connected to QEMU 8.1.50
> 
> (QEMU) query-cpu-model-expansion type=full model={"name":"rv64"}
> {"return": {"model":
>     {"name": "rv64", "props": {... "zic64b": true, ...}}}}
> 
> zic64b is set to 'true', as expected, since all cache sizes are 64
> bytes by default.
> 
> If we change one of the cache blocksizes, zic64b is returned as 'false':
> 
> (QEMU) query-cpu-model-expansion type=full 
> model={"name":"rv64","props":{"cbom_blocksize":128}}
> {"return": {"model":
>     {"name": "rv64", "props": {... "zic64b": false, ...}}}}
> 
> Signed-off-by: Daniel Henrique Barboza <dbarboza@ventanamicro.com>
> ---
>  target/riscv/riscv-qmp-cmds.c | 30 +++++++++++++++++++++++++-----
>  1 file changed, 25 insertions(+), 5 deletions(-)
> 
> diff --git a/target/riscv/riscv-qmp-cmds.c b/target/riscv/riscv-qmp-cmds.c
> index 2f2dbae7c8..5ada279776 100644
> --- a/target/riscv/riscv-qmp-cmds.c
> +++ b/target/riscv/riscv-qmp-cmds.c
> @@ -26,6 +26,7 @@
>  
>  #include "qapi/error.h"
>  #include "qapi/qapi-commands-machine-target.h"
> +#include "qapi/qmp/qbool.h"
>  #include "qapi/qmp/qdict.h"
>  #include "qapi/qmp/qerror.h"
>  #include "qapi/qobject-input-visitor.h"
> @@ -99,6 +100,22 @@ static void riscv_obj_add_multiext_props(Object *obj, 
> QDict *qdict_out,
>      }
>  }
>  
> +static void riscv_obj_add_named_feats_qdict(Object *obj, QDict *qdict_out)
> +{
> +    const RISCVCPUMultiExtConfig *named_cfg;
> +    RISCVCPU *cpu = RISCV_CPU(obj);
> +    QObject *value;
> +    bool flag_val;
> +
> +    for (int i = 0; riscv_cpu_named_features[i].name != NULL; i++) {
> +        named_cfg = &riscv_cpu_named_features[i];
> +        flag_val = isa_ext_is_enabled(cpu, named_cfg->offset);
> +        value = QOBJECT(qbool_from_bool(flag_val));
> +
> +        qdict_put_obj(qdict_out, named_cfg->name, value);
> +    }
> +}
> +
>  static void riscv_cpuobj_validate_qdict_in(Object *obj, QObject *props,
>                                             const QDict *qdict_in,
>                                             Error **errp)
> @@ -129,11 +146,6 @@ static void riscv_cpuobj_validate_qdict_in(Object *obj, 
> QObject *props,
>          goto err;
>      }
>  
> -    riscv_cpu_finalize_features(RISCV_CPU(obj), &local_err);
> -    if (local_err) {
> -        goto err;
> -    }
> -
>      visit_end_struct(visitor, NULL);
>  
>  err:
> @@ -191,6 +203,13 @@ CpuModelExpansionInfo 
> *qmp_query_cpu_model_expansion(CpuModelExpansionType type,
>          }
>      }
>  
> +    riscv_cpu_finalize_features(RISCV_CPU(obj), &local_err);
> +    if (local_err) {
> +        error_propagate(errp, local_err);
> +        object_unref(obj);
> +        return NULL;
> +    }
> +
>      expansion_info = g_new0(CpuModelExpansionInfo, 1);
>      expansion_info->model = g_malloc0(sizeof(*expansion_info->model));
>      expansion_info->model->name = g_strdup(model->name);
> @@ -200,6 +219,7 @@ CpuModelExpansionInfo 
> *qmp_query_cpu_model_expansion(CpuModelExpansionType type,
>      riscv_obj_add_multiext_props(obj, qdict_out, riscv_cpu_extensions);
>      riscv_obj_add_multiext_props(obj, qdict_out, 
> riscv_cpu_experimental_exts);
>      riscv_obj_add_multiext_props(obj, qdict_out, riscv_cpu_vendor_exts);
> +    riscv_obj_add_named_feats_qdict(obj, qdict_out);
>  
>      /* Add our CPU boolean options too */
>      riscv_obj_add_qdict_prop(obj, qdict_out, "mmu");
> -- 
> 2.41.0
>

Reviewed-by: Andrew Jones <ajones@ventanamicro.com>