On Thu, Jun 11, 2026 at 08:57:57AM +0200, Filip Hejsek wrote:
> 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 <[email protected]>
> > > ---
> > > 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/[email protected]/
> > > [2]:
> > > https://lore.kernel.org/all/[email protected]/
> > >
> > > 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.
So why not test it? What does "not a valid feature" mean?
I dislike transport code leaking into devices.
> >
> > > + 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 <[email protected]>
