Re: [PATCH 7/8] target/riscv/cpu.c: honor user choice in cpu_cfg_ext_auto_update()

[Alistair Francis]

On Fri, Jul 28, 2023 at 10:32 AM Daniel Henrique Barboza
<dbarboza@ventanamicro.com> wrote:
>
> Add a new cpu_cfg_ext_is_user_set() helper to check if an extension was
> set by the user in the command line. Use it inside
> cpu_cfg_ext_auto_update() to verify if the user set a certain extension
> and, if that's the case, do not change its value.
>
> This will make us honor user choice instead of overwriting the values.
> Users will then be informed whether they're using an incompatible set of
> extensions instead of QEMU setting a magic value that works.
>
> For example, we'll now error out if the user explictly set 'zce' to true
> and 'zca' to false:
>
> $ ./build/qemu-system-riscv64 -M virt -cpu rv64,zce=true,zca=false -nographic
> qemu-system-riscv64: Zcf/Zcd/Zcb/Zcmp/Zcmt extensions require Zca extension
>
> This didn't happen before because we were enabling 'zca' if 'zce' was enabled
> regardless if the user explictly set 'zca' to false.
>
> Signed-off-by: Daniel Henrique Barboza <dbarboza@ventanamicro.com>

Reviewed-by: Alistair Francis <alistair.francis@wdc.com>

Alistair

> ---
>  target/riscv/cpu.c | 10 ++++++++++
>  1 file changed, 10 insertions(+)
>
> diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
> index a40dc865a0..644d0fdad2 100644
> --- a/target/riscv/cpu.c
> +++ b/target/riscv/cpu.c
> @@ -187,6 +187,12 @@ static int cpu_cfg_ext_get_min_version(uint32_t 
> ext_offset)
>      return PRIV_VERSION_1_10_0;
>  }
>
> +static bool cpu_cfg_ext_is_user_set(uint32_t ext_offset)
> +{
> +    return g_hash_table_contains(multi_ext_user_opts,
> +                                 GUINT_TO_POINTER(ext_offset));
> +}
> +
>  static void cpu_cfg_ext_auto_update(RISCVCPU *cpu, uint32_t ext_offset,
>                                      bool value)
>  {
> @@ -198,6 +204,10 @@ static void cpu_cfg_ext_auto_update(RISCVCPU *cpu, 
> uint32_t ext_offset,
>          return;
>      }
>
> +    if (cpu_cfg_ext_is_user_set(ext_offset)) {
> +        return;
> +    }
> +
>      if (value && env->priv_ver != PRIV_VERSION_LATEST) {
>          /* Do not enable it if priv_ver is older than min_version */
>          min_version = cpu_cfg_ext_get_min_version(ext_offset);
> --
> 2.41.0
>
>