Re: [PATCH v2 2/2] xen/ns16550: drain TX before reinitializing

[Zhang Zheng <zhangzheng@iscas.ac.cn>]

On Mon, Oct 12, 2026 at 12:20:50PM +0200, Oleksii Kurochko wrote:
> 
> 
> On 10/10/26 5:14 AM, Zhang Zheng wrote:
> > An SBI console call may return before the UART finishes transmitting.
> > Wait for UART_LSR_TEMT before ns16550 reprograms the UART and clears its
> > FIFO, preserving pending early-console output and legacy SBI v0.1 support.
> 
> The problem isn't really specific to SBI: anything that wrote to the UART
> before ns16550 takes over (firmware, a bootloader, or Xen's own early
> console) can leave data in the TX FIFO or the shift register. I'd
> describe it in those terms.
Agreed.
> Could the comment say why the wait is needed at this point? Something
> like:
> 
>     /*
>      * Output written before ns16550 takes over (e.g. by firmware or the
>      * early console) may still be in the transmitter. Let it drain before
>      * ns16550_setup_preirq() reprograms the line settings and resets the
>      * FIFOs.
>      */
> 
Thanks, I will use it in next version.
> > +    while ( !(ns_read_reg(uart, UART_LSR) & UART_LSR_TEMT) )
> > +        cpu_relax();
> > +
> 
> IMO waiting for TEMT rather than THRE is the right choice: besides
> resetting the FIFOs, ns16550_setup_preirq() reprograms the divisor and
> LCR, which would corrupt a character still being shifted out. (Linux
> waits for UART_LSR_BOTH_EMPTY, i.e. THRE | TEMT, but on a 16550 TEMT
> already implies THRE.)
> 
Yes, I followed the Linux implementation, which waits in an unbounded loop
and theck UART_LSR_BOTH_EMPTY like you said:

for (;;) {
		status = serial8250_early_in(port, UART_LSR);
		if (uart_lsr_tx_empty(status)) /* checks UART_LSR_BOTH_EMPTY */
				break;
		cpu_relax();
}

So I didn't have timeout handling

Zhang