lkml.org 
[lkml]   [2022]   [Nov]   [22]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
SubjectRe: [PATCH v3] virtio_console: Introduce an ID allocator for virtual console numbers
From
On 22/11/2022 14.46, Cédric Le Goater wrote:
> 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);

Reviewed-by: Thomas Huth <thuth@redhat.com>

\
 
 \ /
  Last update: 2022-11-22 16:17    [W:0.080 / U:0.388 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site