From 51c6d61ac58844b5e3e0d28271084c06f6a15371 Mon Sep 17 00:00:00 2001 From: Sasha Levin Date: Sun, 14 Aug 2011 17:52:31 +0300 Subject: virtio-console: Use virtio_config_val() for retrieving config This patch modifies virtio-console to use virtio_config_val() instead of a 'if(virtio_has_feature()) vdev->config->get()' construct to retrieve optional values from the config space. Cc: Amit Shah Cc: "Michael S. Tsirkin" Cc: Rusty Russell Cc: virtualization@lists.linux-foundation.org Signed-off-by: Sasha Levin Signed-off-by: Rusty Russell --- drivers/char/virtio_console.c | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) (limited to 'drivers/char') diff --git a/drivers/char/virtio_console.c b/drivers/char/virtio_console.c index fb68b1295373..ed99f3bf341a 100644 --- a/drivers/char/virtio_console.c +++ b/drivers/char/virtio_console.c @@ -1675,13 +1675,11 @@ static int __devinit virtcons_probe(struct virtio_device *vdev) multiport = false; portdev->config.max_nr_ports = 1; - if (virtio_has_feature(vdev, VIRTIO_CONSOLE_F_MULTIPORT)) { + if (virtio_config_val(vdev, VIRTIO_CONSOLE_F_MULTIPORT, + offsetof(struct virtio_console_config, + max_nr_ports), + &portdev->config.max_nr_ports) == 0) multiport = true; - vdev->config->get(vdev, offsetof(struct virtio_console_config, - max_nr_ports), - &portdev->config.max_nr_ports, - sizeof(portdev->config.max_nr_ports)); - } err = init_vqs(portdev); if (err < 0) { -- cgit From 286f9a226f11e4a05d08999417fd838c0ca91d7a Mon Sep 17 00:00:00 2001 From: Amit Shah Date: Wed, 14 Sep 2011 13:06:39 +0530 Subject: virtio: console: Fix indentation Convert spaces to tabs and fix indentation for an if statement split into multiple lines. Signed-off-by: Amit Shah Signed-off-by: Rusty Russell --- drivers/char/virtio_console.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'drivers/char') diff --git a/drivers/char/virtio_console.c b/drivers/char/virtio_console.c index ed99f3bf341a..10280a29a34b 100644 --- a/drivers/char/virtio_console.c +++ b/drivers/char/virtio_console.c @@ -1563,7 +1563,7 @@ static int init_vqs(struct ports_device *portdev) portdev->out_vqs = kmalloc(nr_ports * sizeof(struct virtqueue *), GFP_KERNEL); if (!vqs || !io_callbacks || !io_names || !portdev->in_vqs || - !portdev->out_vqs) { + !portdev->out_vqs) { err = -ENOMEM; goto free; } -- cgit From 291024ef351328e7b4ca6bae798abc816a43653c Mon Sep 17 00:00:00 2001 From: Amit Shah Date: Wed, 14 Sep 2011 13:06:40 +0530 Subject: virtio: console: Ignore port name update request if name already set We don't allow port name changes dynamically for a port. So any requests by the host to change the name are ignored. Before this patch, if the hypervisor sent a port name while we had one set already, we would leak memory equivalent to the size of the old name. This scenario wasn't expected so far, but with the suspend-resume support, we'll send the VIRTIO_CONSOLE_PORT_READY message after restore, which can get us into this situation. Signed-off-by: Amit Shah Signed-off-by: Rusty Russell --- drivers/char/virtio_console.c | 7 +++++++ 1 file changed, 7 insertions(+) (limited to 'drivers/char') diff --git a/drivers/char/virtio_console.c b/drivers/char/virtio_console.c index 10280a29a34b..5397884b6c02 100644 --- a/drivers/char/virtio_console.c +++ b/drivers/char/virtio_console.c @@ -1393,6 +1393,13 @@ static void handle_control_message(struct ports_device *portdev, send_sigio_to_port(port); break; case VIRTIO_CONSOLE_PORT_NAME: + /* + * If we woke up after hibernation, we can get this + * again. Skip it in that case. + */ + if (port->name) + break; + /* * Skip the size of the header and the cpkt to get the size * of the name that was sent -- cgit From a08fa92d16f2fa112e3400c6c513d23ae78b960a Mon Sep 17 00:00:00 2001 From: Amit Shah Date: Wed, 14 Sep 2011 13:06:41 +0530 Subject: virtio: console: Use wait_event_freezable instead of _interruptible Get ready to support suspend/resume by using the freezable calls so that blocking read/write syscalls are handled properly across suspend/resume. Signed-off-by: Amit Shah Signed-off-by: Rusty Russell --- drivers/char/virtio_console.c | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) (limited to 'drivers/char') diff --git a/drivers/char/virtio_console.c b/drivers/char/virtio_console.c index 5397884b6c02..8f49d0f034e0 100644 --- a/drivers/char/virtio_console.c +++ b/drivers/char/virtio_console.c @@ -21,6 +21,7 @@ #include #include #include +#include #include #include #include @@ -633,8 +634,8 @@ static ssize_t port_fops_read(struct file *filp, char __user *ubuf, if (filp->f_flags & O_NONBLOCK) return -EAGAIN; - ret = wait_event_interruptible(port->waitqueue, - !will_read_block(port)); + ret = wait_event_freezable(port->waitqueue, + !will_read_block(port)); if (ret < 0) return ret; } @@ -677,8 +678,8 @@ static ssize_t port_fops_write(struct file *filp, const char __user *ubuf, if (nonblock) return -EAGAIN; - ret = wait_event_interruptible(port->waitqueue, - !will_write_block(port)); + ret = wait_event_freezable(port->waitqueue, + !will_write_block(port)); if (ret < 0) return ret; } -- cgit From defde66996476295dc7b1b60ea318965f8c3ad86 Mon Sep 17 00:00:00 2001 From: Amit Shah Date: Wed, 14 Sep 2011 13:06:42 +0530 Subject: virtio: console: Fix return type for get_inbuf() get_inbuf() returns void *. There's no reason to return void pointers instead of the correct struct port_buffer *. Signed-off-by: Amit Shah Signed-off-by: Rusty Russell --- drivers/char/virtio_console.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'drivers/char') diff --git a/drivers/char/virtio_console.c b/drivers/char/virtio_console.c index 8f49d0f034e0..3516aeb7f06c 100644 --- a/drivers/char/virtio_console.c +++ b/drivers/char/virtio_console.c @@ -348,7 +348,7 @@ fail: } /* Callers should take appropriate locks */ -static void *get_inbuf(struct port *port) +static struct port_buffer *get_inbuf(struct port *port) { struct port_buffer *buf; struct virtqueue *vq; -- cgit From d25a9ddae93ca97aa03fdab1363baf0e0c35d960 Mon Sep 17 00:00:00 2001 From: Amit Shah Date: Wed, 14 Sep 2011 13:06:43 +0530 Subject: virtio: console: make get_inbuf() return port->inbuf if present Instead of pulling in a buffer from the vq each time it's called, get_inbuf() now checks if the current active buffer, in port->inbuf is valid. If it is, just returns a pointer to it. This ends up simplifying a lot of code calling get_inbuf() since the check for port->inbuf being valid was done by all the callers. Signed-off-by: Amit Shah Signed-off-by: Rusty Russell --- drivers/char/virtio_console.c | 22 ++++++++-------------- 1 file changed, 8 insertions(+), 14 deletions(-) (limited to 'drivers/char') diff --git a/drivers/char/virtio_console.c b/drivers/char/virtio_console.c index 3516aeb7f06c..cb5edf33bebf 100644 --- a/drivers/char/virtio_console.c +++ b/drivers/char/virtio_console.c @@ -351,11 +351,12 @@ fail: static struct port_buffer *get_inbuf(struct port *port) { struct port_buffer *buf; - struct virtqueue *vq; unsigned int len; - vq = port->in_vq; - buf = virtqueue_get_buf(vq, &len); + if (port->inbuf) + return port->inbuf; + + buf = virtqueue_get_buf(port->in_vq, &len); if (buf) { buf->len = len; buf->offset = 0; @@ -418,18 +419,12 @@ static bool port_has_data(struct port *port) unsigned long flags; bool ret; + ret = false; spin_lock_irqsave(&port->inbuf_lock, flags); - if (port->inbuf) { - ret = true; - goto out; - } port->inbuf = get_inbuf(port); - if (port->inbuf) { + if (port->inbuf) ret = true; - goto out; - } - ret = false; -out: + spin_unlock_irqrestore(&port->inbuf_lock, flags); return ret; } @@ -1489,8 +1484,7 @@ static void in_intr(struct virtqueue *vq) return; spin_lock_irqsave(&port->inbuf_lock, flags); - if (!port->inbuf) - port->inbuf = get_inbuf(port); + port->inbuf = get_inbuf(port); /* * Don't queue up data when port is closed. This condition -- cgit From ce072a0cee420782ed0a079ac17c7ca26056fb95 Mon Sep 17 00:00:00 2001 From: Amit Shah Date: Wed, 14 Sep 2011 13:06:44 +0530 Subject: virtio: console: rename variable 'ret' is a misnomer in discard_port_data() since we don't return the value. Rename it to 'err'. Signed-off-by: Rusty Russell --- drivers/char/virtio_console.c | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) (limited to 'drivers/char') diff --git a/drivers/char/virtio_console.c b/drivers/char/virtio_console.c index cb5edf33bebf..0538425e9a71 100644 --- a/drivers/char/virtio_console.c +++ b/drivers/char/virtio_console.c @@ -387,8 +387,7 @@ static void discard_port_data(struct port *port) { struct port_buffer *buf; struct virtqueue *vq; - unsigned int len; - int ret; + unsigned int len, err; if (!port->portdev) { /* Device has been unplugged. vqs are already gone. */ @@ -400,18 +399,18 @@ static void discard_port_data(struct port *port) else buf = virtqueue_get_buf(vq, &len); - ret = 0; + err = 0; while (buf) { if (add_inbuf(vq, buf) < 0) { - ret++; + err++; free_buf(buf); } buf = virtqueue_get_buf(vq, &len); } port->inbuf = NULL; - if (ret) + if (err) dev_warn(port->dev, "Errors adding %d buffers back to vq\n", - ret); + err); } static bool port_has_data(struct port *port) -- cgit From 2d24cdaa6e389f85dad51eda39f1c2684a4f15b0 Mon Sep 17 00:00:00 2001 From: Amit Shah Date: Wed, 14 Sep 2011 13:06:45 +0530 Subject: virtio: console: make discard_port_data() use get_inbuf() discard_port_data() used virtqueue_get_buf() directly instead of using get_inbuf(). Fix this, so that we get accounting for all received bytes. This also simplifies the code a lot. Signed-off-by: Amit Shah Signed-off-by: Rusty Russell --- drivers/char/virtio_console.c | 15 +++++---------- 1 file changed, 5 insertions(+), 10 deletions(-) (limited to 'drivers/char') diff --git a/drivers/char/virtio_console.c b/drivers/char/virtio_console.c index 0538425e9a71..105181c1e6be 100644 --- a/drivers/char/virtio_console.c +++ b/drivers/char/virtio_console.c @@ -386,28 +386,23 @@ static int add_inbuf(struct virtqueue *vq, struct port_buffer *buf) static void discard_port_data(struct port *port) { struct port_buffer *buf; - struct virtqueue *vq; - unsigned int len, err; + unsigned int err; if (!port->portdev) { /* Device has been unplugged. vqs are already gone. */ return; } - vq = port->in_vq; - if (port->inbuf) - buf = port->inbuf; - else - buf = virtqueue_get_buf(vq, &len); + buf = get_inbuf(port); err = 0; while (buf) { - if (add_inbuf(vq, buf) < 0) { + if (add_inbuf(port->in_vq, buf) < 0) { err++; free_buf(buf); } - buf = virtqueue_get_buf(vq, &len); + port->inbuf = NULL; + buf = get_inbuf(port); } - port->inbuf = NULL; if (err) dev_warn(port->dev, "Errors adding %d buffers back to vq\n", err); -- cgit From 17e5b4f20adbe286fdf14b4d08f296564e97e545 Mon Sep 17 00:00:00 2001 From: Amit Shah Date: Wed, 14 Sep 2011 13:06:46 +0530 Subject: virtio: console: add port stats for bytes received, sent and discarded This commit adds port-specific stats for the number of bytes received, sent and discarded. They're exposed via the debugfs interface. This data can be used to check for data loss bugs (or disprove such claims). It can also be used for accounting, if there's such a need. The stats remain valid throughout the lifetime of the port. Unplugging a port will reset the stats. The numbers are not reset across port opens/closes. Signed-off-by: Amit Shah Signed-off-by: Rusty Russell --- drivers/char/virtio_console.c | 24 ++++++++++++++++++++++++ 1 file changed, 24 insertions(+) (limited to 'drivers/char') diff --git a/drivers/char/virtio_console.c b/drivers/char/virtio_console.c index 105181c1e6be..387fcdf019b7 100644 --- a/drivers/char/virtio_console.c +++ b/drivers/char/virtio_console.c @@ -152,6 +152,10 @@ struct ports_device { int chr_major; }; +struct port_stats { + unsigned long bytes_sent, bytes_received, bytes_discarded; +}; + /* This struct holds the per-port data */ struct port { /* Next port in the list, head is in the ports_device */ @@ -179,6 +183,13 @@ struct port { /* File in the debugfs directory that exposes this port's information */ struct dentry *debugfs_file; + /* + * Keep count of the bytes sent, received and discarded for + * this port for accounting and debugging purposes. These + * counts are not reset across port open / close events. + */ + struct port_stats stats; + /* * The entries in this struct will be valid if this port is * hooked up to an hvc console @@ -360,6 +371,7 @@ static struct port_buffer *get_inbuf(struct port *port) if (buf) { buf->len = len; buf->offset = 0; + port->stats.bytes_received += len; } return buf; } @@ -396,6 +408,7 @@ static void discard_port_data(struct port *port) err = 0; while (buf) { + port->stats.bytes_discarded += buf->len - buf->offset; if (add_inbuf(port->in_vq, buf) < 0) { err++; free_buf(buf); @@ -519,6 +532,8 @@ static ssize_t send_buf(struct port *port, void *in_buf, size_t in_count, cpu_relax(); done: spin_unlock_irqrestore(&port->outvq_lock, flags); + + port->stats.bytes_sent += in_count; /* * We're expected to return the amount of data we wrote -- all * of it @@ -1048,6 +1063,14 @@ static ssize_t debugfs_read(struct file *filp, char __user *ubuf, "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, + "bytes_sent: %lu\n", port->stats.bytes_sent); + out_offset += snprintf(buf + out_offset, out_count - out_offset, + "bytes_received: %lu\n", + port->stats.bytes_received); + out_offset += snprintf(buf + out_offset, out_count - out_offset, + "bytes_discarded: %lu\n", + port->stats.bytes_discarded); out_offset += snprintf(buf + out_offset, out_count - out_offset, "is_console: %s\n", is_console_port(port) ? "yes" : "no"); @@ -1133,6 +1156,7 @@ static int add_port(struct ports_device *portdev, u32 id) port->cons.ws.ws_row = port->cons.ws.ws_col = 0; port->host_connected = port->guest_connected = false; + port->stats = (struct port_stats) { 0 }; port->outvq_full = false; -- cgit From 5e38483b350405542c8080134408fd8897394ba2 Mon Sep 17 00:00:00 2001 From: Christian Borntraeger Date: Thu, 22 Sep 2011 23:44:23 +0530 Subject: virtio: console: wait for first console port for early console output On s390 I have seen some random "Warning: unable to open an initial console" boot failure. Turns out that tty_open fails, because the hvc_alloc was not yet done. In former times this could not happen, since the probe function automatically called hvc_alloc. With newer versions (multiport) some host<->guest interaction is required before hvc_alloc is called. This might be too late, especially if an initramfs is involved. Lets use a completion if we have multiport and an early console. [Amit: * Use NULL instead of 0 for pointer comparison * Rename 'port_added' to 'early_console_added' * Re-format, re-word commit message * Rebase patch on top of current queue] Signed-off-by: Christian Borntraeger Signed-off-by: Amit Shah Acked-by: Chrstian Borntraeger Signed-off-by: Rusty Russell --- drivers/char/virtio_console.c | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) (limited to 'drivers/char') diff --git a/drivers/char/virtio_console.c b/drivers/char/virtio_console.c index 387fcdf019b7..4ca181f1378b 100644 --- a/drivers/char/virtio_console.c +++ b/drivers/char/virtio_console.c @@ -19,6 +19,7 @@ */ #include #include +#include #include #include #include @@ -74,6 +75,7 @@ struct ports_driver_data { static struct ports_driver_data pdrvdata; DEFINE_SPINLOCK(pdrvdata_lock); +DECLARE_COMPLETION(early_console_added); /* This struct holds information that's relevant only for console ports */ struct console { @@ -1366,6 +1368,7 @@ static void handle_control_message(struct ports_device *portdev, break; init_port_console(port); + complete(&early_console_added); /* * Could remove the port here in case init fails - but * have to notify the host first. @@ -1668,6 +1671,10 @@ static int __devinit virtcons_probe(struct virtio_device *vdev) struct ports_device *portdev; int err; bool multiport; + bool early = early_put_chars != NULL; + + /* Ensure to read early_put_chars now */ + barrier(); portdev = kmalloc(sizeof(*portdev), GFP_KERNEL); if (!portdev) { @@ -1737,6 +1744,19 @@ static int __devinit virtcons_probe(struct virtio_device *vdev) __send_control_msg(portdev, VIRTIO_CONSOLE_BAD_ID, VIRTIO_CONSOLE_DEVICE_READY, 1); + + /* + * If there was an early virtio console, assume that there are no + * other consoles. We need to wait until the hvc_alloc matches the + * hvc_instantiate, otherwise tty_open will complain, resulting in + * a "Warning: unable to open an initial console" boot failure. + * Without multiport this is done in add_port above. With multiport + * this might take some host<->guest communication - thus we have to + * wait. + */ + if (multiport && early) + wait_for_completion(&early_console_added); + return 0; free_vqs: -- cgit