[LinuxBIOS] v3: Miscompilation problems

[ron minnich]

On Nov 26, 2007 2:12 AM, Carl-Daniel Hailfinger
<c-d.hailfinger.devel.2006 at gmx.net> wrote:

> After spending hours looking at the generated assembly output, it seems
> the problem is NOT with the compiler, but with the linker. Basically,
> for XIP code, only the first function in the assembly output is
> executed. With -funit-at-a-time, that was usually the function placed
> last in the code. So I'm now chasing a bug either in our linker commands
> or in binutils.

I think we can take a better approach. The real problem is the binary
file created with objcopy. When we use a binary file, we remove a lot
of useful information. Since lar can now handle ELF, we should take
advantage of that, rather than forever relying on the order in which
gld places functions in files.

So how do we fix this? It's a few steps.

First, have the mainboard makefile link the initram starting at 0, not
the linux application default of 0x08whatever. Note the -Ttext 0
addition below to ld. What does this do? It gives us an entry point
that is in effect an offset from the start of the initram, so we can
just add it to the starting point of initram.

+       $(Q)$(LD) -Ttext 0 --entry main -N -R $(obj)/stage0-prefixed.o \
+               $(patsubst %.o,%_xip.o,$(INITRAM_OBJ)) -o
$(obj)/linuxbios.initram.o

This actually is only one step. The problem is that LAR is processing
the binary blob of initram, not the ELF file. So lots of info is gone.
Now that LAR can process ELF files, we should stop using binary files
as much as we can.

Have LAR use the initram.o, not the binary version of initram:
Index: arch/x86/Makefile
===================================================================
--- arch/x86/Makefile   (revision 518)
+++ arch/x86/Makefile   (working copy)
@@ -36,7 +36,7 @@

 ROM_SIZE := $(shell expr $(CONFIG_LINUXBIOS_ROMSIZE_KB) \* 1024)

-LARFILES := nocompress:normal/initram normal/stage2
nocompress:normal/option_table
+LARFILES := nocompress:normal/initram.o normal/stage2
nocompress:normal/option_table
 ifneq ($(CONFIG_PAYLOAD_NONE),y)
 LARFILES += normal/payload
 endif
@@ -57,11 +57,11 @@
 COMPRESSFLAG := -C nrv2b
 endif

-$(obj)/linuxbios.rom $(obj)/linuxbios.map: $(obj)/linuxbios.bootblock
$(obj)/util/lar/lar lzma nrv2b $(obj)/linuxbios.initram
$(obj)/linuxbios.stage2 $(obj)/option_table
+$(obj)/linuxbios.rom $(obj)/linuxbios.map: $(obj)/linuxbios.bootblock
$(obj)/util/lar/lar lzma nrv2b $(obj)/linuxbios.initram.o
$(obj)/linuxbios.stage2 $(obj)/option_table
        $(Q)rm -rf $(obj)/lar.tmp
        $(Q)mkdir $(obj)/lar.tmp
        $(Q)mkdir $(obj)/lar.tmp/normal
-       $(Q)cp $(obj)/linuxbios.initram $(obj)/lar.tmp/normal/initram
+       $(Q)cp $(obj)/linuxbios.initram.o $(obj)/lar.tmp/normal/initram.o
        $(Q)cp $(obj)/linuxbios.stage2 $(obj)/lar.tmp/normal/stage2
        $(Q)cp $(obj)/option_table $(obj)/lar.tmp/normal/option_table
 ifeq ($(CONFIG_PAYLOAD_NONE),y)

When it builds, we get this in the LAR:
  normal/initram.o/segment0 (4948 bytes @0x13bd0);loadaddress 0x0 entry 0x0x6c

note the entry: 0x6c, a nice clean offset from the start. At runtime,
this can be added to the start of the lar entry and we will have a
good entry point.

Now, the name has changed: normal/initram.o/segment0, so we change
stage1.c just a bit:
Index: arch/x86/stage1.c
===================================================================
--- arch/x86/stage1.c   (revision 518)
+++ arch/x86/stage1.c   (working copy)
@@ -138,10 +138,10 @@
        // find first initram
        if (check_normal_boot_flag()) {
                printk(BIOS_DEBUG, "Choosing normal boot.\n");
-               ret = execute_in_place(&archive, "normal/initram");
+               ret = execute_in_place(&archive, "normal/initram.o/segment0");
        } else {
                printk(BIOS_DEBUG, "Choosing fallback boot.\n");
-               ret = execute_in_place(&archive, "fallback/initram");
+               ret = execute_in_place(&archive, "fallback/initram.o/segment0");
                /* Try a normal boot if fallback doesn't exist in the lar.
                 * TODO: There are other ways to do this.
                 * It could be ifdef or the boot flag could be forced.

So the name is a but clumsier but overall this is better. We're using
the ability of LAR to parse elf, and we simplify our messing around
with ld.

Comments?

ron