[3.8.y.z,extended,stable] Patch "x86, efi: retry ExitBootServices() on failure" has been added to staging queue

Message ID 1373387292-21194-1-git-send-email-luis.henriques@canonical.com
State New
Headers show

Commit Message

Luis Henriques July 9, 2013, 4:28 p.m.
This is a note to let you know that I have just added a patch titled

    x86, efi: retry ExitBootServices() on failure

to the linux-3.8.y-queue branch of the 3.8.y.z extended stable tree 
which can be found at:


If you, or anyone else, feels it should not be added to this tree, please 
reply to this email.

For more information about the 3.8.y.z tree, see



From 056248a3b3b5368b1ae130d8d6a243c1dabd2e17 Mon Sep 17 00:00:00 2001
From: Zach Bobroff <zacharyb@ami.com>
Date: Fri, 7 Jun 2013 13:02:50 +0100
Subject: [PATCH] x86, efi: retry ExitBootServices() on failure

commit d3768d885c6ccbf8a137276843177d76c49033a7 upstream.

ExitBootServices is absolutely supposed to return a failure if any
ExitBootServices event handler changes the memory map.  Basically the
get_map loop should run again if ExitBootServices returns an error the
first time.  I would say it would be fair that if ExitBootServices gives
an error the second time then Linux would be fine in returning control
back to BIOS.

The second change is the following line:

        size += sizeof(*mem_map) * 2;

Originally you were incrementing it by the size of one memory map entry.
The issue here is all related to the low_alloc routine you are using.
In this routine you are making allocations to get the memory map itself.
Doing this allocation or allocations can affect the memory map by more
than one record.

[ mfleming - changelog, code style ]
Signed-off-by: Zach Bobroff <zacharyb@ami.com>
Signed-off-by: Matt Fleming <matt.fleming@intel.com>
Signed-off-by: Luis Henriques <luis.henriques@canonical.com>
 arch/x86/boot/compressed/eboot.c | 20 +++++++++++++++++---
 1 file changed, 17 insertions(+), 3 deletions(-)



diff --git a/arch/x86/boot/compressed/eboot.c b/arch/x86/boot/compressed/eboot.c
index c205035..d606463 100644
--- a/arch/x86/boot/compressed/eboot.c
+++ b/arch/x86/boot/compressed/eboot.c
@@ -992,18 +992,20 @@  static efi_status_t exit_boot(struct boot_params *boot_params,
 	efi_memory_desc_t *mem_map;
 	efi_status_t status;
 	__u32 desc_version;
+	bool called_exit = false;
 	u8 nr_entries;
 	int i;

 	size = sizeof(*mem_map) * 32;

-	size += sizeof(*mem_map);
+	size += sizeof(*mem_map) * 2;
 	_size = size;
 	status = low_alloc(size, 1, (unsigned long *)&mem_map);
 	if (status != EFI_SUCCESS)
 		return status;

 	status = efi_call_phys5(sys_table->boottime->get_memory_map, &size,
 				mem_map, &key, &desc_size, &desc_version);
 	if (status == EFI_BUFFER_TOO_SMALL) {
@@ -1029,8 +1031,20 @@  again:
 	/* Might as well exit boot services now */
 	status = efi_call_phys2(sys_table->boottime->exit_boot_services,
 				handle, key);
-	if (status != EFI_SUCCESS)
-		goto free_mem_map;
+	if (status != EFI_SUCCESS) {
+		/*
+		 * ExitBootServices() will fail if any of the event
+		 * handlers change the memory map. In which case, we
+		 * must be prepared to retry, but only once so that
+		 * we're guaranteed to exit on repeated failures instead
+		 * of spinning forever.
+		 */
+		if (called_exit)
+			goto free_mem_map;
+		called_exit = true;
+		goto get_map;
+	}

 	/* Historic? */
 	boot_params->alt_mem_k = 32 * 1024;