Re: [Xen-devel] [PATCH] ns16550: delay resume until dom0 ACPI has a chance to run

[Pasi Kärkkäinen <pasik@xxxxxx>]

On Wed, Jan 16, 2013 at 04:48:41PM -0500, Ben Guthro wrote:
> On Wed, Jan 16, 2013 at 4:40 PM, Malcolm Crossley
> <malcolm.crossley@xxxxxxxxxx> wrote:
> 
> > Do these laptops (T430/T530) have built in serial?
> 
> They seem to have the hardware for it, but no actual serial connector
> out of the machine.
>

I believe you can access the serial port over the network,
thru the AMT management feature.

-- Pasi

> This hardware provides the legacy port that Xen initializes in
> xen/arch/x86/setup.c __start_xen()
> 
> When the resume happened, it was getting stuck in __ns16550_poll()
> because it thought that the
> LSR register was 0xFF - and had lots of data to read. It got stuck in
> that while loop, and never
> exited.
> 
> 
> >
> > Or is the hang fixed because you were using a serial Express card to debug?
> >
> > BTW, nearly all PC's have external SUPERIO devices, it just seems we have
> > managed to be lucky that most platforms seem to default to using the BIOS to
> > enable the serial port after resume instead of the OS.
> >
> > Malcolm
> >
> >> Ben
> >>
> >>
> >>
> >>> -- Pasi
> >>>
> >>>
> >>>> Signed-Off-By: Ben Guthro <benjamin.guthro@xxxxxxxxxx>
> >>>> ---
> >>>>    xen/drivers/char/ns16550.c |   56
> >>>> +++++++++++++++++++++++++++++++++++++++++++-
> >>>>    1 file changed, 55 insertions(+), 1 deletion(-)
> >>>>
> >>>> diff --git a/xen/drivers/char/ns16550.c b/xen/drivers/char/ns16550.c
> >>>> index d77042e..27555c7 100644
> >>>> --- a/xen/drivers/char/ns16550.c
> >>>> +++ b/xen/drivers/char/ns16550.c
> >>>> @@ -42,6 +42,7 @@ static struct ns16550 {
> >>>>        struct irqaction irqaction;
> >>>>        /* UART with no IRQ line: periodically-polled I/O. */
> >>>>        struct timer timer;
> >>>> +    struct timer resume_timer;
> >>>>        unsigned int timeout_ms;
> >>>>        bool_t intr_works;
> >>>>        /* PCI card parameters. */
> >>>> @@ -120,6 +121,10 @@ static struct ns16550 {
> >>>>    /* Frequency of external clock source. This definition assumes PC
> >>>> platform. */
> >>>>    #define UART_CLOCK_HZ   1843200
> >>>>
> >>>> +/* Resume retry settings */
> >>>> +#define RESUME_DELAY    MILLISECS(10)
> >>>> +#define RESUME_RETRIES  100
> >>>> +
> >>>>    static char ns_read_reg(struct ns16550 *uart, int reg)
> >>>>    {
> >>>>        if ( uart->remapped_io_base == NULL )
> >>>> @@ -330,7 +335,7 @@ static void ns16550_suspend(struct serial_port
> >>>> *port)
> >>>>                                      uart->ps_bdf[2], PCI_COMMAND);
> >>>>    }
> >>>>
> >>>> -static void ns16550_resume(struct serial_port *port)
> >>>> +static void __ns16550_resume(struct serial_port *port)
> >>>>    {
> >>>>        struct ns16550 *uart = port->uart;
> >>>>
> >>>> @@ -346,6 +351,55 @@ static void ns16550_resume(struct serial_port
> >>>> *port)
> >>>>        ns16550_setup_postirq(port->uart);
> >>>>    }
> >>>>
> >>>> +static int ns16550_ioport_invalid(struct ns16550 *uart)
> >>>> +{
> >>>> +    return ((((unsigned char)ns_read_reg(uart, LSR)) == 0xff) &&
> >>>> +            (((unsigned char)ns_read_reg(uart, MCR)) == 0xff) &&
> >>>> +            (((unsigned char)ns_read_reg(uart, IER)) == 0xff) &&
> >>>> +            (((unsigned char)ns_read_reg(uart, IIR)) == 0xff) &&
> >>>> +            (((unsigned char)ns_read_reg(uart, LCR)) == 0xff));
> >>>> +}
> >>>> +
> >>>> +static int delayed_resume_tries;
> >>>> +static void ns16550_delayed_resume(void *data)
> >>>> +{
> >>>> +    struct serial_port *port = data;
> >>>> +    struct ns16550 *uart = port->uart;
> >>>> +
> >>>> +    if (ns16550_ioport_invalid(port->uart) && delayed_resume_tries--) {
> >>>> +       set_timer(&uart->resume_timer, NOW() + RESUME_DELAY);
> >>>> +       return;
> >>>> +    }
> >>>> +
> >>>> +    __ns16550_resume(port);
> >>>> +}
> >>>> +
> >>>> +static void ns16550_resume(struct serial_port *port)
> >>>> +{
> >>>> +    struct ns16550 *uart = port->uart;
> >>>> +
> >>>> +    /*
> >>>> +     * Check for ioport access, before fully resuming operation.
> >>>> +     * On some systems, there is a SuperIO card that provides
> >>>> +     * this legacy ioport on the LPC bus.
> >>>> +     *
> >>>> +     * We need to wait for dom0's ACPI processing to run the proper
> >>>> +     * AML to re-initialize the chip, before we can use the card again.
> >>>> +     *
> >>>> +     * This may cause a small amount of garbage to be written
> >>>> +     * to the serial log while we wait patiently for that AML to
> >>>> +     * be executed.
> >>>> +     */
> >>>> +    if (ns16550_ioport_invalid(uart)) {
> >>>> +       delayed_resume_tries = RESUME_RETRIES;
> >>>> +       init_timer(&uart->resume_timer, ns16550_delayed_resume, port,
> >>>> 0);
> >>>> +       set_timer(&uart->resume_timer, NOW() + RESUME_DELAY);
> >>>> +       return;
> >>>> +    }
> >>>> +
> >>>> +    __ns16550_resume(port);
> >>>> +}
> >>>> +
> >>>>    #ifdef CONFIG_X86
> >>>>    static void __init ns16550_endboot(struct serial_port *port)
> >>>>    {
> >>>> --
> >>>> 1.7.9.5
> >>>>
> >>>>
> >>>> _______________________________________________
> >>>> Xen-devel mailing list
> >>>> Xen-devel@xxxxxxxxxxxxx
> >>>> http://lists.xen.org/xen-devel
> >>
> >> _______________________________________________
> >> Xen-devel mailing list
> >> Xen-devel@xxxxxxxxxxxxx
> >> http://lists.xen.org/xen-devel
> >
> >
> >
> > _______________________________________________
> > Xen-devel mailing list
> > Xen-devel@xxxxxxxxxxxxx
> > http://lists.xen.org/xen-devel
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@xxxxxxxxxxxxx
> http://lists.xen.org/xen-devel

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel