On 7/2/20 2:21 PM, Anchal Agarwal wrote:
> From: Munehisa Kamata <kamatam@xxxxxxxxxx>
>
> 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 Agarwal: Changelog]:
> RFC v1->v2: Code refactoring
> v1->v2: Remove unused functions for PM SUSPEND/PM hibernation
>
> Signed-off-by: Anchal Agarwal <anchalag@xxxxxxxxxx>
> Signed-off-by: Munehisa Kamata <kamatam@xxxxxxxxxx>
> ---
> drivers/xen/manage.c | 60 +++++++++++++++++++++++++++++++++++++++++++
> include/xen/xen-ops.h | 1 +
> 2 files changed, 61 insertions(+)
>
> diff --git a/drivers/xen/manage.c b/drivers/xen/manage.c
> index cd046684e0d1..69833fd6cfd1 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,20 @@ enum shutdown_state {
> /* Ignore multiple shutdown requests. */
> static enum shutdown_state shutting_down = SHUTDOWN_INVALID;
>
> +enum suspend_modes {
> + NO_SUSPEND = 0,
> + XEN_SUSPEND,
> + PM_HIBERNATION,
> +};
> +
> +/* Protected by pm_mutex */
> +static enum suspend_modes suspend_mode = NO_SUSPEND;
> +
> +bool xen_is_xen_suspend(void)
Weren't you going to call this pv suspend? (And also --- is this suspend
or hibernation? Your commit messages and cover letter talk about fixing
hibernation).
> +{
> + return suspend_mode == XEN_SUSPEND;
> +}
> +
> +
> +static int xen_pm_notifier(struct notifier_block *notifier,
> + unsigned long pm_event, void *unused)
> +{
> + switch (pm_event) {
> + case PM_SUSPEND_PREPARE:
> + case PM_HIBERNATION_PREPARE:
> + case PM_RESTORE_PREPARE:
> + suspend_mode = PM_HIBERNATION;
Do you ever use this mode? It seems to me all you care about is whether
or not we are doing XEN_SUSPEND. And so perhaps suspend_mode could
become a boolean. And then maybe even drop it altogether because it you
should be able to key off (shutting_down == SHUTDOWN_SUSPEND).
> + 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 int xen_setup_pm_notifier(void)
> +{
> + if (!xen_hvm_domain())
> + return -ENODEV;
I forgot --- what did we decide about non-x86 (i.e. ARM)?
And PVH dom0.
-boris
