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

[Heinrich Schuchardt]

On 6/7/25 11:39, Heinrich Schuchardt wrote:
> 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.

Sorry this function exits. But it does not work correctly on the sandbox 
as the LMB memory start is not 4 MiB aligned. I will send a series for this.

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