Re: [PATCH 2/2] arm64: acpi: Honour firmware_signature field of FACS, if it exists

From: Sudeep Holla <sudeep.holla@arm.com>
To: David Woodhouse <dwmw2@infradead.org>
Cc: linux-arm-kernel@lists.infradead.org,
	Sudeep Holla <sudeep.holla@arm.com>,
	Catalin Marinas <catalin.marinas@arm.com>,
	Will Deacon <will@kernel.org>,
	Robert Moore <robert.moore@intel.com>,
	"Rafael J. Wysocki" <rafael.j.wysocki@intel.com>,
	Len Brown <lenb@kernel.org>,
	mediou@amazon.de, alisaidi@amazon.com,
	linux-kernel@vger.kernel.org, linux-acpi@vger.kernel.org,
	acpica-devel@lists.linux.dev,
	Saket Dumbre <saket.dumbre@intel.com>
Subject: Re: [PATCH 2/2] arm64: acpi: Honour firmware_signature field of FACS, if it exists
Date: Tue, 2 Apr 2024 11:29:35 +0100	[thread overview]
Message-ID: <ZgveD6Hb2HbTNYNO@bogus> (raw)
In-Reply-To: <dbd60df4632e5ea9cef13cdc1a406b47bd8629da.camel@infradead.org>

On Tue, Apr 02, 2024 at 10:29:57AM +0100, David Woodhouse wrote:
> On Tue, 2024-03-12 at 13:41 +0000, David Woodhouse wrote:
> > From: David Woodhouse <dwmw@amazon.co.uk>
> > 
> > If the firmware_signature changes then OSPM should not attempt to resume
> > from hibernate, but should instead perform a clean reboot. Set the global
> > swsusp_hardware_signature to allow the generic code to include the value
> > in the swsusp header on disk, and perform the appropriate check on resume.
> > 
> > Signed-off-by: David Woodhouse <dwmw@amazon.co.uk>
> 
> Ping?
> 
> > ---
> >  arch/arm64/kernel/acpi.c | 10 ++++++++++
> >  1 file changed, 10 insertions(+)
> > 
> > diff --git a/arch/arm64/kernel/acpi.c b/arch/arm64/kernel/acpi.c
> > index dba8fcec7f33..e0e7b93c16cc 100644
> > --- a/arch/arm64/kernel/acpi.c
> > +++ b/arch/arm64/kernel/acpi.c
> > @@ -26,6 +26,7 @@
> >  #include <linux/libfdt.h>
> >  #include <linux/smp.h>
> >  #include <linux/serial_core.h>
> > +#include <linux/suspend.h>
> >  #include <linux/pgtable.h>
> >  
> >  #include <acpi/ghes.h>
> > @@ -227,6 +228,15 @@ void __init acpi_boot_table_init(void)
> >                 if (earlycon_acpi_spcr_enable)
> >                         early_init_dt_scan_chosen_stdout();
> >         } else {
> > +#ifdef CONFIG_HIBERNATION
> > +               struct acpi_table_header *facs = NULL;
> > +               acpi_get_table(ACPI_SIG_FACS, 1, &facs);
> > +               if (facs) {
> > +                       swsusp_hardware_signature =
> > +                               ((struct acpi_table_facs *)facs)->hardware_signature;
> > +                       acpi_put_table(facs);
> > +               }
> > +#endif

I think it is OK as a temporary solution for now. But there was some
investigation last year as part of some work in Linaro to enable
"drivers/acpi/sleep.c" into the build cleaning up some x86-ness in there.
acpi_sleep_hibernate_setup() already does this but enabling sleep.c need
some careful investigation so that it doesn't break any existing arm64/x86
platforms and made need some wordings clarification in the ACPI spec.
Today system suspend work via psci std path bypassing the ACPI paths which
may not be ideal as none of the ACPI methods are honoured. Some arm64
platforms may implement them and expect to be executed in the future,
maybe ?

So, until that happens, I see this as an possible alternative and
temporary solution.

Acked-by: Sudeep Holla <sudeep.holla@arm.com>

--
Regards,
Sudeep

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel