[PATCH v2 0/2] xen/ns16550: preserve UART operation across firmware handover

[PATCH v2 0/2] xen/ns16550: preserve UART operation across firmware handover

[Zhang Zheng]

V1 replaced the RISC-V SBI early console with direct 8250 MMIO access.
Following review, v2 keeps the generic SBI console and instead fixes the
handover in ns16550.

K3 testing exposed two issues during this handover: clearing IER disables
its XScale-compatible UART, and resetting the FIFO can discard output
still pending after an SBI console call.  I originally planned to submit
the XScale support separately, but both fixes are needed for the same
handover, so they are included here as two patches.

Tested with QEMU 11.0.0/OpenSBI 1.7 and on SpacemiT K3 hardware.

Changes in v2:
  - Keep the SBI early console and legacy SBI v0.1 support.
  - Preserve UART_IER_UUE and drain TX before ns16550 reinitialization.

v1: https://lore.kernel.org/xen-devel/20260928023857.1030884-1-zhangzheng@iscas.ac.cn/

Zhang Zheng (2):
  xen/ns16550: detect and preserve UART_IER_UUE
  xen/ns16550: drain TX before reinitializing

 xen/drivers/char/ns16550.c  | 16 +++++++++++++++-
 xen/include/xen/8250-uart.h |  1 +
 2 files changed, 16 insertions(+), 1 deletion(-)


base-commit: 0d3bb3c98bc99ec1c84ebb3df71b6e18daebff4f
-- 
2.54.0

[PATCH v2 1/2] xen/ns16550: detect and preserve UART_IER_UUE

[Zhang Zheng]

XScale-compatible UARTs require UART_IER_UUE to remain set.  Probe the
bit during setup and preserve it when enabling receive interrupts, so
the xscale/PXA UART remains operational after ns16550 takes over.

Signed-off-by: Zhang Zheng <zhangzheng@iscas.ac.cn>
---
 xen/drivers/char/ns16550.c  | 12 +++++++++++-
 xen/include/xen/8250-uart.h |  1 +
 2 files changed, 12 insertions(+), 1 deletion(-)

diff --git a/xen/drivers/char/ns16550.c b/xen/drivers/char/ns16550.c
index cdda28879e..c174465059 100644
--- a/xen/drivers/char/ns16550.c
+++ b/xen/drivers/char/ns16550.c
@@ -64,6 +64,7 @@ static struct ns16550 {
     bool intr_works;
     bool force_polling;
     bool dw_usr_bsy;
+    bool xscale;
 #ifdef NS16550_PCI
     /* PCI card parameters. */
     bool pb_bdf_enable;     /* if =1, pb-bdf effective, port behind bridge */
@@ -352,6 +353,14 @@ static void ns16550_setup_preirq(struct ns16550 *uart)
     /* No interrupts. */
     ns_write_reg(uart, UART_IER, 0);
 
+    uart->xscale = false;
+    if ( !(ns_read_reg(uart, UART_IER) & UART_IER_UUE) )
+    {
+        ns_write_reg(uart, UART_IER, UART_IER_UUE);
+        if ( ns_read_reg(uart, UART_IER) & UART_IER_UUE )
+            uart->xscale = true;
+    }
+
     /* Handle the DesignWare 8250 'busy-detect' quirk. */
     handle_dw_usr_busy_quirk(uart);
 
@@ -446,7 +455,8 @@ static void ns16550_setup_postirq(struct ns16550 *uart)
                      UART_MCR, UART_MCR_OUT2 | UART_MCR_DTR | UART_MCR_RTS);
 
         /* Enable receive interrupts. */
-        ns_write_reg(uart, UART_IER, UART_IER_ERDAI);
+        ns_write_reg(uart, UART_IER,
+                     UART_IER_ERDAI | (uart->xscale ? UART_IER_UUE : 0));
     }
 
     if ( uart->irq >= 0 )
diff --git a/xen/include/xen/8250-uart.h b/xen/include/xen/8250-uart.h
index d13352940c..358d264698 100644
--- a/xen/include/xen/8250-uart.h
+++ b/xen/include/xen/8250-uart.h
@@ -42,6 +42,7 @@
 #define UART_IER_ETHREI   0x02    /* tx reg. empty        */
 #define UART_IER_ELSI     0x04    /* rx line status       */
 #define UART_IER_EMSI     0x08    /* MODEM status         */
+#define UART_IER_UUE      0x40    /* UART unit enable (XScale/PXA) */
 
 /* Interrupt Identification Register */
 #define UART_IIR_NOINT    0x01    /* no interrupt pending */
-- 
2.54.0

Re: [PATCH v2 1/2] xen/ns16550: detect and preserve UART_IER_UUE

[Baptiste Le Duc]

> diff --git a/xen/drivers/char/ns16550.c b/xen/drivers/char/ns16550.c
> index 593d208483..953d166314 100644
> --- a/xen/drivers/char/ns16550.c
> +++ b/xen/drivers/char/ns16550.c
> @@ -352,6 +353,14 @@ static void ns16550_setup_preirq(struct ns16550 *uart)
>      /* No interrupts. */
>      ns_write_reg(uart, UART_IER, 0);
Do we know of a UART where UUE reads back as 1 whatever is written? I
could not find any such hardware, so I am not sure the check is needed
but it might be wrong. If not, then we could drop this write but if so,
then I propose to move it in a dedicated function:

        ```
        static bool ns_is_xscale(const struct ns16550 *uart)
        {
            /*
            * We're going to explicitly set the UUE bit to 0 before
            * trying to write and read a 1 just to make sure it's not
            * already a 1 and maybe locked there before we even start.
            */
            ns_write_reg(uart, UART_IER, 0);

            if ( !(ns_read_reg(uart, UART_IER) & UART_IER_UUE) ) {
                /* Now we are in a 0 state */
                ns_write_reg(uart, UART_IER, UART_IER_UUE);
                if ( ns_read_reg(uart, UART_IER) & UART_IER_UUE )
                    return true;
            }
            return false;
        }
        ```
> +    uart->xscale = false;
> +    if ( !(ns_read_reg(uart, UART_IER) & UART_IER_UUE) )


> +    {
> +        ns_write_reg(uart, UART_IER, UART_IER_UUE);
> +        if ( ns_read_reg(uart, UART_IER) & UART_IER_UUE )
> +            uart->xscale = true;
> +    }
> +
>      /* Handle the DesignWare 8250 'busy-detect' quirk. */
>      handle_dw_usr_busy_quirk(uart);
>  
> @@ -446,7 +455,8 @@ static void ns16550_setup_postirq(struct ns16550 *uart)
>                       UART_MCR, UART_MCR_OUT2 | UART_MCR_DTR | UART_MCR_RTS);
>  
>          /* Enable receive interrupts. */
> -        ns_write_reg(uart, UART_IER, UART_IER_ERDAI);
> +        ns_write_reg(uart, UART_IER,
> +                     UART_IER_ERDAI | (uart->xscale ? UART_IER_UUE : 0));
>
Every IER write has to keep UUE on an XScale UART, otherwise the UART is
switched off. Open-coding the ternary at each call site makes it easy
for a future caller to forget. Would it be better to add a helper, e.g.:

      static void ns_write_ier(const struct ns16550 *uart, unsigned int val)
      {
          ns_write_reg(uart, UART_IER,
                       uart->xscale ? val | UART_IER_UUE : val);
      }

and use it for all futures IER writes?

Thanks for contributing!

-- 
Baptiste Le Duc <baptiste.le-duc@vates.tech>

Re: [PATCH v2 1/2] xen/ns16550: detect and preserve UART_IER_UUE

[Oleksii Kurochko]

On 10/11/26 12:11 AM, Baptiste Le Duc wrote:
>> diff --git a/xen/drivers/char/ns16550.c b/xen/drivers/char/ns16550.c
>> index 593d208483..953d166314 100644
>> --- a/xen/drivers/char/ns16550.c
>> +++ b/xen/drivers/char/ns16550.c
>> @@ -352,6 +353,14 @@ static void ns16550_setup_preirq(struct ns16550 *uart)
>>       /* No interrupts. */
>>       ns_write_reg(uart, UART_IER, 0);
> Do we know of a UART where UUE reads back as 1 whatever is written? I
> could not find any such hardware, so I am not sure the check is needed
> but it might be wrong. If not, then we could drop this write but if so,
> then I propose to move it in a dedicated function:
> 
>          ```
>          static bool ns_is_xscale(const struct ns16550 *uart)
>          {
>              /*
>              * We're going to explicitly set the UUE bit to 0 before
>              * trying to write and read a 1 just to make sure it's not
>              * already a 1 and maybe locked there before we even start.
>              */
>              ns_write_reg(uart, UART_IER, 0);
> 
>              if ( !(ns_read_reg(uart, UART_IER) & UART_IER_UUE) ) {
>                  /* Now we are in a 0 state */
>                  ns_write_reg(uart, UART_IER, UART_IER_UUE);
>                  if ( ns_read_reg(uart, UART_IER) & UART_IER_UUE )
>                      return true;
>              }
>              return false;
>          }
>          ```
>> +    uart->xscale = false;
>> +    if ( !(ns_read_reg(uart, UART_IER) & UART_IER_UUE) )
> 
> 
>> +    {
>> +        ns_write_reg(uart, UART_IER, UART_IER_UUE);
>> +        if ( ns_read_reg(uart, UART_IER) & UART_IER_UUE )
>> +            uart->xscale = true;
>> +    }
>> +
>>       /* Handle the DesignWare 8250 'busy-detect' quirk. */
>>       handle_dw_usr_busy_quirk(uart);
>>   
>> @@ -446,7 +455,8 @@ static void ns16550_setup_postirq(struct ns16550 *uart)
>>                        UART_MCR, UART_MCR_OUT2 | UART_MCR_DTR | UART_MCR_RTS);
>>   
>>           /* Enable receive interrupts. */
>> -        ns_write_reg(uart, UART_IER, UART_IER_ERDAI);
>> +        ns_write_reg(uart, UART_IER,
>> +                     UART_IER_ERDAI | (uart->xscale ? UART_IER_UUE : 0));
>>
> Every IER write has to keep UUE on an XScale UART, otherwise the UART is
> switched off. Open-coding the ternary at each call site makes it easy
> for a future caller to forget. Would it be better to add a helper, e.g.:
> 
>        static void ns_write_ier(const struct ns16550 *uart, unsigned int val)
>        {
>            ns_write_reg(uart, UART_IER,
>                         uart->xscale ? val | UART_IER_UUE : val);
>        }

We could avoid ternary operator here:

static void ns_write_ier(const struct ns16550 *uart, unsigned int val)
{
     ns_write_reg(uart, UART_IER,
                  val | (UART_IER_UUE & -(unsigned int)uart->xscale));
}

but it is less readable...

~ Oleksii

> 
> and use it for all futures IER writes?
> 
> Thanks for contributing!
> 

Re: [PATCH v2 1/2] xen/ns16550: detect and preserve UART_IER_UUE

[Zhang Zheng]

On Sun, Oct 11, 2026 at 12:11:27AM +0200, Baptiste Le Duc wrote:
> > diff --git a/xen/drivers/char/ns16550.c b/xen/drivers/char/ns16550.c
> > index 593d208483..953d166314 100644
> > --- a/xen/drivers/char/ns16550.c
> > +++ b/xen/drivers/char/ns16550.c
> > @@ -352,6 +353,14 @@ static void ns16550_setup_preirq(struct ns16550 *uart)
> >      /* No interrupts. */
> >      ns_write_reg(uart, UART_IER, 0);
> Do we know of a UART where UUE reads back as 1 whatever is written? I
> could not find any such hardware, so I am not sure the check is needed
> but it might be wrong. If not, then we could drop this write but if so,
> then I propose to move it in a dedicated function:
> 
>         ```
>         static bool ns_is_xscale(const struct ns16550 *uart)
>         {
>             /*
>             * We're going to explicitly set the UUE bit to 0 before
>             * trying to write and read a 1 just to make sure it's not
>             * already a 1 and maybe locked there before we even start.
>             */
>             ns_write_reg(uart, UART_IER, 0);
> 
>             if ( !(ns_read_reg(uart, UART_IER) & UART_IER_UUE) ) {
>                 /* Now we are in a 0 state */
>                 ns_write_reg(uart, UART_IER, UART_IER_UUE);
>                 if ( ns_read_reg(uart, UART_IER) & UART_IER_UUE )
>                     return true;
>             }
>             return false;
>         }
>         ```
I agree with Oleksii's suggestion that the XScale quirk should be
derived from the Device Tree compatible for DT-described UARTs. Therefore,
I think will drop this register probe then.
> > +    uart->xscale = false;
> > +    if ( !(ns_read_reg(uart, UART_IER) & UART_IER_UUE) )
> 
> 
> > +    {
> > +        ns_write_reg(uart, UART_IER, UART_IER_UUE);
> > +        if ( ns_read_reg(uart, UART_IER) & UART_IER_UUE )
> > +            uart->xscale = true;
> > +    }
> > +
> >      /* Handle the DesignWare 8250 'busy-detect' quirk. */
> >      handle_dw_usr_busy_quirk(uart);
> >  
> > @@ -446,7 +455,8 @@ static void ns16550_setup_postirq(struct ns16550 *uart)
> >                       UART_MCR, UART_MCR_OUT2 | UART_MCR_DTR | UART_MCR_RTS);
> >  
> >          /* Enable receive interrupts. */
> > -        ns_write_reg(uart, UART_IER, UART_IER_ERDAI);
> > +        ns_write_reg(uart, UART_IER,
> > +                     UART_IER_ERDAI | (uart->xscale ? UART_IER_UUE : 0));
> >
> Every IER write has to keep UUE on an XScale UART, otherwise the UART is
> switched off. Open-coding the ternary at each call site makes it easy
> for a future caller to forget. Would it be better to add a helper, e.g.:
> 
>       static void ns_write_ier(const struct ns16550 *uart, unsigned int val)
>       {
>           ns_write_reg(uart, UART_IER,
>                        uart->xscale ? val | UART_IER_UUE : val);
>       }
> 
> and use it for all futures IER writes?
> 
> Thanks for contributing!
> 
> -- 
> Baptiste Le Duc <baptiste.le-duc@vates.tech>
Agreed. I will add an `ns_write_ier()` helper and use it for all IER
write paths, including `ns16550_setup_preirq()`,
`ns16550_setup_postirq()`, and `ns16550_interrupt()`.

Thanks for pointing this out.

Zhang

Re: [PATCH v2 1/2] xen/ns16550: detect and preserve UART_IER_UUE

[Oleksii Kurochko]

On 10/10/26 5:14 AM, Zhang Zheng wrote:
> XScale-compatible UARTs require UART_IER_UUE to remain set.  Probe the
> bit during setup and preserve it when enabling receive interrupts, so
> the xscale/PXA UART remains operational after ns16550 takes over.
> 
> Signed-off-by: Zhang Zheng <zhangzheng@iscas.ac.cn>
> ---
>   xen/drivers/char/ns16550.c  | 12 +++++++++++-
>   xen/include/xen/8250-uart.h |  1 +
>   2 files changed, 12 insertions(+), 1 deletion(-)
> 
> diff --git a/xen/drivers/char/ns16550.c b/xen/drivers/char/ns16550.c
> index cdda28879e..c174465059 100644
> --- a/xen/drivers/char/ns16550.c
> +++ b/xen/drivers/char/ns16550.c
> @@ -64,6 +64,7 @@ static struct ns16550 {
>       bool intr_works;
>       bool force_polling;
>       bool dw_usr_bsy;
> +    bool xscale;
>   #ifdef NS16550_PCI
>       /* PCI card parameters. */
>       bool pb_bdf_enable;     /* if =1, pb-bdf effective, port behind bridge */
> @@ -352,6 +353,14 @@ static void ns16550_setup_preirq(struct ns16550 *uart)
>       /* No interrupts. */
>       ns_write_reg(uart, UART_IER, 0);
>   
> +    uart->xscale = false;
> +    if ( !(ns_read_reg(uart, UART_IER) & UART_IER_UUE) )
> +    {
> +        ns_write_reg(uart, UART_IER, UART_IER_UUE);
> +        if ( ns_read_reg(uart, UART_IER) & UART_IER_UUE )
> +            uart->xscale = true;
> +    }
> +

This probe only checks whether IER bit 6 is writable, which isn't
specific to XScale. On UARTs with an EFR (e.g. ST16C650, TI TL16C752,
Exar XR17V35x) the upper IER bits become writable once EFR[4] (ECB) is
set, and bit 6 is then the RTS interrupt enable. If ECB is already set
when the probe runs, it would report XScale, and the driver would then
keep the RTS interrupt enabled.

Could the flag be derived from the compatible in ns16550_uart_dt_init()
instead, the same way dw_usr_bsy is?

     uart->xscale = dt_device_is_compatible(dev, "intel,xscale-uart");

That compatible would also need adding to ns16550_dt_match[]. The
SpacemiT K1 nodes, for example, have no "ns16550a" fallback.

>       /* Handle the DesignWare 8250 'busy-detect' quirk. */
>       handle_dw_usr_busy_quirk(uart);
>   
> @@ -446,7 +455,8 @@ static void ns16550_setup_postirq(struct ns16550 *uart)
>                        UART_MCR, UART_MCR_OUT2 | UART_MCR_DTR | UART_MCR_RTS);
>   
>           /* Enable receive interrupts. */
> -        ns_write_reg(uart, UART_IER, UART_IER_ERDAI);
> +        ns_write_reg(uart, UART_IER,
> +                     UART_IER_ERDAI | (uart->xscale ? UART_IER_UUE : 0));

It seems like not all cases are covered. For example, 
ns16550_interrupt() writes 0 to IER when it switches to polling, which 
would turn off the UART on XScale. Am I missing something? If not then 
something like:

static void ns_write_ier(const struct ns16550 *uart, uint8_t ier)
{
     if ( uart->xscale )
         ier |= UART_IER_UUE;

     ns_write_reg(uart, UART_IER, ier);
}

would be needed, used in ns16550_setup_preirq(), ns16550_setup_postirq()
and ns16550_interrupt().

ns16550_start_tx()/ns16550_stop_tx() do a read-modify-write of IER, so
they read UUE back from the register and write it out again unchanged.
They therefore work correctly without the wrapper. They could be
switched to ns_write_ier() as well for consistency, but that isn't
strictly necessary and likely could be left in way it looks like now.

~ Oleksii

>       }
>   
>       if ( uart->irq >= 0 )
> diff --git a/xen/include/xen/8250-uart.h b/xen/include/xen/8250-uart.h
> index d13352940c..358d264698 100644
> --- a/xen/include/xen/8250-uart.h
> +++ b/xen/include/xen/8250-uart.h
> @@ -42,6 +42,7 @@
>   #define UART_IER_ETHREI   0x02    /* tx reg. empty        */
>   #define UART_IER_ELSI     0x04    /* rx line status       */
>   #define UART_IER_EMSI     0x08    /* MODEM status         */
> +#define UART_IER_UUE      0x40    /* UART unit enable (XScale/PXA) */
>   
>   /* Interrupt Identification Register */
>   #define UART_IIR_NOINT    0x01    /* no interrupt pending */

Re: [PATCH v2 1/2] xen/ns16550: detect and preserve UART_IER_UUE

[Jan Beulich]

On 12.10.2026 11:20, Oleksii Kurochko wrote:
> On 10/10/26 5:14 AM, Zhang Zheng wrote:
>> XScale-compatible UARTs require UART_IER_UUE to remain set.  Probe the
>> bit during setup and preserve it when enabling receive interrupts, so
>> the xscale/PXA UART remains operational after ns16550 takes over.
>>
>> Signed-off-by: Zhang Zheng <zhangzheng@iscas.ac.cn>
>> ---
>>   xen/drivers/char/ns16550.c  | 12 +++++++++++-
>>   xen/include/xen/8250-uart.h |  1 +
>>   2 files changed, 12 insertions(+), 1 deletion(-)
>>
>> diff --git a/xen/drivers/char/ns16550.c b/xen/drivers/char/ns16550.c
>> index cdda28879e..c174465059 100644
>> --- a/xen/drivers/char/ns16550.c
>> +++ b/xen/drivers/char/ns16550.c
>> @@ -64,6 +64,7 @@ static struct ns16550 {
>>       bool intr_works;
>>       bool force_polling;
>>       bool dw_usr_bsy;
>> +    bool xscale;
>>   #ifdef NS16550_PCI
>>       /* PCI card parameters. */
>>       bool pb_bdf_enable;     /* if =1, pb-bdf effective, port behind bridge */
>> @@ -352,6 +353,14 @@ static void ns16550_setup_preirq(struct ns16550 *uart)
>>       /* No interrupts. */
>>       ns_write_reg(uart, UART_IER, 0);
>>   
>> +    uart->xscale = false;
>> +    if ( !(ns_read_reg(uart, UART_IER) & UART_IER_UUE) )
>> +    {
>> +        ns_write_reg(uart, UART_IER, UART_IER_UUE);
>> +        if ( ns_read_reg(uart, UART_IER) & UART_IER_UUE )
>> +            uart->xscale = true;
>> +    }
>> +
> 
> This probe only checks whether IER bit 6 is writable, which isn't
> specific to XScale. On UARTs with an EFR (e.g. ST16C650, TI TL16C752,
> Exar XR17V35x) the upper IER bits become writable once EFR[4] (ECB) is
> set, and bit 6 is then the RTS interrupt enable. If ECB is already set
> when the probe runs, it would report XScale, and the driver would then
> keep the RTS interrupt enabled.
> 
> Could the flag be derived from the compatible in ns16550_uart_dt_init()
> instead, the same way dw_usr_bsy is?
> 
>      uart->xscale = dt_device_is_compatible(dev, "intel,xscale-uart");
> 
> That compatible would also need adding to ns16550_dt_match[]. The
> SpacemiT K1 nodes, for example, have no "ns16550a" fallback.

Are you suggesting that Linux'es XScale detection is flawed then, too?

>> @@ -446,7 +455,8 @@ static void ns16550_setup_postirq(struct ns16550 *uart)
>>                        UART_MCR, UART_MCR_OUT2 | UART_MCR_DTR | UART_MCR_RTS);
>>   
>>           /* Enable receive interrupts. */
>> -        ns_write_reg(uart, UART_IER, UART_IER_ERDAI);
>> +        ns_write_reg(uart, UART_IER,
>> +                     UART_IER_ERDAI | (uart->xscale ? UART_IER_UUE : 0));
> 
> It seems like not all cases are covered. For example, 
> ns16550_interrupt() writes 0 to IER when it switches to polling, which 
> would turn off the UART on XScale. Am I missing something? If not then 
> something like:
> 
> static void ns_write_ier(const struct ns16550 *uart, uint8_t ier)
> {
>      if ( uart->xscale )
>          ier |= UART_IER_UUE;
> 
>      ns_write_reg(uart, UART_IER, ier);
> }
> 
> would be needed, used in ns16550_setup_preirq(), ns16550_setup_postirq()

Yes, the more that as per Linux'es driver source, bit 4 may also need keeping
permanently set (not only for XScale, but apparently also for Tegra).

Jan

Re: [PATCH v2 1/2] xen/ns16550: detect and preserve UART_IER_UUE

[Oleksii Kurochko]

On 10/12/26 2:50 PM, Jan Beulich wrote:
> On 12.10.2026 11:20, Oleksii Kurochko wrote:
>> On 10/10/26 5:14 AM, Zhang Zheng wrote:
>>> XScale-compatible UARTs require UART_IER_UUE to remain set.  Probe the
>>> bit during setup and preserve it when enabling receive interrupts, so
>>> the xscale/PXA UART remains operational after ns16550 takes over.
>>>
>>> Signed-off-by: Zhang Zheng <zhangzheng@iscas.ac.cn>
>>> ---
>>>    xen/drivers/char/ns16550.c  | 12 +++++++++++-
>>>    xen/include/xen/8250-uart.h |  1 +
>>>    2 files changed, 12 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/xen/drivers/char/ns16550.c b/xen/drivers/char/ns16550.c
>>> index cdda28879e..c174465059 100644
>>> --- a/xen/drivers/char/ns16550.c
>>> +++ b/xen/drivers/char/ns16550.c
>>> @@ -64,6 +64,7 @@ static struct ns16550 {
>>>        bool intr_works;
>>>        bool force_polling;
>>>        bool dw_usr_bsy;
>>> +    bool xscale;
>>>    #ifdef NS16550_PCI
>>>        /* PCI card parameters. */
>>>        bool pb_bdf_enable;     /* if =1, pb-bdf effective, port behind bridge */
>>> @@ -352,6 +353,14 @@ static void ns16550_setup_preirq(struct ns16550 *uart)
>>>        /* No interrupts. */
>>>        ns_write_reg(uart, UART_IER, 0);
>>>    
>>> +    uart->xscale = false;
>>> +    if ( !(ns_read_reg(uart, UART_IER) & UART_IER_UUE) )
>>> +    {
>>> +        ns_write_reg(uart, UART_IER, UART_IER_UUE);
>>> +        if ( ns_read_reg(uart, UART_IER) & UART_IER_UUE )
>>> +            uart->xscale = true;
>>> +    }
>>> +
>>
>> This probe only checks whether IER bit 6 is writable, which isn't
>> specific to XScale. On UARTs with an EFR (e.g. ST16C650, TI TL16C752,
>> Exar XR17V35x) the upper IER bits become writable once EFR[4] (ECB) is
>> set, and bit 6 is then the RTS interrupt enable. If ECB is already set
>> when the probe runs, it would report XScale, and the driver would then
>> keep the RTS interrupt enabled.
>>
>> Could the flag be derived from the compatible in ns16550_uart_dt_init()
>> instead, the same way dw_usr_bsy is?
>>
>>       uart->xscale = dt_device_is_compatible(dev, "intel,xscale-uart");
>>
>> That compatible would also need adding to ns16550_dt_match[]. The
>> SpacemiT K1 nodes, for example, have no "ns16550a" fallback.
> 
> Are you suggesting that Linux'es XScale detection is flawed then, too?

I looked at it quickly and not flawed as such, but Linux uses that test 
only as a last-resort heuristic, which isn't how it would be used here.

In Linux the bit 6 test sits at the end of autoconfig_16550a(). That
only runs for ports whose type isn't known up front (legacy/PnP/PCI
autoprobing), and only with CONFIG_SERIAL_8250_16550A_VARIANTS or
UPF_FULL_PROBE. It also comes after the EFR probes, so ST16C65x /
TL16C75x style parts are classified and return before reaching it.
Even so, the comment there says "this is probably one of the Xscale
platform's internal UARTs", and a part whose EFR already has ECB set
would fall through the EFR check and be reported as XScale. So yes,
the same corner case exists there.

Where the type is known, Linux doesn't probe at all: 8250_of maps
"intel,xscale-uart" to PORT_XSCALE and sets UPF_FIXED_TYPE, so
autoconfig() (where it is happening the probing in a similar to 
suggested above way) never runs for DT ports. 8250_exar sets 
UPF_FIXED_TYPE as well.

> 
>>> @@ -446,7 +455,8 @@ static void ns16550_setup_postirq(struct ns16550 *uart)
>>>                         UART_MCR, UART_MCR_OUT2 | UART_MCR_DTR | UART_MCR_RTS);
>>>    
>>>            /* Enable receive interrupts. */
>>> -        ns_write_reg(uart, UART_IER, UART_IER_ERDAI);
>>> +        ns_write_reg(uart, UART_IER,
>>> +                     UART_IER_ERDAI | (uart->xscale ? UART_IER_UUE : 0));
>>
>> It seems like not all cases are covered. For example,
>> ns16550_interrupt() writes 0 to IER when it switches to polling, which
>> would turn off the UART on XScale. Am I missing something? If not then
>> something like:
>>
>> static void ns_write_ier(const struct ns16550 *uart, uint8_t ier)
>> {
>>       if ( uart->xscale )
>>           ier |= UART_IER_UUE;
>>
>>       ns_write_reg(uart, UART_IER, ier);
>> }
>>
>> would be needed, used in ns16550_setup_preirq(), ns16550_setup_postirq()
> 
> Yes, the more that as per Linux'es driver source, bit 4 may also need keeping
> permanently set (not only for XScale, but apparently also for Tegra).

Good point.

~ Oleksii

Re: [PATCH v2 1/2] xen/ns16550: detect and preserve UART_IER_UUE

[Zhang Zheng]

On Mon, Oct 12, 2026 at 03:24:20PM +0200, Oleksii Kurochko wrote:
> 
> 
> On 10/12/26 2:50 PM, Jan Beulich wrote:
> > On 12.10.2026 11:20, Oleksii Kurochko wrote:
> > > On 10/10/26 5:14 AM, Zhang Zheng wrote:
> > > > XScale-compatible UARTs require UART_IER_UUE to remain set.  Probe the
> > > > bit during setup and preserve it when enabling receive interrupts, so
> > > > the xscale/PXA UART remains operational after ns16550 takes over.
> > > > 
> > > > Signed-off-by: Zhang Zheng <zhangzheng@iscas.ac.cn>
> > > > ---
> > > >    xen/drivers/char/ns16550.c  | 12 +++++++++++-
> > > >    xen/include/xen/8250-uart.h |  1 +
> > > >    2 files changed, 12 insertions(+), 1 deletion(-)
> > > > 
> > > > diff --git a/xen/drivers/char/ns16550.c b/xen/drivers/char/ns16550.c
> > > > index cdda28879e..c174465059 100644
> > > > --- a/xen/drivers/char/ns16550.c
> > > > +++ b/xen/drivers/char/ns16550.c
> > > > @@ -64,6 +64,7 @@ static struct ns16550 {
> > > >        bool intr_works;
> > > >        bool force_polling;
> > > >        bool dw_usr_bsy;
> > > > +    bool xscale;
> > > >    #ifdef NS16550_PCI
> > > >        /* PCI card parameters. */
> > > >        bool pb_bdf_enable;     /* if =1, pb-bdf effective, port behind bridge */
> > > > @@ -352,6 +353,14 @@ static void ns16550_setup_preirq(struct ns16550 *uart)
> > > >        /* No interrupts. */
> > > >        ns_write_reg(uart, UART_IER, 0);
> > > > +    uart->xscale = false;
> > > > +    if ( !(ns_read_reg(uart, UART_IER) & UART_IER_UUE) )
> > > > +    {
> > > > +        ns_write_reg(uart, UART_IER, UART_IER_UUE);
> > > > +        if ( ns_read_reg(uart, UART_IER) & UART_IER_UUE )
> > > > +            uart->xscale = true;
> > > > +    }
> > > > +
> > > 
> > > This probe only checks whether IER bit 6 is writable, which isn't
> > > specific to XScale. On UARTs with an EFR (e.g. ST16C650, TI TL16C752,
> > > Exar XR17V35x) the upper IER bits become writable once EFR[4] (ECB) is
> > > set, and bit 6 is then the RTS interrupt enable. If ECB is already set
> > > when the probe runs, it would report XScale, and the driver would then
> > > keep the RTS interrupt enabled.
> > > 
> > > Could the flag be derived from the compatible in ns16550_uart_dt_init()
> > > instead, the same way dw_usr_bsy is?
> > > 
> > >       uart->xscale = dt_device_is_compatible(dev, "intel,xscale-uart");
> > > 
> > > That compatible would also need adding to ns16550_dt_match[]. The
> > > SpacemiT K1 nodes, for example, have no "ns16550a" fallback.
> > 
> > Are you suggesting that Linux'es XScale detection is flawed then, too?
> 
> I looked at it quickly and not flawed as such, but Linux uses that test only
> as a last-resort heuristic, which isn't how it would be used here.
> 
> In Linux the bit 6 test sits at the end of autoconfig_16550a(). That
> only runs for ports whose type isn't known up front (legacy/PnP/PCI
> autoprobing), and only with CONFIG_SERIAL_8250_16550A_VARIANTS or
> UPF_FULL_PROBE. It also comes after the EFR probes, so ST16C65x /
> TL16C75x style parts are classified and return before reaching it.
> Even so, the comment there says "this is probably one of the Xscale
> platform's internal UARTs", and a part whose EFR already has ECB set
> would fall through the EFR check and be reported as XScale. So yes,
> the same corner case exists there.
> 
> Where the type is known, Linux doesn't probe at all: 8250_of maps
> "intel,xscale-uart" to PORT_XSCALE and sets UPF_FIXED_TYPE, so
> autoconfig() (where it is happening the probing in a similar to suggested
> above way) never runs for DT ports. 8250_exar sets UPF_FIXED_TYPE as well.
> 
I agree that it is better to drop the UUE probe and identify the XScale UART
directly from its compatible.

Zhang
> > 
> > > > @@ -446,7 +455,8 @@ static void ns16550_setup_postirq(struct ns16550 *uart)
> > > >                         UART_MCR, UART_MCR_OUT2 | UART_MCR_DTR | UART_MCR_RTS);
> > > >            /* Enable receive interrupts. */
> > > > -        ns_write_reg(uart, UART_IER, UART_IER_ERDAI);
> > > > +        ns_write_reg(uart, UART_IER,
> > > > +                     UART_IER_ERDAI | (uart->xscale ? UART_IER_UUE : 0));
> > > 
> > > It seems like not all cases are covered. For example,
> > > ns16550_interrupt() writes 0 to IER when it switches to polling, which
> > > would turn off the UART on XScale. Am I missing something? If not then
> > > something like:
> > > 
> > > static void ns_write_ier(const struct ns16550 *uart, uint8_t ier)
> > > {
> > >       if ( uart->xscale )
> > >           ier |= UART_IER_UUE;
> > > 
> > >       ns_write_reg(uart, UART_IER, ier);
> > > }
> > > 
> > > would be needed, used in ns16550_setup_preirq(), ns16550_setup_postirq()
> > 
> > Yes, the more that as per Linux'es driver source, bit 4 may also need keeping
> > permanently set (not only for XScale, but apparently also for Tegra).
> 
> Good point.
> 
> ~ Oleksii

Re: [PATCH v2 1/2] xen/ns16550: detect and preserve UART_IER_UUE

[Zhang Zheng]

On Mon, Oct 12, 2026 at 11:20:22AM +0200, Oleksii Kurochko wrote:
> 
> 
> On 10/10/26 5:14 AM, Zhang Zheng wrote:
> > XScale-compatible UARTs require UART_IER_UUE to remain set.  Probe the
> > bit during setup and preserve it when enabling receive interrupts, so
> > the xscale/PXA UART remains operational after ns16550 takes over.
> > 
> > Signed-off-by: Zhang Zheng <zhangzheng@iscas.ac.cn>
> > ---
> >   xen/drivers/char/ns16550.c  | 12 +++++++++++-
> >   xen/include/xen/8250-uart.h |  1 +
> >   2 files changed, 12 insertions(+), 1 deletion(-)
> > 
> > diff --git a/xen/drivers/char/ns16550.c b/xen/drivers/char/ns16550.c
> > index cdda28879e..c174465059 100644
> > --- a/xen/drivers/char/ns16550.c
> > +++ b/xen/drivers/char/ns16550.c
> > @@ -64,6 +64,7 @@ static struct ns16550 {
> >       bool intr_works;
> >       bool force_polling;
> >       bool dw_usr_bsy;
> > +    bool xscale;
> >   #ifdef NS16550_PCI
> >       /* PCI card parameters. */
> >       bool pb_bdf_enable;     /* if =1, pb-bdf effective, port behind bridge */
> > @@ -352,6 +353,14 @@ static void ns16550_setup_preirq(struct ns16550 *uart)
> >       /* No interrupts. */
> >       ns_write_reg(uart, UART_IER, 0);
> > +    uart->xscale = false;
> > +    if ( !(ns_read_reg(uart, UART_IER) & UART_IER_UUE) )
> > +    {
> > +        ns_write_reg(uart, UART_IER, UART_IER_UUE);
> > +        if ( ns_read_reg(uart, UART_IER) & UART_IER_UUE )
> > +            uart->xscale = true;
> > +    }
> > +
> 
> This probe only checks whether IER bit 6 is writable, which isn't
> specific to XScale. On UARTs with an EFR (e.g. ST16C650, TI TL16C752,
> Exar XR17V35x) the upper IER bits become writable once EFR[4] (ECB) is
> set, and bit 6 is then the RTS interrupt enable. If ECB is already set
> when the probe runs, it would report XScale, and the driver would then
> keep the RTS interrupt enabled.
> 
> Could the flag be derived from the compatible in ns16550_uart_dt_init()
> instead, the same way dw_usr_bsy is?
> 
>     uart->xscale = dt_device_is_compatible(dev, "intel,xscale-uart");
> 
> That compatible would also need adding to ns16550_dt_match[]. The
> SpacemiT K1 nodes, for example, have no "ns16550a" fallback.
> 
This UEE probe section was adapted from Linux's 8250.c `autoconfig_16550a()`.
Nevertheless, I think using the Device Tree compatible is a reasonable solution,
given your point that this bit may also be writable on other UARTs and could
therefore result in false positives.
> >       /* Handle the DesignWare 8250 'busy-detect' quirk. */
> >       handle_dw_usr_busy_quirk(uart);
> > @@ -446,7 +455,8 @@ static void ns16550_setup_postirq(struct ns16550 *uart)
> >                        UART_MCR, UART_MCR_OUT2 | UART_MCR_DTR | UART_MCR_RTS);
> >           /* Enable receive interrupts. */
> > -        ns_write_reg(uart, UART_IER, UART_IER_ERDAI);
> > +        ns_write_reg(uart, UART_IER,
> > +                     UART_IER_ERDAI | (uart->xscale ? UART_IER_UUE : 0));
> 
> It seems like not all cases are covered. For example, ns16550_interrupt()
> writes 0 to IER when it switches to polling, which would turn off the UART
> on XScale. Am I missing something? If not then something like:
> 
> static void ns_write_ier(const struct ns16550 *uart, uint8_t ier)
> {
>     if ( uart->xscale )
>         ier |= UART_IER_UUE;
> 
>     ns_write_reg(uart, UART_IER, ier);
> }
> 
> would be needed, used in ns16550_setup_preirq(), ns16550_setup_postirq()
> and ns16550_interrupt().
> 
Yes, i missed that path. I will add `ns_write_ier()` and use
it in `ns16550_setup_preirq()`, `ns16550_setup_postirq()`, and
`ns16550_interrupt()` so that UUE is preserved on every direct IER
write.
> ns16550_start_tx()/ns16550_stop_tx() do a read-modify-write of IER, so
> they read UUE back from the register and write it out again unchanged.
> They therefore work correctly without the wrapper. They could be
> switched to ns_write_ier() as well for consistency, but that isn't
> strictly necessary and likely could be left in way it looks like now.
> 
> ~ Oleksii
> 
Agreed. Since these paths already preserve UUE, I will leave them unchanged.

Thanks

Zhang
> >       }
> >       if ( uart->irq >= 0 )
> > diff --git a/xen/include/xen/8250-uart.h b/xen/include/xen/8250-uart.h
> > index d13352940c..358d264698 100644
> > --- a/xen/include/xen/8250-uart.h
> > +++ b/xen/include/xen/8250-uart.h
> > @@ -42,6 +42,7 @@
> >   #define UART_IER_ETHREI   0x02    /* tx reg. empty        */
> >   #define UART_IER_ELSI     0x04    /* rx line status       */
> >   #define UART_IER_EMSI     0x08    /* MODEM status         */
> > +#define UART_IER_UUE      0x40    /* UART unit enable (XScale/PXA) */
> >   /* Interrupt Identification Register */
> >   #define UART_IIR_NOINT    0x01    /* no interrupt pending */

Re: [PATCH v2 1/2] xen/ns16550: detect and preserve UART_IER_UUE

[Jan Beulich]

On 10.10.2026 05:14, Zhang Zheng wrote:
> --- a/xen/drivers/char/ns16550.c
> +++ b/xen/drivers/char/ns16550.c
> @@ -64,6 +64,7 @@ static struct ns16550 {
>      bool intr_works;
>      bool force_polling;
>      bool dw_usr_bsy;
> +    bool xscale;
>  #ifdef NS16550_PCI
>      /* PCI card parameters. */
>      bool pb_bdf_enable;     /* if =1, pb-bdf effective, port behind bridge */
> @@ -352,6 +353,14 @@ static void ns16550_setup_preirq(struct ns16550 *uart)
>      /* No interrupts. */
>      ns_write_reg(uart, UART_IER, 0);
>  
> +    uart->xscale = false;
> +    if ( !(ns_read_reg(uart, UART_IER) & UART_IER_UUE) )
> +    {
> +        ns_write_reg(uart, UART_IER, UART_IER_UUE);
> +        if ( ns_read_reg(uart, UART_IER) & UART_IER_UUE )
> +            uart->xscale = true;
> +    }

Linux'es detection is a little more involved, plus: It doesn't clobber the
value originally present in the other 7 bits.

Jan

Re: [PATCH v2 1/2] xen/ns16550: detect and preserve UART_IER_UUE

[Zhang Zheng]

On Mon, Oct 12, 2026 at 02:52:06PM +0200, Jan Beulich wrote:
> On 10.10.2026 05:14, Zhang Zheng wrote:
> > --- a/xen/drivers/char/ns16550.c
> > +++ b/xen/drivers/char/ns16550.c
> > @@ -64,6 +64,7 @@ static struct ns16550 {
> >      bool intr_works;
> >      bool force_polling;
> >      bool dw_usr_bsy;
> > +    bool xscale;
> >  #ifdef NS16550_PCI
> >      /* PCI card parameters. */
> >      bool pb_bdf_enable;     /* if =1, pb-bdf effective, port behind bridge */
> > @@ -352,6 +353,14 @@ static void ns16550_setup_preirq(struct ns16550 *uart)
> >      /* No interrupts. */
> >      ns_write_reg(uart, UART_IER, 0);
> >  
> > +    uart->xscale = false;
> > +    if ( !(ns_read_reg(uart, UART_IER) & UART_IER_UUE) )
> > +    {
> > +        ns_write_reg(uart, UART_IER, UART_IER_UUE);
> > +        if ( ns_read_reg(uart, UART_IER) & UART_IER_UUE )
> > +            uart->xscale = true;
> > +    }
> 
> Linux'es detection is a little more involved, plus: It doesn't clobber the
> value originally present in the other 7 bits.
> 
> Jan

I noticed that Linux saves the original IER value and clears only UUE:

iersave = serial_in(up, UART_IER);
serial_out(up, UART_IER, iersave & ~UART_IER_UUE);

Xen clears IER immediately before this probe, so I think there is no previous
IER state to preserve. So I did not add `iersave` here.

Re: [PATCH v2 1/2] xen/ns16550: detect and preserve UART_IER_UUE

[Jan Beulich]

On 15.10.2026 06:40, Zhang Zheng wrote:
> On Mon, Oct 12, 2026 at 02:52:06PM +0200, Jan Beulich wrote:
>> On 10.10.2026 05:14, Zhang Zheng wrote:
>>> --- a/xen/drivers/char/ns16550.c
>>> +++ b/xen/drivers/char/ns16550.c
>>> @@ -64,6 +64,7 @@ static struct ns16550 {
>>>      bool intr_works;
>>>      bool force_polling;
>>>      bool dw_usr_bsy;
>>> +    bool xscale;
>>>  #ifdef NS16550_PCI
>>>      /* PCI card parameters. */
>>>      bool pb_bdf_enable;     /* if =1, pb-bdf effective, port behind bridge */
>>> @@ -352,6 +353,14 @@ static void ns16550_setup_preirq(struct ns16550 *uart)
>>>      /* No interrupts. */
>>>      ns_write_reg(uart, UART_IER, 0);
>>>  
>>> +    uart->xscale = false;
>>> +    if ( !(ns_read_reg(uart, UART_IER) & UART_IER_UUE) )
>>> +    {
>>> +        ns_write_reg(uart, UART_IER, UART_IER_UUE);
>>> +        if ( ns_read_reg(uart, UART_IER) & UART_IER_UUE )
>>> +            uart->xscale = true;
>>> +    }
>>
>> Linux'es detection is a little more involved, plus: It doesn't clobber the
>> value originally present in the other 7 bits.
> 
> I noticed that Linux saves the original IER value and clears only UUE:
> 
> iersave = serial_in(up, UART_IER);
> serial_out(up, UART_IER, iersave & ~UART_IER_UUE);
> 
> Xen clears IER immediately before this probe, so I think there is no previous
> IER state to preserve. So I did not add `iersave` here.

Oh, right, I didn't pay attention to that, despite it even being visible in
patch context.

Jan

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

[Baptiste Le Duc]

For any reasons, it seems this patch has not been received by ML server, 
I couldn't find him here: 
https://lore.kernel.org/xen-devel/20261010031406.2057735-1-zhangzheng@iscas.ac.cn/T/#t


On 10/10/26 5:21 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.
> 
> Suggested-by: Oleksii Kurochko <oleksii.kurochko@gmail.com>
> Link: https://lore.kernel.org/xen-devel/8836d1e3-77c8-4447-b1ef-132620a71441@gmail.com/
> Signed-off-by: Zhang Zheng <zhangzheng@iscas.ac.cn>
> ---
>   xen/drivers/char/ns16550.c | 4 ++++
>   1 file changed, 4 insertions(+)
> 
> diff --git a/xen/drivers/char/ns16550.c b/xen/drivers/char/ns16550.c
> index c174465059..278d38822d 100644
> --- 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. */
> +    while ( !(ns_read_reg(uart, UART_LSR) & UART_LSR_TEMT) )
> +        cpu_relax();
> +
>       ns16550_setup_preirq(uart);
>   
>       /* Check this really is a 16550+. Otherwise we have no FIFOs. */

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

[Oleksii Kurochko]

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.
> 
> Suggested-by: Oleksii Kurochko <oleksii.kurochko@gmail.com>
> Link: https://lore.kernel.org/xen-devel/8836d1e3-77c8-4447-b1ef-132620a71441@gmail.com/
> Signed-off-by: Zhang Zheng <zhangzheng@iscas.ac.cn>
> ---
>   xen/drivers/char/ns16550.c | 4 ++++
>   1 file changed, 4 insertions(+)
> 
> diff --git a/xen/drivers/char/ns16550.c b/xen/drivers/char/ns16550.c
> index c174465059..278d38822d 100644
> --- 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.

~ Oleksii

>       ns16550_setup_preirq(uart);
>   
>       /* Check this really is a 16550+. Otherwise we have no FIFOs. */

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

[Jan Beulich]

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

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

[Oleksii Kurochko]

On 10/12/26 2:56 PM, 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

I am perfectly okay with the way suggested by Marek.

~ Oleksii

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

[Zhang Zheng]

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

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

[Jan Beulich]

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.)

Then again we have the .lsr_mask field. Not using it here would then call
for justification in the description.

Jan

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

[Zhang Zheng]

On Mon, Oct 12, 2026 at 03:00:19PM +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.)
> 
> Then again we have the .lsr_mask field. Not using it here would then call
> for justification in the description.
> 
> Jan

I will clarify this distinction in the commit message.

Zhang

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

[Zhang Zheng]

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

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

[Oleksii Kurochko]

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@iscas.ac.cn/T/#meaf24c3bb039dc6aaecffab752d2b107581816e0

I think we still can have timeout check.

~ Oleksii

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

[Zhang Zheng]

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@iscas.ac.cn/T/#meaf24c3bb039dc6aaecffab752d2b107581816e0
> 
> I think we still can have timeout check.
> 
> ~ Oleksii

I saw it afterwards. Thanks.

Zhang