[Date Prev][Date Next] [Thread Prev][Thread Next] [Date Index] [Thread Index]

Bug#763127: UEFI corner case - installer booted in UEFI mode, existing system in BIOS mode - PLEASE REVIEW



Control: tag -1 +patch

On Wed, Oct 01, 2014 at 02:43:53AM +0100, Steve McIntyre wrote:
>On Mon, Sep 29, 2014 at 03:34:17PM +0100, Steve McIntyre wrote:
>>On Sun, Sep 28, 2014 at 08:46:41PM +0100, Steve McIntyre wrote:
>>>On Sun, Sep 28, 2014 at 07:50:38PM +0100, Colin Watson wrote:
>>>>On Sun, Sep 28, 2014 at 06:14:30PM +0100, Steve McIntyre wrote:
>>>>> That sounds better to me too, assuming we can sensibly do a question
>>>>> at that point. Is that allowed? I honestly don't know... :-/
>>>>
>>>>While isinstallable scripts can talk to debconf, they should not ask
>>>>questions, as main-menu will run the isinstallable script for a given
>>>>package potentially many times.  Also, partman-efi isn't a main-menu
>>>>item so any isinstallable script added to it will never be run.
>>>
>>>Right. Thanks for warning before we spend too much time on that
>>>option! :-/
>>>
>>>So, back to other options I guess.
>>
>>I'm hacking on init.d/efi, and it looks like it might do what we
>>need. We can remember a decision taken (choose not to do EFI
>>installation), but we'll need to update either all the places that
>>currently check if we're an EFI platform or the core
>>libdebian-installer code to look for a flag file or similar...
>
>I *think* I have a working solution here, testing now. \o/

Right, here's the changes I have. It works OK for me, tested on a few
different systems. The core of the logic is in
partman-efi/init.d/efi:

 * Check for EFI System Partitions on the target disks - if we have
   none but have other filesystems then that suggests there is a
   BIOS-mode OS already installed. If so, ask the user what they want
   to do.

 * Add a flag file /var/lib/partman/ignore_uefi that other bits of d-i
   can look for to know that although the installer is running in UEFI
   mode we should stick to installing for BIOS-mode boot instead.

Elsewhere in d-i, anywhere we try to work out what system we're on and
care about UEFI, I've added checks for the flag file
/var/lib/partman/ignore_uefi - if that exists, don't do UEFI
installation but instead fall through to BIOS-mode installation. I've
also added a similar check in archdetect in libdebian-installer - that
and the other changes are overkill, more like belt-and-braces. Which
way people would prefer to go, I don't know.

The message in partman-efi/init.d/efi is probably horrible - please
suggest better! It would also need translating, of course.

So, please look at the patches and tell me what you think folks. I'd
like to get this fix in for beta 3 if possible...

-- 
Steve McIntyre, Cambridge, UK.                                steve@einval.com
"This dress doesn't reverse." -- Alden Spiess
>From 046dcbe4f1c7880205c81eabed540407b515d68c Mon Sep 17 00:00:00 2001
From: Steve McIntyre <steve@einval.com>
Date: Thu, 2 Oct 2014 00:08:04 +0100
Subject: [PATCH] Better handle mixed UEFI and non-UEFI booting

Warn the user if we've booted in UEFI mode but we seem to have only
non-UEFI existing OS installations - give them the option to switch
the installer to non-UEFI mode from this point forwards so they don't
break potential dual-boot setup. Closes: #763127

Check for EFI System Partitions on the target disks - if we have none
but have other filesystems then that suggests there is a BIOS-mode OS
already installed. If so, ask the user what they want to do.

Add a flag file /var/lib/partman/ignore_uefi that other bits of d-i
can look for to know that although the installer is running in UEFI
mode we should stick to installing for BIOS-mode boot instead.
---
 check.d/efi                  |    4 ++++
 choose_method/efi/choices    |    4 ++++
 debian/changelog             |   10 ++++++++++
 debian/partman-efi.templates |   16 ++++++++++++++++
 debian/po/templates.pot      |   15 +++++++++++++++
 init.d/efi                   |   25 +++++++++++++++++++++++++
 update.d/efi_visuals         |    4 ++++
 7 files changed, 78 insertions(+)

diff --git a/check.d/efi b/check.d/efi
index 395688d..9d74bd3 100755
--- a/check.d/efi
+++ b/check.d/efi
@@ -4,6 +4,10 @@ if [ ! -d /proc/efi ] && [ ! -d /sys/firmware/efi ]; then
 	exit 0
 fi
 
+if [ -f /var/lib/partman/ignore_uefi ]; then
+	exit 0
+fi
+
 . /lib/partman/lib/base.sh
 
 have_efi=no
diff --git a/choose_method/efi/choices b/choose_method/efi/choices
index 160a565..b4af018 100755
--- a/choose_method/efi/choices
+++ b/choose_method/efi/choices
@@ -9,6 +9,10 @@ if [ ! -d /proc/efi ] && [ ! -d /sys/firmware/efi ]; then
 	exit 0
 fi
 
+if [ -f /var/lib/partman/ignore_uefi ]; then
+	exit 0
+fi
+
 db_metaget partman-efi/text/efi description
 
 printf "efi\t${RET}\n"
diff --git a/debian/changelog b/debian/changelog
index 383b5ae..2a96121 100644
--- a/debian/changelog
+++ b/debian/changelog
@@ -1,3 +1,13 @@
+partman-efi (49) unstable; urgency=medium
+
+  [ Steve McIntyre ]
+  * Warn the user if we've booted in UEFI mode but we seem to have only
+    non-UEFI existing OS installations - give them the option to switch
+    the installer to non-UEFI mode from this point forwards so they don't
+    break potential dual-boot setup. Closes: #763127
+
+ -- Steve McIntyre <93sam@debian.org>  Wed, 01 Oct 2014 01:07:34 +0100
+
 partman-efi (48) unstable; urgency=high
 
   [ Updated translations ]
diff --git a/debian/partman-efi.templates b/debian/partman-efi.templates
index 6fb2f7d..c5bc131 100644
--- a/debian/partman-efi.templates
+++ b/debian/partman-efi.templates
@@ -41,3 +41,19 @@ Type: error
 _Description: EFI partition too small
  EFI System Partitions on this architecture cannot be created with a size
  less than 35 MB. Please make the EFI System Partition larger.
+
+Template: partman-efi/non_efi_system
+Type: boolean
+# :sl5:
+_Description: Force EFI installation?
+ Your computer's firmware has started the installer in UEFI mode but
+ it looks like there may be existing operating systems already
+ installed on your computer using 'BIOS compatibility mode'. If you
+ continue to install Debian in UEFI mode, it might be difficult to
+ reboot your computer into any BIOS-mode operating systems later.
+ .
+ If you wish to install Debian in UEFI mode and don't care about
+ keeping the ability to boot one of the existing systems, you have the
+ option to force that here. If you wish to keep the option to boot an
+ existing operating system, you should choose NOT to force UEFI
+ installation here.
diff --git a/debian/po/templates.pot b/debian/po/templates.pot
index 1a1a857..0fbf475 100644
--- a/debian/po/templates.pot
+++ b/debian/po/templates.pot
@@ -85,3 +85,18 @@ msgid ""
 "EFI System Partitions on this architecture cannot be created with a size "
 "less than 35 MB. Please make the EFI System Partition larger."
 msgstr ""
+
+#. Type: boolean
+#. Description
+#. :sl5:
+#: ../partman-efi.templates:9001
+msgid "Force EFI installation?"
+msgstr ""
+
+#. Type: boolean
+#. Description
+#. :sl5:
+#: ../partman-efi.templates:9001
+msgid "EFI boot, but no ESP found and other filesystems in use"
+msgstr ""
+
diff --git a/init.d/efi b/init.d/efi
index 8955718..9418971 100755
--- a/init.d/efi
+++ b/init.d/efi
@@ -30,6 +30,9 @@ fi
 gpt_efi_type=c12a7328-f81f-11d2-ba4b-00a0c93ec93b
 msdos_efi_type=0xef
 
+NUM_ESP=0
+NUM_NO=0
+
 for dev in /var/lib/partman/devices/*; do
 	[ -d "$dev" ] || continue
 	cd $dev
@@ -52,6 +55,10 @@ for dev in /var/lib/partman/devices/*; do
 		elif [ "$label_type" = gpt ] && \
 		     [ "$(blkid -o value -s PART_ENTRY_TYPE -p "$path" 2>/dev/null)" = "$gpt_efi_type" ]; then
 			partitions="$partitions $id"
+		else
+			if [ "$fs" != "free" ]; then
+				NUM_NO=$(($NUM_NO + 1))
+			fi
 		fi
 	done
 	close_dialog
@@ -62,7 +69,10 @@ for dev in /var/lib/partman/devices/*; do
 	while { read_line flag; [ "$flag" ]; }; do
 		if [ "$flag" = boot ]; then
 			efi=yes
+			NUM_ESP=$(($NUM_ESP + 1))
 			# cannot break here
+		else
+			NUM_NO=$(($NUM_NO + 1))
 		fi
 	done
 	close_dialog
@@ -72,3 +82,18 @@ for dev in /var/lib/partman/devices/*; do
 	fi
 	done
 done
+
+log "Found $NUM_ESP ESPs, $NUM_NO non-ESPs"
+
+if [ $NUM_ESP = 0 ] && [ $NUM_NO -gt 0 ]; then
+	db_input critical partman-efi/non_efi_system || true
+	db_go || exit 1
+	db_fset partman-efi/non_efi_system seen true
+	db_get partman-efi/non_efi_system
+	if [ "$RET" = false ]; then
+		log "User chose to ignore UEFI"
+		touch /var/lib/partman/ignore_uefi
+	else
+		log "User chose to continue in UEFI mode"
+	fi
+fi
diff --git a/update.d/efi_visuals b/update.d/efi_visuals
index eed5412..ac47af5 100755
--- a/update.d/efi_visuals
+++ b/update.d/efi_visuals
@@ -6,6 +6,10 @@ if [ ! -d /proc/efi ] && [ ! -d /sys/firmware/efi ]; then
 	exit 0
 fi
 
+if [ -f /var/lib/partman/ignore_uefi ]; then
+	exit 0
+fi
+
 dev=$1
 num=$2
 id=$3
-- 
1.7.10.4

>From 3d16e8491d1ddc0a8903d173af989906de1b7d43 Mon Sep 17 00:00:00 2001
From: Steve McIntyre <steve@einval.com>
Date: Wed, 1 Oct 2014 23:58:10 +0100
Subject: [PATCH] Recognise the new ignore_uefi flag from partman-efi

---
 debian/changelog |    7 +++++++
 grub-installer   |    6 +++++-
 2 files changed, 12 insertions(+), 1 deletion(-)

diff --git a/debian/changelog b/debian/changelog
index 4baf893..ff8fa14 100644
--- a/debian/changelog
+++ b/debian/changelog
@@ -1,3 +1,10 @@
+grub-installer (1.99) unstable; urgency=medium
+
+  [ Steve McIntyre ]
+  * Recognise the new ignore_uefi flag from partman-efi.
+
+ -- Steve McIntyre <93sam@debian.org>  Wed, 01 Oct 2014 01:42:22 +0100
+
 grub-installer (1.98) unstable; urgency=medium
 
   [ Philip Hands ]
diff --git a/grub-installer b/grub-installer
index bff4174..ad5d688 100755
--- a/grub-installer
+++ b/grub-installer
@@ -329,7 +329,11 @@ case $ARCH in
 	fi
 	;;
     i386/efi|amd64/efi)
-	grub_package="grub-efi"
+	if [ -f /var/lib/partman/ignore_uefi ]; then
+		grub_package="grub-pc"
+	else
+		grub_package="grub-efi"
+	fi
 	;;
     i386/*|amd64/*)
 	grub_package="grub-pc"
-- 
1.7.10.4

>From 37230b9952c74eeb291bced87fc8157355eaf399 Mon Sep 17 00:00:00 2001
From: Steve McIntyre <steve@einval.com>
Date: Thu, 2 Oct 2014 00:01:28 +0100
Subject: [PATCH] Recognise the new ignore_uefi flag from partman-efi

---
 debian/changelog |    7 +++++++
 src/system/efi.c |    9 ++++++++-
 2 files changed, 15 insertions(+), 1 deletion(-)

diff --git a/debian/changelog b/debian/changelog
index d10bc4d..f637394 100644
--- a/debian/changelog
+++ b/debian/changelog
@@ -1,3 +1,10 @@
+libdebian-installer (0.98) unstable; urgency=low
+
+  [ Steve McIntyre ]
+  * Recognise the new ignore_uefi flag from partman-efi.
+
+ -- Steve McIntyre <93sam@debian.org>  Wed, 01 Oct 2014 01:42:22 +0100
+
 libdebian-installer (0.97) unstable; urgency=low
 
   [ Jérémy Bobbio ]
diff --git a/src/system/efi.c b/src/system/efi.c
index 41af731..a078425 100644
--- a/src/system/efi.c
+++ b/src/system/efi.c
@@ -28,7 +28,14 @@ int di_system_is_efi(void)
 {
 	int ret = access("/sys/firmware/efi", R_OK);
 	if (ret == 0)
-		return 1;
+	{
+		/* Have we been told to ignore EFI in partman-efi? */
+		ret = access("/var/lib/partman/ignore_uefi", R_OK);
+		if (ret == 0)
+			return 1;
+		else
+			return 1;
+	}
 	else
 		return 0;
 }
-- 
1.7.10.4

>From d4199b453834801538d0f41b5b18f0a71850425a Mon Sep 17 00:00:00 2001
From: Steve McIntyre <steve@einval.com>
Date: Thu, 2 Oct 2014 00:05:15 +0100
Subject: [PATCH] Recognise the new ignore_uefi flag from partman-efi

---
 debian/changelog                  |    7 +++++++
 os-probes/mounted/x86/05efi       |    2 +-
 os-probes/mounted/x86/20microsoft |    2 +-
 3 files changed, 9 insertions(+), 2 deletions(-)

diff --git a/debian/changelog b/debian/changelog
index 1499695..84a81e4 100644
--- a/debian/changelog
+++ b/debian/changelog
@@ -1,3 +1,10 @@
+os-prober (1.65) unstable; urgency=medium
+
+  [ Steve McIntyre ]
+  * Recognise the new ignore_uefi flag from partman-efi.
+
+ -- Steve McIntyre <93sam@debian.org>  Wed, 01 Oct 2014 01:42:22 +0100
+
 os-prober (1.64) unstable; urgency=medium
 
   [ Frederic Bonnard ]
diff --git a/os-probes/mounted/x86/05efi b/os-probes/mounted/x86/05efi
index e77ca6d..93309ce 100755
--- a/os-probes/mounted/x86/05efi
+++ b/os-probes/mounted/x86/05efi
@@ -8,7 +8,7 @@ mpoint="$2"
 type="$3"
 
 # This file is for UEFI platform only
-if [ ! -d /sys/firmware/efi ]; then
+if [ ! -d /sys/firmware/efi ] || [ -f /var/lib/partman/ignore_uefi ]; then
 	debug "Not on UEFI platform"
 	exit 1
 fi
diff --git a/os-probes/mounted/x86/20microsoft b/os-probes/mounted/x86/20microsoft
index bf829d9..6fb3cc5 100755
--- a/os-probes/mounted/x86/20microsoft
+++ b/os-probes/mounted/x86/20microsoft
@@ -8,7 +8,7 @@ mpoint="$2"
 type="$3"
 
 # This script looks for legacy BIOS bootloaders only. Skip if running UEFI
-if [ -d /sys/firmware/efi ]; then
+if [ -d /sys/firmware/efi ] && [ ! -f /var/lib/partman/ignore_uefi ]; then
 	debug "Skipping legacy bootloaders on UEFI system"
 	exit 1
 fi
-- 
1.7.10.4

>From e5870c91bdf678208a28806370af3b16a09e5e74 Mon Sep 17 00:00:00 2001
From: Steve McIntyre <steve@einval.com>
Date: Thu, 2 Oct 2014 00:03:25 +0100
Subject: [PATCH] Recognise the new ignore_uefi flag from partman-efi

---
 debian/changelog     |    7 +++++++
 debian/isinstallable |    2 +-
 2 files changed, 8 insertions(+), 1 deletion(-)

diff --git a/debian/changelog b/debian/changelog
index 61ac47e..615acee 100644
--- a/debian/changelog
+++ b/debian/changelog
@@ -1,3 +1,10 @@
+lilo-installer (1.47) unstable; urgency=low
+
+  [ Steve McIntyre ]
+  * Recognise the new ignore_uefi flag from partman-efi.
+
+ -- Steve McIntyre <93sam@debian.org>  Wed, 01 Oct 2014 01:42:22 +0100
+
 lilo-installer (1.46) unstable; urgency=low
 
   [ Updated translations ]
diff --git a/debian/isinstallable b/debian/isinstallable
index e3f9708..80a7939 100755
--- a/debian/isinstallable
+++ b/debian/isinstallable
@@ -11,7 +11,7 @@ case $ARCH in
 	# LILO stands a better chance of working in BIOS compatibility mode,
 	# where /sys/firmware/efi doesn't exist.
 	# Note: depends on partman-efi to load the efivars module!
-	if [ -d /sys/firmware/efi ]; then
+	if [ -d /sys/firmware/efi ] && [ ! -f /var/lib/partman/ignore_uefi ]; then
 		log "LILO not usable on EFI PCs without BIOS compatibility; use grub-efi"
 		exit 1
 	fi
-- 
1.7.10.4


Reply to: