Re: [RFC v2 1/7] hw/core: Make CPU topology enumeration arch-agnostic

[Zhao Liu]

[snip]

> > +CPUTopoInfo cpu_topo_descriptors[] = {
> > +    [CPU_TOPO_LEVEL_INVALID] = { .name = "invalid", },
> > +    [CPU_TOPO_LEVEL_THREAD]  = { .name = "thread",  },
> > +    [CPU_TOPO_LEVEL_CORE]    = { .name = "core",    },
> > +    [CPU_TOPO_LEVEL_MODULE]  = { .name = "module",  },
> > +    [CPU_TOPO_LEVEL_CLUSTER] = { .name = "cluster", },
> > +    [CPU_TOPO_LEVEL_DIE]     = { .name = "die",     },
> > +    [CPU_TOPO_LEVEL_SOCKET]  = { .name = "socket",  },
> > +    [CPU_TOPO_LEVEL_BOOK]    = { .name = "book",    },
> > +    [CPU_TOPO_LEVEL_DRAWER]  = { .name = "drawer",  },
> > +    [CPU_TOPO_LEVEL__MAX]    = { .name = NULL,      },
> > +};
> 
> This looks redundant with generated
> 
>     const QEnumLookup CPUTopoLevel_lookup = {
>         .array = (const char *const[]) {
>             [CPU_TOPO_LEVEL_INVALID] = "invalid",
>             [CPU_TOPO_LEVEL_THREAD] = "thread",
>             [CPU_TOPO_LEVEL_CORE] = "core",
>             [CPU_TOPO_LEVEL_MODULE] = "module",
>             [CPU_TOPO_LEVEL_CLUSTER] = "cluster",
>             [CPU_TOPO_LEVEL_DIE] = "die",
>             [CPU_TOPO_LEVEL_SOCKET] = "socket",
>             [CPU_TOPO_LEVEL_BOOK] = "book",
>             [CPU_TOPO_LEVEL_DRAWER] = "drawer",
>         },
>         .size = CPU_TOPO_LEVEL__MAX
>     };
> 
> > +
> > +const char *cpu_topo_to_string(CPUTopoLevel topo)
> > +{
> > +    return cpu_topo_descriptors[topo].name;
> > +}
> 
> And this with generated CPUTopoLevel_str().

Thanks! I missed these generated helpers.

[snip]

> > +##
> > +# @CPUTopoLevel:
> > +#
> > +# An enumeration of CPU topology levels.
> > +#
> > +# @invalid: Invalid topology level, used as a placeholder.
> 
> Placeholder for what?

I was trying to express that when no specific topology level is
specified, it will be initialized to this value by default.

Or what about just deleting this placeholder related words and just
saying it's "Invalid topology level"?

> > +#
> > +# @thread: thread level, which would also be called SMT level or logical
> > +#     processor level. The @threads option in -smp is used to configure
> > +#     the topology of this level.
> > +#
> > +# @core: core level. The @cores option in -smp is used to configure the
> > +#     topology of this level.
> > +#
> > +# @module: module level. The @modules option in -smp is used to
> > +#     configure the topology of this level.
> > +#
> > +# @cluster: cluster level. The @clusters option in -smp is used to
> > +#     configure the topology of this level.
> > +#
> > +# @die: die level. The @dies option in -smp is used to configure the
> > +#     topology of this level.
> > +#
> > +# @socket: socket level, which would also be called package level. The
> > +#     @sockets option in -smp is used to configure the topology of this
> > +#     level.
> > +#
> > +# @book: book level. The @books option in -smp is used to configure the
> > +#     topology of this level.
> > +#
> > +# @drawer: drawer level. The @drawers option in -smp is used to
> > +#     configure the topology of this level.
> 
> As far as I can tell, -smp is sugar for machine property "smp" of QAPI
> type SMPConfiguration.  Should we refer to SMPConfiguration instead of
> -smp?

Yes, SMPConfiguration is better.

Thanks,
Zhao