[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Minios-devel] [UNIKRAFT PATCHv3 15/25] plat/kvm: Force align the stack pointer for Arm64 EL1 exceptions



Hello Wei Chen,

Please find the comments inline:

Thanks & Regards
Sharan

On 12/13/18 10:15 AM, Wei Chen wrote:
If we enable the SCTLR_ELx.SA, Arm64 stack pointer must be aligned to
16-byte before being used as base address. But some valid EL1 exceptions
don't guarantee that SP_EL1 was aligned when entering the exceptions.

Signed-off-by: Wei Chen <wei.chen@xxxxxxx>
---
  plat/kvm/arm/exceptions.S | 65 ++++++++++++++++++++++++++++++++-------
  1 file changed, 54 insertions(+), 11 deletions(-)

diff --git a/plat/kvm/arm/exceptions.S b/plat/kvm/arm/exceptions.S
index e4a5c74..3fafb40 100644
--- a/plat/kvm/arm/exceptions.S
+++ b/plat/kvm/arm/exceptions.S
@@ -29,9 +29,51 @@
  #include <uk/arch/lcpu.h>
  #include <uk/asm.h>
+.macro EXCHANGE_SP_WITH_X0
+       add sp, sp, x0  // new_sp = sp + x0
+       sub x0, sp, x0  // new_x0 = new_sp - x0 = sp + x0 - x0 = sp
+       sub sp, sp, x0  // new_sp = new_sp - new_x0 = sp + x0 - sp = x0
+.endm
+
+.macro ALIGN_STACK
+       // First, exchange the SP with x0
+       EXCHANGE_SP_WITH_X0
+
+       // Check whether the stack is alignment
+       tst x0, #0xf
+       // If yes, save and go out. If not, align the stack
+       b.eq 0f
+
+       // Start to align the stack.
+
+       // We will use the x1 as temporary, save x1 to stack temporary
+       str x1, [x0]
+
+       // Align down sp to 16-byte, save old sp to aligned_sp[__SP_OFFSET]
+       bic x1, x0, #0xf
+       str x0, [x1, #__SP_OFFSET]
+
+       // Restore x1 before x0 is overridden
+       ldr x1, [x0]
+
+       // Save aligned_sp to x0
+       bic x0, x0, #0xf
+       b 1f
+0:
+       str x0, [x0, #__SP_OFFSET]
+1:
+       // Change back the SP from x0
+       EXCHANGE_SP_WITH_X0
+.endm
+
  .macro ENTER_TRAP, el
        sub sp, sp, #__TRAP_STACK_SIZE
+.if \el != 0
+       /* Force align the stack, and save SP to __SP_OFFSET */
+       ALIGN_STACK
+.endif
+
        /* Save general purpose registers */
        stp x0, x1, [sp, #16 * 0]
        stp x2, x3, [sp, #16 * 1]
@@ -58,25 +100,17 @@
        mrs x23, esr_el1
        stp x22, x23, [sp, #16 * 16]
- /* Save stack pointer for lower level exception */
  .if \el == 0
+       /* Save stack pointer for lower level exception */
        mrs x18, sp_el0
-.else
-       add x18, sp, #__TRAP_STACK_SIZE
+       str x18, [sp, #__SP_EL0_OFFSET]
  .endif
-       str x18, [sp, #16 * 17]
+
  .endm
.macro LEAVE_TRAP, el
        /* Mask IRQ to make sure restore would not be interrupted by IRQ */
        msr daifset, #2
-
-       /* Restore stack pointer for lower level exception */
-       ldr x18, [sp, #16 * 17]
-.if \el == 0
-       msr sp_el0, x18
-.endif
-
        /* Restore pstate and exception status register */
        ldp x22, x23, [sp, #16 * 16]
        msr spsr_el1, x22
@@ -103,6 +137,15 @@
        ldp x2, x3, [sp, #16 * 1]
        ldp x0, x1, [sp, #16 * 0]
+.if \el == 0
+       /* Restore stack pointer for exception from EL0 */
+       ldr x18, [sp, #__SP_EL0_OFFSET]
+       msr sp_el0, x18
+.else
+       /* Restore stack pointer for exception from EL1 */
+       ldr x18, [sp, #__SP_OFFSET]
+       mov sp, x18
+.endif
We are restoring the x18 register and overwriting it sp_el0 or sp.
Is this the expected behavior? Probably we can restore the x18, x19 after this operation.

        add sp, sp, #__TRAP_STACK_SIZE
eret


_______________________________________________
Minios-devel mailing list
Minios-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/minios-devel

 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.