virtio: console: Return -EFAULT on copy_xx_user errors, allow larger writes

virtio: console: Return -EFAULT on copy_xx_user errors, allow larger writes

[Amit Shah]

Hey Rusty,

These updated patches in the series return -EFAULT on copy_xx_user
errors and also move the copy_from_user into fops_write() instead of it
being in send_buf. This enables send_buf to just read from kernel
buffers, making it simpler.

This also allows write()s to write more to the host in one go,
removingthe 4k limitation. I do limit the writes to 32k at once to not
put too much pressure on the GFP_KERNEL area.

Both of these were pointed out by Marcelo.

I'm including the relative diff between the version that's in linux-next
and the new version below.

Please apply,
Thanks.
		Amit.


diff --git a/drivers/char/virtio_console.c b/drivers/char/virtio_console.c
index b923b5c..2c2de35 100644
--- a/drivers/char/virtio_console.c
+++ b/drivers/char/virtio_console.c
@@ -162,9 +162,6 @@ struct port {
 	 */
 	spinlock_t inbuf_lock;
 
-	/* Buffer that's used to pass data from the guest to the host */
-	struct port_buffer *outbuf;
-
 	/* The IO vqs for this port */
 	struct virtqueue *in_vq, *out_vq;
 
@@ -399,43 +396,23 @@ static ssize_t send_control_msg(struct port *port, unsigned int event,
 	return 0;
 }
 
-static ssize_t send_buf(struct port *port, const char *in_buf, size_t in_count,
-			bool from_user)
+static ssize_t send_buf(struct port *port, void *in_buf, size_t in_count)
 {
 	struct scatterlist sg[1];
 	struct virtqueue *out_vq;
-	struct port_buffer *buf;
 	ssize_t ret;
-	unsigned int tmplen;
+	unsigned int len;
 
 	out_vq = port->out_vq;
-	buf = port->outbuf;
 
-	if (in_count > buf->size)
-		in_count = buf->size;
-
-	if (from_user) {
-		ret = copy_from_user(buf->buf, in_buf, in_count);
-	} 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, in_buf, in_count);
-		ret = 0; /* Emulate copy_from_user behaviour */
-	}
-	buf->len = in_count - ret;
-
-	sg_init_one(sg, buf->buf, buf->len);
-	ret = out_vq->vq_ops->add_buf(out_vq, sg, 1, 0, buf);
+	sg_init_one(sg, in_buf, in_count);
+	ret = out_vq->vq_ops->add_buf(out_vq, sg, 1, 0, in_buf);
 
 	/* Tell Host to go! */
 	out_vq->vq_ops->kick(out_vq);
 
 	if (ret < 0) {
-		buf->len = 0;
+		len = 0;
 		goto fail;
 	}
 
@@ -444,13 +421,11 @@ static ssize_t send_buf(struct port *port, const char *in_buf, size_t in_count,
 	 * sent. Also ensure we return to userspace the number of
 	 * bytes that were successfully consumed by the host.
 	 */
-	while (!out_vq->vq_ops->get_buf(out_vq, &tmplen))
+	while (!out_vq->vq_ops->get_buf(out_vq, &len))
 		cpu_relax();
-
-	buf->len = tmplen;
 fail:
 	/* We're expected to return the amount of data we wrote */
-	return buf->len;
+	return len;
 }
 
 /*
@@ -461,26 +436,25 @@ static ssize_t fill_readbuf(struct port *port, char *out_buf, size_t out_count,
 			    bool to_user)
 {
 	struct port_buffer *buf;
-	ssize_t ret;
 	unsigned long flags;
 
 	if (!out_count || !port_has_data(port))
 		return 0;
 
 	buf = port->inbuf;
-	if (out_count > buf->len - buf->offset)
-		out_count = buf->len - buf->offset;
+	out_count = min(out_count, buf->len - buf->offset);
 
 	if (to_user) {
+		ssize_t ret;
+
 		ret = copy_to_user(out_buf, buf->buf + buf->offset, out_count);
+		if (ret)
+			return -EFAULT;
 	} else {
 		memcpy(out_buf, buf->buf + buf->offset, out_count);
-		ret = 0; /* Emulate copy_to_user behaviour */
 	}
 
-	/* Return the number of bytes actually copied */
-	ret = out_count - ret;
-	buf->offset += ret;
+	buf->offset += out_count;
 
 	if (buf->offset == buf->len) {
 		/*
@@ -495,7 +469,8 @@ static ssize_t fill_readbuf(struct port *port, char *out_buf, size_t out_count,
 
 		spin_unlock_irqrestore(&port->inbuf_lock, flags);
 	}
-	return ret;
+	/* Return the number of bytes actually copied */
+	return out_count;
 }
 
 /* The condition that must be true for polling to end */
@@ -548,10 +523,27 @@ static ssize_t port_fops_write(struct file *filp, const char __user *ubuf,
 			       size_t count, loff_t *offp)
 {
 	struct port *port;
+	char *buf;
+	ssize_t ret;
 
 	port = filp->private_data;
 
-	return send_buf(port, ubuf, count, true);
+	count = min((size_t)(32 * 1024), count);
+
+	buf = kmalloc(count, GFP_KERNEL);
+	if (!buf)
+		return -ENOMEM;
+
+	ret = copy_from_user(buf, ubuf, count);
+	if (ret) {
+		ret = -EFAULT;
+		goto free_buf;
+	}
+
+	ret = send_buf(port, buf, count);
+free_buf:
+	kfree(buf);
+	return ret;
 }
 
 static unsigned int port_fops_poll(struct file *filp, poll_table *wait)
@@ -657,7 +649,7 @@ static int put_chars(u32 vtermno, const char *buf, int count)
 	if (unlikely(early_put_chars))
 		return early_put_chars(vtermno, buf, count);
 
-	return send_buf(port, buf, count, false);
+	return send_buf(port, (void *)buf, count);
 }
 
 /*
@@ -874,7 +866,6 @@ static int remove_port(struct port *port)
 	cdev_del(&port->cdev);
 
 	discard_port_data(port);
-	free_buf(port->outbuf);
 	kfree(port->name);
 
 	debugfs_remove(port->debugfs_file);
@@ -1143,11 +1134,6 @@ static int add_port(struct ports_device *portdev, u32 id)
 		err = -ENOMEM;
 		goto free_device;
 	}
-	port->outbuf = alloc_buf(PAGE_SIZE);
-	if (!port->outbuf) {
-		err = -ENOMEM;
-		goto free_inbuf;
-	}
 
 	/* Register the input buffer the first time. */
 	add_inbuf(port->in_vq, inbuf);
@@ -1158,7 +1144,7 @@ static int add_port(struct ports_device *portdev, u32 id)
 	if (!use_multiport(port->portdev)) {
 		err = init_port_console(port);
 		if (err)
-			goto free_outbuf;
+			goto free_inbuf;
 	}
 
 	spin_lock_irq(&portdev->ports_lock);
@@ -1186,8 +1172,6 @@ static int add_port(struct ports_device *portdev, u32 id)
 	}
 	return 0;
 
-free_outbuf:
-	free_buf(port->outbuf);
 free_inbuf:
 	free_buf(inbuf);
 free_device:

[PATCH 19/31] virtio: console: Introduce a send_buf function for a common path for sending data to host

[Amit Shah]

Adding support for generic ports that will write to userspace will need
some code changes.

Consolidate the write routine into send_buf() and put_chars() now just
calls into the new function.

Signed-off-by: Amit Shah <amit.shah@redhat.com>
---
 drivers/char/virtio_console.c |   50 +++++++++++++++++++++++++++--------------
 1 files changed, 33 insertions(+), 17 deletions(-)

diff --git a/drivers/char/virtio_console.c b/drivers/char/virtio_console.c
index 5096d92..d010510 100644
--- a/drivers/char/virtio_console.c
+++ b/drivers/char/virtio_console.c
@@ -233,6 +233,38 @@ static bool port_has_data(struct port *port)
 	return ret;
 }
 
+static ssize_t send_buf(struct port *port, void *in_buf, size_t in_count)
+{
+	struct scatterlist sg[1];
+	struct virtqueue *out_vq;
+	ssize_t ret;
+	unsigned int len;
+
+	out_vq = port->out_vq;
+
+	sg_init_one(sg, in_buf, in_count);
+	ret = out_vq->vq_ops->add_buf(out_vq, sg, 1, 0, in_buf);
+
+	/* Tell Host to go! */
+	out_vq->vq_ops->kick(out_vq);
+
+	if (ret < 0) {
+		len = 0;
+		goto fail;
+	}
+
+	/*
+	 * Wait till the host acknowledges it pushed out the data we
+	 * sent. Also ensure we return to userspace the number of
+	 * bytes that were successfully consumed by the host.
+	 */
+	while (!out_vq->vq_ops->get_buf(out_vq, &len))
+		cpu_relax();
+fail:
+	/* We're expected to return the amount of data we wrote */
+	return len;
+}
+
 /*
  * Give out the data that's requested from the buffer that we have
  * queued up.
@@ -280,10 +312,7 @@ static ssize_t fill_readbuf(struct port *port, char *out_buf, size_t out_count)
  */
 static int put_chars(u32 vtermno, const char *buf, int count)
 {
-	struct scatterlist sg[1];
 	struct port *port;
-	struct virtqueue *out_vq;
-	unsigned int len;
 
 	port = find_port_by_vtermno(vtermno);
 	if (!port)
@@ -292,20 +321,7 @@ static int put_chars(u32 vtermno, const char *buf, int count)
 	if (unlikely(early_put_chars))
 		return early_put_chars(vtermno, buf, count);
 
-	out_vq = port->out_vq;
-	/* This is a convenient routine to initialize a single-elem sg list */
-	sg_init_one(sg, buf, count);
-
-	/* This shouldn't fail: if it does, we lose chars. */
-	if (out_vq->vq_ops->add_buf(out_vq, sg, 1, 0, port) >= 0) {
-		/* Tell Host to go! */
-		out_vq->vq_ops->kick(out_vq);
-		while (!out_vq->vq_ops->get_buf(out_vq, &len))
-			cpu_relax();
-	}
-
-	/* We're expected to return the amount of data we wrote: all of it. */
-	return count;
+	return send_buf(port, (void *)buf, count);
 }
 
 /*
-- 
1.6.2.5

[PATCH 20/31] virtio: console: Add a new MULTIPORT feature, support for generic ports

[Amit Shah]

This commit adds a new feature, MULTIPORT. If the host supports this
feature as well, the config space has the number of ports defined for
that device. New ports are spawned according to this information.

The config space also has the maximum number of ports that can be
spawned for a particular device. This is useful in initializing the
appropriate number of virtqueues in advance, as ports might be
hot-plugged in later.

Using this feature, generic ports can be created which are not tied to
hvc consoles.

We also open up a private channel between the host and the guest via
which some "control" messages are exchanged for the ports, like whether
the port being spawned is a console port, resizing the console window,
etc.

Next commits will add support for hotplugging and presenting char
devices in /dev/ for bi-directional guest-host communication.

Signed-off-by: Amit Shah <amit.shah@redhat.com>
---
 drivers/char/virtio_console.c  |  405 ++++++++++++++++++++++++++++++++++------
 include/linux/virtio_console.h |   21 ++
 2 files changed, 370 insertions(+), 56 deletions(-)

diff --git a/drivers/char/virtio_console.c b/drivers/char/virtio_console.c
index d010510..9d33239 100644
--- a/drivers/char/virtio_console.c
+++ b/drivers/char/virtio_console.c
@@ -1,5 +1,6 @@
 /*
  * Copyright (C) 2006, 2007, 2009 Rusty Russell, IBM Corporation
+ * Copyright (C) 2009, 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
@@ -21,6 +22,7 @@
 #include <linux/spinlock.h>
 #include <linux/virtio.h>
 #include <linux/virtio_console.h>
+#include <linux/workqueue.h>
 #include "hvc_console.h"
 
 /*
@@ -69,17 +71,6 @@ struct console {
 	u32 vtermno;
 };
 
-/*
- * This is a per-device struct that stores data common to all the
- * ports for that device (vdev->priv).
- */
-struct ports_device {
-	/* Array of per-port IO virtqueues */
-	struct virtqueue **in_vqs, **out_vqs;
-
-	struct virtio_device *vdev;
-};
-
 struct port_buffer {
 	char *buf;
 
@@ -92,8 +83,49 @@ struct port_buffer {
 	size_t offset;
 };
 
+/*
+ * This is a per-device struct that stores data common to all the
+ * ports for that device (vdev->priv).
+ */
+struct ports_device {
+	/*
+	 * Workqueue handlers where we process deferred work after
+	 * notification
+	 */
+	struct work_struct control_work;
+
+	struct list_head ports;
+
+	/* To protect the list of ports */
+	spinlock_t ports_lock;
+
+	/* To protect the vq operations for the control channel */
+	spinlock_t cvq_lock;
+
+	/* The current config space is stored here */
+	struct virtio_console_config config;
+
+	/* The virtio device we're associated with */
+	struct virtio_device *vdev;
+
+	/*
+	 * A couple of virtqueues for the control channel: one for
+	 * guest->host transfers, one for host->guest transfers
+	 */
+	struct virtqueue *c_ivq, *c_ovq;
+
+	/* Array of per-port IO virtqueues */
+	struct virtqueue **in_vqs, **out_vqs;
+
+	/* The control messages to the Host are sent via this buffer */
+	struct port_buffer *outbuf;
+};
+
 /* This struct holds the per-port data */
 struct port {
+	/* Next port in the list, head is in the ports_device */
+	struct list_head list;
+
 	/* Pointer to the parent virtio_console device */
 	struct ports_device *portdev;
 
@@ -115,6 +147,9 @@ struct port {
 	 * hooked up to an hvc console
 	 */
 	struct console cons;
+
+	/* The 'id' to identify the port with the Host */
+	u32 id;
 };
 
 /* This is the very early arch-specified put chars function. */
@@ -139,25 +174,56 @@ out:
 	return port;
 }
 
+static struct port *find_port_by_id(struct ports_device *portdev, u32 id)
+{
+	struct port *port;
+	unsigned long flags;
+
+	spin_lock_irqsave(&portdev->ports_lock, flags);
+	list_for_each_entry(port, &portdev->ports, list)
+		if (port->id == id)
+			goto out;
+	port = NULL;
+out:
+	spin_unlock_irqrestore(&portdev->ports_lock, flags);
+
+	return port;
+}
+
 static struct port *find_port_by_vq(struct ports_device *portdev,
 				    struct virtqueue *vq)
 {
 	struct port *port;
-	struct console *cons;
 	unsigned long flags;
 
-	spin_lock_irqsave(&pdrvdata_lock, flags);
-	list_for_each_entry(cons, &pdrvdata.consoles, list) {
-		port = container_of(cons, struct port, cons);
+	spin_lock_irqsave(&portdev->ports_lock, flags);
+	list_for_each_entry(port, &portdev->ports, list)
 		if (port->in_vq == vq || port->out_vq == vq)
 			goto out;
-	}
 	port = NULL;
 out:
-	spin_unlock_irqrestore(&pdrvdata_lock, flags);
+	spin_unlock_irqrestore(&portdev->ports_lock, flags);
 	return port;
 }
 
+static bool is_console_port(struct port *port)
+{
+	if (port->cons.hvc)
+		return true;
+	return false;
+}
+
+static inline bool use_multiport(struct ports_device *portdev)
+{
+	/*
+	 * This condition can be true when put_chars is called from
+	 * early_init
+	 */
+	if (!portdev->vdev)
+		return 0;
+	return portdev->vdev->features[0] & (1 << VIRTIO_CONSOLE_F_MULTIPORT);
+}
+
 static void free_buf(struct port_buffer *buf)
 {
 	kfree(buf->buf);
@@ -233,6 +299,36 @@ static bool port_has_data(struct port *port)
 	return ret;
 }
 
+static ssize_t send_control_msg(struct port *port, unsigned int event,
+				unsigned int value)
+{
+	struct scatterlist sg[1];
+	struct virtio_console_control cpkt;
+	struct virtqueue *vq;
+	struct port_buffer *outbuf;
+	int tmplen;
+
+	if (!use_multiport(port->portdev))
+		return 0;
+
+	cpkt.id = port->id;
+	cpkt.event = event;
+	cpkt.value = value;
+
+	vq = port->portdev->c_ovq;
+	outbuf = port->portdev->outbuf;
+
+	memcpy(outbuf->buf, (void *)&cpkt, sizeof(cpkt));
+
+	sg_init_one(sg, outbuf->buf, sizeof(cpkt));
+	if (vq->vq_ops->add_buf(vq, sg, 1, 0, outbuf) >= 0) {
+		vq->vq_ops->kick(vq);
+		while (!vq->vq_ops->get_buf(vq, &tmplen))
+			cpu_relax();
+	}
+	return 0;
+}
+
 static ssize_t send_buf(struct port *port, void *in_buf, size_t in_count)
 {
 	struct scatterlist sg[1];
@@ -387,24 +483,7 @@ static void notifier_del_vio(struct hvc_struct *hp, int data)
 	hp->irq_requested = 0;
 }
 
-static void hvc_handle_input(struct virtqueue *vq)
-{
-	struct port *port;
-	unsigned long flags;
-
-	port = find_port_by_vq(vq->vdev->priv, vq);
-	if (!port)
-		return;
-
-	spin_lock_irqsave(&port->inbuf_lock, flags);
-	port->inbuf = get_inbuf(port);
-	spin_unlock_irqrestore(&port->inbuf_lock, flags);
-
-	if (hvc_poll(port->cons.hvc))
-		hvc_kick();
-}
-
-/* The operations for the console. */
+/* The operations for console ports. */
 static const struct hv_ops hv_ops = {
 	.get_chars = get_chars,
 	.put_chars = put_chars,
@@ -428,7 +507,7 @@ int __init virtio_cons_early_init(int (*put_chars)(u32, const char *, int))
 	return hvc_instantiate(0, 0, &hv_ops);
 }
 
-int __devinit init_port_console(struct port *port)
+int init_port_console(struct port *port)
 {
 	int ret;
 
@@ -465,7 +544,122 @@ int __devinit init_port_console(struct port *port)
 	return 0;
 }
 
-static int __devinit add_port(struct ports_device *portdev)
+/* Any private messages that the Host and Guest want to share */
+static void handle_control_message(struct ports_device *portdev,
+				   struct port_buffer *buf)
+{
+	struct virtio_console_control *cpkt;
+	struct port *port;
+
+	cpkt = (struct virtio_console_control *)(buf->buf + buf->offset);
+
+	port = find_port_by_id(portdev, cpkt->id);
+	if (!port) {
+		/* No valid header at start of buffer.  Drop it. */
+		dev_dbg(&portdev->vdev->dev,
+			"Invalid index %u in control packet\n", cpkt->id);
+		return;
+	}
+
+	switch (cpkt->event) {
+	case VIRTIO_CONSOLE_CONSOLE_PORT:
+		if (!cpkt->value)
+			break;
+		if (is_console_port(port))
+			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_RESIZE:
+		if (!is_console_port(port))
+			break;
+		port->cons.hvc->irq_requested = 1;
+		resize_console(port);
+		break;
+	}
+}
+
+static void control_work_handler(struct work_struct *work)
+{
+	struct ports_device *portdev;
+	struct virtqueue *vq;
+	struct port_buffer *buf;
+	unsigned int len;
+
+	portdev = container_of(work, struct ports_device, control_work);
+	vq = portdev->c_ivq;
+
+	spin_lock(&portdev->cvq_lock);
+	while ((buf = vq->vq_ops->get_buf(vq, &len))) {
+		spin_unlock(&portdev->cvq_lock);
+
+		buf->len = len;
+		buf->offset = 0;
+
+		handle_control_message(portdev, buf);
+
+		spin_lock(&portdev->cvq_lock);
+		if (add_inbuf(portdev->c_ivq, buf) < 0) {
+			dev_warn(&portdev->vdev->dev,
+				 "Error adding buffer to queue\n");
+			free_buf(buf);
+		}
+	}
+	spin_unlock(&portdev->cvq_lock);
+}
+
+static void in_intr(struct virtqueue *vq)
+{
+	struct port *port;
+	unsigned long flags;
+
+	port = find_port_by_vq(vq->vdev->priv, vq);
+	if (!port)
+		return;
+
+	spin_lock_irqsave(&port->inbuf_lock, flags);
+	port->inbuf = get_inbuf(port);
+
+	spin_unlock_irqrestore(&port->inbuf_lock, flags);
+
+	if (is_console_port(port) && hvc_poll(port->cons.hvc))
+		hvc_kick();
+}
+
+static void control_intr(struct virtqueue *vq)
+{
+	struct ports_device *portdev;
+
+	portdev = vq->vdev->priv;
+	schedule_work(&portdev->control_work);
+}
+
+static void fill_queue(struct virtqueue *vq, spinlock_t *lock)
+{
+	struct port_buffer *buf;
+	int ret;
+
+	do {
+		buf = alloc_buf(PAGE_SIZE);
+		if (!buf)
+			break;
+
+		spin_lock_irq(lock);
+		ret = add_inbuf(vq, buf);
+		if (ret < 0) {
+			spin_unlock_irq(lock);
+			free_buf(buf);
+			break;
+		}
+		spin_unlock_irq(lock);
+	} while (ret > 0);
+}
+
+static int add_port(struct ports_device *portdev, u32 id)
 {
 	struct port *port;
 	struct port_buffer *inbuf;
@@ -478,11 +672,13 @@ static int __devinit add_port(struct ports_device *portdev)
 	}
 
 	port->portdev = portdev;
+	port->id = id;
 
 	port->inbuf = NULL;
+	port->cons.hvc = NULL;
 
-	port->in_vq = portdev->in_vqs[0];
-	port->out_vq = portdev->out_vqs[0];
+	port->in_vq = portdev->in_vqs[port->id];
+	port->out_vq = portdev->out_vqs[port->id];
 
 	spin_lock_init(&port->inbuf_lock);
 
@@ -495,9 +691,25 @@ static int __devinit add_port(struct ports_device *portdev)
 	/* Register the input buffer the first time. */
 	add_inbuf(port->in_vq, inbuf);
 
-	err = init_port_console(port);
-	if (err)
-		goto free_inbuf;
+	/*
+	 * If we're not using multiport support, this has to be a console port
+	 */
+	if (!use_multiport(port->portdev)) {
+		err = init_port_console(port);
+		if (err)
+			goto free_inbuf;
+	}
+
+	spin_lock_irq(&portdev->ports_lock);
+	list_add_tail(&port->list, &port->portdev->ports);
+	spin_unlock_irq(&portdev->ports_lock);
+
+	/*
+	 * Tell the Host we're set so that it can send us various
+	 * configuration parameters for this port (eg, port name,
+	 * caching, whether this is a console port, etc.)
+	 */
+	send_control_msg(port, VIRTIO_CONSOLE_PORT_READY, 1);
 
 	return 0;
 
@@ -514,12 +726,11 @@ static int init_vqs(struct ports_device *portdev)
 	vq_callback_t **io_callbacks;
 	char **io_names;
 	struct virtqueue **vqs;
-	u32 nr_ports, nr_queues;
+	u32 i, j, nr_ports, nr_queues;
 	int err;
 
-	/* We currently only have one port and two queues for that port */
-	nr_ports = 1;
-	nr_queues = 2;
+	nr_ports = portdev->config.max_nr_ports;
+	nr_queues = use_multiport(portdev) ? (nr_ports + 1) * 2 : 2;
 
 	vqs = kmalloc(nr_queues * sizeof(struct virtqueue *), GFP_KERNEL);
 	if (!vqs) {
@@ -549,11 +760,32 @@ static int init_vqs(struct ports_device *portdev)
 		goto free_invqs;
 	}
 
-	io_callbacks[0] = hvc_handle_input;
-	io_callbacks[1] = NULL;
-	io_names[0] = "input";
-	io_names[1] = "output";
-
+	/*
+	 * For backward compat (newer host but older guest), the host
+	 * spawns a console port first and also inits the vqs for port
+	 * 0 before others.
+	 */
+	j = 0;
+	io_callbacks[j] = in_intr;
+	io_callbacks[j + 1] = NULL;
+	io_names[j] = "input";
+	io_names[j + 1] = "output";
+	j += 2;
+
+	if (use_multiport(portdev)) {
+		io_callbacks[j] = control_intr;
+		io_callbacks[j + 1] = NULL;
+		io_names[j] = "control-i";
+		io_names[j + 1] = "control-o";
+
+		for (i = 1; i < nr_ports; i++) {
+			j += 2;
+			io_callbacks[j] = in_intr;
+			io_callbacks[j + 1] = NULL;
+			io_names[j] = "input";
+			io_names[j + 1] = "output";
+		}
+	}
 	/* Find the queues. */
 	err = portdev->vdev->config->find_vqs(portdev->vdev, nr_queues, vqs,
 					      io_callbacks,
@@ -561,9 +793,20 @@ static int init_vqs(struct ports_device *portdev)
 	if (err)
 		goto free_outvqs;
 
+	j = 0;
 	portdev->in_vqs[0] = vqs[0];
 	portdev->out_vqs[0] = vqs[1];
-
+	j += 2;
+	if (use_multiport(portdev)) {
+		portdev->c_ivq = vqs[j];
+		portdev->c_ovq = vqs[j + 1];
+
+		for (i = 1; i < nr_ports; i++) {
+			j += 2;
+			portdev->in_vqs[i] = vqs[j];
+			portdev->out_vqs[i] = vqs[j + 1];
+		}
+	}
 	kfree(io_callbacks);
 	kfree(io_names);
 	kfree(vqs);
@@ -587,11 +830,17 @@ fail:
 /*
  * Once we're further in boot, we get probed like any other virtio
  * device.
+ *
+ * If the host also supports multiple console ports, we check the
+ * config space to see how many ports the host has spawned.  We
+ * initialize each port found.
  */
 static int __devinit virtcons_probe(struct virtio_device *vdev)
 {
 	struct ports_device *portdev;
+	u32 i;
 	int err;
+	bool multiport;
 
 	portdev = kmalloc(sizeof(*portdev), GFP_KERNEL);
 	if (!portdev) {
@@ -603,16 +852,59 @@ static int __devinit virtcons_probe(struct virtio_device *vdev)
 	portdev->vdev = vdev;
 	vdev->priv = portdev;
 
+	multiport = false;
+	portdev->config.nr_ports = 1;
+	portdev->config.max_nr_ports = 1;
+	if (virtio_has_feature(vdev, VIRTIO_CONSOLE_F_MULTIPORT)) {
+		multiport = true;
+		vdev->features[0] |= 1 << VIRTIO_CONSOLE_F_MULTIPORT;
+
+		vdev->config->get(vdev, offsetof(struct virtio_console_config,
+						 nr_ports),
+				  &portdev->config.nr_ports,
+				  sizeof(portdev->config.nr_ports));
+		vdev->config->get(vdev, offsetof(struct virtio_console_config,
+						 max_nr_ports),
+				  &portdev->config.max_nr_ports,
+				  sizeof(portdev->config.max_nr_ports));
+		if (portdev->config.nr_ports > portdev->config.max_nr_ports) {
+			dev_warn(&vdev->dev,
+				 "More ports (%u) specified than allowed (%u). Will init %u ports.",
+				 portdev->config.nr_ports,
+				 portdev->config.max_nr_ports,
+				 portdev->config.max_nr_ports);
+
+			portdev->config.nr_ports = portdev->config.max_nr_ports;
+		}
+	}
+
+	/* Let the Host know we support multiple ports.*/
+	vdev->config->finalize_features(vdev);
+
 	err = init_vqs(portdev);
 	if (err < 0) {
 		dev_err(&vdev->dev, "Error %d initializing vqs\n", err);
 		goto free;
 	}
 
-	/* We only have one port. */
-	err = add_port(portdev);
-	if (err)
-		goto free_vqs;
+	spin_lock_init(&portdev->ports_lock);
+	INIT_LIST_HEAD(&portdev->ports);
+
+	if (multiport) {
+		spin_lock_init(&portdev->cvq_lock);
+		INIT_WORK(&portdev->control_work, &control_work_handler);
+
+		portdev->outbuf = alloc_buf(PAGE_SIZE);
+		if (!portdev->outbuf) {
+			err = -ENOMEM;
+			dev_err(&vdev->dev, "OOM for control outbuf\n");
+			goto free_vqs;
+		}
+		fill_queue(portdev->c_ivq, &portdev->cvq_lock);
+	}
+
+	for (i = 0; i < portdev->config.nr_ports; i++)
+		add_port(portdev, i);
 
 	/* Start using the new console output. */
 	early_put_chars = NULL;
@@ -635,6 +927,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 = {
diff --git a/include/linux/virtio_console.h b/include/linux/virtio_console.h
index 9e0da40..cada769 100644
--- a/include/linux/virtio_console.h
+++ b/include/linux/virtio_console.h
@@ -6,18 +6,39 @@
 /*
  * 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? */
 
 struct virtio_console_config {
 	/* colums of the screens */
 	__u16 cols;
 	/* rows of the screens */
 	__u16 rows;
+	/* max. number of ports this device can hold */
+	__u32 max_nr_ports;
+	/* number of ports added so far */
+	__u32 nr_ports;
 } __attribute__((packed));
 
+/*
+ * A message that's passed between the Host and the Guest for a
+ * particular port.
+ */
+struct virtio_console_control {
+	__u32 id;		/* Port number */
+	__u16 event;		/* The kind of control event (see below) */
+	__u16 value;		/* Extra information for the key */
+};
+
+/* Some events for control messages */
+#define VIRTIO_CONSOLE_PORT_READY	0
+#define VIRTIO_CONSOLE_CONSOLE_PORT	1
+#define VIRTIO_CONSOLE_RESIZE		2
 
 #ifdef __KERNEL__
 int __init virtio_cons_early_init(int (*put_chars)(u32, const char *, int));
-- 
1.6.2.5

[PATCH 21/31] virtio: console: Prepare for writing to userspace buffers

[Amit Shah]

When ports get advertised as char devices, the buffers will come from
userspace. Equip the fill_readbuf function with the ability to write
to userspace buffers.

Signed-off-by: Amit Shah <amit.shah@redhat.com>
---
 drivers/char/virtio_console.c |   20 ++++++++++++++------
 1 files changed, 14 insertions(+), 6 deletions(-)

diff --git a/drivers/char/virtio_console.c b/drivers/char/virtio_console.c
index 9d33239..5f61021 100644
--- a/drivers/char/virtio_console.c
+++ b/drivers/char/virtio_console.c
@@ -365,7 +365,8 @@ fail:
  * Give out the data that's requested from the buffer that we have
  * queued up.
  */
-static ssize_t fill_readbuf(struct port *port, char *out_buf, size_t out_count)
+static ssize_t fill_readbuf(struct port *port, char *out_buf, size_t out_count,
+			    bool to_user)
 {
 	struct port_buffer *buf;
 	unsigned long flags;
@@ -374,12 +375,18 @@ static ssize_t fill_readbuf(struct port *port, char *out_buf, size_t out_count)
 		return 0;
 
 	buf = port->inbuf;
-	if (out_count > buf->len - buf->offset)
-		out_count = buf->len - buf->offset;
+	out_count = min(out_count, buf->len - buf->offset);
 
-	memcpy(out_buf, buf->buf + buf->offset, out_count);
+	if (to_user) {
+		ssize_t ret;
+
+		ret = copy_to_user(out_buf, buf->buf + buf->offset, out_count);
+		if (ret)
+			return -EFAULT;
+	} else {
+		memcpy(out_buf, buf->buf + buf->offset, out_count);
+	}
 
-	/* Return the number of bytes actually copied */
 	buf->offset += out_count;
 
 	if (buf->offset == buf->len) {
@@ -395,6 +402,7 @@ static ssize_t fill_readbuf(struct port *port, char *out_buf, size_t out_count)
 
 		spin_unlock_irqrestore(&port->inbuf_lock, flags);
 	}
+	/* Return the number of bytes actually copied */
 	return out_count;
 }
 
@@ -438,7 +446,7 @@ static int get_chars(u32 vtermno, char *buf, int count)
 	/* If we don't have an input queue yet, we can't get input. */
 	BUG_ON(!port->in_vq);
 
-	return fill_readbuf(port, buf, count);
+	return fill_readbuf(port, buf, count, false);
 }
 
 static void resize_console(struct port *port)
-- 
1.6.2.5

[PATCH 22/31] virtio: console: Associate each port with a char device

[Amit Shah]

The char device will be used as an interface by applications on the
guest to communicate with apps on the host.

The devices created are placed in /dev/vportNpn where N is the
virtio-console device number and n is the port number for that device.

One dynamic major device number is allocated for each device and minor
numbers are allocated for the ports contained within that device.

The file operation for the char devs will be added in the following
commits.

Signed-off-by: Amit Shah <amit.shah@redhat.com>
---
 drivers/char/Kconfig          |    8 ++++
 drivers/char/virtio_console.c |   77 +++++++++++++++++++++++++++++++++++++++--
 2 files changed, 82 insertions(+), 3 deletions(-)

diff --git a/drivers/char/Kconfig b/drivers/char/Kconfig
index 31be3ac..bcedee1 100644
--- a/drivers/char/Kconfig
+++ b/drivers/char/Kconfig
@@ -666,6 +666,14 @@ 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/vportNpn will be created when corresponding ports are
+	  found, where N is the device number and n is the port number
+	  within that device.  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 the device.
 
 config HVCS
 	tristate "IBM Hypervisor Virtual Console Server support"
diff --git a/drivers/char/virtio_console.c b/drivers/char/virtio_console.c
index 5f61021..6aa400e 100644
--- a/drivers/char/virtio_console.c
+++ b/drivers/char/virtio_console.c
@@ -16,6 +16,8 @@
  * 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/device.h>
 #include <linux/err.h>
 #include <linux/init.h>
 #include <linux/list.h>
@@ -34,6 +36,12 @@
  * across multiple devices and multiple ports per device.
  */
 struct ports_driver_data {
+	/* Used for registering chardevs */
+	struct class *class;
+
+	/* Number of devices this driver is handling */
+	unsigned int index;
+
 	/*
 	 * This is used to keep track of the number of hvc consoles
 	 * spawned by this driver.  This number is given as the first
@@ -119,6 +127,12 @@ struct ports_device {
 
 	/* The control messages to the Host are sent via this buffer */
 	struct port_buffer *outbuf;
+
+	/* Used for numbering devices for sysfs and debugfs */
+	unsigned int drv_index;
+
+	/* Major number for this device.  Ports will be created as minors. */
+	int chr_major;
 };
 
 /* This struct holds the per-port data */
@@ -148,6 +162,10 @@ struct port {
 	 */
 	struct console cons;
 
+	/* Each port associates with a separate char device */
+	struct cdev cdev;
+	struct device *dev;
+
 	/* The 'id' to identify the port with the Host */
 	u32 id;
 };
@@ -398,7 +416,7 @@ static ssize_t fill_readbuf(struct port *port, char *out_buf, size_t out_count,
 		port->inbuf = NULL;
 
 		if (add_inbuf(port->in_vq, buf) < 0)
-			dev_warn(&port->portdev->vdev->dev, "failed add_buf\n");
+			dev_warn(port->dev, "failed add_buf\n");
 
 		spin_unlock_irqrestore(&port->inbuf_lock, flags);
 	}
@@ -671,6 +689,7 @@ static int add_port(struct ports_device *portdev, u32 id)
 {
 	struct port *port;
 	struct port_buffer *inbuf;
+	dev_t devt;
 	int err;
 
 	port = kmalloc(sizeof(*port), GFP_KERNEL);
@@ -688,12 +707,32 @@ static int add_port(struct ports_device *portdev, u32 id)
 	port->in_vq = portdev->in_vqs[port->id];
 	port->out_vq = portdev->out_vqs[port->id];
 
+	cdev_init(&port->cdev, NULL);
+
+	devt = MKDEV(portdev->chr_major, id);
+	err = cdev_add(&port->cdev, devt, 1);
+	if (err < 0) {
+		dev_err(&port->portdev->vdev->dev,
+			"Error %d adding cdev for port %u\n", err, id);
+		goto free_port;
+	}
+	port->dev = device_create(pdrvdata.class, &port->portdev->vdev->dev,
+				  devt, port, "vport%up%u",
+				  port->portdev->drv_index, id);
+	if (IS_ERR(port->dev)) {
+		err = PTR_ERR(port->dev);
+		dev_err(&port->portdev->vdev->dev,
+			"Error %d creating device for port %u\n",
+			err, id);
+		goto free_cdev;
+	}
+
 	spin_lock_init(&port->inbuf_lock);
 
 	inbuf = alloc_buf(PAGE_SIZE);
 	if (!inbuf) {
 		err = -ENOMEM;
-		goto free_port;
+		goto free_device;
 	}
 
 	/* Register the input buffer the first time. */
@@ -723,6 +762,10 @@ static int add_port(struct ports_device *portdev, u32 id)
 
 free_inbuf:
 	free_buf(inbuf);
+free_device:
+	device_destroy(pdrvdata.class, port->dev->devt);
+free_cdev:
+	cdev_del(&port->cdev);
 free_port:
 	kfree(port);
 fail:
@@ -835,6 +878,10 @@ fail:
 	return err;
 }
 
+static const struct file_operations portdev_fops = {
+	.owner = THIS_MODULE,
+};
+
 /*
  * Once we're further in boot, we get probed like any other virtio
  * device.
@@ -860,6 +907,20 @@ static int __devinit virtcons_probe(struct virtio_device *vdev)
 	portdev->vdev = vdev;
 	vdev->priv = portdev;
 
+	spin_lock_irq(&pdrvdata_lock);
+	portdev->drv_index = pdrvdata.index++;
+	spin_unlock_irq(&pdrvdata_lock);
+
+	portdev->chr_major = register_chrdev(0, "virtio-portsdev",
+					     &portdev_fops);
+	if (portdev->chr_major < 0) {
+		dev_err(&vdev->dev,
+			"Error %d registering chrdev for device %u\n",
+			portdev->chr_major, portdev->drv_index);
+		err = portdev->chr_major;
+		goto free;
+	}
+
 	multiport = false;
 	portdev->config.nr_ports = 1;
 	portdev->config.max_nr_ports = 1;
@@ -892,7 +953,7 @@ static int __devinit virtcons_probe(struct virtio_device *vdev)
 	err = init_vqs(portdev);
 	if (err < 0) {
 		dev_err(&vdev->dev, "Error %d initializing vqs\n", err);
-		goto free;
+		goto free_chrdev;
 	}
 
 	spin_lock_init(&portdev->ports_lock);
@@ -922,6 +983,8 @@ free_vqs:
 	vdev->config->del_vqs(vdev);
 	kfree(portdev->in_vqs);
 	kfree(portdev->out_vqs);
+free_chrdev:
+	unregister_chrdev(portdev->chr_major, "virtio-portsdev");
 free:
 	kfree(portdev);
 fail:
@@ -950,6 +1013,14 @@ static struct virtio_driver virtio_console = {
 
 static int __init init(void)
 {
+	int err;
+
+	pdrvdata.class = class_create(THIS_MODULE, "virtio-ports");
+	if (IS_ERR(pdrvdata.class)) {
+		err = PTR_ERR(pdrvdata.class);
+		pr_err("Error %d creating virtio-ports class\n", err);
+		return err;
+	}
 	INIT_LIST_HEAD(&pdrvdata.consoles);
 
 	return register_virtio_driver(&virtio_console);
-- 
1.6.2.5

[PATCH 23/31] virtio: console: Add file operations to ports for open/read/write/poll

[Amit Shah]

Allow guest userspace applications to open, read from, write to, poll
the ports via the char dev interface.

When a port gets opened, a notification is sent to the host via a
control message indicating a connection has been established. Similarly,
on closing of the port, a notification is sent indicating disconnection.

Signed-off-by: Amit Shah <amit.shah@redhat.com>
---
 drivers/char/virtio_console.c  |  164 +++++++++++++++++++++++++++++++++++++++-
 include/linux/virtio_console.h |    1 +
 2 files changed, 164 insertions(+), 1 deletions(-)

diff --git a/drivers/char/virtio_console.c b/drivers/char/virtio_console.c
index 6aa400e..8d23c41 100644
--- a/drivers/char/virtio_console.c
+++ b/drivers/char/virtio_console.c
@@ -19,11 +19,15 @@
 #include <linux/cdev.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"
 
@@ -166,8 +170,14 @@ struct port {
 	struct cdev cdev;
 	struct device *dev;
 
+	/* A waitqueue for poll() or blocking read operations */
+	wait_queue_head_t waitqueue;
+
 	/* The 'id' to identify the port with the Host */
 	u32 id;
+
+	/* Is the host device open */
+	bool host_connected;
 };
 
 /* This is the very early arch-specified put chars function. */
@@ -424,6 +434,146 @@ static ssize_t fill_readbuf(struct port *port, char *out_buf, size_t out_count,
 	return out_count;
 }
 
+/* The condition that must be true for polling to end */
+static bool wait_is_over(struct port *port)
+{
+	return port_has_data(port) || !port->host_connected;
+}
+
+static ssize_t port_fops_read(struct file *filp, char __user *ubuf,
+			      size_t count, loff_t *offp)
+{
+	struct port *port;
+	ssize_t ret;
+
+	port = filp->private_data;
+
+	if (!port_has_data(port)) {
+		/*
+		 * 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 (!port_has_data(port) && !port->host_connected)
+		return 0;
+
+	return fill_readbuf(port, ubuf, count, true);
+}
+
+static ssize_t port_fops_write(struct file *filp, const char __user *ubuf,
+			       size_t count, loff_t *offp)
+{
+	struct port *port;
+	char *buf;
+	ssize_t ret;
+
+	port = filp->private_data;
+
+	count = min((size_t)(32 * 1024), count);
+
+	buf = kmalloc(count, GFP_KERNEL);
+	if (!buf)
+		return -ENOMEM;
+
+	ret = copy_from_user(buf, ubuf, count);
+	if (ret) {
+		ret = -EFAULT;
+		goto free_buf;
+	}
+
+	ret = send_buf(port, buf, count);
+free_buf:
+	kfree(buf);
+	return ret;
+}
+
+static unsigned int port_fops_poll(struct file *filp, poll_table *wait)
+{
+	struct port *port;
+	unsigned int ret;
+
+	port = filp->private_data;
+	poll_wait(filp, &port->waitqueue, wait);
+
+	ret = 0;
+	if (port->inbuf)
+		ret |= POLLIN | POLLRDNORM;
+	if (port->host_connected)
+		ret |= POLLOUT;
+	if (!port->host_connected)
+		ret |= POLLHUP;
+
+	return ret;
+}
+
+static int port_fops_release(struct inode *inode, struct file *filp)
+{
+	struct port *port;
+
+	port = filp->private_data;
+
+	/* Notify host of port being closed */
+	send_control_msg(port, VIRTIO_CONSOLE_PORT_OPEN, 0);
+
+	return 0;
+}
+
+static int port_fops_open(struct inode *inode, struct file *filp)
+{
+	struct cdev *cdev = inode->i_cdev;
+	struct port *port;
+
+	port = container_of(cdev, struct port, cdev);
+	filp->private_data = port;
+
+	/*
+	 * Don't allow opening of console port devices -- that's done
+	 * via /dev/hvc
+	 */
+	if (is_console_port(port))
+		return -ENXIO;
+
+	/* Notify host of port being opened */
+	send_control_msg(filp->private_data, VIRTIO_CONSOLE_PORT_OPEN, 1);
+
+	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/vport<device number>p<port number>
+ */
+static const struct file_operations port_fops = {
+	.owner = THIS_MODULE,
+	.open  = port_fops_open,
+	.read  = port_fops_read,
+	.write = port_fops_write,
+	.poll  = port_fops_poll,
+	.release = port_fops_release,
+};
+
 /*
  * The put_chars() callback is pretty straightforward.
  *
@@ -567,6 +717,9 @@ int init_port_console(struct port *port)
 	list_add_tail(&port->cons.list, &pdrvdata.consoles);
 	spin_unlock_irq(&pdrvdata_lock);
 
+	/* Notify host of port being opened */
+	send_control_msg(port, VIRTIO_CONSOLE_PORT_OPEN, 1);
+
 	return 0;
 }
 
@@ -606,6 +759,10 @@ static void handle_control_message(struct ports_device *portdev,
 		port->cons.hvc->irq_requested = 1;
 		resize_console(port);
 		break;
+	case VIRTIO_CONSOLE_PORT_OPEN:
+		port->host_connected = cpkt->value;
+		wake_up_interruptible(&port->waitqueue);
+		break;
 	}
 }
 
@@ -652,6 +809,8 @@ static void in_intr(struct virtqueue *vq)
 
 	spin_unlock_irqrestore(&port->inbuf_lock, flags);
 
+	wake_up_interruptible(&port->waitqueue);
+
 	if (is_console_port(port) && hvc_poll(port->cons.hvc))
 		hvc_kick();
 }
@@ -704,10 +863,12 @@ static int add_port(struct ports_device *portdev, u32 id)
 	port->inbuf = NULL;
 	port->cons.hvc = NULL;
 
+	port->host_connected = false;
+
 	port->in_vq = portdev->in_vqs[port->id];
 	port->out_vq = portdev->out_vqs[port->id];
 
-	cdev_init(&port->cdev, NULL);
+	cdev_init(&port->cdev, &port_fops);
 
 	devt = MKDEV(portdev->chr_major, id);
 	err = cdev_add(&port->cdev, devt, 1);
@@ -728,6 +889,7 @@ static int add_port(struct ports_device *portdev, u32 id)
 	}
 
 	spin_lock_init(&port->inbuf_lock);
+	init_waitqueue_head(&port->waitqueue);
 
 	inbuf = alloc_buf(PAGE_SIZE);
 	if (!inbuf) {
diff --git a/include/linux/virtio_console.h b/include/linux/virtio_console.h
index cada769..6a76a44 100644
--- a/include/linux/virtio_console.h
+++ b/include/linux/virtio_console.h
@@ -39,6 +39,7 @@ struct virtio_console_control {
 #define VIRTIO_CONSOLE_PORT_READY	0
 #define VIRTIO_CONSOLE_CONSOLE_PORT	1
 #define VIRTIO_CONSOLE_RESIZE		2
+#define VIRTIO_CONSOLE_PORT_OPEN	3
 
 #ifdef __KERNEL__
 int __init virtio_cons_early_init(int (*put_chars)(u32, const char *, int));
-- 
1.6.2.5

[PATCH 29/31] virtio: console: Add ability to hot-unplug ports

[Amit Shah]

Remove port data; deregister from the hvc core if it's a console port.

Signed-off-by: Amit Shah <amit.shah@redhat.com>
---
 drivers/char/virtio_console.c  |   65 ++++++++++++++++++++++++++++++++++++++-
 include/linux/virtio_console.h |    1 +
 2 files changed, 64 insertions(+), 2 deletions(-)

diff --git a/drivers/char/virtio_console.c b/drivers/char/virtio_console.c
index 6d783b7..8241bb7 100644
--- a/drivers/char/virtio_console.c
+++ b/drivers/char/virtio_console.c
@@ -790,6 +790,36 @@ static struct attribute_group port_attribute_group = {
 	.attrs = port_sysfs_entries,
 };
 
+/* Remove all port-specific data. */
+static int remove_port(struct port *port)
+{
+	spin_lock_irq(&port->portdev->ports_lock);
+	list_del(&port->list);
+	spin_unlock_irq(&port->portdev->ports_lock);
+
+	if (is_console_port(port)) {
+		spin_lock_irq(&pdrvdata_lock);
+		list_del(&port->cons.list);
+		spin_unlock_irq(&pdrvdata_lock);
+		hvc_remove(port->cons.hvc);
+	}
+	if (port->guest_connected)
+		send_control_msg(port, VIRTIO_CONSOLE_PORT_OPEN, 0);
+
+	while (port->in_vq->vq_ops->detach_unused_buf(port->in_vq))
+		;
+
+	sysfs_remove_group(&port->dev->kobj, &port_attribute_group);
+	device_destroy(pdrvdata.class, port->dev->devt);
+	cdev_del(&port->cdev);
+
+	discard_port_data(port);
+	kfree(port->name);
+
+	kfree(port);
+	return 0;
+}
+
 /* Any private messages that the Host and Guest want to share */
 static void handle_control_message(struct ports_device *portdev,
 				   struct port_buffer *buf)
@@ -861,6 +891,32 @@ static void handle_control_message(struct ports_device *portdev,
 				err);
 
 		break;
+	case VIRTIO_CONSOLE_PORT_REMOVE:
+		/*
+		 * Hot unplug the port.  We don't decrement nr_ports
+		 * since we don't want to deal with extra complexities
+		 * of using the lowest-available port id: We can just
+		 * pick up the nr_ports number as the id and not have
+		 * userspace send it to us.  This helps us in two
+		 * ways:
+		 *
+		 * - We don't need to have a 'port_id' field in the
+		 *   config space when a port is hot-added.  This is a
+		 *   good thing as we might queue up multiple hotplug
+		 *   requests issued in our workqueue.
+		 *
+		 * - Another way to deal with this would have been to
+		 *   use a bitmap of the active ports and select the
+		 *   lowest non-active port from that map.  That
+		 *   bloats the already tight config space and we
+		 *   would end up artificially limiting the
+		 *   max. number of ports to sizeof(bitmap).  Right
+		 *   now we can support 2^32 ports (as the port id is
+		 *   stored in a u32 type).
+		 *
+		 */
+		remove_port(port);
+		break;
 	}
 }
 
@@ -1085,12 +1141,17 @@ static void config_work_handler(struct work_struct *work)
 		/*
 		 * Port 0 got hot-added.  Since we already did all the
 		 * other initialisation for it, just tell the Host
-		 * that the port is ready.
+		 * that the port is ready if we find the port.  In
+		 * case the port was hot-removed earlier, we call
+		 * add_port to add the port.
 		 */
 		struct port *port;
 
 		port = find_port_by_id(portdev, 0);
-		send_control_msg(port, VIRTIO_CONSOLE_PORT_READY, 1);
+		if (!port)
+			add_port(portdev, 0);
+		else
+			send_control_msg(port, VIRTIO_CONSOLE_PORT_READY, 1);
 		return;
 	}
 	if (virtconconf.nr_ports > portdev->config.max_nr_ports) {
diff --git a/include/linux/virtio_console.h b/include/linux/virtio_console.h
index 13e56c8..dd08675 100644
--- a/include/linux/virtio_console.h
+++ b/include/linux/virtio_console.h
@@ -41,6 +41,7 @@ struct virtio_console_control {
 #define VIRTIO_CONSOLE_RESIZE		2
 #define VIRTIO_CONSOLE_PORT_OPEN	3
 #define VIRTIO_CONSOLE_PORT_NAME	4
+#define VIRTIO_CONSOLE_PORT_REMOVE	5
 
 #ifdef __KERNEL__
 int __init virtio_cons_early_init(int (*put_chars)(u32, const char *, int));
-- 
1.6.2.5

Re: [PATCH 21/31] virtio: console: Prepare for writing to userspace buffers

[Rusty Russell]

On Sat, 30 Jan 2010 12:12:40 am Amit Shah wrote:
> When ports get advertised as char devices, the buffers will come from
> userspace. Equip the fill_readbuf function with the ability to write
> to userspace buffers.
> 
> Signed-off-by: Amit Shah <amit.shah@redhat.com>
> ---
>  drivers/char/virtio_console.c |   20 ++++++++++++++------
>  1 files changed, 14 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/char/virtio_console.c b/drivers/char/virtio_console.c
> index 9d33239..5f61021 100644
> --- a/drivers/char/virtio_console.c
> +++ b/drivers/char/virtio_console.c
> @@ -365,7 +365,8 @@ fail:
>   * Give out the data that's requested from the buffer that we have
>   * queued up.
>   */
> -static ssize_t fill_readbuf(struct port *port, char *out_buf, size_t out_count)
> +static ssize_t fill_readbuf(struct port *port, char *out_buf, size_t out_count,
> +			    bool to_user)

This patch does not apply: your previous 19/31 "Introduce a send_buf function
for a common path for sending data to host" made this "void *out_buf".  The
rest I can't even sort out.

I have dropped 19/31 onwards, as listed below.  Please re-xmit privately,
tarball is fine.

Thanks,
Rusty.

Subject: virtio: console: Introduce a send_buf function for a common path for sending data to host
Subject: virtio: console: Add a new MULTIPORT feature, support for generic ports
Subject: virtio: console: Prepare for writing to / reading from userspace buffers
Subject: virtio: console: Associate each port with a char device
Subject: virtio: console: Add file operations to ports for open/read/write/poll
Subject: virtio: console: Ensure only one process can have a port open at a time
Subject: virtio: console: Register with sysfs and create a 'name' attribute for ports
Subject: virtio: console: Remove cached data on port close
Subject: virtio: console: Handle port hot-plug
Subject: virtio: console: Add ability to hot-unplug ports
Subject: virtio: console: Add debugfs files for each port to expose debug info
Subject: virtio: console: show error message if hvc_alloc fails for console ports

Re: [PATCH 21/31] virtio: console: Prepare for writing to userspace buffers

[Amit Shah]

On (Mon) Feb 01 2010 [10:49:51], Rusty Russell wrote:
> On Sat, 30 Jan 2010 12:12:40 am Amit Shah wrote:
> > When ports get advertised as char devices, the buffers will come from
> > userspace. Equip the fill_readbuf function with the ability to write
> > to userspace buffers.
> > 
> > Signed-off-by: Amit Shah <amit.shah@redhat.com>
> > ---
> >  drivers/char/virtio_console.c |   20 ++++++++++++++------
> >  1 files changed, 14 insertions(+), 6 deletions(-)
> > 
> > diff --git a/drivers/char/virtio_console.c b/drivers/char/virtio_console.c
> > index 9d33239..5f61021 100644
> > --- a/drivers/char/virtio_console.c
> > +++ b/drivers/char/virtio_console.c
> > @@ -365,7 +365,8 @@ fail:
> >   * Give out the data that's requested from the buffer that we have
> >   * queued up.
> >   */
> > -static ssize_t fill_readbuf(struct port *port, char *out_buf, size_t out_count)
> > +static ssize_t fill_readbuf(struct port *port, char *out_buf, size_t out_count,
> > +			    bool to_user)
> 
> This patch does not apply: your previous 19/31 "Introduce a send_buf function
> for a common path for sending data to host" made this "void *out_buf".  The
> rest I can't even sort out.

Sorry for that, Rusty.

I've sent you a tarball privately.

The tarball contains an additional patch (contained in the series
itself), that drops the use of outbuf for sending control messages
as well:

(The 'outbuf' was used earlier when we had multiple buffers that could
be queued up for the host to consume. We now only queue up one buffer at
a time and wait for the host to ack the buffer before we move along.
When we revisit the design to allow multiple out-buffers, we can go this
path again.)


 drivers/char/virtio_console.c |   21 ++++-----------------
 1 files changed, 4 insertions(+), 17 deletions(-)

diff --git a/drivers/char/virtio_console.c b/drivers/char/virtio_console.c
index 2c2de35..793285d 100644
--- a/drivers/char/virtio_console.c
+++ b/drivers/char/virtio_console.c
@@ -134,9 +134,6 @@ struct ports_device {
 	/* Array of per-port IO virtqueues */
 	struct virtqueue **in_vqs, **out_vqs;
 
-	/* The control messages to the Host are sent via this buffer */
-	struct port_buffer *outbuf;
-
 	/* Used for numbering devices for sysfs and debugfs */
 	unsigned int drv_index;
 
@@ -372,8 +369,7 @@ static ssize_t send_control_msg(struct port *port, unsigned int event,
 	struct scatterlist sg[1];
 	struct virtio_console_control cpkt;
 	struct virtqueue *vq;
-	struct port_buffer *outbuf;
-	int tmplen;
+	int len;
 
 	if (!use_multiport(port->portdev))
 		return 0;
@@ -383,14 +379,11 @@ static ssize_t send_control_msg(struct port *port, unsigned int event,
 	cpkt.value = value;
 
 	vq = port->portdev->c_ovq;
-	outbuf = port->portdev->outbuf;
-
-	memcpy(outbuf->buf, (void *)&cpkt, sizeof(cpkt));
 
-	sg_init_one(sg, outbuf->buf, sizeof(cpkt));
-	if (vq->vq_ops->add_buf(vq, sg, 1, 0, outbuf) >= 0) {
+	sg_init_one(sg, &cpkt, sizeof(cpkt));
+	if (vq->vq_ops->add_buf(vq, sg, 1, 0, &cpkt) >= 0) {
 		vq->vq_ops->kick(vq);
-		while (!vq->vq_ops->get_buf(vq, &tmplen))
+		while (!vq->vq_ops->get_buf(vq, &len))
 			cpu_relax();
 	}
 	return 0;
@@ -1432,12 +1425,6 @@ static int __devinit virtcons_probe(struct virtio_device *vdev)
 		INIT_WORK(&portdev->control_work, &control_work_handler);
 		INIT_WORK(&portdev->config_work, &config_work_handler);
 
-		portdev->outbuf = alloc_buf(PAGE_SIZE);
-		if (!portdev->outbuf) {
-			err = -ENOMEM;
-			dev_err(&vdev->dev, "OOM for control outbuf\n");
-			goto free_vqs;
-		}
 		fill_queue(portdev->c_ivq, &portdev->cvq_lock);
 	}
 
		Amit

Re: virtio: console: Return -EFAULT on copy_xx_user errors, allow larger writes

[Rusty Russell]

On Sat, 30 Jan 2010 12:12:37 am Amit Shah wrote:
> These updated patches in the series return -EFAULT on copy_xx_user
> errors and also move the copy_from_user into fops_write() instead of it
> being in send_buf. This enables send_buf to just read from kernel
> buffers, making it simpler.
> 
> This also allows write()s to write more to the host in one go,
> removingthe 4k limitation. I do limit the writes to 32k at once to not
> put too much pressure on the GFP_KERNEL area.
> 
> Both of these were pointed out by Marcelo.

Applied.

Thanks,
Rusty.