Re: [PATCH v3] i386/cpu: fixup number of addressable IDs for processor cores in the physical package
On Mon, Jul 01, 2024 at 09:48:02AM +0300, Michael Tokarev wrote: > Date: Mon, 1 Jul 2024 09:48:02 +0300 > From: Michael Tokarev > Subject: Re: [PATCH v3] i386/cpu: fixup number of addressable IDs for > processor cores in the physical package > > 11.06.2024 06:23, Chuang Xu 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]. > > > > 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. > > > > Fixes: d7caf13b5fcf ("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; > > In qemu 9.0, the context is a bit different here: > > > if (*eax & 31) { > int host_vcpus_per_cache = 1 + ((*eax & 0x3FFC000) >> 14); > int vcpus_per_socket = cs->nr_cores * cs->nr_threads; > if (cs->nr_cores > 1) { > *eax &= ~0xFC00; > *eax |= (pow2ceil(cs->nr_cores) - 1) << 26; > } > if (host_vcpus_per_cache > vcpus_per_socket) { > > Ie, no max_core_ids_in_package(), cores_per_pkg etc, introduced in > v9.0.0-790-gf602eb925a "i386/cpu: Use CPUCacheInfo.share_level to encode > CPUID[4]" and nearby. > > Am I right the above change becomes > > if (*eax & 31) { > int host_vcpus_per_cache = 1 + ((*eax & 0x3FFC000) >> 14); > int vcpus_per_socket = cs->nr_cores * cs->nr_threads; > -if (cs->nr_cores > 1) { > -*eax &= ~0xFC00; > -*eax |= (pow2ceil(cs->nr_cores) - 1) << 26; > -} > +*eax &= ~0xFC00; > +*eax |= (pow2ceil(cs->nr_cores) - 1) << 26; > if (host_vcpus_per_cache > vcpus_per_socket) { > *eax &= ~0x3FFC000; > *eax |= (pow2ceil(vcpus_per_socket) - 1) << 14; > > in 9.0 -- in other words, just remove the nr_cores condition check > and do the *eax assignment unconditionally ? > > From the patch description it seems like it is, but I thought I'd > ask anyway :) Hi Michael, I can help confirm your changes are correct. -Zhao
Re: [PATCH v3] i386/cpu: fixup number of addressable IDs for processor cores in the physical package
11.06.2024 06:23, Chuang Xu 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]. 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. Fixes: d7caf13b5fcf ("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; In qemu 9.0, the context is a bit different here: if (*eax & 31) { int host_vcpus_per_cache = 1 + ((*eax & 0x3FFC000) >> 14); int vcpus_per_socket = cs->nr_cores * cs->nr_threads; if (cs->nr_cores > 1) { *eax &= ~0xFC00; *eax |= (pow2ceil(cs->nr_cores) - 1) << 26; } if (host_vcpus_per_cache > vcpus_per_socket) { Ie, no max_core_ids_in_package(), cores_per_pkg etc, introduced in v9.0.0-790-gf602eb925a "i386/cpu: Use CPUCacheInfo.share_level to encode CPUID[4]" and nearby. Am I right the above change becomes if (*eax & 31) { int host_vcpus_per_cache = 1 + ((*eax & 0x3FFC000) >> 14); int vcpus_per_socket = cs->nr_cores * cs->nr_threads; -if (cs->nr_cores > 1) { -*eax &= ~0xFC00; -*eax |= (pow2ceil(cs->nr_cores) - 1) << 26; -} +*eax &= ~0xFC00; +*eax |= (pow2ceil(cs->nr_cores) - 1) << 26; if (host_vcpus_per_cache > vcpus_per_socket) { *eax &= ~0x3FFC000; *eax |= (pow2ceil(vcpus_per_socket) - 1) << 14; in 9.0 -- in other words, just remove the nr_cores condition check and do the *eax assignment unconditionally ? From the patch description it seems like it is, but I thought I'd ask anyway :) Thanks, /mjt -- GPG Key transition (from rsa2048 to rsa4096) since 2024-04-24. New key: rsa4096/61AD3D98ECDF2C8E 9D8B E14E 3F2A 9DD7 9199 28F1 61AD 3D98 ECDF 2C8E Old key: rsa2048/457CE0A0804465C5 6EE1 95D1 886E 8FFB 810D 4324 457C E0A0 8044 65C5 Transition statement: http://www.corpit.ru/mjt/gpg-transition-2024.txt
Re: [PATCH v3] i386/cpu: fixup number of addressable IDs for processor cores in the physical package
Queued, thanks. Paolo
Re: [PATCH v3] i386/cpu: fixup number of addressable IDs for processor cores in the physical package
On Tue, Jun 11, 2024 at 11:23:14AM +0800, Chuang Xu wrote: > Date: Tue, 11 Jun 2024 11:23:14 +0800 > From: Chuang Xu > Subject: [PATCH v3] 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 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. > > Fixes: d7caf13b5fcf ("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(-) Thanks! LGTM, Reviewed-by: Zhao Liu
[PATCH v3] 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 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. Fixes: d7caf13b5fcf ("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