|
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] [PATCH] V3 pci uart - better cope with UART being temporarily unavailable
>>> On 27.08.13 at 15:54, Tomasz Wroblewski <tomasz.wroblewski@xxxxxxxxxx>
>>> wrote:
> This happens for example when dom0 disables ioport responses during PCI
> subsystem initialisation. If a __ns16550_poll() happens to be scheduled
> during that time, Xen hangs. Detect and exit that condition.
>
> Amended ns16550_ioport_invalid function to only check IER register, which
> contins 3 reserved (always 0) bits, therefore it's sufficient for that test.
>
> Changes since V2:
> * pulled out invalid port test before the loop in __ns16550_poll
> * coding style/patch size fixes
>
> Changes since V1:
> * added invalid port test in getc()
> * changed ns16550_tx_ready to return -EIO and code in serial.[ch] to cope
> with that error condition. If port is temporarily unavailable tx_ready will
> return -EIO and serial driver code will attempt to buffer the character
> instead of dropping it.
>
> Signed-off-by: Tomasz Wroblewski <tomasz.wroblewski@xxxxxxxxxx>
Reviewed-by: Jan Beulich <jbeulich@xxxxxxxx>
(and you may retain the tag if you decide to undo the first of the
two mentioned changes since V2 - I'm fine with it either way)
> ---
> xen/drivers/char/ehci-dbgp.c | 2 +-
> xen/drivers/char/ns16550.c | 24 +++++++++++++-----------
> xen/drivers/char/serial.c | 38 +++++++++++++++++++++++++-------------
> xen/include/xen/serial.h | 5 +++--
> 4 files changed, 42 insertions(+), 27 deletions(-)
>
> diff --git a/xen/drivers/char/ehci-dbgp.c b/xen/drivers/char/ehci-dbgp.c
> index 6b70660..6aa502e 100644
> --- a/xen/drivers/char/ehci-dbgp.c
> +++ b/xen/drivers/char/ehci-dbgp.c
> @@ -1204,7 +1204,7 @@ static void ehci_dbgp_putc(struct serial_port *port,
> char c)
> ehci_dbgp_flush(port);
> }
>
> -static unsigned int ehci_dbgp_tx_ready(struct serial_port *port)
> +static int ehci_dbgp_tx_ready(struct serial_port *port)
> {
> struct ehci_dbgp *dbgp = port->uart;
>
> diff --git a/xen/drivers/char/ns16550.c b/xen/drivers/char/ns16550.c
> index 6082c85..e8d3232 100644
> --- a/xen/drivers/char/ns16550.c
> +++ b/xen/drivers/char/ns16550.c
> @@ -73,6 +73,11 @@ static void ns_write_reg(struct ns16550 *uart, int reg,
> char c)
> writeb(c, uart->remapped_io_base + reg);
> }
>
> +static int ns16550_ioport_invalid(struct ns16550 *uart)
> +{
> + return (unsigned char)ns_read_reg(uart, UART_IER) == 0xff;
> +}
> +
> static void ns16550_interrupt(
> int irq, void *dev_id, struct cpu_user_regs *regs)
> {
> @@ -102,6 +107,9 @@ static void __ns16550_poll(struct cpu_user_regs *regs)
> if ( uart->intr_works )
> return; /* Interrupts work - no more polling */
>
> + if ( ns16550_ioport_invalid(uart) )
> + return;
> +
> while ( ns_read_reg(uart, UART_LSR) & UART_LSR_DR )
> serial_rx_interrupt(port, regs);
>
> @@ -121,10 +129,12 @@ static void ns16550_poll(void *data)
> #endif
> }
>
> -static unsigned int ns16550_tx_ready(struct serial_port *port)
> +static int ns16550_tx_ready(struct serial_port *port)
> {
> struct ns16550 *uart = port->uart;
>
> + if ( ns16550_ioport_invalid(uart) )
> + return -EIO;
> return ns_read_reg(uart, UART_LSR) & UART_LSR_THRE ? uart->fifo_size : 0;
> }
>
> @@ -138,7 +148,8 @@ static int ns16550_getc(struct serial_port *port, char
> *pc)
> {
> struct ns16550 *uart = port->uart;
>
> - if ( !(ns_read_reg(uart, UART_LSR) & UART_LSR_DR) )
> + if ( ns16550_ioport_invalid(uart) ||
> + !(ns_read_reg(uart, UART_LSR) & UART_LSR_DR) )
> return 0;
>
> *pc = ns_read_reg(uart, UART_RBR);
> @@ -305,15 +316,6 @@ 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, UART_LSR)) == 0xff) &&
> - (((unsigned char)ns_read_reg(uart, UART_MCR)) == 0xff) &&
> - (((unsigned char)ns_read_reg(uart, UART_IER)) == 0xff) &&
> - (((unsigned char)ns_read_reg(uart, UART_IIR)) == 0xff) &&
> - (((unsigned char)ns_read_reg(uart, UART_LCR)) == 0xff));
> -}
> -
> static int delayed_resume_tries;
> static void ns16550_delayed_resume(void *data)
> {
> diff --git a/xen/drivers/char/serial.c b/xen/drivers/char/serial.c
> index cd0b864..9b006f2 100644
> --- a/xen/drivers/char/serial.c
> +++ b/xen/drivers/char/serial.c
> @@ -59,7 +59,7 @@ void serial_rx_interrupt(struct serial_port *port, struct
> cpu_user_regs *regs)
>
> void serial_tx_interrupt(struct serial_port *port, struct cpu_user_regs
> *regs)
> {
> - unsigned int i, n;
> + int i, n;
> unsigned long flags;
>
> local_irq_save(flags);
> @@ -71,7 +71,7 @@ void serial_tx_interrupt(struct serial_port *port, struct
> cpu_user_regs *regs)
> */
> while ( !spin_trylock(&port->tx_lock) )
> {
> - if ( !port->driver->tx_ready(port) )
> + if ( port->driver->tx_ready(port) <= 0 )
> goto out;
> cpu_relax();
> }
> @@ -111,15 +111,18 @@ static void __serial_putc(struct serial_port *port,
> char c)
> if ( port->tx_log_everything )
> {
> /* Buffer is full: we spin waiting for space to appear. */
> - unsigned int n;
> + int n;
>
> while ( (n = port->driver->tx_ready(port)) == 0 )
> cpu_relax();
> - while ( n-- )
> - port->driver->putc(
> - port,
> - port->txbuf[mask_serial_txbuf_idx(port->txbufc++)]);
> - port->txbuf[mask_serial_txbuf_idx(port->txbufp++)] = c;
> + if ( n > 0 )
> + {
> + while ( n-- )
> + port->driver->putc(
> + port,
> +
> port->txbuf[mask_serial_txbuf_idx(port->txbufc++)]);
> + port->txbuf[mask_serial_txbuf_idx(port->txbufp++)] = c;
> + }
> }
> else
> {
> @@ -130,9 +133,9 @@ static void __serial_putc(struct serial_port *port, char
> c)
> }
>
> if ( ((port->txbufp - port->txbufc) == 0) &&
> - port->driver->tx_ready(port) )
> + port->driver->tx_ready(port) > 0 )
> {
> - /* Buffer and UART FIFO are both empty. */
> + /* Buffer and UART FIFO are both empty, and port is available.
> */
> port->driver->putc(port, c);
> }
> else
> @@ -143,10 +146,13 @@ static void __serial_putc(struct serial_port *port,
> char c)
> }
> else if ( port->driver->tx_ready )
> {
> + int n;
> +
> /* Synchronous finite-capacity transmitter. */
> - while ( !port->driver->tx_ready(port) )
> + while ( !(n = port->driver->tx_ready(port)) )
> cpu_relax();
> - port->driver->putc(port, c);
> + if ( n > 0 )
> + port->driver->putc(port, c);
> }
> else
> {
> @@ -390,8 +396,14 @@ void serial_start_sync(int handle)
> {
> while ( (port->txbufp - port->txbufc) != 0 )
> {
> - while ( !port->driver->tx_ready(port) )
> + int n;
> +
> + while ( !(n = port->driver->tx_ready(port)) )
> cpu_relax();
> + if ( n < 0 )
> + /* port is unavailable and might not come up until
> reenabled by
> + dom0, we can't really do proper sync */
> + break;
> port->driver->putc(
> port, port->txbuf[mask_serial_txbuf_idx(port->txbufc++)]);
> }
> diff --git a/xen/include/xen/serial.h b/xen/include/xen/serial.h
> index 403e193..f38c9b7 100644
> --- a/xen/include/xen/serial.h
> +++ b/xen/include/xen/serial.h
> @@ -70,8 +70,9 @@ struct uart_driver {
> /* Driver suspend/resume. */
> void (*suspend)(struct serial_port *);
> void (*resume)(struct serial_port *);
> - /* Return number of characters the port can hold for transmit. */
> - unsigned int (*tx_ready)(struct serial_port *);
> + /* Return number of characters the port can hold for transmit,
> + * or -EIO if port is inaccesible */
> + int (*tx_ready)(struct serial_port *);
> /* Put a character onto the serial line. */
> void (*putc)(struct serial_port *, char);
> /* Flush accumulated characters. */
> --
> 1.7.10.4
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel
|
 |
Lists.xenproject.org is hosted with RackSpace, monitoring our |