Re: [PATCH] ns16550: add "auto" to scan PCI only when the default base has no UART

[Jan Beulich <jbeulich@xxxxxxxx>]

On 09.10.2026 22:24, Benjamin Leggett wrote:
> On some systems the console UART is at the legacy I/O port, and on
> others, such as Amazon EC2 bare metal instances, the only UART is on
> PCI. "com1=...,pci" finds the PCI UART, but it prefers any PCI serial
> device to the legacy port, so it is not correct on a system that has
> both. No option today says "use the legacy port, and scan PCI only if
> it has no UART". A deployment that boots one command line on many
> kinds of machine needs exactly that.
> 
> Add "auto" as a value in the <io-base> position. With it, Xen probes
> the default base as usual. Only if check_existence() fails there does
> it scan PCI, as "pci" does. Without "auto", nothing changes.

I've always been considering check_existence() as somewhat fragile (it's
not very difficult for it to yield a false positive, especially when
HAS_IOPORTS=n), which means that I'm not really convinced it can (reliably)
be used for the purpose here.

> @@ -1628,6 +1633,11 @@ static bool __init parse_positional(struct ns16550 
> *uart, char **str)
>                  return true;
>              conf += 3;
>          }
> +        else if ( strncmp(conf, "auto", 4) == 0 )
> +        {
> +            uart->pci_fallback = true;
> +            conf += 4;
> +        }
>          else
>  #endif
>          {

What about parse_namevalue_pairs()? Once you also adjust that, you'll notice
that ...

> @@ -1819,7 +1829,26 @@ static void __init ns16550_parse_port_config(
>      if ( uart->io_base == 0 )
>          PARSE_ERR("I/O base address must be specified.");
>      if ( !check_existence(uart) )
> -        PARSE_ERR("16550-compatible serial UART not present");
> +    {
> +#ifdef CONFIG_HAS_PCI
> +        if ( uart->pci_fallback )
> +        {
> +            pci_uart_config(uart, 1 /* skip AMT */, uart - ns16550_com);
> +            if ( uart->ps_bdf_enable )

... this isn't a good value to check, as it can also be set from the cmdline.
Perhaps checking ->param is an alternative.

> +                printk("ns16550: com%u: no UART at default base, using 
> %pp\n",
> +                       (unsigned int)(uart - ns16550_com) + 1,

Would be nice if we could get away without cast here. While out printk()
still doesn't support the t modifier, aiui the z one should do here.

Jan