[U-Boot] [PATCH 0/2] efi_loader: Implement reset on RPi
If Linux finds an EFI implementation it always uses the EFI reset handler to reboot or power down the system.
Unfortunately we haven't implemented that one yet. In fact, while we prepared for RTS handling, we never actually implemented a single user.
This is going to change today. This simple patch set enables RTS reset support for the RPi systems, allowing you to reboot and shut down the rpi if booted via EFI.
It also lays the groundwork to show how to implement that functionality at all, so I expect more boards to follow.
Alexander Graf (2): efi_loader: Allow boards to implement get_time and reset_system ARM: bcm283x: Implement EFI RTS reset_system
arch/arm/mach-bcm283x/include/mach/wdog.h | 2 +- arch/arm/mach-bcm283x/reset.c | 59 +++++++++++++++-- cmd/bootefi.c | 4 ++ include/efi_loader.h | 18 ++++++ lib/efi_loader/efi_runtime.c | 101 ++++++++++++++++++++++++++---- 5 files changed, 166 insertions(+), 18 deletions(-)
EFI allows an OS to leverage firmware drivers while the OS is running. In the generic code we so far had to stub those implementations out, because we would need board specific knowledge about MMIO setups for it.
However, boards can easily implement those themselves. This patch provides the framework so that a board can implement its own versions of get_time and reset_system which would actually do something useful.
While at it we also introduce a simple way for code to reserve MMIO pointers as runtime available.
Signed-off-by: Alexander Graf agraf@suse.de --- cmd/bootefi.c | 4 ++ include/efi_loader.h | 18 ++++++++ lib/efi_loader/efi_runtime.c | 101 ++++++++++++++++++++++++++++++++++++++----- 3 files changed, 112 insertions(+), 11 deletions(-)
diff --git a/cmd/bootefi.c b/cmd/bootefi.c index 7f552fc..7546d0a 100644 --- a/cmd/bootefi.c +++ b/cmd/bootefi.c @@ -198,6 +198,10 @@ static unsigned long do_bootefi_exec(void *efi, void *fdt) efi_gop_register(); #endif
+ /* Initialize EFI runtime services */ + efi_reset_system_init(); + efi_get_time_init(); + /* Call our payload! */ #ifdef DEBUG_EFI printf("%s:%d Jumping to 0x%lx\n", __func__, __LINE__, (long)entry); diff --git a/include/efi_loader.h b/include/efi_loader.h index 88b8149..72bc699 100644 --- a/include/efi_loader.h +++ b/include/efi_loader.h @@ -147,11 +147,29 @@ static inline void ascii2unicode(u16 *unicode, char *ascii) #define EFI_RUNTIME_DATA __attribute__ ((section ("efi_runtime_data"))) #define EFI_RUNTIME_TEXT __attribute__ ((section ("efi_runtime_text")))
+/* Call this with mmio_ptr as the _pointer_ to a pointer to an MMIO region + * to make it available at runtime */ +void efi_add_runtime_mmio(void *mmio_ptr, u64 len); + +/* Boards may provide the functions below to implement RTS functionality */ + +void EFI_RUNTIME_TEXT EFIAPI efi_reset_system( + enum efi_reset_type reset_type, + efi_status_t reset_status, + unsigned long data_size, void *reset_data); +void efi_reset_system_init(void); + +efi_status_t EFI_RUNTIME_TEXT EFIAPI efi_get_time( + struct efi_time *time, + struct efi_time_cap *capabilities); +void efi_get_time_init(void); + #else /* defined(EFI_LOADER) && !defined(CONFIG_SPL_BUILD) */
/* Without CONFIG_EFI_LOADER we don't have a runtime section, stub it out */ #define EFI_RUNTIME_DATA #define EFI_RUNTIME_TEXT +static inline void efi_add_runtime_mmio(void **mmio_ptr, u64 len) { }
/* No loader configured, stub out EFI_ENTRY */ static inline void efi_restore_gd(void) { } diff --git a/lib/efi_loader/efi_runtime.c b/lib/efi_loader/efi_runtime.c index 11d0126..61f2585 100644 --- a/lib/efi_loader/efi_runtime.c +++ b/lib/efi_loader/efi_runtime.c @@ -16,6 +16,16 @@ /* For manual relocation support */ DECLARE_GLOBAL_DATA_PTR;
+struct efi_runtime_mmio_list { + struct list_head link; + void **ptr; + u64 paddr; + u64 len; +}; + +/* This list contains all runtime available mmio regions */ +LIST_HEAD(efi_runtime_mmio); + static efi_status_t EFI_RUNTIME_TEXT EFIAPI efi_unimplemented(void); static efi_status_t EFI_RUNTIME_TEXT EFIAPI efi_device_error(void); static efi_status_t EFI_RUNTIME_TEXT EFIAPI efi_invalid_parameter(void); @@ -55,9 +65,10 @@ struct elf_rela { * handle a good number of runtime callbacks */
-static void EFIAPI efi_reset_system(enum efi_reset_type reset_type, - efi_status_t reset_status, - unsigned long data_size, void *reset_data) +static void EFIAPI efi_reset_system_boottime( + enum efi_reset_type reset_type, + efi_status_t reset_status, + unsigned long data_size, void *reset_data) { EFI_ENTRY("%d %lx %lx %p", reset_type, reset_status, data_size, reset_data); @@ -72,11 +83,12 @@ static void EFIAPI efi_reset_system(enum efi_reset_type reset_type, break; }
- EFI_EXIT(EFI_SUCCESS); + while (1) { } }
-static efi_status_t EFIAPI efi_get_time(struct efi_time *time, - struct efi_time_cap *capabilities) +static efi_status_t EFIAPI efi_get_time_boottime( + struct efi_time *time, + struct efi_time_cap *capabilities) { #if defined(CONFIG_CMD_DATE) && defined(CONFIG_DM_RTC) struct rtc_time tm; @@ -107,6 +119,33 @@ static efi_status_t EFIAPI efi_get_time(struct efi_time *time, #endif }
+/* Boards may override the helpers below to implement RTS functionality */ + +void __weak EFI_RUNTIME_TEXT EFIAPI efi_reset_system( + enum efi_reset_type reset_type, + efi_status_t reset_status, + unsigned long data_size, void *reset_data) +{ + /* Nothing we can do */ + while (1) { } +} + +void __weak efi_reset_system_init(void) +{ +} + +efi_status_t __weak EFI_RUNTIME_TEXT EFIAPI efi_get_time( + struct efi_time *time, + struct efi_time_cap *capabilities) +{ + /* Nothing we can do */ + return EFI_DEVICE_ERROR; +} + +void __weak efi_get_time_init(void) +{ +} + struct efi_runtime_detach_list_struct { void *ptr; void *patchto; @@ -116,7 +155,7 @@ static const struct efi_runtime_detach_list_struct efi_runtime_detach_list[] = { { /* do_reset is gone */ .ptr = &efi_runtime_services.reset_system, - .patchto = NULL, + .patchto = efi_reset_system, }, { /* invalidate_*cache_all are gone */ .ptr = &efi_runtime_services.set_virtual_address_map, @@ -124,7 +163,7 @@ static const struct efi_runtime_detach_list_struct efi_runtime_detach_list[] = { }, { /* RTC accessors are gone */ .ptr = &efi_runtime_services.get_time, - .patchto = &efi_device_error, + .patchto = &efi_get_time, }, { /* Clean up system table */ .ptr = &systab.con_in, @@ -241,12 +280,38 @@ static efi_status_t EFIAPI efi_set_virtual_address_map( EFI_ENTRY("%lx %lx %x %p", memory_map_size, descriptor_size, descriptor_version, virtmap);
+ /* Rebind mmio pointers */ + for (i = 0; i < n; i++) { + struct efi_mem_desc *map = (void*)virtmap + + (descriptor_size * i); + struct list_head *lhandle; + efi_physical_addr_t map_start = map->physical_start; + efi_physical_addr_t map_len = map->num_pages << EFI_PAGE_SHIFT; + efi_physical_addr_t map_end = map_start + map_len; + + /* Adjust all mmio pointers in this region */ + list_for_each(lhandle, &efi_runtime_mmio) { + struct efi_runtime_mmio_list *lmmio; + + lmmio = list_entry(lhandle, + struct efi_runtime_mmio_list, + link); + if ((map_start <= lmmio->paddr) && + (map_end >= lmmio->paddr)) { + u64 off = map->virtual_start - map_start; + *lmmio->ptr = (void*)(lmmio->paddr + off); + } + } + } + + /* Move the actual runtime code over */ for (i = 0; i < n; i++) { struct efi_mem_desc *map;
map = (void*)virtmap + (descriptor_size * i); if (map->type == EFI_RUNTIME_SERVICES_CODE) { - ulong new_offset = map->virtual_start - (runtime_start - gd->relocaddr); + ulong new_offset = map->virtual_start - + (runtime_start - gd->relocaddr);
efi_runtime_relocate(new_offset, map); /* Once we're virtual, we can no longer handle @@ -259,6 +324,20 @@ static efi_status_t EFIAPI efi_set_virtual_address_map( return EFI_EXIT(EFI_INVALID_PARAMETER); }
+void efi_add_runtime_mmio(void *mmio_ptr, u64 len) +{ + struct efi_runtime_mmio_list *newmmio; + + u64 pages = (len + EFI_PAGE_SIZE - 1) >> EFI_PAGE_SHIFT; + efi_add_memory_map(*(uintptr_t *)mmio_ptr, pages, EFI_MMAP_IO, false); + + newmmio = calloc(1, sizeof(*newmmio)); + newmmio->ptr = mmio_ptr; + newmmio->paddr = *(uintptr_t *)mmio_ptr; + newmmio->len = len; + list_add_tail(&newmmio->link, &efi_runtime_mmio); +} + /* * In the second stage, U-Boot has disappeared. To isolate our runtime code * that at this point still exists from the rest, we put it into a special @@ -300,7 +379,7 @@ struct efi_runtime_services EFI_RUNTIME_DATA efi_runtime_services = { .revision = EFI_RUNTIME_SERVICES_REVISION, .headersize = sizeof(struct efi_table_hdr), }, - .get_time = &efi_get_time, + .get_time = &efi_get_time_boottime, .set_time = (void *)&efi_device_error, .get_wakeup_time = (void *)&efi_unimplemented, .set_wakeup_time = (void *)&efi_unimplemented, @@ -310,5 +389,5 @@ struct efi_runtime_services EFI_RUNTIME_DATA efi_runtime_services = { .get_next_variable = (void *)&efi_device_error, .set_variable = (void *)&efi_device_error, .get_next_high_mono_count = (void *)&efi_device_error, - .reset_system = &efi_reset_system, + .reset_system = &efi_reset_system_boottime, };
The rpi has a pretty simple way of resetting the whole system. All it takes is to poke a few registers at a well defined location in MMIO space.
This patch adds support for the EFI loader implementation to allow an OS to reset and power off the system when we're outside of boot time.
Signed-off-by: Alexander Graf agraf@suse.de --- arch/arm/mach-bcm283x/include/mach/wdog.h | 2 +- arch/arm/mach-bcm283x/reset.c | 59 +++++++++++++++++++++++++++---- 2 files changed, 54 insertions(+), 7 deletions(-)
diff --git a/arch/arm/mach-bcm283x/include/mach/wdog.h b/arch/arm/mach-bcm283x/include/mach/wdog.h index 7741d7b..b4caca1 100644 --- a/arch/arm/mach-bcm283x/include/mach/wdog.h +++ b/arch/arm/mach-bcm283x/include/mach/wdog.h @@ -16,7 +16,7 @@ struct bcm2835_wdog_regs { u32 unknown0[7]; u32 rstc; - u32 unknown1; + u32 rsts; u32 wdog; };
diff --git a/arch/arm/mach-bcm283x/reset.c b/arch/arm/mach-bcm283x/reset.c index 72cdc31..e87241d 100644 --- a/arch/arm/mach-bcm283x/reset.c +++ b/arch/arm/mach-bcm283x/reset.c @@ -10,19 +10,66 @@ #include <common.h> #include <asm/io.h> #include <asm/arch/wdog.h> +#include <efi_loader.h>
#define RESET_TIMEOUT 10
-void reset_cpu(ulong addr) +/* + * The Raspberry Pi firmware uses the RSTS register to know which partiton + * to boot from. The partiton value is spread into bits 0, 2, 4, 6, 8, 10. + * Partiton 63 is a special partition used by the firmware to indicate halt. + */ +#define BCM2835_WDOG_RSTS_RASPBERRYPI_HALT 0x555 + +EFI_RUNTIME_DATA struct bcm2835_wdog_regs *wdog_regs = + (struct bcm2835_wdog_regs *)BCM2835_WDOG_PHYSADDR; + +void EFI_RUNTIME_TEXT reset_cpu(ulong addr) { - struct bcm2835_wdog_regs *regs = - (struct bcm2835_wdog_regs *)BCM2835_WDOG_PHYSADDR; uint32_t rstc;
- rstc = readl(®s->rstc); + rstc = readl(&wdog_regs->rstc); rstc &= ~BCM2835_WDOG_RSTC_WRCFG_MASK; rstc |= BCM2835_WDOG_RSTC_WRCFG_FULL_RESET;
- writel(BCM2835_WDOG_PASSWORD | RESET_TIMEOUT, ®s->wdog); - writel(BCM2835_WDOG_PASSWORD | rstc, ®s->rstc); + writel(BCM2835_WDOG_PASSWORD | RESET_TIMEOUT, &wdog_regs->wdog); + writel(BCM2835_WDOG_PASSWORD | rstc, &wdog_regs->rstc); +} + +#ifdef CONFIG_EFI_LOADER + +void EFI_RUNTIME_TEXT EFIAPI efi_reset_system( + enum efi_reset_type reset_type, + efi_status_t reset_status, + unsigned long data_size, void *reset_data) +{ + u32 val; + + switch (reset_type) { + case EFI_RESET_COLD: + case EFI_RESET_WARM: + reset_cpu(0); + break; + case EFI_RESET_SHUTDOWN: + /* + * We set the watchdog hard reset bit here to distinguish this reset + * from the normal (full) reset. bootcode.bin will not reboot after a + * hard reset. + */ + val = readl(&wdog_regs->rsts); + val |= BCM2835_WDOG_PASSWORD; + val |= BCM2835_WDOG_RSTS_RASPBERRYPI_HALT; + writel(val, &wdog_regs->rsts); + reset_cpu(0); + break; + } + + while (1) { } } + +void efi_reset_system_init(void) +{ + efi_add_runtime_mmio(&wdog_regs, sizeof(*wdog_regs)); +} + +#endif
Hi Alexander,
On 5 June 2016 at 15:17, Alexander Graf agraf@suse.de wrote:
The rpi has a pretty simple way of resetting the whole system. All it takes is to poke a few registers at a well defined location in MMIO space.
This patch adds support for the EFI loader implementation to allow an OS to reset and power off the system when we're outside of boot time.
Signed-off-by: Alexander Graf agraf@suse.de
arch/arm/mach-bcm283x/include/mach/wdog.h | 2 +- arch/arm/mach-bcm283x/reset.c | 59 +++++++++++++++++++++++++++---- 2 files changed, 54 insertions(+), 7 deletions(-)
diff --git a/arch/arm/mach-bcm283x/include/mach/wdog.h b/arch/arm/mach-bcm283x/include/mach/wdog.h index 7741d7b..b4caca1 100644 --- a/arch/arm/mach-bcm283x/include/mach/wdog.h +++ b/arch/arm/mach-bcm283x/include/mach/wdog.h @@ -16,7 +16,7 @@ struct bcm2835_wdog_regs { u32 unknown0[7]; u32 rstc;
u32 unknown1;
u32 rsts; u32 wdog;};
diff --git a/arch/arm/mach-bcm283x/reset.c b/arch/arm/mach-bcm283x/reset.c index 72cdc31..e87241d 100644 --- a/arch/arm/mach-bcm283x/reset.c +++ b/arch/arm/mach-bcm283x/reset.c @@ -10,19 +10,66 @@ #include <common.h> #include <asm/io.h> #include <asm/arch/wdog.h> +#include <efi_loader.h>
#define RESET_TIMEOUT 10
-void reset_cpu(ulong addr) +/*
- The Raspberry Pi firmware uses the RSTS register to know which partiton
- to boot from. The partiton value is spread into bits 0, 2, 4, 6, 8, 10.
- Partiton 63 is a special partition used by the firmware to indicate halt.
- */
+#define BCM2835_WDOG_RSTS_RASPBERRYPI_HALT 0x555
+EFI_RUNTIME_DATA struct bcm2835_wdog_regs *wdog_regs =
(struct bcm2835_wdog_regs *)BCM2835_WDOG_PHYSADDR;+void EFI_RUNTIME_TEXT reset_cpu(ulong addr) {
struct bcm2835_wdog_regs *regs =(struct bcm2835_wdog_regs *)BCM2835_WDOG_PHYSADDR; uint32_t rstc;rstc = readl(®s->rstc);
rstc = readl(&wdog_regs->rstc); rstc &= ~BCM2835_WDOG_RSTC_WRCFG_MASK; rstc |= BCM2835_WDOG_RSTC_WRCFG_FULL_RESET;
writel(BCM2835_WDOG_PASSWORD | RESET_TIMEOUT, ®s->wdog);writel(BCM2835_WDOG_PASSWORD | rstc, ®s->rstc);
writel(BCM2835_WDOG_PASSWORD | RESET_TIMEOUT, &wdog_regs->wdog);writel(BCM2835_WDOG_PASSWORD | rstc, &wdog_regs->rstc);
This is rpi-specific code, and presumably U-Boot itself would want to provide this reset facility.
But this code is marked with EFI_RUNTIME_TEXT. I think this means that it can still be called as a normal U-Boot function, right?
Is there some enforcement at link-time that all the function called by EFI_RUNTIME_TEXT functions are also EFI_RUNTIME_TEXT?
Also I'd like to suggest a better name. How about __efi_runtime instead of EFI_RUNTIME_TEXT? All those capital letters in function declarations will give me a headache...we use __packed, __weak, etc.
+}
+#ifdef CONFIG_EFI_LOADER
+void EFI_RUNTIME_TEXT EFIAPI efi_reset_system(
enum efi_reset_type reset_type,efi_status_t reset_status,unsigned long data_size, void *reset_data)+{
u32 val;switch (reset_type) {case EFI_RESET_COLD:case EFI_RESET_WARM:reset_cpu(0);break;case EFI_RESET_SHUTDOWN:/** We set the watchdog hard reset bit here to distinguish this reset* from the normal (full) reset. bootcode.bin will not reboot after a* hard reset.*/val = readl(&wdog_regs->rsts);val |= BCM2835_WDOG_PASSWORD;val |= BCM2835_WDOG_RSTS_RASPBERRYPI_HALT;writel(val, &wdog_regs->rsts);reset_cpu(0);break;}while (1) { }}
+void efi_reset_system_init(void) +{
efi_add_runtime_mmio(&wdog_regs, sizeof(*wdog_regs));+}
+#endif
1.8.5.6
Regards, Simon
On 12.07.16 21:35, Simon Glass wrote:
Hi Alexander,
On 5 June 2016 at 15:17, Alexander Graf agraf@suse.de wrote:
The rpi has a pretty simple way of resetting the whole system. All it takes is to poke a few registers at a well defined location in MMIO space.
This patch adds support for the EFI loader implementation to allow an OS to reset and power off the system when we're outside of boot time.
Signed-off-by: Alexander Graf agraf@suse.de
arch/arm/mach-bcm283x/include/mach/wdog.h | 2 +- arch/arm/mach-bcm283x/reset.c | 59 +++++++++++++++++++++++++++---- 2 files changed, 54 insertions(+), 7 deletions(-)
diff --git a/arch/arm/mach-bcm283x/include/mach/wdog.h b/arch/arm/mach-bcm283x/include/mach/wdog.h index 7741d7b..b4caca1 100644 --- a/arch/arm/mach-bcm283x/include/mach/wdog.h +++ b/arch/arm/mach-bcm283x/include/mach/wdog.h @@ -16,7 +16,7 @@ struct bcm2835_wdog_regs { u32 unknown0[7]; u32 rstc;
u32 unknown1;
u32 rsts; u32 wdog;};
diff --git a/arch/arm/mach-bcm283x/reset.c b/arch/arm/mach-bcm283x/reset.c index 72cdc31..e87241d 100644 --- a/arch/arm/mach-bcm283x/reset.c +++ b/arch/arm/mach-bcm283x/reset.c @@ -10,19 +10,66 @@ #include <common.h> #include <asm/io.h> #include <asm/arch/wdog.h> +#include <efi_loader.h>
#define RESET_TIMEOUT 10
-void reset_cpu(ulong addr) +/*
- The Raspberry Pi firmware uses the RSTS register to know which partiton
- to boot from. The partiton value is spread into bits 0, 2, 4, 6, 8, 10.
- Partiton 63 is a special partition used by the firmware to indicate halt.
- */
+#define BCM2835_WDOG_RSTS_RASPBERRYPI_HALT 0x555
+EFI_RUNTIME_DATA struct bcm2835_wdog_regs *wdog_regs =
(struct bcm2835_wdog_regs *)BCM2835_WDOG_PHYSADDR;+void EFI_RUNTIME_TEXT reset_cpu(ulong addr) {
struct bcm2835_wdog_regs *regs =(struct bcm2835_wdog_regs *)BCM2835_WDOG_PHYSADDR; uint32_t rstc;rstc = readl(®s->rstc);
rstc = readl(&wdog_regs->rstc); rstc &= ~BCM2835_WDOG_RSTC_WRCFG_MASK; rstc |= BCM2835_WDOG_RSTC_WRCFG_FULL_RESET;
writel(BCM2835_WDOG_PASSWORD | RESET_TIMEOUT, ®s->wdog);writel(BCM2835_WDOG_PASSWORD | rstc, ®s->rstc);
writel(BCM2835_WDOG_PASSWORD | RESET_TIMEOUT, &wdog_regs->wdog);writel(BCM2835_WDOG_PASSWORD | rstc, &wdog_regs->rstc);This is rpi-specific code, and presumably U-Boot itself would want to provide this reset facility.
But this code is marked with EFI_RUNTIME_TEXT. I think this means that it can still be called as a normal U-Boot function, right?
Exactly. EFI_RUNTIME_TEXT just moves it into a section that allows the code to exist in parallel with Linux as something called "runtime services".
Is there some enforcement at link-time that all the function called by EFI_RUNTIME_TEXT functions are also EFI_RUNTIME_TEXT?
There's a runtime check in the efi_loader linker. I couldn't find a good way to check it at compile time.
Also I'd like to suggest a better name. How about __efi_runtime instead of EFI_RUNTIME_TEXT? All those capital letters in function declarations will give me a headache...we use __packed, __weak, etc.
Uh, sure. The reason it's upper case is that it can either be defined to a section identifier (if EFI is enabled) or to nothing. Since I'm used to the very simple convention that "#define means upper case" I've made the macro upper case too.
But if you think it'd be more readable to treat it like an unconditional attribute marker, I'm certainly not against it.
Alex
Hi Alex,
Am 05.06.2016 um 23:17 schrieb Alexander Graf:
If Linux finds an EFI implementation it always uses the EFI reset handler to reboot or power down the system.
Hm, I thought my powerdown issues on the Jetson TK1 were for lack of CONFIG_AS3277_RESET - sounds like it could be due to EFI instead?
Unfortunately we haven't implemented that one yet. In fact, while we prepared for RTS handling, we never actually implemented a single user.
This is going to change today. This simple patch set enables RTS reset support for the RPi systems, allowing you to reboot and shut down the rpi if booted via EFI.
It also lays the groundwork to show how to implement that functionality at all, so I expect more boards to follow.
Alexander Graf (2): efi_loader: Allow boards to implement get_time and reset_system ARM: bcm283x: Implement EFI RTS reset_system
arch/arm/mach-bcm283x/include/mach/wdog.h | 2 +- arch/arm/mach-bcm283x/reset.c | 59 +++++++++++++++-- cmd/bootefi.c | 4 ++ include/efi_loader.h | 18 ++++++ lib/efi_loader/efi_runtime.c | 101 ++++++++++++++++++++++++++---- 5 files changed, 166 insertions(+), 18 deletions(-)
This all looks very non-generic and would mean that every board will need to be patched individually, which is unrealistic to be tested by just the two of us.
Can't you patch the reset_cpu() declaration (common.h/sysreset.h) instead of all its implementations? We might still need to patch individual implementations but I don't see why any reset_cpu() implementation should be in a different section than others.
Regards, Andreas
Am 14.07.2016 um 06:48 schrieb Andreas Färber afaerber@suse.de:
Hi Alex,
Am 05.06.2016 um 23:17 schrieb Alexander Graf: If Linux finds an EFI implementation it always uses the EFI reset handler to reboot or power down the system.
Hm, I thought my powerdown issues on the Jetson TK1 were for lack of CONFIG_AS3277_RESET - sounds like it could be due to EFI instead?
It depends. IIRC the TK1 is 32bit, where you're probably using grub2 which is not efi Linux aware, but instead boots over the zImage protocol. In that case Linux doesn't know about efi runtime services.
Unfortunately we haven't implemented that one yet. In fact, while we prepared for RTS handling, we never actually implemented a single user.
This is going to change today. This simple patch set enables RTS reset support for the RPi systems, allowing you to reboot and shut down the rpi if booted via EFI.
It also lays the groundwork to show how to implement that functionality at all, so I expect more boards to follow.
Alexander Graf (2): efi_loader: Allow boards to implement get_time and reset_system ARM: bcm283x: Implement EFI RTS reset_system
arch/arm/mach-bcm283x/include/mach/wdog.h | 2 +- arch/arm/mach-bcm283x/reset.c | 59 +++++++++++++++-- cmd/bootefi.c | 4 ++ include/efi_loader.h | 18 ++++++ lib/efi_loader/efi_runtime.c | 101 ++++++++++++++++++++++++++---- 5 files changed, 166 insertions(+), 18 deletions(-)
This all looks very non-generic and would mean that every board will need to be patched individually, which is unrealistic to be tested by just the two of us.
Can't you patch the reset_cpu() declaration (common.h/sysreset.h) instead of all its implementations? We might still need to patch individual implementations but I don't see why any reset_cpu() implementation should be in a different section than others.
Hmm. There are 2 minor problems:
1) Efi also supports power off on top of reset 2) We would have to convert all boards at once, rather than one by one, as we couldn't distinguish between efi aware and unaware ones
And one major issue:
All device memory pointers used by the reset functions need to be marked as such in the efi memory map and live relocated when entering runtime mode. So we need to manually touch every function either way.
That mesns we could either make a generic, broken version. Or we just convert one by one for systems that we can verify it on :). I hope that I designed the APIs easily enough that people who are not us enable RTS support on other platforms too :)
Alex
Regards, Andreas
-- SUSE Linux GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany GF: Felix Imendörffer, Jane Smithard, Graham Norton HRB 21284 (AG Nürnberg)
Am 14.07.2016 um 08:18 schrieb Alexander Graf:
Am 14.07.2016 um 06:48 schrieb Andreas Färber afaerber@suse.de:
Hi Alex,
Am 05.06.2016 um 23:17 schrieb Alexander Graf: If Linux finds an EFI implementation it always uses the EFI reset handler to reboot or power down the system.
Hm, I thought my powerdown issues on the Jetson TK1 were for lack of CONFIG_AS3277_RESET - sounds like it could be due to EFI instead?
It depends. IIRC the TK1 is 32bit, where you're probably using grub2 which is not efi Linux aware, but instead boots over the zImage protocol. In that case Linux doesn't know about efi runtime services.
We've confirmed in the meantime that the Jetson TK1 issues were unrelated to EFI and could be worked around by enabling some as3722 kernel option.
Unfortunately we haven't implemented that one yet. In fact, while we prepared for RTS handling, we never actually implemented a single user.
This is going to change today. This simple patch set enables RTS reset support for the RPi systems, allowing you to reboot and shut down the rpi if booted via EFI.
It also lays the groundwork to show how to implement that functionality at all, so I expect more boards to follow.
Alexander Graf (2): efi_loader: Allow boards to implement get_time and reset_system ARM: bcm283x: Implement EFI RTS reset_system
arch/arm/mach-bcm283x/include/mach/wdog.h | 2 +- arch/arm/mach-bcm283x/reset.c | 59 +++++++++++++++-- cmd/bootefi.c | 4 ++ include/efi_loader.h | 18 ++++++ lib/efi_loader/efi_runtime.c | 101 ++++++++++++++++++++++++++---- 5 files changed, 166 insertions(+), 18 deletions(-)
This all looks very non-generic and would mean that every board will need to be patched individually, which is unrealistic to be tested by just the two of us.
Can't you patch the reset_cpu() declaration (common.h/sysreset.h) instead of all its implementations? We might still need to patch individual implementations but I don't see why any reset_cpu() implementation should be in a different section than others.
Hmm. There are 2 minor problems:
- Efi also supports power off on top of reset
- We would have to convert all boards at once, rather than one by one, as we couldn't distinguish between efi aware and unaware ones
I don't see why we would need to convert everything at once either way.
And one major issue:
All device memory pointers used by the reset functions need to be marked as such in the efi memory map and live relocated when entering runtime mode. So we need to manually touch every function either way.
That mesns we could either make a generic, broken version. Or we just convert one by one for systems that we can verify it on :). I hope that I designed the APIs easily enough that people who are not us enable RTS support on other platforms too :)
Ping! Anyone any comments on the two open questions of uppercase vs. lowercase and placement of attribute?
1/2 should not be affected by those discussion points and already featured in a second series (LS2080ARDB).
Regards, Andreas
Hi,
On 7 August 2016 at 10:59, Andreas Färber afaerber@suse.de wrote:
Am 14.07.2016 um 08:18 schrieb Alexander Graf:
Am 14.07.2016 um 06:48 schrieb Andreas Färber afaerber@suse.de:
Hi Alex,
Am 05.06.2016 um 23:17 schrieb Alexander Graf: If Linux finds an EFI implementation it always uses the EFI reset handler to reboot or power down the system.
Hm, I thought my powerdown issues on the Jetson TK1 were for lack of CONFIG_AS3277_RESET - sounds like it could be due to EFI instead?
It depends. IIRC the TK1 is 32bit, where you're probably using grub2 which is not efi Linux aware, but instead boots over the zImage protocol. In that case Linux doesn't know about efi runtime services.
We've confirmed in the meantime that the Jetson TK1 issues were unrelated to EFI and could be worked around by enabling some as3722 kernel option.
Unfortunately we haven't implemented that one yet. In fact, while we prepared for RTS handling, we never actually implemented a single user.
This is going to change today. This simple patch set enables RTS reset support for the RPi systems, allowing you to reboot and shut down the rpi if booted via EFI.
It also lays the groundwork to show how to implement that functionality at all, so I expect more boards to follow.
Alexander Graf (2): efi_loader: Allow boards to implement get_time and reset_system ARM: bcm283x: Implement EFI RTS reset_system
arch/arm/mach-bcm283x/include/mach/wdog.h | 2 +- arch/arm/mach-bcm283x/reset.c | 59 +++++++++++++++-- cmd/bootefi.c | 4 ++ include/efi_loader.h | 18 ++++++ lib/efi_loader/efi_runtime.c | 101 ++++++++++++++++++++++++++---- 5 files changed, 166 insertions(+), 18 deletions(-)
This all looks very non-generic and would mean that every board will need to be patched individually, which is unrealistic to be tested by just the two of us.
Can't you patch the reset_cpu() declaration (common.h/sysreset.h) instead of all its implementations? We might still need to patch individual implementations but I don't see why any reset_cpu() implementation should be in a different section than others.
Hmm. There are 2 minor problems:
- Efi also supports power off on top of reset
- We would have to convert all boards at once, rather than one by one, as we couldn't distinguish between efi aware and unaware ones
I don't see why we would need to convert everything at once either way.
And one major issue:
All device memory pointers used by the reset functions need to be marked as such in the efi memory map and live relocated when entering runtime mode. So we need to manually touch every function either way.
I'm worried about this. It means that any code used from this run-time needs to be so marked. This could be large tracts of U-Boot. In particular, as I have mentioned a few times, I think the UEFI tables should be 'live' and not just created before booting, which means that much of driver/core needs to be in the UEFI section.
Should we just adjust it so that the whole of U-Boot is in there? How big is the UEFI run-time normally?
That mesns we could either make a generic, broken version. Or we just convert one by one for systems that we can verify it on :). I hope that I designed the APIs easily enough that people who are not us enable RTS support on other platforms too :)
Ping! Anyone any comments on the two open questions of uppercase vs. lowercase and placement of attribute?
I prefer lower case :-)
1/2 should not be affected by those discussion points and already featured in a second series (LS2080ARDB).
Regards, Simon
On 08.08.16 23:44, Simon Glass wrote:
Hi,
On 7 August 2016 at 10:59, Andreas Färber afaerber@suse.de wrote:
Am 14.07.2016 um 08:18 schrieb Alexander Graf:
Am 14.07.2016 um 06:48 schrieb Andreas Färber afaerber@suse.de:
Hi Alex,
Am 05.06.2016 um 23:17 schrieb Alexander Graf: If Linux finds an EFI implementation it always uses the EFI reset handler to reboot or power down the system.
Hm, I thought my powerdown issues on the Jetson TK1 were for lack of CONFIG_AS3277_RESET - sounds like it could be due to EFI instead?
It depends. IIRC the TK1 is 32bit, where you're probably using grub2 which is not efi Linux aware, but instead boots over the zImage protocol. In that case Linux doesn't know about efi runtime services.
We've confirmed in the meantime that the Jetson TK1 issues were unrelated to EFI and could be worked around by enabling some as3722 kernel option.
Unfortunately we haven't implemented that one yet. In fact, while we prepared for RTS handling, we never actually implemented a single user.
This is going to change today. This simple patch set enables RTS reset support for the RPi systems, allowing you to reboot and shut down the rpi if booted via EFI.
It also lays the groundwork to show how to implement that functionality at all, so I expect more boards to follow.
Alexander Graf (2): efi_loader: Allow boards to implement get_time and reset_system ARM: bcm283x: Implement EFI RTS reset_system
arch/arm/mach-bcm283x/include/mach/wdog.h | 2 +- arch/arm/mach-bcm283x/reset.c | 59 +++++++++++++++-- cmd/bootefi.c | 4 ++ include/efi_loader.h | 18 ++++++ lib/efi_loader/efi_runtime.c | 101 ++++++++++++++++++++++++++---- 5 files changed, 166 insertions(+), 18 deletions(-)
This all looks very non-generic and would mean that every board will need to be patched individually, which is unrealistic to be tested by just the two of us.
Can't you patch the reset_cpu() declaration (common.h/sysreset.h) instead of all its implementations? We might still need to patch individual implementations but I don't see why any reset_cpu() implementation should be in a different section than others.
Hmm. There are 2 minor problems:
- Efi also supports power off on top of reset
- We would have to convert all boards at once, rather than one by one, as we couldn't distinguish between efi aware and unaware ones
I don't see why we would need to convert everything at once either way.
And one major issue:
All device memory pointers used by the reset functions need to be marked as such in the efi memory map and live relocated when entering runtime mode. So we need to manually touch every function either way.
I'm worried about this. It means that any code used from this run-time needs to be so marked. This could be large tracts of U-Boot. In
We only need to mark the few bits that are actually executed and used within RTS. That's usually just 2 or 3 functions.
Also, moving forward, we'll see more and more systems implement PSCI which means we can implement a generic PSCI RTS for reset/shutdown.
particular, as I have mentioned a few times, I think the UEFI tables should be 'live' and not just created before booting, which means that much of driver/core needs to be in the UEFI section.
I don't fully understand what you're aiming for here. The tables are always static in a uEFI world. I don't see how they could get more "live" than creating them right before boot.
Should we just adjust it so that the whole of U-Boot is in there? How big is the UEFI run-time normally?
It's really not a problem of putting things in one section or another. It's that if anything within a run time .text section tries to access any memory, be it MMIO or .data memory, it will need to get relocated if we want to be able to run it as RTS.
But really, the RTS interface is very very very slim. Take a look at lib/efi_loader/efi_runtime.c. The only thing that could remotely pull in more code is RTC/variable support. But I'm sure we can find a way to solve that without pulling in all internal abstraction layers.
That mesns we could either make a generic, broken version. Or we just convert one by one for systems that we can verify it on :). I hope that I designed the APIs easily enough that people who are not us enable RTS support on other platforms too :)
Ping! Anyone any comments on the two open questions of uppercase vs. lowercase and placement of attribute?
I prefer lower case :-)
I agree that lower case makes it look less aggressive, but should that hold up this patch?
How about we shove everything in that we have and then I can do a full-tree replacement of EFI_RUNTIME with __efi_runtime. I'm not a big fan of having cosmetics hold up functional changes ;).
Alex
Hi Alex,
On 11 August 2016 at 05:49, Alexander Graf agraf@suse.de wrote:
On 08.08.16 23:44, Simon Glass wrote:
Hi,
On 7 August 2016 at 10:59, Andreas Färber afaerber@suse.de wrote:
Am 14.07.2016 um 08:18 schrieb Alexander Graf:
Am 14.07.2016 um 06:48 schrieb Andreas Färber afaerber@suse.de:
Hi Alex,
Am 05.06.2016 um 23:17 schrieb Alexander Graf: If Linux finds an EFI implementation it always uses the EFI reset handler to reboot or power down the system.
Hm, I thought my powerdown issues on the Jetson TK1 were for lack of CONFIG_AS3277_RESET - sounds like it could be due to EFI instead?
It depends. IIRC the TK1 is 32bit, where you're probably using grub2 which is not efi Linux aware, but instead boots over the zImage protocol. In that case Linux doesn't know about efi runtime services.
We've confirmed in the meantime that the Jetson TK1 issues were unrelated to EFI and could be worked around by enabling some as3722 kernel option.
Unfortunately we haven't implemented that one yet. In fact, while we prepared for RTS handling, we never actually implemented a single user.
This is going to change today. This simple patch set enables RTS reset support for the RPi systems, allowing you to reboot and shut down the rpi if booted via EFI.
It also lays the groundwork to show how to implement that functionality at all, so I expect more boards to follow.
Alexander Graf (2): efi_loader: Allow boards to implement get_time and reset_system ARM: bcm283x: Implement EFI RTS reset_system
arch/arm/mach-bcm283x/include/mach/wdog.h | 2 +- arch/arm/mach-bcm283x/reset.c | 59 +++++++++++++++-- cmd/bootefi.c | 4 ++ include/efi_loader.h | 18 ++++++ lib/efi_loader/efi_runtime.c | 101 ++++++++++++++++++++++++++---- 5 files changed, 166 insertions(+), 18 deletions(-)
This all looks very non-generic and would mean that every board will need to be patched individually, which is unrealistic to be tested by just the two of us.
Can't you patch the reset_cpu() declaration (common.h/sysreset.h) instead of all its implementations? We might still need to patch individual implementations but I don't see why any reset_cpu() implementation should be in a different section than others.
Hmm. There are 2 minor problems:
- Efi also supports power off on top of reset
- We would have to convert all boards at once, rather than one by one, as we couldn't distinguish between efi aware and unaware ones
I don't see why we would need to convert everything at once either way.
And one major issue:
All device memory pointers used by the reset functions need to be marked as such in the efi memory map and live relocated when entering runtime mode. So we need to manually touch every function either way.
I'm worried about this. It means that any code used from this run-time needs to be so marked. This could be large tracts of U-Boot. In
We only need to mark the few bits that are actually executed and used within RTS. That's usually just 2 or 3 functions.
At present you only have reset, and you've only implemented it for one board. Are there other calls that we need to implement? This EFI_RUNTIME is transitive - anything it calls must be in the runtime. Does the linker prevent us from screwing up?
Also, moving forward, we'll see more and more systems implement PSCI which means we can implement a generic PSCI RTS for reset/shutdown.
What system can I get that uses that today?
particular, as I have mentioned a few times, I think the UEFI tables should be 'live' and not just created before booting, which means that much of driver/core needs to be in the UEFI section.
I don't fully understand what you're aiming for here. The tables are always static in a uEFI world. I don't see how they could get more "live" than creating them right before boot.
I'd prefer to see the EFI requests be processed as they are received, rather than with pre-canned data. Your original justification (e.g. for efi_disk_add_dev()) was that there was not authoritative list of block devices in U-Boot. But there is now.
So do you think it would be feasible to drop these tables, and efi_obj_list?
What happens with the tables if I run an app and then come back to U-Boot?
Should we just adjust it so that the whole of U-Boot is in there? How big is the UEFI run-time normally?
It's really not a problem of putting things in one section or another. It's that if anything within a run time .text section tries to access any memory, be it MMIO or .data memory, it will need to get relocated if we want to be able to run it as RTS.
But really, the RTS interface is very very very slim. Take a look at lib/efi_loader/efi_runtime.c. The only thing that could remotely pull in more code is RTC/variable support. But I'm sure we can find a way to solve that without pulling in all internal abstraction layers.
That mesns we could either make a generic, broken version. Or we just convert one by one for systems that we can verify it on :). I hope that I designed the APIs easily enough that people who are not us enable RTS support on other platforms too :)
Ping! Anyone any comments on the two open questions of uppercase vs. lowercase and placement of attribute?
I prefer lower case :-)
I agree that lower case makes it look less aggressive, but should that hold up this patch?
How about we shove everything in that we have and then I can do a full-tree replacement of EFI_RUNTIME with __efi_runtime. I'm not a big fan of having cosmetics hold up functional changes ;).
Sounds good.
Alex
Regards, Simon
On 12.08.16 19:21, Simon Glass wrote:
Hi Alex,
On 11 August 2016 at 05:49, Alexander Graf agraf@suse.de wrote:
On 08.08.16 23:44, Simon Glass wrote:
Hi,
On 7 August 2016 at 10:59, Andreas Färber afaerber@suse.de wrote:
Am 14.07.2016 um 08:18 schrieb Alexander Graf:
Am 14.07.2016 um 06:48 schrieb Andreas Färber afaerber@suse.de:
Hi Alex,
> Am 05.06.2016 um 23:17 schrieb Alexander Graf: > If Linux finds an EFI implementation it always uses the EFI reset handler to > reboot or power down the system.
Hm, I thought my powerdown issues on the Jetson TK1 were for lack of CONFIG_AS3277_RESET - sounds like it could be due to EFI instead?
It depends. IIRC the TK1 is 32bit, where you're probably using grub2 which is not efi Linux aware, but instead boots over the zImage protocol. In that case Linux doesn't know about efi runtime services.
We've confirmed in the meantime that the Jetson TK1 issues were unrelated to EFI and could be worked around by enabling some as3722 kernel option.
> Unfortunately we haven't implemented that one yet. In fact, while we prepared > for RTS handling, we never actually implemented a single user. > > This is going to change today. This simple patch set enables RTS reset support > for the RPi systems, allowing you to reboot and shut down the rpi if booted > via EFI. > > It also lays the groundwork to show how to implement that functionality at all, > so I expect more boards to follow. > > Alexander Graf (2): > efi_loader: Allow boards to implement get_time and reset_system > ARM: bcm283x: Implement EFI RTS reset_system > > arch/arm/mach-bcm283x/include/mach/wdog.h | 2 +- > arch/arm/mach-bcm283x/reset.c | 59 +++++++++++++++-- > cmd/bootefi.c | 4 ++ > include/efi_loader.h | 18 ++++++ > lib/efi_loader/efi_runtime.c | 101 ++++++++++++++++++++++++++---- > 5 files changed, 166 insertions(+), 18 deletions(-)
This all looks very non-generic and would mean that every board will need to be patched individually, which is unrealistic to be tested by just the two of us.
Can't you patch the reset_cpu() declaration (common.h/sysreset.h) instead of all its implementations? We might still need to patch individual implementations but I don't see why any reset_cpu() implementation should be in a different section than others.
Hmm. There are 2 minor problems:
- Efi also supports power off on top of reset
- We would have to convert all boards at once, rather than one by one, as we couldn't distinguish between efi aware and unaware ones
I don't see why we would need to convert everything at once either way.
And one major issue:
All device memory pointers used by the reset functions need to be marked as such in the efi memory map and live relocated when entering runtime mode. So we need to manually touch every function either way.
I'm worried about this. It means that any code used from this run-time needs to be so marked. This could be large tracts of U-Boot. In
We only need to mark the few bits that are actually executed and used within RTS. That's usually just 2 or 3 functions.
At present you only have reset, and you've only implemented it for one board. Are there other calls that we need to implement? This EFI_RUNTIME is transitive - anything it calls must be in the runtime. Does the linker prevent us from screwing up?
It doesn't (I couldn't find a way), but the runtime relocation code does:
https://github.com/trini/u-boot/blob/master/lib/efi_loader/efi_runtime.c#L20...
Also, moving forward, we'll see more and more systems implement PSCI which means we can implement a generic PSCI RTS for reset/shutdown.
What system can I get that uses that today?
Anything that runs ATF:
https://github.com/ARM-software/arm-trusted-firmware
in EL3:
- Pine64 - ZynqMP - S905X - Hikey - probably many many more
I think the only AArch64 systems that I'm aware of that do *not* run ATF in EL3 are
- Anything FSL LS - RPi3 - uniphier
and in all 3 cases it's a bad design decision IMHO, but it's one that their respective creators did.
particular, as I have mentioned a few times, I think the UEFI tables should be 'live' and not just created before booting, which means that much of driver/core needs to be in the UEFI section.
I don't fully understand what you're aiming for here. The tables are always static in a uEFI world. I don't see how they could get more "live" than creating them right before boot.
I'd prefer to see the EFI requests be processed as they are received, rather than with pre-canned data. Your original justification (e.g. for efi_disk_add_dev()) was that there was not authoritative list of block devices in U-Boot. But there is now.
So do you think it would be feasible to drop these tables, and efi_obj_list?
Ah, we're talking about different tables. I was thinking "tables" as in what EFI calls tables. That would be the device tree or SMBIOS tables. Those are really just pointers to memory.
What happens with the tables if I run an app and then come back to U-Boot?
So let me try to grasp what you mean by tables. I guess you mean the list of devices? It gets regenerated on every bootefi invocation. Or did you mean something different?
Alex
Hi Alex,
On 12 August 2016 at 12:50, Alexander Graf agraf@suse.de wrote:
On 12.08.16 19:21, Simon Glass wrote:
Hi Alex,
On 11 August 2016 at 05:49, Alexander Graf agraf@suse.de wrote:
On 08.08.16 23:44, Simon Glass wrote:
Hi,
On 7 August 2016 at 10:59, Andreas Färber afaerber@suse.de wrote:
Am 14.07.2016 um 08:18 schrieb Alexander Graf:
> Am 14.07.2016 um 06:48 schrieb Andreas Färber afaerber@suse.de: > > Hi Alex, > >> Am 05.06.2016 um 23:17 schrieb Alexander Graf: >> If Linux finds an EFI implementation it always uses the EFI reset handler to >> reboot or power down the system. > > Hm, I thought my powerdown issues on the Jetson TK1 were for lack of > CONFIG_AS3277_RESET - sounds like it could be due to EFI instead?
It depends. IIRC the TK1 is 32bit, where you're probably using grub2 which is not efi Linux aware, but instead boots over the zImage protocol. In that case Linux doesn't know about efi runtime services.
We've confirmed in the meantime that the Jetson TK1 issues were unrelated to EFI and could be worked around by enabling some as3722 kernel option.
>> Unfortunately we haven't implemented that one yet. In fact, while we prepared >> for RTS handling, we never actually implemented a single user. >> >> This is going to change today. This simple patch set enables RTS reset support >> for the RPi systems, allowing you to reboot and shut down the rpi if booted >> via EFI. >> >> It also lays the groundwork to show how to implement that functionality at all, >> so I expect more boards to follow. >> >> Alexander Graf (2): >> efi_loader: Allow boards to implement get_time and reset_system >> ARM: bcm283x: Implement EFI RTS reset_system >> >> arch/arm/mach-bcm283x/include/mach/wdog.h | 2 +- >> arch/arm/mach-bcm283x/reset.c | 59 +++++++++++++++-- >> cmd/bootefi.c | 4 ++ >> include/efi_loader.h | 18 ++++++ >> lib/efi_loader/efi_runtime.c | 101 ++++++++++++++++++++++++++---- >> 5 files changed, 166 insertions(+), 18 deletions(-) > > This all looks very non-generic and would mean that every board will > need to be patched individually, which is unrealistic to be tested by > just the two of us. > > Can't you patch the reset_cpu() declaration (common.h/sysreset.h) > instead of all its implementations? We might still need to patch > individual implementations but I don't see why any reset_cpu() > implementation should be in a different section than others.
Hmm. There are 2 minor problems:
- Efi also supports power off on top of reset
- We would have to convert all boards at once, rather than one by one, as we couldn't distinguish between efi aware and unaware ones
I don't see why we would need to convert everything at once either way.
And one major issue:
All device memory pointers used by the reset functions need to be marked as such in the efi memory map and live relocated when entering runtime mode. So we need to manually touch every function either way.
I'm worried about this. It means that any code used from this run-time needs to be so marked. This could be large tracts of U-Boot. In
We only need to mark the few bits that are actually executed and used within RTS. That's usually just 2 or 3 functions.
At present you only have reset, and you've only implemented it for one board. Are there other calls that we need to implement? This EFI_RUNTIME is transitive - anything it calls must be in the runtime. Does the linker prevent us from screwing up?
It doesn't (I couldn't find a way), but the runtime relocation code does:
https://github.com/trini/u-boot/blob/master/lib/efi_loader/efi_runtime.c#L20...
Also, moving forward, we'll see more and more systems implement PSCI which means we can implement a generic PSCI RTS for reset/shutdown.
What system can I get that uses that today?
Anything that runs ATF:
https://github.com/ARM-software/arm-trusted-firmware
in EL3:
- Pine64
- ZynqMP
- S905X
- Hikey
- probably many many more
I think the only AArch64 systems that I'm aware of that do *not* run ATF in EL3 are
- Anything FSL LS
- RPi3
- uniphier
and in all 3 cases it's a bad design decision IMHO, but it's one that their respective creators did.
So there is an interface in ATF to shut the machine down? Is that because of security / permission? I really have not looked into ATF unfortunately.
particular, as I have mentioned a few times, I think the UEFI tables should be 'live' and not just created before booting, which means that much of driver/core needs to be in the UEFI section.
I don't fully understand what you're aiming for here. The tables are always static in a uEFI world. I don't see how they could get more "live" than creating them right before boot.
I'd prefer to see the EFI requests be processed as they are received, rather than with pre-canned data. Your original justification (e.g. for efi_disk_add_dev()) was that there was not authoritative list of block devices in U-Boot. But there is now.
So do you think it would be feasible to drop these tables, and efi_obj_list?
Ah, we're talking about different tables. I was thinking "tables" as in what EFI calls tables. That would be the device tree or SMBIOS tables. Those are really just pointers to memory.
What happens with the tables if I run an app and then come back to U-Boot?
So let me try to grasp what you mean by tables. I guess you mean the list of devices? It gets regenerated on every bootefi invocation. Or did you mean something different?
Yes that's right - I mean the list of devices, and all the stuff in there.
If I had a function like:
struct efi_object *dev_get_efi(struct udevice *dev)
would that be enough to handle the conversion? Basically I'm wondering whether we can scan through the DM tables and obtain EFI objects as needed (on the fly), or whether this is prohibitively expensive. DM doesn't like competition!
Regards, Simon
On 12 Aug 2016, at 22:07, Simon Glass sjg@chromium.org wrote:
Hi Alex,
On 12 August 2016 at 12:50, Alexander Graf agraf@suse.de wrote:
On 12.08.16 19:21, Simon Glass wrote:
Hi Alex,
On 11 August 2016 at 05:49, Alexander Graf agraf@suse.de wrote:
On 08.08.16 23:44, Simon Glass wrote:
Hi,
On 7 August 2016 at 10:59, Andreas Färber afaerber@suse.de wrote:
Am 14.07.2016 um 08:18 schrieb Alexander Graf: >> Am 14.07.2016 um 06:48 schrieb Andreas Färber afaerber@suse.de: >> >> Hi Alex, >> >>> Am 05.06.2016 um 23:17 schrieb Alexander Graf: >>> If Linux finds an EFI implementation it always uses the EFI reset handler to >>> reboot or power down the system. >> >> Hm, I thought my powerdown issues on the Jetson TK1 were for lack of >> CONFIG_AS3277_RESET - sounds like it could be due to EFI instead? > > It depends. IIRC the TK1 is 32bit, where you're probably using grub2 which is not efi Linux aware, but instead boots over the zImage protocol. In that case Linux doesn't know about efi runtime services.
We've confirmed in the meantime that the Jetson TK1 issues were unrelated to EFI and could be worked around by enabling some as3722 kernel option.
>>> Unfortunately we haven't implemented that one yet. In fact, while we prepared >>> for RTS handling, we never actually implemented a single user. >>> >>> This is going to change today. This simple patch set enables RTS reset support >>> for the RPi systems, allowing you to reboot and shut down the rpi if booted >>> via EFI. >>> >>> It also lays the groundwork to show how to implement that functionality at all, >>> so I expect more boards to follow. >>> >>> Alexander Graf (2): >>> efi_loader: Allow boards to implement get_time and reset_system >>> ARM: bcm283x: Implement EFI RTS reset_system >>> >>> arch/arm/mach-bcm283x/include/mach/wdog.h | 2 +- >>> arch/arm/mach-bcm283x/reset.c | 59 +++++++++++++++-- >>> cmd/bootefi.c | 4 ++ >>> include/efi_loader.h | 18 ++++++ >>> lib/efi_loader/efi_runtime.c | 101 ++++++++++++++++++++++++++---- >>> 5 files changed, 166 insertions(+), 18 deletions(-) >> >> This all looks very non-generic and would mean that every board will >> need to be patched individually, which is unrealistic to be tested by >> just the two of us. >> >> Can't you patch the reset_cpu() declaration (common.h/sysreset.h) >> instead of all its implementations? We might still need to patch >> individual implementations but I don't see why any reset_cpu() >> implementation should be in a different section than others. > > Hmm. There are 2 minor problems: > > 1) Efi also supports power off on top of reset > 2) We would have to convert all boards at once, rather than one by one, as we couldn't distinguish between efi aware and unaware ones
I don't see why we would need to convert everything at once either way.
> > And one major issue: > > All device memory pointers used by the reset functions need to be marked as such in the efi memory map and live relocated when entering runtime mode. So we need to manually touch every function either way.
I'm worried about this. It means that any code used from this run-time needs to be so marked. This could be large tracts of U-Boot. In
We only need to mark the few bits that are actually executed and used within RTS. That's usually just 2 or 3 functions.
At present you only have reset, and you've only implemented it for one board. Are there other calls that we need to implement? This EFI_RUNTIME is transitive - anything it calls must be in the runtime. Does the linker prevent us from screwing up?
It doesn't (I couldn't find a way), but the runtime relocation code does:
https://github.com/trini/u-boot/blob/master/lib/efi_loader/efi_runtime.c#L20...
Also, moving forward, we'll see more and more systems implement PSCI which means we can implement a generic PSCI RTS for reset/shutdown.
What system can I get that uses that today?
Anything that runs ATF:
https://github.com/ARM-software/arm-trusted-firmware
in EL3:
- Pine64
- ZynqMP
- S905X
- Hikey
- probably many many more
I think the only AArch64 systems that I'm aware of that do *not* run ATF in EL3 are
- Anything FSL LS
- RPi3
- uniphier
and in all 3 cases it's a bad design decision IMHO, but it's one that their respective creators did.
So there is an interface in ATF to shut the machine down? Is that because of security / permission? I really have not looked into ATF unfortunately.
The main interface that we care about in ATF is PSCI:
http://infocenter.arm.com/help/topic/com.arm.doc.den0022c/DEN0022C_Power_Sta...
which implements interesting functions like
SYSTEM_OFF and SYSTEM_RESET
So yes, it basically implements everything we need. IIUC the NXP folks want to implement their own PSCI backend in U-Boot, so we could use the same infrastructure. I hope that unipher moves to something that implements PSCI as well. That would only leave the RPi3 as non-PSCI aarch64 platform in mainline U-Boot.
The only case where not implementing PSCI is a reasonable compromise is that APM X-Gene, which does not implement EL3. But given that I have neither seen any efforts from any side to push their U-Boot code upstream nor any need for the U-Boot EFI support, as they have a working edk2 port, I doubt anyone really cares :).
particular, as I have mentioned a few times, I think the UEFI tables should be 'live' and not just created before booting, which means that much of driver/core needs to be in the UEFI section.
I don't fully understand what you're aiming for here. The tables are always static in a uEFI world. I don't see how they could get more "live" than creating them right before boot.
I'd prefer to see the EFI requests be processed as they are received, rather than with pre-canned data. Your original justification (e.g. for efi_disk_add_dev()) was that there was not authoritative list of block devices in U-Boot. But there is now.
So do you think it would be feasible to drop these tables, and efi_obj_list?
Ah, we're talking about different tables. I was thinking "tables" as in what EFI calls tables. That would be the device tree or SMBIOS tables. Those are really just pointers to memory.
What happens with the tables if I run an app and then come back to U-Boot?
So let me try to grasp what you mean by tables. I guess you mean the list of devices? It gets regenerated on every bootefi invocation. Or did you mean something different?
Yes that's right - I mean the list of devices, and all the stuff in there.
If I had a function like:
struct efi_object *dev_get_efi(struct udevice *dev)
would that be enough to handle the conversion? Basically I'm wondering whether we can scan through the DM tables and obtain EFI objects as needed (on the fly), or whether this is prohibitively expensive. DM doesn't like competition!
I’m not 100% sure how we should model it yet. One of the nice things of the separate object tree is that we have no (memory & cpu) overhead for people who don’t call bootefi. I doubt it really makes a big difference though.
I don’t have a good grasp on how well the two object models overlap. If we want to get rid of efi_obj_list we’d need to put at least the uuid and class type maps into the actual udevice objects I guess. So I suppose what you’re thinking of is something along the lines of
struct udevice { […]; struct efi_object; }
which would basically give us exactly that? We’d then still have to actually initialize the objects with data somehow. I’m not sure how that would happen. Maybe call into something like efi_disk_add_dev() from the generic block init code?
Alex
Hi Alex,
On 12 August 2016 at 15:20, Alexander Graf agraf@suse.de wrote:
On 12 Aug 2016, at 22:07, Simon Glass sjg@chromium.org wrote:
Hi Alex,
On 12 August 2016 at 12:50, Alexander Graf agraf@suse.de wrote:
On 12.08.16 19:21, Simon Glass wrote:
Hi Alex,
On 11 August 2016 at 05:49, Alexander Graf agraf@suse.de wrote:
On 08.08.16 23:44, Simon Glass wrote:
Hi,
On 7 August 2016 at 10:59, Andreas Färber afaerber@suse.de wrote: > Am 14.07.2016 um 08:18 schrieb Alexander Graf: >>> Am 14.07.2016 um 06:48 schrieb Andreas Färber afaerber@suse.de: >>> >>> Hi Alex, >>> >>>> Am 05.06.2016 um 23:17 schrieb Alexander Graf: >>>> If Linux finds an EFI implementation it always uses the EFI reset handler to >>>> reboot or power down the system. >>> >>> Hm, I thought my powerdown issues on the Jetson TK1 were for lack of >>> CONFIG_AS3277_RESET - sounds like it could be due to EFI instead? >> >> It depends. IIRC the TK1 is 32bit, where you're probably using grub2 which is not efi Linux aware, but instead boots over the zImage protocol. In that case Linux doesn't know about efi runtime services. > > We've confirmed in the meantime that the Jetson TK1 issues were > unrelated to EFI and could be worked around by enabling some as3722 > kernel option. > >>>> Unfortunately we haven't implemented that one yet. In fact, while we prepared >>>> for RTS handling, we never actually implemented a single user. >>>> >>>> This is going to change today. This simple patch set enables RTS reset support >>>> for the RPi systems, allowing you to reboot and shut down the rpi if booted >>>> via EFI. >>>> >>>> It also lays the groundwork to show how to implement that functionality at all, >>>> so I expect more boards to follow. >>>> >>>> Alexander Graf (2): >>>> efi_loader: Allow boards to implement get_time and reset_system >>>> ARM: bcm283x: Implement EFI RTS reset_system >>>> >>>> arch/arm/mach-bcm283x/include/mach/wdog.h | 2 +- >>>> arch/arm/mach-bcm283x/reset.c | 59 +++++++++++++++-- >>>> cmd/bootefi.c | 4 ++ >>>> include/efi_loader.h | 18 ++++++ >>>> lib/efi_loader/efi_runtime.c | 101 ++++++++++++++++++++++++++---- >>>> 5 files changed, 166 insertions(+), 18 deletions(-) >>> >>> This all looks very non-generic and would mean that every board will >>> need to be patched individually, which is unrealistic to be tested by >>> just the two of us. >>> >>> Can't you patch the reset_cpu() declaration (common.h/sysreset.h) >>> instead of all its implementations? We might still need to patch >>> individual implementations but I don't see why any reset_cpu() >>> implementation should be in a different section than others. >> >> Hmm. There are 2 minor problems: >> >> 1) Efi also supports power off on top of reset >> 2) We would have to convert all boards at once, rather than one by one, as we couldn't distinguish between efi aware and unaware ones > > I don't see why we would need to convert everything at once either way. > >> >> And one major issue: >> >> All device memory pointers used by the reset functions need to be marked as such in the efi memory map and live relocated when entering runtime mode. So we need to manually touch every function either way.
I'm worried about this. It means that any code used from this run-time needs to be so marked. This could be large tracts of U-Boot. In
We only need to mark the few bits that are actually executed and used within RTS. That's usually just 2 or 3 functions.
At present you only have reset, and you've only implemented it for one board. Are there other calls that we need to implement? This EFI_RUNTIME is transitive - anything it calls must be in the runtime. Does the linker prevent us from screwing up?
It doesn't (I couldn't find a way), but the runtime relocation code does:
https://github.com/trini/u-boot/blob/master/lib/efi_loader/efi_runtime.c#L20...
Also, moving forward, we'll see more and more systems implement PSCI which means we can implement a generic PSCI RTS for reset/shutdown.
What system can I get that uses that today?
Anything that runs ATF:
https://github.com/ARM-software/arm-trusted-firmware
in EL3:
- Pine64
- ZynqMP
- S905X
- Hikey
- probably many many more
I think the only AArch64 systems that I'm aware of that do *not* run ATF in EL3 are
- Anything FSL LS
- RPi3
- uniphier
and in all 3 cases it's a bad design decision IMHO, but it's one that their respective creators did.
So there is an interface in ATF to shut the machine down? Is that because of security / permission? I really have not looked into ATF unfortunately.
The main interface that we care about in ATF is PSCI:
http://infocenter.arm.com/help/topic/com.arm.doc.den0022c/DEN0022C_Power_Sta...
which implements interesting functions like
SYSTEM_OFF and SYSTEM_RESET
Thanks for the link. Way overkill for a small embedded system, but OK.
So yes, it basically implements everything we need. IIUC the NXP folks want to implement their own PSCI backend in U-Boot, so we could use the same infrastructure. I hope that unipher moves to something that implements PSCI as well. That would only leave the RPi3 as non-PSCI aarch64 platform in mainline U-Boot.
The only case where not implementing PSCI is a reasonable compromise is that APM X-Gene, which does not implement EL3. But given that I have neither seen any efforts from any side to push their U-Boot code upstream nor any need for the U-Boot EFI support, as they have a working edk2 port, I doubt anyone really cares :).
particular, as I have mentioned a few times, I think the UEFI tables should be 'live' and not just created before booting, which means that much of driver/core needs to be in the UEFI section.
I don't fully understand what you're aiming for here. The tables are always static in a uEFI world. I don't see how they could get more "live" than creating them right before boot.
I'd prefer to see the EFI requests be processed as they are received, rather than with pre-canned data. Your original justification (e.g. for efi_disk_add_dev()) was that there was not authoritative list of block devices in U-Boot. But there is now.
So do you think it would be feasible to drop these tables, and efi_obj_list?
Ah, we're talking about different tables. I was thinking "tables" as in what EFI calls tables. That would be the device tree or SMBIOS tables. Those are really just pointers to memory.
What happens with the tables if I run an app and then come back to U-Boot?
So let me try to grasp what you mean by tables. I guess you mean the list of devices? It gets regenerated on every bootefi invocation. Or did you mean something different?
Yes that's right - I mean the list of devices, and all the stuff in there.
If I had a function like:
struct efi_object *dev_get_efi(struct udevice *dev)
would that be enough to handle the conversion? Basically I'm wondering whether we can scan through the DM tables and obtain EFI objects as needed (on the fly), or whether this is prohibitively expensive. DM doesn't like competition!
I’m not 100% sure how we should model it yet. One of the nice things of the separate object tree is that we have no (memory & cpu) overhead for people who don’t call bootefi. I doubt it really makes a big difference though.
I don’t have a good grasp on how well the two object models overlap. If we want to get rid of efi_obj_list we’d need to put at least the uuid and class type maps into the actual udevice objects I guess. So I suppose what you’re thinking of is something along the lines of
struct udevice { […]; struct efi_object; }
which would basically give us exactly that? We’d then still have to actually initialize the objects with data somehow. I’m not sure how that would happen. Maybe call into something like efi_disk_add_dev() from the generic block init code?
I'm not really sure either. My original thought was that each object that visible to EFI would be a child of the real object. So each block device would have a child EFI device, for example. We already do something similar with (for example) an MMC device having a BLK device as a child.
But if it really is possible to 'translate' a struct udevice into a struct efi_object, then perhaps we just need a function that can do that, and the question of where to store it becomes an internal detail.
I'm not sure if we want to add specific EFI support to DM, or whether we have enough already to implement this properly.
Regards, Simon
On 13 Aug 2016, at 00:02, Simon Glass sjg@chromium.org wrote:
Hi Alex,
On 12 August 2016 at 15:20, Alexander Graf agraf@suse.de wrote:
On 12 Aug 2016, at 22:07, Simon Glass sjg@chromium.org wrote:
Hi Alex,
On 12 August 2016 at 12:50, Alexander Graf agraf@suse.de wrote:
On 12.08.16 19:21, Simon Glass wrote:
Hi Alex,
On 11 August 2016 at 05:49, Alexander Graf agraf@suse.de wrote:
On 08.08.16 23:44, Simon Glass wrote: > Hi, > > On 7 August 2016 at 10:59, Andreas Färber afaerber@suse.de wrote: >> Am 14.07.2016 um 08:18 schrieb Alexander Graf: >>>> Am 14.07.2016 um 06:48 schrieb Andreas Färber afaerber@suse.de: >>>> >>>> Hi Alex, >>>> >>>>> Am 05.06.2016 um 23:17 schrieb Alexander Graf: >>>>> If Linux finds an EFI implementation it always uses the EFI reset handler to >>>>> reboot or power down the system. >>>> >>>> Hm, I thought my powerdown issues on the Jetson TK1 were for lack of >>>> CONFIG_AS3277_RESET - sounds like it could be due to EFI instead? >>> >>> It depends. IIRC the TK1 is 32bit, where you're probably using grub2 which is not efi Linux aware, but instead boots over the zImage protocol. In that case Linux doesn't know about efi runtime services. >> >> We've confirmed in the meantime that the Jetson TK1 issues were >> unrelated to EFI and could be worked around by enabling some as3722 >> kernel option. >> >>>>> Unfortunately we haven't implemented that one yet. In fact, while we prepared >>>>> for RTS handling, we never actually implemented a single user. >>>>> >>>>> This is going to change today. This simple patch set enables RTS reset support >>>>> for the RPi systems, allowing you to reboot and shut down the rpi if booted >>>>> via EFI. >>>>> >>>>> It also lays the groundwork to show how to implement that functionality at all, >>>>> so I expect more boards to follow. >>>>> >>>>> Alexander Graf (2): >>>>> efi_loader: Allow boards to implement get_time and reset_system >>>>> ARM: bcm283x: Implement EFI RTS reset_system >>>>> >>>>> arch/arm/mach-bcm283x/include/mach/wdog.h | 2 +- >>>>> arch/arm/mach-bcm283x/reset.c | 59 +++++++++++++++-- >>>>> cmd/bootefi.c | 4 ++ >>>>> include/efi_loader.h | 18 ++++++ >>>>> lib/efi_loader/efi_runtime.c | 101 ++++++++++++++++++++++++++---- >>>>> 5 files changed, 166 insertions(+), 18 deletions(-) >>>> >>>> This all looks very non-generic and would mean that every board will >>>> need to be patched individually, which is unrealistic to be tested by >>>> just the two of us. >>>> >>>> Can't you patch the reset_cpu() declaration (common.h/sysreset.h) >>>> instead of all its implementations? We might still need to patch >>>> individual implementations but I don't see why any reset_cpu() >>>> implementation should be in a different section than others. >>> >>> Hmm. There are 2 minor problems: >>> >>> 1) Efi also supports power off on top of reset >>> 2) We would have to convert all boards at once, rather than one by one, as we couldn't distinguish between efi aware and unaware ones >> >> I don't see why we would need to convert everything at once either way. >> >>> >>> And one major issue: >>> >>> All device memory pointers used by the reset functions need to be marked as such in the efi memory map and live relocated when entering runtime mode. So we need to manually touch every function either way. > > I'm worried about this. It means that any code used from this run-time > needs to be so marked. This could be large tracts of U-Boot. In
We only need to mark the few bits that are actually executed and used within RTS. That's usually just 2 or 3 functions.
At present you only have reset, and you've only implemented it for one board. Are there other calls that we need to implement? This EFI_RUNTIME is transitive - anything it calls must be in the runtime. Does the linker prevent us from screwing up?
It doesn't (I couldn't find a way), but the runtime relocation code does:
https://github.com/trini/u-boot/blob/master/lib/efi_loader/efi_runtime.c#L20...
Also, moving forward, we'll see more and more systems implement PSCI which means we can implement a generic PSCI RTS for reset/shutdown.
What system can I get that uses that today?
Anything that runs ATF:
https://github.com/ARM-software/arm-trusted-firmware
in EL3:
- Pine64
- ZynqMP
- S905X
- Hikey
- probably many many more
I think the only AArch64 systems that I'm aware of that do *not* run ATF in EL3 are
- Anything FSL LS
- RPi3
- uniphier
and in all 3 cases it's a bad design decision IMHO, but it's one that their respective creators did.
So there is an interface in ATF to shut the machine down? Is that because of security / permission? I really have not looked into ATF unfortunately.
The main interface that we care about in ATF is PSCI:
http://infocenter.arm.com/help/topic/com.arm.doc.den0022c/DEN0022C_Power_Sta...
which implements interesting functions like
SYSTEM_OFF and SYSTEM_RESET
Thanks for the link. Way overkill for a small embedded system, but OK.
So yes, it basically implements everything we need. IIUC the NXP folks want to implement their own PSCI backend in U-Boot, so we could use the same infrastructure. I hope that unipher moves to something that implements PSCI as well. That would only leave the RPi3 as non-PSCI aarch64 platform in mainline U-Boot.
The only case where not implementing PSCI is a reasonable compromise is that APM X-Gene, which does not implement EL3. But given that I have neither seen any efforts from any side to push their U-Boot code upstream nor any need for the U-Boot EFI support, as they have a working edk2 port, I doubt anyone really cares :).
> particular, as I have mentioned a few times, I think the UEFI tables > should be 'live' and not just created before booting, which means that > much of driver/core needs to be in the UEFI section.
I don't fully understand what you're aiming for here. The tables are always static in a uEFI world. I don't see how they could get more "live" than creating them right before boot.
I'd prefer to see the EFI requests be processed as they are received, rather than with pre-canned data. Your original justification (e.g. for efi_disk_add_dev()) was that there was not authoritative list of block devices in U-Boot. But there is now.
So do you think it would be feasible to drop these tables, and efi_obj_list?
Ah, we're talking about different tables. I was thinking "tables" as in what EFI calls tables. That would be the device tree or SMBIOS tables. Those are really just pointers to memory.
What happens with the tables if I run an app and then come back to U-Boot?
So let me try to grasp what you mean by tables. I guess you mean the list of devices? It gets regenerated on every bootefi invocation. Or did you mean something different?
Yes that's right - I mean the list of devices, and all the stuff in there.
If I had a function like:
struct efi_object *dev_get_efi(struct udevice *dev)
would that be enough to handle the conversion? Basically I'm wondering whether we can scan through the DM tables and obtain EFI objects as needed (on the fly), or whether this is prohibitively expensive. DM doesn't like competition!
I’m not 100% sure how we should model it yet. One of the nice things of the separate object tree is that we have no (memory & cpu) overhead for people who don’t call bootefi. I doubt it really makes a big difference though.
I don’t have a good grasp on how well the two object models overlap. If we want to get rid of efi_obj_list we’d need to put at least the uuid and class type maps into the actual udevice objects I guess. So I suppose what you’re thinking of is something along the lines of
struct udevice { […]; struct efi_object; }
which would basically give us exactly that? We’d then still have to actually initialize the objects with data somehow. I’m not sure how that would happen. Maybe call into something like efi_disk_add_dev() from the generic block init code?
I'm not really sure either. My original thought was that each object that visible to EFI would be a child of the real object. So each block device would have a child EFI device, for example. We already do something similar with (for example) an MMC device having a BLK device as a child.
But if it really is possible to 'translate' a struct udevice into a struct efi_object, then perhaps we just need a function that can do that, and the question of where to store it becomes an internal detail.
I'm not sure if we want to add specific EFI support to DM, or whether we have enough already to implement this properly.
We would at least need the linkage from “create object of type foo” to “spawn the efi object for that object”, as we need to populate the efi payload facing structs.
Alex
participants (3)
-
Alexander Graf -
Andreas Färber -
Simon Glass