[Qemu-devel] [PATCH v2 0/5] Introduce cpu die topology and enable CPUID.1F for i386

[Qemu-devel] [PATCH v2 0/5] Introduce cpu die topology and enable CPUID.1F for i386

[Like Xu]

Multi-chip packaging technology allows integration of multi-cores in one die
and multi-dies in one single package, for example Intel CLX-AP or AMD EPYC.

This kind of integration can be enabled by high-performance, heterogeneous,
multi-dies interconnect technology, providing a more cost-effective manner. 
QEMU and guests may take advantages of multi-dies host for such as guest
placing or energy efficiency management...

This patch series extend the CPU topology to the socket/dies/core/thread model,
allowing the setting of dies number per one socket on -smp qemu command. For
i386, it upgrades APIC_IDs generation and reversion functions with a new exposed
leaf called CPUID.1F, which is a preferred superset to leaf 0BH. The CPUID.1F
spec is on https://software.intel.com/en-us/articles/intel-sdm, 3-190 Vol 2A.

E.g. we use -smp 4,dies=2,cores=2,threads=1 to run an MCP kvm-guest,
check raw cpuid data and the expected output from guest is following:
0x0000001f 0x00: eax=0x00000000 ebx=0x00000001 ecx=0x00000100 edx=0x00000002
0x0000001f 0x01: eax=0x00000001 ebx=0x00000002 ecx=0x00000201 edx=0x00000001
0x0000001f 0x02: eax=0x00000002 ebx=0x00000004 ecx=0x00000502 edx=0x00000003
0x0000001f 0x03: eax=0x00000000 ebx=0x00000000 ecx=0x00000003 edx=0x00000001

==changelog==

v2:

- Enable cpu die-level topolgy only for PCMachine and X86CPU
- Minimize cpuid.0.eax to the setting value actually used by guest
- Update cmd line -smps docs for die-level configurations
- Refactoring topo-bit tests for x86_apicid_from_cpu_idx() with nr_dies
- Based on "[PATCH v3 00/10] Refactor cpu topo into machine properties"
- Rebase to commit 2259637b95bef3116cc262459271de08e038cc66

v1: https://patchwork.kernel.org/cover/10876667/

Like Xu (5):
  target/i386: Add cpu die-level topology support for X86CPU
  i386/cpu: Consolidate die-id validity in smp context
  vl.c: Add -smp, dies=* command line support and update -smp doc
  i386/cpu: Update apicid parsing rules and topo-bit tests for dies
  target/i386: Add CPUID.1F generation support for multi-die PCMachine

 hmp.c                      |  3 ++
 hw/core/machine.c          | 12 +++++
 hw/i386/pc.c               | 52 +++++++++++++++++-----
 include/hw/i386/pc.h       |  2 +
 include/hw/i386/topology.h | 76 +++++++++++++++++++++++---------
 qapi/misc.json             |  6 ++-
 qemu-options.hx            | 17 ++++----
 target/i386/cpu.c          | 59 ++++++++++++++++++++++---
 target/i386/cpu.h          |  7 +++
 target/i386/kvm.c          | 30 ++++++++++++-
 tests/test-x86-cpuid.c     | 84 ++++++++++++++++++-----------------
 vl.c                       | 89 +++++++++++++++++++++++++++++++++++++-
 12 files changed, 347 insertions(+), 90 deletions(-)

-- 
2.21.0

[Qemu-devel] [PATCH v2 1/5] target/i386: Add cpu die-level topology support for X86CPU

[Like Xu]

The die-level as the first PC-specific cpu topology is added to the
leagcy cpu topology model which only covers sockets/cores/threads.

In the new model with die-level support, the total number of logical
processors (including offline) on board will be calculated as:

     #cpus = #sockets * #dies * #cores * #threads

and considering compatibility, the default value for #dies is 1.

A new set of die-related variables are added in smp context and the
CPUX86State.nr_dies is assigned in x86_cpu_initfn() from PCMachineState.

Signed-off-by: Like Xu <like.xu@linux.intel.com>
---
 hw/i386/pc.c               | 3 +++
 include/hw/i386/pc.h       | 2 ++
 include/hw/i386/topology.h | 2 ++
 qapi/misc.json             | 6 ++++--
 target/i386/cpu.c          | 9 +++++++++
 target/i386/cpu.h          | 3 +++
 6 files changed, 23 insertions(+), 2 deletions(-)

diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index 896c22e32e..83ab53c814 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -2341,6 +2341,7 @@ static void pc_cpu_pre_plug(HotplugHandler *hotplug_dev,
 
         topo.pkg_id = cpu->socket_id;
         topo.core_id = cpu->core_id;
+        topo.die_id = cpu->die_id;
         topo.smt_id = cpu->thread_id;
         cpu->apic_id = apicid_from_topo_ids(smp_cores, smp_threads, &topo);
     }
@@ -2692,6 +2693,8 @@ static const CPUArchIdList *pc_possible_cpu_arch_ids(MachineState *ms)
                                  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;
+        ms->possible_cpus->cpus[i].props.die_id = topo.die_id;
         ms->possible_cpus->cpus[i].props.has_core_id = true;
         ms->possible_cpus->cpus[i].props.core_id = topo.core_id;
         ms->possible_cpus->cpus[i].props.has_thread_id = true;
diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
index ce3c22951e..b5faf2ede9 100644
--- a/include/hw/i386/pc.h
+++ b/include/hw/i386/pc.h
@@ -24,6 +24,7 @@
  * PCMachineState:
  * @acpi_dev: link to ACPI PM device that performs ACPI hotplug handling
  * @boot_cpus: number of present VCPUs
+ * @smp_dies: number of dies per one package
  */
 struct PCMachineState {
     /*< private >*/
@@ -59,6 +60,7 @@ struct PCMachineState {
     bool apic_xrupt_override;
     unsigned apic_id_limit;
     uint16_t boot_cpus;
+    unsigned smp_dies;
 
     /* NUMA information: */
     uint64_t numa_nodes;
diff --git a/include/hw/i386/topology.h b/include/hw/i386/topology.h
index 1ebaee0f76..7f80498eb3 100644
--- a/include/hw/i386/topology.h
+++ b/include/hw/i386/topology.h
@@ -47,6 +47,7 @@ typedef uint32_t apic_id_t;
 
 typedef struct X86CPUTopoInfo {
     unsigned pkg_id;
+    unsigned die_id;
     unsigned core_id;
     unsigned smt_id;
 } X86CPUTopoInfo;
@@ -130,6 +131,7 @@ static inline void x86_topo_ids_from_apicid(apic_id_t apicid,
     topo->core_id = (apicid >> apicid_core_offset(nr_cores, nr_threads)) &
                    ~(0xFFFFFFFFUL << apicid_core_width(nr_cores, nr_threads));
     topo->pkg_id = apicid >> apicid_pkg_offset(nr_cores, nr_threads);
+    topo->die_id = -1;
 }
 
 /* Make APIC ID for the CPU 'cpu_index'
diff --git a/qapi/misc.json b/qapi/misc.json
index 8b3ca4fdd3..cd236c89b3 100644
--- a/qapi/misc.json
+++ b/qapi/misc.json
@@ -2924,10 +2924,11 @@
 #
 # @node-id: NUMA node ID the CPU belongs to
 # @socket-id: socket number within node/board the CPU belongs to
-# @core-id: core number within socket the CPU belongs to
+# @die-id: die number within node/board the CPU belongs to (Since 4.1)
+# @core-id: core number within die the CPU belongs to
 # @thread-id: thread number within core the CPU belongs to
 #
-# Note: currently there are 4 properties that could be present
+# Note: currently there are 5 properties that could be present
 # but management should be prepared to pass through other
 # properties with device_add command to allow for future
 # interface extension. This also requires the filed names to be kept in
@@ -2938,6 +2939,7 @@
 { 'struct': 'CpuInstanceProperties',
   'data': { '*node-id': 'int',
             '*socket-id': 'int',
+            '*die-id': 'int',
             '*core-id': 'int',
             '*thread-id': 'int'
   }
diff --git a/target/i386/cpu.c b/target/i386/cpu.c
index 9a93dd8be7..9bd35b4965 100644
--- a/target/i386/cpu.c
+++ b/target/i386/cpu.c
@@ -55,6 +55,7 @@
 #include "hw/xen/xen.h"
 #include "hw/i386/apic_internal.h"
 #include "hw/boards.h"
+#include "hw/i386/pc.h"
 #endif
 
 #include "disas/capstone.h"
@@ -5595,7 +5596,13 @@ static void x86_cpu_initfn(Object *obj)
     X86CPUClass *xcc = X86_CPU_GET_CLASS(obj);
     CPUX86State *env = &cpu->env;
     FeatureWord w;
+#ifndef CONFIG_USER_ONLY
+    MachineState *machine = MACHINE(qdev_get_machine());
+    PCMachineState *pcms = (PCMachineState *)
+        object_dynamic_cast(OBJECT(machine), TYPE_PC_MACHINE);
 
+    env->nr_dies = pcms ? pcms->smp_dies : 1;
+#endif
     cs->env_ptr = env;
 
     object_property_add(obj, "family", "int",
@@ -5812,11 +5819,13 @@ static Property x86_cpu_properties[] = {
     DEFINE_PROP_UINT32("apic-id", X86CPU, apic_id, 0),
     DEFINE_PROP_INT32("thread-id", X86CPU, thread_id, 0),
     DEFINE_PROP_INT32("core-id", X86CPU, core_id, 0),
+    DEFINE_PROP_INT32("die-id", X86CPU, die_id, 0),
     DEFINE_PROP_INT32("socket-id", X86CPU, socket_id, 0),
 #else
     DEFINE_PROP_UINT32("apic-id", X86CPU, apic_id, UNASSIGNED_APIC_ID),
     DEFINE_PROP_INT32("thread-id", X86CPU, thread_id, -1),
     DEFINE_PROP_INT32("core-id", X86CPU, core_id, -1),
+    DEFINE_PROP_INT32("die-id", X86CPU, die_id, -1),
     DEFINE_PROP_INT32("socket-id", X86CPU, socket_id, -1),
 #endif
     DEFINE_PROP_INT32("node-id", X86CPU, node_id, CPU_UNSET_NUMA_NODE_ID),
diff --git a/target/i386/cpu.h b/target/i386/cpu.h
index fce6660bac..d5f2a60ff5 100644
--- a/target/i386/cpu.h
+++ b/target/i386/cpu.h
@@ -1361,6 +1361,8 @@ typedef struct CPUX86State {
     uint64_t xss;
 
     TPRAccess tpr_access_type;
+
+    unsigned nr_dies;
 } CPUX86State;
 
 struct kvm_msrs;
@@ -1484,6 +1486,7 @@ struct X86CPU {
 
     int32_t node_id; /* NUMA node this CPU belongs to */
     int32_t socket_id;
+    int32_t die_id;
     int32_t core_id;
     int32_t thread_id;
 
-- 
2.21.0

[Qemu-devel] [PATCH v2 2/5] i386/cpu: Consolidate die-id validity in smp context

[Like Xu]

Following the legacy smp check rules, the die_id validity is added to
the same contexts as leagcy smp variables such as hmp_hotpluggable_cpus(),
machine_set_cpu_numa_node(), cpu_slot_to_string() and pc_cpu_pre_plug().

Signed-off-by: Like Xu <like.xu@linux.intel.com>
---
 hmp.c             |  3 +++
 hw/core/machine.c | 12 ++++++++++++
 hw/i386/pc.c      | 11 +++++++++++
 3 files changed, 26 insertions(+)

diff --git a/hmp.c b/hmp.c
index 56a3ed7375..7deb7b7226 100644
--- a/hmp.c
+++ b/hmp.c
@@ -3112,6 +3112,9 @@ void hmp_hotpluggable_cpus(Monitor *mon, const QDict *qdict)
         if (c->has_socket_id) {
             monitor_printf(mon, "    socket-id: \"%" PRIu64 "\"\n", c->socket_id);
         }
+        if (c->has_die_id) {
+            monitor_printf(mon, "    die-id: \"%" PRIu64 "\"\n", c->die_id);
+        }
         if (c->has_core_id) {
             monitor_printf(mon, "    core-id: \"%" PRIu64 "\"\n", c->core_id);
         }
diff --git a/hw/core/machine.c b/hw/core/machine.c
index 5d046a43e3..5116429732 100644
--- a/hw/core/machine.c
+++ b/hw/core/machine.c
@@ -659,6 +659,11 @@ void machine_set_cpu_numa_node(MachineState *machine,
             return;
         }
 
+        if (props->has_die_id && !slot->props.has_die_id) {
+            error_setg(errp, "die-id is not supported");
+            return;
+        }
+
         /* skip slots with explicit mismatch */
         if (props->has_thread_id && props->thread_id != slot->props.thread_id) {
                 continue;
@@ -668,6 +673,10 @@ void machine_set_cpu_numa_node(MachineState *machine,
                 continue;
         }
 
+        if (props->has_die_id && props->die_id != slot->props.die_id) {
+                continue;
+        }
+
         if (props->has_socket_id && props->socket_id != slot->props.socket_id) {
                 continue;
         }
@@ -925,6 +934,9 @@ static char *cpu_slot_to_string(const CPUArchId *cpu)
     if (cpu->props.has_socket_id) {
         g_string_append_printf(s, "socket-id: %"PRId64, cpu->props.socket_id);
     }
+    if (cpu->props.has_die_id) {
+        g_string_append_printf(s, "die-id: %"PRId64, cpu->props.die_id);
+    }
     if (cpu->props.has_core_id) {
         if (s->len) {
             g_string_append_printf(s, ", ");
diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index 83ab53c814..00be2463af 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -2321,6 +2321,10 @@ static void pc_cpu_pre_plug(HotplugHandler *hotplug_dev,
             error_setg(errp, "Invalid CPU socket-id: %u must be in range 0:%u",
                        cpu->socket_id, max_socket);
             return;
+        } else if (cpu->die_id > max_socket) {
+            error_setg(errp, "Invalid CPU die-id: %u must be in range 0:%u",
+                       cpu->die_id, max_socket);
+            return;
         }
         if (cpu->core_id < 0) {
             error_setg(errp, "CPU core-id is not set");
@@ -2378,6 +2382,13 @@ static void pc_cpu_pre_plug(HotplugHandler *hotplug_dev,
     }
     cpu->socket_id = topo.pkg_id;
 
+    if (cpu->die_id != -1 && cpu->die_id != topo.die_id) {
+        error_setg(errp, "property die-id: %u doesn't match set apic-id:"
+            " 0x%x (die-id: %u)", cpu->die_id, cpu->apic_id, topo.die_id);
+        return;
+    }
+    cpu->die_id = topo.die_id;
+
     if (cpu->core_id != -1 && cpu->core_id != topo.core_id) {
         error_setg(errp, "property core-id: %u doesn't match set apic-id:"
             " 0x%x (core-id: %u)", cpu->core_id, cpu->apic_id, topo.core_id);
-- 
2.21.0

[Qemu-devel] [PATCH v2 3/5] vl.c: Add -smp, dies=* command line support and update -smp doc

[Like Xu]

For PC target, users could configure the number of dies per one package
via command line with this patch, such as "-smp dies=2,cores=4".

A new pc-specified pc_smp_parse() is introduced and to keep the interface
consistent, refactoring legacy smp_parse() to __smp_parse() is necessary.

The parsing rules of new cpu-topology model obey the same restrictions/logic
as the legacy socket/core/thread model especially on missing values computing.

Signed-off-by: Like Xu <like.xu@linux.intel.com>
---
 qemu-options.hx | 17 +++++-----
 vl.c            | 89 ++++++++++++++++++++++++++++++++++++++++++++++++-
 2 files changed, 97 insertions(+), 9 deletions(-)

diff --git a/qemu-options.hx b/qemu-options.hx
index 5daa5a8fb0..7fad5b50ff 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -138,25 +138,26 @@ no incompatible TCG features have been enabled (e.g. icount/replay).
 ETEXI
 
 DEF("smp", HAS_ARG, QEMU_OPTION_smp,
-    "-smp [cpus=]n[,maxcpus=cpus][,cores=cores][,threads=threads][,sockets=sockets]\n"
+    "-smp [cpus=]n[,maxcpus=cpus][,cores=cores][,threads=threads][,dies=dies][,sockets=sockets]\n"
     "                set the number of CPUs to 'n' [default=1]\n"
     "                maxcpus= maximum number of total cpus, including\n"
     "                offline CPUs for hotplug, etc\n"
-    "                cores= number of CPU cores on one socket\n"
+    "                cores= number of CPU cores on one socket (for PC, it's on one die)\n"
     "                threads= number of threads on one CPU core\n"
+    "                dies= number of CPU dies on one socket (for PC only)\n"
     "                sockets= number of discrete sockets in the system\n",
         QEMU_ARCH_ALL)
 STEXI
-@item -smp [cpus=]@var{n}[,cores=@var{cores}][,threads=@var{threads}][,sockets=@var{sockets}][,maxcpus=@var{maxcpus}]
+@item -smp [cpus=]@var{n}[,cores=@var{cores}][,threads=@var{threads}][,dies=dies][,sockets=@var{sockets}][,maxcpus=@var{maxcpus}]
 @findex -smp
 Simulate an SMP system with @var{n} CPUs. On the PC target, up to 255
 CPUs are supported. On Sparc32 target, Linux limits the number of usable CPUs
 to 4.
-For the PC target, the number of @var{cores} per socket, the number
-of @var{threads} per cores and the total number of @var{sockets} can be
-specified. Missing values will be computed. If any on the three values is
-given, the total number of CPUs @var{n} can be omitted. @var{maxcpus}
-specifies the maximum number of hotpluggable CPUs.
+For the PC target, the number of @var{cores} per die, the number of @var{threads}
+per cores, the number of @var{dies} per packages and the total number of
+@var{sockets} can be specified. Missing values will be computed.
+If any on the three values is given, the total number of CPUs @var{n} can be omitted.
+@var{maxcpus} specifies the maximum number of hotpluggable CPUs.
 ETEXI
 
 DEF("numa", HAS_ARG, QEMU_OPTION_numa,
diff --git a/vl.c b/vl.c
index 8d92e2d209..66b577f447 100644
--- a/vl.c
+++ b/vl.c
@@ -63,6 +63,7 @@ int main(int argc, char **argv)
 #include "sysemu/watchdog.h"
 #include "hw/firmware/smbios.h"
 #include "hw/acpi/acpi.h"
+#include "hw/i386/pc.h"
 #include "hw/xen/xen.h"
 #include "hw/qdev.h"
 #include "hw/loader.h"
@@ -1248,6 +1249,9 @@ static QemuOptsList qemu_smp_opts = {
         }, {
             .name = "sockets",
             .type = QEMU_OPT_NUMBER,
+        }, {
+            .name = "dies",
+            .type = QEMU_OPT_NUMBER,
         }, {
             .name = "cores",
             .type = QEMU_OPT_NUMBER,
@@ -1262,7 +1266,7 @@ static QemuOptsList qemu_smp_opts = {
     },
 };
 
-static void smp_parse(QemuOpts *opts)
+static void __smp_parse(QemuOpts *opts)
 {
     if (opts) {
         unsigned cpus    = qemu_opt_get_number(opts, "cpus", 0);
@@ -1334,6 +1338,89 @@ static void smp_parse(QemuOpts *opts)
     }
 }
 
+static void pc_smp_parse(QemuOpts *opts)
+{
+    PCMachineState *pcms = (PCMachineState *)
+        object_dynamic_cast(OBJECT(current_machine), TYPE_PC_MACHINE);
+
+    unsigned cpus    = qemu_opt_get_number(opts, "cpus", 0);
+    unsigned sockets = qemu_opt_get_number(opts, "sockets", 0);
+    unsigned dies = qemu_opt_get_number(opts, "dies", 1);
+    unsigned cores   = qemu_opt_get_number(opts, "cores", 0);
+    unsigned threads = qemu_opt_get_number(opts, "threads", 0);
+
+    /* compute missing values, prefer sockets over cores over threads */
+    if (cpus == 0 || sockets == 0) {
+        cores = cores > 0 ? cores : 1;
+        threads = threads > 0 ? threads : 1;
+        if (cpus == 0) {
+            sockets = sockets > 0 ? sockets : 1;
+            cpus = cores * threads * dies * sockets;
+        } else {
+            current_machine->smp.max_cpus =
+                    qemu_opt_get_number(opts, "maxcpus", cpus);
+            sockets = current_machine->smp.max_cpus / (cores * threads * dies);
+        }
+    } else if (cores == 0) {
+        threads = threads > 0 ? threads : 1;
+        cores = cpus / (sockets * dies * threads);
+        cores = cores > 0 ? cores : 1;
+    } else if (threads == 0) {
+        threads = cpus / (cores * dies * sockets);
+        threads = threads > 0 ? threads : 1;
+    } else if (sockets * dies * cores * threads < cpus) {
+        error_report("cpu topology: "
+                        "sockets (%u) * dies (%u) * cores (%u) * threads (%u) < "
+                        "smp_cpus (%u)",
+                        sockets, dies, cores, threads, cpus);
+        exit(1);
+    }
+
+    current_machine->smp.max_cpus =
+            qemu_opt_get_number(opts, "maxcpus", cpus);
+
+    if (current_machine->smp.max_cpus < cpus) {
+        error_report("maxcpus must be equal to or greater than smp");
+        exit(1);
+    }
+
+    if (sockets * dies * cores * threads > current_machine->smp.max_cpus) {
+        error_report("cpu topology: "
+                        "sockets (%u) * dies (%u) * cores (%u) * threads (%u) > "
+                        "maxcpus (%u)",
+                        sockets, dies, cores, threads,
+                        current_machine->smp.max_cpus);
+        exit(1);
+    }
+
+    if (sockets * dies * cores * threads != current_machine->smp.max_cpus) {
+        warn_report("Invalid CPU topology deprecated: "
+                    "sockets (%u) * dies (%u) * cores (%u) * threads (%u) "
+                    "!= maxcpus (%u)",
+                    sockets, dies, cores, threads,
+                    current_machine->smp.max_cpus);
+    }
+
+    current_machine->smp.cpus = cpus;
+    current_machine->smp.cores = cores;
+    current_machine->smp.threads = threads;
+    pcms->smp_dies = dies;
+
+    if (current_machine->smp.cpus > 1) {
+        Error *blocker = NULL;
+        error_setg(&blocker, QERR_REPLAY_NOT_SUPPORTED, "smp");
+        replay_add_blocker(blocker);
+    }
+}
+
+static void smp_parse(QemuOpts *opts)
+{
+    if (object_dynamic_cast(OBJECT(current_machine), TYPE_PC_MACHINE))
+        pc_smp_parse(opts);
+    else
+        __smp_parse(opts);
+}
+
 static void realtime_init(void)
 {
     if (enable_mlock) {
-- 
2.21.0

[Qemu-devel] [PATCH v2 4/5] i386/cpu: Update apicid parsing rules and topo-bit tests for dies

[Like Xu]

On Intel MCP (Multi-chip packaging) platforms, the apicid of logical cpu
would imply die level info of cpu topology thus x86_apicid_from_cpu_idx()
should be refactored with virtual nr_dies, so does apicid_*_offset().

To maintain semantic consistency, the pkg_offset which helps to generate
CPUIDs such as 0x3 for L3 cache is mapping to die_offset from this commit.

The corresponding topo_bits tests are updated to test die configurations.

Signed-off-by: Like Xu <like.xu@linux.intel.com>
---
 hw/i386/pc.c               | 38 +++++++++++------
 include/hw/i386/topology.h | 76 ++++++++++++++++++++++++----------
 target/i386/cpu.c          | 13 +++---
 tests/test-x86-cpuid.c     | 84 ++++++++++++++++++++------------------
 4 files changed, 133 insertions(+), 78 deletions(-)

diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index 00be2463af..e498334cbc 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -935,10 +935,11 @@ void enable_compat_apic_id_mode(void)
 static uint32_t x86_cpu_apic_id_from_index(MachineState *ms,
                                            unsigned int cpu_index)
 {
+    PCMachineState *pcms = PC_MACHINE(ms);
     uint32_t correct_id;
     static bool warned;
 
-    correct_id = x86_apicid_from_cpu_idx(ms->smp.cores,
+    correct_id = x86_apicid_from_cpu_idx(pcms->smp_dies, ms->smp.cores,
                                          ms->smp.threads, cpu_index);
     if (compat_apic_id_mode) {
         if (cpu_index != correct_id && !warned && !qtest_enabled()) {
@@ -2303,6 +2304,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_dies = pcms->smp_dies;
 
     if(!object_dynamic_cast(OBJECT(cpu), ms->cpu_type)) {
         error_setg(errp, "Invalid CPU type, expected cpu type: '%s'",
@@ -2310,9 +2312,13 @@ static void pc_cpu_pre_plug(HotplugHandler *hotplug_dev,
         return;
     }
 
-    /* if APIC ID is not set, set it based on socket/core/thread properties */
+    /*
+     * If APIC ID is not set,
+     * set it based on socket/die/core/thread properties.
+     */
     if (cpu->apic_id == UNASSIGNED_APIC_ID) {
-        int max_socket = (ms->smp.max_cpus - 1) / smp_threads / smp_cores;
+        int max_socket = (ms->smp.max_cpus - 1) /
+                                smp_threads / smp_cores / pcms->smp_dies;
 
         if (cpu->socket_id < 0) {
             error_setg(errp, "CPU socket-id is not set");
@@ -2347,18 +2353,21 @@ static void pc_cpu_pre_plug(HotplugHandler *hotplug_dev,
         topo.core_id = cpu->core_id;
         topo.die_id = cpu->die_id;
         topo.smt_id = cpu->thread_id;
-        cpu->apic_id = apicid_from_topo_ids(smp_cores, smp_threads, &topo);
+        cpu->apic_id = apicid_from_topo_ids(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, smp_cores, smp_threads, &topo);
-        error_setg(errp, "Invalid CPU [socket: %u, core: %u, thread: %u] with"
-                  " APIC ID %" PRIu32 ", valid index range 0:%d",
-                   topo.pkg_id, topo.core_id, topo.smt_id, cpu->apic_id,
-                   ms->possible_cpus->len - 1);
+        x86_topo_ids_from_apicid(cpu->apic_id, 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",
+            topo.pkg_id, topo.die_id, topo.core_id, topo.smt_id,
+            cpu->apic_id, ms->possible_cpus->len - 1);
         return;
     }
 
@@ -2374,7 +2383,8 @@ static void pc_cpu_pre_plug(HotplugHandler *hotplug_dev,
     /* TODO: move socket_id/core_id/thread_id checks into x86_cpu_realizefn()
      * once -smp refactoring is complete and there will be CPU private
      * CPUState::nr_cores and CPUState::nr_threads fields instead of globals */
-    x86_topo_ids_from_apicid(cpu->apic_id, smp_cores, smp_threads, &topo);
+    x86_topo_ids_from_apicid(cpu->apic_id, smp_dies,
+                             smp_cores, smp_threads, &topo);
     if (cpu->socket_id != -1 && cpu->socket_id != topo.pkg_id) {
         error_setg(errp, "property socket-id: %u doesn't match set apic-id:"
             " 0x%x (socket-id: %u)", cpu->socket_id, cpu->apic_id, topo.pkg_id);
@@ -2670,10 +2680,12 @@ pc_cpu_index_to_props(MachineState *ms, unsigned cpu_index)
 static int64_t pc_get_default_cpu_node_id(const MachineState *ms, int idx)
 {
    X86CPUTopoInfo topo;
+   PCMachineState *pcms = PC_MACHINE(ms);
 
    assert(idx < ms->possible_cpus->len);
    x86_topo_ids_from_apicid(ms->possible_cpus->cpus[idx].arch_id,
-                            ms->smp.cores, ms->smp.threads, &topo);
+                            pcms->smp_dies, ms->smp.cores,
+                            ms->smp.threads, &topo);
    return topo.pkg_id % nb_numa_nodes;
 }
 
@@ -2681,6 +2693,7 @@ static const CPUArchIdList *pc_possible_cpu_arch_ids(MachineState *ms)
 {
     int i;
     unsigned int max_cpus = ms->smp.max_cpus;
+    PCMachineState *pcms = PC_MACHINE(ms);
 
     if (ms->possible_cpus) {
         /*
@@ -2701,7 +2714,8 @@ static const CPUArchIdList *pc_possible_cpu_arch_ids(MachineState *ms)
         ms->possible_cpus->cpus[i].vcpus_count = 1;
         ms->possible_cpus->cpus[i].arch_id = x86_cpu_apic_id_from_index(ms, i);
         x86_topo_ids_from_apicid(ms->possible_cpus->cpus[i].arch_id,
-                                 ms->smp.cores, ms->smp.threads, &topo);
+                                 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;
diff --git a/include/hw/i386/topology.h b/include/hw/i386/topology.h
index 7f80498eb3..4ff5b2da6c 100644
--- a/include/hw/i386/topology.h
+++ b/include/hw/i386/topology.h
@@ -63,88 +63,120 @@ static unsigned apicid_bitwidth_for_count(unsigned count)
 
 /* Bit width of the SMT_ID (thread ID) field on the APIC ID
  */
-static inline unsigned apicid_smt_width(unsigned nr_cores, unsigned nr_threads)
+static inline unsigned apicid_smt_width(unsigned nr_dies,
+                                        unsigned nr_cores,
+                                        unsigned nr_threads)
 {
     return apicid_bitwidth_for_count(nr_threads);
 }
 
 /* Bit width of the Core_ID field
  */
-static inline unsigned apicid_core_width(unsigned nr_cores, unsigned nr_threads)
+static inline unsigned apicid_core_width(unsigned nr_dies,
+                                         unsigned nr_cores,
+                                         unsigned nr_threads)
 {
     return apicid_bitwidth_for_count(nr_cores);
 }
 
+/* Bit width of the Die_ID field */
+static inline unsigned apicid_die_width(unsigned nr_dies,
+                                        unsigned nr_cores,
+                                        unsigned nr_threads)
+{
+    return apicid_bitwidth_for_count(nr_dies);
+}
+
 /* Bit offset of the Core_ID field
  */
-static inline unsigned apicid_core_offset(unsigned nr_cores,
+static inline unsigned apicid_core_offset(unsigned nr_dies,
+                                          unsigned nr_cores,
                                           unsigned nr_threads)
 {
-    return apicid_smt_width(nr_cores, nr_threads);
+    return apicid_smt_width(nr_dies, nr_cores, nr_threads);
+}
+
+/* Bit offset of the Die_ID field */
+static inline unsigned apicid_die_offset(unsigned nr_dies,
+                                          unsigned nr_cores,
+                                           unsigned nr_threads)
+{
+    return apicid_core_offset(nr_dies, nr_cores, nr_threads) +
+           apicid_core_width(nr_dies, nr_cores, nr_threads);
 }
 
 /* Bit offset of the Pkg_ID (socket ID) field
  */
-static inline unsigned apicid_pkg_offset(unsigned nr_cores, unsigned nr_threads)
+static inline unsigned apicid_pkg_offset(unsigned nr_dies,
+                                         unsigned nr_cores,
+                                         unsigned nr_threads)
 {
-    return apicid_core_offset(nr_cores, nr_threads) +
-           apicid_core_width(nr_cores, nr_threads);
+    return apicid_die_offset(nr_dies, nr_cores, nr_threads) +
+           apicid_die_width(nr_dies, nr_cores, nr_threads);
 }
 
 /* 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(unsigned nr_cores,
+static inline apic_id_t apicid_from_topo_ids(unsigned nr_dies,
+                                             unsigned nr_cores,
                                              unsigned nr_threads,
                                              const X86CPUTopoInfo *topo)
 {
-    return (topo->pkg_id  << apicid_pkg_offset(nr_cores, nr_threads)) |
-           (topo->core_id << apicid_core_offset(nr_cores, nr_threads)) |
+    return (topo->pkg_id  << apicid_pkg_offset(nr_dies, nr_cores, nr_threads)) |
+           (topo->die_id  << apicid_die_offset(nr_dies, nr_cores, nr_threads)) |
+          (topo->core_id << apicid_core_offset(nr_dies, nr_cores, nr_threads)) |
            topo->smt_id;
 }
 
 /* Calculate thread/core/package IDs for a specific topology,
  * based on (contiguous) CPU index
  */
-static inline void x86_topo_ids_from_idx(unsigned nr_cores,
+static inline void x86_topo_ids_from_idx(unsigned nr_dies,
+                                         unsigned nr_cores,
                                          unsigned nr_threads,
                                          unsigned cpu_index,
                                          X86CPUTopoInfo *topo)
 {
-    unsigned core_index = cpu_index / nr_threads;
+    topo->pkg_id = cpu_index / (nr_dies * nr_cores * nr_threads);
+    topo->die_id = cpu_index / (nr_cores * nr_threads) % nr_dies;
+    topo->core_id = cpu_index / nr_threads % nr_cores;
     topo->smt_id = cpu_index % nr_threads;
-    topo->core_id = core_index % nr_cores;
-    topo->pkg_id = core_index / nr_cores;
 }
 
 /* Calculate thread/core/package IDs for a specific topology,
  * based on APIC ID
  */
 static inline void x86_topo_ids_from_apicid(apic_id_t apicid,
+                                            unsigned nr_dies,
                                             unsigned nr_cores,
                                             unsigned nr_threads,
                                             X86CPUTopoInfo *topo)
 {
     topo->smt_id = apicid &
-                   ~(0xFFFFFFFFUL << apicid_smt_width(nr_cores, nr_threads));
-    topo->core_id = (apicid >> apicid_core_offset(nr_cores, nr_threads)) &
-                   ~(0xFFFFFFFFUL << apicid_core_width(nr_cores, nr_threads));
-    topo->pkg_id = apicid >> apicid_pkg_offset(nr_cores, nr_threads);
-    topo->die_id = -1;
+            ~(0xFFFFFFFFUL << apicid_smt_width(nr_dies, nr_cores, nr_threads));
+    topo->core_id =
+            (apicid >> apicid_core_offset(nr_dies, nr_cores, nr_threads)) &
+            ~(0xFFFFFFFFUL << apicid_core_width(nr_dies, nr_cores, nr_threads));
+    topo->die_id =
+            (apicid >> apicid_die_offset(nr_dies, nr_cores, nr_threads)) &
+            ~(0xFFFFFFFFUL << apicid_die_width(nr_dies, nr_cores, nr_threads));
+    topo->pkg_id = apicid >> apicid_pkg_offset(nr_dies, nr_cores, nr_threads);
 }
 
 /* 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(unsigned nr_cores,
+static inline apic_id_t x86_apicid_from_cpu_idx(unsigned nr_dies,
+                                                unsigned nr_cores,
                                                 unsigned nr_threads,
                                                 unsigned cpu_index)
 {
     X86CPUTopoInfo topo;
-    x86_topo_ids_from_idx(nr_cores, nr_threads, cpu_index, &topo);
-    return apicid_from_topo_ids(nr_cores, nr_threads, &topo);
+    x86_topo_ids_from_idx(nr_dies, nr_cores, nr_threads, cpu_index, &topo);
+    return apicid_from_topo_ids(nr_dies, nr_cores, nr_threads, &topo);
 }
 
 #endif /* HW_I386_TOPOLOGY_H */
diff --git a/target/i386/cpu.c b/target/i386/cpu.c
index 9bd35b4965..3222bd3254 100644
--- a/target/i386/cpu.c
+++ b/target/i386/cpu.c
@@ -4225,7 +4225,7 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t index, uint32_t count,
 {
     X86CPU *cpu = x86_env_get_cpu(env);
     CPUState *cs = CPU(cpu);
-    uint32_t pkg_offset;
+    uint32_t die_offset;
     uint32_t limit;
     uint32_t signature[3];
 
@@ -4314,10 +4314,11 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t index, uint32_t count,
                                     eax, ebx, ecx, edx);
                 break;
             case 3: /* L3 cache info */
-                pkg_offset = apicid_pkg_offset(cs->nr_cores, cs->nr_threads);
+                die_offset = apicid_die_offset(env->nr_dies,
+                                        cs->nr_cores, cs->nr_threads);
                 if (cpu->enable_l3_cache) {
                     encode_cache_cpuid4(env->cache_info_cpuid4.l3_cache,
-                                        (1 << pkg_offset), cs->nr_cores,
+                                        (1 << die_offset), cs->nr_cores,
                                         eax, ebx, ecx, edx);
                     break;
                 }
@@ -4399,12 +4400,14 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t index, uint32_t count,
 
         switch (count) {
         case 0:
-            *eax = apicid_core_offset(cs->nr_cores, cs->nr_threads);
+            *eax = apicid_core_offset(env->nr_dies,
+                                      cs->nr_cores, cs->nr_threads);
             *ebx = cs->nr_threads;
             *ecx |= CPUID_TOPOLOGY_LEVEL_SMT;
             break;
         case 1:
-            *eax = apicid_pkg_offset(cs->nr_cores, cs->nr_threads);
+            *eax = apicid_pkg_offset(env->nr_dies,
+                                     cs->nr_cores, cs->nr_threads);
             *ebx = cs->nr_cores * cs->nr_threads;
             *ecx |= CPUID_TOPOLOGY_LEVEL_CORE;
             break;
diff --git a/tests/test-x86-cpuid.c b/tests/test-x86-cpuid.c
index ff225006e4..1942287f33 100644
--- a/tests/test-x86-cpuid.c
+++ b/tests/test-x86-cpuid.c
@@ -28,74 +28,80 @@
 
 static void test_topo_bits(void)
 {
-    /* simple tests for 1 thread per core, 1 core per socket */
-    g_assert_cmpuint(apicid_smt_width(1, 1), ==, 0);
-    g_assert_cmpuint(apicid_core_width(1, 1), ==, 0);
+    /* simple tests for 1 thread per core, 1 core per die, 1 die per package */
+    g_assert_cmpuint(apicid_smt_width(1, 1, 1), ==, 0);
+    g_assert_cmpuint(apicid_core_width(1, 1, 1), ==, 0);
+    g_assert_cmpuint(apicid_die_width(1, 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);
+    g_assert_cmpuint(x86_apicid_from_cpu_idx(1, 1, 1, 0), ==, 0);
+    g_assert_cmpuint(x86_apicid_from_cpu_idx(1, 1, 1, 1), ==, 1);
+    g_assert_cmpuint(x86_apicid_from_cpu_idx(1, 1, 1, 2), ==, 2);
+    g_assert_cmpuint(x86_apicid_from_cpu_idx(1, 1, 1, 3), ==, 3);
 
 
     /* Test field width calculation for multiple values
      */
-    g_assert_cmpuint(apicid_smt_width(1, 2), ==, 1);
-    g_assert_cmpuint(apicid_smt_width(1, 3), ==, 2);
-    g_assert_cmpuint(apicid_smt_width(1, 4), ==, 2);
+    g_assert_cmpuint(apicid_smt_width(1, 1, 2), ==, 1);
+    g_assert_cmpuint(apicid_smt_width(1, 1, 3), ==, 2);
+    g_assert_cmpuint(apicid_smt_width(1, 1, 4), ==, 2);
 
-    g_assert_cmpuint(apicid_smt_width(1, 14), ==, 4);
-    g_assert_cmpuint(apicid_smt_width(1, 15), ==, 4);
-    g_assert_cmpuint(apicid_smt_width(1, 16), ==, 4);
-    g_assert_cmpuint(apicid_smt_width(1, 17), ==, 5);
+    g_assert_cmpuint(apicid_smt_width(1, 1, 14), ==, 4);
+    g_assert_cmpuint(apicid_smt_width(1, 1, 15), ==, 4);
+    g_assert_cmpuint(apicid_smt_width(1, 1, 16), ==, 4);
+    g_assert_cmpuint(apicid_smt_width(1, 1, 17), ==, 5);
 
 
-    g_assert_cmpuint(apicid_core_width(30, 2), ==, 5);
-    g_assert_cmpuint(apicid_core_width(31, 2), ==, 5);
-    g_assert_cmpuint(apicid_core_width(32, 2), ==, 5);
-    g_assert_cmpuint(apicid_core_width(33, 2), ==, 6);
+    g_assert_cmpuint(apicid_core_width(1, 30, 2), ==, 5);
+    g_assert_cmpuint(apicid_core_width(1, 31, 2), ==, 5);
+    g_assert_cmpuint(apicid_core_width(1, 32, 2), ==, 5);
+    g_assert_cmpuint(apicid_core_width(1, 33, 2), ==, 6);
 
+    g_assert_cmpuint(apicid_die_width(1, 30, 2), ==, 0);
+    g_assert_cmpuint(apicid_die_width(2, 30, 2), ==, 1);
+    g_assert_cmpuint(apicid_die_width(3, 30, 2), ==, 2);
+    g_assert_cmpuint(apicid_die_width(4, 30, 2), ==, 2);
 
     /* build a weird topology and see if IDs are calculated correctly
      */
 
     /* This will use 2 bits for thread ID and 3 bits for core ID
      */
-    g_assert_cmpuint(apicid_smt_width(6, 3), ==, 2);
-    g_assert_cmpuint(apicid_core_width(6, 3), ==, 3);
-    g_assert_cmpuint(apicid_pkg_offset(6, 3), ==, 5);
+    g_assert_cmpuint(apicid_smt_width(1, 6, 3), ==, 2);
+    g_assert_cmpuint(apicid_core_offset(1, 6, 3), ==, 2);
+    g_assert_cmpuint(apicid_die_offset(1, 6, 3), ==, 5);
+    g_assert_cmpuint(apicid_pkg_offset(1, 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(1, 6, 3, 0), ==, 0);
+    g_assert_cmpuint(x86_apicid_from_cpu_idx(1, 6, 3, 1), ==, 1);
+    g_assert_cmpuint(x86_apicid_from_cpu_idx(1, 6, 3, 2), ==, 2);
 
-    g_assert_cmpuint(x86_apicid_from_cpu_idx(6, 3, 1 * 3 + 0), ==,
+    g_assert_cmpuint(x86_apicid_from_cpu_idx(1, 6, 3, 1 * 3 + 0), ==,
                      (1 << 2) | 0);
-    g_assert_cmpuint(x86_apicid_from_cpu_idx(6, 3, 1 * 3 + 1), ==,
+    g_assert_cmpuint(x86_apicid_from_cpu_idx(1, 6, 3, 1 * 3 + 1), ==,
                      (1 << 2) | 1);
-    g_assert_cmpuint(x86_apicid_from_cpu_idx(6, 3, 1 * 3 + 2), ==,
+    g_assert_cmpuint(x86_apicid_from_cpu_idx(1, 6, 3, 1 * 3 + 2), ==,
                      (1 << 2) | 2);
 
-    g_assert_cmpuint(x86_apicid_from_cpu_idx(6, 3, 2 * 3 + 0), ==,
+    g_assert_cmpuint(x86_apicid_from_cpu_idx(1, 6, 3, 2 * 3 + 0), ==,
                      (2 << 2) | 0);
-    g_assert_cmpuint(x86_apicid_from_cpu_idx(6, 3, 2 * 3 + 1), ==,
+    g_assert_cmpuint(x86_apicid_from_cpu_idx(1, 6, 3, 2 * 3 + 1), ==,
                      (2 << 2) | 1);
-    g_assert_cmpuint(x86_apicid_from_cpu_idx(6, 3, 2 * 3 + 2), ==,
+    g_assert_cmpuint(x86_apicid_from_cpu_idx(1, 6, 3, 2 * 3 + 2), ==,
                      (2 << 2) | 2);
 
-    g_assert_cmpuint(x86_apicid_from_cpu_idx(6, 3, 5 * 3 + 0), ==,
+    g_assert_cmpuint(x86_apicid_from_cpu_idx(1, 6, 3, 5 * 3 + 0), ==,
                      (5 << 2) | 0);
-    g_assert_cmpuint(x86_apicid_from_cpu_idx(6, 3, 5 * 3 + 1), ==,
+    g_assert_cmpuint(x86_apicid_from_cpu_idx(1, 6, 3, 5 * 3 + 1), ==,
                      (5 << 2) | 1);
-    g_assert_cmpuint(x86_apicid_from_cpu_idx(6, 3, 5 * 3 + 2), ==,
+    g_assert_cmpuint(x86_apicid_from_cpu_idx(1, 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);
+    g_assert_cmpuint(x86_apicid_from_cpu_idx(1, 6, 3,
+                     1 * 6 * 3 + 0 * 3 + 0), ==, (1 << 5));
+    g_assert_cmpuint(x86_apicid_from_cpu_idx(1, 6, 3,
+                     1 * 6 * 3 + 1 * 3 + 1), ==, (1 << 5) | (1 << 2) | 1);
+    g_assert_cmpuint(x86_apicid_from_cpu_idx(1, 6, 3,
+                     3 * 6 * 3 + 5 * 3 + 2), ==, (3 << 5) | (5 << 2) | 2);
 }
 
 int main(int argc, char **argv)
-- 
2.21.0

[Qemu-devel] [PATCH v2 5/5] target/i386: Add CPUID.1F generation support for multi-die PCMachine

[Like Xu]

The CPUID.1F as Intel V2 Extended Topology Enumeration Leaf would be exposed
if guests want to emulate multiple software-visible die within each package.
Per Intel's SDM, the 0x1f is a superset of 0xb, thus they can be generated
by almost same code as 0xb except die_offset setting.

If the number of dies per package is less than 2, the qemu will not expose
CPUID.1F regardless of whether the host supports CPUID.1F, and in any case,
cpuid.0.eax would store the maximum input value for **guest** basic CPUID.

If users do want to expose CPUID.1F by passing dies > 1 for simulation without
host support, there will be a smp topology warning but it is not blocking.

Signed-off-by: Like Xu <like.xu@linux.intel.com>
---
 target/i386/cpu.c | 37 +++++++++++++++++++++++++++++++++++++
 target/i386/cpu.h |  4 ++++
 target/i386/kvm.c | 30 ++++++++++++++++++++++++++++--
 3 files changed, 69 insertions(+), 2 deletions(-)

diff --git a/target/i386/cpu.c b/target/i386/cpu.c
index 3222bd3254..cd6c9933c3 100644
--- a/target/i386/cpu.c
+++ b/target/i386/cpu.c
@@ -4417,6 +4417,42 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t index, uint32_t count,
             *ecx |= CPUID_TOPOLOGY_LEVEL_INVALID;
         }
 
+        assert(!(*eax & ~0x1f));
+        *ebx &= 0xffff; /* The count doesn't need to be reliable. */
+        break;
+    case 0x1F:
+        /* V2 Extended Topology Enumeration Leaf */
+        if (env->nr_dies < 2 || !cpu->enable_cpuid_0x1f) {
+            *eax = *ebx = *ecx = *edx = 0;
+            break;
+        }
+
+        *ecx = count & 0xff;
+        *edx = cpu->apic_id;
+        switch (count) {
+        case 0:
+            *eax = apicid_core_offset(env->nr_dies, cs->nr_cores,
+                                                    cs->nr_threads);
+            *ebx = cs->nr_threads;
+            *ecx |= CPUID_TOPOLOGY_LEVEL_SMT;
+            break;
+        case 1:
+            *eax = apicid_die_offset(env->nr_dies, cs->nr_cores,
+                                                   cs->nr_threads);
+            *ebx = cs->nr_cores * cs->nr_threads;
+            *ecx |= CPUID_TOPOLOGY_LEVEL_CORE;
+            break;
+        case 2:
+            *eax = apicid_pkg_offset(env->nr_dies, cs->nr_cores,
+                                                   cs->nr_threads);
+            *ebx = env->nr_dies * cs->nr_cores * cs->nr_threads;
+            *ecx |= CPUID_TOPOLOGY_LEVEL_DIE;
+            break;
+        default:
+            *eax = 0;
+            *ebx = 0;
+            *ecx |= CPUID_TOPOLOGY_LEVEL_INVALID;
+        }
         assert(!(*eax & ~0x1f));
         *ebx &= 0xffff; /* The count doesn't need to be reliable. */
         break;
@@ -5864,6 +5900,7 @@ static Property x86_cpu_properties[] = {
     DEFINE_PROP_BOOL("full-cpuid-auto-level", X86CPU, full_cpuid_auto_level, true),
     DEFINE_PROP_STRING("hv-vendor-id", X86CPU, hyperv_vendor_id),
     DEFINE_PROP_BOOL("cpuid-0xb", X86CPU, enable_cpuid_0xb, true),
+    DEFINE_PROP_BOOL("cpuid-0x1f", X86CPU, enable_cpuid_0x1f, true),
     DEFINE_PROP_BOOL("lmce", X86CPU, enable_lmce, false),
     DEFINE_PROP_BOOL("l3-cache", X86CPU, enable_l3_cache, true),
     DEFINE_PROP_BOOL("kvm-no-smi-migration", X86CPU, kvm_no_smi_migration,
diff --git a/target/i386/cpu.h b/target/i386/cpu.h
index d5f2a60ff5..9b54c646e7 100644
--- a/target/i386/cpu.h
+++ b/target/i386/cpu.h
@@ -735,6 +735,7 @@ typedef uint32_t FeatureWordArray[FEATURE_WORDS];
 #define CPUID_TOPOLOGY_LEVEL_INVALID  (0U << 8)
 #define CPUID_TOPOLOGY_LEVEL_SMT      (1U << 8)
 #define CPUID_TOPOLOGY_LEVEL_CORE     (2U << 8)
+#define CPUID_TOPOLOGY_LEVEL_DIE      (5U << 8)
 
 /* MSR Feature Bits */
 #define MSR_ARCH_CAP_RDCL_NO    (1U << 0)
@@ -1455,6 +1456,9 @@ struct X86CPU {
     /* Compatibility bits for old machine types: */
     bool enable_cpuid_0xb;
 
+    /* V2 Compatibility bits for old machine types: */
+    bool enable_cpuid_0x1f;
+
     /* Enable auto level-increase for all CPUID leaves */
     bool full_cpuid_auto_level;
 
diff --git a/target/i386/kvm.c b/target/i386/kvm.c
index 3b29ce5c0d..d8b8bd5c9e 100644
--- a/target/i386/kvm.c
+++ b/target/i386/kvm.c
@@ -931,12 +931,12 @@ int kvm_arch_init_vcpu(CPUState *cs)
     struct kvm_cpuid_entry2 *c;
     uint32_t signature[3];
     int kvm_base = KVM_CPUID_SIGNATURE;
-    int r;
+    int r, cpuid_0_entry, cpuid_min_level;
     Error *local_err = NULL;
 
     memset(&cpuid_data, 0, sizeof(cpuid_data));
 
-    cpuid_i = 0;
+    cpuid_i = cpuid_0_entry = cpuid_min_level = 0;
 
     r = kvm_arch_set_tsc_khz(cs);
     if (r < 0) {
@@ -1050,6 +1050,11 @@ int kvm_arch_init_vcpu(CPUState *cs)
 
     cpu_x86_cpuid(env, 0, 0, &limit, &unused, &unused, &unused);
 
+    if (limit < 0x1f && env->nr_dies > 1 && cpu->enable_cpuid_0x1f) {
+        limit = env->cpuid_level = env->cpuid_min_level = 0x1f;
+        warn_report("CPU topology: the CPUID.1F isn't supported on the host.");
+    }
+
     for (i = 0; i <= limit; i++) {
         if (cpuid_i == KVM_MAX_CPUID_ENTRIES) {
             fprintf(stderr, "unsupported level value: 0x%x\n", limit);
@@ -1081,6 +1086,10 @@ int kvm_arch_init_vcpu(CPUState *cs)
             }
             break;
         }
+        case 0x1f:
+            if (env->nr_dies < 2 || !cpu->enable_cpuid_0x1f) {
+                break;
+            }
         case 4:
         case 0xb:
         case 0xd:
@@ -1088,6 +1097,11 @@ int kvm_arch_init_vcpu(CPUState *cs)
                 if (i == 0xd && j == 64) {
                     break;
                 }
+
+                if (i == 0x1f && j == 64) {
+                    break;
+                }
+
                 c->function = i;
                 c->flags = KVM_CPUID_FLAG_SIGNIFCANT_INDEX;
                 c->index = j;
@@ -1099,6 +1113,9 @@ int kvm_arch_init_vcpu(CPUState *cs)
                 if (i == 0xb && !(c->ecx & 0xff00)) {
                     break;
                 }
+                if (i == 0x1f && !(c->ecx & 0xff00)) {
+                    break;
+                }
                 if (i == 0xd && c->eax == 0) {
                     continue;
                 }
@@ -1139,8 +1156,17 @@ int kvm_arch_init_vcpu(CPUState *cs)
             cpu_x86_cpuid(env, i, 0, &c->eax, &c->ebx, &c->ecx, &c->edx);
             break;
         }
+
+        cpuid_0_entry = (i == 0) ? (cpuid_i - 1) : cpuid_0_entry;
+        cpuid_min_level =
+                ((c->eax | c->ebx | c->ecx | c->edx | c->flags | c->index) &&
+                                (i > cpuid_min_level)) ? i : cpuid_min_level;
     }
 
+    env->cpuid_level = env->cpuid_min_level = cpuid_min_level;
+    c = &cpuid_data.entries[cpuid_0_entry];
+    cpu_x86_cpuid(env, 0, 0, &c->eax, &c->ebx, &c->ecx, &c->edx);
+
     if (limit >= 0x0a) {
         uint32_t eax, edx;
 
-- 
2.21.0

Re: [Qemu-devel] [PATCH v2 2/5] i386/cpu: Consolidate die-id validity in smp context

[Dr. David Alan Gilbert]

* Like Xu (like.xu@linux.intel.com) wrote:
> Following the legacy smp check rules, the die_id validity is added to
> the same contexts as leagcy smp variables such as hmp_hotpluggable_cpus(),
> machine_set_cpu_numa_node(), cpu_slot_to_string() and pc_cpu_pre_plug().
> 
> Signed-off-by: Like Xu <like.xu@linux.intel.com>
> ---
>  hmp.c             |  3 +++
>  hw/core/machine.c | 12 ++++++++++++
>  hw/i386/pc.c      | 11 +++++++++++
>  3 files changed, 26 insertions(+)
> 
> diff --git a/hmp.c b/hmp.c
> index 56a3ed7375..7deb7b7226 100644
> --- a/hmp.c
> +++ b/hmp.c
> @@ -3112,6 +3112,9 @@ void hmp_hotpluggable_cpus(Monitor *mon, const QDict *qdict)
>          if (c->has_socket_id) {
>              monitor_printf(mon, "    socket-id: \"%" PRIu64 "\"\n", c->socket_id);
>          }
> +        if (c->has_die_id) {
> +            monitor_printf(mon, "    die-id: \"%" PRIu64 "\"\n", c->die_id);
> +        }
>          if (c->has_core_id) {
>              monitor_printf(mon, "    core-id: \"%" PRIu64 "\"\n", c->core_id);
>          }
> diff --git a/hw/core/machine.c b/hw/core/machine.c
> index 5d046a43e3..5116429732 100644
> --- a/hw/core/machine.c
> +++ b/hw/core/machine.c
> @@ -659,6 +659,11 @@ void machine_set_cpu_numa_node(MachineState *machine,
>              return;
>          }
>  
> +        if (props->has_die_id && !slot->props.has_die_id) {
> +            error_setg(errp, "die-id is not supported");
> +            return;
> +        }
> +
>          /* skip slots with explicit mismatch */
>          if (props->has_thread_id && props->thread_id != slot->props.thread_id) {
>                  continue;
> @@ -668,6 +673,10 @@ void machine_set_cpu_numa_node(MachineState *machine,
>                  continue;
>          }
>  
> +        if (props->has_die_id && props->die_id != slot->props.die_id) {
> +                continue;
> +        }
> +
>          if (props->has_socket_id && props->socket_id != slot->props.socket_id) {
>                  continue;
>          }
> @@ -925,6 +934,9 @@ static char *cpu_slot_to_string(const CPUArchId *cpu)
>      if (cpu->props.has_socket_id) {
>          g_string_append_printf(s, "socket-id: %"PRId64, cpu->props.socket_id);
>      }
> +    if (cpu->props.has_die_id) {
> +        g_string_append_printf(s, "die-id: %"PRId64, cpu->props.die_id);
> +    }
>      if (cpu->props.has_core_id) {
>          if (s->len) {
>              g_string_append_printf(s, ", ");
> diff --git a/hw/i386/pc.c b/hw/i386/pc.c
> index 83ab53c814..00be2463af 100644
> --- a/hw/i386/pc.c
> +++ b/hw/i386/pc.c
> @@ -2321,6 +2321,10 @@ static void pc_cpu_pre_plug(HotplugHandler *hotplug_dev,
>              error_setg(errp, "Invalid CPU socket-id: %u must be in range 0:%u",
>                         cpu->socket_id, max_socket);
>              return;
> +        } else if (cpu->die_id > max_socket) {
> +            error_setg(errp, "Invalid CPU die-id: %u must be in range 0:%u",
> +                       cpu->die_id, max_socket);
> +            return;

Can you explain why the die_id is related to max_socket?
I'd assumed you could have a 2 socket system where each socket has 4
dies.

However, for the HMP side of it:


Acked-by: Dr. David Alan Gilbert <dgilbert@redhat.com>

>          }
>          if (cpu->core_id < 0) {
>              error_setg(errp, "CPU core-id is not set");
> @@ -2378,6 +2382,13 @@ static void pc_cpu_pre_plug(HotplugHandler *hotplug_dev,
>      }
>      cpu->socket_id = topo.pkg_id;
>  
> +    if (cpu->die_id != -1 && cpu->die_id != topo.die_id) {
> +        error_setg(errp, "property die-id: %u doesn't match set apic-id:"
> +            " 0x%x (die-id: %u)", cpu->die_id, cpu->apic_id, topo.die_id);
> +        return;
> +    }
> +    cpu->die_id = topo.die_id;
> +
>      if (cpu->core_id != -1 && cpu->core_id != topo.core_id) {
>          error_setg(errp, "property core-id: %u doesn't match set apic-id:"
>              " 0x%x (core-id: %u)", cpu->core_id, cpu->apic_id, topo.core_id);
> -- 
> 2.21.0
> 
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK

Re: [Qemu-devel] [PATCH v2 2/5] i386/cpu: Consolidate die-id validity in smp context

[Like Xu]

On 2019/5/22 1:12, Dr. David Alan Gilbert wrote:
> * Like Xu (like.xu@linux.intel.com) wrote:
>> Following the legacy smp check rules, the die_id validity is added to
>> the same contexts as leagcy smp variables such as hmp_hotpluggable_cpus(),
>> machine_set_cpu_numa_node(), cpu_slot_to_string() and pc_cpu_pre_plug().
>>
>> Signed-off-by: Like Xu <like.xu@linux.intel.com>
>> ---
>>   hmp.c             |  3 +++
>>   hw/core/machine.c | 12 ++++++++++++
>>   hw/i386/pc.c      | 11 +++++++++++
>>   3 files changed, 26 insertions(+)
>>
>> diff --git a/hmp.c b/hmp.c
>> index 56a3ed7375..7deb7b7226 100644
>> --- a/hmp.c
>> +++ b/hmp.c
>> @@ -3112,6 +3112,9 @@ void hmp_hotpluggable_cpus(Monitor *mon, const QDict *qdict)
>>           if (c->has_socket_id) {
>>               monitor_printf(mon, "    socket-id: \"%" PRIu64 "\"\n", c->socket_id);
>>           }
>> +        if (c->has_die_id) {
>> +            monitor_printf(mon, "    die-id: \"%" PRIu64 "\"\n", c->die_id);
>> +        }
>>           if (c->has_core_id) {
>>               monitor_printf(mon, "    core-id: \"%" PRIu64 "\"\n", c->core_id);
>>           }
>> diff --git a/hw/core/machine.c b/hw/core/machine.c
>> index 5d046a43e3..5116429732 100644
>> --- a/hw/core/machine.c
>> +++ b/hw/core/machine.c
>> @@ -659,6 +659,11 @@ void machine_set_cpu_numa_node(MachineState *machine,
>>               return;
>>           }
>>   
>> +        if (props->has_die_id && !slot->props.has_die_id) {
>> +            error_setg(errp, "die-id is not supported");
>> +            return;
>> +        }
>> +
>>           /* skip slots with explicit mismatch */
>>           if (props->has_thread_id && props->thread_id != slot->props.thread_id) {
>>                   continue;
>> @@ -668,6 +673,10 @@ void machine_set_cpu_numa_node(MachineState *machine,
>>                   continue;
>>           }
>>   
>> +        if (props->has_die_id && props->die_id != slot->props.die_id) {
>> +                continue;
>> +        }
>> +
>>           if (props->has_socket_id && props->socket_id != slot->props.socket_id) {
>>                   continue;
>>           }
>> @@ -925,6 +934,9 @@ static char *cpu_slot_to_string(const CPUArchId *cpu)
>>       if (cpu->props.has_socket_id) {
>>           g_string_append_printf(s, "socket-id: %"PRId64, cpu->props.socket_id);
>>       }
>> +    if (cpu->props.has_die_id) {
>> +        g_string_append_printf(s, "die-id: %"PRId64, cpu->props.die_id);
>> +    }
>>       if (cpu->props.has_core_id) {
>>           if (s->len) {
>>               g_string_append_printf(s, ", ");
>> diff --git a/hw/i386/pc.c b/hw/i386/pc.c
>> index 83ab53c814..00be2463af 100644
>> --- a/hw/i386/pc.c
>> +++ b/hw/i386/pc.c
>> @@ -2321,6 +2321,10 @@ static void pc_cpu_pre_plug(HotplugHandler *hotplug_dev,
>>               error_setg(errp, "Invalid CPU socket-id: %u must be in range 0:%u",
>>                          cpu->socket_id, max_socket);
>>               return;
>> +        } else if (cpu->die_id > max_socket) {
>> +            error_setg(errp, "Invalid CPU die-id: %u must be in range 0:%u",
>> +                       cpu->die_id, max_socket);
>> +            return;
> 
> Can you explain why the die_id is related to max_socket?
> I'd assumed you could have a 2 socket system where each socket has 4
> dies.

Dr David,thanks for your comments and sorry for the slow reply.

You're right about this and the check rule for cpu->die_id in 
pc_cpu_pre_plug() should be:

	"else if (cpu->die_id > (pcms->smp_dies - 1))"

> 
> However, for the HMP side of it:
> 
> 
> Acked-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
> 
>>           }
>>           if (cpu->core_id < 0) {
>>               error_setg(errp, "CPU core-id is not set");
>> @@ -2378,6 +2382,13 @@ static void pc_cpu_pre_plug(HotplugHandler *hotplug_dev,
>>       }
>>       cpu->socket_id = topo.pkg_id;
>>   
>> +    if (cpu->die_id != -1 && cpu->die_id != topo.die_id) {
>> +        error_setg(errp, "property die-id: %u doesn't match set apic-id:"
>> +            " 0x%x (die-id: %u)", cpu->die_id, cpu->apic_id, topo.die_id);
>> +        return;
>> +    }
>> +    cpu->die_id = topo.die_id;
>> +
>>       if (cpu->core_id != -1 && cpu->core_id != topo.core_id) {
>>           error_setg(errp, "property core-id: %u doesn't match set apic-id:"
>>               " 0x%x (core-id: %u)", cpu->core_id, cpu->apic_id, topo.core_id);
>> -- 
>> 2.21.0
>>
> --
> Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
> 
> 

Re: [Qemu-devel] [PATCH v2 3/5] vl.c: Add -smp, dies=* command line support and update -smp doc

[Eduardo Habkost]

On Tue, May 21, 2019 at 12:50:54AM +0800, Like Xu wrote:
> For PC target, users could configure the number of dies per one package
> via command line with this patch, such as "-smp dies=2,cores=4".
> 
> A new pc-specified pc_smp_parse() is introduced and to keep the interface
> consistent, refactoring legacy smp_parse() to __smp_parse() is necessary.
> 
> The parsing rules of new cpu-topology model obey the same restrictions/logic
> as the legacy socket/core/thread model especially on missing values computing.
> 
> Signed-off-by: Like Xu <like.xu@linux.intel.com>
> ---
>  qemu-options.hx | 17 +++++-----
>  vl.c            | 89 ++++++++++++++++++++++++++++++++++++++++++++++++-
>  2 files changed, 97 insertions(+), 9 deletions(-)
> 
> diff --git a/qemu-options.hx b/qemu-options.hx
> index 5daa5a8fb0..7fad5b50ff 100644
> --- a/qemu-options.hx
> +++ b/qemu-options.hx
> @@ -138,25 +138,26 @@ no incompatible TCG features have been enabled (e.g. icount/replay).
>  ETEXI
>  
>  DEF("smp", HAS_ARG, QEMU_OPTION_smp,
> -    "-smp [cpus=]n[,maxcpus=cpus][,cores=cores][,threads=threads][,sockets=sockets]\n"
> +    "-smp [cpus=]n[,maxcpus=cpus][,cores=cores][,threads=threads][,dies=dies][,sockets=sockets]\n"
>      "                set the number of CPUs to 'n' [default=1]\n"
>      "                maxcpus= maximum number of total cpus, including\n"
>      "                offline CPUs for hotplug, etc\n"
> -    "                cores= number of CPU cores on one socket\n"
> +    "                cores= number of CPU cores on one socket (for PC, it's on one die)\n"
>      "                threads= number of threads on one CPU core\n"
> +    "                dies= number of CPU dies on one socket (for PC only)\n"
>      "                sockets= number of discrete sockets in the system\n",
>          QEMU_ARCH_ALL)
>  STEXI
> -@item -smp [cpus=]@var{n}[,cores=@var{cores}][,threads=@var{threads}][,sockets=@var{sockets}][,maxcpus=@var{maxcpus}]
> +@item -smp [cpus=]@var{n}[,cores=@var{cores}][,threads=@var{threads}][,dies=dies][,sockets=@var{sockets}][,maxcpus=@var{maxcpus}]
>  @findex -smp
>  Simulate an SMP system with @var{n} CPUs. On the PC target, up to 255
>  CPUs are supported. On Sparc32 target, Linux limits the number of usable CPUs
>  to 4.
> -For the PC target, the number of @var{cores} per socket, the number
> -of @var{threads} per cores and the total number of @var{sockets} can be
> -specified. Missing values will be computed. If any on the three values is
> -given, the total number of CPUs @var{n} can be omitted. @var{maxcpus}
> -specifies the maximum number of hotpluggable CPUs.
> +For the PC target, the number of @var{cores} per die, the number of @var{threads}
> +per cores, the number of @var{dies} per packages and the total number of
> +@var{sockets} can be specified. Missing values will be computed.
> +If any on the three values is given, the total number of CPUs @var{n} can be omitted.
> +@var{maxcpus} specifies the maximum number of hotpluggable CPUs.
>  ETEXI
>  
>  DEF("numa", HAS_ARG, QEMU_OPTION_numa,
> diff --git a/vl.c b/vl.c
> index 8d92e2d209..66b577f447 100644
> --- a/vl.c
> +++ b/vl.c
> @@ -63,6 +63,7 @@ int main(int argc, char **argv)
>  #include "sysemu/watchdog.h"
>  #include "hw/firmware/smbios.h"
>  #include "hw/acpi/acpi.h"
> +#include "hw/i386/pc.h"
>  #include "hw/xen/xen.h"
>  #include "hw/qdev.h"
>  #include "hw/loader.h"
> @@ -1248,6 +1249,9 @@ static QemuOptsList qemu_smp_opts = {
>          }, {
>              .name = "sockets",
>              .type = QEMU_OPT_NUMBER,
> +        }, {
> +            .name = "dies",
> +            .type = QEMU_OPT_NUMBER,
>          }, {
>              .name = "cores",
>              .type = QEMU_OPT_NUMBER,
> @@ -1262,7 +1266,7 @@ static QemuOptsList qemu_smp_opts = {
>      },
>  };
>  
> -static void smp_parse(QemuOpts *opts)
> +static void __smp_parse(QemuOpts *opts)
>  {
>      if (opts) {
>          unsigned cpus    = qemu_opt_get_number(opts, "cpus", 0);
> @@ -1334,6 +1338,89 @@ static void smp_parse(QemuOpts *opts)
>      }
>  }
>  
> +static void pc_smp_parse(QemuOpts *opts)
> +{
> +    PCMachineState *pcms = (PCMachineState *)
> +        object_dynamic_cast(OBJECT(current_machine), TYPE_PC_MACHINE);
> +
> +    unsigned cpus    = qemu_opt_get_number(opts, "cpus", 0);
> +    unsigned sockets = qemu_opt_get_number(opts, "sockets", 0);
> +    unsigned dies = qemu_opt_get_number(opts, "dies", 1);
> +    unsigned cores   = qemu_opt_get_number(opts, "cores", 0);
> +    unsigned threads = qemu_opt_get_number(opts, "threads", 0);
> +
> +    /* compute missing values, prefer sockets over cores over threads */
> +    if (cpus == 0 || sockets == 0) {
> +        cores = cores > 0 ? cores : 1;
> +        threads = threads > 0 ? threads : 1;
> +        if (cpus == 0) {
> +            sockets = sockets > 0 ? sockets : 1;
> +            cpus = cores * threads * dies * sockets;
> +        } else {
> +            current_machine->smp.max_cpus =
> +                    qemu_opt_get_number(opts, "maxcpus", cpus);
> +            sockets = current_machine->smp.max_cpus / (cores * threads * dies);
> +        }
> +    } else if (cores == 0) {
> +        threads = threads > 0 ? threads : 1;
> +        cores = cpus / (sockets * dies * threads);
> +        cores = cores > 0 ? cores : 1;
> +    } else if (threads == 0) {
> +        threads = cpus / (cores * dies * sockets);
> +        threads = threads > 0 ? threads : 1;
> +    } else if (sockets * dies * cores * threads < cpus) {
> +        error_report("cpu topology: "
> +                        "sockets (%u) * dies (%u) * cores (%u) * threads (%u) < "
> +                        "smp_cpus (%u)",
> +                        sockets, dies, cores, threads, cpus);
> +        exit(1);
> +    }
> +
> +    current_machine->smp.max_cpus =
> +            qemu_opt_get_number(opts, "maxcpus", cpus);
> +
> +    if (current_machine->smp.max_cpus < cpus) {
> +        error_report("maxcpus must be equal to or greater than smp");
> +        exit(1);
> +    }
> +
> +    if (sockets * dies * cores * threads > current_machine->smp.max_cpus) {
> +        error_report("cpu topology: "
> +                        "sockets (%u) * dies (%u) * cores (%u) * threads (%u) > "
> +                        "maxcpus (%u)",
> +                        sockets, dies, cores, threads,
> +                        current_machine->smp.max_cpus);
> +        exit(1);
> +    }
> +
> +    if (sockets * dies * cores * threads != current_machine->smp.max_cpus) {
> +        warn_report("Invalid CPU topology deprecated: "
> +                    "sockets (%u) * dies (%u) * cores (%u) * threads (%u) "
> +                    "!= maxcpus (%u)",
> +                    sockets, dies, cores, threads,
> +                    current_machine->smp.max_cpus);
> +    }
> +
> +    current_machine->smp.cpus = cpus;
> +    current_machine->smp.cores = cores;
> +    current_machine->smp.threads = threads;
> +    pcms->smp_dies = dies;
> +
> +    if (current_machine->smp.cpus > 1) {
> +        Error *blocker = NULL;
> +        error_setg(&blocker, QERR_REPLAY_NOT_SUPPORTED, "smp");
> +        replay_add_blocker(blocker);
> +    }
> +}
> +
> +static void smp_parse(QemuOpts *opts)
> +{
> +    if (object_dynamic_cast(OBJECT(current_machine), TYPE_PC_MACHINE))
> +        pc_smp_parse(opts);

This is not exactly what I meant when I suggested creating a
pc_smp_parse() function.

The idea was to place the PC-specific function inside
hw/i386/pc.c, and have a MachineClass::smp_parse function pointer
that machine types could override.

The existing generic smp_parse() code, on the other hand, could
be moved to hw/core/machine.c, and be the default implementation
of MachineClass::smp_parse.

Suggestion: If you first create a copy of smp_parse() in one
patch, and then add CPU die support in another patch, it will be
much easier to review the changes you added to the pc_smp_parse()
code.


> +    else
> +        __smp_parse(opts);
> +}
> +
>  static void realtime_init(void)
>  {
>      if (enable_mlock) {
> -- 
> 2.21.0
> 
> 

-- 
Eduardo

Re: [Qemu-devel] [PATCH v2 1/5] target/i386: Add cpu die-level topology support for X86CPU

[Eduardo Habkost]

On Tue, May 21, 2019 at 12:50:52AM +0800, Like Xu wrote:
> The die-level as the first PC-specific cpu topology is added to the
> leagcy cpu topology model which only covers sockets/cores/threads.
> 
> In the new model with die-level support, the total number of logical
> processors (including offline) on board will be calculated as:
> 
>      #cpus = #sockets * #dies * #cores * #threads
> 
> and considering compatibility, the default value for #dies is 1.
> 
> A new set of die-related variables are added in smp context and the
> CPUX86State.nr_dies is assigned in x86_cpu_initfn() from PCMachineState.
> 
> Signed-off-by: Like Xu <like.xu@linux.intel.com>
> ---
>  hw/i386/pc.c               | 3 +++
>  include/hw/i386/pc.h       | 2 ++
>  include/hw/i386/topology.h | 2 ++
>  qapi/misc.json             | 6 ++++--
>  target/i386/cpu.c          | 9 +++++++++
>  target/i386/cpu.h          | 3 +++
>  6 files changed, 23 insertions(+), 2 deletions(-)
> 
> diff --git a/hw/i386/pc.c b/hw/i386/pc.c
> index 896c22e32e..83ab53c814 100644
> --- a/hw/i386/pc.c
> +++ b/hw/i386/pc.c
> @@ -2341,6 +2341,7 @@ static void pc_cpu_pre_plug(HotplugHandler *hotplug_dev,
>  
>          topo.pkg_id = cpu->socket_id;
>          topo.core_id = cpu->core_id;
> +        topo.die_id = cpu->die_id;
>          topo.smt_id = cpu->thread_id;
>          cpu->apic_id = apicid_from_topo_ids(smp_cores, smp_threads, &topo);
>      }
> @@ -2692,6 +2693,8 @@ static const CPUArchIdList *pc_possible_cpu_arch_ids(MachineState *ms)
>                                   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;
> +        ms->possible_cpus->cpus[i].props.die_id = topo.die_id;
>          ms->possible_cpus->cpus[i].props.has_core_id = true;
>          ms->possible_cpus->cpus[i].props.core_id = topo.core_id;
>          ms->possible_cpus->cpus[i].props.has_thread_id = true;
> diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
> index ce3c22951e..b5faf2ede9 100644
> --- a/include/hw/i386/pc.h
> +++ b/include/hw/i386/pc.h
> @@ -24,6 +24,7 @@
>   * PCMachineState:
>   * @acpi_dev: link to ACPI PM device that performs ACPI hotplug handling
>   * @boot_cpus: number of present VCPUs
> + * @smp_dies: number of dies per one package
>   */
>  struct PCMachineState {
>      /*< private >*/
> @@ -59,6 +60,7 @@ struct PCMachineState {
>      bool apic_xrupt_override;
>      unsigned apic_id_limit;
>      uint16_t boot_cpus;
> +    unsigned smp_dies;
>  
>      /* NUMA information: */
>      uint64_t numa_nodes;
> diff --git a/include/hw/i386/topology.h b/include/hw/i386/topology.h
> index 1ebaee0f76..7f80498eb3 100644
> --- a/include/hw/i386/topology.h
> +++ b/include/hw/i386/topology.h
> @@ -47,6 +47,7 @@ typedef uint32_t apic_id_t;
>  
>  typedef struct X86CPUTopoInfo {
>      unsigned pkg_id;
> +    unsigned die_id;
>      unsigned core_id;
>      unsigned smt_id;
>  } X86CPUTopoInfo;
> @@ -130,6 +131,7 @@ static inline void x86_topo_ids_from_apicid(apic_id_t apicid,
>      topo->core_id = (apicid >> apicid_core_offset(nr_cores, nr_threads)) &
>                     ~(0xFFFFFFFFUL << apicid_core_width(nr_cores, nr_threads));
>      topo->pkg_id = apicid >> apicid_pkg_offset(nr_cores, nr_threads);
> +    topo->die_id = -1;
>  }
>  
>  /* Make APIC ID for the CPU 'cpu_index'
> diff --git a/qapi/misc.json b/qapi/misc.json
> index 8b3ca4fdd3..cd236c89b3 100644
> --- a/qapi/misc.json
> +++ b/qapi/misc.json
> @@ -2924,10 +2924,11 @@
>  #
>  # @node-id: NUMA node ID the CPU belongs to
>  # @socket-id: socket number within node/board the CPU belongs to
> -# @core-id: core number within socket the CPU belongs to
> +# @die-id: die number within node/board the CPU belongs to (Since 4.1)
> +# @core-id: core number within die the CPU belongs to
>  # @thread-id: thread number within core the CPU belongs to
>  #
> -# Note: currently there are 4 properties that could be present
> +# Note: currently there are 5 properties that could be present
>  # but management should be prepared to pass through other
>  # properties with device_add command to allow for future
>  # interface extension. This also requires the filed names to be kept in
> @@ -2938,6 +2939,7 @@
>  { 'struct': 'CpuInstanceProperties',
>    'data': { '*node-id': 'int',
>              '*socket-id': 'int',
> +            '*die-id': 'int',
>              '*core-id': 'int',
>              '*thread-id': 'int'
>    }
> diff --git a/target/i386/cpu.c b/target/i386/cpu.c
> index 9a93dd8be7..9bd35b4965 100644
> --- a/target/i386/cpu.c
> +++ b/target/i386/cpu.c
> @@ -55,6 +55,7 @@
>  #include "hw/xen/xen.h"
>  #include "hw/i386/apic_internal.h"
>  #include "hw/boards.h"
> +#include "hw/i386/pc.h"

Now we have a circular dependency between target/i386/cpu.c and
hw/i386/pc.c.

>  #endif
>  
>  #include "disas/capstone.h"
> @@ -5595,7 +5596,13 @@ static void x86_cpu_initfn(Object *obj)
>      X86CPUClass *xcc = X86_CPU_GET_CLASS(obj);
>      CPUX86State *env = &cpu->env;
>      FeatureWord w;
> +#ifndef CONFIG_USER_ONLY
> +    MachineState *machine = MACHINE(qdev_get_machine());
> +    PCMachineState *pcms = (PCMachineState *)
> +        object_dynamic_cast(OBJECT(machine), TYPE_PC_MACHINE);
>  
> +    env->nr_dies = pcms ? pcms->smp_dies : 1;

If this is PC-specific, the best place to do it is probably
pc_new_cpu() and pc_cpu_pre_plug().  x86_cpu_initfn() could just
set nr_dies=1 by default.


> +#endif
>      cs->env_ptr = env;
>  
>      object_property_add(obj, "family", "int",
> @@ -5812,11 +5819,13 @@ static Property x86_cpu_properties[] = {
>      DEFINE_PROP_UINT32("apic-id", X86CPU, apic_id, 0),
>      DEFINE_PROP_INT32("thread-id", X86CPU, thread_id, 0),
>      DEFINE_PROP_INT32("core-id", X86CPU, core_id, 0),
> +    DEFINE_PROP_INT32("die-id", X86CPU, die_id, 0),
>      DEFINE_PROP_INT32("socket-id", X86CPU, socket_id, 0),
>  #else
>      DEFINE_PROP_UINT32("apic-id", X86CPU, apic_id, UNASSIGNED_APIC_ID),
>      DEFINE_PROP_INT32("thread-id", X86CPU, thread_id, -1),
>      DEFINE_PROP_INT32("core-id", X86CPU, core_id, -1),
> +    DEFINE_PROP_INT32("die-id", X86CPU, die_id, -1),
>      DEFINE_PROP_INT32("socket-id", X86CPU, socket_id, -1),
>  #endif
>      DEFINE_PROP_INT32("node-id", X86CPU, node_id, CPU_UNSET_NUMA_NODE_ID),
> diff --git a/target/i386/cpu.h b/target/i386/cpu.h
> index fce6660bac..d5f2a60ff5 100644
> --- a/target/i386/cpu.h
> +++ b/target/i386/cpu.h
> @@ -1361,6 +1361,8 @@ typedef struct CPUX86State {
>      uint64_t xss;
>  
>      TPRAccess tpr_access_type;
> +
> +    unsigned nr_dies;
>  } CPUX86State;
>  
>  struct kvm_msrs;
> @@ -1484,6 +1486,7 @@ struct X86CPU {
>  
>      int32_t node_id; /* NUMA node this CPU belongs to */
>      int32_t socket_id;
> +    int32_t die_id;
>      int32_t core_id;
>      int32_t thread_id;
>  
> -- 
> 2.21.0
> 
> 

-- 
Eduardo

Re: [Qemu-devel] [PATCH v2 1/5] target/i386: Add cpu die-level topology support for X86CPU

[Eduardo Habkost]

On Tue, May 21, 2019 at 12:50:52AM +0800, Like Xu wrote:
> The die-level as the first PC-specific cpu topology is added to the
> leagcy cpu topology model which only covers sockets/cores/threads.
> 
> In the new model with die-level support, the total number of logical
> processors (including offline) on board will be calculated as:
> 
>      #cpus = #sockets * #dies * #cores * #threads
> 
> and considering compatibility, the default value for #dies is 1.
> 
> A new set of die-related variables are added in smp context and the
> CPUX86State.nr_dies is assigned in x86_cpu_initfn() from PCMachineState.
> 
> Signed-off-by: Like Xu <like.xu@linux.intel.com>
> ---
>  hw/i386/pc.c               | 3 +++
>  include/hw/i386/pc.h       | 2 ++
>  include/hw/i386/topology.h | 2 ++
>  qapi/misc.json             | 6 ++++--
>  target/i386/cpu.c          | 9 +++++++++
>  target/i386/cpu.h          | 3 +++
>  6 files changed, 23 insertions(+), 2 deletions(-)
> 
> diff --git a/hw/i386/pc.c b/hw/i386/pc.c
> index 896c22e32e..83ab53c814 100644
> --- a/hw/i386/pc.c
> +++ b/hw/i386/pc.c
> @@ -2341,6 +2341,7 @@ static void pc_cpu_pre_plug(HotplugHandler *hotplug_dev,
>  
>          topo.pkg_id = cpu->socket_id;
>          topo.core_id = cpu->core_id;
> +        topo.die_id = cpu->die_id;
>          topo.smt_id = cpu->thread_id;
>          cpu->apic_id = apicid_from_topo_ids(smp_cores, smp_threads, &topo);
>      }
> @@ -2692,6 +2693,8 @@ static const CPUArchIdList *pc_possible_cpu_arch_ids(MachineState *ms)
>                                   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;
> +        ms->possible_cpus->cpus[i].props.die_id = topo.die_id;
>          ms->possible_cpus->cpus[i].props.has_core_id = true;
>          ms->possible_cpus->cpus[i].props.core_id = topo.core_id;
>          ms->possible_cpus->cpus[i].props.has_thread_id = true;
> diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
> index ce3c22951e..b5faf2ede9 100644
> --- a/include/hw/i386/pc.h
> +++ b/include/hw/i386/pc.h
> @@ -24,6 +24,7 @@
>   * PCMachineState:
>   * @acpi_dev: link to ACPI PM device that performs ACPI hotplug handling
>   * @boot_cpus: number of present VCPUs
> + * @smp_dies: number of dies per one package
>   */
>  struct PCMachineState {
>      /*< private >*/
> @@ -59,6 +60,7 @@ struct PCMachineState {
>      bool apic_xrupt_override;
>      unsigned apic_id_limit;
>      uint16_t boot_cpus;
> +    unsigned smp_dies;
>  
>      /* NUMA information: */
>      uint64_t numa_nodes;
> diff --git a/include/hw/i386/topology.h b/include/hw/i386/topology.h
> index 1ebaee0f76..7f80498eb3 100644
> --- a/include/hw/i386/topology.h
> +++ b/include/hw/i386/topology.h
> @@ -47,6 +47,7 @@ typedef uint32_t apic_id_t;
>  
>  typedef struct X86CPUTopoInfo {
>      unsigned pkg_id;
> +    unsigned die_id;

Isn't it better to add this field only on patch 4/5?

>      unsigned core_id;
>      unsigned smt_id;
>  } X86CPUTopoInfo;
> @@ -130,6 +131,7 @@ static inline void x86_topo_ids_from_apicid(apic_id_t apicid,
>      topo->core_id = (apicid >> apicid_core_offset(nr_cores, nr_threads)) &
>                     ~(0xFFFFFFFFUL << apicid_core_width(nr_cores, nr_threads));
>      topo->pkg_id = apicid >> apicid_pkg_offset(nr_cores, nr_threads);
> +    topo->die_id = -1;

Why are you setting die_id = -1 here?

If die_id isn't valid yet, isn't it better to keep has_die_id =
false at pc_possible_cpu_arch_ids() above, and set has_die_id =
true only on patch 4/5?

>  }
>  
>  /* Make APIC ID for the CPU 'cpu_index'
> diff --git a/qapi/misc.json b/qapi/misc.json
> index 8b3ca4fdd3..cd236c89b3 100644
> --- a/qapi/misc.json
> +++ b/qapi/misc.json
> @@ -2924,10 +2924,11 @@
>  #
>  # @node-id: NUMA node ID the CPU belongs to
>  # @socket-id: socket number within node/board the CPU belongs to
> -# @core-id: core number within socket the CPU belongs to
> +# @die-id: die number within node/board the CPU belongs to (Since 4.1)
> +# @core-id: core number within die the CPU belongs to
>  # @thread-id: thread number within core the CPU belongs to
>  #
> -# Note: currently there are 4 properties that could be present
> +# Note: currently there are 5 properties that could be present
>  # but management should be prepared to pass through other
>  # properties with device_add command to allow for future
>  # interface extension. This also requires the filed names to be kept in
> @@ -2938,6 +2939,7 @@
>  { 'struct': 'CpuInstanceProperties',
>    'data': { '*node-id': 'int',
>              '*socket-id': 'int',
> +            '*die-id': 'int',
>              '*core-id': 'int',
>              '*thread-id': 'int'
>    }
> diff --git a/target/i386/cpu.c b/target/i386/cpu.c
> index 9a93dd8be7..9bd35b4965 100644
> --- a/target/i386/cpu.c
> +++ b/target/i386/cpu.c
> @@ -55,6 +55,7 @@
>  #include "hw/xen/xen.h"
>  #include "hw/i386/apic_internal.h"
>  #include "hw/boards.h"
> +#include "hw/i386/pc.h"
>  #endif
>  
>  #include "disas/capstone.h"
> @@ -5595,7 +5596,13 @@ static void x86_cpu_initfn(Object *obj)
>      X86CPUClass *xcc = X86_CPU_GET_CLASS(obj);
>      CPUX86State *env = &cpu->env;
>      FeatureWord w;
> +#ifndef CONFIG_USER_ONLY
> +    MachineState *machine = MACHINE(qdev_get_machine());
> +    PCMachineState *pcms = (PCMachineState *)
> +        object_dynamic_cast(OBJECT(machine), TYPE_PC_MACHINE);
>  
> +    env->nr_dies = pcms ? pcms->smp_dies : 1;
> +#endif
>      cs->env_ptr = env;
>  
>      object_property_add(obj, "family", "int",
> @@ -5812,11 +5819,13 @@ static Property x86_cpu_properties[] = {
>      DEFINE_PROP_UINT32("apic-id", X86CPU, apic_id, 0),
>      DEFINE_PROP_INT32("thread-id", X86CPU, thread_id, 0),
>      DEFINE_PROP_INT32("core-id", X86CPU, core_id, 0),
> +    DEFINE_PROP_INT32("die-id", X86CPU, die_id, 0),
>      DEFINE_PROP_INT32("socket-id", X86CPU, socket_id, 0),
>  #else
>      DEFINE_PROP_UINT32("apic-id", X86CPU, apic_id, UNASSIGNED_APIC_ID),
>      DEFINE_PROP_INT32("thread-id", X86CPU, thread_id, -1),
>      DEFINE_PROP_INT32("core-id", X86CPU, core_id, -1),
> +    DEFINE_PROP_INT32("die-id", X86CPU, die_id, -1),
>      DEFINE_PROP_INT32("socket-id", X86CPU, socket_id, -1),
>  #endif
>      DEFINE_PROP_INT32("node-id", X86CPU, node_id, CPU_UNSET_NUMA_NODE_ID),
> diff --git a/target/i386/cpu.h b/target/i386/cpu.h
> index fce6660bac..d5f2a60ff5 100644
> --- a/target/i386/cpu.h
> +++ b/target/i386/cpu.h
> @@ -1361,6 +1361,8 @@ typedef struct CPUX86State {
>      uint64_t xss;
>  
>      TPRAccess tpr_access_type;
> +
> +    unsigned nr_dies;
>  } CPUX86State;
>  
>  struct kvm_msrs;
> @@ -1484,6 +1486,7 @@ struct X86CPU {
>  
>      int32_t node_id; /* NUMA node this CPU belongs to */
>      int32_t socket_id;
> +    int32_t die_id;
>      int32_t core_id;
>      int32_t thread_id;
>  
> -- 
> 2.21.0
> 
> 

-- 
Eduardo

Re: [Qemu-devel] [PATCH v2 1/5] target/i386: Add cpu die-level topology support for X86CPU

[Like Xu]

On 2019/6/6 11:32, Eduardo Habkost wrote:
> On Tue, May 21, 2019 at 12:50:52AM +0800, Like Xu wrote:
>> The die-level as the first PC-specific cpu topology is added to the
>> leagcy cpu topology model which only covers sockets/cores/threads.
>>
>> In the new model with die-level support, the total number of logical
>> processors (including offline) on board will be calculated as:
>>
>>       #cpus = #sockets * #dies * #cores * #threads
>>
>> and considering compatibility, the default value for #dies is 1.
>>
>> A new set of die-related variables are added in smp context and the
>> CPUX86State.nr_dies is assigned in x86_cpu_initfn() from PCMachineState.
>>
>> Signed-off-by: Like Xu <like.xu@linux.intel.com>
>> ---
>>   hw/i386/pc.c               | 3 +++
>>   include/hw/i386/pc.h       | 2 ++
>>   include/hw/i386/topology.h | 2 ++
>>   qapi/misc.json             | 6 ++++--
>>   target/i386/cpu.c          | 9 +++++++++
>>   target/i386/cpu.h          | 3 +++
>>   6 files changed, 23 insertions(+), 2 deletions(-)
>>
>> diff --git a/hw/i386/pc.c b/hw/i386/pc.c
>> index 896c22e32e..83ab53c814 100644
>> --- a/hw/i386/pc.c
>> +++ b/hw/i386/pc.c
>> @@ -2341,6 +2341,7 @@ static void pc_cpu_pre_plug(HotplugHandler *hotplug_dev,
>>   
>>           topo.pkg_id = cpu->socket_id;
>>           topo.core_id = cpu->core_id;
>> +        topo.die_id = cpu->die_id;
>>           topo.smt_id = cpu->thread_id;
>>           cpu->apic_id = apicid_from_topo_ids(smp_cores, smp_threads, &topo);
>>       }
>> @@ -2692,6 +2693,8 @@ static const CPUArchIdList *pc_possible_cpu_arch_ids(MachineState *ms)
>>                                    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;
>> +        ms->possible_cpus->cpus[i].props.die_id = topo.die_id;
>>           ms->possible_cpus->cpus[i].props.has_core_id = true;
>>           ms->possible_cpus->cpus[i].props.core_id = topo.core_id;
>>           ms->possible_cpus->cpus[i].props.has_thread_id = true;
>> diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
>> index ce3c22951e..b5faf2ede9 100644
>> --- a/include/hw/i386/pc.h
>> +++ b/include/hw/i386/pc.h
>> @@ -24,6 +24,7 @@
>>    * PCMachineState:
>>    * @acpi_dev: link to ACPI PM device that performs ACPI hotplug handling
>>    * @boot_cpus: number of present VCPUs
>> + * @smp_dies: number of dies per one package
>>    */
>>   struct PCMachineState {
>>       /*< private >*/
>> @@ -59,6 +60,7 @@ struct PCMachineState {
>>       bool apic_xrupt_override;
>>       unsigned apic_id_limit;
>>       uint16_t boot_cpus;
>> +    unsigned smp_dies;
>>   
>>       /* NUMA information: */
>>       uint64_t numa_nodes;
>> diff --git a/include/hw/i386/topology.h b/include/hw/i386/topology.h
>> index 1ebaee0f76..7f80498eb3 100644
>> --- a/include/hw/i386/topology.h
>> +++ b/include/hw/i386/topology.h
>> @@ -47,6 +47,7 @@ typedef uint32_t apic_id_t;
>>   
>>   typedef struct X86CPUTopoInfo {
>>       unsigned pkg_id;
>> +    unsigned die_id;
> 
> Isn't it better to add this field only on patch 4/5?
> 
>>       unsigned core_id;
>>       unsigned smt_id;
>>   } X86CPUTopoInfo;
>> @@ -130,6 +131,7 @@ static inline void x86_topo_ids_from_apicid(apic_id_t apicid,
>>       topo->core_id = (apicid >> apicid_core_offset(nr_cores, nr_threads)) &
>>                      ~(0xFFFFFFFFUL << apicid_core_width(nr_cores, nr_threads));
>>       topo->pkg_id = apicid >> apicid_pkg_offset(nr_cores, nr_threads);
>> +    topo->die_id = -1;
> 
> Why are you setting die_id = -1 here?

Hi Eduardo,thanks for your comments and support.

Would it be a better way to introduce all die related variables 
including has_die_id/nr_dies/cpu->die_id/topo.die_id/smp_dies in one 
patch for consistency check and backport convenient?

In this case the default value for topo->die_id would be 0 (for sure, 
one die per package) with has_die_id = false. Is that acceptable to you？

> 
> If die_id isn't valid yet, isn't it better to keep has_die_id =
> false at pc_possible_cpu_arch_ids() above, and set has_die_id =
> true only on patch 4/5?
> 
>>   }
>>   
>>   /* Make APIC ID for the CPU 'cpu_index'
>> diff --git a/qapi/misc.json b/qapi/misc.json
>> index 8b3ca4fdd3..cd236c89b3 100644
>> --- a/qapi/misc.json
>> +++ b/qapi/misc.json
>> @@ -2924,10 +2924,11 @@
>>   #
>>   # @node-id: NUMA node ID the CPU belongs to
>>   # @socket-id: socket number within node/board the CPU belongs to
>> -# @core-id: core number within socket the CPU belongs to
>> +# @die-id: die number within node/board the CPU belongs to (Since 4.1)
>> +# @core-id: core number within die the CPU belongs to
>>   # @thread-id: thread number within core the CPU belongs to
>>   #
>> -# Note: currently there are 4 properties that could be present
>> +# Note: currently there are 5 properties that could be present
>>   # but management should be prepared to pass through other
>>   # properties with device_add command to allow for future
>>   # interface extension. This also requires the filed names to be kept in
>> @@ -2938,6 +2939,7 @@
>>   { 'struct': 'CpuInstanceProperties',
>>     'data': { '*node-id': 'int',
>>               '*socket-id': 'int',
>> +            '*die-id': 'int',
>>               '*core-id': 'int',
>>               '*thread-id': 'int'
>>     }
>> diff --git a/target/i386/cpu.c b/target/i386/cpu.c
>> index 9a93dd8be7..9bd35b4965 100644
>> --- a/target/i386/cpu.c
>> +++ b/target/i386/cpu.c
>> @@ -55,6 +55,7 @@
>>   #include "hw/xen/xen.h"
>>   #include "hw/i386/apic_internal.h"
>>   #include "hw/boards.h"
>> +#include "hw/i386/pc.h"
>>   #endif
>>   
>>   #include "disas/capstone.h"
>> @@ -5595,7 +5596,13 @@ static void x86_cpu_initfn(Object *obj)
>>       X86CPUClass *xcc = X86_CPU_GET_CLASS(obj);
>>       CPUX86State *env = &cpu->env;
>>       FeatureWord w;
>> +#ifndef CONFIG_USER_ONLY
>> +    MachineState *machine = MACHINE(qdev_get_machine());
>> +    PCMachineState *pcms = (PCMachineState *)
>> +        object_dynamic_cast(OBJECT(machine), TYPE_PC_MACHINE);
>>   
>> +    env->nr_dies = pcms ? pcms->smp_dies : 1;
>> +#endif
>>       cs->env_ptr = env;
>>   
>>       object_property_add(obj, "family", "int",
>> @@ -5812,11 +5819,13 @@ static Property x86_cpu_properties[] = {
>>       DEFINE_PROP_UINT32("apic-id", X86CPU, apic_id, 0),
>>       DEFINE_PROP_INT32("thread-id", X86CPU, thread_id, 0),
>>       DEFINE_PROP_INT32("core-id", X86CPU, core_id, 0),
>> +    DEFINE_PROP_INT32("die-id", X86CPU, die_id, 0),
>>       DEFINE_PROP_INT32("socket-id", X86CPU, socket_id, 0),
>>   #else
>>       DEFINE_PROP_UINT32("apic-id", X86CPU, apic_id, UNASSIGNED_APIC_ID),
>>       DEFINE_PROP_INT32("thread-id", X86CPU, thread_id, -1),
>>       DEFINE_PROP_INT32("core-id", X86CPU, core_id, -1),
>> +    DEFINE_PROP_INT32("die-id", X86CPU, die_id, -1),
>>       DEFINE_PROP_INT32("socket-id", X86CPU, socket_id, -1),
>>   #endif
>>       DEFINE_PROP_INT32("node-id", X86CPU, node_id, CPU_UNSET_NUMA_NODE_ID),
>> diff --git a/target/i386/cpu.h b/target/i386/cpu.h
>> index fce6660bac..d5f2a60ff5 100644
>> --- a/target/i386/cpu.h
>> +++ b/target/i386/cpu.h
>> @@ -1361,6 +1361,8 @@ typedef struct CPUX86State {
>>       uint64_t xss;
>>   
>>       TPRAccess tpr_access_type;
>> +
>> +    unsigned nr_dies;
>>   } CPUX86State;
>>   
>>   struct kvm_msrs;
>> @@ -1484,6 +1486,7 @@ struct X86CPU {
>>   
>>       int32_t node_id; /* NUMA node this CPU belongs to */
>>       int32_t socket_id;
>> +    int32_t die_id;
>>       int32_t core_id;
>>       int32_t thread_id;
>>   
>> -- 
>> 2.21.0
>>
>>
>