linux-kernel - Re: UART/TTY console deadlock

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <877dvg6ft6.fsf@kurt>
Date:   Mon, 06 Jul 2020 13:31:01 +0200
From:   Kurt Kanzenbach <kurt@...utronix.de>
To:     Andy Shevchenko <andy.shevchenko@...il.com>,
        Sergey Senozhatsky <sergey.senozhatsky@...il.com>
Cc:     Tony Lindgren <tony@...mide.com>, Petr Mladek <pmladek@...e.com>,
        Raul Rangel <rrangel@...gle.com>,
        Sergey Senozhatsky <sergey.senozhatsky.work@...il.com>,
        linux-kernel <linux-kernel@...r.kernel.org>,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        Andy Shevchenko <andriy.shevchenko@...ux.intel.com>,
        "S\, Shirish" <Shirish.S@....com>,
        Peter Zijlstra <peterz@...radead.org>,
        John Ogness <john.ogness@...utronix.de>,
        Steven Rostedt <rostedt@...dmis.org>
Subject: Re: UART/TTY console deadlock

Hi,

On Sat Jul 04 2020, Andy Shevchenko wrote:
> On Fri, Jul 3, 2020 at 1:32 PM Sergey Senozhatsky
> <sergey.senozhatsky@...il.com> wrote:
>>
>> On (20/07/02 09:05), Tony Lindgren wrote:
>> > * Sergey Senozhatsky <sergey.senozhatsky@...il.com> [200702 05:13]:
>> > > On (20/06/30 11:02), Tony Lindgren wrote:
>> > > > This conditional disable for irq_shared does not look nice to me
>> > > > from the other device point of view :)
>> > > >
>> > > > Would it be possible to just set up te dummy interrupt handler
>> > > > for the startup, then change it back afterwards? See for example
>> > > > omap8250_no_handle_irq().
>> > >
>> > > I think we can do it. serial8250_do_startup() and irq handler take
>> > > port->lock, so they should be synchronized.
>> > >
>> > > Something like this then?
>> >
>> > Yeah thanks this should work better for shared irq.
>>
>> This is, basically, an equivalent of
>>
>> ---
>>  drivers/tty/serial/8250/8250_port.c | 5 +----
>>  1 file changed, 1 insertion(+), 4 deletions(-)
>>
>> diff --git a/drivers/tty/serial/8250/8250_port.c b/drivers/tty/serial/8250/8250_port.c
>> index d64ca77d9cfa..dba7747d2ddd 100644
>> --- a/drivers/tty/serial/8250/8250_port.c
>> +++ b/drivers/tty/serial/8250/8250_port.c
>> @@ -2275,6 +2275,7 @@ int serial8250_do_startup(struct uart_port *port)
>>
>>         if (port->irq && !(up->port.flags & UPF_NO_THRE_TEST)) {
>>                 unsigned char iir1;
>> +
>>                 /*
>>                  * Test for UARTs that do not reassert THRE when the
>>                  * transmitter is idle and the interrupt has already
>> @@ -2284,8 +2285,6 @@ int serial8250_do_startup(struct uart_port *port)
>>                  * allow register changes to become visible.
>>                  */
>>                 spin_lock_irqsave(&port->lock, flags);
>> -               if (up->port.irqflags & IRQF_SHARED)
>> -                       disable_irq_nosync(port->irq);
>>
>>                 wait_for_xmitr(up, UART_LSR_THRE);
>>                 serial_port_out_sync(port, UART_IER, UART_IER_THRI);
>> @@ -2297,8 +2296,6 @@ int serial8250_do_startup(struct uart_port *port)
>>                 iir = serial_port_in(port, UART_IIR);
>>                 serial_port_out(port, UART_IER, 0);
>>
>> -               if (port->irqflags & IRQF_SHARED)
>> -                       enable_irq(port->irq);
>>                 spin_unlock_irqrestore(&port->lock, flags);
>>
>>                 /*
>
> ...which effectively is a revert of
>
> 768aec0b5bcc ("serial: 8250: fix shared interrupts issues with SMP and
> RT kernels")

Please, don't revert that commit. I've faced the same issue as described
in the commit log. There is hardware available with shared UART
interrupt lines.

Thanks,
Kurt

Download attachment "signature.asc" of type "application/pgp-signature" (833 bytes)