* [Qemu-devel] [PATCH 1/7] s390: Fix error handling and condition code of service call
2012-07-24 7:37 [Qemu-devel] [PATCH 0/7v3] s390: sclp patch set Christian Borntraeger
@ 2012-07-24 7:37 ` Christian Borntraeger
2012-07-24 7:37 ` [Qemu-devel] [PATCH 2/7] s390: provide interface for service interrupt/introduce interrupt.c Christian Borntraeger
` (6 subsequent siblings)
7 siblings, 0 replies; 33+ messages in thread
From: Christian Borntraeger @ 2012-07-24 7:37 UTC (permalink / raw)
To: Alexander Graf
Cc: Heinz Graalfs, qemu-devel, blauwirbel, Christian Borntraeger,
Jens Freimann, afaerber
Invalid sccb addresses will cause specification or addressing exception.
Lets add those checks. Furthermore, the good case (cc=0) was incorrect
for KVM, we did not set the CC at all. We now use return codes < 0
as program checks and return codes > 0 as condition code values.
Signed-off-by: Christian Borntraeger <borntraeger@de.ibm.com>
---
target-s390x/kvm.c | 5 +++--
target-s390x/op_helper.c | 27 ++++++++++++++++++---------
2 files changed, 21 insertions(+), 11 deletions(-)
diff --git a/target-s390x/kvm.c b/target-s390x/kvm.c
index 47008c2..07edf93 100644
--- a/target-s390x/kvm.c
+++ b/target-s390x/kvm.c
@@ -273,9 +273,10 @@ static int kvm_sclp_service_call(CPUS390XState *env, struct kvm_run *run,
code = env->regs[(ipbh0 & 0xf0) >> 4];
r = sclp_service_call(env, sccb, code);
- if (r) {
- setcc(env, 3);
+ if (r < 0) {
+ enter_pgmcheck(env, -r);
}
+ setcc(env, r);
return 0;
}
diff --git a/target-s390x/op_helper.c b/target-s390x/op_helper.c
index 7b72473..91dd8dc 100644
--- a/target-s390x/op_helper.c
+++ b/target-s390x/op_helper.c
@@ -19,6 +19,8 @@
*/
#include "cpu.h"
+#include "memory.h"
+#include "cputlb.h"
#include "dyngen-exec.h"
#include "host-utils.h"
#include "helper.h"
@@ -2366,6 +2368,9 @@ static void ext_interrupt(CPUS390XState *env, int type, uint32_t param,
cpu_inject_ext(env, type, param, param64);
}
+/*
+ * ret < 0 indicates program check, ret = 0,1,2,3 -> cc
+ */
int sclp_service_call(CPUS390XState *env, uint32_t sccb, uint64_t code)
{
int r = 0;
@@ -2375,10 +2380,12 @@ int sclp_service_call(CPUS390XState *env, uint32_t sccb, uint64_t code)
printf("sclp(0x%x, 0x%" PRIx64 ")\n", sccb, code);
#endif
+ /* basic checks */
+ if (!memory_region_is_ram(phys_page_find(sccb >> TARGET_PAGE_BITS)->mr)) {
+ return -PGM_ADDRESSING;
+ }
if (sccb & ~0x7ffffff8ul) {
- fprintf(stderr, "KVM: invalid sccb address 0x%x\n", sccb);
- r = -1;
- goto out;
+ return -PGM_SPECIFICATION;
}
switch(code) {
@@ -2405,22 +2412,24 @@ int sclp_service_call(CPUS390XState *env, uint32_t sccb, uint64_t code)
#ifdef DEBUG_HELPER
printf("KVM: invalid sclp call 0x%x / 0x%" PRIx64 "x\n", sccb, code);
#endif
- r = -1;
+ r = 3;
break;
}
-out:
return r;
}
/* SCLP service call */
uint32_t HELPER(servc)(uint32_t r1, uint64_t r2)
{
- if (sclp_service_call(env, r1, r2)) {
- return 3;
- }
+ int r;
- return 0;
+ r = sclp_service_call(env, r1, r2);
+ if (r < 0) {
+ program_interrupt(env, -r, 4);
+ return 0;
+ }
+ return r;
}
/* DIAG */
--
1.7.0.1
^ permalink raw reply related [flat|nested] 33+ messages in thread* [Qemu-devel] [PATCH 2/7] s390: provide interface for service interrupt/introduce interrupt.c
2012-07-24 7:37 [Qemu-devel] [PATCH 0/7v3] s390: sclp patch set Christian Borntraeger
2012-07-24 7:37 ` [Qemu-devel] [PATCH 1/7] s390: Fix error handling and condition code of service call Christian Borntraeger
@ 2012-07-24 7:37 ` Christian Borntraeger
2012-07-24 7:37 ` [Qemu-devel] [PATCH 3/7] s390: sclp base support Christian Borntraeger
` (5 subsequent siblings)
7 siblings, 0 replies; 33+ messages in thread
From: Christian Borntraeger @ 2012-07-24 7:37 UTC (permalink / raw)
To: Alexander Graf
Cc: Heinz Graalfs, qemu-devel, blauwirbel, Christian Borntraeger,
Jens Freimann, afaerber
This patch creates interrupt.c. The first user is a callback for hw/*
code to trigger an service interrupt for a given sccb value. Several
interrupt types for s390 are floating (can be delivered to all CPUs).
so this code does not belong to a specific CPU.
Other interrupts (like the virtio one) are also floating and can be
moved here later on.
Signed-off-by: Christian Borntraeger <borntraeger@de.ibm.com>
---
target-s390x/Makefile.objs | 2 +-
target-s390x/cpu.h | 3 +++
target-s390x/interrupt.c | 29 +++++++++++++++++++++++++++++
target-s390x/op_helper.c | 16 +---------------
4 files changed, 34 insertions(+), 16 deletions(-)
create mode 100644 target-s390x/interrupt.c
diff --git a/target-s390x/Makefile.objs b/target-s390x/Makefile.objs
index 262747f..80be3bb 100644
--- a/target-s390x/Makefile.objs
+++ b/target-s390x/Makefile.objs
@@ -1,4 +1,4 @@
-obj-y += translate.o op_helper.o helper.o cpu.o
+obj-y += translate.o op_helper.o helper.o cpu.o interrupt.o
obj-$(CONFIG_SOFTMMU) += machine.o
obj-$(CONFIG_KVM) += kvm.o
diff --git a/target-s390x/cpu.h b/target-s390x/cpu.h
index c30ac3a..18ac6e3 100644
--- a/target-s390x/cpu.h
+++ b/target-s390x/cpu.h
@@ -320,6 +320,9 @@ S390CPU *s390_cpu_addr2state(uint16_t cpu_addr);
void s390_add_running_cpu(CPUS390XState *env);
unsigned s390_del_running_cpu(CPUS390XState *env);
+/* service interrupts are floating therefore we must not pass an cpustate */
+void s390_sclp_extint(uint32_t parm);
+
/* from s390-virtio-bus */
extern const target_phys_addr_t virtio_size;
diff --git a/target-s390x/interrupt.c b/target-s390x/interrupt.c
new file mode 100644
index 0000000..c1b034f
--- /dev/null
+++ b/target-s390x/interrupt.c
@@ -0,0 +1,29 @@
+/*
+ * QEMU S/390 Interrupt support
+ *
+ * Copyright IBM, Corp. 2012
+ *
+ * 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 "cpu.h"
+#include "kvm.h"
+
+#if !defined(CONFIG_USER_ONLY)
+/* service interrupts are floating therefore we must not pass an cpustate */
+void s390_sclp_extint(uint32_t parm)
+{
+ S390CPU *dummy_cpu = s390_cpu_addr2state(0);
+ CPUS390XState *env = &dummy_cpu->env;
+
+ if (kvm_enabled()) {
+#ifdef CONFIG_KVM
+ kvm_s390_interrupt_internal(env, KVM_S390_INT_SERVICE, parm, 0, 1);
+#endif
+ } else {
+ env->psw.addr += 4;
+ cpu_inject_ext(env, EXT_SERVICE, parm, 0);
+ }
+}
+#endif
diff --git a/target-s390x/op_helper.c b/target-s390x/op_helper.c
index 91dd8dc..abc35dd 100644
--- a/target-s390x/op_helper.c
+++ b/target-s390x/op_helper.c
@@ -2362,12 +2362,6 @@ static void program_interrupt(CPUS390XState *env, uint32_t code, int ilc)
}
}
-static void ext_interrupt(CPUS390XState *env, int type, uint32_t param,
- uint64_t param64)
-{
- cpu_inject_ext(env, type, param, param64);
-}
-
/*
* ret < 0 indicates program check, ret = 0,1,2,3 -> cc
*/
@@ -2398,15 +2392,7 @@ int sclp_service_call(CPUS390XState *env, uint32_t sccb, uint64_t code)
stb_phys(sccb + SCP_INCREMENT, 1 << shift);
stw_phys(sccb + SCP_RESPONSE_CODE, 0x10);
- if (kvm_enabled()) {
-#ifdef CONFIG_KVM
- kvm_s390_interrupt_internal(env, KVM_S390_INT_SERVICE,
- sccb & ~3, 0, 1);
-#endif
- } else {
- env->psw.addr += 4;
- ext_interrupt(env, EXT_SERVICE, sccb & ~3, 0);
- }
+ s390_sclp_extint(sccb & ~3);
break;
default:
#ifdef DEBUG_HELPER
--
1.7.0.1
^ permalink raw reply related [flat|nested] 33+ messages in thread* [Qemu-devel] [PATCH 3/7] s390: sclp base support
2012-07-24 7:37 [Qemu-devel] [PATCH 0/7v3] s390: sclp patch set Christian Borntraeger
2012-07-24 7:37 ` [Qemu-devel] [PATCH 1/7] s390: Fix error handling and condition code of service call Christian Borntraeger
2012-07-24 7:37 ` [Qemu-devel] [PATCH 2/7] s390: provide interface for service interrupt/introduce interrupt.c Christian Borntraeger
@ 2012-07-24 7:37 ` Christian Borntraeger
2012-07-30 12:38 ` Alexander Graf
2012-08-03 15:23 ` Andreas Färber
2012-07-24 7:37 ` [Qemu-devel] [PATCH 4/7] s390: sclp event support Christian Borntraeger
` (4 subsequent siblings)
7 siblings, 2 replies; 33+ messages in thread
From: Christian Borntraeger @ 2012-07-24 7:37 UTC (permalink / raw)
To: Alexander Graf
Cc: Heinz Graalfs, qemu-devel, blauwirbel, Christian Borntraeger,
Jens Freimann, afaerber
From: Heinz Graalfs <graalfs@linux.vnet.ibm.com>
This adds a more generic infrastructure for handling Service-Call
requests on s390. Currently we only support a small subset of Read
SCP Info directly in target-s390x. This patch provides the base
infrastructure for supporting more commands and moves Read SCP
Info.
In the future we could add additional commands for hotplug, call
home and event handling.
Signed-off-by: Heinz Graalfs <graalfs@linux.vnet.ibm.com>
Signed-off-by: Christian Borntraeger <borntraeger@de.ibm.com>
---
hw/s390-virtio.c | 2 +
hw/s390x/Makefile.objs | 1 +
hw/s390x/sclp.c | 145 ++++++++++++++++++++++++++++++++++++++++++++++
hw/s390x/sclp.h | 79 +++++++++++++++++++++++++
target-s390x/cpu.h | 14 +----
target-s390x/kvm.c | 5 +-
target-s390x/op_helper.c | 31 ++--------
7 files changed, 235 insertions(+), 42 deletions(-)
create mode 100644 hw/s390x/sclp.c
create mode 100644 hw/s390x/sclp.h
diff --git a/hw/s390-virtio.c b/hw/s390-virtio.c
index 47eed35..28e320d 100644
--- a/hw/s390-virtio.c
+++ b/hw/s390-virtio.c
@@ -32,6 +32,7 @@
#include "exec-memory.h"
#include "hw/s390-virtio-bus.h"
+#include "hw/s390x/sclp.h"
//#define DEBUG_S390
@@ -183,6 +184,7 @@ static void s390_init(ram_addr_t my_ram_size,
/* get a BUS */
s390_bus = s390_virtio_bus_init(&my_ram_size);
+ s390_sclp_bus_init();
/* allocate RAM */
memory_region_init_ram(ram, "s390.ram", my_ram_size);
diff --git a/hw/s390x/Makefile.objs b/hw/s390x/Makefile.objs
index dcdcac8..1c14b96 100644
--- a/hw/s390x/Makefile.objs
+++ b/hw/s390x/Makefile.objs
@@ -1,3 +1,4 @@
obj-y = s390-virtio-bus.o s390-virtio.o
obj-y := $(addprefix ../,$(obj-y))
+obj-y += sclp.o
diff --git a/hw/s390x/sclp.c b/hw/s390x/sclp.c
new file mode 100644
index 0000000..4095ba6
--- /dev/null
+++ b/hw/s390x/sclp.c
@@ -0,0 +1,145 @@
+/*
+ * SCLP Support
+ *
+ * Copyright IBM, Corp. 2012
+ *
+ * Authors:
+ * Christian Borntraeger <borntraeger@de.ibm.com>
+ * Heinz Graalfs <graalfs@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 "cpu.h"
+#include "kvm.h"
+#include <hw/sysbus.h>
+#include "sclp.h"
+
+/* There is one SCLP bus per machine */
+static SCLPS390Bus *sbus;
+
+/* Provide information about the configuration, CPUs and storage */
+static int read_SCP_info(SCCB *sccb)
+{
+ ReadInfo *read_info = (ReadInfo *) sccb;
+ int shift = 0;
+
+ while ((ram_size >> (20 + shift)) > 65535) {
+ shift++;
+ }
+ read_info->rnmax = cpu_to_be16(ram_size >> (20 + shift));
+ read_info->rnsize = 1 << shift;
+ sccb->h.response_code = cpu_to_be16(SCLP_RC_NORMAL_READ_COMPLETION);
+
+ return 0;
+}
+
+static int sclp_execute(SCCB *sccb, uint64_t code)
+{
+ int r = 0;
+
+ switch (code) {
+ case SCLP_CMDW_READ_SCP_INFO:
+ case SCLP_CMDW_READ_SCP_INFO_FORCED:
+ r = read_SCP_info(sccb);
+ break;
+ default:
+ sccb->h.response_code = cpu_to_be16(SCLP_RC_INVALID_SCLP_COMMAND);
+ break;
+ }
+ return r;
+}
+
+int do_sclp_service_call(uint32_t sccb, uint64_t code)
+{
+ int r = 0;
+ SCCB work_sccb;
+
+ target_phys_addr_t sccb_len = sizeof(SCCB);
+
+ /*
+ * we want to work on a private copy of the sccb, to prevent guests
+ * from playing dirty tricks by modifying the memory content after
+ * the host has checked the values
+ */
+ cpu_physical_memory_read(sccb, &work_sccb, sccb_len);
+
+ /* Valid sccb sizes */
+ if (be16_to_cpu(work_sccb.h.length) < 8 ||
+ be16_to_cpu(work_sccb.h.length) > 4096) {
+ r = -PGM_SPECIFICATION;
+ goto out;
+ }
+
+ r = sclp_execute((SCCB *)&work_sccb, code);
+
+ cpu_physical_memory_write(sccb, &work_sccb,
+ be16_to_cpu(work_sccb.h.length));
+ if (!r) {
+ sclp_service_interrupt(sccb);
+ }
+
+out:
+ return r;
+}
+
+void sclp_service_interrupt(uint32_t sccb)
+{
+ if (!sccb) {
+ return;
+ }
+ s390_sclp_extint(sccb & ~3);
+}
+
+/* qemu object creation and initialization functions */
+
+static const TypeInfo s390_sclp_bus_info = {
+ .name = TYPE_S390_SCLP_BUS,
+ .parent = TYPE_BUS,
+ .instance_size = sizeof(SCLPS390Bus),
+ };
+
+SCLPS390Bus *s390_sclp_bus_init(void)
+{
+ BusState *bus_state;
+ DeviceState *dev;
+
+ dev = qdev_create(NULL, "s390-sclp-bridge");
+ qdev_init_nofail(dev);
+
+ bus_state = qbus_create(TYPE_S390_SCLP_BUS, dev, NULL);
+ bus_state->allow_hotplug = 0;
+
+ sbus = DO_UPCAST(SCLPS390Bus, bus, bus_state);
+ return sbus;
+}
+
+static int s390_sclp_bridge_init(SysBusDevice *dev)
+{
+ return 0;
+}
+
+static void s390_sclp_bridge_class_init(ObjectClass *klass, void *data)
+{
+ DeviceClass *dc = DEVICE_CLASS(klass);
+ SysBusDeviceClass *k = SYS_BUS_DEVICE_CLASS(klass);
+
+ k->init = s390_sclp_bridge_init;
+ dc->no_user = 1;
+}
+
+static TypeInfo s390_sclp_bridge_info = {
+ .name = "s390-sclp-bridge",
+ .parent = TYPE_SYS_BUS_DEVICE,
+ .instance_size = sizeof(SysBusDevice),
+ .class_init = s390_sclp_bridge_class_init,
+};
+
+static void s390_sclp_register_types(void)
+{
+ type_register_static(&s390_sclp_bridge_info);
+ type_register_static(&s390_sclp_bus_info);
+}
+type_init(s390_sclp_register_types)
diff --git a/hw/s390x/sclp.h b/hw/s390x/sclp.h
new file mode 100644
index 0000000..4d5a946
--- /dev/null
+++ b/hw/s390x/sclp.h
@@ -0,0 +1,79 @@
+/*
+ * SCLP Support
+ *
+ * Copyright IBM, Corp. 2012
+ *
+ * Authors:
+ * Christian Borntraeger <borntraeger@de.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_S390_SCLP_H
+#define HW_S390_SCLP_H
+
+#include <hw/qdev.h>
+
+/* SCLP command codes */
+#define SCLP_CMDW_READ_SCP_INFO 0x00020001
+#define SCLP_CMDW_READ_SCP_INFO_FORCED 0x00120001
+
+/* SCLP response codes */
+#define SCLP_RC_NORMAL_READ_COMPLETION 0x0010
+#define SCLP_RC_INVALID_SCLP_COMMAND 0x01f0
+
+/* Service Call Control Block (SCCB) and its elements */
+
+#define SCCB_SIZE 4096
+
+/*
+ * Normally packed structures are not the right thing to do, since all code
+ * must take care of endianess. We cant use ldl_phys and friends for two
+ * reasons, though:
+ * - some of the embedded structures below the SCCB can appear multiple times
+ * at different locations, so there is no fixed offset
+ * - we work on a private copy of the SCCB, since there are several length
+ * fields, that would cause a security nightmare if we allow the guest to
+ * alter the structure while we parse it. We cannot use ldl_p and friends
+ * either without doing pointer arithmetics
+ * So we have to double check that all users of sclp data structures use the
+ * right endianess wrappers.
+ */
+typedef struct SCCBHeader {
+ uint16_t length;
+ uint8_t function_code;
+ uint8_t control_mask[3];
+ uint16_t response_code;
+} QEMU_PACKED SCCBHeader;
+
+#define SCCB_DATA_LEN (SCCB_SIZE - sizeof(SCCBHeader))
+
+typedef struct ReadInfo {
+ SCCBHeader h;
+ uint16_t rnmax;
+ uint8_t rnsize;
+} QEMU_PACKED ReadInfo;
+
+typedef struct SCCB {
+ SCCBHeader h;
+ char data[SCCB_DATA_LEN];
+ } QEMU_PACKED SCCB;
+
+#define TYPE_S390_SCLP_BUS "s390-sclp-bus"
+#define S390_SCLP_BUS(obj) OBJECT_CHECK(SCLPS390Bus, (obj), TYPE_S390_SCLP_BUS)
+
+typedef struct S390SCLPDevice {
+ DeviceState qdev;
+} S390SCLPDevice;
+
+typedef struct SCLPS390Bus {
+ BusState bus;
+} SCLPS390Bus;
+
+SCLPS390Bus *s390_sclp_bus_init(void);
+
+void sclp_service_interrupt(uint32_t sccb);
+
+#endif
diff --git a/target-s390x/cpu.h b/target-s390x/cpu.h
index 18ac6e3..efd2cda 100644
--- a/target-s390x/cpu.h
+++ b/target-s390x/cpu.h
@@ -294,6 +294,7 @@ void s390x_tod_timer(void *opaque);
void s390x_cpu_timer(void *opaque);
int s390_virtio_hypercall(CPUS390XState *env, uint64_t mem, uint64_t hypercall);
+int do_sclp_service_call(uint32_t sccb, uint64_t code);
#ifdef CONFIG_KVM
void kvm_s390_interrupt(CPUS390XState *env, int type, uint32_t code);
@@ -596,17 +597,6 @@ static inline const char *cc_name(int cc_op)
return cc_names[cc_op];
}
-/* SCLP PV interface defines */
-#define SCLP_CMDW_READ_SCP_INFO 0x00020001
-#define SCLP_CMDW_READ_SCP_INFO_FORCED 0x00120001
-
-#define SCP_LENGTH 0x00
-#define SCP_FUNCTION_CODE 0x02
-#define SCP_CONTROL_MASK 0x03
-#define SCP_RESPONSE_CODE 0x06
-#define SCP_MEM_CODE 0x08
-#define SCP_INCREMENT 0x0a
-
typedef struct LowCore
{
/* prefix area: defined by architecture */
@@ -955,7 +945,7 @@ static inline void ebcdic_put(uint8_t *p, const char *ascii, int len)
void load_psw(CPUS390XState *env, uint64_t mask, uint64_t addr);
int mmu_translate(CPUS390XState *env, target_ulong vaddr, int rw, uint64_t asc,
target_ulong *raddr, int *flags);
-int sclp_service_call(CPUS390XState *env, uint32_t sccb, uint64_t code);
+int sclp_service_call(uint32_t sccb, uint64_t code);
uint32_t calc_cc(CPUS390XState *env, uint32_t cc_op, uint64_t src, uint64_t dst,
uint64_t vr);
diff --git a/target-s390x/kvm.c b/target-s390x/kvm.c
index 07edf93..bcb6b2e 100644
--- a/target-s390x/kvm.c
+++ b/target-s390x/kvm.c
@@ -60,9 +60,6 @@
#define SIGP_STORE_STATUS_ADDR 0x0e
#define SIGP_SET_ARCH 0x12
-#define SCLP_CMDW_READ_SCP_INFO 0x00020001
-#define SCLP_CMDW_READ_SCP_INFO_FORCED 0x00120001
-
const KVMCapabilityInfo kvm_arch_required_capabilities[] = {
KVM_CAP_LAST_INFO
};
@@ -272,7 +269,7 @@ static int kvm_sclp_service_call(CPUS390XState *env, struct kvm_run *run,
sccb = env->regs[ipbh0 & 0xf];
code = env->regs[(ipbh0 & 0xf0) >> 4];
- r = sclp_service_call(env, sccb, code);
+ r = sclp_service_call(sccb, code);
if (r < 0) {
enter_pgmcheck(env, -r);
}
diff --git a/target-s390x/op_helper.c b/target-s390x/op_helper.c
index abc35dd..e7bb980 100644
--- a/target-s390x/op_helper.c
+++ b/target-s390x/op_helper.c
@@ -2363,13 +2363,11 @@ static void program_interrupt(CPUS390XState *env, uint32_t code, int ilc)
}
/*
+ * we handle here the part that belongs to the cpu, e.g. program checks
* ret < 0 indicates program check, ret = 0,1,2,3 -> cc
*/
-int sclp_service_call(CPUS390XState *env, uint32_t sccb, uint64_t code)
+int sclp_service_call(uint32_t sccb, uint64_t code)
{
- int r = 0;
- int shift = 0;
-
#ifdef DEBUG_HELPER
printf("sclp(0x%x, 0x%" PRIx64 ")\n", sccb, code);
#endif
@@ -2382,27 +2380,8 @@ int sclp_service_call(CPUS390XState *env, uint32_t sccb, uint64_t code)
return -PGM_SPECIFICATION;
}
- switch(code) {
- case SCLP_CMDW_READ_SCP_INFO:
- case SCLP_CMDW_READ_SCP_INFO_FORCED:
- while ((ram_size >> (20 + shift)) > 65535) {
- shift++;
- }
- stw_phys(sccb + SCP_MEM_CODE, ram_size >> (20 + shift));
- stb_phys(sccb + SCP_INCREMENT, 1 << shift);
- stw_phys(sccb + SCP_RESPONSE_CODE, 0x10);
-
- s390_sclp_extint(sccb & ~3);
- break;
- default:
-#ifdef DEBUG_HELPER
- printf("KVM: invalid sclp call 0x%x / 0x%" PRIx64 "x\n", sccb, code);
-#endif
- r = 3;
- break;
- }
-
- return r;
+ /* the complex part is handled by external components */
+ return do_sclp_service_call(sccb, code);
}
/* SCLP service call */
@@ -2410,7 +2389,7 @@ uint32_t HELPER(servc)(uint32_t r1, uint64_t r2)
{
int r;
- r = sclp_service_call(env, r1, r2);
+ r = sclp_service_call(r1, r2);
if (r < 0) {
program_interrupt(env, -r, 4);
return 0;
--
1.7.0.1
^ permalink raw reply related [flat|nested] 33+ messages in thread* Re: [Qemu-devel] [PATCH 3/7] s390: sclp base support
2012-07-24 7:37 ` [Qemu-devel] [PATCH 3/7] s390: sclp base support Christian Borntraeger
@ 2012-07-30 12:38 ` Alexander Graf
2012-07-30 14:34 ` Christian Borntraeger
2012-08-20 12:15 ` Heinz Graalfs
2012-08-03 15:23 ` Andreas Färber
1 sibling, 2 replies; 33+ messages in thread
From: Alexander Graf @ 2012-07-30 12:38 UTC (permalink / raw)
To: Christian Borntraeger
Cc: qemu-devel, blauwirbel, Heinz Graalfs, Jens Freimann, afaerber
On 24.07.2012, at 09:37, Christian Borntraeger wrote:
> From: Heinz Graalfs <graalfs@linux.vnet.ibm.com>
>
> This adds a more generic infrastructure for handling Service-Call
> requests on s390. Currently we only support a small subset of Read
> SCP Info directly in target-s390x. This patch provides the base
> infrastructure for supporting more commands and moves Read SCP
> Info.
> In the future we could add additional commands for hotplug, call
> home and event handling.
>
> Signed-off-by: Heinz Graalfs <graalfs@linux.vnet.ibm.com>
> Signed-off-by: Christian Borntraeger <borntraeger@de.ibm.com>
> ---
> hw/s390-virtio.c | 2 +
> hw/s390x/Makefile.objs | 1 +
> hw/s390x/sclp.c | 145 ++++++++++++++++++++++++++++++++++++++++++++++
> hw/s390x/sclp.h | 79 +++++++++++++++++++++++++
> target-s390x/cpu.h | 14 +----
> target-s390x/kvm.c | 5 +-
> target-s390x/op_helper.c | 31 ++--------
> 7 files changed, 235 insertions(+), 42 deletions(-)
> create mode 100644 hw/s390x/sclp.c
> create mode 100644 hw/s390x/sclp.h
>
> diff --git a/hw/s390-virtio.c b/hw/s390-virtio.c
> index 47eed35..28e320d 100644
> --- a/hw/s390-virtio.c
> +++ b/hw/s390-virtio.c
> @@ -32,6 +32,7 @@
> #include "exec-memory.h"
>
> #include "hw/s390-virtio-bus.h"
> +#include "hw/s390x/sclp.h"
>
> //#define DEBUG_S390
>
> @@ -183,6 +184,7 @@ static void s390_init(ram_addr_t my_ram_size,
>
> /* get a BUS */
> s390_bus = s390_virtio_bus_init(&my_ram_size);
> + s390_sclp_bus_init();
>
> /* allocate RAM */
> memory_region_init_ram(ram, "s390.ram", my_ram_size);
> diff --git a/hw/s390x/Makefile.objs b/hw/s390x/Makefile.objs
> index dcdcac8..1c14b96 100644
> --- a/hw/s390x/Makefile.objs
> +++ b/hw/s390x/Makefile.objs
> @@ -1,3 +1,4 @@
> obj-y = s390-virtio-bus.o s390-virtio.o
>
> obj-y := $(addprefix ../,$(obj-y))
> +obj-y += sclp.o
> diff --git a/hw/s390x/sclp.c b/hw/s390x/sclp.c
> new file mode 100644
> index 0000000..4095ba6
> --- /dev/null
> +++ b/hw/s390x/sclp.c
> @@ -0,0 +1,145 @@
> +/*
> + * SCLP Support
> + *
> + * Copyright IBM, Corp. 2012
> + *
> + * Authors:
> + * Christian Borntraeger <borntraeger@de.ibm.com>
> + * Heinz Graalfs <graalfs@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 "cpu.h"
> +#include "kvm.h"
> +#include <hw/sysbus.h>
> +#include "sclp.h"
> +
> +/* There is one SCLP bus per machine */
> +static SCLPS390Bus *sbus;
... but there isn't necessarily one machine per qemu instance. Today there is, but we shouldn't rely on that fact. Please move the bus variable into a machine struct that only the machine knows about.
> +
> +/* Provide information about the configuration, CPUs and storage */
> +static int read_SCP_info(SCCB *sccb)
> +{
> + ReadInfo *read_info = (ReadInfo *) sccb;
> + int shift = 0;
> +
> + while ((ram_size >> (20 + shift)) > 65535) {
> + shift++;
> + }
> + read_info->rnmax = cpu_to_be16(ram_size >> (20 + shift));
> + read_info->rnsize = 1 << shift;
> + sccb->h.response_code = cpu_to_be16(SCLP_RC_NORMAL_READ_COMPLETION);
> +
> + return 0;
> +}
> +
> +static int sclp_execute(SCCB *sccb, uint64_t code)
> +{
> + int r = 0;
> +
> + switch (code) {
> + case SCLP_CMDW_READ_SCP_INFO:
> + case SCLP_CMDW_READ_SCP_INFO_FORCED:
> + r = read_SCP_info(sccb);
> + break;
> + default:
> + sccb->h.response_code = cpu_to_be16(SCLP_RC_INVALID_SCLP_COMMAND);
> + break;
> + }
> + return r;
> +}
> +
> +int do_sclp_service_call(uint32_t sccb, uint64_t code)
> +{
> + int r = 0;
> + SCCB work_sccb;
> +
> + target_phys_addr_t sccb_len = sizeof(SCCB);
> +
> + /*
> + * we want to work on a private copy of the sccb, to prevent guests
> + * from playing dirty tricks by modifying the memory content after
> + * the host has checked the values
> + */
> + cpu_physical_memory_read(sccb, &work_sccb, sccb_len);
> +
> + /* Valid sccb sizes */
> + if (be16_to_cpu(work_sccb.h.length) < 8 ||
> + be16_to_cpu(work_sccb.h.length) > 4096) {
> + r = -PGM_SPECIFICATION;
> + goto out;
> + }
> +
> + r = sclp_execute((SCCB *)&work_sccb, code);
> +
> + cpu_physical_memory_write(sccb, &work_sccb,
> + be16_to_cpu(work_sccb.h.length));
> + if (!r) {
> + sclp_service_interrupt(sccb);
> + }
> +
> +out:
> + return r;
> +}
> +
> +void sclp_service_interrupt(uint32_t sccb)
Does this have to be globally visible? If all the sclp source ends up in this file, it should only be necessary internal to it, right?
> +{
> + if (!sccb) {
Please add a comment when this would trigger. In fact, I'm not sure I fully understand the reason for this function :).
Alex
^ permalink raw reply [flat|nested] 33+ messages in thread* Re: [Qemu-devel] [PATCH 3/7] s390: sclp base support
2012-07-30 12:38 ` Alexander Graf
@ 2012-07-30 14:34 ` Christian Borntraeger
2012-08-20 12:15 ` Heinz Graalfs
1 sibling, 0 replies; 33+ messages in thread
From: Christian Borntraeger @ 2012-07-30 14:34 UTC (permalink / raw)
To: Alexander Graf
Cc: qemu-devel, blauwirbel, Heinz Graalfs, Jens Freimann, afaerber
On 30/07/12 14:38, Alexander Graf wrote:
>> +/* There is one SCLP bus per machine */
>> +static SCLPS390Bus *sbus;
>
> ... but there isn't necessarily one machine per qemu instance. Today there is,
> but we shouldn't rely on that fact. Please move the bus variable into a machine
> struct that only the machine knows about.
Could do. However, we have the same issue with the virtio bus. Would it make sense to leave it
here as is and the do a cleanup later on?
[...]
>> +void sclp_service_interrupt(uint32_t sccb)
>
> Does this have to be globally visible? If all the sclp source ends up in this file, it should only be necessary internal to it, right?
It will be called by the console and quiesce code
>
>> +{
>> + if (!sccb) {
>
> Please add a comment when this would trigger. In fact, I'm not sure I fully understand the reason for this function :).
It will be enhanced by patch 4. Will move that hunk completely there. See also the explanation for patch 4.
^ permalink raw reply [flat|nested] 33+ messages in thread* Re: [Qemu-devel] [PATCH 3/7] s390: sclp base support
2012-07-30 12:38 ` Alexander Graf
2012-07-30 14:34 ` Christian Borntraeger
@ 2012-08-20 12:15 ` Heinz Graalfs
1 sibling, 0 replies; 33+ messages in thread
From: Heinz Graalfs @ 2012-08-20 12:15 UTC (permalink / raw)
To: Alexander Graf
Cc: blauwirbel, Christian Borntraeger, Jens Freimann, afaerber, qemu-devel
Hi Alex,
there was one leftover ...
On Mon, 2012-07-30 at 14:38 +0200, Alexander Graf wrote:
> On 24.07.2012, at 09:37, Christian Borntraeger wrote:
>
> > From: Heinz Graalfs <graalfs@linux.vnet.ibm.com>
> >
> > This adds a more generic infrastructure for handling Service-Call
> > requests on s390. Currently we only support a small subset of Read
> > SCP Info directly in target-s390x. This patch provides the base
> > infrastructure for supporting more commands and moves Read SCP
> > Info.
> > In the future we could add additional commands for hotplug, call
> > home and event handling.
> >
> > Signed-off-by: Heinz Graalfs <graalfs@linux.vnet.ibm.com>
> > Signed-off-by: Christian Borntraeger <borntraeger@de.ibm.com>
> > ---
> > hw/s390-virtio.c | 2 +
> > hw/s390x/Makefile.objs | 1 +
> > hw/s390x/sclp.c | 145 ++++++++++++++++++++++++++++++++++++++++++++++
> > hw/s390x/sclp.h | 79 +++++++++++++++++++++++++
> > target-s390x/cpu.h | 14 +----
> > target-s390x/kvm.c | 5 +-
> > target-s390x/op_helper.c | 31 ++--------
> > 7 files changed, 235 insertions(+), 42 deletions(-)
> > create mode 100644 hw/s390x/sclp.c
> > create mode 100644 hw/s390x/sclp.h
> >
> > diff --git a/hw/s390-virtio.c b/hw/s390-virtio.c
> > index 47eed35..28e320d 100644
> > --- a/hw/s390-virtio.c
> > +++ b/hw/s390-virtio.c
> > @@ -32,6 +32,7 @@
> > #include "exec-memory.h"
> >
> > #include "hw/s390-virtio-bus.h"
> > +#include "hw/s390x/sclp.h"
> >
> > //#define DEBUG_S390
> >
> > @@ -183,6 +184,7 @@ static void s390_init(ram_addr_t my_ram_size,
> >
> > /* get a BUS */
> > s390_bus = s390_virtio_bus_init(&my_ram_size);
> > + s390_sclp_bus_init();
> >
> > /* allocate RAM */
> > memory_region_init_ram(ram, "s390.ram", my_ram_size);
> > diff --git a/hw/s390x/Makefile.objs b/hw/s390x/Makefile.objs
> > index dcdcac8..1c14b96 100644
> > --- a/hw/s390x/Makefile.objs
> > +++ b/hw/s390x/Makefile.objs
> > @@ -1,3 +1,4 @@
> > obj-y = s390-virtio-bus.o s390-virtio.o
> >
> > obj-y := $(addprefix ../,$(obj-y))
> > +obj-y += sclp.o
> > diff --git a/hw/s390x/sclp.c b/hw/s390x/sclp.c
> > new file mode 100644
> > index 0000000..4095ba6
> > --- /dev/null
> > +++ b/hw/s390x/sclp.c
> > @@ -0,0 +1,145 @@
> > +/*
> > + * SCLP Support
> > + *
> > + * Copyright IBM, Corp. 2012
> > + *
> > + * Authors:
> > + * Christian Borntraeger <borntraeger@de.ibm.com>
> > + * Heinz Graalfs <graalfs@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 "cpu.h"
> > +#include "kvm.h"
> > +#include <hw/sysbus.h>
> > +#include "sclp.h"
> > +
> > +/* There is one SCLP bus per machine */
> > +static SCLPS390Bus *sbus;
>
> ... but there isn't necessarily one machine per qemu instance. Today there is, but we shouldn't rely on that fact. Please move the bus variable into a machine struct that only the machine knows about.
>
we removed the complete sclp bus now and keep the event_facility pointer
within the current machines global property-list as opaque pointer
> > +
> > +/* Provide information about the configuration, CPUs and storage */
> > +static int read_SCP_info(SCCB *sccb)
> > +{
> > + ReadInfo *read_info = (ReadInfo *) sccb;
> > + int shift = 0;
> > +
> > + while ((ram_size >> (20 + shift)) > 65535) {
> > + shift++;
> > + }
> > + read_info->rnmax = cpu_to_be16(ram_size >> (20 + shift));
> > + read_info->rnsize = 1 << shift;
> > + sccb->h.response_code = cpu_to_be16(SCLP_RC_NORMAL_READ_COMPLETION);
> > +
> > + return 0;
> > +}
> > +
> > +static int sclp_execute(SCCB *sccb, uint64_t code)
> > +{
> > + int r = 0;
> > +
> > + switch (code) {
> > + case SCLP_CMDW_READ_SCP_INFO:
> > + case SCLP_CMDW_READ_SCP_INFO_FORCED:
> > + r = read_SCP_info(sccb);
> > + break;
> > + default:
> > + sccb->h.response_code = cpu_to_be16(SCLP_RC_INVALID_SCLP_COMMAND);
> > + break;
> > + }
> > + return r;
> > +}
> > +
> > +int do_sclp_service_call(uint32_t sccb, uint64_t code)
> > +{
> > + int r = 0;
> > + SCCB work_sccb;
> > +
> > + target_phys_addr_t sccb_len = sizeof(SCCB);
> > +
> > + /*
> > + * we want to work on a private copy of the sccb, to prevent guests
> > + * from playing dirty tricks by modifying the memory content after
> > + * the host has checked the values
> > + */
> > + cpu_physical_memory_read(sccb, &work_sccb, sccb_len);
> > +
> > + /* Valid sccb sizes */
> > + if (be16_to_cpu(work_sccb.h.length) < 8 ||
> > + be16_to_cpu(work_sccb.h.length) > 4096) {
> > + r = -PGM_SPECIFICATION;
> > + goto out;
> > + }
> > +
> > + r = sclp_execute((SCCB *)&work_sccb, code);
> > +
> > + cpu_physical_memory_write(sccb, &work_sccb,
> > + be16_to_cpu(work_sccb.h.length));
> > + if (!r) {
> > + sclp_service_interrupt(sccb);
> > + }
> > +
> > +out:
> > + return r;
> > +}
> > +
> > +void sclp_service_interrupt(uint32_t sccb)
>
> Does this have to be globally visible? If all the sclp source ends up in this file, it should only be necessary internal to it, right?
>
> > +{
> > + if (!sccb) {
>
> Please add a comment when this would trigger. In fact, I'm not sure I fully understand the reason for this function :).
>
>
> Alex
>
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [Qemu-devel] [PATCH 3/7] s390: sclp base support
2012-07-24 7:37 ` [Qemu-devel] [PATCH 3/7] s390: sclp base support Christian Borntraeger
2012-07-30 12:38 ` Alexander Graf
@ 2012-08-03 15:23 ` Andreas Färber
1 sibling, 0 replies; 33+ messages in thread
From: Andreas Färber @ 2012-08-03 15:23 UTC (permalink / raw)
To: Christian Borntraeger
Cc: blauwirbel, Heinz Graalfs, Alexander Graf, Jens Freimann, qemu-devel
Am 24.07.2012 09:37, schrieb Christian Borntraeger:
> diff --git a/hw/s390x/sclp.c b/hw/s390x/sclp.c
> new file mode 100644
> index 0000000..4095ba6
> --- /dev/null
> +++ b/hw/s390x/sclp.c
[...]
> +
> +static TypeInfo s390_sclp_bridge_info = {
Two minor comments:
static const please.
> + .name = "s390-sclp-bridge",
> + .parent = TYPE_SYS_BUS_DEVICE,
> + .instance_size = sizeof(SysBusDevice),
> + .class_init = s390_sclp_bridge_class_init,
> +};
> +
> +static void s390_sclp_register_types(void)
> +{
> + type_register_static(&s390_sclp_bridge_info);
> + type_register_static(&s390_sclp_bus_info);
> +}
> +type_init(s390_sclp_register_types)
Please insert a white line between the function and type_init().
Both apply to virtually all following patches as well.
Andreas
--
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg
^ permalink raw reply [flat|nested] 33+ messages in thread
* [Qemu-devel] [PATCH 4/7] s390: sclp event support
2012-07-24 7:37 [Qemu-devel] [PATCH 0/7v3] s390: sclp patch set Christian Borntraeger
` (2 preceding siblings ...)
2012-07-24 7:37 ` [Qemu-devel] [PATCH 3/7] s390: sclp base support Christian Borntraeger
@ 2012-07-24 7:37 ` Christian Borntraeger
2012-07-30 13:24 ` Alexander Graf
2012-07-31 12:59 ` Andreas Färber
2012-07-24 7:37 ` [Qemu-devel] [PATCH 5/7] s390: sclp signal quiesce support Christian Borntraeger
` (3 subsequent siblings)
7 siblings, 2 replies; 33+ messages in thread
From: Christian Borntraeger @ 2012-07-24 7:37 UTC (permalink / raw)
To: Alexander Graf
Cc: Heinz Graalfs, qemu-devel, blauwirbel, Christian Borntraeger,
Jens Freimann, afaerber
From: Heinz Graalfs <graalfs@linux.vnet.ibm.com>
Several SCLP features are considered to be events. Those events don't
provide SCLP commands on their own, instead they are all based on
Read Event Data, Write Event Data, Write Event Mask and the service
interrupt. Follow-on patches will provide SCLP's Signal Quiesce (via
system_powerdown) and the ASCII console.
Further down the road the sclp line mode console and configuration
change events (e.g. cpu hotplug) can be implemented.
Signed-off-by: Heinz Graalfs <graalfs@linux.vnet.ibm.com>
Signed-off-by: Christian Borntraeger <borntraeger@de.ibm.com>
---
hw/s390x/Makefile.objs | 1 +
hw/s390x/event-facility.c | 390 +++++++++++++++++++++++++++++++++++++++++++++
hw/s390x/event-facility.h | 107 ++++++++++++
hw/s390x/sclp.c | 48 +++++-
hw/s390x/sclp.h | 44 +++++
5 files changed, 587 insertions(+), 3 deletions(-)
create mode 100644 hw/s390x/event-facility.c
create mode 100644 hw/s390x/event-facility.h
diff --git a/hw/s390x/Makefile.objs b/hw/s390x/Makefile.objs
index 1c14b96..b32fc52 100644
--- a/hw/s390x/Makefile.objs
+++ b/hw/s390x/Makefile.objs
@@ -2,3 +2,4 @@ obj-y = s390-virtio-bus.o s390-virtio.o
obj-y := $(addprefix ../,$(obj-y))
obj-y += sclp.o
+obj-y += event-facility.o
diff --git a/hw/s390x/event-facility.c b/hw/s390x/event-facility.c
new file mode 100644
index 0000000..74a3514
--- /dev/null
+++ b/hw/s390x/event-facility.c
@@ -0,0 +1,390 @@
+/*
+ * SCLP
+ * Event Facility
+ * handles SCLP event types
+ * - Signal Quiesce - system power down
+ * - ASCII Console Data - VT220 read and write
+ *
+ * Copyright IBM, Corp. 2012
+ *
+ * Authors:
+ * Heinz Graalfs <graalfs@de.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 "monitor.h"
+#include "sysemu.h"
+
+#include "sclp.h"
+#include "event-facility.h"
+
+typedef struct EventTypes {
+ BusState qbus;
+ SCLPEventFacility *event_facility;
+} EventTypes;
+
+struct SCLPEventFacility {
+ EventTypes sbus;
+ DeviceState *qdev;
+ /* guest' receive mask */
+ unsigned int receive_mask;
+};
+
+/* return true if any child has event pending set */
+static bool event_pending(SCLPEventFacility *ef)
+{
+ BusChild *kid;
+ SCLPEvent *event;
+
+ QTAILQ_FOREACH(kid, &ef->sbus.qbus.children, sibling) {
+ DeviceState *qdev = kid->child;
+ event = DO_UPCAST(SCLPEvent, qdev, qdev);
+ lock(event);
+ if (event->event_pending) {
+ unlock(event);
+ return true;
+ }
+ unlock(event);
+ }
+ return false;
+}
+
+static unsigned int get_host_send_mask(SCLPEventFacility *ef)
+{
+ unsigned int mask;
+ BusChild *kid;
+ SCLPEventClass *child;
+
+ mask = 0;
+
+ QTAILQ_FOREACH(kid, &ef->sbus.qbus.children, sibling) {
+ DeviceState *qdev = kid->child;
+ child = SCLP_EVENT_GET_CLASS((SCLPEvent *) qdev);
+ mask |= child->get_send_mask();
+ }
+ return mask;
+}
+
+static unsigned int get_host_receive_mask(SCLPEventFacility *ef)
+{
+ unsigned int mask;
+ BusChild *kid;
+ SCLPEventClass *child;
+
+ mask = 0;
+
+ QTAILQ_FOREACH(kid, &ef->sbus.qbus.children, sibling) {
+ DeviceState *qdev = kid->child;
+ child = SCLP_EVENT_GET_CLASS((SCLPEvent *) qdev);
+ mask |= child->get_receive_mask();
+ }
+ return mask;
+}
+
+static bool sccb_events_ok(SCCB *sccb)
+{
+ int slen;
+ unsigned elen = 0;
+ EventBufferHeader *event;
+ WriteEventData *wed = (WriteEventData *) sccb;
+
+ event = (EventBufferHeader *) &wed->ebh;
+ for (slen = be16_to_cpu(sccb->h.length) - sizeof(sccb->h);
+ slen > 0; slen -= elen) {
+ elen = be16_to_cpu(event->length);
+ if (elen < sizeof(*event) || elen > slen) {
+ sccb->h.response_code =
+ cpu_to_be16(SCLP_RC_EVENT_BUFFER_SYNTAX_ERROR);
+ return false;
+ }
+ event = (void *) event + elen;
+ }
+ if (slen) {
+ sccb->h.response_code = cpu_to_be16(SCLP_RC_INCONSISTENT_LENGTHS);
+ return false;
+ }
+ return true;
+}
+
+static void handle_sccb_write_events(SCLPEventFacility *ef, SCCB *sccb)
+{
+ int slen;
+ unsigned elen = 0;
+ EventBufferHeader *event_buf;
+ BusChild *kid;
+ SCLPEvent *event;
+ SCLPEventClass *ec;
+
+ WriteEventData *wed = (WriteEventData *) sccb;
+
+ event_buf = &wed->ebh;
+
+ /* loop over all contained event buffers */
+ for (slen = be16_to_cpu(sccb->h.length) - sizeof(sccb->h);
+ slen > 0; slen -= elen) {
+ elen = be16_to_cpu(event_buf->length);
+
+ /* in case of a previous error mark all trailing buffers
+ * as not accepted */
+ if (sccb->h.response_code !=
+ cpu_to_be16(SCLP_RC_NORMAL_COMPLETION)) {
+ event_buf->flags &= ~(SCLP_EVENT_BUFFER_ACCEPTED);
+ } else {
+ sccb->h.response_code = cpu_to_be16(SCLP_RC_INVALID_FUNCTION);
+ QTAILQ_FOREACH(kid, &ef->sbus.qbus.children, sibling) {
+ DeviceState *qdev = kid->child;
+ event = (SCLPEvent *) qdev;
+ ec = SCLP_EVENT_GET_CLASS(event);
+
+ if (ec->write_event_data &&
+ ec->event_type() == event_buf->type) {
+ sccb->h.response_code = cpu_to_be16(
+ ec->write_event_data(event, event_buf));
+ break;
+ }
+ }
+ }
+ event_buf = (void *) event_buf + elen;
+ }
+}
+
+static int write_event_data(SCLPEventFacility *ef, SCCB *sccb)
+{
+ if (sccb->h.function_code != SCLP_FC_NORMAL_WRITE) {
+ sccb->h.response_code = cpu_to_be16(SCLP_RC_INVALID_FUNCTION);
+ goto out;
+ }
+ if (be16_to_cpu(sccb->h.length) < 8) {
+ sccb->h.response_code = cpu_to_be16(SCLP_RC_INSUFFICIENT_SCCB_LENGTH);
+ goto out;
+ }
+ /* first check the sum of all events */
+ if (!sccb_events_ok(sccb)) {
+ goto out;
+ }
+ /* then execute */
+ sccb->h.response_code = cpu_to_be16(SCLP_RC_NORMAL_COMPLETION);
+ handle_sccb_write_events(ef, sccb);
+
+out:
+ return 0;
+}
+
+static void handle_sccb_read_events(SCLPEventFacility *ef, SCCB *sccb,
+ unsigned int mask)
+{
+ int slen;
+ unsigned elen = 0;
+ BusChild *kid;
+ SCLPEvent *event;
+ SCLPEventClass *ec;
+ EventBufferHeader *event_buf;
+ ReadEventData *red = (ReadEventData *) sccb;
+
+ event_buf = &red->ebh;
+ event_buf->length = 0;
+ slen = sizeof(sccb->data);
+ QTAILQ_FOREACH(kid, &ef->sbus.qbus.children, sibling) {
+ DeviceState *qdev = kid->child;
+ event = (SCLPEvent *) qdev;
+ ec = SCLP_EVENT_GET_CLASS(event);
+
+ if (mask & ec->get_send_mask()) {
+ if (ec->read_event_data(event, event_buf, &slen)) {
+ sccb->h.response_code = cpu_to_be16(SCLP_RC_NORMAL_COMPLETION);
+ }
+ }
+ elen = be16_to_cpu(event_buf->length);
+ event_buf = (void *) event_buf + elen;
+ }
+
+ if (sccb->h.control_mask[2] & SCLP_VARIABLE_LENGTH_RESPONSE) {
+ sccb->h.control_mask[2] &= ~SCLP_VARIABLE_LENGTH_RESPONSE;
+ sccb->h.length = cpu_to_be16(SCCB_SIZE - slen);
+ }
+}
+
+static int read_event_data(SCLPEventFacility *ef, SCCB *sccb)
+{
+ unsigned int sclp_active_selection_mask;
+ unsigned int sclp_cp_receive_mask;
+
+ ReadEventData *red = (ReadEventData *) sccb;
+
+ if (be16_to_cpu(sccb->h.length) != SCCB_SIZE) {
+ sccb->h.response_code = cpu_to_be16(SCLP_RC_INSUFFICIENT_SCCB_LENGTH);
+ goto out;
+ }
+
+ sclp_cp_receive_mask = ef->receive_mask;
+
+ /* get active selection mask */
+ switch (sccb->h.function_code) {
+ case SCLP_UNCONDITIONAL_READ:
+ sclp_active_selection_mask = sclp_cp_receive_mask;
+ break;
+ case SCLP_SELECTIVE_READ:
+ if (!(sclp_cp_receive_mask & be32_to_cpu(red->mask))) {
+ sccb->h.response_code =
+ cpu_to_be16(SCLP_RC_INVALID_SELECTION_MASK);
+ goto out;
+ }
+ sclp_active_selection_mask = be32_to_cpu(red->mask);
+ break;
+ default:
+ sccb->h.response_code = cpu_to_be16(SCLP_RC_INVALID_FUNCTION);
+ goto out;
+ }
+
+ sccb->h.response_code = cpu_to_be16(SCLP_RC_NO_EVENT_BUFFERS_STORED);
+ handle_sccb_read_events(ef, sccb, sclp_active_selection_mask);
+
+out:
+ return 0;
+}
+
+static int write_event_mask(SCLPEventFacility *ef, SCCB *sccb)
+{
+ WriteEventMask *we_mask = (WriteEventMask *) sccb;
+
+ /* Attention: We assume that Linux uses 4-byte masks, what it actually
+ does. Architecture allows for masks of variable size, though */
+ if (be16_to_cpu(we_mask->mask_length) != 4) {
+ sccb->h.response_code = cpu_to_be16(SCLP_RC_INVALID_MASK_LENGTH);
+ goto out;
+ }
+
+ /* keep track of the guest's capability masks */
+ ef->receive_mask = be32_to_cpu(we_mask->cp_receive_mask);
+
+ /* return the SCLP's capability masks to the guest */
+ we_mask->send_mask = cpu_to_be32(get_host_send_mask(ef));
+ we_mask->receive_mask = cpu_to_be32(get_host_receive_mask(ef));
+
+ sccb->h.response_code = cpu_to_be16(SCLP_RC_NORMAL_COMPLETION);
+
+out:
+ return 0;
+}
+
+/* qemu object creation and initialization functions */
+
+#define TYPE_SCLP_EVENTS_BUS "s390-sclp-events-bus"
+#define SCLP_EVENTS_BUS(obj) OBJECT_CHECK(SCLPS390Bus, (obj),\
+ TYPE_SCLP_EVENTS_BUS)
+
+static void sclp_events_bus_class_init(ObjectClass *klass, void *data)
+{
+}
+
+static const TypeInfo s390_sclp_events_bus_info = {
+ .name = TYPE_SCLP_EVENTS_BUS,
+ .parent = TYPE_BUS,
+ .instance_size = sizeof(SCLPS390Bus),
+ .class_init = sclp_events_bus_class_init,
+};
+
+static int command_handler(SCLPEventFacility *ef, SCCB *sccb, uint64_t code)
+{
+ int r = 0;
+
+ switch (code) {
+ case SCLP_CMD_READ_EVENT_DATA:
+ r = read_event_data(ef, sccb);
+ break;
+ case SCLP_CMD_WRITE_EVENT_DATA:
+ r = write_event_data(ef, sccb);
+ break;
+ case SCLP_CMD_WRITE_EVENT_MASK:
+ r = write_event_mask(ef, sccb);
+ break;
+ default:
+#ifdef DEBUG_HELPER
+ printf("KVM: unhandled sclp code 0x%" PRIx64 "x\n", code);
+#endif
+ sccb->h.response_code = cpu_to_be16(SCLP_RC_INVALID_SCLP_COMMAND);
+ break;
+ }
+ return r;
+}
+
+static int init_event_facility(S390SCLPDevice *sdev)
+{
+ SCLPEventFacility *event_facility;
+
+ event_facility = g_malloc0(sizeof(SCLPEventFacility));
+ sdev->instance = event_facility;
+ sdev->sclp_command_handler = command_handler;
+ sdev->event_pending = event_pending;
+
+ /* Spawn a new sclp-events facility */
+ qbus_create_inplace(&event_facility->sbus.qbus,
+ TYPE_SCLP_EVENTS_BUS, (DeviceState *)sdev, NULL);
+ event_facility->sbus.qbus.allow_hotplug = 0;
+ event_facility->sbus.event_facility = event_facility;
+ event_facility->qdev = (DeviceState *) sdev;
+
+ return 0;
+}
+
+static void init_event_facility_class(ObjectClass *klass, void *data)
+{
+ S390SCLPDeviceClass *k = SCLP_S390_DEVICE_CLASS(klass);
+
+ k->init = init_event_facility;
+}
+
+static TypeInfo s390_sclp_event_facility_info = {
+ .name = "s390-sclp-event-facility",
+ .parent = TYPE_DEVICE_S390_SCLP,
+ .instance_size = sizeof(S390SCLPDevice),
+ .class_init = init_event_facility_class,
+};
+
+static int event_qdev_init(DeviceState *qdev)
+{
+ SCLPEvent *event = DO_UPCAST(SCLPEvent, qdev, qdev);
+ SCLPEventClass *child = SCLP_EVENT_GET_CLASS(event);
+
+ return child->init(event);
+}
+
+static int event_qdev_exit(DeviceState *qdev)
+{
+ SCLPEvent *event = DO_UPCAST(SCLPEvent, qdev, qdev);
+ SCLPEventClass *child = SCLP_EVENT_GET_CLASS(event);
+ if (child->exit) {
+ child->exit(event);
+ }
+ return 0;
+}
+
+static void event_class_init(ObjectClass *klass, void *data)
+{
+ DeviceClass *dc = DEVICE_CLASS(klass);
+
+ dc->bus_type = TYPE_SCLP_EVENTS_BUS;
+ dc->unplug = qdev_simple_unplug_cb;
+ dc->init = event_qdev_init;
+ dc->exit = event_qdev_exit;
+}
+
+static TypeInfo s390_sclp_event_type_info = {
+ .name = TYPE_SCLP_EVENT,
+ .parent = TYPE_DEVICE,
+ .instance_size = sizeof(SCLPEvent),
+ .class_init = event_class_init,
+ .class_size = sizeof(SCLPEventClass),
+ .abstract = true,
+};
+
+static void register_types(void)
+{
+ type_register_static(&s390_sclp_events_bus_info);
+ type_register_static(&s390_sclp_event_facility_info);
+ type_register_static(&s390_sclp_event_type_info);
+}
+type_init(register_types)
diff --git a/hw/s390x/event-facility.h b/hw/s390x/event-facility.h
new file mode 100644
index 0000000..1e022a3
--- /dev/null
+++ b/hw/s390x/event-facility.h
@@ -0,0 +1,107 @@
+/*
+ * SCLP
+ * Event Facility definitions
+ *
+ * Copyright IBM, Corp. 2012
+ *
+ * Authors:
+ * Heinz Graalfs <graalfs@de.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_S390_SCLP_EVENT_FACILITY_H
+#define HW_S390_SCLP_EVENT_FACILITY_H
+
+#include <hw/qdev.h>
+#include "qemu-thread.h"
+
+/* SCLP event types */
+#define SCLP_EVENT_ASCII_CONSOLE_DATA 0x1a
+#define SCLP_EVENT_SIGNAL_QUIESCE 0x1d
+
+/* SCLP event masks */
+#define SCLP_EVENT_MASK_SIGNAL_QUIESCE 0x00000008
+#define SCLP_EVENT_MASK_MSG_ASCII 0x00000040
+
+#define SCLP_UNCONDITIONAL_READ 0x00
+#define SCLP_SELECTIVE_READ 0x01
+
+#define TYPE_SCLP_EVENT "s390-sclp-event-type"
+#define SCLP_EVENT(obj) \
+ OBJECT_CHECK(SCLPEvent, (obj), TYPE_SCLP_EVENT)
+#define SCLP_EVENT_CLASS(klass) \
+ OBJECT_CLASS_CHECK(SCLPEventClass, (klass), TYPE_SCLP_EVENT)
+#define SCLP_EVENT_GET_CLASS(obj) \
+ OBJECT_GET_CLASS(SCLPEventClass, (obj), TYPE_SCLP_EVENT)
+
+typedef struct WriteEventMask {
+ SCCBHeader h;
+ uint16_t _reserved;
+ uint16_t mask_length;
+ uint32_t cp_receive_mask;
+ uint32_t cp_send_mask;
+ uint32_t send_mask;
+ uint32_t receive_mask;
+} QEMU_PACKED WriteEventMask;
+
+typedef struct EventBufferHeader {
+ uint16_t length;
+ uint8_t type;
+ uint8_t flags;
+ uint16_t _reserved;
+} QEMU_PACKED EventBufferHeader;
+
+typedef struct WriteEventData {
+ SCCBHeader h;
+ EventBufferHeader ebh;
+} QEMU_PACKED WriteEventData;
+
+typedef struct ReadEventData {
+ SCCBHeader h;
+ EventBufferHeader ebh;
+ uint32_t mask;
+} QEMU_PACKED ReadEventData;
+
+typedef struct SCLPEvent {
+ DeviceState qdev;
+ QemuMutex lock;
+ bool event_pending;
+ uint32_t event_type;
+ char *name;
+} SCLPEvent;
+
+typedef struct SCLPEventClass {
+ DeviceClass parent_class;
+ int (*init)(SCLPEvent *event);
+ int (*exit)(SCLPEvent *event);
+
+ /* get SCLP's send mask */
+ unsigned int (*get_send_mask)(void);
+
+ /* get SCLP's receive mask */
+ unsigned int (*get_receive_mask)(void);
+
+ int (*read_event_data)(SCLPEvent *event, EventBufferHeader *evt_buf_hdr,
+ int *slen);
+
+ int (*write_event_data)(SCLPEvent *event, EventBufferHeader *evt_buf_hdr);
+
+ /* returns the supported event type */
+ int (*event_type)(void);
+
+} SCLPEventClass;
+
+static inline void lock(SCLPEvent *event)
+{
+ qemu_mutex_lock(&event->lock);
+}
+
+static inline void unlock(SCLPEvent *event)
+{
+ qemu_mutex_unlock(&event->lock);
+}
+
+#endif
diff --git a/hw/s390x/sclp.c b/hw/s390x/sclp.c
index 4095ba6..cfc41ea 100644
--- a/hw/s390x/sclp.c
+++ b/hw/s390x/sclp.c
@@ -39,6 +39,7 @@ static int read_SCP_info(SCCB *sccb)
static int sclp_execute(SCCB *sccb, uint64_t code)
{
int r = 0;
+ SCLPEventFacility *ef = sbus->event_facility->instance;
switch (code) {
case SCLP_CMDW_READ_SCP_INFO:
@@ -46,7 +47,7 @@ static int sclp_execute(SCCB *sccb, uint64_t code)
r = read_SCP_info(sccb);
break;
default:
- sccb->h.response_code = cpu_to_be16(SCLP_RC_INVALID_SCLP_COMMAND);
+ r = sbus->event_facility->sclp_command_handler(ef, sccb, code);
break;
}
return r;
@@ -87,10 +88,13 @@ out:
void sclp_service_interrupt(uint32_t sccb)
{
- if (!sccb) {
+ SCLPEventFacility *ef = sbus->event_facility->instance;
+ int event_pending = sbus->event_facility->event_pending(ef);
+
+ if (!event_pending && !sccb) {
return;
}
- s390_sclp_extint(sccb & ~3);
+ s390_sclp_extint((sccb & ~3) + event_pending);
}
/* qemu object creation and initialization functions */
@@ -112,6 +116,9 @@ SCLPS390Bus *s390_sclp_bus_init(void)
bus_state = qbus_create(TYPE_S390_SCLP_BUS, dev, NULL);
bus_state->allow_hotplug = 0;
+ dev = qdev_create(bus_state, "s390-sclp-event-facility");
+ qdev_init_nofail(dev);
+
sbus = DO_UPCAST(SCLPS390Bus, bus, bus_state);
return sbus;
}
@@ -137,9 +144,44 @@ static TypeInfo s390_sclp_bridge_info = {
.class_init = s390_sclp_bridge_class_init,
};
+static int s390_sclp_busdev_init(DeviceState *dev)
+{
+ int r;
+ S390SCLPDevice *sdev = (S390SCLPDevice *)dev;
+ S390SCLPDeviceClass *sclp = SCLP_S390_DEVICE_GET_CLASS(dev);
+ SCLPS390Bus *bus = DO_UPCAST(SCLPS390Bus, bus, sdev->qdev.parent_bus);
+
+ r = sclp->init(sdev);
+ if (!r) {
+ assert(sdev->event_pending);
+ assert(sdev->sclp_command_handler);
+ }
+ bus->event_facility = sdev;
+
+ return r;
+}
+
+static void s390_sclp_device_class_init(ObjectClass *klass, void *data)
+{
+ DeviceClass *dc = DEVICE_CLASS(klass);
+
+ dc->init = s390_sclp_busdev_init;
+ dc->bus_type = TYPE_S390_SCLP_BUS;
+}
+
+static TypeInfo s390_sclp_device_info = {
+ .name = TYPE_DEVICE_S390_SCLP,
+ .parent = TYPE_DEVICE,
+ .instance_size = sizeof(S390SCLPDevice),
+ .class_init = s390_sclp_device_class_init,
+ .class_size = sizeof(S390SCLPDeviceClass),
+ .abstract = true,
+};
+
static void s390_sclp_register_types(void)
{
type_register_static(&s390_sclp_bridge_info);
type_register_static(&s390_sclp_bus_info);
+ type_register_static(&s390_sclp_device_info);
}
type_init(s390_sclp_register_types)
diff --git a/hw/s390x/sclp.h b/hw/s390x/sclp.h
index 4d5a946..d071ccd 100644
--- a/hw/s390x/sclp.h
+++ b/hw/s390x/sclp.h
@@ -19,15 +19,35 @@
/* SCLP command codes */
#define SCLP_CMDW_READ_SCP_INFO 0x00020001
#define SCLP_CMDW_READ_SCP_INFO_FORCED 0x00120001
+#define SCLP_CMD_READ_EVENT_DATA 0x00770005
+#define SCLP_CMD_WRITE_EVENT_DATA 0x00760005
+#define SCLP_CMD_READ_EVENT_DATA 0x00770005
+#define SCLP_CMD_WRITE_EVENT_DATA 0x00760005
+#define SCLP_CMD_WRITE_EVENT_MASK 0x00780005
/* SCLP response codes */
#define SCLP_RC_NORMAL_READ_COMPLETION 0x0010
+#define SCLP_RC_NORMAL_COMPLETION 0x0020
#define SCLP_RC_INVALID_SCLP_COMMAND 0x01f0
+#define SCLP_RC_CONTAINED_EQUIPMENT_CHECK 0x0340
+#define SCLP_RC_INSUFFICIENT_SCCB_LENGTH 0x0300
+#define SCLP_RC_INVALID_FUNCTION 0x40f0
+#define SCLP_RC_NO_EVENT_BUFFERS_STORED 0x60f0
+#define SCLP_RC_INVALID_SELECTION_MASK 0x70f0
+#define SCLP_RC_INCONSISTENT_LENGTHS 0x72f0
+#define SCLP_RC_EVENT_BUFFER_SYNTAX_ERROR 0x73f0
+#define SCLP_RC_INVALID_MASK_LENGTH 0x74f0
+
/* Service Call Control Block (SCCB) and its elements */
#define SCCB_SIZE 4096
+#define SCLP_VARIABLE_LENGTH_RESPONSE 0x80
+#define SCLP_EVENT_BUFFER_ACCEPTED 0x80
+
+#define SCLP_FC_NORMAL_WRITE 0
+
/*
* Normally packed structures are not the right thing to do, since all code
* must take care of endianess. We cant use ldl_phys and friends for two
@@ -64,14 +84,38 @@ typedef struct SCCB {
#define TYPE_S390_SCLP_BUS "s390-sclp-bus"
#define S390_SCLP_BUS(obj) OBJECT_CHECK(SCLPS390Bus, (obj), TYPE_S390_SCLP_BUS)
+#define TYPE_DEVICE_S390_SCLP "s390-sclp-device"
+#define SCLP_S390_DEVIVE(obj) \
+ OBJECT_CHECK(S390SCLPDevice, (obj), TYPE_DEVICE_S390_SCLP)
+#define SCLP_S390_DEVICE_CLASS(klass) \
+ OBJECT_CLASS_CHECK(S390SCLPDeviceClass, (klass), \
+ TYPE_DEVICE_S390_SCLP)
+#define SCLP_S390_DEVICE_GET_CLASS(obj) \
+ OBJECT_GET_CLASS(S390SCLPDeviceClass, (obj), \
+ TYPE_DEVICE_S390_SCLP)
+
+typedef struct SCLPEventFacility SCLPEventFacility;
+
typedef struct S390SCLPDevice {
DeviceState qdev;
+ SCLPEventFacility *instance;
+ int (*sclp_command_handler)(SCLPEventFacility *ef, SCCB *sccb,
+ uint64_t code);
+ bool (*event_pending)(SCLPEventFacility *ef);
} S390SCLPDevice;
typedef struct SCLPS390Bus {
BusState bus;
+ S390SCLPDevice *event_facility;
} SCLPS390Bus;
+typedef struct S390SCLPDeviceClass {
+ DeviceClass qdev;
+
+ int (*init)(S390SCLPDevice *sdev);
+
+} S390SCLPDeviceClass;
+
SCLPS390Bus *s390_sclp_bus_init(void);
void sclp_service_interrupt(uint32_t sccb);
--
1.7.0.1
^ permalink raw reply related [flat|nested] 33+ messages in thread* Re: [Qemu-devel] [PATCH 4/7] s390: sclp event support
2012-07-24 7:37 ` [Qemu-devel] [PATCH 4/7] s390: sclp event support Christian Borntraeger
@ 2012-07-30 13:24 ` Alexander Graf
2012-07-30 14:46 ` Christian Borntraeger
2012-08-20 12:15 ` Heinz Graalfs
2012-07-31 12:59 ` Andreas Färber
1 sibling, 2 replies; 33+ messages in thread
From: Alexander Graf @ 2012-07-30 13:24 UTC (permalink / raw)
To: Christian Borntraeger
Cc: qemu-devel, blauwirbel, Heinz Graalfs, Jens Freimann, afaerber
On 24.07.2012, at 09:37, Christian Borntraeger wrote:
> From: Heinz Graalfs <graalfs@linux.vnet.ibm.com>
>
> Several SCLP features are considered to be events. Those events don't
> provide SCLP commands on their own, instead they are all based on
> Read Event Data, Write Event Data, Write Event Mask and the service
> interrupt. Follow-on patches will provide SCLP's Signal Quiesce (via
> system_powerdown) and the ASCII console.
> Further down the road the sclp line mode console and configuration
> change events (e.g. cpu hotplug) can be implemented.
>
> Signed-off-by: Heinz Graalfs <graalfs@linux.vnet.ibm.com>
> Signed-off-by: Christian Borntraeger <borntraeger@de.ibm.com>
> ---
> hw/s390x/Makefile.objs | 1 +
> hw/s390x/event-facility.c | 390 +++++++++++++++++++++++++++++++++++++++++++++
> hw/s390x/event-facility.h | 107 ++++++++++++
> hw/s390x/sclp.c | 48 +++++-
> hw/s390x/sclp.h | 44 +++++
> 5 files changed, 587 insertions(+), 3 deletions(-)
> create mode 100644 hw/s390x/event-facility.c
> create mode 100644 hw/s390x/event-facility.h
>
> diff --git a/hw/s390x/Makefile.objs b/hw/s390x/Makefile.objs
> index 1c14b96..b32fc52 100644
> --- a/hw/s390x/Makefile.objs
> +++ b/hw/s390x/Makefile.objs
> @@ -2,3 +2,4 @@ obj-y = s390-virtio-bus.o s390-virtio.o
>
> obj-y := $(addprefix ../,$(obj-y))
> obj-y += sclp.o
> +obj-y += event-facility.o
> diff --git a/hw/s390x/event-facility.c b/hw/s390x/event-facility.c
> new file mode 100644
> index 0000000..74a3514
> --- /dev/null
> +++ b/hw/s390x/event-facility.c
> @@ -0,0 +1,390 @@
> +/*
> + * SCLP
> + * Event Facility
> + * handles SCLP event types
> + * - Signal Quiesce - system power down
> + * - ASCII Console Data - VT220 read and write
> + *
> + * Copyright IBM, Corp. 2012
> + *
> + * Authors:
> + * Heinz Graalfs <graalfs@de.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 "monitor.h"
> +#include "sysemu.h"
> +
> +#include "sclp.h"
> +#include "event-facility.h"
> +
> +typedef struct EventTypes {
> + BusState qbus;
> + SCLPEventFacility *event_facility;
> +} EventTypes;
> +
> +struct SCLPEventFacility {
> + EventTypes sbus;
> + DeviceState *qdev;
> + /* guest' receive mask */
> + unsigned int receive_mask;
> +};
> +
> +/* return true if any child has event pending set */
> +static bool event_pending(SCLPEventFacility *ef)
> +{
> + BusChild *kid;
> + SCLPEvent *event;
> +
> + QTAILQ_FOREACH(kid, &ef->sbus.qbus.children, sibling) {
> + DeviceState *qdev = kid->child;
> + event = DO_UPCAST(SCLPEvent, qdev, qdev);
> + lock(event);
While I like short function names, you're certainly not the only person who wants locking in the tree :). Please name this a bit more explicitly.
In fact, why do you need locking in the first place?
> + if (event->event_pending) {
> + unlock(event);
> + return true;
> + }
> + unlock(event);
> + }
> + return false;
> +}
> +
> +static unsigned int get_host_send_mask(SCLPEventFacility *ef)
> +{
> + unsigned int mask;
> + BusChild *kid;
> + SCLPEventClass *child;
> +
> + mask = 0;
> +
> + QTAILQ_FOREACH(kid, &ef->sbus.qbus.children, sibling) {
> + DeviceState *qdev = kid->child;
> + child = SCLP_EVENT_GET_CLASS((SCLPEvent *) qdev);
> + mask |= child->get_send_mask();
> + }
> + return mask;
> +}
> +
> +static unsigned int get_host_receive_mask(SCLPEventFacility *ef)
> +{
> + unsigned int mask;
> + BusChild *kid;
> + SCLPEventClass *child;
> +
> + mask = 0;
> +
> + QTAILQ_FOREACH(kid, &ef->sbus.qbus.children, sibling) {
> + DeviceState *qdev = kid->child;
> + child = SCLP_EVENT_GET_CLASS((SCLPEvent *) qdev);
> + mask |= child->get_receive_mask();
> + }
> + return mask;
> +}
> +
> +static bool sccb_events_ok(SCCB *sccb)
Please return the failure code as return value here. That'd turn the function into completely read-only, making the big picture easier to understand.
> +{
> + int slen;
> + unsigned elen = 0;
> + EventBufferHeader *event;
> + WriteEventData *wed = (WriteEventData *) sccb;
Mind to explain why every sccb_event (coming from the function name) is a WriteEventData?
> +
> + event = (EventBufferHeader *) &wed->ebh;
> + for (slen = be16_to_cpu(sccb->h.length) - sizeof(sccb->h);
> + slen > 0; slen -= elen) {
> + elen = be16_to_cpu(event->length);
> + if (elen < sizeof(*event) || elen > slen) {
> + sccb->h.response_code =
> + cpu_to_be16(SCLP_RC_EVENT_BUFFER_SYNTAX_ERROR);
> + return false;
> + }
> + event = (void *) event + elen;
> + }
> + if (slen) {
> + sccb->h.response_code = cpu_to_be16(SCLP_RC_INCONSISTENT_LENGTHS);
> + return false;
> + }
> + return true;
> +}
> +
> +static void handle_sccb_write_events(SCLPEventFacility *ef, SCCB *sccb)
> +{
> + int slen;
> + unsigned elen = 0;
> + EventBufferHeader *event_buf;
> + BusChild *kid;
> + SCLPEvent *event;
> + SCLPEventClass *ec;
> +
> + WriteEventData *wed = (WriteEventData *) sccb;
> +
> + event_buf = &wed->ebh;
> +
> + /* loop over all contained event buffers */
> + for (slen = be16_to_cpu(sccb->h.length) - sizeof(sccb->h);
How about a
static inline int sccb_data_len(SCCB *sccb) {
return be16_to_cpu(sccb->h.length) - sizeof(sccb->h);
}
in the sclp header, and then
for (slen = sccb_data_len(sccb); slen > 0; slen -= elen) {
...
here and above?
> + slen > 0; slen -= elen) {
> + elen = be16_to_cpu(event_buf->length);
> +
> + /* in case of a previous error mark all trailing buffers
> + * as not accepted */
> + if (sccb->h.response_code !=
> + cpu_to_be16(SCLP_RC_NORMAL_COMPLETION)) {
> + event_buf->flags &= ~(SCLP_EVENT_BUFFER_ACCEPTED);
When would we hit this?
> + } else {
> + sccb->h.response_code = cpu_to_be16(SCLP_RC_INVALID_FUNCTION);
Again, please factor that out to either the parent function or at least the end of this function.
> + QTAILQ_FOREACH(kid, &ef->sbus.qbus.children, sibling) {
> + DeviceState *qdev = kid->child;
> + event = (SCLPEvent *) qdev;
> + ec = SCLP_EVENT_GET_CLASS(event);
> +
> + if (ec->write_event_data &&
> + ec->event_type() == event_buf->type) {
> + sccb->h.response_code = cpu_to_be16(
> + ec->write_event_data(event, event_buf));
> + break;
> + }
> + }
> + }
> + event_buf = (void *) event_buf + elen;
> + }
> +}
> +
> +static int write_event_data(SCLPEventFacility *ef, SCCB *sccb)
> +{
> + if (sccb->h.function_code != SCLP_FC_NORMAL_WRITE) {
> + sccb->h.response_code = cpu_to_be16(SCLP_RC_INVALID_FUNCTION);
> + goto out;
> + }
> + if (be16_to_cpu(sccb->h.length) < 8) {
> + sccb->h.response_code = cpu_to_be16(SCLP_RC_INSUFFICIENT_SCCB_LENGTH);
> + goto out;
> + }
> + /* first check the sum of all events */
> + if (!sccb_events_ok(sccb)) {
> + goto out;
> + }
> + /* then execute */
> + sccb->h.response_code = cpu_to_be16(SCLP_RC_NORMAL_COMPLETION);
> + handle_sccb_write_events(ef, sccb);
> +
> +out:
> + return 0;
This always returns 0? Let me guess - you originally wanted to pass response_code as return value! ;)
I think you get the pattern by now :)
> +}
> +
> +static void handle_sccb_read_events(SCLPEventFacility *ef, SCCB *sccb,
> + unsigned int mask)
> +{
> + int slen;
> + unsigned elen = 0;
> + BusChild *kid;
> + SCLPEvent *event;
> + SCLPEventClass *ec;
> + EventBufferHeader *event_buf;
> + ReadEventData *red = (ReadEventData *) sccb;
> +
> + event_buf = &red->ebh;
> + event_buf->length = 0;
> + slen = sizeof(sccb->data);
> + QTAILQ_FOREACH(kid, &ef->sbus.qbus.children, sibling) {
> + DeviceState *qdev = kid->child;
> + event = (SCLPEvent *) qdev;
> + ec = SCLP_EVENT_GET_CLASS(event);
> +
> + if (mask & ec->get_send_mask()) {
> + if (ec->read_event_data(event, event_buf, &slen)) {
> + sccb->h.response_code = cpu_to_be16(SCLP_RC_NORMAL_COMPLETION);
What is the response code when we don't have any data?
> + }
> + }
> + elen = be16_to_cpu(event_buf->length);
> + event_buf = (void *) event_buf + elen;
> + }
> +
> + if (sccb->h.control_mask[2] & SCLP_VARIABLE_LENGTH_RESPONSE) {
This block deserves a comment.
> + sccb->h.control_mask[2] &= ~SCLP_VARIABLE_LENGTH_RESPONSE;
> + sccb->h.length = cpu_to_be16(SCCB_SIZE - slen);
> + }
> +}
> +
> +static int read_event_data(SCLPEventFacility *ef, SCCB *sccb)
> +{
> + unsigned int sclp_active_selection_mask;
> + unsigned int sclp_cp_receive_mask;
> +
> + ReadEventData *red = (ReadEventData *) sccb;
> +
> + if (be16_to_cpu(sccb->h.length) != SCCB_SIZE) {
> + sccb->h.response_code = cpu_to_be16(SCLP_RC_INSUFFICIENT_SCCB_LENGTH);
> + goto out;
> + }
> +
> + sclp_cp_receive_mask = ef->receive_mask;
> +
> + /* get active selection mask */
> + switch (sccb->h.function_code) {
> + case SCLP_UNCONDITIONAL_READ:
> + sclp_active_selection_mask = sclp_cp_receive_mask;
> + break;
> + case SCLP_SELECTIVE_READ:
> + if (!(sclp_cp_receive_mask & be32_to_cpu(red->mask))) {
> + sccb->h.response_code =
> + cpu_to_be16(SCLP_RC_INVALID_SELECTION_MASK);
> + goto out;
> + }
> + sclp_active_selection_mask = be32_to_cpu(red->mask);
> + break;
> + default:
> + sccb->h.response_code = cpu_to_be16(SCLP_RC_INVALID_FUNCTION);
> + goto out;
> + }
> +
> + sccb->h.response_code = cpu_to_be16(SCLP_RC_NO_EVENT_BUFFERS_STORED);
> + handle_sccb_read_events(ef, sccb, sclp_active_selection_mask);
> +
> +out:
> + return 0;
> +}
> +
> +static int write_event_mask(SCLPEventFacility *ef, SCCB *sccb)
> +{
> + WriteEventMask *we_mask = (WriteEventMask *) sccb;
> +
> + /* Attention: We assume that Linux uses 4-byte masks, what it actually
> + does. Architecture allows for masks of variable size, though */
> + if (be16_to_cpu(we_mask->mask_length) != 4) {
> + sccb->h.response_code = cpu_to_be16(SCLP_RC_INVALID_MASK_LENGTH);
> + goto out;
> + }
> +
> + /* keep track of the guest's capability masks */
> + ef->receive_mask = be32_to_cpu(we_mask->cp_receive_mask);
> +
> + /* return the SCLP's capability masks to the guest */
> + we_mask->send_mask = cpu_to_be32(get_host_send_mask(ef));
> + we_mask->receive_mask = cpu_to_be32(get_host_receive_mask(ef));
> +
> + sccb->h.response_code = cpu_to_be16(SCLP_RC_NORMAL_COMPLETION);
> +
> +out:
> + return 0;
> +}
> +
> +/* qemu object creation and initialization functions */
> +
> +#define TYPE_SCLP_EVENTS_BUS "s390-sclp-events-bus"
> +#define SCLP_EVENTS_BUS(obj) OBJECT_CHECK(SCLPS390Bus, (obj),\
> + TYPE_SCLP_EVENTS_BUS)
> +
> +static void sclp_events_bus_class_init(ObjectClass *klass, void *data)
> +{
> +}
> +
> +static const TypeInfo s390_sclp_events_bus_info = {
> + .name = TYPE_SCLP_EVENTS_BUS,
> + .parent = TYPE_BUS,
> + .instance_size = sizeof(SCLPS390Bus),
> + .class_init = sclp_events_bus_class_init,
> +};
> +
> +static int command_handler(SCLPEventFacility *ef, SCCB *sccb, uint64_t code)
> +{
> + int r = 0;
> +
> + switch (code) {
> + case SCLP_CMD_READ_EVENT_DATA:
> + r = read_event_data(ef, sccb);
> + break;
> + case SCLP_CMD_WRITE_EVENT_DATA:
> + r = write_event_data(ef, sccb);
> + break;
> + case SCLP_CMD_WRITE_EVENT_MASK:
> + r = write_event_mask(ef, sccb);
> + break;
> + default:
> +#ifdef DEBUG_HELPER
Why DEBUG_HELPER?
> + printf("KVM: unhandled sclp code 0x%" PRIx64 "x\n", code);
> +#endif
> + sccb->h.response_code = cpu_to_be16(SCLP_RC_INVALID_SCLP_COMMAND);
> + break;
> + }
> + return r;
> +}
> +
> +static int init_event_facility(S390SCLPDevice *sdev)
> +{
> + SCLPEventFacility *event_facility;
> +
> + event_facility = g_malloc0(sizeof(SCLPEventFacility));
> + sdev->instance = event_facility;
> + sdev->sclp_command_handler = command_handler;
> + sdev->event_pending = event_pending;
> +
> + /* Spawn a new sclp-events facility */
> + qbus_create_inplace(&event_facility->sbus.qbus,
> + TYPE_SCLP_EVENTS_BUS, (DeviceState *)sdev, NULL);
> + event_facility->sbus.qbus.allow_hotplug = 0;
> + event_facility->sbus.event_facility = event_facility;
> + event_facility->qdev = (DeviceState *) sdev;
> +
> + return 0;
> +}
> +
> +static void init_event_facility_class(ObjectClass *klass, void *data)
> +{
> + S390SCLPDeviceClass *k = SCLP_S390_DEVICE_CLASS(klass);
> +
> + k->init = init_event_facility;
> +}
> +
> +static TypeInfo s390_sclp_event_facility_info = {
> + .name = "s390-sclp-event-facility",
> + .parent = TYPE_DEVICE_S390_SCLP,
> + .instance_size = sizeof(S390SCLPDevice),
> + .class_init = init_event_facility_class,
> +};
> +
> +static int event_qdev_init(DeviceState *qdev)
> +{
> + SCLPEvent *event = DO_UPCAST(SCLPEvent, qdev, qdev);
> + SCLPEventClass *child = SCLP_EVENT_GET_CLASS(event);
> +
> + return child->init(event);
> +}
> +
> +static int event_qdev_exit(DeviceState *qdev)
> +{
> + SCLPEvent *event = DO_UPCAST(SCLPEvent, qdev, qdev);
> + SCLPEventClass *child = SCLP_EVENT_GET_CLASS(event);
> + if (child->exit) {
> + child->exit(event);
> + }
> + return 0;
> +}
> +
> +static void event_class_init(ObjectClass *klass, void *data)
> +{
> + DeviceClass *dc = DEVICE_CLASS(klass);
> +
> + dc->bus_type = TYPE_SCLP_EVENTS_BUS;
> + dc->unplug = qdev_simple_unplug_cb;
> + dc->init = event_qdev_init;
> + dc->exit = event_qdev_exit;
> +}
> +
> +static TypeInfo s390_sclp_event_type_info = {
> + .name = TYPE_SCLP_EVENT,
> + .parent = TYPE_DEVICE,
> + .instance_size = sizeof(SCLPEvent),
> + .class_init = event_class_init,
> + .class_size = sizeof(SCLPEventClass),
> + .abstract = true,
> +};
> +
> +static void register_types(void)
> +{
> + type_register_static(&s390_sclp_events_bus_info);
> + type_register_static(&s390_sclp_event_facility_info);
> + type_register_static(&s390_sclp_event_type_info);
> +}
> +type_init(register_types)
> diff --git a/hw/s390x/event-facility.h b/hw/s390x/event-facility.h
> new file mode 100644
> index 0000000..1e022a3
> --- /dev/null
> +++ b/hw/s390x/event-facility.h
> @@ -0,0 +1,107 @@
> +/*
> + * SCLP
> + * Event Facility definitions
> + *
> + * Copyright IBM, Corp. 2012
> + *
> + * Authors:
> + * Heinz Graalfs <graalfs@de.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_S390_SCLP_EVENT_FACILITY_H
> +#define HW_S390_SCLP_EVENT_FACILITY_H
> +
> +#include <hw/qdev.h>
> +#include "qemu-thread.h"
> +
> +/* SCLP event types */
> +#define SCLP_EVENT_ASCII_CONSOLE_DATA 0x1a
> +#define SCLP_EVENT_SIGNAL_QUIESCE 0x1d
> +
> +/* SCLP event masks */
> +#define SCLP_EVENT_MASK_SIGNAL_QUIESCE 0x00000008
> +#define SCLP_EVENT_MASK_MSG_ASCII 0x00000040
> +
> +#define SCLP_UNCONDITIONAL_READ 0x00
> +#define SCLP_SELECTIVE_READ 0x01
> +
> +#define TYPE_SCLP_EVENT "s390-sclp-event-type"
> +#define SCLP_EVENT(obj) \
> + OBJECT_CHECK(SCLPEvent, (obj), TYPE_SCLP_EVENT)
> +#define SCLP_EVENT_CLASS(klass) \
> + OBJECT_CLASS_CHECK(SCLPEventClass, (klass), TYPE_SCLP_EVENT)
> +#define SCLP_EVENT_GET_CLASS(obj) \
> + OBJECT_GET_CLASS(SCLPEventClass, (obj), TYPE_SCLP_EVENT)
> +
> +typedef struct WriteEventMask {
> + SCCBHeader h;
> + uint16_t _reserved;
> + uint16_t mask_length;
> + uint32_t cp_receive_mask;
> + uint32_t cp_send_mask;
> + uint32_t send_mask;
> + uint32_t receive_mask;
> +} QEMU_PACKED WriteEventMask;
> +
> +typedef struct EventBufferHeader {
> + uint16_t length;
> + uint8_t type;
> + uint8_t flags;
> + uint16_t _reserved;
> +} QEMU_PACKED EventBufferHeader;
> +
> +typedef struct WriteEventData {
> + SCCBHeader h;
> + EventBufferHeader ebh;
> +} QEMU_PACKED WriteEventData;
> +
> +typedef struct ReadEventData {
> + SCCBHeader h;
> + EventBufferHeader ebh;
> + uint32_t mask;
> +} QEMU_PACKED ReadEventData;
> +
> +typedef struct SCLPEvent {
> + DeviceState qdev;
> + QemuMutex lock;
> + bool event_pending;
> + uint32_t event_type;
> + char *name;
> +} SCLPEvent;
> +
> +typedef struct SCLPEventClass {
> + DeviceClass parent_class;
> + int (*init)(SCLPEvent *event);
> + int (*exit)(SCLPEvent *event);
> +
> + /* get SCLP's send mask */
> + unsigned int (*get_send_mask)(void);
> +
> + /* get SCLP's receive mask */
> + unsigned int (*get_receive_mask)(void);
> +
> + int (*read_event_data)(SCLPEvent *event, EventBufferHeader *evt_buf_hdr,
> + int *slen);
> +
> + int (*write_event_data)(SCLPEvent *event, EventBufferHeader *evt_buf_hdr);
> +
> + /* returns the supported event type */
> + int (*event_type)(void);
> +
> +} SCLPEventClass;
> +
> +static inline void lock(SCLPEvent *event)
> +{
> + qemu_mutex_lock(&event->lock);
Yeah, I'm fairly sure you don't need the locks :). It might be nice to keep them in as documentation, but make them noops (and name them properly).
> +}
> +
> +static inline void unlock(SCLPEvent *event)
> +{
> + qemu_mutex_unlock(&event->lock);
> +}
> +
> +#endif
> diff --git a/hw/s390x/sclp.c b/hw/s390x/sclp.c
> index 4095ba6..cfc41ea 100644
> --- a/hw/s390x/sclp.c
> +++ b/hw/s390x/sclp.c
> @@ -39,6 +39,7 @@ static int read_SCP_info(SCCB *sccb)
> static int sclp_execute(SCCB *sccb, uint64_t code)
> {
> int r = 0;
> + SCLPEventFacility *ef = sbus->event_facility->instance;
>
> switch (code) {
> case SCLP_CMDW_READ_SCP_INFO:
> @@ -46,7 +47,7 @@ static int sclp_execute(SCCB *sccb, uint64_t code)
> r = read_SCP_info(sccb);
> break;
> default:
> - sccb->h.response_code = cpu_to_be16(SCLP_RC_INVALID_SCLP_COMMAND);
> + r = sbus->event_facility->sclp_command_handler(ef, sccb, code);
> break;
> }
> return r;
> @@ -87,10 +88,13 @@ out:
>
> void sclp_service_interrupt(uint32_t sccb)
> {
> - if (!sccb) {
> + SCLPEventFacility *ef = sbus->event_facility->instance;
> + int event_pending = sbus->event_facility->event_pending(ef);
> +
> + if (!event_pending && !sccb) {
Uh. So when there's no event pending no sclp call may trigger an interrupt?
> return;
> }
> - s390_sclp_extint(sccb & ~3);
> + s390_sclp_extint((sccb & ~3) + event_pending);
event_pending returns a bool, right? Please make this code a bit less magical :).
Alex
> }
>
> /* qemu object creation and initialization functions */
> @@ -112,6 +116,9 @@ SCLPS390Bus *s390_sclp_bus_init(void)
> bus_state = qbus_create(TYPE_S390_SCLP_BUS, dev, NULL);
> bus_state->allow_hotplug = 0;
>
> + dev = qdev_create(bus_state, "s390-sclp-event-facility");
> + qdev_init_nofail(dev);
> +
> sbus = DO_UPCAST(SCLPS390Bus, bus, bus_state);
> return sbus;
> }
> @@ -137,9 +144,44 @@ static TypeInfo s390_sclp_bridge_info = {
> .class_init = s390_sclp_bridge_class_init,
> };
>
> +static int s390_sclp_busdev_init(DeviceState *dev)
> +{
> + int r;
> + S390SCLPDevice *sdev = (S390SCLPDevice *)dev;
> + S390SCLPDeviceClass *sclp = SCLP_S390_DEVICE_GET_CLASS(dev);
> + SCLPS390Bus *bus = DO_UPCAST(SCLPS390Bus, bus, sdev->qdev.parent_bus);
> +
> + r = sclp->init(sdev);
> + if (!r) {
> + assert(sdev->event_pending);
> + assert(sdev->sclp_command_handler);
> + }
> + bus->event_facility = sdev;
> +
> + return r;
> +}
> +
> +static void s390_sclp_device_class_init(ObjectClass *klass, void *data)
> +{
> + DeviceClass *dc = DEVICE_CLASS(klass);
> +
> + dc->init = s390_sclp_busdev_init;
> + dc->bus_type = TYPE_S390_SCLP_BUS;
> +}
> +
> +static TypeInfo s390_sclp_device_info = {
> + .name = TYPE_DEVICE_S390_SCLP,
> + .parent = TYPE_DEVICE,
> + .instance_size = sizeof(S390SCLPDevice),
> + .class_init = s390_sclp_device_class_init,
> + .class_size = sizeof(S390SCLPDeviceClass),
> + .abstract = true,
> +};
> +
> static void s390_sclp_register_types(void)
> {
> type_register_static(&s390_sclp_bridge_info);
> type_register_static(&s390_sclp_bus_info);
> + type_register_static(&s390_sclp_device_info);
> }
> type_init(s390_sclp_register_types)
> diff --git a/hw/s390x/sclp.h b/hw/s390x/sclp.h
> index 4d5a946..d071ccd 100644
> --- a/hw/s390x/sclp.h
> +++ b/hw/s390x/sclp.h
> @@ -19,15 +19,35 @@
> /* SCLP command codes */
> #define SCLP_CMDW_READ_SCP_INFO 0x00020001
> #define SCLP_CMDW_READ_SCP_INFO_FORCED 0x00120001
> +#define SCLP_CMD_READ_EVENT_DATA 0x00770005
> +#define SCLP_CMD_WRITE_EVENT_DATA 0x00760005
> +#define SCLP_CMD_READ_EVENT_DATA 0x00770005
> +#define SCLP_CMD_WRITE_EVENT_DATA 0x00760005
> +#define SCLP_CMD_WRITE_EVENT_MASK 0x00780005
>
> /* SCLP response codes */
> #define SCLP_RC_NORMAL_READ_COMPLETION 0x0010
> +#define SCLP_RC_NORMAL_COMPLETION 0x0020
> #define SCLP_RC_INVALID_SCLP_COMMAND 0x01f0
> +#define SCLP_RC_CONTAINED_EQUIPMENT_CHECK 0x0340
> +#define SCLP_RC_INSUFFICIENT_SCCB_LENGTH 0x0300
> +#define SCLP_RC_INVALID_FUNCTION 0x40f0
> +#define SCLP_RC_NO_EVENT_BUFFERS_STORED 0x60f0
> +#define SCLP_RC_INVALID_SELECTION_MASK 0x70f0
> +#define SCLP_RC_INCONSISTENT_LENGTHS 0x72f0
> +#define SCLP_RC_EVENT_BUFFER_SYNTAX_ERROR 0x73f0
> +#define SCLP_RC_INVALID_MASK_LENGTH 0x74f0
> +
>
> /* Service Call Control Block (SCCB) and its elements */
>
> #define SCCB_SIZE 4096
>
> +#define SCLP_VARIABLE_LENGTH_RESPONSE 0x80
> +#define SCLP_EVENT_BUFFER_ACCEPTED 0x80
> +
> +#define SCLP_FC_NORMAL_WRITE 0
> +
> /*
> * Normally packed structures are not the right thing to do, since all code
> * must take care of endianess. We cant use ldl_phys and friends for two
> @@ -64,14 +84,38 @@ typedef struct SCCB {
> #define TYPE_S390_SCLP_BUS "s390-sclp-bus"
> #define S390_SCLP_BUS(obj) OBJECT_CHECK(SCLPS390Bus, (obj), TYPE_S390_SCLP_BUS)
>
> +#define TYPE_DEVICE_S390_SCLP "s390-sclp-device"
> +#define SCLP_S390_DEVIVE(obj) \
> + OBJECT_CHECK(S390SCLPDevice, (obj), TYPE_DEVICE_S390_SCLP)
> +#define SCLP_S390_DEVICE_CLASS(klass) \
> + OBJECT_CLASS_CHECK(S390SCLPDeviceClass, (klass), \
> + TYPE_DEVICE_S390_SCLP)
> +#define SCLP_S390_DEVICE_GET_CLASS(obj) \
> + OBJECT_GET_CLASS(S390SCLPDeviceClass, (obj), \
> + TYPE_DEVICE_S390_SCLP)
> +
> +typedef struct SCLPEventFacility SCLPEventFacility;
> +
> typedef struct S390SCLPDevice {
> DeviceState qdev;
> + SCLPEventFacility *instance;
> + int (*sclp_command_handler)(SCLPEventFacility *ef, SCCB *sccb,
> + uint64_t code);
> + bool (*event_pending)(SCLPEventFacility *ef);
> } S390SCLPDevice;
>
> typedef struct SCLPS390Bus {
> BusState bus;
> + S390SCLPDevice *event_facility;
> } SCLPS390Bus;
>
> +typedef struct S390SCLPDeviceClass {
> + DeviceClass qdev;
> +
> + int (*init)(S390SCLPDevice *sdev);
> +
> +} S390SCLPDeviceClass;
> +
> SCLPS390Bus *s390_sclp_bus_init(void);
>
> void sclp_service_interrupt(uint32_t sccb);
> --
> 1.7.0.1
>
^ permalink raw reply [flat|nested] 33+ messages in thread* Re: [Qemu-devel] [PATCH 4/7] s390: sclp event support
2012-07-30 13:24 ` Alexander Graf
@ 2012-07-30 14:46 ` Christian Borntraeger
2012-07-30 14:57 ` Alexander Graf
2012-08-20 12:15 ` Heinz Graalfs
1 sibling, 1 reply; 33+ messages in thread
From: Christian Borntraeger @ 2012-07-30 14:46 UTC (permalink / raw)
To: Alexander Graf
Cc: qemu-devel, blauwirbel, Heinz Graalfs, Jens Freimann, afaerber
On 30/07/12 15:24, Alexander Graf wrote:
Thanks for the review.
Here is just a short answer regarding the interrupt, we will adress the other comments in a later
mail.
>> void sclp_service_interrupt(uint32_t sccb)
>> {
>> - if (!sccb) {
>> + SCLPEventFacility *ef = sbus->event_facility->instance;
>> + int event_pending = sbus->event_facility->event_pending(ef);
>> +
>> + if (!event_pending && !sccb) {
>> return;
>> }
>
> Uh. So when there's no event pending no sclp call may trigger an interrupt?
No. If there is no event pending AND no sccb was specified, then we have nothing to report
--> no interrupt.
An service interrupt has a 32bit value as payload. Bits 0-28 (in IBM speak, 31-3 in Linux speak)
contain the sccb address (if any), the other bits are special. The LSB describes if there are still
events pending.
So an unsolicited interrupt will only set the LSB.
An answer to a service call will always contain the sccb and might have the lsb set if there are
events pending.
>> - s390_sclp_extint(sccb & ~3);
>> + s390_sclp_extint((sccb & ~3) + event_pending);
>
> event_pending returns a bool, right? Please make this code a bit less magical :).
Something like "event_pending?1:0" ?
^ permalink raw reply [flat|nested] 33+ messages in thread* Re: [Qemu-devel] [PATCH 4/7] s390: sclp event support
2012-07-30 14:46 ` Christian Borntraeger
@ 2012-07-30 14:57 ` Alexander Graf
0 siblings, 0 replies; 33+ messages in thread
From: Alexander Graf @ 2012-07-30 14:57 UTC (permalink / raw)
To: Christian Borntraeger
Cc: qemu-devel, blauwirbel, Heinz Graalfs, Jens Freimann, afaerber
On 30.07.2012, at 16:46, Christian Borntraeger wrote:
> On 30/07/12 15:24, Alexander Graf wrote:
>
> Thanks for the review.
>
> Here is just a short answer regarding the interrupt, we will adress the other comments in a later
> mail.
>
>>> void sclp_service_interrupt(uint32_t sccb)
>>> {
>>> - if (!sccb) {
>>> + SCLPEventFacility *ef = sbus->event_facility->instance;
>>> + int event_pending = sbus->event_facility->event_pending(ef);
>>> +
>>> + if (!event_pending && !sccb) {
>>> return;
>>> }
>>
>> Uh. So when there's no event pending no sclp call may trigger an interrupt?
>
> No. If there is no event pending AND no sccb was specified, then we have nothing to report
> --> no interrupt.
Oh. My bad :).
>
> An service interrupt has a 32bit value as payload. Bits 0-28 (in IBM speak, 31-3 in Linux speak)
> contain the sccb address (if any), the other bits are special. The LSB describes if there are still
> events pending.
>
> So an unsolicited interrupt will only set the LSB.
> An answer to a service call will always contain the sccb and might have the lsb set if there are
> events pending.
Ok, so this really is a bit we're trying to set then, not an integer we want to add.
>
>
>>> - s390_sclp_extint(sccb & ~3);
>>> + s390_sclp_extint((sccb & ~3) + event_pending);
>>
>> event_pending returns a bool, right? Please make this code a bit less magical :).
>
> Something like "event_pending?1:0" ?
Something like
param = sccb & ~3;
/* Indicate whether an event is still pending */
param |= event_pending ? 1 : 0;
if (!param) {
/* No need to send an interrupt, there's nothing to be notified about */
return;
}
s390_sclp_extint(param);
Alex
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [Qemu-devel] [PATCH 4/7] s390: sclp event support
2012-07-30 13:24 ` Alexander Graf
2012-07-30 14:46 ` Christian Borntraeger
@ 2012-08-20 12:15 ` Heinz Graalfs
1 sibling, 0 replies; 33+ messages in thread
From: Heinz Graalfs @ 2012-08-20 12:15 UTC (permalink / raw)
To: Alexander Graf
Cc: blauwirbel, Christian Borntraeger, Jens Freimann, afaerber, qemu-devel
Alex, some more answers for a couple of leftovers.
Heinz
On Mon, 2012-07-30 at 15:24 +0200, Alexander Graf wrote:
> On 24.07.2012, at 09:37, Christian Borntraeger wrote:
>
> > From: Heinz Graalfs <graalfs@linux.vnet.ibm.com>
> >
> > Several SCLP features are considered to be events. Those events don't
> > provide SCLP commands on their own, instead they are all based on
> > Read Event Data, Write Event Data, Write Event Mask and the service
> > interrupt. Follow-on patches will provide SCLP's Signal Quiesce (via
> > system_powerdown) and the ASCII console.
> > Further down the road the sclp line mode console and configuration
> > change events (e.g. cpu hotplug) can be implemented.
> >
> > Signed-off-by: Heinz Graalfs <graalfs@linux.vnet.ibm.com>
> > Signed-off-by: Christian Borntraeger <borntraeger@de.ibm.com>
> > ---
> > hw/s390x/Makefile.objs | 1 +
> > hw/s390x/event-facility.c | 390 +++++++++++++++++++++++++++++++++++++++++++++
> > hw/s390x/event-facility.h | 107 ++++++++++++
> > hw/s390x/sclp.c | 48 +++++-
> > hw/s390x/sclp.h | 44 +++++
> > 5 files changed, 587 insertions(+), 3 deletions(-)
> > create mode 100644 hw/s390x/event-facility.c
> > create mode 100644 hw/s390x/event-facility.h
> >
> > diff --git a/hw/s390x/Makefile.objs b/hw/s390x/Makefile.objs
> > index 1c14b96..b32fc52 100644
> > --- a/hw/s390x/Makefile.objs
> > +++ b/hw/s390x/Makefile.objs
> > @@ -2,3 +2,4 @@ obj-y = s390-virtio-bus.o s390-virtio.o
> >
> > obj-y := $(addprefix ../,$(obj-y))
> > obj-y += sclp.o
> > +obj-y += event-facility.o
> > diff --git a/hw/s390x/event-facility.c b/hw/s390x/event-facility.c
> > new file mode 100644
> > index 0000000..74a3514
> > --- /dev/null
> > +++ b/hw/s390x/event-facility.c
> > @@ -0,0 +1,390 @@
> > +/*
> > + * SCLP
> > + * Event Facility
> > + * handles SCLP event types
> > + * - Signal Quiesce - system power down
> > + * - ASCII Console Data - VT220 read and write
> > + *
> > + * Copyright IBM, Corp. 2012
> > + *
> > + * Authors:
> > + * Heinz Graalfs <graalfs@de.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 "monitor.h"
> > +#include "sysemu.h"
> > +
> > +#include "sclp.h"
> > +#include "event-facility.h"
> > +
> > +typedef struct EventTypes {
> > + BusState qbus;
> > + SCLPEventFacility *event_facility;
> > +} EventTypes;
> > +
> > +struct SCLPEventFacility {
> > + EventTypes sbus;
> > + DeviceState *qdev;
> > + /* guest' receive mask */
> > + unsigned int receive_mask;
> > +};
> > +
> > +/* return true if any child has event pending set */
> > +static bool event_pending(SCLPEventFacility *ef)
> > +{
> > + BusChild *kid;
> > + SCLPEvent *event;
> > +
> > + QTAILQ_FOREACH(kid, &ef->sbus.qbus.children, sibling) {
> > + DeviceState *qdev = kid->child;
> > + event = DO_UPCAST(SCLPEvent, qdev, qdev);
> > + lock(event);
>
> While I like short function names, you're certainly not the only person who wants locking in the tree :). Please name this a bit more explicitly.
>
> In fact, why do you need locking in the first place?
Ok, we verified with gdb that character layer always holds the
qemu_global_mutex when calling us and the sclp calls from the cpu
threads also hold the qemu_global_mutex. So I just removed these locks
and verified with valgrinds drd. valgrind complains about the aio code,
but the sclp stuff is fine.
>
> > + if (event->event_pending) {
> > + unlock(event);
> > + return true;
> > + }
> > + unlock(event);
> > + }
> > + return false;
> > +}
> > +
> > +static unsigned int get_host_send_mask(SCLPEventFacility *ef)
> > +{
> > + unsigned int mask;
> > + BusChild *kid;
> > + SCLPEventClass *child;
> > +
> > + mask = 0;
> > +
> > + QTAILQ_FOREACH(kid, &ef->sbus.qbus.children, sibling) {
> > + DeviceState *qdev = kid->child;
> > + child = SCLP_EVENT_GET_CLASS((SCLPEvent *) qdev);
> > + mask |= child->get_send_mask();
> > + }
> > + return mask;
> > +}
> > +
> > +static unsigned int get_host_receive_mask(SCLPEventFacility *ef)
> > +{
> > + unsigned int mask;
> > + BusChild *kid;
> > + SCLPEventClass *child;
> > +
> > + mask = 0;
> > +
> > + QTAILQ_FOREACH(kid, &ef->sbus.qbus.children, sibling) {
> > + DeviceState *qdev = kid->child;
> > + child = SCLP_EVENT_GET_CLASS((SCLPEvent *) qdev);
> > + mask |= child->get_receive_mask();
> > + }
> > + return mask;
> > +}
> > +
> > +static bool sccb_events_ok(SCCB *sccb)
>
> Please return the failure code as return value here. That'd turn the function into completely read-only, making the big picture easier to understand.
>
OK...
> > +{
> > + int slen;
> > + unsigned elen = 0;
> > + EventBufferHeader *event;
> > + WriteEventData *wed = (WriteEventData *) sccb;
>
> Mind to explain why every sccb_event (coming from the function name) is a WriteEventData?
>
... and function renamed to write_event_length_check
> > +
> > + event = (EventBufferHeader *) &wed->ebh;
> > + for (slen = be16_to_cpu(sccb->h.length) - sizeof(sccb->h);
> > + slen > 0; slen -= elen) {
> > + elen = be16_to_cpu(event->length);
> > + if (elen < sizeof(*event) || elen > slen) {
> > + sccb->h.response_code =
> > + cpu_to_be16(SCLP_RC_EVENT_BUFFER_SYNTAX_ERROR);
> > + return false;
> > + }
> > + event = (void *) event + elen;
> > + }
> > + if (slen) {
> > + sccb->h.response_code = cpu_to_be16(SCLP_RC_INCONSISTENT_LENGTHS);
> > + return false;
> > + }
> > + return true;
> > +}
> > +
> > +static void handle_sccb_write_events(SCLPEventFacility *ef, SCCB *sccb)
> > +{
> > + int slen;
> > + unsigned elen = 0;
> > + EventBufferHeader *event_buf;
> > + BusChild *kid;
> > + SCLPEvent *event;
> > + SCLPEventClass *ec;
> > +
> > + WriteEventData *wed = (WriteEventData *) sccb;
> > +
> > + event_buf = &wed->ebh;
> > +
> > + /* loop over all contained event buffers */
> > + for (slen = be16_to_cpu(sccb->h.length) - sizeof(sccb->h);
>
> How about a
>
> static inline int sccb_data_len(SCCB *sccb) {
> return be16_to_cpu(sccb->h.length) - sizeof(sccb->h);
> }
>
> in the sclp header, and then
>
> for (slen = sccb_data_len(sccb); slen > 0; slen -= elen) {
> ...
>
> here and above?
ok
>
> > + slen > 0; slen -= elen) {
> > + elen = be16_to_cpu(event_buf->length);
> > +
> > + /* in case of a previous error mark all trailing buffers
> > + * as not accepted */
> > + if (sccb->h.response_code !=
> > + cpu_to_be16(SCLP_RC_NORMAL_COMPLETION)) {
> > + event_buf->flags &= ~(SCLP_EVENT_BUFFER_ACCEPTED);
>
> When would we hit this?
in case of a write failed
>
> > + } else {
> > + sccb->h.response_code = cpu_to_be16(SCLP_RC_INVALID_FUNCTION);
>
> Again, please factor that out to either the parent function or at least the end of this function.
>
ok, end of this function moved to new function
> > + QTAILQ_FOREACH(kid, &ef->sbus.qbus.children, sibling) {
> > + DeviceState *qdev = kid->child;
> > + event = (SCLPEvent *) qdev;
> > + ec = SCLP_EVENT_GET_CLASS(event);
> > +
> > + if (ec->write_event_data &&
> > + ec->event_type() == event_buf->type) {
> > + sccb->h.response_code = cpu_to_be16(
> > + ec->write_event_data(event, event_buf));
> > + break;
> > + }
> > + }
> > + }
> > + event_buf = (void *) event_buf + elen;
> > + }
> > +}
> > +
> > +static int write_event_data(SCLPEventFacility *ef, SCCB *sccb)
> > +{
> > + if (sccb->h.function_code != SCLP_FC_NORMAL_WRITE) {
> > + sccb->h.response_code = cpu_to_be16(SCLP_RC_INVALID_FUNCTION);
> > + goto out;
> > + }
> > + if (be16_to_cpu(sccb->h.length) < 8) {
> > + sccb->h.response_code = cpu_to_be16(SCLP_RC_INSUFFICIENT_SCCB_LENGTH);
> > + goto out;
> > + }
> > + /* first check the sum of all events */
> > + if (!sccb_events_ok(sccb)) {
> > + goto out;
> > + }
> > + /* then execute */
> > + sccb->h.response_code = cpu_to_be16(SCLP_RC_NORMAL_COMPLETION);
> > + handle_sccb_write_events(ef, sccb);
> > +
> > +out:
> > + return 0;
>
> This always returns 0? Let me guess - you originally wanted to pass response_code as return value! ;)
>
> I think you get the pattern by now :)
>
OK
> > +}
> > +
> > +static void handle_sccb_read_events(SCLPEventFacility *ef, SCCB *sccb,
> > + unsigned int mask)
> > +{
> > + int slen;
> > + unsigned elen = 0;
> > + BusChild *kid;
> > + SCLPEvent *event;
> > + SCLPEventClass *ec;
> > + EventBufferHeader *event_buf;
> > + ReadEventData *red = (ReadEventData *) sccb;
> > +
> > + event_buf = &red->ebh;
> > + event_buf->length = 0;
> > + slen = sizeof(sccb->data);
> > + QTAILQ_FOREACH(kid, &ef->sbus.qbus.children, sibling) {
> > + DeviceState *qdev = kid->child;
> > + event = (SCLPEvent *) qdev;
> > + ec = SCLP_EVENT_GET_CLASS(event);
> > +
> > + if (mask & ec->get_send_mask()) {
> > + if (ec->read_event_data(event, event_buf, &slen)) {
> > + sccb->h.response_code = cpu_to_be16(SCLP_RC_NORMAL_COMPLETION);
>
> What is the response code when we don't have any data?
>
SCLP_RC_NO_EVENT_BUFFERS_STORED (moved now from parent to this function)
> > + }
> > + }
> > + elen = be16_to_cpu(event_buf->length);
> > + event_buf = (void *) event_buf + elen;
> > + }
> > +
> > + if (sccb->h.control_mask[2] & SCLP_VARIABLE_LENGTH_RESPONSE) {
>
> This block deserves a comment.
>
OK
> > + sccb->h.control_mask[2] &= ~SCLP_VARIABLE_LENGTH_RESPONSE;
> > + sccb->h.length = cpu_to_be16(SCCB_SIZE - slen);
> > + }
> > +}
> > +
> > +static int read_event_data(SCLPEventFacility *ef, SCCB *sccb)
> > +{
> > + unsigned int sclp_active_selection_mask;
> > + unsigned int sclp_cp_receive_mask;
> > +
> > + ReadEventData *red = (ReadEventData *) sccb;
> > +
> > + if (be16_to_cpu(sccb->h.length) != SCCB_SIZE) {
> > + sccb->h.response_code = cpu_to_be16(SCLP_RC_INSUFFICIENT_SCCB_LENGTH);
> > + goto out;
> > + }
> > +
> > + sclp_cp_receive_mask = ef->receive_mask;
> > +
> > + /* get active selection mask */
> > + switch (sccb->h.function_code) {
> > + case SCLP_UNCONDITIONAL_READ:
> > + sclp_active_selection_mask = sclp_cp_receive_mask;
> > + break;
> > + case SCLP_SELECTIVE_READ:
> > + if (!(sclp_cp_receive_mask & be32_to_cpu(red->mask))) {
> > + sccb->h.response_code =
> > + cpu_to_be16(SCLP_RC_INVALID_SELECTION_MASK);
> > + goto out;
> > + }
> > + sclp_active_selection_mask = be32_to_cpu(red->mask);
> > + break;
> > + default:
> > + sccb->h.response_code = cpu_to_be16(SCLP_RC_INVALID_FUNCTION);
> > + goto out;
> > + }
> > +
> > + sccb->h.response_code = cpu_to_be16(SCLP_RC_NO_EVENT_BUFFERS_STORED);
> > + handle_sccb_read_events(ef, sccb, sclp_active_selection_mask);
> > +
> > +out:
> > + return 0;
> > +}
> > +
> > +static int write_event_mask(SCLPEventFacility *ef, SCCB *sccb)
> > +{
> > + WriteEventMask *we_mask = (WriteEventMask *) sccb;
> > +
> > + /* Attention: We assume that Linux uses 4-byte masks, what it actually
> > + does. Architecture allows for masks of variable size, though */
> > + if (be16_to_cpu(we_mask->mask_length) != 4) {
> > + sccb->h.response_code = cpu_to_be16(SCLP_RC_INVALID_MASK_LENGTH);
> > + goto out;
> > + }
> > +
> > + /* keep track of the guest's capability masks */
> > + ef->receive_mask = be32_to_cpu(we_mask->cp_receive_mask);
> > +
> > + /* return the SCLP's capability masks to the guest */
> > + we_mask->send_mask = cpu_to_be32(get_host_send_mask(ef));
> > + we_mask->receive_mask = cpu_to_be32(get_host_receive_mask(ef));
> > +
> > + sccb->h.response_code = cpu_to_be16(SCLP_RC_NORMAL_COMPLETION);
> > +
> > +out:
> > + return 0;
> > +}
> > +
> > +/* qemu object creation and initialization functions */
> > +
> > +#define TYPE_SCLP_EVENTS_BUS "s390-sclp-events-bus"
> > +#define SCLP_EVENTS_BUS(obj) OBJECT_CHECK(SCLPS390Bus, (obj),\
> > + TYPE_SCLP_EVENTS_BUS)
> > +
> > +static void sclp_events_bus_class_init(ObjectClass *klass, void *data)
> > +{
> > +}
> > +
> > +static const TypeInfo s390_sclp_events_bus_info = {
> > + .name = TYPE_SCLP_EVENTS_BUS,
> > + .parent = TYPE_BUS,
> > + .instance_size = sizeof(SCLPS390Bus),
> > + .class_init = sclp_events_bus_class_init,
> > +};
> > +
> > +static int command_handler(SCLPEventFacility *ef, SCCB *sccb, uint64_t code)
> > +{
> > + int r = 0;
> > +
> > + switch (code) {
> > + case SCLP_CMD_READ_EVENT_DATA:
> > + r = read_event_data(ef, sccb);
> > + break;
> > + case SCLP_CMD_WRITE_EVENT_DATA:
> > + r = write_event_data(ef, sccb);
> > + break;
> > + case SCLP_CMD_WRITE_EVENT_MASK:
> > + r = write_event_mask(ef, sccb);
> > + break;
> > + default:
> > +#ifdef DEBUG_HELPER
>
> Why DEBUG_HELPER?
OK, removed
>
> > + printf("KVM: unhandled sclp code 0x%" PRIx64 "x\n", code);
> > +#endif
> > + sccb->h.response_code = cpu_to_be16(SCLP_RC_INVALID_SCLP_COMMAND);
> > + break;
> > + }
> > + return r;
> > +}
> > +
> > +static int init_event_facility(S390SCLPDevice *sdev)
> > +{
> > + SCLPEventFacility *event_facility;
> > +
> > + event_facility = g_malloc0(sizeof(SCLPEventFacility));
> > + sdev->instance = event_facility;
> > + sdev->sclp_command_handler = command_handler;
> > + sdev->event_pending = event_pending;
> > +
> > + /* Spawn a new sclp-events facility */
> > + qbus_create_inplace(&event_facility->sbus.qbus,
> > + TYPE_SCLP_EVENTS_BUS, (DeviceState *)sdev, NULL);
> > + event_facility->sbus.qbus.allow_hotplug = 0;
> > + event_facility->sbus.event_facility = event_facility;
> > + event_facility->qdev = (DeviceState *) sdev;
> > +
> > + return 0;
> > +}
> > +
> > +static void init_event_facility_class(ObjectClass *klass, void *data)
> > +{
> > + S390SCLPDeviceClass *k = SCLP_S390_DEVICE_CLASS(klass);
> > +
> > + k->init = init_event_facility;
> > +}
> > +
> > +static TypeInfo s390_sclp_event_facility_info = {
> > + .name = "s390-sclp-event-facility",
> > + .parent = TYPE_DEVICE_S390_SCLP,
> > + .instance_size = sizeof(S390SCLPDevice),
> > + .class_init = init_event_facility_class,
> > +};
> > +
> > +static int event_qdev_init(DeviceState *qdev)
> > +{
> > + SCLPEvent *event = DO_UPCAST(SCLPEvent, qdev, qdev);
> > + SCLPEventClass *child = SCLP_EVENT_GET_CLASS(event);
> > +
> > + return child->init(event);
> > +}
> > +
> > +static int event_qdev_exit(DeviceState *qdev)
> > +{
> > + SCLPEvent *event = DO_UPCAST(SCLPEvent, qdev, qdev);
> > + SCLPEventClass *child = SCLP_EVENT_GET_CLASS(event);
> > + if (child->exit) {
> > + child->exit(event);
> > + }
> > + return 0;
> > +}
> > +
> > +static void event_class_init(ObjectClass *klass, void *data)
> > +{
> > + DeviceClass *dc = DEVICE_CLASS(klass);
> > +
> > + dc->bus_type = TYPE_SCLP_EVENTS_BUS;
> > + dc->unplug = qdev_simple_unplug_cb;
> > + dc->init = event_qdev_init;
> > + dc->exit = event_qdev_exit;
> > +}
> > +
> > +static TypeInfo s390_sclp_event_type_info = {
> > + .name = TYPE_SCLP_EVENT,
> > + .parent = TYPE_DEVICE,
> > + .instance_size = sizeof(SCLPEvent),
> > + .class_init = event_class_init,
> > + .class_size = sizeof(SCLPEventClass),
> > + .abstract = true,
> > +};
> > +
> > +static void register_types(void)
> > +{
> > + type_register_static(&s390_sclp_events_bus_info);
> > + type_register_static(&s390_sclp_event_facility_info);
> > + type_register_static(&s390_sclp_event_type_info);
> > +}
> > +type_init(register_types)
> > diff --git a/hw/s390x/event-facility.h b/hw/s390x/event-facility.h
> > new file mode 100644
> > index 0000000..1e022a3
> > --- /dev/null
> > +++ b/hw/s390x/event-facility.h
> > @@ -0,0 +1,107 @@
> > +/*
> > + * SCLP
> > + * Event Facility definitions
> > + *
> > + * Copyright IBM, Corp. 2012
> > + *
> > + * Authors:
> > + * Heinz Graalfs <graalfs@de.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_S390_SCLP_EVENT_FACILITY_H
> > +#define HW_S390_SCLP_EVENT_FACILITY_H
> > +
> > +#include <hw/qdev.h>
> > +#include "qemu-thread.h"
> > +
> > +/* SCLP event types */
> > +#define SCLP_EVENT_ASCII_CONSOLE_DATA 0x1a
> > +#define SCLP_EVENT_SIGNAL_QUIESCE 0x1d
> > +
> > +/* SCLP event masks */
> > +#define SCLP_EVENT_MASK_SIGNAL_QUIESCE 0x00000008
> > +#define SCLP_EVENT_MASK_MSG_ASCII 0x00000040
> > +
> > +#define SCLP_UNCONDITIONAL_READ 0x00
> > +#define SCLP_SELECTIVE_READ 0x01
> > +
> > +#define TYPE_SCLP_EVENT "s390-sclp-event-type"
> > +#define SCLP_EVENT(obj) \
> > + OBJECT_CHECK(SCLPEvent, (obj), TYPE_SCLP_EVENT)
> > +#define SCLP_EVENT_CLASS(klass) \
> > + OBJECT_CLASS_CHECK(SCLPEventClass, (klass), TYPE_SCLP_EVENT)
> > +#define SCLP_EVENT_GET_CLASS(obj) \
> > + OBJECT_GET_CLASS(SCLPEventClass, (obj), TYPE_SCLP_EVENT)
> > +
> > +typedef struct WriteEventMask {
> > + SCCBHeader h;
> > + uint16_t _reserved;
> > + uint16_t mask_length;
> > + uint32_t cp_receive_mask;
> > + uint32_t cp_send_mask;
> > + uint32_t send_mask;
> > + uint32_t receive_mask;
> > +} QEMU_PACKED WriteEventMask;
> > +
> > +typedef struct EventBufferHeader {
> > + uint16_t length;
> > + uint8_t type;
> > + uint8_t flags;
> > + uint16_t _reserved;
> > +} QEMU_PACKED EventBufferHeader;
> > +
> > +typedef struct WriteEventData {
> > + SCCBHeader h;
> > + EventBufferHeader ebh;
> > +} QEMU_PACKED WriteEventData;
> > +
> > +typedef struct ReadEventData {
> > + SCCBHeader h;
> > + EventBufferHeader ebh;
> > + uint32_t mask;
> > +} QEMU_PACKED ReadEventData;
> > +
> > +typedef struct SCLPEvent {
> > + DeviceState qdev;
> > + QemuMutex lock;
> > + bool event_pending;
> > + uint32_t event_type;
> > + char *name;
> > +} SCLPEvent;
> > +
> > +typedef struct SCLPEventClass {
> > + DeviceClass parent_class;
> > + int (*init)(SCLPEvent *event);
> > + int (*exit)(SCLPEvent *event);
> > +
> > + /* get SCLP's send mask */
> > + unsigned int (*get_send_mask)(void);
> > +
> > + /* get SCLP's receive mask */
> > + unsigned int (*get_receive_mask)(void);
> > +
> > + int (*read_event_data)(SCLPEvent *event, EventBufferHeader *evt_buf_hdr,
> > + int *slen);
> > +
> > + int (*write_event_data)(SCLPEvent *event, EventBufferHeader *evt_buf_hdr);
> > +
> > + /* returns the supported event type */
> > + int (*event_type)(void);
> > +
> > +} SCLPEventClass;
> > +
> > +static inline void lock(SCLPEvent *event)
> > +{
> > + qemu_mutex_lock(&event->lock);
>
> Yeah, I'm fairly sure you don't need the locks :). It might be nice to keep them in as documentation, but make them noops (and name them properly).
>
ok, locking removed
> > +}
> > +
> > +static inline void unlock(SCLPEvent *event)
> > +{
> > + qemu_mutex_unlock(&event->lock);
> > +}
> > +
> > +#endif
> > diff --git a/hw/s390x/sclp.c b/hw/s390x/sclp.c
> > index 4095ba6..cfc41ea 100644
> > --- a/hw/s390x/sclp.c
> > +++ b/hw/s390x/sclp.c
> > @@ -39,6 +39,7 @@ static int read_SCP_info(SCCB *sccb)
> > static int sclp_execute(SCCB *sccb, uint64_t code)
> > {
> > int r = 0;
> > + SCLPEventFacility *ef = sbus->event_facility->instance;
> >
> > switch (code) {
> > case SCLP_CMDW_READ_SCP_INFO:
> > @@ -46,7 +47,7 @@ static int sclp_execute(SCCB *sccb, uint64_t code)
> > r = read_SCP_info(sccb);
> > break;
> > default:
> > - sccb->h.response_code = cpu_to_be16(SCLP_RC_INVALID_SCLP_COMMAND);
> > + r = sbus->event_facility->sclp_command_handler(ef, sccb, code);
> > break;
> > }
> > return r;
> > @@ -87,10 +88,13 @@ out:
> >
> > void sclp_service_interrupt(uint32_t sccb)
> > {
> > - if (!sccb) {
> > + SCLPEventFacility *ef = sbus->event_facility->instance;
> > + int event_pending = sbus->event_facility->event_pending(ef);
> > +
> > + if (!event_pending && !sccb) {
>
> Uh. So when there's no event pending no sclp call may trigger an interrupt?
>
> > return;
> > }
> > - s390_sclp_extint(sccb & ~3);
> > + s390_sclp_extint((sccb & ~3) + event_pending);
>
> event_pending returns a bool, right? Please make this code a bit less magical :).
>
>
>
> Alex
>
> > }
> >
> > /* qemu object creation and initialization functions */
> > @@ -112,6 +116,9 @@ SCLPS390Bus *s390_sclp_bus_init(void)
> > bus_state = qbus_create(TYPE_S390_SCLP_BUS, dev, NULL);
> > bus_state->allow_hotplug = 0;
> >
> > + dev = qdev_create(bus_state, "s390-sclp-event-facility");
> > + qdev_init_nofail(dev);
> > +
> > sbus = DO_UPCAST(SCLPS390Bus, bus, bus_state);
> > return sbus;
> > }
> > @@ -137,9 +144,44 @@ static TypeInfo s390_sclp_bridge_info = {
> > .class_init = s390_sclp_bridge_class_init,
> > };
> >
> > +static int s390_sclp_busdev_init(DeviceState *dev)
> > +{
> > + int r;
> > + S390SCLPDevice *sdev = (S390SCLPDevice *)dev;
> > + S390SCLPDeviceClass *sclp = SCLP_S390_DEVICE_GET_CLASS(dev);
> > + SCLPS390Bus *bus = DO_UPCAST(SCLPS390Bus, bus, sdev->qdev.parent_bus);
> > +
> > + r = sclp->init(sdev);
> > + if (!r) {
> > + assert(sdev->event_pending);
> > + assert(sdev->sclp_command_handler);
> > + }
> > + bus->event_facility = sdev;
> > +
> > + return r;
> > +}
> > +
> > +static void s390_sclp_device_class_init(ObjectClass *klass, void *data)
> > +{
> > + DeviceClass *dc = DEVICE_CLASS(klass);
> > +
> > + dc->init = s390_sclp_busdev_init;
> > + dc->bus_type = TYPE_S390_SCLP_BUS;
> > +}
> > +
> > +static TypeInfo s390_sclp_device_info = {
> > + .name = TYPE_DEVICE_S390_SCLP,
> > + .parent = TYPE_DEVICE,
> > + .instance_size = sizeof(S390SCLPDevice),
> > + .class_init = s390_sclp_device_class_init,
> > + .class_size = sizeof(S390SCLPDeviceClass),
> > + .abstract = true,
> > +};
> > +
> > static void s390_sclp_register_types(void)
> > {
> > type_register_static(&s390_sclp_bridge_info);
> > type_register_static(&s390_sclp_bus_info);
> > + type_register_static(&s390_sclp_device_info);
> > }
> > type_init(s390_sclp_register_types)
> > diff --git a/hw/s390x/sclp.h b/hw/s390x/sclp.h
> > index 4d5a946..d071ccd 100644
> > --- a/hw/s390x/sclp.h
> > +++ b/hw/s390x/sclp.h
> > @@ -19,15 +19,35 @@
> > /* SCLP command codes */
> > #define SCLP_CMDW_READ_SCP_INFO 0x00020001
> > #define SCLP_CMDW_READ_SCP_INFO_FORCED 0x00120001
> > +#define SCLP_CMD_READ_EVENT_DATA 0x00770005
> > +#define SCLP_CMD_WRITE_EVENT_DATA 0x00760005
> > +#define SCLP_CMD_READ_EVENT_DATA 0x00770005
> > +#define SCLP_CMD_WRITE_EVENT_DATA 0x00760005
> > +#define SCLP_CMD_WRITE_EVENT_MASK 0x00780005
> >
> > /* SCLP response codes */
> > #define SCLP_RC_NORMAL_READ_COMPLETION 0x0010
> > +#define SCLP_RC_NORMAL_COMPLETION 0x0020
> > #define SCLP_RC_INVALID_SCLP_COMMAND 0x01f0
> > +#define SCLP_RC_CONTAINED_EQUIPMENT_CHECK 0x0340
> > +#define SCLP_RC_INSUFFICIENT_SCCB_LENGTH 0x0300
> > +#define SCLP_RC_INVALID_FUNCTION 0x40f0
> > +#define SCLP_RC_NO_EVENT_BUFFERS_STORED 0x60f0
> > +#define SCLP_RC_INVALID_SELECTION_MASK 0x70f0
> > +#define SCLP_RC_INCONSISTENT_LENGTHS 0x72f0
> > +#define SCLP_RC_EVENT_BUFFER_SYNTAX_ERROR 0x73f0
> > +#define SCLP_RC_INVALID_MASK_LENGTH 0x74f0
> > +
> >
> > /* Service Call Control Block (SCCB) and its elements */
> >
> > #define SCCB_SIZE 4096
> >
> > +#define SCLP_VARIABLE_LENGTH_RESPONSE 0x80
> > +#define SCLP_EVENT_BUFFER_ACCEPTED 0x80
> > +
> > +#define SCLP_FC_NORMAL_WRITE 0
> > +
> > /*
> > * Normally packed structures are not the right thing to do, since all code
> > * must take care of endianess. We cant use ldl_phys and friends for two
> > @@ -64,14 +84,38 @@ typedef struct SCCB {
> > #define TYPE_S390_SCLP_BUS "s390-sclp-bus"
> > #define S390_SCLP_BUS(obj) OBJECT_CHECK(SCLPS390Bus, (obj), TYPE_S390_SCLP_BUS)
> >
> > +#define TYPE_DEVICE_S390_SCLP "s390-sclp-device"
> > +#define SCLP_S390_DEVIVE(obj) \
> > + OBJECT_CHECK(S390SCLPDevice, (obj), TYPE_DEVICE_S390_SCLP)
> > +#define SCLP_S390_DEVICE_CLASS(klass) \
> > + OBJECT_CLASS_CHECK(S390SCLPDeviceClass, (klass), \
> > + TYPE_DEVICE_S390_SCLP)
> > +#define SCLP_S390_DEVICE_GET_CLASS(obj) \
> > + OBJECT_GET_CLASS(S390SCLPDeviceClass, (obj), \
> > + TYPE_DEVICE_S390_SCLP)
> > +
> > +typedef struct SCLPEventFacility SCLPEventFacility;
> > +
> > typedef struct S390SCLPDevice {
> > DeviceState qdev;
> > + SCLPEventFacility *instance;
> > + int (*sclp_command_handler)(SCLPEventFacility *ef, SCCB *sccb,
> > + uint64_t code);
> > + bool (*event_pending)(SCLPEventFacility *ef);
> > } S390SCLPDevice;
> >
> > typedef struct SCLPS390Bus {
> > BusState bus;
> > + S390SCLPDevice *event_facility;
> > } SCLPS390Bus;
> >
> > +typedef struct S390SCLPDeviceClass {
> > + DeviceClass qdev;
> > +
> > + int (*init)(S390SCLPDevice *sdev);
> > +
> > +} S390SCLPDeviceClass;
> > +
> > SCLPS390Bus *s390_sclp_bus_init(void);
> >
> > void sclp_service_interrupt(uint32_t sccb);
> > --
> > 1.7.0.1
> >
>
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [Qemu-devel] [PATCH 4/7] s390: sclp event support
2012-07-24 7:37 ` [Qemu-devel] [PATCH 4/7] s390: sclp event support Christian Borntraeger
2012-07-30 13:24 ` Alexander Graf
@ 2012-07-31 12:59 ` Andreas Färber
2012-08-03 12:31 ` Christian Borntraeger
2012-08-20 12:14 ` Heinz Graalfs
1 sibling, 2 replies; 33+ messages in thread
From: Andreas Färber @ 2012-07-31 12:59 UTC (permalink / raw)
To: Christian Borntraeger
Cc: blauwirbel, Heinz Graalfs, Alexander Graf, Jens Freimann, qemu-devel
Am 24.07.2012 09:37, schrieb Christian Borntraeger:
> From: Heinz Graalfs <graalfs@linux.vnet.ibm.com>
>
> Several SCLP features are considered to be events. Those events don't
> provide SCLP commands on their own, instead they are all based on
> Read Event Data, Write Event Data, Write Event Mask and the service
> interrupt. Follow-on patches will provide SCLP's Signal Quiesce (via
> system_powerdown) and the ASCII console.
> Further down the road the sclp line mode console and configuration
> change events (e.g. cpu hotplug) can be implemented.
>
> Signed-off-by: Heinz Graalfs <graalfs@linux.vnet.ibm.com>
> Signed-off-by: Christian Borntraeger <borntraeger@de.ibm.com>
> ---
> hw/s390x/Makefile.objs | 1 +
> hw/s390x/event-facility.c | 390 +++++++++++++++++++++++++++++++++++++++++++++
> hw/s390x/event-facility.h | 107 ++++++++++++
> hw/s390x/sclp.c | 48 +++++-
> hw/s390x/sclp.h | 44 +++++
> 5 files changed, 587 insertions(+), 3 deletions(-)
> create mode 100644 hw/s390x/event-facility.c
> create mode 100644 hw/s390x/event-facility.h
>
> diff --git a/hw/s390x/Makefile.objs b/hw/s390x/Makefile.objs
> index 1c14b96..b32fc52 100644
> --- a/hw/s390x/Makefile.objs
> +++ b/hw/s390x/Makefile.objs
> @@ -2,3 +2,4 @@ obj-y = s390-virtio-bus.o s390-virtio.o
>
> obj-y := $(addprefix ../,$(obj-y))
> obj-y += sclp.o
> +obj-y += event-facility.o
> diff --git a/hw/s390x/event-facility.c b/hw/s390x/event-facility.c
> new file mode 100644
> index 0000000..74a3514
> --- /dev/null
> +++ b/hw/s390x/event-facility.c
> @@ -0,0 +1,390 @@
> +/*
> + * SCLP
> + * Event Facility
> + * handles SCLP event types
> + * - Signal Quiesce - system power down
> + * - ASCII Console Data - VT220 read and write
> + *
> + * Copyright IBM, Corp. 2012
> + *
> + * Authors:
> + * Heinz Graalfs <graalfs@de.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 "monitor.h"
> +#include "sysemu.h"
> +
> +#include "sclp.h"
> +#include "event-facility.h"
> +
> +typedef struct EventTypes {
> + BusState qbus;
> + SCLPEventFacility *event_facility;
> +} EventTypes;
> +
> +struct SCLPEventFacility {
> + EventTypes sbus;
> + DeviceState *qdev;
> + /* guest' receive mask */
> + unsigned int receive_mask;
> +};
The naming here strikes me as particularly odd...
IIUC this Event Facility is a device sitting on the SCLP bus.
But why does it expose a bus named "EventTypes"? Busses are usually
named ...Bus (PCIBus, IDEBus, etc.). So is this actually a bus? If not,
and if all you need is an SCLP-specific list API, maybe compare the
sPAPR hypercall registration API.
Regards,
Andreas
--
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg
^ permalink raw reply [flat|nested] 33+ messages in thread* Re: [Qemu-devel] [PATCH 4/7] s390: sclp event support
2012-07-31 12:59 ` Andreas Färber
@ 2012-08-03 12:31 ` Christian Borntraeger
2012-08-20 12:14 ` Heinz Graalfs
1 sibling, 0 replies; 33+ messages in thread
From: Christian Borntraeger @ 2012-08-03 12:31 UTC (permalink / raw)
To: Andreas Färber
Cc: blauwirbel, Heinz Graalfs, Alexander Graf, Jens Freimann, qemu-devel
On 31/07/12 14:59, Andreas Färber wrote:
>> +typedef struct EventTypes {
>> + BusState qbus;
>> + SCLPEventFacility *event_facility;
>> +} EventTypes;
>> +
>> +struct SCLPEventFacility {
>> + EventTypes sbus;
>> + DeviceState *qdev;
>> + /* guest' receive mask */
>> + unsigned int receive_mask;
>> +};
>
> The naming here strikes me as particularly odd...
>
> IIUC this Event Facility is a device sitting on the SCLP bus.
>
> But why does it expose a bus named "EventTypes"? Busses are usually
> named ...Bus (PCIBus, IDEBus, etc.). So is this actually a bus? If not,
> and if all you need is an SCLP-specific list API, maybe compare the
> sPAPR hypercall registration API.
So let me explain it and then we can see what is the right thing to do:
The sclp itself is a service processor that will handle service calls and
can send service interrupts. Most service calls deal with one specific thing:
reading info, cpu hotplug, memory hotplug etc.
Some of the service calls are special, because they form an event subsystem:
Events are features that can notify the guest asynchronously.
(e.g. system_powerdown is wired to signal quiesce which will be seen as
ctrl-alt-del in the guest, or several console types where the input is sent
as an event).
The service calls are read_event_data, write_event_data and write_event_mask.
write_event_mask is used by the guest to notify the host
about its event capabilities and to the query the host events.
read_event_data allows a guest to get event data - iow host2guest data.
guest2host traffic also goes via the event mechanism, e.g. console output
is send via write_event_data.
So each event implements several callbacks: event_pending to tell that
this event is pending, read event data to fill in the buffer with the event data,
write_event_data if the event allows guest2host traffic. There are also two bits
per event that tell if that specific event will allow read/write event data.
Since some of the events implement complex things (console) a bus seemed appropriate,
but there are of course other ways of implementing.
Comments?
Christian
^ permalink raw reply [flat|nested] 33+ messages in thread* Re: [Qemu-devel] [PATCH 4/7] s390: sclp event support
2012-07-31 12:59 ` Andreas Färber
2012-08-03 12:31 ` Christian Borntraeger
@ 2012-08-20 12:14 ` Heinz Graalfs
1 sibling, 0 replies; 33+ messages in thread
From: Heinz Graalfs @ 2012-08-20 12:14 UTC (permalink / raw)
To: Andreas Färber
Cc: blauwirbel, Christian Borntraeger, Jens Freimann, Alexander Graf,
qemu-devel
On Tue, 2012-07-31 at 14:59 +0200, Andreas Färber wrote:
> Am 24.07.2012 09:37, schrieb Christian Borntraeger:
> > From: Heinz Graalfs <graalfs@linux.vnet.ibm.com>
> >
> > Several SCLP features are considered to be events. Those events don't
> > provide SCLP commands on their own, instead they are all based on
> > Read Event Data, Write Event Data, Write Event Mask and the service
> > interrupt. Follow-on patches will provide SCLP's Signal Quiesce (via
> > system_powerdown) and the ASCII console.
> > Further down the road the sclp line mode console and configuration
> > change events (e.g. cpu hotplug) can be implemented.
> >
> > Signed-off-by: Heinz Graalfs <graalfs@linux.vnet.ibm.com>
> > Signed-off-by: Christian Borntraeger <borntraeger@de.ibm.com>
> > ---
> > hw/s390x/Makefile.objs | 1 +
> > hw/s390x/event-facility.c | 390 +++++++++++++++++++++++++++++++++++++++++++++
> > hw/s390x/event-facility.h | 107 ++++++++++++
> > hw/s390x/sclp.c | 48 +++++-
> > hw/s390x/sclp.h | 44 +++++
> > 5 files changed, 587 insertions(+), 3 deletions(-)
> > create mode 100644 hw/s390x/event-facility.c
> > create mode 100644 hw/s390x/event-facility.h
> >
> > diff --git a/hw/s390x/Makefile.objs b/hw/s390x/Makefile.objs
> > index 1c14b96..b32fc52 100644
> > --- a/hw/s390x/Makefile.objs
> > +++ b/hw/s390x/Makefile.objs
> > @@ -2,3 +2,4 @@ obj-y = s390-virtio-bus.o s390-virtio.o
> >
> > obj-y := $(addprefix ../,$(obj-y))
> > obj-y += sclp.o
> > +obj-y += event-facility.o
> > diff --git a/hw/s390x/event-facility.c b/hw/s390x/event-facility.c
> > new file mode 100644
> > index 0000000..74a3514
> > --- /dev/null
> > +++ b/hw/s390x/event-facility.c
> > @@ -0,0 +1,390 @@
> > +/*
> > + * SCLP
> > + * Event Facility
> > + * handles SCLP event types
> > + * - Signal Quiesce - system power down
> > + * - ASCII Console Data - VT220 read and write
> > + *
> > + * Copyright IBM, Corp. 2012
> > + *
> > + * Authors:
> > + * Heinz Graalfs <graalfs@de.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 "monitor.h"
> > +#include "sysemu.h"
> > +
> > +#include "sclp.h"
> > +#include "event-facility.h"
> > +
> > +typedef struct EventTypes {
> > + BusState qbus;
> > + SCLPEventFacility *event_facility;
> > +} EventTypes;
> > +
> > +struct SCLPEventFacility {
> > + EventTypes sbus;
> > + DeviceState *qdev;
> > + /* guest' receive mask */
> > + unsigned int receive_mask;
> > +};
>
> The naming here strikes me as particularly odd...
>
> IIUC this Event Facility is a device sitting on the SCLP bus.
>
> But why does it expose a bus named "EventTypes"? Busses are usually
> named ...Bus (PCIBus, IDEBus, etc.). So is this actually a bus? If not,
> and if all you need is an SCLP-specific list API, maybe compare the
> sPAPR hypercall registration API.
>
ok, the Event Facility is now on the main system bus.
We keep the renamed bus EventTypesBus for the sclpquiesce and
sclpconsole (...) items
> Regards,
> Andreas
>
^ permalink raw reply [flat|nested] 33+ messages in thread
* [Qemu-devel] [PATCH 5/7] s390: sclp signal quiesce support
2012-07-24 7:37 [Qemu-devel] [PATCH 0/7v3] s390: sclp patch set Christian Borntraeger
` (3 preceding siblings ...)
2012-07-24 7:37 ` [Qemu-devel] [PATCH 4/7] s390: sclp event support Christian Borntraeger
@ 2012-07-24 7:37 ` Christian Borntraeger
2012-07-24 7:37 ` [Qemu-devel] [PATCH 6/7] s390: sclp ascii console support Christian Borntraeger
` (2 subsequent siblings)
7 siblings, 0 replies; 33+ messages in thread
From: Christian Borntraeger @ 2012-07-24 7:37 UTC (permalink / raw)
To: Alexander Graf
Cc: Heinz Graalfs, qemu-devel, blauwirbel, Christian Borntraeger,
Jens Freimann, afaerber
From: Heinz Graalfs <graalfs@linux.vnet.ibm.com>
This implements the sclp signal quiesce event via the SCLP Event
Facility.
This allows to gracefully shutdown a guest by using system_powerdown.
It creates a service interrupt that will trigger a Read Event Data
command from the guest. This code will then add an event that is
interpreted by linux guests as ctrl-alt-del.
Signed-off-by: Heinz Graalfs <graalfs@linux.vnet.ibm.com>
Signed-off-by: Christian Borntraeger <borntraeger@de.ibm.com>
---
hw/s390x/Makefile.objs | 1 +
hw/s390x/event-facility.c | 7 +++
hw/s390x/sclpquiesce.c | 112 +++++++++++++++++++++++++++++++++++++++++++++
3 files changed, 120 insertions(+), 0 deletions(-)
create mode 100644 hw/s390x/sclpquiesce.c
diff --git a/hw/s390x/Makefile.objs b/hw/s390x/Makefile.objs
index b32fc52..ed4e61a 100644
--- a/hw/s390x/Makefile.objs
+++ b/hw/s390x/Makefile.objs
@@ -3,3 +3,4 @@ obj-y = s390-virtio-bus.o s390-virtio.o
obj-y := $(addprefix ../,$(obj-y))
obj-y += sclp.o
obj-y += event-facility.o
+obj-y += sclpquiesce.o
diff --git a/hw/s390x/event-facility.c b/hw/s390x/event-facility.c
index 74a3514..db45301 100644
--- a/hw/s390x/event-facility.c
+++ b/hw/s390x/event-facility.c
@@ -314,6 +314,7 @@ static int command_handler(SCLPEventFacility *ef, SCCB *sccb, uint64_t code)
static int init_event_facility(S390SCLPDevice *sdev)
{
SCLPEventFacility *event_facility;
+ DeviceState *quiesce;
event_facility = g_malloc0(sizeof(SCLPEventFacility));
sdev->instance = event_facility;
@@ -327,6 +328,12 @@ static int init_event_facility(S390SCLPDevice *sdev)
event_facility->sbus.event_facility = event_facility;
event_facility->qdev = (DeviceState *) sdev;
+ quiesce = qdev_create(&event_facility->sbus.qbus, "sclpquiesce");
+ if (!quiesce) {
+ return -1;
+ }
+ qdev_init_nofail(quiesce);
+
return 0;
}
diff --git a/hw/s390x/sclpquiesce.c b/hw/s390x/sclpquiesce.c
new file mode 100644
index 0000000..367ac4b
--- /dev/null
+++ b/hw/s390x/sclpquiesce.c
@@ -0,0 +1,112 @@
+/*
+ * SCLP event type
+ * Signal Quiesce - trigger system powerdown request
+ *
+ * Copyright IBM, Corp. 2012
+ *
+ * Authors:
+ * Heinz Graalfs <graalfs@de.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/qdev.h>
+#include "sysemu.h"
+#include "sclp.h"
+#include "event-facility.h"
+
+typedef struct SignalQuiesce {
+ EventBufferHeader ebh;
+ uint16_t timeout;
+ uint8_t unit;
+} QEMU_PACKED SignalQuiesce;
+
+static int event_type(void)
+{
+ return SCLP_EVENT_SIGNAL_QUIESCE;
+}
+
+static unsigned int send_mask(void)
+{
+ return SCLP_EVENT_MASK_SIGNAL_QUIESCE;
+}
+
+static unsigned int receive_mask(void)
+{
+ return 0;
+}
+
+static int read_event_data(SCLPEvent *event, EventBufferHeader *evt_buf_hdr,
+ int *slen)
+{
+ SignalQuiesce *sq = (SignalQuiesce *) evt_buf_hdr;
+
+ if (*slen < sizeof(SignalQuiesce)) {
+ return 0;
+ }
+
+ if (!event->event_pending) {
+ return 0;
+ }
+ event->event_pending = false;
+
+ sq->ebh.length = cpu_to_be16(sizeof(SignalQuiesce));
+ sq->ebh.type = SCLP_EVENT_SIGNAL_QUIESCE;
+ sq->ebh.flags |= SCLP_EVENT_BUFFER_ACCEPTED;
+ /*
+ * system_powerdown does not have a timeout. Fortunately the
+ * timeout value is currently ignored by Linux, anyway
+ */
+ sq->timeout = cpu_to_be16(0);
+ sq->unit = cpu_to_be16(0);
+ *slen -= sizeof(SignalQuiesce);
+
+ return 1;
+}
+
+static void trigger_signal_quiesce(void *opaque, int n, int level)
+{
+ SCLPEvent *event = opaque;
+
+ event->event_pending = true;
+ /* trigger SCLP read operation */
+ sclp_service_interrupt(0);
+}
+
+static int quiesce_init(SCLPEvent *event)
+{
+ event->event_type = SCLP_EVENT_SIGNAL_QUIESCE;
+ qemu_system_powerdown = *qemu_allocate_irqs(trigger_signal_quiesce,
+ event, 1);
+ qemu_mutex_init(&event->lock);
+
+ return 0;
+}
+
+static void quiesce_class_init(ObjectClass *klass, void *data)
+{
+ SCLPEventClass *k = SCLP_EVENT_CLASS(klass);
+
+ k->init = quiesce_init;
+
+ k->get_send_mask = send_mask;
+ k->get_receive_mask = receive_mask;
+ k->event_type = event_type;
+ k->read_event_data = read_event_data;
+ k->write_event_data = NULL;
+}
+
+static TypeInfo sclp_quiesce_info = {
+ .name = "sclpquiesce",
+ .parent = TYPE_SCLP_EVENT,
+ .instance_size = sizeof(SCLPEvent),
+ .class_init = quiesce_class_init,
+ .class_size = sizeof(SCLPEventClass),
+};
+
+static void register_types(void)
+{
+ type_register_static(&sclp_quiesce_info);
+}
+type_init(register_types)
--
1.7.0.1
^ permalink raw reply related [flat|nested] 33+ messages in thread* [Qemu-devel] [PATCH 6/7] s390: sclp ascii console support
2012-07-24 7:37 [Qemu-devel] [PATCH 0/7v3] s390: sclp patch set Christian Borntraeger
` (4 preceding siblings ...)
2012-07-24 7:37 ` [Qemu-devel] [PATCH 5/7] s390: sclp signal quiesce support Christian Borntraeger
@ 2012-07-24 7:37 ` Christian Borntraeger
2012-07-24 19:42 ` Blue Swirl
2012-07-24 7:37 ` [Qemu-devel] [PATCH 7/7] s390: make sclp ascii console the default Christian Borntraeger
2012-07-30 9:00 ` [Qemu-devel] [PATCH 0/7v3] s390: sclp patch set Christian Borntraeger
7 siblings, 1 reply; 33+ messages in thread
From: Christian Borntraeger @ 2012-07-24 7:37 UTC (permalink / raw)
To: Alexander Graf
Cc: Heinz Graalfs, qemu-devel, blauwirbel, Christian Borntraeger,
Jens Freimann, afaerber
From: Heinz Graalfs <graalfs@linux.vnet.ibm.com>
This code adds console support by implementing SCLP's ASCII Console
Data event. This is the same console as LPARs ASCII console or z/VMs
sysascii.
The console can be specified manually with something like
-chardev stdio,id=charconsole0 -device sclpconsole,chardev=charconsole0,id=console0
Newer kernels will autodetect that console and prefer that over virtio
console.
When data is received from the character layer it creates a service
interrupt to trigger a Read Event Data command from the guest that will
pick up the received character byte-stream.
When characters are echo'ed by the linux guest a Write Event Data occurs
which is forwarded by the Event Facility to the console that supports
a corresponding mask value.
Console resizing is not supported.
The character layer byte-stream is buffered using a fixed size iov
buffer.
Signed-off-by: Heinz Graalfs <graalfs@linux.vnet.ibm.com>
Signed-off-by: Christian Borntraeger <borntraeger@de.ibm.com>
---
hw/s390x/Makefile.objs | 2 +-
hw/s390x/sclpconsole.c | 323 ++++++++++++++++++++++++++++++++++++++++++++++++
2 files changed, 324 insertions(+), 1 deletions(-)
create mode 100644 hw/s390x/sclpconsole.c
diff --git a/hw/s390x/Makefile.objs b/hw/s390x/Makefile.objs
index ed4e61a..096dfcd 100644
--- a/hw/s390x/Makefile.objs
+++ b/hw/s390x/Makefile.objs
@@ -3,4 +3,4 @@ obj-y = s390-virtio-bus.o s390-virtio.o
obj-y := $(addprefix ../,$(obj-y))
obj-y += sclp.o
obj-y += event-facility.o
-obj-y += sclpquiesce.o
+obj-y += sclpquiesce.o sclpconsole.o
diff --git a/hw/s390x/sclpconsole.c b/hw/s390x/sclpconsole.c
new file mode 100644
index 0000000..9a57032
--- /dev/null
+++ b/hw/s390x/sclpconsole.c
@@ -0,0 +1,323 @@
+/*
+ * SCLP event type
+ * Ascii Console Data (VT220 Console)
+ *
+ * Copyright IBM, Corp. 2012
+ *
+ * Authors:
+ * Heinz Graalfs <graalfs@de.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/qdev.h>
+#include "qemu-thread.h"
+
+#include "sclp.h"
+#include "event-facility.h"
+
+typedef struct ASCIIConsoleData {
+ EventBufferHeader ebh;
+ char data[0];
+} QEMU_PACKED ASCIIConsoleData;
+
+qemu_irq sclp_read_vt220;
+
+/* max size for ASCII data in 4K SCCB page */
+#define SIZE_BUFFER_VT220 4080
+
+typedef struct SCLPConsole {
+ SCLPEvent event;
+ CharDriverState *chr;
+ /* io vector */
+ uint8_t *iov; /* iov buffer pointer */
+ uint8_t *iov_sclp; /* pointer to SCLP read offset */
+ uint8_t *iov_bs; /* pointer byte stream read offset */
+ uint32_t iov_data_len; /* length of byte stream in buffer */
+ uint32_t iov_sclp_rest; /* length of byte stream not read via SCLP */
+} SCLPConsole;
+
+/* character layer call-back functions */
+
+/* Return number of bytes that fit into iov buffer */
+static int chr_can_read(void *opaque)
+{
+ int can_read;
+ SCLPConsole *scon = opaque;
+
+ qemu_mutex_lock(&scon->event.lock);
+ can_read = SIZE_BUFFER_VT220 - scon->iov_data_len;
+ qemu_mutex_unlock(&scon->event.lock);
+
+ return can_read;
+}
+
+/* Receive n bytes from character layer, save in iov buffer,
+ * and set event pending */
+static void receive_from_chr_layer(void *opaque, const uint8_t *buf, int size)
+{
+ SCLPConsole *scon = opaque;
+
+ assert(scon->iov);
+
+ qemu_mutex_lock(&scon->event.lock);
+
+ /* if new data do not fit into current buffer */
+ if (scon->iov_data_len + size > SIZE_BUFFER_VT220) {
+ /* character layer sent more than allowed */
+ qemu_mutex_unlock(&scon->event.lock);
+ return;
+ }
+ /* put byte-stream from character layer into buffer */
+ memcpy(scon->iov_bs, buf, size);
+ scon->iov_data_len += size;
+ scon->iov_sclp_rest += size;
+ scon->iov_bs += size;
+ scon->event.event_pending = true;
+
+ qemu_mutex_unlock(&scon->event.lock);
+}
+
+/* Send data from a char device over to the guest */
+static void chr_read(void *opaque, const uint8_t *buf, int size)
+{
+ assert(opaque);
+
+ receive_from_chr_layer(opaque, buf, size);
+ /* trigger SCLP read operation */
+ qemu_irq_raise(sclp_read_vt220);
+}
+
+static void chr_event(void *opaque, int event)
+{
+ SCLPConsole *scon = opaque;
+
+ switch (event) {
+ case CHR_EVENT_OPENED:
+ if (!scon->iov) {
+ scon->iov = g_malloc0(SIZE_BUFFER_VT220);
+ scon->iov_sclp = scon->iov;
+ scon->iov_bs = scon->iov;
+ scon->iov_data_len = 0;
+ scon->iov_sclp_rest = 0;
+ }
+ break;
+ case CHR_EVENT_CLOSED:
+ if (scon->iov) {
+ g_free(scon->iov);
+ scon->iov = NULL;
+ }
+ break;
+ }
+}
+
+/* functions to be called by event facility */
+
+static int event_type(void)
+{
+ return SCLP_EVENT_ASCII_CONSOLE_DATA;
+}
+
+static unsigned int send_mask(void)
+{
+ return SCLP_EVENT_MASK_MSG_ASCII;
+}
+
+static unsigned int receive_mask(void)
+{
+ return SCLP_EVENT_MASK_MSG_ASCII;
+}
+
+/* triggered by SCLP's read_event_data -
+ * copy console data byte-stream into provided (SCLP) buffer
+ */
+static void get_console_data(SCLPEvent *event, uint8_t *buf, size_t *size,
+ int avail)
+{
+ SCLPConsole *cons = DO_UPCAST(SCLPConsole, event, event);
+
+ /* first byte is hex 0 saying an ascii string follows */
+ *buf++ = '\0';
+ avail--;
+ /* if all data fit into provided SCLP buffer */
+ if (avail >= cons->iov_sclp_rest) {
+ /* copy character byte-stream to SCLP buffer */
+ memcpy(buf, cons->iov_sclp, cons->iov_sclp_rest);
+ *size = cons->iov_sclp_rest + 1;
+ cons->iov_sclp = cons->iov;
+ cons->iov_bs = cons->iov;
+ cons->iov_data_len = 0;
+ cons->iov_sclp_rest = 0;
+ event->event_pending = false;
+ /* data provided and no more data pending */
+ } else {
+ /* if provided buffer is too small, just copy part */
+ memcpy(buf, cons->iov_sclp, avail);
+ *size = avail + 1;
+ cons->iov_sclp_rest -= avail;
+ cons->iov_sclp += avail;
+ /* more data pending */
+ }
+}
+
+static int read_event_data(SCLPEvent *event, EventBufferHeader *evt_buf_hdr,
+ int *slen)
+{
+ int avail;
+ size_t src_len;
+ uint8_t *to;
+ ASCIIConsoleData *acd = (ASCIIConsoleData *) evt_buf_hdr;
+
+ qemu_mutex_lock(&event->lock);
+
+ if (!event->event_pending) {
+ /* no data pending */
+ qemu_mutex_unlock(&event->lock);
+ return 0;
+ }
+
+ to = (uint8_t *)&acd->data;
+ avail = *slen - sizeof(ASCIIConsoleData);
+ get_console_data(event, to, &src_len, avail);
+
+ acd->ebh.length = cpu_to_be16(sizeof(ASCIIConsoleData) + src_len);
+ acd->ebh.type = SCLP_EVENT_ASCII_CONSOLE_DATA;
+ acd->ebh.flags |= SCLP_EVENT_BUFFER_ACCEPTED;
+ *slen = avail - src_len;
+
+ qemu_mutex_unlock(&event->lock);
+
+ return 1;
+}
+
+/* triggered by SCLP's write_event_data
+ * - write console data into character layer
+ * returns < 0 if an error occured
+ */
+static ssize_t write_console_data(SCLPEvent *event, const uint8_t *buf,
+ size_t len)
+{
+ ssize_t ret = 0;
+ const uint8_t *iov_offset;
+ SCLPConsole *scon = DO_UPCAST(SCLPConsole, event, event);
+
+ if (!scon->chr) {
+ /* If there's no backend, we can just say we consumed all data. */
+ return len;
+ }
+
+ iov_offset = buf;
+ while (len > 0) {
+ ret = qemu_chr_fe_write(scon->chr, buf, len);
+ if (ret == 0) {
+ /* a pty doesn't seem to be connected - no error */
+ len = 0;
+ } else if (ret == -EAGAIN || (ret > 0 && ret < len)) {
+ len -= ret;
+ iov_offset += ret;
+ } else {
+ len = 0;
+ }
+ }
+
+ return ret;
+}
+
+static int write_event_data(SCLPEvent *event, EventBufferHeader *evt_buf_hdr)
+{
+ int rc;
+ int length;
+ ssize_t written;
+ ASCIIConsoleData *acd = (ASCIIConsoleData *) evt_buf_hdr;
+
+ length = be16_to_cpu(evt_buf_hdr->length) - sizeof(EventBufferHeader);
+ written = write_console_data(event, (uint8_t *)acd->data, length);
+
+ rc = SCLP_RC_NORMAL_COMPLETION;
+ /* set event buffer accepted flag */
+ evt_buf_hdr->flags |= SCLP_EVENT_BUFFER_ACCEPTED;
+
+ /* written will be zero if a pty is not connected - don't treat as error */
+ if (written < 0) {
+ /* event buffer not accepted due to error in character layer */
+ evt_buf_hdr->flags &= ~(SCLP_EVENT_BUFFER_ACCEPTED);
+ rc = SCLP_RC_CONTAINED_EQUIPMENT_CHECK;
+ }
+
+ return rc;
+}
+
+static void trigger_ascii_console_data(void *env, int n, int level)
+{
+ sclp_service_interrupt(0);
+}
+
+/* qemu object creation and initialization functions */
+
+/* tell character layer our call-back functions */
+static int console_init(SCLPEvent *event)
+{
+ static bool console_available;
+
+ SCLPConsole *scon = DO_UPCAST(SCLPConsole, event, event);
+
+ if (console_available) {
+ error_report("Multiple VT220 operator consoles are not supported");
+ return -1;
+ }
+ console_available = true;
+ event->event_type = SCLP_EVENT_ASCII_CONSOLE_DATA;
+ if (scon->chr) {
+ qemu_chr_add_handlers(scon->chr, chr_can_read,
+ chr_read, chr_event, scon);
+ }
+ sclp_read_vt220 = *qemu_allocate_irqs(trigger_ascii_console_data,
+ NULL, 1);
+
+ qemu_mutex_init(&event->lock);
+
+ return 0;
+}
+
+static int console_exit(SCLPEvent *event)
+{
+ qemu_mutex_destroy(&event->lock);
+
+ return 0;
+}
+
+static Property console_properties[] = {
+ DEFINE_PROP_CHR("chardev", SCLPConsole, chr),
+ DEFINE_PROP_END_OF_LIST(),
+};
+
+static void console_class_init(ObjectClass *klass, void *data)
+{
+ DeviceClass *dc = DEVICE_CLASS(klass);
+ SCLPEventClass *ec = SCLP_EVENT_CLASS(klass);
+
+ dc->props = console_properties;
+ ec->init = console_init;
+ ec->exit = console_exit;
+ ec->get_send_mask = send_mask;
+ ec->get_receive_mask = receive_mask;
+ ec->event_type = event_type;
+ ec->read_event_data = read_event_data;
+ ec->write_event_data = write_event_data;
+}
+
+static TypeInfo sclp_console_info = {
+ .name = "sclpconsole",
+ .parent = TYPE_SCLP_EVENT,
+ .instance_size = sizeof(SCLPConsole),
+ .class_init = console_class_init,
+ .class_size = sizeof(SCLPEventClass),
+};
+
+static void register_types(void)
+{
+ type_register_static(&sclp_console_info);
+}
+type_init(register_types)
--
1.7.0.1
^ permalink raw reply related [flat|nested] 33+ messages in thread* Re: [Qemu-devel] [PATCH 6/7] s390: sclp ascii console support
2012-07-24 7:37 ` [Qemu-devel] [PATCH 6/7] s390: sclp ascii console support Christian Borntraeger
@ 2012-07-24 19:42 ` Blue Swirl
2012-07-26 8:55 ` [Qemu-devel] [PATCH v4 06/07] " Christian Borntraeger
0 siblings, 1 reply; 33+ messages in thread
From: Blue Swirl @ 2012-07-24 19:42 UTC (permalink / raw)
To: Christian Borntraeger
Cc: qemu-devel, Heinz Graalfs, Alexander Graf, afaerber, Jens Freimann
On Tue, Jul 24, 2012 at 7:37 AM, Christian Borntraeger
<borntraeger@de.ibm.com> wrote:
> From: Heinz Graalfs <graalfs@linux.vnet.ibm.com>
>
> This code adds console support by implementing SCLP's ASCII Console
> Data event. This is the same console as LPARs ASCII console or z/VMs
> sysascii.
>
> The console can be specified manually with something like
> -chardev stdio,id=charconsole0 -device sclpconsole,chardev=charconsole0,id=console0
>
> Newer kernels will autodetect that console and prefer that over virtio
> console.
>
> When data is received from the character layer it creates a service
> interrupt to trigger a Read Event Data command from the guest that will
> pick up the received character byte-stream.
> When characters are echo'ed by the linux guest a Write Event Data occurs
> which is forwarded by the Event Facility to the console that supports
> a corresponding mask value.
> Console resizing is not supported.
> The character layer byte-stream is buffered using a fixed size iov
> buffer.
>
> Signed-off-by: Heinz Graalfs <graalfs@linux.vnet.ibm.com>
> Signed-off-by: Christian Borntraeger <borntraeger@de.ibm.com>
> ---
> hw/s390x/Makefile.objs | 2 +-
> hw/s390x/sclpconsole.c | 323 ++++++++++++++++++++++++++++++++++++++++++++++++
> 2 files changed, 324 insertions(+), 1 deletions(-)
> create mode 100644 hw/s390x/sclpconsole.c
>
> diff --git a/hw/s390x/Makefile.objs b/hw/s390x/Makefile.objs
> index ed4e61a..096dfcd 100644
> --- a/hw/s390x/Makefile.objs
> +++ b/hw/s390x/Makefile.objs
> @@ -3,4 +3,4 @@ obj-y = s390-virtio-bus.o s390-virtio.o
> obj-y := $(addprefix ../,$(obj-y))
> obj-y += sclp.o
> obj-y += event-facility.o
> -obj-y += sclpquiesce.o
> +obj-y += sclpquiesce.o sclpconsole.o
> diff --git a/hw/s390x/sclpconsole.c b/hw/s390x/sclpconsole.c
> new file mode 100644
> index 0000000..9a57032
> --- /dev/null
> +++ b/hw/s390x/sclpconsole.c
> @@ -0,0 +1,323 @@
> +/*
> + * SCLP event type
> + * Ascii Console Data (VT220 Console)
> + *
> + * Copyright IBM, Corp. 2012
> + *
> + * Authors:
> + * Heinz Graalfs <graalfs@de.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/qdev.h>
> +#include "qemu-thread.h"
> +
> +#include "sclp.h"
> +#include "event-facility.h"
> +
> +typedef struct ASCIIConsoleData {
> + EventBufferHeader ebh;
> + char data[0];
> +} QEMU_PACKED ASCIIConsoleData;
> +
> +qemu_irq sclp_read_vt220;
This should go into SCLPConsole.
> +
> +/* max size for ASCII data in 4K SCCB page */
> +#define SIZE_BUFFER_VT220 4080
> +
> +typedef struct SCLPConsole {
> + SCLPEvent event;
> + CharDriverState *chr;
> + /* io vector */
> + uint8_t *iov; /* iov buffer pointer */
> + uint8_t *iov_sclp; /* pointer to SCLP read offset */
> + uint8_t *iov_bs; /* pointer byte stream read offset */
> + uint32_t iov_data_len; /* length of byte stream in buffer */
> + uint32_t iov_sclp_rest; /* length of byte stream not read via SCLP */
> +} SCLPConsole;
> +
> +/* character layer call-back functions */
> +
> +/* Return number of bytes that fit into iov buffer */
> +static int chr_can_read(void *opaque)
> +{
> + int can_read;
> + SCLPConsole *scon = opaque;
> +
> + qemu_mutex_lock(&scon->event.lock);
> + can_read = SIZE_BUFFER_VT220 - scon->iov_data_len;
> + qemu_mutex_unlock(&scon->event.lock);
> +
> + return can_read;
> +}
> +
> +/* Receive n bytes from character layer, save in iov buffer,
> + * and set event pending */
> +static void receive_from_chr_layer(void *opaque, const uint8_t *buf, int size)
> +{
> + SCLPConsole *scon = opaque;
> +
> + assert(scon->iov);
> +
> + qemu_mutex_lock(&scon->event.lock);
> +
> + /* if new data do not fit into current buffer */
> + if (scon->iov_data_len + size > SIZE_BUFFER_VT220) {
> + /* character layer sent more than allowed */
> + qemu_mutex_unlock(&scon->event.lock);
> + return;
> + }
> + /* put byte-stream from character layer into buffer */
> + memcpy(scon->iov_bs, buf, size);
> + scon->iov_data_len += size;
> + scon->iov_sclp_rest += size;
> + scon->iov_bs += size;
> + scon->event.event_pending = true;
> +
> + qemu_mutex_unlock(&scon->event.lock);
> +}
> +
> +/* Send data from a char device over to the guest */
> +static void chr_read(void *opaque, const uint8_t *buf, int size)
> +{
> + assert(opaque);
> +
I'd convert the opaque to SCLPConsole here and use that for
receive_from_chr_layer() instead of opaque.
> + receive_from_chr_layer(opaque, buf, size);
> + /* trigger SCLP read operation */
> + qemu_irq_raise(sclp_read_vt220);
> +}
> +
> +static void chr_event(void *opaque, int event)
> +{
> + SCLPConsole *scon = opaque;
> +
> + switch (event) {
> + case CHR_EVENT_OPENED:
> + if (!scon->iov) {
> + scon->iov = g_malloc0(SIZE_BUFFER_VT220);
> + scon->iov_sclp = scon->iov;
> + scon->iov_bs = scon->iov;
> + scon->iov_data_len = 0;
> + scon->iov_sclp_rest = 0;
> + }
> + break;
> + case CHR_EVENT_CLOSED:
> + if (scon->iov) {
> + g_free(scon->iov);
> + scon->iov = NULL;
> + }
> + break;
> + }
> +}
> +
> +/* functions to be called by event facility */
> +
> +static int event_type(void)
> +{
> + return SCLP_EVENT_ASCII_CONSOLE_DATA;
> +}
> +
> +static unsigned int send_mask(void)
> +{
> + return SCLP_EVENT_MASK_MSG_ASCII;
> +}
> +
> +static unsigned int receive_mask(void)
> +{
> + return SCLP_EVENT_MASK_MSG_ASCII;
> +}
> +
> +/* triggered by SCLP's read_event_data -
> + * copy console data byte-stream into provided (SCLP) buffer
> + */
> +static void get_console_data(SCLPEvent *event, uint8_t *buf, size_t *size,
> + int avail)
> +{
> + SCLPConsole *cons = DO_UPCAST(SCLPConsole, event, event);
> +
> + /* first byte is hex 0 saying an ascii string follows */
> + *buf++ = '\0';
> + avail--;
> + /* if all data fit into provided SCLP buffer */
> + if (avail >= cons->iov_sclp_rest) {
> + /* copy character byte-stream to SCLP buffer */
> + memcpy(buf, cons->iov_sclp, cons->iov_sclp_rest);
> + *size = cons->iov_sclp_rest + 1;
> + cons->iov_sclp = cons->iov;
> + cons->iov_bs = cons->iov;
> + cons->iov_data_len = 0;
> + cons->iov_sclp_rest = 0;
> + event->event_pending = false;
> + /* data provided and no more data pending */
> + } else {
> + /* if provided buffer is too small, just copy part */
> + memcpy(buf, cons->iov_sclp, avail);
> + *size = avail + 1;
> + cons->iov_sclp_rest -= avail;
> + cons->iov_sclp += avail;
> + /* more data pending */
> + }
> +}
> +
> +static int read_event_data(SCLPEvent *event, EventBufferHeader *evt_buf_hdr,
> + int *slen)
> +{
> + int avail;
> + size_t src_len;
> + uint8_t *to;
> + ASCIIConsoleData *acd = (ASCIIConsoleData *) evt_buf_hdr;
> +
> + qemu_mutex_lock(&event->lock);
> +
> + if (!event->event_pending) {
> + /* no data pending */
> + qemu_mutex_unlock(&event->lock);
> + return 0;
> + }
> +
> + to = (uint8_t *)&acd->data;
> + avail = *slen - sizeof(ASCIIConsoleData);
> + get_console_data(event, to, &src_len, avail);
> +
> + acd->ebh.length = cpu_to_be16(sizeof(ASCIIConsoleData) + src_len);
> + acd->ebh.type = SCLP_EVENT_ASCII_CONSOLE_DATA;
> + acd->ebh.flags |= SCLP_EVENT_BUFFER_ACCEPTED;
> + *slen = avail - src_len;
> +
> + qemu_mutex_unlock(&event->lock);
> +
> + return 1;
> +}
> +
> +/* triggered by SCLP's write_event_data
> + * - write console data into character layer
> + * returns < 0 if an error occured
> + */
> +static ssize_t write_console_data(SCLPEvent *event, const uint8_t *buf,
> + size_t len)
> +{
> + ssize_t ret = 0;
> + const uint8_t *iov_offset;
> + SCLPConsole *scon = DO_UPCAST(SCLPConsole, event, event);
> +
> + if (!scon->chr) {
> + /* If there's no backend, we can just say we consumed all data. */
> + return len;
> + }
> +
> + iov_offset = buf;
> + while (len > 0) {
> + ret = qemu_chr_fe_write(scon->chr, buf, len);
> + if (ret == 0) {
> + /* a pty doesn't seem to be connected - no error */
> + len = 0;
> + } else if (ret == -EAGAIN || (ret > 0 && ret < len)) {
> + len -= ret;
> + iov_offset += ret;
> + } else {
> + len = 0;
> + }
> + }
> +
> + return ret;
> +}
> +
> +static int write_event_data(SCLPEvent *event, EventBufferHeader *evt_buf_hdr)
> +{
> + int rc;
> + int length;
> + ssize_t written;
> + ASCIIConsoleData *acd = (ASCIIConsoleData *) evt_buf_hdr;
> +
> + length = be16_to_cpu(evt_buf_hdr->length) - sizeof(EventBufferHeader);
> + written = write_console_data(event, (uint8_t *)acd->data, length);
> +
> + rc = SCLP_RC_NORMAL_COMPLETION;
> + /* set event buffer accepted flag */
> + evt_buf_hdr->flags |= SCLP_EVENT_BUFFER_ACCEPTED;
> +
> + /* written will be zero if a pty is not connected - don't treat as error */
> + if (written < 0) {
> + /* event buffer not accepted due to error in character layer */
> + evt_buf_hdr->flags &= ~(SCLP_EVENT_BUFFER_ACCEPTED);
> + rc = SCLP_RC_CONTAINED_EQUIPMENT_CHECK;
> + }
> +
> + return rc;
> +}
> +
> +static void trigger_ascii_console_data(void *env, int n, int level)
> +{
> + sclp_service_interrupt(0);
> +}
> +
> +/* qemu object creation and initialization functions */
> +
> +/* tell character layer our call-back functions */
> +static int console_init(SCLPEvent *event)
> +{
> + static bool console_available;
> +
> + SCLPConsole *scon = DO_UPCAST(SCLPConsole, event, event);
> +
> + if (console_available) {
> + error_report("Multiple VT220 operator consoles are not supported");
> + return -1;
> + }
> + console_available = true;
> + event->event_type = SCLP_EVENT_ASCII_CONSOLE_DATA;
> + if (scon->chr) {
> + qemu_chr_add_handlers(scon->chr, chr_can_read,
> + chr_read, chr_event, scon);
> + }
> + sclp_read_vt220 = *qemu_allocate_irqs(trigger_ascii_console_data,
> + NULL, 1);
> +
> + qemu_mutex_init(&event->lock);
> +
> + return 0;
> +}
> +
> +static int console_exit(SCLPEvent *event)
> +{
> + qemu_mutex_destroy(&event->lock);
> +
> + return 0;
> +}
> +
> +static Property console_properties[] = {
> + DEFINE_PROP_CHR("chardev", SCLPConsole, chr),
> + DEFINE_PROP_END_OF_LIST(),
> +};
> +
> +static void console_class_init(ObjectClass *klass, void *data)
> +{
> + DeviceClass *dc = DEVICE_CLASS(klass);
> + SCLPEventClass *ec = SCLP_EVENT_CLASS(klass);
> +
> + dc->props = console_properties;
> + ec->init = console_init;
> + ec->exit = console_exit;
> + ec->get_send_mask = send_mask;
> + ec->get_receive_mask = receive_mask;
> + ec->event_type = event_type;
> + ec->read_event_data = read_event_data;
> + ec->write_event_data = write_event_data;
> +}
> +
> +static TypeInfo sclp_console_info = {
> + .name = "sclpconsole",
> + .parent = TYPE_SCLP_EVENT,
> + .instance_size = sizeof(SCLPConsole),
> + .class_init = console_class_init,
> + .class_size = sizeof(SCLPEventClass),
> +};
> +
> +static void register_types(void)
> +{
> + type_register_static(&sclp_console_info);
> +}
> +type_init(register_types)
> --
> 1.7.0.1
>
^ permalink raw reply [flat|nested] 33+ messages in thread* [Qemu-devel] [PATCH v4 06/07] s390: sclp ascii console support
2012-07-24 19:42 ` Blue Swirl
@ 2012-07-26 8:55 ` Christian Borntraeger
2012-07-30 14:02 ` Alexander Graf
0 siblings, 1 reply; 33+ messages in thread
From: Christian Borntraeger @ 2012-07-26 8:55 UTC (permalink / raw)
To: Blue Swirl
Cc: Heinz Graalfs, qemu-devel, Alexander Graf, Christian Borntraeger,
Jens Freimann, afaerber
From: Heinz Graalfs <graalfs@linux.vnet.ibm.com>
This code adds console support by implementing SCLP's ASCII Console
Data event. This is the same console as LPARs ASCII console or z/VMs
sysascii.
The console can be specified manually with something like
-chardev stdio,id=charconsole0 -device sclpconsole,chardev=charconsole0,id=console0
Newer kernels will autodetect that console and prefer that over virtio
console.
When data is received from the character layer it creates a service
interrupt to trigger a Read Event Data command from the guest that will
pick up the received character byte-stream.
When characters are echo'ed by the linux guest a Write Event Data occurs
which is forwarded by the Event Facility to the console that supports
a corresponding mask value.
Console resizing is not supported.
The character layer byte-stream is buffered using a fixed size iov
buffer.
changes in v4: fold in suggestions by blueswirl
Signed-off-by: Heinz Graalfs <graalfs@linux.vnet.ibm.com>
Signed-off-by: Christian Borntraeger <borntraeger@de.ibm.com>
---
hw/s390x/Makefile.objs | 2 +-
hw/s390x/sclpconsole.c | 323 ++++++++++++++++++++++++++++++++++++++++++++++++
2 files changed, 324 insertions(+), 1 deletions(-)
create mode 100644 hw/s390x/sclpconsole.c
diff --git a/hw/s390x/Makefile.objs b/hw/s390x/Makefile.objs
index ed4e61a..096dfcd 100644
--- a/hw/s390x/Makefile.objs
+++ b/hw/s390x/Makefile.objs
@@ -3,4 +3,4 @@ obj-y = s390-virtio-bus.o s390-virtio.o
obj-y := $(addprefix ../,$(obj-y))
obj-y += sclp.o
obj-y += event-facility.o
-obj-y += sclpquiesce.o
+obj-y += sclpquiesce.o sclpconsole.o
diff --git a/hw/s390x/sclpconsole.c b/hw/s390x/sclpconsole.c
new file mode 100644
index 0000000..c77b2a1
--- /dev/null
+++ b/hw/s390x/sclpconsole.c
@@ -0,0 +1,323 @@
+/*
+ * SCLP event type
+ * Ascii Console Data (VT220 Console)
+ *
+ * Copyright IBM, Corp. 2012
+ *
+ * Authors:
+ * Heinz Graalfs <graalfs@de.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/qdev.h>
+#include "qemu-thread.h"
+
+#include "sclp.h"
+#include "event-facility.h"
+
+typedef struct ASCIIConsoleData {
+ EventBufferHeader ebh;
+ char data[0];
+} QEMU_PACKED ASCIIConsoleData;
+
+/* max size for ASCII data in 4K SCCB page */
+#define SIZE_BUFFER_VT220 4080
+
+typedef struct SCLPConsole {
+ SCLPEvent event;
+ CharDriverState *chr;
+ /* io vector */
+ uint8_t *iov; /* iov buffer pointer */
+ uint8_t *iov_sclp; /* pointer to SCLP read offset */
+ uint8_t *iov_bs; /* pointer byte stream read offset */
+ uint32_t iov_data_len; /* length of byte stream in buffer */
+ uint32_t iov_sclp_rest; /* length of byte stream not read via SCLP */
+ qemu_irq sclp_read_vt220;
+} SCLPConsole;
+
+/* character layer call-back functions */
+
+/* Return number of bytes that fit into iov buffer */
+static int chr_can_read(void *opaque)
+{
+ int can_read;
+ SCLPConsole *scon = opaque;
+
+ qemu_mutex_lock(&scon->event.lock);
+ can_read = SIZE_BUFFER_VT220 - scon->iov_data_len;
+ qemu_mutex_unlock(&scon->event.lock);
+
+ return can_read;
+}
+
+/* Receive n bytes from character layer, save in iov buffer,
+ * and set event pending */
+static void receive_from_chr_layer(SCLPConsole *scon, const uint8_t *buf,
+ int size)
+{
+ assert(scon->iov);
+
+ qemu_mutex_lock(&scon->event.lock);
+
+ /* if new data do not fit into current buffer */
+ if (scon->iov_data_len + size > SIZE_BUFFER_VT220) {
+ /* character layer sent more than allowed */
+ qemu_mutex_unlock(&scon->event.lock);
+ return;
+ }
+ /* put byte-stream from character layer into buffer */
+ memcpy(scon->iov_bs, buf, size);
+ scon->iov_data_len += size;
+ scon->iov_sclp_rest += size;
+ scon->iov_bs += size;
+ scon->event.event_pending = true;
+
+ qemu_mutex_unlock(&scon->event.lock);
+}
+
+/* Send data from a char device over to the guest */
+static void chr_read(void *opaque, const uint8_t *buf, int size)
+{
+ SCLPConsole *scon = opaque;
+
+ assert(scon);
+
+ receive_from_chr_layer(scon, buf, size);
+ /* trigger SCLP read operation */
+ qemu_irq_raise(scon->sclp_read_vt220);
+}
+
+static void chr_event(void *opaque, int event)
+{
+ SCLPConsole *scon = opaque;
+
+ switch (event) {
+ case CHR_EVENT_OPENED:
+ if (!scon->iov) {
+ scon->iov = g_malloc0(SIZE_BUFFER_VT220);
+ scon->iov_sclp = scon->iov;
+ scon->iov_bs = scon->iov;
+ scon->iov_data_len = 0;
+ scon->iov_sclp_rest = 0;
+ }
+ break;
+ case CHR_EVENT_CLOSED:
+ if (scon->iov) {
+ g_free(scon->iov);
+ scon->iov = NULL;
+ }
+ break;
+ }
+}
+
+/* functions to be called by event facility */
+
+static int event_type(void)
+{
+ return SCLP_EVENT_ASCII_CONSOLE_DATA;
+}
+
+static unsigned int send_mask(void)
+{
+ return SCLP_EVENT_MASK_MSG_ASCII;
+}
+
+static unsigned int receive_mask(void)
+{
+ return SCLP_EVENT_MASK_MSG_ASCII;
+}
+
+/* triggered by SCLP's read_event_data -
+ * copy console data byte-stream into provided (SCLP) buffer
+ */
+static void get_console_data(SCLPEvent *event, uint8_t *buf, size_t *size,
+ int avail)
+{
+ SCLPConsole *cons = DO_UPCAST(SCLPConsole, event, event);
+
+ /* first byte is hex 0 saying an ascii string follows */
+ *buf++ = '\0';
+ avail--;
+ /* if all data fit into provided SCLP buffer */
+ if (avail >= cons->iov_sclp_rest) {
+ /* copy character byte-stream to SCLP buffer */
+ memcpy(buf, cons->iov_sclp, cons->iov_sclp_rest);
+ *size = cons->iov_sclp_rest + 1;
+ cons->iov_sclp = cons->iov;
+ cons->iov_bs = cons->iov;
+ cons->iov_data_len = 0;
+ cons->iov_sclp_rest = 0;
+ event->event_pending = false;
+ /* data provided and no more data pending */
+ } else {
+ /* if provided buffer is too small, just copy part */
+ memcpy(buf, cons->iov_sclp, avail);
+ *size = avail + 1;
+ cons->iov_sclp_rest -= avail;
+ cons->iov_sclp += avail;
+ /* more data pending */
+ }
+}
+
+static int read_event_data(SCLPEvent *event, EventBufferHeader *evt_buf_hdr,
+ int *slen)
+{
+ int avail;
+ size_t src_len;
+ uint8_t *to;
+ ASCIIConsoleData *acd = (ASCIIConsoleData *) evt_buf_hdr;
+
+ qemu_mutex_lock(&event->lock);
+
+ if (!event->event_pending) {
+ /* no data pending */
+ qemu_mutex_unlock(&event->lock);
+ return 0;
+ }
+
+ to = (uint8_t *)&acd->data;
+ avail = *slen - sizeof(ASCIIConsoleData);
+ get_console_data(event, to, &src_len, avail);
+
+ acd->ebh.length = cpu_to_be16(sizeof(ASCIIConsoleData) + src_len);
+ acd->ebh.type = SCLP_EVENT_ASCII_CONSOLE_DATA;
+ acd->ebh.flags |= SCLP_EVENT_BUFFER_ACCEPTED;
+ *slen = avail - src_len;
+
+ qemu_mutex_unlock(&event->lock);
+
+ return 1;
+}
+
+/* triggered by SCLP's write_event_data
+ * - write console data into character layer
+ * returns < 0 if an error occured
+ */
+static ssize_t write_console_data(SCLPEvent *event, const uint8_t *buf,
+ size_t len)
+{
+ ssize_t ret = 0;
+ const uint8_t *iov_offset;
+ SCLPConsole *scon = DO_UPCAST(SCLPConsole, event, event);
+
+ if (!scon->chr) {
+ /* If there's no backend, we can just say we consumed all data. */
+ return len;
+ }
+
+ iov_offset = buf;
+ while (len > 0) {
+ ret = qemu_chr_fe_write(scon->chr, buf, len);
+ if (ret == 0) {
+ /* a pty doesn't seem to be connected - no error */
+ len = 0;
+ } else if (ret == -EAGAIN || (ret > 0 && ret < len)) {
+ len -= ret;
+ iov_offset += ret;
+ } else {
+ len = 0;
+ }
+ }
+
+ return ret;
+}
+
+static int write_event_data(SCLPEvent *event, EventBufferHeader *evt_buf_hdr)
+{
+ int rc;
+ int length;
+ ssize_t written;
+ ASCIIConsoleData *acd = (ASCIIConsoleData *) evt_buf_hdr;
+
+ length = be16_to_cpu(evt_buf_hdr->length) - sizeof(EventBufferHeader);
+ written = write_console_data(event, (uint8_t *)acd->data, length);
+
+ rc = SCLP_RC_NORMAL_COMPLETION;
+ /* set event buffer accepted flag */
+ evt_buf_hdr->flags |= SCLP_EVENT_BUFFER_ACCEPTED;
+
+ /* written will be zero if a pty is not connected - don't treat as error */
+ if (written < 0) {
+ /* event buffer not accepted due to error in character layer */
+ evt_buf_hdr->flags &= ~(SCLP_EVENT_BUFFER_ACCEPTED);
+ rc = SCLP_RC_CONTAINED_EQUIPMENT_CHECK;
+ }
+
+ return rc;
+}
+
+static void trigger_ascii_console_data(void *env, int n, int level)
+{
+ sclp_service_interrupt(0);
+}
+
+/* qemu object creation and initialization functions */
+
+/* tell character layer our call-back functions */
+static int console_init(SCLPEvent *event)
+{
+ static bool console_available;
+
+ SCLPConsole *scon = DO_UPCAST(SCLPConsole, event, event);
+
+ if (console_available) {
+ error_report("Multiple VT220 operator consoles are not supported");
+ return -1;
+ }
+ console_available = true;
+ event->event_type = SCLP_EVENT_ASCII_CONSOLE_DATA;
+ if (scon->chr) {
+ qemu_chr_add_handlers(scon->chr, chr_can_read,
+ chr_read, chr_event, scon);
+ }
+ scon->sclp_read_vt220 = *qemu_allocate_irqs(trigger_ascii_console_data,
+ NULL, 1);
+
+ qemu_mutex_init(&event->lock);
+
+ return 0;
+}
+
+static int console_exit(SCLPEvent *event)
+{
+ qemu_mutex_destroy(&event->lock);
+
+ return 0;
+}
+
+static Property console_properties[] = {
+ DEFINE_PROP_CHR("chardev", SCLPConsole, chr),
+ DEFINE_PROP_END_OF_LIST(),
+};
+
+static void console_class_init(ObjectClass *klass, void *data)
+{
+ DeviceClass *dc = DEVICE_CLASS(klass);
+ SCLPEventClass *ec = SCLP_EVENT_CLASS(klass);
+
+ dc->props = console_properties;
+ ec->init = console_init;
+ ec->exit = console_exit;
+ ec->get_send_mask = send_mask;
+ ec->get_receive_mask = receive_mask;
+ ec->event_type = event_type;
+ ec->read_event_data = read_event_data;
+ ec->write_event_data = write_event_data;
+}
+
+static TypeInfo sclp_console_info = {
+ .name = "sclpconsole",
+ .parent = TYPE_SCLP_EVENT,
+ .instance_size = sizeof(SCLPConsole),
+ .class_init = console_class_init,
+ .class_size = sizeof(SCLPEventClass),
+};
+
+static void register_types(void)
+{
+ type_register_static(&sclp_console_info);
+}
+type_init(register_types)
--
1.7.0.1
^ permalink raw reply related [flat|nested] 33+ messages in thread* Re: [Qemu-devel] [PATCH v4 06/07] s390: sclp ascii console support
2012-07-26 8:55 ` [Qemu-devel] [PATCH v4 06/07] " Christian Borntraeger
@ 2012-07-30 14:02 ` Alexander Graf
2012-07-31 13:05 ` Christian Borntraeger
0 siblings, 1 reply; 33+ messages in thread
From: Alexander Graf @ 2012-07-30 14:02 UTC (permalink / raw)
To: Christian Borntraeger
Cc: Blue Swirl, Heinz Graalfs, qemu-devel, Jens Freimann, afaerber
On 26.07.2012, at 10:55, Christian Borntraeger wrote:
> From: Heinz Graalfs <graalfs@linux.vnet.ibm.com>
>
> This code adds console support by implementing SCLP's ASCII Console
> Data event. This is the same console as LPARs ASCII console or z/VMs
> sysascii.
>
> The console can be specified manually with something like
> -chardev stdio,id=charconsole0 -device sclpconsole,chardev=charconsole0,id=console0
>
> Newer kernels will autodetect that console and prefer that over virtio
> console.
>
> When data is received from the character layer it creates a service
> interrupt to trigger a Read Event Data command from the guest that will
> pick up the received character byte-stream.
> When characters are echo'ed by the linux guest a Write Event Data occurs
> which is forwarded by the Event Facility to the console that supports
> a corresponding mask value.
> Console resizing is not supported.
> The character layer byte-stream is buffered using a fixed size iov
> buffer.
>
> changes in v4: fold in suggestions by blueswirl
>
> Signed-off-by: Heinz Graalfs <graalfs@linux.vnet.ibm.com>
> Signed-off-by: Christian Borntraeger <borntraeger@de.ibm.com>
> ---
> hw/s390x/Makefile.objs | 2 +-
> hw/s390x/sclpconsole.c | 323 ++++++++++++++++++++++++++++++++++++++++++++++++
> 2 files changed, 324 insertions(+), 1 deletions(-)
> create mode 100644 hw/s390x/sclpconsole.c
>
> diff --git a/hw/s390x/Makefile.objs b/hw/s390x/Makefile.objs
> index ed4e61a..096dfcd 100644
> --- a/hw/s390x/Makefile.objs
> +++ b/hw/s390x/Makefile.objs
> @@ -3,4 +3,4 @@ obj-y = s390-virtio-bus.o s390-virtio.o
> obj-y := $(addprefix ../,$(obj-y))
> obj-y += sclp.o
> obj-y += event-facility.o
> -obj-y += sclpquiesce.o
> +obj-y += sclpquiesce.o sclpconsole.o
> diff --git a/hw/s390x/sclpconsole.c b/hw/s390x/sclpconsole.c
> new file mode 100644
> index 0000000..c77b2a1
> --- /dev/null
> +++ b/hw/s390x/sclpconsole.c
> @@ -0,0 +1,323 @@
> +/*
> + * SCLP event type
> + * Ascii Console Data (VT220 Console)
> + *
> + * Copyright IBM, Corp. 2012
> + *
> + * Authors:
> + * Heinz Graalfs <graalfs@de.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/qdev.h>
> +#include "qemu-thread.h"
> +
> +#include "sclp.h"
> +#include "event-facility.h"
> +
> +typedef struct ASCIIConsoleData {
> + EventBufferHeader ebh;
> + char data[0];
> +} QEMU_PACKED ASCIIConsoleData;
> +
> +/* max size for ASCII data in 4K SCCB page */
> +#define SIZE_BUFFER_VT220 4080
> +
> +typedef struct SCLPConsole {
> + SCLPEvent event;
> + CharDriverState *chr;
> + /* io vector */
> + uint8_t *iov; /* iov buffer pointer */
> + uint8_t *iov_sclp; /* pointer to SCLP read offset */
> + uint8_t *iov_bs; /* pointer byte stream read offset */
> + uint32_t iov_data_len; /* length of byte stream in buffer */
> + uint32_t iov_sclp_rest; /* length of byte stream not read via SCLP */
> + qemu_irq sclp_read_vt220;
I'm sure this one wants a name that indicates it's an irq line ;)
> +} SCLPConsole;
> +
> +/* character layer call-back functions */
> +
> +/* Return number of bytes that fit into iov buffer */
> +static int chr_can_read(void *opaque)
> +{
> + int can_read;
> + SCLPConsole *scon = opaque;
> +
> + qemu_mutex_lock(&scon->event.lock);
Explicit locks now?
> + can_read = SIZE_BUFFER_VT220 - scon->iov_data_len;
> + qemu_mutex_unlock(&scon->event.lock);
> +
> + return can_read;
> +}
> +
> +/* Receive n bytes from character layer, save in iov buffer,
> + * and set event pending */
> +static void receive_from_chr_layer(SCLPConsole *scon, const uint8_t *buf,
> + int size)
> +{
> + assert(scon->iov);
> +
> + qemu_mutex_lock(&scon->event.lock);
> +
> + /* if new data do not fit into current buffer */
> + if (scon->iov_data_len + size > SIZE_BUFFER_VT220) {
> + /* character layer sent more than allowed */
> + qemu_mutex_unlock(&scon->event.lock);
> + return;
So we drop the bytes it did send? Isn't there usually some can_read function from the char layer that we can indicate to that we have enough space?
If so, then this should be an assert(), right?
> + }
> + /* put byte-stream from character layer into buffer */
> + memcpy(scon->iov_bs, buf, size);
> + scon->iov_data_len += size;
> + scon->iov_sclp_rest += size;
> + scon->iov_bs += size;
> + scon->event.event_pending = true;
> +
> + qemu_mutex_unlock(&scon->event.lock);
> +}
> +
> +/* Send data from a char device over to the guest */
> +static void chr_read(void *opaque, const uint8_t *buf, int size)
> +{
> + SCLPConsole *scon = opaque;
> +
> + assert(scon);
> +
> + receive_from_chr_layer(scon, buf, size);
> + /* trigger SCLP read operation */
> + qemu_irq_raise(scon->sclp_read_vt220);
> +}
> +
> +static void chr_event(void *opaque, int event)
> +{
> + SCLPConsole *scon = opaque;
> +
> + switch (event) {
> + case CHR_EVENT_OPENED:
> + if (!scon->iov) {
> + scon->iov = g_malloc0(SIZE_BUFFER_VT220);
> + scon->iov_sclp = scon->iov;
> + scon->iov_bs = scon->iov;
> + scon->iov_data_len = 0;
> + scon->iov_sclp_rest = 0;
> + }
> + break;
> + case CHR_EVENT_CLOSED:
> + if (scon->iov) {
> + g_free(scon->iov);
> + scon->iov = NULL;
> + }
> + break;
> + }
> +}
> +
> +/* functions to be called by event facility */
> +
> +static int event_type(void)
> +{
> + return SCLP_EVENT_ASCII_CONSOLE_DATA;
> +}
> +
> +static unsigned int send_mask(void)
> +{
> + return SCLP_EVENT_MASK_MSG_ASCII;
> +}
> +
> +static unsigned int receive_mask(void)
> +{
> + return SCLP_EVENT_MASK_MSG_ASCII;
> +}
> +
> +/* triggered by SCLP's read_event_data -
> + * copy console data byte-stream into provided (SCLP) buffer
> + */
> +static void get_console_data(SCLPEvent *event, uint8_t *buf, size_t *size,
> + int avail)
> +{
> + SCLPConsole *cons = DO_UPCAST(SCLPConsole, event, event);
> +
> + /* first byte is hex 0 saying an ascii string follows */
> + *buf++ = '\0';
> + avail--;
> + /* if all data fit into provided SCLP buffer */
> + if (avail >= cons->iov_sclp_rest) {
> + /* copy character byte-stream to SCLP buffer */
> + memcpy(buf, cons->iov_sclp, cons->iov_sclp_rest);
> + *size = cons->iov_sclp_rest + 1;
> + cons->iov_sclp = cons->iov;
> + cons->iov_bs = cons->iov;
> + cons->iov_data_len = 0;
> + cons->iov_sclp_rest = 0;
> + event->event_pending = false;
> + /* data provided and no more data pending */
> + } else {
> + /* if provided buffer is too small, just copy part */
> + memcpy(buf, cons->iov_sclp, avail);
> + *size = avail + 1;
> + cons->iov_sclp_rest -= avail;
> + cons->iov_sclp += avail;
> + /* more data pending */
> + }
> +}
I'm wondering whether we actually need this indirection from
chr layer -> buffer -> sclp buffer.
Why can't we just do
chr layer -> sclp buffer?
Alex
^ permalink raw reply [flat|nested] 33+ messages in thread* Re: [Qemu-devel] [PATCH v4 06/07] s390: sclp ascii console support
2012-07-30 14:02 ` Alexander Graf
@ 2012-07-31 13:05 ` Christian Borntraeger
0 siblings, 0 replies; 33+ messages in thread
From: Christian Borntraeger @ 2012-07-31 13:05 UTC (permalink / raw)
To: Alexander Graf
Cc: Blue Swirl, Heinz Graalfs, qemu-devel, Jens Freimann, afaerber
On 30/07/12 16:02, Alexander Graf wrote:
>> + qemu_irq sclp_read_vt220;
>
> I'm sure this one wants a name that indicates it's an irq line ;)
ok.
>
>> +} SCLPConsole;
>> +
>> +/* character layer call-back functions */
>> +
>> +/* Return number of bytes that fit into iov buffer */
>> +static int chr_can_read(void *opaque)
>> +{
>> + int can_read;
>> + SCLPConsole *scon = opaque;
>> +
>> + qemu_mutex_lock(&scon->event.lock);
>
> Explicit locks now?
IIRC Heinz introduced the locks since we have multiple threads working on the same
kind of buffers (the cpu threads and the main thread). We could not verify that the
main thread has the big qemu lock in all cases. Furthermore, this makes the code already
thread safe if we get rid of the big qemu lock somewhen. But I agree, we have to double
check why there are two different kind of locks.
[...]
>> + /* if new data do not fit into current buffer */
>> + if (scon->iov_data_len + size > SIZE_BUFFER_VT220) {
>> + /* character layer sent more than allowed */
>> + qemu_mutex_unlock(&scon->event.lock);
>> + return;
>
> So we drop the bytes it did send? Isn't there usually some can_read function from the char layer that we can indicate to that we have enough space?
>
> If so, then this should be an assert(), right?
Probably yes. Will double check.
[..]
>> + SCLPConsole *cons = DO_UPCAST(SCLPConsole, event, event);
>> +
>> + /* first byte is hex 0 saying an ascii string follows */
>> + *buf++ = '\0';
>> + avail--;
>> + /* if all data fit into provided SCLP buffer */
>> + if (avail >= cons->iov_sclp_rest) {
>> + /* copy character byte-stream to SCLP buffer */
>> + memcpy(buf, cons->iov_sclp, cons->iov_sclp_rest);
>> + *size = cons->iov_sclp_rest + 1;
>> + cons->iov_sclp = cons->iov;
>> + cons->iov_bs = cons->iov;
>> + cons->iov_data_len = 0;
>> + cons->iov_sclp_rest = 0;
>> + event->event_pending = false;
>> + /* data provided and no more data pending */
>> + } else {
>> + /* if provided buffer is too small, just copy part */
>> + memcpy(buf, cons->iov_sclp, avail);
>> + *size = avail + 1;
>> + cons->iov_sclp_rest -= avail;
>> + cons->iov_sclp += avail;
>> + /* more data pending */
>> + }
>> +}
>
> I'm wondering whether we actually need this indirection from
>
> chr layer -> buffer -> sclp buffer.
>
> Why can't we just do
>
> chr layer -> sclp buffer?
>
The sclp interface is a bit different here. On an input event, the hw generated an service
interrupt with the event pending bit set. Then the guest kernel does a read event data sclp
call with input buffer. The input data has to copied into that and then returned via an
additional interrupt.
Without the buffering our can_read function could only return 0 as size since there is yet no
buffer. Makes sense?
Christian
^ permalink raw reply [flat|nested] 33+ messages in thread
* [Qemu-devel] [PATCH 7/7] s390: make sclp ascii console the default
2012-07-24 7:37 [Qemu-devel] [PATCH 0/7v3] s390: sclp patch set Christian Borntraeger
` (5 preceding siblings ...)
2012-07-24 7:37 ` [Qemu-devel] [PATCH 6/7] s390: sclp ascii console support Christian Borntraeger
@ 2012-07-24 7:37 ` Christian Borntraeger
2012-07-24 19:35 ` Blue Swirl
2012-07-30 9:00 ` [Qemu-devel] [PATCH 0/7v3] s390: sclp patch set Christian Borntraeger
7 siblings, 1 reply; 33+ messages in thread
From: Christian Borntraeger @ 2012-07-24 7:37 UTC (permalink / raw)
To: Alexander Graf
Cc: Heinz Graalfs, qemu-devel, blauwirbel, Christian Borntraeger,
Jens Freimann, afaerber
This patch makes the sclp ascii default for S390.
Signed-off-by: Christian Borntraeger <borntraeger@de.ibm.com>
---
hw/s390-virtio.c | 1 -
vl.c | 44 ++++++++++++++++++++++++++++++++++++++++++++
2 files changed, 44 insertions(+), 1 deletions(-)
diff --git a/hw/s390-virtio.c b/hw/s390-virtio.c
index 28e320d..8b48f66 100644
--- a/hw/s390-virtio.c
+++ b/hw/s390-virtio.c
@@ -341,7 +341,6 @@ static QEMUMachine s390_machine = {
.no_serial = 1,
.no_parallel = 1,
.no_sdcard = 1,
- .use_virtcon = 1,
.max_cpus = 255,
.is_default = 1,
};
diff --git a/vl.c b/vl.c
index 46248b9..7197724 100644
--- a/vl.c
+++ b/vl.c
@@ -168,6 +168,7 @@ int main(int argc, char **argv)
#define DEFAULT_RAM_SIZE 128
#define MAX_VIRTIO_CONSOLES 1
+#define MAX_SCLP_CONSOLES 1
static const char *data_dir;
const char *bios_name = NULL;
@@ -195,6 +196,7 @@ int no_quit = 0;
CharDriverState *serial_hds[MAX_SERIAL_PORTS];
CharDriverState *parallel_hds[MAX_PARALLEL_PORTS];
CharDriverState *virtcon_hds[MAX_VIRTIO_CONSOLES];
+CharDriverState *sclpcon_hds[MAX_SCLP_CONSOLES];
int win2k_install_hack = 0;
int usb_enabled = 0;
int singlestep = 0;
@@ -268,6 +270,7 @@ static int default_floppy = 1;
static int default_cdrom = 1;
static int default_sdcard = 1;
static int default_vga = 1;
+static int default_sclpcon = 1;
static struct {
const char *driver;
@@ -289,6 +292,7 @@ static struct {
{ .driver = "isa-cirrus-vga", .flag = &default_vga },
{ .driver = "vmware-svga", .flag = &default_vga },
{ .driver = "qxl-vga", .flag = &default_vga },
+ { .driver = "sclpconsole", .flag = &default_sclpcon },
};
static void res_free(void)
@@ -1935,6 +1939,7 @@ struct device_config {
DEV_VIRTCON, /* -virtioconsole */
DEV_DEBUGCON, /* -debugcon */
DEV_GDB, /* -gdb, -s */
+ DEV_SCLPCON, /* sclp console */
} type;
const char *cmdline;
Location loc;
@@ -2014,6 +2019,36 @@ static int parallel_parse(const char *devname)
return 0;
}
+static int sclpcon_parse(const char *devname)
+{
+ QemuOptsList *device = qemu_find_opts("device");
+ static int index = 0;
+ char label[32];
+ QemuOpts *dev_opts;
+
+ if (strcmp(devname, "none") == 0)
+ return 0;
+ if (index == MAX_SCLP_CONSOLES) {
+ fprintf(stderr, "qemu: too many sclp consoles\n");
+ exit(1);
+ }
+
+ dev_opts = qemu_opts_create(device, NULL, 0, NULL);
+ qemu_opt_set(dev_opts, "driver", "sclpconsole");
+
+ snprintf(label, sizeof(label), "sclpcon%d", index);
+ sclpcon_hds[index] = qemu_chr_new(label, devname, NULL);
+ if (!sclpcon_hds[index]) {
+ fprintf(stderr, "qemu: could not open sclp console '%s': %s\n",
+ devname, strerror(errno));
+ return -1;
+ }
+ qemu_opt_set(dev_opts, "chardev", label);
+
+ index++;
+ return 0;
+}
+
static int virtcon_parse(const char *devname)
{
QemuOptsList *device = qemu_find_opts("device");
@@ -3122,6 +3157,7 @@ int main(int argc, char **argv, char **envp)
default_cdrom = 0;
default_sdcard = 0;
default_vga = 0;
+ default_sclpcon = 0;
break;
case QEMU_OPTION_xen_domid:
if (!(xen_available())) {
@@ -3303,11 +3339,15 @@ int main(int argc, char **argv, char **envp)
add_device_config(DEV_PARALLEL, "null");
if (default_serial && default_monitor) {
add_device_config(DEV_SERIAL, "mon:stdio");
+ } else if (default_sclpcon && default_monitor) {
+ add_device_config(DEV_SCLPCON, "mon:stdio");
} else if (default_virtcon && default_monitor) {
add_device_config(DEV_VIRTCON, "mon:stdio");
} else {
if (default_serial)
add_device_config(DEV_SERIAL, "stdio");
+ if (default_sclpcon)
+ add_device_config(DEV_SCLPCON, "stdio");
if (default_virtcon)
add_device_config(DEV_VIRTCON, "stdio");
if (default_monitor)
@@ -3320,6 +3360,8 @@ int main(int argc, char **argv, char **envp)
add_device_config(DEV_PARALLEL, "vc:80Cx24C");
if (default_monitor)
monitor_parse("vc:80Cx24C", "readline");
+ if (default_sclpcon)
+ add_device_config(DEV_SCLPCON, "vc:80Cx24C");
if (default_virtcon)
add_device_config(DEV_VIRTCON, "vc:80Cx24C");
}
@@ -3490,6 +3532,8 @@ int main(int argc, char **argv, char **envp)
exit(1);
if (foreach_device_config(DEV_PARALLEL, parallel_parse) < 0)
exit(1);
+ if (foreach_device_config(DEV_SCLPCON, sclpcon_parse) < 0)
+ exit(1);
if (foreach_device_config(DEV_VIRTCON, virtcon_parse) < 0)
exit(1);
if (foreach_device_config(DEV_DEBUGCON, debugcon_parse) < 0)
--
1.7.0.1
^ permalink raw reply related [flat|nested] 33+ messages in thread* Re: [Qemu-devel] [PATCH 7/7] s390: make sclp ascii console the default
2012-07-24 7:37 ` [Qemu-devel] [PATCH 7/7] s390: make sclp ascii console the default Christian Borntraeger
@ 2012-07-24 19:35 ` Blue Swirl
2012-07-25 6:55 ` Christian Borntraeger
` (2 more replies)
0 siblings, 3 replies; 33+ messages in thread
From: Blue Swirl @ 2012-07-24 19:35 UTC (permalink / raw)
To: Christian Borntraeger
Cc: qemu-devel, Heinz Graalfs, Alexander Graf, afaerber, Jens Freimann
On Tue, Jul 24, 2012 at 7:37 AM, Christian Borntraeger
<borntraeger@de.ibm.com> wrote:
> This patch makes the sclp ascii default for S390.
>
> Signed-off-by: Christian Borntraeger <borntraeger@de.ibm.com>
> ---
> hw/s390-virtio.c | 1 -
> vl.c | 44 ++++++++++++++++++++++++++++++++++++++++++++
> 2 files changed, 44 insertions(+), 1 deletions(-)
>
> diff --git a/hw/s390-virtio.c b/hw/s390-virtio.c
> index 28e320d..8b48f66 100644
> --- a/hw/s390-virtio.c
> +++ b/hw/s390-virtio.c
> @@ -341,7 +341,6 @@ static QEMUMachine s390_machine = {
> .no_serial = 1,
> .no_parallel = 1,
> .no_sdcard = 1,
> - .use_virtcon = 1,
> .max_cpus = 255,
> .is_default = 1,
> };
> diff --git a/vl.c b/vl.c
> index 46248b9..7197724 100644
> --- a/vl.c
> +++ b/vl.c
> @@ -168,6 +168,7 @@ int main(int argc, char **argv)
> #define DEFAULT_RAM_SIZE 128
>
> #define MAX_VIRTIO_CONSOLES 1
> +#define MAX_SCLP_CONSOLES 1
>
> static const char *data_dir;
> const char *bios_name = NULL;
> @@ -195,6 +196,7 @@ int no_quit = 0;
> CharDriverState *serial_hds[MAX_SERIAL_PORTS];
> CharDriverState *parallel_hds[MAX_PARALLEL_PORTS];
> CharDriverState *virtcon_hds[MAX_VIRTIO_CONSOLES];
> +CharDriverState *sclpcon_hds[MAX_SCLP_CONSOLES];
> int win2k_install_hack = 0;
> int usb_enabled = 0;
> int singlestep = 0;
> @@ -268,6 +270,7 @@ static int default_floppy = 1;
> static int default_cdrom = 1;
> static int default_sdcard = 1;
> static int default_vga = 1;
> +static int default_sclpcon = 1;
>
> static struct {
> const char *driver;
> @@ -289,6 +292,7 @@ static struct {
> { .driver = "isa-cirrus-vga", .flag = &default_vga },
> { .driver = "vmware-svga", .flag = &default_vga },
> { .driver = "qxl-vga", .flag = &default_vga },
> + { .driver = "sclpconsole", .flag = &default_sclpcon },
> };
>
> static void res_free(void)
> @@ -1935,6 +1939,7 @@ struct device_config {
> DEV_VIRTCON, /* -virtioconsole */
> DEV_DEBUGCON, /* -debugcon */
> DEV_GDB, /* -gdb, -s */
> + DEV_SCLPCON, /* sclp console */
> } type;
> const char *cmdline;
> Location loc;
> @@ -2014,6 +2019,36 @@ static int parallel_parse(const char *devname)
> return 0;
> }
>
> +static int sclpcon_parse(const char *devname)
> +{
> + QemuOptsList *device = qemu_find_opts("device");
> + static int index = 0;
> + char label[32];
> + QemuOpts *dev_opts;
> +
> + if (strcmp(devname, "none") == 0)
Braces.
> + return 0;
> + if (index == MAX_SCLP_CONSOLES) {
> + fprintf(stderr, "qemu: too many sclp consoles\n");
The user may wonder what is the max, you could tell that.
> + exit(1);
> + }
> +
> + dev_opts = qemu_opts_create(device, NULL, 0, NULL);
> + qemu_opt_set(dev_opts, "driver", "sclpconsole");
> +
> + snprintf(label, sizeof(label), "sclpcon%d", index);
> + sclpcon_hds[index] = qemu_chr_new(label, devname, NULL);
> + if (!sclpcon_hds[index]) {
> + fprintf(stderr, "qemu: could not open sclp console '%s': %s\n",
> + devname, strerror(errno));
> + return -1;
> + }
> + qemu_opt_set(dev_opts, "chardev", label);
> +
> + index++;
> + return 0;
> +}
> +
> static int virtcon_parse(const char *devname)
> {
> QemuOptsList *device = qemu_find_opts("device");
> @@ -3122,6 +3157,7 @@ int main(int argc, char **argv, char **envp)
> default_cdrom = 0;
> default_sdcard = 0;
> default_vga = 0;
> + default_sclpcon = 0;
> break;
> case QEMU_OPTION_xen_domid:
> if (!(xen_available())) {
> @@ -3303,11 +3339,15 @@ int main(int argc, char **argv, char **envp)
> add_device_config(DEV_PARALLEL, "null");
> if (default_serial && default_monitor) {
> add_device_config(DEV_SERIAL, "mon:stdio");
> + } else if (default_sclpcon && default_monitor) {
> + add_device_config(DEV_SCLPCON, "mon:stdio");
> } else if (default_virtcon && default_monitor) {
> add_device_config(DEV_VIRTCON, "mon:stdio");
> } else {
> if (default_serial)
> add_device_config(DEV_SERIAL, "stdio");
> + if (default_sclpcon)
Braces, also below two times.
> + add_device_config(DEV_SCLPCON, "stdio");
> if (default_virtcon)
> add_device_config(DEV_VIRTCON, "stdio");
> if (default_monitor)
> @@ -3320,6 +3360,8 @@ int main(int argc, char **argv, char **envp)
> add_device_config(DEV_PARALLEL, "vc:80Cx24C");
> if (default_monitor)
> monitor_parse("vc:80Cx24C", "readline");
> + if (default_sclpcon)
> + add_device_config(DEV_SCLPCON, "vc:80Cx24C");
> if (default_virtcon)
> add_device_config(DEV_VIRTCON, "vc:80Cx24C");
> }
> @@ -3490,6 +3532,8 @@ int main(int argc, char **argv, char **envp)
> exit(1);
> if (foreach_device_config(DEV_PARALLEL, parallel_parse) < 0)
> exit(1);
> + if (foreach_device_config(DEV_SCLPCON, sclpcon_parse) < 0)
> + exit(1);
> if (foreach_device_config(DEV_VIRTCON, virtcon_parse) < 0)
> exit(1);
> if (foreach_device_config(DEV_DEBUGCON, debugcon_parse) < 0)
> --
> 1.7.0.1
>
^ permalink raw reply [flat|nested] 33+ messages in thread* Re: [Qemu-devel] [PATCH 7/7] s390: make sclp ascii console the default
2012-07-24 19:35 ` Blue Swirl
@ 2012-07-25 6:55 ` Christian Borntraeger
2012-07-26 13:48 ` Andreas Färber
2012-07-26 8:59 ` [Qemu-devel] [PATCH v4 07/07] " Christian Borntraeger
2012-07-26 9:11 ` Christian Borntraeger
2 siblings, 1 reply; 33+ messages in thread
From: Christian Borntraeger @ 2012-07-25 6:55 UTC (permalink / raw)
To: Blue Swirl
Cc: qemu-devel, Heinz Graalfs, Alexander Graf, afaerber, Jens Freimann
On 24/07/12 21:35, Blue Swirl wrote:
[...]
> Braces
[...]
> Braces, also below two times.
a generic question. We did it that way because the other code in vl.c is like that.
So we should make new code follow the CodingStyle even if the surrounding code looks
different, but we dont touch that surrounding code. right?
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [Qemu-devel] [PATCH 7/7] s390: make sclp ascii console the default
2012-07-25 6:55 ` Christian Borntraeger
@ 2012-07-26 13:48 ` Andreas Färber
0 siblings, 0 replies; 33+ messages in thread
From: Andreas Färber @ 2012-07-26 13:48 UTC (permalink / raw)
To: Christian Borntraeger
Cc: Blue Swirl, Heinz Graalfs, qemu-devel, Alexander Graf, Jens Freimann
Am 25.07.2012 08:55, schrieb Christian Borntraeger:
> On 24/07/12 21:35, Blue Swirl wrote:
> [...]
>> Braces
> [...]
>> Braces, also below two times.
>
> a generic question. We did it that way because the other code in vl.c is like that.
>
> So we should make new code follow the CodingStyle even if the surrounding code looks
> different, but we dont touch that surrounding code. right?
There has been resistance towards doing touch-all Coding Style cleanups
since it is invasive and messes with git-blame.
Our policy is to have new code comply with the current Coding Style
(with some exceptions in audio code) and to apply fixes (e.g., add
braces) on the lines touched or in the same hunk.
scripts/checkpatch.pl complains otherwise.
Depending on amount and invasiveness of such cleanups it has sometimes
been requested (e.g., for code movements that show up as deletes and
adds) that larger cleanups be split out into a preceding patch, to keep
things reviewable and to avoid checkpatch.pl errors.
HTE,
Andreas
--
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg
^ permalink raw reply [flat|nested] 33+ messages in thread
* [Qemu-devel] [PATCH v4 07/07] s390: make sclp ascii console the default
2012-07-24 19:35 ` Blue Swirl
2012-07-25 6:55 ` Christian Borntraeger
@ 2012-07-26 8:59 ` Christian Borntraeger
2012-07-30 14:05 ` Alexander Graf
2012-07-26 9:11 ` Christian Borntraeger
2 siblings, 1 reply; 33+ messages in thread
From: Christian Borntraeger @ 2012-07-26 8:59 UTC (permalink / raw)
To: Blue Swirl
Cc: Heinz Graalfs, qemu-devel, Alexander Graf, Christian Borntraeger,
Jens Freimann, afaerber
This patch makes the sclp ascii default for S390. It requires a guest
kernel that autodetects the console and which not blindly assumes
that kvm means virtio console.
(commit cd1834591fe9564720ac4b0193bf1c790fe89f0d
KVM: s390: Perform early event mask processing during boot)
Otherwise the guest admin has to explicitely add console=ttyS1 to the
command line.
Signed-off-by: Christian Borntraeger <borntraeger@de.ibm.com>
---
hw/s390-virtio.c | 1 -
vl.c | 48 ++++++++++++++++++++++++++++++++++++++++++++++++
2 files changed, 48 insertions(+), 1 deletions(-)
diff --git a/hw/s390-virtio.c b/hw/s390-virtio.c
index 28e320d..8b48f66 100644
--- a/hw/s390-virtio.c
+++ b/hw/s390-virtio.c
@@ -341,7 +341,6 @@ static QEMUMachine s390_machine = {
.no_serial = 1,
.no_parallel = 1,
.no_sdcard = 1,
- .use_virtcon = 1,
.max_cpus = 255,
.is_default = 1,
};
diff --git a/vl.c b/vl.c
index 46248b9..30fd506 100644
--- a/vl.c
+++ b/vl.c
@@ -168,6 +168,7 @@ int main(int argc, char **argv)
#define DEFAULT_RAM_SIZE 128
#define MAX_VIRTIO_CONSOLES 1
+#define MAX_SCLP_CONSOLES 1
static const char *data_dir;
const char *bios_name = NULL;
@@ -195,6 +196,7 @@ int no_quit = 0;
CharDriverState *serial_hds[MAX_SERIAL_PORTS];
CharDriverState *parallel_hds[MAX_PARALLEL_PORTS];
CharDriverState *virtcon_hds[MAX_VIRTIO_CONSOLES];
+CharDriverState *sclpcon_hds[MAX_SCLP_CONSOLES];
int win2k_install_hack = 0;
int usb_enabled = 0;
int singlestep = 0;
@@ -268,6 +270,7 @@ static int default_floppy = 1;
static int default_cdrom = 1;
static int default_sdcard = 1;
static int default_vga = 1;
+static int default_sclpcon = 1;
static struct {
const char *driver;
@@ -289,6 +292,7 @@ static struct {
{ .driver = "isa-cirrus-vga", .flag = &default_vga },
{ .driver = "vmware-svga", .flag = &default_vga },
{ .driver = "qxl-vga", .flag = &default_vga },
+ { .driver = "sclpconsole", .flag = &default_sclpcon },
};
static void res_free(void)
@@ -1935,6 +1939,7 @@ struct device_config {
DEV_VIRTCON, /* -virtioconsole */
DEV_DEBUGCON, /* -debugcon */
DEV_GDB, /* -gdb, -s */
+ DEV_SCLPCON, /* sclp console */
} type;
const char *cmdline;
Location loc;
@@ -2014,6 +2019,37 @@ static int parallel_parse(const char *devname)
return 0;
}
+static int sclpcon_parse(const char *devname)
+{
+ QemuOptsList *device = qemu_find_opts("device");
+ static int index;
+ char label[32];
+ QemuOpts *dev_opts;
+
+ if (strcmp(devname, "none") == 0) {
+ return 0;
+ }
+ if (index == MAX_SCLP_CONSOLES) {
+ fprintf(stderr, "qemu: only one sclpconsole may be specified\n");
+ exit(1);
+ }
+
+ dev_opts = qemu_opts_create(device, NULL, 0, NULL);
+ qemu_opt_set(dev_opts, "driver", "sclpconsole");
+
+ snprintf(label, sizeof(label), "sclpcon%d", index);
+ sclpcon_hds[index] = qemu_chr_new(label, devname, NULL);
+ if (!sclpcon_hds[index]) {
+ fprintf(stderr, "qemu: could not open sclp console '%s': %s\n",
+ devname, strerror(errno));
+ return -1;
+ }
+ qemu_opt_set(dev_opts, "chardev", label);
+
+ index++;
+ return 0;
+}
+
static int virtcon_parse(const char *devname)
{
QemuOptsList *device = qemu_find_opts("device");
@@ -3122,6 +3158,7 @@ int main(int argc, char **argv, char **envp)
default_cdrom = 0;
default_sdcard = 0;
default_vga = 0;
+ default_sclpcon = 0;
break;
case QEMU_OPTION_xen_domid:
if (!(xen_available())) {
@@ -3303,11 +3340,16 @@ int main(int argc, char **argv, char **envp)
add_device_config(DEV_PARALLEL, "null");
if (default_serial && default_monitor) {
add_device_config(DEV_SERIAL, "mon:stdio");
+ } else if (default_sclpcon && default_monitor) {
+ add_device_config(DEV_SCLPCON, "mon:stdio");
} else if (default_virtcon && default_monitor) {
add_device_config(DEV_VIRTCON, "mon:stdio");
} else {
if (default_serial)
add_device_config(DEV_SERIAL, "stdio");
+ if (default_sclpcon) {
+ add_device_config(DEV_SCLPCON, "stdio");
+ }
if (default_virtcon)
add_device_config(DEV_VIRTCON, "stdio");
if (default_monitor)
@@ -3320,6 +3362,9 @@ int main(int argc, char **argv, char **envp)
add_device_config(DEV_PARALLEL, "vc:80Cx24C");
if (default_monitor)
monitor_parse("vc:80Cx24C", "readline");
+ if (default_sclpcon) {
+ add_device_config(DEV_SCLPCON, "vc:80Cx24C");
+ }
if (default_virtcon)
add_device_config(DEV_VIRTCON, "vc:80Cx24C");
}
@@ -3490,6 +3535,9 @@ int main(int argc, char **argv, char **envp)
exit(1);
if (foreach_device_config(DEV_PARALLEL, parallel_parse) < 0)
exit(1);
+ if (foreach_device_config(DEV_SCLPCON, sclpcon_parse) < 0) {
+ exit(1);
+ }
if (foreach_device_config(DEV_VIRTCON, virtcon_parse) < 0)
exit(1);
if (foreach_device_config(DEV_DEBUGCON, debugcon_parse) < 0)
--
1.7.0.1
^ permalink raw reply related [flat|nested] 33+ messages in thread* Re: [Qemu-devel] [PATCH v4 07/07] s390: make sclp ascii console the default
2012-07-26 8:59 ` [Qemu-devel] [PATCH v4 07/07] " Christian Borntraeger
@ 2012-07-30 14:05 ` Alexander Graf
2012-07-31 12:44 ` Christian Borntraeger
0 siblings, 1 reply; 33+ messages in thread
From: Alexander Graf @ 2012-07-30 14:05 UTC (permalink / raw)
To: Christian Borntraeger
Cc: Blue Swirl, Heinz Graalfs, qemu-devel, Jens Freimann, afaerber
On 26.07.2012, at 10:59, Christian Borntraeger wrote:
> This patch makes the sclp ascii default for S390. It requires a guest
> kernel that autodetects the console and which not blindly assumes
> that kvm means virtio console.
> (commit cd1834591fe9564720ac4b0193bf1c790fe89f0d
> KVM: s390: Perform early event mask processing during boot)
> Otherwise the guest admin has to explicitely add console=ttyS1 to the
> command line.
Hrm. Could we make this the non-default for the time being with a simple -machine option to switch it to the default? When enough kernels are aware of the switch, we could then make it the default.
The best case would be if we could determine what type of kernel (default to virtio-console vs runtime detect) we run before we spawn the VM, so we could spawn it with the respective default and keep all kernels working...
Alex
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [Qemu-devel] [PATCH v4 07/07] s390: make sclp ascii console the default
2012-07-30 14:05 ` Alexander Graf
@ 2012-07-31 12:44 ` Christian Borntraeger
2012-07-31 12:52 ` Alexander Graf
0 siblings, 1 reply; 33+ messages in thread
From: Christian Borntraeger @ 2012-07-31 12:44 UTC (permalink / raw)
To: Alexander Graf
Cc: Blue Swirl, Heinz Graalfs, qemu-devel, Jens Freimann, afaerber
On 30/07/12 16:05, Alexander Graf wrote:
>
> On 26.07.2012, at 10:59, Christian Borntraeger wrote:
>
>> This patch makes the sclp ascii default for S390. It requires a guest
>> kernel that autodetects the console and which not blindly assumes
>> that kvm means virtio console.
>> (commit cd1834591fe9564720ac4b0193bf1c790fe89f0d
>> KVM: s390: Perform early event mask processing during boot)
>> Otherwise the guest admin has to explicitely add console=ttyS1 to the
>> command line.
>
> Hrm. Could we make this the non-default for the time being with a simple -machine option to switch it to the default? When enough kernels are aware of the switch, we could then make it the default.
No problem, we can defer that patch. It was merely a testing aid for those who want to test the
sclp code. Later on maybe we can make that decision based on other thing.
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [Qemu-devel] [PATCH v4 07/07] s390: make sclp ascii console the default
2012-07-31 12:44 ` Christian Borntraeger
@ 2012-07-31 12:52 ` Alexander Graf
0 siblings, 0 replies; 33+ messages in thread
From: Alexander Graf @ 2012-07-31 12:52 UTC (permalink / raw)
To: Christian Borntraeger
Cc: Blue Swirl, Heinz Graalfs, qemu-devel, Jens Freimann, afaerber
On 07/31/2012 02:44 PM, Christian Borntraeger wrote:
> On 30/07/12 16:05, Alexander Graf wrote:
>> On 26.07.2012, at 10:59, Christian Borntraeger wrote:
>>
>>> This patch makes the sclp ascii default for S390. It requires a guest
>>> kernel that autodetects the console and which not blindly assumes
>>> that kvm means virtio console.
>>> (commit cd1834591fe9564720ac4b0193bf1c790fe89f0d
>>> KVM: s390: Perform early event mask processing during boot)
>>> Otherwise the guest admin has to explicitely add console=ttyS1 to the
>>> command line.
>> Hrm. Could we make this the non-default for the time being with a simple -machine option to switch it to the default? When enough kernels are aware of the switch, we could then make it the default.
> No problem, we can defer that patch. It was merely a testing aid for those who want to test the
> sclp code. Later on maybe we can make that decision based on other thing.
Well, I'd like to have the logic around and easy to choose. Basically
what I envision is that one day we do a flag day that introduces a
compatibility machine type which only differs from the current one in
that it sets said machine opt to "use ascii console as default".
That way for the time being, people who want to play with it only need
to do -machine ascii_console=on and everything else would happen
automatically. It'd definitely make it easier for everyone involved.
Alex
^ permalink raw reply [flat|nested] 33+ messages in thread
* [Qemu-devel] [PATCH v4 07/07] s390: make sclp ascii console the default
2012-07-24 19:35 ` Blue Swirl
2012-07-25 6:55 ` Christian Borntraeger
2012-07-26 8:59 ` [Qemu-devel] [PATCH v4 07/07] " Christian Borntraeger
@ 2012-07-26 9:11 ` Christian Borntraeger
2 siblings, 0 replies; 33+ messages in thread
From: Christian Borntraeger @ 2012-07-26 9:11 UTC (permalink / raw)
To: Blue Swirl
Cc: Heinz Graalfs, qemu-devel, Alexander Graf, Christian Borntraeger,
Jens Freimann, afaerber
This patch makes the sclp ascii default for S390. It requires a guest
kernel that autodetects the console and which not blindly assumes
that kvm means virtio console.
(commit cd1834591fe9564720ac4b0193bf1c790fe89f0d
KVM: s390: Perform early event mask processing during boot)
Otherwise the guest admin has to explicitely add console=ttyS1 to the
command line.
Signed-off-by: Christian Borntraeger <borntraeger@de.ibm.com>
---
hw/s390-virtio.c | 1 -
vl.c | 48 ++++++++++++++++++++++++++++++++++++++++++++++++
2 files changed, 48 insertions(+), 1 deletions(-)
diff --git a/hw/s390-virtio.c b/hw/s390-virtio.c
index 28e320d..8b48f66 100644
--- a/hw/s390-virtio.c
+++ b/hw/s390-virtio.c
@@ -341,7 +341,6 @@ static QEMUMachine s390_machine = {
.no_serial = 1,
.no_parallel = 1,
.no_sdcard = 1,
- .use_virtcon = 1,
.max_cpus = 255,
.is_default = 1,
};
diff --git a/vl.c b/vl.c
index 46248b9..30fd506 100644
--- a/vl.c
+++ b/vl.c
@@ -168,6 +168,7 @@ int main(int argc, char **argv)
#define DEFAULT_RAM_SIZE 128
#define MAX_VIRTIO_CONSOLES 1
+#define MAX_SCLP_CONSOLES 1
static const char *data_dir;
const char *bios_name = NULL;
@@ -195,6 +196,7 @@ int no_quit = 0;
CharDriverState *serial_hds[MAX_SERIAL_PORTS];
CharDriverState *parallel_hds[MAX_PARALLEL_PORTS];
CharDriverState *virtcon_hds[MAX_VIRTIO_CONSOLES];
+CharDriverState *sclpcon_hds[MAX_SCLP_CONSOLES];
int win2k_install_hack = 0;
int usb_enabled = 0;
int singlestep = 0;
@@ -268,6 +270,7 @@ static int default_floppy = 1;
static int default_cdrom = 1;
static int default_sdcard = 1;
static int default_vga = 1;
+static int default_sclpcon = 1;
static struct {
const char *driver;
@@ -289,6 +292,7 @@ static struct {
{ .driver = "isa-cirrus-vga", .flag = &default_vga },
{ .driver = "vmware-svga", .flag = &default_vga },
{ .driver = "qxl-vga", .flag = &default_vga },
+ { .driver = "sclpconsole", .flag = &default_sclpcon },
};
static void res_free(void)
@@ -1935,6 +1939,7 @@ struct device_config {
DEV_VIRTCON, /* -virtioconsole */
DEV_DEBUGCON, /* -debugcon */
DEV_GDB, /* -gdb, -s */
+ DEV_SCLPCON, /* sclp console */
} type;
const char *cmdline;
Location loc;
@@ -2014,6 +2019,37 @@ static int parallel_parse(const char *devname)
return 0;
}
+static int sclpcon_parse(const char *devname)
+{
+ QemuOptsList *device = qemu_find_opts("device");
+ static int index;
+ char label[32];
+ QemuOpts *dev_opts;
+
+ if (strcmp(devname, "none") == 0) {
+ return 0;
+ }
+ if (index == MAX_SCLP_CONSOLES) {
+ fprintf(stderr, "qemu: only one sclpconsole may be specified\n");
+ exit(1);
+ }
+
+ dev_opts = qemu_opts_create(device, NULL, 0, NULL);
+ qemu_opt_set(dev_opts, "driver", "sclpconsole");
+
+ snprintf(label, sizeof(label), "sclpcon%d", index);
+ sclpcon_hds[index] = qemu_chr_new(label, devname, NULL);
+ if (!sclpcon_hds[index]) {
+ fprintf(stderr, "qemu: could not open sclp console '%s': %s\n",
+ devname, strerror(errno));
+ return -1;
+ }
+ qemu_opt_set(dev_opts, "chardev", label);
+
+ index++;
+ return 0;
+}
+
static int virtcon_parse(const char *devname)
{
QemuOptsList *device = qemu_find_opts("device");
@@ -3122,6 +3158,7 @@ int main(int argc, char **argv, char **envp)
default_cdrom = 0;
default_sdcard = 0;
default_vga = 0;
+ default_sclpcon = 0;
break;
case QEMU_OPTION_xen_domid:
if (!(xen_available())) {
@@ -3303,11 +3340,16 @@ int main(int argc, char **argv, char **envp)
add_device_config(DEV_PARALLEL, "null");
if (default_serial && default_monitor) {
add_device_config(DEV_SERIAL, "mon:stdio");
+ } else if (default_sclpcon && default_monitor) {
+ add_device_config(DEV_SCLPCON, "mon:stdio");
} else if (default_virtcon && default_monitor) {
add_device_config(DEV_VIRTCON, "mon:stdio");
} else {
if (default_serial)
add_device_config(DEV_SERIAL, "stdio");
+ if (default_sclpcon) {
+ add_device_config(DEV_SCLPCON, "stdio");
+ }
if (default_virtcon)
add_device_config(DEV_VIRTCON, "stdio");
if (default_monitor)
@@ -3320,6 +3362,9 @@ int main(int argc, char **argv, char **envp)
add_device_config(DEV_PARALLEL, "vc:80Cx24C");
if (default_monitor)
monitor_parse("vc:80Cx24C", "readline");
+ if (default_sclpcon) {
+ add_device_config(DEV_SCLPCON, "vc:80Cx24C");
+ }
if (default_virtcon)
add_device_config(DEV_VIRTCON, "vc:80Cx24C");
}
@@ -3490,6 +3535,9 @@ int main(int argc, char **argv, char **envp)
exit(1);
if (foreach_device_config(DEV_PARALLEL, parallel_parse) < 0)
exit(1);
+ if (foreach_device_config(DEV_SCLPCON, sclpcon_parse) < 0) {
+ exit(1);
+ }
if (foreach_device_config(DEV_VIRTCON, virtcon_parse) < 0)
exit(1);
if (foreach_device_config(DEV_DEBUGCON, debugcon_parse) < 0)
--
1.7.0.1
^ permalink raw reply related [flat|nested] 33+ messages in thread
* Re: [Qemu-devel] [PATCH 0/7v3] s390: sclp patch set
2012-07-24 7:37 [Qemu-devel] [PATCH 0/7v3] s390: sclp patch set Christian Borntraeger
` (6 preceding siblings ...)
2012-07-24 7:37 ` [Qemu-devel] [PATCH 7/7] s390: make sclp ascii console the default Christian Borntraeger
@ 2012-07-30 9:00 ` Christian Borntraeger
2012-07-30 14:11 ` Alexander Graf
7 siblings, 1 reply; 33+ messages in thread
From: Christian Borntraeger @ 2012-07-30 9:00 UTC (permalink / raw)
To: Alexander Graf
Cc: Heinz Graalfs, qemu-devel, blauwirbel, Christian Borntraeger,
Jens Freimann, afaerber
Alex,
any opinion on patches 1-6? Ok to apply?
Christian
^ permalink raw reply [flat|nested] 33+ messages in thread* Re: [Qemu-devel] [PATCH 0/7v3] s390: sclp patch set
2012-07-30 9:00 ` [Qemu-devel] [PATCH 0/7v3] s390: sclp patch set Christian Borntraeger
@ 2012-07-30 14:11 ` Alexander Graf
0 siblings, 0 replies; 33+ messages in thread
From: Alexander Graf @ 2012-07-30 14:11 UTC (permalink / raw)
To: Christian Borntraeger
Cc: blauwirbel, Heinz Graalfs, qemu-devel, afaerber, Jens Freimann
On 30.07.2012, at 11:00, Christian Borntraeger wrote:
> Alex,
>
> any opinion on patches 1-6? Ok to apply?
Thanks, I applied 1 and 2 to s390-next, but had comments on the rest. Sorry again for the delay.
Alex
^ permalink raw reply [flat|nested] 33+ messages in thread