From patchwork Fri Mar 11 19:20:17 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: David Woodhouse X-Patchwork-Id: 551056 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 vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id 2CFD1C433EF for ; Fri, 11 Mar 2022 19:20:27 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1345996AbiCKTV3 (ORCPT ); Fri, 11 Mar 2022 14:21:29 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:51442 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S236052AbiCKTV2 (ORCPT ); Fri, 11 Mar 2022 14:21:28 -0500 Received: from casper.infradead.org (casper.infradead.org [IPv6:2001:8b0:10b:1236::1]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 81A6381884; Fri, 11 Mar 2022 11:20:21 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=infradead.org; s=casper.20170209; h=MIME-Version:Content-Type:References: In-Reply-To:Date:Cc:To:From:Subject:Message-ID:Sender:Reply-To: Content-Transfer-Encoding:Content-ID:Content-Description; bh=t1MNIGj+KFYZG9aGwrBZTTH39NuRFX1gyJctTHRiY/k=; b=fvXRlXcGfl/VX/YthinvXJg1hk h19PZCHCvRYF1ozQuWq3u9FNwYV85s9hhqx73y9M/usi2qnDBkySmj8IDj/PoPNCZE3/g1KZMz/jk hzd6Ygvi5cptER9ujtfszby5tUymex/OiOaY/MRN5rvV5USxUrKO7IzkXCiO8FsfQu2YAeSMklBpD dXhT9D90e3kghbuzKLieDTLNRXpHjOcjepT6EbyiMWj77rdj5jv/3aD+6vMDjLRhn1HC8QvgRnsos xUnqvK4QJrKb99HG8jSdDLyEtUyetLFb8tMvyGvsiSZJEJ5AB80xPpq/wrtMEbn3T+NdH1MosgyA2 ux2f1uyw==; Received: from [2001:8b0:10b:1::3ae] (helo=u3832b3a9db3152.infradead.org) by casper.infradead.org with esmtpsa (Exim 4.94.2 #2 (Red Hat Linux)) id 1nSkoD-001qkO-VK; Fri, 11 Mar 2022 19:20:18 +0000 Message-ID: <9099d8903e9b2b16daec712acc9aa533fe84d102.camel@infradead.org> Subject: [PATCH] PM / hibernate: Honour ACPI hardware signature by default for virtual guests From: David Woodhouse To: "Rafael J. Wysocki" Cc: linux-pm , linux-kernel , benh , "van der Linden, Frank" , Amit Shah Date: Fri, 11 Mar 2022 19:20:17 +0000 In-Reply-To: References: <26decf155bffc021a97846c0a0ed09c2b5e0bef1.camel@infradead.org> User-Agent: Evolution 3.36.5-0ubuntu1 MIME-Version: 1.0 X-SRS-Rewrite: SMTP reverse-path rewritten from by casper.infradead.org. See http://www.infradead.org/rpr.html Precedence: bulk List-ID: X-Mailing-List: linux-pm@vger.kernel.org From: David Woodhouse The ACPI specification says that OSPM should refuse to restore from hibernate if the hardware signature changes, and should boot from scratch. However, real BIOSes often vary the hardware signature in cases where we *do* want to resume from hibernate, so Linux doesn't follow the spec by default. However, in a virtual environment there's no reason for the VMM to vary the hardware signature *unless* it wants to trigger a clean reboot as defined by the ACPI spec. So enable the check by default if a hypervisor is detected. Signed-off-by: David Woodhouse --- On Wed, 2021-12-08 at 16:07 +0100, Rafael J. Wysocki wrote: > On Mon, Nov 8, 2021 at 5:09 PM David Woodhouse wrote: > > A follow-up patch may do this automatically for certain "known good" > > machines based on a DMI match, or perhaps just for all hypervisor > > guests since there's no good reason a hypervisor would change the > > hardware_signature that it exposes to guests *unless* it wants them > > to obey the ACPI specification. > > > > Signed-off-by: David Woodhouse > > Applied as 5.17 material, sorry for the delay. Here's the threatened follow-up. I think that a blanket enablement for all hypervisors is sane enough; there's no reason why a virtual environment would vary the hardware signature *unless* it wanted to trigger the ACPI defined hibernate behaviour, is there? arch/x86/kernel/acpi/sleep.c | 23 +++++++++++++++++++++-- drivers/acpi/sleep.c | 11 +++-------- include/linux/acpi.h | 2 +- 3 files changed, 25 insertions(+), 11 deletions(-) diff --git a/arch/x86/kernel/acpi/sleep.c b/arch/x86/kernel/acpi/sleep.c index 1e97f944b47d..3b7f4cdbf2e0 100644 --- a/arch/x86/kernel/acpi/sleep.c +++ b/arch/x86/kernel/acpi/sleep.c @@ -15,6 +15,7 @@ #include #include #include +#include #include #include "../../realmode/rm/wakeup.h" @@ -140,9 +141,9 @@ static int __init acpi_sleep_setup(char *str) acpi_realmode_flags |= 4; #ifdef CONFIG_HIBERNATION if (strncmp(str, "s4_hwsig", 8) == 0) - acpi_check_s4_hw_signature(1); + acpi_check_s4_hw_signature = 1; if (strncmp(str, "s4_nohwsig", 10) == 0) - acpi_check_s4_hw_signature(0); + acpi_check_s4_hw_signature = 0; #endif if (strncmp(str, "nonvs", 5) == 0) acpi_nvs_nosave(); @@ -160,3 +161,21 @@ static int __init acpi_sleep_setup(char *str) } __setup("acpi_sleep=", acpi_sleep_setup); + +#if defined(CONFIG_HIBERNATION) && defined(CONFIG_HYPERVISOR_GUEST) +static int __init init_s4_sigcheck(void) +{ + /* + * If running on a hypervisor, honour the ACPI specification + * by default and trigger a clean reboot when the hardware + * signature in FACS is changed after hibernation. + */ + if (acpi_check_s4_hw_signature == -1 && + !hypervisor_is_type(X86_HYPER_NATIVE)) + acpi_check_s4_hw_signature = 1; + + return 0; +} +/* This must happen before acpi_init() which is a subsys initcall */ +arch_initcall(init_s4_sigcheck); +#endif diff --git a/drivers/acpi/sleep.c b/drivers/acpi/sleep.c index a60ff5dfed3a..4c498e1051e9 100644 --- a/drivers/acpi/sleep.c +++ b/drivers/acpi/sleep.c @@ -874,12 +874,7 @@ static inline void acpi_sleep_syscore_init(void) {} #ifdef CONFIG_HIBERNATION static unsigned long s4_hardware_signature; static struct acpi_table_facs *facs; -static int sigcheck = -1; /* Default behaviour is just to warn */ - -void __init acpi_check_s4_hw_signature(int check) -{ - sigcheck = check; -} +int acpi_check_s4_hw_signature = -1; /* Default behaviour is just to warn */ static int acpi_hibernation_begin(pm_message_t stage) { @@ -1004,7 +999,7 @@ static void acpi_sleep_hibernate_setup(void) hibernation_set_ops(old_suspend_ordering ? &acpi_hibernation_ops_old : &acpi_hibernation_ops); sleep_states[ACPI_STATE_S4] = 1; - if (!sigcheck) + if (!acpi_check_s4_hw_signature) return; acpi_get_table(ACPI_SIG_FACS, 1, (struct acpi_table_header **)&facs); @@ -1016,7 +1011,7 @@ static void acpi_sleep_hibernate_setup(void) */ s4_hardware_signature = facs->hardware_signature; - if (sigcheck > 0) { + if (acpi_check_s4_hw_signature > 0) { /* * If we're actually obeying the ACPI specification * then the signature is written out as part of the diff --git a/include/linux/acpi.h b/include/linux/acpi.h index 6274758648e3..766dbcb82df1 100644 --- a/include/linux/acpi.h +++ b/include/linux/acpi.h @@ -526,7 +526,7 @@ acpi_status acpi_release_memory(acpi_handle handle, struct resource *res, int acpi_resources_are_enforced(void); #ifdef CONFIG_HIBERNATION -void __init acpi_check_s4_hw_signature(int check); +extern int acpi_check_s4_hw_signature; #endif #ifdef CONFIG_PM_SLEEP