All of lore.kernel.org
 help / color / mirror / Atom feed
* [U-Boot] [PATCH v3 0/3] efi_loader: ARM: add support for ARMV7_NONSEC=y
@ 2018-06-13 22:41 Mark Kettenis
  2018-06-13 22:41 ` [U-Boot] [PATCH v3 1/3] ARM: HYP/non-sec: migrate stack Mark Kettenis
                   ` (4 more replies)
  0 siblings, 5 replies; 27+ messages in thread
From: Mark Kettenis @ 2018-06-13 22:41 UTC (permalink / raw)
  To: u-boot

This series makes it possible to run EFI applications in non-secure
mode.  It allows me to run OpenBSD on the Technexion PICO-PI-IMX7 and
Banana Pi boards using the PSCI implementation provided by U-Boot.

The second version avoids using r3 to pass the original stack pointer.
For some reason that register gets clobbered on the Banana Pi.  Instead
this version just migrates SP_svc to SP_hyp.

This third version avoids saving r3 on the stack and fixes an include
guard as suggested by Alexander Graf.

Mark Kettenis (3):
  ARM: HYP/non-sec: migrate 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 |  2 ++
 cmd/bootefi.c                    | 32 ++++++++++++++++++++++++++++++++
 doc/README.uefi                  |  2 --
 lib/efi_loader/Kconfig           |  2 --
 4 files changed, 34 insertions(+), 4 deletions(-)

-- 
2.16.2

^ permalink raw reply	[flat|nested] 27+ messages in thread

* [U-Boot] [PATCH v3 1/3] ARM: HYP/non-sec: migrate stack
  2018-06-13 22:41 [U-Boot] [PATCH v3 0/3] efi_loader: ARM: add support for ARMV7_NONSEC=y Mark Kettenis
@ 2018-06-13 22:41 ` Mark Kettenis
  2018-06-13 22:41 ` [U-Boot] [PATCH v3 2/3] efi_loader: ARM: run EFI payloads non-secure Mark Kettenis
                   ` (3 subsequent siblings)
  4 siblings, 0 replies; 27+ messages in thread
From: Mark Kettenis @ 2018-06-13 22:41 UTC (permalink / raw)
  To: u-boot

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 migrating the stack
pointer from SP_svc to SP_hyp while in Monitor mode.
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 | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/arch/arm/cpu/armv7/nonsec_virt.S b/arch/arm/cpu/armv7/nonsec_virt.S
index 56bdba1d38..1773fae205 100644
--- a/arch/arm/cpu/armv7/nonsec_virt.S
+++ b/arch/arm/cpu/armv7/nonsec_virt.S
@@ -80,6 +80,8 @@ _secure_monitor:
 #ifdef CONFIG_ARMV7_VIRT
 	orreq	r5, r5, #0x100			@ allow HVC instruction
 	moveq	r6, #HYP_MODE			@ Enter the kernel as HYP
+	mrseq	r3, sp_svc
+	msreq	sp_hyp, r3			@ migrate SP
 #endif
 
 	mcr	p15, 0, r5, c1, c1, 0		@ write SCR (with NS bit set)
-- 
2.16.2

^ permalink raw reply related	[flat|nested] 27+ messages in thread

* [U-Boot] [PATCH v3 2/3] efi_loader: ARM: run EFI payloads non-secure
  2018-06-13 22:41 [U-Boot] [PATCH v3 0/3] efi_loader: ARM: add support for ARMV7_NONSEC=y Mark Kettenis
  2018-06-13 22:41 ` [U-Boot] [PATCH v3 1/3] ARM: HYP/non-sec: migrate stack Mark Kettenis
@ 2018-06-13 22:41 ` Mark Kettenis
  2018-06-14 11:54   ` Marc Zyngier
  2018-08-31 17:37   ` Heinrich Schuchardt
  2018-06-13 22:41 ` [U-Boot] [PATCH v3 3/3] Revert "efi_loader: no support for ARMV7_NONSEC=y" Mark Kettenis
                   ` (2 subsequent siblings)
  4 siblings, 2 replies; 27+ messages in thread
From: Mark Kettenis @ 2018-06-13 22:41 UTC (permalink / raw)
  To: u-boot

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..12a6b84ce6 100644
--- a/cmd/bootefi.c
+++ b/cmd/bootefi.c
@@ -20,6 +20,11 @@
 #include <asm-generic/unaligned.h>
 #include <linux/linkage.h>
 
+#ifdef CONFIG_ARMV7_NONSEC
+#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

^ permalink raw reply related	[flat|nested] 27+ messages in thread

* [U-Boot] [PATCH v3 3/3] Revert "efi_loader: no support for ARMV7_NONSEC=y"
  2018-06-13 22:41 [U-Boot] [PATCH v3 0/3] efi_loader: ARM: add support for ARMV7_NONSEC=y Mark Kettenis
  2018-06-13 22:41 ` [U-Boot] [PATCH v3 1/3] ARM: HYP/non-sec: migrate stack Mark Kettenis
  2018-06-13 22:41 ` [U-Boot] [PATCH v3 2/3] efi_loader: ARM: run EFI payloads non-secure Mark Kettenis
@ 2018-06-13 22:41 ` Mark Kettenis
  2018-06-14  8:46 ` [U-Boot] [PATCH v3 0/3] efi_loader: ARM: add support for ARMV7_NONSEC=y Alexander Graf
  2018-06-14 17:55 ` Heinrich Schuchardt
  4 siblings, 0 replies; 27+ messages in thread
From: Mark Kettenis @ 2018-06-13 22:41 UTC (permalink / raw)
  To: u-boot

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

^ permalink raw reply related	[flat|nested] 27+ messages in thread

* [U-Boot] [PATCH v3 0/3] efi_loader: ARM: add support for ARMV7_NONSEC=y
  2018-06-13 22:41 [U-Boot] [PATCH v3 0/3] efi_loader: ARM: add support for ARMV7_NONSEC=y Mark Kettenis
                   ` (2 preceding siblings ...)
  2018-06-13 22:41 ` [U-Boot] [PATCH v3 3/3] Revert "efi_loader: no support for ARMV7_NONSEC=y" Mark Kettenis
@ 2018-06-14  8:46 ` Alexander Graf
  2018-06-14 17:55 ` Heinrich Schuchardt
  4 siblings, 0 replies; 27+ messages in thread
From: Alexander Graf @ 2018-06-14  8:46 UTC (permalink / raw)
  To: u-boot

On 06/14/2018 12:41 AM, Mark Kettenis wrote:
> This series makes it possible to run EFI applications in non-secure
> mode.  It allows me to run OpenBSD on the Technexion PICO-PI-IMX7 and
> Banana Pi boards using the PSCI implementation provided by U-Boot.
>
> The second version avoids using r3 to pass the original stack pointer.
> For some reason that register gets clobbered on the Banana Pi.  Instead
> this version just migrates SP_svc to SP_hyp.
>
> This third version avoids saving r3 on the stack and fixes an include
> guard as suggested by Alexander Graf.

Looks great to me, but I'd like to see at least Heinrich give his 
reviewed-by before I pull it in :).

Alex

^ permalink raw reply	[flat|nested] 27+ messages in thread

* [U-Boot] [PATCH v3 2/3] efi_loader: ARM: run EFI payloads non-secure
  2018-06-13 22:41 ` [U-Boot] [PATCH v3 2/3] efi_loader: ARM: run EFI payloads non-secure Mark Kettenis
@ 2018-06-14 11:54   ` Marc Zyngier
  2018-06-14 20:55     ` Mark Kettenis
  2018-08-31 17:37   ` Heinrich Schuchardt
  1 sibling, 1 reply; 27+ messages in thread
From: Marc Zyngier @ 2018-06-14 11:54 UTC (permalink / raw)
  To: u-boot

On 13/06/18 23:41, 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..12a6b84ce6 100644
> --- a/cmd/bootefi.c
> +++ b/cmd/bootefi.c
> @@ -20,6 +20,11 @@
>  #include <asm-generic/unaligned.h>
>  #include <linux/linkage.h>
>  
> +#ifdef CONFIG_ARMV7_NONSEC
> +#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 */
> +

What is the rational for disabling/enabling caches across the transition
to HYP? I'm sure there is a good reason, but I'd rather see it explained
here.

> +		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:
> 

Thanks,

	M.
-- 
Jazz is not dead. It just smells funny...

^ permalink raw reply	[flat|nested] 27+ messages in thread

* [U-Boot] [PATCH v3 0/3] efi_loader: ARM: add support for ARMV7_NONSEC=y
  2018-06-13 22:41 [U-Boot] [PATCH v3 0/3] efi_loader: ARM: add support for ARMV7_NONSEC=y Mark Kettenis
                   ` (3 preceding siblings ...)
  2018-06-14  8:46 ` [U-Boot] [PATCH v3 0/3] efi_loader: ARM: add support for ARMV7_NONSEC=y Alexander Graf
@ 2018-06-14 17:55 ` Heinrich Schuchardt
  2018-06-14 18:00   ` Alexander Graf
  2018-06-14 20:50   ` Mark Kettenis
  4 siblings, 2 replies; 27+ messages in thread
From: Heinrich Schuchardt @ 2018-06-14 17:55 UTC (permalink / raw)
  To: u-boot

On 06/14/2018 12:41 AM, Mark Kettenis wrote:
> This series makes it possible to run EFI applications in non-secure
> mode.  It allows me to run OpenBSD on the Technexion PICO-PI-IMX7 and
> Banana Pi boards using the PSCI implementation provided by U-Boot.
> 
> The second version avoids using r3 to pass the original stack pointer.
> For some reason that register gets clobbered on the Banana Pi.  Instead
> this version just migrates SP_svc to SP_hyp.
> 
> This third version avoids saving r3 on the stack and fixes an include
> guard as suggested by Alexander Graf.
> 
> Mark Kettenis (3):
>   ARM: HYP/non-sec: migrate 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 |  2 ++
>  cmd/bootefi.c                    | 32 ++++++++++++++++++++++++++++++++
>  doc/README.uefi                  |  2 --
>  lib/efi_loader/Kconfig           |  2 --
>  4 files changed, 34 insertions(+), 4 deletions(-)
> 
Hello Mark,

with this patch series running bootefi hello twice in sequence fails on
the 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!
Hello, world!
Running on UEFI 2.7
Have SMBIOS table
Load options: earlyprintk nosmp
## Application terminated, r = 0
=> bootefi hello
WARNING: booting without device tree
## Starting EFI application at 42000000 ...
WARNING: using memory device/image path, this may confuse some payloads!
<!-- no output after the preceding line -->

Please, keep in mind that we expect multiple EFI binaries to be executed
in sequence. E.g. the first binary installs a driver. The second is the
application using it.

Running iPXE's snp.efi binary shows changed behavior on the console. New
characters are displayed in "slow motion" (3 characters per second).
Setting up the network interface fails in iPXE.

Best regards

Heinrich

^ permalink raw reply	[flat|nested] 27+ messages in thread

* [U-Boot] [PATCH v3 0/3] efi_loader: ARM: add support for ARMV7_NONSEC=y
  2018-06-14 17:55 ` Heinrich Schuchardt
@ 2018-06-14 18:00   ` Alexander Graf
  2018-06-14 20:50   ` Mark Kettenis
  1 sibling, 0 replies; 27+ messages in thread
From: Alexander Graf @ 2018-06-14 18:00 UTC (permalink / raw)
  To: u-boot



On 14.06.18 19:55, Heinrich Schuchardt wrote:
> On 06/14/2018 12:41 AM, Mark Kettenis wrote:
>> This series makes it possible to run EFI applications in non-secure
>> mode.  It allows me to run OpenBSD on the Technexion PICO-PI-IMX7 and
>> Banana Pi boards using the PSCI implementation provided by U-Boot.
>>
>> The second version avoids using r3 to pass the original stack pointer.
>> For some reason that register gets clobbered on the Banana Pi.  Instead
>> this version just migrates SP_svc to SP_hyp.
>>
>> This third version avoids saving r3 on the stack and fixes an include
>> guard as suggested by Alexander Graf.
>>
>> Mark Kettenis (3):
>>   ARM: HYP/non-sec: migrate 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 |  2 ++
>>  cmd/bootefi.c                    | 32 ++++++++++++++++++++++++++++++++
>>  doc/README.uefi                  |  2 --
>>  lib/efi_loader/Kconfig           |  2 --
>>  4 files changed, 34 insertions(+), 4 deletions(-)
>>
> Hello Mark,
> 
> with this patch series running bootefi hello twice in sequence fails on
> the 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!
> Hello, world!
> Running on UEFI 2.7
> Have SMBIOS table
> Load options: earlyprintk nosmp
> ## Application terminated, r = 0
> => bootefi hello
> WARNING: booting without device tree
> ## Starting EFI application at 42000000 ...
> WARNING: using memory device/image path, this may confuse some payloads!
> <!-- no output after the preceding line -->
> 
> Please, keep in mind that we expect multiple EFI binaries to be executed
> in sequence. E.g. the first binary installs a driver. The second is the
> application using it.
> 
> Running iPXE's snp.efi binary shows changed behavior on the console. New
> characters are displayed in "slow motion" (3 characters per second).

Cache disabled maybe?

Alex

> Setting up the network interface fails in iPXE.
> 
> Best regards
> 
> Heinrich
> 

^ permalink raw reply	[flat|nested] 27+ messages in thread

* [U-Boot] [PATCH v3 0/3] efi_loader: ARM: add support for ARMV7_NONSEC=y
  2018-06-14 17:55 ` Heinrich Schuchardt
  2018-06-14 18:00   ` Alexander Graf
@ 2018-06-14 20:50   ` Mark Kettenis
  2018-06-15  3:39     ` Heinrich Schuchardt
  2018-06-15 20:35     ` Heinrich Schuchardt
  1 sibling, 2 replies; 27+ messages in thread
From: Mark Kettenis @ 2018-06-14 20:50 UTC (permalink / raw)
  To: u-boot

> From: Heinrich Schuchardt <xypron.glpk@gmx.de>
> Date: Thu, 14 Jun 2018 19:55:51 +0200
> 
> On 06/14/2018 12:41 AM, Mark Kettenis wrote:
> > This series makes it possible to run EFI applications in non-secure
> > mode.  It allows me to run OpenBSD on the Technexion PICO-PI-IMX7 and
> > Banana Pi boards using the PSCI implementation provided by U-Boot.
> > 
> > The second version avoids using r3 to pass the original stack pointer.
> > For some reason that register gets clobbered on the Banana Pi.  Instead
> > this version just migrates SP_svc to SP_hyp.
> > 
> > This third version avoids saving r3 on the stack and fixes an include
> > guard as suggested by Alexander Graf.
> > 
> > Mark Kettenis (3):
> >   ARM: HYP/non-sec: migrate 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 |  2 ++
> >  cmd/bootefi.c                    | 32 ++++++++++++++++++++++++++++++++
> >  doc/README.uefi                  |  2 --
> >  lib/efi_loader/Kconfig           |  2 --
> >  4 files changed, 34 insertions(+), 4 deletions(-)
> > 
> Hello Mark,
> 
> with this patch series running bootefi hello twice in sequence fails on
> the 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!
> Hello, world!
> Running on UEFI 2.7
> Have SMBIOS table
> Load options: earlyprintk nosmp
> ## Application terminated, r = 0
> => bootefi hello
> WARNING: booting without device tree
> ## Starting EFI application at 42000000 ...
> WARNING: using memory device/image path, this may confuse some payloads!
> <!-- no output after the preceding line -->

Yeah.  Trying to enter non-secure mode when we're already in
non-secure mode doesn't really work.  We should skip the switching
code in that case.  Now checking whether we are in non-secure mode
isn't really possible.  But I guess we can set a variable and check it
before we go down the switching codepath.  With that in I can exit the
OpenBSD bootloader and then reload and run it again.  I'll include
that fix in the next respin.

> Please, keep in mind that we expect multiple EFI binaries to be executed
> in sequence. E.g. the first binary installs a driver. The second is the
> application using it.
> 
> Running iPXE's snp.efi binary shows changed behavior on the console. New
> characters are displayed in "slow motion" (3 characters per second).
> Setting up the network interface fails in iPXE.

The same happens on my Banana Pi.  But not on the imx7 board.  

^ permalink raw reply	[flat|nested] 27+ messages in thread

* [U-Boot] [PATCH v3 2/3] efi_loader: ARM: run EFI payloads non-secure
  2018-06-14 11:54   ` Marc Zyngier
@ 2018-06-14 20:55     ` Mark Kettenis
  2018-06-15  7:59       ` Marc Zyngier
  0 siblings, 1 reply; 27+ messages in thread
From: Mark Kettenis @ 2018-06-14 20:55 UTC (permalink / raw)
  To: u-boot

> From: Marc Zyngier <marc.zyngier@arm.com>
> Date: Thu, 14 Jun 2018 12:54:53 +0100
> 
> On 13/06/18 23:41, 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..12a6b84ce6 100644
> > --- a/cmd/bootefi.c
> > +++ b/cmd/bootefi.c
> > @@ -20,6 +20,11 @@
> >  #include <asm-generic/unaligned.h>
> >  #include <linux/linkage.h>
> >  
> > +#ifdef CONFIG_ARMV7_NONSEC
> > +#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 */
> > +
> 
> What is the rational for disabling/enabling caches across the transition
> to HYP? I'm sure there is a good reason, but I'd rather see it explained
> here.

Can't say I fully understan why.  But the AArch64 code does this as
well and if I don't flush the cache here the contents of efi_gd (which
gets initialized before the switch) sometimes gets lost.

> > +		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:
> > 

^ permalink raw reply	[flat|nested] 27+ messages in thread

* [U-Boot] [PATCH v3 0/3] efi_loader: ARM: add support for ARMV7_NONSEC=y
  2018-06-14 20:50   ` Mark Kettenis
@ 2018-06-15  3:39     ` Heinrich Schuchardt
  2018-06-15 10:49       ` Alexander Graf
  2018-06-15 20:35     ` Heinrich Schuchardt
  1 sibling, 1 reply; 27+ messages in thread
From: Heinrich Schuchardt @ 2018-06-15  3:39 UTC (permalink / raw)
  To: u-boot

On 06/14/2018 10:50 PM, Mark Kettenis wrote:
>> From: Heinrich Schuchardt <xypron.glpk@gmx.de>
>> Date: Thu, 14 Jun 2018 19:55:51 +0200
>>
>> On 06/14/2018 12:41 AM, Mark Kettenis wrote:
>>> This series makes it possible to run EFI applications in non-secure
>>> mode.  It allows me to run OpenBSD on the Technexion PICO-PI-IMX7 and
>>> Banana Pi boards using the PSCI implementation provided by U-Boot.
>>>
>>> The second version avoids using r3 to pass the original stack pointer.
>>> For some reason that register gets clobbered on the Banana Pi.  Instead
>>> this version just migrates SP_svc to SP_hyp.
>>>
>>> This third version avoids saving r3 on the stack and fixes an include
>>> guard as suggested by Alexander Graf.
>>>
>>> Mark Kettenis (3):
>>>   ARM: HYP/non-sec: migrate 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 |  2 ++
>>>  cmd/bootefi.c                    | 32 ++++++++++++++++++++++++++++++++
>>>  doc/README.uefi                  |  2 --
>>>  lib/efi_loader/Kconfig           |  2 --
>>>  4 files changed, 34 insertions(+), 4 deletions(-)
>>>
>> Hello Mark,
>>
>> with this patch series running bootefi hello twice in sequence fails on
>> the 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!
>> Hello, world!
>> Running on UEFI 2.7
>> Have SMBIOS table
>> Load options: earlyprintk nosmp
>> ## Application terminated, r = 0
>> => bootefi hello
>> WARNING: booting without device tree
>> ## Starting EFI application at 42000000 ...
>> WARNING: using memory device/image path, this may confuse some payloads!
>> <!-- no output after the preceding line -->
> 
> Yeah.  Trying to enter non-secure mode when we're already in
> non-secure mode doesn't really work.  We should skip the switching
> code in that case.  Now checking whether we are in non-secure mode
> isn't really possible.  But I guess we can set a variable and check it
> before we go down the switching codepath.  With that in I can exit the
> OpenBSD bootloader and then reload and run it again.  I'll include
> that fix in the next respin.

Hello Mark,

you might move the call to switch to non-secure mode to efi_init_obj_list().

Best regards

Heinrich

> 
>> Please, keep in mind that we expect multiple EFI binaries to be executed
>> in sequence. E.g. the first binary installs a driver. The second is the
>> application using it.
>>
>> Running iPXE's snp.efi binary shows changed behavior on the console. New
>> characters are displayed in "slow motion" (3 characters per second).
>> Setting up the network interface fails in iPXE.
> 
> The same happens on my Banana Pi.  But not on the imx7 board.  
> 

^ permalink raw reply	[flat|nested] 27+ messages in thread

* [U-Boot] [PATCH v3 2/3] efi_loader: ARM: run EFI payloads non-secure
  2018-06-14 20:55     ` Mark Kettenis
@ 2018-06-15  7:59       ` Marc Zyngier
  2018-06-15 12:51         ` Mark Kettenis
  0 siblings, 1 reply; 27+ messages in thread
From: Marc Zyngier @ 2018-06-15  7:59 UTC (permalink / raw)
  To: u-boot

On 14/06/18 21:55, Mark Kettenis wrote:
>> From: Marc Zyngier <marc.zyngier@arm.com>
>> Date: Thu, 14 Jun 2018 12:54:53 +0100
>>
>> On 13/06/18 23:41, 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..12a6b84ce6 100644
>>> --- a/cmd/bootefi.c
>>> +++ b/cmd/bootefi.c
>>> @@ -20,6 +20,11 @@
>>>  #include <asm-generic/unaligned.h>
>>>  #include <linux/linkage.h>
>>>  
>>> +#ifdef CONFIG_ARMV7_NONSEC
>>> +#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 */
>>> +
>>
>> What is the rational for disabling/enabling caches across the transition
>> to HYP? I'm sure there is a good reason, but I'd rather see it explained
>> here.
> 
> Can't say I fully understan why.  But the AArch64 code does this as
> well and if I don't flush the cache here the contents of efi_gd (which
> gets initialized before the switch) sometimes gets lost.

I guess the following can happen:

- EL1 code (or SVC for AArch32) has its MMU enabled, and caches are on
- Writes from EL1 are nicely sitting in the dcache
- Enter EL2 (HYP) where the MMU is off, and thus the caches are too
- The uncached accesses do not hit in the cache, and sh*t happens

dcache_disable also cleans to the PoC, making sure that everything is
visible even when the MMU and caches are off. I have the strong feeling
that dcache_enable is utterly useless as I don't think you install any
page table at HYP (that code was never designed to run anything other
than jumping into the kernel).

It would make a lot more sense if you installed id-mapped page tables at
HYP too in order to enable the caches, and geta  bit of performance back
(otherwise anything you run at HYP will negatively compare to the speed
of an anaemic snail stuck on sand).

Thanks,

	M.
-- 
Jazz is not dead. It just smells funny...

^ permalink raw reply	[flat|nested] 27+ messages in thread

* [U-Boot] [PATCH v3 0/3] efi_loader: ARM: add support for ARMV7_NONSEC=y
  2018-06-15  3:39     ` Heinrich Schuchardt
@ 2018-06-15 10:49       ` Alexander Graf
  2018-06-15 13:01         ` Mark Kettenis
  0 siblings, 1 reply; 27+ messages in thread
From: Alexander Graf @ 2018-06-15 10:49 UTC (permalink / raw)
  To: u-boot



On 15.06.18 05:39, Heinrich Schuchardt wrote:
> On 06/14/2018 10:50 PM, Mark Kettenis wrote:
>>> From: Heinrich Schuchardt <xypron.glpk@gmx.de>
>>> Date: Thu, 14 Jun 2018 19:55:51 +0200
>>>
>>> On 06/14/2018 12:41 AM, Mark Kettenis wrote:
>>>> This series makes it possible to run EFI applications in non-secure
>>>> mode.  It allows me to run OpenBSD on the Technexion PICO-PI-IMX7 and
>>>> Banana Pi boards using the PSCI implementation provided by U-Boot.
>>>>
>>>> The second version avoids using r3 to pass the original stack pointer.
>>>> For some reason that register gets clobbered on the Banana Pi.  Instead
>>>> this version just migrates SP_svc to SP_hyp.
>>>>
>>>> This third version avoids saving r3 on the stack and fixes an include
>>>> guard as suggested by Alexander Graf.
>>>>
>>>> Mark Kettenis (3):
>>>>   ARM: HYP/non-sec: migrate 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 |  2 ++
>>>>  cmd/bootefi.c                    | 32 ++++++++++++++++++++++++++++++++
>>>>  doc/README.uefi                  |  2 --
>>>>  lib/efi_loader/Kconfig           |  2 --
>>>>  4 files changed, 34 insertions(+), 4 deletions(-)
>>>>
>>> Hello Mark,
>>>
>>> with this patch series running bootefi hello twice in sequence fails on
>>> the 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!
>>> Hello, world!
>>> Running on UEFI 2.7
>>> Have SMBIOS table
>>> Load options: earlyprintk nosmp
>>> ## Application terminated, r = 0
>>> => bootefi hello
>>> WARNING: booting without device tree
>>> ## Starting EFI application at 42000000 ...
>>> WARNING: using memory device/image path, this may confuse some payloads!
>>> <!-- no output after the preceding line -->
>>
>> Yeah.  Trying to enter non-secure mode when we're already in
>> non-secure mode doesn't really work.  We should skip the switching
>> code in that case.  Now checking whether we are in non-secure mode
>> isn't really possible.  But I guess we can set a variable and check it
>> before we go down the switching codepath.  With that in I can exit the
>> OpenBSD bootloader and then reload and run it again.  I'll include
>> that fix in the next respin.
> 
> Hello Mark,
> 
> you might move the call to switch to non-secure mode to efi_init_obj_list().

I would actually prefer to keep it where it is. That way we have the
option to move the object initialization to a different stage later on.

The only thing missing is really a check which level we're on. The
aarch64 code does this with a current_el() == 3 condition.


Alex

^ permalink raw reply	[flat|nested] 27+ messages in thread

* [U-Boot] [PATCH v3 2/3] efi_loader: ARM: run EFI payloads non-secure
  2018-06-15  7:59       ` Marc Zyngier
@ 2018-06-15 12:51         ` Mark Kettenis
  2018-06-15 13:20           ` Marc Zyngier
  0 siblings, 1 reply; 27+ messages in thread
From: Mark Kettenis @ 2018-06-15 12:51 UTC (permalink / raw)
  To: u-boot

> From: Marc Zyngier <marc.zyngier@arm.com>
> Date: Fri, 15 Jun 2018 08:59:59 +0100
> 
> On 14/06/18 21:55, Mark Kettenis wrote:
> >> From: Marc Zyngier <marc.zyngier@arm.com>
> >> Date: Thu, 14 Jun 2018 12:54:53 +0100
> >>
> >> On 13/06/18 23:41, 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..12a6b84ce6 100644
> >>> --- a/cmd/bootefi.c
> >>> +++ b/cmd/bootefi.c
> >>> @@ -20,6 +20,11 @@
> >>>  #include <asm-generic/unaligned.h>
> >>>  #include <linux/linkage.h>
> >>>  
> >>> +#ifdef CONFIG_ARMV7_NONSEC
> >>> +#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 */
> >>> +
> >>
> >> What is the rational for disabling/enabling caches across the transition
> >> to HYP? I'm sure there is a good reason, but I'd rather see it explained
> >> here.
> > 
> > Can't say I fully understan why.  But the AArch64 code does this as
> > well and if I don't flush the cache here the contents of efi_gd (which
> > gets initialized before the switch) sometimes gets lost.
> 
> I guess the following can happen:
> 
> - EL1 code (or SVC for AArch32) has its MMU enabled, and caches are on
> - Writes from EL1 are nicely sitting in the dcache
> - Enter EL2 (HYP) where the MMU is off, and thus the caches are too
> - The uncached accesses do not hit in the cache, and sh*t happens
> 
> dcache_disable also cleans to the PoC, making sure that everything is
> visible even when the MMU and caches are off. I have the strong feeling
> that dcache_enable is utterly useless as I don't think you install any
> page table at HYP (that code was never designed to run anything other
> than jumping into the kernel).

There actually is code in arch/arm/lib/cache-cp15.c to set up the page
tables for HYP and enable the MMU.  And it would run as part of the
dcache_enable() call if CONFIG_ARMV7_LPAE was defined.  But that isn't
set on the boards I'm looking at.  I'll have a go at enabling that option.

> It would make a lot more sense if you installed id-mapped page tables at
> HYP too in order to enable the caches, and geta  bit of performance back
> (otherwise anything you run at HYP will negatively compare to the speed
> of an anaemic snail stuck on sand).

On the Allwinner A20 it certainly does crawl; console output is really
slow.  Didn't notice it on the NXP i.MX7D board though.  Anyway,
thanks for the hint!

^ permalink raw reply	[flat|nested] 27+ messages in thread

* [U-Boot] [PATCH v3 0/3] efi_loader: ARM: add support for ARMV7_NONSEC=y
  2018-06-15 10:49       ` Alexander Graf
@ 2018-06-15 13:01         ` Mark Kettenis
  2018-06-15 13:12           ` Alexander Graf
  0 siblings, 1 reply; 27+ messages in thread
From: Mark Kettenis @ 2018-06-15 13:01 UTC (permalink / raw)
  To: u-boot

> From: Alexander Graf <agraf@suse.de>
> Date: Fri, 15 Jun 2018 12:49:48 +0200
> 
> On 15.06.18 05:39, Heinrich Schuchardt wrote:
> > On 06/14/2018 10:50 PM, Mark Kettenis wrote:
> >>> From: Heinrich Schuchardt <xypron.glpk@gmx.de>
> >>> Date: Thu, 14 Jun 2018 19:55:51 +0200
> >>>
> >>> On 06/14/2018 12:41 AM, Mark Kettenis wrote:
> >>>> This series makes it possible to run EFI applications in non-secure
> >>>> mode.  It allows me to run OpenBSD on the Technexion PICO-PI-IMX7 and
> >>>> Banana Pi boards using the PSCI implementation provided by U-Boot.
> >>>>
> >>>> The second version avoids using r3 to pass the original stack pointer.
> >>>> For some reason that register gets clobbered on the Banana Pi.  Instead
> >>>> this version just migrates SP_svc to SP_hyp.
> >>>>
> >>>> This third version avoids saving r3 on the stack and fixes an include
> >>>> guard as suggested by Alexander Graf.
> >>>>
> >>>> Mark Kettenis (3):
> >>>>   ARM: HYP/non-sec: migrate 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 |  2 ++
> >>>>  cmd/bootefi.c                    | 32 ++++++++++++++++++++++++++++++++
> >>>>  doc/README.uefi                  |  2 --
> >>>>  lib/efi_loader/Kconfig           |  2 --
> >>>>  4 files changed, 34 insertions(+), 4 deletions(-)
> >>>>
> >>> Hello Mark,
> >>>
> >>> with this patch series running bootefi hello twice in sequence fails on
> >>> the 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!
> >>> Hello, world!
> >>> Running on UEFI 2.7
> >>> Have SMBIOS table
> >>> Load options: earlyprintk nosmp
> >>> ## Application terminated, r = 0
> >>> => bootefi hello
> >>> WARNING: booting without device tree
> >>> ## Starting EFI application at 42000000 ...
> >>> WARNING: using memory device/image path, this may confuse some payloads!
> >>> <!-- no output after the preceding line -->
> >>
> >> Yeah.  Trying to enter non-secure mode when we're already in
> >> non-secure mode doesn't really work.  We should skip the switching
> >> code in that case.  Now checking whether we are in non-secure mode
> >> isn't really possible.  But I guess we can set a variable and check it
> >> before we go down the switching codepath.  With that in I can exit the
> >> OpenBSD bootloader and then reload and run it again.  I'll include
> >> that fix in the next respin.
> > 
> > Hello Mark,
> > 
> > you might move the call to switch to non-secure mode to efi_init_obj_list().
> 
> I would actually prefer to keep it where it is. That way we have the
> option to move the object initialization to a different stage later on.

Also I'd rather not touch the aarch64 code in this series and I think
it makes sense to keep the switching in the same place for aarch32 and
aarch64.

> The only thing missing is really a check which level we're on. The
> aarch64 code does this with a current_el() == 3 condition.

As I replied to Heinrich, checking whether we're secure or not isn't
simple as reading the SCR.NS bit will trap if we're in non-secure
mode.  But using a global variable to remember the state we're in,
works and isn't too ugly.

I'm going to look into enabling the MMU for HYP over the weekend.
I'll do another respin once I've figured that issue out.

^ permalink raw reply	[flat|nested] 27+ messages in thread

* [U-Boot] [PATCH v3 0/3] efi_loader: ARM: add support for ARMV7_NONSEC=y
  2018-06-15 13:01         ` Mark Kettenis
@ 2018-06-15 13:12           ` Alexander Graf
  2018-06-15 13:31             ` Mark Kettenis
  0 siblings, 1 reply; 27+ messages in thread
From: Alexander Graf @ 2018-06-15 13:12 UTC (permalink / raw)
  To: u-boot



Am 15.06.2018 um 15:01 schrieb Mark Kettenis <mark.kettenis@xs4all.nl>:

>> From: Alexander Graf <agraf@suse.de>
>> Date: Fri, 15 Jun 2018 12:49:48 +0200
>> 
>>> On 15.06.18 05:39, Heinrich Schuchardt wrote:
>>> On 06/14/2018 10:50 PM, Mark Kettenis wrote:
>>>>> From: Heinrich Schuchardt <xypron.glpk@gmx.de>
>>>>> Date: Thu, 14 Jun 2018 19:55:51 +0200
>>>>> 
>>>>>> On 06/14/2018 12:41 AM, Mark Kettenis wrote:
>>>>>> This series makes it possible to run EFI applications in non-secure
>>>>>> mode.  It allows me to run OpenBSD on the Technexion PICO-PI-IMX7 and
>>>>>> Banana Pi boards using the PSCI implementation provided by U-Boot.
>>>>>> 
>>>>>> The second version avoids using r3 to pass the original stack pointer.
>>>>>> For some reason that register gets clobbered on the Banana Pi.  Instead
>>>>>> this version just migrates SP_svc to SP_hyp.
>>>>>> 
>>>>>> This third version avoids saving r3 on the stack and fixes an include
>>>>>> guard as suggested by Alexander Graf.
>>>>>> 
>>>>>> Mark Kettenis (3):
>>>>>>  ARM: HYP/non-sec: migrate 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 |  2 ++
>>>>>> cmd/bootefi.c                    | 32 ++++++++++++++++++++++++++++++++
>>>>>> doc/README.uefi                  |  2 --
>>>>>> lib/efi_loader/Kconfig           |  2 --
>>>>>> 4 files changed, 34 insertions(+), 4 deletions(-)
>>>>>> 
>>>>> Hello Mark,
>>>>> 
>>>>> with this patch series running bootefi hello twice in sequence fails on
>>>>> the 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!
>>>>> Hello, world!
>>>>> Running on UEFI 2.7
>>>>> Have SMBIOS table
>>>>> Load options: earlyprintk nosmp
>>>>> ## Application terminated, r = 0
>>>>> => bootefi hello
>>>>> WARNING: booting without device tree
>>>>> ## Starting EFI application at 42000000 ...
>>>>> WARNING: using memory device/image path, this may confuse some payloads!
>>>>> <!-- no output after the preceding line -->
>>>> 
>>>> Yeah.  Trying to enter non-secure mode when we're already in
>>>> non-secure mode doesn't really work.  We should skip the switching
>>>> code in that case.  Now checking whether we are in non-secure mode
>>>> isn't really possible.  But I guess we can set a variable and check it
>>>> before we go down the switching codepath.  With that in I can exit the
>>>> OpenBSD bootloader and then reload and run it again.  I'll include
>>>> that fix in the next respin.
>>> 
>>> Hello Mark,
>>> 
>>> you might move the call to switch to non-secure mode to efi_init_obj_list().
>> 
>> I would actually prefer to keep it where it is. That way we have the
>> option to move the object initialization to a different stage later on.
> 
> Also I'd rather not touch the aarch64 code in this series and I think
> it makes sense to keep the switching in the same place for aarch32 and
> aarch64.
> 
>> The only thing missing is really a check which level we're on. The
>> aarch64 code does this with a current_el() == 3 condition.
> 
> As I replied to Heinrich, checking whether we're secure or not isn't
> simple as reading the SCR.NS bit will trap if we're in non-secure
> mode.  But using a global variable to remember the state we're in,
> works and isn't too ugly.

Can you figure out something else? Like whether you're in SVC?

> I'm going to look into enabling the MMU for HYP over the weekend.
> I'll do another respin once I've figured that issue out.

It might just be as simple as depending on LPAE in Kconfig when NS is set :)

Alex

^ permalink raw reply	[flat|nested] 27+ messages in thread

* [U-Boot] [PATCH v3 2/3] efi_loader: ARM: run EFI payloads non-secure
  2018-06-15 12:51         ` Mark Kettenis
@ 2018-06-15 13:20           ` Marc Zyngier
  0 siblings, 0 replies; 27+ messages in thread
From: Marc Zyngier @ 2018-06-15 13:20 UTC (permalink / raw)
  To: u-boot

On 15/06/18 13:51, Mark Kettenis wrote:
>> From: Marc Zyngier <marc.zyngier@arm.com>
>> Date: Fri, 15 Jun 2018 08:59:59 +0100
>>
>> On 14/06/18 21:55, Mark Kettenis wrote:
>>>> From: Marc Zyngier <marc.zyngier@arm.com>
>>>> Date: Thu, 14 Jun 2018 12:54:53 +0100
>>>>
>>>> On 13/06/18 23:41, 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..12a6b84ce6 100644
>>>>> --- a/cmd/bootefi.c
>>>>> +++ b/cmd/bootefi.c
>>>>> @@ -20,6 +20,11 @@
>>>>>  #include <asm-generic/unaligned.h>
>>>>>  #include <linux/linkage.h>
>>>>>  
>>>>> +#ifdef CONFIG_ARMV7_NONSEC
>>>>> +#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 */
>>>>> +
>>>>
>>>> What is the rational for disabling/enabling caches across the transition
>>>> to HYP? I'm sure there is a good reason, but I'd rather see it explained
>>>> here.
>>>
>>> Can't say I fully understan why.  But the AArch64 code does this as
>>> well and if I don't flush the cache here the contents of efi_gd (which
>>> gets initialized before the switch) sometimes gets lost.
>>
>> I guess the following can happen:
>>
>> - EL1 code (or SVC for AArch32) has its MMU enabled, and caches are on
>> - Writes from EL1 are nicely sitting in the dcache
>> - Enter EL2 (HYP) where the MMU is off, and thus the caches are too
>> - The uncached accesses do not hit in the cache, and sh*t happens
>>
>> dcache_disable also cleans to the PoC, making sure that everything is
>> visible even when the MMU and caches are off. I have the strong feeling
>> that dcache_enable is utterly useless as I don't think you install any
>> page table at HYP (that code was never designed to run anything other
>> than jumping into the kernel).
> 
> There actually is code in arch/arm/lib/cache-cp15.c to set up the page
> tables for HYP and enable the MMU.  And it would run as part of the
> dcache_enable() call if CONFIG_ARMV7_LPAE was defined.  But that isn't
> set on the boards I'm looking at.  I'll have a go at enabling that option.

Yeah, you most definitely want to have that one, and LPAE is the only
thing that makes sense if you have a virtualization-capable CPU.

> 
>> It would make a lot more sense if you installed id-mapped page tables at
>> HYP too in order to enable the caches, and geta  bit of performance back
>> (otherwise anything you run at HYP will negatively compare to the speed
>> of an anaemic snail stuck on sand).
> 
> On the Allwinner A20 it certainly does crawl; console output is really
> slow.  Didn't notice it on the NXP i.MX7D board though.  Anyway,
> thanks for the hint!

No worries.

	M.
-- 
Jazz is not dead. It just smells funny...

^ permalink raw reply	[flat|nested] 27+ messages in thread

* [U-Boot] [PATCH v3 0/3] efi_loader: ARM: add support for ARMV7_NONSEC=y
  2018-06-15 13:12           ` Alexander Graf
@ 2018-06-15 13:31             ` Mark Kettenis
  0 siblings, 0 replies; 27+ messages in thread
From: Mark Kettenis @ 2018-06-15 13:31 UTC (permalink / raw)
  To: u-boot

> From: Alexander Graf <agraf@suse.de>
> Date: Fri, 15 Jun 2018 15:12:31 +0200
> 
> Am 15.06.2018 um 15:01 schrieb Mark Kettenis <mark.kettenis@xs4all.nl>:
> 
> >> From: Alexander Graf <agraf@suse.de>
> >> Date: Fri, 15 Jun 2018 12:49:48 +0200
> >> 
> >>> On 15.06.18 05:39, Heinrich Schuchardt wrote:
> >>> On 06/14/2018 10:50 PM, Mark Kettenis wrote:
> >>>>> From: Heinrich Schuchardt <xypron.glpk@gmx.de>
> >>>>> Date: Thu, 14 Jun 2018 19:55:51 +0200
> >>>>> 
> >>>>>> On 06/14/2018 12:41 AM, Mark Kettenis wrote:
> >>>>>> This series makes it possible to run EFI applications in non-secure
> >>>>>> mode.  It allows me to run OpenBSD on the Technexion PICO-PI-IMX7 and
> >>>>>> Banana Pi boards using the PSCI implementation provided by U-Boot.
> >>>>>> 
> >>>>>> The second version avoids using r3 to pass the original stack pointer.
> >>>>>> For some reason that register gets clobbered on the Banana Pi.  Instead
> >>>>>> this version just migrates SP_svc to SP_hyp.
> >>>>>> 
> >>>>>> This third version avoids saving r3 on the stack and fixes an include
> >>>>>> guard as suggested by Alexander Graf.
> >>>>>> 
> >>>>>> Mark Kettenis (3):
> >>>>>>  ARM: HYP/non-sec: migrate 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 |  2 ++
> >>>>>> cmd/bootefi.c                    | 32 ++++++++++++++++++++++++++++++++
> >>>>>> doc/README.uefi                  |  2 --
> >>>>>> lib/efi_loader/Kconfig           |  2 --
> >>>>>> 4 files changed, 34 insertions(+), 4 deletions(-)
> >>>>>> 
> >>>>> Hello Mark,
> >>>>> 
> >>>>> with this patch series running bootefi hello twice in sequence fails on
> >>>>> the 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!
> >>>>> Hello, world!
> >>>>> Running on UEFI 2.7
> >>>>> Have SMBIOS table
> >>>>> Load options: earlyprintk nosmp
> >>>>> ## Application terminated, r = 0
> >>>>> => bootefi hello
> >>>>> WARNING: booting without device tree
> >>>>> ## Starting EFI application at 42000000 ...
> >>>>> WARNING: using memory device/image path, this may confuse some payloads!
> >>>>> <!-- no output after the preceding line -->
> >>>> 
> >>>> Yeah.  Trying to enter non-secure mode when we're already in
> >>>> non-secure mode doesn't really work.  We should skip the switching
> >>>> code in that case.  Now checking whether we are in non-secure mode
> >>>> isn't really possible.  But I guess we can set a variable and check it
> >>>> before we go down the switching codepath.  With that in I can exit the
> >>>> OpenBSD bootloader and then reload and run it again.  I'll include
> >>>> that fix in the next respin.
> >>> 
> >>> Hello Mark,
> >>> 
> >>> you might move the call to switch to non-secure mode to efi_init_obj_list().
> >> 
> >> I would actually prefer to keep it where it is. That way we have the
> >> option to move the object initialization to a different stage later on.
> > 
> > Also I'd rather not touch the aarch64 code in this series and I think
> > it makes sense to keep the switching in the same place for aarch32 and
> > aarch64.
> > 
> >> The only thing missing is really a check which level we're on. The
> >> aarch64 code does this with a current_el() == 3 condition.
> > 
> > As I replied to Heinrich, checking whether we're secure or not isn't
> > simple as reading the SCR.NS bit will trap if we're in non-secure
> > mode.  But using a global variable to remember the state we're in,
> > works and isn't too ugly.
> 
> Can you figure out something else? Like whether you're in SVC?

Unfortunately not.  On hardware with security extensions, but without
virtualization extensions, we'll start out in secure SVC before
running the first EFI payload and in non-secure SVC afterwards.

> > I'm going to look into enabling the MMU for HYP over the weekend.
> > I'll do another respin once I've figured that issue out.
> 
> It might just be as simple as depending on LPAE in Kconfig when NS is set :)

Indeed.

^ permalink raw reply	[flat|nested] 27+ messages in thread

* [U-Boot] [PATCH v3 0/3] efi_loader: ARM: add support for ARMV7_NONSEC=y
  2018-06-14 20:50   ` Mark Kettenis
  2018-06-15  3:39     ` Heinrich Schuchardt
@ 2018-06-15 20:35     ` Heinrich Schuchardt
  2018-06-15 22:03       ` Mark Kettenis
  1 sibling, 1 reply; 27+ messages in thread
From: Heinrich Schuchardt @ 2018-06-15 20:35 UTC (permalink / raw)
  To: u-boot

On 06/14/2018 10:50 PM, Mark Kettenis wrote:
>> From: Heinrich Schuchardt <xypron.glpk@gmx.de>
>> Hello Mark,
>>
>> with this patch series running bootefi hello twice in sequence fails on
>> the 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!
>> Hello, world!
>> Running on UEFI 2.7
>> Have SMBIOS table
>> Load options: earlyprintk nosmp
>> ## Application terminated, r = 0
>> => bootefi hello
>> WARNING: booting without device tree
>> ## Starting EFI application at 42000000 ...
>> WARNING: using memory device/image path, this may confuse some payloads!
>> <!-- no output after the preceding line -->
> 
> Yeah.  Trying to enter non-secure mode when we're already in
> non-secure mode doesn't really work.  We should skip the switching
> code in that case.  Now checking whether we are in non-secure mode
> isn't really possible.  But I guess we can set a variable and check it
> before we go down the switching codepath.  With that in I can exit the
> OpenBSD bootloader and then reload and run it again.  I'll include
> that fix in the next respin.
> 
>> Please, keep in mind that we expect multiple EFI binaries to be executed
>> in sequence. E.g. the first binary installs a driver. The second is the
>> application using it.
>>
>> Running iPXE's snp.efi binary shows changed behavior on the console. New
>> characters are displayed in "slow motion" (3 characters per second).
>> Setting up the network interface fails in iPXE.
> 
> The same happens on my Banana Pi.  But not on the imx7 board.  
> 

Just for reference run the Banana Pi with only patch 3 (w/o patch 1 and
2) and you will see adequate speed in iPXE. And if you specify

   setenv bootargs nosmp

you will be able to boot Linux via GRUB from your iSCSI drive. So this
is not a deficiency of the board.

Regards

Heinrich

^ permalink raw reply	[flat|nested] 27+ messages in thread

* [U-Boot] [PATCH v3 0/3] efi_loader: ARM: add support for ARMV7_NONSEC=y
  2018-06-15 20:35     ` Heinrich Schuchardt
@ 2018-06-15 22:03       ` Mark Kettenis
  0 siblings, 0 replies; 27+ messages in thread
From: Mark Kettenis @ 2018-06-15 22:03 UTC (permalink / raw)
  To: u-boot

> From: Heinrich Schuchardt <xypron.glpk@gmx.de>
> Date: Fri, 15 Jun 2018 22:35:58 +0200
> 
> On 06/14/2018 10:50 PM, Mark Kettenis wrote:
> >> From: Heinrich Schuchardt <xypron.glpk@gmx.de>
> >> Hello Mark,
> >>
> >> with this patch series running bootefi hello twice in sequence fails on
> >> the 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!
> >> Hello, world!
> >> Running on UEFI 2.7
> >> Have SMBIOS table
> >> Load options: earlyprintk nosmp
> >> ## Application terminated, r = 0
> >> => bootefi hello
> >> WARNING: booting without device tree
> >> ## Starting EFI application at 42000000 ...
> >> WARNING: using memory device/image path, this may confuse some payloads!
> >> <!-- no output after the preceding line -->
> > 
> > Yeah.  Trying to enter non-secure mode when we're already in
> > non-secure mode doesn't really work.  We should skip the switching
> > code in that case.  Now checking whether we are in non-secure mode
> > isn't really possible.  But I guess we can set a variable and check it
> > before we go down the switching codepath.  With that in I can exit the
> > OpenBSD bootloader and then reload and run it again.  I'll include
> > that fix in the next respin.
> > 
> >> Please, keep in mind that we expect multiple EFI binaries to be executed
> >> in sequence. E.g. the first binary installs a driver. The second is the
> >> application using it.
> >>
> >> Running iPXE's snp.efi binary shows changed behavior on the console. New
> >> characters are displayed in "slow motion" (3 characters per second).
> >> Setting up the network interface fails in iPXE.
> > 
> > The same happens on my Banana Pi.  But not on the imx7 board.  
> > 
> 
> Just for reference run the Banana Pi with only patch 3 (w/o patch 1 and
> 2) and you will see adequate speed in iPXE. And if you specify
> 
>    setenv bootargs nosmp
> 
> you will be able to boot Linux via GRUB from your iSCSI drive. So this
> is not a deficiency of the board.

The update series I just posted makes things fast again. So far I
haven't been able to build an iPXE EFI binary so that remains
untested.  Would be great if you could do that for me.

But even if iPXE still doesn't work, I'd argue that efi_loader support
for armv7 that works in the most common case of booting an OS using an
EFI bootloader is better than having no efi_loader support at all...

Cheers,

Mark

^ permalink raw reply	[flat|nested] 27+ messages in thread

* [U-Boot] [PATCH v3 2/3] efi_loader: ARM: run EFI payloads non-secure
  2018-06-13 22:41 ` [U-Boot] [PATCH v3 2/3] efi_loader: ARM: run EFI payloads non-secure Mark Kettenis
  2018-06-14 11:54   ` Marc Zyngier
@ 2018-08-31 17:37   ` Heinrich Schuchardt
  2018-08-31 18:45     ` Mark Kettenis
  1 sibling, 1 reply; 27+ messages in thread
From: Heinrich Schuchardt @ 2018-08-31 17:37 UTC (permalink / raw)
  To: u-boot

On 06/14/2018 12:41 AM, 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>

bootefi hello fails on vexpress_ca15_tc2_defconfig when run on qemu with

        QEMU_AUDIO_DRV=none qemu-system-arm \
        -M vexpress-a15 -cpu cortex-a15 -kernel u-boot \
        -net user -net nic,model=lan9118 \
        -m 1024M --nographic \
        -drive if=sd,file=img.vexpress,media=disk,format=raw

Bisection points to
efi_loader: ARM: run EFI payloads non-secure
commit dc500c369486fbe04000fd325c46bb309e4a1827

Best regards

Heinrich Schuchardt

^ permalink raw reply	[flat|nested] 27+ messages in thread

* [U-Boot] [PATCH v3 2/3] efi_loader: ARM: run EFI payloads non-secure
  2018-08-31 17:37   ` Heinrich Schuchardt
@ 2018-08-31 18:45     ` Mark Kettenis
  2018-09-01 10:21       ` Alexander Graf
  0 siblings, 1 reply; 27+ messages in thread
From: Mark Kettenis @ 2018-08-31 18:45 UTC (permalink / raw)
  To: u-boot

> From: Heinrich Schuchardt <xypron.glpk@gmx.de>
> Date: Fri, 31 Aug 2018 19:37:25 +0200
> 
> On 06/14/2018 12:41 AM, 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>
> 
> bootefi hello fails on vexpress_ca15_tc2_defconfig when run on qemu with
> 
>         QEMU_AUDIO_DRV=none qemu-system-arm \
>         -M vexpress-a15 -cpu cortex-a15 -kernel u-boot \
>         -net user -net nic,model=lan9118 \
>         -m 1024M --nographic \
>         -drive if=sd,file=img.vexpress,media=disk,format=raw

Works for me with:

$ qemu-system-arm --version
QEMU emulator version 3.0.0
Copyright (c) 2003-2017 Fabrice Bellard and the QEMU Project developers

=> bootefi hello
Scanning disks on mmc...
MMC Device 1 not found
MMC Device 2 not found
MMC Device 3 not found
Found 3 disks
WARNING: booting without device tree
## Starting EFI application at a0008000 ...
WARNING: using memory device/image path, this may confuse some payloads!
Hello, world!
Running on UEFI 2.7
Have SMBIOS table
Load options: <none>
## Application terminated, r = 0

That is with CONFIG_CMD_BOOTEFI_HELLO=y added to the
vexpress_ca15_tc2_defconfig of course.

> Bisection points to
> efi_loader: ARM: run EFI payloads non-secure
> commit dc500c369486fbe04000fd325c46bb309e4a1827

That suggests an issue with emulation if the mode switching
instructions or HYP support in qemu.  Or a toolchain issue of course.

Cheers,

Mark

^ permalink raw reply	[flat|nested] 27+ messages in thread

* [U-Boot] [PATCH v3 2/3] efi_loader: ARM: run EFI payloads non-secure
  2018-08-31 18:45     ` Mark Kettenis
@ 2018-09-01 10:21       ` Alexander Graf
  2018-09-22 23:30         ` Heinrich Schuchardt
  0 siblings, 1 reply; 27+ messages in thread
From: Alexander Graf @ 2018-09-01 10:21 UTC (permalink / raw)
  To: u-boot



On 31.08.18 20:45, Mark Kettenis wrote:
>> From: Heinrich Schuchardt <xypron.glpk@gmx.de>
>> Date: Fri, 31 Aug 2018 19:37:25 +0200
>>
>> On 06/14/2018 12:41 AM, 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>
>>
>> bootefi hello fails on vexpress_ca15_tc2_defconfig when run on qemu with
>>
>>         QEMU_AUDIO_DRV=none qemu-system-arm \
>>         -M vexpress-a15 -cpu cortex-a15 -kernel u-boot \
>>         -net user -net nic,model=lan9118 \
>>         -m 1024M --nographic \
>>         -drive if=sd,file=img.vexpress,media=disk,format=raw
> 
> Works for me with:
> 
> $ qemu-system-arm --version
> QEMU emulator version 3.0.0
> Copyright (c) 2003-2017 Fabrice Bellard and the QEMU Project developers
> 
> => bootefi hello
> Scanning disks on mmc...
> MMC Device 1 not found
> MMC Device 2 not found
> MMC Device 3 not found
> Found 3 disks
> WARNING: booting without device tree
> ## Starting EFI application at a0008000 ...
> WARNING: using memory device/image path, this may confuse some payloads!
> Hello, world!
> Running on UEFI 2.7
> Have SMBIOS table
> Load options: <none>
> ## Application terminated, r = 0
> 
> That is with CONFIG_CMD_BOOTEFI_HELLO=y added to the
> vexpress_ca15_tc2_defconfig of course.
> 
>> Bisection points to
>> efi_loader: ARM: run EFI payloads non-secure
>> commit dc500c369486fbe04000fd325c46bb309e4a1827
> 
> That suggests an issue with emulation if the mode switching
> instructions or HYP support in qemu.  Or a toolchain issue of course.

Or maybe Heinrich's QEMU version starts up in a different EL mode?


Alex

^ permalink raw reply	[flat|nested] 27+ messages in thread

* [U-Boot] [PATCH v3 2/3] efi_loader: ARM: run EFI payloads non-secure
  2018-09-01 10:21       ` Alexander Graf
@ 2018-09-22 23:30         ` Heinrich Schuchardt
  2018-09-23  2:39           ` Jonathan Gray
  0 siblings, 1 reply; 27+ messages in thread
From: Heinrich Schuchardt @ 2018-09-22 23:30 UTC (permalink / raw)
  To: u-boot

On 09/01/2018 12:21 PM, Alexander Graf wrote:
> 
> 
> On 31.08.18 20:45, Mark Kettenis wrote:
>>> From: Heinrich Schuchardt <xypron.glpk@gmx.de>
>>> Date: Fri, 31 Aug 2018 19:37:25 +0200
>>>
>>> On 06/14/2018 12:41 AM, 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>
>>>
>>> bootefi hello fails on vexpress_ca15_tc2_defconfig when run on qemu with
>>>
>>>         QEMU_AUDIO_DRV=none qemu-system-arm \
>>>         -M vexpress-a15 -cpu cortex-a15 -kernel u-boot \
>>>         -net user -net nic,model=lan9118 \
>>>         -m 1024M --nographic \
>>>         -drive if=sd,file=img.vexpress,media=disk,format=raw
>>
>> Works for me with:
>>
>> $ qemu-system-arm --version
>> QEMU emulator version 3.0.0
>> Copyright (c) 2003-2017 Fabrice Bellard and the QEMU Project developers
>>
>> => bootefi hello
>> Scanning disks on mmc...
>> MMC Device 1 not found
>> MMC Device 2 not found
>> MMC Device 3 not found
>> Found 3 disks
>> WARNING: booting without device tree
>> ## Starting EFI application at a0008000 ...
>> WARNING: using memory device/image path, this may confuse some payloads!
>> Hello, world!
>> Running on UEFI 2.7
>> Have SMBIOS table
>> Load options: <none>
>> ## Application terminated, r = 0
>>
>> That is with CONFIG_CMD_BOOTEFI_HELLO=y added to the
>> vexpress_ca15_tc2_defconfig of course.
>>
>>> Bisection points to
>>> efi_loader: ARM: run EFI payloads non-secure
>>> commit dc500c369486fbe04000fd325c46bb309e4a1827
>>
>> That suggests an issue with emulation if the mode switching
>> instructions or HYP support in qemu.  Or a toolchain issue of course.
> 
> Or maybe Heinrich's QEMU version starts up in a different EL mode?
> 
> 
> Alex
> _______________________________________________
> U-Boot mailing list
> U-Boot at lists.denx.de
> https://lists.denx.de/listinfo/u-boot
> 
The problem is reproducible with qemu 3.0.0.

git reset --hard dc500c369486fbe04000fd325c46bb309e4a1827

--- a/lib/efi_loader/Kconfig
+++ b/lib/efi_loader/Kconfig
@@ -2,7 +2,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

make vexpress_ca15_tc2_defconfig

Set CONFIG_CMD_BOOTEFI_HELLO=y

export CROSS_COMPILE=arm-linux-gnueabihf-
make

QEMU_AUDIO_DRV=none qemu-system-arm \
-M vexpress-a15 -cpu cortex-a15 -kernel u-boot \
-netdev \
user,id=net0,tftp=tftp,net=192.168.76.0/24,dhcpstart=192.168.76.9 \
-net nic,model=lan9118,netdev=net0 \
-m 1024M --nographic \
-drive if=sd,file=img.vexpress,media=disk,format=raw

=> bootefi hello
Scanning disks on mmc...
MMC Device 1 not found
MMC Device 2 not found
MMC Device 3 not found
Found 2 disks
WARNING: booting without device tree
## Starting EFI application at a0008000 ...
WARNING: using memory device/image path, this may confuse some payloads!

No further output after this line :(

Best regards

Heinrich

^ permalink raw reply	[flat|nested] 27+ messages in thread

* [U-Boot] [PATCH v3 2/3] efi_loader: ARM: run EFI payloads non-secure
  2018-09-22 23:30         ` Heinrich Schuchardt
@ 2018-09-23  2:39           ` Jonathan Gray
  2018-09-23  3:38             ` Heinrich Schuchardt
  0 siblings, 1 reply; 27+ messages in thread
From: Jonathan Gray @ 2018-09-23  2:39 UTC (permalink / raw)
  To: u-boot

On Sun, Sep 23, 2018 at 01:30:11AM +0200, Heinrich Schuchardt wrote:
> On 09/01/2018 12:21 PM, Alexander Graf wrote:
> > 
> > 
> > On 31.08.18 20:45, Mark Kettenis wrote:
> >>> From: Heinrich Schuchardt <xypron.glpk@gmx.de>
> >>> Date: Fri, 31 Aug 2018 19:37:25 +0200
> >>>
> >>> On 06/14/2018 12:41 AM, 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>
> >>>
> >>> bootefi hello fails on vexpress_ca15_tc2_defconfig when run on qemu with
> >>>
> >>>         QEMU_AUDIO_DRV=none qemu-system-arm \
> >>>         -M vexpress-a15 -cpu cortex-a15 -kernel u-boot \
> >>>         -net user -net nic,model=lan9118 \
> >>>         -m 1024M --nographic \
> >>>         -drive if=sd,file=img.vexpress,media=disk,format=raw
> >>
> >> Works for me with:
> >>
> >> $ qemu-system-arm --version
> >> QEMU emulator version 3.0.0
> >> Copyright (c) 2003-2017 Fabrice Bellard and the QEMU Project developers
> >>
> >> => bootefi hello
> >> Scanning disks on mmc...
> >> MMC Device 1 not found
> >> MMC Device 2 not found
> >> MMC Device 3 not found
> >> Found 3 disks
> >> WARNING: booting without device tree
> >> ## Starting EFI application at a0008000 ...
> >> WARNING: using memory device/image path, this may confuse some payloads!
> >> Hello, world!
> >> Running on UEFI 2.7
> >> Have SMBIOS table
> >> Load options: <none>
> >> ## Application terminated, r = 0
> >>
> >> That is with CONFIG_CMD_BOOTEFI_HELLO=y added to the
> >> vexpress_ca15_tc2_defconfig of course.
> >>
> >>> Bisection points to
> >>> efi_loader: ARM: run EFI payloads non-secure
> >>> commit dc500c369486fbe04000fd325c46bb309e4a1827
> >>
> >> That suggests an issue with emulation if the mode switching
> >> instructions or HYP support in qemu.  Or a toolchain issue of course.
> > 
> > Or maybe Heinrich's QEMU version starts up in a different EL mode?
> > 
> > 
> > Alex
> > _______________________________________________
> > U-Boot mailing list
> > U-Boot at lists.denx.de
> > https://lists.denx.de/listinfo/u-boot
> > 
> The problem is reproducible with qemu 3.0.0.
> 
> git reset --hard dc500c369486fbe04000fd325c46bb309e4a1827
> 
> --- a/lib/efi_loader/Kconfig
> +++ b/lib/efi_loader/Kconfig
> @@ -2,7 +2,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
> 
> make vexpress_ca15_tc2_defconfig
> 
> Set CONFIG_CMD_BOOTEFI_HELLO=y
> 
> export CROSS_COMPILE=arm-linux-gnueabihf-
> make
> 
> QEMU_AUDIO_DRV=none qemu-system-arm \
> -M vexpress-a15 -cpu cortex-a15 -kernel u-boot \
> -netdev \
> user,id=net0,tftp=tftp,net=192.168.76.0/24,dhcpstart=192.168.76.9 \
> -net nic,model=lan9118,netdev=net0 \
> -m 1024M --nographic \
> -drive if=sd,file=img.vexpress,media=disk,format=raw
> 
> => bootefi hello
> Scanning disks on mmc...
> MMC Device 1 not found
> MMC Device 2 not found
> MMC Device 3 not found
> Found 2 disks
> WARNING: booting without device tree
> ## Starting EFI application at a0008000 ...
> WARNING: using memory device/image path, this may confuse some payloads!
> 
> No further output after this line :(

bootefi hello works fine with 2018.09 and qemu 3.0 here

gmake PYTHON=python2.7 CROSS_COMPILE=arm-none-eabi- vexpress_ca15_tc2_defconfig O=build/vexpress
gmake PYTHON=python2.7 CROSS_COMPILE=arm-none-eabi- O=build/vexpress menuconfig
gmake PYTHON=python2.7 CROSS_COMPILE=arm-none-eabi- O=build/vexpress all

$ arm-none-eabi-gcc -v
Using built-in specs.
COLLECT_GCC=arm-none-eabi-gcc
COLLECT_LTO_WRAPPER=/usr/local/libexec/gcc/arm-none-eabi/6.3.1/lto-wrapper
Target: arm-none-eabi
Configured with: /usr/obj/ports/arm-none-eabi-gcc-linaro-6.3.2017.02-arm/gcc-linaro-6.3-2017.02/configure --enable-languages=c,c++ --enable-multilib --enable-interwork --with-gmp=/usr/local --with-newlib --disable-lto --enable-cpp --target=arm-none-eabi --disable-shared --disable-nls --disable-werror --prefix=/usr/local --sysconfdir=/etc --mandir=/usr/local/man --infodir=/usr/local/info --localstatedir=/var --disable-silent-rules --disable-gtk-doc
Thread model: single
gcc version 6.3.1 20170109 (Linaro GCC 6.3-2017.02)

$ qemu-system-arm --version
QEMU emulator version 3.0.0
Copyright (c) 2003-2017 Fabrice Bellard and the QEMU Project developers

qemu-system-arm -m 1024 -M vexpress-a15 -kernel build/vexpress/u-boot -serial stdio

U-Boot 2018.09-00001-g2a9fbd55c3 (Sep 23 2018 - 12:24:33 +1000)

DRAM:  1 GiB
WARNING: Caches not enabled
Flash: 128 MiB
MMC:   MMC: 0
*** Warning - bad CRC, using default environment

In:    serial
Out:   serial
Err:   serial
Net:   smc911x-0
Hit any key to stop autoboot:  0
=> bootefi hello
Scanning disks on mmc...
Card did not respond to voltage select!
MMC Device 1 not found
MMC Device 2 not found
MMC Device 3 not found
Found 0 disks
WARNING: booting without device tree
## Starting EFI application at a0008000 ...
WARNING: using memory device/image path, this may confuse some payloads!
Hello, world!
Running on UEFI 2.7
Have SMBIOS table
Load options: <none>
## Application terminated, r = 0
=> 

^ permalink raw reply	[flat|nested] 27+ messages in thread

* [U-Boot] [PATCH v3 2/3] efi_loader: ARM: run EFI payloads non-secure
  2018-09-23  2:39           ` Jonathan Gray
@ 2018-09-23  3:38             ` Heinrich Schuchardt
  2018-09-23 18:22               ` Mark Kettenis
  0 siblings, 1 reply; 27+ messages in thread
From: Heinrich Schuchardt @ 2018-09-23  3:38 UTC (permalink / raw)
  To: u-boot

On 09/23/2018 04:39 AM, Jonathan Gray wrote:
> On Sun, Sep 23, 2018 at 01:30:11AM +0200, Heinrich Schuchardt wrote:
>> On 09/01/2018 12:21 PM, Alexander Graf wrote:
>>>
>>>
>>> On 31.08.18 20:45, Mark Kettenis wrote:
>>>>> From: Heinrich Schuchardt <xypron.glpk@gmx.de>
>>>>> Date: Fri, 31 Aug 2018 19:37:25 +0200
>>>>>
>>>>> On 06/14/2018 12:41 AM, 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>
>>>>>
>>>>> bootefi hello fails on vexpress_ca15_tc2_defconfig when run on qemu with
>>>>>
>>>>>         QEMU_AUDIO_DRV=none qemu-system-arm \
>>>>>         -M vexpress-a15 -cpu cortex-a15 -kernel u-boot \
>>>>>         -net user -net nic,model=lan9118 \
>>>>>         -m 1024M --nographic \
>>>>>         -drive if=sd,file=img.vexpress,media=disk,format=raw
>>>>
>>>> Works for me with:
>>>>
>>>> $ qemu-system-arm --version
>>>> QEMU emulator version 3.0.0
>>>> Copyright (c) 2003-2017 Fabrice Bellard and the QEMU Project developers
>>>>
>>>> => bootefi hello
>>>> Scanning disks on mmc...
>>>> MMC Device 1 not found
>>>> MMC Device 2 not found
>>>> MMC Device 3 not found
>>>> Found 3 disks
>>>> WARNING: booting without device tree
>>>> ## Starting EFI application at a0008000 ...
>>>> WARNING: using memory device/image path, this may confuse some payloads!
>>>> Hello, world!
>>>> Running on UEFI 2.7
>>>> Have SMBIOS table
>>>> Load options: <none>
>>>> ## Application terminated, r = 0
>>>>
>>>> That is with CONFIG_CMD_BOOTEFI_HELLO=y added to the
>>>> vexpress_ca15_tc2_defconfig of course.
>>>>
>>>>> Bisection points to
>>>>> efi_loader: ARM: run EFI payloads non-secure
>>>>> commit dc500c369486fbe04000fd325c46bb309e4a1827
>>>>
>>>> That suggests an issue with emulation if the mode switching
>>>> instructions or HYP support in qemu.  Or a toolchain issue of course.
>>>
>>> Or maybe Heinrich's QEMU version starts up in a different EL mode?
>>>
>>>
>>> Alex
>>> _______________________________________________
>>> U-Boot mailing list
>>> U-Boot at lists.denx.de
>>> https://lists.denx.de/listinfo/u-boot
>>>
>> The problem is reproducible with qemu 3.0.0.
>>
>> git reset --hard dc500c369486fbe04000fd325c46bb309e4a1827
>>
>> --- a/lib/efi_loader/Kconfig
>> +++ b/lib/efi_loader/Kconfig
>> @@ -2,7 +2,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
>>
>> make vexpress_ca15_tc2_defconfig
>>
>> Set CONFIG_CMD_BOOTEFI_HELLO=y
>>
>> export CROSS_COMPILE=arm-linux-gnueabihf-
>> make
>>
>> QEMU_AUDIO_DRV=none qemu-system-arm \
>> -M vexpress-a15 -cpu cortex-a15 -kernel u-boot \
>> -netdev \
>> user,id=net0,tftp=tftp,net=192.168.76.0/24,dhcpstart=192.168.76.9 \
>> -net nic,model=lan9118,netdev=net0 \
>> -m 1024M --nographic \
>> -drive if=sd,file=img.vexpress,media=disk,format=raw
>>
>> => bootefi hello
>> Scanning disks on mmc...
>> MMC Device 1 not found
>> MMC Device 2 not found
>> MMC Device 3 not found
>> Found 2 disks
>> WARNING: booting without device tree
>> ## Starting EFI application at a0008000 ...
>> WARNING: using memory device/image path, this may confuse some payloads!
>>
>> No further output after this line :(
> 
> bootefi hello works fine with 2018.09 and qemu 3.0 here
> 
> gmake PYTHON=python2.7 CROSS_COMPILE=arm-none-eabi- vexpress_ca15_tc2_defconfig O=build/vexpress
> gmake PYTHON=python2.7 CROSS_COMPILE=arm-none-eabi- O=build/vexpress menuconfig
> gmake PYTHON=python2.7 CROSS_COMPILE=arm-none-eabi- O=build/vexpress all
> 
> $ arm-none-eabi-gcc -v
> Using built-in specs.
> COLLECT_GCC=arm-none-eabi-gcc
> COLLECT_LTO_WRAPPER=/usr/local/libexec/gcc/arm-none-eabi/6.3.1/lto-wrapper
> Target: arm-none-eabi
> Configured with: /usr/obj/ports/arm-none-eabi-gcc-linaro-6.3.2017.02-arm/gcc-linaro-6.3-2017.02/configure --enable-languages=c,c++ --enable-multilib --enable-interwork --with-gmp=/usr/local --with-newlib --disable-lto --enable-cpp --target=arm-none-eabi --disable-shared --disable-nls --disable-werror --prefix=/usr/local --sysconfdir=/etc --mandir=/usr/local/man --infodir=/usr/local/info --localstatedir=/var --disable-silent-rules --disable-gtk-doc
> Thread model: single
> gcc version 6.3.1 20170109 (Linaro GCC 6.3-2017.02)
> 
> $ qemu-system-arm --version
> QEMU emulator version 3.0.0
> Copyright (c) 2003-2017 Fabrice Bellard and the QEMU Project developers
> 
> qemu-system-arm -m 1024 -M vexpress-a15 -kernel build/vexpress/u-boot -serial stdio
> 
> U-Boot 2018.09-00001-g2a9fbd55c3 (Sep 23 2018 - 12:24:33 +1000)
> 
> DRAM:  1 GiB
> WARNING: Caches not enabled
> Flash: 128 MiB
> MMC:   MMC: 0
> *** Warning - bad CRC, using default environment
> 
> In:    serial
> Out:   serial
> Err:   serial
> Net:   smc911x-0
> Hit any key to stop autoboot:  0
> => bootefi hellopi
> Scanning disks on mmc...
> Card did not respond to voltage select!
> MMC Device 1 not found
> MMC Device 2 not found
> MMC Device 3 not found
> Found 0 disks
> WARNING: booting without device tree
> ## Starting EFI application at a0008000 ...
> WARNING: using memory device/image path, this may confuse some payloads!
> Hello, world!
> Running on UEFI 2.7
> Have SMBIOS table
> Load options: <none>
> ## Application terminated, r = 0
> => 
> 
The difference seems to be in the compiler:
With the recipe above arm-none-eabi-gcc 6.3.1 is working but
arm-linux-gnueabihf-gcc 8.2.0 (as supplied with Debian Buster) is still
failing.

Errors like out of bound accesses may become apparent or disappearing
depending of generated addresses. So it may be by chance that we do not
see the error with gcc 6.3.1. This does not imply that the code is correct.

Best regards

Heinrich

^ permalink raw reply	[flat|nested] 27+ messages in thread

* [U-Boot] [PATCH v3 2/3] efi_loader: ARM: run EFI payloads non-secure
  2018-09-23  3:38             ` Heinrich Schuchardt
@ 2018-09-23 18:22               ` Mark Kettenis
  0 siblings, 0 replies; 27+ messages in thread
From: Mark Kettenis @ 2018-09-23 18:22 UTC (permalink / raw)
  To: u-boot

> From: Heinrich Schuchardt <xypron.glpk@gmx.de>
> Date: Sun, 23 Sep 2018 05:38:29 +0200
> 
> On 09/23/2018 04:39 AM, Jonathan Gray wrote:
> > On Sun, Sep 23, 2018 at 01:30:11AM +0200, Heinrich Schuchardt wrote:
> >> On 09/01/2018 12:21 PM, Alexander Graf wrote:
> >>>
> >>>
> >>> On 31.08.18 20:45, Mark Kettenis wrote:
> >>>>> From: Heinrich Schuchardt <xypron.glpk@gmx.de>
> >>>>> Date: Fri, 31 Aug 2018 19:37:25 +0200
> >>>>>
> >>>>> On 06/14/2018 12:41 AM, 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>
> >>>>>
> >>>>> bootefi hello fails on vexpress_ca15_tc2_defconfig when run on qemu with
> >>>>>
> >>>>>         QEMU_AUDIO_DRV=none qemu-system-arm \
> >>>>>         -M vexpress-a15 -cpu cortex-a15 -kernel u-boot \
> >>>>>         -net user -net nic,model=lan9118 \
> >>>>>         -m 1024M --nographic \
> >>>>>         -drive if=sd,file=img.vexpress,media=disk,format=raw
> >>>>
> >>>> Works for me with:
> >>>>
> >>>> $ qemu-system-arm --version
> >>>> QEMU emulator version 3.0.0
> >>>> Copyright (c) 2003-2017 Fabrice Bellard and the QEMU Project developers
> >>>>
> >>>> => bootefi hello
> >>>> Scanning disks on mmc...
> >>>> MMC Device 1 not found
> >>>> MMC Device 2 not found
> >>>> MMC Device 3 not found
> >>>> Found 3 disks
> >>>> WARNING: booting without device tree
> >>>> ## Starting EFI application at a0008000 ...
> >>>> WARNING: using memory device/image path, this may confuse some payloads!
> >>>> Hello, world!
> >>>> Running on UEFI 2.7
> >>>> Have SMBIOS table
> >>>> Load options: <none>
> >>>> ## Application terminated, r = 0
> >>>>
> >>>> That is with CONFIG_CMD_BOOTEFI_HELLO=y added to the
> >>>> vexpress_ca15_tc2_defconfig of course.
> >>>>
> >>>>> Bisection points to
> >>>>> efi_loader: ARM: run EFI payloads non-secure
> >>>>> commit dc500c369486fbe04000fd325c46bb309e4a1827
> >>>>
> >>>> That suggests an issue with emulation if the mode switching
> >>>> instructions or HYP support in qemu.  Or a toolchain issue of course.
> >>>
> >>> Or maybe Heinrich's QEMU version starts up in a different EL mode?
> >>>
> >>>
> >>> Alex
> >>> _______________________________________________
> >>> U-Boot mailing list
> >>> U-Boot at lists.denx.de
> >>> https://lists.denx.de/listinfo/u-boot
> >>>
> >> The problem is reproducible with qemu 3.0.0.
> >>
> >> git reset --hard dc500c369486fbe04000fd325c46bb309e4a1827
> >>
> >> --- a/lib/efi_loader/Kconfig
> >> +++ b/lib/efi_loader/Kconfig
> >> @@ -2,7 +2,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
> >>
> >> make vexpress_ca15_tc2_defconfig
> >>
> >> Set CONFIG_CMD_BOOTEFI_HELLO=y
> >>
> >> export CROSS_COMPILE=arm-linux-gnueabihf-
> >> make
> >>
> >> QEMU_AUDIO_DRV=none qemu-system-arm \
> >> -M vexpress-a15 -cpu cortex-a15 -kernel u-boot \
> >> -netdev \
> >> user,id=net0,tftp=tftp,net=192.168.76.0/24,dhcpstart=192.168.76.9 \
> >> -net nic,model=lan9118,netdev=net0 \
> >> -m 1024M --nographic \
> >> -drive if=sd,file=img.vexpress,media=disk,format=raw
> >>
> >> => bootefi hello
> >> Scanning disks on mmc...
> >> MMC Device 1 not found
> >> MMC Device 2 not found
> >> MMC Device 3 not found
> >> Found 2 disks
> >> WARNING: booting without device tree
> >> ## Starting EFI application at a0008000 ...
> >> WARNING: using memory device/image path, this may confuse some payloads!
> >>
> >> No further output after this line :(
> > 
> > bootefi hello works fine with 2018.09 and qemu 3.0 here
> > 
> > gmake PYTHON=python2.7 CROSS_COMPILE=arm-none-eabi- vexpress_ca15_tc2_defconfig O=build/vexpress
> > gmake PYTHON=python2.7 CROSS_COMPILE=arm-none-eabi- O=build/vexpress menuconfig
> > gmake PYTHON=python2.7 CROSS_COMPILE=arm-none-eabi- O=build/vexpress all
> > 
> > $ arm-none-eabi-gcc -v
> > Using built-in specs.
> > COLLECT_GCC=arm-none-eabi-gcc
> > COLLECT_LTO_WRAPPER=/usr/local/libexec/gcc/arm-none-eabi/6.3.1/lto-wrapper
> > Target: arm-none-eabi
> > Configured with: /usr/obj/ports/arm-none-eabi-gcc-linaro-6.3.2017.02-arm/gcc-linaro-6.3-2017.02/configure --enable-languages=c,c++ --enable-multilib --enable-interwork --with-gmp=/usr/local --with-newlib --disable-lto --enable-cpp --target=arm-none-eabi --disable-shared --disable-nls --disable-werror --prefix=/usr/local --sysconfdir=/etc --mandir=/usr/local/man --infodir=/usr/local/info --localstatedir=/var --disable-silent-rules --disable-gtk-doc
> > Thread model: single
> > gcc version 6.3.1 20170109 (Linaro GCC 6.3-2017.02)
> > 
> > $ qemu-system-arm --version
> > QEMU emulator version 3.0.0
> > Copyright (c) 2003-2017 Fabrice Bellard and the QEMU Project developers
> > 
> > qemu-system-arm -m 1024 -M vexpress-a15 -kernel build/vexpress/u-boot -serial stdio
> > 
> > U-Boot 2018.09-00001-g2a9fbd55c3 (Sep 23 2018 - 12:24:33 +1000)
> > 
> > DRAM:  1 GiB
> > WARNING: Caches not enabled
> > Flash: 128 MiB
> > MMC:   MMC: 0
> > *** Warning - bad CRC, using default environment
> > 
> > In:    serial
> > Out:   serial
> > Err:   serial
> > Net:   smc911x-0
> > Hit any key to stop autoboot:  0
> > => bootefi hellopi
> > Scanning disks on mmc...
> > Card did not respond to voltage select!
> > MMC Device 1 not found
> > MMC Device 2 not found
> > MMC Device 3 not found
> > Found 0 disks
> > WARNING: booting without device tree
> > ## Starting EFI application at a0008000 ...
> > WARNING: using memory device/image path, this may confuse some payloads!
> > Hello, world!
> > Running on UEFI 2.7
> > Have SMBIOS table
> > Load options: <none>
> > ## Application terminated, r = 0
> > => 
> > 
> The difference seems to be in the compiler:
> With the recipe above arm-none-eabi-gcc 6.3.1 is working but
> arm-linux-gnueabihf-gcc 8.2.0 (as supplied with Debian Buster) is still
> failing.

Are you sure it's the compiler that causes the breakage and not the
linker?  Apparently there was a recent change in binutils that affects
sectipn alignment.  See commit
792b204798453d96b00e0817e8472c19455e92a2 for a workaround.  It
wouldn't surprise me more tweaks like that are necessary.

> Errors like out of bound accesses may become apparent or disappearing
> depending of generated addresses. So it may be by chance that we do not
> see the error with gcc 6.3.1. This does not imply that the code is correct.

Everything is possible.  But there is very little C code involved
here.  It's mostly scary assembly code...

Cheers,

Mark

^ permalink raw reply	[flat|nested] 27+ messages in thread

end of thread, other threads:[~2018-09-23 18:22 UTC | newest]

Thread overview: 27+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-06-13 22:41 [U-Boot] [PATCH v3 0/3] efi_loader: ARM: add support for ARMV7_NONSEC=y Mark Kettenis
2018-06-13 22:41 ` [U-Boot] [PATCH v3 1/3] ARM: HYP/non-sec: migrate stack Mark Kettenis
2018-06-13 22:41 ` [U-Boot] [PATCH v3 2/3] efi_loader: ARM: run EFI payloads non-secure Mark Kettenis
2018-06-14 11:54   ` Marc Zyngier
2018-06-14 20:55     ` Mark Kettenis
2018-06-15  7:59       ` Marc Zyngier
2018-06-15 12:51         ` Mark Kettenis
2018-06-15 13:20           ` Marc Zyngier
2018-08-31 17:37   ` Heinrich Schuchardt
2018-08-31 18:45     ` Mark Kettenis
2018-09-01 10:21       ` Alexander Graf
2018-09-22 23:30         ` Heinrich Schuchardt
2018-09-23  2:39           ` Jonathan Gray
2018-09-23  3:38             ` Heinrich Schuchardt
2018-09-23 18:22               ` Mark Kettenis
2018-06-13 22:41 ` [U-Boot] [PATCH v3 3/3] Revert "efi_loader: no support for ARMV7_NONSEC=y" Mark Kettenis
2018-06-14  8:46 ` [U-Boot] [PATCH v3 0/3] efi_loader: ARM: add support for ARMV7_NONSEC=y Alexander Graf
2018-06-14 17:55 ` Heinrich Schuchardt
2018-06-14 18:00   ` Alexander Graf
2018-06-14 20:50   ` Mark Kettenis
2018-06-15  3:39     ` Heinrich Schuchardt
2018-06-15 10:49       ` Alexander Graf
2018-06-15 13:01         ` Mark Kettenis
2018-06-15 13:12           ` Alexander Graf
2018-06-15 13:31             ` Mark Kettenis
2018-06-15 20:35     ` Heinrich Schuchardt
2018-06-15 22:03       ` Mark Kettenis

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.