Re: [PATCH v3 01/19] xen/arm: Save ESR_EL2 to avoid using mismatched value in syndrome check

[Stefano Stabellini <sstabellini@kernel.org>]

On Fri, 31 Mar 2017, Wei Chen wrote:
> Xen will do exception syndrome check while some types of exception
> take place in EL2. The syndrome check code read the ESR_EL2 register
> directly, but in some situation this register maybe overridden by
> nested exception.
> 
> For example, if we re-enable IRQ before reading ESR_EL2 which means
> Xen may enter in IRQ exception mode and return the processor with
> clobbered ESR_EL2 (See ARM ARM DDI 0487A.j D7.2.25)
> 
> In this case the guest exception syndrome has been overridden, we will
> check the syndrome for guest sync exception with an incorrect ESR_EL2
> value. So we want to save ESR_EL2 to cpu_user_regs as soon as the
> exception takes place in EL2 to avoid using an incorrect syndrome value.
> 
> In order to save ESR_EL2, we added a 32-bit member hsr to cpu_user_regs.
> But while saving registers in trap entry, we use stp to save ELR and
> CPSR at the same time through 64-bit general registers. If we keep this
> code, the hsr will be overridden by upper 32-bit of CPSR. So adjust the
> code to use str to save ELR in a separate instruction and use stp to
> save CPSR and HSR at the same time through 32-bit general registers.
> This change affects the registers restore in trap exit, we can't use the
> ldp to restore ELR and CPSR from stack at the same time. We have to use
> ldr to restore them separately.
> 
> Signed-off-by: Wei Chen <Wei.Chen@arm.com>

Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>


> ---
> Note:
> This patch is a bug fix, this bug affects the 4.8 and 4.7 source trees
> too.
> 
> v2->v3:
> 1. Add note to the commit message.
> 2. Read ESR_EL2 value from vCPU context instead of reading from register
>    directly in the places where use the ESR_EL2 value.
> ---
>  xen/arch/arm/arm32/asm-offsets.c      |  1 +
>  xen/arch/arm/arm32/entry.S            |  3 +++
>  xen/arch/arm/arm64/asm-offsets.c      |  1 +
>  xen/arch/arm/arm64/entry.S            | 13 +++++++++----
>  xen/arch/arm/arm64/traps.c            |  2 +-
>  xen/arch/arm/traps.c                  |  4 ++--
>  xen/include/asm-arm/arm32/processor.h |  2 +-
>  xen/include/asm-arm/arm64/processor.h |  3 +--
>  8 files changed, 19 insertions(+), 10 deletions(-)
> 
> diff --git a/xen/arch/arm/arm32/asm-offsets.c b/xen/arch/arm/arm32/asm-offsets.c
> index f8e6b53..5b543ab 100644
> --- a/xen/arch/arm/arm32/asm-offsets.c
> +++ b/xen/arch/arm/arm32/asm-offsets.c
> @@ -26,6 +26,7 @@ void __dummy__(void)
>     OFFSET(UREGS_lr, struct cpu_user_regs, lr);
>     OFFSET(UREGS_pc, struct cpu_user_regs, pc);
>     OFFSET(UREGS_cpsr, struct cpu_user_regs, cpsr);
> +   OFFSET(UREGS_hsr, struct cpu_user_regs, hsr);
>  
>     OFFSET(UREGS_LR_usr, struct cpu_user_regs, lr_usr);
>     OFFSET(UREGS_SP_usr, struct cpu_user_regs, sp_usr);
> diff --git a/xen/arch/arm/arm32/entry.S b/xen/arch/arm/arm32/entry.S
> index 2a6f4f0..2187226 100644
> --- a/xen/arch/arm/arm32/entry.S
> +++ b/xen/arch/arm/arm32/entry.S
> @@ -23,6 +23,9 @@
>          add r11, sp, #UREGS_kernel_sizeof+4;                            \
>          str r11, [sp, #UREGS_sp];                                       \
>                                                                          \
> +        mrc CP32(r11, HSR);             /* Save exception syndrome */   \
> +        str r11, [sp, #UREGS_hsr];                                      \
> +                                                                        \
>          mrs r11, SPSR_hyp;                                              \
>          str r11, [sp, #UREGS_cpsr];                                     \
>          and r11, #PSR_MODE_MASK;                                        \
> diff --git a/xen/arch/arm/arm64/asm-offsets.c b/xen/arch/arm/arm64/asm-offsets.c
> index 69ea92a..ce24e44 100644
> --- a/xen/arch/arm/arm64/asm-offsets.c
> +++ b/xen/arch/arm/arm64/asm-offsets.c
> @@ -27,6 +27,7 @@ void __dummy__(void)
>     OFFSET(UREGS_SP, struct cpu_user_regs, sp);
>     OFFSET(UREGS_PC, struct cpu_user_regs, pc);
>     OFFSET(UREGS_CPSR, struct cpu_user_regs, cpsr);
> +   OFFSET(UREGS_ESR_el2, struct cpu_user_regs, hsr);
>  
>     OFFSET(UREGS_SPSR_el1, struct cpu_user_regs, spsr_el1);
>  
> diff --git a/xen/arch/arm/arm64/entry.S b/xen/arch/arm/arm64/entry.S
> index c181b5e..02802c0 100644
> --- a/xen/arch/arm/arm64/entry.S
> +++ b/xen/arch/arm/arm64/entry.S
> @@ -121,9 +121,13 @@ lr      .req    x30             // link register
>  
>          stp     lr, x21, [sp, #UREGS_LR]
>  
> -        mrs     x22, elr_el2
> -        mrs     x23, spsr_el2
> -        stp     x22, x23, [sp, #UREGS_PC]
> +        mrs     x21, elr_el2
> +        str     x21, [sp, #UREGS_PC]
> +
> +        add     x21, sp, #UREGS_CPSR
> +        mrs     x22, spsr_el2
> +        mrs     x23, esr_el2
> +        stp     w22, w23, [x21]
>  
>          .endm
>  
> @@ -307,7 +311,8 @@ ENTRY(return_to_new_vcpu64)
>  return_from_trap:
>          msr     daifset, #2 /* Mask interrupts */
>  
> -        ldp     x21, x22, [sp, #UREGS_PC]       // load ELR, SPSR
> +        ldr     x21, [sp, #UREGS_PC]            // load ELR
> +        ldr     w22, [sp, #UREGS_CPSR]          // load SPSR
>  
>          pop     x0, x1
>          pop     x2, x3
> diff --git a/xen/arch/arm/arm64/traps.c b/xen/arch/arm/arm64/traps.c
> index 8e89376..36b3a30 100644
> --- a/xen/arch/arm/arm64/traps.c
> +++ b/xen/arch/arm/arm64/traps.c
> @@ -32,7 +32,7 @@ static const char *handler[]= {
>  
>  asmlinkage void do_bad_mode(struct cpu_user_regs *regs, int reason)
>  {
> -    union hsr hsr = { .bits = READ_SYSREG32(ESR_EL2) };
> +    union hsr hsr = { .bits = regs->hsr };
>  
>      printk("Bad mode in %s handler detected\n", handler[reason]);
>      printk("ESR=0x%08"PRIx32":  EC=%"PRIx32", IL=%"PRIx32", ISS=%"PRIx32"\n",
> diff --git a/xen/arch/arm/traps.c b/xen/arch/arm/traps.c
> index 614501f..6137272 100644
> --- a/xen/arch/arm/traps.c
> +++ b/xen/arch/arm/traps.c
> @@ -843,7 +843,7 @@ static void _show_registers(struct cpu_user_regs *regs,
>      printk("   HCR_EL2: %016"PRIregister"\n", READ_SYSREG(HCR_EL2));
>      printk(" TTBR0_EL2: %016"PRIx64"\n", READ_SYSREG64(TTBR0_EL2));
>      printk("\n");
> -    printk("   ESR_EL2: %08"PRIx32"\n", READ_SYSREG32(ESR_EL2));
> +    printk("   ESR_EL2: %08"PRIx32"\n", regs->hsr);
>      printk(" HPFAR_EL2: %016"PRIregister"\n", READ_SYSREG(HPFAR_EL2));
>  
>  #ifdef CONFIG_ARM_32
> @@ -2641,7 +2641,7 @@ static void enter_hypervisor_head(struct cpu_user_regs *regs)
>  
>  asmlinkage void do_trap_hypervisor(struct cpu_user_regs *regs)
>  {
> -    const union hsr hsr = { .bits = READ_SYSREG32(ESR_EL2) };
> +    const union hsr hsr = { .bits = regs->hsr };
>  
>      enter_hypervisor_head(regs);
>  
> diff --git a/xen/include/asm-arm/arm32/processor.h b/xen/include/asm-arm/arm32/processor.h
> index db3b17b..f6d5df3 100644
> --- a/xen/include/asm-arm/arm32/processor.h
> +++ b/xen/include/asm-arm/arm32/processor.h
> @@ -37,7 +37,7 @@ struct cpu_user_regs
>          uint32_t pc, pc32;
>      };
>      uint32_t cpsr; /* Return mode */
> -    uint32_t pad0; /* Doubleword-align the kernel half of the frame */
> +    uint32_t hsr;  /* Exception Syndrome */
>  
>      /* Outer guest frame only from here on... */
>  
> diff --git a/xen/include/asm-arm/arm64/processor.h b/xen/include/asm-arm/arm64/processor.h
> index b0726ff..24f836b 100644
> --- a/xen/include/asm-arm/arm64/processor.h
> +++ b/xen/include/asm-arm/arm64/processor.h
> @@ -66,8 +66,7 @@ struct cpu_user_regs
>      /* Return address and mode */
>      __DECL_REG(pc,           pc32);             /* ELR_EL2 */
>      uint32_t cpsr;                              /* SPSR_EL2 */
> -
> -    uint32_t pad0; /* Align end of kernel frame. */
> +    uint32_t hsr;                               /* ESR_EL2 */
>  
>      /* Outer guest frame only from here on... */
>  
> -- 
> 2.7.4
> 
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> https://lists.xen.org/xen-devel
> 

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