On Fri, 25 Apr 2025 at 21:31, Heinrich Schuchardt <xypron.g...@gmx.de> wrote:
> On 4/25/25 21:05, Paul Liu wrote: > > > > > > On Thu, 24 Apr 2025 at 23:02, Heinrich Schuchardt <xypron.g...@gmx.de > > <mailto:xypron.g...@gmx.de>> wrote: > > > > On 24.04.25 22:19, Ying-Chun Liu (PaulLiu) wrote: > > > From: "Ying-Chun Liu (PaulLiu)" <paul....@linaro.org > > <mailto:paul....@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....@linaro.org > > <mailto:paul....@linaro.org>> > > > Cc: Heinrich Schuchardt <xypron.g...@gmx.de > > <mailto:xypron.g...@gmx.de>> > > > Cc: Ilias Apalodimas <ilias.apalodi...@linaro.org > > <mailto:ilias.apalodi...@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 > > > Hi Heinrich, Yes. All other structures in EDK II are packed. But this one is not packed. And yes, seems it is aligned to sizeof(uintptr_t) in edk2. How do we implement this if we want the alignment? I plan to just remove that union and use "struct efi_debug_image_info_normal" directly. But maybe the alignment is not important here? I'll do a bit of research on this. Because we are not searching for this structure in the debugger. We just read it from EFI_DEBUG_IMAGE_INFO_TABLE. Which will be in the Configuration Table with specific GUID. The only search in the debugger is for the SYSTEM_TABLE_POINTER structure. Yours, Paul