Re: [Qemu-devel] [PATCH 4/7] s390: sclp event support

["Andreas Färber" <afaerber@suse.de>]

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