[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

[Xen-devel] [PATCH] V4 pci uart - better cope with UART being temporarily unavailable



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 V3:
* readded invalid port test in the loop in __ns16550, moved it before 
serial_rx_interrupt call

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>
---
 xen/drivers/char/ehci-dbgp.c |    2 +-
 xen/drivers/char/ns16550.c   |   27 ++++++++++++++++-----------
 xen/drivers/char/serial.c    |   38 +++++++++++++++++++++++++-------------
 xen/include/xen/serial.h     |    5 +++--
 4 files changed, 45 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..3245886 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)
 {
@@ -103,11 +108,17 @@ static void __ns16550_poll(struct cpu_user_regs *regs)
         return; /* Interrupts work - no more polling */
 
     while ( ns_read_reg(uart, UART_LSR) & UART_LSR_DR )
+    {
+        if ( ns16550_ioport_invalid(uart) )
+            goto out;
+
         serial_rx_interrupt(port, regs);
+    }
 
     if ( ns_read_reg(uart, UART_LSR) & UART_LSR_THRE )
         serial_tx_interrupt(port, regs);
 
+out:
     set_timer(&uart->timer, NOW() + MILLISECS(uart->timeout_ms));
 }
 
@@ -121,10 +132,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 +151,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 +319,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


 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.