2020-01-25 19:56:50

by Dexuan Cui

[permalink] [raw]
Subject: [PATCH v3 0/4] hv_utils: Add the support of hibernation

Hi,
This is an updated version of the v2 patchset
(https://lkml.org/lkml/2020/1/13/42).

I addressed all the comments from Michael, and documented the changes
after the "---" line in every patch's changelog.

Please review.

Thanks!

Dexuan Cui (4):
Tools: hv: Reopen the devices if read() or write() returns errors
hv_utils: Support host-initiated restart request
hv_utils: Support host-initiated hibernation request
hv_utils: Add the support of hibernation

drivers/hv/hv_fcopy.c | 54 +++++++++++++-
drivers/hv/hv_kvp.c | 43 ++++++++++-
drivers/hv/hv_snapshot.c | 55 +++++++++++++-
drivers/hv/hv_util.c | 144 ++++++++++++++++++++++++++++++++++++-
drivers/hv/hyperv_vmbus.h | 6 ++
include/linux/hyperv.h | 2 +
tools/hv/hv_fcopy_daemon.c | 33 +++++++--
tools/hv/hv_kvp_daemon.c | 36 ++++++----
tools/hv/hv_vss_daemon.c | 49 ++++++++++---
9 files changed, 385 insertions(+), 37 deletions(-)

--
2.19.1


2020-01-25 19:57:05

by Dexuan Cui

[permalink] [raw]
Subject: [PATCH v3 3/4] hv_utils: Support host-initiated hibernation request

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 <[email protected]>

---
Changes in v2:
Send the host-initiated hibernation request to the user space via udev.
(v1 used call_usermodehelper() and "/sbin/hyperv-hibernate".)

Changes in v3 (I addressed Michael's comoments):
Fixed the order issue in sd_versions[].
Moved schedule_work() to a later place for consistency.

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 d815bea0fda3..e07197dfb4a2 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,8 +54,9 @@ 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_2,
SD_VERSION_3_1,
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 hibernation_supported;
+
+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;
+
+ hibernation_supported = 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);
@@ -143,6 +182,7 @@ static void shutdown_onchannelcallback(void *context)
u64 requestid;
bool execute_shutdown = false;
bool execute_reboot = false;
+ bool execute_hibernate = false;
u8 *shut_txf_buf = util_shutdown.recv_buffer;

struct shutdown_msg_data *shutdown_msg;
@@ -194,6 +234,14 @@ static void shutdown_onchannelcallback(void *context)
pr_info("Restart request received -"
" graceful restart initiated\n");
break;
+ case 4:
+ case 5:
+ pr_info("Hibernation request received\n");
+
+ icmsghdrp->status = hibernation_supported ?
+ HV_S_OK : HV_E_FAIL;
+ execute_hibernate = hibernation_supported;
+ break;
default:
icmsghdrp->status = HV_E_FAIL;
execute_shutdown = false;
@@ -216,6 +264,8 @@ static void shutdown_onchannelcallback(void *context)
schedule_work(&shutdown_work);
if (execute_reboot == true)
schedule_work(&restart_work);
+ if (execute_hibernate == true)
+ schedule_work(&hibernate_context.work);
}

/*
--
2.19.1

2020-01-25 19:57:33

by Dexuan Cui

[permalink] [raw]
Subject: [PATCH v3 2/4] hv_utils: Support host-initiated restart request

The hv_utils driver currently supports a "shutdown" operation initiated
from the Hyper-V host. Newer versions of Hyper-V also support a "restart"
operation. So add support for the updated protocol version that has
"restart" support, and perform a clean reboot when such a message is
received from Hyper-V.

To test the restart functionality, run this PowerShell command on the
Hyper-V host:

Restart-VM <vmname> -Type Reboot

Signed-off-by: Dexuan Cui <[email protected]>

---
Changes in v2:
It's the same as v1.

Changes in v3 (I addressed Michael's comments):
Used a better version of changelog from Michael.
Added a comment about the meaning of shutdown_msg->flags.
Call schedule_work() at the end of the function for consistency.

drivers/hv/hv_util.c | 32 +++++++++++++++++++++++++++++++-
1 file changed, 31 insertions(+), 1 deletion(-)

diff --git a/drivers/hv/hv_util.c b/drivers/hv/hv_util.c
index 766bd8457346..d815bea0fda3 100644
--- a/drivers/hv/hv_util.c
+++ b/drivers/hv/hv_util.c
@@ -24,6 +24,8 @@

#define SD_MAJOR 3
#define SD_MINOR 0
+#define SD_MINOR_1 1
+#define SD_VERSION_3_1 (SD_MAJOR << 16 | SD_MINOR_1)
#define SD_VERSION (SD_MAJOR << 16 | SD_MINOR)

#define SD_MAJOR_1 1
@@ -50,8 +52,9 @@ static int sd_srv_version;
static int ts_srv_version;
static int hb_srv_version;

-#define SD_VER_COUNT 2
+#define SD_VER_COUNT 3
static const int sd_versions[] = {
+ SD_VERSION_3_1,
SD_VERSION,
SD_VERSION_1
};
@@ -118,17 +121,28 @@ static void perform_shutdown(struct work_struct *dummy)
orderly_poweroff(true);
}

+static void perform_restart(struct work_struct *dummy)
+{
+ orderly_reboot();
+}
+
/*
* Perform the shutdown operation in a thread context.
*/
static DECLARE_WORK(shutdown_work, perform_shutdown);

+/*
+ * Perform the restart operation in a thread context.
+ */
+static DECLARE_WORK(restart_work, perform_restart);
+
static void shutdown_onchannelcallback(void *context)
{
struct vmbus_channel *channel = context;
u32 recvlen;
u64 requestid;
bool execute_shutdown = false;
+ bool execute_reboot = false;
u8 *shut_txf_buf = util_shutdown.recv_buffer;

struct shutdown_msg_data *shutdown_msg;
@@ -157,6 +171,12 @@ static void shutdown_onchannelcallback(void *context)
sizeof(struct vmbuspipe_hdr) +
sizeof(struct icmsg_hdr)];

+ /*
+ * shutdown_msg->flags can be 0 (shut down), 2(reboot),
+ * or 4(hibernate). It may bitwise-OR 1, which means
+ * performing the request by force. Linux always tries
+ * to perform the request by force.
+ */
switch (shutdown_msg->flags) {
case 0:
case 1:
@@ -166,6 +186,14 @@ static void shutdown_onchannelcallback(void *context)
pr_info("Shutdown request received -"
" graceful shutdown initiated\n");
break;
+ case 2:
+ case 3:
+ icmsghdrp->status = HV_S_OK;
+ execute_reboot = true;
+
+ pr_info("Restart request received -"
+ " graceful restart initiated\n");
+ break;
default:
icmsghdrp->status = HV_E_FAIL;
execute_shutdown = false;
@@ -186,6 +214,8 @@ static void shutdown_onchannelcallback(void *context)

if (execute_shutdown == true)
schedule_work(&shutdown_work);
+ if (execute_reboot == true)
+ schedule_work(&restart_work);
}

/*
--
2.19.1

2020-01-26 04:58:11

by Michael Kelley (LINUX)

[permalink] [raw]
Subject: RE: [PATCH v3 2/4] hv_utils: Support host-initiated restart request

From: Dexuan Cui <[email protected]> Sent: Saturday, January 25, 2020 11:54 AM
>
> The hv_utils driver currently supports a "shutdown" operation initiated
> from the Hyper-V host. Newer versions of Hyper-V also support a "restart"
> operation. So add support for the updated protocol version that has
> "restart" support, and perform a clean reboot when such a message is
> received from Hyper-V.
>
> To test the restart functionality, run this PowerShell command on the
> Hyper-V host:
>
> Restart-VM <vmname> -Type Reboot
>
> Signed-off-by: Dexuan Cui <[email protected]>
>
> ---
> Changes in v2:
> It's the same as v1.
>
> Changes in v3 (I addressed Michael's comments):
> Used a better version of changelog from Michael.
> Added a comment about the meaning of shutdown_msg->flags.
> Call schedule_work() at the end of the function for consistency.
>
> drivers/hv/hv_util.c | 32 +++++++++++++++++++++++++++++++-
> 1 file changed, 31 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/hv/hv_util.c b/drivers/hv/hv_util.c
> index 766bd8457346..d815bea0fda3 100644
> --- a/drivers/hv/hv_util.c
> +++ b/drivers/hv/hv_util.c
> @@ -24,6 +24,8 @@
>
> #define SD_MAJOR 3
> #define SD_MINOR 0
> +#define SD_MINOR_1 1
> +#define SD_VERSION_3_1 (SD_MAJOR << 16 | SD_MINOR_1)
> #define SD_VERSION (SD_MAJOR << 16 | SD_MINOR)
>
> #define SD_MAJOR_1 1
> @@ -50,8 +52,9 @@ static int sd_srv_version;
> static int ts_srv_version;
> static int hb_srv_version;
>
> -#define SD_VER_COUNT 2
> +#define SD_VER_COUNT 3
> static const int sd_versions[] = {
> + SD_VERSION_3_1,
> SD_VERSION,
> SD_VERSION_1
> };
> @@ -118,17 +121,28 @@ static void perform_shutdown(struct work_struct *dummy)
> orderly_poweroff(true);
> }
>
> +static void perform_restart(struct work_struct *dummy)
> +{
> + orderly_reboot();
> +}
> +
> /*
> * Perform the shutdown operation in a thread context.
> */
> static DECLARE_WORK(shutdown_work, perform_shutdown);
>
> +/*
> + * Perform the restart operation in a thread context.
> + */
> +static DECLARE_WORK(restart_work, perform_restart);
> +
> static void shutdown_onchannelcallback(void *context)
> {
> struct vmbus_channel *channel = context;
> u32 recvlen;
> u64 requestid;
> bool execute_shutdown = false;
> + bool execute_reboot = false;
> u8 *shut_txf_buf = util_shutdown.recv_buffer;
>
> struct shutdown_msg_data *shutdown_msg;
> @@ -157,6 +171,12 @@ static void shutdown_onchannelcallback(void *context)
> sizeof(struct vmbuspipe_hdr) +
> sizeof(struct icmsg_hdr)];
>
> + /*
> + * shutdown_msg->flags can be 0 (shut down), 2(reboot),
> + * or 4(hibernate). It may bitwise-OR 1, which means
> + * performing the request by force. Linux always tries
> + * to perform the request by force.
> + */
> switch (shutdown_msg->flags) {
> case 0:
> case 1:
> @@ -166,6 +186,14 @@ static void shutdown_onchannelcallback(void *context)
> pr_info("Shutdown request received -"
> " graceful shutdown initiated\n");
> break;
> + case 2:
> + case 3:
> + icmsghdrp->status = HV_S_OK;
> + execute_reboot = true;
> +
> + pr_info("Restart request received -"
> + " graceful restart initiated\n");
> + break;
> default:
> icmsghdrp->status = HV_E_FAIL;
> execute_shutdown = false;
> @@ -186,6 +214,8 @@ static void shutdown_onchannelcallback(void *context)
>
> if (execute_shutdown == true)
> schedule_work(&shutdown_work);
> + if (execute_reboot == true)
> + schedule_work(&restart_work);

This works, and responds to my comment. But FWIW, a more compact
approach would be to drop the boolean execute_shutdown, execute_restart,
etc. local variables and have just this local variable:

struct work_struct *work_to_do = NULL;

In the "case" branches do:

work_to_do = &shutdown_work;

or
work_to_do = &restart_work;

Then at the bottom of the function, just do:

if (work_to_do)
schedule_work(work_to_do);

Patch 3 of this series would then be a little simpler as well.

> }
>
> /*
> --
> 2.19.1

2020-01-26 05:06:41

by Dexuan Cui

[permalink] [raw]
Subject: RE: [PATCH v3 2/4] hv_utils: Support host-initiated restart request

> From: Michael Kelley <[email protected]>
> Sent: Saturday, January 25, 2020 8:57 PM
> > @@ -186,6 +214,8 @@ static void shutdown_onchannelcallback(void
> *context)
> >
> > if (execute_shutdown == true)
> > schedule_work(&shutdown_work);
> > + if (execute_reboot == true)
> > + schedule_work(&restart_work);
>
> This works, and responds to my comment. But FWIW, a more compact
> approach would be to drop the boolean execute_shutdown, execute_restart,
> etc. local variables and have just this local variable:
>
> struct work_struct *work_to_do = NULL;
>
> In the "case" branches do:
>
> work_to_do = &shutdown_work;
>
> or
> work_to_do = &restart_work;
>
> Then at the bottom of the function, just do:
>
> if (work_to_do)
> schedule_work(work_to_do);
>
> Patch 3 of this series would then be a little simpler as well.

Good idea! I'll do this in v4.

Thanks,
-- Dexuan