[Qemu-devel] [RFC] virtio-serial device

[Qemu-devel] [RFC] virtio-serial device

[Amit Shah]

Hello,

These patches implement a virtio-serial pci device that can expose
multiple serial ports over one pci device.

It's still under development and hacky but I'm sending out for early
review comments.

Thanks,
Amit

[Qemu-devel] [PATCH] pci: add define for communication class devices

[Amit Shah]

Signed-off-by: Amit Shah <amit.shah@redhat.com>
---
 hw/pci_ids.h |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/hw/pci_ids.h b/hw/pci_ids.h
index 427fcd5..3afe674 100644
--- a/hw/pci_ids.h
+++ b/hw/pci_ids.h
@@ -35,6 +35,8 @@
 #define PCI_CLASS_BRIDGE_PCI             0x0604
 #define PCI_CLASS_BRIDGE_OTHER           0x0680
 
+#define PCI_CLASS_COMMUNICATION_OTHER    0x0780
+
 #define PCI_CLASS_PROCESSOR_CO           0x0b40
 #define PCI_CLASS_PROCESSOR_POWERPC      0x0b20
 
-- 
1.6.0.6

[Qemu-devel] [PATCH] qdev: add virtserial char drv init hacks

[Amit Shah]

Signed-off-by: Amit Shah <amit.shah@redhat.com>
---
 hw/qdev.c |    6 +++++-
 1 files changed, 5 insertions(+), 1 deletions(-)

diff --git a/hw/qdev.c b/hw/qdev.c
index 689cf4c..661bcde 100644
--- a/hw/qdev.c
+++ b/hw/qdev.c
@@ -172,9 +172,13 @@ CharDriverState *qdev_init_chardev(DeviceState *dev)
 {
     static int next_serial;
     static int next_virtconsole;
+    static int next_virtserial;
+
     /* FIXME: This is a nasty hack that needs to go away.  */
-    if (strncmp(dev->type->name, "virtio", 6) == 0) {
+    if (strncmp(dev->name, "virtio-console", 14) == 0) {
         return virtcon_hds[next_virtconsole++];
+    } else if (strncmp(dev->name, "virtio-serial", 13) == 0) {
+        return virtio_serial_hds[next_virtserial++];
     } else {
         return serial_hds[next_serial++];
     }
-- 
1.6.0.6

[Qemu-devel] [PATCH] virtio-serial: PCI device for simple host <-> guest communication

[Amit Shah]

This interface presents a char device from which bits can be
sent and read.

Sample uses for such a device can be obtaining info from the
guest like the file systems used, apps installed, etc. for
offline usage and logged-in users, clipboard copy-paste, etc.
for online usage.

Signed-off-by: Amit Shah <amit.shah@redhat.com>
---
 Makefile.target    |    2 +-
 hw/pc.c            |   17 +++
 hw/pci.h           |    1 +
 hw/virtio-pci.c    |   15 +++
 hw/virtio-serial.c |  320 ++++++++++++++++++++++++++++++++++++++++++++++++++++
 hw/virtio-serial.h |   31 +++++
 hw/virtio.h        |    1 +
 qemu-options.hx    |    8 ++
 sysemu.h           |   11 ++
 vl.c               |   62 ++++++++++
 10 files changed, 467 insertions(+), 1 deletions(-)
 create mode 100644 hw/virtio-serial.c
 create mode 100644 hw/virtio-serial.h

diff --git a/Makefile.target b/Makefile.target
index 27de4b9..d8ad787 100644
--- a/Makefile.target
+++ b/Makefile.target
@@ -497,7 +497,7 @@ OBJS=vl.o osdep.o monitor.o pci.o loader.o isa_mmio.o machine.o \
      gdbstub.o gdbstub-xml.o
 # virtio has to be here due to weird dependency between PCI and virtio-net.
 # need to fix this properly
-OBJS+=virtio-blk.o virtio-balloon.o virtio-net.o virtio-console.o
+OBJS+=virtio-blk.o virtio-balloon.o virtio-net.o virtio-console.o virtio-serial.o
 ifdef CONFIG_KVM
 OBJS+=kvm.o kvm-all.o
 endif
diff --git a/hw/pc.c b/hw/pc.c
index 0934778..b4136a0 100644
--- a/hw/pc.c
+++ b/hw/pc.c
@@ -37,6 +37,8 @@
 #include "watchdog.h"
 #include "smbios.h"
 
+void *virtio_serial_new_port(PCIDevice *dev, int idx, char *name);
+
 /* output Bochs bios info messages */
 //#define DEBUG_BIOS
 
@@ -1164,6 +1166,21 @@ static void pc_init1(ram_addr_t ram_size,
             }
         }
     }
+
+    /* Add virtio serial devices */
+    if (pci_enabled && virtio_serial_index) {
+        void *dev;
+
+        dev = pci_create_simple(pci_bus, -1, "virtio-serial-pci");
+        if (!dev) {
+            fprintf(stderr, "qemu: could not create virtio serial pci device\n");
+            exit(1);
+        }
+
+        for (i = 0; i < virtio_serial_index; i++) {
+                virtio_serial_new_port(dev, i, virtio_serial_names[i]);
+        }
+    }
 }
 
 static void pc_init_pci(ram_addr_t ram_size,
diff --git a/hw/pci.h b/hw/pci.h
index 0405837..ab06008 100644
--- a/hw/pci.h
+++ b/hw/pci.h
@@ -69,6 +69,7 @@ extern target_phys_addr_t pci_mem_base;
 #define PCI_DEVICE_ID_VIRTIO_BLOCK       0x1001
 #define PCI_DEVICE_ID_VIRTIO_BALLOON     0x1002
 #define PCI_DEVICE_ID_VIRTIO_CONSOLE     0x1003
+#define PCI_DEVICE_ID_VIRTIO_SERIAL      0x1004
 
 typedef void PCIConfigWriteFunc(PCIDevice *pci_dev,
                                 uint32_t address, uint32_t data, int len);
diff --git a/hw/virtio-pci.c b/hw/virtio-pci.c
index c072423..30e56e1 100644
--- a/hw/virtio-pci.c
+++ b/hw/virtio-pci.c
@@ -334,6 +334,19 @@ static void virtio_balloon_init_pci(PCIDevice *pci_dev)
                     0x00);
 }
 
+static void virtio_serial_init_pci(PCIDevice *pci_dev)
+{
+    VirtIOPCIProxy *proxy = DO_UPCAST(VirtIOPCIProxy, pci_dev, pci_dev);
+    VirtIODevice *vdev;
+
+    vdev = virtio_serial_init(&pci_dev->qdev);
+    virtio_init_pci(proxy, vdev,
+                    PCI_VENDOR_ID_REDHAT_QUMRANET,
+                    PCI_DEVICE_ID_VIRTIO_SERIAL,
+                    PCI_CLASS_COMMUNICATION_OTHER,
+                    0x00);
+}
+
 static void virtio_pci_register_devices(void)
 {
     pci_qdev_register("virtio-blk-pci", sizeof(VirtIOPCIProxy),
@@ -344,6 +357,8 @@ static void virtio_pci_register_devices(void)
                       virtio_console_init_pci);
     pci_qdev_register("virtio-balloon-pci", sizeof(VirtIOPCIProxy),
                       virtio_balloon_init_pci);
+    pci_qdev_register("virtio-serial-pci", sizeof(VirtIOPCIProxy),
+                      virtio_serial_init_pci);
 }
 
 device_init(virtio_pci_register_devices)
diff --git a/hw/virtio-serial.c b/hw/virtio-serial.c
new file mode 100644
index 0000000..9ce06f0
--- /dev/null
+++ b/hw/virtio-serial.c
@@ -0,0 +1,320 @@
+/*
+ * Virtio serial interface
+ *
+ * This serial interface is a paravirtualised guest<->host
+ * communication channel for relaying short messages and events in
+ * either direction.
+ *
+ * There's support for multiple serial channels within one virtio PCI
+ * device to keep the guest PCI device count low.
+ *
+ * Copyright (C) 2009, Red Hat, Inc.
+ *
+ * Author(s): Amit Shah <amit.shah@redhat.com>
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2.  See
+ * the COPYING file in the top-level directory.
+ */
+
+#include "hw.h"
+#include "pci.h"
+#include "monitor.h"
+#include "qemu-char.h"
+#include "virtio.h"
+#include "virtio-serial.h"
+#include "sysemu.h"
+
+typedef struct VirtIOSerial {
+    VirtIODevice *vdev;
+    VirtQueue *cvq;
+    struct VirtIOSerialPort *ports;
+} VirtIOSerial;
+
+typedef struct VirtIOSerialPort {
+    VirtIOSerial *virtserial;
+    VirtQueue *ivq, *ovq;
+    CharDriverState *hd;
+    char name[VIRTIO_SERIAL_NAME_MAX_LEN];
+} VirtIOSerialPort;
+
+VirtIOSerial virtio_serial;
+
+static char *get_port_name_from_idx(uint8_t idx)
+{
+    if (idx > virtio_serial_index)
+        return NULL;
+
+    return virtio_serial.ports[idx].name;
+}
+
+static VirtIOSerialPort *get_port_from_vq(VirtQueue *vq)
+{
+    int i;
+
+    for (i = 0; i < virtio_serial_index; i++) {
+        if (vq == virtio_serial.ports[i].ivq
+            || vq == virtio_serial.ports[i].ovq) {
+            return &virtio_serial.ports[i];
+        }
+    }
+    return NULL;
+}
+
+static void virtio_serial_handle_control(VirtIODevice *vdev, VirtQueue *vq)
+{
+    VirtQueueElement elem;
+
+    while (virtqueue_pop(vq, &elem)) {
+        int i;
+        char *name;
+        uint8_t idx;
+        uint32_t key;
+        ssize_t len, strlen;
+
+        len = 0;
+        for (i = 0; i < elem.out_num; i++) {
+            memcpy(&key, elem.out_sg[i].iov_base, 4);
+
+            switch(key) {
+            case VIRTIO_SERIAL_GET_PORT_NAME:
+                memcpy(&idx, elem.out_sg[i].iov_base + 4, 1);
+                name = get_port_name_from_idx(idx);
+
+                strlen = strnlen(name, VIRTIO_SERIAL_NAME_MAX_LEN);
+
+                memcpy(elem.in_sg[0].iov_base, &key, 4);
+                memcpy(elem.in_sg[0].iov_base + 4, &idx, 1);
+                memcpy(elem.in_sg[0].iov_base + 5, name, strlen);
+                len = 5 + strlen;
+                break;
+            }
+        }
+        virtqueue_push(vq, &elem, len);
+    }
+    virtio_notify(vdev, vq);
+
+    return;
+}
+
+static void virtio_serial_handle_output(VirtIODevice *vdev, VirtQueue *vq)
+{
+    VirtIOSerialPort *port = get_port_from_vq(vq);
+    VirtQueueElement elem;
+
+    while (virtqueue_pop(vq, &elem)) {
+        ssize_t len = 0;
+        int i;
+
+        if (port->hd) {
+            for (i = 0; i < elem.out_num; i++) {
+                len += qemu_chr_write(port->hd, elem.out_sg[i].iov_base,
+                                      elem.out_sg[i].iov_len);
+            }
+        }
+        virtqueue_push(vq, &elem, len);
+    }
+    virtio_notify(vdev, vq);
+}
+
+static void virtio_serial_handle_input(VirtIODevice *vdev, VirtQueue *vq)
+{
+}
+
+
+/* FIXME: we just accept a single string */
+void virtio_serial_send_input(const char *command, const char *key,
+			      const char *value)
+{
+    VirtQueueElement elem;
+    VirtIOSerialPort *s = &virtio_serial.ports[0];
+    VirtIODevice *vdev = s->virtserial->vdev;
+    VirtQueue *vq = s->ivq;
+    char buf[300];
+    ssize_t len;
+    int ret;
+    unsigned int i;
+
+    if (!virtio_queue_ready(s->ivq)) {
+        goto queue_not_ready;
+    }
+
+    len = snprintf(buf, 299, "%s %s %s\n", command, key, value);
+
+    /* FIXME! actually handle this in a for loop */
+
+    ret = virtqueue_pop(vq, &elem);
+    if (!ret) {
+        goto queue_not_ready;
+    }
+
+    i = 0;
+    /* Note: We only have PAGE_SIZE sized buffers */
+    memcpy(elem.in_sg[i].iov_base, buf, len);
+    elem.in_sg[i].iov_len = len;
+
+    virtqueue_push(vq, &elem, len);
+    virtio_notify(vdev, vq);
+    return;
+
+queue_not_ready:
+    monitor_printf(cur_mon,
+                   "vmserial: No free virtio buffer found. Message not sent.\n");
+    return;
+}
+
+static int cons_can_read(void *opaque)
+{
+    VirtIOSerialPort *port = (VirtIOSerialPort *) opaque;
+
+    if (!virtio_queue_ready(port->ivq)) {
+        return 0;
+    }
+
+    /* current implementations have a page sized buffer.
+     * We fall back to a one byte per read if there is not enough room.
+     */
+    if (virtqueue_avail_bytes(port->ivq, TARGET_PAGE_SIZE, 0)) {
+        return TARGET_PAGE_SIZE;
+    }
+    if (virtqueue_avail_bytes(port->ivq, 1, 0)) {
+        return 1;
+    }
+    return 0;
+}
+
+static void cons_read(void *opaque, const uint8_t *buf, int size)
+{
+    VirtIOSerialPort *port = (VirtIOSerialPort *) opaque;
+    VirtQueueElement elem;
+    int offset = 0;
+
+    /* The current kernel implementation has only one outstanding input
+     * buffer of PAGE_SIZE. Nevertheless, this function is prepared to
+     * handle multiple buffers with multiple sg element for input
+     */
+    while (offset < size) {
+        int i = 0;
+
+        if (!virtqueue_pop(port->ivq, &elem)) {
+            break;
+        }
+        while (offset < size && i < elem.in_num) {
+            int len = MIN(elem.in_sg[i].iov_len, size - offset);
+            memcpy(elem.in_sg[i].iov_base, buf + offset, len);
+            offset += len;
+            i++;
+        }
+        virtqueue_push(port->ivq, &elem, size);
+    }
+    virtio_notify(port->virtserial->vdev, port->ivq);
+}
+
+static void cons_event(void *opaque, int event)
+{
+    /* we will ignore any event for the time being */
+}
+
+static uint32_t virtio_serial_get_features(VirtIODevice *vdev)
+{
+    return 0;
+}
+
+static void virtio_serial_get_config(VirtIODevice *vdev, uint8_t *config_data)
+{
+    struct virtio_serial_config config;
+
+    /* This might have to be updated for serial port hotplug */
+    config.nr_ports = virtio_serial_index;
+    config.status = 0;
+
+    memcpy(config_data, &config, sizeof(config));
+}
+
+static void virtio_serial_set_config(VirtIODevice *vdev,
+                                     const uint8_t *config_data)
+{
+    struct virtio_serial_config config;
+
+    memcpy(&config, config_data, sizeof(config));
+
+    /* Nothing to do as of now */
+}
+
+static void virtio_serial_save(QEMUFile *f, void *opaque)
+{
+    VirtIODevice *vdev = opaque;
+
+    virtio_save(vdev, f);
+}
+
+static int virtio_serial_load(QEMUFile *f, void *opaque, int version_id)
+{
+    VirtIODevice *vdev = opaque;
+
+    if (version_id != 1)
+        return -EINVAL;
+
+    virtio_load(vdev, f);
+    return 0;
+}
+
+void *virtio_serial_new_port(PCIDevice *dev, int idx, char *name)
+{
+    VirtIOSerialPort *port;
+
+    port = &virtio_serial.ports[idx];
+
+    port->virtserial = &virtio_serial;
+
+    memcpy(port->name, name, VIRTIO_SERIAL_NAME_MAX_LEN);
+
+    port->hd = qdev_init_chardev(&dev->qdev);
+    if (port->hd) {
+        qemu_chr_add_handlers(port->hd, cons_can_read, cons_read, cons_event,
+                              port);
+    }
+
+    /* Add queue for host to guest transfers */
+    port->ivq = virtio_add_queue(port->virtserial->vdev, 2,
+                                 virtio_serial_handle_input);
+    /* Add queue for guest to host transfers */
+    port->ovq = virtio_add_queue(port->virtserial->vdev, 2,
+                                 virtio_serial_handle_output);
+
+    /* Send an update to the guest about this new port added */
+    virtio_notify_config(port->virtserial->vdev);
+    return port;
+}
+
+VirtIODevice *virtio_serial_init(DeviceState *dev)
+{
+    VirtIODevice *vdev;
+    int nr_ports = 4;
+
+    vdev = virtio_common_init("virtio-serial",
+			      VIRTIO_ID_SERIAL,
+			      sizeof(struct virtio_serial_config),
+			      sizeof(VirtIODevice));
+    if (vdev == NULL)
+        return NULL;
+
+    virtio_serial.vdev = vdev;
+    vdev->get_config = virtio_serial_get_config;
+    vdev->set_config = virtio_serial_set_config;
+    vdev->get_features = virtio_serial_get_features;
+
+    /* Add a queue for control information transfer common to all
+     * serial ports
+     */
+    virtio_serial.cvq = virtio_add_queue(vdev, 2, virtio_serial_handle_control);
+
+    /* Allocate space for the number of serial ports specified on the
+     * command line
+     */
+    virtio_serial.ports = qemu_mallocz(sizeof(VirtIOSerialPort) * nr_ports);
+
+    register_savevm("virtio-serial", -1, 1, virtio_serial_save,
+                    virtio_serial_load, vdev);
+
+    return vdev;
+}
diff --git a/hw/virtio-serial.h b/hw/virtio-serial.h
new file mode 100644
index 0000000..7d46c1f
--- /dev/null
+++ b/hw/virtio-serial.h
@@ -0,0 +1,31 @@
+/*
+ * Virtio Serial Support
+ *
+ * Copyright (C) 2009, Red Hat, Inc.
+ *
+ * Author(s): Amit Shah <amit.shah@redhat.com>
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2.  See
+ * the COPYING file in the top-level directory.
+ *
+ */
+#ifndef _QEMU_VIRTIO_SERIAL_H
+#define _QEMU_VIRTIO_SERIAL_H
+
+/* The ID for virtio serial */
+#define VIRTIO_ID_SERIAL 7
+
+struct virtio_serial_config
+{
+    uint8_t nr_ports;
+    uint16_t status;
+} __attribute__((packed));
+
+#define VIRTIO_SERIAL_GET_PORT_NAME 1
+
+
+void *virtio_serial_new_port(PCIDevice *dev, int idx, char *name);
+void virtio_serial_send_input(const char *command,
+			      const char *key, const char *value);
+
+#endif
diff --git a/hw/virtio.h b/hw/virtio.h
index 425727e..d2b50c3 100644
--- a/hw/virtio.h
+++ b/hw/virtio.h
@@ -151,5 +151,6 @@ VirtIODevice *virtio_blk_init(DeviceState *dev);
 VirtIODevice *virtio_net_init(DeviceState *dev);
 VirtIODevice *virtio_console_init(DeviceState *dev);
 VirtIODevice *virtio_balloon_init(DeviceState *dev);
+VirtIODevice *virtio_serial_init(DeviceState *dev);
 
 #endif
diff --git a/qemu-options.hx b/qemu-options.hx
index 87af798..857d8f1 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -1523,6 +1523,14 @@ STEXI
 Set virtio console.
 ETEXI
 
+DEF("virtioserial", HAS_ARG, QEMU_OPTION_virtioserial, \
+    "-virtioserial c\n" \
+    "                define virtio serial device\n")
+STEXI
+@item -virtserial @var{c}
+Set virtio serial device.
+ETEXI
+
 DEF("show-cursor", 0, QEMU_OPTION_show_cursor, \
     "-show-cursor    show cursor\n")
 STEXI
diff --git a/sysemu.h b/sysemu.h
index 658aeec..4f2b5af 100644
--- a/sysemu.h
+++ b/sysemu.h
@@ -229,6 +229,17 @@ extern CharDriverState *parallel_hds[MAX_PARALLEL_PORTS];
 
 extern CharDriverState *virtcon_hds[MAX_VIRTIO_CONSOLES];
 
+/* virtio serial ports */
+
+#define MAX_VIRTIO_SERIAL_PORTS 4
+#define VIRTIO_SERIAL_NAME_MAX_LEN 30
+
+
+extern CharDriverState *virtio_serial_hds[MAX_VIRTIO_SERIAL_PORTS];
+extern char virtio_serial_names[MAX_VIRTIO_SERIAL_PORTS][VIRTIO_SERIAL_NAME_MAX_LEN];
+extern int virtio_serial_index;
+
+
 #define TFR(expr) do { if ((expr) != -1) break; } while (errno == EINTR)
 
 #ifdef NEED_CPU_H
diff --git a/vl.c b/vl.c
index fcf8532..4d31281 100644
--- a/vl.c
+++ b/vl.c
@@ -234,6 +234,9 @@ int no_quit = 0;
 CharDriverState *serial_hds[MAX_SERIAL_PORTS];
 CharDriverState *parallel_hds[MAX_PARALLEL_PORTS];
 CharDriverState *virtcon_hds[MAX_VIRTIO_CONSOLES];
+CharDriverState *virtio_serial_hds[MAX_VIRTIO_SERIAL_PORTS];
+char virtio_serial_names[MAX_VIRTIO_SERIAL_PORTS][VIRTIO_SERIAL_NAME_MAX_LEN];
+int virtio_serial_index;
 #ifdef TARGET_I386
 int win2k_install_hack = 0;
 int rtc_td_hack = 0;
@@ -4945,6 +4948,7 @@ int main(int argc, char **argv, char **envp)
     int parallel_device_index;
     const char *virtio_consoles[MAX_VIRTIO_CONSOLES];
     int virtio_console_index;
+    const char *virtio_serials[MAX_VIRTIO_SERIAL_PORTS];
     const char *loadvm = NULL;
     QEMUMachine *machine;
     const char *cpu_model;
@@ -5024,6 +5028,10 @@ int main(int argc, char **argv, char **envp)
         virtio_consoles[i] = NULL;
     virtio_console_index = 0;
 
+    for (i = 0; i < MAX_VIRTIO_SERIAL_PORTS; i++)
+        virtio_serials[i] = NULL;
+    virtio_serial_index = 0;
+
     for (i = 0; i < MAX_NODES; i++) {
         node_mem[i] = 0;
         node_cpumask[i] = 0;
@@ -5453,6 +5461,14 @@ int main(int argc, char **argv, char **envp)
                 virtio_consoles[virtio_console_index] = optarg;
                 virtio_console_index++;
                 break;
+	    case QEMU_OPTION_virtioserial:
+                if (virtio_serial_index >= MAX_VIRTIO_SERIAL_PORTS) {
+                    fprintf(stderr, "qemu: too many virtio serial ports\n");
+                    exit(1);
+                }
+                virtio_serials[virtio_serial_index] = optarg;
+                virtio_serial_index++;
+                break;
             case QEMU_OPTION_parallel:
                 if (parallel_device_index >= MAX_PARALLEL_PORTS) {
                     fprintf(stderr, "qemu: too many parallel ports\n");
@@ -6038,6 +6054,43 @@ int main(int argc, char **argv, char **envp)
         }
     }
 
+    for (i = 0; i < virtio_serial_index; i++) {
+        const char *virtseropt;
+        char devname[80];
+        int j, k;
+
+        memset(devname, 0, 80);
+        j = k = 0;
+        while (isalnum(virtio_serials[i][j])) {
+            devname[k] = virtio_serials[i][j];
+            k++;
+            j++;
+        }
+
+        if (devname[0] && strncmp(devname, "none", 4)) {
+            char label[32];
+            snprintf(label, sizeof(label), "virtio-serial%d", i);
+            virtio_serial_hds[i] = qemu_chr_open(label, devname, NULL);
+            if (!virtio_serial_hds[i]) {
+                fprintf(stderr, "qemu: could not open virtio serial '%s'\n",
+                        devname);
+                exit(1);
+            }
+        }
+        virtseropt = strstr(virtio_serials[i], ",name=");
+        if (virtseropt) {
+            int j, k = 6;
+
+            for (j = 0; j < VIRTIO_SERIAL_NAME_MAX_LEN && isalnum(virtseropt[k]);
+                 j++, k++) {
+                virtio_serial_names[i][j] = virtseropt[k];
+            }
+            if (j < VIRTIO_SERIAL_NAME_MAX_LEN - 1) {
+                virtio_serial_names[i][j + 1] = 0;
+            }
+        }
+    }
+
     module_call_init(MODULE_INIT_DEVICE);
 
     machine->init(ram_size, boot_devices,
@@ -6172,6 +6225,15 @@ int main(int argc, char **argv, char **envp)
         }
     }
 
+    for(i = 0; i < MAX_VIRTIO_SERIAL_PORTS; i++) {
+        const char *devname = virtio_serials[i];
+        if (virtio_serial_hds[i] && devname) {
+            if (strstart(devname, "vc", 0)) {
+                qemu_chr_printf(virtio_serial_hds[i], "virtio serial%d\r\n", i);
+            }
+        }
+    }
+
     if (gdbstub_dev && gdbserver_start(gdbstub_dev) < 0) {
         fprintf(stderr, "qemu: could not open gdbserver on device '%s'\n",
                 gdbstub_dev);
-- 
1.6.0.6

[Qemu-devel] Re: [PATCH] virtio-serial: PCI device for simple host <-> guest communication

[Jan Kiszka]

Amit Shah wrote:
> This interface presents a char device from which bits can be
> sent and read.
> 
> Sample uses for such a device can be obtaining info from the
> guest like the file systems used, apps installed, etc. for
> offline usage and logged-in users, clipboard copy-paste, etc.
> for online usage.

Out of curiosity: What is the advantage over instantiating a multiport
16550A PCI card? Will those channels be so performance critical?

Jan

-- 
Siemens AG, Corporate Technology, CT SE 2
Corporate Competence Center Embedded Linux

Re: [Qemu-devel] Re: [PATCH] virtio-serial: PCI device for simple host <-> guest communication

[Richard W.M. Jones]

On Tue, Jun 09, 2009 at 07:15:25PM +0200, Jan Kiszka wrote:
> Amit Shah wrote:
> > This interface presents a char device from which bits can be
> > sent and read.
> > 
> > Sample uses for such a device can be obtaining info from the
> > guest like the file systems used, apps installed, etc. for
> > offline usage and logged-in users, clipboard copy-paste, etc.
> > for online usage.
> 
> Out of curiosity: What is the advantage over instantiating a multiport
> 16550A PCI card? Will those channels be so performance critical?

16550A has a couple of problems: The fixed sized FIFO (16 bytes) can
cause problems if you send data too quickly.  (I've seen this problem
when sending too much data from host to the guest, but I can't find
the bug report right now ...).  Secondly UARTs are complicated to
emulate compared to a clean virtio device.  Would we be happy with the
address space taken up and amount of emulation going on once we had,
say, 16 emulated ports?

On the other hand, multiport cards don't require special drivers in
the guest ...

Rich.

-- 
Richard Jones, Emerging Technologies, Red Hat  http://et.redhat.com/~rjones
Read my programming blog: http://rwmj.wordpress.com
Fedora now supports 75 OCaml packages (the OPEN alternative to F#)
http://cocan.org/getting_started_with_ocaml_on_red_hat_and_fedora

[Qemu-devel] Re: [PATCH] virtio-serial: PCI device for simple host <-> guest communication

[Jan Kiszka]

[-- Attachment #1: Type: text/plain, Size: 1604 bytes --]

Richard W.M. Jones wrote:
> On Tue, Jun 09, 2009 at 07:15:25PM +0200, Jan Kiszka wrote:
>> Amit Shah wrote:
>>> This interface presents a char device from which bits can be
>>> sent and read.
>>>
>>> Sample uses for such a device can be obtaining info from the
>>> guest like the file systems used, apps installed, etc. for
>>> offline usage and logged-in users, clipboard copy-paste, etc.
>>> for online usage.
>> Out of curiosity: What is the advantage over instantiating a multiport
>> 16550A PCI card? Will those channels be so performance critical?
> 
> 16550A has a couple of problems: The fixed sized FIFO (16 bytes) can
> cause problems if you send data too quickly.  (I've seen this problem
> when sending too much data from host to the guest, but I can't find
> the bug report right now ...).

If you buy a real 16550A card with high-speed support (eg. 1 MBit/s),
you get larger FIFOs (eg. 256 byte). Thresholds are scaled
correspondingly by such cards. So there would be no problem in extending
this in QEMU as well, ie. making it a parameter to the init function.

>  Secondly UARTs are complicated to
> emulate compared to a clean virtio device.  Would we be happy with the
> address space taken up and amount of emulation going on once we had,
> say, 16 emulated ports?

Memory-mapped 16550As do not suffer from such a limitation.

> 
> On the other hand, multiport cards don't require special drivers in
> the guest ...

That's what raised my interest: What benefit of a virtio-serial can
over-compensate this advantage of standard hardware?

Jan



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 257 bytes --]

Re: [Qemu-devel] [PATCH] virtio-serial: PCI device for simple host <-> guest communication

[Richard W.M. Jones]

On Tue, Jun 09, 2009 at 10:12:48PM +0530, Amit Shah wrote:
[...]

Are you going to post the virtio driver for the Linux kernel later?

> +DEF("virtioserial", HAS_ARG, QEMU_OPTION_virtioserial, \
> +    "-virtioserial c\n" \
> +    "                define virtio serial device\n")

Can it be made to look like a real serial port, ie.
'-serial dev,model=virtio' ?

Rich.

-- 
Richard Jones, Emerging Technologies, Red Hat  http://et.redhat.com/~rjones
virt-p2v converts physical machines to virtual machines.  Boot with a
live CD or over the network (PXE) and turn machines into Xen guests.
http://et.redhat.com/~rjones/virt-p2v

Re: [Qemu-devel] [PATCH] virtio-serial: PCI device for simple host <-> guest communication

[Stuart Brady]

On Tue, Jun 09, 2009 at 07:17:14PM +0100, Richard W.M. Jones wrote:
> Can it be made to look like a real serial port, ie.
> '-serial dev,model=virtio' ?

A rather pedantic point, perhaps, but this doesn't really look like a
serial device to me (in the same way that it doesn't look like a
parallel device, either).

But then I might as well argue that flash-based storage devices aren't 
"disks", etc., etc... :-)

Cheers,
-- 
Stuart Brady

Re: [Qemu-devel] [PATCH] virtio-serial: PCI device for simple host <-> guest communication

[Anthony Liguori]

Richard W.M. Jones wrote:
> On Tue, Jun 09, 2009 at 10:12:48PM +0530, Amit Shah wrote:
> [...]
>
> Are you going to post the virtio driver for the Linux kernel later?
>   

Yeah, I'd like to see the Linux driver before drawing conclusions.

Plus, having a virtio serial device and a virtio console seems 
unfortunate to me.  Anyway we can unify the two?  Maybe make virtio 
serial support hvc with a feature flag or something instead of ttyS?

Regards,

Anthony Liguori

Re: [Qemu-devel] [PATCH] virtio-serial: PCI device for simple host <-> guest communication

[Paul Brook]

> Plus, having a virtio serial device and a virtio console seems
> unfortunate to me.  Anyway we can unify the two?  Maybe make virtio
> serial support hvc with a feature flag or something instead of ttyS?

I agree. virtio-console and virtio-serial seem to be duplicating the exact 
same functionality. If you really are that short of PCI devices then I suggest 
you enhance virtio-console to support multiple channels.

Paul

Re: [Qemu-devel] [PATCH] virtio-serial: PCI device for simple host <-> guest communication

[Amit Shah]

On (Tue) Jun 09 2009 [13:56:57], Anthony Liguori wrote:
> Richard W.M. Jones wrote:
>> On Tue, Jun 09, 2009 at 10:12:48PM +0530, Amit Shah wrote:
>> [...]
>>
>> Are you going to post the virtio driver for the Linux kernel later?
>>   
>
> Yeah, I'd like to see the Linux driver before drawing conclusions.

OK; will send in a while.

> Plus, having a virtio serial device and a virtio console seems  
> unfortunate to me.  Anyway we can unify the two?  Maybe make virtio  
> serial support hvc with a feature flag or something instead of ttyS?

Yes, it's possible. I'll look into that once I reach a point where
virtio-serial is ready to be merged. I'll also have to look at making
sure existing guest kernels work with the new, unified device.

		Amit

Re: [Qemu-devel] [PATCH] virtio-serial: PCI device for simple host <-> guest communication

[Jamie Lokier]

Amit Shah wrote:
> > Plus, having a virtio serial device and a virtio console seems  
> > unfortunate to me.  Anyway we can unify the two?  Maybe make virtio  
> > serial support hvc with a feature flag or something instead of ttyS?
> 
> Yes, it's possible. I'll look into that once I reach a point where
> virtio-serial is ready to be merged. I'll also have to look at making
> sure existing guest kernels work with the new, unified device.

One of the most useful things about a serial port is that it's so easy
for basic OS components like debuggers and bootloaders and BIOSes to
use it, with very little code, and without having to do very complex
things like initialise and enumerate PCI buses etc., and without
interference by an OS so they are useful for things like kernel
debugging and reliable debugging output.

For this reason, virtually every real machine has a simple serial
port, even if it isn't exposed on an external connector.

Are virtio-serial/virtio-console very simple to drive from guest code,
without PCI enumeration, or do they require complex initialisation and
OS cooperation?

-- Jamie

Re: [Qemu-devel] [PATCH] virtio-serial: PCI device for simple host <-> guest communication

[Amit Shah]

On (Wed) Jun 10 2009 [19:00:27], Jamie Lokier wrote:
> Amit Shah wrote:
> > > Plus, having a virtio serial device and a virtio console seems  
> > > unfortunate to me.  Anyway we can unify the two?  Maybe make virtio  
> > > serial support hvc with a feature flag or something instead of ttyS?
> > 
> > Yes, it's possible. I'll look into that once I reach a point where
> > virtio-serial is ready to be merged. I'll also have to look at making
> > sure existing guest kernels work with the new, unified device.
> 
> One of the most useful things about a serial port is that it's so easy
> for basic OS components like debuggers and bootloaders and BIOSes to
> use it, with very little code, and without having to do very complex
> things like initialise and enumerate PCI buses etc., and without
> interference by an OS so they are useful for things like kernel
> debugging and reliable debugging output.
> 
> For this reason, virtually every real machine has a simple serial
> port, even if it isn't exposed on an external connector.
> 
> Are virtio-serial/virtio-console very simple to drive from guest code,
> without PCI enumeration, or do they require complex initialisation and
> OS cooperation?

OK, great question. I'll post my Linux driver that'll answer your
question precisely, but this is what a guest userspace app will have to
do to use it:

        fd = open("/dev/vmser0", O_RDWR);
        if (fd == -1) {
                perror("open");
                return -errno;
        }

        ret = read(fd, buf, 100);
        if (ret < 0) {
                perror("read");
        }

        ret = ioctl(fd, VIRTIO_SERIAL_IOCTL_GET_PORT_NAME, &port_name);
        if (ret) perror("ioctl");
        printf("string is %s\n", port_name.name);

So the guest driver exposes a simple char device to userspace.

The driver itself hooks up with the virtio code which abstracts away all
the pci stuff. It just has to manage the rings and the queues.

		Amit

Re: [Qemu-devel] [PATCH] virtio-serial: PCI device for simple host <-> guest communication

[Jamie Lokier]

Amit Shah wrote:
> On (Wed) Jun 10 2009 [19:00:27], Jamie Lokier wrote:
> > Amit Shah wrote:
> > > > Plus, having a virtio serial device and a virtio console seems  
> > > > unfortunate to me.  Anyway we can unify the two?  Maybe make virtio  
> > > > serial support hvc with a feature flag or something instead of ttyS?
> > > 
> > > Yes, it's possible. I'll look into that once I reach a point where
> > > virtio-serial is ready to be merged. I'll also have to look at making
> > > sure existing guest kernels work with the new, unified device.
> > 
> > One of the most useful things about a serial port is that it's so easy
> > for basic OS components like debuggers and bootloaders and BIOSes to
> > use it, with very little code, and without having to do very complex
> > things like initialise and enumerate PCI buses etc., and without
> > interference by an OS so they are useful for things like kernel
> > debugging and reliable debugging output.
> > 
> > For this reason, virtually every real machine has a simple serial
> > port, even if it isn't exposed on an external connector.
> > 
> > Are virtio-serial/virtio-console very simple to drive from guest code,
> > without PCI enumeration, or do they require complex initialisation and
> > OS cooperation?
> 
> OK, great question. I'll post my Linux driver that'll answer your
> question precisely, but this is what a guest userspace app will have to
> do to use it:

Userspace doesn't matter so much - I'd assumed a simple char driver
carrying a pipe of bytes, no matter how the emulated hardware works.
But that's not relevant to the things I'm asking about, since they
run outside userspace, sometimes without a kernel.

> The driver itself hooks up with the virtio code which abstracts away all
> the pci stuff. It just has to manage the rings and the queues.

On the face of it, simply because virtio uses PCI, it looks a _lot_
more complicated than a serial port (UART) at a fixed I/O or MMIO
address, for things like boot loaders, boot ROMS, BIOS debugging,
kernel debuggers and minimal kernels.

I say on the face of it, because I don't know enough about how virtio
generally is set up in the guest.

Can you make virtio-serial start enabled with fixed, known I/O or MMIO
addresss, in a similar way to the way VGA cards and IDE ports are PCI
devices but can be used without PCI enumeration?

Can you make it pollable, so it's usable without having to route
interrupts, which can be quite complicated, or impossible when you're
debugging something that thinks it is control?

-- Jamie

Re: [Qemu-devel] [PATCH] virtio-serial: PCI device for simple host <-> guest communication

[Amit Shah]

On (Tue) Jun 09 2009 [13:56:57], Anthony Liguori wrote:
> Richard W.M. Jones wrote:
>> On Tue, Jun 09, 2009 at 10:12:48PM +0530, Amit Shah wrote:
>> [...]
>>
>> Are you going to post the virtio driver for the Linux kernel later?
>>   
>
> Yeah, I'd like to see the Linux driver before drawing conclusions.

Pasting here a slightly older version of the patch. I'm actively working
on it; a few things might have changed but the basic idea remains the
same.

		Amit


diff --git a/drivers/char/Kconfig b/drivers/char/Kconfig
index 735bbe2..a023346 100644
--- a/drivers/char/Kconfig
+++ b/drivers/char/Kconfig
@@ -666,6 +666,12 @@ config VIRTIO_CONSOLE
 	help
 	  Virtio console for use with lguest and other hypervisors.
 
+config VIRTIO_SERIAL
+	tristate "Virtio serial"
+	select VIRTIO
+	select VIRTIO_RING
+	help
+	  Virtio serial device driver for simple guest and host communication
 
 config HVCS
 	tristate "IBM Hypervisor Virtual Console Server support"
diff --git a/drivers/char/Makefile b/drivers/char/Makefile
index 9caf5b5..e5bee21 100644
--- a/drivers/char/Makefile
+++ b/drivers/char/Makefile
@@ -53,6 +53,7 @@ obj-$(CONFIG_HVC_XEN)		+= hvc_xen.o
 obj-$(CONFIG_HVC_IUCV)		+= hvc_iucv.o
 obj-$(CONFIG_HVC_UDBG)		+= hvc_udbg.o
 obj-$(CONFIG_VIRTIO_CONSOLE)	+= virtio_console.o
+obj-$(CONFIG_VIRTIO_SERIAL)	+= virtio_serial.o
 obj-$(CONFIG_RAW_DRIVER)	+= raw.o
 obj-$(CONFIG_SGI_SNSC)		+= snsc.o snsc_event.o
 obj-$(CONFIG_MSPEC)		+= mspec.o
diff --git a/drivers/char/virtio_serial.c b/drivers/char/virtio_serial.c
new file mode 100644
index 0000000..0a5e90a
--- /dev/null
+++ b/drivers/char/virtio_serial.c
@@ -0,0 +1,531 @@
+/*
+ * VirtIO Serial driver
+ *
+ * This is paravirtualised serial guest<->host communication channel
+ * for relaying short messages and events in either direction.
+ *
+ * One PCI device can have multiple serial ports so that different
+ * applications can communicate without polluting the PCI device space
+ * in the guest.
+ *
+ * Copyright (C) 2009, Red Hat, Inc.
+ *
+ * Author(s): Amit Shah <amit.shah@redhat.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307  USA
+ */
+
+#include <linux/cdev.h>
+#include <linux/err.h>
+#include <linux/fs.h>
+#include <linux/init.h>
+#include <linux/poll.h>
+#include <linux/virtio.h>
+#include <linux/virtio_serial.h>
+
+struct virtio_serial_struct {
+	struct virtio_device *vdev;
+	struct virtqueue *ctrl_vq;
+	struct virtio_serial_port *ports;
+	u8 nr_devs; /* number of devices we can open */
+};
+
+struct virtio_serial_port {
+	struct virtio_serial_struct *virtserial;
+	struct virtqueue *in_vq, *out_vq;
+
+	/* The name given to this channel, if any. NOT zero-terminated */
+	char *name;
+
+	/* Each port associates with a separate char device */
+	dev_t dev;
+	struct cdev cdev;
+
+#define RECV_BUFFERS_PER_PORT 1
+	char *pages[RECV_BUFFERS_PER_PORT];
+	struct scatterlist *recv_sg;
+	unsigned int read_len;
+	char *read_buf;
+};
+
+static struct virtio_serial_struct virtserial;
+
+/* This should become per-port data */
+static DECLARE_COMPLETION(have_data);
+
+static int major = 60; /* from the experimental range */
+
+static struct virtio_serial_port *get_port_from_idx(u8 idx)
+{
+	if (idx > virtserial.nr_devs || idx < 0)
+		return NULL;
+
+	return &virtserial.ports[idx];
+}
+
+static int get_id_from_vq(struct virtqueue *vq)
+{
+	int i;
+
+	for (i = 0; i < virtserial.nr_devs; i++) {
+		if (virtserial.ports[i].in_vq == vq
+		    || virtserial.ports[i].out_vq == vq)
+			return i;
+	}
+	return -1;
+}
+
+static struct virtio_serial_port *get_port_from_vq(struct virtqueue *vq)
+{
+	int idx;
+
+	idx = get_id_from_vq(vq);
+	if (idx < 0)
+		return NULL;
+	return &virtserial.ports[idx];
+}
+
+static void receive_data(struct virtqueue *vq)
+{
+	struct virtio_serial_port *port;
+
+	port = get_port_from_vq(vq);
+
+	/* We can get spurious callbacks, e.g. shared IRQs + virtio_pci. */
+	port->read_buf = vq->vq_ops->get_buf(vq, &port->read_len);
+	if (!port->read_buf)
+		return;
+
+	complete(&have_data);
+}
+
+static int receive_control_response(struct virtqueue *vq)
+{
+	int ret;
+	char *data;
+	uint8_t idx;
+	uint32_t key;
+	unsigned int len;
+	struct virtio_serial_port *port;
+
+	/* We can get spurious callbacks, e.g. shared IRQs + virtio_pci. */
+	data = vq->vq_ops->get_buf(vq, &len);
+	if (!data)
+		return 0;
+
+	memcpy(&key, data, 4);
+	ret = -EINVAL;
+	switch (key) {
+	case VIRTIO_SERIAL_GET_PORT_NAME:
+		if (len < 6) {
+			/* Not enough data for this ioctl:
+			 * key (4 bytes) + idx (1 byte) + port name
+			 */
+			/* XXX: return something else? */
+			ret = -EIO;
+			break;
+		}
+		memcpy(&idx, data + 4, 1);
+
+		port = get_port_from_idx(idx);
+		if (!port) {
+			ret = -EINVAL;
+			break;
+		}
+		len -= 5;
+		if (len > VIRTIO_SERIAL_NAME_MAX_LEN)
+			len = VIRTIO_SERIAL_NAME_MAX_LEN;
+
+		/* If the port is getting renamed */
+		kfree(port->name);
+
+		port->name = kzalloc(len, GFP_KERNEL);
+		if (!port->name) {
+			ret = -ENOMEM;
+			break;
+		}
+		memcpy(port->name, data + 5, len);
+		ret = len;
+		break;
+	}
+	return ret;
+}
+
+static int request_control_info(u32 key, u8 idx)
+{
+	int ret;
+	char *vbuf, *obuf;
+	struct scatterlist sg[2];
+
+	vbuf = kmalloc(PAGE_SIZE, GFP_KERNEL);
+	if (!vbuf)
+		return -ENOMEM;
+	obuf = kmalloc(PAGE_SIZE, GFP_KERNEL);
+	if (!obuf)
+		return -ENOMEM;
+
+	memcpy(vbuf, &key, 4);
+	memcpy(vbuf + 4, &idx, 1);
+
+	sg_init_table(sg, 2);
+	sg_set_buf(&sg[0], vbuf, PAGE_SIZE);
+	sg_set_buf(&sg[1], obuf, PAGE_SIZE);
+
+	if (virtserial.ctrl_vq->vq_ops->add_buf(virtserial.ctrl_vq, sg, 1, 1,
+						obuf) == 0) {
+		/* Tell Host to go! */
+		virtserial.ctrl_vq->vq_ops->kick(virtserial.ctrl_vq);
+
+		/* Chill out until it's done with the buffer. */
+		ret = receive_control_response(virtserial.ctrl_vq);
+		while (ret == 0) {
+			cpu_relax();
+			ret = receive_control_response(virtserial.ctrl_vq);
+		}
+	}
+	kfree(vbuf);
+	kfree(obuf);
+
+	return ret;
+}
+
+
+static ssize_t virtserial_read(struct file *filp, char __user *ubuf,
+			       size_t count, loff_t *offp)
+{
+	struct virtqueue *in_vq;
+	struct virtio_serial_port *port;
+	ssize_t ret;
+
+	port = filp->private_data;
+	in_vq = port->in_vq;
+
+	if (port->read_len == 0)
+		return 0;
+
+	if (count > port->read_len)
+		count = port->read_len;
+
+	ret = copy_to_user(ubuf, port->read_buf, count);
+
+	/* Return the number of bytes actually copied */
+	ret = count - ret;
+
+	port->read_buf += ret;
+	port->read_len -= ret;
+
+	if (!port->read_len) {
+		/* FIXME: Assuming only one page per port */
+		in_vq->vq_ops->add_buf(in_vq, port->recv_sg, 0, 1,
+				       port->pages[0]);
+		/* FIXME: it's a bug if add_buf didn't succeed */
+		in_vq->vq_ops->kick(in_vq);
+	}
+	return ret;
+}
+
+static ssize_t virtserial_write(struct file *filp, const char __user *ubuf,
+				size_t count, loff_t *offp)
+{
+	ssize_t ret;
+	char *vbuf;
+	unsigned int len, size;
+	struct virtqueue *out_vq;
+	struct scatterlist sg[1];
+	struct virtio_serial_port *port;
+
+	size = min(count, PAGE_SIZE);
+	vbuf = kmalloc(size, GFP_KERNEL);
+	if (!vbuf)
+		return -EFBIG;
+	ret = copy_from_user(vbuf, ubuf, size);
+
+	/* Return the number of bytes actually written */
+	ret = size - ret;
+
+	port = filp->private_data;
+	out_vq = port->out_vq;
+
+	sg_init_one(sg, vbuf, ret);
+
+	/* add_buf wants a token to identify this buffer: we hand it any
+	 * non-NULL pointer, since there's only ever one buffer.
+	 */
+	if (out_vq->vq_ops->add_buf(out_vq, sg, 1, 0, (void *)1) == 0) {
+		/* Tell Host to go! */
+		out_vq->vq_ops->kick(out_vq);
+		/* Chill out until it's done with the buffer. */
+		while (!out_vq->vq_ops->get_buf(out_vq, &len))
+			cpu_relax();
+	}
+	kfree(vbuf);
+
+	/* We're expected to return the amount of data we wrote */
+	return ret;
+}
+
+static long virtserial_ioctl(struct file *filp, unsigned int ioctl,
+			     unsigned long arg)
+{
+	int ret;
+	struct virtio_serial_port *port;
+	struct virtio_serial_port_name *port_name;
+
+	port = filp->private_data;
+
+	ret = -EINVAL;
+	switch (ioctl) {
+	case VIRTIO_SERIAL_IOCTL_GET_PORT_NAME:
+		port_name = (struct virtio_serial_port_name *)arg;
+
+		if (!port->name) {
+			ret = request_control_info(VIRTIO_SERIAL_GET_PORT_NAME,
+						   get_id_from_vq(port->in_vq));
+			if (ret < 0) {
+				/* Of IOCTL error return codes, only
+				 * this one comes close to what has
+				 * happened: either out of memory or
+				 * the virtio-serial backend didn't
+				 * have the associated name for the
+				 * port
+				 */
+				ret = -EFAULT;
+				break;
+			}
+		}
+		ret = copy_to_user(port_name->name,
+				   port->name, VIRTIO_SERIAL_NAME_MAX_LEN);
+		if (ret < 0) {
+			ret = -EINVAL;
+			break;
+		}
+		if (ret > 0) {
+			/* Do something? -- There is still data to be
+			 * copied to userspace
+			 */
+			ret = 0;
+		}
+		break;
+	}
+	return ret;
+}
+
+static int virtserial_release(struct inode *inode, struct file *filp)
+{
+	printk(KERN_NOTICE "%s\n", __func__);
+	return 0;
+}
+
+static int virtserial_open(struct inode *inode, struct file *filp)
+{
+	struct cdev *cdev = inode->i_cdev;
+	struct virtio_serial_port *port;
+
+	port = container_of(cdev, struct virtio_serial_port,
+			    cdev);
+
+	filp->private_data = port;
+	return 0;
+}
+
+static unsigned int virtserial_poll(struct file *filp, poll_table *wait)
+{
+	printk(KERN_NOTICE "%s\n", __func__);
+	return 0;
+}
+
+static const struct file_operations virtserial_fops = {
+	.owner = THIS_MODULE,
+	.open  = virtserial_open,
+	.read  = virtserial_read,
+	.write = virtserial_write,
+	.compat_ioctl = virtserial_ioctl,
+	.unlocked_ioctl = virtserial_ioctl,
+	.poll  = virtserial_poll,
+	.release = virtserial_release,
+};
+
+static int port_setup_recv_buffers(struct virtio_serial_port *port)
+{
+	int ret;
+	int i, nr_buffers = RECV_BUFFERS_PER_PORT;
+	struct virtqueue *vq = port->in_vq;
+
+	ret = -ENOMEM;
+	port->recv_sg = kzalloc(sizeof(struct scatterlist) * nr_buffers,
+				GFP_KERNEL);
+	if (!port->recv_sg)
+		return ret;
+
+	sg_init_table(port->recv_sg, nr_buffers);
+
+	for (i = 0; i < nr_buffers; i++) {
+		port->pages[i] = kzalloc(PAGE_SIZE, GFP_KERNEL);
+		if (!port->pages[i])
+			goto free_pages;
+
+		sg_set_buf(&port->recv_sg[i], port->pages[i], PAGE_SIZE);
+		ret = vq->vq_ops->add_buf(vq, port->recv_sg, 0, 1,
+					  port->pages[i]);
+		if (ret < 0)
+			goto free_pages;
+	}
+	vq->vq_ops->kick(vq);
+
+	return 0;
+
+free_pages:
+	for (; i >= 0; i--)
+		kfree(port->pages[i]);
+	kfree(port->recv_sg);
+
+	return ret;
+}
+
+static int virtserial_probe(struct virtio_device *vdev)
+{
+	int i, ret;
+	struct virtio_serial_port *port;
+	struct virtio_serial_config virtsercfg;
+
+	vdev->config->get(vdev, offsetof(struct virtio_serial_config, nr_ports),
+			  &virtsercfg.nr_ports, sizeof(virtsercfg.nr_ports));
+
+	virtserial.vdev = vdev;
+	virtserial.nr_devs = virtsercfg.nr_ports;
+	virtserial.ports = kzalloc(sizeof(struct virtio_serial_port)
+				   * virtserial.nr_devs, GFP_KERNEL);
+	if (!virtserial.ports) {
+		ret = -ENOMEM;
+		goto fail;
+	}
+
+	virtserial.ctrl_vq = vdev->config->find_vq(vdev, 0, NULL);
+	for (i = 0; i < virtserial.nr_devs; i++) {
+		port = &virtserial.ports[i];
+
+		port->in_vq = vdev->config->find_vq(vdev, (i * 2) + 1,
+						    receive_data);
+		if (IS_ERR(port->in_vq)) {
+			ret = PTR_ERR(port->in_vq);
+			goto fail;
+		}
+		port->out_vq = vdev->config->find_vq(vdev, (i * 2) + 2, NULL);
+		if (IS_ERR(port->out_vq)) {
+			ret = PTR_ERR(port->out_vq);
+			goto free_in_vq;
+		}
+		cdev_init(&port->cdev, &virtserial_fops);
+		port->dev = MKDEV(major, i);
+
+		/* XXX Do this for each port or do it once? */
+		ret = register_chrdev_region(port->dev, 1, "virtio-serial");
+		if (ret < 0) {
+			printk(KERN_ERR "%s: can't register chrdev region\n",
+			       __func__);
+			goto free_out_vq;
+		}
+		ret = cdev_add(&port->cdev, port->dev, 1);
+		if (ret < 0) {
+			printk(KERN_ERR "%s: can't add cdev\n", __func__);
+			goto free_cdev_region;
+		}
+
+		/* Finally set up some receive buffers */
+		ret = port_setup_recv_buffers(port);
+		if (ret < 0)
+			printk(KERN_NOTICE "%s: err for setup_recv_buff\n", __func__);
+	}
+	return 0;
+
+/* Rehaul this */
+free_cdev_region:
+	unregister_chrdev_region(port->dev, 1);
+free_out_vq:
+	vdev->config->del_vq(port->out_vq);
+free_in_vq:
+	vdev->config->del_vq(port->in_vq);
+fail:
+	return ret;
+}
+
+
+static void virtserial_remove(struct virtio_device *vdev)
+{
+	int i, j;
+
+	vdev->config->del_vq(virtserial.ctrl_vq);
+
+	for (i = 0; i < virtserial.nr_devs; i++) {
+		struct virtio_serial_port *port;
+
+		port = &virtserial.ports[i];
+		if (port->dev) {
+			/* FIXME: Take a long, hard look at this */
+			unregister_chrdev_region(port->dev, 1);
+			cdev_del(&port->cdev);
+
+			vdev->config->del_vq(port->out_vq);
+			vdev->config->del_vq(port->in_vq);
+
+			kfree(port->name);
+			kfree(port->recv_sg);
+			for (j = 0; j < RECV_BUFFERS_PER_PORT; j++)
+				kfree(port->pages[j]);
+		}
+	}
+}
+
+static void virtserial_apply_config(struct virtio_device *vdev)
+{
+	struct virtio_serial_config virtserconf;
+
+	vdev->config->get(vdev, offsetof(struct virtio_serial_config, nr_ports),
+			  &virtserconf.nr_ports, sizeof(virtserconf.nr_ports));
+	printk(KERN_NOTICE "%s: %u ports found\n", __func__, virtserconf.nr_ports);
+}
+
+
+static struct virtio_device_id id_table[] = {
+	{ VIRTIO_ID_SERIAL, VIRTIO_DEV_ANY_ID },
+	{ 0 },
+};
+
+static struct virtio_driver virtio_serial = {
+  //	.feature_table = features,
+  //	.feature_table_size = ARRAY_SIZE(features),
+	.driver.name =	KBUILD_MODNAME,
+	.driver.owner =	THIS_MODULE,
+	.id_table =	id_table,
+	.probe =	virtserial_probe,
+	.remove =	virtserial_remove,
+	.config_changed = virtserial_apply_config,
+};
+
+static int __init init(void)
+{
+	return register_virtio_driver(&virtio_serial);
+}
+
+static void __exit fini(void)
+{
+	unregister_virtio_driver(&virtio_serial);
+}
+module_init(init);
+module_exit(fini);
+
+MODULE_DEVICE_TABLE(virtio, id_table);
+MODULE_DESCRIPTION("Virtio serial driver");
+MODULE_LICENSE("GPL");
diff --git a/include/linux/virtio_serial.h b/include/linux/virtio_serial.h
new file mode 100644
index 0000000..5528b11
--- /dev/null
+++ b/include/linux/virtio_serial.h
@@ -0,0 +1,34 @@
+#ifndef _LINUX_VIRTIO_SERIAL_H
+#define _LINUX_VIRTIO_SERIAL_H
+#include <linux/types.h>
+#include <linux/virtio_config.h>
+
+/* Guest kernel - Host interface */
+
+/* The ID for virtio serial */
+#define VIRTIO_ID_SERIAL	7
+
+struct virtio_serial_config {
+	__u8 nr_ports;
+	__u16 status;
+} __attribute__((packed));
+
+
+#define VIRTIO_SERIAL_NAME_MAX_LEN 30
+
+/* Some defines for the control channel key */
+#define VIRTIO_SERIAL_GET_PORT_NAME 1
+
+/* Guest kernel - Guest userspace interface */
+
+/* IOCTL-related */
+#define VIRTIO_SERIAL_IO 0xAF
+
+struct virtio_serial_port_name {
+	char name[VIRTIO_SERIAL_NAME_MAX_LEN];
+};
+
+#define VIRTIO_SERIAL_IOCTL_GET_PORT_NAME	_IOWR(VIRTIO_SERIAL_IO, 0x00, \
+			    struct virtio_serial_port_name)
+
+#endif /* _LINUX_VIRTIO_SERIAL_H */

Re: [Qemu-devel] [PATCH] virtio-serial: PCI device for simple host <-> guest communication

[Blue Swirl]

On 6/11/09, Amit Shah <amit.shah@redhat.com> wrote:
> On (Tue) Jun 09 2009 [13:56:57], Anthony Liguori wrote:
>  > Richard W.M. Jones wrote:
>  >> On Tue, Jun 09, 2009 at 10:12:48PM +0530, Amit Shah wrote:
>  >> [...]
>  >>
>  >> Are you going to post the virtio driver for the Linux kernel later?
>  >>
>  >
>  > Yeah, I'd like to see the Linux driver before drawing conclusions.
>
>  Pasting here a slightly older version of the patch. I'm actively working
>  on it; a few things might have changed but the basic idea remains the
>  same.

>  + * along with this program; if not, write to the Free Software
>  + * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307  USA

Old FSF address, the new one is on Franklin St.

>  +struct virtio_serial_port {

>  +       unsigned int read_len;

This limits the maximum transfer length to 4G, which is way too short
on 64 bit hosts. ;-)

>  +struct virtio_serial_config {
>  +       __u8 nr_ports;
>  +       __u16 status;
>  +} __attribute__((packed));

The structure should be redesigned to work without packed attribute.
status field is unaligned.

Re: [Qemu-devel] [PATCH] virtio-serial: PCI device for simple host <-> guest communication

[Amit Shah]

On (Thu) Jun 11 2009 [19:07:26], Blue Swirl wrote:
> On 6/11/09, Amit Shah <amit.shah@redhat.com> wrote:
> > On (Tue) Jun 09 2009 [13:56:57], Anthony Liguori wrote:
> >  > Richard W.M. Jones wrote:
> >  >> On Tue, Jun 09, 2009 at 10:12:48PM +0530, Amit Shah wrote:
> >  >> [...]
> >  >>
> >  >> Are you going to post the virtio driver for the Linux kernel later?
> >  >>
> >  >
> >  > Yeah, I'd like to see the Linux driver before drawing conclusions.
> >
> >  Pasting here a slightly older version of the patch. I'm actively working
> >  on it; a few things might have changed but the basic idea remains the
> >  same.
> 
> >  + * along with this program; if not, write to the Free Software
> >  + * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307  USA
> 
> Old FSF address, the new one is on Franklin St.

OK, will update.

> >  +struct virtio_serial_port {
> 
> >  +       unsigned int read_len;
> 
> This limits the maximum transfer length to 4G, which is way too short
> on 64 bit hosts. ;-)

This is only valid for a single packet that's transmitted via the queue
at a time and I doubt anyone's going to use bigger packets :-)

(That size is also limited by the kernel -- and the current code only
alloctes one PAGE_SIZE buffer for the transfer.)

> >  +struct virtio_serial_config {
> >  +       __u8 nr_ports;
> >  +       __u16 status;
> >  +} __attribute__((packed));
> 
> The structure should be redesigned to work without packed attribute.
> status field is unaligned.

I'll see how to untangle it.

		Amit

Re: [Qemu-devel] [PATCH] virtio-serial: PCI device for simple host <-> guest communication

[Amit Shah]

On (Tue) Jun 09 2009 [19:17:14], Richard W.M. Jones wrote:
> On Tue, Jun 09, 2009 at 10:12:48PM +0530, Amit Shah wrote:
> [...]
> 
> Are you going to post the virtio driver for the Linux kernel later?

A few cleanups needed. I'll get to that right now.

> > +DEF("virtioserial", HAS_ARG, QEMU_OPTION_virtioserial, \
> > +    "-virtioserial c\n" \
> > +    "                define virtio serial device\n")
> 
> Can it be made to look like a real serial port, ie.
> '-serial dev,model=virtio' ?

Yes, this part is still in flux: there are a few more parameters that
are going to be passed (like the name for the port). Also, I'm planning
to add an API within qemu for data transfer over serial ports and a
key-value pair will be suitable for that. So that'll require a separate
parameter as well. This API will be useful to support migration better
and also qemu to converse with the guest.

So extending the -serial option is certainly possible but I've to make
sure the virtio serial-specific options don't apply to the others. I've
not gotten to look into that yet.

		Amit