Re: [Qemu-devel] [PATCH 0/7] Delete 16 *_cpu_class_by_name() functions

[Markus Armbruster <armbru@redhat.com>]

Eduardo Habkost <ehabkost@redhat.com> writes:

> On Mon, May 06, 2019 at 01:53:28PM +0200, Markus Armbruster wrote:
>> Eduardo Habkost <ehabkost@redhat.com> writes:
>> 
>> > This series adds a new CPUClass::class_name_format field, which
>> > allows us to delete 16 of the 21 *_cpu_class_by_name() functions
>> > that exist today.
>> 
>> Which five remain, and why?
>
> alpha_cpu_class_by_name:
> * Translates aliases based on alpha_cpu_aliases;
> * Falls back to "ev67" unconditionally
>   (there's a "TODO: remove match everything nonsense" comment).
>
> cris_cpu_class_by_name:
> * Translates "any" alias to "crisv32" if CONFIG_USER_ONLY.
>
> ppc_cpu_class_by_name:
> * Supports lookup by PVR if CPU model is a 8 digit hex number;
> * Converts CPU model to lowercase.
>
> superh_cpu_class_by_name:
> * Translates "any" alias to TYPE_SH7750R_CPU.
>
> sparc_cpu_class_by_name:
> * Replaces whitespaces with '-' on CPU model name.

I'm of course asking because I wonder whether we can dumb down this CPU
naming business to something simpler and more regular.

Let's review what we have.

For all <TARGET> in target/*:

* arm i386 lm32 m68k mips moxie openrisc riscv s390x s390x tricore
  unicore32 xtensa

  CPU type name format is <TARGET>_CPU_TYPE_NAME("%s"), which boils down
  to:

  - arm lm32 m68k moxie riscv s390x tricore unicore32 xtensa
    "%s-<TARGET>-cpu"

  - openrisc
    "%s-or1k-cpu"

  - i386
    "%s-x86_64-cpu" #ifdef TARGET_X86_64
    "%s-i386-cpu" #else

  - mips
    "%s-mips64-cpu" #ifdef TARGET_MIPS64
    "%s-mips-cpu" #else

  The %s gets replaced by the user's cpu model.

* hppa microblaze nios2 tilegx

  CPU type name format is <TARGET>-cpu.  The user's cpu model seems
  silently ignored.

* alpha cris ppc sh4 sparc

  No format, using ->class_by_name()

  - alpha

    CPU type name format is "%s-alpha-cpu".

    alpha_cpu_class_by_name() recognizes the full name, the full name
    without "-alpha-cpu" suffix, and a bunch of aliases.

  - cris

    CPU type name format is "%s-cris-cpu".

    cris_cpu_class_by_name() recognizes the name without the "-cris-cpu"
    suffix, plus "any" as alias for "crisv32-cris-cpu" #ifdef
    CONFIG_USER_ONLY (this is the default CPU type for machine
    "axis-dev88"; the other machine "none" has no default).

  - ppc

    CPU type name format is
    "%s-powerpc64-cpu" #ifdef TARGET_PPC64
    "%s-powerpc-cpu" #else

    ppc_cpu_class_by_name() recognizes the name without the suffix, plus
    the CPU type's PVR (8 digit hex number), plus a bunch of (case
    insensitive) aliases.

  - sh4

    CPU type name format is "%s-superh-cpu".

    superh_cpu_class_by_name() recognizes the name without the suffix,
    plus "any" as alias for "sh7750r-superh-cpu" (this is the default
    CPU type for machine "shix"; machines "r2d" defaults to "sh7751r",
    and "none" has no default).

  - sparc

    CPU type name format is
    "%s-sparc64-cpu" #ifdef TARGET_SPARC64
    "%s-sparc-cpu" #else

    sparc_cpu_class_by_name() recognizes the name without the suffix,
    mapping any spaces in the user's cpu model to '-'.

Observations:

* The CPU type name format is generally "%s-T-cpu", where T is either
  <TARGET> or <TARGET>64.

  Exceptions:

  - openrisc, sh4 uses or1k, superh instead.  Looks pointless to me.

  - i386 uses x86_64 instead of i38664.  Makes sense.

  - hppa, microblaze, nios2 and tilegx use CPU type name format "T-cpu",
    ignoring the user's cpu model.  These exceptions looks pointless to
    me.

* The user's CPU model is generally the "%s" part of the format.

  Exceptions:

  - alpha additionaly recognizes full type names.  If that's useful for
    alpha (I'm not sure it is), why isn't it useful for all other
    targets?

  - cris and sh4 additionaly recognize an "any" alias, cris only #ifdef
    CONFIG_USER_ONLY.

    Until PATCH 4, arm also recognizes an "any" alias #ifdef
    CONFIG_USER_ONLY.  PATCH 4 drops that, because it's redundant with
    the "any" CPU, which is a copy instead of an alias.  Sure we want to
    do have different targets do "any" in different ways?

    See aliases below.

  - ppc additionaly recognizes PVR aliases and additional (case
    insensitive) aliases.  Feels overengineered to me.  See aliases
    below.

  - sparc additionally recognizes aliases with ' ' instead of '-'.
    Feels pointless to me.  See aliases below.

* What about deprecating pointless exceptions?

* Aliases

  We have several targets roll their own CPU name aliases code.
  Assuming aliases are here to stay (i.e. we're not deprecating all of
  them): what about letting each CPU type specify a set of aliases, so
  we can recognize them in generic code?