linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Miche Baker-Harvey <miche@google.com>
To: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
Cc: Greg Kroah-Hartman <gregkh@suse.de>,
	linux-kernel@vger.kernel.org, xen-devel@lists.xensource.com,
	Amit Shah <amit.shah@redhat.com>,
	Anton Blanchard <anton@samba.org>,
	Benjamin Herrenschmidt <benh@kernel.crashing.org>,
	virtualization@lists.linux-foundation.org,
	Miche Baker-Harvey <miche@google.com>
Subject: [PATCH] VirtioConsole support.
Date: Thu, 27 Oct 2011 11:39:53 -0700	[thread overview]
Message-ID: <1319740793-2187-1-git-send-email-miche@google.com> (raw)
In-Reply-To: <20111027173527.GA23839@phenom.dumpdata.com>

Multiple HVC console terminals enabled.

Serialize device and port open and initialization.  Added a mutex
which gates the handling of control messages in virtio_console.c.
This includes adding and removing ports, and making opened ports be
consoles.  Extended the use of the prvdata spinlock to cover some missed
modifications to prvdata.next_vtermno.

I also added a mutex in hvc_console::hvc_alloc() to coordinate waiting
for the driver to be ready, and for the one-time call to hvc_init().  It
had been the case that this was sometimes being called mulitple times, and
partially setup state was being used by the second caller of hvc_alloc().

Make separate struct console* for each new port.  There was a single static
struct console* hvc_console, to be used for early printk.  We aren't doing
early printk, but more importantly, there is no code to multiplex on that
one console.  Multiple virtio_console ports were "sharing" this, which was
disasterous since both the index and the flags for the console are stored
there. The console struct is remembered in the hvc_struct, and it is
deallocated when the hvc_struct is deallocated.

------------------

Konrad, thanks for trying this out on Xen.
This is working in my KVM environment, letting me start multiple
virtio_consoles with getty's on them, but I'm really not sure how
all the console pieces fit together yet.  Feedback is welcome.

Signed-off-by: Miche Baker-Harvey <miche@google.com>
---
 drivers/char/virtio_console.c |   22 +++++++++++++++++++---
 drivers/tty/hvc/hvc_console.c |   39 +++++++++++++++++++++++++++++++++------
 drivers/tty/hvc/hvc_console.h |    1 +
 3 files changed, 53 insertions(+), 9 deletions(-)

diff --git a/drivers/char/virtio_console.c b/drivers/char/virtio_console.c
index fb68b12..e819d46 100644
--- a/drivers/char/virtio_console.c
+++ b/drivers/char/virtio_console.c
@@ -24,6 +24,7 @@
 #include <linux/fs.h>
 #include <linux/init.h>
 #include <linux/list.h>
+#include <linux/mutex.h>
 #include <linux/poll.h>
 #include <linux/sched.h>
 #include <linux/slab.h>
@@ -95,6 +96,11 @@ struct console {
 	u32 vtermno;
 };
 
+/* serialize the handling of control messages, which includes
+ * the initialization of the virtio_consoles.
+ */
+static DEFINE_MUTEX(virtio_console_mutex);
+
 struct port_buffer {
 	char *buf;
 
@@ -979,8 +985,14 @@ int init_port_console(struct port *port)
 	 * pointers.  The final argument is the output buffer size: we
 	 * can do any size, so we put PAGE_SIZE here.
 	 */
-	port->cons.vtermno = pdrvdata.next_vtermno;
+	spin_lock_irq(&pdrvdata_lock);
+	port->cons.vtermno = pdrvdata.next_vtermno++;
+	spin_unlock_irq(&pdrvdata_lock);
 
+	/*
+	 * xxx Use index 0 for now assuming there is no early HVC, since
+	 * we don't support it.
+	 */
 	port->cons.hvc = hvc_alloc(port->cons.vtermno, 0, &hv_ops, PAGE_SIZE);
 	if (IS_ERR(port->cons.hvc)) {
 		ret = PTR_ERR(port->cons.hvc);
@@ -990,7 +1002,6 @@ int init_port_console(struct port *port)
 		return ret;
 	}
 	spin_lock_irq(&pdrvdata_lock);
-	pdrvdata.next_vtermno++;
 	list_add_tail(&port->cons.list, &pdrvdata.consoles);
 	spin_unlock_irq(&pdrvdata_lock);
 	port->guest_connected = true;
@@ -1317,7 +1328,6 @@ static void handle_control_message(struct ports_device *portdev,
 	int err;
 
 	cpkt = (struct virtio_console_control *)(buf->buf + buf->offset);
-
 	port = find_port_by_id(portdev, cpkt->id);
 	if (!port && cpkt->event != VIRTIO_CONSOLE_PORT_ADD) {
 		/* No valid header at start of buffer.  Drop it. */
@@ -1326,6 +1336,11 @@ static void handle_control_message(struct ports_device *portdev,
 		return;
 	}
 
+	/*
+	 * These are rare initialization-time events that should be
+	 * serialized.
+	 */
+	mutex_lock(&virtio_console_mutex);
 	switch (cpkt->event) {
 	case VIRTIO_CONSOLE_PORT_ADD:
 		if (port) {
@@ -1429,6 +1444,7 @@ static void handle_control_message(struct ports_device *portdev,
 		}
 		break;
 	}
+	mutex_unlock(&virtio_console_mutex);
 }
 
 static void control_work_handler(struct work_struct *work)
diff --git a/drivers/tty/hvc/hvc_console.c b/drivers/tty/hvc/hvc_console.c
index 7430bc3..03ff6ed 100644
--- a/drivers/tty/hvc/hvc_console.c
+++ b/drivers/tty/hvc/hvc_console.c
@@ -29,8 +29,9 @@
 #include <linux/kernel.h>
 #include <linux/kthread.h>
 #include <linux/list.h>
-#include <linux/module.h>
 #include <linux/major.h>
+#include <linux/module.h>
+#include <linux/mutex.h>
 #include <linux/sysrq.h>
 #include <linux/tty.h>
 #include <linux/tty_flip.h>
@@ -84,6 +85,10 @@ static LIST_HEAD(hvc_structs);
  * list traversal.
  */
 static DEFINE_SPINLOCK(hvc_structs_lock);
+/*
+ * only one task does allocation at a time.
+ */
+static DEFINE_MUTEX(hvc_ports_mutex);
 
 /*
  * This value is used to assign a tty->index value to a hvc_struct based
@@ -242,6 +247,7 @@ static void destroy_hvc_struct(struct kref *kref)
 
 	spin_unlock(&hvc_structs_lock);
 
+	kfree(hp->hvc_console);
 	kfree(hp);
 }
 
@@ -822,19 +828,25 @@ struct hvc_struct *hvc_alloc(uint32_t vtermno, int data,
 			     int outbuf_size)
 {
 	struct hvc_struct *hp;
+	struct console *cp;
 	int i;
 
 	/* We wait until a driver actually comes along */
+	mutex_lock(&hvc_ports_mutex);
 	if (!hvc_driver) {
 		int err = hvc_init();
-		if (err)
+		if (err) {
+			mutex_unlock(&hvc_ports_mutex);
 			return ERR_PTR(err);
+		}
 	}
 
 	hp = kzalloc(ALIGN(sizeof(*hp), sizeof(long)) + outbuf_size,
 			GFP_KERNEL);
-	if (!hp)
+	if (!hp) {
+		mutex_unlock(&hvc_ports_mutex);
 		return ERR_PTR(-ENOMEM);
+	}
 
 	hp->vtermno = vtermno;
 	hp->data = data;
@@ -845,6 +857,19 @@ struct hvc_struct *hvc_alloc(uint32_t vtermno, int data,
 	kref_init(&hp->kref);
 
 	INIT_WORK(&hp->tty_resize, hvc_set_winsz);
+	/*
+	 * make each console its own struct console.
+	 * No need to do allocation and copy under lock.
+	 */
+	cp = kzalloc(sizeof(*cp), GFP_KERNEL);
+	if (!cp) {
+		kfree(hp);
+		mutex_unlock(&hvc_ports_mutex);
+		return ERR_PTR(-ENOMEM);
+	}
+	memcpy(cp, &hvc_console, sizeof(*cp));
+	hp->hvc_console = cp;
+
 	spin_lock_init(&hp->lock);
 	spin_lock(&hvc_structs_lock);
 
@@ -862,13 +887,14 @@ struct hvc_struct *hvc_alloc(uint32_t vtermno, int data,
 		i = ++last_hvc;
 
 	hp->index = i;
-	hvc_console.index = i;
+	cp->index = i;
 	vtermnos[i] = vtermno;
 	cons_ops[i] = ops;
 
 	list_add_tail(&(hp->next), &hvc_structs);
 	spin_unlock(&hvc_structs_lock);
-	register_console(&hvc_console);
+	register_console(cp);
+	mutex_unlock(&hvc_ports_mutex);
 
 	return hp;
 }
@@ -879,7 +905,8 @@ int hvc_remove(struct hvc_struct *hp)
 	unsigned long flags;
 	struct tty_struct *tty;
 
-	unregister_console(&hvc_console);
+	BUG_ON(!hp->hvc_console);
+	unregister_console(hp->hvc_console);
 	spin_lock_irqsave(&hp->lock, flags);
 	tty = tty_kref_get(hp->tty);
 
diff --git a/drivers/tty/hvc/hvc_console.h b/drivers/tty/hvc/hvc_console.h
index c335a14..2d20ab7 100644
--- a/drivers/tty/hvc/hvc_console.h
+++ b/drivers/tty/hvc/hvc_console.h
@@ -58,6 +58,7 @@ struct hvc_struct {
 	const struct hv_ops *ops;
 	int irq_requested;
 	int data;
+	struct console *hvc_console;
 	struct winsize ws;
 	struct work_struct tty_resize;
 	struct list_head next;
-- 
1.7.3.1


  reply	other threads:[~2011-10-27 18:40 UTC|newest]

Thread overview: 24+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2011-10-27  5:30 Regression: patch " hvc_console: display printk messages on console." causing infinite loop with 3.2-rc0 + Xen Konrad Rzeszutek Wilk
2011-10-27  5:34 ` Konrad Rzeszutek Wilk
2011-10-27  5:48 ` Greg KH
     [not found]   ` <CAB8Rdaq2r4Uc-WZK-k=wiEW8DKdixn+9dB7E4_XDL7k8jCMwSw@mail.gmail.com>
2011-10-27 14:50     ` Fwd: " Miche Baker-Harvey
2011-10-27 15:08     ` Konrad Rzeszutek Wilk
2011-10-27 15:31       ` Konrad Rzeszutek Wilk
2011-10-27 17:22         ` Miche Baker-Harvey
2011-10-27 17:35           ` Konrad Rzeszutek Wilk
2011-10-27 18:39             ` Miche Baker-Harvey [this message]
2011-10-27 21:15               ` [PATCH] VirtioConsole support Konrad Rzeszutek Wilk
2011-10-31  9:55               ` Amit Shah
2011-10-27 21:43             ` Miche Baker-Harvey
2011-10-27 21:55               ` Konrad Rzeszutek Wilk
2011-10-28 16:27                 ` Greg KH
2011-10-28  2:55               ` Joe Perches
2011-10-28 18:19                 ` Stephen Boyd
2011-11-02 22:02                   ` Rusty Russell
2011-11-03 12:38                     ` Christian Borntraeger
2011-11-03 14:15                       ` Konrad Rzeszutek Wilk
2011-11-02  1:13   ` Regression: patch " hvc_console: display printk messages on console." causing infinite loop with 3.2-rc0 + Xen Stephen Rothwell
2011-11-03  1:30     ` Greg KH
2011-11-07  6:19       ` Stephen Rothwell
2011-11-07 20:24         ` Greg KH
2011-10-31  7:48 ` Rusty Russell

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1319740793-2187-1-git-send-email-miche@google.com \
    --to=miche@google.com \
    --cc=amit.shah@redhat.com \
    --cc=anton@samba.org \
    --cc=benh@kernel.crashing.org \
    --cc=gregkh@suse.de \
    --cc=konrad.wilk@oracle.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=virtualization@lists.linux-foundation.org \
    --cc=xen-devel@lists.xensource.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).