Re: [PATCH-for-10.0 5/5] qtest/bios-tables-test: Update aarch64/virt/APIC.its_off blob

[Gustavo Romero <gustavo.romero@linaro.org>]

Hi Phil,

On 4/2/25 07:34, Philippe Mathieu-Daudé wrote:
> On 2/4/25 08:45, Gustavo Romero wrote:
>> Hi Phil,
>>
>> On 3/31/25 19:12, Philippe Mathieu-Daudé wrote:
>>> Changes in the tables:
>>>
>>>    @@ -1,32 +1,32 @@
>>>     /*
>>>      * Intel ACPI Component Architecture
>>>      * AML/ASL+ Disassembler version 20240927 (64-bit version)
>>>      * Copyright (c) 2000 - 2023 Intel Corporation
>>>      *
>>>      * Disassembly of tests/data/acpi/aarch64/virt/APIC.its_off
>>>      *
>>>      * ACPI Data Table [APIC]
>>>      *
>>>      * Format: [HexOffset DecimalOffset ByteLength]  FieldName : FieldValue (in hex)
>>>      */
>>>
>>>     [000h 0000 004h]                   Signature : "APIC"    [Multiple APIC Description Table (MADT)]
>>>    -[004h 0004 004h]                Table Length : 000000B8
>>>    +[004h 0004 004h]                Table Length : 000000A4
>>>     [008h 0008 001h]                    Revision : 04
>>>    -[009h 0009 001h]                    Checksum : A7
>>>    +[009h 0009 001h]                    Checksum : EE
>>>     [00Ah 0010 006h]                      Oem ID : "BOCHS "
>>>     [010h 0016 008h]                Oem Table ID : "BXPC    "
>>>     [018h 0024 004h]                Oem Revision : 00000001
>>>     [01Ch 0028 004h]             Asl Compiler ID : "BXPC"
>>>     [020h 0032 004h]       Asl Compiler Revision : 00000001
>>>
>>>     [024h 0036 004h]          Local Apic Address : 00000000
>>>     [028h 0040 004h]       Flags (decoded below) : 00000000
>>>                              PC-AT Compatibility : 0
>>>
>>>     [02Ch 0044 001h]               Subtable Type : 0C [Generic Interrupt Distributor]
>>>     [02Dh 0045 001h]                      Length : 18
>>>     [02Eh 0046 002h]                    Reserved : 0000
>>>     [030h 0048 004h]       Local GIC Hardware ID : 00000000
>>>     [034h 0052 008h]                Base Address : 0000000008000000
>>>     [03Ch 0060 004h]              Interrupt Base : 00000000
>>>    @@ -49,37 +49,29 @@
>>>     [06Ch 0108 008h]    Virtual GIC Base Address : 0000000000000000
>>>     [074h 0116 008h] Hypervisor GIC Base Address : 0000000000000000
>>>     [07Ch 0124 004h]       Virtual GIC Interrupt : 00000019
>>>     [080h 0128 008h]  Redistributor Base Address : 0000000000000000
>>>     [088h 0136 008h]                   ARM MPIDR : 0000000000000000
>>>     [090h 0144 001h]            Efficiency Class : 00
>>>     [091h 0145 001h]                    Reserved : 00
>>>     [092h 0146 002h]      SPE Overflow Interrupt : 0000
>>>     [094h 0148 002h]              TRBE Interrupt : 100E
>>>
>>>     [094h 0148 001h]               Subtable Type : 0E [Generic Interrupt Redistributor]
>>>     [095h 0149 001h]                      Length : 10
>>>     [097h 0151 002h]                    Reserved : 0000
>>>     [098h 0152 008h]                Base Address : 00000000080A0000
>>>     [0A0h 0160 004h]                      Length : 00F60000
>>>
>>>    -[0A4h 0164 001h]               Subtable Type : 0F [Generic Interrupt Translator]
>>>    -[0A5h 0165 001h]                      Length : 14
>>>    -[0A7h 0167 002h]                    Reserved : 0000
>>>    -[0A8h 0168 004h]              Translation ID : 00000000
>>>    -[0ACh 0172 008h]                Base Address : 0000000008080000
>>>    -[0B4h 0180 004h]                    Reserved : 00000000
>>>    +Raw Table Data: Length 164 (0xA4)
>>>
>>>    -Raw Table Data: Length 184 (0xB8)
>>>    -
>>>    -    0000: 41 50 49 43 B8 00 00 00 04 A7 42 4F 43 48 53 20  // APIC......BOCHS
>>>    +    0000: 41 50 49 43 A4 00 00 00 04 EE 42 4F 43 48 53 20  // APIC......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 0C 18 00 00  // ................
>>>         0030: 00 00 00 00 00 00 00 08 00 00 00 00 00 00 00 00  // ................
>>>         0040: 04 00 00 00 0B 50 00 00 00 00 00 00 00 00 00 00  // .....P..........
>>>         0050: 01 00 00 00 00 00 00 00 17 00 00 00 00 00 00 00  // ................
>>>         0060: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  // ................
>>>         0070: 00 00 00 00 00 00 00 00 00 00 00 00 19 00 00 00  // ................
>>>         0080: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  // ................
>>>         0090: 00 00 00 00 0E 10 00 00 00 00 0A 08 00 00 00 00  // ................
>>>    -    00A0: 00 00 F6 00 0F 14 00 00 00 00 00 00 00 00 08 08  // ................
>>>    -    00B0: 00 00 00 00 00 00 00 00                          // ........
>>>    +    00A0: 00 00 F6 00                                      // ....
>>>
>>> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>>> ---
>>>   tests/qtest/bios-tables-test-allowed-diff.h |   1 -
>>>   tests/data/acpi/aarch64/virt/APIC.its_off   | Bin 184 -> 164 bytes
>>>   2 files changed, 1 deletion(-)
>>>
>>> diff --git a/tests/qtest/bios-tables-test-allowed-diff.h b/tests/ qtest/bios-tables-test-allowed-diff.h
>>> index bfc4d601243..dfb8523c8bf 100644
>>> --- a/tests/qtest/bios-tables-test-allowed-diff.h
>>> +++ b/tests/qtest/bios-tables-test-allowed-diff.h
>>> @@ -1,2 +1 @@
>>>   /* List of comma-separated changed AML files to ignore */
>>> -"tests/data/acpi/aarch64/virt/APIC.its_off",
>>> diff --git a/tests/data/acpi/aarch64/virt/APIC.its_off b/tests/data/ acpi/aarch64/virt/APIC.its_off
>>> index c37d05d6e05805304f10afe73eb7cb9100fcccfa..f24ac8fbff5261a52434abcfcff96dbdc7709de4 100644
>>> GIT binary patch
>>> delta 18
>>> ZcmdnNxP+0*F~HM#2?GNI%e#qOvj8xy1yKM1
>>>
>>> delta 39
>>> jcmZ3&xPy_)F~HM#2Ll5G%kqg_vqbnsfJ`vp;DE6Jpmzmj
>>
>> If the change affects other tables besides APIC (IORT and FACP in this series),
>> then I think that's the moment to update all the correct blobs (final versions)
>> and drop the list of blobs in bios-tables-test-allowed-diff.h. I also think it's
>> better to list all the table diffs in the commit message, not only the diff for
>> the APIC table.
> 
> When following the script steps with V=2, I only see diff change in the
> MADT table.

I was thinking of the case if empty blobs were added to 1/5. Indeed, FACP table does
not change, sorry for the confusion.

After the fix just APIC table changes, so the patch is correctly listing it in the
commit message and removing it from the "ignore list".

However, as I mentioned in 3/5, the IORT table is being gated as well by its_enabled(),
so after the fix no IORT table is generated by QEMU, hence tests/qtest/bios-tables-test.c
never sees/loads any IORT table to compare against IORT.its_off added in 1/5. There are
no test failures but IORT.its_off is a "dead blob". I think it should be removed in this
patch just like APIC table blob is updated, for consistence.


Cheers,
Gustavo
  
> Igor, am I missing something?
> 
>>
>> Cheers,
>> Gustavo
>