diff mbox series

[v2,1/3] efi: add EFI_SYSTEM_TABLE_POINTER for debug

Message ID 20250424201927.2027257-2-paulliu@debian.org
State New
Headers show
Series efi: Add EFI Debug Support Table feature | expand

Commit Message

Ying-Chun Liu (PaulLiu) April 24, 2025, 8:19 p.m. UTC 
From: "Ying-Chun Liu (PaulLiu)" <paul.liu@linaro.org>

Add EFI_SYSTEM_TABLE_POINTER structure for debugger to locate
the address of EFI_SYSTEM_TABLE.

This feature is described in UEFI SPEC version 2.10. Section 18.4.2.

Signed-off-by: Ying-Chun Liu (PaulLiu) <paul.liu@linaro.org>
Cc: Heinrich Schuchardt <xypron.glpk@gmx.de>
Cc: Ilias Apalodimas <ilias.apalodimas@linaro.org>
---
V2: add Kconfig options to turn on/off this feature.
---
 include/efi_api.h             |  6 +++++
 include/efi_loader.h          |  2 ++
 lib/efi_loader/Kconfig        |  7 ++++++
 lib/efi_loader/efi_boottime.c | 44 +++++++++++++++++++++++++++++++++++
 lib/efi_loader/efi_setup.c    |  9 +++++++
 5 files changed, 68 insertions(+)

Comments

Heinrich Schuchardt April 24, 2025, 9:45 p.m. UTC | #1 
On 24.04.25 22:19, Ying-Chun Liu (PaulLiu) wrote:
> From: "Ying-Chun Liu (PaulLiu)" <paul.liu@linaro.org>
> 
> Add EFI_SYSTEM_TABLE_POINTER structure for debugger to locate
> the address of EFI_SYSTEM_TABLE.
> 
> This feature is described in UEFI SPEC version 2.10. Section 18.4.2.

Hello Ying-Chun,

Thank you for your contribution.

Unfortunately it is not clear what your use case is. The reason for 
implementing a feature should be mentioned in the commit message. 
Cover-letters for a patch series are the right place to describe 
overarching topics.

The relevant information needed to debug U-Boot is the u-boot.map file 
and the relocation address. On arm64 the relocation address can be found 
via the global data structure pointed to by register x18.

Once you have the relocation offset, you can determine the system table 
address via the u-boot.map file.

If you need information about loaded EFI images, you could enable debug 
messages using U-Boot's log command.

> 
> Signed-off-by: Ying-Chun Liu (PaulLiu) <paul.liu@linaro.org>
> Cc: Heinrich Schuchardt <xypron.glpk@gmx.de>
> Cc: Ilias Apalodimas <ilias.apalodimas@linaro.org>
> ---
> V2: add Kconfig options to turn on/off this feature.
> ---
>   include/efi_api.h             |  6 +++++
>   include/efi_loader.h          |  2 ++
>   lib/efi_loader/Kconfig        |  7 ++++++
>   lib/efi_loader/efi_boottime.c | 44 +++++++++++++++++++++++++++++++++++
>   lib/efi_loader/efi_setup.c    |  9 +++++++
>   5 files changed, 68 insertions(+)
> 
> diff --git a/include/efi_api.h b/include/efi_api.h
> index eb61eafa028..5017d9037cd 100644
> --- a/include/efi_api.h
> +++ b/include/efi_api.h
> @@ -259,6 +259,12 @@ struct efi_capsule_result_variable_header {
>   	efi_status_t capsule_status;
>   } __packed;
>   
> +struct efi_system_table_pointer {
> +	u64 signature;
> +	efi_physical_addr_t efi_system_table_base;
> +	u32 crc32;
> +};
> +
>   struct efi_memory_range {
>   	efi_physical_addr_t	address;
>   	u64			length;
> diff --git a/include/efi_loader.h b/include/efi_loader.h
> index 1d75d97ebbc..370bc042f70 100644
> --- a/include/efi_loader.h
> +++ b/include/efi_loader.h
> @@ -615,6 +615,8 @@ efi_status_t efi_tcg2_measure_dtb(void *dtb);
>   efi_status_t efi_root_node_register(void);
>   /* Called by bootefi to initialize runtime */
>   efi_status_t efi_initialize_system_table(void);
> +/* Called by bootefi to initialize debug */
> +efi_status_t efi_initialize_system_table_pointer(void);
>   /* efi_runtime_detach() - detach unimplemented runtime functions */
>   void efi_runtime_detach(void);
>   /* efi_convert_pointer() - convert pointer to virtual address */
> diff --git a/lib/efi_loader/Kconfig b/lib/efi_loader/Kconfig
> index d4f6b56afaa..94a75981389 100644
> --- a/lib/efi_loader/Kconfig
> +++ b/lib/efi_loader/Kconfig
> @@ -70,6 +70,13 @@ config EFI_SECURE_BOOT
>   config EFI_SIGNATURE_SUPPORT
>   	bool
>   
> +config EFI_DEBUG_SUPPORT_TABLE
> +	bool "EFI Debug Support Table"
> +	help
> +	  Select this option if you want to setup the EFI Debug Support
> +	  Table which is used by the debug agent or an external debugger to
> +	  determine loaded image information in a quiescent manner.
> +
>   menu "UEFI services"
>   
>   config EFI_GET_TIME
> diff --git a/lib/efi_loader/efi_boottime.c b/lib/efi_loader/efi_boottime.c
> index 5164cb15986..366bc0d2b1d 100644
> --- a/lib/efi_loader/efi_boottime.c
> +++ b/lib/efi_loader/efi_boottime.c
> @@ -4001,6 +4001,8 @@ struct efi_system_table __efi_runtime_data systab = {
>   	.tables = NULL,
>   };
>   
> +struct efi_system_table_pointer __efi_runtime_data * systab_pointer = NULL;
> +
>   /**
>    * efi_initialize_system_table() - Initialize system table
>    *
> @@ -4035,3 +4037,45 @@ efi_status_t efi_initialize_system_table(void)
>   
>   	return ret;
>   }
> +
> +/**
> + * efi_initialize_system_table() - Initialize system table
> + *
> + * Return:	status code
> + */
> +efi_status_t efi_initialize_system_table_pointer(void)

This function name seems to be a misnomer as you are setting up much 
more with the next patches.

> +{
> +	efi_status_t ret;
> +	const int size_4MB = 0x00400000;
> +	int pages = efi_size_in_pages(sizeof(struct efi_system_table_pointer));
> +	int alignment_mask = size_4MB - 1;
> +	int real_pages = pages + efi_size_in_pages(size_4MB);
> +	u64 addr;
> +	u64 aligned_addr;
> +	u32 crc32_value;
> +
> +	/* Allocate configuration table array */
> +	ret = efi_allocate_pages(EFI_ALLOCATE_ANY_PAGES,
> +				 EFI_RUNTIME_SERVICES_DATA,
> +				 real_pages,
> +				 &addr);
> +
> +	/* alignment to 4MB boundary */
> +	aligned_addr = (addr + alignment_mask) & ~alignment_mask;
> +
> +	systab_pointer = (struct efi_system_table_pointer *)aligned_addr;
> +	memset(systab_pointer, 0, sizeof(struct efi_system_table_pointer));
> +
> +	/*
> +	 * These entries will be set to NULL in ExitBootServices(). To avoid
> +	 * relocation in SetVirtualAddressMap(), set them dynamically.
> +	 */

This comment does not match the submitted code.

> +	systab_pointer->signature = EFI_SYSTEM_TABLE_SIGNATURE;
> +	systab_pointer->efi_system_table_base = (efi_physical_addr_t)&systab;
> +	systab_pointer->crc32 = 0;
> +	crc32_value = crc32(0,
> +			    (const unsigned char *)systab_pointer,
> +			    sizeof(struct efi_system_table_pointer));
> +	systab_pointer->crc32 = crc32_value;
> +	return ret;
> +}
> diff --git a/lib/efi_loader/efi_setup.c b/lib/efi_loader/efi_setup.c
> index aa59bc7779d..6be23ca43bf 100644
> --- a/lib/efi_loader/efi_setup.c
> +++ b/lib/efi_loader/efi_setup.c
> @@ -259,6 +259,15 @@ efi_status_t efi_init_obj_list(void)
>   	if (ret != EFI_SUCCESS)
>   		goto out;
>   
> +	/* Initialize system table */
> +	if (IS_ENABLED(CONFIG_EFI_DEBUG_SUPPORT_TABLE)) {
> +		ret = efi_initialize_system_table_pointer();
> +		if (ret != EFI_SUCCESS) {
> +			printf("EFI init system table pointer fail\n");

Maybe: "Installing EFI system table pointer failed\n".

Please, use log_error() and move the output to the appropriate place in 
the efi_initialize_system_table_pointer() function.

Best regards

Heinrich

> +			goto out;
> +		}
> +	}
> +
>   	if (IS_ENABLED(CONFIG_EFI_ECPT)) {
>   		ret = efi_ecpt_register();
>   		if (ret != EFI_SUCCESS)
diff mbox series

Patch

diff --git a/include/efi_api.h b/include/efi_api.h
index eb61eafa028..5017d9037cd 100644
--- a/include/efi_api.h
+++ b/include/efi_api.h
@@ -259,6 +259,12 @@  struct efi_capsule_result_variable_header {
 	efi_status_t capsule_status;
 } __packed;
 
+struct efi_system_table_pointer {
+	u64 signature;
+	efi_physical_addr_t efi_system_table_base;
+	u32 crc32;
+};
+
 struct efi_memory_range {
 	efi_physical_addr_t	address;
 	u64			length;
diff --git a/include/efi_loader.h b/include/efi_loader.h
index 1d75d97ebbc..370bc042f70 100644
--- a/include/efi_loader.h
+++ b/include/efi_loader.h
@@ -615,6 +615,8 @@  efi_status_t efi_tcg2_measure_dtb(void *dtb);
 efi_status_t efi_root_node_register(void);
 /* Called by bootefi to initialize runtime */
 efi_status_t efi_initialize_system_table(void);
+/* Called by bootefi to initialize debug */
+efi_status_t efi_initialize_system_table_pointer(void);
 /* efi_runtime_detach() - detach unimplemented runtime functions */
 void efi_runtime_detach(void);
 /* efi_convert_pointer() - convert pointer to virtual address */
diff --git a/lib/efi_loader/Kconfig b/lib/efi_loader/Kconfig
index d4f6b56afaa..94a75981389 100644
--- a/lib/efi_loader/Kconfig
+++ b/lib/efi_loader/Kconfig
@@ -70,6 +70,13 @@  config EFI_SECURE_BOOT
 config EFI_SIGNATURE_SUPPORT
 	bool
 
+config EFI_DEBUG_SUPPORT_TABLE
+	bool "EFI Debug Support Table"
+	help
+	  Select this option if you want to setup the EFI Debug Support
+	  Table which is used by the debug agent or an external debugger to
+	  determine loaded image information in a quiescent manner.
+
 menu "UEFI services"
 
 config EFI_GET_TIME
diff --git a/lib/efi_loader/efi_boottime.c b/lib/efi_loader/efi_boottime.c
index 5164cb15986..366bc0d2b1d 100644
--- a/lib/efi_loader/efi_boottime.c
+++ b/lib/efi_loader/efi_boottime.c
@@ -4001,6 +4001,8 @@  struct efi_system_table __efi_runtime_data systab = {
 	.tables = NULL,
 };
 
+struct efi_system_table_pointer __efi_runtime_data * systab_pointer = NULL;
+
 /**
  * efi_initialize_system_table() - Initialize system table
  *
@@ -4035,3 +4037,45 @@  efi_status_t efi_initialize_system_table(void)
 
 	return ret;
 }
+
+/**
+ * efi_initialize_system_table() - Initialize system table
+ *
+ * Return:	status code
+ */
+efi_status_t efi_initialize_system_table_pointer(void)
+{
+	efi_status_t ret;
+	const int size_4MB = 0x00400000;
+	int pages = efi_size_in_pages(sizeof(struct efi_system_table_pointer));
+	int alignment_mask = size_4MB - 1;
+	int real_pages = pages + efi_size_in_pages(size_4MB);
+	u64 addr;
+	u64 aligned_addr;
+	u32 crc32_value;
+
+	/* Allocate configuration table array */
+	ret = efi_allocate_pages(EFI_ALLOCATE_ANY_PAGES,
+				 EFI_RUNTIME_SERVICES_DATA,
+				 real_pages,
+				 &addr);
+
+	/* alignment to 4MB boundary */
+	aligned_addr = (addr + alignment_mask) & ~alignment_mask;
+
+	systab_pointer = (struct efi_system_table_pointer *)aligned_addr;
+	memset(systab_pointer, 0, sizeof(struct efi_system_table_pointer));
+
+	/*
+	 * These entries will be set to NULL in ExitBootServices(). To avoid
+	 * relocation in SetVirtualAddressMap(), set them dynamically.
+	 */
+	systab_pointer->signature = EFI_SYSTEM_TABLE_SIGNATURE;
+	systab_pointer->efi_system_table_base = (efi_physical_addr_t)&systab;
+	systab_pointer->crc32 = 0;
+	crc32_value = crc32(0,
+			    (const unsigned char *)systab_pointer,
+			    sizeof(struct efi_system_table_pointer));
+	systab_pointer->crc32 = crc32_value;
+	return ret;
+}
diff --git a/lib/efi_loader/efi_setup.c b/lib/efi_loader/efi_setup.c
index aa59bc7779d..6be23ca43bf 100644
--- a/lib/efi_loader/efi_setup.c
+++ b/lib/efi_loader/efi_setup.c
@@ -259,6 +259,15 @@  efi_status_t efi_init_obj_list(void)
 	if (ret != EFI_SUCCESS)
 		goto out;
 
+	/* Initialize system table */
+	if (IS_ENABLED(CONFIG_EFI_DEBUG_SUPPORT_TABLE)) {
+		ret = efi_initialize_system_table_pointer();
+		if (ret != EFI_SUCCESS) {
+			printf("EFI init system table pointer fail\n");
+			goto out;
+		}
+	}
+
 	if (IS_ENABLED(CONFIG_EFI_ECPT)) {
 		ret = efi_ecpt_register();
 		if (ret != EFI_SUCCESS)