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

2024-06-04 Thread Xiaoyao Li

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

2024-06-04 Thread Zhao Liu
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

2024-06-03 Thread Chuang Xu
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