qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
* [Qemu-devel] [PATCH] optionrom: fix compilation with mingw docker target
@ 2016-08-09 20:56 Paolo Bonzini
  2016-08-09 21:47 ` Peter Maydell
  0 siblings, 1 reply; 3+ messages in thread
From: Paolo Bonzini @ 2016-08-09 20:56 UTC (permalink / raw)
  To: qemu-devel; +Cc: rjones

Two fixes are needed.  First, mingw does not have -D_FORTIFY_SOURCE,
hence --enable-debug disables optimization.  This is not acceptable
for ROMs, which should override CFLAGS to force inclusion of -O2.

Second, PE stores global constructors and destructors using the
following linker script snippet:

     ___CTOR_LIST__ = .; __CTOR_LIST__ = . ;
			LONG (-1);*(.ctors); *(.ctor); *(SORT(.ctors.*));  LONG (0);
     ___DTOR_LIST__ = .; __DTOR_LIST__ = . ;
			LONG (-1); *(.dtors); *(.dtor); *(SORT(.dtors.*));  LONG (0);

The LONG directives cause the .img files to be 16 bytes too large;
the recently added check to signrom.py catches this.  To fix this,
replace -T and -e options with a linker script.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 pc-bios/optionrom/Makefile | 10 +++++++++-
 pc-bios/optionrom/flat.lds |  6 ++++++
 2 files changed, 15 insertions(+), 1 deletion(-)
 create mode 100644 pc-bios/optionrom/flat.lds

diff --git a/pc-bios/optionrom/Makefile b/pc-bios/optionrom/Makefile
index 9c018ea..8aef152 100644
--- a/pc-bios/optionrom/Makefile
+++ b/pc-bios/optionrom/Makefile
@@ -9,6 +9,14 @@ $(call set-vpath, $(SRC_PATH)/pc-bios/optionrom)
 
 .PHONY : all clean build-all
 
+# Compiling with no optimization creates ROMs that are too large
+ifeq ($(filter -O%, $(CFLAGS)),)
+override CFLAGS += -O2
+endif
+ifeq ($(filter -O%, $(CFLAGS)),-O0)
+override CFLAGS += -O2
+endif
+
 # Drop -fstack-protector and the like
 QEMU_CFLAGS := $(filter -W%, $(QEMU_CFLAGS)) $(CFLAGS_NOPIE) -ffreestanding
 QEMU_CFLAGS += $(call cc-option, $(QEMU_CFLAGS), -fno-stack-protector)
@@ -51,7 +59,7 @@ endif
 endif
 
 %.img: %.o
-	$(call quiet-command,$(LD) $(LDFLAGS_NOPIE) -m $(LD_EMULATION) -Ttext 0 -e _start -s -o $@ $<,"  Building $(TARGET_DIR)$@")
+	$(call quiet-command,$(LD) $(LDFLAGS_NOPIE) -m $(LD_EMULATION) -T $(SRC_PATH)/pc-bios/optionrom/flat.lds -s -o $@ $<,"  Building $(TARGET_DIR)$@")
 
 %.raw: %.img
 	$(call quiet-command,$(OBJCOPY) -O binary -j .text $< $@,"  Building $(TARGET_DIR)$@")
diff --git a/pc-bios/optionrom/flat.lds b/pc-bios/optionrom/flat.lds
new file mode 100644
index 0000000..cee2eda
--- /dev/null
+++ b/pc-bios/optionrom/flat.lds
@@ -0,0 +1,6 @@
+SECTIONS
+{
+  . = 0;
+  .text : { *(.text) *(.text.$) }
+}
+ENTRY(_start)
-- 
1.8.3.1

^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [Qemu-devel] [PATCH] optionrom: fix compilation with mingw docker target
  2016-08-09 20:56 [Qemu-devel] [PATCH] optionrom: fix compilation with mingw docker target Paolo Bonzini
@ 2016-08-09 21:47 ` Peter Maydell
  2016-08-09 22:09   ` Paolo Bonzini
  0 siblings, 1 reply; 3+ messages in thread
From: Peter Maydell @ 2016-08-09 21:47 UTC (permalink / raw)
  To: Paolo Bonzini; +Cc: QEMU Developers, Richard W.M. Jones

On 9 August 2016 at 21:56, Paolo Bonzini <pbonzini@redhat.com> wrote:
> Two fixes are needed.  First, mingw does not have -D_FORTIFY_SOURCE,
> hence --enable-debug disables optimization.  This is not acceptable
> for ROMs, which should override CFLAGS to force inclusion of -O2.
>
> Second, PE stores global constructors and destructors using the
> following linker script snippet:
>
>      ___CTOR_LIST__ = .; __CTOR_LIST__ = . ;
>                         LONG (-1);*(.ctors); *(.ctor); *(SORT(.ctors.*));  LONG (0);
>      ___DTOR_LIST__ = .; __DTOR_LIST__ = . ;
>                         LONG (-1); *(.dtors); *(.dtor); *(SORT(.dtors.*));  LONG (0);
>
> The LONG directives cause the .img files to be 16 bytes too large;
> the recently added check to signrom.py catches this.  To fix this,
> replace -T and -e options with a linker script.

...this is really starting to strongly suggest to me that we should
not be trying to build our ROM images with whatever random
maybe-this-is-an-x86-compiler $CC happens to be...

thanks
-- PMM

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [Qemu-devel] [PATCH] optionrom: fix compilation with mingw docker target
  2016-08-09 21:47 ` Peter Maydell
@ 2016-08-09 22:09   ` Paolo Bonzini
  0 siblings, 0 replies; 3+ messages in thread
From: Paolo Bonzini @ 2016-08-09 22:09 UTC (permalink / raw)
  To: Peter Maydell; +Cc: QEMU Developers, Richard W.M. Jones



----- Original Message -----
> From: "Peter Maydell" <peter.maydell@linaro.org>
> To: "Paolo Bonzini" <pbonzini@redhat.com>
> Cc: "QEMU Developers" <qemu-devel@nongnu.org>, "Richard W.M. Jones" <rjones@redhat.com>
> Sent: Tuesday, August 9, 2016 11:47:17 PM
> Subject: Re: [Qemu-devel] [PATCH] optionrom: fix compilation with mingw docker target
> 
> On 9 August 2016 at 21:56, Paolo Bonzini <pbonzini@redhat.com> wrote:
> > Two fixes are needed.  First, mingw does not have -D_FORTIFY_SOURCE,
> > hence --enable-debug disables optimization.  This is not acceptable
> > for ROMs, which should override CFLAGS to force inclusion of -O2.
> >
> > Second, PE stores global constructors and destructors using the
> > following linker script snippet:
> >
> >      ___CTOR_LIST__ = .; __CTOR_LIST__ = . ;
> >                         LONG (-1);*(.ctors); *(.ctor); *(SORT(.ctors.*));
> >                         LONG (0);
> >      ___DTOR_LIST__ = .; __DTOR_LIST__ = . ;
> >                         LONG (-1); *(.dtors); *(.dtor); *(SORT(.dtors.*));
> >                         LONG (0);
> >
> > The LONG directives cause the .img files to be 16 bytes too large;
> > the recently added check to signrom.py catches this.  To fix this,
> > replace -T and -e options with a linker script.
> 
> ...this is really starting to strongly suggest to me that we should
> not be trying to build our ROM images with whatever random
> maybe-this-is-an-x86-compiler $CC happens to be...

Well, to be precise the problems are in the linker (both the "emulation"
thing to this one), not the compiler.  That's where most of the variability
is between OSes indeed, but the GNU linker is very customizable.  So
after this patch this is looking a lot like what firmware build systems
do (and kvm-unit-tests too), which is in some sense a good sign.

If anything, pc-bios/ should be separately configured and should not use
anything from the output of the toplevel configure script.  That's a
different project and would let everyone compile all ROMs locally just
by installing a suitable cross-compiler.  It would be nice to have...

Paolo

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2016-08-09 22:09 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-08-09 20:56 [Qemu-devel] [PATCH] optionrom: fix compilation with mingw docker target Paolo Bonzini
2016-08-09 21:47 ` Peter Maydell
2016-08-09 22:09   ` Paolo Bonzini

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).