diff mbox series

[U-Boot,v2,18/18] efi_loader: fix ExitBootServices

Message ID 20180117191612.17108-19-xypron.glpk@gmx.de
State Superseded, archived
Delegated to: Alexander Graf
Headers show
Series efi_loader: enable EFI driver provided block device | expand

Commit Message

Heinrich Schuchardt Jan. 17, 2018, 7:16 p.m. UTC
This patch lets the implementation of ExitBootServices conform to
the UEFI standard.

The timer events must be disabled before calling the notification
functions of the exit boot services events.

The boot services must be disabled in the system table.

The handles in the system table should be defined as efi_handle_t.

Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
---
v2
	new patch
---
 include/efi_api.h             |  6 +++---
 lib/efi_loader/efi_boottime.c | 36 +++++++++++++++++++++++++++++++-----
 2 files changed, 34 insertions(+), 8 deletions(-)

Comments

Alexander Graf Jan. 19, 2018, 10:56 a.m. UTC | #1
On 17.01.18 20:16, Heinrich Schuchardt wrote:
> This patch lets the implementation of ExitBootServices conform to
> the UEFI standard.
> 
> The timer events must be disabled before calling the notification
> functions of the exit boot services events.
> 
> The boot services must be disabled in the system table.
> 
> The handles in the system table should be defined as efi_handle_t.
> 
> Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
> ---
> v2
> 	new patch
> ---
>  include/efi_api.h             |  6 +++---
>  lib/efi_loader/efi_boottime.c | 36 +++++++++++++++++++++++++++++++-----
>  2 files changed, 34 insertions(+), 8 deletions(-)
> 
> diff --git a/include/efi_api.h b/include/efi_api.h
> index 0bc244444d..4252d11398 100644
> --- a/include/efi_api.h
> +++ b/include/efi_api.h
> @@ -247,11 +247,11 @@ struct efi_system_table {
>  	struct efi_table_hdr hdr;
>  	unsigned long fw_vendor;   /* physical addr of wchar_t vendor string */
>  	u32 fw_revision;
> -	unsigned long con_in_handle;
> +	efi_handle_t con_in_handle;
>  	struct efi_simple_input_interface *con_in;
> -	unsigned long con_out_handle;
> +	efi_handle_t con_out_handle;
>  	struct efi_simple_text_output_protocol *con_out;
> -	unsigned long stderr_handle;
> +	efi_handle_t stderr_handle;
>  	struct efi_simple_text_output_protocol *std_err;
>  	struct efi_runtime_services *runtime;
>  	struct efi_boot_services *boottime;
> diff --git a/lib/efi_loader/efi_boottime.c b/lib/efi_loader/efi_boottime.c
> index 4b3b63e39a..2c5499e0c8 100644
> --- a/lib/efi_loader/efi_boottime.c
> +++ b/lib/efi_loader/efi_boottime.c
> @@ -1645,12 +1645,16 @@ static void efi_exit_caches(void)
>  }
>  
>  /*
> - * Stop boot services.
> + * Stop all boot services.
>   *
>   * This function implements the ExitBootServices service.
>   * See the Unified Extensible Firmware Interface (UEFI) specification
>   * for details.
>   *
> + * All timer events are disabled.
> + * For exit boot services events the notification function is called.
> + * The boot services are disabled in the system table.
> + *
>   * @image_handle	handle of the loaded image
>   * @map_key		key of the memory map
>   * @return		status code
> @@ -1662,16 +1666,24 @@ static efi_status_t EFIAPI efi_exit_boot_services(efi_handle_t image_handle,
>  
>  	EFI_ENTRY("%p, %ld", image_handle, map_key);
>  
> +	/* Make sure that notification functions are not called anymore */
> +	efi_tpl = TPL_HIGH_LEVEL;
> +
> +	/* Check if ExitBootServices has already been called */
> +	if (!systab.boottime)
> +		return EFI_EXIT(EFI_SUCCESS);
> +
>  	/* Notify that ExitBootServices is invoked. */
>  	for (i = 0; i < ARRAY_SIZE(efi_events); ++i) {
>  		if (efi_events[i].type != EVT_SIGNAL_EXIT_BOOT_SERVICES)
>  			continue;
> -		efi_signal_event(&efi_events[i]);
> +		if (!efi_events[i].notify_function)
> +			continue;
> +		EFI_CALL_VOID(efi_events[i].notify_function(
> +				&efi_events[i], efi_events[i].notify_context));

This basically just bypasses the event->notify_tpl check. Can't you add
an enum parameter to efi_signal_event to indicate EFI_SIGNAL_CHECK_TPL
vs EFI_SIGNAL_NOCHECK and only do the TPL check in there if
EFI_SIGNAL_CHECK_TPL is set?


Alex

>  	}
> -	/* Make sure that notification functions are not called anymore */
> -	efi_tpl = TPL_HIGH_LEVEL;
>  
> -	/* XXX Should persist EFI variables here */
> +	/* TODO Should persist EFI variables here */
>  
>  	board_quiesce_devices();
>  
> @@ -1681,6 +1693,20 @@ static efi_status_t EFIAPI efi_exit_boot_services(efi_handle_t image_handle,
>  	/* This stops all lingering devices */
>  	bootm_disable_interrupts();
>  
> +	/* Disable boottime services */
> +	systab.con_in_handle = NULL;
> +	systab.con_in = NULL;
> +	systab.con_out_handle = NULL;
> +	systab.con_out = NULL;
> +	systab.stderr_handle = NULL;
> +	systab.std_err = NULL;
> +	systab.boottime = NULL;
> +
> +	/* Recalculate CRC32 */
> +	systab.hdr.crc32 = 0;
> +	systab.hdr.crc32 = crc32(0, (const unsigned char *)&systab,
> +				 sizeof(struct efi_system_table));
> +
>  	/* Give the payload some time to boot */
>  	efi_set_watchdog(0);
>  	WATCHDOG_RESET();
>
diff mbox series

Patch

diff --git a/include/efi_api.h b/include/efi_api.h
index 0bc244444d..4252d11398 100644
--- a/include/efi_api.h
+++ b/include/efi_api.h
@@ -247,11 +247,11 @@  struct efi_system_table {
 	struct efi_table_hdr hdr;
 	unsigned long fw_vendor;   /* physical addr of wchar_t vendor string */
 	u32 fw_revision;
-	unsigned long con_in_handle;
+	efi_handle_t con_in_handle;
 	struct efi_simple_input_interface *con_in;
-	unsigned long con_out_handle;
+	efi_handle_t con_out_handle;
 	struct efi_simple_text_output_protocol *con_out;
-	unsigned long stderr_handle;
+	efi_handle_t stderr_handle;
 	struct efi_simple_text_output_protocol *std_err;
 	struct efi_runtime_services *runtime;
 	struct efi_boot_services *boottime;
diff --git a/lib/efi_loader/efi_boottime.c b/lib/efi_loader/efi_boottime.c
index 4b3b63e39a..2c5499e0c8 100644
--- a/lib/efi_loader/efi_boottime.c
+++ b/lib/efi_loader/efi_boottime.c
@@ -1645,12 +1645,16 @@  static void efi_exit_caches(void)
 }
 
 /*
- * Stop boot services.
+ * Stop all boot services.
  *
  * This function implements the ExitBootServices service.
  * See the Unified Extensible Firmware Interface (UEFI) specification
  * for details.
  *
+ * All timer events are disabled.
+ * For exit boot services events the notification function is called.
+ * The boot services are disabled in the system table.
+ *
  * @image_handle	handle of the loaded image
  * @map_key		key of the memory map
  * @return		status code
@@ -1662,16 +1666,24 @@  static efi_status_t EFIAPI efi_exit_boot_services(efi_handle_t image_handle,
 
 	EFI_ENTRY("%p, %ld", image_handle, map_key);
 
+	/* Make sure that notification functions are not called anymore */
+	efi_tpl = TPL_HIGH_LEVEL;
+
+	/* Check if ExitBootServices has already been called */
+	if (!systab.boottime)
+		return EFI_EXIT(EFI_SUCCESS);
+
 	/* Notify that ExitBootServices is invoked. */
 	for (i = 0; i < ARRAY_SIZE(efi_events); ++i) {
 		if (efi_events[i].type != EVT_SIGNAL_EXIT_BOOT_SERVICES)
 			continue;
-		efi_signal_event(&efi_events[i]);
+		if (!efi_events[i].notify_function)
+			continue;
+		EFI_CALL_VOID(efi_events[i].notify_function(
+				&efi_events[i], efi_events[i].notify_context));
 	}
-	/* Make sure that notification functions are not called anymore */
-	efi_tpl = TPL_HIGH_LEVEL;
 
-	/* XXX Should persist EFI variables here */
+	/* TODO Should persist EFI variables here */
 
 	board_quiesce_devices();
 
@@ -1681,6 +1693,20 @@  static efi_status_t EFIAPI efi_exit_boot_services(efi_handle_t image_handle,
 	/* This stops all lingering devices */
 	bootm_disable_interrupts();
 
+	/* Disable boottime services */
+	systab.con_in_handle = NULL;
+	systab.con_in = NULL;
+	systab.con_out_handle = NULL;
+	systab.con_out = NULL;
+	systab.stderr_handle = NULL;
+	systab.std_err = NULL;
+	systab.boottime = NULL;
+
+	/* Recalculate CRC32 */
+	systab.hdr.crc32 = 0;
+	systab.hdr.crc32 = crc32(0, (const unsigned char *)&systab,
+				 sizeof(struct efi_system_table));
+
 	/* Give the payload some time to boot */
 	efi_set_watchdog(0);
 	WATCHDOG_RESET();