diff mbox series

[bug#40955,3/5] build: bootloader: Add install-efi procedure.

Message ID 20200429084756.25072-3-m.othacehe@gmail.com
State Accepted
Headers show
Series Add new image API. | expand

Checks

Context Check Description
cbaines/comparison success View comparision
cbaines/git branch success View Git branch
cbaines/applying patch success View Laminar job

Commit Message

Mathieu Othacehe April 29, 2020, 8:47 a.m. UTC
* gnu/build/bootloader.scm (install-efi-loader): New exported procedure. This
is based on install-efi from (gnu build vm) module.
* gnu/build/vm.scm (initialize-hard-disk): Adapt to use install-efi-loader.
---
 gnu/build/bootloader.scm | 55 +++++++++++++++++++++++++++++++++++++++-
 gnu/build/vm.scm         | 19 +++-----------
 2 files changed, 57 insertions(+), 17 deletions(-)

Comments

Ludovic Courtès May 2, 2020, 12:16 p.m. UTC | #1
Mathieu Othacehe <m.othacehe@gmail.com> skribis:

> * gnu/build/bootloader.scm (install-efi-loader): New exported procedure. This
> is based on install-efi from (gnu build vm) module.

Please mention the two new procedures.

> * gnu/build/vm.scm (initialize-hard-disk): Adapt to use install-efi-loader.

[…]

> +(define (install-efi-loader grub-efi esp)
> +  ;; Create a tiny configuration file telling the embedded grub
> +  ;; where to load the real thing.

Could you turn that into a docstring mentioning GRUB-EFI and ESP?

> +  ;; XXX This is quite fragile, and can prevent the image from booting
> +  ;; when there's more than one volume with this label present.
> +  ;; Reproducible almost-UUIDs could reduce the risk (not eliminate it).
> +  (let ((grub-config "grub.cfg"))
> +    (call-with-output-file grub-config
> +        (lambda (port)
         ^
Indentation is off.  :-)

> +          (format port
> +                  "insmod part_msdos~@
> +               search --set=root --label Guix_image~@
> +               configfile /boot/grub/grub.cfg~%")
> +          (fsync port)))

You sure we need an ‘fsync’ call here?  Perhaps add a comment explaining
why.

Otherwise LGTM!

Ludo’.
Mathieu Othacehe May 5, 2020, 1:43 p.m. UTC | #2
> You sure we need an ‘fsync’ call here?  Perhaps add a comment explaining
> why.

Turns out this is useless as the port is closed, hence everything should
be flushed.

Thanks,

Mathieu
diff mbox series

Patch

diff --git a/gnu/build/bootloader.scm b/gnu/build/bootloader.scm
index 9570d6dd18..e15e7c0828 100644
--- a/gnu/build/bootloader.scm
+++ b/gnu/build/bootloader.scm
@@ -18,8 +18,12 @@ 
 ;;; along with GNU Guix.  If not, see <http://www.gnu.org/licenses/>.
 
 (define-module (gnu build bootloader)
+  #:use-module (guix build utils)
+  #:use-module (guix utils)
   #:use-module (ice-9 binary-ports)
-  #:export (write-file-on-device))
+  #:use-module (ice-9 format)
+  #:export (write-file-on-device
+            install-efi-loader))
 
 
 ;;;
@@ -36,3 +40,52 @@ 
             (seek output offset SEEK_SET)
             (put-bytevector output bv))
           #:binary #t)))))
+
+
+;;;
+;;; EFI bootloader.
+;;;
+
+(define (install-efi grub grub-config esp)
+  "Write a self-contained GRUB EFI loader to the mounted ESP using GRUB-CONFIG."
+  (let* ((system %host-type)
+         ;; Hard code the output location to a well-known path recognized by
+         ;; compliant firmware. See "3.5.1.1 Removable Media Boot Behaviour":
+         ;; http://www.uefi.org/sites/default/files/resources/UEFI%20Spec%202_6.pdf
+         (grub-mkstandalone (string-append grub "/bin/grub-mkstandalone"))
+         (efi-directory (string-append esp "/EFI/BOOT"))
+         ;; Map grub target names to boot file names.
+         (efi-targets (cond ((string-prefix? "x86_64" system)
+                             '("x86_64-efi" . "BOOTX64.EFI"))
+                            ((string-prefix? "i686" system)
+                             '("i386-efi" . "BOOTIA32.EFI"))
+                            ((string-prefix? "armhf" system)
+                             '("arm-efi" . "BOOTARM.EFI"))
+                            ((string-prefix? "aarch64" system)
+                             '("arm64-efi" . "BOOTAA64.EFI")))))
+    ;; grub-mkstandalone requires a TMPDIR to prepare the firmware image.
+    (setenv "TMPDIR" esp)
+
+    (mkdir-p efi-directory)
+    (invoke grub-mkstandalone "-O" (car efi-targets)
+            "-o" (string-append efi-directory "/"
+                                (cdr efi-targets))
+            ;; Graft the configuration file onto the image.
+            (string-append "boot/grub/grub.cfg=" grub-config))))
+
+(define (install-efi-loader grub-efi esp)
+  ;; Create a tiny configuration file telling the embedded grub
+  ;; where to load the real thing.
+  ;; XXX This is quite fragile, and can prevent the image from booting
+  ;; when there's more than one volume with this label present.
+  ;; Reproducible almost-UUIDs could reduce the risk (not eliminate it).
+  (let ((grub-config "grub.cfg"))
+    (call-with-output-file grub-config
+        (lambda (port)
+          (format port
+                  "insmod part_msdos~@
+               search --set=root --label Guix_image~@
+               configfile /boot/grub/grub.cfg~%")
+          (fsync port)))
+    (install-efi grub-efi grub-config esp)
+    (delete-file grub-config)))
diff --git a/gnu/build/vm.scm b/gnu/build/vm.scm
index 9caa110463..bc6071daa9 100644
--- a/gnu/build/vm.scm
+++ b/gnu/build/vm.scm
@@ -27,6 +27,7 @@ 
   #:use-module (guix build store-copy)
   #:use-module (guix build syscalls)
   #:use-module (guix store database)
+  #:use-module (gnu build bootloader)
   #:use-module (gnu build linux-boot)
   #:use-module (gnu build install)
   #:use-module (gnu system uuid)
@@ -610,30 +611,16 @@  passing it a directory name where it is mounted."
 
     (when esp
       ;; Mount the ESP somewhere and install GRUB UEFI image.
-      (let ((mount-point (string-append target "/boot/efi"))
-            (grub-config (string-append target "/tmp/grub-standalone.cfg")))
+      (let ((mount-point (string-append target "/boot/efi")))
         (display "mounting EFI system partition...\n")
         (mkdir-p mount-point)
         (mount (partition-device esp) mount-point
                (partition-file-system esp))
 
-        ;; Create a tiny configuration file telling the embedded grub
-        ;; where to load the real thing.
-        ;; XXX This is quite fragile, and can prevent the image from booting
-        ;; when there's more than one volume with this label present.
-        ;; Reproducible almost-UUIDs could reduce the risk (not eliminate it).
-        (call-with-output-file grub-config
-          (lambda (port)
-            (format port
-                    "insmod part_msdos~@
-                    search --set=root --label Guix_image~@
-                    configfile /boot/grub/grub.cfg~%")))
-
         (display "creating EFI firmware image...")
-        (install-efi grub-efi mount-point grub-config)
+        (install-efi-loader grub-efi mount-point)
         (display "done.\n")
 
-        (delete-file grub-config)
         (umount mount-point)))
 
     ;; Register BOOTCFG as a GC root.