Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1390596 > unrolled thread

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

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  [PATCH] 8250: Hypervisors always export working 16550A UARTs. "Richard W.M. Jones" <rjones@redhat.com> - 2016-04-29 00:20 +0200
    Re: [PATCH] 8250: Hypervisors always export working 16550A UARTs. Greg KH <gregkh@linuxfoundation.org> - 2016-04-29 01:00 +0200
      Re: [PATCH] 8250: Hypervisors always export working 16550A UARTs. "Richard W.M. Jones" <rjones@redhat.com> - 2016-04-29 10:20 +0200
        Re: [PATCH] 8250: Hypervisors always export working 16550A UARTs. Greg KH <gregkh@linuxfoundation.org> - 2016-04-29 17:20 +0200
          Re: [PATCH] 8250: Hypervisors always export working 16550A UARTs. "Richard W.M. Jones" <rjones@redhat.com> - 2016-04-29 17:40 +0200
            Re: [PATCH] 8250: Hypervisors always export working 16550A UARTs. Greg KH <gregkh@linuxfoundation.org> - 2016-04-29 18:00 +0200
              Re: [PATCH] 8250: Hypervisors always export working 16550A UARTs. "Richard W.M. Jones" <rjones@redhat.com> - 2016-04-29 18:10 +0200
                Re: [PATCH] 8250: Hypervisors always export working 16550A UARTs. Greg KH <gregkh@linuxfoundation.org> - 2016-04-29 19:40 +0200
            Re: [PATCH] 8250: Hypervisors always export working 16550A UARTs. Don Dutile <ddutile@redhat.com> - 2016-04-29 20:20 +0200

#1390596 — [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 <rjones@redhat.com>
---
 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

[toc] | [next] | [standalone]

#1390607

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@redhat.com>
> ---
>  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

[toc] | [prev] | [next] | [standalone]

#1390859

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@redhat.com>
> > ---
> >  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" <rjones@redhat.com>
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 <rjones@redhat.com>
---
 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

[toc] | [prev] | [next] | [standalone]

#1391236

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@redhat.com>
> > > ---
> > >  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

[toc] | [prev] | [next] | [standalone]

#1391255

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@redhat.com>
> > > > ---
> > > >  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

[toc] | [prev] | [next] | [standalone]

#1391268

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 <rjones@redhat.com>
> > > > > ---
> > > > >  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

[toc] | [prev] | [next] | [standalone]

#1391273

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

[toc] | [prev] | [next] | [standalone]

#1391303

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

[toc] | [prev] | [next] | [standalone]

#1391320

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@redhat.com>
>>>>> ---
>>>>>   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.

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web