Re: [PATCH v9 19/19] target/riscv/tcg: do not support profiles for 'max' CPU

[Andrew Jones]

On Thu, Nov 02, 2023 at 07:44:45PM -0300, Daniel Henrique Barboza wrote:
> There's no gain in allowing the 'max' CPU to support profiles, since it
> already contains everything that QEMU can support. And we'll open the
> door for 'unorthodox' stuff like users disabling profiles of the 'max'
> CPU.

I don't see a lot of value in this patch, but maybe I'm just too cruel to
users that don't know what they're doing. I even see a negative value to
this patch because I can conceive of writing a script where I generally
want to use rv64i with my explicit list of profiles/extensions, but then
I may want to temporarily "boost" my CPU to 'max' for some reason. If
I write my script like

 CPU=rv64i
 EXTENSIONS=profile=on,extension=on
 qemu -cpu $CPU,$EXTENSIONS ...

then I can't just do

 CPU=max ./my-script

to boost my CPU, since max will error out when it sees profiles being
enabled (even though that should be no-op for it). Instead, I need to
do

 CPU=max EXTENSIONS= ./my-script

which isn't horrible, but a bit annoying.

So, personally, I would drop this patch.

Thanks,
drew

> 
> Signed-off-by: Daniel Henrique Barboza <dbarboza@ventanamicro.com>
> ---
>  target/riscv/tcg/tcg-cpu.c | 16 +++++++++++-----
>  1 file changed, 11 insertions(+), 5 deletions(-)
> 
> diff --git a/target/riscv/tcg/tcg-cpu.c b/target/riscv/tcg/tcg-cpu.c
> index 553fb337e7..9a964a426e 100644
> --- a/target/riscv/tcg/tcg-cpu.c
> +++ b/target/riscv/tcg/tcg-cpu.c
> @@ -825,6 +825,11 @@ static bool riscv_cpu_is_vendor(Object *cpu_obj)
>      return object_dynamic_cast(cpu_obj, TYPE_RISCV_VENDOR_CPU) != NULL;
>  }
>  
> +static bool riscv_cpu_has_max_extensions(Object *cpu_obj)
> +{
> +    return object_dynamic_cast(cpu_obj, TYPE_RISCV_CPU_MAX) != NULL;
> +}
> +
>  /*
>   * We'll get here via the following path:
>   *
> @@ -1003,6 +1008,12 @@ static void cpu_set_profile(Object *obj, Visitor *v, 
> const char *name,
>          return;
>      }
>  
> +    if (riscv_cpu_has_max_extensions(obj)) {
> +        error_setg(errp, "Profile %s is not available for the 'max' CPU",
> +                   profile->name);
> +        return;
> +    }
> +
>      if (cpu->env.misa_mxl != MXL_RV64) {
>          error_setg(errp, "Profile %s only available for 64 bit CPUs",
>                     profile->name);
> @@ -1251,11 +1262,6 @@ static void riscv_init_max_cpu_extensions(Object *obj)
>      }
>  }
>  
> -static bool riscv_cpu_has_max_extensions(Object *cpu_obj)
> -{
> -    return object_dynamic_cast(cpu_obj, TYPE_RISCV_CPU_MAX) != NULL;
> -}
> -
>  static void tcg_cpu_instance_init(CPUState *cs)
>  {
>      RISCVCPU *cpu = RISCV_CPU(cs);
> -- 
> 2.41.0
>