multi console: Assert that consoles aren't registered twice
In the multi console driver, allowing to register the same console more
than once may result in an infinte loop when putc is called.
If, for example, a boot message is trying to be printed, but the
consoles in the loop in the linked list are runtime consoles, putc will
iterate forever looking for a console that can print boot messages (or
a NULL pointer that will never come).
This loop in the linked list can occur after restoring the system from a
system suspend. The boot console is registered during the cold boot in
BL31, but the runtime console is registered even in the warm boot path.
Consoles are always added to the start of the linked list when they are
registered, so this it what should happen if they were actually
different structures:
console_list -> NULL
console_list -> BOOT -> NULL
console_list -> RUNTIME -> BOOT -> NULL
console_list -> RUNTIME -> RUNTIME -> BOOT -> NULL
In practice, the two runtime consoles are the same one, so they create
this loop:
console_list -> RUNTIME -. X -> BOOT -> NULL
^ |
`----'
This patch adds an assertion to detect this problem. The assertion will
fail whenever the same structure tries to be registered while being on
the list.
In order to assert this, console_is_registered() has been implemented.
It returns 1 if the specified console is registered, 0 if not.
Change-Id: I922485e743775ca9bd1af9cbd491ddd360526a6d
Signed-off-by: Antonio Nino Diaz <antonio.ninodiaz@arm.com>
diff --git a/drivers/console/aarch64/multi_console.S b/drivers/console/aarch64/multi_console.S
index 15c3ba4..a85a6a5 100644
--- a/drivers/console/aarch64/multi_console.S
+++ b/drivers/console/aarch64/multi_console.S
@@ -1,5 +1,5 @@
/*
- * Copyright (c) 2015-2017, ARM Limited and Contributors. All rights reserved.
+ * Copyright (c) 2015-2018, ARM Limited and Contributors. All rights reserved.
*
* SPDX-License-Identifier: BSD-3-Clause
*/
@@ -10,7 +10,8 @@
.globl console_register
.globl console_unregister
- .globl console_set_scope
+ .globl console_is_registered
+ .globl console_set_scope
.globl console_switch_state
.globl console_putc
.globl console_getc
@@ -38,13 +39,15 @@
* persistent memory (e.g. the data section).
* In : x0 - address of console_t structure
* Out: x0 - Always 1 (for easier tail calling)
- * Clobber list: x0, x1, x14
+ * Clobber list: x0, x1, x14, x15
* -----------------------------------------------
*/
func console_register
#if ENABLE_ASSERTIONS
+ /* Assert that x0 isn't a NULL pointer */
cmp x0, #0
ASM_ASSERT(ne)
+ /* Assert that the struct isn't in the stack */
adrp x1, __STACKS_START__
add x1, x1, :lo12:__STACKS_START__
cmp x0, x1
@@ -54,6 +57,14 @@
cmp x0, x1
ASM_ASSERT(hs)
not_on_stack:
+ /* Assert that this struct isn't in the list */
+ mov x1, x0 /* Preserve x0 and x30 */
+ mov x15, x30
+ bl console_is_registered
+ cmp x0, #0
+ ASM_ASSERT(eq)
+ mov x30, x15
+ mov x0, x1
#endif /* ENABLE_ASSERTIONS */
adrp x14, console_list
ldr x1, [x14, :lo12:console_list] /* X1 = first struct in list */
@@ -73,6 +84,11 @@
* -----------------------------------------------
*/
func console_unregister
+#if ENABLE_ASSERTIONS
+ /* Assert that x0 isn't a NULL pointer */
+ cmp x0, #0
+ ASM_ASSERT(ne)
+#endif /* ENABLE_ASSERTIONS */
adrp x14, console_list
add x14, x14, :lo12:console_list /* X14 = ptr to first struct */
ldr x1, [x14] /* X1 = first struct */
@@ -96,6 +112,37 @@
endfunc console_unregister
/* -----------------------------------------------
+ * int console_is_registered(console_t *console)
+ * Function to detect if a specific console is
+ * registered or not.
+ * In: x0 - address of console_t struct to remove
+ * Out: x0 - 1 if it is registered, 0 if not.
+ * Clobber list: x0, x14
+ * -----------------------------------------------
+ */
+func console_is_registered
+#if ENABLE_ASSERTIONS
+ /* Assert that x0 isn't a NULL pointer */
+ cmp x0, #0
+ ASM_ASSERT(ne)
+#endif /* ENABLE_ASSERTIONS */
+ adrp x14, console_list
+ ldr x14, [x14, :lo12:console_list] /* X14 = first console struct */
+check_registered_loop:
+ cbz x14, console_not_registered /* Check if end of list */
+ cmp x0, x14 /* Check if the pointers are different */
+ b.eq console_registered
+ ldr x14, [x14, #CONSOLE_T_NEXT] /* Get pointer to next struct */
+ b check_registered_loop
+console_not_registered:
+ mov x0, #0
+ ret
+console_registered:
+ mov x0, #1
+ ret
+endfunc console_is_registered
+
+ /* -----------------------------------------------
* void console_switch_state(unsigned int new_state)
* Function to switch the current console state.
* The console state determines which of the
diff --git a/include/drivers/console.h b/include/drivers/console.h
index f8ec83d..0855170 100644
--- a/include/drivers/console.h
+++ b/include/drivers/console.h
@@ -50,7 +50,12 @@
*/
/* Remove a single console_t instance from the console list. */
int console_unregister(console_t *console);
-/* Set scope mask of a console that determines in what states it is active. */
+/* Returns 1 if this console is already registered, 0 if not */
+int console_is_registered(console_t *console);
+/*
+ * Set scope mask of a console that determines in what states it is active.
+ * By default they are registered with (CONSOLE_FLAG_BOOT|CONSOLE_FLAG_CRASH).
+ */
void console_set_scope(console_t *console, unsigned int scope);
/* Switch to a new global console state (CONSOLE_FLAG_BOOT/RUNTIME/CRASH). */