Re: [Qemu-devel] [PATCH 7/8] s390: Add SCLP vt220 console support

[Alexander Graf <agraf@suse.de>]

On 06/06/2012 02:05 PM, Jens Freimann wrote:
> From: Heinz Graalfs<graalfs@linux.vnet.ibm.com>
>
> Adds console support (in vt220 mode).
> In order to run qemu exploiting the SCLP's console functionality in vt220 mode
> the user has to specify the following console related parameters:
>
>   -chardev stdio,id=charconsole0 -device sclpconsole,chardev=charconsole0,id=console0
>
> Signed-off-by: Heinz Graalfs<graalfs@linux.vnet.ibm.com>
> Signed-off-by: Jens Freimann<jfrei@linux.vnet.ibm.com>
> ---
>   hw/s390-event-facility.c |  209 ++++++++++++++++++++++++++++++++++++++++++++++
>   hw/s390-event-facility.h |    8 ++
>   hw/s390-sclp.c           |  177 ++++++++++++++++++++++++++++++++++++++-
>   hw/s390-sclp.h           |   22 ++++-
>   sysemu.h                 |    1 +
>   target-s390x/op_helper.c |    6 ++
>   vl.c                     |   41 +++++++++
>   7 files changed, 460 insertions(+), 4 deletions(-)
>
> diff --git a/hw/s390-event-facility.c b/hw/s390-event-facility.c
> index b8106a6..cfa5dd4 100644
> --- a/hw/s390-event-facility.c
> +++ b/hw/s390-event-facility.c
> @@ -16,6 +16,11 @@
>   #include "s390-sclp.h"
>   #include "s390-event-facility.h"
>
> +qemu_irq sclp_read_vt220;
> +
> +static int size_buffer = 4096;
> +static char *sclp_console_data_vt220;

Globals?

> +
>   struct SCLPDevice {
>       const char *name;
>       bool vm_running;
> @@ -224,9 +229,213 @@ static TypeInfo sclp_quiesce_info = {
>       .class_init    = sclpef_quiesce_class_init,
>   };
>
> +/* ----------- SCLP VT220 console ------------ */
> +
> +static void s390_signal_read_vt220(void *opaque, int n, int level)
> +{
> +    sclp_enable_signal_read_vt220();
> +    sclp_service_interrupt(opaque, 0);
> +}
> +
> +static void sclpef_set_console(SCLPEvent *event)
> +{
> +    if (event->id == ID_VT220) {
> +        sclp_read_vt220 = *qemu_allocate_irqs(s390_signal_read_vt220,
> +            event->evt_fac->opaque, 1);
> +    }
> +}
> +
> +void sclpef_write_console_vt220(SCLPDevice *sdev, char *buf)
> +{
> +    DeviceState *dev;
> +    SCLPEventFacility *event_facility;
> +    static SCLPEvent *event;
> +    static SCLPEventClass *cons;
> +
> +    event_facility = DO_UPCAST(SCLPEventFacility, sdev, sdev);
> +
> +    if (!cons) {
> +        QTAILQ_FOREACH(dev,&event_facility->sbus.qbus.children, sibling) {
> +            event = (SCLPEvent *) dev;
> +            if (event->id == ID_VT220) {
> +                cons = SCLP_EVENT_GET_CLASS(event);
> +                assert(cons->have_data);
> +                break;
> +            }
> +        }
> +    }

I don't understand the above code. Why do you have to search for 
anything when you're in a call to process a write?

> +    if (cons) {
> +        cons->have_data(event, (const uint8_t *)buf, strlen(buf));
> +    }
> +}
> +
> +char *sclpef_get_console_data_vt220(SCLPDevice *sdev)
> +{
> +    DeviceState *dev;
> +    SCLPEventFacility *event_facility;
> +    SCLPEvent *event = NULL;
> +    static SCLPEventClass *cons;
> +
> +    event_facility = DO_UPCAST(SCLPEventFacility, sdev, sdev);
> +
> +    if (!cons) {
> +        QTAILQ_FOREACH(dev,&event_facility->sbus.qbus.children, sibling) {
> +            event = (SCLPEvent *) dev;
> +            if (event->id == ID_VT220) {
> +                cons = SCLP_EVENT_GET_CLASS(event);
> +                assert(cons->get_data);
> +                break;
> +            }
> +        }
> +    }

See above.

> +    if (cons) {
> +        return cons->get_data();
> +    }
> +    return NULL;
> +}
> +
> +static char *console_data_vt220(void)
> +{
> +    return sclp_console_data_vt220;
> +}
> +
> +static ssize_t flush_buf(SCLPEvent *event, const uint8_t *buf, size_t len)
> +{
> +    SCLPConsole *scon = DO_UPCAST(SCLPConsole, event, event);
> +    ssize_t ret;
> +
> +    if (!scon->chr) {
> +        /* If there's no backend, we can just say we consumed all data. */
> +        return len;
> +    }
> +
> +    ret = qemu_chr_fe_write(scon->chr, buf, len);
> +
> +    if (ret<  0) {
> +        /* see virtio-console comments */
> +        ret = 0;
> +    }
> +
> +    return ret;
> +}
> +
> +static void guest_open(SCLPEvent *event)
> +{
> +    SCLPConsole *scon = DO_UPCAST(SCLPConsole, event, event);
> +
> +    if (!scon->chr) {
> +        return;
> +    }
> +    qemu_chr_fe_open(scon->chr);
> +}
> +
> +static void guest_close(SCLPEvent *event)
> +{
> +    SCLPConsole *scon = DO_UPCAST(SCLPConsole, event, event);
> +
> +    if (!scon->chr) {
> +        return;
> +    }
> +    qemu_chr_fe_close(scon->chr);
> +}
> +
> +static int chr_can_read(void *opaque)
> +{
> +    return 1;
> +}
> +
> +static void chr_read_vt220(void *opaque, const uint8_t *buf, int size)
> +{
> +    char *offset;
> +
> +    if (!sclp_console_data_vt220) {
> +        size_buffer = 2 * size;

Why 2*?

> +        sclp_console_data_vt220 = malloc(size_buffer);

%s/malloc/g_malloc0/g

> +    }
> +    if (size_buffer<  size + 1) {

This could use a comment.

> +        free(sclp_console_data_vt220);
> +        size_buffer = 2 * size;
> +        sclp_console_data_vt220 = malloc(size_buffer);
> +    }
> +    offset = sclp_console_data_vt220;
> +    if (offset) {
> +        memcpy(offset, buf, size);
> +        offset += size;
> +        *offset = '\0';

How do you know you're not out of bounds?

> +        qemu_irq_raise(sclp_read_vt220);
> +    } else {
> +        size_buffer = 0;
> +    }
> +}
> +
> +static void chr_event(void *opaque, int event)
> +{
> +    switch (event) {
> +    case CHR_EVENT_OPENED:
> +        if (!sclp_console_data_vt220) {
> +            sclp_console_data_vt220 = malloc(size_buffer);
> +        }
> +        break;
> +    case CHR_EVENT_CLOSED:
> +        break;
> +    }
> +}
> +
> +static unsigned int send_mask_vt220(void)
> +{
> +    return SCLP_EVENT_MASK_MSG_ASCII;
> +}
> +
> +static unsigned int receive_mask_vt220(void)
> +{
> +    return SCLP_EVENT_MASK_MSG_ASCII;
> +}
> +
> +static int sclpconsole_initfn_vt220(SCLPEvent *event)
> +{
> +    SCLPConsole *scon = DO_UPCAST(SCLPConsole, event, event);
> +
> +    event->id = ID_VT220;
> +    sclpef_set_console(event);
> +    if (scon->chr) {
> +        qemu_chr_add_handlers(scon->chr, chr_can_read,
> +                chr_read_vt220, chr_event, scon);
> +    }
> +
> +    return 0;
> +}
> +
> +static Property sclpconsole_properties[] = {
> +    DEFINE_PROP_CHR("chardev", SCLPConsole, chr),
> +    DEFINE_PROP_END_OF_LIST(),
> +};
> +
> +static void sclpconsole_class_init(ObjectClass *klass, void *data)
> +{
> +    DeviceClass *dc = DEVICE_CLASS(klass);
> +    SCLPEventClass *k = SCLP_EVENT_CLASS(klass);
> +
> +    k->init = sclpconsole_initfn_vt220;
> +    k->have_data = flush_buf;
> +    k->guest_open = guest_open;
> +    k->guest_close = guest_close;
> +    k->get_send_mask = send_mask_vt220;
> +    k->get_receive_mask = receive_mask_vt220;
> +    k->get_data = console_data_vt220;
> +    dc->props = sclpconsole_properties;
> +}
> +
> +static TypeInfo sclpconsole_info = {
> +    .name          = "sclpconsole",
> +    .parent        = TYPE_SCLP_EVENT,
> +    .instance_size = sizeof(SCLPConsole),
> +    .class_init    = sclpconsole_class_init,
> +};
> +
>   static void sclpef_register_types(void)
>   {
>       type_register_static(&sclp_event_facility_type_info);
>       type_register_static(&sclp_quiesce_info);
> +    type_register_static(&sclpconsole_info);
>   }
>   type_init(sclpef_register_types)
> diff --git a/hw/s390-event-facility.h b/hw/s390-event-facility.h
> index 40d4049..d6bde7d 100644
> --- a/hw/s390-event-facility.h
> +++ b/hw/s390-event-facility.h
> @@ -14,6 +14,7 @@
>   #include "qemu-common.h"
>
>   #define ID_QUIESCE SCLP_EVENT_SIGNAL_QUIESCE
> +#define ID_VT220   SCLP_EVENT_ASCII_CONSOLE_DATA
>
>   #define TYPE_SCLP_EVENT "s390-sclp-event-type"
>   #define SCLP_EVENT(obj) \
> @@ -34,6 +35,11 @@ typedef struct SCLPEventClass {
>       int (*exit)(SCLPEvent *event);
>       unsigned int (*get_send_mask)(void);
>       unsigned int (*get_receive_mask)(void);
> +    void (*guest_open)(SCLPEvent *event);
> +    void (*guest_close)(SCLPEvent *event);
> +    void (*guest_ready)(SCLPEvent *event);
> +    ssize_t (*have_data)(SCLPEvent *event, const uint8_t *buf, size_t len);
> +    char *(*get_data)(void);
>   } SCLPEventClass;
>
>   struct SCLPEvent {
> @@ -50,5 +56,7 @@ 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);
> +void sclpef_write_console_vt220(SCLPDevice *sdev, char *buf);
> +char *sclpef_get_console_data_vt220(SCLPDevice *sdev);
>
>   #endif
> diff --git a/hw/s390-sclp.c b/hw/s390-sclp.c
> index 683a709..8f45773 100644
> --- a/hw/s390-sclp.c
> +++ b/hw/s390-sclp.c
> @@ -11,6 +11,11 @@
>   #include "hw/s390-sclp.h"
>   #include "hw/s390-event-facility.h"
>
> +/* input buffer handling */
> +#define INP_BUFFER_SIZE 4096
> +static int sclp_curr_buf_size;
> +static char *sclp_input_vt220;
> +
>   /* Host capabilites */
>   static unsigned int sclp_send_mask;
>   static unsigned int sclp_receive_mask;
> @@ -21,6 +26,7 @@ static unsigned int sclp_cp_receive_mask;
>
>   static int quiesce;
>   static int event_pending;
> +static int vt220;

If anything, this is a machine variable, no? What is this supposed to 
express?

>
>   int sclp_read_info(CPUS390XState *env, struct sccb *sccb)
>   {
> @@ -66,6 +72,7 @@ void sclp_enable_signal_quiesce(void)
>   {
>       quiesce = 1;
>       event_pending = 1;
> +    vt220 = 0;
>   }
>
>   static void sclp_set_masks(void)
> @@ -81,7 +88,112 @@ static void sclp_set_masks(void)
>
>       sclp_send_mask = sclpef_send_mask(evt_fac->sdev);
>       sclp_receive_mask = sclpef_receive_mask(evt_fac->sdev);
> - }
> +}
> +
> +static int signal_vt220_event(struct sccb *sccb, int *slen)
> +{
> +    char *p;
> +
> +    if (!sclp_input_vt220 || !vt220) {
> +        return 0;
> +    }
> +
> +    int l = strlen(sclp_input_vt220);
> +
> +    if (*slen<  sizeof(struct ascii_cons_data_command) + l + 1) {
> +        event_pending = 1;
> +        return 0;
> +    }
> +    p = (char *)&sccb->c.read.acd_cmd.data;
> +    /* first byte is hex 0 saying an ascii string follows */
> +    *p++ = '\0';
> +    memmove(p, sclp_input_vt220, l);
> +    *sclp_input_vt220 = '\0';
> +
> +    sccb->c.read.acd_cmd.h.length =
> +            cpu_to_be16(sizeof(struct ascii_cons_data_command) + l + 1);
> +    sccb->c.read.acd_cmd.h.type = SCLP_EVENT_ASCII_CONSOLE_DATA;
> +
> +    vt220 = 0;
> +    *slen -= sizeof(struct ascii_cons_data_command) + l + 1;
> +    return 1;
> +}
> +
> +static char *grow_buffer(int size)
> +{
> +    char *p = (char *) malloc(size);
> +
> +    if (!p) {
> +        sclp_curr_buf_size = 0;
> +        return NULL;
> +    }
> +    memset(p, '\0', size);
> +    sclp_curr_buf_size = size;
> +    return p;
> +}
> +
> +static int sclp_write_vt220(struct event_buffer_header *event)
> +{
> +    int l;
> +    char *msg;
> +    SCLPS390EventFacility *evt_fac;
> +    struct ascii_cons_data_command *ad =
> +            (struct ascii_cons_data_command *) event;
> +
> +    assert(sclp_bus);
> +
> +    l = event->length - sizeof(struct event_buffer_header);
> +    msg = (char *) malloc(l + 1);
> +    assert(msg);
> +    memset(msg, '\0', l + 1);
> +    memmove(msg, ad->data, l);

Why the copy? Also, for such short lived data, you're probably better 
off using alloca.

> +
> +    evt_fac = sclp_bus->event_facility;
> +    sclpef_write_console_vt220(evt_fac->sdev, msg);
> +
> +    free(msg);
> +
> +    return SCLP_RC_NORMAL_COMPLETION;
> +}
> +
> +void sclp_enable_signal_read_vt220(void)
> +{
> +    int len;
> +    char *input;
> +    SCLPS390EventFacility *evt_fac;
> +
> +    if (!sclp_bus) {
> +        return;
> +    }
> +    evt_fac = sclp_bus->event_facility;
> +
> +    assert(evt_fac);
> +
> +    input = sclpef_get_console_data_vt220(evt_fac->sdev);
> +
> +    if (!input) {
> +        return;
> +    }
> +
> +    vt220 = 1;
> +    quiesce = 0;
> +    event_pending = 1;
> +    len = strlen((char *) input);
> +    if (sclp_input_vt220 == NULL) {
> +        /* get new buffer */
> +        sclp_input_vt220 = grow_buffer(2 * len + 1);
> +    } else {
> +        if (len>= sclp_curr_buf_size) {
> +            /* get larger buffer */
> +            char *p = grow_buffer(2 * len + 1);
> +            free(sclp_input_vt220);
> +            sclp_input_vt220 = p;
> +        }
> +    }
> +    if (sclp_input_vt220) {
> +        strcat(sclp_input_vt220, (char *)input);
> +    }
> +}
>
>   int sclp_read_event_data(CPUS390XState *env, struct sccb *sccb)
>   {
> @@ -99,7 +211,8 @@ int sclp_read_event_data(CPUS390XState *env, struct sccb *sccb)
>           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);
> +            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);
> @@ -117,7 +230,12 @@ int sclp_read_event_data(CPUS390XState *env, struct sccb *sccb)
>               sccb->h.response_code = cpu_to_be16(SCLP_RC_NORMAL_COMPLETION);
>           }
>       }
> -
> +    if (sclp_active_selection_mask&  SCLP_EVENT_MASK_MSG_ASCII) {
> +        if (signal_vt220_event(sccb,&slen)) {
> +            sccb->h.response_code = cpu_to_be16(SCLP_RC_NORMAL_COMPLETION);
> +            sccb->h.length = cpu_to_be16(SCCB_SIZE - slen);
> +        }
> +    }
>       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);
> @@ -127,6 +245,59 @@ out:
>       return 0;
>   }
>
> +int sclp_write_event_data(CPUS390XState *env, struct sccb *sccb)
> +{
> +    struct event_buffer_header *event;
> +    int slen;
> +    unsigned elen = 0;
> +
> +    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 */
> +    event =&sccb->c.event;
> +    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);
> +            goto out;
> +        }
> +        event = (void *) event + elen;
> +    }
> +    if (slen) {
> +        sccb->h.response_code = cpu_to_be16(SCLP_RC_INCONSISTENT_LENGTHS);
> +        goto out;
> +    }
> +
> +    /* the execute */
> +    event =&sccb->c.event;
> +    for (slen = be16_to_cpu(sccb->h.length) - sizeof(sccb->h);
> +         slen>  0; slen -= elen) {
> +        elen = be16_to_cpu(event->length);
> +        switch (event->type) {
> +        case SCLP_EVENT_ASCII_CONSOLE_DATA:
> +            sccb->h.response_code = cpu_to_be16(sclp_write_vt220(event));
> +            break;

This also screams for a generic dispatcher.

> +        default:
> +            sccb->h.response_code = SCLP_RC_INVALID_FUNCTION;
> +            break;
> +        }
> +        event = (void *) event + elen;
> +    }
> +    sccb->h.response_code = SCLP_RC_NORMAL_COMPLETION;
> +
> +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
> diff --git a/hw/s390-sclp.h b/hw/s390-sclp.h
> index f61421b..c86bca8 100644
> --- a/hw/s390-sclp.h
> +++ b/hw/s390-sclp.h
> @@ -7,6 +7,8 @@
>   /* 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_WRITE_EVENT_MASK               0x00780005
>
>   /* SCLP response codes */
> @@ -20,11 +22,12 @@
>   #define SCLP_RC_INVALID_MASK_LENGTH             0x74f0
>
>   /* 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                     0x40000000
> +#define SCLP_EVENT_MASK_MSG_ASCII               0x00000040
>
>   #define SCLP_UNCONDITIONAL_READ                 0x00
>   #define SCLP_SELECTIVE_READ                     0x01
> @@ -44,6 +47,13 @@ struct write_event_mask {
>       uint32_t receive_mask;
>   } __attribute__((packed));
>
> +struct mdb_header {
> +    uint16_t length;
> +    uint16_t type;
> +    uint32_t tag;
> +    uint32_t revision_code;
> +} __attribute__((packed));
> +
>   struct event_buffer_header {
>       uint16_t length;
>       uint8_t  type;
> @@ -58,9 +68,15 @@ struct signal_quiesce {
>       uint8_t unit;
>   } __attribute__((packed));
>
> +struct ascii_cons_data_command {
> +    struct event_buffer_header h;
> +    char data[0];
> +} __attribute__((packed));
> +
>   struct read_event_data {
>       union {
>           struct signal_quiesce quiesce;
> +        struct ascii_cons_data_command acd_cmd;
>           uint32_t mask;
>       };
>   } __attribute__((packed));
> @@ -84,15 +100,19 @@ struct sccb {
>       struct sccb_header h;
>       union {
>           struct read_info_sccb read_info;
> +        struct event_buffer_header event;
>           struct read_event_data read;
> +        struct ascii_cons_data_command acd_cmd;
>           struct write_event_mask we_mask;
>           char data[SCCB_DATA_LEN];
>       } c;
>    } __attribute__((packed));
>
>   void sclp_enable_signal_quiesce(void);
> +void sclp_enable_signal_read_vt220(void);
>   int sclp_read_info(CPUS390XState *env, struct sccb *sccb);
>   int sclp_read_event_data(CPUS390XState *env, struct sccb *sccb);
> +int sclp_write_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);
>
> diff --git a/sysemu.h b/sysemu.h
> index bc2c788..b4d399c 100644
> --- a/sysemu.h
> +++ b/sysemu.h
> @@ -62,6 +62,7 @@ int qemu_powerdown_requested(void);
>   void qemu_system_killed(int signal, pid_t pid);
>   void qemu_kill_report(void);
>   extern qemu_irq qemu_system_powerdown;
> +extern qemu_irq sclp_read_vt220;
>   void qemu_system_reset(bool report);
>
>   void qemu_add_exit_notifier(Notifier *notify);
> diff --git a/target-s390x/op_helper.c b/target-s390x/op_helper.c
> index 3e5eff4..4d49472 100644
> --- a/target-s390x/op_helper.c
> +++ b/target-s390x/op_helper.c
> @@ -2391,6 +2391,12 @@ 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_READ_EVENT_DATA:
> +            r = sclp_read_event_data(env,&work_sccb);
> +            break;
> +        case SCLP_CMD_WRITE_EVENT_DATA:
> +            r = sclp_write_event_data(env,&work_sccb);
> +            break;
>           case SCLP_CMD_WRITE_EVENT_MASK:
>               r = sclp_write_event_mask(env,&work_sccb);
>               break;
> diff --git a/vl.c b/vl.c
> index 23ab3a3..aba7ab0 100644
> --- a/vl.c
> +++ b/vl.c
> @@ -174,6 +174,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;
> @@ -201,6 +202,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;
> @@ -274,6 +276,8 @@ 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 int default_loader = 1;
>
>   static struct {
>       const char *driver;
> @@ -295,6 +299,8 @@ static struct {
>       { .driver = "isa-cirrus-vga",       .flag =&default_vga       },
>       { .driver = "vmware-svga",          .flag =&default_vga       },
>       { .driver = "qxl-vga",              .flag =&default_vga       },
> +    { .driver = "s390-sclp",            .flag =&default_sclpcon   },
> +    { .driver = "s390-ipl",             .flag =&default_loader    },
>   };
>
>   static void res_free(void)
> @@ -1942,6 +1948,7 @@ struct device_config {
>           DEV_VIRTCON,   /* -virtioconsole */
>           DEV_DEBUGCON,  /* -debugcon */
>           DEV_GDB,       /* -gdb, -s */
> +        DEV_SCLPCON,   /* sclp console */
>       } type;
>       const char *cmdline;
>       Location loc;
> @@ -2058,6 +2065,36 @@ static int virtcon_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;

Apart from that part not passing checkpatch, why do we have to 
reimplement this for every char device?


Alex

> +    if (index == MAX_SCLP_CONSOLES) {
> +        fprintf(stderr, "qemu: too many sclp consoles\n");
> +        exit(1);
> +    }
> +
> +    dev_opts = qemu_opts_create(device, NULL, 0);
> +    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 debugcon_parse(const char *devname)
>   {
>       QemuOpts *opts;
> @@ -3304,6 +3341,8 @@ int main(int argc, char **argv, char **envp)
>               add_device_config(DEV_SERIAL, "mon:stdio");
>           } else if (default_virtcon&&  default_monitor) {
>               add_device_config(DEV_VIRTCON, "mon:stdio");
> +        } else if (default_sclpcon&&  default_monitor) {
> +            add_device_config(DEV_SCLPCON, "mon:stdio");
>           } else {
>               if (default_serial)
>                   add_device_config(DEV_SERIAL, "stdio");
> @@ -3491,6 +3530,8 @@ int main(int argc, char **argv, char **envp)
>           exit(1);
>       if (foreach_device_config(DEV_VIRTCON, virtcon_parse)<  0)
>           exit(1);
> +    if (foreach_device_config(DEV_SCLPCON, sclpcon_parse)<  0)
> +        exit(1);
>       if (foreach_device_config(DEV_DEBUGCON, debugcon_parse)<  0)
>           exit(1);
>