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

[Daniel Henrique Barboza]

On 11/3/23 06:01, Andrew Jones wrote:
> 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.


Fair enough. I wasn't creative enough with scripting to see the value of 'max' 
and
profiles. Let's drop it.

Thanks,

Daniel

> 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