On Tue, Nov 15, 2016 at 10:07:39PM +0100, Alexander Graf wrote:
>
>
> 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 <ag...@suse.de>
> >>>---
> >>> 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
OK but right know we build FDT code for every EFI platform. This
is not nice because not every EFI platform uses/knows FDT (at least
today). So, I think that we should revert that patch and then you
can provide another patch which puts FDT code in separate file, e.g.
grub-core/kern/efi/fdt.c build only on platforms supporting FDT.
Does it make sense?
Daniel
_______________________________________________
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
