re-adding Ard, who was dropped due to a bug in my MUA - sorry.
I seems that removing the packed attribute from the GUID struct alone
does not solve it.
I am about to post a revert of the whole GUID thing so we can move on
with the 2.12 release. If we want this feature, I can spend some time on
GUIDs after the release.
Oliver
Quoting Oliver Steffen (2023-09-13 13:48:21)
> On Wed, Sep 13, 2023 at 1:43 PM Ard Biesheuvel <a...@kernel.org> wrote:
> >
> > On Wed, 13 Sept 2023 at 13:42, Vladimir 'phcoder' Serbinenko
> > <phco...@gmail.com> wrote:
> > >
> > >
> > >
> > > Le mer. 13 sept. 2023, 12:33, Ard Biesheuvel <a...@kernel.org> a écrit :
> > >>
> > >> On Wed, 13 Sept 2023 at 12:18, John Paul Adrian Glaubitz
> > >> <glaub...@physik.fu-berlin.de> wrote:
> > >> >
> > >> > Hi Oliver!
> > >> >
> > >> > On Wed, 2023-09-13 at 12:14 +0200, Oliver Steffen wrote:
> > >> > > On Wed, Sep 13, 2023 at 6:10 AM Pedro Miguel Justo
> > >> > > <pm...@texair.net> wrote:
> > >> > > >
> > >> > > >
> > >> > > > I can confirm that, taking [1][2] and making [3] on top of it, my
> > >> > > > Montvale-based rx2660 machine still boots fine.
> > >> > >
> > >> > > Wonderful! Thanks for testing!
> > >> >
> > >> > Are you going to submit a patch to fix the issue with the new
> > >> > information?
> > >> >
> > >> > Would be great if the bug could be fixed before the 2.12 release.
> > >> >
> > >>
> > >> Yes, this needs to be fixed. The EFI GUID type should not have the
> > >> packed attribute in the general case, only in places where it could
> > >> really appear misaligned (e.g., in device path nodes), although I
> > >> suspect that adding the packed attribute to the outer struct would be
> > >> sufficient there (given that the guid struct has no internal padding
> > >> so the attribute only affects its minimum alignment)
> > >>
> > >> E.g,
> > >>
> > >> struct grub_efi_vendor_device_path
> > >> {
> > >> grub_efi_device_path_t header;
> > >> grub_guid_t vendor_guid;
> > >> grub_efi_uint8_t vendor_defined_data[0];
> > >> } GRUB_PACKED;
> > >
> > > Tried this. Compiler doesn't allow it
> >
> > Tried what exactly? This is just copy/paste from the existing tree
>
> With
>
> diff --git a/include/grub/types.h b/include/grub/types.h
> index 0d96006fe..fb0dfc8b6 100644
> --- a/include/grub/types.h
> +++ b/include/grub/types.h
> @@ -376,7 +376,7 @@ struct grub_guid
> grub_uint16_t data2;
> grub_uint16_t data3;
> grub_uint8_t data4[8];
> -} GRUB_PACKED;
> +};
> typedef struct grub_guid grub_guid_t;
> #endif /* ! GRUB_TYPES_HEADER */
>
> I get:
>
> [...]
> commands/efi/loadbios.c: In function ‘fake_bios_data’:
> commands/efi/loadbios.c:109:9: warning: taking address of packed
> member of ‘struct grub_efi_configuration_table’ may result in an
> unaligned pointer value [-Waddress-of-packed-member]
> 109 | &grub_efi_system_table->configuration_table[i].vendor_guid;
> | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> commands/probe.c: In function ‘grub_cmd_probe’:
> commands/probe.c:132:22: warning: taking address of packed member of
> ‘struct grub_gpt_partentry’ may result in an unaligned pointer value
> [-Waddress-of-packed-member]
> 132 | guid = &entry.guid;
> | ^~~~~~~~~~~
> [...]
>
> also in a couple of other places.
>
>
> struct grub_efi_configuration_table
> {
> grub_guid_t vendor_guid;
> void *vendor_table;
> } GRUB_PACKED;
>
> and
>
> struct grub_gpt_partentry
> {
> grub_guid_t type;
> grub_guid_t guid;
> grub_uint64_t start;
> grub_uint64_t end;
> grub_uint64_t attrib;
> char name[72];
> } GRUB_PACKED;
>
>
>
> Is this what you mean, Vladimir?
>
> Oliver
--
🎩Oliver Steffen (he/him) - Software Engineer, Virtualization
Red Hat GmbH <https://www.redhat.com/de/global/dach>,
Registered seat: Werner-von-Siemens-Ring 12, D-85630 Grasbrunn, Germany
Commercial register: Amtsgericht München/Munich, HRB 153243,
Managing Directors: Ryan Barnhart, Charles Cachera, Michael O'Neill,
Amy Ross
Everyone has different working hours… Please do not feel obligated to
reply outside of your normal work schedule.
_______________________________________________
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
