On Wed, 2025-03-19 at 18:13 +0100, Halil Pasic wrote: > On Wed, 19 Mar 2025 11:00:06 -0400 > "Michael S. Tsirkin" <m...@redhat.com> 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
Thanks, I didn't notice that, as I only tested this feature on x86. I double checked struct virtio_console_config as it also defines cols before rows, but there the kernel follows the spec (including endianness). I'll send a patch v2 shortly.
