[PATCH v2 2/3] efi: add EFI_DEBUG_IMAGE_INFO_TABLE for debug
Heinrich Schuchardt
xypron.glpk at gmx.de
Fri Apr 25 22:31:16 CEST 2025
On 4/25/25 21:05, Paul Liu wrote:
>
>
> On Thu, 24 Apr 2025 at 23:02, Heinrich Schuchardt <xypron.glpk at gmx.de
> <mailto:xypron.glpk at gmx.de>> wrote:
>
> On 24.04.25 22:19, Ying-Chun Liu (PaulLiu) wrote:
> > From: "Ying-Chun Liu (PaulLiu)" <paul.liu at linaro.org
> <mailto: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
> <mailto:paul.liu at linaro.org>>
> > Cc: Heinrich Schuchardt <xypron.glpk at gmx.de
> <mailto:xypron.glpk at gmx.de>>
> > Cc: Ilias Apalodimas <ilias.apalodimas at linaro.org
> <mailto: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 <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?
>
>
> Not aligned. This structure in EDK2 is not aligned, not packed. And it
> is really not packed because the size of this structure is larger. 4
> bytes are padded in the 64-bit environment I think.
All other structures in EDK II are packed. Did you look at the structure
in a debugger?
The compiler will assume that a pointer to "struct
efi_debug_image_info_normal *" points to a sizeof(uintptr_t) aligned
structure if no other indication is made.
Best regards
Heinrich
>
> > +
> > +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.
>
>
> I just copied it from the EDK2. But we can remove it. I think edk2 wants
> to support not only normal images. However, currently there is only a
> normal image.
> So it looks redundant.
> I will remove this union in the next version.
>
> > + 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?
>
>
> The volatile is needed because we like the status to be updated
> immediately to the memory.
> If we are using a hardware jtag. And it stops the code at random places
> at the beginning. It might have paused just inside these logics.
> So this update_status is to tell the gdb python extensions to not
> referencing EFI tables because it is not ready yet.
> Thus this value cannot be optimized for updating later. And thus the
> volatile is needed.
>
> 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;
> > }
>
More information about the U-Boot
mailing list