| Message ID | 20250331221239.87150-3-philmd@linaro.org |
|---|---|
| State | New |
| Headers | show |
| Series | hw/arm/virt-acpi: Do not advertise disabled GIC ITS in MADT table | expand |
Hi Phil, On 3/31/25 19:12, Philippe Mathieu-Daudé wrote: > Prepare for ACPI table change in aarch64/virt/APIC.its_off. The comment could be smth like: Ignore APIC.its_off expected table (blob) for now until we update it later, after fixing the code that generates this table correctly. ? > Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org> > --- > tests/qtest/bios-tables-test-allowed-diff.h | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/tests/qtest/bios-tables-test-allowed-diff.h b/tests/qtest/bios-tables-test-allowed-diff.h > index dfb8523c8bf..bfc4d601243 100644 > --- a/tests/qtest/bios-tables-test-allowed-diff.h > +++ b/tests/qtest/bios-tables-test-allowed-diff.h > @@ -1 +1,2 @@ > /* List of comma-separated changed AML files to ignore */ > +"tests/data/acpi/aarch64/virt/APIC.its_off", I think this patch should be merged into 1/2, accordingly to my comment in 1/5. FACP and IORT .its_off files should be added to the list as well. Btw, IMHO the name of this header is a tad misleading, because actually "allowed-diff" means that "we allow the machine's table to be different from the tables listed in this header", so it doesn't look like an allowlist (whitelist), it works more like an ignore list? Cheers, Gustavo
On 2/4/25 08:43, Gustavo Romero wrote: > Hi Phil, > > On 3/31/25 19:12, Philippe Mathieu-Daudé wrote: >> Prepare for ACPI table change in aarch64/virt/APIC.its_off. > > The comment could be smth like: > > Ignore APIC.its_off expected table (blob) for now until > we update it later, after fixing the code that generates > this table correctly. > > ? > > >> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org> >> --- >> tests/qtest/bios-tables-test-allowed-diff.h | 1 + >> 1 file changed, 1 insertion(+) >> >> diff --git a/tests/qtest/bios-tables-test-allowed-diff.h b/tests/ >> qtest/bios-tables-test-allowed-diff.h >> index dfb8523c8bf..bfc4d601243 100644 >> --- a/tests/qtest/bios-tables-test-allowed-diff.h >> +++ b/tests/qtest/bios-tables-test-allowed-diff.h >> @@ -1 +1,2 @@ >> /* List of comma-separated changed AML files to ignore */ >> +"tests/data/acpi/aarch64/virt/APIC.its_off", > > I think this patch should be merged into 1/2, accordingly to my > comment in 1/5. FACP and IORT .its_off files should be added to the > list as well. No, otherwise the test added in previous patch fails. > > Btw, IMHO the name of this header is a tad misleading, because actually > "allowed-diff" means that "we allow the machine's table to be different > from the tables listed in this header", so it doesn't look like an > allowlist (whitelist), it works more like an ignore list? > > > Cheers, > Gustavo
Hi Phil, On 4/2/25 07:31, Philippe Mathieu-Daudé wrote: > On 2/4/25 08:43, Gustavo Romero wrote: >> Hi Phil, >> >> On 3/31/25 19:12, Philippe Mathieu-Daudé wrote: >>> Prepare for ACPI table change in aarch64/virt/APIC.its_off. >> >> The comment could be smth like: >> >> Ignore APIC.its_off expected table (blob) for now until >> we update it later, after fixing the code that generates >> this table correctly. >> >> ? >> >> >>> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org> >>> --- >>> tests/qtest/bios-tables-test-allowed-diff.h | 1 + >>> 1 file changed, 1 insertion(+) >>> >>> diff --git a/tests/qtest/bios-tables-test-allowed-diff.h b/tests/ qtest/bios-tables-test-allowed-diff.h >>> index dfb8523c8bf..bfc4d601243 100644 >>> --- a/tests/qtest/bios-tables-test-allowed-diff.h >>> +++ b/tests/qtest/bios-tables-test-allowed-diff.h >>> @@ -1 +1,2 @@ >>> /* List of comma-separated changed AML files to ignore */ >>> +"tests/data/acpi/aarch64/virt/APIC.its_off", >> >> I think this patch should be merged into 1/2, accordingly to my >> comment in 1/5. FACP and IORT .its_off files should be added to the >> list as well. > > No, otherwise the test added in previous patch fails. I can't see how adding the tests to the list in tests/qtest/bios-tables-test-allowed-diff.h can cause any failure. The list in this header file works as a "ignore list", so even if the .its_off blobs in 1/5 were empty (for instance) the test would pass if they are in this list. That said, as per my comments in 1/5, this preparation is correct to me: the fix will cause changes to APIC table so the current expected blob (committed) needs to be ignored until it gets updated later, in 5/5. Reviewed-by: Gustavo Romero <gustavo.romero@linaro.org> Cheers, Gustavo
diff --git a/tests/qtest/bios-tables-test-allowed-diff.h b/tests/qtest/bios-tables-test-allowed-diff.h index dfb8523c8bf..bfc4d601243 100644 --- a/tests/qtest/bios-tables-test-allowed-diff.h +++ b/tests/qtest/bios-tables-test-allowed-diff.h @@ -1 +1,2 @@ /* List of comma-separated changed AML files to ignore */ +"tests/data/acpi/aarch64/virt/APIC.its_off",
Prepare for ACPI table change in aarch64/virt/APIC.its_off. Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org> --- tests/qtest/bios-tables-test-allowed-diff.h | 1 + 1 file changed, 1 insertion(+)