Skip to content

Commit 90b8596

Browse files
ij-intelgregkh
authored andcommitted
serial: 8250: Prevent starting up DMA Rx on THRI interrupt
Hans de Goede reported Bluetooth adapters (HCIs) connected over an UART connection failed due corrupted Rx payload. The problem was narrowed down to DMA Rx starting on UART_IIR_THRI interrupt. The problem occurs despite LSR having DR bit set, which is precondition for attempting to start DMA Rx in the first place. From a debug patch: [x.807834] 8250irq: iir=cc lsr+saved=60 received=0/15 ier=0f dma_t/rx/err=0/0/0 [x.808676] 8250irq: iir=c2 lsr+saved=61 received=0/0 ier=0f dma_t/rx/err=0/0/0 [x.808776] 8250irq: iir=cc lsr+saved=60 received=1/12 ier=0d dma_t/rx/err=0/1/0 [x.808870] Bluetooth: hci0: Frame reassembly failed (-84) In the debug snippet, received field indicates 1 byte was transferred over DMA and 12 bytes after that with the non-DMA Rx. The sole byte DMA handled was corrupted (gets zeroed) which leads to the HCI failure. This problem became apparent after commit e8ffbb7 ("serial: 8250: use THRE & __stop_tx also with DMA") changed Tx stop behavior. Tx stop is now triggered from a THRI interrupt. Despite that this problem looks like a HW bug, this fix is not adding UART_BUG_xx flag to the driver beucase it seems useful in general to avoid starting DMA when there are only a few bytes to transfer. Skipping DMA for small transfers avoids the extra overhead DMA incurs. Thus, don't setup DMA Rx on UART_IIR_THRI but leave it to a subsequent interrupt which has Rx a related IIR value. By returning false from handle_rx_dma(), the DMA vs non-DMA decision is postponed until either UART_IIR_RDI (FIFO threshold worth of bytes awaiting) or UART_IIR_TIMEOUT (inter-character timeout) triggers at a later time which allows better to discern whether the number of bytes warrants starting DMA or not. Reported-by: Hans de Goede <hdegoede@redhat.com> Tested-by: Hans de Goede <hdegoede@redhat.com> Fixes: e8ffbb7 ("serial: 8250: use THRE & __stop_tx also with DMA") Cc: stable@vger.kernel.org Signed-off-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com> Acked-by: Hans de Goede <hdegoede@redhat.com> Link: https://lore.kernel.org/r/20230317103034.12881-1-ilpo.jarvinen@linux.intel.com Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
1 parent 7b21f32 commit 90b8596

File tree

1 file changed

+11
-0
lines changed

1 file changed

+11
-0
lines changed

drivers/tty/serial/8250/8250_port.c

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1903,6 +1903,17 @@ EXPORT_SYMBOL_GPL(serial8250_modem_status);
19031903
static bool handle_rx_dma(struct uart_8250_port *up, unsigned int iir)
19041904
{
19051905
switch (iir & 0x3f) {
1906+
case UART_IIR_THRI:
1907+
/*
1908+
* Postpone DMA or not decision to IIR_RDI or IIR_RX_TIMEOUT
1909+
* because it's impossible to do an informed decision about
1910+
* that with IIR_THRI.
1911+
*
1912+
* This also fixes one known DMA Rx corruption issue where
1913+
* DR is asserted but DMA Rx only gets a corrupted zero byte
1914+
* (too early DR?).
1915+
*/
1916+
return false;
19061917
case UART_IIR_RDI:
19071918
if (!up->dma->rx_running)
19081919
break;

0 commit comments

Comments
 (0)