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

Alexey Kardashevskiy aik at ozlabs.ru
Mon Oct 10 17:18:20 AEDT 2016


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 at 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)
> 


-- 
Alexey


More information about the SLOF mailing list