[Qemu-devel] [RFC PATCH 0/5] APIC ID fixes for AMD EPYC CPU models

[Qemu-devel] [RFC PATCH 0/5] APIC ID fixes for AMD EPYC CPU models

[Moger, Babu]

These series fixes the problems encoding APIC ID for AMD EPYC cpu models.
https://bugzilla.redhat.com/show_bug.cgi?id=1728166

This is the first pass to give an idea of the changes required to address
the issue. Please feel free to comment.

Currently, apic id is decoded based on sockets/dies/cores/threads. This appears
to work for most standard configurations for AMD and other vendors. But this
decoding does not follow AMD's APIC ID enumeration. In some cases this
causes CPU topology inconstancy. While booting guest Kernel is trying to
validate topology. It finds the topology not aligning to EPYC models.

To fix the problem we need to build the topology as per the 
Processor Programming Reference (PPR) for AMD Family 17h Model 01h, Revision B1
Processors. It is available at https://www.amd.com/en/support/tech-docs

Here is the text from the PPR.
2.1.10.2.1.3
ApicId Enumeration Requirements
Operating systems are expected to use
Core::X86::Cpuid::SizeId[ApicIdCoreIdSize], the number of least
significant bits in the Initial APIC ID that indicate core ID within a
processor, in constructing per-core CPUID
masks. Core::X86::Cpuid::SizeId[ApicIdCoreIdSize] determines the maximum number
of cores (MNC) that the
processor could theoretically support, not the actual number of cores that are
actually implemented or enabled on
the processor, as indicated by Core::X86::Cpuid::SizeId[NC].
Each Core::X86::Apic::ApicId[ApicId] register is preset as follows:
• ApicId[6] = Socket ID.
• ApicId[5:4] = Node ID.
• ApicId[3] = Logical CCX L3 complex ID
• ApicId[2:0]= (SMT) ? {LogicalCoreID[1:0],ThreadId} :
{1'b0,LogicalCoreID[1:0]}.
"""

Babu Moger (5):
  hw/boards: Add sockets in CpuTopology structure
  hw/i386: Add AMD EPYC topology encoding
  i386: Use topology functions from topology.h
  hw/i386: Generate apicid based on cpu_type
  i386: Fix pkg_id offset EPYC

 hw/core/machine.c          |   1 +
 hw/i386/pc.c               |  82 ++++++++++++++++---
 include/hw/boards.h        |   2 +
 include/hw/i386/topology.h | 140 ++++++++++++++++++++++++++++++++
 target/i386/cpu.c          | 160 +++++++++----------------------------
 vl.c                       |   1 +
 6 files changed, 251 insertions(+), 135 deletions(-)

-- 
2.20.1

[Qemu-devel] [RFC PATCH 1/5] hw/boards: Add sockets in CpuTopology structure

[Moger, Babu]

Add sockets in CpuTopology. This is required when building
the CPU topology.

Signed-off-by: Babu Moger <babu.moger@amd.com>
---
 hw/core/machine.c   | 1 +
 hw/i386/pc.c        | 1 +
 include/hw/boards.h | 2 ++
 vl.c                | 1 +
 4 files changed, 5 insertions(+)

diff --git a/hw/core/machine.c b/hw/core/machine.c
index c58a8e594e..4034b7e903 100644
--- a/hw/core/machine.c
+++ b/hw/core/machine.c
@@ -795,6 +795,7 @@ static void smp_parse(MachineState *ms, QemuOpts *opts)
         ms->smp.cpus = cpus;
         ms->smp.cores = cores;
         ms->smp.threads = threads;
+        ms->smp.sockets = sockets;
     }
 
     if (ms->smp.cpus > 1) {
diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index 549c437050..ef39463fd5 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -1605,6 +1605,7 @@ void pc_smp_parse(MachineState *ms, QemuOpts *opts)
         ms->smp.cpus = cpus;
         ms->smp.cores = cores;
         ms->smp.threads = threads;
+        ms->smp.sockets = sockets;
         pcms->smp_dies = dies;
     }
 
diff --git a/include/hw/boards.h b/include/hw/boards.h
index a71d1a53a5..12eb5032a5 100644
--- a/include/hw/boards.h
+++ b/include/hw/boards.h
@@ -245,12 +245,14 @@ typedef struct DeviceMemoryState {
  * @cpus: the number of present logical processors on the machine
  * @cores: the number of cores in one package
  * @threads: the number of threads in one core
+ * @sockets: the number of sockets on the machine
  * @max_cpus: the maximum number of logical processors on the machine
  */
 typedef struct CpuTopology {
     unsigned int cpus;
     unsigned int cores;
     unsigned int threads;
+    unsigned int sockets;
     unsigned int max_cpus;
 } CpuTopology;
 
diff --git a/vl.c b/vl.c
index b426b32134..d8faf5ab43 100644
--- a/vl.c
+++ b/vl.c
@@ -3981,6 +3981,7 @@ int main(int argc, char **argv, char **envp)
     current_machine->smp.max_cpus = machine_class->default_cpus;
     current_machine->smp.cores = 1;
     current_machine->smp.threads = 1;
+    current_machine->smp.sockets = 1;
 
     machine_class->smp_parse(current_machine,
         qemu_opts_find(qemu_find_opts("smp-opts"), NULL));
-- 
2.20.1

[Qemu-devel] [RFC PATCH 2/5] hw/i386: Add AMD EPYC topology encoding

[Moger, Babu]

Currently, the apicid is a sequential number in x86 cpu models. This
works fine for most of the cases. But, in certain cases this will
result into cpu topology inconsistency. This problem was observed in
AMD EPYC cpu models.

To address that we need to build apicid as per the hardware specification.
We are attempting to build the topology close to the hardware as much as
possible.

Per Processor Programming Reference (PPR) for AMD Family 17h Models,
we need to follow the following decoding.

    2.1.10.2.1.3
    ApicId Enumeration Requirements
    Operating systems are expected to use
    Core::X86::Cpuid::SizeId[ApicIdCoreIdSize], the number of least
    significant bits in the Initial APIC ID that indicate core ID within a
    processor, in constructing per-core CPUID
    masks. Core::X86::Cpuid::SizeId[ApicIdCoreIdSize] determines the maximum
    number of cores (MNC) that the
    processor could theoretically support, not the actual number of cores that
    are actually implemented or enabled on
    the processor, as indicated by Core::X86::Cpuid::SizeId[NC].
    Each Core::X86::Apic::ApicId[ApicId] register is preset as follows:
    • ApicId[6] = Socket ID.
    • ApicId[5:4] = Node ID.
    • ApicId[3] = Logical CCX L3 complex ID
    • ApicId[2:0]= (SMT) ? {LogicalCoreID[1:0],ThreadId} :
    {1'b0,LogicalCoreID[1:0]}.

Add the structures and functions to decode the ApicId for AMD EPYC models.

Fixes: https://bugzilla.redhat.com/show_bug.cgi?id=1728166

Signed-off-by: Babu Moger <babu.moger@amd.com>
---
 include/hw/i386/topology.h | 140 +++++++++++++++++++++++++++++++++++++
 1 file changed, 140 insertions(+)

diff --git a/include/hw/i386/topology.h b/include/hw/i386/topology.h
index 4ff5b2da6c..0b2fd46b41 100644
--- a/include/hw/i386/topology.h
+++ b/include/hw/i386/topology.h
@@ -50,6 +50,7 @@ typedef struct X86CPUTopoInfo {
     unsigned die_id;
     unsigned core_id;
     unsigned smt_id;
+    unsigned ccx_id;
 } X86CPUTopoInfo;
 
 /* Return the bit width needed for 'count' IDs
@@ -179,4 +180,143 @@ static inline apic_id_t x86_apicid_from_cpu_idx(unsigned nr_dies,
     return apicid_from_topo_ids(nr_dies, nr_cores, nr_threads, &topo);
 }
 
+/* Definitions used for building CPUID Leaf 0x8000001D and 0x8000001E
+ * Please refer to the AMD64 Architecture Programmer’s Manual Volume 3.
+ * Define the constants to build the cpu topology. Right now, TOPOEXT
+ * feature is enabled only on EPYC. So, these constants are based on
+ * EPYC supported configurations. We may need to handle the cases if
+ * these values change in future.
+ */
+
+/* Maximum core complexes in a node */
+#define MAX_CCX                  2
+/* Maximum cores in a core complex */
+#define MAX_CORES_IN_CCX         4
+/* Maximum cores in a node */
+#define MAX_CORES_IN_DIE         8
+#define CCX_OFFSET_EPYC          3
+#define DIE_OFFSET_EPYC          4
+#define PKG_OFFSET_EPYC          6
+
+/* Figure out the number of dies required to build this config.
+ * Max cores in a die is 8
+ */
+static inline int dies_in_socket(int numa_nodes, int smp_sockets, int smp_cores)
+{
+    /*
+     * Create a config with user given (nr_nodes > 1) numa node config,
+     * else go with a standard configuration
+     */
+    if (numa_nodes > 1)
+        return DIV_ROUND_UP(numa_nodes, smp_sockets);
+    else
+        return DIV_ROUND_UP(smp_cores, MAX_CORES_IN_DIE);
+}
+
+/* Decide the number of cores in a core complex with the given nr_cores using
+ * following set constants MAX_CCX, MAX_CORES_IN_CCX, MAX_CORES_IN_DIE and
+ * MAX_NODES_PER_SOCKET. Maintain symmetry as much as possible
+ * L3 cache is shared across all cores in a core complex. So, this will also
+ * tell us how many cores are sharing the L3 cache.
+ */
+static inline int cores_in_ccx(int numa_nodes, int smp_sockets, int smp_cores)
+{
+    int dies;
+
+    /* Check if we can fit all the cores in one core complex */
+    if (smp_cores <= MAX_CORES_IN_CCX) {
+        return smp_cores;
+    }
+
+    /* Get the number of nodes required to build this config */
+    dies = dies_in_socket(numa_nodes, smp_sockets, smp_cores);
+
+    /*
+     * Divide the cores accros all the core complexes
+     * Return rounded up value
+     */
+    return DIV_ROUND_UP(smp_cores, dies * MAX_CCX);
+}
+
+/* Calculate thread/core/package IDs for a specific topology,
+ * based on APIC ID
+ */
+static inline void x86_topo_ids_from_apicid_epyc(apic_id_t apicid,
+                                                 unsigned smp_dies,
+                                                 unsigned smp_cores,
+                                                 unsigned smp_threads,
+                                                 X86CPUTopoInfo *topo)
+{
+    topo->smt_id = apicid &
+                   ~(0xFFFFFFFFUL << apicid_smt_width(smp_dies, smp_cores, smp_threads));
+    topo->core_id = (apicid >> apicid_core_offset(smp_dies, smp_cores, smp_threads)) &
+                   ~(0xFFFFFFFFUL << apicid_core_width(smp_dies, smp_cores, smp_threads));
+    topo->ccx_id = (apicid >> CCX_OFFSET_EPYC) & 1;
+    topo->die_id = (apicid >> DIE_OFFSET_EPYC) & 3;
+    topo->pkg_id = apicid >> PKG_OFFSET_EPYC;
+}
+
+/* Calculate thread/core/package IDs for a specific topology,
+ * based on (contiguous) CPU index
+ * Processor Programming Reference (PPR) for AMD Family 17h Model 01h,
+ * Revision B1 Processors
+ * ApicId Enumeration Requirements
+ • ApicId[6] = Socket ID.
+ • ApicId[5:4] = Node ID.
+ • ApicId[3] = Logical CCX L3 complex ID
+ • ApicId[2:0]= (SMT) ? {LogicalCoreID[1:0],ThreadId} : {1'b0,LogicalCoreID[1:0]}.
+ */
+static inline void x86_topo_ids_from_idx_epyc(unsigned numa_nodes,
+		                              unsigned smp_sockets,
+					      unsigned smp_cores,
+                                              unsigned smp_threads,
+                                              unsigned cpu_index,
+                                              X86CPUTopoInfo *topo)
+{
+    unsigned core_index = cpu_index / smp_threads;
+    unsigned core_id = core_index % smp_cores; /* Core id inside the socket */
+    unsigned ccx_cores;
+
+    topo->smt_id = cpu_index % smp_threads;
+
+    ccx_cores = cores_in_ccx(numa_nodes, smp_sockets, smp_cores);
+
+    topo->core_id = core_id % ccx_cores; /* core id inside the ccx */
+    topo->ccx_id = (core_id % (ccx_cores * MAX_CCX)) / ccx_cores;
+    topo->die_id = core_id / (ccx_cores * MAX_CCX);
+    topo->pkg_id = core_index / smp_cores;
+}
+
+/* Make APIC ID for the CPU based on Pkg_ID, Core_ID, SMT_ID
+ *
+ * The caller must make sure core_id < nr_cores and smt_id < nr_threads.
+ */
+static inline apic_id_t apicid_from_topo_ids_epyc(unsigned smp_cores,
+                                                  unsigned smp_threads,
+                                                  const X86CPUTopoInfo *topo)
+{
+    if (smp_threads - 1)
+        return ((topo->pkg_id << PKG_OFFSET_EPYC) | (topo->die_id << DIE_OFFSET_EPYC) |
+                (topo->ccx_id << CCX_OFFSET_EPYC) | (topo->core_id << 1) | (topo->smt_id));
+    else
+        return ((topo->pkg_id << PKG_OFFSET_EPYC) | (topo->die_id << DIE_OFFSET_EPYC) |
+                (topo->ccx_id << CCX_OFFSET_EPYC) | (topo->core_id));
+}
+
+/* Make APIC ID for the CPU 'cpu_index'
+ *
+ * 'cpu_index' is a sequential, contiguous ID for the CPU.
+ */
+static inline apic_id_t x86_apicid_from_cpu_idx_epyc(unsigned numa_nodes,
+		                                     unsigned smp_sockets,
+                                                     unsigned smp_cores,
+                                                     unsigned smp_threads,
+                                                     unsigned cpu_index)
+{
+    X86CPUTopoInfo topo;
+    x86_topo_ids_from_idx_epyc(numa_nodes, smp_sockets, smp_cores, smp_threads,
+		               cpu_index, &topo);
+    return apicid_from_topo_ids_epyc(smp_cores, smp_threads, &topo);
+}
+
 #endif /* HW_I386_TOPOLOGY_H */
-- 
2.20.1

[Qemu-devel] [RFC PATCH 3/5] i386: Use topology functions from topology.h

[Moger, Babu]

Use the functions defined in topology.h and remove the old code.

Signed-off-by: Babu Moger <babu.moger@amd.com>
---
 target/i386/cpu.c | 146 +++++++++-------------------------------------
 1 file changed, 27 insertions(+), 119 deletions(-)

diff --git a/target/i386/cpu.c b/target/i386/cpu.c
index 19751e37a7..be4583068c 100644
--- a/target/i386/cpu.c
+++ b/target/i386/cpu.c
@@ -28,6 +28,7 @@
 #include "sysemu/kvm.h"
 #include "sysemu/hvf.h"
 #include "sysemu/cpus.h"
+#include "sysemu/numa.h"
 #include "kvm_i386.h"
 #include "sev_i386.h"
 
@@ -338,64 +339,8 @@ static void encode_cache_cpuid80000006(CPUCacheInfo *l2,
     }
 }
 
-/*
- * Definitions used for building CPUID Leaf 0x8000001D and 0x8000001E
- * Please refer to the AMD64 Architecture Programmer’s Manual Volume 3.
- * Define the constants to build the cpu topology. Right now, TOPOEXT
- * feature is enabled only on EPYC. So, these constants are based on
- * EPYC supported configurations. We may need to handle the cases if
- * these values change in future.
- */
-/* Maximum core complexes in a node */
-#define MAX_CCX 2
-/* Maximum cores in a core complex */
-#define MAX_CORES_IN_CCX 4
-/* Maximum cores in a node */
-#define MAX_CORES_IN_NODE 8
-/* Maximum nodes in a socket */
-#define MAX_NODES_PER_SOCKET 4
-
-/*
- * Figure out the number of nodes required to build this config.
- * Max cores in a node is 8
- */
-static int nodes_in_socket(int nr_cores)
-{
-    int nodes;
-
-    nodes = DIV_ROUND_UP(nr_cores, MAX_CORES_IN_NODE);
-
-   /* Hardware does not support config with 3 nodes, return 4 in that case */
-    return (nodes == 3) ? 4 : nodes;
-}
-
-/*
- * Decide the number of cores in a core complex with the given nr_cores using
- * following set constants MAX_CCX, MAX_CORES_IN_CCX, MAX_CORES_IN_NODE and
- * MAX_NODES_PER_SOCKET. Maintain symmetry as much as possible
- * L3 cache is shared across all cores in a core complex. So, this will also
- * tell us how many cores are sharing the L3 cache.
- */
-static int cores_in_core_complex(int nr_cores)
-{
-    int nodes;
-
-    /* Check if we can fit all the cores in one core complex */
-    if (nr_cores <= MAX_CORES_IN_CCX) {
-        return nr_cores;
-    }
-    /* Get the number of nodes required to build this config */
-    nodes = nodes_in_socket(nr_cores);
-
-    /*
-     * Divide the cores accros all the core complexes
-     * Return rounded up value
-     */
-    return DIV_ROUND_UP(nr_cores, nodes * MAX_CCX);
-}
-
 /* Encode cache info for CPUID[8000001D] */
-static void encode_cache_cpuid8000001d(CPUCacheInfo *cache, CPUState *cs,
+static void encode_cache_cpuid8000001d(CPUCacheInfo *cache, MachineState *ms,
                                 uint32_t *eax, uint32_t *ebx,
                                 uint32_t *ecx, uint32_t *edx)
 {
@@ -408,10 +353,10 @@ static void encode_cache_cpuid8000001d(CPUCacheInfo *cache, CPUState *cs,
 
     /* L3 is shared among multiple cores */
     if (cache->level == 3) {
-        l3_cores = cores_in_core_complex(cs->nr_cores);
-        *eax |= ((l3_cores * cs->nr_threads) - 1) << 14;
+        l3_cores = cores_in_ccx(nb_numa_nodes, ms->smp.sockets, ms->smp.cores);
+        *eax |= ((l3_cores * ms->smp.threads) - 1) << 14;
     } else {
-        *eax |= ((cs->nr_threads - 1) << 14);
+        *eax |= ((ms->smp.threads - 1) << 14);
     }
 
     assert(cache->line_size > 0);
@@ -431,55 +376,19 @@ static void encode_cache_cpuid8000001d(CPUCacheInfo *cache, CPUState *cs,
            (cache->complex_indexing ? CACHE_COMPLEX_IDX : 0);
 }
 
-/* Data structure to hold the configuration info for a given core index */
-struct core_topology {
-    /* core complex id of the current core index */
-    int ccx_id;
-    /*
-     * Adjusted core index for this core in the topology
-     * This can be 0,1,2,3 with max 4 cores in a core complex
-     */
-    int core_id;
-    /* Node id for this core index */
-    int node_id;
-    /* Number of nodes in this config */
-    int num_nodes;
-};
-
-/*
- * Build the configuration closely match the EPYC hardware. Using the EPYC
- * hardware configuration values (MAX_CCX, MAX_CORES_IN_CCX, MAX_CORES_IN_NODE)
- * right now. This could change in future.
- * nr_cores : Total number of cores in the config
- * core_id  : Core index of the current CPU
- * topo     : Data structure to hold all the config info for this core index
- */
-static void build_core_topology(int nr_cores, int core_id,
-                                struct core_topology *topo)
-{
-    int nodes, cores_in_ccx;
-
-    /* First get the number of nodes required */
-    nodes = nodes_in_socket(nr_cores);
-
-    cores_in_ccx = cores_in_core_complex(nr_cores);
-
-    topo->node_id = core_id / (cores_in_ccx * MAX_CCX);
-    topo->ccx_id = (core_id % (cores_in_ccx * MAX_CCX)) / cores_in_ccx;
-    topo->core_id = core_id % cores_in_ccx;
-    topo->num_nodes = nodes;
-}
-
 /* Encode cache info for CPUID[8000001E] */
-static void encode_topo_cpuid8000001e(CPUState *cs, X86CPU *cpu,
+static void encode_topo_cpuid8000001e(MachineState *ms, X86CPU *cpu,
                                        uint32_t *eax, uint32_t *ebx,
                                        uint32_t *ecx, uint32_t *edx)
 {
-    struct core_topology topo = {0};
-    unsigned long nodes;
-    int shift;
+    X86CPUTopoInfo topo = {0};
+    unsigned long dies, temp_dies, shift;
+    CPUX86State *env = &cpu->env;
+
+    dies = dies_in_socket(nb_numa_nodes, ms->smp.sockets, ms->smp.cores);
+    x86_topo_ids_from_apicid_epyc(cpu->apic_id, env->nr_dies, ms->smp.cores,
+		                  ms->smp.threads, &topo);
 
-    build_core_topology(cs->nr_cores, cpu->core_id, &topo);
     *eax = cpu->apic_id;
     /*
      * CPUID_Fn8000001E_EBX
@@ -496,11 +405,11 @@ static void encode_topo_cpuid8000001e(CPUState *cs, X86CPU *cpu,
      *             3 Core complex id
      *           1:0 Core id
      */
-    if (cs->nr_threads - 1) {
-        *ebx = ((cs->nr_threads - 1) << 8) | (topo.node_id << 3) |
+    if (ms->smp.threads - 1) {
+        *ebx = ((ms->smp.threads - 1) << 8) | (topo.die_id << 3) |
                 (topo.ccx_id << 2) | topo.core_id;
     } else {
-        *ebx = (topo.node_id << 4) | (topo.ccx_id << 3) | topo.core_id;
+        *ebx = (topo.die_id << 4) | (topo.ccx_id << 3) | topo.core_id;
     }
     /*
      * CPUID_Fn8000001E_ECX
@@ -510,9 +419,8 @@ static void encode_topo_cpuid8000001e(CPUState *cs, X86CPU *cpu,
      *         2  Socket id
      *       1:0  Node id
      */
-    if (topo.num_nodes <= 4) {
-        *ecx = ((topo.num_nodes - 1) << 8) | (cpu->socket_id << 2) |
-                topo.node_id;
+    if (dies <= 4) {
+        *ecx = ((dies - 1) << 8) | (topo.pkg_id << 2) | topo.die_id;
     } else {
         /*
          * Node id fix up. Actual hardware supports up to 4 nodes. But with
@@ -527,10 +435,9 @@ static void encode_topo_cpuid8000001e(CPUState *cs, X86CPU *cpu,
          * number of nodes. find_last_bit returns last set bit(0 based). Left
          * shift(+1) the socket id to represent all the nodes.
          */
-        nodes = topo.num_nodes - 1;
-        shift = find_last_bit(&nodes, 8);
-        *ecx = ((topo.num_nodes - 1) << 8) | (cpu->socket_id << (shift + 1)) |
-                topo.node_id;
+        temp_dies = dies - 1;
+        shift = find_last_bit(&temp_dies, 8);
+        *ecx = ((dies - 1) << 8) | (topo.pkg_id << (shift + 1)) | topo.die_id;
     }
     *edx = 0;
 }
@@ -4169,6 +4076,7 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t index, uint32_t count,
                    uint32_t *eax, uint32_t *ebx,
                    uint32_t *ecx, uint32_t *edx)
 {
+    MachineState *ms = MACHINE(qdev_get_machine());
     X86CPU *cpu = env_archcpu(env);
     CPUState *cs = env_cpu(env);
     uint32_t die_offset;
@@ -4584,19 +4492,19 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t index, uint32_t count,
         }
         switch (count) {
         case 0: /* L1 dcache info */
-            encode_cache_cpuid8000001d(env->cache_info_amd.l1d_cache, cs,
+            encode_cache_cpuid8000001d(env->cache_info_amd.l1d_cache, ms,
                                        eax, ebx, ecx, edx);
             break;
         case 1: /* L1 icache info */
-            encode_cache_cpuid8000001d(env->cache_info_amd.l1i_cache, cs,
+            encode_cache_cpuid8000001d(env->cache_info_amd.l1i_cache, ms,
                                        eax, ebx, ecx, edx);
             break;
         case 2: /* L2 cache info */
-            encode_cache_cpuid8000001d(env->cache_info_amd.l2_cache, cs,
+            encode_cache_cpuid8000001d(env->cache_info_amd.l2_cache, ms,
                                        eax, ebx, ecx, edx);
             break;
         case 3: /* L3 cache info */
-            encode_cache_cpuid8000001d(env->cache_info_amd.l3_cache, cs,
+            encode_cache_cpuid8000001d(env->cache_info_amd.l3_cache, ms,
                                        eax, ebx, ecx, edx);
             break;
         default: /* end of info */
@@ -4606,7 +4514,7 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t index, uint32_t count,
         break;
     case 0x8000001E:
         assert(cpu->core_id <= 255);
-        encode_topo_cpuid8000001e(cs, cpu,
+        encode_topo_cpuid8000001e(ms, cpu,
                                   eax, ebx, ecx, edx);
         break;
     case 0xC0000000:
-- 
2.20.1

[Qemu-devel] [RFC PATCH 4/5] hw/i386: Generate apicid based on cpu_type

[Moger, Babu]

Check the cpu_type before calling the apicid functions
from topology.h.

Signed-off-by: Babu Moger <babu.moger@amd.com>
---
 hw/i386/pc.c | 81 +++++++++++++++++++++++++++++++++++++++++++++-------
 1 file changed, 70 insertions(+), 11 deletions(-)

diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index ef39463fd5..dad55c940f 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -947,6 +947,36 @@ static uint32_t x86_cpu_apic_id_from_index(PCMachineState *pcms,
     }
 }
 
+/* Calculates initial APIC ID for a specific CPU index
+ *
+ * Currently we need to be able to calculate the APIC ID from the CPU index
+ * alone (without requiring a CPU object), as the QEMU<->Seabios interfaces have
+ * no concept of "CPU index", and the NUMA tables on fw_cfg need the APIC ID of
+ * all CPUs up to max_cpus.
+ */
+static uint32_t x86_cpu_apic_id_from_index_epyc(PCMachineState *pcms,
+                                                unsigned int cpu_index)
+{
+    MachineState *ms = MACHINE(pcms);
+    PCMachineClass *pcmc = PC_MACHINE_GET_CLASS(pcms);
+    uint32_t correct_id;
+    static bool warned;
+
+    correct_id = x86_apicid_from_cpu_idx_epyc(nb_numa_nodes, ms->smp.sockets,
+		                              ms->smp.cores, ms->smp.threads,
+					      cpu_index);
+    if (pcmc->compat_apic_id_mode) {
+        if (cpu_index != correct_id && !warned && !qtest_enabled()) {
+            error_report("APIC IDs set in compatibility mode, "
+                         "CPU topology won't match the configuration");
+            warned = true;
+        }
+        return cpu_index;
+    } else {
+        return correct_id;
+    }
+}
+
 static void pc_build_smbios(PCMachineState *pcms)
 {
     uint8_t *smbios_tables, *smbios_anchor;
@@ -1619,7 +1649,7 @@ void pc_smp_parse(MachineState *ms, QemuOpts *opts)
 void pc_hot_add_cpu(MachineState *ms, const int64_t id, Error **errp)
 {
     PCMachineState *pcms = PC_MACHINE(ms);
-    int64_t apic_id = x86_cpu_apic_id_from_index(pcms, id);
+    int64_t apic_id;
     Error *local_err = NULL;
 
     if (id < 0) {
@@ -1627,6 +1657,11 @@ void pc_hot_add_cpu(MachineState *ms, const int64_t id, Error **errp)
         return;
     }
 
+    if(!strncmp(ms->cpu_type, "EPYC", 4))
+        apic_id = x86_cpu_apic_id_from_index_epyc(pcms, id);
+    else
+        apic_id = x86_cpu_apic_id_from_index(pcms, id);
+
     if (apic_id >= ACPI_CPU_HOTPLUG_ID_LIMIT) {
         error_setg(errp, "Unable to add CPU: %" PRIi64
                    ", resulting APIC ID (%" PRIi64 ") is too large",
@@ -1658,8 +1693,13 @@ void pc_cpus_init(PCMachineState *pcms)
      *
      * This is used for FW_CFG_MAX_CPUS. See comments on bochs_bios_init().
      */
-    pcms->apic_id_limit = x86_cpu_apic_id_from_index(pcms,
-                                                     ms->smp.max_cpus - 1) + 1;
+    if(!strncmp(ms->cpu_type, "EPYC", 4))
+        pcms->apic_id_limit = x86_cpu_apic_id_from_index_epyc(pcms,
+                                                              ms->smp.max_cpus - 1) + 1;
+    else
+        pcms->apic_id_limit = x86_cpu_apic_id_from_index(pcms,
+                                                         ms->smp.max_cpus - 1) + 1;
+
     possible_cpus = mc->possible_cpu_arch_ids(ms);
     for (i = 0; i < ms->smp.cpus; i++) {
         pc_new_cpu(pcms, possible_cpus->cpus[i].arch_id, &error_fatal);
@@ -2387,6 +2427,7 @@ static void pc_cpu_pre_plug(HotplugHandler *hotplug_dev,
     PCMachineState *pcms = PC_MACHINE(hotplug_dev);
     unsigned int smp_cores = ms->smp.cores;
     unsigned int smp_threads = ms->smp.threads;
+    unsigned int smp_sockets = ms->smp.sockets;
 
     if(!object_dynamic_cast(OBJECT(cpu), ms->cpu_type)) {
         error_setg(errp, "Invalid CPU type, expected cpu type: '%s'",
@@ -2437,16 +2478,26 @@ static void pc_cpu_pre_plug(HotplugHandler *hotplug_dev,
         topo.die_id = cpu->die_id;
         topo.core_id = cpu->core_id;
         topo.smt_id = cpu->thread_id;
-        cpu->apic_id = apicid_from_topo_ids(pcms->smp_dies, smp_cores,
-                                            smp_threads, &topo);
+	if (!strncmp(ms->cpu_type, "EPYC", 4)) {
+            x86_topo_ids_from_idx_epyc(nb_numa_nodes, smp_sockets, smp_cores,
+                                       smp_threads, idx, &topo);
+            cpu->apic_id = apicid_from_topo_ids_epyc(smp_cores, smp_threads,
+                                                     &topo);
+	} else
+            cpu->apic_id = apicid_from_topo_ids(pcms->smp_dies, smp_cores,
+                                                smp_threads, &topo);
     }
 
     cpu_slot = pc_find_cpu_slot(MACHINE(pcms), cpu->apic_id, &idx);
     if (!cpu_slot) {
         MachineState *ms = MACHINE(pcms);
 
-        x86_topo_ids_from_apicid(cpu->apic_id, pcms->smp_dies,
-                                 smp_cores, smp_threads, &topo);
+        if(!strncmp(ms->cpu_type, "EPYC", 4))
+            x86_topo_ids_from_apicid_epyc(cpu->apic_id, pcms->smp_dies,
+                                          smp_cores, smp_threads, &topo);
+        else
+            x86_topo_ids_from_apicid(cpu->apic_id, pcms->smp_dies,
+                                     smp_cores, smp_threads, &topo);
         error_setg(errp,
             "Invalid CPU [socket: %u, die: %u, core: %u, thread: %u] with"
             " APIC ID %" PRIu32 ", valid index range 0:%d",
@@ -2874,10 +2925,18 @@ static const CPUArchIdList *pc_possible_cpu_arch_ids(MachineState *ms)
 
         ms->possible_cpus->cpus[i].type = ms->cpu_type;
         ms->possible_cpus->cpus[i].vcpus_count = 1;
-        ms->possible_cpus->cpus[i].arch_id = x86_cpu_apic_id_from_index(pcms, i);
-        x86_topo_ids_from_apicid(ms->possible_cpus->cpus[i].arch_id,
-                                 pcms->smp_dies, ms->smp.cores,
-                                 ms->smp.threads, &topo);
+	if(!strncmp(ms->cpu_type, "EPYC", 4)) {
+            ms->possible_cpus->cpus[i].arch_id =
+                            x86_cpu_apic_id_from_index_epyc(pcms, i);
+            x86_topo_ids_from_apicid_epyc(ms->possible_cpus->cpus[i].arch_id,
+                                          pcms->smp_dies, ms->smp.cores,
+					  ms->smp.threads, &topo);
+	} else {
+            ms->possible_cpus->cpus[i].arch_id = x86_cpu_apic_id_from_index(pcms, i);
+            x86_topo_ids_from_apicid(ms->possible_cpus->cpus[i].arch_id,
+                                     pcms->smp_dies, ms->smp.cores,
+                                     ms->smp.threads, &topo);
+	}
         ms->possible_cpus->cpus[i].props.has_socket_id = true;
         ms->possible_cpus->cpus[i].props.socket_id = topo.pkg_id;
         ms->possible_cpus->cpus[i].props.has_die_id = true;
-- 
2.20.1

[Qemu-devel] [RFC PATCH 5/5] i386: Fix pkg_id offset EPYC

[Moger, Babu]

Per Processor Programming Reference (PPR) for AMD Family 17h Models,
the pkg_id offset in apicid is 6. Fix the offset based on EPYC models.

Signed-off-by: Babu Moger <babu.moger@amd.com>
---
 target/i386/cpu.c | 14 +++++++++-----
 1 file changed, 9 insertions(+), 5 deletions(-)

diff --git a/target/i386/cpu.c b/target/i386/cpu.c
index be4583068c..235496a9c1 100644
--- a/target/i386/cpu.c
+++ b/target/i386/cpu.c
@@ -4079,7 +4079,7 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t index, uint32_t count,
     MachineState *ms = MACHINE(qdev_get_machine());
     X86CPU *cpu = env_archcpu(env);
     CPUState *cs = env_cpu(env);
-    uint32_t die_offset;
+    uint32_t die_offset, pkg_offset;
     uint32_t limit;
     uint32_t signature[3];
 
@@ -4102,6 +4102,12 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t index, uint32_t count,
         index = env->cpuid_level;
     }
 
+    if (!strncmp(ms->cpu_type, "EPYC", 4))
+        pkg_offset = PKG_OFFSET_EPYC;
+    else
+        pkg_offset = apicid_pkg_offset(env->nr_dies, cs->nr_cores,
+                                       cs->nr_threads);
+
     switch(index) {
     case 0:
         *eax = env->cpuid_level;
@@ -4260,8 +4266,7 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t index, uint32_t count,
             *ecx |= CPUID_TOPOLOGY_LEVEL_SMT;
             break;
         case 1:
-            *eax = apicid_pkg_offset(env->nr_dies,
-                                     cs->nr_cores, cs->nr_threads);
+            *eax = pkg_offset;
             *ebx = cs->nr_cores * cs->nr_threads;
             *ecx |= CPUID_TOPOLOGY_LEVEL_CORE;
             break;
@@ -4297,8 +4302,7 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t index, uint32_t count,
             *ecx |= CPUID_TOPOLOGY_LEVEL_CORE;
             break;
         case 2:
-            *eax = apicid_pkg_offset(env->nr_dies, cs->nr_cores,
-                                                   cs->nr_threads);
+            *eax = pkg_offset;
             *ebx = env->nr_dies * cs->nr_cores * cs->nr_threads;
             *ecx |= CPUID_TOPOLOGY_LEVEL_DIE;
             break;
-- 
2.20.1

Re: [Qemu-devel] [RFC PATCH 4/5] hw/i386: Generate apicid based on cpu_type

[Eduardo Habkost]

Thanks for the patches.

I still haven't looked closely at all patches in the series, but
patches 1-3 seem good on the first look.  A few comments on this
one:

On Wed, Jul 31, 2019 at 11:20:50PM +0000, Moger, Babu wrote:
> Check the cpu_type before calling the apicid functions
> from topology.h.
> 
> Signed-off-by: Babu Moger <babu.moger@amd.com>
> ---
[...]
> @@ -2437,16 +2478,26 @@ static void pc_cpu_pre_plug(HotplugHandler *hotplug_dev,
>          topo.die_id = cpu->die_id;
>          topo.core_id = cpu->core_id;
>          topo.smt_id = cpu->thread_id;
> -        cpu->apic_id = apicid_from_topo_ids(pcms->smp_dies, smp_cores,
> -                                            smp_threads, &topo);
> +	if (!strncmp(ms->cpu_type, "EPYC", 4)) {

Please don't add semantics to the CPU type name.  If you want
some behavior to be configurable per CPU type, please do it at
the X86CPUDefinition struct.

In this specific case, maybe the new APIC ID calculation code
could
be conditional on:
  (vendor == AMD) && (env->features[...] & TOPOEXT).

Also, we must keep compatibility with the old APIC ID calculation
code on older machine types.  We need a compatibility flag to
enable the existing APIC ID calculation.


> +            x86_topo_ids_from_idx_epyc(nb_numa_nodes, smp_sockets, smp_cores,
> +                                       smp_threads, idx, &topo);
> +            cpu->apic_id = apicid_from_topo_ids_epyc(smp_cores, smp_threads,
> +                                                     &topo);
> +	} else

There's a tab character here.  Please use spaces instead of tabs.

> +            cpu->apic_id = apicid_from_topo_ids(pcms->smp_dies, smp_cores,
> +                                                smp_threads, &topo);

I see you are duplicating very similar logic in 3 different
places, to call apicid_from_topo_ids() and
x86_topo_ids_from_apicid().

Also, apicid_from_topo_ids() and x86_topo_ids_from_apicid() have very generic
names, and people could call them expecting them to work for every CPU model
(which they don't).  This makes the topology API very easy to misuse.

Why don't we make the existing generic
apicid_from_topo_ids()/x86_topo_ids_from_apicid() functions work
on all cases?  If they need additional input to handle EPYC and
call EPYC-specific functions, we can make them get additional
arguments.  This way we'll be sure that we'll never call the
wrong implementation by accident.

This might make the list of arguments for
x86_topo_ids_from_apicid() and apicid_from_topo_ids() become
large.  We can address this by making them get a CpuTopology
argument.


In other words, the API could look like this:


static inline apic_id_t apicid_from_topo_ids(const X86CPUTopology *topo,
                                             const X86CPUTopologyIds *ids)
{
    if (topo->epyc_mode) {
        return apicid_from_topo_ids_epyc(topo, ids);
    }

    /* existing QEMU 4.1 logic: */
    return (ids->pkg_id  << apicid_pkg_offset(topo)) |
           (ids->die_id  << apicid_die_offset(topo)) |
           (ids->core_id << apicid_core_offset(topo)) |
           ids->smt_id;
}

static inline void x86_topo_ids_from_apicid(apic_id_t apicid,
                                            const X86CPUTopology *topo,
                                            X86CPUTopologyIds *ids)
{
    if (topo->epyc_mode) {
        x86_topo_ids_from_apicid_epyc(apicid, topo, ids);
        return;
    }

    /* existing QEMU 4.1 logic: */
    ids->smt_id =
            apicid &
            ~(0xFFFFFFFFUL << apicid_smt_width(topo));
    ids->core_id =
            (apicid >> apicid_core_offset(topo)) &
            ~(0xFFFFFFFFUL << apicid_core_width(topo));
    ids->die_id =
            (apicid >> apicid_die_offset(topo)) &
            ~(0xFFFFFFFFUL << apicid_die_width(topo));
    ids->pkg_id = apicid >> apicid_pkg_offset(topo);
}

 
>      }
>  
>      cpu_slot = pc_find_cpu_slot(MACHINE(pcms), cpu->apic_id, &idx);
>      if (!cpu_slot) {
>          MachineState *ms = MACHINE(pcms);
>  
> -        x86_topo_ids_from_apicid(cpu->apic_id, pcms->smp_dies,
> -                                 smp_cores, smp_threads, &topo);
> +        if(!strncmp(ms->cpu_type, "EPYC", 4))
> +            x86_topo_ids_from_apicid_epyc(cpu->apic_id, pcms->smp_dies,
> +                                          smp_cores, smp_threads, &topo);
> +        else
> +            x86_topo_ids_from_apicid(cpu->apic_id, pcms->smp_dies,
> +                                     smp_cores, smp_threads, &topo);
>          error_setg(errp,
>              "Invalid CPU [socket: %u, die: %u, core: %u, thread: %u] with"
>              " APIC ID %" PRIu32 ", valid index range 0:%d",
> @@ -2874,10 +2925,18 @@ static const CPUArchIdList *pc_possible_cpu_arch_ids(MachineState *ms)
>  
>          ms->possible_cpus->cpus[i].type = ms->cpu_type;
>          ms->possible_cpus->cpus[i].vcpus_count = 1;
> -        ms->possible_cpus->cpus[i].arch_id = x86_cpu_apic_id_from_index(pcms, i);
> -        x86_topo_ids_from_apicid(ms->possible_cpus->cpus[i].arch_id,
> -                                 pcms->smp_dies, ms->smp.cores,
> -                                 ms->smp.threads, &topo);
> +	if(!strncmp(ms->cpu_type, "EPYC", 4)) {
> +            ms->possible_cpus->cpus[i].arch_id =
> +                            x86_cpu_apic_id_from_index_epyc(pcms, i);
> +            x86_topo_ids_from_apicid_epyc(ms->possible_cpus->cpus[i].arch_id,
> +                                          pcms->smp_dies, ms->smp.cores,
> +					  ms->smp.threads, &topo);
> +	} else {
> +            ms->possible_cpus->cpus[i].arch_id = x86_cpu_apic_id_from_index(pcms, i);
> +            x86_topo_ids_from_apicid(ms->possible_cpus->cpus[i].arch_id,
> +                                     pcms->smp_dies, ms->smp.cores,
> +                                     ms->smp.threads, &topo);
> +	}
>          ms->possible_cpus->cpus[i].props.has_socket_id = true;
>          ms->possible_cpus->cpus[i].props.socket_id = topo.pkg_id;
>          ms->possible_cpus->cpus[i].props.has_die_id = true;
> -- 
> 2.20.1
> 

-- 
Eduardo

Re: [Qemu-devel] [RFC PATCH 4/5] hw/i386: Generate apicid based on cpu_type

[Moger, Babu]

Hi Eduardo,
  Thanks for the quick comments. I will look into your comments closely
and will let you know if I have questions.

> -----Original Message-----
> From: Eduardo Habkost <ehabkost@redhat.com>
> Sent: Thursday, August 1, 2019 2:29 PM
> To: Moger, Babu <Babu.Moger@amd.com>
> Cc: marcel.apfelbaum@gmail.com; mst@redhat.com; pbonzini@redhat.com;
> rth@twiddle.net; imammedo@redhat.com; qemu-devel@nongnu.org
> Subject: Re: [RFC PATCH 4/5] hw/i386: Generate apicid based on cpu_type
> 
> Thanks for the patches.
> 
> I still haven't looked closely at all patches in the series, but
> patches 1-3 seem good on the first look.  A few comments on this
> one:
> 
> On Wed, Jul 31, 2019 at 11:20:50PM +0000, Moger, Babu wrote:
> > Check the cpu_type before calling the apicid functions
> > from topology.h.
> >
> > Signed-off-by: Babu Moger <babu.moger@amd.com>
> > ---
> [...]
> > @@ -2437,16 +2478,26 @@ static void pc_cpu_pre_plug(HotplugHandler
> *hotplug_dev,
> >          topo.die_id = cpu->die_id;
> >          topo.core_id = cpu->core_id;
> >          topo.smt_id = cpu->thread_id;
> > -        cpu->apic_id = apicid_from_topo_ids(pcms->smp_dies, smp_cores,
> > -                                            smp_threads, &topo);
> > +	if (!strncmp(ms->cpu_type, "EPYC", 4)) {
> 
> Please don't add semantics to the CPU type name.  If you want
> some behavior to be configurable per CPU type, please do it at
> the X86CPUDefinition struct.
> 
> In this specific case, maybe the new APIC ID calculation code
> could
> be conditional on:
>   (vendor == AMD) && (env->features[...] & TOPOEXT).
> 
> Also, we must keep compatibility with the old APIC ID calculation
> code on older machine types.  We need a compatibility flag to
> enable the existing APIC ID calculation.
> 
> 
> > +            x86_topo_ids_from_idx_epyc(nb_numa_nodes, smp_sockets,
> smp_cores,
> > +                                       smp_threads, idx, &topo);
> > +            cpu->apic_id = apicid_from_topo_ids_epyc(smp_cores,
> smp_threads,
> > +                                                     &topo);
> > +	} else
> 
> There's a tab character here.  Please use spaces instead of tabs.
> 
> > +            cpu->apic_id = apicid_from_topo_ids(pcms->smp_dies, smp_cores,
> > +                                                smp_threads, &topo);
> 
> I see you are duplicating very similar logic in 3 different
> places, to call apicid_from_topo_ids() and
> x86_topo_ids_from_apicid().
> 
> Also, apicid_from_topo_ids() and x86_topo_ids_from_apicid() have very
> generic
> names, and people could call them expecting them to work for every CPU
> model
> (which they don't).  This makes the topology API very easy to misuse.
> 
> Why don't we make the existing generic
> apicid_from_topo_ids()/x86_topo_ids_from_apicid() functions work
> on all cases?  If they need additional input to handle EPYC and
> call EPYC-specific functions, we can make them get additional
> arguments.  This way we'll be sure that we'll never call the
> wrong implementation by accident.
> 
> This might make the list of arguments for
> x86_topo_ids_from_apicid() and apicid_from_topo_ids() become
> large.  We can address this by making them get a CpuTopology
> argument.
> 
> 
> In other words, the API could look like this:
> 
> 
> static inline apic_id_t apicid_from_topo_ids(const X86CPUTopology *topo,
>                                              const X86CPUTopologyIds *ids)
> {
>     if (topo->epyc_mode) {
>         return apicid_from_topo_ids_epyc(topo, ids);
>     }
> 
>     /* existing QEMU 4.1 logic: */
>     return (ids->pkg_id  << apicid_pkg_offset(topo)) |
>            (ids->die_id  << apicid_die_offset(topo)) |
>            (ids->core_id << apicid_core_offset(topo)) |
>            ids->smt_id;
> }
> 
> static inline void x86_topo_ids_from_apicid(apic_id_t apicid,
>                                             const X86CPUTopology *topo,
>                                             X86CPUTopologyIds *ids)
> {
>     if (topo->epyc_mode) {
>         x86_topo_ids_from_apicid_epyc(apicid, topo, ids);
>         return;
>     }
> 
>     /* existing QEMU 4.1 logic: */
>     ids->smt_id =
>             apicid &
>             ~(0xFFFFFFFFUL << apicid_smt_width(topo));
>     ids->core_id =
>             (apicid >> apicid_core_offset(topo)) &
>             ~(0xFFFFFFFFUL << apicid_core_width(topo));
>     ids->die_id =
>             (apicid >> apicid_die_offset(topo)) &
>             ~(0xFFFFFFFFUL << apicid_die_width(topo));
>     ids->pkg_id = apicid >> apicid_pkg_offset(topo);
> }
> 
> 
> >      }
> >
> >      cpu_slot = pc_find_cpu_slot(MACHINE(pcms), cpu->apic_id, &idx);
> >      if (!cpu_slot) {
> >          MachineState *ms = MACHINE(pcms);
> >
> > -        x86_topo_ids_from_apicid(cpu->apic_id, pcms->smp_dies,
> > -                                 smp_cores, smp_threads, &topo);
> > +        if(!strncmp(ms->cpu_type, "EPYC", 4))
> > +            x86_topo_ids_from_apicid_epyc(cpu->apic_id, pcms->smp_dies,
> > +                                          smp_cores, smp_threads, &topo);
> > +        else
> > +            x86_topo_ids_from_apicid(cpu->apic_id, pcms->smp_dies,
> > +                                     smp_cores, smp_threads, &topo);
> >          error_setg(errp,
> >              "Invalid CPU [socket: %u, die: %u, core: %u, thread: %u] with"
> >              " APIC ID %" PRIu32 ", valid index range 0:%d",
> > @@ -2874,10 +2925,18 @@ static const CPUArchIdList
> *pc_possible_cpu_arch_ids(MachineState *ms)
> >
> >          ms->possible_cpus->cpus[i].type = ms->cpu_type;
> >          ms->possible_cpus->cpus[i].vcpus_count = 1;
> > -        ms->possible_cpus->cpus[i].arch_id =
> x86_cpu_apic_id_from_index(pcms, i);
> > -        x86_topo_ids_from_apicid(ms->possible_cpus->cpus[i].arch_id,
> > -                                 pcms->smp_dies, ms->smp.cores,
> > -                                 ms->smp.threads, &topo);
> > +	if(!strncmp(ms->cpu_type, "EPYC", 4)) {
> > +            ms->possible_cpus->cpus[i].arch_id =
> > +                            x86_cpu_apic_id_from_index_epyc(pcms, i);
> > +            x86_topo_ids_from_apicid_epyc(ms->possible_cpus-
> >cpus[i].arch_id,
> > +                                          pcms->smp_dies, ms->smp.cores,
> > +					  ms->smp.threads, &topo);
> > +	} else {
> > +            ms->possible_cpus->cpus[i].arch_id =
> x86_cpu_apic_id_from_index(pcms, i);
> > +            x86_topo_ids_from_apicid(ms->possible_cpus->cpus[i].arch_id,
> > +                                     pcms->smp_dies, ms->smp.cores,
> > +                                     ms->smp.threads, &topo);
> > +	}
> >          ms->possible_cpus->cpus[i].props.has_socket_id = true;
> >          ms->possible_cpus->cpus[i].props.socket_id = topo.pkg_id;
> >          ms->possible_cpus->cpus[i].props.has_die_id = true;
> > --
> > 2.20.1
> >
> 
> --
> Eduardo