lkml.org 
[lkml]   [2022]   [Oct]   [19]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    From
    Subject[PATCH 6.0 558/862] serial: 8250: Toggle IER bits on only after irq has been set up
    Date
    From: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>

    [ Upstream commit 039d4926379b1d1c17b51cf21c500a5eed86899e ]

    Invoking TIOCVHANGUP on 8250_mid port on Ice Lake-D and then reopening
    the port triggers these faults during serial8250_do_startup():

    DMAR: DRHD: handling fault status reg 3
    DMAR: [DMA Write NO_PASID] Request device [00:1a.0] fault addr 0x0 [fault reason 0x05] PTE Write access is not set

    If the IRQ hasn't been set up yet, the UART will have zeroes in its MSI
    address/data registers. Disabling the IRQ at the interrupt controller
    won't stop the UART from performing a DMA write to the address programmed
    in its MSI address register (zero) when it wants to signal an interrupt.

    The UARTs (in Ice Lake-D) implement PCI 2.1 style MSI without masking
    capability, so there is no way to mask the interrupt at the source PCI
    function level, except disabling the MSI capability entirely, but that
    would cause it to fall back to INTx# assertion, and the PCI specification
    prohibits disabling the MSI capability as a way to mask a function's
    interrupt service request.

    The MSI address register is zeroed by the hangup as the irq is freed.
    The interrupt is signalled during serial8250_do_startup() performing a
    THRE test that temporarily toggles THRI in IER. The THRE test currently
    occurs before UART's irq (and MSI address) is properly set up.

    Refactor serial8250_do_startup() such that irq is set up before the
    THRE test. The current irq setup code is intermixed with the timer
    setup code. As THRE test must be performed prior to the timer setup,
    extract it into own function and call it only after the THRE test.

    The ->setup_timer() needs to be part of the struct uart_8250_ops in
    order to not create circular dependency between 8250 and 8250_base
    modules.

    Fixes: 40b36daad0ac ("[PATCH] 8250 UART backup timer")
    Reported-by: Lennert Buytenhek <buytenh@arista.com>
    Tested-by: Lennert Buytenhek <buytenh@arista.com>
    Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
    Signed-off-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
    Link: https://lore.kernel.org/r/20220922070005.2965-1-ilpo.jarvinen@linux.intel.com
    Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
    Signed-off-by: Sasha Levin <sashal@kernel.org>
    ---
    drivers/tty/serial/8250/8250_core.c | 16 +++++++++++-----
    drivers/tty/serial/8250/8250_port.c | 8 +++++---
    include/linux/serial_8250.h | 1 +
    3 files changed, 17 insertions(+), 8 deletions(-)

    diff --git a/drivers/tty/serial/8250/8250_core.c b/drivers/tty/serial/8250/8250_core.c
    index 2e83e7367441..94fbf0add2ce 100644
    --- a/drivers/tty/serial/8250/8250_core.c
    +++ b/drivers/tty/serial/8250/8250_core.c
    @@ -298,10 +298,9 @@ static void serial8250_backup_timeout(struct timer_list *t)
    jiffies + uart_poll_timeout(&up->port) + HZ / 5);
    }

    -static int univ8250_setup_irq(struct uart_8250_port *up)
    +static void univ8250_setup_timer(struct uart_8250_port *up)
    {
    struct uart_port *port = &up->port;
    - int retval = 0;

    /*
    * The above check will only give an accurate result the first time
    @@ -322,10 +321,16 @@ static int univ8250_setup_irq(struct uart_8250_port *up)
    */
    if (!port->irq)
    mod_timer(&up->timer, jiffies + uart_poll_timeout(port));
    - else
    - retval = serial_link_irq_chain(up);
    +}

    - return retval;
    +static int univ8250_setup_irq(struct uart_8250_port *up)
    +{
    + struct uart_port *port = &up->port;
    +
    + if (port->irq)
    + return serial_link_irq_chain(up);
    +
    + return 0;
    }

    static void univ8250_release_irq(struct uart_8250_port *up)
    @@ -381,6 +386,7 @@ static struct uart_ops univ8250_port_ops;
    static const struct uart_8250_ops univ8250_driver_ops = {
    .setup_irq = univ8250_setup_irq,
    .release_irq = univ8250_release_irq,
    + .setup_timer = univ8250_setup_timer,
    };

    static struct uart_8250_port serial8250_ports[UART_NR];
    diff --git a/drivers/tty/serial/8250/8250_port.c b/drivers/tty/serial/8250/8250_port.c
    index 6a9d3c8ffa56..ec7dca43619f 100644
    --- a/drivers/tty/serial/8250/8250_port.c
    +++ b/drivers/tty/serial/8250/8250_port.c
    @@ -2300,6 +2300,10 @@ int serial8250_do_startup(struct uart_port *port)
    if (port->irq && (up->port.flags & UPF_SHARE_IRQ))
    up->port.irqflags |= IRQF_SHARED;

    + retval = up->ops->setup_irq(up);
    + if (retval)
    + goto out;
    +
    if (port->irq && !(up->port.flags & UPF_NO_THRE_TEST)) {
    unsigned char iir1;

    @@ -2342,9 +2346,7 @@ int serial8250_do_startup(struct uart_port *port)
    }
    }

    - retval = up->ops->setup_irq(up);
    - if (retval)
    - goto out;
    + up->ops->setup_timer(up);

    /*
    * Now, initialize the UART
    diff --git a/include/linux/serial_8250.h b/include/linux/serial_8250.h
    index 8c7b793aa4d7..16e3d75a324c 100644
    --- a/include/linux/serial_8250.h
    +++ b/include/linux/serial_8250.h
    @@ -74,6 +74,7 @@ struct uart_8250_port;
    struct uart_8250_ops {
    int (*setup_irq)(struct uart_8250_port *);
    void (*release_irq)(struct uart_8250_port *);
    + void (*setup_timer)(struct uart_8250_port *);
    };

    struct uart_8250_em485 {
    --
    2.35.1


    \
     
     \ /
      Last update: 2022-10-19 13:58    [W:2.426 / U:1.888 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site