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
>>