[PATCH v3] efi: implement mandatory locking for UEFI Runtime Services
According to section 7.1 of the UEFI spec, Runtime Services are not fully reentrant, and there are particular combinations of calls that need to be serialized. Use a spinlock to serialize all Runtime Services with respect to all others, even if this is more than strictly needed. Signed-off-by: Ard Biesheuvel ard.biesheu...@linaro.org --- v3: Keep value of in_nmi() cached in local scope to help the compiler understand that it cannot change while the function is active v2: So this is v2 of the UEFI Runtime Services serialization patch: this time, I use a single spinlock rather than a set of mutexes, resulting in all services to be serialized with respect to all others. Also added handling of NMI state, as this results in some of the restrictions being lifted (x86, ia64 only) arch/x86/include/asm/efi.h | 2 + drivers/firmware/efi/runtime-wrappers.c | 175 +--- 2 files changed, 165 insertions(+), 12 deletions(-) diff --git a/arch/x86/include/asm/efi.h b/arch/x86/include/asm/efi.h index 1eb5f6433ad8..35f67a253e33 100644 --- a/arch/x86/include/asm/efi.h +++ b/arch/x86/include/asm/efi.h @@ -86,6 +86,8 @@ extern void __iomem *efi_ioremap(unsigned long addr, unsigned long size, #endif /* CONFIG_X86_32 */ +#define efi_in_nmi() in_nmi() + extern int add_efi_memmap; extern struct efi_scratch efi_scratch; extern void efi_set_executable(efi_memory_desc_t *md, bool executable); diff --git a/drivers/firmware/efi/runtime-wrappers.c b/drivers/firmware/efi/runtime-wrappers.c index 10daa4bbb258..55ac6d7eac1b 100644 --- a/drivers/firmware/efi/runtime-wrappers.c +++ b/drivers/firmware/efi/runtime-wrappers.c @@ -14,11 +14,76 @@ * This file is released under the GPLv2. */ +#include linux/bug.h #include linux/efi.h -#include linux/spinlock.h /* spinlock_t */ +#include linux/mutex.h +#include linux/spinlock.h #include asm/efi.h /* + * According to section 7.1 of the UEFI spec, Runtime Services are not fully + * reentrant, and there are particular combinations of calls that need to be + * serialized. + * + * Table 31. Rules for Reentry Into Runtime Services + * ++---+ + * | If previous call is busy in | Forbidden to call | + * ++---+ + * | Any | SetVirtualAddressMap()| + * ++---+ + * | ConvertPointer() | ConvertPointer() | + * ++---+ + * | SetVariable() | ResetSystem() | + * | UpdateCapsule() | | + * | SetTime() | | + * | SetWakeupTime() | | + * | GetNextHighMonotonicCount() | | + * ++---+ + * | GetVariable() | GetVariable() | + * | GetNextVariableName() | GetNextVariableName() | + * | SetVariable() | SetVariable() | + * | QueryVariableInfo() | QueryVariableInfo() | + * | UpdateCapsule() | UpdateCapsule() | + * | QueryCapsuleCapabilities()| QueryCapsuleCapabilities() | + * | GetNextHighMonotonicCount() | GetNextHighMonotonicCount() | + * ++---+ + * | GetTime() | GetTime() | + * | SetTime() | SetTime() | + * | GetWakeupTime() | GetWakeupTime() | + * | SetWakeupTime() | SetWakeupTime() | + * ++---+ + * + * Instead of adding locks for each of the groups, we add a single spinlock + * that serializes all runtime services calls with respect to all others. + */ +static DEFINE_SPINLOCK(efi_runtime_lock); + +/* + * Some runtime services calls can be reentrant under NMI, even if the table + * above says they are not. + * + * Table 32. Functions that may be called after Machine Check ,INIT and NMI + * ++--+ + * | Function | Called after Machine Check, INIT and NMI | + * ++--+ + * | GetTime() | Yes, even if previously busy.| + * | GetVariable() | Yes, even if previously busy | + * | GetNextVariableName() | Yes, even if previously busy | + * | QueryVariableInfo() | Yes, even if
Re: [PATCH] x86, eboot: Support initrd loaded above 4G
On Thu, 10 Jul, at 11:00:06AM, Yinghai Lu wrote: Oh, no. so efi could allocate buffer above 4g but can not access it? I'm not exactly sure what's wrong with the buffer - whether it's a case of not being able to access it properly or somehing buggy in the EFI code for reading files. No fault occurs when reading into it, it just doesn't contain the correct data. Either way, I'm going to leave your patch as-is and just ensure I fix this before the merge window. I think it's a good idea to have whatever workaround we come up with documented via an entirely separate patch. -- Matt Fleming, Intel Open Source Technology Center -- To unsubscribe from this list: send the line unsubscribe linux-efi in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3] efi: Request desired alignment via the PE/COFF headers
On Fri, 11 Jul, at 01:18:43AM, Michael Brown wrote: The ...headers now include... part was referring to the previously merged patch to add the .bss section. I haven't actually looked at the code which performs the alignment; I was going on hpa's concern that merely exposing init_size would be insufficient due to the potential for alignment. My understanding (possibly incorrect) was that the alignment was carried out using something simple along the lines of: new_kernel_start = align ( kernel_start, kernel_alignment ); memmove ( new_kernel_start, kernel_start, kernel_len ); i.e. that the memory used for alignment was not explicitly allocated. If the EFI boot stub instead allocates space for the aligned kernel using AllocatePages() (and allocates enough space for the whole of init_size), then the problem I described does not exist. Right, this shouldn't be a problem because we do in fact allocate space using the EFI boottime services in efi_relocate_kernel(), taking the alignment into account, and then perform the kernel image copy. I still think your change makes sense, I'm just inclined to delete the paragraph referring to the corruption bug (which we've established doesn't exist). -- Matt Fleming, Intel Open Source Technology Center -- To unsubscribe from this list: send the line unsubscribe linux-efi in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] x86/efi: autoload efivars
On 9 July 2014 08:40, joeyli j...@suse.com wrote:
On Tue, Jul 08, 2014 at 01:19:42PM +0100, Ben Hutchings wrote:
On Tue, 2014-07-08 at 11:14 +0100, Matt Fleming wrote:
On Tue, 08 Jul, at 11:00:58AM, Lee, Chun-Yi wrote:
[...]
--- a/arch/x86/platform/efi/efi.c
+++ b/arch/x86/platform/efi/efi.c
@@ -44,6 +44,7 @@
#include linux/io.h
#include linux/reboot.h
#include linux/bcd.h
+#include linux/platform_device.h
#include asm/setup.h
#include asm/efi.h
@@ -780,6 +781,20 @@ void __init efi_late_init(void)
efi_bgrt_init();
}
+#ifdef CONFIG_EFI_VARS_MODULE
+static int __init efi_load_efivars(void)
+{
+ struct platform_device *pdev;
+
+ if (!efi_enabled(EFI_RUNTIME_SERVICES))
+ return 0;
+
+ pdev = platform_device_register_simple(efivars, 0, NULL, 0);
+ return IS_ERR(pdev) ? PTR_ERR(pdev) : 0;
+}
+device_initcall(efi_load_efivars);
+#endif
+
Could this be moved to drivers/firmware/efi/efi.c? That way the arm64
folks could benefit from it too.
It seems like that should work now that efi_enabled() is not specific to
x86.
Ben.
Thanks for Matt and Ban's review, I will move platform device register code to
drivers/firmware/efi/efi.c and send version 2 patch.
Hi all,
I tested the version that is in Matt's -next now and it works fine on arm64.
--
Ard.
--
To unsubscribe from this list: send the line unsubscribe linux-efi in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] x86/efi: autoload efivars
On Fri, 11 Jul, at 09:47:23AM, Ard Biesheuvel wrote: Hi all, I tested the version that is in Matt's -next now and it works fine on arm64. Great, thanks Ard. May I add your Tested-by to the patch? -- Matt Fleming, Intel Open Source Technology Center -- To unsubscribe from this list: send the line unsubscribe linux-efi in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] x86/efi: autoload efivars
On 11 July 2014 09:52, Matt Fleming m...@console-pimps.org wrote: On Fri, 11 Jul, at 09:47:23AM, Ard Biesheuvel wrote: Hi all, I tested the version that is in Matt's -next now and it works fine on arm64. Great, thanks Ard. May I add your Tested-by to the patch? Sure, Tested-by: Ard Biesheuvel ard.biesheu...@linaro.org -- To unsubscribe from this list: send the line unsubscribe linux-efi in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3] efi: Request desired alignment via the PE/COFF headers
On 11/07/14 08:41, Matt Fleming wrote: i.e. that the memory used for alignment was not explicitly allocated. If the EFI boot stub instead allocates space for the aligned kernel using AllocatePages() (and allocates enough space for the whole of init_size), then the problem I described does not exist. Right, this shouldn't be a problem because we do in fact allocate space using the EFI boottime services in efi_relocate_kernel(), taking the alignment into account, and then perform the kernel image copy. I still think your change makes sense, I'm just inclined to delete the paragraph referring to the corruption bug (which we've established doesn't exist). Since the patch itself is so trivial, I'm happy for you to commit it as your own, with whatever commit message you think makes most sense. Michael -- To unsubscribe from this list: send the line unsubscribe linux-efi in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] efi: Include a .bss section within the PE/COFF headers
Am 10.07.2014 16:48, schrieb Matt Fleming: On Thu, 10 Jul, at 11:34:31AM, Matt Fleming wrote: Thomas, the patch in question is here this one, http://git.kernel.org/cgit/linux/kernel/git/mfleming/efi.git/commit/?h=urgentid=db0f1ff0ee1750cc52ead0ba1ddf95c47b3bd133 Correction, it's now this one, http://git.kernel.org/cgit/linux/kernel/git/mfleming/efi.git/commit/?h=urgentid=c7fb93ec51d462ec3540a729ba446663c26a0505 Hello Matt, thanks for forwarding the patch. We applied it to our linux 3.15 package, and at least one user still reported failure. Due to the random nature of the problem, we will need to wait for a few builds to see if the problem occurs less often. See here https://bugs.archlinux.org/task/33745#comment125251: Comment by Steven V (steabert) - Friday, 11 July 2014, 15:59 GMT+2 For me, 3.15.5-2 still doesn't work (Dell Latitude E4310). signature.asc Description: OpenPGP digital signature
Re: [PATCH] efi: Request desired alignment via the PE/COFF headers
On 07/09/2014 03:44 PM, Michael Brown wrote: Signed-off-by: Michael Brown mbr...@fensystems.co.uk --- arch/x86/boot/header.S | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/arch/x86/boot/header.S b/arch/x86/boot/header.S index 7a6d43a..16ef025 100644 --- a/arch/x86/boot/header.S +++ b/arch/x86/boot/header.S @@ -154,7 +154,7 @@ extra_header_fields: #else .quad 0 # ImageBase #endif - .long 0x20# SectionAlignment + .long CONFIG_PHYSICAL_ALIGN # SectionAlignment .long 0x20# FileAlignment .word 0 # MajorOperatingSystemVersion .word 0 # MinorOperatingSystemVersion Are large alignments (16 MiB!) actually allowed in PE/COFF? -hpa -- To unsubscribe from this list: send the line unsubscribe linux-efi in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
