[PATCH] v3: fix calls from initram to stage1 the right way

[Carl-Daniel Hailfinger]

Current v3 code has a big problem: Shared functions.

Calling non-PIC code from PIC code needs an ABI wrapper and we don't
provide one. Our trick with function pointers is exceedingly fragile:
- it depends on gcc not being clever enough
- it forces us to compile all initram source files in one go
- parallelizing initram compilation breaks the code
- compiling one initram source file at a time breaks the code
- enabling higher optimizations breaks the code
- enabling -fwhole-program breaks the code
- declaring the function pointers const breaks the code
- it's an undocumented side effect of gcc which will go away
- we need excessively ugly shared function wrappers
- the shared function wrappers had more than their fair share of bugs
- almost nobody understands the wrappers completely
- Segher warns against them: "So why do you think this should work?
You're telling it to link PIC to non-PIC. Did you read the manual? It's
just not allowed. It cannot ever work."

Kill the SHARED wrappers and use a real ABI wrapper.
The wrapper code is autogenerated on demand.
Any function compiled into stage0 is now shared by default, yet the size
and code generation of stage0/1/2 code are unchanged. Initram code size
does decrease quite a bit and the difficulty of creating shared
functions is now zero.

The patch includes extensive documentation about the inner workings of
the new wrappers and the reasons why they look like this.

Build and boot tested on qemu.
Build tested on all targets.

Signed-off-by: Carl-Daniel Hailfinger

Index: corebootv3-PIC_wrapper/include/shared.h
===================================================================
--- corebootv3-PIC_wrapper/include/shared.h	(revision 773)
+++ corebootv3-PIC_wrapper/include/shared.h	(working copy)
@@ -22,35 +22,9 @@
 #ifndef SHARED_H
 #define SHARED_H

-#ifdef _SHARED
-/* _SHARED mode enforces some functions to be called with an
- * absolute address, even in PIE mode. This is required since
- * the relative distance between XIP code and stage 0 is not known
- */
 #define FUNC(func, ret, attr, args...)   \
-	ret stage0_##func(args) attr
-
-/*
- * The introduction of the _SHARED macros had one flaw: If multiple files
- * had _SHARED defined during compilation, each of them would contain an
- * assignment of stage0_printk to *printk. During linking, this caused
- * errors as multiple definitions of printk existed.
- * Make sure _SHARED alone gives you only the printk prototype, and iff
- * _MAINOBJECT is defined as well, include the assignment.
- */
-#ifdef _MAINOBJECT
-#define EXTERN(func, ret, attr, args...) \
-	ret (*func)(args) attr= stage0_##func
-#else
-#define EXTERN(func, ret, attr, args...) \
-	extern ret (*func)(args) attr
-#endif
-
-#else
-#define FUNC(func, ret, attr, args...)   \
    ret func(args) attr
 #define EXTERN(func, ret, attr, args...)
-#endif

/**
  * Use the SHARED macro to create a universally usable function 
Index: corebootv3-PIC_wrapper/arch/x86/Makefile
===================================================================
--- corebootv3-PIC_wrapper/arch/x86/Makefile	(revision 773)
+++ corebootv3-PIC_wrapper/arch/x86/Makefile	(working copy)
@@ -269,10 +269,17 @@
 $(obj)/coreboot.initram $(obj)/coreboot.initram.map: $(obj)/stage0.init $(obj)/stage0-prefixed.o $(INITRAM_SRC)
    $(Q)printf "  CC      $(subst $(shell pwd)/,,$(@)) (XIP)\n"
    $(Q)$(CC) $(INITCFLAGS) -D_SHARED -fPIE -c -combine $(INITRAM_SRC) -o $(obj)/coreboot.initram_partiallylinked.o
+	$(Q)printf "  WRAP    $(subst $(shell pwd)/,,$(@)) (PIC->non-PIC)\n"
+	$(Q)$(NM) --undefined-only $(obj)/coreboot.initram_partiallylinked.o |\
+		grep -v _GLOBAL_OFFSET_TABLE_ | grep " U " | sed "s/^ *U //" |\
+		$(src)/util/picwrapper/picwrapper > $(obj)/initram_picwrapper.s
+	$(Q)printf "  AS      initram_picwrapper.s\n"
+	$(Q)$(AS) $(obj)/initram_picwrapper.s -o $(obj)/initram_picwrapper.o
    $(Q)# initram links against stage0
    $(Q)printf "  LD      $(subst $(shell pwd)/,,$(@))\n"
    $(Q)$(LD) -Ttext 0 --entry main -N -R $(obj)/stage0-prefixed.o \
-		$(obj)/coreboot.initram_partiallylinked.o -o $(obj)/coreboot.initram
+		$(obj)/coreboot.initram_partiallylinked.o \
+		$(obj)/initram_picwrapper.o -o $(obj)/coreboot.initram
    $(Q)printf "  NM      $(subst $(shell pwd)/,,$(@))\n"
    $(Q)$(NM) $(obj)/coreboot.initram | sort -u > $(obj)/coreboot.initram.map

Index: corebootv3-PIC_wrapper/util/picwrapper/picwrapper
===================================================================
--- corebootv3-PIC_wrapper/util/picwrapper/picwrapper	(revision 0)
+++ corebootv3-PIC_wrapper/util/picwrapper/picwrapper	(revision 0)
@@ -0,0 +1,60 @@
+#!/bin/bash
+#
+# This file is part of the coreboot project.
+#
+# Copyright (C) 2008 Carl-Daniel Hailfinger
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; version 2 of the License.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program; if not, write to the Free Software
+# Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA  02110-1301 USA
+#
+#
+# This program creates wrappers to be able to call non-PIC code from PIC code
+# for the x86 instruction architecture. The wrappers are needed because of the
+# ABI mismatch between PIC and non-PIC.
+# Other architectures may need similar wrappers.
+
+echo -e "\
+# These wrappers must fulfill the following conditions:
+# - They must not clobber the stuff already on the stack
+# - They must not clobber any register
+# - The jump/call must be absolute
+# - Neither caller nor callee shall have to be modified
+# - The parameters on the stack shall not be touched
+# - Referencing fixed memory locations is a no-go for PIC
+#
+# That rules out the following instructions:
+# - All relative jumps
+# - All jumps with address given in register/fixed memory
+# - All call instructions
+#
+# The only jump instructions left are far jumps, but hardcoding the code
+# segment in advance is ugly/bad/unworkable.
+#
+# The trick is to push the destination address on the stack with an immediate
+# instruction (no register/memory references), then return to that address.
+# This emulates the nonexisting "Jump near, absolute, addrees in operand"
+# instruction in the x86 instruction set.
+#
+
+	.text
+
+"
+while read a; do
+echo -e "\
+	.globl $a
+	.type $a, @function
+$a:
+	pushl $stage0_$a
+	ret
+"
+done

Property changes on: corebootv3-PIC_wrapper/util/picwrapper/picwrapper
___________________________________________________________________
Name: svn:executable
   + *

[PATCH] v3: fix calls from initram to stage1 the right way

[ron minnich]

Wow. we've need this fix for a long time.

Acked-by: Ronald G. Minnich

[PATCH] v3: fix calls from initram to stage1 the right way

[Carl-Daniel Hailfinger]

On 16.08.2008 01:31, ron minnich wrote:
> Wow. we've need this fix for a long time.
>
> Acked-by: Ronald G. Minnich

Thanks, committed in r775.

Regards,
Carl-Daniel