[PATCH v6 2/4] efi: add EFI_DEBUG_IMAGE_INFO_TABLE for debug
Paul Liu
paul.liu at linaro.org
Wed Jun 18 16:11:36 CEST 2025
On Wed, 18 Jun 2025 at 07:39, Heinrich Schuchardt <xypron.glpk at gmx.de>
wrote:
> On 6/17/25 14:08, 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.
> > The implementation ensures support for hardware-assisted debugging and
> > provides a standardized mechanism for debuggers to discover and interact
> > with system-level debug resources.
> >
> > Signed-off-by: Ying-Chun Liu (PaulLiu) <paul.liu at linaro.org>
> > Reviewed-by: Ilias Apalodimas <ilias.apalodimas at linaro.org>
> > Cc: Heinrich Schuchardt <xypron.glpk at gmx.de>
> > Cc: Peter Robinson <pbrobinson at gmail.com>
> > Cc: Simon Glass <sjg at chromium.org>
> > ---
> > V3: Fix the way of installation of configuration table.
> > V5: move the code into a separate module.
> > ---
> > include/efi_api.h | 53 ++++++++++++++++++++++++++++++
> > include/efi_loader.h | 2 ++
> > lib/efi_loader/efi_debug_support.c | 6 ++++
> > lib/efi_loader/efi_setup.c | 12 +++++++
> > 4 files changed, 73 insertions(+)
> >
> > diff --git a/include/efi_api.h b/include/efi_api.h
> > index 6c4c1a0cc7b..8da0a350ce3 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;
> > @@ -574,6 +578,55 @@ 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
> > +
> > +/**
> > + * struct efi_debug_image_info_normal - Store Debug Information for
> normal
> > + * image.
> > + * @image_info_type: the type of image info.
> > + * @loaded_image_protocol_instance: the pointer to struct
> efi_loaded_image.
> > + * @image_handle: the EFI handle of the image.
> > + *
> > + * This struct is created by efi_load_image() and store the information
> > + * for debugging an normal image.
> > + */
> > +struct efi_debug_image_info_normal {
> > + u32 image_info_type;
> > + struct efi_loaded_image *loaded_image_protocol_instance;
> > + efi_handle_t image_handle;
> > +};
> > +
> > +/**
> > + * union efi_debug_image_info - The union to store a pointer for EFI
> > + * DEBUG IMAGE INFO.
> > + * @image_info_type: the type of the image_info if it is not a normal
> image.
> > + * @normal_image: The pointer to a normal image.
> > + *
> > + * This union is for a pointer that can point to the struct of
> normal_image.
> > + * Or it points to an image_info_type.
> > + */
> > +union efi_debug_image_info {
> > + u32 *image_info_type;
> > + struct efi_debug_image_info_normal *normal_image;
> > +};
> > +
> > +/**
> > + * struct efi_debug_image_info_table_header - store the array of
> > + * struct efi_debug_image_info.
> > + * @update_status: Status to notify this struct is ready to use or not.
> > + * @table_size: The number of elements of efi_debug_image_info_table.
> > + * @efi_debug_image_info_table: The array of efi_debug_image_info.
> > + *
> > + * This struct stores the array of efi_debug_image_info. The
> > + * number of elements is table_size.
> > + */
> > +struct efi_debug_image_info_table_header {
> > + volatile u32 update_status;
> > + 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/include/efi_loader.h b/include/efi_loader.h
> > index 8f065244d8e..d0c72d0bc58 100644
> > --- a/include/efi_loader.h
> > +++ b/include/efi_loader.h
> > @@ -315,6 +315,8 @@ extern const struct efi_hii_config_routing_protocol
> efi_hii_config_routing;
> > extern const struct efi_hii_config_access_protocol
> efi_hii_config_access;
> > extern const struct efi_hii_database_protocol efi_hii_database;
> > extern const struct efi_hii_string_protocol efi_hii_string;
> > +/* structure for EFI_DEBUG_SUPPORT_PROTOCOL */
> > +extern struct efi_debug_image_info_table_header
> efi_m_debug_info_table_header;
> >
> > uint16_t *efi_dp_str(struct efi_device_path *dp);
> >
> > diff --git a/lib/efi_loader/efi_debug_support.c
> b/lib/efi_loader/efi_debug_support.c
> > index 3794b31c4cd..f2dfcc5e6b2 100644
> > --- a/lib/efi_loader/efi_debug_support.c
> > +++ b/lib/efi_loader/efi_debug_support.c
> > @@ -11,6 +11,12 @@
> >
> > struct efi_system_table_pointer __efi_runtime_data * systab_pointer =
> NULL;
> >
> > +struct efi_debug_image_info_table_header efi_m_debug_info_table_header
> = {
> > + 0,
> > + 0,
> > + NULL
> > +};
> > +
> > /**
> > * efi_initialize_system_table_pointer() - Initialize system table
> pointer
> > *
> > diff --git a/lib/efi_loader/efi_setup.c b/lib/efi_loader/efi_setup.c
> > index 6193b9f6d0c..60f3a7e5fbf 100644
> > --- a/lib/efi_loader/efi_setup.c
> > +++ b/lib/efi_loader/efi_setup.c
> > @@ -18,6 +18,9 @@
> >
> > efi_status_t efi_obj_list_initialized = OBJ_LIST_NOT_INITIALIZED;
> >
> > +const efi_guid_t efi_debug_image_info_table_guid =
> > + EFI_DEBUG_IMAGE_INFO_TABLE_GUID;
> > +
> > /*
> > * Allow unaligned memory access.
> > *
> > @@ -280,10 +283,19 @@ efi_status_t efi_init_obj_list(void)
> >
> > /* Initialize system table pointer */
> > if (IS_ENABLED(CONFIG_EFI_DEBUG_SUPPORT)) {
> > + efi_guid_t debug_image_info_table_guid =
>
> This variable shadows the variable defined above.
>
> > + efi_debug_image_info_table_guid;
>
> This self assignment provides a random value from the stack and not the
> expected GUID.
>
> Please, provide a test in lib/efi_selftest/ verifying that the table is
> installed with the correct GUID.
>
>
Hi Heinrich,
This is not a self assignment. The new variable is called
"debug_image_info_table_guid". The global one is called
"efi_debug_image_info_table_guid".
I'll implement a selftest anyway to make sure that the GUID is correct.
> > +
> > ret = efi_initialize_system_table_pointer();
> > if (ret != EFI_SUCCESS) {
> > goto out;
> > }
> > +
> > + ret =
> efi_install_configuration_table(&debug_image_info_table_guid,
> > +
> &efi_m_debug_info_table_header);
> > + if (ret != EFI_SUCCESS) {
> > + goto out;
> > + }
>
> No braces here. Please, use scripts/checkpatch.pl before resubmitting.
>
>
Do you set some parameters of the scripts/checkpatch.pl ?
I'm using ./scripts/checkpatch.pl -g <COMMIT_ID> --u-boot.
But it only gives me a warning about using the volatile.
And for patch 1/4 it only gives me a warning about adding new file and the
MAINTAINER stuff.
This is weird. I didn't get the warning about the braces at all.
Yours,
Paul
More information about the U-Boot
mailing list