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

[Zhang Zheng <zhangzheng@xxxxxxxxxxx>]

On Thu, Oct 15, 2026 at 09:54:46AM +0200, Oleksii Kurochko wrote:
> 
> 
> On 10/15/26 8:44 AM, Zhang Zheng wrote:
> > On Mon, Oct 12, 2026 at 12:20:50PM +0200, Oleksii Kurochko wrote:
> > > > --- a/xen/drivers/char/ns16550.c
> > > > +++ b/xen/drivers/char/ns16550.c
> 
> [...]
> 
> > > > +    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
> > > The unbounded wait can hang early Xen boot if the UART never reports TEMT
> > > for some reason(?). Bounding the wait would make console initialization 
> > > more
> > > robust, allowing boot to continue when the UART is unavailable
> > > 
> > >      for ( i = 0; i < 50000; i++ )
> > >      {
> > >          if ( ns_read_reg(uart, UART_LSR) & UART_LSR_TEMT )
> > >              break;
> > >          udelay(10);
> > >      }
> > > 
> > > would need udelay() on RISC-V, which is still a BUG_ON() stub, so a
> > > separate patch would be needed for that. On x86, udelay() doesn't delay
> > > yet at this point (cpu_khz is only set by early_time_init()), so there
> > > the loop would just be an iteration count. Perhaps someone has a better
> > > idea of how to implement a timeout here.
> > > 
> > > ~ Oleksii
> > > 
> > Leaving aside the unavailability of udelay() at this point, is a timeout
> > considered necessary here?
> 
> In the way Jan B. mentioned here: 
> https://lore.kernel.org/xen-devel/20261010031406.2057735-1-zhangzheng@xxxxxxxxxxx/T/#meaf24c3bb039dc6aaecffab752d2b107581816e0
> 
> I think we still can have timeout check.
> 
> ~ Oleksii

I saw it afterwards. Thanks.

Zhang