2009-11-03 16:39:54

by Amit Shah

[permalink] [raw]
Subject: [PATCH v10 0/1] virtio-console: Support for generic ports and multiple consoles

(Rusty, would it improve the chances of getting this patch in your
tree if this description were written with latex instead of without
it?)

Here is a new iteration of the patch series that implements a
transport for guest and host communications.

I've tested for compatibility (old qemu & new kernel, new qemu & old
kernel, new qemu & new kernel) and it all works fine. To maintain
backward compatibility, the first port to be spawned (port at id 0) is
to be a console port, to be bound to hvc. This keeps new host, old
guest happy.

A testsuite is put up at the following site that has an interactive
test program that can be run in the guest. It also has an automated
test suite that tests the major functionalities presented here:

http://fedorapeople.org/gitweb?p=amitshah/public_git/test-virtserial.git

This version passes all the automated tests there.

Christian has tested the console functionality on s390.

v10:
- only one process can open a port in the guest
- minor fixes for throttling and caching cases


Please review and apply,
Amit

Amit Shah (1):
virtio_console: Add support for multiple ports for generic guest and
host communication

drivers/char/Kconfig | 6 +
drivers/char/virtio_console.c | 1412 ++++++++++++++++++++++++++++++++++++----
include/linux/virtio_console.h | 50 ++-
3 files changed, 1332 insertions(+), 136 deletions(-)


2009-11-03 16:40:07

by Amit Shah

[permalink] [raw]
Subject: [PATCH v10 1/1] virtio_console: Add support for multiple ports for generic guest and host communication

Expose multiple char devices ("ports") for simple communication
between the host userspace and guest.

Sample offline usages can be: poking around in a guest to find out
the file systems used, applications installed, etc. Online usages
can be sharing of clipboard data between the guest and the host,
sending information about logged-in users to the host, locking the
screen or session when a vnc session is closed, and so on.

This patch also enables spawning of multiple console ports that
can be attached to hvc consoles.

The interface presented to guest userspace is of a simple char
device, so it can be used like this:

fd = open("/dev/vcon2", O_RDWR);
ret = read(fd, buf, 100);
ret = write(fd, string, strlen(string));

A name property, if set by the host, is exposed in

/sys/class/virtio-console/vconNN/name

that can be used to create symlinks by udev scripts, example:

/dev/vcon/org.libvirt.channel.0 -> /dev/vcon1

For requirements, use-cases, test cases, udev scripts and some
history see

http://www.linux-kvm.org/page/VMchannel_Requirements

Signed-off-by: Amit Shah <[email protected]>
Acked-by: Christian Borntraeger <[email protected]> (console)
CC: Christian Borntraeger <[email protected]>
---
drivers/char/Kconfig | 6 +
drivers/char/virtio_console.c | 1412 ++++++++++++++++++++++++++++++++++++----
include/linux/virtio_console.h | 50 ++-
3 files changed, 1332 insertions(+), 136 deletions(-)

diff --git a/drivers/char/Kconfig b/drivers/char/Kconfig
index 6aad99e..bc71c60 100644
--- a/drivers/char/Kconfig
+++ b/drivers/char/Kconfig
@@ -679,6 +679,12 @@ config VIRTIO_CONSOLE
help
Virtio console for use with lguest and other hypervisors.

+ Also serves as a general-purpose serial device for data
+ transfer between the guest and host. Character devices at
+ /dev/vconNN will be created when corresponding ports are
+ found. If specified by the host, a sysfs attribute called
+ 'name' will be populated with a name for the port which can
+ be used by udev scripts to create a symlink to /dev/vconNN.

config HVCS
tristate "IBM Hypervisor Virtual Console Server support"
diff --git a/drivers/char/virtio_console.c b/drivers/char/virtio_console.c
index a035ae3..48c533c 100644
--- a/drivers/char/virtio_console.c
+++ b/drivers/char/virtio_console.c
@@ -9,10 +9,8 @@
* functions.
:*/

-/*M:002 The console can be flooded: while the Guest is processing input the
- * Host can send more. Buffering in the Host could alleviate this, but it is a
- * difficult problem in general. :*/
/* Copyright (C) 2006, 2007 Rusty Russell, IBM Corporation
+ * Copyright (C) 2009, Amit Shah, Red Hat, Inc.
*
* 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
@@ -28,114 +26,582 @@
* 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/debugfs.h>
+#include <linux/device.h>
#include <linux/err.h>
+#include <linux/fs.h>
#include <linux/init.h>
+#include <linux/list.h>
+#include <linux/poll.h>
+#include <linux/sched.h>
+#include <linux/spinlock.h>
#include <linux/virtio.h>
#include <linux/virtio_console.h>
+#include <linux/wait.h>
+#include <linux/workqueue.h>
#include "hvc_console.h"

-/*D:340 These represent our input and output console queues, and the virtio
- * operations for them. */
-static struct virtqueue *in_vq, *out_vq;
-static struct virtio_device *vdev;
+/* This struct stores data that's common to all the ports */
+struct virtio_console_struct {
+ /*
+ * Workqueue handlers where we process deferred work after an
+ * interrupt
+ */
+ struct work_struct rx_work;
+ struct work_struct tx_work;
+ struct work_struct config_work;

-/* This is our input buffer, and how much data is left in it. */
-static unsigned int in_len;
-static char *in, *inbuf;
+ struct list_head port_head;
+ struct list_head unused_read_head;
+ struct list_head unused_write_head;

-/* The operations for our console. */
-static struct hv_ops virtio_cons;
+ /* To protect the list of unused write buffers and the out_vq */
+ spinlock_t write_list_lock;
+
+ struct virtio_device *vdev;
+ struct class *class;
+ /* The input and the output queues */
+ struct virtqueue *in_vq, *out_vq;
+
+ /* Used for exporting per-port information to debugfs */
+ struct dentry *debugfs_dir;
+
+ /* The current config space is stored here */
+ struct virtio_console_config config;
+};
+
+/* This struct holds individual buffers received for each port */
+struct virtio_console_port_buffer {
+ struct list_head next;
+
+ char *buf;
+
+ /* length of the buffer */
+ size_t len;
+ /* offset in the buf from which to consume data */
+ size_t offset;
+};
+
+/* This struct holds the per-port data */
+struct virtio_console_port {
+ /* Next port in the list, head is in the virtio_console_struct */
+ struct list_head next;
+
+ /* Pointer to the virtio_console device */
+ struct virtio_console_struct *vcon;
+
+ /* File in the debugfs directory that exposes this port's information */
+ struct dentry *debugfs_file;
+
+ /* Buffer management */
+ struct list_head readbuf_head;
+
+ /*
+ * To protect the readbuf_head list. Has to be a spinlock
+ * because it can be called from interrupt context
+ * (cons_get_char())
+ */
+ spinlock_t readbuf_list_lock;
+
+ /* A waitqueue for poll() or blocking read operations */
+ wait_queue_head_t waitqueue;
+
+ /* Each port associates with a separate char device */
+ struct cdev cdev;
+ struct device *dev;
+
+ /* The hvc device, if this port is associated with a console */
+ struct hvc_struct *hvc;
+
+ /* The 'name' of the port that we expose via sysfs properties */
+ char *name;
+
+ /* The 'id' to identify the port with the Host */
+ u32 id;
+
+ /*
+ * If this port is a console port, this number identifies the
+ * number that we used to register with hvc in
+ * hvc_instantiate() and hvc_alloc().
+ */
+ u32 vtermno;

-/* The hvc device */
-static struct hvc_struct *hvc;
+ /*
+ * The Host can set some limit to the number of outstanding
+ * buffers we can have in the readbuf_head list before we
+ * should ask the Host to stop any more data from coming
+ * in. This is to not allow a rogue process on the Host to OOM
+ * the entire guest.
+ */
+ u32 buffer_limit;

-/*D:310 The put_chars() callback is pretty straightforward.
+ /*
+ * The actual number of buffers we have pending in the
+ * readbuf_head list
+ */
+ u32 nr_buffers;
+
+ /* Is the host device open */
+ bool host_connected;
+
+ /* Does the Host not want to accept more data currently? */
+ bool host_throttled;
+
+ /* We should allow only one process to open a port */
+ bool guest_connected;
+
+ /* Have we sent out a throttle request to the Host? */
+ bool guest_throttled;
+
+ /* Does the port want to cache data when disconnected? */
+ bool cache_buffers;
+};
+
+static struct virtio_console_struct virtconsole;
+
+/*
+ * This is used to keep track of the number of hvc consoles spawned.
+ * This number is given as first argument to hvc_alloc(). We could as
+ * well pass on the minor number of the char device but to correctly
+ * map an initial console spawned via hvc_instantiate to the console
+ * being hooked up via hvc_alloc, we need to pass the same vtermno.
*
- * We turn the characters into a scatter-gather list, add it to the output
- * queue and then kick the Host. Then we sit here waiting for it to finish:
- * inefficient in theory, but in practice implementations will do it
- * immediately (lguest's Launcher does). */
-static int put_chars(u32 vtermno, const char *buf, int count)
+ * With this int, we just assume the first console being initialised
+ * was the first one that got used as the initial console.
+ */
+static unsigned int hvc_vtermno;
+
+static struct virtio_console_port *get_port_from_vtermno(u32 vtermno)
{
- struct scatterlist sg[1];
- unsigned int len;
+ struct virtio_console_port *port;
+
+ list_for_each_entry(port, &virtconsole.port_head, next) {
+ if (port->hvc && port->vtermno == vtermno)
+ return port;
+ }
+ return NULL;
+}

- /* This is a convenient routine to initialize a single-elem sg list */
- sg_init_one(sg, buf, count);
+static struct virtio_console_port *get_port_from_devt(dev_t devt)
+{
+ struct virtio_console_port *port;

- /* 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();
+ list_for_each_entry(port, &virtconsole.port_head, next) {
+ if (port->dev->devt == devt)
+ return port;
}
+ return NULL;
+}

- /* We're expected to return the amount of data we wrote: all of it. */
- return count;
+static struct virtio_console_port *get_port_from_id(u32 id)
+{
+ struct virtio_console_port *port;
+
+ list_for_each_entry(port, &virtconsole.port_head, next) {
+ if (port->id == id)
+ return port;
+ }
+ return NULL;
+}
+
+static int get_id_from_port(struct virtio_console_port *port)
+{
+ return port->id;
+}
+
+static bool is_console_port(struct virtio_console_port *port)
+{
+ if (port->hvc)
+ return true;
+ return false;
+}
+
+static inline bool use_multiport(struct virtio_console_struct *vcon)
+{
+ /*
+ * This condition can be true when put_chars is called from
+ * early_init
+ */
+ if (!vcon->vdev)
+ return 0;
+ return vcon->vdev->features[0] & (1 << VIRTIO_CONSOLE_F_MULTIPORT);
}

-/* Create a scatter-gather list representing our input buffer and put it in the
- * queue. */
-static void add_inbuf(void)
+static inline bool is_internal(u32 flags)
{
+ return flags & VIRTIO_CONSOLE_ID_INTERNAL;
+}
+
+static ssize_t send_buf(struct virtio_console_port *port,
+ const char *in_buf, size_t in_count,
+ u32 flags, bool from_user)
+{
+ struct virtqueue *out_vq;
+ struct virtio_console_port_buffer *buf, *buf2;
struct scatterlist sg[1];
- sg_init_one(sg, inbuf, PAGE_SIZE);
+ struct virtio_console_header header;
+ size_t in_offset, copy_size;
+ ssize_t ret;
+ unsigned int header_len;
+
+ if (!in_count)
+ return 0;
+
+ out_vq = port->vcon->out_vq;
+ /*
+ * We should not send internal messages to a host that won't
+ * understand them
+ */
+ if (!use_multiport(port->vcon) && is_internal(flags))
+ return 0;
+ header_len = 0;
+ if (use_multiport(port->vcon)) {
+ header.id = get_id_from_port(port);
+ header.flags = flags;
+ header.size = in_count;
+ header_len = sizeof(header);
+ }
+ in_offset = 0; /* offset in the user buffer */
+ while (in_count - in_offset) {
+ copy_size = min(in_count - in_offset + header_len, PAGE_SIZE);
+
+ spin_lock(&port->vcon->write_list_lock);
+ list_for_each_entry_safe(buf, buf2,
+ &port->vcon->unused_write_head,
+ next) {
+ list_del(&buf->next);
+ break;
+ }
+ spin_unlock(&port->vcon->write_list_lock);
+ if (!buf)
+ break;
+ if (header_len) {
+ memcpy(buf->buf, &header, header_len);
+ copy_size -= header_len;
+ }
+ if (from_user) {
+ ret = copy_from_user(buf->buf + header_len,
+ in_buf + in_offset, copy_size);
+ } else {
+ /*
+ * Since we're not sure when the host will actually
+ * consume the data and tell us about it, we have
+ * to copy the data here in case the caller
+ * frees the in_buf
+ */
+ memcpy(buf->buf + header_len,
+ in_buf + in_offset, copy_size);
+ ret = 0; /* Emulate copy_from_user behaviour */
+ }
+ buf->len = header_len + copy_size - ret;
+ sg_init_one(sg, buf->buf, buf->len);
+
+ spin_lock(&port->vcon->write_list_lock);
+ ret = out_vq->vq_ops->add_buf(out_vq, sg, 1, 0, buf);
+ spin_unlock(&port->vcon->write_list_lock);
+ if (ret < 0) {
+ memset(buf->buf, 0, buf->len);
+ spin_lock(&virtconsole.write_list_lock);
+ list_add_tail(&buf->next,
+ &port->vcon->unused_write_head);
+ spin_unlock(&port->vcon->write_list_lock);
+ break;
+ }
+ in_offset += buf->len - header_len;
+ /*
+ * Only send size with the first buffer. This way
+ * userspace can find out a continuous stream of data
+ * belonging to one write request and consume it
+ * appropriately
+ */
+ header.size = 0;
+
+ /* No space left in the vq anyway */
+ if (!ret)
+ break;
+ }
+ /* Tell Host to go! */
+ spin_lock(&port->vcon->write_list_lock);
+ out_vq->vq_ops->kick(out_vq);
+ spin_unlock(&port->vcon->write_list_lock);

- /* We should always be able to add one buffer to an empty queue. */
- if (in_vq->vq_ops->add_buf(in_vq, sg, 0, 1, inbuf) < 0)
- BUG();
- in_vq->vq_ops->kick(in_vq);
+ /* We're expected to return the amount of data we wrote */
+ return in_offset;
}

-/*D:350 get_chars() is the callback from the hvc_console infrastructure when
- * an interrupt is received.
- *
- * Most of the code deals with the fact that the hvc_console() infrastructure
- * only asks us for 16 bytes at a time. We keep in_offset and in_used fields
- * for partially-filled buffers. */
-static int get_chars(u32 vtermno, char *buf, int count)
+/*
+ * Give out the data that's requested from the buffers that we have
+ * queued up per port
+ */
+static ssize_t fill_readbuf(struct virtio_console_port *port,
+ char *out_buf, size_t out_count, bool to_user)
{
- /* If we don't have an input queue yet, we can't get input. */
- BUG_ON(!in_vq);
+ struct virtio_console_port_buffer *buf, *buf2;
+ ssize_t out_offset, ret;
+
+ out_offset = 0;
+ /*
+ * Not taking the port->readbuf_list_lock here relying on the
+ * fact that buffers are taken out from the list only in this
+ * function so buf2 should be available all the time.
+ *
+ * Also, copy_to_user() might sleep.
+ */
+ list_for_each_entry_safe(buf, buf2, &port->readbuf_head, next) {
+ size_t copy_size;
+
+ copy_size = out_count;
+ if (copy_size > buf->len - buf->offset)
+ copy_size = buf->len - buf->offset;
+
+ if (to_user) {
+ ret = copy_to_user(out_buf + out_offset,
+ buf->buf + buf->offset,
+ copy_size);
+ /* FIXME: Deal with ret != 0 */
+ } else {
+ memcpy(out_buf + out_offset,
+ buf->buf + buf->offset,
+ copy_size);
+ ret = 0; /* Emulate copy_to_user behaviour */
+ }
+
+ /* Return the number of bytes actually copied */
+ ret = copy_size - ret;
+ buf->offset += ret;
+ out_offset += ret;
+ out_count -= ret;
+
+ if (buf->len - buf->offset == 0) {
+ spin_lock(&port->readbuf_list_lock);
+ list_del(&buf->next);
+ port->nr_buffers--;
+ spin_unlock(&port->readbuf_list_lock);
+ kfree(buf->buf);
+ kfree(buf);
+ }
+ if (!out_count)
+ break;
+ }
+ if (port->guest_throttled && port->nr_buffers < port->buffer_limit) {
+ struct virtio_console_control cpkt;
+
+ cpkt.event = VIRTIO_CONSOLE_THROTTLE_PORT;
+ cpkt.value = false;
+ send_buf(port, (char *)&cpkt, sizeof(cpkt),
+ VIRTIO_CONSOLE_ID_INTERNAL, false);
+ port->guest_throttled = false;
+ }
+ return out_offset;
+}
+
+/* The condition that must be true for polling to end */
+static bool wait_is_over(struct virtio_console_port *port)
+{
+ return !list_empty(&port->readbuf_head) || !port->host_connected;
+}
+
+static ssize_t virtconsole_read(struct file *filp, char __user *ubuf,
+ size_t count, loff_t *offp)
+{
+ struct virtio_console_port *port;
+ ssize_t ret;

- /* No buffer? Try to get one. */
- if (!in_len) {
- in = in_vq->vq_ops->get_buf(in_vq, &in_len);
- if (!in)
+ port = filp->private_data;
+
+ if (list_empty(&port->readbuf_head)) {
+ /*
+ * If nothing's connected on the host just return 0 in
+ * case of list_empty; this tells the userspace app
+ * that there's no connection
+ */
+ if (!port->host_connected)
return 0;
+ if (filp->f_flags & O_NONBLOCK)
+ return -EAGAIN;
+
+ ret = wait_event_interruptible(port->waitqueue,
+ wait_is_over(port));
+ if (ret < 0)
+ return ret;
}
+ /*
+ * We could've received a disconnection message while we were
+ * waiting for more data.
+ *
+ * This check is not clubbed in the if() statement above as we
+ * might receive some data as well as the host could get
+ * disconnected after we got woken up from our wait. So we
+ * really want to give off whatever data we have and only then
+ * check for host_connected
+ */
+ if (list_empty(&port->readbuf_head) && !port->host_connected)
+ return 0;

- /* You want more than we have to give? Well, try wanting less! */
- if (in_len < count)
- count = in_len;
+ return fill_readbuf(port, ubuf, count, true);
+}
+
+static ssize_t virtconsole_write(struct file *filp, const char __user *ubuf,
+ size_t count, loff_t *offp)
+{
+ struct virtio_console_port *port;

- /* Copy across to their buffer and increment offset. */
- memcpy(buf, in, count);
- in += count;
- in_len -= count;
+ port = filp->private_data;

- /* Finished? Re-register buffer so Host will use it again. */
- if (in_len == 0)
- add_inbuf();
+ if (port->host_throttled)
+ return -ENOSPC;

- return count;
+ return send_buf(port, ubuf, count, 0, true);
}
-/*:*/

-/*D:320 Console drivers are initialized very early so boot messages can go out,
- * so we do things slightly differently from the generic virtio initialization
- * of the net and block drivers.
+static unsigned int virtconsole_poll(struct file *filp, poll_table *wait)
+{
+ struct virtio_console_port *port;
+ unsigned int ret;
+
+ port = filp->private_data;
+ poll_wait(filp, &port->waitqueue, wait);
+
+ ret = 0;
+ if (!list_empty(&port->readbuf_head))
+ ret |= POLLIN | POLLRDNORM;
+ if (port->host_connected && !port->host_throttled)
+ ret |= POLLOUT;
+ if (!port->host_connected)
+ ret |= POLLHUP;
+
+ return ret;
+}
+
+static int virtconsole_release(struct inode *inode, struct file *filp)
+{
+ struct virtio_console_port *port;
+ struct virtio_console_control cpkt;
+
+ port = filp->private_data;
+
+ /* Notify host of port being closed */
+ cpkt.event = VIRTIO_CONSOLE_PORT_OPEN;
+ cpkt.value = 0;
+ send_buf(filp->private_data, (char *)&cpkt, sizeof(cpkt),
+ VIRTIO_CONSOLE_ID_INTERNAL, false);
+
+ spin_lock(&port->readbuf_list_lock);
+ port->guest_connected = false;
+
+ /*
+ * If the port doesn't want to cache buffers while closed,
+ * flush the unread buffers queue
+ */
+ if (!port->cache_buffers) {
+ struct virtio_console_port_buffer *buf, *buf2;
+
+ list_for_each_entry_safe(buf, buf2, &port->readbuf_head, next) {
+ list_del(&buf->next);
+ kfree(buf->buf);
+ kfree(buf);
+ port->nr_buffers--;
+ }
+ }
+ spin_unlock(&port->readbuf_list_lock);
+ return 0;
+}
+
+static int virtconsole_open(struct inode *inode, struct file *filp)
+{
+ struct cdev *cdev = inode->i_cdev;
+ struct virtio_console_port *port;
+ struct virtio_console_control cpkt;
+
+ port = container_of(cdev, struct virtio_console_port, cdev);
+ filp->private_data = port;
+
+ /*
+ * Don't allow opening of console port devices -- that's done
+ * via /dev/hvc
+ */
+ if (port->hvc)
+ return -ENXIO;
+
+ /* Allow only one process to open a particular port at a time */
+ spin_lock(&port->readbuf_list_lock);
+ if (port->guest_connected) {
+ spin_unlock(&port->readbuf_list_lock);
+ return -EMFILE;
+ }
+ port->guest_connected = true;
+ spin_unlock(&port->readbuf_list_lock);
+
+ /* Notify host of port being opened */
+ cpkt.event = VIRTIO_CONSOLE_PORT_OPEN;
+ cpkt.value = 1;
+ send_buf(filp->private_data, (char *)&cpkt, sizeof(cpkt),
+ VIRTIO_CONSOLE_ID_INTERNAL, false);
+
+ return 0;
+}
+
+/*
+ * The file operations that we support: programs in the guest can open
+ * a console device, read from it, write to it, poll for data and
+ * close it. The devices are at /dev/vconNN
+ */
+static const struct file_operations virtconsole_fops = {
+ .owner = THIS_MODULE,
+ .open = virtconsole_open,
+ .read = virtconsole_read,
+ .write = virtconsole_write,
+ .poll = virtconsole_poll,
+ .release = virtconsole_release,
+};
+
+
+/*D:310
+ * The cons_put_chars() callback is pretty straightforward.
*
- * At this stage, the console is output-only. It's too early to set up a
- * virtqueue, so we let the drivers do some boutique early-output thing. */
-int __init virtio_cons_early_init(int (*put_chars)(u32, const char *, int))
+ * We turn the characters into a scatter-gather list, add it to the output
+ * queue and then kick the Host.
+ *
+ * If the data to be outpu spans more than a page, it's split into
+ * page-sized buffers and then individual buffers are pushed to Host.
+ */
+static int cons_put_chars(u32 vtermno, const char *buf, int count)
{
- virtio_cons.put_chars = put_chars;
- return hvc_instantiate(0, 0, &virtio_cons);
+ struct virtio_console_port *port;
+
+ port = get_port_from_vtermno(vtermno);
+ if (!port)
+ return 0;
+
+ return send_buf(port, buf, count, 0, false);
+}
+
+/*D:350
+ * cons_get_chars() is the callback from the hvc_console
+ * infrastructure when an interrupt is received.
+ *
+ * We call out to fill_readbuf that gets us the required data from the
+ * buffers that are queued up.
+ */
+static int cons_get_chars(u32 vtermno, char *buf, int count)
+{
+ struct virtio_console_port *port;
+
+ /* If we don't have an input queue yet, we can't get input. */
+ BUG_ON(!virtconsole.in_vq);
+
+ port = get_port_from_vtermno(vtermno);
+ if (!port)
+ return 0;
+
+ if (list_empty(&port->readbuf_head))
+ return 0;
+
+ return fill_readbuf(port, buf, count, false);
}
+/*:*/

/*
* virtio console configuration. This supports:
@@ -152,98 +618,751 @@ static void virtcons_apply_config(struct virtio_device *dev)
dev->config->get(dev,
offsetof(struct virtio_console_config, rows),
&ws.ws_row, sizeof(u16));
- hvc_resize(hvc, ws);
+ /*
+ * We'll use this way of resizing only for legacy
+ * support. For newer userspace (VIRTIO_CONSOLE_F_MULTPORT+),
+ * use internal messages to indicate console size
+ * changes so that it can be done per-port
+ */
+ if (!use_multiport(&virtconsole))
+ hvc_resize(get_port_from_id(0)->hvc, ws);
}
}

/*
- * we support only one console, the hvc struct is a global var
* We set the configuration at this point, since we now have a tty
*/
-static int notifier_add_vio(struct hvc_struct *hp, int data)
+static int cons_notifier_add_vio(struct hvc_struct *hp, int data)
{
hp->irq_requested = 1;
- virtcons_apply_config(vdev);
+ virtcons_apply_config(virtconsole.vdev);

return 0;
}

-static void notifier_del_vio(struct hvc_struct *hp, int data)
+static void cons_notifier_del_vio(struct hvc_struct *hp, int data)
{
hp->irq_requested = 0;
}

-static void hvc_handle_input(struct virtqueue *vq)
+/* The operations for our console. */
+static struct hv_ops virtio_cons = {
+ .get_chars = cons_get_chars,
+ .put_chars = cons_put_chars,
+ .notifier_add = cons_notifier_add_vio,
+ .notifier_del = cons_notifier_del_vio,
+ .notifier_hangup = cons_notifier_del_vio,
+};
+
+/*D:320
+ * Console drivers are initialized very early so boot messages can go out,
+ * so we do things slightly differently from the generic virtio initialization
+ * of the net and block drivers.
+ *
+ * At this stage, the console is output-only. It's too early to set up a
+ * virtqueue, so we let the drivers do some boutique early-output thing.
+ */
+int __init virtio_cons_early_init(int (*put_chars)(u32, const char *, int))
+{
+ virtio_cons.put_chars = put_chars;
+ return hvc_instantiate(0, 0, &virtio_cons);
+}
+
+int init_port_console(struct virtio_console_port *port)
{
- if (hvc_poll(hvc))
- hvc_kick();
+ int ret = 0;
+
+ /*
+ * The Host's telling us this port is a console port. Hook it
+ * up with an hvc console.
+ *
+ * To set up and manage our virtual console, we call
+ * hvc_alloc().
+ *
+ * The first argument of hvc_alloc() is the virtual console
+ * number. The second argument is the parameter for the
+ * notification mechanism (like irq number). We currently
+ * leave this as zero, virtqueues have implicit notifications.
+ *
+ * The third argument is a "struct hv_ops" containing the
+ * put_chars() get_chars(), notifier_add() and notifier_del()
+ * pointers. The final argument is the output buffer size: we
+ * can do any size, so we put PAGE_SIZE here.
+ */
+ port->hvc = hvc_alloc(hvc_vtermno, 0, &virtio_cons, PAGE_SIZE);
+ if (IS_ERR(port->hvc)) {
+ ret = PTR_ERR(port->hvc);
+ pr_err("%s: Could not alloc hvc for virtio console port, ret = %d\n",
+ __func__, ret);
+ port->hvc = NULL;
+ } else {
+ port->vtermno = hvc_vtermno++;
+ port->guest_connected = true;
+ }
+ return ret;
}

-/*D:370 Once we're further in boot, we get probed like any other virtio device.
- * At this stage we set up the output virtqueue.
+/* Any secret messages that the Host and Guest want to share */
+static void handle_control_message(struct virtio_console_port *port,
+ struct virtio_console_port_buffer *buf)
+{
+ struct virtio_console_control *cpkt;
+ size_t name_size;
+
+ cpkt = (struct virtio_console_control *)(buf->buf + buf->offset);
+
+ switch (cpkt->event) {
+ case VIRTIO_CONSOLE_PORT_OPEN:
+ port->host_connected = cpkt->value;
+ break;
+ case VIRTIO_CONSOLE_PORT_NAME:
+ /*
+ * Skip the size of the header and the cpkt to get the size
+ * of the name that was sent
+ */
+ name_size = buf->len - buf->offset - sizeof(*cpkt) + 1;
+
+ port->name = kmalloc(name_size, GFP_KERNEL);
+ if (!port->name) {
+ pr_err("%s: not enough space to store port name\n",
+ __func__);
+ break;
+ }
+ strncpy(port->name, buf->buf + buf->offset + sizeof(*cpkt),
+ name_size - 1);
+ port->name[name_size - 1] = 0;
+ break;
+ case VIRTIO_CONSOLE_CONSOLE_PORT:
+ if (!cpkt->value)
+ break;
+ init_port_console(port);
+ /*
+ * Could remove the port here in case init fails - but
+ * have to notify the host first
+ */
+ break;
+ case VIRTIO_CONSOLE_CACHE_BUFFERS:
+ if (!cpkt->value)
+ break;
+ port->cache_buffers = true;
+ break;
+ case VIRTIO_CONSOLE_THROTTLE_PORT:
+ /*
+ * Hosts can govern some policy to disallow rogue
+ * processes write indefinitely to ports leading to
+ * OOM situations. If we receive this message here,
+ * it means the Host side of the port either reached
+ * its max. limit to cache data or signal to us that
+ * the host is ready to accept more data.
+ */
+ port->host_throttled = cpkt->value;
+ break;
+ case VIRTIO_CONSOLE_BUFFER_LIMIT:
+ /*
+ * We can ask the Host to stop sending data to us in
+ * case some rogue process on the Host injects data to
+ * OOM the entire guest (as unread buffers keep piling
+ * up in the kernel space). The Host, via this
+ * message, can tell us the number of Mbytes to
+ * buffer.
+ *
+ * The calculation is easy: we use 4K-sized pages in
+ * our readbuf_head list. So on each buffer added to
+ * the list, increment count by 1 (and decrement by 1
+ * when one gets removed). So we just store the
+ * (number-in-MB * 1024) / 4 to calculate the number of
+ * buffers we can have in the list.
+ */
+ port->buffer_limit = (u32)cpkt->value * 1024 / 4;
+ break;
+ }
+}
+
+
+static struct virtio_console_port_buffer *get_buf(size_t buf_size)
+{
+ struct virtio_console_port_buffer *buf;
+
+ buf = kzalloc(sizeof(*buf), GFP_KERNEL);
+ if (!buf)
+ goto out;
+ buf->buf = kzalloc(buf_size, GFP_KERNEL);
+ if (!buf->buf) {
+ kfree(buf);
+ goto out;
+ }
+ buf->len = buf_size;
+out:
+ return buf;
+}
+
+static void fill_queue(struct virtqueue *vq, size_t buf_size,
+ struct list_head *unused_head)
+{
+ struct scatterlist sg[1];
+ struct virtio_console_port_buffer *buf;
+ int ret;
+
+ do {
+ buf = get_buf(buf_size);
+ if (!buf)
+ break;
+ sg_init_one(sg, buf->buf, buf_size);
+
+ ret = vq->vq_ops->add_buf(vq, sg, 0, 1, buf);
+ if (ret < 0) {
+ kfree(buf->buf);
+ kfree(buf);
+ break;
+ }
+ /*
+ * We have to keep track of the unused buffers so that
+ * they can be freed when the module is being removed
+ */
+ list_add_tail(&buf->next, unused_head);
+ } while (ret > 0);
+ vq->vq_ops->kick(vq);
+}
+
+static void fill_receive_queue(struct virtio_console_struct *vcon)
+{
+ fill_queue(vcon->in_vq, PAGE_SIZE, &vcon->unused_read_head);
+}
+
+/*
+ * This function is only called from the init routine so the spinlock
+ * for the unused_write_head list isn't taken
+ */
+static void alloc_write_bufs(struct virtio_console_struct *vcon)
+{
+ struct virtio_console_port_buffer *buf;
+ int i;
+
+ for (i = 0; i < 1024; i++) {
+ buf = get_buf(PAGE_SIZE);
+ if (!buf)
+ break;
+ list_add_tail(&buf->next, &vcon->unused_write_head);
+ }
+}
+
+/*
+ * The workhandler for any buffers that appear on our input queue.
+ * Pick the buffer; if it's some internal communication meant for the
+ * us, just process it. Otherwise queue it up for the read() or
+ * get_chars() routines to pick the data up later.
+ */
+static void virtio_console_rx_work_handler(struct work_struct *work)
+{
+ struct virtio_console_struct *vcon;
+ struct virtio_console_port *port;
+ struct virtio_console_port_buffer *buf;
+ struct virtio_console_header header;
+ struct virtqueue *vq;
+ unsigned int tmplen, header_len;
+
+ vcon = container_of(work, struct virtio_console_struct, rx_work);
+ header_len = use_multiport(vcon) ? sizeof(header) : 0;
+
+ port = NULL;
+ vq = vcon->in_vq;
+ while ((buf = vq->vq_ops->get_buf(vq, &tmplen))) {
+ /* The buffer is no longer unused */
+ list_del(&buf->next);
+
+ if (use_multiport(vcon)) {
+ memcpy(&header, buf->buf, header_len);
+ port = get_port_from_id(header.id);
+ } else {
+ port = get_port_from_id(0);
+ }
+ if (!port) {
+ /* No valid header at start of buffer. Drop it. */
+ pr_debug("%s: invalid index in buffer, %c %d\n",
+ __func__, buf->buf[0], buf->buf[0]);
+ /*
+ * OPT: This buffer can be added to the unused
+ * list to avoid free / alloc
+ */
+ kfree(buf->buf);
+ kfree(buf);
+ break;
+ }
+ buf->len = tmplen;
+ buf->offset = header_len;
+ if (use_multiport(vcon) && is_internal(header.flags)) {
+ handle_control_message(port, buf);
+ /*
+ * OPT: This buffer can be added to the unused
+ * list to avoid free/alloc
+ */
+ kfree(buf->buf);
+ kfree(buf);
+ } else {
+ /*
+ * We might have missed a connection
+ * notification, e.g. before the queues were
+ * initialised.
+ */
+ port->host_connected = true;
+ /*
+ * Don't queue up buffers if caching is
+ * disabled and port is closed. This condition
+ * can be reached when a console port is not
+ * yet connected (no tty is spawned) and the
+ * host sends out data to console ports. For
+ * generic serial ports, the host won't send
+ * data till the guest is connected.
+ */
+ if (!port->guest_connected && !port->cache_buffers) {
+ kfree(buf->buf);
+ kfree(buf);
+ continue;
+ }
+ if (port->guest_throttled) {
+ kfree(buf->buf);
+ kfree(buf);
+ continue;
+ }
+ spin_lock(&port->readbuf_list_lock);
+ list_add_tail(&buf->next, &port->readbuf_head);
+ port->nr_buffers++;
+ spin_unlock(&port->readbuf_list_lock);
+ /*
+ * If we reached the throttle level, send out
+ * a message to the host to ask it to stop
+ * sending us any more data.
+ */
+ if (port->nr_buffers >= port->buffer_limit) {
+ struct virtio_console_control cpkt;
+
+ port->guest_throttled = true;
+ cpkt.event = VIRTIO_CONSOLE_THROTTLE_PORT;
+ cpkt.value = true;
+ send_buf(port, (char *)&cpkt, sizeof(cpkt),
+ VIRTIO_CONSOLE_ID_INTERNAL, false);
+ }
+ }
+ wake_up_interruptible(&port->waitqueue);
+
+ if (is_console_port(port) && !list_empty(&port->readbuf_head))
+ if (hvc_poll(port->hvc))
+ hvc_kick();
+ }
+ /* Allocate buffers for all the ones that got used up */
+ fill_receive_queue(&virtconsole);
+}
+
+/*
+ * This is the workhandler for buffers that get received on the output
+ * virtqueue, which is an indication that Host consumed the data we
+ * sent it. Since all our buffers going out are of a fixed size we can
+ * just reuse them instead of freeing them and allocating new ones.
*
- * To set up and manage our virtual console, we call hvc_alloc(). Since we
- * never remove the console device we never need this pointer again.
+ * Zero out the buffer so that we don't leak any information from
+ * other processes. There's a small optimisation here as well: the
+ * buffers are PAGE_SIZE-sized; but instead of zeroing the entire
+ * page, we just zero the length that was most recently used and we
+ * can be sure the rest of the page is already set to 0s.
*
- * Finally we put our input buffer in the input queue, ready to receive. */
-static int __devinit virtcons_probe(struct virtio_device *dev)
+ * So once we zero them out we add them back to the unused buffers
+ * list
+ */
+static void virtio_console_tx_work_handler(struct work_struct *work)
+{
+ struct virtio_console_struct *vcon;
+ struct virtqueue *vq;
+ struct virtio_console_port_buffer *buf;
+ unsigned int tmplen;
+
+ vcon = container_of(work, struct virtio_console_struct, tx_work);
+
+ vq = vcon->out_vq;
+ spin_lock(&vcon->write_list_lock);
+ while ((buf = vq->vq_ops->get_buf(vq, &tmplen))) {
+ /* 0 the buffer to not leak data from other processes */
+ memset(buf->buf, 0, buf->len);
+ list_add_tail(&buf->next, &vcon->unused_write_head);
+ }
+ spin_unlock(&vcon->write_list_lock);
+}
+
+static void rx_intr(struct virtqueue *vq)
+{
+ schedule_work(&virtconsole.rx_work);
+}
+
+static void tx_intr(struct virtqueue *vq)
+{
+ schedule_work(&virtconsole.tx_work);
+}
+
+static void config_intr(struct virtio_device *vdev)
+{
+ if (use_multiport(&virtconsole)) {
+ /* Handle port hot-add */
+ schedule_work(&virtconsole.config_work);
+ }
+ /* Handle console size changes */
+ virtcons_apply_config(vdev);
+}
+
+
+static ssize_t show_port_name(struct device *dev,
+ struct device_attribute *attr, char *buffer)
+{
+ struct virtio_console_port *port;
+
+ port = get_port_from_devt(dev->devt);
+ if (!port || !port->name)
+ return 0;
+
+ return sprintf(buffer, "%s\n", port->name);
+}
+
+static DEVICE_ATTR(name, S_IRUGO, show_port_name, NULL);
+
+static struct attribute *virtcon_sysfs_entries[] = {
+ &dev_attr_name.attr,
+ NULL
+};
+
+static struct attribute_group virtcon_attribute_group = {
+ .name = NULL, /* put in device directory */
+ .attrs = virtcon_sysfs_entries,
+};
+
+
+static int virtcon_port_debugfs_open(struct inode *inode, struct file *filp)
{
- vq_callback_t *callbacks[] = { hvc_handle_input, NULL};
+ filp->private_data = inode->i_private;
+ return 0;
+}
+
+static ssize_t virtcon_port_debugfs_read(struct file *filp, char __user *ubuf,
+ size_t count, loff_t *offp)
+{
+ struct virtio_console_port *port;
+ char *buf;
+ ssize_t ret, out_offset, out_count;
+
+ out_count = 1024;
+ buf = kmalloc(out_count, GFP_KERNEL);
+ if (!buf)
+ return -ENOMEM;
+
+ port = filp->private_data;
+ out_offset = 0;
+ out_offset += snprintf(buf + out_offset, out_count,
+ "name: %s\n", port->name);
+ out_offset += snprintf(buf + out_offset, out_count - out_offset,
+ "guest_connected: %d\n", port->guest_connected);
+ out_offset += snprintf(buf + out_offset, out_count - out_offset,
+ "guest_throttled: %d\n", port->guest_throttled);
+ out_offset += snprintf(buf + out_offset, out_count - out_offset,
+ "host_connected: %d\n", port->host_connected);
+ out_offset += snprintf(buf + out_offset, out_count - out_offset,
+ "host_throttled: %d\n", port->host_throttled);
+ out_offset += snprintf(buf + out_offset, out_count - out_offset,
+ "is_console: %d\n", port->hvc ? 1 : 0);
+ out_offset += snprintf(buf + out_offset, out_count - out_offset,
+ "console_vtermno: %u\n", port->vtermno);
+ out_offset += snprintf(buf + out_offset, out_count - out_offset,
+ "buffer_limit: %u\n", port->buffer_limit);
+ out_offset += snprintf(buf + out_offset, out_count - out_offset,
+ "nr_buffers: %u\n", port->nr_buffers);
+ out_offset += snprintf(buf + out_offset, out_count - out_offset,
+ "cache_buffers: %d\n", port->cache_buffers);
+
+ ret = simple_read_from_buffer(ubuf, count, offp, buf, out_offset);
+ kfree(buf);
+ return ret;
+}
+
+static const struct file_operations virtcon_port_debugfs = {
+ .owner = THIS_MODULE,
+ .open = virtcon_port_debugfs_open,
+ .read = virtcon_port_debugfs_read,
+};
+
+
+static int virtconsole_add_port(u32 port_nr)
+{
+ struct virtio_console_port *port;
+ struct virtio_console_control cpkt;
+ char debugfs_name[8];
+ dev_t devt;
+ int ret;
+
+ port = kzalloc(sizeof(*port), GFP_KERNEL);
+ if (!port)
+ return -ENOMEM;
+
+ port->vcon = &virtconsole;
+ port->id = port_nr;
+
+ port->buffer_limit = ~(u32)0;
+
+ cdev_init(&port->cdev, &virtconsole_fops);
+
+ ret = alloc_chrdev_region(&devt, 0, 1, "virtio-console");
+ if (ret < 0) {
+ pr_err("%s: error allocing chrdev region, ret = %d\n",
+ __func__, ret);
+ goto free_port;
+ }
+ ret = cdev_add(&port->cdev, devt, 1);
+ if (ret < 0) {
+ pr_err("%s: error adding cdev, ret = %d\n", __func__, ret);
+ goto free_chrdev;
+ }
+ port->dev = device_create(port->vcon->class, NULL, devt, NULL,
+ "vcon%u", port_nr);
+ if (IS_ERR(port->dev)) {
+ ret = PTR_ERR(port->dev);
+ pr_err("%s: error creating device, ret = %d\n", __func__, ret);
+ goto free_cdev;
+ }
+ ret = sysfs_create_group(&port->dev->kobj, &virtcon_attribute_group);
+ if (ret) {
+ pr_err("%s: error creating sysfs device attributes, ret = %d\n",
+ __func__, ret);
+ goto free_cdev;
+ }
+
+ spin_lock_init(&port->readbuf_list_lock);
+ INIT_LIST_HEAD(&port->readbuf_head);
+ init_waitqueue_head(&port->waitqueue);
+
+ list_add_tail(&port->next, &port->vcon->port_head);
+
+ /*
+ * Ask for the port's name from Host. The string that we
+ * receive in 'name' can be of arbitrary length; so pass the
+ * maximum available buffer size: PAGE_SIZE.
+ */
+ cpkt.event = VIRTIO_CONSOLE_PORT_NAME;
+ send_buf(port, (char *)&cpkt, sizeof(cpkt),
+ VIRTIO_CONSOLE_ID_INTERNAL, false);
+
+ /*
+ * If we're not using multiport support, this has to be a console port
+ */
+ if (!use_multiport(&virtconsole)) {
+ ret = init_port_console(port);
+ if (ret)
+ goto free_cdev;
+ }
+
+ /*
+ * Finally, create the debugfs file that we can use to inspect
+ * a port's state at any time
+ */
+ sprintf(debugfs_name, "vcon%u", port->id);
+ port->debugfs_file = debugfs_create_file(debugfs_name, 0444,
+ port->vcon->debugfs_dir, port,
+ &virtcon_port_debugfs);
+ return 0;
+free_cdev:
+ cdev_del(&port->cdev);
+free_chrdev:
+ unregister_chrdev_region(devt, 1);
+free_port:
+ kfree(port);
+ return ret;
+}
+
+
+/* The workhandler for config-space updates
+ *
+ * This is used when new ports are added
+ */
+static void virtio_console_config_work_handler(struct work_struct *work)
+{
+ struct virtio_console_struct *vcon;
+ struct virtio_console_config virtconconf;
+ struct virtio_device *vdev;
+ int ret;
+
+ vcon = container_of(work, struct virtio_console_struct, config_work);
+
+ vdev = vcon->vdev;
+ vdev->config->get(vdev,
+ offsetof(struct virtio_console_config, nr_active_ports),
+ &virtconconf.nr_active_ports,
+ sizeof(virtconconf.nr_active_ports));
+
+ if (vcon->config.nr_active_ports == virtconconf.nr_active_ports) {
+ /*
+ * Port 0 got hot-added. Since we already did all the other
+ * initialisation for it, just ask the Host for the name
+ * if set
+ */
+ struct virtio_console_control cpkt;
+ struct virtio_console_port *port;
+
+ port = get_port_from_id(0);
+ cpkt.event = VIRTIO_CONSOLE_PORT_NAME;
+ send_buf(port, (char *)&cpkt, sizeof(cpkt),
+ VIRTIO_CONSOLE_ID_INTERNAL, false);
+ return;
+ }
+ /* Hot-add ports */
+ while(virtconconf.nr_active_ports - vcon->config.nr_active_ports) {
+ ret = virtconsole_add_port(vcon->config.nr_active_ports);
+ if (!ret)
+ vcon->config.nr_active_ports++;
+ else
+ break;
+ }
+}
+
+/*D:370
+ * Once we're further in boot, we get probed like any other virtio device.
+ * At this stage we set up the output virtqueue.
+ *
+ * Finally we put our input buffer in the input queue, ready to receive.
+ */
+static int __devinit virtcons_probe(struct virtio_device *vdev)
+{
+ vq_callback_t *callbacks[] = { rx_intr, tx_intr };
const char *names[] = { "input", "output" };
struct virtqueue *vqs[2];
- int err;
-
- vdev = dev;
+ u32 i;
+ int ret;
+ bool multiport;

- /* This is the scratch page we use to receive console input */
- inbuf = kmalloc(PAGE_SIZE, GFP_KERNEL);
- if (!inbuf) {
- err = -ENOMEM;
- goto fail;
+ if (virtconsole.vdev) {
+ pr_err("Multiple virtio-console devices not supported yet\n");
+ return -EEXIST;
}
+ virtconsole.vdev = vdev;
+
+ multiport = false;
+ if (virtio_has_feature(vdev, VIRTIO_CONSOLE_F_MULTIPORT)) {
+ multiport = true;
+ vdev->features[0] |= 1 << VIRTIO_CONSOLE_F_MULTIPORT;
+ vdev->config->finalize_features(vdev);

+ vdev->config->get(vdev, offsetof(struct virtio_console_config,
+ nr_active_ports),
+ &virtconsole.config.nr_active_ports,
+ sizeof(virtconsole.config.nr_active_ports));
+ }
/* Find the queues. */
/* FIXME: This is why we want to wean off hvc: we do nothing
* when input comes in. */
- err = vdev->config->find_vqs(vdev, 2, vqs, callbacks, names);
- if (err)
- goto free;
+ ret = vdev->config->find_vqs(vdev, 2, vqs, callbacks, names);
+ if (ret)
+ goto fail;

- in_vq = vqs[0];
- out_vq = vqs[1];
+ virtconsole.in_vq = vqs[0];
+ virtconsole.out_vq = vqs[1];
+
+ /*
+ * We had set the virtio_cons put_chars implementation to
+ * put_chars for early_init. Now that we're done with the
+ * early init phase, replace it with our cons_put_chars
+ * implementation.
+ */
+ virtio_cons.put_chars = cons_put_chars;
+
+ INIT_LIST_HEAD(&virtconsole.port_head);
+ INIT_LIST_HEAD(&virtconsole.unused_read_head);
+ INIT_LIST_HEAD(&virtconsole.unused_write_head);
+
+ INIT_WORK(&virtconsole.rx_work, &virtio_console_rx_work_handler);
+ INIT_WORK(&virtconsole.tx_work, &virtio_console_tx_work_handler);
+ INIT_WORK(&virtconsole.config_work, &virtio_console_config_work_handler);
+ spin_lock_init(&virtconsole.write_list_lock);
+
+ fill_receive_queue(&virtconsole);
+ alloc_write_bufs(&virtconsole);
+
+ virtconsole_add_port(0);
+ if (multiport)
+ for (i = 1; i < virtconsole.config.nr_active_ports; i++)
+ virtconsole_add_port(i);

- /* Start using the new console output. */
- virtio_cons.get_chars = get_chars;
- virtio_cons.put_chars = put_chars;
- virtio_cons.notifier_add = notifier_add_vio;
- virtio_cons.notifier_del = notifier_del_vio;
- virtio_cons.notifier_hangup = notifier_del_vio;
-
- /* The first argument of hvc_alloc() is the virtual console number, so
- * we use zero. The second argument is the parameter for the
- * notification mechanism (like irq number). We currently leave this
- * as zero, virtqueues have implicit notifications.
- *
- * The third argument is a "struct hv_ops" containing the put_chars()
- * get_chars(), notifier_add() and notifier_del() pointers.
- * The final argument is the output buffer size: we can do any size,
- * so we put PAGE_SIZE here. */
- hvc = hvc_alloc(0, 0, &virtio_cons, PAGE_SIZE);
- if (IS_ERR(hvc)) {
- err = PTR_ERR(hvc);
- goto free_vqs;
- }
-
- /* Register the input buffer the first time. */
- add_inbuf();
return 0;

-free_vqs:
- vdev->config->del_vqs(vdev);
-free:
- kfree(inbuf);
fail:
- return err;
+ return ret;
+}
+
+/*
+ * Remove port-specific data.
+ * In case the port can't be removed, return non-zero. This could
+ * then be used in the port hot-unplug case.
+ */
+static int virtcons_remove_port_data(struct virtio_console_port *port)
+{
+ struct virtio_console_port_buffer *buf, *buf2;
+
+ if (is_console_port(port)) {
+ /* hvc_console is compiled in, at least on Fedora. */
+ /* hvc_remove(hvc); */
+ return 1;
+ }
+
+ sysfs_remove_group(&port->dev->kobj, &virtcon_attribute_group);
+ device_destroy(virtconsole.class, port->dev->devt);
+ unregister_chrdev_region(port->dev->devt, 1);
+ cdev_del(&port->cdev);
+
+ kfree(port->name);
+
+ /* Remove the buffers in which we have unconsumed data */
+ spin_lock(&port->readbuf_list_lock);
+ list_for_each_entry_safe(buf, buf2, &port->readbuf_head, next) {
+ list_del(&buf->next);
+ kfree(buf->buf);
+ kfree(buf);
+ }
+ spin_unlock(&port->readbuf_list_lock);
+ debugfs_remove(port->debugfs_file);
+ return 0;
+}
+
+static void virtcons_remove(struct virtio_device *vdev)
+{
+ struct virtio_console_port *port, *port2;
+ struct virtio_console_port_buffer *buf, *buf2;
+ char *tmpbuf;
+ unsigned int len;
+
+ class_destroy(virtconsole.class);
+
+ cancel_work_sync(&virtconsole.rx_work);
+ /*
+ * Free up the buffers that we queued up for the Host to pass
+ * us data
+ */
+ while ((tmpbuf = virtconsole.in_vq->vq_ops->get_buf(virtconsole.in_vq,
+ &len)))
+ kfree(tmpbuf);
+
+ vdev->config->del_vqs(vdev);
+ /*
+ * Free up the buffers that were sent to us by Host but were
+ * left unused
+ */
+ list_for_each_entry_safe(buf, buf2, &virtconsole.unused_read_head, next) {
+ list_del(&buf->next);
+ kfree(buf->buf);
+ kfree(buf);
+ }
+ list_for_each_entry_safe(buf, buf2, &virtconsole.unused_write_head, next) {
+ list_del(&buf->next);
+ kfree(buf->buf);
+ kfree(buf);
+ }
+ list_for_each_entry_safe(port, port2, &virtconsole.port_head, next) {
+ list_del(&port->next);
+ virtcons_remove_port_data(port);
+ kfree(port);
+ }
+ debugfs_remove_recursive(virtconsole.debugfs_dir);
}

static struct virtio_device_id id_table[] = {
@@ -253,6 +1372,7 @@ static struct virtio_device_id id_table[] = {

static unsigned int features[] = {
VIRTIO_CONSOLE_F_SIZE,
+ VIRTIO_CONSOLE_F_MULTIPORT,
};

static struct virtio_driver virtio_console = {
@@ -262,14 +1382,38 @@ static struct virtio_driver virtio_console = {
.driver.owner = THIS_MODULE,
.id_table = id_table,
.probe = virtcons_probe,
- .config_changed = virtcons_apply_config,
+ .remove = virtcons_remove,
+ .config_changed = config_intr,
};

static int __init init(void)
{
- return register_virtio_driver(&virtio_console);
+ int ret;
+
+ virtconsole.class = class_create(THIS_MODULE, "virtio-console");
+ if (IS_ERR(virtconsole.class)) {
+ pr_err("Error creating virtio-console class\n");
+ ret = PTR_ERR(virtconsole.class);
+ return ret;
+ }
+ ret = register_virtio_driver(&virtio_console);
+ if (ret) {
+ class_destroy(virtconsole.class);
+ return ret;
+ }
+ virtconsole.debugfs_dir = debugfs_create_dir("virtio-console", NULL);
+ if (!virtconsole.debugfs_dir)
+ pr_debug("virtio-console: Couldn't create debugfs dir, ret = %ld\n",
+ PTR_ERR(virtconsole.debugfs_dir));
+ return 0;
+}
+
+static void __exit fini(void)
+{
+ unregister_virtio_driver(&virtio_console);
}
module_init(init);
+module_exit(fini);

MODULE_DEVICE_TABLE(virtio, id_table);
MODULE_DESCRIPTION("Virtio console driver");
diff --git a/include/linux/virtio_console.h b/include/linux/virtio_console.h
index fe88517..335d787 100644
--- a/include/linux/virtio_console.h
+++ b/include/linux/virtio_console.h
@@ -3,20 +3,66 @@
#include <linux/types.h>
#include <linux/virtio_ids.h>
#include <linux/virtio_config.h>
-/* This header, excluding the #ifdef __KERNEL__ part, is BSD licensed so
- * anyone can use the definitions to implement compatible drivers/servers. */
+/*
+ * This header, excluding the #ifdef __KERNEL__ part, is BSD licensed so
+ * anyone can use the definitions to implement compatible drivers/servers.
+ *
+ * Copyright (C) Red Hat, Inc., 2009
+ */

/* Feature bits */
#define VIRTIO_CONSOLE_F_SIZE 0 /* Does host provide console size? */
+#define VIRTIO_CONSOLE_F_MULTIPORT 1 /* Does host provide multiple ports? */
+
+#define VIRTIO_CONSOLE_BAD_ID (~(u32)0) /* Invalid port number */

struct virtio_console_config {
/* colums of the screens */
__u16 cols;
/* rows of the screens */
__u16 rows;
+ /* number of ports in use */
+ __u32 nr_active_ports;
} __attribute__((packed));


+/*
+ * An internal-only message that's passed between the Host and the
+ * Guest for a particular port.
+ */
+struct virtio_console_control {
+ __u16 event;
+ __u16 value;
+};
+
+/* Some events for internal messages (control packets) */
+#define VIRTIO_CONSOLE_PORT_OPEN 0
+#define VIRTIO_CONSOLE_PORT_NAME 1
+#define VIRTIO_CONSOLE_CONSOLE_PORT 2
+#define VIRTIO_CONSOLE_CACHE_BUFFERS 3
+#define VIRTIO_CONSOLE_THROTTLE_PORT 4
+#define VIRTIO_CONSOLE_BUFFER_LIMIT 5
+
+/*
+ * This struct is put in each buffer that gets passed to userspace and
+ * vice-versa
+ */
+struct virtio_console_header {
+ /* Port number */
+ u32 id;
+ /* Some message between host and guest */
+ u32 flags;
+ /*
+ * Complete size of the write request - only sent with the
+ * first buffer for each write request
+ */
+ u32 size;
+} __attribute__((packed));
+
+/* Messages between host and guest ('flags' field in the header above) */
+#define VIRTIO_CONSOLE_ID_INTERNAL (1 << 0)
+
+
#ifdef __KERNEL__
int __init virtio_cons_early_init(int (*put_chars)(u32, const char *, int));
#endif /* __KERNEL__ */
--
1.6.2.5

2009-11-06 07:10:04

by Rusty Russell

[permalink] [raw]
Subject: Re: [PATCH v10 1/1] virtio_console: Add support for multiple ports for generic guest and host communication

On Wed, 4 Nov 2009 03:08:39 am Amit Shah wrote:
> Expose multiple char devices ("ports") for simple communication
> between the host userspace and guest.

OK, I've taken the chance to audit this patch. I started adding patches
until I got overwhelmed. It's a complete mess and needs a total rewrite :(

This shows the problem with feeding me a complete driver rewrite in one
big hit. You've combined lots of changes and techniques in here at once:

1) Moved work out of interrupt handlers and into work_struct.
2) Encapsulated the buffer information in a structure.
3) Added an optional header to the buffer(s).
4) Added control messages.
5) Encapsulated the per-port information into a structure.
6) Allocated a static buffer pool of over 4MB (!)
7) Added an ad-hoc throttling mechanism.
8) Added debugfs support.

Less important nitpicks:
1) Don't call a struct list_head 'next', it confused the crap out of me.
Call it list or some other non-name.
2) Don't call control messages "internal". Use ONE good name, ie. control.
3) Don't use list_for_each_safe() to get the head entry of a list. Your
use is buggy anyway: buf will never be NULL afterwards.

In summary: this is the one we're going to throw away.

Now I'm going to start again, one patch at a time, and see how that works.

Sorry,
Rusty.

2009-11-06 07:43:29

by Christian Borntraeger

[permalink] [raw]
Subject: Re: [PATCH v10 1/1] virtio_console: Add support for multiple ports for generic guest and host communication

Am Dienstag 03 November 2009 17:38:39 schrieb Amit Shah:

> drivers/char/Kconfig | 6 +
> drivers/char/virtio_console.c | 1412 ++++++++++++++++++++++++++++++++++++----
> include/linux/virtio_console.h | 50 ++-
> 3 files changed, 1332 insertions(+), 136 deletions(-)

I have not looked at your last two versions of the patch since you did not add
a changelog to your patch description. I thought you only did minor changes.
Turned out you added/changed several places. (e.g. VIRTIO_CONSOLE_CACHE_BUFFERS,
VIRTIO_CONSOLE_THROTTLE_PORT and VIRTIO_CONSOLE_BUFFER_LIMIT are all new)

When you do bigger changes can you add a changelog in your patch description in
the future?

2009-11-06 08:00:48

by Christian Borntraeger

[permalink] [raw]
Subject: Re: [PATCH v10 1/1] virtio_console: Add support for multiple ports for generic guest and host communication

Am Freitag 06 November 2009 08:10:02 schrieb Rusty Russell:
> On Wed, 4 Nov 2009 03:08:39 am Amit Shah wrote:
> > Expose multiple char devices ("ports") for simple communication
> > between the host userspace and guest.
>
> OK, I've taken the chance to audit this patch. I started adding patches
> until I got overwhelmed. It's a complete mess and needs a total rewrite
> :(

I know that Anthony disagrees, but _If we start over_, I still think we should
use that chance and leave the old virtio console untouched and add a new driver
for the host guest communication. IMHO it turned out that there is only a tiny
bit of commonality. (most code pathes check for use_multiport and then do two
completely different things).
I like simplicity. According to David A. Wheeler's SLOCCount, the old console
has 141 lines of code and the I truly believe that a separate guest-host comm
vehicle would also be a lot simpler if it must not take care of the old
virtio_console interface.

On the other hand we all should agree on one driver vs. two drivers before we go
on. Everything else would be unfair to Amit, who had the unpleasant task to
implement conflicting review comments....

Christian

2009-11-06 13:00:47

by Anthony Liguori

[permalink] [raw]
Subject: Re: [PATCH v10 1/1] virtio_console: Add support for multiple ports for generic guest and host communication

Christian Borntraeger wrote:
> I know that Anthony disagrees, but _If we start over_, I still think we should
> use that chance and leave the old virtio console untouched and add a new driver
> for the host guest communication. IMHO it turned out that there is only a tiny
> bit of commonality. (most code pathes check for use_multiport and then do two
> completely different things).
> I like simplicity. According to David A. Wheeler's SLOCCount, the old console
> has 141 lines of code and the I truly believe that a separate guest-host comm
> vehicle would also be a lot simpler if it must not take care of the old
> virtio_console interface.
>

It's the wrong metrics for evaluating a device ABI. We should consider
device ABIs based on whether they make sense--not about how many lines
of code it takes to implement the Linux driver.

Fundamentally speaking, right now, virtio-console is a single stream
that acts as an interactive terminal. What we're looking to add here is
to support multiple terminals that can be enumerated in a rationale way.

I see no reason why that should be two separate devices.

> On the other hand we all should agree on one driver vs. two drivers before we go
> on. Everything else would be unfair to Amit, who had the unpleasant task to
> implement conflicting review comments....
>

I agree and there are multiple maintainers on the qemu side who feel the
same way I do. I'm really strongly opposed to making this separate devices.

If you think it's easier, you can do a check in the virtio_probe
function that checks for the feature bits and calls a completely
separate virtio initialization routine so it ends up being two separate
files in Linux. But that's a Linux implementation detail.

> Christian
>


--
Regards,

Anthony Liguori

2009-11-06 14:12:38

by Christian Borntraeger

[permalink] [raw]
Subject: Re: [PATCH v10 1/1] virtio_console: Add support for multiple ports for generic guest and host communication

Am Freitag 06 November 2009 14:00:32 schrieb Anthony Liguori:
> > I like simplicity. According to David A. Wheeler's SLOCCount, the old
> > console has 141 lines of code and the I truly believe that a separate
> > guest-host comm vehicle would also be a lot simpler if it must not take
> > care of the old virtio_console interface.
>
> It's the wrong metrics for evaluating a device ABI. We should consider
> device ABIs based on whether they make sense--not about how many lines
> of code it takes to implement the Linux driver.

Well, code size is often related to complexity of the interface and affects
maintainability. But if that does not convince you, what about intended use and
semantics?

> Fundamentally speaking, right now, virtio-console is a single stream
> that acts as an interactive terminal. What we're looking to add here is
> to support multiple terminals that can be enumerated in a rationale way.

Right, that is a point where I disagree. The original purpose and intent of the
multiple port thing was to have a generic guest/host comm channels and *NOT* to
have multiple console devices. Having multiple console devices is just a fall-
out of the current implementation.

Following your argument about single-streaming, we could also merge virtio-rng,
no?
If a common interface for stream workload is desired I would have preferred a
write/read virtqueue_op besides or on top of add_buf/getbuf. I think that would
have been the right level of abstraction.

>I agree and there are multiple maintainers on the qemu side who feel the
>same way I do. I'm really strongly opposed to making this separate devices.

As a maintainer you sometimes have to make a controversial decision. If you made
this final decision (and Rusty agrees) I am fine, even if I disagree.

(If it turns out to be a wrong decision you have been warned. ;-) )

Christian

2009-11-06 14:32:12

by Anthony Liguori

[permalink] [raw]
Subject: Re: [PATCH v10 1/1] virtio_console: Add support for multiple ports for generic guest and host communication

Christian Borntraeger wrote:
>> Fundamentally speaking, right now, virtio-console is a single stream
>> that acts as an interactive terminal. What we're looking to add here is
>> to support multiple terminals that can be enumerated in a rationale way.
>>
>
> Right, that is a point where I disagree. The original purpose and intent of the
> multiple port thing was to have a generic guest/host comm channels and *NOT* to
> have multiple console devices. Having multiple console devices is just a fall-
> out of the current implementation.
>

This is part of what makes this series so difficult. As a generic
guest/host communications channel, this series does not at all meet
reasonable expectations. It makes no attempt to deal with things like
live migration. It can at best be described as a hack.

The whole thing's a mess because it's never been thought through
properly which is probably because the actual consumer of it has not
been released publicly.

I've always advocated that this use case would be better served as a
_userspace_ interface that implements a protocol on top of an arbitrary
transport. It could be a network socket, a virtio device (even
virtio-console unmodified), a real serial device, etc.

You can do multiplexing, connect/disconnect notification, etc. all as
elements of a streaming protocol. Throwing all of these channel
semantics coupled tightly with the transport and then trying to make a
kernel interface be what's the supported user interface is really just
asking for trouble.

--
Regards,

Anthony Liguori

2009-11-09 12:09:44

by Amit Shah

[permalink] [raw]
Subject: Re: [PATCH v10 1/1] virtio_console: Add support for multiple ports for generic guest and host communication

Hey Rusty,

On (Fri) Nov 06 2009 [17:40:02], Rusty Russell wrote:
> On Wed, 4 Nov 2009 03:08:39 am Amit Shah wrote:
> > Expose multiple char devices ("ports") for simple communication
> > between the host userspace and guest.
>
> OK, I've taken the chance to audit this patch. I started adding patches
> until I got overwhelmed. It's a complete mess and needs a total rewrite :(
>
> This shows the problem with feeding me a complete driver rewrite in one
> big hit. You've combined lots of changes and techniques in here at once:

I'm really sorry for dumping this all at once.

I've separated out the patches for easier reviewing, the result passes
my tests but I'm going to give them some more time to run.

> Less important nitpicks:

...

> 3) Don't use list_for_each_safe() to get the head entry of a list. Your
> use is buggy anyway: buf will never be NULL afterwards.

As long as 'head' isn't the only node in the list we should loop. And
list_foreach_safe() does that. So it's OK right?

> In summary: this is the one we're going to throw away.
>
> Now I'm going to start again, one patch at a time, and see how that works.

I'm sorry for having created more work for you; I'll send out my patches
to you in a couple of days.

Thanks!
Amit

2009-11-09 12:10:25

by Amit Shah

[permalink] [raw]
Subject: Re: [PATCH v10 1/1] virtio_console: Add support for multiple ports for generic guest and host communication

On (Fri) Nov 06 2009 [08:43:05], Christian Borntraeger wrote:
> Am Dienstag 03 November 2009 17:38:39 schrieb Amit Shah:
>
> > drivers/char/Kconfig | 6 +
> > drivers/char/virtio_console.c | 1412 ++++++++++++++++++++++++++++++++++++----
> > include/linux/virtio_console.h | 50 ++-
> > 3 files changed, 1332 insertions(+), 136 deletions(-)
>
> I have not looked at your last two versions of the patch since you did not add
> a changelog to your patch description. I thought you only did minor changes.
> Turned out you added/changed several places. (e.g. VIRTIO_CONSOLE_CACHE_BUFFERS,
> VIRTIO_CONSOLE_THROTTLE_PORT and VIRTIO_CONSOLE_BUFFER_LIMIT are all new)

I see v8 slipped out without a changelog; sorry for that. v9, v10 had
the changelogs but v8 had included the caching and throttling support.

Amit

2009-11-10 02:19:14

by Rusty Russell

[permalink] [raw]
Subject: Re: [PATCH v10 1/1] virtio_console: Add support for multiple ports for generic guest and host communication

On Mon, 9 Nov 2009 10:38:39 pm Amit Shah wrote:
> > 3) Don't use list_for_each_safe() to get the head entry of a list. Your
> > use is buggy anyway: buf will never be NULL afterwards.
>
> As long as 'head' isn't the only node in the list we should loop. And
> list_foreach_safe() does that. So it's OK right?

If there's more than one, buf will be == head.

> I'm sorry for having created more work for you; I'll send out my patches
> to you in a couple of days.

I spent some time on the weekend creating patches. They're not finished,
but they start the process of maturing the driver to where it can handle
multiple ports.

The next step is real buffer management (ie. the infrastructure for more than
one in flight, even though the normal console won't do this), then adding
headers.

I will post them now so you can take a look...
Rusty.