qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: Laszlo Ersek <lersek@redhat.com>
To: Olaf Hering <olaf@aepfle.de>
Cc: "Philippe Mathieu-Daudé" <philmd@redhat.com>,
	qemu-devel@nongnu.org, "Igor Mammedov" <imammedo@redhat.com>,
	"Gerd Hoffmann" <kraxel@redhat.com>,
	"Michael S . Tsirkin" <mst@redhat.com>
Subject: Re: [Qemu-devel] [PATCH for-4.0 v2 2/2] roms: Allow the EDK2_EFIROM variable to be overridden
Date: Wed, 10 Apr 2019 16:54:50 +0200	[thread overview]
Message-ID: <6746a549-3ebf-80f3-e67a-62e536aa06be@redhat.com> (raw)
In-Reply-To: <20190410082554.6c595b3f.olaf@aepfle.de>

On 04/10/19 08:25, Olaf Hering wrote:
> Am Mon, 8 Apr 2019 13:05:07 +0200
> schrieb Laszlo Ersek <lersek@redhat.com>:
>
>> Then build scripts could be updated to invoke:
>>
>>   make -C roms \
>>     EDK2_BASETOOLS_OPTFLAGS='...' \
>>     EDK2_BASETOOLS_LDFLAGS='...' \
>>     efirom
>
> The question remains: 'But why?'.

Because it lets you pass "-fno-pie" & friends.

> Why can edk2 not be built with '-fno-pie' right away?

Those flags are not universal across gcc/binutils versions.

Some gcc/binutils versions don't enable PIE to begin with.

Some gcc/binutils versions take different PIE-disablement flags
(considering both the compiler and the linker) from other gcc/binutils
versions.

For example, please refer to the following *iPXE* commit:

> commit 7c395b0e21806b946fe944a27fc273407f357ea1
> Author: Michael Brown <mcb30@ipxe.org>
> Date:   Wed Jun 14 12:33:16 2017 +0100
>
>     [build] Use -no-pie on newer versions of gcc
>
>     Some distributions patch gcc to generate position independent
>     executables by default.  We currently include a workaround to check
>     for this and to add -fno-PIE -nopie to CFLAGS if required.
>
>     Newer patched versions of gcc require -fno-PIE -no-pie instead.  Check
>     for both variants.
>
>     Reported-by: Nathan Rennie-Waldock <nathan.renniewaldock@gmail.com>
>     Originally-fixed-by: Markos Chandras <mchandras@suse.de>
>     Signed-off-by: Michael Brown <mcb30@ipxe.org>

(I remember this commit because the logic it had added actually failed
on a system that I used once for building iPXE -- it was an x86_64
system with both a native gcc, and a *different version*
x86_64-to-x86_64 *cross* gcc. The selection logic determined the flags
for the one compiler toolchain, but then passed the flags to the other
compiler toolchain. The build broke. I narrowed it down to the above
commit, and then shrugged it off; it wasn't important enough to spend
more time on it.)

I also refer you to the following *edk2* commit:

> commit 11d0cd23dd1bc15a6e6a1598250ea2e0c4c36e9a
> Author: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> Date:   Mon Jun 18 10:23:49 2018 +0200
>
>     BaseTools/tools_def IA32: drop -no-pie linker option for GCC49
>
>     As reported by Liming, GCC 4.9.2 does not support the -no-pie
>     linker option that we added to the GCC49 and GCC5 toolchain
>     profiles in commit c25d3905523a ("BaseTools/tools_def IA32:
>     disable PIE code generation explicitly") to work around issues
>     with recent distro toolchains that enable PIE code generation
>     by default.
>
>     So rollback the changes for GCC49 but preserve them for GCC5
>
>     Contributed-under: TianoCore Contribution Agreement 1.1
>     Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
>     Acked-by: Laszlo Ersek <lersek@redhat.com>

The above facility will let you stuff the options into the edk2
BaseTools build, in your downstream qemu.spec file, that match your
downstream gcc/binutils version.

Most build hosts will need no flags.

> Did noone approach the edk2 developers yet that their buildsystem is
> broken in that regard?

Well, have you?

I don't remember anyone else reporting such an issue yet, on edk2-devel
or elsewhere, with building BaseTools. I certainly remember
collaborating with Gary Lin from SUSE on various stuff, but not this.

You are welcome to file a bug at <https://bugzilla.tianocore.org/>.
Please pick "EDK2" for "Product", "Code" for "Component", and
"BaseTools" for "Package".

... Please don't try to force a "zero build-interface changes" policy on
upstream, just so you can avoid a small tweak in a downstream build
script when you rebase. Not even the gcc/binutils command lines conform
to that. We all hope downstream rebases to be painless, and upstreams do
strive to help with that, but the downstream rebase experience is never
*entirely* painless (or automated).

Thanks,
Laszlo

WARNING: multiple messages have this Message-ID (diff)
From: Laszlo Ersek <lersek@redhat.com>
To: Olaf Hering <olaf@aepfle.de>
Cc: "Igor Mammedov" <imammedo@redhat.com>,
	"Michael S . Tsirkin" <mst@redhat.com>,
	"Philippe Mathieu-Daudé" <philmd@redhat.com>,
	qemu-devel@nongnu.org, "Gerd Hoffmann" <kraxel@redhat.com>
Subject: Re: [Qemu-devel] [PATCH for-4.0 v2 2/2] roms: Allow the EDK2_EFIROM variable to be overridden
Date: Wed, 10 Apr 2019 16:54:50 +0200	[thread overview]
Message-ID: <6746a549-3ebf-80f3-e67a-62e536aa06be@redhat.com> (raw)
Message-ID: <20190410145450.e7QqZAcXVByYjZNkvsijjbMD0HPf8OE9PpSCOWRoY5Y@z> (raw)
In-Reply-To: <20190410082554.6c595b3f.olaf@aepfle.de>

On 04/10/19 08:25, Olaf Hering wrote:
> Am Mon, 8 Apr 2019 13:05:07 +0200
> schrieb Laszlo Ersek <lersek@redhat.com>:
>
>> Then build scripts could be updated to invoke:
>>
>>   make -C roms \
>>     EDK2_BASETOOLS_OPTFLAGS='...' \
>>     EDK2_BASETOOLS_LDFLAGS='...' \
>>     efirom
>
> The question remains: 'But why?'.

Because it lets you pass "-fno-pie" & friends.

> Why can edk2 not be built with '-fno-pie' right away?

Those flags are not universal across gcc/binutils versions.

Some gcc/binutils versions don't enable PIE to begin with.

Some gcc/binutils versions take different PIE-disablement flags
(considering both the compiler and the linker) from other gcc/binutils
versions.

For example, please refer to the following *iPXE* commit:

> commit 7c395b0e21806b946fe944a27fc273407f357ea1
> Author: Michael Brown <mcb30@ipxe.org>
> Date:   Wed Jun 14 12:33:16 2017 +0100
>
>     [build] Use -no-pie on newer versions of gcc
>
>     Some distributions patch gcc to generate position independent
>     executables by default.  We currently include a workaround to check
>     for this and to add -fno-PIE -nopie to CFLAGS if required.
>
>     Newer patched versions of gcc require -fno-PIE -no-pie instead.  Check
>     for both variants.
>
>     Reported-by: Nathan Rennie-Waldock <nathan.renniewaldock@gmail.com>
>     Originally-fixed-by: Markos Chandras <mchandras@suse.de>
>     Signed-off-by: Michael Brown <mcb30@ipxe.org>

(I remember this commit because the logic it had added actually failed
on a system that I used once for building iPXE -- it was an x86_64
system with both a native gcc, and a *different version*
x86_64-to-x86_64 *cross* gcc. The selection logic determined the flags
for the one compiler toolchain, but then passed the flags to the other
compiler toolchain. The build broke. I narrowed it down to the above
commit, and then shrugged it off; it wasn't important enough to spend
more time on it.)

I also refer you to the following *edk2* commit:

> commit 11d0cd23dd1bc15a6e6a1598250ea2e0c4c36e9a
> Author: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> Date:   Mon Jun 18 10:23:49 2018 +0200
>
>     BaseTools/tools_def IA32: drop -no-pie linker option for GCC49
>
>     As reported by Liming, GCC 4.9.2 does not support the -no-pie
>     linker option that we added to the GCC49 and GCC5 toolchain
>     profiles in commit c25d3905523a ("BaseTools/tools_def IA32:
>     disable PIE code generation explicitly") to work around issues
>     with recent distro toolchains that enable PIE code generation
>     by default.
>
>     So rollback the changes for GCC49 but preserve them for GCC5
>
>     Contributed-under: TianoCore Contribution Agreement 1.1
>     Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
>     Acked-by: Laszlo Ersek <lersek@redhat.com>

The above facility will let you stuff the options into the edk2
BaseTools build, in your downstream qemu.spec file, that match your
downstream gcc/binutils version.

Most build hosts will need no flags.

> Did noone approach the edk2 developers yet that their buildsystem is
> broken in that regard?

Well, have you?

I don't remember anyone else reporting such an issue yet, on edk2-devel
or elsewhere, with building BaseTools. I certainly remember
collaborating with Gary Lin from SUSE on various stuff, but not this.

You are welcome to file a bug at <https://bugzilla.tianocore.org/>.
Please pick "EDK2" for "Product", "Code" for "Component", and
"BaseTools" for "Package".

... Please don't try to force a "zero build-interface changes" policy on
upstream, just so you can avoid a small tweak in a downstream build
script when you rebase. Not even the gcc/binutils command lines conform
to that. We all hope downstream rebases to be painless, and upstreams do
strive to help with that, but the downstream rebase experience is never
*entirely* painless (or automated).

Thanks,
Laszlo


  parent reply	other threads:[~2019-04-10 14:55 UTC|newest]

Thread overview: 18+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-04-05 15:33 [Qemu-devel] [PATCH for-4.0 v2 0/2] roms: Rename the EFIROM variable and let it be overridable Philippe Mathieu-Daudé
2019-04-05 15:33 ` Philippe Mathieu-Daudé
2019-04-05 15:33 ` [Qemu-devel] [PATCH for-4.0 v2 1/2] roms: Rename the EFIROM variable to avoid clashing with iPXE Philippe Mathieu-Daudé
2019-04-05 15:33   ` Philippe Mathieu-Daudé
2019-04-08 10:54   ` Laszlo Ersek
2019-04-08 10:54     ` Laszlo Ersek
2019-04-05 15:33 ` [Qemu-devel] [PATCH for-4.0 v2 2/2] roms: Allow the EDK2_EFIROM variable to be overridden Philippe Mathieu-Daudé
2019-04-05 15:33   ` Philippe Mathieu-Daudé
2019-04-08 11:05   ` Laszlo Ersek
2019-04-08 11:05     ` Laszlo Ersek
2019-04-10  6:25     ` Olaf Hering
2019-04-10  6:25       ` Olaf Hering
2019-04-10 14:54       ` Laszlo Ersek [this message]
2019-04-10 14:54         ` Laszlo Ersek
2019-04-08  9:02 ` [Qemu-devel] [PATCH for-4.0 v2 0/2] roms: Rename the EFIROM variable and let it be overridable Laszlo Ersek
2019-04-08  9:02   ` Laszlo Ersek
2019-04-08  9:20   ` Laszlo Ersek
2019-04-08  9:20     ` Laszlo Ersek

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=6746a549-3ebf-80f3-e67a-62e536aa06be@redhat.com \
    --to=lersek@redhat.com \
    --cc=imammedo@redhat.com \
    --cc=kraxel@redhat.com \
    --cc=mst@redhat.com \
    --cc=olaf@aepfle.de \
    --cc=philmd@redhat.com \
    --cc=qemu-devel@nongnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).