On 14/12/2018 15:49, Ard Biesheuvel wrote:

On Fri, 14 Dec 2018 at 16:23, Julien Thierry [off-list ref] wrote:

quoted

Hi,

On 13/12/2018 15:03, Julien Thierry wrote:

quoted

Argh, not as simple as I had expected.

Turns out include/linux/efi.h does not include asm/efi.h (including it
at the beginning of the file breaks the build because asm/efi.h misses
the efi type definitions.

So a thing like:

#ifndef efi_get_irqflags
#define efi_get_irqflags(flags) local_save_flags(flags)
#endif

in include/linux/efi.h cannot be overridden.

Either I would need to introduce the definitions arm, arm64 and x86 (I
don't think there are other arch supporting EFI right now) or I'll need
to come up with another solution.

It might be a bit nasty, but can we put the #ifndef above in
runtime-wrappers.c directly? The only reference in linux/efi.h is a
macro, so that shouldn't matter afaict.

Sadly, in arch/x86/platform/uv/bios_uv.c, uv_bios_call() has a reference
to the macro efi_call_virt_pointer() which wouldn't be able to see the
definition in runtime-wrappers.c

Otherwise, we could've moved efi_call_virt_pointer() and
__efi_call_virt_pointer in runtime-wrappers.c and things would not have
been as nasty.

But no, I don't think we can do that without breaking some x86 build :( .

Thanks,

quoted

Would the following patch be acceptable for the EFI generic side?

If it is, I'll add it to the next iteration of this series.

Thanks,

Julien

-->

From 7acaa8e17142263addafb18ae10bd5d2d49cfb39 Mon Sep 17 00:00:00 2001
From: Julien Thierry <redacted>
Date: Fri, 14 Dec 2018 14:20:13 +0000
Subject: [RFC] efi: Let architectures decide the flags that should be
 saved/restored

Currently, irqflags are saved before calling runtime services and
checked for mismatch on return.

Add a config option to let architectures define a set of flags to be
checked and (if needed) restored when coming back from runtime services.
This allows to use check flags that are not necesarly related to
irqflags.

Signed-off-by: Julien Thierry <redacted>
---
 arch/Kconfig                            |  8 ++++++++
 drivers/firmware/efi/runtime-wrappers.c |  4 ++--
 include/linux/efi.h                     | 12 ++++++++++--
 3 files changed, 20 insertions(+), 4 deletions(-)

diff --git a/arch/Kconfig b/arch/Kconfig
index e1e540f..cbec325 100644
--- a/arch/Kconfig
+++ b/arch/Kconfig

@@ -695,6 +695,14 @@ config HAVE_ARCH_HASH
          file which provides platform-specific implementations of some
          functions in <linux/hash.h> or fs/namei.c.

+config HAVE_GENERIC_EFI_FLAGS
+       bool
+       default n
+       help
+         Architecture defines a set of flags that EFI runtime services
+         should take care to restore when returning to the OS.
+         If this is not set, the set of flags defaults to the arch irqflags.
+
 config ISA_BUS_API
        def_bool ISA

diff --git a/drivers/firmware/efi/runtime-wrappers.c b/drivers/firmware/efi/runtime-wrappers.c
index 8903b9c..6dafa04 100644
--- a/drivers/firmware/efi/runtime-wrappers.c
+++ b/drivers/firmware/efi/runtime-wrappers.c

@@ -93,7 +93,7 @@ void efi_call_virt_check_flags(unsigned long flags, const char *call)
 {
        unsigned long cur_flags, mismatch;

-       local_save_flags(cur_flags);
+       efi_save_flags(cur_flags);

        mismatch = flags ^ cur_flags;
        if (!WARN_ON_ONCE(mismatch & ARCH_EFI_IRQ_FLAGS_MASK))

@@ -102,7 +102,7 @@ void efi_call_virt_check_flags(unsigned long flags, const char *call)
        add_taint(TAINT_FIRMWARE_WORKAROUND, LOCKDEP_NOW_UNRELIABLE);
        pr_err_ratelimited(FW_BUG "IRQ flags corrupted (0x%08lx=>0x%08lx) by EFI %s\n",
                           flags, cur_flags, call);
-       local_irq_restore(flags);
+       efi_restore_flags(flags);
 }

 /*

diff --git a/include/linux/efi.h b/include/linux/efi.h
index 100ce4a..41c110a 100644
--- a/include/linux/efi.h
+++ b/include/linux/efi.h

@@ -1594,6 +1594,14 @@ enum efi_secureboot_mode {

 void efi_retrieve_tpm2_eventlog(efi_system_table_t *sys_table);

+#ifdef CONFIG_HAVE_GENERIC_EFI_FLAGS
+#define efi_save_flags(state_flags)    arch_efi_save_flags(state_flags)
+#define efi_restore_flags(state_flags) arch_efi_restore_flags(state_flags)
+#else
+#define efi_save_flags(state_flags)    local_save_flags(state_flags)
+#define efi_restore_flags(state_flags) local_irq_restore(state_flags)
+#endif
+
 /*
  * Arch code can implement the following three template macros, avoiding
  * reptition for the void/non-void return cases of {__,}efi_call_virt():

@@ -1621,7 +1629,7 @@ enum efi_secureboot_mode {
                                                                        \
        arch_efi_call_virt_setup();                                     \
                                                                        \
-       local_save_flags(__flags);                                      \
+       efi_save_flags(__flags);                                        \
        __s = arch_efi_call_virt(p, f, args);                           \
        efi_call_virt_check_flags(__flags, __stringify(f));             \
                                                                        \

@@ -1636,7 +1644,7 @@ enum efi_secureboot_mode {
                                                                        \
        arch_efi_call_virt_setup();                                     \
                                                                        \
-       local_save_flags(__flags);                                      \
+       efi_save_flags(__flags);                                        \
        arch_efi_call_virt(p, f, args);                                 \
        efi_call_virt_check_flags(__flags, __stringify(f));             \
                                                                        \
--

1.9.1




--
Julien Thierry

-- 
Julien Thierry

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel