Add new alignment parameter to func assembler macro
authorJulius Werner <jwerner@chromium.org>
Tue, 1 Aug 2017 22:16:36 +0000 (15:16 -0700)
committerJulius Werner <jwerner@chromium.org>
Tue, 15 Aug 2017 23:37:08 +0000 (16:37 -0700)
Assembler programmers are used to being able to define functions with a
specific aligment with a pattern like this:

    .align X
  myfunction:

However, this pattern is subtly broken when instead of a direct label
like 'myfunction:', you use the 'func myfunction' macro that's standard
in Trusted Firmware. Since the func macro declares a new section for the
function, the .align directive written above it actually applies to the
*previous* section in the assembly file, and the function it was
supposed to apply to is linked with default alignment.

An extreme case can be seen in Rockchip's plat_helpers.S which contains
this code:

  [...]
  endfunc plat_crash_console_putc

  .align 16
  func platform_cpu_warmboot
  [...]

This assembles into the following plat_helpers.o:

  Sections:
  Idx Name                             Size  [...]  Algn
   9 .text.plat_crash_console_putc 00010000  [...]  2**16
  10 .text.platform_cpu_warmboot   00000080  [...]  2**3

As can be seen, the *previous* function actually got the alignment
constraint, and it is also 64KB big even though it contains only two
instructions, because the .align directive at the end of its section
forces the assembler to insert a giant sled of NOPs. The function we
actually wanted to align has the default constraint. This code only
works at all because the linker just happens to put the two functions
right behind each other when linking the final image, and since the end
of plat_crash_console_putc is aligned the start of platform_cpu_warmboot
will also be. But it still wastes almost 64KB of image space
unnecessarily, and it will break under certain circumstances (e.g. if
the plat_crash_console_putc function becomes unused and its section gets
garbage-collected out).

There's no real way to fix this with the existing func macro. Code like

 func myfunc
 .align X

happens to do the right thing, but is still not really correct code
(because the function label is inserted before the .align directive, so
the assembler is technically allowed to insert padding at the beginning
of the function which would then get executed as instructions if the
function was called). Therefore, this patch adds a new parameter with a
default value to the func macro that allows overriding its alignment.

Also fix up all existing instances of this dangerous antipattern.

Change-Id: I5696a07e2fde896f21e0e83644c95b7b6ac79a10
Signed-off-by: Julius Werner <jwerner@chromium.org>
bl32/tsp/aarch64/tsp_entrypoint.S
include/common/asm_macros_common.S
plat/hisilicon/hikey/hisi_pwrc_sram.S
plat/nvidia/tegra/common/aarch64/tegra_helpers.S
plat/nvidia/tegra/soc/t186/plat_trampoline.S
plat/rockchip/common/aarch64/plat_helpers.S

index 2c3257852ceef98da2bc26ac63a987ac9141820e..489183c52a4ea2927102108b148c2be6ebf0b3dd 100644 (file)
        msr     spsr_el1, \reg2
        .endm
 
-       .section        .text, "ax"
-       .align 3
-
-func tsp_entrypoint
+func tsp_entrypoint _align=3
 
        /* ---------------------------------------------
         * Set the exception vector to something sane.
index b529246d8e53be25b9869aa8ab96d200d45066e2..dbc9e2d308533407400c1a354b5784544661e245 100644 (file)
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2013-2016, ARM Limited and Contributors. All rights reserved.
+ * Copyright (c) 2013-2017, ARM Limited and Contributors. All rights reserved.
  *
  * SPDX-License-Identifier: BSD-3-Clause
  */
         * code into a separate text section based on the function name
         * to enable elimination of unused code during linking. It also adds
         * basic debug information to enable call stack printing most of the
-        * time.
+        * time. The optional _align parameter can be used to force a
+        * non-standard alignment (indicated in powers of 2). Do *not* try to
+        * use a raw .align directive. Since func switches to a new section,
+        * this would not have the desired effect.
         */
-       .macro func _name
+       .macro func _name, _align=-1
        /*
         * Add Call Frame Information entry in the .debug_frame section for
         * debugger consumption. This enables callstack printing in debuggers.
@@ -33,6 +36,9 @@
         * .debug_frame
         */
        .cfi_startproc
+       .if (\_align) != -1
+               .align \_align
+       .endif
        \_name:
        .endm
 
index 1fb63eaf17dda85599bd304e33feb1ef9c11768f..f9e1de411476fcb01340e57a80ed1fdb4db89994 100644 (file)
@@ -15,8 +15,7 @@
        .global v7_asm
        .global v7_asm_end
 
-       .align  3
-func pm_asm_code
+func pm_asm_code _align=3
        mov     x0, 0
        msr     oslar_el1, x0
 
index e5e512685a788fa224c1cc7d4c301cf858e0999d..691b90af0de551a3ace1de841302cd83fd1ded06 100644 (file)
@@ -307,8 +307,7 @@ endfunc plat_reset_handler
         * Secure entrypoint function for CPU boot
         * ----------------------------------------
         */
-       .align 6
-func tegra_secure_entrypoint
+func tegra_secure_entrypoint _align=6
 
 #if ERRATA_TEGRA_INVALIDATE_BTB_AT_BOOT
 
index 4841aa20963d76a030c749891061971b2cc428f6..6a17c3328b59c6477bfc6f7eb5feccb2f01c343f 100644 (file)
 
 #define TEGRA186_SMMU_CTX_SIZE         0x420
 
-       .align 4
        .globl  tegra186_cpu_reset_handler
 
 /* CPU reset handler routine */
-func tegra186_cpu_reset_handler
+func tegra186_cpu_reset_handler _align=4
        /*
         * The TZRAM loses state during System Suspend. We use this
         * information to decide if the reset handler is running after a
index 1c8aefcb4841e2682442dc4c085b7b1d809ebc6c..abfb5a7951d95ce8a9cb5195a8aff104bd7899ef 100644 (file)
@@ -112,8 +112,7 @@ endfunc plat_crash_console_putc
         * cpus online or resume enterpoint
         * --------------------------------------------------------------------
         */
-       .align  16
-func platform_cpu_warmboot
+func platform_cpu_warmboot _align=16
        mrs     x0, MPIDR_EL1
        and     x19, x0, #MPIDR_CPU_MASK
        and     x20, x0, #MPIDR_CLUSTER_MASK