On Mon, 2025-03-03 at 12:54 +0100, Amit Shah wrote:
> On Tue, 2025-02-25 at 10:21 +0100, Maximilian Immanuel Brandtner
> wrote:
> > According to the virtio spec[0] the virtio console resize struct
> > defines
> > cols before rows. In the kernel implementation it is the other way
> > around
> > resulting in the two properties being switched.
>
> Not true, see below.
>
> > While QEMU doesn't currently support resizing consoles, TinyEMU
>
> QEMU does support console resizing - just that it uses the classical
> way of doing it: via the config space, and not via a control message
> (yet).
>
> https://web.git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/char/virtio_console.c#n1787
>
> https://lists.nongnu.org/archive/html/qemu-devel/2010-05/msg00031.html
>
> > diff --git a/drivers/char/virtio_console.c
> > b/drivers/char/virtio_console.c
> > index 24442485e73e..9668e89873cf 100644
> > --- a/drivers/char/virtio_console.c
> > +++ b/drivers/char/virtio_console.c
> > @@ -1579,8 +1579,8 @@ static void handle_control_message(struct
> > virtio_device *vdev,
> > break;
> > case VIRTIO_CONSOLE_RESIZE: {
> > struct {
> > - __u16 rows;
> > __u16 cols;
> > + __u16 rows;
> > } size;
> >
> > if (!is_console_port(port))
>
> This VIRTIO_CONSOLE_RESIZE message is a control message, as opposed
> to
> the config space row/col values that is documented in the spec.
>
> Maybe more context will be helpful:
>
> Initially, virtio_console was just a way to create one hvc console
> port
> over the virtio transport. The size of that console port could be
> changed by changing the size parameters in the virtio device's
> configuration space. Those are the values documented in the spec.
> These are read via virtio_cread(), and do not have a struct
> representation.
>
> When the MULTIPORT feature was added to the virtio_console.c driver,
> more than one console port could be associated with the single
> device.
> Eg. we could have hvc0, hvc1, hvc2 all as part of the same device.
> With this, the single config space value for row/col could not be
> used
> for the "extra" hvc1/hvc2 devices -- so a new VIRTIO_CONSOLE_RESIZE
> control message was added that conveys each console's dimensions.
>
> Your patch is trying to change the control message, and not the
> config
> space.
>
> Now - the lack of the 'struct size' definition for the control
> message
> in the spec is unfortunate, but that can be easily added -- and I
> prefer we add it based on this Linux implementation (ie. first rows,
> then cols).
Under section 5.3.6.2 multiport device operation for
VIRTIO_CONSOLE_RESIZE the spec says the following
```
Sent by the device to indicate a console size change. value is unused.
The buffer is followed by the number of columns and rows:
struct virtio_console_resize {
le16 cols;
le16 rows;
};
```
It would be extremely surprising to me if the section `multiport device
operation` does not document resize for multiport control messages, but
rather config messages, especially as VIRTIO_CONSOLE_RESIZE is
documented as a virtio_console_control event.
In fact as far as I can tell this is the only part of the spec that
documents resize. I would be legitimately interested in resizing
without multiport and I would genuinely like to find out about how it
could be used. In what section of the documentation could I find it?
>
> But note that all this only affects devices that implement multiport
> support, and have multiple console ports on a single device. I don't
> recall there are any implementations using such a configuration.
>
> ... which all leads me to ask if you've actually seen a
> misconfiguration happen when trying to resize consoles which led to
> this patch.
>
> Amit
