Hi Gabriel,
On 11/11/2015 09:18 AM, Peter Hurley wrote:
> On 11/11/2015 08:15 AM, Gabriel Krisman Bertazi wrote:
>> When a PCI error is detected, reset code in 8250_pci.c suspends the uart
>> ports to block IO while recovery procedure is performed. Nevertheless,
>> TIOCMGET and TIOCMSET ioctls are still able to reach the device even
>> when it is suspended, which might cause EEH recovery of serial adapters
>> to fail on Power systems. This patch blocks any TIOCMGET/TIOCMSET calls
>> from reaching the device if the port is suspended, to avoid racing with
>> the recovery procedure.
>
> This really should be handled by the 8250_pci sub-driver ticking
> TTY_IO_ERROR so that almost all tty operations fail (a minor bug fix
> to properly order the clearing of ASYNC_INITIALIZED w/ setting TTY_IO_ERROR
> in uart_shutdown() would be req'd).
>
> Unfortunately the machinery to do that doesn't exist.
> I could trivially add a means given a uart_port ptr but the difficulty
> of _safely_ getting a uart_port from line index still remains;
> serial8250_get_port() should have just added a full ref count mechanism.
Re-checking the 8250_pci sub-driver, I see it goes right through
serial8250_suspend_port() which assumes the 8250 port will not be unregistered
concurrently, so I assume the PCI subsystem serializes i/o error handling
with device removal, in which case the translation from
line => uart_8250_port => uart_port is straightforward.
Below is the serial core machinery that can be used to tick TTY_IO_ERROR.
Regards,
Peter Hurley
--- >% ---
Subject: [PATCH] serial: core: Add notification interface for i/o errors
Add uart_handle_io_error() for notifying tty core the device has
an error which prevents i/o, or the error has cleared and i/o can
resume.
Care is taken to preserve the tty error state if the device error
occurs or is cleared during port startup
Signed-off-by: Peter Hurley <peter@xxxxxxxxxxxxxxxxxx>
---
drivers/tty/serial/serial_core.c | 40 ++++++++++++++++++++++++++++++++++++++--
include/linux/serial_core.h | 3 +++
2 files changed, 41 insertions(+), 2 deletions(-)
diff --git a/drivers/tty/serial/serial_core.c b/drivers/tty/serial/serial_core.c
index 418587f..34bcca0 100644
--- a/drivers/tty/serial/serial_core.c
+++ b/drivers/tty/serial/serial_core.c
@@ -195,6 +195,7 @@ static int uart_port_startup(struct tty_struct *tty, struct uart_state *state,
static int uart_startup(struct tty_struct *tty, struct uart_state *state,
int init_hw)
{
+ struct uart_port *uport = state->uart_port;
struct tty_port *port = &state->port;
int retval;
@@ -209,8 +210,11 @@ static int uart_startup(struct tty_struct *tty, struct uart_state *state,
retval = uart_port_startup(tty, state, init_hw);
if (!retval) {
+ spin_lock_irq(&uport->lock);
set_bit(ASYNCB_INITIALIZED, &port->flags);
- clear_bit(TTY_IO_ERROR, &tty->flags);
+ if (~uport->status & UPSTAT_IO_ERROR)
+ clear_bit(TTY_IO_ERROR, &tty->flags);
+ spin_unlock_irq(&uport->lock);
} else if (retval > 0)
retval = 0;
@@ -226,6 +230,9 @@ static void uart_shutdown(struct tty_struct *tty, struct uart_state *state)
{
struct uart_port *uport = state->uart_port;
struct tty_port *port = &state->port;
+ int initialized;
+
+ initialized = test_and_clear_bit(ASYNCB_INITIALIZED, &port->flags);
/*
* Set the TTY IO error marker
@@ -233,7 +240,7 @@ static void uart_shutdown(struct tty_struct *tty, struct uart_state *state)
if (tty)
set_bit(TTY_IO_ERROR, &tty->flags);
- if (test_and_clear_bit(ASYNCB_INITIALIZED, &port->flags)) {
+ if (initialized) {
/*
* Turn off DTR and RTS early.
*/
@@ -2839,6 +2846,35 @@ int uart_match_port(struct uart_port *port1, struct uart_port *port2)
EXPORT_SYMBOL(uart_match_port);
/**
+ * uart_handle_io_error - report device io error status change
+ * @uport: uart_port structure for the open port
+ * @status: io error status, nonzero if error
+ *
+ * Caller must hold uport->lock
+ */
+
+void uart_handle_io_error(struct uart_port *uport, unsigned int status)
+{
+ struct tty_port *port = &uport->state->port;
+ struct tty_struct *tty = port->tty;
+
+ lockdep_assert_held_once(&uport->lock);
+
+ if (status)
+ uport->status |= UPSTAT_IO_ERROR;
+ else
+ uport->status &= ~UPSTAT_IO_ERROR;
+
+ if (tty) {
+ if (status)
+ set_bit(TTY_IO_ERROR, &tty->flags);
+ else if (port->flags & ASYNC_INITIALIZED)
+ clear_bit(TTY_IO_ERROR, &tty->flags);
+ }
+}
+EXPORT_SYMBOL_GPL(uart_handle_io_error);
+
+/**
* uart_handle_dcd_change - handle a change of carrier detect state
* @uport: uart_port structure for the open port
* @status: new carrier detect status, nonzero if active
diff --git a/include/linux/serial_core.h b/include/linux/serial_core.h
index 297d4fa..1969ca2 100644
--- a/include/linux/serial_core.h
+++ b/include/linux/serial_core.h
@@ -227,6 +227,7 @@ struct uart_port {
#define UPSTAT_AUTORTS ((__force upstat_t) (1 << 2))
#define UPSTAT_AUTOCTS ((__force upstat_t) (1 << 3))
#define UPSTAT_AUTOXOFF ((__force upstat_t) (1 << 4))
+#define UPSTAT_IO_ERROR ((__force upstat_t) (1 << 5))
int hw_stopped; /* sw-assisted CTS flow state */
unsigned int mctrl; /* current modem ctrl settings */
@@ -418,6 +419,8 @@ static inline bool uart_softcts_mode(struct uart_port *uport)
* The following are helper functions for the low level drivers.
*/
+extern void uart_handle_io_error(struct uart_port *uport,
+ unsigned int status);
extern void uart_handle_dcd_change(struct uart_port *uport,
unsigned int status);
extern void uart_handle_cts_change(struct uart_port *uport,
--
2.6.3
--
To unsubscribe from this list: send the line "unsubscribe linux-serial" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
