All of lore.kernel.org
 help / color / mirror / Atom feed
* virtio_console: support for multiple ports, console and generic.
@ 2009-11-10  9:41 Amit Shah
  2009-11-10  9:41 ` [PATCH 01/15] virtio_console: Initialize hv_ops struct at declaration instead of in the probe() routine Amit Shah
  0 siblings, 1 reply; 20+ messages in thread
From: Amit Shah @ 2009-11-10  9:41 UTC (permalink / raw)
  To: rusty; +Cc: virtualization


Hey Rusty,

This is the way I did the split; patches 1..7 are preparation for
multiple ports.

Patch 8 adds multiport support. It's the big one since we have to put
the header in and retain support for multiple consoles and console
resizing.

Patch 9 adds port hotplug

Patch 10 adds sysfs entries and 12 adds debugfs.

Patches 13, 14 and 15 add throttling, caching and unplug features.

There's also a one-device restriction; which would be addressed once I
use vdev->priv.

My main aim is to get comments on the send_buf and fill_readbuf
functions, the input/output workqueues and hotplug.

Amit

^ permalink raw reply	[flat|nested] 20+ messages in thread

* [PATCH 01/15] virtio_console: Initialize hv_ops struct at declaration instead of in the probe() routine
  2009-11-10  9:41 virtio_console: support for multiple ports, console and generic Amit Shah
@ 2009-11-10  9:41 ` Amit Shah
  2009-11-10  9:41   ` [PATCH 02/15] virtio_console: We support only one device at a time Amit Shah
  2009-11-10 13:07   ` [PATCH 01/15] virtio_console: Initialize hv_ops struct at declaration instead of in the probe() routine Rusty Russell
  0 siblings, 2 replies; 20+ messages in thread
From: Amit Shah @ 2009-11-10  9:41 UTC (permalink / raw)
  To: rusty; +Cc: Amit Shah, virtualization

Initialize the hv_ops function pointers using C99 style. However, we
need to re-init the 'put_chars' field to our implementation in the
probe() routine as we replace it for early_init to use the caller's
put_chars implementation.

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

diff --git a/drivers/char/virtio_console.c b/drivers/char/virtio_console.c
index a035ae3..e5284a9 100644
--- a/drivers/char/virtio_console.c
+++ b/drivers/char/virtio_console.c
@@ -43,9 +43,6 @@ static struct virtio_device *vdev;
 static unsigned int in_len;
 static char *in, *inbuf;
 
-/* The operations for our console. */
-static struct hv_ops virtio_cons;
-
 /* The hvc device */
 static struct hvc_struct *hvc;
 
@@ -125,18 +122,6 @@ static int get_chars(u32 vtermno, char *buf, int count)
 }
 /*:*/
 
-/*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);
-}
-
 /*
  * virtio console configuration. This supports:
  * - console resize
@@ -173,6 +158,30 @@ static void notifier_del_vio(struct hvc_struct *hp, int data)
 	hp->irq_requested = 0;
 }
 
+/* The operations for our console. */
+static struct hv_ops virtio_cons = {
+	.get_chars = get_chars,
+	.put_chars = put_chars,
+	.notifier_add = notifier_add_vio,
+	.notifier_del = notifier_del_vio,
+	.notifier_hangup = 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);
+}
+
 static void hvc_handle_input(struct virtqueue *vq)
 {
 	if (hvc_poll(hvc))
@@ -212,12 +221,13 @@ static int __devinit virtcons_probe(struct virtio_device *dev)
 	in_vq = vqs[0];
 	out_vq = vqs[1];
 
-	/* Start using the new console output. */
-	virtio_cons.get_chars = get_chars;
+	/*
+	 * We had set the virtio_cons ->put_chars implementation to an
+	 * architecture-provided put_chars for early_init. Now that
+	 * we're done with the early init phase, replace it with our
+	 * implementation.
+	 */
 	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
-- 
1.6.2.5

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH 02/15] virtio_console: We support only one device at a time
  2009-11-10  9:41 ` [PATCH 01/15] virtio_console: Initialize hv_ops struct at declaration instead of in the probe() routine Amit Shah
@ 2009-11-10  9:41   ` Amit Shah
  2009-11-10  9:41     ` [PATCH 03/15] virtio_console: Club all globals into one struct Amit Shah
  2009-11-10 13:07   ` [PATCH 01/15] virtio_console: Initialize hv_ops struct at declaration instead of in the probe() routine Rusty Russell
  1 sibling, 1 reply; 20+ messages in thread
From: Amit Shah @ 2009-11-10  9:41 UTC (permalink / raw)
  To: rusty; +Cc: Amit Shah, virtualization

We support only one virtio_console device at a time. If multiple are
found, error out if one is already initialized.

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

diff --git a/drivers/char/virtio_console.c b/drivers/char/virtio_console.c
index e5284a9..21039b8 100644
--- a/drivers/char/virtio_console.c
+++ b/drivers/char/virtio_console.c
@@ -202,6 +202,10 @@ static int __devinit virtcons_probe(struct virtio_device *dev)
 	struct virtqueue *vqs[2];
 	int err;
 
+	if (vdev) {
+		pr_err("Multiple virtio-console devices not supported yet\n");
+		return -EEXIST;
+	}
 	vdev = dev;
 
 	/* This is the scratch page we use to receive console input */
-- 
1.6.2.5

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH 03/15] virtio_console: Club all globals into one struct
  2009-11-10  9:41   ` [PATCH 02/15] virtio_console: We support only one device at a time Amit Shah
@ 2009-11-10  9:41     ` Amit Shah
  2009-11-10  9:41       ` [PATCH 04/15] virtio_console: Introduce a workqueue for handling host->guest notifications Amit Shah
  0 siblings, 1 reply; 20+ messages in thread
From: Amit Shah @ 2009-11-10  9:41 UTC (permalink / raw)
  To: rusty; +Cc: Amit Shah, virtualization

Club all the globals -- the virtio_device struct, virtqueues,
hvc_console struct, inbuf, len, etc. into one virtioconsole
struct.

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

diff --git a/drivers/char/virtio_console.c b/drivers/char/virtio_console.c
index 21039b8..ed4d9a4 100644
--- a/drivers/char/virtio_console.c
+++ b/drivers/char/virtio_console.c
@@ -34,17 +34,23 @@
 #include <linux/virtio_console.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;
+struct virtio_console_struct {
+	/*D:340
+	 * These represent our input and output console queues,
+	 * and the virtio operations for them.
+	 */
+	struct virtqueue *in_vq, *out_vq;
+	struct virtio_device *vdev;
+
+	/* This is our input buffer, and how much data is left in it. */
+	unsigned int in_len;
+	char *in, *inbuf;
 
-/* This is our input buffer, and how much data is left in it. */
-static unsigned int in_len;
-static char *in, *inbuf;
+	/* The hvc device */
+	struct hvc_struct *hvc;
+};
 
-/* The hvc device */
-static struct hvc_struct *hvc;
+struct virtio_console_struct virtconsole;
 
 /*D:310 The put_chars() callback is pretty straightforward.
  *
@@ -54,12 +60,14 @@ static struct hvc_struct *hvc;
  * immediately (lguest's Launcher does). */
 static int put_chars(u32 vtermno, const char *buf, int count)
 {
+	struct virtqueue *out_vq;
 	struct scatterlist sg[1];
 	unsigned int len;
 
 	/* This is a convenient routine to initialize a single-elem sg list */
 	sg_init_one(sg, buf, count);
 
+	out_vq = virtconsole.out_vq;
 	/* 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) {
@@ -78,11 +86,14 @@ static int put_chars(u32 vtermno, const char *buf, int count)
  * queue. */
 static void add_inbuf(void)
 {
+	struct virtqueue *in_vq;
 	struct scatterlist sg[1];
-	sg_init_one(sg, inbuf, PAGE_SIZE);
 
+	sg_init_one(sg, virtconsole.inbuf, PAGE_SIZE);
+
+	in_vq = virtconsole.in_vq;
 	/* 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)
+	if (in_vq->vq_ops->add_buf(in_vq, sg, 0, 1, virtconsole.inbuf) < 0)
 		BUG();
 	in_vq->vq_ops->kick(in_vq);
 }
@@ -95,27 +106,31 @@ static void add_inbuf(void)
  * for partially-filled buffers. */
 static int get_chars(u32 vtermno, char *buf, int count)
 {
+	struct virtqueue *in_vq;
+
+	in_vq = virtconsole.in_vq;
 	/* If we don't have an input queue yet, we can't get input. */
 	BUG_ON(!in_vq);
 
 	/* No buffer?  Try to get one. */
-	if (!in_len) {
-		in = in_vq->vq_ops->get_buf(in_vq, &in_len);
-		if (!in)
+	if (!virtconsole.in_len) {
+		virtconsole.in = in_vq->vq_ops->get_buf(in_vq,
+							&virtconsole.in_len);
+		if (!virtconsole.in)
 			return 0;
 	}
 
 	/* You want more than we have to give?  Well, try wanting less! */
-	if (in_len < count)
-		count = in_len;
+	if (virtconsole.in_len < count)
+		count = virtconsole.in_len;
 
 	/* Copy across to their buffer and increment offset. */
-	memcpy(buf, in, count);
-	in += count;
-	in_len -= count;
+	memcpy(buf, virtconsole.in, count);
+	virtconsole.in += count;
+	virtconsole.in_len -= count;
 
 	/* Finished?  Re-register buffer so Host will use it again. */
-	if (in_len == 0)
+	if (virtconsole.in_len == 0)
 		add_inbuf();
 
 	return count;
@@ -137,7 +152,7 @@ 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);
+		hvc_resize(virtconsole.hvc, ws);
 	}
 }
 
@@ -148,7 +163,7 @@ static void virtcons_apply_config(struct virtio_device *dev)
 static int notifier_add_vio(struct hvc_struct *hp, int data)
 {
 	hp->irq_requested = 1;
-	virtcons_apply_config(vdev);
+	virtcons_apply_config(virtconsole.vdev);
 
 	return 0;
 }
@@ -184,7 +199,7 @@ int __init virtio_cons_early_init(int (*put_chars)(u32, const char *, int))
 
 static void hvc_handle_input(struct virtqueue *vq)
 {
-	if (hvc_poll(hvc))
+	if (hvc_poll(virtconsole.hvc))
 		hvc_kick();
 }
 
@@ -195,22 +210,22 @@ static void hvc_handle_input(struct virtqueue *vq)
  * never remove the console device we never need this pointer again.
  *
  * Finally we put our input buffer in the input queue, ready to receive. */
-static int __devinit virtcons_probe(struct virtio_device *dev)
+static int __devinit virtcons_probe(struct virtio_device *vdev)
 {
 	vq_callback_t *callbacks[] = { hvc_handle_input, NULL};
 	const char *names[] = { "input", "output" };
 	struct virtqueue *vqs[2];
 	int err;
 
-	if (vdev) {
+	if (virtconsole.vdev) {
 		pr_err("Multiple virtio-console devices not supported yet\n");
 		return -EEXIST;
 	}
-	vdev = dev;
+	virtconsole.vdev = vdev;
 
 	/* This is the scratch page we use to receive console input */
-	inbuf = kmalloc(PAGE_SIZE, GFP_KERNEL);
-	if (!inbuf) {
+	virtconsole.inbuf = kmalloc(PAGE_SIZE, GFP_KERNEL);
+	if (!virtconsole.inbuf) {
 		err = -ENOMEM;
 		goto fail;
 	}
@@ -222,8 +237,8 @@ static int __devinit virtcons_probe(struct virtio_device *dev)
 	if (err)
 		goto free;
 
-	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 an
@@ -242,9 +257,9 @@ static int __devinit virtcons_probe(struct virtio_device *dev)
 	 * 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);
+	virtconsole.hvc = hvc_alloc(0, 0, &virtio_cons, PAGE_SIZE);
+	if (IS_ERR(virtconsole.hvc)) {
+		err = PTR_ERR(virtconsole.hvc);
 		goto free_vqs;
 	}
 
@@ -255,7 +270,7 @@ static int __devinit virtcons_probe(struct virtio_device *dev)
 free_vqs:
 	vdev->config->del_vqs(vdev);
 free:
-	kfree(inbuf);
+	kfree(virtconsole.inbuf);
 fail:
 	return err;
 }
-- 
1.6.2.5

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH 04/15] virtio_console: Introduce a workqueue for handling host->guest notifications
  2009-11-10  9:41     ` [PATCH 03/15] virtio_console: Club all globals into one struct Amit Shah
@ 2009-11-10  9:41       ` Amit Shah
  2009-11-10  9:41         ` [PATCH 05/15] virtio_console: Buffer data that comes in from the host Amit Shah
  0 siblings, 1 reply; 20+ messages in thread
From: Amit Shah @ 2009-11-10  9:41 UTC (permalink / raw)
  To: rusty; +Cc: Amit Shah, virtualization

We currently only maintain one buffer for any data the host sends
us. If we're slow in consuming that data, we might lose old data.
To buffer the data that the host sends us, we need to be able to
allocate buffers as and when the host sends us some.

Using a workqueue gives us the freedom to allocate kernel buffers
since the workqueues are executed in process context.

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

diff --git a/drivers/char/virtio_console.c b/drivers/char/virtio_console.c
index ed4d9a4..19c9729 100644
--- a/drivers/char/virtio_console.c
+++ b/drivers/char/virtio_console.c
@@ -35,6 +35,12 @@
 #include "hvc_console.h"
 
 struct virtio_console_struct {
+	/*
+	 * Workqueue handlers where we process deferred work after an
+	 * interrupt
+	 */
+	struct work_struct rx_work;
+
 	/*D:340
 	 * These represent our input and output console queues,
 	 * and the virtio operations for them.
@@ -197,12 +203,21 @@ int __init virtio_cons_early_init(int (*put_chars)(u32, const char *, int))
 	return hvc_instantiate(0, 0, &virtio_cons);
 }
 
-static void hvc_handle_input(struct virtqueue *vq)
+static void virtio_console_rx_work_handler(struct work_struct *work)
 {
-	if (hvc_poll(virtconsole.hvc))
+	struct virtio_console_struct *vcon;
+
+	vcon = container_of(work, struct virtio_console_struct, rx_work);
+
+	if (hvc_poll(vcon->hvc))
 		hvc_kick();
 }
 
+static void rx_intr(struct virtqueue *vq)
+{
+	schedule_work(&virtconsole.rx_work);
+}
+
 /*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.
  *
@@ -212,7 +227,7 @@ static void hvc_handle_input(struct virtqueue *vq)
  * 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[] = { hvc_handle_input, NULL};
+	vq_callback_t *callbacks[] = { rx_intr, NULL};
 	const char *names[] = { "input", "output" };
 	struct virtqueue *vqs[2];
 	int err;
@@ -248,6 +263,8 @@ static int __devinit virtcons_probe(struct virtio_device *vdev)
 	 */
 	virtio_cons.put_chars = put_chars;
 
+	INIT_WORK(&virtconsole.rx_work, &virtio_console_rx_work_handler);
+
 	/* 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
-- 
1.6.2.5

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH 05/15] virtio_console: Buffer data that comes in from the host
  2009-11-10  9:41       ` [PATCH 04/15] virtio_console: Introduce a workqueue for handling host->guest notifications Amit Shah
@ 2009-11-10  9:41         ` Amit Shah
  2009-11-10  9:41           ` [PATCH 06/15] virtio_console: Create a buffer pool for sending data to host Amit Shah
  0 siblings, 1 reply; 20+ messages in thread
From: Amit Shah @ 2009-11-10  9:41 UTC (permalink / raw)
  To: rusty; +Cc: Amit Shah, virtualization

The console could be flooded with data from the host; handle
this situation by buffering the data.

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

diff --git a/drivers/char/virtio_console.c b/drivers/char/virtio_console.c
index 19c9729..585ad3c 100644
--- a/drivers/char/virtio_console.c
+++ b/drivers/char/virtio_console.c
@@ -9,9 +9,6 @@
  * 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
  *
  * This program is free software; you can redistribute it and/or modify
@@ -30,6 +27,8 @@
  */
 #include <linux/err.h>
 #include <linux/init.h>
+#include <linux/list.h>
+#include <linux/spinlock.h>
 #include <linux/virtio.h>
 #include <linux/virtio_console.h>
 #include "hvc_console.h"
@@ -41,6 +40,17 @@ struct virtio_console_struct {
 	 */
 	struct work_struct rx_work;
 
+	/* Buffer management */
+	struct list_head unused_read_head;
+	struct list_head readbuf_head;
+
+	/*
+	 * To protect the readbuf_head list. Has to be a spinlock
+	 * because it can be called from interrupt context
+	 * (get_char())
+	 */
+	spinlock_t readbuf_list_lock;
+
 	/*D:340
 	 * These represent our input and output console queues,
 	 * and the virtio operations for them.
@@ -48,14 +58,22 @@ struct virtio_console_struct {
 	struct virtqueue *in_vq, *out_vq;
 	struct virtio_device *vdev;
 
-	/* This is our input buffer, and how much data is left in it. */
-	unsigned int in_len;
-	char *in, *inbuf;
-
 	/* The hvc device */
 	struct hvc_struct *hvc;
 };
 
+/* This struct holds individual buffers received from the host */
+struct virtio_console_port_buffer {
+	struct list_head list;
+
+	char *buf;
+
+	/* length of the buffer */
+	size_t len;
+	/* offset in the buf from which to consume data */
+	size_t offset;
+};
+
 struct virtio_console_struct virtconsole;
 
 /*D:310 The put_chars() callback is pretty straightforward.
@@ -88,58 +106,65 @@ static int put_chars(u32 vtermno, const char *buf, int count)
 	return count;
 }
 
-/* Create a scatter-gather list representing our input buffer and put it in the
- * queue. */
-static void add_inbuf(void)
+/*
+ * Give out the data that's requested from the buffers that we have
+ * queued up.
+ */
+static ssize_t fill_readbuf(char *out_buf, size_t out_count)
 {
-	struct virtqueue *in_vq;
-	struct scatterlist sg[1];
-
-	sg_init_one(sg, virtconsole.inbuf, PAGE_SIZE);
+	struct virtio_console_port_buffer *buf, *buf2;
+	ssize_t out_offset, ret;
 
-	in_vq = virtconsole.in_vq;
-	/* 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, virtconsole.inbuf) < 0)
-		BUG();
-	in_vq->vq_ops->kick(in_vq);
+	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.
+	 */
+	list_for_each_entry_safe(buf, buf2, &virtconsole.readbuf_head, list) {
+		size_t copy_size;
+
+		copy_size = out_count;
+		if (copy_size > buf->len - buf->offset)
+			copy_size = buf->len - buf->offset;
+
+		memcpy(out_buf + out_offset, buf->buf + buf->offset, copy_size);
+
+		/* Return the number of bytes actually copied */
+		ret = copy_size;
+		buf->offset += ret;
+		out_offset += ret;
+		out_count -= ret;
+
+		if (buf->len - buf->offset == 0) {
+			spin_lock(&virtconsole.readbuf_list_lock);
+			list_del(&buf->list);
+			spin_unlock(&virtconsole.readbuf_list_lock);
+			kfree(buf->buf);
+			kfree(buf);
+		}
+		if (!out_count)
+			break;
+	}
+	return out_offset;
 }
 
-/*D:350 get_chars() is the callback from the hvc_console infrastructure when
+/*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. */
+ * We call out to fill_readbuf that gets us the required data from the
+ * buffers that are queued up.
+ */
 static int get_chars(u32 vtermno, char *buf, int count)
 {
-	struct virtqueue *in_vq;
-
-	in_vq = virtconsole.in_vq;
 	/* If we don't have an input queue yet, we can't get input. */
-	BUG_ON(!in_vq);
-
-	/* No buffer?  Try to get one. */
-	if (!virtconsole.in_len) {
-		virtconsole.in = in_vq->vq_ops->get_buf(in_vq,
-							&virtconsole.in_len);
-		if (!virtconsole.in)
-			return 0;
-	}
-
-	/* You want more than we have to give?  Well, try wanting less! */
-	if (virtconsole.in_len < count)
-		count = virtconsole.in_len;
+	BUG_ON(!virtconsole.in_vq);
 
-	/* Copy across to their buffer and increment offset. */
-	memcpy(buf, virtconsole.in, count);
-	virtconsole.in += count;
-	virtconsole.in_len -= count;
+	if (list_empty(&virtconsole.readbuf_head))
+		return 0;
 
-	/* Finished?  Re-register buffer so Host will use it again. */
-	if (virtconsole.in_len == 0)
-		add_inbuf();
-
-	return count;
+	return fill_readbuf(buf, count);
 }
 /*:*/
 
@@ -203,14 +228,82 @@ int __init virtio_cons_early_init(int (*put_chars)(u32, const char *, int))
 	return hvc_instantiate(0, 0, &virtio_cons);
 }
 
+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->list, 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);
+}
+
 static void virtio_console_rx_work_handler(struct work_struct *work)
 {
 	struct virtio_console_struct *vcon;
+	struct virtio_console_port_buffer *buf;
+	struct virtqueue *vq;
+	unsigned int tmplen;
 
 	vcon = container_of(work, struct virtio_console_struct, rx_work);
 
-	if (hvc_poll(vcon->hvc))
+	vq = vcon->in_vq;
+	while ((buf = vq->vq_ops->get_buf(vq, &tmplen))) {
+		/* The buffer is no longer unused */
+		list_del(&buf->list);
+
+		buf->len = tmplen;
+		buf->offset = 0;
+
+		spin_lock(&virtconsole.readbuf_list_lock);
+		list_add_tail(&buf->list, &virtconsole.readbuf_head);
+		spin_unlock(&virtconsole.readbuf_list_lock);
+	}
+	if (hvc_poll(virtconsole.hvc))
 		hvc_kick();
+
+	/* Allocate buffers for all the ones that got used up */
+	fill_receive_queue(&virtconsole);
 }
 
 static void rx_intr(struct virtqueue *vq)
@@ -238,19 +331,12 @@ static int __devinit virtcons_probe(struct virtio_device *vdev)
 	}
 	virtconsole.vdev = vdev;
 
-	/* This is the scratch page we use to receive console input */
-	virtconsole.inbuf = kmalloc(PAGE_SIZE, GFP_KERNEL);
-	if (!virtconsole.inbuf) {
-		err = -ENOMEM;
-		goto fail;
-	}
-
 	/* 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;
+		goto fail;
 
 	virtconsole.in_vq = vqs[0];
 	virtconsole.out_vq = vqs[1];
@@ -263,8 +349,14 @@ static int __devinit virtcons_probe(struct virtio_device *vdev)
 	 */
 	virtio_cons.put_chars = put_chars;
 
+	spin_lock_init(&virtconsole.readbuf_list_lock);
+	INIT_LIST_HEAD(&virtconsole.readbuf_head);
+	INIT_LIST_HEAD(&virtconsole.unused_read_head);
+
 	INIT_WORK(&virtconsole.rx_work, &virtio_console_rx_work_handler);
 
+	fill_receive_queue(&virtconsole);
+
 	/* 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
@@ -280,14 +372,10 @@ static int __devinit virtcons_probe(struct virtio_device *vdev)
 		goto free_vqs;
 	}
 
-	/* Register the input buffer the first time. */
-	add_inbuf();
 	return 0;
 
 free_vqs:
 	vdev->config->del_vqs(vdev);
-free:
-	kfree(virtconsole.inbuf);
 fail:
 	return err;
 }
-- 
1.6.2.5

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH 06/15] virtio_console: Create a buffer pool for sending data to host
  2009-11-10  9:41         ` [PATCH 05/15] virtio_console: Buffer data that comes in from the host Amit Shah
@ 2009-11-10  9:41           ` Amit Shah
  2009-11-10  9:41             ` [PATCH 07/15] virtio_console: config: Prepare for handling multiple ports and hotplug Amit Shah
  0 siblings, 1 reply; 20+ messages in thread
From: Amit Shah @ 2009-11-10  9:41 UTC (permalink / raw)
  To: rusty; +Cc: Amit Shah, virtualization

The current implementation used to write data to the host
and then wait till the host consumed it.

Also, there was no support to send large chunks of data.

This patch adds support to send big chunks in multiple
buffers of PAGE_SIZE each to the host. It also 'sends
and forgets' the data it sent.

When the host consumes the data and signals to us via
an interrupt, we add the consumed buffer back to our
unconsumed list in a new work handler.

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

diff --git a/drivers/char/virtio_console.c b/drivers/char/virtio_console.c
index 585ad3c..cc720d3 100644
--- a/drivers/char/virtio_console.c
+++ b/drivers/char/virtio_console.c
@@ -39,11 +39,16 @@ struct virtio_console_struct {
 	 * interrupt
 	 */
 	struct work_struct rx_work;
+	struct work_struct tx_work;
 
 	/* Buffer management */
 	struct list_head unused_read_head;
+	struct list_head unused_write_head;
 	struct list_head readbuf_head;
 
+	/* To protect the list of unused write buffers and the out_vq */
+	spinlock_t write_list_lock;
+
 	/*
 	 * To protect the readbuf_head list. Has to be a spinlock
 	 * because it can be called from interrupt context
@@ -76,34 +81,77 @@ struct virtio_console_port_buffer {
 
 struct virtio_console_struct virtconsole;
 
-/*D:310 The put_chars() callback is pretty straightforward.
+/*D:310
+ * The put_chars() callback is pretty straightforward.
  *
  * 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)
+ * queue and then kick the Host.
+ *
+ * If the data to be output spans more than a page, it's split into
+ * page-sized buffers and then individual buffers are pushed to the Host.
+ */
+static int put_chars(u32 vtermno, const char *in_buf, int in_count)
 {
 	struct virtqueue *out_vq;
+	struct virtio_console_port_buffer *buf, *buf2;
 	struct scatterlist sg[1];
-	unsigned int len;
+	size_t in_offset, copy_size;
+	ssize_t ret;
 
-	/* This is a convenient routine to initialize a single-elem sg list */
-	sg_init_one(sg, buf, count);
+	if (!in_count)
+		return 0;
 
 	out_vq = virtconsole.out_vq;
-	/* 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();
+
+	in_offset = 0; /* offset in the user buffer */
+	while (in_count - in_offset) {
+		copy_size = min(in_count - in_offset, PAGE_SIZE);
+
+		spin_lock(&virtconsole.write_list_lock);
+		list_for_each_entry_safe(buf, buf2,
+					 &virtconsole.unused_write_head,
+					 list) {
+			list_del(&buf->list);
+			break;
+		}
+		spin_unlock(&virtconsole.write_list_lock);
+		if (!buf)
+			break;
+		/*
+		 * 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_offset, copy_size);
+
+		buf->len = copy_size;
+		sg_init_one(sg, buf->buf, buf->len);
+
+		spin_lock(&virtconsole.write_list_lock);
+		ret = out_vq->vq_ops->add_buf(out_vq, sg, 1, 0, buf);
+		spin_unlock(&virtconsole.write_list_lock);
+		if (ret < 0) {
+			memset(buf->buf, 0, buf->len);
+			spin_lock(&virtconsole.write_list_lock);
+			list_add_tail(&buf->list,
+				      &virtconsole.unused_write_head);
+			spin_unlock(&virtconsole.write_list_lock);
+			break;
+		}
+		in_offset += buf->len;
+
+		/* No space left in the vq anyway */
+		if (!ret)
+			break;
 	}
+	/* Tell Host to go! */
+	spin_lock(&virtconsole.write_list_lock);
+	out_vq->vq_ops->kick(out_vq);
+	spin_unlock(&virtconsole.write_list_lock);
 
-	/* We're expected to return the amount of data we wrote: all of it. */
-	return count;
+	/* We're expected to return the amount of data we wrote */
+	return in_offset;
 }
 
 /*
@@ -278,6 +326,23 @@ 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 < 128; i++) {
+		buf = get_buf(PAGE_SIZE);
+		if (!buf)
+			break;
+		list_add_tail(&buf->list, &vcon->unused_write_head);
+	}
+}
+
 static void virtio_console_rx_work_handler(struct work_struct *work)
 {
 	struct virtio_console_struct *vcon;
@@ -306,11 +371,50 @@ static void virtio_console_rx_work_handler(struct work_struct *work)
 	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.
+ *
+ * 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.
+ *
+ * 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->list, &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);
+}
+
 /*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.
  *
@@ -320,7 +424,7 @@ static void rx_intr(struct virtqueue *vq)
  * 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, NULL};
+	vq_callback_t *callbacks[] = { rx_intr, tx_intr };
 	const char *names[] = { "input", "output" };
 	struct virtqueue *vqs[2];
 	int err;
@@ -350,12 +454,17 @@ static int __devinit virtcons_probe(struct virtio_device *vdev)
 	virtio_cons.put_chars = put_chars;
 
 	spin_lock_init(&virtconsole.readbuf_list_lock);
+	spin_lock_init(&virtconsole.write_list_lock);
+
 	INIT_LIST_HEAD(&virtconsole.readbuf_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);
 
 	fill_receive_queue(&virtconsole);
+	alloc_write_bufs(&virtconsole);
 
 	/* The first argument of hvc_alloc() is the virtual console number, so
 	 * we use zero.  The second argument is the parameter for the
-- 
1.6.2.5

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH 07/15] virtio_console: config: Prepare for handling multiple ports and hotplug
  2009-11-10  9:41           ` [PATCH 06/15] virtio_console: Create a buffer pool for sending data to host Amit Shah
@ 2009-11-10  9:41             ` Amit Shah
  2009-11-10  9:41               ` [PATCH 08/15] virtio_console: Add support for multiple ports: console as well as generic Amit Shah
  0 siblings, 1 reply; 20+ messages in thread
From: Amit Shah @ 2009-11-10  9:41 UTC (permalink / raw)
  To: rusty; +Cc: Amit Shah, virtualization

The config_changed function works on the assumption of there
being only one port and no hotplugging of ports.

Abstract away the call to apply_config in a function call so
that we can handle multiple ports later.

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

diff --git a/drivers/char/virtio_console.c b/drivers/char/virtio_console.c
index cc720d3..e97188e 100644
--- a/drivers/char/virtio_console.c
+++ b/drivers/char/virtio_console.c
@@ -415,6 +415,12 @@ static void tx_intr(struct virtqueue *vq)
 	schedule_work(&virtconsole.tx_work);
 }
 
+static void config_intr(struct virtio_device *vdev)
+{
+	/* Handle console size changes */
+	virtcons_apply_config(vdev);
+}
+
 /*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.
  *
@@ -505,7 +511,7 @@ static struct virtio_driver virtio_console = {
 	.driver.owner =	THIS_MODULE,
 	.id_table =	id_table,
 	.probe =	virtcons_probe,
-	.config_changed = virtcons_apply_config,
+	.config_changed = config_intr,
 };
 
 static int __init init(void)
-- 
1.6.2.5

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH 08/15] virtio_console: Add support for multiple ports: console as well as generic
  2009-11-10  9:41             ` [PATCH 07/15] virtio_console: config: Prepare for handling multiple ports and hotplug Amit Shah
@ 2009-11-10  9:41               ` Amit Shah
  2009-11-10  9:41                 ` [PATCH 09/15] virtio_console: Handle port hot-plug Amit Shah
  0 siblings, 1 reply; 20+ messages in thread
From: Amit Shah @ 2009-11-10  9:41 UTC (permalink / raw)
  To: rusty; +Cc: Amit Shah, virtualization

Add a bit more spice by adding support for multiple console ports.
This introduces a new feature: VIRTIO_CONSOLE_F_MULTIPORT. If a
willing host is found, this support is switched on and a small
'header' is transmitted along with every data packet that's sent
in either direction.

This header stores the port number the data is meant for, along
with a few other fields which will be used for other features
to support generic ports.

The interface is of a simple char device with open, read, write,
poll and close calls, eg:

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

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

diff --git a/drivers/char/virtio_console.c b/drivers/char/virtio_console.c
index e97188e..4119c37 100644
--- a/drivers/char/virtio_console.c
+++ b/drivers/char/virtio_console.c
@@ -10,6 +10,7 @@
  :*/
 
 /* 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
@@ -25,12 +26,20 @@
  * 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/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"
 
 struct virtio_console_struct {
@@ -41,20 +50,15 @@ struct virtio_console_struct {
 	struct work_struct rx_work;
 	struct work_struct tx_work;
 
-	/* Buffer management */
+	struct list_head port_head;
 	struct list_head unused_read_head;
 	struct list_head unused_write_head;
-	struct list_head readbuf_head;
 
 	/* To protect the list of unused write buffers and the out_vq */
 	spinlock_t write_list_lock;
 
-	/*
-	 * To protect the readbuf_head list. Has to be a spinlock
-	 * because it can be called from interrupt context
-	 * (get_char())
-	 */
-	spinlock_t readbuf_list_lock;
+	/* The current config space is stored here */
+	struct virtio_console_config config;
 
 	/*D:340
 	 * These represent our input and output console queues,
@@ -63,11 +67,22 @@ struct virtio_console_struct {
 	struct virtqueue *in_vq, *out_vq;
 	struct virtio_device *vdev;
 
-	/* The hvc device */
-	struct hvc_struct *hvc;
+	struct class *class;
+
+	/*
+	 * This is used to keep track of the number of hvc consoles
+	 * spawned.  This number is given as first argument to
+	 * hvc_alloc().  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 also just assume the first console being initialised was
+	 * the first one that got used as the initial console.
+	 */
+	unsigned int hvc_vtermno;
 };
 
-/* This struct holds individual buffers received from the host */
+/* This struct holds individual buffers received for each port */
 struct virtio_console_port_buffer {
 	struct list_head list;
 
@@ -79,76 +94,206 @@ struct virtio_console_port_buffer {
 	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 list;
+
+	/* Pointer to the virtio_console device */
+	struct virtio_console_struct *vcon;
+
+	/* 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
+	 * (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;
+
+	/* Is the host device open */
+	bool host_connected;
+};
+
 struct virtio_console_struct virtconsole;
 
-/*D:310
- * The put_chars() callback is pretty straightforward.
- *
- * 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 output spans more than a page, it's split into
- * page-sized buffers and then individual buffers are pushed to the Host.
- */
-static int put_chars(u32 vtermno, const char *in_buf, int in_count)
+static struct virtio_console_port *get_port_from_vtermno(u32 vtermno)
+{
+	struct virtio_console_port *port;
+
+	list_for_each_entry(port, &virtconsole.port_head, list) {
+		if (port->hvc && port->vtermno == vtermno)
+			return port;
+	}
+	return NULL;
+}
+
+static struct virtio_console_port *get_port_from_id(u32 id)
+{
+	struct virtio_console_port *port;
+
+	list_for_each_entry(port, &virtconsole.port_head, list) {
+		if (port->id == id)
+			return port;
+	}
+	return NULL;
+}
+
+static struct virtio_console_port *get_port_from_hvc(struct hvc_struct *hvc)
+{
+	struct virtio_console_port *port;
+
+	list_for_each_entry(port, &virtconsole.port_head, list) {
+		if (port->hvc == hvc)
+			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);
+}
+
+static inline bool is_control(u32 flags)
+{
+	return flags & VIRTIO_CONSOLE_ID_CONTROL;
+}
+
+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];
+	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 = virtconsole.out_vq;
-
+	out_vq = port->vcon->out_vq;
+	/*
+	 * We should not send control messages to a host that won't
+	 * understand them
+	 */
+	if (!use_multiport(port->vcon) && is_control(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, PAGE_SIZE);
+		copy_size = min(in_count - in_offset + header_len, PAGE_SIZE);
 
-		spin_lock(&virtconsole.write_list_lock);
+		spin_lock(&port->vcon->write_list_lock);
 		list_for_each_entry_safe(buf, buf2,
-					 &virtconsole.unused_write_head,
+					 &port->vcon->unused_write_head,
 					 list) {
 			list_del(&buf->list);
 			break;
 		}
-		spin_unlock(&virtconsole.write_list_lock);
+		spin_unlock(&port->vcon->write_list_lock);
 		if (!buf)
 			break;
-		/*
-		 * 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_offset, copy_size);
-
-		buf->len = copy_size;
+		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(&virtconsole.write_list_lock);
+		spin_lock(&port->vcon->write_list_lock);
 		ret = out_vq->vq_ops->add_buf(out_vq, sg, 1, 0, buf);
-		spin_unlock(&virtconsole.write_list_lock);
+		spin_unlock(&port->vcon->write_list_lock);
 		if (ret < 0) {
 			memset(buf->buf, 0, buf->len);
-			spin_lock(&virtconsole.write_list_lock);
+			spin_lock(&port->vcon->write_list_lock);
 			list_add_tail(&buf->list,
-				      &virtconsole.unused_write_head);
-			spin_unlock(&virtconsole.write_list_lock);
+				      &port->vcon->unused_write_head);
+			spin_unlock(&port->vcon->write_list_lock);
 			break;
 		}
-		in_offset += buf->len;
+		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(&virtconsole.write_list_lock);
+	spin_lock(&port->vcon->write_list_lock);
 	out_vq->vq_ops->kick(out_vq);
-	spin_unlock(&virtconsole.write_list_lock);
+	spin_unlock(&port->vcon->write_list_lock);
 
 	/* We're expected to return the amount of data we wrote */
 	return in_offset;
@@ -158,7 +303,8 @@ static int put_chars(u32 vtermno, const char *in_buf, int in_count)
  * Give out the data that's requested from the buffers that we have
  * queued up.
  */
-static ssize_t fill_readbuf(char *out_buf, size_t out_count)
+static ssize_t fill_readbuf(struct virtio_console_port *port,
+			    char *out_buf, size_t out_count, bool to_user)
 {
 	struct virtio_console_port_buffer *buf, *buf2;
 	ssize_t out_offset, ret;
@@ -168,26 +314,38 @@ static ssize_t fill_readbuf(char *out_buf, size_t out_count)
 	 * 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, &virtconsole.readbuf_head, list) {
+	list_for_each_entry_safe(buf, buf2, &port->readbuf_head, list) {
 		size_t copy_size;
 
 		copy_size = out_count;
 		if (copy_size > buf->len - buf->offset)
 			copy_size = buf->len - buf->offset;
 
-		memcpy(out_buf + out_offset, buf->buf + buf->offset, copy_size);
+		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 = copy_size - ret;
 		buf->offset += ret;
 		out_offset += ret;
 		out_count -= ret;
 
 		if (buf->len - buf->offset == 0) {
-			spin_lock(&virtconsole.readbuf_list_lock);
+			spin_lock(&port->readbuf_list_lock);
 			list_del(&buf->list);
-			spin_unlock(&virtconsole.readbuf_list_lock);
+			spin_unlock(&port->readbuf_list_lock);
 			kfree(buf->buf);
 			kfree(buf);
 		}
@@ -197,6 +355,156 @@ static ssize_t fill_readbuf(char *out_buf, size_t out_count)
 	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;
+
+	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;
+
+	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;
+
+	port = filp->private_data;
+
+	return send_buf(port, ubuf, count, 0, true);
+}
+
+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)
+		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_CONTROL, false);
+
+	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;
+
+	/* 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_CONTROL, 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 put_chars() callback is pretty straightforward.
+ *
+ * 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 output spans more than a page, it's split into
+ * page-sized buffers and then individual buffers are pushed to the Host.
+ */
+static int put_chars(u32 vtermno, const char *buf, int count)
+{
+	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
  * get_chars() is the callback from the hvc_console infrastructure when
  * an interrupt is received.
@@ -206,13 +514,19 @@ static ssize_t fill_readbuf(char *out_buf, size_t out_count)
  */
 static int 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);
 
-	if (list_empty(&virtconsole.readbuf_head))
+	port = get_port_from_vtermno(vtermno);
+	if (!port)
 		return 0;
 
-	return fill_readbuf(buf, count);
+	if (list_empty(&port->readbuf_head))
+		return 0;
+
+	return fill_readbuf(port, buf, count, false);
 }
 /*:*/
 
@@ -220,29 +534,36 @@ static int get_chars(u32 vtermno, char *buf, int count)
  * virtio console configuration. This supports:
  * - console resize
  */
-static void virtcons_apply_config(struct virtio_device *dev)
+static void virtcons_apply_config(struct virtio_console_port *port)
 {
+	struct virtio_device *vdev;
 	struct winsize ws;
 
-	if (virtio_has_feature(dev, VIRTIO_CONSOLE_F_SIZE)) {
-		dev->config->get(dev,
-				 offsetof(struct virtio_console_config, cols),
-				 &ws.ws_col, sizeof(u16));
-		dev->config->get(dev,
-				 offsetof(struct virtio_console_config, rows),
-				 &ws.ws_row, sizeof(u16));
-		hvc_resize(virtconsole.hvc, ws);
+	vdev = port->vcon->vdev;
+	if (virtio_has_feature(port->vcon->vdev, VIRTIO_CONSOLE_F_SIZE)) {
+		vdev->config->get(vdev,
+				  offsetof(struct virtio_console_config, cols),
+				  &ws.ws_col, sizeof(u16));
+		vdev->config->get(vdev,
+				  offsetof(struct virtio_console_config, rows),
+				  &ws.ws_row, sizeof(u16));
+		hvc_resize(port->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)
 {
+	struct virtio_console_port *port;
+
+	port = get_port_from_hvc(hp);
+	if (!port)
+		return -EINVAL;
+
 	hp->irq_requested = 1;
-	virtcons_apply_config(virtconsole.vdev);
+	virtcons_apply_config(port);
 
 	return 0;
 }
@@ -276,6 +597,91 @@ int __init virtio_cons_early_init(int (*put_chars)(u32, const char *, int))
 	return hvc_instantiate(0, 0, &virtio_cons);
 }
 
+int init_port_console(struct virtio_console_port *port)
+{
+	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(port->vcon->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 = port->vcon->hvc_vtermno++;
+	}
+	return ret;
+}
+
+/* Any private 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;
+		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->hvc->irq_requested = 1;
+		virtcons_apply_config(port);
+		break;
+	}
+}
+
 static struct virtio_console_port_buffer *get_buf(size_t buf_size)
 {
 	struct virtio_console_port_buffer *buf;
@@ -343,30 +749,76 @@ static void alloc_write_bufs(struct virtio_console_struct *vcon)
 	}
 }
 
+/*
+ * The workhandler for any buffers that appear on our input queue.
+ * Pick the buffer; if it's some internal control 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;
+	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->list);
 
+		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 = 0;
+		buf->offset = header_len;
+		if (use_multiport(vcon) && is_control(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;
+
+			spin_lock(&port->readbuf_list_lock);
+			list_add_tail(&buf->list, &port->readbuf_head);
+			spin_unlock(&port->readbuf_list_lock);
+		}
+		wake_up_interruptible(&port->waitqueue);
 
-		spin_lock(&virtconsole.readbuf_list_lock);
-		list_add_tail(&buf->list, &virtconsole.readbuf_head);
-		spin_unlock(&virtconsole.readbuf_list_lock);
+		if (is_console_port(port) && !list_empty(&port->readbuf_head))
+			if (hvc_poll(port->hvc))
+				hvc_kick();
 	}
-	if (hvc_poll(virtconsole.hvc))
-		hvc_kick();
-
 	/* Allocate buffers for all the ones that got used up */
 	fill_receive_queue(&virtconsole);
 }
@@ -417,23 +869,95 @@ static void tx_intr(struct virtqueue *vq)
 
 static void config_intr(struct virtio_device *vdev)
 {
-	/* Handle console size changes */
-	virtcons_apply_config(vdev);
+	/*
+	 * We'll use this way of resizing only for legacy support. For
+	 * newer userspace (VIRTIO_CONSOLE_F_MULTPORT+), use control
+	 * messages to indicate console size changes so that it can be
+	 * done per-port
+	 */
+	virtcons_apply_config(get_port_from_id(0));
+}
+
+static int virtconsole_add_port(u32 port_nr)
+{
+	struct virtio_console_port *port;
+	struct virtio_console_control cpkt;
+	dev_t devt;
+	int ret;
+
+	port = kzalloc(sizeof(*port), GFP_KERNEL);
+	if (!port)
+		return -ENOMEM;
+
+	port->vcon = &virtconsole;
+	port->id = port_nr;
+
+	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;
+	}
+	/*
+	 * 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;
+	}
+
+	spin_lock_init(&port->readbuf_list_lock);
+	INIT_LIST_HEAD(&port->readbuf_head);
+	init_waitqueue_head(&port->waitqueue);
+
+	list_add_tail(&port->list, &port->vcon->port_head);
+
+	/* Ask for the port's name from Host. */
+	cpkt.event = VIRTIO_CONSOLE_PORT_NAME;
+	send_buf(port, (char *)&cpkt, sizeof(cpkt),
+		 VIRTIO_CONSOLE_ID_CONTROL, false);
+
+	return 0;
+free_cdev:
+	cdev_del(&port->cdev);
+free_chrdev:
+	unregister_chrdev_region(devt, 1);
+free_port:
+	kfree(port);
+	return ret;
 }
 
-/*D:370 Once we're further in boot, we get probed like any other virtio device.
+/*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.
  *
- * 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.
- *
- * Finally we put our input buffer in the input queue, ready to receive. */
+ * 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)
 {
 	vq_callback_t *callbacks[] = { rx_intr, tx_intr };
 	const char *names[] = { "input", "output" };
 	struct virtqueue *vqs[2];
+	u32 i;
 	int err;
+	bool multiport;
 
 	if (virtconsole.vdev) {
 		pr_err("Multiple virtio-console devices not supported yet\n");
@@ -441,6 +965,17 @@ static int __devinit virtcons_probe(struct virtio_device *vdev)
 	}
 	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_ports),
+				  &virtconsole.config.nr_ports,
+				  sizeof(virtconsole.config.nr_ports));
+	}
 	/* Find the queues. */
 	/* FIXME: This is why we want to wean off hvc: we do nothing
 	 * when input comes in. */
@@ -459,10 +994,9 @@ static int __devinit virtcons_probe(struct virtio_device *vdev)
 	 */
 	virtio_cons.put_chars = put_chars;
 
-	spin_lock_init(&virtconsole.readbuf_list_lock);
 	spin_lock_init(&virtconsole.write_list_lock);
 
-	INIT_LIST_HEAD(&virtconsole.readbuf_head);
+	INIT_LIST_HEAD(&virtconsole.port_head);
 	INIT_LIST_HEAD(&virtconsole.unused_read_head);
 	INIT_LIST_HEAD(&virtconsole.unused_write_head);
 
@@ -472,25 +1006,13 @@ static int __devinit virtcons_probe(struct virtio_device *vdev)
 	fill_receive_queue(&virtconsole);
 	alloc_write_bufs(&virtconsole);
 
-	/* 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. */
-	virtconsole.hvc = hvc_alloc(0, 0, &virtio_cons, PAGE_SIZE);
-	if (IS_ERR(virtconsole.hvc)) {
-		err = PTR_ERR(virtconsole.hvc);
-		goto free_vqs;
-	}
+	virtconsole_add_port(0);
+	if (multiport)
+		for (i = 1; i < virtconsole.config.nr_ports; i++)
+			virtconsole_add_port(i);
 
 	return 0;
 
-free_vqs:
-	vdev->config->del_vqs(vdev);
 fail:
 	return err;
 }
@@ -502,6 +1024,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 = {
@@ -516,7 +1039,20 @@ static struct virtio_driver virtio_console = {
 
 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;
+	}
+	return 0;
 }
 module_init(init);
 
diff --git a/include/linux/virtio_console.h b/include/linux/virtio_console.h
index fe88517..b93dba4 100644
--- a/include/linux/virtio_console.h
+++ b/include/linux/virtio_console.h
@@ -3,19 +3,59 @@
 #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? */
 
 struct virtio_console_config {
 	/* colums of the screens */
 	__u16 cols;
 	/* rows of the screens */
 	__u16 rows;
+	/* 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 {
+	__u16 event;
+	__u16 value;
+};
+
+/* Some events for control messages */
+#define VIRTIO_CONSOLE_PORT_OPEN	0
+#define VIRTIO_CONSOLE_PORT_NAME	1
+#define VIRTIO_CONSOLE_CONSOLE_PORT	2
+#define VIRTIO_CONSOLE_RESIZE		3
+
+/*
+ * 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_CONTROL	(1 << 0)
 
 #ifdef __KERNEL__
 int __init virtio_cons_early_init(int (*put_chars)(u32, const char *, int));
-- 
1.6.2.5

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH 09/15] virtio_console: Handle port hot-plug
  2009-11-10  9:41               ` [PATCH 08/15] virtio_console: Add support for multiple ports: console as well as generic Amit Shah
@ 2009-11-10  9:41                 ` Amit Shah
  2009-11-10  9:41                   ` [PATCH 10/15] virtio_console: Register with sysfs and create a 'name' attribute Amit Shah
  0 siblings, 1 reply; 20+ messages in thread
From: Amit Shah @ 2009-11-10  9:41 UTC (permalink / raw)
  To: rusty; +Cc: Amit Shah, virtualization

If the 'nr_ports' variable in the config space is updated to
a higher value, that means new ports have been hotplugged.

Introduce a new workqueue to handle such updates and create
new ports.

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

diff --git a/drivers/char/virtio_console.c b/drivers/char/virtio_console.c
index 4119c37..3458340 100644
--- a/drivers/char/virtio_console.c
+++ b/drivers/char/virtio_console.c
@@ -49,6 +49,7 @@ struct virtio_console_struct {
 	 */
 	struct work_struct rx_work;
 	struct work_struct tx_work;
+	struct work_struct config_work;
 
 	struct list_head port_head;
 	struct list_head unused_read_head;
@@ -869,6 +870,10 @@ static void tx_intr(struct virtqueue *vq)
 
 static void config_intr(struct virtio_device *vdev)
 {
+	if (use_multiport(&virtconsole)) {
+		/* Handle port hot-add */
+		schedule_work(&virtconsole.config_work);
+	}
 	/*
 	 * We'll use this way of resizing only for legacy support. For
 	 * newer userspace (VIRTIO_CONSOLE_F_MULTPORT+), use control
@@ -942,6 +947,53 @@ free_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 err;
+
+	vcon = container_of(work, struct virtio_console_struct, config_work);
+
+	vdev = vcon->vdev;
+	vdev->config->get(vdev,
+			  offsetof(struct virtio_console_config, nr_ports),
+			  &virtconconf.nr_ports,
+			  sizeof(virtconconf.nr_ports));
+
+	if (vcon->config.nr_ports == virtconconf.nr_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_CONTROL, false);
+		return;
+	}
+	if (virtconconf.nr_ports < vcon->config.nr_ports)
+		return;
+
+	/* Hot-add ports */
+	while(virtconconf.nr_ports - vcon->config.nr_ports) {
+		err = virtconsole_add_port(vcon->config.nr_ports);
+		if (err)
+			break;
+		vcon->config.nr_ports++;
+	}
+}
+
 /*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.
@@ -1002,6 +1054,7 @@ static int __devinit virtcons_probe(struct virtio_device *vdev)
 
 	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);
 
 	fill_receive_queue(&virtconsole);
 	alloc_write_bufs(&virtconsole);
-- 
1.6.2.5

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH 10/15] virtio_console: Register with sysfs and create a 'name' attribute
  2009-11-10  9:41                 ` [PATCH 09/15] virtio_console: Handle port hot-plug Amit Shah
@ 2009-11-10  9:41                   ` Amit Shah
  2009-11-10  9:41                     ` [PATCH 11/15] virtio_console: Ensure only one process can have a port open at a time Amit Shah
  0 siblings, 1 reply; 20+ messages in thread
From: Amit Shah @ 2009-11-10  9:41 UTC (permalink / raw)
  To: rusty; +Cc: Amit Shah, virtualization

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

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

diff --git a/drivers/char/virtio_console.c b/drivers/char/virtio_console.c
index 3458340..c3498ec 100644
--- a/drivers/char/virtio_console.c
+++ b/drivers/char/virtio_console.c
@@ -153,6 +153,17 @@ static struct virtio_console_port *get_port_from_vtermno(u32 vtermno)
 	return NULL;
 }
 
+static struct virtio_console_port *get_port_from_devt(dev_t devt)
+{
+	struct virtio_console_port *port;
+
+	list_for_each_entry(port, &virtconsole.port_head, list) {
+		if (port->dev->devt == devt)
+			return port;
+	}
+	return NULL;
+}
+
 static struct virtio_console_port *get_port_from_id(u32 id)
 {
 	struct virtio_console_port *port;
@@ -883,6 +894,30 @@ static void config_intr(struct virtio_device *vdev)
 	virtcons_apply_config(get_port_from_id(0));
 }
 
+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 virtconsole_add_port(u32 port_nr)
 {
 	struct virtio_console_port *port;
@@ -917,6 +952,12 @@ static int virtconsole_add_port(u32 port_nr)
 		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;
+	}
 	/*
 	 * If we're not using multiport support, this has to be a console port
 	 */
-- 
1.6.2.5

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH 11/15] virtio_console: Ensure only one process can have a port open at a time
  2009-11-10  9:41                   ` [PATCH 10/15] virtio_console: Register with sysfs and create a 'name' attribute Amit Shah
@ 2009-11-10  9:41                     ` Amit Shah
  2009-11-10  9:41                       ` [PATCH 12/15] virtio_console: Add debugfs files for each port to expose debug info Amit Shah
  0 siblings, 1 reply; 20+ messages in thread
From: Amit Shah @ 2009-11-10  9:41 UTC (permalink / raw)
  To: rusty; +Cc: Amit Shah, virtualization

Add a guest_connected field that ensures only one process
can have a port open at a time.

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

diff --git a/drivers/char/virtio_console.c b/drivers/char/virtio_console.c
index c3498ec..8c9bac8 100644
--- a/drivers/char/virtio_console.c
+++ b/drivers/char/virtio_console.c
@@ -138,6 +138,9 @@ struct virtio_console_port {
 
 	/* Is the host device open */
 	bool host_connected;
+
+	/* We should allow only one process to open a port */
+	bool guest_connected;
 };
 
 struct virtio_console_struct virtconsole;
@@ -455,6 +458,10 @@ static int virtconsole_release(struct inode *inode, struct file *filp)
 	send_buf(filp->private_data, (char *)&cpkt, sizeof(cpkt),
 		 VIRTIO_CONSOLE_ID_CONTROL, false);
 
+	spin_lock(&port->readbuf_list_lock);
+	port->guest_connected = false;
+	spin_unlock(&port->readbuf_list_lock);
+
 	return 0;
 }
 
@@ -474,6 +481,15 @@ static int virtconsole_open(struct inode *inode, struct file *filp)
 	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;
@@ -639,6 +655,7 @@ int init_port_console(struct virtio_console_port *port)
 		port->hvc = NULL;
 	} else {
 		port->vtermno = port->vcon->hvc_vtermno++;
+		port->guest_connected = true;
 	}
 	return ret;
 }
-- 
1.6.2.5

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH 12/15] virtio_console: Add debugfs files for each port to expose debug info
  2009-11-10  9:41                     ` [PATCH 11/15] virtio_console: Ensure only one process can have a port open at a time Amit Shah
@ 2009-11-10  9:41                       ` Amit Shah
  2009-11-10  9:41                         ` [PATCH 13/15] virtio_console: Add throttling support to prevent flooding ports Amit Shah
  0 siblings, 1 reply; 20+ messages in thread
From: Amit Shah @ 2009-11-10  9:41 UTC (permalink / raw)
  To: rusty; +Cc: Amit Shah, virtualization

This is helpful in examining ports' state.

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

diff --git a/drivers/char/virtio_console.c b/drivers/char/virtio_console.c
index 8c9bac8..f7db92a 100644
--- a/drivers/char/virtio_console.c
+++ b/drivers/char/virtio_console.c
@@ -28,6 +28,7 @@
  */
 
 #include <linux/cdev.h>
+#include <linux/debugfs.h>
 #include <linux/device.h>
 #include <linux/err.h>
 #include <linux/fs.h>
@@ -70,6 +71,9 @@ struct virtio_console_struct {
 
 	struct class *class;
 
+	/* Used for exporting per-port information to debugfs */
+	struct dentry *debugfs_dir;
+
 	/*
 	 * This is used to keep track of the number of hvc consoles
 	 * spawned.  This number is given as first argument to
@@ -103,6 +107,9 @@ struct virtio_console_port {
 	/* 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;
 
@@ -935,10 +942,53 @@ static struct attribute_group virtcon_attribute_group = {
 	.attrs = virtcon_sysfs_entries,
 };
 
+static int virtcon_port_debugfs_open(struct inode *inode, struct file *filp)
+{
+	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,
+			       "host_connected: %d\n", port->host_connected);
+	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);
+
+	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;
 
@@ -983,7 +1033,6 @@ static int virtconsole_add_port(u32 port_nr)
 		if (ret)
 			goto free_cdev;
 	}
-
 	spin_lock_init(&port->readbuf_list_lock);
 	INIT_LIST_HEAD(&port->readbuf_head);
 	init_waitqueue_head(&port->waitqueue);
@@ -995,6 +1044,15 @@ static int virtconsole_add_port(u32 port_nr)
 	send_buf(port, (char *)&cpkt, sizeof(cpkt),
 		 VIRTIO_CONSOLE_ID_CONTROL, false);
 
+	/*
+	 * 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);
@@ -1163,6 +1221,10 @@ static int __init init(void)
 		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;
 }
 module_init(init);
-- 
1.6.2.5

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH 13/15] virtio_console: Add throttling support to prevent flooding ports
  2009-11-10  9:41                       ` [PATCH 12/15] virtio_console: Add debugfs files for each port to expose debug info Amit Shah
@ 2009-11-10  9:41                         ` Amit Shah
  2009-11-10  9:41                           ` [PATCH 14/15] virtio_console: Add option to remove cached buffers on port close Amit Shah
  0 siblings, 1 reply; 20+ messages in thread
From: Amit Shah @ 2009-11-10  9:41 UTC (permalink / raw)
  To: rusty; +Cc: Amit Shah, virtualization

Rogue processes on guests or hosts could pump in data
and cause an OOM condition. Add support for throttling
so that a limit to the number of outstanding buffers
can be specified.

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

diff --git a/drivers/char/virtio_console.c b/drivers/char/virtio_console.c
index f7db92a..c45b987 100644
--- a/drivers/char/virtio_console.c
+++ b/drivers/char/virtio_console.c
@@ -143,11 +143,32 @@ struct virtio_console_port {
 	 */
 	u32 vtermno;
 
+	/*
+	 * 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;
+
+	/*
+	 * The actual number of buffers we have pending in the
+	 * readbuf_head list
+	 */
+	u32 nr_buffers;
+
 	/* Is the host device open */
 	bool host_connected;
 
 	/* We should allow only one process to open a port */
 	bool guest_connected;
+
+	/* Does the Host not want to accept more data currently?  */
+	bool host_throttled;
+
+	/* Have we sent out a throttle request to the Host? */
+	bool guest_throttled;
 };
 
 struct virtio_console_struct virtconsole;
@@ -367,6 +388,7 @@ static ssize_t fill_readbuf(struct virtio_console_port *port,
 		if (buf->len - buf->offset == 0) {
 			spin_lock(&port->readbuf_list_lock);
 			list_del(&buf->list);
+			port->nr_buffers--;
 			spin_unlock(&port->readbuf_list_lock);
 			kfree(buf->buf);
 			kfree(buf);
@@ -374,6 +396,15 @@ static ssize_t fill_readbuf(struct virtio_console_port *port,
 		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_CONTROL, false);
+		port->guest_throttled = false;
+	}
 	return out_offset;
 }
 
@@ -430,6 +461,9 @@ static ssize_t virtconsole_write(struct file *filp, const char __user *ubuf,
 
 	port = filp->private_data;
 
+	if (port->host_throttled)
+		return -ENOSPC;
+
 	return send_buf(port, ubuf, count, 0, true);
 }
 
@@ -444,7 +478,7 @@ static unsigned int virtconsole_poll(struct file *filp, poll_table *wait)
 	ret = 0;
 	if (!list_empty(&port->readbuf_head))
 		ret |= POLLIN | POLLRDNORM;
-	if (port->host_connected)
+	if (port->host_connected && !port->host_throttled)
 		ret |= POLLOUT;
 	if (!port->host_connected)
 		ret |= POLLHUP;
@@ -715,6 +749,35 @@ static void handle_control_message(struct virtio_console_port *port,
 		port->hvc->irq_requested = 1;
 		virtcons_apply_config(port);
 		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;
 	}
 }
 
@@ -845,9 +908,31 @@ static void virtio_console_rx_work_handler(struct work_struct *work)
 			 */
 			port->host_connected = true;
 
+			if (port->guest_throttled) {
+				kfree(buf->buf);
+				kfree(buf);
+				continue;
+			}
 			spin_lock(&port->readbuf_list_lock);
 			list_add_tail(&buf->list, &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 (virtio_has_feature(port->vcon->vdev,
+					       VIRTIO_CONSOLE_F_THROTTLE)
+			    && (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_CONTROL, false);
+			}
 		}
 		wake_up_interruptible(&port->waitqueue);
 
@@ -967,8 +1052,16 @@ static ssize_t virtcon_port_debugfs_read(struct file *filp, char __user *ubuf,
 	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,
+			       "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,
 			       "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);
@@ -999,6 +1092,8 @@ static int virtconsole_add_port(u32 port_nr)
 	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");
@@ -1144,6 +1239,10 @@ static int __devinit virtcons_probe(struct virtio_device *vdev)
 				  &virtconsole.config.nr_ports,
 				  sizeof(virtconsole.config.nr_ports));
 	}
+	if (virtio_has_feature(vdev, VIRTIO_CONSOLE_F_THROTTLE)) {
+		vdev->features[0] |= 1 << VIRTIO_CONSOLE_F_THROTTLE;
+		vdev->config->finalize_features(vdev);
+	}
 	/* Find the queues. */
 	/* FIXME: This is why we want to wean off hvc: we do nothing
 	 * when input comes in. */
@@ -1194,6 +1293,7 @@ static struct virtio_device_id id_table[] = {
 static unsigned int features[] = {
 	VIRTIO_CONSOLE_F_SIZE,
 	VIRTIO_CONSOLE_F_MULTIPORT,
+	VIRTIO_CONSOLE_F_THROTTLE,
 };
 
 static struct virtio_driver virtio_console = {
diff --git a/include/linux/virtio_console.h b/include/linux/virtio_console.h
index b93dba4..5336a75 100644
--- a/include/linux/virtio_console.h
+++ b/include/linux/virtio_console.h
@@ -13,6 +13,7 @@
 /* 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_F_THROTTLE  2	/* Do we throttle data to prevent OOM */
 
 struct virtio_console_config {
 	/* colums of the screens */
@@ -37,6 +38,8 @@ struct virtio_console_control {
 #define VIRTIO_CONSOLE_PORT_NAME	1
 #define VIRTIO_CONSOLE_CONSOLE_PORT	2
 #define VIRTIO_CONSOLE_RESIZE		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
-- 
1.6.2.5

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH 14/15] virtio_console: Add option to remove cached buffers on port close
  2009-11-10  9:41                         ` [PATCH 13/15] virtio_console: Add throttling support to prevent flooding ports Amit Shah
@ 2009-11-10  9:41                           ` Amit Shah
  2009-11-10  9:41                             ` [PATCH 15/15] virtio_console: Add ability to hot-unplug ports Amit Shah
  0 siblings, 1 reply; 20+ messages in thread
From: Amit Shah @ 2009-11-10  9:41 UTC (permalink / raw)
  To: rusty; +Cc: Amit Shah, virtualization

If caching of buffers upon closing a port is not desired,
delete all the buffers queued up for the port upon port
close.

This only applies to generic ports; console ports aren't
opened / closed by the chardev interface.

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

diff --git a/drivers/char/virtio_console.c b/drivers/char/virtio_console.c
index c45b987..90bb221 100644
--- a/drivers/char/virtio_console.c
+++ b/drivers/char/virtio_console.c
@@ -169,6 +169,9 @@ struct virtio_console_port {
 
 	/* 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;
 };
 
 struct virtio_console_struct virtconsole;
@@ -501,6 +504,21 @@ static int virtconsole_release(struct inode *inode, struct file *filp)
 
 	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, list) {
+			list_del(&buf->list);
+			kfree(buf->buf);
+			kfree(buf);
+			port->nr_buffers--;
+		}
+	}
 	spin_unlock(&port->readbuf_list_lock);
 
 	return 0;
@@ -778,6 +796,9 @@ static void handle_control_message(struct virtio_console_port *port,
 		 */
 		port->buffer_limit = (u32)cpkt->value * 1024 / 4;
 		break;
+	case VIRTIO_CONSOLE_CACHE_BUFFERS:
+		port->cache_buffers = cpkt->value;
+		break;
 	}
 }
 
@@ -907,7 +928,20 @@ static void virtio_console_rx_work_handler(struct work_struct *work)
 			 * 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);
@@ -1058,6 +1092,8 @@ static ssize_t virtcon_port_debugfs_read(struct file *filp, char __user *ubuf,
 	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);
+	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);
@@ -1232,17 +1268,19 @@ static int __devinit virtcons_probe(struct virtio_device *vdev)
 	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_ports),
 				  &virtconsole.config.nr_ports,
 				  sizeof(virtconsole.config.nr_ports));
 	}
-	if (virtio_has_feature(vdev, VIRTIO_CONSOLE_F_THROTTLE)) {
+	if (virtio_has_feature(vdev, VIRTIO_CONSOLE_F_THROTTLE))
 		vdev->features[0] |= 1 << VIRTIO_CONSOLE_F_THROTTLE;
-		vdev->config->finalize_features(vdev);
-	}
+
+	if (virtio_has_feature(vdev, VIRTIO_CONSOLE_F_CACHING))
+		vdev->features[0] |= 1 << VIRTIO_CONSOLE_F_CACHING;
+
+	vdev->config->finalize_features(vdev);
 	/* Find the queues. */
 	/* FIXME: This is why we want to wean off hvc: we do nothing
 	 * when input comes in. */
@@ -1294,6 +1332,7 @@ static unsigned int features[] = {
 	VIRTIO_CONSOLE_F_SIZE,
 	VIRTIO_CONSOLE_F_MULTIPORT,
 	VIRTIO_CONSOLE_F_THROTTLE,
+	VIRTIO_CONSOLE_F_CACHING,
 };
 
 static struct virtio_driver virtio_console = {
diff --git a/include/linux/virtio_console.h b/include/linux/virtio_console.h
index 5336a75..ce5eef2 100644
--- a/include/linux/virtio_console.h
+++ b/include/linux/virtio_console.h
@@ -14,6 +14,7 @@
 #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_F_THROTTLE  2	/* Do we throttle data to prevent OOM */
+#define VIRTIO_CONSOLE_F_CACHING   3	/* Do we cache data after port close? */
 
 struct virtio_console_config {
 	/* colums of the screens */
@@ -40,6 +41,7 @@ struct virtio_console_control {
 #define VIRTIO_CONSOLE_RESIZE		3
 #define VIRTIO_CONSOLE_THROTTLE_PORT	4
 #define VIRTIO_CONSOLE_BUFFER_LIMIT	5
+#define VIRTIO_CONSOLE_CACHE_BUFFERS	6
 
 /*
  * This struct is put in each buffer that gets passed to userspace and
-- 
1.6.2.5

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH 15/15] virtio_console: Add ability to hot-unplug ports
  2009-11-10  9:41                           ` [PATCH 14/15] virtio_console: Add option to remove cached buffers on port close Amit Shah
@ 2009-11-10  9:41                             ` Amit Shah
  0 siblings, 0 replies; 20+ messages in thread
From: Amit Shah @ 2009-11-10  9:41 UTC (permalink / raw)
  To: rusty; +Cc: Amit Shah, virtualization

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  |   64 ++++++++++++++++++++++++++++++++++++++++
 include/linux/virtio_console.h |    2 +
 2 files changed, 66 insertions(+), 0 deletions(-)

diff --git a/drivers/char/virtio_console.c b/drivers/char/virtio_console.c
index 90bb221..9035f80 100644
--- a/drivers/char/virtio_console.c
+++ b/drivers/char/virtio_console.c
@@ -719,6 +719,42 @@ int init_port_console(struct virtio_console_port *port)
 	return ret;
 }
 
+static struct attribute_group virtcon_attribute_group;
+
+/* Remove port-specific data. */
+static int virtcons_remove_port_data(struct virtio_console_port *port)
+{
+	struct virtio_console_port_buffer *buf, *buf2;
+	struct virtio_console_control cpkt;
+
+	if (port->guest_connected) {
+		cpkt.event = VIRTIO_CONSOLE_PORT_OPEN;
+		cpkt.value = 0;
+		send_buf(port, (char *)&cpkt, sizeof(cpkt),
+			 VIRTIO_CONSOLE_ID_CONTROL, false);
+	}
+	if (is_console_port(port))
+		hvc_remove(port->hvc);
+
+	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, list) {
+		list_del(&buf->list);
+		kfree(buf->buf);
+		kfree(buf);
+	}
+	spin_unlock(&port->readbuf_list_lock);
+	debugfs_remove(port->debugfs_file);
+	return 0;
+}
+
 /* Any private 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)
@@ -799,6 +835,30 @@ static void handle_control_message(struct virtio_console_port *port,
 	case VIRTIO_CONSOLE_CACHE_BUFFERS:
 		port->cache_buffers = cpkt->value;
 		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).
+		 *
+		 */
+		virtcons_remove_port_data(port);
+		break;
 	}
 }
 
@@ -1280,6 +1340,9 @@ static int __devinit virtcons_probe(struct virtio_device *vdev)
 	if (virtio_has_feature(vdev, VIRTIO_CONSOLE_F_CACHING))
 		vdev->features[0] |= 1 << VIRTIO_CONSOLE_F_CACHING;
 
+	if (virtio_has_feature(vdev, VIRTIO_CONSOLE_F_UNPLUG))
+		vdev->features[0] |= 1 << VIRTIO_CONSOLE_F_UNPLUG;
+
 	vdev->config->finalize_features(vdev);
 	/* Find the queues. */
 	/* FIXME: This is why we want to wean off hvc: we do nothing
@@ -1333,6 +1396,7 @@ static unsigned int features[] = {
 	VIRTIO_CONSOLE_F_MULTIPORT,
 	VIRTIO_CONSOLE_F_THROTTLE,
 	VIRTIO_CONSOLE_F_CACHING,
+	VIRTIO_CONSOLE_F_UNPLUG,
 };
 
 static struct virtio_driver virtio_console = {
diff --git a/include/linux/virtio_console.h b/include/linux/virtio_console.h
index ce5eef2..703696a 100644
--- a/include/linux/virtio_console.h
+++ b/include/linux/virtio_console.h
@@ -15,6 +15,7 @@
 #define VIRTIO_CONSOLE_F_MULTIPORT 1	/* Does host provide multiple ports? */
 #define VIRTIO_CONSOLE_F_THROTTLE  2	/* Do we throttle data to prevent OOM */
 #define VIRTIO_CONSOLE_F_CACHING   3	/* Do we cache data after port close? */
+#define VIRTIO_CONSOLE_F_UNPLUG	4	/* Can we hot-unplug ports? */
 
 struct virtio_console_config {
 	/* colums of the screens */
@@ -42,6 +43,7 @@ struct virtio_console_control {
 #define VIRTIO_CONSOLE_THROTTLE_PORT	4
 #define VIRTIO_CONSOLE_BUFFER_LIMIT	5
 #define VIRTIO_CONSOLE_CACHE_BUFFERS	6
+#define VIRTIO_CONSOLE_PORT_REMOVE	7
 
 /*
  * This struct is put in each buffer that gets passed to userspace and
-- 
1.6.2.5

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* Re: [PATCH 01/15] virtio_console: Initialize hv_ops struct at declaration instead of in the probe() routine
  2009-11-10  9:41 ` [PATCH 01/15] virtio_console: Initialize hv_ops struct at declaration instead of in the probe() routine Amit Shah
  2009-11-10  9:41   ` [PATCH 02/15] virtio_console: We support only one device at a time Amit Shah
@ 2009-11-10 13:07   ` Rusty Russell
  2009-11-11 18:42     ` Amit Shah
  1 sibling, 1 reply; 20+ messages in thread
From: Rusty Russell @ 2009-11-10 13:07 UTC (permalink / raw)
  To: Amit Shah; +Cc: virtualization

On Tue, 10 Nov 2009 08:11:27 pm Amit Shah wrote:
> Initialize the hv_ops function pointers using C99 style. However, we
> need to re-init the 'put_chars' field to our implementation in the
> probe() routine as we replace it for early_init to use the caller's
> put_chars implementation.

(I've decided one email for all the feedback, because obviously the merge
of the two series will cause substantial changes)

It's interesting to contrast this with mine.  There's nothing *wrong*
with this, but the larger issue of making as much kernel data const lead
me to the separate early_put_chars hook.

Re: [PATCH 02/15] virtio_console: We support only one device at a time

> We support only one virtio_console device at a time. If multiple are
> found, error out if one is already initialized.

I like this; it should be integrated early in my series; technically it's
a fix.  But it should use dev_err() (or dev_warn maybe) not pr_err().

Re: [PATCH 03/15] virtio_console: Club all globals into one struct

I chose "port" as a name; it's better once we're finished than
"struct virtio_console_struct".  You also dropped the static, which is
sloppy.

Re: [PATCH 04/15] virtio_console: Introduce a workqueue for handling host->guest notifications

This one I disagree with.  The host will wait until a buffer comes available.
It has to anyway, and it should work just fine.  *If* we need speed, then we
might need workqueues so we reduce guest exits, but I don't think we need that
yet.

Will defer most of the other patches until we sort this issue out.  But one
minor thing:

Re: [PATCH 10/15] virtio_console: Register with sysfs and create a 'name' attribute

Is there a better way to have that attribute dynamic, rather than just having
the sysfs file there but empty if it has no name?

I think get_port_from_dev_t cannot fail (BUG() at the end?).

Thanks!
Rusty.

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 01/15] virtio_console: Initialize hv_ops struct at declaration instead of in the probe() routine
  2009-11-10 13:07   ` [PATCH 01/15] virtio_console: Initialize hv_ops struct at declaration instead of in the probe() routine Rusty Russell
@ 2009-11-11 18:42     ` Amit Shah
  2009-11-12  2:01       ` Rusty Russell
  0 siblings, 1 reply; 20+ messages in thread
From: Amit Shah @ 2009-11-11 18:42 UTC (permalink / raw)
  To: Rusty Russell; +Cc: virtualization

On (Tue) Nov 10 2009 [23:37:39], Rusty Russell wrote:
> On Tue, 10 Nov 2009 08:11:27 pm Amit Shah wrote:
> > Initialize the hv_ops function pointers using C99 style. However, we
> > need to re-init the 'put_chars' field to our implementation in the
> > probe() routine as we replace it for early_init to use the caller's
> > put_chars implementation.
> 
> (I've decided one email for all the feedback, because obviously the merge
> of the two series will cause substantial changes)
> 
> It's interesting to contrast this with mine.  There's nothing *wrong*
> with this, but the larger issue of making as much kernel data const lead
> me to the separate early_put_chars hook.

Yeah; I'll use yours.

> Re: [PATCH 02/15] virtio_console: We support only one device at a time
> 
> > We support only one virtio_console device at a time. If multiple are
> > found, error out if one is already initialized.
> 
> I like this; it should be integrated early in my series; technically it's
> a fix.  But it should use dev_err() (or dev_warn maybe) not pr_err().

Sure.

> Re: [PATCH 03/15] virtio_console: Club all globals into one struct
> 
> I chose "port" as a name; it's better once we're finished than
> "struct virtio_console_struct". 

I wanted to separate out a 'virtio device' which has vqs common to all
ports and a 'port', which has its buffers, tells us if it's a console
port, and all the other port-specific stuff.

I find 'port' and 'ports' confusing. In your implementation, 'port' has
vqs.

> You also dropped the static, which is
> sloppy.

Yeah; I still have a small diff compared to my single big patch and this
is one of the hunks.

> Re: [PATCH 04/15] virtio_console: Introduce a workqueue for handling host->guest notifications
> 
> This one I disagree with.  The host will wait until a buffer comes available.
> It has to anyway, and it should work just fine.  *If* we need speed, then we
> might need workqueues so we reduce guest exits, but I don't think we need that
> yet.

So each port has a buffer that gets added to the vq and when data is
received, it would be queued in the port-specific ring. Ports may want
data to be cached when they're not open or when the userspace app is
slow in reading the data. In that case, we'll have to allocate a new
buffer to be stuffed in the vq.

For the output case too a workqueue might be needed -- the output
routine can be called from interrupt context (put_chars); so sleeping is
best deferred to a workqueue?

> Re: [PATCH 10/15] virtio_console: Register with sysfs and create a 'name' attribute
> 
> Is there a better way to have that attribute dynamic, rather than just having
> the sysfs file there but empty if it has no name?

I think it should be easy to do (but what I need to figure out first is
how to get the port from the devt given we can now support multiple
virtio console devices).

> I think get_port_from_dev_t cannot fail (BUG() at the end?).

Will add.

Thanks, Rusty!

		Amit

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 01/15] virtio_console: Initialize hv_ops struct at declaration instead of in the probe() routine
  2009-11-11 18:42     ` Amit Shah
@ 2009-11-12  2:01       ` Rusty Russell
  2009-11-12  5:54         ` Amit Shah
  0 siblings, 1 reply; 20+ messages in thread
From: Rusty Russell @ 2009-11-12  2:01 UTC (permalink / raw)
  To: Amit Shah; +Cc: virtualization

On Thu, 12 Nov 2009 05:12:54 am Amit Shah wrote:
> On (Tue) Nov 10 2009 [23:37:39], Rusty Russell wrote:
> > I chose "port" as a name; it's better once we're finished than
> > "struct virtio_console_struct". 
> 
> I wanted to separate out a 'virtio device' which has vqs common to all
> ports and a 'port', which has its buffers, tells us if it's a console
> port, and all the other port-specific stuff.
> 
> I find 'port' and 'ports' confusing. In your implementation, 'port' has
> vqs.

Yep, that's wrong, no wonder you found it confusing.  They should be in the
'struct ports'.  ie. vq == struct ports =contains= N x struct port.

I'm steering away from the word "console", since that will now be one use
of this driver.  We may end up renaming it "virtio_ports".

> > Re: [PATCH 04/15] virtio_console: Introduce a workqueue for handling host->guest notifications
> > 
> > This one I disagree with.  The host will wait until a buffer comes available.
> > It has to anyway, and it should work just fine.  *If* we need speed, then we
> > might need workqueues so we reduce guest exits, but I don't think we need that
> > yet.
> 
> So each port has a buffer that gets added to the vq and when data is
> received, it would be queued in the port-specific ring. Ports may want
> data to be cached when they're not open or when the userspace app is
> slow in reading the data. In that case, we'll have to allocate a new
> buffer to be stuffed in the vq.

OK, I was thinking one buffer per vq.  But that doesn't really work for input,
since we might get input on any port, and not want it.  Seems OK for output
tho.

Hmm, radical idea: how about 1 vq per input?  Share one for output, but
separate ones for each input port?  That limits us to 255 ports, but would
allow the simplistic "just put a buffer in the queue when you want to read".

May need a control input vq as well in this case?

Thanks,
Rusty.

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 01/15] virtio_console: Initialize hv_ops struct at declaration instead of in the probe() routine
  2009-11-12  2:01       ` Rusty Russell
@ 2009-11-12  5:54         ` Amit Shah
  0 siblings, 0 replies; 20+ messages in thread
From: Amit Shah @ 2009-11-12  5:54 UTC (permalink / raw)
  To: Rusty Russell; +Cc: virtualization

On (Thu) Nov 12 2009 [12:31:59], Rusty Russell wrote:
> On Thu, 12 Nov 2009 05:12:54 am Amit Shah wrote:
> > On (Tue) Nov 10 2009 [23:37:39], Rusty Russell wrote:
> > > I chose "port" as a name; it's better once we're finished than
> > > "struct virtio_console_struct". 
> > 
> > I wanted to separate out a 'virtio device' which has vqs common to all
> > ports and a 'port', which has its buffers, tells us if it's a console
> > port, and all the other port-specific stuff.
> > 
> > I find 'port' and 'ports' confusing. In your implementation, 'port' has
> > vqs.
> 
> Yep, that's wrong, no wonder you found it confusing.  They should be in the
> 'struct ports'.  ie. vq == struct ports =contains= N x struct port.
> 
> I'm steering away from the word "console", since that will now be one use
> of this driver.  We may end up renaming it "virtio_ports".

Cool!

> > > Re: [PATCH 04/15] virtio_console: Introduce a workqueue for handling host->guest notifications
> > > 
> > > This one I disagree with.  The host will wait until a buffer comes available.
> > > It has to anyway, and it should work just fine.  *If* we need speed, then we
> > > might need workqueues so we reduce guest exits, but I don't think we need that
> > > yet.
> > 
> > So each port has a buffer that gets added to the vq and when data is
> > received, it would be queued in the port-specific ring. Ports may want
> > data to be cached when they're not open or when the userspace app is
> > slow in reading the data. In that case, we'll have to allocate a new
> > buffer to be stuffed in the vq.
> 
> OK, I was thinking one buffer per vq.  But that doesn't really work for input,
> since we might get input on any port, and not want it.  Seems OK for output
> tho.

One buffer works OK for output, but again the waiting might be the
issue.

> Hmm, radical idea: how about 1 vq per input?  Share one for output, but
> separate ones for each input port?  That limits us to 255 ports, but would
> allow the simplistic "just put a buffer in the queue when you want to read".
> 
> May need a control input vq as well in this case?

I went down that path too; only to discover hotplug will be an issue
with the new MSI code: it wants us to pre-declare all the vqs. That's
some overhead to incur. Also, (as Avi pointed out initially) one vq per
port is too much of an overhead anyway for such simple functionality.

Given this, can you please look at the series I had posted (I'll move it
to your series plus some delta) so that I have a general idea of
proceeding before git-rebase drives me totally mad? :-)

Thanks,
		Amit

^ permalink raw reply	[flat|nested] 20+ messages in thread

end of thread, other threads:[~2009-11-12  5:54 UTC | newest]

Thread overview: 20+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2009-11-10  9:41 virtio_console: support for multiple ports, console and generic Amit Shah
2009-11-10  9:41 ` [PATCH 01/15] virtio_console: Initialize hv_ops struct at declaration instead of in the probe() routine Amit Shah
2009-11-10  9:41   ` [PATCH 02/15] virtio_console: We support only one device at a time Amit Shah
2009-11-10  9:41     ` [PATCH 03/15] virtio_console: Club all globals into one struct Amit Shah
2009-11-10  9:41       ` [PATCH 04/15] virtio_console: Introduce a workqueue for handling host->guest notifications Amit Shah
2009-11-10  9:41         ` [PATCH 05/15] virtio_console: Buffer data that comes in from the host Amit Shah
2009-11-10  9:41           ` [PATCH 06/15] virtio_console: Create a buffer pool for sending data to host Amit Shah
2009-11-10  9:41             ` [PATCH 07/15] virtio_console: config: Prepare for handling multiple ports and hotplug Amit Shah
2009-11-10  9:41               ` [PATCH 08/15] virtio_console: Add support for multiple ports: console as well as generic Amit Shah
2009-11-10  9:41                 ` [PATCH 09/15] virtio_console: Handle port hot-plug Amit Shah
2009-11-10  9:41                   ` [PATCH 10/15] virtio_console: Register with sysfs and create a 'name' attribute Amit Shah
2009-11-10  9:41                     ` [PATCH 11/15] virtio_console: Ensure only one process can have a port open at a time Amit Shah
2009-11-10  9:41                       ` [PATCH 12/15] virtio_console: Add debugfs files for each port to expose debug info Amit Shah
2009-11-10  9:41                         ` [PATCH 13/15] virtio_console: Add throttling support to prevent flooding ports Amit Shah
2009-11-10  9:41                           ` [PATCH 14/15] virtio_console: Add option to remove cached buffers on port close Amit Shah
2009-11-10  9:41                             ` [PATCH 15/15] virtio_console: Add ability to hot-unplug ports Amit Shah
2009-11-10 13:07   ` [PATCH 01/15] virtio_console: Initialize hv_ops struct at declaration instead of in the probe() routine Rusty Russell
2009-11-11 18:42     ` Amit Shah
2009-11-12  2:01       ` Rusty Russell
2009-11-12  5:54         ` Amit Shah

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.