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

[Gavin Shan]

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?

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