"Michael S. Tsirkin" <m...@redhat.com> writes:
> On Wed, May 17, 2023 at 04:16:47PM +0100, Alex Bennée wrote:
>>
>> Ani Sinha <anisi...@redhat.com> writes:
>>
>> >> On 17-May-2023, at 8:06 PM, Michael S. Tsirkin <m...@redhat.com> wrote:
>> >>
>> >> On Wed, May 17, 2023 at 07:57:53PM +0530, Ani Sinha wrote:
>> >>>
>> >>>
>> >>>> On 17-May-2023, at 7:47 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.
>>
>> Usually we work the other way by checking at configure time and skipping
>> the feature if the prerequisites are not in place. We do this with gdb:
>>
>> ../../configure
>> --gdb=/home/alex/src/tools/binutils-gdb.git/builds/all/install/bin/gdb
>>
>> which checks gdb is at least new enough to support the features we need:
>>
>> if test -n "$gdb_bin"; then
>> gdb_version=$($gdb_bin --version | head -n 1)
>> if version_ge ${gdb_version##* } 9.1; then
>> echo "HAVE_GDB_BIN=$gdb_bin" >> $config_host_mak
>> gdb_arches=$("$source_path/scripts/probe-gdb-support.py" $gdb_bin)
>> else
>> gdb_bin=""
>> fi
>> fi
>>
>> >>>>> - 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.
>>
>> I think configure should be checking if iasl is new enough and reporting
>> to the user at configure time they need to do something different. We
>> don't want to attempt to run tests that will fail unless the user has
>> added the right magic to their environment.
>
> iasl is a disassembler we trigger for user convenience in case tests
> fail. It will never cause tests to fail.
Fair enough. But I still think the place to report it is in configure.
Maybe something like:
iasl : /usr/bin/iasl (version 20200925, might not
handle all ACPI)
in the Host Binaries section. Re-configuring shouldn't cause too much of
the build to be regenerated although we could certainly do better in
this regard.
--
Alex Bennée
Virtualisation Tech Lead @ Linaro
