Re: [Qemu-devel] [PATCH for-4.0 v2 2/2] roms: Allow the EDK2_EFIROM variable to be overridden

[Laszlo Ersek]

On 04/05/19 17:33, Philippe Mathieu-Daudé wrote:
> Since commit f590a812c210 we build the EDK2 EfiRom utility
> unconditionally. This has been tested on all the Linux
> distribution providing continuous integration (namely Debian
> and Fedora). Not all distributions are able to build the
> EfiRom without specific patches (In particular SUSE which
> enforces the PIE protection, see [*]).
> 
> Restore the possibility to other distributions to override
> the EDK2_EFIROM variable.
> 
> [*] https://lists.opensuse.org/opensuse-factory/2017-06/msg00403.html
> 
> Reported-by: Olaf Hering <address@hidden>
> Signed-off-by: Philippe Mathieu-Daudé <address@hidden>
> ---
>  roms/Makefile | 9 ++++++++-
>  1 file changed, 8 insertions(+), 1 deletion(-)
> 
> diff --git a/roms/Makefile b/roms/Makefile
> index d28252dafdf..ea19aa9b33c 100644
> --- a/roms/Makefile
> +++ b/roms/Makefile
> @@ -47,7 +47,11 @@ SEABIOS_EXTRAVERSION="-prebuilt.qemu.org"
>  # We need that to combine multiple images (legacy bios,
>  # efi ia32, efi x64) into a single rom binary.
>  #
> -EDK2_EFIROM = edk2/BaseTools/Source/C/bin/EfiRom
> +# By default we build the latest EDK2 stable EfiRom utility.
> +# If you have to use another one, you can also pass the location on
> +# the command line, i.e. "make EDK2_EFIROM=$(type -P EfiRom) efirom"
> +#
> +EDK2_EFIROM ?= edk2/BaseTools/Source/C/bin/EfiRom
>  
>  default:
>       @echo "nothing is build by default"
> @@ -120,8 +124,11 @@ build-efi-roms: build-pxe-roms
>               $(patsubst %,bin-i386-efi/%.efidrv,$(pxerom_targets)) \
>               $(patsubst %,bin-x86_64-efi/%.efidrv,$(pxerom_targets))
>  
> +# Do not compile $(EDK2_EFIROM) if the variable is overridden
> +ifeq "$(origin EDK2_EFIROM)" "file"
>  $(EDK2_EFIROM):
>       $(MAKE) -C edk2/BaseTools
> +endif
>  
>  slof:
>       $(MAKE) -C SLOF CROSS=$(powerpc64_cross_prefix) qemu
> 

I agree with the problem statement, from Olaf's message in the

  [PATCH for-4.0] roms: Allow the EFIROM variable to be overridden

thread: "roms/ seems to be unable to receive configure options".

That's the issue in need of a fix.


The present patch would break what was fixed by f590a812c210. In other
words, we should stick with using a single EfiRom for all purposes (of
roms/Makefile), but we should let the caller easily inject compiler &
linker options.

So I think the recipe should do something like:

        $(MAKE) -C edk2/BaseTools \
                EXTRA_OPTFLAGS='$(EDK2_BASETOOLS_OPTFLAGS)' \
                EXTRA_LDFLAGS='$(EDK2_BASETOOLS_LDFLAGS)'

Then build scripts could be updated to invoke:

  make -C roms \
    EDK2_BASETOOLS_OPTFLAGS='...' \
    EDK2_BASETOOLS_LDFLAGS='...' \
    efirom

Thanks,
Laszlo