Messages in this thread | | | Date | Tue, 22 Nov 2022 12:12:42 +0100 | From | Thomas Huth <> | Subject | Re: [PATCH v2] virtio_console: Introduce an ID allocator for virtual console numbers |
| |
On 14/11/2022 18.38, 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.
Sounds like a good idea!
> @@ -1244,8 +1236,11 @@ 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_range(&vtermno_ida, 1, ~0, GFP_KERNEL);
Just cosmetics: I think you could use ida_alloc_min() instead.
> + 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);
What if this if (IS_ERR()) error happens? The code seems to return early in this case - shouldn't the ID be freed again in this case?
Thomas
| |