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

[Gavin Shan]

Hi Richard,

On 7/14/23 05:27, Richard Henderson wrote:
> On 7/13/23 13:34, Gavin Shan wrote:
> > On 7/13/23 21:52, Marcin Juszkiewicz wrote:
> > > W dniu 13.07.2023 o 13:44, Peter Maydell pisze:
> > >
> > > > 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...
> > >
> > > Internally those cpu names are "max-{TYPE_ARM_CPU}" and similar for other 
> > > architectures.
> > >
> > > I like the change but it (IMHO) needs to cut "-{TYPE_*_CPU}" string from names:
> > >
> > > 13:37 marcin@applejack:qemu$ ./build/aarch64-softmmu/qemu-system-aarch64 -M 
> > > virt -cpu cortex-r5
> > > qemu-system-aarch64: Invalid CPU type: cortex-r5-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, host-arm-cpu, max-arm-cpu
> > >
> > > 13:37 marcin@applejack:qemu$ ./build/aarch64-softmmu/qemu-system-aarch64 -M 
> > > virt -cpu cortex-a57-arm-cpu
> > > qemu-system-aarch64: unable to find CPU model 'cortex-a57-arm-cpu'
> >
> > The suffix of CPU types are provided in hw/arm/virt.c::valid_cpu_types in 
> > PATCH[2].
> > In the generic validation, the complete CPU type is used. The error message also
> > have complete CPU type there.
> >
> > Peter and Marcin, how about to split the CPU types to two fields, as below? In 
> > this
> > way, the complete CPU type will be used for validation and the 'internal' names 
> > will
> > be used for the error messages.
> >
> > struct MachineClass {
> >      const char *valid_cpu_type_suffix;
> >      const char **valid_cpu_types;
>
> While you're changing this:
>
> const char * const *valid_cpu_types;

yes, will do.

> > };
> >
> > hw/arm/virt.c
> > -------------
> >
> > static const char *valid_cpu_types[] = {
>
> So that you can then do
>
> static const char * const valid_cpu_types[]

yes, will do.

Thanks,
Gavin