[PATCH v2] 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. Signed-off-by: Ard Biesheuvel ard.biesheu...@linaro.org --- 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) One question remains: with the NMI deadlock handling in place, is it really necessary to disable interrupts in all cases? drivers/firmware/efi/runtime-wrappers.c | 167 +--- 1 file changed, 155 insertions(+), 12 deletions(-) diff --git a/drivers/firmware/efi/runtime-wrappers.c b/drivers/firmware/efi/runtime-wrappers.c index 10daa4bbb258..5ee3b16498a2 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 previously busy | + * | SetVariable() | Yes, even if previously busy | + * | UpdateCapsule() | Yes, even if previously busy | + * | QueryCapsuleCapabilities()| Yes, even if previously busy | + * | ResetSystem() | Yes, even if previously busy | + * ++--+ + * + * In order to prevent deadlocks under NMI, the wrappers for these functions + * only grab the efi_runtime_lock or rtc_lock spinlocks if !efi_in_nmi(). + */ +#ifndef efi_in_nmi +#define efi_in_nmi() (0) +#endif + +/* * As
Re: [PATCH v2] efi: implement mandatory locking for UEFI Runtime Services
On 8 July 2014 13:21, Ard Biesheuvel ard.biesheu...@linaro.org wrote: 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. Signed-off-by: Ard Biesheuvel ard.biesheu...@linaro.org --- 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) One question remains: with the NMI deadlock handling in place, is it really necessary to disable interrupts in all cases? I omitted this hunk from the patch by accident: diff --git a/arch/x86/include/asm/efi.h b/arch/x86/include/asm/efi.h index 55059a50a01f..d8ee5bb527e5 100644 --- a/arch/x86/include/asm/efi.h +++ b/arch/x86/include/asm/efi.h @@ -2,6 +2,7 @@ #define _ASM_X86_EFI_H #include asm/i387.h +#include asm/nmi.h /* * We map the EFI regions needed for runtime services non-contiguously, * with preserved alignment on virtual addresses starting from -4G down @@ -52,6 +53,8 @@ extern unsigned long asmlinkage efi_call_phys(void *, ...); kernel_fpu_end(); \ }) +#define efi_in_nmi() in_nmi() + #define efi_ioremap(addr, size, type, attr)ioremap_cache(addr, size) #else /* !CONFIG_X86_32 */ drivers/firmware/efi/runtime-wrappers.c | 167 +--- 1 file changed, 155 insertions(+), 12 deletions(-) diff --git a/drivers/firmware/efi/runtime-wrappers.c b/drivers/firmware/efi/runtime-wrappers.c index 10daa4bbb258..5ee3b16498a2 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()
