[Qemu-devel] [PATCH v3 0/8] s390x: storage key migration

[Qemu-devel] [PATCH v3 0/8] s390x: storage key migration

[Cornelia Huck]

The next edition of the storage key migration patches.

Changes from v2:
- more tweaks to use qemu error handling infrastructure
- some message tweaks

Cornelia Huck (1):
  s390x: add 2.5 compat s390-ccw-virtio machine

Jason J. Herne (7):
  s390x: Create QOM device for s390 storage keys
  s390x: Enable new s390-storage-keys device
  s390x: Dump storage keys qmp command
  s390x: Dump-skeys hmp support
  s390x: Info skeys sub-command
  s390x: Migrate guest storage keys (initial memory only)
  s390x: Disable storage key migration on old machine type

 MAINTAINERS                     |   1 +
 hmp-commands.hx                 |  18 ++
 hw/s390x/Makefile.objs          |   2 +
 hw/s390x/s390-skeys-kvm.c       |  75 ++++++++
 hw/s390x/s390-skeys.c           | 414 ++++++++++++++++++++++++++++++++++++++++
 hw/s390x/s390-virtio-ccw.c      |  39 +++-
 hw/s390x/s390-virtio.c          |  11 +-
 hw/s390x/s390-virtio.h          |   2 +-
 include/hw/s390x/storage-keys.h |  60 ++++++
 monitor.c                       |  20 ++
 qapi-schema.json                |  14 ++
 qmp-commands.hx                 |  25 +++
 target-s390x/cpu.h              |   2 -
 target-s390x/mem_helper.c       |  46 ++++-
 target-s390x/mmu_helper.c       |  28 ++-
 trace-events                    |   4 +
 16 files changed, 735 insertions(+), 26 deletions(-)
 create mode 100644 hw/s390x/s390-skeys-kvm.c
 create mode 100644 hw/s390x/s390-skeys.c
 create mode 100644 include/hw/s390x/storage-keys.h

-- 
2.5.1

[Qemu-devel] [PATCH v3 1/8] s390x: add 2.5 compat s390-ccw-virtio machine

[Cornelia Huck]

Reviewed-by: Jason J. Herne <jjherne@linux.vnet.ibm.com>
Signed-off-by: Cornelia Huck <cornelia.huck@de.ibm.com>
Acked-by: Christian Borntraeger <borntraeger@de.ibm.com>
---
 hw/s390x/s390-virtio-ccw.c | 19 +++++++++++++++++--
 1 file changed, 17 insertions(+), 2 deletions(-)

diff --git a/hw/s390x/s390-virtio-ccw.c b/hw/s390x/s390-virtio-ccw.c
index 4c51d1a..71df282 100644
--- a/hw/s390x/s390-virtio-ccw.c
+++ b/hw/s390x/s390-virtio-ccw.c
@@ -287,9 +287,7 @@ static void ccw_machine_2_4_class_init(ObjectClass *oc, void *data)
     MachineClass *mc = MACHINE_CLASS(oc);
 
     mc->name = "s390-ccw-virtio-2.4";
-    mc->alias = "s390-ccw-virtio";
     mc->desc = "VirtIO-ccw based S390 machine v2.4";
-    mc->is_default = 1;
 }
 
 static const TypeInfo ccw_machine_2_4_info = {
@@ -298,10 +296,27 @@ static const TypeInfo ccw_machine_2_4_info = {
     .class_init    = ccw_machine_2_4_class_init,
 };
 
+static void ccw_machine_2_5_class_init(ObjectClass *oc, void *data)
+{
+    MachineClass *mc = MACHINE_CLASS(oc);
+
+    mc->name = "s390-ccw-virtio-2.5";
+    mc->alias = "s390-ccw-virtio";
+    mc->desc = "VirtIO-ccw based S390 machine v2.5";
+    mc->is_default = 1;
+}
+
+static const TypeInfo ccw_machine_2_5_info = {
+    .name          = TYPE_S390_CCW_MACHINE "2.5",
+    .parent        = TYPE_S390_CCW_MACHINE,
+    .class_init    = ccw_machine_2_5_class_init,
+};
+
 static void ccw_machine_register_types(void)
 {
     type_register_static(&ccw_machine_info);
     type_register_static(&ccw_machine_2_4_info);
+    type_register_static(&ccw_machine_2_5_info);
 }
 
 type_init(ccw_machine_register_types)
-- 
2.5.1

[Qemu-devel] [PATCH v3 2/8] s390x: Create QOM device for s390 storage keys

[Cornelia Huck]

From: "Jason J. Herne" <jjherne@linux.vnet.ibm.com>

A new QOM style device is provided to back guest storage keys. A special
version for KVM is created, which handles the storage key access via
KVM_S390_GET_SKEYS and KVM_S390_SET_SKEYS ioctl.

Reviewed-by: David Hildenbrand <dahi@linux.vnet.ibm.com>
Signed-off-by: Jason J. Herne <jjherne@linux.vnet.ibm.com>
Signed-off-by: Cornelia Huck <cornelia.huck@de.ibm.com>
---
 MAINTAINERS                     |   1 +
 hw/s390x/Makefile.objs          |   2 +
 hw/s390x/s390-skeys-kvm.c       |  75 +++++++++++++++++++++
 hw/s390x/s390-skeys.c           | 141 ++++++++++++++++++++++++++++++++++++++++
 include/hw/s390x/storage-keys.h |  55 ++++++++++++++++
 5 files changed, 274 insertions(+)
 create mode 100644 hw/s390x/s390-skeys-kvm.c
 create mode 100644 hw/s390x/s390-skeys.c
 create mode 100644 include/hw/s390x/storage-keys.h

diff --git a/MAINTAINERS b/MAINTAINERS
index 08f356a..5a4bac8 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -569,6 +569,7 @@ F: hw/s390x/css.[hc]
 F: hw/s390x/sclp*.[hc]
 F: hw/s390x/ipl*.[hc]
 F: hw/s390x/*pci*.[hc]
+F: hw/s390x/s390-skeys*.c
 F: include/hw/s390x/
 F: pc-bios/s390-ccw/
 T: git git://github.com/cohuck/qemu virtio-ccw-upstr
diff --git a/hw/s390x/Makefile.objs b/hw/s390x/Makefile.objs
index 27cd75a..527d754 100644
--- a/hw/s390x/Makefile.objs
+++ b/hw/s390x/Makefile.objs
@@ -9,3 +9,5 @@ obj-y += css.o
 obj-y += s390-virtio-ccw.o
 obj-y += virtio-ccw.o
 obj-y += s390-pci-bus.o s390-pci-inst.o
+obj-y += s390-skeys.o
+obj-$(CONFIG_KVM) += s390-skeys-kvm.o
diff --git a/hw/s390x/s390-skeys-kvm.c b/hw/s390x/s390-skeys-kvm.c
new file mode 100644
index 0000000..682949a
--- /dev/null
+++ b/hw/s390x/s390-skeys-kvm.c
@@ -0,0 +1,75 @@
+/*
+ * s390 storage key device
+ *
+ * Copyright 2015 IBM Corp.
+ * Author(s): Jason J. Herne <jjherne@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 "hw/s390x/storage-keys.h"
+#include "sysemu/kvm.h"
+#include "qemu/error-report.h"
+
+static int kvm_s390_skeys_enabled(S390SKeysState *ss)
+{
+    S390SKeysClass *skeyclass = S390_SKEYS_GET_CLASS(ss);
+    uint8_t single_key;
+    int r;
+
+    r = skeyclass->get_skeys(ss, 0, 1, &single_key);
+    if (r != 0 && r != KVM_S390_GET_SKEYS_NONE) {
+        error_report("S390_GET_KEYS error %d\n", r);
+    }
+    return (r == 0);
+}
+
+static int kvm_s390_skeys_get(S390SKeysState *ss, uint64_t start_gfn,
+                              uint64_t count, uint8_t *keys)
+{
+    struct kvm_s390_skeys args = {
+        .start_gfn = start_gfn,
+        .count = count,
+        .skeydata_addr = (__u64)keys
+    };
+
+    return kvm_vm_ioctl(kvm_state, KVM_S390_GET_SKEYS, &args);
+}
+
+static int kvm_s390_skeys_set(S390SKeysState *ss, uint64_t start_gfn,
+                              uint64_t count, uint8_t *keys)
+{
+    struct kvm_s390_skeys args = {
+        .start_gfn = start_gfn,
+        .count = count,
+        .skeydata_addr = (__u64)keys
+    };
+
+    return kvm_vm_ioctl(kvm_state, KVM_S390_SET_SKEYS, &args);
+}
+
+static void kvm_s390_skeys_class_init(ObjectClass *oc, void *data)
+{
+    S390SKeysClass *skeyclass = S390_SKEYS_CLASS(oc);
+
+    skeyclass->skeys_enabled = kvm_s390_skeys_enabled;
+    skeyclass->get_skeys = kvm_s390_skeys_get;
+    skeyclass->set_skeys = kvm_s390_skeys_set;
+}
+
+static const TypeInfo kvm_s390_skeys_info = {
+    .name          = TYPE_KVM_S390_SKEYS,
+    .parent        = TYPE_S390_SKEYS,
+    .instance_size = sizeof(S390SKeysState),
+    .class_init    = kvm_s390_skeys_class_init,
+    .class_size    = sizeof(S390SKeysClass),
+};
+
+static void kvm_s390_skeys_register_types(void)
+{
+    type_register_static(&kvm_s390_skeys_info);
+}
+
+type_init(kvm_s390_skeys_register_types)
diff --git a/hw/s390x/s390-skeys.c b/hw/s390x/s390-skeys.c
new file mode 100644
index 0000000..77c42ff
--- /dev/null
+++ b/hw/s390x/s390-skeys.c
@@ -0,0 +1,141 @@
+/*
+ * s390 storage key device
+ *
+ * Copyright 2015 IBM Corp.
+ * Author(s): Jason J. Herne <jjherne@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 "hw/boards.h"
+#include "hw/s390x/storage-keys.h"
+#include "qemu/error-report.h"
+
+S390SKeysState *s390_get_skeys_device(void)
+{
+    S390SKeysState *ss;
+
+    ss = S390_SKEYS(object_resolve_path_type("", TYPE_S390_SKEYS, NULL));
+    assert(ss);
+    return ss;
+}
+
+void s390_skeys_init(void)
+{
+    Object *obj;
+
+    if (kvm_enabled()) {
+        obj = object_new(TYPE_KVM_S390_SKEYS);
+    } else {
+        obj = object_new(TYPE_QEMU_S390_SKEYS);
+    }
+    object_property_add_child(qdev_get_machine(), TYPE_S390_SKEYS,
+                              obj, NULL);
+    object_unref(obj);
+
+    qdev_init_nofail(DEVICE(obj));
+}
+
+static void qemu_s390_skeys_init(Object *obj)
+{
+    QEMUS390SKeysState *skeys = QEMU_S390_SKEYS(obj);
+    MachineState *machine = MACHINE(qdev_get_machine());
+
+    skeys->key_count = machine->maxram_size / TARGET_PAGE_SIZE;
+    skeys->keydata = g_malloc0(skeys->key_count);
+}
+
+static int qemu_s390_skeys_enabled(S390SKeysState *ss)
+{
+    return 1;
+}
+
+/*
+ * TODO: for memory hotplug support qemu_s390_skeys_set and qemu_s390_skeys_get
+ * will have to make sure that the given gfn belongs to a memory region and not
+ * a memory hole.
+ */
+static int qemu_s390_skeys_set(S390SKeysState *ss, uint64_t start_gfn,
+                              uint64_t count, uint8_t *keys)
+{
+    QEMUS390SKeysState *skeydev = QEMU_S390_SKEYS(ss);
+    int i;
+
+    /* Check for uint64 overflow and access beyond end of key data */
+    if (start_gfn + count > skeydev->key_count || start_gfn + count < count) {
+        error_report("Error: Setting storage keys for page beyond the end "
+                "of memory. gfn=%" PRIx64 " count=%" PRId64 "\n", start_gfn,
+                count);
+        return -EINVAL;
+    }
+
+    for (i = 0; i < count; i++) {
+        skeydev->keydata[start_gfn + i] = keys[i];
+    }
+    return 0;
+}
+
+static int qemu_s390_skeys_get(S390SKeysState *ss, uint64_t start_gfn,
+                               uint64_t count, uint8_t *keys)
+{
+    QEMUS390SKeysState *skeydev = QEMU_S390_SKEYS(ss);
+    int i;
+
+    /* Check for uint64 overflow and access beyond end of key data */
+    if (start_gfn + count > skeydev->key_count || start_gfn + count < count) {
+        error_report("Error: Getting storage keys for page beyond the end "
+                "of memory. gfn=%" PRIx64 " count=%" PRId64 "\n", start_gfn,
+                count);
+        return -EINVAL;
+    }
+
+    for (i = 0; i < count; i++) {
+        keys[i] = skeydev->keydata[start_gfn + i];
+    }
+    return 0;
+}
+
+static void qemu_s390_skeys_class_init(ObjectClass *oc, void *data)
+{
+    S390SKeysClass *skeyclass = S390_SKEYS_CLASS(oc);
+
+    skeyclass->skeys_enabled = qemu_s390_skeys_enabled;
+    skeyclass->get_skeys = qemu_s390_skeys_get;
+    skeyclass->set_skeys = qemu_s390_skeys_set;
+}
+
+static const TypeInfo qemu_s390_skeys_info = {
+    .name          = TYPE_QEMU_S390_SKEYS,
+    .parent        = TYPE_S390_SKEYS,
+    .instance_init = qemu_s390_skeys_init,
+    .instance_size = sizeof(QEMUS390SKeysState),
+    .class_init    = qemu_s390_skeys_class_init,
+    .instance_size = sizeof(S390SKeysClass),
+};
+
+static void s390_skeys_class_init(ObjectClass *oc, void *data)
+{
+    DeviceClass *dc = DEVICE_CLASS(oc);
+
+    dc->hotpluggable = false;
+    set_bit(DEVICE_CATEGORY_MISC, dc->categories);
+}
+
+static const TypeInfo s390_skeys_info = {
+    .name          = TYPE_S390_SKEYS,
+    .parent        = TYPE_DEVICE,
+    .instance_size = sizeof(S390SKeysState),
+    .class_init    = s390_skeys_class_init,
+    .class_size    = sizeof(S390SKeysClass),
+    .abstract = true,
+};
+
+static void qemu_s390_skeys_register_types(void)
+{
+    type_register_static(&s390_skeys_info);
+    type_register_static(&qemu_s390_skeys_info);
+}
+
+type_init(qemu_s390_skeys_register_types)
diff --git a/include/hw/s390x/storage-keys.h b/include/hw/s390x/storage-keys.h
new file mode 100644
index 0000000..cfd7da7
--- /dev/null
+++ b/include/hw/s390x/storage-keys.h
@@ -0,0 +1,55 @@
+/*
+ * s390 storage key device
+ *
+ * Copyright 2015 IBM Corp.
+ * Author(s): Jason J. Herne <jjherne@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 __S390_STORAGE_KEYS_H
+#define __S390_STORAGE_KEYS_H
+
+#include <hw/qdev.h>
+
+#define TYPE_S390_SKEYS "s390-skeys"
+#define S390_SKEYS(obj) \
+    OBJECT_CHECK(S390SKeysState, (obj), TYPE_S390_SKEYS)
+
+typedef struct S390SKeysState {
+    DeviceState parent_obj;
+
+} S390SKeysState;
+
+#define S390_SKEYS_CLASS(klass) \
+    OBJECT_CLASS_CHECK(S390SKeysClass, (klass), TYPE_S390_SKEYS)
+#define S390_SKEYS_GET_CLASS(obj) \
+    OBJECT_GET_CLASS(S390SKeysClass, (obj), TYPE_S390_SKEYS)
+
+typedef struct S390SKeysClass {
+    DeviceClass parent_class;
+    int (*skeys_enabled)(S390SKeysState *ks);
+    int (*get_skeys)(S390SKeysState *ks, uint64_t start_gfn, uint64_t count,
+                     uint8_t *keys);
+    int (*set_skeys)(S390SKeysState *ks, uint64_t start_gfn, uint64_t count,
+                     uint8_t *keys);
+} S390SKeysClass;
+
+#define TYPE_KVM_S390_SKEYS "s390-skeys-kvm"
+#define TYPE_QEMU_S390_SKEYS "s390-skeys-qemu"
+#define QEMU_S390_SKEYS(obj) \
+    OBJECT_CHECK(QEMUS390SKeysState, (obj), TYPE_QEMU_S390_SKEYS)
+
+typedef struct QEMUS390SKeysState {
+    S390SKeysState parent_obj;
+    uint8_t *keydata;
+    uint32_t key_count;
+} QEMUS390SKeysState;
+
+void s390_skeys_init(void);
+
+S390SKeysState *s390_get_skeys_device(void);
+
+#endif /* __S390_STORAGE_KEYS_H */
-- 
2.5.1

[Qemu-devel] [PATCH v3 3/8] s390x: Enable new s390-storage-keys device

[Cornelia Huck]

From: "Jason J. Herne" <jjherne@linux.vnet.ibm.com>

s390 guest initialization is modified to make use of new s390-storage-keys
device. Old code that globally allocated storage key array is removed.
The new device enables storage key access for kvm guests.

Cache storage key QOM objects in frequently used helper functions to avoid a
performance hit every time we use one of these functions.

Reviewed-by: Cornelia Huck <cornelia.huck@de.ibm.com>
Reviewed-by: Thomas Huth <thuth@linux.vnet.ibm.com>
Reviewed-by: David Hildenbrand <dahi@linux.vnet.ibm.com>
Signed-off-by: Jason J. Herne <jjherne@linux.vnet.ibm.com>
Signed-off-by: Cornelia Huck <cornelia.huck@de.ibm.com>
---
 hw/s390x/s390-virtio-ccw.c |  8 ++++----
 hw/s390x/s390-virtio.c     | 11 +++++------
 hw/s390x/s390-virtio.h     |  2 +-
 target-s390x/cpu.h         |  2 --
 target-s390x/mem_helper.c  | 46 ++++++++++++++++++++++++++++++++++++++++------
 target-s390x/mmu_helper.c  | 28 +++++++++++++++++++++++-----
 trace-events               |  4 ++++
 7 files changed, 77 insertions(+), 24 deletions(-)

diff --git a/hw/s390x/s390-virtio-ccw.c b/hw/s390x/s390-virtio-ccw.c
index 71df282..0a057ae 100644
--- a/hw/s390x/s390-virtio-ccw.c
+++ b/hw/s390x/s390-virtio-ccw.c
@@ -19,6 +19,7 @@
 #include "virtio-ccw.h"
 #include "qemu/config-file.h"
 #include "s390-pci-bus.h"
+#include "hw/s390x/storage-keys.h"
 
 #define TYPE_S390_CCW_MACHINE               "s390-ccw-machine"
 
@@ -105,7 +106,6 @@ static void ccw_init(MachineState *machine)
     MemoryRegion *sysmem = get_system_memory();
     MemoryRegion *ram = g_new(MemoryRegion, 1);
     sclpMemoryHotplugDev *mhd = init_sclp_memory_hotplug_dev();
-    uint8_t *storage_keys;
     int ret;
     VirtualCssBus *css_bus;
     DeviceState *dev;
@@ -179,11 +179,11 @@ static void ccw_init(MachineState *machine)
         mhd->standby_mem_size = standby_mem_size;
     }
 
-    /* allocate storage keys */
-    storage_keys = g_malloc0(my_ram_size / TARGET_PAGE_SIZE);
+    /* Initialize storage key device */
+    s390_skeys_init();
 
     /* init CPUs */
-    s390_init_cpus(machine->cpu_model, storage_keys);
+    s390_init_cpus(machine->cpu_model);
 
     if (kvm_enabled()) {
         kvm_s390_enable_css_support(s390_cpu_addr2state(0));
diff --git a/hw/s390x/s390-virtio.c b/hw/s390x/s390-virtio.c
index 1284e77..6cc6b5d 100644
--- a/hw/s390x/s390-virtio.c
+++ b/hw/s390x/s390-virtio.c
@@ -38,6 +38,7 @@
 #include "hw/s390x/sclp.h"
 #include "hw/s390x/s390_flic.h"
 #include "hw/s390x/s390-virtio.h"
+#include "hw/s390x/storage-keys.h"
 #include "cpu.h"
 
 //#define DEBUG_S390
@@ -164,7 +165,7 @@ void s390_init_ipl_dev(const char *kernel_filename,
     qdev_init_nofail(dev);
 }
 
-void s390_init_cpus(const char *cpu_model, uint8_t *storage_keys)
+void s390_init_cpus(const char *cpu_model)
 {
     int i;
 
@@ -184,7 +185,6 @@ void s390_init_cpus(const char *cpu_model, uint8_t *storage_keys)
         ipi_states[i] = cpu;
         cs->halted = 1;
         cs->exception_index = EXCP_HLT;
-        cpu->env.storage_keys = storage_keys;
     }
 }
 
@@ -264,7 +264,6 @@ static void s390_init(MachineState *machine)
     MemoryRegion *sysmem = get_system_memory();
     MemoryRegion *ram = g_new(MemoryRegion, 1);
     int increment_size = 20;
-    uint8_t *storage_keys;
     void *virtio_region;
     hwaddr virtio_region_len;
     hwaddr virtio_region_start;
@@ -306,11 +305,11 @@ static void s390_init(MachineState *machine)
     cpu_physical_memory_unmap(virtio_region, virtio_region_len, 1,
                               virtio_region_len);
 
-    /* allocate storage keys */
-    storage_keys = g_malloc0(my_ram_size / TARGET_PAGE_SIZE);
+    /* Initialize storage key device */
+    s390_skeys_init();
 
     /* init CPUs */
-    s390_init_cpus(machine->cpu_model, storage_keys);
+    s390_init_cpus(machine->cpu_model);
 
     /* Create VirtIO network adapters */
     s390_create_virtio_net((BusState *)s390_bus, "virtio-net-s390");
diff --git a/hw/s390x/s390-virtio.h b/hw/s390x/s390-virtio.h
index c847853..cf68796 100644
--- a/hw/s390x/s390-virtio.h
+++ b/hw/s390x/s390-virtio.h
@@ -19,7 +19,7 @@
 typedef int (*s390_virtio_fn)(const uint64_t *args);
 void s390_register_virtio_hypercall(uint64_t code, s390_virtio_fn fn);
 
-void s390_init_cpus(const char *cpu_model, uint8_t *storage_keys);
+void s390_init_cpus(const char *cpu_model);
 void s390_init_ipl_dev(const char *kernel_filename,
                        const char *kernel_cmdline,
                        const char *initrd_filename,
diff --git a/target-s390x/cpu.h b/target-s390x/cpu.h
index 63aebf4..b650890 100644
--- a/target-s390x/cpu.h
+++ b/target-s390x/cpu.h
@@ -143,8 +143,6 @@ typedef struct CPUS390XState {
     uint32_t cpu_num;
     uint32_t machine_type;
 
-    uint8_t *storage_keys;
-
     uint64_t tod_offset;
     uint64_t tod_basetime;
     QEMUTimer *tod_timer;
diff --git a/target-s390x/mem_helper.c b/target-s390x/mem_helper.c
index 6f8bd79..84bf198 100644
--- a/target-s390x/mem_helper.c
+++ b/target-s390x/mem_helper.c
@@ -21,6 +21,7 @@
 #include "cpu.h"
 #include "exec/helper-proto.h"
 #include "exec/cpu_ldst.h"
+#include "hw/s390x/storage-keys.h"
 
 /*****************************************************************************/
 /* Softmmu support */
@@ -937,40 +938,73 @@ uint32_t HELPER(tprot)(uint64_t a1, uint64_t a2)
 /* insert storage key extended */
 uint64_t HELPER(iske)(CPUS390XState *env, uint64_t r2)
 {
+    static S390SKeysState *ss;
+    static S390SKeysClass *skeyclass;
     uint64_t addr = get_address(env, 0, 0, r2);
+    uint8_t key;
 
     if (addr > ram_size) {
         return 0;
     }
 
-    return env->storage_keys[addr / TARGET_PAGE_SIZE];
+    if (unlikely(!ss)) {
+        ss = s390_get_skeys_device();
+        skeyclass = S390_SKEYS_GET_CLASS(ss);
+    }
+
+    if (skeyclass->get_skeys(ss, addr / TARGET_PAGE_SIZE, 1, &key)) {
+        return 0;
+    }
+    return key;
 }
 
 /* set storage key extended */
 void HELPER(sske)(CPUS390XState *env, uint64_t r1, uint64_t r2)
 {
+    static S390SKeysState *ss;
+    static S390SKeysClass *skeyclass;
     uint64_t addr = get_address(env, 0, 0, r2);
+    uint8_t key;
 
     if (addr > ram_size) {
         return;
     }
 
-    env->storage_keys[addr / TARGET_PAGE_SIZE] = r1;
+    if (unlikely(!ss)) {
+        ss = s390_get_skeys_device();
+        skeyclass = S390_SKEYS_GET_CLASS(ss);
+    }
+
+    key = (uint8_t) r1;
+    skeyclass->set_skeys(ss, addr / TARGET_PAGE_SIZE, 1, &key);
 }
 
 /* reset reference bit extended */
 uint32_t HELPER(rrbe)(CPUS390XState *env, uint64_t r2)
 {
-    uint8_t re;
-    uint8_t key;
+    static S390SKeysState *ss;
+    static S390SKeysClass *skeyclass;
+    uint8_t re, key;
 
     if (r2 > ram_size) {
         return 0;
     }
 
-    key = env->storage_keys[r2 / TARGET_PAGE_SIZE];
+    if (unlikely(!ss)) {
+        ss = s390_get_skeys_device();
+        skeyclass = S390_SKEYS_GET_CLASS(ss);
+    }
+
+    if (skeyclass->get_skeys(ss, r2 / TARGET_PAGE_SIZE, 1, &key)) {
+        return 0;
+    }
+
     re = key & (SK_R | SK_C);
-    env->storage_keys[r2 / TARGET_PAGE_SIZE] = (key & ~SK_R);
+    key &= ~SK_R;
+
+    if (skeyclass->set_skeys(ss, r2 / TARGET_PAGE_SIZE, 1, &key)) {
+        return 0;
+    }
 
     /*
      * cc
diff --git a/target-s390x/mmu_helper.c b/target-s390x/mmu_helper.c
index 1ea6d81..058a370 100644
--- a/target-s390x/mmu_helper.c
+++ b/target-s390x/mmu_helper.c
@@ -19,6 +19,8 @@
 #include "exec/address-spaces.h"
 #include "cpu.h"
 #include "sysemu/kvm.h"
+#include "trace.h"
+#include "hw/s390x/storage-keys.h"
 
 /* #define DEBUG_S390 */
 /* #define DEBUG_S390_PTE */
@@ -309,8 +311,15 @@ static int mmu_translate_asce(CPUS390XState *env, target_ulong vaddr,
 int mmu_translate(CPUS390XState *env, target_ulong vaddr, int rw, uint64_t asc,
                   target_ulong *raddr, int *flags, bool exc)
 {
+    static S390SKeysState *ss;
+    static S390SKeysClass *skeyclass;
     int r = -1;
-    uint8_t *sk;
+    uint8_t key;
+
+    if (unlikely(!ss)) {
+        ss = s390_get_skeys_device();
+        skeyclass = S390_SKEYS_GET_CLASS(ss);
+    }
 
     *flags = PAGE_READ | PAGE_WRITE | PAGE_EXEC;
     vaddr &= TARGET_PAGE_MASK;
@@ -358,14 +367,23 @@ int mmu_translate(CPUS390XState *env, target_ulong vaddr, int rw, uint64_t asc,
     /* Convert real address -> absolute address */
     *raddr = mmu_real2abs(env, *raddr);
 
-    if (*raddr < ram_size) {
-        sk = &env->storage_keys[*raddr / TARGET_PAGE_SIZE];
+    if (r == 0 && *raddr < ram_size) {
+        if (skeyclass->get_skeys(ss, *raddr / TARGET_PAGE_SIZE, 1, &key)) {
+            trace_get_skeys_nonzero(r);
+            return 0;
+        }
+
         if (*flags & PAGE_READ) {
-            *sk |= SK_R;
+            key |= SK_R;
         }
 
         if (*flags & PAGE_WRITE) {
-            *sk |= SK_C;
+            key |= SK_C;
+        }
+
+        if (skeyclass->set_skeys(ss, *raddr / TARGET_PAGE_SIZE, 1, &key)) {
+            trace_set_skeys_nonzero(r);
+            return 0;
         }
     }
 
diff --git a/trace-events b/trace-events
index 8f9614a..0a82f0c 100644
--- a/trace-events
+++ b/trace-events
@@ -1373,6 +1373,10 @@ hbitmap_iter_skip_words(const void *hb, void *hbi, uint64_t pos, unsigned long c
 hbitmap_reset(void *hb, uint64_t start, uint64_t count, uint64_t sbit, uint64_t ebit) "hb %p items %"PRIu64",%"PRIu64" bits %"PRIu64"..%"PRIu64
 hbitmap_set(void *hb, uint64_t start, uint64_t count, uint64_t sbit, uint64_t ebit) "hb %p items %"PRIu64",%"PRIu64" bits %"PRIu64"..%"PRIu64
 
+# target-s390x/mmu_helper.c
+get_skeys_nonzero(int rc) "SKEY: Call to get_skeys unexpectedly returned %d"
+set_skeys_nonzero(int rc) "SKEY: Call to set_skeys unexpectedly returned %d"
+
 # target-s390x/ioinst.c
 ioinst(const char *insn) "IOINST: %s"
 ioinst_sch_id(const char *insn, int cssid, int ssid, int schid) "IOINST: %s (%x.%x.%04x)"
-- 
2.5.1

[Qemu-devel] [PATCH v3 4/8] s390x: Dump storage keys qmp command

[Cornelia Huck]

From: "Jason J. Herne" <jjherne@linux.vnet.ibm.com>

Provide a dump-skeys qmp command to allow the end user to dump storage
keys. This is useful for debugging problems with guest storage key support
within Qemu and for guest operating system developers.

Reviewed-by: Thomas Huth <thuth@linux.vnet.ibm.com>
Reviewed-by: David Hildenbrand <dahi@linux.vnet.ibm.com>
Signed-off-by: Jason J. Herne <jjherne@linux.vnet.ibm.com>
Signed-off-by: Cornelia Huck <cornelia.huck@de.ibm.com>
---
 hw/s390x/s390-skeys.c | 90 +++++++++++++++++++++++++++++++++++++++++++++++++--
 monitor.c             |  7 ++++
 qapi-schema.json      | 14 ++++++++
 qmp-commands.hx       | 25 ++++++++++++++
 4 files changed, 134 insertions(+), 2 deletions(-)

diff --git a/hw/s390x/s390-skeys.c b/hw/s390x/s390-skeys.c
index 77c42ff..ea5297d 100644
--- a/hw/s390x/s390-skeys.c
+++ b/hw/s390x/s390-skeys.c
@@ -10,9 +10,12 @@
  */
 
 #include "hw/boards.h"
+#include "qmp-commands.h"
 #include "hw/s390x/storage-keys.h"
 #include "qemu/error-report.h"
 
+#define S390_SKEYS_BUFFER_SIZE 131072  /* Room for 128k storage keys */
+
 S390SKeysState *s390_get_skeys_device(void)
 {
     S390SKeysState *ss;
@@ -38,6 +41,89 @@ void s390_skeys_init(void)
     qdev_init_nofail(DEVICE(obj));
 }
 
+static void write_keys(QEMUFile *f, uint8_t *keys, uint64_t startgfn,
+                       uint64_t count, Error **errp)
+{
+    uint64_t curpage = startgfn;
+    uint64_t maxpage = curpage + count - 1;
+    const char *fmt = "page=%03" PRIx64 ": key(%d) => ACC=%X, FP=%d, REF=%d,"
+                      " ch=%d, reserved=%d\n";
+    char buf[128];
+    int len;
+
+    for (; curpage <= maxpage; curpage++) {
+        uint8_t acc = (*keys & 0xF0) >> 4;
+        int fp =  (*keys & 0x08);
+        int ref = (*keys & 0x04);
+        int ch = (*keys & 0x02);
+        int res = (*keys & 0x01);
+
+        len = snprintf(buf, sizeof(buf), fmt, curpage,
+                       *keys, acc, fp, ref, ch, res);
+        qemu_put_buffer(f, (uint8_t *)buf, len);
+        keys++;
+    }
+}
+
+void qmp_dump_skeys(const char *filename, Error **errp)
+{
+    S390SKeysState *ss = s390_get_skeys_device();
+    S390SKeysClass *skeyclass = S390_SKEYS_GET_CLASS(ss);
+    const uint64_t total_count = ram_size / TARGET_PAGE_SIZE;
+    uint64_t handled_count = 0, cur_count;
+    Error *lerr = NULL;
+    vaddr cur_gfn = 0;
+    uint8_t *buf;
+    int ret;
+    QEMUFile *f;
+
+    /* Quick check to see if guest is using storage keys*/
+    if (!skeyclass->skeys_enabled(ss)) {
+        error_setg(errp, "This guest is not using storage keys - "
+                         "nothing to dump");
+        return;
+    }
+
+    f = qemu_fopen(filename, "wb");
+    if (!f) {
+        error_setg_file_open(errp, errno, filename);
+        return;
+    }
+
+    buf = g_try_malloc(S390_SKEYS_BUFFER_SIZE);
+    if (!buf) {
+        error_setg(errp, "Could not allocate memory");
+        goto out;
+    }
+
+    /* we'll only dump initial memory for now */
+    while (handled_count < total_count) {
+        /* Calculate how many keys to ask for & handle overflow case */
+        cur_count = MIN(total_count - handled_count, S390_SKEYS_BUFFER_SIZE);
+
+        ret = skeyclass->get_skeys(ss, cur_gfn, cur_count, buf);
+        if (ret < 0) {
+            error_setg(errp, "get_keys error %d", ret);
+            goto out_free;
+        }
+
+        /* write keys to stream */
+        write_keys(f, buf, cur_gfn, cur_count, &lerr);
+        if (lerr) {
+            goto out_free;
+        }
+
+        cur_gfn += cur_count;
+        handled_count += cur_count;
+    }
+
+out_free:
+    error_propagate(errp, lerr);
+    g_free(buf);
+out:
+    qemu_fclose(f);
+}
+
 static void qemu_s390_skeys_init(Object *obj)
 {
     QEMUS390SKeysState *skeys = QEMU_S390_SKEYS(obj);
@@ -66,7 +152,7 @@ static int qemu_s390_skeys_set(S390SKeysState *ss, uint64_t start_gfn,
     /* Check for uint64 overflow and access beyond end of key data */
     if (start_gfn + count > skeydev->key_count || start_gfn + count < count) {
         error_report("Error: Setting storage keys for page beyond the end "
-                "of memory. gfn=%" PRIx64 " count=%" PRId64 "\n", start_gfn,
+                "of memory: gfn=%" PRIx64 " count=%" PRId64 "\n", start_gfn,
                 count);
         return -EINVAL;
     }
@@ -86,7 +172,7 @@ static int qemu_s390_skeys_get(S390SKeysState *ss, uint64_t start_gfn,
     /* Check for uint64 overflow and access beyond end of key data */
     if (start_gfn + count > skeydev->key_count || start_gfn + count < count) {
         error_report("Error: Getting storage keys for page beyond the end "
-                "of memory. gfn=%" PRIx64 " count=%" PRId64 "\n", start_gfn,
+                "of memory: gfn=%" PRIx64 " count=%" PRId64 "\n", start_gfn,
                 count);
         return -EINVAL;
     }
diff --git a/monitor.c b/monitor.c
index fc32f12..daa3d98 100644
--- a/monitor.c
+++ b/monitor.c
@@ -5361,3 +5361,10 @@ void qmp_rtc_reset_reinjection(Error **errp)
     error_setg(errp, QERR_FEATURE_DISABLED, "rtc-reset-reinjection");
 }
 #endif
+
+#ifndef TARGET_S390X
+void qmp_dump_skeys(const char *filename, Error **errp)
+{
+    error_setg(errp, QERR_FEATURE_DISABLED, "dump-skeys");
+}
+#endif
diff --git a/qapi-schema.json b/qapi-schema.json
index 4342a08..67fef37 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -2058,6 +2058,20 @@
   'returns': 'DumpGuestMemoryCapability' }
 
 ##
+# @dump-skeys
+#
+# Dump guest's storage keys
+#
+# @filename: the path to the file to dump to
+#
+# This command is only supported on s390 architecture.
+#
+# Since: 2.5
+##
+{ 'command': 'dump-skeys',
+  'data': { 'filename': 'str' } }
+
+##
 # @netdev_add:
 #
 # Add a network backend.
diff --git a/qmp-commands.hx b/qmp-commands.hx
index ba630b1..9848fd8 100644
--- a/qmp-commands.hx
+++ b/qmp-commands.hx
@@ -872,6 +872,31 @@ Example:
 
 EQMP
 
+#if defined TARGET_S390X
+    {
+        .name       = "dump-skeys",
+        .args_type  = "filename:F",
+        .mhandler.cmd_new = qmp_marshal_input_dump_skeys,
+    },
+#endif
+
+SQMP
+dump-skeys
+----------
+
+Save guest storage keys to file.
+
+Arguments:
+
+- "filename": file path (json-string)
+
+Example:
+
+-> { "execute": "dump-skeys", "arguments": { "filename": "/tmp/skeys" } }
+<- { "return": {} }
+
+EQMP
+
     {
         .name       = "netdev_add",
         .args_type  = "netdev:O",
-- 
2.5.1

[Qemu-devel] [PATCH v3 5/8] s390x: Dump-skeys hmp support

[Cornelia Huck]

From: "Jason J. Herne" <jjherne@linux.vnet.ibm.com>

Add dump-skeys command to the human monitor.

Reviewed-by: Thomas Huth <thuth@linux.vnet.ibm.com>
Reviewed-by: David Hildenbrand <dahi@linux.vnet.ibm.com>
Signed-off-by: Jason J. Herne <jjherne@linux.vnet.ibm.com>
Signed-off-by: Cornelia Huck <cornelia.huck@de.ibm.com>
---
 hmp-commands.hx                 | 16 ++++++++++++++++
 hw/s390x/s390-skeys.c           | 12 ++++++++++++
 include/hw/s390x/storage-keys.h |  2 ++
 monitor.c                       |  4 ++++
 4 files changed, 34 insertions(+)

diff --git a/hmp-commands.hx b/hmp-commands.hx
index d3b7932..803ff91 100644
--- a/hmp-commands.hx
+++ b/hmp-commands.hx
@@ -1053,6 +1053,22 @@ gdb. Without -z|-l|-s, the dump format is ELF.
             together with begin.
 ETEXI
 
+#if defined(TARGET_S390X)
+    {
+        .name       = "dump-skeys",
+        .args_type  = "filename:F",
+        .params     = "",
+        .help       = "Save guest storage keys into file 'filename'.\n",
+        .mhandler.cmd = hmp_dump_skeys,
+    },
+#endif
+
+STEXI
+@item dump-skeys @var{filename}
+@findex dump-skeys
+Save guest storage keys to a file.
+ETEXI
+
     {
         .name       = "snapshot_blkdev",
         .args_type  = "reuse:-n,device:B,snapshot-file:s?,format:s?",
diff --git a/hw/s390x/s390-skeys.c b/hw/s390x/s390-skeys.c
index ea5297d..0ef3aea 100644
--- a/hw/s390x/s390-skeys.c
+++ b/hw/s390x/s390-skeys.c
@@ -65,6 +65,18 @@ static void write_keys(QEMUFile *f, uint8_t *keys, uint64_t startgfn,
     }
 }
 
+void hmp_dump_skeys(Monitor *mon, const QDict *qdict)
+{
+    const char *filename = qdict_get_str(qdict, "filename");
+    Error *err = NULL;
+
+    qmp_dump_skeys(filename, &err);
+    if (err) {
+        monitor_printf(mon, "%s\n", error_get_pretty(err));
+        error_free(err);
+    }
+}
+
 void qmp_dump_skeys(const char *filename, Error **errp)
 {
     S390SKeysState *ss = s390_get_skeys_device();
diff --git a/include/hw/s390x/storage-keys.h b/include/hw/s390x/storage-keys.h
index cfd7da7..0d04f19 100644
--- a/include/hw/s390x/storage-keys.h
+++ b/include/hw/s390x/storage-keys.h
@@ -13,6 +13,7 @@
 #define __S390_STORAGE_KEYS_H
 
 #include <hw/qdev.h>
+#include "monitor/monitor.h"
 
 #define TYPE_S390_SKEYS "s390-skeys"
 #define S390_SKEYS(obj) \
@@ -52,4 +53,5 @@ void s390_skeys_init(void);
 
 S390SKeysState *s390_get_skeys_device(void);
 
+void hmp_dump_skeys(Monitor *mon, const QDict *qdict);
 #endif /* __S390_STORAGE_KEYS_H */
diff --git a/monitor.c b/monitor.c
index daa3d98..3deba38 100644
--- a/monitor.c
+++ b/monitor.c
@@ -82,6 +82,10 @@
 #endif
 #include "hw/lm32/lm32_pic.h"
 
+#if defined(TARGET_S390X)
+#include "hw/s390x/storage-keys.h"
+#endif
+
 /*
  * Supported types:
  *
-- 
2.5.1

[Qemu-devel] [PATCH v3 6/8] s390x: Info skeys sub-command

[Cornelia Huck]

From: "Jason J. Herne" <jjherne@linux.vnet.ibm.com>

Provide an  info skeys hmp sub-command to allow the end user to dump a storage
key for a given address. This is useful for guest operating system developers.

Reviewed-by: Thomas Huth <thuth@linux.vnet.ibm.com>
Reviewed-by: David Hildenbrand <dahi@linux.vnet.ibm.com>
Signed-off-by: Jason J. Herne <jjherne@linux.vnet.ibm.com>
Signed-off-by: Cornelia Huck <cornelia.huck@de.ibm.com>
---
 hmp-commands.hx                 |  2 ++
 hw/s390x/s390-skeys.c           | 23 +++++++++++++++++++++++
 include/hw/s390x/storage-keys.h |  2 ++
 monitor.c                       |  9 +++++++++
 4 files changed, 36 insertions(+)

diff --git a/hmp-commands.hx b/hmp-commands.hx
index 803ff91..c61468e 100644
--- a/hmp-commands.hx
+++ b/hmp-commands.hx
@@ -1806,6 +1806,8 @@ show roms
 show the TPM device
 @item info memory-devices
 show the memory devices
+@item info skeys
+Display the value of a storage key (s390 only)
 @end table
 ETEXI
 
diff --git a/hw/s390x/s390-skeys.c b/hw/s390x/s390-skeys.c
index 0ef3aea..b8aacc8 100644
--- a/hw/s390x/s390-skeys.c
+++ b/hw/s390x/s390-skeys.c
@@ -65,6 +65,29 @@ static void write_keys(QEMUFile *f, uint8_t *keys, uint64_t startgfn,
     }
 }
 
+void hmp_info_skeys(Monitor *mon, const QDict *qdict)
+{
+    S390SKeysState *ss = s390_get_skeys_device();
+    S390SKeysClass *skeyclass = S390_SKEYS_GET_CLASS(ss);
+    uint64_t addr = qdict_get_int(qdict, "addr");
+    uint8_t key;
+    int r;
+
+    /* Quick check to see if guest is using storage keys*/
+    if (!skeyclass->skeys_enabled(ss)) {
+        monitor_printf(mon, "Error: This guest is not using storage keys\n");
+        return;
+    }
+
+    r = skeyclass->get_skeys(ss, addr / TARGET_PAGE_SIZE, 1, &key);
+    if (r < 0) {
+        monitor_printf(mon, "Error: %s\n", strerror(-r));
+        return;
+    }
+
+    monitor_printf(mon, "  key: 0x%X\n", key);
+}
+
 void hmp_dump_skeys(Monitor *mon, const QDict *qdict)
 {
     const char *filename = qdict_get_str(qdict, "filename");
diff --git a/include/hw/s390x/storage-keys.h b/include/hw/s390x/storage-keys.h
index 0d04f19..18e08d2 100644
--- a/include/hw/s390x/storage-keys.h
+++ b/include/hw/s390x/storage-keys.h
@@ -54,4 +54,6 @@ void s390_skeys_init(void);
 S390SKeysState *s390_get_skeys_device(void);
 
 void hmp_dump_skeys(Monitor *mon, const QDict *qdict);
+void hmp_info_skeys(Monitor *mon, const QDict *qdict);
+
 #endif /* __S390_STORAGE_KEYS_H */
diff --git a/monitor.c b/monitor.c
index 3deba38..451af6f 100644
--- a/monitor.c
+++ b/monitor.c
@@ -2881,6 +2881,15 @@ static mon_cmd_t info_cmds[] = {
         .help       = "Show rocker OF-DPA groups",
         .mhandler.cmd = hmp_rocker_of_dpa_groups,
     },
+#if defined(TARGET_S390X)
+    {
+        .name       = "skeys",
+        .args_type  = "addr:l",
+        .params     = "address",
+        .help       = "Display the value of a storage key",
+        .mhandler.cmd = hmp_info_skeys,
+    },
+#endif
     {
         .name       = NULL,
     },
-- 
2.5.1

[Qemu-devel] [PATCH v3 7/8] s390x: Migrate guest storage keys (initial memory only)

[Cornelia Huck]

From: "Jason J. Herne" <jjherne@linux.vnet.ibm.com>

Routines to save/load guest storage keys are provided. register_savevm is
called to register them as migration handlers.

We prepare the protocol to support more complex parameters. So we will
later be able to support standby memory (having empty holes), compression
and "state live migration" like done for ram.

Reviewed-by: David Hildenbrand <dahi@linux.vnet.ibm.com>
Signed-off-by: Jason J. Herne <jjherne@linux.vnet.ibm.com>
Signed-off-by: Cornelia Huck <cornelia.huck@de.ibm.com>
---
 hw/s390x/s390-skeys.c | 125 ++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 125 insertions(+)

diff --git a/hw/s390x/s390-skeys.c b/hw/s390x/s390-skeys.c
index b8aacc8..72df19d 100644
--- a/hw/s390x/s390-skeys.c
+++ b/hw/s390x/s390-skeys.c
@@ -11,10 +11,14 @@
 
 #include "hw/boards.h"
 #include "qmp-commands.h"
+#include "migration/qemu-file.h"
 #include "hw/s390x/storage-keys.h"
 #include "qemu/error-report.h"
 
 #define S390_SKEYS_BUFFER_SIZE 131072  /* Room for 128k storage keys */
+#define S390_SKEYS_SAVE_FLAG_EOS 0x01
+#define S390_SKEYS_SAVE_FLAG_SKEYS 0x02
+#define S390_SKEYS_SAVE_FLAG_ERROR 0x04
 
 S390SKeysState *s390_get_skeys_device(void)
 {
@@ -236,6 +240,126 @@ static const TypeInfo qemu_s390_skeys_info = {
     .instance_size = sizeof(S390SKeysClass),
 };
 
+static void s390_storage_keys_save(QEMUFile *f, void *opaque)
+{
+    S390SKeysState *ss = S390_SKEYS(opaque);
+    S390SKeysClass *skeyclass = S390_SKEYS_GET_CLASS(ss);
+    uint64_t pages_left = ram_size / TARGET_PAGE_SIZE;
+    uint64_t read_count, eos = S390_SKEYS_SAVE_FLAG_EOS;
+    vaddr cur_gfn = 0;
+    int error = 0;
+    uint8_t *buf;
+
+    if (!skeyclass->skeys_enabled(ss)) {
+        goto end_stream;
+    }
+
+    buf = g_try_malloc(S390_SKEYS_BUFFER_SIZE);
+    if (!buf) {
+        error_report("storage key save could not allocate memory\n");
+        goto end_stream;
+    }
+
+    /* We only support initial memory. Standby memory is not handled yet. */
+    qemu_put_be64(f, (cur_gfn * TARGET_PAGE_SIZE) | S390_SKEYS_SAVE_FLAG_SKEYS);
+    qemu_put_be64(f, pages_left);
+
+    while (pages_left) {
+        read_count = MIN(pages_left, S390_SKEYS_BUFFER_SIZE);
+
+        if (!error) {
+            error = skeyclass->get_skeys(ss, cur_gfn, read_count, buf);
+            if (error) {
+                /*
+                 * If error: we want to fill the stream with valid data instead
+                 * of stopping early so we pad the stream with 0x00 values and
+                 * use S390_SKEYS_SAVE_FLAG_ERROR to indicate failure to the
+                 * reading side.
+                 */
+                error_report("S390_GET_KEYS error %d\n", error);
+                memset(buf, 0, S390_SKEYS_BUFFER_SIZE);
+                eos = S390_SKEYS_SAVE_FLAG_ERROR;
+            }
+        }
+
+        qemu_put_buffer(f, buf, read_count);
+        cur_gfn += read_count;
+        pages_left -= read_count;
+    }
+
+    g_free(buf);
+end_stream:
+    qemu_put_be64(f, eos);
+}
+
+static int s390_storage_keys_load(QEMUFile *f, void *opaque, int version_id)
+{
+    S390SKeysState *ss = S390_SKEYS(opaque);
+    S390SKeysClass *skeyclass = S390_SKEYS_GET_CLASS(ss);
+    int ret = 0;
+
+    while (!ret) {
+        ram_addr_t addr;
+        int flags;
+
+        addr = qemu_get_be64(f);
+        flags = addr & ~TARGET_PAGE_MASK;
+        addr &= TARGET_PAGE_MASK;
+
+        switch (flags) {
+        case S390_SKEYS_SAVE_FLAG_SKEYS: {
+            const uint64_t total_count = qemu_get_be64(f);
+            uint64_t handled_count = 0, cur_count;
+            uint64_t cur_gfn = addr / TARGET_PAGE_SIZE;
+            uint8_t *buf = g_try_malloc(S390_SKEYS_BUFFER_SIZE);
+
+            if (!buf) {
+                error_report("storage key load could not allocate memory\n");
+                ret = -ENOMEM;
+                break;
+            }
+
+            while (handled_count < total_count) {
+                cur_count = MIN(total_count - handled_count,
+                                S390_SKEYS_BUFFER_SIZE);
+                qemu_get_buffer(f, buf, cur_count);
+
+                ret = skeyclass->set_skeys(ss, cur_gfn, cur_count, buf);
+                if (ret < 0) {
+                    error_report("S390_SET_KEYS error %d\n", ret);
+                    break;
+                }
+                handled_count += cur_count;
+                cur_gfn += cur_count;
+            }
+            g_free(buf);
+            break;
+        }
+        case S390_SKEYS_SAVE_FLAG_ERROR: {
+            error_report("Storage key data is incomplete");
+            ret = -EINVAL;
+            break;
+        }
+        case S390_SKEYS_SAVE_FLAG_EOS:
+            /* normal exit */
+            return 0;
+        default:
+            error_report("Unexpected storage key flag data: %#x", flags);
+            ret = -EINVAL;
+        }
+    }
+
+    return ret;
+}
+
+static void s390_skeys_instance_init(Object *obj)
+{
+    S390SKeysState *ss = S390_SKEYS(obj);
+
+    register_savevm(NULL, TYPE_S390_SKEYS, 0, 1, s390_storage_keys_save,
+                    s390_storage_keys_load, ss);
+}
+
 static void s390_skeys_class_init(ObjectClass *oc, void *data)
 {
     DeviceClass *dc = DEVICE_CLASS(oc);
@@ -247,6 +371,7 @@ static void s390_skeys_class_init(ObjectClass *oc, void *data)
 static const TypeInfo s390_skeys_info = {
     .name          = TYPE_S390_SKEYS,
     .parent        = TYPE_DEVICE,
+    .instance_init = s390_skeys_instance_init,
     .instance_size = sizeof(S390SKeysState),
     .class_init    = s390_skeys_class_init,
     .class_size    = sizeof(S390SKeysClass),
-- 
2.5.1

[Qemu-devel] [PATCH v3 8/8] s390x: Disable storage key migration on old machine type

[Cornelia Huck]

From: "Jason J. Herne" <jjherne@linux.vnet.ibm.com>

This code disables storage key migration when an older machine type is
specified.

Reviewed-by: David Hildenbrand <dahi@linux.vnet.ibm.com>
Signed-off-by: Jason J. Herne <jjherne@linux.vnet.ibm.com>
Signed-off-by: Cornelia Huck <cornelia.huck@de.ibm.com>
---
 hw/s390x/s390-skeys.c           | 33 ++++++++++++++++++++++++++++++---
 hw/s390x/s390-virtio-ccw.c      | 12 ++++++++++++
 include/hw/s390x/storage-keys.h |  1 +
 3 files changed, 43 insertions(+), 3 deletions(-)

diff --git a/hw/s390x/s390-skeys.c b/hw/s390x/s390-skeys.c
index 72df19d..ba18834 100644
--- a/hw/s390x/s390-skeys.c
+++ b/hw/s390x/s390-skeys.c
@@ -352,12 +352,39 @@ static int s390_storage_keys_load(QEMUFile *f, void *opaque, int version_id)
     return ret;
 }
 
-static void s390_skeys_instance_init(Object *obj)
+static inline bool s390_skeys_get_migration_enabled(Object *obj, Error **errp)
+{
+    S390SKeysState *ss = S390_SKEYS(obj);
+
+    return ss->migration_enabled;
+}
+
+static inline void s390_skeys_set_migration_enabled(Object *obj, bool value,
+                                            Error **errp)
 {
     S390SKeysState *ss = S390_SKEYS(obj);
 
-    register_savevm(NULL, TYPE_S390_SKEYS, 0, 1, s390_storage_keys_save,
-                    s390_storage_keys_load, ss);
+    /* Prevent double registration of savevm handler */
+    if (ss->migration_enabled == value) {
+        return;
+    }
+
+    ss->migration_enabled = value;
+
+    if (ss->migration_enabled) {
+        register_savevm(NULL, TYPE_S390_SKEYS, 0, 1, s390_storage_keys_save,
+                        s390_storage_keys_load, ss);
+    } else {
+        unregister_savevm(DEVICE(ss), TYPE_S390_SKEYS, ss);
+    }
+}
+
+static void s390_skeys_instance_init(Object *obj)
+{
+    object_property_add_bool(obj, "migration-enabled",
+                             s390_skeys_get_migration_enabled,
+                             s390_skeys_set_migration_enabled, NULL);
+    object_property_set_bool(obj, true, "migration-enabled", NULL);
 }
 
 static void s390_skeys_class_init(ObjectClass *oc, void *data)
diff --git a/hw/s390x/s390-virtio-ccw.c b/hw/s390x/s390-virtio-ccw.c
index 0a057ae..e2a26e9 100644
--- a/hw/s390x/s390-virtio-ccw.c
+++ b/hw/s390x/s390-virtio-ccw.c
@@ -282,12 +282,24 @@ static const TypeInfo ccw_machine_info = {
     },
 };
 
+#define CCW_COMPAT_2_4 \
+        {\
+            .driver   = TYPE_S390_SKEYS,\
+            .property = "migration-enabled",\
+            .value    = "off",\
+        },
+
 static void ccw_machine_2_4_class_init(ObjectClass *oc, void *data)
 {
     MachineClass *mc = MACHINE_CLASS(oc);
+    static GlobalProperty compat_props[] = {
+        CCW_COMPAT_2_4
+        { /* end of list */ }
+    };
 
     mc->name = "s390-ccw-virtio-2.4";
     mc->desc = "VirtIO-ccw based S390 machine v2.4";
+    mc->compat_props = compat_props;
 }
 
 static const TypeInfo ccw_machine_2_4_info = {
diff --git a/include/hw/s390x/storage-keys.h b/include/hw/s390x/storage-keys.h
index 18e08d2..72b850c 100644
--- a/include/hw/s390x/storage-keys.h
+++ b/include/hw/s390x/storage-keys.h
@@ -21,6 +21,7 @@
 
 typedef struct S390SKeysState {
     DeviceState parent_obj;
+    bool migration_enabled;
 
 } S390SKeysState;
 
-- 
2.5.1

Re: [Qemu-devel] [PATCH v3 4/8] s390x: Dump storage keys qmp command

[Eric Blake]

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

On 08/31/2015 05:00 AM, Cornelia Huck wrote:
> From: "Jason J. Herne" <jjherne@linux.vnet.ibm.com>
> 
> Provide a dump-skeys qmp command to allow the end user to dump storage
> keys. This is useful for debugging problems with guest storage key support
> within Qemu and for guest operating system developers.
> 
> Reviewed-by: Thomas Huth <thuth@linux.vnet.ibm.com>
> Reviewed-by: David Hildenbrand <dahi@linux.vnet.ibm.com>
> Signed-off-by: Jason J. Herne <jjherne@linux.vnet.ibm.com>
> Signed-off-by: Cornelia Huck <cornelia.huck@de.ibm.com>
> ---
>  hw/s390x/s390-skeys.c | 90 +++++++++++++++++++++++++++++++++++++++++++++++++--
>  monitor.c             |  7 ++++
>  qapi-schema.json      | 14 ++++++++
>  qmp-commands.hx       | 25 ++++++++++++++
>  4 files changed, 134 insertions(+), 2 deletions(-)
> 

> +static void write_keys(QEMUFile *f, uint8_t *keys, uint64_t startgfn,
> +                       uint64_t count, Error **errp)
> +{
> +    uint64_t curpage = startgfn;
> +    uint64_t maxpage = curpage + count - 1;
> +    const char *fmt = "page=%03" PRIx64 ": key(%d) => ACC=%X, FP=%d, REF=%d,"
> +                      " ch=%d, reserved=%d\n";
> +    char buf[128];
> +    int len;
> +
> +    for (; curpage <= maxpage; curpage++) {
> +        uint8_t acc = (*keys & 0xF0) >> 4;
> +        int fp =  (*keys & 0x08);
> +        int ref = (*keys & 0x04);
> +        int ch = (*keys & 0x02);
> +        int res = (*keys & 0x01);
> +
> +        len = snprintf(buf, sizeof(buf), fmt, curpage,
> +                       *keys, acc, fp, ref, ch, res);
> +        qemu_put_buffer(f, (uint8_t *)buf, len);

While I can overlook the use of snprintf(), I still think you ought to
at least have assert(len < sizeof(buf)) here before the
qemu_put_buffer(), to ensure that future changes to fmt don't attempt to
print beyond the end of the fixed-width buffer.

With an assert added,
Reviewed-by: Eric Blake <eblake@redhat.com>

-- 
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 --]

Re: [Qemu-devel] [PATCH v3 4/8] s390x: Dump storage keys qmp command

[Cornelia Huck]

On Mon, 31 Aug 2015 08:48:03 -0600
Eric Blake <eblake@redhat.com> wrote:

> On 08/31/2015 05:00 AM, Cornelia Huck wrote:
> > From: "Jason J. Herne" <jjherne@linux.vnet.ibm.com>
> > 
> > Provide a dump-skeys qmp command to allow the end user to dump storage
> > keys. This is useful for debugging problems with guest storage key support
> > within Qemu and for guest operating system developers.
> > 
> > Reviewed-by: Thomas Huth <thuth@linux.vnet.ibm.com>
> > Reviewed-by: David Hildenbrand <dahi@linux.vnet.ibm.com>
> > Signed-off-by: Jason J. Herne <jjherne@linux.vnet.ibm.com>
> > Signed-off-by: Cornelia Huck <cornelia.huck@de.ibm.com>
> > ---
> >  hw/s390x/s390-skeys.c | 90 +++++++++++++++++++++++++++++++++++++++++++++++++--
> >  monitor.c             |  7 ++++
> >  qapi-schema.json      | 14 ++++++++
> >  qmp-commands.hx       | 25 ++++++++++++++
> >  4 files changed, 134 insertions(+), 2 deletions(-)
> > 
> 
> > +static void write_keys(QEMUFile *f, uint8_t *keys, uint64_t startgfn,
> > +                       uint64_t count, Error **errp)
> > +{
> > +    uint64_t curpage = startgfn;
> > +    uint64_t maxpage = curpage + count - 1;
> > +    const char *fmt = "page=%03" PRIx64 ": key(%d) => ACC=%X, FP=%d, REF=%d,"
> > +                      " ch=%d, reserved=%d\n";
> > +    char buf[128];
> > +    int len;
> > +
> > +    for (; curpage <= maxpage; curpage++) {
> > +        uint8_t acc = (*keys & 0xF0) >> 4;
> > +        int fp =  (*keys & 0x08);
> > +        int ref = (*keys & 0x04);
> > +        int ch = (*keys & 0x02);
> > +        int res = (*keys & 0x01);
> > +
> > +        len = snprintf(buf, sizeof(buf), fmt, curpage,
> > +                       *keys, acc, fp, ref, ch, res);
> > +        qemu_put_buffer(f, (uint8_t *)buf, len);
> 
> While I can overlook the use of snprintf(), I still think you ought to
> at least have assert(len < sizeof(buf)) here before the
> qemu_put_buffer(), to ensure that future changes to fmt don't attempt to
> print beyond the end of the fixed-width buffer.

I'll just merge that in when sending the pull request.

> 
> With an assert added,
> Reviewed-by: Eric Blake <eblake@redhat.com>
> 
Thanks!

Re: [Qemu-devel] [PATCH v3 5/8] s390x: Dump-skeys hmp support

[Eric Blake]

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

On 08/31/2015 05:00 AM, Cornelia Huck wrote:
> From: "Jason J. Herne" <jjherne@linux.vnet.ibm.com>
> 
> Add dump-skeys command to the human monitor.
> 
> Reviewed-by: Thomas Huth <thuth@linux.vnet.ibm.com>
> Reviewed-by: David Hildenbrand <dahi@linux.vnet.ibm.com>
> Signed-off-by: Jason J. Herne <jjherne@linux.vnet.ibm.com>
> Signed-off-by: Cornelia Huck <cornelia.huck@de.ibm.com>
> ---
>  hmp-commands.hx                 | 16 ++++++++++++++++
>  hw/s390x/s390-skeys.c           | 12 ++++++++++++
>  include/hw/s390x/storage-keys.h |  2 ++
>  monitor.c                       |  4 ++++
>  4 files changed, 34 insertions(+)
> 
> diff --git a/hmp-commands.hx b/hmp-commands.hx
> index d3b7932..803ff91 100644
> --- a/hmp-commands.hx
> +++ b/hmp-commands.hx
> @@ -1053,6 +1053,22 @@ gdb. Without -z|-l|-s, the dump format is ELF.
>              together with begin.
>  ETEXI
>  
> +#if defined(TARGET_S390X)
> +    {
> +        .name       = "dump-skeys",

Most HMP commands use '_', not '-', for word separation.

-- 
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 --]

Re: [Qemu-devel] [PATCH v3 5/8] s390x: Dump-skeys hmp support

[Jason J. Herne]

On 08/31/2015 12:30 PM, Eric Blake wrote:
> On 08/31/2015 05:00 AM, Cornelia Huck wrote:
>> From: "Jason J. Herne" <jjherne@linux.vnet.ibm.com>
>>
>> Add dump-skeys command to the human monitor.
>>
>> Reviewed-by: Thomas Huth <thuth@linux.vnet.ibm.com>
>> Reviewed-by: David Hildenbrand <dahi@linux.vnet.ibm.com>
>> Signed-off-by: Jason J. Herne <jjherne@linux.vnet.ibm.com>
>> Signed-off-by: Cornelia Huck <cornelia.huck@de.ibm.com>
>> ---
>>   hmp-commands.hx                 | 16 ++++++++++++++++
>>   hw/s390x/s390-skeys.c           | 12 ++++++++++++
>>   include/hw/s390x/storage-keys.h |  2 ++
>>   monitor.c                       |  4 ++++
>>   4 files changed, 34 insertions(+)
>>
>> diff --git a/hmp-commands.hx b/hmp-commands.hx
>> index d3b7932..803ff91 100644
>> --- a/hmp-commands.hx
>> +++ b/hmp-commands.hx
>> @@ -1053,6 +1053,22 @@ gdb. Without -z|-l|-s, the dump format is ELF.
>>               together with begin.
>>   ETEXI
>>
>> +#if defined(TARGET_S390X)
>> +    {
>> +        .name       = "dump-skeys",
>
> Most HMP commands use '_', not '-', for word separation.
>

I patterned my new command after dump-guest-memory since the functionality
was similar. Though it is easy enough to change if you would like.

-- 
-- Jason J. Herne (jjherne@linux.vnet.ibm.com)

Re: [Qemu-devel] [PATCH v3 5/8] s390x: Dump-skeys hmp support

[Cornelia Huck]

On Mon, 31 Aug 2015 14:57:51 -0400
"Jason J. Herne" <jjherne@linux.vnet.ibm.com> wrote:

> On 08/31/2015 12:30 PM, Eric Blake wrote:
> > On 08/31/2015 05:00 AM, Cornelia Huck wrote:
> >> From: "Jason J. Herne" <jjherne@linux.vnet.ibm.com>
> >>
> >> Add dump-skeys command to the human monitor.
> >>
> >> Reviewed-by: Thomas Huth <thuth@linux.vnet.ibm.com>
> >> Reviewed-by: David Hildenbrand <dahi@linux.vnet.ibm.com>
> >> Signed-off-by: Jason J. Herne <jjherne@linux.vnet.ibm.com>
> >> Signed-off-by: Cornelia Huck <cornelia.huck@de.ibm.com>
> >> ---
> >>   hmp-commands.hx                 | 16 ++++++++++++++++
> >>   hw/s390x/s390-skeys.c           | 12 ++++++++++++
> >>   include/hw/s390x/storage-keys.h |  2 ++
> >>   monitor.c                       |  4 ++++
> >>   4 files changed, 34 insertions(+)
> >>
> >> diff --git a/hmp-commands.hx b/hmp-commands.hx
> >> index d3b7932..803ff91 100644
> >> --- a/hmp-commands.hx
> >> +++ b/hmp-commands.hx
> >> @@ -1053,6 +1053,22 @@ gdb. Without -z|-l|-s, the dump format is ELF.
> >>               together with begin.
> >>   ETEXI
> >>
> >> +#if defined(TARGET_S390X)
> >> +    {
> >> +        .name       = "dump-skeys",
> >
> > Most HMP commands use '_', not '-', for word separation.
> >
> 
> I patterned my new command after dump-guest-memory since the functionality
> was similar. Though it is easy enough to change if you would like.

Eric, do you have a strong preference? I think either is fine; I can
either keep it as-is or merge in a change for the pull.

Re: [Qemu-devel] [PATCH v3 5/8] s390x: Dump-skeys hmp support

[Eric Blake]

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

On 09/01/2015 08:30 AM, Cornelia Huck wrote:

>>>> +++ b/hmp-commands.hx
>>>> @@ -1053,6 +1053,22 @@ gdb. Without -z|-l|-s, the dump format is ELF.
>>>>               together with begin.
>>>>   ETEXI
>>>>
>>>> +#if defined(TARGET_S390X)
>>>> +    {
>>>> +        .name       = "dump-skeys",
>>>
>>> Most HMP commands use '_', not '-', for word separation.
>>>
>>
>> I patterned my new command after dump-guest-memory since the functionality
>> was similar. Though it is easy enough to change if you would like.
> 
> Eric, do you have a strong preference? I think either is fine; I can
> either keep it as-is or merge in a change for the pull.

HMP is not ABI; we can change it at will without worrying about
back-compat issues.  Consistency is nice, so I'd lean towards
consistently using _ throughout HMP, but the preference is not strong
enough so I don't object to keeping this commit as-is for the sake of
merging, especially if dump-guest-memory also needs changing (that is, a
followup patch that changes all HMP to be consistent in one go is not
that much harder, even if this patch adds to the workload of that patch).

-- 
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 --]

Re: [Qemu-devel] [PATCH v3 5/8] s390x: Dump-skeys hmp support

[Cornelia Huck]

On Tue, 1 Sep 2015 10:05:17 -0600
Eric Blake <eblake@redhat.com> wrote:

> On 09/01/2015 08:30 AM, Cornelia Huck wrote:
> 
> >>>> +++ b/hmp-commands.hx
> >>>> @@ -1053,6 +1053,22 @@ gdb. Without -z|-l|-s, the dump format is ELF.
> >>>>               together with begin.
> >>>>   ETEXI
> >>>>
> >>>> +#if defined(TARGET_S390X)
> >>>> +    {
> >>>> +        .name       = "dump-skeys",
> >>>
> >>> Most HMP commands use '_', not '-', for word separation.
> >>>
> >>
> >> I patterned my new command after dump-guest-memory since the functionality
> >> was similar. Though it is easy enough to change if you would like.
> > 
> > Eric, do you have a strong preference? I think either is fine; I can
> > either keep it as-is or merge in a change for the pull.
> 
> HMP is not ABI; we can change it at will without worrying about
> back-compat issues.  Consistency is nice, so I'd lean towards
> consistently using _ throughout HMP, but the preference is not strong
> enough so I don't object to keeping this commit as-is for the sake of
> merging, especially if dump-guest-memory also needs changing (that is, a
> followup patch that changes all HMP to be consistent in one go is not
> that much harder, even if this patch adds to the workload of that patch).

Given that there are even more commands using - and that we can change
this later on, I'll just leave it as-is.