From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from bombadil.infradead.org (bombadil.infradead.org [198.137.202.133]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id A6C99C43217 for ; Wed, 9 Nov 2022 14:44:15 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20210309; h=Sender: Content-Transfer-Encoding:Content-Type:List-Subscribe:List-Help:List-Post: List-Archive:List-Unsubscribe:List-Id:In-Reply-To:MIME-Version:References: Message-ID:Subject:Cc:To:From:Date:Reply-To:Content-ID:Content-Description: Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID: List-Owner; bh=/l9Vh4ktjGpJr3LcMNZ7rBCsjctJEeqA7S6qUiyZpUc=; b=vV0TukdKScDBIE Vhmdvvk1nRLPbRkdlVJNgyGx/TLAESIqpg24PpCMhjh1svidzHPr/O8mFiVw4AIzMUsn5PQjrOM8e Z1Nc6mklpq3h47796FUrxujYyOquYBe7xttqMAPN+FK6CIFUT++vRyvdrADR67bt3SEXNZFtYqoto lw2P3SjWqS/jYQlN9jOk34PV1BY0Wx3im+FQIaZUppmOsYmbSoGheakk77zBiDAaSkUlJJZNc6Q6A i53PxZLRGJ6H7pCamZUkKvEWJPiV3oxvcW/tITsl1boMPxMuzeccLejx6evJwhZThAf2/DGcuacBn fplwsENYCzyagxt/SJDQ==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.94.2 #2 (Red Hat Linux)) id 1osmI2-00ED1r-G8; Wed, 09 Nov 2022 14:42:54 +0000 Received: from foss.arm.com ([217.140.110.172]) by bombadil.infradead.org with esmtp (Exim 4.94.2 #2 (Red Hat Linux)) id 1osmHy-00ECzb-U2 for linux-arm-kernel@lists.infradead.org; Wed, 09 Nov 2022 14:42:53 +0000 Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.121.207.14]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id D267B1FB; Wed, 9 Nov 2022 06:42:50 -0800 (PST) Received: from monolith.localdoman (unknown [172.31.20.19]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id A97263F73D; Wed, 9 Nov 2022 06:42:43 -0800 (PST) Date: Wed, 9 Nov 2022 14:42:41 +0000 From: Alexandru Elisei To: Ard Biesheuvel Cc: linux-efi@vger.kernel.org, linux-arm-kernel@lists.infradead.org, catalin.marinas@arm.com, will@kernel.org Subject: Re: [PATCH] arm64: efi: Disable only the misbehaving runtime service on sync exceptions Message-ID: References: <20221108151509.2250968-1-ardb@kernel.org> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <20221108151509.2250968-1-ardb@kernel.org> X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20221109_064251_089377_951CCBBB X-CRM114-Status: GOOD ( 32.70 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org Hi, On Tue, Nov 08, 2022 at 04:15:09PM +0100, Ard Biesheuvel wrote: > Alexandru reports that his Ampere Altra machine, whose buggy firmware > triggers a synchronous exception in its implementation of SetTime() when > called without SetVirtualAddressMap() having been called first, doesn't > quite recover from this, and starts spewing error messages into the log > that are unrelated to the buggy runtime service. > > The driver in question is the EFI RTC driver, which should be fixed in > any case, as flooding the log like that (or doing any logging to the > kernel log at all on something whuch is not a severe issue) is not ok. > > However, in this particular case, it would be beneficial for both > ordinary use as well as diagnostics regarding broken firmware if we only > prevent the broken runtime service from being called again, and permit > others (such as GetTime() which triggers the logging or the variable > services) from being used as normal. > > So wire up the existing efi.runtime_supported_mask, and clear the > service's bit in the mask if the generic runtime service wrapper > observes a return value of EFI_ABORTED, which only happens if a service > call is aborted due to an exception. (EFI_ABORTED is not documented as a > valid error code for any of the EFI runtime services). With a kernel built from v6.1-rc4, when doing efibootmgr after the EFI panic happens (so with runtime services disabled), this is what I get: # efibootmgr Skipping unreadable variable "Boot0001": Interrupted system call Skipping unreadable variable "Boot0002": Interrupted system call show_order(): Interrupted system call And dmesg shows: [ 55.941312] efi: EFI Runtime Services are disabled! With this patch on top of v6.1-rc4: # efibootmgr Skipping unreadable variable "Boot0001": Invalid argument Skipping unreadable variable "Boot0002": Invalid argument show_order(): Invalid argument Same thing happens if I cat the Boot001 efivars file. Nothing is printed on dmesg. Changed efi_call_rts() to print the return value, status is 0x8000_0000_0000_000f (or 15 in decimal if casted into an int). Tried to debug further, but I'm not familiar with all the structs and what they represent (for example, efi_call_virt(get_variable, args) calls efi_call_virt_pointer(efi.runtime, get_variable, args), does it end up as __efi_rt_asm_wrapper((efi.runtime)->get_variable, "get_variable", args?) As an aside, it would be really helpful to document the arguments for __efi_rt_asm_wrapper. Pointers here how to debug further would be very welcome. Thanks, Alex > > Cc: Alexandru Elisei > Signed-off-by: Ard Biesheuvel > --- > arch/arm64/kernel/efi.c | 1 - > arch/x86/platform/efi/quirks.c | 2 +- > drivers/firmware/efi/runtime-wrappers.c | 6 ++++-- > include/linux/efi.h | 24 ++++++++++++------------ > 4 files changed, 17 insertions(+), 16 deletions(-) > > diff --git a/arch/arm64/kernel/efi.c b/arch/arm64/kernel/efi.c > index ee53f2a0aa03f54a..a3b8852f2698d9f7 100644 > --- a/arch/arm64/kernel/efi.c > +++ b/arch/arm64/kernel/efi.c > @@ -153,7 +153,6 @@ asmlinkage efi_status_t __efi_rt_asm_recover(void); > asmlinkage efi_status_t efi_handle_runtime_exception(const char *f) > { > pr_err(FW_BUG "Synchronous exception occurred in EFI runtime service %s()\n", f); > - clear_bit(EFI_RUNTIME_SERVICES, &efi.flags); > return EFI_ABORTED; > } > > diff --git a/arch/x86/platform/efi/quirks.c b/arch/x86/platform/efi/quirks.c > index b0b848d6933afbcf..3f48fdf42d97dc1f 100644 > --- a/arch/x86/platform/efi/quirks.c > +++ b/arch/x86/platform/efi/quirks.c > @@ -757,9 +757,9 @@ void efi_crash_gracefully_on_page_fault(unsigned long phys_addr) > > /* Signal error status to the efi caller process */ > efi_rts_work.status = EFI_ABORTED; > + efi.runtime_supported_mask = 0; > complete(&efi_rts_work.efi_rts_comp); > > - clear_bit(EFI_RUNTIME_SERVICES, &efi.flags); > pr_info("Froze efi_rts_wq and disabled EFI Runtime Services\n"); > > /* > diff --git a/drivers/firmware/efi/runtime-wrappers.c b/drivers/firmware/efi/runtime-wrappers.c > index f3e54f6616f02475..336b8bcec86d0127 100644 > --- a/drivers/firmware/efi/runtime-wrappers.c > +++ b/drivers/firmware/efi/runtime-wrappers.c > @@ -60,8 +60,8 @@ struct efi_runtime_work efi_rts_work; > ({ \ > efi_rts_work.status = EFI_ABORTED; \ > \ > - if (!efi_enabled(EFI_RUNTIME_SERVICES)) { \ > - pr_warn_once("EFI Runtime Services are disabled!\n"); \ > + if (!efi_rt_services_supported(efi_rts_work.efi_rts_id)) { \ > + pr_warn_ratelimited("EFI Runtime Service is disabled!\n");\ > goto exit; \ > } \ > \ > @@ -83,6 +83,8 @@ struct efi_runtime_work efi_rts_work; > else \ > pr_err("Failed to queue work to efi_rts_wq.\n"); \ > \ > + if (efi_rts_work.status == EFI_ABORTED) \ > + efi.runtime_supported_mask &= ~efi_rts_work.efi_rts_id; \ > exit: \ > efi_rts_work.efi_rts_id = EFI_NONE; \ > efi_rts_work.status; \ > diff --git a/include/linux/efi.h b/include/linux/efi.h > index 929d559ad41d29c6..61b252386d61cc4d 100644 > --- a/include/linux/efi.h > +++ b/include/linux/efi.h > @@ -1254,18 +1254,18 @@ extern unsigned long rci2_table_phys; > */ > enum efi_rts_ids { > EFI_NONE, > - EFI_GET_TIME, > - EFI_SET_TIME, > - EFI_GET_WAKEUP_TIME, > - EFI_SET_WAKEUP_TIME, > - EFI_GET_VARIABLE, > - EFI_GET_NEXT_VARIABLE, > - EFI_SET_VARIABLE, > - EFI_QUERY_VARIABLE_INFO, > - EFI_GET_NEXT_HIGH_MONO_COUNT, > - EFI_RESET_SYSTEM, > - EFI_UPDATE_CAPSULE, > - EFI_QUERY_CAPSULE_CAPS, > + EFI_GET_TIME = EFI_RT_SUPPORTED_GET_TIME, > + EFI_SET_TIME = EFI_RT_SUPPORTED_SET_TIME, > + EFI_GET_WAKEUP_TIME = EFI_RT_SUPPORTED_GET_WAKEUP_TIME, > + EFI_SET_WAKEUP_TIME = EFI_RT_SUPPORTED_SET_WAKEUP_TIME, > + EFI_GET_VARIABLE = EFI_RT_SUPPORTED_GET_VARIABLE, > + EFI_GET_NEXT_VARIABLE = EFI_RT_SUPPORTED_GET_NEXT_VARIABLE_NAME, > + EFI_SET_VARIABLE = EFI_RT_SUPPORTED_SET_VARIABLE, > + EFI_GET_NEXT_HIGH_MONO_COUNT = EFI_RT_SUPPORTED_GET_NEXT_HIGH_MONOTONIC_COUNT, > + EFI_RESET_SYSTEM = EFI_RT_SUPPORTED_RESET_SYSTEM, > + EFI_UPDATE_CAPSULE = EFI_RT_SUPPORTED_UPDATE_CAPSULE, > + EFI_QUERY_CAPSULE_CAPS = EFI_RT_SUPPORTED_QUERY_CAPSULE_CAPABILITIES, > + EFI_QUERY_VARIABLE_INFO = EFI_RT_SUPPORTED_QUERY_VARIABLE_INFO, > }; > > /* > -- > 2.35.1 > _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel