Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]
Groups > linux.kernel > #1386464 > unrolled thread
| Started by | Mark Rutland <mark.rutland@arm.com> |
|---|---|
| First post | 2016-04-25 15:50 +0200 |
| Last post | 2016-04-25 18:10 +0200 |
| Articles | 11 — 4 participants |
Back to article view | Back to linux.kernel
[PATCHv3 0/5] efi: detect erroneous firmware IRQ manipulation Mark Rutland <mark.rutland@arm.com> - 2016-04-25 15:50 +0200
[PATCHv3 3/5] arm/efi: enable runtime call flag checking Mark Rutland <mark.rutland@arm.com> - 2016-04-25 15:50 +0200
[PATCHv3 1/5] efi/runtime-wrappers: detect FW irq flag corruption Mark Rutland <mark.rutland@arm.com> - 2016-04-25 15:50 +0200
Re: [PATCHv3 1/5] efi/runtime-wrappers: detect FW irq flag corruption Robin Murphy <robin.murphy@arm.com> - 2016-04-25 16:20 +0200
Re: [PATCHv3 1/5] efi/runtime-wrappers: detect FW irq flag corruption Matt Fleming <matt@codeblueprint.co.uk> - 2016-04-25 16:20 +0200
Re: [PATCHv3 1/5] efi/runtime-wrappers: detect FW irq flag corruption Ard Biesheuvel <ard.biesheuvel@linaro.org> - 2016-04-25 16:20 +0200
Re: [PATCHv3 1/5] efi/runtime-wrappers: detect FW irq flag corruption Matt Fleming <matt@codeblueprint.co.uk> - 2016-04-25 16:30 +0200
Re: [PATCHv3 1/5] efi/runtime-wrappers: detect FW irq flag corruption Mark Rutland <mark.rutland@arm.com> - 2016-04-25 16:30 +0200
Re: [PATCHv3 1/5] efi/runtime-wrappers: detect FW irq flag corruption Matt Fleming <matt@codeblueprint.co.uk> - 2016-04-25 18:00 +0200
Re: [PATCHv3 1/5] efi/runtime-wrappers: detect FW irq flag corruption Mark Rutland <mark.rutland@arm.com> - 2016-04-25 18:10 +0200
Re: [PATCHv3 0/5] efi: detect erroneous firmware IRQ manipulation Matt Fleming <matt@codeblueprint.co.uk> - 2016-04-25 18:10 +0200
| From | Mark Rutland <mark.rutland@arm.com> |
|---|---|
| Date | 2016-04-25 15:50 +0200 |
| Subject | [PATCHv3 0/5] efi: detect erroneous firmware IRQ manipulation |
| Message-ID | <rrP3I-6Kr-3@gated-at.bofh.it> |
Note: this is largely a rework of the final patch from v2 [2], which now has a per-arch component (and hence additional patches). The rest of v2 has already been picked up, and hence dropped from this posting. 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 (patches 1-5 of [2]). I have left ia64 as-is, without this check, as ia64 doesn't currently use the generic runtime wrappers, 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. Since v1 [1]: * Fix thinko: s/local_irq_save/local_save_flags/ * Remove ifdefs after conversion * Remove reundant semicolon from x86 patch * Move efi_call_virt_check_flags before first use * Add Acked-bys and Reviewed-bys Since v2 [2]: * Drop the refactoring patches (1-5), which Matt has queued * Add per-arch ARCH_EFI_IRQ_FLAGS_MASK, as discussed for v2 [3,4] As with v2, this has been build-tested for each target, but other than arm64 I don't have a good platform for testing. Hopefully this causes fewer explosions than v2. Thanks, Mark. [1] https://lkml.org/lkml/2016/4/21/260 [2] https://lkml.org/lkml/2016/4/22/542 [3] https://lkml.org/lkml/2016/4/25/230 [4] https://lkml.org/lkml/2016/4/25/243 Mark Rutland (5): efi/runtime-wrappers: detect FW irq flag corruption arm64/efi: enable runtime call flag checking arm/efi: enable runtime call flag checking x86/efi: enable runtime call flag checking efi/runtime-wrappers: remove ARCH_EFI_IRQ_FLAGS_MASK ifdef arch/arm/include/asm/efi.h | 5 +++++ arch/arm64/include/asm/efi.h | 3 +++ arch/x86/include/asm/efi.h | 4 +++- drivers/firmware/efi/runtime-wrappers.c | 25 +++++++++++++++++++++++++ 4 files changed, 36 insertions(+), 1 deletion(-) -- 1.9.1
[toc] | [next] | [standalone]
| From | Mark Rutland <mark.rutland@arm.com> |
|---|---|
| Date | 2016-04-25 15:50 +0200 |
| Subject | [PATCHv3 3/5] arm/efi: enable runtime call flag checking |
| Message-ID | <rrP3K-6Kr-45@gated-at.bofh.it> |
| In reply to | #1386464 |
Define ARCH_EFI_IRQ_FLAGS_MASK for arm, which will enable the generic
runtime wrapper code to detect when firmware erroneously modifies flags
over a runtime services function call.
We check all allocated flags, barring those which firmware has
legitimate reason to modify (condition flags and IT state). While in
practice corruption of some flags (e.g. J) would already be fatal, we
include these for consistency and documentation purposes.
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
---
arch/arm/include/asm/efi.h | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/arch/arm/include/asm/efi.h b/arch/arm/include/asm/efi.h
index e1461fc..b05ff68 100644
--- a/arch/arm/include/asm/efi.h
+++ b/arch/arm/include/asm/efi.h
@@ -17,6 +17,7 @@
#include <asm/mach/map.h>
#include <asm/mmu_context.h>
#include <asm/pgtable.h>
+#include <asm/ptrace.h>
#ifdef CONFIG_EFI
void efi_init(void);
@@ -33,6 +34,10 @@ int efi_create_mapping(struct mm_struct *mm, efi_memory_desc_t *md);
__f(args); \
})
+#define ARCH_EFI_IRQ_FLAGS_MASK \
+ (PSR_J_BIT | PSR_E_BIT | PSR_A_BIT | PSR_I_BIT | PSR_F_BIT | \
+ PSR_T_BIT | MODE_MASK)
+
static inline void efi_set_pgd(struct mm_struct *mm)
{
check_and_switch_context(mm, NULL);
--
1.9.1
[toc] | [prev] | [next] | [standalone]
| From | Mark Rutland <mark.rutland@arm.com> |
|---|---|
| Date | 2016-04-25 15:50 +0200 |
| Subject | [PATCHv3 1/5] efi/runtime-wrappers: detect FW irq flag corruption |
| Message-ID | <rrP3K-6Kr-47@gated-at.bofh.it> |
| In reply to | #1386464 |
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).
The set of bits in flags which we want to check is architecture-specific
(e.g. we want to check FIQ on arm64, but not the zero flag on x86), so
each arch must provide ARCH_EFI_IRQ_FLAGS_MASK to describe those. In the
absence of this mask, the check is a no-op, and we redundantly save the
flags twice, but that will be short-lived as subsequent patches
will implement this and remove the scaffolding.
Signed-off-by: Mark Rutland <mark.rutland@arm.com>
Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Cc: Matt Fleming <matt@codeblueprint.co.uk>
Cc: linux-efi@vger.kernel.org
---
drivers/firmware/efi/runtime-wrappers.c | 32 ++++++++++++++++++++++++++++++++
1 file changed, 32 insertions(+)
diff --git a/drivers/firmware/efi/runtime-wrappers.c b/drivers/firmware/efi/runtime-wrappers.c
index a2c8e70..1f0277e 100644
--- a/drivers/firmware/efi/runtime-wrappers.c
+++ b/drivers/firmware/efi/runtime-wrappers.c
@@ -16,23 +16,55 @@
#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>
+/*
+ * Temporary scaffolding until all users provide ARCH_EFI_IRQ_FLAGS_MASK.
+ */
+#ifdef ARCH_EFI_IRQ_FLAGS_MASK
+static void efi_call_virt_check_flags(unsigned long flags, const char *call)
+{
+ unsigned long cur_flags;
+ bool mismatch;
+
+ local_save_flags(cur_flags);
+
+ mismatch = !!((cur_flags ^ flags) & ARCH_EFI_IRQ_FLAGS_MASK);
+ if (!WARN_ON_ONCE(mismatch))
+ 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);
+}
+#else /* ARCH_EFI_IRQ_FLAGS_MASK */
+static inline void efi_call_virt_check_flags(unsigned long flags, const char *call) {}
+#endif /* ARCH_EFI_IRQ_FLAGS_MASK */
+
#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; \
})
#define __efi_call_virt(f, args...) \
({ \
+ unsigned long flags; \
arch_efi_call_virt_setup(); \
+ local_save_flags(flags); \
arch_efi_call_virt(f, args); \
+ efi_call_virt_check_flags(flags, __stringify(f)); \
arch_efi_call_virt_teardown(); \
})
--
1.9.1
[toc] | [prev] | [next] | [standalone]
| From | Robin Murphy <robin.murphy@arm.com> |
|---|---|
| Date | 2016-04-25 16:20 +0200 |
| Subject | Re: [PATCHv3 1/5] efi/runtime-wrappers: detect FW irq flag corruption |
| Message-ID | <rrPwJ-7gP-1@gated-at.bofh.it> |
| In reply to | #1386466 |
Hi Mark,
On 25/04/16 14:46, Mark Rutland wrote:
> 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).
>
> The set of bits in flags which we want to check is architecture-specific
> (e.g. we want to check FIQ on arm64, but not the zero flag on x86), so
> each arch must provide ARCH_EFI_IRQ_FLAGS_MASK to describe those. In the
> absence of this mask, the check is a no-op, and we redundantly save the
> flags twice, but that will be short-lived as subsequent patches
> will implement this and remove the scaffolding.
>
> Signed-off-by: Mark Rutland <mark.rutland@arm.com>
> Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> Cc: Matt Fleming <matt@codeblueprint.co.uk>
> Cc: linux-efi@vger.kernel.org
> ---
> drivers/firmware/efi/runtime-wrappers.c | 32 ++++++++++++++++++++++++++++++++
> 1 file changed, 32 insertions(+)
>
> diff --git a/drivers/firmware/efi/runtime-wrappers.c b/drivers/firmware/efi/runtime-wrappers.c
> index a2c8e70..1f0277e 100644
> --- a/drivers/firmware/efi/runtime-wrappers.c
> +++ b/drivers/firmware/efi/runtime-wrappers.c
> @@ -16,23 +16,55 @@
>
> #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>
>
> +/*
> + * Temporary scaffolding until all users provide ARCH_EFI_IRQ_FLAGS_MASK.
> + */
> +#ifdef ARCH_EFI_IRQ_FLAGS_MASK
> +static void efi_call_virt_check_flags(unsigned long flags, const char *call)
> +{
> + unsigned long cur_flags;
> + bool mismatch;
> +
> + local_save_flags(cur_flags);
> +
> + mismatch = !!((cur_flags ^ flags) & ARCH_EFI_IRQ_FLAGS_MASK);
nit: the assignment itself is already a conversion to bool, so the
excitement is redundant here.
Robin.
> + if (!WARN_ON_ONCE(mismatch))
> + 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);
> +}
> +#else /* ARCH_EFI_IRQ_FLAGS_MASK */
> +static inline void efi_call_virt_check_flags(unsigned long flags, const char *call) {}
> +#endif /* ARCH_EFI_IRQ_FLAGS_MASK */
> +
> #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; \
> })
>
> #define __efi_call_virt(f, args...) \
> ({ \
> + unsigned long flags; \
> arch_efi_call_virt_setup(); \
> + local_save_flags(flags); \
> arch_efi_call_virt(f, args); \
> + efi_call_virt_check_flags(flags, __stringify(f)); \
> arch_efi_call_virt_teardown(); \
> })
>
>
[toc] | [prev] | [next] | [standalone]
| From | Matt Fleming <matt@codeblueprint.co.uk> |
|---|---|
| Date | 2016-04-25 16:20 +0200 |
| Subject | Re: [PATCHv3 1/5] efi/runtime-wrappers: detect FW irq flag corruption |
| Message-ID | <rrPwK-7gP-17@gated-at.bofh.it> |
| In reply to | #1386492 |
On Mon, 25 Apr, at 03:12:01PM, Robin Murphy wrote:
> >+static void efi_call_virt_check_flags(unsigned long flags, const char *call)
> >+{
> >+ unsigned long cur_flags;
> >+ bool mismatch;
> >+
> >+ local_save_flags(cur_flags);
> >+
> >+ mismatch = !!((cur_flags ^ flags) & ARCH_EFI_IRQ_FLAGS_MASK);
>
> nit: the assignment itself is already a conversion to bool, so the
> excitement is redundant here.
This was intentional. I asked Mark to make this change so that it's
explicit for the developer that we're performing the type conversion.
[toc] | [prev] | [next] | [standalone]
| From | Ard Biesheuvel <ard.biesheuvel@linaro.org> |
|---|---|
| Date | 2016-04-25 16:20 +0200 |
| Subject | Re: [PATCHv3 1/5] efi/runtime-wrappers: detect FW irq flag corruption |
| Message-ID | <rrPwK-7gP-31@gated-at.bofh.it> |
| In reply to | #1386496 |
On 25 April 2016 at 16:15, Matt Fleming <matt@codeblueprint.co.uk> wrote:
> On Mon, 25 Apr, at 03:12:01PM, Robin Murphy wrote:
>> >+static void efi_call_virt_check_flags(unsigned long flags, const char *call)
>> >+{
>> >+ unsigned long cur_flags;
>> >+ bool mismatch;
>> >+
>> >+ local_save_flags(cur_flags);
>> >+
>> >+ mismatch = !!((cur_flags ^ flags) & ARCH_EFI_IRQ_FLAGS_MASK);
>>
>> nit: the assignment itself is already a conversion to bool, so the
>> excitement is redundant here.
>
> This was intentional. I asked Mark to make this change so that it's
> explicit for the developer that we're performing the type conversion.
But replacing an implicit boolean cast with an explicit one makes
little sense, no? Don't we simply want '!= 0' here if you need a
boolean expression?
[toc] | [prev] | [next] | [standalone]
| From | Matt Fleming <matt@codeblueprint.co.uk> |
|---|---|
| Date | 2016-04-25 16:30 +0200 |
| Subject | Re: [PATCHv3 1/5] efi/runtime-wrappers: detect FW irq flag corruption |
| Message-ID | <rrPGq-7oh-33@gated-at.bofh.it> |
| In reply to | #1386503 |
On Mon, 25 Apr, at 04:18:41PM, Ard Biesheuvel wrote:
> On 25 April 2016 at 16:15, Matt Fleming <matt@codeblueprint.co.uk> wrote:
> > On Mon, 25 Apr, at 03:12:01PM, Robin Murphy wrote:
> >> >+static void efi_call_virt_check_flags(unsigned long flags, const char *call)
> >> >+{
> >> >+ unsigned long cur_flags;
> >> >+ bool mismatch;
> >> >+
> >> >+ local_save_flags(cur_flags);
> >> >+
> >> >+ mismatch = !!((cur_flags ^ flags) & ARCH_EFI_IRQ_FLAGS_MASK);
> >>
> >> nit: the assignment itself is already a conversion to bool, so the
> >> excitement is redundant here.
> >
> > This was intentional. I asked Mark to make this change so that it's
> > explicit for the developer that we're performing the type conversion.
>
> But replacing an implicit boolean cast with an explicit one makes
> little sense, no? Don't we simply want '!= 0' here if you need a
> boolean expression?
Aha but '!!' is fewer characters to type!!
I'm not that bothered as long as we don't stuff an int into a bool
without giving the programmer some idea we're doing that. It's not
about the compiler getting it wrong, more about a developer
introducing a bug when they change the code in the future.
Unless anyone objects, I'll fix this up to use '!= 0' when I apply it.
[toc] | [prev] | [next] | [standalone]
| From | Mark Rutland <mark.rutland@arm.com> |
|---|---|
| Date | 2016-04-25 16:30 +0200 |
| Subject | Re: [PATCHv3 1/5] efi/runtime-wrappers: detect FW irq flag corruption |
| Message-ID | <rrPGq-7oh-37@gated-at.bofh.it> |
| In reply to | #1386512 |
On Mon, Apr 25, 2016 at 03:24:35PM +0100, Matt Fleming wrote:
> On Mon, 25 Apr, at 04:18:41PM, Ard Biesheuvel wrote:
> > On 25 April 2016 at 16:15, Matt Fleming <matt@codeblueprint.co.uk> wrote:
> > > On Mon, 25 Apr, at 03:12:01PM, Robin Murphy wrote:
> > >> >+static void efi_call_virt_check_flags(unsigned long flags, const char *call)
> > >> >+{
> > >> >+ unsigned long cur_flags;
> > >> >+ bool mismatch;
> > >> >+
> > >> >+ local_save_flags(cur_flags);
> > >> >+
> > >> >+ mismatch = !!((cur_flags ^ flags) & ARCH_EFI_IRQ_FLAGS_MASK);
> > >>
> > >> nit: the assignment itself is already a conversion to bool, so the
> > >> excitement is redundant here.
> > >
> > > This was intentional. I asked Mark to make this change so that it's
> > > explicit for the developer that we're performing the type conversion.
> >
> > But replacing an implicit boolean cast with an explicit one makes
> > little sense, no? Don't we simply want '!= 0' here if you need a
> > boolean expression?
>
> Aha but '!!' is fewer characters to type!!
>
> I'm not that bothered as long as we don't stuff an int into a bool
> without giving the programmer some idea we're doing that. It's not
> about the compiler getting it wrong, more about a developer
> introducing a bug when they change the code in the future.
>
> Unless anyone objects, I'll fix this up to use '!= 0' when I apply it.
I have no strong preference so long as the code is correct.
Another option is to get rid of the bool entirely:
flags ^= cur_flags;
if (!WARN_ON(flags & ARCH_EFI_IRQ_FLAGS_MASK))
return;
Mark.
[toc] | [prev] | [next] | [standalone]
| From | Matt Fleming <matt@codeblueprint.co.uk> |
|---|---|
| Date | 2016-04-25 18:00 +0200 |
| Subject | Re: [PATCHv3 1/5] efi/runtime-wrappers: detect FW irq flag corruption |
| Message-ID | <rrR5x-8ma-23@gated-at.bofh.it> |
| In reply to | #1386514 |
On Mon, 25 Apr, at 03:27:35PM, Mark Rutland wrote: > > I have no strong preference so long as the code is correct. > > Another option is to get rid of the bool entirely: > > flags ^= cur_flags; > if (!WARN_ON(flags & ARCH_EFI_IRQ_FLAGS_MASK)) > return; OK, let's do the following because we need flags to be preserved for printing, --- unsigned long cur_flags, mismatch; local_save_flags(cur_flags); mismatch = flags ^ cur_flags; if (!WARN_ON_ONCE(mismatch & ARCH_EFI_IRQ_FLAGS_MASK)) return;
[toc] | [prev] | [next] | [standalone]
| From | Mark Rutland <mark.rutland@arm.com> |
|---|---|
| Date | 2016-04-25 18:10 +0200 |
| Subject | Re: [PATCHv3 1/5] efi/runtime-wrappers: detect FW irq flag corruption |
| Message-ID | <rrRfd-hZ-29@gated-at.bofh.it> |
| In reply to | #1386615 |
On Mon, Apr 25, 2016 at 04:59:22PM +0100, Matt Fleming wrote: > On Mon, 25 Apr, at 03:27:35PM, Mark Rutland wrote: > > > > I have no strong preference so long as the code is correct. > > > > Another option is to get rid of the bool entirely: > > > > flags ^= cur_flags; > > if (!WARN_ON(flags & ARCH_EFI_IRQ_FLAGS_MASK)) > > return; > > OK, let's do the following because we need flags to be preserved for > printing, > > --- > > unsigned long cur_flags, mismatch; > > local_save_flags(cur_flags); > > mismatch = flags ^ cur_flags; > if (!WARN_ON_ONCE(mismatch & ARCH_EFI_IRQ_FLAGS_MASK)) > return; > Sure; that looks good to me. Cheers, Mark.
[toc] | [prev] | [next] | [standalone]
| From | Matt Fleming <matt@codeblueprint.co.uk> |
|---|---|
| Date | 2016-04-25 18:10 +0200 |
| Message-ID | <rrRfd-hZ-25@gated-at.bofh.it> |
| In reply to | #1386464 |
On Mon, 25 Apr, at 02:46:29PM, Mark Rutland wrote: > Note: this is largely a rework of the final patch from v2 [2], which now has a > per-arch component (and hence additional patches). The rest of v2 has already > been picked up, and hence dropped from this posting. > > 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 (patches 1-5 of [2]). > > I have left ia64 as-is, without this check, as ia64 doesn't currently use the > generic runtime wrappers, 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. Thanks Mark. I've picked up the series and applied it to the v4.7 queue.
[toc] | [prev] | [standalone]
Back to top | Article view | linux.kernel
csiph-web