[Qemu-devel] [RFC] i386: cpu: Always report power-of-2 maximum core IDs

[Eduardo Habkost]

(Note: as this patch is just a RFC, machine-type compatibility code is
not included yet, as I first want to check if the solution I have
implemented is acceptable)

This is a bit tricky, so I am sending this as an RFC to get some
feedback. Maybe somebody from Intel could help us figure this out?


This changes the CPUID code on target-i386 to make sure the core count
reported by the CPUID instruction is always a power of 2.

Note that reporting a maximum number of IDs larger than the actual
number of cores/threads is explicitly allowed by Intel docs. Quoting
Intel SDM Volume 3, Section 8.6 Detecting hardware multi-threading
support and topology:

    Software should note that the number of logical processors enabled
    by system software may be less than the value of “Addressable IDs
    for Logical processors”. Similarly, the number of cores enabled by
    system software may be less than the value of “Addressable IDs for
    processor cores”.

This is necessary to make sure Package ID bit offset calculated using
just CPUID.1:EBX[23:16] (the method used by Linux currently[1]) matches
the offset calculated using the three-level method described at the "Sub
ID Extraction Parameters for Initial APIC ID" section in the Intel® 64
Architecture Processor Topology Enumeration Whitepaper.

 [1] arch/x86/kernel/cpu/common.c:detect_ht()

Example where the current code breaks:

Using "-smp 15,cores=5,threads=3,sockets=1", we have:

  CPUID.1:EBX[23:16] = 15
  CPUID[EAX=4,ECX=0):EAX[31:26] = 4
  RoundToNearestPof2(CPUID.1:EBX[23:16]) = 16
  RoundToNearestPof2(CPUID.1:EBX[23:16]) / ((CPUID.(EAX=4, ECX=0):EAX[31:26] ) 
+ 1) = 16/5 = 3.2
  Log2( RoundToNearestPof2(CPUID.1:EBX[23:16]) / ((CPUID.(EAX=4, 
ECX=0):EAX[31:26] ) + 1)) = ~1.7 (rounded to 2)
  SMT_Mask_Width = 2
  Log2(1 + (CPUID.(EAX=4, ECX=0):EAX[31:26] )) = Log2(5) = ~2.3 (rounded to 3)
  CoreOnly_Mask_Width = 3
  CorePlus_Mask_Width = CoreOnly_Mask_Width + SMT_Mask_Width = 3 + 2
  CorePlus_Mask_Width = 5

On the Linux code we have:

  cpuid(1, &eax, &ebx, &ecx, &edx);
  smp_num_siblings = (ebx & 0xff0000) >> 16;     /* smp_num_siblings = 15 */
  index_msb = get_count_order(smp_num_siblings); /* index_msb = 4 */
  c->phys_proc_id = apic->phys_pkg_id(c->initial_apicid, index_msb)
  /* On most cases apic->phys_pkg_id() returns (c->initial_apicid >> index_msb)
   */

Thus, the calculated CorePlus_Mask_Width doesn't match the Package ID
offset calculated using only CPUID.1:EBX[23:16] (as 15 threads would
need only 4 bits).

In other words, the current code makes the three-level topology
enumeration method (using CPUID leaves 1 and 4) incompatible with the
old two-level method, that uses only CPUID.1:EBX[23:16].

Signed-off-by: Eduardo Habkost <address@hidden>
---
 target-i386/cpu-qom.h |  4 +++-
 target-i386/cpu.c     | 23 ++++++++++++++++-------
 2 files changed, 19 insertions(+), 8 deletions(-)

diff --git a/target-i386/cpu-qom.h b/target-i386/cpu-qom.h
index 37c61e7..c619f3b 100644
--- a/target-i386/cpu-qom.h
+++ b/target-i386/cpu-qom.h
@@ -56,7 +56,8 @@ typedef struct X86CPUClass {
 /**
  * X86CPU:
  * @env: #CPUX86State
- * @nr_cores: Number of cores within this CPU package.
+ * @max_cores: Max number of core IDs (reported by CPUID instruction)
+ * @nr_cores: Actual number of cores within this CPU package.
  *
  * An x86 CPU.
  */
@@ -70,6 +71,7 @@ typedef struct X86CPU {
     /* Features that were filtered out because of missing host capabilities */
     uint32_t filtered_features[FEATURE_WORDS];
 
+    int max_cores;
     int nr_cores;
 } X86CPU;
 
diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index 60d967c..64319a4 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -1930,8 +1930,8 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t index, 
uint32_t count,
         *ebx = (env->cpuid_apic_id << 24) | 8 << 8; /* CLFLUSH size in quad 
words, Linux wants it. */
         *ecx = env->features[FEAT_1_ECX];
         *edx = env->features[FEAT_1_EDX];
-        if (cpu->nr_cores * cs->nr_threads > 1) {
-            *ebx |= (cpu->nr_cores * cs->nr_threads) << 16;
+        if (cpu->max_cores * cs->nr_threads > 1) {
+            *ebx |= (cpu->max_cores * cs->nr_threads) << 16;
             *edx |= 1 << 28;    /* HTT bit */
         }
         break;
@@ -1944,8 +1944,8 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t index, 
uint32_t count,
         break;
     case 4:
         /* cache info: needed for Core compatibility */
-        if (cpu->nr_cores > 1) {
-            *eax = (cpu->nr_cores - 1) << 26;
+        if (cpu->max_cores > 1) {
+            *eax = (cpu->max_cores - 1) << 26;
         } else {
             *eax = 0;
         }
@@ -2069,7 +2069,7 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t index, 
uint32_t count,
          * discards multiple thread information if it is set.
          * So dont set it here for Intel to make Linux guests happy.
          */
-        if (cpu->nr_cores * cs->nr_threads > 1) {
+        if (cpu->max_cores * cs->nr_threads > 1) {
             uint32_t tebx, tecx, tedx;
             get_cpuid_vendor(env, &tebx, &tecx, &tedx);
             if (tebx != CPUID_VENDOR_INTEL_1 ||
@@ -2118,8 +2118,8 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t index, 
uint32_t count,
         *ebx = 0;
         *ecx = 0;
         *edx = 0;
-        if (cpu->nr_cores * cs->nr_threads > 1) {
-            *ecx |= (cpu->nr_cores * cs->nr_threads) - 1;
+        if (cpu->max_cores * cs->nr_threads > 1) {
+            *ecx |= (cpu->max_cores * cs->nr_threads) - 1;
         }
         break;
     case 0x8000000A:
@@ -2345,6 +2345,15 @@ static void x86_cpu_realizefn(DeviceState *dev, Error 
**errp)
 
     cpu->nr_cores = smp_cores;
 
+    /* We will always report a power-of-2 core count on CPUID to make sure the
+     * Package ID bit offset calculated by guests using only CPUID.1:EBX[23:16]
+     * matches the value returned by apicid_pkg_offset(), which in turn matches
+     * the one calculated using the Pkg_ID formula found at Intel® 64
+     * Architecture Processor Topology Enumeration Whitepaper, section "Sub ID
+     * Extraction Parameters for Initial APIC ID".
+     */
+    cpu->max_cores = 1 << apicid_pkg_offset(smp_cores, smp_threads);
+
     if (env->features[FEAT_7_0_EBX] && env->cpuid_level < 7) {
         env->cpuid_level = 7;
     }
-- 
1.8.1.4


-- 
Eduardo