bug#47889: [installer image] grub-install efi fails getting canonical path to /boot/efi on dos-formatted disk

bug#47889: [installer image] grub-install efi fails getting canonical path to /boot/efi on dos-formatted disk

[pelzflorian (Florian Pelz)]

When installing from the Guix install image, grub-install fails on my
x86_64 ASRock Beebox PC:

guix system: error: '/gnu/store/hsc3gsqbxkl64nx38sf2fgs2fxzbpr0i-grub-efi-2.04/sbin/grub-install --boot-directory /mnt/boot --bootloader-id=Guix --efi-directory /boot/efi' exited with status 1; output follows:

 Installing for x86_64-efi platform.
 /gnu/store/hsc3gsqbxkl64nx38sf2fgs2fxzbpr0i-grub-efi-2.04/sbin/grub-install: error: failed to get canonical path of `/boot/efi'.

I let the Guix installer (git master commit
fef2f08bc640f78cc0a86fc7be3eccbc07b5e98c) auto-partition my entire
disk without encryption.

“fdisk -l /dev/sda” shows:

Disk /dev/sda: 238.49 GiB, 256060514304 bytes, 500118192 sectors
[…]
Disklabel type: dos
Disk identifier: 0xd0416d83

Device     Boot   Start       End   Sectors   Size Id Type
/dev/sda1          2048   7811071   7009024   3.7G 82 Linux swap / Solaris
/dev/sda2  *    7011072 500117583 492306432 234.8G 83 Linux

Regards,
Florian

bug#47889: [installer image] grub-install efi fails getting canonical path to /boot/efi on dos-formatted disk

[pelzflorian (Florian Pelz)]

On Mon, Apr 19, 2021 at 11:29:54AM +0200, pelzflorian (Florian Pelz) wrote:
> When installing from the Guix install image, grub-install fails on my
> x86_64 ASRock Beebox PC:

This error occurred on multiple install attempts, but apparently the
last one broke my UEFI; the Beebox can no longer output anything on
the screen.  I will need some time until I can jumper the mainboard.

Regards,
Florian

bug#47889: [installer image] grub-install efi fails getting canonical path to /boot/efi on dos-formatted disk

[Leo Famulari]

On Mon, Apr 19, 2021 at 11:29:54AM +0200, pelzflorian (Florian Pelz) wrote:
> When installing from the Guix install image, grub-install fails on my
> x86_64 ASRock Beebox PC:

Can you be specific about which install image you used? For example, can
you provide the URL of the image?

bug#47889: [installer image] grub-install efi fails getting canonical path to /boot/efi on dos-formatted disk

[pelzflorian (Florian Pelz)]

On Mon, Apr 19, 2021 at 01:27:39PM -0400, Leo Famulari wrote:
> On Mon, Apr 19, 2021 at 11:29:54AM +0200, pelzflorian (Florian Pelz) wrote:
> > When installing from the Guix install image, grub-install fails on my
> > x86_64 ASRock Beebox PC:
> 
> Can you be specific about which install image you used? For example, can
> you provide the URL of the image?

This one:

https://ci.guix.gnu.org/build/175632/details

Same thing happened on an image manually created by running

./pre-inst-env guix system image -t iso9660 gnu/system/install.scm

on commit fef2f08bc640f78cc0a86fc7be3eccbc07b5e98c (after fixing it by
removing the broken “vi” entry from po/packages/LINGUAS which Julien
had only fixed in a later commit).

Regards,
Florian

P.S. I can test again, the PC works again after clearing the CMOS.

bug#47889: [installer image] grub-install efi fails getting canonical path to /boot/efi on dos-formatted disk

[Ludovic Courtès]

Hi Florian,

"pelzflorian (Florian Pelz)" <pelzflorian@pelzflorian.de> skribis:

> When installing from the Guix install image, grub-install fails on my
> x86_64 ASRock Beebox PC:
>
> guix system: error: '/gnu/store/hsc3gsqbxkl64nx38sf2fgs2fxzbpr0i-grub-efi-2.04/sbin/grub-install --boot-directory /mnt/boot --bootloader-id=Guix --efi-directory /boot/efi' exited with status 1; output follows:
>
>  Installing for x86_64-efi platform.
>  /gnu/store/hsc3gsqbxkl64nx38sf2fgs2fxzbpr0i-grub-efi-2.04/sbin/grub-install: error: failed to get canonical path of `/boot/efi'.
>
> I let the Guix installer (git master commit
> fef2f08bc640f78cc0a86fc7be3eccbc07b5e98c) auto-partition my entire
> disk without encryption.
>
> “fdisk -l /dev/sda” shows:
>
> Disk /dev/sda: 238.49 GiB, 256060514304 bytes, 500118192 sectors
> […]
> Disklabel type: dos
> Disk identifier: 0xd0416d83
>
> Device     Boot   Start       End   Sectors   Size Id Type
> /dev/sda1          2048   7811071   7009024   3.7G 82 Linux swap / Solaris
> /dev/sda2  *    7011072 500117583 492306432 234.8G 83 Linux

There’s no EFI (vfat) partition here.  Is it an EFI machine?

Is /boot/efi mounted when you boot the installation image?

If it were EFI, it would be a GPT partition table, not a DOS one.

It seems to me there’s confusion here: the generated config file seems
to be using ‘grub-efi-bootloader’, but the machine is actually not EFI.
WDYT?

Thanks,
Ludo’.

bug#47889: [installer image] grub-install efi fails getting canonical path to /boot/efi on dos-formatted disk

[pelzflorian (Florian Pelz)]

On Thu, Apr 22, 2021 at 03:28:39PM +0200, Ludovic Courtès wrote:
> There’s no EFI (vfat) partition here.  Is it an EFI machine?
> 
> Is /boot/efi mounted when you boot the installation image?

No because there is no EFI partition.  If I create one and restart the
installer, then it is *not* mounted either, only /mnt/boot/efi later
during the install.  (I believe since the installer is installed as on
an external medium, it does not need an EFI partition.)

I had booted the install image via UEFI boot and had expected auto
partitioning and the default configuration to do the right thing.

> If it were EFI, it would be a GPT partition table, not a DOS one.

Thank you for explaining.  If Guix defaults require GPT on EFI
(according to
<https://wiki.archlinux.org/index.php/EFI_system_partition#Create_the_partition>)
some computers apparently really do not support dos partition tables
for the EFI system partition), then I think
gnu/installer/newt/partition.scm should disallow selecting a DOS
scheme when auto partitioning the entire disk.

Regards,
Florian

bug#47889: [installer image] grub-install efi fails getting canonical path to /boot/efi on dos-formatted disk

[Ludovic Courtès]

Hi Florian,

(Cc: Mathieu.)

"pelzflorian (Florian Pelz)" <pelzflorian@pelzflorian.de> skribis:

> On Thu, Apr 22, 2021 at 03:28:39PM +0200, Ludovic Courtès wrote:
>> There’s no EFI (vfat) partition here.  Is it an EFI machine?
>> 
>> Is /boot/efi mounted when you boot the installation image?
>
> No because there is no EFI partition.  If I create one and restart the
> installer, then it is *not* mounted either, only /mnt/boot/efi later
> during the install.  (I believe since the installer is installed as on
> an external medium, it does not need an EFI partition.)
>
> I had booted the install image via UEFI boot and had expected auto
> partitioning and the default configuration to do the right thing.

The installer determines whether it’s doing a UEFI installation like so:

  (define (efi-installation?)
    "Return #t if an EFI installation should be performed, #f otherwise."
    (file-exists? "/sys/firmware/efi"))

It uses that to determine whether to create an EFI System Partition
(ESP) and whether to use ‘grub-efi-bootloader’.

Did it create an ESP in your case?

I’m not entirely sure how it decides between GPT and DOS, though;
Mathieu?

We should add UEFI installation tests using OVMF.

Thanks,
Ludo’.

bug#47889: [installer image] grub-install efi fails getting canonical path to /boot/efi on dos-formatted disk

[pelzflorian (Florian Pelz)]

On Fri, Apr 23, 2021 at 12:39:13PM +0200, Ludovic Courtès wrote:
> The installer determines whether it’s doing a UEFI installation like so:
> 
>   (define (efi-installation?)
>     "Return #t if an EFI installation should be performed, #f otherwise."
>     (file-exists? "/sys/firmware/efi"))
> 
> It uses that to determine whether to create an EFI System Partition
> (ESP) and whether to use ‘grub-efi-bootloader’.
> 
> Did it create an ESP in your case?

No, not on a DOS layout, only on GPT layout.  I think DOS should not
be allowed in auto partitioning (which would be the safer option) or
an ESP should be created on DOS as well.

Regards,
Florian

bug#47889: [installer image] grub-install efi fails getting canonical path to /boot/efi on dos-formatted disk

[Bengt Richter]

Hi Ludo, Florian,

On +2021-04-23 12:39:13 +0200, Ludovic Courtès wrote:
> Hi Florian,
> 
> (Cc: Mathieu.)
> 
> "pelzflorian (Florian Pelz)" <pelzflorian@pelzflorian.de> skribis:
> 
> > On Thu, Apr 22, 2021 at 03:28:39PM +0200, Ludovic Courtès wrote:
> >> There’s no EFI (vfat) partition here.  Is it an EFI machine?
> >> 
> >> Is /boot/efi mounted when you boot the installation image?
> >
> > No because there is no EFI partition.  If I create one and restart the
> > installer, then it is *not* mounted either, only /mnt/boot/efi later
> > during the install.  (I believe since the installer is installed as on
> > an external medium, it does not need an EFI partition.)
> >
> > I had booted the install image via UEFI boot and had expected auto
> > partitioning and the default configuration to do the right thing.
> 
> The installer determines whether it’s doing a UEFI installation like so:
> 
>   (define (efi-installation?)
>     "Return #t if an EFI installation should be performed, #f otherwise."
>     (file-exists? "/sys/firmware/efi"))
> 
> It uses that to determine whether to create an EFI System Partition
> (ESP) and whether to use ‘grub-efi-bootloader’.
>

How does that work if you want to mount an external USB disk as the target
of your installation partitioning and formatting etc, but which may be intended
for another laptop with a different BIOS booting in a different mode than your
installer was booted into? (Maybe plug the finished USB disk into another laptop?
USB C3.1 is fast enough if connected to a good SSD cassette).

I.e., suppose your installer machine was booted UEFI but you want the target disk
to be legacy MBR booted on a laptop that can only do that, loading grub2 as embedded
in the target disk? Or vice versa?

I'd like an interactive install, maybe selecting a target disk something like
--8<---------------cut here---------------start------------->8---
$ select choice in $(lsblk -o kname,model,serial|tr -s ' ' _); do break;done
1) KNAME_MODEL_SERIAL					   6) dm-0_
2) sdb_Ultra_Fit_XXXXXXXXXXXXXXXXXXXX			   7) nvme0n1_Samsung_SSD_970_EVO_Plus_500GB_XXXXXXXXXXXXXXX
3) sdb1_						   8) nvme0n1p1_
4) sdb2_						   9) nvme0n1p2_
5) sr0_USB_SCSI_CD-ROM_XXXXXXXXXXXXXXXX
#? 2
$ echo "$choice"
sdb_Ultra_Fit_XXXXXXXXXXXXXXXXXXXX
--8<---------------cut here---------------end--------------->8---

so then the installation script can continue and mount the associated disk device
--8<---------------cut here---------------start------------->8---
$ echo "$choice"|cut -d _ -f1 
sdb 
--8<---------------cut here---------------end--------------->8---

It seems like the /sys/... file system that would show whether the disk is EFI-bootable
will be determined by booting the very disk image we are trying to create -- both by its
content (MBR and/or GPT, and what bootloader + .cfg, etc) and the BIOS trying to boot it.

Sorry for the noise if I am missing some context.

> Did it create an ESP in your case?
> 
> I’m not entirely sure how it decides between GPT and DOS, though;
> Mathieu?
> 
> We should add UEFI installation tests using OVMF.
> 
> Thanks,
> Ludo’.
> 
> 
> 

-- 
Regards,
Bengt Richter

bug#47889: [installer image] grub-install efi fails getting canonical path to /boot/efi on dos-formatted disk

[pelzflorian (Florian Pelz)]

Hello Bengt,

On Sat, Apr 24, 2021 at 05:24:26AM +0200, Bengt Richter wrote:
> How does that work if you want to mount an external USB disk as the target
> of your installation partitioning and formatting etc, but which may be intended
> for another laptop with a different BIOS booting in a different mode than your
> installer was booted into? (Maybe plug the finished USB disk into another laptop?
> USB C3.1 is fast enough if connected to a good SSD cassette).

Do we want to support such rare use cases in the default
configuration?  Currently the configuration created by the installer
assumes you want to install to the same boot mode (UEFI or “legacy”
BIOS) as was selected when booting the installer.  This means that if
your computer only supports one or the other, you have no choice.  It
does not depend on what’s on the disk.

Also, in UEFI mode, the grub-efi-bootloader from
gnu/bootloader/grub.scm would need to be copied and adapted to add the
--removable option if you want to install to a removable USB in UEFI
mode.  This is because grub-efi-bootloader otherwise writes boot
information into your BIOS’s NVRAM on the mainboard of the computer
doing the install.

Regards,
Florian

bug#47889: [installer image] grub-install efi fails getting canonical path to /boot/efi on dos-formatted disk

[Mathieu Othacehe]

Hello,

> I’m not entirely sure how it decides between GPT and DOS, though;
> Mathieu?
>
> We should add UEFI installation tests using OVMF.

I could reproduce this issue with the following steps:

--8<---------------cut here---------------start------------->8---
qemu-img create -f qcow2 guix-system.img 50G
qemu-system-x86_64 -m 1024 -bios $(guix build ovmf)/share/firmware/ovmf_x64.bin -cdrom /gnu/store/<lastest-image.iso> -hda guix-system.img
Choose the msdos disk type in the auto-partitioning installer partitioning menu.
--8<---------------cut here---------------end--------------->8---

When using the auto-partitioning, the installer probes the selected
installation device type. If it has no type, like with my reproducer, it
asks whether to use msdos or gpt.

If the disk is already using an msdos or gpt layout, it is preserved.

Then, the auto-partitioning considers erroneously that ESP partitions
are not supported on msdos disks with the following code snippet:

--8<---------------cut here---------------start------------->8---
(and (not has-extended?) ;not msdos?
                 (if (efi-installation?)
                     (and (not esp-partition)
                          (user-partition
                           (fs-type 'fat32)
                           (esp? #t)
                           (size new-esp-size)
                           (mount-point (default-esp-mount-point))))
                     (user-partition
                      (fs-type 'ext4)
                      (bootable? #t)
                      (bios-grub? #t)
                      (size bios-grub-size))))
--8<---------------cut here---------------end--------------->8---

Finally, grub-efi fails because there's no /boot/efi mount point.

This problem can then occur for two reasons:

1. The user is booting the installation image with UEFI support, using
  an empty installation device, choosing auto-partition and msdos as
  disk type.

2. The user is booting the installation image with UEFI support, using
  an already msdos formatted installation device and choosing
  auto-partition.

I think we could solve 1. easily, by forcing the GPT layout. Solving
2. is a bit trickier.

As Ludo suggested, we also need to create new installer tests covering
UEFI installations.

I'll try to come up with a patch soon.

Thanks,

Mathieu

  

bug#47889: [installer image] grub-install efi fails getting canonical path to /boot/efi on dos-formatted disk

[pelzflorian (Florian Pelz)]

On Sun, Apr 25, 2021 at 04:15:56PM +0200, Mathieu Othacehe wrote:
> Finally, grub-efi fails because there's no /boot/efi mount point.
> 
> This problem can then occur for two reasons:
> 
> 1. The user is booting the installation image with UEFI support, using
>   an empty installation device, choosing auto-partition and msdos as
>   disk type.
> 
> 2. The user is booting the installation image with UEFI support, using
>   an already msdos formatted installation device and choosing
>   auto-partition.
> 
> I think we could solve 1. easily, by forcing the GPT layout. Solving
> 2. is a bit trickier.

Can you force GPT also in 2.?  All disk partitions get removed anyway.
Arch wiki says MSDOS layout does not work on all EFI systems
<https://wiki.archlinux.org/index.php/EFI_system_partition#Create_the_partition>.

Regards,
Florian

bug#47889: [installer image] grub-install efi fails getting canonical path to /boot/efi on dos-formatted disk

[Mathieu Othacehe]

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


Hello Florian,

> Can you force GPT also in 2.?  All disk partitions get removed anyway.
> Arch wiki says MSDOS layout does not work on all EFI systems
> <https://wiki.archlinux.org/index.php/EFI_system_partition#Create_the_partition>.

Well I'm not sure, if someone has an MSDOS partition table with an ESP
partition it wouldn't be a good idea to wipe it.

The attached patch forces GPT if there's no partition table, and will
preserve/create an ESP partition on an MSDOS/EFI setup. I'm testing it
on various machines, seems to work fine.

WDYT?

Thanks,

Mathieu

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-wip-esp.patch --]
[-- Type: text/x-patch, Size: 4386 bytes --]

From 65339286fe9d0c4758c04cfcaa177334577741cc Mon Sep 17 00:00:00 2001
From: Mathieu Othacehe <othacehe@gnu.org>
Date: Sun, 25 Apr 2021 19:06:31 +0200
Subject: [PATCH] wip esp

---
 gnu/installer/newt/partition.scm | 16 +++++++-----
 gnu/installer/parted.scm         | 45 ++++++++++++++------------------
 2 files changed, 28 insertions(+), 33 deletions(-)

diff --git a/gnu/installer/newt/partition.scm b/gnu/installer/newt/partition.scm
index 81cf68d782..2dcb10bbdb 100644
--- a/gnu/installer/newt/partition.scm
+++ b/gnu/installer/newt/partition.scm
@@ -95,14 +95,16 @@ DEVICES list."
 
 (define (run-label-page button-text button-callback)
   "Run a page asking the user to select a partition table label."
-  (run-listbox-selection-page
-   #:info-text (G_ "Select a new partition table type. \
+  (if (efi-installation?)
+      "gpt"
+      (run-listbox-selection-page
+       #:info-text (G_ "Select a new partition table type. \
 Be careful, all data on the disk will be lost.")
-   #:title (G_ "Partition table")
-   #:listbox-items '("msdos" "gpt")
-   #:listbox-item->text identity
-   #:button-text button-text
-   #:button-callback-procedure button-callback))
+       #:title (G_ "Partition table")
+       #:listbox-items '("msdos" "gpt")
+       #:listbox-item->text identity
+       #:button-text button-text
+       #:button-callback-procedure button-callback)))
 
 (define (run-type-page partition)
   "Run a page asking the user to select a partition type."
diff --git a/gnu/installer/parted.scm b/gnu/installer/parted.scm
index 9ef263d1f9..6d6e500d71 100644
--- a/gnu/installer/parted.scm
+++ b/gnu/installer/parted.scm
@@ -70,6 +70,7 @@
             small-freespace-partition?
             esp-partition?
             boot-partition?
+            efi-installation?
             default-esp-mount-point
 
             with-delay-device-in-use?
@@ -193,12 +194,8 @@ inferior to MAX-SIZE, #f otherwise."
 (define (esp-partition? partition)
   "Return #t if partition has the ESP flag, return #f otherwise."
   (let* ((disk (partition-disk partition))
-         (disk-type (disk-disk-type disk))
-         (has-extended? (disk-type-check-feature
-                         disk-type
-                         DISK-TYPE-FEATURE-EXTENDED)))
+         (disk-type (disk-disk-type disk)))
     (and (data-partition? partition)
-         (not has-extended?)
          (partition-is-flag-available? partition PARTITION-FLAG-ESP)
          (partition-get-flag partition PARTITION-FLAG-ESP))))
 
@@ -918,30 +915,26 @@ exists."
          ;; disk space. Otherwise, set the swap size to 5% of the disk space.
          (swap-size (min default-swap-size five-percent-disk)))
 
-    (if has-extended?
-        ;; msdos - remove everything.
-        (disk-remove-all-partitions disk)
-        ;; gpt - remove everything but esp if it exists.
-        (for-each
-         (lambda (partition)
-           (and (data-partition? partition)
-                (disk-remove-partition* disk partition)))
-         non-boot-partitions))
+    ;; Remove everything but esp if it exists.
+    (for-each
+     (lambda (partition)
+       (and (data-partition? partition)
+            (disk-remove-partition* disk partition)))
+     non-boot-partitions)
 
     (let* ((start-partition
-            (and (not has-extended?)
-                 (if (efi-installation?)
-                     (and (not esp-partition)
-                          (user-partition
-                           (fs-type 'fat32)
-                           (esp? #t)
-                           (size new-esp-size)
-                           (mount-point (default-esp-mount-point))))
+            (if (efi-installation?)
+                (and (not esp-partition)
                      (user-partition
-                      (fs-type 'ext4)
-                      (bootable? #t)
-                      (bios-grub? #t)
-                      (size bios-grub-size)))))
+                      (fs-type 'fat32)
+                      (esp? #t)
+                      (size new-esp-size)
+                      (mount-point (default-esp-mount-point))))
+                (user-partition
+                 (fs-type 'ext4)
+                 (bootable? #t)
+                 (bios-grub? #t)
+                 (size bios-grub-size))))
            (new-partitions
             (cond
              ((or (eq? scheme 'entire-root)
-- 
2.31.1

bug#47889: [installer image] grub-install efi fails getting canonical path to /boot/efi on dos-formatted disk

[pelzflorian (Florian Pelz)]

On Sun, Apr 25, 2021 at 07:12:39PM +0200, Mathieu Othacehe wrote:
> 
> Hello Florian,
> 
> > Can you force GPT also in 2.?  All disk partitions get removed anyway.
> > Arch wiki says MSDOS layout does not work on all EFI systems
> > <https://wiki.archlinux.org/index.php/EFI_system_partition#Create_the_partition>.
> 
> Well I'm not sure, if someone has an MSDOS partition table with an ESP
> partition it wouldn't be a good idea to wipe it.

You are right of course; the old ESP may contain boot information for
other disks or network boot or whatever.


> The attached patch forces GPT if there's no partition table, and will
> preserve/create an ESP partition on an MSDOS/EFI setup. I'm testing it
> on various machines, seems to work fine.

Thank you for this patch!!  It installs EFI on msdos layout
successfully on my Beebox PC.  That said, it could force GPT on msdos
layouts as well if they lack an ESP.  But I’m already happy.

Regards,
Florian

bug#47889: [installer image] grub-install efi fails getting canonical path to /boot/efi on dos-formatted disk

[Ludovic Courtès]

Hi Mathieu,

Mathieu Othacehe <othacehe@gnu.org> skribis:

>> Can you force GPT also in 2.?  All disk partitions get removed anyway.
>> Arch wiki says MSDOS layout does not work on all EFI systems
>> <https://wiki.archlinux.org/index.php/EFI_system_partition#Create_the_partition>.
>
> Well I'm not sure, if someone has an MSDOS partition table with an ESP
> partition it wouldn't be a good idea to wipe it.
>
> The attached patch forces GPT if there's no partition table, and will
> preserve/create an ESP partition on an MSDOS/EFI setup. I'm testing it
> on various machines, seems to work fine.

The patch looks reasonable to me, thanks a lot!

(It’s also reasonable to ignore ESP-on-MSDOS IMO.)

Ludo’.

bug#47889: [installer image] grub-install efi fails getting canonical path to /boot/efi on dos-formatted disk

[Mathieu Othacehe]

Hey Ludo & Florian,

> The patch looks reasonable to me, thanks a lot!

Thanks for having a look! I'm working on an UEFI installation test as we
discussed. Almost there but for a strange reason, even if the
installation works smoothly, the created image isn't bootable in QEMU.

I'll keep investigating.

Thanks,

Mathieu

bug#47889: [installer image] grub-install efi fails getting canonical path to /boot/efi on dos-formatted disk

[Mathieu Othacehe]

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


Hello,

Here is a patchset solving this issue and adding a new UEFI system
test. The boot issue was quite complex. Turns out it's related to how
the OVMF firmware saves persistently the UEFI variables.

Regarding the MSDOS/UEFI patch, it is almost identical to what Florian
tested. I chose not to force GPT on UEFI if the user disk is already
MBR formatted, mostly to keep the code readable.

Thanks,

Mathieu

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-installer-Force-GPT-disk-label-when-UEFI-is-supporte.patch --]
[-- Type: text/x-patch, Size: 1622 bytes --]

From c75278d39e977488516507fb8a67c167facda926 Mon Sep 17 00:00:00 2001
From: Mathieu Othacehe <othacehe@gnu.org>
Date: Tue, 27 Apr 2021 17:39:42 +0200
Subject: [PATCH 1/3] installer: Force GPT disk label when UEFI is supported.

* gnu/installer/newt/partition.scm (run-label-page): Force the GPT disk label
when UEFI is supported.
---
 gnu/installer/newt/partition.scm | 17 ++++++++++-------
 1 file changed, 10 insertions(+), 7 deletions(-)

diff --git a/gnu/installer/newt/partition.scm b/gnu/installer/newt/partition.scm
index 81cf68d782..2bb9b16945 100644
--- a/gnu/installer/newt/partition.scm
+++ b/gnu/installer/newt/partition.scm
@@ -95,14 +95,17 @@ DEVICES list."
 
 (define (run-label-page button-text button-callback)
   "Run a page asking the user to select a partition table label."
-  (run-listbox-selection-page
-   #:info-text (G_ "Select a new partition table type. \
+  ;; Force the GPT label if UEFI is supported.
+  (if (efi-installation?)
+      "gpt"
+      (run-listbox-selection-page
+       #:info-text (G_ "Select a new partition table type. \
 Be careful, all data on the disk will be lost.")
-   #:title (G_ "Partition table")
-   #:listbox-items '("msdos" "gpt")
-   #:listbox-item->text identity
-   #:button-text button-text
-   #:button-callback-procedure button-callback))
+       #:title (G_ "Partition table")
+       #:listbox-items '("msdos" "gpt")
+       #:listbox-item->text identity
+       #:button-text button-text
+       #:button-callback-procedure button-callback)))
 
 (define (run-type-page partition)
   "Run a page asking the user to select a partition type."
-- 
2.31.1


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #3: 0002-installer-Add-MSDOS-disk-label-support-on-UEFI-syste.patch --]
[-- Type: text/x-patch, Size: 3492 bytes --]

From b2ca61abc2f3faf918a9a4d006cb3d7cd2a71f19 Mon Sep 17 00:00:00 2001
From: Mathieu Othacehe <othacehe@gnu.org>
Date: Sun, 25 Apr 2021 19:06:31 +0200
Subject: [PATCH 2/3] installer: Add MSDOS disk label support on UEFI systems.

* gnu/installer/parted.scm (esp-partition?): Remove the MSDOS check.
(auto-partition!): On MSDOS disks, check if an ESP partition is present. If
that's the case, do not remove it. Otherwise, if UEFI is supported, create
one.
---
 gnu/installer/parted.scm | 45 +++++++++++++++++-----------------------
 1 file changed, 19 insertions(+), 26 deletions(-)

diff --git a/gnu/installer/parted.scm b/gnu/installer/parted.scm
index 9ef263d1f9..6d6e500d71 100644
--- a/gnu/installer/parted.scm
+++ b/gnu/installer/parted.scm
@@ -70,6 +70,7 @@
             small-freespace-partition?
             esp-partition?
             boot-partition?
+            efi-installation?
             default-esp-mount-point
 
             with-delay-device-in-use?
@@ -193,12 +194,8 @@ inferior to MAX-SIZE, #f otherwise."
 (define (esp-partition? partition)
   "Return #t if partition has the ESP flag, return #f otherwise."
   (let* ((disk (partition-disk partition))
-         (disk-type (disk-disk-type disk))
-         (has-extended? (disk-type-check-feature
-                         disk-type
-                         DISK-TYPE-FEATURE-EXTENDED)))
+         (disk-type (disk-disk-type disk)))
     (and (data-partition? partition)
-         (not has-extended?)
          (partition-is-flag-available? partition PARTITION-FLAG-ESP)
          (partition-get-flag partition PARTITION-FLAG-ESP))))
 
@@ -918,30 +915,26 @@ exists."
          ;; disk space. Otherwise, set the swap size to 5% of the disk space.
          (swap-size (min default-swap-size five-percent-disk)))
 
-    (if has-extended?
-        ;; msdos - remove everything.
-        (disk-remove-all-partitions disk)
-        ;; gpt - remove everything but esp if it exists.
-        (for-each
-         (lambda (partition)
-           (and (data-partition? partition)
-                (disk-remove-partition* disk partition)))
-         non-boot-partitions))
+    ;; Remove everything but esp if it exists.
+    (for-each
+     (lambda (partition)
+       (and (data-partition? partition)
+            (disk-remove-partition* disk partition)))
+     non-boot-partitions)
 
     (let* ((start-partition
-            (and (not has-extended?)
-                 (if (efi-installation?)
-                     (and (not esp-partition)
-                          (user-partition
-                           (fs-type 'fat32)
-                           (esp? #t)
-                           (size new-esp-size)
-                           (mount-point (default-esp-mount-point))))
+            (if (efi-installation?)
+                (and (not esp-partition)
                      (user-partition
-                      (fs-type 'ext4)
-                      (bootable? #t)
-                      (bios-grub? #t)
-                      (size bios-grub-size)))))
+                      (fs-type 'fat32)
+                      (esp? #t)
+                      (size new-esp-size)
+                      (mount-point (default-esp-mount-point))))
+                (user-partition
+                 (fs-type 'ext4)
+                 (bootable? #t)
+                 (bios-grub? #t)
+                 (size bios-grub-size))))
            (new-partitions
             (cond
              ((or (eq? scheme 'entire-root)
-- 
2.31.1


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #4: 0003-tests-Add-gui-uefi-installed-os-test.patch --]
[-- Type: text/x-patch, Size: 15618 bytes --]

From 7ae4219b78906f6be1764f487430680afe8ae8ee Mon Sep 17 00:00:00 2001
From: Mathieu Othacehe <othacehe@gnu.org>
Date: Tue, 27 Apr 2021 17:30:28 +0200
Subject: [PATCH 3/3] tests: Add gui-uefi-installed-os test.

* gnu/installer/tests.scm (conclude-installation): Rename it into ...
(start-installation): ... this new procedure.
(complete-installation): New procedure.
(choose-partitioning): Add an uefi-support? argument.
* gnu/tests/install.scm (uefi-firmware): New procedure.
(run-install, qemu-command/writable-image, gui-test-program,
installation-target-os-for-gui-tests): Add an uefi-support? argument.
(%extra-packages): Add grub-efi, fatfsck/static and dosfstools.
(%test-gui-installed-os): New variable.
---
 gnu/installer/tests.scm |  37 +++++++++++---
 gnu/tests/install.scm   | 108 ++++++++++++++++++++++++++++++++++++----
 2 files changed, 128 insertions(+), 17 deletions(-)

diff --git a/gnu/installer/tests.scm b/gnu/installer/tests.scm
index f318546a2f..8ccd327a7c 100644
--- a/gnu/installer/tests.scm
+++ b/gnu/installer/tests.scm
@@ -37,7 +37,8 @@
             enter-host-name+passwords
             choose-services
             choose-partitioning
-            conclude-installation
+            start-installation
+            complete-installation
 
             edit-configuration-file))
 
@@ -281,14 +282,19 @@ instrumented for further testing."
 (define* (choose-partitioning port
                               #:key
                               (encrypted? #t)
+                              (uefi-support? #f)
                               (passphrase "thepassphrase")
                               (edit-configuration-file
                                edit-configuration-file))
   "Converse over PORT to choose the partitioning method.  When ENCRYPTED? is
 true, choose full-disk encryption with PASSPHRASE as the LUKS passphrase.
+
+When UEFI-SUPPORT? is true, assume that we are running the installation tests
+on an UEFI capable machine.
+
 This conversation stops when the user partitions have been formatted, right
 before the installer generates the configuration file and shows it in a dialog
-box."
+box. "
   (converse port
     ((list-selection (title "Partitioning method")
                      (multiple-choices? #f)
@@ -306,11 +312,15 @@ box."
            disks))
 
     ;; The "Partition table" dialog pops up only if there's not already a
-    ;; partition table.
+    ;; partition table and if the system does not support UEFI.
     ((list-selection (title "Partition table")
                      (multiple-choices? #f)
                      (items _))
+     ;; When UEFI is supported, the partition is forced to GPT by the
+     ;; installer.
+     (not uefi-support?)
      "gpt")
+
     ((list-selection (title "Partition scheme")
                      (multiple-choices? #f)
                      (items (,one-partition _ ...)))
@@ -338,10 +348,10 @@ box."
      ;; UUIDs before it generates the configuration file.
      (values))))
 
-(define (conclude-installation port)
-  "Conclude the installation by checking over PORT that we get the generated
+(define (start-installation port)
+  "Start the installation by checking over PORT that we get the generated
 configuration file, accepting it and starting the installation, and then
-receiving the final messages once the 'guix system init' process has
+receiving the pause message once the 'guix system init' process has
 completed."
   ;; Assume the previous message received was 'starting-final-step'; here we
   ;; send the reply to that message, which lets the installer continue.
@@ -355,8 +365,19 @@ completed."
                   (file ,configuration-file))
      (edit-configuration-file configuration-file))
     ((pause)                                      ;"Press Enter to continue."
-     #t)
-    ((installation-complete)                      ;congratulations!
+     (values))))
+
+(define (complete-installation port)
+  "Complete the installation by replying to the installer pause message and
+waiting for the installation-complete message."
+  ;; Assume the previous message received was 'pause'; here we send the reply
+  ;; to that message, which lets the installer continue.
+  (write #t port)
+  (newline port)
+  (force-output port)
+
+  (converse port
+    ((installation-complete)
      (values))))
 
 ;;; Local Variables:
diff --git a/gnu/tests/install.scm b/gnu/tests/install.scm
index 4b8963eadd..b5263f5f0d 100644
--- a/gnu/tests/install.scm
+++ b/gnu/tests/install.scm
@@ -36,8 +36,10 @@
   #:use-module (gnu packages bootloaders)
   #:use-module (gnu packages commencement)       ;for 'guile-final'
   #:use-module (gnu packages cryptsetup)
+  #:use-module (gnu packages disk)
   #:use-module (gnu packages emacs)
   #:use-module (gnu packages emacs-xyz)
+  #:use-module (gnu packages firmware)
   #:use-module (gnu packages linux)
   #:use-module (gnu packages ocr)
   #:use-module (gnu packages openbox)
@@ -73,6 +75,7 @@
             %test-lvm-separate-home-os
 
             %test-gui-installed-os
+            %test-gui-uefi-installed-os
             %test-gui-installed-os-encrypted
             %test-gui-installed-desktop-os-encrypted))
 
@@ -206,6 +209,15 @@ guix system init /mnt/etc/config.scm /mnt --no-substitutes
 sync
 reboot\n")
 
+(define (uefi-firmware system)
+  "Return the appropriate QEMU OVMF UEFI firmware for the given SYSTEM."
+  (cond
+   ((string-prefix? "x86_64" system)
+    (file-append ovmf "/share/firmware/ovmf_x64.bin"))
+   ((string-prefix? "i686" system)
+    (file-append ovmf "/share/firmware/ovmf_ia32.bin"))
+   (else #f)))
+
 (define* (run-install target-os target-os-source
                       #:key
                       (script %simple-installation-script)
@@ -224,6 +236,7 @@ reboot\n")
                            #:imported-modules '((gnu services herd)
                                                 (gnu installer tests)
                                                 (guix combinators))))
+                      (uefi-support? #f)
                       (installation-image-type 'efi-raw)
                       (install-size 'guess)
                       (target-size (* 2200 MiB)))
@@ -235,6 +248,8 @@ packages defined in installation-os."
   (mlet* %store-monad ((_      (set-grafting #f))
                        (system (current-system))
 
+                       (uefi-firmware -> (and uefi-support?
+                                              (uefi-firmware system)))
                        ;; Since the installation system has no network access,
                        ;; we cheat a little bit by adding TARGET to its GC
                        ;; roots.  This way, we know 'guix system init' will
@@ -273,6 +288,9 @@ packages defined in installation-os."
                `(,(which #$(qemu-command system))
                  "-no-reboot"
                  "-m" "1200"
+                 ,@(if #$uefi-firmware
+                       '("-bios" #$uefi-firmware)
+                       '())
                  #$@(cond
                      ((eq? 'efi-raw installation-image-type)
                       #~("-drive"
@@ -322,10 +340,15 @@ packages defined in installation-os."
     (gexp->derivation "installation" install
                       #:substitutable? #f)))      ;too big
 
-(define* (qemu-command/writable-image image #:key (memory-size 256))
+(define* (qemu-command/writable-image image
+                                      #:key
+                                      (uefi-support? #f)
+                                      (memory-size 256))
   "Return as a monadic value the command to run QEMU on a writable copy of
 IMAGE, a disk image.  The QEMU VM has access to MEMORY-SIZE MiB of RAM."
-  (mlet %store-monad ((system (current-system)))
+  (mlet* %store-monad ((system (current-system))
+                       (uefi-firmware -> (and uefi-support?
+                                              (uefi-firmware system))))
     (return #~(let ((image #$image))
                 ;; First we need a writable copy of the image.
                 (format #t "creating writable image from '~a'...~%" image)
@@ -343,6 +366,9 @@ IMAGE, a disk image.  The QEMU VM has access to MEMORY-SIZE MiB of RAM."
                   ,@(if (file-exists? "/dev/kvm")
                         '("-enable-kvm")
                         '())
+                  ,@(if #$uefi-firmware
+                        '("-bios" #$uefi-firmware)
+                        '())
                   "-no-reboot" "-m" #$(number->string memory-size)
                   "-drive" "file=disk.img,if=virtio")))))
 
@@ -1400,7 +1426,9 @@ build (current-guix) and then store a couple of full system images.")
 (define* (gui-test-program marionette
                            #:key
                            (desktop? #f)
-                           (encrypted? #f))
+                           (encrypted? #f)
+                           (uefi-support? #f)
+                           (system (%current-system)))
   #~(let ()
       (define (screenshot file)
         (marionette-control (string-append "screendump " file)
@@ -1466,7 +1494,8 @@ build (current-guix) and then store a couple of full system images.")
 
       (marionette-eval* '(choose-partitioning installer-socket
                                               #:encrypted? #$encrypted?
-                                              #:passphrase #$%luks-passphrase)
+                                              #:passphrase #$%luks-passphrase
+                                              #:uefi-support? #$uefi-support?)
                         #$marionette)
       (screenshot "installer-run.ppm")
 
@@ -1480,9 +1509,43 @@ build (current-guix) and then store a couple of full system images.")
                                    "/dev/vda2")
                           #$marionette))
 
-      (marionette-eval* '(conclude-installation installer-socket)
+      (marionette-eval* '(start-installation installer-socket)
                         #$marionette)
 
+      ;; XXX: The grub-install process uses efibootmgr to add an UEFI Guix
+      ;; boot entry. The corresponding UEFI variable is stored in RAM, and
+      ;; possibly saved persistently on QEMU reboot in a NvVars file, see:
+      ;; https://lists.gnu.org/archive/html/qemu-discuss/2018-04/msg00045.html.
+      ;;
+      ;; As we are running QEMU with the no-reboot flag, this variable is
+      ;; never saved persistently, QEMU fails to boot the installed system and
+      ;; an UEFI shell is displayed instead.
+      ;;
+      ;; To make the installed UEFI system bootable, register Grub as the
+      ;; default UEFI boot entry, in the same way as if grub-install was
+      ;; invoked with the --removable option.
+      (when #$uefi-support?
+        (marionette-eval*
+         '(begin
+            (use-modules (ice-9 match))
+            (let ((targets (cond
+                            ((string-prefix? "x86_64" #$system)
+                             '("grubx64.efi" "BOOTX64.EFI"))
+                            ((string-prefix? "i686" #$system)
+                             '("grubia32.efi" "BOOTIA32.EFI"))
+                            (else #f))))
+              (match targets
+                ((src dest)
+                 (rename-file "/mnt/boot/efi/EFI/Guix"
+                              "/mnt/boot/efi/EFI/BOOT")
+                 (rename-file
+                  (string-append "/mnt/boot/efi/EFI/BOOT/" src)
+                  (string-append "/mnt/boot/efi/EFI/BOOT/" dest)))
+                (_ #f))))
+         #$marionette))
+
+      (marionette-eval* '(complete-installation installer-socket)
+                        #$marionette)
       (sync)
       #t))
 
@@ -1490,7 +1553,7 @@ build (current-guix) and then store a couple of full system images.")
   ;; Packages needed when installing with an encrypted root.
   (list isc-dhcp
         lvm2-static cryptsetup-static e2fsck/static
-        loadkeys-static))
+        loadkeys-static grub-efi fatfsck/static dosfstools))
 
 (define installation-os-for-gui-tests
   ;; Operating system that contains all of %EXTRA-PACKAGES, needed for the
@@ -1509,9 +1572,22 @@ build (current-guix) and then store a couple of full system images.")
                         (guix combinators))))
 
 (define* (installation-target-os-for-gui-tests
-          #:key (encrypted? #f))
+          #:key
+          (encrypted? #f)
+          (uefi-support? #f))
   (operating-system
     (inherit %minimal-os-on-vda)
+    (file-systems `(,(file-system
+                       (device (file-system-label "my-root"))
+                       (mount-point "/")
+                       (type "ext4"))
+                    ,@(if uefi-support?
+                          (list (file-system
+                                  (device (uuid "1234-ABCD" 'fat))
+                                  (mount-point "/boot/efi")
+                                  (type "vfat")))
+                          '())
+                    ,@%base-file-systems))
     (users (append (list (user-account
                           (name "alice")
                           (comment "Bob's sister")
@@ -1569,6 +1645,7 @@ build (current-guix) and then store a couple of full system images.")
                                    #:key
                                    (desktop? #f)
                                    (encrypted? #f)
+                                   (uefi-support? #f)
                                    target-os
                                    (install-size 'guess)
                                    (target-size (* 2200 MiB)))
@@ -1581,6 +1658,7 @@ build (current-guix) and then store a couple of full system images.")
         ((image   (run-install target-os '(this is unused)
                                #:script #f
                                #:os installation-os-for-gui-tests
+                               #:uefi-support? uefi-support?
                                #:install-size install-size
                                #:target-size target-size
                                #:installation-image-type
@@ -1590,8 +1668,11 @@ build (current-guix) and then store a couple of full system images.")
                                  (gui-test-program
                                   marionette
                                   #:desktop? desktop?
-                                  #:encrypted? encrypted?))))
-         (command (qemu-command/writable-image image #:memory-size 512)))
+                                  #:encrypted? encrypted?
+                                  #:uefi-support? uefi-support?))))
+         (command (qemu-command/writable-image image
+                                               #:uefi-support? uefi-support?
+                                               #:memory-size 512)))
       (run-basic-test target-os command name
                       #:initialization (and encrypted? enter-luks-passphrase)
                       #:root-password %root-password
@@ -1602,6 +1683,15 @@ build (current-guix) and then store a couple of full system images.")
    "gui-installed-os"
    #:target-os (installation-target-os-for-gui-tests)))
 
+;; Test the UEFI installation of Guix System using the graphical installer.
+(define %test-gui-uefi-installed-os
+  (guided-installation-test
+   "gui-uefi-installed-os"
+   #:uefi-support? #t
+   #:target-os (installation-target-os-for-gui-tests
+                #:uefi-support? #t)
+   #:target-size (* 3200 MiB)))
+
 (define %test-gui-installed-os-encrypted
   (guided-installation-test
    "gui-installed-os-encrypted"
-- 
2.31.1

bug#47889: [installer image] grub-install efi fails getting canonical path to /boot/efi on dos-formatted disk

[Mathieu Othacehe]

Hello,

> Regarding the MSDOS/UEFI patch, it is almost identical to what Florian
> tested. I chose not to force GPT on UEFI if the user disk is already
> MBR formatted, mostly to keep the code readable.

Pushed on master and cherry-picked on version-1.3.0 branch.

Thanks,

Mathieu

bug#47889: [installer image] grub-install efi fails getting canonical path to /boot/efi on dos-formatted disk

[Ludovic Courtès]

Hi,

Mathieu Othacehe <othacehe@gnu.org> skribis:

>> Regarding the MSDOS/UEFI patch, it is almost identical to what Florian
>> tested. I chose not to force GPT on UEFI if the user disk is already
>> MBR formatted, mostly to keep the code readable.
>
> Pushed on master and cherry-picked on version-1.3.0 branch.

Thanks!

Ludo’.