bug#26815: [PATCH 0/3] Hybrid UEFI disk image

bug#26815: [PATCH 0/3] Hybrid UEFI disk image

[Marius Bakke]

This ended up a lot simpler than anticipated.

These patches are on top of <https://bugs.gnu.org/26339>, but can be
ported to master since it ultimately didn't use any of the new tricks.

Test with:
  $ cp $(./pre-inst-env guix system disk-image --image-size=1G \
    gnu/system/install.scm) install.img
  $ chmod 644 install.img
  $ qemu-system-x86_64 -enable-kvm -m 512M -serial stdio \
    -bios $(guix build ovmf)/share/firmware/ovmf_x64.bin \
    -drive file=install.img,if=virtio

One caveat is that it probably only works on most(?) 64-bit systems.
Please test it on real hardware, especially older EFI systems. Not sure
what to do about 32-bit yet.

Marius Bakke (3):
  vm: Add support for arbitrary partition flags.
  vm: Support creating FAT partitions.
  vm: Support EFI boot in base image.

 gnu/build/vm.scm            | 98 +++++++++++++++++++++++++++++++++++++++------
 gnu/system/linux-initrd.scm |  4 +-
 gnu/system/vm.scm           | 16 ++++++--
 3 files changed, 98 insertions(+), 20 deletions(-)

-- 
2.12.2

bug#26815: [PATCH 1/3] vm: Add support for arbitrary partition flags.

[Marius Bakke]

* gnu/build/vm.scm (<partition>): Change BOOTABLE? to FLAGS.
(initialize-partition-table): Pass each flag to parted.
(initialize-hard-disk): Search for root partition by "boot" flag.
* gnu/system/vm.scm (qemu-image): Adjust partitions accordingly.
---
 gnu/build/vm.scm  | 18 +++++++++++++-----
 gnu/system/vm.scm |  2 +-
 2 files changed, 14 insertions(+), 6 deletions(-)

diff --git a/gnu/build/vm.scm b/gnu/build/vm.scm
index 440596a40..3286ffb02 100644
--- a/gnu/build/vm.scm
+++ b/gnu/build/vm.scm
@@ -3,6 +3,7 @@
 ;;; Copyright © 2016 Christopher Allan Webber <cwebber@dustycloud.org>
 ;;; Copyright © 2016 Leo Famulari <leo@famulari.name>
 ;;; Copyright © 2017 Mathieu Othacehe <m.othacehe@gmail.com>
+;;; Copyright © 2017 Marius Bakke <mbakke@fastmail.com>
 ;;;
 ;;; This file is part of GNU Guix.
 ;;;
@@ -41,7 +42,7 @@
             partition-size
             partition-file-system
             partition-label
-            partition-bootable?
+            partition-flags
             partition-initializer
 
             root-partition-initializer
@@ -141,7 +142,7 @@ the #:references-graphs parameter of 'derivation'."
   (size        partition-size)
   (file-system partition-file-system (default "ext4"))
   (label       partition-label (default #f))
-  (bootable?   partition-bootable? (default #f))
+  (flags       partition-flags (default '()))
   (initializer partition-initializer (default (const #t))))
 
 (define (fold2 proc seed1 seed2 lst)              ;TODO: factorize
@@ -168,9 +169,11 @@ actual /dev name based on DEVICE."
     (cons* "mkpart" "primary" "ext2"
            (format #f "~aB" offset)
            (format #f "~aB" (+ offset (partition-size part)))
-           (if (partition-bootable? part)
-               `("set" ,(number->string index) "boot" "on")
-               '())))
+           (apply append (map (lambda (flag)
+                                (cons* "set" (number->string index)
+                                       (symbol->string flag)
+                                       "on" '()))
+                              (partition-flags part)))))
 
   (define (options partitions offset)
     (let loop ((partitions partitions)
@@ -303,6 +306,11 @@ in PARTITIONS, and using BOOTCFG as its bootloader configuration file.
 
 Each partition is initialized by calling its 'initializer' procedure,
 passing it a directory name where it is mounted."
+
+  (define (partition-bootable? partitions)
+    "Return the first partition found with the boot flag set."
+    (member 'boot (partition-flags partitions)))
+
   (let* ((partitions (initialize-partition-table device partitions))
          (root       (find partition-bootable? partitions))
          (target     "/fs"))
diff --git a/gnu/system/vm.scm b/gnu/system/vm.scm
index cc13abe2a..42c7690b1 100644
--- a/gnu/system/vm.scm
+++ b/gnu/system/vm.scm
@@ -230,7 +230,7 @@ the image."
                                                 (* 10 (expt 2 20))))
                                      (label #$file-system-label)
                                      (file-system #$file-system-type)
-                                     (bootable? #t)
+                                     (flags '(boot))
                                      (initializer initialize)))))
              (initialize-hard-disk "/dev/vda"
                                    #:partitions partitions
-- 
2.12.2

bug#26815: [PATCH 2/3] vm: Support creating FAT partitions.

[Marius Bakke]

* gnu/build/vm.scm (create-ext-file-system, create-fat-file-system): New procedures.
(format-partition): Use procedures. Error for unknown file systems.
* gnu/system/vm.scm (qemu-image): Add DOSFSTOOLS to the closure.
* gnu/system/linux-initrd.scm (base-initrd): Add nls_is8859-1.ko regardless of
whether a FAT filesystem is present.
---
 gnu/build/vm.scm            | 43 ++++++++++++++++++++++++++++++++++++-------
 gnu/system/linux-initrd.scm |  4 +---
 gnu/system/vm.scm           |  2 +-
 3 files changed, 38 insertions(+), 11 deletions(-)

diff --git a/gnu/build/vm.scm b/gnu/build/vm.scm
index 3286ffb02..ad39e29ce 100644
--- a/gnu/build/vm.scm
+++ b/gnu/build/vm.scm
@@ -214,17 +214,46 @@ actual /dev name based on DEVICE."
 
 (define MS_BIND 4096)                             ; <sys/mounts.h> again!
 
+(define* (create-ext-file-system partition type
+                                 #:key label)
+  "Create an ext-family filesystem of TYPE on PARTITION.  If LABEL is true,
+use that as the volume name."
+  (format #t "creating ~a partition...\n" type)
+  (apply system* (string-append "mkfs." type)
+         "-F" partition
+         (if label
+             `("-L" ,label)
+             '())))
+
+(define* (create-fat32-file-system partition
+                                   #:key label)
+  "Create a FAT32 filesystem on PARTITION, which must be at least 32 MiB long.
+If LABEL is true, use that as volume name."
+  (format #t "Creating FAT32 partition...\n")
+  ;; Without the -F parameter, mkfs.fat will automatically determine
+  ;; the number of file allocation tables based on partition size.
+  ;; Ensure a FAT32 partition for compatibility with e.g. UEFI.
+  (apply system* "mkfs.fat" "-F32" partition
+         (if label
+             `("-n" ,label)
+             '())))
+
 (define* (format-partition partition type
                            #:key label)
   "Create a file system TYPE on PARTITION.  If LABEL is true, use that as the
 volume name."
-  (format #t "creating ~a partition...\n" type)
-  (unless (zero? (apply system* (string-append "mkfs." type)
-                        "-F" partition
-                        (if label
-                            `("-L" ,label)
-                            '())))
-    (error "failed to create partition")))
+  (define format-procedure
+    (cond
+     ((string-prefix? "ext" type)
+      (create-ext-file-system partition type #:label label))
+     ((string-suffix? "fat" type)
+      (create-fat32-file-system partition #:label label))
+     (else #f)))
+  (if format-procedure
+      (match (status:exit-val format-procedure)
+        (0 #t)
+        (_ (error "Formatting partition failed.")))
+      (error "Unsupported file system.")))
 
 (define (initialize-partition partition)
   "Format PARTITION, a <partition> object with a non-#f 'device' field, mount
diff --git a/gnu/system/linux-initrd.scm b/gnu/system/linux-initrd.scm
index dfe198e43..3a5e76034 100644
--- a/gnu/system/linux-initrd.scm
+++ b/gnu/system/linux-initrd.scm
@@ -268,6 +268,7 @@ loaded at boot time in the order in which they appear."
       "usbhid" "hid-generic" "hid-apple"      ;keyboards during early boot
       "dm-crypt" "xts" "serpent_generic" "wp512" ;for encrypted root partitions
       "nvme"                                     ;for new SSD NVMe devices
+      "nls_iso8859-1"                            ;for `mkfs.fat`, et.al
       ,@(if (string-match "^(x86_64|i[3-6]86)-" (%current-system))
             '("pata_acpi" "pata_atiixp"    ;for ATA controllers
               "isci")                      ;for SAS controllers like Intel C602
@@ -281,9 +282,6 @@ loaded at boot time in the order in which they appear."
       ,@(if (find (file-system-type-predicate "9p") file-systems)
             virtio-9p-modules
             '())
-      ,@(if (find (file-system-type-predicate "vfat") file-systems)
-            '("nls_iso8859-1")
-            '())
       ,@(if (find (file-system-type-predicate "btrfs") file-systems)
             '("btrfs")
             '())
diff --git a/gnu/system/vm.scm b/gnu/system/vm.scm
index 42c7690b1..099e3fac3 100644
--- a/gnu/system/vm.scm
+++ b/gnu/system/vm.scm
@@ -202,7 +202,7 @@ the image."
                       (guix build utils))
 
          (let ((inputs
-                '#$(append (list qemu parted e2fsprogs)
+                '#$(append (list qemu parted e2fsprogs dosfstools)
                            (map canonical-package
                                 (list sed grep coreutils findutils gawk))
                            (if register-closures? (list guix) '())))
-- 
2.12.2

bug#26815: [PATCH 2/3] vm: Support creating FAT partitions.

[Danny Milosavljevic]

Hi Marius,

On Sun,  7 May 2017 16:36:46 +0200
Marius Bakke <mbakke@fastmail.com> wrote:
> +(define* (create-ext-file-system partition type
> +                                 #:key label)
> +  "Create an ext-family filesystem of TYPE on PARTITION.  If LABEL is true,
> +use that as the volume name."
> +  (format #t "creating ~a partition...\n" type)
> +  (apply system* (string-append "mkfs." type)
> +         "-F" partition
> +         (if label
> +             `("-L" ,label)
> +             '())))

Could you document that the result of the procedure is the system* status?  Also, is that wise?  I think it should instead do the error handling and (error ...) out on error.  It's longer but less surprising.

> +(define* (create-fat32-file-system partition
> +                                   #:key label)
> +  "Create a FAT32 filesystem on PARTITION, which must be at least 32 MiB long.
> +If LABEL is true, use that as volume name."

Same as above.

>  (define* (format-partition partition type
[...]
> +  (define format-procedure
> +    (cond
> +     ((string-prefix? "ext" type)
> +      (create-ext-file-system partition type #:label label))
> +     ((string-suffix? "fat" type)
> +      (create-fat32-file-system partition #:label label))
> +     (else #f)))

"format-procedure" is not actually the procedure, right? It's already the formatting-status ...

> +  (if format-procedure
> +      (match (status:exit-val format-procedure)
> +        (0 #t)
> +        (_ (error "Formatting partition failed.")))
> +      (error "Unsupported file system.")))

status:exit-val will fail when given #f, which it will get in the error case of format-procedure.

scheme@(guile-user)> (status:exit-val #f)
ERROR: In procedure status:exit-val:
ERROR: Wrong type (expecting exact integer): #f

> +      "nls_iso8859-1"                            ;for `mkfs.fat`, et.al

This adds nls_iso8859-1 unconditionally. OK.

Otherwise LGTM.

bug#26815: [PATCH 2/3] vm: Support creating FAT partitions.

[Marius Bakke]

[-- Attachment #1: Type: text/plain, Size: 2543 bytes --]

Danny Milosavljevic <dannym@scratchpost.org> writes:

> Hi Marius,
>
> On Sun,  7 May 2017 16:36:46 +0200
> Marius Bakke <mbakke@fastmail.com> wrote:
>> +(define* (create-ext-file-system partition type
>> +                                 #:key label)
>> +  "Create an ext-family filesystem of TYPE on PARTITION.  If LABEL is true,
>> +use that as the volume name."
>> +  (format #t "creating ~a partition...\n" type)
>> +  (apply system* (string-append "mkfs." type)
>> +         "-F" partition
>> +         (if label
>> +             `("-L" ,label)
>> +             '())))
>
> Could you document that the result of the procedure is the system* status?  Also, is that wise?  I think it should instead do the error handling and (error ...) out on error.  It's longer but less surprising.

I had that first, but the error handling was exactly identical, so opted
to just handle it in the caller. It does sound safer to handle errors
there instead of passing system* around though, will do that in lieu of
other comments. 

>> +(define* (create-fat32-file-system partition
>> +                                   #:key label)
>> +  "Create a FAT32 filesystem on PARTITION, which must be at least 32 MiB long.
>> +If LABEL is true, use that as volume name."
>
> Same as above.
>
>>  (define* (format-partition partition type
> [...]
>> +  (define format-procedure
>> +    (cond
>> +     ((string-prefix? "ext" type)
>> +      (create-ext-file-system partition type #:label label))
>> +     ((string-suffix? "fat" type)
>> +      (create-fat32-file-system partition #:label label))
>> +     (else #f)))
>
> "format-procedure" is not actually the procedure, right? It's already the formatting-status ...

Oops, an artifact of rebasing a lot of revisions...

>> +  (if format-procedure
>> +      (match (status:exit-val format-procedure)
>> +        (0 #t)
>> +        (_ (error "Formatting partition failed.")))
>> +      (error "Unsupported file system.")))
>
> status:exit-val will fail when given #f, which it will get in the error case of format-procedure.

Thanks for pointing that out. Will re-submit this patch shortly.

> scheme@(guile-user)> (status:exit-val #f)
> ERROR: In procedure status:exit-val:
> ERROR: Wrong type (expecting exact integer): #f
>
>> +      "nls_iso8859-1"                            ;for `mkfs.fat`, et.al
>
> This adds nls_iso8859-1 unconditionally. OK.

It's required by "dosfstools" which is also added unconditionally.
Not sure how to improve it.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 487 bytes --]

bug#26815: [PATCH v2 2/3] vm: Support creating FAT partitions.

[Marius Bakke]

* gnu/build/vm.scm (create-ext-file-system, create-fat-file-system): New procedures.
(format-partition): Use them. Error for unknown file systems.
* gnu/system/vm.scm (qemu-image): Add DOSFSTOOLS to the closure.
* gnu/system/linux-initrd.scm (base-initrd): Add nls_is8859-1.ko regardless of
whether a FAT filesystem is present.
---
 gnu/build/vm.scm            | 35 +++++++++++++++++++++++++++++++----
 gnu/system/linux-initrd.scm |  4 +---
 gnu/system/vm.scm           |  2 +-
 3 files changed, 33 insertions(+), 8 deletions(-)

diff --git a/gnu/build/vm.scm b/gnu/build/vm.scm
index 3286ffb02..0b9622bc6 100644
--- a/gnu/build/vm.scm
+++ b/gnu/build/vm.scm
@@ -214,10 +214,10 @@ actual /dev name based on DEVICE."
 
 (define MS_BIND 4096)                             ; <sys/mounts.h> again!
 
-(define* (format-partition partition type
-                           #:key label)
-  "Create a file system TYPE on PARTITION.  If LABEL is true, use that as the
-volume name."
+(define* (create-ext-file-system partition type
+                                 #:key label)
+  "Create an ext-family filesystem of TYPE on PARTITION.  If LABEL is true,
+use that as the volume name."
   (format #t "creating ~a partition...\n" type)
   (unless (zero? (apply system* (string-append "mkfs." type)
                         "-F" partition
@@ -226,6 +226,33 @@ volume name."
                             '())))
     (error "failed to create partition")))
 
+(define* (create-fat-file-system partition
+                                 #:key label)
+  "Create a FAT filesystem on PARTITION.  The number of File Allocation Tables
+will be determined based on filesystem size.  If LABEL is true, use that as the
+volume name."
+  (format #t "creating FAT partition...\n")
+  (unless (zero? (apply system* "mkfs.fat" partition
+                        (if label
+                            `("-n" ,label)
+                            '())))
+    (error "failed to create FAT partition")))
+
+(define* (format-partition partition type
+                           #:key label)
+  "Create a file system TYPE on PARTITION.  If LABEL is true, use that as the
+volume name."
+  (define format-procedure
+    (cond
+     ((string-prefix? "ext" type)
+      (create-ext-file-system partition type #:label label))
+     ((string-suffix? "fat" type)
+      (create-fat-file-system partition #:label label))
+     (else #f)))
+  (if format-procedure
+      format-procedure
+      (error "Unsupported file system.")))
+
 (define (initialize-partition partition)
   "Format PARTITION, a <partition> object with a non-#f 'device' field, mount
 it, run its initializer, and unmount it."
diff --git a/gnu/system/linux-initrd.scm b/gnu/system/linux-initrd.scm
index dfe198e43..3a5e76034 100644
--- a/gnu/system/linux-initrd.scm
+++ b/gnu/system/linux-initrd.scm
@@ -268,6 +268,7 @@ loaded at boot time in the order in which they appear."
       "usbhid" "hid-generic" "hid-apple"      ;keyboards during early boot
       "dm-crypt" "xts" "serpent_generic" "wp512" ;for encrypted root partitions
       "nvme"                                     ;for new SSD NVMe devices
+      "nls_iso8859-1"                            ;for `mkfs.fat`, et.al
       ,@(if (string-match "^(x86_64|i[3-6]86)-" (%current-system))
             '("pata_acpi" "pata_atiixp"    ;for ATA controllers
               "isci")                      ;for SAS controllers like Intel C602
@@ -281,9 +282,6 @@ loaded at boot time in the order in which they appear."
       ,@(if (find (file-system-type-predicate "9p") file-systems)
             virtio-9p-modules
             '())
-      ,@(if (find (file-system-type-predicate "vfat") file-systems)
-            '("nls_iso8859-1")
-            '())
       ,@(if (find (file-system-type-predicate "btrfs") file-systems)
             '("btrfs")
             '())
diff --git a/gnu/system/vm.scm b/gnu/system/vm.scm
index 42c7690b1..099e3fac3 100644
--- a/gnu/system/vm.scm
+++ b/gnu/system/vm.scm
@@ -202,7 +202,7 @@ the image."
                       (guix build utils))
 
          (let ((inputs
-                '#$(append (list qemu parted e2fsprogs)
+                '#$(append (list qemu parted e2fsprogs dosfstools)
                            (map canonical-package
                                 (list sed grep coreutils findutils gawk))
                            (if register-closures? (list guix) '())))
-- 
2.12.2

bug#26815: [PATCH 2/3] vm: Support creating FAT partitions.

[Danny Milosavljevic]

Hi Marius,

On Sun, 07 May 2017 17:52:43 +0200
Marius Bakke <mbakke@fastmail.com> wrote:

> I had that first, but the error handling was exactly identical, so opted
> to just handle it in the caller. It does sound safer to handle errors
> there instead of passing system* around though, will do that in lieu of
> other comments. 

Yeah, I did that in the past, too (and for many things still do).  But this procedure is critical enough that ignoring the return value would do some very strange things - and for those I lately do defensive programming.

> > "format-procedure" is not actually the procedure, right? It's already the formatting-status ...  
> 
> Oops, an artifact of rebasing a lot of revisions...

Your v2 patch still calls it that :)

> > This adds nls_iso8859-1 unconditionally. OK.  
> 
> It's required by "dosfstools" which is also added unconditionally.
> Not sure how to improve it.

Oh, it's fine.  It's just for other reviewers and future-me so that I don't have to think about why this is done again...

bug#26815: [PATCH 2/3] vm: Support creating FAT partitions.

[Marius Bakke]

[-- Attachment #1.1: Type: text/plain, Size: 948 bytes --]

Danny Milosavljevic <dannym@scratchpost.org> writes:

> Hi Marius,
>
> On Sun, 07 May 2017 17:52:43 +0200
> Marius Bakke <mbakke@fastmail.com> wrote:
>
>> I had that first, but the error handling was exactly identical, so opted
>> to just handle it in the caller. It does sound safer to handle errors
>> there instead of passing system* around though, will do that in lieu of
>> other comments. 
>
> Yeah, I did that in the past, too (and for many things still do).  But this procedure is critical enough that ignoring the return value would do some very strange things - and for those I lately do defensive programming.
>
>> > "format-procedure" is not actually the procedure, right? It's already the formatting-status ...  
>> 
>> Oops, an artifact of rebasing a lot of revisions...
>
> Your v2 patch still calls it that :)

D'oh. It could of course be much simpler after removing the error
handling too..

v3 attached.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1.2: 0001-vm-Support-creating-FAT-partitions.patch --]
[-- Type: text/x-patch, Size: 4628 bytes --]

From 44c112dfe0d92847f32040d558f5970ae99c600d Mon Sep 17 00:00:00 2001
From: Marius Bakke <mbakke@fastmail.com>
Date: Sat, 6 May 2017 22:53:58 +0200
Subject: [PATCH] vm: Support creating FAT partitions.

* gnu/build/vm.scm (create-ext-file-system, create-fat-file-system): New procedures.
(format-partition): Use procedures. Error for unknown file systems.
* gnu/system/vm.scm (qemu-image): Add DOSFSTOOLS to the closure.
* gnu/system/linux-initrd.scm (base-initrd): Add nls_is8859-1.ko regardless of
whether a FAT filesystem is present.
---
 gnu/build/vm.scm            | 31 +++++++++++++++++++++++++++----
 gnu/system/linux-initrd.scm |  4 +---
 gnu/system/vm.scm           |  2 +-
 3 files changed, 29 insertions(+), 8 deletions(-)

diff --git a/gnu/build/vm.scm b/gnu/build/vm.scm
index 3286ffb02..206696ef6 100644
--- a/gnu/build/vm.scm
+++ b/gnu/build/vm.scm
@@ -214,10 +214,10 @@ actual /dev name based on DEVICE."
 
 (define MS_BIND 4096)                             ; <sys/mounts.h> again!
 
-(define* (format-partition partition type
-                           #:key label)
-  "Create a file system TYPE on PARTITION.  If LABEL is true, use that as the
-volume name."
+(define* (create-ext-file-system partition type
+                                 #:key label)
+  "Create an ext-family filesystem of TYPE on PARTITION.  If LABEL is true,
+use that as the volume name."
   (format #t "creating ~a partition...\n" type)
   (unless (zero? (apply system* (string-append "mkfs." type)
                         "-F" partition
@@ -226,6 +226,29 @@ volume name."
                             '())))
     (error "failed to create partition")))
 
+(define* (create-fat-file-system partition
+                                 #:key label)
+  "Create a FAT filesystem on PARTITION.  The number of File Allocation Tables
+will be determined based on filesystem size.  If LABEL is true, use that as the
+volume name."
+  (format #t "creating FAT partition...\n")
+  (unless (zero? (apply system* "mkfs.fat" partition
+                        (if label
+                            `("-n" ,label)
+                            '())))
+    (error "failed to create FAT partition")))
+
+(define* (format-partition partition type
+                           #:key label)
+  "Create a file system TYPE on PARTITION.  If LABEL is true, use that as the
+volume name."
+  (cond
+   ((string-prefix? "ext" type)
+    (create-ext-file-system partition type #:label label))
+   ((string-suffix? "fat" type)
+    (create-fat-file-system partition #:label label))
+     (else (error "Unsupported file system."))))
+
 (define (initialize-partition partition)
   "Format PARTITION, a <partition> object with a non-#f 'device' field, mount
 it, run its initializer, and unmount it."
diff --git a/gnu/system/linux-initrd.scm b/gnu/system/linux-initrd.scm
index dfe198e43..3a5e76034 100644
--- a/gnu/system/linux-initrd.scm
+++ b/gnu/system/linux-initrd.scm
@@ -268,6 +268,7 @@ loaded at boot time in the order in which they appear."
       "usbhid" "hid-generic" "hid-apple"      ;keyboards during early boot
       "dm-crypt" "xts" "serpent_generic" "wp512" ;for encrypted root partitions
       "nvme"                                     ;for new SSD NVMe devices
+      "nls_iso8859-1"                            ;for `mkfs.fat`, et.al
       ,@(if (string-match "^(x86_64|i[3-6]86)-" (%current-system))
             '("pata_acpi" "pata_atiixp"    ;for ATA controllers
               "isci")                      ;for SAS controllers like Intel C602
@@ -281,9 +282,6 @@ loaded at boot time in the order in which they appear."
       ,@(if (find (file-system-type-predicate "9p") file-systems)
             virtio-9p-modules
             '())
-      ,@(if (find (file-system-type-predicate "vfat") file-systems)
-            '("nls_iso8859-1")
-            '())
       ,@(if (find (file-system-type-predicate "btrfs") file-systems)
             '("btrfs")
             '())
diff --git a/gnu/system/vm.scm b/gnu/system/vm.scm
index 42c7690b1..099e3fac3 100644
--- a/gnu/system/vm.scm
+++ b/gnu/system/vm.scm
@@ -202,7 +202,7 @@ the image."
                       (guix build utils))
 
          (let ((inputs
-                '#$(append (list qemu parted e2fsprogs)
+                '#$(append (list qemu parted e2fsprogs dosfstools)
                            (map canonical-package
                                 (list sed grep coreutils findutils gawk))
                            (if register-closures? (list guix) '())))
-- 
2.12.2


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 487 bytes --]

bug#26815: [PATCH 2/3] vm: Support creating FAT partitions.

[Danny Milosavljevic]

>+(define* (format-partition partition type
>+                           #:key label)
>+  "Create a file system TYPE on PARTITION.  If LABEL is true, use that as the
>+volume name."
>+  (cond
>+   ((string-prefix? "ext" type)
>+    (create-ext-file-system partition type #:label label))
>+   ((string-suffix? "fat" type)
>+    (create-fat-file-system partition #:label label))
>+     (else (error "Unsupported file system."))))

       ^^^ Indentation of the "else" is strange.

Otherwise LGTM!

bug#26815: [PATCH 2/3] vm: Support creating FAT partitions.

[Ludovic Courtès]

Danny Milosavljevic <dannym@scratchpost.org> skribis:

>>+(define* (format-partition partition type
>>+                           #:key label)
>>+  "Create a file system TYPE on PARTITION.  If LABEL is true, use that as the
>>+volume name."
>>+  (cond
>>+   ((string-prefix? "ext" type)
>>+    (create-ext-file-system partition type #:label label))
>>+   ((string-suffix? "fat" type)
>>+    (create-fat-file-system partition #:label label))
>>+     (else (error "Unsupported file system."))))
>
>        ^^^ Indentation of the "else" is strange.
>
> Otherwise LGTM!

Same here!

Ludo'.

bug#26815: [PATCH 2/3] vm: Support creating FAT partitions.

[Maxim Cournoyer]

Hi,

Marius Bakke <mbakke@fastmail.com> writes:

> * gnu/build/vm.scm (create-ext-file-system, create-fat-file-system): New procedures.
> (format-partition): Use procedures. Error for unknown file systems.
> * gnu/system/vm.scm (qemu-image): Add DOSFSTOOLS to the closure.
> * gnu/system/linux-initrd.scm (base-initrd): Add nls_is8859-1.ko regardless of
> whether a FAT filesystem is present.
> ---
>  gnu/build/vm.scm            | 43 ++++++++++++++++++++++++++++++++++++-------
>  gnu/system/linux-initrd.scm |  4 +---
>  gnu/system/vm.scm           |  2 +-
>  3 files changed, 38 insertions(+), 11 deletions(-)
>
> diff --git a/gnu/build/vm.scm b/gnu/build/vm.scm
> index 3286ffb02..ad39e29ce 100644
> --- a/gnu/build/vm.scm
> +++ b/gnu/build/vm.scm
> @@ -214,17 +214,46 @@ actual /dev name based on DEVICE."
>  
>  (define MS_BIND 4096)                             ; <sys/mounts.h> again!
>  
> +(define* (create-ext-file-system partition type
> +                                 #:key label)
> +  "Create an ext-family filesystem of TYPE on PARTITION.  If LABEL is true,
> +use that as the volume name."
> +  (format #t "creating ~a partition...\n" type)
> +  (apply system* (string-append "mkfs." type)

Is there a reason why we don't simply call system* directly here? And
similarly below. I don't see the gain of using `apply'. Maybe I'm
missing something.

> +         "-F" partition
> +         (if label
> +             `("-L" ,label)
> +             '())))
> +
> +(define* (create-fat32-file-system partition
> +                                   #:key label)
> +  "Create a FAT32 filesystem on PARTITION, which must be at least 32 MiB long.
> +If LABEL is true, use that as volume name."
> +  (format #t "Creating FAT32 partition...\n")
> +  ;; Without the -F parameter, mkfs.fat will automatically determine
> +  ;; the number of file allocation tables based on partition size.
> +  ;; Ensure a FAT32 partition for compatibility with e.g. UEFI.
> +  (apply system* "mkfs.fat" "-F32" partition
> +         (if label
> +             `("-n" ,label)
> +             '())))
> +

Maxim

bug#26815: [PATCH 3/3] vm: Support EFI boot in base image.

[Marius Bakke]

* gnu/system/vm.scm (qemu-image): Add GRUB-EFI to inputs. Append 40MB
EFI System Partition.
* gnu/build/vm.scm (initialize-hard-disk): Generate grub EFI blob when ESP is
present.
---
 gnu/build/vm.scm  | 37 ++++++++++++++++++++++++++++++++++++-
 gnu/system/vm.scm | 14 +++++++++++---
 2 files changed, 47 insertions(+), 4 deletions(-)

diff --git a/gnu/build/vm.scm b/gnu/build/vm.scm
index ad39e29ce..a1f419776 100644
--- a/gnu/build/vm.scm
+++ b/gnu/build/vm.scm
@@ -27,6 +27,7 @@
   #:use-module (gnu build linux-boot)
   #:use-module (gnu build install)
   #:use-module (guix records)
+  #:use-module (ice-9 format)
   #:use-module (ice-9 match)
   #:use-module (ice-9 regex)
   #:use-module (srfi srfi-1)
@@ -340,9 +341,16 @@ passing it a directory name where it is mounted."
     "Return the first partition found with the boot flag set."
     (member 'boot (partition-flags partitions)))
 
+  (define (esp-partition? partitions)
+    "Return the first EFI System Partition."
+    (member 'esp (partition-flags partitions)))
+
   (let* ((partitions (initialize-partition-table device partitions))
          (root       (find partition-bootable? partitions))
-         (target     "/fs"))
+         (esp        (find esp-partition? partitions))
+         (target     "/fs")
+         ;; Grub expects EFI System Partitions to be mounted here.
+         (efi-directory (string-append target "/boot/efi")))
     (unless root
       (error "no bootable partition specified" partitions))
 
@@ -352,6 +360,33 @@ passing it a directory name where it is mounted."
     (mkdir-p target)
     (mount (partition-device root) target (partition-file-system root))
     (install-boot-config bootcfg bootcfg-location target)
+
+    ;; If we have an ESP partition, generate a self-contained grub EFI
+    ;; image and write it to a location known by most firmwares.
+    (when esp
+      (display "mounting EFI system partition...\n")
+      (mkdir-p efi-directory)
+      (mount (partition-device esp) efi-directory
+             (partition-file-system esp))
+      (mkdir-p (string-append efi-directory "/EFI/BOOT"))
+
+      ;; Grub needs a tmpdir to prepare the image.
+      (setenv "TMPDIR" (string-append target "/tmp"))
+      ;; It also requires a tiny configuration file telling the
+      ;; memdisk where to find the real thing. TODO: Delete it.
+      (with-output-to-file (string-append target "/tmp/grubdisk.cfg")
+        (lambda _
+          (format #t
+                  "insmod part_msdos~@
+                  search --set=root --label gnu-disk-image~@
+                  configfile /boot/grub/grub.cfg~%")))
+      (system* "grub-mkstandalone" "-O" "x86_64-efi" "-o"
+               (string-append efi-directory "/EFI/BOOT/BOOTX64.EFI")
+               ;; Embed the contents of grubdisk.cfg.
+               (string-append "boot/grub/grub.cfg=" target
+                              "/tmp/grubdisk.cfg"))
+      (umount efi-directory))
+
     (when bootloader-installer
       (bootloader-installer bootloader device target))
 
diff --git a/gnu/system/vm.scm b/gnu/system/vm.scm
index 099e3fac3..04ccc3650 100644
--- a/gnu/system/vm.scm
+++ b/gnu/system/vm.scm
@@ -3,6 +3,7 @@
 ;;; Copyright © 2016 Christopher Allan Webber <cwebber@dustycloud.org>
 ;;; Copyright © 2016 Leo Famulari <leo@famulari.name>
 ;;; Copyright © 2017 Mathieu Othacehe <m.othacehe@gmail.com>
+;;; Copyright © 2017 Marius Bakke <mbakke@fastmail.com>
 ;;;
 ;;; This file is part of GNU Guix.
 ;;;
@@ -202,7 +203,7 @@ the image."
                       (guix build utils))
 
          (let ((inputs
-                '#$(append (list qemu parted e2fsprogs dosfstools)
+                '#$(append (list qemu parted e2fsprogs dosfstools grub-efi)
                            (map canonical-package
                                 (list sed grep coreutils findutils gawk))
                            (if register-closures? (list guix) '())))
@@ -227,11 +228,18 @@ the image."
                                #:system-directory #$os.drv))
                   (partitions (list (partition
                                      (size #$(- disk-image-size
-                                                (* 10 (expt 2 20))))
+                                                (* 50 (expt 2 20))))
                                      (label #$file-system-label)
                                      (file-system #$file-system-type)
                                      (flags '(boot))
-                                     (initializer initialize)))))
+                                     (initializer initialize))
+                                    (partition
+                                     ;; Append a small FAT32 partition for
+                                     ;; use with UEFI bootloaders.
+                                     (size (* 40 (expt 2 20)))
+                                     (label "gnu-esp")
+                                     (file-system "vfat")
+                                     (flags '(esp))))))
              (initialize-hard-disk "/dev/vda"
                                    #:partitions partitions
                                    #:bootloader
-- 
2.12.2

bug#26815: [PATCH 3/3] vm: Support EFI boot in base image.

[Danny Milosavljevic]

Hi Marius,

> +      (system* "grub-mkstandalone" "-O" "x86_64-efi" "-o"
> +               (string-append efi-directory "/EFI/BOOT/BOOTX64.EFI")
> +               ;; Embed the contents of grubdisk.cfg.
> +               (string-append "boot/grub/grub.cfg=" target
> +                              "/tmp/grubdisk.cfg"))

Check return value of system* ?

>                    (partitions (list (partition
>                                       (size #$(- disk-image-size
> -                                                (* 10 (expt 2 20))))
> +                                                (* 50 (expt 2 20))))

<https://wiki.archlinux.org/index.php/EFI_System_Partition> says:

>According to a Microsoft note[2], the minimum size for the EFI System Partition (ESP) would be 100 MB, though this is not stated in the UEFI Specification. Note that for Advanced Format 4K Native drives (4-KB-per-sector) drives, the size is at least 256 MiB, because it is the minimum partition size of FAT32 drives (calculated as sector size (4KB) x 65527 = 256 MiB), due to a limitation of the FAT32 file format. 

Is this relevant for us?

Also, <https://gist.github.com/Apsu/4108795> says that F32 is not required for most UEFI firmwares. FAT12, FAT32 usually work.  But better be safe than sorry, I guess.

> +                                     (size (* 40 (expt 2 20)))
> +                                     (label "gnu-esp")
> +                                     (file-system "vfat")

Is it really vfat with long file names and everything? Or only FAT32 (with short file names)?

I ask because a lot of UEFT file names look as if they are for FAT32 with their short all-uppercase names.

bug#26815: [PATCH 3/3] vm: Support EFI boot in base image.

[Marius Bakke]

[-- Attachment #1: Type: text/plain, Size: 2278 bytes --]

Hi Danny, thanks for the quick feedback!

Danny Milosavljevic <dannym@scratchpost.org> writes:

>> +      (system* "grub-mkstandalone" "-O" "x86_64-efi" "-o"
>> +               (string-append efi-directory "/EFI/BOOT/BOOTX64.EFI")
>> +               ;; Embed the contents of grubdisk.cfg.
>> +               (string-append "boot/grub/grub.cfg=" target
>> +                              "/tmp/grubdisk.cfg"))
>
> Check return value of system* ?

Good point. Will wrap it in an (unless (zero? ...) (error ...)).

>>                    (partitions (list (partition
>>                                       (size #$(- disk-image-size
>> -                                                (* 10 (expt 2 20))))
>> +                                                (* 50 (expt 2 20))))
>
> <https://wiki.archlinux.org/index.php/EFI_System_Partition> says:
>
>>According to a Microsoft note[2], the minimum size for the EFI System Partition (ESP) would be 100 MB, though this is not stated in the UEFI Specification. Note that for Advanced Format 4K Native drives (4-KB-per-sector) drives, the size is at least 256 MiB, because it is the minimum partition size of FAT32 drives (calculated as sector size (4KB) x 65527 = 256 MiB), due to a limitation of the FAT32 file format. 

I have no idea what happens if you dd this image onto a 4k drive, only
tested with a 512B flash drive.

> Is this relevant for us?
>
> Also, <https://gist.github.com/Apsu/4108795> says that F32 is not required for most UEFI firmwares. FAT12, FAT32 usually work.  But better be safe than sorry, I guess.

If FAT32 may cause problems on some install media, perhaps it's better
to rely on the calculated allocation tables and "hope for the best". I
don't have a spare 4k drive to test with, though.

>> +                                     (size (* 40 (expt 2 20)))
>> +                                     (label "gnu-esp")
>> +                                     (file-system "vfat")
>
> Is it really vfat with long file names and everything? Or only FAT32 (with short file names)?

Good catch. I think it should be FAT32 indeed, according to parted from
a prepared disk image. AFAIK parted does not actually use this flag for
anything, but it's good to be accurate.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 487 bytes --]

bug#26815: [PATCH 3/3] vm: Support EFI boot in base image.

[Marius Bakke]

[-- Attachment #1.1: Type: text/plain, Size: 574 bytes --]

Danny Milosavljevic <dannym@scratchpost.org> writes:

> Hi Marius,
>
>> +      (system* "grub-mkstandalone" "-O" "x86_64-efi" "-o"
>> +               (string-append efi-directory "/EFI/BOOT/BOOTX64.EFI")
>> +               ;; Embed the contents of grubdisk.cfg.
>> +               (string-append "boot/grub/grub.cfg=" target
>> +                              "/tmp/grubdisk.cfg"))
>
> Check return value of system* ?

I rewrote most of this to be (hopefully) more understandable. Also, some
rudimentary steps towards other platforms, native testing required.

V2 attached.


[-- Attachment #1.2: 0001-vm-Support-EFI-boot-in-base-image.patch --]
[-- Type: text/x-patch, Size: 6833 bytes --]

From e94c9071c53db28c3e73998654b185c36a210df6 Mon Sep 17 00:00:00 2001
From: Marius Bakke <mbakke@fastmail.com>
Date: Sun, 7 May 2017 15:31:30 +0200
Subject: [PATCH] vm: Support EFI boot in base image.

* gnu/system/vm.scm (qemu-image): Add GRUB-EFI to inputs. Append 40MB
EFI System Partition.
* gnu/build/vm.scm (initialize-hard-disk): Generate grub EFI blob when ESP is
present.
---
 gnu/build/vm.scm  | 59 ++++++++++++++++++++++++++++++++++++++++++++++++++++++-
 gnu/system/vm.scm | 14 ++++++++++---
 2 files changed, 69 insertions(+), 4 deletions(-)

diff --git a/gnu/build/vm.scm b/gnu/build/vm.scm
index 206696ef6..8b4a44579 100644
--- a/gnu/build/vm.scm
+++ b/gnu/build/vm.scm
@@ -27,6 +27,7 @@
   #:use-module (gnu build linux-boot)
   #:use-module (gnu build install)
   #:use-module (guix records)
+  #:use-module (ice-9 format)
   #:use-module (ice-9 match)
   #:use-module (ice-9 regex)
   #:use-module (srfi srfi-1)
@@ -334,9 +335,16 @@ passing it a directory name where it is mounted."
     "Return the first partition found with the boot flag set."
     (member 'boot (partition-flags partitions)))
 
+  (define (partition-esp? partitions)
+    "Return the first EFI System Partition."
+    (member 'esp (partition-flags partitions)))
+
   (let* ((partitions (initialize-partition-table device partitions))
          (root       (find partition-bootable? partitions))
-         (target     "/fs"))
+         (esp        (find partition-esp? partitions))
+         (target     "/fs")
+         ;; Grub expects to find an EFI System Partition here.
+         (efi-directory (string-append target "/boot/efi")))
     (unless root
       (error "no bootable partition specified" partitions))
 
@@ -346,6 +354,55 @@ passing it a directory name where it is mounted."
     (mkdir-p target)
     (mount (partition-device root) target (partition-file-system root))
     (install-boot-config bootcfg bootcfg-location target)
+
+    ;; If we have an ESP partition, generate a self-contained grub EFI
+    ;; image and write it to a well-known location.
+    (when esp
+      (let* ((system %host-type)
+             (efi-payload-directory (string-append efi-directory "/EFI/BOOT"))
+             (efi-payload-file-name (if (any (cute string-prefix? <> system)
+                                             '("i686" "armhf"))
+                                        "BOOTX32.EFI"       ;hail mary
+                                        "BOOTX64.EFI"))     ;fairly supported
+             (efi-payload-format (cond
+                                  ((string-prefix? "x86_64" system)
+                                   "x86_64-efi")
+                                  ((string-prefix? "i686" system)
+                                   "i386-efi")
+                                  ((string-prefix? "armhf" system)
+                                   "arm-efi")
+                                  ((string-prefix? "aarch64" system)
+                                   "arm64-efi")))
+             (grub-tmp (string-append target "/tmp"))
+             (grub.cfg (string-append grub-tmp "/grub.cfg")))
+        (display "mounting EFI system partition...\n")
+        (mkdir-p efi-directory)
+        (mount (partition-device esp) efi-directory
+               (partition-file-system esp))
+        (mkdir-p efi-payload-directory)
+
+        ;; Grub needs a tmpdir to prepare the image.
+        (setenv "TMPDIR" grub-tmp)
+        ;; We also need a tiny configuration file telling the EFI blob where
+        ;; to find the real thing.
+        (with-output-to-file grub.cfg
+          (lambda _
+            (format #t
+                    "insmod part_msdos~@
+                    search --set=root --label gnu-disk-image~@
+                    configfile /boot/grub/grub.cfg~%")))
+        (display "creating grub firmware image...\n")
+        (unless (zero? (system* "grub-mkstandalone" "-O" efi-payload-format
+                                "-o" (string-append efi-payload-directory "/"
+                                                    efi-payload-file-name)
+                                ;; Graft the contents of our configuration file
+                                ;; into the image.  See grub-mkstandalone(1).
+                                (string-append "boot/grub/grub.cfg=" grub.cfg)))
+          (error "failed to create grub EFI image"))
+
+        (delete-file grub.cfg)
+        (umount efi-directory)))
+
     (when bootloader-installer
       (bootloader-installer bootloader device target))
 
diff --git a/gnu/system/vm.scm b/gnu/system/vm.scm
index 099e3fac3..04ccc3650 100644
--- a/gnu/system/vm.scm
+++ b/gnu/system/vm.scm
@@ -3,6 +3,7 @@
 ;;; Copyright © 2016 Christopher Allan Webber <cwebber@dustycloud.org>
 ;;; Copyright © 2016 Leo Famulari <leo@famulari.name>
 ;;; Copyright © 2017 Mathieu Othacehe <m.othacehe@gmail.com>
+;;; Copyright © 2017 Marius Bakke <mbakke@fastmail.com>
 ;;;
 ;;; This file is part of GNU Guix.
 ;;;
@@ -202,7 +203,7 @@ the image."
                       (guix build utils))
 
          (let ((inputs
-                '#$(append (list qemu parted e2fsprogs dosfstools)
+                '#$(append (list qemu parted e2fsprogs dosfstools grub-efi)
                            (map canonical-package
                                 (list sed grep coreutils findutils gawk))
                            (if register-closures? (list guix) '())))
@@ -227,11 +228,18 @@ the image."
                                #:system-directory #$os.drv))
                   (partitions (list (partition
                                      (size #$(- disk-image-size
-                                                (* 10 (expt 2 20))))
+                                                (* 50 (expt 2 20))))
                                      (label #$file-system-label)
                                      (file-system #$file-system-type)
                                      (flags '(boot))
-                                     (initializer initialize)))))
+                                     (initializer initialize))
+                                    (partition
+                                     ;; Append a small FAT32 partition for
+                                     ;; use with UEFI bootloaders.
+                                     (size (* 40 (expt 2 20)))
+                                     (label "gnu-esp")
+                                     (file-system "vfat")
+                                     (flags '(esp))))))
              (initialize-hard-disk "/dev/vda"
                                    #:partitions partitions
                                    #:bootloader
-- 
2.12.2


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 487 bytes --]

bug#26815: [PATCH 3/3] vm: Support EFI boot in base image.

[Marius Bakke]

[-- Attachment #1.1: Type: text/plain, Size: 770 bytes --]

Marius Bakke <mbakke@fastmail.com> writes:

> Danny Milosavljevic <dannym@scratchpost.org> writes:
>
>> Hi Marius,
>>
>>> +      (system* "grub-mkstandalone" "-O" "x86_64-efi" "-o"
>>> +               (string-append efi-directory "/EFI/BOOT/BOOTX64.EFI")
>>> +               ;; Embed the contents of grubdisk.cfg.
>>> +               (string-append "boot/grub/grub.cfg=" target
>>> +                              "/tmp/grubdisk.cfg"))
>>
>> Check return value of system* ?
>
> I rewrote most of this to be (hopefully) more understandable. Also, some
> rudimentary steps towards other platforms, native testing required.
>
> V2 attached.

...and here is V3, now with compliant file names for all supported
arches from the UEFI specification. Will test i386 later today.


[-- Attachment #1.2: 0001-vm-Support-EFI-boot-in-base-image.patch --]
[-- Type: text/x-patch, Size: 6823 bytes --]

From 9555239cfc9362a15cc3f255040c410395d49e04 Mon Sep 17 00:00:00 2001
From: Marius Bakke <mbakke@fastmail.com>
Date: Sun, 7 May 2017 15:31:30 +0200
Subject: [PATCH] vm: Support EFI boot in base image.

* gnu/system/vm.scm (qemu-image): Add GRUB-EFI to inputs. Append 40MB
EFI System Partition.
* gnu/build/vm.scm (initialize-hard-disk): Generate grub EFI blob when ESP is
present.
---
 gnu/build/vm.scm  | 58 ++++++++++++++++++++++++++++++++++++++++++++++++++++++-
 gnu/system/vm.scm | 14 +++++++++++---
 2 files changed, 68 insertions(+), 4 deletions(-)

diff --git a/gnu/build/vm.scm b/gnu/build/vm.scm
index 70e66fb35..57056fced 100644
--- a/gnu/build/vm.scm
+++ b/gnu/build/vm.scm
@@ -27,6 +27,7 @@
   #:use-module (gnu build linux-boot)
   #:use-module (gnu build install)
   #:use-module (guix records)
+  #:use-module (ice-9 format)
   #:use-module (ice-9 match)
   #:use-module (ice-9 regex)
   #:use-module (srfi srfi-1)
@@ -334,9 +335,16 @@ passing it a directory name where it is mounted."
     "Return the first partition found with the boot flag set."
     (member 'boot (partition-flags partitions)))
 
+  (define (partition-esp? partitions)
+    "Return the first EFI System Partition."
+    (member 'esp (partition-flags partitions)))
+
   (let* ((partitions (initialize-partition-table device partitions))
          (root       (find partition-bootable? partitions))
-         (target     "/fs"))
+         (esp        (find partition-esp? partitions))
+         (target     "/fs")
+         ;; Grub expects to find an EFI System Partition here.
+         (efi-directory (string-append target "/boot/efi")))
     (unless root
       (error "no bootable partition specified" partitions))
 
@@ -346,6 +354,54 @@ passing it a directory name where it is mounted."
     (mkdir-p target)
     (mount (partition-device root) target (partition-file-system root))
     (install-boot-config bootcfg bootcfg-location target)
+
+    ;; If we have an ESP partition, generate a self-contained grub EFI
+    ;; image and write it to a well-known location.
+    (when esp
+      (let* ((system %host-type)
+             (efi-payload-directory (string-append efi-directory "/EFI/BOOT"))
+             ;; Map the grub targets to the boot file names expected by
+             ;; UEFI compliant firmware. See "Removable Media Boot Behavior":
+             ;; http://www.uefi.org/sites/default/files/resources/UEFI%20Spec%202_6.pdf
+             (efi-target-map (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-tmp (string-append target "/tmp"))
+             (grub.cfg (string-append grub-tmp "/grub.cfg")))
+        (display "mounting EFI system partition...\n")
+        (mkdir-p efi-directory)
+        (mount (partition-device esp) efi-directory
+               (partition-file-system esp))
+        (mkdir-p efi-payload-directory)
+
+        ;; Grub needs a tmpdir to prepare the image.
+        (setenv "TMPDIR" grub-tmp)
+        ;; We also need a tiny configuration file telling the EFI blob where
+        ;; to find the real thing.
+        (with-output-to-file grub.cfg
+          (lambda _
+            (format #t
+                    "insmod part_msdos~@
+                    search --set=root --label gnu-disk-image~@
+                    configfile /boot/grub/grub.cfg~%")))
+        (display "creating grub firmware image...\n")
+        (unless (zero? (system* "grub-mkstandalone" "-O" (car efi-target-map)
+                                "-o" (string-append efi-payload-directory "/"
+                                                    (cdr efi-target-map))
+                                ;; Graft the contents of our configuration file
+                                ;; into the image.  See grub-mkstandalone(1).
+                                (string-append "boot/grub/grub.cfg=" grub.cfg)))
+          (error "failed to create grub EFI image"))
+
+        (delete-file grub.cfg)
+        (umount efi-directory)))
+
     (when bootloader-installer
       (bootloader-installer bootloader device target))
 
diff --git a/gnu/system/vm.scm b/gnu/system/vm.scm
index 099e3fac3..04ccc3650 100644
--- a/gnu/system/vm.scm
+++ b/gnu/system/vm.scm
@@ -3,6 +3,7 @@
 ;;; Copyright © 2016 Christopher Allan Webber <cwebber@dustycloud.org>
 ;;; Copyright © 2016 Leo Famulari <leo@famulari.name>
 ;;; Copyright © 2017 Mathieu Othacehe <m.othacehe@gmail.com>
+;;; Copyright © 2017 Marius Bakke <mbakke@fastmail.com>
 ;;;
 ;;; This file is part of GNU Guix.
 ;;;
@@ -202,7 +203,7 @@ the image."
                       (guix build utils))
 
          (let ((inputs
-                '#$(append (list qemu parted e2fsprogs dosfstools)
+                '#$(append (list qemu parted e2fsprogs dosfstools grub-efi)
                            (map canonical-package
                                 (list sed grep coreutils findutils gawk))
                            (if register-closures? (list guix) '())))
@@ -227,11 +228,18 @@ the image."
                                #:system-directory #$os.drv))
                   (partitions (list (partition
                                      (size #$(- disk-image-size
-                                                (* 10 (expt 2 20))))
+                                                (* 50 (expt 2 20))))
                                      (label #$file-system-label)
                                      (file-system #$file-system-type)
                                      (flags '(boot))
-                                     (initializer initialize)))))
+                                     (initializer initialize))
+                                    (partition
+                                     ;; Append a small FAT32 partition for
+                                     ;; use with UEFI bootloaders.
+                                     (size (* 40 (expt 2 20)))
+                                     (label "gnu-esp")
+                                     (file-system "vfat")
+                                     (flags '(esp))))))
              (initialize-hard-disk "/dev/vda"
                                    #:partitions partitions
                                    #:bootloader
-- 
2.12.2


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 487 bytes --]

bug#26815: [PATCH 3/3] vm: Support EFI boot in base image.

[Ludovic Courtès]

Marius Bakke <mbakke@fastmail.com> skribis:

> From 9555239cfc9362a15cc3f255040c410395d49e04 Mon Sep 17 00:00:00 2001
> From: Marius Bakke <mbakke@fastmail.com>
> Date: Sun, 7 May 2017 15:31:30 +0200
> Subject: [PATCH] vm: Support EFI boot in base image.
>
> * gnu/system/vm.scm (qemu-image): Add GRUB-EFI to inputs. Append 40MB
> EFI System Partition.
> * gnu/build/vm.scm (initialize-hard-disk): Generate grub EFI blob when ESP is
> present.

[...]

> +    ;; If we have an ESP partition, generate a self-contained grub EFI
> +    ;; image and write it to a well-known location.
> +    (when esp
> +      (let* ((system %host-type)
> +             (efi-payload-directory (string-append efi-directory "/EFI/BOOT"))
> +             ;; Map the grub targets to the boot file names expected by
> +             ;; UEFI compliant firmware. See "Removable Media Boot Behavior":
> +             ;; http://www.uefi.org/sites/default/files/resources/UEFI%20Spec%202_6.pdf
> +             (efi-target-map (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-tmp (string-append target "/tmp"))
> +             (grub.cfg (string-append grub-tmp "/grub.cfg")))
> +        (display "mounting EFI system partition...\n")
> +        (mkdir-p efi-directory)
> +        (mount (partition-device esp) efi-directory
> +               (partition-file-system esp))
> +        (mkdir-p efi-payload-directory)
> +
> +        ;; Grub needs a tmpdir to prepare the image.
> +        (setenv "TMPDIR" grub-tmp)
> +        ;; We also need a tiny configuration file telling the EFI blob where
> +        ;; to find the real thing.
> +        (with-output-to-file grub.cfg
> +          (lambda _
> +            (format #t
> +                    "insmod part_msdos~@
> +                    search --set=root --label gnu-disk-image~@
> +                    configfile /boot/grub/grub.cfg~%")))
> +        (display "creating grub firmware image...\n")
> +        (unless (zero? (system* "grub-mkstandalone" "-O" (car efi-target-map)
> +                                "-o" (string-append efi-payload-directory "/"
> +                                                    (cdr efi-target-map))
> +                                ;; Graft the contents of our configuration file
> +                                ;; into the image.  See grub-mkstandalone(1).
> +                                (string-append "boot/grub/grub.cfg=" grub.cfg)))
> +          (error "failed to create grub EFI image"))
> +
> +        (delete-file grub.cfg)
> +        (umount efi-directory)))

Could you move the body hi of ‘when’ to a separate procedure, say
‘install-efi’, such that this reduces to something like:

  (when esp
    (install-efi esp grub.cfg))

> +                                    (partition
> +                                     ;; Append a small FAT32 partition for
> +                                     ;; use with UEFI bootloaders.
> +                                     (size (* 40 (expt 2 20)))
> +                                     (label "gnu-esp")
> +                                     (file-system "vfat")
> +                                     (flags '(esp))))))
>               (initialize-hard-disk "/dev/vda"
>                                     #:partitions partitions
>                                     #:bootloader

All the images we create will now have that extra ESP, but maybe that’s
OK.

Is the “gnu-esp” label of this partition used for lookup anywhere?  If
it was, we’d run into problems as soon as we have several partitions
with this hard-coded label (say you have your installed GuixSD as well
as the installation image on a USB key that’s plugged in.)  If the label
is not used for lookup, that’s OK.

Apart from that LGTM, thank you!

Ludo’.

bug#26815: [PATCH 1/3] vm: Support arbitrary partition flags.

[Marius Bakke]

* gnu/build/vm.scm (<partition>): Change BOOTABLE? to FLAGS.
(initialize-partition-table): Pass each flag to parted.
(initialize-hard-disk): Locate boot partition.
* gnu/system/vm.scm (qemu-image): Adjust partition flags.
---
 gnu/build/vm.scm  | 17 ++++++++++++-----
 gnu/system/vm.scm |  2 +-
 2 files changed, 13 insertions(+), 6 deletions(-)

diff --git a/gnu/build/vm.scm b/gnu/build/vm.scm
index 440596a40..588b2d58b 100644
--- a/gnu/build/vm.scm
+++ b/gnu/build/vm.scm
@@ -3,6 +3,7 @@
 ;;; Copyright © 2016 Christopher Allan Webber <cwebber@dustycloud.org>
 ;;; Copyright © 2016 Leo Famulari <leo@famulari.name>
 ;;; Copyright © 2017 Mathieu Othacehe <m.othacehe@gmail.com>
+;;; Copyright © 2017 Marius Bakke <mbakke@fastmail.com>
 ;;;
 ;;; This file is part of GNU Guix.
 ;;;
@@ -41,7 +42,7 @@
             partition-size
             partition-file-system
             partition-label
-            partition-bootable?
+            partition-flags
             partition-initializer
 
             root-partition-initializer
@@ -141,7 +142,7 @@ the #:references-graphs parameter of 'derivation'."
   (size        partition-size)
   (file-system partition-file-system (default "ext4"))
   (label       partition-label (default #f))
-  (bootable?   partition-bootable? (default #f))
+  (flags       partition-flags (default '()))
   (initializer partition-initializer (default (const #t))))
 
 (define (fold2 proc seed1 seed2 lst)              ;TODO: factorize
@@ -168,9 +169,10 @@ actual /dev name based on DEVICE."
     (cons* "mkpart" "primary" "ext2"
            (format #f "~aB" offset)
            (format #f "~aB" (+ offset (partition-size part)))
-           (if (partition-bootable? part)
-               `("set" ,(number->string index) "boot" "on")
-               '())))
+           (append-map (lambda (flag)
+                         (list "set" (number->string index)
+                               (symbol->string flag) "on"))
+                       (partition-flags part))))
 
   (define (options partitions offset)
     (let loop ((partitions partitions)
@@ -303,6 +305,11 @@ in PARTITIONS, and using BOOTCFG as its bootloader configuration file.
 
 Each partition is initialized by calling its 'initializer' procedure,
 passing it a directory name where it is mounted."
+
+  (define (partition-bootable? partition)
+    "Return the first partition found with the boot flag set."
+    (member 'boot (partition-flags partition)))
+
   (let* ((partitions (initialize-partition-table device partitions))
          (root       (find partition-bootable? partitions))
          (target     "/fs"))
diff --git a/gnu/system/vm.scm b/gnu/system/vm.scm
index cc13abe2a..42c7690b1 100644
--- a/gnu/system/vm.scm
+++ b/gnu/system/vm.scm
@@ -230,7 +230,7 @@ the image."
                                                 (* 10 (expt 2 20))))
                                      (label #$file-system-label)
                                      (file-system #$file-system-type)
-                                     (bootable? #t)
+                                     (flags '(boot))
                                      (initializer initialize)))))
              (initialize-hard-disk "/dev/vda"
                                    #:partitions partitions
-- 
2.12.2

bug#26815: [PATCH 2/3] vm: Support creating FAT partitions.

[Marius Bakke]

* gnu/build/vm.scm (create-ext-file-system, create-fat-file-system): New procedures.
(format-partition): Use them. Error for unknown file systems.
* gnu/system/vm.scm (qemu-image): Include DOSFSTOOLS.
* gnu/system/linux-initrd.scm (base-initrd): Always add nls_is8859-1.ko.
---
 gnu/build/vm.scm            | 30 ++++++++++++++++++++++++++----
 gnu/system/linux-initrd.scm |  4 +---
 gnu/system/vm.scm           |  2 +-
 3 files changed, 28 insertions(+), 8 deletions(-)

diff --git a/gnu/build/vm.scm b/gnu/build/vm.scm
index 588b2d58b..935a174fc 100644
--- a/gnu/build/vm.scm
+++ b/gnu/build/vm.scm
@@ -213,10 +213,10 @@ actual /dev name based on DEVICE."
 
 (define MS_BIND 4096)                             ; <sys/mounts.h> again!
 
-(define* (format-partition partition type
-                           #:key label)
-  "Create a file system TYPE on PARTITION.  If LABEL is true, use that as the
-volume name."
+(define* (create-ext-file-system partition type
+                                 #:key label)
+  "Create an ext-family filesystem of TYPE on PARTITION.  If LABEL is true,
+use that as the volume name."
   (format #t "creating ~a partition...\n" type)
   (unless (zero? (apply system* (string-append "mkfs." type)
                         "-F" partition
@@ -225,6 +225,28 @@ volume name."
                             '())))
     (error "failed to create partition")))
 
+(define* (create-fat-file-system partition
+                                 #:key label)
+  "Create a FAT filesystem on PARTITION.  The number of File Allocation Tables
+will be determined based on filesystem size.  If LABEL is true, use that as the
+volume name."
+  (format #t "creating FAT partition...\n")
+  (unless (zero? (apply system* "mkfs.fat" partition
+                        (if label
+                            `("-n" ,label)
+                            '())))
+    (error "failed to create FAT partition")))
+
+(define* (format-partition partition type
+                           #:key label)
+  "Create a file system TYPE on PARTITION.  If LABEL is true, use that as the
+volume name."
+  (cond ((string-prefix? "ext" type)
+         (create-ext-file-system partition type #:label label))
+        ((or (string-prefix? "fat" type) (string= "vfat" type))
+         (create-fat-file-system partition #:label label))
+        (else (error "Unsupported file system."))))
+
 (define (initialize-partition partition)
   "Format PARTITION, a <partition> object with a non-#f 'device' field, mount
 it, run its initializer, and unmount it."
diff --git a/gnu/system/linux-initrd.scm b/gnu/system/linux-initrd.scm
index dfe198e43..3a5e76034 100644
--- a/gnu/system/linux-initrd.scm
+++ b/gnu/system/linux-initrd.scm
@@ -268,6 +268,7 @@ loaded at boot time in the order in which they appear."
       "usbhid" "hid-generic" "hid-apple"      ;keyboards during early boot
       "dm-crypt" "xts" "serpent_generic" "wp512" ;for encrypted root partitions
       "nvme"                                     ;for new SSD NVMe devices
+      "nls_iso8859-1"                            ;for `mkfs.fat`, et.al
       ,@(if (string-match "^(x86_64|i[3-6]86)-" (%current-system))
             '("pata_acpi" "pata_atiixp"    ;for ATA controllers
               "isci")                      ;for SAS controllers like Intel C602
@@ -281,9 +282,6 @@ loaded at boot time in the order in which they appear."
       ,@(if (find (file-system-type-predicate "9p") file-systems)
             virtio-9p-modules
             '())
-      ,@(if (find (file-system-type-predicate "vfat") file-systems)
-            '("nls_iso8859-1")
-            '())
       ,@(if (find (file-system-type-predicate "btrfs") file-systems)
             '("btrfs")
             '())
diff --git a/gnu/system/vm.scm b/gnu/system/vm.scm
index 42c7690b1..099e3fac3 100644
--- a/gnu/system/vm.scm
+++ b/gnu/system/vm.scm
@@ -202,7 +202,7 @@ the image."
                       (guix build utils))
 
          (let ((inputs
-                '#$(append (list qemu parted e2fsprogs)
+                '#$(append (list qemu parted e2fsprogs dosfstools)
                            (map canonical-package
                                 (list sed grep coreutils findutils gawk))
                            (if register-closures? (list guix) '())))
-- 
2.12.2

bug#26815: [PATCH 3/3] vm: Add UEFI loader to disk images.

[Marius Bakke]

* gnu/system/vm.scm (qemu-image): Add GRUB-EFI to inputs. Append 40MiB EFI
System Partition.
* gnu/build/vm.scm (install-efi): New procedure.
(initialize-hard-disk): Generate grub EFI blob when ESP is present.
---
 gnu/build/vm.scm  | 60 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
 gnu/system/vm.scm | 19 +++++++++++++++---
 2 files changed, 76 insertions(+), 3 deletions(-)

diff --git a/gnu/build/vm.scm b/gnu/build/vm.scm
index 935a174fc..258c0b8ae 100644
--- a/gnu/build/vm.scm
+++ b/gnu/build/vm.scm
@@ -27,6 +27,7 @@
   #:use-module (gnu build linux-boot)
   #:use-module (gnu build install)
   #:use-module (guix records)
+  #:use-module (ice-9 format)
   #:use-module (ice-9 match)
   #:use-module (ice-9 regex)
   #:use-module (srfi srfi-1)
@@ -315,6 +316,34 @@ SYSTEM-DIRECTORY is the name of the directory of the 'system' derivation."
     (mkdir-p directory)
     (symlink bootcfg (string-append directory "/bootcfg"))))
 
+(define (install-efi esp grub.cfg)
+  "Write a self-contained Grub UEFI blob to the mounted ESP using GRUB.CFG."
+  (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
+         (efi-directory (string-append esp "/EFI/BOOT"))
+         ;; Map grub target names to boot file names.
+         (efi-target-map (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 needs a TMPDIR to prepare the firmware image.
+    (setenv "TMPDIR" esp)
+
+    (mkdir-p efi-directory)
+    (unless (zero? (system* "grub-mkstandalone" "-O" (car efi-target-map)
+                            "-o" (string-append efi-directory "/"
+                                                (cdr efi-target-map))
+                            ;; Graft the configuration file onto the image.
+                            (string-append "boot/grub/grub.cfg=" grub.cfg)))
+      (error "failed to create grub EFI image"))))
+
 (define* (initialize-hard-disk device
                                #:key
                                bootloader
@@ -332,8 +361,13 @@ passing it a directory name where it is mounted."
     "Return the first partition found with the boot flag set."
     (member 'boot (partition-flags partition)))
 
+  (define (partition-esp? partition)
+    "Return the first EFI System Partition."
+    (member 'esp (partition-flags partition)))
+
   (let* ((partitions (initialize-partition-table device partitions))
          (root       (find partition-bootable? partitions))
+         (esp        (find partition-esp? partitions))
          (target     "/fs"))
     (unless root
       (error "no bootable partition specified" partitions))
@@ -345,8 +379,34 @@ passing it a directory name where it is mounted."
     (mount (partition-device root) target (partition-file-system root))
     (install-boot-config bootcfg bootcfg-location target)
     (when bootloader-installer
+      (display "installing bootloader...\n")
       (bootloader-installer bootloader device target))
 
+    (when esp
+      ;; Mount the ESP somewhere and install Grub UEFI image.
+      (let ((mount-point (string-append target "/boot/efi"))
+            (grub.cfg    (string-append target "/tmp/grub-standalone.cfg")))
+        (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.
+        (with-output-to-file grub.cfg
+          (lambda _
+            (format #t
+                    "insmod part_msdos~@
+                    search --set=root --label gnu-disk-image~@
+                    configfile /boot/grub/grub.cfg~%")))
+
+        (display "creating EFI firmware image...")
+        (install-efi mount-point grub.cfg)
+        (display "done.\n")
+
+        (delete-file grub.cfg)
+        (umount mount-point)))
+
     ;; Register BOOTCFG as a GC root.
     (register-bootcfg-root target bootcfg)
 
diff --git a/gnu/system/vm.scm b/gnu/system/vm.scm
index 099e3fac3..fbabcf6c3 100644
--- a/gnu/system/vm.scm
+++ b/gnu/system/vm.scm
@@ -3,6 +3,7 @@
 ;;; Copyright © 2016 Christopher Allan Webber <cwebber@dustycloud.org>
 ;;; Copyright © 2016 Leo Famulari <leo@famulari.name>
 ;;; Copyright © 2017 Mathieu Othacehe <m.othacehe@gmail.com>
+;;; Copyright © 2017 Marius Bakke <mbakke@fastmail.com>
 ;;;
 ;;; This file is part of GNU Guix.
 ;;;
@@ -202,7 +203,7 @@ the image."
                       (guix build utils))
 
          (let ((inputs
-                '#$(append (list qemu parted e2fsprogs dosfstools)
+                '#$(append (list qemu parted e2fsprogs dosfstools grub-efi)
                            (map canonical-package
                                 (list sed grep coreutils findutils gawk))
                            (if register-closures? (list guix) '())))
@@ -227,11 +228,23 @@ the image."
                                #:system-directory #$os.drv))
                   (partitions (list (partition
                                      (size #$(- disk-image-size
-                                                (* 10 (expt 2 20))))
+                                                (* 50 (expt 2 20))))
                                      (label #$file-system-label)
                                      (file-system #$file-system-type)
                                      (flags '(boot))
-                                     (initializer initialize)))))
+                                     (initializer initialize))
+                                    ;; Append a small EFI System Partition for
+                                    ;; use with UEFI bootloaders.
+                                    (partition
+                                     ;; The standalone grub image is about 10MiB, but
+                                     ;; leave some room for custom or multiple images.
+                                     (size (* 40 (expt 2 20)))
+                                     (label "GNU-ESP")             ;cosmetic only
+                                     ;; Use "vfat" here since this property is used
+                                     ;; when mounting. The actual FAT-ness is based
+                                     ;; on filesystem size (16 in this case).
+                                     (file-system "vfat")
+                                     (flags '(esp))))))
              (initialize-hard-disk "/dev/vda"
                                    #:partitions partitions
                                    #:bootloader
-- 
2.12.2

bug#26815: [PATCH 3/3] vm: Add UEFI loader to disk images.

[Ludovic Courtès]

Hi Marius,

Marius Bakke <mbakke@fastmail.com> skribis:

> * gnu/system/vm.scm (qemu-image): Add GRUB-EFI to inputs. Append 40MiB EFI
> System Partition.
> * gnu/build/vm.scm (install-efi): New procedure.
> (initialize-hard-disk): Generate grub EFI blob when ESP is present.

[...]

> +(define (install-efi esp grub.cfg)
> +  "Write a self-contained Grub UEFI blob to the mounted ESP using GRUB.CFG."

I’m really nitpicking here, but the spelling is “GRUB”.  :-)

> +        (with-output-to-file grub.cfg
> +          (lambda _
> +            (format #t
> +                    "insmod part_msdos~@

Nitpicking too, but I have a preference for the more explicit:

  (call-with-output-file grub.cfg
    (lambda (port)
      (format port …)))

Otherwise the whole series LGTM.

Do I get it right that ‘disk-image’ will now give us UEFI-enabled
images?  (Should we mention it in guix.texi?)

Thanks a lot!

Ludo’.

bug#26815: [PATCH 3/3] vm: Add UEFI loader to disk images.

[Marius Bakke]

[-- Attachment #1: Type: text/plain, Size: 1170 bytes --]

Ludovic Courtès <ludo@gnu.org> writes:

> Hi Marius,
>
> Marius Bakke <mbakke@fastmail.com> skribis:
>
>> * gnu/system/vm.scm (qemu-image): Add GRUB-EFI to inputs. Append 40MiB EFI
>> System Partition.
>> * gnu/build/vm.scm (install-efi): New procedure.
>> (initialize-hard-disk): Generate grub EFI blob when ESP is present.
>
> [...]
>
>> +(define (install-efi esp grub.cfg)
>> +  "Write a self-contained Grub UEFI blob to the mounted ESP using GRUB.CFG."
>
> I’m really nitpicking here, but the spelling is “GRUB”.  :-)

Aah, forgot it was an acronym. Will fix!

>> +        (with-output-to-file grub.cfg
>> +          (lambda _
>> +            (format #t
>> +                    "insmod part_msdos~@
>
> Nitpicking too, but I have a preference for the more explicit:
>
>   (call-with-output-file grub.cfg
>     (lambda (port)
>       (format port …)))

Duly noted.

> Do I get it right that ‘disk-image’ will now give us UEFI-enabled
> images?  (Should we mention it in guix.texi?)

Indeed! It should even work on EFI-enabled ARM hardware.

It probably deserves a mention in the manual somewhere. Will have a look.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 487 bytes --]

bug#26815: [PATCH 3/3] vm: Support EFI boot in base image.

[Marius Bakke]

[-- Attachment #1: Type: text/plain, Size: 5167 bytes --]

Ludovic Courtès <ludo@gnu.org> writes:

> Marius Bakke <mbakke@fastmail.com> skribis:
>
>> From 9555239cfc9362a15cc3f255040c410395d49e04 Mon Sep 17 00:00:00 2001
>> From: Marius Bakke <mbakke@fastmail.com>
>> Date: Sun, 7 May 2017 15:31:30 +0200
>> Subject: [PATCH] vm: Support EFI boot in base image.
>>
>> * gnu/system/vm.scm (qemu-image): Add GRUB-EFI to inputs. Append 40MB
>> EFI System Partition.
>> * gnu/build/vm.scm (initialize-hard-disk): Generate grub EFI blob when ESP is
>> present.
>
> [...]
>
>> +    ;; If we have an ESP partition, generate a self-contained grub EFI
>> +    ;; image and write it to a well-known location.
>> +    (when esp
>> +      (let* ((system %host-type)
>> +             (efi-payload-directory (string-append efi-directory "/EFI/BOOT"))
>> +             ;; Map the grub targets to the boot file names expected by
>> +             ;; UEFI compliant firmware. See "Removable Media Boot Behavior":
>> +             ;; http://www.uefi.org/sites/default/files/resources/UEFI%20Spec%202_6.pdf
>> +             (efi-target-map (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-tmp (string-append target "/tmp"))
>> +             (grub.cfg (string-append grub-tmp "/grub.cfg")))
>> +        (display "mounting EFI system partition...\n")
>> +        (mkdir-p efi-directory)
>> +        (mount (partition-device esp) efi-directory
>> +               (partition-file-system esp))
>> +        (mkdir-p efi-payload-directory)
>> +
>> +        ;; Grub needs a tmpdir to prepare the image.
>> +        (setenv "TMPDIR" grub-tmp)
>> +        ;; We also need a tiny configuration file telling the EFI blob where
>> +        ;; to find the real thing.
>> +        (with-output-to-file grub.cfg
>> +          (lambda _
>> +            (format #t
>> +                    "insmod part_msdos~@
>> +                    search --set=root --label gnu-disk-image~@
>> +                    configfile /boot/grub/grub.cfg~%")))
>> +        (display "creating grub firmware image...\n")
>> +        (unless (zero? (system* "grub-mkstandalone" "-O" (car efi-target-map)
>> +                                "-o" (string-append efi-payload-directory "/"
>> +                                                    (cdr efi-target-map))
>> +                                ;; Graft the contents of our configuration file
>> +                                ;; into the image.  See grub-mkstandalone(1).
>> +                                (string-append "boot/grub/grub.cfg=" grub.cfg)))
>> +          (error "failed to create grub EFI image"))
>> +
>> +        (delete-file grub.cfg)
>> +        (umount efi-directory)))
>
> Could you move the body hi of ‘when’ to a separate procedure, say
> ‘install-efi’, such that this reduces to something like:
>
>   (when esp
>     (install-efi esp grub.cfg))

Good idea. I've moved the grub parts into a separate procedure, but kept
the mounting etc here.

>> +                                    (partition
>> +                                     ;; Append a small FAT32 partition for
>> +                                     ;; use with UEFI bootloaders.
>> +                                     (size (* 40 (expt 2 20)))
>> +                                     (label "gnu-esp")
>> +                                     (file-system "vfat")
>> +                                     (flags '(esp))))))
>>               (initialize-hard-disk "/dev/vda"
>>                                     #:partitions partitions
>>                                     #:bootloader
>
> All the images we create will now have that extra ESP, but maybe that’s
> OK.

It's "only" 40MiB, so didn't see a reason to complicate it. Grub is
actually just ~10MiB, but left some space for..stuff?

There are some rare systems with 32-bit UEFI and 64-bit CPU, those users
should be able to copy "BOOTIA32.EFI" from an i686 image onto the ESP.

> Is the “gnu-esp” label of this partition used for lookup anywhere?  If
> it was, we’d run into problems as soon as we have several partitions
> with this hard-coded label (say you have your installed GuixSD as well
> as the installation image on a USB key that’s plugged in.)  If the label
> is not used for lookup, that’s OK.

It's only used for informational purposes. Now even uppercase to display
properly on ancient systems :-P

I've sent a new patch series taking the reviews into account. The
patches from id:20170506154154.17836-1-m.othacehe@gmail.com are still
required.

(Also, --size=1G is no longer enough after the Guile 2.2 transition!)

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 487 bytes --]

bug#26815: [PATCH 3/3] vm: Support EFI boot in base image.

[Ludovic Courtès]

Hey Marius!

(+ Cc: Mathieu O.)

Marius Bakke <mbakke@fastmail.com> skribis:

> I've sent a new patch series taking the reviews into account. The
> patches from id:20170506154154.17836-1-m.othacehe@gmail.com are still
> required.

Gnus doesn’t find this message ID.  Which patch is it?  It seems that
I’m preventing progress but I’d rather not!

> (Also, --size=1G is no longer enough after the Guile 2.2 transition!)

Heheh.  :-)

Ludo’.

bug#26815: [PATCH 3/3] vm: Support EFI boot in base image.

[Marius Bakke]

[-- Attachment #1: Type: text/plain, Size: 708 bytes --]

Ludovic Courtès <ludo@gnu.org> writes:

> Hey Marius!
>
> (+ Cc: Mathieu O.)
>
> Marius Bakke <mbakke@fastmail.com> skribis:
>
>> I've sent a new patch series taking the reviews into account. The
>> patches from id:20170506154154.17836-1-m.othacehe@gmail.com are still
>> required.
>
> Gnus doesn’t find this message ID.  Which patch is it?  It seems that
> I’m preventing progress but I’d rather not!

Clicking that ID brings me to..

https://debbugs.gnu.org/cgi/bugreport.cgi?bug=26339#236

I think v4 is being prepared, I'll rebase this series once that is out.

Note: for emergency testing you can also fetch all commits here:

https://github.com/mbakke/guix/tree/wip-uefi

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 487 bytes --]

bug#26815: [PATCH 3/3] vm: Support EFI boot in base image.

[Mathieu Othacehe]

Hi all !

> I think v4 is being prepared, I'll rebase this series once that is out.

Yes I'm fighting with btrfs-root-os test failling for my serie. I have
the following error that I'm investingating :

--8<---------------cut here---------------start------------->8---
+ btrfs subvolume create /mnt/home
Create subvolume '/mnt/home'
+ herd start cow-store /mnt
herd: exception caught while executing 'start' on service 'cow-store':
ERROR: In procedure mount: mount "/.rw-store" on "/gnu/store": Invalid argument
environment variable `PATH' set to `/gnu/store/73p6bldnhm9lzcrhy8g96dzxawncd852-qemu-minimal-2.9.0/bin'
--8<---------------cut here---------------end--------------->8---

Mathieu

bug#26815: [PATCH 3/3] vm: Support EFI boot in base image.

[Ludovic Courtès]

Hello!

Mathieu Othacehe <m.othacehe@gmail.com> skribis:

>> I think v4 is being prepared, I'll rebase this series once that is out.

Thanks Marius for bearing with me.  :-)

Are there specific bits that are needed here?

> Yes I'm fighting with btrfs-root-os test failling for my serie. I have
> the following error that I'm investingating :
>
> + btrfs subvolume create /mnt/home
> Create subvolume '/mnt/home'
> + herd start cow-store /mnt
> herd: exception caught while executing 'start' on service 'cow-store':
> ERROR: In procedure mount: mount "/.rw-store" on "/gnu/store": Invalid argument
> environment variable `PATH' set to `/gnu/store/73p6bldnhm9lzcrhy8g96dzxawncd852-qemu-minimal-2.9.0/bin'

I think this has been reported before and I suspect it’s a
non-deterministic bug, not related to the specifics of this test.  Not
sure what happens.

Ludo’.

bug#26815: [PATCH 3/3] vm: Support EFI boot in base image.

[Marius Bakke]

[-- Attachment #1: Type: text/plain, Size: 794 bytes --]

Ludovic Courtès <ludo@gnu.org> writes:

> Hello!
>
> Mathieu Othacehe <m.othacehe@gmail.com> skribis:
>
>>> I think v4 is being prepared, I'll rebase this series once that is out.
>
> Thanks Marius for bearing with me.  :-)
>
> Are there specific bits that are needed here?

The main reason it's blocking is that without it, there is a conflict
between "grub" and "grub-efi" in PATH of the qemu image. So
"grub-mkstandalone" may not find the EFI target.

With the bootloader refactoring, the "normal" bootloader is passed down
as a gexp from (gnu system vm), allowing "grub-efi" to reign over PATH.

I'd like to make "grub-mkstandalone" a proper bootloader type in the
long run, but that requires some other refactoring that I did not feel
like embarking on right now :P

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 487 bytes --]

bug#26815: [PATCH 3/3] vm: Support EFI boot in base image.

[Ludovic Courtès]

Marius Bakke <mbakke@fastmail.com> skribis:

> Ludovic Courtès <ludo@gnu.org> writes:
>
>> Hello!
>>
>> Mathieu Othacehe <m.othacehe@gmail.com> skribis:
>>
>>>> I think v4 is being prepared, I'll rebase this series once that is out.
>>
>> Thanks Marius for bearing with me.  :-)
>>
>> Are there specific bits that are needed here?
>
> The main reason it's blocking is that without it, there is a conflict
> between "grub" and "grub-efi" in PATH of the qemu image. So
> "grub-mkstandalone" may not find the EFI target.
>
> With the bootloader refactoring, the "normal" bootloader is passed down
> as a gexp from (gnu system vm), allowing "grub-efi" to reign over PATH.
>
> I'd like to make "grub-mkstandalone" a proper bootloader type in the
> long run, but that requires some other refactoring that I did not feel
> like embarking on right now :P

OK, understood!

Ludo’.

bug#26815: [PATCH 3/3] vm: Support EFI boot in base image.

[Ludovic Courtès]

Hi Marius!

Marius Bakke <mbakke@fastmail.com> skribis:

> Ludovic Courtès <ludo@gnu.org> writes:
>
>> Hello!
>>
>> Mathieu Othacehe <m.othacehe@gmail.com> skribis:
>>
>>>> I think v4 is being prepared, I'll rebase this series once that is out.
>>
>> Thanks Marius for bearing with me.  :-)
>>
>> Are there specific bits that are needed here?
>
> The main reason it's blocking is that without it, there is a conflict
> between "grub" and "grub-efi" in PATH of the qemu image. So
> "grub-mkstandalone" may not find the EFI target.
>
> With the bootloader refactoring, the "normal" bootloader is passed down
> as a gexp from (gnu system vm), allowing "grub-efi" to reign over PATH.

Mathieu just pushed the bootloader patch series in ‘master’.

However, for the release, I thought it’d be more reasonable to not rush
that patch series, which is why the new ‘version-0.13.0’ branch does not
have it.

So now the question is, how can we merge your UEFI work?  How much work
is it to have it on ‘version-0.13.0’ without merging all the of the
bootloader patch series?

It would be awesome if we could quickly find a way to address this, so
we can indeed release tomorrow or later this week.

TIA,
Ludo’.

bug#26815: [PATCH 3/3] vm: Support EFI boot in base image.

[Marius Bakke]

[-- Attachment #1: Type: text/plain, Size: 902 bytes --]

Hi Ludovic,

Ludovic Courtès <ludo@gnu.org> writes:

> Mathieu just pushed the bootloader patch series in ‘master’.

Excellent news!

> However, for the release, I thought it’d be more reasonable to not rush
> that patch series, which is why the new ‘version-0.13.0’ branch does not
> have it.
>
> So now the question is, how can we merge your UEFI work?  How much work
> is it to have it on ‘version-0.13.0’ without merging all the of the
> bootloader patch series?

The problem was that "grub-efi" and "grub" conflicts in PATH without the
bootloader patches. Any ideas for how to use the proper grub-efi object?

> It would be awesome if we could quickly find a way to address this, so
> we can indeed release tomorrow or later this week.

Yay! I've sent an updated patch set, although yet untested due to lack
of substitutes! Will get on the OpenSSH bug ASAP.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 487 bytes --]

bug#26815: [PATCH 3/3] vm: Support EFI boot in base image.

[Marius Bakke]

[-- Attachment #1.1: Type: text/plain, Size: 380 bytes --]

Marius Bakke <mbakke@fastmail.com> writes:

> Yay! I've sent an updated patch set, although yet untested due to lack
> of substitutes! Will get on the OpenSSH bug ASAP.

Both patch series tested on a physical and virtual machine.

I do get some scary warnings at login time, not sure if they were there
before.

"stty: 'standard input': Input/output error"

Screenshot attached.


[-- Attachment #1.2: install.png --]
[-- Type: image/png, Size: 19793 bytes --]

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 487 bytes --]

bug#26815: [PATCH 3/3] vm: Support EFI boot in base image.

[Ricardo Wurmus]

Marius Bakke <mbakke@fastmail.com> writes:

> Marius Bakke <mbakke@fastmail.com> writes:
>
>> Yay! I've sent an updated patch set, although yet untested due to lack
>> of substitutes! Will get on the OpenSSH bug ASAP.
>
> Both patch series tested on a physical and virtual machine.
>
> I do get some scary warnings at login time, not sure if they were there
> before.
>
> "stty: 'standard input': Input/output error"
>
> Screenshot attached.

I have noticed messages like this before on my laptop.  I don’t think
that’s related to your changes.

-- 
Ricardo

GPG: BCA6 89B6 3655 3801 C3C6  2150 197A 5888 235F ACAC
https://elephly.net

bug#26815: [PATCH 3/3] vm: Support EFI boot in base image.

[Ludovic Courtès]

Marius Bakke <mbakke@fastmail.com> skribis:

> Marius Bakke <mbakke@fastmail.com> writes:
>
>> Yay! I've sent an updated patch set, although yet untested due to lack
>> of substitutes! Will get on the OpenSSH bug ASAP.
>
> Both patch series tested on a physical and virtual machine.
>
> I do get some scary warnings at login time, not sure if they were there
> before.
>
> "stty: 'standard input': Input/output error"
>
> Screenshot attached.

Indeed, not related to your changes, you can feel good.  :-)
https://bugs.gnu.org/23194

Ludo’.

bug#26815: [PATCH v4 1/3] vm: Support arbitrary partition flags.

[Marius Bakke]

* gnu/build/vm.scm (<partition>): Change BOOTABLE? to FLAGS.
(initialize-partition-table): Pass each flag to parted.
(initialize-hard-disk): Locate boot partition.
* gnu/system/vm.scm (qemu-image): Adjust partition flags.
---
 gnu/build/vm.scm  | 17 ++++++++++++-----
 gnu/system/vm.scm |  2 +-
 2 files changed, 13 insertions(+), 6 deletions(-)

diff --git a/gnu/build/vm.scm b/gnu/build/vm.scm
index 9a77bee72..c7449cfbe 100644
--- a/gnu/build/vm.scm
+++ b/gnu/build/vm.scm
@@ -3,6 +3,7 @@
 ;;; Copyright © 2016 Christopher Allan Webber <cwebber@dustycloud.org>
 ;;; Copyright © 2016 Leo Famulari <leo@famulari.name>
 ;;; Copyright © 2017 Mathieu Othacehe <m.othacehe@gmail.com>
+;;; Copyright © 2017 Marius Bakke <mbakke@fastmail.com>
 ;;;
 ;;; This file is part of GNU Guix.
 ;;;
@@ -41,7 +42,7 @@
             partition-size
             partition-file-system
             partition-label
-            partition-bootable?
+            partition-flags
             partition-initializer
 
             root-partition-initializer
@@ -141,7 +142,7 @@ the #:references-graphs parameter of 'derivation'."
   (size        partition-size)
   (file-system partition-file-system (default "ext4"))
   (label       partition-label (default #f))
-  (bootable?   partition-bootable? (default #f))
+  (flags       partition-flags (default '()))
   (initializer partition-initializer (default (const #t))))
 
 (define (fold2 proc seed1 seed2 lst)              ;TODO: factorize
@@ -168,9 +169,10 @@ actual /dev name based on DEVICE."
     (cons* "mkpart" "primary" "ext2"
            (format #f "~aB" offset)
            (format #f "~aB" (+ offset (partition-size part)))
-           (if (partition-bootable? part)
-               `("set" ,(number->string index) "boot" "on")
-               '())))
+           (append-map (lambda (flag)
+                         (list "set" (number->string index)
+                               (symbol->string flag) "on"))
+                       (partition-flags part))))
 
   (define (options partitions offset)
     (let loop ((partitions partitions)
@@ -303,6 +305,11 @@ in PARTITIONS, and using BOOTCFG as its bootloader configuration file.
 
 Each partition is initialized by calling its 'initializer' procedure,
 passing it a directory name where it is mounted."
+
+  (define (partition-bootable? partition)
+    "Return the first partition found with the boot flag set."
+    (member 'boot (partition-flags partition)))
+
   (let* ((partitions (initialize-partition-table device partitions))
          (root       (find partition-bootable? partitions))
          (target     "/fs"))
diff --git a/gnu/system/vm.scm b/gnu/system/vm.scm
index 2ee5c2b1e..e0e4d33d4 100644
--- a/gnu/system/vm.scm
+++ b/gnu/system/vm.scm
@@ -231,7 +231,7 @@ the image."
                                                 (* 10 (expt 2 20))))
                                      (label #$file-system-label)
                                      (file-system #$file-system-type)
-                                     (bootable? #t)
+                                     (flags '(boot))
                                      (initializer initialize)))))
              (initialize-hard-disk "/dev/vda"
                                    #:partitions partitions
-- 
2.13.0

bug#26815: [PATCH v4 2/3] vm: Support creating FAT partitions.

[Marius Bakke]

* gnu/build/vm.scm (create-ext-file-system, create-fat-file-system): New procedures.
(format-partition): Use them. Error for unknown file systems.
* gnu/system/vm.scm (qemu-image): Include DOSFSTOOLS.
* gnu/system/linux-initrd.scm (base-initrd): Always add nls_is8859-1.ko.
---
 gnu/build/vm.scm            | 30 ++++++++++++++++++++++++++----
 gnu/system/linux-initrd.scm |  4 +---
 gnu/system/vm.scm           |  2 +-
 3 files changed, 28 insertions(+), 8 deletions(-)

diff --git a/gnu/build/vm.scm b/gnu/build/vm.scm
index c7449cfbe..7ce1ec8e1 100644
--- a/gnu/build/vm.scm
+++ b/gnu/build/vm.scm
@@ -213,10 +213,10 @@ actual /dev name based on DEVICE."
 
 (define MS_BIND 4096)                             ; <sys/mounts.h> again!
 
-(define* (format-partition partition type
-                           #:key label)
-  "Create a file system TYPE on PARTITION.  If LABEL is true, use that as the
-volume name."
+(define* (create-ext-file-system partition type
+                                 #:key label)
+  "Create an ext-family filesystem of TYPE on PARTITION.  If LABEL is true,
+use that as the volume name."
   (format #t "creating ~a partition...\n" type)
   (unless (zero? (apply system* (string-append "mkfs." type)
                         "-F" partition
@@ -225,6 +225,28 @@ volume name."
                             '())))
     (error "failed to create partition")))
 
+(define* (create-fat-file-system partition
+                                 #:key label)
+  "Create a FAT filesystem on PARTITION.  The number of File Allocation Tables
+will be determined based on filesystem size.  If LABEL is true, use that as the
+volume name."
+  (format #t "creating FAT partition...\n")
+  (unless (zero? (apply system* "mkfs.fat" partition
+                        (if label
+                            `("-n" ,label)
+                            '())))
+    (error "failed to create FAT partition")))
+
+(define* (format-partition partition type
+                           #:key label)
+  "Create a file system TYPE on PARTITION.  If LABEL is true, use that as the
+volume name."
+  (cond ((string-prefix? "ext" type)
+         (create-ext-file-system partition type #:label label))
+        ((or (string-prefix? "fat" type) (string= "vfat" type))
+         (create-fat-file-system partition #:label label))
+        (else (error "Unsupported file system."))))
+
 (define (initialize-partition partition)
   "Format PARTITION, a <partition> object with a non-#f 'device' field, mount
 it, run its initializer, and unmount it."
diff --git a/gnu/system/linux-initrd.scm b/gnu/system/linux-initrd.scm
index dfe198e43..3a5e76034 100644
--- a/gnu/system/linux-initrd.scm
+++ b/gnu/system/linux-initrd.scm
@@ -268,6 +268,7 @@ loaded at boot time in the order in which they appear."
       "usbhid" "hid-generic" "hid-apple"      ;keyboards during early boot
       "dm-crypt" "xts" "serpent_generic" "wp512" ;for encrypted root partitions
       "nvme"                                     ;for new SSD NVMe devices
+      "nls_iso8859-1"                            ;for `mkfs.fat`, et.al
       ,@(if (string-match "^(x86_64|i[3-6]86)-" (%current-system))
             '("pata_acpi" "pata_atiixp"    ;for ATA controllers
               "isci")                      ;for SAS controllers like Intel C602
@@ -281,9 +282,6 @@ loaded at boot time in the order in which they appear."
       ,@(if (find (file-system-type-predicate "9p") file-systems)
             virtio-9p-modules
             '())
-      ,@(if (find (file-system-type-predicate "vfat") file-systems)
-            '("nls_iso8859-1")
-            '())
       ,@(if (find (file-system-type-predicate "btrfs") file-systems)
             '("btrfs")
             '())
diff --git a/gnu/system/vm.scm b/gnu/system/vm.scm
index e0e4d33d4..64770baf1 100644
--- a/gnu/system/vm.scm
+++ b/gnu/system/vm.scm
@@ -203,7 +203,7 @@ the image."
                       (guix build utils))
 
          (let ((inputs
-                '#$(append (list qemu parted e2fsprogs)
+                '#$(append (list qemu parted e2fsprogs dosfstools)
                            (map canonical-package
                                 (list sed grep coreutils findutils gawk))
                            (if register-closures? (list guix) '())))
-- 
2.13.0

bug#26815: [PATCH v4 3/3] vm: Add UEFI loader to disk images.

[Marius Bakke]

* gnu/system/vm.scm (qemu-image): Add GRUB-EFI to inputs. Append 40MiB EFI
System Partition.
* gnu/build/vm.scm (install-efi): New procedure.
(initialize-hard-disk): Generate grub EFI blob when ESP is present.
---
 gnu/build/vm.scm  | 59 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
 gnu/system/vm.scm | 19 +++++++++++++++---
 2 files changed, 75 insertions(+), 3 deletions(-)

diff --git a/gnu/build/vm.scm b/gnu/build/vm.scm
index 7ce1ec8e1..aca35999b 100644
--- a/gnu/build/vm.scm
+++ b/gnu/build/vm.scm
@@ -27,6 +27,7 @@
   #:use-module (gnu build linux-boot)
   #:use-module (gnu build install)
   #:use-module (guix records)
+  #:use-module (ice-9 format)
   #:use-module (ice-9 match)
   #:use-module (ice-9 regex)
   #:use-module (srfi srfi-1)
@@ -315,6 +316,33 @@ SYSTEM-DIRECTORY is the name of the directory of the 'system' derivation."
     (mkdir-p directory)
     (symlink bootcfg (string-append directory "/bootcfg"))))
 
+(define (install-efi esp config-file)
+  "Write a self-contained GRUB EFI loader to the mounted ESP using CONFIG-FILE."
+  (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
+         (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 requires a TMPDIR to prepare the firmware image.
+    (setenv "TMPDIR" esp)
+
+    (mkdir-p efi-directory)
+    (unless (zero? (system* "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=" config-file)))
+      (error "failed to create GRUB EFI image"))))
+
 (define* (initialize-hard-disk device
                                #:key
                                bootloader-package
@@ -332,8 +360,13 @@ passing it a directory name where it is mounted."
     "Return the first partition found with the boot flag set."
     (member 'boot (partition-flags partition)))
 
+  (define (partition-esp? partition)
+    "Return the first EFI System Partition."
+    (member 'esp (partition-flags partition)))
+
   (let* ((partitions (initialize-partition-table device partitions))
          (root       (find partition-bootable? partitions))
+         (esp        (find partition-esp? partitions))
          (target     "/fs"))
     (unless root
       (error "no bootable partition specified" partitions))
@@ -345,8 +378,34 @@ passing it a directory name where it is mounted."
     (mount (partition-device root) target (partition-file-system root))
     (install-boot-config bootcfg bootcfg-location target)
     (when bootloader-installer
+      (display "installing bootloader...\n")
       (bootloader-installer bootloader-package device target))
 
+    (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")))
+        (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.
+        (call-with-output-file grub-config
+          (lambda (port)
+            (format port
+                    "insmod part_msdos~@
+                    search --set=root --label gnu-disk-image~@
+                    configfile /boot/grub/grub.cfg~%")))
+
+        (display "creating EFI firmware image...")
+        (install-efi mount-point grub-config)
+        (display "done.\n")
+
+        (delete-file grub-config)
+        (umount mount-point)))
+
     ;; Register BOOTCFG as a GC root.
     (register-bootcfg-root target bootcfg)
 
diff --git a/gnu/system/vm.scm b/gnu/system/vm.scm
index 64770baf1..e0af90bee 100644
--- a/gnu/system/vm.scm
+++ b/gnu/system/vm.scm
@@ -3,6 +3,7 @@
 ;;; Copyright © 2016 Christopher Allan Webber <cwebber@dustycloud.org>
 ;;; Copyright © 2016 Leo Famulari <leo@famulari.name>
 ;;; Copyright © 2017 Mathieu Othacehe <m.othacehe@gmail.com>
+;;; Copyright © 2017 Marius Bakke <mbakke@fastmail.com>
 ;;;
 ;;; This file is part of GNU Guix.
 ;;;
@@ -203,7 +204,7 @@ the image."
                       (guix build utils))
 
          (let ((inputs
-                '#$(append (list qemu parted e2fsprogs dosfstools)
+                '#$(append (list qemu parted e2fsprogs dosfstools grub-efi)
                            (map canonical-package
                                 (list sed grep coreutils findutils gawk))
                            (if register-closures? (list guix) '())))
@@ -228,11 +229,23 @@ the image."
                                #:system-directory #$os-drv))
                   (partitions (list (partition
                                      (size #$(- disk-image-size
-                                                (* 10 (expt 2 20))))
+                                                (* 50 (expt 2 20))))
                                      (label #$file-system-label)
                                      (file-system #$file-system-type)
                                      (flags '(boot))
-                                     (initializer initialize)))))
+                                     (initializer initialize))
+                                    ;; Append a small EFI System Partition for
+                                    ;; use with UEFI bootloaders.
+                                    (partition
+                                     ;; The standalone grub image is about 10MiB, but
+                                     ;; leave some room for custom or multiple images.
+                                     (size (* 40 (expt 2 20)))
+                                     (label "GNU-ESP")             ;cosmetic only
+                                     ;; Use "vfat" here since this property is used
+                                     ;; when mounting. The actual FAT-ness is based
+                                     ;; on filesystem size (16 in this case).
+                                     (file-system "vfat")
+                                     (flags '(esp))))))
              (initialize-hard-disk "/dev/vda"
                                    #:partitions partitions
                                    #:bootloader-package
-- 
2.13.0

bug#26815: [PATCH v4 3/3] vm: Add UEFI loader to disk images.

[Ludovic Courtès]

Marius Bakke <mbakke@fastmail.com> skribis:

> * gnu/system/vm.scm (qemu-image): Add GRUB-EFI to inputs. Append 40MiB EFI
> System Partition.
> * gnu/build/vm.scm (install-efi): New procedure.
> (initialize-hard-disk): Generate grub EFI blob when ESP is present.

[...]

> +(define (install-efi esp config-file)
> +  "Write a self-contained GRUB EFI loader to the mounted ESP using CONFIG-FILE."
> +  (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
> +         (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 requires a TMPDIR to prepare the firmware image.
> +    (setenv "TMPDIR" esp)
> +
> +    (mkdir-p efi-directory)
> +    (unless (zero? (system* "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=" config-file)))

So this is where we need to make sure we’re taking ‘grub-mkstandalone’
from ‘grub-efi’ and not ‘grub’, right?  How do we ensure that?

> +++ b/gnu/system/vm.scm
> @@ -3,6 +3,7 @@
>  ;;; Copyright © 2016 Christopher Allan Webber <cwebber@dustycloud.org>
>  ;;; Copyright © 2016 Leo Famulari <leo@famulari.name>
>  ;;; Copyright © 2017 Mathieu Othacehe <m.othacehe@gmail.com>
> +;;; Copyright © 2017 Marius Bakke <mbakke@fastmail.com>
>  ;;;
>  ;;; This file is part of GNU Guix.
>  ;;;
> @@ -203,7 +204,7 @@ the image."
>                        (guix build utils))
>  
>           (let ((inputs
> -                '#$(append (list qemu parted e2fsprogs dosfstools)
> +                '#$(append (list qemu parted e2fsprogs dosfstools grub-efi)
>                             (map canonical-package
>                                  (list sed grep coreutils findutils gawk))
>                             (if register-closures? (list guix) '())))
> @@ -228,11 +229,23 @@ the image."
>                                 #:system-directory #$os-drv))
>                    (partitions (list (partition
>                                       (size #$(- disk-image-size
> -                                                (* 10 (expt 2 20))))
> +                                                (* 50 (expt 2 20))))
>                                       (label #$file-system-label)
>                                       (file-system #$file-system-type)
>                                       (flags '(boot))
> -                                     (initializer initialize)))))
> +                                     (initializer initialize))
> +                                    ;; Append a small EFI System Partition for
> +                                    ;; use with UEFI bootloaders.
> +                                    (partition
> +                                     ;; The standalone grub image is about 10MiB, but
> +                                     ;; leave some room for custom or multiple images.
> +                                     (size (* 40 (expt 2 20)))
> +                                     (label "GNU-ESP")             ;cosmetic only
> +                                     ;; Use "vfat" here since this property is used
> +                                     ;; when mounting. The actual FAT-ness is based
> +                                     ;; on filesystem size (16 in this case).
> +                                     (file-system "vfat")
> +                                     (flags '(esp))))))
>               (initialize-hard-disk "/dev/vda"
>                                     #:partitions partitions
>                                     #:bootloader-package

I thought we’d pass something like

  (initialize-hard-disk … #:grub-efi #$grub-eif)

to avoid the ambiguity?


Once we’re done with that, we’ll have to update one of the
‘operating-system’ declaration examples to show UEFI configuration, and
to update the doc to explain the installation process for UEFI.

Anyway, thanks for the updated patch series, looks like we’re almost
there!  :-)

Ludo’.

bug#26815: [PATCH v4 3/3] vm: Add UEFI loader to disk images.

[Marius Bakke]

[-- Attachment #1.1: Type: text/plain, Size: 2663 bytes --]

Ludovic Courtès <ludo@gnu.org> writes:

>> +++ b/gnu/system/vm.scm
>> @@ -228,11 +229,23 @@ the image."
>>                                 #:system-directory #$os-drv))
>>                    (partitions (list (partition
>>                                       (size #$(- disk-image-size
>> -                                                (* 10 (expt 2 20))))
>> +                                                (* 50 (expt 2 20))))
>>                                       (label #$file-system-label)
>>                                       (file-system #$file-system-type)
>>                                       (flags '(boot))
>> -                                     (initializer initialize)))))
>> +                                     (initializer initialize))
>> +                                    ;; Append a small EFI System Partition for
>> +                                    ;; use with UEFI bootloaders.
>> +                                    (partition
>> +                                     ;; The standalone grub image is about 10MiB, but
>> +                                     ;; leave some room for custom or multiple images.
>> +                                     (size (* 40 (expt 2 20)))
>> +                                     (label "GNU-ESP")             ;cosmetic only
>> +                                     ;; Use "vfat" here since this property is used
>> +                                     ;; when mounting. The actual FAT-ness is based
>> +                                     ;; on filesystem size (16 in this case).
>> +                                     (file-system "vfat")
>> +                                     (flags '(esp))))))
>>               (initialize-hard-disk "/dev/vda"
>>                                     #:partitions partitions
>>                                     #:bootloader-package
>
> I thought we’d pass something like
>
>   (initialize-hard-disk … #:grub-efi #$grub-eif)
>
> to avoid the ambiguity?

Thanks for the hint. With the attached patch, grub-efi is now passed
through to "install-efi". I haven't tested it on "version-0.13.0" but
assume it will work.

Now, I would like to instead implement an #:efi-loader keyword, and pass
it a derivation for the standalone EFI blob, but I'm not sure if I can
do that before the weekend. What do you think, is this "good enough" for
the time being?

> Once we’re done with that, we’ll have to update one of the
> ‘operating-system’ declaration examples to show UEFI configuration, and
> to update the doc to explain the installation process for UEFI.

I'll get on that.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1.2: 0001-vm-Initialize-hard-disk-now-takes-a-grub-efi-paramet.patch --]
[-- Type: text/x-patch, Size: 3921 bytes --]

From b2236aa915bfb32e974546790a1e87ef1b268403 Mon Sep 17 00:00:00 2001
From: Marius Bakke <mbakke@fastmail.com>
Date: Thu, 18 May 2017 18:10:31 +0200
Subject: [PATCH] vm: Initialize-hard-disk now takes a grub-efi parameter.

* gnu/build/vm.scm (initialize-hard-disk): Learn #:grub-efi.
(install-efi): Use it.
* gnu/system/vm.scm (qemu-image): Pass it.
---
 gnu/build/vm.scm  | 8 +++++---
 gnu/system/vm.scm | 3 ++-
 2 files changed, 7 insertions(+), 4 deletions(-)

diff --git a/gnu/build/vm.scm b/gnu/build/vm.scm
index c0c4c17fb..57619764c 100644
--- a/gnu/build/vm.scm
+++ b/gnu/build/vm.scm
@@ -316,12 +316,13 @@ SYSTEM-DIRECTORY is the name of the directory of the 'system' derivation."
     (mkdir-p directory)
     (symlink bootcfg (string-append directory "/bootcfg"))))
 
-(define (install-efi esp config-file)
+(define (install-efi grub esp config-file)
   "Write a self-contained GRUB EFI loader to the mounted ESP using CONFIG-FILE."
   (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)
@@ -336,7 +337,7 @@ SYSTEM-DIRECTORY is the name of the directory of the 'system' derivation."
     (setenv "TMPDIR" esp)
 
     (mkdir-p efi-directory)
-    (unless (zero? (system* "grub-mkstandalone" "-O" (car efi-targets)
+    (unless (zero? (system* grub-mkstandalone "-O" (car efi-targets)
                             "-o" (string-append efi-directory "/"
                                                 (cdr efi-targets))
                             ;; Graft the configuration file onto the image.
@@ -349,6 +350,7 @@ SYSTEM-DIRECTORY is the name of the directory of the 'system' derivation."
                                bootcfg
                                bootcfg-location
                                bootloader-installer
+                               (grub-efi #f)
                                (partitions '()))
   "Initialize DEVICE as a disk containing all the <partition> objects listed
 in PARTITIONS, and using BOOTCFG as its bootloader configuration file.
@@ -400,7 +402,7 @@ passing it a directory name where it is mounted."
                     configfile /boot/grub/grub.cfg~%")))
 
         (display "creating EFI firmware image...")
-        (install-efi mount-point grub-config)
+        (install-efi grub-efi mount-point grub-config)
         (display "done.\n")
 
         (delete-file grub-config)
diff --git a/gnu/system/vm.scm b/gnu/system/vm.scm
index e0af90bee..d282ba557 100644
--- a/gnu/system/vm.scm
+++ b/gnu/system/vm.scm
@@ -204,7 +204,7 @@ the image."
                       (guix build utils))
 
          (let ((inputs
-                '#$(append (list qemu parted e2fsprogs dosfstools grub-efi)
+                '#$(append (list qemu parted e2fsprogs dosfstools)
                            (map canonical-package
                                 (list sed grep coreutils findutils gawk))
                            (if register-closures? (list guix) '())))
@@ -248,6 +248,7 @@ the image."
                                      (flags '(esp))))))
              (initialize-hard-disk "/dev/vda"
                                    #:partitions partitions
+                                   #:grub-efi #$grub-efi
                                    #:bootloader-package
                                    #$(bootloader-package bootloader)
                                    #:bootcfg #$bootcfg-drv
-- 
2.13.0


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 487 bytes --]

bug#26815: [PATCH v4 3/3] vm: Add UEFI loader to disk images.

[Marius Bakke]

[-- Attachment #1.1: Type: text/plain, Size: 485 bytes --]

Marius Bakke <mbakke@fastmail.com> writes:

>> Once we’re done with that, we’ll have to update one of the
>> ‘operating-system’ declaration examples to show UEFI configuration, and
>> to update the doc to explain the installation process for UEFI.
>
> I'll get on that.

Here is a start on UEFI documentation update. Not sure if it's okay to
hijack the lightweight desktop template, or if it's better to have an
example closer to the (GRUB Configuration) section.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1.2: 0001-doc-Update-GRUB-EFI-instructions.patch --]
[-- Type: text/x-patch, Size: 5034 bytes --]

From 9db5b8acb017b8f283d36d09835f947879be4e30 Mon Sep 17 00:00:00 2001
From: Marius Bakke <mbakke@fastmail.com>
Date: Thu, 18 May 2017 19:24:36 +0200
Subject: [PATCH] doc: Update GRUB EFI instructions.

* doc/guix.texi (USB Stick Installation): Mention UEFI.
(Preparing for Installation): Add note about EFI System Partition.
(Proceeding with the Installation): Mention the GRUB-EFI package.
(GRUB Configuration): Likewise.
(Using the Configuration System): Hijack lightweight desktop to include UEFI.
* gnu/system/examples/lightweight-desktop.tmpl: Adjust native EFI configuration.
---
 doc/guix.texi                                | 19 ++++++++++++++-----
 gnu/system/examples/lightweight-desktop.tmpl | 25 ++++++++++++++++---------
 2 files changed, 30 insertions(+), 14 deletions(-)

diff --git a/doc/guix.texi b/doc/guix.texi
index 72741e034..fb67ea50d 100644
--- a/doc/guix.texi
+++ b/doc/guix.texi
@@ -7322,8 +7322,8 @@ Access to @file{/dev/sdX} usually requires root privileges.
 @end enumerate
 
 Once this is done, you should be able to reboot the system and boot from
-the USB stick.  The latter usually requires you to get in the BIOS' boot
-menu, where you can choose to boot from the USB stick.
+the USB stick.  The latter usually requires you to get in the BIOS' or
+UEFI boot menu, where you can choose to boot from the USB stick.
 
 @xref{Installing GuixSD in a VM}, if, instead, you would like to install
 GuixSD in a virtual machine (VM).
@@ -7477,6 +7477,14 @@ install BIOS-based GRUB (which is the default), make sure a BIOS Boot
 Partition is available (@pxref{BIOS installation,,, grub, GNU GRUB
 manual}).
 
+If you instead wish to use EFI-based grub, a FAT32 EFI System Partition
+is required.  This partition should be mounted at @file{/boot/efi} and
+must have the @dfn{esp} flag set.  E.g. for @command{parted}:
+
+@example
+parted /dev/sda set 1 esp on
+@end example
+
 Once you are done partitioning the target hard disk drive, you have to
 create a file system on the relevant partition(s)@footnote{Currently
 GuixSD only supports ext4 and btrfs file systems.  In particular, code
@@ -7590,7 +7598,8 @@ in particular:
 @itemize
 @item
 Make sure the @code{grub-configuration} form refers to the device you
-want to install GRUB on.
+want to install GRUB on.  You also need to specify the @code{grub-efi}
+package if you wish to use native UEFI boot.
 
 @item
 Be sure that your partition labels match the value of their respective
@@ -7879,7 +7888,7 @@ management, power management, and more, would look like this:
 @include os-config-desktop.texi
 @end lisp
 
-A graphical environment with a choice of lightweight window managers
+A graphical UEFI system with a choice of lightweight window managers
 instead of full-blown desktop environments would look like this:
 
 @lisp
@@ -15277,7 +15286,7 @@ The number of seconds to wait for keyboard input before booting.  Set to
 The @code{grub-theme} object describing the theme to use.
 
 @item @code{grub} (default: @code{grub})
-The GRUB package to use.
+The GRUB package to use.  Currently either @code{grub} or @code{grub-efi}.
 
 @item @code{terminal-outputs} (default: @code{'gfxterm})
 The output terminals used for the GRUB boot menu, as a list of symbols.
diff --git a/gnu/system/examples/lightweight-desktop.tmpl b/gnu/system/examples/lightweight-desktop.tmpl
index 389ec8574..ad92ad163 100644
--- a/gnu/system/examples/lightweight-desktop.tmpl
+++ b/gnu/system/examples/lightweight-desktop.tmpl
@@ -4,7 +4,7 @@
 
 (use-modules (gnu) (gnu system nss))
 (use-service-modules desktop)
-(use-package-modules wm ratpoison certs suckless)
+(use-package-modules bootloaders wm ratpoison certs suckless)
 
 (operating-system
   (host-name "antelope")
@@ -13,14 +13,21 @@
 
   ;; Assuming /dev/sdX is the target hard disk, and "my-root"
   ;; is the label of the target root file system.
-  (bootloader (grub-configuration (device "/dev/sdX")))
-
-  (file-systems (cons (file-system
-                        (device "my-root")
-                        (title 'label)
-                        (mount-point "/")
-                        (type "ext4"))
-                      %base-file-systems))
+  (bootloader (grub-configuration (grub grub-efi)
+                                  (device "/dev/sdX")))
+
+  (file-systems (cons* (file-system
+                         (device "my-root")
+                         (title 'label)
+                         (mount-point "/")
+                         (type "ext4"))
+                       (file-system
+                         ;; Specify partition here since FAT
+                         ;; labels are currently unsupported.
+                         (device "/dev/sdX1")
+                         (mount-point "/boot/efi")
+                         (type "vfat"))
+                       %base-file-systems))
 
   (users (cons (user-account
                 (name "alice")
-- 
2.13.0


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 487 bytes --]

bug#26815: [PATCH v4 3/3] vm: Add UEFI loader to disk images.

[Ludovic Courtès]

Marius Bakke <mbakke@fastmail.com> skribis:

> Here is a start on UEFI documentation update. Not sure if it's okay to
> hijack the lightweight desktop template, or if it's better to have an
> example closer to the (GRUB Configuration) section.

I think it’s OK.

> From 9db5b8acb017b8f283d36d09835f947879be4e30 Mon Sep 17 00:00:00 2001
> From: Marius Bakke <mbakke@fastmail.com>
> Date: Thu, 18 May 2017 19:24:36 +0200
> Subject: [PATCH] doc: Update GRUB EFI instructions.
>
> * doc/guix.texi (USB Stick Installation): Mention UEFI.
> (Preparing for Installation): Add note about EFI System Partition.
> (Proceeding with the Installation): Mention the GRUB-EFI package.
> (GRUB Configuration): Likewise.
> (Using the Configuration System): Hijack lightweight desktop to include UEFI.
> * gnu/system/examples/lightweight-desktop.tmpl: Adjust native EFI configuration.

Great!  Minor suggestions:

> +If you instead wish to use EFI-based grub, a FAT32 EFI System Partition
> +is required.  This partition should be mounted at @file{/boot/efi} and
> +must have the @dfn{esp} flag set.  E.g. for @command{parted}:

I would change this paragraph like this

--8<---------------cut here---------------start------------->8---
@cindex EFI, installation
@cindex UEFI, installation
@cindex ESP, EFI system partition
If you instead wish to use EFI-based GRUB, a FAT32 @dfn{EFI System Partition}
(ESP) is required.  This partition should be mounted at @file{/boot/efi} and
must have the @code{esp} flag set.  E.g., for @command{parted}:
--8<---------------cut here---------------end--------------->8---

>  @item @code{grub} (default: @code{grub})
> -The GRUB package to use.
> +The GRUB package to use.  Currently either @code{grub} or @code{grub-efi}.

Maybe:

--8<---------------cut here---------------start------------->8---
@cindex EFI, bootloader
@cindex UEFI, bootloader
@cindex BIOS, bootloader
The GRUB package to use.  Currently either @code{grub}, for ``legacy''
x86 BIOS systems, or @code{grub-efi}, for modern systems using the
@dfn{Unified Extensible Firmware Interface} (UEFI).
--8<---------------cut here---------------end--------------->8---

>    ;; Assuming /dev/sdX is the target hard disk, and "my-root"
>    ;; is the label of the target root file system.
> -  (bootloader (grub-configuration (device "/dev/sdX")))
> -
> -  (file-systems (cons (file-system
> -                        (device "my-root")
> -                        (title 'label)
> -                        (mount-point "/")
> -                        (type "ext4"))
> -                      %base-file-systems))
> +  (bootloader (grub-configuration (grub grub-efi)
> +                                  (device "/dev/sdX")))

In the comment could you add that this is for a UEFI/EFI system?

Thank you!

Ludo’.

bug#26815: [PATCH v4 3/3] vm: Add UEFI loader to disk images.

[Marius Bakke]

[-- Attachment #1: Type: text/plain, Size: 1221 bytes --]

Ludovic Courtès <ludo@gnu.org> writes:

> Marius Bakke <mbakke@fastmail.com> skribis:
>
>> Here is a start on UEFI documentation update. Not sure if it's okay to
>> hijack the lightweight desktop template, or if it's better to have an
>> example closer to the (GRUB Configuration) section.
>
> I think it’s OK.
>
>> From 9db5b8acb017b8f283d36d09835f947879be4e30 Mon Sep 17 00:00:00 2001
>> From: Marius Bakke <mbakke@fastmail.com>
>> Date: Thu, 18 May 2017 19:24:36 +0200
>> Subject: [PATCH] doc: Update GRUB EFI instructions.
>>
>> * doc/guix.texi (USB Stick Installation): Mention UEFI.
>> (Preparing for Installation): Add note about EFI System Partition.
>> (Proceeding with the Installation): Mention the GRUB-EFI package.
>> (GRUB Configuration): Likewise.
>> (Using the Configuration System): Hijack lightweight desktop to include UEFI.
>> * gnu/system/examples/lightweight-desktop.tmpl: Adjust native EFI configuration.
>
> Great!  Minor suggestions:

[...]

Thanks again for the excellent feedback. I did an installation by
"naively" following the manual and noticed that it does not mention
mounting other partitions before `guix system init`. So, I added it.

Pushed at last!

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 487 bytes --]

bug#26815: [PATCH v4 3/3] vm: Add UEFI loader to disk images.

[Mathieu Othacehe]

Hi Marius,

> Thanks again for the excellent feedback. I did an installation by
> "naively" following the manual and noticed that it does not mention
> mounting other partitions before `guix system init`. So, I added it.
>
> Pushed at last!

Thanks for this serie !

As a follow-up, when things will calm down, you might consider moving
some stuff in a grub-efi <bootloader> structure.

For example the (when esp ...) snippet in initialize-hard-disk or the
install-efi procedure.

Mathieu

bug#26815: [PATCH v4 3/3] vm: Add UEFI loader to disk images.

[Marius Bakke]

[-- Attachment #1: Type: text/plain, Size: 1420 bytes --]

Mathieu Othacehe <m.othacehe@gmail.com> writes:

> Hi Marius,
>
>> Thanks again for the excellent feedback. I did an installation by
>> "naively" following the manual and noticed that it does not mention
>> mounting other partitions before `guix system init`. So, I added it.
>>
>> Pushed at last!
>
> Thanks for this serie !
>
> As a follow-up, when things will calm down, you might consider moving
> some stuff in a grub-efi <bootloader> structure.

I plan to make a generic "grub-standalone" variant. Will try to fit it
into the new bootloader API. The regular grub-efi variant won't cut it
because it can't update EFI variables on the target system, so we need
the "self-contained" grub snapshot generated by grub-mkstandalone(1).

Also, the normal configuration file doesn't work with it, but a minimal
config that loads the real one at boot is okay. Not sure why!

> For example the (when esp ...) snippet in initialize-hard-disk or the
> install-efi procedure.

Here, I plan on implementing an #:efi-loader keyword that is passed a
gexp for the generated EFI firmware image, instead of the current
approach that passes #:grub-efi and builds it outside the store.

On a related topic: The "device" field for grub-efi is a noop;
`grub-install` looks for "/boot/efi" only (or "--efi-directory").
Currently the example uses the ESP for "device", but it should not have
to be set. Not sure what to do about it, ideas?

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 487 bytes --]

bug#26815: [PATCH v4 3/3] vm: Add UEFI loader to disk images.

[Ludovic Courtès]

Hi Marius,

I’m unable to cherry-pick ecf5d5376979fadd971559367bf553df89fcc62b to
‘version-0.13.0’ because there are conflicts and I’m not sure how to
resolve them.

Could you take a look?

TIA!

Ludo’.

bug#26815: [PATCH v4 3/3] vm: Add UEFI loader to disk images.

[Mathieu Othacehe]

Hi !

> I’m unable to cherry-pick ecf5d5376979fadd971559367bf553df89fcc62b to
> ‘version-0.13.0’ because there are conflicts and I’m not sure how to
> resolve them.

I just had a look, the following conflict :

--8<---------------cut here---------------start------------->8---
<<<<<<< HEAD
                                   #:grub.cfg #$grub-configuration)
=======
                                   #:grub-efi #$grub-efi
                                   #:bootloader-package
                                   #$(bootloader-package bootloader)
                                   #:bootcfg #$bootcfg-drv
                                   #:bootcfg-location
                                   #$(bootloader-configuration-file bootloader)
                                   #:bootloader-installer
                                   #$(bootloader-installer bootloader))
>>>>>>> ecf5d5376... vm: Add UEFI loader to disk images.
--8<---------------cut here---------------end--------------->8---

means that patches b09a8da4a..5641dffb0 are required here. As I agree it
is not a good idea to take them into 0.13 release, the only solution
left is to rewrite Marius patch on top of a pre-bootloader rework
commit.

Mathieu

bug#26815: [PATCH v4 3/3] vm: Add UEFI loader to disk images.

[Marius Bakke]

[-- Attachment #1: Type: text/plain, Size: 349 bytes --]

Ludovic Courtès <ludo@gnu.org> writes:

> Hi Marius,
>
> I’m unable to cherry-pick ecf5d5376979fadd971559367bf553df89fcc62b to
> ‘version-0.13.0’ because there are conflicts and I’m not sure how to
> resolve them.
>
> Could you take a look?

I resolved the conflicts, booted a UEFI VM, and pushed the commit.

Still works! \o/

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 487 bytes --]

bug#26815: [PATCH v4 3/3] vm: Add UEFI loader to disk images.

[Ludovic Courtès]

Marius Bakke <mbakke@fastmail.com> skribis:

> Ludovic Courtès <ludo@gnu.org> writes:
>
>> Hi Marius,
>>
>> I’m unable to cherry-pick ecf5d5376979fadd971559367bf553df89fcc62b to
>> ‘version-0.13.0’ because there are conflicts and I’m not sure how to
>> resolve them.
>>
>> Could you take a look?
>
> I resolved the conflicts, booted a UEFI VM, and pushed the commit.
>
> Still works! \o/

Awesome, thanks for the super-fast reply!

Ludo’.

bug#26815: [PATCH v4 3/3] vm: Add UEFI loader to disk images.

[Mathieu Othacehe]

Hi Marius,

> I plan to make a generic "grub-standalone" variant. Will try to fit it
> into the new bootloader API. The regular grub-efi variant won't cut it
> because it can't update EFI variables on the target system, so we need
> the "self-contained" grub snapshot generated by grub-mkstandalone(1).
>
> Also, the normal configuration file doesn't work with it, but a minimal
> config that loads the real one at boot is okay. Not sure why!

Ok, but don't we need to do it on system reconfigure too ?

>
>> For example the (when esp ...) snippet in initialize-hard-disk or the
>> install-efi procedure.
>
> Here, I plan on implementing an #:efi-loader keyword that is passed a
> gexp for the generated EFI firmware image, instead of the current
> approach that passes #:grub-efi and builds it outside the store.

Great !

>
> On a related topic: The "device" field for grub-efi is a noop;
> `grub-install` looks for "/boot/efi" only (or "--efi-directory").
> Currently the example uses the ESP for "device", but it should not have
> to be set. Not sure what to do about it, ideas?

The "device" field of <bootloader> defaults to #f so maybe you can skip
it ?

Mathieu

bug#26815: [PATCH v4 3/3] vm: Add UEFI loader to disk images.

[Marius Bakke]

[-- Attachment #1: Type: text/plain, Size: 1372 bytes --]

Mathieu Othacehe <m.othacehe@gmail.com> writes:

> Hi Marius,
>
>> I plan to make a generic "grub-standalone" variant. Will try to fit it
>> into the new bootloader API. The regular grub-efi variant won't cut it
>> because it can't update EFI variables on the target system, so we need
>> the "self-contained" grub snapshot generated by grub-mkstandalone(1).
>>
>> Also, the normal configuration file doesn't work with it, but a minimal
>> config that loads the real one at boot is okay. Not sure why!
>
> Ok, but don't we need to do it on system reconfigure too ?

Using "grub-mkstandalone" as an everyday bootloader is not a priority,
though it would be nice to support that too. It doesn't make much sense
to use that instead of "grub-efi" on a native system, but it can also
generate e.g. standalone PXE, u-boot and yeelong loaders.

>> On a related topic: The "device" field for grub-efi is a noop;
>> `grub-install` looks for "/boot/efi" only (or "--efi-directory").
>> Currently the example uses the ESP for "device", but it should not have
>> to be set. Not sure what to do about it, ideas?
>
> The "device" field of <bootloader> defaults to #f so maybe you can skip
> it ?

I tried that, but the "install-grub" procedure refused to function
without a device argument. Will probably have to implement an
"install-grub-efi" procedure, didn't look much into it yet.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 487 bytes --]

bug#26815: [PATCH v4 3/3] vm: Add UEFI loader to disk images.

[Ludovic Courtès]

Marius Bakke <mbakke@fastmail.com> skribis:

> Ludovic Courtès <ludo@gnu.org> writes:
>
>> Marius Bakke <mbakke@fastmail.com> skribis:
>>
>>> Here is a start on UEFI documentation update. Not sure if it's okay to
>>> hijack the lightweight desktop template, or if it's better to have an
>>> example closer to the (GRUB Configuration) section.
>>
>> I think it’s OK.
>>
>>> From 9db5b8acb017b8f283d36d09835f947879be4e30 Mon Sep 17 00:00:00 2001
>>> From: Marius Bakke <mbakke@fastmail.com>
>>> Date: Thu, 18 May 2017 19:24:36 +0200
>>> Subject: [PATCH] doc: Update GRUB EFI instructions.
>>>
>>> * doc/guix.texi (USB Stick Installation): Mention UEFI.
>>> (Preparing for Installation): Add note about EFI System Partition.
>>> (Proceeding with the Installation): Mention the GRUB-EFI package.
>>> (GRUB Configuration): Likewise.
>>> (Using the Configuration System): Hijack lightweight desktop to include UEFI.
>>> * gnu/system/examples/lightweight-desktop.tmpl: Adjust native EFI configuration.
>>
>> Great!  Minor suggestions:
>
> [...]
>
> Thanks again for the excellent feedback. I did an installation by
> "naively" following the manual and noticed that it does not mention
> mounting other partitions before `guix system init`. So, I added it.

Wonderful, thanks for testing the installation procedure!

So modulo the Guile-SSH bug, we should be all set for the release…

Ludo’.

bug#26815: [PATCH v4 3/3] vm: Add UEFI loader to disk images.

[Ludovic Courtès]

Marius Bakke <mbakke@fastmail.com> skribis:

> Ludovic Courtès <ludo@gnu.org> writes:

[...]

>> I thought we’d pass something like
>>
>>   (initialize-hard-disk … #:grub-efi #$grub-eif)
>>
>> to avoid the ambiguity?
>
> Thanks for the hint. With the attached patch, grub-efi is now passed
> through to "install-efi". I haven't tested it on "version-0.13.0" but
> assume it will work.

Awesome!

> Now, I would like to instead implement an #:efi-loader keyword, and pass
> it a derivation for the standalone EFI blob, but I'm not sure if I can
> do that before the weekend. What do you think, is this "good enough" for
> the time being?

I think so.  I’d like to push that release out.  :-)

>> Once we’re done with that, we’ll have to update one of the
>> ‘operating-system’ declaration examples to show UEFI configuration, and
>> to update the doc to explain the installation process for UEFI.
>
> I'll get on that.

Cool.

> From b2236aa915bfb32e974546790a1e87ef1b268403 Mon Sep 17 00:00:00 2001
> From: Marius Bakke <mbakke@fastmail.com>
> Date: Thu, 18 May 2017 18:10:31 +0200
> Subject: [PATCH] vm: Initialize-hard-disk now takes a grub-efi parameter.
>
> * gnu/build/vm.scm (initialize-hard-disk): Learn #:grub-efi.
> (install-efi): Use it.
> * gnu/system/vm.scm (qemu-image): Pass it.

LGTM!  I suppose you’d squash it with the initial patch?

Thanks a lot!

Ludo’.

bug#26815: [PATCH v4 3/3] vm: Add UEFI loader to disk images.

[Marius Bakke]

[-- Attachment #1: Type: text/plain, Size: 773 bytes --]

Ludovic Courtès <ludo@gnu.org> writes:

>> From b2236aa915bfb32e974546790a1e87ef1b268403 Mon Sep 17 00:00:00 2001
>> From: Marius Bakke <mbakke@fastmail.com>
>> Date: Thu, 18 May 2017 18:10:31 +0200
>> Subject: [PATCH] vm: Initialize-hard-disk now takes a grub-efi parameter.
>>
>> * gnu/build/vm.scm (initialize-hard-disk): Learn #:grub-efi.
>> (install-efi): Use it.
>> * gnu/system/vm.scm (qemu-image): Pass it.
>
> LGTM!  I suppose you’d squash it with the initial patch?

Indeed. Figured this was an easier review format, though.

I'll push this series shortly, after some basic sanity checks. Thanks a
lot for the great feedback! I'll work on a more Guixy way of creating
the standalone GRUB image and also a system test for the next release ;-)

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 487 bytes --]

bug#26815: [PATCH v4 3/3] vm: Add UEFI loader to disk images.

[Ludovic Courtès]

Marius Bakke <mbakke@fastmail.com> skribis:

> Ludovic Courtès <ludo@gnu.org> writes:
>
>>> From b2236aa915bfb32e974546790a1e87ef1b268403 Mon Sep 17 00:00:00 2001
>>> From: Marius Bakke <mbakke@fastmail.com>
>>> Date: Thu, 18 May 2017 18:10:31 +0200
>>> Subject: [PATCH] vm: Initialize-hard-disk now takes a grub-efi parameter.
>>>
>>> * gnu/build/vm.scm (initialize-hard-disk): Learn #:grub-efi.
>>> (install-efi): Use it.
>>> * gnu/system/vm.scm (qemu-image): Pass it.
>>
>> LGTM!  I suppose you’d squash it with the initial patch?
>
> Indeed. Figured this was an easier review format, though.

Definitely.

> I'll push this series shortly, after some basic sanity checks. Thanks a
> lot for the great feedback! I'll work on a more Guixy way of creating
> the standalone GRUB image and also a system test for the next release ;-)

Awesome, thank you!

Ludo’.

bug#26815: [PATCH v4 1/3] vm: Support arbitrary partition flags.

[Ludovic Courtès]

Marius Bakke <mbakke@fastmail.com> skribis:

> * gnu/build/vm.scm (<partition>): Change BOOTABLE? to FLAGS.
> (initialize-partition-table): Pass each flag to parted.
> (initialize-hard-disk): Locate boot partition.
> * gnu/system/vm.scm (qemu-image): Adjust partition flags.

[...]

> * gnu/build/vm.scm (create-ext-file-system, create-fat-file-system): New procedures.
> (format-partition): Use them. Error for unknown file systems.
> * gnu/system/vm.scm (qemu-image): Include DOSFSTOOLS.
> * gnu/system/linux-initrd.scm (base-initrd): Always add nls_is8859-1.ko.

LGTM (I think they’re unchanged, right?).

Ludo’.

bug#26815: [PATCH 1/3] vm: Add support for arbitrary partition flags.

[Danny Milosavljevic]

LGTM!

bug#26815: [PATCH 1/3] vm: Add support for arbitrary partition flags.

[Ludovic Courtès]

Hi Marius!

Marius Bakke <mbakke@fastmail.com> skribis:

> * gnu/build/vm.scm (<partition>): Change BOOTABLE? to FLAGS.
> (initialize-partition-table): Pass each flag to parted.
> (initialize-hard-disk): Search for root partition by "boot" flag.
> * gnu/system/vm.scm (qemu-image): Adjust partitions accordingly.

[...]

> +           (apply append (map (lambda (flag)
> +                                (cons* "set" (number->string index)
> +                                       (symbol->string flag)
> +                                       "on" '()))
> +                              (partition-flags part)))))

Use ‘append-map’ instead of (apply append (map …)).

OK with this change, thanks!

Ludo’.

bug#26815: [PATCH 1/3] vm: Add support for arbitrary partition flags.

[Maxim Cournoyer]

[-- Attachment #1: Type: text/plain, Size: 2407 bytes --]

Hello Marius,

Marius Bakke <mbakke@fastmail.com> writes:

> * gnu/build/vm.scm (<partition>): Change BOOTABLE? to FLAGS.
> (initialize-partition-table): Pass each flag to parted.
> (initialize-hard-disk): Search for root partition by "boot" flag.
> * gnu/system/vm.scm (qemu-image): Adjust partitions accordingly.
> ---
>  gnu/build/vm.scm  | 18 +++++++++++++-----
>  gnu/system/vm.scm |  2 +-
>  2 files changed, 14 insertions(+), 6 deletions(-)
>
> diff --git a/gnu/build/vm.scm b/gnu/build/vm.scm
> index 440596a40..3286ffb02 100644
> --- a/gnu/build/vm.scm
> +++ b/gnu/build/vm.scm
> @@ -3,6 +3,7 @@
>  ;;; Copyright © 2016 Christopher Allan Webber <cwebber@dustycloud.org>
>  ;;; Copyright © 2016 Leo Famulari <leo@famulari.name>
>  ;;; Copyright © 2017 Mathieu Othacehe <m.othacehe@gmail.com>
> +;;; Copyright © 2017 Marius Bakke <mbakke@fastmail.com>
>  ;;;
>  ;;; This file is part of GNU Guix.
>  ;;;
> @@ -41,7 +42,7 @@
>              partition-size
>              partition-file-system
>              partition-label
> -            partition-bootable?
> +            partition-flags
>              partition-initializer
>  
>              root-partition-initializer
> @@ -141,7 +142,7 @@ the #:references-graphs parameter of 'derivation'."
>    (size        partition-size)
>    (file-system partition-file-system (default "ext4"))
>    (label       partition-label (default #f))
> -  (bootable?   partition-bootable? (default #f))
> +  (flags       partition-flags (default '()))
>    (initializer partition-initializer (default (const #t))))
>  
>  (define (fold2 proc seed1 seed2 lst)              ;TODO: factorize
> @@ -168,9 +169,11 @@ actual /dev name based on DEVICE."
>      (cons* "mkpart" "primary" "ext2"
>             (format #f "~aB" offset)
>             (format #f "~aB" (+ offset (partition-size part)))
> -           (if (partition-bootable? part)
> -               `("set" ,(number->string index) "boot" "on")
> -               '())))
> +           (apply append (map (lambda (flag)

I think you could replace the 'apply, append, map' by `map-append' here.

> +                                (cons* "set" (number->string index)
> +                                       (symbol->string flag)
> +                                       "on" '()))
> +                              (partition-flags part)))))

Maxim

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 832 bytes --]

bug#26815: [PATCH 1/3] vm: Add support for arbitrary partition flags.

[Danny Milosavljevic]

> +  (define (partition-bootable? partitions)

Why the plural?

> +    "Return the first partition found with the boot flag set."

It's only one, right?

Otherwise LGTM!

bug#26815: [PATCH 0/3] Hybrid UEFI disk image

[Marius Bakke]

[-- Attachment #1: Type: text/plain, Size: 374 bytes --]

Marius Bakke <mbakke@fastmail.com> writes:

> This ended up a lot simpler than anticipated.
>
> These patches are on top of <https://bugs.gnu.org/26339>, but can be
> ported to master since it ultimately didn't use any of the new tricks.

Never mind, grub-install picks up grub-efi when it should use normal
grub with these patches on current master, so #26339 is required.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 487 bytes --]