> On 21-May-2023, at 8:21 PM, Ani Sinha <anisi...@redhat.com> wrote:
>
>
>
>> On 21-May-2023, at 2:24 PM, Michael S. Tsirkin <m...@redhat.com> wrote:
>>
>> On Wed, May 17, 2023 at 05:37:51PM +0530, Ani Sinha wrote:
>>> 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. 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>
>>
>> I don't much like it that the default is now a bit harder
>> to run. Case of playing with iasl is really esotetic.
>> I propose a simpler idea.
>> - add config-iasl.h with only CONFIG_IASL set to path
>> - include from bios test
>>
>> Now if you change path only bios test is rebuilt.
>>
>> Hmm?
>
> We can do this.
I sent a patch " acpi/tests/bios-tables-test: make iasl tool handling simpler”
I think we should just go ahead with this simple idea and be done with this.
> If an environment variable CONFIG_IASL is set or a command line is passed (we
> can do one or the other), use that. Else use a default path /usr/bin/iasl.
> This way we do not even need to rebuild the test if the developer wishes to
> use another iasl binary from a different location.
> I prefer env var slightly because its easier to implement from meson. Test
> specific command lines in meson.build would need more work.
>
>>
>>
>>> ---
>>> 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);
>>>
>>> --
>>> 2.39.1
