Virtualization - Nov 2022 - [PATCH] virtio: console: remove check for cpkt value when nominating console port

On Sat, 2022-11-12 at 23:45 +1100, Andy Bui wrote:
> The virtIO spec does not specify a need for a value when nominating a
> port as a console port, yet the virtio_console driver requires the value
> to be 1.
> 
> Besides being a check that's not specified by the virtIO spec, I
don't
> see anywhere else in the kernel the value is used when the corresponding
> event is VIRTIO_CONSOLE_CONSOLE_PORT. As an example QEMU also currently
> only passes in value=1 when nominating a console port.
The original virtio-console driver just had the one port, and it was,
as the name suggests, was a console port.  When we converted this
console driver to a generic serial driver, the first port was kept as
the console port to not break old drivers or hypervisors.  I'm afraid
we'll have to keep this bit of backward compatibility forever.


		Amit

On Mon, Nov 14, 2022 at 09:43:48AM -0800, Amit Shah
wrote:
> 
> On Sat, 2022-11-12 at 23:45 +1100, Andy Bui wrote:
> > The virtIO spec does not specify a need for a value when nominating a
> > port as a console port, yet the virtio_console driver requires the
value
> > to be 1.
> > 
> > Besides being a check that's not specified by the virtIO spec, I
don't
> > see anywhere else in the kernel the value is used when the
corresponding
> > event is VIRTIO_CONSOLE_CONSOLE_PORT. As an example QEMU also
currently
> > only passes in value=1 when nominating a console port.
> 
> The original virtio-console driver just had the one port, and it was,
> as the name suggests, was a console port.  When we converted this
> console driver to a generic serial driver, the first port was kept as
> the console port to not break old drivers or hypervisors.  I'm afraid
> we'll have to keep this bit of backward compatibility forever.
> 
I think I might be misunderstanding the exact usage of the cpkt value.
Is error'ing and not continuing with the console port nomination the
intended
behaviour when value != 1? I'm guessing that there are some hypervisors out
there that set value to something not 1 and we don't want those ports to be
the console ports?

If this is correct I think a warning should be emitted here? Not setting the
value to 1 can exhibit undefined behaviour (at least when interpreting the
virtIO spec) and it would help hypervisor developers.

Thanks and lmk your thoughts,
Andy