qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
* [Qemu-devel] [PATCH v5 0/2] Guest Support for DIAGNOSE 0x318
@ 2019-06-25 15:17 Collin Walling
  2019-06-25 15:17 ` [Qemu-devel] [PATCH v5 1/2] s390/kvm: header sync for diag318 Collin Walling
                   ` (2 more replies)
  0 siblings, 3 replies; 13+ messages in thread
From: Collin Walling @ 2019-06-25 15:17 UTC (permalink / raw)
  To: qemu-devel, qemu-s390x, cohuck, rth, david, pasic, borntraeger,
	mst, pbonzini

Changelog:

    v5
        - split off header updates in kvm_s390x.h to a separate patch
        - implemented CPU model feature for this insn made available with
            zEC12-full and later models
        - s/cpc/diag318_info in order to make the relevant data more clear
        - reduced S390_MAX_CPUS from 248 to 247

The DIAGNOSE 0x318 instruction is a privileged instruction that is executed by the
linux kernel once and only once during setup. This requires interception by KVM to 
handle the instruction call safely. The instruction assists with determining the 
environment the VM is running in -- this is better described in the KVM patches.

The analogous KVM patches still under review can be found here:
https://marc.info/?l=linux-s390&m=156147521528818&w=2

Guest support for the diag 318 instruction is accomplished by implementing a device 
class, a cpu model feature, and adjusting the Read Info struct. The Read Info struct
adjustment coincidentally reduces the maximum number of VCPUs we can have by one.

A device class is used for this instruction in order to streamline the migration and 
reset of the DIAG 318 related data.

A CPU model feature is added for this instruction, appropriately named diag318.

The instruction is determined by a Read Info byte 134 bit 0. This is a new byte that
expands into the space of the Read Info SCCB that is also used to contain CPU entry
data. Due to this expansion, we lose space for one CPU entry and we must reduce the
maximum possible CPUs from 248 to 247. Hopefully this drawback does not affect many 
VMs.

Collin Walling (2):
  s390/kvm: header sync for diag318
  s390: diagnose 318 info reset and migration support

 hw/s390x/Makefile.objs          |  1 +
 hw/s390x/diag318.c              | 80 +++++++++++++++++++++++++++++++++++++++++
 hw/s390x/diag318.h              | 38 ++++++++++++++++++++
 hw/s390x/s390-virtio-ccw.c      | 17 +++++++++
 hw/s390x/sclp.c                 |  3 ++
 include/hw/s390x/sclp.h         |  2 ++
 linux-headers/asm-s390/kvm.h    |  4 +++
 target/s390x/cpu.h              |  8 ++++-
 target/s390x/cpu_features.c     |  3 ++
 target/s390x/cpu_features.h     |  1 +
 target/s390x/cpu_features_def.h |  3 ++
 target/s390x/gen-features.c     |  1 +
 target/s390x/kvm-stub.c         | 10 ++++++
 target/s390x/kvm.c              | 29 +++++++++++++++
 target/s390x/kvm_s390x.h        |  2 ++
 15 files changed, 201 insertions(+), 1 deletion(-)
 create mode 100644 hw/s390x/diag318.c
 create mode 100644 hw/s390x/diag318.h

-- 
2.7.4



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

* [Qemu-devel] [PATCH v5 1/2] s390/kvm: header sync for diag318
  2019-06-25 15:17 [Qemu-devel] [PATCH v5 0/2] Guest Support for DIAGNOSE 0x318 Collin Walling
@ 2019-06-25 15:17 ` Collin Walling
  2019-06-26  9:43   ` David Hildenbrand
  2019-06-25 15:17 ` [Qemu-devel] [PATCH v5 2/2] s390: diagnose 318 info reset and migration support Collin Walling
  2019-06-25 16:34 ` [Qemu-devel] [PATCH v5 0/2] Guest Support for DIAGNOSE 0x318 no-reply
  2 siblings, 1 reply; 13+ messages in thread
From: Collin Walling @ 2019-06-25 15:17 UTC (permalink / raw)
  To: qemu-devel, qemu-s390x, cohuck, rth, david, pasic, borntraeger,
	mst, pbonzini

Signed-off-by: Collin Walling <walling@linux.ibm.com>
---
 linux-headers/asm-s390/kvm.h | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/linux-headers/asm-s390/kvm.h b/linux-headers/asm-s390/kvm.h
index 03ab596..4a857bb 100644
--- a/linux-headers/asm-s390/kvm.h
+++ b/linux-headers/asm-s390/kvm.h
@@ -74,6 +74,7 @@ struct kvm_s390_io_adapter_req {
 #define KVM_S390_VM_CRYPTO		2
 #define KVM_S390_VM_CPU_MODEL		3
 #define KVM_S390_VM_MIGRATION		4
+#define KVM_S390_VM_MISC		5
 
 /* kvm attributes for mem_ctrl */
 #define KVM_S390_VM_MEM_ENABLE_CMMA	0
@@ -171,6 +172,9 @@ struct kvm_s390_vm_cpu_subfunc {
 #define KVM_S390_VM_MIGRATION_START	1
 #define KVM_S390_VM_MIGRATION_STATUS	2
 
+/* kvm attributes for KVM_S390_VM_MISC */
+#define KVM_S390_VM_MISC_DIAG318	0
+
 /* for KVM_GET_REGS and KVM_SET_REGS */
 struct kvm_regs {
 	/* general purpose regs for s390 */
-- 
2.7.4



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

* [Qemu-devel] [PATCH v5 2/2] s390: diagnose 318 info reset and migration support
  2019-06-25 15:17 [Qemu-devel] [PATCH v5 0/2] Guest Support for DIAGNOSE 0x318 Collin Walling
  2019-06-25 15:17 ` [Qemu-devel] [PATCH v5 1/2] s390/kvm: header sync for diag318 Collin Walling
@ 2019-06-25 15:17 ` Collin Walling
  2019-06-26  9:12   ` Christian Borntraeger
  2019-06-26 12:33   ` [Qemu-devel] " Cornelia Huck
  2019-06-25 16:34 ` [Qemu-devel] [PATCH v5 0/2] Guest Support for DIAGNOSE 0x318 no-reply
  2 siblings, 2 replies; 13+ messages in thread
From: Collin Walling @ 2019-06-25 15:17 UTC (permalink / raw)
  To: qemu-devel, qemu-s390x, cohuck, rth, david, pasic, borntraeger,
	mst, pbonzini

DIAGNOSE 0x318 (diag318) is a privileged s390x instruction that must
be intercepted by SIE and handled via KVM. Let's introduce some
functions to communicate between QEMU and KVM via ioctls. These
will be used to get/set the diag318 information.

The availability of this instruction is determined by byte 134, bit 0
of the Read Info block. This coincidentally expands into the space used
for CPU entries, which means VMs running with the diag318 capability
will have a reduced maximum CPU count. Let's reduce the maximum CPU
count from 248 to 247.

In order to simplify the migration and system reset requirements of
the diag318 data, let's introduce it as a device class and include
a VMStateDescription.

Diag318 is set to 0 during modified clear and load normal resets.

Signed-off-by: Collin Walling <walling@linux.ibm.com>
---
 hw/s390x/Makefile.objs          |  1 +
 hw/s390x/diag318.c              | 80 +++++++++++++++++++++++++++++++++++++++++
 hw/s390x/diag318.h              | 38 ++++++++++++++++++++
 hw/s390x/s390-virtio-ccw.c      | 17 +++++++++
 hw/s390x/sclp.c                 |  3 ++
 include/hw/s390x/sclp.h         |  2 ++
 target/s390x/cpu.h              |  8 ++++-
 target/s390x/cpu_features.c     |  3 ++
 target/s390x/cpu_features.h     |  1 +
 target/s390x/cpu_features_def.h |  3 ++
 target/s390x/gen-features.c     |  1 +
 target/s390x/kvm-stub.c         | 10 ++++++
 target/s390x/kvm.c              | 29 +++++++++++++++
 target/s390x/kvm_s390x.h        |  2 ++
 14 files changed, 197 insertions(+), 1 deletion(-)
 create mode 100644 hw/s390x/diag318.c
 create mode 100644 hw/s390x/diag318.h

diff --git a/hw/s390x/Makefile.objs b/hw/s390x/Makefile.objs
index e02ed80..93621dc 100644
--- a/hw/s390x/Makefile.objs
+++ b/hw/s390x/Makefile.objs
@@ -34,3 +34,4 @@ obj-$(CONFIG_KVM) += s390-stattrib-kvm.o
 obj-y += s390-ccw.o
 obj-y += ap-device.o
 obj-y += ap-bridge.o
+obj-y += diag318.o
diff --git a/hw/s390x/diag318.c b/hw/s390x/diag318.c
new file mode 100644
index 0000000..0eb80fe
--- /dev/null
+++ b/hw/s390x/diag318.c
@@ -0,0 +1,80 @@
+/*
+ * DIAGNOSE 0x318 functions for reset and migration
+ *
+ * Copyright IBM, Corp. 2019
+ *
+ * Authors:
+ *  Collin Walling <walling@linux.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/diag318.h"
+#include "qapi/error.h"
+#include "kvm_s390x.h"
+#include "sysemu/kvm.h"
+
+static int diag318_post_load(void *opaque, int version_id)
+{
+    DIAG318State *d = opaque;
+
+    kvm_s390_set_diag318_info(d->info);
+    return 0;
+}
+
+static int diag318_pre_save(void *opaque)
+{
+    DIAG318State *d = opaque;
+
+    kvm_s390_get_diag318_info(&d->info);
+    return 0;
+}
+
+static bool diag318_needed(void *opaque)
+{
+    return s390_has_feat(S390_FEAT_DIAG318);
+}
+
+const VMStateDescription vmstate_diag318 = {
+    .name = "vmstate_diag318",
+    .post_load = diag318_post_load,
+    .pre_save = diag318_pre_save,
+    .version_id = 1,
+    .minimum_version_id = 1,
+    .needed = diag318_needed,
+    .fields = (VMStateField[]) {
+        VMSTATE_UINT64(info, DIAG318State),
+        VMSTATE_END_OF_LIST()
+    }
+};
+
+static void s390_diag318_reset(DeviceState *dev)
+{
+    kvm_s390_set_diag318_info(0);
+}
+
+static void s390_diag318_class_init(ObjectClass *klass, void *data)
+{
+    DeviceClass *dc = DEVICE_CLASS(klass);
+
+    dc->reset = s390_diag318_reset;
+    dc->vmsd = &vmstate_diag318;
+    dc->hotpluggable = false;
+    /* Reason: Set automatically during IPL */
+    dc->user_creatable = false;
+}
+
+static const TypeInfo s390_diag318_info = {
+    .class_init = s390_diag318_class_init,
+    .parent = TYPE_DEVICE,
+    .name = TYPE_S390_DIAG318,
+    .instance_size = sizeof(DIAG318State),
+};
+
+static void s390_diag318_register_types(void)
+{
+    type_register_static(&s390_diag318_info);
+}
+
+type_init(s390_diag318_register_types)
diff --git a/hw/s390x/diag318.h b/hw/s390x/diag318.h
new file mode 100644
index 0000000..d588bdd
--- /dev/null
+++ b/hw/s390x/diag318.h
@@ -0,0 +1,38 @@
+/*
+ * DIAGNOSE 0x318 functions for reset and migration
+ *
+ * Copyright IBM, Corp. 2019
+ *
+ * Authors:
+ *  Collin Walling <walling@linux.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 HW_DIAG318_H
+#define HW_DIAG318_H
+
+#include "qemu/osdep.h"
+#include "hw/qdev.h"
+
+#define TYPE_S390_DIAG318 "diag318"
+#define DIAG318(obj) \
+    OBJECT_CHECK(DIAG318State, (obj), TYPE_S390_DIAG318)
+
+typedef struct DIAG318State {
+    /*< private >*/
+    DeviceState parent_obj;
+
+    /*< public >*/
+    uint64_t info;
+} DIAG318State;
+
+typedef struct DIAG318Class {
+    /*< private >*/
+    DeviceClass parent_class;
+
+    /*< public >*/
+} DIAG318Class;
+
+#endif /* HW_DIAG318_H */
\ No newline at end of file
diff --git a/hw/s390x/s390-virtio-ccw.c b/hw/s390x/s390-virtio-ccw.c
index 87b2039..54230c7 100644
--- a/hw/s390x/s390-virtio-ccw.c
+++ b/hw/s390x/s390-virtio-ccw.c
@@ -38,6 +38,7 @@
 #include "cpu_models.h"
 #include "hw/nmi.h"
 #include "hw/s390x/tod.h"
+#include "hw/s390x/diag318.h"
 
 S390CPU *s390_cpu_addr2state(uint16_t cpu_addr)
 {
@@ -94,6 +95,7 @@ static const char *const reset_dev_types[] = {
     "s390-sclp-event-facility",
     "s390-flic",
     "diag288",
+    TYPE_S390_DIAG318,
 };
 
 static void subsystem_reset(void)
@@ -258,6 +260,17 @@ static void s390_create_sclpconsole(const char *type, Chardev *chardev)
     qdev_init_nofail(dev);
 }
 
+static void s390_init_diag318(void)
+{
+    Object *new = object_new(TYPE_S390_DIAG318);
+    DeviceState *dev = DEVICE(new);
+
+    object_property_add_child(qdev_get_machine(), TYPE_S390_DIAG318,
+                              new, NULL);
+    object_unref(new);
+    qdev_init_nofail(dev);
+}
+
 static void ccw_init(MachineState *machine)
 {
     int ret;
@@ -315,6 +328,9 @@ static void ccw_init(MachineState *machine)
 
     /* init the TOD clock */
     s390_init_tod();
+
+    /* init object used for migrating diag318 info */
+    s390_init_diag318();
 }
 
 static void s390_cpu_plug(HotplugHandler *hotplug_dev,
@@ -583,6 +599,7 @@ static void machine_set_loadparm(Object *obj, const char *val, Error **errp)
         ms->loadparm[i] = ' '; /* pad right with spaces */
     }
 }
+
 static inline void s390_machine_initfn(Object *obj)
 {
     object_property_add_bool(obj, "aes-key-wrap",
diff --git a/hw/s390x/sclp.c b/hw/s390x/sclp.c
index 4510a80..f905af0 100644
--- a/hw/s390x/sclp.c
+++ b/hw/s390x/sclp.c
@@ -22,6 +22,7 @@
 #include "hw/s390x/event-facility.h"
 #include "hw/s390x/s390-pci-bus.h"
 #include "hw/s390x/ipl.h"
+#include "kvm_s390x.h"
 
 static inline SCLPDevice *get_sclp_device(void)
 {
@@ -74,6 +75,8 @@ static void read_SCP_info(SCLPDevice *sclp, SCCB *sccb)
     s390_get_feat_block(S390_FEAT_TYPE_SCLP_CONF_CHAR_EXT,
                          read_info->conf_char_ext);
 
+    s390_get_feat_block(S390_FEAT_TYPE_SCLP_BYTE_134, read_info->fac134);
+
     read_info->facilities = cpu_to_be64(SCLP_HAS_CPU_INFO |
                                         SCLP_HAS_IOA_RECONFIG);
 
diff --git a/include/hw/s390x/sclp.h b/include/hw/s390x/sclp.h
index f9db243..667da49 100644
--- a/include/hw/s390x/sclp.h
+++ b/include/hw/s390x/sclp.h
@@ -133,6 +133,8 @@ typedef struct ReadInfo {
     uint16_t highest_cpu;
     uint8_t  _reserved5[124 - 122];     /* 122-123 */
     uint32_t hmfai;
+    uint8_t  _reserved7[134 - 128];     /* 128-133 */
+    uint8_t  fac134[1];
     struct CPUEntry entries[0];
 } QEMU_PACKED ReadInfo;
 
diff --git a/target/s390x/cpu.h b/target/s390x/cpu.h
index a606547..4c26754 100644
--- a/target/s390x/cpu.h
+++ b/target/s390x/cpu.h
@@ -39,7 +39,13 @@
 
 #define MMU_USER_IDX 0
 
-#define S390_MAX_CPUS 248
+/*
+ * HACK: The introduction of additional facility bytes in the Read Info
+ * struct consumes space used for CPU entries, thus we must reduce the
+ * original maximum CPUs of 248 by one for each new byte or risk smashing
+ * the stack.
+ */
+#define S390_MAX_CPUS 247
 
 typedef struct PSW {
     uint64_t mask;
diff --git a/target/s390x/cpu_features.c b/target/s390x/cpu_features.c
index f64f581..77a1df5 100644
--- a/target/s390x/cpu_features.c
+++ b/target/s390x/cpu_features.c
@@ -127,6 +127,9 @@ static const S390FeatDef s390_features[] = {
     FEAT_INIT("pfmfi", S390_FEAT_TYPE_SCLP_CONF_CHAR_EXT, 9, "SIE: PFMF interpretation facility"),
     FEAT_INIT("ibs", S390_FEAT_TYPE_SCLP_CONF_CHAR_EXT, 10, "SIE: Interlock-and-broadcast-suppression facility"),
 
+    /* SCLP SCCB Byte 134 */
+    FEAT_INIT("diag318", S390_FEAT_TYPE_SCLP_BYTE_134, 0, "Control program name and version codes"),
+
     FEAT_INIT("sief2", S390_FEAT_TYPE_SCLP_CPU, 4, "SIE: interception format 2 (Virtual SIE)"),
     FEAT_INIT("skey", S390_FEAT_TYPE_SCLP_CPU, 5, "SIE: Storage-key facility"),
     FEAT_INIT("gpereh", S390_FEAT_TYPE_SCLP_CPU, 10, "SIE: Guest-PER enhancement facility"),
diff --git a/target/s390x/cpu_features.h b/target/s390x/cpu_features.h
index da695a8..954544e 100644
--- a/target/s390x/cpu_features.h
+++ b/target/s390x/cpu_features.h
@@ -23,6 +23,7 @@ typedef enum {
     S390_FEAT_TYPE_STFL,
     S390_FEAT_TYPE_SCLP_CONF_CHAR,
     S390_FEAT_TYPE_SCLP_CONF_CHAR_EXT,
+    S390_FEAT_TYPE_SCLP_BYTE_134,
     S390_FEAT_TYPE_SCLP_CPU,
     S390_FEAT_TYPE_MISC,
     S390_FEAT_TYPE_PLO,
diff --git a/target/s390x/cpu_features_def.h b/target/s390x/cpu_features_def.h
index 292b17b..4f2c23e 100644
--- a/target/s390x/cpu_features_def.h
+++ b/target/s390x/cpu_features_def.h
@@ -115,6 +115,9 @@ typedef enum {
     S390_FEAT_SIE_PFMFI,
     S390_FEAT_SIE_IBS,
 
+    /* Sclp Byte 134 */
+    S390_FEAT_DIAG318,
+
     /* Sclp Cpu */
     S390_FEAT_SIE_F2,
     S390_FEAT_SIE_SKEY,
diff --git a/target/s390x/gen-features.c b/target/s390x/gen-features.c
index dc320a0..cdd1875 100644
--- a/target/s390x/gen-features.c
+++ b/target/s390x/gen-features.c
@@ -521,6 +521,7 @@ static uint16_t full_GEN12_GA1[] = {
     S390_FEAT_AP_QUERY_CONFIG_INFO,
     S390_FEAT_AP_FACILITIES_TEST,
     S390_FEAT_AP,
+    S390_FEAT_DIAG318,
 };
 
 static uint16_t full_GEN12_GA2[] = {
diff --git a/target/s390x/kvm-stub.c b/target/s390x/kvm-stub.c
index 5152e2b..7c39d6a 100644
--- a/target/s390x/kvm-stub.c
+++ b/target/s390x/kvm-stub.c
@@ -107,3 +107,13 @@ void kvm_s390_stop_interrupt(S390CPU *cpu)
 void kvm_s390_restart_interrupt(S390CPU *cpu)
 {
 }
+
+int kvm_s390_get_diag318_info(uint64_t *info)
+{
+    return 0;
+}
+
+int kvm_s390_set_diag318_info(uint64_t info)
+{
+    return 0;
+}
diff --git a/target/s390x/kvm.c b/target/s390x/kvm.c
index 0267c6c..1fda9cd 100644
--- a/target/s390x/kvm.c
+++ b/target/s390x/kvm.c
@@ -750,6 +750,28 @@ int kvm_s390_set_clock_ext(uint8_t tod_high, uint64_t tod_low)
     return kvm_vm_ioctl(kvm_state, KVM_SET_DEVICE_ATTR, &attr);
 }
 
+int kvm_s390_get_diag318_info(uint64_t *info)
+{
+    struct kvm_device_attr attr = {
+        .group = KVM_S390_VM_MISC,
+        .attr = KVM_S390_VM_MISC_DIAG318,
+        .addr = (uint64_t)info,
+    };
+
+    return kvm_vm_ioctl(kvm_state, KVM_GET_DEVICE_ATTR, &attr);
+}
+
+int kvm_s390_set_diag318_info(uint64_t info)
+{
+    struct kvm_device_attr attr = {
+        .group = KVM_S390_VM_MISC,
+        .attr = KVM_S390_VM_MISC_DIAG318,
+        .addr = (uint64_t)&info,
+    };
+
+    return kvm_vm_ioctl(kvm_state, KVM_SET_DEVICE_ATTR, &attr);
+}
+
 /**
  * kvm_s390_mem_op:
  * @addr:      the logical start address in guest memory
@@ -2323,6 +2345,13 @@ void kvm_s390_get_host_cpu_model(S390CPUModel *model, Error **errp)
         KVM_S390_VM_CRYPTO_ENABLE_APIE)) {
         set_bit(S390_FEAT_AP, model->features);
     }
+
+    /* if KVM supports interception of diag318, then let's provide the bit */
+    if (kvm_vm_check_attr(kvm_state, KVM_S390_VM_MISC,
+        KVM_S390_VM_MISC_DIAG318)) {
+        set_bit(S390_FEAT_DIAG318, model->features);
+    }
+
     /* strip of features that are not part of the maximum model */
     bitmap_and(model->features, model->features, model->def->full_feat,
                S390_FEAT_MAX);
diff --git a/target/s390x/kvm_s390x.h b/target/s390x/kvm_s390x.h
index caf9859..50df93e 100644
--- a/target/s390x/kvm_s390x.h
+++ b/target/s390x/kvm_s390x.h
@@ -29,6 +29,8 @@ int kvm_s390_get_clock(uint8_t *tod_high, uint64_t *tod_clock);
 int kvm_s390_get_clock_ext(uint8_t *tod_high, uint64_t *tod_clock);
 int kvm_s390_set_clock(uint8_t tod_high, uint64_t tod_clock);
 int kvm_s390_set_clock_ext(uint8_t tod_high, uint64_t tod_clock);
+int kvm_s390_get_diag318_info(uint64_t *info);
+int kvm_s390_set_diag318_info(uint64_t info);
 void kvm_s390_enable_css_support(S390CPU *cpu);
 int kvm_s390_assign_subch_ioeventfd(EventNotifier *notifier, uint32_t sch,
                                     int vq, bool assign);
-- 
2.7.4



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

* Re: [Qemu-devel] [PATCH v5 0/2] Guest Support for DIAGNOSE 0x318
  2019-06-25 15:17 [Qemu-devel] [PATCH v5 0/2] Guest Support for DIAGNOSE 0x318 Collin Walling
  2019-06-25 15:17 ` [Qemu-devel] [PATCH v5 1/2] s390/kvm: header sync for diag318 Collin Walling
  2019-06-25 15:17 ` [Qemu-devel] [PATCH v5 2/2] s390: diagnose 318 info reset and migration support Collin Walling
@ 2019-06-25 16:34 ` no-reply
  2 siblings, 0 replies; 13+ messages in thread
From: no-reply @ 2019-06-25 16:34 UTC (permalink / raw)
  To: walling
  Cc: david, cohuck, qemu-devel, pasic, borntraeger, qemu-s390x, mst,
	pbonzini, rth

Patchew URL: https://patchew.org/QEMU/1561475829-19202-1-git-send-email-walling@linux.ibm.com/



Hi,

This series seems to have some coding style problems. See output below for
more information:

Subject: [Qemu-devel] [PATCH v5 0/2] Guest Support for DIAGNOSE 0x318
Type: series
Message-id: 1561475829-19202-1-git-send-email-walling@linux.ibm.com

=== TEST SCRIPT BEGIN ===
#!/bin/bash
git rev-parse base > /dev/null || exit 0
git config --local diff.renamelimit 0
git config --local diff.renames True
git config --local diff.algorithm histogram
./scripts/checkpatch.pl --mailback base..
=== TEST SCRIPT END ===

From https://github.com/patchew-project/qemu
 * [new tag]         patchew/1561475829-19202-1-git-send-email-walling@linux.ibm.com -> patchew/1561475829-19202-1-git-send-email-walling@linux.ibm.com
Switched to a new branch 'test'
2cd6021 s390: diagnose 318 info reset and migration support
f01785b s390/kvm: header sync for diag318

=== OUTPUT BEGIN ===
1/2 Checking commit f01785b90310 (s390/kvm: header sync for diag318)
2/2 Checking commit 2cd6021b6d2a (s390: diagnose 318 info reset and migration support)
WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
#37: 
new file mode 100644

ERROR: adding a line without newline at end of file
#165: FILE: hw/s390x/diag318.h:38:
+#endif /* HW_DIAG318_H */

ERROR: line over 90 characters
#285: FILE: target/s390x/cpu_features.c:131:
+    FEAT_INIT("diag318", S390_FEAT_TYPE_SCLP_BYTE_134, 0, "Control program name and version codes"),

total: 2 errors, 1 warnings, 300 lines checked

Patch 2/2 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.

=== OUTPUT END ===

Test command exited with code: 1


The full log is available at
http://patchew.org/logs/1561475829-19202-1-git-send-email-walling@linux.ibm.com/testing.checkpatch/?type=message.
---
Email generated automatically by Patchew [https://patchew.org/].
Please send your feedback to patchew-devel@redhat.com

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

* Re: [Qemu-devel] [PATCH v5 2/2] s390: diagnose 318 info reset and migration support
  2019-06-25 15:17 ` [Qemu-devel] [PATCH v5 2/2] s390: diagnose 318 info reset and migration support Collin Walling
@ 2019-06-26  9:12   ` Christian Borntraeger
  2019-06-26 12:14     ` Cornelia Huck
  2019-06-26 12:33   ` [Qemu-devel] " Cornelia Huck
  1 sibling, 1 reply; 13+ messages in thread
From: Christian Borntraeger @ 2019-06-26  9:12 UTC (permalink / raw)
  To: Collin Walling, qemu-devel, qemu-s390x, cohuck, rth, david,
	pasic, mst, pbonzini



On 25.06.19 17:17, Collin Walling wrote:
> index a606547..4c26754 100644
> --- a/target/s390x/cpu.h
> +++ b/target/s390x/cpu.h
> @@ -39,7 +39,13 @@
>  
>  #define MMU_USER_IDX 0
>  
> -#define S390_MAX_CPUS 248
> +/*
> + * HACK: The introduction of additional facility bytes in the Read Info
> + * struct consumes space used for CPU entries, thus we must reduce the
> + * original maximum CPUs of 248 by one for each new byte or risk smashing
> + * the stack.
> + */
> +#define S390_MAX_CPUS 247

I think we decided to not change that. Only if the cpu model contains the diag318
feature we are limited to 247 but only for the sclp response.
So we said: 
- we continue to allow 248 cpus
- the sclp response will be limited to 247 CPUs if the feature is one
- (optional) we print a warning that the guest might not see all CPUs



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

* Re: [Qemu-devel] [PATCH v5 1/2] s390/kvm: header sync for diag318
  2019-06-25 15:17 ` [Qemu-devel] [PATCH v5 1/2] s390/kvm: header sync for diag318 Collin Walling
@ 2019-06-26  9:43   ` David Hildenbrand
  0 siblings, 0 replies; 13+ messages in thread
From: David Hildenbrand @ 2019-06-26  9:43 UTC (permalink / raw)
  To: Collin Walling, qemu-devel, qemu-s390x, cohuck, rth, pasic,
	borntraeger, mst, pbonzini

On 25.06.19 17:17, Collin Walling wrote:
> Signed-off-by: Collin Walling <walling@linux.ibm.com>
> ---
>  linux-headers/asm-s390/kvm.h | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/linux-headers/asm-s390/kvm.h b/linux-headers/asm-s390/kvm.h
> index 03ab596..4a857bb 100644
> --- a/linux-headers/asm-s390/kvm.h
> +++ b/linux-headers/asm-s390/kvm.h
> @@ -74,6 +74,7 @@ struct kvm_s390_io_adapter_req {
>  #define KVM_S390_VM_CRYPTO		2
>  #define KVM_S390_VM_CPU_MODEL		3
>  #define KVM_S390_VM_MIGRATION		4
> +#define KVM_S390_VM_MISC		5
>  
>  /* kvm attributes for mem_ctrl */
>  #define KVM_S390_VM_MEM_ENABLE_CMMA	0
> @@ -171,6 +172,9 @@ struct kvm_s390_vm_cpu_subfunc {
>  #define KVM_S390_VM_MIGRATION_START	1
>  #define KVM_S390_VM_MIGRATION_STATUS	2
>  
> +/* kvm attributes for KVM_S390_VM_MISC */
> +#define KVM_S390_VM_MISC_DIAG318	0
> +
>  /* for KVM_GET_REGS and KVM_SET_REGS */
>  struct kvm_regs {
>  	/* general purpose regs for s390 */
> 

Acked-by: David Hildenbrand <david@redhat.com>

-- 

Thanks,

David / dhildenb


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

* Re: [Qemu-devel] [PATCH v5 2/2] s390: diagnose 318 info reset and migration support
  2019-06-26  9:12   ` Christian Borntraeger
@ 2019-06-26 12:14     ` Cornelia Huck
  2019-06-26 14:22       ` [Qemu-devel] [qemu-s390x] " Collin Walling
  0 siblings, 1 reply; 13+ messages in thread
From: Cornelia Huck @ 2019-06-26 12:14 UTC (permalink / raw)
  To: Christian Borntraeger
  Cc: Collin Walling, david, mst, qemu-devel, pasic, qemu-s390x, pbonzini, rth

On Wed, 26 Jun 2019 11:12:04 +0200
Christian Borntraeger <borntraeger@de.ibm.com> wrote:

> On 25.06.19 17:17, Collin Walling wrote:
> > index a606547..4c26754 100644
> > --- a/target/s390x/cpu.h
> > +++ b/target/s390x/cpu.h
> > @@ -39,7 +39,13 @@
> >  
> >  #define MMU_USER_IDX 0
> >  
> > -#define S390_MAX_CPUS 248
> > +/*
> > + * HACK: The introduction of additional facility bytes in the Read Info
> > + * struct consumes space used for CPU entries, thus we must reduce the
> > + * original maximum CPUs of 248 by one for each new byte or risk smashing
> > + * the stack.
> > + */
> > +#define S390_MAX_CPUS 247  
> 
> I think we decided to not change that. Only if the cpu model contains the diag318
> feature we are limited to 247 but only for the sclp response.
> So we said: 
> - we continue to allow 248 cpus
> - the sclp response will be limited to 247 CPUs if the feature is one
> - (optional) we print a warning that the guest might not see all CPUs
> 

Yes, that's what I remember as well... and printing/logging a warning
is a good idea.


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

* Re: [Qemu-devel] [PATCH v5 2/2] s390: diagnose 318 info reset and migration support
  2019-06-25 15:17 ` [Qemu-devel] [PATCH v5 2/2] s390: diagnose 318 info reset and migration support Collin Walling
  2019-06-26  9:12   ` Christian Borntraeger
@ 2019-06-26 12:33   ` Cornelia Huck
  2019-06-26 14:07     ` Collin Walling
  1 sibling, 1 reply; 13+ messages in thread
From: Cornelia Huck @ 2019-06-26 12:33 UTC (permalink / raw)
  To: Collin Walling
  Cc: david, mst, qemu-devel, pasic, borntraeger, qemu-s390x, pbonzini, rth

On Tue, 25 Jun 2019 11:17:09 -0400
Collin Walling <walling@linux.ibm.com> wrote:

> DIAGNOSE 0x318 (diag318) is a privileged s390x instruction that must
> be intercepted by SIE and handled via KVM. Let's introduce some
> functions to communicate between QEMU and KVM via ioctls. These
> will be used to get/set the diag318 information.
> 
> The availability of this instruction is determined by byte 134, bit 0
> of the Read Info block. This coincidentally expands into the space used
> for CPU entries, which means VMs running with the diag318 capability
> will have a reduced maximum CPU count. Let's reduce the maximum CPU
> count from 248 to 247.
> 
> In order to simplify the migration and system reset requirements of
> the diag318 data, let's introduce it as a device class and include
> a VMStateDescription.
> 
> Diag318 is set to 0 during modified clear and load normal resets.
> 
> Signed-off-by: Collin Walling <walling@linux.ibm.com>
> ---
>  hw/s390x/Makefile.objs          |  1 +
>  hw/s390x/diag318.c              | 80 +++++++++++++++++++++++++++++++++++++++++
>  hw/s390x/diag318.h              | 38 ++++++++++++++++++++
>  hw/s390x/s390-virtio-ccw.c      | 17 +++++++++
>  hw/s390x/sclp.c                 |  3 ++
>  include/hw/s390x/sclp.h         |  2 ++
>  target/s390x/cpu.h              |  8 ++++-
>  target/s390x/cpu_features.c     |  3 ++
>  target/s390x/cpu_features.h     |  1 +
>  target/s390x/cpu_features_def.h |  3 ++
>  target/s390x/gen-features.c     |  1 +
>  target/s390x/kvm-stub.c         | 10 ++++++
>  target/s390x/kvm.c              | 29 +++++++++++++++
>  target/s390x/kvm_s390x.h        |  2 ++
>  14 files changed, 197 insertions(+), 1 deletion(-)
>  create mode 100644 hw/s390x/diag318.c
>  create mode 100644 hw/s390x/diag318.h

(...)

> diff --git a/hw/s390x/diag318.c b/hw/s390x/diag318.c
> new file mode 100644
> index 0000000..0eb80fe
> --- /dev/null
> +++ b/hw/s390x/diag318.c
> @@ -0,0 +1,80 @@
> +/*
> + * DIAGNOSE 0x318 functions for reset and migration
> + *
> + * Copyright IBM, Corp. 2019
> + *
> + * Authors:
> + *  Collin Walling <walling@linux.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/diag318.h"
> +#include "qapi/error.h"
> +#include "kvm_s390x.h"
> +#include "sysemu/kvm.h"
> +
> +static int diag318_post_load(void *opaque, int version_id)
> +{
> +    DIAG318State *d = opaque;
> +
> +    kvm_s390_set_diag318_info(d->info);

Shouldn't we at least moan if something goes wrong here?

> +    return 0;
> +}
> +
> +static int diag318_pre_save(void *opaque)
> +{
> +    DIAG318State *d = opaque;
> +
> +    kvm_s390_get_diag318_info(&d->info);
> +    return 0;
> +}
> +
> +static bool diag318_needed(void *opaque)
> +{
> +    return s390_has_feat(S390_FEAT_DIAG318);

What happens if we emulate diag 318 in tcg and set this feature bit? We
probably don't want to call kvm_ functions in that case.

> +}
> +
> +const VMStateDescription vmstate_diag318 = {
> +    .name = "vmstate_diag318",
> +    .post_load = diag318_post_load,
> +    .pre_save = diag318_pre_save,
> +    .version_id = 1,
> +    .minimum_version_id = 1,
> +    .needed = diag318_needed,
> +    .fields = (VMStateField[]) {
> +        VMSTATE_UINT64(info, DIAG318State),
> +        VMSTATE_END_OF_LIST()
> +    }
> +};
> +
> +static void s390_diag318_reset(DeviceState *dev)
> +{
> +    kvm_s390_set_diag318_info(0);

Same here; we probably need a s390_set_diag318_info() function that
either calls the kvm_ function or resets some internal state.

> +}
> +
> +static void s390_diag318_class_init(ObjectClass *klass, void *data)
> +{
> +    DeviceClass *dc = DEVICE_CLASS(klass);
> +
> +    dc->reset = s390_diag318_reset;
> +    dc->vmsd = &vmstate_diag318;
> +    dc->hotpluggable = false;
> +    /* Reason: Set automatically during IPL */

"Created automatically during machine instantiation" ?

> +    dc->user_creatable = false;
> +}
> +
> +static const TypeInfo s390_diag318_info = {
> +    .class_init = s390_diag318_class_init,
> +    .parent = TYPE_DEVICE,
> +    .name = TYPE_S390_DIAG318,
> +    .instance_size = sizeof(DIAG318State),
> +};
> +
> +static void s390_diag318_register_types(void)
> +{
> +    type_register_static(&s390_diag318_info);
> +}
> +
> +type_init(s390_diag318_register_types)

(..)

> diff --git a/hw/s390x/s390-virtio-ccw.c b/hw/s390x/s390-virtio-ccw.c
> index 87b2039..54230c7 100644
> --- a/hw/s390x/s390-virtio-ccw.c
> +++ b/hw/s390x/s390-virtio-ccw.c
> @@ -38,6 +38,7 @@
>  #include "cpu_models.h"
>  #include "hw/nmi.h"
>  #include "hw/s390x/tod.h"
> +#include "hw/s390x/diag318.h"
>  
>  S390CPU *s390_cpu_addr2state(uint16_t cpu_addr)
>  {
> @@ -94,6 +95,7 @@ static const char *const reset_dev_types[] = {
>      "s390-sclp-event-facility",
>      "s390-flic",
>      "diag288",
> +    TYPE_S390_DIAG318,

This looks a bit odd, although it is not wrong :)

>  };
>  
>  static void subsystem_reset(void)
> @@ -258,6 +260,17 @@ static void s390_create_sclpconsole(const char *type, Chardev *chardev)
>      qdev_init_nofail(dev);
>  }
>  
> +static void s390_init_diag318(void)
> +{
> +    Object *new = object_new(TYPE_S390_DIAG318);
> +    DeviceState *dev = DEVICE(new);
> +
> +    object_property_add_child(qdev_get_machine(), TYPE_S390_DIAG318,
> +                              new, NULL);
> +    object_unref(new);
> +    qdev_init_nofail(dev);
> +}
> +
>  static void ccw_init(MachineState *machine)
>  {
>      int ret;
> @@ -315,6 +328,9 @@ static void ccw_init(MachineState *machine)
>  
>      /* init the TOD clock */
>      s390_init_tod();
> +
> +    /* init object used for migrating diag318 info */
> +    s390_init_diag318();
>  }
>  
>  static void s390_cpu_plug(HotplugHandler *hotplug_dev,
> @@ -583,6 +599,7 @@ static void machine_set_loadparm(Object *obj, const char *val, Error **errp)
>          ms->loadparm[i] = ' '; /* pad right with spaces */
>      }
>  }
> +

unrelated whitespace change :)

>  static inline void s390_machine_initfn(Object *obj)
>  {
>      object_property_add_bool(obj, "aes-key-wrap",

(...)

> diff --git a/target/s390x/cpu_features.c b/target/s390x/cpu_features.c
> index f64f581..77a1df5 100644
> --- a/target/s390x/cpu_features.c
> +++ b/target/s390x/cpu_features.c
> @@ -127,6 +127,9 @@ static const S390FeatDef s390_features[] = {
>      FEAT_INIT("pfmfi", S390_FEAT_TYPE_SCLP_CONF_CHAR_EXT, 9, "SIE: PFMF interpretation facility"),
>      FEAT_INIT("ibs", S390_FEAT_TYPE_SCLP_CONF_CHAR_EXT, 10, "SIE: Interlock-and-broadcast-suppression facility"),
>  
> +    /* SCLP SCCB Byte 134 */
> +    FEAT_INIT("diag318", S390_FEAT_TYPE_SCLP_BYTE_134, 0, "Control program name and version codes"),
> +
>      FEAT_INIT("sief2", S390_FEAT_TYPE_SCLP_CPU, 4, "SIE: interception format 2 (Virtual SIE)"),
>      FEAT_INIT("skey", S390_FEAT_TYPE_SCLP_CPU, 5, "SIE: Storage-key facility"),
>      FEAT_INIT("gpereh", S390_FEAT_TYPE_SCLP_CPU, 10, "SIE: Guest-PER enhancement facility"),
> diff --git a/target/s390x/cpu_features.h b/target/s390x/cpu_features.h
> index da695a8..954544e 100644
> --- a/target/s390x/cpu_features.h
> +++ b/target/s390x/cpu_features.h
> @@ -23,6 +23,7 @@ typedef enum {
>      S390_FEAT_TYPE_STFL,
>      S390_FEAT_TYPE_SCLP_CONF_CHAR,
>      S390_FEAT_TYPE_SCLP_CONF_CHAR_EXT,
> +    S390_FEAT_TYPE_SCLP_BYTE_134,
>      S390_FEAT_TYPE_SCLP_CPU,
>      S390_FEAT_TYPE_MISC,
>      S390_FEAT_TYPE_PLO,
> diff --git a/target/s390x/cpu_features_def.h b/target/s390x/cpu_features_def.h
> index 292b17b..4f2c23e 100644
> --- a/target/s390x/cpu_features_def.h
> +++ b/target/s390x/cpu_features_def.h
> @@ -115,6 +115,9 @@ typedef enum {
>      S390_FEAT_SIE_PFMFI,
>      S390_FEAT_SIE_IBS,
>  
> +    /* Sclp Byte 134 */
> +    S390_FEAT_DIAG318,
> +
>      /* Sclp Cpu */
>      S390_FEAT_SIE_F2,
>      S390_FEAT_SIE_SKEY,
> diff --git a/target/s390x/gen-features.c b/target/s390x/gen-features.c
> index dc320a0..cdd1875 100644
> --- a/target/s390x/gen-features.c
> +++ b/target/s390x/gen-features.c
> @@ -521,6 +521,7 @@ static uint16_t full_GEN12_GA1[] = {
>      S390_FEAT_AP_QUERY_CONFIG_INFO,
>      S390_FEAT_AP_FACILITIES_TEST,
>      S390_FEAT_AP,
> +    S390_FEAT_DIAG318,
>  };
>  
>  static uint16_t full_GEN12_GA2[] = {

The feature bit stuff probably needs some (easy) adaption on top of my
s390-next branch.


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

* Re: [Qemu-devel] [PATCH v5 2/2] s390: diagnose 318 info reset and migration support
  2019-06-26 12:33   ` [Qemu-devel] " Cornelia Huck
@ 2019-06-26 14:07     ` Collin Walling
  2019-07-02 11:05       ` Cornelia Huck
  0 siblings, 1 reply; 13+ messages in thread
From: Collin Walling @ 2019-06-26 14:07 UTC (permalink / raw)
  To: Cornelia Huck
  Cc: david, mst, qemu-devel, pasic, borntraeger, qemu-s390x, pbonzini, rth

On 6/26/19 8:33 AM, Cornelia Huck wrote:
> On Tue, 25 Jun 2019 11:17:09 -0400
> Collin Walling <walling@linux.ibm.com> wrote:
> 
>> DIAGNOSE 0x318 (diag318) is a privileged s390x instruction that must
>> be intercepted by SIE and handled via KVM. Let's introduce some
>> functions to communicate between QEMU and KVM via ioctls. These
>> will be used to get/set the diag318 information.
>>
>> The availability of this instruction is determined by byte 134, bit 0
>> of the Read Info block. This coincidentally expands into the space used
>> for CPU entries, which means VMs running with the diag318 capability
>> will have a reduced maximum CPU count. Let's reduce the maximum CPU
>> count from 248 to 247.
>>
>> In order to simplify the migration and system reset requirements of
>> the diag318 data, let's introduce it as a device class and include
>> a VMStateDescription.
>>
>> Diag318 is set to 0 during modified clear and load normal resets.
>>
>> Signed-off-by: Collin Walling <walling@linux.ibm.com>
>> ---
>>   hw/s390x/Makefile.objs          |  1 +
>>   hw/s390x/diag318.c              | 80 +++++++++++++++++++++++++++++++++++++++++
>>   hw/s390x/diag318.h              | 38 ++++++++++++++++++++
>>   hw/s390x/s390-virtio-ccw.c      | 17 +++++++++
>>   hw/s390x/sclp.c                 |  3 ++
>>   include/hw/s390x/sclp.h         |  2 ++
>>   target/s390x/cpu.h              |  8 ++++-
>>   target/s390x/cpu_features.c     |  3 ++
>>   target/s390x/cpu_features.h     |  1 +
>>   target/s390x/cpu_features_def.h |  3 ++
>>   target/s390x/gen-features.c     |  1 +
>>   target/s390x/kvm-stub.c         | 10 ++++++
>>   target/s390x/kvm.c              | 29 +++++++++++++++
>>   target/s390x/kvm_s390x.h        |  2 ++
>>   14 files changed, 197 insertions(+), 1 deletion(-)
>>   create mode 100644 hw/s390x/diag318.c
>>   create mode 100644 hw/s390x/diag318.h
> 
> (...)
> 
>> diff --git a/hw/s390x/diag318.c b/hw/s390x/diag318.c
>> new file mode 100644
>> index 0000000..0eb80fe
>> --- /dev/null
>> +++ b/hw/s390x/diag318.c
>> @@ -0,0 +1,80 @@
>> +/*
>> + * DIAGNOSE 0x318 functions for reset and migration
>> + *
>> + * Copyright IBM, Corp. 2019
>> + *
>> + * Authors:
>> + *  Collin Walling <walling@linux.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/diag318.h"
>> +#include "qapi/error.h"
>> +#include "kvm_s390x.h"
>> +#include "sysemu/kvm.h"
>> +
>> +static int diag318_post_load(void *opaque, int version_id)
>> +{
>> +    DIAG318State *d = opaque;
>> +
>> +    kvm_s390_set_diag318_info(d->info);
> 
> Shouldn't we at least moan if something goes wrong here?
> 

What are some things that might go wrong at this point? We
should only call this if the diag318 feature is enabled. If
we try to toggle that feature on a machine that cannot support
it, the guest will fail to start, stating (paraphrased) "some
features are not available in the configuration: diag318"

Is there some sort of issue that could arise from QEMU that I'm not
considering?

>> +    return 0;
>> +}
>> +
>> +static int diag318_pre_save(void *opaque)
>> +{
>> +    DIAG318State *d = opaque;
>> +
>> +    kvm_s390_get_diag318_info(&d->info);
>> +    return 0;
>> +}
>> +
>> +static bool diag318_needed(void *opaque)
>> +{
>> +    return s390_has_feat(S390_FEAT_DIAG318);
> 
> What happens if we emulate diag 318 in tcg and set this feature bit? We
> probably don't want to call kvm_ functions in that case.
> 

I have not tested on tcg.

>> +}
>> +
>> +const VMStateDescription vmstate_diag318 = {
>> +    .name = "vmstate_diag318",
>> +    .post_load = diag318_post_load,
>> +    .pre_save = diag318_pre_save,
>> +    .version_id = 1,
>> +    .minimum_version_id = 1,
>> +    .needed = diag318_needed,
>> +    .fields = (VMStateField[]) {
>> +        VMSTATE_UINT64(info, DIAG318State),
>> +        VMSTATE_END_OF_LIST()
>> +    }
>> +};
>> +
>> +static void s390_diag318_reset(DeviceState *dev)
>> +{
>> +    kvm_s390_set_diag318_info(0);
> 
> Same here; we probably need a s390_set_diag318_info() function that
> either calls the kvm_ function or resets some internal state.
> 

Hmm okay. I recall doing this for the TOD-clock stuff way-back. I'll
include these functions next round.

>> +}
>> +
>> +static void s390_diag318_class_init(ObjectClass *klass, void *data)
>> +{
>> +    DeviceClass *dc = DEVICE_CLASS(klass);
>> +
>> +    dc->reset = s390_diag318_reset;
>> +    dc->vmsd = &vmstate_diag318;
>> +    dc->hotpluggable = false;
>> +    /* Reason: Set automatically during IPL */
> 
> "Created automatically during machine instantiation" ?
> 

I like it.

>> +    dc->user_creatable = false;
>> +}
>> +
>> +static const TypeInfo s390_diag318_info = {
>> +    .class_init = s390_diag318_class_init,
>> +    .parent = TYPE_DEVICE,
>> +    .name = TYPE_S390_DIAG318,
>> +    .instance_size = sizeof(DIAG318State),
>> +};
>> +
>> +static void s390_diag318_register_types(void)
>> +{
>> +    type_register_static(&s390_diag318_info);
>> +}
>> +
>> +type_init(s390_diag318_register_types)
> 
> (..)
> 
>> diff --git a/hw/s390x/s390-virtio-ccw.c b/hw/s390x/s390-virtio-ccw.c
>> index 87b2039..54230c7 100644
>> --- a/hw/s390x/s390-virtio-ccw.c
>> +++ b/hw/s390x/s390-virtio-ccw.c
>> @@ -38,6 +38,7 @@
>>   #include "cpu_models.h"
>>   #include "hw/nmi.h"
>>   #include "hw/s390x/tod.h"
>> +#include "hw/s390x/diag318.h"
>>   
>>   S390CPU *s390_cpu_addr2state(uint16_t cpu_addr)
>>   {
>> @@ -94,6 +95,7 @@ static const char *const reset_dev_types[] = {
>>       "s390-sclp-event-facility",
>>       "s390-flic",
>>       "diag288",
>> +    TYPE_S390_DIAG318,
> 
> This looks a bit odd, although it is not wrong :)
> 

It's cut-off here, but TYPE_VIRTUAL_CSS_BRIDGE is also in the list :)

>>   };
>>   
>>   static void subsystem_reset(void)
>> @@ -258,6 +260,17 @@ static void s390_create_sclpconsole(const char *type, Chardev *chardev)
>>       qdev_init_nofail(dev);
>>   }
>>   
>> +static void s390_init_diag318(void)
>> +{
>> +    Object *new = object_new(TYPE_S390_DIAG318);
>> +    DeviceState *dev = DEVICE(new);
>> +
>> +    object_property_add_child(qdev_get_machine(), TYPE_S390_DIAG318,
>> +                              new, NULL);
>> +    object_unref(new);
>> +    qdev_init_nofail(dev);
>> +}
>> +
>>   static void ccw_init(MachineState *machine)
>>   {
>>       int ret;
>> @@ -315,6 +328,9 @@ static void ccw_init(MachineState *machine)
>>   
>>       /* init the TOD clock */
>>       s390_init_tod();
>> +
>> +    /* init object used for migrating diag318 info */
>> +    s390_init_diag318();
>>   }
>>   
>>   static void s390_cpu_plug(HotplugHandler *hotplug_dev,
>> @@ -583,6 +599,7 @@ static void machine_set_loadparm(Object *obj, const char *val, Error **errp)
>>           ms->loadparm[i] = ' '; /* pad right with spaces */
>>       }
>>   }
>> +
> 
> unrelated whitespace change :)
> 

Whoops, thanks.

>>   static inline void s390_machine_initfn(Object *obj)
>>   {
>>       object_property_add_bool(obj, "aes-key-wrap",
> 
> (...)
> 
>> diff --git a/target/s390x/cpu_features.c b/target/s390x/cpu_features.c
>> index f64f581..77a1df5 100644
>> --- a/target/s390x/cpu_features.c
>> +++ b/target/s390x/cpu_features.c
>> @@ -127,6 +127,9 @@ static const S390FeatDef s390_features[] = {
>>       FEAT_INIT("pfmfi", S390_FEAT_TYPE_SCLP_CONF_CHAR_EXT, 9, "SIE: PFMF interpretation facility"),
>>       FEAT_INIT("ibs", S390_FEAT_TYPE_SCLP_CONF_CHAR_EXT, 10, "SIE: Interlock-and-broadcast-suppression facility"),
>>   
>> +    /* SCLP SCCB Byte 134 */
>> +    FEAT_INIT("diag318", S390_FEAT_TYPE_SCLP_BYTE_134, 0, "Control program name and version codes"),
>> +
>>       FEAT_INIT("sief2", S390_FEAT_TYPE_SCLP_CPU, 4, "SIE: interception format 2 (Virtual SIE)"),
>>       FEAT_INIT("skey", S390_FEAT_TYPE_SCLP_CPU, 5, "SIE: Storage-key facility"),
>>       FEAT_INIT("gpereh", S390_FEAT_TYPE_SCLP_CPU, 10, "SIE: Guest-PER enhancement facility"),
>> diff --git a/target/s390x/cpu_features.h b/target/s390x/cpu_features.h
>> index da695a8..954544e 100644
>> --- a/target/s390x/cpu_features.h
>> +++ b/target/s390x/cpu_features.h
>> @@ -23,6 +23,7 @@ typedef enum {
>>       S390_FEAT_TYPE_STFL,
>>       S390_FEAT_TYPE_SCLP_CONF_CHAR,
>>       S390_FEAT_TYPE_SCLP_CONF_CHAR_EXT,
>> +    S390_FEAT_TYPE_SCLP_BYTE_134,
>>       S390_FEAT_TYPE_SCLP_CPU,
>>       S390_FEAT_TYPE_MISC,
>>       S390_FEAT_TYPE_PLO,
>> diff --git a/target/s390x/cpu_features_def.h b/target/s390x/cpu_features_def.h
>> index 292b17b..4f2c23e 100644
>> --- a/target/s390x/cpu_features_def.h
>> +++ b/target/s390x/cpu_features_def.h
>> @@ -115,6 +115,9 @@ typedef enum {
>>       S390_FEAT_SIE_PFMFI,
>>       S390_FEAT_SIE_IBS,
>>   
>> +    /* Sclp Byte 134 */
>> +    S390_FEAT_DIAG318,
>> +
>>       /* Sclp Cpu */
>>       S390_FEAT_SIE_F2,
>>       S390_FEAT_SIE_SKEY,
>> diff --git a/target/s390x/gen-features.c b/target/s390x/gen-features.c
>> index dc320a0..cdd1875 100644
>> --- a/target/s390x/gen-features.c
>> +++ b/target/s390x/gen-features.c
>> @@ -521,6 +521,7 @@ static uint16_t full_GEN12_GA1[] = {
>>       S390_FEAT_AP_QUERY_CONFIG_INFO,
>>       S390_FEAT_AP_FACILITIES_TEST,
>>       S390_FEAT_AP,
>> +    S390_FEAT_DIAG318,
>>   };
>>   
>>   static uint16_t full_GEN12_GA2[] = {
> 
> The feature bit stuff probably needs some (easy) adaption on top of my
> s390-next branch.
> 

I just noticed David's changes to that code. Should make this
feature stuff a bit easier.

Thanks for the review!



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

* Re: [Qemu-devel] [qemu-s390x] [PATCH v5 2/2] s390: diagnose 318 info reset and migration support
  2019-06-26 12:14     ` Cornelia Huck
@ 2019-06-26 14:22       ` Collin Walling
  2019-06-26 14:30         ` David Hildenbrand
  2019-06-27 10:29         ` Christian Borntraeger
  0 siblings, 2 replies; 13+ messages in thread
From: Collin Walling @ 2019-06-26 14:22 UTC (permalink / raw)
  To: Cornelia Huck, Christian Borntraeger
  Cc: david, mst, qemu-devel, pasic, qemu-s390x, pbonzini, rth

On 6/26/19 8:14 AM, Cornelia Huck wrote:
> On Wed, 26 Jun 2019 11:12:04 +0200
> Christian Borntraeger <borntraeger@de.ibm.com> wrote:
> 
>> On 25.06.19 17:17, Collin Walling wrote:
>>> index a606547..4c26754 100644
>>> --- a/target/s390x/cpu.h
>>> +++ b/target/s390x/cpu.h
>>> @@ -39,7 +39,13 @@
>>>   
>>>   #define MMU_USER_IDX 0
>>>   
>>> -#define S390_MAX_CPUS 248
>>> +/*
>>> + * HACK: The introduction of additional facility bytes in the Read Info
>>> + * struct consumes space used for CPU entries, thus we must reduce the
>>> + * original maximum CPUs of 248 by one for each new byte or risk smashing
>>> + * the stack.
>>> + */
>>> +#define S390_MAX_CPUS 247
>>
>> I think we decided to not change that. Only if the cpu model contains the diag318
>> feature we are limited to 247 but only for the sclp response.
>> So we said:
>> - we continue to allow 248 cpus
>> - the sclp response will be limited to 247 CPUs if the feature is one
>> - (optional) we print a warning that the guest might not see all CPUs
>>
> 
> Yes, that's what I remember as well... and printing/logging a warning
> is a good idea.
> 

I recall this conversation, but I encountered a bit of a hangup when
running some tests with the new changes.

Since we're adding a new field in the ReadInfo struct, we're permanently
intruding on the space used for CPU entries. A machine with these 
changes and 248 CPUs resulted in stack smash when the guest would start 
up. This happened with diag318 on *and* off. This is a limitation to the
4k SCCB size right now :/

Prior to these patches, I was restricting the max_cpus depending on the
compat version. I failed to do some tests with earlier versions to catch
this error (there are a lot of moving parts... sorry).

I do not think the new byte and the full 248 CPU count can co-exist. We
might be able to union byte 134 and some extra space with the first CPU 
entry... but that could get messy.




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

* Re: [Qemu-devel] [qemu-s390x] [PATCH v5 2/2] s390: diagnose 318 info reset and migration support
  2019-06-26 14:22       ` [Qemu-devel] [qemu-s390x] " Collin Walling
@ 2019-06-26 14:30         ` David Hildenbrand
  2019-06-27 10:29         ` Christian Borntraeger
  1 sibling, 0 replies; 13+ messages in thread
From: David Hildenbrand @ 2019-06-26 14:30 UTC (permalink / raw)
  To: Collin Walling, Cornelia Huck, Christian Borntraeger
  Cc: mst, qemu-devel, pasic, qemu-s390x, pbonzini, rth

On 26.06.19 16:22, Collin Walling wrote:
> On 6/26/19 8:14 AM, Cornelia Huck wrote:
>> On Wed, 26 Jun 2019 11:12:04 +0200
>> Christian Borntraeger <borntraeger@de.ibm.com> wrote:
>>
>>> On 25.06.19 17:17, Collin Walling wrote:
>>>> index a606547..4c26754 100644
>>>> --- a/target/s390x/cpu.h
>>>> +++ b/target/s390x/cpu.h
>>>> @@ -39,7 +39,13 @@
>>>>   
>>>>   #define MMU_USER_IDX 0
>>>>   
>>>> -#define S390_MAX_CPUS 248
>>>> +/*
>>>> + * HACK: The introduction of additional facility bytes in the Read Info
>>>> + * struct consumes space used for CPU entries, thus we must reduce the
>>>> + * original maximum CPUs of 248 by one for each new byte or risk smashing
>>>> + * the stack.
>>>> + */
>>>> +#define S390_MAX_CPUS 247
>>>
>>> I think we decided to not change that. Only if the cpu model contains the diag318
>>> feature we are limited to 247 but only for the sclp response.
>>> So we said:
>>> - we continue to allow 248 cpus
>>> - the sclp response will be limited to 247 CPUs if the feature is one
>>> - (optional) we print a warning that the guest might not see all CPUs
>>>
>>
>> Yes, that's what I remember as well... and printing/logging a warning
>> is a good idea.
>>
> 
> I recall this conversation, but I encountered a bit of a hangup when
> running some tests with the new changes.
> 
> Since we're adding a new field in the ReadInfo struct, we're permanently
> intruding on the space used for CPU entries. A machine with these 
> changes and 248 CPUs resulted in stack smash when the guest would start 
> up. This happened with diag318 on *and* off. This is a limitation to the
> 4k SCCB size right now :/

I think we discussed to only indicate 247 CPUs, which should solve the
issue?

> 
> Prior to these patches, I was restricting the max_cpus depending on the
> compat version. I failed to do some tests with earlier versions to catch
> this error (there are a lot of moving parts... sorry).
> 
> I do not think the new byte and the full 248 CPU count can co-exist. We
> might be able to union byte 134 and some extra space with the first CPU 
> entry... but that could get messy.
> 
> 


-- 

Thanks,

David / dhildenb


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

* Re: [Qemu-devel] [qemu-s390x] [PATCH v5 2/2] s390: diagnose 318 info reset and migration support
  2019-06-26 14:22       ` [Qemu-devel] [qemu-s390x] " Collin Walling
  2019-06-26 14:30         ` David Hildenbrand
@ 2019-06-27 10:29         ` Christian Borntraeger
  1 sibling, 0 replies; 13+ messages in thread
From: Christian Borntraeger @ 2019-06-27 10:29 UTC (permalink / raw)
  To: Collin Walling, Cornelia Huck
  Cc: david, mst, qemu-devel, pasic, qemu-s390x, pbonzini, rth



On 26.06.19 16:22, Collin Walling wrote:
> On 6/26/19 8:14 AM, Cornelia Huck wrote:
>> On Wed, 26 Jun 2019 11:12:04 +0200
>> Christian Borntraeger <borntraeger@de.ibm.com> wrote:
>>
>>> On 25.06.19 17:17, Collin Walling wrote:
>>>> index a606547..4c26754 100644
>>>> --- a/target/s390x/cpu.h
>>>> +++ b/target/s390x/cpu.h
>>>> @@ -39,7 +39,13 @@
>>>>     #define MMU_USER_IDX 0
>>>>   -#define S390_MAX_CPUS 248
>>>> +/*
>>>> + * HACK: The introduction of additional facility bytes in the Read Info
>>>> + * struct consumes space used for CPU entries, thus we must reduce the
>>>> + * original maximum CPUs of 248 by one for each new byte or risk smashing
>>>> + * the stack.
>>>> + */
>>>> +#define S390_MAX_CPUS 247
>>>
>>> I think we decided to not change that. Only if the cpu model contains the diag318
>>> feature we are limited to 247 but only for the sclp response.
>>> So we said:
>>> - we continue to allow 248 cpus
>>> - the sclp response will be limited to 247 CPUs if the feature is one
>>> - (optional) we print a warning that the guest might not see all CPUs
>>>
>>
>> Yes, that's what I remember as well... and printing/logging a warning
>> is a good idea.
>>
> 
> I recall this conversation, but I encountered a bit of a hangup when
> running some tests with the new changes.
> 
> Since we're adding a new field in the ReadInfo struct, we're permanently
> intruding on the space used for CPU entries. A machine with these changes and 248 CPUs resulted in stack smash when the guest would start up. This happened with diag318 on *and* off. This is a limitation to the
> 4k SCCB size right now :/

Yes of course, you need to touch sclp_read_cpu_info to not overwrite the buffer.

Since we are going to provider larger sccbs in the future I do not see a point
in limiting this now to 247 just the increase that back later on.



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

* Re: [Qemu-devel] [PATCH v5 2/2] s390: diagnose 318 info reset and migration support
  2019-06-26 14:07     ` Collin Walling
@ 2019-07-02 11:05       ` Cornelia Huck
  0 siblings, 0 replies; 13+ messages in thread
From: Cornelia Huck @ 2019-07-02 11:05 UTC (permalink / raw)
  To: Collin Walling
  Cc: david, mst, qemu-devel, pasic, borntraeger, qemu-s390x, pbonzini, rth

On Wed, 26 Jun 2019 10:07:39 -0400
Collin Walling <walling@linux.ibm.com> wrote:

> On 6/26/19 8:33 AM, Cornelia Huck wrote:
> > On Tue, 25 Jun 2019 11:17:09 -0400
> > Collin Walling <walling@linux.ibm.com> wrote:
> >   
> >> DIAGNOSE 0x318 (diag318) is a privileged s390x instruction that must
> >> be intercepted by SIE and handled via KVM. Let's introduce some
> >> functions to communicate between QEMU and KVM via ioctls. These
> >> will be used to get/set the diag318 information.
> >>
> >> The availability of this instruction is determined by byte 134, bit 0
> >> of the Read Info block. This coincidentally expands into the space used
> >> for CPU entries, which means VMs running with the diag318 capability
> >> will have a reduced maximum CPU count. Let's reduce the maximum CPU
> >> count from 248 to 247.
> >>
> >> In order to simplify the migration and system reset requirements of
> >> the diag318 data, let's introduce it as a device class and include
> >> a VMStateDescription.
> >>
> >> Diag318 is set to 0 during modified clear and load normal resets.
> >>
> >> Signed-off-by: Collin Walling <walling@linux.ibm.com>
> >> ---
> >>   hw/s390x/Makefile.objs          |  1 +
> >>   hw/s390x/diag318.c              | 80 +++++++++++++++++++++++++++++++++++++++++
> >>   hw/s390x/diag318.h              | 38 ++++++++++++++++++++
> >>   hw/s390x/s390-virtio-ccw.c      | 17 +++++++++
> >>   hw/s390x/sclp.c                 |  3 ++
> >>   include/hw/s390x/sclp.h         |  2 ++
> >>   target/s390x/cpu.h              |  8 ++++-
> >>   target/s390x/cpu_features.c     |  3 ++
> >>   target/s390x/cpu_features.h     |  1 +
> >>   target/s390x/cpu_features_def.h |  3 ++
> >>   target/s390x/gen-features.c     |  1 +
> >>   target/s390x/kvm-stub.c         | 10 ++++++
> >>   target/s390x/kvm.c              | 29 +++++++++++++++
> >>   target/s390x/kvm_s390x.h        |  2 ++
> >>   14 files changed, 197 insertions(+), 1 deletion(-)
> >>   create mode 100644 hw/s390x/diag318.c
> >>   create mode 100644 hw/s390x/diag318.h  
> > 
> > (...)
> >   
> >> diff --git a/hw/s390x/diag318.c b/hw/s390x/diag318.c
> >> new file mode 100644
> >> index 0000000..0eb80fe
> >> --- /dev/null
> >> +++ b/hw/s390x/diag318.c
> >> @@ -0,0 +1,80 @@
> >> +/*
> >> + * DIAGNOSE 0x318 functions for reset and migration
> >> + *
> >> + * Copyright IBM, Corp. 2019
> >> + *
> >> + * Authors:
> >> + *  Collin Walling <walling@linux.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/diag318.h"
> >> +#include "qapi/error.h"
> >> +#include "kvm_s390x.h"
> >> +#include "sysemu/kvm.h"
> >> +
> >> +static int diag318_post_load(void *opaque, int version_id)
> >> +{
> >> +    DIAG318State *d = opaque;
> >> +
> >> +    kvm_s390_set_diag318_info(d->info);  
> > 
> > Shouldn't we at least moan if something goes wrong here?
> >   
> 
> What are some things that might go wrong at this point? We
> should only call this if the diag318 feature is enabled. If
> we try to toggle that feature on a machine that cannot support
> it, the guest will fail to start, stating (paraphrased) "some
> features are not available in the configuration: diag318"
> 
> Is there some sort of issue that could arise from QEMU that I'm not
> considering?

The function might return an error; so I suspected that something may
possibly go wrong... checking seems like good practice in general.

> 
> >> +    return 0;
> >> +}
> >> +
> >> +static int diag318_pre_save(void *opaque)
> >> +{
> >> +    DIAG318State *d = opaque;
> >> +
> >> +    kvm_s390_get_diag318_info(&d->info);
> >> +    return 0;
> >> +}
> >> +
> >> +static bool diag318_needed(void *opaque)
> >> +{
> >> +    return s390_has_feat(S390_FEAT_DIAG318);  
> > 
> > What happens if we emulate diag 318 in tcg and set this feature bit? We
> > probably don't want to call kvm_ functions in that case.
> >   
> 
> I have not tested on tcg.

Well, it is not implemented right now :) My point is that we should not
tie this to *kvm* functions, but rather more generic functions.

> 
> >> +}
> >> +
> >> +const VMStateDescription vmstate_diag318 = {
> >> +    .name = "vmstate_diag318",
> >> +    .post_load = diag318_post_load,
> >> +    .pre_save = diag318_pre_save,
> >> +    .version_id = 1,
> >> +    .minimum_version_id = 1,
> >> +    .needed = diag318_needed,
> >> +    .fields = (VMStateField[]) {
> >> +        VMSTATE_UINT64(info, DIAG318State),
> >> +        VMSTATE_END_OF_LIST()
> >> +    }
> >> +};
> >> +
> >> +static void s390_diag318_reset(DeviceState *dev)
> >> +{
> >> +    kvm_s390_set_diag318_info(0);  
> > 
> > Same here; we probably need a s390_set_diag318_info() function that
> > either calls the kvm_ function or resets some internal state.
> >   
> 
> Hmm okay. I recall doing this for the TOD-clock stuff way-back. I'll
> include these functions next round.

Just a simple wrapper will suffice; we can then add any possible tcg
stuff whenever we get around to it (or never ;)

> 
> >> +}
> >> +
> >> +static void s390_diag318_class_init(ObjectClass *klass, void *data)
> >> +{
> >> +    DeviceClass *dc = DEVICE_CLASS(klass);
> >> +
> >> +    dc->reset = s390_diag318_reset;
> >> +    dc->vmsd = &vmstate_diag318;
> >> +    dc->hotpluggable = false;
> >> +    /* Reason: Set automatically during IPL */  
> > 
> > "Created automatically during machine instantiation" ?
> >   
> 
> I like it.
> 
> >> +    dc->user_creatable = false;
> >> +}
> >> +
> >> +static const TypeInfo s390_diag318_info = {
> >> +    .class_init = s390_diag318_class_init,
> >> +    .parent = TYPE_DEVICE,
> >> +    .name = TYPE_S390_DIAG318,
> >> +    .instance_size = sizeof(DIAG318State),
> >> +};
> >> +
> >> +static void s390_diag318_register_types(void)
> >> +{
> >> +    type_register_static(&s390_diag318_info);
> >> +}
> >> +
> >> +type_init(s390_diag318_register_types)  
> > 
> > (..)
> >   
> >> diff --git a/hw/s390x/s390-virtio-ccw.c b/hw/s390x/s390-virtio-ccw.c
> >> index 87b2039..54230c7 100644
> >> --- a/hw/s390x/s390-virtio-ccw.c
> >> +++ b/hw/s390x/s390-virtio-ccw.c
> >> @@ -38,6 +38,7 @@
> >>   #include "cpu_models.h"
> >>   #include "hw/nmi.h"
> >>   #include "hw/s390x/tod.h"
> >> +#include "hw/s390x/diag318.h"
> >>   
> >>   S390CPU *s390_cpu_addr2state(uint16_t cpu_addr)
> >>   {
> >> @@ -94,6 +95,7 @@ static const char *const reset_dev_types[] = {
> >>       "s390-sclp-event-facility",
> >>       "s390-flic",
> >>       "diag288",
> >> +    TYPE_S390_DIAG318,  
> > 
> > This looks a bit odd, although it is not wrong :)
> >   
> 
> It's cut-off here, but TYPE_VIRTUAL_CSS_BRIDGE is also in the list :)

Hah, better :)



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

end of thread, other threads:[~2019-07-02 11:13 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-06-25 15:17 [Qemu-devel] [PATCH v5 0/2] Guest Support for DIAGNOSE 0x318 Collin Walling
2019-06-25 15:17 ` [Qemu-devel] [PATCH v5 1/2] s390/kvm: header sync for diag318 Collin Walling
2019-06-26  9:43   ` David Hildenbrand
2019-06-25 15:17 ` [Qemu-devel] [PATCH v5 2/2] s390: diagnose 318 info reset and migration support Collin Walling
2019-06-26  9:12   ` Christian Borntraeger
2019-06-26 12:14     ` Cornelia Huck
2019-06-26 14:22       ` [Qemu-devel] [qemu-s390x] " Collin Walling
2019-06-26 14:30         ` David Hildenbrand
2019-06-27 10:29         ` Christian Borntraeger
2019-06-26 12:33   ` [Qemu-devel] " Cornelia Huck
2019-06-26 14:07     ` Collin Walling
2019-07-02 11:05       ` Cornelia Huck
2019-06-25 16:34 ` [Qemu-devel] [PATCH v5 0/2] Guest Support for DIAGNOSE 0x318 no-reply

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).