|
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] [PATCH] ns16550: add "auto" to scan PCI only when the default base has no UART
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, and as "dev=auto".
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 decision rests on check_existence(), which until now reported an
MMIO base as present without testing it. Make it return -1 for a base
that it cannot probe, so that callers can tell "no UART" apart from
"not probed". The HAS_IOPORTS=n branch returns -1 too, for the same
reason, although it is not reachable today: the function is x86-only,
and x86 always selects HAS_IOPORTS. Existing behavior does not change,
as -1 is still non-zero, but "auto" now refuses a base that cannot be
probed. The decision therefore always comes from the I/O port probe,
and with nothing at the port, reads return 0xff and the IER test
fails.
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.
Assisted-by: Claude:claude-opus-5-5
Signed-off-by: Benjamin Leggett <benjamin@xxxxxxxx>
---
docs/misc/xen-command-line.pandoc | 9 ++++--
xen/drivers/char/ns16550.c | 46 ++++++++++++++++++++++++++++---
2 files changed, 48 insertions(+), 7 deletions(-)
diff --git a/docs/misc/xen-command-line.pandoc
b/docs/misc/xen-command-line.pandoc
index a6f08463d1..6fff62800f 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`
@@ -411,9 +414,9 @@ The accepted name keywords for name=value pairs are:
* `clock-hz`- accepts large integers to setup UART clock frequencies.
Do note - these values are multiplied by 16.
* `data-bits` - integer between 5 and 8
-* `dev` - accepted values are `pci` OR `amt`. If this option
+* `dev` - accepted values are `pci`, `amt` OR `auto`. If this option
is used to specify if the serial device is pci-based. The io_base
- cannot be specified when `dev=pci` or `dev=amt` is used.
+ cannot be specified when `dev=pci`, `dev=amt` or `dev=auto` is used.
* `io-base` - accepts integer which specified IO base port for UART registers
* `irq` - IRQ number to use
* `parity` - accepted values are same as positional parameters
diff --git a/xen/drivers/char/ns16550.c b/xen/drivers/char/ns16550.c
index cdda28879e..56de4d50fb 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;
@@ -727,6 +728,7 @@ static int __init parse_parity_char(int c)
return 0;
}
+/* Returns 1 if a UART answers at io_base, 0 if none does, -1 if untestable. */
static int __init check_existence(struct ns16550 *uart)
{
unsigned char status, scratch, scratch2, scratch3;
@@ -737,9 +739,9 @@ static int __init check_existence(struct ns16550 *uart)
* if we're getting MMIO UARTs, the arch code knows what it's doing.
*/
if ( uart->io_base >= 0x10000 )
- return 1;
+ return -1;
#else
- return 1; /* Everything is MMIO */
+ return -1; /* Everything is MMIO */
#endif
pci_serial_early_init(uart);
@@ -1321,6 +1323,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 +1634,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
{
@@ -1745,6 +1756,11 @@ static bool __init parse_namevalue_pairs(char *str,
struct ns16550 *uart)
pci_uart_config(uart, 0, uart - ns16550_com);
dev_set = true;
}
+ else if ( strncmp(param_value, "auto", 4) == 0 )
+ {
+ uart->pci_fallback = true;
+ dev_set = true;
+ }
else
PARSE_ERR_RET("Unknown device type %s\n", param_value);
break;
@@ -1785,6 +1801,7 @@ static void __init ns16550_parse_port_config(
{
char com_console_options[128];
char *str;
+ int present;
/* No user-specified configuration? */
if ( (conf == NULL) || (*conf == '\0') )
@@ -1818,8 +1835,29 @@ static void __init ns16550_parse_port_config(
PARSE_ERR("%d stop bits are unsupported.", uart->stop_bits);
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");
+ present = check_existence(uart);
+#ifdef CONFIG_HAS_PCI
+ if ( uart->pci_fallback && present < 0 )
+ PARSE_ERR("\"auto\" needs a base that can be probed");
+#endif
+ if ( !present )
+ {
+#ifdef CONFIG_HAS_PCI
+ if ( uart->pci_fallback )
+ pci_uart_config(uart, 1 /* skip AMT */, uart - ns16550_com);
+
+ /*
+ * A failed scan puts back the rejected base, and check_existence()
+ * can't test an MMIO BAR, so only ->param says that a device was
+ * found.
+ */
+ if ( uart->pci_fallback && uart->param && check_existence(uart) )
+ printk("ns16550: com%zd: no UART at default base, using %pp\n",
+ uart - ns16550_com + 1, &uart->pci_device);
+ else
+#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
|
 |
Lists.xenproject.org is hosted with RackSpace, monitoring our |