* [PATCH] efi: Move fdt helper into own file
@ 2016-11-18 12:50 Alexander Graf
2016-11-21 14:08 ` Daniel Kiper
0 siblings, 1 reply; 3+ messages in thread
From: Alexander Graf @ 2016-11-18 12:50 UTC (permalink / raw)
To: dkiper; +Cc: grub-devel, arvidjaar, leif.lindholm
We only support FDT files with EFI on arm and arm64 systems, not
on x86. So move the helper that finds a prepopulated FDT UUID
into its own file and only build it for architectures where it
also gets called.
Signed-off-by: Alexander Graf <agraf@suse.de>
---
grub-core/Makefile.core.def | 2 ++
grub-core/kern/efi/fdt.c | 50 +++++++++++++++++++++++++++++++++++++++++++++
grub-core/kern/efi/init.c | 22 --------------------
include/grub/efi/efi.h | 2 ++
4 files changed, 54 insertions(+), 22 deletions(-)
create mode 100644 grub-core/kern/efi/fdt.c
diff --git a/grub-core/Makefile.core.def b/grub-core/Makefile.core.def
index 21ad0dd..2dfa22a 100644
--- a/grub-core/Makefile.core.def
+++ b/grub-core/Makefile.core.def
@@ -212,8 +212,10 @@ kernel = {
arm_efi = kern/arm/efi/init.c;
arm_efi = kern/arm/efi/misc.c;
+ arm_efi = kern/efi/fdt.c;
arm64_efi = kern/arm64/efi/init.c;
+ arm64_efi = kern/efi/fdt.c;
i386_pc = kern/i386/pc/init.c;
i386_pc = kern/i386/pc/mmap.c;
diff --git a/grub-core/kern/efi/fdt.c b/grub-core/kern/efi/fdt.c
new file mode 100644
index 0000000..74a3669
--- /dev/null
+++ b/grub-core/kern/efi/fdt.c
@@ -0,0 +1,50 @@
+/* fdt.c - EFI Flattened Device Tree interaction */
+/*
+ * GRUB -- GRand Unified Bootloader
+ * Copyright (C) 2006,2007 Free Software Foundation, Inc.
+ *
+ * GRUB is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation, either version 3 of the License, or
+ * (at your option) any later version.
+ *
+ * GRUB is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with GRUB. If not, see <http://www.gnu.org/licenses/>.
+ */
+
+#include <grub/efi/efi.h>
+#include <grub/efi/console.h>
+#include <grub/efi/disk.h>
+#include <grub/term.h>
+#include <grub/misc.h>
+#include <grub/env.h>
+#include <grub/mm.h>
+#include <grub/kernel.h>
+
+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;
+}
+
diff --git a/grub-core/kern/efi/init.c b/grub-core/kern/efi/init.c
index fb90ecd..e9c85de 100644
--- a/grub-core/kern/efi/init.c
+++ b/grub-core/kern/efi/init.c
@@ -72,28 +72,6 @@ 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;
-}
-
void
grub_efi_fini (void)
{
diff --git a/include/grub/efi/efi.h b/include/grub/efi/efi.h
index 2acf85e..e9c601f 100644
--- a/include/grub/efi/efi.h
+++ b/include/grub/efi/efi.h
@@ -81,7 +81,9 @@ extern void (*EXPORT_VAR(grub_efi_net_config)) (grub_efi_handle_t hnd,
char **device,
char **path);
+#if defined(__arm__) || defined(__aarch64__)
void *EXPORT_FUNC(grub_efi_get_firmware_fdt)(void);
+#endif
grub_addr_t grub_efi_modules_addr (void);
--
1.8.5.6
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH] efi: Move fdt helper into own file
2016-11-18 12:50 [PATCH] efi: Move fdt helper into own file Alexander Graf
@ 2016-11-21 14:08 ` Daniel Kiper
2016-11-21 15:20 ` Alexander Graf
0 siblings, 1 reply; 3+ messages in thread
From: Daniel Kiper @ 2016-11-21 14:08 UTC (permalink / raw)
To: agraf, grub-devel; +Cc: dkiper, arvidjaar, leif.lindholm
On Fri, Nov 18, 2016 at 01:50:07PM +0100, Alexander Graf wrote:
> We only support FDT files with EFI on arm and arm64 systems, not
> on x86. So move the helper that finds a prepopulated FDT UUID
> into its own file and only build it for architectures where it
> also gets called.
>
> Signed-off-by: Alexander Graf <agraf@suse.de>
> ---
> grub-core/Makefile.core.def | 2 ++
> grub-core/kern/efi/fdt.c | 50 +++++++++++++++++++++++++++++++++++++++++++++
> grub-core/kern/efi/init.c | 22 --------------------
> include/grub/efi/efi.h | 2 ++
> 4 files changed, 54 insertions(+), 22 deletions(-)
> create mode 100644 grub-core/kern/efi/fdt.c
>
> diff --git a/grub-core/Makefile.core.def b/grub-core/Makefile.core.def
> index 21ad0dd..2dfa22a 100644
> --- a/grub-core/Makefile.core.def
> +++ b/grub-core/Makefile.core.def
> @@ -212,8 +212,10 @@ kernel = {
>
> arm_efi = kern/arm/efi/init.c;
> arm_efi = kern/arm/efi/misc.c;
> + arm_efi = kern/efi/fdt.c;
>
> arm64_efi = kern/arm64/efi/init.c;
> + arm64_efi = kern/efi/fdt.c;
>
> i386_pc = kern/i386/pc/init.c;
> i386_pc = kern/i386/pc/mmap.c;
> diff --git a/grub-core/kern/efi/fdt.c b/grub-core/kern/efi/fdt.c
> new file mode 100644
> index 0000000..74a3669
> --- /dev/null
> +++ b/grub-core/kern/efi/fdt.c
> @@ -0,0 +1,50 @@
> +/* fdt.c - EFI Flattened Device Tree interaction */
> +/*
> + * GRUB -- GRand Unified Bootloader
> + * Copyright (C) 2006,2007 Free Software Foundation, Inc.
2016?
> + *
> + * GRUB is free software: you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation, either version 3 of the License, or
> + * (at your option) any later version.
> + *
> + * GRUB is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> + * GNU General Public License for more details.
> + *
> + * You should have received a copy of the GNU General Public License
> + * along with GRUB. If not, see <http://www.gnu.org/licenses/>.
> + */
> +
> +#include <grub/efi/efi.h>
> +#include <grub/efi/console.h>
> +#include <grub/efi/disk.h>
> +#include <grub/term.h>
> +#include <grub/misc.h>
> +#include <grub/env.h>
> +#include <grub/mm.h>
> +#include <grub/kernel.h>
Do we really need all these headers here?
> +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;
> +}
> +
> diff --git a/grub-core/kern/efi/init.c b/grub-core/kern/efi/init.c
> index fb90ecd..e9c85de 100644
> --- a/grub-core/kern/efi/init.c
> +++ b/grub-core/kern/efi/init.c
> @@ -72,28 +72,6 @@ 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;
> -}
> -
> void
> grub_efi_fini (void)
> {
> diff --git a/include/grub/efi/efi.h b/include/grub/efi/efi.h
> index 2acf85e..e9c601f 100644
> --- a/include/grub/efi/efi.h
> +++ b/include/grub/efi/efi.h
> @@ -81,7 +81,9 @@ extern void (*EXPORT_VAR(grub_efi_net_config)) (grub_efi_handle_t hnd,
> char **device,
> char **path);
>
> +#if defined(__arm__) || defined(__aarch64__)
> void *EXPORT_FUNC(grub_efi_get_firmware_fdt)(void);
It seems to me that #if defined(... is not needed here.
Daniel
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] efi: Move fdt helper into own file
2016-11-21 14:08 ` Daniel Kiper
@ 2016-11-21 15:20 ` Alexander Graf
0 siblings, 0 replies; 3+ messages in thread
From: Alexander Graf @ 2016-11-21 15:20 UTC (permalink / raw)
To: Daniel Kiper, grub-devel; +Cc: arvidjaar, leif.lindholm
On 21/11/2016 15:08, Daniel Kiper wrote:
> On Fri, Nov 18, 2016 at 01:50:07PM +0100, Alexander Graf wrote:
>> We only support FDT files with EFI on arm and arm64 systems, not
>> on x86. So move the helper that finds a prepopulated FDT UUID
>> into its own file and only build it for architectures where it
>> also gets called.
>>
>> Signed-off-by: Alexander Graf <agraf@suse.de>
>> ---
>> grub-core/Makefile.core.def | 2 ++
>> grub-core/kern/efi/fdt.c | 50 +++++++++++++++++++++++++++++++++++++++++++++
>> grub-core/kern/efi/init.c | 22 --------------------
>> include/grub/efi/efi.h | 2 ++
>> 4 files changed, 54 insertions(+), 22 deletions(-)
>> create mode 100644 grub-core/kern/efi/fdt.c
>>
>> diff --git a/grub-core/Makefile.core.def b/grub-core/Makefile.core.def
>> index 21ad0dd..2dfa22a 100644
>> --- a/grub-core/Makefile.core.def
>> +++ b/grub-core/Makefile.core.def
>> @@ -212,8 +212,10 @@ kernel = {
>>
>> arm_efi = kern/arm/efi/init.c;
>> arm_efi = kern/arm/efi/misc.c;
>> + arm_efi = kern/efi/fdt.c;
>>
>> arm64_efi = kern/arm64/efi/init.c;
>> + arm64_efi = kern/efi/fdt.c;
>>
>> i386_pc = kern/i386/pc/init.c;
>> i386_pc = kern/i386/pc/mmap.c;
>> diff --git a/grub-core/kern/efi/fdt.c b/grub-core/kern/efi/fdt.c
>> new file mode 100644
>> index 0000000..74a3669
>> --- /dev/null
>> +++ b/grub-core/kern/efi/fdt.c
>> @@ -0,0 +1,50 @@
>> +/* fdt.c - EFI Flattened Device Tree interaction */
>> +/*
>> + * GRUB -- GRand Unified Bootloader
>> + * Copyright (C) 2006,2007 Free Software Foundation, Inc.
>
> 2016?
I just copied the header of the old file, as I didn't contribute
anything new to it really. I don't think adding a new copyright header
just because I split a file is reasonable.
>
>> + *
>> + * GRUB is free software: you can redistribute it and/or modify
>> + * it under the terms of the GNU General Public License as published by
>> + * the Free Software Foundation, either version 3 of the License, or
>> + * (at your option) any later version.
>> + *
>> + * GRUB is distributed in the hope that it will be useful,
>> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
>> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
>> + * GNU General Public License for more details.
>> + *
>> + * You should have received a copy of the GNU General Public License
>> + * along with GRUB. If not, see <http://www.gnu.org/licenses/>.
>> + */
>> +
>> +#include <grub/efi/efi.h>
>> +#include <grub/efi/console.h>
>> +#include <grub/efi/disk.h>
>> +#include <grub/term.h>
>> +#include <grub/misc.h>
>> +#include <grub/env.h>
>> +#include <grub/mm.h>
>> +#include <grub/kernel.h>
>
> Do we really need all these headers here?
Uh, probably not :). I'll condense them.
>
>> +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;
>> +}
>> +
>> diff --git a/grub-core/kern/efi/init.c b/grub-core/kern/efi/init.c
>> index fb90ecd..e9c85de 100644
>> --- a/grub-core/kern/efi/init.c
>> +++ b/grub-core/kern/efi/init.c
>> @@ -72,28 +72,6 @@ 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;
>> -}
>> -
>> void
>> grub_efi_fini (void)
>> {
>> diff --git a/include/grub/efi/efi.h b/include/grub/efi/efi.h
>> index 2acf85e..e9c601f 100644
>> --- a/include/grub/efi/efi.h
>> +++ b/include/grub/efi/efi.h
>> @@ -81,7 +81,9 @@ extern void (*EXPORT_VAR(grub_efi_net_config)) (grub_efi_handle_t hnd,
>> char **device,
>> char **path);
>>
>> +#if defined(__arm__) || defined(__aarch64__)
>> void *EXPORT_FUNC(grub_efi_get_firmware_fdt)(void);
>
> It seems to me that #if defined(... is not needed here.
That's what I thought at first too. But if you leave it there, the
function will get exposed in some module loader magic even on non-arm
systems. I thought the goal of this patch was to remove any trace of
this function on non-arm builds:
--- before 2016-11-21 16:18:47.654893320 +0100
+++ after 2016-11-21 16:19:23.203414706 +0100
@@ -1,5 +1,15 @@
./grub-core/kern/efi/fdt.c:grub_efi_get_firmware_fdt (void)
+./grub-core/symlist.c: {"grub_efi_get_firmware_fdt",
grub_efi_get_firmware_fdt, 1},
+Binary file ./grub-core/kernel.exec matches
+./grub-core/kernel_syms.lst:defined kernel grub_efi_get_firmware_fdt
+Binary file ./grub-core/kernel_exec-symlist.o matches
./grub-core/loader/arm64/fdt.c: raw_fdt = grub_efi_get_firmware_fdt();
+./grub-core/syminfo.lst:defined kernel grub_efi_get_firmware_fdt
+Binary file ./grub-core/kernel.img matches
+./before:./grub-core/kern/efi/fdt.c:grub_efi_get_firmware_fdt (void)
+./before:./grub-core/loader/arm64/fdt.c: raw_fdt =
grub_efi_get_firmware_fdt();
+./before:./include/grub/efi/efi.h:void
*EXPORT_FUNC(grub_efi_get_firmware_fdt)(void);
+./before:./include/grub/arm/linux.h:# define
grub_arm_firmware_get_boot_data (grub_addr_t)grub_efi_get_firmware_fdt
Binary file ./include/grub/efi/.efi.h.swp matches
./include/grub/efi/efi.h:void
*EXPORT_FUNC(grub_efi_get_firmware_fdt)(void);
./include/grub/arm/linux.h:# define grub_arm_firmware_get_boot_data
(grub_addr_t)grub_efi_get_firmware_fdt
Alex
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2016-11-21 15:20 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-11-18 12:50 [PATCH] efi: Move fdt helper into own file Alexander Graf
2016-11-21 14:08 ` Daniel Kiper
2016-11-21 15:20 ` Alexander Graf
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).