Re: [RFC 1/3] efi: add configuration table search function

[Vladimir 'phcoder' Serbinenko]

On Tuesday, March 1, 2016, Leif Lindholm <address@hidden> wrote:
> On Tue, Mar 01, 2016 at 08:13:50PM +0000, Vladimir 'phcoder' Serbinenko wrote:
> > Le mar. 1 mars 2016 21:09, Andrei Borzenkov <address@hidden> a écrit :
> >
> > > 01.03.2016 20:41, Leif Lindholm пишет:
> > > > Several places in the code iterates through the configuration tables
> > > > presented by the system table.
> > > > Add new function grub_efi_find_config_table to use for this instead.
> > > > ---
> > > >  grub-core/kern/efi/efi.c | 18 ++++++++++++++++++
> > > >  include/grub/efi/efi.h   |  3 +++
> > > >  2 files changed, 21 insertions(+)
> > > >
> > > > diff --git a/grub-core/kern/efi/efi.c b/grub-core/kern/efi/efi.c
> > > > index caf9bcc..41686ee 100644
> > > > --- a/grub-core/kern/efi/efi.c
> > > > +++ b/grub-core/kern/efi/efi.c
> > > > @@ -911,3 +911,21 @@ grub_efi_compare_device_paths (const
> > > grub_efi_device_path_t *dp1,
> > > >
> > > >    return 0;
> > > >  }
> > > > +
> > > > +/* Return pointer to vendor table identified by guid. */
> > > > +void *
> > > > +grub_efi_find_config_table (const grub_efi_guid_t *guid)
> > > > +{
> > > > +  grub_efi_uintn_t i;
> > > > +
> > > > +  for (i = 0; i < grub_efi_system_table->num_table_entries; i++)
> > > > +    {
> > > > +      grub_efi_configuration_table_t *table =
> > > > +     &grub_efi_system_table->configuration_table[i];
> > > > +
> > > > +      if (! grub_memcmp (&table->vendor_guid, guid,
> > > > +                      sizeof (grub_efi_packed_guid_t)))
> > >
> > > Either both types are exactly the same and one should be dropped or this
> > > cannot be right. In view of recent discussion about structure member
> > > alignments I am not sure why grub_efi_packed_guid_t is needed and the
> > > code already was inconsistent in usage.
> > >
> > Types differ only in alignment, not in internal structure. We can guarantee
> > alignment of structure that we allocated but not of some EFI pointer
>
> Of course you can.
> The UEFI specification explicitly states
> "Unless otherwise specified, aligned on a 64-bit boundary."
> which is double the natural alignment of the largest member
> defined in this struct.

Lies, damn lies and firmware specifications. I'd avoid relying on firmware not having subtle bugs with this.  

> /
>     Leif
>
> _______________________________________________
> Grub-devel mailing list
> address@hidden
> https://lists.gnu.org/mailman/listinfo/grub-devel


-- 
Regards
Vladimir 'phcoder' Serbinenko