[RFC PATCH V2 01/11] xen/manage: keep track of the on-going suspend mode

[RFC PATCH V2 01/11] xen/manage: keep track of the on-going suspend mode

[Anchal Agarwal]

From: Munehisa Kamata <kamatam@amazon.com>

Guest hibernation is different from xen suspend/resume/live migration.
Xen save/restore does not use pm_ops as is needed by guest hibernation.
Hibernation in guest follows ACPI path and is guest inititated , the
hibernation image is saved within guest as compared to later modes
which are xen toolstack assisted and image creation/storage is in
control of hypervisor/host machine.
To differentiate between Xen suspend and PM hibernation, keep track
of the on-going suspend mode by mainly using a new PM notifier.
Introduce simple functions which help to know the on-going suspend mode
so that other Xen-related code can behave differently according to the
current suspend mode.
Since Xen suspend doesn't have corresponding PM event, its main logic
is modfied to acquire pm_mutex and set the current mode.

Though, acquirng pm_mutex is still right thing to do, we may
see deadlock if PM hibernation is interrupted by Xen suspend.
PM hibernation depends on xenwatch thread to process xenbus state
transactions, but the thread will sleep to wait pm_mutex which is
already held by PM hibernation context in the scenario. Xen shutdown
code may need some changes to avoid the issue.

[Anchal Changelog: Merged patch xen/manage: introduce helper function
to know the on-going suspend mode into this one for better readability]
Signed-off-by: Anchal Agarwal <anchalag@amazon.com>
Signed-off-by: Munehisa Kamata <kamatam@amazon.com>
---
 drivers/xen/manage.c  | 73 +++++++++++++++++++++++++++++++++++++++++++++++++++
 include/xen/xen-ops.h |  3 +++
 2 files changed, 76 insertions(+)

diff --git a/drivers/xen/manage.c b/drivers/xen/manage.c
index cd046684e0d1..0b30ab522b77 100644
--- a/drivers/xen/manage.c
+++ b/drivers/xen/manage.c
@@ -14,6 +14,7 @@
 #include <linux/freezer.h>
 #include <linux/syscore_ops.h>
 #include <linux/export.h>
+#include <linux/suspend.h>
 
 #include <xen/xen.h>
 #include <xen/xenbus.h>
@@ -40,6 +41,31 @@ enum shutdown_state {
 /* Ignore multiple shutdown requests. */
 static enum shutdown_state shutting_down = SHUTDOWN_INVALID;
 
+enum suspend_modes {
+	NO_SUSPEND = 0,
+	XEN_SUSPEND,
+	PM_SUSPEND,
+	PM_HIBERNATION,
+};
+
+/* Protected by pm_mutex */
+static enum suspend_modes suspend_mode = NO_SUSPEND;
+
+bool xen_suspend_mode_is_xen_suspend(void)
+{
+	return suspend_mode == XEN_SUSPEND;
+}
+
+bool xen_suspend_mode_is_pm_suspend(void)
+{
+	return suspend_mode == PM_SUSPEND;
+}
+
+bool xen_suspend_mode_is_pm_hibernation(void)
+{
+	return suspend_mode == PM_HIBERNATION;
+}
+
 struct suspend_info {
 	int cancelled;
 };
@@ -99,6 +125,10 @@ static void do_suspend(void)
 	int err;
 	struct suspend_info si;
 
+	lock_system_sleep();
+
+	suspend_mode = XEN_SUSPEND;
+
 	shutting_down = SHUTDOWN_SUSPEND;
 
 	err = freeze_processes();
@@ -162,6 +192,10 @@ static void do_suspend(void)
 	thaw_processes();
 out:
 	shutting_down = SHUTDOWN_INVALID;
+
+	suspend_mode = NO_SUSPEND;
+
+	unlock_system_sleep();
 }
 #endif	/* CONFIG_HIBERNATE_CALLBACKS */
 
@@ -387,3 +421,42 @@ int xen_setup_shutdown_event(void)
 EXPORT_SYMBOL_GPL(xen_setup_shutdown_event);
 
 subsys_initcall(xen_setup_shutdown_event);
+
+static int xen_pm_notifier(struct notifier_block *notifier,
+			   unsigned long pm_event, void *unused)
+{
+	switch (pm_event) {
+	case PM_SUSPEND_PREPARE:
+		suspend_mode = PM_SUSPEND;
+		break;
+	case PM_HIBERNATION_PREPARE:
+	case PM_RESTORE_PREPARE:
+		suspend_mode = PM_HIBERNATION;
+		break;
+	case PM_POST_SUSPEND:
+	case PM_POST_RESTORE:
+	case PM_POST_HIBERNATION:
+		/* Set back to the default */
+		suspend_mode = NO_SUSPEND;
+		break;
+	default:
+		pr_warn("Receive unknown PM event 0x%lx\n", pm_event);
+		return -EINVAL;
+	}
+
+	return 0;
+};
+
+static struct notifier_block xen_pm_notifier_block = {
+	.notifier_call = xen_pm_notifier
+};
+
+static int xen_setup_pm_notifier(void)
+{
+	if (!xen_hvm_domain())
+		return -ENODEV;
+
+	return register_pm_notifier(&xen_pm_notifier_block);
+}
+
+subsys_initcall(xen_setup_pm_notifier);
diff --git a/include/xen/xen-ops.h b/include/xen/xen-ops.h
index d89969aa9942..6c36e161dfd1 100644
--- a/include/xen/xen-ops.h
+++ b/include/xen/xen-ops.h
@@ -40,6 +40,9 @@ u64 xen_steal_clock(int cpu);
 
 int xen_setup_shutdown_event(void);
 
+bool xen_suspend_mode_is_xen_suspend(void);
+bool xen_suspend_mode_is_pm_suspend(void);
+bool xen_suspend_mode_is_pm_hibernation(void);
 extern unsigned long *xen_contiguous_bitmap;
 
 #if defined(CONFIG_XEN_PV) || defined(CONFIG_ARM) || defined(CONFIG_ARM64)
-- 
2.15.3.AMZN

Re: [RFC PATCH V2 01/11] xen/manage: keep track of the on-going suspend mode

[Boris Ostrovsky]

On 1/7/20 6:37 PM, Anchal Agarwal wrote:
> +
> +static int xen_setup_pm_notifier(void)
> +{
> +	if (!xen_hvm_domain())
> +		return -ENODEV;

ARM guests are also HVM domains. Is it OK for them to register the 
notifier? The diffstat suggests that you are supporting ARM.

-boris

> +
> +	return register_pm_notifier(&xen_pm_notifier_block);
> +}
>

Re: [RFC PATCH V2 01/11] xen/manage: keep track of the on-going suspend mode

[Boris Ostrovsky]

On 1/9/20 6:46 PM, Boris Ostrovsky wrote:
>
>
> On 1/7/20 6:37 PM, Anchal Agarwal wrote:
>> +
>> +static int xen_setup_pm_notifier(void)
>> +{
>> +    if (!xen_hvm_domain())
>> +        return -ENODEV;
>
> ARM guests are also HVM domains. Is it OK for them to register the 
> notifier? The diffstat suggests that you are supporting ARM.

I obviously meant *not* supporting ARM, sorry.

-boris

>
> -boris
>
>> +
>> +    return register_pm_notifier(&xen_pm_notifier_block);
>> +}
>>
>

Re: [RFC PATCH V2 01/11] xen/manage: keep track of the on-going suspend mode

[Anchal Agarwal]

On Thu, Jan 09, 2020 at 06:49:07PM -0500, Boris Ostrovsky wrote:
> 
> 
> On 1/9/20 6:46 PM, Boris Ostrovsky wrote:
> >
> >
> >On 1/7/20 6:37 PM, Anchal Agarwal wrote:
> >>+
> >>+static int xen_setup_pm_notifier(void)
> >>+{
> >>+    if (!xen_hvm_domain())
> >>+        return -ENODEV;
> >
> >ARM guests are also HVM domains. Is it OK for them to register the
> >notifier? The diffstat suggests that you are supporting ARM.
> 
> I obviously meant *not* supporting ARM, sorry.
> 
> -boris
> 
> >
> >-boris
> >

TBH, I have not yet experimented with these patches on
ARM guest yet but that will be the next step. The same 
code with changes as needed should be made to work for ARM.
Currently I am focussed on getting a sane set of 
patches into mainline for x86 guests.

Thanks,

Anchal

> >>+
> >>+    return register_pm_notifier(&xen_pm_notifier_block);
> >>+}
> >>
> >
>