Re: [PATCH v2] target/i386: Avoid cpu number overflow in legacy topology

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

Hi Qian,

On Sun, Aug 13, 2023 at 06:49:40PM +0800, Wen, Qian wrote:

[snip]

> 
> > also perhaps double check if we could do induce similar overflow
> > tweaking other -smp properties (todo for another patch[es] if there are such places).
> 
> I have a check, the CPUID.0x4:EAX[31:26] indicates the Maximum number of addressable IDs for processor cores in the physical package.
> If we launch over 64 cores VM, the 6-bits field will also overflow. I will add the following fix to patch2 in v2.

Good catch!

Just discussion, I find if we use APIC ID offset to encode 0x4, then it
seems no need for an explicit check [1], right?

[1]: https://lists.gnu.org/archive/html/qemu-devel/2023-08/msg00027.html

Thanks,
Zhao

> 
> diff --git a/target/i386/cpu.c b/target/i386/cpu.c
> index 52a2a1a1c7..9c1ae3d83d 100644
> --- a/target/i386/cpu.c
> +++ b/target/i386/cpu.c
> @@ -243,6 +243,7 @@ static void encode_cache_cpuid4(CPUCacheInfo *cache,
>                            cache->partitions * cache->sets);
> 
>      assert(num_apic_ids > 0);
> +    num_cores = num_cores > 64 ? 64 : num_cores;
>      *eax = CACHE_TYPE(cache->type) |
>             CACHE_LEVEL(cache->level) |
>             (cache->self_init ? CACHE_SELF_INIT_LEVEL : 0) |
> 
> 
> Thanks,
> Qian
> >>>>              *edx |= CPUID_HT;
> >>>>          }
> >>>>          if (!cpu->enable_pmu) {