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

[Gavin Shan]

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