Re: [PATCH 0/3] hw/arm/virt: Use generic CPU invalidation

[Gavin Shan <gshan@redhat.com>]

Hi Peter,

On 7/13/23 21:44, Peter Maydell wrote:
> On Thu, 13 Jul 2023 at 06:45, Gavin Shan <gshan@redhat.com> wrote:
>>
>> There is a generic CPU type invalidation in machine_run_board_init()
>> and we needn't a same and private invalidation for hw/arm/virt machines.
>> This series intends to use the generic CPU type invalidation on the
>> hw/arm/virt machines.
>>
>> PATCH[1] factors the CPU type invalidation logic in machine_run_board_init()
>>           to a helper validate_cpu_type().
>> PATCH[2] uses the generic CPU type invalidation for hw/arm/virt machines
>> PATCH[3] support "host-arm-cpu" CPU type only when KVM or HVF is visible
>>
>> Testing
>> =======
>>
>> With the following command lines, the output messages are varied before
>> and after the series is applied.
>>
>>    /home/gshan/sandbox/src/qemu/main/build/qemu-system-aarch64 \
>>    -accel tcg -machine virt,gic-version=3,nvdimm=on            \
>>    -cpu cortex-a8 -smp maxcpus=2,cpus=1                        \
>>      :
>>
>> Before the series is applied:
>>
>>    qemu-system-aarch64: mach-virt: CPU type cortex-a8-arm-cpu not supported
>>
>> After the series is applied:
>>
>>    qemu-system-aarch64: Invalid CPU type: cortex-a8-arm-cpu
>>    The valid types are: cortex-a7-arm-cpu, cortex-a15-arm-cpu, \
>>    cortex-a35-arm-cpu, cortex-a55-arm-cpu, cortex-a72-arm-cpu, \
>>    cortex-a76-arm-cpu, a64fx-arm-cpu, neoverse-n1-arm-cpu,     \
>>    neoverse-v1-arm-cpu, cortex-a53-arm-cpu, cortex-a57-arm-cpu, \
>>    max-arm-cpu
> 
> I see this isn't a change in this patch, but given that
> what the user specifies is not "cortex-a8-arm-cpu" but
> "cortex-a8", why do we include the "-arm-cpu" suffix in
> the error messages? It's not valid syntax to say
> "-cpu cortex-a8-arm-cpu", so it's a bit misleading...
> 

Good point. Right, the complete CPU type names are provided by board (hw/arm/virt).
The compelte CPU type names are used in hw/core/machine.c to search the object
class. In the error messages in the same source file, the complete CPU type names
are also used. Actually, we need 'internal' names like 'cortex-a8' to be shown in the
error messages.

For the solution, I've suggested to split the MachineClass::valid_cpu_types to
two fields (valid_cpu_types and valid_cpu_type_suffix). Their combination is
the complete CPU type name and 'valid_cpu_types[i]' corresponds to the 'internal'
name, to be used in the error messages. Please take a look on that thread and reply
to it.

Thanks,
Gavin