|
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] [PATCH 05/18] xen/arm: Save ESR_EL2 to avoid using mismatched value in syndrome check
Hi Stefano, On 2017/3/17 6:27, Stefano Stabellini wrote: > On Thu, 16 Mar 2017, Julien Grall wrote: >> Hi Wei, >> >> On 03/13/2017 10:55 AM, 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 will enter in IRQ exception mode and return the processor with >> >> s/will/may/ >> >>> 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 a mismatched ESR_EL2 >> >> s/mismatched/incorrect/ I think >> >>> value. So we want to save ESR_EL2 to cpu_user_regs as soon as the >>> exception takes place in EL2 to avoid using a mismatched syndrome value. >> >> Ditto. > > Please address Julien's comments. The code looks good. > Thanks, I will address them in next version. > >>> >>> Signed-off-by: Wei Chen <Wei.Chen@xxxxxxx> >>> --- >>> 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/traps.c | 2 +- >>> xen/include/asm-arm/arm32/processor.h | 2 +- >>> xen/include/asm-arm/arm64/processor.h | 10 ++++++++-- >>> 7 files changed, 24 insertions(+), 8 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] >> >> Please explain the commit message you modify the lines above ... >> >>> + >>> + 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 >> >> as long as those one. >> >>> >>> pop x0, x1 >>> pop x2, x3 >> >> [...] >> >>> diff --git a/xen/include/asm-arm/arm64/processor.h >>> b/xen/include/asm-arm/arm64/processor.h >>> index b0726ff..d381428 100644 >>> --- a/xen/include/asm-arm/arm64/processor.h >>> +++ b/xen/include/asm-arm/arm64/processor.h >>> @@ -65,9 +65,15 @@ struct cpu_user_regs >>> >>> /* Return address and mode */ >>> __DECL_REG(pc, pc32); /* ELR_EL2 */ >>> + /* >>> + * Be careful for 32-bit registers, if we use xN to save 32-bit >>> register >>> + * to stack, its next field on stack will be overridden. >>> + * For example, if we use xN to save SPSR_EL2 to stack will override >>> the >>> + * hsr field on stack. >>> + * So, it's better to use wN to save 32-bit registers to stack. >>> + */ >> >> This comment is pointless. This is true for any 32-bit register, you should >> use wN unless you now that you have a padding after. >> >>> 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... */ >>> >>> >> >> Cheers, >> >> -- >> Julien Grall >> > -- Regards, Wei Chen _______________________________________________ Xen-devel mailing list Xen-devel@xxxxxxxxxxxxx https://lists.xen.org/xen-devel
|
 |
Lists.xenproject.org is hosted with RackSpace, monitoring our |