diff mbox series

grub-install: check for arm-efi as a default target

Message ID 20190221144611.27440-1-93sam@debian.org
State New
Headers show
Series grub-install: check for arm-efi as a default target | expand

Commit Message

Steve McIntyre Feb. 21, 2019, 2:46 p.m. UTC
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.

Split out the code to (maybe) load the efivar module and check for
/sys/firmware/efi into a common helper routine is_efi_system()

Signed-off-by: Steve McIntyre <93sam@debian.org>

---
 grub-core/osdep/basic/platform.c |  6 ++++++
 grub-core/osdep/linux/platform.c | 46 +++++++++++++++++++++++++++++++---------
 include/grub/util/install.h      |  3 +++
 util/grub-install.c              |  2 +-
 4 files changed, 46 insertions(+), 11 deletions(-)

-- 
2.11.0


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

Comments

Leif Lindholm Feb. 21, 2019, 3:04 p.m. UTC | #1
On Thu, Feb 21, 2019 at 02:46:11PM +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.

> 

> Split out the code to (maybe) load the efivar module and check for

> /sys/firmware/efi into a common helper routine is_efi_system()

> 

> Signed-off-by: Steve McIntyre <93sam@debian.org>


LGTM:
Reviewed-by: Leif Lindholm <leif.lindholm@linaro.org>


> ---

>  grub-core/osdep/basic/platform.c |  6 ++++++

>  grub-core/osdep/linux/platform.c | 46 +++++++++++++++++++++++++++++++---------

>  include/grub/util/install.h      |  3 +++

>  util/grub-install.c              |  2 +-

>  4 files changed, 46 insertions(+), 11 deletions(-)

> 

> 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..5cb7bf923 100644

> --- a/grub-core/osdep/linux/platform.c

> +++ b/grub-core/osdep/linux/platform.c

> @@ -97,16 +97,19 @@ read_platform_size (void)

>    return ret;

>  }

>  

> -const char *

> -grub_install_get_default_x86_platform (void)

> -{ 

> +/* Are we running on an EFI-based system? */

> +static int

> +is_efi_system (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.

> -   */

> +     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.

> +  */

>    grub_util_exec_redirect_all ((const char * []){ "modprobe", "efivars", NULL },

>  			       NULL, NULL, "/dev/null");

>  

> @@ -114,13 +117,36 @@ grub_install_get_default_x86_platform (void)

>    if (is_not_empty_directory ("/sys/firmware/efi"))

>      {

>        grub_util_info ("...found");

> +      return 1;

> +    }

> +  else

> +    {

> +      grub_util_info ("... not found");

> +      return 0;

> +    }

> +}

> +

> +const char *

> +grub_install_get_default_arm_platform (void)

> +{

> +  if (is_efi_system())

> +    return "arm-efi";

> +  else

> +    return "arm-uboot";

> +}

> +

> +const char *

> +grub_install_get_default_x86_platform (void)

> +{ 

> +  if (is_efi_system())

> +    {

>        if (read_platform_size() == 64)

>  	return "x86_64-efi";

>        else

>  	return "i386-efi";

>      }

>  

> -  grub_util_info ("... not found. Looking for /proc/device-tree ..");

> +  grub_util_info ("Looking for /proc/device-tree ..");

>    if (is_not_empty_directory ("/proc/device-tree"))

>      {

>        grub_util_info ("...found");

> 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
Daniel Kiper Feb. 21, 2019, 3:31 p.m. UTC | #2
On Thu, Feb 21, 2019 at 03:04:26PM +0000, Leif Lindholm wrote:
> On Thu, Feb 21, 2019 at 02:46:11PM +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.

> >

> > Split out the code to (maybe) load the efivar module and check for

> > /sys/firmware/efi into a common helper routine is_efi_system()

> >

> > Signed-off-by: Steve McIntyre <93sam@debian.org>

>

> LGTM:

> Reviewed-by: Leif Lindholm <leif.lindholm@linaro.org>


Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>


If there are no objections I will push this patch next week.

Daniel

_______________________________________________
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
Leif Lindholm Feb. 26, 2019, 1:27 p.m. UTC | #3
On Thu, Feb 21, 2019 at 04:31:32PM +0100, Daniel Kiper wrote:
> On Thu, Feb 21, 2019 at 03:04:26PM +0000, Leif Lindholm wrote:

> > On Thu, Feb 21, 2019 at 02:46:11PM +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.

> > >

> > > Split out the code to (maybe) load the efivar module and check for

> > > /sys/firmware/efi into a common helper routine is_efi_system()

> > >

> > > Signed-off-by: Steve McIntyre <93sam@debian.org>

> >

> > LGTM:

> > Reviewed-by: Leif Lindholm <leif.lindholm@linaro.org>

> 

> Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>

> 

> If there are no objections I will push this patch next week.


Eep.

As pointed out by Colin on IRC - it appears you have pushed Steves
original submission rather than the revised one which I gave my R-b
for.

Could you address?

Regards,

Leif

_______________________________________________
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
Daniel Kiper Feb. 26, 2019, 2:55 p.m. UTC | #4
On Tue, Feb 26, 2019 at 01:27:21PM +0000, Leif Lindholm wrote:
> On Thu, Feb 21, 2019 at 04:31:32PM +0100, Daniel Kiper wrote:

> > On Thu, Feb 21, 2019 at 03:04:26PM +0000, Leif Lindholm wrote:

> > > On Thu, Feb 21, 2019 at 02:46:11PM +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.

> > > >

> > > > Split out the code to (maybe) load the efivar module and check for

> > > > /sys/firmware/efi into a common helper routine is_efi_system()

> > > >

> > > > Signed-off-by: Steve McIntyre <93sam@debian.org>

> > >

> > > LGTM:

> > > Reviewed-by: Leif Lindholm <leif.lindholm@linaro.org>

> >

> > Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>

> >

> > If there are no objections I will push this patch next week.

>

> Eep.

>

> As pointed out by Colin on IRC - it appears you have pushed Steves

> original submission rather than the revised one which I gave my R-b

> for.

>

> Could you address?


Argh... Addressed. Sorry for the confusion. I took first email from
the thread by mistake. Folks, next time please send new patches as
not a reply to. Just create new threads.

Daniel

_______________________________________________
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
Steve McIntyre Feb. 26, 2019, 3:41 p.m. UTC | #5
On Tue, Feb 26, 2019 at 03:55:47PM +0100, Daniel Kiper wrote:
>On Tue, Feb 26, 2019 at 01:27:21PM +0000, Leif Lindholm wrote:

>>

>> As pointed out by Colin on IRC - it appears you have pushed Steves

>> original submission rather than the revised one which I gave my R-b

>> for.

>>

>> Could you address?

>

>Argh... Addressed. Sorry for the confusion. I took first email from

>the thread by mistake. Folks, next time please send new patches as

>not a reply to. Just create new threads.


ACK, will do!

-- 
Steve McIntyre, Cambridge, UK.                                steve@einval.com
"You can't barbecue lettuce!" -- Ellie Crane


_______________________________________________
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
diff mbox series

Patch

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..5cb7bf923 100644
--- a/grub-core/osdep/linux/platform.c
+++ b/grub-core/osdep/linux/platform.c
@@ -97,16 +97,19 @@  read_platform_size (void)
   return ret;
 }
 
-const char *
-grub_install_get_default_x86_platform (void)
-{ 
+/* Are we running on an EFI-based system? */
+static int
+is_efi_system (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.
-   */
+     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.
+  */
   grub_util_exec_redirect_all ((const char * []){ "modprobe", "efivars", NULL },
 			       NULL, NULL, "/dev/null");
 
@@ -114,13 +117,36 @@  grub_install_get_default_x86_platform (void)
   if (is_not_empty_directory ("/sys/firmware/efi"))
     {
       grub_util_info ("...found");
+      return 1;
+    }
+  else
+    {
+      grub_util_info ("... not found");
+      return 0;
+    }
+}
+
+const char *
+grub_install_get_default_arm_platform (void)
+{
+  if (is_efi_system())
+    return "arm-efi";
+  else
+    return "arm-uboot";
+}
+
+const char *
+grub_install_get_default_x86_platform (void)
+{ 
+  if (is_efi_system())
+    {
       if (read_platform_size() == 64)
 	return "x86_64-efi";
       else
 	return "i386-efi";
     }
 
-  grub_util_info ("... not found. Looking for /proc/device-tree ..");
+  grub_util_info ("Looking for /proc/device-tree ..");
   if (is_not_empty_directory ("/proc/device-tree"))
     {
       grub_util_info ("...found");
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__)