efi: preserve NEON registers on UEFI services calls

PATCH Superseded Ard Biesheuvel 5 participants 16 messages Jun 26, 2014 – Jul 8, 2014

Subsystems (3)

x86 arm64 EFI

lore

Share & tools

lore

Ard Biesheuvel

author

2014-06-26 10:09 UTC #0 lore raw

v2 cover efi: preserve NEON registers on UEFI services calls

The current UEFI implementation for arm64 fails to preserve/restore the contents of the NEON register file, which may result in data corruption, especially now that those contents are lazily restored for user processes.

This series proposes to fix this by wrapping all runtime services calls, and adding kernel_neon_begin()/kernel_neon_end() pairs to the wrappers.

The first patch moves the existing x86 versions of those wrappers to generic code, so that the second patch can easily enable them by supplying a definition for efi_call_virt and adding a call to efi_native_runtime_setup().

Changes since v1:
- rename runtime.c -> runtime-wrappers.c
- make build depend on new Kconfig symbol EFI_RUNTIME_WRAPPERS to fix ia64 breakage
- remove default #defines for efi_call_virt()/__efi_call_virt(), they are not needed anymore now that it is built conditionally
- add references to applicable UEFI/AAPCS spec sections

Ard Biesheuvel (2):
efi/x86: move UEFI Runtime Services wrappers to generic code efi/arm64: preserve FP/SIMD registers on UEFI runtime services calls

 arch/arm64/Kconfig                      |   1 +
 arch/arm64/include/asm/efi.h            |  21 +++++
 arch/arm64/kernel/efi.c                 |  14 +--
 arch/x86/Kconfig                        |   1 +
 arch/x86/platform/efi/efi.c             | 144 +---------------------------
 drivers/firmware/efi/Kconfig            |   7 ++
 drivers/firmware/efi/Makefile           |   1 +
 drivers/firmware/efi/runtime-wrappers.c | 161 ++++++++++++++++++++++++++++++++
 include/linux/efi.h                     |   2 +
 9 files changed, 197 insertions(+), 155 deletions(-)
 create mode 100644 drivers/firmware/efi/runtime-wrappers.c

Signature · 1 line

1.8.3.2

Ard Biesheuvel

author

2014-06-26 10:09 UTC #1 lore raw

PATCH v2 1/2 efi/x86: move UEFI Runtime Services wrappers to generic code

In order for other archs (such as arm64) to be able to reuse the virtual mode function call wrappers, move them to drivers/firmware/efi/runtime-wrappers.c.

Signed-off-by Ard Biesheuvel <[email protected]>

---
 arch/x86/Kconfig                        |   1 +
 arch/x86/platform/efi/efi.c             | 144 +---------------------------
 drivers/firmware/efi/Kconfig            |   7 ++
 drivers/firmware/efi/Makefile           |   1 +
 drivers/firmware/efi/runtime-wrappers.c | 161 ++++++++++++++++++++++++++++++++
 include/linux/efi.h                     |   2 +
 6 files changed, 174 insertions(+), 142 deletions(-)
 create mode 100644 drivers/firmware/efi/runtime-wrappers.c

arch/x86/Kconfig +1 -0

index a8f749ef0fdc..4c3f026aa5ce 100644
@@ -1522,6 +1522,7 @@ config EFI
 	bool "EFI runtime service support"
 	depends on ACPI
 	select UCS2_STRING
+	select EFI_RUNTIME_WRAPPERS
 	---help---
 	  This enables the kernel to use EFI runtime services that are
 	  available (such as the EFI variable services).

arch/x86/platform/efi/efi.c +2 ~~-142~~

168 lines not on this page · raw message

drivers/firmware/efi/Kconfig +7 -0

index d420ae2d3413..04a7af46736a 100644
@@ -54,6 +54,13 @@ config EFI_PARAMS_FROM_FDT
 	  the EFI runtime support gets system table address, memory
           map address, and other parameters from the device tree.
 
+config EFI_RUNTIME_WRAPPERS
+	bool
+	help
+	  Selected by the arch if it needs to wrap UEFI Runtime Services calls,
+	  in which case it needs to provide #definitions of efi_call_virt and
+	  __efi_call_virt in <asm/efi.h>
+
 endmenu
 
 config UEFI_CPER

drivers/firmware/efi/Makefile +1 -0

index 9553496b0f43..e1096539eedb 100644
@@ -6,3 +6,4 @@ obj-$(CONFIG_EFI_VARS)			+= efivars.o
 obj-$(CONFIG_EFI_VARS_PSTORE)		+= efi-pstore.o
 obj-$(CONFIG_UEFI_CPER)			+= cper.o
 obj-$(CONFIG_EFI_RUNTIME_MAP)		+= runtime-map.o
+obj-$(CONFIG_EFI_RUNTIME_WRAPPERS)	+= runtime-wrappers.o

drivers/firmware/efi/runtime-wrappers.c new file +161 -0

161 lines not on this page · raw message

include/linux/efi.h +2 -0

index 41bbf8ba4ba8..0ceb816bdfc2 100644
@@ -521,6 +521,8 @@ typedef efi_status_t efi_query_capsule_caps_t(efi_capsule_header_t **capsules,
 					      int *reset_type);
 typedef efi_status_t efi_query_variable_store_t(u32 attributes, unsigned long size);
 
+void efi_native_runtime_setup(void);
+
 /*
  *  EFI Configuration Table and GUID definitions
  */

Signature · 1 line

1.8.3.2

Matt Fleming

2014-07-01 13:30 UTC #2 lore raw

PATCH v2 1/2 efi/x86: move UEFI Runtime Services wrappers to generic code

Quoted 4 lines · Ard Biesheuvel

In order for other archs (such as arm64) to be able to reuse the virtual mode function call wrappers, move them to drivers/firmware/efi/runtime-wrappers.c.
Signed-off-by Ard Biesheuvel <[email protected]>

[...]

drivers/firmware/efi/Kconfig config EFI_PARAMS_FROM_FDT · line 64

Quoted 12 lines

drivers/firmware/efi/Kconfig +7 -0

@@ -54,6 +54,13 @@ config EFI_PARAMS_FROM_FDT
 	  the EFI runtime support gets system table address, memory
           map address, and other parameters from the device tree.
 
+config EFI_RUNTIME_WRAPPERS
+	bool
+	help
+	  Selected by the arch if it needs to wrap UEFI Runtime Services calls,
+	  in which case it needs to provide #definitions of efi_call_virt and
+	  __efi_call_virt in <asm/efi.h>
+
 endmenu

Actually, could we drop this help text?

That may seem like a backwards step, but I have concerns that we'll fail to keep this help text in sync with the code. Furthermore, by providing help text that kinda says, "casual users need the help text to understand when to enable this feature". Clearly that's not what this Kconfig symbol is for.

Most of the other guard Kconfig symbols don't provide this kind of text, and I think there's good reason to follow suit.

What do you think?

Signature · 1 line

Matt Fleming, Intel Open Source Technology Center

Ard Biesheuvel

author

2014-07-01 13:35 UTC #3 lore raw

PATCH v2 1/2 efi/x86: move UEFI Runtime Services wrappers to generic code

Quoted 29 lines · Matt Fleming

Quoted 4 lines · Ard Biesheuvel
In order for other archs (such as arm64) to be able to reuse the virtual mode function call wrappers, move them to drivers/firmware/efi/runtime-wrappers.c.
Signed-off-by Ard Biesheuvel <[email protected]>
[...]
drivers/firmware/efi/Kconfig config EFI_PARAMS_FROM_FDT · line 64
Quoted 12 lines
drivers/firmware/efi/Kconfig +7 -0
@@ -54,6 +54,13 @@ config EFI_PARAMS_FROM_FDT
        the EFI runtime support gets system table address, memory
           map address, and other parameters from the device tree.
 
+config EFI_RUNTIME_WRAPPERS
+     bool
+     help
+       Selected by the arch if it needs to wrap UEFI Runtime Services calls,
+       in which case it needs to provide #definitions of efi_call_virt and
+       __efi_call_virt in <asm/efi.h>
+
 endmenu
Actually, could we drop this help text?
That may seem like a backwards step, but I have concerns that we'll fail to keep this help text in sync with the code. Furthermore, by providing help text that kinda says, "casual users need the help text to understand when to enable this feature". Clearly that's not what this Kconfig symbol is for.

Quoted 5 lines

Most of the other guard Kconfig symbols don't provide this kind of text, and I think there's good reason to follow suit.
What do you think?

I agree, but I kind of followed the example of the Kconfig symbols in the vicinity.
So sure, rip it out. Or would you like me to do a v(n+1)?

Signature · 1 line

Ard.

Matt Fleming

2014-07-01 13:48 UTC #4 lore raw

PATCH v2 1/2 efi/x86: move UEFI Runtime Services wrappers to generic code

Quoted 4 lines · Ard Biesheuvel

I agree, but I kind of followed the example of the Kconfig symbols in the vicinity.
So sure, rip it out. Or would you like me to do a v(n+1)?

Thanks. Nah, no need for another version, I'ved fixed this up in 'next', but please do check the commits to make sure nothing untoward has occurred.

Signature · 1 line

Matt Fleming, Intel Open Source Technology Center

Ard Biesheuvel

author

2014-06-26 10:09 UTC #5 lore raw

PATCH v2 2/2 efi/arm64: preserve FP/SIMD registers on UEFI runtime services calls

In reply to Ard Biesheuvel

According to the UEFI spec section 2.3.6.4, the use of FP/SIMD instructions is allowed, and should adhere to the AAPCS64 calling convention, which states that 'only the bottom 64 bits of each value stored in registers v8-v15 need to be preserved' (section 5.1.2).

This applies equally to UEFI Runtime Services called by the kernel, so make sure the FP/SIMD register file is preserved in this case.

Signed-off-by Ard Biesheuvel <[email protected]>

---
 arch/arm64/Kconfig           |  1 +
 arch/arm64/include/asm/efi.h | 21 +++++++++++++++++++++
 arch/arm64/kernel/efi.c      | 14 +-------------
 3 files changed, 23 insertions(+), 13 deletions(-)

arch/arm64/Kconfig +1 -0

index a474de346be6..93e11f4d9513 100644
@@ -299,6 +299,7 @@ config EFI
 	select LIBFDT
 	select UCS2_STRING
 	select EFI_PARAMS_FROM_FDT
+	select EFI_RUNTIME_WRAPPERS
 	default y
 	help
 	  This option provides support for runtime services provided

arch/arm64/include/asm/efi.h +21 -0

index 5a46c4e7f539..375ba342dca6 100644
@@ -2,6 +2,7 @@
 #define _ASM_EFI_H
 
 #include <asm/io.h>
+#include <asm/neon.h>
 
 #ifdef CONFIG_EFI
 extern void efi_init(void);

@@ -11,4 +12,24 @@ extern void efi_idmap_init(void);
 #define efi_idmap_init()
 #endif
 
+#define efi_call_virt(f, ...)						\
+({									\
+	efi_##f##_t *__f = efi.systab->runtime->f;			\
+	efi_status_t __s;						\
+									\
+	kernel_neon_begin();						\
+	__s = __f(__VA_ARGS__);						\
+	kernel_neon_end();						\
+	__s;								\
+})
+
+#define __efi_call_virt(f, ...)						\
+({									\
+	efi_##f##_t *__f = efi.systab->runtime->f;			\
+									\
+	kernel_neon_begin();						\
+	__f(__VA_ARGS__);						\
+	kernel_neon_end();						\
+})
+
 #endif /* _ASM_EFI_H */

arch/arm64/kernel/efi.c +1 ~~-13~~

index 14db1f6e8d7f..56c3327bbf79 100644
@@ -449,19 +449,7 @@ static int __init arm64_enter_virtual_mode(void)
 
 	/* Set up runtime services function pointers */
 	runtime = efi.systab->runtime;
-	efi.get_time = runtime->get_time;
-	efi.set_time = runtime->set_time;
-	efi.get_wakeup_time = runtime->get_wakeup_time;
-	efi.set_wakeup_time = runtime->set_wakeup_time;
-	efi.get_variable = runtime->get_variable;
-	efi.get_next_variable = runtime->get_next_variable;
-	efi.set_variable = runtime->set_variable;
-	efi.query_variable_info = runtime->query_variable_info;
-	efi.update_capsule = runtime->update_capsule;
-	efi.query_capsule_caps = runtime->query_capsule_caps;
-	efi.get_next_high_mono_count = runtime->get_next_high_mono_count;
-	efi.reset_system = runtime->reset_system;
-
+	efi_native_runtime_setup();
 	set_bit(EFI_RUNTIME_SERVICES, &efi.flags);
 
 	return 0;

Signature · 1 line

1.8.3.2

Catalin Marinas

maintainer

2014-07-04 15:45 UTC #6 lore raw

PATCH v2 2/2 efi/arm64: preserve FP/SIMD registers on UEFI runtime services calls

Quoted 9 lines · Ard Biesheuvel

According to the UEFI spec section 2.3.6.4, the use of FP/SIMD instructions is allowed, and should adhere to the AAPCS64 calling convention, which states that 'only the bottom 64 bits of each value stored in registers v8-v15 need to be preserved' (section 5.1.2).
This applies equally to UEFI Runtime Services called by the kernel, so make sure the FP/SIMD register file is preserved in this case.
Signed-off-by Ard Biesheuvel <[email protected]>

While the code looks fine, I think there is a mismatch between what the subject says and what the patch does (enabling EFI_RUNTIME_WRAPPERS).

Signature · 1 line

Catalin

Ard Biesheuvel

author

2014-07-04 15:51 UTC #7 lore raw

PATCH v2 2/2 efi/arm64: preserve FP/SIMD registers on UEFI runtime services calls

Quoted 6 lines · Catalin Marinas

Quoted 5 lines · Ard Biesheuvel
According to the UEFI spec section 2.3.6.4, the use of FP/SIMD instructions is allowed, and should adhere to the AAPCS64 calling convention, which states that 'only the bottom 64 bits of each value stored in registers v8-v15 need to be preserved' (section 5.1.2).

Quoted 8 lines

Quoted 4 lines
This applies equally to UEFI Runtime Services called by the kernel, so make sure the FP/SIMD register file is preserved in this case.
Signed-off-by Ard Biesheuvel <[email protected]>
While the code looks fine, I think there is a mismatch between what the subject says and what the patch does (enabling EFI_RUNTIME_WRAPPERS).

Not entirely. In order to be able to insert calls to kernel_neon_begin()/end() into the runtime services calls, we need
a) to supply definitions for efi_call_virt() and __efi_call_virt() that contain those calls to kernel_neon_begin()/end()
b) to enable runtime wrappers (which is what uses those definitions)

Would you prefer those to be split in 2 patches?

--
Ard.

Catalin Marinas

maintainer

2014-07-04 16:59 UTC #8 lore raw

PATCH v2 2/2 efi/arm64: preserve FP/SIMD registers on UEFI runtime services calls

Quoted 16 lines · Ard Biesheuvel

Quoted 6 lines · Catalin Marinas
Quoted 5 lines · Ard Biesheuvel
According to the UEFI spec section 2.3.6.4, the use of FP/SIMD instructions is allowed, and should adhere to the AAPCS64 calling convention, which states that 'only the bottom 64 bits of each value stored in registers v8-v15 need to be preserved' (section 5.1.2).
Quoted 8 lines
Quoted 4 lines
This applies equally to UEFI Runtime Services called by the kernel, so make sure the FP/SIMD register file is preserved in this case.
Signed-off-by Ard Biesheuvel <[email protected]>
While the code looks fine, I think there is a mismatch between what the subject says and what the patch does (enabling EFI_RUNTIME_WRAPPERS).

Quoted 7 lines

Not entirely. In order to be able to insert calls to kernel_neon_begin()/end() into the runtime services calls, we need
a) to supply definitions for efi_call_virt() and __efi_call_virt() that contain those calls to kernel_neon_begin()/end()
b) to enable runtime wrappers (which is what uses those definitions)
Would you prefer those to be split in 2 patches?

No, that's fine. You could just add the above explanation to the commit log. Otherwise:

Acked-by Catalin Marinas <[email protected]>

Ard Biesheuvel

author

2014-07-04 17:15 UTC #9 lore raw

PATCH v2 2/2 efi/arm64: preserve FP/SIMD registers on UEFI runtime services calls

Quoted 23 lines · Catalin Marinas

Quoted 22 lines · Ard Biesheuvel
Quoted 6 lines · Catalin Marinas
Quoted 5 lines · Ard Biesheuvel
According to the UEFI spec section 2.3.6.4, the use of FP/SIMD instructions is allowed, and should adhere to the AAPCS64 calling convention, which states that 'only the bottom 64 bits of each value stored in registers v8-v15 need to be preserved' (section 5.1.2).
Quoted 8 lines
Quoted 4 lines
This applies equally to UEFI Runtime Services called by the kernel, so make sure the FP/SIMD register file is preserved in this case.
Signed-off-by Ard Biesheuvel <[email protected]>
While the code looks fine, I think there is a mismatch between what the subject says and what the patch does (enabling EFI_RUNTIME_WRAPPERS).
Not entirely. In order to be able to insert calls to kernel_neon_begin()/end() into the runtime services calls, we need
a) to supply definitions for efi_call_virt() and __efi_call_virt() that contain those calls to kernel_neon_begin()/end()
b) to enable runtime wrappers (which is what uses those definitions)

Quoted 6 lines

Quoted 1 line
Would you prefer those to be split in 2 patches?
No, that's fine. You could just add the above explanation to the commit log. Otherwise:
Acked-by Catalin Marinas <[email protected]>

OK, thanks. I will add your ack and ask Matt to take it.

Signature · 1 line

Ard.

Mark Salter

2014-06-26 13:58 UTC #10 lore raw

v2 cover efi: preserve NEON registers on UEFI services calls

In reply to Ard Biesheuvel

Quoted 2 lines · Ard Biesheuvel

The current UEFI implementation for arm64 fails to preserve/restore the contents of the NEON register file,

Does the current implementation actually use NEON registers? I know there are some at least, which build with -mgeneral-regs-only to keep gcc from using NEON registers.

Not that that means we don't need this. Just curious.

Ard Biesheuvel

author

2014-06-26 14:22 UTC #11 lore raw

v2 cover efi: preserve NEON registers on UEFI services calls

Quoted 8 lines · Mark Salter

Quoted 2 lines · Ard Biesheuvel
The current UEFI implementation for arm64 fails to preserve/restore the contents of the NEON register file,
Does the current implementation actually use NEON registers? I know there are some at least, which build with -mgeneral-regs-only to keep gcc from using NEON registers.

Tianocore/EDK2 does not use -mgeneral-regs-only when building for AArch64, and my current build shows that, for instance, GCC starts spilling to FP registers when compiling the LZMA decoder, and it is likely to do the same for the crypto bits once I start enabling them (although, interestingly enough, the OpenSSL SHA-1 C implementation runs 60% faster with -mgeneral-regs-only set)

Quoted 2 lines

Not that that means we don't need this. Just curious.

The spec does not forbid it, so we will need it anyhow ...

Signature · 1 line

Ard.

Matt Fleming

2014-07-01 13:26 UTC #12 lore raw

v2 cover efi: preserve NEON registers on UEFI services calls

In reply to Ard Biesheuvel

Quoted 19 lines · Ard Biesheuvel

The current UEFI implementation for arm64 fails to preserve/restore the contents of the NEON register file, which may result in data corruption, especially now that those contents are lazily restored for user processes.
This series proposes to fix this by wrapping all runtime services calls, and adding kernel_neon_begin()/kernel_neon_end() pairs to the wrappers.
The first patch moves the existing x86 versions of those wrappers to generic code, so that the second patch can easily enable them by supplying a definition for efi_call_virt and adding a call to efi_native_runtime_setup().
Changes since v1:
- rename runtime.c -> runtime-wrappers.c
- make build depend on new Kconfig symbol EFI_RUNTIME_WRAPPERS to fix ia64 breakage
- remove default #defines for efi_call_virt()/__efi_call_virt(), they are not needed anymore now that it is built conditionally
- add references to applicable UEFI/AAPCS spec sections

Quoted 3 lines

Ard Biesheuvel (2):
efi/x86: move UEFI Runtime Services wrappers to generic code efi/arm64: preserve FP/SIMD registers on UEFI runtime services calls

Thanks Ard. These patches look OK to me, and I've now pulled them into my 'next' branch.

It'd be nice if we could get some ACKs from other people since this is an ABI issue.

Signature · 1 line

Matt Fleming, Intel Open Source Technology Center

Ard Biesheuvel

author

2014-07-01 13:46 UTC #13 lore raw

v2 cover efi: preserve NEON registers on UEFI services calls

Quoted 24 lines · Matt Fleming

Quoted 19 lines · Ard Biesheuvel
The current UEFI implementation for arm64 fails to preserve/restore the contents of the NEON register file, which may result in data corruption, especially now that those contents are lazily restored for user processes.
This series proposes to fix this by wrapping all runtime services calls, and adding kernel_neon_begin()/kernel_neon_end() pairs to the wrappers.
The first patch moves the existing x86 versions of those wrappers to generic code, so that the second patch can easily enable them by supplying a definition for efi_call_virt and adding a call to efi_native_runtime_setup().
Changes since v1:
- rename runtime.c -> runtime-wrappers.c
- make build depend on new Kconfig symbol EFI_RUNTIME_WRAPPERS to fix ia64 breakage
- remove default #defines for efi_call_virt()/__efi_call_virt(), they are not needed anymore now that it is built conditionally
- add references to applicable UEFI/AAPCS spec sections
Quoted 3 lines
Ard Biesheuvel (2):
efi/x86: move UEFI Runtime Services wrappers to generic code efi/arm64: preserve FP/SIMD registers on UEFI runtime services calls

Quoted 5 lines

Thanks Ard. These patches look OK to me, and I've now pulled them into my 'next' branch.
It'd be nice if we could get some ACKs from other people since this is an ABI issue.

Yes, they have been awfully quiet, haven't they?

At least Roy has volunteered to get involved in the USWG to get the spec clarified.

@Leif, Catalin, Olivier: care to chime in?

Signature · 1 line

Ard.

Olivier Martin

2014-07-08 15:33 UTC #14 lore raw

v2 cover efi: preserve NEON registers on UEFI services calls

It is correct the UEFI 2.4 spec says 'Floating point and SIMD instructions may be used'. And it is also correct these registers are not preserved in the current UEFI EDK2 implementation. I will keep track in ARM UEFI todo list.

Acked-by Olivier Martin <[email protected]>

Thanks Ard for your patch,
Olivier

Quoted 61 lines

-----Original Message-----
From: Ard Biesheuvel [mailto:ard.biesheuvel at linaro.org]
Sent: 01 July 2014 14:46
To: Matt Fleming
Cc: Matt Fleming; x86 at kernel.org; Catalin Marinas; linux- efi at vger.kernel.org; linux-arm-kernel at lists.infradead.org; hpa at zytor.com; Leif Lindholm; Roy Franz; msalter at redhat.com; Olivier Martin
Subject: Re: [PATCH v2 0/2] efi: preserve NEON registers on UEFI services calls
Quoted 2 lines · Matt Fleming
Quoted 1 line · Ard Biesheuvel
The current UEFI implementation for arm64 fails to preserve/restore
the contents
Quoted 1 line
Quoted 1 line
of the NEON register file, which may result in data corruption,
especially now
Quoted 3 lines
Quoted 3 lines
that those contents are lazily restored for user processes.
This series proposes to fix this by wrapping all runtime services
calls, and
Quoted 3 lines
Quoted 3 lines
adding kernel_neon_begin()/kernel_neon_end() pairs to the wrappers.
The first patch moves the existing x86 versions of those wrappers to
generic
Quoted 1 line
Quoted 1 line
code, so that the second patch can easily enable them by supplying a
definition
Quoted 5 lines
Quoted 5 lines
for efi_call_virt and adding a call to efi_native_runtime_setup().
Changes since v1:
- rename runtime.c -> runtime-wrappers.c
- make build depend on new Kconfig symbol EFI_RUNTIME_WRAPPERS to
fix ia64
Quoted 2 lines
Quoted 2 lines
breakage
- remove default #defines for efi_call_virt()/__efi_call_virt(),
they are not
Quoted 6 lines
Quoted 6 lines
needed anymore now that it is built conditionally
- add references to applicable UEFI/AAPCS spec sections
Ard Biesheuvel (2):
efi/x86: move UEFI Runtime Services wrappers to generic code efi/arm64: preserve FP/SIMD registers on UEFI runtime services
calls
Quoted 2 lines
Thanks Ard. These patches look OK to me, and I've now pulled them
into
Quoted 3 lines
my 'next' branch.
It'd be nice if we could get some ACKs from other people since this
is
Quoted 1 line
an ABI issue.
Yes, they have been awfully quiet, haven't they?
At least Roy has volunteered to get involved in the USWG to get the spec clarified.
@Leif, Catalin, Olivier: care to chime in?
--
Ard.

Olivier Martin

Acked-by on the v2 series

2014-07-08 15:33 UTC #15 lore raw

In reply to Ard Biesheuvel

Acked-by Olivier Martin <[email protected]>

Thanks Ard for your patch,
Olivier

Quoted 61 lines

-----Original Message-----
From: Ard Biesheuvel [mailto:[email protected]]
Sent: 01 July 2014 14:46
To: Matt Fleming
Cc: Matt Fleming; [email protected]; Catalin Marinas; linux- [email protected]; [email protected]; [email protected]; Leif Lindholm; Roy Franz; [email protected]; Olivier Martin
Subject: Re: [PATCH v2 0/2] efi: preserve NEON registers on UEFI services calls
Quoted 2 lines · Matt Fleming
Quoted 1 line · Ard Biesheuvel
The current UEFI implementation for arm64 fails to preserve/restore
the contents
Quoted 1 line
Quoted 1 line
of the NEON register file, which may result in data corruption,
especially now
Quoted 3 lines
Quoted 3 lines
that those contents are lazily restored for user processes.
This series proposes to fix this by wrapping all runtime services
calls, and
Quoted 3 lines
Quoted 3 lines
adding kernel_neon_begin()/kernel_neon_end() pairs to the wrappers.
The first patch moves the existing x86 versions of those wrappers to
generic
Quoted 1 line
Quoted 1 line
code, so that the second patch can easily enable them by supplying a
definition
Quoted 5 lines
Quoted 5 lines
for efi_call_virt and adding a call to efi_native_runtime_setup().
Changes since v1:
- rename runtime.c -> runtime-wrappers.c
- make build depend on new Kconfig symbol EFI_RUNTIME_WRAPPERS to
fix ia64
Quoted 2 lines
Quoted 2 lines
breakage
- remove default #defines for efi_call_virt()/__efi_call_virt(),
they are not
Quoted 6 lines
Quoted 6 lines
needed anymore now that it is built conditionally
- add references to applicable UEFI/AAPCS spec sections
Ard Biesheuvel (2):
efi/x86: move UEFI Runtime Services wrappers to generic code efi/arm64: preserve FP/SIMD registers on UEFI runtime services
calls
Quoted 2 lines
Thanks Ard. These patches look OK to me, and I've now pulled them
into
Quoted 3 lines
my 'next' branch.
It'd be nice if we could get some ACKs from other people since this
is
Quoted 1 line
an ABI issue.
Yes, they have been awfully quiet, haven't they?
At least Roy has volunteered to get involved in the USWG to get the spec clarified.
@Leif, Catalin, Olivier: care to chime in?
--
Ard.