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

[Mediouni, Mohamed]

> On 2. Apr 2024, at 12:29, Sudeep Holla <sudeep.holla@xxxxxxx> wrote:
> 
> 
> 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@xxxxxxxxxxxx>
>>> 
>>> 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@xxxxxxxxxxxx>
>> 
>> 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 ?
Current Windows on Arm platforms (seen on SC8280XP) don’t have _GTS
or _PTS methods, and don’t have sleeping objects either.

As such, I don’t expect any users for that potential functionality. Am I missing something 
or hibernation signalling to firmware (on ARM64) can be made PSCI only indefinitely?

Thank you,
-Mohamed
> So, until that happens, I see this as an possible alternative and
> temporary solution.
> 
> Acked-by: Sudeep Holla <sudeep.holla@xxxxxxx>
> 
> --
> Regards,
> Sudeep




Amazon Development Center Germany GmbH
Krausenstr. 38
10117 Berlin
Geschaeftsfuehrung: Christian Schlaeger, Jonathan Weiss
Eingetragen am Amtsgericht Charlottenburg unter HRB 149173 B
Sitz: Berlin
Ust-ID: DE 289 237 879