Re: [PATCH 1/2] arm64: Move firmware fdt search into global function

[Alexander Graf]

On 15/11/2016 22:00, Daniel Kiper wrote:
> On Sat, Nov 12, 2016 at 12:03:33PM +0300, Andrei Borzenkov wrote:
> > 29.02.2016 02:22, Alexander Graf ??????????:
> > > Searching for a device tree that EFI passes to us via configuration tables
> > > is nothing architecture specific. Move it into generic code.
> > >
> > > Signed-off-by: Alexander Graf <address@hidden>
> > > ---
> > >  grub-core/kern/efi/init.c    | 22 ++++++++++++++++++++++
> > >  grub-core/loader/arm64/fdt.c | 24 +-----------------------
> > >  include/grub/efi/efi.h       |  2 ++
> > >  3 files changed, 25 insertions(+), 23 deletions(-)
> > >
> > > diff --git a/grub-core/kern/efi/init.c b/grub-core/kern/efi/init.c
> > > index e9c85de..fb90ecd 100644
> > > --- a/grub-core/kern/efi/init.c
> > > +++ b/grub-core/kern/efi/init.c
> > > @@ -72,6 +72,28 @@ grub_machine_get_bootlocation (char **device, char **path)
> > >      }
> > >  }
> > >
> > > +void *
> > > +grub_efi_get_firmware_fdt (void)
> > > +{
> > > +  grub_efi_configuration_table_t *tables;
> > > +  grub_efi_guid_t fdt_guid = GRUB_EFI_DEVICE_TREE_GUID;
> > > +  void *firmware_fdt = NULL;
> > > +  unsigned int i;
> > > +
> > > +  /* Look for FDT in UEFI config tables. */
> > > +  tables = grub_efi_system_table->configuration_table;
> > > +
> > > +  for (i = 0; i < grub_efi_system_table->num_table_entries; i++)
> > > +    if (grub_memcmp (&tables[i].vendor_guid, &fdt_guid, sizeof (fdt_guid)) == 
> > > 0)
> > > +      {
> > > +       firmware_fdt = tables[i].vendor_table;
> > > +       grub_dprintf ("linux", "found registered FDT @ %p\n", firmware_fdt);
> > > +       break;
> > > +      }
> > > +
> > > +  return firmware_fdt;
> > > +}
> > > +
> >
> > Is it relevant for x86? I cannot even find anything related to FDT or
> > "device tree" in UEFI spec, it falls under vendor extensions. What other
> > use it has except in Linux loader?
> >
> > I really fail to see why it should be moved into core until at least one
> > more non-trivial caller is present.
>
> Hmmm... It looks that I was too fast. TBH, I can agree to some extend.
> Should we revert this patch or just #ifdef relevant code? Revert seems
> better for me.

Uh, the point of the function was to share code between 32bit and 64bit 
arm platforms which are completely separate archs.

There's also effort underway to run with device tree on x86:

  https://plus.google.com/+ChristopherFriedt/posts/SfDifuC1xsR


Alex