Merge pull request #1700 from jwerner-chromium/JW_crashfix

MULTI_CONSOLE_API fixes and cleanups

15 files changed, 149 insertions, 132 deletions

diff --git a/plat/allwinner/common/allwinner-common.mk b/plat/allwinner/common/allwinner-common.mk
index 2dc058f5..f20f5157 100644
--- a/plat/allwinner/common/allwinner-common.mk
+++ b/plat/allwinner/common/allwinner-common.mk
@@ -15,8 +15,7 @@ PLAT_INCLUDES		:=	-Iinclude/plat/arm/common		\
 
 include lib/libfdt/libfdt.mk
 
-PLAT_BL_COMMON_SOURCES	:=	drivers/console/${ARCH}/console.S	\
-				drivers/ti/uart/${ARCH}/16550_console.S	\
+PLAT_BL_COMMON_SOURCES	:=	drivers/ti/uart/${ARCH}/16550_console.S	\
 				${XLAT_TABLES_LIB_SRCS}			\
 				${AW_PLAT}/common/plat_helpers.S	\
 				${AW_PLAT}/common/sunxi_common.c
diff --git a/plat/common/aarch64/crash_console_helpers.S b/plat/common/aarch64/crash_console_helpers.S
index 5af8db25..8f8ca112 100644
--- a/plat/common/aarch64/crash_console_helpers.S
+++ b/plat/common/aarch64/crash_console_helpers.S
@@ -16,77 +16,167 @@
 	.globl	plat_crash_console_putc
 	.globl	plat_crash_console_flush
 
-#if MULTI_CONSOLE_API
+#if !MULTI_CONSOLE_API
+#error "This crash console implementation only works with the MULTI_CONSOLE_API!"
+#endif
 
-	/* -----------------------------------------------------
-	 * int plat_crash_console_init(void)
-	 * Use normal console by default. Switch it to crash
-	 * mode so serial consoles become active again.
-	 * NOTE: This default implementation will only work for
-	 * crashes that occur after a normal console (marked
-	 * valid for the crash state) has been registered with
-	 * the console framework. To debug crashes that occur
-	 * earlier, the platform has to override these functions
-	 * with an implementation that initializes a console
-	 * driver with hardcoded parameters. See
-	 * docs/porting-guide.rst for more information.
-	 * -----------------------------------------------------
-	 */
-func plat_crash_console_init
-#if defined(IMAGE_BL1)
 	/*
-	 * BL1 code can possibly crash so early that the data segment is not yet
-	 * accessible. Don't risk undefined behavior by trying to run the normal
-	 * console framework. Platforms that want to debug BL1 will need to
-	 * override this with custom functions that can run from registers only.
+	 * Spinlock to syncronize access to crash_console_triggered. We cannot
+	 * acquire spinlocks when the cache is disabled, so in some cases (like
+	 * late during CPU suspend) some risk remains.
 	 */
-	mov	x0, #0
-	ret
-#else	/* IMAGE_BL1 */
-	mov	x3, x30
-	mov	x0, #CONSOLE_FLAG_CRASH
-	bl	console_switch_state
-	mov	x0, #1
-	ret	x3
-#endif
-endfunc plat_crash_console_init
+.section .data.crash_console_spinlock
+	define_asm_spinlock crash_console_spinlock
 
-	/* -----------------------------------------------------
-	 * void plat_crash_console_putc(int character)
-	 * Output through the normal console by default.
-	 * -----------------------------------------------------
+	/*
+	 * Flag to make sure that only one CPU can write a crash dump even if
+	 * multiple crash at the same time. Interleaving crash dumps on the same
+	 * console would just make the output unreadable, so it's better to only
+	 * get a single but uncorrupted dump. This also means that we don't have
+	 * to duplicate the reg_stash below for each CPU.
 	 */
-func plat_crash_console_putc
-	b	console_putc
-endfunc plat_crash_console_putc
+.section .data.crash_console_triggered
+	crash_console_triggered: .byte 0
 
-	/* -----------------------------------------------------
-	 * void plat_crash_console_flush(void)
-	 * Flush normal console by default.
-	 * -----------------------------------------------------
+	/*
+	 * Space to stash away some register values while we're calling into
+	 * console drivers and don't have a real stack available. We need x14,
+	 * x15 and x30 for bookkeeping within the plat_crash_console functions
+	 * themselves, and some console drivers use x16 and x17 as additional
+	 * scratch space that is not preserved by the main crash reporting
+	 * framework. (Note that x16 and x17 should really never be expected to
+	 * retain their values across any function call, even between carefully
+	 * designed assembly functions, since the linker is always free to
+	 * insert a function call veneer that uses these registers as scratch
+	 * space at any time. The current crash reporting framework doesn't
+	 * really respect that, but since TF is usually linked as a single
+	 * contiguous binary of less than 128MB, it seems to work in practice.)
 	 */
-func plat_crash_console_flush
-	b	console_flush
-endfunc plat_crash_console_flush
-
-#else	/* MULTI_CONSOLE_API */
+.section .data.crash_console_reg_stash
+	.align 3
+	crash_console_reg_stash: .quad 0, 0, 0, 0, 0
 
-	/* -----------------------------------------------------
-	 * In the old API these are all no-op stubs that need to
-	 * be overridden by the platform to be useful.
-	 * -----------------------------------------------------
+	/* --------------------------------------------------------------------
+	 * int plat_crash_console_init(void)
+	 * Takes the crash console spinlock (if possible) and checks the trigger
+	 * flag to make sure we're the first CPU to dump. If not, return an
+	 * error (so crash dumping will fail but the CPU will still call
+	 * plat_panic_handler() which may do important platform-specific tasks
+	 * that may be needed on all crashing CPUs). In either case, the lock
+	 * will be released so other CPUs can make forward progress on this.
+	 * Clobbers: x0 - x4, x30
+	 * --------------------------------------------------------------------
 	 */
 func plat_crash_console_init
-	mov	x0, #0
+#if defined(IMAGE_BL31)
+	mov	x4, x30		/* x3 and x4 are not clobbered by spin_lock() */
+	mov	x3, #0		/* return value */
+
+	mrs	x1, sctlr_el3
+	tst	x1, #SCTLR_C_BIT
+	beq	skip_spinlock	/* can't synchronize when cache disabled */
+
+	adrp	x0, crash_console_spinlock
+	add	x0, x0, :lo12:crash_console_spinlock
+	bl	spin_lock
+
+skip_spinlock:
+	adrp	x1, crash_console_triggered
+	add	x1, x1, :lo12:crash_console_triggered
+	ldarb	w2, [x1]
+	cmp	w2, #0
+	bne	init_error
+
+	mov	x3, #1		/* set return value to success */
+	stlrb	w3, [x1]
+
+init_error:
+	bl	spin_unlock	/* harmless if we didn't acquire the lock */
+	mov	x0, x3
+	ret	x4
+#else	/* Only one CPU in BL1/BL2, no need to synchronize anything */
+	mov	x0, #1
 	ret
+#endif
 endfunc plat_crash_console_init
 
+	/* --------------------------------------------------------------------
+	 * int plat_crash_console_putc(char c)
+	 * Prints the character on all consoles registered with the console
+	 * framework that have CONSOLE_FLAG_CRASH set. Note that this is only
+	 * helpful for crashes that occur after the platform intialization code
+	 * has registered a console. Platforms using this implementation need to
+	 * ensure that all console drivers they use that have the CRASH flag set
+	 * support this (i.e. are written in assembly and comply to the register
+	 * clobber requirements of plat_crash_console_putc().
+	 * --------------------------------------------------------------------
+	 */
 func plat_crash_console_putc
+	adrp	x1, crash_console_reg_stash
+	add	x1, x1, :lo12:crash_console_reg_stash
+	stp	x14, x15, [x1]
+	stp	x16, x17, [x1, #16]
+	str	x30, [x1, #32]
+
+	mov	w14, w0				/* W14 = character to print */
+	adrp	x15, console_list
+	ldr	x15, [x15, :lo12:console_list]	/* X15 = first console struct */
+
+putc_loop:
+	cbz	x15, putc_done
+	ldr	w1, [x15, #CONSOLE_T_FLAGS]
+	tst	w1, #CONSOLE_FLAG_CRASH
+	b.eq	putc_continue
+	ldr	x2, [x15, #CONSOLE_T_PUTC]
+	cbz	x2, putc_continue
+	mov	x1, x15
+	blr	x2
+	mov	w0, w14
+putc_continue:
+	ldr	x15, [x15]			/* X15 = next struct */
+	b	putc_loop
+
+putc_done:
+	adrp	x1, crash_console_reg_stash
+	add	x1, x1, :lo12:crash_console_reg_stash
+	ldp	x14, x15, [x1]
+	ldp	x16, x17, [x1, #16]
+	ldr	x30, [x1, #32]
 	ret
 endfunc plat_crash_console_putc
 
+	/* --------------------------------------------------------------------
+	 * int plat_crash_console_flush(char c)
+	 * Flushes all consoles registered with the console framework that have
+	 * CONSOLE_FLAG_CRASH set. Same requirements as putc().
+	 * --------------------------------------------------------------------
+	 */
 func plat_crash_console_flush
+	adrp	x1, crash_console_reg_stash
+	add	x1, x1, :lo12:crash_console_reg_stash
+	stp	x30, x15, [x1]
+	stp	x16, x17, [x1, #16]
+
+	adrp	x15, console_list
+	ldr	x15, [x15, :lo12:console_list]	/* X15 = first console struct */
+
+flush_loop:
+	cbz	x15, flush_done
+	ldr	w1, [x15, #CONSOLE_T_FLAGS]
+	tst	w1, #CONSOLE_FLAG_CRASH
+	b.eq	flush_continue
+	ldr	x2, [x15, #CONSOLE_T_FLUSH]
+	cbz	x2, flush_continue
+	mov	x0, x15
+	blr	x2
+flush_continue:
+	ldr	x15, [x15]			/* X15 = next struct */
+	b	flush_loop
+
+flush_done:
+	adrp	x1, crash_console_reg_stash
+	add	x1, x1, :lo12:crash_console_reg_stash
+	ldp	x30, x15, [x1]
+	ldp	x16, x17, [x1, #16]
 	ret
 endfunc plat_crash_console_flush
-
-#endif
diff --git a/plat/common/aarch64/platform_helpers.S b/plat/common/aarch64/platform_helpers.S
index d3ffcaf1..89523194 100644
--- a/plat/common/aarch64/platform_helpers.S
+++ b/plat/common/aarch64/platform_helpers.S
@@ -40,65 +40,6 @@ func plat_report_exception
 endfunc plat_report_exception
 
 #if !ERROR_DEPRECATED
-#if MULTI_CONSOLE_API
-	/* -----------------------------------------------------
-	 * int plat_crash_console_init(void)
-	 * Use normal console by default. Switch it to crash
-	 * mode so serial consoles become active again.
-	 * NOTE: This default implementation will only work for
-	 * crashes that occur after a normal console (marked
-	 * valid for the crash state) has been registered with
-	 * the console framework. To debug crashes that occur
-	 * earlier, the platform has to override these functions
-	 * with an implementation that initializes a console
-	 * driver with hardcoded parameters. See
-	 * docs/porting-guide.rst for more information.
-	 * -----------------------------------------------------
-	 */
-func plat_crash_console_init
-#if defined(IMAGE_BL1)
-	/*
-	 * BL1 code can possibly crash so early that the data segment is not yet
-	 * accessible. Don't risk undefined behavior by trying to run the normal
-	 * console framework. Platforms that want to debug BL1 will need to
-	 * override this with custom functions that can run from registers only.
-	 */
-	mov	x0, #0
-	ret
-#else	/* IMAGE_BL1 */
-	mov	x3, x30
-	mov	x0, #CONSOLE_FLAG_CRASH
-	bl	console_switch_state
-	mov	x0, #1
-	ret	x3
-#endif
-endfunc plat_crash_console_init
-
-	/* -----------------------------------------------------
-	 * void plat_crash_console_putc(int character)
-	 * Output through the normal console by default.
-	 * -----------------------------------------------------
-	 */
-func plat_crash_console_putc
-	b	console_putc
-endfunc plat_crash_console_putc
-
-	/* -----------------------------------------------------
-	 * void plat_crash_console_flush(void)
-	 * Flush normal console by default.
-	 * -----------------------------------------------------
-	 */
-func plat_crash_console_flush
-	b	console_flush
-endfunc plat_crash_console_flush
-
-#else	/* MULTI_CONSOLE_API */
-
-	/* -----------------------------------------------------
-	 * In the old API these are all no-op stubs that need to
-	 * be overridden by the platform to be useful.
-	 * -----------------------------------------------------
-	 */
 func plat_crash_console_init
 	mov	x0, #0
 	ret
@@ -111,7 +52,6 @@ endfunc plat_crash_console_putc
 func plat_crash_console_flush
 	ret
 endfunc plat_crash_console_flush
-#endif
 #endif /* ERROR_DEPRECATED */
 
 	/* -----------------------------------------------------
diff --git a/plat/imx/imx8qm/platform.mk b/plat/imx/imx8qm/platform.mk
index ce84e2ba..dc45e901 100644
--- a/plat/imx/imx8qm/platform.mk
+++ b/plat/imx/imx8qm/platform.mk
@@ -26,7 +26,6 @@ BL31_SOURCES		+=	plat/imx/common/lpuart_console.S	\
 				lib/xlat_tables/xlat_tables_common.c		\
 				lib/cpus/aarch64/cortex_a53.S			\
 				lib/cpus/aarch64/cortex_a72.S			\
-				drivers/console/aarch64/console.S		\
 				drivers/arm/cci/cci.c				\
 				${IMX_GIC_SOURCES}				\
 
diff --git a/plat/imx/imx8qx/platform.mk b/plat/imx/imx8qx/platform.mk
index 02559b46..a831bf2f 100644
--- a/plat/imx/imx8qx/platform.mk
+++ b/plat/imx/imx8qx/platform.mk
@@ -25,7 +25,6 @@ BL31_SOURCES		+=	plat/imx/common/lpuart_console.S	\
 				lib/xlat_tables/xlat_tables_common.c	\
 				lib/xlat_tables/aarch64/xlat_tables.c	\
 				lib/cpus/aarch64/cortex_a35.S		\
-				drivers/console/aarch64/console.S	\
 				${IMX_GIC_SOURCES}			\
 
 include plat/imx/common/sci/sci_api.mk
diff --git a/plat/layerscape/board/ls1043/platform.mk b/plat/layerscape/board/ls1043/platform.mk
index 678205cd..795d9248 100644
--- a/plat/layerscape/board/ls1043/platform.mk
+++ b/plat/layerscape/board/ls1043/platform.mk
@@ -28,8 +28,7 @@ PLAT_INCLUDES			:=	-Iplat/layerscape/board/ls1043/include   \
 					-Iinclude/drivers/io
 
 
-PLAT_BL_COMMON_SOURCES		:=	drivers/console/aarch64/console.S	\
-					plat/layerscape/common/aarch64/ls_console.S
+PLAT_BL_COMMON_SOURCES		:=	plat/layerscape/common/aarch64/ls_console.S
 
 LS1043_CPU_LIBS			:=	lib/cpus/${ARCH}/aem_generic.S
 
diff --git a/plat/marvell/a3700/common/a3700_common.mk b/plat/marvell/a3700/common/a3700_common.mk
index 3983c707..e2ac97ff 100644
--- a/plat/marvell/a3700/common/a3700_common.mk
+++ b/plat/marvell/a3700/common/a3700_common.mk
@@ -96,7 +96,6 @@ PLAT_INCLUDES		:=	-I$(PLAT_FAMILY_BASE)/$(PLAT)		\
 				$(ATF_INCLUDES)
 
 PLAT_BL_COMMON_SOURCES	:=	$(PLAT_COMMON_BASE)/aarch64/a3700_common.c \
-				drivers/console/aarch64/console.S	   \
 				$(MARVELL_COMMON_BASE)/marvell_cci.c	   \
 				$(MARVELL_DRV_BASE)/uart/a3700_console.S
 
diff --git a/plat/marvell/a8k/common/a8k_common.mk b/plat/marvell/a8k/common/a8k_common.mk
index 88d93111..e350d6ac 100644
--- a/plat/marvell/a8k/common/a8k_common.mk
+++ b/plat/marvell/a8k/common/a8k_common.mk
@@ -55,7 +55,6 @@ PLAT_INCLUDES		:=	-I$(PLAT_FAMILY_BASE)/$(PLAT)		\
 				$(ATF_INCLUDES)
 
 PLAT_BL_COMMON_SOURCES	:=	$(PLAT_COMMON_BASE)/aarch64/a8k_common.c \
-				drivers/console/aarch64/console.S	 \
 				drivers/ti/uart/aarch64/16550_console.S
 
 BLE_PORTING_SOURCES	:=	$(PLAT_FAMILY_BASE)/$(PLAT)/board/dram_port.c \
diff --git a/plat/meson/gxbb/platform.mk b/plat/meson/gxbb/platform.mk
index e6f5ae48..68ff4007 100644
--- a/plat/meson/gxbb/platform.mk
+++ b/plat/meson/gxbb/platform.mk
@@ -14,8 +14,7 @@ GXBB_GIC_SOURCES	:=	drivers/arm/gic/common/gic_common.c	\
 				drivers/arm/gic/v2/gicv2_helpers.c	\
 				plat/common/plat_gicv2.c
 
-PLAT_BL_COMMON_SOURCES	:=	drivers/console/aarch64/multi_console.S	\
-				drivers/meson/console/aarch64/meson_console.S \
+PLAT_BL_COMMON_SOURCES	:=	drivers/meson/console/aarch64/meson_console.S \
 				plat/meson/gxbb/gxbb_common.c		\
 				plat/meson/gxbb/gxbb_topology.c		\
 				${XLAT_TABLES_LIB_SRCS}
diff --git a/plat/rockchip/rk3328/platform.mk b/plat/rockchip/rk3328/platform.mk
index 785f6403..2b2ac51c 100644
--- a/plat/rockchip/rk3328/platform.mk
+++ b/plat/rockchip/rk3328/platform.mk
@@ -34,7 +34,6 @@ PLAT_BL_COMMON_SOURCES	:=	lib/xlat_tables/aarch64/xlat_tables.c		\
 
 BL31_SOURCES		+=	${RK_GIC_SOURCES}				\
 				drivers/arm/cci/cci.c				\
-				drivers/console/aarch64/console.S		\
 				drivers/ti/uart/aarch64/16550_console.S		\
 				drivers/delay_timer/delay_timer.c		\
 				drivers/delay_timer/generic_delay_timer.c	\
diff --git a/plat/rockchip/rk3368/platform.mk b/plat/rockchip/rk3368/platform.mk
index a3e593e6..c0164c17 100644
--- a/plat/rockchip/rk3368/platform.mk
+++ b/plat/rockchip/rk3368/platform.mk
@@ -31,7 +31,6 @@ PLAT_BL_COMMON_SOURCES	:=	lib/xlat_tables/xlat_tables_common.c		\
 
 BL31_SOURCES		+=	${RK_GIC_SOURCES}				\
 				drivers/arm/cci/cci.c				\
-				drivers/console/aarch64/console.S		\
 				drivers/ti/uart/aarch64/16550_console.S		\
 				drivers/delay_timer/delay_timer.c		\
 				drivers/delay_timer/generic_delay_timer.c	\
diff --git a/plat/rockchip/rk3399/platform.mk b/plat/rockchip/rk3399/platform.mk
index eccf1cc8..b624717d 100644
--- a/plat/rockchip/rk3399/platform.mk
+++ b/plat/rockchip/rk3399/platform.mk
@@ -37,7 +37,6 @@ PLAT_BL_COMMON_SOURCES	:=	lib/xlat_tables/xlat_tables_common.c	\
 
 BL31_SOURCES	+=	${RK_GIC_SOURCES}				\
 			drivers/arm/cci/cci.c				\
-			drivers/console/aarch64/console.S		\
 			drivers/ti/uart/aarch64/16550_console.S		\
 			drivers/delay_timer/delay_timer.c		\
 			drivers/delay_timer/generic_delay_timer.c	\
diff --git a/plat/rpi3/platform.mk b/plat/rpi3/platform.mk
index db96de82..46c139ea 100644
--- a/plat/rpi3/platform.mk
+++ b/plat/rpi3/platform.mk
@@ -10,8 +10,7 @@ include lib/xlat_tables_v2/xlat_tables.mk
 PLAT_INCLUDES		:=	-Iinclude/common/tbbr			\
 				-Iplat/rpi3/include
 
-PLAT_BL_COMMON_SOURCES	:=	drivers/console/aarch64/console.S	\
-				drivers/ti/uart/aarch64/16550_console.S	\
+PLAT_BL_COMMON_SOURCES	:=	drivers/ti/uart/aarch64/16550_console.S	\
 				plat/rpi3/rpi3_common.c			\
 				${XLAT_TABLES_LIB_SRCS}
 
diff --git a/plat/st/stm32mp1/platform.mk b/plat/st/stm32mp1/platform.mk
index f4a0ca4a..545b140d 100644
--- a/plat/st/stm32mp1/platform.mk
+++ b/plat/st/stm32mp1/platform.mk
@@ -35,8 +35,7 @@ include lib/libfdt/libfdt.mk
 
 PLAT_BL_COMMON_SOURCES	:=	plat/st/stm32mp1/stm32mp1_common.c
 
-PLAT_BL_COMMON_SOURCES	+=	drivers/console/aarch32/console.S			\
-				drivers/st/uart/aarch32/stm32_console.S
+PLAT_BL_COMMON_SOURCES	+=	drivers/st/uart/aarch32/stm32_console.S
 
 ifneq (${ENABLE_STACK_PROTECTOR},0)
 PLAT_BL_COMMON_SOURCES	+=	plat/st/stm32mp1/stm32mp1_stack_protector.c
diff --git a/plat/ti/k3/common/plat_common.mk b/plat/ti/k3/common/plat_common.mk
index fbd79b4a..9b3e7d84 100644
--- a/plat/ti/k3/common/plat_common.mk
+++ b/plat/ti/k3/common/plat_common.mk
@@ -39,7 +39,6 @@ PLAT_INCLUDES		+=	\
 				-I${PLAT_PATH}/common/drivers/ti_sci	\
 
 K3_CONSOLE_SOURCES	+=	\
-				drivers/console/aarch64/console.S	\
 				drivers/ti/uart/aarch64/16550_console.S	\
 				${PLAT_PATH}/common/k3_console.c	\