* [Qemu-devel] [PATCH v1 1/5] cpu: introduce die, the new cpu toppolgy emulation level
2019-01-14 12:24 [Qemu-devel] [PATCH v1 0/5] Introduce cpu die topology and enable CPUID.1F for i386 Like Xu
@ 2019-01-14 12:24 ` Like Xu
2019-01-14 20:08 ` Eric Blake
2019-01-14 12:24 ` [Qemu-devel] [PATCH v1 2/5] vl.c: add -smp, dies=* command line support Like Xu
` (4 subsequent siblings)
5 siblings, 1 reply; 15+ messages in thread
From: Like Xu @ 2019-01-14 12:24 UTC (permalink / raw)
To: qemu-devel
Cc: like.xu, imammedo, drjones, Michael S. Tsirkin, Marcelo Tosatti,
Marcel Apfelbaum, Eduardo Habkost, Paolo Bonzini,
Peter Crosthwaite, Richard Henderson
Following codes on smp_cores, the smp_dies/nr_dies/die-id is added to
machine and CPUState. In addition to enable_cpuid_0xb, enable_cpuid_0x1f
is introduced to track wether host is a new MCP macine or just ignored.
The number for die level_type on Intel is 5 while core type keeps 2.
Signed-off-by: Like Xu <like.xu@linux.intel.com>
---
cpus.c | 1 +
include/qom/cpu.h | 1 +
include/sysemu/cpus.h | 1 +
qapi/misc.json | 1 +
target/i386/cpu.h | 5 +++++
5 files changed, 9 insertions(+)
diff --git a/cpus.c b/cpus.c
index b09b702..503558d 100644
--- a/cpus.c
+++ b/cpus.c
@@ -2066,6 +2066,7 @@ static void qemu_dummy_start_vcpu(CPUState *cpu)
void qemu_init_vcpu(CPUState *cpu)
{
+ cpu->nr_dies = smp_dies;
cpu->nr_cores = smp_cores;
cpu->nr_threads = smp_threads;
cpu->stopped = true;
diff --git a/include/qom/cpu.h b/include/qom/cpu.h
index 16bbed1..ee53862 100644
--- a/include/qom/cpu.h
+++ b/include/qom/cpu.h
@@ -332,6 +332,7 @@ struct CPUState {
DeviceState parent_obj;
/*< public >*/
+ int nr_dies;
int nr_cores;
int nr_threads;
diff --git a/include/sysemu/cpus.h b/include/sysemu/cpus.h
index 731756d..4243c8f 100644
--- a/include/sysemu/cpus.h
+++ b/include/sysemu/cpus.h
@@ -34,6 +34,7 @@ void qtest_clock_warp(int64_t dest);
#ifndef CONFIG_USER_ONLY
/* vl.c */
/* *-user doesn't have configurable SMP topology */
+extern int smp_dies;
extern int smp_cores;
extern int smp_threads;
#endif
diff --git a/qapi/misc.json b/qapi/misc.json
index 24d20a8..a01a9fe 100644
--- a/qapi/misc.json
+++ b/qapi/misc.json
@@ -3229,6 +3229,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.h b/target/i386/cpu.h
index ef41a03..aa2ee8a 100644
--- a/target/i386/cpu.h
+++ b/target/i386/cpu.h
@@ -732,6 +732,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)
@@ -1450,6 +1451,9 @@ struct X86CPU {
/* Compatibility bits for old machine types: */
bool enable_cpuid_0xb;
+ /* Compatibility bits for new machine types: */
+ bool enable_cpuid_0x1f;
+
/* Enable auto level-increase for all CPUID leaves */
bool full_cpuid_auto_level;
@@ -1475,6 +1479,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;
--
1.8.3.1
^ permalink raw reply related [flat|nested] 15+ messages in thread* Re: [Qemu-devel] [PATCH v1 1/5] cpu: introduce die, the new cpu toppolgy emulation level
2019-01-14 12:24 ` [Qemu-devel] [PATCH v1 1/5] cpu: introduce die, the new cpu toppolgy emulation level Like Xu
@ 2019-01-14 20:08 ` Eric Blake
2019-01-15 1:34 ` Xu, Like
0 siblings, 1 reply; 15+ messages in thread
From: Eric Blake @ 2019-01-14 20:08 UTC (permalink / raw)
To: Like Xu, qemu-devel
Cc: drjones, Peter Crosthwaite, Eduardo Habkost, Michael S. Tsirkin,
like.xu, Marcelo Tosatti, Paolo Bonzini, imammedo,
Richard Henderson
[-- Attachment #1: Type: text/plain, Size: 1129 bytes --]
On 1/14/19 6:24 AM, Like Xu wrote:
> Following codes on smp_cores, the smp_dies/nr_dies/die-id is added to
> machine and CPUState. In addition to enable_cpuid_0xb, enable_cpuid_0x1f
> is introduced to track wether host is a new MCP macine or just ignored.
s/wether/whether/, s/macine/machine/
> The number for die level_type on Intel is 5 while core type keeps 2.
>
> Signed-off-by: Like Xu <like.xu@linux.intel.com>
> ---
> cpus.c | 1 +
> include/qom/cpu.h | 1 +
> include/sysemu/cpus.h | 1 +
> qapi/misc.json | 1 +
> target/i386/cpu.h | 5 +++++
> 5 files changed, 9 insertions(+)
>
> +++ b/qapi/misc.json
> @@ -3229,6 +3229,7 @@
> { 'struct': 'CpuInstanceProperties',
> 'data': { '*node-id': 'int',
> '*socket-id': 'int',
> + '*die-id': 'int',
Missing documentation of the new field, including a '(since 4.0)' tag.
> '*core-id': 'int',
> '*thread-id': 'int'
> }
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3226
Virtualization: qemu.org | libvirt.org
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply [flat|nested] 15+ messages in thread* Re: [Qemu-devel] [PATCH v1 1/5] cpu: introduce die, the new cpu toppolgy emulation level
2019-01-14 20:08 ` Eric Blake
@ 2019-01-15 1:34 ` Xu, Like
0 siblings, 0 replies; 15+ messages in thread
From: Xu, Like @ 2019-01-15 1:34 UTC (permalink / raw)
To: Eric Blake, Like Xu, qemu-devel
Cc: drjones, Peter Crosthwaite, Eduardo Habkost, Michael S. Tsirkin,
Marcelo Tosatti, Paolo Bonzini, imammedo, Richard Henderson
> -----Original Message-----
> From: Eric Blake [mailto:eblake@redhat.com]
> Sent: Tuesday, January 15, 2019 4:08 AM
> To: Like Xu <like.xu@linux.intel.com>; qemu-devel@nongnu.org
> Cc: drjones@redhat.com; Peter Crosthwaite
> <crosthwaite.peter@gmail.com>; Eduardo Habkost <ehabkost@redhat.com>;
> Michael S. Tsirkin <mst@redhat.com>; Xu, Like <like.xu@intel.com>;
> Marcelo Tosatti <mtosatti@redhat.com>; Paolo Bonzini
> <pbonzini@redhat.com>; imammedo@redhat.com; Richard Henderson
> <rth@twiddle.net>
> Subject: Re: [Qemu-devel] [PATCH v1 1/5] cpu: introduce die, the new cpu
> toppolgy emulation level
>
> On 1/14/19 6:24 AM, Like Xu wrote:
> > Following codes on smp_cores, the smp_dies/nr_dies/die-id is added to
> > machine and CPUState. In addition to enable_cpuid_0xb,
> enable_cpuid_0x1f
> > is introduced to track wether host is a new MCP macine or just ignored.
>
> s/wether/whether/, s/macine/machine/
[Xu, Like] Sorry for typos and inconvenience.
>
> > The number for die level_type on Intel is 5 while core type keeps 2.
> >
> > Signed-off-by: Like Xu <like.xu@linux.intel.com>
> > ---
> > cpus.c | 1 +
> > include/qom/cpu.h | 1 +
> > include/sysemu/cpus.h | 1 +
> > qapi/misc.json | 1 +
> > target/i386/cpu.h | 5 +++++
> > 5 files changed, 9 insertions(+)
> >
>
> > +++ b/qapi/misc.json
> > @@ -3229,6 +3229,7 @@
> > { 'struct': 'CpuInstanceProperties',
> > 'data': { '*node-id': 'int',
> > '*socket-id': 'int',
> > + '*die-id': 'int',
>
> Missing documentation of the new field, including a '(since 4.0)' tag.
[Xu, Like] Let me add more docs in next version and thanks.
>
> > '*core-id': 'int',
> > '*thread-id': 'int'
> > }
>
>
> --
> Eric Blake, Principal Software Engineer
> Red Hat, Inc. +1-919-301-3226
> Virtualization: qemu.org | libvirt.org
^ permalink raw reply [flat|nested] 15+ messages in thread
* [Qemu-devel] [PATCH v1 2/5] vl.c: add -smp, dies=* command line support
2019-01-14 12:24 [Qemu-devel] [PATCH v1 0/5] Introduce cpu die topology and enable CPUID.1F for i386 Like Xu
2019-01-14 12:24 ` [Qemu-devel] [PATCH v1 1/5] cpu: introduce die, the new cpu toppolgy emulation level Like Xu
@ 2019-01-14 12:24 ` Like Xu
2019-01-14 20:51 ` Eduardo Habkost
2019-01-14 12:24 ` [Qemu-devel] [PATCH v1 3/5] i386: extend x86_apicid_* functions for smp_dies support Like Xu
` (3 subsequent siblings)
5 siblings, 1 reply; 15+ messages in thread
From: Like Xu @ 2019-01-14 12:24 UTC (permalink / raw)
To: qemu-devel
Cc: like.xu, imammedo, drjones, Michael S. Tsirkin, Marcelo Tosatti,
Marcel Apfelbaum, Eduardo Habkost, Paolo Bonzini,
Peter Crosthwaite, Richard Henderson
This patch updates the check rules on legeacy -smp parse from user command
and it's designed to obey the same restrictions as socket/core/thread model.
Signed-off-by: Like Xu <like.xu@linux.intel.com>
---
hmp.c | 3 +++
hw/core/machine.c | 12 ++++++++++++
vl.c | 33 ++++++++++++++++++++-------------
3 files changed, 35 insertions(+), 13 deletions(-)
diff --git a/hmp.c b/hmp.c
index 80aa5ab..05ac133 100644
--- a/hmp.c
+++ b/hmp.c
@@ -3013,6 +3013,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 95dc7c3..05bc545 100644
--- a/hw/core/machine.c
+++ b/hw/core/machine.c
@@ -601,6 +601,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;
+ }
+
if (props->has_socket_id && !slot->props.has_socket_id) {
error_setg(errp, "socket-id is not supported");
return;
@@ -615,6 +620,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;
}
@@ -849,6 +858,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/vl.c b/vl.c
index 9b8ea3f..72be689 100644
--- a/vl.c
+++ b/vl.c
@@ -169,6 +169,7 @@ int win2k_install_hack = 0;
int singlestep = 0;
int smp_cpus;
unsigned int max_cpus;
+int smp_dies = 1;
int smp_cores = 1;
int smp_threads = 1;
int acpi_enabled = 1;
@@ -1208,6 +1209,9 @@ static QemuOptsList qemu_smp_opts = {
.name = "sockets",
.type = QEMU_OPT_NUMBER,
}, {
+ .name = "dies",
+ .type = QEMU_OPT_NUMBER,
+ }, {
.name = "cores",
.type = QEMU_OPT_NUMBER,
}, {
@@ -1226,32 +1230,34 @@ 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 dies = qemu_opt_get_number(opts, "dies", 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 */
+ dies = dies > 0 ? dies : 1;
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;
+ cpus = cores * threads * dies * sockets;
} else {
max_cpus = qemu_opt_get_number(opts, "maxcpus", cpus);
- sockets = max_cpus / (cores * threads);
+ sockets = 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);
}
@@ -1262,22 +1268,23 @@ static void smp_parse(QemuOpts *opts)
exit(1);
}
- if (sockets * cores * threads > max_cpus) {
+ if (sockets * dies * cores * threads > 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, max_cpus);
+ sockets, dies, cores, threads, max_cpus);
exit(1);
}
- if (sockets * cores * threads != max_cpus) {
+ if (sockets * dies * cores * threads != 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, max_cpus);
+ sockets, dies, cores, threads, max_cpus);
}
smp_cpus = cpus;
+ smp_dies = dies;
smp_cores = cores;
smp_threads = threads;
}
--
1.8.3.1
^ permalink raw reply related [flat|nested] 15+ messages in thread* Re: [Qemu-devel] [PATCH v1 2/5] vl.c: add -smp, dies=* command line support
2019-01-14 12:24 ` [Qemu-devel] [PATCH v1 2/5] vl.c: add -smp, dies=* command line support Like Xu
@ 2019-01-14 20:51 ` Eduardo Habkost
2019-01-15 3:58 ` Xu, Like
2019-01-16 18:26 ` Daniel P. Berrangé
0 siblings, 2 replies; 15+ messages in thread
From: Eduardo Habkost @ 2019-01-14 20:51 UTC (permalink / raw)
To: Like Xu
Cc: qemu-devel, like.xu, imammedo, drjones, Michael S. Tsirkin,
Marcelo Tosatti, Marcel Apfelbaum, Paolo Bonzini,
Peter Crosthwaite, Richard Henderson
On Mon, Jan 14, 2019 at 08:24:56PM +0800, Like Xu wrote:
> This patch updates the check rules on legeacy -smp parse from user command
> and it's designed to obey the same restrictions as socket/core/thread model.
>
> Signed-off-by: Like Xu <like.xu@linux.intel.com>
This would require the documentation for -smp to be updated.
qemu-options.hx still says that "cores=" is the number of cores
per socket.
Also, I'm not completely sure we should change the meaning of
"cores=" and smp_cores to be per-die instead of per-socket. Most
machines won't have any code for tracking dies, so we probably
shouldn't make the extra complexity affect all machines.[1]
What would be the disadvantages of a simple -machine
"dies-per-socket" option, specific for PC?
Keeping core-id and smp_cores per-socket instead of per-die also
seems necessary to keep backwards compatibility on the interface
for identifying CPU hotplug slots. Igor, what do you think?
[1] I would even argue that the rest of the -smp options belong
to the machine object, and topology rules should be
machine-specific, but cleaning this up will require
additional work.
> ---
> hmp.c | 3 +++
> hw/core/machine.c | 12 ++++++++++++
> vl.c | 33 ++++++++++++++++++++-------------
> 3 files changed, 35 insertions(+), 13 deletions(-)
>
> diff --git a/hmp.c b/hmp.c
> index 80aa5ab..05ac133 100644
> --- a/hmp.c
> +++ b/hmp.c
> @@ -3013,6 +3013,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 95dc7c3..05bc545 100644
> --- a/hw/core/machine.c
> +++ b/hw/core/machine.c
> @@ -601,6 +601,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;
> + }
> +
> if (props->has_socket_id && !slot->props.has_socket_id) {
> error_setg(errp, "socket-id is not supported");
> return;
> @@ -615,6 +620,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;
> }
> @@ -849,6 +858,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/vl.c b/vl.c
> index 9b8ea3f..72be689 100644
> --- a/vl.c
> +++ b/vl.c
> @@ -169,6 +169,7 @@ int win2k_install_hack = 0;
> int singlestep = 0;
> int smp_cpus;
> unsigned int max_cpus;
> +int smp_dies = 1;
> int smp_cores = 1;
> int smp_threads = 1;
> int acpi_enabled = 1;
> @@ -1208,6 +1209,9 @@ static QemuOptsList qemu_smp_opts = {
> .name = "sockets",
> .type = QEMU_OPT_NUMBER,
> }, {
> + .name = "dies",
> + .type = QEMU_OPT_NUMBER,
> + }, {
> .name = "cores",
> .type = QEMU_OPT_NUMBER,
> }, {
> @@ -1226,32 +1230,34 @@ 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 dies = qemu_opt_get_number(opts, "dies", 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 */
> + dies = dies > 0 ? dies : 1;
> 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;
> + cpus = cores * threads * dies * sockets;
> } else {
> max_cpus = qemu_opt_get_number(opts, "maxcpus", cpus);
> - sockets = max_cpus / (cores * threads);
> + sockets = 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);
> }
>
> @@ -1262,22 +1268,23 @@ static void smp_parse(QemuOpts *opts)
> exit(1);
> }
>
> - if (sockets * cores * threads > max_cpus) {
> + if (sockets * dies * cores * threads > 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, max_cpus);
> + sockets, dies, cores, threads, max_cpus);
> exit(1);
> }
>
> - if (sockets * cores * threads != max_cpus) {
> + if (sockets * dies * cores * threads != 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, max_cpus);
> + sockets, dies, cores, threads, max_cpus);
> }
>
> smp_cpus = cpus;
> + smp_dies = dies;
> smp_cores = cores;
> smp_threads = threads;
> }
> --
> 1.8.3.1
>
--
Eduardo
^ permalink raw reply [flat|nested] 15+ messages in thread* Re: [Qemu-devel] [PATCH v1 2/5] vl.c: add -smp, dies=* command line support
2019-01-14 20:51 ` Eduardo Habkost
@ 2019-01-15 3:58 ` Xu, Like
2019-01-16 18:26 ` Daniel P. Berrangé
1 sibling, 0 replies; 15+ messages in thread
From: Xu, Like @ 2019-01-15 3:58 UTC (permalink / raw)
To: Eduardo Habkost, Like Xu, igor.v.kovalenko
Cc: qemu-devel, imammedo, drjones, Michael S. Tsirkin,
Marcelo Tosatti, Marcel Apfelbaum, Paolo Bonzini,
Peter Crosthwaite, Richard Henderson
> -----Original Message-----
> From: Eduardo Habkost [mailto:ehabkost@redhat.com]
> Sent: Tuesday, January 15, 2019 4:52 AM
> To: Like Xu <like.xu@linux.intel.com>
> Cc: qemu-devel@nongnu.org; Xu, Like <like.xu@intel.com>;
> imammedo@redhat.com; drjones@redhat.com; Michael S. Tsirkin
> <mst@redhat.com>; Marcelo Tosatti <mtosatti@redhat.com>; Marcel
> Apfelbaum <marcel.apfelbaum@gmail.com>; Paolo Bonzini
> <pbonzini@redhat.com>; Peter Crosthwaite
> <crosthwaite.peter@gmail.com>; Richard Henderson <rth@twiddle.net>
> Subject: Re: [Qemu-devel] [PATCH v1 2/5] vl.c: add -smp,dies=* command
> line support
>
> On Mon, Jan 14, 2019 at 08:24:56PM +0800, Like Xu wrote:
> > This patch updates the check rules on legeacy -smp parse from user
> > command and it's designed to obey the same restrictions as
> socket/core/thread model.
> >
> > Signed-off-by: Like Xu <like.xu@linux.intel.com>
>
> This would require the documentation for -smp to be updated.
> qemu-options.hx still says that "cores=" is the number of cores per socket.
[Xu, Like] I'll add more docs in next version and thanks.
>
> Also, I'm not completely sure we should change the meaning of "cores="
> and smp_cores to be per-die instead of per-socket. Most machines won't
> have any code for tracking dies, so we probably shouldn't make the extra
> complexity affect all machines.[1]
[Xu, Like] I'd prefer to apply die level in a general way without extra affect.
>
> What would be the disadvantages of a simple -machine "dies-per-socket"
> option, specific for PC?
[Xu, Like] It may not be a good choice to cut up cpu topo parser logic and
die level is so generic that any machine provided by qemu as far as I know
could benefit from potential socket/die/core/thread model.
>
> Keeping core-id and smp_cores per-socket instead of per-die also seems
> necessary to keep backwards compatibility on the interface for identifying
> CPU hotplug slots. Igor, what do you think?
[Xu, Like] The compatibility issue on hotplug from MCP challenge is still being
evaluated and Igor, what do you think :D ?
>
>
> [1] I would even argue that the rest of the -smp options belong
> to the machine object, and topology rules should be
> machine-specific, but cleaning this up will require
> additional work.
[Xu, Like] I agree and Intel may have another
two cpu topo levels named module and tile from SDM spec for packaging
and that should be machine-specific as proposal if any. However
the die level I believe is much more generic just like core or thread.
>
> > ---
> > hmp.c | 3 +++
> > hw/core/machine.c | 12 ++++++++++++
> > vl.c | 33 ++++++++++++++++++++-------------
> > 3 files changed, 35 insertions(+), 13 deletions(-)
> >
> > diff --git a/hmp.c b/hmp.c
> > index 80aa5ab..05ac133 100644
> > --- a/hmp.c
> > +++ b/hmp.c
> > @@ -3013,6 +3013,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
> > 95dc7c3..05bc545 100644
> > --- a/hw/core/machine.c
> > +++ b/hw/core/machine.c
> > @@ -601,6 +601,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;
> > + }
> > +
> > if (props->has_socket_id && !slot->props.has_socket_id) {
> > error_setg(errp, "socket-id is not supported");
> > return;
> > @@ -615,6 +620,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;
> > }
> > @@ -849,6 +858,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/vl.c b/vl.c
> > index 9b8ea3f..72be689 100644
> > --- a/vl.c
> > +++ b/vl.c
> > @@ -169,6 +169,7 @@ int win2k_install_hack = 0; int singlestep = 0;
> > int smp_cpus; unsigned int max_cpus;
> > +int smp_dies = 1;
> > int smp_cores = 1;
> > int smp_threads = 1;
> > int acpi_enabled = 1;
> > @@ -1208,6 +1209,9 @@ static QemuOptsList qemu_smp_opts = {
> > .name = "sockets",
> > .type = QEMU_OPT_NUMBER,
> > }, {
> > + .name = "dies",
> > + .type = QEMU_OPT_NUMBER,
> > + }, {
> > .name = "cores",
> > .type = QEMU_OPT_NUMBER,
> > }, {
> > @@ -1226,32 +1230,34 @@ 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 dies = qemu_opt_get_number(opts, "dies", 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 */
> > + dies = dies > 0 ? dies : 1;
> > 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;
> > + cpus = cores * threads * dies * sockets;
> > } else {
> > max_cpus = qemu_opt_get_number(opts, "maxcpus", cpus);
> > - sockets = max_cpus / (cores * threads);
> > + sockets = 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);
> > }
> >
> > @@ -1262,22 +1268,23 @@ static void smp_parse(QemuOpts *opts)
> > exit(1);
> > }
> >
> > - if (sockets * cores * threads > max_cpus) {
> > + if (sockets * dies * cores * threads > 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, max_cpus);
> > + sockets, dies, cores, threads, max_cpus);
> > exit(1);
> > }
> >
> > - if (sockets * cores * threads != max_cpus) {
> > + if (sockets * dies * cores * threads != 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, max_cpus);
> > + sockets, dies, cores, threads, max_cpus);
> > }
> >
> > smp_cpus = cpus;
> > + smp_dies = dies;
> > smp_cores = cores;
> > smp_threads = threads;
> > }
> > --
> > 1.8.3.1
> >
>
> --
> Eduardo
^ permalink raw reply [flat|nested] 15+ messages in thread* Re: [Qemu-devel] [PATCH v1 2/5] vl.c: add -smp, dies=* command line support
2019-01-14 20:51 ` Eduardo Habkost
2019-01-15 3:58 ` Xu, Like
@ 2019-01-16 18:26 ` Daniel P. Berrangé
2019-01-17 1:18 ` Like Xu
1 sibling, 1 reply; 15+ messages in thread
From: Daniel P. Berrangé @ 2019-01-16 18:26 UTC (permalink / raw)
To: Eduardo Habkost
Cc: Like Xu, drjones, Peter Crosthwaite, Michael S. Tsirkin, like.xu,
Marcelo Tosatti, qemu-devel, Paolo Bonzini, imammedo,
Richard Henderson
On Mon, Jan 14, 2019 at 06:51:34PM -0200, Eduardo Habkost wrote:
> On Mon, Jan 14, 2019 at 08:24:56PM +0800, Like Xu wrote:
> > This patch updates the check rules on legeacy -smp parse from user command
> > and it's designed to obey the same restrictions as socket/core/thread model.
> >
> > Signed-off-by: Like Xu <like.xu@linux.intel.com>
>
> This would require the documentation for -smp to be updated.
> qemu-options.hx still says that "cores=" is the number of cores
> per socket.
>
> Also, I'm not completely sure we should change the meaning of
> "cores=" and smp_cores to be per-die instead of per-socket. Most
> machines won't have any code for tracking dies, so we probably
> shouldn't make the extra complexity affect all machines.[1]
Could we not simply have a 'max-dies' property against the machine
base class which defaults to 1. Then no existing machine types
need any changes unless they want to opt-in to supporting
"dies > 1".
> What would be the disadvantages of a simple -machine
> "dies-per-socket" option, specific for PC?
Libvirt currently has
<cpu>
<topology sockets='1' cores='2' threads='1'/>
</cpu>
To me the natural way to expand that is to use
<cpu>
<topology sockets='1' dies='2' cores='2' threads='1'/>
</cpu>
but this rather implies dies-per-socket + cores-per-die
not cores-per-socket. Libvirt could of course convert
its value from cores-per-die into cores-per-socket
before giving it to QEMU, albeit with the potential
for confusion from people comparing the libvirt and QEMU
level configs
> Keeping core-id and smp_cores per-socket instead of per-die also
> seems necessary to keep backwards compatibility on the interface
> for identifying CPU hotplug slots. Igor, what do you think?
Is there really a backwards compatibility problem, given that
no existing mgmt app will have created a VM with "dies != 1".
IOW, if an application adds logic to support configuring a
VM with "dies > 1" it seems fine that they should need to
understand how this impacts the way you identify CPUs for
hotplug.
> [1] I would even argue that the rest of the -smp options belong
> to the machine object, and topology rules should be
> machine-specific, but cleaning this up will require
> additional work.
If we ever expect to support non-homogenous CPUs then our
modelling of topology is fatally flawed, as it doesm't allow
us to specify creating a VM with 1 socket containing 2
cores and a second socket containing 4 cores. Fixing that
might require modelling each socket, die, and core as a
distinct set of nested QOM objects which gets real fun.
Regards,
Daniel
--
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o- https://fstop138.berrange.com :|
|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
^ permalink raw reply [flat|nested] 15+ messages in thread* Re: [Qemu-devel] [PATCH v1 2/5] vl.c: add -smp, dies=* command line support
2019-01-16 18:26 ` Daniel P. Berrangé
@ 2019-01-17 1:18 ` Like Xu
2019-01-17 9:53 ` Daniel P. Berrangé
0 siblings, 1 reply; 15+ messages in thread
From: Like Xu @ 2019-01-17 1:18 UTC (permalink / raw)
To: Daniel P. Berrangé, Eduardo Habkost
Cc: drjones, Peter Crosthwaite, Michael S. Tsirkin, like.xu,
Marcelo Tosatti, qemu-devel, imammedo, Paolo Bonzini,
Richard Henderson
On 2019/1/17 2:26, Daniel P. Berrangé wrote:
> On Mon, Jan 14, 2019 at 06:51:34PM -0200, Eduardo Habkost wrote:
>> On Mon, Jan 14, 2019 at 08:24:56PM +0800, Like Xu wrote:
>>> This patch updates the check rules on legeacy -smp parse from user command
>>> and it's designed to obey the same restrictions as socket/core/thread model.
>>>
>>> Signed-off-by: Like Xu <like.xu@linux.intel.com>
>>
>> This would require the documentation for -smp to be updated.
>> qemu-options.hx still says that "cores=" is the number of cores
>> per socket.
>>
>> Also, I'm not completely sure we should change the meaning of
>> "cores=" and smp_cores to be per-die instead of per-socket. Most
>> machines won't have any code for tracking dies, so we probably
>> shouldn't make the extra complexity affect all machines.[1]
>
> Could we not simply have a 'max-dies' property against the machine
> base class which defaults to 1. Then no existing machine types
> need any changes unless they want to opt-in to supporting
> "dies > 1".
It's nice to have max-dies for machine base class.
>
>> What would be the disadvantages of a simple -machine
>> "dies-per-socket" option, specific for PC?
>
> Libvirt currently has
>
> <cpu>
> <topology sockets='1' cores='2' threads='1'/>
> </cpu>
>
> To me the natural way to expand that is to use
>
> <cpu>
> <topology sockets='1' dies='2' cores='2' threads='1'/>
> </cpu>
>
> but this rather implies dies-per-socket + cores-per-die
> not cores-per-socket. Libvirt could of course convert
> its value from cores-per-die into cores-per-socket
> before giving it to QEMU, albeit with the potential
> for confusion from people comparing the libvirt and QEMU
> level configs
It is a recommended update on cpu topo configuration of libvirt
as well as other upper layer apps.
>
>> Keeping core-id and smp_cores per-socket instead of per-die also
>> seems necessary to keep backwards compatibility on the interface
>> for identifying CPU hotplug slots. Igor, what do you think?
>
> Is there really a backwards compatibility problem, given that
> no existing mgmt app will have created a VM with "dies != 1".
> IOW, if an application adds logic to support configuring a
> VM with "dies > 1" it seems fine that they should need to
> understand how this impacts the way you identify CPUs for
> hotplug.
The impacts from MCP model will be documented continuously.
Any concerns for hot-plugging CPUs in MCP socket is welcomed.
>
>> [1] I would even argue that the rest of the -smp options belong
>> to the machine object, and topology rules should be
>> machine-specific, but cleaning this up will require
>> additional work.
>
> If we ever expect to support non-homogenous CPUs then our
> modelling of topology is fatally flawed, as it doesm't allow
> us to specify creating a VM with 1 socket containing 2
> cores and a second socket containing 4 cores. Fixing that
> might require modelling each socket, die, and core as a
> distinct set of nested QOM objects which gets real fun.
Do we really need to go out of this non-homogeneous step？
Currently there is no support on physical host AFAIK.
Is there enough benefit?
>
>
> Regards,
> Daniel
>
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [Qemu-devel] [PATCH v1 2/5] vl.c: add -smp, dies=* command line support
2019-01-17 1:18 ` Like Xu
@ 2019-01-17 9:53 ` Daniel P. Berrangé
0 siblings, 0 replies; 15+ messages in thread
From: Daniel P. Berrangé @ 2019-01-17 9:53 UTC (permalink / raw)
To: Like Xu
Cc: Eduardo Habkost, drjones, Peter Crosthwaite, Michael S. Tsirkin,
like.xu, Marcelo Tosatti, qemu-devel, imammedo, Paolo Bonzini,
Richard Henderson
On Thu, Jan 17, 2019 at 09:18:29AM +0800, Like Xu wrote:
> On 2019/1/17 2:26, Daniel P. Berrangé wrote:
> > On Mon, Jan 14, 2019 at 06:51:34PM -0200, Eduardo Habkost wrote:
> > > On Mon, Jan 14, 2019 at 08:24:56PM +0800, Like Xu wrote:
> > > > This patch updates the check rules on legeacy -smp parse from user command
> > > > and it's designed to obey the same restrictions as socket/core/thread model.
> > > >
> > > > Signed-off-by: Like Xu <like.xu@linux.intel.com>
> > >
> > > This would require the documentation for -smp to be updated.
> > > qemu-options.hx still says that "cores=" is the number of cores
> > > per socket.
> > >
> > > Also, I'm not completely sure we should change the meaning of
> > > "cores=" and smp_cores to be per-die instead of per-socket. Most
> > > machines won't have any code for tracking dies, so we probably
> > > shouldn't make the extra complexity affect all machines.[1]
> >
> > Could we not simply have a 'max-dies' property against the machine
> > base class which defaults to 1. Then no existing machine types
> > need any changes unless they want to opt-in to supporting
> > "dies > 1".
> It's nice to have max-dies for machine base class.
> >
> > > What would be the disadvantages of a simple -machine
> > > "dies-per-socket" option, specific for PC?
> >
> > Libvirt currently has
> >
> > <cpu>
> > <topology sockets='1' cores='2' threads='1'/>
> > </cpu>
> >
> > To me the natural way to expand that is to use
> >
> > <cpu>
> > <topology sockets='1' dies='2' cores='2' threads='1'/>
> > </cpu>
> >
> > but this rather implies dies-per-socket + cores-per-die
> > not cores-per-socket. Libvirt could of course convert
> > its value from cores-per-die into cores-per-socket
> > before giving it to QEMU, albeit with the potential
> > for confusion from people comparing the libvirt and QEMU
> > level configs
> It is a recommended update on cpu topo configuration of libvirt
> as well as other upper layer apps.
> >
> > > Keeping core-id and smp_cores per-socket instead of per-die also
> > > seems necessary to keep backwards compatibility on the interface
> > > for identifying CPU hotplug slots. Igor, what do you think?
> >
> > Is there really a backwards compatibility problem, given that
> > no existing mgmt app will have created a VM with "dies != 1".
> > IOW, if an application adds logic to support configuring a
> > VM with "dies > 1" it seems fine that they should need to
> > understand how this impacts the way you identify CPUs for
> > hotplug.
> The impacts from MCP model will be documented continuously.
> Any concerns for hot-plugging CPUs in MCP socket is welcomed.
> >
> > > [1] I would even argue that the rest of the -smp options belong
> > > to the machine object, and topology rules should be
> > > machine-specific, but cleaning this up will require
> > > additional work.
> >
> > If we ever expect to support non-homogenous CPUs then our
> > modelling of topology is fatally flawed, as it doesm't allow
> > us to specify creating a VM with 1 socket containing 2
> > cores and a second socket containing 4 cores. Fixing that
> > might require modelling each socket, die, and core as a
> > distinct set of nested QOM objects which gets real fun.
> Do we really need to go out of this non-homogeneous step？
> Currently there is no support on physical host AFAIK.
> Is there enough benefit?
I'm not suggesting we need to solve this now - I just meant to indicate
that we shouldn't over-think representing of the 'dies' parameter today,
because any problems with the simple solution you proposed are negligible
compared to the problem of non-homogeneous CPUs. IOW, I think it is fine
to keep your simple proposal now. Worry about the hard problems later
when we'll need better modelling of everything.
Regards,
Daniel
--
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o- https://fstop138.berrange.com :|
|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
^ permalink raw reply [flat|nested] 15+ messages in thread
* [Qemu-devel] [PATCH v1 3/5] i386: extend x86_apicid_* functions for smp_dies support
2019-01-14 12:24 [Qemu-devel] [PATCH v1 0/5] Introduce cpu die topology and enable CPUID.1F for i386 Like Xu
2019-01-14 12:24 ` [Qemu-devel] [PATCH v1 1/5] cpu: introduce die, the new cpu toppolgy emulation level Like Xu
2019-01-14 12:24 ` [Qemu-devel] [PATCH v1 2/5] vl.c: add -smp, dies=* command line support Like Xu
@ 2019-01-14 12:24 ` Like Xu
2019-01-14 12:24 ` [Qemu-devel] [PATCH v1 4/5] i386: enable CPUID.1F leaf generation based on spec Like Xu
` (2 subsequent siblings)
5 siblings, 0 replies; 15+ messages in thread
From: Like Xu @ 2019-01-14 12:24 UTC (permalink / raw)
To: qemu-devel
Cc: like.xu, imammedo, drjones, Michael S. Tsirkin, Marcelo Tosatti,
Marcel Apfelbaum, Eduardo Habkost, Paolo Bonzini,
Peter Crosthwaite, Richard Henderson
This patch rewrites the width/offset-apicid calculation with compatibility.
It keeps the original symmetry as a default method and sets the die_id
as well as node_id for leageacy numde_node auto-configuration.
Signed-off-by: Like Xu <like.xu@linux.intel.com>
---
hw/i386/pc.c | 37 +++++++++++++++-------
include/hw/i386/topology.h | 79 ++++++++++++++++++++++++++++++++++------------
2 files changed, 84 insertions(+), 32 deletions(-)
diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index fc65049..2f01886 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -909,7 +909,8 @@ static uint32_t x86_cpu_apic_id_from_index(unsigned int cpu_index)
uint32_t correct_id;
static bool warned;
- correct_id = x86_apicid_from_cpu_idx(smp_cores, smp_threads, cpu_index);
+ correct_id = x86_apicid_from_cpu_idx(smp_dies, smp_cores,
+ smp_threads, cpu_index);
if (compat_apic_id_mode) {
if (cpu_index != correct_id && !warned && !qtest_enabled()) {
error_report("APIC IDs set in compatibility mode, "
@@ -2137,9 +2138,12 @@ 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 = (max_cpus - 1) / smp_threads / smp_cores;
+ int max_socket = (max_cpus - 1) / smp_threads / smp_cores / smp_dies;
if (cpu->socket_id < 0) {
error_setg(errp, "CPU socket-id is not set");
@@ -2148,6 +2152,10 @@ static void pc_cpu_pre_plug(HotplugHandler *hotplug_dev,
error_setg(errp, "Invalid CPU socket-id: %u must be in range 0:%u",
cpu->socket_id, max_socket);
return;
+ } else if (cpu->die_id > max_socket) {
+ error_setg(errp, "Invalid CPU die-id: %u must be in range 0:%u",
+ cpu->die_id, max_socket);
+ return;
}
if (cpu->core_id < 0) {
error_setg(errp, "CPU core-id is not set");
@@ -2167,20 +2175,24 @@ 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);
+ cpu->apic_id = apicid_from_topo_ids(smp_dies, smp_cores,
+ smp_threads, &topo);
}
cpu_slot = pc_find_cpu_slot(MACHINE(pcms), cpu->apic_id, &idx);
if (!cpu_slot) {
MachineState *ms = MACHINE(pcms);
- x86_topo_ids_from_apicid(cpu->apic_id, smp_cores, smp_threads, &topo);
- error_setg(errp, "Invalid CPU [socket: %u, core: %u, thread: %u] with"
+ x86_topo_ids_from_apicid(cpu->apic_id, smp_dies, smp_cores,
+ smp_threads, &topo);
+ error_setg(errp,
+ "Invalid CPU [socket: %u, die: %u, core: %u, thread: %u] with"
" APIC ID %" PRIu32 ", valid index range 0:%d",
- topo.pkg_id, topo.core_id, topo.smt_id, cpu->apic_id,
- ms->possible_cpus->len - 1);
+ topo.pkg_id, topo.die_id, topo.core_id, topo.smt_id,
+ cpu->apic_id, ms->possible_cpus->len - 1);
return;
}
@@ -2196,7 +2208,8 @@ static void pc_cpu_pre_plug(HotplugHandler *hotplug_dev,
/* TODO: move socket_id/core_id/thread_id checks into x86_cpu_realizefn()
* once -smp refactoring is complete and there will be CPU private
* CPUState::nr_cores and CPUState::nr_threads fields instead of globals */
- x86_topo_ids_from_apicid(cpu->apic_id, smp_cores, smp_threads, &topo);
+ x86_topo_ids_from_apicid(cpu->apic_id,
+ smp_dies, smp_cores, smp_threads, &topo);
if (cpu->socket_id != -1 && cpu->socket_id != topo.pkg_id) {
error_setg(errp, "property socket-id: %u doesn't match set apic-id:"
" 0x%x (socket-id: %u)", cpu->socket_id, cpu->apic_id, topo.pkg_id);
@@ -2529,7 +2542,7 @@ static int64_t pc_get_default_cpu_node_id(const MachineState *ms, int idx)
assert(idx < ms->possible_cpus->len);
x86_topo_ids_from_apicid(ms->possible_cpus->cpus[idx].arch_id,
- smp_cores, smp_threads, &topo);
+ smp_dies, smp_cores, smp_threads, &topo);
return topo.pkg_id % nb_numa_nodes;
}
@@ -2556,9 +2569,11 @@ 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(i);
x86_topo_ids_from_apicid(ms->possible_cpus->cpus[i].arch_id,
- smp_cores, smp_threads, &topo);
+ smp_dies, smp_cores, 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 1ebaee0..1e344db 100644
--- a/include/hw/i386/topology.h
+++ b/include/hw/i386/topology.h
@@ -47,6 +47,8 @@ typedef uint32_t apic_id_t;
typedef struct X86CPUTopoInfo {
unsigned pkg_id;
+ unsigned node_id;
+ unsigned die_id;
unsigned core_id;
unsigned smt_id;
} X86CPUTopoInfo;
@@ -62,87 +64,122 @@ 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_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_smt_width(nr_cores, 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->node_id = cpu_index / (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);
+ ~(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);
+ topo->node_id = apicid / (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 */
--
1.8.3.1
^ permalink raw reply related [flat|nested] 15+ messages in thread* [Qemu-devel] [PATCH v1 4/5] i386: enable CPUID.1F leaf generation based on spec
2019-01-14 12:24 [Qemu-devel] [PATCH v1 0/5] Introduce cpu die topology and enable CPUID.1F for i386 Like Xu
` (2 preceding siblings ...)
2019-01-14 12:24 ` [Qemu-devel] [PATCH v1 3/5] i386: extend x86_apicid_* functions for smp_dies support Like Xu
@ 2019-01-14 12:24 ` Like Xu
2019-01-14 12:24 ` [Qemu-devel] [PATCH v1 5/5] i386: add CPUID.1F to cpuid_data with host_cpuid check Like Xu
2019-01-17 14:24 ` [Qemu-devel] [PATCH v1 0/5] Introduce cpu die topology and enable CPUID.1F for i386 Igor Mammedov
5 siblings, 0 replies; 15+ messages in thread
From: Like Xu @ 2019-01-14 12:24 UTC (permalink / raw)
To: qemu-devel
Cc: like.xu, imammedo, drjones, Michael S. Tsirkin, Marcelo Tosatti,
Marcel Apfelbaum, Eduardo Habkost, Paolo Bonzini,
Peter Crosthwaite, Richard Henderson
This patch uses the new socket/die/core/thread model to generate
cpuid.1f eax/ebx/ecx/edx values especially the subleaf 2 which
keeps die level information and adds an option in the cpu_x86_cpuid.
Signed-off-by: Like Xu <like.xu@linux.intel.com>
---
target/i386/cpu.c | 57 +++++++++++++++++++++++++++++++++++++++++++++++++------
1 file changed, 51 insertions(+), 6 deletions(-)
diff --git a/target/i386/cpu.c b/target/i386/cpu.c
index fa37203..85f9074 100644
--- a/target/i386/cpu.c
+++ b/target/i386/cpu.c
@@ -4137,7 +4137,7 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t index, uint32_t count,
{
X86CPU *cpu = x86_env_get_cpu(env);
CPUState *cs = CPU(cpu);
- uint32_t pkg_offset;
+ uint32_t die_offset;
uint32_t limit;
uint32_t signature[3];
@@ -4152,7 +4152,7 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t index, uint32_t count,
limit = env->cpuid_level;
}
- if (index > limit) {
+ if (index > limit && index != 0x1F) {
/* Intel documentation states that invalid EAX input will
* return the same information as EAX=cpuid_level
* (Intel SDM Vol. 2A - Instruction Set Reference - CPUID)
@@ -4226,10 +4226,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(cs->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;
}
@@ -4311,12 +4312,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(cs->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(cs->nr_dies, cs->nr_cores,
+ cs->nr_threads);
*ebx = cs->nr_cores * cs->nr_threads;
*ecx |= CPUID_TOPOLOGY_LEVEL_CORE;
break;
@@ -4325,7 +4328,46 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t index, uint32_t count,
*ebx = 0;
*ecx |= CPUID_TOPOLOGY_LEVEL_INVALID;
}
+ assert(!(*eax & ~0x1f));
+ *ebx &= 0xffff; /* The count doesn't need to be reliable. */
+ break;
+ case 0x1F:
+ if (cs->nr_dies > 1 && !cpu->enable_cpuid_0x1f) {
+ printf("Host CPU may not use multi-chip packaging technology.\n");
+ exit(0);
+ }
+ if (cs->nr_dies < 2) {
+ *eax = *ebx = *ecx = *edx = 0;
+ break;
+ }
+
+ *ecx = count & 0xff;
+ *edx = cpu->apic_id;
+ switch (count) {
+ case 0:
+ *eax = apicid_core_offset(cs->nr_dies, cs->nr_cores,
+ cs->nr_threads);
+ *ebx = cs->nr_threads;
+ *ecx |= CPUID_TOPOLOGY_LEVEL_SMT;
+ break;
+ case 1:
+ *eax = apicid_die_offset(cs->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(cs->nr_dies, cs->nr_cores,
+ cs->nr_threads);
+ *ebx = cs->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;
@@ -5709,11 +5751,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),
@@ -5748,6 +5792,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,
--
1.8.3.1
^ permalink raw reply related [flat|nested] 15+ messages in thread* [Qemu-devel] [PATCH v1 5/5] i386: add CPUID.1F to cpuid_data with host_cpuid check
2019-01-14 12:24 [Qemu-devel] [PATCH v1 0/5] Introduce cpu die topology and enable CPUID.1F for i386 Like Xu
` (3 preceding siblings ...)
2019-01-14 12:24 ` [Qemu-devel] [PATCH v1 4/5] i386: enable CPUID.1F leaf generation based on spec Like Xu
@ 2019-01-14 12:24 ` Like Xu
2019-01-17 14:24 ` [Qemu-devel] [PATCH v1 0/5] Introduce cpu die topology and enable CPUID.1F for i386 Igor Mammedov
5 siblings, 0 replies; 15+ messages in thread
From: Like Xu @ 2019-01-14 12:24 UTC (permalink / raw)
To: qemu-devel
Cc: like.xu, imammedo, drjones, Michael S. Tsirkin, Marcelo Tosatti,
Marcel Apfelbaum, Eduardo Habkost, Paolo Bonzini,
Peter Crosthwaite, Richard Henderson
When cs->nr_dies is larger than 1, the CPUID.1F should be generated
and is added to cpuid_data.entries for guest awareness. This patch
provides a return option in kvm_has_cpuid_1f for default choice.
Signed-off-by: Like Xu <like.xu@linux.intel.com>
---
target/i386/kvm.c | 34 +++++++++++++++++++++++++++++++++-
target/i386/kvm_i386.h | 1 +
2 files changed, 34 insertions(+), 1 deletion(-)
diff --git a/target/i386/kvm.c b/target/i386/kvm.c
index 739cf8c..eb0d1ee 100644
--- a/target/i386/kvm.c
+++ b/target/i386/kvm.c
@@ -120,6 +120,17 @@ bool kvm_has_smm(void)
return kvm_check_extension(kvm_state, KVM_CAP_X86_SMM);
}
+bool kvm_has_cpuid_1f(void)
+{
+ uint32_t eax = 0x1f, ecx = 1, ebx = 0, edx = 0;
+ host_cpuid(0x1f, 0, &eax, &ebx, &ecx, &edx);
+ if (eax != 0) {
+ printf("It's recommended to disable CPUID.1F emulation \
+ on Intel non-MCP platform.\n");
+ }
+ return true;
+}
+
bool kvm_has_adjust_clock_stable(void)
{
int ret = kvm_check_extension(kvm_state, KVM_CAP_ADJUST_CLOCK);
@@ -1035,7 +1046,6 @@ int kvm_arch_init_vcpu(CPUState *cs)
}
cpu_x86_cpuid(env, 0, 0, &limit, &unused, &unused, &unused);
-
for (i = 0; i <= limit; i++) {
if (cpuid_i == KVM_MAX_CPUID_ENTRIES) {
fprintf(stderr, "unsupported level value: 0x%x\n", limit);
@@ -1127,6 +1137,28 @@ int kvm_arch_init_vcpu(CPUState *cs)
}
}
+ cpu->enable_cpuid_0x1f = kvm_has_cpuid_1f();
+ if (cs->nr_dies > 1) {
+ i = 0x1f;
+ for (j = 0; ; j++) {
+ c->function = i;
+ c->flags = KVM_CPUID_FLAG_SIGNIFCANT_INDEX;
+ c->index = j;
+ cpu_x86_cpuid(env, i, j, &c->eax, &c->ebx, &c->ecx, &c->edx);
+ if (i == 0x1f && j == 3) {
+ break;
+ }
+ if (cpuid_i == KVM_MAX_CPUID_ENTRIES) {
+ fprintf(stderr, "cpuid_data is full, no space for "
+ "cpuid(eax:0x%x,ecx:0x%x)\n", i, j);
+ abort();
+ }
+ c = &cpuid_data.entries[cpuid_i++];
+ if (!cpu->enable_cpuid_0x1f)
+ break;
+ }
+ }
+
if (limit >= 0x0a) {
uint32_t eax, edx;
diff --git a/target/i386/kvm_i386.h b/target/i386/kvm_i386.h
index 3057ba4..ef9d41e 100644
--- a/target/i386/kvm_i386.h
+++ b/target/i386/kvm_i386.h
@@ -38,6 +38,7 @@ bool kvm_has_adjust_clock_stable(void);
void kvm_synchronize_all_tsc(void);
void kvm_arch_reset_vcpu(X86CPU *cs);
void kvm_arch_do_init_vcpu(X86CPU *cs);
+bool kvm_has_cpuid_1f(void);
int kvm_device_pci_assign(KVMState *s, PCIHostDeviceAddress *dev_addr,
uint32_t flags, uint32_t *dev_id);
--
1.8.3.1
^ permalink raw reply related [flat|nested] 15+ messages in thread* Re: [Qemu-devel] [PATCH v1 0/5] Introduce cpu die topology and enable CPUID.1F for i386
2019-01-14 12:24 [Qemu-devel] [PATCH v1 0/5] Introduce cpu die topology and enable CPUID.1F for i386 Like Xu
` (4 preceding siblings ...)
2019-01-14 12:24 ` [Qemu-devel] [PATCH v1 5/5] i386: add CPUID.1F to cpuid_data with host_cpuid check Like Xu
@ 2019-01-17 14:24 ` Igor Mammedov
2019-01-17 14:51 ` Like Xu
5 siblings, 1 reply; 15+ messages in thread
From: Igor Mammedov @ 2019-01-17 14:24 UTC (permalink / raw)
To: Like Xu
Cc: qemu-devel, drjones, Peter Crosthwaite, Eduardo Habkost,
Michael S. Tsirkin, like.xu, Marcelo Tosatti, Paolo Bonzini,
Richard Henderson
On Mon, 14 Jan 2019 20:24:54 +0800
Like Xu <like.xu@linux.intel.com> wrote:
> As we know, die is a rectangular piece of a semiconductor wafer. It's very common
> that chip manufacturers put a multi-core die in one package and one die always has
> a one-to-one relationship with one socket. Inside the die, it cotains multi-cores
> and core contains threads topologically. We apply this socket/core/thread model to
> the qemu -smp configurable space and save it into APIC_IDs for identification.
>
> The undercurrent Is surging. Multi-chip packaging technology allows for integration
> of multi-die devices in a single package, for example Intel CLX-AP or AMD EPYC.
> Integration can be enabled by high-performance, heterogeneous, multi-dies interconnect
> technology, providing a more cost-effective manner. QEMU and guests may take advantages
> of multi-dies host for such as guest placing or energy efficiency management...
>
> This patch series extend the CPU topology to the socket/dies/core/thread model,
> allowing the setting of dies number per one socket on -smp qemu command. For
> i386, it upgrades APIC_IDs generation and reversion functions with a new exposed
> leaf called CPUID.1F, which is a preferred superset to leaf 0BH. The CPUID.1F
> spec is on https://software.intel.com/en-us/articles/intel-sdm, 3-190 Vol 2A.
Looked into discussion within this mail thread, so here is my thoughts
on mentioned issues:
* I agree with Daniel on the point that "-smp cores" should change 'cores'
meaning to cores within die when die is used.
* I dislike extending smp_parse() though and adding one more global,
as it just adds up more mess to topology thingy QEMU has and puts
an additional burden on whoever comes later to make it right.
After talking with Eduardo, I think it's reasonable to refactor
smp_parse() into machine properties first before adding 'dies',
i.e. 'dies' and the rest of '-smp' should be implemented as machine
properties where 'cpu_dies' is PCMachine specific property.
If it ever becomes useful for other machines we can move common code up
hierarchy to the generic Machine object later and let machine versions take
care of compat issues if any (which is not possible with generic smp_parse()).
So I'd suggest to:
1: make existing cores/threads/sockets into machine properties and
get rid of global variables they use currently
2: #1 should make this trivial: add 'cpu_dies' to PCMachine and
an optional CpuInstanceProperties::die-id with necessary generic glue
where it's necessary.
* for 'cpu_dies', we may use 0 (default) to make old machines work as they
used to (i.e. not reporting dies existence) and new machines could
enable/require dies by default or on request. In this case it should
not cause any migration/hotplug issues.
^ permalink raw reply [flat|nested] 15+ messages in thread* Re: [Qemu-devel] [PATCH v1 0/5] Introduce cpu die topology and enable CPUID.1F for i386
2019-01-17 14:24 ` [Qemu-devel] [PATCH v1 0/5] Introduce cpu die topology and enable CPUID.1F for i386 Igor Mammedov
@ 2019-01-17 14:51 ` Like Xu
0 siblings, 0 replies; 15+ messages in thread
From: Like Xu @ 2019-01-17 14:51 UTC (permalink / raw)
To: Igor Mammedov
Cc: qemu-devel, drjones, Peter Crosthwaite, Eduardo Habkost,
Michael S. Tsirkin, like.xu, Marcelo Tosatti, Paolo Bonzini,
Richard Henderson
On 2019/1/17 22:24, Igor Mammedov wrote:
> On Mon, 14 Jan 2019 20:24:54 +0800
> Like Xu <like.xu@linux.intel.com> wrote:
>
>> As we know, die is a rectangular piece of a semiconductor wafer. It's very common
>> that chip manufacturers put a multi-core die in one package and one die always has
>> a one-to-one relationship with one socket. Inside the die, it cotains multi-cores
>> and core contains threads topologically. We apply this socket/core/thread model to
>> the qemu -smp configurable space and save it into APIC_IDs for identification.
>>
>> The undercurrent Is surging. Multi-chip packaging technology allows for integration
>> of multi-die devices in a single package, for example Intel CLX-AP or AMD EPYC.
>> Integration can be enabled by high-performance, heterogeneous, multi-dies interconnect
>> technology, providing a more cost-effective manner. QEMU and guests may take advantages
>> of multi-dies host for such as guest placing or energy efficiency management...
>>
>> This patch series extend the CPU topology to the socket/dies/core/thread model,
>> allowing the setting of dies number per one socket on -smp qemu command. For
>> i386, it upgrades APIC_IDs generation and reversion functions with a new exposed
>> leaf called CPUID.1F, which is a preferred superset to leaf 0BH. The CPUID.1F
>> spec is on https://software.intel.com/en-us/articles/intel-sdm, 3-190 Vol 2A.
>
> Looked into discussion within this mail thread, so here is my thoughts
> on mentioned issues:
>
> * I agree with Daniel on the point that "-smp cores" should change 'cores'
> meaning to cores within die when die is used.
> * I dislike extending smp_parse() though and adding one more global,
> as it just adds up more mess to topology thingy QEMU has and puts
> an additional burden on whoever comes later to make it right.
> After talking with Eduardo, I think it's reasonable to refactor
> smp_parse() into machine properties first before adding 'dies',
> i.e. 'dies' and the rest of '-smp' should be implemented as machine
> properties where 'cpu_dies' is PCMachine specific property.
> If it ever becomes useful for other machines we can move common code up
> hierarchy to the generic Machine object later and let machine versions take
> care of compat issues if any (which is not possible with generic smp_parse()).
> So I'd suggest to:
> 1: make existing cores/threads/sockets into machine properties and
> get rid of global variables they use currently
> 2: #1 should make this trivial: add 'cpu_dies' to PCMachine and
> an optional CpuInstanceProperties::die-id with necessary generic glue
> where it's necessary.
> * for 'cpu_dies', we may use 0 (default) to make old machines work as they
> used to (i.e. not reporting dies existence) and new machines could
> enable/require dies by default or on request. In this case it should
> not cause any migration/hotplug issues.
>
Hi Igor, thanks for your suggestions on machine properties.
Let me try this additional (RFC) work for the expandable cpu topo model.
^ permalink raw reply [flat|nested] 15+ messages in thread