qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

[Qemu-devel] [PATCH v2 2/3] i386: use CpuTopoInfo instead apic_id as arg


From: Chen Fan
Subject: [Qemu-devel] [PATCH v2 2/3] i386: use CpuTopoInfo instead apic_id as argument for pc_new_cpu()
Date: Thu, 20 Mar 2014 14:33:25 +0800

introduce x86_cpu_topo_ids_from_index() to calculate the cpu topology
information, and the compat old mode mechanism moved into there.
remove unused funciton x86_apicid_from_cpu_idx().

Unit tests included.

Signed-off-by: Chen Fan <address@hidden>
---
 hw/i386/pc.c           |  22 ++++---
 target-i386/cpu.c      |  33 +++++-----
 target-i386/cpu.h      |   5 ++
 target-i386/topology.h |  26 +++++---
 tests/test-x86-cpuid.c | 165 ++++++++++++++++++++++++++++++++++++++-----------
 5 files changed, 181 insertions(+), 70 deletions(-)

diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index e715a33..84a017e 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -927,7 +927,7 @@ void pc_acpi_smi_interrupt(void *opaque, int irq, int level)
     }
 }
 
-static X86CPU *pc_new_cpu(const char *cpu_model, int64_t apic_id,
+static X86CPU *pc_new_cpu(const char *cpu_model, X86CPUTopoInfo *topo,
                           DeviceState *icc_bridge, Error **errp)
 {
     X86CPU *cpu;
@@ -939,7 +939,7 @@ static X86CPU *pc_new_cpu(const char *cpu_model, int64_t 
apic_id,
         return NULL;
     }
 
-    object_property_set_int(OBJECT(cpu), apic_id, "apic-id", &local_err);
+    object_property_set_int(OBJECT(cpu), topo->apic_id, "apic-id", &local_err);
     object_property_set_bool(OBJECT(cpu), true, "realized", &local_err);
 
     if (local_err) {
@@ -955,28 +955,29 @@ static const char *current_cpu_model;
 void pc_hot_add_cpu(const int64_t id, Error **errp)
 {
     DeviceState *icc_bridge;
-    int64_t apic_id = x86_cpu_apic_id_from_index(id);
+    X86CPUTopoInfo topo;
 
     if (id < 0) {
         error_setg(errp, "Invalid CPU id: %" PRIi64, id);
         return;
     }
 
-    if (cpu_exists(apic_id)) {
+    if (id >= max_cpus) {
         error_setg(errp, "Unable to add CPU: %" PRIi64
-                   ", it already exists", id);
+                   ", max allowed: %d", id, max_cpus - 1);
         return;
     }
 
-    if (id >= max_cpus) {
+    x86_cpu_topo_ids_from_index(id, &topo);
+    if (cpu_exists(topo.apic_id)) {
         error_setg(errp, "Unable to add CPU: %" PRIi64
-                   ", max allowed: %d", id, max_cpus - 1);
+                   ", it already exists", id);
         return;
     }
 
     icc_bridge = DEVICE(object_resolve_path_type("icc-bridge",
                                                  TYPE_ICC_BRIDGE, NULL));
-    pc_new_cpu(current_cpu_model, apic_id, icc_bridge, errp);
+    pc_new_cpu(current_cpu_model, &topo, icc_bridge, errp);
 }
 
 void pc_cpus_init(const char *cpu_model, DeviceState *icc_bridge)
@@ -996,8 +997,9 @@ void pc_cpus_init(const char *cpu_model, DeviceState 
*icc_bridge)
     current_cpu_model = cpu_model;
 
     for (i = 0; i < smp_cpus; i++) {
-        cpu = pc_new_cpu(cpu_model, x86_cpu_apic_id_from_index(i),
-                         icc_bridge, &error);
+        X86CPUTopoInfo topo;
+        x86_cpu_topo_ids_from_index(i, &topo);
+        cpu = pc_new_cpu(cpu_model, &topo, icc_bridge, &error);
         if (error) {
             error_report("%s", error_get_pretty(error));
             error_free(error);
diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index 0e8812a..6f2ba1c 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -23,8 +23,6 @@
 
 #include "cpu.h"
 #include "sysemu/kvm.h"
-#include "sysemu/cpus.h"
-#include "topology.h"
 
 #include "qemu/option.h"
 #include "qemu/config-file.h"
@@ -2584,6 +2582,21 @@ void enable_compat_apic_id_mode(void)
     compat_apic_id_mode = true;
 }
 
+void x86_cpu_topo_ids_from_index(unsigned int cpu_index, X86CPUTopoInfo *topo)
+{
+    static bool warned;
+
+    x86_topo_ids_from_idx(smp_cores, smp_threads, cpu_index, topo);
+    if (compat_apic_id_mode) {
+        if (cpu_index != topo->apic_id && !warned) {
+            error_report("APIC IDs set in compatibility mode, "
+                         "CPU topology won't match the configuration");
+            warned = true;
+        }
+        x86_topo_ids_from_apic_id(smp_cores, smp_threads, cpu_index, topo);
+    }
+}
+
 /* Calculates initial APIC ID for a specific CPU index
  *
  * Currently we need to be able to calculate the APIC ID from the CPU index
@@ -2593,20 +2606,10 @@ void enable_compat_apic_id_mode(void)
  */
 uint32_t x86_cpu_apic_id_from_index(unsigned int cpu_index)
 {
-    uint32_t correct_id;
-    static bool warned;
+    X86CPUTopoInfo topo;
 
-    correct_id = x86_apicid_from_cpu_idx(smp_cores, smp_threads, cpu_index);
-    if (compat_apic_id_mode) {
-        if (cpu_index != correct_id && !warned) {
-            error_report("APIC IDs set in compatibility mode, "
-                         "CPU topology won't match the configuration");
-            warned = true;
-        }
-        return cpu_index;
-    } else {
-        return correct_id;
-    }
+    x86_cpu_topo_ids_from_index(cpu_index, &topo);
+    return topo.apic_id;
 }
 
 static void x86_cpu_initfn(Object *obj)
diff --git a/target-i386/cpu.h b/target-i386/cpu.h
index 0014acc..a410b16 100644
--- a/target-i386/cpu.h
+++ b/target-i386/cpu.h
@@ -22,6 +22,9 @@
 #include "config.h"
 #include "qemu-common.h"
 
+#include "sysemu/cpus.h"
+#include "topology.h"
+
 #ifdef TARGET_X86_64
 #define TARGET_LONG_BITS 64
 #else
@@ -1286,6 +1289,8 @@ void x86_cpu_compat_set_features(const char *cpu_model, 
FeatureWord w,
 const char *get_register_name_32(unsigned int reg);
 
 uint32_t x86_cpu_apic_id_from_index(unsigned int cpu_index);
+void x86_cpu_topo_ids_from_index(unsigned int cpu_index,
+                                 X86CPUTopoInfo *topo);
 void enable_compat_apic_id_mode(void);
 
 #define APIC_DEFAULT_ADDRESS 0xfee00000
diff --git a/target-i386/topology.h b/target-i386/topology.h
index e9ff89c..e89e0cc 100644
--- a/target-i386/topology.h
+++ b/target-i386/topology.h
@@ -51,6 +51,7 @@ typedef struct X86CPUTopoInfo {
     unsigned pkg_id;
     unsigned core_id;
     unsigned smt_id;
+    apic_id_t apic_id;
 } X86CPUTopoInfo;
 
 /* Return the bit width needed for 'count' IDs
@@ -117,19 +118,26 @@ static inline void x86_topo_ids_from_idx(unsigned 
nr_cores,
     topo->smt_id = cpu_index % nr_threads;
     topo->core_id = core_index % nr_cores;
     topo->pkg_id = core_index / nr_cores;
+    topo->apic_id = apicid_from_topo_ids(nr_cores, nr_threads, topo);
 }
 
-/* Make APIC ID for the CPU 'cpu_index'
- *
- * 'cpu_index' is a sequential, contiguous ID for the CPU.
+/* Calculate CPU topology based on CPU APIC ID.
  */
-static inline apic_id_t x86_apicid_from_cpu_idx(unsigned nr_cores,
-                                                unsigned nr_threads,
-                                                unsigned cpu_index)
+static inline void x86_topo_ids_from_apic_id(unsigned nr_cores,
+                                             unsigned nr_threads,
+                                             apic_id_t apic_id,
+                                             X86CPUTopoInfo *topo)
 {
-    X86CPUTopoInfo topo;
-    x86_topo_ids_from_idx(nr_cores, nr_threads, cpu_index, &topo);
-    return apicid_from_topo_ids(nr_cores, nr_threads, &topo);
+    unsigned offset_mask;
+    topo->pkg_id = apic_id >> apicid_pkg_offset(nr_cores, nr_threads);
+
+    offset_mask = (1L << apicid_pkg_offset(nr_cores, nr_threads)) - 1;
+    topo->core_id = (apic_id & offset_mask)
+                    >> apicid_core_offset(nr_cores, nr_threads);
+
+    offset_mask = (1L << apicid_core_offset(nr_cores, nr_threads)) - 1;
+    topo->smt_id = apic_id & offset_mask;
+    topo->apic_id = apic_id;
 }
 
 #endif /* TARGET_I386_TOPOLOGY_H */
diff --git a/tests/test-x86-cpuid.c b/tests/test-x86-cpuid.c
index 8d9f96a..ea2cab8 100644
--- a/tests/test-x86-cpuid.c
+++ b/tests/test-x86-cpuid.c
@@ -32,11 +32,36 @@ static void test_topo_bits(void)
     g_assert_cmpuint(apicid_smt_width(1, 1), ==, 0);
     g_assert_cmpuint(apicid_core_width(1, 1), ==, 0);
 
-    g_assert_cmpuint(x86_apicid_from_cpu_idx(1, 1, 0), ==, 0);
-    g_assert_cmpuint(x86_apicid_from_cpu_idx(1, 1, 1), ==, 1);
-    g_assert_cmpuint(x86_apicid_from_cpu_idx(1, 1, 2), ==, 2);
-    g_assert_cmpuint(x86_apicid_from_cpu_idx(1, 1, 3), ==, 3);
-
+    X86CPUTopoInfo topo = {0};
+
+    x86_topo_ids_from_idx(1, 1, 0, &topo);
+    g_assert_cmpuint(topo.apic_id, ==, 0);
+    x86_topo_ids_from_idx(1, 1, 1, &topo);
+    g_assert_cmpuint(topo.apic_id, ==, 1);
+    x86_topo_ids_from_idx(1, 1, 2, &topo);
+    g_assert_cmpuint(topo.apic_id, ==, 2);
+    x86_topo_ids_from_idx(1, 1, 3, &topo);
+    g_assert_cmpuint(topo.apic_id, ==, 3);
+
+    x86_topo_ids_from_apic_id(1, 1, 0, &topo);
+    g_assert_cmpuint(topo.pkg_id,  ==, 0);
+    g_assert_cmpuint(topo.core_id, ==, 0);
+    g_assert_cmpuint(topo.smt_id,  ==, 0);
+
+    x86_topo_ids_from_apic_id(1, 1, 1, &topo);
+    g_assert_cmpuint(topo.pkg_id,  ==, 1);
+    g_assert_cmpuint(topo.core_id, ==, 0);
+    g_assert_cmpuint(topo.smt_id,  ==, 0);
+
+    x86_topo_ids_from_apic_id(1, 1, 2, &topo);
+    g_assert_cmpuint(topo.pkg_id,  ==, 2);
+    g_assert_cmpuint(topo.core_id, ==, 0);
+    g_assert_cmpuint(topo.smt_id,  ==, 0);
+
+    x86_topo_ids_from_apic_id(1, 1, 3, &topo);
+    g_assert_cmpuint(topo.pkg_id,  ==, 3);
+    g_assert_cmpuint(topo.core_id, ==, 0);
+    g_assert_cmpuint(topo.smt_id,  ==, 0);
 
     /* Test field width calculation for multiple values
      */
@@ -65,37 +90,105 @@ static void test_topo_bits(void)
     g_assert_cmpuint(apicid_core_width(6, 3), ==, 3);
     g_assert_cmpuint(apicid_pkg_offset(6, 3), ==, 5);
 
-    g_assert_cmpuint(x86_apicid_from_cpu_idx(6, 3, 0), ==, 0);
-    g_assert_cmpuint(x86_apicid_from_cpu_idx(6, 3, 1), ==, 1);
-    g_assert_cmpuint(x86_apicid_from_cpu_idx(6, 3, 2), ==, 2);
-
-    g_assert_cmpuint(x86_apicid_from_cpu_idx(6, 3, 1 * 3 + 0), ==,
-                     (1 << 2) | 0);
-    g_assert_cmpuint(x86_apicid_from_cpu_idx(6, 3, 1 * 3 + 1), ==,
-                     (1 << 2) | 1);
-    g_assert_cmpuint(x86_apicid_from_cpu_idx(6, 3, 1 * 3 + 2), ==,
-                     (1 << 2) | 2);
-
-    g_assert_cmpuint(x86_apicid_from_cpu_idx(6, 3, 2 * 3 + 0), ==,
-                     (2 << 2) | 0);
-    g_assert_cmpuint(x86_apicid_from_cpu_idx(6, 3, 2 * 3 + 1), ==,
-                     (2 << 2) | 1);
-    g_assert_cmpuint(x86_apicid_from_cpu_idx(6, 3, 2 * 3 + 2), ==,
-                     (2 << 2) | 2);
-
-    g_assert_cmpuint(x86_apicid_from_cpu_idx(6, 3, 5 * 3 + 0), ==,
-                     (5 << 2) | 0);
-    g_assert_cmpuint(x86_apicid_from_cpu_idx(6, 3, 5 * 3 + 1), ==,
-                     (5 << 2) | 1);
-    g_assert_cmpuint(x86_apicid_from_cpu_idx(6, 3, 5 * 3 + 2), ==,
-                     (5 << 2) | 2);
-
-    g_assert_cmpuint(x86_apicid_from_cpu_idx(6, 3, 1 * 6 * 3 + 0 * 3 + 0), ==,
-                     (1 << 5));
-    g_assert_cmpuint(x86_apicid_from_cpu_idx(6, 3, 1 * 6 * 3 + 1 * 3 + 1), ==,
-                     (1 << 5) | (1 << 2) | 1);
-    g_assert_cmpuint(x86_apicid_from_cpu_idx(6, 3, 3 * 6 * 3 + 5 * 3 + 2), ==,
-                     (3 << 5) | (5 << 2) | 2);
+    x86_topo_ids_from_idx(6, 3, 0, &topo);
+    g_assert_cmpuint(topo.apic_id, ==, 0);
+    x86_topo_ids_from_idx(6, 3, 1, &topo);
+    g_assert_cmpuint(topo.apic_id, ==, 1);
+    x86_topo_ids_from_idx(6, 3, 2, &topo);
+    g_assert_cmpuint(topo.apic_id, ==, 2);
+
+    x86_topo_ids_from_idx(6, 3, 1 * 3 + 0, &topo);
+    g_assert_cmpuint(topo.apic_id, ==, (1 << 2) | 0);
+    x86_topo_ids_from_idx(6, 3, 1 * 3 + 1, &topo);
+    g_assert_cmpuint(topo.apic_id, ==, (1 << 2) | 1);
+    x86_topo_ids_from_idx(6, 3, 1 * 3 + 2, &topo);
+    g_assert_cmpuint(topo.apic_id, ==, (1 << 2) | 2);
+
+    x86_topo_ids_from_idx(6, 3, 2 * 3 + 0, &topo);
+    g_assert_cmpuint(topo.apic_id, ==, (2 << 2) | 0);
+    x86_topo_ids_from_idx(6, 3, 2 * 3 + 1, &topo);
+    g_assert_cmpuint(topo.apic_id, ==, (2 << 2) | 1);
+    x86_topo_ids_from_idx(6, 3, 2 * 3 + 2, &topo);
+    g_assert_cmpuint(topo.apic_id, ==, (2 << 2) | 2);
+
+    x86_topo_ids_from_idx(6, 3, 5 * 3 + 0, &topo);
+    g_assert_cmpuint(topo.apic_id, ==, (5 << 2) | 0);
+    x86_topo_ids_from_idx(6, 3, 5 * 3 + 1, &topo);
+    g_assert_cmpuint(topo.apic_id, ==, (5 << 2) | 1);
+    x86_topo_ids_from_idx(6, 3, 5 * 3 + 2, &topo);
+    g_assert_cmpuint(topo.apic_id, ==, (5 << 2) | 2);
+
+    x86_topo_ids_from_idx(6, 3, 1 * 6 * 3 + 0 * 3 + 0, &topo);
+    g_assert_cmpuint(topo.apic_id, ==, (1 << 5) | 0);
+    x86_topo_ids_from_idx(6, 3, 1 * 6 * 3 + 1 * 3 + 1, &topo);
+    g_assert_cmpuint(topo.apic_id, ==, (1 << 5) | (1 << 2) | 1);
+    x86_topo_ids_from_idx(6, 3, 3 * 6 * 3 + 5 * 3 + 2, &topo);
+    g_assert_cmpuint(topo.apic_id, ==, (3 << 5) | (5 << 2) | 2);
+
+    x86_topo_ids_from_apic_id(6, 3, 0, &topo);
+    g_assert_cmpuint(topo.pkg_id,  ==, 0);
+    g_assert_cmpuint(topo.core_id, ==, 0);
+    g_assert_cmpuint(topo.smt_id,  ==, 0);
+    x86_topo_ids_from_apic_id(6, 3, 1, &topo);
+    g_assert_cmpuint(topo.pkg_id,  ==, 0);
+    g_assert_cmpuint(topo.core_id, ==, 0);
+    g_assert_cmpuint(topo.smt_id,  ==, 1);
+    x86_topo_ids_from_apic_id(6, 3, 2, &topo);
+    g_assert_cmpuint(topo.pkg_id,  ==, 0);
+    g_assert_cmpuint(topo.core_id, ==, 0);
+    g_assert_cmpuint(topo.smt_id,  ==, 2);
+
+    x86_topo_ids_from_apic_id(6, 3, (1 << 2) | 0, &topo);
+    g_assert_cmpuint(topo.pkg_id,  ==, 0);
+    g_assert_cmpuint(topo.core_id, ==, 1);
+    g_assert_cmpuint(topo.smt_id,  ==, 0);
+    x86_topo_ids_from_apic_id(6, 3, (1 << 2) | 1, &topo);
+    g_assert_cmpuint(topo.pkg_id,  ==, 0);
+    g_assert_cmpuint(topo.core_id, ==, 1);
+    g_assert_cmpuint(topo.smt_id,  ==, 1);
+    x86_topo_ids_from_apic_id(6, 3, (1 << 2) | 2, &topo);
+    g_assert_cmpuint(topo.pkg_id,  ==, 0);
+    g_assert_cmpuint(topo.core_id, ==, 1);
+    g_assert_cmpuint(topo.smt_id,  ==, 2);
+
+    x86_topo_ids_from_apic_id(6, 3, (2 << 2) | 0, &topo);
+    g_assert_cmpuint(topo.pkg_id,  ==, 0);
+    g_assert_cmpuint(topo.core_id, ==, 2);
+    g_assert_cmpuint(topo.smt_id,  ==, 0);
+    x86_topo_ids_from_apic_id(6, 3, (2 << 2) | 1, &topo);
+    g_assert_cmpuint(topo.pkg_id,  ==, 0);
+    g_assert_cmpuint(topo.core_id, ==, 2);
+    g_assert_cmpuint(topo.smt_id,  ==, 1);
+    x86_topo_ids_from_apic_id(6, 3, (2 << 2) | 2, &topo);
+    g_assert_cmpuint(topo.pkg_id,  ==, 0);
+    g_assert_cmpuint(topo.core_id, ==, 2);
+    g_assert_cmpuint(topo.smt_id,  ==, 2);
+
+    x86_topo_ids_from_apic_id(6, 3, (5 << 2) | 0, &topo);
+    g_assert_cmpuint(topo.pkg_id,  ==, 0);
+    g_assert_cmpuint(topo.core_id, ==, 5);
+    g_assert_cmpuint(topo.smt_id,  ==, 0);
+    x86_topo_ids_from_apic_id(6, 3, (5 << 2) | 1, &topo);
+    g_assert_cmpuint(topo.pkg_id,  ==, 0);
+    g_assert_cmpuint(topo.core_id, ==, 5);
+    g_assert_cmpuint(topo.smt_id,  ==, 1);
+    x86_topo_ids_from_apic_id(6, 3, (5 << 2) | 2, &topo);
+    g_assert_cmpuint(topo.pkg_id,  ==, 0);
+    g_assert_cmpuint(topo.core_id, ==, 5);
+    g_assert_cmpuint(topo.smt_id,  ==, 2);
+
+    x86_topo_ids_from_apic_id(6, 3, (1 << 5) | 0, &topo);
+    g_assert_cmpuint(topo.pkg_id,  ==, 1);
+    g_assert_cmpuint(topo.core_id, ==, 0);
+    g_assert_cmpuint(topo.smt_id,  ==, 0);
+    x86_topo_ids_from_apic_id(6, 3, (1 << 5) | (1 << 2) | 1, &topo);
+    g_assert_cmpuint(topo.pkg_id,  ==, 1);
+    g_assert_cmpuint(topo.core_id, ==, 1);
+    g_assert_cmpuint(topo.smt_id,  ==, 1);
+    x86_topo_ids_from_apic_id(6, 3, (1 << 5) | (1 << 2) | 2, &topo);
+    g_assert_cmpuint(topo.pkg_id,  ==, 1);
+    g_assert_cmpuint(topo.core_id, ==, 1);
+    g_assert_cmpuint(topo.smt_id,  ==, 2);
 }
 
 int main(int argc, char **argv)
-- 
1.8.1.4




reply via email to

[Prev in Thread] Current Thread [Next in Thread]