* [Qemu-devel] [PATCH v6 00/17] s390 cpu model implementation
@ 2015-04-27 14:53 Michael Mueller
2015-04-27 14:53 ` [Qemu-devel] [PATCH v6 01/17] Introduce stub routine cpu_desc_avail Michael Mueller
` (16 more replies)
0 siblings, 17 replies; 55+ messages in thread
From: Michael Mueller @ 2015-04-27 14:53 UTC (permalink / raw)
To: qemu-devel
Cc: Eduardo Habkost, Gleb Natapov, Alexander Graf,
Christian Borntraeger, Daniel Hansel, Jason J. Herne,
Cornelia Huck, Paolo Bonzini, Richard Henderson, Andreas Faerber,
Michael Mueller
This patch set in combination with its kernel kvm patch set proposes an
implementation of S390 CPU models. The origin of this item is to provide
a means for management interfaces like libvirt to draw decisions if life
guest migration to a target hypervisor is reasonable.
A migration constraint is that a target hypervisor is capable to run a
guest with the same CPU model as the source hypervisor does. To
verify this condition, the administration interface employes the existing
QMP command "query-cpu-definitions" which returns a list of all currently
supported CPU models of a given host system. Together with the extension of
QMP command "query-cpus", the current CPU model of a guest is retrievable
and a conclusion can be drawn if a migration is possible.
A S390 CPU model is defined as a triple of machine type, CPU facility set
and IBC value. Each historic, current and future triple receives a name
composed of the machine type and its general availability counter. This name
forms the CPU model name (e.g.: "2817-ga2".)
With means of the Instruction Blocking Control feature (IBC), the instruction
set available to a given guest is limitable.
Examples:
{"execute":"query-cpus"}
{"return":[{"current":true,"CPU":0,"model":"2094-ga3","halted":false,
"accel":"kvm","thread_id":57423},...]}
{"execute":"query-cpu-definitions","arguments":{"accel":"kvm",
"machine":"s390-ccw-virtio"}}
{"return":[{"order":3345,"name":"2964-ga1","live-migration-safe":true},
{"name":"zBC12","runnable":true},
{"name":"2828","runnable":true},
{"order":3105,"name":"2828-ga1","runnable":true,"live-migration-safe":true},
{"name":"host","runnable":true},
{"name":"zEC12","runnable":true},
{"name":"2827","runnable":true},
{"order":3090,"name":"2827-ga2","runnable":true,"live-migration-safe":true",
"is-default":true},
...
{"name":"none","runnable":true}]}
A comment to the optional "order" attribute in the answer of query-cpu-definitions
is welcomed as well.
Thanks a lot
Michael
v5-v6:
- git am reported whitespace issues fixed
- facility bit 74 added to S390Facility enumeration and comment adjusted
- comment for get_machine_props_fallback() added in patch description
- cpu_model_get/set/() arguments now use void pointers instead of uint64_t
- zPCI related facility definitions merged into series
- Christian's ACKs added to series
v4-v5:
- common code patches moved to begin of series
- linux header related patch dropped as changes are already upstream
- ibc_range renamed to ibc and fac_list_mask renamed to fac_mask to
correspond with struct fields in linux header
- test for NULL skipped after strdup_s390_cpu_name()
- strdup_cpu_name() now also handles none cpu model case
- s390_cpu_compare_name() now uses strdup_cpu_name() as well
- omit runnable and is-default field from query-cpu-definitions
answer when they are false
- global variable cpu_models_used and routine s390_cpu_models_used()
dropped
- routine query-cpu-definitions has a common code path now
- CPU model none is always runnable and presented in answer of command
query-cpu-definitions
- live-migration-safe field added to QAPI type CpuDefinitionInfo
- cpu model aliases presented in query-cpu-definitions answer
- CPUState field accel_id added to represent the used accelerator (ACCEL_ID)
and initialized in cpu_generic_init()
- CPUState field model_name added to represent the used cpu model,
initialized in arch specific init function s390_cpu_initfn()
- QMP command query-cpus extended to display accelerator and CPU model
name of all CPUs
- QMP command query-cpu-model dropped in favor to query-cpus
- HMP command info cpus now displays CPU model and accelerator
- optional order field added to query-cpu-definitions
v3-v4:
- qemu probe mode is gone now
- optional parameters 'accel' and 'machine' added to QMP query-cpu-definitions
- z13 related facilities added
- private build directory issue fixed
v2-v3:
- using GTK-Doc style format now for function descriptions
- typo fixed (2/16)
- gen-facilties now used to generate cpu model specific facility lists
and the qemu side facility mask during build time (5/16)
- gen-facilities added to make magic (5/16)
- element of struct S390CPUMachineProps now statically in cpu class (6/16)
- element of struct S390CPUProcessorProps now statically in cpu class (6/16)
- facility list also static now (6/16)
- typo fixed (7/16)
- zBC12-ga1 model now active on zEC12-ga2 host (11/16)
- operations on facility lists use QEMU bitmap API now (11/16)
- routine s390_cpu_model_init() introduced, called during cpu object
realization to prepare the current accelarator (12/16) if a cpu
model was selected
- missing comment added in description of CpuModelInfo type (13/16)
- accelerator field now mandatory for "query-cpu-model" (13/16)
- sorted list related comment to "query-cpu-definitions" dropped in
commit message (13/16)
- comment for AccelCpuInfo type updated (13/16)
- routine s390_facility_test() factored out (15/16)
v1-v2:
- QEMU-side facility list mask introduced: this allows to enable guest
facilities that are handled by instruction interception handlers
implemented on qemu side. Similar to the facilities enabled by means
of the KVM side facility list mask which are handled by kvm/kernel.
- Concept of soft facilities has been dropped
- Result type of QMP command query-cpu-definitions extended to hold
additional information beside the cpu model name including which
cpu model is runnable in current accelerator and machine context.
Michael Mueller (17):
Introduce stub routine cpu_desc_avail
Add accelerator id and model name to CPUState
Extend QMP command query-cpus to return accelerator id and model name
Extend HMP command info cpus to display accelerator id and model name
Add optional parameters to QMP command query-cpu-definitions
target-s390x: Introduce S390 CPU facilities
target-s390x: Generate facility defines per S390 CPU model
target-s390x: Introduce S390 CPU models
target-s390x: Define S390 CPU model specific facility lists
target-s390x: Add S390 CPU model alias definition routines
target-s390x: Add KVM VM attribute interface for S390 CPU models
target-s390x: Add S390 CPU class initialization routines
target-s390x: Prepare accelerator during S390 CPU object realization
target-s390x: Initialize S390 CPU model name in CPUState
target-s390x: Extend arch specific QMP command query-cpu-definitions
target-s390x: Introduce S390 CPU facility test routine
target-s390x: Enable S390 CPU model usage
Makefile.target | 2 +-
cpus.c | 4 +
hmp.c | 7 +
hw/s390x/s390-virtio.c | 12 +-
include/qemu-common.h | 2 +
include/qom/cpu.h | 5 +
include/sysemu/arch_init.h | 7 +-
qapi-schema.json | 44 ++-
qmp-commands.hx | 2 +-
qmp.c | 11 +-
qom/cpu.c | 14 +
rules.mak | 1 +
stubs/Makefile.objs | 1 +
stubs/arch-query-cpu-def.c | 6 +-
stubs/cpu-desc-avail.c | 6 +
target-arm/helper.c | 6 +-
target-i386/cpu.c | 6 +-
target-ppc/translate_init.c | 6 +-
target-s390x/Makefile.objs | 21 ++
target-s390x/cpu-facilities.h | 86 +++++
target-s390x/cpu-models.c | 746 ++++++++++++++++++++++++++++++++++++++++++
target-s390x/cpu-models.h | 168 ++++++++++
target-s390x/cpu-qom.h | 49 +++
target-s390x/cpu.c | 149 ++++++++-
target-s390x/gen-facilities.c | 424 ++++++++++++++++++++++++
target-s390x/helper.c | 9 +-
target-s390x/kvm.c | 129 ++++++++
trace-events | 3 +
vl.c | 2 +-
29 files changed, 1898 insertions(+), 30 deletions(-)
create mode 100644 stubs/cpu-desc-avail.c
create mode 100644 target-s390x/cpu-facilities.h
create mode 100644 target-s390x/cpu-models.c
create mode 100644 target-s390x/cpu-models.h
create mode 100644 target-s390x/gen-facilities.c
--
1.8.3.1
^ permalink raw reply [flat|nested] 55+ messages in thread
* [Qemu-devel] [PATCH v6 01/17] Introduce stub routine cpu_desc_avail
2015-04-27 14:53 [Qemu-devel] [PATCH v6 00/17] s390 cpu model implementation Michael Mueller
@ 2015-04-27 14:53 ` Michael Mueller
2015-05-05 13:55 ` Eduardo Habkost
2015-04-27 14:53 ` [Qemu-devel] [PATCH v6 02/17] Add accelerator id and model name to CPUState Michael Mueller
` (15 subsequent siblings)
16 siblings, 1 reply; 55+ messages in thread
From: Michael Mueller @ 2015-04-27 14:53 UTC (permalink / raw)
To: qemu-devel
Cc: Eduardo Habkost, Gleb Natapov, Alexander Graf,
Christian Borntraeger, Daniel Hansel, Jason J. Herne,
Cornelia Huck, Paolo Bonzini, Richard Henderson, Andreas Faerber,
Michael Mueller
This patch introduces the function cpu_desc_avail() which returns by
default true if not architecture specific implemented. Its intention
is to indicate if the cpu model description is available for display
by list_cpus(). This change allows cpu model descriptions to become
dynamically created by evaluating the runtime context instead of
putting static cpu model information at display.
Signed-off-by: Michael Mueller <mimu@linux.vnet.ibm.com>
Reviewed-by: Thomas Huth <thuth@linux.vnet.ibm.com>
Acked-by: Christian Borntraeger <borntraeger@de.ibm.com>
---
include/qemu-common.h | 2 ++
stubs/Makefile.objs | 1 +
stubs/cpu-desc-avail.c | 6 ++++++
vl.c | 2 +-
4 files changed, 10 insertions(+), 1 deletion(-)
create mode 100644 stubs/cpu-desc-avail.c
diff --git a/include/qemu-common.h b/include/qemu-common.h
index 1b5cffb..386750f 100644
--- a/include/qemu-common.h
+++ b/include/qemu-common.h
@@ -484,4 +484,6 @@ int parse_debug_env(const char *name, int max, int initial);
const char *qemu_ether_ntoa(const MACAddr *mac);
+bool cpu_desc_avail(void);
+
#endif
diff --git a/stubs/Makefile.objs b/stubs/Makefile.objs
index 8beff4c..dce9cd2 100644
--- a/stubs/Makefile.objs
+++ b/stubs/Makefile.objs
@@ -39,3 +39,4 @@ stub-obj-$(CONFIG_WIN32) += fd-register.o
stub-obj-y += cpus.o
stub-obj-y += kvm.o
stub-obj-y += qmp_pc_dimm_device_list.o
+stub-obj-y += cpu-desc-avail.o
diff --git a/stubs/cpu-desc-avail.c b/stubs/cpu-desc-avail.c
new file mode 100644
index 0000000..0cd594e
--- /dev/null
+++ b/stubs/cpu-desc-avail.c
@@ -0,0 +1,6 @@
+#include "qemu-common.h"
+
+bool cpu_desc_avail(void)
+{
+ return true;
+}
diff --git a/vl.c b/vl.c
index 74c2681..c552561 100644
--- a/vl.c
+++ b/vl.c
@@ -3820,7 +3820,7 @@ int main(int argc, char **argv, char **envp)
*/
cpudef_init();
- if (cpu_model && is_help_option(cpu_model)) {
+ if (cpu_model && cpu_desc_avail() && is_help_option(cpu_model)) {
list_cpus(stdout, &fprintf, cpu_model);
exit(0);
}
--
1.8.3.1
^ permalink raw reply related [flat|nested] 55+ messages in thread
* [Qemu-devel] [PATCH v6 02/17] Add accelerator id and model name to CPUState
2015-04-27 14:53 [Qemu-devel] [PATCH v6 00/17] s390 cpu model implementation Michael Mueller
2015-04-27 14:53 ` [Qemu-devel] [PATCH v6 01/17] Introduce stub routine cpu_desc_avail Michael Mueller
@ 2015-04-27 14:53 ` Michael Mueller
2015-05-05 13:26 ` Eduardo Habkost
2015-04-27 14:53 ` [Qemu-devel] [PATCH v6 03/17] Extend QMP command query-cpus to return accelerator id and model name Michael Mueller
` (14 subsequent siblings)
16 siblings, 1 reply; 55+ messages in thread
From: Michael Mueller @ 2015-04-27 14:53 UTC (permalink / raw)
To: qemu-devel
Cc: Eduardo Habkost, Gleb Natapov, Alexander Graf,
Christian Borntraeger, Daniel Hansel, Jason J. Herne,
Cornelia Huck, Paolo Bonzini, Richard Henderson, Andreas Faerber,
Michael Mueller
The patch defines ids per accelerator and adds the accel_id and
the model_name to the CPUState. The accel_id is initialized by
common code, the model name needs to be initialized by target
specific code.
Signed-off-by: Michael Mueller <mimu@linux.vnet.ibm.com>
Acked-by: Christian Borntraeger <borntraeger@de.ibm.com>
---
include/qom/cpu.h | 5 +++++
qapi-schema.json | 9 +++++++++
qom/cpu.c | 14 ++++++++++++++
3 files changed, 28 insertions(+)
diff --git a/include/qom/cpu.h b/include/qom/cpu.h
index 9dafb48..4ffc050 100644
--- a/include/qom/cpu.h
+++ b/include/qom/cpu.h
@@ -236,6 +236,8 @@ struct kvm_run;
* @mem_io_pc: Host Program Counter at which the memory was accessed.
* @mem_io_vaddr: Target virtual address at which the memory was accessed.
* @kvm_fd: vCPU file descriptor for KVM.
+ * @accel_id: accelerator id of this CPU.
+ * @model_name: model name of this CPU
*
* State of one CPU core or thread.
*/
@@ -313,6 +315,9 @@ struct CPUState {
(absolute value) offset as small as possible. This reduces code
size, especially for hosts without large memory offsets. */
volatile sig_atomic_t tcg_exit_req;
+
+ AccelId accel_id;
+ char *model_name;
};
QTAILQ_HEAD(CPUTailQ, CPUState);
diff --git a/qapi-schema.json b/qapi-schema.json
index ac9594d..540e520 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -2515,6 +2515,15 @@
##
{ 'command': 'query-machines', 'returns': ['MachineInfo'] }
+# @AccelId
+#
+# Defines accelerator ids
+#
+# Since: 2.4
+##
+{ 'enum': 'AccelId',
+ 'data': ['qtest', 'tcg', 'kvm', 'xen'] }
+
##
# @CpuDefinitionInfo:
#
diff --git a/qom/cpu.c b/qom/cpu.c
index 108bfa2..457afc7 100644
--- a/qom/cpu.c
+++ b/qom/cpu.c
@@ -67,6 +67,20 @@ CPUState *cpu_generic_init(const char *typename, const char *cpu_model)
goto out;
}
+ if (tcg_enabled()) {
+ cpu->accel_id = ACCEL_ID_TCG;
+ } else if (kvm_enabled()) {
+ cpu->accel_id = ACCEL_ID_KVM;
+ }
+#ifdef CONFIG_XEN
+ else if (xen_enabled()) {
+ cpu->accel_id = ACCEL_ID_XEN;
+ }
+#endif
+ else {
+ cpu->accel_id = ACCEL_ID_QTEST;
+ }
+
object_property_set_bool(OBJECT(cpu), true, "realized", &err);
out:
--
1.8.3.1
^ permalink raw reply related [flat|nested] 55+ messages in thread
* [Qemu-devel] [PATCH v6 03/17] Extend QMP command query-cpus to return accelerator id and model name
2015-04-27 14:53 [Qemu-devel] [PATCH v6 00/17] s390 cpu model implementation Michael Mueller
2015-04-27 14:53 ` [Qemu-devel] [PATCH v6 01/17] Introduce stub routine cpu_desc_avail Michael Mueller
2015-04-27 14:53 ` [Qemu-devel] [PATCH v6 02/17] Add accelerator id and model name to CPUState Michael Mueller
@ 2015-04-27 14:53 ` Michael Mueller
2015-05-05 13:11 ` Eduardo Habkost
2015-04-27 14:53 ` [Qemu-devel] [PATCH v6 04/17] Extend HMP command info cpus to display " Michael Mueller
` (13 subsequent siblings)
16 siblings, 1 reply; 55+ messages in thread
From: Michael Mueller @ 2015-04-27 14:53 UTC (permalink / raw)
To: qemu-devel
Cc: Eduardo Habkost, Gleb Natapov, Alexander Graf,
Christian Borntraeger, Daniel Hansel, Jason J. Herne,
Cornelia Huck, Paolo Bonzini, Richard Henderson, Andreas Faerber,
Michael Mueller
The QMP command query-cpus now additionally displays a model name and
the backing accelerator. Both are omitted if the model name is not
initialized.
request:
{ "execute" : "query-cpus" }
answer:
{ { "current": true,
"CPU": 0,
"model": "2827-ga2",
"halted": false,
"accel": "kvm",
"thread_id": 31917
}, ... }
Signed-off-by: Michael Mueller <mimu@linux.vnet.ibm.com>
Acked-by: Christian Borntraeger <borntraeger@de.ibm.com>
---
cpus.c | 4 ++++
qapi-schema.json | 7 ++++++-
2 files changed, 10 insertions(+), 1 deletion(-)
diff --git a/cpus.c b/cpus.c
index e6dcae3..00f33b9 100644
--- a/cpus.c
+++ b/cpus.c
@@ -1436,6 +1436,10 @@ CpuInfoList *qmp_query_cpus(Error **errp)
info->value->current = (cpu == first_cpu);
info->value->halted = cpu->halted;
info->value->thread_id = cpu->thread_id;
+ info->value->model = g_strdup(cpu->model_name);
+ info->value->accel = cpu->accel_id;
+ info->value->has_model = info->value->has_accel =
+ info->value->model != NULL;
#if defined(TARGET_I386)
info->value->has_pc = true;
info->value->pc = env->eip + env->segs[R_CS].base;
diff --git a/qapi-schema.json b/qapi-schema.json
index 540e520..215a7bc 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -616,6 +616,10 @@
#
# @thread_id: ID of the underlying host thread
#
+# @accel: #optional accelerator id (since 2.4)
+#
+# @model: #optional cpu model name (since 2.4)
+#
# Since: 0.14.0
#
# Notes: @halted is a transient state that changes frequently. By the time the
@@ -623,7 +627,8 @@
##
{ 'type': 'CpuInfo',
'data': {'CPU': 'int', 'current': 'bool', 'halted': 'bool', '*pc': 'int',
- '*nip': 'int', '*npc': 'int', '*PC': 'int', 'thread_id': 'int'} }
+ '*nip': 'int', '*npc': 'int', '*PC': 'int', 'thread_id': 'int',
+ '*accel': 'AccelId', '*model': 'str'} }
##
# @query-cpus:
--
1.8.3.1
^ permalink raw reply related [flat|nested] 55+ messages in thread
* [Qemu-devel] [PATCH v6 04/17] Extend HMP command info cpus to display accelerator id and model name
2015-04-27 14:53 [Qemu-devel] [PATCH v6 00/17] s390 cpu model implementation Michael Mueller
` (2 preceding siblings ...)
2015-04-27 14:53 ` [Qemu-devel] [PATCH v6 03/17] Extend QMP command query-cpus to return accelerator id and model name Michael Mueller
@ 2015-04-27 14:53 ` Michael Mueller
2015-05-05 13:14 ` Eduardo Habkost
2015-04-27 14:53 ` [Qemu-devel] [PATCH v6 05/17] Add optional parameters to QMP command query-cpu-definitions Michael Mueller
` (12 subsequent siblings)
16 siblings, 1 reply; 55+ messages in thread
From: Michael Mueller @ 2015-04-27 14:53 UTC (permalink / raw)
To: qemu-devel
Cc: Eduardo Habkost, Gleb Natapov, Alexander Graf,
Christian Borntraeger, Daniel Hansel, Jason J. Herne,
Cornelia Huck, Paolo Bonzini, Richard Henderson, Andreas Faerber,
Michael Mueller
The HMP command info cpus now displays the CPU model name and the
backing accelerator if part of the CPUState.
(qemu) info cpus
* CPU #0: (halted) model=2827-ga2 accel=kvm thread_id=1679
Signed-off-by: Michael Mueller <mimu@linux.vnet.ibm.com>
Acked-by: Christian Borntraeger <borntraeger@de.ibm.com>
---
hmp.c | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/hmp.c b/hmp.c
index f142d36..676d821 100644
--- a/hmp.c
+++ b/hmp.c
@@ -290,6 +290,13 @@ void hmp_info_cpus(Monitor *mon, const QDict *qdict)
monitor_printf(mon, " (halted)");
}
+ if (cpu->value->has_model) {
+ monitor_printf(mon, " model=%s", cpu->value->model);
+ }
+ if (cpu->value->has_accel) {
+ monitor_printf(mon, " accel=%s", AccelId_lookup[cpu->value->accel]);
+ }
+
monitor_printf(mon, " thread_id=%" PRId64 "\n", cpu->value->thread_id);
}
--
1.8.3.1
^ permalink raw reply related [flat|nested] 55+ messages in thread
* [Qemu-devel] [PATCH v6 05/17] Add optional parameters to QMP command query-cpu-definitions
2015-04-27 14:53 [Qemu-devel] [PATCH v6 00/17] s390 cpu model implementation Michael Mueller
` (3 preceding siblings ...)
2015-04-27 14:53 ` [Qemu-devel] [PATCH v6 04/17] Extend HMP command info cpus to display " Michael Mueller
@ 2015-04-27 14:53 ` Michael Mueller
2015-05-06 12:42 ` Eduardo Habkost
2015-04-27 14:53 ` [Qemu-devel] [PATCH v6 06/17] target-s390x: Introduce S390 CPU facilities Michael Mueller
` (11 subsequent siblings)
16 siblings, 1 reply; 55+ messages in thread
From: Michael Mueller @ 2015-04-27 14:53 UTC (permalink / raw)
To: qemu-devel
Cc: Eduardo Habkost, Gleb Natapov, Alexander Graf,
Christian Borntraeger, Daniel Hansel, Jason J. Herne,
Cornelia Huck, Paolo Bonzini, Richard Henderson, Andreas Faerber,
Michael Mueller
The patch adds optional parameters to the QMP command query-cpu-definitions.
Thus the signature of routine arch_query_cpu_definitions needs to be changed
for the stub function and all target implementations:
target-arm
target-i386
target-ppc
target-s390
Signed-off-by: Michael Mueller <mimu@linux.vnet.ibm.com>
Acked-by: Christian Borntraeger <borntraeger@de.ibm.com>
---
include/sysemu/arch_init.h | 7 +++++--
qapi-schema.json | 28 +++++++++++++++++++++++++---
qmp-commands.hx | 2 +-
qmp.c | 11 ++++++++---
stubs/arch-query-cpu-def.c | 6 +++++-
target-arm/helper.c | 6 +++++-
target-i386/cpu.c | 6 +++++-
target-ppc/translate_init.c | 6 +++++-
target-s390x/cpu.c | 6 +++++-
9 files changed, 64 insertions(+), 14 deletions(-)
diff --git a/include/sysemu/arch_init.h b/include/sysemu/arch_init.h
index 54b36c1..0d4bf51 100644
--- a/include/sysemu/arch_init.h
+++ b/include/sysemu/arch_init.h
@@ -36,6 +36,9 @@ void audio_init(void);
int kvm_available(void);
int xen_available(void);
-CpuDefinitionInfoList *arch_query_cpu_definitions(Error **errp);
-
+CpuDefinitionInfoList *arch_query_cpu_definitions(bool has_machine,
+ const char *machine,
+ bool has_accel,
+ AccelId accel,
+ Error **errp);
#endif
diff --git a/qapi-schema.json b/qapi-schema.json
index 215a7bc..285b2d3 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -2536,21 +2536,43 @@
#
# @name: the name of the CPU definition
#
+# @default: #optional true if cpu model is the default,
+# omitted if false (since 2.4)
+#
+# @runnable: #optional true if cpu model is runnable,
+# omitted if false (since 2.4)
+#
+# @live-migration-safe: #optional true if cpu model represents a
+# cpu model that is safely migratable
+# omitted if false (since 2.4)
+#
+# @order: #optional order criterion
+#
# Since: 1.2.0
##
{ 'type': 'CpuDefinitionInfo',
- 'data': { 'name': 'str' } }
+ 'data': { 'name': 'str', '*is-default': 'bool', '*runnable': 'bool',
+ '*live-migration-safe': 'bool', '*order': 'int' } }
##
# @query-cpu-definitions:
#
-# Return a list of supported virtual CPU definitions
+# Return a list of supported virtual CPU definitions. In context with the
+# optional parameters @machine and @accel the returned list contains
+# also information if the respective cpu definition is runnable or the
+# default to be used.
+#
+# @machine: #optional machine type (since 2.4)
+#
+# @accel: #optional accelerator id (since 2.4)
#
# Returns: a list of CpuDefInfo
#
# Since: 1.2.0
##
-{ 'command': 'query-cpu-definitions', 'returns': ['CpuDefinitionInfo'] }
+{ 'command': 'query-cpu-definitions',
+ 'data': { '*machine': 'str', '*accel': 'AccelId' },
+ 'returns': ['CpuDefinitionInfo'] }
# @AddfdInfo:
#
diff --git a/qmp-commands.hx b/qmp-commands.hx
index 3a42ad0..6592e2c 100644
--- a/qmp-commands.hx
+++ b/qmp-commands.hx
@@ -3412,7 +3412,7 @@ EQMP
{
.name = "query-cpu-definitions",
- .args_type = "",
+ .args_type = "machine:s?,accel:s?",
.mhandler.cmd_new = qmp_marshal_input_query_cpu_definitions,
},
diff --git a/qmp.c b/qmp.c
index e6c7050..16a24d1 100644
--- a/qmp.c
+++ b/qmp.c
@@ -567,9 +567,14 @@ DevicePropertyInfoList *qmp_device_list_properties(const char *typename,
return prop_list;
}
-CpuDefinitionInfoList *qmp_query_cpu_definitions(Error **errp)
-{
- return arch_query_cpu_definitions(errp);
+CpuDefinitionInfoList *qmp_query_cpu_definitions(bool has_machine,
+ const char *machine,
+ bool has_accel,
+ AccelId accel,
+ Error **errp)
+{
+ return arch_query_cpu_definitions(has_machine, machine,
+ has_accel, accel, errp);
}
void qmp_add_client(const char *protocol, const char *fdname,
diff --git a/stubs/arch-query-cpu-def.c b/stubs/arch-query-cpu-def.c
index 22e0b43..6f8904e 100644
--- a/stubs/arch-query-cpu-def.c
+++ b/stubs/arch-query-cpu-def.c
@@ -2,7 +2,11 @@
#include "sysemu/arch_init.h"
#include "qapi/qmp/qerror.h"
-CpuDefinitionInfoList *arch_query_cpu_definitions(Error **errp)
+CpuDefinitionInfoList *arch_query_cpu_definitions(bool has_machine,
+ const char *machine,
+ bool has_accel,
+ AccelId accel,
+ Error **errp)
{
error_set(errp, QERR_UNSUPPORTED);
return NULL;
diff --git a/target-arm/helper.c b/target-arm/helper.c
index d77c6de..41ff2be 100644
--- a/target-arm/helper.c
+++ b/target-arm/helper.c
@@ -3535,7 +3535,11 @@ static void arm_cpu_add_definition(gpointer data, gpointer user_data)
*cpu_list = entry;
}
-CpuDefinitionInfoList *arch_query_cpu_definitions(Error **errp)
+CpuDefinitionInfoList *arch_query_cpu_definitions(bool has_machine,
+ const char *machine,
+ bool has_accel,
+ AccelId accel,
+ Error **errp)
{
CpuDefinitionInfoList *cpu_list = NULL;
GSList *list;
diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index 03b33cf..3c1c8db 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -2023,7 +2023,11 @@ void x86_cpu_list(FILE *f, fprintf_function cpu_fprintf)
}
}
-CpuDefinitionInfoList *arch_query_cpu_definitions(Error **errp)
+CpuDefinitionInfoList *arch_query_cpu_definitions(bool has_machine,
+ const char *machine,
+ bool has_accel,
+ AccelId accel,
+ Error **errp)
{
CpuDefinitionInfoList *cpu_list = NULL;
X86CPUDefinition *def;
diff --git a/target-ppc/translate_init.c b/target-ppc/translate_init.c
index d74f4f0..2729b9f 100644
--- a/target-ppc/translate_init.c
+++ b/target-ppc/translate_init.c
@@ -9472,7 +9472,11 @@ static void ppc_cpu_defs_entry(gpointer data, gpointer user_data)
*first = entry;
}
-CpuDefinitionInfoList *arch_query_cpu_definitions(Error **errp)
+CpuDefinitionInfoList *arch_query_cpu_definitions(bool has_machine,
+ const char *machine,
+ bool has_accel,
+ AccelId accel,
+ Error **errp)
{
CpuDefinitionInfoList *cpu_list = NULL;
GSList *list;
diff --git a/target-s390x/cpu.c b/target-s390x/cpu.c
index e0537fa..4d43ace 100644
--- a/target-s390x/cpu.c
+++ b/target-s390x/cpu.c
@@ -45,7 +45,11 @@ void s390_cpu_list(FILE *f, fprintf_function cpu_fprintf)
}
#ifndef CONFIG_USER_ONLY
-CpuDefinitionInfoList *arch_query_cpu_definitions(Error **errp)
+CpuDefinitionInfoList *arch_query_cpu_definitions(bool has_machine,
+ const char *machine,
+ bool has_accel,
+ AccelId accel,
+ Error **errp)
{
CpuDefinitionInfoList *entry;
CpuDefinitionInfo *info;
--
1.8.3.1
^ permalink raw reply related [flat|nested] 55+ messages in thread
* [Qemu-devel] [PATCH v6 06/17] target-s390x: Introduce S390 CPU facilities
2015-04-27 14:53 [Qemu-devel] [PATCH v6 00/17] s390 cpu model implementation Michael Mueller
` (4 preceding siblings ...)
2015-04-27 14:53 ` [Qemu-devel] [PATCH v6 05/17] Add optional parameters to QMP command query-cpu-definitions Michael Mueller
@ 2015-04-27 14:53 ` Michael Mueller
2015-04-27 14:53 ` [Qemu-devel] [PATCH v6 07/17] target-s390x: Generate facility defines per S390 CPU model Michael Mueller
` (10 subsequent siblings)
16 siblings, 0 replies; 55+ messages in thread
From: Michael Mueller @ 2015-04-27 14:53 UTC (permalink / raw)
To: qemu-devel
Cc: Eduardo Habkost, Gleb Natapov, Alexander Graf,
Christian Borntraeger, Daniel Hansel, Jason J. Herne,
Cornelia Huck, Paolo Bonzini, Richard Henderson, Andreas Faerber,
Michael Mueller
The patch introduces S390 CPU facility bit numbers and names
as well as the architectural facility size limit in bytes.
Signed-off-by: Michael Mueller <mimu@linux.vnet.ibm.com>
Acked-by: Christian Borntraeger <borntraeger@de.ibm.com>
---
target-s390x/cpu-facilities.h | 86 +++++++++++++++++++++++++++++++++++++++++++
1 file changed, 86 insertions(+)
create mode 100644 target-s390x/cpu-facilities.h
diff --git a/target-s390x/cpu-facilities.h b/target-s390x/cpu-facilities.h
new file mode 100644
index 0000000..d8ab5ba
--- /dev/null
+++ b/target-s390x/cpu-facilities.h
@@ -0,0 +1,86 @@
+/*
+ * CPU facilities for s390
+ *
+ * Copyright 2015 IBM Corp.
+ *
+ * Author(s): Michael Mueller <mimu@linux.vnet.ibm.com>
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or (at
+ * your option) any later version. See the COPYING file in the top-level
+ * directory.
+ */
+
+#ifndef TARGET_S390X_CPU_FACILITIES_H
+#define TARGET_S390X_CPU_FACILITIES_H
+
+/* architectural size of facilities is 2KB */
+#define FAC_LIST_ARCH_S390_SIZE_UINT8 (1<<11)
+
+/* CPU facility bits */
+typedef enum {
+ FAC_N3 = 0,
+ FAC_ZARCH = 1,
+ FAC_ZARCH_ACTIVE = 2,
+ FAC_DAT_ENH = 3,
+ FAC_ASN_LX_REUSE = 6,
+ FAC_STFLE = 7,
+ FAC_ENHANCED_DAT_1 = 8,
+ FAC_SENSE_RUNNING_STATUS = 9,
+ FAC_CONDITIONAL_SSKE = 10,
+ FAC_CONFIGURATION_TOPOLOGY = 11,
+ FAC_IPTE_RANGE = 13,
+ FAC_NONQ_KEY_SETTING = 14,
+ FAC_EXTENDED_TRANSLATION_2 = 16,
+ FAC_MESSAGE_SECURITY_ASSIST = 17,
+ FAC_LONG_DISPLACEMENT = 18,
+ FAC_LONG_DISPLACEMENT_FAST = 19,
+ FAC_HFP_MADDSUB = 20,
+ FAC_EXTENDED_IMMEDIATE = 21,
+ FAC_EXTENDED_TRANSLATION_3 = 22,
+ FAC_HFP_UNNORMALIZED_EXT = 23,
+ FAC_ETF2_ENH = 24,
+ FAC_STORE_CLOCK_FAST = 25,
+ FAC_PARSING_ENH = 26,
+ FAC_MOVE_WITH_OPTIONAL_SPEC = 27,
+ FAC_TOD_CLOCK_STEERING = 28,
+ FAC_ETF3_ENH = 30,
+ FAC_EXTRACT_CPU_TIME = 31,
+ FAC_COMPARE_AND_SWAP_AND_STORE = 32,
+ FAC_COMPARE_AND_SWAP_AND_STORE_2 = 33,
+ FAC_GENERAL_INSTRUCTIONS_EXT = 34,
+ FAC_EXECUTE_EXT = 35,
+ FAC_ENHANCED_MONITOR = 36,
+ FAC_FLOATING_POINT_EXT = 37,
+ FAC_LOAD_PROGRAM_PARAMETERS = 40,
+ FAC_FLOATING_POINT_SUPPPORT_ENH = 41,
+ FAC_DFP = 42,
+ FAC_DFP_FAST = 43,
+ FAC_PFPO = 44,
+ FAC_MULTI_45 = 45,
+ FAC_CMPSC_ENH = 47,
+ FAC_DFP_ZONED_CONVERSION = 48,
+ FAC_MULTI_49 = 49,
+ FAC_CONSTRAINT_TRANSACTIONAL_EXE = 50,
+ FAC_LOCAL_TLB_CLEARING = 51,
+ FAC_INTERLOCKED_ACCESS_2 = 52,
+ FAC_LOAD_STORE_ON_COND_2 = 53,
+ FAC_MESSAGE_SECURITY_ASSIST_5 = 57,
+ FAC_RESET_REFERENCE_BITS_MULTIPLE = 66,
+ FAC_CPU_MEASUREMENT_COUNTER = 67,
+ FAC_CPU_MEASUREMENT_SAMPLING = 68,
+ FAC_ZPCI = 69,
+ FAC_ADAPTER_EVENT_NOTIFICATION = 71,
+ FAC_ADAPTER_INT_SUPPRESSION = 72,
+ FAC_TRANSACTIONAL_EXE = 73,
+ /* z/VM-specific, see: SC24-6179-05 page 953) */
+ FAC_STHYI = 74,
+ FAC_ACCESS_EXCEPTION_FS_INDICATION = 75,
+ FAC_MESSAGE_SECURITY_ASSIST_3 = 76,
+ FAC_MESSAGE_SECURITY_ASSIST_4 = 77,
+ FAC_ENHANCED_DAT_2 = 78,
+ FAC_DFP_PACKED_CONVERSION = 80,
+ FAC_VECTOR = 129,
+ FAC_STORE_CPU_COUNTER_MULTI = 142,
+} S390Facility;
+
+#endif
--
1.8.3.1
^ permalink raw reply related [flat|nested] 55+ messages in thread
* [Qemu-devel] [PATCH v6 07/17] target-s390x: Generate facility defines per S390 CPU model
2015-04-27 14:53 [Qemu-devel] [PATCH v6 00/17] s390 cpu model implementation Michael Mueller
` (5 preceding siblings ...)
2015-04-27 14:53 ` [Qemu-devel] [PATCH v6 06/17] target-s390x: Introduce S390 CPU facilities Michael Mueller
@ 2015-04-27 14:53 ` Michael Mueller
2015-04-27 14:53 ` [Qemu-devel] [PATCH v6 08/17] target-s390x: Introduce S390 CPU models Michael Mueller
` (9 subsequent siblings)
16 siblings, 0 replies; 55+ messages in thread
From: Michael Mueller @ 2015-04-27 14:53 UTC (permalink / raw)
To: qemu-devel
Cc: Eduardo Habkost, Gleb Natapov, Alexander Graf,
Christian Borntraeger, Daniel Hansel, Jason J. Herne,
Cornelia Huck, Paolo Bonzini, Richard Henderson, Andreas Faerber,
Michael Mueller
This patch introduces the helper "gen-facilities" which allows to generate
facility list definitions and masks at compile time. Its flexibility is
better and the error-proneness is lower when compared to static programming
time added statements.
The helper includes "target-s390x/cpu-facilities.h" to be able to use named
facility bits instead of numbers. Its output will be feed back into the
cpu model related header file "target-s390x/cpu-models.h" by including
gen-facilities.h to implement model related data structures.
The following defines/symbols are expected to be provided by the cpu-facilities
header file:
FAC_LIST_ARCH_S390_SIZE_UINT8
FAC_N3
FAC_ZARCH
FAC_ZARCH_ACTIVE
...
The defines provided by gen-facilities follow the following schema:
FAC_LIST_CPU_S390_SIZE_UINT1 %PRIu32
FAC_LIST_CPU_S390_SIZE_UINT8 %PRIu32
FAC_LIST_CPU_S390_SIZE_UINT64 %PRIu32
FAC_LIST_CPU_S390_MASK_QEMU 0x%016PRIx64,0x%016PRIx64,...
FAC_LIST_CPU_S390_<TYPE>_GA<n> 0x%016PRIx64,0x%016PRIx64,...
Signed-off-by: Michael Mueller <mimu@linux.vnet.ibm.com>
Acked-by: Christian Borntraeger <borntraeger@de.ibm.com>
---
Makefile.target | 2 +-
rules.mak | 1 +
target-s390x/Makefile.objs | 20 ++
target-s390x/gen-facilities.c | 424 ++++++++++++++++++++++++++++++++++++++++++
4 files changed, 446 insertions(+), 1 deletion(-)
create mode 100644 target-s390x/gen-facilities.c
diff --git a/Makefile.target b/Makefile.target
index 2262d89..58cfc1b 100644
--- a/Makefile.target
+++ b/Makefile.target
@@ -190,7 +190,7 @@ hmp-commands.h: $(SRC_PATH)/hmp-commands.hx
qmp-commands-old.h: $(SRC_PATH)/qmp-commands.hx
$(call quiet-command,sh $(SRC_PATH)/scripts/hxtool -h < $< > $@," GEN $(TARGET_DIR)$@")
-clean:
+clean: clean-target
rm -f *.a *~ $(PROGS)
rm -f $(shell find . -name '*.[od]')
rm -f hmp-commands.h qmp-commands-old.h gdbstub-xml.c
diff --git a/rules.mak b/rules.mak
index 3a05627..43cf05c 100644
--- a/rules.mak
+++ b/rules.mak
@@ -12,6 +12,7 @@ MAKEFLAGS += -rR
%.cpp:
%.m:
%.mak:
+clean-target:
# Flags for C++ compilation
QEMU_CXXFLAGS = -D__STDC_LIMIT_MACROS $(filter-out -Wstrict-prototypes -Wmissing-prototypes -Wnested-externs -Wold-style-declaration -Wold-style-definition -Wredundant-decls, $(QEMU_CFLAGS))
diff --git a/target-s390x/Makefile.objs b/target-s390x/Makefile.objs
index dd62cbd..997dda4 100644
--- a/target-s390x/Makefile.objs
+++ b/target-s390x/Makefile.objs
@@ -3,3 +3,23 @@ obj-y += int_helper.o fpu_helper.o cc_helper.o mem_helper.o misc_helper.o
obj-y += gdbstub.o
obj-$(CONFIG_SOFTMMU) += machine.o ioinst.o arch_dump.o mmu_helper.o
obj-$(CONFIG_KVM) += kvm.o
+
+# build and run facility generator
+#
+fac = gen-facilities
+fac-src = $(SRC_PATH)/target-$(TARGET_BASE_ARCH)
+fac-dst = $(BUILD_DIR)/$(TARGET_DIR)
+
+ifneq ($(MAKECMDGOALS),clean)
+GENERATED_HEADERS += $(fac-dst)$(fac).h
+endif
+
+$(fac-dst)$(fac).h: $(fac-dst)$(fac)
+ $(call quiet-command,$< >$@," GEN $(TARGET_DIR)$(fac).h")
+
+$(fac-dst)$(fac): $(fac-src)/$(fac).c $(fac-src)/cpu-facilities.h
+ $(call quiet-command,$(CC) $(QEMU_INCLUDES) $(QEMU_CFLAGS) $(CFLAGS) -o $@ $<," CC $(TARGET_DIR)$(fac)")
+
+clean-target:
+ rm -f $(fac).h
+ rm -f $(fac)
diff --git a/target-s390x/gen-facilities.c b/target-s390x/gen-facilities.c
new file mode 100644
index 0000000..4b9c6a2
--- /dev/null
+++ b/target-s390x/gen-facilities.c
@@ -0,0 +1,424 @@
+/*
+ * S390 facility list/mask generator
+ *
+ * Copyright 2015 IBM Corp.
+ *
+ * Author(s): Michael Mueller <mimu@linux.vnet.ibm.com>
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or (at
+ * your option) any later version. See the COPYING file in the top-level
+ * directory.
+ *
+ */
+
+#include <stdio.h>
+#include <stdlib.h>
+#include <stdint.h>
+#include <inttypes.h>
+#include <assert.h>
+#include <values.h>
+#include "cpu-facilities.h"
+
+/***** BEGIN FACILITY DEFS *****/
+
+/*******************************
+ * CMOS G7 processors
+ *******************************/
+
+/* 2064-GA1 */
+static uint16_t set_2064_GA1[] = {
+ FAC_N3,
+ FAC_ZARCH,
+ FAC_ZARCH_ACTIVE,
+};
+#define clear_2064_GA1 EmptyFacs
+
+/* 2064-GA2 */
+static uint16_t set_2064_GA2[] = {
+ FAC_EXTENDED_TRANSLATION_2,
+};
+#define clear_2064_GA2 EmptyFacs
+
+/* 2064-GA3 */
+#define set_2064_GA3 EmptyFacs
+#define clear_2064_GA3 EmptyFacs
+
+/* 2066-GA1 */
+#define set_2066_GA1 EmptyFacs
+#define clear_2066_GA1 EmptyFacs
+
+/*******************************
+ * CMOS G8 processors
+ *******************************/
+
+/* 2084-GA1 */
+static uint16_t set_2084_GA1[] = {
+ FAC_DAT_ENH,
+ FAC_MESSAGE_SECURITY_ASSIST,
+ FAC_LONG_DISPLACEMENT,
+ FAC_LONG_DISPLACEMENT_FAST,
+ FAC_HFP_MADDSUB,
+};
+#define clear_2084_GA1 EmptyFacs
+
+/* 2084-GA2 */
+static uint16_t set_2084_GA2[] = {
+ 4,
+};
+#define clear_2084_GA2 EmptyFacs
+
+/* 2084-GA3 */
+static uint16_t set_2084_GA3[] = {
+ FAC_ASN_LX_REUSE,
+ FAC_EXTENDED_TRANSLATION_3,
+};
+#define clear_2084_GA3 EmptyFacs
+
+/* 2084-GA4 */
+#define set_2084_GA4 EmptyFacs
+#define clear_2084_GA4 EmptyFacs
+
+/* 2084-GA5 */
+static uint16_t set_2084_GA5[] = {
+ FAC_TOD_CLOCK_STEERING,
+};
+#define clear_2084_GA5 EmptyFacs
+
+/* 2086-GA1 */
+#define set_2086_GA1 EmptyFacs
+#define clear_2086_GA1 EmptyFacs
+
+/* 2086-GA2 */
+#define set_2086_GA2 EmptyFacs
+#define clear_2086_GA2 EmptyFacs
+
+/* 2086-GA3 */
+#define set_2086_GA3 EmptyFacs
+#define clear_2086_GA3 EmptyFacs
+
+/*******************************
+ * CMOS G9 processors
+ *******************************/
+
+/* 2094-GA1 */
+static uint16_t set_2094_GA1[] = {
+ FAC_STFLE,
+ FAC_EXTENDED_IMMEDIATE,
+ FAC_HFP_UNNORMALIZED_EXT,
+ FAC_ETF2_ENH,
+ FAC_STORE_CLOCK_FAST,
+ FAC_ETF3_ENH,
+ FAC_EXTRACT_CPU_TIME,
+};
+#define clear_2094_GA1 EmptyFacs
+
+/* 2094-GA2 */
+static uint16_t set_2094_GA2[] = {
+ FAC_SENSE_RUNNING_STATUS,
+ FAC_MOVE_WITH_OPTIONAL_SPEC,
+ FAC_COMPARE_AND_SWAP_AND_STORE,
+ FAC_FLOATING_POINT_SUPPPORT_ENH,
+ FAC_DFP,
+};
+#define clear_2094_GA2 EmptyFacs
+
+/* 2094-GA3 */
+static uint16_t set_2094_GA3[] = {
+ FAC_PFPO,
+};
+#define clear_2094_GA3 EmptyFacs
+
+/* 2096-GA1 */
+#define set_2096_GA1 EmptyFacs
+#define clear_2096_GA1 EmptyFacs
+
+/* 2096-GA2 */
+#define set_2096_GA2 EmptyFacs
+#define clear_2096_GA2 EmptyFacs
+
+/*******************************
+ * CMOS G10 processors
+ *******************************/
+
+/* 2097-GA1 */
+static uint16_t set_2097_GA1[] = {
+ FAC_ENHANCED_DAT_1,
+ FAC_CONDITIONAL_SSKE,
+ FAC_CONFIGURATION_TOPOLOGY,
+ FAC_PARSING_ENH,
+ FAC_COMPARE_AND_SWAP_AND_STORE_2,
+ FAC_GENERAL_INSTRUCTIONS_EXT,
+ FAC_EXECUTE_EXT,
+ FAC_DFP_FAST,
+};
+#define clear_2097_GA1 EmptyFacs
+
+/* 2097-GA2 */
+static uint16_t set_2097_GA2[] = {
+ 65,
+ FAC_CPU_MEASUREMENT_COUNTER,
+ FAC_CPU_MEASUREMENT_SAMPLING,
+};
+#define clear_2097_GA2 EmptyFacs
+
+/* 2097-GA3 */
+static uint16_t set_2097_GA3[] = {
+ FAC_LOAD_PROGRAM_PARAMETERS,
+};
+#define clear_2097_GA3 EmptyFacs
+
+/* 2098-GA1 */
+#define set_2098_GA1 EmptyFacs
+#define clear_2098_GA1 EmptyFacs
+
+/* 2098-GA2 */
+#define set_2098_GA2 EmptyFacs
+#define clear_2098_GA2 EmptyFacs
+
+/*******************************
+ * CMOS G11 processors
+ *******************************/
+
+/* 2817-GA1 */
+static uint16_t set_2817_GA1[] = {
+ FAC_ENHANCED_MONITOR,
+ FAC_FLOATING_POINT_EXT,
+ FAC_MULTI_45,
+ 46,
+ FAC_ACCESS_EXCEPTION_FS_INDICATION,
+};
+static uint16_t clear_2817_GA1[] = {
+ 65,
+};
+
+/* 2817-GA2 */
+static uint16_t set_2817_GA2[] = {
+ FAC_IPTE_RANGE,
+ FAC_NONQ_KEY_SETTING,
+ FAC_CMPSC_ENH,
+ FAC_RESET_REFERENCE_BITS_MULTIPLE,
+ FAC_MESSAGE_SECURITY_ASSIST_3,
+ FAC_MESSAGE_SECURITY_ASSIST_4,
+ FAC_ZPCI,
+ FAC_ADAPTER_EVENT_NOTIFICATION,
+ FAC_ADAPTER_INT_SUPPRESSION,
+};
+#define clear_2817_GA2 EmptyFacs
+
+/* 2818-GA1 */
+#define set_2818_GA1 EmptyFacs
+#define clear_2818_GA1 EmptyFacs
+
+/*******************************
+ * CMOS G12 processors
+ *******************************/
+
+/* 2827-GA1 */
+static uint16_t set_2827_GA1[] = {
+ FAC_DFP_ZONED_CONVERSION,
+ FAC_MULTI_49,
+ FAC_CONSTRAINT_TRANSACTIONAL_EXE,
+ FAC_LOCAL_TLB_CLEARING,
+ FAC_INTERLOCKED_ACCESS_2,
+ FAC_TRANSACTIONAL_EXE,
+ FAC_ENHANCED_DAT_2,
+};
+#define clear_2827_GA1 EmptyFacs
+
+/* 2827-GA2 */
+static uint16_t set_2827_GA2[] = {
+ FAC_MESSAGE_SECURITY_ASSIST_5,
+};
+#define clear_2827_GA2 EmptyFacs
+
+/* 2828-GA1 */
+#define set_2828_GA1 EmptyFacs
+#define clear_2828_GA1 EmptyFacs
+
+/*******************************
+ * CMOS G13 processors
+ *******************************/
+
+/* 2964-GA1 */
+static uint16_t set_2964_GA1[] = {
+ FAC_LOAD_STORE_ON_COND_2,
+ FAC_DFP_PACKED_CONVERSION,
+ FAC_VECTOR,
+ FAC_STORE_CPU_COUNTER_MULTI,
+};
+#define clear_2964_GA1 EmptyFacs
+
+/****** END FACILITY DEFS ******/
+
+#define S390_ARCH_FAC_LIST_SIZE_BYTE \
+ (S390_ARCH_FAC_LIST_SIZE / 8)
+
+#define _YEARS "2014, 2015"
+#define _NAME_H "TARGET_S390X_GEN_FACILITIES_H"
+
+#define ARRAY_SIZE(a) (sizeof(a) / sizeof(a[0]))
+
+#define FAC_INITIALIZER(_name) \
+ { \
+ .Name = "FAC_LIST_CPU_S390_" #_name, \
+ .SetBits = \
+ { .Data = set_##_name, \
+ .Length = ARRAY_SIZE(set_##_name) }, \
+ .ClearBits = \
+ { .Data = clear_##_name, \
+ .Length = ARRAY_SIZE(clear_##_name) }, \
+ }
+
+enum BitOps {
+ SetBit,
+ ClearBit,
+};
+
+typedef struct BitSpec {
+ uint16_t *Data;
+ uint32_t Length;
+} BitSpec;
+
+typedef struct FacDefSpec {
+ const char *Name;
+ BitSpec SetBits;
+ BitSpec ClearBits;
+} FacDefSpec;
+
+static uint16_t EmptyFacs[] = {};
+
+/*******************************
+ * processor GA series
+ *******************************/
+static FacDefSpec FacDef[] = {
+ FAC_INITIALIZER(2064_GA1),
+ FAC_INITIALIZER(2064_GA2),
+ FAC_INITIALIZER(2064_GA3),
+ FAC_INITIALIZER(2066_GA1),
+ FAC_INITIALIZER(2084_GA1),
+ FAC_INITIALIZER(2084_GA2),
+ FAC_INITIALIZER(2084_GA3),
+ FAC_INITIALIZER(2086_GA1),
+ FAC_INITIALIZER(2084_GA4),
+ FAC_INITIALIZER(2086_GA2),
+ FAC_INITIALIZER(2084_GA5),
+ FAC_INITIALIZER(2086_GA3),
+ FAC_INITIALIZER(2094_GA1),
+ FAC_INITIALIZER(2094_GA2),
+ FAC_INITIALIZER(2094_GA3),
+ FAC_INITIALIZER(2096_GA1),
+ FAC_INITIALIZER(2096_GA2),
+ FAC_INITIALIZER(2097_GA1),
+ FAC_INITIALIZER(2097_GA2),
+ FAC_INITIALIZER(2098_GA1),
+ FAC_INITIALIZER(2097_GA3),
+ FAC_INITIALIZER(2098_GA2),
+ FAC_INITIALIZER(2817_GA1),
+ FAC_INITIALIZER(2817_GA2),
+ FAC_INITIALIZER(2818_GA1),
+ FAC_INITIALIZER(2827_GA1),
+ FAC_INITIALIZER(2827_GA2),
+ FAC_INITIALIZER(2828_GA1),
+ FAC_INITIALIZER(2964_GA1),
+};
+
+/*******************************
+ * QEMU facility list mask
+ *******************************/
+static uint16_t set_MASK_QEMU[] = {
+ FAC_ZPCI,
+ FAC_ADAPTER_EVENT_NOTIFICATION,
+ FAC_ADAPTER_INT_SUPPRESSION,
+};
+#define clear_MASK_QEMU EmptyFacs
+
+static FacDefSpec QemuMaskDef = FAC_INITIALIZER(MASK_QEMU);
+
+static void BigEndianBitOps(enum BitOps BitOp, uint64_t Facility[],
+ uint32_t MaxWord, BitSpec Bits)
+{
+ uint32_t i, Bit, Word, WidthInBits;
+
+ WidthInBits = _TYPEBITS(typeof(Facility[0]));
+ for (i = 0; i < Bits.Length; i++) {
+ Bit = (WidthInBits - 1) - (Bits.Data[i] & (WidthInBits - 1));
+ Word = Bits.Data[i] / WidthInBits;
+ assert(Word < MaxWord);
+ switch (BitOp) {
+ case SetBit:
+ Facility[Word] |= __UINT64_C(1) << Bit;
+ break;
+ case ClearBit:
+ Facility[Word] &= ~(__UINT64_C(1) << Bit);
+ break;
+ }
+ }
+}
+
+static uint32_t PrintFacilityList(uint32_t Model, FacDefSpec FacDef[])
+{
+ uint32_t i, Words, Size;
+ uint64_t Facility[FAC_LIST_ARCH_S390_SIZE_UINT8];
+
+ for (Size = ARRAY_SIZE(Facility), i = 0; i < Size; i++) {
+ Facility[i] = __UINT64_C(0);
+ }
+ for (i = 0; i <= Model; i++) {
+ BigEndianBitOps(SetBit, Facility, Size, FacDef[i].SetBits);
+ BigEndianBitOps(ClearBit, Facility, Size, FacDef[i].ClearBits);
+ }
+ for (Words = 0, i = 0; i < Size; i++) {
+ if (Facility[i]) {
+ Words = i;
+ }
+ }
+ printf("#define %s\t", FacDef[Model].Name);
+ for (i = 0; i <= Words; i++) {
+ printf("0x%016"PRIx64"%c", Facility[i], i < Words ? ',' : '\n');
+ }
+ return ++Words;
+}
+
+static inline uint32_t Max(uint32_t uIntA, uint32_t uIntB)
+{
+ if (uIntA > uIntB) {
+ return uIntA;
+ }
+ return uIntB;
+}
+
+static inline void PrintAllFacilityDefs(void)
+{
+ uint32_t i, MaxWords, MaxBytes, MaxBits;
+
+ printf("\n/* CPU model facility list data */\n");
+ for (MaxWords = 0, i = 0; i < ARRAY_SIZE(FacDef); i++) {
+ MaxWords = Max(PrintFacilityList(i, FacDef), MaxWords);
+ }
+ printf("\n/* QEMU facility mask data */\n");
+ MaxWords = Max(PrintFacilityList(0, &QemuMaskDef), MaxWords);
+ MaxBytes = MaxWords * sizeof(uint64_t);
+ MaxBits = MaxBytes * CHARBITS;
+ printf("\n/* facility list/mask sizes */\n");
+ printf("#define FAC_LIST_CPU_S390_SIZE_UINT1\t%"PRIu32"\n", MaxBits);
+ printf("#define FAC_LIST_CPU_S390_SIZE_UINT8\t%"PRIu32"\n", MaxBytes);
+ printf("#define FAC_LIST_CPU_S390_SIZE_UINT64\t%"PRIu32"\n", MaxWords);
+}
+
+int main(int argc, char *argv[])
+{
+ printf("/*\n"
+ " * AUTOMATICALLY GENERATED, DO NOT MODIFY HERE, EDIT\n"
+ " * SOURCE FILE \"%s\" INSTEAD.\n"
+ " *\n"
+ " * Copyright %s IBM Corp.\n"
+ " *\n"
+ " * This work is licensed under the terms of the GNU GPL, "
+ "version 2 or (at\n * your option) any later version. See "
+ "the COPYING file in the top-level\n * directory.\n"
+ " */\n\n"
+ "#ifndef %s\n#define %s\n", __FILE__, _YEARS, _NAME_H, _NAME_H);
+ PrintAllFacilityDefs();
+ printf("\n#endif\n");
+ return 0;
+}
--
1.8.3.1
^ permalink raw reply related [flat|nested] 55+ messages in thread
* [Qemu-devel] [PATCH v6 08/17] target-s390x: Introduce S390 CPU models
2015-04-27 14:53 [Qemu-devel] [PATCH v6 00/17] s390 cpu model implementation Michael Mueller
` (6 preceding siblings ...)
2015-04-27 14:53 ` [Qemu-devel] [PATCH v6 07/17] target-s390x: Generate facility defines per S390 CPU model Michael Mueller
@ 2015-04-27 14:53 ` Michael Mueller
2015-04-27 14:53 ` [Qemu-devel] [PATCH v6 09/17] target-s390x: Define S390 CPU model specific facility lists Michael Mueller
` (8 subsequent siblings)
16 siblings, 0 replies; 55+ messages in thread
From: Michael Mueller @ 2015-04-27 14:53 UTC (permalink / raw)
To: qemu-devel
Cc: Eduardo Habkost, Gleb Natapov, Alexander Graf,
Christian Borntraeger, Daniel Hansel, Jason J. Herne,
Cornelia Huck, Paolo Bonzini, Richard Henderson, Andreas Faerber,
Michael Mueller
This patch implements the static part of the S390 CPU class definitions.
It defines S390 CPU models by means of virtual CPU ids (enum) which contain
information on the CPU generation, the machine class, the GA number and
the machine type. The CPU id is used to instantiate a CPU class per CPU
model.
Furthermore it extends S390CPUClass by model related properties.
Signed-off-by: Michael Mueller <mimu@linux.vnet.ibm.com>
Acked-by: Christian Borntraeger <borntraeger@de.ibm.com>
---
target-s390x/Makefile.objs | 1 +
target-s390x/cpu-models.c | 78 ++++++++++++++++++++++++++++++++++++++++++++++
target-s390x/cpu-models.h | 72 ++++++++++++++++++++++++++++++++++++++++++
target-s390x/cpu-qom.h | 49 +++++++++++++++++++++++++++++
target-s390x/cpu.c | 2 ++
5 files changed, 202 insertions(+)
create mode 100644 target-s390x/cpu-models.c
create mode 100644 target-s390x/cpu-models.h
diff --git a/target-s390x/Makefile.objs b/target-s390x/Makefile.objs
index 997dda4..97c4177 100644
--- a/target-s390x/Makefile.objs
+++ b/target-s390x/Makefile.objs
@@ -1,6 +1,7 @@
obj-y += translate.o helper.o cpu.o interrupt.o
obj-y += int_helper.o fpu_helper.o cc_helper.o mem_helper.o misc_helper.o
obj-y += gdbstub.o
+obj-y += cpu-models.o
obj-$(CONFIG_SOFTMMU) += machine.o ioinst.o arch_dump.o mmu_helper.o
obj-$(CONFIG_KVM) += kvm.o
diff --git a/target-s390x/cpu-models.c b/target-s390x/cpu-models.c
new file mode 100644
index 0000000..147e221
--- /dev/null
+++ b/target-s390x/cpu-models.c
@@ -0,0 +1,78 @@
+/*
+ * CPU models for s390
+ *
+ * Copyright 2014,2015 IBM Corp.
+ *
+ * Author(s): Michael Mueller <mimu@linux.vnet.ibm.com>
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or (at
+ * your option) any later version. See the COPYING file in the top-level
+ * directory.
+ */
+
+#include "qemu-common.h"
+#include "cpu-models.h"
+
+#define S390_PROC_DEF(_name, _cpu_id, _desc) \
+ static void \
+ glue(_cpu_id, _cpu_class_init) \
+ (ObjectClass *oc, void *data) \
+ { \
+ DeviceClass *dc = DEVICE_CLASS(oc); \
+ S390CPUClass *cc = S390_CPU_CLASS(oc); \
+ \
+ cc->is_migration_safe = true; \
+ cc->mach.ga = cpu_ga(_cpu_id); \
+ cc->mach.class = cpu_class(_cpu_id); \
+ cc->mach.order = cpu_order(_cpu_id); \
+ cc->proc.gen = cpu_generation(_cpu_id); \
+ cc->proc.ver = S390_DEF_VERSION; \
+ cc->proc.id = S390_DEF_ID; \
+ cc->proc.type = cpu_type(_cpu_id); \
+ cc->proc.ibc = S390_DEF_IBC; \
+ dc->desc = _desc; \
+ } \
+ static const TypeInfo \
+ glue(_cpu_id, _cpu_type_info) = { \
+ .name = _name "-" TYPE_S390_CPU, \
+ .parent = TYPE_S390_CPU, \
+ .class_init = glue(_cpu_id, _cpu_class_init), \
+ }; \
+ static void \
+ glue(_cpu_id, _cpu_register_types)(void) \
+ { \
+ type_register_static( \
+ &glue(_cpu_id, _cpu_type_info)); \
+ } \
+ type_init(glue(_cpu_id, _cpu_register_types))
+
+/* define S390 CPU model classes */
+S390_PROC_DEF("2064-ga1", CPU_S390_2064_GA1, "IBM zSeries 900 GA1")
+S390_PROC_DEF("2064-ga2", CPU_S390_2064_GA2, "IBM zSeries 900 GA2")
+S390_PROC_DEF("2064-ga3", CPU_S390_2064_GA3, "IBM zSeries 900 GA3")
+S390_PROC_DEF("2066-ga1", CPU_S390_2066_GA1, "IBM zSeries 800 GA1")
+S390_PROC_DEF("2084-ga1", CPU_S390_2084_GA1, "IBM zSeries 990 GA1")
+S390_PROC_DEF("2084-ga2", CPU_S390_2084_GA2, "IBM zSeries 990 GA2")
+S390_PROC_DEF("2084-ga3", CPU_S390_2084_GA3, "IBM zSeries 990 GA3")
+S390_PROC_DEF("2084-ga4", CPU_S390_2084_GA4, "IBM zSeries 990 GA4")
+S390_PROC_DEF("2084-ga5", CPU_S390_2084_GA5, "IBM zSeries 990 GA5")
+S390_PROC_DEF("2086-ga1", CPU_S390_2086_GA1, "IBM zSeries 890 GA1")
+S390_PROC_DEF("2086-ga2", CPU_S390_2086_GA2, "IBM zSeries 890 GA2")
+S390_PROC_DEF("2086-ga3", CPU_S390_2086_GA3, "IBM zSeries 890 GA3")
+S390_PROC_DEF("2094-ga1", CPU_S390_2094_GA1, "IBM System z9 EC GA1")
+S390_PROC_DEF("2094-ga2", CPU_S390_2094_GA2, "IBM System z9 EC GA2")
+S390_PROC_DEF("2094-ga3", CPU_S390_2094_GA3, "IBM System z9 EC GA3")
+S390_PROC_DEF("2096-ga1", CPU_S390_2096_GA1, "IBM System z9 BC GA1")
+S390_PROC_DEF("2096-ga2", CPU_S390_2096_GA2, "IBM System z9 BC GA2")
+S390_PROC_DEF("2097-ga1", CPU_S390_2097_GA1, "IBM System z10 EC GA1")
+S390_PROC_DEF("2097-ga2", CPU_S390_2097_GA2, "IBM System z10 EC GA2")
+S390_PROC_DEF("2097-ga3", CPU_S390_2097_GA3, "IBM System z10 EC GA3")
+S390_PROC_DEF("2098-ga1", CPU_S390_2098_GA1, "IBM System z10 BC GA1")
+S390_PROC_DEF("2098-ga2", CPU_S390_2098_GA2, "IBM System z10 BC GA2")
+S390_PROC_DEF("2817-ga1", CPU_S390_2817_GA1, "IBM zEnterprise 196 GA1")
+S390_PROC_DEF("2817-ga2", CPU_S390_2817_GA2, "IBM zEnterprise 196 GA2")
+S390_PROC_DEF("2818-ga1", CPU_S390_2818_GA1, "IBM zEnterprise 114 GA1")
+S390_PROC_DEF("2827-ga1", CPU_S390_2827_GA1, "IBM zEnterprise EC12 GA1")
+S390_PROC_DEF("2827-ga2", CPU_S390_2827_GA2, "IBM zEnterprise EC12 GA2")
+S390_PROC_DEF("2828-ga1", CPU_S390_2828_GA1, "IBM zEnterprise BC12 GA1")
+S390_PROC_DEF("2964-ga1", CPU_S390_2964_GA1, "IBM z13 GA1")
diff --git a/target-s390x/cpu-models.h b/target-s390x/cpu-models.h
new file mode 100644
index 0000000..b02c38b
--- /dev/null
+++ b/target-s390x/cpu-models.h
@@ -0,0 +1,72 @@
+/*
+ * CPU models for s390
+ *
+ * Copyright 2014,2015 IBM Corp.
+ *
+ * Author(s): Michael Mueller <mimu@linux.vnet.ibm.com>
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or (at
+ * your option) any later version. See the COPYING file in the top-level
+ * directory.
+ */
+
+#ifndef TARGET_S390X_CPU_MODELS_H
+#define TARGET_S390X_CPU_MODELS_H
+
+#define S390_EC 0x1
+#define S390_BC 0x2
+
+#define S390_DEF_VERSION 0xff
+#define S390_DEF_IBC 0x0
+#define S390_DEF_ID 0xdecade
+#define S390_DEF_TYPE 0x2064
+
+#define cpu_type(x) (((x) >> 0) & 0xffff)
+#define cpu_order(x) (((x) >> 16) & 0xffff)
+#define cpu_ga(x) (((x) >> 16) & 0xf)
+#define cpu_class(x) (((x) >> 20) & 0x3)
+#define cpu_generation(x) (((x) >> 24) & 0xff)
+
+/*
+ * bits 0-7 : CMOS generation
+ * bits 8-9 : reserved
+ * bits 10-11 : machine class 0=unknown 1=EC 2=BC
+ * bits 12-15 : GA
+ * bits 16-31 : machine type
+ *
+ * note: bits are named according to s390
+ * architecture specific endienness
+ */
+enum {
+ CPU_S390_2064_GA1 = 0x07112064,
+ CPU_S390_2064_GA2 = 0x07122064,
+ CPU_S390_2064_GA3 = 0x07132064,
+ CPU_S390_2066_GA1 = 0x07212066,
+ CPU_S390_2084_GA1 = 0x08112084,
+ CPU_S390_2084_GA2 = 0x08122084,
+ CPU_S390_2084_GA3 = 0x08132084,
+ CPU_S390_2084_GA4 = 0x08142084,
+ CPU_S390_2084_GA5 = 0x08152084,
+ CPU_S390_2086_GA1 = 0x08212086,
+ CPU_S390_2086_GA2 = 0x08222086,
+ CPU_S390_2086_GA3 = 0x08232086,
+ CPU_S390_2094_GA1 = 0x09112094,
+ CPU_S390_2094_GA2 = 0x09122094,
+ CPU_S390_2094_GA3 = 0x09132094,
+ CPU_S390_2096_GA1 = 0x09212096,
+ CPU_S390_2096_GA2 = 0x09222096,
+ CPU_S390_2097_GA1 = 0x0a112097,
+ CPU_S390_2097_GA2 = 0x0a122097,
+ CPU_S390_2097_GA3 = 0x0a132097,
+ CPU_S390_2098_GA1 = 0x0a212098,
+ CPU_S390_2098_GA2 = 0x0a222098,
+ CPU_S390_2817_GA1 = 0x0b112817,
+ CPU_S390_2817_GA2 = 0x0b122817,
+ CPU_S390_2818_GA1 = 0x0b212818,
+ CPU_S390_2827_GA1 = 0x0c112827,
+ CPU_S390_2827_GA2 = 0x0c122827,
+ CPU_S390_2828_GA1 = 0x0c212828,
+ CPU_S390_2964_GA1 = 0x0d112964,
+};
+
+#endif
diff --git a/target-s390x/cpu-qom.h b/target-s390x/cpu-qom.h
index 8b376df..909450d 100644
--- a/target-s390x/cpu-qom.h
+++ b/target-s390x/cpu-qom.h
@@ -22,6 +22,7 @@
#include "qom/cpu.h"
#include "cpu.h"
+#include "gen-facilities.h"
#define TYPE_S390_CPU "s390-cpu"
@@ -32,6 +33,35 @@
#define S390_CPU_GET_CLASS(obj) \
OBJECT_GET_CLASS(S390CPUClass, (obj), TYPE_S390_CPU)
+/*
+ * The accelerator mode is used to parametrize if cpu class setup
+ * is performed for the cmdline selected accelerator or for a
+ * temporarily selected acellerator.
+ */
+typedef enum S390AccelMode {
+ ACCEL_CURRENT = 0,
+ ACCEL_TEMP = 1,
+ ACCEL_MODE_MAX = 2,
+} S390AccelMode;
+
+/* machine related properties */
+typedef struct S390CPUMachineProps {
+ uint16_t class; /* machine class */
+ uint16_t ga; /* availability number of machine */
+ uint16_t order; /* order of availability */
+} S390CPUMachineProps;
+
+/* processor related properties */
+typedef struct S390CPUProcessorProps {
+ uint16_t gen; /* S390 CMOS generation */
+ uint16_t ver; /* version of processor */
+ uint32_t id; /* processor identification*/
+ uint16_t type; /* machine type */
+ uint16_t ibc; /* IBC value */
+ /* processor related facility list */
+ uint64_t fac_list[FAC_LIST_CPU_S390_SIZE_UINT64];
+} S390CPUProcessorProps;
+
/**
* S390CPUClass:
* @parent_realize: The parent class' realize handler.
@@ -52,8 +82,27 @@ typedef struct S390CPUClass {
void (*load_normal)(CPUState *cpu);
void (*cpu_reset)(CPUState *cpu);
void (*initial_cpu_reset)(CPUState *cpu);
+ bool is_migration_safe; /* model is safely migratable */
+ bool is_active[ACCEL_MODE_MAX]; /* model enabled for given host and accel */
+ bool is_host[ACCEL_MODE_MAX]; /* model markes host for given accel */
+ /* active facility list */
+ uint64_t fac_list[ACCEL_MODE_MAX][FAC_LIST_CPU_S390_SIZE_UINT64];
+ S390CPUMachineProps mach; /* machine specific properties */
+ S390CPUProcessorProps proc; /* processor specific properties */
} S390CPUClass;
+static inline bool is_cpu_class_none(S390CPUClass *cc)
+{
+ return cc->proc.type == 0;
+}
+
+static inline char *strdup_cpu_name(S390CPUClass *cc)
+{
+ return is_cpu_class_none(cc) ?
+ g_strdup("none") :
+ g_strdup_printf("%04x-ga%u", cc->proc.type, cc->mach.ga);
+}
+
/**
* S390CPU:
* @env: #CPUS390XState.
diff --git a/target-s390x/cpu.c b/target-s390x/cpu.c
index 4d43ace..8f49813 100644
--- a/target-s390x/cpu.c
+++ b/target-s390x/cpu.c
@@ -331,6 +331,8 @@ static void s390_cpu_class_init(ObjectClass *oc, void *data)
#endif
cc->gdb_num_core_regs = S390_NUM_CORE_REGS;
cc->gdb_core_xml_file = "s390x-core64.xml";
+ scc->is_active[ACCEL_CURRENT] = is_cpu_class_none(scc);
+ scc->is_active[ACCEL_TEMP] = is_cpu_class_none(scc);
}
static const TypeInfo s390_cpu_type_info = {
--
1.8.3.1
^ permalink raw reply related [flat|nested] 55+ messages in thread
* [Qemu-devel] [PATCH v6 09/17] target-s390x: Define S390 CPU model specific facility lists
2015-04-27 14:53 [Qemu-devel] [PATCH v6 00/17] s390 cpu model implementation Michael Mueller
` (7 preceding siblings ...)
2015-04-27 14:53 ` [Qemu-devel] [PATCH v6 08/17] target-s390x: Introduce S390 CPU models Michael Mueller
@ 2015-04-27 14:53 ` Michael Mueller
2015-04-27 14:53 ` [Qemu-devel] [PATCH v6 10/17] target-s390x: Add S390 CPU model alias definition routines Michael Mueller
` (7 subsequent siblings)
16 siblings, 0 replies; 55+ messages in thread
From: Michael Mueller @ 2015-04-27 14:53 UTC (permalink / raw)
To: qemu-devel
Cc: Eduardo Habkost, Gleb Natapov, Alexander Graf,
Christian Borntraeger, Daniel Hansel, Jason J. Herne,
Cornelia Huck, Paolo Bonzini, Richard Henderson, Andreas Faerber,
Michael Mueller
This patch defines S390 CPU facilities and their presence at the
different CPU model levels. Beside defining a base which facilities
have to be requested per CPU model, these sets are associated to the
defined CPU classes and used to calculate the list of supported
CPU models in context of the current hosting machine model.
The also defined qemu side facility mask allows to implement and enable
facilities in QEMU land.
Signed-off-by: Michael Mueller <mimu@linux.vnet.ibm.com>
Acked-by: Christian Borntraeger <borntraeger@de.ibm.com>
---
target-s390x/cpu-models.c | 11 +++++++++++
target-s390x/cpu-models.h | 8 ++++++++
target-s390x/cpu.c | 1 +
3 files changed, 20 insertions(+)
diff --git a/target-s390x/cpu-models.c b/target-s390x/cpu-models.c
index 147e221..7bb80db 100644
--- a/target-s390x/cpu-models.c
+++ b/target-s390x/cpu-models.c
@@ -12,6 +12,7 @@
#include "qemu-common.h"
#include "cpu-models.h"
+#include "gen-facilities.h"
#define S390_PROC_DEF(_name, _cpu_id, _desc) \
static void \
@@ -20,6 +21,10 @@
{ \
DeviceClass *dc = DEVICE_CLASS(oc); \
S390CPUClass *cc = S390_CPU_CLASS(oc); \
+ uint64_t nbits = FAC_LIST_CPU_S390_SIZE_UINT1; \
+ uint64_t fac_list[FAC_LIST_CPU_S390_SIZE_UINT64] = { \
+ glue(FAC_LIST_, _cpu_id) \
+ }; \
\
cc->is_migration_safe = true; \
cc->mach.ga = cpu_ga(_cpu_id); \
@@ -30,6 +35,7 @@
cc->proc.id = S390_DEF_ID; \
cc->proc.type = cpu_type(_cpu_id); \
cc->proc.ibc = S390_DEF_IBC; \
+ bitmap_copy(cc->proc.fac_list, fac_list, nbits); \
dc->desc = _desc; \
} \
static const TypeInfo \
@@ -46,6 +52,11 @@
} \
type_init(glue(_cpu_id, _cpu_register_types))
+/* facilities implemented by qemu */
+uint64_t qemu_s390_fac_list_mask[FAC_LIST_CPU_S390_SIZE_UINT64] = {
+ FAC_LIST_CPU_S390_MASK_QEMU
+};
+
/* define S390 CPU model classes */
S390_PROC_DEF("2064-ga1", CPU_S390_2064_GA1, "IBM zSeries 900 GA1")
S390_PROC_DEF("2064-ga2", CPU_S390_2064_GA2, "IBM zSeries 900 GA2")
diff --git a/target-s390x/cpu-models.h b/target-s390x/cpu-models.h
index b02c38b..948af10 100644
--- a/target-s390x/cpu-models.h
+++ b/target-s390x/cpu-models.h
@@ -13,6 +13,14 @@
#ifndef TARGET_S390X_CPU_MODELS_H
#define TARGET_S390X_CPU_MODELS_H
+#include "cpu-facilities.h"
+#include "gen-facilities.h"
+
+#define FAC_LIST_ARCH_S390_SIZE_UINT1 \
+ (FAC_LIST_ARCH_S390_SIZE_UINT8 * BITS_PER_BYTE)
+#define FAC_LIST_ARCH_S390_SIZE_UINT64 \
+ (FAC_LIST_ARCH_S390_SIZE_UINT8 / sizeof(uint64_t))
+
#define S390_EC 0x1
#define S390_BC 0x2
diff --git a/target-s390x/cpu.c b/target-s390x/cpu.c
index 8f49813..9232a97 100644
--- a/target-s390x/cpu.c
+++ b/target-s390x/cpu.c
@@ -29,6 +29,7 @@
#include "qemu/error-report.h"
#include "hw/hw.h"
#include "trace.h"
+#include "cpu-models.h"
#ifndef CONFIG_USER_ONLY
#include "sysemu/arch_init.h"
#endif
--
1.8.3.1
^ permalink raw reply related [flat|nested] 55+ messages in thread
* [Qemu-devel] [PATCH v6 10/17] target-s390x: Add S390 CPU model alias definition routines
2015-04-27 14:53 [Qemu-devel] [PATCH v6 00/17] s390 cpu model implementation Michael Mueller
` (8 preceding siblings ...)
2015-04-27 14:53 ` [Qemu-devel] [PATCH v6 09/17] target-s390x: Define S390 CPU model specific facility lists Michael Mueller
@ 2015-04-27 14:53 ` Michael Mueller
2015-04-27 14:53 ` [Qemu-devel] [PATCH v6 11/17] target-s390x: Add KVM VM attribute interface for S390 CPU models Michael Mueller
` (6 subsequent siblings)
16 siblings, 0 replies; 55+ messages in thread
From: Michael Mueller @ 2015-04-27 14:53 UTC (permalink / raw)
To: qemu-devel
Cc: Eduardo Habkost, Gleb Natapov, Alexander Graf,
Christian Borntraeger, Daniel Hansel, Jason J. Herne,
Cornelia Huck, Paolo Bonzini, Richard Henderson, Andreas Faerber,
Michael Mueller
This patch implements the infrastructure to dynamically add
CPU model aliases.
Signed-off-by: Michael Mueller <mimu@linux.vnet.ibm.com>
Reviewed-by: Cornelia Huck <cornelia.huck@de.ibm.com>
Acked-by: Christian Borntraeger <borntraeger@de.ibm.com>
---
target-s390x/cpu-models.c | 82 +++++++++++++++++++++++++++++++++++++++++++++++
target-s390x/cpu-models.h | 13 ++++++++
target-s390x/cpu.c | 1 +
3 files changed, 96 insertions(+)
diff --git a/target-s390x/cpu-models.c b/target-s390x/cpu-models.c
index 7bb80db..b840ebe 100644
--- a/target-s390x/cpu-models.c
+++ b/target-s390x/cpu-models.c
@@ -57,6 +57,8 @@ uint64_t qemu_s390_fac_list_mask[FAC_LIST_CPU_S390_SIZE_UINT64] = {
FAC_LIST_CPU_S390_MASK_QEMU
};
+GSList *s390_cpu_aliases;
+
/* define S390 CPU model classes */
S390_PROC_DEF("2064-ga1", CPU_S390_2064_GA1, "IBM zSeries 900 GA1")
S390_PROC_DEF("2064-ga2", CPU_S390_2064_GA2, "IBM zSeries 900 GA2")
@@ -87,3 +89,83 @@ S390_PROC_DEF("2827-ga1", CPU_S390_2827_GA1, "IBM zEnterprise EC12 GA1")
S390_PROC_DEF("2827-ga2", CPU_S390_2827_GA2, "IBM zEnterprise EC12 GA2")
S390_PROC_DEF("2828-ga1", CPU_S390_2828_GA1, "IBM zEnterprise BC12 GA1")
S390_PROC_DEF("2964-ga1", CPU_S390_2964_GA1, "IBM z13 GA1")
+
+static gint s390_cpu_compare_name(gconstpointer a, gconstpointer b)
+{
+ char *aname = strdup_cpu_name((S390CPUClass *) a);
+ gint rc;
+
+ rc = strcasecmp(aname, (char *) b);
+ free(aname);
+
+ return rc;
+}
+
+/**
+ * s390_cpu_class_by_name:
+ * @name: a cpu model or alias name
+ *
+ * The function searches for the requested cpu model name or an alias
+ * cpu model name and returns the associated object class.
+ *
+ * Returns: reference to object class on success or %NULL elsewise.
+ *
+ * Since: 2.4
+ */
+ObjectClass *s390_cpu_class_by_name(const char *name)
+{
+ GSList *list, *item;
+ ObjectClass *ret = NULL;
+ S390CPUAlias *alias;
+
+ for (item = s390_cpu_aliases; item != NULL; item = item->next) {
+ alias = (S390CPUAlias *) item->data;
+ if (strcmp(alias->name, name) == 0) {
+ return s390_cpu_class_by_name(alias->model);
+ }
+ }
+ list = object_class_get_list(TYPE_S390_CPU, false);
+ item = g_slist_find_custom(list, name, s390_cpu_compare_name);
+ if (item) {
+ ret = OBJECT_CLASS(item->data);
+ }
+ g_slist_free(list);
+ return ret;
+}
+
+/**
+ * set_s390_cpu_alias:
+ * @name: the cpu alias name
+ * @model: the cpu model name
+ *
+ * The function registers the alias @name for an existing cpu @model.
+ *
+ * Returns: %0 in case of success
+ * -%EINVAL if name or model is %NULL or both are idential
+ * or model is not a valid cpu model
+ * -%ENOMEM if internal memory allocation fails
+ *
+ * Since: 2.4
+ */
+int set_s390_cpu_alias(const char *name, const char *model)
+{
+ S390CPUAlias *alias;
+
+ if (!name || !model) {
+ return -EINVAL;
+ }
+ if (!strcmp(name, model)) {
+ return -EINVAL;
+ }
+ if (!s390_cpu_class_by_name(model)) {
+ return -EINVAL;
+ }
+ alias = g_try_malloc0(sizeof(S390CPUAlias));
+ if (!alias) {
+ return -ENOMEM;
+ }
+ alias->name = g_strdup(name);
+ alias->model = g_strdup(model);
+ s390_cpu_aliases = g_slist_append(s390_cpu_aliases, alias);
+ return 0;
+}
diff --git a/target-s390x/cpu-models.h b/target-s390x/cpu-models.h
index 948af10..35dde5a 100644
--- a/target-s390x/cpu-models.h
+++ b/target-s390x/cpu-models.h
@@ -36,6 +36,19 @@
#define cpu_generation(x) (((x) >> 24) & 0xff)
/*
+ * S390 cpu aliases will be added dynamically
+ */
+typedef struct S390CPUAlias {
+ char *name;
+ char *model;
+} S390CPUAlias;
+
+extern GSList *s390_cpu_aliases;
+
+ObjectClass *s390_cpu_class_by_name(const char *name);
+int set_s390_cpu_alias(const char *name, const char *model);
+
+/*
* bits 0-7 : CMOS generation
* bits 8-9 : reserved
* bits 10-11 : machine class 0=unknown 1=EC 2=BC
diff --git a/target-s390x/cpu.c b/target-s390x/cpu.c
index 9232a97..c081885 100644
--- a/target-s390x/cpu.c
+++ b/target-s390x/cpu.c
@@ -314,6 +314,7 @@ static void s390_cpu_class_init(ObjectClass *oc, void *data)
#endif
scc->cpu_reset = s390_cpu_reset;
scc->initial_cpu_reset = s390_cpu_initial_reset;
+ cc->class_by_name = s390_cpu_class_by_name;
cc->reset = s390_cpu_full_reset;
cc->has_work = s390_cpu_has_work;
cc->do_interrupt = s390_cpu_do_interrupt;
--
1.8.3.1
^ permalink raw reply related [flat|nested] 55+ messages in thread
* [Qemu-devel] [PATCH v6 11/17] target-s390x: Add KVM VM attribute interface for S390 CPU models
2015-04-27 14:53 [Qemu-devel] [PATCH v6 00/17] s390 cpu model implementation Michael Mueller
` (9 preceding siblings ...)
2015-04-27 14:53 ` [Qemu-devel] [PATCH v6 10/17] target-s390x: Add S390 CPU model alias definition routines Michael Mueller
@ 2015-04-27 14:53 ` Michael Mueller
2015-04-27 14:53 ` [Qemu-devel] [PATCH v6 12/17] target-s390x: Add S390 CPU class initialization routines Michael Mueller
` (5 subsequent siblings)
16 siblings, 0 replies; 55+ messages in thread
From: Michael Mueller @ 2015-04-27 14:53 UTC (permalink / raw)
To: qemu-devel
Cc: Eduardo Habkost, Gleb Natapov, Alexander Graf,
Christian Borntraeger, Daniel Hansel, Jason J. Herne,
Cornelia Huck, Paolo Bonzini, Richard Henderson, Andreas Faerber,
Michael Mueller
The patch implements routines to set and retrieve processor configuration
data and to retrieve machine configuration data. The machine related data
is used together with the CPU model facility lists to determine the list of
supported CPU models of this host. The above mentioned routines have QEMU
trace point instrumentation.
Signed-off-by: Michael Mueller <mimu@linux.vnet.ibm.com>
Acked-by: Christian Borntraeger <borntraeger@de.ibm.com>
---
target-s390x/cpu-models.h | 36 ++++++++++++++++++++-
target-s390x/kvm.c | 79 +++++++++++++++++++++++++++++++++++++++++++++++
trace-events | 3 ++
3 files changed, 117 insertions(+), 1 deletion(-)
diff --git a/target-s390x/cpu-models.h b/target-s390x/cpu-models.h
index 35dde5a..efa255e 100644
--- a/target-s390x/cpu-models.h
+++ b/target-s390x/cpu-models.h
@@ -43,11 +43,45 @@ typedef struct S390CPUAlias {
char *model;
} S390CPUAlias;
-extern GSList *s390_cpu_aliases;
+typedef struct S390ProcessorProps {
+ uint64_t cpuid;
+ uint16_t ibc;
+ uint8_t pad[6];
+ uint64_t fac_list[FAC_LIST_ARCH_S390_SIZE_UINT64];
+} S390ProcessorProps;
+
+typedef struct S390MachineProps {
+ uint64_t cpuid;
+ uint32_t ibc;
+ uint8_t pad[4];
+ uint64_t fac_mask[FAC_LIST_ARCH_S390_SIZE_UINT64];
+ uint64_t fac_list[FAC_LIST_ARCH_S390_SIZE_UINT64];
+} S390MachineProps;
ObjectClass *s390_cpu_class_by_name(const char *name);
int set_s390_cpu_alias(const char *name, const char *model);
+#ifdef CONFIG_KVM
+int kvm_s390_get_processor_props(S390ProcessorProps *prop);
+int kvm_s390_set_processor_props(S390ProcessorProps *prop);
+bool kvm_s390_cpu_classes_initialized(void);
+#else
+static inline int kvm_s390_get_processor_props(S390ProcessorProps *prop)
+{
+ return -ENOSYS;
+}
+static inline int kvm_s390_set_processor_props(S390ProcessorProps *prop)
+{
+ return -ENOSYS;
+}
+static inline bool kvm_s390_cpu_classes_initialized(void)
+{
+ return false;
+}
+#endif
+
+extern GSList *s390_cpu_aliases;
+
/*
* bits 0-7 : CMOS generation
* bits 8-9 : reserved
diff --git a/target-s390x/kvm.c b/target-s390x/kvm.c
index b48c643..ffa9efa 100644
--- a/target-s390x/kvm.c
+++ b/target-s390x/kvm.c
@@ -44,6 +44,7 @@
#include "hw/s390x/s390-pci-inst.h"
#include "hw/s390x/s390-pci-bus.h"
#include "hw/s390x/ipl.h"
+#include "cpu-models.h"
/* #define DEBUG_KVM */
@@ -122,6 +123,7 @@ const KVMCapabilityInfo kvm_arch_required_capabilities[] = {
static int cap_sync_regs;
static int cap_async_pf;
+static bool cpu_classes_initialized;
static void *legacy_s390_alloc(size_t size, uint64_t *align);
@@ -242,6 +244,59 @@ static void kvm_s390_init_crypto(void)
kvm_s390_init_dea_kw();
}
+static int cpu_model_get(KVMState *s, uint64_t attr, void *addr)
+{
+ int rc = -ENOSYS;
+ struct kvm_device_attr dev_attr = {
+ .group = KVM_S390_VM_CPU_MODEL,
+ .attr = attr,
+ .addr = (uint64_t) addr,
+ };
+
+ if (kvm_vm_check_attr(s, dev_attr.group, dev_attr.attr)) {
+ rc = kvm_vm_ioctl(s, KVM_GET_DEVICE_ATTR, &dev_attr);
+ }
+
+ return rc;
+}
+
+static int cpu_model_set(KVMState *s, uint64_t attr, void *addr)
+{
+ int rc = -ENOSYS;
+ struct kvm_device_attr dev_attr = {
+ .group = KVM_S390_VM_CPU_MODEL,
+ .attr = attr,
+ .addr = (uint64_t) addr,
+ };
+
+ if (kvm_vm_check_attr(s, dev_attr.group, dev_attr.attr)) {
+ rc = kvm_vm_ioctl(s, KVM_SET_DEVICE_ATTR, &dev_attr);
+ }
+
+ return rc;
+}
+
+static int kvm_s390_get_machine_props(KVMState *s, S390MachineProps *prop)
+{
+ int rc = -EFAULT;
+
+ if (s) {
+ rc = cpu_model_get(s, KVM_S390_VM_CPU_MACHINE, prop);
+ }
+ trace_kvm_get_machine_props(rc, prop->cpuid, prop->ibc);
+
+ return rc;
+}
+
+static void kvm_setup_cpu_classes(KVMState *s)
+{
+ S390MachineProps mach;
+
+ if (!kvm_s390_get_machine_props(s, &mach)) {
+ cpu_classes_initialized = false;
+ }
+}
+
int kvm_arch_init(MachineState *ms, KVMState *s)
{
cap_sync_regs = kvm_check_extension(s, KVM_CAP_SYNC_REGS);
@@ -255,6 +310,7 @@ int kvm_arch_init(MachineState *ms, KVMState *s)
}
kvm_vm_enable_cap(s, KVM_CAP_S390_USER_SIGP, 0);
+ kvm_setup_cpu_classes(s);
return 0;
}
@@ -1940,3 +1996,26 @@ int kvm_arch_fixup_msi_route(struct kvm_irq_routing_entry *route,
route->u.adapter.adapter_id = pbdev->routes.adapter.adapter_id;
return 0;
}
+
+int kvm_s390_get_processor_props(S390ProcessorProps *prop)
+{
+ int rc;
+
+ rc = cpu_model_get(kvm_state, KVM_S390_VM_CPU_PROCESSOR, prop);
+ trace_kvm_get_processor_props(rc, prop->cpuid, prop->ibc);
+ return rc;
+}
+
+int kvm_s390_set_processor_props(S390ProcessorProps *prop)
+{
+ int rc;
+
+ rc = cpu_model_set(kvm_state, KVM_S390_VM_CPU_PROCESSOR, prop);
+ trace_kvm_set_processor_props(rc);
+ return rc;
+}
+
+bool kvm_s390_cpu_classes_initialized(void)
+{
+ return cpu_classes_initialized;
+}
diff --git a/trace-events b/trace-events
index 30eba92..2f1d734 100644
--- a/trace-events
+++ b/trace-events
@@ -1582,6 +1582,9 @@ kvm_enable_cmma(int rc) "CMMA: enabling with result code %d"
kvm_clear_cmma(int rc) "CMMA: clearing with result code %d"
kvm_failed_cpu_state_set(int cpu_index, uint8_t state, const char *msg) "Warning: Unable to set cpu %d state %" PRIu8 " to KVM: %s"
kvm_sigp_finished(uint8_t order, int cpu_index, int dst_index, int cc) "SIGP: Finished order %u on cpu %d -> cpu %d with cc=%d"
+kvm_get_machine_props(int rc, uint64_t cpuid, uint16_t ibc) "CPU-MODEL: fetching machine properties rc=%d cpuid=0x%"PRIx64" ibc=0x%"PRIx16
+kvm_get_processor_props(int rc, uint64_t cpuid, uint32_t ibc_range) "CPU-MODEL: fetching processor properties rc=%d cpuid=0x%"PRIx64" ibc_range=0x%"PRIx32
+kvm_set_processor_props(int rc) "CPU-MODEL: requesting processor properties rc=%d"
# hw/dma/i8257.c
i8257_unregistered_dma(int nchan, int dma_pos, int dma_len) "unregistered DMA channel used nchan=%d dma_pos=%d dma_len=%d"
--
1.8.3.1
^ permalink raw reply related [flat|nested] 55+ messages in thread
* [Qemu-devel] [PATCH v6 12/17] target-s390x: Add S390 CPU class initialization routines
2015-04-27 14:53 [Qemu-devel] [PATCH v6 00/17] s390 cpu model implementation Michael Mueller
` (10 preceding siblings ...)
2015-04-27 14:53 ` [Qemu-devel] [PATCH v6 11/17] target-s390x: Add KVM VM attribute interface for S390 CPU models Michael Mueller
@ 2015-04-27 14:53 ` Michael Mueller
2015-05-05 14:34 ` Eduardo Habkost
2015-04-27 14:53 ` [Qemu-devel] [PATCH v6 13/17] target-s390x: Prepare accelerator during S390 CPU object realization Michael Mueller
` (4 subsequent siblings)
16 siblings, 1 reply; 55+ messages in thread
From: Michael Mueller @ 2015-04-27 14:53 UTC (permalink / raw)
To: qemu-devel
Cc: Eduardo Habkost, Gleb Natapov, Alexander Graf,
Christian Borntraeger, Daniel Hansel, Jason J. Herne,
Cornelia Huck, Paolo Bonzini, Richard Henderson, Andreas Faerber,
Michael Mueller
This patch provides routines to dynamically update the previously defined
S390 CPU classes in the current host context. The main function performing
this process is s390_setup_cpu_classes(). It takes the current host context
and a facility list mask as parameter to setup the classes accordingly. It
basically performs the following sub-tasks:
- Update of CPU classes with accelerator specific host and QEMU properties
- Mark adequate CPU class as default CPU class to be used for CPU model 'host'
- Invalidate CPU classes not supported by this hosting machine
- Define machine type aliases to latest GA number of a processor model
- Define aliases for common CPU model names
- Set CPU model alias 'host' to default CPU class
Forthermore the patch provides the following routines:
- cpu_desc_avail(), s390 specific stub indicating that list_cpus() can run
- s390_setup_cpu_aliases(), adds cu model aliases
- s390_cpu_classes_initialized(), test if CPU classes have been initialized
- s390_fac_list_mask_by_machine(), returns facility list mask by machine
- s390_current_fac_list_mask(), returns facility list mask of current machine
Signed-off-by: Michael Mueller <mimu@linux.vnet.ibm.com>
Acked-by: Christian Borntraeger <borntraeger@de.ibm.com>
---
target-s390x/cpu-models.c | 501 ++++++++++++++++++++++++++++++++++++++++++++++
target-s390x/cpu-models.h | 29 +++
target-s390x/cpu.c | 17 +-
target-s390x/kvm.c | 5 +-
4 files changed, 550 insertions(+), 2 deletions(-)
diff --git a/target-s390x/cpu-models.c b/target-s390x/cpu-models.c
index b840ebe..3743f38 100644
--- a/target-s390x/cpu-models.c
+++ b/target-s390x/cpu-models.c
@@ -13,6 +13,11 @@
#include "qemu-common.h"
#include "cpu-models.h"
#include "gen-facilities.h"
+#include "qemu/error-report.h"
+#ifndef CONFIG_USER_ONLY
+#include "exec/cpu-common.h"
+#include "hw/boards.h"
+#endif
#define S390_PROC_DEF(_name, _cpu_id, _desc) \
static void \
@@ -90,6 +95,39 @@ S390_PROC_DEF("2827-ga2", CPU_S390_2827_GA2, "IBM zEnterprise EC12 GA2")
S390_PROC_DEF("2828-ga1", CPU_S390_2828_GA1, "IBM zEnterprise BC12 GA1")
S390_PROC_DEF("2964-ga1", CPU_S390_2964_GA1, "IBM z13 GA1")
+/* some types for calls to g_list_foreach() with parameters */
+typedef struct ParmBoolShortShort {
+ bool valid;
+ unsigned short type;
+ union {
+ unsigned short class;
+ unsigned short gen;
+ unsigned short ga;
+ };
+} ParmBoolShortShort;
+
+typedef struct ParmAddrAddrModeMask {
+ S390MachineProps *prop;
+ S390CPUClass *host_cc;
+ S390AccelMode mode;
+ uint64_t *mask;
+} ParmAddrAddrModeMask;
+
+/* compare order of two cpu classes for ascending sort */
+gint s390_cpu_class_asc_order_compare(gconstpointer a, gconstpointer b)
+{
+ S390CPUClass *cc_a = S390_CPU_CLASS((ObjectClass *) a);
+ S390CPUClass *cc_b = S390_CPU_CLASS((ObjectClass *) b);
+
+ if (cc_a->mach.order < cc_b->mach.order) {
+ return -1;
+ }
+ if (cc_a->mach.order > cc_b->mach.order) {
+ return 1;
+ }
+ return 0;
+}
+
static gint s390_cpu_compare_name(gconstpointer a, gconstpointer b)
{
char *aname = strdup_cpu_name((S390CPUClass *) a);
@@ -169,3 +207,466 @@ int set_s390_cpu_alias(const char *name, const char *model)
s390_cpu_aliases = g_slist_append(s390_cpu_aliases, alias);
return 0;
}
+
+/* return machine class for specific machine type */
+static void s390_machine_class_test_cpu_class(gpointer data, gpointer user_data)
+{
+ S390CPUClass *cc = S390_CPU_CLASS((ObjectClass *) data);
+ ParmBoolShortShort *parm = user_data;
+
+ if (parm->valid || is_cpu_class_none(cc) || parm->type != cc->proc.type) {
+ return;
+ }
+
+ parm->class = cc->mach.class;
+ parm->valid = true;
+}
+
+/* return machine class by machine type */
+static unsigned short machine_class(unsigned short type, void *user_data)
+{
+ GSList *list = object_class_get_list(TYPE_S390_CPU, false);
+ ParmBoolShortShort parm_class, *parm = user_data;
+
+ if (parm->type != type) {
+ parm->class = 0;
+ }
+ if (!parm->class) {
+ parm_class.type = type;
+ parm_class.class = 0;
+ parm_class.valid = false;
+ g_slist_foreach(list, (GFunc) s390_machine_class_test_cpu_class,
+ &parm_class);
+ g_slist_free(list);
+ if (parm_class.valid) {
+ parm->class = parm_class.class;
+ }
+ }
+ parm->type = type;
+
+ return parm->class;
+}
+
+/* return CMOS generation for specific machine type */
+static void s390_machine_class_test_cpu_gen(gpointer data, gpointer user_data)
+{
+ S390CPUClass *cc = S390_CPU_CLASS((ObjectClass *) data);
+ ParmBoolShortShort *parm = user_data;
+
+ if (parm->valid) {
+ return;
+ }
+
+ if (parm->type == cc->proc.type) {
+ parm->gen = cc->proc.gen;
+ parm->valid = true;
+ }
+}
+
+/* return CMOS generation by machine type */
+static uint16_t machine_gen(unsigned short type)
+{
+ GSList *list = object_class_get_list(TYPE_S390_CPU, false);
+ ParmBoolShortShort parm;
+
+ parm.type = type;
+ parm.gen = 0;
+ parm.valid = false;
+ g_slist_foreach(list, (GFunc) s390_machine_class_test_cpu_gen, &parm);
+ g_slist_free(list);
+
+ return parm.gen;
+}
+
+/* mark cpu class, used in host cpu model case */
+static void s390_mark_host_cpu_class(gpointer data, gpointer user_data)
+{
+ S390CPUClass *cc = S390_CPU_CLASS((ObjectClass *) data);
+ ParmAddrAddrModeMask *parm = user_data;
+ ParmBoolShortShort parm_tc;
+
+ if (!cc->is_active[parm->mode]) {
+ return;
+ }
+
+ parm_tc.type = 0;
+ parm_tc.class = 0;
+ if (cc->mach.class != machine_class(cpuid_type(parm->prop->cpuid),
+ &parm_tc)) {
+ /* sort out machines that differ from host machine class */
+ return;
+ }
+ if (!parm->host_cc) {
+ /* use first matching machine type */
+ cc->is_host[parm->mode] = true;
+ parm->host_cc = cc;
+ return;
+ }
+ if (cc->proc.gen > machine_gen(cpuid_type(parm->prop->cpuid))) {
+ /* sort out CMOS generations later than hosts generation */
+ cc->is_active[parm->mode] = false;
+ return;
+ }
+ if (cc->mach.order > parm->host_cc->mach.order) {
+ /* select later machine as host */
+ parm->host_cc->is_host[parm->mode] = false;
+ cc->is_host[parm->mode] = true;
+ parm->host_cc = cc;
+ }
+}
+
+/* update a specific cpu model class with host retrieved configuration */
+static void s390_update_cpu_class(gpointer data, gpointer user_data)
+{
+ ObjectClass *oc = data;
+ ParmAddrAddrModeMask *parm = user_data;
+ S390CPUClass *cc = S390_CPU_CLASS(oc);
+ uint64_t nbits = FAC_LIST_CPU_S390_SIZE_UINT1;
+ static uint64_t fac_list[FAC_LIST_CPU_S390_SIZE_UINT64];
+
+ cc->is_host[parm->mode] = false;
+
+ if (is_cpu_class_none(cc)) {
+ return;
+ }
+
+ /* Set processor identifier */
+ cc->proc.id = cpuid_id(parm->prop->cpuid);
+
+ /*
+ * Define model specific IBC value in current host context.
+ * IBC was introduced with CMOS version 10 i.e. type 2097.
+ * For older CPUs it is assumed to be 0x000. The BC system
+ * has always the same IBC version as the previous EC system.
+ * If the host supports IBC but not the requested type, it
+ * will be set to the oldest supported value.
+ */
+ if (has_ibc(parm->prop->ibc)) {
+ if (cc->proc.gen >= S390_CMOS_G10) {
+ cc->proc.ibc = ((cc->proc.gen - S390_CMOS_G10) << 4);
+ cc->proc.ibc += cc->mach.ga;
+ if (cc->mach.class == S390_BC) {
+ cc->proc.ibc++;
+ }
+ if (cc->proc.ibc < oldest_ibc(parm->prop->ibc)) {
+ cc->proc.ibc = oldest_ibc(parm->prop->ibc);
+ }
+ if (cc->proc.ibc > newest_ibc(parm->prop->ibc)) {
+ cc->proc.ibc = newest_ibc(parm->prop->ibc);
+ }
+ } else {
+ cc->proc.ibc = oldest_ibc(parm->prop->ibc);
+ }
+ }
+
+ /*
+ * Processor generation and GA level specific facility properties:
+ *
+ * - cc->fac_list (RFL):
+ * resulting facility list to be requested for guest cpus
+ * - cc->proc.fac_list (PFL):
+ * facility list defined per processor generation and GA level
+ *
+ * Machine specific facility properties reported by the host:
+ *
+ * - parm->prop->fac_list (MFL):
+ * host specifc facility list, might be reduced by some facilities
+ * in case the host is backed by z/VM and not a LPAR
+ * - parm->prop->fac_list_mask (MFM):
+ * host specific facility list mask containing facilities
+ *
+ * QEMU defined properties:
+ *
+ * - qemu_s390_fac_list_mask (QFM):
+ * locally defined facilities, they are added to the set of
+ * facilities requested for a guest vcpu. They are visible in
+ * the guest and require qemu side instruction handling
+ *
+ * The calculation for the vcpu specific facility list (RFL) from the
+ * above defined lists/masks works as follows:
+ *
+ * RFL = PFL & (QFM | MFM)
+ *
+ * Set resulting/desired facilities of given cpu class
+ */
+ if (parm->mask) {
+ bitmap_or(cc->fac_list[parm->mode], parm->mask,
+ parm->prop->fac_mask, nbits);
+ bitmap_and(cc->fac_list[parm->mode], cc->fac_list[parm->mode],
+ cc->proc.fac_list, nbits);
+ } else {
+ bitmap_and(cc->fac_list[parm->mode], parm->prop->fac_mask,
+ cc->proc.fac_list, nbits);
+ }
+
+ /*
+ * Finally, mark the cpu class active if all resulting/desired
+ * facilities are offered by the host.
+ * (RFL & MFL) != RFL
+ */
+ bitmap_and(fac_list, cc->fac_list[parm->mode], parm->prop->fac_list, nbits);
+ if (bitmap_equal(fac_list, cc->fac_list[parm->mode], nbits)) {
+ cc->is_active[parm->mode] = true;
+ }
+}
+
+/* cpu models newer than the hosting machine are not supported */
+static void s390_disable_not_supported_cpu_class(gpointer data,
+ gpointer user_data)
+{
+ S390CPUClass *cc = S390_CPU_CLASS((ObjectClass *) data);
+ ParmAddrAddrModeMask *parm = user_data;
+
+ if (!cc->is_active[parm->mode]) {
+ return;
+ }
+ if (parm->host_cc) {
+ if (cc->proc.gen < parm->host_cc->proc.gen) {
+ return;
+ }
+ if (cc->proc.gen == parm->host_cc->proc.gen) {
+ if (cc->mach.class == parm->host_cc->mach.class &&
+ cc->mach.ga <= parm->host_cc->mach.ga) {
+ return;
+ }
+ if (cc->mach.class == S390_EC &&
+ cc->mach.ga > parm->host_cc->mach.ga) {
+ return;
+ }
+ if (cc->mach.class == S390_BC &&
+ cc->mach.ga < parm->host_cc->mach.ga) {
+ return;
+ }
+ }
+ }
+ cc->is_active[parm->mode] = false;
+}
+
+static void set_s390_cpu_alias_by_type_ga(unsigned short type,
+ unsigned short ga)
+{
+ set_s390_cpu_alias(g_strdup_printf("%04x", type),
+ g_strdup_printf("%04x-ga%u", type, ga));
+}
+
+/* set cpu model type alias to newest ga release */
+static void s390_set_ga_alias(gpointer data, gpointer user_data)
+{
+ S390CPUClass *cc = S390_CPU_CLASS((ObjectClass *) data);
+ ParmBoolShortShort *parm = user_data;
+
+ if (!cc->is_active[ACCEL_CURRENT]) {
+ return;
+ }
+ if (!parm->type) {
+ parm->type = cc->proc.type;
+ }
+ if (cc->proc.type == parm->type) {
+ parm->ga = cc->mach.ga;
+ return;
+ }
+ set_s390_cpu_alias_by_type_ga(parm->type, parm->ga);
+ parm->type = cc->proc.type;
+ parm->ga = cc->mach.ga;
+}
+
+/* set cpu model alias host to cpu class marked is host cpu class */
+static void s390_set_host_alias(gpointer data, gpointer user_data)
+{
+ S390CPUClass *cc = S390_CPU_CLASS((ObjectClass *) data);
+
+ if (cc->is_active[ACCEL_CURRENT] && cc->is_host[ACCEL_CURRENT]) {
+ set_s390_cpu_alias("host", g_strdup_printf("%04x-ga%u",
+ cc->proc.type,
+ cc->mach.ga));
+ }
+}
+
+/**
+ * s390_setup_cpu_classes:
+ * @mode: the accelerator mode
+ * @prop: the machine property structure's address
+ *
+ * This function validates the defined cpu classes against the given
+ * machine properties @prop. Only cpu classes that are runnable on the
+ * current host will be set active. In addition the corresponding
+ * cpuid, ibc value and the active set of facilities will be
+ * initialized. Depending on @mode, the function porforms operations
+ * on the current or the temporary accelerator properies.
+ *
+ * Since: 2.4
+ */
+void s390_setup_cpu_classes(S390AccelMode mode, S390MachineProps *prop,
+ uint64_t *fac_list_mask)
+{
+ GSList *list;
+ ParmAddrAddrModeMask parm = {
+ .mode = mode,
+ .prop = prop,
+ .mask = fac_list_mask,
+ .host_cc = NULL,
+ };
+
+ list = object_class_get_list(TYPE_S390_CPU, false);
+ list = g_slist_sort(list, s390_cpu_class_asc_order_compare);
+
+ g_slist_foreach(list, (GFunc) s390_update_cpu_class, (gpointer) &parm);
+ g_slist_foreach(list, (GFunc) s390_mark_host_cpu_class, (gpointer) &parm);
+ g_slist_foreach(list, (GFunc) s390_disable_not_supported_cpu_class, &parm);
+
+ g_slist_free(list);
+}
+
+/**
+ * s390_setup_cpu_aliases:
+ *
+ * This function addes cpu model aliases that will allow to specify common
+ * model names in cunjunction with the -cpu command line parameter.
+ * There will be aliases for cpu types, pointing to the respective newest
+ * ga of a cpu type, aliases like z-something which are widely known and
+ * a the alias host pointing to the cpu type representing the current hosting
+ * mahine.
+ *
+ * Since: 2.4
+ */
+void s390_setup_cpu_aliases(void)
+{
+ GSList *list;
+ ParmBoolShortShort parm = { .type = 0, { .ga = 0 } };
+
+ list = object_class_get_list(TYPE_S390_CPU, false);
+ list = g_slist_sort(list, s390_cpu_class_asc_order_compare);
+
+ g_slist_foreach(list, (GFunc) s390_set_ga_alias, &parm);
+ set_s390_cpu_alias_by_type_ga(parm.type, parm.ga);
+
+ set_s390_cpu_alias("z900", "2064");
+ set_s390_cpu_alias("z800", "2066");
+ set_s390_cpu_alias("z990", "2084");
+ set_s390_cpu_alias("z890", "2086");
+ set_s390_cpu_alias("z9-109", "2094-ga1");
+ set_s390_cpu_alias("z9", "2094");
+ set_s390_cpu_alias("z9-ec", "2094");
+ set_s390_cpu_alias("z9-bc", "2096");
+ set_s390_cpu_alias("z10", "2097");
+ set_s390_cpu_alias("z10-ec", "2097");
+ set_s390_cpu_alias("z10-bc", "2098");
+ set_s390_cpu_alias("z196", "2817");
+ set_s390_cpu_alias("z114", "2818");
+ set_s390_cpu_alias("zEC12", "2827");
+ set_s390_cpu_alias("zBC12", "2828");
+ set_s390_cpu_alias("z13", "2964");
+
+ g_slist_foreach(list, (GFunc) s390_set_host_alias, &parm);
+
+ g_slist_free(list);
+}
+
+/* list all supported cpu models and alias names */
+void s390_cpu_list_entry(gpointer data, gpointer user_data)
+{
+ ObjectClass *alias_oc, *oc = data;
+ CPUListState *s = user_data;
+ DeviceClass *dc = DEVICE_CLASS(oc);
+ S390CPUClass *cc = S390_CPU_CLASS(oc);
+ S390CPUAlias *alias;
+ GSList *item;
+ char *name;
+
+ if (!kvm_enabled()) {
+ return;
+ }
+ if (!cc->is_active[ACCEL_CURRENT]) {
+ return;
+ }
+ name = strdup_cpu_name(cc);
+ (*s->cpu_fprintf)(s->file, "s390 %-10s %s\n",
+ name, dc->desc ? dc->desc : "");
+
+ for (item = s390_cpu_aliases; item != NULL; item = item->next) {
+ alias = (S390CPUAlias *) item->data;
+ alias_oc = s390_cpu_class_by_name(alias->model);
+ if (alias_oc != oc) {
+ continue;
+ }
+ (*s->cpu_fprintf)(s->file, "s390 %-10s (alias for %s)\n",
+ alias->name, name);
+ }
+
+ g_free(name);
+}
+
+/**
+ * s390_cpu_classes_initialized:
+ *
+ * This function indicates if the all cpu classes and their properties
+ * have been initialized.
+ *
+ * Returns: a boolean value.
+ *
+ * Since: 2.4
+ */
+bool s390_cpu_classes_initialized(void)
+{
+ if (kvm_enabled()) {
+ return kvm_s390_cpu_classes_initialized();
+ }
+ return false;
+}
+
+bool cpu_desc_avail(void)
+{
+ return s390_cpu_classes_initialized();
+}
+
+/**
+ * s390_fac_list_mask_by_machine:
+ * @name: machine name
+ *
+ * This function returns the address of a facility list mask to
+ * be used in cunjunction with the specified machine type name
+ * or alias.
+ *
+ * Returns: The address of the facility list mask or %NULL in case
+ * @name is not a valid machine type name or alias
+ *
+ * Since: 2.4
+ */
+#ifndef CONFIG_USER_ONLY
+uint64_t *s390_fac_list_mask_by_machine(const char *name)
+{
+ uint64_t *mask = NULL;
+ GSList *machine, *mlist = object_class_get_list(TYPE_MACHINE, false);
+ MachineClass *mc;
+
+ for (machine = mlist; machine; machine = machine->next) {
+ mc = machine->data;
+ if (!strcmp(mc->name, name) ||
+ (mc->alias && !strcmp(mc->alias, name))) {
+ /* add cases as required */
+ mask = qemu_s390_fac_list_mask;
+ break;
+ }
+ }
+ return mask;
+}
+#endif
+
+/**
+ * s390_current_fac_list_mask:
+ *
+ * This function returns the address of a facility list mask of the
+ * currently active machine.
+ *
+ * Returns: The address of the facility list mask.
+ *
+ * Since: 2.4
+ */
+#ifndef CONFIG_USER_ONLY
+uint64_t *s390_current_fac_list_mask(void)
+{
+ MachineClass *mc = MACHINE_GET_CLASS(MACHINE(qdev_get_machine()));
+
+ return s390_fac_list_mask_by_machine(mc->name);
+}
+#endif
diff --git a/target-s390x/cpu-models.h b/target-s390x/cpu-models.h
index efa255e..00454d3 100644
--- a/target-s390x/cpu-models.h
+++ b/target-s390x/cpu-models.h
@@ -29,12 +29,32 @@
#define S390_DEF_ID 0xdecade
#define S390_DEF_TYPE 0x2064
+/* first s390 CMOS generation supporting IBC */
+#define S390_CMOS_G10 0xa
+
#define cpu_type(x) (((x) >> 0) & 0xffff)
#define cpu_order(x) (((x) >> 16) & 0xffff)
#define cpu_ga(x) (((x) >> 16) & 0xf)
#define cpu_class(x) (((x) >> 20) & 0x3)
#define cpu_generation(x) (((x) >> 24) & 0xff)
+#define cpuid_type(x) (((x) >> 16) & 0xffff)
+#define cpuid_id(x) (((x) >> 32) & 0xffffff)
+#define cpuid_ver(x) (((x) >> 56) & 0xff)
+
+#define type_cpuid(x) ((uint64_t)((x) & 0xffff) << 16)
+#define id_cpuid(x) ((uint64_t)((x) & 0xffffff) << 32)
+#define ver_cpuid(x) ((uint64_t)((x) & 0xff) << 56)
+
+#define oldest_ibc(x) (((uint32_t)(x) >> 16) & 0xfff)
+#define newest_ibc(x) ((uint32_t)(x) & 0xfff)
+#define has_ibc(x) (oldest_ibc(x) != 0)
+
+#define S390_DEF_CPUID \
+ (ver_cpuid(S390_DEF_VERSION) | \
+ id_cpuid(S390_DEF_ID) | \
+ type_cpuid(S390_DEF_TYPE))
+
/*
* S390 cpu aliases will be added dynamically
*/
@@ -60,6 +80,14 @@ typedef struct S390MachineProps {
ObjectClass *s390_cpu_class_by_name(const char *name);
int set_s390_cpu_alias(const char *name, const char *model);
+void s390_setup_cpu_classes(S390AccelMode mode, S390MachineProps *prop,
+ uint64_t *fac_list_mask);
+void s390_setup_cpu_aliases(void);
+gint s390_cpu_class_asc_order_compare(gconstpointer a, gconstpointer b);
+void s390_cpu_list_entry(gpointer data, gpointer user_data);
+bool s390_cpu_classes_initialized(void);
+uint64_t *s390_fac_list_mask_by_machine(const char *name);
+uint64_t *s390_current_fac_list_mask(void);
#ifdef CONFIG_KVM
int kvm_s390_get_processor_props(S390ProcessorProps *prop);
@@ -81,6 +109,7 @@ static inline bool kvm_s390_cpu_classes_initialized(void)
#endif
extern GSList *s390_cpu_aliases;
+extern uint64_t qemu_s390_fac_list_mask[];
/*
* bits 0-7 : CMOS generation
diff --git a/target-s390x/cpu.c b/target-s390x/cpu.c
index c081885..cbab627 100644
--- a/target-s390x/cpu.c
+++ b/target-s390x/cpu.c
@@ -41,7 +41,22 @@
void s390_cpu_list(FILE *f, fprintf_function cpu_fprintf)
{
#ifdef CONFIG_KVM
- (*cpu_fprintf)(f, "s390 %16s\n", "host");
+ CPUListState s = {
+ .file = f,
+ .cpu_fprintf = cpu_fprintf,
+ };
+ GSList *list;
+
+ if (kvm_enabled() && s390_cpu_classes_initialized()) {
+ list = object_class_get_list(TYPE_S390_CPU, false);
+ list = g_slist_sort(list, s390_cpu_class_asc_order_compare);
+ g_slist_foreach(list, s390_cpu_list_entry, &s);
+ g_slist_free(list);
+ } else {
+#endif
+ (*cpu_fprintf)(f, "s390 host\n");
+#ifdef CONFIG_KVM
+ }
#endif
}
diff --git a/target-s390x/kvm.c b/target-s390x/kvm.c
index ffa9efa..4d75ff0 100644
--- a/target-s390x/kvm.c
+++ b/target-s390x/kvm.c
@@ -293,7 +293,10 @@ static void kvm_setup_cpu_classes(KVMState *s)
S390MachineProps mach;
if (!kvm_s390_get_machine_props(s, &mach)) {
- cpu_classes_initialized = false;
+ s390_setup_cpu_classes(ACCEL_CURRENT, &mach,
+ s390_current_fac_list_mask());
+ s390_setup_cpu_aliases();
+ cpu_classes_initialized = true;
}
}
--
1.8.3.1
^ permalink raw reply related [flat|nested] 55+ messages in thread
* [Qemu-devel] [PATCH v6 13/17] target-s390x: Prepare accelerator during S390 CPU object realization
2015-04-27 14:53 [Qemu-devel] [PATCH v6 00/17] s390 cpu model implementation Michael Mueller
` (11 preceding siblings ...)
2015-04-27 14:53 ` [Qemu-devel] [PATCH v6 12/17] target-s390x: Add S390 CPU class initialization routines Michael Mueller
@ 2015-04-27 14:53 ` Michael Mueller
2015-04-27 14:53 ` [Qemu-devel] [PATCH v6 14/17] target-s390x: Initialize S390 CPU model name in CPUState Michael Mueller
` (3 subsequent siblings)
16 siblings, 0 replies; 55+ messages in thread
From: Michael Mueller @ 2015-04-27 14:53 UTC (permalink / raw)
To: qemu-devel
Cc: Eduardo Habkost, Gleb Natapov, Alexander Graf,
Christian Borntraeger, Daniel Hansel, Jason J. Herne,
Cornelia Huck, Paolo Bonzini, Richard Henderson, Andreas Faerber,
Michael Mueller
This patch implements routine s390_cpu_model_init(). It is called by the
realize function during instantiation of an CPU object. Its task is to
initialize the current accelerator with the properties of the selected
processor model.
Signed-off-by: Michael Mueller <mimu@linux.vnet.ibm.com>
Acked-by: Christian Borntraeger <borntraeger@de.ibm.com>
---
target-s390x/cpu-models.c | 30 ++++++++++++++++++++++++++++++
target-s390x/cpu-models.h | 4 ++++
target-s390x/cpu.c | 1 +
3 files changed, 35 insertions(+)
diff --git a/target-s390x/cpu-models.c b/target-s390x/cpu-models.c
index 3743f38..dfdfb7b 100644
--- a/target-s390x/cpu-models.c
+++ b/target-s390x/cpu-models.c
@@ -670,3 +670,33 @@ uint64_t *s390_current_fac_list_mask(void)
return s390_fac_list_mask_by_machine(mc->name);
}
#endif
+
+/**
+ * s390_cpu_model_init:
+ * @cc: S390 CPU class
+ *
+ * This function intitializes the current accelerator with processor
+ * related properties.
+ *
+ * Since: 2.4
+ */
+void s390_cpu_model_init(S390CPUClass *cc)
+{
+ S390ProcessorProps proc = {
+ .cpuid = cpuid(cc->proc),
+ .ibc = cc->proc.ibc,
+ };
+
+ /* none cpu model case */
+ if (!proc.cpuid) {
+ return;
+ }
+
+ bitmap_zero(proc.fac_list, FAC_LIST_ARCH_S390_SIZE_UINT1);
+ bitmap_copy(proc.fac_list, cc->fac_list[ACCEL_CURRENT],
+ FAC_LIST_CPU_S390_SIZE_UINT1);
+
+ if (kvm_enabled()) {
+ kvm_s390_set_processor_props(&proc);
+ }
+}
diff --git a/target-s390x/cpu-models.h b/target-s390x/cpu-models.h
index 00454d3..c24550b 100644
--- a/target-s390x/cpu-models.h
+++ b/target-s390x/cpu-models.h
@@ -45,6 +45,9 @@
#define type_cpuid(x) ((uint64_t)((x) & 0xffff) << 16)
#define id_cpuid(x) ((uint64_t)((x) & 0xffffff) << 32)
#define ver_cpuid(x) ((uint64_t)((x) & 0xff) << 56)
+#define cpuid(x) (ver_cpuid(x.ver) | \
+ id_cpuid(x.id) | \
+ type_cpuid(x.type))
#define oldest_ibc(x) (((uint32_t)(x) >> 16) & 0xfff)
#define newest_ibc(x) ((uint32_t)(x) & 0xfff)
@@ -88,6 +91,7 @@ void s390_cpu_list_entry(gpointer data, gpointer user_data);
bool s390_cpu_classes_initialized(void);
uint64_t *s390_fac_list_mask_by_machine(const char *name);
uint64_t *s390_current_fac_list_mask(void);
+void s390_cpu_model_init(S390CPUClass *cc);
#ifdef CONFIG_KVM
int kvm_s390_get_processor_props(S390ProcessorProps *prop);
diff --git a/target-s390x/cpu.c b/target-s390x/cpu.c
index cbab627..65dee3e 100644
--- a/target-s390x/cpu.c
+++ b/target-s390x/cpu.c
@@ -184,6 +184,7 @@ static void s390_cpu_realizefn(DeviceState *dev, Error **errp)
CPUState *cs = CPU(dev);
S390CPUClass *scc = S390_CPU_GET_CLASS(dev);
+ s390_cpu_model_init(scc);
s390_cpu_gdb_init(cs);
qemu_init_vcpu(cs);
#if !defined(CONFIG_USER_ONLY)
--
1.8.3.1
^ permalink raw reply related [flat|nested] 55+ messages in thread
* [Qemu-devel] [PATCH v6 14/17] target-s390x: Initialize S390 CPU model name in CPUState
2015-04-27 14:53 [Qemu-devel] [PATCH v6 00/17] s390 cpu model implementation Michael Mueller
` (12 preceding siblings ...)
2015-04-27 14:53 ` [Qemu-devel] [PATCH v6 13/17] target-s390x: Prepare accelerator during S390 CPU object realization Michael Mueller
@ 2015-04-27 14:53 ` Michael Mueller
2015-04-27 14:53 ` [Qemu-devel] [PATCH v6 15/17] target-s390x: Extend arch specific QMP command query-cpu-definitions Michael Mueller
` (2 subsequent siblings)
16 siblings, 0 replies; 55+ messages in thread
From: Michael Mueller @ 2015-04-27 14:53 UTC (permalink / raw)
To: qemu-devel
Cc: Eduardo Habkost, Gleb Natapov, Alexander Graf,
Christian Borntraeger, Daniel Hansel, Jason J. Herne,
Cornelia Huck, Paolo Bonzini, Richard Henderson, Andreas Faerber,
Michael Mueller
The cpu model name now gets initialized during CPU instance
initialization.
Signed-off-by: Michael Mueller <mimu@linux.vnet.ibm.com>
Acked-by: Christian Borntraeger <borntraeger@de.ibm.com>
---
target-s390x/cpu.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/target-s390x/cpu.c b/target-s390x/cpu.c
index 65dee3e..771acbd 100644
--- a/target-s390x/cpu.c
+++ b/target-s390x/cpu.c
@@ -207,6 +207,7 @@ static void s390_cpu_initfn(Object *obj)
struct tm tm;
#endif
+ cs->model_name = strdup_cpu_name(S390_CPU_GET_CLASS(cs));
cs->env_ptr = env;
cpu_exec_init(env);
#if !defined(CONFIG_USER_ONLY)
--
1.8.3.1
^ permalink raw reply related [flat|nested] 55+ messages in thread
* [Qemu-devel] [PATCH v6 15/17] target-s390x: Extend arch specific QMP command query-cpu-definitions
2015-04-27 14:53 [Qemu-devel] [PATCH v6 00/17] s390 cpu model implementation Michael Mueller
` (13 preceding siblings ...)
2015-04-27 14:53 ` [Qemu-devel] [PATCH v6 14/17] target-s390x: Initialize S390 CPU model name in CPUState Michael Mueller
@ 2015-04-27 14:53 ` Michael Mueller
2015-05-05 18:40 ` Eduardo Habkost
2015-05-06 12:37 ` Eduardo Habkost
2015-04-27 14:53 ` [Qemu-devel] [PATCH v6 16/17] target-s390x: Introduce S390 CPU facility test routine Michael Mueller
2015-04-27 14:53 ` [Qemu-devel] [PATCH v6 17/17] target-s390x: Enable S390 CPU model usage Michael Mueller
16 siblings, 2 replies; 55+ messages in thread
From: Michael Mueller @ 2015-04-27 14:53 UTC (permalink / raw)
To: qemu-devel
Cc: Eduardo Habkost, Gleb Natapov, Alexander Graf,
Christian Borntraeger, Daniel Hansel, Jason J. Herne,
Cornelia Huck, Paolo Bonzini, Richard Henderson, Andreas Faerber,
Michael Mueller
This patch implements the QMP command 'query-cpu-definitions' in the S390
context. The command returns a list of cpu definitions in the current host
context. A runnable and migratable cpu model has the related attributes
set to true. The order attribute is used to bring the listed cpu definitions
in a release order.
request:
{"execute": "query-cpu-definitions",
"arguments": { "accel": "kvm",
"machine": "s390-ccw-virtio" }
}
answer:
{"return": [ { "order": 3345, "name": "2964-ga1",
"live-migration-safe": true },
{ "name": "zBC12", "runnable": true },
{ "name": "2828", "runnable": true },
...
{ "name": "host", "runnable": true },
...
{ "order": 3090, "name": "2827-ga2", "runnable": true,
"live-migration-safe": true, "is-default": true },
...
{ "name": "none", "runnable": true } ]
}
The request arguments are optional and if omitted lead to different answers.
Eventually only the CPU model none gets returned as runnable and as default
...
{ "name": "none", "runnable": true, "is-default": true }
The returned values for attributes like runnable depend on the machine type QEMU
is running on. The function kvm_s390_get_machine_props() is used to determine that.
If QEMU was started for accelerator KVM, a KVMstate is established and machine
properties are retrieved by cpu_model_get(). In case no KVMstate was established,
e.g. during QEMU startup in daemonized mode with the default accelerator TCG, a
fallback routine named get_machine_props_fallback() is used to retrieve the KVM
machine properties. It first creates a temporary VM, performs the required
ioctls and finally destroys the VM again.
Signed-off-by: Michael Mueller <mimu@linux.vnet.ibm.com>
Acked-by: Christian Borntraeger <borntraeger@de.ibm.com>
---
target-s390x/cpu-models.c | 15 ++++++
target-s390x/cpu-models.h | 7 +++
target-s390x/cpu.c | 126 +++++++++++++++++++++++++++++++++++++++++++---
target-s390x/kvm.c | 49 +++++++++++++++++-
4 files changed, 189 insertions(+), 8 deletions(-)
diff --git a/target-s390x/cpu-models.c b/target-s390x/cpu-models.c
index dfdfb7b..7efb3e7 100644
--- a/target-s390x/cpu-models.c
+++ b/target-s390x/cpu-models.c
@@ -208,6 +208,21 @@ int set_s390_cpu_alias(const char *name, const char *model)
return 0;
}
+/* compare order of two cpu classes for descending sort */
+gint s390_cpu_class_desc_order_compare(gconstpointer a, gconstpointer b)
+{
+ S390CPUClass *cc_a = S390_CPU_CLASS((ObjectClass *) a);
+ S390CPUClass *cc_b = S390_CPU_CLASS((ObjectClass *) b);
+
+ if (cc_a->mach.order < cc_b->mach.order) {
+ return 1;
+ }
+ if (cc_a->mach.order > cc_b->mach.order) {
+ return -1;
+ }
+ return 0;
+}
+
/* return machine class for specific machine type */
static void s390_machine_class_test_cpu_class(gpointer data, gpointer user_data)
{
diff --git a/target-s390x/cpu-models.h b/target-s390x/cpu-models.h
index c24550b..76673f4 100644
--- a/target-s390x/cpu-models.h
+++ b/target-s390x/cpu-models.h
@@ -87,6 +87,7 @@ void s390_setup_cpu_classes(S390AccelMode mode, S390MachineProps *prop,
uint64_t *fac_list_mask);
void s390_setup_cpu_aliases(void);
gint s390_cpu_class_asc_order_compare(gconstpointer a, gconstpointer b);
+gint s390_cpu_class_desc_order_compare(gconstpointer a, gconstpointer b);
void s390_cpu_list_entry(gpointer data, gpointer user_data);
bool s390_cpu_classes_initialized(void);
uint64_t *s390_fac_list_mask_by_machine(const char *name);
@@ -94,10 +95,16 @@ uint64_t *s390_current_fac_list_mask(void);
void s390_cpu_model_init(S390CPUClass *cc);
#ifdef CONFIG_KVM
+int kvm_s390_get_machine_props(KVMState *s, S390MachineProps *prop);
int kvm_s390_get_processor_props(S390ProcessorProps *prop);
int kvm_s390_set_processor_props(S390ProcessorProps *prop);
bool kvm_s390_cpu_classes_initialized(void);
#else
+static inline int kvm_s390_get_machine_props(KVMState *s,
+ S390MachineProps *prop)
+{
+ return -ENOSYS;
+}
static inline int kvm_s390_get_processor_props(S390ProcessorProps *prop)
{
return -ENOSYS;
diff --git a/target-s390x/cpu.c b/target-s390x/cpu.c
index 771acbd..a006b93 100644
--- a/target-s390x/cpu.c
+++ b/target-s390x/cpu.c
@@ -30,7 +30,10 @@
#include "hw/hw.h"
#include "trace.h"
#include "cpu-models.h"
+#include "sysemu/accel.h"
+#include "qapi/qmp/qerror.h"
#ifndef CONFIG_USER_ONLY
+#include "hw/boards.h"
#include "sysemu/arch_init.h"
#endif
@@ -61,22 +64,131 @@ void s390_cpu_list(FILE *f, fprintf_function cpu_fprintf)
}
#ifndef CONFIG_USER_ONLY
+static CpuDefinitionInfoList *qmp_query_cpu_definition_host(void)
+{
+ CpuDefinitionInfoList *host = NULL;
+ CpuDefinitionInfo *info;
+
+ info = g_try_new0(CpuDefinitionInfo, 1);
+ if (!info) {
+ goto out;
+ }
+ info->name = g_strdup("host");
+
+ host = g_try_new0(CpuDefinitionInfoList, 1);
+ if (!host) {
+ g_free(info->name);
+ g_free(info);
+ goto out;
+ }
+ host->value = info;
+out:
+ return host;
+}
+
+static void qmp_query_cpu_definition_entry(gpointer data, gpointer user_data)
+{
+ ObjectClass *oc = (ObjectClass *) data;
+ S390CPUClass *cc = S390_CPU_CLASS(oc);
+ CpuDefinitionInfoList *entry, **list = user_data;
+ CpuDefinitionInfo *info;
+ GSList *item;
+ S390CPUAlias *alias;
+ ObjectClass *alias_oc;
+
+ info = g_try_new0(CpuDefinitionInfo, 1);
+ if (!info) {
+ goto out;
+ }
+ info->name = strdup_cpu_name(cc);
+ info->runnable = cc->is_active[ACCEL_TEMP];
+ info->has_runnable = info->runnable;
+ info->is_default = cc->is_host[ACCEL_TEMP];
+ info->has_is_default = info->is_default;
+ info->live_migration_safe = cc->is_migration_safe;
+ info->has_live_migration_safe = info->live_migration_safe;
+ info->order = cc->mach.order;
+ info->has_order = info->order != 0;
+
+ entry = g_try_new0(CpuDefinitionInfoList, 1);
+ if (!entry) {
+ goto out;
+ }
+ entry->value = info;
+ entry->next = *list;
+ *list = entry;
+
+ for (item = s390_cpu_aliases; item != NULL; item = item->next) {
+ alias = (S390CPUAlias *) item->data;
+ alias_oc = s390_cpu_class_by_name(alias->model);
+ if (alias_oc != oc) {
+ continue;
+ }
+
+ info = g_try_new0(CpuDefinitionInfo, 1);
+ if (!info) {
+ goto out;
+ }
+ info->name = g_strdup(alias->name);
+ info->runnable = cc->is_active[ACCEL_TEMP];
+ info->has_runnable = info->runnable;
+
+ entry = g_try_new0(CpuDefinitionInfoList, 1);
+ if (!entry) {
+ goto out;
+ }
+ entry->value = info;
+ entry->next = *list;
+ *list = entry;
+ }
+
+ return;
+out:
+ if (info) {
+ g_free(info->name);
+ g_free(info);
+ }
+}
+
CpuDefinitionInfoList *arch_query_cpu_definitions(bool has_machine,
const char *machine,
bool has_accel,
AccelId accel,
Error **errp)
{
- CpuDefinitionInfoList *entry;
- CpuDefinitionInfo *info;
+ S390MachineProps mach;
+ GSList *classes;
+ uint64_t *mask = NULL;
+ CpuDefinitionInfoList *list = NULL;
+
+ if (has_machine) {
+ mask = s390_fac_list_mask_by_machine(machine);
+ if (!mask) {
+ error_set(errp, QERR_INVALID_PARAMETER_VALUE, "machine",
+ "a valid machine type");
+ return NULL;
+ }
+ }
- info = g_malloc0(sizeof(*info));
- info->name = g_strdup("host");
+ memset(&mach, 0, sizeof(mach));
+ if (has_accel) {
+ switch (accel) {
+ case ACCEL_ID_KVM:
+ kvm_s390_get_machine_props(NULL, &mach);
+ break;
+ default:
+ return qmp_query_cpu_definition_host();
+ }
+ }
- entry = g_malloc0(sizeof(*entry));
- entry->value = info;
+ s390_setup_cpu_classes(ACCEL_TEMP, &mach, mask);
+
+ classes = object_class_get_list(TYPE_S390_CPU, false);
+ classes = g_slist_sort(classes, s390_cpu_class_asc_order_compare);
+ g_slist_foreach(classes, qmp_query_cpu_definition_entry, &list);
+ g_slist_free(classes);
- return entry;
+ return list;
}
#endif
diff --git a/target-s390x/kvm.c b/target-s390x/kvm.c
index 4d75ff0..94fede5 100644
--- a/target-s390x/kvm.c
+++ b/target-s390x/kvm.c
@@ -276,12 +276,59 @@ static int cpu_model_set(KVMState *s, uint64_t attr, void *addr)
return rc;
}
-static int kvm_s390_get_machine_props(KVMState *s, S390MachineProps *prop)
+static int get_machine_props_fallback(S390MachineProps *prop)
+{
+ struct kvm_device_attr dev_attr;
+ int rc, kvmfd = -1, vmfd = -1;
+
+ rc = qemu_open("/dev/kvm", O_RDWR);
+ if (rc < 0) {
+ goto out_err;
+ }
+ kvmfd = rc;
+
+ rc = ioctl(kvmfd, KVM_CREATE_VM, 0);
+ if (rc < 0) {
+ goto out_err;
+ }
+ vmfd = rc;
+
+ rc = ioctl(vmfd, KVM_CHECK_EXTENSION, KVM_CAP_VM_ATTRIBUTES);
+ if (rc < 0) {
+ rc = -ENOSYS;
+ goto out_err;
+ }
+
+ dev_attr.group = KVM_S390_VM_CPU_MODEL;
+ dev_attr.attr = KVM_S390_VM_CPU_MACHINE;
+ rc = ioctl(vmfd, KVM_HAS_DEVICE_ATTR, &dev_attr);
+ if (rc < 0) {
+ rc = -EFAULT;
+ goto out_err;
+ }
+
+ dev_attr.addr = (uint64_t) prop;
+ rc = ioctl(vmfd, KVM_GET_DEVICE_ATTR, &dev_attr);
+
+out_err:
+ if (vmfd >= 0) {
+ close(vmfd);
+ }
+ if (kvmfd >= 0) {
+ close(kvmfd);
+ }
+
+ return rc;
+}
+
+int kvm_s390_get_machine_props(KVMState *s, S390MachineProps *prop)
{
int rc = -EFAULT;
if (s) {
rc = cpu_model_get(s, KVM_S390_VM_CPU_MACHINE, prop);
+ } else {
+ rc = get_machine_props_fallback(prop);
}
trace_kvm_get_machine_props(rc, prop->cpuid, prop->ibc);
--
1.8.3.1
^ permalink raw reply related [flat|nested] 55+ messages in thread
* [Qemu-devel] [PATCH v6 16/17] target-s390x: Introduce S390 CPU facility test routine
2015-04-27 14:53 [Qemu-devel] [PATCH v6 00/17] s390 cpu model implementation Michael Mueller
` (14 preceding siblings ...)
2015-04-27 14:53 ` [Qemu-devel] [PATCH v6 15/17] target-s390x: Extend arch specific QMP command query-cpu-definitions Michael Mueller
@ 2015-04-27 14:53 ` Michael Mueller
2015-04-27 14:53 ` [Qemu-devel] [PATCH v6 17/17] target-s390x: Enable S390 CPU model usage Michael Mueller
16 siblings, 0 replies; 55+ messages in thread
From: Michael Mueller @ 2015-04-27 14:53 UTC (permalink / raw)
To: qemu-devel
Cc: Eduardo Habkost, Gleb Natapov, Alexander Graf,
Christian Borntraeger, Daniel Hansel, Jason J. Herne,
Cornelia Huck, Paolo Bonzini, Richard Henderson, Andreas Faerber,
Michael Mueller
The patch introduces routine s390_facility_test() which allows to
verify if a specific facility bit is set.
Signed-off-by: Michael Mueller <mimu@linux.vnet.ibm.com>
Acked-by: Christian Borntraeger <borntraeger@de.ibm.com>
---
target-s390x/cpu-models.c | 29 +++++++++++++++++++++++++++++
target-s390x/cpu-models.h | 1 +
2 files changed, 30 insertions(+)
diff --git a/target-s390x/cpu-models.c b/target-s390x/cpu-models.c
index 7efb3e7..b70b7e1 100644
--- a/target-s390x/cpu-models.c
+++ b/target-s390x/cpu-models.c
@@ -715,3 +715,32 @@ void s390_cpu_model_init(S390CPUClass *cc)
kvm_s390_set_processor_props(&proc);
}
}
+
+static inline int test_facility(uint64_t *fac_list, uint16_t nr)
+{
+ uint16_t word = nr / BITS_PER_LONG;
+ uint16_t be_bit = (BITS_PER_LONG - 1) - (nr % BITS_PER_LONG);
+
+ return (nr < FAC_LIST_CPU_S390_SIZE_UINT1) ?
+ (fac_list[word] >> be_bit) & __UINT64_C(1) : 0;
+}
+
+/**
+ * s390_test_facility:
+ * @nr: facility bit number to test
+ * @cc: cpu class to test
+ *
+ * The functions tests if the cpu facility identified by bit @nr is available
+ * to the cpu class @cc.
+ *
+ * Returns: a boolean value.
+ *
+ * Since: 2.4
+ */
+bool s390_test_facility(S390CPUClass *cc, uint16_t nr)
+{
+ if (!cc) {
+ return false;
+ }
+ return test_facility(cc->fac_list[ACCEL_CURRENT], nr) ? true : false;
+}
diff --git a/target-s390x/cpu-models.h b/target-s390x/cpu-models.h
index 76673f4..23242aa 100644
--- a/target-s390x/cpu-models.h
+++ b/target-s390x/cpu-models.h
@@ -93,6 +93,7 @@ bool s390_cpu_classes_initialized(void);
uint64_t *s390_fac_list_mask_by_machine(const char *name);
uint64_t *s390_current_fac_list_mask(void);
void s390_cpu_model_init(S390CPUClass *cc);
+bool s390_test_facility(S390CPUClass *cc, uint16_t nr);
#ifdef CONFIG_KVM
int kvm_s390_get_machine_props(KVMState *s, S390MachineProps *prop);
--
1.8.3.1
^ permalink raw reply related [flat|nested] 55+ messages in thread
* [Qemu-devel] [PATCH v6 17/17] target-s390x: Enable S390 CPU model usage
2015-04-27 14:53 [Qemu-devel] [PATCH v6 00/17] s390 cpu model implementation Michael Mueller
` (15 preceding siblings ...)
2015-04-27 14:53 ` [Qemu-devel] [PATCH v6 16/17] target-s390x: Introduce S390 CPU facility test routine Michael Mueller
@ 2015-04-27 14:53 ` Michael Mueller
16 siblings, 0 replies; 55+ messages in thread
From: Michael Mueller @ 2015-04-27 14:53 UTC (permalink / raw)
To: qemu-devel
Cc: Eduardo Habkost, Gleb Natapov, Alexander Graf,
Christian Borntraeger, Daniel Hansel, Jason J. Herne,
Cornelia Huck, Paolo Bonzini, Richard Henderson, Andreas Faerber,
Michael Mueller
This patch enables QEMU to instantiate S390 CPUs with CPU model types.
Signed-off-by: Michael Mueller <mimu@linux.vnet.ibm.com>
Acked-by: Christian Borntraeger <borntraeger@de.ibm.com>
---
hw/s390x/s390-virtio.c | 12 +++++++++++-
target-s390x/helper.c | 9 ++-------
2 files changed, 13 insertions(+), 8 deletions(-)
diff --git a/hw/s390x/s390-virtio.c b/hw/s390x/s390-virtio.c
index bdb5388..8fda717 100644
--- a/hw/s390x/s390-virtio.c
+++ b/hw/s390x/s390-virtio.c
@@ -32,6 +32,7 @@
#include "hw/virtio/virtio.h"
#include "hw/sysbus.h"
#include "sysemu/kvm.h"
+#include "sysemu/cpus.h"
#include "exec/address-spaces.h"
#include "hw/s390x/s390-virtio-bus.h"
@@ -157,7 +158,12 @@ void s390_init_cpus(const char *cpu_model, uint8_t *storage_keys)
int i;
if (cpu_model == NULL) {
- cpu_model = "host";
+ cpu_model = "none";
+ }
+
+ if (is_help_option(cpu_model)) {
+ list_cpus(stdout, &fprintf, cpu_model);
+ exit(0);
}
ipi_states = g_malloc(sizeof(S390CPU *) * smp_cpus);
@@ -167,6 +173,10 @@ void s390_init_cpus(const char *cpu_model, uint8_t *storage_keys)
CPUState *cs;
cpu = cpu_s390x_init(cpu_model);
+ if (cpu == NULL) {
+ fprintf(stderr, "Unable to find CPU definition\n");
+ exit(1);
+ }
cs = CPU(cpu);
ipi_states[i] = cpu;
diff --git a/target-s390x/helper.c b/target-s390x/helper.c
index f1060c2..29dabe2 100644
--- a/target-s390x/helper.c
+++ b/target-s390x/helper.c
@@ -22,6 +22,7 @@
#include "exec/gdbstub.h"
#include "qemu/timer.h"
#include "exec/cpu_ldst.h"
+#include "cpu-models.h"
#ifndef CONFIG_USER_ONLY
#include "sysemu/sysemu.h"
#endif
@@ -66,13 +67,7 @@ void s390x_cpu_timer(void *opaque)
S390CPU *cpu_s390x_init(const char *cpu_model)
{
- S390CPU *cpu;
-
- cpu = S390_CPU(object_new(TYPE_S390_CPU));
-
- object_property_set_bool(OBJECT(cpu), true, "realized", NULL);
-
- return cpu;
+ return S390_CPU(cpu_generic_init(TYPE_S390_CPU, cpu_model));
}
#if defined(CONFIG_USER_ONLY)
--
1.8.3.1
^ permalink raw reply related [flat|nested] 55+ messages in thread
* Re: [Qemu-devel] [PATCH v6 03/17] Extend QMP command query-cpus to return accelerator id and model name
2015-04-27 14:53 ` [Qemu-devel] [PATCH v6 03/17] Extend QMP command query-cpus to return accelerator id and model name Michael Mueller
@ 2015-05-05 13:11 ` Eduardo Habkost
2015-05-06 9:49 ` Michael Mueller
0 siblings, 1 reply; 55+ messages in thread
From: Eduardo Habkost @ 2015-05-05 13:11 UTC (permalink / raw)
To: Michael Mueller
Cc: Gleb Natapov, qemu-devel, Alexander Graf, Christian Borntraeger,
Daniel Hansel, Jason J. Herne, Cornelia Huck, Paolo Bonzini,
Andreas Faerber, Richard Henderson
On Mon, Apr 27, 2015 at 04:53:17PM +0200, Michael Mueller wrote:
> The QMP command query-cpus now additionally displays a model name and
> the backing accelerator. Both are omitted if the model name is not
> initialized.
>
> request:
> { "execute" : "query-cpus" }
>
> answer:
> { { "current": true,
> "CPU": 0,
> "model": "2827-ga2",
> "halted": false,
> "accel": "kvm",
> "thread_id": 31917
> }, ... }
>
> Signed-off-by: Michael Mueller <mimu@linux.vnet.ibm.com>
> Acked-by: Christian Borntraeger <borntraeger@de.ibm.com>
With the new qom-path field I submitted yesterday, this can be provided
as QOM properties through qom-get.
--
Eduardo
^ permalink raw reply [flat|nested] 55+ messages in thread
* Re: [Qemu-devel] [PATCH v6 04/17] Extend HMP command info cpus to display accelerator id and model name
2015-04-27 14:53 ` [Qemu-devel] [PATCH v6 04/17] Extend HMP command info cpus to display " Michael Mueller
@ 2015-05-05 13:14 ` Eduardo Habkost
2015-05-06 7:32 ` Michael Mueller
0 siblings, 1 reply; 55+ messages in thread
From: Eduardo Habkost @ 2015-05-05 13:14 UTC (permalink / raw)
To: Michael Mueller
Cc: Gleb Natapov, qemu-devel, Alexander Graf, Christian Borntraeger,
Daniel Hansel, Jason J. Herne, Cornelia Huck, Paolo Bonzini,
Luiz Capitulino, Andreas Faerber, Richard Henderson
On Mon, Apr 27, 2015 at 04:53:18PM +0200, Michael Mueller wrote:
> The HMP command info cpus now displays the CPU model name and the
> backing accelerator if part of the CPUState.
>
> (qemu) info cpus
> * CPU #0: (halted) model=2827-ga2 accel=kvm thread_id=1679
>
> Signed-off-by: Michael Mueller <mimu@linux.vnet.ibm.com>
> Acked-by: Christian Borntraeger <borntraeger@de.ibm.com>
Do we really need this? I mean: I expect the amount of CPU data we
provide to QMP clients to grow a lot in the near future, but that
doesn't mean HMP users need all that data to be printed by "info cpus".
> ---
> hmp.c | 7 +++++++
> 1 file changed, 7 insertions(+)
>
> diff --git a/hmp.c b/hmp.c
> index f142d36..676d821 100644
> --- a/hmp.c
> +++ b/hmp.c
> @@ -290,6 +290,13 @@ void hmp_info_cpus(Monitor *mon, const QDict *qdict)
> monitor_printf(mon, " (halted)");
> }
>
> + if (cpu->value->has_model) {
> + monitor_printf(mon, " model=%s", cpu->value->model);
> + }
> + if (cpu->value->has_accel) {
> + monitor_printf(mon, " accel=%s", AccelId_lookup[cpu->value->accel]);
> + }
> +
> monitor_printf(mon, " thread_id=%" PRId64 "\n", cpu->value->thread_id);
> }
>
> --
> 1.8.3.1
>
--
Eduardo
^ permalink raw reply [flat|nested] 55+ messages in thread
* Re: [Qemu-devel] [PATCH v6 02/17] Add accelerator id and model name to CPUState
2015-04-27 14:53 ` [Qemu-devel] [PATCH v6 02/17] Add accelerator id and model name to CPUState Michael Mueller
@ 2015-05-05 13:26 ` Eduardo Habkost
2015-05-05 14:36 ` Eric Blake
2015-05-06 9:59 ` Michael Mueller
0 siblings, 2 replies; 55+ messages in thread
From: Eduardo Habkost @ 2015-05-05 13:26 UTC (permalink / raw)
To: Michael Mueller
Cc: Gleb Natapov, qemu-devel, Alexander Graf, Christian Borntraeger,
Daniel Hansel, Jason J. Herne, Cornelia Huck, Paolo Bonzini,
Andreas Faerber, Richard Henderson
On Mon, Apr 27, 2015 at 04:53:16PM +0200, Michael Mueller wrote:
> The patch defines ids per accelerator and adds the accel_id and
> the model_name to the CPUState. The accel_id is initialized by
> common code, the model name needs to be initialized by target
> specific code.
>
> Signed-off-by: Michael Mueller <mimu@linux.vnet.ibm.com>
> Acked-by: Christian Borntraeger <borntraeger@de.ibm.com>
> ---
> include/qom/cpu.h | 5 +++++
> qapi-schema.json | 9 +++++++++
> qom/cpu.c | 14 ++++++++++++++
> 3 files changed, 28 insertions(+)
>
> diff --git a/include/qom/cpu.h b/include/qom/cpu.h
> index 9dafb48..4ffc050 100644
> --- a/include/qom/cpu.h
> +++ b/include/qom/cpu.h
> @@ -236,6 +236,8 @@ struct kvm_run;
> * @mem_io_pc: Host Program Counter at which the memory was accessed.
> * @mem_io_vaddr: Target virtual address at which the memory was accessed.
> * @kvm_fd: vCPU file descriptor for KVM.
> + * @accel_id: accelerator id of this CPU.
> + * @model_name: model name of this CPU
> *
> * State of one CPU core or thread.
> */
> @@ -313,6 +315,9 @@ struct CPUState {
> (absolute value) offset as small as possible. This reduces code
> size, especially for hosts without large memory offsets. */
> volatile sig_atomic_t tcg_exit_req;
> +
> + AccelId accel_id;
This can be a AccelState pointer, set on initialization, because we have
another user case for having a AccelState pointer: query-cpu-definition
implementations may create temporary CPU objects with a different accel
object to be able to probe for accel-specific data.
(The pointer may become a link QOM property later.)
> + char *model_name;
> };
>
> QTAILQ_HEAD(CPUTailQ, CPUState);
> diff --git a/qapi-schema.json b/qapi-schema.json
> index ac9594d..540e520 100644
> --- a/qapi-schema.json
> +++ b/qapi-schema.json
> @@ -2515,6 +2515,15 @@
> ##
> { 'command': 'query-machines', 'returns': ['MachineInfo'] }
>
> +# @AccelId
> +#
> +# Defines accelerator ids
> +#
> +# Since: 2.4
> +##
> +{ 'enum': 'AccelId',
> + 'data': ['qtest', 'tcg', 'kvm', 'xen'] }
> +
Not sure if it is better to have an enum or simply a string here.
> ##
> # @CpuDefinitionInfo:
> #
> diff --git a/qom/cpu.c b/qom/cpu.c
> index 108bfa2..457afc7 100644
> --- a/qom/cpu.c
> +++ b/qom/cpu.c
> @@ -67,6 +67,20 @@ CPUState *cpu_generic_init(const char *typename, const char *cpu_model)
> goto out;
> }
>
> + if (tcg_enabled()) {
> + cpu->accel_id = ACCEL_ID_TCG;
> + } else if (kvm_enabled()) {
> + cpu->accel_id = ACCEL_ID_KVM;
> + }
> +#ifdef CONFIG_XEN
> + else if (xen_enabled()) {
> + cpu->accel_id = ACCEL_ID_XEN;
> + }
> +#endif
> + else {
> + cpu->accel_id = ACCEL_ID_QTEST;
> + }
You can simply use ACCEL_GET_CLASS(current_machine->accelerator)->name
here. If we really want an enum, we can add an AccelId field to
AccelClass, and initialize it properly on the accel class_init
functions.
CONFIG_USER may require some special code when returning the accelerator
ID as "tcg" because IIRC it doesn't use the QOM accel classes (yet).
> +
> object_property_set_bool(OBJECT(cpu), true, "realized", &err);
>
> out:
> --
> 1.8.3.1
>
--
Eduardo
^ permalink raw reply [flat|nested] 55+ messages in thread
* Re: [Qemu-devel] [PATCH v6 01/17] Introduce stub routine cpu_desc_avail
2015-04-27 14:53 ` [Qemu-devel] [PATCH v6 01/17] Introduce stub routine cpu_desc_avail Michael Mueller
@ 2015-05-05 13:55 ` Eduardo Habkost
2015-05-05 16:12 ` Michael Mueller
0 siblings, 1 reply; 55+ messages in thread
From: Eduardo Habkost @ 2015-05-05 13:55 UTC (permalink / raw)
To: Michael Mueller
Cc: Gleb Natapov, qemu-devel, Alexander Graf, Christian Borntraeger,
Daniel Hansel, Jason J. Herne, Cornelia Huck, Paolo Bonzini,
Andreas Faerber, Richard Henderson
On Mon, Apr 27, 2015 at 04:53:15PM +0200, Michael Mueller wrote:
> This patch introduces the function cpu_desc_avail() which returns by
> default true if not architecture specific implemented. Its intention
> is to indicate if the cpu model description is available for display
> by list_cpus(). This change allows cpu model descriptions to become
> dynamically created by evaluating the runtime context instead of
> putting static cpu model information at display.
Why are you deliberately breaking "-cpu ?" when cpu_desc_avail() is
false?
What exactly could cause cpu_desc_avail() to be false? If CPU model
information is not yet available when cpu_list() is called, it is a bug.
>
> Signed-off-by: Michael Mueller <mimu@linux.vnet.ibm.com>
> Reviewed-by: Thomas Huth <thuth@linux.vnet.ibm.com>
> Acked-by: Christian Borntraeger <borntraeger@de.ibm.com>
> ---
> include/qemu-common.h | 2 ++
> stubs/Makefile.objs | 1 +
> stubs/cpu-desc-avail.c | 6 ++++++
> vl.c | 2 +-
> 4 files changed, 10 insertions(+), 1 deletion(-)
> create mode 100644 stubs/cpu-desc-avail.c
>
> diff --git a/include/qemu-common.h b/include/qemu-common.h
> index 1b5cffb..386750f 100644
> --- a/include/qemu-common.h
> +++ b/include/qemu-common.h
> @@ -484,4 +484,6 @@ int parse_debug_env(const char *name, int max, int initial);
>
> const char *qemu_ether_ntoa(const MACAddr *mac);
>
> +bool cpu_desc_avail(void);
> +
> #endif
> diff --git a/stubs/Makefile.objs b/stubs/Makefile.objs
> index 8beff4c..dce9cd2 100644
> --- a/stubs/Makefile.objs
> +++ b/stubs/Makefile.objs
> @@ -39,3 +39,4 @@ stub-obj-$(CONFIG_WIN32) += fd-register.o
> stub-obj-y += cpus.o
> stub-obj-y += kvm.o
> stub-obj-y += qmp_pc_dimm_device_list.o
> +stub-obj-y += cpu-desc-avail.o
> diff --git a/stubs/cpu-desc-avail.c b/stubs/cpu-desc-avail.c
> new file mode 100644
> index 0000000..0cd594e
> --- /dev/null
> +++ b/stubs/cpu-desc-avail.c
> @@ -0,0 +1,6 @@
> +#include "qemu-common.h"
> +
> +bool cpu_desc_avail(void)
> +{
> + return true;
> +}
> diff --git a/vl.c b/vl.c
> index 74c2681..c552561 100644
> --- a/vl.c
> +++ b/vl.c
> @@ -3820,7 +3820,7 @@ int main(int argc, char **argv, char **envp)
> */
> cpudef_init();
>
> - if (cpu_model && is_help_option(cpu_model)) {
> + if (cpu_model && cpu_desc_avail() && is_help_option(cpu_model)) {
> list_cpus(stdout, &fprintf, cpu_model);
> exit(0);
> }
> --
> 1.8.3.1
>
--
Eduardo
^ permalink raw reply [flat|nested] 55+ messages in thread
* Re: [Qemu-devel] [PATCH v6 12/17] target-s390x: Add S390 CPU class initialization routines
2015-04-27 14:53 ` [Qemu-devel] [PATCH v6 12/17] target-s390x: Add S390 CPU class initialization routines Michael Mueller
@ 2015-05-05 14:34 ` Eduardo Habkost
2015-05-06 8:02 ` Michael Mueller
0 siblings, 1 reply; 55+ messages in thread
From: Eduardo Habkost @ 2015-05-05 14:34 UTC (permalink / raw)
To: Michael Mueller
Cc: Gleb Natapov, qemu-devel, Alexander Graf, Christian Borntraeger,
Daniel Hansel, Jason J. Herne, Cornelia Huck, Paolo Bonzini,
Andreas Faerber, Richard Henderson
On Mon, Apr 27, 2015 at 04:53:26PM +0200, Michael Mueller wrote:
> This patch provides routines to dynamically update the previously defined
> S390 CPU classes in the current host context. The main function performing
> this process is s390_setup_cpu_classes(). It takes the current host context
> and a facility list mask as parameter to setup the classes accordingly. It
> basically performs the following sub-tasks:
>
> - Update of CPU classes with accelerator specific host and QEMU properties
> - Mark adequate CPU class as default CPU class to be used for CPU model 'host'
> - Invalidate CPU classes not supported by this hosting machine
> - Define machine type aliases to latest GA number of a processor model
> - Define aliases for common CPU model names
> - Set CPU model alias 'host' to default CPU class
>
> Forthermore the patch provides the following routines:
>
> - cpu_desc_avail(), s390 specific stub indicating that list_cpus() can run
> - s390_setup_cpu_aliases(), adds cu model aliases
> - s390_cpu_classes_initialized(), test if CPU classes have been initialized
> - s390_fac_list_mask_by_machine(), returns facility list mask by machine
> - s390_current_fac_list_mask(), returns facility list mask of current machine
>
> Signed-off-by: Michael Mueller <mimu@linux.vnet.ibm.com>
> Acked-by: Christian Borntraeger <borntraeger@de.ibm.com>
> ---
[...]
> +/**
> + * s390_setup_cpu_classes:
> + * @mode: the accelerator mode
> + * @prop: the machine property structure's address
> + *
> + * This function validates the defined cpu classes against the given
> + * machine properties @prop. Only cpu classes that are runnable on the
> + * current host will be set active. In addition the corresponding
> + * cpuid, ibc value and the active set of facilities will be
> + * initialized. Depending on @mode, the function porforms operations
> + * on the current or the temporary accelerator properies.
> + *
> + * Since: 2.4
> + */
> +void s390_setup_cpu_classes(S390AccelMode mode, S390MachineProps *prop,
> + uint64_t *fac_list_mask)
> +{
Can't you replace the S390AccelMode arguments everywhere with simply an
AccelState pointer? That's the kind of thing that should have been
easier to implement using the accel QOM stuff.
If you still need to save accel-specific data somewhere (like the
is_active, is_host and fac_list arrays), maybe it can be indexed using
the AccelId enum you have introduced, instead of S390AccelMode?
> + GSList *list;
> + ParmAddrAddrModeMask parm = {
> + .mode = mode,
> + .prop = prop,
> + .mask = fac_list_mask,
> + .host_cc = NULL,
> + };
> +
> + list = object_class_get_list(TYPE_S390_CPU, false);
> + list = g_slist_sort(list, s390_cpu_class_asc_order_compare);
> +
> + g_slist_foreach(list, (GFunc) s390_update_cpu_class, (gpointer) &parm);
> + g_slist_foreach(list, (GFunc) s390_mark_host_cpu_class, (gpointer) &parm);
> + g_slist_foreach(list, (GFunc) s390_disable_not_supported_cpu_class, &parm);
> +
> + g_slist_free(list);
> +}
> +
[...]
--
Eduardo
^ permalink raw reply [flat|nested] 55+ messages in thread
* Re: [Qemu-devel] [PATCH v6 02/17] Add accelerator id and model name to CPUState
2015-05-05 13:26 ` Eduardo Habkost
@ 2015-05-05 14:36 ` Eric Blake
2015-05-05 14:46 ` Eduardo Habkost
2015-05-06 9:59 ` Michael Mueller
1 sibling, 1 reply; 55+ messages in thread
From: Eric Blake @ 2015-05-05 14:36 UTC (permalink / raw)
To: Eduardo Habkost, Michael Mueller
Cc: Cornelia Huck, Gleb Natapov, qemu-devel, Alexander Graf,
Christian Borntraeger, Jason J. Herne, Daniel Hansel,
Paolo Bonzini, Andreas Faerber, Richard Henderson
[-- Attachment #1: Type: text/plain, Size: 1277 bytes --]
On 05/05/2015 07:26 AM, Eduardo Habkost wrote:
> On Mon, Apr 27, 2015 at 04:53:16PM +0200, Michael Mueller wrote:
>> The patch defines ids per accelerator and adds the accel_id and
>> the model_name to the CPUState. The accel_id is initialized by
>> common code, the model name needs to be initialized by target
>> specific code.
>>
>> Signed-off-by: Michael Mueller <mimu@linux.vnet.ibm.com>
>> Acked-by: Christian Borntraeger <borntraeger@de.ibm.com>
>> ---
>> include/qom/cpu.h | 5 +++++
>> qapi-schema.json | 9 +++++++++
>> qom/cpu.c | 14 ++++++++++++++
>> 3 files changed, 28 insertions(+)
>>
>>
>> +# @AccelId
>> +#
>> +# Defines accelerator ids
>> +#
>> +# Since: 2.4
Might be nice to document the possible values, but not a show-stopper if
you don't.
>> +##
>> +{ 'enum': 'AccelId',
>> + 'data': ['qtest', 'tcg', 'kvm', 'xen'] }
>> +
>
> Not sure if it is better to have an enum or simply a string here.
enum is almost always better when there is a finite set of possible
strings - it's better documented, and when introspection is in place,
will make it easier to determine when the set has grown.
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]
^ permalink raw reply [flat|nested] 55+ messages in thread
* Re: [Qemu-devel] [PATCH v6 02/17] Add accelerator id and model name to CPUState
2015-05-05 14:36 ` Eric Blake
@ 2015-05-05 14:46 ` Eduardo Habkost
2015-05-06 9:28 ` Michael Mueller
0 siblings, 1 reply; 55+ messages in thread
From: Eduardo Habkost @ 2015-05-05 14:46 UTC (permalink / raw)
To: Eric Blake
Cc: Cornelia Huck, Michael Mueller, Gleb Natapov, Alexander Graf,
qemu-devel, Christian Borntraeger, Jason J. Herne, Daniel Hansel,
Paolo Bonzini, Andreas Faerber, Richard Henderson
On Tue, May 05, 2015 at 08:36:45AM -0600, Eric Blake wrote:
> On 05/05/2015 07:26 AM, Eduardo Habkost wrote:
> >> +{ 'enum': 'AccelId',
> >> + 'data': ['qtest', 'tcg', 'kvm', 'xen'] }
> >> +
> >
> > Not sure if it is better to have an enum or simply a string here.
>
> enum is almost always better when there is a finite set of possible
> strings - it's better documented, and when introspection is in place,
> will make it easier to determine when the set has grown.
True, and there are other cases where we could use an enum internally in
QEMU (e.g. arrays for accelerator-specific data inside CPU classes).
--
Eduardo
^ permalink raw reply [flat|nested] 55+ messages in thread
* Re: [Qemu-devel] [PATCH v6 01/17] Introduce stub routine cpu_desc_avail
2015-05-05 13:55 ` Eduardo Habkost
@ 2015-05-05 16:12 ` Michael Mueller
2015-05-05 17:41 ` Eduardo Habkost
0 siblings, 1 reply; 55+ messages in thread
From: Michael Mueller @ 2015-05-05 16:12 UTC (permalink / raw)
To: Eduardo Habkost
Cc: Cornelia Huck, Gleb Natapov, qemu-devel, Alexander Graf,
Christian Borntraeger, Jason J. Herne, Daniel Hansel,
Paolo Bonzini, Andreas Faerber, Richard Henderson
On Tue, 5 May 2015 10:55:47 -0300
Eduardo Habkost <ehabkost@redhat.com> wrote:
> On Mon, Apr 27, 2015 at 04:53:15PM +0200, Michael Mueller wrote:
> > This patch introduces the function cpu_desc_avail() which returns by
> > default true if not architecture specific implemented. Its intention
> > is to indicate if the cpu model description is available for display
> > by list_cpus(). This change allows cpu model descriptions to become
> > dynamically created by evaluating the runtime context instead of
> > putting static cpu model information at display.
>
> Why are you deliberately breaking "-cpu ?" when cpu_desc_avail() is
> false?
In the s390x case cpu_desc_avail() is per se false in this code section of vl.c:
/* Init CPU def lists, based on config
* - Must be called after all the qemu_read_config_file() calls
* - Must be called before list_cpu()
* - Must be called before machine->init()
*/
cpudef_init();
if (cpu_model && cpu_desc_avail() && is_help_option(cpu_model)) {
list_cpus(stdout, &fprintf, cpu_model);
exit(0);
}
That is because the output does not solely depend on static definitions
but also on runtime context. Here the host machine type this instance of
QEMU is running on, at least for the KVM case.
Once the accelerator has been initialized AND the S390 cpu classes have
been setup by means of the following code:
static void kvm_setup_cpu_classes(KVMState *s)
{
S390MachineProps mach;
if (!kvm_s390_get_machine_props(s, &mach)) {
s390_setup_cpu_classes(ACCEL_CURRENT, &mach,
s390_current_fac_list_mask());
s390_setup_cpu_aliases();
cpu_classes_initialized = true;
}
}
cpu_desc_avail() becomes true. In case the selceted mode was "?"
the list_cpu() is now done right before the cpu model is used as part
of the cpu initialization (hw/s390-virtio.c):
void s390_init_cpus(const char *cpu_model, uint8_t *storage_keys)
{
int i;
if (cpu_model == NULL) {
cpu_model = "none";
}
if (is_help_option(cpu_model)) {
list_cpus(stdout, &fprintf, cpu_model);
exit(0);
}
...
for (i = 0; i < smp_cpus; i++) {
...
cpu = cpu_s390x_init(cpu_model);
...
}
}
>
> What exactly could cause cpu_desc_avail() to be false? If CPU model
> information is not yet available when cpu_list() is called, it is a bug.
>
Here an example output that shows only runnable cpu models:
$ ./s390x-softmmu/qemu-system-s390x -machine s390,accel=kvm -cpu ?
s390 none
s390 2064-ga1 IBM zSeries 900 GA1
s390 2064-ga2 IBM zSeries 900 GA2
s390 2064-ga3 IBM zSeries 900 GA3
s390 2064 (alias for 2064-ga3)
s390 z900 (alias for 2064-ga3)
s390 2066-ga1 IBM zSeries 800 GA1
s390 2066 (alias for 2066-ga1)
s390 z800 (alias for 2066-ga1)
s390 2084-ga1 IBM zSeries 990 GA1
s390 2084-ga2 IBM zSeries 990 GA2
s390 2084-ga3 IBM zSeries 990 GA3
s390 2084-ga4 IBM zSeries 990 GA4
s390 2084-ga5 IBM zSeries 990 GA5
s390 2084 (alias for 2084-ga5)
s390 z990 (alias for 2084-ga5)
s390 2086-ga1 IBM zSeries 890 GA1
s390 2086-ga2 IBM zSeries 890 GA2
s390 2086-ga3 IBM zSeries 890 GA3
s390 2086 (alias for 2086-ga3)
s390 z890 (alias for 2086-ga3)
s390 2094-ga1 IBM System z9 EC GA1
s390 z9-109 (alias for 2094-ga1)
s390 2094-ga2 IBM System z9 EC GA2
s390 2094-ga3 IBM System z9 EC GA3
s390 2094 (alias for 2094-ga3)
s390 z9 (alias for 2094-ga3)
s390 z9-ec (alias for 2094-ga3)
s390 2096-ga1 IBM System z9 BC GA1
s390 2096-ga2 IBM System z9 BC GA2
s390 2096 (alias for 2096-ga2)
s390 z9-bc (alias for 2096-ga2)
s390 2097-ga1 IBM System z10 EC GA1
s390 2097-ga2 IBM System z10 EC GA2
s390 2097-ga3 IBM System z10 EC GA3
s390 2097 (alias for 2097-ga3)
s390 z10 (alias for 2097-ga3)
s390 z10-ec (alias for 2097-ga3)
s390 2098-ga1 IBM System z10 BC GA1
s390 2098-ga2 IBM System z10 BC GA2
s390 2098 (alias for 2098-ga2)
s390 z10-bc (alias for 2098-ga2)
s390 2817-ga1 IBM zEnterprise 196 GA1
s390 2817-ga2 IBM zEnterprise 196 GA2
s390 2817 (alias for 2817-ga2)
s390 z196 (alias for 2817-ga2)
s390 2818-ga1 IBM zEnterprise 114 GA1
s390 2818 (alias for 2818-ga1)
s390 z114 (alias for 2818-ga1)
s390 2827-ga1 IBM zEnterprise EC12 GA1
s390 2827-ga2 IBM zEnterprise EC12 GA2
s390 2827 (alias for 2827-ga2)
s390 zEC12 (alias for 2827-ga2)
s390 host (alias for 2827-ga2)
s390 2828-ga1 IBM zEnterprise BC12 GA1
s390 2828 (alias for 2828-ga1)
s390 zBC12 (alias for 2828-ga1)
> >
> > Signed-off-by: Michael Mueller <mimu@linux.vnet.ibm.com>
> > Reviewed-by: Thomas Huth <thuth@linux.vnet.ibm.com>
> > Acked-by: Christian Borntraeger <borntraeger@de.ibm.com>
> > ---
> > include/qemu-common.h | 2 ++
> > stubs/Makefile.objs | 1 +
> > stubs/cpu-desc-avail.c | 6 ++++++
> > vl.c | 2 +-
> > 4 files changed, 10 insertions(+), 1 deletion(-)
> > create mode 100644 stubs/cpu-desc-avail.c
> >
> > diff --git a/include/qemu-common.h b/include/qemu-common.h
> > index 1b5cffb..386750f 100644
> > --- a/include/qemu-common.h
> > +++ b/include/qemu-common.h
> > @@ -484,4 +484,6 @@ int parse_debug_env(const char *name, int max, int initial);
> >
> > const char *qemu_ether_ntoa(const MACAddr *mac);
> >
> > +bool cpu_desc_avail(void);
> > +
> > #endif
> > diff --git a/stubs/Makefile.objs b/stubs/Makefile.objs
> > index 8beff4c..dce9cd2 100644
> > --- a/stubs/Makefile.objs
> > +++ b/stubs/Makefile.objs
> > @@ -39,3 +39,4 @@ stub-obj-$(CONFIG_WIN32) += fd-register.o
> > stub-obj-y += cpus.o
> > stub-obj-y += kvm.o
> > stub-obj-y += qmp_pc_dimm_device_list.o
> > +stub-obj-y += cpu-desc-avail.o
> > diff --git a/stubs/cpu-desc-avail.c b/stubs/cpu-desc-avail.c
> > new file mode 100644
> > index 0000000..0cd594e
> > --- /dev/null
> > +++ b/stubs/cpu-desc-avail.c
> > @@ -0,0 +1,6 @@
> > +#include "qemu-common.h"
> > +
> > +bool cpu_desc_avail(void)
> > +{
> > + return true;
> > +}
> > diff --git a/vl.c b/vl.c
> > index 74c2681..c552561 100644
> > --- a/vl.c
> > +++ b/vl.c
> > @@ -3820,7 +3820,7 @@ int main(int argc, char **argv, char **envp)
> > */
> > cpudef_init();
> >
> > - if (cpu_model && is_help_option(cpu_model)) {
> > + if (cpu_model && cpu_desc_avail() && is_help_option(cpu_model)) {
> > list_cpus(stdout, &fprintf, cpu_model);
> > exit(0);
> > }
> > --
> > 1.8.3.1
> >
>
^ permalink raw reply [flat|nested] 55+ messages in thread
* Re: [Qemu-devel] [PATCH v6 01/17] Introduce stub routine cpu_desc_avail
2015-05-05 16:12 ` Michael Mueller
@ 2015-05-05 17:41 ` Eduardo Habkost
2015-05-06 9:17 ` Michael Mueller
0 siblings, 1 reply; 55+ messages in thread
From: Eduardo Habkost @ 2015-05-05 17:41 UTC (permalink / raw)
To: Michael Mueller
Cc: Cornelia Huck, Gleb Natapov, qemu-devel, Alexander Graf,
Christian Borntraeger, Jason J. Herne, Daniel Hansel,
Paolo Bonzini, Andreas Faerber, Richard Henderson
On Tue, May 05, 2015 at 06:12:16PM +0200, Michael Mueller wrote:
> On Tue, 5 May 2015 10:55:47 -0300
> Eduardo Habkost <ehabkost@redhat.com> wrote:
>
> > On Mon, Apr 27, 2015 at 04:53:15PM +0200, Michael Mueller wrote:
> > > This patch introduces the function cpu_desc_avail() which returns by
> > > default true if not architecture specific implemented. Its intention
> > > is to indicate if the cpu model description is available for display
> > > by list_cpus(). This change allows cpu model descriptions to become
> > > dynamically created by evaluating the runtime context instead of
> > > putting static cpu model information at display.
> >
> > Why are you deliberately breaking "-cpu ?" when cpu_desc_avail() is
> > false?
>
> In the s390x case cpu_desc_avail() is per se false in this code section of vl.c:
>
> /* Init CPU def lists, based on config
> * - Must be called after all the qemu_read_config_file() calls
> * - Must be called before list_cpu()
> * - Must be called before machine->init()
> */
(Side note: I believe the above outdated, I will send a patch to update
it.)
> cpudef_init();
>
> if (cpu_model && cpu_desc_avail() && is_help_option(cpu_model)) {
> list_cpus(stdout, &fprintf, cpu_model);
> exit(0);
> }
>
> That is because the output does not solely depend on static definitions
> but also on runtime context. Here the host machine type this instance of
> QEMU is running on, at least for the KVM case.
Is this a required feature? I would prefer to have the main() code
simple even if it means not having runnable information in "-cpu ?" by
now (about possible ways to implement this without cpu_desc_avail(), see
below).
>
> Once the accelerator has been initialized AND the S390 cpu classes have
> been setup by means of the following code:
>
> static void kvm_setup_cpu_classes(KVMState *s)
> {
> S390MachineProps mach;
>
> if (!kvm_s390_get_machine_props(s, &mach)) {
> s390_setup_cpu_classes(ACCEL_CURRENT, &mach,
> s390_current_fac_list_mask());
> s390_setup_cpu_aliases();
> cpu_classes_initialized = true;
> }
> }
>
> cpu_desc_avail() becomes true. In case the selceted mode was "?"
> the list_cpu() is now done right before the cpu model is used as part
> of the cpu initialization (hw/s390-virtio.c):
>
> void s390_init_cpus(const char *cpu_model, uint8_t *storage_keys)
> {
> int i;
>
> if (cpu_model == NULL) {
> cpu_model = "none";
> }
>
> if (is_help_option(cpu_model)) {
> list_cpus(stdout, &fprintf, cpu_model);
> exit(0);
> }
>
> ...
> for (i = 0; i < smp_cpus; i++) {
> ...
> cpu = cpu_s390x_init(cpu_model);
> ...
> }
> }
In other words, you just need to ensure that s390_cpu_list() run after
kvm_setup_cpu_classes().
Can't you simply call s390_setup_cpu_classes(ACCEL_TEMP) inside
s390_init_cpus(), just like arch_query_cpu_definitions()? You could even
share code between both functions.
(In the future, we should be able to implement "-cpu ?" by simply
calling the query-cpu-definitions implementation.)
>
> >
> > What exactly could cause cpu_desc_avail() to be false? If CPU model
> > information is not yet available when cpu_list() is called, it is a bug.
> >
>
> Here an example output that shows only runnable cpu models:
>
> $ ./s390x-softmmu/qemu-system-s390x -machine s390,accel=kvm -cpu ?
> s390 none
> s390 2064-ga1 IBM zSeries 900 GA1
> s390 2064-ga2 IBM zSeries 900 GA2
> s390 2064-ga3 IBM zSeries 900 GA3
> s390 2064 (alias for 2064-ga3)
> s390 z900 (alias for 2064-ga3)
> s390 2066-ga1 IBM zSeries 800 GA1
> s390 2066 (alias for 2066-ga1)
> s390 z800 (alias for 2066-ga1)
> s390 2084-ga1 IBM zSeries 990 GA1
> s390 2084-ga2 IBM zSeries 990 GA2
> s390 2084-ga3 IBM zSeries 990 GA3
> s390 2084-ga4 IBM zSeries 990 GA4
> s390 2084-ga5 IBM zSeries 990 GA5
> s390 2084 (alias for 2084-ga5)
> s390 z990 (alias for 2084-ga5)
> s390 2086-ga1 IBM zSeries 890 GA1
> s390 2086-ga2 IBM zSeries 890 GA2
> s390 2086-ga3 IBM zSeries 890 GA3
> s390 2086 (alias for 2086-ga3)
> s390 z890 (alias for 2086-ga3)
> s390 2094-ga1 IBM System z9 EC GA1
> s390 z9-109 (alias for 2094-ga1)
> s390 2094-ga2 IBM System z9 EC GA2
> s390 2094-ga3 IBM System z9 EC GA3
> s390 2094 (alias for 2094-ga3)
> s390 z9 (alias for 2094-ga3)
> s390 z9-ec (alias for 2094-ga3)
> s390 2096-ga1 IBM System z9 BC GA1
> s390 2096-ga2 IBM System z9 BC GA2
> s390 2096 (alias for 2096-ga2)
> s390 z9-bc (alias for 2096-ga2)
> s390 2097-ga1 IBM System z10 EC GA1
> s390 2097-ga2 IBM System z10 EC GA2
> s390 2097-ga3 IBM System z10 EC GA3
> s390 2097 (alias for 2097-ga3)
> s390 z10 (alias for 2097-ga3)
> s390 z10-ec (alias for 2097-ga3)
> s390 2098-ga1 IBM System z10 BC GA1
> s390 2098-ga2 IBM System z10 BC GA2
> s390 2098 (alias for 2098-ga2)
> s390 z10-bc (alias for 2098-ga2)
> s390 2817-ga1 IBM zEnterprise 196 GA1
> s390 2817-ga2 IBM zEnterprise 196 GA2
> s390 2817 (alias for 2817-ga2)
> s390 z196 (alias for 2817-ga2)
> s390 2818-ga1 IBM zEnterprise 114 GA1
> s390 2818 (alias for 2818-ga1)
> s390 z114 (alias for 2818-ga1)
> s390 2827-ga1 IBM zEnterprise EC12 GA1
> s390 2827-ga2 IBM zEnterprise EC12 GA2
> s390 2827 (alias for 2827-ga2)
> s390 zEC12 (alias for 2827-ga2)
> s390 host (alias for 2827-ga2)
> s390 2828-ga1 IBM zEnterprise BC12 GA1
> s390 2828 (alias for 2828-ga1)
> s390 zBC12 (alias for 2828-ga1)
>
>
[...]
--
Eduardo
^ permalink raw reply [flat|nested] 55+ messages in thread
* Re: [Qemu-devel] [PATCH v6 15/17] target-s390x: Extend arch specific QMP command query-cpu-definitions
2015-04-27 14:53 ` [Qemu-devel] [PATCH v6 15/17] target-s390x: Extend arch specific QMP command query-cpu-definitions Michael Mueller
@ 2015-05-05 18:40 ` Eduardo Habkost
2015-05-06 15:31 ` Michael Mueller
2015-05-06 12:37 ` Eduardo Habkost
1 sibling, 1 reply; 55+ messages in thread
From: Eduardo Habkost @ 2015-05-05 18:40 UTC (permalink / raw)
To: Michael Mueller
Cc: Gleb Natapov, qemu-devel, Alexander Graf, Christian Borntraeger,
Daniel Hansel, Jason J. Herne, Cornelia Huck, Paolo Bonzini,
Andreas Faerber, Richard Henderson
On Mon, Apr 27, 2015 at 04:53:29PM +0200, Michael Mueller wrote:
[...]
> diff --git a/target-s390x/kvm.c b/target-s390x/kvm.c
> index 4d75ff0..94fede5 100644
> --- a/target-s390x/kvm.c
> +++ b/target-s390x/kvm.c
> @@ -276,12 +276,59 @@ static int cpu_model_set(KVMState *s, uint64_t attr, void *addr)
> return rc;
> }
>
> -static int kvm_s390_get_machine_props(KVMState *s, S390MachineProps *prop)
This seems to duplicate lots of the existing KVM code. (See additional
comment below about possible ways to avoid it).
> +static int get_machine_props_fallback(S390MachineProps *prop)
> +{
> + struct kvm_device_attr dev_attr;
> + int rc, kvmfd = -1, vmfd = -1;
> +
> + rc = qemu_open("/dev/kvm", O_RDWR);
> + if (rc < 0) {
> + goto out_err;
> + }
> + kvmfd = rc;
> +
> + rc = ioctl(kvmfd, KVM_CREATE_VM, 0);
> + if (rc < 0) {
> + goto out_err;
> + }
> + vmfd = rc;
> +
> + rc = ioctl(vmfd, KVM_CHECK_EXTENSION, KVM_CAP_VM_ATTRIBUTES);
> + if (rc < 0) {
> + rc = -ENOSYS;
> + goto out_err;
> + }
> +
> + dev_attr.group = KVM_S390_VM_CPU_MODEL;
> + dev_attr.attr = KVM_S390_VM_CPU_MACHINE;
> + rc = ioctl(vmfd, KVM_HAS_DEVICE_ATTR, &dev_attr);
> + if (rc < 0) {
> + rc = -EFAULT;
> + goto out_err;
> + }
> +
> + dev_attr.addr = (uint64_t) prop;
> + rc = ioctl(vmfd, KVM_GET_DEVICE_ATTR, &dev_attr);
> +
> +out_err:
> + if (vmfd >= 0) {
> + close(vmfd);
> + }
> + if (kvmfd >= 0) {
> + close(kvmfd);
> + }
> +
> + return rc;
> +}
> +
> +int kvm_s390_get_machine_props(KVMState *s, S390MachineProps *prop)
> {
> int rc = -EFAULT;
>
> if (s) {
> rc = cpu_model_get(s, KVM_S390_VM_CPU_MACHINE, prop);
> + } else {
> + rc = get_machine_props_fallback(prop);
> }
The comments below are just suggestions, not something which should
block the patch, in my opinion:
First, if s is always NULL inside arch_query_cpu_definitions(), and is
always non-NULL inside kvm_setup_cpu_classes(), why don't you just call
keep the original kvm_s390_get_machine_props() function, and call and
get_machine_props_fallback() inside arch_query_cpu_definitions()?
The only thing common to both cases is the tracing point, but if we are
running two completely different code paths I assume it would be a good
thing to have a different tracing point for
get_machine_props_fallback().
Second, you shouldn't even need to duplicate code in
get_machine_props_fallback() if you are able to create an accel object
and do just basic initialization so that cpu_model_get() works.
Allowing accel objects to be created on the fly was one of the main
purposes of the accel QOM work.
For example, if we do something like this:
https://github.com/ehabkost/qemu-hacks/commit/36a250e34c5fd0d43a25271f5bc9b04681fdd56a [1]
https://github.com/ehabkost/qemu-hacks/commits/work/accel-open-func
Then the code could look like this:
accel.c:
/* configure_accelerator() would be changed to reuse this function: */
AccelState *accel_create(const char *accel_name)
{
AccelClass *acc = accel_find(accel_name);
/*TODO: error handling, checking acc->available() */
return ACCEL(object_new(object_class_get_name(OBJECT_CLASS(acc))));
}
/* Do basic accel initialization without affecting global QEMU state */
/* accel_init_machine() would be changed to reuse this function: */
void accel_open(AccelState *s, Error **errp)
{
object_property_set_bool(OBJECT(s), true, "open", errp);
}
target-s390/kvm.c:
/* Using a different function name would be interesting, as it would be
* the main arch_query_cpu_definitions() code path, not a fallback.
*/
int get_machine_props_fallback(S390MachineProps *prop)
{
int r;
AccelState *ac = accel_create("kvm");
/*TODO: error handling */
accel_open(ac, &err);
r = cpu_model_get(ac, prop);
object_unref(OBJECT(ac));
return r;
}
[1] I only moved the /dev/kvm opening to the open method, but maybe the
whole code up to KVM_CREATE_VM and capabitlity checking could be
moved.
(But I don't know how to handle kvm_type, as it is currently
provided by MachineClass. Maybe kvm_type() belongs to CPUClass
instead of MachineClass?)
> trace_kvm_get_machine_props(rc, prop->cpuid, prop->ibc);
>
> --
> 1.8.3.1
>
--
Eduardo
^ permalink raw reply [flat|nested] 55+ messages in thread
* Re: [Qemu-devel] [PATCH v6 04/17] Extend HMP command info cpus to display accelerator id and model name
2015-05-05 13:14 ` Eduardo Habkost
@ 2015-05-06 7:32 ` Michael Mueller
2015-05-06 10:38 ` Eduardo Habkost
0 siblings, 1 reply; 55+ messages in thread
From: Michael Mueller @ 2015-05-06 7:32 UTC (permalink / raw)
To: Eduardo Habkost
Cc: Cornelia Huck, Gleb Natapov, qemu-devel, Alexander Graf,
Christian Borntraeger, Jason J. Herne, Daniel Hansel,
Paolo Bonzini, Luiz Capitulino, Andreas Faerber,
Richard Henderson
On Tue, 5 May 2015 10:14:32 -0300
Eduardo Habkost <ehabkost@redhat.com> wrote:
> On Mon, Apr 27, 2015 at 04:53:18PM +0200, Michael Mueller wrote:
> > The HMP command info cpus now displays the CPU model name and the
> > backing accelerator if part of the CPUState.
> >
> > (qemu) info cpus
> > * CPU #0: (halted) model=2827-ga2 accel=kvm thread_id=1679
> >
> > Signed-off-by: Michael Mueller <mimu@linux.vnet.ibm.com>
> > Acked-by: Christian Borntraeger <borntraeger@de.ibm.com>
>
> Do we really need this? I mean: I expect the amount of CPU data we
> provide to QMP clients to grow a lot in the near future, but that
> doesn't mean HMP users need all that data to be printed by "info cpus".
Where do you see the limit of what is worth to be shown an what not.
I personally use "info cpus" less then sporadic but already got a comment
internally on that information being worthwhile to be shown.
>
>
> > ---
> > hmp.c | 7 +++++++
> > 1 file changed, 7 insertions(+)
> >
> > diff --git a/hmp.c b/hmp.c
> > index f142d36..676d821 100644
> > --- a/hmp.c
> > +++ b/hmp.c
> > @@ -290,6 +290,13 @@ void hmp_info_cpus(Monitor *mon, const QDict *qdict)
> > monitor_printf(mon, " (halted)");
> > }
> >
> > + if (cpu->value->has_model) {
> > + monitor_printf(mon, " model=%s", cpu->value->model);
> > + }
> > + if (cpu->value->has_accel) {
> > + monitor_printf(mon, " accel=%s", AccelId_lookup[cpu->value->accel]);
> > + }
> > +
> > monitor_printf(mon, " thread_id=%" PRId64 "\n", cpu->value->thread_id);
> > }
> >
> > --
> > 1.8.3.1
> >
>
^ permalink raw reply [flat|nested] 55+ messages in thread
* Re: [Qemu-devel] [PATCH v6 12/17] target-s390x: Add S390 CPU class initialization routines
2015-05-05 14:34 ` Eduardo Habkost
@ 2015-05-06 8:02 ` Michael Mueller
2015-05-06 12:20 ` Eduardo Habkost
0 siblings, 1 reply; 55+ messages in thread
From: Michael Mueller @ 2015-05-06 8:02 UTC (permalink / raw)
To: Eduardo Habkost
Cc: Cornelia Huck, Gleb Natapov, qemu-devel, Alexander Graf,
Christian Borntraeger, Jason J. Herne, Daniel Hansel,
Paolo Bonzini, Andreas Faerber, Richard Henderson
On Tue, 5 May 2015 11:34:06 -0300
Eduardo Habkost <ehabkost@redhat.com> wrote:
> On Mon, Apr 27, 2015 at 04:53:26PM +0200, Michael Mueller wrote:
> > This patch provides routines to dynamically update the previously defined
> > S390 CPU classes in the current host context. The main function performing
> > this process is s390_setup_cpu_classes(). It takes the current host context
> > and a facility list mask as parameter to setup the classes accordingly. It
> > basically performs the following sub-tasks:
> >
> > - Update of CPU classes with accelerator specific host and QEMU properties
> > - Mark adequate CPU class as default CPU class to be used for CPU model 'host'
> > - Invalidate CPU classes not supported by this hosting machine
> > - Define machine type aliases to latest GA number of a processor model
> > - Define aliases for common CPU model names
> > - Set CPU model alias 'host' to default CPU class
> >
> > Forthermore the patch provides the following routines:
> >
> > - cpu_desc_avail(), s390 specific stub indicating that list_cpus() can run
> > - s390_setup_cpu_aliases(), adds cu model aliases
> > - s390_cpu_classes_initialized(), test if CPU classes have been initialized
> > - s390_fac_list_mask_by_machine(), returns facility list mask by machine
> > - s390_current_fac_list_mask(), returns facility list mask of current machine
> >
> > Signed-off-by: Michael Mueller <mimu@linux.vnet.ibm.com>
> > Acked-by: Christian Borntraeger <borntraeger@de.ibm.com>
> > ---
> [...]
> > +/**
> > + * s390_setup_cpu_classes:
> > + * @mode: the accelerator mode
> > + * @prop: the machine property structure's address
> > + *
> > + * This function validates the defined cpu classes against the given
> > + * machine properties @prop. Only cpu classes that are runnable on the
> > + * current host will be set active. In addition the corresponding
> > + * cpuid, ibc value and the active set of facilities will be
> > + * initialized. Depending on @mode, the function porforms operations
> > + * on the current or the temporary accelerator properies.
> > + *
> > + * Since: 2.4
> > + */
> > +void s390_setup_cpu_classes(S390AccelMode mode, S390MachineProps *prop,
> > + uint64_t *fac_list_mask)
> > +{
>
> Can't you replace the S390AccelMode arguments everywhere with simply an
> AccelState pointer? That's the kind of thing that should have been
> easier to implement using the accel QOM stuff.
Would just make sense in conjunction with an AccelId indexed array
in the CPU class but see my concerns below.
>
> If you still need to save accel-specific data somewhere (like the
> is_active, is_host and fac_list arrays), maybe it can be indexed using
> the AccelId enum you have introduced, instead of S390AccelMode?
I had an AccelId indexed array in a previous version of the patch but
dismissed it in favor to this AccelMode index approach for the following
reasons:
a) There is just one accelerator active and and a second set of values is
used for the query-cpu-definitions case. Using the AcceldId index would
instantly double the required memory being used for no reason. The size
of the second dimension in uint64_t fac_list[ACCEL_MODE_MAX][FAC_LIST_CPU_S390_SIZE_UINT64];
is architecturally allowed to grow up to 2KB.
b) The information stored has more dimensions than just the accelerator,
it also contains the selected machine (s390-virtio) which is represented
by means of qemu_s390_fac_list_mask[] which currently is identical for
all machines but that will change as the implementation progresses.
So AccelMode (current, tmp) might also not fully express the semantics.
Michael
>
> > + GSList *list;
> > + ParmAddrAddrModeMask parm = {
> > + .mode = mode,
> > + .prop = prop,
> > + .mask = fac_list_mask,
> > + .host_cc = NULL,
> > + };
> > +
> > + list = object_class_get_list(TYPE_S390_CPU, false);
> > + list = g_slist_sort(list, s390_cpu_class_asc_order_compare);
> > +
> > + g_slist_foreach(list, (GFunc) s390_update_cpu_class, (gpointer) &parm);
> > + g_slist_foreach(list, (GFunc) s390_mark_host_cpu_class, (gpointer) &parm);
> > + g_slist_foreach(list, (GFunc) s390_disable_not_supported_cpu_class, &parm);
> > +
> > + g_slist_free(list);
> > +}
> > +
> [...]
>
^ permalink raw reply [flat|nested] 55+ messages in thread
* Re: [Qemu-devel] [PATCH v6 01/17] Introduce stub routine cpu_desc_avail
2015-05-05 17:41 ` Eduardo Habkost
@ 2015-05-06 9:17 ` Michael Mueller
2015-05-06 11:23 ` Eduardo Habkost
0 siblings, 1 reply; 55+ messages in thread
From: Michael Mueller @ 2015-05-06 9:17 UTC (permalink / raw)
To: Eduardo Habkost
Cc: Cornelia Huck, Gleb Natapov, qemu-devel, Alexander Graf,
Christian Borntraeger, Jason J. Herne, Daniel Hansel,
Paolo Bonzini, Andreas Faerber, Richard Henderson
On Tue, 5 May 2015 14:41:01 -0300
Eduardo Habkost <ehabkost@redhat.com> wrote:
> On Tue, May 05, 2015 at 06:12:16PM +0200, Michael Mueller wrote:
> > On Tue, 5 May 2015 10:55:47 -0300
> > Eduardo Habkost <ehabkost@redhat.com> wrote:
> >
> > > On Mon, Apr 27, 2015 at 04:53:15PM +0200, Michael Mueller wrote:
> > > > This patch introduces the function cpu_desc_avail() which returns by
> > > > default true if not architecture specific implemented. Its intention
> > > > is to indicate if the cpu model description is available for display
> > > > by list_cpus(). This change allows cpu model descriptions to become
> > > > dynamically created by evaluating the runtime context instead of
> > > > putting static cpu model information at display.
> > >
> > > Why are you deliberately breaking "-cpu ?" when cpu_desc_avail() is
> > > false?
> >
> > In the s390x case cpu_desc_avail() is per se false in this code section of vl.c:
> >
> > /* Init CPU def lists, based on config
> > * - Must be called after all the qemu_read_config_file() calls
> > * - Must be called before list_cpu()
> > * - Must be called before machine->init()
> > */
>
> (Side note: I believe the above outdated, I will send a patch to update
> it.)
Will be interesting to see what the change is, master is currently showing this code.
>
> > cpudef_init();
> >
> > if (cpu_model && cpu_desc_avail() && is_help_option(cpu_model)) {
> > list_cpus(stdout, &fprintf, cpu_model);
> > exit(0);
> > }
> >
> > That is because the output does not solely depend on static definitions
> > but also on runtime context. Here the host machine type this instance of
> > QEMU is running on, at least for the KVM case.
>
> Is this a required feature? I would prefer to have the main() code
> simple even if it means not having runnable information in "-cpu ?" by
> now (about possible ways to implement this without cpu_desc_avail(), see
> below).
I think it is more than a desired feature because one might end up with a failed
CPU object instantiation although the help screen claims to CPU model to be valid.
>
>
> >
> > Once the accelerator has been initialized AND the S390 cpu classes have
> > been setup by means of the following code:
> >
> > static void kvm_setup_cpu_classes(KVMState *s)
> > {
> > S390MachineProps mach;
> >
> > if (!kvm_s390_get_machine_props(s, &mach)) {
> > s390_setup_cpu_classes(ACCEL_CURRENT, &mach,
> > s390_current_fac_list_mask());
> > s390_setup_cpu_aliases();
> > cpu_classes_initialized = true;
> > }
> > }
> >
> > cpu_desc_avail() becomes true. In case the selceted mode was "?"
> > the list_cpu() is now done right before the cpu model is used as part
> > of the cpu initialization (hw/s390-virtio.c):
> >
> > void s390_init_cpus(const char *cpu_model, uint8_t *storage_keys)
> > {
> > int i;
> >
> > if (cpu_model == NULL) {
> > cpu_model = "none";
> > }
> >
> > if (is_help_option(cpu_model)) {
> > list_cpus(stdout, &fprintf, cpu_model);
> > exit(0);
> > }
> >
> > ...
> > for (i = 0; i < smp_cpus; i++) {
> > ...
> > cpu = cpu_s390x_init(cpu_model);
> > ...
> > }
> > }
>
>
> In other words, you just need to ensure that s390_cpu_list() run after
> kvm_setup_cpu_classes().
... which is part of the KVM/accel init process but it could of course make use
of the ACCEL_TMP use case as query-cpu-definitions does.
>
> Can't you simply call s390_setup_cpu_classes(ACCEL_TEMP) inside
> s390_init_cpus(), just like arch_query_cpu_definitions()? You could even
> share code between both functions.
That would not help with the current placement of list_cpus() in main() as it happens
way to early. Not s390_init_cpus() is the issue, the context information has been
processed already at that time. Currently I just kind of delay the list_cpus() until
all required information is available.
>
> (In the future, we should be able to implement "-cpu ?" by simply
> calling the query-cpu-definitions implementation.)
Right but the -machine <name>,accel=<accel> options have to be processed already.
>
> >
> > >
> > > What exactly could cause cpu_desc_avail() to be false? If CPU model
> > > information is not yet available when cpu_list() is called, it is a bug.
> > >
> >
> > Here an example output that shows only runnable cpu models:
> >
> > $ ./s390x-softmmu/qemu-system-s390x -machine s390,accel=kvm -cpu ?
> > s390 none
> > s390 2064-ga1 IBM zSeries 900 GA1
> > s390 2064-ga2 IBM zSeries 900 GA2
> > s390 2064-ga3 IBM zSeries 900 GA3
> > s390 2064 (alias for 2064-ga3)
> > s390 z900 (alias for 2064-ga3)
> > s390 2066-ga1 IBM zSeries 800 GA1
> > s390 2066 (alias for 2066-ga1)
> > s390 z800 (alias for 2066-ga1)
> > s390 2084-ga1 IBM zSeries 990 GA1
> > s390 2084-ga2 IBM zSeries 990 GA2
> > s390 2084-ga3 IBM zSeries 990 GA3
> > s390 2084-ga4 IBM zSeries 990 GA4
> > s390 2084-ga5 IBM zSeries 990 GA5
> > s390 2084 (alias for 2084-ga5)
> > s390 z990 (alias for 2084-ga5)
> > s390 2086-ga1 IBM zSeries 890 GA1
> > s390 2086-ga2 IBM zSeries 890 GA2
> > s390 2086-ga3 IBM zSeries 890 GA3
> > s390 2086 (alias for 2086-ga3)
> > s390 z890 (alias for 2086-ga3)
> > s390 2094-ga1 IBM System z9 EC GA1
> > s390 z9-109 (alias for 2094-ga1)
> > s390 2094-ga2 IBM System z9 EC GA2
> > s390 2094-ga3 IBM System z9 EC GA3
> > s390 2094 (alias for 2094-ga3)
> > s390 z9 (alias for 2094-ga3)
> > s390 z9-ec (alias for 2094-ga3)
> > s390 2096-ga1 IBM System z9 BC GA1
> > s390 2096-ga2 IBM System z9 BC GA2
> > s390 2096 (alias for 2096-ga2)
> > s390 z9-bc (alias for 2096-ga2)
> > s390 2097-ga1 IBM System z10 EC GA1
> > s390 2097-ga2 IBM System z10 EC GA2
> > s390 2097-ga3 IBM System z10 EC GA3
> > s390 2097 (alias for 2097-ga3)
> > s390 z10 (alias for 2097-ga3)
> > s390 z10-ec (alias for 2097-ga3)
> > s390 2098-ga1 IBM System z10 BC GA1
> > s390 2098-ga2 IBM System z10 BC GA2
> > s390 2098 (alias for 2098-ga2)
> > s390 z10-bc (alias for 2098-ga2)
> > s390 2817-ga1 IBM zEnterprise 196 GA1
> > s390 2817-ga2 IBM zEnterprise 196 GA2
> > s390 2817 (alias for 2817-ga2)
> > s390 z196 (alias for 2817-ga2)
> > s390 2818-ga1 IBM zEnterprise 114 GA1
> > s390 2818 (alias for 2818-ga1)
> > s390 z114 (alias for 2818-ga1)
> > s390 2827-ga1 IBM zEnterprise EC12 GA1
> > s390 2827-ga2 IBM zEnterprise EC12 GA2
> > s390 2827 (alias for 2827-ga2)
> > s390 zEC12 (alias for 2827-ga2)
> > s390 host (alias for 2827-ga2)
> > s390 2828-ga1 IBM zEnterprise BC12 GA1
> > s390 2828 (alias for 2828-ga1)
> > s390 zBC12 (alias for 2828-ga1)
> >
> >
> [...]
>
^ permalink raw reply [flat|nested] 55+ messages in thread
* Re: [Qemu-devel] [PATCH v6 02/17] Add accelerator id and model name to CPUState
2015-05-05 14:46 ` Eduardo Habkost
@ 2015-05-06 9:28 ` Michael Mueller
0 siblings, 0 replies; 55+ messages in thread
From: Michael Mueller @ 2015-05-06 9:28 UTC (permalink / raw)
To: Eduardo Habkost
Cc: Cornelia Huck, Gleb Natapov, qemu-devel, Alexander Graf,
Christian Borntraeger, Jason J. Herne, Daniel Hansel,
Paolo Bonzini, Andreas Faerber, Richard Henderson
On Tue, 5 May 2015 11:46:04 -0300
Eduardo Habkost <ehabkost@redhat.com> wrote:
> On Tue, May 05, 2015 at 08:36:45AM -0600, Eric Blake wrote:
> > On 05/05/2015 07:26 AM, Eduardo Habkost wrote:
> > >> +{ 'enum': 'AccelId',
> > >> + 'data': ['qtest', 'tcg', 'kvm', 'xen'] }
> > >> +
> > >
> > > Not sure if it is better to have an enum or simply a string here.
> >
> > enum is almost always better when there is a finite set of possible
> > strings - it's better documented, and when introspection is in place,
> > will make it easier to determine when the set has grown.
>
> True, and there are other cases where we could use an enum internally in
> QEMU (e.g. arrays for accelerator-specific data inside CPU classes).
Actually the accelerator name currently represented as string (ac->name = "KVM";)
can be initialized from the respective lookup value (AccelId_lookup[]) instead.
>
^ permalink raw reply [flat|nested] 55+ messages in thread
* Re: [Qemu-devel] [PATCH v6 03/17] Extend QMP command query-cpus to return accelerator id and model name
2015-05-05 13:11 ` Eduardo Habkost
@ 2015-05-06 9:49 ` Michael Mueller
2015-05-06 11:33 ` Eduardo Habkost
0 siblings, 1 reply; 55+ messages in thread
From: Michael Mueller @ 2015-05-06 9:49 UTC (permalink / raw)
To: Eduardo Habkost
Cc: Cornelia Huck, Gleb Natapov, qemu-devel, Alexander Graf,
Christian Borntraeger, Jason J. Herne, Daniel Hansel,
Paolo Bonzini, Andreas Faerber, Richard Henderson
On Tue, 5 May 2015 10:11:15 -0300
Eduardo Habkost <ehabkost@redhat.com> wrote:
> On Mon, Apr 27, 2015 at 04:53:17PM +0200, Michael Mueller wrote:
> > The QMP command query-cpus now additionally displays a model name and
> > the backing accelerator. Both are omitted if the model name is not
> > initialized.
> >
> > request:
> > { "execute" : "query-cpus" }
> >
> > answer:
> > { { "current": true,
> > "CPU": 0,
> > "model": "2827-ga2",
> > "halted": false,
> > "accel": "kvm",
> > "thread_id": 31917
> > }, ... }
> >
> > Signed-off-by: Michael Mueller <mimu@linux.vnet.ibm.com>
> > Acked-by: Christian Borntraeger <borntraeger@de.ibm.com>
>
> With the new qom-path field I submitted yesterday, this can be provided
> as QOM properties through qom-get.
Is that really a good idea to make the object representation part of the ABI.
I guess there is a related discussion already somewhere. I mean not just adding
the qom-path field, I saw that suggested patch, I mean the approach to expose the
objects themselves...
I will try your patch of course as well...
Michael
>
^ permalink raw reply [flat|nested] 55+ messages in thread
* Re: [Qemu-devel] [PATCH v6 02/17] Add accelerator id and model name to CPUState
2015-05-05 13:26 ` Eduardo Habkost
2015-05-05 14:36 ` Eric Blake
@ 2015-05-06 9:59 ` Michael Mueller
2015-05-06 11:41 ` Eduardo Habkost
1 sibling, 1 reply; 55+ messages in thread
From: Michael Mueller @ 2015-05-06 9:59 UTC (permalink / raw)
To: Eduardo Habkost
Cc: Cornelia Huck, Gleb Natapov, qemu-devel, Alexander Graf,
Christian Borntraeger, Jason J. Herne, Daniel Hansel,
Paolo Bonzini, Andreas Faerber, Richard Henderson
On Tue, 5 May 2015 10:26:02 -0300
Eduardo Habkost <ehabkost@redhat.com> wrote:
> On Mon, Apr 27, 2015 at 04:53:16PM +0200, Michael Mueller wrote:
> > The patch defines ids per accelerator and adds the accel_id and
> > the model_name to the CPUState. The accel_id is initialized by
> > common code, the model name needs to be initialized by target
> > specific code.
> >
> > Signed-off-by: Michael Mueller <mimu@linux.vnet.ibm.com>
> > Acked-by: Christian Borntraeger <borntraeger@de.ibm.com>
> > ---
> > include/qom/cpu.h | 5 +++++
> > qapi-schema.json | 9 +++++++++
> > qom/cpu.c | 14 ++++++++++++++
> > 3 files changed, 28 insertions(+)
> >
> > diff --git a/include/qom/cpu.h b/include/qom/cpu.h
> > index 9dafb48..4ffc050 100644
> > --- a/include/qom/cpu.h
> > +++ b/include/qom/cpu.h
> > @@ -236,6 +236,8 @@ struct kvm_run;
> > * @mem_io_pc: Host Program Counter at which the memory was accessed.
> > * @mem_io_vaddr: Target virtual address at which the memory was accessed.
> > * @kvm_fd: vCPU file descriptor for KVM.
> > + * @accel_id: accelerator id of this CPU.
> > + * @model_name: model name of this CPU
> > *
> > * State of one CPU core or thread.
> > */
> > @@ -313,6 +315,9 @@ struct CPUState {
> > (absolute value) offset as small as possible. This reduces code
> > size, especially for hosts without large memory offsets. */
> > volatile sig_atomic_t tcg_exit_req;
> > +
> > + AccelId accel_id;
>
> This can be a AccelState pointer, set on initialization, because we have
> another user case for having a AccelState pointer: query-cpu-definition
> implementations may create temporary CPU objects with a different accel
> object to be able to probe for accel-specific data.
>
> (The pointer may become a link QOM property later.)
>
>
> > + char *model_name;
> > };
> >
> > QTAILQ_HEAD(CPUTailQ, CPUState);
> > diff --git a/qapi-schema.json b/qapi-schema.json
> > index ac9594d..540e520 100644
> > --- a/qapi-schema.json
> > +++ b/qapi-schema.json
> > @@ -2515,6 +2515,15 @@
> > ##
> > { 'command': 'query-machines', 'returns': ['MachineInfo'] }
> >
> > +# @AccelId
> > +#
> > +# Defines accelerator ids
> > +#
> > +# Since: 2.4
> > +##
> > +{ 'enum': 'AccelId',
> > + 'data': ['qtest', 'tcg', 'kvm', 'xen'] }
> > +
>
> Not sure if it is better to have an enum or simply a string here.
>
> > ##
> > # @CpuDefinitionInfo:
> > #
> > diff --git a/qom/cpu.c b/qom/cpu.c
> > index 108bfa2..457afc7 100644
> > --- a/qom/cpu.c
> > +++ b/qom/cpu.c
> > @@ -67,6 +67,20 @@ CPUState *cpu_generic_init(const char *typename, const char *cpu_model)
> > goto out;
> > }
> >
> > + if (tcg_enabled()) {
> > + cpu->accel_id = ACCEL_ID_TCG;
> > + } else if (kvm_enabled()) {
> > + cpu->accel_id = ACCEL_ID_KVM;
> > + }
> > +#ifdef CONFIG_XEN
> > + else if (xen_enabled()) {
> > + cpu->accel_id = ACCEL_ID_XEN;
> > + }
> > +#endif
> > + else {
> > + cpu->accel_id = ACCEL_ID_QTEST;
> > + }
>
> You can simply use ACCEL_GET_CLASS(current_machine->accelerator)->name
> here. If we really want an enum, we can add an AccelId field to
> AccelClass, and initialize it properly on the accel class_init
> functions.
The AccelClass (ac = ACCEL_GET_CLASS(current_machine->accelerator) would be ok though.
That will allow to access the ac->accel_id (not yet there) at places where required.
I'm just not sure how to access current_machine here.
>
> CONFIG_USER may require some special code when returning the accelerator
> ID as "tcg" because IIRC it doesn't use the QOM accel classes (yet).
>
> > +
> > object_property_set_bool(OBJECT(cpu), true, "realized", &err);
> >
> > out:
> > --
> > 1.8.3.1
> >
>
^ permalink raw reply [flat|nested] 55+ messages in thread
* Re: [Qemu-devel] [PATCH v6 04/17] Extend HMP command info cpus to display accelerator id and model name
2015-05-06 7:32 ` Michael Mueller
@ 2015-05-06 10:38 ` Eduardo Habkost
2015-05-06 12:59 ` Luiz Capitulino
0 siblings, 1 reply; 55+ messages in thread
From: Eduardo Habkost @ 2015-05-06 10:38 UTC (permalink / raw)
To: Michael Mueller
Cc: Cornelia Huck, Gleb Natapov, qemu-devel, Alexander Graf,
Christian Borntraeger, Jason J. Herne, Daniel Hansel,
Paolo Bonzini, Luiz Capitulino, Andreas Faerber,
Richard Henderson
On Wed, May 06, 2015 at 09:32:58AM +0200, Michael Mueller wrote:
> On Tue, 5 May 2015 10:14:32 -0300
> Eduardo Habkost <ehabkost@redhat.com> wrote:
>
> > On Mon, Apr 27, 2015 at 04:53:18PM +0200, Michael Mueller wrote:
> > > The HMP command info cpus now displays the CPU model name and the
> > > backing accelerator if part of the CPUState.
> > >
> > > (qemu) info cpus
> > > * CPU #0: (halted) model=2827-ga2 accel=kvm thread_id=1679
> > >
> > > Signed-off-by: Michael Mueller <mimu@linux.vnet.ibm.com>
> > > Acked-by: Christian Borntraeger <borntraeger@de.ibm.com>
> >
> > Do we really need this? I mean: I expect the amount of CPU data we
> > provide to QMP clients to grow a lot in the near future, but that
> > doesn't mean HMP users need all that data to be printed by "info cpus".
>
> Where do you see the limit of what is worth to be shown an what not.
> I personally use "info cpus" less then sporadic but already got a comment
> internally on that information being worthwhile to be shown.
I really don't know, but I think we shouldn't add stuff to HMP unless we
have a good reason. For each new piece of data in HMP I would like to at
least see the description of a real use case that justifies adding it to
HMP and not just implementing a simple script on top of QMP.
For accel info we already have "info kvm" that is not ideal but is
enough for current use cases, isn't it? CPU model name information seems
to be more useful, but if it is just for debugging, people can just run
QMP query-cpus command.
Luiz, what do you think?
>
> >
> >
> > > ---
> > > hmp.c | 7 +++++++
> > > 1 file changed, 7 insertions(+)
> > >
> > > diff --git a/hmp.c b/hmp.c
> > > index f142d36..676d821 100644
> > > --- a/hmp.c
> > > +++ b/hmp.c
> > > @@ -290,6 +290,13 @@ void hmp_info_cpus(Monitor *mon, const QDict *qdict)
> > > monitor_printf(mon, " (halted)");
> > > }
> > >
> > > + if (cpu->value->has_model) {
> > > + monitor_printf(mon, " model=%s", cpu->value->model);
> > > + }
> > > + if (cpu->value->has_accel) {
> > > + monitor_printf(mon, " accel=%s", AccelId_lookup[cpu->value->accel]);
> > > + }
> > > +
> > > monitor_printf(mon, " thread_id=%" PRId64 "\n", cpu->value->thread_id);
> > > }
> > >
> > > --
> > > 1.8.3.1
> > >
> >
>
--
Eduardo
^ permalink raw reply [flat|nested] 55+ messages in thread
* Re: [Qemu-devel] [PATCH v6 01/17] Introduce stub routine cpu_desc_avail
2015-05-06 9:17 ` Michael Mueller
@ 2015-05-06 11:23 ` Eduardo Habkost
2015-05-06 16:23 ` Michael Mueller
0 siblings, 1 reply; 55+ messages in thread
From: Eduardo Habkost @ 2015-05-06 11:23 UTC (permalink / raw)
To: Michael Mueller
Cc: Cornelia Huck, Gleb Natapov, qemu-devel, Alexander Graf,
Christian Borntraeger, Jason J. Herne, Daniel Hansel,
Paolo Bonzini, Andreas Faerber, Richard Henderson
On Wed, May 06, 2015 at 11:17:20AM +0200, Michael Mueller wrote:
> On Tue, 5 May 2015 14:41:01 -0300
> Eduardo Habkost <ehabkost@redhat.com> wrote:
>
> > On Tue, May 05, 2015 at 06:12:16PM +0200, Michael Mueller wrote:
> > > On Tue, 5 May 2015 10:55:47 -0300
> > > Eduardo Habkost <ehabkost@redhat.com> wrote:
> > >
> > > > On Mon, Apr 27, 2015 at 04:53:15PM +0200, Michael Mueller wrote:
> > > > > This patch introduces the function cpu_desc_avail() which returns by
> > > > > default true if not architecture specific implemented. Its intention
> > > > > is to indicate if the cpu model description is available for display
> > > > > by list_cpus(). This change allows cpu model descriptions to become
> > > > > dynamically created by evaluating the runtime context instead of
> > > > > putting static cpu model information at display.
> > > >
> > > > Why are you deliberately breaking "-cpu ?" when cpu_desc_avail() is
> > > > false?
> > >
> > > In the s390x case cpu_desc_avail() is per se false in this code section of vl.c:
> > >
> > > /* Init CPU def lists, based on config
> > > * - Must be called after all the qemu_read_config_file() calls
> > > * - Must be called before list_cpu()
> > > * - Must be called before machine->init()
> > > */
> >
> > (Side note: I believe the above outdated, I will send a patch to update
> > it.)
>
> Will be interesting to see what the change is, master is currently showing this code.
We don't (and shouldn't) have the qemu_read_config_file() requirement,
as CPU models are not loaded from config files anymore.
And we should be able to eliminate cpudef_init() completely, soon. I
think the only user of cpudef_init() is x86.
>
> >
> > > cpudef_init();
> > >
> > > if (cpu_model && cpu_desc_avail() && is_help_option(cpu_model)) {
> > > list_cpus(stdout, &fprintf, cpu_model);
> > > exit(0);
> > > }
> > >
> > > That is because the output does not solely depend on static definitions
> > > but also on runtime context. Here the host machine type this instance of
> > > QEMU is running on, at least for the KVM case.
> >
> > Is this a required feature? I would prefer to have the main() code
> > simple even if it means not having runnable information in "-cpu ?" by
> > now (about possible ways to implement this without cpu_desc_avail(), see
> > below).
>
> I think it is more than a desired feature because one might end up with a failed
> CPU object instantiation although the help screen claims to CPU model to be valid.
I think you are more likely to confuse users by not showing information
on "-cpu ?" when -machine is not present. I believe most people use
"-cpu ?" with no other arguments, to see what the QEMU binary is capable
of.
Anyway, whatever we decide to do, I believe we should start with
something simple to get things working, and after that we can look for
ways improve the help output with "runnable" info.
>
> >
> >
> > >
> > > Once the accelerator has been initialized AND the S390 cpu classes have
> > > been setup by means of the following code:
> > >
> > > static void kvm_setup_cpu_classes(KVMState *s)
> > > {
> > > S390MachineProps mach;
> > >
> > > if (!kvm_s390_get_machine_props(s, &mach)) {
> > > s390_setup_cpu_classes(ACCEL_CURRENT, &mach,
> > > s390_current_fac_list_mask());
> > > s390_setup_cpu_aliases();
> > > cpu_classes_initialized = true;
> > > }
> > > }
> > >
> > > cpu_desc_avail() becomes true. In case the selceted mode was "?"
> > > the list_cpu() is now done right before the cpu model is used as part
> > > of the cpu initialization (hw/s390-virtio.c):
> > >
> > > void s390_init_cpus(const char *cpu_model, uint8_t *storage_keys)
> > > {
> > > int i;
> > >
> > > if (cpu_model == NULL) {
> > > cpu_model = "none";
> > > }
> > >
> > > if (is_help_option(cpu_model)) {
> > > list_cpus(stdout, &fprintf, cpu_model);
> > > exit(0);
> > > }
> > >
> > > ...
> > > for (i = 0; i < smp_cpus; i++) {
> > > ...
> > > cpu = cpu_s390x_init(cpu_model);
> > > ...
> > > }
> > > }
> >
> >
> > In other words, you just need to ensure that s390_cpu_list() run after
> > kvm_setup_cpu_classes().
>
> ... which is part of the KVM/accel init process but it could of course make use
> of the ACCEL_TMP use case as query-cpu-definitions does.
Exactly. I don't see a reason to not share code between
query-cpu-definitions and "-cpu ?".
>
> >
> > Can't you simply call s390_setup_cpu_classes(ACCEL_TEMP) inside
> > s390_init_cpus(), just like arch_query_cpu_definitions()? You could even
> > share code between both functions.
>
> That would not help with the current placement of list_cpus() in main() as it happens
> way to early. Not s390_init_cpus() is the issue, the context information has been
> processed already at that time. Currently I just kind of delay the list_cpus() until
> all required information is available.
I understand you are just delaying it. But it is requiring a hack inside
generic main() code that could be avoided. About the actual reasons to
delay it, see below.
>
> >
> > (In the future, we should be able to implement "-cpu ?" by simply
> > calling the query-cpu-definitions implementation.)
>
> Right but the -machine <name>,accel=<accel> options have to be processed already.
About query-cpu-definitions: its code shouldn't depend on -machine
options to work. Maybe it can use current_machine to get the default
accel and machine argument, but it should always output the same data
for a machine+accel combination even if you use "-machine none".
About "-cpu ?": do we really want it to depend on -machine processing?
Today, help output shows what the QEMU binary is capable of, not just
what the host system and -machine option are capable of.
If we decide to change that assumption, let's do it in a generic way and
not as a arch-specific hack. The options I see are:
1) Continue with the current policy where "-cpu ?" does not depend on
-machine arguments, and show all CPU models on "-cpu ?".
2) Deciding that, yes, it is OK to make "-cpu ?" depend on -machine
arguments, and move the list_cpus() call after machine initialization
inside generic main() code for all arches.
2.1) We could delay the list_cpus() call inside main() on all cases.
2.2) We could delay the list_cpus() call inside main() only if
an explicit -machine option is present.
I prefer (1) and my second choice would be (2.2), but the main point is
that none of the options above require making s390 special and
introducing cpu_desc_avail().
--
Eduardo
^ permalink raw reply [flat|nested] 55+ messages in thread
* Re: [Qemu-devel] [PATCH v6 03/17] Extend QMP command query-cpus to return accelerator id and model name
2015-05-06 9:49 ` Michael Mueller
@ 2015-05-06 11:33 ` Eduardo Habkost
0 siblings, 0 replies; 55+ messages in thread
From: Eduardo Habkost @ 2015-05-06 11:33 UTC (permalink / raw)
To: Michael Mueller
Cc: Cornelia Huck, Gleb Natapov, qemu-devel, Alexander Graf,
Christian Borntraeger, Jason J. Herne, Daniel Hansel,
Paolo Bonzini, Andreas Faerber, Richard Henderson
On Wed, May 06, 2015 at 11:49:50AM +0200, Michael Mueller wrote:
> On Tue, 5 May 2015 10:11:15 -0300
> Eduardo Habkost <ehabkost@redhat.com> wrote:
>
> > On Mon, Apr 27, 2015 at 04:53:17PM +0200, Michael Mueller wrote:
> > > The QMP command query-cpus now additionally displays a model name and
> > > the backing accelerator. Both are omitted if the model name is not
> > > initialized.
> > >
> > > request:
> > > { "execute" : "query-cpus" }
> > >
> > > answer:
> > > { { "current": true,
> > > "CPU": 0,
> > > "model": "2827-ga2",
> > > "halted": false,
> > > "accel": "kvm",
> > > "thread_id": 31917
> > > }, ... }
> > >
> > > Signed-off-by: Michael Mueller <mimu@linux.vnet.ibm.com>
> > > Acked-by: Christian Borntraeger <borntraeger@de.ibm.com>
> >
> > With the new qom-path field I submitted yesterday, this can be provided
> > as QOM properties through qom-get.
>
> Is that really a good idea to make the object representation part of the ABI.
I believe that's the whole point of QOM properties.
> I guess there is a related discussion already somewhere. I mean not just adding
> the qom-path field, I saw that suggested patch, I mean the approach to expose the
> objects themselves...
>
> I will try your patch of course as well...
Yes, there are two approaches we are considering to allow clients to
find the CPU QOM objects (qom-path in query-cpus, and links/aliases in
/machine/cpus). But whatever approach we use, if clients can find the
CPU objects in the QOM tree, you won't need the new fields in query-cpus
and the info can be provided using qom-get.
--
Eduardo
^ permalink raw reply [flat|nested] 55+ messages in thread
* Re: [Qemu-devel] [PATCH v6 02/17] Add accelerator id and model name to CPUState
2015-05-06 9:59 ` Michael Mueller
@ 2015-05-06 11:41 ` Eduardo Habkost
2015-05-07 7:55 ` Michael Mueller
0 siblings, 1 reply; 55+ messages in thread
From: Eduardo Habkost @ 2015-05-06 11:41 UTC (permalink / raw)
To: Michael Mueller
Cc: Cornelia Huck, Gleb Natapov, qemu-devel, Alexander Graf,
Christian Borntraeger, Jason J. Herne, Daniel Hansel,
Paolo Bonzini, Andreas Faerber, Richard Henderson
On Wed, May 06, 2015 at 11:59:38AM +0200, Michael Mueller wrote:
> On Tue, 5 May 2015 10:26:02 -0300
> Eduardo Habkost <ehabkost@redhat.com> wrote:
>
> > On Mon, Apr 27, 2015 at 04:53:16PM +0200, Michael Mueller wrote:
> > > The patch defines ids per accelerator and adds the accel_id and
> > > the model_name to the CPUState. The accel_id is initialized by
> > > common code, the model name needs to be initialized by target
> > > specific code.
> > >
> > > Signed-off-by: Michael Mueller <mimu@linux.vnet.ibm.com>
> > > Acked-by: Christian Borntraeger <borntraeger@de.ibm.com>
> > > ---
> > > include/qom/cpu.h | 5 +++++
> > > qapi-schema.json | 9 +++++++++
> > > qom/cpu.c | 14 ++++++++++++++
> > > 3 files changed, 28 insertions(+)
> > >
> > > diff --git a/include/qom/cpu.h b/include/qom/cpu.h
> > > index 9dafb48..4ffc050 100644
> > > --- a/include/qom/cpu.h
> > > +++ b/include/qom/cpu.h
> > > @@ -236,6 +236,8 @@ struct kvm_run;
> > > * @mem_io_pc: Host Program Counter at which the memory was accessed.
> > > * @mem_io_vaddr: Target virtual address at which the memory was accessed.
> > > * @kvm_fd: vCPU file descriptor for KVM.
> > > + * @accel_id: accelerator id of this CPU.
> > > + * @model_name: model name of this CPU
> > > *
> > > * State of one CPU core or thread.
> > > */
> > > @@ -313,6 +315,9 @@ struct CPUState {
> > > (absolute value) offset as small as possible. This reduces code
> > > size, especially for hosts without large memory offsets. */
> > > volatile sig_atomic_t tcg_exit_req;
> > > +
> > > + AccelId accel_id;
> >
> > This can be a AccelState pointer, set on initialization, because we have
> > another user case for having a AccelState pointer: query-cpu-definition
> > implementations may create temporary CPU objects with a different accel
> > object to be able to probe for accel-specific data.
> >
> > (The pointer may become a link QOM property later.)
> >
> >
> > > + char *model_name;
> > > };
> > >
> > > QTAILQ_HEAD(CPUTailQ, CPUState);
> > > diff --git a/qapi-schema.json b/qapi-schema.json
> > > index ac9594d..540e520 100644
> > > --- a/qapi-schema.json
> > > +++ b/qapi-schema.json
> > > @@ -2515,6 +2515,15 @@
> > > ##
> > > { 'command': 'query-machines', 'returns': ['MachineInfo'] }
> > >
> > > +# @AccelId
> > > +#
> > > +# Defines accelerator ids
> > > +#
> > > +# Since: 2.4
> > > +##
> > > +{ 'enum': 'AccelId',
> > > + 'data': ['qtest', 'tcg', 'kvm', 'xen'] }
> > > +
> >
> > Not sure if it is better to have an enum or simply a string here.
> >
> > > ##
> > > # @CpuDefinitionInfo:
> > > #
> > > diff --git a/qom/cpu.c b/qom/cpu.c
> > > index 108bfa2..457afc7 100644
> > > --- a/qom/cpu.c
> > > +++ b/qom/cpu.c
> > > @@ -67,6 +67,20 @@ CPUState *cpu_generic_init(const char *typename, const char *cpu_model)
> > > goto out;
> > > }
> > >
> > > + if (tcg_enabled()) {
> > > + cpu->accel_id = ACCEL_ID_TCG;
> > > + } else if (kvm_enabled()) {
> > > + cpu->accel_id = ACCEL_ID_KVM;
> > > + }
> > > +#ifdef CONFIG_XEN
> > > + else if (xen_enabled()) {
> > > + cpu->accel_id = ACCEL_ID_XEN;
> > > + }
> > > +#endif
> > > + else {
> > > + cpu->accel_id = ACCEL_ID_QTEST;
> > > + }
> >
> > You can simply use ACCEL_GET_CLASS(current_machine->accelerator)->name
> > here. If we really want an enum, we can add an AccelId field to
> > AccelClass, and initialize it properly on the accel class_init
> > functions.
>
> The AccelClass (ac = ACCEL_GET_CLASS(current_machine->accelerator) would be ok though.
> That will allow to access the ac->accel_id (not yet there) at places where required.
>
> I'm just not sure how to access current_machine here.
It is a global variable declared in hw/boards.h. But it makes sense only
if !CONFIG_USER.
But I am starting to wonder if we shouldn't simply expose accel info as
a /mmachine property, instead of per-CPU information. It may become
per-CPU in the future, but right now we really don't support having
multiple accelerators so why are we trying to pretend we do?
>
> >
> > CONFIG_USER may require some special code when returning the accelerator
> > ID as "tcg" because IIRC it doesn't use the QOM accel classes (yet).
> >
> > > +
> > > object_property_set_bool(OBJECT(cpu), true, "realized", &err);
> > >
> > > out:
> > > --
> > > 1.8.3.1
> > >
> >
>
--
Eduardo
^ permalink raw reply [flat|nested] 55+ messages in thread
* Re: [Qemu-devel] [PATCH v6 12/17] target-s390x: Add S390 CPU class initialization routines
2015-05-06 8:02 ` Michael Mueller
@ 2015-05-06 12:20 ` Eduardo Habkost
0 siblings, 0 replies; 55+ messages in thread
From: Eduardo Habkost @ 2015-05-06 12:20 UTC (permalink / raw)
To: Michael Mueller
Cc: Cornelia Huck, Gleb Natapov, qemu-devel, Alexander Graf,
Christian Borntraeger, Jason J. Herne, Daniel Hansel,
Paolo Bonzini, Andreas Faerber, Richard Henderson
On Wed, May 06, 2015 at 10:02:22AM +0200, Michael Mueller wrote:
> On Tue, 5 May 2015 11:34:06 -0300
> Eduardo Habkost <ehabkost@redhat.com> wrote:
>
> > On Mon, Apr 27, 2015 at 04:53:26PM +0200, Michael Mueller wrote:
> > > This patch provides routines to dynamically update the previously defined
> > > S390 CPU classes in the current host context. The main function performing
> > > this process is s390_setup_cpu_classes(). It takes the current host context
> > > and a facility list mask as parameter to setup the classes accordingly. It
> > > basically performs the following sub-tasks:
> > >
> > > - Update of CPU classes with accelerator specific host and QEMU properties
> > > - Mark adequate CPU class as default CPU class to be used for CPU model 'host'
> > > - Invalidate CPU classes not supported by this hosting machine
> > > - Define machine type aliases to latest GA number of a processor model
> > > - Define aliases for common CPU model names
> > > - Set CPU model alias 'host' to default CPU class
> > >
> > > Forthermore the patch provides the following routines:
> > >
> > > - cpu_desc_avail(), s390 specific stub indicating that list_cpus() can run
> > > - s390_setup_cpu_aliases(), adds cu model aliases
> > > - s390_cpu_classes_initialized(), test if CPU classes have been initialized
> > > - s390_fac_list_mask_by_machine(), returns facility list mask by machine
> > > - s390_current_fac_list_mask(), returns facility list mask of current machine
> > >
> > > Signed-off-by: Michael Mueller <mimu@linux.vnet.ibm.com>
> > > Acked-by: Christian Borntraeger <borntraeger@de.ibm.com>
> > > ---
> > [...]
> > > +/**
> > > + * s390_setup_cpu_classes:
> > > + * @mode: the accelerator mode
> > > + * @prop: the machine property structure's address
> > > + *
> > > + * This function validates the defined cpu classes against the given
> > > + * machine properties @prop. Only cpu classes that are runnable on the
> > > + * current host will be set active. In addition the corresponding
> > > + * cpuid, ibc value and the active set of facilities will be
> > > + * initialized. Depending on @mode, the function porforms operations
> > > + * on the current or the temporary accelerator properies.
> > > + *
> > > + * Since: 2.4
> > > + */
> > > +void s390_setup_cpu_classes(S390AccelMode mode, S390MachineProps *prop,
> > > + uint64_t *fac_list_mask)
> > > +{
> >
> > Can't you replace the S390AccelMode arguments everywhere with simply an
> > AccelState pointer? That's the kind of thing that should have been
> > easier to implement using the accel QOM stuff.
>
> Would just make sense in conjunction with an AccelId indexed array
> in the CPU class but see my concerns below.
>
> >
> > If you still need to save accel-specific data somewhere (like the
> > is_active, is_host and fac_list arrays), maybe it can be indexed using
> > the AccelId enum you have introduced, instead of S390AccelMode?
>
> I had an AccelId indexed array in a previous version of the patch but
> dismissed it in favor to this AccelMode index approach for the following
> reasons:
>
> a) There is just one accelerator active and and a second set of values is
> used for the query-cpu-definitions case. Using the AcceldId index would
> instantly double the required memory being used for no reason. The size
> of the second dimension in uint64_t fac_list[ACCEL_MODE_MAX][FAC_LIST_CPU_S390_SIZE_UINT64];
> is architecturally allowed to grow up to 2KB.
>
> b) The information stored has more dimensions than just the accelerator,
> it also contains the selected machine (s390-virtio) which is represented
> by means of qemu_s390_fac_list_mask[] which currently is identical for
> all machines but that will change as the implementation progresses.
>
> So AccelMode (current, tmp) might also not fully express the semantics.
Right. So the data depends on (cpu_model, accel, machine), but we need to cache
the results for very few (just 2) accel+machine combinations at any given
moment. AccelMode makes more sense to me, now.
--
Eduardo
^ permalink raw reply [flat|nested] 55+ messages in thread
* Re: [Qemu-devel] [PATCH v6 15/17] target-s390x: Extend arch specific QMP command query-cpu-definitions
2015-04-27 14:53 ` [Qemu-devel] [PATCH v6 15/17] target-s390x: Extend arch specific QMP command query-cpu-definitions Michael Mueller
2015-05-05 18:40 ` Eduardo Habkost
@ 2015-05-06 12:37 ` Eduardo Habkost
2015-05-06 14:48 ` Michael Mueller
1 sibling, 1 reply; 55+ messages in thread
From: Eduardo Habkost @ 2015-05-06 12:37 UTC (permalink / raw)
To: Michael Mueller
Cc: Cornelia Huck, Gleb Natapov, qemu-devel, Alexander Graf,
Christian Borntraeger, Jason J. Herne, Daniel Hansel,
Paolo Bonzini, Andreas Faerber, Richard Henderson
On Mon, Apr 27, 2015 at 04:53:29PM +0200, Michael Mueller wrote:
[...]
> #ifndef CONFIG_USER_ONLY
> +static CpuDefinitionInfoList *qmp_query_cpu_definition_host(void)
> +{
> + CpuDefinitionInfoList *host = NULL;
> + CpuDefinitionInfo *info;
> +
> + info = g_try_new0(CpuDefinitionInfo, 1);
> + if (!info) {
> + goto out;
> + }
> + info->name = g_strdup("host");
> +
> + host = g_try_new0(CpuDefinitionInfoList, 1);
> + if (!host) {
> + g_free(info->name);
> + g_free(info);
> + goto out;
> + }
> + host->value = info;
> +out:
> + return host;
> +}
[...]
> CpuDefinitionInfoList *arch_query_cpu_definitions(bool has_machine,
> const char *machine,
> bool has_accel,
> AccelId accel,
> Error **errp)
> {
> - CpuDefinitionInfoList *entry;
> - CpuDefinitionInfo *info;
> + S390MachineProps mach;
> + GSList *classes;
> + uint64_t *mask = NULL;
> + CpuDefinitionInfoList *list = NULL;
> +
> + if (has_machine) {
> + mask = s390_fac_list_mask_by_machine(machine);
> + if (!mask) {
> + error_set(errp, QERR_INVALID_PARAMETER_VALUE, "machine",
> + "a valid machine type");
> + return NULL;
> + }
> + }
I would like to understand better the meaning of "runnable" when machine
is omitted. Is it really possible to tell if a CPU model is runnable if
no machine/mask info is provided as input?
If machine is omitted and the command returns runnable=true, does that
mean the CPU model is runnable using any machine? Does it mean it is
runnable using some of the available machines? If so, which ones? Does
it mean something else?
>
> - info = g_malloc0(sizeof(*info));
> - info->name = g_strdup("host");
> + memset(&mach, 0, sizeof(mach));
> + if (has_accel) {
> + switch (accel) {
> + case ACCEL_ID_KVM:
> + kvm_s390_get_machine_props(NULL, &mach);
> + break;
> + default:
> + return qmp_query_cpu_definition_host();
This will return only a single element. I don't think that's correct. If
machine or accel is omitted, I believe we should just omit the
"runnable" field, but always return the full list of CPU models.
> + }
> + }
>
> - entry = g_malloc0(sizeof(*entry));
> - entry->value = info;
> + s390_setup_cpu_classes(ACCEL_TEMP, &mach, mask);
> +
> + classes = object_class_get_list(TYPE_S390_CPU, false);
> + classes = g_slist_sort(classes, s390_cpu_class_asc_order_compare);
> + g_slist_foreach(classes, qmp_query_cpu_definition_entry, &list);
> + g_slist_free(classes);
>
> - return entry;
> + return list;
> }
> #endif
>
[...]
--
Eduardo
^ permalink raw reply [flat|nested] 55+ messages in thread
* Re: [Qemu-devel] [PATCH v6 05/17] Add optional parameters to QMP command query-cpu-definitions
2015-04-27 14:53 ` [Qemu-devel] [PATCH v6 05/17] Add optional parameters to QMP command query-cpu-definitions Michael Mueller
@ 2015-05-06 12:42 ` Eduardo Habkost
2015-05-07 7:37 ` Michael Mueller
0 siblings, 1 reply; 55+ messages in thread
From: Eduardo Habkost @ 2015-05-06 12:42 UTC (permalink / raw)
To: Michael Mueller
Cc: Cornelia Huck, Gleb Natapov, qemu-devel, Alexander Graf,
Christian Borntraeger, Jason J. Herne, Daniel Hansel,
Paolo Bonzini, Andreas Faerber, Richard Henderson
On Mon, Apr 27, 2015 at 04:53:19PM +0200, Michael Mueller wrote:
[...]
> diff --git a/qapi-schema.json b/qapi-schema.json
> index 215a7bc..285b2d3 100644
> --- a/qapi-schema.json
> +++ b/qapi-schema.json
> @@ -2536,21 +2536,43 @@
> #
> # @name: the name of the CPU definition
> #
> +# @default: #optional true if cpu model is the default,
> +# omitted if false (since 2.4)
Maybe we should clarify that it is the default in the machine provided
as argument to query-cpu-definitions?
> +#
> +# @runnable: #optional true if cpu model is runnable,
> +# omitted if false (since 2.4)
Maybe we should clarify that it means the CPU model is runnable using
the machine+accel combination provided as arguments to
query-cpu-definitions?
(See also my question about the meaning of runnable when machine is
omitted, in my reply to patch 15/17).
> +#
> +# @live-migration-safe: #optional true if cpu model represents a
> +# cpu model that is safely migratable
> +# omitted if false (since 2.4)
> +#
> +# @order: #optional order criterion
> +#
> # Since: 1.2.0
> ##
> { 'type': 'CpuDefinitionInfo',
> - 'data': { 'name': 'str' } }
> + 'data': { 'name': 'str', '*is-default': 'bool', '*runnable': 'bool',
> + '*live-migration-safe': 'bool', '*order': 'int' } }
>
> ##
> # @query-cpu-definitions:
> #
> -# Return a list of supported virtual CPU definitions
> +# Return a list of supported virtual CPU definitions. In context with the
> +# optional parameters @machine and @accel the returned list contains
> +# also information if the respective cpu definition is runnable or the
> +# default to be used.
> +#
> +# @machine: #optional machine type (since 2.4)
> +#
> +# @accel: #optional accelerator id (since 2.4)
> #
> # Returns: a list of CpuDefInfo
> #
> # Since: 1.2.0
> ##
> -{ 'command': 'query-cpu-definitions', 'returns': ['CpuDefinitionInfo'] }
> +{ 'command': 'query-cpu-definitions',
> + 'data': { '*machine': 'str', '*accel': 'AccelId' },
> + 'returns': ['CpuDefinitionInfo'] }
>
--
Eduardo
^ permalink raw reply [flat|nested] 55+ messages in thread
* Re: [Qemu-devel] [PATCH v6 04/17] Extend HMP command info cpus to display accelerator id and model name
2015-05-06 10:38 ` Eduardo Habkost
@ 2015-05-06 12:59 ` Luiz Capitulino
2015-05-06 13:33 ` Eduardo Habkost
0 siblings, 1 reply; 55+ messages in thread
From: Luiz Capitulino @ 2015-05-06 12:59 UTC (permalink / raw)
To: Eduardo Habkost
Cc: Cornelia Huck, Michael Mueller, Gleb Natapov, Alexander Graf,
qemu-devel, Christian Borntraeger, Jason J. Herne, Daniel Hansel,
Paolo Bonzini, Andreas Faerber, Richard Henderson
On Wed, 6 May 2015 07:38:53 -0300
Eduardo Habkost <ehabkost@redhat.com> wrote:
> On Wed, May 06, 2015 at 09:32:58AM +0200, Michael Mueller wrote:
> > On Tue, 5 May 2015 10:14:32 -0300
> > Eduardo Habkost <ehabkost@redhat.com> wrote:
> >
> > > On Mon, Apr 27, 2015 at 04:53:18PM +0200, Michael Mueller wrote:
> > > > The HMP command info cpus now displays the CPU model name and the
> > > > backing accelerator if part of the CPUState.
> > > >
> > > > (qemu) info cpus
> > > > * CPU #0: (halted) model=2827-ga2 accel=kvm thread_id=1679
> > > >
> > > > Signed-off-by: Michael Mueller <mimu@linux.vnet.ibm.com>
> > > > Acked-by: Christian Borntraeger <borntraeger@de.ibm.com>
> > >
> > > Do we really need this? I mean: I expect the amount of CPU data we
> > > provide to QMP clients to grow a lot in the near future, but that
> > > doesn't mean HMP users need all that data to be printed by "info cpus".
> >
> > Where do you see the limit of what is worth to be shown an what not.
> > I personally use "info cpus" less then sporadic but already got a comment
> > internally on that information being worthwhile to be shown.
>
> I really don't know, but I think we shouldn't add stuff to HMP unless we
> have a good reason. For each new piece of data in HMP I would like to at
> least see the description of a real use case that justifies adding it to
> HMP and not just implementing a simple script on top of QMP.
>
> For accel info we already have "info kvm" that is not ideal but is
> enough for current use cases, isn't it? CPU model name information seems
> to be more useful, but if it is just for debugging, people can just run
> QMP query-cpus command.
>
> Luiz, what do you think?
I don't see a problem with that. HMP is a debugging interface anyways.
Actually, I think it's a good test-case for QMP having a high-level
in-tree client (vs. qmp-shell, which is too low-level).
If the problem is that a command is dumping too much information to
the point of hurting usability, we can split the command or add a '-a'
option or something like that.
>
> >
> > >
> > >
> > > > ---
> > > > hmp.c | 7 +++++++
> > > > 1 file changed, 7 insertions(+)
> > > >
> > > > diff --git a/hmp.c b/hmp.c
> > > > index f142d36..676d821 100644
> > > > --- a/hmp.c
> > > > +++ b/hmp.c
> > > > @@ -290,6 +290,13 @@ void hmp_info_cpus(Monitor *mon, const QDict *qdict)
> > > > monitor_printf(mon, " (halted)");
> > > > }
> > > >
> > > > + if (cpu->value->has_model) {
> > > > + monitor_printf(mon, " model=%s", cpu->value->model);
> > > > + }
> > > > + if (cpu->value->has_accel) {
> > > > + monitor_printf(mon, " accel=%s", AccelId_lookup[cpu->value->accel]);
> > > > + }
> > > > +
> > > > monitor_printf(mon, " thread_id=%" PRId64 "\n", cpu->value->thread_id);
> > > > }
> > > >
> > > > --
> > > > 1.8.3.1
> > > >
> > >
> >
>
^ permalink raw reply [flat|nested] 55+ messages in thread
* Re: [Qemu-devel] [PATCH v6 04/17] Extend HMP command info cpus to display accelerator id and model name
2015-05-06 12:59 ` Luiz Capitulino
@ 2015-05-06 13:33 ` Eduardo Habkost
2015-05-06 13:44 ` Michael Mueller
0 siblings, 1 reply; 55+ messages in thread
From: Eduardo Habkost @ 2015-05-06 13:33 UTC (permalink / raw)
To: Luiz Capitulino
Cc: Cornelia Huck, Michael Mueller, Gleb Natapov, Alexander Graf,
qemu-devel, Christian Borntraeger, Jason J. Herne, Daniel Hansel,
Paolo Bonzini, Andreas Faerber, Richard Henderson
On Wed, May 06, 2015 at 08:59:56AM -0400, Luiz Capitulino wrote:
> On Wed, 6 May 2015 07:38:53 -0300
> Eduardo Habkost <ehabkost@redhat.com> wrote:
>
> > On Wed, May 06, 2015 at 09:32:58AM +0200, Michael Mueller wrote:
> > > On Tue, 5 May 2015 10:14:32 -0300
> > > Eduardo Habkost <ehabkost@redhat.com> wrote:
> > >
> > > > On Mon, Apr 27, 2015 at 04:53:18PM +0200, Michael Mueller wrote:
> > > > > The HMP command info cpus now displays the CPU model name and the
> > > > > backing accelerator if part of the CPUState.
> > > > >
> > > > > (qemu) info cpus
> > > > > * CPU #0: (halted) model=2827-ga2 accel=kvm thread_id=1679
> > > > >
> > > > > Signed-off-by: Michael Mueller <mimu@linux.vnet.ibm.com>
> > > > > Acked-by: Christian Borntraeger <borntraeger@de.ibm.com>
> > > >
> > > > Do we really need this? I mean: I expect the amount of CPU data we
> > > > provide to QMP clients to grow a lot in the near future, but that
> > > > doesn't mean HMP users need all that data to be printed by "info cpus".
> > >
> > > Where do you see the limit of what is worth to be shown an what not.
> > > I personally use "info cpus" less then sporadic but already got a comment
> > > internally on that information being worthwhile to be shown.
> >
> > I really don't know, but I think we shouldn't add stuff to HMP unless we
> > have a good reason. For each new piece of data in HMP I would like to at
> > least see the description of a real use case that justifies adding it to
> > HMP and not just implementing a simple script on top of QMP.
> >
> > For accel info we already have "info kvm" that is not ideal but is
> > enough for current use cases, isn't it? CPU model name information seems
> > to be more useful, but if it is just for debugging, people can just run
> > QMP query-cpus command.
> >
> > Luiz, what do you think?
>
> I don't see a problem with that. HMP is a debugging interface anyways.
> Actually, I think it's a good test-case for QMP having a high-level
> in-tree client (vs. qmp-shell, which is too low-level).
>
> If the problem is that a command is dumping too much information to
> the point of hurting usability, we can split the command or add a '-a'
> option or something like that.
Thanks! If HMP is seen as a debugging interface, my main objections
aren't valid.
That said, I would prefer to keep the command output cleaner and add
only the "model" field, as people can use "info kvm" for the accel info
by now.
--
Eduardo
^ permalink raw reply [flat|nested] 55+ messages in thread
* Re: [Qemu-devel] [PATCH v6 04/17] Extend HMP command info cpus to display accelerator id and model name
2015-05-06 13:33 ` Eduardo Habkost
@ 2015-05-06 13:44 ` Michael Mueller
0 siblings, 0 replies; 55+ messages in thread
From: Michael Mueller @ 2015-05-06 13:44 UTC (permalink / raw)
To: Eduardo Habkost
Cc: Cornelia Huck, Gleb Natapov, Alexander Graf, qemu-devel,
Christian Borntraeger, Jason J. Herne, Daniel Hansel,
Paolo Bonzini, Luiz Capitulino, Andreas Faerber,
Richard Henderson
On Wed, 6 May 2015 10:33:55 -0300
Eduardo Habkost <ehabkost@redhat.com> wrote:
> On Wed, May 06, 2015 at 08:59:56AM -0400, Luiz Capitulino wrote:
> > On Wed, 6 May 2015 07:38:53 -0300
> > Eduardo Habkost <ehabkost@redhat.com> wrote:
> >
> > > On Wed, May 06, 2015 at 09:32:58AM +0200, Michael Mueller wrote:
> > > > On Tue, 5 May 2015 10:14:32 -0300
> > > > Eduardo Habkost <ehabkost@redhat.com> wrote:
> > > >
> > > > > On Mon, Apr 27, 2015 at 04:53:18PM +0200, Michael Mueller wrote:
> > > > > > The HMP command info cpus now displays the CPU model name and the
> > > > > > backing accelerator if part of the CPUState.
> > > > > >
> > > > > > (qemu) info cpus
> > > > > > * CPU #0: (halted) model=2827-ga2 accel=kvm thread_id=1679
> > > > > >
> > > > > > Signed-off-by: Michael Mueller <mimu@linux.vnet.ibm.com>
> > > > > > Acked-by: Christian Borntraeger <borntraeger@de.ibm.com>
> > > > >
> > > > > Do we really need this? I mean: I expect the amount of CPU data we
> > > > > provide to QMP clients to grow a lot in the near future, but that
> > > > > doesn't mean HMP users need all that data to be printed by "info cpus".
> > > >
> > > > Where do you see the limit of what is worth to be shown an what not.
> > > > I personally use "info cpus" less then sporadic but already got a comment
> > > > internally on that information being worthwhile to be shown.
> > >
> > > I really don't know, but I think we shouldn't add stuff to HMP unless we
> > > have a good reason. For each new piece of data in HMP I would like to at
> > > least see the description of a real use case that justifies adding it to
> > > HMP and not just implementing a simple script on top of QMP.
> > >
> > > For accel info we already have "info kvm" that is not ideal but is
> > > enough for current use cases, isn't it? CPU model name information seems
> > > to be more useful, but if it is just for debugging, people can just run
> > > QMP query-cpus command.
> > >
> > > Luiz, what do you think?
> >
> > I don't see a problem with that. HMP is a debugging interface anyways.
> > Actually, I think it's a good test-case for QMP having a high-level
> > in-tree client (vs. qmp-shell, which is too low-level).
> >
> > If the problem is that a command is dumping too much information to
> > the point of hurting usability, we can split the command or add a '-a'
> > option or something like that.
>
> Thanks! If HMP is seen as a debugging interface, my main objections
> aren't valid.
>
> That said, I would prefer to keep the command output cleaner and add
> only the "model" field, as people can use "info kvm" for the accel info
> by now.
Ok, I buy that and will kick the accel name out again.
Michael
>
^ permalink raw reply [flat|nested] 55+ messages in thread
* Re: [Qemu-devel] [PATCH v6 15/17] target-s390x: Extend arch specific QMP command query-cpu-definitions
2015-05-06 12:37 ` Eduardo Habkost
@ 2015-05-06 14:48 ` Michael Mueller
2015-05-11 16:59 ` Eduardo Habkost
0 siblings, 1 reply; 55+ messages in thread
From: Michael Mueller @ 2015-05-06 14:48 UTC (permalink / raw)
To: Eduardo Habkost
Cc: Cornelia Huck, Gleb Natapov, qemu-devel, Alexander Graf,
Christian Borntraeger, Jason J. Herne, Daniel Hansel,
Paolo Bonzini, Andreas Faerber, Richard Henderson
On Wed, 6 May 2015 09:37:41 -0300
Eduardo Habkost <ehabkost@redhat.com> wrote:
> On Mon, Apr 27, 2015 at 04:53:29PM +0200, Michael Mueller wrote:
> [...]
> > #ifndef CONFIG_USER_ONLY
> > +static CpuDefinitionInfoList *qmp_query_cpu_definition_host(void)
> > +{
> > + CpuDefinitionInfoList *host = NULL;
> > + CpuDefinitionInfo *info;
> > +
> > + info = g_try_new0(CpuDefinitionInfo, 1);
> > + if (!info) {
> > + goto out;
> > + }
> > + info->name = g_strdup("host");
> > +
> > + host = g_try_new0(CpuDefinitionInfoList, 1);
> > + if (!host) {
> > + g_free(info->name);
> > + g_free(info);
> > + goto out;
> > + }
> > + host->value = info;
> > +out:
> > + return host;
> > +}
> [...]
> > CpuDefinitionInfoList *arch_query_cpu_definitions(bool has_machine,
> > const char *machine,
> > bool has_accel,
> > AccelId accel,
> > Error **errp)
> > {
> > - CpuDefinitionInfoList *entry;
> > - CpuDefinitionInfo *info;
> > + S390MachineProps mach;
> > + GSList *classes;
> > + uint64_t *mask = NULL;
> > + CpuDefinitionInfoList *list = NULL;
> > +
> > + if (has_machine) {
> > + mask = s390_fac_list_mask_by_machine(machine);
> > + if (!mask) {
> > + error_set(errp, QERR_INVALID_PARAMETER_VALUE, "machine",
> > + "a valid machine type");
> > + return NULL;
> > + }
> > + }
>
> I would like to understand better the meaning of "runnable" when machine
> is omitted. Is it really possible to tell if a CPU model is runnable if
> no machine/mask info is provided as input?
Yes it is. The list of runnable CPU models is derived from the set of S390 CPU
facilities available. One subset of facilities depends on the accelerator and
the second on the capabilities of QEMU which is represented by the mask. In the
case the no machine is specified only those of the accelerator are taken into
account for the calculation.
It is even possible to omit both, the accelerator and the machine, in that case
only the model none is runnable and becomes the default:
{"return":[{"order":3345,"name":"2964-ga1","live-migration-safe":true},{"name":"zBC12"},
{"name":"2828"},{"order":3105,"name":"2828-ga1","live-migration-safe":true},{"name":"host"},
{"name":"zEC12"},{"name":"2827"},{"order":3090,"name":"2827-ga2","live-migration-safe":true},
{"order":3089,"name":"2827-ga1","live-migration-safe":true},{"name":"z114"},{"name":"2818"},
{"order":2849,"name":"2818-ga1","live-migration-safe":true},{"name":"z196"},{"name":"2817"},
{"order":2834,"name":"2817-ga2","live-migration-safe":true},{"order":2833,"name":"2817-ga1",
"live-migration-safe":true},{"name":"z10-bc"},{"name":"2098"},{"order":2594,"name":"2098-ga2",
"live-migration-safe":true},{"order":2593,"name":"2098-ga1","live-migration-safe":true},
{"name":"z10-ec"},{"name":"z10"},{"name":"2097"},{"order":2579,"name":"2097-ga3",
"live-migration-safe":true},{"order":2578,"name":"2097-ga2","live-migration-safe":true},
{"order":2577,"name":"2097-ga1","live-migration-safe":true},{"name":"z9-bc"},{"name":"2096"},
{"order":2338,"name":"2096-ga2","live-migration-safe":true},{"order":2337,"name":"2096-ga1",
"live-migration-safe":true},{"name":"z9-ec"},{"name":"z9"},{"name":"2094"},{"order":2323,
"name":"2094-ga3","live-migration-safe":true},{"order":2322,"name":"2094-ga2",
"live-migration-safe":true},{"name":"z9-109"},{"order":2321,"name":"2094-ga1",
"live-migration-safe":true},{"name":"z890"},{"name":"2086"},{"order":2083,"name":"2086-ga3",
"live-migration-safe":true},{"order":2082,"name":"2086-ga2","live-migration-safe":true},
{"order":2081,"name":"2086-ga1","live-migration-safe":true},{"name":"z990"},{"name":"2084"},
{"order":2069,"name":"2084-ga5","live-migration-safe":true},{"order":2068,"name":"2084-ga4",
"live-migration-safe":true},{"order":2067,"name":"2084-ga3","live-migration-safe":true},
{"order":2066,"name":"2084-ga2","live-migration-safe":true},{"order":2065,"name":"2084-ga1",
"live-migration-safe":true},{"name":"z800"},{"name":"2066"},{"order":1825,"name":"2066-ga1",
"live-migration-safe":true},{"name":"z900"},{"name":"2064"},{"order":1811,"name":"2064-ga3",
"live-migration-safe":true},{"order":1810,"name":"2064-ga2","live-migration-safe":true},
{"order":1809,"name":"2064-ga1","live-migration-safe":true},{"name":"none","runnable":true,
"is-default":true}],"id":"libvirt-15"}
>
> If machine is omitted and the command returns runnable=true, does that
> mean the CPU model is runnable using any machine? Does it mean it is
> runnable using some of the available machines? If so, which ones? Does
> it mean something else?
As it is a lower limit all machines shall be able to run it. Although a somewhat
clever management interface should iterate through all accel/machine combinations.
>
> >
> > - info = g_malloc0(sizeof(*info));
> > - info->name = g_strdup("host");
> > + memset(&mach, 0, sizeof(mach));
> > + if (has_accel) {
> > + switch (accel) {
> > + case ACCEL_ID_KVM:
> > + kvm_s390_get_machine_props(NULL, &mach);
> > + break;
> > + default:
> > + return qmp_query_cpu_definition_host();
>
> This will return only a single element. I don't think that's correct. If
> machine or accel is omitted, I believe we should just omit the
> "runnable" field, but always return the full list of CPU models.
That is the !KVM case where I keep the behavior currently unchanged to
the existing implementation. But right for the TCG case there has to be
a comparable get_machine_props() call returning whatever TCG is implementing.
But that is not part of this patch series.
>
> > + }
> > + }
> >
> > - entry = g_malloc0(sizeof(*entry));
> > - entry->value = info;
> > + s390_setup_cpu_classes(ACCEL_TEMP, &mach, mask);
> > +
> > + classes = object_class_get_list(TYPE_S390_CPU, false);
> > + classes = g_slist_sort(classes, s390_cpu_class_asc_order_compare);
> > + g_slist_foreach(classes, qmp_query_cpu_definition_entry, &list);
> > + g_slist_free(classes);
> >
> > - return entry;
> > + return list;
> > }
> > #endif
> >
> [...]
>
^ permalink raw reply [flat|nested] 55+ messages in thread
* Re: [Qemu-devel] [PATCH v6 15/17] target-s390x: Extend arch specific QMP command query-cpu-definitions
2015-05-05 18:40 ` Eduardo Habkost
@ 2015-05-06 15:31 ` Michael Mueller
2015-05-06 16:00 ` Eduardo Habkost
0 siblings, 1 reply; 55+ messages in thread
From: Michael Mueller @ 2015-05-06 15:31 UTC (permalink / raw)
To: Eduardo Habkost
Cc: Cornelia Huck, Gleb Natapov, qemu-devel, Alexander Graf,
Christian Borntraeger, Jason J. Herne, Daniel Hansel,
Paolo Bonzini, Andreas Faerber, Richard Henderson
On Tue, 5 May 2015 15:40:34 -0300
Eduardo Habkost <ehabkost@redhat.com> wrote:
> On Mon, Apr 27, 2015 at 04:53:29PM +0200, Michael Mueller wrote:
> [...]
> > diff --git a/target-s390x/kvm.c b/target-s390x/kvm.c
> > index 4d75ff0..94fede5 100644
> > --- a/target-s390x/kvm.c
> > +++ b/target-s390x/kvm.c
> > @@ -276,12 +276,59 @@ static int cpu_model_set(KVMState *s, uint64_t attr, void *addr)
> > return rc;
> > }
> >
> > -static int kvm_s390_get_machine_props(KVMState *s, S390MachineProps *prop)
>
> This seems to duplicate lots of the existing KVM code. (See additional
> comment below about possible ways to avoid it).
some :-)
>
> > +static int get_machine_props_fallback(S390MachineProps *prop)
> > +{
> > + struct kvm_device_attr dev_attr;
> > + int rc, kvmfd = -1, vmfd = -1;
> > +
> > + rc = qemu_open("/dev/kvm", O_RDWR);
> > + if (rc < 0) {
> > + goto out_err;
> > + }
> > + kvmfd = rc;
> > +
> > + rc = ioctl(kvmfd, KVM_CREATE_VM, 0);
> > + if (rc < 0) {
> > + goto out_err;
> > + }
> > + vmfd = rc;
> > +
> > + rc = ioctl(vmfd, KVM_CHECK_EXTENSION, KVM_CAP_VM_ATTRIBUTES);
> > + if (rc < 0) {
> > + rc = -ENOSYS;
> > + goto out_err;
> > + }
> > +
> > + dev_attr.group = KVM_S390_VM_CPU_MODEL;
> > + dev_attr.attr = KVM_S390_VM_CPU_MACHINE;
> > + rc = ioctl(vmfd, KVM_HAS_DEVICE_ATTR, &dev_attr);
> > + if (rc < 0) {
> > + rc = -EFAULT;
> > + goto out_err;
> > + }
> > +
> > + dev_attr.addr = (uint64_t) prop;
> > + rc = ioctl(vmfd, KVM_GET_DEVICE_ATTR, &dev_attr);
> > +
> > +out_err:
> > + if (vmfd >= 0) {
> > + close(vmfd);
> > + }
> > + if (kvmfd >= 0) {
> > + close(kvmfd);
> > + }
> > +
> > + return rc;
> > +}
> > +
> > +int kvm_s390_get_machine_props(KVMState *s, S390MachineProps *prop)
> > {
> > int rc = -EFAULT;
> >
> > if (s) {
> > rc = cpu_model_get(s, KVM_S390_VM_CPU_MACHINE, prop);
> > + } else {
> > + rc = get_machine_props_fallback(prop);
> > }
>
> The comments below are just suggestions, not something which should
> block the patch, in my opinion:
>
> First, if s is always NULL inside arch_query_cpu_definitions(), and is
> always non-NULL inside kvm_setup_cpu_classes(), why don't you just call
> keep the original kvm_s390_get_machine_props() function, and call and
> get_machine_props_fallback() inside arch_query_cpu_definitions()?
My reason for pulling both paths through the same internal interface call is
to have just single call for the same purpose.
>
> The only thing common to both cases is the tracing point, but if we are
> running two completely different code paths I assume it would be a good
> thing to have a different tracing point for
> get_machine_props_fallback().
>
>
> Second, you shouldn't even need to duplicate code in
> get_machine_props_fallback() if you are able to create an accel object
> and do just basic initialization so that cpu_model_get() works.
> Allowing accel objects to be created on the fly was one of the main
> purposes of the accel QOM work.
>
> For example, if we do something like this:
> https://github.com/ehabkost/qemu-hacks/commit/36a250e34c5fd0d43a25271f5bc9b04681fdd56a [1]
> https://github.com/ehabkost/qemu-hacks/commits/work/accel-open-func
I had a look at your qemu-hacks before writing the _fallback() routine
but did not wanted to base on some not yet published code. Once your part goes
upstream my intend is to provide a cleanup patch... And I was missing the
KVM_CREATE_VM actually.
>
> Then the code could look like this:
>
> accel.c:
>
> /* configure_accelerator() would be changed to reuse this function: */
> AccelState *accel_create(const char *accel_name)
> {
> AccelClass *acc = accel_find(accel_name);
> /*TODO: error handling, checking acc->available() */
> return ACCEL(object_new(object_class_get_name(OBJECT_CLASS(acc))));
> }
>
> /* Do basic accel initialization without affecting global QEMU state */
> /* accel_init_machine() would be changed to reuse this function: */
> void accel_open(AccelState *s, Error **errp)
> {
> object_property_set_bool(OBJECT(s), true, "open", errp);
> }
>
> target-s390/kvm.c:
>
> /* Using a different function name would be interesting, as it would be
> * the main arch_query_cpu_definitions() code path, not a fallback.
> */
> int get_machine_props_fallback(S390MachineProps *prop)
> {
> int r;
> AccelState *ac = accel_create("kvm");
> /*TODO: error handling */
> accel_open(ac, &err);
> r = cpu_model_get(ac, prop);
> object_unref(OBJECT(ac));
> return r;
> }
>
>
> [1] I only moved the /dev/kvm opening to the open method, but maybe the
> whole code up to KVM_CREATE_VM and capabitlity checking could be
> moved.
Yes as mentioned above.
> (But I don't know how to handle kvm_type, as it is currently
> provided by MachineClass. Maybe kvm_type() belongs to CPUClass
> instead of MachineClass?)
>
> > trace_kvm_get_machine_props(rc, prop->cpuid, prop->ibc);
> >
> > --
> > 1.8.3.1
> >
>
^ permalink raw reply [flat|nested] 55+ messages in thread
* Re: [Qemu-devel] [PATCH v6 15/17] target-s390x: Extend arch specific QMP command query-cpu-definitions
2015-05-06 15:31 ` Michael Mueller
@ 2015-05-06 16:00 ` Eduardo Habkost
2015-05-06 16:27 ` Michael Mueller
0 siblings, 1 reply; 55+ messages in thread
From: Eduardo Habkost @ 2015-05-06 16:00 UTC (permalink / raw)
To: Michael Mueller
Cc: Cornelia Huck, Gleb Natapov, qemu-devel, Alexander Graf,
Christian Borntraeger, Jason J. Herne, Daniel Hansel,
Paolo Bonzini, Andreas Faerber, Richard Henderson
On Wed, May 06, 2015 at 05:31:06PM +0200, Michael Mueller wrote:
> On Tue, 5 May 2015 15:40:34 -0300
> Eduardo Habkost <ehabkost@redhat.com> wrote:
[...]
> > Second, you shouldn't even need to duplicate code in
> > get_machine_props_fallback() if you are able to create an accel object
> > and do just basic initialization so that cpu_model_get() works.
> > Allowing accel objects to be created on the fly was one of the main
> > purposes of the accel QOM work.
> >
> > For example, if we do something like this:
> > https://github.com/ehabkost/qemu-hacks/commit/36a250e34c5fd0d43a25271f5bc9b04681fdd56a [1]
> > https://github.com/ehabkost/qemu-hacks/commits/work/accel-open-func
>
> I had a look at your qemu-hacks before writing the _fallback() routine
> but did not wanted to base on some not yet published code. Once your part goes
> upstream my intend is to provide a cleanup patch... And I was missing the
> KVM_CREATE_VM actually.
No problem to me if you prefer to keep it this way and change it to use
AccelState as a follow-up, after appropriate methods are provided by
TYPE_KVM_ACCEL. Your patch was helpful to show that just extracting the
open("/dev/kvm") part isn't enough and more work is needed.
--
Eduardo
^ permalink raw reply [flat|nested] 55+ messages in thread
* Re: [Qemu-devel] [PATCH v6 01/17] Introduce stub routine cpu_desc_avail
2015-05-06 11:23 ` Eduardo Habkost
@ 2015-05-06 16:23 ` Michael Mueller
2015-05-06 17:06 ` Eduardo Habkost
0 siblings, 1 reply; 55+ messages in thread
From: Michael Mueller @ 2015-05-06 16:23 UTC (permalink / raw)
To: Eduardo Habkost
Cc: Cornelia Huck, Gleb Natapov, qemu-devel, Alexander Graf,
Christian Borntraeger, Jason J. Herne, Daniel Hansel,
Paolo Bonzini, Andreas Faerber, Richard Henderson
On Wed, 6 May 2015 08:23:32 -0300
Eduardo Habkost <ehabkost@redhat.com> wrote:
> On Wed, May 06, 2015 at 11:17:20AM +0200, Michael Mueller wrote:
> > On Tue, 5 May 2015 14:41:01 -0300
> > Eduardo Habkost <ehabkost@redhat.com> wrote:
> >
> > > On Tue, May 05, 2015 at 06:12:16PM +0200, Michael Mueller wrote:
> > > > On Tue, 5 May 2015 10:55:47 -0300
> > > > Eduardo Habkost <ehabkost@redhat.com> wrote:
> > > >
> > > > > On Mon, Apr 27, 2015 at 04:53:15PM +0200, Michael Mueller wrote:
> > > > > > This patch introduces the function cpu_desc_avail() which returns by
> > > > > > default true if not architecture specific implemented. Its intention
> > > > > > is to indicate if the cpu model description is available for display
> > > > > > by list_cpus(). This change allows cpu model descriptions to become
> > > > > > dynamically created by evaluating the runtime context instead of
> > > > > > putting static cpu model information at display.
> > > > >
> > > > > Why are you deliberately breaking "-cpu ?" when cpu_desc_avail() is
> > > > > false?
> > > >
> > > > In the s390x case cpu_desc_avail() is per se false in this code section of vl.c:
> > > >
> > > > /* Init CPU def lists, based on config
> > > > * - Must be called after all the qemu_read_config_file() calls
> > > > * - Must be called before list_cpu()
> > > > * - Must be called before machine->init()
> > > > */
> > >
> > > (Side note: I believe the above outdated, I will send a patch to update
> > > it.)
> >
> > Will be interesting to see what the change is, master is currently showing this code.
>
> We don't (and shouldn't) have the qemu_read_config_file() requirement,
> as CPU models are not loaded from config files anymore.
>
> And we should be able to eliminate cpudef_init() completely, soon. I
> think the only user of cpudef_init() is x86.
right.
>
> >
> > >
> > > > cpudef_init();
> > > >
> > > > if (cpu_model && cpu_desc_avail() && is_help_option(cpu_model)) {
> > > > list_cpus(stdout, &fprintf, cpu_model);
> > > > exit(0);
> > > > }
> > > >
> > > > That is because the output does not solely depend on static definitions
> > > > but also on runtime context. Here the host machine type this instance of
> > > > QEMU is running on, at least for the KVM case.
> > >
> > > Is this a required feature? I would prefer to have the main() code
> > > simple even if it means not having runnable information in "-cpu ?" by
> > > now (about possible ways to implement this without cpu_desc_avail(), see
> > > below).
> >
> > I think it is more than a desired feature because one might end up with a failed
> > CPU object instantiation although the help screen claims to CPU model to be valid.
>
> I think you are more likely to confuse users by not showing information
> on "-cpu ?" when -machine is not present. I believe most people use
> "-cpu ?" with no other arguments, to see what the QEMU binary is capable
> of.
I don't disagree with that, both cases are to some extend confusing...
But the accelerator makes a big difference and a tended user should really be aware
of that.
Also that TCG is the default:
$ ./s390x-softmmu/qemu-system-s390x -cpu ?
s390 host
And I don't see a way to make a user belief that all the defined CPU models are available to a
TCG user in the S390 case where most of the CPU facilities are not implemented.
>
> Anyway, whatever we decide to do, I believe we should start with
> something simple to get things working, and after that we can look for
> ways improve the help output with "runnable" info.
I don't see how to solve this without cpu_desc_avail() or some other comparable mechanism, the
aliases e.g. are also dynamic...
>
> >
> > >
> > >
> > > >
> > > > Once the accelerator has been initialized AND the S390 cpu classes have
> > > > been setup by means of the following code:
> > > >
> > > > static void kvm_setup_cpu_classes(KVMState *s)
> > > > {
> > > > S390MachineProps mach;
> > > >
> > > > if (!kvm_s390_get_machine_props(s, &mach)) {
> > > > s390_setup_cpu_classes(ACCEL_CURRENT, &mach,
> > > > s390_current_fac_list_mask());
> > > > s390_setup_cpu_aliases();
> > > > cpu_classes_initialized = true;
> > > > }
> > > > }
> > > >
> > > > cpu_desc_avail() becomes true. In case the selceted mode was "?"
> > > > the list_cpu() is now done right before the cpu model is used as part
> > > > of the cpu initialization (hw/s390-virtio.c):
> > > >
> > > > void s390_init_cpus(const char *cpu_model, uint8_t *storage_keys)
> > > > {
> > > > int i;
> > > >
> > > > if (cpu_model == NULL) {
> > > > cpu_model = "none";
> > > > }
> > > >
> > > > if (is_help_option(cpu_model)) {
> > > > list_cpus(stdout, &fprintf, cpu_model);
> > > > exit(0);
> > > > }
> > > >
> > > > ...
> > > > for (i = 0; i < smp_cpus; i++) {
> > > > ...
> > > > cpu = cpu_s390x_init(cpu_model);
> > > > ...
> > > > }
> > > > }
> > >
> > >
> > > In other words, you just need to ensure that s390_cpu_list() run after
> > > kvm_setup_cpu_classes().
> >
> > ... which is part of the KVM/accel init process but it could of course make use
> > of the ACCEL_TMP use case as query-cpu-definitions does.
>
> Exactly. I don't see a reason to not share code between
> query-cpu-definitions and "-cpu ?".
>
> >
> > >
> > > Can't you simply call s390_setup_cpu_classes(ACCEL_TEMP) inside
> > > s390_init_cpus(), just like arch_query_cpu_definitions()? You could even
> > > share code between both functions.
> >
> > That would not help with the current placement of list_cpus() in main() as it happens
> > way to early. Not s390_init_cpus() is the issue, the context information has been
> > processed already at that time. Currently I just kind of delay the list_cpus() until
> > all required information is available.
>
> I understand you are just delaying it. But it is requiring a hack inside
> generic main() code that could be avoided. About the actual reasons to
> delay it, see below.
>
> >
> > >
> > > (In the future, we should be able to implement "-cpu ?" by simply
> > > calling the query-cpu-definitions implementation.)
> >
> > Right but the -machine <name>,accel=<accel> options have to be processed already.
>
> About query-cpu-definitions: its code shouldn't depend on -machine
> options to work. Maybe it can use current_machine to get the default
> accel and machine argument, but it should always output the same data
> for a machine+accel combination even if you use "-machine none".
No it should not but it takes the same parameters.
>
> About "-cpu ?": do we really want it to depend on -machine processing?
> Today, help output shows what the QEMU binary is capable of, not just
> what the host system and -machine option are capable of.
I think we have to take it into account because the available CPU models might
deviate substantially as in the case for S390 for KVM and TCG.
>
> If we decide to change that assumption, let's do it in a generic way and
> not as a arch-specific hack. The options I see are:
welcome
>
> 1) Continue with the current policy where "-cpu ?" does not depend on
> -machine arguments, and show all CPU models on "-cpu ?".
> 2) Deciding that, yes, it is OK to make "-cpu ?" depend on -machine
> arguments, and move the list_cpus() call after machine initialization
> inside generic main() code for all arches.
> 2.1) We could delay the list_cpus() call inside main() on all cases.
> 2.2) We could delay the list_cpus() call inside main() only if
> an explicit -machine option is present.
>
> I prefer (1) and my second choice would be (2.2), but the main point is
> that none of the options above require making s390 special and
> introducing cpu_desc_avail().
My take here is 2.1 because omitting option -machine is a decision to some
defaults for machine type and accelerator type already.
>
^ permalink raw reply [flat|nested] 55+ messages in thread
* Re: [Qemu-devel] [PATCH v6 15/17] target-s390x: Extend arch specific QMP command query-cpu-definitions
2015-05-06 16:00 ` Eduardo Habkost
@ 2015-05-06 16:27 ` Michael Mueller
0 siblings, 0 replies; 55+ messages in thread
From: Michael Mueller @ 2015-05-06 16:27 UTC (permalink / raw)
To: Eduardo Habkost
Cc: Gleb Natapov, qemu-devel, Alexander Graf, Christian Borntraeger,
Daniel Hansel, Jason J. Herne, Cornelia Huck, Paolo Bonzini,
Andreas Faerber, Richard Henderson
On Wed, 6 May 2015 13:00:32 -0300
Eduardo Habkost <ehabkost@redhat.com> wrote:
> On Wed, May 06, 2015 at 05:31:06PM +0200, Michael Mueller wrote:
> > On Tue, 5 May 2015 15:40:34 -0300
> > Eduardo Habkost <ehabkost@redhat.com> wrote:
> [...]
> > > Second, you shouldn't even need to duplicate code in
> > > get_machine_props_fallback() if you are able to create an accel object
> > > and do just basic initialization so that cpu_model_get() works.
> > > Allowing accel objects to be created on the fly was one of the main
> > > purposes of the accel QOM work.
> > >
> > > For example, if we do something like this:
> > > https://github.com/ehabkost/qemu-hacks/commit/36a250e34c5fd0d43a25271f5bc9b04681fdd56a [1]
> > > https://github.com/ehabkost/qemu-hacks/commits/work/accel-open-func
> >
> > I had a look at your qemu-hacks before writing the _fallback() routine
> > but did not wanted to base on some not yet published code. Once your part goes
> > upstream my intend is to provide a cleanup patch... And I was missing the
> > KVM_CREATE_VM actually.
>
> No problem to me if you prefer to keep it this way and change it to use
> AccelState as a follow-up, after appropriate methods are provided by
> TYPE_KVM_ACCEL. Your patch was helpful to show that just extracting the
> open("/dev/kvm") part isn't enough and more work is needed.
promised :-)
>
^ permalink raw reply [flat|nested] 55+ messages in thread
* Re: [Qemu-devel] [PATCH v6 01/17] Introduce stub routine cpu_desc_avail
2015-05-06 16:23 ` Michael Mueller
@ 2015-05-06 17:06 ` Eduardo Habkost
2015-05-07 7:35 ` Michael Mueller
0 siblings, 1 reply; 55+ messages in thread
From: Eduardo Habkost @ 2015-05-06 17:06 UTC (permalink / raw)
To: Michael Mueller
Cc: Cornelia Huck, Gleb Natapov, qemu-devel, Alexander Graf,
Christian Borntraeger, Jason J. Herne, Daniel Hansel,
Paolo Bonzini, Andreas Faerber, Richard Henderson
On Wed, May 06, 2015 at 06:23:05PM +0200, Michael Mueller wrote:
> On Wed, 6 May 2015 08:23:32 -0300
> Eduardo Habkost <ehabkost@redhat.com> wrote:
[...]
> > > > > cpudef_init();
> > > > >
> > > > > if (cpu_model && cpu_desc_avail() && is_help_option(cpu_model)) {
> > > > > list_cpus(stdout, &fprintf, cpu_model);
> > > > > exit(0);
> > > > > }
> > > > >
> > > > > That is because the output does not solely depend on static definitions
> > > > > but also on runtime context. Here the host machine type this instance of
> > > > > QEMU is running on, at least for the KVM case.
> > > >
> > > > Is this a required feature? I would prefer to have the main() code
> > > > simple even if it means not having runnable information in "-cpu ?" by
> > > > now (about possible ways to implement this without cpu_desc_avail(), see
> > > > below).
> > >
> > > I think it is more than a desired feature because one might end up with a failed
> > > CPU object instantiation although the help screen claims to CPU model to be valid.
> >
> > I think you are more likely to confuse users by not showing information
> > on "-cpu ?" when -machine is not present. I believe most people use
> > "-cpu ?" with no other arguments, to see what the QEMU binary is capable
> > of.
>
> I don't disagree with that, both cases are to some extend confusing...
> But the accelerator makes a big difference and a tended user should really be aware
> of that.
>
> Also that TCG is the default:
>
> $ ./s390x-softmmu/qemu-system-s390x -cpu ?
> s390 host
>
> And I don't see a way to make a user belief that all the defined CPU models are available to a
> TCG user in the S390 case where most of the CPU facilities are not implemented.
Well, we could simply add a "KVM required" note (maybe just an asterisk beside
the CPU model description). But maybe we have a reasonable alternative below:
>
> >
> > Anyway, whatever we decide to do, I believe we should start with
> > something simple to get things working, and after that we can look for
> > ways improve the help output with "runnable" info.
>
> I don't see how to solve this without cpu_desc_avail() or some other comparable mechanism, the
> aliases e.g. are also dynamic...
What bothers me in cpu_desc_avail() is that it depends on global state that is
non-trivial (one needs to follow the whole KVM initialization path to find out
if cpu_desc_avail() will be true or false).
We could instead simply skip the cpu_list() call unconditionally on s390. e.g.:
target-s390x/cpu.h:
/* Delete the existing cpu_list macro */
cpus.c:
int list_cpus(FILE *f, fprintf_function cpu_fprintf, const char *optarg)
{
#if defined(cpu_list)
cpu_list(f, cpu_fprintf);
return 1;
#else
return 0;
#endif
}
vl.c:
if (cpu_model && is_help_option(cpu_model)) {
/* zero list_cpus() return value means "-cpu ?" will be
* handled later by machine initialization code */
if (list_cpus(stdout, &fprintf, cpu_model)) {
exit(0);
}
}
[...]
> >
> > About "-cpu ?": do we really want it to depend on -machine processing?
> > Today, help output shows what the QEMU binary is capable of, not just
> > what the host system and -machine option are capable of.
>
> I think we have to take it into account because the available CPU models might
> deviate substantially as in the case for S390 for KVM and TCG.
That's true, on s390 the set of available CPU models is very different on both
cases. It breaks assumptions in the existing "-cpu ?" handling code in main().
>
> >
> > If we decide to change that assumption, let's do it in a generic way and
> > not as a arch-specific hack. The options I see are:
>
> welcome
>
> >
> > 1) Continue with the current policy where "-cpu ?" does not depend on
> > -machine arguments, and show all CPU models on "-cpu ?".
> > 2) Deciding that, yes, it is OK to make "-cpu ?" depend on -machine
> > arguments, and move the list_cpus() call after machine initialization
> > inside generic main() code for all arches.
> > 2.1) We could delay the list_cpus() call inside main() on all cases.
> > 2.2) We could delay the list_cpus() call inside main() only if
> > an explicit -machine option is present.
> >
> > I prefer (1) and my second choice would be (2.2), but the main point is
> > that none of the options above require making s390 special and
> > introducing cpu_desc_avail().
>
> My take here is 2.1 because omitting option -machine is a decision to some
> defaults for machine type and accelerator type already.
The problem with 2.1 is that some machine init functions require that
additional command-line parameters are set and will abort (e.g. mips machines).
So we can't do that unconditionally for all architectures.
The proposal above is like 2.1, but conditional: it will delay "-cpu ?"
handling only on architectures that don't define cpu_list().
--
Eduardo
^ permalink raw reply [flat|nested] 55+ messages in thread
* Re: [Qemu-devel] [PATCH v6 01/17] Introduce stub routine cpu_desc_avail
2015-05-06 17:06 ` Eduardo Habkost
@ 2015-05-07 7:35 ` Michael Mueller
0 siblings, 0 replies; 55+ messages in thread
From: Michael Mueller @ 2015-05-07 7:35 UTC (permalink / raw)
To: Eduardo Habkost
Cc: Cornelia Huck, Gleb Natapov, qemu-devel, Alexander Graf,
Christian Borntraeger, Jason J. Herne, Daniel Hansel,
Paolo Bonzini, Andreas Faerber, Richard Henderson
On Wed, 6 May 2015 14:06:16 -0300
Eduardo Habkost <ehabkost@redhat.com> wrote:
> On Wed, May 06, 2015 at 06:23:05PM +0200, Michael Mueller wrote:
> > On Wed, 6 May 2015 08:23:32 -0300
> > Eduardo Habkost <ehabkost@redhat.com> wrote:
> [...]
> > > > > > cpudef_init();
> > > > > >
> > > > > > if (cpu_model && cpu_desc_avail() && is_help_option(cpu_model)) {
> > > > > > list_cpus(stdout, &fprintf, cpu_model);
> > > > > > exit(0);
> > > > > > }
> > > > > >
> > > > > > That is because the output does not solely depend on static definitions
> > > > > > but also on runtime context. Here the host machine type this instance of
> > > > > > QEMU is running on, at least for the KVM case.
> > > > >
> > > > > Is this a required feature? I would prefer to have the main() code
> > > > > simple even if it means not having runnable information in "-cpu ?" by
> > > > > now (about possible ways to implement this without cpu_desc_avail(), see
> > > > > below).
> > > >
> > > > I think it is more than a desired feature because one might end up with a failed
> > > > CPU object instantiation although the help screen claims to CPU model to be valid.
> > >
> > > I think you are more likely to confuse users by not showing information
> > > on "-cpu ?" when -machine is not present. I believe most people use
> > > "-cpu ?" with no other arguments, to see what the QEMU binary is capable
> > > of.
> >
> > I don't disagree with that, both cases are to some extend confusing...
> > But the accelerator makes a big difference and a tended user should really be aware
> > of that.
> >
> > Also that TCG is the default:
> >
> > $ ./s390x-softmmu/qemu-system-s390x -cpu ?
> > s390 host
> >
> > And I don't see a way to make a user belief that all the defined CPU models are available to
> > a TCG user in the S390 case where most of the CPU facilities are not implemented.
>
> Well, we could simply add a "KVM required" note (maybe just an asterisk beside
> the CPU model description). But maybe we have a reasonable alternative below:
>
> >
> > >
> > > Anyway, whatever we decide to do, I believe we should start with
> > > something simple to get things working, and after that we can look for
> > > ways improve the help output with "runnable" info.
> >
> > I don't see how to solve this without cpu_desc_avail() or some other comparable mechanism, the
> > aliases e.g. are also dynamic...
>
> What bothers me in cpu_desc_avail() is that it depends on global state that is
> non-trivial (one needs to follow the whole KVM initialization path to find out
> if cpu_desc_avail() will be true or false).
>
> We could instead simply skip the cpu_list() call unconditionally on s390. e.g.:
>
> target-s390x/cpu.h:
> /* Delete the existing cpu_list macro */
>
> cpus.c:
> int list_cpus(FILE *f, fprintf_function cpu_fprintf, const char *optarg)
> {
> #if defined(cpu_list)
> cpu_list(f, cpu_fprintf);
> return 1;
> #else
> return 0;
> #endif
> }
>
> vl.c:
> if (cpu_model && is_help_option(cpu_model)) {
> /* zero list_cpus() return value means "-cpu ?" will be
> * handled later by machine initialization code */
> if (list_cpus(stdout, &fprintf, cpu_model)) {
> exit(0);
> }
> }
That approach is will do the job as well. I will prepare a patch for the next version.
Thanks!
>
> [...]
> > >
> > > About "-cpu ?": do we really want it to depend on -machine processing?
> > > Today, help output shows what the QEMU binary is capable of, not just
> > > what the host system and -machine option are capable of.
> >
> > I think we have to take it into account because the available CPU models might
> > deviate substantially as in the case for S390 for KVM and TCG.
>
> That's true, on s390 the set of available CPU models is very different on both
> cases. It breaks assumptions in the existing "-cpu ?" handling code in main().
>
> >
> > >
> > > If we decide to change that assumption, let's do it in a generic way and
> > > not as a arch-specific hack. The options I see are:
> >
> > welcome
> >
> > >
> > > 1) Continue with the current policy where "-cpu ?" does not depend on
> > > -machine arguments, and show all CPU models on "-cpu ?".
> > > 2) Deciding that, yes, it is OK to make "-cpu ?" depend on -machine
> > > arguments, and move the list_cpus() call after machine initialization
> > > inside generic main() code for all arches.
> > > 2.1) We could delay the list_cpus() call inside main() on all cases.
> > > 2.2) We could delay the list_cpus() call inside main() only if
> > > an explicit -machine option is present.
> > >
> > > I prefer (1) and my second choice would be (2.2), but the main point is
> > > that none of the options above require making s390 special and
> > > introducing cpu_desc_avail().
> >
> > My take here is 2.1 because omitting option -machine is a decision to some
> > defaults for machine type and accelerator type already.
>
> The problem with 2.1 is that some machine init functions require that
> additional command-line parameters are set and will abort (e.g. mips machines).
> So we can't do that unconditionally for all architectures.
>
> The proposal above is like 2.1, but conditional: it will delay "-cpu ?"
> handling only on architectures that don't define cpu_list().
perfect.
Michael
>
^ permalink raw reply [flat|nested] 55+ messages in thread
* Re: [Qemu-devel] [PATCH v6 05/17] Add optional parameters to QMP command query-cpu-definitions
2015-05-06 12:42 ` Eduardo Habkost
@ 2015-05-07 7:37 ` Michael Mueller
0 siblings, 0 replies; 55+ messages in thread
From: Michael Mueller @ 2015-05-07 7:37 UTC (permalink / raw)
To: Eduardo Habkost
Cc: Cornelia Huck, Gleb Natapov, qemu-devel, Alexander Graf,
Christian Borntraeger, Jason J. Herne, Daniel Hansel,
Paolo Bonzini, Andreas Faerber, Richard Henderson
On Wed, 6 May 2015 09:42:26 -0300
Eduardo Habkost <ehabkost@redhat.com> wrote:
> On Mon, Apr 27, 2015 at 04:53:19PM +0200, Michael Mueller wrote:
> [...]
> > diff --git a/qapi-schema.json b/qapi-schema.json
> > index 215a7bc..285b2d3 100644
> > --- a/qapi-schema.json
> > +++ b/qapi-schema.json
> > @@ -2536,21 +2536,43 @@
> > #
> > # @name: the name of the CPU definition
> > #
> > +# @default: #optional true if cpu model is the default,
> > +# omitted if false (since 2.4)
>
> Maybe we should clarify that it is the default in the machine provided
> as argument to query-cpu-definitions?
>
> > +#
> > +# @runnable: #optional true if cpu model is runnable,
> > +# omitted if false (since 2.4)
>
> Maybe we should clarify that it means the CPU model is runnable using
> the machine+accel combination provided as arguments to
> query-cpu-definitions?
I will extend the comments accordingly.
>
> (See also my question about the meaning of runnable when machine is
> omitted, in my reply to patch 15/17).
>
> > +#
> > +# @live-migration-safe: #optional true if cpu model represents a
> > +# cpu model that is safely migratable
> > +# omitted if false (since 2.4)
> > +#
> > +# @order: #optional order criterion
> > +#
> > # Since: 1.2.0
> > ##
> > { 'type': 'CpuDefinitionInfo',
> > - 'data': { 'name': 'str' } }
> > + 'data': { 'name': 'str', '*is-default': 'bool', '*runnable': 'bool',
> > + '*live-migration-safe': 'bool', '*order': 'int' } }
> >
> > ##
> > # @query-cpu-definitions:
> > #
> > -# Return a list of supported virtual CPU definitions
> > +# Return a list of supported virtual CPU definitions. In context with the
> > +# optional parameters @machine and @accel the returned list contains
> > +# also information if the respective cpu definition is runnable or the
> > +# default to be used.
> > +#
> > +# @machine: #optional machine type (since 2.4)
> > +#
> > +# @accel: #optional accelerator id (since 2.4)
> > #
> > # Returns: a list of CpuDefInfo
> > #
> > # Since: 1.2.0
> > ##
> > -{ 'command': 'query-cpu-definitions', 'returns': ['CpuDefinitionInfo'] }
> > +{ 'command': 'query-cpu-definitions',
> > + 'data': { '*machine': 'str', '*accel': 'AccelId' },
> > + 'returns': ['CpuDefinitionInfo'] }
> >
>
^ permalink raw reply [flat|nested] 55+ messages in thread
* Re: [Qemu-devel] [PATCH v6 02/17] Add accelerator id and model name to CPUState
2015-05-06 11:41 ` Eduardo Habkost
@ 2015-05-07 7:55 ` Michael Mueller
2015-05-07 15:04 ` Eduardo Habkost
0 siblings, 1 reply; 55+ messages in thread
From: Michael Mueller @ 2015-05-07 7:55 UTC (permalink / raw)
To: Eduardo Habkost
Cc: Cornelia Huck, Gleb Natapov, qemu-devel, Alexander Graf,
Christian Borntraeger, Jason J. Herne, Daniel Hansel,
Paolo Bonzini, Andreas Faerber, Richard Henderson
On Wed, 6 May 2015 08:41:50 -0300
Eduardo Habkost <ehabkost@redhat.com> wrote:
> On Wed, May 06, 2015 at 11:59:38AM +0200, Michael Mueller wrote:
> > On Tue, 5 May 2015 10:26:02 -0300
> > Eduardo Habkost <ehabkost@redhat.com> wrote:
> >
> > > On Mon, Apr 27, 2015 at 04:53:16PM +0200, Michael Mueller wrote:
> > > > The patch defines ids per accelerator and adds the accel_id and
> > > > the model_name to the CPUState. The accel_id is initialized by
> > > > common code, the model name needs to be initialized by target
> > > > specific code.
> > > >
> > > > Signed-off-by: Michael Mueller <mimu@linux.vnet.ibm.com>
> > > > Acked-by: Christian Borntraeger <borntraeger@de.ibm.com>
> > > > ---
> > > > include/qom/cpu.h | 5 +++++
> > > > qapi-schema.json | 9 +++++++++
> > > > qom/cpu.c | 14 ++++++++++++++
> > > > 3 files changed, 28 insertions(+)
> > > >
> > > > diff --git a/include/qom/cpu.h b/include/qom/cpu.h
> > > > index 9dafb48..4ffc050 100644
> > > > --- a/include/qom/cpu.h
> > > > +++ b/include/qom/cpu.h
> > > > @@ -236,6 +236,8 @@ struct kvm_run;
> > > > * @mem_io_pc: Host Program Counter at which the memory was accessed.
> > > > * @mem_io_vaddr: Target virtual address at which the memory was accessed.
> > > > * @kvm_fd: vCPU file descriptor for KVM.
> > > > + * @accel_id: accelerator id of this CPU.
> > > > + * @model_name: model name of this CPU
> > > > *
> > > > * State of one CPU core or thread.
> > > > */
> > > > @@ -313,6 +315,9 @@ struct CPUState {
> > > > (absolute value) offset as small as possible. This reduces code
> > > > size, especially for hosts without large memory offsets. */
> > > > volatile sig_atomic_t tcg_exit_req;
> > > > +
> > > > + AccelId accel_id;
> > >
> > > This can be a AccelState pointer, set on initialization, because we have
> > > another user case for having a AccelState pointer: query-cpu-definition
> > > implementations may create temporary CPU objects with a different accel
> > > object to be able to probe for accel-specific data.
> > >
> > > (The pointer may become a link QOM property later.)
> > >
> > >
> > > > + char *model_name;
> > > > };
> > > >
> > > > QTAILQ_HEAD(CPUTailQ, CPUState);
> > > > diff --git a/qapi-schema.json b/qapi-schema.json
> > > > index ac9594d..540e520 100644
> > > > --- a/qapi-schema.json
> > > > +++ b/qapi-schema.json
> > > > @@ -2515,6 +2515,15 @@
> > > > ##
> > > > { 'command': 'query-machines', 'returns': ['MachineInfo'] }
> > > >
> > > > +# @AccelId
> > > > +#
> > > > +# Defines accelerator ids
> > > > +#
> > > > +# Since: 2.4
> > > > +##
> > > > +{ 'enum': 'AccelId',
> > > > + 'data': ['qtest', 'tcg', 'kvm', 'xen'] }
> > > > +
> > >
> > > Not sure if it is better to have an enum or simply a string here.
> > >
> > > > ##
> > > > # @CpuDefinitionInfo:
> > > > #
> > > > diff --git a/qom/cpu.c b/qom/cpu.c
> > > > index 108bfa2..457afc7 100644
> > > > --- a/qom/cpu.c
> > > > +++ b/qom/cpu.c
> > > > @@ -67,6 +67,20 @@ CPUState *cpu_generic_init(const char *typename, const char *cpu_model)
> > > > goto out;
> > > > }
> > > >
> > > > + if (tcg_enabled()) {
> > > > + cpu->accel_id = ACCEL_ID_TCG;
> > > > + } else if (kvm_enabled()) {
> > > > + cpu->accel_id = ACCEL_ID_KVM;
> > > > + }
> > > > +#ifdef CONFIG_XEN
> > > > + else if (xen_enabled()) {
> > > > + cpu->accel_id = ACCEL_ID_XEN;
> > > > + }
> > > > +#endif
> > > > + else {
> > > > + cpu->accel_id = ACCEL_ID_QTEST;
> > > > + }
> > >
> > > You can simply use ACCEL_GET_CLASS(current_machine->accelerator)->name
> > > here. If we really want an enum, we can add an AccelId field to
> > > AccelClass, and initialize it properly on the accel class_init
> > > functions.
> >
> > The AccelClass (ac = ACCEL_GET_CLASS(current_machine->accelerator) would be ok though.
> > That will allow to access the ac->accel_id (not yet there) at places where required.
> >
> > I'm just not sure how to access current_machine here.
>
> It is a global variable declared in hw/boards.h. But it makes sense only
> if !CONFIG_USER.
>
> But I am starting to wonder if we shouldn't simply expose accel info as
> a /mmachine property, instead of per-CPU information. It may become
> per-CPU in the future, but right now we really don't support having
> multiple accelerators so why are we trying to pretend we do?
>
That brings me back into the direction I was before with the proposed query-cpu-model
QMP call... but that's OK. In the end that holds true also for the cpu model name as
well...
> >
> > >
> > > CONFIG_USER may require some special code when returning the accelerator
> > > ID as "tcg" because IIRC it doesn't use the QOM accel classes (yet).
> > >
> > > > +
> > > > object_property_set_bool(OBJECT(cpu), true, "realized", &err);
> > > >
> > > > out:
> > > > --
> > > > 1.8.3.1
> > > >
> > >
> >
>
^ permalink raw reply [flat|nested] 55+ messages in thread
* Re: [Qemu-devel] [PATCH v6 02/17] Add accelerator id and model name to CPUState
2015-05-07 7:55 ` Michael Mueller
@ 2015-05-07 15:04 ` Eduardo Habkost
0 siblings, 0 replies; 55+ messages in thread
From: Eduardo Habkost @ 2015-05-07 15:04 UTC (permalink / raw)
To: Michael Mueller
Cc: Cornelia Huck, Gleb Natapov, qemu-devel, Alexander Graf,
Christian Borntraeger, Jason J. Herne, Daniel Hansel,
Paolo Bonzini, Andreas Faerber, Richard Henderson
On Thu, May 07, 2015 at 09:55:45AM +0200, Michael Mueller wrote:
> On Wed, 6 May 2015 08:41:50 -0300
> Eduardo Habkost <ehabkost@redhat.com> wrote:
[...]
> > > > > diff --git a/qom/cpu.c b/qom/cpu.c
> > > > > index 108bfa2..457afc7 100644
> > > > > --- a/qom/cpu.c
> > > > > +++ b/qom/cpu.c
> > > > > @@ -67,6 +67,20 @@ CPUState *cpu_generic_init(const char *typename, const char *cpu_model)
> > > > > goto out;
> > > > > }
> > > > >
> > > > > + if (tcg_enabled()) {
> > > > > + cpu->accel_id = ACCEL_ID_TCG;
> > > > > + } else if (kvm_enabled()) {
> > > > > + cpu->accel_id = ACCEL_ID_KVM;
> > > > > + }
> > > > > +#ifdef CONFIG_XEN
> > > > > + else if (xen_enabled()) {
> > > > > + cpu->accel_id = ACCEL_ID_XEN;
> > > > > + }
> > > > > +#endif
> > > > > + else {
> > > > > + cpu->accel_id = ACCEL_ID_QTEST;
> > > > > + }
> > > >
> > > > You can simply use ACCEL_GET_CLASS(current_machine->accelerator)->name
> > > > here. If we really want an enum, we can add an AccelId field to
> > > > AccelClass, and initialize it properly on the accel class_init
> > > > functions.
> > >
> > > The AccelClass (ac = ACCEL_GET_CLASS(current_machine->accelerator) would be ok though.
> > > That will allow to access the ac->accel_id (not yet there) at places where required.
> > >
> > > I'm just not sure how to access current_machine here.
> >
> > It is a global variable declared in hw/boards.h. But it makes sense only
> > if !CONFIG_USER.
> >
> > But I am starting to wonder if we shouldn't simply expose accel info as
> > a /mmachine property, instead of per-CPU information. It may become
> > per-CPU in the future, but right now we really don't support having
> > multiple accelerators so why are we trying to pretend we do?
> >
>
> That brings me back into the direction I was before with the proposed query-cpu-model
> QMP call... but that's OK. In the end that holds true also for the cpu model name as
> well...
That's probably true with the current command-line options and current
machine implementations, but CPU model information is already inside the
CPU objects. Accel information, on the other hand, is still in global
variables such as kvm_allowed and current_machine.
I still believe per-CPU accel links are the right way to go in the
future, but it's not how the accelerator code works today.
That means I am OK with both approaches: exposing CPU::accel info, or
MachineState::accel info. The difference is that /machine/accel is
simpler to implement today.
--
Eduardo
^ permalink raw reply [flat|nested] 55+ messages in thread
* Re: [Qemu-devel] [PATCH v6 15/17] target-s390x: Extend arch specific QMP command query-cpu-definitions
2015-05-06 14:48 ` Michael Mueller
@ 2015-05-11 16:59 ` Eduardo Habkost
0 siblings, 0 replies; 55+ messages in thread
From: Eduardo Habkost @ 2015-05-11 16:59 UTC (permalink / raw)
To: Michael Mueller
Cc: Cornelia Huck, Gleb Natapov, qemu-devel, Alexander Graf,
Christian Borntraeger, Jason J. Herne, Daniel Hansel,
Paolo Bonzini, Andreas Faerber, Richard Henderson
On Wed, May 06, 2015 at 04:48:57PM +0200, Michael Mueller wrote:
> On Wed, 6 May 2015 09:37:41 -0300
> Eduardo Habkost <ehabkost@redhat.com> wrote:
[...]
> >
> > >
> > > - info = g_malloc0(sizeof(*info));
> > > - info->name = g_strdup("host");
> > > + memset(&mach, 0, sizeof(mach));
> > > + if (has_accel) {
> > > + switch (accel) {
> > > + case ACCEL_ID_KVM:
> > > + kvm_s390_get_machine_props(NULL, &mach);
> > > + break;
> > > + default:
> > > + return qmp_query_cpu_definition_host();
> >
> > This will return only a single element. I don't think that's correct. If
> > machine or accel is omitted, I believe we should just omit the
> > "runnable" field, but always return the full list of CPU models.
>
> That is the !KVM case where I keep the behavior currently unchanged to
> the existing implementation. But right for the TCG case there has to be
> a comparable get_machine_props() call returning whatever TCG is implementing.
> But that is not part of this patch series.
I believe the right thing to do is to return the full list of CPU
models, but report them as not runnable if accel is TCG, and simply omit
the runnable field if no accel argument is provided.
This way, existing management code that already runs query-cpu-models
(e.g. libvirt, that does that with "-machine none") will keep working.
--
Eduardo
^ permalink raw reply [flat|nested] 55+ messages in thread
end of thread, other threads:[~2015-05-11 16:59 UTC | newest]
Thread overview: 55+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-04-27 14:53 [Qemu-devel] [PATCH v6 00/17] s390 cpu model implementation Michael Mueller
2015-04-27 14:53 ` [Qemu-devel] [PATCH v6 01/17] Introduce stub routine cpu_desc_avail Michael Mueller
2015-05-05 13:55 ` Eduardo Habkost
2015-05-05 16:12 ` Michael Mueller
2015-05-05 17:41 ` Eduardo Habkost
2015-05-06 9:17 ` Michael Mueller
2015-05-06 11:23 ` Eduardo Habkost
2015-05-06 16:23 ` Michael Mueller
2015-05-06 17:06 ` Eduardo Habkost
2015-05-07 7:35 ` Michael Mueller
2015-04-27 14:53 ` [Qemu-devel] [PATCH v6 02/17] Add accelerator id and model name to CPUState Michael Mueller
2015-05-05 13:26 ` Eduardo Habkost
2015-05-05 14:36 ` Eric Blake
2015-05-05 14:46 ` Eduardo Habkost
2015-05-06 9:28 ` Michael Mueller
2015-05-06 9:59 ` Michael Mueller
2015-05-06 11:41 ` Eduardo Habkost
2015-05-07 7:55 ` Michael Mueller
2015-05-07 15:04 ` Eduardo Habkost
2015-04-27 14:53 ` [Qemu-devel] [PATCH v6 03/17] Extend QMP command query-cpus to return accelerator id and model name Michael Mueller
2015-05-05 13:11 ` Eduardo Habkost
2015-05-06 9:49 ` Michael Mueller
2015-05-06 11:33 ` Eduardo Habkost
2015-04-27 14:53 ` [Qemu-devel] [PATCH v6 04/17] Extend HMP command info cpus to display " Michael Mueller
2015-05-05 13:14 ` Eduardo Habkost
2015-05-06 7:32 ` Michael Mueller
2015-05-06 10:38 ` Eduardo Habkost
2015-05-06 12:59 ` Luiz Capitulino
2015-05-06 13:33 ` Eduardo Habkost
2015-05-06 13:44 ` Michael Mueller
2015-04-27 14:53 ` [Qemu-devel] [PATCH v6 05/17] Add optional parameters to QMP command query-cpu-definitions Michael Mueller
2015-05-06 12:42 ` Eduardo Habkost
2015-05-07 7:37 ` Michael Mueller
2015-04-27 14:53 ` [Qemu-devel] [PATCH v6 06/17] target-s390x: Introduce S390 CPU facilities Michael Mueller
2015-04-27 14:53 ` [Qemu-devel] [PATCH v6 07/17] target-s390x: Generate facility defines per S390 CPU model Michael Mueller
2015-04-27 14:53 ` [Qemu-devel] [PATCH v6 08/17] target-s390x: Introduce S390 CPU models Michael Mueller
2015-04-27 14:53 ` [Qemu-devel] [PATCH v6 09/17] target-s390x: Define S390 CPU model specific facility lists Michael Mueller
2015-04-27 14:53 ` [Qemu-devel] [PATCH v6 10/17] target-s390x: Add S390 CPU model alias definition routines Michael Mueller
2015-04-27 14:53 ` [Qemu-devel] [PATCH v6 11/17] target-s390x: Add KVM VM attribute interface for S390 CPU models Michael Mueller
2015-04-27 14:53 ` [Qemu-devel] [PATCH v6 12/17] target-s390x: Add S390 CPU class initialization routines Michael Mueller
2015-05-05 14:34 ` Eduardo Habkost
2015-05-06 8:02 ` Michael Mueller
2015-05-06 12:20 ` Eduardo Habkost
2015-04-27 14:53 ` [Qemu-devel] [PATCH v6 13/17] target-s390x: Prepare accelerator during S390 CPU object realization Michael Mueller
2015-04-27 14:53 ` [Qemu-devel] [PATCH v6 14/17] target-s390x: Initialize S390 CPU model name in CPUState Michael Mueller
2015-04-27 14:53 ` [Qemu-devel] [PATCH v6 15/17] target-s390x: Extend arch specific QMP command query-cpu-definitions Michael Mueller
2015-05-05 18:40 ` Eduardo Habkost
2015-05-06 15:31 ` Michael Mueller
2015-05-06 16:00 ` Eduardo Habkost
2015-05-06 16:27 ` Michael Mueller
2015-05-06 12:37 ` Eduardo Habkost
2015-05-06 14:48 ` Michael Mueller
2015-05-11 16:59 ` Eduardo Habkost
2015-04-27 14:53 ` [Qemu-devel] [PATCH v6 16/17] target-s390x: Introduce S390 CPU facility test routine Michael Mueller
2015-04-27 14:53 ` [Qemu-devel] [PATCH v6 17/17] target-s390x: Enable S390 CPU model usage Michael Mueller
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.