Re: [PATCH v2] i386/cpu: fixup number of addressable IDs for processor cores in the physical package
On 6/4/2024 5:43 PM, Zhao Liu wrote: Hi Chuang, On Mon, Jun 03, 2024 at 04:36:41PM +0800, Chuang Xu wrote: Date: Mon, 3 Jun 2024 16:36:41 +0800 From: Chuang Xu Subject: [PATCH v2] i386/cpu: fixup number of addressable IDs for processor cores in the physical package X-Mailer: git-send-email 2.24.3 (Apple Git-128) 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]. When testing Intel TDX, guest attempts to acquire extended topology from CPUID[0bH], but because the TDX module doesn't provide the emulation of CPUID[0bH], guest will instead acquire extended topology from CPUID[04H]. However, due to QEMU's inaccurate emulation of CPUID[04H], one of the vcpus in 2c TDX guest would be offline. I guess this case is based on downstream's TDX patches... Since TDX hasn't landed in QEMU yet, it's a bit ahead of the curve to elaborate on TDX-specific case. Because normal VM will also face the such cache topology error, I think it could be stated a bit more generically like: yes. it's not TDX specific though it's found by TDX case. I think We can reproduce it by limiting the "min-level" to less than 0xb and disable "full-cpuid-auto-level". With it, CPUID leaves 0xb are not exposed to guest and guest will use leaves 0x4 to enumerate the CPU topology. When creating a CPU topology of 1 core per package, host-cache-info only uses the Host's addressable core IDs field (CPUID.04H.EAX[bits 31-26]), resulting in a conflict (on the multicore Host) between the Guest core topology information in this field and the Guest's actual cores number. Fix it by removing the unnecessary condition to cover 1 core per package case. This is safe because cores_per_pkg will not be 0 and will be at least 1. Fix it by removing the unnecessary condition. Fixes: d7caf13b5fcf742e5680c1d3448ba070fc811644 ("x86: cpu: fixup number of addressable IDs for logical processors sharing cache") yeah. This is the exact commit that introduced the issue. Because it moved *eax &= ~0xFC00; into the condition of cs->nr_cores > 1 12 characters (d7caf13b5fcf) is enough. No blank line. ;-) Signed-off-by: Guixiong Wei Signed-off-by: Yipeng Yin Signed-off-by: Chuang Xu --- target/i386/cpu.c | 6 ++ 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/target/i386/cpu.c b/target/i386/cpu.c index bc2dceb647..b68f7460db 100644 --- a/target/i386/cpu.c +++ b/target/i386/cpu.c @@ -6426,10 +6426,8 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t index, uint32_t count, if (*eax & 31) { int host_vcpus_per_cache = 1 + ((*eax & 0x3FFC000) >> 14); -if (cores_per_pkg > 1) { -*eax &= ~0xFC00; -*eax |= max_core_ids_in_package(_info) << 26; -} +*eax &= ~0xFC00; +*eax |= max_core_ids_in_package(_info) << 26; if (host_vcpus_per_cache > threads_per_pkg) { *eax &= ~0x3FFC000; -- 2.20.1
Re: [PATCH v2] i386/cpu: fixup number of addressable IDs for processor cores in the physical package
Hi Chuang, On Mon, Jun 03, 2024 at 04:36:41PM +0800, Chuang Xu wrote: > Date: Mon, 3 Jun 2024 16:36:41 +0800 > From: Chuang Xu > Subject: [PATCH v2] i386/cpu: fixup number of addressable IDs for processor > cores in the physical package > X-Mailer: git-send-email 2.24.3 (Apple Git-128) > > 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]. > > When testing Intel TDX, guest attempts to acquire extended topology from > CPUID[0bH], > but because the TDX module doesn't provide the emulation of CPUID[0bH], > guest will instead acquire extended topology from CPUID[04H]. However, > due to QEMU's inaccurate emulation of CPUID[04H], one of the vcpus in 2c TDX > guest would be offline. I guess this case is based on downstream's TDX patches... Since TDX hasn't landed in QEMU yet, it's a bit ahead of the curve to elaborate on TDX-specific case. Because normal VM will also face the such cache topology error, I think it could be stated a bit more generically like: When creating a CPU topology of 1 core per package, host-cache-info only uses the Host's addressable core IDs field (CPUID.04H.EAX[bits 31-26]), resulting in a conflict (on the multicore Host) between the Guest core topology information in this field and the Guest's actual cores number. Fix it by removing the unnecessary condition to cover 1 core per package case. This is safe because cores_per_pkg will not be 0 and will be at least 1. > Fix it by removing the unnecessary condition. > > Fixes: d7caf13b5fcf742e5680c1d3448ba070fc811644 ("x86: cpu: fixup number of > addressable IDs for logical processors sharing cache") 12 characters (d7caf13b5fcf) is enough. No blank line. ;-) > Signed-off-by: Guixiong Wei > Signed-off-by: Yipeng Yin > Signed-off-by: Chuang Xu > --- > target/i386/cpu.c | 6 ++ > 1 file changed, 2 insertions(+), 4 deletions(-) > > diff --git a/target/i386/cpu.c b/target/i386/cpu.c > index bc2dceb647..b68f7460db 100644 > --- a/target/i386/cpu.c > +++ b/target/i386/cpu.c > @@ -6426,10 +6426,8 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t index, > uint32_t count, > if (*eax & 31) { > int host_vcpus_per_cache = 1 + ((*eax & 0x3FFC000) >> 14); > > -if (cores_per_pkg > 1) { > -*eax &= ~0xFC00; > -*eax |= max_core_ids_in_package(_info) << 26; > -} > +*eax &= ~0xFC00; > +*eax |= max_core_ids_in_package(_info) << 26; > if (host_vcpus_per_cache > threads_per_pkg) { > *eax &= ~0x3FFC000; > > -- > 2.20.1 >
[PATCH v2] i386/cpu: fixup number of addressable IDs for processor cores in the physical package
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]. When testing Intel TDX, guest attempts to acquire extended topology from CPUID[0bH], but because the TDX module doesn't provide the emulation of CPUID[0bH], guest will instead acquire extended topology from CPUID[04H]. However, due to QEMU's inaccurate emulation of CPUID[04H], one of the vcpus in 2c TDX guest would be offline. Fix it by removing the unnecessary condition. Fixes: d7caf13b5fcf742e5680c1d3448ba070fc811644 ("x86: cpu: fixup number of addressable IDs for logical processors sharing cache") Signed-off-by: Guixiong Wei Signed-off-by: Yipeng Yin Signed-off-by: Chuang Xu --- target/i386/cpu.c | 6 ++ 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/target/i386/cpu.c b/target/i386/cpu.c index bc2dceb647..b68f7460db 100644 --- a/target/i386/cpu.c +++ b/target/i386/cpu.c @@ -6426,10 +6426,8 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t index, uint32_t count, if (*eax & 31) { int host_vcpus_per_cache = 1 + ((*eax & 0x3FFC000) >> 14); -if (cores_per_pkg > 1) { -*eax &= ~0xFC00; -*eax |= max_core_ids_in_package(_info) << 26; -} +*eax &= ~0xFC00; +*eax |= max_core_ids_in_package(_info) << 26; if (host_vcpus_per_cache > threads_per_pkg) { *eax &= ~0x3FFC000; -- 2.20.1