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

[Igor Mammedov]

On Tue, 18 Jul 2023 20:31:39 +1000
Gavin Shan <gshan@redhat.com> wrote:

> Hi Igor,
> 
> On 7/17/23 22:44, Igor Mammedov wrote:
> > On Fri, 14 Jul 2023 13:56:00 +0100
> > Peter Maydell <peter.maydell@linaro.org> wrote:
> >   
> >> On Fri, 14 Jul 2023 at 12:50, Igor Mammedov <imammedo@redhat.com> wrote:  
> >>>
> >>> On Thu, 13 Jul 2023 12:59:55 +0100
> >>> Peter Maydell <peter.maydell@linaro.org> wrote:
> >>>     
> >>>> On Thu, 13 Jul 2023 at 12:52, Marcin Juszkiewicz
> >>>> <marcin.juszkiewicz@linaro.org> 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.  
> >>>>
> >>>> Yes; my question is "why are we not using the user-facing
> >>>> string rather than the internal type name?".  
> >>>
> >>> With other targets full CPU type name can also be valid
> >>> user-facing string. Namely we use it with -device/device_add
> >>> interface. Considering we would like to have CPU hotplug
> >>> on ARM as well, we shouldn't not outlaw full type name.
> >>> (QMP/monitor interface also mostly uses full type names)  
> >>
> >> You don't seem to be able to use the full type name on
> >> x86-64 either:
> >>
> >> $ ./build/all/qemu-system-x86_64 -cpu pentium-x86_64-cpu
> >> qemu-system-x86_64: unable to find CPU model 'pentium-x86_64-cpu'  
> > 
> > that's because it also tied into old cpu_model resolving
> > routines, and I haven't added typename lookup the last
> > time I've touched it (it was out of topic change anyway).
> > 
> > but some targets do recognize typename, while some
> > do a lot more juggling with cpu_model (alpha/ppc),
> > and yet another class (garbage in => cpu type out).
> > 
> > With the last one we could just error,
> > while with alpha/ppc we could dumb it down to typenames
> > only.
> >   
> 
> Your summary here is correct to me. However, I don't quiet understand
> the issue you're trying to resolve. As you mentioned, there are two
> cases where the users need to specify CPU typename: (1) In the command
> lines to start VM; (2) When CPU is hot added.
> 
> For (1), the list of all available CPU is provided by each individual
> target. It's to say, each individual target is responsible for correlating
> the name (typename, CPU model name, or whatever else). Each individual
> target has its own rules for this correlation. Why do we bother to unify
> the rules so that only the typename is allowed?

I've seen others asking why you print type name instead of shorter cpu-model
used on CLI. To do that would make you write a patch to implement reverse 
mapping.
In some cases it's simple, in others plain impossible unless you can get
access to -cpu foo stored somewhere.

What I don't particularity like about adding reverse type->cpu_model mapping,
is that it would complicate code to carter to QEMU's interface inconsistencies.
And if you do it easy way (instead of fixing every target) touching only ARM,
it will be spotty at best and just add to technical debt elsewhere ->
more inconsistencies.

What I'm proposing is for you to keep printing type names.
So if others won't object to type names I'm more or less fine with your
current approach.

Instead of adding type->cpu_model mapping (it's not the first time
this particular question had arisen - there were similar patches before
on qemu-devel), get rid of shortened cpu_model in user interface (-cpu)
altogether and use CPU type name there.
Beside making UI consistent across QEMU it will also simplify
QEMU codebase (cpu-model -> type resolving machinery + all legacy junk
that was accumulated for years QEMU has existed).

>    // The output can be directly used in the command lines to start VM.
>    // I don't see any problems we have. Note that the list of available
>    // CPU names is printed by cpu_list(), which is a target specific
>    // implementation.
>    //
>    # aarch64-softmmu/qemu-system-aarch64 -cpu help
>    Available CPUs:
>      a64fx
>      arm1026
>      arm1136
>      arm1136-r2
>      arm1176
>      arm11mpcore
>      arm926
>      arm946
>      cortex-a15
>      cortex-a35
>      cortex-a53
> 
> For (2) where CPU is hot added, the help option can also be used to dump
> the available CPUs. Nothing went to wrong as I can see. The rule used here
> to correlate names with CPUs is global: typename <-> CPU
> 
>    //
>    // The printed CPU typenames can be taken as the driver directly
>    //
>    (qemu) device_add driver=?
>       :
>    CPU devices:
>    name "a64fx-arm-cpu"
>    name "cortex-a35-arm-cpu"
>    name "cortex-a53-arm-cpu"
>    name "cortex-a55-arm-cpu"
>    name "cortex-a57-arm-cpu"
>    name "cortex-a72-arm-cpu"
>    name "cortex-a76-arm-cpu"
>    name "max-arm-cpu"
>    name "neoverse-n1-arm-cpu"
> 
> >> and '-cpu help' does not list them with the suffix.  
> > 
> > both above points are fixable,
> > 
> > I can prepare PoC patches for that if there is
> > no opposition to the idea.
> >   
> 
> Please refer to above for the argument. If I'm correct, there is nothing
> to be resolved or improved.
> 
> >>> Instead it might be better to consolidate on what has
> >>> been done on making CPU '-device' compatible and
> >>> allow to use full CPU type name with '-cpu' on arm machines.
> >>>
> >>> Then later call suffix-less legacy => deprecate/drop it from
> >>> user-facing side including cleanup of all the infra we've
> >>> invented to keep mapping between cpu_model and typename.  
> >>
> >> This seems to me like a worsening of the user interface,
> >> and in practice there is not much likelihood of being
> >> able to deprecate-and-drop the nicer user-facing names,
> >> because they are baked into so many command lines and
> >> scripts.  
> > Nice names are subjective point, I suspect in a long run
> > once users switched to using longer names, they won't care much
> > about that either.
> > 
> > Also it's arguable if it is worsening UI or not.
> > I see using consolidated typenames across the board (incl. UI)
> > as a positive development.
> > 
> > As for scripts/CLI users out there, yes it would be disruptive
> > for a while but one can adapt to new naming (or use a wrapper
> > around QEMU that does suffix adding/model mapping as a crutch).
> > 
> > It weren't possible to drop anything before we introduced
> > deprecation process, but with it we can do it.
> >   
> 
> Thanks,
> Gavin
>