> > Hi Rafael, > > Sorry for the back and forth on this. > > With the patch which provides the helper, LKP reported build error: > > drivers/acpi/sleep.c: In function 'acpi_suspend_enter': > > > drivers/acpi/sleep.c:600:22: error: 'acpi_suspend_lowlevel' undeclared (first > use in this function); did you mean 'acpi_set_suspend_lowlevel'? > 600 | if (!acpi_suspend_lowlevel) > | ^~~~~~~~~~~~~~~~~~~~~ > | acpi_set_suspend_lowlevel > > Turns out I disabled both suspend/hibernation in my own kernel build test, sigh. > > The common ACPI sleep code requires the ARCH to declare 'acpi_suspend_lowlevel' > in <asm/acpi.h>, and define it somewhere in the ARCH code too. > > So sadly I cannot remove the acpi_suspend_lowlevel variable declaration in > <asm/acpi.h>. And the ending patch would have both below in <asm/acpi.h>: > > /* Low-level suspend routine. */ > extern int (*acpi_suspend_lowlevel)(void); > > +/* To override @acpi_suspend_lowlevel at early boot */ > +void acpi_set_suspend_lowlevel(int (*suspend_lowlevel)(void)); > + > > Thus I am not sure whether it still a good idea to have the helper? > Hi Rafael, Could you help to take a look whether you still want the helper? Ans sorry in previous reply I pasted an old/wrong patch, please ignore. Below is the patch that can build successfully. x86/acpi: Add a helper to override ACPI lowlevel suspend function ACPI S3 suspend code requires a valid 'acpi_suspend_lowlevel' function pointer to work. Each ARCH needs to set the acpi_suspend_lowlevel to its own implementation to make ACPI S3 suspend work on that ARCH. X86 implements a default function for that, and Xen PV dom0 overrides it with its own version during early kernel boot. Intel Trusted Domain Extensions (TDX) doesn't play nice with ACPI S3. ACPI S3 suspend will gets disabled during kernel early boot if TDX is enabled. Add a helper function to override the acpi_suspend_lowlevel at kernel early boot, so that the callers don't manipulate the function pointer directly. Change the Xen code to use the helper. It will be used by TDX code to disable ACPI S3 suspend too. No functional change is intended. Signed-off-by: Kai Huang <kai.huang@xxxxxxxxx> --- arch/x86/include/asm/acpi.h | 3 +++ arch/x86/kernel/acpi/boot.c | 5 +++++ include/xen/acpi.h | 2 +- 3 files changed, 9 insertions(+), 1 deletion(-) diff --git a/arch/x86/include/asm/acpi.h b/arch/x86/include/asm/acpi.h index c8a7fc23f63c..6001df87526e 100644 --- a/arch/x86/include/asm/acpi.h +++ b/arch/x86/include/asm/acpi.h @@ -63,6 +63,9 @@ static inline void acpi_disable_pci(void) /* Low-level suspend routine. */ extern int (*acpi_suspend_lowlevel)(void); +/* To override @acpi_suspend_lowlevel at early boot */ +void acpi_set_suspend_lowlevel(int (*suspend_lowlevel)(void)); + /* Physical address to resume after wakeup */ unsigned long acpi_get_wakeup_address(void); diff --git a/arch/x86/kernel/acpi/boot.c b/arch/x86/kernel/acpi/boot.c index 2a0ea38955df..f9858b108387 100644 --- a/arch/x86/kernel/acpi/boot.c +++ b/arch/x86/kernel/acpi/boot.c @@ -784,6 +784,11 @@ int (*acpi_suspend_lowlevel)(void) = x86_acpi_suspend_lowlevel; int (*acpi_suspend_lowlevel)(void); #endif +void __init acpi_set_suspend_lowlevel(int (*suspend_lowlevel)(void)) +{ + acpi_suspend_lowlevel = suspend_lowlevel; +} + /* * success: return IRQ number (>=0) * failure: return < 0 diff --git a/include/xen/acpi.h b/include/xen/acpi.h index b1e11863144d..81a1b6ee8fc2 100644 --- a/include/xen/acpi.h +++ b/include/xen/acpi.h @@ -64,7 +64,7 @@ static inline void xen_acpi_sleep_register(void) acpi_os_set_prepare_extended_sleep( &xen_acpi_notify_hypervisor_extended_sleep); - acpi_suspend_lowlevel = xen_acpi_suspend_lowlevel; + acpi_set_suspend_lowlevel(xen_acpi_suspend_lowlevel); } } #else -- 2.41.0