1. Patchew
  2. QEMU
  3. [Qemu-devel] [PATCH 00/10] bundle edk2 platform firmware with QEMU
  4. View patch

[Qemu-devel] [PATCH 06/10] roms/Makefile: replace the $(EFIROM) target with "edk2-basetools"

Laszlo Ersek posted 10 patches 6 years, 8 months ago
There is a newer version of this series
[Qemu-devel] [PATCH 06/10] roms/Makefile: replace the $(EFIROM) target with "edk2-basetools"
Posted by Laszlo Ersek 6 years, 8 months ago 
We don't (can't) have a recipe for building just $(EFIROM); therefore,
while we call the target $(EFIROM), we actually build all of the edk2
BaseTools. Rename the target to edk2-basetools, and update the iPXE
prerequisite accordingly. This will let other targets depend on
"edk2-basetools", where an $(EFIROM) pre-requisite would be misleading.

Signed-off-by: Laszlo Ersek <lersek@redhat.com>
---
 roms/Makefile | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/roms/Makefile b/roms/Makefile
index 78d5dd18c301..2e83ececa25a 100644
--- a/roms/Makefile
+++ b/roms/Makefile
@@ -102,7 +102,7 @@ pxe-rom-%: build-pxe-roms
 
 efirom: $(patsubst %,efi-rom-%,$(pxerom_variants))
 
-efi-rom-%: build-pxe-roms build-efi-roms $(EFIROM)
+efi-rom-%: build-pxe-roms build-efi-roms edk2-basetools
 	$(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 \
@@ -120,7 +120,7 @@ build-efi-roms: build-pxe-roms
 		$(patsubst %,bin-i386-efi/%.efidrv,$(pxerom_targets)) \
 		$(patsubst %,bin-x86_64-efi/%.efidrv,$(pxerom_targets))
 
-$(EFIROM):
+edk2-basetools:
 	$(MAKE) -C edk2/BaseTools
 
 slof:
-- 
2.19.1.3.g30247aa5d201
Re: [Qemu-devel] [PATCH 06/10] roms/Makefile: replace the $(EFIROM) target with "edk2-basetools"
Posted by Philippe Mathieu-Daudé 6 years, 8 months ago 
Hi Laszlo,

On 3/9/19 1:48 AM, Laszlo Ersek wrote:
> We don't (can't) have a recipe for building just $(EFIROM); therefore,
> while we call the target $(EFIROM), we actually build all of the edk2
> BaseTools. Rename the target to edk2-basetools, and update the iPXE
> prerequisite accordingly. This will let other targets depend on
> "edk2-basetools", where an $(EFIROM) pre-requisite would be misleading.
> 
> Signed-off-by: Laszlo Ersek <lersek@redhat.com>
> ---
>  roms/Makefile | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/roms/Makefile b/roms/Makefile
> index 78d5dd18c301..2e83ececa25a 100644
> --- a/roms/Makefile
> +++ b/roms/Makefile
> @@ -102,7 +102,7 @@ pxe-rom-%: build-pxe-roms
>  
>  efirom: $(patsubst %,efi-rom-%,$(pxerom_variants))
>  
> -efi-rom-%: build-pxe-roms build-efi-roms $(EFIROM)
> +efi-rom-%: build-pxe-roms build-efi-roms edk2-basetools
>  	$(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 \
> @@ -120,7 +120,7 @@ build-efi-roms: build-pxe-roms
>  		$(patsubst %,bin-i386-efi/%.efidrv,$(pxerom_targets)) \
>  		$(patsubst %,bin-x86_64-efi/%.efidrv,$(pxerom_targets))
>  
> -$(EFIROM):
> +edk2-basetools:

Should we add:

        $(MAKE) -f Makefile.edk2 submodules

from your next patch?

There might be a circular dependency else, if the user doesn't init the
submodules manually.

The next patch add a make dependency 'efi' -> 'edk2-basetools', but
'edk2-basetools' build the BaseTools within edk2/.

>  	$(MAKE) -C edk2/BaseTools
>  
>  slof:
>
Re: [Qemu-devel] [PATCH 06/10] roms/Makefile: replace the $(EFIROM) target with "edk2-basetools"
Posted by Philippe Mathieu-Daudé 6 years, 8 months ago 
On 3/9/19 2:32 AM, Philippe Mathieu-Daudé wrote:
> Hi Laszlo,
> 
> On 3/9/19 1:48 AM, Laszlo Ersek wrote:
>> We don't (can't) have a recipe for building just $(EFIROM); therefore,
>> while we call the target $(EFIROM), we actually build all of the edk2
>> BaseTools. Rename the target to edk2-basetools, and update the iPXE
>> prerequisite accordingly. This will let other targets depend on
>> "edk2-basetools", where an $(EFIROM) pre-requisite would be misleading.
>>
>> Signed-off-by: Laszlo Ersek <lersek@redhat.com>
>> ---
>>  roms/Makefile | 4 ++--
>>  1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/roms/Makefile b/roms/Makefile
>> index 78d5dd18c301..2e83ececa25a 100644
>> --- a/roms/Makefile
>> +++ b/roms/Makefile
>> @@ -102,7 +102,7 @@ pxe-rom-%: build-pxe-roms
>>  
>>  efirom: $(patsubst %,efi-rom-%,$(pxerom_variants))
>>  
>> -efi-rom-%: build-pxe-roms build-efi-roms $(EFIROM)
>> +efi-rom-%: build-pxe-roms build-efi-roms edk2-basetools
>>  	$(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 \
>> @@ -120,7 +120,7 @@ build-efi-roms: build-pxe-roms
>>  		$(patsubst %,bin-i386-efi/%.efidrv,$(pxerom_targets)) \
>>  		$(patsubst %,bin-x86_64-efi/%.efidrv,$(pxerom_targets))
>>  
>> -$(EFIROM):
>> +edk2-basetools:
> 
> Should we add:
> 
>         $(MAKE) -f Makefile.edk2 submodules
> 
> from your next patch?
> 
> There might be a circular dependency else, if the user doesn't init the
> submodules manually.
> 
> The next patch add a make dependency 'efi' -> 'edk2-basetools', but
> 'edk2-basetools' build the BaseTools within edk2/.
> 

OK I mixed, the Makefile.edk2's submodules rules is for the edk2
repository submodules, while by 'the user submodules' I mean the QEMU
submodules...

So the user has to initialize the roms/edk2 submodule previous to run
both efi/edk2-basetools rules of this Makefile.

>>  	$(MAKE) -C edk2/BaseTools
>>  
>>  slof:
>>
Re: [Qemu-devel] [PATCH 06/10] roms/Makefile: replace the $(EFIROM) target with "edk2-basetools"
Posted by Philippe Mathieu-Daudé 6 years, 8 months ago 
On 3/9/19 2:53 AM, Philippe Mathieu-Daudé wrote:
> On 3/9/19 2:32 AM, Philippe Mathieu-Daudé wrote:
>> Hi Laszlo,
>>
>> On 3/9/19 1:48 AM, Laszlo Ersek wrote:
>>> We don't (can't) have a recipe for building just $(EFIROM); therefore,
>>> while we call the target $(EFIROM), we actually build all of the edk2
>>> BaseTools. Rename the target to edk2-basetools, and update the iPXE
>>> prerequisite accordingly. This will let other targets depend on
>>> "edk2-basetools", where an $(EFIROM) pre-requisite would be misleading.
>>>
>>> Signed-off-by: Laszlo Ersek <lersek@redhat.com>
>>> ---
>>>  roms/Makefile | 4 ++--
>>>  1 file changed, 2 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/roms/Makefile b/roms/Makefile
>>> index 78d5dd18c301..2e83ececa25a 100644
>>> --- a/roms/Makefile
>>> +++ b/roms/Makefile
>>> @@ -102,7 +102,7 @@ pxe-rom-%: build-pxe-roms
>>>  
>>>  efirom: $(patsubst %,efi-rom-%,$(pxerom_variants))
>>>  
>>> -efi-rom-%: build-pxe-roms build-efi-roms $(EFIROM)
>>> +efi-rom-%: build-pxe-roms build-efi-roms edk2-basetools
>>>  	$(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 \
>>> @@ -120,7 +120,7 @@ build-efi-roms: build-pxe-roms
>>>  		$(patsubst %,bin-i386-efi/%.efidrv,$(pxerom_targets)) \
>>>  		$(patsubst %,bin-x86_64-efi/%.efidrv,$(pxerom_targets))
>>>  
>>> -$(EFIROM):
>>> +edk2-basetools:
>>
>> Should we add:
>>
>>         $(MAKE) -f Makefile.edk2 submodules
>>
>> from your next patch?
>>
>> There might be a circular dependency else, if the user doesn't init the
>> submodules manually.
>>
>> The next patch add a make dependency 'efi' -> 'edk2-basetools', but
>> 'edk2-basetools' build the BaseTools within edk2/.
>>
> 
> OK I mixed, the Makefile.edk2's submodules rules is for the edk2
> repository submodules, while by 'the user submodules' I mean the QEMU
> submodules...
> 
> So the user has to initialize the roms/edk2 submodule previous to run
> both efi/edk2-basetools rules of this Makefile.

Maybe we can 'test -e edk2/.git' and display an error "edk2 submodule
not initialized.", what do you think?

Regardless:
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Tested-by: Philippe Mathieu-Daudé <philmd@redhat.com>

>>>  	$(MAKE) -C edk2/BaseTools
>>>  
>>>  slof:
>>>
Re: [Qemu-devel] [PATCH 06/10] roms/Makefile: replace the $(EFIROM) target with "edk2-basetools"
Posted by Laszlo Ersek 6 years, 8 months ago 
On 03/10/19 16:16, Philippe Mathieu-Daudé wrote:
> On 3/9/19 2:53 AM, Philippe Mathieu-Daudé wrote:
>> On 3/9/19 2:32 AM, Philippe Mathieu-Daudé wrote:
>>> Hi Laszlo,
>>>
>>> On 3/9/19 1:48 AM, Laszlo Ersek wrote:
>>>> We don't (can't) have a recipe for building just $(EFIROM); therefore,
>>>> while we call the target $(EFIROM), we actually build all of the edk2
>>>> BaseTools. Rename the target to edk2-basetools, and update the iPXE
>>>> prerequisite accordingly. This will let other targets depend on
>>>> "edk2-basetools", where an $(EFIROM) pre-requisite would be misleading.
>>>>
>>>> Signed-off-by: Laszlo Ersek <lersek@redhat.com>
>>>> ---
>>>>  roms/Makefile | 4 ++--
>>>>  1 file changed, 2 insertions(+), 2 deletions(-)
>>>>
>>>> diff --git a/roms/Makefile b/roms/Makefile
>>>> index 78d5dd18c301..2e83ececa25a 100644
>>>> --- a/roms/Makefile
>>>> +++ b/roms/Makefile
>>>> @@ -102,7 +102,7 @@ pxe-rom-%: build-pxe-roms
>>>>  
>>>>  efirom: $(patsubst %,efi-rom-%,$(pxerom_variants))
>>>>  
>>>> -efi-rom-%: build-pxe-roms build-efi-roms $(EFIROM)
>>>> +efi-rom-%: build-pxe-roms build-efi-roms edk2-basetools
>>>>  	$(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 \
>>>> @@ -120,7 +120,7 @@ build-efi-roms: build-pxe-roms
>>>>  		$(patsubst %,bin-i386-efi/%.efidrv,$(pxerom_targets)) \
>>>>  		$(patsubst %,bin-x86_64-efi/%.efidrv,$(pxerom_targets))
>>>>  
>>>> -$(EFIROM):
>>>> +edk2-basetools:
>>>
>>> Should we add:
>>>
>>>         $(MAKE) -f Makefile.edk2 submodules
>>>
>>> from your next patch?
>>>
>>> There might be a circular dependency else, if the user doesn't init the
>>> submodules manually.
>>>
>>> The next patch add a make dependency 'efi' -> 'edk2-basetools', but
>>> 'edk2-basetools' build the BaseTools within edk2/.
>>>
>>
>> OK I mixed, the Makefile.edk2's submodules rules is for the edk2
>> repository submodules, while by 'the user submodules' I mean the QEMU
>> submodules...
>>
>> So the user has to initialize the roms/edk2 submodule previous to run
>> both efi/edk2-basetools rules of this Makefile.
> 
> Maybe we can 'test -e edk2/.git' and display an error "edk2 submodule
> not initialized.", what do you think?

The point was to simply stick with the current workflow for maintainers
that are already used to rebuilding ROM files from roms/, for pc-bios/.
Those maintainers are used to initializing / updating submodules
directly under roms/, but not used to initing / updating submodules
recursively (because edk2's OpenSSL submodule is the first such
submodule). This is the idea that's captured in the patches.

> Regardless:
> Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> Tested-by: Philippe Mathieu-Daudé <philmd@redhat.com>

Thanks!
Laszlo

> 
>>>>  	$(MAKE) -C edk2/BaseTools
>>>>  
>>>>  slof:
>>>>

Patchew 2.1-devel-b67e4c2

© 2016 - 2025 Red Hat, Inc.