[SLOF] [PATCH RFC v0 3/3] Add virtio-serial device support
Nikunj A Dadhania
nikunj at linux.vnet.ibm.com
Mon Oct 10 19:27:08 AEDT 2016
Alexey Kardashevskiy <aik at 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 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.
"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
More information about the SLOF
mailing list