[U-Boot,1/3] ARM: HYP/non-sec: save and restore stack

Message ID 20180612172748.36790-2-kettenis@openbsd.org
State Superseded
Delegated to: Alexander Graf
Headers show
Series
  • efi_loader: ARM: add support for ARMV7_NONSEC=y
Related show

Commit Message

Mark Kettenis June 12, 2018, 5:27 p.m.
The current code that switches into HYP mode doesn't bother to set
up a stack for HYP mode.  This doesn't work for EFI applications
as they expect a usable stack.  Fix this by saving the stack
pointer before switching and use it to set SP_hyp from monitor.
This restores the stack pointer when we drop into HYP mode.

Signed-off-by: Mark Kettenis <kettenis@openbsd.org>
---
 arch/arm/cpu/armv7/nonsec_virt.S | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

Comments

Alexander Graf June 12, 2018, 6:46 p.m. | #1
On 12.06.18 19:27, Mark Kettenis wrote:
> The current code that switches into HYP mode doesn't bother to set
> up a stack for HYP mode.  This doesn't work for EFI applications
> as they expect a usable stack.  Fix this by saving the stack
> pointer before switching and use it to set SP_hyp from monitor.
> This restores the stack pointer when we drop into HYP mode.
> 
> Signed-off-by: Mark Kettenis <kettenis@openbsd.org>

Can we be sure that the stack in MON is usable from HYP?

Alex

> ---
>  arch/arm/cpu/armv7/nonsec_virt.S | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/arm/cpu/armv7/nonsec_virt.S b/arch/arm/cpu/armv7/nonsec_virt.S
> index 56bdba1d38..246d817340 100644
> --- a/arch/arm/cpu/armv7/nonsec_virt.S
> +++ b/arch/arm/cpu/armv7/nonsec_virt.S
> @@ -52,9 +52,9 @@ _secure_monitor:
>  	bl	psci_stack_setup
>  
>  	@ Configure the PSCI backend
> -	push	{r0, r1, r2, ip}
> +	push	{r0, r1, r2, r3, ip}
>  	bl	psci_arch_init
> -	pop	{r0, r1, r2, ip}
> +	pop	{r0, r1, r2, r3, ip}
>  #endif
>  
>  #ifdef CONFIG_ARM_ERRATA_773022
> @@ -80,6 +80,7 @@ _secure_monitor:
>  #ifdef CONFIG_ARMV7_VIRT
>  	orreq	r5, r5, #0x100			@ allow HVC instruction
>  	moveq	r6, #HYP_MODE			@ Enter the kernel as HYP
> +	msreq	sp_hyp, r3			@ restore saved stack
>  #endif
>  
>  	mcr	p15, 0, r5, c1, c1, 0		@ write SCR (with NS bit set)
> @@ -106,6 +107,7 @@ ENTRY(_do_nonsec_entry)
>  	mov	r0, r1
>  	mov	r1, r2
>  	mov	r2, r3
> +	mov	r3, sp
>  	smc	#0
>  ENDPROC(_do_nonsec_entry)
>  
>
Mark Kettenis June 12, 2018, 8:17 p.m. | #2
> From: Alexander Graf <agraf@suse.de>
> Date: Tue, 12 Jun 2018 20:46:02 +0200
> 
> On 12.06.18 19:27, Mark Kettenis wrote:
> > The current code that switches into HYP mode doesn't bother to set
> > up a stack for HYP mode.  This doesn't work for EFI applications
> > as they expect a usable stack.  Fix this by saving the stack
> > pointer before switching and use it to set SP_hyp from monitor.
> > This restores the stack pointer when we drop into HYP mode.
> > 
> > Signed-off-by: Mark Kettenis <kettenis@openbsd.org>
> 
> Can we be sure that the stack in MON is usable from HYP?

I think so.  It is the stack that U-Boot sets up for itself in normal
memory.  As far as I can tell arm64 re-uses this stack when dropping
down into EL2 as well.

> > ---
> >  arch/arm/cpu/armv7/nonsec_virt.S | 6 ++++--
> >  1 file changed, 4 insertions(+), 2 deletions(-)
> > 
> > diff --git a/arch/arm/cpu/armv7/nonsec_virt.S b/arch/arm/cpu/armv7/nonsec_virt.S
> > index 56bdba1d38..246d817340 100644
> > --- a/arch/arm/cpu/armv7/nonsec_virt.S
> > +++ b/arch/arm/cpu/armv7/nonsec_virt.S
> > @@ -52,9 +52,9 @@ _secure_monitor:
> >  	bl	psci_stack_setup
> >  
> >  	@ Configure the PSCI backend
> > -	push	{r0, r1, r2, ip}
> > +	push	{r0, r1, r2, r3, ip}
> >  	bl	psci_arch_init
> > -	pop	{r0, r1, r2, ip}
> > +	pop	{r0, r1, r2, r3, ip}
> >  #endif
> >  
> >  #ifdef CONFIG_ARM_ERRATA_773022
> > @@ -80,6 +80,7 @@ _secure_monitor:
> >  #ifdef CONFIG_ARMV7_VIRT
> >  	orreq	r5, r5, #0x100			@ allow HVC instruction
> >  	moveq	r6, #HYP_MODE			@ Enter the kernel as HYP
> > +	msreq	sp_hyp, r3			@ restore saved stack
> >  #endif
> >  
> >  	mcr	p15, 0, r5, c1, c1, 0		@ write SCR (with NS bit set)
> > @@ -106,6 +107,7 @@ ENTRY(_do_nonsec_entry)
> >  	mov	r0, r1
> >  	mov	r1, r2
> >  	mov	r2, r3
> > +	mov	r3, sp
> >  	smc	#0
> >  ENDPROC(_do_nonsec_entry)
> >  
> > 
>
Alexander Graf June 12, 2018, 8:32 p.m. | #3
On 12.06.18 22:17, Mark Kettenis wrote:
>> From: Alexander Graf <agraf@suse.de>
>> Date: Tue, 12 Jun 2018 20:46:02 +0200
>>
>> On 12.06.18 19:27, Mark Kettenis wrote:
>>> The current code that switches into HYP mode doesn't bother to set
>>> up a stack for HYP mode.  This doesn't work for EFI applications
>>> as they expect a usable stack.  Fix this by saving the stack
>>> pointer before switching and use it to set SP_hyp from monitor.
>>> This restores the stack pointer when we drop into HYP mode.
>>>
>>> Signed-off-by: Mark Kettenis <kettenis@openbsd.org>
>>
>> Can we be sure that the stack in MON is usable from HYP?
> 
> I think so.  It is the stack that U-Boot sets up for itself in normal
> memory.  As far as I can tell arm64 re-uses this stack when dropping
> down into EL2 as well.

Well, the question is whether it's secure or non-secure memory. Usually
the DRAM controller can be configured to have a window of RAM only
available to secure and I'd certainly hope that at least the U-Boot
parts that are preserved in EL3 live in such a secured area :)


Alex
Mark Kettenis June 13, 2018, 2:55 p.m. | #4
> From: Alexander Graf <agraf@suse.de>
> Date: Tue, 12 Jun 2018 22:32:38 +0200
> 
> On 12.06.18 22:17, Mark Kettenis wrote:
> >> From: Alexander Graf <agraf@suse.de>
> >> Date: Tue, 12 Jun 2018 20:46:02 +0200
> >>
> >> On 12.06.18 19:27, Mark Kettenis wrote:
> >>> The current code that switches into HYP mode doesn't bother to set
> >>> up a stack for HYP mode.  This doesn't work for EFI applications
> >>> as they expect a usable stack.  Fix this by saving the stack
> >>> pointer before switching and use it to set SP_hyp from monitor.
> >>> This restores the stack pointer when we drop into HYP mode.
> >>>
> >>> Signed-off-by: Mark Kettenis <kettenis@openbsd.org>
> >>
> >> Can we be sure that the stack in MON is usable from HYP?
> > 
> > I think so.  It is the stack that U-Boot sets up for itself in normal
> > memory.  As far as I can tell arm64 re-uses this stack when dropping
> > down into EL2 as well.
> 
> Well, the question is whether it's secure or non-secure memory. Usually
> the DRAM controller can be configured to have a window of RAM only
> available to secure and I'd certainly hope that at least the U-Boot
> parts that are preserved in EL3 live in such a secured area :)

The U-Boot PSCI implementation ends up in a special memory region and
uses a separate stack in that same region.  Whether that memory region
is marked as secure in hardware depends on board-specific code.  On
the i.MX7D board I'm playing with it ends up in on on-chip RAM but I'm
not sure the current U-Boot code actually marks that region as secure.
In principle the PSCI code is all that is preserved for MON/EL3.  My
diffs really don't change how that works.  The code is already there
and it already works (I assume) when booting Linux kernels in
non-secure mode the traditional (non-EFI) way.

Patch

diff --git a/arch/arm/cpu/armv7/nonsec_virt.S b/arch/arm/cpu/armv7/nonsec_virt.S
index 56bdba1d38..246d817340 100644
--- a/arch/arm/cpu/armv7/nonsec_virt.S
+++ b/arch/arm/cpu/armv7/nonsec_virt.S
@@ -52,9 +52,9 @@  _secure_monitor:
 	bl	psci_stack_setup
 
 	@ Configure the PSCI backend
-	push	{r0, r1, r2, ip}
+	push	{r0, r1, r2, r3, ip}
 	bl	psci_arch_init
-	pop	{r0, r1, r2, ip}
+	pop	{r0, r1, r2, r3, ip}
 #endif
 
 #ifdef CONFIG_ARM_ERRATA_773022
@@ -80,6 +80,7 @@  _secure_monitor:
 #ifdef CONFIG_ARMV7_VIRT
 	orreq	r5, r5, #0x100			@ allow HVC instruction
 	moveq	r6, #HYP_MODE			@ Enter the kernel as HYP
+	msreq	sp_hyp, r3			@ restore saved stack
 #endif
 
 	mcr	p15, 0, r5, c1, c1, 0		@ write SCR (with NS bit set)
@@ -106,6 +107,7 @@  ENTRY(_do_nonsec_entry)
 	mov	r0, r1
 	mov	r1, r2
 	mov	r2, r3
+	mov	r3, sp
 	smc	#0
 ENDPROC(_do_nonsec_entry)