Re: [PATCH RESEND] virtio_console: read size from config space during device init

[Filip Hejsek <filip.hejsek@gmail.com>]

On Wed, 2026-06-10 at 03:04 -0400, Michael S. Tsirkin wrote:
> On Mon, Feb 23, 2026 at 06:37:02PM +0100, Filip Hejsek wrote:
> > Previously, the size was only read upon receiving the config interrupt.
> > This interrupt is sent when the size changes. However, we also need to
> > read the initial size.
> > 
> > Also make sure to only read the size from config if F_SIZE is enabled.
> > 
> > Fixes: 9778829cffd4 ("virtio: console: Store each console's size in the console structure")
> > Signed-off-by: Filip Hejsek <filip.hejsek@gmail.com>
> > ---
> > This is a resend of [1], which hasn't received any response.
> > 
> > I found this bug while developing patches for QEMU that add virtio
> > console resize support. If you want to test this, you can get my QEMU
> > patches from [2]. You will need to disable multiport
> > using `-device virtio-serial,max_ports=1`.
> > 
> > [1]: https://lore.kernel.org/all/20251224-virtio-console-fix-v1-1-69d0349692dc@gmail.com/
> > [2]: https://lore.kernel.org/all/20250921-console-resize-v5-0-89e3c6727060@gmail.com/
> > 
> > I'll also repeat my questions from the previous submission here. These are
> > things that confused me when I was trying to understand the surrounding code,
> > but should in no way prevent merging this patch.
> > 
> >   - Why does use_multiport use __virtio_test_bit instead of
> >     virtio_has_feature?
> > 
> >   - The VIRTIO_CONSOLE_RESIZE handler sets irq_requested to 1, which I
> >     think makes no sense?
> > ---
> >  drivers/char/virtio_console.c | 52 ++++++++++++++++++++++++++-----------------
> >  1 file changed, 31 insertions(+), 21 deletions(-)
> > 
> > diff --git a/drivers/char/virtio_console.c b/drivers/char/virtio_console.c
> > index 088182e54d..c355f6d392 100644
> > --- a/drivers/char/virtio_console.c
> > +++ b/drivers/char/virtio_console.c
> > @@ -1771,32 +1771,40 @@ static void config_intr(struct virtio_device *vdev)
> >  		schedule_work(&portdev->config_work);
> >  }
> >  
> > -static void config_work_handler(struct work_struct *work)
> > +static void update_size_from_config(struct ports_device *portdev)
> >  {
> > -	struct ports_device *portdev;
> > +	struct virtio_device *vdev;
> > +	struct port *port;
> > +	u16 rows, cols;
> >  
> > -	portdev = container_of(work, struct ports_device, config_work);
> > -	if (!use_multiport(portdev)) {
> > -		struct virtio_device *vdev;
> > -		struct port *port;
> > -		u16 rows, cols;
> > +	vdev = portdev->vdev;
> >  
> > -		vdev = portdev->vdev;
> > -		virtio_cread(vdev, struct virtio_console_config, cols, &cols);
> > -		virtio_cread(vdev, struct virtio_console_config, rows, &rows);
> > +	/*
> > +	 * We'll use this way of resizing only for legacy support.
> > +	 * For multiport devices, use control messages to indicate
> > +	 * console size changes so that it can be done per-port.
> > +	 *
> > +	 * Don't test F_SIZE at all if we're rproc: not a valid feature.
> > +	 */
> > +	if (is_rproc_serial(vdev) ||
> 
> Wait a second. Why is there this rproc test here?
> Was not in the original code and commit log says nothing about it.
> 

Previously, this code was in config_work_handler(), which was never
called for rproc_serial (it's scheduled from config_intr(), which is
the config_changed handler only for virtio_console).

Now update_size_from_config() is called unconditionally from
virtcons_probe(), so it will be called for rproc_serial too, which
doesn't have the F_SIZE feature.

> 
> > +	    use_multiport(portdev) ||
> > +	    !virtio_has_feature(vdev, VIRTIO_CONSOLE_F_SIZE))
> > +		return;
> >  
> > -		port = find_port_by_id(portdev, 0);
> > -		set_console_size(port, rows, cols);
> > +	virtio_cread(vdev, struct virtio_console_config, cols, &cols);
> > +	virtio_cread(vdev, struct virtio_console_config, rows, &rows);
> >  
> > -		/*
> > -		 * We'll use this way of resizing only for legacy
> > -		 * support.  For newer userspace
> > -		 * (VIRTIO_CONSOLE_F_MULTPORT+), use control messages
> > -		 * to indicate console size changes so that it can be
> > -		 * done per-port.
> > -		 */
> > -		resize_console(port);
> > -	}
> > +	port = find_port_by_id(portdev, 0);
> > +	set_console_size(port, rows, cols);
> > +	resize_console(port);
> > +}
> > +
> > +static void config_work_handler(struct work_struct *work)
> > +{
> > +	struct ports_device *portdev;
> > +
> > +	portdev = container_of(work, struct ports_device, config_work);
> > +	update_size_from_config(portdev);
> >  }
> >  
> >  static int init_vqs(struct ports_device *portdev)
> > @@ -2054,6 +2062,8 @@ static int virtcons_probe(struct virtio_device *vdev)
> >  	__send_control_msg(portdev, VIRTIO_CONSOLE_BAD_ID,
> >  			   VIRTIO_CONSOLE_DEVICE_READY, 1);
> >  
> > +	update_size_from_config(portdev);
> > +
> >  	return 0;
> >  
> >  free_chrdev:
> > 
> > ---
> > base-commit: b927546677c876e26eba308550207c2ddf812a43
> > change-id: 20251224-virtio-console-fix-3d46980ef569
> > 
> > Best regards,
> > -- 
> > Filip Hejsek <filip.hejsek@gmail.com>