[Qemu-devel] [PATCH for-4.0 v3 0/2] roms: Avoid iPXE/EDK2 EFIROM variable clash, pass CFLAGS to EDK2 build tools

[Qemu-devel] [PATCH for-4.0 v3 0/2] roms: Avoid iPXE/EDK2 EFIROM variable clash, pass CFLAGS to EDK2 build tools

[Philippe Mathieu-Daudé]

Hi,

Two trivial fixes to avoid the latest EDK2 testing series to
cause trouble to downstream distributions (in particular if
they have PIE enforced).

Since v2:
- addressed Laszlo's suggestion
- reworded patch description

v2: https://lists.gnu.org/archive/html/qemu-devel/2019-04/msg01033.html
v1: https://lists.gnu.org/archive/html/qemu-devel/2019-04/msg01006.html
error report: https://lists.gnu.org/archive/html/qemu-devel/2019-04/msg00992.html

Regards,

Phil.

Philippe Mathieu-Daudé (2):
  roms: Rename the EFIROM variable to avoid clashing with iPXE
  roms: Allow passing configure options to the EDK2 build tools

 roms/Makefile | 23 ++++++++++++++++++-----
 1 file changed, 18 insertions(+), 5 deletions(-)

-- 
2.20.1

[Qemu-devel] [PATCH for-4.0 v3 0/2] roms: Avoid iPXE/EDK2 EFIROM variable clash, pass CFLAGS to EDK2 build tools

[Philippe Mathieu-Daudé]

Hi,

Two trivial fixes to avoid the latest EDK2 testing series to
cause trouble to downstream distributions (in particular if
they have PIE enforced).

Since v2:
- addressed Laszlo's suggestion
- reworded patch description

v2: https://lists.gnu.org/archive/html/qemu-devel/2019-04/msg01033.html
v1: https://lists.gnu.org/archive/html/qemu-devel/2019-04/msg01006.html
error report: https://lists.gnu.org/archive/html/qemu-devel/2019-04/msg00992.html

Regards,

Phil.

Philippe Mathieu-Daudé (2):
  roms: Rename the EFIROM variable to avoid clashing with iPXE
  roms: Allow passing configure options to the EDK2 build tools

 roms/Makefile | 23 ++++++++++++++++++-----
 1 file changed, 18 insertions(+), 5 deletions(-)

-- 
2.20.1

[Qemu-devel] [PATCH for-4.0 v3 1/2] roms: Rename the EFIROM variable to avoid clashing with iPXE

[Philippe Mathieu-Daudé]

The iPXE's 'veryclean' recipe removes $(EFIROM) even if the EFIROM
macro originates from elsewhere:

  $ git checkout f590a812c21~
  $ make -C roms clean EFIROM=$(type -P EfiRom)
  make: Entering directory '/source/qemu/roms'
  [...]
  make -C ipxe/src veryclean
  make[1]: Entering directory '/source/qemu/roms/ipxe/src'
  rm -f bin{,-*}/*.* bin{,-*}/.certificate.* bin{,-*}/.certificates.* bin{,-*}/.private_key.* bin{,-*}/errors bin{,-*}/NIC ./util/zbin ./util/elf2efi32 ./util/elf2efi64 /usr/bin/EfiRom ./util/efifatbin ./util/iccfix ./util/einfo TAGS bin{,-*}/symtab
  rm: cannot remove '/usr/bin/EfiRom': Permission denied
  make[1]: *** [Makefile.housekeeping:1564: clean] Error 1
  make[1]: Leaving directory '/source/qemu/roms/ipxe/src'
  make: *** [Makefile:152: clean] Error 2
  make: Leaving directory '/source/qemu/roms'

Before f590a812c21 this variable could be overridden or unset,
and the 'veryclean' Makefile rule would not complain.

Commit f590a812c21 enforces this variable to the Intel EfiRom
tool provided by the EDK2 project.

To avoid the name clash and make the difference between the
projects obvious, rename the variable used by the EDK2 project
as EDK2_EFIROM.

Fixes: f590a812c21074e82228de3e1dfb57b75fc02b62
Reported-by: Olaf Hering <olaf@aepfle.de>
Reviewed-by: Laszlo Ersek <lersek@redhat.com>
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
 roms/Makefile | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/roms/Makefile b/roms/Makefile
index 78d5dd18c30..d28252dafdf 100644
--- a/roms/Makefile
+++ b/roms/Makefile
@@ -47,7 +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.
 #
-EFIROM = edk2/BaseTools/Source/C/bin/EfiRom
+EDK2_EFIROM = edk2/BaseTools/Source/C/bin/EfiRom
 
 default:
 	@echo "nothing is build by default"
@@ -102,8 +102,8 @@ pxe-rom-%: build-pxe-roms
 
 efirom: $(patsubst %,efi-rom-%,$(pxerom_variants))
 
-efi-rom-%: build-pxe-roms build-efi-roms $(EFIROM)
-	$(EFIROM) -f "0x$(VID)" -i "0x$(DID)" -l 0x02 \
+efi-rom-%: build-pxe-roms build-efi-roms $(EDK2_EFIROM)
+	$(EDK2_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 \
 		-ec ipxe/src/bin-x86_64-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_EFIROM):
 	$(MAKE) -C edk2/BaseTools
 
 slof:
-- 
2.20.1

[Qemu-devel] [PATCH for-4.0 v3 1/2] roms: Rename the EFIROM variable to avoid clashing with iPXE

[Philippe Mathieu-Daudé]

The iPXE's 'veryclean' recipe removes $(EFIROM) even if the EFIROM
macro originates from elsewhere:

  $ git checkout f590a812c21~
  $ make -C roms clean EFIROM=$(type -P EfiRom)
  make: Entering directory '/source/qemu/roms'
  [...]
  make -C ipxe/src veryclean
  make[1]: Entering directory '/source/qemu/roms/ipxe/src'
  rm -f bin{,-*}/*.* bin{,-*}/.certificate.* bin{,-*}/.certificates.* bin{,-*}/.private_key.* bin{,-*}/errors bin{,-*}/NIC ./util/zbin ./util/elf2efi32 ./util/elf2efi64 /usr/bin/EfiRom ./util/efifatbin ./util/iccfix ./util/einfo TAGS bin{,-*}/symtab
  rm: cannot remove '/usr/bin/EfiRom': Permission denied
  make[1]: *** [Makefile.housekeeping:1564: clean] Error 1
  make[1]: Leaving directory '/source/qemu/roms/ipxe/src'
  make: *** [Makefile:152: clean] Error 2
  make: Leaving directory '/source/qemu/roms'

Before f590a812c21 this variable could be overridden or unset,
and the 'veryclean' Makefile rule would not complain.

Commit f590a812c21 enforces this variable to the Intel EfiRom
tool provided by the EDK2 project.

To avoid the name clash and make the difference between the
projects obvious, rename the variable used by the EDK2 project
as EDK2_EFIROM.

Fixes: f590a812c21074e82228de3e1dfb57b75fc02b62
Reported-by: Olaf Hering <olaf@aepfle.de>
Reviewed-by: Laszlo Ersek <lersek@redhat.com>
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
 roms/Makefile | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/roms/Makefile b/roms/Makefile
index 78d5dd18c30..d28252dafdf 100644
--- a/roms/Makefile
+++ b/roms/Makefile
@@ -47,7 +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.
 #
-EFIROM = edk2/BaseTools/Source/C/bin/EfiRom
+EDK2_EFIROM = edk2/BaseTools/Source/C/bin/EfiRom
 
 default:
 	@echo "nothing is build by default"
@@ -102,8 +102,8 @@ pxe-rom-%: build-pxe-roms
 
 efirom: $(patsubst %,efi-rom-%,$(pxerom_variants))
 
-efi-rom-%: build-pxe-roms build-efi-roms $(EFIROM)
-	$(EFIROM) -f "0x$(VID)" -i "0x$(DID)" -l 0x02 \
+efi-rom-%: build-pxe-roms build-efi-roms $(EDK2_EFIROM)
+	$(EDK2_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 \
 		-ec ipxe/src/bin-x86_64-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_EFIROM):
 	$(MAKE) -C edk2/BaseTools
 
 slof:
-- 
2.20.1

[Qemu-devel] [PATCH for-4.0 v3 2/2] roms: Allow passing configure options to the EDK2 build tools

[Philippe Mathieu-Daudé]

Since commit f590a812c210 we build the EDK2 EfiRom utility
unconditionally.

Some distributions require to use extra compiler/linker flags,
i.e. SUSE which enforces the PIE protection (see [*]).

EDK2 build tools already provide a set of variables for that,
use them to allow the caller to easily inject compiler/linker
options..

Now build scripts can pass extra options, example:

  $ make -C roms \
      EDK2_BASETOOLS_OPTFLAGS='-fPIE' \
      efirom

[*] https://lists.opensuse.org/opensuse-factory/2017-06/msg00403.html

Reported-by: Olaf Hering <olaf@aepfle.de>
Suggested-by: Laszlo Ersek <lersek@redhat.com>
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
 roms/Makefile | 15 ++++++++++++++-
 1 file changed, 14 insertions(+), 1 deletion(-)

diff --git a/roms/Makefile b/roms/Makefile
index d28252dafdf..1ff78b63bb3 100644
--- a/roms/Makefile
+++ b/roms/Makefile
@@ -120,8 +120,21 @@ build-efi-roms: build-pxe-roms
 		$(patsubst %,bin-i386-efi/%.efidrv,$(pxerom_targets)) \
 		$(patsubst %,bin-x86_64-efi/%.efidrv,$(pxerom_targets))
 
+# Build scripts can pass compiler/linker flags to the EDK2 build tools
+# via the EDK2_BASETOOLS_OPTFLAGS (CPPFLAGS and CFLAGS) and
+# EDK2_BASETOOLS_LDFLAGS (LDFLAGS) environment variables.
+#
+# Example:
+#
+#  make -C roms \
+#    EDK2_BASETOOLS_OPTFLAGS='...' \
+#    EDK2_BASETOOLS_LDFLAGS='...' \
+#    efirom
+#
 $(EDK2_EFIROM):
-	$(MAKE) -C edk2/BaseTools
+	$(MAKE) -C edk2/BaseTools \
+		EXTRA_OPTFLAGS='$(EDK2_BASETOOLS_OPTFLAGS)' \
+		EXTRA_LDFLAGS='$(EDK2_BASETOOLS_LDFLAGS)'
 
 slof:
 	$(MAKE) -C SLOF CROSS=$(powerpc64_cross_prefix) qemu
-- 
2.20.1

[Qemu-devel] [PATCH for-4.0 v3 2/2] roms: Allow passing configure options to the EDK2 build tools

[Philippe Mathieu-Daudé]

Since commit f590a812c210 we build the EDK2 EfiRom utility
unconditionally.

Some distributions require to use extra compiler/linker flags,
i.e. SUSE which enforces the PIE protection (see [*]).

EDK2 build tools already provide a set of variables for that,
use them to allow the caller to easily inject compiler/linker
options..

Now build scripts can pass extra options, example:

  $ make -C roms \
      EDK2_BASETOOLS_OPTFLAGS='-fPIE' \
      efirom

[*] https://lists.opensuse.org/opensuse-factory/2017-06/msg00403.html

Reported-by: Olaf Hering <olaf@aepfle.de>
Suggested-by: Laszlo Ersek <lersek@redhat.com>
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
 roms/Makefile | 15 ++++++++++++++-
 1 file changed, 14 insertions(+), 1 deletion(-)

diff --git a/roms/Makefile b/roms/Makefile
index d28252dafdf..1ff78b63bb3 100644
--- a/roms/Makefile
+++ b/roms/Makefile
@@ -120,8 +120,21 @@ build-efi-roms: build-pxe-roms
 		$(patsubst %,bin-i386-efi/%.efidrv,$(pxerom_targets)) \
 		$(patsubst %,bin-x86_64-efi/%.efidrv,$(pxerom_targets))
 
+# Build scripts can pass compiler/linker flags to the EDK2 build tools
+# via the EDK2_BASETOOLS_OPTFLAGS (CPPFLAGS and CFLAGS) and
+# EDK2_BASETOOLS_LDFLAGS (LDFLAGS) environment variables.
+#
+# Example:
+#
+#  make -C roms \
+#    EDK2_BASETOOLS_OPTFLAGS='...' \
+#    EDK2_BASETOOLS_LDFLAGS='...' \
+#    efirom
+#
 $(EDK2_EFIROM):
-	$(MAKE) -C edk2/BaseTools
+	$(MAKE) -C edk2/BaseTools \
+		EXTRA_OPTFLAGS='$(EDK2_BASETOOLS_OPTFLAGS)' \
+		EXTRA_LDFLAGS='$(EDK2_BASETOOLS_LDFLAGS)'
 
 slof:
 	$(MAKE) -C SLOF CROSS=$(powerpc64_cross_prefix) qemu
-- 
2.20.1

Re: [Qemu-devel] [PATCH for-4.0 v3 0/2] roms: Avoid iPXE/EDK2 EFIROM variable clash, pass CFLAGS to EDK2 build tools

[Peter Maydell]

On Tue, 9 Apr 2019 at 14:47, Philippe Mathieu-Daudé <philmd@redhat.com> wrote:
>
> Hi,
>
> Two trivial fixes to avoid the latest EDK2 testing series to
> cause trouble to downstream distributions (in particular if
> they have PIE enforced).
>
> Since v2:
> - addressed Laszlo's suggestion
> - reworded patch description
>
> v2: https://lists.gnu.org/archive/html/qemu-devel/2019-04/msg01033.html
> v1: https://lists.gnu.org/archive/html/qemu-devel/2019-04/msg01006.html
> error report: https://lists.gnu.org/archive/html/qemu-devel/2019-04/msg00992.html
>
> Regards,
>
> Phil.
>
> Philippe Mathieu-Daudé (2):
>   roms: Rename the EFIROM variable to avoid clashing with iPXE
>   roms: Allow passing configure options to the EDK2 build tools
>
>  roms/Makefile | 23 ++++++++++++++++++-----
>  1 file changed, 18 insertions(+), 5 deletions(-)

Thanks. This is one of a handful of remaining bugs we
should fix for 4.0, so I would ideally like to get this
in rc3 today (or tomorrow if we have to slip rc3 a day).
If people wanting to review this series could do so
promptly I'd definitely appreciate it.

-- PMM

Re: [Qemu-devel] [PATCH for-4.0 v3 0/2] roms: Avoid iPXE/EDK2 EFIROM variable clash, pass CFLAGS to EDK2 build tools

[Peter Maydell]

On Tue, 9 Apr 2019 at 14:47, Philippe Mathieu-Daudé <philmd@redhat.com> wrote:
>
> Hi,
>
> Two trivial fixes to avoid the latest EDK2 testing series to
> cause trouble to downstream distributions (in particular if
> they have PIE enforced).
>
> Since v2:
> - addressed Laszlo's suggestion
> - reworded patch description
>
> v2: https://lists.gnu.org/archive/html/qemu-devel/2019-04/msg01033.html
> v1: https://lists.gnu.org/archive/html/qemu-devel/2019-04/msg01006.html
> error report: https://lists.gnu.org/archive/html/qemu-devel/2019-04/msg00992.html
>
> Regards,
>
> Phil.
>
> Philippe Mathieu-Daudé (2):
>   roms: Rename the EFIROM variable to avoid clashing with iPXE
>   roms: Allow passing configure options to the EDK2 build tools
>
>  roms/Makefile | 23 ++++++++++++++++++-----
>  1 file changed, 18 insertions(+), 5 deletions(-)

Thanks. This is one of a handful of remaining bugs we
should fix for 4.0, so I would ideally like to get this
in rc3 today (or tomorrow if we have to slip rc3 a day).
If people wanting to review this series could do so
promptly I'd definitely appreciate it.

-- PMM

Re: [Qemu-devel] [PATCH for-4.0 v3 2/2] roms: Allow passing configure options to the EDK2 build tools

[Igor Mammedov]

On Tue,  9 Apr 2019 15:45:36 +0200
Philippe Mathieu-Daudé <philmd@redhat.com> wrote:

> Since commit f590a812c210 we build the EDK2 EfiRom utility
> unconditionally.
> 
> Some distributions require to use extra compiler/linker flags,
> i.e. SUSE which enforces the PIE protection (see [*]).
> 
> EDK2 build tools already provide a set of variables for that,
> use them to allow the caller to easily inject compiler/linker
> options..
> 
> Now build scripts can pass extra options, example:
> 
>   $ make -C roms \
>       EDK2_BASETOOLS_OPTFLAGS='-fPIE' \
>       efirom
> 
> [*] https://lists.opensuse.org/opensuse-factory/2017-06/msg00403.html
> 
> Reported-by: Olaf Hering <olaf@aepfle.de>
> Suggested-by: Laszlo Ersek <lersek@redhat.com>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>

Reviewed-by: Igor Mammedov <imammedo@redhat.com>

> ---
>  roms/Makefile | 15 ++++++++++++++-
>  1 file changed, 14 insertions(+), 1 deletion(-)
> 
> diff --git a/roms/Makefile b/roms/Makefile
> index d28252dafdf..1ff78b63bb3 100644
> --- a/roms/Makefile
> +++ b/roms/Makefile
> @@ -120,8 +120,21 @@ build-efi-roms: build-pxe-roms
>  		$(patsubst %,bin-i386-efi/%.efidrv,$(pxerom_targets)) \
>  		$(patsubst %,bin-x86_64-efi/%.efidrv,$(pxerom_targets))
>  
> +# Build scripts can pass compiler/linker flags to the EDK2 build tools
> +# via the EDK2_BASETOOLS_OPTFLAGS (CPPFLAGS and CFLAGS) and
> +# EDK2_BASETOOLS_LDFLAGS (LDFLAGS) environment variables.
> +#
> +# Example:
> +#
> +#  make -C roms \
> +#    EDK2_BASETOOLS_OPTFLAGS='...' \
> +#    EDK2_BASETOOLS_LDFLAGS='...' \
> +#    efirom
> +#
>  $(EDK2_EFIROM):
> -	$(MAKE) -C edk2/BaseTools
> +	$(MAKE) -C edk2/BaseTools \
> +		EXTRA_OPTFLAGS='$(EDK2_BASETOOLS_OPTFLAGS)' \
> +		EXTRA_LDFLAGS='$(EDK2_BASETOOLS_LDFLAGS)'
>  
>  slof:
>  	$(MAKE) -C SLOF CROSS=$(powerpc64_cross_prefix) qemu

Re: [Qemu-devel] [PATCH for-4.0 v3 2/2] roms: Allow passing configure options to the EDK2 build tools

[Igor Mammedov]

On Tue,  9 Apr 2019 15:45:36 +0200
Philippe Mathieu-Daudé <philmd@redhat.com> wrote:

> Since commit f590a812c210 we build the EDK2 EfiRom utility
> unconditionally.
> 
> Some distributions require to use extra compiler/linker flags,
> i.e. SUSE which enforces the PIE protection (see [*]).
> 
> EDK2 build tools already provide a set of variables for that,
> use them to allow the caller to easily inject compiler/linker
> options..
> 
> Now build scripts can pass extra options, example:
> 
>   $ make -C roms \
>       EDK2_BASETOOLS_OPTFLAGS='-fPIE' \
>       efirom
> 
> [*] https://lists.opensuse.org/opensuse-factory/2017-06/msg00403.html
> 
> Reported-by: Olaf Hering <olaf@aepfle.de>
> Suggested-by: Laszlo Ersek <lersek@redhat.com>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>

Reviewed-by: Igor Mammedov <imammedo@redhat.com>

> ---
>  roms/Makefile | 15 ++++++++++++++-
>  1 file changed, 14 insertions(+), 1 deletion(-)
> 
> diff --git a/roms/Makefile b/roms/Makefile
> index d28252dafdf..1ff78b63bb3 100644
> --- a/roms/Makefile
> +++ b/roms/Makefile
> @@ -120,8 +120,21 @@ build-efi-roms: build-pxe-roms
>  		$(patsubst %,bin-i386-efi/%.efidrv,$(pxerom_targets)) \
>  		$(patsubst %,bin-x86_64-efi/%.efidrv,$(pxerom_targets))
>  
> +# Build scripts can pass compiler/linker flags to the EDK2 build tools
> +# via the EDK2_BASETOOLS_OPTFLAGS (CPPFLAGS and CFLAGS) and
> +# EDK2_BASETOOLS_LDFLAGS (LDFLAGS) environment variables.
> +#
> +# Example:
> +#
> +#  make -C roms \
> +#    EDK2_BASETOOLS_OPTFLAGS='...' \
> +#    EDK2_BASETOOLS_LDFLAGS='...' \
> +#    efirom
> +#
>  $(EDK2_EFIROM):
> -	$(MAKE) -C edk2/BaseTools
> +	$(MAKE) -C edk2/BaseTools \
> +		EXTRA_OPTFLAGS='$(EDK2_BASETOOLS_OPTFLAGS)' \
> +		EXTRA_LDFLAGS='$(EDK2_BASETOOLS_LDFLAGS)'
>  
>  slof:
>  	$(MAKE) -C SLOF CROSS=$(powerpc64_cross_prefix) qemu

Re: [Qemu-devel] [PATCH for-4.0 v3 0/2] roms: Avoid iPXE/EDK2 EFIROM variable clash, pass CFLAGS to EDK2 build tools

[Michael S. Tsirkin]

On Tue, Apr 09, 2019 at 03:45:34PM +0200, Philippe Mathieu-Daudé wrote:
> Hi,
> 
> Two trivial fixes to avoid the latest EDK2 testing series to
> cause trouble to downstream distributions (in particular if
> they have PIE enforced).
> 
> Since v2:
> - addressed Laszlo's suggestion
> - reworded patch description
> 
> v2: https://lists.gnu.org/archive/html/qemu-devel/2019-04/msg01033.html
> v1: https://lists.gnu.org/archive/html/qemu-devel/2019-04/msg01006.html
> error report: https://lists.gnu.org/archive/html/qemu-devel/2019-04/msg00992.html
> 
> Regards,
> 
> Phil.

Reviewed-by: Michael S. Tsirkin <mst@redhat.com>


Peter do you want to merge this directly?

> Philippe Mathieu-Daudé (2):
>   roms: Rename the EFIROM variable to avoid clashing with iPXE
>   roms: Allow passing configure options to the EDK2 build tools
> 
>  roms/Makefile | 23 ++++++++++++++++++-----
>  1 file changed, 18 insertions(+), 5 deletions(-)
> 
> -- 
> 2.20.1

Re: [Qemu-devel] [PATCH for-4.0 v3 0/2] roms: Avoid iPXE/EDK2 EFIROM variable clash, pass CFLAGS to EDK2 build tools

[Michael S. Tsirkin]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset="UTF-8", Size: 949 bytes --]

On Tue, Apr 09, 2019 at 03:45:34PM +0200, Philippe Mathieu-Daudé wrote:
> Hi,
> 
> Two trivial fixes to avoid the latest EDK2 testing series to
> cause trouble to downstream distributions (in particular if
> they have PIE enforced).
> 
> Since v2:
> - addressed Laszlo's suggestion
> - reworded patch description
> 
> v2: https://lists.gnu.org/archive/html/qemu-devel/2019-04/msg01033.html
> v1: https://lists.gnu.org/archive/html/qemu-devel/2019-04/msg01006.html
> error report: https://lists.gnu.org/archive/html/qemu-devel/2019-04/msg00992.html
> 
> Regards,
> 
> Phil.

Reviewed-by: Michael S. Tsirkin <mst@redhat.com>


Peter do you want to merge this directly?

> Philippe Mathieu-Daudé (2):
>   roms: Rename the EFIROM variable to avoid clashing with iPXE
>   roms: Allow passing configure options to the EDK2 build tools
> 
>  roms/Makefile | 23 ++++++++++++++++++-----
>  1 file changed, 18 insertions(+), 5 deletions(-)
> 
> -- 
> 2.20.1

Re: [Qemu-devel] [PATCH for-4.0 v3 0/2] roms: Avoid iPXE/EDK2 EFIROM variable clash, pass CFLAGS to EDK2 build tools

[Peter Maydell]

On Tue, 9 Apr 2019 at 16:06, Michael S. Tsirkin <mst@redhat.com> wrote:
>
> On Tue, Apr 09, 2019 at 03:45:34PM +0200, Philippe Mathieu-Daudé wrote:
> > Hi,
> >
> > Two trivial fixes to avoid the latest EDK2 testing series to
> > cause trouble to downstream distributions (in particular if
> > they have PIE enforced).
> >
> > Since v2:
> > - addressed Laszlo's suggestion
> > - reworded patch description
> >
> > v2: https://lists.gnu.org/archive/html/qemu-devel/2019-04/msg01033.html
> > v1: https://lists.gnu.org/archive/html/qemu-devel/2019-04/msg01006.html
> > error report: https://lists.gnu.org/archive/html/qemu-devel/2019-04/msg00992.html
> >
> > Regards,
> >
> > Phil.
>
> Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
>
>
> Peter do you want to merge this directly?

Yes, I'll do that.

thanks
-- PMM

Re: [Qemu-devel] [PATCH for-4.0 v3 0/2] roms: Avoid iPXE/EDK2 EFIROM variable clash, pass CFLAGS to EDK2 build tools

[Peter Maydell]

On Tue, 9 Apr 2019 at 16:06, Michael S. Tsirkin <mst@redhat.com> wrote:
>
> On Tue, Apr 09, 2019 at 03:45:34PM +0200, Philippe Mathieu-Daudé wrote:
> > Hi,
> >
> > Two trivial fixes to avoid the latest EDK2 testing series to
> > cause trouble to downstream distributions (in particular if
> > they have PIE enforced).
> >
> > Since v2:
> > - addressed Laszlo's suggestion
> > - reworded patch description
> >
> > v2: https://lists.gnu.org/archive/html/qemu-devel/2019-04/msg01033.html
> > v1: https://lists.gnu.org/archive/html/qemu-devel/2019-04/msg01006.html
> > error report: https://lists.gnu.org/archive/html/qemu-devel/2019-04/msg00992.html
> >
> > Regards,
> >
> > Phil.
>
> Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
>
>
> Peter do you want to merge this directly?

Yes, I'll do that.

thanks
-- PMM

Re: [Qemu-devel] [PATCH for-4.0 v3 1/2] roms: Rename the EFIROM variable to avoid clashing with iPXE

[Laszlo Ersek]

On 04/09/19 15:45, Philippe Mathieu-Daudé wrote:
> The iPXE's 'veryclean' recipe removes $(EFIROM) even if the EFIROM
> macro originates from elsewhere:
> 
>   $ git checkout f590a812c21~
>   $ make -C roms clean EFIROM=$(type -P EfiRom)
>   make: Entering directory '/source/qemu/roms'
>   [...]
>   make -C ipxe/src veryclean
>   make[1]: Entering directory '/source/qemu/roms/ipxe/src'
>   rm -f bin{,-*}/*.* bin{,-*}/.certificate.* bin{,-*}/.certificates.* bin{,-*}/.private_key.* bin{,-*}/errors bin{,-*}/NIC ./util/zbin ./util/elf2efi32 ./util/elf2efi64 /usr/bin/EfiRom ./util/efifatbin ./util/iccfix ./util/einfo TAGS bin{,-*}/symtab
>   rm: cannot remove '/usr/bin/EfiRom': Permission denied
>   make[1]: *** [Makefile.housekeeping:1564: clean] Error 1
>   make[1]: Leaving directory '/source/qemu/roms/ipxe/src'
>   make: *** [Makefile:152: clean] Error 2
>   make: Leaving directory '/source/qemu/roms'
> 
> Before f590a812c21 this variable could be overridden or unset,
> and the 'veryclean' Makefile rule would not complain.
> 
> Commit f590a812c21 enforces this variable to the Intel EfiRom
> tool provided by the EDK2 project.
> 
> To avoid the name clash and make the difference between the
> projects obvious, rename the variable used by the EDK2 project
> as EDK2_EFIROM.
> 
> Fixes: f590a812c21074e82228de3e1dfb57b75fc02b62
> Reported-by: Olaf Hering <olaf@aepfle.de>
> Reviewed-by: Laszlo Ersek <lersek@redhat.com>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> ---
>  roms/Makefile | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/roms/Makefile b/roms/Makefile
> index 78d5dd18c30..d28252dafdf 100644
> --- a/roms/Makefile
> +++ b/roms/Makefile
> @@ -47,7 +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.
>  #
> -EFIROM = edk2/BaseTools/Source/C/bin/EfiRom
> +EDK2_EFIROM = edk2/BaseTools/Source/C/bin/EfiRom
>  
>  default:
>  	@echo "nothing is build by default"
> @@ -102,8 +102,8 @@ pxe-rom-%: build-pxe-roms
>  
>  efirom: $(patsubst %,efi-rom-%,$(pxerom_variants))
>  
> -efi-rom-%: build-pxe-roms build-efi-roms $(EFIROM)
> -	$(EFIROM) -f "0x$(VID)" -i "0x$(DID)" -l 0x02 \
> +efi-rom-%: build-pxe-roms build-efi-roms $(EDK2_EFIROM)
> +	$(EDK2_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 \
>  		-ec ipxe/src/bin-x86_64-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_EFIROM):
>  	$(MAKE) -C edk2/BaseTools
>  
>  slof:
> 

looks good, thank you; my R-b stands.

Laszlo

Re: [Qemu-devel] [PATCH for-4.0 v3 1/2] roms: Rename the EFIROM variable to avoid clashing with iPXE

[Laszlo Ersek]

On 04/09/19 15:45, Philippe Mathieu-Daudé wrote:
> The iPXE's 'veryclean' recipe removes $(EFIROM) even if the EFIROM
> macro originates from elsewhere:
> 
>   $ git checkout f590a812c21~
>   $ make -C roms clean EFIROM=$(type -P EfiRom)
>   make: Entering directory '/source/qemu/roms'
>   [...]
>   make -C ipxe/src veryclean
>   make[1]: Entering directory '/source/qemu/roms/ipxe/src'
>   rm -f bin{,-*}/*.* bin{,-*}/.certificate.* bin{,-*}/.certificates.* bin{,-*}/.private_key.* bin{,-*}/errors bin{,-*}/NIC ./util/zbin ./util/elf2efi32 ./util/elf2efi64 /usr/bin/EfiRom ./util/efifatbin ./util/iccfix ./util/einfo TAGS bin{,-*}/symtab
>   rm: cannot remove '/usr/bin/EfiRom': Permission denied
>   make[1]: *** [Makefile.housekeeping:1564: clean] Error 1
>   make[1]: Leaving directory '/source/qemu/roms/ipxe/src'
>   make: *** [Makefile:152: clean] Error 2
>   make: Leaving directory '/source/qemu/roms'
> 
> Before f590a812c21 this variable could be overridden or unset,
> and the 'veryclean' Makefile rule would not complain.
> 
> Commit f590a812c21 enforces this variable to the Intel EfiRom
> tool provided by the EDK2 project.
> 
> To avoid the name clash and make the difference between the
> projects obvious, rename the variable used by the EDK2 project
> as EDK2_EFIROM.
> 
> Fixes: f590a812c21074e82228de3e1dfb57b75fc02b62
> Reported-by: Olaf Hering <olaf@aepfle.de>
> Reviewed-by: Laszlo Ersek <lersek@redhat.com>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> ---
>  roms/Makefile | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/roms/Makefile b/roms/Makefile
> index 78d5dd18c30..d28252dafdf 100644
> --- a/roms/Makefile
> +++ b/roms/Makefile
> @@ -47,7 +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.
>  #
> -EFIROM = edk2/BaseTools/Source/C/bin/EfiRom
> +EDK2_EFIROM = edk2/BaseTools/Source/C/bin/EfiRom
>  
>  default:
>  	@echo "nothing is build by default"
> @@ -102,8 +102,8 @@ pxe-rom-%: build-pxe-roms
>  
>  efirom: $(patsubst %,efi-rom-%,$(pxerom_variants))
>  
> -efi-rom-%: build-pxe-roms build-efi-roms $(EFIROM)
> -	$(EFIROM) -f "0x$(VID)" -i "0x$(DID)" -l 0x02 \
> +efi-rom-%: build-pxe-roms build-efi-roms $(EDK2_EFIROM)
> +	$(EDK2_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 \
>  		-ec ipxe/src/bin-x86_64-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_EFIROM):
>  	$(MAKE) -C edk2/BaseTools
>  
>  slof:
> 

looks good, thank you; my R-b stands.

Laszlo

Re: [Qemu-devel] [PATCH for-4.0 v3 2/2] roms: Allow passing configure options to the EDK2 build tools

[Laszlo Ersek]

On 04/09/19 15:45, Philippe Mathieu-Daudé wrote:
> Since commit f590a812c210 we build the EDK2 EfiRom utility
> unconditionally.
> 
> Some distributions require to use extra compiler/linker flags,
> i.e. SUSE which enforces the PIE protection (see [*]).
> 
> EDK2 build tools already provide a set of variables for that,
> use them to allow the caller to easily inject compiler/linker
> options..
> 
> Now build scripts can pass extra options, example:
> 
>   $ make -C roms \
>       EDK2_BASETOOLS_OPTFLAGS='-fPIE' \
>       efirom
> 
> [*] https://lists.opensuse.org/opensuse-factory/2017-06/msg00403.html
> 
> Reported-by: Olaf Hering <olaf@aepfle.de>
> Suggested-by: Laszlo Ersek <lersek@redhat.com>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> ---
>  roms/Makefile | 15 ++++++++++++++-
>  1 file changed, 14 insertions(+), 1 deletion(-)
> 
> diff --git a/roms/Makefile b/roms/Makefile
> index d28252dafdf..1ff78b63bb3 100644
> --- a/roms/Makefile
> +++ b/roms/Makefile
> @@ -120,8 +120,21 @@ build-efi-roms: build-pxe-roms
>  		$(patsubst %,bin-i386-efi/%.efidrv,$(pxerom_targets)) \
>  		$(patsubst %,bin-x86_64-efi/%.efidrv,$(pxerom_targets))
>  
> +# Build scripts can pass compiler/linker flags to the EDK2 build tools
> +# via the EDK2_BASETOOLS_OPTFLAGS (CPPFLAGS and CFLAGS) and
> +# EDK2_BASETOOLS_LDFLAGS (LDFLAGS) environment variables.
> +#
> +# Example:
> +#
> +#  make -C roms \
> +#    EDK2_BASETOOLS_OPTFLAGS='...' \
> +#    EDK2_BASETOOLS_LDFLAGS='...' \
> +#    efirom
> +#
>  $(EDK2_EFIROM):
> -	$(MAKE) -C edk2/BaseTools
> +	$(MAKE) -C edk2/BaseTools \
> +		EXTRA_OPTFLAGS='$(EDK2_BASETOOLS_OPTFLAGS)' \
> +		EXTRA_LDFLAGS='$(EDK2_BASETOOLS_LDFLAGS)'
>  
>  slof:
>  	$(MAKE) -C SLOF CROSS=$(powerpc64_cross_prefix) qemu
> 

Reviewed-by: Laszlo Ersek <lersek@redhat.com>

Re: [Qemu-devel] [PATCH for-4.0 v3 2/2] roms: Allow passing configure options to the EDK2 build tools

[Laszlo Ersek]

On 04/09/19 15:45, Philippe Mathieu-Daudé wrote:
> Since commit f590a812c210 we build the EDK2 EfiRom utility
> unconditionally.
> 
> Some distributions require to use extra compiler/linker flags,
> i.e. SUSE which enforces the PIE protection (see [*]).
> 
> EDK2 build tools already provide a set of variables for that,
> use them to allow the caller to easily inject compiler/linker
> options..
> 
> Now build scripts can pass extra options, example:
> 
>   $ make -C roms \
>       EDK2_BASETOOLS_OPTFLAGS='-fPIE' \
>       efirom
> 
> [*] https://lists.opensuse.org/opensuse-factory/2017-06/msg00403.html
> 
> Reported-by: Olaf Hering <olaf@aepfle.de>
> Suggested-by: Laszlo Ersek <lersek@redhat.com>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> ---
>  roms/Makefile | 15 ++++++++++++++-
>  1 file changed, 14 insertions(+), 1 deletion(-)
> 
> diff --git a/roms/Makefile b/roms/Makefile
> index d28252dafdf..1ff78b63bb3 100644
> --- a/roms/Makefile
> +++ b/roms/Makefile
> @@ -120,8 +120,21 @@ build-efi-roms: build-pxe-roms
>  		$(patsubst %,bin-i386-efi/%.efidrv,$(pxerom_targets)) \
>  		$(patsubst %,bin-x86_64-efi/%.efidrv,$(pxerom_targets))
>  
> +# Build scripts can pass compiler/linker flags to the EDK2 build tools
> +# via the EDK2_BASETOOLS_OPTFLAGS (CPPFLAGS and CFLAGS) and
> +# EDK2_BASETOOLS_LDFLAGS (LDFLAGS) environment variables.
> +#
> +# Example:
> +#
> +#  make -C roms \
> +#    EDK2_BASETOOLS_OPTFLAGS='...' \
> +#    EDK2_BASETOOLS_LDFLAGS='...' \
> +#    efirom
> +#
>  $(EDK2_EFIROM):
> -	$(MAKE) -C edk2/BaseTools
> +	$(MAKE) -C edk2/BaseTools \
> +		EXTRA_OPTFLAGS='$(EDK2_BASETOOLS_OPTFLAGS)' \
> +		EXTRA_LDFLAGS='$(EDK2_BASETOOLS_LDFLAGS)'
>  
>  slof:
>  	$(MAKE) -C SLOF CROSS=$(powerpc64_cross_prefix) qemu
> 

Reviewed-by: Laszlo Ersek <lersek@redhat.com>

Re: [Qemu-devel] [PATCH for-4.0 v3 0/2] roms: Avoid iPXE/EDK2 EFIROM variable clash, pass CFLAGS to EDK2 build tools

[Laszlo Ersek]

On 04/09/19 16:00, Peter Maydell wrote:
> On Tue, 9 Apr 2019 at 14:47, Philippe Mathieu-Daudé <philmd@redhat.com> wrote:
>>
>> Hi,
>>
>> Two trivial fixes to avoid the latest EDK2 testing series to
>> cause trouble to downstream distributions (in particular if
>> they have PIE enforced).
>>
>> Since v2:
>> - addressed Laszlo's suggestion
>> - reworded patch description
>>
>> v2: https://lists.gnu.org/archive/html/qemu-devel/2019-04/msg01033.html
>> v1: https://lists.gnu.org/archive/html/qemu-devel/2019-04/msg01006.html
>> error report: https://lists.gnu.org/archive/html/qemu-devel/2019-04/msg00992.html
>>
>> Regards,
>>
>> Phil.
>>
>> Philippe Mathieu-Daudé (2):
>>   roms: Rename the EFIROM variable to avoid clashing with iPXE
>>   roms: Allow passing configure options to the EDK2 build tools
>>
>>  roms/Makefile | 23 ++++++++++++++++++-----
>>  1 file changed, 18 insertions(+), 5 deletions(-)
> 
> Thanks. This is one of a handful of remaining bugs we
> should fix for 4.0, so I would ideally like to get this
> in rc3 today (or tomorrow if we have to slip rc3 a day).
> If people wanting to review this series could do so
> promptly I'd definitely appreciate it.
> 
> -- PMM
> 

I hope I was quick enough.

Thanks,
Laszlo

Re: [Qemu-devel] [PATCH for-4.0 v3 0/2] roms: Avoid iPXE/EDK2 EFIROM variable clash, pass CFLAGS to EDK2 build tools

[Laszlo Ersek]

On 04/09/19 16:00, Peter Maydell wrote:
> On Tue, 9 Apr 2019 at 14:47, Philippe Mathieu-Daudé <philmd@redhat.com> wrote:
>>
>> Hi,
>>
>> Two trivial fixes to avoid the latest EDK2 testing series to
>> cause trouble to downstream distributions (in particular if
>> they have PIE enforced).
>>
>> Since v2:
>> - addressed Laszlo's suggestion
>> - reworded patch description
>>
>> v2: https://lists.gnu.org/archive/html/qemu-devel/2019-04/msg01033.html
>> v1: https://lists.gnu.org/archive/html/qemu-devel/2019-04/msg01006.html
>> error report: https://lists.gnu.org/archive/html/qemu-devel/2019-04/msg00992.html
>>
>> Regards,
>>
>> Phil.
>>
>> Philippe Mathieu-Daudé (2):
>>   roms: Rename the EFIROM variable to avoid clashing with iPXE
>>   roms: Allow passing configure options to the EDK2 build tools
>>
>>  roms/Makefile | 23 ++++++++++++++++++-----
>>  1 file changed, 18 insertions(+), 5 deletions(-)
> 
> Thanks. This is one of a handful of remaining bugs we
> should fix for 4.0, so I would ideally like to get this
> in rc3 today (or tomorrow if we have to slip rc3 a day).
> If people wanting to review this series could do so
> promptly I'd definitely appreciate it.
> 
> -- PMM
> 

I hope I was quick enough.

Thanks,
Laszlo

Re: [Qemu-devel] [PATCH for-4.0 v3 0/2] roms: Avoid iPXE/EDK2 EFIROM variable clash, pass CFLAGS to EDK2 build tools

[Peter Maydell]

On Tue, 9 Apr 2019 at 16:25, Laszlo Ersek <lersek@redhat.com> wrote:
>
> On 04/09/19 16:00, Peter Maydell wrote:
> > Thanks. This is one of a handful of remaining bugs we
> > should fix for 4.0, so I would ideally like to get this
> > in rc3 today (or tomorrow if we have to slip rc3 a day).
> > If people wanting to review this series could do so
> > promptly I'd definitely appreciate it.
>
> I hope I was quick enough.

Yep, thanks. (I knew you'd had comments on v1 so I was
assuming you'd want to review v2.)

thanks
-- PMM

Re: [Qemu-devel] [PATCH for-4.0 v3 0/2] roms: Avoid iPXE/EDK2 EFIROM variable clash, pass CFLAGS to EDK2 build tools

[Peter Maydell]

On Tue, 9 Apr 2019 at 16:25, Laszlo Ersek <lersek@redhat.com> wrote:
>
> On 04/09/19 16:00, Peter Maydell wrote:
> > Thanks. This is one of a handful of remaining bugs we
> > should fix for 4.0, so I would ideally like to get this
> > in rc3 today (or tomorrow if we have to slip rc3 a day).
> > If people wanting to review this series could do so
> > promptly I'd definitely appreciate it.
>
> I hope I was quick enough.

Yep, thanks. (I knew you'd had comments on v1 so I was
assuming you'd want to review v2.)

thanks
-- PMM