Re: [PATCH v14 21/23] x86/virt/tdx: Handle TDX interaction with ACPI S3 and deeper states

[Huang, Kai]

> 
> 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