[U-Boot] [PATCH 0/3] efi_loader: ARM: add support for ARMV7_NONSEC=y

[U-Boot] [PATCH 0/3] efi_loader: ARM: add support for ARMV7_NONSEC=y

[Mark Kettenis]

This series makes it possible to run EFI applications in non-secure
mode.  It allows me to run OpenBSD on the imx7d-pico-pi board while
using the PSCI implementation provided by U-Boot.

Mark Kettenis (3):
  ARM: HYP/non-sec: save and restore stack
  efi_loader: ARM: run EFI payloads non-secure
  Revert "efi_loader: no support for ARMV7_NONSEC=y"

 arch/arm/cpu/armv7/nonsec_virt.S |  6 ++++--
 cmd/bootefi.c                    | 32 ++++++++++++++++++++++++++++++++
 doc/README.uefi                  |  2 --
 lib/efi_loader/Kconfig           |  2 --
 4 files changed, 36 insertions(+), 6 deletions(-)

-- 
2.16.2

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

[Mark Kettenis]

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(-)

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)
 
-- 
2.16.2

[U-Boot] [PATCH 2/3] efi_loader: ARM: run EFI payloads non-secure

[Mark Kettenis]

If desired (and possible) switch into HYP mode or non-secure SVC mode
before calling the entry point of an EFI application.  This allows
U-Boot to provide a usable PSCI implementation and makes it possible
to boot kernels into hypervisor mode using an EFI bootloader.

Based on diffs from Heinrich Schuchardt and Alexander Graf.

Signed-off-by: Mark Kettenis <kettenis@openbsd.org>
---
 cmd/bootefi.c | 32 ++++++++++++++++++++++++++++++++
 1 file changed, 32 insertions(+)

diff --git a/cmd/bootefi.c b/cmd/bootefi.c
index 707d159bac..a5d144d9a4 100644
--- a/cmd/bootefi.c
+++ b/cmd/bootefi.c
@@ -20,6 +20,11 @@
 #include <asm-generic/unaligned.h>
 #include <linux/linkage.h>
 
+#ifdef CONFIG_ARM
+#include <asm/armv7.h>
+#include <asm/secure.h>
+#endif
+
 DECLARE_GLOBAL_DATA_PTR;
 
 #define OBJ_LIST_NOT_INITIALIZED 1
@@ -189,6 +194,18 @@ static efi_status_t efi_run_in_el2(EFIAPI efi_status_t (*entry)(
 }
 #endif
 
+#ifdef CONFIG_ARMV7_NONSEC
+static efi_status_t efi_run_in_hyp(EFIAPI efi_status_t (*entry)(
+			efi_handle_t image_handle, struct efi_system_table *st),
+			efi_handle_t image_handle, struct efi_system_table *st)
+{
+	/* Enable caches again */
+	dcache_enable();
+
+	return efi_do_enter(image_handle, st, entry);
+}
+#endif
+
 /* Carve out DT reserved memory ranges */
 static efi_status_t efi_carve_out_dt_rsv(void *fdt)
 {
@@ -338,6 +355,21 @@ static efi_status_t do_bootefi_exec(void *efi,
 	}
 #endif
 
+#ifdef CONFIG_ARMV7_NONSEC
+	if (armv7_boot_nonsec()) {
+		dcache_disable();	/* flush cache before switch to HYP */
+
+		armv7_init_nonsec();
+		secure_ram_addr(_do_nonsec_entry)(efi_run_in_hyp,
+				(uintptr_t)entry,
+				(uintptr_t)loaded_image_info_obj.handle,
+				(uintptr_t)&systab);
+
+		/* Should never reach here, efi exits with longjmp */
+		while (1) { }
+	}
+#endif
+
 	ret = efi_do_enter(loaded_image_info_obj.handle, &systab, entry);
 
 exit:
-- 
2.16.2

[U-Boot] [PATCH 3/3] Revert "efi_loader: no support for ARMV7_NONSEC=y"

[Mark Kettenis]

This reverts commit c524997acb3d322e1bbd36c06ad02ef589705e7c.

Booting ARMv7 in non-secure mode using bootefi works now.

Signed-off-by: Mark Kettenis <kettenis@openbsd.org>
---
 doc/README.uefi        | 2 --
 lib/efi_loader/Kconfig | 2 --
 2 files changed, 4 deletions(-)

diff --git a/doc/README.uefi b/doc/README.uefi
index d4031ef8e8..6b9759cfed 100644
--- a/doc/README.uefi
+++ b/doc/README.uefi
@@ -329,8 +329,6 @@ This driver is only available if U-Boot is configured with
   * persistence
   * runtime support
 
-* support bootefi booting ARMv7 in non-secure mode (CONFIG_ARMV7_NONSEC=y)
-
 ## Links
 
 * [1](http://uefi.org/specifications)
diff --git a/lib/efi_loader/Kconfig b/lib/efi_loader/Kconfig
index df58e633d1..ce6a09f0b4 100644
--- a/lib/efi_loader/Kconfig
+++ b/lib/efi_loader/Kconfig
@@ -1,8 +1,6 @@
 config EFI_LOADER
 	bool "Support running EFI Applications in U-Boot"
 	depends on (ARM || X86 || RISCV) && OF_LIBFDT
-	# We do not support bootefi booting ARMv7 in non-secure mode
-	depends on !ARMV7_NONSEC
 	# We need EFI_STUB_64BIT to be set on x86_64 with EFI_STUB
 	depends on !EFI_STUB || !X86_64 || EFI_STUB_64BIT
 	# We need EFI_STUB_32BIT to be set on x86_32 with EFI_STUB
-- 
2.16.2

[U-Boot] [PATCH 0/3] efi_loader: ARM: add support for ARMV7_NONSEC=y

[Heinrich Schuchardt]

On 06/12/2018 07:27 PM, Mark Kettenis wrote:
> This series makes it possible to run EFI applications in non-secure
> mode.  It allows me to run OpenBSD on the imx7d-pico-pi board while
> using the PSCI implementation provided by U-Boot.
> 
> Mark Kettenis (3):
>   ARM: HYP/non-sec: save and restore stack
>   efi_loader: ARM: run EFI payloads non-secure
>   Revert "efi_loader: no support for ARMV7_NONSEC=y"
> 
>  arch/arm/cpu/armv7/nonsec_virt.S |  6 ++++--
>  cmd/bootefi.c                    | 32 ++++++++++++++++++++++++++++++++
>  doc/README.uefi                  |  2 --
>  lib/efi_loader/Kconfig           |  2 --
>  4 files changed, 36 insertions(+), 6 deletions(-)
> 

This is the output I got with your patches when trying to boot my BananaPi:

=> bootefi hello
Scanning disk mmc at 01c0f000.blk...
Found 3 disks
WARNING: booting without device tree
## Starting EFI application at 42000000 ...
WARNING: using memory device/image path, this may confuse some payloads!

U-Boot SPL 2018.07-rc1-D001-00104-g5b859da7ca8 (Jun 12 2018 - 19:52:34
+0200)
DRAM:

Where able to run bootefi hello on your board?
Which board are you on?

Best regards

Heinrich

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

[Alexander Graf]

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)
>  
> 

[U-Boot] [PATCH 2/3] efi_loader: ARM: run EFI payloads non-secure

[Alexander Graf]

On 12.06.18 19:27, Mark Kettenis wrote:
> If desired (and possible) switch into HYP mode or non-secure SVC mode
> before calling the entry point of an EFI application.  This allows
> U-Boot to provide a usable PSCI implementation and makes it possible
> to boot kernels into hypervisor mode using an EFI bootloader.
> 
> Based on diffs from Heinrich Schuchardt and Alexander Graf.
> 
> Signed-off-by: Mark Kettenis <kettenis@openbsd.org>
> ---
>  cmd/bootefi.c | 32 ++++++++++++++++++++++++++++++++
>  1 file changed, 32 insertions(+)
> 
> diff --git a/cmd/bootefi.c b/cmd/bootefi.c
> index 707d159bac..a5d144d9a4 100644
> --- a/cmd/bootefi.c
> +++ b/cmd/bootefi.c
> @@ -20,6 +20,11 @@
>  #include <asm-generic/unaligned.h>
>  #include <linux/linkage.h>
>  
> +#ifdef CONFIG_ARM

Is this the correct guard? CONFIG_ARM is also set for AArch64 for
example. Maybe use CONFIG_ARMV7_NONSEC instead?

> +#include <asm/armv7.h>
> +#include <asm/secure.h>
> +#endif
> +
>  DECLARE_GLOBAL_DATA_PTR;
>  
>  #define OBJ_LIST_NOT_INITIALIZED 1
> @@ -189,6 +194,18 @@ static efi_status_t efi_run_in_el2(EFIAPI efi_status_t (*entry)(
>  }
>  #endif
>  
> +#ifdef CONFIG_ARMV7_NONSEC
> +static efi_status_t efi_run_in_hyp(EFIAPI efi_status_t (*entry)(
> +			efi_handle_t image_handle, struct efi_system_table *st),
> +			efi_handle_t image_handle, struct efi_system_table *st)
> +{
> +	/* Enable caches again */
> +	dcache_enable();

Are you sure the dcache_enable/disable bits do what you want? IIRC the
situation on armv7 wasn't quite as obvious.


Alex

> +
> +	return efi_do_enter(image_handle, st, entry);
> +}
> +#endif
> +
>  /* Carve out DT reserved memory ranges */
>  static efi_status_t efi_carve_out_dt_rsv(void *fdt)
>  {
> @@ -338,6 +355,21 @@ static efi_status_t do_bootefi_exec(void *efi,
>  	}
>  #endif
>  
> +#ifdef CONFIG_ARMV7_NONSEC
> +	if (armv7_boot_nonsec()) {
> +		dcache_disable();	/* flush cache before switch to HYP */
> +
> +		armv7_init_nonsec();
> +		secure_ram_addr(_do_nonsec_entry)(efi_run_in_hyp,
> +				(uintptr_t)entry,
> +				(uintptr_t)loaded_image_info_obj.handle,
> +				(uintptr_t)&systab);
> +
> +		/* Should never reach here, efi exits with longjmp */
> +		while (1) { }
> +	}
> +#endif
> +
>  	ret = efi_do_enter(loaded_image_info_obj.handle, &systab, entry);
>  
>  exit:
> 

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

[Mark Kettenis]

> 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)
> >  
> > 
> 

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

[Alexander Graf]

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

[U-Boot] [PATCH 0/3] efi_loader: ARM: add support for ARMV7_NONSEC=y

[Mark Kettenis]

> From: Heinrich Schuchardt <xypron.glpk@gmx.de>
> Date: Tue, 12 Jun 2018 20:00:28 +0200
> 
> On 06/12/2018 07:27 PM, Mark Kettenis wrote:
> > This series makes it possible to run EFI applications in non-secure
> > mode.  It allows me to run OpenBSD on the imx7d-pico-pi board while
> > using the PSCI implementation provided by U-Boot.
> > 
> > Mark Kettenis (3):
> >   ARM: HYP/non-sec: save and restore stack
> >   efi_loader: ARM: run EFI payloads non-secure
> >   Revert "efi_loader: no support for ARMV7_NONSEC=y"
> > 
> >  arch/arm/cpu/armv7/nonsec_virt.S |  6 ++++--
> >  cmd/bootefi.c                    | 32 ++++++++++++++++++++++++++++++++
> >  doc/README.uefi                  |  2 --
> >  lib/efi_loader/Kconfig           |  2 --
> >  4 files changed, 36 insertions(+), 6 deletions(-)
> > 
> 
> This is the output I got with your patches when trying to boot my BananaPi:
> 
> => bootefi hello
> Scanning disk mmc at 01c0f000.blk...
> Found 3 disks
> WARNING: booting without device tree
> ## Starting EFI application at 42000000 ...
> WARNING: using memory device/image path, this may confuse some payloads!
> 
> U-Boot SPL 2018.07-rc1-D001-00104-g5b859da7ca8 (Jun 12 2018 - 19:52:34
> +0200)
> DRAM:
> 
> Where able to run bootefi hello on your board?

I can run helloworld.efi on my board with this diff.

> Which board are you on?

Technexion PICO-PI-IMX7; it's a board with an i.MX7D SoC.

I have a Banana Pi as well, so I'll give that one a go tomorrow.

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

[Mark Kettenis]

> 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.

[U-Boot] [PATCH 0/3] efi_loader: ARM: add support for ARMV7_NONSEC=y

[Mark Kettenis]

> From: Heinrich Schuchardt <xypron.glpk@gmx.de>
> Date: Tue, 12 Jun 2018 20:00:28 +0200
> 
> On 06/12/2018 07:27 PM, Mark Kettenis wrote:
> > This series makes it possible to run EFI applications in non-secure
> > mode.  It allows me to run OpenBSD on the imx7d-pico-pi board while
> > using the PSCI implementation provided by U-Boot.
> > 
> > Mark Kettenis (3):
> >   ARM: HYP/non-sec: save and restore stack
> >   efi_loader: ARM: run EFI payloads non-secure
> >   Revert "efi_loader: no support for ARMV7_NONSEC=y"
> > 
> >  arch/arm/cpu/armv7/nonsec_virt.S |  6 ++++--
> >  cmd/bootefi.c                    | 32 ++++++++++++++++++++++++++++++++
> >  doc/README.uefi                  |  2 --
> >  lib/efi_loader/Kconfig           |  2 --
> >  4 files changed, 36 insertions(+), 6 deletions(-)
> > 
> 
> This is the output I got with your patches when trying to boot my BananaPi:
> 
> => bootefi hello
> Scanning disk mmc at 01c0f000.blk...
> Found 3 disks
> WARNING: booting without device tree
> ## Starting EFI application at 42000000 ...
> WARNING: using memory device/image path, this may confuse some payloads!
> 
> U-Boot SPL 2018.07-rc1-D001-00104-g5b859da7ca8 (Jun 12 2018 - 19:52:34
> +0200)
> DRAM:

I could reproduce that.  For some reason r3 gets clobbered and we
still end up with a broken stack.  Posted v2 that simply migrates
SP_svc to SP_hyp.  That seems to work on both imx7 and sun7i.