diff mbox

[RFC,v0,3/3] Add virtio-serial device support

Message ID 1476029391-26067-4-git-send-email-nikunj@linux.vnet.ibm.com
State Superseded
Headers show

Commit Message

Nikunj A Dadhania Oct. 9, 2016, 4:09 p.m. UTC
Add support for virtio serial device to be used as a console device.
Currently, SLOF only supports spapr-vty device. With this addition
virtio console can be used during boot.

Signed-off-by: Nikunj A Dadhania <nikunj@linux.vnet.ibm.com>
---
 board-qemu/slof/Makefile                |   3 +
 board-qemu/slof/OF.fs                   |  24 +++-
 board-qemu/slof/pci-device_1af4_1003.fs |  25 ++++
 board-qemu/slof/pci-device_1af4_1043.fs |  15 +++
 board-qemu/slof/virtio-serial.fs        |  95 +++++++++++++++
 lib/libvirtio/Makefile                  |   2 +-
 lib/libvirtio/virtio-serial.c           | 202 ++++++++++++++++++++++++++++++++
 lib/libvirtio/virtio-serial.h           |  28 +++++
 lib/libvirtio/virtio.code               |  33 ++++++
 lib/libvirtio/virtio.in                 |   6 +
 10 files changed, 426 insertions(+), 7 deletions(-)
 create mode 100644 board-qemu/slof/pci-device_1af4_1003.fs
 create mode 100644 board-qemu/slof/pci-device_1af4_1043.fs
 create mode 100644 board-qemu/slof/virtio-serial.fs
 create mode 100644 lib/libvirtio/virtio-serial.c
 create mode 100644 lib/libvirtio/virtio-serial.h

Comments

Alexey Kardashevskiy Oct. 10, 2016, 6:18 a.m. UTC | #1
On 10/10/16 03:09, Nikunj A Dadhania wrote:
> Add support for virtio serial device to be used as a console device.
> Currently, SLOF only supports spapr-vty device. With this addition
> virtio console can be used during boot.


Cool, good job, works fine!

> 
> Signed-off-by: Nikunj A Dadhania <nikunj@linux.vnet.ibm.com>
> ---
>  board-qemu/slof/Makefile                |   3 +
>  board-qemu/slof/OF.fs                   |  24 +++-
>  board-qemu/slof/pci-device_1af4_1003.fs |  25 ++++
>  board-qemu/slof/pci-device_1af4_1043.fs |  15 +++
>  board-qemu/slof/virtio-serial.fs        |  95 +++++++++++++++
>  lib/libvirtio/Makefile                  |   2 +-
>  lib/libvirtio/virtio-serial.c           | 202 ++++++++++++++++++++++++++++++++
>  lib/libvirtio/virtio-serial.h           |  28 +++++
>  lib/libvirtio/virtio.code               |  33 ++++++
>  lib/libvirtio/virtio.in                 |   6 +
>  10 files changed, 426 insertions(+), 7 deletions(-)
>  create mode 100644 board-qemu/slof/pci-device_1af4_1003.fs
>  create mode 100644 board-qemu/slof/pci-device_1af4_1043.fs
>  create mode 100644 board-qemu/slof/virtio-serial.fs
>  create mode 100644 lib/libvirtio/virtio-serial.c
>  create mode 100644 lib/libvirtio/virtio-serial.h
> 
> diff --git a/board-qemu/slof/Makefile b/board-qemu/slof/Makefile
> index 940a15a..cf57f16 100644
> --- a/board-qemu/slof/Makefile
> +++ b/board-qemu/slof/Makefile
> @@ -69,6 +69,8 @@ VIO_FFS_FILES = \
>  	$(SLOFBRDDIR)/pci-device_1af4_1041.fs \
>  	$(SLOFBRDDIR)/pci-device_1af4_1001.fs \
>  	$(SLOFBRDDIR)/pci-device_1af4_1042.fs \
> +	$(SLOFBRDDIR)/pci-device_1af4_1003.fs \
> +	$(SLOFBRDDIR)/pci-device_1af4_1043.fs \
>  	$(SLOFBRDDIR)/pci-device_1af4_1004.fs \
>  	$(SLOFBRDDIR)/pci-device_1af4_1048.fs \
>  	$(SLOFBRDDIR)/pci-device_1af4_1009.fs \
> @@ -79,6 +81,7 @@ VIO_FFS_FILES = \
>  	$(SLOFBRDDIR)/vio-veth.fs \
>  	$(SLOFBRDDIR)/rtas-nvram.fs \
>  	$(SLOFBRDDIR)/virtio-net.fs \
> +	$(SLOFBRDDIR)/virtio-serial.fs \
>  	$(SLOFBRDDIR)/virtio-block.fs \
>  	$(SLOFBRDDIR)/virtio-fs.fs \
>  	$(SLOFBRDDIR)/dev-null.fs \
> diff --git a/board-qemu/slof/OF.fs b/board-qemu/slof/OF.fs
> index c8df9ca..6b13e03 100644
> --- a/board-qemu/slof/OF.fs
> +++ b/board-qemu/slof/OF.fs
> @@ -162,6 +162,10 @@ CREATE version-str 10 ALLOT
>  : dump-display-write
>      s" screen" find-alias  IF
>          drop terminal-write drop
> +    ELSE
> +        s" vsterm" find-alias  IF
> +            drop type
> +        THEN


Why new alias, not usual "screen"? "screen" and "vsterm" cannot work
simultaneously any way.


>      THEN
>  ;
>  
> @@ -236,12 +240,20 @@ romfs-base 400000 0 ' claim CATCH IF ." claim failed!" cr 2drop THEN drop
>                  ." using hvterm" cr
>                  " hvterm" io
>              ELSE
> -		    		    " /openprom" find-node ?dup IF
> -		    		        set-node
> -		    		        ." and no default found, creating dev-null" cr
> -		    		        " dev-null.fs" included
> -		    		        " devnull-console" io
> -		    		        0 set-node

Here that indentation is fixed :)


> +                " vsterm" find-alias IF
> +                    drop
> +                    ." using vsterm" cr
> +                    " vsterm" io
> +                    false to store-prevga?
> +                    dump-display-buffer
> +                ELSE
> +                    " /openprom" find-node ?dup IF
> +                        set-node
> +                        ." and no default found, creating dev-null" cr
> +                        " dev-null.fs" included
> +                        " devnull-console" io
> +                        0 set-node
> +                    THEN
>                  THEN
>              THEN
>          THEN
> diff --git a/board-qemu/slof/pci-device_1af4_1003.fs b/board-qemu/slof/pci-device_1af4_1003.fs
> new file mode 100644
> index 0000000..a7cd53b
> --- /dev/null
> +++ b/board-qemu/slof/pci-device_1af4_1003.fs
> @@ -0,0 +1,25 @@
> +\ *****************************************************************************
> +\ * Copyright (c) 2016 IBM Corporation
> +\ * All rights reserved.
> +\ * This program and the accompanying materials
> +\ * are made available under the terms of the BSD License
> +\ * which accompanies this distribution, and is available at
> +\ * http://www.opensource.org/licenses/bsd-license.php
> +\ *
> +\ * Contributors:
> +\ *     IBM Corporation - initial implementation
> +\ ****************************************************************************/
> +
> +\ Handle virtio-serial device
> +
> +s" virtio [ serial ]" type cr
> +
> +my-space pci-device-generic-setup
> +
> +pci-master-enable
> +pci-mem-enable
> +pci-io-enable
> +
> +s" virtio-serial.fs" included
> +
> +pci-device-disable
> diff --git a/board-qemu/slof/pci-device_1af4_1043.fs b/board-qemu/slof/pci-device_1af4_1043.fs
> new file mode 100644
> index 0000000..f044450
> --- /dev/null
> +++ b/board-qemu/slof/pci-device_1af4_1043.fs
> @@ -0,0 +1,15 @@
> +\ *****************************************************************************
> +\ * Copyright (c) 2016 IBM Corporation
> +\ * All rights reserved.
> +\ * This program and the accompanying materials
> +\ * are made available under the terms of the BSD License
> +\ * which accompanies this distribution, and is available at
> +\ * http://www.opensource.org/licenses/bsd-license.php
> +\ *
> +\ * Contributors:
> +\ *     IBM Corporation - initial implementation
> +\ ****************************************************************************/
> +
> +\ Device ID 1044 is for virtio-serial non-transitional device.
> +\ Include the driver for virtio-serial
> +s" pci-device_1af4_1003.fs" included
> diff --git a/board-qemu/slof/virtio-serial.fs b/board-qemu/slof/virtio-serial.fs
> new file mode 100644
> index 0000000..0f25a80
> --- /dev/null
> +++ b/board-qemu/slof/virtio-serial.fs
> @@ -0,0 +1,95 @@
> +\ *****************************************************************************
> +\ * Copyright (c) 2016 IBM Corporation
> +\ * All rights reserved.
> +\ * This program and the accompanying materials
> +\ * are made available under the terms of the BSD License
> +\ * which accompanies this distribution, and is available at
> +\ * http://www.opensource.org/licenses/bsd-license.php
> +\ *
> +\ * Contributors:
> +\ *     IBM Corporation - initial implementation
> +\ ****************************************************************************/
> +
> +s" serial" device-type
> +
> +FALSE VALUE initialized?
> +
> +virtio-setup-vd VALUE virtiodev
> +
> +\ Quiesce the virtqueue of this device so that no more background

s/Quiesce/Quiescence/

> +\ transactions can be pending.
> +: shutdown  ( -- )
> +    initialized? IF
> +        my-phandle node>path open-dev ?dup IF
> +            virtiodev virtio-serial-shutdown
> +            close-dev
> +        THEN
> +        FALSE to initialized?
> +    THEN
> +;
> +
> +: virtio-serial-term-emit
> +    virtiodev SWAP virtio-serial-putchar
> +;
> +
> +: virtio-serial-term-key?  virtiodev virtio-serial-haschar ;
> +: virtio-serial-term-key   BEGIN virtio-serial-term-key? UNTIL virtiodev virtio-serial-getchar ;
> +
> +\ Basic device initialization - which has only to be done once
> +: init  ( -- )
> +virtiodev virtio-serial-init drop
> +    TRUE to initialized?
> +    ['] virtio-serial-term-emit to emit
> +    ['] virtio-serial-term-key  to key
> +    ['] virtio-serial-term-key? to key?
> +    ['] shutdown add-quiesce-xt
> +;
> +
> +0 VALUE open-count
> +
> +\ Standard node "open" function
> +: open  ( -- okay? )
> +    open-count 0= IF
> +        open IF initialized? 0= IF init THEN
> +            true
> +        ELSE false exit
> +        THEN
> +    ELSE true THEN
> +    open-count 1 + to open-count
> +;
> +
> +: close
> +    open-count 0> IF
> +        open-count 1 - dup to open-count
> +        0= IF close THEN
> +    THEN
> +    close
> +;
> +
> +: write ( adr len -- actual )

s/adr/addr/ ?

> +    tuck
> +    0 ?DO
> +        dup c@ virtiodev SWAP virtio-serial-putchar
> +        1 +
> +    LOOP
> +    drop
> +;
> +
> +: read ( adr len -- actual )
> +    0= IF drop 0 EXIT THEN
> +    virtiodev virtio-serial-haschar 0= IF 0 swap c! -2 EXIT THEN
> +    virtiodev virtio-serial-getchar swap c! 1
> +;
> +
> +: setup-alias
> +    " vsterm" find-alias 0= IF
> +        " vsterm" get-node node>path set-alias
> +    ELSE drop THEN
> +;
> +setup-alias
> +
> +\ Override serial methods to make term-io.fs happy
> +: serial-emit virtio-serial-term-emit ;
> +: serial-key? virtio-serial-term-key? ;
> +: serial-key  virtio-serial-term-key  ;
> +

"git am" says:

/home/aik/p/slof/.git/rebase-apply/patch:234: new blank line at EOF.


> diff --git a/lib/libvirtio/Makefile b/lib/libvirtio/Makefile
> index bd6a1fa..87d9513 100644
> --- a/lib/libvirtio/Makefile
> +++ b/lib/libvirtio/Makefile
> @@ -24,7 +24,7 @@ TARGET = ../libvirtio.a
>  
>  all: $(TARGET)
>  
> -SRCS =  virtio.c virtio-blk.c p9.c virtio-9p.c virtio-scsi.c virtio-net.c
> +SRCS =  virtio.c virtio-blk.c p9.c virtio-9p.c virtio-scsi.c virtio-net.c virtio-serial.c
>  
>  OBJS = $(SRCS:%.c=%.o)
>  
> diff --git a/lib/libvirtio/virtio-serial.c b/lib/libvirtio/virtio-serial.c
> new file mode 100644
> index 0000000..ea56bef
> --- /dev/null
> +++ b/lib/libvirtio/virtio-serial.c
> @@ -0,0 +1,202 @@
> +/******************************************************************************
> + * Copyright (c) 2016 IBM Corporation
> + * All rights reserved.
> + * This program and the accompanying materials
> + * are made available under the terms of the BSD License
> + * which accompanies this distribution, and is available at
> + * http://www.opensource.org/licenses/bsd-license.php
> + *
> + * Contributors:
> + *     IBM Corporation - initial implementation
> + *****************************************************************************/
> +
> +/*
> + * Virtio serial device definitions.
> + * See Virtio 1.0 - 5.3 Console Device, for details
> + */
> +#include <stdio.h>
> +#include <string.h>
> +#include <cpu.h>
> +#include <helpers.h>
> +#include <byteorder.h>
> +#include "virtio.h"
> +#include "virtio-serial.h"
> +#include "virtio-internal.h"
> +
> +#define DRIVER_FEATURE_SUPPORT  (VIRTIO_F_VERSION_1)

No need in braces.

> +#define RX_ELEM_SIZE            4
> +#define RX_NUM_ELEMS            128
> +
> +#define RX_Q 0
> +#define TX_Q 1
> +
> +static struct vqs vq_rx;
> +static struct vqs vq_tx;
> +static uint16_t last_rx_idx;	/* Last index in RX "used" ring */
> +
> +int virtio_serial_init(struct virtio_device *dev)
> +{
> +	struct vring_avail *vq_avail;
> +	int status = VIRTIO_STAT_ACKNOWLEDGE;
> +
> +	/* Reset device */
> +	virtio_reset_device(dev);
> +
> +	/* Acknowledge device. */
> +	virtio_set_status(dev, status);
> +
> +	/* Tell HV that we know how to drive the device. */
> +	status |= VIRTIO_STAT_DRIVER;
> +	virtio_set_status(dev, status);
> +
> +	if (dev->is_modern) {
> +		/* Negotiate features and sets FEATURES_OK if successful */
> +		if (virtio_negotiate_guest_features(dev, DRIVER_FEATURE_SUPPORT))
> +			goto dev_error;
> +
> +		virtio_get_status(dev, &status);
> +	}
> +
> +	if (virtio_queue_init_vq(dev, &vq_rx, RX_Q))
> +		goto dev_error;
> +
> +	/* Allocate memory for multiple receive buffers */
> +	vq_rx.buf_mem = SLOF_alloc_mem(RX_ELEM_SIZE * RX_NUM_ELEMS);
> +	if (!vq_rx.buf_mem) {
> +		printf("virtio-serial: Failed to allocate buffers!\n");
> +		goto dev_error;
> +	}
> +
> +	/* Prepare receive buffer queue */
> +	for (int i = 0; i < RX_NUM_ELEMS; i++) {
> +		uint64_t addr = (uint64_t)vq_rx.buf_mem + i * RX_ELEM_SIZE;
> +
> +		/* Descriptor for data: */
> +		virtio_fill_desc(&vq_rx.desc[i], dev->is_modern, addr, 1, VRING_DESC_F_WRITE, 0);
> +		vq_rx.avail->ring[i] = virtio_cpu_to_modern16(dev, i);
> +	}
> +	vq_rx.avail->flags = virtio_cpu_to_modern16(dev, VRING_AVAIL_F_NO_INTERRUPT);
> +	vq_rx.avail->idx = virtio_cpu_to_modern16(dev, RX_NUM_ELEMS);
> +	sync();
> +
> +	last_rx_idx = virtio_modern16_to_cpu(dev, vq_rx.used->idx);
> +
> +	if (virtio_queue_init_vq(dev, &vq_tx, TX_Q))
> +		goto dev_error;
> +
> +	vq_avail = virtio_get_vring_avail(dev, TX_Q);
> +	vq_avail->flags = virtio_cpu_to_modern16(dev, VRING_AVAIL_F_NO_INTERRUPT);
> +	vq_avail->idx = 0;
> +
> +	/* Tell HV that setup succeeded */
> +	status |= VIRTIO_STAT_DRIVER_OK;
> +	virtio_set_status(dev, status);
> +
> +	return 1;
> + dev_error:
> +	printf("%s: failed\n", __func__);
> +	status |= VIRTIO_STAT_FAILED;
> +	virtio_set_status(dev, status);
> +	return 0;
> +}
> +
> +void virtio_serial_shutdown(struct virtio_device *dev)
> +{
> +	/* Quiesce device */
> +	virtio_set_status(dev, VIRTIO_STAT_FAILED);
> +
> +	/* Reset device */
> +	virtio_reset_device(dev);
> +}
> +
> +int virtio_serial_putchar(struct virtio_device *dev, char c)
> +{
> +	struct vring_desc *desc;
> +	int id;
> +	uint32_t vq_size, time;
> +	struct vring_desc *vq_desc;		/* Descriptor vring */
> +	struct vring_avail *vq_avail;		/* "Available" vring */
> +	struct vring_used *vq_used;		/* "Used" vring */


The comments do not seem to tell lot more than variable names themselves.


> +	volatile uint16_t *current_used_idx;
> +	uint16_t last_used_idx, avail_idx;
> +
> +	vq_size = virtio_get_qsize(dev, TX_Q);
> +	vq_desc = virtio_get_vring_desc(dev, TX_Q);
> +	vq_avail = virtio_get_vring_avail(dev, TX_Q);
> +	vq_used = virtio_get_vring_used(dev, TX_Q);
> +
> +	avail_idx = virtio_modern16_to_cpu(dev, vq_avail->idx);
> +
> +	last_used_idx = vq_used->idx;
> +	current_used_idx = &vq_used->idx;
> +
> +	/* Determine descriptor index */
> +	id = (avail_idx * 1) % vq_size;

s/(avail_idx * 1)/avail_idx/


> +
> +	/* Set up virtqueue descriptor for header */
> +	desc = &vq_desc[id];
> +	virtio_fill_desc(desc, dev->is_modern, (uint64_t)&c, 1, 0, 0);
> +
> +	vq_avail->ring[avail_idx % vq_size] = virtio_cpu_to_modern16 (dev, id);
> +	mb();
> +	vq_avail->idx = virtio_cpu_to_modern16(dev, avail_idx + 1);
> +
> +	/* Tell HV that the queue is ready */
> +	virtio_queue_notify(dev, TX_Q);
> +
> +	/* Wait for host to consume the descriptor */
> +	time = SLOF_GetTimer() + VIRTIO_TIMEOUT;
> +	while (*current_used_idx == last_used_idx) {
> +		// do something better

We should probably stop copying this line from driver to driver :)

> +		mb();
> +		if (time < SLOF_GetTimer()) {
> +			printf("virtio_serial_putchar failed! \n");
> +			return 0;
> +		}
> +	}
> +
> +	return 1;
> +}
> +
> +static uint16_t last_rx_idx;	/* Last index in RX "used" ring */
> +
> +char virtio_serial_getchar(struct virtio_device *dev)
> +{
> +	int id, idx;
> +	char buf[RX_NUM_ELEMS] = {0};
> +	uint16_t avail_idx;
> +
> +	idx = virtio_modern16_to_cpu(dev, vq_rx.used->idx);
> +	if (last_rx_idx == idx) {
> +		/* Nothing received yet */
> +		return 0;
> +	}
> +
> +	id = (virtio_modern32_to_cpu(dev, vq_rx.used->ring[last_rx_idx % vq_rx.size].id) + 1)
> +		% vq_rx.size;
> +
> +	/* Copy data to destination buffer */
> +	memcpy(buf, (void *)virtio_modern64_to_cpu(dev, vq_rx.desc[id - 1].addr), RX_ELEM_SIZE);
> +
> +	/* Move indices to next entries */
> +	last_rx_idx = last_rx_idx + 1;
> +
> +	avail_idx = virtio_modern16_to_cpu(dev, vq_rx.avail->idx);
> +	vq_rx.avail->ring[avail_idx % vq_rx.size] = virtio_cpu_to_modern16(dev, id - 1);
> +	sync();
> +	vq_rx.avail->idx = virtio_cpu_to_modern16(dev, avail_idx + 1);
> +	sync();
> +
> +	/* Tell HV that RX queue entry is ready */
> +	virtio_queue_notify(dev, RX_Q);
> +
> +	return buf[0];
> +}
> +
> +int virtio_serial_haschar(struct virtio_device *dev)
> +{
> +	if (last_rx_idx == virtio_modern16_to_cpu(dev, vq_rx.used->idx))
> +		return 0;
> +	else
> +		return 1;
> +}
> diff --git a/lib/libvirtio/virtio-serial.h b/lib/libvirtio/virtio-serial.h
> new file mode 100644
> index 0000000..8abfd16
> --- /dev/null
> +++ b/lib/libvirtio/virtio-serial.h
> @@ -0,0 +1,28 @@
> +/******************************************************************************
> + * Copyright (c) 2016 IBM Corporation
> + * All rights reserved.
> + * This program and the accompanying materials
> + * are made available under the terms of the BSD License
> + * which accompanies this distribution, and is available at
> + * http://www.opensource.org/licenses/bsd-license.php
> + *
> + * Contributors:
> + *     IBM Corporation - initial implementation
> + *****************************************************************************/
> +
> +/*
> + * Virtio serial device definitions.
> + * See Virtio 1.0 - 5.3 Console Device, for details
> + */
> +
> +#ifndef _VIRTIO_SERIAL_H
> +#define _VIRTIO_SERIAL_H
> +
> +extern int virtio_serial_init(struct virtio_device *dev);
> +extern void virtio_serial_shutdown(struct virtio_device *dev);
> +extern int virtio_serial_putchar(struct virtio_device *dev, char c);
> +extern char virtio_serial_getchar(struct virtio_device *dev);
> +extern int virtio_serial_haschar(struct virtio_device *dev);
> +
> +#endif  /* _VIRTIO_SERIAL_H */
> +

And here
/home/aik/p/slof/.git/rebase-apply/patch:489: new blank line at EOF.


> diff --git a/lib/libvirtio/virtio.code b/lib/libvirtio/virtio.code
> index 8eec8f0..5cfffcc 100644
> --- a/lib/libvirtio/virtio.code
> +++ b/lib/libvirtio/virtio.code
> @@ -15,6 +15,7 @@
>  #include <virtio-9p.h>
>  #include <virtio-scsi.h>
>  #include <virtio-net.h>
> +#include <virtio-serial.h>
>  
>  /******** core virtio ********/
>  
> @@ -165,3 +166,35 @@ PRIM(virtio_X2d_net_X2d_write)
>  	TOS.n = virtionet_write(TOS.a, len);
>  }
>  MIRP
> +
> +/*********** virtio-serial ***********/
> +// : virtio-serial-init ( dev -- false | true)
> +PRIM(virtio_X2d_serial_X2d_init)
> +	void *dev = TOS.a;
> +	TOS.u = virtio_serial_init(dev);
> +MIRP
> +
> +// : virtio-serial-shutdown ( dev -- )
> +PRIM(virtio_X2d_serial_X2d_shutdown)
> +	void *dev = TOS.a; POP;
> +	virtio_serial_shutdown(dev);
> +MIRP
> +
> +// : virtio-serial-putchar ( dev char -- )
> +PRIM(virtio_X2d_serial_X2d_putchar)
> +	unsigned long c = TOS.n; POP;
> +	void *dev = TOS.a; POP;
> +	virtio_serial_putchar(dev, c);
> +MIRP
> +
> +// : virtio-serial-getchar ( dev -- char)
> +PRIM(virtio_X2d_serial_X2d_getchar)
> +	void *dev = TOS.a;
> +	TOS.n = virtio_serial_getchar(dev);
> +MIRP
> +
> +// : virtio-serial-haschar ( dev -- true | false)
> +PRIM(virtio_X2d_serial_X2d_haschar)
> +	void *dev = TOS.a;
> +	TOS.n = virtio_serial_haschar(dev);
> +MIRP
> diff --git a/lib/libvirtio/virtio.in b/lib/libvirtio/virtio.in
> index 195840e..d2b1641 100644
> --- a/lib/libvirtio/virtio.in
> +++ b/lib/libvirtio/virtio.in
> @@ -33,3 +33,9 @@ cod(virtio-net-open)
>  cod(virtio-net-close)
>  cod(virtio-net-read)
>  cod(virtio-net-write)
> +
> +cod(virtio-serial-init)
> +cod(virtio-serial-shutdown)
> +cod(virtio-serial-putchar)
> +cod(virtio-serial-getchar)
> +cod(virtio-serial-haschar)
>
Nikunj A Dadhania Oct. 10, 2016, 8:27 a.m. UTC | #2
Alexey Kardashevskiy <aik@ozlabs.ru> writes:

> On 10/10/16 03:09, Nikunj A Dadhania wrote:
>> Add support for virtio serial device to be used as a console device.
>> Currently, SLOF only supports spapr-vty device. With this addition
>> virtio console can be used during boot.
>
>
> Cool, good job, works fine!

Thanks for testing. Did you see kernel logs as well, as I could get
login prompt, but not messages from kernel after quiesce

>> 
>> Signed-off-by: Nikunj A Dadhania <nikunj@linux.vnet.ibm.com>
>> ---
>>  board-qemu/slof/Makefile                |   3 +
>>  board-qemu/slof/OF.fs                   |  24 +++-
>>  board-qemu/slof/pci-device_1af4_1003.fs |  25 ++++
>>  board-qemu/slof/pci-device_1af4_1043.fs |  15 +++
>>  board-qemu/slof/virtio-serial.fs        |  95 +++++++++++++++
>>  lib/libvirtio/Makefile                  |   2 +-
>>  lib/libvirtio/virtio-serial.c           | 202 ++++++++++++++++++++++++++++++++
>>  lib/libvirtio/virtio-serial.h           |  28 +++++
>>  lib/libvirtio/virtio.code               |  33 ++++++
>>  lib/libvirtio/virtio.in                 |   6 +
>>  10 files changed, 426 insertions(+), 7 deletions(-)
>>  create mode 100644 board-qemu/slof/pci-device_1af4_1003.fs
>>  create mode 100644 board-qemu/slof/pci-device_1af4_1043.fs
>>  create mode 100644 board-qemu/slof/virtio-serial.fs
>>  create mode 100644 lib/libvirtio/virtio-serial.c
>>  create mode 100644 lib/libvirtio/virtio-serial.h
>> 
>> diff --git a/board-qemu/slof/Makefile b/board-qemu/slof/Makefile
>> index 940a15a..cf57f16 100644
>> --- a/board-qemu/slof/Makefile
>> +++ b/board-qemu/slof/Makefile
>> @@ -69,6 +69,8 @@ VIO_FFS_FILES = \
>>  	$(SLOFBRDDIR)/pci-device_1af4_1041.fs \
>>  	$(SLOFBRDDIR)/pci-device_1af4_1001.fs \
>>  	$(SLOFBRDDIR)/pci-device_1af4_1042.fs \
>> +	$(SLOFBRDDIR)/pci-device_1af4_1003.fs \
>> +	$(SLOFBRDDIR)/pci-device_1af4_1043.fs \
>>  	$(SLOFBRDDIR)/pci-device_1af4_1004.fs \
>>  	$(SLOFBRDDIR)/pci-device_1af4_1048.fs \
>>  	$(SLOFBRDDIR)/pci-device_1af4_1009.fs \
>> @@ -79,6 +81,7 @@ VIO_FFS_FILES = \
>>  	$(SLOFBRDDIR)/vio-veth.fs \
>>  	$(SLOFBRDDIR)/rtas-nvram.fs \
>>  	$(SLOFBRDDIR)/virtio-net.fs \
>> +	$(SLOFBRDDIR)/virtio-serial.fs \
>>  	$(SLOFBRDDIR)/virtio-block.fs \
>>  	$(SLOFBRDDIR)/virtio-fs.fs \
>>  	$(SLOFBRDDIR)/dev-null.fs \
>> diff --git a/board-qemu/slof/OF.fs b/board-qemu/slof/OF.fs
>> index c8df9ca..6b13e03 100644
>> --- a/board-qemu/slof/OF.fs
>> +++ b/board-qemu/slof/OF.fs
>> @@ -162,6 +162,10 @@ CREATE version-str 10 ALLOT
>>  : dump-display-write
>>      s" screen" find-alias  IF
>>          drop terminal-write drop
>> +    ELSE
>> +        s" vsterm" find-alias  IF
>> +            drop type
>> +        THEN
>
>
> Why new alias, not usual "screen"? "screen" and "vsterm" cannot work
> simultaneously any way.

"screen" is for vga display, so did not want to override, and is used in
different context.

>>      THEN
>>  ;
>>  
>> @@ -236,12 +240,20 @@ romfs-base 400000 0 ' claim CATCH IF ." claim failed!" cr 2drop THEN drop
>>                  ." using hvterm" cr
>>                  " hvterm" io
>>              ELSE
>> -		    		    " /openprom" find-node ?dup IF
>> -		    		        set-node
>> -		    		        ." and no default found, creating dev-null" cr
>> -		    		        " dev-null.fs" included
>> -		    		        " devnull-console" io
>> -		    		        0 set-node
>
> Here that indentation is fixed :)

The above hunk is the removal. I will check 1/3 and fix there.

>
>> +                " vsterm" find-alias IF
>> +                    drop
>> +                    ." using vsterm" cr
>> +                    " vsterm" io
>> +                    false to store-prevga?
>> +                    dump-display-buffer
>> +                ELSE
>> +                    " /openprom" find-node ?dup IF
>> +                        set-node
>> +                        ." and no default found, creating dev-null" cr
>> +                        " dev-null.fs" included
>> +                        " devnull-console" io
>> +                        0 set-node

I think you meant here. But then i had to move this one more level deep.

>> +                    THEN
>>                  THEN
>>              THEN
>>          THEN
>> diff --git a/board-qemu/slof/pci-device_1af4_1003.fs b/board-qemu/slof/pci-device_1af4_1003.fs
>> new file mode 100644
>> index 0000000..a7cd53b
>> --- /dev/null
>> +++ b/board-qemu/slof/pci-device_1af4_1003.fs
>> @@ -0,0 +1,25 @@
>> +\ *****************************************************************************
>> +\ * Copyright (c) 2016 IBM Corporation
>> +\ * All rights reserved.
>> +\ * This program and the accompanying materials
>> +\ * are made available under the terms of the BSD License
>> +\ * which accompanies this distribution, and is available at
>> +\ * http://www.opensource.org/licenses/bsd-license.php
>> +\ *
>> +\ * Contributors:
>> +\ *     IBM Corporation - initial implementation
>> +\ ****************************************************************************/
>> +
>> +\ Handle virtio-serial device
>> +
>> +s" virtio [ serial ]" type cr
>> +
>> +my-space pci-device-generic-setup
>> +
>> +pci-master-enable
>> +pci-mem-enable
>> +pci-io-enable
>> +
>> +s" virtio-serial.fs" included
>> +
>> +pci-device-disable
>> diff --git a/board-qemu/slof/pci-device_1af4_1043.fs b/board-qemu/slof/pci-device_1af4_1043.fs
>> new file mode 100644
>> index 0000000..f044450
>> --- /dev/null
>> +++ b/board-qemu/slof/pci-device_1af4_1043.fs
>> @@ -0,0 +1,15 @@
>> +\ *****************************************************************************
>> +\ * Copyright (c) 2016 IBM Corporation
>> +\ * All rights reserved.
>> +\ * This program and the accompanying materials
>> +\ * are made available under the terms of the BSD License
>> +\ * which accompanies this distribution, and is available at
>> +\ * http://www.opensource.org/licenses/bsd-license.php
>> +\ *
>> +\ * Contributors:
>> +\ *     IBM Corporation - initial implementation
>> +\ ****************************************************************************/
>> +
>> +\ Device ID 1044 is for virtio-serial non-transitional device.
>> +\ Include the driver for virtio-serial
>> +s" pci-device_1af4_1003.fs" included
>> diff --git a/board-qemu/slof/virtio-serial.fs b/board-qemu/slof/virtio-serial.fs
>> new file mode 100644
>> index 0000000..0f25a80
>> --- /dev/null
>> +++ b/board-qemu/slof/virtio-serial.fs
>> @@ -0,0 +1,95 @@
>> +\ *****************************************************************************
>> +\ * Copyright (c) 2016 IBM Corporation
>> +\ * All rights reserved.
>> +\ * This program and the accompanying materials
>> +\ * are made available under the terms of the BSD License
>> +\ * which accompanies this distribution, and is available at
>> +\ * http://www.opensource.org/licenses/bsd-license.php
>> +\ *
>> +\ * Contributors:
>> +\ *     IBM Corporation - initial implementation
>> +\ ****************************************************************************/
>> +
>> +s" serial" device-type
>> +
>> +FALSE VALUE initialized?
>> +
>> +virtio-setup-vd VALUE virtiodev
>> +
>> +\ Quiesce the virtqueue of this device so that no more background
>
> s/Quiesce/Quiescence/

Sure

>
>> +\ transactions can be pending.
>> +: shutdown  ( -- )
>> +    initialized? IF
>> +        my-phandle node>path open-dev ?dup IF
>> +            virtiodev virtio-serial-shutdown
>> +            close-dev
>> +        THEN
>> +        FALSE to initialized?
>> +    THEN
>> +;
>> +
>> +: virtio-serial-term-emit
>> +    virtiodev SWAP virtio-serial-putchar
>> +;
>> +
>> +: virtio-serial-term-key?  virtiodev virtio-serial-haschar ;
>> +: virtio-serial-term-key   BEGIN virtio-serial-term-key? UNTIL virtiodev virtio-serial-getchar ;
>> +
>> +\ Basic device initialization - which has only to be done once
>> +: init  ( -- )
>> +virtiodev virtio-serial-init drop
>> +    TRUE to initialized?
>> +    ['] virtio-serial-term-emit to emit
>> +    ['] virtio-serial-term-key  to key
>> +    ['] virtio-serial-term-key? to key?
>> +    ['] shutdown add-quiesce-xt
>> +;
>> +
>> +0 VALUE open-count
>> +
>> +\ Standard node "open" function
>> +: open  ( -- okay? )
>> +    open-count 0= IF
>> +        open IF initialized? 0= IF init THEN
>> +            true
>> +        ELSE false exit
>> +        THEN
>> +    ELSE true THEN
>> +    open-count 1 + to open-count
>> +;
>> +
>> +: close
>> +    open-count 0> IF
>> +        open-count 1 - dup to open-count
>> +        0= IF close THEN
>> +    THEN
>> +    close
>> +;
>> +
>> +: write ( adr len -- actual )
>
> s/adr/addr/ ?

Sure, and in read as well.

>
>> +    tuck
>> +    0 ?DO
>> +        dup c@ virtiodev SWAP virtio-serial-putchar
>> +        1 +
>> +    LOOP
>> +    drop
>> +;
>> +
>> +: read ( adr len -- actual )
>> +    0= IF drop 0 EXIT THEN
>> +    virtiodev virtio-serial-haschar 0= IF 0 swap c! -2 EXIT THEN
>> +    virtiodev virtio-serial-getchar swap c! 1
>> +;
>> +
>> +: setup-alias
>> +    " vsterm" find-alias 0= IF
>> +        " vsterm" get-node node>path set-alias
>> +    ELSE drop THEN
>> +;
>> +setup-alias
>> +
>> +\ Override serial methods to make term-io.fs happy
>> +: serial-emit virtio-serial-term-emit ;
>> +: serial-key? virtio-serial-term-key? ;
>> +: serial-key  virtio-serial-term-key  ;
>> +
>
> "git am" says:
>
> /home/aik/p/slof/.git/rebase-apply/patch:234: new blank line at EOF.

Will remove that in next revision

>
>
>> diff --git a/lib/libvirtio/Makefile b/lib/libvirtio/Makefile
>> index bd6a1fa..87d9513 100644
>> --- a/lib/libvirtio/Makefile
>> +++ b/lib/libvirtio/Makefile
>> @@ -24,7 +24,7 @@ TARGET = ../libvirtio.a
>>  
>>  all: $(TARGET)
>>  
>> -SRCS =  virtio.c virtio-blk.c p9.c virtio-9p.c virtio-scsi.c virtio-net.c
>> +SRCS =  virtio.c virtio-blk.c p9.c virtio-9p.c virtio-scsi.c virtio-net.c virtio-serial.c
>>  
>>  OBJS = $(SRCS:%.c=%.o)
>>  
>> diff --git a/lib/libvirtio/virtio-serial.c b/lib/libvirtio/virtio-serial.c
>> new file mode 100644
>> index 0000000..ea56bef
>> --- /dev/null
>> +++ b/lib/libvirtio/virtio-serial.c
>> @@ -0,0 +1,202 @@
>> +/******************************************************************************
>> + * Copyright (c) 2016 IBM Corporation
>> + * All rights reserved.
>> + * This program and the accompanying materials
>> + * are made available under the terms of the BSD License
>> + * which accompanies this distribution, and is available at
>> + * http://www.opensource.org/licenses/bsd-license.php
>> + *
>> + * Contributors:
>> + *     IBM Corporation - initial implementation
>> + *****************************************************************************/
>> +
>> +/*
>> + * Virtio serial device definitions.
>> + * See Virtio 1.0 - 5.3 Console Device, for details
>> + */
>> +#include <stdio.h>
>> +#include <string.h>
>> +#include <cpu.h>
>> +#include <helpers.h>
>> +#include <byteorder.h>
>> +#include "virtio.h"
>> +#include "virtio-serial.h"
>> +#include "virtio-internal.h"
>> +
>> +#define DRIVER_FEATURE_SUPPORT  (VIRTIO_F_VERSION_1)
>
> No need in braces.

Sure.

>> +#define RX_ELEM_SIZE            4
>> +#define RX_NUM_ELEMS            128
>> +
>> +#define RX_Q 0
>> +#define TX_Q 1
>> +
>> +static struct vqs vq_rx;
>> +static struct vqs vq_tx;
>> +static uint16_t last_rx_idx;	/* Last index in RX "used" ring */
>> +
>> +int virtio_serial_init(struct virtio_device *dev)
>> +{
>> +	struct vring_avail *vq_avail;
>> +	int status = VIRTIO_STAT_ACKNOWLEDGE;
>> +
>> +	/* Reset device */
>> +	virtio_reset_device(dev);
>> +
>> +	/* Acknowledge device. */
>> +	virtio_set_status(dev, status);
>> +
>> +	/* Tell HV that we know how to drive the device. */
>> +	status |= VIRTIO_STAT_DRIVER;
>> +	virtio_set_status(dev, status);
>> +
>> +	if (dev->is_modern) {
>> +		/* Negotiate features and sets FEATURES_OK if successful */
>> +		if (virtio_negotiate_guest_features(dev, DRIVER_FEATURE_SUPPORT))
>> +			goto dev_error;
>> +
>> +		virtio_get_status(dev, &status);
>> +	}
>> +
>> +	if (virtio_queue_init_vq(dev, &vq_rx, RX_Q))
>> +		goto dev_error;
>> +
>> +	/* Allocate memory for multiple receive buffers */
>> +	vq_rx.buf_mem = SLOF_alloc_mem(RX_ELEM_SIZE * RX_NUM_ELEMS);
>> +	if (!vq_rx.buf_mem) {
>> +		printf("virtio-serial: Failed to allocate buffers!\n");
>> +		goto dev_error;
>> +	}
>> +
>> +	/* Prepare receive buffer queue */
>> +	for (int i = 0; i < RX_NUM_ELEMS; i++) {
>> +		uint64_t addr = (uint64_t)vq_rx.buf_mem + i * RX_ELEM_SIZE;
>> +
>> +		/* Descriptor for data: */
>> +		virtio_fill_desc(&vq_rx.desc[i], dev->is_modern, addr, 1, VRING_DESC_F_WRITE, 0);
>> +		vq_rx.avail->ring[i] = virtio_cpu_to_modern16(dev, i);
>> +	}
>> +	vq_rx.avail->flags = virtio_cpu_to_modern16(dev, VRING_AVAIL_F_NO_INTERRUPT);
>> +	vq_rx.avail->idx = virtio_cpu_to_modern16(dev, RX_NUM_ELEMS);
>> +	sync();
>> +
>> +	last_rx_idx = virtio_modern16_to_cpu(dev, vq_rx.used->idx);
>> +
>> +	if (virtio_queue_init_vq(dev, &vq_tx, TX_Q))
>> +		goto dev_error;
>> +
>> +	vq_avail = virtio_get_vring_avail(dev, TX_Q);
>> +	vq_avail->flags = virtio_cpu_to_modern16(dev, VRING_AVAIL_F_NO_INTERRUPT);
>> +	vq_avail->idx = 0;
>> +
>> +	/* Tell HV that setup succeeded */
>> +	status |= VIRTIO_STAT_DRIVER_OK;
>> +	virtio_set_status(dev, status);
>> +
>> +	return 1;
>> + dev_error:
>> +	printf("%s: failed\n", __func__);
>> +	status |= VIRTIO_STAT_FAILED;
>> +	virtio_set_status(dev, status);
>> +	return 0;
>> +}
>> +
>> +void virtio_serial_shutdown(struct virtio_device *dev)
>> +{
>> +	/* Quiesce device */
>> +	virtio_set_status(dev, VIRTIO_STAT_FAILED);
>> +
>> +	/* Reset device */
>> +	virtio_reset_device(dev);
>> +}
>> +
>> +int virtio_serial_putchar(struct virtio_device *dev, char c)
>> +{
>> +	struct vring_desc *desc;
>> +	int id;
>> +	uint32_t vq_size, time;
>> +	struct vring_desc *vq_desc;		/* Descriptor vring */
>> +	struct vring_avail *vq_avail;		/* "Available" vring */
>> +	struct vring_used *vq_used;		/* "Used" vring */
>
>
> The comments do not seem to tell lot more than variable names
> themselves.

Yes, can get rid of them.

>
>> +	volatile uint16_t *current_used_idx;
>> +	uint16_t last_used_idx, avail_idx;
>> +
>> +	vq_size = virtio_get_qsize(dev, TX_Q);
>> +	vq_desc = virtio_get_vring_desc(dev, TX_Q);
>> +	vq_avail = virtio_get_vring_avail(dev, TX_Q);
>> +	vq_used = virtio_get_vring_used(dev, TX_Q);
>> +
>> +	avail_idx = virtio_modern16_to_cpu(dev, vq_avail->idx);
>> +
>> +	last_used_idx = vq_used->idx;
>> +	current_used_idx = &vq_used->idx;
>> +
>> +	/* Determine descriptor index */
>> +	id = (avail_idx * 1) % vq_size;
>
> s/(avail_idx * 1)/avail_idx/

Right.

>
>> +
>> +	/* Set up virtqueue descriptor for header */
>> +	desc = &vq_desc[id];
>> +	virtio_fill_desc(desc, dev->is_modern, (uint64_t)&c, 1, 0, 0);
>> +
>> +	vq_avail->ring[avail_idx % vq_size] = virtio_cpu_to_modern16 (dev, id);
>> +	mb();
>> +	vq_avail->idx = virtio_cpu_to_modern16(dev, avail_idx + 1);
>> +
>> +	/* Tell HV that the queue is ready */
>> +	virtio_queue_notify(dev, TX_Q);
>> +
>> +	/* Wait for host to consume the descriptor */
>> +	time = SLOF_GetTimer() + VIRTIO_TIMEOUT;
>> +	while (*current_used_idx == last_used_idx) {
>> +		// do something better
>
> We should probably stop copying this line from driver to driver :)

Let that be there, so some day we can grep and fix :-)

>> +		mb();
>> +		if (time < SLOF_GetTimer()) {
>> +			printf("virtio_serial_putchar failed! \n");
>> +			return 0;
>> +		}
>> +	}
>> +
>> +	return 1;
>> +}
>> +
>> +static uint16_t last_rx_idx;	/* Last index in RX "used" ring */
>> +
>> +char virtio_serial_getchar(struct virtio_device *dev)
>> +{
>> +	int id, idx;
>> +	char buf[RX_NUM_ELEMS] = {0};
>> +	uint16_t avail_idx;
>> +
>> +	idx = virtio_modern16_to_cpu(dev, vq_rx.used->idx);
>> +	if (last_rx_idx == idx) {
>> +		/* Nothing received yet */
>> +		return 0;
>> +	}
>> +
>> +	id = (virtio_modern32_to_cpu(dev, vq_rx.used->ring[last_rx_idx % vq_rx.size].id) + 1)
>> +		% vq_rx.size;
>> +
>> +	/* Copy data to destination buffer */
>> +	memcpy(buf, (void *)virtio_modern64_to_cpu(dev, vq_rx.desc[id - 1].addr), RX_ELEM_SIZE);
>> +
>> +	/* Move indices to next entries */
>> +	last_rx_idx = last_rx_idx + 1;
>> +
>> +	avail_idx = virtio_modern16_to_cpu(dev, vq_rx.avail->idx);
>> +	vq_rx.avail->ring[avail_idx % vq_rx.size] = virtio_cpu_to_modern16(dev, id - 1);
>> +	sync();
>> +	vq_rx.avail->idx = virtio_cpu_to_modern16(dev, avail_idx + 1);
>> +	sync();
>> +
>> +	/* Tell HV that RX queue entry is ready */
>> +	virtio_queue_notify(dev, RX_Q);
>> +
>> +	return buf[0];
>> +}
>> +
>> +int virtio_serial_haschar(struct virtio_device *dev)
>> +{
>> +	if (last_rx_idx == virtio_modern16_to_cpu(dev, vq_rx.used->idx))
>> +		return 0;
>> +	else
>> +		return 1;
>> +}
>> diff --git a/lib/libvirtio/virtio-serial.h b/lib/libvirtio/virtio-serial.h
>> new file mode 100644
>> index 0000000..8abfd16
>> --- /dev/null
>> +++ b/lib/libvirtio/virtio-serial.h
>> @@ -0,0 +1,28 @@
>> +/******************************************************************************
>> + * Copyright (c) 2016 IBM Corporation
>> + * All rights reserved.
>> + * This program and the accompanying materials
>> + * are made available under the terms of the BSD License
>> + * which accompanies this distribution, and is available at
>> + * http://www.opensource.org/licenses/bsd-license.php
>> + *
>> + * Contributors:
>> + *     IBM Corporation - initial implementation
>> + *****************************************************************************/
>> +
>> +/*
>> + * Virtio serial device definitions.
>> + * See Virtio 1.0 - 5.3 Console Device, for details
>> + */
>> +
>> +#ifndef _VIRTIO_SERIAL_H
>> +#define _VIRTIO_SERIAL_H
>> +
>> +extern int virtio_serial_init(struct virtio_device *dev);
>> +extern void virtio_serial_shutdown(struct virtio_device *dev);
>> +extern int virtio_serial_putchar(struct virtio_device *dev, char c);
>> +extern char virtio_serial_getchar(struct virtio_device *dev);
>> +extern int virtio_serial_haschar(struct virtio_device *dev);
>> +
>> +#endif  /* _VIRTIO_SERIAL_H */
>> +
>
> And here
> /home/aik/p/slof/.git/rebase-apply/patch:489: new blank line at EOF.

Sure.

>> diff --git a/lib/libvirtio/virtio.code b/lib/libvirtio/virtio.code
>> index 8eec8f0..5cfffcc 100644
>> --- a/lib/libvirtio/virtio.code
>> +++ b/lib/libvirtio/virtio.code
>> @@ -15,6 +15,7 @@
>>  #include <virtio-9p.h>
>>  #include <virtio-scsi.h>
>>  #include <virtio-net.h>
>> +#include <virtio-serial.h>
>>  
>>  /******** core virtio ********/
>>  
>> @@ -165,3 +166,35 @@ PRIM(virtio_X2d_net_X2d_write)
>>  	TOS.n = virtionet_write(TOS.a, len);
>>  }
>>  MIRP
>> +
>> +/*********** virtio-serial ***********/
>> +// : virtio-serial-init ( dev -- false | true)
>> +PRIM(virtio_X2d_serial_X2d_init)
>> +	void *dev = TOS.a;
>> +	TOS.u = virtio_serial_init(dev);
>> +MIRP
>> +
>> +// : virtio-serial-shutdown ( dev -- )
>> +PRIM(virtio_X2d_serial_X2d_shutdown)
>> +	void *dev = TOS.a; POP;
>> +	virtio_serial_shutdown(dev);
>> +MIRP
>> +
>> +// : virtio-serial-putchar ( dev char -- )
>> +PRIM(virtio_X2d_serial_X2d_putchar)
>> +	unsigned long c = TOS.n; POP;
>> +	void *dev = TOS.a; POP;
>> +	virtio_serial_putchar(dev, c);
>> +MIRP
>> +
>> +// : virtio-serial-getchar ( dev -- char)
>> +PRIM(virtio_X2d_serial_X2d_getchar)
>> +	void *dev = TOS.a;
>> +	TOS.n = virtio_serial_getchar(dev);
>> +MIRP
>> +
>> +// : virtio-serial-haschar ( dev -- true | false)
>> +PRIM(virtio_X2d_serial_X2d_haschar)
>> +	void *dev = TOS.a;
>> +	TOS.n = virtio_serial_haschar(dev);
>> +MIRP
>> diff --git a/lib/libvirtio/virtio.in b/lib/libvirtio/virtio.in
>> index 195840e..d2b1641 100644
>> --- a/lib/libvirtio/virtio.in
>> +++ b/lib/libvirtio/virtio.in
>> @@ -33,3 +33,9 @@ cod(virtio-net-open)
>>  cod(virtio-net-close)
>>  cod(virtio-net-read)
>>  cod(virtio-net-write)
>> +
>> +cod(virtio-serial-init)
>> +cod(virtio-serial-shutdown)
>> +cod(virtio-serial-putchar)
>> +cod(virtio-serial-getchar)
>> +cod(virtio-serial-haschar)
>> 

Regards
Nikunj
Alexey Kardashevskiy Oct. 10, 2016, 10:04 a.m. UTC | #3
On 10/10/16 19:27, Nikunj A Dadhania wrote:
> Alexey Kardashevskiy <aik@ozlabs.ru> writes:
> 
>> On 10/10/16 03:09, Nikunj A Dadhania wrote:
>>> Add support for virtio serial device to be used as a console device.
>>> Currently, SLOF only supports spapr-vty device. With this addition
>>> virtio console can be used during boot.
>>
>>
>> Cool, good job, works fine!
> 
> Thanks for testing. Did you see kernel logs as well, as I could get
> login prompt, but not messages from kernel after quiesce

Same here, I do not remember why is this though, I remember hacking the
guest kernel to see the full log... Still, cool.

> 
>>>
>>> Signed-off-by: Nikunj A Dadhania <nikunj@linux.vnet.ibm.com>
>>> ---
>>>  board-qemu/slof/Makefile                |   3 +
>>>  board-qemu/slof/OF.fs                   |  24 +++-
>>>  board-qemu/slof/pci-device_1af4_1003.fs |  25 ++++
>>>  board-qemu/slof/pci-device_1af4_1043.fs |  15 +++
>>>  board-qemu/slof/virtio-serial.fs        |  95 +++++++++++++++
>>>  lib/libvirtio/Makefile                  |   2 +-
>>>  lib/libvirtio/virtio-serial.c           | 202 ++++++++++++++++++++++++++++++++
>>>  lib/libvirtio/virtio-serial.h           |  28 +++++
>>>  lib/libvirtio/virtio.code               |  33 ++++++
>>>  lib/libvirtio/virtio.in                 |   6 +
>>>  10 files changed, 426 insertions(+), 7 deletions(-)
>>>  create mode 100644 board-qemu/slof/pci-device_1af4_1003.fs
>>>  create mode 100644 board-qemu/slof/pci-device_1af4_1043.fs
>>>  create mode 100644 board-qemu/slof/virtio-serial.fs
>>>  create mode 100644 lib/libvirtio/virtio-serial.c
>>>  create mode 100644 lib/libvirtio/virtio-serial.h
>>>
>>> diff --git a/board-qemu/slof/Makefile b/board-qemu/slof/Makefile
>>> index 940a15a..cf57f16 100644
>>> --- a/board-qemu/slof/Makefile
>>> +++ b/board-qemu/slof/Makefile
>>> @@ -69,6 +69,8 @@ VIO_FFS_FILES = \
>>>  	$(SLOFBRDDIR)/pci-device_1af4_1041.fs \
>>>  	$(SLOFBRDDIR)/pci-device_1af4_1001.fs \
>>>  	$(SLOFBRDDIR)/pci-device_1af4_1042.fs \
>>> +	$(SLOFBRDDIR)/pci-device_1af4_1003.fs \
>>> +	$(SLOFBRDDIR)/pci-device_1af4_1043.fs \
>>>  	$(SLOFBRDDIR)/pci-device_1af4_1004.fs \
>>>  	$(SLOFBRDDIR)/pci-device_1af4_1048.fs \
>>>  	$(SLOFBRDDIR)/pci-device_1af4_1009.fs \
>>> @@ -79,6 +81,7 @@ VIO_FFS_FILES = \
>>>  	$(SLOFBRDDIR)/vio-veth.fs \
>>>  	$(SLOFBRDDIR)/rtas-nvram.fs \
>>>  	$(SLOFBRDDIR)/virtio-net.fs \
>>> +	$(SLOFBRDDIR)/virtio-serial.fs \
>>>  	$(SLOFBRDDIR)/virtio-block.fs \
>>>  	$(SLOFBRDDIR)/virtio-fs.fs \
>>>  	$(SLOFBRDDIR)/dev-null.fs \
>>> diff --git a/board-qemu/slof/OF.fs b/board-qemu/slof/OF.fs
>>> index c8df9ca..6b13e03 100644
>>> --- a/board-qemu/slof/OF.fs
>>> +++ b/board-qemu/slof/OF.fs
>>> @@ -162,6 +162,10 @@ CREATE version-str 10 ALLOT
>>>  : dump-display-write
>>>      s" screen" find-alias  IF
>>>          drop terminal-write drop
>>> +    ELSE
>>> +        s" vsterm" find-alias  IF
>>> +            drop type
>>> +        THEN
>>
>>
>> Why new alias, not usual "screen"? "screen" and "vsterm" cannot work
>> simultaneously any way.
> 
> "screen" is for vga display, so did not want to override, and is used in
> different context.

When is the context different?


> 
>>>      THEN
>>>  ;
>>>  
>>> @@ -236,12 +240,20 @@ romfs-base 400000 0 ' claim CATCH IF ." claim failed!" cr 2drop THEN drop
>>>                  ." using hvterm" cr
>>>                  " hvterm" io
>>>              ELSE
>>> -		    		    " /openprom" find-node ?dup IF
>>> -		    		        set-node
>>> -		    		        ." and no default found, creating dev-null" cr
>>> -		    		        " dev-null.fs" included
>>> -		    		        " devnull-console" io
>>> -		    		        0 set-node
>>
>> Here that indentation is fixed :)
> 
> The above hunk is the removal. I will check 1/3 and fix there.
> 
>>
>>> +                " vsterm" find-alias IF
>>> +                    drop
>>> +                    ." using vsterm" cr
>>> +                    " vsterm" io
>>> +                    false to store-prevga?
>>> +                    dump-display-buffer
>>> +                ELSE
>>> +                    " /openprom" find-node ?dup IF
>>> +                        set-node
>>> +                        ." and no default found, creating dev-null" cr
>>> +                        " dev-null.fs" included
>>> +                        " devnull-console" io
>>> +                        0 set-node
> 
> I think you meant here. But then i had to move this one more level deep.

Correct :)
Nikunj A Dadhania Oct. 10, 2016, 10:11 a.m. UTC | #4
Alexey Kardashevskiy <aik@ozlabs.ru> writes:

> On 10/10/16 19:27, Nikunj A Dadhania wrote:
>> Alexey Kardashevskiy <aik@ozlabs.ru> writes:
>> 
>>> On 10/10/16 03:09, Nikunj A Dadhania wrote:
>>>> Add support for virtio serial device to be used as a console device.
>>>> Currently, SLOF only supports spapr-vty device. With this addition
>>>> virtio console can be used during boot.
>>>
>>>
>>> Cool, good job, works fine!
>> 
>> Thanks for testing. Did you see kernel logs as well, as I could get
>> login prompt, but not messages from kernel after quiesce
>
> Same here, I do not remember why is this though, I remember hacking the
> guest kernel to see the full log... Still, cool.
>
>> 
>>>>
>>>> Signed-off-by: Nikunj A Dadhania <nikunj@linux.vnet.ibm.com>
>>>> ---
>>>>  board-qemu/slof/Makefile                |   3 +
>>>>  board-qemu/slof/OF.fs                   |  24 +++-
>>>>  board-qemu/slof/pci-device_1af4_1003.fs |  25 ++++
>>>>  board-qemu/slof/pci-device_1af4_1043.fs |  15 +++
>>>>  board-qemu/slof/virtio-serial.fs        |  95 +++++++++++++++
>>>>  lib/libvirtio/Makefile                  |   2 +-
>>>>  lib/libvirtio/virtio-serial.c           | 202 ++++++++++++++++++++++++++++++++
>>>>  lib/libvirtio/virtio-serial.h           |  28 +++++
>>>>  lib/libvirtio/virtio.code               |  33 ++++++
>>>>  lib/libvirtio/virtio.in                 |   6 +
>>>>  10 files changed, 426 insertions(+), 7 deletions(-)
>>>>  create mode 100644 board-qemu/slof/pci-device_1af4_1003.fs
>>>>  create mode 100644 board-qemu/slof/pci-device_1af4_1043.fs
>>>>  create mode 100644 board-qemu/slof/virtio-serial.fs
>>>>  create mode 100644 lib/libvirtio/virtio-serial.c
>>>>  create mode 100644 lib/libvirtio/virtio-serial.h
>>>>
>>>> diff --git a/board-qemu/slof/Makefile b/board-qemu/slof/Makefile
>>>> index 940a15a..cf57f16 100644
>>>> --- a/board-qemu/slof/Makefile
>>>> +++ b/board-qemu/slof/Makefile
>>>> @@ -69,6 +69,8 @@ VIO_FFS_FILES = \
>>>>  	$(SLOFBRDDIR)/pci-device_1af4_1041.fs \
>>>>  	$(SLOFBRDDIR)/pci-device_1af4_1001.fs \
>>>>  	$(SLOFBRDDIR)/pci-device_1af4_1042.fs \
>>>> +	$(SLOFBRDDIR)/pci-device_1af4_1003.fs \
>>>> +	$(SLOFBRDDIR)/pci-device_1af4_1043.fs \
>>>>  	$(SLOFBRDDIR)/pci-device_1af4_1004.fs \
>>>>  	$(SLOFBRDDIR)/pci-device_1af4_1048.fs \
>>>>  	$(SLOFBRDDIR)/pci-device_1af4_1009.fs \
>>>> @@ -79,6 +81,7 @@ VIO_FFS_FILES = \
>>>>  	$(SLOFBRDDIR)/vio-veth.fs \
>>>>  	$(SLOFBRDDIR)/rtas-nvram.fs \
>>>>  	$(SLOFBRDDIR)/virtio-net.fs \
>>>> +	$(SLOFBRDDIR)/virtio-serial.fs \
>>>>  	$(SLOFBRDDIR)/virtio-block.fs \
>>>>  	$(SLOFBRDDIR)/virtio-fs.fs \
>>>>  	$(SLOFBRDDIR)/dev-null.fs \
>>>> diff --git a/board-qemu/slof/OF.fs b/board-qemu/slof/OF.fs
>>>> index c8df9ca..6b13e03 100644
>>>> --- a/board-qemu/slof/OF.fs
>>>> +++ b/board-qemu/slof/OF.fs
>>>> @@ -162,6 +162,10 @@ CREATE version-str 10 ALLOT
>>>>  : dump-display-write
>>>>      s" screen" find-alias  IF
>>>>          drop terminal-write drop
>>>> +    ELSE
>>>> +        s" vsterm" find-alias  IF
>>>> +            drop type
>>>> +        THEN
>>>
>>>
>>> Why new alias, not usual "screen"? "screen" and "vsterm" cannot work
>>> simultaneously any way.
>> 
>> "screen" is for vga display, so did not want to override, and is used in
>> different context.
>
> When is the context different?

First problem is there in set-default-console:

        " screen" find-alias dup IF nip THEN
        " keyboard" find-alias dup IF nip THEN
        AND IF
            ." using screen & keyboard" cr
            " screen" output
            " keyboard" input

If " screen" is there, it looks for " keyboard" i.e. usb-keyboard.

Only if " screen" is not set, we start looking for hvterm and now with
this  vsterm

Regards
Nikunj
Alexey Kardashevskiy Oct. 13, 2016, 1:25 a.m. UTC | #5
On 10/10/16 21:11, Nikunj A Dadhania wrote:
> Alexey Kardashevskiy <aik@ozlabs.ru> writes:
> 
>> On 10/10/16 19:27, Nikunj A Dadhania wrote:
>>> Alexey Kardashevskiy <aik@ozlabs.ru> writes:
>>>
>>>> On 10/10/16 03:09, Nikunj A Dadhania wrote:
>>>>> Add support for virtio serial device to be used as a console device.
>>>>> Currently, SLOF only supports spapr-vty device. With this addition
>>>>> virtio console can be used during boot.
>>>>
>>>>
>>>> Cool, good job, works fine!
>>>
>>> Thanks for testing. Did you see kernel logs as well, as I could get
>>> login prompt, but not messages from kernel after quiesce
>>
>> Same here, I do not remember why is this though, I remember hacking the
>> guest kernel to see the full log... Still, cool.


btw I remembered it wrong. Adding "console=hvc0" to the kernel command line
(via QEMU's -append or in grub/fedora23) makes the kernel print everything.
diff mbox

Patch

diff --git a/board-qemu/slof/Makefile b/board-qemu/slof/Makefile
index 940a15a..cf57f16 100644
--- a/board-qemu/slof/Makefile
+++ b/board-qemu/slof/Makefile
@@ -69,6 +69,8 @@  VIO_FFS_FILES = \
 	$(SLOFBRDDIR)/pci-device_1af4_1041.fs \
 	$(SLOFBRDDIR)/pci-device_1af4_1001.fs \
 	$(SLOFBRDDIR)/pci-device_1af4_1042.fs \
+	$(SLOFBRDDIR)/pci-device_1af4_1003.fs \
+	$(SLOFBRDDIR)/pci-device_1af4_1043.fs \
 	$(SLOFBRDDIR)/pci-device_1af4_1004.fs \
 	$(SLOFBRDDIR)/pci-device_1af4_1048.fs \
 	$(SLOFBRDDIR)/pci-device_1af4_1009.fs \
@@ -79,6 +81,7 @@  VIO_FFS_FILES = \
 	$(SLOFBRDDIR)/vio-veth.fs \
 	$(SLOFBRDDIR)/rtas-nvram.fs \
 	$(SLOFBRDDIR)/virtio-net.fs \
+	$(SLOFBRDDIR)/virtio-serial.fs \
 	$(SLOFBRDDIR)/virtio-block.fs \
 	$(SLOFBRDDIR)/virtio-fs.fs \
 	$(SLOFBRDDIR)/dev-null.fs \
diff --git a/board-qemu/slof/OF.fs b/board-qemu/slof/OF.fs
index c8df9ca..6b13e03 100644
--- a/board-qemu/slof/OF.fs
+++ b/board-qemu/slof/OF.fs
@@ -162,6 +162,10 @@  CREATE version-str 10 ALLOT
 : dump-display-write
     s" screen" find-alias  IF
         drop terminal-write drop
+    ELSE
+        s" vsterm" find-alias  IF
+            drop type
+        THEN
     THEN
 ;
 
@@ -236,12 +240,20 @@  romfs-base 400000 0 ' claim CATCH IF ." claim failed!" cr 2drop THEN drop
                 ." using hvterm" cr
                 " hvterm" io
             ELSE
-		    		    " /openprom" find-node ?dup IF
-		    		        set-node
-		    		        ." and no default found, creating dev-null" cr
-		    		        " dev-null.fs" included
-		    		        " devnull-console" io
-		    		        0 set-node
+                " vsterm" find-alias IF
+                    drop
+                    ." using vsterm" cr
+                    " vsterm" io
+                    false to store-prevga?
+                    dump-display-buffer
+                ELSE
+                    " /openprom" find-node ?dup IF
+                        set-node
+                        ." and no default found, creating dev-null" cr
+                        " dev-null.fs" included
+                        " devnull-console" io
+                        0 set-node
+                    THEN
                 THEN
             THEN
         THEN
diff --git a/board-qemu/slof/pci-device_1af4_1003.fs b/board-qemu/slof/pci-device_1af4_1003.fs
new file mode 100644
index 0000000..a7cd53b
--- /dev/null
+++ b/board-qemu/slof/pci-device_1af4_1003.fs
@@ -0,0 +1,25 @@ 
+\ *****************************************************************************
+\ * Copyright (c) 2016 IBM Corporation
+\ * All rights reserved.
+\ * This program and the accompanying materials
+\ * are made available under the terms of the BSD License
+\ * which accompanies this distribution, and is available at
+\ * http://www.opensource.org/licenses/bsd-license.php
+\ *
+\ * Contributors:
+\ *     IBM Corporation - initial implementation
+\ ****************************************************************************/
+
+\ Handle virtio-serial device
+
+s" virtio [ serial ]" type cr
+
+my-space pci-device-generic-setup
+
+pci-master-enable
+pci-mem-enable
+pci-io-enable
+
+s" virtio-serial.fs" included
+
+pci-device-disable
diff --git a/board-qemu/slof/pci-device_1af4_1043.fs b/board-qemu/slof/pci-device_1af4_1043.fs
new file mode 100644
index 0000000..f044450
--- /dev/null
+++ b/board-qemu/slof/pci-device_1af4_1043.fs
@@ -0,0 +1,15 @@ 
+\ *****************************************************************************
+\ * Copyright (c) 2016 IBM Corporation
+\ * All rights reserved.
+\ * This program and the accompanying materials
+\ * are made available under the terms of the BSD License
+\ * which accompanies this distribution, and is available at
+\ * http://www.opensource.org/licenses/bsd-license.php
+\ *
+\ * Contributors:
+\ *     IBM Corporation - initial implementation
+\ ****************************************************************************/
+
+\ Device ID 1044 is for virtio-serial non-transitional device.
+\ Include the driver for virtio-serial
+s" pci-device_1af4_1003.fs" included
diff --git a/board-qemu/slof/virtio-serial.fs b/board-qemu/slof/virtio-serial.fs
new file mode 100644
index 0000000..0f25a80
--- /dev/null
+++ b/board-qemu/slof/virtio-serial.fs
@@ -0,0 +1,95 @@ 
+\ *****************************************************************************
+\ * Copyright (c) 2016 IBM Corporation
+\ * All rights reserved.
+\ * This program and the accompanying materials
+\ * are made available under the terms of the BSD License
+\ * which accompanies this distribution, and is available at
+\ * http://www.opensource.org/licenses/bsd-license.php
+\ *
+\ * Contributors:
+\ *     IBM Corporation - initial implementation
+\ ****************************************************************************/
+
+s" serial" device-type
+
+FALSE VALUE initialized?
+
+virtio-setup-vd VALUE virtiodev
+
+\ Quiesce the virtqueue of this device so that no more background
+\ transactions can be pending.
+: shutdown  ( -- )
+    initialized? IF
+        my-phandle node>path open-dev ?dup IF
+            virtiodev virtio-serial-shutdown
+            close-dev
+        THEN
+        FALSE to initialized?
+    THEN
+;
+
+: virtio-serial-term-emit
+    virtiodev SWAP virtio-serial-putchar
+;
+
+: virtio-serial-term-key?  virtiodev virtio-serial-haschar ;
+: virtio-serial-term-key   BEGIN virtio-serial-term-key? UNTIL virtiodev virtio-serial-getchar ;
+
+\ Basic device initialization - which has only to be done once
+: init  ( -- )
+virtiodev virtio-serial-init drop
+    TRUE to initialized?
+    ['] virtio-serial-term-emit to emit
+    ['] virtio-serial-term-key  to key
+    ['] virtio-serial-term-key? to key?
+    ['] shutdown add-quiesce-xt
+;
+
+0 VALUE open-count
+
+\ Standard node "open" function
+: open  ( -- okay? )
+    open-count 0= IF
+        open IF initialized? 0= IF init THEN
+            true
+        ELSE false exit
+        THEN
+    ELSE true THEN
+    open-count 1 + to open-count
+;
+
+: close
+    open-count 0> IF
+        open-count 1 - dup to open-count
+        0= IF close THEN
+    THEN
+    close
+;
+
+: write ( adr len -- actual )
+    tuck
+    0 ?DO
+        dup c@ virtiodev SWAP virtio-serial-putchar
+        1 +
+    LOOP
+    drop
+;
+
+: read ( adr len -- actual )
+    0= IF drop 0 EXIT THEN
+    virtiodev virtio-serial-haschar 0= IF 0 swap c! -2 EXIT THEN
+    virtiodev virtio-serial-getchar swap c! 1
+;
+
+: setup-alias
+    " vsterm" find-alias 0= IF
+        " vsterm" get-node node>path set-alias
+    ELSE drop THEN
+;
+setup-alias
+
+\ Override serial methods to make term-io.fs happy
+: serial-emit virtio-serial-term-emit ;
+: serial-key? virtio-serial-term-key? ;
+: serial-key  virtio-serial-term-key  ;
+
diff --git a/lib/libvirtio/Makefile b/lib/libvirtio/Makefile
index bd6a1fa..87d9513 100644
--- a/lib/libvirtio/Makefile
+++ b/lib/libvirtio/Makefile
@@ -24,7 +24,7 @@  TARGET = ../libvirtio.a
 
 all: $(TARGET)
 
-SRCS =  virtio.c virtio-blk.c p9.c virtio-9p.c virtio-scsi.c virtio-net.c
+SRCS =  virtio.c virtio-blk.c p9.c virtio-9p.c virtio-scsi.c virtio-net.c virtio-serial.c
 
 OBJS = $(SRCS:%.c=%.o)
 
diff --git a/lib/libvirtio/virtio-serial.c b/lib/libvirtio/virtio-serial.c
new file mode 100644
index 0000000..ea56bef
--- /dev/null
+++ b/lib/libvirtio/virtio-serial.c
@@ -0,0 +1,202 @@ 
+/******************************************************************************
+ * Copyright (c) 2016 IBM Corporation
+ * All rights reserved.
+ * This program and the accompanying materials
+ * are made available under the terms of the BSD License
+ * which accompanies this distribution, and is available at
+ * http://www.opensource.org/licenses/bsd-license.php
+ *
+ * Contributors:
+ *     IBM Corporation - initial implementation
+ *****************************************************************************/
+
+/*
+ * Virtio serial device definitions.
+ * See Virtio 1.0 - 5.3 Console Device, for details
+ */
+#include <stdio.h>
+#include <string.h>
+#include <cpu.h>
+#include <helpers.h>
+#include <byteorder.h>
+#include "virtio.h"
+#include "virtio-serial.h"
+#include "virtio-internal.h"
+
+#define DRIVER_FEATURE_SUPPORT  (VIRTIO_F_VERSION_1)
+#define RX_ELEM_SIZE            4
+#define RX_NUM_ELEMS            128
+
+#define RX_Q 0
+#define TX_Q 1
+
+static struct vqs vq_rx;
+static struct vqs vq_tx;
+static uint16_t last_rx_idx;	/* Last index in RX "used" ring */
+
+int virtio_serial_init(struct virtio_device *dev)
+{
+	struct vring_avail *vq_avail;
+	int status = VIRTIO_STAT_ACKNOWLEDGE;
+
+	/* Reset device */
+	virtio_reset_device(dev);
+
+	/* Acknowledge device. */
+	virtio_set_status(dev, status);
+
+	/* Tell HV that we know how to drive the device. */
+	status |= VIRTIO_STAT_DRIVER;
+	virtio_set_status(dev, status);
+
+	if (dev->is_modern) {
+		/* Negotiate features and sets FEATURES_OK if successful */
+		if (virtio_negotiate_guest_features(dev, DRIVER_FEATURE_SUPPORT))
+			goto dev_error;
+
+		virtio_get_status(dev, &status);
+	}
+
+	if (virtio_queue_init_vq(dev, &vq_rx, RX_Q))
+		goto dev_error;
+
+	/* Allocate memory for multiple receive buffers */
+	vq_rx.buf_mem = SLOF_alloc_mem(RX_ELEM_SIZE * RX_NUM_ELEMS);
+	if (!vq_rx.buf_mem) {
+		printf("virtio-serial: Failed to allocate buffers!\n");
+		goto dev_error;
+	}
+
+	/* Prepare receive buffer queue */
+	for (int i = 0; i < RX_NUM_ELEMS; i++) {
+		uint64_t addr = (uint64_t)vq_rx.buf_mem + i * RX_ELEM_SIZE;
+
+		/* Descriptor for data: */
+		virtio_fill_desc(&vq_rx.desc[i], dev->is_modern, addr, 1, VRING_DESC_F_WRITE, 0);
+		vq_rx.avail->ring[i] = virtio_cpu_to_modern16(dev, i);
+	}
+	vq_rx.avail->flags = virtio_cpu_to_modern16(dev, VRING_AVAIL_F_NO_INTERRUPT);
+	vq_rx.avail->idx = virtio_cpu_to_modern16(dev, RX_NUM_ELEMS);
+	sync();
+
+	last_rx_idx = virtio_modern16_to_cpu(dev, vq_rx.used->idx);
+
+	if (virtio_queue_init_vq(dev, &vq_tx, TX_Q))
+		goto dev_error;
+
+	vq_avail = virtio_get_vring_avail(dev, TX_Q);
+	vq_avail->flags = virtio_cpu_to_modern16(dev, VRING_AVAIL_F_NO_INTERRUPT);
+	vq_avail->idx = 0;
+
+	/* Tell HV that setup succeeded */
+	status |= VIRTIO_STAT_DRIVER_OK;
+	virtio_set_status(dev, status);
+
+	return 1;
+ dev_error:
+	printf("%s: failed\n", __func__);
+	status |= VIRTIO_STAT_FAILED;
+	virtio_set_status(dev, status);
+	return 0;
+}
+
+void virtio_serial_shutdown(struct virtio_device *dev)
+{
+	/* Quiesce device */
+	virtio_set_status(dev, VIRTIO_STAT_FAILED);
+
+	/* Reset device */
+	virtio_reset_device(dev);
+}
+
+int virtio_serial_putchar(struct virtio_device *dev, char c)
+{
+	struct vring_desc *desc;
+	int id;
+	uint32_t vq_size, time;
+	struct vring_desc *vq_desc;		/* Descriptor vring */
+	struct vring_avail *vq_avail;		/* "Available" vring */
+	struct vring_used *vq_used;		/* "Used" vring */
+	volatile uint16_t *current_used_idx;
+	uint16_t last_used_idx, avail_idx;
+
+	vq_size = virtio_get_qsize(dev, TX_Q);
+	vq_desc = virtio_get_vring_desc(dev, TX_Q);
+	vq_avail = virtio_get_vring_avail(dev, TX_Q);
+	vq_used = virtio_get_vring_used(dev, TX_Q);
+
+	avail_idx = virtio_modern16_to_cpu(dev, vq_avail->idx);
+
+	last_used_idx = vq_used->idx;
+	current_used_idx = &vq_used->idx;
+
+	/* Determine descriptor index */
+	id = (avail_idx * 1) % vq_size;
+
+	/* Set up virtqueue descriptor for header */
+	desc = &vq_desc[id];
+	virtio_fill_desc(desc, dev->is_modern, (uint64_t)&c, 1, 0, 0);
+
+	vq_avail->ring[avail_idx % vq_size] = virtio_cpu_to_modern16 (dev, id);
+	mb();
+	vq_avail->idx = virtio_cpu_to_modern16(dev, avail_idx + 1);
+
+	/* Tell HV that the queue is ready */
+	virtio_queue_notify(dev, TX_Q);
+
+	/* Wait for host to consume the descriptor */
+	time = SLOF_GetTimer() + VIRTIO_TIMEOUT;
+	while (*current_used_idx == last_used_idx) {
+		// do something better
+		mb();
+		if (time < SLOF_GetTimer()) {
+			printf("virtio_serial_putchar failed! \n");
+			return 0;
+		}
+	}
+
+	return 1;
+}
+
+static uint16_t last_rx_idx;	/* Last index in RX "used" ring */
+
+char virtio_serial_getchar(struct virtio_device *dev)
+{
+	int id, idx;
+	char buf[RX_NUM_ELEMS] = {0};
+	uint16_t avail_idx;
+
+	idx = virtio_modern16_to_cpu(dev, vq_rx.used->idx);
+	if (last_rx_idx == idx) {
+		/* Nothing received yet */
+		return 0;
+	}
+
+	id = (virtio_modern32_to_cpu(dev, vq_rx.used->ring[last_rx_idx % vq_rx.size].id) + 1)
+		% vq_rx.size;
+
+	/* Copy data to destination buffer */
+	memcpy(buf, (void *)virtio_modern64_to_cpu(dev, vq_rx.desc[id - 1].addr), RX_ELEM_SIZE);
+
+	/* Move indices to next entries */
+	last_rx_idx = last_rx_idx + 1;
+
+	avail_idx = virtio_modern16_to_cpu(dev, vq_rx.avail->idx);
+	vq_rx.avail->ring[avail_idx % vq_rx.size] = virtio_cpu_to_modern16(dev, id - 1);
+	sync();
+	vq_rx.avail->idx = virtio_cpu_to_modern16(dev, avail_idx + 1);
+	sync();
+
+	/* Tell HV that RX queue entry is ready */
+	virtio_queue_notify(dev, RX_Q);
+
+	return buf[0];
+}
+
+int virtio_serial_haschar(struct virtio_device *dev)
+{
+	if (last_rx_idx == virtio_modern16_to_cpu(dev, vq_rx.used->idx))
+		return 0;
+	else
+		return 1;
+}
diff --git a/lib/libvirtio/virtio-serial.h b/lib/libvirtio/virtio-serial.h
new file mode 100644
index 0000000..8abfd16
--- /dev/null
+++ b/lib/libvirtio/virtio-serial.h
@@ -0,0 +1,28 @@ 
+/******************************************************************************
+ * Copyright (c) 2016 IBM Corporation
+ * All rights reserved.
+ * This program and the accompanying materials
+ * are made available under the terms of the BSD License
+ * which accompanies this distribution, and is available at
+ * http://www.opensource.org/licenses/bsd-license.php
+ *
+ * Contributors:
+ *     IBM Corporation - initial implementation
+ *****************************************************************************/
+
+/*
+ * Virtio serial device definitions.
+ * See Virtio 1.0 - 5.3 Console Device, for details
+ */
+
+#ifndef _VIRTIO_SERIAL_H
+#define _VIRTIO_SERIAL_H
+
+extern int virtio_serial_init(struct virtio_device *dev);
+extern void virtio_serial_shutdown(struct virtio_device *dev);
+extern int virtio_serial_putchar(struct virtio_device *dev, char c);
+extern char virtio_serial_getchar(struct virtio_device *dev);
+extern int virtio_serial_haschar(struct virtio_device *dev);
+
+#endif  /* _VIRTIO_SERIAL_H */
+
diff --git a/lib/libvirtio/virtio.code b/lib/libvirtio/virtio.code
index 8eec8f0..5cfffcc 100644
--- a/lib/libvirtio/virtio.code
+++ b/lib/libvirtio/virtio.code
@@ -15,6 +15,7 @@ 
 #include <virtio-9p.h>
 #include <virtio-scsi.h>
 #include <virtio-net.h>
+#include <virtio-serial.h>
 
 /******** core virtio ********/
 
@@ -165,3 +166,35 @@  PRIM(virtio_X2d_net_X2d_write)
 	TOS.n = virtionet_write(TOS.a, len);
 }
 MIRP
+
+/*********** virtio-serial ***********/
+// : virtio-serial-init ( dev -- false | true)
+PRIM(virtio_X2d_serial_X2d_init)
+	void *dev = TOS.a;
+	TOS.u = virtio_serial_init(dev);
+MIRP
+
+// : virtio-serial-shutdown ( dev -- )
+PRIM(virtio_X2d_serial_X2d_shutdown)
+	void *dev = TOS.a; POP;
+	virtio_serial_shutdown(dev);
+MIRP
+
+// : virtio-serial-putchar ( dev char -- )
+PRIM(virtio_X2d_serial_X2d_putchar)
+	unsigned long c = TOS.n; POP;
+	void *dev = TOS.a; POP;
+	virtio_serial_putchar(dev, c);
+MIRP
+
+// : virtio-serial-getchar ( dev -- char)
+PRIM(virtio_X2d_serial_X2d_getchar)
+	void *dev = TOS.a;
+	TOS.n = virtio_serial_getchar(dev);
+MIRP
+
+// : virtio-serial-haschar ( dev -- true | false)
+PRIM(virtio_X2d_serial_X2d_haschar)
+	void *dev = TOS.a;
+	TOS.n = virtio_serial_haschar(dev);
+MIRP
diff --git a/lib/libvirtio/virtio.in b/lib/libvirtio/virtio.in
index 195840e..d2b1641 100644
--- a/lib/libvirtio/virtio.in
+++ b/lib/libvirtio/virtio.in
@@ -33,3 +33,9 @@  cod(virtio-net-open)
 cod(virtio-net-close)
 cod(virtio-net-read)
 cod(virtio-net-write)
+
+cod(virtio-serial-init)
+cod(virtio-serial-shutdown)
+cod(virtio-serial-putchar)
+cod(virtio-serial-getchar)
+cod(virtio-serial-haschar)