2016-04-28 22:18:51

by Richard W.M. Jones

[permalink] [raw]
Subject: [PATCH] 8250: Hypervisors always export working 16550A UARTs.

[This is an opinionated patch, mainly for discussion.]

I'm trying to reduce the time taken in the kernel in initcalls, with
my aim being to reduce the current ~700ms spent in initcalls before
userspace, down to something like 100ms. All times on my Broadwell-U
laptop, under virtualization. The purpose of this is to be able to
launch VMs around containers with minimal overhead, like Intel Clear
Containers, but using standard distro kernels and qemu.

Currently the kernel spends 25ms inspecting the UART that we passed to
it from qemu to find out whether it's an 8250/16550/16550A perhaps
with a non-working FIFO or other quirks. Well, it isn't -- it's a
working emulated 16550A, with a FIFO and no quirks, and if it isn't,
we should fix qemu.

So the patch detects if we're running virtualized (perhaps it should
only check for qemu/KVM?) and if so, shortcuts the tests.

Rich.


2016-04-28 22:18:47

by Richard W.M. Jones

[permalink] [raw]
Subject: [PATCH] 8250: Hypervisors always export working 16550A UARTs.

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 <[email protected]>
---
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;
+ }
+
DEBUG_AUTOCONF("ttyS%d: autoconf (0x%04lx, 0x%p): ",
serial_index(port), port->iobase, port->membase);

--
2.7.4

2016-04-28 22:56:36

by Greg KH

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

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 <[email protected]>
> ---
> 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 :)

thanks,

greg k-h

2016-04-29 00:04:57

by Peter Hurley

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

On 04/28/2016 03:18 PM, Richard W.M. Jones wrote:
> [This is an opinionated patch, mainly for discussion.]
>
> I'm trying to reduce the time taken in the kernel in initcalls, with
> my aim being to reduce the current ~700ms spent in initcalls before
> userspace, down to something like 100ms. All times on my Broadwell-U
> laptop, under virtualization. The purpose of this is to be able to
> launch VMs around containers with minimal overhead, like Intel Clear
> Containers, but using standard distro kernels and qemu.
>
> Currently the kernel spends 25ms inspecting the UART that we passed to
> it from qemu to find out whether it's an 8250/16550/16550A perhaps
> with a non-working FIFO or other quirks. Well, it isn't -- it's a
> working emulated 16550A, with a FIFO and no quirks, and if it isn't,
> we should fix qemu.

I'm sure all of this delay is sizing the fifo:

static int size_fifo(struct uart_8250_port *up)
{
...
for (count = 0; count < 256; count++)
serial_out(up, UART_TX, count);
===> mdelay(20);/* FIXME - schedule_timeout */
for (count = 0; (serial_in(up, UART_LSR) & UART_LSR_DR) &&
(count < 256); count++)
serial_in(up, UART_RX);
...

return count;
}


> So the patch detects if we're running virtualized (perhaps it should
> only check for qemu/KVM?) and if so, shortcuts the tests.

Yeah, sorry, that's not going to fly.

I'm assuming x86, yes?

There's are multiple ways already supported by this driver to specify
the port based on the platform:

1. Register "serial8250" platform device.
Set UPF_FIXED_TYPE in ::flags and PORT_16550A in ::type
2. early_serial_setup()
Same as above, but must be before console_init()
3. Don't enumerate PNP0501 ACPI devices
These are going to be probed dynamically
4. Add ACPI/PNP custom device
Fixing the port type isn't supported now but could be
5. Modify the SERIAL_PORT_DFNS
Fixing the port type isn't supported now either but could be

Regards,
Peter Hurley

2016-04-29 07:01:32

by Matwey V. Kornilov

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

2016-04-29 1:18 GMT+03:00 Richard W.M. Jones <[email protected]>:
> [This is an opinionated patch, mainly for discussion.]
>
> I'm trying to reduce the time taken in the kernel in initcalls, with
> my aim being to reduce the current ~700ms spent in initcalls before
> userspace, down to something like 100ms. All times on my Broadwell-U
> laptop, under virtualization. The purpose of this is to be able to
> launch VMs around containers with minimal overhead, like Intel Clear
> Containers, but using standard distro kernels and qemu.
>
> Currently the kernel spends 25ms inspecting the UART that we passed to
> it from qemu to find out whether it's an 8250/16550/16550A perhaps
> with a non-working FIFO or other quirks. Well, it isn't -- it's a
> working emulated 16550A, with a FIFO and no quirks, and if it isn't,
> we should fix qemu.
>
> So the patch detects if we're running virtualized (perhaps it should
> only check for qemu/KVM?) and if so, shortcuts the tests.

Does anybody know, whether it is possible to pass through real
hardware serial port to a guest? It seems to be as simple as to pass
through an interrupt and memory IO ports.

>
> Rich.
>



--
With best regards,
Matwey V. Kornilov.
Sternberg Astronomical Institute, Lomonosov Moscow State University, Russia
119991, Moscow, Universitetsky pr-k 13, +7 (495) 9392382

2016-04-29 07:41:20

by Richard W.M. Jones

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

On Fri, Apr 29, 2016 at 10:01:08AM +0300, Matwey V. Kornilov wrote:
> 2016-04-29 1:18 GMT+03:00 Richard W.M. Jones <[email protected]>:
> > [This is an opinionated patch, mainly for discussion.]
> >
> > I'm trying to reduce the time taken in the kernel in initcalls, with
> > my aim being to reduce the current ~700ms spent in initcalls before
> > userspace, down to something like 100ms. All times on my Broadwell-U
> > laptop, under virtualization. The purpose of this is to be able to
> > launch VMs around containers with minimal overhead, like Intel Clear
> > Containers, but using standard distro kernels and qemu.
> >
> > Currently the kernel spends 25ms inspecting the UART that we passed to
> > it from qemu to find out whether it's an 8250/16550/16550A perhaps
> > with a non-working FIFO or other quirks. Well, it isn't -- it's a
> > working emulated 16550A, with a FIFO and no quirks, and if it isn't,
> > we should fix qemu.
> >
> > So the patch detects if we're running virtualized (perhaps it should
> > only check for qemu/KVM?) and if so, shortcuts the tests.
>
> Does anybody know, whether it is possible to pass through real
> hardware serial port to a guest? It seems to be as simple as to pass
> through an interrupt and memory IO ports.

In theory it seems like something VFIO could do. Passing something as
low performance as a serial port through would seem to make little
sense though.

Rich.

--
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
virt-builder quickly builds VMs from scratch
http://libguestfs.org/virt-builder.1.html

2016-04-29 08:10:18

by Richard W.M. Jones

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

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 <[email protected]>
> > ---
> > 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.

Rich.

>From 5694addf0e05de9c842274be8392ebce5dc24280 Mon Sep 17 00:00:00 2001
From: "Richard W.M. Jones" <[email protected]>
Date: Thu, 28 Apr 2016 23:08:54 +0100
Subject: [PATCH] 8250: Allow user to force 16550A UART and avoid probing.

Currently autoconf spends 25ms (on my laptop) testing the UART.

Allow the user to avoid this delay if they know that all serial ports
(eg on a virtual machine) are ordinary 16550A. The user does this by
passing '8250_base.really_16550a' on the command line.

Signed-off-by: Richard W.M. Jones <[email protected]>
---
drivers/tty/serial/8250/8250_port.c | 10 ++++++++++
1 file changed, 10 insertions(+)

diff --git a/drivers/tty/serial/8250/8250_port.c b/drivers/tty/serial/8250/8250_port.c
index 00ad2637..ac92f55 100644
--- a/drivers/tty/serial/8250/8250_port.c
+++ b/drivers/tty/serial/8250/8250_port.c
@@ -53,6 +53,10 @@
#define DEBUG_AUTOCONF(fmt...) do { } while (0)
#endif

+static bool really_16550a;
+module_param(really_16550a, bool, 0644);
+MODULE_PARM_DESC(really_16550a, "Don't probe, assume 16550A");
+
#define BOTH_EMPTY (UART_LSR_TEMT | UART_LSR_THRE)

/*
@@ -1171,6 +1175,12 @@ static void autoconfig(struct uart_8250_port *up)
if (!port->iobase && !port->mapbase && !port->membase)
return;

+ if (really_16550a) {
+ up->port.type = PORT_16550A;
+ up->capabilities |= UART_CAP_FIFO;
+ return;
+ }
+
DEBUG_AUTOCONF("ttyS%d: autoconf (0x%04lx, 0x%p): ",
serial_index(port), port->iobase, port->membase);

--
2.7.4



--
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
virt-p2v converts physical machines to virtual machines. Boot with a
live CD or over the network (PXE) and turn machines into KVM guests.
http://libguestfs.org/virt-v2v

2016-04-29 15:15:43

by Greg KH

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

On Fri, Apr 29, 2016 at 08:41:14AM +0100, Richard W.M. Jones wrote:
> On Fri, Apr 29, 2016 at 10:01:08AM +0300, Matwey V. Kornilov wrote:
> > 2016-04-29 1:18 GMT+03:00 Richard W.M. Jones <[email protected]>:
> > > [This is an opinionated patch, mainly for discussion.]
> > >
> > > I'm trying to reduce the time taken in the kernel in initcalls, with
> > > my aim being to reduce the current ~700ms spent in initcalls before
> > > userspace, down to something like 100ms. All times on my Broadwell-U
> > > laptop, under virtualization. The purpose of this is to be able to
> > > launch VMs around containers with minimal overhead, like Intel Clear
> > > Containers, but using standard distro kernels and qemu.
> > >
> > > Currently the kernel spends 25ms inspecting the UART that we passed to
> > > it from qemu to find out whether it's an 8250/16550/16550A perhaps
> > > with a non-working FIFO or other quirks. Well, it isn't -- it's a
> > > working emulated 16550A, with a FIFO and no quirks, and if it isn't,
> > > we should fix qemu.
> > >
> > > So the patch detects if we're running virtualized (perhaps it should
> > > only check for qemu/KVM?) and if so, shortcuts the tests.
> >
> > Does anybody know, whether it is possible to pass through real
> > hardware serial port to a guest? It seems to be as simple as to pass
> > through an interrupt and memory IO ports.
>
> In theory it seems like something VFIO could do. Passing something as
> low performance as a serial port through would seem to make little
> sense though.

Multi-port serial PCI cards are not "low performance" and might be
passed through directly to a guest.

thanks,

greg k-h

2016-04-29 15:16:37

by Greg KH

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

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 <[email protected]>
> > > ---
> > > 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?

greg k-h

2016-04-29 15:38:03

by Richard W.M. Jones

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

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 <[email protected]>
> > > > ---
> > > > 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.

--
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
virt-builder quickly builds VMs from scratch
http://libguestfs.org/virt-builder.1.html

2016-04-29 15:54:16

by Greg KH

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

On Fri, Apr 29, 2016 at 04:37:57PM +0100, 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 <[email protected]>
> > > > > ---
> > > > > 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.

You are trying to take a generalized kernel and somehow "know" about the
hardware ahead of time it is going to run on. That seems like two
conflicting requirements, don't you agree?

good luck,

greg k-h

2016-04-29 16:03:00

by Richard W.M. Jones

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

On Fri, Apr 29, 2016 at 08:54:13AM -0700, Greg KH wrote:
> You are trying to take a generalized kernel and somehow "know" about the
> hardware ahead of time it is going to run on. That seems like two
> conflicting requirements, don't you agree?

We would have the 8250 serial port in any kernel. Even if Fedora
kernel maintainers allowed us to have specialized kernels for each
purpose, I would use the simple ISA serial port here because it allows
us to capture debug messages very early in the boot. Alternatives
like virtio-console don't allow that.

The kernel does know what hardware it's running on - via the CPUID
hypervisor leaf. It's also possible for us to tell the kernel about
the hardware using the command line, ACPI[*], DT, etc. I'd really
like to tell the kernel this is a 16550A, not broken, you don't need
to spend time testing that.

There is prior art here: no_timer_check & lpj=..

Rich.

[*] Although ACPI is really slow, adding another 190ms, and for this
reason I have disabled it for now, but not investigated why it's so
slow.

--
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
libguestfs lets you edit virtual machines. Supports shell scripting,
bindings from many languages. http://libguestfs.org

2016-04-29 17:39:42

by Greg KH

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

On Fri, Apr 29, 2016 at 05:02:57PM +0100, Richard W.M. Jones wrote:
> On Fri, Apr 29, 2016 at 08:54:13AM -0700, Greg KH wrote:
> > You are trying to take a generalized kernel and somehow "know" about the
> > hardware ahead of time it is going to run on. That seems like two
> > conflicting requirements, don't you agree?
>
> We would have the 8250 serial port in any kernel. Even if Fedora
> kernel maintainers allowed us to have specialized kernels for each
> purpose, I would use the simple ISA serial port here because it allows
> us to capture debug messages very early in the boot. Alternatives
> like virtio-console don't allow that.
>
> The kernel does know what hardware it's running on - via the CPUID
> hypervisor leaf. It's also possible for us to tell the kernel about
> the hardware using the command line, ACPI[*], DT, etc. I'd really
> like to tell the kernel this is a 16550A, not broken, you don't need
> to spend time testing that.

Then properly describe the device to the kernel in a way that you know
you don't have to do the 20ms sleep from your system configuration file
(acpi, dt, etc.)

good luck,

greg k-h

2016-04-29 18:14:09

by Donald Dutile

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

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 <[email protected]>
>>>>> ---
>>>>> 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.