Re: [PATCH v9 02/11] Unify GUID types

[Daniel Kiper]

On Fri, Aug 11, 2023 at 04:10:14AM -0700, Oliver Steffen wrote:
> Quoting John Paul Adrian Glaubitz (2023-08-11 10:32:17)
> > Hi Oliver!
> >
> > On Fri, 2023-05-26 at 13:35 +0200, Oliver Steffen wrote:
> > > There are 3 implementations of a GUID in Grub. Replace them with a
> > > common one, placed in types.h.
> > >
> > > It uses the "packed" flavor of the GUID structs, the alignment attribute
> > > is dropped, since it is not required.
> > >
> > > Signed-off-by: Oliver Steffen <osteffen@redhat.com>
> > > Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>

[...]

> > According to [1], this change broke GRUB on ia64:
> >
> > Welcome to GRUB!
> >
> > 7 0 0x00006B 0x000000000000001E unexpected trap
> > 7 0 0x000066 0x000000000000001E trap taken, number in ext PE
> > 7 0 0x00003C 0x0000000000005A00 trap taken, offset in ext PE
> >
> > I assume this is because of the strict alignment requirements on ia64.
> >
> > Could you have a look?
>
> I am very sorry for this mistake.
> My goal was to unify the two GUID types we had in grub but I missed the
> fact that in my "solution" the alignments are not correct in all cases.
>
> The quickest way out could be to revert the GUID unification and printf
> format specifier commits:
>
> 6ad116e5f guid: Make use of GUID printf format specifier
> f82dbf2bd kern/misc: Add a format specifier GUIDs
> 06edd40db guid: Unify GUID types
>
> And use the explicit, long-winded format string for printing the GUID
> in the bli module instead (added in the commits following those).
>
> I am open to suggestions / comments.

Adrian, could you check what will happen when you add alignment to the
grub_guid_t as it was suggested by Frank here [2]?

Personally I would avoid adding another GUID type with just alignment
requirement as the difference. Making one GUID type with always enforced
alignment should not cost us a lot. Or we can enforce alignment on EFI
platforms only.

Daniel

> > > [1] https://lists.gnu.org/archive/html/grub-devel/2023-08/msg00016.html

[2] https://lists.gnu.org/archive/html/grub-devel/2023-08/msg00020.html