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

[Igor Mammedov]

On Wed, 08 May 2019 10:34:44 +0200
Markus Armbruster <address@hidden> wrote:

> Eduardo Habkost <address@hidden> writes:
> 
> > On Mon, May 06, 2019 at 01:53:28PM +0200, Markus Armbruster wrote:
> >> Eduardo Habkost <address@hidden> 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?
Last time I was refactoring all this CPU name thingy, there wasn't way to 
deprecate anything
so I had to keep it. But I'd support deprecation in there now.

The same applies to aliases, sometimes its code even too complicated (ppc).
I'd ditch it if possible.
(added in loop PPC folks for opinions)

> 
> * 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?
>