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

[John Paul Adrian Glaubitz]

Hi Daniel!

On Fri, 2023-08-11 at 17:31 +0200, Daniel Kiper wrote:
> 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.

My Itanium hardware is not available for bootloader tests at the moment, so I 
would
like to ask Pedro Miguel Justo or Frank Scheiner to test the proposed fix.

Thanks,
Adrian

-- 
 .''`.  John Paul Adrian Glaubitz
: :' :  Debian Developer
`. `'   Physicist
  `-    GPG: 62FF 8A75 84E0 2956 9546  0006 7426 3B37 F5B5 F913