linux-kernel - Re: [PATCH 3/3] 8250: add workaround for MPC8[356]xx UART break IRQ storm

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]

Date:	Thu, 1 Dec 2011 20:42:13 -0500
From:	Paul Gortmaker <paul.gortmaker@...driver.com>
To:	Alan Cox <alan@...ux.intel.com>
Cc:	gregkh@...e.de, galak@...nel.crashing.org, scottwood@...escale.com,
	linux-serial@...r.kernel.org, linuxppc-dev@...ts.ozlabs.org,
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH 3/3] 8250: add workaround for MPC8[356]xx UART break IRQ storm

On Thu, Dec 1, 2011 at 7:57 PM, Alan Cox <alan@...ux.intel.com> wrote:
>
>> @@ -1553,7 +1554,15 @@ static void serial8250_handle_port(struct
>> uart_8250_port *up)
>>       spin_lock_irqsave(&up->port.lock, flags);
>>
>> -     status = serial_inp(up, UART_LSR);
>> +     /* Workaround for IRQ storm errata on break with Freescale
>> 16550 */
>> +     if (UART_BUG_FSLBK & up->port.bugs && up->lsr_last &
>> UART_LSR_BI) {
>> +             up->lsr_last &= ~UART_LSR_BI;
>> +             serial_inp(up, UART_RX);
>> +             spin_unlock_irqrestore(&up->port.lock, flags);
>> +             return;
>> +     }
>> +
>> +     status = up->lsr_last = serial_inp(up, UART_LSR);
>
> We've now had a recent pile of IRQ function changes adding more quirk
> bits and special casing. This doesn't scale. We either need to make
> handle_port a method the specific drivers can override or you could
> hide the mess in your serial_inp implementation and not touch any core
> code.

To be fair, this one is zero cost for !PPC, but I understand your point,
and the idea of hiding it somehow in serial_inp was something that
never crossed my mind.  I'll look into seeing if I can abuse that.

>
> I really don't mind which but I suspect dealing with it in your
> serial_inp handler so that when you read UART_LSR you do the fixup
> might be simplest providing you can just do that.
>
> Sorting out a ->handle_port override is probably ultimately the right
> thing to do and then we can push some of the other funnies out further.
>
> At this point it's becoming increasingly clear that 8250 UART cloners
> are both very bad at cloning the things accurately, and very busy adding
> extra useful features so we need to start to treat 8250.c as a library
> you can wrap.

Sad but true.  It is like a time warp back to lib8390.c  --- whee.

P.

>
> Alan
> --
> To unsubscribe from this list: send the line "unsubscribe linux-serial" in
> the body of a message to majordomo@...r.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/