[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 6/8] s390: sclp event facility and signal quiesc
|
From: |
Heinz Graalfs |
|
Subject: |
Re: [Qemu-devel] [PATCH 6/8] s390: sclp event facility and signal quiesce support via system_powerdown |
|
Date: |
Wed, 13 Jun 2012 09:00:21 +0200 |
Alex, thanks for the comments,
On Tue, 2012-06-12 at 13:38 +0200, Alexander Graf wrote:
> On 06/06/2012 02:05 PM, Jens Freimann wrote:
> > From: Heinz Graalfs<address@hidden>
> >
> > This patch implements a subset of the sclp event facility as well
> > as the signal quiesce event. This allows to gracefully shutdown
> > a guest by using system_powerdown. This sends a signal quiesce
> > signal that is interpreted by linux guests as ctrl-alt-del.
> > In addition the event facility is modeled using the QOM.
> >
> > Signed-off-by: Heinz Graalfs<address@hidden>
> > Signed-off-by: Christian Borntraeger<address@hidden>
> > Signed-off-by: Jens Freimann<address@hidden>
>
> Andreas, I'm always getting headaches reviewing qdev and/or qom patches.
> Could you please check this for layering violations?
>
> > ---
> > Makefile.target | 1 +
> > hw/s390-event-facility.c | 232 +++++++++++++++++++++++++++++++++++++++++
> > hw/s390-event-facility.h | 54 ++++++++++
> > hw/s390-sclp.c | 256
> > +++++++++++++++++++++++++++++++++++++++++++++-
> > hw/s390-sclp.h | 98 +++++++++++++++++-
> > hw/s390-virtio.c | 11 +-
> > target-s390x/op_helper.c | 3 +
> > 7 files changed, 649 insertions(+), 6 deletions(-)
> > create mode 100644 hw/s390-event-facility.c
> > create mode 100644 hw/s390-event-facility.h
> >
> > diff --git a/Makefile.target b/Makefile.target
> > index fed2d72..f939c3a 100644
> > --- a/Makefile.target
> > +++ b/Makefile.target
> > @@ -375,6 +375,7 @@ obj-m68k-y = an5206.o mcf5206.o mcf_uart.o mcf_intc.o
> > mcf5208.o mcf_fec.o
> > obj-m68k-y += m68k-semi.o dummy_m68k.o
> >
> > obj-s390x-y = s390-virtio-bus.o s390-virtio.o s390-sclp.o
> > +obj-s390x-y += s390-event-facility.o
> >
> > obj-alpha-y = mc146818rtc.o
> > obj-alpha-y += alpha_pci.o alpha_dp264.o alpha_typhoon.o
> > diff --git a/hw/s390-event-facility.c b/hw/s390-event-facility.c
> > new file mode 100644
> > index 0000000..b8106a6
> > --- /dev/null
> > +++ b/hw/s390-event-facility.c
> > @@ -0,0 +1,232 @@
> > +/*
> > + * SCLP Event Facility
> > + *
> > + * Copyright IBM Corp. 2007,2012
> > + * Author: Heinz Graalfs<address@hidden>
> > + *
> > + * This file is licensed under the terms of the GNU General Public
> > License(GPL)
> > + */
> > +
> > +#include "iov.h"
> > +#include "monitor.h"
> > +#include "qemu-queue.h"
> > +#include "sysbus.h"
> > +#include "sysemu.h"
> > +
> > +#include "s390-sclp.h"
> > +#include "s390-event-facility.h"
> > +
> > +struct SCLPDevice {
> > + const char *name;
> > + bool vm_running;
> > +};
> > +
> > +struct SCLP {
> > + BusState qbus;
> > + SCLPEventFacility *event_facility;
> > +};
> > +
> > +struct SCLPEventFacility {
> > + SCLPDevice sdev;
> > + SCLP sbus;
> > + DeviceState *qdev;
> > + void *opaque;
> > +};
> > +
> > +typedef struct SCLPConsole {
> > + SCLPEvent event;
> > + CharDriverState *chr;
> > +} SCLPConsole;
> > +
> > +static void sclpef_bus_dev_print(Monitor *mon, DeviceState *qdev, int
> > indent)
> > +{
> > + SCLPEvent *event = DO_UPCAST(SCLPEvent, dev, qdev);
> > +
> > + monitor_printf(mon, "%*sid %d\n", indent, "", event->id);
> > +}
> > +
> > +static unsigned int send_mask_quiesce(void)
> > +{
> > + return SCLP_EVENT_MASK_SIGNAL_QUIESCE;
> > +}
> > +
> > +static unsigned int receive_mask_quiesce(void)
> > +{
> > + return 0;
> > +}
> > +
> > +static void s390_signal_quiesce(void *opaque, int n, int level)
> > +{
> > + sclp_enable_signal_quiesce();
> > + sclp_service_interrupt(opaque, 0);
> > +}
> > +
> > +unsigned int sclpef_send_mask(SCLPDevice *sdev)
> > +{
> > + unsigned int mask;
> > + DeviceState *dev;
> > + SCLPEventFacility *event_facility;
> > + SCLPEventClass *cons;
> > +
> > + event_facility = DO_UPCAST(SCLPEventFacility, sdev, sdev);
> > +
> > + mask = 0;
> > +
> > + QTAILQ_FOREACH(dev,&event_facility->sbus.qbus.children, sibling) {
> > + cons = SCLP_EVENT_GET_CLASS((SCLPEvent *) dev);
> > + mask |= cons->get_send_mask();
> > + }
> > + return mask;
> > +}
> > +
> > +unsigned int sclpef_receive_mask(SCLPDevice *sdev)
> > +{
> > + unsigned int mask;
> > + DeviceState *dev;
> > + SCLPEventClass *cons;
> > + SCLPEventFacility *event_facility;
> > +
> > + event_facility = DO_UPCAST(SCLPEventFacility, sdev, sdev);
> > +
> > + mask = 0;
> > +
> > + QTAILQ_FOREACH(dev,&event_facility->sbus.qbus.children, sibling) {
> > + cons = SCLP_EVENT_GET_CLASS((SCLPEvent *) dev);
> > + mask |= cons->get_receive_mask();
> > + }
> > + return mask;
> > +}
> > +
> > +static struct BusInfo sclp_bus_info = {
> > + .name = "s390-sclp-bus",
> > + .size = sizeof(SCLPS390Bus),
> > + .print_dev = sclpef_bus_dev_print,
> > + .props = (Property[]) {
> > + DEFINE_PROP_STRING("name", SCLPEvent, name),
> > + DEFINE_PROP_END_OF_LIST()
> > + }
> > +};
> > +
> > +static int sclpef_qdev_init(DeviceState *qdev)
> > +{
> > + int rc;
> > + SCLPEvent *event = DO_UPCAST(SCLPEvent, dev, qdev);
> > + SCLPEventClass *cons = SCLP_EVENT_GET_CLASS(event);
> > + SCLP *bus = DO_UPCAST(SCLP, qbus, qdev->parent_bus);
> > +
> > + event->evt_fac = bus->event_facility;
> > + rc = cons->init(event);
> > +
> > + return rc;
> > +}
> > +
> > +static int sclpef_qdev_exit(DeviceState *qdev)
> > +{
> > + SCLPEvent *event = DO_UPCAST(SCLPEvent, dev, qdev);
> > + SCLPEventClass *cons = SCLP_EVENT_GET_CLASS(event);
> > + if (cons->exit) {
> > + cons->exit(event);
> > + }
> > + return 0;
> > +}
> > +
> > +static SCLPDevice *sclpef_common_init(const char *name, size_t struct_size)
> > +{
> > + SCLPDevice *sdev;
> > +
> > + sdev = malloc(struct_size);
>
> g_malloc please. I suppose even g_malloc0?
>
OK, I will look into this
> > + if (!sdev) {
> > + return NULL;
> > + }
> > + sdev->vm_running = runstate_is_running();
> > + sdev->name = name;
> > +
> > + return sdev;
> > +}
> > +
> > +void sclpef_enable_irqs(SCLPDevice *sdev, void *opaque)
> > +{
> > + SCLPEventFacility *event_facility;
> > +
> > + event_facility = DO_UPCAST(SCLPEventFacility, sdev, sdev);
> > + qemu_system_powerdown = *qemu_allocate_irqs(s390_signal_quiesce,
> > + opaque, 1);
> > + event_facility->opaque = opaque;
> > +}
> > +
> > +SCLPDevice *sclpef_init_event_facility(DeviceState *dev)
> > +{
> > + DeviceState *quiesce;
> > + SCLPDevice *sdev;
> > + SCLPEventFacility *event_facility;
> > +
> > + sdev = sclpef_common_init("sclp-event-facility",
> > + sizeof(SCLPEventFacility));
> > +
> > + if (!sdev) {
> > + return NULL;
> > + }
> > + event_facility = DO_UPCAST(SCLPEventFacility, sdev, sdev);
> > +
> > + /* Spawn a new sclp-facility */
> > + qbus_create_inplace(&event_facility->sbus.qbus,&sclp_bus_info, dev,
> > NULL);
> > + event_facility->sbus.qbus.allow_hotplug = 0;
> > + event_facility->sbus.event_facility = event_facility;
> > + event_facility->qdev = dev;
> > + event_facility->opaque = NULL;
> > + quiesce = qdev_create(&event_facility->sbus.qbus, "sclpquiesce");
> > +
> > + if (!quiesce) {
> > + return NULL;
> > + }
> > +
> > + return sdev;
> > +}
> > +
> > +static void sclpef_class_init(ObjectClass *klass, void *data)
> > +{
> > + DeviceClass *dc = DEVICE_CLASS(klass);
> > + dc->init = sclpef_qdev_init;
> > + dc->bus_info =&sclp_bus_info;
> > + dc->exit = sclpef_qdev_exit;
> > + dc->unplug = qdev_simple_unplug_cb;
> > +}
> > +
> > +static TypeInfo sclp_event_facility_type_info = {
> > + .name = TYPE_SCLP_EVENT,
> > + .parent = TYPE_DEVICE,
> > + .instance_size = sizeof(SCLPEvent),
> > + .class_size = sizeof(SCLPEventClass),
> > + .class_init = sclpef_class_init,
> > + .abstract = true,
> > +};
> > +
> > +static int sclpquiesce_initfn(SCLPEvent *event)
> > +{
> > + event->id = ID_QUIESCE;
> > +
> > + return 0;
> > +}
> > +
> > +static void sclpef_quiesce_class_init(ObjectClass *klass, void *data)
> > +{
> > + SCLPEventClass *k = SCLP_EVENT_CLASS(klass);
> > +
> > + k->init = sclpquiesce_initfn;
> > + k->get_send_mask = send_mask_quiesce;
> > + k->get_receive_mask = receive_mask_quiesce;
> > +}
> > +
> > +static TypeInfo sclp_quiesce_info = {
> > + .name = "sclpquiesce",
> > + .parent = TYPE_SCLP_EVENT,
> > + .instance_size = sizeof(SCLPEvent),
> > + .class_init = sclpef_quiesce_class_init,
> > +};
> > +
> > +static void sclpef_register_types(void)
> > +{
> > + type_register_static(&sclp_event_facility_type_info);
> > + type_register_static(&sclp_quiesce_info);
> > +}
> > +type_init(sclpef_register_types)
> > diff --git a/hw/s390-event-facility.h b/hw/s390-event-facility.h
> > new file mode 100644
> > index 0000000..40d4049
> > --- /dev/null
> > +++ b/hw/s390-event-facility.h
> > @@ -0,0 +1,54 @@
> > +/*
> > + * SCLP definitions
> > + *
> > + * Copyright IBM Corp. 2007,2012
> > + * Author: Heinz Graalfs<address@hidden>
> > + *
> > + * This file is licensed under the terms of the GNU General Public
> > License(GPL)
> > + */
> > +
> > +#ifndef _QEMU_SCLP_EVENT_H
> > +#define _QEMU_SCLP_EVENT_H
> > +
> > +#include "qdev.h"
> > +#include "qemu-common.h"
> > +
> > +#define ID_QUIESCE SCLP_EVENT_SIGNAL_QUIESCE
> > +
> > +#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 SCLP SCLP;
> > +typedef struct SCLPEventFacility SCLPEventFacility;
> > +typedef struct SCLPEvent SCLPEvent;
> > +typedef struct SCLPDevice SCLPDevice;
> > +
> > +typedef struct SCLPEventClass {
> > + DeviceClass parent_class;
> > + int (*init)(SCLPEvent *event);
> > + int (*exit)(SCLPEvent *event);
> > + unsigned int (*get_send_mask)(void);
> > + unsigned int (*get_receive_mask)(void);
> > +} SCLPEventClass;
> > +
> > +struct SCLPEvent {
> > + DeviceState dev;
> > + SCLPEventFacility *evt_fac;
> > + char *name;
> > + uint32_t id;
> > +};
> > +
> > +SCLPDevice *sclpef_init_event_facility(DeviceState *dev);
> > +
> > +void sclpef_enable_irqs(SCLPDevice *sdev, void *opaque);
> > +
> > +void sclpef_set_masks(void);
> > +unsigned int sclpef_send_mask(SCLPDevice *sdev);
> > +unsigned int sclpef_receive_mask(SCLPDevice *sdev);
> > +
> > +#endif
> > diff --git a/hw/s390-sclp.c b/hw/s390-sclp.c
> > index c046441..683a709 100644
> > --- a/hw/s390-sclp.c
> > +++ b/hw/s390-sclp.c
> > @@ -7,7 +7,20 @@
> >
> > #include "cpu.h"
> > #include "kvm.h"
> > +#include "hw/sysbus.h"
> > #include "hw/s390-sclp.h"
> > +#include "hw/s390-event-facility.h"
> > +
> > +/* Host capabilites */
> > +static unsigned int sclp_send_mask;
> > +static unsigned int sclp_receive_mask;
> > +
> > +/* Guest capabilities */
> > +static unsigned int sclp_cp_send_mask;
> > +static unsigned int sclp_cp_receive_mask;
> > +
> > +static int quiesce;
> > +static int event_pending;
>
> Global variables? Bad idea :). These should be properties of your
> quiesce device.
>
OK, I'll look onto this
> >
> > int sclp_read_info(CPUS390XState *env, struct sccb *sccb)
> > {
> > @@ -23,20 +36,257 @@ int sclp_read_info(CPUS390XState *env, struct sccb
> > *sccb)
> > return 0;
> > }
> >
> > +static int signal_quiesce_event(struct sccb *sccb, int *slen)
> > +{
> > + if (!quiesce) {
> > + return 0;
> > + }
> > +
> > + if (*slen< sizeof(struct signal_quiesce)) {
> > + event_pending = 1;
> > + return 0;
> > + }
> > +
> > + sccb->c.read.quiesce.h.length = cpu_to_be16(sizeof(struct
> > signal_quiesce));
> > + sccb->c.read.quiesce.h.type = SCLP_EVENT_SIGNAL_QUIESCE;
> > + sccb->c.read.quiesce.h.flags&= ~SCLP_EVENT_BUFFER_ACCEPTED;
> > + /*
> > + * system_powerdown does not have a timeout. Fortunately the
> > + * timeout value is curently ignored by Linux, anyway
> > + */
> > + sccb->c.read.quiesce.timeout = cpu_to_be16(0);
> > + sccb->c.read.quiesce.unit = cpu_to_be16(0);
> > +
> > + quiesce = 0;
> > + *slen -= sizeof(struct signal_quiesce);
> > + return 1;
> > +}
> > +
> > +void sclp_enable_signal_quiesce(void)
> > +{
> > + quiesce = 1;
> > + event_pending = 1;
> > +}
> > +
> > +static void sclp_set_masks(void)
> > +{
> > + SCLPS390EventFacility *evt_fac;
> > +
> > + if (!sclp_bus) {
> > + return;
> > + }
> > + evt_fac = sclp_bus->event_facility;
> > +
> > + assert(evt_fac);
> > +
> > + sclp_send_mask = sclpef_send_mask(evt_fac->sdev);
> > + sclp_receive_mask = sclpef_receive_mask(evt_fac->sdev);
> > + }
> > +
> > +int sclp_read_event_data(CPUS390XState *env, struct sccb *sccb)
> > +{
> > + unsigned int sclp_active_selection_mask;
> > + int slen;
> > +
> > + if (be16_to_cpu(sccb->h.length) != SCCB_SIZE) {
> > + sccb->h.response_code =
> > cpu_to_be16(SCLP_RC_INSUFFICIENT_SCCB_LENGTH);
> > + goto out;
> > + }
> > +
> > + 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(sccb->c.read.mask))) {
> > + sccb->h.response_code =
> > cpu_to_be16(SCLP_RC_INVALID_SELECTION_MASK);
> > + goto out;
> > + }
> > + sclp_active_selection_mask = be32_to_cpu(sccb->c.read.mask);
> > + break;
> > + default:
> > + sccb->h.response_code = cpu_to_be16(SCLP_RC_INVALID_FUNCTION);
> > + goto out;
> > + }
> > +
> > + slen = sizeof(sccb->c.data);
> > + sccb->h.response_code = cpu_to_be16(SCLP_RC_NO_EVENT_BUFFERS_STORED);
> > +
> > + if (sclp_active_selection_mask& SCLP_EVENT_MASK_SIGNAL_QUIESCE) {
> > + if (signal_quiesce_event(sccb,&slen)) {
> > + sccb->h.response_code = cpu_to_be16(SCLP_RC_NORMAL_COMPLETION);
> > + }
> > + }
> > +
> > + 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);
> > + }
> > +
> > +out:
> > + return 0;
> > +}
> > +
> > +int sclp_write_event_mask(CPUS390XState *env, struct sccb *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(sccb->c.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 */
> > + sclp_cp_send_mask = be32_to_cpu(sccb->c.we_mask.cp_send_mask);
> > + sclp_cp_receive_mask = be32_to_cpu(sccb->c.we_mask.cp_receive_mask);
> > +
> > + sclp_set_masks();
> > +
> > + /* return the SCLP's capability masks to the guest */
> > + sccb->c.we_mask.send_mask = cpu_to_be32(sclp_send_mask);
> > + sccb->c.we_mask.receive_mask = cpu_to_be32(sclp_receive_mask);
> > +
> > + sccb->h.response_code = cpu_to_be32(SCLP_RC_NORMAL_COMPLETION);
> > +
> > +out:
> > + return 0;
> > +}
> > +
> > void sclp_service_interrupt(CPUS390XState *env, uint32_t sccb)
> > {
> > - if (!sccb) {
> > + if (!event_pending&& !sccb) {
> > return;
> > }
> >
> > if (kvm_enabled()) {
> > #ifdef CONFIG_KVM
> > kvm_s390_interrupt_internal(env, KVM_S390_INT_SERVICE,
> > - (sccb& ~3), 0, 1);
> > + (sccb& ~3) + event_pending, 0, 1);
> > #endif
> > } else {
> > env->psw.addr += 4;
> > - cpu_inject_ext(env, EXT_SERVICE, (sccb& ~3), 0);
> > + cpu_inject_ext(env, EXT_SERVICE, (sccb& ~3) + event_pending, 0);
> > + }
>
> Please fix the above piece of code to use the same mechanism regarless
> of TCG or KVM, so that SCLP code doesn't have to be aware of KVM.
>
> > + event_pending = 0;
> > +}
> > +
> > +struct BusInfo s390_sclp_bus_info = {
> > + .name = "s390-sclp",
> > + .size = sizeof(SCLPS390Bus),
> > +};
> > +
> > +SCLPS390Bus *s390_sclp_bus_init(void)
> > +{
> > + SCLPS390Bus *bus;
> > + BusState *bus_state;
> > + DeviceState *dev;
> > +
> > + dev = qdev_create(NULL, "s390-sclp-bridge");
> > + qdev_init_nofail(dev);
> > + bus_state = qbus_create(&s390_sclp_bus_info, dev, "s390-sclp-bus");
> > + bus = DO_UPCAST(SCLPS390Bus, bus, bus_state);
> > +
> > + bus_state->allow_hotplug = 0;
> > +
> > + return bus;
> > +}
> > +
> > +static int s390_sclp_device_init(SCLPS390EventFacility *dev, SCLPDevice
> > *sdev)
> > +{
> > + dev->sdev = sdev;
> > +
> > + return 0;
> > +}
> > +
> > +static int s390_sclp_init(SCLPS390EventFacility *dev)
> > +{
> > + int rc;
> > + SCLPS390Bus *bus;
> > + SCLPDevice *sdev;
> > +
> > + bus = DO_UPCAST(SCLPS390Bus, bus, dev->qdev.parent_bus);
> > +
> > + sdev = sclpef_init_event_facility((DeviceState *)dev);
> > + if (!sdev) {
> > + return -1;
> > }
> > +
> > + rc = s390_sclp_device_init(dev, sdev);
> > + if (!rc) {
> > + bus->event_facility = dev;
> > + }
> > +
> > + return rc;
> > +}
> > +
> > +static void s390_sclp_class_init(ObjectClass *klass, void *data)
> > +{
> > + SCLPS390EventFacilityClass *k = SCLP_S390_EVENT_FACILITY_CLASS(klass);
> > +
> > + k->init = s390_sclp_init;
> > +}
> > +
> > +static TypeInfo s390_sclp_event_facility = {
> > + .name = "sclp-event-facility",
> > + .parent = TYPE_SCLP_S390_EVENT_FACILITY,
> > + .instance_size = sizeof(SCLPS390EventFacility),
> > + .class_init = s390_sclp_class_init,
> > +};
> > +
> > +static int s390_sclp_busdev_init(DeviceState *dev)
> > +{
> > + SCLPS390EventFacility *_dev = (SCLPS390EventFacility *)dev;
> > + SCLPS390EventFacilityClass *evt_fac =
> > + SCLP_S390_EVENT_FACILITY_GET_CLASS(dev);
> > +
> > + return evt_fac->init(_dev);
> > }
> >
> > +static void sclp_s390_device_class_init(ObjectClass *klass, void *data)
> > +{
> > + DeviceClass *dc = DEVICE_CLASS(klass);
> > +
> > + dc->init = s390_sclp_busdev_init;
> > + dc->bus_info =&s390_sclp_bus_info;
> > +}
> > +
> > +static TypeInfo sclp_s390_device_info = {
> > + .name = TYPE_SCLP_S390_EVENT_FACILITY,
> > + .parent = TYPE_DEVICE,
> > + .instance_size = sizeof(SCLPS390EventFacility),
> > + .class_init = sclp_s390_device_class_init,
> > + .class_size = sizeof(SCLPS390EventFacilityClass),
> > + .abstract = true,
> > +};
> > +
> > +/***************** S390 SCLP Bus Bridge Device *******************/
> > +/* Only required to have the sclp bus as child in the system bus */
> > +
> > +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(&sclp_s390_device_info);
> > + type_register_static(&s390_sclp_event_facility);
> > + type_register_static(&s390_sclp_bridge_info);
> > +}
> > +type_init(s390_sclp_register_types)
> > diff --git a/hw/s390-sclp.h b/hw/s390-sclp.h
> > index e335b21..f61421b 100644
> > --- a/hw/s390-sclp.h
> > +++ b/hw/s390-sclp.h
> > @@ -1,13 +1,69 @@
> > #include<stdint.h>
> > #include<qemu-common.h>
> >
> > +#include "sysbus.h"
> > +#include "hw/s390-event-facility.h"
> >
> > /* SCLP command codes */
> > #define SCLP_CMDW_READ_SCP_INFO 0x00020001
> > #define SCLP_CMDW_READ_SCP_INFO_FORCED 0x00120001
> > +#define SCLP_CMD_WRITE_EVENT_MASK 0x00780005
> >
> > /* SCLP response codes */
> > -#define SCLP_RC_SCCB_RESOURCE_INSUFFICENT 0x07f0
> > +#define SCLP_RC_NORMAL_COMPLETION 0x0020
> > +#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
> > +
> > +/* SCLP event types */
> > +#define SCLP_EVENT_SIGNAL_QUIESCE 0x1d
> > +
> > +/* SCLP event masks */
> > +#define SCLP_EVENT_MASK_SIGNAL_QUIESCE 0x00000008
> > +#define SCLP_EVENT_MASK_MSG 0x40000000
> > +
> > +#define SCLP_UNCONDITIONAL_READ 0x00
> > +#define SCLP_SELECTIVE_READ 0x01
> > +
> > +#define SCLP_VARIABLE_LENGTH_RESPONSE 0x80
> > +
> > +#define SCCB_SIZE 4096
> > +
> > +/* Service Call Control Block (SCCB) and its elements */
> > +
> > +struct write_event_mask {
> > + 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;
> > +} __attribute__((packed));
> > +
> > +struct event_buffer_header {
> > + uint16_t length;
> > + uint8_t type;
> > +#define SCLP_EVENT_BUFFER_ACCEPTED 0x80
> > + uint8_t flags;
> > + uint16_t _reserved;
> > +} __attribute__((packed));
> > +
> > +struct signal_quiesce {
> > + struct event_buffer_header h;
> > + uint16_t timeout;
> > + uint8_t unit;
> > +} __attribute__((packed));
> > +
> > +struct read_event_data {
> > + union {
> > + struct signal_quiesce quiesce;
> > + uint32_t mask;
> > + };
> > +} __attribute__((packed));
> >
> > struct sccb_header {
> > uint16_t length;
> > @@ -22,13 +78,51 @@ struct read_info_sccb {
> > uint8_t rnsize;
> > } __attribute__((packed));
> >
> > +#define SCCB_DATA_LEN 4088
> > +
> > struct sccb {
> > struct sccb_header h;
> > union {
> > struct read_info_sccb read_info;
> > - char data[4088];
> > + struct read_event_data read;
> > + struct write_event_mask we_mask;
> > + char data[SCCB_DATA_LEN];
> > } c;
> > } __attribute__((packed));
> >
> > +void sclp_enable_signal_quiesce(void);
> > int sclp_read_info(CPUS390XState *env, struct sccb *sccb);
> > +int sclp_read_event_data(CPUS390XState *env, struct sccb *sccb);
> > +int sclp_write_event_mask(CPUS390XState *env, struct sccb *sccb);
> > void sclp_service_interrupt(CPUS390XState *env, uint32_t sccb);
> > +
> > +#define TYPE_SCLP_S390_EVENT_FACILITY "s390-sclp-event-facility"
> > +#define SCLP_S390_EVENT_FACILITY(obj) \
> > + OBJECT_CHECK(SCLPS390EventFacility, (obj),
> > TYPE_SCLP_S390_EVENT_FACILITY)
> > +#define SCLP_S390_EVENT_FACILITY_CLASS(klass) \
> > + OBJECT_CLASS_CHECK(SCLPS390EventFacilityClass, (klass), \
> > + TYPE_SCLP_S390_EVENT_FACILITY)
> > +#define SCLP_S390_EVENT_FACILITY_GET_CLASS(obj) \
> > + OBJECT_GET_CLASS(SCLPS390EventFacilityClass, (obj), \
> > + TYPE_SCLP_S390_EVENT_FACILITY)
> > +
> > +typedef struct SCLPS390EventFacility SCLPS390EventFacility;
> > +
> > +typedef struct SCLPS390EventFacilityClass {
> > + DeviceClass qdev;
> > + int (*init)(SCLPS390EventFacility *dev);
> > +} SCLPS390EventFacilityClass;
> > +
> > +struct SCLPS390EventFacility {
> > + DeviceState qdev;
> > + SCLPDevice *sdev;
> > +};
> > +
> > +typedef struct SCLPS390Bus {
> > + BusState bus;
> > + SCLPS390EventFacility *event_facility;
> > +} SCLPS390Bus;
> > +
> > +extern SCLPS390Bus *sclp_bus;
> > +
> > +SCLPS390Bus *s390_sclp_bus_init(void);
> > diff --git a/hw/s390-virtio.c b/hw/s390-virtio.c
> > index c0e19fd..0babf27 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/s390-sclp.h"
> >
> > //#define DEBUG_S390
> >
> > @@ -60,7 +61,9 @@
> >
> > #define MAX_BLK_DEVS 10
> >
> > -static VirtIOS390Bus *s390_bus;
> > +VirtIOS390Bus *s390_bus;
> > +SCLPS390Bus *sclp_bus;
> > +
> > static CPUS390XState **ipi_states;
> >
> > CPUS390XState *s390_cpu_addr2state(uint16_t cpu_addr)
> > @@ -170,6 +173,7 @@ static void s390_init(ram_addr_t my_ram_size,
> > target_phys_addr_t virtio_region_len;
> > target_phys_addr_t virtio_region_start;
> > int i;
> > + DeviceState *dev;
> >
> > /* s390x ram size detection needs a 16bit multiplier + an increment.
> > So
> > guests> 64GB can be specified in 2MB steps etc. */
> > @@ -183,6 +187,7 @@ static void s390_init(ram_addr_t my_ram_size,
> >
> > /* get a BUS */
> > s390_bus = s390_virtio_bus_init(&my_ram_size);
> > + sclp_bus = s390_sclp_bus_init();
> >
> > /* allocate RAM */
> > memory_region_init_ram(ram, "s390.ram", my_ram_size);
> > @@ -325,6 +330,10 @@ static void s390_init(ram_addr_t my_ram_size,
> > qdev_prop_set_drive_nofail(dev, "drive", dinfo->bdrv);
> > qdev_init_nofail(dev);
> > }
> > +
> > + dev = qdev_create((BusState *)sclp_bus, "sclp-event-facility");
> > + qdev_init_nofail(dev);
> > + sclpef_enable_irqs(sclp_bus->event_facility->sdev, ipi_states[0]);
> > }
> >
> > static QEMUMachine s390_machine = {
> > diff --git a/target-s390x/op_helper.c b/target-s390x/op_helper.c
> > index 74bd9ad..3e5eff4 100644
> > --- a/target-s390x/op_helper.c
> > +++ b/target-s390x/op_helper.c
> > @@ -2391,6 +2391,9 @@ int sclp_service_call(CPUS390XState *env, uint32_t
> > sccb, uint64_t code)
> > case SCLP_CMDW_READ_SCP_INFO_FORCED:
> > r = sclp_read_info(env,&work_sccb);
> > break;
> > + case SCLP_CMD_WRITE_EVENT_MASK:
> > + r = sclp_write_event_mask(env,&work_sccb);
> > + break;
> > default:
>
> I don't understand most of the patch tbh. It does a lot of things at the
> same time, but none of them really thorough. Please split this off into
> separate patches and make them actually do something small, but
> constistently.
>
> Things I saw while looking over this:
>
> - you create a bus to plug in sclp users. This needs to be separate.
> Don't introduce users of a framework when introducing the framework
> itself, unless really neccessary (or if the patch only becomes 5 lines
> longer)
> - you create objects for the new event parser, but not all the other
> sclp users. There's also no generic distribution mechanism in place
> - the event parser itself uses globals - that's nowhere near object
> oriented :)
> - I don't see the header inclusions i commented on in the last patch
> remedied here, please think your model over. In general, moving sclp to
> hw/ is probably a good idea, but then please do it in a properly
> abstracted way.
> - A lot of the above code could use some comments, so people reading
> it would actually understand what's going on :)
>
OK
> Alex
>
[Qemu-devel] [PATCH 5/8] s390: Cleanup sclp functions, Jens Freimann, 2012/06/06
Re: [Qemu-devel] [PATCH 5/8] s390: Cleanup sclp functions, Anthony Liguori, 2012/06/12