Re: [PATCH v3 0/2] Fix overflow of the max number of IDs for logic processor and core
On 8/18/2023 3:33 AM, Isaku Yamahata wrote: > On Wed, Aug 16, 2023 at 04:06:56PM +0800, > Qian Wen wrote: > >> CPUID.1.EBX[23:16]: Maximum number of addressable IDs for logical >> processors in this physical package. >> CPUID.4:EAX[31:26]: Maximum number of addressable IDs for processor cores >> in the physical package. >> >> The current qemu code doesn't limit the value written to these two fields. >> If the guest has a huge number of cores, APs (application processor) will >> fail to bring up and the wrong info will be reported. >> According to HW behavior, setting max value written to CPUID.1.EBX[23:16] >> to 255, and CPUID.4:EAX[31:26] to 63. >> >> --- >> Changes v2 -> v3: >> - Add patch 2. >> - Revise the commit message and comment to be clearer. >> - Using MIN() for limitation. >> Changes v1 -> v2: >> - Revise the commit message and comment to more clearer. >> - Rebased to v8.1.0-rc2. >> >> Qian Wen (2): >> target/i386: Avoid cpu number overflow in legacy topology >> target/i386: Avoid overflow of the cache parameter enumerated by leaf 4 >> >> target/i386/cpu.c | 8 +--- >> 1 file changed, 5 insertions(+), 3 deletions(-) >> >> base-commit: 0d52116fd82cdd1f4a88837336af5b6290c364a4 >> -- >> 2.25.1 >> > The patch itself looks good. Can we add test cases? > We have some in qemu/tests/unit/test-x86-cpuid.c. Hi Isaku, thanks for your comments! I take a look, the test-x86-cpuid.c has some tests for topology functions, e.g., apicid_smt/core/die_width, apicid_core/die/pkg_offset, x86_apicid_from_cpu_idx... Do you mean adding another test for function cpu_x86_cpuid() like below? If so, it seems that some effort is required to instantiate CPUX86State for input of cpu_x86_cpuid, but the result of this test case is obvious. So, is it necessary to add this test? + uint32_t unused, eax, ebx; + /* CPUID.1.EBX[23:16]: Maximum number of addressable IDs for logical + processors in this physical package. + */ + cpu_x86_cpuid(env, 1, 0, , , , ); + g_assert_cmpuint(ebx & 0xFF, ==, 0xFF); + + /* CPUID.4:EAX[31:26]: Maximum number of addressable IDs for processor + cores in the physical package. + */ + cpu_x86_cpuid(env, 4, 0, , , , ); + g_assert_cmpuint(ebx & 0xFC00, ==, 0xFC00); Thanks, Qian
Re: [PATCH v3 0/2] Fix overflow of the max number of IDs for logic processor and core
On Wed, Aug 16, 2023 at 04:06:56PM +0800, Qian Wen wrote: > CPUID.1.EBX[23:16]: Maximum number of addressable IDs for logical > processors in this physical package. > CPUID.4:EAX[31:26]: Maximum number of addressable IDs for processor cores > in the physical package. > > The current qemu code doesn't limit the value written to these two fields. > If the guest has a huge number of cores, APs (application processor) will > fail to bring up and the wrong info will be reported. > According to HW behavior, setting max value written to CPUID.1.EBX[23:16] > to 255, and CPUID.4:EAX[31:26] to 63. > > --- > Changes v2 -> v3: > - Add patch 2. > - Revise the commit message and comment to be clearer. > - Using MIN() for limitation. > Changes v1 -> v2: > - Revise the commit message and comment to more clearer. > - Rebased to v8.1.0-rc2. > > Qian Wen (2): > target/i386: Avoid cpu number overflow in legacy topology > target/i386: Avoid overflow of the cache parameter enumerated by leaf 4 > > target/i386/cpu.c | 8 +--- > 1 file changed, 5 insertions(+), 3 deletions(-) > > base-commit: 0d52116fd82cdd1f4a88837336af5b6290c364a4 > -- > 2.25.1 > The patch itself looks good. Can we add test cases? We have some in qemu/tests/unit/test-x86-cpuid.c. -- Isaku Yamahata
[PATCH v3 0/2] Fix overflow of the max number of IDs for logic processor and core
CPUID.1.EBX[23:16]: Maximum number of addressable IDs for logical processors in this physical package. CPUID.4:EAX[31:26]: Maximum number of addressable IDs for processor cores in the physical package. The current qemu code doesn't limit the value written to these two fields. If the guest has a huge number of cores, APs (application processor) will fail to bring up and the wrong info will be reported. According to HW behavior, setting max value written to CPUID.1.EBX[23:16] to 255, and CPUID.4:EAX[31:26] to 63. --- Changes v2 -> v3: - Add patch 2. - Revise the commit message and comment to be clearer. - Using MIN() for limitation. Changes v1 -> v2: - Revise the commit message and comment to more clearer. - Rebased to v8.1.0-rc2. Qian Wen (2): target/i386: Avoid cpu number overflow in legacy topology target/i386: Avoid overflow of the cache parameter enumerated by leaf 4 target/i386/cpu.c | 8 +--- 1 file changed, 5 insertions(+), 3 deletions(-) base-commit: 0d52116fd82cdd1f4a88837336af5b6290c364a4 -- 2.25.1
