Re: [kvm-unit-tests PATCH] arm: io: Set is_pl011_uart flag in ACPI initialization

[Alexandru Elisei <alexandru.elisei@arm.com>]

Hi,

On Wed, May 13, 2026 at 09:16:24PM +0000, Colton Lewis wrote:
> When booting via ACPI on ARM64, the SPCR table is parsed to initialize
> the console UART. However, the code was neglecting to set the
> is_pl011_uart flag. This caused __getchar() to fall back to treating
> the UART as a 16550A device, which fails on systems with a PL011.
> 
> Tests that wait for serial input (like its-migration) would hang
> indefinitely because __getchar() would always return -1.
> 
> Update uart0_init_acpi() to inspect the SPCR interface_type and set
> is_pl011_uart accordingly. Also define macros for the first four SPCR
> interface types in lib/acpi.h based on the Microsoft specification.
> 
> Link: https://learn.microsoft.com/en-us/windows-hardware/drivers/bringup/acpi-debug-port-table#table-3-debug-port-types-and-subtypes
> Assisted-by: Gemini:gemini-3.1-pro
> Signed-off-by: Colton Lewis <coltonlewis@google.com>
> ---
>  lib/acpi.h   | 6 ++++++
>  lib/arm/io.c | 3 +++
>  2 files changed, 9 insertions(+)
> 
> diff --git a/lib/acpi.h b/lib/acpi.h
> index 66e3062d..29b3edd5 100644
> --- a/lib/acpi.h
> +++ b/lib/acpi.h
> @@ -251,6 +251,12 @@ enum acpi_madt_type {
>  /* MADT Local APIC flags */
>  #define ACPI_MADT_ENABLED		(1)	/* 00: Processor is usable if set */
>  
> +/* Values for interface_type field in struct spcr_descriptor */
> +#define ACPI_SPCR_INTERFACE_TYPE_16550		0x0000
> +#define ACPI_SPCR_INTERFACE_TYPE_16450		0x0001
> +#define ACPI_SPCR_INTERFACE_TYPE_16550_32BIT	0x0002
> +#define ACPI_SPCR_INTERFACE_TYPE_ARM_PL011	0x0003

We could also use the defines from the Linux source tree, from
include/acpi/actbl1.h (look below struct acpi_dbg2_device).

I got a bit confused about the value 2 above, because in the Debug Port
Table 2 (DBG2) from the link, that value is for a 'MAX311xE SPI UART'. Not
saying that it's not the same thing as a 32 bit 16550 uart (or that it
is!), just saying that the names don't appear to be matching.

> +
>  struct spcr_descriptor {
>  	ACPI_TABLE_HEADER_DEF	/* ACPI common table header */
>  	u8 interface_type;	/* 0=full 16550, 1=subset of 16550 */
> diff --git a/lib/arm/io.c b/lib/arm/io.c
> index 836fa854..55102bd7 100644
> --- a/lib/arm/io.c
> +++ b/lib/arm/io.c
> @@ -81,6 +81,9 @@ static void uart0_init_acpi(void)
>  
>  	assert_msg(spcr, "Unable to find ACPI SPCR");
>  	uart0_base = ioremap(spcr->serial_port.address, spcr->serial_port.bit_width);
> +
> +	if (spcr->interface_type == ACPI_SPCR_INTERFACE_TYPE_ARM_PL011)
> +		is_pl011_uart = true;

The patch makes sense to me. With or without the Linux define names:

Reviewed-by: Alexandru Elisei <alexandru.elisei@arm.com>

Thanks,
Alex