[PATCH v4 0/2] hw/acpi: correct field sequence in SPCR table

[PATCH v4 0/2] 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.

v4:
	Reproduce the test data changes using rebuild-expected-aml.sh
	creating the same diff as v3 but in two separate patches.
v3:
	Add test data changes	
v2:
	Update commit message

Heinrich Schuchardt (2):
  hw/acpi: correct field sequence in SPCR table
  tests: update SPCR loongarch64 and riscv64 test data

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

-- 
2.53.0

[PATCH v4 1/2] 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>
---
v4:
	Reproduce the test data changes using rebuild-expected-aml.sh
	creating the same diff as v3 but in two separate patches.
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 ++--
 1 file 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 */
-- 
2.53.0

[PATCH v4 2/2] tests: update SPCR loongarch64 and riscv64 test data

[Heinrich Schuchardt]

On LoongArch and RISC-V the SPCR test data contained:

    Terminal Type : 00
    Language : 03

The corrected values are:

    Terminal Type : 03
    Language : 00

See the specification of the table in
https://learn.microsoft.com/en-us/windows-hardware/drivers/bringup/serial-port-console-redirection-table

The ACPI table data was rebuilt with
tests/data/acpi/rebuild-expected-aml.sh.

Signed-off-by: Heinrich Schuchardt <heinrich.schuchardt@canonical.com>
---
v4:
	Reproduce the test data changes using rebuild-expected-aml.sh
	creating the same diff as v3 but in two separate patches.
v3:
	Update test data
	https://lore.kernel.org/qemu-devel/20260618124020.640794-1-heinrich.schuchardt@canonical.com/
---
 tests/data/acpi/loongarch64/virt/SPCR | Bin 80 -> 80 bytes
 tests/data/acpi/riscv64/virt/SPCR     | Bin 90 -> 90 bytes
 2 files changed, 0 insertions(+), 0 deletions(-)

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 v4 1/2] hw/acpi: correct field sequence in SPCR table

[Alistair Francis]

On Mon, Jun 22, 2026 at 9:18 PM Heinrich Schuchardt
<heinrich.schuchardt@canonical.com> 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>
> ---
> v4:
>         Reproduce the test data changes using rebuild-expected-aml.sh
>         creating the same diff as v3 but in two separate patches.
> 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 ++--
>  1 file 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);

Won't this break tests? Don't you need to disable the test first (then
re-enable it and fix it in patch 3)

Alistair

>      /* PCI Device ID  */
>      build_append_int_noprefix(table_data, f->pci_device_id, 2);
>      /* PCI Vendor ID */
> --
> 2.53.0
>

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

[Heinrich Schuchardt]

On 6/29/26 02:58, Alistair Francis wrote:
> On Mon, Jun 22, 2026 at 9:18 PM Heinrich Schuchardt
> <heinrich.schuchardt@canonical.com> 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>
>> ---
>> v4:
>>          Reproduce the test data changes using rebuild-expected-aml.sh
>>          creating the same diff as v3 but in two separate patches.
>> 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 ++--
>>   1 file 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);
> 
> Won't this break tests? Don't you need to disable the test first (then
> re-enable it and fix it in patch 3)

Hello Alistair,

It would be helpful if /tests/qtest/bios-tables-test.c would clearly 
point out if changes to tests/qtest/bios-tables-test-allowed-diff.h and 
their reversion should be in the submitted patch series or not and what 
the acceptance criteria for the patch series are.

I was not able to identify this from the existing text. So I eliminated 
those redundant changes.

Best regards

Heinrich

> 
> Alistair
> 
>>       /* PCI Device ID  */
>>       build_append_int_noprefix(table_data, f->pci_device_id, 2);
>>       /* PCI Vendor ID */
>> --
>> 2.53.0
>>

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

[Michael S. Tsirkin]

On Mon, Jun 29, 2026 at 08:12:12AM +0200, Heinrich Schuchardt wrote:
> On 6/29/26 02:58, Alistair Francis wrote:
> > On Mon, Jun 22, 2026 at 9:18 PM Heinrich Schuchardt
> > <heinrich.schuchardt@canonical.com> 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>
> > > ---
> > > v4:
> > >          Reproduce the test data changes using rebuild-expected-aml.sh
> > >          creating the same diff as v3 but in two separate patches.
> > > 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 ++--
> > >   1 file 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);
> > 
> > Won't this break tests? Don't you need to disable the test first (then
> > re-enable it and fix it in patch 3)
> 
> Hello Alistair,
> 
> It would be helpful if /tests/qtest/bios-tables-test.c would clearly point
> out if changes to tests/qtest/bios-tables-test-allowed-diff.h and their
> reversion should be in the submitted patch series or not and what the
> acceptance criteria for the patch series are.
> 

1. add to allowed
2. code change
3. update expected
4. remove from allowed

1+2 and 3+4 can be combined


> I was not able to identify this from the existing text. So I eliminated
> those redundant changes.
> 
> Best regards
> 
> Heinrich

they are not redundant - they are to keep bisect working.
i don't know how to make it clearer.

> > 
> > Alistair
> > 
> > >       /* PCI Device ID  */
> > >       build_append_int_noprefix(table_data, f->pci_device_id, 2);
> > >       /* PCI Vendor ID */
> > > --
> > > 2.53.0
> > > 
> 
> 
>