[PATCH v2 0/3] fix for two ACPI GTDT physical addresses

[PATCH v2 0/3] fix for two ACPI GTDT physical addresses

[Miguel Luis]

The ACPI GTDT table contains two invalid 64-bit physical addresses according to
the ACPI spec. 6.5 [1]. Those are the Counter Control Base physical address and
the Counter Read Base physical address. Those fields of the GTDT table should be
set to 0xFFFFFFFFFFFFFFFF if not provided, rather than 0x0.

[1]: https://uefi.org/specs/ACPI/6.5/05_ACPI_Software_Programming_Model.html#gtdt-table-structure

Changelog:

v2:
    Updated with collected tags from v1.

v1: https://lists.nongnu.org/archive/html/qemu-devel/2022-09/msg02847.html

Miguel Luis (3):
  tests/acpi: virt: allow acpi GTDT changes
  acpi: arm/virt: build_gtdt: fix invalid 64-bit physical addresses
  tests/acpi: virt: update ACPI GTDT binaries

 hw/arm/virt-acpi-build.c          |   5 ++---
 tests/data/acpi/virt/GTDT         | Bin 96 -> 96 bytes
 tests/data/acpi/virt/GTDT.memhp   | Bin 96 -> 96 bytes
 tests/data/acpi/virt/GTDT.numamem | Bin 96 -> 96 bytes
 4 files changed, 2 insertions(+), 3 deletions(-)

-- 
2.37.3

[PATCH v2 1/3] tests/acpi: virt: allow acpi GTDT changes

[Miguel Luis]

Step 3 from bios-tables-test.c documented procedure.

Signed-off-by: Miguel Luis <miguel.luis@oracle.com>
Acked-by: Ani Sinha <ani@anisinha.ca>
---
 tests/qtest/bios-tables-test-allowed-diff.h | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/tests/qtest/bios-tables-test-allowed-diff.h b/tests/qtest/bios-tables-test-allowed-diff.h
index dfb8523c8b..957bd1b4f6 100644
--- a/tests/qtest/bios-tables-test-allowed-diff.h
+++ b/tests/qtest/bios-tables-test-allowed-diff.h
@@ -1 +1,4 @@
 /* List of comma-separated changed AML files to ignore */
+"tests/data/acpi/virt/GTDT",
+"tests/data/acpi/virt/GTDT.memhp",
+"tests/data/acpi/virt/GTDT.numamem",
-- 
2.37.3

[PATCH v2 2/3] acpi: arm/virt: build_gtdt: fix invalid 64-bit physical addresses

[Miguel Luis]

Per the ACPI 6.5 specification, on the GTDT Table Structure, the Counter Control
Block Address and Counter Read Block Address fields of the GTDT table should be
set to 0xFFFFFFFFFFFFFFFF if not provided, rather than 0x0.

Fixes: 41041e57085 ("acpi: arm/virt: build_gtdt: use acpi_table_begin()/acpi_table_end() instead of build_header()")

Signed-off-by: Miguel Luis <miguel.luis@oracle.com>
Reviewed-by: Ani Sinha <ani@anisinha.ca>
---
 hw/arm/virt-acpi-build.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c
index 9b3aee01bf..13c6e3e468 100644
--- a/hw/arm/virt-acpi-build.c
+++ b/hw/arm/virt-acpi-build.c
@@ -592,8 +592,7 @@ build_gtdt(GArray *table_data, BIOSLinker *linker, VirtMachineState *vms)
     acpi_table_begin(&table, table_data);
 
     /* CntControlBase Physical Address */
-    /* FIXME: invalid value, should be 0xFFFFFFFFFFFFFFFF if not impl. ? */
-    build_append_int_noprefix(table_data, 0, 8);
+    build_append_int_noprefix(table_data, 0xFFFFFFFFFFFFFFFF, 8);
     build_append_int_noprefix(table_data, 0, 4); /* Reserved */
     /*
      * FIXME: clarify comment:
@@ -618,7 +617,7 @@ build_gtdt(GArray *table_data, BIOSLinker *linker, VirtMachineState *vms)
     /* Non-Secure EL2 timer Flags */
     build_append_int_noprefix(table_data, irqflags, 4);
     /* CntReadBase Physical address */
-    build_append_int_noprefix(table_data, 0, 8);
+    build_append_int_noprefix(table_data, 0xFFFFFFFFFFFFFFFF, 8);
     /* Platform Timer Count */
     build_append_int_noprefix(table_data, 0, 4);
     /* Platform Timer Offset */
-- 
2.37.3

[PATCH v2 3/3] tests/acpi: virt: update ACPI GTDT binaries

[Miguel Luis]

Step 6 & 7 of the bios-tables-test.c documented procedure.

Differences between disassembled ASL files for GTDT:

    @@ -13,14 +13,14 @@
     [000h 0000   4]                    Signature : "GTDT"    [Generic Timer Description Table]
     [004h 0004   4]                 Table Length : 00000060
     [008h 0008   1]                     Revision : 02
    -[009h 0009   1]                     Checksum : 8C
    +[009h 0009   1]                     Checksum : 9C
     [00Ah 0010   6]                       Oem ID : "BOCHS "
     [010h 0016   8]                 Oem Table ID : "BXPC    "
     [018h 0024   4]                 Oem Revision : 00000001
     [01Ch 0028   4]              Asl Compiler ID : "BXPC"
     [020h 0032   4]        Asl Compiler Revision : 00000001

    -[024h 0036   8]        Counter Block Address : 0000000000000000
    +[024h 0036   8]        Counter Block Address : FFFFFFFFFFFFFFFF
     [02Ch 0044   4]                     Reserved : 00000000

     [030h 0048   4]         Secure EL1 Interrupt : 0000001D
    @@ -46,16 +46,16 @@
                                     Trigger Mode : 0
                                         Polarity : 0
                                        Always On : 0
    -[050h 0080   8]   Counter Read Block Address : 0000000000000000
    +[050h 0080   8]   Counter Read Block Address : FFFFFFFFFFFFFFFF

     [058h 0088   4]         Platform Timer Count : 00000000
     [05Ch 0092   4]        Platform Timer Offset : 00000000

     Raw Table Data: Length 96 (0x60)

    -    0000: 47 54 44 54 60 00 00 00 02 8C 42 4F 43 48 53 20  // GTDT`.....BOCHS
    +    0000: 47 54 44 54 60 00 00 00 02 9C 42 4F 43 48 53 20  // GTDT`.....BOCHS
         0010: 42 58 50 43 20 20 20 20 01 00 00 00 42 58 50 43  // BXPC    ....BXPC
    -    0020: 01 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  // ................
    +    0020: 01 00 00 00 FF FF FF FF FF FF FF FF 00 00 00 00  // ................
         0030: 1D 00 00 00 00 00 00 00 1E 00 00 00 04 00 00 00  // ................
         0040: 1B 00 00 00 00 00 00 00 1A 00 00 00 00 00 00 00  // ................
    -    0050: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  // ................
    +    0050: FF FF FF FF FF FF FF FF 00 00 00 00 00 00 00 00  // ................

Signed-off-by: Miguel Luis <miguel.luis@oracle.com>
Acked-by: Ani Sinha <ani@anisinha.ca>
---
 tests/data/acpi/virt/GTDT                   | Bin 96 -> 96 bytes
 tests/data/acpi/virt/GTDT.memhp             | Bin 96 -> 96 bytes
 tests/data/acpi/virt/GTDT.numamem           | Bin 96 -> 96 bytes
 tests/qtest/bios-tables-test-allowed-diff.h |   3 ---
 4 files changed, 3 deletions(-)

diff --git a/tests/data/acpi/virt/GTDT b/tests/data/acpi/virt/GTDT
index 9408b71b59c0e0f2991c0053562280155b47bc0b..6f8cb9b8f30b55f4c93fe515982621e3db50feb2 100644
GIT binary patch
delta 45
kcmYdD;BpUf2}xjJU|^avkxPo>KNL*VQ4xT#fs$YV0LH=;ng9R*

delta 45
jcmYdD;BpUf2}xjJU|{N*$R))AWPrg$9Tfo>8%6^Foy!E8

diff --git a/tests/data/acpi/virt/GTDT.memhp b/tests/data/acpi/virt/GTDT.memhp
index 9408b71b59c0e0f2991c0053562280155b47bc0b..6f8cb9b8f30b55f4c93fe515982621e3db50feb2 100644
GIT binary patch
delta 45
kcmYdD;BpUf2}xjJU|^avkxPo>KNL*VQ4xT#fs$YV0LH=;ng9R*

delta 45
jcmYdD;BpUf2}xjJU|{N*$R))AWPrg$9Tfo>8%6^Foy!E8

diff --git a/tests/data/acpi/virt/GTDT.numamem b/tests/data/acpi/virt/GTDT.numamem
index 9408b71b59c0e0f2991c0053562280155b47bc0b..6f8cb9b8f30b55f4c93fe515982621e3db50feb2 100644
GIT binary patch
delta 45
kcmYdD;BpUf2}xjJU|^avkxPo>KNL*VQ4xT#fs$YV0LH=;ng9R*

delta 45
jcmYdD;BpUf2}xjJU|{N*$R))AWPrg$9Tfo>8%6^Foy!E8

diff --git a/tests/qtest/bios-tables-test-allowed-diff.h b/tests/qtest/bios-tables-test-allowed-diff.h
index 957bd1b4f6..dfb8523c8b 100644
--- a/tests/qtest/bios-tables-test-allowed-diff.h
+++ b/tests/qtest/bios-tables-test-allowed-diff.h
@@ -1,4 +1 @@
 /* List of comma-separated changed AML files to ignore */
-"tests/data/acpi/virt/GTDT",
-"tests/data/acpi/virt/GTDT.memhp",
-"tests/data/acpi/virt/GTDT.numamem",
-- 
2.37.3

Re: [PATCH v2 0/3] fix for two ACPI GTDT physical addresses

[Ani Sinha]

On Fri, Oct 7, 2022 at 8:16 PM Miguel Luis <miguel.luis@oracle.com> wrote:
>
> The ACPI GTDT table contains two invalid 64-bit physical addresses according to
> the ACPI spec. 6.5 [1]. Those are the Counter Control Base physical address and
> the Counter Read Base physical address. Those fields of the GTDT table should be
> set to 0xFFFFFFFFFFFFFFFF if not provided, rather than 0x0.
>
> [1]: https://uefi.org/specs/ACPI/6.5/05_ACPI_Software_Programming_Model.html#gtdt-table-structure
>
> Changelog:
>
> v2:
>     Updated with collected tags from v1.

For future reference, there is no need to send out a new version with
just the tags added. The tooling make sure that the tags are collected
correctly from the last version.

>
> v1: https://lists.nongnu.org/archive/html/qemu-devel/2022-09/msg02847.html
>
> Miguel Luis (3):
>   tests/acpi: virt: allow acpi GTDT changes
>   acpi: arm/virt: build_gtdt: fix invalid 64-bit physical addresses
>   tests/acpi: virt: update ACPI GTDT binaries
>
>  hw/arm/virt-acpi-build.c          |   5 ++---
>  tests/data/acpi/virt/GTDT         | Bin 96 -> 96 bytes
>  tests/data/acpi/virt/GTDT.memhp   | Bin 96 -> 96 bytes
>  tests/data/acpi/virt/GTDT.numamem | Bin 96 -> 96 bytes
>  4 files changed, 2 insertions(+), 3 deletions(-)
>
> --
> 2.37.3
>

Re: [External] : Re: [PATCH v2 0/3] fix for two ACPI GTDT physical addresses

[Miguel Luis]

> On 7 Oct 2022, at 15:21, Ani Sinha <ani@anisinha.ca> wrote:
> 
> On Fri, Oct 7, 2022 at 8:16 PM Miguel Luis <miguel.luis@oracle.com> wrote:
>> 
>> The ACPI GTDT table contains two invalid 64-bit physical addresses according to
>> the ACPI spec. 6.5 [1]. Those are the Counter Control Base physical address and
>> the Counter Read Base physical address. Those fields of the GTDT table should be
>> set to 0xFFFFFFFFFFFFFFFF if not provided, rather than 0x0.
>> 
>> [1]: https://urldefense.com/v3/__https://uefi.org/specs/ACPI/6.5/05_ACPI_Software_Programming_Model.html*gtdt-table-structure__;Iw!!ACWV5N9M2RV99hQ!I-YqmAwYNhk19YHxcbjQBMwEE9a8rZOvufvOOonAPEtgTynOYOf5AyYKLTTGJ2RRzsjvkjIuleSubpg$  
>> 
>> Changelog:
>> 
>> v2:
>>    Updated with collected tags from v1.
> 
> For future reference, there is no need to send out a new version with
> just the tags added. The tooling make sure that the tags are collected
> correctly from the last version.
> 

Great! Thanks for the tip which is very helpful.

Miguel

>> 
>> v1: https://urldefense.com/v3/__https://lists.nongnu.org/archive/html/qemu-devel/2022-09/msg02847.html__;!!ACWV5N9M2RV99hQ!I-YqmAwYNhk19YHxcbjQBMwEE9a8rZOvufvOOonAPEtgTynOYOf5AyYKLTTGJ2RRzsjvkjIulSis4m4$  
>> 
>> Miguel Luis (3):
>>  tests/acpi: virt: allow acpi GTDT changes
>>  acpi: arm/virt: build_gtdt: fix invalid 64-bit physical addresses
>>  tests/acpi: virt: update ACPI GTDT binaries
>> 
>> hw/arm/virt-acpi-build.c          |   5 ++---
>> tests/data/acpi/virt/GTDT         | Bin 96 -> 96 bytes
>> tests/data/acpi/virt/GTDT.memhp   | Bin 96 -> 96 bytes
>> tests/data/acpi/virt/GTDT.numamem | Bin 96 -> 96 bytes
>> 4 files changed, 2 insertions(+), 3 deletions(-)
>> 
>> --
>> 2.37.3
>>