Preserve x19-x29 across world switch for exception handling

Previously exception handlers in BL3-1, X19-X29 were not saved
and restored on every SMC/trap into EL3. Instead these registers
were 'saved as needed' as a side effect of the A64 ABI used by the C
compiler.

That approach failed when world switching but was not visible
with the TSP/TSPD code because the TSP is 64-bit, did not
clobber these registers when running and did not support pre-emption
by normal world interrupts. These scenarios showed
that the values in these registers can be passed through a world
switch, which broke the normal and trusted world assumptions
about these registers being preserved.

The Ideal solution saves and restores these registers when a
world switch occurs - but that type of implementation is more complex.
So this patch always saves and restores these registers on entry and
exit of EL3.

Fixes ARM-software/tf-issues#141

Change-Id: I9a727167bbc594454e81cf78a97ca899dfb11c27

diff --git a/bl31/aarch64/bl31_entrypoint.S b/bl31/aarch64/bl31_entrypoint.S
index 39fa605ee0826ac1ac8ee7464e2ba963f73ea4d1..bc9b3e0037b5a63c31e4edc4b74912c1d39dea56 100644 (file)
--- a/bl31/aarch64/bl31_entrypoint.S
+++ b/bl31/aarch64/bl31_entrypoint.S
@@ -164,7 +164,6 @@ func bl31_entrypoint
         */
        bl      bl31_main
 
-       zero_callee_saved_regs
        b       el3_exit
 
 _panic:

diff --git a/bl31/aarch64/runtime_exceptions.S b/bl31/aarch64/runtime_exceptions.S
index 53cc176e05230fec6bd446f010e52fd1c18d8379..9c98ad6a5de1b555b495ca3deb993beb439b3327 100644 (file)
--- a/bl31/aarch64/runtime_exceptions.S
+++ b/bl31/aarch64/runtime_exceptions.S
@@ -39,6 +39,17 @@
        .globl  el3_exit
        .globl  get_exception_stack
 
+       .macro save_x18_to_x29_sp_el0
+       stp     x18, x19, [sp, #CTX_GPREGS_OFFSET + CTX_GPREG_X18]
+       stp     x20, x21, [sp, #CTX_GPREGS_OFFSET + CTX_GPREG_X20]
+       stp     x22, x23, [sp, #CTX_GPREGS_OFFSET + CTX_GPREG_X22]
+       stp     x24, x25, [sp, #CTX_GPREGS_OFFSET + CTX_GPREG_X24]
+       stp     x26, x27, [sp, #CTX_GPREGS_OFFSET + CTX_GPREG_X26]
+       stp     x28, x29, [sp, #CTX_GPREGS_OFFSET + CTX_GPREG_X28]
+       mrs     x18, sp_el0
+       str     x18, [sp, #CTX_GPREGS_OFFSET + CTX_GPREG_SP_EL0]
+       .endm
+
        .section        .vectors, "ax"; .align 11
 
        .align  7
@@ -250,6 +261,9 @@ smc_handler64:
        stp     x4, x5, [sp, #CTX_GPREGS_OFFSET + CTX_GPREG_X4]
        stp     x6, x7, [sp, #CTX_GPREGS_OFFSET + CTX_GPREG_X6]
 
+       /* Save rest of the gpregs and sp_el0*/
+       save_x18_to_x29_sp_el0
+
        mov     x5, xzr
        mov     x6, sp
 
@@ -264,10 +278,6 @@ smc_handler64:
        adr     x14, rt_svc_descs_indices
        ldrb    w15, [x14, x16]
 
-       /* Save x18 and SP_EL0 */
-       mrs     x17, sp_el0
-       stp     x18, x17, [x6, #CTX_GPREGS_OFFSET + CTX_GPREG_X18]
-
        /* -----------------------------------------------------
         * Restore the saved C runtime stack value which will
         * become the new SP_EL0 i.e. EL3 runtime stack. It was
@@ -357,8 +367,8 @@ el3_exit: ; .type el3_exit, %function
        msr     elr_el3, x17
 
        /* Restore saved general purpose registers and return */
-       bl      restore_scratch_registers
-       ldp     x30, xzr, [sp, #CTX_GPREGS_OFFSET + CTX_GPREG_LR]
+       bl      restore_gp_registers
+       ldr     x30, [sp, #CTX_GPREGS_OFFSET + CTX_GPREG_LR]
        eret
 
 smc_unknown:
@@ -369,10 +379,10 @@ smc_unknown:
         * content). Either way, we aren't leaking any secure information
         * through them
         */
-       bl      restore_scratch_registers_callee
+       bl      restore_gp_registers_callee
 
 smc_prohibited:
-       ldp     x30, xzr, [sp, #CTX_GPREGS_OFFSET + CTX_GPREG_LR]
+       ldr     x30, [sp, #CTX_GPREGS_OFFSET + CTX_GPREG_LR]
        mov     w0, #SMC_UNK
        eret
 
@@ -381,12 +391,16 @@ rt_svc_fw_critical_error:
 
        /* -----------------------------------------------------
         * The following functions are used to saved and restore
-        * all the caller saved registers as per the aapcs_64.
+        * all the general pupose registers. Ideally we would
+        * only save and restore the callee saved registers when
+        * a world switch occurs but that type of implementation
+        * is more complex. So currently we will always save and
+        * restore these registers on entry and exit of EL3.
         * These are not macros to ensure their invocation fits
         * within the 32 instructions per exception vector.
         * -----------------------------------------------------
         */
-func save_scratch_registers
+func save_gp_registers
        stp     x0, x1, [sp, #CTX_GPREGS_OFFSET + CTX_GPREG_X0]
        stp     x2, x3, [sp, #CTX_GPREGS_OFFSET + CTX_GPREG_X2]
        stp     x4, x5, [sp, #CTX_GPREGS_OFFSET + CTX_GPREG_X4]
@@ -396,16 +410,15 @@ func save_scratch_registers
        stp     x12, x13, [sp, #CTX_GPREGS_OFFSET + CTX_GPREG_X12]
        stp     x14, x15, [sp, #CTX_GPREGS_OFFSET + CTX_GPREG_X14]
        stp     x16, x17, [sp, #CTX_GPREGS_OFFSET + CTX_GPREG_X16]
-       mrs     x17, sp_el0
-       stp     x18, x17, [sp, #CTX_GPREGS_OFFSET + CTX_GPREG_X18]
+       save_x18_to_x29_sp_el0
        ret
 
-func restore_scratch_registers
+func restore_gp_registers
        ldp     x0, x1, [sp, #CTX_GPREGS_OFFSET + CTX_GPREG_X0]
        ldp     x2, x3, [sp, #CTX_GPREGS_OFFSET + CTX_GPREG_X2]
 
-restore_scratch_registers_callee:
-       ldp     x18, x17, [sp, #CTX_GPREGS_OFFSET + CTX_GPREG_X18]
+restore_gp_registers_callee:
+       ldr     x17, [sp, #CTX_GPREGS_OFFSET + CTX_GPREG_SP_EL0]
 
        ldp     x4, x5, [sp, #CTX_GPREGS_OFFSET + CTX_GPREG_X4]
        ldp     x6, x7, [sp, #CTX_GPREGS_OFFSET + CTX_GPREG_X6]
@@ -413,9 +426,14 @@ restore_scratch_registers_callee:
        ldp     x10, x11, [sp, #CTX_GPREGS_OFFSET + CTX_GPREG_X10]
        ldp     x12, x13, [sp, #CTX_GPREGS_OFFSET + CTX_GPREG_X12]
        ldp     x14, x15, [sp, #CTX_GPREGS_OFFSET + CTX_GPREG_X14]
-
        msr     sp_el0, x17
        ldp     x16, x17, [sp, #CTX_GPREGS_OFFSET + CTX_GPREG_X16]
+       ldp     x18, x19, [sp, #CTX_GPREGS_OFFSET + CTX_GPREG_X18]
+       ldp     x20, x21, [sp, #CTX_GPREGS_OFFSET + CTX_GPREG_X20]
+       ldp     x22, x23, [sp, #CTX_GPREGS_OFFSET + CTX_GPREG_X22]
+       ldp     x24, x25, [sp, #CTX_GPREGS_OFFSET + CTX_GPREG_X24]
+       ldp     x26, x27, [sp, #CTX_GPREGS_OFFSET + CTX_GPREG_X26]
+       ldp     x28, x29, [sp, #CTX_GPREGS_OFFSET + CTX_GPREG_X28]
        ret
 
        /* -----------------------------------------------------

diff --git a/include/bl31/cm_macros.S b/include/bl31/cm_macros.S
index d26495662a7a0c630cc77015d8e5342dcbae15a3..e82f3a32ed19decc0feae56e920ab0d118f8c034 100644 (file)
--- a/include/bl31/cm_macros.S
+++ b/include/bl31/cm_macros.S
@@ -27,31 +27,9 @@
  * ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE
  * POSSIBILITY OF SUCH DAMAGE.
  */
-
 #include <arch.h>
 #include <context.h>
 
-
-       /* ---------------------------------------------
-        * Zero out the callee saved register to prevent
-        * leakage of secure state into the normal world
-        * during the first ERET after a cold/warm boot.
-        * ---------------------------------------------
-        */
-       .macro  zero_callee_saved_regs
-       mov     x19, xzr
-       mov     x20, xzr
-       mov     x21, xzr
-       mov     x22, xzr
-       mov     x23, xzr
-       mov     x24, xzr
-       mov     x25, xzr
-       mov     x26, xzr
-       mov     x27, xzr
-       mov     x28, xzr
-       mov     x29, xzr
-       .endm
-
        .macro  switch_to_exception_stack reg1 reg2
        mov     \reg1 , sp
        ldr     \reg2, [\reg1, #CTX_EL3STATE_OFFSET + CTX_EXCEPTION_SP]
@@ -64,7 +42,7 @@
         * -----------------------------------------------------
         */
        .macro  handle_sync_exception
-       stp     x30, xzr, [sp, #CTX_GPREGS_OFFSET + CTX_GPREG_LR]
+       str     x30, [sp, #CTX_GPREGS_OFFSET + CTX_GPREG_LR]
        mrs     x30, esr_el3
        ubfx    x30, x30, #ESR_EC_SHIFT, #ESR_EC_LENGTH
 
@@ -83,7 +61,7 @@
         * not expect any such exceptions.
         * -----------------------------------------------------
         */
-       bl      save_scratch_registers
+       bl      save_gp_registers
        switch_to_exception_stack x0 x1
 
        /* Save the core_context pointer for handled faults */
@@ -92,8 +70,8 @@
        ldp     x0, xzr, [sp], #0x10
 
        mov     sp, x0
-       bl      restore_scratch_registers
-       ldp     x30, xzr, [sp, #CTX_GPREGS_OFFSET + CTX_GPREG_LR]
+       bl      restore_gp_registers
+       ldr     x30, [sp, #CTX_GPREGS_OFFSET + CTX_GPREG_LR]
        eret
        .endm
 
@@ -103,8 +81,8 @@
         * -----------------------------------------------------
         */
        .macro  handle_async_exception type
-       stp     x30, xzr, [sp, #CTX_GPREGS_OFFSET + CTX_GPREG_LR]
-       bl      save_scratch_registers
+       str     x30, [sp, #CTX_GPREGS_OFFSET + CTX_GPREG_LR]
+       bl      save_gp_registers
        switch_to_exception_stack x0 x1
 
        /* Save the core_context pointer */
@@ -114,7 +92,7 @@
        ldp     x0, xzr, [sp], #0x10
 
        mov     sp, x0
-       bl      restore_scratch_registers
-       ldp     x30, xzr, [sp, #CTX_GPREGS_OFFSET + CTX_GPREG_LR]
+       bl      restore_gp_registers
+       ldr     x30, [sp, #CTX_GPREGS_OFFSET + CTX_GPREG_LR]
        .endm
 

diff --git a/include/bl31/context.h b/include/bl31/context.h
index 989b2e6a4451599e6c8eef3bfea95d37f82694d4..549fa2127553a842258fa27a02dd5468f8dea96c 100644 (file)
--- a/include/bl31/context.h
+++ b/include/bl31/context.h
@@ -55,10 +55,20 @@
 #define CTX_GPREG_X16          0x80
 #define CTX_GPREG_X17          0x88
 #define CTX_GPREG_X18          0x90
-#define CTX_GPREG_SP_EL0       0x98
-#define CTX_GPREG_LR           0xa0
-/* Unused space to allow registers to be stored as pairs */
-#define CTX_GPREGS_END         0xb0
+#define CTX_GPREG_X19          0x98
+#define CTX_GPREG_X20          0xa0
+#define CTX_GPREG_X21          0xa8
+#define CTX_GPREG_X22          0xb0
+#define CTX_GPREG_X23          0xb8
+#define CTX_GPREG_X24          0xc0
+#define CTX_GPREG_X25          0xc8
+#define CTX_GPREG_X26          0xd0
+#define CTX_GPREG_X27          0xd8
+#define CTX_GPREG_X28          0xe0
+#define CTX_GPREG_X29          0xe8
+#define CTX_GPREG_LR           0xf0
+#define CTX_GPREG_SP_EL0       0xf8
+#define CTX_GPREGS_END         0x100
 
 /*******************************************************************************
  * Constants that allow assembler code to access members of and the 'el3_state'
@@ -188,10 +198,11 @@
 #define CTX_EL3STATE_ALL       (CTX_EL3STATE_END >> DWORD_SHIFT)
 
 /*
- * AArch64 general purpose register context structure. Only x0-x18, lr
- * are saved as the compiler is expected to preserve the remaining
+ * AArch64 general purpose register context structure. Usually x0-x18,
+ * lr are saved as the compiler is expected to preserve the remaining
  * callee saved registers if used by the C runtime and the assembler
- * does not touch the remaining.
+ * does not touch the remaining. But in case of world switch during
+ * exception handling, we need to save the callee registers too.
  */
 DEFINE_REG_STRUCT(gp_regs, CTX_GPREG_ALL);
 

diff --git a/services/std_svc/psci/psci_entry.S b/services/std_svc/psci/psci_entry.S
index e2c690db3a7e51b4a9fb664199f8ad8cfa26c9d9..50f74a6c5a6dc380e3e515c14c1ce79dfa1a4aab 100644 (file)
--- a/services/std_svc/psci/psci_entry.S
+++ b/services/std_svc/psci/psci_entry.S
@@ -104,7 +104,6 @@ psci_aff_common_finish_entry:
        mov     x0, x19
        bl      platform_set_stack
 
-       zero_callee_saved_regs
        b       el3_exit
 _panic:
        b       _panic