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 15c3ba43d7261e9955a4a01cec0cc1d303f41fd4..a85a6a56830c7ecb84070cb405070c9411bfef6e 100644 (file)
--- 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 @@ func console_register
        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 @@ endfunc console_register
         * -----------------------------------------------
         */
 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 */
@@ -95,6 +111,37 @@ unregister_not_found:
        ret
 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.

diff --git a/include/drivers/console.h b/include/drivers/console.h
index f8ec83d2fa15e500de45ca579ad381c793c0cf05..0855170eb928708c1f0b3e0eee00d553e5d894d3 100644 (file)
--- a/include/drivers/console.h
+++ b/include/drivers/console.h
@@ -50,7 +50,12 @@ typedef struct console {
  */
 /* 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). */