* [PATCHv2 1/5] efi/runtime-wrappers: add {__,}efi_call_virt templates
2016-04-21 11:35 [PATCHv2 0/5] efi: detect erroneous firmware IRQ manipulation Mark Rutland
@ 2016-04-21 11:35 ` Mark Rutland
[not found] ` <1461238529-12810-2-git-send-email-mark.rutland-5wv7dgnIgG8@public.gmane.org>
2016-04-21 11:35 ` Mark Rutland
` (3 subsequent siblings)
4 siblings, 1 reply; 26+ messages in thread
From: Mark Rutland @ 2016-04-21 11:35 UTC (permalink / raw)
To: linux-efi
Cc: ard.biesheuvel, catalin.marinas, hpa, leif.lindholm, linux,
mark.rutland, matt, mingo, tglx, will.deacon, linux-kernel
Currently each architecture must implement two macros, efi_call_virt and
__efi_call_virt, which only differ by the presence or absence of a
return type. Otherwise, the logic surrounding the call is identical.
As each architecture must define the entire body of each, we can't place
any generic manipulation (e.g. irq flag validation) in the middle.
This patch adds template implementations of these macros. With these,
arch code can implement three template macros, avoiding reptition for
the void/non-void return cases:
* arch_efi_call_virt_setup
Sets up the environment for the call (e.g. switching page tables,
allowing kernel-mode use of floating point, if required).
* arch_efi_call_virt
Performs the call. The last expression in the macro must be the call
itself, allowing the logic to be shared by the void and non-void
cases.
* arch_efi_call_virt_teardown
Restores the usual kernel environment once the call has returned.
While the savings from repition are minimal, we additionally gain the
ability to add common code around the call with the call enviroment set
up. This can be used to detect common firmware issues (e.g. bad irq mask
management).
Signed-off-by: Mark Rutland <mark.rutland@arm.com>
Cc: Matt Fleming <matt@codeblueprint.co.uk>
Cc: linux-efi@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
---
drivers/firmware/efi/runtime-wrappers.c | 21 +++++++++++++++++++++
1 file changed, 21 insertions(+)
diff --git a/drivers/firmware/efi/runtime-wrappers.c b/drivers/firmware/efi/runtime-wrappers.c
index de69530..1b9fa54 100644
--- a/drivers/firmware/efi/runtime-wrappers.c
+++ b/drivers/firmware/efi/runtime-wrappers.c
@@ -20,6 +20,27 @@
#include <linux/spinlock.h>
#include <asm/efi.h>
+
+#ifndef efi_call_virt
+#define efi_call_virt(f, args...) \
+({ \
+ efi_status_t __s; \
+ arch_efi_call_virt_setup(); \
+ __s = arch_efi_call_virt(f, args); \
+ arch_efi_call_virt_teardown(); \
+ __s; \
+})
+#endif
+
+#ifndef __efi_call_virt
+#define __efi_call_virt(f, args...) \
+({ \
+ arch_efi_call_virt_setup(); \
+ arch_efi_call_virt(f, args); \
+ arch_efi_call_virt_teardown(); \
+})
+#endif
+
/*
* 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
--
1.9.1
^ permalink raw reply related [flat|nested] 26+ messages in thread* [PATCHv2 2/5] arm64/efi: move to generic {__,}efi_call_virt
2016-04-21 11:35 [PATCHv2 0/5] efi: detect erroneous firmware IRQ manipulation Mark Rutland
@ 2016-04-21 11:35 ` Mark Rutland
2016-04-21 11:35 ` Mark Rutland
` (3 subsequent siblings)
4 siblings, 0 replies; 26+ messages in thread
From: Mark Rutland @ 2016-04-21 11:35 UTC (permalink / raw)
To: linux-efi
Cc: ard.biesheuvel, catalin.marinas, hpa, leif.lindholm, linux,
mark.rutland, matt, mingo, tglx, will.deacon, linux-arm-kernel,
linux-kernel
Now there's a common template for {__,}efi_call_virt, remove the
duplicate logic from the arm64 efi code.
Signed-off-by: Mark Rutland <mark.rutland@arm.com>
Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Cc: Catalin Marinas <catalin.marinas@arm.com>
Cc: Leif Lindholm <leif.lindholm@linaro.org>
Cc: Matt Fleming <matt@codeblueprint.co.uk>
Cc: Will Deacon <will.deacon@arm.com>
Cc: linux-arm-kernel@lists.infradead.org
Cc: linux-efi@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
---
arch/arm64/include/asm/efi.h | 21 +++++++--------------
1 file changed, 7 insertions(+), 14 deletions(-)
diff --git a/arch/arm64/include/asm/efi.h b/arch/arm64/include/asm/efi.h
index 8e88a69..f4f71224 100644
--- a/arch/arm64/include/asm/efi.h
+++ b/arch/arm64/include/asm/efi.h
@@ -14,28 +14,21 @@ extern void efi_init(void);
int efi_create_mapping(struct mm_struct *mm, efi_memory_desc_t *md);
-#define efi_call_virt(f, ...) \
+#define arch_efi_call_virt_setup() \
({ \
- efi_##f##_t *__f; \
- efi_status_t __s; \
- \
kernel_neon_begin(); \
efi_virtmap_load(); \
- __f = efi.systab->runtime->f; \
- __s = __f(__VA_ARGS__); \
- efi_virtmap_unload(); \
- kernel_neon_end(); \
- __s; \
})
-#define __efi_call_virt(f, ...) \
+#define arch_efi_call_virt(f, args...) \
({ \
efi_##f##_t *__f; \
- \
- kernel_neon_begin(); \
- efi_virtmap_load(); \
__f = efi.systab->runtime->f; \
- __f(__VA_ARGS__); \
+ __f(args); \
+})
+
+#define arch_efi_call_virt_teardown() \
+({ \
efi_virtmap_unload(); \
kernel_neon_end(); \
})
--
1.9.1
^ permalink raw reply related [flat|nested] 26+ messages in thread* [PATCHv2 2/5] arm64/efi: move to generic {__,}efi_call_virt
@ 2016-04-21 11:35 ` Mark Rutland
0 siblings, 0 replies; 26+ messages in thread
From: Mark Rutland @ 2016-04-21 11:35 UTC (permalink / raw)
To: linux-arm-kernel
Now there's a common template for {__,}efi_call_virt, remove the
duplicate logic from the arm64 efi code.
Signed-off-by: Mark Rutland <mark.rutland@arm.com>
Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Cc: Catalin Marinas <catalin.marinas@arm.com>
Cc: Leif Lindholm <leif.lindholm@linaro.org>
Cc: Matt Fleming <matt@codeblueprint.co.uk>
Cc: Will Deacon <will.deacon@arm.com>
Cc: linux-arm-kernel at lists.infradead.org
Cc: linux-efi at vger.kernel.org
Cc: linux-kernel at vger.kernel.org
---
arch/arm64/include/asm/efi.h | 21 +++++++--------------
1 file changed, 7 insertions(+), 14 deletions(-)
diff --git a/arch/arm64/include/asm/efi.h b/arch/arm64/include/asm/efi.h
index 8e88a69..f4f71224 100644
--- a/arch/arm64/include/asm/efi.h
+++ b/arch/arm64/include/asm/efi.h
@@ -14,28 +14,21 @@ extern void efi_init(void);
int efi_create_mapping(struct mm_struct *mm, efi_memory_desc_t *md);
-#define efi_call_virt(f, ...) \
+#define arch_efi_call_virt_setup() \
({ \
- efi_##f##_t *__f; \
- efi_status_t __s; \
- \
kernel_neon_begin(); \
efi_virtmap_load(); \
- __f = efi.systab->runtime->f; \
- __s = __f(__VA_ARGS__); \
- efi_virtmap_unload(); \
- kernel_neon_end(); \
- __s; \
})
-#define __efi_call_virt(f, ...) \
+#define arch_efi_call_virt(f, args...) \
({ \
efi_##f##_t *__f; \
- \
- kernel_neon_begin(); \
- efi_virtmap_load(); \
__f = efi.systab->runtime->f; \
- __f(__VA_ARGS__); \
+ __f(args); \
+})
+
+#define arch_efi_call_virt_teardown() \
+({ \
efi_virtmap_unload(); \
kernel_neon_end(); \
})
--
1.9.1
^ permalink raw reply related [flat|nested] 26+ messages in thread[parent not found: <1461238529-12810-3-git-send-email-mark.rutland-5wv7dgnIgG8@public.gmane.org>]
* Re: [PATCHv2 2/5] arm64/efi: move to generic {__,}efi_call_virt
2016-04-21 11:35 ` Mark Rutland
(?)
@ 2016-04-21 16:48 ` Will Deacon
-1 siblings, 0 replies; 26+ messages in thread
From: Will Deacon @ 2016-04-21 16:48 UTC (permalink / raw)
To: Mark Rutland
Cc: linux-efi-u79uwXL29TY76Z2rM5mHXA,
ard.biesheuvel-QSEj5FYQhm4dnm+yROfE0A,
catalin.marinas-5wv7dgnIgG8, hpa-YMNOUZJC4hwAvxtiuMwx3w,
leif.lindholm-QSEj5FYQhm4dnm+yROfE0A,
linux-lFZ/pmaqli7XmaaqVzeoHQ,
matt-mF/unelCI9GS6iBeEJttW/XRex20P6io,
mingo-H+wXaHxf7aLQT0dZR+AlfA, tglx-hfZtesqFncYOwBW4kG4KsQ,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
linux-kernel-u79uwXL29TY76Z2rM5mHXA
On Thu, Apr 21, 2016 at 12:35:26PM +0100, Mark Rutland wrote:
> Now there's a common template for {__,}efi_call_virt, remove the
> duplicate logic from the arm64 efi code.
>
> Signed-off-by: Mark Rutland <mark.rutland-5wv7dgnIgG8@public.gmane.org>
> Cc: Ard Biesheuvel <ard.biesheuvel-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
> Cc: Catalin Marinas <catalin.marinas-5wv7dgnIgG8@public.gmane.org>
> Cc: Leif Lindholm <leif.lindholm-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
> Cc: Matt Fleming <matt-mF/unelCI9GS6iBeEJttW/XRex20P6io@public.gmane.org>
> Cc: Will Deacon <will.deacon-5wv7dgnIgG8@public.gmane.org>
> Cc: linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org
> Cc: linux-efi-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> Cc: linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> ---
> arch/arm64/include/asm/efi.h | 21 +++++++--------------
> 1 file changed, 7 insertions(+), 14 deletions(-)
>
> diff --git a/arch/arm64/include/asm/efi.h b/arch/arm64/include/asm/efi.h
> index 8e88a69..f4f71224 100644
> --- a/arch/arm64/include/asm/efi.h
> +++ b/arch/arm64/include/asm/efi.h
> @@ -14,28 +14,21 @@ extern void efi_init(void);
>
> int efi_create_mapping(struct mm_struct *mm, efi_memory_desc_t *md);
>
> -#define efi_call_virt(f, ...) \
> +#define arch_efi_call_virt_setup() \
> ({ \
> - efi_##f##_t *__f; \
> - efi_status_t __s; \
> - \
> kernel_neon_begin(); \
> efi_virtmap_load(); \
> - __f = efi.systab->runtime->f; \
> - __s = __f(__VA_ARGS__); \
> - efi_virtmap_unload(); \
> - kernel_neon_end(); \
> - __s; \
> })
>
> -#define __efi_call_virt(f, ...) \
> +#define arch_efi_call_virt(f, args...) \
> ({ \
> efi_##f##_t *__f; \
> - \
> - kernel_neon_begin(); \
> - efi_virtmap_load(); \
> __f = efi.systab->runtime->f; \
> - __f(__VA_ARGS__); \
> + __f(args); \
Any reason to change this to a named argument? This patch is hard enough
to review as it is, given the way the diff has been generated!
Either way:
Acked-by: Will Deacon <will.deacon-5wv7dgnIgG8@public.gmane.org>
Will
^ permalink raw reply [flat|nested] 26+ messages in thread* Re: [PATCHv2 2/5] arm64/efi: move to generic {__,}efi_call_virt
@ 2016-04-21 16:48 ` Will Deacon
0 siblings, 0 replies; 26+ messages in thread
From: Will Deacon @ 2016-04-21 16:48 UTC (permalink / raw)
To: Mark Rutland
Cc: linux-efi, ard.biesheuvel, catalin.marinas, hpa, leif.lindholm,
linux, matt, mingo, tglx, linux-arm-kernel, linux-kernel
On Thu, Apr 21, 2016 at 12:35:26PM +0100, Mark Rutland wrote:
> Now there's a common template for {__,}efi_call_virt, remove the
> duplicate logic from the arm64 efi code.
>
> Signed-off-by: Mark Rutland <mark.rutland@arm.com>
> Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> Cc: Catalin Marinas <catalin.marinas@arm.com>
> Cc: Leif Lindholm <leif.lindholm@linaro.org>
> Cc: Matt Fleming <matt@codeblueprint.co.uk>
> Cc: Will Deacon <will.deacon@arm.com>
> Cc: linux-arm-kernel@lists.infradead.org
> Cc: linux-efi@vger.kernel.org
> Cc: linux-kernel@vger.kernel.org
> ---
> arch/arm64/include/asm/efi.h | 21 +++++++--------------
> 1 file changed, 7 insertions(+), 14 deletions(-)
>
> diff --git a/arch/arm64/include/asm/efi.h b/arch/arm64/include/asm/efi.h
> index 8e88a69..f4f71224 100644
> --- a/arch/arm64/include/asm/efi.h
> +++ b/arch/arm64/include/asm/efi.h
> @@ -14,28 +14,21 @@ extern void efi_init(void);
>
> int efi_create_mapping(struct mm_struct *mm, efi_memory_desc_t *md);
>
> -#define efi_call_virt(f, ...) \
> +#define arch_efi_call_virt_setup() \
> ({ \
> - efi_##f##_t *__f; \
> - efi_status_t __s; \
> - \
> kernel_neon_begin(); \
> efi_virtmap_load(); \
> - __f = efi.systab->runtime->f; \
> - __s = __f(__VA_ARGS__); \
> - efi_virtmap_unload(); \
> - kernel_neon_end(); \
> - __s; \
> })
>
> -#define __efi_call_virt(f, ...) \
> +#define arch_efi_call_virt(f, args...) \
> ({ \
> efi_##f##_t *__f; \
> - \
> - kernel_neon_begin(); \
> - efi_virtmap_load(); \
> __f = efi.systab->runtime->f; \
> - __f(__VA_ARGS__); \
> + __f(args); \
Any reason to change this to a named argument? This patch is hard enough
to review as it is, given the way the diff has been generated!
Either way:
Acked-by: Will Deacon <will.deacon@arm.com>
Will
^ permalink raw reply [flat|nested] 26+ messages in thread* [PATCHv2 2/5] arm64/efi: move to generic {__,}efi_call_virt
@ 2016-04-21 16:48 ` Will Deacon
0 siblings, 0 replies; 26+ messages in thread
From: Will Deacon @ 2016-04-21 16:48 UTC (permalink / raw)
To: linux-arm-kernel
On Thu, Apr 21, 2016 at 12:35:26PM +0100, Mark Rutland wrote:
> Now there's a common template for {__,}efi_call_virt, remove the
> duplicate logic from the arm64 efi code.
>
> Signed-off-by: Mark Rutland <mark.rutland@arm.com>
> Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> Cc: Catalin Marinas <catalin.marinas@arm.com>
> Cc: Leif Lindholm <leif.lindholm@linaro.org>
> Cc: Matt Fleming <matt@codeblueprint.co.uk>
> Cc: Will Deacon <will.deacon@arm.com>
> Cc: linux-arm-kernel at lists.infradead.org
> Cc: linux-efi at vger.kernel.org
> Cc: linux-kernel at vger.kernel.org
> ---
> arch/arm64/include/asm/efi.h | 21 +++++++--------------
> 1 file changed, 7 insertions(+), 14 deletions(-)
>
> diff --git a/arch/arm64/include/asm/efi.h b/arch/arm64/include/asm/efi.h
> index 8e88a69..f4f71224 100644
> --- a/arch/arm64/include/asm/efi.h
> +++ b/arch/arm64/include/asm/efi.h
> @@ -14,28 +14,21 @@ extern void efi_init(void);
>
> int efi_create_mapping(struct mm_struct *mm, efi_memory_desc_t *md);
>
> -#define efi_call_virt(f, ...) \
> +#define arch_efi_call_virt_setup() \
> ({ \
> - efi_##f##_t *__f; \
> - efi_status_t __s; \
> - \
> kernel_neon_begin(); \
> efi_virtmap_load(); \
> - __f = efi.systab->runtime->f; \
> - __s = __f(__VA_ARGS__); \
> - efi_virtmap_unload(); \
> - kernel_neon_end(); \
> - __s; \
> })
>
> -#define __efi_call_virt(f, ...) \
> +#define arch_efi_call_virt(f, args...) \
> ({ \
> efi_##f##_t *__f; \
> - \
> - kernel_neon_begin(); \
> - efi_virtmap_load(); \
> __f = efi.systab->runtime->f; \
> - __f(__VA_ARGS__); \
> + __f(args); \
Any reason to change this to a named argument? This patch is hard enough
to review as it is, given the way the diff has been generated!
Either way:
Acked-by: Will Deacon <will.deacon@arm.com>
Will
^ permalink raw reply [flat|nested] 26+ messages in thread[parent not found: <20160421164840.GO929-5wv7dgnIgG8@public.gmane.org>]
* Re: [PATCHv2 2/5] arm64/efi: move to generic {__,}efi_call_virt
2016-04-21 16:48 ` Will Deacon
(?)
@ 2016-04-21 16:58 ` Mark Rutland
-1 siblings, 0 replies; 26+ messages in thread
From: Mark Rutland @ 2016-04-21 16:58 UTC (permalink / raw)
To: Will Deacon
Cc: linux-efi-u79uwXL29TY76Z2rM5mHXA,
ard.biesheuvel-QSEj5FYQhm4dnm+yROfE0A,
catalin.marinas-5wv7dgnIgG8, hpa-YMNOUZJC4hwAvxtiuMwx3w,
leif.lindholm-QSEj5FYQhm4dnm+yROfE0A,
linux-lFZ/pmaqli7XmaaqVzeoHQ,
matt-mF/unelCI9GS6iBeEJttW/XRex20P6io,
mingo-H+wXaHxf7aLQT0dZR+AlfA, tglx-hfZtesqFncYOwBW4kG4KsQ,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
linux-kernel-u79uwXL29TY76Z2rM5mHXA
On Thu, Apr 21, 2016 at 05:48:40PM +0100, Will Deacon wrote:
> On Thu, Apr 21, 2016 at 12:35:26PM +0100, Mark Rutland wrote:
> > Now there's a common template for {__,}efi_call_virt, remove the
> > duplicate logic from the arm64 efi code.
> >
> > Signed-off-by: Mark Rutland <mark.rutland-5wv7dgnIgG8@public.gmane.org>
> > Cc: Ard Biesheuvel <ard.biesheuvel-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
> > Cc: Catalin Marinas <catalin.marinas-5wv7dgnIgG8@public.gmane.org>
> > Cc: Leif Lindholm <leif.lindholm-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
> > Cc: Matt Fleming <matt-mF/unelCI9GS6iBeEJttW/XRex20P6io@public.gmane.org>
> > Cc: Will Deacon <will.deacon-5wv7dgnIgG8@public.gmane.org>
> > Cc: linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org
> > Cc: linux-efi-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> > Cc: linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> > ---
> > arch/arm64/include/asm/efi.h | 21 +++++++--------------
> > 1 file changed, 7 insertions(+), 14 deletions(-)
> >
> > diff --git a/arch/arm64/include/asm/efi.h b/arch/arm64/include/asm/efi.h
> > index 8e88a69..f4f71224 100644
> > --- a/arch/arm64/include/asm/efi.h
> > +++ b/arch/arm64/include/asm/efi.h
> > @@ -14,28 +14,21 @@ extern void efi_init(void);
> >
> > int efi_create_mapping(struct mm_struct *mm, efi_memory_desc_t *md);
> >
> > -#define efi_call_virt(f, ...) \
> > +#define arch_efi_call_virt_setup() \
> > ({ \
> > - efi_##f##_t *__f; \
> > - efi_status_t __s; \
> > - \
> > kernel_neon_begin(); \
> > efi_virtmap_load(); \
> > - __f = efi.systab->runtime->f; \
> > - __s = __f(__VA_ARGS__); \
> > - efi_virtmap_unload(); \
> > - kernel_neon_end(); \
> > - __s; \
> > })
> >
> > -#define __efi_call_virt(f, ...) \
> > +#define arch_efi_call_virt(f, args...) \
> > ({ \
> > efi_##f##_t *__f; \
> > - \
> > - kernel_neon_begin(); \
> > - efi_virtmap_load(); \
> > __f = efi.systab->runtime->f; \
> > - __f(__VA_ARGS__); \
> > + __f(args); \
>
> Any reason to change this to a named argument? This patch is hard enough
> to review as it is, given the way the diff has been generated!
That was to enforce consistency across arm/arm64/x86 for this.
It seemed nicer to make them all use a named argument than it did to
make them all use __VA_ARGS__, either of which was nice than each of
them doing something slightly different.
Sorry for the pain that evidently caused!
> Either way:
>
> Acked-by: Will Deacon <will.deacon-5wv7dgnIgG8@public.gmane.org>
Cheers!
Mark.
^ permalink raw reply [flat|nested] 26+ messages in thread* Re: [PATCHv2 2/5] arm64/efi: move to generic {__,}efi_call_virt
@ 2016-04-21 16:58 ` Mark Rutland
0 siblings, 0 replies; 26+ messages in thread
From: Mark Rutland @ 2016-04-21 16:58 UTC (permalink / raw)
To: Will Deacon
Cc: linux-efi, ard.biesheuvel, catalin.marinas, hpa, leif.lindholm,
linux, matt, mingo, tglx, linux-arm-kernel, linux-kernel
On Thu, Apr 21, 2016 at 05:48:40PM +0100, Will Deacon wrote:
> On Thu, Apr 21, 2016 at 12:35:26PM +0100, Mark Rutland wrote:
> > Now there's a common template for {__,}efi_call_virt, remove the
> > duplicate logic from the arm64 efi code.
> >
> > Signed-off-by: Mark Rutland <mark.rutland@arm.com>
> > Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> > Cc: Catalin Marinas <catalin.marinas@arm.com>
> > Cc: Leif Lindholm <leif.lindholm@linaro.org>
> > Cc: Matt Fleming <matt@codeblueprint.co.uk>
> > Cc: Will Deacon <will.deacon@arm.com>
> > Cc: linux-arm-kernel@lists.infradead.org
> > Cc: linux-efi@vger.kernel.org
> > Cc: linux-kernel@vger.kernel.org
> > ---
> > arch/arm64/include/asm/efi.h | 21 +++++++--------------
> > 1 file changed, 7 insertions(+), 14 deletions(-)
> >
> > diff --git a/arch/arm64/include/asm/efi.h b/arch/arm64/include/asm/efi.h
> > index 8e88a69..f4f71224 100644
> > --- a/arch/arm64/include/asm/efi.h
> > +++ b/arch/arm64/include/asm/efi.h
> > @@ -14,28 +14,21 @@ extern void efi_init(void);
> >
> > int efi_create_mapping(struct mm_struct *mm, efi_memory_desc_t *md);
> >
> > -#define efi_call_virt(f, ...) \
> > +#define arch_efi_call_virt_setup() \
> > ({ \
> > - efi_##f##_t *__f; \
> > - efi_status_t __s; \
> > - \
> > kernel_neon_begin(); \
> > efi_virtmap_load(); \
> > - __f = efi.systab->runtime->f; \
> > - __s = __f(__VA_ARGS__); \
> > - efi_virtmap_unload(); \
> > - kernel_neon_end(); \
> > - __s; \
> > })
> >
> > -#define __efi_call_virt(f, ...) \
> > +#define arch_efi_call_virt(f, args...) \
> > ({ \
> > efi_##f##_t *__f; \
> > - \
> > - kernel_neon_begin(); \
> > - efi_virtmap_load(); \
> > __f = efi.systab->runtime->f; \
> > - __f(__VA_ARGS__); \
> > + __f(args); \
>
> Any reason to change this to a named argument? This patch is hard enough
> to review as it is, given the way the diff has been generated!
That was to enforce consistency across arm/arm64/x86 for this.
It seemed nicer to make them all use a named argument than it did to
make them all use __VA_ARGS__, either of which was nice than each of
them doing something slightly different.
Sorry for the pain that evidently caused!
> Either way:
>
> Acked-by: Will Deacon <will.deacon@arm.com>
Cheers!
Mark.
^ permalink raw reply [flat|nested] 26+ messages in thread* [PATCHv2 2/5] arm64/efi: move to generic {__,}efi_call_virt
@ 2016-04-21 16:58 ` Mark Rutland
0 siblings, 0 replies; 26+ messages in thread
From: Mark Rutland @ 2016-04-21 16:58 UTC (permalink / raw)
To: linux-arm-kernel
On Thu, Apr 21, 2016 at 05:48:40PM +0100, Will Deacon wrote:
> On Thu, Apr 21, 2016 at 12:35:26PM +0100, Mark Rutland wrote:
> > Now there's a common template for {__,}efi_call_virt, remove the
> > duplicate logic from the arm64 efi code.
> >
> > Signed-off-by: Mark Rutland <mark.rutland@arm.com>
> > Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> > Cc: Catalin Marinas <catalin.marinas@arm.com>
> > Cc: Leif Lindholm <leif.lindholm@linaro.org>
> > Cc: Matt Fleming <matt@codeblueprint.co.uk>
> > Cc: Will Deacon <will.deacon@arm.com>
> > Cc: linux-arm-kernel at lists.infradead.org
> > Cc: linux-efi at vger.kernel.org
> > Cc: linux-kernel at vger.kernel.org
> > ---
> > arch/arm64/include/asm/efi.h | 21 +++++++--------------
> > 1 file changed, 7 insertions(+), 14 deletions(-)
> >
> > diff --git a/arch/arm64/include/asm/efi.h b/arch/arm64/include/asm/efi.h
> > index 8e88a69..f4f71224 100644
> > --- a/arch/arm64/include/asm/efi.h
> > +++ b/arch/arm64/include/asm/efi.h
> > @@ -14,28 +14,21 @@ extern void efi_init(void);
> >
> > int efi_create_mapping(struct mm_struct *mm, efi_memory_desc_t *md);
> >
> > -#define efi_call_virt(f, ...) \
> > +#define arch_efi_call_virt_setup() \
> > ({ \
> > - efi_##f##_t *__f; \
> > - efi_status_t __s; \
> > - \
> > kernel_neon_begin(); \
> > efi_virtmap_load(); \
> > - __f = efi.systab->runtime->f; \
> > - __s = __f(__VA_ARGS__); \
> > - efi_virtmap_unload(); \
> > - kernel_neon_end(); \
> > - __s; \
> > })
> >
> > -#define __efi_call_virt(f, ...) \
> > +#define arch_efi_call_virt(f, args...) \
> > ({ \
> > efi_##f##_t *__f; \
> > - \
> > - kernel_neon_begin(); \
> > - efi_virtmap_load(); \
> > __f = efi.systab->runtime->f; \
> > - __f(__VA_ARGS__); \
> > + __f(args); \
>
> Any reason to change this to a named argument? This patch is hard enough
> to review as it is, given the way the diff has been generated!
That was to enforce consistency across arm/arm64/x86 for this.
It seemed nicer to make them all use a named argument than it did to
make them all use __VA_ARGS__, either of which was nice than each of
them doing something slightly different.
Sorry for the pain that evidently caused!
> Either way:
>
> Acked-by: Will Deacon <will.deacon@arm.com>
Cheers!
Mark.
^ permalink raw reply [flat|nested] 26+ messages in thread
* [PATCHv2 3/5] arm/efi: move to generic {__,}efi_call_virt
2016-04-21 11:35 [PATCHv2 0/5] efi: detect erroneous firmware IRQ manipulation Mark Rutland
@ 2016-04-21 11:35 ` Mark Rutland
2016-04-21 11:35 ` Mark Rutland
` (3 subsequent siblings)
4 siblings, 0 replies; 26+ messages in thread
From: Mark Rutland @ 2016-04-21 11:35 UTC (permalink / raw)
To: linux-efi
Cc: ard.biesheuvel, catalin.marinas, hpa, leif.lindholm, linux,
mark.rutland, matt, mingo, tglx, will.deacon, linux-arm-kernel,
linux-kernel
Now there's a common template for {__,}efi_call_virt, remove the
duplicate logic from the arm efi code.
Signed-off-by: Mark Rutland <mark.rutland@arm.com>
Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Cc: Leif Lindholm <leif.lindholm@linaro.org>
Cc: Matt Fleming <matt@codeblueprint.co.uk>
Cc: Russell King <linux@arm.linux.org.uk>
Cc: linux-arm-kernel@lists.infradead.org
Cc: linux-efi@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
---
arch/arm/include/asm/efi.h | 20 ++++----------------
1 file changed, 4 insertions(+), 16 deletions(-)
diff --git a/arch/arm/include/asm/efi.h b/arch/arm/include/asm/efi.h
index e0eea72..e1461fc 100644
--- a/arch/arm/include/asm/efi.h
+++ b/arch/arm/include/asm/efi.h
@@ -23,26 +23,14 @@ void efi_init(void);
int efi_create_mapping(struct mm_struct *mm, efi_memory_desc_t *md);
-#define efi_call_virt(f, ...) \
-({ \
- efi_##f##_t *__f; \
- efi_status_t __s; \
- \
- efi_virtmap_load(); \
- __f = efi.systab->runtime->f; \
- __s = __f(__VA_ARGS__); \
- efi_virtmap_unload(); \
- __s; \
-})
+#define arch_efi_call_virt_setup() efi_virtmap_load()
+#define arch_efi_call_virt_teardown() efi_virtmap_unload()
-#define __efi_call_virt(f, ...) \
+#define arch_efi_call_virt(f, args...) \
({ \
efi_##f##_t *__f; \
- \
- efi_virtmap_load(); \
__f = efi.systab->runtime->f; \
- __f(__VA_ARGS__); \
- efi_virtmap_unload(); \
+ __f(args); \
})
static inline void efi_set_pgd(struct mm_struct *mm)
--
1.9.1
^ permalink raw reply related [flat|nested] 26+ messages in thread* [PATCHv2 3/5] arm/efi: move to generic {__,}efi_call_virt
@ 2016-04-21 11:35 ` Mark Rutland
0 siblings, 0 replies; 26+ messages in thread
From: Mark Rutland @ 2016-04-21 11:35 UTC (permalink / raw)
To: linux-arm-kernel
Now there's a common template for {__,}efi_call_virt, remove the
duplicate logic from the arm efi code.
Signed-off-by: Mark Rutland <mark.rutland@arm.com>
Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Cc: Leif Lindholm <leif.lindholm@linaro.org>
Cc: Matt Fleming <matt@codeblueprint.co.uk>
Cc: Russell King <linux@arm.linux.org.uk>
Cc: linux-arm-kernel at lists.infradead.org
Cc: linux-efi at vger.kernel.org
Cc: linux-kernel at vger.kernel.org
---
arch/arm/include/asm/efi.h | 20 ++++----------------
1 file changed, 4 insertions(+), 16 deletions(-)
diff --git a/arch/arm/include/asm/efi.h b/arch/arm/include/asm/efi.h
index e0eea72..e1461fc 100644
--- a/arch/arm/include/asm/efi.h
+++ b/arch/arm/include/asm/efi.h
@@ -23,26 +23,14 @@ void efi_init(void);
int efi_create_mapping(struct mm_struct *mm, efi_memory_desc_t *md);
-#define efi_call_virt(f, ...) \
-({ \
- efi_##f##_t *__f; \
- efi_status_t __s; \
- \
- efi_virtmap_load(); \
- __f = efi.systab->runtime->f; \
- __s = __f(__VA_ARGS__); \
- efi_virtmap_unload(); \
- __s; \
-})
+#define arch_efi_call_virt_setup() efi_virtmap_load()
+#define arch_efi_call_virt_teardown() efi_virtmap_unload()
-#define __efi_call_virt(f, ...) \
+#define arch_efi_call_virt(f, args...) \
({ \
efi_##f##_t *__f; \
- \
- efi_virtmap_load(); \
__f = efi.systab->runtime->f; \
- __f(__VA_ARGS__); \
- efi_virtmap_unload(); \
+ __f(args); \
})
static inline void efi_set_pgd(struct mm_struct *mm)
--
1.9.1
^ permalink raw reply related [flat|nested] 26+ messages in thread
[parent not found: <1461238529-12810-1-git-send-email-mark.rutland-5wv7dgnIgG8@public.gmane.org>]
* [PATCHv2 4/5] x86/efi: move to generic {__,}efi_call_virt
2016-04-21 11:35 [PATCHv2 0/5] efi: detect erroneous firmware IRQ manipulation Mark Rutland
@ 2016-04-21 11:35 ` Mark Rutland
2016-04-21 11:35 ` Mark Rutland
` (3 subsequent siblings)
4 siblings, 0 replies; 26+ messages in thread
From: Mark Rutland @ 2016-04-21 11:35 UTC (permalink / raw)
To: linux-efi-u79uwXL29TY76Z2rM5mHXA
Cc: ard.biesheuvel-QSEj5FYQhm4dnm+yROfE0A,
catalin.marinas-5wv7dgnIgG8, hpa-YMNOUZJC4hwAvxtiuMwx3w,
leif.lindholm-QSEj5FYQhm4dnm+yROfE0A,
linux-lFZ/pmaqli7XmaaqVzeoHQ, mark.rutland-5wv7dgnIgG8,
matt-mF/unelCI9GS6iBeEJttW/XRex20P6io,
mingo-H+wXaHxf7aLQT0dZR+AlfA, tglx-hfZtesqFncYOwBW4kG4KsQ,
will.deacon-5wv7dgnIgG8, linux-kernel-u79uwXL29TY76Z2rM5mHXA
Now there's a common template for {__,}efi_call_virt, remove the
duplicate logic from the x86 efi code.
Signed-off-by: Mark Rutland <mark.rutland-5wv7dgnIgG8@public.gmane.org>
Cc: "H. Peter Anvin" <hpa-YMNOUZJC4hwAvxtiuMwx3w@public.gmane.org>
Cc: Ingo Molnar <mingo-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
Cc: Matt Fleming <matt-mF/unelCI9GS6iBeEJttW/XRex20P6io@public.gmane.org>
Cc: Thomas Gleixner <tglx-hfZtesqFncYOwBW4kG4KsQ@public.gmane.org>
Cc: linux-efi-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
Cc: linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
---
arch/x86/include/asm/efi.h | 41 ++++++++++++-----------------------------
1 file changed, 12 insertions(+), 29 deletions(-)
diff --git a/arch/x86/include/asm/efi.h b/arch/x86/include/asm/efi.h
index 53748c4..cf79c8f 100644
--- a/arch/x86/include/asm/efi.h
+++ b/arch/x86/include/asm/efi.h
@@ -33,28 +33,16 @@
extern unsigned long asmlinkage efi_call_phys(void *, ...);
+#define arch_efi_call_virt_setup() kernel_fpu_begin()
+#define arch_efi_call_virt_teardown() kernel_fpu_end()
+
/*
* Wrap all the virtual calls in a way that forces the parameters on the stack.
*/
-
-/* Use this macro if your virtual returns a non-void value */
-#define efi_call_virt(f, args...) \
-({ \
- efi_status_t __s; \
- kernel_fpu_begin(); \
- __s = ((efi_##f##_t __attribute__((regparm(0)))*) \
- efi.systab->runtime->f)(args); \
- kernel_fpu_end(); \
- __s; \
-})
-
-/* Use this macro if your virtual call does not return any value */
-#define __efi_call_virt(f, args...) \
+#define arch_efi_call_virt(f, args...) \
({ \
- kernel_fpu_begin(); \
((efi_##f##_t __attribute__((regparm(0)))*) \
efi.systab->runtime->f)(args); \
- kernel_fpu_end(); \
})
#define efi_ioremap(addr, size, type, attr) ioremap_cache(addr, size)
@@ -78,10 +66,8 @@ struct efi_scratch {
u64 phys_stack;
} __packed;
-#define efi_call_virt(f, ...) \
+#define arch_efi_call_virt_setup() \
({ \
- efi_status_t __s; \
- \
efi_sync_low_kernel_mappings(); \
preempt_disable(); \
__kernel_fpu_begin(); \
@@ -91,9 +77,13 @@ struct efi_scratch {
write_cr3((unsigned long)efi_scratch.efi_pgt); \
__flush_tlb_all(); \
} \
- \
- __s = efi_call((void *)efi.systab->runtime->f, __VA_ARGS__); \
- \
+})
+
+#define arch_efi_call_virt(f, args...) \
+ efi_call((void *)efi.systab->runtime->f, args); \
+
+#define arch_efi_call_virt_teardown() \
+({ \
if (efi_scratch.use_pgd) { \
write_cr3(efi_scratch.prev_cr3); \
__flush_tlb_all(); \
@@ -101,15 +91,8 @@ struct efi_scratch {
\
__kernel_fpu_end(); \
preempt_enable(); \
- __s; \
})
-/*
- * All X86_64 virt calls return non-void values. Thus, use non-void call for
- * virt calls that would be void on X86_32.
- */
-#define __efi_call_virt(f, args...) efi_call_virt(f, args)
-
extern void __iomem *__init efi_ioremap(unsigned long addr, unsigned long size,
u32 type, u64 attribute);
--
1.9.1
^ permalink raw reply related [flat|nested] 26+ messages in thread* [PATCHv2 4/5] x86/efi: move to generic {__,}efi_call_virt
@ 2016-04-21 11:35 ` Mark Rutland
0 siblings, 0 replies; 26+ messages in thread
From: Mark Rutland @ 2016-04-21 11:35 UTC (permalink / raw)
To: linux-efi
Cc: ard.biesheuvel, catalin.marinas, hpa, leif.lindholm, linux,
mark.rutland, matt, mingo, tglx, will.deacon, linux-kernel
Now there's a common template for {__,}efi_call_virt, remove the
duplicate logic from the x86 efi code.
Signed-off-by: Mark Rutland <mark.rutland@arm.com>
Cc: "H. Peter Anvin" <hpa@zytor.com>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: Matt Fleming <matt@codeblueprint.co.uk>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: linux-efi@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
---
arch/x86/include/asm/efi.h | 41 ++++++++++++-----------------------------
1 file changed, 12 insertions(+), 29 deletions(-)
diff --git a/arch/x86/include/asm/efi.h b/arch/x86/include/asm/efi.h
index 53748c4..cf79c8f 100644
--- a/arch/x86/include/asm/efi.h
+++ b/arch/x86/include/asm/efi.h
@@ -33,28 +33,16 @@
extern unsigned long asmlinkage efi_call_phys(void *, ...);
+#define arch_efi_call_virt_setup() kernel_fpu_begin()
+#define arch_efi_call_virt_teardown() kernel_fpu_end()
+
/*
* Wrap all the virtual calls in a way that forces the parameters on the stack.
*/
-
-/* Use this macro if your virtual returns a non-void value */
-#define efi_call_virt(f, args...) \
-({ \
- efi_status_t __s; \
- kernel_fpu_begin(); \
- __s = ((efi_##f##_t __attribute__((regparm(0)))*) \
- efi.systab->runtime->f)(args); \
- kernel_fpu_end(); \
- __s; \
-})
-
-/* Use this macro if your virtual call does not return any value */
-#define __efi_call_virt(f, args...) \
+#define arch_efi_call_virt(f, args...) \
({ \
- kernel_fpu_begin(); \
((efi_##f##_t __attribute__((regparm(0)))*) \
efi.systab->runtime->f)(args); \
- kernel_fpu_end(); \
})
#define efi_ioremap(addr, size, type, attr) ioremap_cache(addr, size)
@@ -78,10 +66,8 @@ struct efi_scratch {
u64 phys_stack;
} __packed;
-#define efi_call_virt(f, ...) \
+#define arch_efi_call_virt_setup() \
({ \
- efi_status_t __s; \
- \
efi_sync_low_kernel_mappings(); \
preempt_disable(); \
__kernel_fpu_begin(); \
@@ -91,9 +77,13 @@ struct efi_scratch {
write_cr3((unsigned long)efi_scratch.efi_pgt); \
__flush_tlb_all(); \
} \
- \
- __s = efi_call((void *)efi.systab->runtime->f, __VA_ARGS__); \
- \
+})
+
+#define arch_efi_call_virt(f, args...) \
+ efi_call((void *)efi.systab->runtime->f, args); \
+
+#define arch_efi_call_virt_teardown() \
+({ \
if (efi_scratch.use_pgd) { \
write_cr3(efi_scratch.prev_cr3); \
__flush_tlb_all(); \
@@ -101,15 +91,8 @@ struct efi_scratch {
\
__kernel_fpu_end(); \
preempt_enable(); \
- __s; \
})
-/*
- * All X86_64 virt calls return non-void values. Thus, use non-void call for
- * virt calls that would be void on X86_32.
- */
-#define __efi_call_virt(f, args...) efi_call_virt(f, args)
-
extern void __iomem *__init efi_ioremap(unsigned long addr, unsigned long size,
u32 type, u64 attribute);
--
1.9.1
^ permalink raw reply related [flat|nested] 26+ messages in thread
* Re: [PATCHv2 0/5] efi: detect erroneous firmware IRQ manipulation
[not found] ` <1461238529-12810-1-git-send-email-mark.rutland-5wv7dgnIgG8@public.gmane.org>
2016-04-21 11:35 ` Mark Rutland
@ 2016-04-21 11:47 ` Leif Lindholm
[not found] ` <20160421114737.GQ2904-t77nlHhSwNqAroYi2ySoxKxOck334EZe@public.gmane.org>
1 sibling, 1 reply; 26+ messages in thread
From: Leif Lindholm @ 2016-04-21 11:47 UTC (permalink / raw)
To: Mark Rutland
Cc: linux-efi-u79uwXL29TY76Z2rM5mHXA,
ard.biesheuvel-QSEj5FYQhm4dnm+yROfE0A,
catalin.marinas-5wv7dgnIgG8, hpa-YMNOUZJC4hwAvxtiuMwx3w,
linux-lFZ/pmaqli7XmaaqVzeoHQ,
matt-mF/unelCI9GS6iBeEJttW/XRex20P6io,
mingo-H+wXaHxf7aLQT0dZR+AlfA, tglx-hfZtesqFncYOwBW4kG4KsQ,
will.deacon-5wv7dgnIgG8
On Thu, Apr 21, 2016 at 12:35:24PM +0100, Mark Rutland wrote:
> Some firmware erroneously unmask IRQs (and potentially other architecture
> specific exceptions) during runtime services functions, in violation of both
> common sense and the UEFI specification. This can result in a number of issues
> if said exceptions are taken when they are expected to be masked, and
> additionally can confuse IRQ tracing if the original mask state is not
> restored prior to returning from firmware.
>
> In practice it's difficult to check that firmware never unmasks exceptions, but
> we can at least check that the IRQ flags are at least consistent upon entry to
> and return from a runtime services function call. This series implements said
> check in the shared EFI runtime wrappers code, after an initial round of
> refactoring such that this can be generic.
>
> I have left ia64 as-is, without this check, as ia64 has many special cases for
> the runtime calls which don't fit well with the generic code, and I don't
> expect a new, buggy ia64 firmware to appear soon.
>
> The first time corruption of the IRQ flags is detected, we dump a stack trace,
> and set TAINT_FIRMWARE_WORKAROUND. Additionally, and in all subsequent cases,
> we log (with ratelimiting) the specific corruption of the flags, and restore
> the expected flags to avoid redundant warnings elsewhere.
>
> For example, if the firmware on an arm64 system erroneously unmasked IRQ+FIQ,
> we would see warnings in dmesg of the form:
>
> [ 5.639616] ------------[ cut here ]------------
> [ 5.644233] WARNING: CPU: 3 PID: 1120 at drivers/firmware/efi/runtime-wrappers.c:57 efi_call_virt_check_flags+0x84/0x90
> [ 5.655001] Modules linked in:
> [ 5.658046]
> [ 5.659528] CPU: 3 PID: 1120 Comm: mount Not tainted 4.6.0-rc4+ #6
> [ 5.674293] task: ffffffc3ecbcc800 ti: ffffffc0fa498000 task.ti: ffffffc0fa498000
> [ 5.681763] PC is at efi_call_virt_check_flags+0x84/0x90
> [ 5.687063] LR is at virt_efi_get_next_variable+0x74/0xa0
> [ 5.692449] pc : [<ffffff8008618ccc>] lr : [<ffffff8008618f2c>] pstate: 20000105
> [ 5.699831] sp : ffffffc0fa49bbd0
> [ 5.703133] x29: ffffffc0fa49bbd0 x28: ffffffc0fa498000
> [ 5.708436] x27: ffffff8008776000 x26: 0000000000000001
> [ 5.713739] x25: ffffff8008bf2000 x24: 0000000000000000
> [ 5.719042] x23: ffffffc0fa49bcc0 x22: ffffffc3eb84d400
> [ 5.724344] x21: 00000000000001c0 x20: 0000000000000100
> [ 5.729646] x19: ffffff8008bf2a20 x18: 0000007fc1f31aa0
> [ 5.734948] x17: 0000000000425060 x16: ffffff80081e4578
> [ 5.740251] x15: ffffffffffffffff x14: 0000000000000000
> [ 5.745553] x13: 0000000000000000 x12: 0000000000000000
> [ 5.750855] x11: 0101010101010101 x10: 7f7f7f7f7f7f7f7f
> [ 5.756157] x9 : 0000000000000000 x8 : 0000000000000000
> [ 5.761459] x7 : 0000000000000000 x6 : 0000000000000001
> [ 5.766761] x5 : 00000000220e94ba x4 : 0000000022c61190
> [ 5.772063] x3 : 0000000000000001 x2 : ffffff8008bc0000
> [ 5.777366] x1 : ffffff80089d9d10 x0 : 00000000000001c0
> [ 5.782667]
> [ 5.784147] ---[ end trace b1612054aa2afdce ]---
> [ 5.788751] Call trace:
> [ 5.791186] Exception stack(0xffffffc0fa49ba10 to 0xffffffc0fa49bb30)
> [ 5.797614] ba00: ffffff8008bf2a20 0000000000000100
> [ 5.805430] ba20: ffffffc0fa49bbd0 ffffff8008618ccc ffffffc0fa49ba60 00000000220e94be
> [ 5.813247] ba40: ffffffc0fa49ba70 0000000022c5a208 ffffffc0fa49ba80 0000000000000010
> [ 5.821063] ba60: 0000000022168ff8 ffffffc0fa49bcc0 ffffffc0fa49baa0 0000000022c4d440
> [ 5.828880] ba80: ffffffc0fa49baa0 0000000000000010 0000000022168ff8 ffffffc0fa49bcc0
> [ 5.836696] baa0: ffffffc0fa49bb10 0000000022c4db50 00000000000001c0 ffffff80089d9d10
> [ 5.844512] bac0: ffffff8008bc0000 0000000000000001 0000000022c61190 00000000220e94ba
> [ 5.852329] bae0: 0000000000000001 0000000000000000 0000000000000000 0000000000000000
> [ 5.860145] bb00: 7f7f7f7f7f7f7f7f 0101010101010101 0000000000000000 0000000000000000
> [ 5.867961] bb20: 0000000000000000 ffffffffffffffff
> [ 5.872826] [<ffffff8008618ccc>] efi_call_virt_check_flags+0x84/0x90
> [ 5.879168] [<ffffff8008618f2c>] virt_efi_get_next_variable+0x74/0xa0
> [ 5.885596] [<ffffff80086182a4>] efivar_init+0x7c/0x2a0
> [ 5.890810] [<ffffff80082f7c7c>] efivarfs_fill_super+0xac/0xe8
> [ 5.896633] [<ffffff80081c5e34>] mount_single+0x8c/0xb8
> [ 5.901846] [<ffffff80082f7bc8>] efivarfs_mount+0x18/0x20
> [ 5.907232] [<ffffff80081c5f0c>] mount_fs+0x4c/0x168
> [ 5.912184] [<ffffff80081e0f38>] vfs_kern_mount+0x50/0x118
> [ 5.917658] [<ffffff80081e3890>] do_mount+0x208/0xc08
> [ 5.922697] [<ffffff80081e4608>] SyS_mount+0x90/0xf8
> [ 5.927649] [<ffffff8008085e70>] el0_svc_naked+0x24/0x28
> [ 5.932960] Disabling lock debugging due to kernel taint
> [ 5.938263] [Firmware Bug]: IRQ flags corrupted (0x000001c0=>0x00000100) by EFI get_next_variable
>
> Then subsequently:
>
> [ 5.938263] [Firmware Bug]: IRQ flags corrupted (0x000001c0=>0x00000100) by EFI get_next_variable
> [ 5.947143] [Firmware Bug]: IRQ flags corrupted (0x000001c0=>0x00000100) by EFI get_variable
So, I think this is a good thing, but the diffs end up being quite
hard to deciphre. Is there any non-insane shuffling around of things
that can make the changeset more clear?
/
Leif
> Thanks,
> Mark.
>
> Mark Rutland (5):
> efi/runtime-wrappers: add {__,}efi_call_virt templates
> arm64/efi: move to generic {__,}efi_call_virt
> arm/efi: move to generic {__,}efi_call_virt
> x86/efi: move to generic {__,}efi_call_virt
> efi/runtime-wrappers: detect FW irq flag corruption
>
> arch/arm/include/asm/efi.h | 20 +++------------
> arch/arm64/include/asm/efi.h | 21 ++++++----------
> arch/x86/include/asm/efi.h | 41 +++++++++----------------------
> drivers/firmware/efi/runtime-wrappers.c | 43 +++++++++++++++++++++++++++++++++
> 4 files changed, 66 insertions(+), 59 deletions(-)
>
> --
> 1.9.1
>
^ permalink raw reply [flat|nested] 26+ messages in thread
* [PATCHv2 5/5] efi/runtime-wrappers: detect FW irq flag corruption
2016-04-21 11:35 [PATCHv2 0/5] efi: detect erroneous firmware IRQ manipulation Mark Rutland
` (3 preceding siblings ...)
[not found] ` <1461238529-12810-1-git-send-email-mark.rutland-5wv7dgnIgG8@public.gmane.org>
@ 2016-04-21 11:35 ` Mark Rutland
[not found] ` <1461238529-12810-6-git-send-email-mark.rutland-5wv7dgnIgG8@public.gmane.org>
4 siblings, 1 reply; 26+ messages in thread
From: Mark Rutland @ 2016-04-21 11:35 UTC (permalink / raw)
To: linux-efi
Cc: ard.biesheuvel, catalin.marinas, hpa, leif.lindholm, linux,
mark.rutland, matt, mingo, tglx, will.deacon, linux-kernel
The UEFI spec allows runtime services to be called with interrupts
masked or unmasked, and if a runtime service function needs to mask
interrupts, it must restore the mask to its original state before
returning (i.e. from the PoV of the OS, this does not change across a
call). Firmware should never unmask exceptions, as these may then be
taken by the OS unexpectedly.
Unfortunately, some firmware has been seen to unmask IRQs (and
potentially other maskable exceptions) across runtime services calls,
leaving irq flags corrupted after returning from a runtime services
function call. This may be detected by the IRQ tracing code, but often
goes unnoticed, leaving a potentially disastrous bug hidden.
This patch detects when the irq flags are corrupted by an EFI runtime
services call, logging the call and specific corruption to the console.
While restoring the expected value of the flags is insufficient to avoid
problems, we do so to avoid redundant warnings from elsewhere (e.g. IRQ
tracing).
Signed-off-by: Mark Rutland <mark.rutland@arm.com>
Cc: Matt Fleming <matt@codeblueprint.co.uk>
Cc: linux-efi@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
---
drivers/firmware/efi/runtime-wrappers.c | 22 ++++++++++++++++++++++
1 file changed, 22 insertions(+)
diff --git a/drivers/firmware/efi/runtime-wrappers.c b/drivers/firmware/efi/runtime-wrappers.c
index 1b9fa54..aeb65f2 100644
--- a/drivers/firmware/efi/runtime-wrappers.c
+++ b/drivers/firmware/efi/runtime-wrappers.c
@@ -16,8 +16,10 @@
#include <linux/bug.h>
#include <linux/efi.h>
+#include <linux/irqflags.h>
#include <linux/mutex.h>
#include <linux/spinlock.h>
+#include <linux/stringify.h>
#include <asm/efi.h>
@@ -25,8 +27,11 @@
#define efi_call_virt(f, args...) \
({ \
efi_status_t __s; \
+ unsigned long flags; \
arch_efi_call_virt_setup(); \
+ local_save_flags(flags); \
__s = arch_efi_call_virt(f, args); \
+ efi_call_virt_check_flags(flags, __stringify(f)); \
arch_efi_call_virt_teardown(); \
__s; \
})
@@ -35,12 +40,29 @@
#ifndef __efi_call_virt
#define __efi_call_virt(f, args...) \
({ \
+ unsigned long flags; \
arch_efi_call_virt_setup(); \
+ local_irq_save(flags); \
arch_efi_call_virt(f, args); \
+ efi_call_virt_check_flags(flags, __stringify(f)); \
arch_efi_call_virt_teardown(); \
})
#endif
+static void efi_call_virt_check_flags(unsigned long flags, const char *call)
+{
+ unsigned long cur_flags;
+
+ local_save_flags(cur_flags);
+ if (!WARN_ON_ONCE(cur_flags != flags))
+ return;
+
+ 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);
+}
+
/*
* 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
--
1.9.1
^ permalink raw reply related [flat|nested] 26+ messages in thread