[U-Boot] [PATCH 0/5] armv8: Add generic PSCI client support

[U-Boot] [PATCH 0/5] armv8: Add generic PSCI client support

[Alexander Graf]

On aarch64 systems, Linux only has managed to push almost everyone to using
PSCI as standardized interface for CPU start/stop operations. Fortunately
PSCI also gives us a nicely abstracted interface to reset and power off
the system.

This patch set adds support for generic PSCI reset/shutdown support and ties
it up to EFI Run Time Services, so that an OS like Linux will magically have
reset and shutdown work.

Alexander Graf (5):
  efi_loader: Allow boards to implement get_time and reset_system
  arm: Disable HVC PSCI calls by default
  arm: Add PSCI shutdown function
  arm: Provide common PSCI based reset handler
  efi_loader: Add generic PSCI RTS

 arch/arm/cpu/armv8/Kconfig    |  18 ++++++++
 arch/arm/cpu/armv8/fwcall.c   |  59 ++++++++++++++++++++++--
 arch/arm/include/asm/system.h |  12 +----
 arch/arm/mach-meson/board.c   |   5 ---
 board/xilinx/zynqmp/zynqmp.c  |   5 ---
 cmd/bootefi.c                 |   4 ++
 include/efi_loader.h          |  18 ++++++++
 lib/efi_loader/efi_runtime.c  | 101 +++++++++++++++++++++++++++++++++++++-----
 8 files changed, 187 insertions(+), 35 deletions(-)

-- 
1.8.5.6

[U-Boot] [PATCH 1/5] efi_loader: Allow boards to implement get_time and reset_system

[Alexander Graf]

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 b52ba9c..53a6ee3 100644
--- a/cmd/bootefi.c
+++ b/cmd/bootefi.c
@@ -206,6 +206,10 @@ static unsigned long do_bootefi_exec(void *efi, void *fdt)
 		loaded_image_info.device_handle = nethandle;
 #endif
 
+	/* Initialize EFI runtime services */
+	efi_reset_system_init();
+	efi_get_time_init();
+
 	/* Call our payload! */
 	debug("%s:%d Jumping to 0x%lx\n", __func__, __LINE__, (long)entry);
 
diff --git a/include/efi_loader.h b/include/efi_loader.h
index 9738835..91d6a84 100644
--- a/include/efi_loader.h
+++ b/include/efi_loader.h
@@ -150,11 +150,29 @@ static inline void ascii2unicode(u16 *unicode, const 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 99b5ef1..e697226 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,
@@ -233,12 +272,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
@@ -251,6 +316,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
@@ -292,7 +371,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,
@@ -302,5 +381,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,
 };
-- 
1.8.5.6

[U-Boot] [PATCH 1/5] efi_loader: Allow boards to implement get_time and reset_system

[Simon Glass]

Hi Alex,

On 16 August 2016 at 13:08, Alexander Graf <agraf@suse.de> wrote:
> 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(-)

Instead of weak functions, can you use the existing driver model
sysreset uclass?

Regards,
Simon

[U-Boot] [PATCH 1/5] efi_loader: Allow boards to implement get_time and reset_system

[Alexander Graf]

> Am 18.08.2016 um 05:44 schrieb Simon Glass <sjg@chromium.org>:
> 
> Hi Alex,
> 
>> On 16 August 2016 at 13:08, Alexander Graf <agraf@suse.de> wrote:
>> 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(-)
> 
> Instead of weak functions, can you use the existing driver model
> sysreset uclass?

Unfortunately not, at least not generically.

The EFI RTS code gets dynamically relocated by the executing OS (like Linux) and expects any pointers to dynamically move to where the OS remaps things to. So memory at 0x1234 during boot time might become 0x75995234 in RTS.

While we can automatically catch all linker relocations, dynamically created objects or pointers that rely on the 1:1 map will fail to work once we're in Linux because of the above.

So the "obvious" path forward would be for systems that implement the sysreset uclass to provide an efi rts reset function which calls into their sysreset device. In a patch that enables this, whoever does it would also have to go through all the the dm code, annotate it as efi runtime and wrap all pointers in the dynamic efi relocation call. The same goes for pointers to device memory that needs to get accessed, which then also needs to get annotated as efi mmio regions in the efi memory map.

This is insanely tedious and very hard to get right. The less complexity we have in RTS code, the better we are off.

The less obvious - but imho more sane - route would be to just implement PSCI on armv8 systems. York has patches in the works to make U-Boot be a PSCI provider on armv8, armv7 already has PSCI provider code. In EL3 / TZ you can maintain your 1:1 map, don't need to swizzle pointers and could just reuse dm as much as you like. Linux gets secondary smp bootup (and core power off for power saving) for free and efi reset would "just work" with this patch set. Even better yet, it would "just work" without efi too ;).


Alex

[U-Boot] [PATCH 1/5] efi_loader: Allow boards to implement get_time and reset_system

[Simon Glass]

Hi Alex,

On 17 August 2016 at 22:49, Alexander Graf <agraf@suse.de> wrote:
>
>
>> Am 18.08.2016 um 05:44 schrieb Simon Glass <sjg@chromium.org>:
>>
>> Hi Alex,
>>
>>> On 16 August 2016 at 13:08, Alexander Graf <agraf@suse.de> wrote:
>>> 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(-)
>>
>> Instead of weak functions, can you use the existing driver model
>> sysreset uclass?
>
> Unfortunately not, at least not generically.
>
> The EFI RTS code gets dynamically relocated by the executing OS (like Linux) and expects any pointers to dynamically move to where the OS remaps things to. So memory at 0x1234 during boot time might become 0x75995234 in RTS.
>
> While we can automatically catch all linker relocations, dynamically created objects or pointers that rely on the 1:1 map will fail to work once we're in Linux because of the above.
>
> So the "obvious" path forward would be for systems that implement the sysreset uclass to provide an efi rts reset function which calls into their sysreset device. In a patch that enables this, whoever does it would also have to go through all the the dm code, annotate it as efi runtime and wrap all pointers in the dynamic efi relocation call. The same goes for pointers to device memory that needs to get accessed, which then also needs to get annotated as efi mmio regions in the efi memory map.
>
> This is insanely tedious and very hard to get right. The less complexity we have in RTS code, the better we are off.

Yes I thought this would be the issue. I've done something similar
before where part of U-Boot was linked to be in one image and another
part was loaded later only if needed. It was pretty brittle.

I agree that annotating functions that are used is tedious. In fact as
mentioned I'm really not keen on that in any case. It would be great
to drop the runtime annotation altogether.

Some options:
- Include all of U-Boot in the run-time portion (at least for now)
- Include drivers/{core,sysreset,sysreset} (which should be enough)
- link the image once with just the EFI loader binary and its
dependencies (using --gc-sections) and then used the list of included
object files as your list to put in the EFI runtime region

I suspect the middle one might be quite easy?

My concern with what you have is that you are working around driver
model, creating a second ad-hoc driver approach. I can certainly see
your reasons (short-term expediency mostly :-), but it might not be
that hard to just bring in the DM code that is needed. Remember that
each board should keep its driver in the correct drivers/...
directory, so this does not need to be board-specific.

>
> The less obvious - but imho more sane - route would be to just implement PSCI on armv8 systems. York has patches in the works to make U-Boot be a PSCI provider on armv8, armv7 already has PSCI provider code. In EL3 / TZ you can maintain your 1:1 map, don't need to swizzle pointers and could just reuse dm as much as you like. Linux gets secondary smp bootup (and core power off for power saving) for free and efi reset would "just work" with this patch set. Even better yet, it would "just work" without efi too ;).

Interesting - then ATF would not provide any run-time services? I
suppose that means that the code is maintained in two places, but it
sounds reasonable to me. It's probably not a lot of code.

Regards,
Simon

[U-Boot] [PATCH 1/5] efi_loader: Allow boards to implement get_time and reset_system

[Alexander Graf]

> On 18 Aug 2016, at 08:43, Simon Glass <sjg@chromium.org> wrote:
> 
> Hi Alex,
> 
> On 17 August 2016 at 22:49, Alexander Graf <agraf@suse.de> wrote:
>> 
>> 
>>> Am 18.08.2016 um 05:44 schrieb Simon Glass <sjg@chromium.org>:
>>> 
>>> Hi Alex,
>>> 
>>>> On 16 August 2016 at 13:08, Alexander Graf <agraf@suse.de> wrote:
>>>> 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(-)
>>> 
>>> Instead of weak functions, can you use the existing driver model
>>> sysreset uclass?
>> 
>> Unfortunately not, at least not generically.
>> 
>> The EFI RTS code gets dynamically relocated by the executing OS (like Linux) and expects any pointers to dynamically move to where the OS remaps things to. So memory at 0x1234 during boot time might become 0x75995234 in RTS.
>> 
>> While we can automatically catch all linker relocations, dynamically created objects or pointers that rely on the 1:1 map will fail to work once we're in Linux because of the above.
>> 
>> So the "obvious" path forward would be for systems that implement the sysreset uclass to provide an efi rts reset function which calls into their sysreset device. In a patch that enables this, whoever does it would also have to go through all the the dm code, annotate it as efi runtime and wrap all pointers in the dynamic efi relocation call. The same goes for pointers to device memory that needs to get accessed, which then also needs to get annotated as efi mmio regions in the efi memory map.
>> 
>> This is insanely tedious and very hard to get right. The less complexity we have in RTS code, the better we are off.
> 
> Yes I thought this would be the issue. I've done something similar
> before where part of U-Boot was linked to be in one image and another
> part was loaded later only if needed. It was pretty brittle.
> 
> I agree that annotating functions that are used is tedious. In fact as
> mentioned I'm really not keen on that in any case. It would be great
> to drop the runtime annotation altogether.
> 
> Some options:
> - Include all of U-Boot in the run-time portion (at least for now)
> - Include drivers/{core,sysreset,sysreset} (which should be enough)
> - link the image once with just the EFI loader binary and its
> dependencies (using --gc-sections) and then used the list of included
> object files as your list to put in the EFI runtime region
> 
> I suspect the middle one might be quite easy?

All of the above would be reasonably easy to do with linker scripts. The hard part are pointers that are not tracked by relocations, so we can?t easily relocate them when going into Linux land. And without that solved, I don?t think it?s worth thinking about over engineering run time support. I?d much prefer to put as much as we can in very trivial helpers, generically with PSCI where possible.

> My concern with what you have is that you are working around driver
> model, creating a second ad-hoc driver approach. I can certainly see
> your reasons (short-term expediency mostly :-), but it might not be
> that hard to just bring in the DM code that is needed. Remember that
> each board should keep its driver in the correct drivers/...
> directory, so this does not need to be board-specific.
> 
>> 
>> The less obvious - but imho more sane - route would be to just implement PSCI on armv8 systems. York has patches in the works to make U-Boot be a PSCI provider on armv8, armv7 already has PSCI provider code. In EL3 / TZ you can maintain your 1:1 map, don't need to swizzle pointers and could just reuse dm as much as you like. Linux gets secondary smp bootup (and core power off for power saving) for free and efi reset would "just work" with this patch set. Even better yet, it would "just work" without efi too ;).
> 
> Interesting - then ATF would not provide any run-time services? I
> suppose that means that the code is maintained in two places, but it
> sounds reasonable to me. It's probably not a lot of code.

Well, ATF would still provide PSCI which Linux can directly call into ;). But U-Boot wouldn?t have to maintain anything internally for that functionality.


Alex

[U-Boot] [PATCH 1/5] efi_loader: Allow boards to implement get_time and reset_system

[Simon Glass]

Hi Alex,

On 18 August 2016 at 15:02, Alexander Graf <agraf@suse.de> wrote:
>
>> On 18 Aug 2016, at 08:43, Simon Glass <sjg@chromium.org> wrote:
>>
>> Hi Alex,
>>
>> On 17 August 2016 at 22:49, Alexander Graf <agraf@suse.de> wrote:
>>>
>>>
>>>> Am 18.08.2016 um 05:44 schrieb Simon Glass <sjg@chromium.org>:
>>>>
>>>> Hi Alex,
>>>>
>>>>> On 16 August 2016 at 13:08, Alexander Graf <agraf@suse.de> wrote:
>>>>> 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(-)
>>>>
>>>> Instead of weak functions, can you use the existing driver model
>>>> sysreset uclass?
>>>
>>> Unfortunately not, at least not generically.
>>>
>>> The EFI RTS code gets dynamically relocated by the executing OS (like Linux) and expects any pointers to dynamically move to where the OS remaps things to. So memory at 0x1234 during boot time might become 0x75995234 in RTS.
>>>
>>> While we can automatically catch all linker relocations, dynamically created objects or pointers that rely on the 1:1 map will fail to work once we're in Linux because of the above.
>>>
>>> So the "obvious" path forward would be for systems that implement the sysreset uclass to provide an efi rts reset function which calls into their sysreset device. In a patch that enables this, whoever does it would also have to go through all the the dm code, annotate it as efi runtime and wrap all pointers in the dynamic efi relocation call. The same goes for pointers to device memory that needs to get accessed, which then also needs to get annotated as efi mmio regions in the efi memory map.
>>>
>>> This is insanely tedious and very hard to get right. The less complexity we have in RTS code, the better we are off.
>>
>> Yes I thought this would be the issue. I've done something similar
>> before where part of U-Boot was linked to be in one image and another
>> part was loaded later only if needed. It was pretty brittle.
>>
>> I agree that annotating functions that are used is tedious. In fact as
>> mentioned I'm really not keen on that in any case. It would be great
>> to drop the runtime annotation altogether.
>>
>> Some options:
>> - Include all of U-Boot in the run-time portion (at least for now)
>> - Include drivers/{core,sysreset,sysreset} (which should be enough)
>> - link the image once with just the EFI loader binary and its
>> dependencies (using --gc-sections) and then used the list of included
>> object files as your list to put in the EFI runtime region
>>
>> I suspect the middle one might be quite easy?
>
> All of the above would be reasonably easy to do with linker scripts. The hard part are pointers that are not tracked by relocations, so we can?t easily relocate them when going into Linux land. And without that solved, I don?t think it?s worth thinking about over engineering run time support. I?d much prefer to put as much as we can in very trivial helpers, generically with PSCI where possible.

Yes that's a whole other problem that I hadn't considered. It means
that the currently implementation is a barrier to adoption of driver
model. This really need some thought. How do you deal with pointers at
present? It may need do be a new execution phase, where we set up a
new global_data and init driver model. Pretty simple I suppose (we do
this in spl_init() at present), but I'm sure there would be fishhooks.

If we continue down the path you have created, it is only going to get
harder to fix this up. If I had known we would end up here I would
have been much less comfortable with the implementation.

Let's figure this out now. I'm happy to help with refactoring or
whatever is needed.

>
>> My concern with what you have is that you are working around driver
>> model, creating a second ad-hoc driver approach. I can certainly see
>> your reasons (short-term expediency mostly :-), but it might not be
>> that hard to just bring in the DM code that is needed. Remember that
>> each board should keep its driver in the correct drivers/...
>> directory, so this does not need to be board-specific.
>>
>>>
>>> The less obvious - but imho more sane - route would be to just implement PSCI on armv8 systems. York has patches in the works to make U-Boot be a PSCI provider on armv8, armv7 already has PSCI provider code. In EL3 / TZ you can maintain your 1:1 map, don't need to swizzle pointers and could just reuse dm as much as you like. Linux gets secondary smp bootup (and core power off for power saving) for free and efi reset would "just work" with this patch set. Even better yet, it would "just work" without efi too ;).
>>
>> Interesting - then ATF would not provide any run-time services? I
>> suppose that means that the code is maintained in two places, but it
>> sounds reasonable to me. It's probably not a lot of code.
>
> Well, ATF would still provide PSCI which Linux can directly call into ;). But U-Boot wouldn?t have to maintain anything internally for that functionality.

Ah I see, so in what way is U-Boot the provider in this case? Does it
mean U-Boot does not have run-time code anymore?

Regards,
Simon

[U-Boot] [U-Boot, 1/5] efi_loader: Allow boards to implement get_time and reset_system

[Alexander Graf]

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

Thanks, applied to 

[U-Boot] [PATCH 2/5] arm: Disable HVC PSCI calls by default

[Alexander Graf]

All systems that are running on armv8 are running bare metal with firmware
that implements PSCI running in EL3. That means we don't really need to expose
the hypercall variants of them.

This patch leaves the code in, but makes the code explicit enough to have the
compiler optimize it out. With this we don't need to worry about hvc vs smc
calling convention when calling psci helper functions.

Signed-off-by: Alexander Graf <agraf@suse.de>
---
 arch/arm/cpu/armv8/fwcall.c   | 14 +++++++++++---
 arch/arm/include/asm/system.h | 11 +----------
 arch/arm/mach-meson/board.c   |  2 +-
 board/xilinx/zynqmp/zynqmp.c  |  2 +-
 4 files changed, 14 insertions(+), 15 deletions(-)

diff --git a/arch/arm/cpu/armv8/fwcall.c b/arch/arm/cpu/armv8/fwcall.c
index 079e250..6bb68f2 100644
--- a/arch/arm/cpu/armv8/fwcall.c
+++ b/arch/arm/cpu/armv8/fwcall.c
@@ -17,7 +17,7 @@
  * x0~x7: input arguments
  * x0~x3: output arguments
  */
-void hvc_call(struct pt_regs *args)
+static void hvc_call(struct pt_regs *args)
 {
 	asm volatile(
 		"ldr x0, %0\n"
@@ -75,13 +75,21 @@ void smc_call(struct pt_regs *args)
 		  "x16", "x17");
 }
 
-void __noreturn psci_system_reset(bool conduit_smc)
+/*
+ * For now, all systems we support run at least in EL2 and thus
+ * trigger PSCI calls to EL3 using SMC. If anyone ever wants to
+ * use PSCI on U-Boot running below a hypervisor, please detect
+ * this and set the flag accordingly.
+ */
+static const bool use_smc_for_psci = true;
+
+void __noreturn psci_system_reset(void)
 {
 	struct pt_regs regs;
 
 	regs.regs[0] = ARM_PSCI_0_2_FN_SYSTEM_RESET;
 
-	if (conduit_smc)
+	if (use_smc_for_psci)
 		smc_call(&regs);
 	else
 		hvc_call(&regs);
diff --git a/arch/arm/include/asm/system.h b/arch/arm/include/asm/system.h
index 7b7b867..96f3912 100644
--- a/arch/arm/include/asm/system.h
+++ b/arch/arm/include/asm/system.h
@@ -107,15 +107,6 @@ void smp_kick_all_cpus(void);
 void flush_l3_cache(void);
 
 /*
- *Issue a hypervisor call in accordance with ARM "SMC Calling convention",
- * DEN0028A
- *
- * @args: input and output arguments
- *
- */
-void hvc_call(struct pt_regs *args);
-
-/*
  *Issue a secure monitor call in accordance with ARM "SMC Calling convention",
  * DEN0028A
  *
@@ -124,7 +115,7 @@ void hvc_call(struct pt_regs *args);
  */
 void smc_call(struct pt_regs *args);
 
-void __noreturn psci_system_reset(bool smc);
+void __noreturn psci_system_reset(void);
 
 #endif	/* __ASSEMBLY__ */
 
diff --git a/arch/arm/mach-meson/board.c b/arch/arm/mach-meson/board.c
index 1dd53e2..f159cbf 100644
--- a/arch/arm/mach-meson/board.c
+++ b/arch/arm/mach-meson/board.c
@@ -43,7 +43,7 @@ void dram_init_banksize(void)
 
 void reset_cpu(ulong addr)
 {
-	psci_system_reset(true);
+	psci_system_reset();
 }
 
 static struct mm_region gxbb_mem_map[] = {
diff --git a/board/xilinx/zynqmp/zynqmp.c b/board/xilinx/zynqmp/zynqmp.c
index 0c5d997..5108b94 100644
--- a/board/xilinx/zynqmp/zynqmp.c
+++ b/board/xilinx/zynqmp/zynqmp.c
@@ -310,5 +310,5 @@ int board_usb_cleanup(int index, enum usb_init_type init)
 
 void reset_misc(void)
 {
-	psci_system_reset(true);
+	psci_system_reset();
 }
-- 
1.8.5.6

[U-Boot] [PATCH 2/5] arm: Disable HVC PSCI calls by default

[Simon Glass]

On 16 August 2016 at 13:08, Alexander Graf <agraf@suse.de> wrote:
> All systems that are running on armv8 are running bare metal with firmware
> that implements PSCI running in EL3. That means we don't really need to expose
> the hypercall variants of them.
>
> This patch leaves the code in, but makes the code explicit enough to have the
> compiler optimize it out. With this we don't need to worry about hvc vs smc
> calling convention when calling psci helper functions.
>
> Signed-off-by: Alexander Graf <agraf@suse.de>
> ---
>  arch/arm/cpu/armv8/fwcall.c   | 14 +++++++++++---
>  arch/arm/include/asm/system.h | 11 +----------
>  arch/arm/mach-meson/board.c   |  2 +-
>  board/xilinx/zynqmp/zynqmp.c  |  2 +-
>  4 files changed, 14 insertions(+), 15 deletions(-)

Reviewed-by: Simon Glass <sjg@chromium.org>

[U-Boot] [U-Boot,2/5] arm: Disable HVC PSCI calls by default

[Alexander Graf]

> All systems that are running on armv8 are running bare metal with firmware
> that implements PSCI running in EL3. That means we don't really need to expose
> the hypercall variants of them.
> 
> This patch leaves the code in, but makes the code explicit enough to have the
> compiler optimize it out. With this we don't need to worry about hvc vs smc
> calling convention when calling psci helper functions.
> 
> Signed-off-by: Alexander Graf <agraf@suse.de>
> Reviewed-by: Simon Glass <sjg@chromium.org>

Thanks, applied to 

[U-Boot] [PATCH 3/5] arm: Add PSCI shutdown function

[Alexander Graf]

Using PSCI you can not only reset the system, you can also shut it down!
This patch exposes a function to do exactly that to whatever code wants
to make use of it.

Signed-off-by: Alexander Graf <agraf@suse.de>
---
 arch/arm/cpu/armv8/fwcall.c   | 15 +++++++++++++++
 arch/arm/include/asm/system.h |  1 +
 2 files changed, 16 insertions(+)

diff --git a/arch/arm/cpu/armv8/fwcall.c b/arch/arm/cpu/armv8/fwcall.c
index 6bb68f2..b3ef7c0 100644
--- a/arch/arm/cpu/armv8/fwcall.c
+++ b/arch/arm/cpu/armv8/fwcall.c
@@ -97,3 +97,18 @@ void __noreturn psci_system_reset(void)
 	while (1)
 		;
 }
+
+void __noreturn psci_system_off(void)
+{
+	struct pt_regs regs;
+
+	regs.regs[0] = ARM_PSCI_0_2_FN_SYSTEM_OFF;
+
+	if (use_smc_for_psci)
+		smc_call(&regs);
+	else
+		hvc_call(&regs);
+
+	while (1)
+		;
+}
diff --git a/arch/arm/include/asm/system.h b/arch/arm/include/asm/system.h
index 96f3912..211b1e7 100644
--- a/arch/arm/include/asm/system.h
+++ b/arch/arm/include/asm/system.h
@@ -116,6 +116,7 @@ void flush_l3_cache(void);
 void smc_call(struct pt_regs *args);
 
 void __noreturn psci_system_reset(void);
+void __noreturn psci_system_off(void);
 
 #endif	/* __ASSEMBLY__ */
 
-- 
1.8.5.6

[U-Boot] [PATCH 3/5] arm: Add PSCI shutdown function

[Simon Glass]

On 16 August 2016 at 13:08, Alexander Graf <agraf@suse.de> wrote:
> Using PSCI you can not only reset the system, you can also shut it down!
> This patch exposes a function to do exactly that to whatever code wants
> to make use of it.
>
> Signed-off-by: Alexander Graf <agraf@suse.de>
> ---
>  arch/arm/cpu/armv8/fwcall.c   | 15 +++++++++++++++
>  arch/arm/include/asm/system.h |  1 +
>  2 files changed, 16 insertions(+)

Reviewed-by: Simon Glass <sjg@chromium.org>

[U-Boot] [U-Boot,3/5] arm: Add PSCI shutdown function

[Alexander Graf]

> Using PSCI you can not only reset the system, you can also shut it down!
> This patch exposes a function to do exactly that to whatever code wants
> to make use of it.
> 
> Signed-off-by: Alexander Graf <agraf@suse.de>
> Reviewed-by: Simon Glass <sjg@chromium.org>

Thanks, applied to 

[U-Boot] [PATCH 4/5] arm: Provide common PSCI based reset handler

[Alexander Graf]

Most armv8 systems have PSCI support enabled in EL3, either through
ARM Trusted Firmware or other firmware.

On these systems, we do not need to implement system reset manually,
but can instead rely on higher level firmware to deal with it.

The exclude list seems excessive right now, but NXP is working on
providing an in-tree PSCI implementation, so that all NXP systems
can eventually use PSCI as well.

Signed-off-by: Alexander Graf <agraf@suse.de>
---
 arch/arm/cpu/armv8/Kconfig   | 18 ++++++++++++++++++
 arch/arm/cpu/armv8/fwcall.c  |  7 +++++++
 arch/arm/mach-meson/board.c  |  5 -----
 board/xilinx/zynqmp/zynqmp.c |  5 -----
 4 files changed, 25 insertions(+), 10 deletions(-)

diff --git a/arch/arm/cpu/armv8/Kconfig b/arch/arm/cpu/armv8/Kconfig
index 7e1fc4c..cd2d9bb 100644
--- a/arch/arm/cpu/armv8/Kconfig
+++ b/arch/arm/cpu/armv8/Kconfig
@@ -21,4 +21,22 @@ config ARMV8_SPIN_TABLE
 	    - Reserve the code for the spin-table and the release address
 	      via a /memreserve/ region in the Device Tree.
 
+config PSCI_RESET
+	bool "Use PSCI for reset and shutdown"
+	default y
+	depends on !ARCH_EXYNOS7 && !ARCH_BCM283X && !TARGET_LS2080A_EMU && \
+		   !TARGET_LS2080A_SIMU && !TARGET_LS2080AQDS && \
+		   !TARGET_LS2080ARDB && !TARGET_LS1012AQDS && \
+		   !TARGET_LS1012ARDB && !TARGET_LS1012AFRDM && \
+		   !TARGET_LS1043ARDB && !ARCH_UNIPHIER && !ARCH_SNAPDRAGON && \
+		   !TARGET_S32V234EVB
+	help
+	  Most armv8 systems have PSCI support enabled in EL3, either through
+	  ARM Trusted Firmware or other firmware.
+
+	  On these systems, we do not need to implement system reset manually,
+	  but can instead rely on higher level firmware to deal with it.
+
+	  Select Y here to make use of PSCI calls for system reset
+
 endif
diff --git a/arch/arm/cpu/armv8/fwcall.c b/arch/arm/cpu/armv8/fwcall.c
index b3ef7c0..c57b15f 100644
--- a/arch/arm/cpu/armv8/fwcall.c
+++ b/arch/arm/cpu/armv8/fwcall.c
@@ -112,3 +112,10 @@ void __noreturn psci_system_off(void)
 	while (1)
 		;
 }
+
+#ifdef CONFIG_PSCI_RESET
+void reset_misc(void)
+{
+	psci_system_reset();
+}
+#endif /* CONFIG_PSCI_RESET */
diff --git a/arch/arm/mach-meson/board.c b/arch/arm/mach-meson/board.c
index f159cbf..775872d 100644
--- a/arch/arm/mach-meson/board.c
+++ b/arch/arm/mach-meson/board.c
@@ -41,11 +41,6 @@ void dram_init_banksize(void)
 	gd->bd->bi_dram[0].size = gd->ram_size - (16 * 1024 * 1024);
 }
 
-void reset_cpu(ulong addr)
-{
-	psci_system_reset();
-}
-
 static struct mm_region gxbb_mem_map[] = {
 	{
 		.virt = 0x0UL,
diff --git a/board/xilinx/zynqmp/zynqmp.c b/board/xilinx/zynqmp/zynqmp.c
index 5108b94..f15dc5d 100644
--- a/board/xilinx/zynqmp/zynqmp.c
+++ b/board/xilinx/zynqmp/zynqmp.c
@@ -307,8 +307,3 @@ int board_usb_cleanup(int index, enum usb_init_type init)
 	return 0;
 }
 #endif
-
-void reset_misc(void)
-{
-	psci_system_reset();
-}
-- 
1.8.5.6

[U-Boot] [PATCH 4/5] arm: Provide common PSCI based reset handler

[Simon Glass]

Hi Alex,

On 16 August 2016 at 13:08, Alexander Graf <agraf@suse.de> wrote:
> Most armv8 systems have PSCI support enabled in EL3, either through
> ARM Trusted Firmware or other firmware.
>
> On these systems, we do not need to implement system reset manually,
> but can instead rely on higher level firmware to deal with it.
>
> The exclude list seems excessive right now, but NXP is working on
> providing an in-tree PSCI implementation, so that all NXP systems
> can eventually use PSCI as well.
>
> Signed-off-by: Alexander Graf <agraf@suse.de>
> ---
>  arch/arm/cpu/armv8/Kconfig   | 18 ++++++++++++++++++
>  arch/arm/cpu/armv8/fwcall.c  |  7 +++++++
>  arch/arm/mach-meson/board.c  |  5 -----
>  board/xilinx/zynqmp/zynqmp.c |  5 -----
>  4 files changed, 25 insertions(+), 10 deletions(-)

Reviewed-by: Simon Glass <sjg@chromium.org>

>
> diff --git a/arch/arm/cpu/armv8/Kconfig b/arch/arm/cpu/armv8/Kconfig
> index 7e1fc4c..cd2d9bb 100644
> --- a/arch/arm/cpu/armv8/Kconfig
> +++ b/arch/arm/cpu/armv8/Kconfig
> @@ -21,4 +21,22 @@ config ARMV8_SPIN_TABLE
>             - Reserve the code for the spin-table and the release address
>               via a /memreserve/ region in the Device Tree.
>
> +config PSCI_RESET
> +       bool "Use PSCI for reset and shutdown"
> +       default y
> +       depends on !ARCH_EXYNOS7 && !ARCH_BCM283X && !TARGET_LS2080A_EMU && \
> +                  !TARGET_LS2080A_SIMU && !TARGET_LS2080AQDS && \
> +                  !TARGET_LS2080ARDB && !TARGET_LS1012AQDS && \
> +                  !TARGET_LS1012ARDB && !TARGET_LS1012AFRDM && \
> +                  !TARGET_LS1043ARDB && !ARCH_UNIPHIER && !ARCH_SNAPDRAGON && \
> +                  !TARGET_S32V234EVB

Will this break new systems that are added?

> +       help
> +         Most armv8 systems have PSCI support enabled in EL3, either through
> +         ARM Trusted Firmware or other firmware.
> +
> +         On these systems, we do not need to implement system reset manually,
> +         but can instead rely on higher level firmware to deal with it.
> +
> +         Select Y here to make use of PSCI calls for system reset
> +
>  endif
> diff --git a/arch/arm/cpu/armv8/fwcall.c b/arch/arm/cpu/armv8/fwcall.c
> index b3ef7c0..c57b15f 100644
> --- a/arch/arm/cpu/armv8/fwcall.c
> +++ b/arch/arm/cpu/armv8/fwcall.c
> @@ -112,3 +112,10 @@ void __noreturn psci_system_off(void)
>         while (1)
>                 ;
>  }
> +
> +#ifdef CONFIG_PSCI_RESET
> +void reset_misc(void)
> +{
> +       psci_system_reset();
> +}
> +#endif /* CONFIG_PSCI_RESET */
> diff --git a/arch/arm/mach-meson/board.c b/arch/arm/mach-meson/board.c
> index f159cbf..775872d 100644
> --- a/arch/arm/mach-meson/board.c
> +++ b/arch/arm/mach-meson/board.c
> @@ -41,11 +41,6 @@ void dram_init_banksize(void)
>         gd->bd->bi_dram[0].size = gd->ram_size - (16 * 1024 * 1024);
>  }
>
> -void reset_cpu(ulong addr)
> -{
> -       psci_system_reset();
> -}
> -
>  static struct mm_region gxbb_mem_map[] = {
>         {
>                 .virt = 0x0UL,
> diff --git a/board/xilinx/zynqmp/zynqmp.c b/board/xilinx/zynqmp/zynqmp.c
> index 5108b94..f15dc5d 100644
> --- a/board/xilinx/zynqmp/zynqmp.c
> +++ b/board/xilinx/zynqmp/zynqmp.c
> @@ -307,8 +307,3 @@ int board_usb_cleanup(int index, enum usb_init_type init)
>         return 0;
>  }
>  #endif
> -
> -void reset_misc(void)
> -{
> -       psci_system_reset();
> -}
> --
> 1.8.5.6
>

Regards,
Simon

[U-Boot] [PATCH 4/5] arm: Provide common PSCI based reset handler

[Alexander Graf]

> Am 18.08.2016 um 05:44 schrieb Simon Glass <sjg@chromium.org>:
> 
> Hi Alex,
> 
>> On 16 August 2016 at 13:08, Alexander Graf <agraf@suse.de> wrote:
>> Most armv8 systems have PSCI support enabled in EL3, either through
>> ARM Trusted Firmware or other firmware.
>> 
>> On these systems, we do not need to implement system reset manually,
>> but can instead rely on higher level firmware to deal with it.
>> 
>> The exclude list seems excessive right now, but NXP is working on
>> providing an in-tree PSCI implementation, so that all NXP systems
>> can eventually use PSCI as well.
>> 
>> Signed-off-by: Alexander Graf <agraf@suse.de>
>> ---
>> arch/arm/cpu/armv8/Kconfig   | 18 ++++++++++++++++++
>> arch/arm/cpu/armv8/fwcall.c  |  7 +++++++
>> arch/arm/mach-meson/board.c  |  5 -----
>> board/xilinx/zynqmp/zynqmp.c |  5 -----
>> 4 files changed, 25 insertions(+), 10 deletions(-)
> 
> Reviewed-by: Simon Glass <sjg@chromium.org>
> 
>> 
>> diff --git a/arch/arm/cpu/armv8/Kconfig b/arch/arm/cpu/armv8/Kconfig
>> index 7e1fc4c..cd2d9bb 100644
>> --- a/arch/arm/cpu/armv8/Kconfig
>> +++ b/arch/arm/cpu/armv8/Kconfig
>> @@ -21,4 +21,22 @@ config ARMV8_SPIN_TABLE
>>            - Reserve the code for the spin-table and the release address
>>              via a /memreserve/ region in the Device Tree.
>> 
>> +config PSCI_RESET
>> +       bool "Use PSCI for reset and shutdown"
>> +       default y
>> +       depends on !ARCH_EXYNOS7 && !ARCH_BCM283X && !TARGET_LS2080A_EMU && \
>> +                  !TARGET_LS2080A_SIMU && !TARGET_LS2080AQDS && \
>> +                  !TARGET_LS2080ARDB && !TARGET_LS1012AQDS && \
>> +                  !TARGET_LS1012ARDB && !TARGET_LS1012AFRDM && \
>> +                  !TARGET_LS1043ARDB && !ARCH_UNIPHIER && !ARCH_SNAPDRAGON && \
>> +                  !TARGET_S32V234EVB
> 
> Will this break new systems that are added?

I would word it differently - it will gently push new systems into the right direction ;). With PSCI available, Linux SMP will "just work" too.

The worst that can happen is that someone enables a nee system which does not provide PSCI. In that case we have non-working reboot and shutdown handlers. The alternative would be no reboot and shutdown handlers (which is a fatal panic on Linux for example), so we're not worse off imho.


Alex

[U-Boot] [PATCH 4/5] arm: Provide common PSCI based reset handler

[Simon Glass]

Hi Alex,

On 17 August 2016 at 22:00, Alexander Graf <agraf@suse.de> wrote:
>
>
>> Am 18.08.2016 um 05:44 schrieb Simon Glass <sjg@chromium.org>:
>>
>> Hi Alex,
>>
>>> On 16 August 2016 at 13:08, Alexander Graf <agraf@suse.de> wrote:
>>> Most armv8 systems have PSCI support enabled in EL3, either through
>>> ARM Trusted Firmware or other firmware.
>>>
>>> On these systems, we do not need to implement system reset manually,
>>> but can instead rely on higher level firmware to deal with it.
>>>
>>> The exclude list seems excessive right now, but NXP is working on
>>> providing an in-tree PSCI implementation, so that all NXP systems
>>> can eventually use PSCI as well.
>>>
>>> Signed-off-by: Alexander Graf <agraf@suse.de>
>>> ---
>>> arch/arm/cpu/armv8/Kconfig   | 18 ++++++++++++++++++
>>> arch/arm/cpu/armv8/fwcall.c  |  7 +++++++
>>> arch/arm/mach-meson/board.c  |  5 -----
>>> board/xilinx/zynqmp/zynqmp.c |  5 -----
>>> 4 files changed, 25 insertions(+), 10 deletions(-)
>>
>> Reviewed-by: Simon Glass <sjg@chromium.org>
>>
>>>
>>> diff --git a/arch/arm/cpu/armv8/Kconfig b/arch/arm/cpu/armv8/Kconfig
>>> index 7e1fc4c..cd2d9bb 100644
>>> --- a/arch/arm/cpu/armv8/Kconfig
>>> +++ b/arch/arm/cpu/armv8/Kconfig
>>> @@ -21,4 +21,22 @@ config ARMV8_SPIN_TABLE
>>>            - Reserve the code for the spin-table and the release address
>>>              via a /memreserve/ region in the Device Tree.
>>>
>>> +config PSCI_RESET
>>> +       bool "Use PSCI for reset and shutdown"
>>> +       default y
>>> +       depends on !ARCH_EXYNOS7 && !ARCH_BCM283X && !TARGET_LS2080A_EMU && \
>>> +                  !TARGET_LS2080A_SIMU && !TARGET_LS2080AQDS && \
>>> +                  !TARGET_LS2080ARDB && !TARGET_LS1012AQDS && \
>>> +                  !TARGET_LS1012ARDB && !TARGET_LS1012AFRDM && \
>>> +                  !TARGET_LS1043ARDB && !ARCH_UNIPHIER && !ARCH_SNAPDRAGON && \
>>> +                  !TARGET_S32V234EVB
>>
>> Will this break new systems that are added?
>
> I would word it differently - it will gently push new systems into the right direction ;). With PSCI available, Linux SMP will "just work" too.
>
> The worst that can happen is that someone enables a nee system which does not provide PSCI. In that case we have non-working reboot and shutdown handlers. The alternative would be no reboot and shutdown handlers (which is a fatal panic on Linux for example), so we're not worse off imho.

Sounds good.

Regards,
Simon

[U-Boot] [PATCH 4/5] arm: Provide common PSCI based reset handler

[Michal Simek]

Hi Alex,

On 16.8.2016 21:08, Alexander Graf wrote:
> Most armv8 systems have PSCI support enabled in EL3, either through
> ARM Trusted Firmware or other firmware.
> 
> On these systems, we do not need to implement system reset manually,
> but can instead rely on higher level firmware to deal with it.
> 
> The exclude list seems excessive right now, but NXP is working on
> providing an in-tree PSCI implementation, so that all NXP systems
> can eventually use PSCI as well.
> 
> Signed-off-by: Alexander Graf <agraf@suse.de>
> ---
>  arch/arm/cpu/armv8/Kconfig   | 18 ++++++++++++++++++
>  arch/arm/cpu/armv8/fwcall.c  |  7 +++++++
>  arch/arm/mach-meson/board.c  |  5 -----
>  board/xilinx/zynqmp/zynqmp.c |  5 -----
>  4 files changed, 25 insertions(+), 10 deletions(-)
> 
> diff --git a/arch/arm/cpu/armv8/Kconfig b/arch/arm/cpu/armv8/Kconfig
> index 7e1fc4c..cd2d9bb 100644
> --- a/arch/arm/cpu/armv8/Kconfig
> +++ b/arch/arm/cpu/armv8/Kconfig
> @@ -21,4 +21,22 @@ config ARMV8_SPIN_TABLE
>  	    - Reserve the code for the spin-table and the release address
>  	      via a /memreserve/ region in the Device Tree.
>  
> +config PSCI_RESET
> +	bool "Use PSCI for reset and shutdown"
> +	default y
> +	depends on !ARCH_EXYNOS7 && !ARCH_BCM283X && !TARGET_LS2080A_EMU && \
> +		   !TARGET_LS2080A_SIMU && !TARGET_LS2080AQDS && \
> +		   !TARGET_LS2080ARDB && !TARGET_LS1012AQDS && \
> +		   !TARGET_LS1012ARDB && !TARGET_LS1012AFRDM && \
> +		   !TARGET_LS1043ARDB && !ARCH_UNIPHIER && !ARCH_SNAPDRAGON && \
> +		   !TARGET_S32V234EVB
> +	help
> +	  Most armv8 systems have PSCI support enabled in EL3, either through
> +	  ARM Trusted Firmware or other firmware.
> +
> +	  On these systems, we do not need to implement system reset manually,
> +	  but can instead rely on higher level firmware to deal with it.
> +
> +	  Select Y here to make use of PSCI calls for system reset
> +
>  endif
> diff --git a/arch/arm/cpu/armv8/fwcall.c b/arch/arm/cpu/armv8/fwcall.c
> index b3ef7c0..c57b15f 100644
> --- a/arch/arm/cpu/armv8/fwcall.c
> +++ b/arch/arm/cpu/armv8/fwcall.c
> @@ -112,3 +112,10 @@ void __noreturn psci_system_off(void)
>  	while (1)
>  		;
>  }
> +
> +#ifdef CONFIG_PSCI_RESET
> +void reset_misc(void)
> +{
> +	psci_system_reset();


maybe we should also consider to check which level we are running at.

if (current_el() != 3) {
...
}

But maybe this should be in checked in smc_call and hvc_call.

Thanks,
Michal

[U-Boot] [U-Boot, 4/5] arm: Provide common PSCI based reset handler

[Alexander Graf]

> Most armv8 systems have PSCI support enabled in EL3, either through
> ARM Trusted Firmware or other firmware.
> 
> On these systems, we do not need to implement system reset manually,
> but can instead rely on higher level firmware to deal with it.
> 
> The exclude list seems excessive right now, but NXP is working on
> providing an in-tree PSCI implementation, so that all NXP systems
> can eventually use PSCI as well.
> 
> Signed-off-by: Alexander Graf <agraf@suse.de>
> Reviewed-by: Simon Glass <sjg@chromium.org>

Thanks, applied to 

[U-Boot] [PATCH 5/5] efi_loader: Add generic PSCI RTS

[Alexander Graf]

Now that we have generic PSCI reset and shutdown support in place, we can
advertise those as EFI Run Time Services, allowing efi applications and
OSs to reset and shut down systems.

Signed-off-by: Alexander Graf <agraf@suse.de>
---
 arch/arm/cpu/armv8/fwcall.c | 31 ++++++++++++++++++++++++++-----
 1 file changed, 26 insertions(+), 5 deletions(-)

diff --git a/arch/arm/cpu/armv8/fwcall.c b/arch/arm/cpu/armv8/fwcall.c
index c57b15f..64539f9 100644
--- a/arch/arm/cpu/armv8/fwcall.c
+++ b/arch/arm/cpu/armv8/fwcall.c
@@ -6,6 +6,7 @@
 
 #include <asm-offsets.h>
 #include <config.h>
+#include <efi_loader.h>
 #include <version.h>
 #include <asm/macro.h>
 #include <asm/psci.h>
@@ -17,7 +18,7 @@
  * x0~x7: input arguments
  * x0~x3: output arguments
  */
-static void hvc_call(struct pt_regs *args)
+static void EFI_RUNTIME_TEXT hvc_call(struct pt_regs *args)
 {
 	asm volatile(
 		"ldr x0, %0\n"
@@ -51,7 +52,7 @@ static void hvc_call(struct pt_regs *args)
  * x0~x3: output arguments
  */
 
-void smc_call(struct pt_regs *args)
+void EFI_RUNTIME_TEXT smc_call(struct pt_regs *args)
 {
 	asm volatile(
 		"ldr x0, %0\n"
@@ -81,9 +82,9 @@ void smc_call(struct pt_regs *args)
  * use PSCI on U-Boot running below a hypervisor, please detect
  * this and set the flag accordingly.
  */
-static const bool use_smc_for_psci = true;
+static const EFI_RUNTIME_DATA bool use_smc_for_psci = true;
 
-void __noreturn psci_system_reset(void)
+void __noreturn EFI_RUNTIME_TEXT psci_system_reset(void)
 {
 	struct pt_regs regs;
 
@@ -98,7 +99,7 @@ void __noreturn psci_system_reset(void)
 		;
 }
 
-void __noreturn psci_system_off(void)
+void __noreturn EFI_RUNTIME_TEXT psci_system_off(void)
 {
 	struct pt_regs regs;
 
@@ -118,4 +119,24 @@ void reset_misc(void)
 {
 	psci_system_reset();
 }
+
+#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)
+{
+	switch (reset_type) {
+	case EFI_RESET_COLD:
+	case EFI_RESET_WARM:
+		psci_system_reset();
+		break;
+	case EFI_RESET_SHUTDOWN:
+		psci_system_off();
+		break;
+	}
+
+	while (1) { }
+}
+#endif /* CONFIG_EFI_LOADER */
 #endif /* CONFIG_PSCI_RESET */
-- 
1.8.5.6

[U-Boot] [PATCH 5/5] efi_loader: Add generic PSCI RTS

[Simon Glass]

On 16 August 2016 at 13:08, Alexander Graf <agraf@suse.de> wrote:
> Now that we have generic PSCI reset and shutdown support in place, we can
> advertise those as EFI Run Time Services, allowing efi applications and
> OSs to reset and shut down systems.
>
> Signed-off-by: Alexander Graf <agraf@suse.de>
> ---
>  arch/arm/cpu/armv8/fwcall.c | 31 ++++++++++++++++++++++++++-----
>  1 file changed, 26 insertions(+), 5 deletions(-)

Reviewed-by: Simon Glass <sjg@chromium.org>

(assuming you are going to do a patch to rename EFI_RUNTIME_TEXT)

[U-Boot] [PATCH 5/5] efi_loader: Add generic PSCI RTS

[Alexander Graf]

> Am 18.08.2016 um 05:44 schrieb Simon Glass <sjg@chromium.org>:
> 
>> On 16 August 2016 at 13:08, Alexander Graf <agraf@suse.de> wrote:
>> Now that we have generic PSCI reset and shutdown support in place, we can
>> advertise those as EFI Run Time Services, allowing efi applications and
>> OSs to reset and shut down systems.
>> 
>> Signed-off-by: Alexander Graf <agraf@suse.de>
>> ---
>> arch/arm/cpu/armv8/fwcall.c | 31 ++++++++++++++++++++++++++-----
>> 1 file changed, 26 insertions(+), 5 deletions(-)
> 
> Reviewed-by: Simon Glass <sjg@chromium.org>
> 
> (assuming you are going to do a patch to rename EFI_RUNTIME_TEXT)

Yes, but with the number of patches in flight that use the macro, I don't want to hold up functional improvements for cosmetics just yet. Once this is merged I'll rename.

Alex

[U-Boot] [PATCH 5/5] efi_loader: Add generic PSCI RTS

[Simon Glass]

Hi Alex,

On 17 August 2016 at 22:01, Alexander Graf <agraf@suse.de> wrote:
>
>
>> Am 18.08.2016 um 05:44 schrieb Simon Glass <sjg@chromium.org>:
>>
>>> On 16 August 2016 at 13:08, Alexander Graf <agraf@suse.de> wrote:
>>> Now that we have generic PSCI reset and shutdown support in place, we can
>>> advertise those as EFI Run Time Services, allowing efi applications and
>>> OSs to reset and shut down systems.
>>>
>>> Signed-off-by: Alexander Graf <agraf@suse.de>
>>> ---
>>> arch/arm/cpu/armv8/fwcall.c | 31 ++++++++++++++++++++++++++-----
>>> 1 file changed, 26 insertions(+), 5 deletions(-)
>>
>> Reviewed-by: Simon Glass <sjg@chromium.org>
>>
>> (assuming you are going to do a patch to rename EFI_RUNTIME_TEXT)
>
> Yes, but with the number of patches in flight that use the macro, I don't want to hold up functional improvements for cosmetics just yet. Once this is merged I'll rename.

OK that's what I thought.

Regards,
Simon

[U-Boot] [U-Boot,5/5] efi_loader: Add generic PSCI RTS

[Alexander Graf]

> Now that we have generic PSCI reset and shutdown support in place, we can
> advertise those as EFI Run Time Services, allowing efi applications and
> OSs to reset and shut down systems.
> 
> Signed-off-by: Alexander Graf <agraf@suse.de>
> Reviewed-by: Simon Glass <sjg@chromium.org>

Thanks, applied to 

[U-Boot] [PATCH 0/5] armv8: Add generic PSCI client support

[Michal Simek]

On 16.8.2016 21:08, Alexander Graf wrote:
> On aarch64 systems, Linux only has managed to push almost everyone to using
> PSCI as standardized interface for CPU start/stop operations. Fortunately
> PSCI also gives us a nicely abstracted interface to reset and power off
> the system.
> 
> This patch set adds support for generic PSCI reset/shutdown support and ties
> it up to EFI Run Time Services, so that an OS like Linux will magically have
> reset and shutdown work.
> 
> Alexander Graf (5):
>   efi_loader: Allow boards to implement get_time and reset_system
>   arm: Disable HVC PSCI calls by default
>   arm: Add PSCI shutdown function
>   arm: Provide common PSCI based reset handler
>   efi_loader: Add generic PSCI RTS
> 
>  arch/arm/cpu/armv8/Kconfig    |  18 ++++++++
>  arch/arm/cpu/armv8/fwcall.c   |  59 ++++++++++++++++++++++--
>  arch/arm/include/asm/system.h |  12 +----
>  arch/arm/mach-meson/board.c   |   5 ---
>  board/xilinx/zynqmp/zynqmp.c  |   5 ---
>  cmd/bootefi.c                 |   4 ++
>  include/efi_loader.h          |  18 ++++++++
>  lib/efi_loader/efi_runtime.c  | 101 +++++++++++++++++++++++++++++++++++++-----
>  8 files changed, 187 insertions(+), 35 deletions(-)
> 

btw: I think that efi should be out of this series as separate patch.

Thanks,
Michal