[PATCH v2 3/4] hv_utils: Support host-initiated hibernation request

[PATCH v2 3/4] hv_utils: Support host-initiated hibernation request

[Dexuan Cui]

Update the Shutdown IC version to 3.2, which is required for the host to
send the hibernation request.

The user is expected to create the below udev rule file, which is applied
upon the host-initiated hibernation request:

root@localhost:~# cat /usr/lib/udev/rules.d/40-vm-hibernation.rules
SUBSYSTEM=="vmbus", ACTION=="change", DRIVER=="hv_utils", ENV{EVENT}=="hibernate", RUN+="/usr/bin/systemctl hibernate"

Signed-off-by: Dexuan Cui <decui@microsoft.com>
---
 drivers/hv/hv_util.c | 52 +++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 51 insertions(+), 1 deletion(-)

diff --git a/drivers/hv/hv_util.c b/drivers/hv/hv_util.c
index fe3a316380c2..d5216af62788 100644
--- a/drivers/hv/hv_util.c
+++ b/drivers/hv/hv_util.c
@@ -25,7 +25,9 @@
 #define SD_MAJOR	3
 #define SD_MINOR	0
 #define SD_MINOR_1	1
+#define SD_MINOR_2	2
 #define SD_VERSION_3_1	(SD_MAJOR << 16 | SD_MINOR_1)
+#define SD_VERSION_3_2	(SD_MAJOR << 16 | SD_MINOR_2)
 #define SD_VERSION	(SD_MAJOR << 16 | SD_MINOR)
 
 #define SD_MAJOR_1	1
@@ -52,9 +54,10 @@ static int sd_srv_version;
 static int ts_srv_version;
 static int hb_srv_version;
 
-#define SD_VER_COUNT 3
+#define SD_VER_COUNT 4
 static const int sd_versions[] = {
 	SD_VERSION_3_1,
+	SD_VERSION_3_2,
 	SD_VERSION,
 	SD_VERSION_1
 };
@@ -78,9 +81,45 @@ static const int fw_versions[] = {
 	UTIL_WS2K8_FW_VERSION
 };
 
+/*
+ * Send the "hibernate" udev event in a thread context.
+ */
+struct hibernate_work_context {
+	struct work_struct work;
+	struct hv_device *dev;
+};
+
+static struct hibernate_work_context hibernate_context;
+static bool execute_hibernate;
+
+static void send_hibernate_uevent(struct work_struct *work)
+{
+	char *uevent_env[2] = { "EVENT=hibernate", NULL };
+	struct hibernate_work_context *ctx;
+
+	ctx = container_of(work, struct hibernate_work_context, work);
+
+	kobject_uevent_env(&ctx->dev->device.kobj, KOBJ_CHANGE, uevent_env);
+
+	pr_info("Sent hibernation uevent\n");
+}
+
+static int hv_shutdown_init(struct hv_util_service *srv)
+{
+	struct vmbus_channel *channel = srv->channel;
+
+	INIT_WORK(&hibernate_context.work, send_hibernate_uevent);
+	hibernate_context.dev = channel->device_obj;
+
+	execute_hibernate = hv_is_hibernation_supported();
+
+	return 0;
+}
+
 static void shutdown_onchannelcallback(void *context);
 static struct hv_util_service util_shutdown = {
 	.util_cb = shutdown_onchannelcallback,
+	.util_init = hv_shutdown_init,
 };
 
 static int hv_timesync_init(struct hv_util_service *srv);
@@ -187,6 +226,17 @@ static void shutdown_onchannelcallback(void *context)
 
 				schedule_work(&restart_work);
 				break;
+			case 4:
+			case 5:
+				pr_info("Hibernation request received\n");
+
+				if (execute_hibernate) {
+					icmsghdrp->status = HV_S_OK;
+					schedule_work(&hibernate_context.work);
+				} else {
+					icmsghdrp->status = HV_E_FAIL;
+				}
+				break;
 			default:
 				icmsghdrp->status = HV_E_FAIL;
 				execute_shutdown = false;
-- 
2.19.1

RE: [PATCH v2 3/4] hv_utils: Support host-initiated hibernation request

[Michael Kelley]

From: Dexuan Cui <decui@microsoft.com> Sent: Sunday, January 12, 2020 10:32 PM
> 
> Update the Shutdown IC version to 3.2, which is required for the host to
> send the hibernation request.
> 
> The user is expected to create the below udev rule file, which is applied
> upon the host-initiated hibernation request:
> 
> root@localhost:~# cat /usr/lib/udev/rules.d/40-vm-hibernation.rules
> SUBSYSTEM=="vmbus", ACTION=="change", DRIVER=="hv_utils",
> ENV{EVENT}=="hibernate", RUN+="/usr/bin/systemctl hibernate"
> 
> Signed-off-by: Dexuan Cui <decui@microsoft.com>
> ---
>  drivers/hv/hv_util.c | 52 +++++++++++++++++++++++++++++++++++++++++++-
>  1 file changed, 51 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/hv/hv_util.c b/drivers/hv/hv_util.c
> index fe3a316380c2..d5216af62788 100644
> --- a/drivers/hv/hv_util.c
> +++ b/drivers/hv/hv_util.c
> @@ -25,7 +25,9 @@
>  #define SD_MAJOR	3
>  #define SD_MINOR	0
>  #define SD_MINOR_1	1
> +#define SD_MINOR_2	2
>  #define SD_VERSION_3_1	(SD_MAJOR << 16 | SD_MINOR_1)
> +#define SD_VERSION_3_2	(SD_MAJOR << 16 | SD_MINOR_2)
>  #define SD_VERSION	(SD_MAJOR << 16 | SD_MINOR)
> 
>  #define SD_MAJOR_1	1
> @@ -52,9 +54,10 @@ static int sd_srv_version;
>  static int ts_srv_version;
>  static int hb_srv_version;
> 
> -#define SD_VER_COUNT 3
> +#define SD_VER_COUNT 4
>  static const int sd_versions[] = {
>  	SD_VERSION_3_1,
> +	SD_VERSION_3_2,

I think these versions need to listed in descending order, so the new
SD_VERSION_3_2 should be listed first.  Otherwise a Hyper-V host that
supports both 3.1 and 3.2 might match on 3.1 first without ever checking
for a match with 3.2.

>  	SD_VERSION,
>  	SD_VERSION_1
>  };
> @@ -78,9 +81,45 @@ static const int fw_versions[] = {
>  	UTIL_WS2K8_FW_VERSION
>  };
> 
> +/*
> + * Send the "hibernate" udev event in a thread context.
> + */
> +struct hibernate_work_context {
> +	struct work_struct work;
> +	struct hv_device *dev;
> +};
> +
> +static struct hibernate_work_context hibernate_context;
> +static bool execute_hibernate;
> +
> +static void send_hibernate_uevent(struct work_struct *work)
> +{
> +	char *uevent_env[2] = { "EVENT=hibernate", NULL };
> +	struct hibernate_work_context *ctx;
> +
> +	ctx = container_of(work, struct hibernate_work_context, work);
> +
> +	kobject_uevent_env(&ctx->dev->device.kobj, KOBJ_CHANGE, uevent_env);
> +
> +	pr_info("Sent hibernation uevent\n");
> +}
> +
> +static int hv_shutdown_init(struct hv_util_service *srv)
> +{
> +	struct vmbus_channel *channel = srv->channel;
> +
> +	INIT_WORK(&hibernate_context.work, send_hibernate_uevent);
> +	hibernate_context.dev = channel->device_obj;
> +
> +	execute_hibernate = hv_is_hibernation_supported();
> +
> +	return 0;
> +}
> +
>  static void shutdown_onchannelcallback(void *context);
>  static struct hv_util_service util_shutdown = {
>  	.util_cb = shutdown_onchannelcallback,
> +	.util_init = hv_shutdown_init,
>  };
> 
>  static int hv_timesync_init(struct hv_util_service *srv);
> @@ -187,6 +226,17 @@ static void shutdown_onchannelcallback(void *context)
> 
>  				schedule_work(&restart_work);
>  				break;
> +			case 4:
> +			case 5:

As before, I'm wondering about the interpretation of these numbers.

> +				pr_info("Hibernation request received\n");
> +
> +				if (execute_hibernate) {
> +					icmsghdrp->status = HV_S_OK;
> +					schedule_work(&hibernate_context.work);

Same comment here about the ordering of the schedule_work() call and the
sending of the response message.  Seems like the code should be consistent
for all three cases -- shutdown, restart, and hibernate.

> +				} else {
> +					icmsghdrp->status = HV_E_FAIL;
> +				}
> +				break;
>  			default:
>  				icmsghdrp->status = HV_E_FAIL;
>  				execute_shutdown = false;
> --
> 2.19.1

RE: [PATCH v2 3/4] hv_utils: Support host-initiated hibernation request

[Dexuan Cui]

> From: Michael Kelley <mikelley@microsoft.com>
> > --- a/drivers/hv/hv_util.c
> > +++ b/drivers/hv/hv_util.c
> > ...
> >  static const int sd_versions[] = {
> >  	SD_VERSION_3_1,
> > +	SD_VERSION_3_2,
> 
> I think these versions need to listed in descending order, so the new
> SD_VERSION_3_2 should be listed first.  Otherwise a Hyper-V host that
> supports both 3.1 and 3.2 might match on 3.1 first without ever checking
> for a match with 3.2.

Thanks! This is a rebasing mistake. Will fix it.

> > @@ -187,6 +226,17 @@ static void shutdown_onchannelcallback(void
> *context)
> >
> >  				schedule_work(&restart_work);
> >  				break;
> > +			case 4:
> > +			case 5:
> 
> As before, I'm wondering about the interpretation of these numbers.

Will add a comment.
 
> > +				pr_info("Hibernation request received\n");
> > +
> > +				if (execute_hibernate) {
> > +					icmsghdrp->status = HV_S_OK;
> > +					schedule_work(&hibernate_context.work);
> 
> Same comment here about the ordering of the schedule_work() call and the
> sending of the response message.  Seems like the code should be consistent
> for all three cases -- shutdown, restart, and hibernate.

I agree. Will fix this.

Thanks,
-- Dexuan