mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: "Cédric Le Goater" <clg@kaod.org>
To: Amit Shah <amit@kernel.org>
Cc: "Arnd Bergmann" <arnd@arndb.de>,
	"Greg Kroah-Hartman" <gregkh@linuxfoundation.org>,
	"Michael S. Tsirkin" <mst@redhat.com>,
	virtualization@lists.linux-foundation.org,
	linux-kernel@vger.kernel.org, "Cédric Le Goater" <clg@kaod.org>
Subject: [PATCH v3] virtio_console: Introduce an ID allocator for virtual console numbers
Date: Tue, 22 Nov 2022 14:46:43 +0100	[thread overview]
Message-ID: <20221122134643.376184-1-clg@kaod.org> (raw)

When a virtio console port is initialized, it is registered as an hvc
console using a virtual console number. If a KVM guest is started with
multiple virtio console devices, the same vtermno (or virtual console
number) can be used to allocate different hvc consoles, which leads to
various communication problems later on.

This is also reported in debugfs :

  # grep vtermno /sys/kernel/debug/virtio-ports/*
  /sys/kernel/debug/virtio-ports/vport1p1:console_vtermno: 1
  /sys/kernel/debug/virtio-ports/vport2p1:console_vtermno: 1
  /sys/kernel/debug/virtio-ports/vport3p1:console_vtermno: 2
  /sys/kernel/debug/virtio-ports/vport4p1:console_vtermno: 3

Replace the next_vtermno global with an ID allocator and start the
allocation at 1 as it is today. Also recycle IDs when a console port
is removed.

Signed-off-by: Cédric Le Goater <clg@kaod.org>
---

 Changes in v3:
 - made use of ida_alloc_min()
 - free ID in case of error 
 
 Changes in v2:
 - introduced an ID allocator

 drivers/char/virtio_console.c | 26 +++++++++++---------------
 1 file changed, 11 insertions(+), 15 deletions(-)

diff --git a/drivers/char/virtio_console.c b/drivers/char/virtio_console.c
index 9fa3c76a267f..6a821118d553 100644
--- a/drivers/char/virtio_console.c
+++ b/drivers/char/virtio_console.c
@@ -13,6 +13,7 @@
 #include <linux/fs.h>
 #include <linux/splice.h>
 #include <linux/pagemap.h>
+#include <linux/idr.h>
 #include <linux/init.h>
 #include <linux/list.h>
 #include <linux/poll.h>
@@ -48,22 +49,11 @@ struct ports_driver_data {
 	/* List of all the devices we're handling */
 	struct list_head portdevs;
 
-	/*
-	 * This is used to keep track of the number of hvc consoles
-	 * spawned by this driver.  This number is given as the first
-	 * argument to hvc_alloc().  To correctly map an initial
-	 * console spawned via hvc_instantiate to the console being
-	 * hooked up via hvc_alloc, we need to pass the same vtermno.
-	 *
-	 * We also just assume the first console being initialised was
-	 * the first one that got used as the initial console.
-	 */
-	unsigned int next_vtermno;
-
 	/* All the console devices handled by this driver */
 	struct list_head consoles;
 };
-static struct ports_driver_data pdrvdata = { .next_vtermno = 1};
+
+static struct ports_driver_data pdrvdata;
 
 static DEFINE_SPINLOCK(pdrvdata_lock);
 static DECLARE_COMPLETION(early_console_added);
@@ -89,6 +79,8 @@ struct console {
 	u32 vtermno;
 };
 
+static DEFINE_IDA(vtermno_ida);
+
 struct port_buffer {
 	char *buf;
 
@@ -1244,18 +1236,21 @@ static 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;
+	ret = ida_alloc_min(&vtermno_ida, 1, GFP_KERNEL);
+	if (ret < 0)
+		return ret;
 
+	port->cons.vtermno = ret;
 	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);
 		dev_err(port->dev,
 			"error %d allocating hvc for port\n", ret);
 		port->cons.hvc = NULL;
+		ida_free(&vtermno_ida, port->cons.vtermno);
 		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;
@@ -1532,6 +1527,7 @@ static void unplug_port(struct port *port)
 		list_del(&port->cons.list);
 		spin_unlock_irq(&pdrvdata_lock);
 		hvc_remove(port->cons.hvc);
+		ida_free(&vtermno_ida, port->cons.vtermno);
 	}
 
 	remove_port_data(port);
-- 
2.38.1


             reply	other threads:[~2022-11-22 13:47 UTC|newest]

Thread overview: 2+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-11-22 13:46 Cédric Le Goater [this message]
2022-11-22 15:15 ` Thomas Huth

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=20221122134643.376184-1-clg@kaod.org \
    --to=clg@kaod.org \
    --cc=amit@kernel.org \
    --cc=arnd@arndb.de \
    --cc=gregkh@linuxfoundation.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mst@redhat.com \
    --cc=virtualization@lists.linux-foundation.org \
    /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

all inboxes | Powered by JetHome®