Re: [Qemu-devel] [PATCH v2 07/12] roms: build edk2 firmware binaries and variable store templates

[Philippe Mathieu-Daudé]

On 3/14/19 7:38 PM, Philippe Mathieu-Daudé wrote:
> Hi Laszlo,
> 
> On 3/13/19 10:00 PM, Laszlo Ersek wrote:
>> Add the "efi" target to "Makefile".
>>
>> Introduce "Makefile.edk2" for building and cleaning the firmware images
>> and varstore templates.
>>
>> Collect the common bits from the recipes in the helper script
>> "edk2-build.sh".
>>
>> Signed-off-by: Laszlo Ersek <address@hidden>
>> ---
>>
>> Notes:
>>     v2:
>>     
>>     - drop comma after copyright year, in both new files [Eric]
>>     
>>     - define the SHELL macro as /bin/bash near the top of "Makefile.edk2",
>>       so that various bashisms (e.g. the "source" built-in, and the ==
>>       operator of "[") work even if /bin/sh isn't bash [Phil, Eric]
>>     
>>     - rework "Makefile.edk2" to produce xz-compressed flash device files
>>       [Dan, Michael, Phil]:
>>     
>>       - add implicit rule for compression,
>>     
>>       - mark uncompressed FD files as intermediate,
>>     
>>       - factor out the "flashdevs" macro for sharing between the "all" and
>>         ".INTERMEDIATE" targets
>>     
>>     - due to said rework above, do not pick up Phil's R-b / T-b, and
>>       Michal's and Michael's R-b's
>>
>>  roms/Makefile      |   5 +
>>  roms/Makefile.edk2 | 148 ++++++++++++++++++++
>>  roms/edk2-build.sh |  55 ++++++++
>>  3 files changed, 208 insertions(+)
>>
>> diff --git a/roms/Makefile b/roms/Makefile
>> index 2e83ececa25a..054b432834ba 100644
>> --- a/roms/Makefile
>> +++ b/roms/Makefile
>> @@ -61,6 +61,7 @@ default:
>>      @echo "  skiboot        -- update skiboot.lid"
>>      @echo "  u-boot.e500    -- update u-boot.e500"
>>      @echo "  u-boot.sam460  -- update u-boot.sam460"
>> +    @echo "  efi            -- update UEFI (edk2) platform firmware"
>>  
>>  bios: build-seabios-config-seabios-128k build-seabios-config-seabios-256k
>>      cp seabios/builds/seabios-128k/bios.bin ../pc-bios/bios.bin
>> @@ -143,6 +144,9 @@ skiboot:
>>      $(MAKE) -C skiboot CROSS=$(powerpc64_cross_prefix)
>>      cp skiboot/skiboot.lid ../pc-bios/skiboot.lid
>>  
>> +efi: edk2-basetools
>> +    $(MAKE) -f Makefile.edk2
>> +
>>  clean:
>>      rm -rf seabios/.config seabios/out seabios/builds
>>      $(MAKE) -C sgabios clean
>> @@ -153,3 +157,4 @@ clean:
>>      rm -rf u-boot/build.e500
>>      $(MAKE) -C u-boot-sam460ex distclean
>>      $(MAKE) -C skiboot clean
>> +    $(MAKE) -f Makefile.edk2 clean
>> diff --git a/roms/Makefile.edk2 b/roms/Makefile.edk2
>> new file mode 100644
>> index 000000000000..e5c3036949c8
>> --- /dev/null
>> +++ b/roms/Makefile.edk2
>> @@ -0,0 +1,148 @@
>> +# Makefile for building firmware binaries and variable store templates for a
>> +# number of virtual platforms in edk2.
>> +#
>> +# 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.
>> +
>> +SHELL = /bin/bash
>> +
>> +toolchain = $(shell source ./edk2-funcs.sh && qemu_edk2_get_toolchain $(1))
>> +
>> +licenses := \
>> +    edk2/License.txt \
>> +    edk2/OvmfPkg/License.txt \
>> +    edk2/CryptoPkg/Library/OpensslLib/openssl/LICENSE
>> +
>> +# The "edk2-arm-vars.fd" varstore template is suitable for aarch64 as well.
>> +# Similarly, the "edk2-i386-vars.fd" varstore template is suitable for 
>> x86_64
>> +# as well, independently of "secure" too.
>> +flashdevs := \
>> +    aarch64-code \
>> +    arm-code \
>> +    i386-code \
>> +    i386-secure-code \
>> +    x86_64-code \
>> +    x86_64-secure-code \
>> +    \
>> +    arm-vars \
>> +    i386-vars
>> +
>> +all: $(foreach flashdev,$(flashdevs),../pc-bios/edk2-$(flashdev).fd.xz) \
>> +    ../pc-bios/edk2-licenses.txt
>> +
>> +../pc-bios/edk2-%.fd.xz: ../pc-bios/edk2-%.fd
>> +    xz -9 -e -c $< > $@
> 
> I'm not sure but I think we should add new entries to .gitignore now:
> 
> $ git status
> Changes not staged for commit:
>   (use "git add <file>..." to update what will be committed)
>   (use "git checkout -- <file>..." to discard changes in working directory)
> 
>         modified:   pc-bios/edk2-aarch64-code.fd.xz
>         modified:   pc-bios/edk2-arm-code.fd.xz
>         modified:   pc-bios/edk2-i386-code.fd.xz
>         modified:   pc-bios/edk2-i386-secure-code.fd.xz
>         modified:   pc-bios/edk2-x86_64-code.fd.xz
>         modified:   pc-bios/edk2-x86_64-secure-code.fd.xz
> 
> This change might be enough:
> 
> -- >8 --
> diff --git a/.gitignore b/.gitignore
> @@ -96,7 +96,7 @@
>  *.gcno
>  *.gcov
>  /pc-bios/bios-pq/status
> -/pc-bios/edk2-*.fd
> +/pc-bios/edk2-*.fd.xz
>  /pc-bios/vgabios-pq/status
>  /pc-bios/optionrom/linuxboot.asm
>  /pc-bios/optionrom/linuxboot.bin
> ---
> 
> I don't want to bikeshed again, but I understood the idea of that series
> was for Igor to use the binaries without having to rebuild them.
> 
> Previously we had the .fd directly usable, and 'make .fd' would rebuild
> them. Now they aren't directly usable.
> 
> IOW I was expecting the opposite Make rule:
> 
> ../pc-bios/edk2-%.fd: ../pc-bios/edk2-%.fd.xz
>       xz -d -c $< > $@
> 
> And the next rules to include the final compression.
> 
> So someone wanting to use simply decompress from the repository, while
> to rebuild a .fd you git clone and etc..

So I processed your series using your FIFO style and reached patch
11/12, and now understand:
- roms/Makefile does build + compression,
- Makefile does the decompression.

This is clever and indeed what I'm expecting, easier for QEMU
developpers as EDK2 consumers :)

Reviewed-by: Philippe Mathieu-Daudé <address@hidden>

Next time I'll reuse my double FIFO scheme again ;)

>> +
>> +# When the build completes, we need not keep the uncompressed flash device
>> +# files.
>> +.INTERMEDIATE: $(foreach 
>> flashdev,$(flashdevs),../pc-bios/edk2-$(flashdev).fd)
>> +
>> +submodules:
>> +    cd edk2 && git submodule update --init --force
>> +
>> +# See notes on the ".NOTPARALLEL" target and the "+" indicator in
>> +# "tests/uefi-test-tools/Makefile".
>> +.NOTPARALLEL:
>> +
>> +../pc-bios/edk2-aarch64-code.fd: submodules
>> +    +./edk2-build.sh \
>> +            aarch64 \
>> +            --arch=AARCH64 \
>> +            --platform=ArmVirtPkg/ArmVirtQemu.dsc \
>> +            -D NETWORK_IP6_ENABLE \
>> +            -D HTTP_BOOT_ENABLE
>> +    cp edk2/Build/ArmVirtQemu-AARCH64/DEBUG_$(call 
>> toolchain,aarch64)/FV/QEMU_EFI.fd \
>> +            $@
>> +    truncate --size=64M $@
>> +
>> +../pc-bios/edk2-arm-code.fd: submodules
>> +    +./edk2-build.sh \
>> +            arm \
>> +            --arch=ARM \
>> +            --platform=ArmVirtPkg/ArmVirtQemu.dsc \
>> +            -D NETWORK_IP6_ENABLE \
>> +            -D HTTP_BOOT_ENABLE
>> +    cp edk2/Build/ArmVirtQemu-ARM/DEBUG_$(call 
>> toolchain,arm)/FV/QEMU_EFI.fd \
>> +            $@
>> +    truncate --size=64M $@
>> +
>> +../pc-bios/edk2-i386-code.fd: submodules
>> +    +./edk2-build.sh \
>> +            i386 \
>> +            --arch=IA32 \
>> +            --platform=OvmfPkg/OvmfPkgIa32.dsc \
>> +            -D NETWORK_IP6_ENABLE \
>> +            -D HTTP_BOOT_ENABLE \
>> +            -D TLS_ENABLE \
>> +            -D TPM2_ENABLE \
>> +            -D TPM2_CONFIG_ENABLE
>> +    cp edk2/Build/OvmfIa32/DEBUG_$(call toolchain,i386)/FV/OVMF_CODE.fd $@
>> +
>> +../pc-bios/edk2-i386-secure-code.fd: submodules
>> +    +./edk2-build.sh \
>> +            i386 \
>> +            --arch=IA32 \
>> +            --platform=OvmfPkg/OvmfPkgIa32.dsc \
>> +            -D NETWORK_IP6_ENABLE \
>> +            -D HTTP_BOOT_ENABLE \
>> +            -D TLS_ENABLE \
>> +            -D TPM2_ENABLE \
>> +            -D TPM2_CONFIG_ENABLE \
>> +            -D SECURE_BOOT_ENABLE \
>> +            -D SMM_REQUIRE
>> +    cp edk2/Build/OvmfIa32/DEBUG_$(call toolchain,i386)/FV/OVMF_CODE.fd $@
>> +
>> +../pc-bios/edk2-x86_64-code.fd: submodules
>> +    +./edk2-build.sh \
>> +            x86_64 \
>> +            --arch=X64 \
>> +            --platform=OvmfPkg/OvmfPkgX64.dsc \
>> +            -D NETWORK_IP6_ENABLE \
>> +            -D HTTP_BOOT_ENABLE \
>> +            -D TLS_ENABLE \
>> +            -D TPM2_ENABLE \
>> +            -D TPM2_CONFIG_ENABLE
>> +    cp edk2/Build/OvmfX64/DEBUG_$(call toolchain,x86_64)/FV/OVMF_CODE.fd $@
>> +
>> +../pc-bios/edk2-x86_64-secure-code.fd: submodules
>> +    +./edk2-build.sh \
>> +            x86_64 \
>> +            --arch=IA32 \
>> +            --arch=X64 \
>> +            --platform=OvmfPkg/OvmfPkgIa32X64.dsc \
>> +            -D NETWORK_IP6_ENABLE \
>> +            -D HTTP_BOOT_ENABLE \
>> +            -D TLS_ENABLE \
>> +            -D TPM2_ENABLE \
>> +            -D TPM2_CONFIG_ENABLE \
>> +            -D SECURE_BOOT_ENABLE \
>> +            -D SMM_REQUIRE
>> +    cp edk2/Build/Ovmf3264/DEBUG_$(call toolchain,x86_64)/FV/OVMF_CODE.fd $@
>> +
>> +../pc-bios/edk2-arm-vars.fd: ../pc-bios/edk2-arm-code.fd
>> +    cp edk2/Build/ArmVirtQemu-ARM/DEBUG_$(call 
>> toolchain,arm)/FV/QEMU_VARS.fd \
>> +            $@
>> +    truncate --size=64M $@
>> +
>> +../pc-bios/edk2-i386-vars.fd: ../pc-bios/edk2-i386-code.fd
>> +    cp edk2/Build/OvmfIa32/DEBUG_$(call toolchain,i386)/FV/OVMF_VARS.fd $@
>> +
>> +# The license file accumulates several individual licenses from under edk2,
>> +# prefixing each individual license with a header (generated by "tail") that
>> +# states its pathname.
>> +../pc-bios/edk2-licenses.txt: submodules
>> +    tail -n $(shell cat $(licenses) | wc -l) $(licenses) > $@
>> +    dos2unix $@
>> +
>> +clean:
>> +    rm -rf edk2/Build
>> +    cd edk2/Conf && \
>> +            rm -rf .cache BuildEnv.sh build_rule.txt target.txt \
>> +                    tools_def.txt
>> diff --git a/roms/edk2-build.sh b/roms/edk2-build.sh
>> new file mode 100755
>> index 000000000000..4f46f8a6a217
>> --- /dev/null
>> +++ b/roms/edk2-build.sh
>> @@ -0,0 +1,55 @@
>> +#!/bin/bash
>> +
>> +# Wrapper shell script for building a  virtual platform firmware in edk2.
>> +#
>> +# 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.
>> +
>> +set -e -u -C
>> +
>> +# Save the command line arguments. We need to reset $# to 0 before sourcing
>> +# "edksetup.sh", as it will inherit address@hidden
>> +emulation_target=$1
>> +shift
>> +num_args=0
>> +args=()
>> +for arg in "$@"; do
>> +  args[num_args++]="$arg"
>> +done
>> +shift $num_args
>> +
>> +cd edk2
>> +
>> +# Work around <https://bugzilla.tianocore.org/show_bug.cgi?id=1607>.
>> +export PYTHON_COMMAND=python2
>> +
>> +# Source "edksetup.sh" carefully.
>> +set +e +u +C
>> +source ./edksetup.sh
>> +ret=$?
>> +set -e -u -C
>> +if [ $ret -ne 0 ]; then
>> +  exit $ret
>> +fi
>> +
>> +# Fetch some option arguments, and set the cross-compilation environment (if
>> +# any), for the edk2 "build" utility.
>> +source ../edk2-funcs.sh
>> +edk2_toolchain=$(qemu_edk2_get_toolchain "$emulation_target")
>> +edk2_thread_count=$(qemu_edk2_get_thread_count "$MAKEFLAGS")
>> +qemu_edk2_set_cross_env "$emulation_target"
>> +
>> +# Build the platform firmware.
>> +build \
>> +  --cmd-len=65536 \
>> +  -n "$edk2_thread_count" \
>> +  --buildtarget=DEBUG \
>> +  --tagname="$edk2_toolchain" \
>> +  "address@hidden"
>>
> 
> Tested-by: Philippe Mathieu-Daudé <address@hidden>
>