From 505b0451c47699ca63db70bd5ec3bba187ec4bfd Mon Sep 17 00:00:00 2001 From: "Michael S. Tsirkin" Date: Mon, 12 Apr 2010 16:18:32 +0300 Subject: virtio_console: use virtqueue_xxx wrappers Switch virtio_console to new virtqueue_xxx wrappers. Signed-off-by: Michael S. Tsirkin Signed-off-by: Rusty Russell --- drivers/char/virtio_console.c | 32 ++++++++++++++++---------------- 1 file changed, 16 insertions(+), 16 deletions(-) (limited to 'drivers/char') diff --git a/drivers/char/virtio_console.c b/drivers/char/virtio_console.c index 196428c2287a..48ce834306b5 100644 --- a/drivers/char/virtio_console.c +++ b/drivers/char/virtio_console.c @@ -328,7 +328,7 @@ static void *get_inbuf(struct port *port) unsigned int len; vq = port->in_vq; - buf = vq->vq_ops->get_buf(vq, &len); + buf = virtqueue_get_buf(vq, &len); if (buf) { buf->len = len; buf->offset = 0; @@ -349,8 +349,8 @@ static int add_inbuf(struct virtqueue *vq, struct port_buffer *buf) sg_init_one(sg, buf->buf, buf->size); - ret = vq->vq_ops->add_buf(vq, sg, 0, 1, buf); - vq->vq_ops->kick(vq); + ret = virtqueue_add_buf(vq, sg, 0, 1, buf); + virtqueue_kick(vq); return ret; } @@ -366,7 +366,7 @@ static void discard_port_data(struct port *port) if (port->inbuf) buf = port->inbuf; else - buf = vq->vq_ops->get_buf(vq, &len); + buf = virtqueue_get_buf(vq, &len); ret = 0; while (buf) { @@ -374,7 +374,7 @@ static void discard_port_data(struct port *port) ret++; free_buf(buf); } - buf = vq->vq_ops->get_buf(vq, &len); + buf = virtqueue_get_buf(vq, &len); } port->inbuf = NULL; if (ret) @@ -421,9 +421,9 @@ static ssize_t send_control_msg(struct port *port, unsigned int event, vq = port->portdev->c_ovq; sg_init_one(sg, &cpkt, sizeof(cpkt)); - if (vq->vq_ops->add_buf(vq, sg, 1, 0, &cpkt) >= 0) { - vq->vq_ops->kick(vq); - while (!vq->vq_ops->get_buf(vq, &len)) + if (virtqueue_add_buf(vq, sg, 1, 0, &cpkt) >= 0) { + virtqueue_kick(vq); + while (!virtqueue_get_buf(vq, &len)) cpu_relax(); } return 0; @@ -439,10 +439,10 @@ static ssize_t send_buf(struct port *port, void *in_buf, size_t in_count) out_vq = port->out_vq; sg_init_one(sg, in_buf, in_count); - ret = out_vq->vq_ops->add_buf(out_vq, sg, 1, 0, in_buf); + ret = virtqueue_add_buf(out_vq, sg, 1, 0, in_buf); /* Tell Host to go! */ - out_vq->vq_ops->kick(out_vq); + virtqueue_kick(out_vq); if (ret < 0) { in_count = 0; @@ -450,7 +450,7 @@ static ssize_t send_buf(struct port *port, void *in_buf, size_t in_count) } /* Wait till the host acknowledges it pushed out the data we sent. */ - while (!out_vq->vq_ops->get_buf(out_vq, &len)) + while (!virtqueue_get_buf(out_vq, &len)) cpu_relax(); fail: /* We're expected to return the amount of data we wrote */ @@ -901,7 +901,7 @@ static int remove_port(struct port *port) discard_port_data(port); /* Remove buffers we queued up for the Host to send us data in. */ - while ((buf = port->in_vq->vq_ops->detach_unused_buf(port->in_vq))) + while ((buf = virtqueue_detach_unused_buf(port->in_vq))) free_buf(buf); kfree(port->name); @@ -1030,7 +1030,7 @@ static void control_work_handler(struct work_struct *work) vq = portdev->c_ivq; spin_lock(&portdev->cvq_lock); - while ((buf = vq->vq_ops->get_buf(vq, &len))) { + while ((buf = virtqueue_get_buf(vq, &len))) { spin_unlock(&portdev->cvq_lock); buf->len = len; @@ -1224,7 +1224,7 @@ static int add_port(struct ports_device *portdev, u32 id) return 0; free_inbufs: - while ((buf = port->in_vq->vq_ops->detach_unused_buf(port->in_vq))) + while ((buf = virtqueue_detach_unused_buf(port->in_vq))) free_buf(buf); free_device: device_destroy(pdrvdata.class, port->dev->devt); @@ -1536,10 +1536,10 @@ static void virtcons_remove(struct virtio_device *vdev) unregister_chrdev(portdev->chr_major, "virtio-portsdev"); - while ((buf = portdev->c_ivq->vq_ops->get_buf(portdev->c_ivq, &len))) + while ((buf = virtqueue_get_buf(portdev->c_ivq, &len))) free_buf(buf); - while ((buf = portdev->c_ivq->vq_ops->detach_unused_buf(portdev->c_ivq))) + while ((buf = virtqueue_detach_unused_buf(portdev->c_ivq))) free_buf(buf); vdev->config->del_vqs(vdev); -- cgit v1.2.3-59-g8ed1b From 28cfc828e7e1efc620df70b99519784333abcffd Mon Sep 17 00:00:00 2001 From: "Michael S. Tsirkin" Date: Tue, 13 Apr 2010 16:11:42 +0300 Subject: virtio-rng: use virtqueue_xxx wrappers Switch virtio-rng to new virtqueue_xxx wrappers. Signed-off-by: Michael S. Tsirkin Signed-off-by: Rusty Russell --- drivers/char/hw_random/virtio-rng.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) (limited to 'drivers/char') diff --git a/drivers/char/hw_random/virtio-rng.c b/drivers/char/hw_random/virtio-rng.c index 64fe0a793efd..75f1cbd61c17 100644 --- a/drivers/char/hw_random/virtio-rng.c +++ b/drivers/char/hw_random/virtio-rng.c @@ -32,7 +32,7 @@ static bool busy; static void random_recv_done(struct virtqueue *vq) { /* We can get spurious callbacks, e.g. shared IRQs + virtio_pci. */ - if (!vq->vq_ops->get_buf(vq, &data_avail)) + if (!virtqueue_get_buf(vq, &data_avail)) return; complete(&have_data); @@ -46,10 +46,10 @@ static void register_buffer(u8 *buf, size_t size) sg_init_one(&sg, buf, size); /* There should always be room for one buffer. */ - if (vq->vq_ops->add_buf(vq, &sg, 0, 1, buf) < 0) + if (virtqueue_add_buf(vq, &sg, 0, 1, buf) < 0) BUG(); - vq->vq_ops->kick(vq); + virtqueue_kick(vq); } static int virtio_read(struct hwrng *rng, void *buf, size_t size, bool wait) -- cgit v1.2.3-59-g8ed1b From b99fa815d71023b2e330d63cd7f47d6247ffa321 Mon Sep 17 00:00:00 2001 From: Amit Shah Date: Wed, 19 May 2010 22:15:46 -0600 Subject: virtio: Revert "virtio: disable multiport console support." 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. Signed-off-by: Amit Shah Signed-off-by: Rusty Russell --- drivers/char/virtio_console.c | 49 +++++++----------------------------------- include/linux/virtio_console.h | 23 ++++++++++++++++++++ 2 files changed, 31 insertions(+), 41 deletions(-) (limited to 'drivers/char') diff --git a/drivers/char/virtio_console.c b/drivers/char/virtio_console.c index 48ce834306b5..e53c52b904fb 100644 --- a/drivers/char/virtio_console.c +++ b/drivers/char/virtio_console.c @@ -33,35 +33,6 @@ #include #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 92228a8fbcbc..ae4f039515b4 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__ */ -- cgit v1.2.3-59-g8ed1b From 3425e706bf6faa2965c4e99f39085f7367a8f4e2 Mon Sep 17 00:00:00 2001 From: Amit Shah Date: Wed, 19 May 2010 22:15:46 -0600 Subject: virtio: console: Add a __send_control_msg() that can send messages without a valid port 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 Signed-off-by: Rusty Russell --- drivers/char/virtio_console.c | 16 +++++++++++----- 1 file changed, 11 insertions(+), 5 deletions(-) (limited to 'drivers/char') diff --git a/drivers/char/virtio_console.c b/drivers/char/virtio_console.c index e53c52b904fb..8c24b86aadc9 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 (virtqueue_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]; -- cgit v1.2.3-59-g8ed1b From eaeff9608a8cf43a676b6f4b6235ea9d76192230 Mon Sep 17 00:00:00 2001 From: Amit Shah Date: Wed, 19 May 2010 22:15:47 -0600 Subject: virtio: console: Let host know of port or device add failures 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 Signed-off-by: Rusty Russell --- drivers/char/virtio_console.c | 5 +++++ include/linux/virtio_console.h | 3 +++ 2 files changed, 8 insertions(+) (limited to 'drivers/char') diff --git a/drivers/char/virtio_console.c b/drivers/char/virtio_console.c index 8c24b86aadc9..f63bf77a4825 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 ae4f039515b4..015736113c25 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)); -- cgit v1.2.3-59-g8ed1b From 6dc69f970231387d8fe646a831920da26408b5f5 Mon Sep 17 00:00:00 2001 From: Amit Shah Date: Wed, 19 May 2010 22:15:47 -0600 Subject: virtio: console: Return -EPIPE to hvc_console if we lost the connection hvc_console handles -EPIPE properly when the connection to the host is lost. Signed-off-by: Amit Shah Signed-off-by: Rusty Russell --- drivers/char/virtio_console.c | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) (limited to 'drivers/char') diff --git a/drivers/char/virtio_console.c b/drivers/char/virtio_console.c index f63bf77a4825..11a9573f901e 100644 --- a/drivers/char/virtio_console.c +++ b/drivers/char/virtio_console.c @@ -653,7 +653,7 @@ static int put_chars(u32 vtermno, const char *buf, int count) port = find_port_by_vtermno(vtermno); if (!port) - return 0; + return -EPIPE; return send_buf(port, (void *)buf, count); } @@ -669,9 +669,13 @@ static int get_chars(u32 vtermno, char *buf, int count) { struct port *port; + /* If we've not set up the port yet, we have no input to give. */ + if (unlikely(early_put_chars)) + return 0; + port = find_port_by_vtermno(vtermno); if (!port) - return 0; + return -EPIPE; /* If we don't have an input queue yet, we can't get input. */ BUG_ON(!port->in_vq); -- cgit v1.2.3-59-g8ed1b From 69eb9a9f69572c0ebe08a0a46f56bdfdcdaa19a0 Mon Sep 17 00:00:00 2001 From: Amit Shah Date: Wed, 19 May 2010 22:15:47 -0600 Subject: virtio: console: Don't call hvc_remove() on unplugging console ports hvc_remove() has some bug which freezes other active hvc ports when one port is removed. So disable calling of hvc_remove() which deregisters a port with the hvc_console. If the hvc_console code calls into our get_chars() routine as a result of a poll operation, we will return -EPIPE and the hvc_console code will then do the necessary cleanup. This call will be restored when the bug in hvc_remove() is found and fixed. Signed-off-by: Amit Shah Signed-off-by: Rusty Russell --- drivers/char/virtio_console.c | 11 +++++++++++ 1 file changed, 11 insertions(+) (limited to 'drivers/char') diff --git a/drivers/char/virtio_console.c b/drivers/char/virtio_console.c index 11a9573f901e..9a698707e14b 100644 --- a/drivers/char/virtio_console.c +++ b/drivers/char/virtio_console.c @@ -869,7 +869,18 @@ static int remove_port(struct port *port) spin_lock_irq(&pdrvdata_lock); list_del(&port->cons.list); spin_unlock_irq(&pdrvdata_lock); +#if 0 + /* + * hvc_remove() not called as removing one hvc port + * results in other hvc ports getting frozen. + * + * Once this is resolved in hvc, this functionality + * will be enabled. Till that is done, the -EPIPE + * return from get_chars() above will help + * hvc_console.c to clean up on ports we remove here. + */ hvc_remove(port->cons.hvc); +#endif } if (port->guest_connected) send_control_msg(port, VIRTIO_CONSOLE_PORT_OPEN, 0); -- cgit v1.2.3-59-g8ed1b From 99f905f88a5b8478755605e08ed4bce40034cc6c Mon Sep 17 00:00:00 2001 From: Amit Shah Date: Wed, 19 May 2010 22:15:48 -0600 Subject: virtio: console: Remove config work handler We're going to switch to using control messages for port hot-plug and initial port discovery. Remove the config work handler which handled port hot-plug so far. Signed-off-by: Amit Shah Signed-off-by: Rusty Russell --- drivers/char/virtio_console.c | 64 +------------------------------------------ 1 file changed, 1 insertion(+), 63 deletions(-) (limited to 'drivers/char') diff --git a/drivers/char/virtio_console.c b/drivers/char/virtio_console.c index 9a698707e14b..e1f92a0c5c5f 100644 --- a/drivers/char/virtio_console.c +++ b/drivers/char/virtio_console.c @@ -110,7 +110,6 @@ struct ports_device { * notification */ struct work_struct control_work; - struct work_struct config_work; struct list_head ports; @@ -1084,10 +1083,7 @@ static void config_intr(struct virtio_device *vdev) struct ports_device *portdev; portdev = vdev->priv; - if (use_multiport(portdev)) { - /* Handle port hot-add */ - schedule_work(&portdev->config_work); - } + /* * We'll use this way of resizing only for legacy support. * For newer userspace (VIRTIO_CONSOLE_F_MULTPORT+), use @@ -1230,62 +1226,6 @@ fail: return err; } -/* - * The workhandler for config-space updates. - * - * This is called when ports are hot-added. - */ -static void config_work_handler(struct work_struct *work) -{ - struct virtio_console_config virtconconf; - struct ports_device *portdev; - struct virtio_device *vdev; - int err; - - portdev = container_of(work, struct ports_device, config_work); - - vdev = portdev->vdev; - vdev->config->get(vdev, - offsetof(struct virtio_console_config, nr_ports), - &virtconconf.nr_ports, - sizeof(virtconconf.nr_ports)); - - if (portdev->config.nr_ports == virtconconf.nr_ports) { - /* - * Port 0 got hot-added. Since we already did all the - * other initialisation for it, just tell the Host - * that the port is ready if we find the port. In - * case the port was hot-removed earlier, we call - * add_port to add the port. - */ - struct port *port; - - port = find_port_by_id(portdev, 0); - if (!port) - add_port(portdev, 0); - else - send_control_msg(port, VIRTIO_CONSOLE_PORT_READY, 1); - return; - } - if (virtconconf.nr_ports > portdev->config.max_nr_ports) { - dev_warn(&vdev->dev, - "More ports specified (%u) than allowed (%u)", - portdev->config.nr_ports + 1, - portdev->config.max_nr_ports); - return; - } - if (virtconconf.nr_ports < portdev->config.nr_ports) - return; - - /* Hot-add ports */ - while (virtconconf.nr_ports - portdev->config.nr_ports) { - err = add_port(portdev, portdev->config.nr_ports); - if (err) - break; - portdev->config.nr_ports++; - } -} - static int init_vqs(struct ports_device *portdev) { vq_callback_t **io_callbacks; @@ -1478,7 +1418,6 @@ static int __devinit virtcons_probe(struct virtio_device *vdev) spin_lock_init(&portdev->cvq_lock); INIT_WORK(&portdev->control_work, &control_work_handler); - INIT_WORK(&portdev->config_work, &config_work_handler); nr_added_bufs = fill_queue(portdev->c_ivq, &portdev->cvq_lock); if (!nr_added_bufs) { @@ -1521,7 +1460,6 @@ static void virtcons_remove(struct virtio_device *vdev) portdev = vdev->priv; cancel_work_sync(&portdev->control_work); - cancel_work_sync(&portdev->config_work); list_for_each_entry_safe(port, port2, &portdev->ports, list) remove_port(port); -- cgit v1.2.3-59-g8ed1b From c446f8fcc9fba3369bffb894b31756cf7a09f783 Mon Sep 17 00:00:00 2001 From: Amit Shah Date: Wed, 19 May 2010 22:15:48 -0600 Subject: virtio: console: Move code around for future patches We're going to use add_port() from handle_control_message() in the next patch. Move the add_port() and fill_queue(), which depends on it, above handle_control_message() to avoid forward declarations. Signed-off-by: Amit Shah Signed-off-by: Rusty Russell --- drivers/char/virtio_console.c | 266 +++++++++++++++++++++--------------------- 1 file changed, 133 insertions(+), 133 deletions(-) (limited to 'drivers/char') diff --git a/drivers/char/virtio_console.c b/drivers/char/virtio_console.c index e1f92a0c5c5f..237eee26fbc3 100644 --- a/drivers/char/virtio_console.c +++ b/drivers/char/virtio_console.c @@ -855,6 +855,139 @@ static const struct file_operations port_debugfs_ops = { .read = debugfs_read, }; +static unsigned int fill_queue(struct virtqueue *vq, spinlock_t *lock) +{ + struct port_buffer *buf; + unsigned int nr_added_bufs; + int ret; + + nr_added_bufs = 0; + do { + buf = alloc_buf(PAGE_SIZE); + if (!buf) + break; + + spin_lock_irq(lock); + ret = add_inbuf(vq, buf); + if (ret < 0) { + spin_unlock_irq(lock); + free_buf(buf); + break; + } + nr_added_bufs++; + spin_unlock_irq(lock); + } while (ret > 0); + + return nr_added_bufs; +} + +static int add_port(struct ports_device *portdev, u32 id) +{ + char debugfs_name[16]; + struct port *port; + struct port_buffer *buf; + dev_t devt; + unsigned int nr_added_bufs; + int err; + + port = kmalloc(sizeof(*port), GFP_KERNEL); + if (!port) { + err = -ENOMEM; + goto fail; + } + + port->portdev = portdev; + port->id = id; + + port->name = NULL; + port->inbuf = NULL; + port->cons.hvc = NULL; + + port->host_connected = port->guest_connected = false; + + port->in_vq = portdev->in_vqs[port->id]; + port->out_vq = portdev->out_vqs[port->id]; + + cdev_init(&port->cdev, &port_fops); + + devt = MKDEV(portdev->chr_major, id); + err = cdev_add(&port->cdev, devt, 1); + if (err < 0) { + dev_err(&port->portdev->vdev->dev, + "Error %d adding cdev for port %u\n", err, id); + goto free_port; + } + port->dev = device_create(pdrvdata.class, &port->portdev->vdev->dev, + devt, port, "vport%up%u", + port->portdev->drv_index, id); + if (IS_ERR(port->dev)) { + err = PTR_ERR(port->dev); + dev_err(&port->portdev->vdev->dev, + "Error %d creating device for port %u\n", + err, id); + goto free_cdev; + } + + spin_lock_init(&port->inbuf_lock); + init_waitqueue_head(&port->waitqueue); + + /* Fill the in_vq with buffers so the host can send us data. */ + nr_added_bufs = fill_queue(port->in_vq, &port->inbuf_lock); + if (!nr_added_bufs) { + dev_err(port->dev, "Error allocating inbufs\n"); + err = -ENOMEM; + goto free_device; + } + + /* + * If we're not using multiport support, this has to be a console port + */ + if (!use_multiport(port->portdev)) { + err = init_port_console(port); + if (err) + goto free_inbufs; + } + + spin_lock_irq(&portdev->ports_lock); + list_add_tail(&port->list, &port->portdev->ports); + spin_unlock_irq(&portdev->ports_lock); + + /* + * Tell the Host we're set so that it can send us various + * configuration parameters for this port (eg, port name, + * caching, whether this is a console port, etc.) + */ + send_control_msg(port, VIRTIO_CONSOLE_PORT_READY, 1); + + if (pdrvdata.debugfs_dir) { + /* + * Finally, create the debugfs file that we can use to + * inspect a port's state at any time + */ + sprintf(debugfs_name, "vport%up%u", + port->portdev->drv_index, id); + port->debugfs_file = debugfs_create_file(debugfs_name, 0444, + pdrvdata.debugfs_dir, + port, + &port_debugfs_ops); + } + return 0; + +free_inbufs: + while ((buf = virtqueue_detach_unused_buf(port->in_vq))) + free_buf(buf); +free_device: + device_destroy(pdrvdata.class, port->dev->devt); +free_cdev: + cdev_del(&port->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; +} + /* Remove all port-specific data. */ static int remove_port(struct port *port) { @@ -1093,139 +1226,6 @@ static void config_intr(struct virtio_device *vdev) resize_console(find_port_by_id(portdev, 0)); } -static unsigned int fill_queue(struct virtqueue *vq, spinlock_t *lock) -{ - struct port_buffer *buf; - unsigned int nr_added_bufs; - int ret; - - nr_added_bufs = 0; - do { - buf = alloc_buf(PAGE_SIZE); - if (!buf) - break; - - spin_lock_irq(lock); - ret = add_inbuf(vq, buf); - if (ret < 0) { - spin_unlock_irq(lock); - free_buf(buf); - break; - } - nr_added_bufs++; - spin_unlock_irq(lock); - } while (ret > 0); - - return nr_added_bufs; -} - -static int add_port(struct ports_device *portdev, u32 id) -{ - char debugfs_name[16]; - struct port *port; - struct port_buffer *buf; - dev_t devt; - unsigned int nr_added_bufs; - int err; - - port = kmalloc(sizeof(*port), GFP_KERNEL); - if (!port) { - err = -ENOMEM; - goto fail; - } - - port->portdev = portdev; - port->id = id; - - port->name = NULL; - port->inbuf = NULL; - port->cons.hvc = NULL; - - port->host_connected = port->guest_connected = false; - - port->in_vq = portdev->in_vqs[port->id]; - port->out_vq = portdev->out_vqs[port->id]; - - cdev_init(&port->cdev, &port_fops); - - devt = MKDEV(portdev->chr_major, id); - err = cdev_add(&port->cdev, devt, 1); - if (err < 0) { - dev_err(&port->portdev->vdev->dev, - "Error %d adding cdev for port %u\n", err, id); - goto free_port; - } - port->dev = device_create(pdrvdata.class, &port->portdev->vdev->dev, - devt, port, "vport%up%u", - port->portdev->drv_index, id); - if (IS_ERR(port->dev)) { - err = PTR_ERR(port->dev); - dev_err(&port->portdev->vdev->dev, - "Error %d creating device for port %u\n", - err, id); - goto free_cdev; - } - - spin_lock_init(&port->inbuf_lock); - init_waitqueue_head(&port->waitqueue); - - /* Fill the in_vq with buffers so the host can send us data. */ - nr_added_bufs = fill_queue(port->in_vq, &port->inbuf_lock); - if (!nr_added_bufs) { - dev_err(port->dev, "Error allocating inbufs\n"); - err = -ENOMEM; - goto free_device; - } - - /* - * If we're not using multiport support, this has to be a console port - */ - if (!use_multiport(port->portdev)) { - err = init_port_console(port); - if (err) - goto free_inbufs; - } - - spin_lock_irq(&portdev->ports_lock); - list_add_tail(&port->list, &port->portdev->ports); - spin_unlock_irq(&portdev->ports_lock); - - /* - * Tell the Host we're set so that it can send us various - * configuration parameters for this port (eg, port name, - * caching, whether this is a console port, etc.) - */ - send_control_msg(port, VIRTIO_CONSOLE_PORT_READY, 1); - - if (pdrvdata.debugfs_dir) { - /* - * Finally, create the debugfs file that we can use to - * inspect a port's state at any time - */ - sprintf(debugfs_name, "vport%up%u", - port->portdev->drv_index, id); - port->debugfs_file = debugfs_create_file(debugfs_name, 0444, - pdrvdata.debugfs_dir, - port, - &port_debugfs_ops); - } - return 0; - -free_inbufs: - while ((buf = virtqueue_detach_unused_buf(port->in_vq))) - free_buf(buf); -free_device: - device_destroy(pdrvdata.class, port->dev->devt); -free_cdev: - cdev_del(&port->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; -} - static int init_vqs(struct ports_device *portdev) { vq_callback_t **io_callbacks; -- cgit v1.2.3-59-g8ed1b From f909f850d666e3dbac1ee7c9d5d83416bd02f84e Mon Sep 17 00:00:00 2001 From: Amit Shah Date: Wed, 19 May 2010 22:15:48 -0600 Subject: virtio: console: Use a control message to add ports Instead of the host and guest independently enumerating ports, switch to a control message to add ports where the host supplies the port number so there's no ambiguity or a possibility of a race between the host and the guest port numbers. We now no longer need the 'nr_ports' config value. Since no kernel has been released with the MULTIPORT changes yet, we have a chance to fiddle with the config space without adding compatibility features. This is beneficial for management software, which would now be able to instantiate ports at known locations and avoid problems that arise with implicit numbering in the host and the guest. This removes the 'guessing game' part of it, and management software can now actually indicate which id to spawn a particular port on. Signed-off-by: Amit Shah Signed-off-by: Rusty Russell --- drivers/char/virtio_console.c | 77 ++++++++++++++++++------------------------ include/linux/virtio_console.h | 17 +++++----- 2 files changed, 41 insertions(+), 53 deletions(-) (limited to 'drivers/char') diff --git a/drivers/char/virtio_console.c b/drivers/char/virtio_console.c index 237eee26fbc3..7671914be172 100644 --- a/drivers/char/virtio_console.c +++ b/drivers/char/virtio_console.c @@ -1048,7 +1048,7 @@ static void handle_control_message(struct ports_device *portdev, cpkt = (struct virtio_console_control *)(buf->buf + buf->offset); port = find_port_by_id(portdev, cpkt->id); - if (!port) { + if (!port && cpkt->event != VIRTIO_CONSOLE_PORT_ADD) { /* No valid header at start of buffer. Drop it. */ dev_dbg(&portdev->vdev->dev, "Invalid index %u in control packet\n", cpkt->id); @@ -1056,6 +1056,30 @@ static void handle_control_message(struct ports_device *portdev, } switch (cpkt->event) { + case VIRTIO_CONSOLE_PORT_ADD: + if (port) { + /* + * This can happen for port 0: we have to + * create a console port during probe() as was + * the behaviour before the MULTIPORT feature. + * On a newer host, when the host tells us + * that a port 0 is available, we should just + * say we have the port all set up. + */ + send_control_msg(port, VIRTIO_CONSOLE_PORT_READY, 1); + break; + } + if (cpkt->id >= portdev->config.max_nr_ports) { + dev_warn(&portdev->vdev->dev, + "Request for adding port with out-of-bound id %u, max. supported id: %u\n", + cpkt->id, portdev->config.max_nr_ports - 1); + break; + } + add_port(portdev, cpkt->id); + break; + case VIRTIO_CONSOLE_PORT_REMOVE: + remove_port(port); + break; case VIRTIO_CONSOLE_CONSOLE_PORT: if (!cpkt->value) break; @@ -1114,32 +1138,6 @@ static void handle_control_message(struct ports_device *portdev, kobject_uevent(&port->dev->kobj, KOBJ_CHANGE); } break; - case VIRTIO_CONSOLE_PORT_REMOVE: - /* - * Hot unplug the port. We don't decrement nr_ports - * since we don't want to deal with extra complexities - * of using the lowest-available port id: We can just - * pick up the nr_ports number as the id and not have - * userspace send it to us. This helps us in two - * ways: - * - * - We don't need to have a 'port_id' field in the - * config space when a port is hot-added. This is a - * good thing as we might queue up multiple hotplug - * requests issued in our workqueue. - * - * - Another way to deal with this would have been to - * use a bitmap of the active ports and select the - * lowest non-active port from that map. That - * bloats the already tight config space and we - * would end up artificially limiting the - * max. number of ports to sizeof(bitmap). Right - * now we can support 2^32 ports (as the port id is - * stored in a u32 type). - * - */ - remove_port(port); - break; } } @@ -1347,7 +1345,6 @@ static const struct file_operations portdev_fops = { static int __devinit virtcons_probe(struct virtio_device *vdev) { struct ports_device *portdev; - u32 i; int err; bool multiport; @@ -1376,29 +1373,15 @@ static int __devinit virtcons_probe(struct virtio_device *vdev) } multiport = false; - portdev->config.nr_ports = 1; portdev->config.max_nr_ports = 1; if (virtio_has_feature(vdev, VIRTIO_CONSOLE_F_MULTIPORT)) { multiport = true; vdev->features[0] |= 1 << VIRTIO_CONSOLE_F_MULTIPORT; - vdev->config->get(vdev, offsetof(struct virtio_console_config, - nr_ports), - &portdev->config.nr_ports, - sizeof(portdev->config.nr_ports)); vdev->config->get(vdev, offsetof(struct virtio_console_config, max_nr_ports), &portdev->config.max_nr_ports, sizeof(portdev->config.max_nr_ports)); - if (portdev->config.nr_ports > portdev->config.max_nr_ports) { - dev_warn(&vdev->dev, - "More ports (%u) specified than allowed (%u). Will init %u ports.", - portdev->config.nr_ports, - portdev->config.max_nr_ports, - portdev->config.max_nr_ports); - - portdev->config.nr_ports = portdev->config.max_nr_ports; - } } /* Let the Host know we support multiple ports.*/ @@ -1428,11 +1411,17 @@ static int __devinit virtcons_probe(struct virtio_device *vdev) } } - for (i = 0; i < portdev->config.nr_ports; i++) - add_port(portdev, i); + /* + * For backward compatibility: if we're running on an older + * host, we always want to create a console port. + */ + add_port(portdev, 0); /* Start using the new console output. */ early_put_chars = NULL; + + __send_control_msg(portdev, VIRTIO_CONSOLE_BAD_ID, + VIRTIO_CONSOLE_DEVICE_READY, 1); return 0; free_vqs: diff --git a/include/linux/virtio_console.h b/include/linux/virtio_console.h index 015736113c25..a85064db8f94 100644 --- a/include/linux/virtio_console.h +++ b/include/linux/virtio_console.h @@ -23,8 +23,6 @@ struct virtio_console_config { __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)); /* @@ -38,13 +36,14 @@ struct virtio_console_control { }; /* 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 -#define VIRTIO_CONSOLE_DEVICE_READY 6 +#define VIRTIO_CONSOLE_DEVICE_READY 0 +#define VIRTIO_CONSOLE_PORT_ADD 1 +#define VIRTIO_CONSOLE_PORT_REMOVE 2 +#define VIRTIO_CONSOLE_PORT_READY 3 +#define VIRTIO_CONSOLE_CONSOLE_PORT 4 +#define VIRTIO_CONSOLE_RESIZE 5 +#define VIRTIO_CONSOLE_PORT_OPEN 6 +#define VIRTIO_CONSOLE_PORT_NAME 7 #ifdef __KERNEL__ int __init virtio_cons_early_init(int (*put_chars)(u32, const char *, int)); -- cgit v1.2.3-59-g8ed1b From 1d05160be743c506b1d6926e7c637496fa750cd3 Mon Sep 17 00:00:00 2001 From: Amit Shah Date: Wed, 19 May 2010 22:15:49 -0600 Subject: virtio: console: Don't always create a port 0 if using multiport If we're using multiport, there's no point in always creating a console port. Create the console port only if the host doesn't support multiport. Signed-off-by: Amit Shah Signed-off-by: Rusty Russell --- drivers/char/virtio_console.c | 32 +++++++++++++++----------------- 1 file changed, 15 insertions(+), 17 deletions(-) (limited to 'drivers/char') diff --git a/drivers/char/virtio_console.c b/drivers/char/virtio_console.c index 7671914be172..6207e3729923 100644 --- a/drivers/char/virtio_console.c +++ b/drivers/char/virtio_console.c @@ -784,6 +784,13 @@ int init_port_console(struct port *port) spin_unlock_irq(&pdrvdata_lock); port->guest_connected = true; + /* + * Start using the new console output if this is the first + * console to come up. + */ + if (early_put_chars) + early_put_chars = NULL; + /* Notify host of port being opened */ send_control_msg(port, VIRTIO_CONSOLE_PORT_OPEN, 1); @@ -1058,14 +1065,8 @@ static void handle_control_message(struct ports_device *portdev, switch (cpkt->event) { case VIRTIO_CONSOLE_PORT_ADD: if (port) { - /* - * This can happen for port 0: we have to - * create a console port during probe() as was - * the behaviour before the MULTIPORT feature. - * On a newer host, when the host tells us - * that a port 0 is available, we should just - * say we have the port all set up. - */ + dev_dbg(&portdev->vdev->dev, + "Port %u already added\n", port->id); send_control_msg(port, VIRTIO_CONSOLE_PORT_READY, 1); break; } @@ -1409,17 +1410,14 @@ static int __devinit virtcons_probe(struct virtio_device *vdev) err = -ENOMEM; goto free_vqs; } + } else { + /* + * For backward compatibility: Create a console port + * if we're running on older host. + */ + add_port(portdev, 0); } - /* - * For backward compatibility: if we're running on an older - * host, we always want to create a console port. - */ - add_port(portdev, 0); - - /* Start using the new console output. */ - early_put_chars = NULL; - __send_control_msg(portdev, VIRTIO_CONSOLE_BAD_ID, VIRTIO_CONSOLE_DEVICE_READY, 1); return 0; -- cgit v1.2.3-59-g8ed1b From 60caacd3eeab68672961e88db01e26735527d521 Mon Sep 17 00:00:00 2001 From: Amit Shah Date: Wed, 19 May 2010 22:15:49 -0600 Subject: virtio: console: Rename wait_is_over() to will_read_block() We'll introduce a function that checks if write will block. Have function names that are similar for the two cases. Signed-off-by: Amit Shah Signed-off-by: Rusty Russell --- drivers/char/virtio_console.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) (limited to 'drivers/char') diff --git a/drivers/char/virtio_console.c b/drivers/char/virtio_console.c index 6207e3729923..a39bf191da02 100644 --- a/drivers/char/virtio_console.c +++ b/drivers/char/virtio_console.c @@ -479,9 +479,9 @@ static ssize_t fill_readbuf(struct port *port, char *out_buf, size_t out_count, } /* The condition that must be true for polling to end */ -static bool wait_is_over(struct port *port) +static bool will_read_block(struct port *port) { - return port_has_data(port) || !port->host_connected; + return !port_has_data(port) && port->host_connected; } static ssize_t port_fops_read(struct file *filp, char __user *ubuf, @@ -504,7 +504,7 @@ static ssize_t port_fops_read(struct file *filp, char __user *ubuf, return -EAGAIN; ret = wait_event_interruptible(port->waitqueue, - wait_is_over(port)); + !will_read_block(port)); if (ret < 0) return ret; } -- cgit v1.2.3-59-g8ed1b From cdfadfc1adb87fc7e8a631b1f299715feacbde90 Mon Sep 17 00:00:00 2001 From: Amit Shah Date: Wed, 19 May 2010 22:15:50 -0600 Subject: virtio: console: Add support for nonblocking write()s If the host port is not open, a write() should either just return if the file is opened in non-blocking mode, or block till the host port is opened. Also, don't spin till host consumes data for nonblocking ports. For non-blocking ports, we can do away with the spinning and reclaim the buffers consumed by the host on the next write call or on the condition that'll make poll return. Signed-off-by: Amit Shah Signed-off-by: Rusty Russell --- drivers/char/virtio_console.c | 119 +++++++++++++++++++++++++++++++++++++++--- 1 file changed, 111 insertions(+), 8 deletions(-) (limited to 'drivers/char') diff --git a/drivers/char/virtio_console.c b/drivers/char/virtio_console.c index a39bf191da02..4175a2ae972f 100644 --- a/drivers/char/virtio_console.c +++ b/drivers/char/virtio_console.c @@ -159,6 +159,9 @@ struct port { */ spinlock_t inbuf_lock; + /* Protect the operations on the out_vq. */ + spinlock_t outvq_lock; + /* The IO vqs for this port */ struct virtqueue *in_vq, *out_vq; @@ -184,6 +187,8 @@ struct port { /* The 'id' to identify the port with the Host */ u32 id; + bool outvq_full; + /* Is the host device open */ bool host_connected; @@ -405,15 +410,33 @@ static ssize_t send_control_msg(struct port *port, unsigned int event, 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) +/* Callers must take the port->outvq_lock */ +static void reclaim_consumed_buffers(struct port *port) +{ + void *buf; + unsigned int len; + + while ((buf = virtqueue_get_buf(port->out_vq, &len))) { + kfree(buf); + port->outvq_full = false; + } +} + +static ssize_t send_buf(struct port *port, void *in_buf, size_t in_count, + bool nonblock) { struct scatterlist sg[1]; struct virtqueue *out_vq; ssize_t ret; + unsigned long flags; unsigned int len; out_vq = port->out_vq; + spin_lock_irqsave(&port->outvq_lock, flags); + + reclaim_consumed_buffers(port); + sg_init_one(sg, in_buf, in_count); ret = virtqueue_add_buf(out_vq, sg, 1, 0, in_buf); @@ -422,14 +445,29 @@ static ssize_t send_buf(struct port *port, void *in_buf, size_t in_count) if (ret < 0) { in_count = 0; - goto fail; + goto done; } - /* Wait till the host acknowledges it pushed out the data we sent. */ + if (ret == 0) + port->outvq_full = true; + + if (nonblock) + goto done; + + /* + * Wait till the host acknowledges it pushed out the data we + * sent. This is done for ports in blocking mode or for data + * from the hvc_console; the tty operations are performed with + * spinlocks held so we can't sleep here. + */ while (!virtqueue_get_buf(out_vq, &len)) cpu_relax(); -fail: - /* We're expected to return the amount of data we wrote */ +done: + spin_unlock_irqrestore(&port->outvq_lock, flags); + /* + * We're expected to return the amount of data we wrote -- all + * of it + */ return in_count; } @@ -484,6 +522,25 @@ static bool will_read_block(struct port *port) return !port_has_data(port) && port->host_connected; } +static bool will_write_block(struct port *port) +{ + bool ret; + + if (!port->host_connected) + return true; + + spin_lock_irq(&port->outvq_lock); + /* + * Check if the Host has consumed any buffers since we last + * sent data (this is only applicable for nonblocking ports). + */ + reclaim_consumed_buffers(port); + ret = port->outvq_full; + spin_unlock_irq(&port->outvq_lock); + + return ret; +} + static ssize_t port_fops_read(struct file *filp, char __user *ubuf, size_t count, loff_t *offp) { @@ -530,9 +587,22 @@ static ssize_t port_fops_write(struct file *filp, const char __user *ubuf, struct port *port; char *buf; ssize_t ret; + bool nonblock; port = filp->private_data; + nonblock = filp->f_flags & O_NONBLOCK; + + if (will_write_block(port)) { + if (nonblock) + return -EAGAIN; + + ret = wait_event_interruptible(port->waitqueue, + !will_write_block(port)); + if (ret < 0) + return ret; + } + count = min((size_t)(32 * 1024), count); buf = kmalloc(count, GFP_KERNEL); @@ -545,9 +615,14 @@ static ssize_t port_fops_write(struct file *filp, const char __user *ubuf, goto free_buf; } - ret = send_buf(port, buf, count); + ret = send_buf(port, buf, count, nonblock); + + if (nonblock && ret > 0) + goto out; + free_buf: kfree(buf); +out: return ret; } @@ -562,7 +637,7 @@ static unsigned int port_fops_poll(struct file *filp, poll_table *wait) ret = 0; if (port->inbuf) ret |= POLLIN | POLLRDNORM; - if (port->host_connected) + if (!will_write_block(port)) ret |= POLLOUT; if (!port->host_connected) ret |= POLLHUP; @@ -586,6 +661,10 @@ static int port_fops_release(struct inode *inode, struct file *filp) spin_unlock_irq(&port->inbuf_lock); + spin_lock_irq(&port->outvq_lock); + reclaim_consumed_buffers(port); + spin_unlock_irq(&port->outvq_lock); + return 0; } @@ -614,6 +693,15 @@ static int port_fops_open(struct inode *inode, struct file *filp) port->guest_connected = true; spin_unlock_irq(&port->inbuf_lock); + spin_lock_irq(&port->outvq_lock); + /* + * There might be a chance that we missed reclaiming a few + * buffers in the window of the port getting previously closed + * and opening now. + */ + reclaim_consumed_buffers(port); + spin_unlock_irq(&port->outvq_lock); + /* Notify host of port being opened */ send_control_msg(filp->private_data, VIRTIO_CONSOLE_PORT_OPEN, 1); @@ -654,7 +742,7 @@ static int put_chars(u32 vtermno, const char *buf, int count) if (!port) return -EPIPE; - return send_buf(port, (void *)buf, count); + return send_buf(port, (void *)buf, count, false); } /* @@ -845,6 +933,8 @@ static ssize_t debugfs_read(struct file *filp, char __user *ubuf, "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, + "outvq_full: %d\n", port->outvq_full); out_offset += snprintf(buf + out_offset, out_count - out_offset, "is_console: %s\n", is_console_port(port) ? "yes" : "no"); @@ -912,6 +1002,8 @@ static int add_port(struct ports_device *portdev, u32 id) port->host_connected = port->guest_connected = false; + port->outvq_full = false; + port->in_vq = portdev->in_vqs[port->id]; port->out_vq = portdev->out_vqs[port->id]; @@ -936,6 +1028,7 @@ static int add_port(struct ports_device *portdev, u32 id) } spin_lock_init(&port->inbuf_lock); + spin_lock_init(&port->outvq_lock); init_waitqueue_head(&port->waitqueue); /* Fill the in_vq with buffers so the host can send us data. */ @@ -1031,6 +1124,8 @@ static int remove_port(struct port *port) /* Remove unused data this port might have received. */ discard_port_data(port); + reclaim_consumed_buffers(port); + /* Remove buffers we queued up for the Host to send us data in. */ while ((buf = virtqueue_detach_unused_buf(port->in_vq))) free_buf(buf); @@ -1102,6 +1197,14 @@ static void handle_control_message(struct ports_device *portdev, case VIRTIO_CONSOLE_PORT_OPEN: port->host_connected = cpkt->value; wake_up_interruptible(&port->waitqueue); + /* + * If the host port got closed and the host had any + * unconsumed buffers, we'll be able to reclaim them + * now. + */ + spin_lock_irq(&port->outvq_lock); + reclaim_consumed_buffers(port); + spin_unlock_irq(&port->outvq_lock); break; case VIRTIO_CONSOLE_PORT_NAME: /* -- cgit v1.2.3-59-g8ed1b From 4038f5b767a610c5a5d92d7047755c663ead1568 Mon Sep 17 00:00:00 2001 From: Amit Shah Date: Thu, 6 May 2010 02:05:07 +0530 Subject: virtio: console: Resize console port 0 on config intr only if multiport is off When using multiport, we'll use control messages. Ensure we don't accidentally update port 0 size on config interrupts. Signed-off-by: Amit Shah CC: Christian Borntraeger CC: linuxppc-dev@ozlabs.org CC: Kusanagi Kouichi Signed-off-by: Rusty Russell --- drivers/char/virtio_console.c | 17 ++++++++++------- 1 file changed, 10 insertions(+), 7 deletions(-) (limited to 'drivers/char') diff --git a/drivers/char/virtio_console.c b/drivers/char/virtio_console.c index 4175a2ae972f..1e3f4674da4a 100644 --- a/drivers/char/virtio_console.c +++ b/drivers/char/virtio_console.c @@ -1319,13 +1319,16 @@ static void config_intr(struct virtio_device *vdev) portdev = vdev->priv; - /* - * 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 - */ - resize_console(find_port_by_id(portdev, 0)); + if (!use_multiport(portdev)) { + /* + * 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. + */ + resize_console(find_port_by_id(portdev, 0)); + } } static int init_vqs(struct ports_device *portdev) -- cgit v1.2.3-59-g8ed1b From 9778829cffd4d8d68c7e457645f958a82d4c4d8b Mon Sep 17 00:00:00 2001 From: Amit Shah Date: Thu, 6 May 2010 02:05:08 +0530 Subject: virtio: console: Store each console's size in the console structure With support for multiple consoles, just using one {rows,cols} pair in the config space is not going to suffice. Store each console's size as part of the console struct. This changes the behaviour for one case when multiport is not enabled: when notifier_add_vio() is called, the console size is taken from that of the last config-space update instead of fetching it afresh from the config space. Also add a helper to update the size in the console struct as we'll need to use the same code to update the size via control messages when multiport support is enabled. Signed-off-by: Amit Shah CC: Christian Borntraeger CC: linuxppc-dev@ozlabs.org CC: Kusanagi Kouichi Signed-off-by: Rusty Russell --- drivers/char/virtio_console.c | 43 +++++++++++++++++++++++++++++++------------ 1 file changed, 31 insertions(+), 12 deletions(-) (limited to 'drivers/char') diff --git a/drivers/char/virtio_console.c b/drivers/char/virtio_console.c index 1e3f4674da4a..f1fe11a344e9 100644 --- a/drivers/char/virtio_console.c +++ b/drivers/char/virtio_console.c @@ -78,6 +78,9 @@ struct console { /* The hvc device associated with this console port */ struct hvc_struct *hvc; + /* The size of the console */ + struct winsize ws; + /* * This number identifies the number that we used to register * with hvc in hvc_instantiate() and hvc_alloc(); this is the @@ -773,22 +776,14 @@ static int get_chars(u32 vtermno, char *buf, int count) static void resize_console(struct port *port) { struct virtio_device *vdev; - struct winsize ws; /* The port could have been hot-unplugged */ - if (!port) + if (!port || !is_console_port(port)) return; vdev = port->portdev->vdev; - if (virtio_has_feature(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->cons.hvc, ws); - } + if (virtio_has_feature(vdev, VIRTIO_CONSOLE_F_SIZE)) + hvc_resize(port->cons.hvc, port->cons.ws); } /* We set the configuration at this point, since we now have a tty */ @@ -952,6 +947,15 @@ static const struct file_operations port_debugfs_ops = { .read = debugfs_read, }; +static void set_console_size(struct port *port, u16 rows, u16 cols) +{ + if (!port || !is_console_port(port)) + return; + + port->cons.ws.ws_row = rows; + port->cons.ws.ws_col = cols; +} + static unsigned int fill_queue(struct virtqueue *vq, spinlock_t *lock) { struct port_buffer *buf; @@ -1000,6 +1004,8 @@ static int add_port(struct ports_device *portdev, u32 id) port->inbuf = NULL; port->cons.hvc = NULL; + port->cons.ws.ws_row = port->cons.ws.ws_col = 0; + port->host_connected = port->guest_connected = false; port->outvq_full = false; @@ -1320,6 +1326,19 @@ static void config_intr(struct virtio_device *vdev) portdev = vdev->priv; if (!use_multiport(portdev)) { + struct port *port; + u16 rows, cols; + + vdev->config->get(vdev, + offsetof(struct virtio_console_config, cols), + &cols, sizeof(u16)); + vdev->config->get(vdev, + offsetof(struct virtio_console_config, rows), + &rows, sizeof(u16)); + + port = find_port_by_id(portdev, 0); + set_console_size(port, rows, cols); + /* * We'll use this way of resizing only for legacy * support. For newer userspace @@ -1327,7 +1346,7 @@ static void config_intr(struct virtio_device *vdev) * to indicate console size changes so that it can be * done per-port. */ - resize_console(find_port_by_id(portdev, 0)); + resize_console(port); } } -- cgit v1.2.3-59-g8ed1b From 8345adbf96fc1bde7d9846aadbe5af9b2ae90882 Mon Sep 17 00:00:00 2001 From: Amit Shah Date: Thu, 6 May 2010 02:05:09 +0530 Subject: virtio: console: Accept console size along with resize control message The VIRTIO_CONSOLE_RESIZE control message sent to us by the host now contains the new {rows, cols} values for the console. This ensures each console port gets its own size, and we don't depend on the config-space rows and cols values at all now. Signed-off-by: Amit Shah CC: Christian Borntraeger CC: linuxppc-dev@ozlabs.org CC: Kusanagi Kouichi Signed-off-by: Rusty Russell --- drivers/char/virtio_console.c | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) (limited to 'drivers/char') diff --git a/drivers/char/virtio_console.c b/drivers/char/virtio_console.c index f1fe11a344e9..458d907e3621 100644 --- a/drivers/char/virtio_console.c +++ b/drivers/char/virtio_console.c @@ -1194,12 +1194,23 @@ static void handle_control_message(struct ports_device *portdev, * have to notify the host first. */ break; - case VIRTIO_CONSOLE_RESIZE: + case VIRTIO_CONSOLE_RESIZE: { + struct { + __u16 rows; + __u16 cols; + } size; + if (!is_console_port(port)) break; + + memcpy(&size, buf->buf + buf->offset + sizeof(*cpkt), + sizeof(size)); + set_console_size(port, size.rows, size.cols); + port->cons.hvc->irq_requested = 1; resize_console(port); break; + } case VIRTIO_CONSOLE_PORT_OPEN: port->host_connected = cpkt->value; wake_up_interruptible(&port->waitqueue); -- cgit v1.2.3-59-g8ed1b From 0643e4c6e4fd67778fa886a89e6ec2320e0ff4d3 Mon Sep 17 00:00:00 2001 From: Julia Lawall Date: Sat, 15 May 2010 11:45:53 +0200 Subject: drivers/char: Eliminate use after free In each case, the first argument to send_control_msg or __send_control_msg, respectively, has either not been successfully allocated or has been freed at the point of the call. In the first case, the first argument, port, is only used to access the portdev and id fields, in order to call __send_control_msg. Thus it seems possible instead to call __send_control_msg directly. In the second case, the call to __send_control_msg is moved up to a place where it seems like the first argument, portdev, has been initialized sufficiently to make the call to __send_control_msg meaningful. This has only been compile tested. A simplified version of the semantic match that finds this problem is as follows: (http://coccinelle.lip6.fr/) // @free@ expression E; position p; @@ kfree@p(E) @@ expression free.E, subE<=free.E, E1; position free.p; @@ kfree@p(E) ... ( subE = E1 | * E ) // Signed-off-by: Julia Lawall Acked-by: Amit Shah Signed-off-by: Rusty Russell --- drivers/char/virtio_console.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) (limited to 'drivers/char') diff --git a/drivers/char/virtio_console.c b/drivers/char/virtio_console.c index 458d907e3621..8c99bf1b5e9f 100644 --- a/drivers/char/virtio_console.c +++ b/drivers/char/virtio_console.c @@ -1090,7 +1090,7 @@ 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); + __send_control_msg(portdev, id, VIRTIO_CONSOLE_PORT_READY, 0); return err; } @@ -1559,6 +1559,9 @@ static int __devinit virtcons_probe(struct virtio_device *vdev) return 0; free_vqs: + /* 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); vdev->config->del_vqs(vdev); kfree(portdev->in_vqs); kfree(portdev->out_vqs); @@ -1567,9 +1570,6 @@ 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; } -- cgit v1.2.3-59-g8ed1b