[PATCH v2 2/3] efi: add EFI_DEBUG_IMAGE_INFO_TABLE for debug

[Heinrich Schuchardt]

On 24.04.25 22:19, Ying-Chun Liu (PaulLiu) wrote:
> From: "Ying-Chun Liu (PaulLiu)" <paul.liu at linaro.org>
> 
> EFI_DEBUG_IMAGE_INFO_TABLE is used to store EFI_LOADED_IMAGE for
> debug purpose. This commit adds the table to the EFI_CONFIGURATION_TABLE.
> 
> This feature is described in UEFI Spec version 2.10. Section 18.4.
> 
> 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>
> ---
>   include/efi_api.h             | 24 ++++++++++++++++++++++++
>   lib/efi_loader/efi_boottime.c | 16 ++++++++++++++++
>   2 files changed, 40 insertions(+)
> 
> diff --git a/include/efi_api.h b/include/efi_api.h
> index 5017d9037cd..0f4245091f1 100644
> --- a/include/efi_api.h
> +++ b/include/efi_api.h
> @@ -238,6 +238,10 @@ enum efi_reset_type {
>   	EFI_GUID(0xcce33c35, 0x74ac, 0x4087, 0xbc, 0xe7, \
>   		 0x8b, 0x29, 0xb0, 0x2e, 0xeb, 0x27)
>   
> +#define EFI_DEBUG_IMAGE_INFO_TABLE_GUID \
> +	EFI_GUID(0x49152e77, 0x1ada, 0x4764, 0xb7, 0xa2, \
> +		 0x7a, 0xfe, 0xfe, 0xd9, 0x5e, 0x8b)
> +
>   struct efi_conformance_profiles_table {
>   	u16 version;
>   	u16 number_of_profiles;
> @@ -564,6 +568,26 @@ struct efi_loaded_image {
>   	efi_status_t (EFIAPI *unload)(efi_handle_t image_handle);
>   };
>   
> +#define EFI_DEBUG_IMAGE_INFO_UPDATE_IN_PROGRESS 0x01
> +#define EFI_DEBUG_IMAGE_INFO_TABLE_MODIFIED     0x02
> +

All structures and unions and their elements should be describe according to
https://docs.kernel.org/doc-guide/kernel-doc.html#structure-union-and-enumeration-documentation

> +struct efi_debug_image_info_normal {
> +	u32 image_info_type;
> +	struct efi_loaded_image *loaded_image_protocol_instance;
> +	efi_handle_t image_handle;
> +};

What are the assumptions about alignment?
Should this structure be marked as packed like most UEFI structures?

> +
> +union efi_debug_image_info {

Why do we need the union? The first field of struct 
efi_debug_image_info_normal is image_info_type.

> +	u32 *image_info_type;
> +	struct efi_debug_image_info_normal *normal_image;
> +};
> +
> +struct efi_debug_image_info_table_header {
> +	volatile u32 update_status;

Why is volatile needed here?

Best regards

Heinrich

> +	u32 table_size;
> +	union efi_debug_image_info *efi_debug_image_info_table;
> +};
> +
>   #define EFI_DEVICE_PATH_PROTOCOL_GUID \
>   	EFI_GUID(0x09576e91, 0x6d3f, 0x11d2, \
>   		 0x8e, 0x39, 0x00, 0xa0, 0xc9, 0x69, 0x72, 0x3b)
> diff --git a/lib/efi_loader/efi_boottime.c b/lib/efi_loader/efi_boottime.c
> index 366bc0d2b1d..e2665459e44 100644
> --- a/lib/efi_loader/efi_boottime.c
> +++ b/lib/efi_loader/efi_boottime.c
> @@ -4038,6 +4038,16 @@ efi_status_t efi_initialize_system_table(void)
>   	return ret;
>   }
>   



> +struct efi_debug_image_info_table_header __efi_runtime_data



> +efi_m_debug_info_table_header = {
> +	0,
> +	0,
> +	NULL
> +};
> +
> +const efi_guid_t efi_debug_image_info_table_guid =
> +	EFI_DEBUG_IMAGE_INFO_TABLE_GUID;
> +
>   /**
>    * efi_initialize_system_table() - Initialize system table
>    *
> @@ -4053,6 +4063,8 @@ efi_status_t efi_initialize_system_table_pointer(void)
>   	u64 addr;
>   	u64 aligned_addr;
>   	u32 crc32_value;
> +	efi_guid_t debug_image_info_table_guid =
> +		efi_debug_image_info_table_guid;
>   
>   	/* Allocate configuration table array */
>   	ret = efi_allocate_pages(EFI_ALLOCATE_ANY_PAGES,
> @@ -4077,5 +4089,9 @@ efi_status_t efi_initialize_system_table_pointer(void)
>   			    (const unsigned char *)systab_pointer,
>   			    sizeof(struct efi_system_table_pointer));
>   	systab_pointer->crc32 = crc32_value;
> +
> +	ret = efi_install_configuration_table(&debug_image_info_table_guid,
> +					      &efi_m_debug_info_table_header);
> +
>   	return ret;
>   }