Re: [PATCH] virtio: console: Make resizing compliant with virtio spec

[Amit Shah <amit@kernel.org>]

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).

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