Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754723AbcDYOMJ (ORCPT ); Mon, 25 Apr 2016 10:12:09 -0400 Received: from foss.arm.com ([217.140.101.70]:46749 "EHLO foss.arm.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1754197AbcDYOMG (ORCPT ); Mon, 25 Apr 2016 10:12:06 -0400 Subject: Re: [PATCHv3 1/5] efi/runtime-wrappers: detect FW irq flag corruption To: Mark Rutland , linux-efi@vger.kernel.org References: <1461591994-14918-1-git-send-email-mark.rutland@arm.com> <1461591994-14918-2-git-send-email-mark.rutland@arm.com> Cc: linux@arm.linux.org.uk, ard.biesheuvel@linaro.org, matt@codeblueprint.co.uk, catalin.marinas@arm.com, will.deacon@arm.com, linux-kernel@vger.kernel.org, leif.lindholm@linaro.org, mingo@redhat.com, hpa@zytor.com, tglx@linutronix.de, linux-arm-kernel@lists.infradead.org From: Robin Murphy Message-ID: <571E25B1.8070305@arm.com> Date: Mon, 25 Apr 2016 15:12:01 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.6.0 MIME-Version: 1.0 In-Reply-To: <1461591994-14918-2-git-send-email-mark.rutland@arm.com> Content-Type: text/plain; charset=windows-1252; format=flowed Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 3865 Lines: 104 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 > Cc: Ard Biesheuvel > Cc: Matt Fleming > 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 > #include > +#include > #include > #include > +#include > #include > > +/* > + * 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(); \ > }) > >