Re: [PATCH RESEND v8 19/20] target/riscv/cpu.c: honor user choice in cpu_cfg_ext_auto_update()

[Andrew Jones <ajones@ventanamicro.com>]

On Thu, Aug 24, 2023 at 07:14:39PM -0300, Daniel Henrique Barboza 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>
> ---
>  target/riscv/cpu.c | 10 ++++++++++
>  1 file changed, 10 insertions(+)
> 
> diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
> index ae8c35402f..e07b2c73e7 100644
> --- a/target/riscv/cpu.c
> +++ b/target/riscv/cpu.c
> @@ -196,6 +196,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)
>  {
> @@ -207,6 +213,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
> 
>

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