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

[Benjamin Leggett <benjamin@xxxxxxxx>]

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.

The scan runs only when the command line asks for it, at the same
point in boot as "pci" and "amt". There is no console yet, but the
scan is not silent: it logs the device that it picked, and
PARSE_ERR() logs a failure. Both messages go to the console ring, and
console_init_preirq() replays the ring to each console that comes up,
VGA included. Only a hang or a crash inside the scan goes unreported,
as with "pci" and "amt" today.

To limit what an unexpected scan can touch, "auto" only considers
devices that are in uart_config[]. A serial-class device without
built-in parameters is skipped before its BARs are sized, so a
firmware that misreports the legacy port cannot move com1 to an
arbitrary plug-in card.

When the scan finds nothing, pci_uart_config() puts back the original
base, and check_existence() does not test MMIO addresses. Thus
ps_bdf_enable, not check_existence(), tells whether a device was
found.

Assisted-by: Claude:claude-opus-5-5
Signed-off-by: Benjamin Leggett <benjamin@xxxxxxxx>
---
 docs/misc/xen-command-line.pandoc |  5 ++++-
 xen/drivers/char/ns16550.c        | 31 ++++++++++++++++++++++++++++++-
 2 files changed, 34 insertions(+), 2 deletions(-)

diff --git a/docs/misc/xen-command-line.pandoc 
b/docs/misc/xen-command-line.pandoc
index ea0e3368b1..58ef53a5b2 100644
--- a/docs/misc/xen-command-line.pandoc
+++ b/docs/misc/xen-command-line.pandoc
@@ -357,7 +357,7 @@ ACPI indicating none to be there.
 
 ### com1 (x86)
 ### com2 (x86)
-> `= 
<baud>[/<base-baud>][,[DPS][,[<io-base>|pci|amt][,[<irq>|msi][,[<port-bdf>][,[<bridge-bdf>]]]]]]`
+> `= 
<baud>[/<base-baud>][,[DPS][,[<io-base>|pci|amt|auto][,[<irq>|msi][,[<port-bdf>][,[<bridge-bdf>]]]]]]`
 
 Both option `com1` and `com2` follow the same format.
 
@@ -387,6 +387,9 @@ Both option `com1` and `com2` follow the same format.
   avoiding Intel AMT devices.
 * `amt` indicated that Xen should scan the PCI bus for the UART,
   including Intel AMT devices if present.
+* `auto` indicates that Xen should use the default I/O base if a UART
+  responds there, and otherwise scan the PCI bus for the UART as `pci`
+  does, but only for devices with built-in parameters in Xen.
 
 A typical setup for most situations might be `com1=115200,8n1`
 
diff --git a/xen/drivers/char/ns16550.c b/xen/drivers/char/ns16550.c
index cdda28879e..ab55e2ba45 100644
--- a/xen/drivers/char/ns16550.c
+++ b/xen/drivers/char/ns16550.c
@@ -66,6 +66,7 @@ static struct ns16550 {
     bool dw_usr_bsy;
 #ifdef NS16550_PCI
     /* PCI card parameters. */
+    bool pci_fallback;      /* if =1, scan PCI if io_base has no UART */
     bool pb_bdf_enable;     /* if =1, pb-bdf effective, port behind bridge */
     bool ps_bdf_enable;     /* if =1, ps_bdf effective, port on pci card */
     pci_sbdf_t pci_bridge;
@@ -1321,6 +1322,10 @@ pci_uart_config(struct ns16550 *uart, bool skip_amt, 
unsigned int idx)
                     }
                 }
 
+                /* Keep "auto" off unknown devices, including their BARs. */
+                if ( uart->pci_fallback && i == ARRAY_SIZE(uart_config) )
+                    continue;
+
                 if ( port_idx >= param->max_ports )
                 {
                     idx -= param->max_ports;
@@ -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
         {
@@ -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 )
+                printk("ns16550: com%u: no UART at default base, using %pp\n",
+                       (unsigned int)(uart - ns16550_com) + 1,
+                       &uart->pci_device);
+        }
+
+        /*
+         * A failed scan puts back the rejected base, and check_existence()
+         * passes MMIO addresses untested, so only ps_bdf_enable says that
+         * a device was found.
+         */
+        if ( !uart->ps_bdf_enable || !check_existence(uart) )
+#endif
+            PARSE_ERR("16550-compatible serial UART not present");
+    }
 
     /* Register with generic serial driver. */
     serial_register_uart(uart - ns16550_com, &ns16550_driver, uart);
-- 
2.55.0