[coreboot-gerrit] Patch set updated for coreboot: 1ebdc51 arm: Thumb ALL the things!

Isaac Christensen (isaac.christensen@se-eng.com) gerrit at coreboot.org
Mon Sep 29 21:06:28 CEST 2014 

Isaac Christensen (isaac.christensen at se-eng.com) just uploaded a new patch set to gerrit, which you can find at http://review.coreboot.org/6930

-gerrit

commit 1ebdc517093face074f428dd07346f2e38d06491
Author: Julius Werner <jwerner at chromium.org>
Date:   Mon Jan 13 13:24:30 2014 -0800

    arm: Thumb ALL the things!
    
    This patch switches every last part of Coreboot on ARM over to Thumb
    mode: libpayload, the internal libgcc, and assorted assembly files. In
    combination with the respective depthcharge patch, this will switch to
    Thumb mode right after the entry point of the bootblock and not switch
    back to ARM until the final assembly stub that jumps to the kernel.
    
    The required changes to make this work include some new headers and
    Makefile flags to handle assembly files (using the unified syntax and
    the same helper macros as Linux), modifying our custom-written libgcc
    code for 64-bit division to support Thumb (removing some stale old files
    that were never really used for clarity), and flipping the general
    CFLAGS to Thumb (some more cleanup there as well while I'm at it).
    
    Change-Id: I80c04281e3adbf74f9f477486a96b9fafeb455b3
    Signed-off-by: Julius Werner <jwerner at chromium.org>
    Reviewed-on: https://chromium-review.googlesource.com/182212
    Reviewed-by: Gabe Black <gabeblack at chromium.org>
    (cherry picked from commit 5f65c17cbfae165a95354146ae79e06c512c2c5a)
    Signed-off-by: Isaac Christensen <isaac.christensen at se-eng.com>
---
 payloads/libpayload/arch/arm/Makefile.inc  |  7 ++++++-
 payloads/libpayload/include/arm/arch/asm.h | 19 +++++++++++++------
 src/arch/arm/Makefile.inc                  |  8 ++++----
 src/arch/arm/armv7/Makefile.inc            | 10 ++++++----
 src/arch/arm/armv7/bootblock.S             | 23 ++++++++++-------------
 src/arch/arm/include/arch/asm.h            | 19 +++++++++++++------
 src/soc/nvidia/tegra124/maincpu.S          | 10 +++++-----
 7 files changed, 57 insertions(+), 39 deletions(-)

diff --git a/payloads/libpayload/arch/arm/Makefile.inc b/payloads/libpayload/arch/arm/Makefile.inc
index 0078ffe..2f8da15 100644
--- a/payloads/libpayload/arch/arm/Makefile.inc
+++ b/payloads/libpayload/arch/arm/Makefile.inc
@@ -27,7 +27,8 @@
 ## SUCH DAMAGE.
 ##
 
-CFLAGS += -mfloat-abi=hard -marm -mabi=aapcs-linux -march=armv7-a
+CFLAGS += -mthumb -march=armv7-a
+arm_asm_flags = -Wa,-mthumb -Wa,-mimplicit-it=always -Wa,-mno-warn-deprecated
 
 head.o-y += head.S
 libc-y += main.c sysinfo.c
@@ -37,3 +38,7 @@ libc-y += memcpy.S memset.S memmove.S
 libc-y += exception_asm.S exception.c
 libc-y += cache.c cpu.S
 libcbfs-$(CONFIG_LP_CBFS) += dummy_media.c
+
+# Add other classes here when you put assembly files into them!
+head.o-S-ccopts += $(arm_asm_flags)
+libc-S-ccopts += $(arm_asm_flags)
diff --git a/payloads/libpayload/include/arm/arch/asm.h b/payloads/libpayload/include/arm/arch/asm.h
index 2f88599..9e9e7ce 100644
--- a/payloads/libpayload/include/arm/arch/asm.h
+++ b/payloads/libpayload/include/arm/arch/asm.h
@@ -20,16 +20,17 @@
 #ifndef __ARM_ASM_H
 #define __ARM_ASM_H
 
-#if defined __arm__
-#  define ARM(x...)	x
-#  define THUMB(x...)
-#  define W(instr)	instr
-#elif defined __thumb__
+/* __arm__ is defined regardless of Thumb mode, so need to order this right */
+#if defined __thumb2__
 #  define ARM(x...)
 #  define THUMB(x...)	x
 #  define W(instr)	instr.w
+#elif defined __thumb__
+#  error You are not compiling Thumb2, this won't work!
 #else
-#  error Not in ARM or thumb mode!
+#  define ARM(x...)	x
+#  define THUMB(x...)
+#  define W(instr)	instr
 #endif
 
 #define ALIGN .align 0
@@ -46,4 +47,10 @@
 #define END(name) \
 	.size name, .-name
 
+/* Everything should go into the text section by default. */
+	.text
+
+/* Thumb code uses the (new) unified assembly syntax. */
+THUMB(	.syntax unified )
+
 #endif	/* __ARM_ASM_H */
diff --git a/src/arch/arm/Makefile.inc b/src/arch/arm/Makefile.inc
index 427e3a4..f10e371 100644
--- a/src/arch/arm/Makefile.inc
+++ b/src/arch/arm/Makefile.inc
@@ -2,7 +2,7 @@
 ##
 ## This file is part of the coreboot project.
 ##
-## Copyright (C) 2012 The ChromiumOS Authors
+## Copyright (C) 2012-2013 The ChromiumOS Authors
 ## Copyright (C) 2012 Alexandru Gagniuc <mr.nuke.me at gmail.com>
 ## Copyright (C) 2009-2010 coresystems GmbH
 ## Copyright (C) 2009 Ronald G. Minnich
@@ -64,7 +64,7 @@ $(objcbfs)/bootblock.debug: $(src)/arch/arm/bootblock.ld $(obj)/ldoptions $$(boo
 ifeq ($(CONFIG_COMPILER_LLVM_CLANG),y)
 	$(LD_bootblock) -m armelf_linux_eabi --gc-sections -static -o $@ -L$(obj) $< -T $(src)/arch/arm/bootblock.ld
 else
-	$(CC_bootblock) $(CFLAGS_bootblock) -nostartfiles -Wl,--gc-sections -static -o $@ -L$(obj) -T $(src)/arch/arm/bootblock.ld -Wl,--start-group $(bootblock-objs) $(LIBGCC_FILE_NAME_bootblock) -Wl,--end-group
+	$(CC_bootblock) $(CFLAGS_bootblock) -nostdlib -Wl,--gc-sections -static -o $@ -L$(obj) -T $(src)/arch/arm/bootblock.ld -Wl,--start-group $(bootblock-objs) $(LIBGCC_FILE_NAME_bootblock) -Wl,--end-group
 endif
 
 endif # CONFIG_ARCH_BOOTBLOCK_ARM
@@ -87,9 +87,9 @@ VBOOT_STUB_DEPS += $(obj)/arch/arm/eabi_compat.rmodules_arm.o
 $(objcbfs)/romstage.debug: $$(romstage-objs) $(src)/arch/arm/romstage.ld $(obj)/ldoptions
 	@printf "    LINK       $(subst $(obj)/,,$(@))\n"
 ifeq ($(CONFIG_COMPILER_LLVM_CLANG),y)
-	$(LD_romstage) -nostdlib -nostartfiles --gc-sections -static -o $@ -L$(obj) $(romstage-objs) -T $(src)/arch/arm/romstage.ld
+	$(LD_romstage) -nostdlib -nostdlib --gc-sections -static -o $@ -L$(obj) $(romstage-objs) -T $(src)/arch/arm/romstage.ld
 else
-	$(CC_romstage) $(CFLAGS_romstage) -nostartfiles -Wl,--gc-sections -static -o $@ -L$(obj) -T $(src)/arch/arm/romstage.ld -Wl,--start-group $(romstage-objs) $(LIBGCC_FILE_NAME_romstage) -Wl,--end-group
+	$(CC_romstage) $(CFLAGS_romstage) -nostdlib -Wl,--gc-sections -static -o $@ -L$(obj) -T $(src)/arch/arm/romstage.ld -Wl,--start-group $(romstage-objs) $(LIBGCC_FILE_NAME_romstage) -Wl,--end-group
 endif
 
 endif # CONFIG_ARCH_ROMSTAGE_ARM
diff --git a/src/arch/arm/armv7/Makefile.inc b/src/arch/arm/armv7/Makefile.inc
index c46fb39..b03879a 100644
--- a/src/arch/arm/armv7/Makefile.inc
+++ b/src/arch/arm/armv7/Makefile.inc
@@ -19,8 +19,10 @@
 ##
 ###############################################################################
 
-armv7_flags = -march=armv7-a -mthumb -mthumb-interwork \
+armv7_flags = -march=armv7-a -mthumb \
 	-I$(src)/arch/arm/include/armv7/ -D__COREBOOT_ARM_ARCH__=7
+armv7_asm_flags = $(armv7_flags) -Wa,-mthumb -Wa,-mimplicit-it=always \
+	-Wa,-mno-warn-deprecated
 
 ###############################################################################
 # bootblock
@@ -40,7 +42,7 @@ bootblock-$(CONFIG_BOOTBLOCK_CONSOLE) += exception_asm.S
 bootblock-y += mmu.c
 
 CFLAGS_bootblock += $(armv7_flags)
-CPPFLAGS_bootblock += $(armv7_flags)
+CPPFLAGS_bootblock += $(armv7_asm_flags)
 
 endif # CONFIG_ARCH_BOOTBLOCK_ARMV7
 
@@ -57,7 +59,7 @@ romstage-y += exception_asm.S
 romstage-y += mmu.c
 
 CFLAGS_romstage += $(armv7_flags)
-CPPFLAGS_romstage += $(armv7_flags)
+CPPFLAGS_romstage += $(armv7_asm_flags)
 
 endif # CONFIG_ARCH_ROMSTAGE_ARMV7
 
@@ -74,6 +76,6 @@ ramstage-y += exception_asm.S
 ramstage-y += mmu.c
 
 CFLAGS_ramstage += $(armv7_flags)
-CPPFLAGS_ramstage += $(armv7_flags)
+CPPFLAGS_ramstage += $(armv7_asm_flags)
 
 endif # CONFIG_ARCH_RAMSTAGE_ARMV7
diff --git a/src/arch/arm/armv7/bootblock.S b/src/arch/arm/armv7/bootblock.S
index a8d0973..4258caf 100644
--- a/src/arch/arm/armv7/bootblock.S
+++ b/src/arch/arm/armv7/bootblock.S
@@ -32,6 +32,7 @@
 #include <arch/asm.h>
 
 .section ".start", "a", %progbits
+.arm
 ENTRY(_start)
 	/*
 	 * Set the cpu to System mode with IRQ and FIQ disabled. Prefetch/Data
@@ -40,7 +41,11 @@ ENTRY(_start)
 	 * causes it.
 	 */
 	msr	cpsr_cxf, #0xdf
+	bl	_thumb_start
+ENDPROC(_start)
 
+.thumb
+ENTRY(_thumb_start)
 	/*
 	 * From Cortex-A Series Programmer's Guide:
 	 * Only CPU 0 performs initialization. Other CPUs go into WFI
@@ -72,25 +77,17 @@ call_bootblock:
 	ldr	sp, .Stack /* Set up stack pointer */
 	ldr	r0,=0x00000000
 	 /*
-	  * The current design of cpu_info places the
-	  * struct at the top of the stack. The number of
-	  * words pushed must be at least as large as that
-	  * struct.
+	  * The current design of cpu_info places the struct at the top of the
+	  * stack. Free enough space to accomodate for that, but make sure it's
+	  * 8-byte aligned for ABI compliance.
 	  */
-	push	{r0-r2}
-	bic	sp, sp, #7 /* 8-byte alignment for ABI compliance */
-	/*
-	 * Use "bl" instead of "b" even though we do not intend to return.
-	 * "bl" gets compiled to "blx" if we're transitioning from ARM to
-	 * Thumb. However, "b" will not and GCC may attempt to create a
-	 * wrapper which is currently broken.
-	 */
+	sub	sp, sp, #16
 	bl	main
 
 wait_for_interrupt:
 	wfi
 	mov	pc, lr			@ back to my caller
-ENDPROC(_start)
+ENDPROC(_thumb_start)
 
 /* we do it this way because it's a 32-bit constant and
  * in some cases too far away to be loaded as just an offset
diff --git a/src/arch/arm/include/arch/asm.h b/src/arch/arm/include/arch/asm.h
index 5f3e55f..a57ce4d 100644
--- a/src/arch/arm/include/arch/asm.h
+++ b/src/arch/arm/include/arch/asm.h
@@ -20,19 +20,20 @@
 #ifndef __ARM_ASM_H
 #define __ARM_ASM_H
 
-#if defined __arm__
-#  define ARM(x...)	x
-#  define THUMB(x...)
-#  define W(instr)	instr
-#elif defined __thumb__
+/* __arm__ is defined regardless of Thumb mode, so need to order this right */
+#if defined __thumb2__
 #  define ARM(x...)
 #  define THUMB(x...)	x
 #  define W(instr)	instr.w
 #  if __COREBOOT_ARM_ARCH__ < 7
 #    error thumb mode has not been tested with ARM < v7!
 #  endif
+#elif defined __thumb__
+#  error You are not compiling Thumb2, this won't work!
 #else
-#  error Not in ARM or thumb mode!
+#  define ARM(x...)	x
+#  define THUMB(x...)
+#  define W(instr)	instr
 #endif
 
 #define ALIGN .align 0
@@ -49,4 +50,10 @@
 #define END(name) \
 	.size name, .-name
 
+/* Everything should go into the text section by default. */
+	.text
+
+/* Thumb code uses the (new) unified assembly syntax. */
+THUMB(	.syntax unified )
+
 #endif	/* __ARM_ASM_H */
diff --git a/src/soc/nvidia/tegra124/maincpu.S b/src/soc/nvidia/tegra124/maincpu.S
index 7b217d8..91981ac 100644
--- a/src/soc/nvidia/tegra124/maincpu.S
+++ b/src/soc/nvidia/tegra124/maincpu.S
@@ -27,8 +27,9 @@
  * SUCH DAMAGE.
  */
 
+#include <arch/asm.h>
+
 	.align 2
-	.arm
 
 	.global maincpu_stack_pointer
 maincpu_stack_pointer:
@@ -38,10 +39,8 @@ maincpu_stack_pointer:
 maincpu_entry_point:
 	.word 0
 
-	.global maincpu_setup
-	.type maincpu_setup, function
-	maincpu_setup:
-
+.arm
+ENTRY(maincpu_setup)
 	/*
 	 * Set the cpu to System mode with IRQ and FIQ disabled. Prefetch/Data
 	 * aborts may happen early and crash before the abort handlers are
@@ -54,3 +53,4 @@ maincpu_entry_point:
 	eor	lr, lr
 	ldr	r0, maincpu_entry_point
 	bx	r0
+ENDPROC(maincpu_setup)

More information about the coreboot-gerrit mailing list