| Message ID | 20180119134103.3390-8-julien.grall@linaro.org |
|---|---|
| State | Superseded |
| Headers | show |
| Series | xen/arm32: Branch predictor hardening (XSA-254 variant 2) | expand |
On Fri, 19 Jan 2018, Julien Grall wrote: > It took me a bit of time to understand why __DEFINE_TRAP_ENTRY is > storing the original stack pointer in r11. It is working in pair with > return_traps_entry where sp will be restored from r11. > > This is fine because per the AAPCS r11 must be preserved by the > subroutine. So in return_from_trap, r11 will still contain the original > stack pointer. > > Add some documentation in the code to point the 2 sides to each other. > > Signed-off-by: Julien Grall <julien.grall@linaro.org> Reviewed-by: Stefano Stabellini <sstabellini@kernel.org> > --- > xen/arch/arm/arm32/entry.S | 8 ++++++++ > 1 file changed, 8 insertions(+) > > diff --git a/xen/arch/arm/arm32/entry.S b/xen/arch/arm/arm32/entry.S > index c529592d20..7f323de484 100644 > --- a/xen/arch/arm/arm32/entry.S > +++ b/xen/arch/arm/arm32/entry.S > @@ -136,6 +136,10 @@ trap_##trap: \ > cpsie iflags; \ > adr lr, return_from_trap; \ > mov r0, sp; \ > + /* \ > + * Save the stack pointer in r11. It will be restored after the \ > + * trap has been handled (see return_from_trap). \ > + */ \ > mov r11, sp; \ > bic sp, #7; /* Align the stack pointer (noop on guest trap) */ \ > b do_trap_##trap > @@ -246,6 +250,10 @@ DEFINE_TRAP_ENTRY_NOIRQ(fiq) > DEFINE_TRAP_ENTRY_NOABORT(data_abort) > > return_from_trap: > + /* > + * Restore the stack pointer from r11. It was saved on exception > + * entry (see __DEFINE_TRAP_ENTRY). > + */ > mov sp, r11 > ENTRY(return_to_new_vcpu32) > ldr r11, [sp, #UREGS_cpsr] > -- > 2.11.0 >
diff --git a/xen/arch/arm/arm32/entry.S b/xen/arch/arm/arm32/entry.S index c529592d20..7f323de484 100644 --- a/xen/arch/arm/arm32/entry.S +++ b/xen/arch/arm/arm32/entry.S @@ -136,6 +136,10 @@ trap_##trap: \ cpsie iflags; \ adr lr, return_from_trap; \ mov r0, sp; \ + /* \ + * Save the stack pointer in r11. It will be restored after the \ + * trap has been handled (see return_from_trap). \ + */ \ mov r11, sp; \ bic sp, #7; /* Align the stack pointer (noop on guest trap) */ \ b do_trap_##trap @@ -246,6 +250,10 @@ DEFINE_TRAP_ENTRY_NOIRQ(fiq) DEFINE_TRAP_ENTRY_NOABORT(data_abort) return_from_trap: + /* + * Restore the stack pointer from r11. It was saved on exception + * entry (see __DEFINE_TRAP_ENTRY). + */ mov sp, r11 ENTRY(return_to_new_vcpu32) ldr r11, [sp, #UREGS_cpsr]
It took me a bit of time to understand why __DEFINE_TRAP_ENTRY is storing the original stack pointer in r11. It is working in pair with return_traps_entry where sp will be restored from r11. This is fine because per the AAPCS r11 must be preserved by the subroutine. So in return_from_trap, r11 will still contain the original stack pointer. Add some documentation in the code to point the 2 sides to each other. Signed-off-by: Julien Grall <julien.grall@linaro.org> --- xen/arch/arm/arm32/entry.S | 8 ++++++++ 1 file changed, 8 insertions(+)