Linux Virtualization - Apr 2010 - [PATCH 00/11] (v6) virtio: console: Fixes, new way of discovering ports

Hello,

This series reworks the ABI to allow port discovery (only) via the
control queue and enable multiport again.

In addition, it adds support for non-blocking write() support, which
means no spinning. This works fine with the recent patches that are on
qemu-devel.

Also included is removal of hvc_remove() as removing one such console
port causes other console ports (registered with hvc) to stall. This
has to be debugged in the hvc_console.c file, I'll do that later, but
we have a nice workaround for this: returning -EPIPE on any hvc
operations will make the hvc console core perform any cleanups for the
removed ports. Looks like we don't lose much by removing hvc_remove().

New in this version:
- Locking for the out_vq. Slightly changed version compared to the
  diff I sent earlier in that I don't lock inside
  reclaim_used_buffers() now, letting us use spin_lock_irq() instead
  of spin_lock_irqsave() from non-irq contexts.
- Remove extra line spotted by Rusty

Rusty, please apply for -next if all is well.

Amit Shah (11):
  Revert "virtio: disable multiport console support."
  virtio: console: Add a __send_control_msg() that can send messages
    without a valid port
  virtio: console: Let host know of port or device add failures
  virtio: console: Return -EPIPE to hvc_console if we lost the
    connection
  virtio: console: Don't call hvc_remove() on unplugging console ports
  virtio: console: Remove config work handler
  virtio: console: Move code around for future patches
  virtio: console: Use a control message to add ports
  virtio: console: Don't always create a port 0 if using multiport
  virtio: console: Rename wait_is_over() to will_read_block()
  virtio: console: Add support for nonblocking write()s

 drivers/char/virtio_console.c  |  605 ++++++++++++++++++++--------------------
 include/linux/virtio_console.h |   25 ++
 2 files changed, 334 insertions(+), 296 deletions(-)

This reverts commit b7a413015d2986edf020fba765c906cc9cbcbfc9.

Multiport support was disabled for 2.6.34 because we wanted to introduce
a new ABI and since we didn't have any released kernel with the older
ABI and were out of the merge window, it didn't make sense keeping the
older ABI around.

Now we revert the patch disabling multiport and rework the ABI in the
following patches.
---
 drivers/char/virtio_console.c  |   49 ++++++---------------------------------
 include/linux/virtio_console.h |   23 ++++++++++++++++++
 2 files changed, 31 insertions(+), 41 deletions(-)

diff --git a/drivers/char/virtio_console.c b/drivers/char/virtio_console.c
index 196428c..86e9011 100644
--- a/drivers/char/virtio_console.c
+++ b/drivers/char/virtio_console.c
@@ -33,35 +33,6 @@
 #include <linux/workqueue.h>
 #include "hvc_console.h"
 
-/* Moved here from .h file in order to disable MULTIPORT. */
-#define VIRTIO_CONSOLE_F_MULTIPORT 1	/* Does host provide multiple ports? */
-
-struct virtio_console_multiport_conf {
-	struct virtio_console_config config;
-	/* max. number of ports this device can hold */
-	__u32 max_nr_ports;
-	/* number of ports added so far */
-	__u32 nr_ports;
-} __attribute__((packed));
-
-/*
- * A message that's passed between the Host and the Guest for a
- * particular port.
- */
-struct virtio_console_control {
-	__u32 id;		/* Port number */
-	__u16 event;		/* The kind of control event (see below) */
-	__u16 value;		/* Extra information for the key */
-};
-
-/* Some events for control messages */
-#define VIRTIO_CONSOLE_PORT_READY	0
-#define VIRTIO_CONSOLE_CONSOLE_PORT	1
-#define VIRTIO_CONSOLE_RESIZE		2
-#define VIRTIO_CONSOLE_PORT_OPEN	3
-#define VIRTIO_CONSOLE_PORT_NAME	4
-#define VIRTIO_CONSOLE_PORT_REMOVE	5
-
 /*
  * This is a global struct for storing common data for all the devices
  * this driver handles.
@@ -150,7 +121,7 @@ struct ports_device {
 	spinlock_t cvq_lock;
 
 	/* The current config space is stored here */
-	struct virtio_console_multiport_conf config;
+	struct virtio_console_config config;
 
 	/* The virtio device we're associated with */
 	struct virtio_device *vdev;
@@ -1243,7 +1214,7 @@ fail:
  */
 static void config_work_handler(struct work_struct *work)
 {
-	struct virtio_console_multiport_conf virtconconf;
+	struct virtio_console_config virtconconf;
 	struct ports_device *portdev;
 	struct virtio_device *vdev;
 	int err;
@@ -1252,8 +1223,7 @@ static void config_work_handler(struct work_struct *work)
 
 	vdev = portdev->vdev;
 	vdev->config->get(vdev,
-			  offsetof(struct virtio_console_multiport_conf,
-				   nr_ports),
+			  offsetof(struct virtio_console_config, nr_ports),
 			  &virtconconf.nr_ports,
 			  sizeof(virtconconf.nr_ports));
 
@@ -1445,19 +1415,16 @@ static int __devinit virtcons_probe(struct virtio_device
*vdev)
 	multiport = false;
 	portdev->config.nr_ports = 1;
 	portdev->config.max_nr_ports = 1;
-#if 0 /* Multiport is not quite ready yet --RR */
 	if (virtio_has_feature(vdev, VIRTIO_CONSOLE_F_MULTIPORT)) {
 		multiport = true;
 		vdev->features[0] |= 1 << VIRTIO_CONSOLE_F_MULTIPORT;
 
-		vdev->config->get(vdev,
-				  offsetof(struct virtio_console_multiport_conf,
-					   nr_ports),
+		vdev->config->get(vdev, offsetof(struct virtio_console_config,
+						 nr_ports),
 				  &portdev->config.nr_ports,
 				  sizeof(portdev->config.nr_ports));
-		vdev->config->get(vdev,
-				  offsetof(struct virtio_console_multiport_conf,
-					   max_nr_ports),
+		vdev->config->get(vdev, offsetof(struct virtio_console_config,
+						 max_nr_ports),
 				  &portdev->config.max_nr_ports,
 				  sizeof(portdev->config.max_nr_ports));
 		if (portdev->config.nr_ports > portdev->config.max_nr_ports) {
@@ -1473,7 +1440,6 @@ static int __devinit virtcons_probe(struct virtio_device
*vdev)
 
 	/* Let the Host know we support multiple ports.*/
 	vdev->config->finalize_features(vdev);
-#endif
 
 	err = init_vqs(portdev);
 	if (err < 0) {
@@ -1556,6 +1522,7 @@ static struct virtio_device_id id_table[] = {
 
 static unsigned int features[] = {
 	VIRTIO_CONSOLE_F_SIZE,
+	VIRTIO_CONSOLE_F_MULTIPORT,
 };
 
 static struct virtio_driver virtio_console = {
diff --git a/include/linux/virtio_console.h b/include/linux/virtio_console.h
index 92228a8..ae4f039 100644
--- a/include/linux/virtio_console.h
+++ b/include/linux/virtio_console.h
@@ -12,14 +12,37 @@
 
 /* Feature bits */
 #define VIRTIO_CONSOLE_F_SIZE	0	/* Does host provide console size? */
+#define VIRTIO_CONSOLE_F_MULTIPORT 1	/* Does host provide multiple ports? */
 
 struct virtio_console_config {
 	/* colums of the screens */
 	__u16 cols;
 	/* rows of the screens */
 	__u16 rows;
+	/* max. number of ports this device can hold */
+	__u32 max_nr_ports;
+	/* number of ports added so far */
+	__u32 nr_ports;
 } __attribute__((packed));
 
+/*
+ * A message that's passed between the Host and the Guest for a
+ * particular port.
+ */
+struct virtio_console_control {
+	__u32 id;		/* Port number */
+	__u16 event;		/* The kind of control event (see below) */
+	__u16 value;		/* Extra information for the key */
+};
+
+/* Some events for control messages */
+#define VIRTIO_CONSOLE_PORT_READY	0
+#define VIRTIO_CONSOLE_CONSOLE_PORT	1
+#define VIRTIO_CONSOLE_RESIZE		2
+#define VIRTIO_CONSOLE_PORT_OPEN	3
+#define VIRTIO_CONSOLE_PORT_NAME	4
+#define VIRTIO_CONSOLE_PORT_REMOVE	5
+
 #ifdef __KERNEL__
 int __init virtio_cons_early_init(int (*put_chars)(u32, const char *, int));
 #endif /* __KERNEL__ */
-- 
1.6.2.5

We will introduce control messages that operate on the device as a whole
rather than just ports. Make send_control_msg() a wrapper around
__send_control_msg() which does not need a valid port.

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

diff --git a/drivers/char/virtio_console.c b/drivers/char/virtio_console.c
index 86e9011..cfd6aa9 100644
--- a/drivers/char/virtio_console.c
+++ b/drivers/char/virtio_console.c
@@ -374,22 +374,22 @@ out:
 	return ret;
 }
 
-static ssize_t send_control_msg(struct port *port, unsigned int event,
-				unsigned int value)
+static ssize_t __send_control_msg(struct ports_device *portdev, u32 port_id,
+				  unsigned int event, unsigned int value)
 {
 	struct scatterlist sg[1];
 	struct virtio_console_control cpkt;
 	struct virtqueue *vq;
 	unsigned int len;
 
-	if (!use_multiport(port->portdev))
+	if (!use_multiport(portdev))
 		return 0;
 
-	cpkt.id = port->id;
+	cpkt.id = port_id;
 	cpkt.event = event;
 	cpkt.value = value;
 
-	vq = port->portdev->c_ovq;
+	vq = portdev->c_ovq;
 
 	sg_init_one(sg, &cpkt, sizeof(cpkt));
 	if (vq->vq_ops->add_buf(vq, sg, 1, 0, &cpkt) >= 0) {
@@ -400,6 +400,12 @@ static ssize_t send_control_msg(struct port *port, unsigned
int event,
 	return 0;
 }
 
+static ssize_t send_control_msg(struct port *port, unsigned int event,
+				unsigned int value)
+{
+	return __send_control_msg(port->portdev, port->id, event, value);
+}
+
 static ssize_t send_buf(struct port *port, void *in_buf, size_t in_count)
 {
 	struct scatterlist sg[1];
-- 
1.6.2.5

The host may want to know and let management apps notify of port or
device add failures. Send a control message saying the device or port is
not ready in this case.

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

diff --git a/drivers/char/virtio_console.c b/drivers/char/virtio_console.c
index cfd6aa9..5aa1891 100644
--- a/drivers/char/virtio_console.c
+++ b/drivers/char/virtio_console.c
@@ -1210,6 +1210,8 @@ free_cdev:
 free_port:
 	kfree(port);
 fail:
+	/* The host might want to notify management sw about port add failure */
+	send_control_msg(port, VIRTIO_CONSOLE_PORT_READY, 0);
 	return err;
 }
 
@@ -1488,6 +1490,9 @@ free_chrdev:
 free:
 	kfree(portdev);
 fail:
+	/* The host might want to notify mgmt sw about device add failure */
+	__send_control_msg(portdev, VIRTIO_CONSOLE_BAD_ID,
+			   VIRTIO_CONSOLE_DEVICE_READY, 0);
 	return err;
 }
 
diff --git a/include/linux/virtio_console.h b/include/linux/virtio_console.h
index ae4f039..0157361 100644
--- a/include/linux/virtio_console.h
+++ b/include/linux/virtio_console.h
@@ -14,6 +14,8 @@
 #define VIRTIO_CONSOLE_F_SIZE	0	/* Does host provide console size? */
 #define VIRTIO_CONSOLE_F_MULTIPORT 1	/* Does host provide multiple ports? */
 
+#define VIRTIO_CONSOLE_BAD_ID		(~(u32)0)
+
 struct virtio_console_config {
 	/* colums of the screens */
 	__u16 cols;
@@ -42,6 +44,7 @@ struct virtio_console_control {
 #define VIRTIO_CONSOLE_PORT_OPEN	3
 #define VIRTIO_CONSOLE_PORT_NAME	4
 #define VIRTIO_CONSOLE_PORT_REMOVE	5
+#define VIRTIO_CONSOLE_DEVICE_READY	6
 
 #ifdef __KERNEL__
 int __init virtio_cons_early_init(int (*put_chars)(u32, const char *, int));
-- 
1.6.2.5