> On 17-May-2023, at 10:13 PM, Bernhard Beschow <shen...@gmail.com> wrote:
>
>
>
> Am 17. Mai 2023 12:07:51 UTC schrieb Ani Sinha <anisi...@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 -- l
Submodules are disliked with all fervour by the community. There is a long
thread somewhere in the mailing list.
> ike 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 <anisi...@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);
>>
>
