efi: implement mandatory locking for UEFI Runtime Services
PATCH v4 1/1 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.
--- arch/x86/include/asm/efi.h | 2 + drivers/firmware/efi/runtime-wrappers.c | 154 +++++++++++++++++++++++++++++--- 2 files changed, 146 insertions(+), 10 deletions(-)
arch/x86/include/asm/efi.h +2 -0
@@ -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);
drivers/firmware/efi/runtime-wrappers.c +144 -10
248 lines not on this page · raw message
Signature · 1 line
1.8.3.2
PATCH v4 1/1 efi: implement mandatory locking for UEFI Runtime Services
Quoted 4 lines · Ard Biesheuvel
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.
I think we should include something that explains how we've managed to get away without an explicit runtime lock for so long,
We've managed to get away without requiring a runtime services lock until now because most of the interactions with EFI involve EFI variables, and those operations are already serialised with __efivars->lock.
and then a little bit of blurb about why that's no longer good enough.
Quoted 11 lines
Signed-off-by Ard Biesheuvel <[email protected]>--- arch/x86/include/asm/efi.h | 2 + drivers/firmware/efi/runtime-wrappers.c | 154 +++++++++++++++++++++++++++++--- 2 files changed, 146 insertions(+), 10 deletions(-)arch/x86/include/asm/efi.h +0
-0
arch/x86/include/asm/efi.h extern void __iomem *efi_ioremap(unsigned long addr, unsigned long size, · line 90
Quoted 5 lines
arch/x86/include/asm/efi.h +2
-0@@ -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() +
Remind me why this is needed for x86 but not for arm again?
The rest looks good. I'm just running it through some tests now.
Thanks Ard.
Signature · 1 line
Matt Fleming, Intel Open Source Technology Center
PATCH v4 1/1 efi: implement mandatory locking for UEFI Runtime Services
Quoted 6 lines · Matt Fleming
Quoted 4 lines · Ard Biesheuvel
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.
Quoted 8 lines
I think we should include something that explains how we've managed to get away without an explicit runtime lock for so long,
We've managed to get away without requiring a runtime services lock until now because most of the interactions with EFI involve EFI variables, and those operations are already serialised with
__efivars->lock.
Yes, that can go straight into the commit log imo
Quoted 2 lines
and then a little bit of blurb about why that's no longer good enough.
"""
Some of the assumptions underlying the decision whether locks are needed or not (e.g., SetVariable() against ResetSystem()) may not apply universally to all [new] architectures that implement UEFI. Rather than try to reason our way out of this, let's just implement at least what the spec requires in terms of locking.
"""
Quoted 12 lines
Quoted 12 lines
Signed-off-by Ard Biesheuvel <[email protected]>--- arch/x86/include/asm/efi.h | 2 + drivers/firmware/efi/runtime-wrappers.c | 154 +++++++++++++++++++++++++++++--- 2 files changed, 146 insertions(+), 10 deletions(-)arch/x86/include/asm/efi.h +0
-0@@ -86,6 +86,8 @@ extern void __iomem *efi_ioremap(unsigned long addr, unsigned long size,
Quoted 7 lines
arch/x86/include/asm/efi.h extern void __iomem *efi_ioremap(unsigned long addr, unsigned long size, · line 90
Quoted 4 lines
arch/x86/include/asm/efi.h +2
-0#endif /* CONFIG_X86_32 */ +#define efi_in_nmi () in_nmi() +Remind me why this is needed for x86 but not for arm again?
The spec specifically mentions "NMI on IA-32 and x64 systems, Machine Check and INIT on Itanium systems" as asynchronous events that require additional attention.
No such events exist on ARM or arm64 currently, so there is no need to implement it there.
Quoted 2 lines
The rest looks good. I'm just running it through some tests now.
Cheers,
Ard.
PATCH v4 1/1 efi: implement mandatory locking for UEFI Runtime Services
Quoted 8 lines · Ard Biesheuvel
"""
Some of the assumptions underlying the decision whether locks are needed or not (e.g.,SetVariable()againstResetSystem()) may not apply universally to all [new] architectures that implement UEFI. Rather than try to reason our way out of this, let's just implement at least what the spec requires in terms of locking.
"""
Thanks Ard, I've picked this commit up for the 'next' branch.
Signature · 1 line
Matt Fleming, Intel Open Source Technology Center
A message that is not in the archive (sent off-list or not yet received): 20140805141417.GQ15082-HNK1S37rvNbeXh+fF434Mdi2O/[email protected]
Quoted 10 lines · Matt Fleming
Quoted 8 lines · Ard Biesheuvel
"""
Some of the assumptions underlying the decision whether locks are needed or not (e.g.,SetVariable()againstResetSystem()) may not apply universally to all [new] architectures that implement UEFI. Rather than try to reason our way out of this, let's just implement at least what the spec requires in terms of locking.
"""
Quoted 2 lines
Thanks Ard, I've picked this commit up for the 'next' branch.
Did I miss a #define efi_in_nmi() for IA-64? The spec mentions Machine Check and INIT, but it is difficult for me to judge whether that is a case that would need to be addressed as well.
Signature · 1 line
Ard.