| Message ID | 20220404104044.37652-3-roger.pau@citrix.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | firmware: build fixes with gcc-11 | expand |
On Mon, Apr 04, 2022 at 12:40:44PM +0200, Roger Pau Monne wrote: > Prevent the assembler from creating a .note.gnu.property section on > the output objects, as it's not useful for firmware related binaries, > and breaks the resulting rombios image. > > This requires modifying the cc-option Makefile macro so it can test > assembler options (by replacing the usage of the -S flag with -c) and > also stripping the -Wa, prefix if present when checking for the test > output. > > Signed-off-by: Roger Pau Monné <roger.pau@citrix.com> > --- > Changes since v1: > - Add the option to CFLAGS. > --- > Config.mk | 2 +- > tools/firmware/Rules.mk | 4 ++++ > 2 files changed, 5 insertions(+), 1 deletion(-) > > diff --git a/Config.mk b/Config.mk > index f56f7dc334..82832945e5 100644 > --- a/Config.mk > +++ b/Config.mk > @@ -91,7 +91,7 @@ PYTHON_PREFIX_ARG ?= --prefix="$(prefix)" > # > # Usage: cflags-y += $(call cc-option,$(CC),-march=winchip-c6,-march=i586) > cc-option = $(shell if test -z "`echo 'void*p=1;' | \ > - $(1) $(2) -S -o /dev/null -x c - 2>&1 | grep -- $(2) -`"; \ > + $(1) $(2) -c -o /dev/null -x c - 2>&1 | grep -- $(2:-Wa$(comma)%=%) -`"; \ > then echo "$(2)"; else echo "$(3)"; fi ;) Hopefully, changing "-S" to "-c" in this macro will not break anything. I would be of the opinion to create a new macro which deal with assembler options. But if that works and doesn't changes CFLAGS in the testing we do in GitLab, I guess that would be OK. Whether you introduce a macro or keep this one: Acked-by: Anthony PERARD <anthony.perard@citrix.com> Thanks,
On Mon, Apr 04, 2022 at 12:12:54PM +0100, Anthony PERARD wrote: > On Mon, Apr 04, 2022 at 12:40:44PM +0200, Roger Pau Monne wrote: > > Prevent the assembler from creating a .note.gnu.property section on > > the output objects, as it's not useful for firmware related binaries, > > and breaks the resulting rombios image. > > > > This requires modifying the cc-option Makefile macro so it can test > > assembler options (by replacing the usage of the -S flag with -c) and > > also stripping the -Wa, prefix if present when checking for the test > > output. > > > > Signed-off-by: Roger Pau Monné <roger.pau@citrix.com> > > --- > > Changes since v1: > > - Add the option to CFLAGS. > > --- > > Config.mk | 2 +- > > tools/firmware/Rules.mk | 4 ++++ > > 2 files changed, 5 insertions(+), 1 deletion(-) > > > > diff --git a/Config.mk b/Config.mk > > index f56f7dc334..82832945e5 100644 > > --- a/Config.mk > > +++ b/Config.mk > > @@ -91,7 +91,7 @@ PYTHON_PREFIX_ARG ?= --prefix="$(prefix)" > > # > > # Usage: cflags-y += $(call cc-option,$(CC),-march=winchip-c6,-march=i586) > > cc-option = $(shell if test -z "`echo 'void*p=1;' | \ > > - $(1) $(2) -S -o /dev/null -x c - 2>&1 | grep -- $(2) -`"; \ > > + $(1) $(2) -c -o /dev/null -x c - 2>&1 | grep -- $(2:-Wa$(comma)%=%) -`"; \ > > then echo "$(2)"; else echo "$(3)"; fi ;) > > Hopefully, changing "-S" to "-c" in this macro will not break anything. > I would be of the opinion to create a new macro which deal with > assembler options. But if that works and doesn't changes CFLAGS in the > testing we do in GitLab, I guess that would be OK. It looks like Linux already use "-c" for this macro, and with "-Wa," options. They just don't use grep. So asking CC to do more work here is probably fine (adding compile stage). Cheers,
On 04.04.2022 12:40, Roger Pau Monne wrote: > Prevent the assembler from creating a .note.gnu.property section on > the output objects, as it's not useful for firmware related binaries, > and breaks the resulting rombios image. > > This requires modifying the cc-option Makefile macro so it can test > assembler options (by replacing the usage of the -S flag with -c) and > also stripping the -Wa, prefix if present when checking for the test > output. I notice you've ack-ed and committed this patch, which I'm happy to see. However, I don't understand why you gave your ack here, when you did refused to ack (and to explain yourself!) for "x86emul: fix test harness build for gas 2.36". Why is this note section useful there but not similarly useful here (or, the other way around, useless)? (This, as an aside, also makes pretty clear that - unlike the title of the series suggests - this has nothing to do with gcc 11.) Jan
On Mon, Apr 04, 2022 at 02:45:05PM +0200, Jan Beulich wrote: > (This, as an aside, also makes pretty clear that - unlike the title > of the series suggests - this has nothing to do with gcc 11.) I've started debugging this as reported as an issue associated with building with gcc 11, until I realized it was the assembler that was adding such section. I guess in my mind I still had the idea of fixing the build with gcc 11, and hence the misleading subject of the cover letter. The commit messages should be fine however. Thanks, Roger.
On 04/04/2022 13:45, Jan Beulich wrote: > On 04.04.2022 12:40, Roger Pau Monne wrote: >> Prevent the assembler from creating a .note.gnu.property section on >> the output objects, as it's not useful for firmware related binaries, >> and breaks the resulting rombios image. >> >> This requires modifying the cc-option Makefile macro so it can test >> assembler options (by replacing the usage of the -S flag with -c) and >> also stripping the -Wa, prefix if present when checking for the test >> output. > I notice you've ack-ed and committed this patch, which I'm happy to > see. However, I don't understand why you gave your ack here, when you > did refused to ack (and to explain yourself!) for "x86emul: fix test > harness build for gas 2.36". Why is this note section useful there > but not similarly useful here (or, the other way around, useless)? TBH, I'd forgotten that patch. This work of Roger's came from a real XenServer regression which causes RomBIOS to explode. It needs backporting. In the longterm, I would like to see us use real linker scripts for the artefacts which have custom link requirements, because this is still a bodge. ~Andrew
diff --git a/Config.mk b/Config.mk index f56f7dc334..82832945e5 100644 --- a/Config.mk +++ b/Config.mk @@ -91,7 +91,7 @@ PYTHON_PREFIX_ARG ?= --prefix="$(prefix)" # # Usage: cflags-y += $(call cc-option,$(CC),-march=winchip-c6,-march=i586) cc-option = $(shell if test -z "`echo 'void*p=1;' | \ - $(1) $(2) -S -o /dev/null -x c - 2>&1 | grep -- $(2) -`"; \ + $(1) $(2) -c -o /dev/null -x c - 2>&1 | grep -- $(2:-Wa$(comma)%=%) -`"; \ then echo "$(2)"; else echo "$(3)"; fi ;) # cc-option-add: Add an option to compilation flags, but only if supported. diff --git a/tools/firmware/Rules.mk b/tools/firmware/Rules.mk index c227fe2524..278cca01e4 100644 --- a/tools/firmware/Rules.mk +++ b/tools/firmware/Rules.mk @@ -17,6 +17,10 @@ $(call cc-options-add,CFLAGS,CC,$(EMBEDDED_EXTRA_CFLAGS)) $(call cc-option-add,CFLAGS,CC,-fcf-protection=none) +# Do not add the .note.gnu.property section to any of the firmware objects: it +# breaks the rombios binary and is not useful for firmware anyway. +$(call cc-option-add,CFLAGS,CC,-Wa$$(comma)-mx86-used-note=no) + # Extra CFLAGS suitable for an embedded type of environment. CFLAGS += -ffreestanding -msoft-float
Prevent the assembler from creating a .note.gnu.property section on the output objects, as it's not useful for firmware related binaries, and breaks the resulting rombios image. This requires modifying the cc-option Makefile macro so it can test assembler options (by replacing the usage of the -S flag with -c) and also stripping the -Wa, prefix if present when checking for the test output. Signed-off-by: Roger Pau Monné <roger.pau@citrix.com> --- Changes since v1: - Add the option to CFLAGS. --- Config.mk | 2 +- tools/firmware/Rules.mk | 4 ++++ 2 files changed, 5 insertions(+), 1 deletion(-)