[PATCH v4 1/4] efi: add EFI_SYSTEM_TABLE_POINTER for debug

[Heinrich Schuchardt]

On 5/29/25 06:05, Ying-Chun Liu (PaulLiu) wrote:
> From: "Ying-Chun Liu (PaulLiu)" <paul.liu at linaro.org>
> 
> Add EFI_SYSTEM_TABLE_POINTER structure for remote debugger to locate
> the address of EFI_SYSTEM_TABLE.
> 
> This feature is described in UEFI SPEC version 2.10. Section 18.4.2.
> The implementation ensures support for hardware-assisted debugging and
> provides a standardized mechanism for debuggers to discover the EFI
> system table.
> 
> Signed-off-by: Ying-Chun Liu (PaulLiu) <paul.liu at linaro.org>
> Cc: Heinrich Schuchardt <xypron.glpk at gmx.de>
> Cc: Ilias Apalodimas <ilias.apalodimas at linaro.org>
> Cc: Peter Robinson <pbrobinson at gmail.com>
> Cc: Simon Glass <sjg at chromium.org>
> ---
> V2: add Kconfig options to turn on/off this feature.
> V3: make efi_initialize_system_table_pointer() do the job as described.
> V4: use efi_alloc_aligned_pages() for system_table_pointer.
> ---
>   include/efi_api.h             | 16 +++++++++++++++
>   include/efi_loader.h          |  2 ++
>   lib/efi_loader/Kconfig        |  7 +++++++
>   lib/efi_loader/efi_boottime.c | 37 +++++++++++++++++++++++++++++++++++
>   lib/efi_loader/efi_setup.c    |  8 ++++++++
>   5 files changed, 70 insertions(+)
> 
> diff --git a/include/efi_api.h b/include/efi_api.h
> index eb61eafa028..6c4c1a0cc7b 100644
> --- a/include/efi_api.h
> +++ b/include/efi_api.h
> @@ -259,6 +259,22 @@ struct efi_capsule_result_variable_header {
>   	efi_status_t capsule_status;
>   } __packed;
>   
> +/**
> + * struct efi_system_table_pointer - struct to store the pointer of system
> + * table.
> + * @signature: The signature of this struct.
> + * @efi_system_table_base: The physical address of System Table.
> + * @crc32: CRC32 checksum
> + *
> + * This struct is design for hardware debugger to search through memory to
> + * get the address of EFI System Table.
> + */
> +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 8f9f2bcf1cb..8f065244d8e 100644
> --- a/include/efi_loader.h
> +++ b/include/efi_loader.h
> @@ -646,6 +646,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 7f02a83e2a2..d878fc9021d 100644
> --- a/lib/efi_loader/Kconfig
> +++ b/lib/efi_loader/Kconfig
> @@ -71,6 +71,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 dbebb37dc04..16e921781e9 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,38 @@ efi_status_t efi_initialize_system_table(void)
>   
>   	return ret;
>   }
> +
> +/**
> + * efi_initialize_system_table() - Initialize system table pointer
> + *
> + * Return:	status code
> + */
> +efi_status_t efi_initialize_system_table_pointer(void)
> +{
> +	const int size_4MB = 0x00400000;
> +	u64 addr;
> +	u32 crc32_value;
> +
> +	/* Allocate configuration table array */
> +	addr = (u64)efi_alloc_aligned_pages(sizeof(struct efi_system_table_pointer),
> +					    EFI_RUNTIME_SERVICES_DATA,
> +					    size_4MB);

We expect the code to compile after each individual step. Function 
efi_alloc_aligned_pages() does not exist when this step is applied.

Even if I apply all patches your code does not compile on 32bit systems:

+lib/efi_loader/efi_boottime.c:4072:16: error: cast from pointer to 
integer of different size [-Werror=pointer-to-int-cast]
+ 4072 |         addr = (u64)efi_alloc_aligned_pages(sizeof(struct 
efi_system_table_pointer),
+      |

Use (uintptr_t) here.

Best regards

Heinrich

> +
> +	if (!addr) {
> +		log_err("Installing EFI system table pointer failed\n");
> +		return EFI_OUT_OF_RESOURCES;
> +	}
> +
> +	systab_pointer = (struct efi_system_table_pointer *)addr;
> +	memset(systab_pointer, 0, sizeof(struct efi_system_table_pointer));
> +
> +	systab_pointer->signature = EFI_SYSTEM_TABLE_SIGNATURE;
> +	systab_pointer->efi_system_table_base = (efi_physical_addr_t)&systab;
> +
> +	crc32_value = crc32(0,
> +			    (const unsigned char *)systab_pointer,
> +			    sizeof(struct efi_system_table_pointer));
> +	systab_pointer->crc32 = crc32_value;
> +
> +	return EFI_SUCCESS;
> +}
> diff --git a/lib/efi_loader/efi_setup.c b/lib/efi_loader/efi_setup.c
> index 48f91da5df7..a6b37d994f6 100644
> --- a/lib/efi_loader/efi_setup.c
> +++ b/lib/efi_loader/efi_setup.c
> @@ -278,6 +278,14 @@ efi_status_t efi_init_obj_list(void)
>   	if (ret != EFI_SUCCESS)
>   		goto out;
>   
> +	/* Initialize system table pointer */
> +	if (IS_ENABLED(CONFIG_EFI_DEBUG_SUPPORT_TABLE)) {
> +		ret = efi_initialize_system_table_pointer();
> +		if (ret != EFI_SUCCESS) {
> +			goto out;
> +		}
> +	}
> +
>   	if (IS_ENABLED(CONFIG_EFI_ECPT)) {
>   		ret = efi_ecpt_register();
>   		if (ret != EFI_SUCCESS)