[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 4/5] tests/uefi-test-tools: add build scripts
From: |
Laszlo Ersek |
Subject: |
Re: [Qemu-devel] [PATCH 4/5] tests/uefi-test-tools: add build scripts |
Date: |
Mon, 21 Jan 2019 20:05:57 +0100 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.9.1 |
On 01/21/19 13:17, Philippe Mathieu-Daudé wrote:
> Hi Laszlo,
>
> On 1/18/19 11:33 PM, 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>
>> ---
>> tests/uefi-test-tools/Makefile | 92 +++++++++++++
>> tests/uefi-test-tools/.gitignore | 3 +
>> tests/uefi-test-tools/build.sh | 145 ++++++++++++++++++++
>> 3 files changed, 240 insertions(+)
>>
>> diff --git a/tests/uefi-test-tools/Makefile b/tests/uefi-test-tools/Makefile
>> new file mode 100644
>> index 000000000000..7b6dd227e433
>> --- /dev/null
>> +++ b/tests/uefi-test-tools/Makefile
>> @@ -0,0 +1,92 @@
>> +# 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/bios-tables-test.%.efi: build-edk2-tools
>> + ./build.sh $(edk2_dir) BiosTablesTest $* $@
>> +
>> +build-edk2-tools:
>> + $(MAKE) -C $(edk2_dir)/BaseTools
>> +
> [...]
>
> I got errors [1] and [2] I couldn't figure out while running 'make -j4'.
>
> The following patch didn't help, any idea?
>
> -- >8 --
> diff --git a/tests/uefi-test-tools/Makefile b/tests/uefi-test-tools/Makefile
> index 7b6dd227e4..798c55c823 100644
> --- a/tests/uefi-test-tools/Makefile
> +++ b/tests/uefi-test-tools/Makefile
> @@ -81,6 +81,8 @@ Build/%.fat: Build/%.efi
> # component name from the edk2 platform DSC file (such as
> "BiosTablesTest") is
> # explicit in each rule.
>
> +.NOTPARALLEL: $(foreach
> target,$(emulation_targets),Build/bios-tables-test.$(target).efi)
> +
> Build/bios-tables-test.%.efi: build-edk2-tools
> ./build.sh $(edk2_dir) BiosTablesTest $* $@
>
> diff --git a/tests/uefi-test-tools/build.sh b/tests/uefi-test-tools/build.sh
> index 155cb75c4d..13891a7385 100755
> --- a/tests/uefi-test-tools/build.sh
> +++ b/tests/uefi-test-tools/build.sh
> @@ -133,6 +133,7 @@ esac
> # Build the UEFI binary
> mkdir -p log
> build \
> + -n 1 \
> --arch="$edk2_arch" \
> --buildtarget=DEBUG \
> --platform=UefiTestToolsPkg/UefiTestToolsPkg.dsc \
> ---
It wasn't clear to me whether and how multi-threaded builds were
supposed to be used by maintainers, whenever they'd update
"tests/data/uefi-boot-images/*".
I saw that "make" was invoked everywhere as $(MAKE), but that didn't
clarify any intent around "-j". So I didn't test "-j" at all, and in
fact I wouldn't have expected it to work:
- At any point in time, there shouldn't be more than one instance of the
"build" base tool working.
- the "build" base tool itself contains some level of support (although
not complete) for parallelizing edk2 module builds: it does not
parallelize the compilation of source files between each other *within*
a single edk2 module (= INF file). It parallelizes modules (INF files)
between each other.
- This means if you build only one module at a time, passing the "-m"
switch (plus an INF file) to the "build" base tool, then there's nothing
to parallelize.
- "build.sh" does pass the "-m" switch to the "build" tool, because I
wanted the build output to be granular. That is, it should be possible
for someone to build just a given uefi-test-tools image for just a given
architecture, if they specify the corresponding pathname (as target)
when they invoke "make". This usage requires the "-m" flag, which in
turn precludes the parallelism built into the "build" base tool itself.
And, invoking multiple "build" instances in parallel is not supposed to
work.
Regarding why ".NOTPARALLEL" doesn't work -- the GNU Make documentation
writes:
[...] Any recursively invoked make command will still run recipes in
parallel (unless its makefile also contains this target). [...]
The "build" base tool in edk2 implements part of the job with generated
makefiles and invoking "make" itself, thus, despite .NOTPARALLEL, it
likely inherits the outermost -j<N> setting -- and it doesn't expect such.
So the best I can offer here is to check $MAKEFLAGS in "build.sh", and
exit with an early, explicit error if $MAKEFLAGS contains "-j", "-l", or
their long variants (--jobs, --load-average).
I realize this would not be acceptable for building QEMU itself, but
"tests/uefi-test-tools/Makefile" should be run by some subsystem
maintainers only, interactively (or in their on custom scripts).
Thanks
Laszlo
- Re: [Qemu-devel] [PATCH 1/5] roms: add the edk2 project as a git submodule, (continued)
[Qemu-devel] [PATCH 2/5] roms: build the EfiRom utility from the roms/edk2 submodule, Laszlo Ersek, 2019/01/18
[Qemu-devel] [PATCH 4/5] tests/uefi-test-tools: add build scripts, Laszlo Ersek, 2019/01/18
[Qemu-devel] [PATCH 3/5] tests: introduce "uefi-test-tools" with the BiosTablesTest UEFI app, Laszlo Ersek, 2019/01/18
[Qemu-devel] [PATCH 5/5] tests/data: introduce "uefi-boot-images" with the "bios-tables-test" ISOs, Laszlo Ersek, 2019/01/18