Re: [PATCH] acpi/tests/bios-tables-test: add an environment variable for iasl location

[Bernhard Beschow]

Am 17. Mai 2023 12:07:51 UTC schrieb Ani Sinha <anisinha@redhat.com>:
>Currently the meson based QEMU build process locates the iasl binary from the
>current PATH and other locations [1] and uses that to set CONFIG_IASL which is
>then used by the test.
>
>This has two disadvantages:
> - If iasl was not previously installed in the PATH, one has to install iasl
>   and rebuild QEMU in order to pick up the iasl location. One cannot simply
>   use the existing bios-tables-test binary because CONFIG_IASL is only set
>   during the QEMU build time by meson and then bios-tables-test has to be
>   rebuilt with CONFIG_IASL set in order to use iasl.
> - Sometimes, the stock iasl that comes with distributions is simply not good
>   enough because it does not support the latest ACPI changes - newly
>   introduced tables or new table attributes etc. In order to test ACPI code
>   in QEMU, one has to clone the latest acpica upstream repository and
>   rebuild iasl in order to get support for it. In those cases, one may want
>   the test to use the iasl binary from a non-standard location.
>
>In order to overcome the above two disadvantages, we introduce a new
>environment variable IASL_PATH that can be set by the tester pointing to an
>possibly non-standard iasl binary location.

Why not add a submodule -- like we do with dtc -- and use that? Then we could 
possibly convert the ACPI blobs used in bios-tables-test into text files which 
would make AML patches a bit more comprehensible. We also didn't have to trust 
the commit messages to contain the actual change because one would see it right 
in the patch.

Best regards,
Bernhard

>Bios-tables-test then uses this
>environment variable to set its iasl location, possibly also overriding the
>location that was pointed to by CONFIG_IASL that was set by meson. This way
>developers can not only use this new environment variable to set iasl
>location to quickly run bios-tables-test but also can point the test to a
>custom iasl if required.
>
>[1] https://mesonbuild.com/Reference-manual_functions.html#find_program
>
>Signed-off-by: Ani Sinha <anisinha@redhat.com>
>---
> tests/qtest/bios-tables-test.c | 14 ++++++++++++++
> 1 file changed, 14 insertions(+)
>
>sample runs:
>
>$ QTEST_QEMU_BINARY=./qemu-system-x86_64 V=2 ./tests/qtest/bios-tables-test
>...
>acpi-test: Warning! APIC binary file mismatch. Actual [aml:/tmp/aml-DLHA51], 
>Expected [aml:tests/data/acpi/pc/APIC].
>See source file tests/qtest/bios-tables-test.c for instructions on how to 
>update expected files.
>Using iasl: /usr/bin/iasl
>acpi-test: Warning! APIC mismatch. Actual [asl:/tmp/asl-L9GA51.dsl, 
>aml:/tmp/aml-DLHA51], Expected [asl:/tmp/asl-10EA51.dsl, 
>aml:tests/data/acpi/pc/APIC].
>
>$ QTEST_QEMU_BINARY=./qemu-system-x86_64 V=2 
>IASL_PATH=/home/anisinha/workspace/acpica/generate/unix/bin/iasl 
>./tests/qtest/bios-tables-test
>...
>acpi-test: Warning! APIC binary file mismatch. Actual [aml:/tmp/aml-5CQ341], 
>Expected [aml:tests/data/acpi/pc/APIC].
>See source file tests/qtest/bios-tables-test.c for instructions on how to 
>update expected files.
>User has provided an iasl path, using that: 
>/home/anisinha/workspace/acpica/generate/unix/bin/iasl
>acpi-test: Warning! APIC mismatch. Actual [asl:/tmp/asl-2GQ341.dsl, 
>aml:/tmp/aml-5CQ341], Expected [asl:/tmp/asl-IBR341.dsl, 
>aml:tests/data/acpi/pc/APIC].
>
>diff --git a/tests/qtest/bios-tables-test.c b/tests/qtest/bios-tables-test.c
>index 7fd88b0e9c..37e8e484cb 100644
>--- a/tests/qtest/bios-tables-test.c
>+++ b/tests/qtest/bios-tables-test.c
>@@ -440,7 +440,12 @@ static void test_acpi_asl(test_data *data)
>     AcpiSdtTable *sdt, *exp_sdt;
>     test_data exp_data = {};
>     gboolean exp_err, err, all_tables_match = true;
>+    const char *user_iasl_path = getenv("IASL_PATH");
> 
>+    /* if user has provided a path to iasl, use that */
>+    if (user_iasl_path) {
>+        iasl = user_iasl_path;
>+    }
>     exp_data.tables = load_expected_aml(data);
>     dump_aml_files(data, false);
>     for (i = 0; i < data->tables->len; ++i) {
>@@ -473,6 +478,15 @@ static void test_acpi_asl(test_data *data)
>             continue;
>         }
> 
>+        if (verbosity_level >= 2) {
>+            if (user_iasl_path) {
>+                fprintf(stderr, "User has provided an iasl path," \
>+                        "using that: %s\n", user_iasl_path);
>+            } else {
>+                fprintf(stderr, "Using iasl: %s\n", iasl);
>+            }
>+        }
>+
>         err = load_asl(data->tables, sdt);
>         asl = normalize_asl(sdt->asl);
>