Re: [Qemu-devel] [PATCH v3 4/5] tests/uefi-test-tools: add build scripts

[Laszlo Ersek]

On 02/04/19 18:47, Michael S. Tsirkin wrote:
> On Mon, Feb 04, 2019 at 05:03:24PM +0100, Laszlo Ersek wrote:
>> Introduce the following build scripts under "tests/uefi-test-tools":
>>
>> * "build.sh" builds a single module (a UEFI application) from
>>   UefiTestToolsPkg, for a single QEMU emulation target.
>>
>>   "build.sh" relies on cross-compilers when the emulation target and the
>>   build host architecture don't match. The cross-compiler prefix is
>>   computed according to a fixed, Linux-specific pattern. No attempt is
>>   made to copy or reimplement the GNU Make magic from "qemu/roms/Makefile"
>>   for cross-compiler prefix determination. The reason is that the build
>>   host OSes that are officially supported by edk2, and those that are
>>   supported by QEMU, intersect only in Linux. (Note that the UNIXGCC
>>   toolchain is being removed from edk2,
>>   <https://bugzilla.tianocore.org/show_bug.cgi?id=1377>.)
>>
>> * "Makefile" currently builds the "UefiTestToolsPkg/BiosTablesTest"
>>   application, for arm, aarch64, i386, and x86_64, with the help of
>>   "build.sh".
>>
>>   "Makefile" turns each resultant UEFI executable into a UEFI-bootable,
>>   qcow2-compressed ISO image. The ISO images are output as
>>   "tests/data/uefi-boot-images/bios-tables-test.<TARGET>.iso.qcow2".
>>
>>   Each ISO image should be passed to QEMU as follows:
>>
>>     -drive id=boot-cd,if=none,readonly,format=qcow2,file=$ISO \
>>     -device virtio-scsi-pci,id=scsi0 \
>>     -device scsi-cd,drive=boot-cd,bus=scsi0.0,bootindex=0 \
>>
>>   "Makefile" assumes that "mkdosfs", "mtools", and "genisoimage" are
>>   present.
>>
>> Cc: "Michael S. Tsirkin" <address@hidden>
>> Cc: Ard Biesheuvel <address@hidden>
>> Cc: Gerd Hoffmann <address@hidden>
>> Cc: Igor Mammedov <address@hidden>
>> Cc: Philippe Mathieu-Daudé <address@hidden>
>> Cc: Shannon Zhao <address@hidden>
>> Signed-off-by: Laszlo Ersek <address@hidden>
>> Reviewed-by: Philippe Mathieu-Daudé <address@hidden>
>> Tested-by: Philippe Mathieu-Daudé <address@hidden>
>> ---
>>
>> Notes:
>>     v3:
>>     - explicitly mark the "./build.sh" recipe as recursive, with the "+"
>>       indicator; document it in a comment [Phil]
>>     - pick up R-b, T-b [Phil]
>>
>>     v2:
>>     - add the .NOTPARALLEL target [Phil, help-make, edk2-devel]
>>
>>  tests/uefi-test-tools/Makefile   | 106 ++++++++++++++
>>  tests/uefi-test-tools/.gitignore |   3 +
>>  tests/uefi-test-tools/build.sh   | 145 ++++++++++++++++++++
>>  3 files changed, 254 insertions(+)
>>
>> diff --git a/tests/uefi-test-tools/Makefile b/tests/uefi-test-tools/Makefile
>> new file mode 100644
>> index 000000000000..1d78bc14d51a
>> --- /dev/null
>> +++ b/tests/uefi-test-tools/Makefile
>> @@ -0,0 +1,106 @@
>> +# Makefile for the test helper UEFI applications that run in guests.
>> +#
>> +# Copyright (C) 2019, Red Hat, Inc.
>> +#
>> +# This program and the accompanying materials are licensed and made 
>> available
>> +# under the terms and conditions of the BSD License that accompanies this
>> +# distribution. The full text of the license may be found at
>> +# <http://opensource.org/licenses/bsd-license.php>.
>> +#
>> +# THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS, 
>> WITHOUT
>> +# WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
>> +
>> +edk2_dir              := ../../roms/edk2
>> +images_dir            := ../data/uefi-boot-images
>> +emulation_targets     := arm aarch64 i386 x86_64
>> +uefi_binaries         := bios-tables-test
>> +intermediate_suffixes := .efi .fat .iso.raw
>> +
>> +images: $(foreach binary,$(uefi_binaries), \
>> +            $(foreach target,$(emulation_targets), \
>> +                    $(images_dir)/$(binary).$(target).iso.qcow2))
>> +
>> +# Preserve all intermediate targets if the build succeeds.
>> +# - Intermediate targets help with development & debugging.
>> +# - Preserving intermediate targets also keeps spurious changes out of the
>> +#   final build products, in case the user re-runs "make" without any 
>> changes
>> +#   to the UEFI source code. Normally, the intermediate files would have 
>> been
>> +#   removed by the last "make" invocation, hence the re-run would rebuild 
>> them
>> +#   from the unchanged UEFI sources. Unfortunately, the "mkdosfs" and
>> +#   "genisoimage" utilities embed timestamp-based information in their 
>> outputs,
>> +#   which causes git to report differences for the tracked qcow2 ISO images.
>> +.SECONDARY: $(foreach binary,$(uefi_binaries), \
>> +            $(foreach target,$(emulation_targets), \
>> +                    $(foreach suffix,$(intermediate_suffixes), \
>> +                            Build/$(binary).$(target)$(suffix))))
>> +
>> +# In the pattern rules below, the stem (%, $*) stands for
>> +# "$(binary).$(target)".
>> +
>> +# Convert the raw ISO image to a qcow2 one, enabling compression, and using 
>> a
>> +# small cluster size. This allows for small binary files under git control,
>> +# hence for small binary patches.
>> +$(images_dir)/%.iso.qcow2: Build/%.iso.raw
>> +    mkdir -p -- $(images_dir)
>> +    $${QTEST_QEMU_IMG:-qemu-img} convert -f raw -O qcow2 -c \
>> +            -o cluster_size=512 -- $< $@
>> +
>> +# Embed the "UEFI system partition" into an ISO9660 file system as an 
>> ElTorito
>> +# boot image.
>> +Build/%.iso.raw: Build/%.fat
>> +    genisoimage -input-charset ASCII -efi-boot $(notdir $<) -no-emul-boot \
>> +            -quiet -o $@ -- $<
>> +
>> +# Define chained macros in order to map QEMU system emulation targets to
>> +# *short* UEFI architecture identifiers. Periods are allowed in, and 
>> ultimately
>> +# stripped from, the argument.
>> +map_arm_to_uefi     = $(subst arm,ARM,$(1))
>> +map_aarch64_to_uefi = $(subst aarch64,AA64,$(call map_arm_to_uefi,$(1)))
>> +map_i386_to_uefi    = $(subst i386,IA32,$(call map_aarch64_to_uefi,$(1)))
>> +map_x86_64_to_uefi  = $(subst x86_64,X64,$(call map_i386_to_uefi,$(1)))
>> +map_to_uefi         = $(subst .,,$(call map_x86_64_to_uefi,$(1)))
>> +
>> +# Format a "UEFI system partition", using the UEFI binary as the default 
>> boot
>> +# loader. Add 10% size for filesystem metadata, round up to the next KB, and
>> +# make sure the size is large enough for a FAT filesystem. Name the 
>> filesystem
>> +# after the UEFI binary. (Excess characters are automatically dropped from 
>> the
>> +# filesystem label.)
>> +Build/%.fat: Build/%.efi
>> +    rm -f -- $@
>> +    uefi_bin_b=$$(stat --format=%s -- $<) && \
>> +            uefi_fat_kb=$$(( (uefi_bin_b * 11 / 10 + 1023) / 1024 )) && \
>> +            uefi_fat_kb=$$(( uefi_fat_kb >= 64 ? uefi_fat_kb : 64 )) && \
>> +            mkdosfs -C $@ -n $(basename $(@F)) -- $$uefi_fat_kb
>> +    MTOOLS_SKIP_CHECK=1 mmd -i $@ ::EFI
>> +    MTOOLS_SKIP_CHECK=1 mmd -i $@ ::EFI/BOOT
>> +    MTOOLS_SKIP_CHECK=1 mcopy -i $@ -- $< \
>> +            ::EFI/BOOT/BOOT$(call map_to_uefi,$(suffix $*)).EFI
>> +
>> +# In the pattern rules below, the stem (%, $*) stands for "$(target)" only. 
>> The
>> +# association between the UEFI binary (such as "bios-tables-test") and the
>> +# component name from the edk2 platform DSC file (such as "BiosTablesTest") 
>> is
>> +# explicit in each rule.
>> +
>> +# "build.sh" invokes the "build" utility of edk2 BaseTools. In any given 
>> edk2
>> +# workspace, at most one "build" instance may be operating at a time. 
>> Therefore
>> +# we must serialize the rebuilding of targets in this Makefile.
>> +.NOTPARALLEL:
>> +
>> +# In turn, the "build" utility of edk2 BaseTools invokes another "make".
>> +# Although the outer "make" process advertizes its job server to all child
>> +# processes via MAKEFLAGS in the environment, the outer "make" closes the 
>> job
>> +# server file descriptors (exposed in MAKEFLAGS) before executing a recipe 
>> --
>> +# unless the recipe is recognized as a recursive "make" recipe. Recipes that
>> +# call $(MAKE) are classified automatically as recursive; for "build.sh" 
>> below,
>> +# we must mark the recipe manually as recursive, by using the "+" indicator.
>> +# This way, when the inner "make" starts a parallel build of the target edk2
>> +# module, it can communicate with the outer "make"'s job server.
>> +Build/bios-tables-test.%.efi: build-edk2-tools
>> +    +./build.sh $(edk2_dir) BiosTablesTest $* $@
>
> Does this actually work with an out of tree build?

It's not supposed to.

Again, it's not something that a normal QEMU build includes. It is only
for maintainers to rebuild when there is a reason to do so. The output
binaries are tracked by git, and will be used as-is (in binary form) by
the ACPI test suite. If there are updates to the UEFI source code, the
binaries will have to be rebuilt by a maintainer (or by me, if I submit
the UEFI code changes), and the refreshed blobs are to be checked into
git. Think iPXE oproms for an analogy.

> Shouldn't this be SRC_PATH/tests/uefi-test-tools/ ?

No; nothing under roms/ is built like that, and the same applies to this
patch as well.

*Conceptually*, this patch is for roms/. However, in earlier discussion,
it was suggested that roms/ be kept dedicated to external git submodules
only, and that we not add such ROM source to roms/ whose master repo is
genuinely the QEMU repo. Please see the sub-thread at:

  Re: [PATCH 10/14] tests: acpi: ignore SMBIOS tests when UEFI firmware is used
  http://mid.mail-archive.com/address@hidden

The last idea was that the UEFI source code should be kept in a direct
subdirectory of tests/ (rather than in roms/). And the binaries should
go under tests/data/uefi-boot-images/ (rather than pc-bios/).

Thanks
Laszlo