Re: [PATCH] 8250: Hypervisors always export working 16550A UARTs.

From: Don Dutile
Date: Fri Apr 29 2016 - 14:14:12 EST


On 04/29/2016 11:37 AM, Richard W.M. Jones wrote:
On Fri, Apr 29, 2016 at 08:16:35AM -0700, Greg KH wrote:
On Fri, Apr 29, 2016 at 09:10:06AM +0100, Richard W.M. Jones wrote:
On Thu, Apr 28, 2016 at 03:56:33PM -0700, Greg KH wrote:
On Thu, Apr 28, 2016 at 11:18:33PM +0100, Richard W.M. Jones wrote:
Currently autoconf spends 25ms (on my laptop) testing if the UART
exported to it by KVM is an 8250 without FIFO and/or with strange
quirks, which it obviously isn't. Assume it is exported to us by a
hypervisor, it's a normal, working 16550A.

Signed-off-by: Richard W.M. Jones <rjones@xxxxxxxxxx>
---
drivers/tty/serial/8250/8250_port.c | 7 +++++++
1 file changed, 7 insertions(+)

diff --git a/drivers/tty/serial/8250/8250_port.c b/drivers/tty/serial/8250/8250_port.c
index 00ad2637..de19924 100644
--- a/drivers/tty/serial/8250/8250_port.c
+++ b/drivers/tty/serial/8250/8250_port.c
@@ -1171,6 +1171,13 @@ static void autoconfig(struct uart_8250_port *up)
if (!port->iobase && !port->mapbase && !port->membase)
return;

+ /* Hypervisors always export working 16550A devices. */
+ if (cpu_has_hypervisor) {
+ up->port.type = PORT_16550A;
+ up->capabilities |= UART_CAP_FIFO;
+ return;
+ }

Have you audited vmware, virtualbox, and everyone else that provides a
virtual uart device that it will work properly here?

qemu isn't all the world :)

Attached below is a slightly different approach. If the user passes a
special flag on the kernel command line then we force 16550A and avoid
the 25ms delay. Since the user chooses the flag, any concerns about
the behaviour of the hypervisor or use of VFIO should be moot.

No, no more module parameters, that's crazy, what happens when you have
64 serial ports in a system, which one is this option for?

In this (very special) case, the domain is running under qemu and
I know it only has one serial port that doesn't need probing.

What's the right way to avoid this 25ms delay?

Rich.

param && cpu_has_hypervisor?
.... that restricts it to your use case.
... you can 1+ which hypervisor it is if you add export for pv_info(.name).
... all of which only works on x86, as cpu_has_hypervisor is defined here:
arch/x86/include/asm/cpufeature.h

which points to a better design based on config option:
if(CONFIG_<NEW_OPTION>) <your optimization of choice>
then it can be optimized in & out as needed.
Single, binary kernel for bare-metal & virt (e.g., rhel) would
bear the additional CONFIG_xxx check, but that's during boot/init,
which isn't perf sensitive on real hw.

You could then use the above CONFIG_NEW_OPTION to optimize other kvm-guest
boot init callbacks/paths.