RE: [PATCH] ACPI: PM: Export the function acpi_sleep_state_supported()

[Dexuan Cui <decui@microsoft.com>]

> From: linux-hyperv-owner@vger.kernel.org
> <linux-hyperv-owner@vger.kernel.org> On Behalf Of Lorenzo Pieralisi
> Sent: Monday, June 17, 2019 9:15 AM
> > ...
> > + some ARM experts who worked on arch/arm/kernel/hibernate.c.
> >
> > drivers/acpi/sleep.c is only built if ACPI_SYSTEM_POWER_STATES_SUPPORT
> > is defined, but it looks this option is not defined on ARM.
> >
> > It looks ARM does not support the ACPI S4 state, then how do we know
> > if an ARM host supports hibernation or not?
> 
> Maybe we should start from understanding why you need to know whether
> Hibernate is possible to answer your question ?
> 
> On ARM64 platforms system states are entered through PSCI firmware
> interface that works for ACPI and device tree alike.
> 
> Lorenzo

Hi Lorenzo,
It looks I may have confused you as I didn't realize the word "ARM" only means
32-bit ARM. It looks the "ARM" arch and the "ARM64" arch are very different.

As far as I know, Hyper-V only supports x86 and "ARM64", and it's unlikely to
support 32-bit ARM in the future, so actually I don't really need to know if and
how a 32-bit ARM machine supports hibernation.

When a Linux guest runs on Hyper-V (x86_32, x86_64, or ARM64) , we have a
front-end balloon driver in the guest, which balloons up/down and
hot adds/removes the guest's memory when the host requests that. The problem
is: the back-end driver on the host can not really save and restore the states
related to the front-end balloon driver on guest hibernation, so we made the
decision that balloon up/down and hot-add/remove are not supported when
we enable hibernation for a guest; BTW, we still want to load the front-end
driver in the guest, because the dirver has a functionality of reporting the
guest's memory pressure to the host, which we think is useful.

On x86_32 and x86_64, we enable hibernation for a guest by enabling
the virtual ACPI S4 state for the guest; on ARM64, so far we don't have the
host side changes required to support guest hibernation, so the details are
still unclear.

After I discussed with Michael Kelley, it looks we don't really need to
export drivers/acpi/sleep.c: acpi_sleep_state_supported(), but I think we do
need to make it non-static.

Now I propose the below changes. I plan to submit a patch first for the
changes made to drivers/acpi/sleep.c and include/acpi/acpi_bus.h in a few
days, if there is no objection.

Please let me know how you think of this. Thanks!

diff --git a/drivers/acpi/sleep.c b/drivers/acpi/sleep.c
index a34deccd7317..18d2e5922448 100644
--- a/drivers/acpi/sleep.c
+++ b/drivers/acpi/sleep.c
@@ -79,7 +79,7 @@ static int acpi_sleep_prepare(u32 acpi_state)
 	return 0;
 }
 
-static bool acpi_sleep_state_supported(u8 sleep_state)
+bool acpi_sleep_state_supported(u8 sleep_state)
 {
 	acpi_status status;
 	u8 type_a, type_b;
diff --git a/include/acpi/acpi_bus.h b/include/acpi/acpi_bus.h
index 31b6c87d6240..3e6563e1a2c0 100644
--- a/include/acpi/acpi_bus.h
+++ b/include/acpi/acpi_bus.h
@@ -651,6 +651,12 @@ static inline int acpi_pm_set_bridge_wakeup(struct device *dev, bool enable)
 }
 #endif
 
+#ifdef CONFIG_ACPI_SYSTEM_POWER_STATES_SUPPORT
+bool acpi_sleep_state_supported(u8 sleep_state);
+#else
+bool acpi_sleep_state_supported(u8 sleep_state) { return false; }
+#endif
+
 #ifdef CONFIG_ACPI_SLEEP
 u32 acpi_target_system_state(void);
 #else
diff --git a/arch/x86/hyperv/hv_init.c b/arch/x86/hyperv/hv_init.c
index 79f626ab8306..197e8fa32871 100644
--- a/arch/x86/hyperv/hv_init.c
+++ b/arch/x86/hyperv/hv_init.c
@@ -17,6 +17,7 @@
  *
  */
 
+#include <linux/acpi.h>
 #include <linux/efi.h>
 #include <linux/types.h>
 #include <asm/apic.h>
@@ -420,3 +421,9 @@ bool hv_is_hyperv_initialized(void)
 	return hypercall_msr.enable;
 }
 EXPORT_SYMBOL_GPL(hv_is_hyperv_initialized);
+
+bool hv_is_hibernation_supported(void)
+{
+	return acpi_sleep_state_supported(ACPI_STATE_S4);
+}
+EXPORT_SYMBOL_GPL(hv_is_hibernation_supported);
diff --git a/include/asm-generic/mshyperv.h b/include/asm-generic/mshyperv.h
index 0becb7d9704d..1cb40016ee53 100644
--- a/include/asm-generic/mshyperv.h
+++ b/include/asm-generic/mshyperv.h
@@ -166,9 +166,11 @@ static inline int cpumask_to_vpset(struct hv_vpset *vpset,
 void hyperv_report_panic(struct pt_regs *regs, long err);
 void hyperv_report_panic_msg(phys_addr_t pa, size_t size);
 bool hv_is_hyperv_initialized(void);
+bool hv_is_hibernation_supported(void);
 void hyperv_cleanup(void);
 #else /* CONFIG_HYPERV */
 static inline bool hv_is_hyperv_initialized(void) { return false; }
+static inline bool hv_is_hibernation_supported(void) { return false; }
 static inline void hyperv_cleanup(void) {}
 #endif /* CONFIG_HYPERV */
 
diff --git a/drivers/hv/hv_balloon.c b/drivers/hv/hv_balloon.c
index 111ea3599659..39fd4e4a8fd7 100644
--- a/drivers/hv/hv_balloon.c
+++ b/drivers/hv/hv_balloon.c
@@ -34,6 +34,8 @@
 
 #include <linux/hyperv.h>
 
+#include <asm/mshyperv.h>
+
 #define CREATE_TRACE_POINTS
 #include "hv_trace_balloon.h"
 
@@ -1682,6 +1684,9 @@ static int balloon_probe(struct hv_device *dev,
 {
 	int ret;
 
+	if (hv_is_hibernation_supported())
+		hot_add = false;
+
 #ifdef CONFIG_MEMORY_HOTPLUG
 	do_hot_add = hot_add;
 #else


Thanks,
-- Dexuan