* [PATCH] target/i386: Avoid cpu number overflow in legacy topology
@ 2023-07-28 8:01 Qian Wen
2023-08-07 7:36 ` Xiaoyao Li
2023-08-07 8:08 ` Zhao Liu
0 siblings, 2 replies; 5+ messages in thread
From: Qian Wen @ 2023-07-28 8:01 UTC (permalink / raw)
To: qemu-devel; +Cc: xiaoyao.li, zhao1.liu, Qian Wen
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
^ permalink raw reply related [flat|nested] 5+ messages in thread* Re: [PATCH] target/i386: Avoid cpu number overflow in legacy topology
2023-07-28 8:01 [PATCH] target/i386: Avoid cpu number overflow in legacy topology Qian Wen
@ 2023-08-07 7:36 ` Xiaoyao Li
2023-08-07 10:08 ` Wen, Qian
2023-08-07 8:08 ` Zhao Liu
1 sibling, 1 reply; 5+ messages in thread
From: Xiaoyao Li @ 2023-08-07 7:36 UTC (permalink / raw)
To: Qian Wen, qemu-devel; +Cc: zhao1.liu
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;
> }
> /*
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH] target/i386: Avoid cpu number overflow in legacy topology
2023-08-07 7:36 ` Xiaoyao Li
@ 2023-08-07 10:08 ` Wen, Qian
0 siblings, 0 replies; 5+ messages in thread
From: Wen, Qian @ 2023-08-07 10:08 UTC (permalink / raw)
To: Xiaoyao Li, qemu-devel
Cc: zhao1.liu, Paolo Bonzini, richard.henderson, babu.moger
[-- Attachment #1: Type: text/plain, Size: 2714 bytes --]
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;
>> }
>> /*
>
[-- Attachment #2: Type: text/html, Size: 4661 bytes --]
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] target/i386: Avoid cpu number overflow in legacy topology
2023-07-28 8:01 [PATCH] target/i386: Avoid cpu number overflow in legacy topology Qian Wen
2023-08-07 7:36 ` Xiaoyao Li
@ 2023-08-07 8:08 ` Zhao Liu
2023-08-07 10:10 ` Wen, Qian
1 sibling, 1 reply; 5+ messages in thread
From: Zhao Liu @ 2023-08-07 8:08 UTC (permalink / raw)
To: Qian Wen; +Cc: qemu-devel, xiaoyao.li, zhao1.liu
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
>
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH] target/i386: Avoid cpu number overflow in legacy topology
2023-08-07 8:08 ` Zhao Liu
@ 2023-08-07 10:10 ` Wen, Qian
0 siblings, 0 replies; 5+ messages in thread
From: Wen, Qian @ 2023-08-07 10:10 UTC (permalink / raw)
To: Zhao Liu
Cc: qemu-devel, xiaoyao.li, zhao1.liu, Paolo Bonzini,
richard.henderson, babu.moger
[-- Attachment #1: Type: text/plain, Size: 2131 bytes --]
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
>>
[-- Attachment #2: Type: text/html, Size: 3055 bytes --]
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2023-08-07 10:12 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-07-28 8:01 [PATCH] target/i386: Avoid cpu number overflow in legacy topology Qian Wen
2023-08-07 7:36 ` Xiaoyao Li
2023-08-07 10:08 ` Wen, Qian
2023-08-07 8:08 ` Zhao Liu
2023-08-07 10:10 ` Wen, Qian
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).