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

Zhao Liu
2024-11-01T02:21:11Z

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