| Message ID | 20230816080658.3562730-2-qian.wen@intel.com |
|---|---|
| State | New |
| Headers | show |
| Series | Fix overflow of the max number of IDs for logic processor and core | expand |
On 8/16/2023 4:06 PM, Qian Wen wrote: > The legacy topology enumerated by CPUID.1.EBX[23:16] is defined in SDM > Vol2: > > Bits 23-16: Maximum number of addressable IDs for logical processors in > this physical package. > > When threads_per_socket > 255, it will 1) overwrite bits[31:24] which is > apic_id, 2) bits [23:16] get truncated. > > Specifically, if launching the VM with -smp 256, the value written to > EBX[23:16] is 0 because of data overflow. If the guest only supports > legacy topology, without V2 Extended Topology enumerated by CPUID.0x1f > or Extended Topology enumerated by CPUID.0x0b to support over 255 CPUs, > the return of the kernel invoking cpu_smt_allowed() is false and APs > (application processors) will fail to bring up. Then only CPU 0 is online, > and others are offline. > > For example, launch VM via: > qemu-system-x86_64 -M q35,accel=kvm,kernel-irqchip=split \ > -cpu qemu64,cpuid-0xb=off -smp 256 -m 32G \ > -drive file=guest.img,if=none,id=virtio-disk0,format=raw \ > -device virtio-blk-pci,drive=virtio-disk0,bootindex=1 --nographic > > The guest shows: > CPU(s): 256 > On-line CPU(s) list: 0 > Off-line CPU(s) list: 1-255 > > To avoid this issue caused by overflow, limit the max value written to > EBX[23:16] to 255 as the HW does. > > Signed-off-by: Qian Wen <qian.wen@intel.com> > Reviewed-by: Zhao Liu <zhao1.liu@intel.com> Reviewed-by: Xiaoyao Li <xiaoyao.li@intel.com> > --- > target/i386/cpu.c | 6 ++++-- > 1 file changed, 4 insertions(+), 2 deletions(-) > > diff --git a/target/i386/cpu.c b/target/i386/cpu.c > index 97ad229d8b..5c008b9d7e 100644 > --- a/target/i386/cpu.c > +++ b/target/i386/cpu.c > @@ -6008,6 +6008,7 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t index, uint32_t count, > uint32_t die_offset; > uint32_t limit; > uint32_t signature[3]; > + uint32_t threads_per_socket; > X86CPUTopoInfo topo_info; > > topo_info.dies_per_pkg = env->nr_dies; > @@ -6049,8 +6050,9 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t index, uint32_t count, > *ecx |= CPUID_EXT_OSXSAVE; > } > *edx = env->features[FEAT_1_EDX]; > - if (cs->nr_cores * cs->nr_threads > 1) { > - *ebx |= (cs->nr_cores * cs->nr_threads) << 16; > + threads_per_socket = cs->nr_cores * cs->nr_threads; > + if (threads_per_socket > 1) { > + *ebx |= MIN(threads_per_socket, 255) << 16; > *edx |= CPUID_HT; > } > if (!cpu->enable_pmu) {
On Wed, Aug 16, 2023 at 04:06:57PM +0800, Qian Wen <qian.wen@intel.com> wrote: > The legacy topology enumerated by CPUID.1.EBX[23:16] is defined in SDM > Vol2: > > Bits 23-16: Maximum number of addressable IDs for logical processors in > this physical package. > > When threads_per_socket > 255, it will 1) overwrite bits[31:24] which is > apic_id, 2) bits [23:16] get truncated. > > Specifically, if launching the VM with -smp 256, the value written to > EBX[23:16] is 0 because of data overflow. If the guest only supports > legacy topology, without V2 Extended Topology enumerated by CPUID.0x1f > or Extended Topology enumerated by CPUID.0x0b to support over 255 CPUs, > the return of the kernel invoking cpu_smt_allowed() is false and APs > (application processors) will fail to bring up. Then only CPU 0 is online, > and others are offline. > > For example, launch VM via: > qemu-system-x86_64 -M q35,accel=kvm,kernel-irqchip=split \ > -cpu qemu64,cpuid-0xb=off -smp 256 -m 32G \ > -drive file=guest.img,if=none,id=virtio-disk0,format=raw \ > -device virtio-blk-pci,drive=virtio-disk0,bootindex=1 --nographic > > The guest shows: > CPU(s): 256 > On-line CPU(s) list: 0 > Off-line CPU(s) list: 1-255 > > To avoid this issue caused by overflow, limit the max value written to > EBX[23:16] to 255 as the HW does. > > Signed-off-by: Qian Wen <qian.wen@intel.com> > Reviewed-by: Zhao Liu <zhao1.liu@intel.com> > --- > target/i386/cpu.c | 6 ++++-- > 1 file changed, 4 insertions(+), 2 deletions(-) > > diff --git a/target/i386/cpu.c b/target/i386/cpu.c > index 97ad229d8b..5c008b9d7e 100644 > --- a/target/i386/cpu.c > +++ b/target/i386/cpu.c > @@ -6008,6 +6008,7 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t index, uint32_t count, > uint32_t die_offset; > uint32_t limit; > uint32_t signature[3]; > + uint32_t threads_per_socket; > X86CPUTopoInfo topo_info; > > topo_info.dies_per_pkg = env->nr_dies; > @@ -6049,8 +6050,9 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t index, uint32_t count, > *ecx |= CPUID_EXT_OSXSAVE; > } > *edx = env->features[FEAT_1_EDX]; > - if (cs->nr_cores * cs->nr_threads > 1) { > - *ebx |= (cs->nr_cores * cs->nr_threads) << 16; > + threads_per_socket = cs->nr_cores * cs->nr_threads; > + if (threads_per_socket > 1) { > + *ebx |= MIN(threads_per_socket, 255) << 16; > *edx |= CPUID_HT; > } > if (!cpu->enable_pmu) { > -- > 2.25.1 > > Reviewed-by: Isaku Yamahata <isaku.yamahata@intel.com>
diff --git a/target/i386/cpu.c b/target/i386/cpu.c index 97ad229d8b..5c008b9d7e 100644 --- a/target/i386/cpu.c +++ b/target/i386/cpu.c @@ -6008,6 +6008,7 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t index, uint32_t count, uint32_t die_offset; uint32_t limit; uint32_t signature[3]; + uint32_t threads_per_socket; X86CPUTopoInfo topo_info; topo_info.dies_per_pkg = env->nr_dies; @@ -6049,8 +6050,9 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t index, uint32_t count, *ecx |= CPUID_EXT_OSXSAVE; } *edx = env->features[FEAT_1_EDX]; - if (cs->nr_cores * cs->nr_threads > 1) { - *ebx |= (cs->nr_cores * cs->nr_threads) << 16; + threads_per_socket = cs->nr_cores * cs->nr_threads; + if (threads_per_socket > 1) { + *ebx |= MIN(threads_per_socket, 255) << 16; *edx |= CPUID_HT; } if (!cpu->enable_pmu) {
The legacy topology enumerated by CPUID.1.EBX[23:16] is defined in SDM Vol2: Bits 23-16: Maximum number of addressable IDs for logical processors in this physical package. When threads_per_socket > 255, it will 1) overwrite bits[31:24] which is apic_id, 2) bits [23:16] get truncated. Specifically, if launching the VM with -smp 256, the value written to EBX[23:16] is 0 because of data overflow. If the guest only supports legacy topology, without V2 Extended Topology enumerated by CPUID.0x1f or Extended Topology enumerated by CPUID.0x0b to support over 255 CPUs, the return of the kernel invoking cpu_smt_allowed() is false and APs (application processors) will fail to bring up. Then only CPU 0 is online, and others are offline. For example, launch VM via: qemu-system-x86_64 -M q35,accel=kvm,kernel-irqchip=split \ -cpu qemu64,cpuid-0xb=off -smp 256 -m 32G \ -drive file=guest.img,if=none,id=virtio-disk0,format=raw \ -device virtio-blk-pci,drive=virtio-disk0,bootindex=1 --nographic The guest shows: CPU(s): 256 On-line CPU(s) list: 0 Off-line CPU(s) list: 1-255 To avoid this issue caused by overflow, limit the max value written to EBX[23:16] to 255 as the HW does. Signed-off-by: Qian Wen <qian.wen@intel.com> Reviewed-by: Zhao Liu <zhao1.liu@intel.com> --- target/i386/cpu.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-)