Re: [Xen-devel] [PATCH] arm: restore ELR_hyp and SPSR_hyp on return from hypervisor to hypervisor.

[Ian Campbell <Ian.Campbell@xxxxxxxxxx>]

On Mon, 2012-02-20 at 15:22 +0000, Ian Campbell wrote:
> This is necessary to handle nested traps to the hypervisor more than one deep.

A sort of corollary to this patch is the following.

The situation with the LR register when in hyp mode is a bit odd, since
this is normally a banked register but for hyp mode it actually accesses
LR_usr instead.

However, although I think the following is correct I'm not sure if it is
useful to combine LR_usr and LR like this -- in particular I fear it
might be more confusing than helpful. Making the return-to-user case a
proper superset of return-to-hypervisor (as is the case on the save
path) is nice though.

What do people think?

8<---------------------------------------------------------------

>From 3132c2976ae4c83d6d9b94ad80ee04c4880f13da Mon Sep 17 00:00:00 2001
From: Ian Campbell <ian.campbell@xxxxxxxxxx>
Date: Mon, 20 Feb 2012 15:07:09 +0000
Subject: [PATCH] arm: lr register in hyp mode is really LR_usr.

Save and restore it in the same way for both hypervisor and user stack frames
rather than saving both individually in the user stack frame.

Signed-off-by: Ian Campbell <ian.campbell@xxxxxxxxxx>
---
 xen/arch/arm/entry.S          |   14 ++------------
 xen/include/public/arch-arm.h |   15 ++++++++++++---
 2 files changed, 14 insertions(+), 15 deletions(-)

diff --git a/xen/arch/arm/entry.S b/xen/arch/arm/entry.S
index 36f1119..0e85c3c 100644
--- a/xen/arch/arm/entry.S
+++ b/xen/arch/arm/entry.S
@@ -29,8 +29,6 @@
        blne save_guest_regs
 
 save_guest_regs:
-       ldr r11, [sp, #UREGS_lr]
-       str r11, [sp, #UREGS_LR_usr]
        ldr r11, =0xffffffff  /* Clobber SP which is only valid for hypervisor 
frames. */
        str r11, [sp, #UREGS_sp]
        SAVE_ONE_BANKED(SP_usr)
@@ -78,15 +76,11 @@ ENTRY(return_from_trap)
        and r11, #PSR_MODE_MASK
        cmp r11, #PSR_MODE_HYP
        beq return_to_hypervisor
-
+       /* Fall thru */
 ENTRY(return_to_guest)
        mov r11, sp
        bic sp, #7 /* Align the stack pointer */
        bl leave_hypervisor_tail
-       ldr r11, [sp, #UREGS_pc]
-       msr ELR_hyp, r11
-       ldr r11, [sp, #UREGS_cpsr]
-       msr SPSR_hyp, r11
        RESTORE_ONE_BANKED(SP_usr)
        RESTORE_BANKED(svc)
        RESTORE_BANKED(abt)
@@ -95,11 +89,7 @@ ENTRY(return_to_guest)
        RESTORE_BANKED(fiq)
        RESTORE_ONE_BANKED(R8_fiq); RESTORE_ONE_BANKED(R9_fiq); 
RESTORE_ONE_BANKED(R10_fiq)
        RESTORE_ONE_BANKED(R11_fiq); RESTORE_ONE_BANKED(R12_fiq);
-       ldr lr, [sp, #UREGS_LR_usr]
-       pop {r0-r12}
-       add sp, #(UREGS_R8_fiq - UREGS_sp); /* SP, LR, SPSR, PC */
-       eret
-
+       /* Fall thru */
 ENTRY(return_to_hypervisor)
        ldr lr, [sp, #UREGS_lr]
        ldr r11, [sp, #UREGS_pc]
diff --git a/xen/include/public/arch-arm.h b/xen/include/public/arch-arm.h
index e3d5c08..edb78b4 100644
--- a/xen/include/public/arch-arm.h
+++ b/xen/include/public/arch-arm.h
@@ -63,7 +63,12 @@ struct cpu_user_regs
     uint32_t r12;
 
     uint32_t sp; /* r13 - SP: Valid for Hyp. frames only, o/w banked (see 
below) */
-    uint32_t lr; /* r14 - LR: Valid for Hyp. Same physical register as lr_usr. 
*/
+
+    /* r14 - LR: is the same physical register as LR_usr */
+    union {
+        uint32_t lr; /* r14 - LR: Valid for Hyp. Same physical register as 
lr_usr. */
+        uint32_t lr_usr;
+    };
 
     uint32_t pc; /* Return IP */
     uint32_t cpsr; /* Return mode */
@@ -73,10 +78,14 @@ struct cpu_user_regs
 
     uint32_t r8_fiq, r9_fiq, r10_fiq, r11_fiq, r12_fiq;
 
-    uint32_t sp_usr, sp_svc, sp_abt, sp_und, sp_irq, sp_fiq;
-    uint32_t lr_usr, lr_svc, lr_abt, lr_und, lr_irq, lr_fiq;
+    uint32_t sp_usr; /* LR_usr is the same register as LR, see above */
+
+    uint32_t sp_svc, sp_abt, sp_und, sp_irq, sp_fiq;
+    uint32_t lr_svc, lr_abt, lr_und, lr_irq, lr_fiq;
 
     uint32_t spsr_svc, spsr_abt, spsr_und, spsr_irq, spsr_fiq;
+
+    uint32_t pad1; /* Doubleword-align the user half of the frame */
 };
 typedef struct cpu_user_regs cpu_user_regs_t;
 DEFINE_XEN_GUEST_HANDLE(cpu_user_regs_t);
-- 
1.7.2.5




_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xensource.com/xen-devel