Re: [Qemu-devel] [PATCH 2/5] roms: build the EfiRom utility from the roms/edk2 submodule

[Laszlo Ersek]

On 01/21/19 12:37, Philippe Mathieu-Daudé wrote:
> On 1/21/19 12:27 PM, Philippe Mathieu-Daudé wrote:
>> Hi Laszlo,
>>
>> On 1/18/19 11:33 PM, Laszlo Ersek wrote:
>>> Building the EfiRom utility from "roms/edk2/BaseTools" should make
>>> "roms/Makefile" more self-contained. Otherwise, we'd call the system-wide
>>> EfiRom for building the combined iPXE option ROMs, but call the sibling
>>> utilities from "roms/edk2/BaseTools" for building "roms/edk2" content.
>>>
>>> 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>
>>> ---
>>>  roms/Makefile | 13 ++++++-------
>>>  1 file changed, 6 insertions(+), 7 deletions(-)
>>>
>>> diff --git a/roms/Makefile b/roms/Makefile
>>> index a6043eff37e9..78d5dd18c301 100644
>>> --- a/roms/Makefile
>>> +++ b/roms/Makefile
>>> @@ -47,10 +47,7 @@ SEABIOS_EXTRAVERSION="-prebuilt.qemu.org"
>>>  # We need that to combine multiple images (legacy bios,
>>>  # efi ia32, efi x64) into a single rom binary.
>>>  #
>>> -# We try to find it in the path.  You can also pass the location on
>>> -# the command line, i.e. "make EFIROM=/path/to/EfiRom efirom"
>>> -#
>>> -EFIROM ?= $(shell which EfiRom 2>/dev/null)
>>> +EFIROM = edk2/BaseTools/Source/C/bin/EfiRom
>>>  
>>>  default:
>>>     @echo "nothing is build by default"
>>> @@ -59,8 +56,7 @@ default:
>>>     @echo "  vgabios        -- update vgabios binaries (seabios)"
>>>     @echo "  sgabios        -- update sgabios binaries"
>>>     @echo "  pxerom         -- update nic roms (bios only)"
>>> -   @echo "  efirom         -- update nic roms (bios+efi, this needs"
>>> -   @echo "                    the EfiRom utility from edk2 / tianocore)"
>>> +   @echo "  efirom         -- update nic roms (bios+efi)"
>>>     @echo "  slof           -- update slof.bin"
>>>     @echo "  skiboot        -- update skiboot.lid"
>>>     @echo "  u-boot.e500    -- update u-boot.e500"
>>> @@ -106,7 +102,7 @@ pxe-rom-%: build-pxe-roms
>>>  
>>>  efirom: $(patsubst %,efi-rom-%,$(pxerom_variants))
>>>  
>>> -efi-rom-%: build-pxe-roms build-efi-roms
>>> +efi-rom-%: build-pxe-roms build-efi-roms $(EFIROM)
>>>     $(EFIROM) -f "0x$(VID)" -i "0x$(DID)" -l 0x02 \
>>>             -b ipxe/src/bin/$(VID)$(DID).rom \
>>>             -ec ipxe/src/bin-i386-efi/$(VID)$(DID).efidrv \
>>> @@ -124,6 +120,8 @@ build-efi-roms: build-pxe-roms
>>>             $(patsubst %,bin-i386-efi/%.efidrv,$(pxerom_targets)) \
>>>             $(patsubst %,bin-x86_64-efi/%.efidrv,$(pxerom_targets))
>>>  
>>> +$(EFIROM):
>>> +   $(MAKE) -C edk2/BaseTools
>>
>> Since this is a single tool/file, can we simply use:
>>
>>         $(MAKE) -C edk2/BaseTools/Source/C/EfiRom
> 
> Also, note that this doesn't work for out-of-tree builds, but this
> problem is pre-existent to your series.

Right, "roms/Makefile" is unrelated to building QEMU itself (which is
supposed to work outside of the source tree); it is for maintainers when
they intend to refresh the bundled fw stuff.

Thanks
Laszlo

> 
>>
>>>  
>>>  slof:
>>>     $(MAKE) -C SLOF CROSS=$(powerpc64_cross_prefix) qemu
>>> @@ -150,6 +148,7 @@ clean:
>>>     $(MAKE) -C sgabios clean
>>>     rm -f sgabios/.depend
>>>     $(MAKE) -C ipxe/src veryclean
>>> +   $(MAKE) -C edk2/BaseTools clean
>>
>> Ditto.
>>
>>>     $(MAKE) -C SLOF clean
>>>     rm -rf u-boot/build.e500
>>>     $(MAKE) -C u-boot-sam460ex distclean
>>>
>>
>> Whichever path used:
>> Reviewed-by: Philippe Mathieu-Daudé <address@hidden>
>> Tested-by: Philippe Mathieu-Daudé <address@hidden>
>>