linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v4 00/15] s390x cpu model implementation
@ 2015-03-30 14:28 Michael Mueller
  2015-03-30 14:28 ` [PATCH v4 01/15] Introduce stub routine cpu_desc_avail Michael Mueller
                   ` (14 more replies)
  0 siblings, 15 replies; 45+ messages in thread
From: Michael Mueller @ 2015-03-30 14:28 UTC (permalink / raw)
  To: qemu-devel, kvm, linux-s390, linux-kernel
  Cc: Eduardo Habkost, Gleb Natapov, Alexander Graf,
	Christian Borntraeger, Jason J. Herne, Cornelia Huck,
	Paolo Bonzini, Michael Mueller, Andreas Faerber,
	Richard Henderson, Daniel Hansel

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 S390 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 S390 cpu models of a given host system. Together with the newly
defined QMP command "query-cpu-model", which returns the current active
S390 cpu model of a guest, 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.

Details:
- The QMP command query-cpu-model returns the active cpu model and the
  accellerator name it is using:

  {"name":"2066-ga1","accel":"kvm"}

Or just the empty model in case an accelerator does not take care of cpu models:

  {}

- A management instance like libvirt may probe by means of the QMP command
  query-cpu-definitions which models are defined and usable for specific
  accelerators and machine types. To implement this the cpu definition info
  type gets two optional fields named 'runnable' and 'is-default' representing
  the runnable cpu model for the given accelertor and machine type as well as
  the host cpu model.

  { "execute": "query-cpu-definitions",
    "arguments": { "accel": "kvm", "machine": "s390-ccw-virtio" } }

  { "return": [ {"name": "2964-ga1","runnable": false, "is-default": false},
                {"name": "2828-ga1","runnable": true,  "is-default": false},
                {"name": "2827-ga2","runnable": true,  "is-default": true},
                ...
                {"name": "2064-ga1","runnable": true,  "is-default": false} ] }
		
Or just host in case an accelerator does not take care of the cpu models:

  { "return": [ {"name": "host"} ] }

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 (15):
  Introduce stub routine cpu_desc_avail
  target-s390x: Introduce cpu facilities
  target-s390x: Generate facility defines per cpu model
  target-s390x: Introduce cpu models
  target-s390x: Define cpu model specific facility lists
  target-s390x: Add cpu model alias definition routines
  target-s390x: Update linux-headers/asm-s390/kvm.h
  target-s390x: Add KVM VM attribute interface for cpu models
  target-s390x: Add cpu class initialization routines
  target-s390x: Prepare accelerator during cpu object realization
  target-s390x: New QMP command query-cpu-model
  Add optional parameters to QMP command query-cpu-definitions
  target-s390x: Extend QMP command query-cpu-definitions
  target-s390x: Introduce facility test routine
  target-s390x: Enable cpu model usage

 Makefile.target               |   2 +-
 hw/s390x/s390-virtio.c        |  12 +-
 include/qemu-common.h         |   2 +
 include/sysemu/arch_init.h    |   7 +-
 linux-headers/asm-s390/kvm.h  |  18 +-
 qapi-schema.json              |  49 ++-
 qmp-commands.hx               |   8 +-
 qmp.c                         |  14 +-
 rules.mak                     |   1 +
 stubs/Makefile.objs           |   2 +
 stubs/arch-query-cpu-def.c    |   6 +-
 stubs/arch-query-cpu-mod.c    |   9 +
 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     | 767 ++++++++++++++++++++++++++++++++++++++++++
 target-s390x/cpu-models.h     | 169 ++++++++++
 target-s390x/cpu-qom.h        |  36 ++
 target-s390x/cpu.c            | 163 ++++++++-
 target-s390x/gen-facilities.c | 417 +++++++++++++++++++++++
 target-s390x/helper.c         |   9 +-
 target-s390x/kvm.c            | 129 +++++++
 trace-events                  |   3 +
 vl.c                          |   2 +-
 27 files changed, 1921 insertions(+), 35 deletions(-)
 create mode 100644 stubs/arch-query-cpu-mod.c
 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] 45+ messages in thread

* [PATCH v4 01/15] Introduce stub routine cpu_desc_avail
  2015-03-30 14:28 [PATCH v4 00/15] s390x cpu model implementation Michael Mueller
@ 2015-03-30 14:28 ` Michael Mueller
  2015-03-30 14:28 ` [PATCH v4 02/15] target-s390x: Introduce cpu facilities Michael Mueller
                   ` (13 subsequent siblings)
  14 siblings, 0 replies; 45+ messages in thread
From: Michael Mueller @ 2015-03-30 14:28 UTC (permalink / raw)
  To: qemu-devel, kvm, linux-s390, linux-kernel
  Cc: Eduardo Habkost, Gleb Natapov, Alexander Graf,
	Christian Borntraeger, Jason J. Herne, Cornelia Huck,
	Paolo Bonzini, Michael Mueller, Andreas Faerber,
	Richard Henderson, Daniel Hansel

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>
---
 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] 45+ messages in thread

* [PATCH v4 02/15] target-s390x: Introduce cpu facilities
  2015-03-30 14:28 [PATCH v4 00/15] s390x cpu model implementation Michael Mueller
  2015-03-30 14:28 ` [PATCH v4 01/15] Introduce stub routine cpu_desc_avail Michael Mueller
@ 2015-03-30 14:28 ` Michael Mueller
  2015-03-30 14:28 ` [PATCH v4 03/15] target-s390x: Generate facility defines per cpu model Michael Mueller
                   ` (12 subsequent siblings)
  14 siblings, 0 replies; 45+ messages in thread
From: Michael Mueller @ 2015-03-30 14:28 UTC (permalink / raw)
  To: qemu-devel, kvm, linux-s390, linux-kernel
  Cc: Eduardo Habkost, Gleb Natapov, Alexander Graf,
	Christian Borntraeger, Jason J. Herne, Cornelia Huck,
	Paolo Bonzini, Michael Mueller, Andreas Faerber,
	Richard Henderson, Daniel Hansel

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>
---
 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..db1f064
--- /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_TRANSACTIONAL_EXE               = 73,
+    /*
+     * The store-hypervisor-information facility #74 is
+     * z/VM related and when added to be handled by QEMU
+     * when hosted on LPAR. (see: SC24-6179-05 page 953)
+     */
+    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] 45+ messages in thread

* [PATCH v4 03/15] target-s390x: Generate facility defines per cpu model
  2015-03-30 14:28 [PATCH v4 00/15] s390x cpu model implementation Michael Mueller
  2015-03-30 14:28 ` [PATCH v4 01/15] Introduce stub routine cpu_desc_avail Michael Mueller
  2015-03-30 14:28 ` [PATCH v4 02/15] target-s390x: Introduce cpu facilities Michael Mueller
@ 2015-03-30 14:28 ` Michael Mueller
  2015-03-30 14:28 ` [PATCH v4 04/15] target-s390x: Introduce cpu models Michael Mueller
                   ` (11 subsequent siblings)
  14 siblings, 0 replies; 45+ messages in thread
From: Michael Mueller @ 2015-03-30 14:28 UTC (permalink / raw)
  To: qemu-devel, kvm, linux-s390, linux-kernel
  Cc: Eduardo Habkost, Gleb Natapov, Alexander Graf,
	Christian Borntraeger, Jason J. Herne, Cornelia Huck,
	Paolo Bonzini, Michael Mueller, Andreas Faerber,
	Richard Henderson, Daniel Hansel

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
"target-s390x/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,...

fix: target-s390/Makefile.obj

private build dir now supported, but make clean still has an issue:

[mimu@p57lp59 (bb/mimu/devel-cpu-model-v6+) qemu]$ make clean
rm -f config.mak op-i386.h opc-i386.h gen-op-i386.h op-arm.h opc-arm.h gen-op-arm.h
...
for d in s390x-softmmu s390x-linux-user pc-bios/s390-ccw; do \
if test -d $d; then make -C $d clean || exit 1; fi; \
rm -f $d/qemu-options.def; \
        done
make[1]: Entering directory `/home/mimu/REPO/qemu/foobuild/s390x-softmmu'
  CC    gen-facilities
cc1: error: -I/usr/include/pixman-1: No such file or directory [-Werror]
cc1: all warnings being treated as errors
make[1]: *** [/gen-facilities] Error 1
make[1]: Leaving directory `/home/mimu/REPO/qemu/foobuild/s390x-softmmu'
make: *** [clean] Error 1

Signed-off-by: Michael Mueller <mimu@linux.vnet.ibm.com>
---
 Makefile.target               |   2 +-
 rules.mak                     |   1 +
 target-s390x/Makefile.objs    |  20 ++
 target-s390x/gen-facilities.c | 417 ++++++++++++++++++++++++++++++++++++++++++
 4 files changed, 439 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..f4f4c57
--- /dev/null
+++ b/target-s390x/gen-facilities.c
@@ -0,0 +1,417 @@
+/*
+ * 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,
+};
+#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
+ *******************************/
+#define set_MASK_QEMU EmptyFacs
+#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] 45+ messages in thread

* [PATCH v4 04/15] target-s390x: Introduce cpu models
  2015-03-30 14:28 [PATCH v4 00/15] s390x cpu model implementation Michael Mueller
                   ` (2 preceding siblings ...)
  2015-03-30 14:28 ` [PATCH v4 03/15] target-s390x: Generate facility defines per cpu model Michael Mueller
@ 2015-03-30 14:28 ` Michael Mueller
  2015-03-30 14:28 ` [PATCH v4 05/15] target-s390x: Define cpu model specific facility lists Michael Mueller
                   ` (10 subsequent siblings)
  14 siblings, 0 replies; 45+ messages in thread
From: Michael Mueller @ 2015-03-30 14:28 UTC (permalink / raw)
  To: qemu-devel, kvm, linux-s390, linux-kernel
  Cc: Eduardo Habkost, Gleb Natapov, Alexander Graf,
	Christian Borntraeger, Jason J. Herne, Cornelia Huck,
	Paolo Bonzini, Michael Mueller, Andreas Faerber,
	Richard Henderson, Daniel Hansel

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 the existing S390CPUClass by model related properties.

Signed-off-by: Michael Mueller <mimu@linux.vnet.ibm.com>
Reviewed-by: Thomas Huth <thuth@linux.vnet.ibm.com>
---
 target-s390x/Makefile.objs |  1 +
 target-s390x/cpu-models.c  | 77 ++++++++++++++++++++++++++++++++++++++++++++++
 target-s390x/cpu-models.h  | 72 +++++++++++++++++++++++++++++++++++++++++++
 target-s390x/cpu-qom.h     | 36 ++++++++++++++++++++++
 4 files changed, 186 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..908e8eb
--- /dev/null
+++ b/target-s390x/cpu-models.c
@@ -0,0 +1,77 @@
+/*
+ * 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->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..250d6d1 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,6 +82,12 @@ typedef struct S390CPUClass {
     void (*load_normal)(CPUState *cpu);
     void (*cpu_reset)(CPUState *cpu);
     void (*initial_cpu_reset)(CPUState *cpu);
+    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;
 
 /**
-- 
1.8.3.1


^ permalink raw reply related	[flat|nested] 45+ messages in thread

* [PATCH v4 05/15] target-s390x: Define cpu model specific facility lists
  2015-03-30 14:28 [PATCH v4 00/15] s390x cpu model implementation Michael Mueller
                   ` (3 preceding siblings ...)
  2015-03-30 14:28 ` [PATCH v4 04/15] target-s390x: Introduce cpu models Michael Mueller
@ 2015-03-30 14:28 ` Michael Mueller
  2015-03-30 14:28 ` [PATCH v4 06/15] target-s390x: Add cpu model alias definition routines Michael Mueller
                   ` (9 subsequent siblings)
  14 siblings, 0 replies; 45+ messages in thread
From: Michael Mueller @ 2015-03-30 14:28 UTC (permalink / raw)
  To: qemu-devel, kvm, linux-s390, linux-kernel
  Cc: Eduardo Habkost, Gleb Natapov, Alexander Graf,
	Christian Borntraeger, Jason J. Herne, Cornelia Huck,
	Paolo Bonzini, Michael Mueller, Andreas Faerber,
	Richard Henderson, Daniel Hansel

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>
---
 target-s390x/cpu-models.c | 12 ++++++++++++
 target-s390x/cpu-models.h |  8 ++++++++
 target-s390x/cpu.c        |  1 +
 3 files changed, 21 insertions(+)

diff --git a/target-s390x/cpu-models.c b/target-s390x/cpu-models.c
index 908e8eb..028e3de 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->mach.ga    = cpu_ga(_cpu_id);                               \
         cc->mach.class = cpu_class(_cpu_id);                            \
@@ -29,6 +34,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                                               \
@@ -45,6 +51,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")
@@ -75,3 +86,4 @@ 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
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 e0537fa..0db62c2 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] 45+ messages in thread

* [PATCH v4 06/15] target-s390x: Add cpu model alias definition routines
  2015-03-30 14:28 [PATCH v4 00/15] s390x cpu model implementation Michael Mueller
                   ` (4 preceding siblings ...)
  2015-03-30 14:28 ` [PATCH v4 05/15] target-s390x: Define cpu model specific facility lists Michael Mueller
@ 2015-03-30 14:28 ` Michael Mueller
  2015-03-30 14:28 ` [PATCH v4 07/15] target-s390x: Update linux-headers/asm-s390/kvm.h Michael Mueller
                   ` (8 subsequent siblings)
  14 siblings, 0 replies; 45+ messages in thread
From: Michael Mueller @ 2015-03-30 14:28 UTC (permalink / raw)
  To: qemu-devel, kvm, linux-s390, linux-kernel
  Cc: Eduardo Habkost, Gleb Natapov, Alexander Graf,
	Christian Borntraeger, Jason J. Herne, Cornelia Huck,
	Paolo Bonzini, Michael Mueller, Andreas Faerber,
	Richard Henderson, Daniel Hansel

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>
---
 target-s390x/cpu-models.c | 89 +++++++++++++++++++++++++++++++++++++++++++++++
 target-s390x/cpu-models.h | 11 ++++++
 target-s390x/cpu.c        |  1 +
 3 files changed, 101 insertions(+)

diff --git a/target-s390x/cpu-models.c b/target-s390x/cpu-models.c
index 028e3de..f792066 100644
--- a/target-s390x/cpu-models.c
+++ b/target-s390x/cpu-models.c
@@ -87,3 +87,92 @@ 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 GSList *s390_cpu_aliases;
+
+static gint s390_cpu_compare_class_name(gconstpointer a, gconstpointer b)
+{
+    const char *aname = object_class_get_name((ObjectClass *) a);
+    const char *bname = b;
+    int blen;
+
+    if (!strcmp(bname, "none") &&
+        !strcmp(aname, TYPE_S390_CPU)) {
+        return 0;
+    }
+    blen = strlen(bname);
+    if (!strncasecmp(bname, aname, blen) &&
+        !strcmp(aname + blen, "-" TYPE_S390_CPU)) {
+        return 0;
+    }
+    return -1;
+}
+
+/**
+ * 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_class_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..562464f 100644
--- a/target-s390x/cpu-models.h
+++ b/target-s390x/cpu-models.h
@@ -35,6 +35,17 @@
 #define cpu_class(x)      (((x) >> 20) & 0x3)
 #define cpu_generation(x) (((x) >> 24) & 0xff)
 
+ObjectClass *s390_cpu_class_by_name(const char *name);
+int set_s390_cpu_alias(const char *name, const char *model);
+
+/*
+ * S390 cpu aliases will be added dynamically
+ */
+typedef struct S390CPUAlias {
+    char *name;
+    char *model;
+} S390CPUAlias;
+
 /*
  * bits 0-7   : CMOS generation
  * bits 8-9   : reserved
diff --git a/target-s390x/cpu.c b/target-s390x/cpu.c
index 0db62c2..2b78e6a 100644
--- a/target-s390x/cpu.c
+++ b/target-s390x/cpu.c
@@ -310,6 +310,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] 45+ messages in thread

* [PATCH v4 07/15] target-s390x: Update linux-headers/asm-s390/kvm.h
  2015-03-30 14:28 [PATCH v4 00/15] s390x cpu model implementation Michael Mueller
                   ` (5 preceding siblings ...)
  2015-03-30 14:28 ` [PATCH v4 06/15] target-s390x: Add cpu model alias definition routines Michael Mueller
@ 2015-03-30 14:28 ` Michael Mueller
  2015-03-30 19:36   ` Christian Borntraeger
  2015-03-30 14:28 ` [PATCH v4 08/15] target-s390x: Add KVM VM attribute interface for cpu models Michael Mueller
                   ` (7 subsequent siblings)
  14 siblings, 1 reply; 45+ messages in thread
From: Michael Mueller @ 2015-03-30 14:28 UTC (permalink / raw)
  To: qemu-devel, kvm, linux-s390, linux-kernel
  Cc: Eduardo Habkost, Gleb Natapov, Alexander Graf,
	Christian Borntraeger, Jason J. Herne, Cornelia Huck,
	Paolo Bonzini, Michael Mueller, Andreas Faerber,
	Richard Henderson, Daniel Hansel

Signed-off-by: Michael Mueller <mimu@linux.vnet.ibm.com>
---
 linux-headers/asm-s390/kvm.h | 18 +++++++++---------
 1 file changed, 9 insertions(+), 9 deletions(-)

diff --git a/linux-headers/asm-s390/kvm.h b/linux-headers/asm-s390/kvm.h
index c5a93eb..bfe6925 100644
--- a/linux-headers/asm-s390/kvm.h
+++ b/linux-headers/asm-s390/kvm.h
@@ -70,8 +70,14 @@ struct kvm_s390_io_adapter_req {
 #define KVM_S390_VM_TOD_LOW		0
 #define KVM_S390_VM_TOD_HIGH		1
 
+/* kvm attributes for crypto */
+#define KVM_S390_VM_CRYPTO_ENABLE_AES_KW	0
+#define KVM_S390_VM_CRYPTO_ENABLE_DEA_KW	1
+#define KVM_S390_VM_CRYPTO_DISABLE_AES_KW	2
+#define KVM_S390_VM_CRYPTO_DISABLE_DEA_KW	3
+
 /* kvm attributes for KVM_S390_VM_CPU_MODEL */
-/* processor related attributes are r/w */
+/* kvm S390 processor related attributes are r/w */
 #define KVM_S390_VM_CPU_PROCESSOR	0
 struct kvm_s390_vm_cpu_processor {
 	__u64 cpuid;
@@ -80,22 +86,16 @@ struct kvm_s390_vm_cpu_processor {
 	__u64 fac_list[256];
 };
 
-/* machine related attributes are r/o */
+/* kvm S390 machine related attributes are r/o */
 #define KVM_S390_VM_CPU_MACHINE		1
 struct kvm_s390_vm_cpu_machine {
 	__u64 cpuid;
-	__u32 ibc;
+	__u32 ibc_range;
 	__u8  pad[4];
 	__u64 fac_mask[256];
 	__u64 fac_list[256];
 };
 
-/* kvm attributes for crypto */
-#define KVM_S390_VM_CRYPTO_ENABLE_AES_KW	0
-#define KVM_S390_VM_CRYPTO_ENABLE_DEA_KW	1
-#define KVM_S390_VM_CRYPTO_DISABLE_AES_KW	2
-#define KVM_S390_VM_CRYPTO_DISABLE_DEA_KW	3
-
 /* for KVM_GET_REGS and KVM_SET_REGS */
 struct kvm_regs {
 	/* general purpose regs for s390 */
-- 
1.8.3.1


^ permalink raw reply related	[flat|nested] 45+ messages in thread

* [PATCH v4 08/15] target-s390x: Add KVM VM attribute interface for cpu models
  2015-03-30 14:28 [PATCH v4 00/15] s390x cpu model implementation Michael Mueller
                   ` (6 preceding siblings ...)
  2015-03-30 14:28 ` [PATCH v4 07/15] target-s390x: Update linux-headers/asm-s390/kvm.h Michael Mueller
@ 2015-03-30 14:28 ` Michael Mueller
  2015-03-30 14:28 ` [PATCH v4 09/15] target-s390x: Add cpu class initialization routines Michael Mueller
                   ` (6 subsequent siblings)
  14 siblings, 0 replies; 45+ messages in thread
From: Michael Mueller @ 2015-03-30 14:28 UTC (permalink / raw)
  To: qemu-devel, kvm, linux-s390, linux-kernel
  Cc: Eduardo Habkost, Gleb Natapov, Alexander Graf,
	Christian Borntraeger, Jason J. Herne, Cornelia Huck,
	Paolo Bonzini, Michael Mueller, Andreas Faerber,
	Richard Henderson, Daniel Hansel

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>
---
 target-s390x/cpu-models.h | 34 ++++++++++++++++++++
 target-s390x/kvm.c        | 79 +++++++++++++++++++++++++++++++++++++++++++++++
 trace-events              |  3 ++
 3 files changed, 116 insertions(+)

diff --git a/target-s390x/cpu-models.h b/target-s390x/cpu-models.h
index 562464f..3b75236 100644
--- a/target-s390x/cpu-models.h
+++ b/target-s390x/cpu-models.h
@@ -46,6 +46,40 @@ typedef struct S390CPUAlias {
     char *model;
 } S390CPUAlias;
 
+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_range;
+    uint8_t  pad[4];
+    uint64_t fac_list_mask[FAC_LIST_ARCH_S390_SIZE_UINT64];
+    uint64_t fac_list[FAC_LIST_ARCH_S390_SIZE_UINT64];
+} S390MachineProps;
+
+#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
+
 /*
  * bits 0-7   : CMOS generation
  * bits 8-9   : reserved
diff --git a/target-s390x/kvm.c b/target-s390x/kvm.c
index b48c643..bde4aaa 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, uint64_t addr)
+{
+    int rc = -ENOSYS;
+    struct kvm_device_attr dev_attr = {
+        .group = KVM_S390_VM_CPU_MODEL,
+        .attr = attr,
+        .addr = 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, uint64_t addr)
+{
+    int rc = -ENOSYS;
+    struct kvm_device_attr dev_attr = {
+        .group = KVM_S390_VM_CPU_MODEL,
+        .attr = attr,
+        .addr = 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, (uint64_t) prop);
+    }
+    trace_kvm_get_machine_props(rc, prop->cpuid, prop->ibc_range);
+
+    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, (uint64_t) 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, (uint64_t) 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] 45+ messages in thread

* [PATCH v4 09/15] target-s390x: Add cpu class initialization routines
  2015-03-30 14:28 [PATCH v4 00/15] s390x cpu model implementation Michael Mueller
                   ` (7 preceding siblings ...)
  2015-03-30 14:28 ` [PATCH v4 08/15] target-s390x: Add KVM VM attribute interface for cpu models Michael Mueller
@ 2015-03-30 14:28 ` Michael Mueller
  2015-03-30 14:28 ` [PATCH v4 10/15] target-s390x: Prepare accelerator during cpu object realization Michael Mueller
                   ` (5 subsequent siblings)
  14 siblings, 0 replies; 45+ messages in thread
From: Michael Mueller @ 2015-03-30 14:28 UTC (permalink / raw)
  To: qemu-devel, kvm, linux-s390, linux-kernel
  Cc: Eduardo Habkost, Gleb Natapov, Alexander Graf,
	Christian Borntraeger, Jason J. Herne, Cornelia Huck,
	Paolo Bonzini, Michael Mueller, Andreas Faerber,
	Richard Henderson, Daniel Hansel

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>
---
 target-s390x/cpu-models.c | 494 ++++++++++++++++++++++++++++++++++++++++++++++
 target-s390x/cpu-models.h |  31 +++
 target-s390x/cpu.c        |  17 +-
 target-s390x/kvm.c        |   5 +-
 4 files changed, 545 insertions(+), 2 deletions(-)

diff --git a/target-s390x/cpu-models.c b/target-s390x/cpu-models.c
index f792066..8a877d3 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                                                         \
@@ -87,8 +92,41 @@ 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;
+
 static GSList *s390_cpu_aliases;
 
+/* 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_class_name(gconstpointer a, gconstpointer b)
 {
     const char *aname = object_class_get_name((ObjectClass *) a);
@@ -176,3 +214,459 @@ int set_s390_cpu_alias(const char *name, const char *model)
     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 || !cc->proc.type || 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];
+
+    if (!cc->proc.type) {
+        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_range)) {
+        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_range)) {
+                cc->proc.ibc = oldest_ibc(parm->prop->ibc_range);
+            }
+            if (cc->proc.ibc > newest_ibc(parm->prop->ibc_range)) {
+                cc->proc.ibc = newest_ibc(parm->prop->ibc_range);
+            }
+        } else {
+            cc->proc.ibc = oldest_ibc(parm->prop->ibc_range);
+        }
+    }
+
+    /*
+     * 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_list_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_list_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] ||
+        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);
+    const char *typename = object_class_get_name(oc);
+    S390CPUAlias *alias;
+    GSList *item;
+    char *name;
+
+    if (!kvm_enabled()) {
+        return;
+    }
+    if (!cc->is_active[ACCEL_CURRENT]) {
+        return;
+    }
+    name = g_strndup(typename, strlen(typename) - strlen("-" TYPE_S390_CPU));
+    (*s->cpu_fprintf)(s->file, "s390 %-10s %s\n", name, 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 3b75236..9562088 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))
+
 ObjectClass *s390_cpu_class_by_name(const char *name);
 int set_s390_cpu_alias(const char *name, const char *model);
 
@@ -80,6 +100,17 @@ static inline bool kvm_s390_cpu_classes_initialized(void)
 }
 #endif
 
+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);
+
+extern uint64_t qemu_s390_fac_list_mask[];
+
 /*
  * bits 0-7   : CMOS generation
  * bits 8-9   : reserved
diff --git a/target-s390x/cpu.c b/target-s390x/cpu.c
index 2b78e6a..c33f05e 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 bde4aaa..9b2cfeb 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] 45+ messages in thread

* [PATCH v4 10/15] target-s390x: Prepare accelerator during cpu object realization
  2015-03-30 14:28 [PATCH v4 00/15] s390x cpu model implementation Michael Mueller
                   ` (8 preceding siblings ...)
  2015-03-30 14:28 ` [PATCH v4 09/15] target-s390x: Add cpu class initialization routines Michael Mueller
@ 2015-03-30 14:28 ` Michael Mueller
  2015-03-30 19:33   ` Eduardo Habkost
  2015-03-30 14:28 ` [PATCH v4 11/15] target-s390x: New QMP command query-cpu-model Michael Mueller
                   ` (4 subsequent siblings)
  14 siblings, 1 reply; 45+ messages in thread
From: Michael Mueller @ 2015-03-30 14:28 UTC (permalink / raw)
  To: qemu-devel, kvm, linux-s390, linux-kernel
  Cc: Eduardo Habkost, Gleb Natapov, Alexander Graf,
	Christian Borntraeger, Jason J. Herne, Cornelia Huck,
	Paolo Bonzini, Michael Mueller, Andreas Faerber,
	Richard Henderson, Daniel Hansel

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>
---
 target-s390x/cpu-models.c | 37 +++++++++++++++++++++++++++++++++++++
 target-s390x/cpu-models.h |  4 ++++
 target-s390x/cpu.c        |  1 +
 3 files changed, 42 insertions(+)

diff --git a/target-s390x/cpu-models.c b/target-s390x/cpu-models.c
index 8a877d3..ba873ea 100644
--- a/target-s390x/cpu-models.c
+++ b/target-s390x/cpu-models.c
@@ -111,6 +111,7 @@ typedef struct ParmAddrAddrModeMask {
 } ParmAddrAddrModeMask;
 
 static GSList *s390_cpu_aliases;
+static bool cpu_models_used;
 
 /* compare order of two cpu classes for ascending sort */
 gint s390_cpu_class_asc_order_compare(gconstpointer a, gconstpointer b)
@@ -670,3 +671,39 @@ 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;
+
+    /* none cpu model case */
+    if (!strcmp(object_class_get_name(OBJECT_CLASS(cc)), TYPE_S390_CPU)) {
+        return;
+    }
+
+    /* accelerator already prepared */
+    if (cpu_models_used) {
+        return;
+    }
+
+    proc.cpuid = cpuid(cc->proc);
+    proc.ibc = cc->proc.ibc;
+    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()) {
+        if (!kvm_s390_set_processor_props(&proc)) {
+            cpu_models_used = true;
+        }
+    }
+}
diff --git a/target-s390x/cpu-models.h b/target-s390x/cpu-models.h
index 9562088..fe3997f 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)
@@ -108,6 +111,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);
 
 extern uint64_t qemu_s390_fac_list_mask[];
 
diff --git a/target-s390x/cpu.c b/target-s390x/cpu.c
index c33f05e..829945d 100644
--- a/target-s390x/cpu.c
+++ b/target-s390x/cpu.c
@@ -180,6 +180,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] 45+ messages in thread

* [PATCH v4 11/15] target-s390x: New QMP command query-cpu-model
  2015-03-30 14:28 [PATCH v4 00/15] s390x cpu model implementation Michael Mueller
                   ` (9 preceding siblings ...)
  2015-03-30 14:28 ` [PATCH v4 10/15] target-s390x: Prepare accelerator during cpu object realization Michael Mueller
@ 2015-03-30 14:28 ` Michael Mueller
  2015-03-30 19:50   ` Eduardo Habkost
                     ` (3 more replies)
  2015-03-30 14:28 ` [PATCH v4 12/15] Add optional parameters to QMP command query-cpu-definitions Michael Mueller
                   ` (3 subsequent siblings)
  14 siblings, 4 replies; 45+ messages in thread
From: Michael Mueller @ 2015-03-30 14:28 UTC (permalink / raw)
  To: qemu-devel, kvm, linux-s390, linux-kernel
  Cc: Eduardo Habkost, Gleb Natapov, Alexander Graf,
	Christian Borntraeger, Jason J. Herne, Cornelia Huck,
	Paolo Bonzini, Michael Mueller, Andreas Faerber,
	Richard Henderson, Daniel Hansel

This patch implements a new QMP request named 'query-cpu-model'.
It returns the cpu model of cpu 0 and its backing accelerator.

request:
  {"execute" : "query-cpu-model" }

answer:
  {"return" : {"name": "2827-ga2", "accel": "kvm" }}

Alias names are resolved to their respective machine type and GA names
already during cpu instantiation. Thus, also a cpu model like 'host'
which is implemented as alias will return its normalized cpu model name.

Furthermore the patch implements the following function:

- s390_cpu_models_used(), returns true if S390 cpu models are in use

Signed-off-by: Michael Mueller <mimu@linux.vnet.ibm.com>
---
 include/sysemu/arch_init.h |  1 +
 qapi-schema.json           | 35 +++++++++++++++++++++++++++++++++++
 qmp-commands.hx            |  6 ++++++
 qmp.c                      |  5 +++++
 stubs/Makefile.objs        |  1 +
 stubs/arch-query-cpu-mod.c |  9 +++++++++
 target-s390x/cpu-models.c  | 14 ++++++++++++++
 target-s390x/cpu-models.h  |  1 +
 target-s390x/cpu.c         | 29 +++++++++++++++++++++++++++++
 9 files changed, 101 insertions(+)
 create mode 100644 stubs/arch-query-cpu-mod.c

diff --git a/include/sysemu/arch_init.h b/include/sysemu/arch_init.h
index 54b36c1..86344a2 100644
--- a/include/sysemu/arch_init.h
+++ b/include/sysemu/arch_init.h
@@ -37,5 +37,6 @@ int kvm_available(void);
 int xen_available(void);
 
 CpuDefinitionInfoList *arch_query_cpu_definitions(Error **errp);
+CpuModelInfo *arch_query_cpu_model(Error **errp);
 
 #endif
diff --git a/qapi-schema.json b/qapi-schema.json
index ac9594d..14d294f 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -2516,6 +2516,16 @@
 { 'command': 'query-machines', 'returns': ['MachineInfo'] }
 
 ##
+# @AccelId
+#
+# Defines accelerator ids
+#
+# Since: 2.4
+##
+{ 'enum': 'AccelId',
+  'data': ['qtest', 'tcg', 'kvm', 'xen'  ] }
+
+##
 # @CpuDefinitionInfo:
 #
 # Virtual CPU definition.
@@ -2538,6 +2548,31 @@
 ##
 { 'command': 'query-cpu-definitions', 'returns': ['CpuDefinitionInfo'] }
 
+##
+# @CpuModelInfo:
+#
+# Virtual CPU model definition.
+#
+# @name: the name of the CPU model definition
+#
+# @accel: AccelId (name) of this cpu models accelerator
+#
+# Since: 2.4
+##
+{ 'type': 'CpuModelInfo',
+  'data': { 'name': 'str', 'accel': 'AccelId' } }
+
+##
+# @query-cpu-model:
+#
+# Return the current virtual CPU model
+#
+# Returns: CpuModelInfo
+#
+# Since: 2.4
+##
+{ 'command': 'query-cpu-model', 'returns': 'CpuModelInfo' }
+
 # @AddfdInfo:
 #
 # Information about a file descriptor that was added to an fd set.
diff --git a/qmp-commands.hx b/qmp-commands.hx
index 3a42ad0..8fe577f 100644
--- a/qmp-commands.hx
+++ b/qmp-commands.hx
@@ -3417,6 +3417,12 @@ EQMP
     },
 
     {
+        .name       = "query-cpu-model",
+        .args_type  = "",
+        .mhandler.cmd_new = qmp_marshal_input_query_cpu_model,
+    },
+
+    {
         .name       = "query-target",
         .args_type  = "",
         .mhandler.cmd_new = qmp_marshal_input_query_target,
diff --git a/qmp.c b/qmp.c
index c479e77..ad63803 100644
--- a/qmp.c
+++ b/qmp.c
@@ -572,6 +572,11 @@ CpuDefinitionInfoList *qmp_query_cpu_definitions(Error **errp)
     return arch_query_cpu_definitions(errp);
 }
 
+CpuModelInfo *qmp_query_cpu_model(Error **errp)
+{
+    return arch_query_cpu_model(errp);
+}
+
 void qmp_add_client(const char *protocol, const char *fdname,
                     bool has_skipauth, bool skipauth, bool has_tls, bool tls,
                     Error **errp)
diff --git a/stubs/Makefile.objs b/stubs/Makefile.objs
index dce9cd2..ca70d5d 100644
--- a/stubs/Makefile.objs
+++ b/stubs/Makefile.objs
@@ -1,4 +1,5 @@
 stub-obj-y += arch-query-cpu-def.o
+stub-obj-y += arch-query-cpu-mod.o
 stub-obj-y += bdrv-commit-all.o
 stub-obj-y += chr-baum-init.o
 stub-obj-y += chr-msmouse.o
diff --git a/stubs/arch-query-cpu-mod.c b/stubs/arch-query-cpu-mod.c
new file mode 100644
index 0000000..90ebd08
--- /dev/null
+++ b/stubs/arch-query-cpu-mod.c
@@ -0,0 +1,9 @@
+#include "qemu-common.h"
+#include "sysemu/arch_init.h"
+#include "qapi/qmp/qerror.h"
+
+CpuModelInfo *arch_query_cpu_model(Error **errp)
+{
+    error_set(errp, QERR_UNSUPPORTED);
+    return NULL;
+}
diff --git a/target-s390x/cpu-models.c b/target-s390x/cpu-models.c
index ba873ea..187d110 100644
--- a/target-s390x/cpu-models.c
+++ b/target-s390x/cpu-models.c
@@ -707,3 +707,17 @@ void s390_cpu_model_init(S390CPUClass *cc)
         }
     }
 }
+
+/**
+ * s390_cpu_models_used:
+ *
+ * This function indicates if cpus with model properties are in use.
+ *
+ * Returns: a boolean value.
+ *
+ * Since: 2.4
+ */
+bool s390_cpu_models_used(void)
+{
+    return cpu_models_used;
+}
diff --git a/target-s390x/cpu-models.h b/target-s390x/cpu-models.h
index fe3997f..67f0519 100644
--- a/target-s390x/cpu-models.h
+++ b/target-s390x/cpu-models.h
@@ -112,6 +112,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_cpu_models_used(void);
 
 extern uint64_t qemu_s390_fac_list_mask[];
 
diff --git a/target-s390x/cpu.c b/target-s390x/cpu.c
index 829945d..1698b52 100644
--- a/target-s390x/cpu.c
+++ b/target-s390x/cpu.c
@@ -37,6 +37,11 @@
 #define CR0_RESET       0xE0UL
 #define CR14_RESET      0xC2000000UL;
 
+static inline char *strdup_s390_cpu_name(S390CPUClass *cc)
+{
+    return g_strdup_printf("%04x-ga%u", cc->proc.type, cc->mach.ga);
+}
+
 /* generate CPU information for cpu -? */
 void s390_cpu_list(FILE *f, fprintf_function cpu_fprintf)
 {
@@ -74,6 +79,30 @@ CpuDefinitionInfoList *arch_query_cpu_definitions(Error **errp)
 
     return entry;
 }
+
+CpuModelInfo *arch_query_cpu_model(Error **errp)
+{
+    CpuModelInfo *info;
+    S390CPUClass *cc;
+
+    if (!s390_cpu_models_used()) {
+        return NULL;
+    }
+    info = g_try_new0(CpuModelInfo, 1);
+    if (!info) {
+        return NULL;
+    }
+    cc = S390_CPU_GET_CLASS(s390_cpu_addr2state(0));
+    info->name = strdup_s390_cpu_name(cc);
+    if (!info->name) {
+        g_free(info);
+        return NULL;
+    }
+    if (kvm_enabled()) {
+        info->accel = ACCEL_ID_KVM;
+    }
+    return info;
+}
 #endif
 
 static void s390_cpu_set_pc(CPUState *cs, vaddr value)
-- 
1.8.3.1


^ permalink raw reply related	[flat|nested] 45+ messages in thread

* [PATCH v4 12/15] Add optional parameters to QMP command query-cpu-definitions
  2015-03-30 14:28 [PATCH v4 00/15] s390x cpu model implementation Michael Mueller
                   ` (10 preceding siblings ...)
  2015-03-30 14:28 ` [PATCH v4 11/15] target-s390x: New QMP command query-cpu-model Michael Mueller
@ 2015-03-30 14:28 ` Michael Mueller
  2015-03-30 20:28   ` [Qemu-devel] " Eric Blake
  2015-03-31 19:46   ` Eduardo Habkost
  2015-03-30 14:28 ` [PATCH v4 13/15] target-s390x: Extend " Michael Mueller
                   ` (2 subsequent siblings)
  14 siblings, 2 replies; 45+ messages in thread
From: Michael Mueller @ 2015-03-30 14:28 UTC (permalink / raw)
  To: qemu-devel, kvm, linux-s390, linux-kernel
  Cc: Eduardo Habkost, Gleb Natapov, Alexander Graf,
	Christian Borntraeger, Jason J. Herne, Cornelia Huck,
	Paolo Bonzini, Michael Mueller, Andreas Faerber,
	Richard Henderson, Daniel Hansel

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>
---
 include/sysemu/arch_init.h  |  6 +++++-
 qapi-schema.json            | 14 ++++++++++++--
 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, 51 insertions(+), 12 deletions(-)

diff --git a/include/sysemu/arch_init.h b/include/sysemu/arch_init.h
index 86344a2..ab48c35 100644
--- a/include/sysemu/arch_init.h
+++ b/include/sysemu/arch_init.h
@@ -36,7 +36,11 @@ 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);
 CpuModelInfo *arch_query_cpu_model(Error **errp);
 
 #endif
diff --git a/qapi-schema.json b/qapi-schema.json
index 14d294f..61a145d 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -2532,21 +2532,31 @@
 #
 # @name: the name of the CPU definition
 #
+# @default: #optional defines if cpu model is the default (since 2.4)
+#
+# @runnable: #optional defines if cpu model is runnable (since 2.4)
+#
 # Since: 1.2.0
 ##
 { 'type': 'CpuDefinitionInfo',
-  'data': { 'name': 'str' } }
+  'data': { 'name': 'str', '*is-default': 'bool', '*runnable': 'bool' } }
 
 ##
 # @query-cpu-definitions:
 #
 # Return a list of supported virtual CPU definitions
 #
+# @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'] }
 
 ##
 # @CpuModelInfo:
diff --git a/qmp-commands.hx b/qmp-commands.hx
index 8fe577f..ed86773d 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 ad63803..ad52fb3 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);
 }
 
 CpuModelInfo *qmp_query_cpu_model(Error **errp)
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 10886c5..d56d47a 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 b2d1c95..c7fa98e 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 1698b52..615173f 100644
--- a/target-s390x/cpu.c
+++ b/target-s390x/cpu.c
@@ -66,7 +66,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] 45+ messages in thread

* [PATCH v4 13/15] target-s390x: Extend QMP command query-cpu-definitions
  2015-03-30 14:28 [PATCH v4 00/15] s390x cpu model implementation Michael Mueller
                   ` (11 preceding siblings ...)
  2015-03-30 14:28 ` [PATCH v4 12/15] Add optional parameters to QMP command query-cpu-definitions Michael Mueller
@ 2015-03-30 14:28 ` Michael Mueller
  2015-03-30 19:54   ` Eduardo Habkost
  2015-03-30 14:28 ` [PATCH v4 14/15] target-s390x: Introduce facility test routine Michael Mueller
  2015-03-30 14:28 ` [PATCH v4 15/15] target-s390x: Enable cpu model usage Michael Mueller
  14 siblings, 1 reply; 45+ messages in thread
From: Michael Mueller @ 2015-03-30 14:28 UTC (permalink / raw)
  To: qemu-devel, kvm, linux-s390, linux-kernel
  Cc: Eduardo Habkost, Gleb Natapov, Alexander Graf,
	Christian Borntraeger, Jason J. Herne, Cornelia Huck,
	Paolo Bonzini, Michael Mueller, Andreas Faerber,
	Richard Henderson, Daniel Hansel

This patch implements the QMP command 'query-cpu-definitions' in the S390
context. The command returns a list of cpu model names in the current host
context. A consumer may successfully request each listed cpu model as long
for a given accelerator and machine this model is runnable.

request:
  {"execute": "query-cpu-definitions",
              "arguments": { "accel": "kvm",
                             "machine": "s390-ccw-virtio" }
  }

answer:
  {"return": [ {"name":"2964-ga1","runnable":false,"is-default":false},
               {"name":"2828-ga1","runnable":true,"is-default":false},
               {"name":"2827-ga2","runnable":true,"is-default":true},
               {"name":"2827-ga1","runnable":true,"is-default":false},
               ...
               {"name":"2064-ga1","runnable":true,"is-default":false} ]
  }

The request arguments are optional and if omitted only the cpu model names
are returned without the runnable or is-default attributes.

Signed-off-by: Michael Mueller <mimu@linux.vnet.ibm.com>
---
 target-s390x/cpu-models.c |  15 ++++++
 target-s390x/cpu-models.h |   7 +++
 target-s390x/cpu.c        | 114 +++++++++++++++++++++++++++++++++++++++++++---
 target-s390x/kvm.c        |  49 +++++++++++++++++++-
 4 files changed, 177 insertions(+), 8 deletions(-)

diff --git a/target-s390x/cpu-models.c b/target-s390x/cpu-models.c
index 187d110..aba44dd 100644
--- a/target-s390x/cpu-models.c
+++ b/target-s390x/cpu-models.c
@@ -215,6 +215,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 67f0519..4734c58 100644
--- a/target-s390x/cpu-models.h
+++ b/target-s390x/cpu-models.h
@@ -85,10 +85,16 @@ typedef struct S390MachineProps {
 } S390MachineProps;
 
 #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;
@@ -107,6 +113,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);
diff --git a/target-s390x/cpu.c b/target-s390x/cpu.c
index 615173f..0ccacb7 100644
--- a/target-s390x/cpu.c
+++ b/target-s390x/cpu.c
@@ -30,13 +30,21 @@
 #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
 
 #define CR0_RESET       0xE0UL
 #define CR14_RESET      0xC2000000UL;
 
+typedef struct CpuDefParm {
+    CpuDefinitionInfoList *list;
+    bool showFullEntry;
+} CpuDefParm;
+
 static inline char *strdup_s390_cpu_name(S390CPUClass *cc)
 {
     return g_strdup_printf("%04x-ga%u", cc->proc.type, cc->mach.ga);
@@ -66,22 +74,114 @@ 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;
+    CpuDefinitionInfo *info;
+    CpuDefParm *parm = user_data;
+
+    if (!strcmp(object_class_get_name(oc), TYPE_S390_CPU)) {
+        return;
+    }
+    info = g_try_new0(CpuDefinitionInfo, 1);
+    if (!info) {
+        goto out;
+    }
+    info->name = strdup_s390_cpu_name(cc);
+    if (parm->showFullEntry) {
+        info->runnable = cc->is_active[ACCEL_TEMP];
+        info->has_runnable = true;
+        info->is_default = cc->is_host[ACCEL_TEMP];
+        info->has_is_default = true;
+    }
+
+    entry = g_try_new0(CpuDefinitionInfoList, 1);
+    if (!entry) {
+        goto out;
+    }
+    entry->value = info;
+    entry->next = parm->list;
+    parm->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;
+    CpuDefParm parm = {
+        .list = NULL,
+        .showFullEntry = false,
+    };
 
-    info = g_malloc0(sizeof(*info));
-    info->name = g_strdup("host");
+    if (!s390_cpu_models_used()) {
+        return qmp_query_cpu_definition_host();
+    }
 
-    entry = g_malloc0(sizeof(*entry));
-    entry->value = info;
+    if (!has_accel || !has_machine) {
+        goto out;
+    }
+
+    switch (accel) {
+    case ACCEL_ID_KVM:
+        if (kvm_s390_get_machine_props(NULL, &mach) < 0) {
+            return qmp_query_cpu_definition_host();
+        }
+        break;
+    default:
+        goto out_empty;
+    }
+
+    mask = s390_fac_list_mask_by_machine(machine);
+    if (!mask) {
+        error_set(errp, QERR_INVALID_PARAMETER_VALUE, "machine",
+                  "a valid machine type");
+        goto out_empty;
+    }
 
-    return entry;
+    s390_setup_cpu_classes(ACCEL_TEMP, &mach, mask);
+    parm.showFullEntry = true;
+out:
+    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, &parm);
+    g_slist_free(classes);
+out_empty:
+    return parm.list;
 }
 
 CpuModelInfo *arch_query_cpu_model(Error **errp)
diff --git a/target-s390x/kvm.c b/target-s390x/kvm.c
index 9b2cfeb..f40e879 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, uint64_t 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, (uint64_t) prop);
+    } else {
+        rc = get_machine_props_fallback(prop);
     }
     trace_kvm_get_machine_props(rc, prop->cpuid, prop->ibc_range);
 
-- 
1.8.3.1


^ permalink raw reply related	[flat|nested] 45+ messages in thread

* [PATCH v4 14/15] target-s390x: Introduce facility test routine
  2015-03-30 14:28 [PATCH v4 00/15] s390x cpu model implementation Michael Mueller
                   ` (12 preceding siblings ...)
  2015-03-30 14:28 ` [PATCH v4 13/15] target-s390x: Extend " Michael Mueller
@ 2015-03-30 14:28 ` Michael Mueller
  2015-03-30 14:28 ` [PATCH v4 15/15] target-s390x: Enable cpu model usage Michael Mueller
  14 siblings, 0 replies; 45+ messages in thread
From: Michael Mueller @ 2015-03-30 14:28 UTC (permalink / raw)
  To: qemu-devel, kvm, linux-s390, linux-kernel
  Cc: Eduardo Habkost, Gleb Natapov, Alexander Graf,
	Christian Borntraeger, Jason J. Herne, Cornelia Huck,
	Paolo Bonzini, Michael Mueller, Andreas Faerber,
	Richard Henderson, Daniel Hansel

The patch introduces routine s390_facility_test() which allows to
verify a specific facility bit is set.

Signed-off-by: Michael Mueller <mimu@linux.vnet.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 aba44dd..a9f7372 100644
--- a/target-s390x/cpu-models.c
+++ b/target-s390x/cpu-models.c
@@ -736,3 +736,32 @@ bool s390_cpu_models_used(void)
 {
     return cpu_models_used;
 }
+
+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 4734c58..eb0476f 100644
--- a/target-s390x/cpu-models.h
+++ b/target-s390x/cpu-models.h
@@ -120,6 +120,7 @@ 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_cpu_models_used(void);
+bool s390_test_facility(S390CPUClass *cc, uint16_t nr);
 
 extern uint64_t qemu_s390_fac_list_mask[];
 
-- 
1.8.3.1


^ permalink raw reply related	[flat|nested] 45+ messages in thread

* [PATCH v4 15/15] target-s390x: Enable cpu model usage
  2015-03-30 14:28 [PATCH v4 00/15] s390x cpu model implementation Michael Mueller
                   ` (13 preceding siblings ...)
  2015-03-30 14:28 ` [PATCH v4 14/15] target-s390x: Introduce facility test routine Michael Mueller
@ 2015-03-30 14:28 ` Michael Mueller
  14 siblings, 0 replies; 45+ messages in thread
From: Michael Mueller @ 2015-03-30 14:28 UTC (permalink / raw)
  To: qemu-devel, kvm, linux-s390, linux-kernel
  Cc: Eduardo Habkost, Gleb Natapov, Alexander Graf,
	Christian Borntraeger, Jason J. Herne, Cornelia Huck,
	Paolo Bonzini, Michael Mueller, Andreas Faerber,
	Richard Henderson, Daniel Hansel

This patch enables QEMU to instantiate S390 CPUs with cpu model types.

Signed-off-by: Michael Mueller <mimu@linux.vnet.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] 45+ messages in thread

* Re: [PATCH v4 10/15] target-s390x: Prepare accelerator during cpu object realization
  2015-03-30 14:28 ` [PATCH v4 10/15] target-s390x: Prepare accelerator during cpu object realization Michael Mueller
@ 2015-03-30 19:33   ` Eduardo Habkost
  2015-03-31 10:26     ` [Qemu-devel] " Michael Mueller
  0 siblings, 1 reply; 45+ messages in thread
From: Eduardo Habkost @ 2015-03-30 19:33 UTC (permalink / raw)
  To: Michael Mueller
  Cc: qemu-devel, kvm, linux-s390, linux-kernel, Gleb Natapov,
	Alexander Graf, Christian Borntraeger, Jason J. Herne,
	Cornelia Huck, Paolo Bonzini, Andreas Faerber, Richard Henderson,
	Daniel Hansel

On Mon, Mar 30, 2015 at 04:28:23PM +0200, Michael Mueller wrote:
> 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>
> ---
>  target-s390x/cpu-models.c | 37 +++++++++++++++++++++++++++++++++++++
>  target-s390x/cpu-models.h |  4 ++++
>  target-s390x/cpu.c        |  1 +
>  3 files changed, 42 insertions(+)
> 
> diff --git a/target-s390x/cpu-models.c b/target-s390x/cpu-models.c
> index 8a877d3..ba873ea 100644
> --- a/target-s390x/cpu-models.c
> +++ b/target-s390x/cpu-models.c
> @@ -111,6 +111,7 @@ typedef struct ParmAddrAddrModeMask {
>  } ParmAddrAddrModeMask;
>  
>  static GSList *s390_cpu_aliases;
> +static bool cpu_models_used;
>  
>  /* compare order of two cpu classes for ascending sort */
>  gint s390_cpu_class_asc_order_compare(gconstpointer a, gconstpointer b)
> @@ -670,3 +671,39 @@ 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;
> +
> +    /* none cpu model case */
> +    if (!strcmp(object_class_get_name(OBJECT_CLASS(cc)), TYPE_S390_CPU)) {
> +        return;
> +    }

Instead of checking the class name, can't you just check a S390CPUClass
field that may indicate that s390_cpu_model_init() doesn't need to do
anything for the class? Maybe (cpuid(cc->proc) == 0)?


> +
> +    /* accelerator already prepared */
> +    if (cpu_models_used) {
> +        return;
> +    }

I'm still trying to understand the need for this global. But I will ask
for more details in the following patches that actually use
s390_cpu_models_used()?

> +
> +    proc.cpuid = cpuid(cc->proc);
> +    proc.ibc = cc->proc.ibc;
> +    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()) {
> +        if (!kvm_s390_set_processor_props(&proc)) {
> +            cpu_models_used = true;
> +        }
> +    }
> +}
> diff --git a/target-s390x/cpu-models.h b/target-s390x/cpu-models.h
> index 9562088..fe3997f 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)
> @@ -108,6 +111,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);
>  
>  extern uint64_t qemu_s390_fac_list_mask[];
>  
> diff --git a/target-s390x/cpu.c b/target-s390x/cpu.c
> index c33f05e..829945d 100644
> --- a/target-s390x/cpu.c
> +++ b/target-s390x/cpu.c
> @@ -180,6 +180,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
> 

-- 
Eduardo

^ permalink raw reply	[flat|nested] 45+ messages in thread

* Re: [PATCH v4 07/15] target-s390x: Update linux-headers/asm-s390/kvm.h
  2015-03-30 14:28 ` [PATCH v4 07/15] target-s390x: Update linux-headers/asm-s390/kvm.h Michael Mueller
@ 2015-03-30 19:36   ` Christian Borntraeger
  2015-03-31  7:25     ` Michael Mueller
  0 siblings, 1 reply; 45+ messages in thread
From: Christian Borntraeger @ 2015-03-30 19:36 UTC (permalink / raw)
  To: Michael Mueller, qemu-devel, kvm, linux-s390, linux-kernel
  Cc: Eduardo Habkost, Gleb Natapov, Alexander Graf, Jason J. Herne,
	Cornelia Huck, Paolo Bonzini, Andreas Faerber, Richard Henderson,
	Daniel Hansel

Am 30.03.2015 um 16:28 schrieb Michael Mueller:
> Signed-off-by: Michael Mueller <mimu@linux.vnet.ibm.com>
> ---
>  linux-headers/asm-s390/kvm.h | 18 +++++++++---------
>  1 file changed, 9 insertions(+), 9 deletions(-)
> 

Looks like a leftover. Drop that patch and rename ibc_range to ibc in the other patch.


> diff --git a/linux-headers/asm-s390/kvm.h b/linux-headers/asm-s390/kvm.h
> index c5a93eb..bfe6925 100644
> --- a/linux-headers/asm-s390/kvm.h
> +++ b/linux-headers/asm-s390/kvm.h
> @@ -70,8 +70,14 @@ struct kvm_s390_io_adapter_req {
>  #define KVM_S390_VM_TOD_LOW		0
>  #define KVM_S390_VM_TOD_HIGH		1
> 
> +/* kvm attributes for crypto */
> +#define KVM_S390_VM_CRYPTO_ENABLE_AES_KW	0
> +#define KVM_S390_VM_CRYPTO_ENABLE_DEA_KW	1
> +#define KVM_S390_VM_CRYPTO_DISABLE_AES_KW	2
> +#define KVM_S390_VM_CRYPTO_DISABLE_DEA_KW	3
> +
>  /* kvm attributes for KVM_S390_VM_CPU_MODEL */
> -/* processor related attributes are r/w */
> +/* kvm S390 processor related attributes are r/w */
>  #define KVM_S390_VM_CPU_PROCESSOR	0
>  struct kvm_s390_vm_cpu_processor {
>  	__u64 cpuid;
> @@ -80,22 +86,16 @@ struct kvm_s390_vm_cpu_processor {
>  	__u64 fac_list[256];
>  };
> 
> -/* machine related attributes are r/o */
> +/* kvm S390 machine related attributes are r/o */
>  #define KVM_S390_VM_CPU_MACHINE		1
>  struct kvm_s390_vm_cpu_machine {
>  	__u64 cpuid;
> -	__u32 ibc;
> +	__u32 ibc_range;
>  	__u8  pad[4];
>  	__u64 fac_mask[256];
>  	__u64 fac_list[256];
>  };
> 
> -/* kvm attributes for crypto */
> -#define KVM_S390_VM_CRYPTO_ENABLE_AES_KW	0
> -#define KVM_S390_VM_CRYPTO_ENABLE_DEA_KW	1
> -#define KVM_S390_VM_CRYPTO_DISABLE_AES_KW	2
> -#define KVM_S390_VM_CRYPTO_DISABLE_DEA_KW	3
> -
>  /* for KVM_GET_REGS and KVM_SET_REGS */
>  struct kvm_regs {
>  	/* general purpose regs for s390 */
> 


^ permalink raw reply	[flat|nested] 45+ messages in thread

* Re: [PATCH v4 11/15] target-s390x: New QMP command query-cpu-model
  2015-03-30 14:28 ` [PATCH v4 11/15] target-s390x: New QMP command query-cpu-model Michael Mueller
@ 2015-03-30 19:50   ` Eduardo Habkost
  2015-03-31  9:10     ` [Qemu-devel] " Michael Mueller
  2015-03-30 20:17   ` Eduardo Habkost
                     ` (2 subsequent siblings)
  3 siblings, 1 reply; 45+ messages in thread
From: Eduardo Habkost @ 2015-03-30 19:50 UTC (permalink / raw)
  To: Michael Mueller
  Cc: qemu-devel, kvm, linux-s390, linux-kernel, Gleb Natapov,
	Alexander Graf, Christian Borntraeger, Jason J. Herne,
	Cornelia Huck, Paolo Bonzini, Andreas Faerber, Richard Henderson,
	Daniel Hansel

On Mon, Mar 30, 2015 at 04:28:24PM +0200, Michael Mueller wrote:
[...]
> diff --git a/target-s390x/cpu.c b/target-s390x/cpu.c
> index 829945d..1698b52 100644
> --- a/target-s390x/cpu.c
> +++ b/target-s390x/cpu.c
> @@ -37,6 +37,11 @@
>  #define CR0_RESET       0xE0UL
>  #define CR14_RESET      0xC2000000UL;
>  
> +static inline char *strdup_s390_cpu_name(S390CPUClass *cc)
> +{
> +    return g_strdup_printf("%04x-ga%u", cc->proc.type, cc->mach.ga);
> +}
> +
>  /* generate CPU information for cpu -? */
>  void s390_cpu_list(FILE *f, fprintf_function cpu_fprintf)
>  {
> @@ -74,6 +79,30 @@ CpuDefinitionInfoList *arch_query_cpu_definitions(Error **errp)
>  
>      return entry;
>  }
> +
> +CpuModelInfo *arch_query_cpu_model(Error **errp)
> +{
> +    CpuModelInfo *info;
> +    S390CPUClass *cc;
> +
> +    if (!s390_cpu_models_used()) {
> +        return NULL;
> +    }

First question is: why you don't want to just return
  { name: "none", accel: ...  }
when TYPE_S390_CPU ("-cpu none") is used?

Now, assuming that you really want to return NULL if -cpu none is used,
why don't you just ask for the first CPU (like you already do below) and
check if it is a named CPU model, instead of adding a new global? e.g.:

    static bool s390_cpu_class_is_model(S390CPUClass *cc)
    {
        return cpuid(cc->proc) != 0;
    }
    
    CpuModelInfo *arch_query_cpu_model(Error **errp)
    {
        S390CPUClass *cc;
        S390CPUClass *cc;

        cc = S390_CPU_GET_CLASS(s390_cpu_addr2state(0));
        if (!s390_cpu_class_is_model(cc)) {
            return NULL;
        }
        [...]
    }


> +    info = g_try_new0(CpuModelInfo, 1);
> +    if (!info) {
> +        return NULL;
> +    }
> +    cc = S390_CPU_GET_CLASS(s390_cpu_addr2state(0));
> +    info->name = strdup_s390_cpu_name(cc);

I don't think the current version of strdup_s390_cpu_name() can ever
return NULL. Do you expect it to return NULL in the future?

> +    if (!info->name) {
> +        g_free(info);
> +        return NULL;
> +    }
> +    if (kvm_enabled()) {
> +        info->accel = ACCEL_ID_KVM;

This could be a CPUState field, added automatically by
cpu_generic_init() using current_machine->accel.

> +    }
> +    return info;
> +}
>  #endif
>  
>  static void s390_cpu_set_pc(CPUState *cs, vaddr value)
> -- 
> 1.8.3.1
> 

-- 
Eduardo

^ permalink raw reply	[flat|nested] 45+ messages in thread

* Re: [PATCH v4 13/15] target-s390x: Extend QMP command query-cpu-definitions
  2015-03-30 14:28 ` [PATCH v4 13/15] target-s390x: Extend " Michael Mueller
@ 2015-03-30 19:54   ` Eduardo Habkost
  0 siblings, 0 replies; 45+ messages in thread
From: Eduardo Habkost @ 2015-03-30 19:54 UTC (permalink / raw)
  To: Michael Mueller
  Cc: qemu-devel, kvm, linux-s390, linux-kernel, Gleb Natapov,
	Alexander Graf, Christian Borntraeger, Jason J. Herne,
	Cornelia Huck, Paolo Bonzini, Andreas Faerber, Richard Henderson,
	Daniel Hansel

On Mon, Mar 30, 2015 at 04:28:26PM +0200, Michael Mueller wrote:
[...]
>  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;
> +    CpuDefParm parm = {
> +        .list = NULL,
> +        .showFullEntry = false,
> +    };
>  
> -    info = g_malloc0(sizeof(*info));
> -    info->name = g_strdup("host");
> +    if (!s390_cpu_models_used()) {
> +        return qmp_query_cpu_definition_host();
> +    }

Why exactly are you returning different results depending on a global
set by machine initialization? I expect query-cpu-definitions to always
return the same data, no matter what were the QEMU command-line options
used to initialize the machine.

>  
> -    entry = g_malloc0(sizeof(*entry));
> -    entry->value = info;
> +    if (!has_accel || !has_machine) {
> +        goto out;
> +    }
> +
> +    switch (accel) {
> +    case ACCEL_ID_KVM:
> +        if (kvm_s390_get_machine_props(NULL, &mach) < 0) {
> +            return qmp_query_cpu_definition_host();
> +        }
> +        break;
> +    default:
> +        goto out_empty;
> +    }
> +
> +    mask = s390_fac_list_mask_by_machine(machine);
> +    if (!mask) {
> +        error_set(errp, QERR_INVALID_PARAMETER_VALUE, "machine",
> +                  "a valid machine type");
> +        goto out_empty;
> +    }
>  
> -    return entry;
> +    s390_setup_cpu_classes(ACCEL_TEMP, &mach, mask);
> +    parm.showFullEntry = true;
> +out:
> +    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, &parm);
> +    g_slist_free(classes);
> +out_empty:
> +    return parm.list;
>  }
>  

-- 
Eduardo

^ permalink raw reply	[flat|nested] 45+ messages in thread

* Re: [PATCH v4 11/15] target-s390x: New QMP command query-cpu-model
  2015-03-30 14:28 ` [PATCH v4 11/15] target-s390x: New QMP command query-cpu-model Michael Mueller
  2015-03-30 19:50   ` Eduardo Habkost
@ 2015-03-30 20:17   ` Eduardo Habkost
  2015-03-30 20:20     ` [Qemu-devel] " Eric Blake
  2015-03-31 11:21     ` Michael Mueller
  2015-03-30 20:19   ` Eric Blake
  2015-03-31 18:35   ` Eduardo Habkost
  3 siblings, 2 replies; 45+ messages in thread
From: Eduardo Habkost @ 2015-03-30 20:17 UTC (permalink / raw)
  To: Michael Mueller
  Cc: qemu-devel, kvm, linux-s390, linux-kernel, Gleb Natapov,
	Alexander Graf, Christian Borntraeger, Jason J. Herne,
	Cornelia Huck, Paolo Bonzini, Andreas Faerber, Richard Henderson,
	Daniel Hansel

On Mon, Mar 30, 2015 at 04:28:24PM +0200, Michael Mueller wrote:
> This patch implements a new QMP request named 'query-cpu-model'.
> It returns the cpu model of cpu 0 and its backing accelerator.
> 
> request:
>   {"execute" : "query-cpu-model" }
> 
> answer:
>   {"return" : {"name": "2827-ga2", "accel": "kvm" }}

If you are returning information about an existing CPU, why not just
extend the output of "query-cpus"?

(Existing qmp_query_cpus() calls cpu_synchronize_state(), which may be
undesired. But in this case we could add an optional parameter to
disable the return of data that requires stopping the VCPU).

> 
> Alias names are resolved to their respective machine type and GA names
> already during cpu instantiation. Thus, also a cpu model like 'host'
> which is implemented as alias will return its normalized cpu model name.
> 
> Furthermore the patch implements the following function:
> 
> - s390_cpu_models_used(), returns true if S390 cpu models are in use
> 
> Signed-off-by: Michael Mueller <mimu@linux.vnet.ibm.com>
[...]

-- 
Eduardo

^ permalink raw reply	[flat|nested] 45+ messages in thread

* Re: [Qemu-devel] [PATCH v4 11/15] target-s390x: New QMP command query-cpu-model
  2015-03-30 14:28 ` [PATCH v4 11/15] target-s390x: New QMP command query-cpu-model Michael Mueller
  2015-03-30 19:50   ` Eduardo Habkost
  2015-03-30 20:17   ` Eduardo Habkost
@ 2015-03-30 20:19   ` Eric Blake
  2015-03-31  7:56     ` Michael Mueller
  2015-03-31 18:35   ` Eduardo Habkost
  3 siblings, 1 reply; 45+ messages in thread
From: Eric Blake @ 2015-03-30 20:19 UTC (permalink / raw)
  To: Michael Mueller, qemu-devel, kvm, linux-s390, linux-kernel
  Cc: Eduardo Habkost, Gleb Natapov, Alexander Graf,
	Christian Borntraeger, Daniel Hansel, Jason J. Herne,
	Cornelia Huck, Paolo Bonzini, Richard Henderson, Andreas Faerber

[-- Attachment #1: Type: text/plain, Size: 1461 bytes --]

On 03/30/2015 08:28 AM, Michael Mueller wrote:
> This patch implements a new QMP request named 'query-cpu-model'.
> It returns the cpu model of cpu 0 and its backing accelerator.
> 
> request:
>   {"execute" : "query-cpu-model" }
> 
> answer:
>   {"return" : {"name": "2827-ga2", "accel": "kvm" }}
> 
> Alias names are resolved to their respective machine type and GA names
> already during cpu instantiation. Thus, also a cpu model like 'host'
> which is implemented as alias will return its normalized cpu model name.
> 
> Furthermore the patch implements the following function:
> 
> - s390_cpu_models_used(), returns true if S390 cpu models are in use
> 
> Signed-off-by: Michael Mueller <mimu@linux.vnet.ibm.com>
> ---

> +++ b/qapi-schema.json
> @@ -2516,6 +2516,16 @@
>  { 'command': 'query-machines', 'returns': ['MachineInfo'] }
>  
>  ##
> +# @AccelId
> +#
> +# Defines accelerator ids
> +#
> +# Since: 2.4
> +##
> +{ 'enum': 'AccelId',
> +  'data': ['qtest', 'tcg', 'kvm', 'xen'  ] }

Unusual spacing (0 spaces after '[' but 2 spaces before closing ']'?),
but not necessarily wrong.


> +##
> +# @CpuModelInfo:
> +#
> +# Virtual CPU model definition.
> +#
> +# @name: the name of the CPU model definition
> +#
> +# @accel: AccelId (name) of this cpu models accelerator

s/models/model's/


-- 
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] 45+ messages in thread

* Re: [Qemu-devel] [PATCH v4 11/15] target-s390x: New QMP command query-cpu-model
  2015-03-30 20:17   ` Eduardo Habkost
@ 2015-03-30 20:20     ` Eric Blake
  2015-03-31 13:16       ` Eduardo Habkost
  2015-03-31 11:21     ` Michael Mueller
  1 sibling, 1 reply; 45+ messages in thread
From: Eric Blake @ 2015-03-30 20:20 UTC (permalink / raw)
  To: Eduardo Habkost, Michael Mueller
  Cc: linux-s390, kvm, Gleb Natapov, linux-kernel, Alexander Graf,
	qemu-devel, Christian Borntraeger, Daniel Hansel, Jason J. Herne,
	Cornelia Huck, Paolo Bonzini, Andreas Faerber, Richard Henderson

[-- Attachment #1: Type: text/plain, Size: 1101 bytes --]

On 03/30/2015 02:17 PM, Eduardo Habkost wrote:
> On Mon, Mar 30, 2015 at 04:28:24PM +0200, Michael Mueller wrote:
>> This patch implements a new QMP request named 'query-cpu-model'.
>> It returns the cpu model of cpu 0 and its backing accelerator.
>>
>> request:
>>   {"execute" : "query-cpu-model" }
>>
>> answer:
>>   {"return" : {"name": "2827-ga2", "accel": "kvm" }}
> 
> If you are returning information about an existing CPU, why not just
> extend the output of "query-cpus"?
> 
> (Existing qmp_query_cpus() calls cpu_synchronize_state(), which may be
> undesired. But in this case we could add an optional parameter to
> disable the return of data that requires stopping the VCPU).

And how would libvirt learn about the existence of that optional
parameter?  Without introspection, a new command is easier to query than
learning about whether an optional parameter exists (then again, we're
hoping to get introspection into 2.4, so it may be a moot question).

-- 
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] 45+ messages in thread

* Re: [Qemu-devel] [PATCH v4 12/15] Add optional parameters to QMP command query-cpu-definitions
  2015-03-30 14:28 ` [PATCH v4 12/15] Add optional parameters to QMP command query-cpu-definitions Michael Mueller
@ 2015-03-30 20:28   ` Eric Blake
  2015-03-31  7:42     ` Michael Mueller
  2015-03-31 19:46   ` Eduardo Habkost
  1 sibling, 1 reply; 45+ messages in thread
From: Eric Blake @ 2015-03-30 20:28 UTC (permalink / raw)
  To: Michael Mueller, qemu-devel, kvm, linux-s390, linux-kernel
  Cc: Eduardo Habkost, Gleb Natapov, Alexander Graf,
	Christian Borntraeger, Daniel Hansel, Jason J. Herne,
	Cornelia Huck, Paolo Bonzini, Richard Henderson, Andreas Faerber

[-- Attachment #1: Type: text/plain, Size: 1461 bytes --]

On 03/30/2015 08:28 AM, Michael Mueller wrote:
> 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>
> ---

> +++ b/qapi-schema.json
> @@ -2532,21 +2532,31 @@
>  #
>  # @name: the name of the CPU definition
>  #
> +# @default: #optional defines if cpu model is the default (since 2.4)

Reads poorly.  How about:

# @default: #optional true if cpu model is the default, omitted if false
(since 2.4)


> +#
> +# @runnable: #optional defines if cpu model is runnable (since 2.4)

Similarly:

# @runnable: #optional true if cpu model is runnable, omitted if false
(since 2.4)

> +#
>  # Since: 1.2.0
>  ##
>  { 'type': 'CpuDefinitionInfo',
> -  'data': { 'name': 'str' } }
> +  'data': { 'name': 'str', '*is-default': 'bool', '*runnable': 'bool' } }
>  
>  ##
>  # @query-cpu-definitions:
>  #
>  # Return a list of supported virtual CPU definitions
>  #
> +# @machine: #optional machine type (since 2.4)
> +#
> +# @accel: #optional accelerator id (since 2.4)

Maybe mention that these two fields are for filtering results.

-- 
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] 45+ messages in thread

* Re: [PATCH v4 07/15] target-s390x: Update linux-headers/asm-s390/kvm.h
  2015-03-30 19:36   ` Christian Borntraeger
@ 2015-03-31  7:25     ` Michael Mueller
  0 siblings, 0 replies; 45+ messages in thread
From: Michael Mueller @ 2015-03-31  7:25 UTC (permalink / raw)
  To: Christian Borntraeger
  Cc: qemu-devel, kvm, linux-s390, linux-kernel, Eduardo Habkost,
	Gleb Natapov, Alexander Graf, Jason J. Herne, Cornelia Huck,
	Paolo Bonzini, Andreas Faerber, Richard Henderson, Daniel Hansel

On Mon, 30 Mar 2015 21:36:22 +0200
Christian Borntraeger <borntraeger@de.ibm.com> wrote:

> Am 30.03.2015 um 16:28 schrieb Michael Mueller:
> > Signed-off-by: Michael Mueller <mimu@linux.vnet.ibm.com>
> > ---
> >  linux-headers/asm-s390/kvm.h | 18 +++++++++---------
> >  1 file changed, 9 insertions(+), 9 deletions(-)
> > 
> 
> Looks like a leftover. Drop that patch and rename ibc_range to ibc in the other patch.

Will drop the patch and stay with the current name used upstream already.
> 
> 
> > diff --git a/linux-headers/asm-s390/kvm.h b/linux-headers/asm-s390/kvm.h
> > index c5a93eb..bfe6925 100644
> > --- a/linux-headers/asm-s390/kvm.h
> > +++ b/linux-headers/asm-s390/kvm.h
> > @@ -70,8 +70,14 @@ struct kvm_s390_io_adapter_req {
> >  #define KVM_S390_VM_TOD_LOW		0
> >  #define KVM_S390_VM_TOD_HIGH		1
> > 
> > +/* kvm attributes for crypto */
> > +#define KVM_S390_VM_CRYPTO_ENABLE_AES_KW	0
> > +#define KVM_S390_VM_CRYPTO_ENABLE_DEA_KW	1
> > +#define KVM_S390_VM_CRYPTO_DISABLE_AES_KW	2
> > +#define KVM_S390_VM_CRYPTO_DISABLE_DEA_KW	3
> > +
> >  /* kvm attributes for KVM_S390_VM_CPU_MODEL */
> > -/* processor related attributes are r/w */
> > +/* kvm S390 processor related attributes are r/w */
> >  #define KVM_S390_VM_CPU_PROCESSOR	0
> >  struct kvm_s390_vm_cpu_processor {
> >  	__u64 cpuid;
> > @@ -80,22 +86,16 @@ struct kvm_s390_vm_cpu_processor {
> >  	__u64 fac_list[256];
> >  };
> > 
> > -/* machine related attributes are r/o */
> > +/* kvm S390 machine related attributes are r/o */
> >  #define KVM_S390_VM_CPU_MACHINE		1
> >  struct kvm_s390_vm_cpu_machine {
> >  	__u64 cpuid;
> > -	__u32 ibc;
> > +	__u32 ibc_range;
> >  	__u8  pad[4];
> >  	__u64 fac_mask[256];
> >  	__u64 fac_list[256];
> >  };
> > 
> > -/* kvm attributes for crypto */
> > -#define KVM_S390_VM_CRYPTO_ENABLE_AES_KW	0
> > -#define KVM_S390_VM_CRYPTO_ENABLE_DEA_KW	1
> > -#define KVM_S390_VM_CRYPTO_DISABLE_AES_KW	2
> > -#define KVM_S390_VM_CRYPTO_DISABLE_DEA_KW	3
> > -
> >  /* for KVM_GET_REGS and KVM_SET_REGS */
> >  struct kvm_regs {
> >  	/* general purpose regs for s390 */
> > 
> 


^ permalink raw reply	[flat|nested] 45+ messages in thread

* Re: [Qemu-devel] [PATCH v4 12/15] Add optional parameters to QMP command query-cpu-definitions
  2015-03-30 20:28   ` [Qemu-devel] " Eric Blake
@ 2015-03-31  7:42     ` Michael Mueller
  0 siblings, 0 replies; 45+ messages in thread
From: Michael Mueller @ 2015-03-31  7:42 UTC (permalink / raw)
  To: Eric Blake
  Cc: qemu-devel, kvm, linux-s390, linux-kernel, Eduardo Habkost,
	Gleb Natapov, Alexander Graf, Christian Borntraeger,
	Daniel Hansel, Jason J. Herne, Cornelia Huck, Paolo Bonzini,
	Richard Henderson, Andreas Faerber

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset=US-ASCII, Size: 2704 bytes --]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On Mon, 30 Mar 2015 14:28:01 -0600
Eric Blake <eblake@redhat.com> wrote:

> On 03/30/2015 08:28 AM, Michael Mueller wrote:
> > 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>
> > ---
> 
> > +++ b/qapi-schema.json
> > @@ -2532,21 +2532,31 @@
> >  #
> >  # @name: the name of the CPU definition
> >  #
> > +# @default: #optional defines if cpu model is the default (since 2.4)
> 
> Reads poorly.  How about:
> 
> # @default: #optional true if cpu model is the default, omitted if false
> (since 2.4)

Yep, will change

> 
> 
> > +#
> > +# @runnable: #optional defines if cpu model is runnable (since 2.4)
> 
> Similarly:
> 
> # @runnable: #optional true if cpu model is runnable, omitted if false
> (since 2.4)

here as well

> 
> > +#
> >  # Since: 1.2.0
> >  ##
> >  { 'type': 'CpuDefinitionInfo',
> > -  'data': { 'name': 'str' } }
> > +  'data': { 'name': 'str', '*is-default': 'bool', '*runnable': 'bool' } }
> >  
> >  ##
> >  # @query-cpu-definitions:
> >  #
> >  # Return a list of supported virtual CPU definitions
> >  #
> > +# @machine: #optional machine type (since 2.4)
> > +#
> > +# @accel: #optional accelerator id (since 2.4)
> 
> Maybe mention that these two fields are for filtering results.

I will add a comment as it is more than filtering, it is execution context information that allows
to determine if a cpu model is runnable.

Thanks a lot,
Michael

> 

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1

iQIcBAEBAgAGBQJVGk/gAAoJELPcPLQSJsKQ9qcQAJiURTXS+NZO/kmKfP1aH18s
RC/bhXyV6gVmfsvZ1X7S0cH5eO4Z7AfpNNT73Mw0lYDIXei+94/Mdby4AplF7S8q
RoPVu9KxWXV6oM1nigEMvExt5n6BxIM3+/xvKt1Rkef4cx8qIRju+rCLNekmBd3E
yJtSs3Oasft8LoG+4ZPEv26jC7uvHa04bp1nZslXhgUmbUJZzRtArRohp0JA0kfl
BIzpFSKJEvGB/xwyj4bvfC4NQJ9nMtel6BhO04oxHgQNXmpaJK4vN5h7wi6PG2ac
I7mKhC/nNFPUXvQUGQ91itWH/ir1fyim4Rjhtd2Pvpq19waEg2M+dHp2YKAqg1xd
YrHpAQA/6MLqlBqrsqYzVS/LHz7juXP3u/sX5azdbZY8LPynAXqnSwqiNinvk2bA
sc3NG/JwZnbtASFrjJEpmrudS29IXuNNycISzGwrL06pwgmrFaJkpyzD6gOkJfnh
UByIMqTYskk3yP8G8K4n6775al0Zx8v39E7En+dQozEnVa/SxA5YdjJMVPOZiEt4
O/szkqCr5kcQHZJ/x42Sz0YFZ5QIidhMkX6jEqeak7q0Ow0awXgreuxXEmPYu6lG
5wHo6WP1h6tdogQnJGnyFXC5kWzp2iBYxVDP86/4aKLGyZViNPS1XDSjhd9NH4X/
9IPbCIOJYwpZF9l5GeG/
=JAwu
-----END PGP SIGNATURE-----
ÿôèº{.nÇ+‰·Ÿ®‰­†+%ŠËÿ±éݶ\x17¥Šwÿº{.nÇ+‰·¥Š{±þG«éÿŠ{ayº\x1dʇڙë,j\a­¢f£¢·hšïêÿ‘êçz_è®\x03(­éšŽŠÝ¢j"ú\x1a¶^[m§ÿÿ¾\a«þG«éÿ¢¸?™¨è­Ú&£ø§~á¶iO•æ¬z·švØ^\x14\x04\x1a¶^[m§ÿÿÃ\fÿ¶ìÿ¢¸?–I¥

^ permalink raw reply	[flat|nested] 45+ messages in thread

* Re: [Qemu-devel] [PATCH v4 11/15] target-s390x: New QMP command query-cpu-model
  2015-03-30 20:19   ` Eric Blake
@ 2015-03-31  7:56     ` Michael Mueller
  0 siblings, 0 replies; 45+ messages in thread
From: Michael Mueller @ 2015-03-31  7:56 UTC (permalink / raw)
  To: Eric Blake
  Cc: qemu-devel, kvm, linux-s390, linux-kernel, Eduardo Habkost,
	Gleb Natapov, Alexander Graf, Christian Borntraeger,
	Daniel Hansel, Jason J. Herne, Cornelia Huck, Paolo Bonzini,
	Richard Henderson, Andreas Faerber

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset=US-ASCII, Size: 2547 bytes --]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On Mon, 30 Mar 2015 14:19:27 -0600
Eric Blake <eblake@redhat.com> wrote:

> On 03/30/2015 08:28 AM, Michael Mueller wrote:
> > This patch implements a new QMP request named 'query-cpu-model'.
> > It returns the cpu model of cpu 0 and its backing accelerator.
> > 
> > request:
> >   {"execute" : "query-cpu-model" }
> > 
> > answer:
> >   {"return" : {"name": "2827-ga2", "accel": "kvm" }}
> > 
> > Alias names are resolved to their respective machine type and GA names
> > already during cpu instantiation. Thus, also a cpu model like 'host'
> > which is implemented as alias will return its normalized cpu model name.
> > 
> > Furthermore the patch implements the following function:
> > 
> > - s390_cpu_models_used(), returns true if S390 cpu models are in use
> > 
> > Signed-off-by: Michael Mueller <mimu@linux.vnet.ibm.com>
> > ---
> 
> > +++ b/qapi-schema.json
> > @@ -2516,6 +2516,16 @@
> >  { 'command': 'query-machines', 'returns': ['MachineInfo'] }
> >  
> >  ##
> > +# @AccelId
> > +#
> > +# Defines accelerator ids
> > +#
> > +# Since: 2.4
> > +##
> > +{ 'enum': 'AccelId',
> > +  'data': ['qtest', 'tcg', 'kvm', 'xen'  ] }
> 
> Unusual spacing (0 spaces after '[' but 2 spaces before closing ']'?),
> but not necessarily wrong.
> 
> 
> > +##
> > +# @CpuModelInfo:
> > +#
> > +# Virtual CPU model definition.
> > +#
> > +# @name: the name of the CPU model definition
> > +#
> > +# @accel: AccelId (name) of this cpu models accelerator
> 
> s/models/model's/
> 
> 

Will fix both findings...

Thanks,
Michael
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1

iQIcBAEBAgAGBQJVGlM7AAoJELPcPLQSJsKQV1IP/07cv8CB1ggyWFq7pCjo/x4G
A6sjzPziW9sZdrcaMsDb2Px62ddyx+4KymnzzdjKTrKGePZGOBQNd0xwCeIDanw9
Z+hVBG8vzqPiwBM25xKUMYl+NDC7SPeqv6YSFelEBAcbsZGtNb9CBqycjbc8AWtj
Qy8PiLlbPQLxUiFIvnTYe3umW60CtnfUoAiRXs3wQyBkWsUZ7HI7/Rsm8wRIq0tE
HX52/xuD79rQvgqeJi+2zxHyB+WGv0GrvOzIB22d93peMOrdXyQj9GYM6R4Fj7pT
XASifUzztLYlUqNh6MRYmrUaLbsV12ERUKitcd1Cw7l8T7tpIh/NpaOhrm8VosVh
P1Wm9UWha1MfF/rW/u/3sW/82Pv+eQ54a9XYd4H4tD3PFMMmIbZK/9D69BpyxtSZ
45fe4jiKO87bryMtaYPH9oAc8VOmOR9EI94p4q/GK8swaYQ5DGNAPiFlWlfmgg4B
VGXmiHE0VeJOH5dh5+wT/5gjZu3ZmUQNtqhT09rfG0jvMZVUFT0qr5vXQtYXW4Gj
DbK3uir15ovHYBErfun11vs15tUoy4PT3OMsmgelgQl1FQG8T7FD9asj72m3vLUh
UVvRj1KduWET4b4W5SBgzLlMdRyTAcXRPKuDCcSCtqfxE4+jsAp3+mYbLpB2y5Aw
Qjklt025kmr0A7pNrL4d
=ZBtF
-----END PGP SIGNATURE-----
ÿôèº{.nÇ+‰·Ÿ®‰­†+%ŠËÿ±éݶ\x17¥Šwÿº{.nÇ+‰·¥Š{±þG«éÿŠ{ayº\x1dʇڙë,j\a­¢f£¢·hšïêÿ‘êçz_è®\x03(­éšŽŠÝ¢j"ú\x1a¶^[m§ÿÿ¾\a«þG«éÿ¢¸?™¨è­Ú&£ø§~á¶iO•æ¬z·švØ^\x14\x04\x1a¶^[m§ÿÿÃ\fÿ¶ìÿ¢¸?–I¥

^ permalink raw reply	[flat|nested] 45+ messages in thread

* Re: [Qemu-devel] [PATCH v4 11/15] target-s390x: New QMP command query-cpu-model
  2015-03-30 19:50   ` Eduardo Habkost
@ 2015-03-31  9:10     ` Michael Mueller
  0 siblings, 0 replies; 45+ messages in thread
From: Michael Mueller @ 2015-03-31  9:10 UTC (permalink / raw)
  To: Eduardo Habkost
  Cc: linux-s390, kvm, Gleb Natapov, linux-kernel, Alexander Graf,
	qemu-devel, Christian Borntraeger, Daniel Hansel, Jason J. Herne,
	Cornelia Huck, Paolo Bonzini, Andreas Faerber, Richard Henderson

On Mon, 30 Mar 2015 16:50:20 -0300
Eduardo Habkost <ehabkost@redhat.com> wrote:

Hello Eduardo,

> On Mon, Mar 30, 2015 at 04:28:24PM +0200, Michael Mueller wrote:
> [...]
> > diff --git a/target-s390x/cpu.c b/target-s390x/cpu.c
> > index 829945d..1698b52 100644
> > --- a/target-s390x/cpu.c
> > +++ b/target-s390x/cpu.c
> > @@ -37,6 +37,11 @@
> >  #define CR0_RESET       0xE0UL
> >  #define CR14_RESET      0xC2000000UL;
> >  
> > +static inline char *strdup_s390_cpu_name(S390CPUClass *cc)
> > +{
> > +    return g_strdup_printf("%04x-ga%u", cc->proc.type, cc->mach.ga);
> > +}
> > +
> >  /* generate CPU information for cpu -? */
> >  void s390_cpu_list(FILE *f, fprintf_function cpu_fprintf)
> >  {
> > @@ -74,6 +79,30 @@ CpuDefinitionInfoList *arch_query_cpu_definitions(Error **errp)
> >  
> >      return entry;
> >  }
> > +
> > +CpuModelInfo *arch_query_cpu_model(Error **errp)
> > +{
> > +    CpuModelInfo *info;
> > +    S390CPUClass *cc;
> > +
> > +    if (!s390_cpu_models_used()) {
> > +        return NULL;
> > +    }
> 
> First question is: why you don't want to just return
>   { name: "none", accel: ...  }
> when TYPE_S390_CPU ("-cpu none") is used?

That's a good idea, indeed! CPU model "none" is used as well in case option -cpu is omitted.

> 
> Now, assuming that you really want to return NULL if -cpu none is used,
> why don't you just ask for the first CPU (like you already do below) and
> check if it is a named CPU model, instead of adding a new global? e.g.:
> 
>     static bool s390_cpu_class_is_model(S390CPUClass *cc)
>     {
>         return cpuid(cc->proc) != 0;
>     }
>     
>     CpuModelInfo *arch_query_cpu_model(Error **errp)
>     {
>         S390CPUClass *cc;
>         S390CPUClass *cc;
> 
>         cc = S390_CPU_GET_CLASS(s390_cpu_addr2state(0));
>         if (!s390_cpu_class_is_model(cc)) {
>             return NULL;
>         }
>         [...]
>     }
> 
> 
> > +    info = g_try_new0(CpuModelInfo, 1);
> > +    if (!info) {
> > +        return NULL;
> > +    }
> > +    cc = S390_CPU_GET_CLASS(s390_cpu_addr2state(0));
> > +    info->name = strdup_s390_cpu_name(cc);
> 
> I don't think the current version of strdup_s390_cpu_name() can ever
> return NULL. Do you expect it to return NULL in the future?

No I do not expect this. I will drop the NULL test.

> 
> > +    if (!info->name) {
> > +        g_free(info);
> > +        return NULL;
> > +    }
> > +    if (kvm_enabled()) {
> > +        info->accel = ACCEL_ID_KVM;
> 
> This could be a CPUState field, added automatically by
> cpu_generic_init() using current_machine->accel.

I will add a CPUState field and see how it works out...

> 
> > +    }
> > +    return info;
> > +}
> >  #endif
> >  
> >  static void s390_cpu_set_pc(CPUState *cs, vaddr value)
> > -- 
> > 1.8.3.1
> > 
> 

Thanks a lot,
Michael


^ permalink raw reply	[flat|nested] 45+ messages in thread

* Re: [Qemu-devel] [PATCH v4 10/15] target-s390x: Prepare accelerator during cpu object realization
  2015-03-30 19:33   ` Eduardo Habkost
@ 2015-03-31 10:26     ` Michael Mueller
  0 siblings, 0 replies; 45+ messages in thread
From: Michael Mueller @ 2015-03-31 10:26 UTC (permalink / raw)
  To: Eduardo Habkost
  Cc: linux-s390, kvm, Gleb Natapov, linux-kernel, Alexander Graf,
	qemu-devel, Christian Borntraeger, Daniel Hansel, Jason J. Herne,
	Cornelia Huck, Paolo Bonzini, Andreas Faerber, Richard Henderson

On Mon, 30 Mar 2015 16:33:51 -0300
Eduardo Habkost <ehabkost@redhat.com> wrote:

> On Mon, Mar 30, 2015 at 04:28:23PM +0200, Michael Mueller wrote:
> > 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>
> > ---
> >  target-s390x/cpu-models.c | 37 +++++++++++++++++++++++++++++++++++++
> >  target-s390x/cpu-models.h |  4 ++++
> >  target-s390x/cpu.c        |  1 +
> >  3 files changed, 42 insertions(+)
> > 
> > diff --git a/target-s390x/cpu-models.c b/target-s390x/cpu-models.c
> > index 8a877d3..ba873ea 100644
> > --- a/target-s390x/cpu-models.c
> > +++ b/target-s390x/cpu-models.c
> > @@ -111,6 +111,7 @@ typedef struct ParmAddrAddrModeMask {
> >  } ParmAddrAddrModeMask;
> >  
> >  static GSList *s390_cpu_aliases;
> > +static bool cpu_models_used;
> >  
> >  /* compare order of two cpu classes for ascending sort */
> >  gint s390_cpu_class_asc_order_compare(gconstpointer a, gconstpointer b)
> > @@ -670,3 +671,39 @@ 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;
> > +
> > +    /* none cpu model case */
> > +    if (!strcmp(object_class_get_name(OBJECT_CLASS(cc)), TYPE_S390_CPU)) {
> > +        return;
> > +    }
> 
> Instead of checking the class name, can't you just check a S390CPUClass
> field that may indicate that s390_cpu_model_init() doesn't need to do
> anything for the class? Maybe (cpuid(cc->proc) == 0)?

That will work as well but excludes cpuid 0 from being a valid cpu id.

> 
> 
> > +
> > +    /* accelerator already prepared */
> > +    if (cpu_models_used) {
> > +        return;
> > +    }
> 
> I'm still trying to understand the need for this global. But I will ask
> for more details in the following patches that actually use
> s390_cpu_models_used()?

I'm currently using the variable to avoid multiple calls to kvm_s390_set_processor_props() as it
sets the processor properties for all vcpus. I will not hurt to call it twice or more often, it
is just not required and just consumes cpu cycles.

> 
> > +
> > +    proc.cpuid = cpuid(cc->proc);
> > +    proc.ibc = cc->proc.ibc;
> > +    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()) {
> > +        if (!kvm_s390_set_processor_props(&proc)) {
> > +            cpu_models_used = true;
> > +        }
> > +    }
> > +}
> > diff --git a/target-s390x/cpu-models.h b/target-s390x/cpu-models.h
> > index 9562088..fe3997f 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)
> > @@ -108,6 +111,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);
> >  
> >  extern uint64_t qemu_s390_fac_list_mask[];
> >  
> > diff --git a/target-s390x/cpu.c b/target-s390x/cpu.c
> > index c33f05e..829945d 100644
> > --- a/target-s390x/cpu.c
> > +++ b/target-s390x/cpu.c
> > @@ -180,6 +180,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	[flat|nested] 45+ messages in thread

* Re: [Qemu-devel] [PATCH v4 11/15] target-s390x: New QMP command query-cpu-model
  2015-03-30 20:17   ` Eduardo Habkost
  2015-03-30 20:20     ` [Qemu-devel] " Eric Blake
@ 2015-03-31 11:21     ` Michael Mueller
  2015-03-31 18:28       ` Eduardo Habkost
  1 sibling, 1 reply; 45+ messages in thread
From: Michael Mueller @ 2015-03-31 11:21 UTC (permalink / raw)
  To: Eduardo Habkost
  Cc: linux-s390, kvm, Gleb Natapov, linux-kernel, Alexander Graf,
	qemu-devel, Christian Borntraeger, Daniel Hansel, Jason J. Herne,
	Cornelia Huck, Paolo Bonzini, Andreas Faerber, Richard Henderson

On Mon, 30 Mar 2015 17:17:21 -0300
Eduardo Habkost <ehabkost@redhat.com> wrote:

> On Mon, Mar 30, 2015 at 04:28:24PM +0200, Michael Mueller wrote:
> > This patch implements a new QMP request named 'query-cpu-model'.
> > It returns the cpu model of cpu 0 and its backing accelerator.
> > 
> > request:
> >   {"execute" : "query-cpu-model" }
> > 
> > answer:
> >   {"return" : {"name": "2827-ga2", "accel": "kvm" }}
> 
> If you are returning information about an existing CPU, why not just
> extend the output of "query-cpus"?
> 
> (Existing qmp_query_cpus() calls cpu_synchronize_state(), which may be
> undesired. But in this case we could add an optional parameter to
> disable the return of data that requires stopping the VCPU).

Will the cpu_cpu_syncronize_state() really hurt in real life? query-cpus will be called only once
a while...

I will prepare the extension of query-cpus as an option but initially without the optional
parameter.

> 
> > 
> > Alias names are resolved to their respective machine type and GA names
> > already during cpu instantiation. Thus, also a cpu model like 'host'
> > which is implemented as alias will return its normalized cpu model name.
> > 
> > Furthermore the patch implements the following function:
> > 
> > - s390_cpu_models_used(), returns true if S390 cpu models are in use
> > 
> > Signed-off-by: Michael Mueller <mimu@linux.vnet.ibm.com>
> [...]
> 

Thanks,
Michael



^ permalink raw reply	[flat|nested] 45+ messages in thread

* Re: [Qemu-devel] [PATCH v4 11/15] target-s390x: New QMP command query-cpu-model
  2015-03-30 20:20     ` [Qemu-devel] " Eric Blake
@ 2015-03-31 13:16       ` Eduardo Habkost
  0 siblings, 0 replies; 45+ messages in thread
From: Eduardo Habkost @ 2015-03-31 13:16 UTC (permalink / raw)
  To: Eric Blake
  Cc: Michael Mueller, linux-s390, kvm, Gleb Natapov, linux-kernel,
	Alexander Graf, qemu-devel, Christian Borntraeger, Daniel Hansel,
	Jason J. Herne, Cornelia Huck, Paolo Bonzini, Andreas Faerber,
	Richard Henderson

On Mon, Mar 30, 2015 at 02:20:43PM -0600, Eric Blake wrote:
> On 03/30/2015 02:17 PM, Eduardo Habkost wrote:
> > On Mon, Mar 30, 2015 at 04:28:24PM +0200, Michael Mueller wrote:
> >> This patch implements a new QMP request named 'query-cpu-model'.
> >> It returns the cpu model of cpu 0 and its backing accelerator.
> >>
> >> request:
> >>   {"execute" : "query-cpu-model" }
> >>
> >> answer:
> >>   {"return" : {"name": "2827-ga2", "accel": "kvm" }}
> > 
> > If you are returning information about an existing CPU, why not just
> > extend the output of "query-cpus"?
> > 
> > (Existing qmp_query_cpus() calls cpu_synchronize_state(), which may be
> > undesired. But in this case we could add an optional parameter to
> > disable the return of data that requires stopping the VCPU).
> 
> And how would libvirt learn about the existence of that optional
> parameter?  Without introspection, a new command is easier to query than
> learning about whether an optional parameter exists (then again, we're
> hoping to get introspection into 2.4, so it may be a moot question).

Even if we don't get introspection, a query-cpus-v2 command (with the
extra parameter) could be extended in the future to return additional
information about CPUs, instead of being specific for CPU model
information.

We also have the option of simply providing predictable QOM paths for
CPU objects, then clients could use qom-get to get what they need
through QOM properties. There was a discussion about it some time ago,
but I don't remember the conclusion.

-- 
Eduardo

^ permalink raw reply	[flat|nested] 45+ messages in thread

* Re: [Qemu-devel] [PATCH v4 11/15] target-s390x: New QMP command query-cpu-model
  2015-03-31 11:21     ` Michael Mueller
@ 2015-03-31 18:28       ` Eduardo Habkost
  0 siblings, 0 replies; 45+ messages in thread
From: Eduardo Habkost @ 2015-03-31 18:28 UTC (permalink / raw)
  To: Michael Mueller
  Cc: linux-s390, kvm, Gleb Natapov, linux-kernel, Alexander Graf,
	qemu-devel, Christian Borntraeger, Daniel Hansel, Jason J. Herne,
	Cornelia Huck, Paolo Bonzini, Andreas Faerber, Richard Henderson

On Tue, Mar 31, 2015 at 01:21:05PM +0200, Michael Mueller wrote:
> On Mon, 30 Mar 2015 17:17:21 -0300
> Eduardo Habkost <ehabkost@redhat.com> wrote:
> 
> > On Mon, Mar 30, 2015 at 04:28:24PM +0200, Michael Mueller wrote:
> > > This patch implements a new QMP request named 'query-cpu-model'.
> > > It returns the cpu model of cpu 0 and its backing accelerator.
> > > 
> > > request:
> > >   {"execute" : "query-cpu-model" }
> > > 
> > > answer:
> > >   {"return" : {"name": "2827-ga2", "accel": "kvm" }}
> > 
> > If you are returning information about an existing CPU, why not just
> > extend the output of "query-cpus"?
> > 
> > (Existing qmp_query_cpus() calls cpu_synchronize_state(), which may be
> > undesired. But in this case we could add an optional parameter to
> > disable the return of data that requires stopping the VCPU).
> 
> Will the cpu_cpu_syncronize_state() really hurt in real life?
> query-cpus will be called only once a while...
> 

I was just thinking about possible reasons you wouldn't want to reuse
query-cpus, and thought cpu_synchronize_state() call could be one of
them.

> I will prepare the extension of query-cpus as an option but initially
> without the optional parameter.

I agree we can simply add the new info to query-cpus without any extra
parameter, and (if really necessary) we can worry about optimizing it by
avoiding the cpu_synchronize_state() call later.

-- 
Eduardo

^ permalink raw reply	[flat|nested] 45+ messages in thread

* Re: [PATCH v4 11/15] target-s390x: New QMP command query-cpu-model
  2015-03-30 14:28 ` [PATCH v4 11/15] target-s390x: New QMP command query-cpu-model Michael Mueller
                     ` (2 preceding siblings ...)
  2015-03-30 20:19   ` Eric Blake
@ 2015-03-31 18:35   ` Eduardo Habkost
  2015-03-31 20:09     ` Michael Mueller
  3 siblings, 1 reply; 45+ messages in thread
From: Eduardo Habkost @ 2015-03-31 18:35 UTC (permalink / raw)
  To: Michael Mueller
  Cc: qemu-devel, kvm, linux-s390, linux-kernel, Gleb Natapov,
	Alexander Graf, Christian Borntraeger, Jason J. Herne,
	Cornelia Huck, Paolo Bonzini, Andreas Faerber, Richard Henderson,
	Daniel Hansel

On Mon, Mar 30, 2015 at 04:28:24PM +0200, Michael Mueller wrote:
> This patch implements a new QMP request named 'query-cpu-model'.
> It returns the cpu model of cpu 0 and its backing accelerator.
> 
> request:
>   {"execute" : "query-cpu-model" }
> 
> answer:
>   {"return" : {"name": "2827-ga2", "accel": "kvm" }}
> 
> Alias names are resolved to their respective machine type and GA names
> already during cpu instantiation. Thus, also a cpu model like 'host'
> which is implemented as alias will return its normalized cpu model name.
> 
> Furthermore the patch implements the following function:
> 
> - s390_cpu_models_used(), returns true if S390 cpu models are in use
> 
> Signed-off-by: Michael Mueller <mimu@linux.vnet.ibm.com>
> ---
[...]
> +static inline char *strdup_s390_cpu_name(S390CPUClass *cc)
> +{
> +    return g_strdup_printf("%04x-ga%u", cc->proc.type, cc->mach.ga);
> +}

How exactly is this information going to be used by clients? If getting
the correct type and ga values is important for them, maybe you could
add them as integer fields, instead of requiring clients to parse the
CPU model name?

-- 
Eduardo

^ permalink raw reply	[flat|nested] 45+ messages in thread

* Re: [PATCH v4 12/15] Add optional parameters to QMP command query-cpu-definitions
  2015-03-30 14:28 ` [PATCH v4 12/15] Add optional parameters to QMP command query-cpu-definitions Michael Mueller
  2015-03-30 20:28   ` [Qemu-devel] " Eric Blake
@ 2015-03-31 19:46   ` Eduardo Habkost
  2015-03-31 19:50     ` Eric Blake
  2015-03-31 20:22     ` [Qemu-devel] " Michael Mueller
  1 sibling, 2 replies; 45+ messages in thread
From: Eduardo Habkost @ 2015-03-31 19:46 UTC (permalink / raw)
  To: Michael Mueller
  Cc: qemu-devel, kvm, linux-s390, linux-kernel, Gleb Natapov,
	Alexander Graf, Christian Borntraeger, Jason J. Herne,
	Cornelia Huck, Paolo Bonzini, Andreas Faerber, Richard Henderson,
	Daniel Hansel, Eric Blake

On Mon, Mar 30, 2015 at 04:28:25PM +0200, Michael Mueller wrote:
[...]
>  ##
>  # @query-cpu-definitions:
>  #
>  # Return a list of supported virtual CPU definitions
>  #
> +# @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'] }

What happens if the new parameters are provided to an old QEMU version
that doesn't accept them? It looks like we need an introspection
mechanism or a new command name.

-- 
Eduardo

^ permalink raw reply	[flat|nested] 45+ messages in thread

* Re: [PATCH v4 12/15] Add optional parameters to QMP command query-cpu-definitions
  2015-03-31 19:46   ` Eduardo Habkost
@ 2015-03-31 19:50     ` Eric Blake
  2015-03-31 20:22     ` [Qemu-devel] " Michael Mueller
  1 sibling, 0 replies; 45+ messages in thread
From: Eric Blake @ 2015-03-31 19:50 UTC (permalink / raw)
  To: Eduardo Habkost, Michael Mueller
  Cc: qemu-devel, kvm, linux-s390, linux-kernel, Gleb Natapov,
	Alexander Graf, Christian Borntraeger, Jason J. Herne,
	Cornelia Huck, Paolo Bonzini, Andreas Faerber, Richard Henderson,
	Daniel Hansel

[-- Attachment #1: Type: text/plain, Size: 1211 bytes --]

On 03/31/2015 01:46 PM, Eduardo Habkost wrote:
> On Mon, Mar 30, 2015 at 04:28:25PM +0200, Michael Mueller wrote:
> [...]
>>  ##
>>  # @query-cpu-definitions:
>>  #
>>  # Return a list of supported virtual CPU definitions
>>  #
>> +# @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'] }
> 
> What happens if the new parameters are provided to an old QEMU version
> that doesn't accept them? It looks like we need an introspection
> mechanism or a new command name.

Providing an optional parameter that a new qemu understands to an older
qemu gracefully errors out about an unknown parameter.  But it's
annoying to have to probe for whether the parameter is understood by
exploiting that particular error message from older qemu.

-- 
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] 45+ messages in thread

* Re: [PATCH v4 11/15] target-s390x: New QMP command query-cpu-model
  2015-03-31 18:35   ` Eduardo Habkost
@ 2015-03-31 20:09     ` Michael Mueller
  2015-04-01 13:01       ` Eduardo Habkost
  0 siblings, 1 reply; 45+ messages in thread
From: Michael Mueller @ 2015-03-31 20:09 UTC (permalink / raw)
  To: Eduardo Habkost
  Cc: qemu-devel, kvm, linux-s390, linux-kernel, Gleb Natapov,
	Alexander Graf, Christian Borntraeger, Jason J. Herne,
	Cornelia Huck, Paolo Bonzini, Andreas Faerber, Richard Henderson,
	Daniel Hansel

On Tue, 31 Mar 2015 15:35:26 -0300
Eduardo Habkost <ehabkost@redhat.com> wrote:

> On Mon, Mar 30, 2015 at 04:28:24PM +0200, Michael Mueller wrote:
> > This patch implements a new QMP request named 'query-cpu-model'.
> > It returns the cpu model of cpu 0 and its backing accelerator.
> > 
> > request:
> >   {"execute" : "query-cpu-model" }
> > 
> > answer:
> >   {"return" : {"name": "2827-ga2", "accel": "kvm" }}
> > 
> > Alias names are resolved to their respective machine type and GA names
> > already during cpu instantiation. Thus, also a cpu model like 'host'
> > which is implemented as alias will return its normalized cpu model name.
> > 
> > Furthermore the patch implements the following function:
> > 
> > - s390_cpu_models_used(), returns true if S390 cpu models are in use
> > 
> > Signed-off-by: Michael Mueller <mimu@linux.vnet.ibm.com>
> > ---
> [...]
> > +static inline char *strdup_s390_cpu_name(S390CPUClass *cc)
> > +{
> > +    return g_strdup_printf("%04x-ga%u", cc->proc.type, cc->mach.ga);
> > +}
> 
> How exactly is this information going to be used by clients? If getting
> the correct type and ga values is important for them, maybe you could
> add them as integer fields, instead of requiring clients to parse the
> CPU model name?

The consumer don't need to parse the name, it is just important for them to have
distinctive names that correlate with the names returned by query-cpu-definitions.
Once the name of an active guest is known, e.g. ("2827-ga2", "kvm") a potential
migration target can be verified, i.e. its query-cpu-definitions answer for "kvm"
has to contain "2827-ga2" with the attribute runnable set to true. With that mechanism
also the largest common denominator can be calculated. That model will be used then.

I also changed the above mentioned routine to map the cpu model none case:

static inline char *strdup_s390_cpu_name(S390CPUClass *cc)
{
    if (cpuid(cc->proc)) {
        return g_strdup_printf("%04x-ga%u", cc->proc.type, cc->mach.ga);
    } else {
        return g_strdup("none");
    }
}

This implicitly will fail a comparison for cpu model ("none", "kvm") as that will
never be part of the query-cpu-definitions answer.

I actually applied a couple of your suggestions like:

- test for NULL skipped after strdup_s390_cpu_name()
- strdup_s390_cpu_name() now also handles none cpu model case
- omit runnable and is-default field from query-cpu-definitions
  answer when they are false
- global variable cpu_models_used dropped
- function s390_cpu_models_used() dropped
- routine query-cpu-definitions has a single code path now

Only the integration of the ACCEL_ID with the cpu state in cpu_generic_init() and
the change for the query-cpus implementation is under construction. I hope to resend
the patches by tomorrow evening.

Thanks,
Michael 

> 


^ permalink raw reply	[flat|nested] 45+ messages in thread

* Re: [Qemu-devel] [PATCH v4 12/15] Add optional parameters to QMP command query-cpu-definitions
  2015-03-31 19:46   ` Eduardo Habkost
  2015-03-31 19:50     ` Eric Blake
@ 2015-03-31 20:22     ` Michael Mueller
  1 sibling, 0 replies; 45+ messages in thread
From: Michael Mueller @ 2015-03-31 20:22 UTC (permalink / raw)
  To: Eduardo Habkost
  Cc: linux-s390, kvm, Gleb Natapov, linux-kernel, Alexander Graf,
	qemu-devel, Christian Borntraeger, Daniel Hansel, Jason J. Herne,
	Cornelia Huck, Paolo Bonzini, Andreas Faerber, Richard Henderson

On Tue, 31 Mar 2015 16:46:56 -0300
Eduardo Habkost <ehabkost@redhat.com> wrote:

> On Mon, Mar 30, 2015 at 04:28:25PM +0200, Michael Mueller wrote:
> [...]
> >  ##
> >  # @query-cpu-definitions:
> >  #
> >  # Return a list of supported virtual CPU definitions
> >  #
> > +# @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'] }
> 
> What happens if the new parameters are provided to an old QEMU version
> that doesn't accept them? It looks like we need an introspection
> mechanism or a new command name.

Yep, as Eric mentions:

[mimu@p57lp59 (master) qemu]$ sudo virsh qemu-monitor-command zhyp027 '{ "execute":
"query-cpu-definitions", "arguments": { "accel": "kvm", "machine":
"s390-ccw-virtio" } }'

{"id":"libvirt-13","error":{"class":"GenericError","desc":"Invalid parameter 'accel'"}}



> 


^ permalink raw reply	[flat|nested] 45+ messages in thread

* Re: [PATCH v4 11/15] target-s390x: New QMP command query-cpu-model
  2015-03-31 20:09     ` Michael Mueller
@ 2015-04-01 13:01       ` Eduardo Habkost
  2015-04-01 16:31         ` [Qemu-devel] " Michael Mueller
  0 siblings, 1 reply; 45+ messages in thread
From: Eduardo Habkost @ 2015-04-01 13:01 UTC (permalink / raw)
  To: Michael Mueller
  Cc: qemu-devel, kvm, linux-s390, linux-kernel, Gleb Natapov,
	Alexander Graf, Christian Borntraeger, Jason J. Herne,
	Cornelia Huck, Paolo Bonzini, Andreas Faerber, Richard Henderson,
	Daniel Hansel

On Tue, Mar 31, 2015 at 10:09:09PM +0200, Michael Mueller wrote:
> On Tue, 31 Mar 2015 15:35:26 -0300
> Eduardo Habkost <ehabkost@redhat.com> wrote:
> 
> > On Mon, Mar 30, 2015 at 04:28:24PM +0200, Michael Mueller wrote:
> > > This patch implements a new QMP request named 'query-cpu-model'.
> > > It returns the cpu model of cpu 0 and its backing accelerator.
> > > 
> > > request:
> > >   {"execute" : "query-cpu-model" }
> > > 
> > > answer:
> > >   {"return" : {"name": "2827-ga2", "accel": "kvm" }}
> > > 
> > > Alias names are resolved to their respective machine type and GA names
> > > already during cpu instantiation. Thus, also a cpu model like 'host'
> > > which is implemented as alias will return its normalized cpu model name.
> > > 
> > > Furthermore the patch implements the following function:
> > > 
> > > - s390_cpu_models_used(), returns true if S390 cpu models are in use
> > > 
> > > Signed-off-by: Michael Mueller <mimu@linux.vnet.ibm.com>
> > > ---
> > [...]
> > > +static inline char *strdup_s390_cpu_name(S390CPUClass *cc)
> > > +{
> > > +    return g_strdup_printf("%04x-ga%u", cc->proc.type, cc->mach.ga);
> > > +}
> > 
> > How exactly is this information going to be used by clients? If getting
> > the correct type and ga values is important for them, maybe you could
> > add them as integer fields, instead of requiring clients to parse the
> > CPU model name?
> 
> The consumer don't need to parse the name, it is just important for them to have
> distinctive names that correlate with the names returned by query-cpu-definitions.
> Once the name of an active guest is known, e.g. ("2827-ga2", "kvm") a potential
> migration target can be verified, i.e. its query-cpu-definitions answer for "kvm"
> has to contain "2827-ga2" with the attribute runnable set to true. With that mechanism
> also the largest common denominator can be calculated. That model will be used then.

Understood. So the point is to really have a name that can be found at
query-cpu-definitions. Makes sense.

(BTW, if you reused strdup_s390_cpu_name() inside
s390_cpu_compare_class_name() too, you would automatically ensure that
query-cpus, query-cpu-definitions and s390_cpu_class_by_name() will
always agree with each other).

> 
> I also changed the above mentioned routine to map the cpu model none case:
> 
> static inline char *strdup_s390_cpu_name(S390CPUClass *cc)
> {
>     if (cpuid(cc->proc)) {
>         return g_strdup_printf("%04x-ga%u", cc->proc.type, cc->mach.ga);
>     } else {
>         return g_strdup("none");
>     }
> }

What about:

  static const char *s390_cpu_name(S390CPUClass *cc)
  {
      return cc->model_name;
  }

And then you can just set cc->model_name=_name inside S390_PROC_DEF (and
set it to "none" inside s390_cpu_class_init()).

I wonder if this class->model_name conversion could be made generic
inside the CPU class. We already have a CPU::class_by_name() method, so
it makes sense to have the opposite function too.

(But I wouldn't mind making this s390-specific first, and converted
later to generic code if appropriate).

> 
> This implicitly will fail a comparison for cpu model ("none", "kvm") as that will
> never be part of the query-cpu-definitions answer.

I am not sure I follow. If ("none", "kvm") is never in the list, is
"-cpu none -machine accel=kvm" always an invalid use case?

(I don't understand completely the meaning of "-cpu none" yet. How does
the CPU look like for the guest in this case? Is it possible to
live-migrate when using -cpu none?)

-- 
Eduardo

^ permalink raw reply	[flat|nested] 45+ messages in thread

* Re: [Qemu-devel] [PATCH v4 11/15] target-s390x: New QMP command query-cpu-model
  2015-04-01 13:01       ` Eduardo Habkost
@ 2015-04-01 16:31         ` Michael Mueller
  2015-04-01 16:59           ` Eduardo Habkost
  0 siblings, 1 reply; 45+ messages in thread
From: Michael Mueller @ 2015-04-01 16:31 UTC (permalink / raw)
  To: Eduardo Habkost
  Cc: linux-s390, kvm, Gleb Natapov, linux-kernel, Alexander Graf,
	qemu-devel, Christian Borntraeger, Daniel Hansel, Jason J. Herne,
	Cornelia Huck, Paolo Bonzini, Andreas Faerber, Richard Henderson

On Wed, 1 Apr 2015 10:01:13 -0300
Eduardo Habkost <ehabkost@redhat.com> wrote:

> On Tue, Mar 31, 2015 at 10:09:09PM +0200, Michael Mueller wrote:
> > On Tue, 31 Mar 2015 15:35:26 -0300
> > Eduardo Habkost <ehabkost@redhat.com> wrote:
> > 
> > > On Mon, Mar 30, 2015 at 04:28:24PM +0200, Michael Mueller wrote:
> > > > This patch implements a new QMP request named 'query-cpu-model'.
> > > > It returns the cpu model of cpu 0 and its backing accelerator.
> > > > 
> > > > request:
> > > >   {"execute" : "query-cpu-model" }
> > > > 
> > > > answer:
> > > >   {"return" : {"name": "2827-ga2", "accel": "kvm" }}
> > > > 
> > > > Alias names are resolved to their respective machine type and GA names
> > > > already during cpu instantiation. Thus, also a cpu model like 'host'
> > > > which is implemented as alias will return its normalized cpu model name.
> > > > 
> > > > Furthermore the patch implements the following function:
> > > > 
> > > > - s390_cpu_models_used(), returns true if S390 cpu models are in use
> > > > 
> > > > Signed-off-by: Michael Mueller <mimu@linux.vnet.ibm.com>
> > > > ---
> > > [...]
> > > > +static inline char *strdup_s390_cpu_name(S390CPUClass *cc)
> > > > +{
> > > > +    return g_strdup_printf("%04x-ga%u", cc->proc.type, cc->mach.ga);
> > > > +}
> > > 
> > > How exactly is this information going to be used by clients? If getting
> > > the correct type and ga values is important for them, maybe you could
> > > add them as integer fields, instead of requiring clients to parse the
> > > CPU model name?
> > 
> > The consumer don't need to parse the name, it is just important for them to have
> > distinctive names that correlate with the names returned by query-cpu-definitions.
> > Once the name of an active guest is known, e.g. ("2827-ga2", "kvm") a potential
> > migration target can be verified, i.e. its query-cpu-definitions answer for "kvm"
> > has to contain "2827-ga2" with the attribute runnable set to true. With that mechanism
> > also the largest common denominator can be calculated. That model will be used then.
> 
> Understood. So the point is to really have a name that can be found at
> query-cpu-definitions. Makes sense.
> 
> (BTW, if you reused strdup_s390_cpu_name() inside
> s390_cpu_compare_class_name() too, you would automatically ensure that
> query-cpus, query-cpu-definitions and s390_cpu_class_by_name() will
> always agree with each other).

I have to verify but it seems to make sense from reading... I will do that if it works. :-)

> 
> > 
> > I also changed the above mentioned routine to map the cpu model none case:
> > 
> > static inline char *strdup_s390_cpu_name(S390CPUClass *cc)
> > {
> >     if (cpuid(cc->proc)) {
> >         return g_strdup_printf("%04x-ga%u", cc->proc.type, cc->mach.ga);
> >     } else {
> >         return g_strdup("none");
> >     }
> > }
> 
> What about:
> 
>   static const char *s390_cpu_name(S390CPUClass *cc)
>   {
>       return cc->model_name;
>   }
> 
> And then you can just set cc->model_name=_name inside S390_PROC_DEF (and
> set it to "none" inside s390_cpu_class_init()).
> 

Wouldn't that store redundant information... but it would at least shift the work into the
initialization phase and do the format just once per model.

> I wonder if this class->model_name conversion could be made generic
> inside the CPU class. We already have a CPU::class_by_name() method, so
> it makes sense to have the opposite function too.
> 
> (But I wouldn't mind making this s390-specific first, and converted
> later to generic code if appropriate).

ok

> 
> > 
> > This implicitly will fail a comparison for cpu model ("none", "kvm") as that will
> > never be part of the query-cpu-definitions answer.
> 
> I am not sure I follow. If ("none", "kvm") is never in the list, is
> "-cpu none -machine accel=kvm" always an invalid use case?

Not directly invalid as "-cpu none" will be the same as omitting the -cpu option.
KVM will setup the vcpu properties withou any QEMU control to whatever the hosting
machine and the kvm kernel code offers. That will allow to run QEMU against a KVM
version that is not aware of the s390 cpu model ioctls.

> 
> (I don't understand completely the meaning of "-cpu none" yet. How does
> the CPU look like for the guest in this case? Is it possible to
> live-migrate when using -cpu none?)

And yes, that does not make sense in a migration context. The answer on query-cpu-model
(or query-cpus) will be ("none", "kvm") and that will never match a runnable model.
The guest cpu will be derived from the hosting system and the kvm kernel as it is currently
without the cpu model interface. 

I hope I made it better to understand now...

Michael

> 


^ permalink raw reply	[flat|nested] 45+ messages in thread

* Re: [Qemu-devel] [PATCH v4 11/15] target-s390x: New QMP command query-cpu-model
  2015-04-01 16:31         ` [Qemu-devel] " Michael Mueller
@ 2015-04-01 16:59           ` Eduardo Habkost
  2015-04-01 19:05             ` Michael Mueller
  0 siblings, 1 reply; 45+ messages in thread
From: Eduardo Habkost @ 2015-04-01 16:59 UTC (permalink / raw)
  To: Michael Mueller
  Cc: linux-s390, kvm, Gleb Natapov, linux-kernel, Alexander Graf,
	qemu-devel, Christian Borntraeger, Daniel Hansel, Jason J. Herne,
	Cornelia Huck, Paolo Bonzini, Andreas Faerber, Richard Henderson,
	libvir-list, Jiri Denemark

(CCing libvir-list and Jiri Denemark for libvirt-related discussion
about -cpu host/none, and live-migration safety expectations)

On Wed, Apr 01, 2015 at 06:31:23PM +0200, Michael Mueller wrote:
> On Wed, 1 Apr 2015 10:01:13 -0300
> Eduardo Habkost <ehabkost@redhat.com> wrote:
> 
> > On Tue, Mar 31, 2015 at 10:09:09PM +0200, Michael Mueller wrote:
> > > On Tue, 31 Mar 2015 15:35:26 -0300
> > > Eduardo Habkost <ehabkost@redhat.com> wrote:
> > > 
> > > > On Mon, Mar 30, 2015 at 04:28:24PM +0200, Michael Mueller wrote:
> > > > > This patch implements a new QMP request named 'query-cpu-model'.
> > > > > It returns the cpu model of cpu 0 and its backing accelerator.
> > > > > 
> > > > > request:
> > > > >   {"execute" : "query-cpu-model" }
> > > > > 
> > > > > answer:
> > > > >   {"return" : {"name": "2827-ga2", "accel": "kvm" }}
> > > > > 
> > > > > Alias names are resolved to their respective machine type and GA names
> > > > > already during cpu instantiation. Thus, also a cpu model like 'host'
> > > > > which is implemented as alias will return its normalized cpu model name.
> > > > > 
> > > > > Furthermore the patch implements the following function:
> > > > > 
> > > > > - s390_cpu_models_used(), returns true if S390 cpu models are in use
> > > > > 
> > > > > Signed-off-by: Michael Mueller <mimu@linux.vnet.ibm.com>
> > > > > ---
> > > > [...]
> > > > > +static inline char *strdup_s390_cpu_name(S390CPUClass *cc)
> > > > > +{
> > > > > +    return g_strdup_printf("%04x-ga%u", cc->proc.type, cc->mach.ga);
> > > > > +}
> > > > 
> > > > How exactly is this information going to be used by clients? If getting
> > > > the correct type and ga values is important for them, maybe you could
> > > > add them as integer fields, instead of requiring clients to parse the
> > > > CPU model name?
> > > 
> > > The consumer don't need to parse the name, it is just important for them to have
> > > distinctive names that correlate with the names returned by query-cpu-definitions.
> > > Once the name of an active guest is known, e.g. ("2827-ga2", "kvm") a potential
> > > migration target can be verified, i.e. its query-cpu-definitions answer for "kvm"
> > > has to contain "2827-ga2" with the attribute runnable set to true. With that mechanism
> > > also the largest common denominator can be calculated. That model will be used then.
> > 
> > Understood. So the point is to really have a name that can be found at
> > query-cpu-definitions. Makes sense.
> > 
> > (BTW, if you reused strdup_s390_cpu_name() inside
> > s390_cpu_compare_class_name() too, you would automatically ensure that
> > query-cpus, query-cpu-definitions and s390_cpu_class_by_name() will
> > always agree with each other).
> 
> I have to verify but it seems to make sense from reading... I will do that if it works. :-)
> 
> > 
> > > 
> > > I also changed the above mentioned routine to map the cpu model none case:
> > > 
> > > static inline char *strdup_s390_cpu_name(S390CPUClass *cc)
> > > {
> > >     if (cpuid(cc->proc)) {
> > >         return g_strdup_printf("%04x-ga%u", cc->proc.type, cc->mach.ga);
> > >     } else {
> > >         return g_strdup("none");
> > >     }
> > > }
> > 
> > What about:
> > 
> >   static const char *s390_cpu_name(S390CPUClass *cc)
> >   {
> >       return cc->model_name;
> >   }
> > 
> > And then you can just set cc->model_name=_name inside S390_PROC_DEF (and
> > set it to "none" inside s390_cpu_class_init()).
> > 
> 
> Wouldn't that store redundant information... but it would at least shift the work into the
> initialization phase and do the format just once per model.

To be honest, calculating the CPU model name on the fly with
strdup_s390_cpu_name() like you did above wouldn't be a problem to me.
I just wanted to avoid having the same CPU model name logic (and special
cases like "none") duplicated inside strdup_s390_cpu_name(),
S390_PROC_DEF, s390_cpu_class_by_name(), and maybe other places. Maybe
this duplication can be eliminated by simply reusing
strdup_s390_cpu_name() inside s390_cpu_compare_class_name().


> 
> > I wonder if this class->model_name conversion could be made generic
> > inside the CPU class. We already have a CPU::class_by_name() method, so
> > it makes sense to have the opposite function too.
> > 
> > (But I wouldn't mind making this s390-specific first, and converted
> > later to generic code if appropriate).
> 
> ok
> 
> > 
> > > 
> > > This implicitly will fail a comparison for cpu model ("none", "kvm") as that will
> > > never be part of the query-cpu-definitions answer.
> > 
> > I am not sure I follow. If ("none", "kvm") is never in the list, is
> > "-cpu none -machine accel=kvm" always an invalid use case?
> 
> Not directly invalid as "-cpu none" will be the same as omitting the -cpu option.
> KVM will setup the vcpu properties withou any QEMU control to whatever the hosting
> machine and the kvm kernel code offers. That will allow to run QEMU against a KVM
> version that is not aware of the s390 cpu model ioctls.

It looks like we have conflicting expectations about
query-cpu-definitions, it seems: on the one hand, if "-cpu none" is
valid I believe it should appear on the query-cpu-definitions return
value; on the other hand, it is not (always?) migration-safe, so just
comparing the source query-cpus data with the target
query-cpu-definitions data wouldn't be enough to ensure live-migration
safety.

On x86, we have a similar problem with "-cpu host", that changes
depending on the host hardware and host kernel. We solve this problem by
making libvirt code aware of the set of valid CPU models, and libvirt
has special cases for "-cpu host".

If you don't want to encode that knowledge in libvirt or other
management software for s390, it looks like you need something like a
"stable-abi-safe" field on CpuDefinitionInfo?

> 
> > 
> > (I don't understand completely the meaning of "-cpu none" yet. How does
> > the CPU look like for the guest in this case? Is it possible to
> > live-migrate when using -cpu none?)
> 
> And yes, that does not make sense in a migration context. The answer on query-cpu-model
> (or query-cpus) will be ("none", "kvm") and that will never match a runnable model.
> The guest cpu will be derived from the hosting system and the kvm kernel as it is currently
> without the cpu model interface. 
> 
> I hope I made it better to understand now...

Yes, thanks!

-- 
Eduardo

^ permalink raw reply	[flat|nested] 45+ messages in thread

* Re: [Qemu-devel] [PATCH v4 11/15] target-s390x: New QMP command query-cpu-model
  2015-04-01 16:59           ` Eduardo Habkost
@ 2015-04-01 19:05             ` Michael Mueller
  2015-04-01 19:10               ` Michael Mueller
  2015-04-01 23:05               ` Eduardo Habkost
  0 siblings, 2 replies; 45+ messages in thread
From: Michael Mueller @ 2015-04-01 19:05 UTC (permalink / raw)
  To: Eduardo Habkost
  Cc: linux-s390, Cornelia Huck, kvm, Gleb Natapov, qemu-devel,
	linux-kernel, libvir-list, Christian Borntraeger, Alexander Graf,
	Jason J. Herne, Daniel Hansel, Paolo Bonzini, Jiri Denemark,
	Andreas Faerber, Richard Henderson

On Wed, 1 Apr 2015 13:59:05 -0300
Eduardo Habkost <ehabkost@redhat.com> wrote:

> > Not directly invalid as "-cpu none" will be the same as omitting the -cpu option.
> > KVM will setup the vcpu properties withou any QEMU control to whatever the hosting
> > machine and the kvm kernel code offers. That will allow to run QEMU against a KVM
> > version that is not aware of the s390 cpu model ioctls.  
> 
> It looks like we have conflicting expectations about
> query-cpu-definitions, it seems: on the one hand, if "-cpu none" is
> valid I believe it should appear on the query-cpu-definitions return
> value; on the other hand, it is not (always?) migration-safe, so just
> comparing the source query-cpus data with the target
> query-cpu-definitions data wouldn't be enough to ensure live-migration
> safety.

There are other cases as well where the value given with -cpu is *not* part
of the cpu definition list and that is aliases:

[mimu@p57lp59 (master-cpu-model) qemu]$ ./s390x-softmmu/qemu-system-s390x -machine
s390-ccw,accel=kvm -cpu ?
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 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)

As you can see "host" is in s390x case always an alias and also all other aliases
are normalized to their real cpu models in the cpu-definitions list.

> 
> On x86, we have a similar problem with "-cpu host", that changes
> depending on the host hardware and host kernel. We solve this problem by
> making libvirt code aware of the set of valid CPU models, and libvirt
> has special cases for "-cpu host".

"-cpu host" is not a special case for s390, it will return ("2827-ga2", "kvm") as
cpu model or whatever model the hosting system implements.

> 
> If you don't want to encode that knowledge in libvirt or other
> management software for s390, it looks like you need something like a
> "stable-abi-safe" field on CpuDefinitionInfo?

Exactly that fulfills the "name" field for s390 already in my view.

And cpu model "none" just means that QEMU does not manage the cpu model. That's also
the reason why I initially returned an empty "[]" model and not "none". This somewhat
convinces me to go back to this approach...

Michael


^ permalink raw reply	[flat|nested] 45+ messages in thread

* Re: [Qemu-devel] [PATCH v4 11/15] target-s390x: New QMP command query-cpu-model
  2015-04-01 19:05             ` Michael Mueller
@ 2015-04-01 19:10               ` Michael Mueller
  2015-04-01 23:05               ` Eduardo Habkost
  1 sibling, 0 replies; 45+ messages in thread
From: Michael Mueller @ 2015-04-01 19:10 UTC (permalink / raw)
  To: Michael Mueller
  Cc: Eduardo Habkost, linux-s390, kvm, Christian Borntraeger,
	Gleb Natapov, qemu-devel, linux-kernel, libvir-list,
	Alexander Graf, Daniel Hansel, Jason J. Herne, Cornelia Huck,
	Paolo Bonzini, Jiri Denemark, Andreas Faerber, Richard Henderson

On Wed, 1 Apr 2015 21:05:31 +0200
Michael Mueller <mimu@linux.vnet.ibm.com> wrote:

> And cpu model "none" just means that QEMU does not manage the cpu model. That's also
> the reason why I initially returned an empty "[]" model and not "none". This somewhat
> convinces me to go back to this approach...

And for query-cpus that can be represented as a non-existent model field:

[{"current":true,"CPU":0,"halted":false,"thread_id":39110}, ...]


^ permalink raw reply	[flat|nested] 45+ messages in thread

* Re: [Qemu-devel] [PATCH v4 11/15] target-s390x: New QMP command query-cpu-model
  2015-04-01 19:05             ` Michael Mueller
  2015-04-01 19:10               ` Michael Mueller
@ 2015-04-01 23:05               ` Eduardo Habkost
  2015-04-02  7:09                 ` Michael Mueller
  1 sibling, 1 reply; 45+ messages in thread
From: Eduardo Habkost @ 2015-04-01 23:05 UTC (permalink / raw)
  To: Michael Mueller
  Cc: linux-s390, Cornelia Huck, kvm, Gleb Natapov, qemu-devel,
	linux-kernel, libvir-list, Christian Borntraeger, Alexander Graf,
	Jason J. Herne, Daniel Hansel, Paolo Bonzini, Jiri Denemark,
	Andreas Faerber, Richard Henderson

On Wed, Apr 01, 2015 at 09:05:31PM +0200, Michael Mueller wrote:
> On Wed, 1 Apr 2015 13:59:05 -0300
> Eduardo Habkost <ehabkost@redhat.com> wrote:
> 
> > > Not directly invalid as "-cpu none" will be the same as omitting the -cpu option.
> > > KVM will setup the vcpu properties withou any QEMU control to whatever the hosting
> > > machine and the kvm kernel code offers. That will allow to run QEMU against a KVM
> > > version that is not aware of the s390 cpu model ioctls.  
> > 
> > It looks like we have conflicting expectations about
> > query-cpu-definitions, it seems: on the one hand, if "-cpu none" is
> > valid I believe it should appear on the query-cpu-definitions return
> > value; on the other hand, it is not (always?) migration-safe, so just
> > comparing the source query-cpus data with the target
> > query-cpu-definitions data wouldn't be enough to ensure live-migration
> > safety.
> 
> There are other cases as well where the value given with -cpu is *not* part
> of the cpu definition list and that is aliases:
> 
> [mimu@p57lp59 (master-cpu-model) qemu]$ ./s390x-softmmu/qemu-system-s390x -machine
> s390-ccw,accel=kvm -cpu ?
> 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 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)
> 
> As you can see "host" is in s390x case always an alias and also all other aliases
> are normalized to their real cpu models in the cpu-definitions list.
> 
> > 
> > On x86, we have a similar problem with "-cpu host", that changes
> > depending on the host hardware and host kernel. We solve this problem by
> > making libvirt code aware of the set of valid CPU models, and libvirt
> > has special cases for "-cpu host".
> 
> "-cpu host" is not a special case for s390, it will return ("2827-ga2", "kvm") as
> cpu model or whatever model the hosting system implements.
> 
> > 
> > If you don't want to encode that knowledge in libvirt or other
> > management software for s390, it looks like you need something like a
> > "stable-abi-safe" field on CpuDefinitionInfo?
> 
> Exactly that fulfills the "name" field for s390 already in my view.
> 
> And cpu model "none" just means that QEMU does not manage the cpu model. That's also
> the reason why I initially returned an empty "[]" model and not "none". This somewhat
> convinces me to go back to this approach...

I understand the reasons for your approach and it seems to work for
s390, but the only problem I see is that you are adding an additional
(undocumented?) s390-specific constraint to the semantics of
query-cpu-models: that the model name will appear on the list only if it
can be safely migratable. This may prevent us from unifying CPU model
code into generic code later.

But if we add a simple stable-abi-safe field to the list (even if s390
set it to to true for all models and omit aliases and "none" in this
first version), we will have clearer semantics that can still be
honoured by other architectures (and by generic code) later.

-- 
Eduardo

^ permalink raw reply	[flat|nested] 45+ messages in thread

* Re: [Qemu-devel] [PATCH v4 11/15] target-s390x: New QMP command query-cpu-model
  2015-04-01 23:05               ` Eduardo Habkost
@ 2015-04-02  7:09                 ` Michael Mueller
  2015-04-02 15:15                   ` Eduardo Habkost
  0 siblings, 1 reply; 45+ messages in thread
From: Michael Mueller @ 2015-04-02  7:09 UTC (permalink / raw)
  To: Eduardo Habkost
  Cc: linux-s390, Cornelia Huck, kvm, Gleb Natapov, qemu-devel,
	linux-kernel, libvir-list, Christian Borntraeger, Alexander Graf,
	Jason J. Herne, Daniel Hansel, Paolo Bonzini, Jiri Denemark,
	Andreas Faerber, Richard Henderson

On Wed, 1 Apr 2015 20:05:24 -0300
Eduardo Habkost <ehabkost@redhat.com> wrote:

> > > 
> > > If you don't want to encode that knowledge in libvirt or other
> > > management software for s390, it looks like you need something like a
> > > "stable-abi-safe" field on CpuDefinitionInfo?  
> > 
> > Exactly that fulfills the "name" field for s390 already in my view.
> > 
> > And cpu model "none" just means that QEMU does not manage the cpu model. That's also
> > the reason why I initially returned an empty "[]" model and not "none". This somewhat
> > convinces me to go back to this approach...  
> 
> I understand the reasons for your approach and it seems to work for
> s390, but the only problem I see is that you are adding an additional
> (undocumented?) s390-specific constraint to the semantics of
> query-cpu-models: that the model name will appear on the list only if it
> can be safely migratable. This may prevent us from unifying CPU model
> code into generic code later.

I agree that an aliases is something different compared with the CPU model none as
there is a CPU class representing it. And thus, when implicitly or explicitly selected,
shall be presented in the CPU definition list as well. If I would set "runnable" to
false as it now (bad), it would be sorted out by the "considered for migration" test but it
would be misleading as it is always runnable. Though an additional field like "migrate-able"
could express that characteristic.

> 
> But if we add a simple stable-abi-safe field to the list (even if s390
> set it to to true for all models and omit aliases and "none" in this
> first version), we will have clearer semantics that can still be
> honoured by other architectures (and by generic code) later.

To be honest I currently don't right get the idea that you follow with that
stable-abi-save field... But eventually yes (I wrote this before the section above)

The stable-abi-save field means: "Take me into account for whatever kind of
CPU model related comparison you perform between two running QEMU instances as I
represent a well defined aspect.

Thus CPU model none will be { "name": "none", "runnable: true, "stable-abi-save": false } and
the aliases can be represented as { "name": <alias>, "runnable": <true|false>, "stable-abi-save":
false } in the s390 case, right?

Michael



^ permalink raw reply	[flat|nested] 45+ messages in thread

* Re: [Qemu-devel] [PATCH v4 11/15] target-s390x: New QMP command query-cpu-model
  2015-04-02  7:09                 ` Michael Mueller
@ 2015-04-02 15:15                   ` Eduardo Habkost
  0 siblings, 0 replies; 45+ messages in thread
From: Eduardo Habkost @ 2015-04-02 15:15 UTC (permalink / raw)
  To: Michael Mueller
  Cc: linux-s390, Cornelia Huck, kvm, Gleb Natapov, qemu-devel,
	linux-kernel, libvir-list, Christian Borntraeger, Alexander Graf,
	Jason J. Herne, Daniel Hansel, Paolo Bonzini, Jiri Denemark,
	Andreas Faerber, Richard Henderson

On Thu, Apr 02, 2015 at 09:09:07AM +0200, Michael Mueller wrote:
> On Wed, 1 Apr 2015 20:05:24 -0300
> Eduardo Habkost <ehabkost@redhat.com> wrote:
> 
> > > > 
> > > > If you don't want to encode that knowledge in libvirt or other
> > > > management software for s390, it looks like you need something like a
> > > > "stable-abi-safe" field on CpuDefinitionInfo?  
> > > 
> > > Exactly that fulfills the "name" field for s390 already in my view.
> > > 
> > > And cpu model "none" just means that QEMU does not manage the cpu model. That's also
> > > the reason why I initially returned an empty "[]" model and not "none". This somewhat
> > > convinces me to go back to this approach...  
> > 
> > I understand the reasons for your approach and it seems to work for
> > s390, but the only problem I see is that you are adding an additional
> > (undocumented?) s390-specific constraint to the semantics of
> > query-cpu-models: that the model name will appear on the list only if it
> > can be safely migratable. This may prevent us from unifying CPU model
> > code into generic code later.
> 
> I agree that an aliases is something different compared with the CPU model none as
> there is a CPU class representing it. And thus, when implicitly or explicitly selected,
> shall be presented in the CPU definition list as well. If I would set "runnable" to
> false as it now (bad), it would be sorted out by the "considered for migration" test but it
> would be misleading as it is always runnable. Though an additional field like "migrate-able"
> could express that characteristic.

Exactly.

> 
> > 
> > But if we add a simple stable-abi-safe field to the list (even if s390
> > set it to to true for all models and omit aliases and "none" in this
> > first version), we will have clearer semantics that can still be
> > honoured by other architectures (and by generic code) later.
> 
> To be honest I currently don't right get the idea that you follow with that
> stable-abi-save field... But eventually yes (I wrote this before the section above)
> 
> The stable-abi-save field means: "Take me into account for whatever kind of
> CPU model related comparison you perform between two running QEMU instances as I
> represent a well defined aspect.

Yes. "stable-abi-safe" would mean that nothing guest-visible will change
in the CPU model when running a different QEMU version or running in a
different host, thus making it safe to live-migrate (as long as you keep
the same machine+accelerator and don't change other guest-visible
configuration in the QEMU command-line, of course). That's a constraint
we already keep in the x86 CPU models, except for "-cpu host".

In other words, it means "as long as the name matches the query-cpus
output from the source host, it is guaranteed to be safe to
live-migrate".  Which is the constraint you need, right?

(I am not 100% sure about the naming. Maybe we should call it
"live-migration-safe"?)

> 
> Thus CPU model none will be { "name": "none", "runnable: true, "stable-abi-save": false } and
> the aliases can be represented as { "name": <alias>, "runnable": <true|false>, "stable-abi-save":
> false } in the s390 case, right?

Exactly. We don't need to return them in the first version if you don't
need to (althought I don't see a reason to not return them). It will
just allow us to return them in the future.

-- 
Eduardo

^ permalink raw reply	[flat|nested] 45+ messages in thread

end of thread, other threads:[~2015-04-02 15:15 UTC | newest]

Thread overview: 45+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-03-30 14:28 [PATCH v4 00/15] s390x cpu model implementation Michael Mueller
2015-03-30 14:28 ` [PATCH v4 01/15] Introduce stub routine cpu_desc_avail Michael Mueller
2015-03-30 14:28 ` [PATCH v4 02/15] target-s390x: Introduce cpu facilities Michael Mueller
2015-03-30 14:28 ` [PATCH v4 03/15] target-s390x: Generate facility defines per cpu model Michael Mueller
2015-03-30 14:28 ` [PATCH v4 04/15] target-s390x: Introduce cpu models Michael Mueller
2015-03-30 14:28 ` [PATCH v4 05/15] target-s390x: Define cpu model specific facility lists Michael Mueller
2015-03-30 14:28 ` [PATCH v4 06/15] target-s390x: Add cpu model alias definition routines Michael Mueller
2015-03-30 14:28 ` [PATCH v4 07/15] target-s390x: Update linux-headers/asm-s390/kvm.h Michael Mueller
2015-03-30 19:36   ` Christian Borntraeger
2015-03-31  7:25     ` Michael Mueller
2015-03-30 14:28 ` [PATCH v4 08/15] target-s390x: Add KVM VM attribute interface for cpu models Michael Mueller
2015-03-30 14:28 ` [PATCH v4 09/15] target-s390x: Add cpu class initialization routines Michael Mueller
2015-03-30 14:28 ` [PATCH v4 10/15] target-s390x: Prepare accelerator during cpu object realization Michael Mueller
2015-03-30 19:33   ` Eduardo Habkost
2015-03-31 10:26     ` [Qemu-devel] " Michael Mueller
2015-03-30 14:28 ` [PATCH v4 11/15] target-s390x: New QMP command query-cpu-model Michael Mueller
2015-03-30 19:50   ` Eduardo Habkost
2015-03-31  9:10     ` [Qemu-devel] " Michael Mueller
2015-03-30 20:17   ` Eduardo Habkost
2015-03-30 20:20     ` [Qemu-devel] " Eric Blake
2015-03-31 13:16       ` Eduardo Habkost
2015-03-31 11:21     ` Michael Mueller
2015-03-31 18:28       ` Eduardo Habkost
2015-03-30 20:19   ` Eric Blake
2015-03-31  7:56     ` Michael Mueller
2015-03-31 18:35   ` Eduardo Habkost
2015-03-31 20:09     ` Michael Mueller
2015-04-01 13:01       ` Eduardo Habkost
2015-04-01 16:31         ` [Qemu-devel] " Michael Mueller
2015-04-01 16:59           ` Eduardo Habkost
2015-04-01 19:05             ` Michael Mueller
2015-04-01 19:10               ` Michael Mueller
2015-04-01 23:05               ` Eduardo Habkost
2015-04-02  7:09                 ` Michael Mueller
2015-04-02 15:15                   ` Eduardo Habkost
2015-03-30 14:28 ` [PATCH v4 12/15] Add optional parameters to QMP command query-cpu-definitions Michael Mueller
2015-03-30 20:28   ` [Qemu-devel] " Eric Blake
2015-03-31  7:42     ` Michael Mueller
2015-03-31 19:46   ` Eduardo Habkost
2015-03-31 19:50     ` Eric Blake
2015-03-31 20:22     ` [Qemu-devel] " Michael Mueller
2015-03-30 14:28 ` [PATCH v4 13/15] target-s390x: Extend " Michael Mueller
2015-03-30 19:54   ` Eduardo Habkost
2015-03-30 14:28 ` [PATCH v4 14/15] target-s390x: Introduce facility test routine Michael Mueller
2015-03-30 14:28 ` [PATCH v4 15/15] target-s390x: Enable cpu model usage Michael Mueller

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).