From 1acba98f810a14b1255e34bc620594f83de37e36 Mon Sep 17 00:00:00 2001 From: Matthew Garrett Date: Sun, 2 Jun 2013 18:12:25 -0400 Subject: [PATCH 1/3] UEFI: Don't pass boot services regions to SetVirtualAddressMap() We need to map boot services regions during startup in order to avoid firmware bugs, but we shouldn't be passing those regions to SetVirtualAddressMap(). Ensure that we're only passing regions that are marked as being mapped at runtime. Signed-off-by: Matthew Garrett Signed-off-by: Matt Fleming --- arch/x86/platform/efi/efi.c | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/arch/x86/platform/efi/efi.c b/arch/x86/platform/efi/efi.c index 55856b2310d3..339e11f9b3a9 100644 --- a/arch/x86/platform/efi/efi.c +++ b/arch/x86/platform/efi/efi.c @@ -1026,6 +1026,13 @@ void __init efi_enter_virtual_mode(void) va = efi_ioremap(md->phys_addr, size, md->type, md->attribute); + if (!(md->attribute & EFI_MEMORY_RUNTIME)) { + if (!va) + pr_err("ioremap of 0x%llX failed!\n", + (unsigned long long)md->phys_addr); + continue; + } + md->virt_addr = (u64) (unsigned long) va; if (!va) { From 43ab0476a648053e5998bf081f47f215375a4502 Mon Sep 17 00:00:00 2001 From: Borislav Petkov Date: Sun, 2 Jun 2013 14:56:07 +0200 Subject: [PATCH 2/3] efi: Convert runtime services function ptrs ... to void * like the boot services and lose all the void * casts. No functionality change. Signed-off-by: Borislav Petkov Signed-off-by: Matt Fleming --- arch/x86/include/asm/efi.h | 28 ++++++++++++++-------------- include/linux/efi.h | 28 ++++++++++++++-------------- 2 files changed, 28 insertions(+), 28 deletions(-) diff --git a/arch/x86/include/asm/efi.h b/arch/x86/include/asm/efi.h index 2fb5d5884e23..5b33686b6995 100644 --- a/arch/x86/include/asm/efi.h +++ b/arch/x86/include/asm/efi.h @@ -52,40 +52,40 @@ extern u64 efi_call6(void *fp, u64 arg1, u64 arg2, u64 arg3, u64 arg4, u64 arg5, u64 arg6); #define efi_call_phys0(f) \ - efi_call0((void *)(f)) + efi_call0((f)) #define efi_call_phys1(f, a1) \ - efi_call1((void *)(f), (u64)(a1)) + efi_call1((f), (u64)(a1)) #define efi_call_phys2(f, a1, a2) \ - efi_call2((void *)(f), (u64)(a1), (u64)(a2)) + efi_call2((f), (u64)(a1), (u64)(a2)) #define efi_call_phys3(f, a1, a2, a3) \ - efi_call3((void *)(f), (u64)(a1), (u64)(a2), (u64)(a3)) + efi_call3((f), (u64)(a1), (u64)(a2), (u64)(a3)) #define efi_call_phys4(f, a1, a2, a3, a4) \ - efi_call4((void *)(f), (u64)(a1), (u64)(a2), (u64)(a3), \ + efi_call4((f), (u64)(a1), (u64)(a2), (u64)(a3), \ (u64)(a4)) #define efi_call_phys5(f, a1, a2, a3, a4, a5) \ - efi_call5((void *)(f), (u64)(a1), (u64)(a2), (u64)(a3), \ + efi_call5((f), (u64)(a1), (u64)(a2), (u64)(a3), \ (u64)(a4), (u64)(a5)) #define efi_call_phys6(f, a1, a2, a3, a4, a5, a6) \ - efi_call6((void *)(f), (u64)(a1), (u64)(a2), (u64)(a3), \ + efi_call6((f), (u64)(a1), (u64)(a2), (u64)(a3), \ (u64)(a4), (u64)(a5), (u64)(a6)) #define efi_call_virt0(f) \ - efi_call0((void *)(efi.systab->runtime->f)) + efi_call0((efi.systab->runtime->f)) #define efi_call_virt1(f, a1) \ - efi_call1((void *)(efi.systab->runtime->f), (u64)(a1)) + efi_call1((efi.systab->runtime->f), (u64)(a1)) #define efi_call_virt2(f, a1, a2) \ - efi_call2((void *)(efi.systab->runtime->f), (u64)(a1), (u64)(a2)) + efi_call2((efi.systab->runtime->f), (u64)(a1), (u64)(a2)) #define efi_call_virt3(f, a1, a2, a3) \ - efi_call3((void *)(efi.systab->runtime->f), (u64)(a1), (u64)(a2), \ + efi_call3((efi.systab->runtime->f), (u64)(a1), (u64)(a2), \ (u64)(a3)) #define efi_call_virt4(f, a1, a2, a3, a4) \ - efi_call4((void *)(efi.systab->runtime->f), (u64)(a1), (u64)(a2), \ + efi_call4((efi.systab->runtime->f), (u64)(a1), (u64)(a2), \ (u64)(a3), (u64)(a4)) #define efi_call_virt5(f, a1, a2, a3, a4, a5) \ - efi_call5((void *)(efi.systab->runtime->f), (u64)(a1), (u64)(a2), \ + efi_call5((efi.systab->runtime->f), (u64)(a1), (u64)(a2), \ (u64)(a3), (u64)(a4), (u64)(a5)) #define efi_call_virt6(f, a1, a2, a3, a4, a5, a6) \ - efi_call6((void *)(efi.systab->runtime->f), (u64)(a1), (u64)(a2), \ + efi_call6((efi.systab->runtime->f), (u64)(a1), (u64)(a2), \ (u64)(a3), (u64)(a4), (u64)(a5), (u64)(a6)) extern void __iomem *efi_ioremap(unsigned long addr, unsigned long size, diff --git a/include/linux/efi.h b/include/linux/efi.h index 2bc0ad78d058..21ae6b3c0359 100644 --- a/include/linux/efi.h +++ b/include/linux/efi.h @@ -287,20 +287,20 @@ typedef struct { typedef struct { efi_table_hdr_t hdr; - unsigned long get_time; - unsigned long set_time; - unsigned long get_wakeup_time; - unsigned long set_wakeup_time; - unsigned long set_virtual_address_map; - unsigned long convert_pointer; - unsigned long get_variable; - unsigned long get_next_variable; - unsigned long set_variable; - unsigned long get_next_high_mono_count; - unsigned long reset_system; - unsigned long update_capsule; - unsigned long query_capsule_caps; - unsigned long query_variable_info; + void *get_time; + void *set_time; + void *get_wakeup_time; + void *set_wakeup_time; + void *set_virtual_address_map; + void *convert_pointer; + void *get_variable; + void *get_next_variable; + void *set_variable; + void *get_next_high_mono_count; + void *reset_system; + void *update_capsule; + void *query_capsule_caps; + void *query_variable_info; } efi_runtime_services_t; typedef efi_status_t efi_get_time_t (efi_time_t *tm, efi_time_cap_t *tc); From d3768d885c6ccbf8a137276843177d76c49033a7 Mon Sep 17 00:00:00 2001 From: Zach Bobroff Date: Fri, 7 Jun 2013 13:02:50 +0100 Subject: [PATCH 3/3] x86, efi: retry ExitBootServices() on failure 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: again: 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 Cc: Signed-off-by: Matt Fleming --- 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 35ee62fccf98..7c6e5d957cc5 100644 --- a/arch/x86/boot/compressed/eboot.c +++ b/arch/x86/boot/compressed/eboot.c @@ -1037,18 +1037,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; again: - 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; +get_map: status = efi_call_phys5(sys_table->boottime->get_memory_map, &size, mem_map, &key, &desc_size, &desc_version); if (status == EFI_BUFFER_TOO_SMALL) { @@ -1074,8 +1076,20 @@ static efi_status_t exit_boot(struct boot_params *boot_params, /* 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;