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

Reply via email to