Re: [PATCH] ns16550: find the console UART on PCI when there is no legacy one

[Jan Beulich <jbeulich@xxxxxxxx>]

On 22.09.2026 20:43, Benjamin Leggett wrote:
> Amazon EC2 bare metal instances have no UART at the legacy I/O port
> 0x3f8. Their only serial port is a 16550-compatible PCI device (vendor
> 0x1d0f, device 0x8250) with its registers in the MMIO space of BAR 0.
> Today Xen has no console on these systems unless the command line
> names that device, and "com1=...,pci" cannot find it, because
> uart_config[] doesn't have it.
> 
> Add the device to uart_config[].

Is there a spec that you could point to here?

> Additionally, when the port that com1 describes is not present, scan PCI
> for a known UART before giving up. This way the same command line works on
> systems with and without a legacy UART, and a machine-specific "pci"
> option is not needed. The fallback does not run when the command line
> gave an I/O base, or when it already asked for a scan with "pci" or
> "amt", so explicit config keeps its current meaning. It is
> for com1 only: for com2, pci_uart_config() skips the first port it
> finds, so it can never match a single-port device.

This needs to be split to a separate patch, and not only because right
now you're doing two entirely unrelated things in a single change. The
addition of the Amazon device is likely uncontroversial, so presumably
can go in quickly. Doing a scan when none was asked for, otoh, is
potentially problematic: What if there's an issue during scanning? With
not having any output set up yet, we couldn't even indicate the problem,
and a possible crash would also go entirely silently.

> When the scan finds nothing, pci_uart_config() puts back the original
> base, and check_existence() does not test MMIO addresses.
> 
> On a system with a legacy UART, check_existence() passes and nothing
> changes.

This isn't really true, is it? You check ...

> @@ -1805,7 +1825,32 @@ 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");
> +    {
> +        bool present = false;
> +
> +#ifdef NS16550_PCI
> +        /*
> +         * Some systems, EC2 bare metal among them, have no legacy UART and
> +         * carry their only serial port on PCI. Look for one before giving 
> up,
> +         * unless the command line named a base or already asked for a scan.
> +         * com1 only: for com2 the scan skips the first port it finds, so it
> +         * can never match a single-port device.
> +         */
> +        if ( uart == ns16550_com && !uart->io_base_set && !uart->pci_scanned 
> )

... ->io_base_set here, which can only be true when a command line option
provided the base address. A cmdline option like "com1=115200" doesn't,
and instead the value set by (on x86) __start_xen() is used. If that isn't
the correct address to use, check_existence() is (hopefully) going to fail.
(For example, I have a system where firmware mixes up COM1 and COM2
settings, which you may only notice after having played with things for a
while. In such a case it doesn't help if unexpected PCI bus scanning gets
in the way.)

Jan