On Thu, Feb 21, 2019 at 12:42 PM Leif Lindholm <leif.lindh...@linaro.org> wrote: > > Hi Steve, > > On Mon, Feb 11, 2019 at 02:42:34AM +0000, Steve McIntyre wrote: > > Much like on x86, we can work out if the system is running on top of > > EFI firmware. If so, return "arm-efi". If not, fall back to > > "arm-uboot" as previously. > > Right, this clearly needs a fix. > > > Heavily inspired by the existing code for x86. > > Mmm. I would much prefer if we could break out the efi test in a > separate helper function. And clean it up while we're at it.
fyi, I made an attempt at this a while back: http://lists.gnu.org/archive/html/grub-devel/2018-08/msg00082.html http://lists.gnu.org/archive/html/grub-devel/2018-08/msg00081.html http://lists.gnu.org/archive/html/grub-devel/2018-08/msg00083.html -dann > > Signed-off-by: Steve McIntyre <93...@debian.org> > > --- > > grub-core/osdep/basic/platform.c | 6 ++++++ > > grub-core/osdep/linux/platform.c | 24 ++++++++++++++++++++++++ > > include/grub/util/install.h | 3 +++ > > util/grub-install.c | 2 +- > > 4 files changed, 34 insertions(+), 1 deletion(-) > > > > diff --git a/grub-core/osdep/basic/platform.c > > b/grub-core/osdep/basic/platform.c > > index 4b5502aeb..a7dafd85a 100644 > > --- a/grub-core/osdep/basic/platform.c > > +++ b/grub-core/osdep/basic/platform.c > > @@ -19,6 +19,12 @@ > > #include <grub/util/install.h> > > > > const char * > > +grub_install_get_default_arm_platform (void) > > +{ > > + return "arm-uboot"; > > +} > > + > > +const char * > > grub_install_get_default_x86_platform (void) > > { > > return "i386-pc"; > > diff --git a/grub-core/osdep/linux/platform.c > > b/grub-core/osdep/linux/platform.c > > index 775b6c031..a3f9e9d28 100644 > > --- a/grub-core/osdep/linux/platform.c > > +++ b/grub-core/osdep/linux/platform.c > > @@ -98,6 +98,30 @@ read_platform_size (void) > > } > > > > const char * > > +grub_install_get_default_arm_platform (void) > > +{ > > + /* > > + On Linux, we need the efivars kernel modules. > > + If no EFI is available this module just does nothing > > + besides a small hello and if we detect efi we'll load it > > + anyway later. So it should be safe to > > + try to load it here. > > + */ > > + grub_util_exec_redirect_all ((const char * []){ "modprobe", "efivars", > > NULL }, > > + NULL, NULL, "/dev/null"); > > So, I guess this is a "safe" operation. But efivars isn't the > interface that should be used these days - efivarfs is. > The efivars library will always use efivarfs > (mounted on /sys/firmware/efi/efivars) in preference to efivars > (available through /sys/firmware/efi/vars). > > Since it's safe, we may leave it in for someone running bleeding edge > grub on an ancient system, or just using a misconfigured kernel. > But the comment is misleading. I would suggest changing it to > something like: > > /* > Linux uses efivarfs (mounted on /sys/firmware/efi/efivars) > to access the EFI variable store. > Some legacy systems may still use the deprecated efivars > interface (accessed through /sys/firmware/efi/vars). > Where both are present, libefivar will use the former in > preference, so attempting to load efivars will not interfere > with later operations. > */ > > / > Leif > > > + grub_util_info ("Looking for /sys/firmware/efi .."); > > + if (is_not_empty_directory ("/sys/firmware/efi")) > > + { > > + grub_util_info ("...found"); > > + return "arm-efi"; > > + } > > + > > + grub_util_info ("... not found"); > > + return "arm-uboot"; > > +} > > + > > +const char * > > grub_install_get_default_x86_platform (void) > > { > > /* > > diff --git a/include/grub/util/install.h b/include/grub/util/install.h > > index af2bf65d7..80a51fcb1 100644 > > --- a/include/grub/util/install.h > > +++ b/include/grub/util/install.h > > @@ -209,6 +209,9 @@ void > > grub_install_create_envblk_file (const char *name); > > > > const char * > > +grub_install_get_default_arm_platform (void); > > + > > +const char * > > grub_install_get_default_x86_platform (void); > > > > int > > diff --git a/util/grub-install.c b/util/grub-install.c > > index 4a0a66168..1d68cc5bb 100644 > > --- a/util/grub-install.c > > +++ b/util/grub-install.c > > @@ -319,7 +319,7 @@ get_default_platform (void) > > #elif defined (__ia64__) > > return "ia64-efi"; > > #elif defined (__arm__) > > - return "arm-uboot"; > > + return grub_install_get_default_arm_platform (); > > #elif defined (__aarch64__) > > return "arm64-efi"; > > #elif defined (__amd64__) || defined (__x86_64__) || defined (__i386__) > > -- > > 2.11.0 > > > > > > _______________________________________________ > > Grub-devel mailing list > > Grub-devel@gnu.org > > https://lists.gnu.org/mailman/listinfo/grub-devel > > _______________________________________________ > Grub-devel mailing list > Grub-devel@gnu.org > https://lists.gnu.org/mailman/listinfo/grub-devel _______________________________________________ Grub-devel mailing list Grub-devel@gnu.org https://lists.gnu.org/mailman/listinfo/grub-devel