Re: [PATCH v3 1/1] hw/acpi: correct field sequence in SPCR table

["Michael S. Tsirkin" <mst@redhat.com>]

On Thu, Jun 18, 2026 at 02:40:20PM +0200, Heinrich Schuchardt wrote:
> On LoongArch and RISC-V invalid SPCR tables are created:
> 
> Terminal Type : 00
> Language : 03
> 
> The correct values are:
> 
> Terminal Type : 03
> Language : 00
> 
> This is due to commit 7dd0b070fa09 ("hw/arm/virt-acpi-build.c: Migrate
> SPCR creation to common location") that swapped the fields.
> 
> See the specification of the table in
> https://learn.microsoft.com/en-us/windows-hardware/drivers/bringup/serial-port-console-redirection-table
> 
> This page shows version 1.10. But the sequence of the fields was not changed
> since version 1.0.
> 
> Our LoongArch and ARM code uses version 1.07 of the specification.
> Our RISC-V code uses version 1.10 of the specification.
> 
> Fixes: 7dd0b070fa09 ("hw/arm/virt-acpi-build.c: Migrate SPCR creation to common location")
> Origin: https://lore.kernel.org/qemu-devel/20260326121947.51200-1-heinrich.schuchardt@canonical.com/T/#u
> Bug-Ubuntu: https://bugs.launchpad.net/ubuntu/+source/qemu/+bug/2146419
> Signed-off-by: Heinrich Schuchardt <heinrich.schuchardt@canonical.com>

Thanks for the patch!
Yet something to improve:

That's not the way to submit expected table changes -
it would prevent backporting and rebasing.

Pls follow instructions in ./tests/qtest/bios-tables-test.c

Thanks!

> ---
> v3:
> 	update test data
> v2:
> 	Mention the different specification versions used by our code
> 	https://lore.kernel.org/qemu-devel/20260326144835.67911-1-heinrich.schuchardt@canonical.com/
> v1:
> 	https://lore.kernel.org/qemu-devel/20260326121947.51200-1-heinrich.schuchardt@canonical.com/T/#u
> ---
>  hw/acpi/aml-build.c                   |   4 ++--
>  tests/data/acpi/loongarch64/virt/SPCR | Bin 80 -> 80 bytes
>  tests/data/acpi/riscv64/virt/SPCR     | Bin 90 -> 90 bytes
>  3 files changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/hw/acpi/aml-build.c b/hw/acpi/aml-build.c
> index 9b3cdd3781..990abc64cd 100644
> --- a/hw/acpi/aml-build.c
> +++ b/hw/acpi/aml-build.c
> @@ -2144,10 +2144,10 @@ void build_spcr(GArray *table_data, BIOSLinker *linker,
>      build_append_int_noprefix(table_data, f->stop_bits, 1);
>      /* Flow Control */
>      build_append_int_noprefix(table_data, f->flow_control, 1);
> -    /* Language */
> -    build_append_int_noprefix(table_data, f->language, 1);
>      /* Terminal Type */
>      build_append_int_noprefix(table_data, f->terminal_type, 1);
> +    /* Language */
> +    build_append_int_noprefix(table_data, f->language, 1);
>      /* PCI Device ID  */
>      build_append_int_noprefix(table_data, f->pci_device_id, 2);
>      /* PCI Vendor ID */
> diff --git a/tests/data/acpi/loongarch64/virt/SPCR b/tests/data/acpi/loongarch64/virt/SPCR
> index 3cc9bbcfb8051e632592d9db0fe3dba0af53ed8d..7bb819cd0d2ad20269e40e4a738709a45260cbb2 100644
> GIT binary patch
> delta 23
> TcmWFtm|!Qw%<vxw7?1z}S|J61
> 
> delta 23
> TcmWFtm|!Qw!2BNw7?1z}S{?;}
> 
> diff --git a/tests/data/acpi/riscv64/virt/SPCR b/tests/data/acpi/riscv64/virt/SPCR
> index 09617f8793a6f7b1f08172f735b58aa748671540..59d2c8f7f215a604612cbd0294c18bc6301e208a 100644
> GIT binary patch
> delta 10
> Rcma!wnqbGo%rMbG3IGpM0&@TW
> 
> delta 10
> Rcma!wnqbGoz&z1G3IGpJ0&@TW
> 
> -- 
> 2.53.0