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

[Jan Beulich <jbeulich@xxxxxxxx>]

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