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

[Zhang Zheng <zhangzheng@xxxxxxxxxxx>]

On Mon, Oct 12, 2026 at 02:56:56PM +0200, Jan Beulich wrote:
> On 12.10.2026 12:20, Oleksii Kurochko wrote:
> > On 10/10/26 5:14 AM, Zhang Zheng wrote:
> >> --- a/xen/drivers/char/ns16550.c
> >> +++ b/xen/drivers/char/ns16550.c
> >> @@ -427,6 +427,10 @@ static void __init cf_check 
> >> ns16550_init_preirq(struct serial_port *port)
> >>   #endif
> >>       }
> >>   
> >> +    /* Preserve output queued by firmware before taking over the UART. */
> > я'
> > 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.
> >       */
> > 
> >> +    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.)
> > 
> > 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.
> 
> Whatever it is, it's going to be more or less crude. See e.g. Marek's
> solution for a similar issue (yet to be committed):
> https://lists.xen.org/archives/html/xen-devel/2026-10/msg00439.html
> 
> Jan

Thanks for the pointer. I will follow Marek's approach for the bounded
wait.

Zhang