[PATCH] virtio: console: Make resize control event handling compliant with spec

[PATCH] virtio: console: Make resize control event handling compliant with spec

[Maximilian Immanuel Brandtner]

According to section 5.3.6.2 of the virtio spec a control buffer with
the event VIRITO_CONSOLE_RESIZE is followed by a virtio_console_resize
struct containing 2 little endian 16bit integerts cols,rows. The kernel
implementation assums native endianness (which results in mangled values
on big endian architectures)(a seperate patch by Halil Pasic will deal with
that issue) and swaps the ordering of columns and rows.
This patch changes the ordering of rows and columns to columns then
rows.

Fixes: 8345adbf96fc1 ("virtio: console: Accept console size along with resize control message")
Signed-off-by: Maximilian Immanuel Brandtner <maxbr@linux.ibm.com>
Cc: stable@vger.kernel.org # v2.6.35+
---
 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 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))
-- 
2.48.1

Re: [PATCH] virtio: console: Make resize control event handling compliant with spec

[Maximilian Immanuel Brandtner]

On Thu, 2025-03-20 at 18:26 +0100, Maximilian Immanuel Brandtner wrote:
> According to section 5.3.6.2 of the virtio spec a control buffer with
> the event VIRITO_CONSOLE_RESIZE is followed by a
> virtio_console_resize
> struct containing 2 little endian 16bit integerts cols,rows. The
> kernel
> implementation assums native endianness (which results in mangled
> values
> on big endian architectures)(a seperate patch by Halil Pasic will
> deal with
> that issue) and swaps the ordering of columns and rows.
> This patch changes the ordering of rows and columns to columns then
> rows.
> 
> Fixes: 8345adbf96fc1 ("virtio: console: Accept console size along
> with resize control message")
> Signed-off-by: Maximilian Immanuel Brandtner <maxbr@linux.ibm.com>
> Cc: stable@vger.kernel.org # v2.6.35+
> ---
>  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 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))

I forgot to run git format-patch with --subject-prefix="PATCH v2".
Sorry about that

Re: [PATCH] virtio: console: Make resize control event handling compliant with spec

[Halil Pasic]

On Thu, 20 Mar 2025 18:26:54 +0100
Maximilian Immanuel Brandtner <maxbr@linux.ibm.com> wrote:

Let my try to do some nitpicking here. First on the subject. I would go
with something more specific like "fix order of fields cols and rows",
especially now that we have figured out the byte order handling is absent
as well.

> According to section 5.3.6.2 of the virtio spec a control buffer with

I would refrain form referencing section from the spec only by its
number without also telling which incarnation of the spec you mean. I
don't think those numbers are set in stone.

> the event VIRITO_CONSOLE_RESIZE is followed by a virtio_console_resize
> struct containing 2 little endian 16bit integerts cols,rows. 

s/integerts/integers/

I think technically it is still the same buffer. I would opt for struct
virtio_console_control is followed by struct virtio_console_resize. 

> The kernel
> implementation assums native endianness (which results in mangled
> values on big endian architectures)(a seperate patch by Halil Pasic
> will deal with that issue) and swaps the ordering of columns and rows.

I would simply omit the reference to the other patch. I understand that
you introduced this because your patch does not deliver everything that
the short description promises.

I didn't realize that addressing the byte order stuff would cause
a merge conflict, but it does. So maybe it would make sense to rebase
your fix on top of mine and send it as a series titled.
"Make resize control event handling compliant with spec". Just an idea.


> This patch changes the ordering of rows and columns to columns then
> rows.
> 
> Fixes: 8345adbf96fc1 ("virtio: console: Accept console size along with
> resize control message") Signed-off-by: Maximilian Immanuel Brandtner
> <maxbr@linux.ibm.com> Cc: stable@vger.kernel.org # v2.6.35+
> ---
>  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 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))

Re: [PATCH] virtio: console: Make resize control event handling compliant with spec

[Amit Shah]

On Thu, 2025-03-20 at 18:26 +0100, Maximilian Immanuel Brandtner wrote:
> According to section 5.3.6.2 of the virtio spec a control buffer with
> the event VIRITO_CONSOLE_RESIZE is followed by a
> virtio_console_resize
> struct containing 2 little endian 16bit integerts cols,rows. The
> kernel
> implementation assums native endianness (which results in mangled
> values
> on big endian architectures)(a seperate patch by Halil Pasic will
> deal with
> that issue) and swaps the ordering of columns and rows.
> This patch changes the ordering of rows and columns to columns then
> rows.

It's clearer and cleaner to only describe the issue you're solving
here; the other patch will take care of the other issue.

You can add more info here, like: the console resize messages via the
config space (section 5.3.4) are used for resizing, and that works fine
for single-console devices.  However, for multi-console devices
(enabled by multiport) had the spec and Linux implementation out of
sync.  As far as I know, there are no device-side implementations of
mutiport console, so patching Linux is the way to go.  (You can replace
the 'I' with my name as the maintainer there.)

> Fixes: 8345adbf96fc1 ("virtio: console: Accept console size along
> with resize control message")
> Signed-off-by: Maximilian Immanuel Brandtner <maxbr@linux.ibm.com>
> Cc: stable@vger.kernel.org # v2.6.35+
> ---
>  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 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))


Thanks,

		Amit