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

[Halil Pasic]

On Wed, 19 Mar 2025 11:00:06 -0400
"Michael S. Tsirkin" <mst@xxxxxxxxxx> wrote:

> > > I was mistaken in my earlier reply - I had missed this
> > > virtio_console_resize definition in the spec.  So indeed there's a
> > > discrepancy in Linux kernel and the spec's ordering for the control
> > > message.
> > > 
> > > OK, that needs fixing someplace.  Perhaps in the kernel (like your
> > > orig. patch), but with an accurate commit message.  
> > 
> > So should I send a patch v2 or should the spec be changed instead? Or
> > would you like to first await the opinion of the spec maintainers?
> > 
> > The mail I initially sent to the virtio mailing list seems to have
> > fallen on deaf ears. I now added Michael Tsirkin to this thread so that
> > things might get going.  
> 
> 
> If we can fix the driver to fit the spec, that's best.
> We generally try to avoid changing the spec just because
> drivers are buggy.

I think the call if fixing the driver is possible needs to be made by
the maintainers of the driver. Fixing the driver IMHO implies that
if this is seeing any usage in the wild where it properly works a
fix on the driver side would imply a function regression. But any
implementers should have complained. So IMHO it is not unreasonable to
assume that this is not seeing any usage in the wild.

And people would still have the opportunity to catch the regression
during testing and complain about it.

I agree with Michael, changing the spec because of a buggy
implementation should rather be the exception than the rule. And AFAIK
it is not like we have declared something a reference implementation,
so in that sense the implementation in Linux is just one implementation.

I suppose making it runtime configurable via module parameter is an
overkill at this point.

So based no what we know I'm slightly in favor of let us just fix it
in Linux and see if anybody complains.

Another thing I noticed during looking at this. AFAICT Linux does not
seem to handle endiannes here. And AFAIU the message is supposed to hold
le16 that is little endian u16! Maximilian, is this in your opinion
something we need to fix as well? Or am I just missing the conversion?

Regards,
Halil