Re: [PATCH v8 5/6] serial: 8250: retire canceled RX DMA before reusing the receive buffer

[James Hilliard]

On Thu, Oct 29, 2026 at 4:27 PM Raul E Rangel <rrangel@xxxxxxxxxxxx> wrote:
>
> Hi James,
>
> Thanks for the v8 updates and for opening PR #1 on the TLA+ repo! Confirming
> that the v7 issues in Patch 2/6 (lantiq, dz, zs, jsm_cls), Patch 4/6
> (__dma_tx_complete tx_running check), and 8250_omap.c shutdown locking all
> look resolved in v8.
>
> On Patch 5/6, we updated our TLA+ model for the new rx_pause_pending worker
> retry loop:
>
> > +     ret = dmaengine_pause(dma->rxchan);
> > +     status = dmaengine_tx_status(dma->rxchan, dma->rx_cookie, &state);
> > +     if (status == DMA_ERROR) {
> > +             ret = -EIO;
> > +             goto recover;
> > +     }
> > +     if (ret && status != DMA_COMPLETE) {
> > +             dma->rx_pause_pending = true;
> > +             serial8250_out_IER(p, p->ier);
> > +             return ret;
> > +     }
> ...
> > +     for (;;) {
> > +             scoped_guard(uart_port_lock_irqsave, &p->port) {
> > +                     if (dma->shutting_down)
> > +                             return;
> > +                     if (dma->rx_pause_pending && !dma->rx_terminating)
> > +                             serial8250_rx_dma_try_flush(p);
> > +                     retry = dma->rx_pause_pending && !dma->rx_terminating;
> > +                     recover = dma->rx_err;
> > +             }
> > +             if (!retry)
> > +                     break;
> > +             usleep_range(1000, 2000);
> > +     }
>
> This cleanly resolves the race where a full 4 KB transfer completed in
> hardware just before dmaengine_pause() ran while its DMA completion hardirq
> was still pending (once serial8250_rx_dma_flush_work() drops port->lock and
> sleeps, the pending DMA hardirq runs and marks rx_cookie as DMA_COMPLETE).
>
> However, what happens if dmaengine_pause() fails on a *partial* RX transfer
> (0 < count < 4096 bytes) during a normal UART_IIR_RX_TIMEOUT where the remote
> peer stopped sending and is waiting for a reply?

With a persistent pause failure and an incomplete transfer, the
worker keeps retrying. RX remains masked and the runtime PM reference
is held until the condition resolves or shutdown takes over. There
is no receive-progress guarantee if the peer remains idle and pause
continues to fail.

> (In PR #1's Serial8250RxLifetime.tla, Spec puts WF_vars(ProviderProgress) on
> Receive at lines 67-70, which forces the remote peer to always keep
> transmitting until count == Capacity and CompleteInHw fires.)

That fairness assumption cannot establish progress for an idle peer.
The broader local RX lifecycle model includes permanent partial-
transfer pause failure, but checks ownership safety and close
progress, not eventual reception. Passing those checks does not
establish recovery from a permanently failing pause operation.

>
> When the peer is idle after sending a partial burst (< 4096 bytes):
> 1. Hardware never reaches 4096 bytes, so status remains DMA_IN_PROGRESS
>    (neither DMA_COMPLETE nor DMA_ERROR).
> 2. If dmaengine_pause(dma->rxchan) persistently returns an error (-ENXIO,
>    -EBUSY, -EAGAIN, -EINVAL, etc.) on that partial transfer,
>    serial8250_rx_dma_flush_work() loops forever in `for (;;)` calling
>    usleep_range(1000, 2000):
>    - It holds a runtime PM reference (`guard(serial8250_rpm)(p)`) and wakes a
>      system_percpu_wq worker 500-1000 times per second until the port is closed.
>    - RX interrupts remain masked in hardware (`rx_pause_pending == true`), the
>      partial bytes are never flushed, and the port never falls back to PIO.
>    - On 8250_dw, dw8250_idle_enter() gets -EAGAIN from serial8250_rx_dma_flush()
>      while rx_pause_pending is true, causing dw8250_set_divisor() and
>      dw8250_check_lcr() to silently abort baud-rate and LCR updates.
>
> Since the only reason to retry pause/status is to let an already-pending DMA
> completion hardirq run after port->lock (and local IRQs) are released, could
> we bound the retry loop in serial8250_rx_dma_flush_work() to a small number of
> iterations (e.g. 5 retries)?

A failed pause with an incomplete cookie does not distinguish a
persistent hardware fault from completion that has not yet been
published. A fixed retry limit does not resolve that ambiguity.

Terminating after the limit may discard received data, including a
completed buffer whose completion is still unpublished. Successful
termination would make subsequent buffer reuse safe, but would not
recover those bytes. This would therefore be a potentially lossy
error-recovery policy rather than a lossless timeout flush.

> If rx_pause_pending is still unresolved after the
> retry budget expires, calling serial8250_rx_dma_recover(p) under port->lock
> will set `dma->rx_err = true` and `dma->rx_terminating = true`, letting the
> worker exit the retry loop, run dmaengine_terminate_sync(dma->rxchan), and
> switch cleanly to PIO.

That requires termination to succeed and callbacks to be drained.
If termination fails, the channel may still access the FIFO, so PIO
must remain excluded.

The recovery requirements depend on why the provider cannot pause.

>
> We pushed the updated v8 TLA+ models (`Serial8250RxDmaV8.tla`,
> `Serial8250TxDmaV8.tla`, and `./run_v8.sh`) to:
> https://github.com/ismell/linux-8250-dma-tla
>
> Thanks,
> Raul