From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:52597) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1SvqYL-00012P-4b for qemu-devel@nongnu.org; Mon, 30 Jul 2012 10:02:41 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1SvqYA-0008Lv-1A for qemu-devel@nongnu.org; Mon, 30 Jul 2012 10:02:33 -0400 Received: from cantor2.suse.de ([195.135.220.15]:47844 helo=mx2.suse.de) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1SvqY9-0008KG-Jl for qemu-devel@nongnu.org; Mon, 30 Jul 2012 10:02:21 -0400 Mime-Version: 1.0 (Apple Message framework v1278) Content-Type: text/plain; charset=us-ascii From: Alexander Graf In-Reply-To: <1343292959-18308-1-git-send-email-borntraeger@de.ibm.com> Date: Mon, 30 Jul 2012 16:02:16 +0200 Content-Transfer-Encoding: quoted-printable Message-Id: <5717F05F-62A3-47DE-8961-12031F1713B5@suse.de> References: <1343292959-18308-1-git-send-email-borntraeger@de.ibm.com> Subject: Re: [Qemu-devel] [PATCH v4 06/07] s390: sclp ascii console support List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Christian Borntraeger Cc: Blue Swirl , Heinz Graalfs , qemu-devel , Jens Freimann , afaerber@suse.de On 26.07.2012, at 10:55, Christian Borntraeger wrote: > From: Heinz Graalfs >=20 > 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. >=20 > The console can be specified manually with something like > -chardev stdio,id=3Dcharconsole0 -device = sclpconsole,chardev=3Dcharconsole0,id=3Dconsole0 >=20 > Newer kernels will autodetect that console and prefer that over virtio > console. >=20 > 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. >=20 > changes in v4: fold in suggestions by blueswirl >=20 > Signed-off-by: Heinz Graalfs > Signed-off-by: Christian Borntraeger > --- > hw/s390x/Makefile.objs | 2 +- > hw/s390x/sclpconsole.c | 323 = ++++++++++++++++++++++++++++++++++++++++++++++++ > 2 files changed, 324 insertions(+), 1 deletions(-) > create mode 100644 hw/s390x/sclpconsole.c >=20 > 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 =3D s390-virtio-bus.o s390-virtio.o > obj-y :=3D $(addprefix ../,$(obj-y)) > obj-y +=3D sclp.o > obj-y +=3D event-facility.o > -obj-y +=3D sclpquiesce.o > +obj-y +=3D 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 > + * > + * 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 > +#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 =3D opaque; > + > + qemu_mutex_lock(&scon->event.lock); Explicit locks now? > + can_read =3D 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 +=3D size; > + scon->iov_sclp_rest +=3D size; > + scon->iov_bs +=3D size; > + scon->event.event_pending =3D 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 =3D 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 =3D opaque; > + > + switch (event) { > + case CHR_EVENT_OPENED: > + if (!scon->iov) { > + scon->iov =3D g_malloc0(SIZE_BUFFER_VT220); > + scon->iov_sclp =3D scon->iov; > + scon->iov_bs =3D scon->iov; > + scon->iov_data_len =3D 0; > + scon->iov_sclp_rest =3D 0; > + } > + break; > + case CHR_EVENT_CLOSED: > + if (scon->iov) { > + g_free(scon->iov); > + scon->iov =3D 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 =3D DO_UPCAST(SCLPConsole, event, event); > + > + /* first byte is hex 0 saying an ascii string follows */ > + *buf++ =3D '\0'; > + avail--; > + /* if all data fit into provided SCLP buffer */ > + if (avail >=3D cons->iov_sclp_rest) { > + /* copy character byte-stream to SCLP buffer */ > + memcpy(buf, cons->iov_sclp, cons->iov_sclp_rest); > + *size =3D cons->iov_sclp_rest + 1; > + cons->iov_sclp =3D cons->iov; > + cons->iov_bs =3D cons->iov; > + cons->iov_data_len =3D 0; > + cons->iov_sclp_rest =3D 0; > + event->event_pending =3D 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 =3D avail + 1; > + cons->iov_sclp_rest -=3D avail; > + cons->iov_sclp +=3D 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