Re: [PATCH for-4.22] char/ns16550: bound execution time of ns16550_interrupt()

["Roger Pau Monné" <roger.pau@citrix.com>]

On Tue, Jun 23, 2026 at 03:46:11PM +0200, Jan Beulich wrote:
> On 23.06.2026 15:36, Oleksii Kurochko wrote:
> > On 6/23/26 12:31 PM, Roger Pau Monne wrote:
> >> The current logic in ns16550_interrupt() will loop until the device sets
> >> the NOINT in IIR.  At least on the Lenovo ThinkSystem SR630 V4 the flow
> >> control of the serial-over-lan emulated UART seems to be broken, as it
> >> doesn't set the NOINT bit consistently.  The Transmitter Holding Register
> >> Empty in LSR also seems to not be properly signaled, as even with it set
> >> writes to the transmit register take ~6ms.  This leads to the watchdog
> >> triggering very easily on such system.
> >>
> >> Introduce an upper bound on the execution time of ns16550_interrupt(), this
> >> is currently set as 4x the polling interval, which is calculated as the
> >> time to fill RX FIFO and/or empty TX FIFO.  The current maximum is 5ms.
> >> Once the timeout triggers the interrupt is disabled and the uart is
> >> switched to polling mode.
> > 
> > Don't you mmiss Fixes: tag?
> 
> Fixes: "SoL on Lenovo ThinkSystem SR630 V4"
> 
> you mean? I think there's nothing wrong with our pre-existing code, and
> the changes here instead are a workaround for some (apparently) badly
> implemented SoL.

It was on purpose that no Fixes tag was provided.

Xen code would be fine with well-behaved uarts, however most of the
serial-over-lan emulated ones are not well behaved it seems.

There's a possible issue with the unbounded loop in
ns16550_interrupt() as it's relying solely on hardware register values
to terminate, which again would be OK if hardware was correctly
implemented.  I don't think this warrants a Fixes tag.

Thanks, Roger.