Re: [Xen-devel] [PATCH 01/13] xen: arm: tweak arm64 stack frame layout

[Stefano Stabellini <stefano.stabellini@xxxxxxxxxxxxx>]

On Wed, 17 Apr 2013, Ian Campbell wrote:
> Correct definition of UREGS_kernel_sizeof and use it.
> 
> Correct adjustment of stack on entry and exit.
> 
> Add 64-bit versions of the build time checks for stack pointer alignment
> correctness when pushing the stack frames.
> 
> Lastly, correct the padding in the stack frames to properly align the inner 
> and
> outer frames and also avoid an unnecessary 64bit padding field.
> 
> Signed-off-by: Ian Campbell <ian.campbell@xxxxxxxxxx>

Acked-by: Stefano Stabellini <stefano.stabellini@xxxxxxxxxxxxx>


>  xen/arch/arm/arm64/asm-offsets.c      |    2 +-
>  xen/arch/arm/arm64/entry.S            |    9 +++++----
>  xen/arch/arm/domain.c                 |    2 ++
>  xen/arch/arm/traps.c                  |    7 +++++++
>  xen/include/asm-arm/arm64/processor.h |    7 +++----
>  5 files changed, 18 insertions(+), 9 deletions(-)
> 
> diff --git a/xen/arch/arm/arm64/asm-offsets.c 
> b/xen/arch/arm/arm64/asm-offsets.c
> index 7949e3e..7544082 100644
> --- a/xen/arch/arm/arm64/asm-offsets.c
> +++ b/xen/arch/arm/arm64/asm-offsets.c
> @@ -39,7 +39,7 @@ void __dummy__(void)
>     OFFSET(UREGS_SP_el1, struct cpu_user_regs, sp_el1);
>     OFFSET(UREGS_ELR_el1, struct cpu_user_regs, elr_el1);
>  
> -   OFFSET(UREGS_kernel_sizeof, struct cpu_user_regs, cpsr);
> +   OFFSET(UREGS_kernel_sizeof, struct cpu_user_regs, spsr_el1);
>     DEFINE(UREGS_user_sizeof, sizeof(struct cpu_user_regs));
>     BLANK();
>  
> diff --git a/xen/arch/arm/arm64/entry.S b/xen/arch/arm/arm64/entry.S
> index 5656f45..b5af1e2 100644
> --- a/xen/arch/arm/arm64/entry.S
> +++ b/xen/arch/arm/arm64/entry.S
> @@ -35,7 +35,7 @@ lr      .req    x30             // link register
>          mrs     x22, SP_el0
>          str     x22, [x21]
>  
> -        add     x21, sp, #UREGS_ELR_el1
> +        add     x21, sp, #UREGS_SP_el1
>          mrs     x22, SP_el1
>          mrs     x23, ELR_el1
>          stp     x22, x23, [x21]
> @@ -60,7 +60,7 @@ lr      .req    x30             // link register
>   * Save state on entry to hypervisor
>   */
>          .macro  entry, hyp, compat
> -        sub     sp, sp, #(UREGS_SPSR_el1 - UREGS_SP)
> +        sub     sp, sp, #(UREGS_SPSR_el1 - UREGS_LR) /* CPSR, PC, SP, LR */
>          push    x28, x29
>          push    x26, x27
>          push    x24, x25
> @@ -79,7 +79,7 @@ lr      .req    x30             // link register
>  
>          .if \hyp == 1        /* Hypervisor mode */
>  
> -        add     x21, sp, #(UREGS_X0 - UREGS_SP)
> +        add     x21, sp, #UREGS_kernel_sizeof
>  
>          .else                /* Guest mode */
>  
> @@ -214,7 +214,8 @@ ENTRY(return_to_hypervisor)
>          pop     x26, x27
>          pop     x28, x29
>  
> -        ldr     lr, [sp], #(UREGS_SPSR_el1 - UREGS_SP)
> +        ldr     lr, [sp], #(UREGS_SPSR_el1 - UREGS_LR) /* CPSR, PC, SP, LR */
> +
>          eret
>  
>  /*
> diff --git a/xen/arch/arm/domain.c b/xen/arch/arm/domain.c
> index f369871..08bb132 100644
> --- a/xen/arch/arm/domain.c
> +++ b/xen/arch/arm/domain.c
> @@ -419,6 +419,8 @@ int vcpu_initialise(struct vcpu *v)
>  {
>      int rc = 0;
>  
> +    BUILD_BUG_ON( sizeof(struct cpu_info) > STACK_SIZE );
> +
>      v->arch.stack = alloc_xenheap_pages(STACK_ORDER, 
> MEMF_node(vcpu_to_node(v)));
>      if ( v->arch.stack == NULL )
>          return -ENOMEM;
> diff --git a/xen/arch/arm/traps.c b/xen/arch/arm/traps.c
> index a98a45e..6b19bc5 100644
> --- a/xen/arch/arm/traps.c
> +++ b/xen/arch/arm/traps.c
> @@ -43,9 +43,16 @@
>   * entry.S) and struct cpu_info (which lives at the bottom of a Xen
>   * stack) must be doubleword-aligned in size.  */
>  static inline void check_stack_alignment_constraints(void) {
> +#ifdef CONFIG_ARM_64
> +    BUILD_BUG_ON((sizeof (struct cpu_user_regs)) & 0xf);
> +    BUILD_BUG_ON((offsetof(struct cpu_user_regs, spsr_el1)) & 0xf);
> +    BUILD_BUG_ON((offsetof(struct cpu_user_regs, lr)) & 0xf);
> +    BUILD_BUG_ON((sizeof (struct cpu_info)) & 0xf);
> +#else
>      BUILD_BUG_ON((sizeof (struct cpu_user_regs)) & 0x7);
>      BUILD_BUG_ON((offsetof(struct cpu_user_regs, sp_usr)) & 0x7);
>      BUILD_BUG_ON((sizeof (struct cpu_info)) & 0x7);
> +#endif
>  }
>  
>  static int debug_stack_lines = 20;
> diff --git a/xen/include/asm-arm/arm64/processor.h 
> b/xen/include/asm-arm/arm64/processor.h
> index b4602fa..bf436c8 100644
> --- a/xen/include/asm-arm/arm64/processor.h
> +++ b/xen/include/asm-arm/arm64/processor.h
> @@ -51,6 +51,7 @@ struct cpu_user_regs
>      __DECL_REG(x27,          r11_fiq);
>      __DECL_REG(x28,          r12_fiq);
>      __DECL_REG(/* x29 */ fp, /* r13_fiq */ sp_fiq);
> +
>      __DECL_REG(/* x30 */ lr, /* r14_fiq */ lr_fiq);
>  
>      register_t sp; /* Valid for hypervisor frames */
> @@ -59,7 +60,7 @@ struct cpu_user_regs
>      __DECL_REG(pc,           pc32);             /* ELR_EL2 */
>      uint32_t cpsr;                              /* SPSR_EL2 */
>  
> -    uint64_t pad0;
> +    uint32_t pad0; /* Align end of kernel frame. */
>  
>      /* Outer guest frame only from here on... */
>  
> @@ -68,7 +69,7 @@ struct cpu_user_regs
>          uint32_t spsr_svc;       /* AArch32 */
>      };
>  
> -    uint32_t pad1; /* Align */
> +    uint32_t pad1; /* Doubleword-align the user half of the frame */
>  
>      /* AArch32 guests only */
>      uint32_t spsr_fiq, spsr_irq, spsr_und, spsr_abt;
> @@ -76,8 +77,6 @@ struct cpu_user_regs
>      /* AArch64 guests only */
>      uint64_t sp_el0;
>      uint64_t sp_el1, elr_el1;
> -
> -    uint64_t pad2; /* Doubleword-align the user half of the frame */
>  };
>  
>  #undef __DECL_REG
> -- 
> 1.7.2.5
> 

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