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

[Qemu-devel] [PATCH v3 0/9] 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 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 a multi-dies guest and
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

Guest system could discover multi-die/package topology through CPUID.1F.
and its benefit is primarily for _reporting_ of the (virtual) CPU topology.
The guest kernel with multi-die/package support have no impact on its 
cache topology, NUMA topology, Linux scheduler, or system performance. 

==changelog==

v3:

- add a MachineClass::smp_parse function pointer
- place the PC-specific function inside hw/i386/pc.c
- introduce die_id in a separate patch with default value 0
- set env->nr_dies in pc_new_cpu() and pc_cpu_pre_plug()
- fix a circular dependency between target/i386/cpu.c and hw/i386/pc.c
- fix cpu->die_id check in pc_cpu_pre_plug()
- Based on "[PATCH v3 00/10] Refactor cpu topo into machine properties"
- Rebase to commit 219dca61ebf41625831d4f96a720852baf44b762

v2: https://patchwork.kernel.org/cover/10953191/

- 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 (9):
  i386: Add die-level cpu topology to x86CPU on PCMachine
  hw/i386: Adjust nr_dies with configured smp_dies for PCMachine
  i386/cpu: Consolidate die-id validity in smp context
  i386: Update new x86_apicid parsing rules with die_offset support
  tests/x86-cpuid: Update testcases in test_topo_bits() with multiple dies
  i386/cpu: Add CPUID.1F generation support for multi-dies PCMachine
  target/i386: Support multi-dies when host doesn't support CPUID.1F
  machine: Refactor smp_parse() in vl.c as MachineClass::smp_parse()
  vl.c: Add -smp, dies=* command line support and update doc

 hmp.c                      |   3 +
 hw/core/machine.c          |  89 ++++++++++++++++++++++
 hw/i386/pc.c               | 148 ++++++++++++++++++++++++++++++++-----
 include/hw/boards.h        |   5 ++
 include/hw/i386/pc.h       |   3 +
 include/hw/i386/topology.h |  76 +++++++++++++------
 qapi/misc.json             |   6 +-
 qemu-options.hx            |  17 +++--
 target/i386/cpu.c          |  53 +++++++++++--
 target/i386/cpu.h          |   7 ++
 target/i386/kvm.c          |  36 ++++++++-
 tests/test-x86-cpuid.c     |  84 +++++++++++----------
 vl.c                       |  78 ++-----------------
 13 files changed, 438 insertions(+), 167 deletions(-)

-- 
2.21.0

[Qemu-devel] [PATCH v3 1/9] i386: Add die-level cpu topology to x86CPU on PCMachine

[Like Xu]

The die-level as the first PC-specific cpu topology is added to the leagcy
cpu topology model, which has one die per package implicitly and only the
numbers of sockets/cores/threads are configurable.

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 would be
initialized to one in x86_cpu_initfn() and pc_machine_initfn().

Signed-off-by: Like Xu <like.xu@linux.intel.com>
---
 hw/i386/pc.c         | 9 +++++++--
 include/hw/i386/pc.h | 2 ++
 target/i386/cpu.c    | 1 +
 target/i386/cpu.h    | 2 ++
 4 files changed, 12 insertions(+), 2 deletions(-)

diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index 12c1e08b85..9e9a42f007 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -2308,9 +2308,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");
@@ -2620,6 +2624,7 @@ static void pc_machine_initfn(Object *obj)
     pcms->smbus_enabled = true;
     pcms->sata_enabled = true;
     pcms->pit_enabled = true;
+    pcms->smp_dies = 1;
 
     pc_system_flash_create(pcms);
 }
diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
index b260262640..fae9217e34 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/target/i386/cpu.c b/target/i386/cpu.c
index 23119699de..a16be205fe 100644
--- a/target/i386/cpu.c
+++ b/target/i386/cpu.c
@@ -5619,6 +5619,7 @@ static void x86_cpu_initfn(Object *obj)
     CPUX86State *env = &cpu->env;
     FeatureWord w;
 
+    env->nr_dies = 1;
     cpu_set_cpustate_pointers(cpu);
 
     object_property_add(obj, "family", "int",
diff --git a/target/i386/cpu.h b/target/i386/cpu.h
index edad6e1efb..5daa2eeafa 100644
--- a/target/i386/cpu.h
+++ b/target/i386/cpu.h
@@ -1349,6 +1349,8 @@ typedef struct CPUX86State {
     uint64_t xss;
 
     TPRAccess tpr_access_type;
+
+    unsigned nr_dies;
 } CPUX86State;
 
 struct kvm_msrs;
-- 
2.21.0

[Qemu-devel] [PATCH v3 2/9] hw/i386: Adjust nr_dies with configured smp_dies for PCMachine

[Like Xu]

To support multiple dies configuration on PCMachine, the best place to
set CPUX86State->nr_dies with requested PCMachineState->smp_dies is in
pc_new_cpu() and pc_cpu_pre_plug(). Refactoring pc_new_cpu() is applied
and redundant parameter "const char *typename" would be removed.

Suggested-by: Eduardo Habkost <ehabkost@redhat.com>
Signed-off-by: Like Xu <like.xu@linux.intel.com>
---
 hw/i386/pc.c | 16 +++++++++++-----
 1 file changed, 11 insertions(+), 5 deletions(-)

diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index 9e9a42f007..af2e95a1b9 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -1520,12 +1520,16 @@ void pc_acpi_smi_interrupt(void *opaque, int irq, int level)
     }
 }
 
-static void pc_new_cpu(const char *typename, int64_t apic_id, Error **errp)
+static void pc_new_cpu(PCMachineState *pcms, int64_t apic_id, Error **errp)
 {
     Object *cpu = NULL;
     Error *local_err = NULL;
+    CPUX86State *env = NULL;
 
-    cpu = object_new(typename);
+    cpu = object_new(MACHINE(pcms)->cpu_type);
+
+    env = &X86_CPU(cpu)->env;
+    env->nr_dies = pcms->smp_dies;
 
     object_property_set_uint(cpu, apic_id, "apic-id", &local_err);
     object_property_set_bool(cpu, true, "realized", &local_err);
@@ -1551,7 +1555,7 @@ void pc_hot_add_cpu(MachineState *ms, const int64_t id, Error **errp)
         return;
     }
 
-    pc_new_cpu(ms->cpu_type, apic_id, &local_err);
+    pc_new_cpu(PC_MACHINE(ms), apic_id, &local_err);
     if (local_err) {
         error_propagate(errp, local_err);
         return;
@@ -1576,8 +1580,7 @@ void pc_cpus_init(PCMachineState *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(possible_cpus->cpus[i].type, possible_cpus->cpus[i].arch_id,
-                   &error_fatal);
+        pc_new_cpu(pcms, possible_cpus->cpus[i].arch_id, &error_fatal);
     }
 }
 
@@ -2297,6 +2300,7 @@ static void pc_cpu_pre_plug(HotplugHandler *hotplug_dev,
     CPUArchId *cpu_slot;
     X86CPUTopoInfo topo;
     X86CPU *cpu = X86_CPU(dev);
+    CPUX86State *env = &cpu->env;
     MachineState *ms = MACHINE(hotplug_dev);
     PCMachineState *pcms = PC_MACHINE(hotplug_dev);
     unsigned int smp_cores = ms->smp.cores;
@@ -2308,6 +2312,8 @@ static void pc_cpu_pre_plug(HotplugHandler *hotplug_dev,
         return;
     }
 
+    env->nr_dies = pcms->smp_dies;
+
     /*
      * If APIC ID is not set,
      * set it based on socket/die/core/thread properties.
-- 
2.21.0

[Qemu-devel] [PATCH v3 3/9] i386/cpu: Consolidate die-id validity in smp context

[Like Xu]

The field die_id (default as 0) and has_die_id are introduced to X86CPU.
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().

Acked-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
Signed-off-by: Like Xu <like.xu@linux.intel.com>
---
 hmp.c                      |  3 +++
 hw/core/machine.c          | 12 ++++++++++++
 hw/i386/pc.c               | 14 ++++++++++++++
 include/hw/i386/topology.h |  2 ++
 qapi/misc.json             |  6 ++++--
 target/i386/cpu.c          |  2 ++
 target/i386/cpu.h          |  1 +
 7 files changed, 38 insertions(+), 2 deletions(-)

diff --git a/hmp.c b/hmp.c
index be5e345c6f..b567c86628 100644
--- a/hmp.c
+++ b/hmp.c
@@ -3113,6 +3113,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 f1a0f45f9c..9eeba448ed 100644
--- a/hw/core/machine.c
+++ b/hw/core/machine.c
@@ -679,6 +679,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;
@@ -688,6 +693,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;
         }
@@ -945,6 +954,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 af2e95a1b9..6e774c6c8e 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -2329,6 +2329,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 > pcms->smp_dies - 1) {
+            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");
@@ -2348,6 +2352,7 @@ static void pc_cpu_pre_plug(HotplugHandler *hotplug_dev,
         }
 
         topo.pkg_id = cpu->socket_id;
+        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(smp_cores, smp_threads, &topo);
@@ -2385,6 +2390,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);
@@ -2701,6 +2713,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/topology.h b/include/hw/i386/topology.h
index 1ebaee0f76..c9fb41588e 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 = 0;
 }
 
 /* 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 a16be205fe..0fc543096f 100644
--- a/target/i386/cpu.c
+++ b/target/i386/cpu.c
@@ -5845,11 +5845,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 5daa2eeafa..69495f0a8a 100644
--- a/target/i386/cpu.h
+++ b/target/i386/cpu.h
@@ -1475,6 +1475,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 v3 4/9] i386: Update new x86_apicid parsing rules with die_offset support

[Like Xu]

In new sockets/dies/cores/threads model, the apicid of logical cpu could
imply die level info of guest cpu topology thus x86_apicid_from_cpu_idx()
need to be refactored with #dies value, so does apicid_*_offset().

To keep semantic compatibility, the legacy pkg_offset which helps to
generate CPUIDs such as 0x3 for L3 cache should be mapping to die_offset.

Signed-off-by: Like Xu <like.xu@linux.intel.com>
---
 hw/i386/pc.c               | 29 ++++++++++-----
 include/hw/i386/topology.h | 76 +++++++++++++++++++++++++++-----------
 target/i386/cpu.c          | 13 ++++---
 3 files changed, 81 insertions(+), 37 deletions(-)

diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index 6e774c6c8e..b4dbd1064d 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -933,10 +933,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()) {
@@ -2355,18 +2356,21 @@ 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(smp_cores, smp_threads, &topo);
+        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, 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, 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",
+            topo.pkg_id, topo.die_id, topo.core_id, topo.smt_id,
+            cpu->apic_id, ms->possible_cpus->len - 1);
         return;
     }
 
@@ -2382,7 +2386,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, pcms->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);
@@ -2679,10 +2684,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;
 }
 
@@ -2690,6 +2697,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) {
         /*
@@ -2710,7 +2718,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 c9fb41588e..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 = 0;
+            ~(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 0fc543096f..09e20a2c3b 100644
--- a/target/i386/cpu.c
+++ b/target/i386/cpu.c
@@ -4245,7 +4245,7 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t index, uint32_t count,
 {
     X86CPU *cpu = env_archcpu(env);
     CPUState *cs = env_cpu(env);
-    uint32_t pkg_offset;
+    uint32_t die_offset;
     uint32_t limit;
     uint32_t signature[3];
 
@@ -4334,10 +4334,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;
                 }
@@ -4419,12 +4420,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;
-- 
2.21.0

[Qemu-devel] [PATCH v3 5/9] tests/x86-cpuid: Update testcases in test_topo_bits() with multiple dies

[Like Xu]

The corresponding topo_bits tests are updated to support die configurations.

Signed-off-by: Like Xu <like.xu@linux.intel.com>
---
 tests/test-x86-cpuid.c | 84 ++++++++++++++++++++++--------------------
 1 file changed, 45 insertions(+), 39 deletions(-)

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 v3 6/9] i386/cpu: Add CPUID.1F generation support for multi-dies 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.

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

diff --git a/target/i386/cpu.c b/target/i386/cpu.c
index 09e20a2c3b..127aff74a6 100644
--- a/target/i386/cpu.c
+++ b/target/i386/cpu.c
@@ -4437,6 +4437,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;
@@ -5890,6 +5926,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 69495f0a8a..0434dfb62a 100644
--- a/target/i386/cpu.h
+++ b/target/i386/cpu.h
@@ -726,6 +726,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)
@@ -1444,6 +1445,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..9b4da9b265 100644
--- a/target/i386/kvm.c
+++ b/target/i386/kvm.c
@@ -1081,6 +1081,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 +1092,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 +1108,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;
                 }
-- 
2.21.0

[Qemu-devel] [PATCH v3 7/9] target/i386: Support multi-dies when host doesn't support CPUID.1F

[Like Xu]

In guest CPUID generation process, the cpuid_min_level would be adjusted to
the maximum passed value for basic CPUID configuration and it should not be
restricted by the limited value returned from cpu_x86_cpuid(). After the basic
cpu_x86_cpuid() loop is finished, the cpuid_0_entry.eax needs to be configured
again by the last adjusted cpuid_min_level value.

If a user wants to expose CPUID.1F by passing dies > 1 for any reason without
host support, a per-cpu smp topology warning will appear but it's not blocked.

Signed-off-by: Like Xu <like.xu@linux.intel.com>
---
 target/i386/kvm.c | 24 ++++++++++++++++++++++--
 1 file changed, 22 insertions(+), 2 deletions(-)

diff --git a/target/i386/kvm.c b/target/i386/kvm.c
index 9b4da9b265..8bf1604d2b 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,12 @@ int kvm_arch_init_vcpu(CPUState *cs)
 
     cpu_x86_cpuid(env, 0, 0, &limit, &unused, &unused, &unused);
 
+    /* Allow 0x1f setting regardless of kvm support if nr_dies > 1 */
+    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);
@@ -1151,8 +1157,22 @@ int kvm_arch_init_vcpu(CPUState *cs)
             cpu_x86_cpuid(env, i, 0, &c->eax, &c->ebx, &c->ecx, &c->edx);
             break;
         }
+
+        /* Remember the index of cpuid.0 leaf for reconfiguration. */
+        cpuid_0_entry = (i == 0) ? (cpuid_i - 1) : cpuid_0_entry;
+
+        /* Adjust cpuid_min_level to the maximum index of valid basic cpuids. */
+        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;
+
+    /* Reconfigure cpuid_0_eax value to follow CPUID.0 instruction spec.*/
+    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

[Qemu-devel] [PATCH v3 8/9] machine: Refactor smp_parse() in vl.c as MachineClass::smp_parse()

[Like Xu]

To make smp_parse() more flexible and expansive, a smp_parse function
pointer is added to MachineClass that machine types could override.

The generic smp_parse() code in vl.c is moved to hw/core/machine.c, and
become the default implementation of MachineClass::smp_parse. A PC-specific
function called pc_smp_parse() has been added to hw/i386/pc.c, which in
this patch changes nothing against the default one .

Suggested-by: Eduardo Habkost <ehabkost@redhat.com>
Signed-off-by: Like Xu <like.xu@linux.intel.com>
---
 hw/core/machine.c    | 77 ++++++++++++++++++++++++++++++++++++++++++++
 hw/i386/pc.c         | 76 +++++++++++++++++++++++++++++++++++++++++++
 include/hw/boards.h  |  5 +++
 include/hw/i386/pc.h |  1 +
 vl.c                 | 75 ++----------------------------------------
 5 files changed, 161 insertions(+), 73 deletions(-)

diff --git a/hw/core/machine.c b/hw/core/machine.c
index 9eeba448ed..d58a684abf 100644
--- a/hw/core/machine.c
+++ b/hw/core/machine.c
@@ -11,6 +11,9 @@
  */
 
 #include "qemu/osdep.h"
+#include "qemu/option.h"
+#include "qapi/qmp/qerror.h"
+#include "sysemu/replay.h"
 #include "qemu/units.h"
 #include "hw/boards.h"
 #include "qapi/error.h"
@@ -722,6 +725,79 @@ void machine_set_cpu_numa_node(MachineState *machine,
     }
 }
 
+static void smp_parse(MachineState *ms, QemuOpts *opts)
+{
+    /* copy it from legacy smp_parse() in vl.c */
+    if (opts) {
+        unsigned cpus    = qemu_opt_get_number(opts, "cpus", 0);
+        unsigned sockets = qemu_opt_get_number(opts, "sockets", 0);
+        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 * sockets;
+            } else {
+                ms->smp.max_cpus =
+                        qemu_opt_get_number(opts, "maxcpus", cpus);
+                sockets = ms->smp.max_cpus / (cores * threads);
+            }
+        } else if (cores == 0) {
+            threads = threads > 0 ? threads : 1;
+            cores = cpus / (sockets * threads);
+            cores = cores > 0 ? cores : 1;
+        } else if (threads == 0) {
+            threads = cpus / (cores * sockets);
+            threads = threads > 0 ? threads : 1;
+        } else if (sockets * cores * threads < cpus) {
+            error_report("cpu topology: "
+                         "sockets (%u) * cores (%u) * threads (%u) < "
+                         "smp_cpus (%u)",
+                         sockets, cores, threads, cpus);
+            exit(1);
+        }
+
+        ms->smp.max_cpus =
+                qemu_opt_get_number(opts, "maxcpus", cpus);
+
+        if (ms->smp.max_cpus < cpus) {
+            error_report("maxcpus must be equal to or greater than smp");
+            exit(1);
+        }
+
+        if (sockets * cores * threads > ms->smp.max_cpus) {
+            error_report("cpu topology: "
+                         "sockets (%u) * cores (%u) * threads (%u) > "
+                         "maxcpus (%u)",
+                         sockets, cores, threads,
+                         ms->smp.max_cpus);
+            exit(1);
+        }
+
+        if (sockets * cores * threads != ms->smp.max_cpus) {
+            warn_report("Invalid CPU topology deprecated: "
+                        "sockets (%u) * cores (%u) * threads (%u) "
+                        "!= maxcpus (%u)",
+                        sockets, cores, threads,
+                        ms->smp.max_cpus);
+        }
+
+        ms->smp.cpus = cpus;
+        ms->smp.cores = cores;
+        ms->smp.threads = threads;
+    }
+
+    if (ms->smp.cpus > 1) {
+        Error *blocker = NULL;
+        error_setg(&blocker, QERR_REPLAY_NOT_SUPPORTED, "smp");
+        replay_add_blocker(blocker);
+    }
+}
+
 static void machine_class_init(ObjectClass *oc, void *data)
 {
     MachineClass *mc = MACHINE_CLASS(oc);
@@ -729,6 +805,7 @@ static void machine_class_init(ObjectClass *oc, void *data)
     /* Default 128 MB as guest ram size */
     mc->default_ram_size = 128 * MiB;
     mc->rom_file_has_mr = true;
+    mc->smp_parse = smp_parse;
 
     /* numa node memory size aligned on 8MB by default.
      * On Linux, each node's border has to be 8MB aligned
diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index b4dbd1064d..63b44bd2bd 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -78,6 +78,8 @@
 #include "hw/i386/intel_iommu.h"
 #include "hw/net/ne2000-isa.h"
 #include "standard-headers/asm-x86/bootparam.h"
+#include "sysemu/replay.h"
+#include "qapi/qmp/qerror.h"
 
 /* debug PC/ISA interrupts */
 //#define DEBUG_IRQ
@@ -1539,6 +1541,79 @@ static void pc_new_cpu(PCMachineState *pcms, int64_t apic_id, Error **errp)
     error_propagate(errp, local_err);
 }
 
+void pc_smp_parse(MachineState *ms, QemuOpts *opts)
+{
+    /* copy it from legacy smp_parse() in vl.c */
+    if (opts) {
+        unsigned cpus    = qemu_opt_get_number(opts, "cpus", 0);
+        unsigned sockets = qemu_opt_get_number(opts, "sockets", 0);
+        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 * sockets;
+            } else {
+                ms->smp.max_cpus =
+                        qemu_opt_get_number(opts, "maxcpus", cpus);
+                sockets = ms->smp.max_cpus / (cores * threads);
+            }
+        } else if (cores == 0) {
+            threads = threads > 0 ? threads : 1;
+            cores = cpus / (sockets * threads);
+            cores = cores > 0 ? cores : 1;
+        } else if (threads == 0) {
+            threads = cpus / (cores * sockets);
+            threads = threads > 0 ? threads : 1;
+        } else if (sockets * cores * threads < cpus) {
+            error_report("cpu topology: "
+                         "sockets (%u) * cores (%u) * threads (%u) < "
+                         "smp_cpus (%u)",
+                         sockets, cores, threads, cpus);
+            exit(1);
+        }
+
+        ms->smp.max_cpus =
+                qemu_opt_get_number(opts, "maxcpus", cpus);
+
+        if (ms->smp.max_cpus < cpus) {
+            error_report("maxcpus must be equal to or greater than smp");
+            exit(1);
+        }
+
+        if (sockets * cores * threads > ms->smp.max_cpus) {
+            error_report("cpu topology: "
+                         "sockets (%u) * cores (%u) * threads (%u) > "
+                         "maxcpus (%u)",
+                         sockets, cores, threads,
+                         ms->smp.max_cpus);
+            exit(1);
+        }
+
+        if (sockets * cores * threads != ms->smp.max_cpus) {
+            warn_report("Invalid CPU topology deprecated: "
+                        "sockets (%u) * cores (%u) * threads (%u) "
+                        "!= maxcpus (%u)",
+                        sockets, cores, threads,
+                        ms->smp.max_cpus);
+        }
+
+        ms->smp.cpus = cpus;
+        ms->smp.cores = cores;
+        ms->smp.threads = threads;
+    }
+
+    if (ms->smp.cpus > 1) {
+        Error *blocker = NULL;
+        error_setg(&blocker, QERR_REPLAY_NOT_SUPPORTED, "smp");
+        replay_add_blocker(blocker);
+    }
+}
+
 void pc_hot_add_cpu(MachineState *ms, const int64_t id, Error **errp)
 {
     int64_t apic_id = x86_cpu_apic_id_from_index(ms, id);
@@ -2779,6 +2854,7 @@ static void pc_machine_class_init(ObjectClass *oc, void *data)
     mc->has_hotpluggable_cpus = true;
     mc->default_boot_order = "cad";
     mc->hot_add_cpu = pc_hot_add_cpu;
+    mc->smp_parse = pc_smp_parse;
     mc->block_default_type = IF_IDE;
     mc->max_cpus = 255;
     mc->reset = pc_machine_reset;
diff --git a/include/hw/boards.h b/include/hw/boards.h
index 1e000229e1..c025f33407 100644
--- a/include/hw/boards.h
+++ b/include/hw/boards.h
@@ -158,6 +158,10 @@ typedef struct {
  * @kvm_type:
  *    Return the type of KVM corresponding to the kvm-type string option or
  *    computed based on other criteria such as the host kernel capabilities.
+ * @smp_parse:
+ *    The function pointer to hook different machine specific functions for
+ *    parsing "smp-opts" from QemuOpts to MachineState::CpuTopology and more
+ *    machine specific topology fields, such as smp_dies for PCMachine.
  */
 struct MachineClass {
     /*< private >*/
@@ -174,6 +178,7 @@ struct MachineClass {
     void (*reset)(MachineState *state);
     void (*hot_add_cpu)(MachineState *state, const int64_t id, Error **errp);
     int (*kvm_type)(MachineState *machine, const char *arg);
+    void (*smp_parse)(MachineState *ms, QemuOpts *opts);
 
     BlockInterfaceType block_default_type;
     int units_per_default_bus;
diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
index fae9217e34..7ca24746cc 100644
--- a/include/hw/i386/pc.h
+++ b/include/hw/i386/pc.h
@@ -189,6 +189,7 @@ void pc_acpi_smi_interrupt(void *opaque, int irq, int level);
 
 void pc_cpus_init(PCMachineState *pcms);
 void pc_hot_add_cpu(MachineState *ms, const int64_t id, Error **errp);
+void pc_smp_parse(MachineState *ms, QemuOpts *opts);
 
 void pc_guest_info_init(PCMachineState *pcms);
 
diff --git a/vl.c b/vl.c
index 0760b2724e..53ea9b6d6f 100644
--- a/vl.c
+++ b/vl.c
@@ -1245,78 +1245,6 @@ static QemuOptsList qemu_smp_opts = {
     },
 };
 
-static void smp_parse(QemuOpts *opts)
-{
-    if (opts) {
-        unsigned cpus    = qemu_opt_get_number(opts, "cpus", 0);
-        unsigned sockets = qemu_opt_get_number(opts, "sockets", 0);
-        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 * sockets;
-            } else {
-                current_machine->smp.max_cpus =
-                        qemu_opt_get_number(opts, "maxcpus", cpus);
-                sockets = current_machine->smp.max_cpus / (cores * threads);
-            }
-        } else if (cores == 0) {
-            threads = threads > 0 ? threads : 1;
-            cores = cpus / (sockets * threads);
-            cores = cores > 0 ? cores : 1;
-        } else if (threads == 0) {
-            threads = cpus / (cores * sockets);
-            threads = threads > 0 ? threads : 1;
-        } else if (sockets * cores * threads < cpus) {
-            error_report("cpu topology: "
-                         "sockets (%u) * cores (%u) * threads (%u) < "
-                         "smp_cpus (%u)",
-                         sockets, 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 * cores * threads > current_machine->smp.max_cpus) {
-            error_report("cpu topology: "
-                         "sockets (%u) * cores (%u) * threads (%u) > "
-                         "maxcpus (%u)",
-                         sockets, cores, threads,
-                         current_machine->smp.max_cpus);
-            exit(1);
-        }
-
-        if (sockets * cores * threads != current_machine->smp.max_cpus) {
-            warn_report("Invalid CPU topology deprecated: "
-                        "sockets (%u) * cores (%u) * threads (%u) "
-                        "!= maxcpus (%u)",
-                        sockets, cores, threads,
-                        current_machine->smp.max_cpus);
-        }
-
-        current_machine->smp.cpus = cpus;
-        current_machine->smp.cores = cores;
-        current_machine->smp.threads = threads;
-    }
-
-    if (current_machine->smp.cpus > 1) {
-        Error *blocker = NULL;
-        error_setg(&blocker, QERR_REPLAY_NOT_SUPPORTED, "smp");
-        replay_add_blocker(blocker);
-    }
-}
-
 static void realtime_init(void)
 {
     if (enable_mlock) {
@@ -4043,7 +3971,8 @@ int main(int argc, char **argv, char **envp)
     current_machine->smp.cores = 1;
     current_machine->smp.threads = 1;
 
-    smp_parse(qemu_opts_find(qemu_find_opts("smp-opts"), NULL));
+    machine_class->smp_parse(current_machine,
+        qemu_opts_find(qemu_find_opts("smp-opts"), NULL));
 
     /* sanity-check smp_cpus and max_cpus against machine_class */
     if (current_machine->smp.cpus < machine_class->min_cpus) {
-- 
2.21.0

[Qemu-devel] [PATCH v3 9/9] vl.c: Add -smp, dies=* command line support and update 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".

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>
---
 hw/i386/pc.c    | 32 ++++++++++++++++++--------------
 qemu-options.hx | 17 +++++++++--------
 vl.c            |  3 +++
 3 files changed, 30 insertions(+), 22 deletions(-)

diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index 63b44bd2bd..8a5da4f0c1 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -1543,10 +1543,13 @@ static void pc_new_cpu(PCMachineState *pcms, int64_t apic_id, Error **errp)
 
 void pc_smp_parse(MachineState *ms, QemuOpts *opts)
 {
-    /* copy it from legacy smp_parse() in vl.c */
+    PCMachineState *pcms = (PCMachineState *)
+        object_dynamic_cast(OBJECT(ms), TYPE_PC_MACHINE);
+
     if (opts) {
         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);
 
@@ -1556,24 +1559,24 @@ void pc_smp_parse(MachineState *ms, QemuOpts *opts)
             threads = threads > 0 ? threads : 1;
             if (cpus == 0) {
                 sockets = sockets > 0 ? sockets : 1;
-                cpus = cores * threads * sockets;
+                cpus = cores * threads * dies * sockets;
             } else {
                 ms->smp.max_cpus =
                         qemu_opt_get_number(opts, "maxcpus", cpus);
-                sockets = ms->smp.max_cpus / (cores * threads);
+                sockets = ms->smp.max_cpus / (cores * threads * dies);
             }
         } else if (cores == 0) {
             threads = threads > 0 ? threads : 1;
-            cores = cpus / (sockets * threads);
+            cores = cpus / (sockets * dies * threads);
             cores = cores > 0 ? cores : 1;
         } else if (threads == 0) {
-            threads = cpus / (cores * sockets);
+            threads = cpus / (cores * dies * sockets);
             threads = threads > 0 ? threads : 1;
-        } else if (sockets * cores * threads < cpus) {
+        } else if (sockets * dies * cores * threads < cpus) {
             error_report("cpu topology: "
-                         "sockets (%u) * cores (%u) * threads (%u) < "
+                         "sockets (%u) * dies (%u) * cores (%u) * threads (%u) < "
                          "smp_cpus (%u)",
-                         sockets, cores, threads, cpus);
+                         sockets, dies, cores, threads, cpus);
             exit(1);
         }
 
@@ -1585,26 +1588,27 @@ void pc_smp_parse(MachineState *ms, QemuOpts *opts)
             exit(1);
         }
 
-        if (sockets * cores * threads > ms->smp.max_cpus) {
+        if (sockets * dies * cores * threads > ms->smp.max_cpus) {
             error_report("cpu topology: "
-                         "sockets (%u) * cores (%u) * threads (%u) > "
+                         "sockets (%u) * dies (%u) * cores (%u) * threads (%u) > "
                          "maxcpus (%u)",
-                         sockets, cores, threads,
+                         sockets, dies, cores, threads,
                          ms->smp.max_cpus);
             exit(1);
         }
 
-        if (sockets * cores * threads != ms->smp.max_cpus) {
+        if (sockets * dies * cores * threads != ms->smp.max_cpus) {
             warn_report("Invalid CPU topology deprecated: "
-                        "sockets (%u) * cores (%u) * threads (%u) "
+                        "sockets (%u) * dies (%u) * cores (%u) * threads (%u) "
                         "!= maxcpus (%u)",
-                        sockets, cores, threads,
+                        sockets, dies, cores, threads,
                         ms->smp.max_cpus);
         }
 
         ms->smp.cpus = cpus;
         ms->smp.cores = cores;
         ms->smp.threads = threads;
+        pcms->smp_dies = dies;
     }
 
     if (ms->smp.cpus > 1) {
diff --git a/qemu-options.hx b/qemu-options.hx
index 0d8beb4afd..a5b314a448 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 53ea9b6d6f..c6d1339442 100644
--- a/vl.c
+++ b/vl.c
@@ -1231,6 +1231,9 @@ static QemuOptsList qemu_smp_opts = {
         }, {
             .name = "sockets",
             .type = QEMU_OPT_NUMBER,
+        }, {
+            .name = "dies",
+            .type = QEMU_OPT_NUMBER,
         }, {
             .name = "cores",
             .type = QEMU_OPT_NUMBER,
-- 
2.21.0

Re: [Qemu-devel] [PATCH v3 0/9] Introduce cpu die topology and enable CPUID.1F for i386

[Like Xu]

Ping for timely review.

On 2019/6/12 16:40, Like Xu wrote:
> 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 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 a multi-dies guest and
> 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
> 
> Guest system could discover multi-die/package topology through CPUID.1F.
> and its benefit is primarily for _reporting_ of the (virtual) CPU topology.
> The guest kernel with multi-die/package support have no impact on its
> cache topology, NUMA topology, Linux scheduler, or system performance.
> 
> ==changelog==
> 
> v3:
> 
> - add a MachineClass::smp_parse function pointer
> - place the PC-specific function inside hw/i386/pc.c
> - introduce die_id in a separate patch with default value 0
> - set env->nr_dies in pc_new_cpu() and pc_cpu_pre_plug()
> - fix a circular dependency between target/i386/cpu.c and hw/i386/pc.c
> - fix cpu->die_id check in pc_cpu_pre_plug()
> - Based on "[PATCH v3 00/10] Refactor cpu topo into machine properties"
> - Rebase to commit 219dca61ebf41625831d4f96a720852baf44b762
> 
> v2: https://patchwork.kernel.org/cover/10953191/
> 
> - 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 (9):
>    i386: Add die-level cpu topology to x86CPU on PCMachine
>    hw/i386: Adjust nr_dies with configured smp_dies for PCMachine
>    i386/cpu: Consolidate die-id validity in smp context
>    i386: Update new x86_apicid parsing rules with die_offset support
>    tests/x86-cpuid: Update testcases in test_topo_bits() with multiple dies
>    i386/cpu: Add CPUID.1F generation support for multi-dies PCMachine
>    target/i386: Support multi-dies when host doesn't support CPUID.1F
>    machine: Refactor smp_parse() in vl.c as MachineClass::smp_parse()
>    vl.c: Add -smp, dies=* command line support and update doc
> 
>   hmp.c                      |   3 +
>   hw/core/machine.c          |  89 ++++++++++++++++++++++
>   hw/i386/pc.c               | 148 ++++++++++++++++++++++++++++++++-----
>   include/hw/boards.h        |   5 ++
>   include/hw/i386/pc.h       |   3 +
>   include/hw/i386/topology.h |  76 +++++++++++++------
>   qapi/misc.json             |   6 +-
>   qemu-options.hx            |  17 +++--
>   target/i386/cpu.c          |  53 +++++++++++--
>   target/i386/cpu.h          |   7 ++
>   target/i386/kvm.c          |  36 ++++++++-
>   tests/test-x86-cpuid.c     |  84 +++++++++++----------
>   vl.c                       |  78 ++-----------------
>   13 files changed, 438 insertions(+), 167 deletions(-)
> 

Re: [Qemu-devel] [PATCH v3 1/9] i386: Add die-level cpu topology to x86CPU on PCMachine

[Eduardo Habkost]

On Wed, Jun 12, 2019 at 04:40:56PM +0800, Like Xu wrote:
> The die-level as the first PC-specific cpu topology is added to the leagcy
> cpu topology model, which has one die per package implicitly and only the
> numbers of sockets/cores/threads are configurable.
> 
> 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 would be
> initialized to one in x86_cpu_initfn() and pc_machine_initfn().
> 
> Signed-off-by: Like Xu <like.xu@linux.intel.com>

Reviewed-by: Eduardo Habkost <ehabkost@redhat.com>

> ---
>  hw/i386/pc.c         | 9 +++++++--
>  include/hw/i386/pc.h | 2 ++
>  target/i386/cpu.c    | 1 +
>  target/i386/cpu.h    | 2 ++
>  4 files changed, 12 insertions(+), 2 deletions(-)
> 
> diff --git a/hw/i386/pc.c b/hw/i386/pc.c
> index 12c1e08b85..9e9a42f007 100644
> --- a/hw/i386/pc.c
> +++ b/hw/i386/pc.c
> @@ -2308,9 +2308,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");
> @@ -2620,6 +2624,7 @@ static void pc_machine_initfn(Object *obj)
>      pcms->smbus_enabled = true;
>      pcms->sata_enabled = true;
>      pcms->pit_enabled = true;
> +    pcms->smp_dies = 1;
>  
>      pc_system_flash_create(pcms);
>  }
> diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
> index b260262640..fae9217e34 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/target/i386/cpu.c b/target/i386/cpu.c
> index 23119699de..a16be205fe 100644
> --- a/target/i386/cpu.c
> +++ b/target/i386/cpu.c
> @@ -5619,6 +5619,7 @@ static void x86_cpu_initfn(Object *obj)
>      CPUX86State *env = &cpu->env;
>      FeatureWord w;
>  
> +    env->nr_dies = 1;
>      cpu_set_cpustate_pointers(cpu);
>  
>      object_property_add(obj, "family", "int",
> diff --git a/target/i386/cpu.h b/target/i386/cpu.h
> index edad6e1efb..5daa2eeafa 100644
> --- a/target/i386/cpu.h
> +++ b/target/i386/cpu.h
> @@ -1349,6 +1349,8 @@ typedef struct CPUX86State {
>      uint64_t xss;
>  
>      TPRAccess tpr_access_type;
> +
> +    unsigned nr_dies;
>  } CPUX86State;
>  
>  struct kvm_msrs;
> -- 
> 2.21.0
> 

-- 
Eduardo

Re: [Qemu-devel] [PATCH v3 2/9] hw/i386: Adjust nr_dies with configured smp_dies for PCMachine

[Eduardo Habkost]

On Wed, Jun 12, 2019 at 04:40:57PM +0800, Like Xu wrote:
> To support multiple dies configuration on PCMachine, the best place to
> set CPUX86State->nr_dies with requested PCMachineState->smp_dies is in
> pc_new_cpu() and pc_cpu_pre_plug(). Refactoring pc_new_cpu() is applied
> and redundant parameter "const char *typename" would be removed.
> 
> Suggested-by: Eduardo Habkost <ehabkost@redhat.com>
> Signed-off-by: Like Xu <like.xu@linux.intel.com>

Reviewed-by: Eduardo Habkost <ehabkost@redhat.com>

> ---
>  hw/i386/pc.c | 16 +++++++++++-----
>  1 file changed, 11 insertions(+), 5 deletions(-)
> 
> diff --git a/hw/i386/pc.c b/hw/i386/pc.c
> index 9e9a42f007..af2e95a1b9 100644
> --- a/hw/i386/pc.c
> +++ b/hw/i386/pc.c
> @@ -1520,12 +1520,16 @@ void pc_acpi_smi_interrupt(void *opaque, int irq, int level)
>      }
>  }
>  
> -static void pc_new_cpu(const char *typename, int64_t apic_id, Error **errp)
> +static void pc_new_cpu(PCMachineState *pcms, int64_t apic_id, Error **errp)
>  {
>      Object *cpu = NULL;
>      Error *local_err = NULL;
> +    CPUX86State *env = NULL;
>  
> -    cpu = object_new(typename);
> +    cpu = object_new(MACHINE(pcms)->cpu_type);
> +
> +    env = &X86_CPU(cpu)->env;
> +    env->nr_dies = pcms->smp_dies;
>  
>      object_property_set_uint(cpu, apic_id, "apic-id", &local_err);
>      object_property_set_bool(cpu, true, "realized", &local_err);
> @@ -1551,7 +1555,7 @@ void pc_hot_add_cpu(MachineState *ms, const int64_t id, Error **errp)
>          return;
>      }
>  
> -    pc_new_cpu(ms->cpu_type, apic_id, &local_err);
> +    pc_new_cpu(PC_MACHINE(ms), apic_id, &local_err);
>      if (local_err) {
>          error_propagate(errp, local_err);
>          return;
> @@ -1576,8 +1580,7 @@ void pc_cpus_init(PCMachineState *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(possible_cpus->cpus[i].type, possible_cpus->cpus[i].arch_id,
> -                   &error_fatal);
> +        pc_new_cpu(pcms, possible_cpus->cpus[i].arch_id, &error_fatal);
>      }
>  }
>  
> @@ -2297,6 +2300,7 @@ static void pc_cpu_pre_plug(HotplugHandler *hotplug_dev,
>      CPUArchId *cpu_slot;
>      X86CPUTopoInfo topo;
>      X86CPU *cpu = X86_CPU(dev);
> +    CPUX86State *env = &cpu->env;
>      MachineState *ms = MACHINE(hotplug_dev);
>      PCMachineState *pcms = PC_MACHINE(hotplug_dev);
>      unsigned int smp_cores = ms->smp.cores;
> @@ -2308,6 +2312,8 @@ static void pc_cpu_pre_plug(HotplugHandler *hotplug_dev,
>          return;
>      }
>  
> +    env->nr_dies = pcms->smp_dies;
> +
>      /*
>       * If APIC ID is not set,
>       * set it based on socket/die/core/thread properties.
> -- 
> 2.21.0
> 

-- 
Eduardo

Re: [Qemu-devel] [PATCH v3 3/9] i386/cpu: Consolidate die-id validity in smp context

[Eduardo Habkost]

On Wed, Jun 12, 2019 at 04:40:58PM +0800, Like Xu wrote:
> The field die_id (default as 0) and has_die_id are introduced to X86CPU.
> 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().
> 
> Acked-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
> Signed-off-by: Like Xu <like.xu@linux.intel.com>

Reviewed-by: Eduardo Habkost <ehabkost@redhat.com>

> ---
>  hmp.c                      |  3 +++
>  hw/core/machine.c          | 12 ++++++++++++
>  hw/i386/pc.c               | 14 ++++++++++++++
>  include/hw/i386/topology.h |  2 ++
>  qapi/misc.json             |  6 ++++--
>  target/i386/cpu.c          |  2 ++
>  target/i386/cpu.h          |  1 +
>  7 files changed, 38 insertions(+), 2 deletions(-)
> 
> diff --git a/hmp.c b/hmp.c
> index be5e345c6f..b567c86628 100644
> --- a/hmp.c
> +++ b/hmp.c
> @@ -3113,6 +3113,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 f1a0f45f9c..9eeba448ed 100644
> --- a/hw/core/machine.c
> +++ b/hw/core/machine.c
> @@ -679,6 +679,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;
> @@ -688,6 +693,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;
>          }
> @@ -945,6 +954,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 af2e95a1b9..6e774c6c8e 100644
> --- a/hw/i386/pc.c
> +++ b/hw/i386/pc.c
> @@ -2329,6 +2329,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 > pcms->smp_dies - 1) {
> +            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");
> @@ -2348,6 +2352,7 @@ static void pc_cpu_pre_plug(HotplugHandler *hotplug_dev,
>          }
>  
>          topo.pkg_id = cpu->socket_id;
> +        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(smp_cores, smp_threads, &topo);
> @@ -2385,6 +2390,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);
> @@ -2701,6 +2713,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/topology.h b/include/hw/i386/topology.h
> index 1ebaee0f76..c9fb41588e 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 = 0;
>  }
>  
>  /* 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 a16be205fe..0fc543096f 100644
> --- a/target/i386/cpu.c
> +++ b/target/i386/cpu.c
> @@ -5845,11 +5845,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 5daa2eeafa..69495f0a8a 100644
> --- a/target/i386/cpu.h
> +++ b/target/i386/cpu.h
> @@ -1475,6 +1475,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 v3 4/9] i386: Update new x86_apicid parsing rules with die_offset support

[Eduardo Habkost]

On Wed, Jun 12, 2019 at 04:40:59PM +0800, Like Xu wrote:
> In new sockets/dies/cores/threads model, the apicid of logical cpu could
> imply die level info of guest cpu topology thus x86_apicid_from_cpu_idx()
> need to be refactored with #dies value, so does apicid_*_offset().
> 
> To keep semantic compatibility, the legacy pkg_offset which helps to
> generate CPUIDs such as 0x3 for L3 cache should be mapping to die_offset.
> 
> Signed-off-by: Like Xu <like.xu@linux.intel.com>

Reviewed-by: Eduardo Habkost <ehabkost@redhat.com>

> ---
>  hw/i386/pc.c               | 29 ++++++++++-----
>  include/hw/i386/topology.h | 76 +++++++++++++++++++++++++++-----------
>  target/i386/cpu.c          | 13 ++++---
>  3 files changed, 81 insertions(+), 37 deletions(-)
> 
> diff --git a/hw/i386/pc.c b/hw/i386/pc.c
> index 6e774c6c8e..b4dbd1064d 100644
> --- a/hw/i386/pc.c
> +++ b/hw/i386/pc.c
> @@ -933,10 +933,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()) {
> @@ -2355,18 +2356,21 @@ 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(smp_cores, smp_threads, &topo);
> +        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, 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, 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",
> +            topo.pkg_id, topo.die_id, topo.core_id, topo.smt_id,
> +            cpu->apic_id, ms->possible_cpus->len - 1);
>          return;
>      }
>  
> @@ -2382,7 +2386,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, pcms->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);
> @@ -2679,10 +2684,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;
>  }
>  
> @@ -2690,6 +2697,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) {
>          /*
> @@ -2710,7 +2718,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 c9fb41588e..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 = 0;
> +            ~(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 0fc543096f..09e20a2c3b 100644
> --- a/target/i386/cpu.c
> +++ b/target/i386/cpu.c
> @@ -4245,7 +4245,7 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t index, uint32_t count,
>  {
>      X86CPU *cpu = env_archcpu(env);
>      CPUState *cs = env_cpu(env);
> -    uint32_t pkg_offset;
> +    uint32_t die_offset;
>      uint32_t limit;
>      uint32_t signature[3];
>  
> @@ -4334,10 +4334,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;
>                  }
> @@ -4419,12 +4420,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;
> -- 
> 2.21.0
> 

-- 
Eduardo

Re: [Qemu-devel] [PATCH v3 5/9] tests/x86-cpuid: Update testcases in test_topo_bits() with multiple dies

[Eduardo Habkost]

On Wed, Jun 12, 2019 at 04:41:00PM +0800, Like Xu wrote:
> The corresponding topo_bits tests are updated to support die configurations.
> 
> Signed-off-by: Like Xu <like.xu@linux.intel.com>

Reviewed-by: Eduardo Habkost <ehabkost@redhat.com>

This probably should be squashed into patch 4/9 to keep
bisectability.  I can do this while committing.

> ---
>  tests/test-x86-cpuid.c | 84 ++++++++++++++++++++++--------------------
>  1 file changed, 45 insertions(+), 39 deletions(-)
> 
> 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
> 

-- 
Eduardo

Re: [Qemu-devel] [PATCH v3 6/9] i386/cpu: Add CPUID.1F generation support for multi-dies PCMachine

[Eduardo Habkost]

On Wed, Jun 12, 2019 at 04:41:01PM +0800, Like Xu wrote:
> 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.
> 
> Signed-off-by: Like Xu <like.xu@linux.intel.com>

Reviewed-by: Eduardo Habkost <ehabkost@redhat.com>

> ---
>  target/i386/cpu.c | 37 +++++++++++++++++++++++++++++++++++++
>  target/i386/cpu.h |  4 ++++
>  target/i386/kvm.c | 12 ++++++++++++
>  3 files changed, 53 insertions(+)
> 
> diff --git a/target/i386/cpu.c b/target/i386/cpu.c
> index 09e20a2c3b..127aff74a6 100644
> --- a/target/i386/cpu.c
> +++ b/target/i386/cpu.c
> @@ -4437,6 +4437,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;
> @@ -5890,6 +5926,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 69495f0a8a..0434dfb62a 100644
> --- a/target/i386/cpu.h
> +++ b/target/i386/cpu.h
> @@ -726,6 +726,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)
> @@ -1444,6 +1445,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..9b4da9b265 100644
> --- a/target/i386/kvm.c
> +++ b/target/i386/kvm.c
> @@ -1081,6 +1081,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 +1092,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 +1108,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;
>                  }
> -- 
> 2.21.0
> 

-- 
Eduardo

Re: [Qemu-devel] [PATCH v3 7/9] target/i386: Support multi-dies when host doesn't support CPUID.1F

[Eduardo Habkost]

On Wed, Jun 12, 2019 at 04:41:02PM +0800, Like Xu wrote:
> In guest CPUID generation process, the cpuid_min_level would be adjusted to
> the maximum passed value for basic CPUID configuration and it should not be
> restricted by the limited value returned from cpu_x86_cpuid(). After the basic
> cpu_x86_cpuid() loop is finished, the cpuid_0_entry.eax needs to be configured
> again by the last adjusted cpuid_min_level value.
> 
> If a user wants to expose CPUID.1F by passing dies > 1 for any reason without
> host support, a per-cpu smp topology warning will appear but it's not blocked.
> 
> Signed-off-by: Like Xu <like.xu@linux.intel.com>

This code doesn't look at host CPUID at all, as far as I can see.
Isn't it simpler to just make cpuid_x86_cpuid() return the
correct data?

> ---
>  target/i386/kvm.c | 24 ++++++++++++++++++++++--
>  1 file changed, 22 insertions(+), 2 deletions(-)
> 
> diff --git a/target/i386/kvm.c b/target/i386/kvm.c
> index 9b4da9b265..8bf1604d2b 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,12 @@ int kvm_arch_init_vcpu(CPUState *cs)
>  
>      cpu_x86_cpuid(env, 0, 0, &limit, &unused, &unused, &unused);
>  
> +    /* Allow 0x1f setting regardless of kvm support if nr_dies > 1 */
> +    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);
> @@ -1151,8 +1157,22 @@ int kvm_arch_init_vcpu(CPUState *cs)
>              cpu_x86_cpuid(env, i, 0, &c->eax, &c->ebx, &c->ecx, &c->edx);
>              break;
>          }
> +
> +        /* Remember the index of cpuid.0 leaf for reconfiguration. */
> +        cpuid_0_entry = (i == 0) ? (cpuid_i - 1) : cpuid_0_entry;
> +
> +        /* Adjust cpuid_min_level to the maximum index of valid basic cpuids. */
> +        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;
> +
> +    /* Reconfigure cpuid_0_eax value to follow CPUID.0 instruction spec.*/
> +    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
> 

-- 
Eduardo

Re: [Qemu-devel] [PATCH v3 8/9] machine: Refactor smp_parse() in vl.c as MachineClass::smp_parse()

[Eduardo Habkost]

On Wed, Jun 12, 2019 at 04:41:03PM +0800, Like Xu wrote:
> To make smp_parse() more flexible and expansive, a smp_parse function
> pointer is added to MachineClass that machine types could override.
> 
> The generic smp_parse() code in vl.c is moved to hw/core/machine.c, and
> become the default implementation of MachineClass::smp_parse. A PC-specific
> function called pc_smp_parse() has been added to hw/i386/pc.c, which in
> this patch changes nothing against the default one .
> 
> Suggested-by: Eduardo Habkost <ehabkost@redhat.com>
> Signed-off-by: Like Xu <like.xu@linux.intel.com>
> ---
>  hw/core/machine.c    | 77 ++++++++++++++++++++++++++++++++++++++++++++
>  hw/i386/pc.c         | 76 +++++++++++++++++++++++++++++++++++++++++++
>  include/hw/boards.h  |  5 +++
>  include/hw/i386/pc.h |  1 +
>  vl.c                 | 75 ++----------------------------------------
>  5 files changed, 161 insertions(+), 73 deletions(-)
> 
> diff --git a/hw/core/machine.c b/hw/core/machine.c
> index 9eeba448ed..d58a684abf 100644
> --- a/hw/core/machine.c
> +++ b/hw/core/machine.c
> @@ -11,6 +11,9 @@
>   */
>  
>  #include "qemu/osdep.h"
> +#include "qemu/option.h"
> +#include "qapi/qmp/qerror.h"
> +#include "sysemu/replay.h"
>  #include "qemu/units.h"
>  #include "hw/boards.h"
>  #include "qapi/error.h"
> @@ -722,6 +725,79 @@ void machine_set_cpu_numa_node(MachineState *machine,
>      }
>  }
>  
> +static void smp_parse(MachineState *ms, QemuOpts *opts)
> +{
> +    /* copy it from legacy smp_parse() in vl.c */

This comment stops making sense for people reading the code as
soon as we delete smp_parse() from vl.c.  Was it kept by
accident?


> +    if (opts) {
> +        unsigned cpus    = qemu_opt_get_number(opts, "cpus", 0);
> +        unsigned sockets = qemu_opt_get_number(opts, "sockets", 0);
> +        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 * sockets;
> +            } else {
> +                ms->smp.max_cpus =
> +                        qemu_opt_get_number(opts, "maxcpus", cpus);
> +                sockets = ms->smp.max_cpus / (cores * threads);
> +            }
> +        } else if (cores == 0) {
> +            threads = threads > 0 ? threads : 1;
> +            cores = cpus / (sockets * threads);
> +            cores = cores > 0 ? cores : 1;
> +        } else if (threads == 0) {
> +            threads = cpus / (cores * sockets);
> +            threads = threads > 0 ? threads : 1;
> +        } else if (sockets * cores * threads < cpus) {
> +            error_report("cpu topology: "
> +                         "sockets (%u) * cores (%u) * threads (%u) < "
> +                         "smp_cpus (%u)",
> +                         sockets, cores, threads, cpus);
> +            exit(1);
> +        }
> +
> +        ms->smp.max_cpus =
> +                qemu_opt_get_number(opts, "maxcpus", cpus);
> +
> +        if (ms->smp.max_cpus < cpus) {
> +            error_report("maxcpus must be equal to or greater than smp");
> +            exit(1);
> +        }
> +
> +        if (sockets * cores * threads > ms->smp.max_cpus) {
> +            error_report("cpu topology: "
> +                         "sockets (%u) * cores (%u) * threads (%u) > "
> +                         "maxcpus (%u)",
> +                         sockets, cores, threads,
> +                         ms->smp.max_cpus);
> +            exit(1);
> +        }
> +
> +        if (sockets * cores * threads != ms->smp.max_cpus) {
> +            warn_report("Invalid CPU topology deprecated: "
> +                        "sockets (%u) * cores (%u) * threads (%u) "
> +                        "!= maxcpus (%u)",
> +                        sockets, cores, threads,
> +                        ms->smp.max_cpus);
> +        }
> +
> +        ms->smp.cpus = cpus;
> +        ms->smp.cores = cores;
> +        ms->smp.threads = threads;
> +    }
> +
> +    if (ms->smp.cpus > 1) {
> +        Error *blocker = NULL;
> +        error_setg(&blocker, QERR_REPLAY_NOT_SUPPORTED, "smp");
> +        replay_add_blocker(blocker);
> +    }
> +}
> +
>  static void machine_class_init(ObjectClass *oc, void *data)
>  {
>      MachineClass *mc = MACHINE_CLASS(oc);
> @@ -729,6 +805,7 @@ static void machine_class_init(ObjectClass *oc, void *data)
>      /* Default 128 MB as guest ram size */
>      mc->default_ram_size = 128 * MiB;
>      mc->rom_file_has_mr = true;
> +    mc->smp_parse = smp_parse;
>  
>      /* numa node memory size aligned on 8MB by default.
>       * On Linux, each node's border has to be 8MB aligned
> diff --git a/hw/i386/pc.c b/hw/i386/pc.c
> index b4dbd1064d..63b44bd2bd 100644
> --- a/hw/i386/pc.c
> +++ b/hw/i386/pc.c
> @@ -78,6 +78,8 @@
>  #include "hw/i386/intel_iommu.h"
>  #include "hw/net/ne2000-isa.h"
>  #include "standard-headers/asm-x86/bootparam.h"
> +#include "sysemu/replay.h"
> +#include "qapi/qmp/qerror.h"
>  
>  /* debug PC/ISA interrupts */
>  //#define DEBUG_IRQ
> @@ -1539,6 +1541,79 @@ static void pc_new_cpu(PCMachineState *pcms, int64_t apic_id, Error **errp)
>      error_propagate(errp, local_err);
>  }
>  
> +void pc_smp_parse(MachineState *ms, QemuOpts *opts)
> +{
> +    /* copy it from legacy smp_parse() in vl.c */

I suggest replacing this comment with one saying this function is
very similar to smp_parse() in hw/core/machine.c but includes CPU
die support.

I can remove the comment while committing, and the comment may be
submitted as a follow up patch.

Reviewed-by: Eduardo Habkost <ehabkost@redhat.com>

> +    if (opts) {
> +        unsigned cpus    = qemu_opt_get_number(opts, "cpus", 0);
> +        unsigned sockets = qemu_opt_get_number(opts, "sockets", 0);
> +        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 * sockets;
> +            } else {
> +                ms->smp.max_cpus =
> +                        qemu_opt_get_number(opts, "maxcpus", cpus);
> +                sockets = ms->smp.max_cpus / (cores * threads);
> +            }
> +        } else if (cores == 0) {
> +            threads = threads > 0 ? threads : 1;
> +            cores = cpus / (sockets * threads);
> +            cores = cores > 0 ? cores : 1;
> +        } else if (threads == 0) {
> +            threads = cpus / (cores * sockets);
> +            threads = threads > 0 ? threads : 1;
> +        } else if (sockets * cores * threads < cpus) {
> +            error_report("cpu topology: "
> +                         "sockets (%u) * cores (%u) * threads (%u) < "
> +                         "smp_cpus (%u)",
> +                         sockets, cores, threads, cpus);
> +            exit(1);
> +        }
> +
> +        ms->smp.max_cpus =
> +                qemu_opt_get_number(opts, "maxcpus", cpus);
> +
> +        if (ms->smp.max_cpus < cpus) {
> +            error_report("maxcpus must be equal to or greater than smp");
> +            exit(1);
> +        }
> +
> +        if (sockets * cores * threads > ms->smp.max_cpus) {
> +            error_report("cpu topology: "
> +                         "sockets (%u) * cores (%u) * threads (%u) > "
> +                         "maxcpus (%u)",
> +                         sockets, cores, threads,
> +                         ms->smp.max_cpus);
> +            exit(1);
> +        }
> +
> +        if (sockets * cores * threads != ms->smp.max_cpus) {
> +            warn_report("Invalid CPU topology deprecated: "
> +                        "sockets (%u) * cores (%u) * threads (%u) "
> +                        "!= maxcpus (%u)",
> +                        sockets, cores, threads,
> +                        ms->smp.max_cpus);
> +        }
> +
> +        ms->smp.cpus = cpus;
> +        ms->smp.cores = cores;
> +        ms->smp.threads = threads;
> +    }
> +
> +    if (ms->smp.cpus > 1) {
> +        Error *blocker = NULL;
> +        error_setg(&blocker, QERR_REPLAY_NOT_SUPPORTED, "smp");
> +        replay_add_blocker(blocker);
> +    }
> +}
> +
>  void pc_hot_add_cpu(MachineState *ms, const int64_t id, Error **errp)
>  {
>      int64_t apic_id = x86_cpu_apic_id_from_index(ms, id);
> @@ -2779,6 +2854,7 @@ static void pc_machine_class_init(ObjectClass *oc, void *data)
>      mc->has_hotpluggable_cpus = true;
>      mc->default_boot_order = "cad";
>      mc->hot_add_cpu = pc_hot_add_cpu;
> +    mc->smp_parse = pc_smp_parse;
>      mc->block_default_type = IF_IDE;
>      mc->max_cpus = 255;
>      mc->reset = pc_machine_reset;
> diff --git a/include/hw/boards.h b/include/hw/boards.h
> index 1e000229e1..c025f33407 100644
> --- a/include/hw/boards.h
> +++ b/include/hw/boards.h
> @@ -158,6 +158,10 @@ typedef struct {
>   * @kvm_type:
>   *    Return the type of KVM corresponding to the kvm-type string option or
>   *    computed based on other criteria such as the host kernel capabilities.
> + * @smp_parse:
> + *    The function pointer to hook different machine specific functions for
> + *    parsing "smp-opts" from QemuOpts to MachineState::CpuTopology and more
> + *    machine specific topology fields, such as smp_dies for PCMachine.
>   */
>  struct MachineClass {
>      /*< private >*/
> @@ -174,6 +178,7 @@ struct MachineClass {
>      void (*reset)(MachineState *state);
>      void (*hot_add_cpu)(MachineState *state, const int64_t id, Error **errp);
>      int (*kvm_type)(MachineState *machine, const char *arg);
> +    void (*smp_parse)(MachineState *ms, QemuOpts *opts);
>  
>      BlockInterfaceType block_default_type;
>      int units_per_default_bus;
> diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
> index fae9217e34..7ca24746cc 100644
> --- a/include/hw/i386/pc.h
> +++ b/include/hw/i386/pc.h
> @@ -189,6 +189,7 @@ void pc_acpi_smi_interrupt(void *opaque, int irq, int level);
>  
>  void pc_cpus_init(PCMachineState *pcms);
>  void pc_hot_add_cpu(MachineState *ms, const int64_t id, Error **errp);
> +void pc_smp_parse(MachineState *ms, QemuOpts *opts);
>  
>  void pc_guest_info_init(PCMachineState *pcms);
>  
> diff --git a/vl.c b/vl.c
> index 0760b2724e..53ea9b6d6f 100644
> --- a/vl.c
> +++ b/vl.c
> @@ -1245,78 +1245,6 @@ static QemuOptsList qemu_smp_opts = {
>      },
>  };
>  
> -static void smp_parse(QemuOpts *opts)
> -{
> -    if (opts) {
> -        unsigned cpus    = qemu_opt_get_number(opts, "cpus", 0);
> -        unsigned sockets = qemu_opt_get_number(opts, "sockets", 0);
> -        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 * sockets;
> -            } else {
> -                current_machine->smp.max_cpus =
> -                        qemu_opt_get_number(opts, "maxcpus", cpus);
> -                sockets = current_machine->smp.max_cpus / (cores * threads);
> -            }
> -        } else if (cores == 0) {
> -            threads = threads > 0 ? threads : 1;
> -            cores = cpus / (sockets * threads);
> -            cores = cores > 0 ? cores : 1;
> -        } else if (threads == 0) {
> -            threads = cpus / (cores * sockets);
> -            threads = threads > 0 ? threads : 1;
> -        } else if (sockets * cores * threads < cpus) {
> -            error_report("cpu topology: "
> -                         "sockets (%u) * cores (%u) * threads (%u) < "
> -                         "smp_cpus (%u)",
> -                         sockets, 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 * cores * threads > current_machine->smp.max_cpus) {
> -            error_report("cpu topology: "
> -                         "sockets (%u) * cores (%u) * threads (%u) > "
> -                         "maxcpus (%u)",
> -                         sockets, cores, threads,
> -                         current_machine->smp.max_cpus);
> -            exit(1);
> -        }
> -
> -        if (sockets * cores * threads != current_machine->smp.max_cpus) {
> -            warn_report("Invalid CPU topology deprecated: "
> -                        "sockets (%u) * cores (%u) * threads (%u) "
> -                        "!= maxcpus (%u)",
> -                        sockets, cores, threads,
> -                        current_machine->smp.max_cpus);
> -        }
> -
> -        current_machine->smp.cpus = cpus;
> -        current_machine->smp.cores = cores;
> -        current_machine->smp.threads = threads;
> -    }
> -
> -    if (current_machine->smp.cpus > 1) {
> -        Error *blocker = NULL;
> -        error_setg(&blocker, QERR_REPLAY_NOT_SUPPORTED, "smp");
> -        replay_add_blocker(blocker);
> -    }
> -}
> -
>  static void realtime_init(void)
>  {
>      if (enable_mlock) {
> @@ -4043,7 +3971,8 @@ int main(int argc, char **argv, char **envp)
>      current_machine->smp.cores = 1;
>      current_machine->smp.threads = 1;
>  
> -    smp_parse(qemu_opts_find(qemu_find_opts("smp-opts"), NULL));
> +    machine_class->smp_parse(current_machine,
> +        qemu_opts_find(qemu_find_opts("smp-opts"), NULL));
>  
>      /* sanity-check smp_cpus and max_cpus against machine_class */
>      if (current_machine->smp.cpus < machine_class->min_cpus) {
> -- 
> 2.21.0
> 

-- 
Eduardo

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

[Eduardo Habkost]

On Wed, Jun 12, 2019 at 04:41:04PM +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".
> 
> 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>
> ---
>  hw/i386/pc.c    | 32 ++++++++++++++++++--------------
>  qemu-options.hx | 17 +++++++++--------
>  vl.c            |  3 +++
>  3 files changed, 30 insertions(+), 22 deletions(-)
> 
> diff --git a/hw/i386/pc.c b/hw/i386/pc.c
> index 63b44bd2bd..8a5da4f0c1 100644
> --- a/hw/i386/pc.c
> +++ b/hw/i386/pc.c
> @@ -1543,10 +1543,13 @@ static void pc_new_cpu(PCMachineState *pcms, int64_t apic_id, Error **errp)
>  
>  void pc_smp_parse(MachineState *ms, QemuOpts *opts)
>  {
> -    /* copy it from legacy smp_parse() in vl.c */
> +    PCMachineState *pcms = (PCMachineState *)
> +        object_dynamic_cast(OBJECT(ms), TYPE_PC_MACHINE);
> +

Any specific reason for doing this instead of PC_MACHINE(ms)?

The rest of the patch looks good.


>      if (opts) {
>          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);
>  
> @@ -1556,24 +1559,24 @@ void pc_smp_parse(MachineState *ms, QemuOpts *opts)
>              threads = threads > 0 ? threads : 1;
>              if (cpus == 0) {
>                  sockets = sockets > 0 ? sockets : 1;
> -                cpus = cores * threads * sockets;
> +                cpus = cores * threads * dies * sockets;
>              } else {
>                  ms->smp.max_cpus =
>                          qemu_opt_get_number(opts, "maxcpus", cpus);
> -                sockets = ms->smp.max_cpus / (cores * threads);
> +                sockets = ms->smp.max_cpus / (cores * threads * dies);
>              }
>          } else if (cores == 0) {
>              threads = threads > 0 ? threads : 1;
> -            cores = cpus / (sockets * threads);
> +            cores = cpus / (sockets * dies * threads);
>              cores = cores > 0 ? cores : 1;
>          } else if (threads == 0) {
> -            threads = cpus / (cores * sockets);
> +            threads = cpus / (cores * dies * sockets);
>              threads = threads > 0 ? threads : 1;
> -        } else if (sockets * cores * threads < cpus) {
> +        } else if (sockets * dies * cores * threads < cpus) {
>              error_report("cpu topology: "
> -                         "sockets (%u) * cores (%u) * threads (%u) < "
> +                         "sockets (%u) * dies (%u) * cores (%u) * threads (%u) < "
>                           "smp_cpus (%u)",
> -                         sockets, cores, threads, cpus);
> +                         sockets, dies, cores, threads, cpus);
>              exit(1);
>          }
>  
> @@ -1585,26 +1588,27 @@ void pc_smp_parse(MachineState *ms, QemuOpts *opts)
>              exit(1);
>          }
>  
> -        if (sockets * cores * threads > ms->smp.max_cpus) {
> +        if (sockets * dies * cores * threads > ms->smp.max_cpus) {
>              error_report("cpu topology: "
> -                         "sockets (%u) * cores (%u) * threads (%u) > "
> +                         "sockets (%u) * dies (%u) * cores (%u) * threads (%u) > "
>                           "maxcpus (%u)",
> -                         sockets, cores, threads,
> +                         sockets, dies, cores, threads,
>                           ms->smp.max_cpus);
>              exit(1);
>          }
>  
> -        if (sockets * cores * threads != ms->smp.max_cpus) {
> +        if (sockets * dies * cores * threads != ms->smp.max_cpus) {
>              warn_report("Invalid CPU topology deprecated: "
> -                        "sockets (%u) * cores (%u) * threads (%u) "
> +                        "sockets (%u) * dies (%u) * cores (%u) * threads (%u) "
>                          "!= maxcpus (%u)",
> -                        sockets, cores, threads,
> +                        sockets, dies, cores, threads,
>                          ms->smp.max_cpus);
>          }
>  
>          ms->smp.cpus = cpus;
>          ms->smp.cores = cores;
>          ms->smp.threads = threads;
> +        pcms->smp_dies = dies;
>      }
>  
>      if (ms->smp.cpus > 1) {
> diff --git a/qemu-options.hx b/qemu-options.hx
> index 0d8beb4afd..a5b314a448 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 53ea9b6d6f..c6d1339442 100644
> --- a/vl.c
> +++ b/vl.c
> @@ -1231,6 +1231,9 @@ static QemuOptsList qemu_smp_opts = {
>          }, {
>              .name = "sockets",
>              .type = QEMU_OPT_NUMBER,
> +        }, {
> +            .name = "dies",
> +            .type = QEMU_OPT_NUMBER,
>          }, {
>              .name = "cores",
>              .type = QEMU_OPT_NUMBER,
> -- 
> 2.21.0
> 

-- 
Eduardo

Re: [Qemu-devel] [PATCH v3 6/9] i386/cpu: Add CPUID.1F generation support for multi-dies PCMachine

[Eduardo Habkost]

I've just noticed one thing I don't understand:

On Wed, Jun 12, 2019 at 04:41:01PM +0800, Like Xu wrote:
> 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.
> 
> Signed-off-by: Like Xu <like.xu@linux.intel.com>
> ---
>  target/i386/cpu.c | 37 +++++++++++++++++++++++++++++++++++++
>  target/i386/cpu.h |  4 ++++
>  target/i386/kvm.c | 12 ++++++++++++
>  3 files changed, 53 insertions(+)
> 
> diff --git a/target/i386/cpu.c b/target/i386/cpu.c
> index 09e20a2c3b..127aff74a6 100644
> --- a/target/i386/cpu.c
> +++ b/target/i386/cpu.c
> @@ -4437,6 +4437,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;

Why exactly do you need cpu->enable_cpuid_0x1f?  When would it
make sense to set dies > 1 but disable CPUID.1F?


> +            break;
> +        }
[...]

-- 
Eduardo

Re: [Qemu-devel] [PATCH v3 7/9] target/i386: Support multi-dies when host doesn't support CPUID.1F

[Eduardo Habkost]

On Wed, Jun 19, 2019 at 04:15:46PM -0300, Eduardo Habkost wrote:
> On Wed, Jun 12, 2019 at 04:41:02PM +0800, Like Xu wrote:
> > In guest CPUID generation process, the cpuid_min_level would be adjusted to
> > the maximum passed value for basic CPUID configuration and it should not be
> > restricted by the limited value returned from cpu_x86_cpuid(). After the basic
> > cpu_x86_cpuid() loop is finished, the cpuid_0_entry.eax needs to be configured
> > again by the last adjusted cpuid_min_level value.
> > 
> > If a user wants to expose CPUID.1F by passing dies > 1 for any reason without
> > host support, a per-cpu smp topology warning will appear but it's not blocked.
> > 
> > Signed-off-by: Like Xu <like.xu@linux.intel.com>
> 
> This code doesn't look at host CPUID at all, as far as I can see.
> Isn't it simpler to just make cpuid_x86_cpuid() return the
> correct data?

I suggest the following change instead.

Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
---
 target/i386/cpu.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/target/i386/cpu.c b/target/i386/cpu.c
index 6db38e145b..d05a224092 100644
--- a/target/i386/cpu.c
+++ b/target/i386/cpu.c
@@ -5152,6 +5152,10 @@ static void x86_cpu_expand_features(X86CPU *cpu, Error **errp)
             x86_cpu_adjust_level(cpu, &cpu->env.cpuid_min_level, 0x14);
         }
 
+        if (env->nr_dies > 1) {
+            x86_cpu_adjust_level(cpu, &env->cpuid_min_level, 0x1F);
+        }
+
         /* SVM requires CPUID[0x8000000A] */
         if (env->features[FEAT_8000_0001_ECX] & CPUID_EXT3_SVM) {
             x86_cpu_adjust_level(cpu, &env->cpuid_min_xlevel, 0x8000000A);
-- 
2.18.0.rc1.1.g3f1ff2140

Re: [Qemu-devel] [PATCH v3 7/9] target/i386: Support multi-dies when host doesn't support CPUID.1F

[Like Xu]

On 2019/6/20 7:36, Eduardo Habkost wrote:
> On Wed, Jun 19, 2019 at 04:15:46PM -0300, Eduardo Habkost wrote:
>> On Wed, Jun 12, 2019 at 04:41:02PM +0800, Like Xu wrote:
>>> In guest CPUID generation process, the cpuid_min_level would be adjusted to
>>> the maximum passed value for basic CPUID configuration and it should not be
>>> restricted by the limited value returned from cpu_x86_cpuid(). After the basic
>>> cpu_x86_cpuid() loop is finished, the cpuid_0_entry.eax needs to be configured
>>> again by the last adjusted cpuid_min_level value.
>>>
>>> If a user wants to expose CPUID.1F by passing dies > 1 for any reason without
>>> host support, a per-cpu smp topology warning will appear but it's not blocked.
>>>
>>> Signed-off-by: Like Xu <like.xu@linux.intel.com>
>>
>> This code doesn't look at host CPUID at all, as far as I can see.
>> Isn't it simpler to just make cpuid_x86_cpuid() return the
>> correct data?
> 
> I suggest the following change instead.
> 
> Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>

Hi Eduardo,

Your code is more reasonable and concise than mine on this
so let's not break cpuid_x86_cpuid().

I'll remove the use of enable_cpuid_0x1f in next version, and should I 
resend the patch series "Refactor cpu topo into machine properties" 
because rebase-fix may distract you ?

> ---
>   target/i386/cpu.c | 4 ++++
>   1 file changed, 4 insertions(+)
> 
> diff --git a/target/i386/cpu.c b/target/i386/cpu.c
> index 6db38e145b..d05a224092 100644
> --- a/target/i386/cpu.c
> +++ b/target/i386/cpu.c
> @@ -5152,6 +5152,10 @@ static void x86_cpu_expand_features(X86CPU *cpu, Error **errp)
>               x86_cpu_adjust_level(cpu, &cpu->env.cpuid_min_level, 0x14);
>           }
>   
> +        if (env->nr_dies > 1) {
> +            x86_cpu_adjust_level(cpu, &env->cpuid_min_level, 0x1F);
> +        }
> +
>           /* SVM requires CPUID[0x8000000A] */
>           if (env->features[FEAT_8000_0001_ECX] & CPUID_EXT3_SVM) {
>               x86_cpu_adjust_level(cpu, &env->cpuid_min_xlevel, 0x8000000A);
> 

Re: [Qemu-devel] [PATCH v3 7/9] target/i386: Support multi-dies when host doesn't support CPUID.1F

[Eduardo Habkost]

On Thu, Jun 20, 2019 at 10:03:07AM +0800, Like Xu wrote:
> On 2019/6/20 7:36, Eduardo Habkost wrote:
> > On Wed, Jun 19, 2019 at 04:15:46PM -0300, Eduardo Habkost wrote:
> > > On Wed, Jun 12, 2019 at 04:41:02PM +0800, Like Xu wrote:
> > > > In guest CPUID generation process, the cpuid_min_level would be adjusted to
> > > > the maximum passed value for basic CPUID configuration and it should not be
> > > > restricted by the limited value returned from cpu_x86_cpuid(). After the basic
> > > > cpu_x86_cpuid() loop is finished, the cpuid_0_entry.eax needs to be configured
> > > > again by the last adjusted cpuid_min_level value.
> > > > 
> > > > If a user wants to expose CPUID.1F by passing dies > 1 for any reason without
> > > > host support, a per-cpu smp topology warning will appear but it's not blocked.
> > > > 
> > > > Signed-off-by: Like Xu <like.xu@linux.intel.com>
> > > 
> > > This code doesn't look at host CPUID at all, as far as I can see.
> > > Isn't it simpler to just make cpuid_x86_cpuid() return the
> > > correct data?
> > 
> > I suggest the following change instead.
> > 
> > Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
> 
> Hi Eduardo,
> 
> Your code is more reasonable and concise than mine on this
> so let's not break cpuid_x86_cpuid().
> 
> I'll remove the use of enable_cpuid_0x1f in next version, and should I
> resend the patch series "Refactor cpu topo into machine properties" because
> rebase-fix may distract you ?

"Refactor cpu topo" and patches 1-4 of this series are already
queued on my machine-next branch.  You can send the next version
of the series using that branch as base:

  https://github.com/ehabkost/qemu.git machine-next

-- 
Eduardo