Re: [PATCH v4 2/9] hw/core: Make CPU topology enumeration arch-agnostic

[Zhao Liu <zhao1.liu@intel.com>]

Hi Phil,

> > -static uint32_t cpuid1f_topo_type(enum CPUTopoLevel topo_level)
> > +static uint32_t cpuid1f_topo_type(enum CpuTopologyLevel topo_level)
> >   {
> >       switch (topo_level) {
> > -    case CPU_TOPO_LEVEL_INVALID:
> > +    case CPU_TOPOLOGY_LEVEL_INVALID:
> 
> Since we use an enum, I'd rather directly use CPU_TOPOLOGY_LEVEL__MAX.
>
> Or maybe in this case ...
> 
> >           return CPUID_1F_ECX_TOPO_LEVEL_INVALID;
> > -    case CPU_TOPO_LEVEL_SMT:
> > +    case CPU_TOPOLOGY_LEVEL_THREAD:
> >           return CPUID_1F_ECX_TOPO_LEVEL_SMT;
> > -    case CPU_TOPO_LEVEL_CORE:
> > +    case CPU_TOPOLOGY_LEVEL_CORE:
> >           return CPUID_1F_ECX_TOPO_LEVEL_CORE;
> > -    case CPU_TOPO_LEVEL_MODULE:
> > +    case CPU_TOPOLOGY_LEVEL_MODULE:
> >           return CPUID_1F_ECX_TOPO_LEVEL_MODULE;
> > -    case CPU_TOPO_LEVEL_DIE:
> > +    case CPU_TOPOLOGY_LEVEL_DIE:
> >           return CPUID_1F_ECX_TOPO_LEVEL_DIE;
> >       default:
>            /* Other types are not supported in QEMU. */
>            g_assert_not_reached();
> 
> ... return CPUID_1F_ECX_TOPO_LEVEL_INVALID as default.

I prefer the first way you mentioned since I want "default" to keep
to detact unimplemented levels.

> Can be cleaned on top, so:

Yes, I'll rebase (now there's the conflict) with this fixed.

> Acked-by: Philippe Mathieu-Daudé <philmd@linaro.org>

Thanks!

Regards,
Zhao