Multi-console: Deprecate the `finish_console_register` macro

The `finish_console_register` macro is used by the multi console
framework to register the `console_t` driver callbacks. It relied
on weak references to the `ldr` instruction to populate 0 to the
callback in case the driver has not defined the appropriate
function. Use of `ldr` instruction to load absolute address to a
reference makes the binary position dependant. These instructions
should be replaced with adrp/adr instruction for position independant
executable(PIE). But adrp/adr instructions don't work well with weak
references as described in GNU ld bugzilla issue 22589.

This patch defines a new version of `finish_console_register` macro
which can spcify which driver callbacks are valid and deprecates the
old one. If any of the argument is not specified, then the macro
populates 0 for that callback. Hence the functionality of the previous
deprecated macro is preserved. The USE_FINISH_CONSOLE_REG_2 define
is used to select the new variant of the macro and will be removed
once the deprecated variant is removed.

All the upstream console drivers have been migrated to use the new
macro in this patch.

NOTE: Platforms be aware that the new variant of the
`finish_console_register` should be used and the old variant is
deprecated.

Change-Id: Ia6a67aaf2aa3ba93932992d683587bbd0ad25259
Signed-off-by: Soby Mathew <soby.mathew@arm.com>

diff --git a/drivers/arm/pl011/aarch32/pl011_console.S b/drivers/arm/pl011/aarch32/pl011_console.S
index 841ea446c7930b9bfb7493dc8a543d3b0d334857..46ff225870c57d799f6b6e043c06e5c99295a7c7 100644 (file)
--- a/drivers/arm/pl011/aarch32/pl011_console.S
+++ b/drivers/arm/pl011/aarch32/pl011_console.S
@@ -6,6 +6,7 @@
 #include <arch.h>
 #include <asm_macros.S>
 #include <assert_macros.S>
+#define USE_FINISH_CONSOLE_REG_2
 #include <console_macros.S>
 #include <pl011.h>
 
@@ -116,7 +117,7 @@ func console_pl011_register
 
        mov     r0, r4
        pop     {r4, lr}
-       finish_console_register pl011
+       finish_console_register pl011 putc=1, getc=1, flush=1
 
 register_fail:
        pop     {r4, pc}

diff --git a/drivers/arm/pl011/aarch64/pl011_console.S b/drivers/arm/pl011/aarch64/pl011_console.S
index d6a2d6b89950663015f9c63a21bd111f8b89eeef..3886f3b77c72909435c8b6560a88360dd77b6dcf 100644 (file)
--- a/drivers/arm/pl011/aarch64/pl011_console.S
+++ b/drivers/arm/pl011/aarch64/pl011_console.S
@@ -6,6 +6,7 @@
 #include <arch.h>
 #include <asm_macros.S>
 #include <assert_macros.S>
+#define USE_FINISH_CONSOLE_REG_2
 #include <console_macros.S>
 #include <pl011.h>
 
@@ -110,7 +111,7 @@ func console_pl011_register
 
        mov     x0, x6
        mov     x30, x7
-       finish_console_register pl011
+       finish_console_register pl011 putc=1, getc=1, flush=1
 
 register_fail:
        ret     x7

diff --git a/drivers/cadence/uart/aarch64/cdns_console.S b/drivers/cadence/uart/aarch64/cdns_console.S
index 71359a6d2006cfff21b428dd2045b801e1ff908f..8f46a62721578931eb91c081ef4d91a2dcc00b71 100644 (file)
--- a/drivers/cadence/uart/aarch64/cdns_console.S
+++ b/drivers/cadence/uart/aarch64/cdns_console.S
@@ -7,6 +7,8 @@
 #include <asm_macros.S>
 #include <assert_macros.S>
 #include <cadence/cdns_uart.h>
+#define USE_FINISH_CONSOLE_REG_2
+#include <console_macros.S>
 
        /*
         * "core" functions are low-level implementations that don't require
@@ -77,7 +79,7 @@ func console_cdns_register
 
        mov     x0, x6
        mov     x30, v7
-       finish_console_register cdns
+       finish_console_register cdns putc=1, getc=1, flush=1
 
 register_fail:
        ret     x7

diff --git a/drivers/console/aarch64/skeleton_console.S b/drivers/console/aarch64/skeleton_console.S
index 1b5d7393ccd09032e8bafb1fbe83e47904d85f14..3993eef99d1dcfd91ba5ef55e89dc1b1b46719d5 100644 (file)
--- a/drivers/console/aarch64/skeleton_console.S
+++ b/drivers/console/aarch64/skeleton_console.S
@@ -4,6 +4,7 @@
  * SPDX-License-Identifier: BSD-3-Clause
  */
 #include <asm_macros.S>
+#define USE_FINISH_CONSOLE_REG_2
 #include <console_macros.S>
 
        /*
@@ -60,8 +61,12 @@ func console_xxx_register
         * Keep console_t pointer in x0 for later.
         */
 
-       /* Macro to finish up registration and return (needs valid x0 + x30). */
-       finish_console_register xxx
+       /*
+        * Macro to finish up registration and return (needs valid x0 + x30).
+        * If any of the argument is unspecified, then the corresponding
+        * entry in console_t is set to 0.
+        */
+       finish_console_register xxx putc=1, getc=1, flush=1
 
        /* Jump here if hardware init fails or parameters are invalid. */
 register_fail:

diff --git a/drivers/coreboot/cbmem_console/aarch64/cbmem_console.S b/drivers/coreboot/cbmem_console/aarch64/cbmem_console.S
index 184853d9d91a528f24ea685f1d754e2a09a99117..89be349c0eb790aee41edf4e6dfb637fffa714a1 100644 (file)
--- a/drivers/coreboot/cbmem_console/aarch64/cbmem_console.S
+++ b/drivers/coreboot/cbmem_console/aarch64/cbmem_console.S
@@ -6,6 +6,7 @@
 
 #include <asm_macros.S>
 #include <cbmem_console.h>
+#define USE_FINISH_CONSOLE_REG_2
 #include <console_macros.S>
 
 /*
@@ -39,7 +40,7 @@ func console_cbmc_register
        ldr     w2, [x0]
        str     w2, [x1, #CONSOLE_T_CBMC_SIZE]
        mov     x0, x1
-       finish_console_register cbmc
+       finish_console_register cbmc putc=1, flush=1
 endfunc console_cbmc_register
 
        /* -----------------------------------------------

diff --git a/drivers/ti/uart/aarch64/16550_console.S b/drivers/ti/uart/aarch64/16550_console.S
index 0f9a9d5764d13382448a7f05076c88c29e8f3b60..785b640dd21c463f278a3f62aa70b6ce92b81287 100644 (file)
--- a/drivers/ti/uart/aarch64/16550_console.S
+++ b/drivers/ti/uart/aarch64/16550_console.S
@@ -7,6 +7,7 @@
 #include <arch.h>
 #include <asm_macros.S>
 #include <assert_macros.S>
+#define USE_FINISH_CONSOLE_REG_2
 #include <console_macros.S>
 #include <uart_16550.h>
 
@@ -112,7 +113,7 @@ func console_16550_register
 
        mov     x0, x6
        mov     x30, x7
-       finish_console_register 16550
+       finish_console_register 16550 putc=1, getc=1, flush=1
 
 register_fail:
        ret     x7

diff --git a/include/common/aarch32/console_macros.S b/include/common/aarch32/console_macros.S
index 480e3c298cc3f760a4e77dfe60b7147d43525e8c..7c30688fb4cc7593f7caed118d3b0ab065b1ae64 100644 (file)
--- a/include/common/aarch32/console_macros.S
+++ b/include/common/aarch32/console_macros.S
@@ -17,6 +17,14 @@
  * with a tail call that will include return to the caller.
  * REQUIRES console_t pointer in x0 and a valid return address in x30.
  */
+/*
+ * The USE_FINISH_CONSOLE_REG_2 guard is introduced to allow selection between
+ * the 2 variants of the finish_console_register macro and will be removed
+ * once the deprecated variant is removed.
+ */
+#ifndef USE_FINISH_CONSOLE_REG_2
+#if !ERROR_DEPRECATED
+       /* This version of the macro is deprecated. Use the new version */
        .macro  finish_console_register _driver
        /*
         * Add these weak definitions so we will automatically write a 0 if the
@@ -39,5 +47,38 @@
        str     r1, [r0, #CONSOLE_T_FLAGS]
        b       console_register
        .endm
+#endif /* ERROR_DEPRECATED */
+#else /* USE_FINISH_CONSOLE_REG_2 */
+       /* The new version of the macro not using weak references */
+       .macro  finish_console_register _driver, putc=0, getc=0, flush=0
+       /*
+        * If any of the callback is not specified or set as 0, then the
+        * corresponding callback entry in console_t is set to 0.
+        */
+       .ifne \putc
+         ldr   r1, =console_\_driver\()_putc
+       .else
+         mov   r1, #0
+       .endif
+       str     r1, [r0, #CONSOLE_T_PUTC]
+
+       .ifne \getc
+         ldr   r1, =console_\_driver\()_getc
+       .else
+         mov   r1, #0
+       .endif
+       str     r1, [r0, #CONSOLE_T_GETC]
 
+       .ifne \flush
+         ldr   r1, =console_\_driver\()_flush
+       .else
+         mov   r1, #0
+       .endif
+       str     r1, [r0, #CONSOLE_T_FLUSH]
+
+       mov     r1, #(CONSOLE_FLAG_BOOT | CONSOLE_FLAG_CRASH)
+       str     r1, [r0, #CONSOLE_T_FLAGS]
+       b       console_register
+       .endm
+#endif /* USE_FINISH_CONSOLE_REG_2 */
 #endif /* __CONSOLE_MACROS_S__ */

diff --git a/include/common/aarch64/console_macros.S b/include/common/aarch64/console_macros.S
index 0ebea2c1b5e540ef198138d03f6342e95b7e5072..b285ecce909723f801b3e1ce062024c15a8eccb5 100644 (file)
--- a/include/common/aarch64/console_macros.S
+++ b/include/common/aarch64/console_macros.S
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2017, ARM Limited and Contributors. All rights reserved.
+ * Copyright (c) 2017-2018, ARM Limited and Contributors. All rights reserved.
  *
  * SPDX-License-Identifier: BSD-3-Clause
  */
@@ -17,6 +17,14 @@
  * with a tail call that will include return to the caller.
  * REQUIRES console_t pointer in x0 and a valid return address in x30.
  */
+/*
+ * The USE_FINISH_CONSOLE_REG_2 guard is introduced to allow selection between
+ * the 2 variants of the finish_console_register macro and will be removed
+ * once the deprecated variant is removed.
+ */
+#ifndef USE_FINISH_CONSOLE_REG_2
+#if !ERROR_DEPRECATED
+       /* This version of the macro is deprecated. Use the new version */
        .macro  finish_console_register _driver
        /*
         * Add these weak definitions so we will automatically write a 0 if the
@@ -39,5 +47,41 @@
        str     x1, [x0, #CONSOLE_T_FLAGS]
        b       console_register
        .endm
+#endif /* ERROR_DEPRECATED */
+#else /* USE_FINISH_CONSOLE_REG_2 */
+       /* The new version of the macro not using weak references */
+       .macro  finish_console_register _driver, putc=0, getc=0, flush=0
+       /*
+        * If any of the callback is not specified or set as 0, then the
+        * corresponding callback entry in console_t is set to 0.
+        */
+       .ifne \putc
+         adrp  x1, console_\_driver\()_putc
+         add   x1, x1, :lo12:console_\_driver\()_putc
+         str   x1, [x0, #CONSOLE_T_PUTC]
+       .else
+         str   xzr, [x0, #CONSOLE_T_PUTC]
+       .endif
+
+       .ifne \getc
+         adrp  x1, console_\_driver\()_getc
+         add   x1, x1, :lo12:console_\_driver\()_getc
+         str   x1, [x0, #CONSOLE_T_GETC]
+       .else
+         str   xzr, [x0, #CONSOLE_T_GETC]
+       .endif
 
+       .ifne \flush
+         adrp  x1, console_\_driver\()_flush
+         add   x1, x1, :lo12:console_\_driver\()_flush
+         str   x1, [x0, #CONSOLE_T_FLUSH]
+       .else
+         str   xzr, [x0, #CONSOLE_T_FLUSH]
+       .endif
+
+       mov     x1, #(CONSOLE_FLAG_BOOT | CONSOLE_FLAG_CRASH)
+       str     x1, [x0, #CONSOLE_T_FLAGS]
+       b       console_register
+       .endm
+#endif /* USE_FINISH_CONSOLE_REG_2 */
 #endif /* __CONSOLE_MACROS_S__ */

diff --git a/plat/imx/common/lpuart_console.S b/plat/imx/common/lpuart_console.S
index ad71b89f2ace2daeab572137afdae16382429919..668fd624964ab0e1bccd0091b97f1bf026909bda 100644 (file)
--- a/plat/imx/common/lpuart_console.S
+++ b/plat/imx/common/lpuart_console.S
@@ -6,6 +6,7 @@
 
 #include <arch.h>
 #include <asm_macros.S>
+#define USE_FINISH_CONSOLE_REG_2
 #include <console_macros.S>
 #include <assert_macros.S>
 #include "imx8_lpuart.h"
@@ -26,7 +27,7 @@ func console_lpuart_register
 
        mov     x0, x6
        mov     x30, x7
-       finish_console_register lpuart
+       finish_console_register lpuart putc=1, getc=1
 
 register_fail:
        ret     x7

diff --git a/plat/layerscape/common/aarch64/ls_console.S b/plat/layerscape/common/aarch64/ls_console.S
index 5c87465ebcd59f249cd6ab326b4e111c8e2cfee7..ec4390a69de1dadcb0be172df2d82fa5c68a711a 100644 (file)
--- a/plat/layerscape/common/aarch64/ls_console.S
+++ b/plat/layerscape/common/aarch64/ls_console.S
@@ -6,6 +6,7 @@
 
 #include <arch.h>
 #include <asm_macros.S>
+#define USE_FINISH_CONSOLE_REG_2
 #include <console_macros.S>
 #include <assert_macros.S>
 #include "ls_16550.h"
@@ -106,7 +107,7 @@ func console_ls_16550_register
 
        mov     x0, x6
        mov     x30, x7
-       finish_console_register ls_16550
+       finish_console_register ls_16550 putc=1, getc=1, flush=1
 
 register_fail:
        ret     x7