On Mon, Mar 24, 2025 at 12:53:29PM -0700, Daniel Verkamp wrote:
> On Mon, Mar 24, 2025 at 7:43 AM Maximilian Immanuel Brandtner
> <ma...@linux.ibm.com> wrote:
> >
> > According to section 5.3.6.2 (Multiport Device Operation) of the virtio
> > spec(version 1.2) a control buffer with the event VIRTIO_CONSOLE_RESIZE
> > is followed by a virtio_console_resize struct containing cols then rows.
> > The kernel implements this the wrong way around (rows then cols) resulting
> > in the two values being swapped.
> >
> > Signed-off-by: Maximilian Immanuel Brandtner <ma...@linux.ibm.com>
> > ---
> > drivers/char/virtio_console.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/char/virtio_console.c b/drivers/char/virtio_console.c
> > index 21de774996ad..38af3029da39 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 {
> > - __virtio16 rows;
> > __virtio16 cols;
> > + __virtio16 rows;
> > } size;
>
> The order of the fields after the patch matches the spec, so from that
> perspective, looks fine:
> Reviewed-by: Daniel Verkamp <dverk...@chromium.org>
>
> Since the driver code has been using the wrong order since support for
> this message was added in 2010, but there is no support for sending
> this message in the current qemu device implementation, I wondered
> what device code was used to test this when it was originally added. I
> dug up what I assume is the corresponding qemu device change from the
> same era, which sends the VIRTIO_CONSOLE_RESIZE message using the
> rows, cols order that matches the kernel driver (and differs from the
> spec):
>
> https://lore.kernel.org/qemu-devel/1273092505-22783-1-git-send-email-amit.s...@redhat.com/
> ("[Qemu-devel] [PATCH] virtio-serial: Send per-console port resize
> notifications to guest", May 6, 2010)
>
> However, I don't believe that patch ever made it into an actual qemu
> release, so it's probably not a compatibility concern. (If there are
> any other device implementations that use the kernel driver order
> rather than the spec order, then maybe this would need more
> consideration, but I don't personally know of any.)
>
> Thanks,
> -- Daniel
I agree. Cc author of the qemu patch for confirmation.
--
MST
