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

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

[Heinrich Schuchardt]

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>
---
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

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

[Michael S. Tsirkin]

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

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

[Heinrich Schuchardt]

On 6/18/26 14:48, Michael S. Tsirkin wrote:
> 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!

I tried it. But it does not work:

After running all tests successfully with make check the script
/tests/data/acpi/rebuild-expected-aml.sh
told me that I should run make check.

That did not make sense to me.

I there a specific configuration needed to let make check write what 
rebuild-expected-aml.sh is looking for?

I was using

   ./configure --prefix=/usr/local --static 
--target-list="riscv64-linux-user"

and

   ./configure --prefix=/usr/local --static 
--target-list="loongarch64-linux-user"

Best regards

Heinrich

> 
>> ---
>> 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
> 

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

[Michael S. Tsirkin]

On Thu, Jun 18, 2026 at 03:26:40PM +0200, Heinrich Schuchardt wrote:
> On 6/18/26 14:48, Michael S. Tsirkin wrote:
> > 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!
> 
> I tried it. But it does not work:
> 
> After running all tests successfully with make check the script
> /tests/data/acpi/rebuild-expected-aml.sh
> told me that I should run make check.
> 
> That did not make sense to me.
> 
> I there a specific configuration needed to let make check write what
> rebuild-expected-aml.sh is looking for?
> 
> I was using
> 
>   ./configure --prefix=/usr/local --static
> --target-list="riscv64-linux-user"
> 
> and
> 
>   ./configure --prefix=/usr/local --static
> --target-list="loongarch64-linux-user"
> 
> Best regards
> 
> Heinrich


Because linux-user does not support these tests?
Drop the target list limitation and it will work.

> > 
> > > ---
> > > 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
> >