target/i386/cpu.c | 15 +++++++++++++-- 1 file changed, 13 insertions(+), 2 deletions(-)
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.
To avoid data overflow, limit the max value written to EBX[23:16] to
255.
Signed-off-by: Qian Wen <qian.wen@intel.com>
---
target/i386/cpu.c | 15 +++++++++++++--
1 file changed, 13 insertions(+), 2 deletions(-)
diff --git a/target/i386/cpu.c b/target/i386/cpu.c
index 1294be374ab2..70589a58b727 100644
--- a/target/i386/cpu.c
+++ b/target/i386/cpu.c
@@ -5356,6 +5356,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;
@@ -5397,8 +5398,18 @@ 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;
+ /*
+ * The vCPU number more than 255 needs support of V2 Extended
+ * Topology enumerated by CPUID.0x1f or Extended Topology
+ * enumerated by CPUID.0x0b.
+ */
+ threads_per_socket = cs->nr_cores * cs->nr_threads;
+ if (threads_per_socket > 255) {
+ threads_per_socket = 255;
+ }
+
+ if (threads_per_socket > 1) {
+ *ebx |= threads_per_socket << 16;
*edx |= CPUID_HT;
}
/*
--
2.25.1
On Fri, Jul 28, 2023 at 04:01:50PM +0800, Qian Wen wrote: > Date: Fri, 28 Jul 2023 16:01:50 +0800 > From: Qian Wen <qian.wen@intel.com> > Subject: [PATCH] target/i386: Avoid cpu number overflow in legacy topology > X-Mailer: git-send-email 2.25.1 > > 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. > > To avoid data overflow, limit the max value written to EBX[23:16] to > 255. > > Signed-off-by: Qian Wen <qian.wen@intel.com> > --- > target/i386/cpu.c | 15 +++++++++++++-- > 1 file changed, 13 insertions(+), 2 deletions(-) > > diff --git a/target/i386/cpu.c b/target/i386/cpu.c > index 1294be374ab2..70589a58b727 100644 > --- a/target/i386/cpu.c > +++ b/target/i386/cpu.c > @@ -5356,6 +5356,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; > @@ -5397,8 +5398,18 @@ 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; > + /* > + * The vCPU number more than 255 needs support of V2 Extended > + * Topology enumerated by CPUID.0x1f or Extended Topology > + * enumerated by CPUID.0x0b. > + */ > + threads_per_socket = cs->nr_cores * cs->nr_threads; > + if (threads_per_socket > 255) { > + threads_per_socket = 255; Straight encoding to 255 is good for me! -Zhao > + } > + > + if (threads_per_socket > 1) { > + *ebx |= threads_per_socket << 16; > *edx |= CPUID_HT; > } > /* > -- > 2.25.1 >
On 8/7/2023 4:08 PM, Zhao Liu wrote: > On Fri, Jul 28, 2023 at 04:01:50PM +0800, Qian Wen wrote: >> Date: Fri, 28 Jul 2023 16:01:50 +0800 >> From: Qian Wen <qian.wen@intel.com> >> Subject: [PATCH] target/i386: Avoid cpu number overflow in legacy topology >> X-Mailer: git-send-email 2.25.1 >> >> 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. >> >> To avoid data overflow, limit the max value written to EBX[23:16] to >> 255. >> >> Signed-off-by: Qian Wen <qian.wen@intel.com> >> --- >> target/i386/cpu.c | 15 +++++++++++++-- >> 1 file changed, 13 insertions(+), 2 deletions(-) >> >> diff --git a/target/i386/cpu.c b/target/i386/cpu.c >> index 1294be374ab2..70589a58b727 100644 >> --- a/target/i386/cpu.c >> +++ b/target/i386/cpu.c >> @@ -5356,6 +5356,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; >> @@ -5397,8 +5398,18 @@ 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; >> + /* >> + * The vCPU number more than 255 needs support of V2 Extended >> + * Topology enumerated by CPUID.0x1f or Extended Topology >> + * enumerated by CPUID.0x0b. >> + */ >> + threads_per_socket = cs->nr_cores * cs->nr_threads; >> + if (threads_per_socket > 255) { >> + threads_per_socket = 255; > Straight encoding to 255 is good for me! > > -Zhao Got it, thanks! Thanks, Qian >> + } >> + >> + if (threads_per_socket > 1) { >> + *ebx |= threads_per_socket << 16; >> *edx |= CPUID_HT; >> } >> /* >> -- >> 2.25.1 >>
On 7/28/2023 4:01 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. > > To avoid data overflow, limit the max value written to EBX[23:16] to > 255. It's better explain what's issue when overflow happens. > Signed-off-by: Qian Wen <qian.wen@intel.com> > --- > target/i386/cpu.c | 15 +++++++++++++-- > 1 file changed, 13 insertions(+), 2 deletions(-) > > diff --git a/target/i386/cpu.c b/target/i386/cpu.c > index 1294be374ab2..70589a58b727 100644 > --- a/target/i386/cpu.c > +++ b/target/i386/cpu.c > @@ -5356,6 +5356,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; > @@ -5397,8 +5398,18 @@ 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; > + /* > + * The vCPU number more than 255 needs support of V2 Extended > + * Topology enumerated by CPUID.0x1f or Extended Topology > + * enumerated by CPUID.0x0b. > + */ the above comment doesn't explain why it needs below. you can explain only bits [23:16] represents the maximum number of addressable IDs for logical processors in this physical package. When thread_per_socket > 255, it will 1) overwrite bits[31:24] which is apic_id, 2) bits [23:16] gets truncated. > + threads_per_socket = cs->nr_cores * cs->nr_threads; > + if (threads_per_socket > 255) { > + threads_per_socket = 255; > + } > + > + if (threads_per_socket > 1) { > + *ebx |= threads_per_socket << 16; > *edx |= CPUID_HT; > } > /*
On 8/7/2023 3:36 PM, Xiaoyao Li wrote: > On 7/28/2023 4:01 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. >> >> To avoid data overflow, limit the max value written to EBX[23:16] to >> 255. > > It's better explain what's issue when overflow happens. > When launch vm with -smp 256, the value writes to EBX[23:16] is 0. If the guest only support legacy topology, the result of kernel invokes cpu_smt_allowed() is false and AP's bring-up will fail. Then only CPU 0 is online, others offline. >> Signed-off-by: Qian Wen <qian.wen@intel.com> >> --- >> target/i386/cpu.c | 15 +++++++++++++-- >> 1 file changed, 13 insertions(+), 2 deletions(-) >> >> diff --git a/target/i386/cpu.c b/target/i386/cpu.c >> index 1294be374ab2..70589a58b727 100644 >> --- a/target/i386/cpu.c >> +++ b/target/i386/cpu.c >> @@ -5356,6 +5356,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; >> @@ -5397,8 +5398,18 @@ 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; >> + /* >> + * The vCPU number more than 255 needs support of V2 Extended >> + * Topology enumerated by CPUID.0x1f or Extended Topology >> + * enumerated by CPUID.0x0b. >> + */ > > the above comment doesn't explain why it needs below. > > you can explain only bits [23:16] represents the maximum number of addressable IDs for logical processors in this physical package. > > When thread_per_socket > 255, it will 1) overwrite bits[31:24] which is apic_id, 2) bits [23:16] gets truncated. Thanks for your suggestion, I will add your description in v2. Thanks, Qian > >> + threads_per_socket = cs->nr_cores * cs->nr_threads; >> + if (threads_per_socket > 255) { >> + threads_per_socket = 255; >> + } >> + >> + if (threads_per_socket > 1) { >> + *ebx |= threads_per_socket << 16; >> *edx |= CPUID_HT; >> } >> /* >
© 2016 - 2024 Red Hat, Inc.