Re: [PATCH] x86: cpu: fixup number of addressable IDs for processor cores in the physical package

[Igor Mammedov <imammedo@redhat.com>]

On Mon, 27 May 2024 11:13:33 +0800
Chuang Xu <xuchuangxclwt@bytedance.com> wrote:

> When QEMU is started with:
> -cpu host,host-cache-info=on,l3-cache=off \
> -smp 2,sockets=1,dies=1,cores=1,threads=2
> Guest can't acquire maximum number of addressable IDs for processor cores in
> the physical package from CPUID[04H].

please add commit message, what you are actually seeing
and expected values as well.
And if guest complains about it also include related dmesg output.


> This bug was introduced in commit d7caf13b5fcf742e5680c1d3448ba070fc811644.
> Fix it by changing the judgement condition to a >= 1.
> 
> Signed-off-by: Chuang Xu <xuchuangxclwt@bytedance.com>
> Signed-off-by: Guixiong Wei <weiguixiong@bytedance.com>
> Signed-off-by: Yipeng Yin <yinyipeng@bytedance.com>
> ---
>  target/i386/cpu.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/target/i386/cpu.c b/target/i386/cpu.c
> index cd16cb893d..0369c01153 100644
> --- a/target/i386/cpu.c
> +++ b/target/i386/cpu.c
> @@ -6097,7 +6097,7 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t index, uint32_t count,
>              if (*eax & 31) {
>                  int host_vcpus_per_cache = 1 + ((*eax & 0x3FFC000) >> 14);
>                  int vcpus_per_socket = cs->nr_cores * cs->nr_threads;

in light of dies and recent modules shouldn't we also account for them here?


> -                if (cs->nr_cores > 1) {
> +                if (cs->nr_cores >= 1) {
>                      *eax &= ~0xFC000000;
>                      *eax |= (pow2ceil(cs->nr_cores) - 1) << 26;
>                  }
above and also following condition

                if (host_vcpus_per_cache > vcpus_per_socket) {
                    ...
                    *eax |= (pow2ceil(vcpus_per_socket) - 1) << 14;

Makes me think, do we really have to have both conditionals,
Why not just drop conditions and always encode both values
to ones configured on '-smp' CLI?