[PATCH] hvmloader: fix build with LLVM Linker

[PATCH] hvmloader: fix build with LLVM Linker

[Roger Pau Monne]

The hvmloader binary generated when using LLVM LD doesn't work
properly and seems to get stuck while trying to generate and load the
ACPI tables. This is caused by the layout of the binary when linked
with LLVM LD.

LLVM LD has a different default linker script that GNU LD, and the
resulting hvmloader binary is slightly different:

LLVM LD:
Program Headers:
  Type           Offset   VirtAddr   PhysAddr   FileSiz MemSiz  Flg Align
  PHDR           0x000034 0x000ff034 0x000ff034 0x00060 0x00060 R   0x4
  LOAD           0x000000 0x000ff000 0x000ff000 0x38000 0x38000 RWE 0x1000
  GNU_STACK      0x000000 0x00000000 0x00000000 0x00000 0x00000 RW  0

GNU LD:
Program Headers:
  Type           Offset   VirtAddr   PhysAddr   FileSiz MemSiz  Flg Align
  LOAD           0x000080 0x00100000 0x00100000 0x36308 0x3fd74 RWE 0x10
  GNU_STACK      0x000000 0x00000000 0x00000000 0x00000 0x00000 RW  0x4

Note that in the LLVM LD case (as with GNU LD) the .text section does
indeed have the address set to 0x100000 as requested on the command
line:

[ 1] .text             PROGBITS        00100000 001000 00dd10 00  AX  0   0 16

There's however the PHDR which is not present when using GNU LD.

Fix this by using a very simple linker script that generates the same
binary regardless of whether LLVM or GNU LD is used. By using a linker
script the usage of -Ttext can also be avoided by placing the desired
.text load address directly in the linker script.

Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
---
Jan Beulich <jbeulich@suse.com>
Andrew Cooper <andrew.cooper3@citrix.com>
Wei Liu <wei.liu2@citrix.com>
Ian Jackson <ian.jackson@eu.citrix.com>
---
 tools/firmware/hvmloader/Makefile      |  7 ++-----
 tools/firmware/hvmloader/hvmloader.lds | 13 +++++++++++++
 2 files changed, 15 insertions(+), 5 deletions(-)
 create mode 100644 tools/firmware/hvmloader/hvmloader.lds

diff --git a/tools/firmware/hvmloader/Makefile b/tools/firmware/hvmloader/Makefile
index 496ac72b77..e980ce7c5f 100644
--- a/tools/firmware/hvmloader/Makefile
+++ b/tools/firmware/hvmloader/Makefile
@@ -20,9 +20,6 @@
 XEN_ROOT = $(CURDIR)/../../..
 include $(XEN_ROOT)/tools/firmware/Rules.mk
 
-
-LOADADDR = 0x100000
-
 # SMBIOS spec requires format mm/dd/yyyy
 SMBIOS_REL_DATE ?= $(shell date +%m/%d/%Y)
 
@@ -82,8 +79,8 @@ vpath build.c $(ACPI_PATH)
 vpath static_tables.c $(ACPI_PATH)
 OBJS += $(ACPI_OBJS)
 
-hvmloader: $(OBJS)
-	$(LD) $(LDFLAGS_DIRECT) -N -Ttext $(LOADADDR) -o $@ $^
+hvmloader: $(OBJS) hvmloader.lds
+	$(LD) $(LDFLAGS_DIRECT) -N -T hvmloader.lds -o $@ $(OBJS)
 
 roms.inc: $(ROMS)
 	echo "/* Autogenerated file. DO NOT EDIT */" > $@.new
diff --git a/tools/firmware/hvmloader/hvmloader.lds b/tools/firmware/hvmloader/hvmloader.lds
new file mode 100644
index 0000000000..15d8f38fff
--- /dev/null
+++ b/tools/firmware/hvmloader/hvmloader.lds
@@ -0,0 +1,13 @@
+SECTIONS
+{
+  . = 0x100000;
+  /*
+   * NB: there's no need to use the AT keyword in order to set the LMA, by
+   * default the linker will use VMA = LMA unless specified otherwise.
+   */
+  .text : { *(.text) }
+  .rodata : { *(.rodata) }
+  .data : { *(.data) }
+  .bss : { *(.bss) }
+  _end = .;
+}
-- 
2.18.0


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [PATCH] hvmloader: fix build with LLVM Linker

[Jan Beulich]

>>> On 24.08.18 at 11:58, <roger.pau@citrix.com> wrote:
> --- /dev/null
> +++ b/tools/firmware/hvmloader/hvmloader.lds
> @@ -0,0 +1,13 @@
> +SECTIONS
> +{
> +  . = 0x100000;
> +  /*
> +   * NB: there's no need to use the AT keyword in order to set the LMA, by
> +   * default the linker will use VMA = LMA unless specified otherwise.
> +   */
> +  .text : { *(.text) }
> +  .rodata : { *(.rodata) }
> +  .data : { *(.data) }
> +  .bss : { *(.bss) }
> +  _end = .;
> +}

Is this really sufficient? Iirc the compiler could create quite a few
more variants of the sections named above, like .rodata.str* or
.text.cold. Hence at the very least I'd expect .<section> on the
right sides above to be accompanied by .<section>.* .

Jan



_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [PATCH] hvmloader: fix build with LLVM Linker

[Roger Pau Monné]

On Mon, Aug 27, 2018 at 03:50:39AM -0600, Jan Beulich wrote:
> >>> On 24.08.18 at 11:58, <roger.pau@citrix.com> wrote:
> > --- /dev/null
> > +++ b/tools/firmware/hvmloader/hvmloader.lds
> > @@ -0,0 +1,13 @@
> > +SECTIONS
> > +{
> > +  . = 0x100000;
> > +  /*
> > +   * NB: there's no need to use the AT keyword in order to set the LMA, by
> > +   * default the linker will use VMA = LMA unless specified otherwise.
> > +   */
> > +  .text : { *(.text) }
> > +  .rodata : { *(.rodata) }
> > +  .data : { *(.data) }
> > +  .bss : { *(.bss) }
> > +  _end = .;
> > +}
> 
> Is this really sufficient? Iirc the compiler could create quite a few
> more variants of the sections named above, like .rodata.str* or
> .text.cold. Hence at the very least I'd expect .<section> on the
> right sides above to be accompanied by .<section>.* .

Right, ATM the compiler I'm using doesn't generate any of those, but
you are correct that this could be the case.

Do you have a preference for adding .<section>.* instead of just using
a single .<section>*?

Thanks, Roger.

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [PATCH] hvmloader: fix build with LLVM Linker

[Jan Beulich]

>>> On 03.09.18 at 16:19, <roger.pau@citrix.com> wrote:
> On Mon, Aug 27, 2018 at 03:50:39AM -0600, Jan Beulich wrote:
>> >>> On 24.08.18 at 11:58, <roger.pau@citrix.com> wrote:
>> > --- /dev/null
>> > +++ b/tools/firmware/hvmloader/hvmloader.lds
>> > @@ -0,0 +1,13 @@
>> > +SECTIONS
>> > +{
>> > +  . = 0x100000;
>> > +  /*
>> > +   * NB: there's no need to use the AT keyword in order to set the LMA, by
>> > +   * default the linker will use VMA = LMA unless specified otherwise.
>> > +   */
>> > +  .text : { *(.text) }
>> > +  .rodata : { *(.rodata) }
>> > +  .data : { *(.data) }
>> > +  .bss : { *(.bss) }
>> > +  _end = .;
>> > +}
>> 
>> Is this really sufficient? Iirc the compiler could create quite a few
>> more variants of the sections named above, like .rodata.str* or
>> .text.cold. Hence at the very least I'd expect .<section> on the
>> right sides above to be accompanied by .<section>.* .
> 
> Right, ATM the compiler I'm using doesn't generate any of those, but
> you are correct that this could be the case.
> 
> Do you have a preference for adding .<section>.* instead of just using
> a single .<section>*?

Yes, I think we shouldn't match more aggressively than we have to.
.text.* etc are well known forms, and while .text1 etc may also be,
I wouldn't want to match more exotic ones like .textual.

Jan



_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel