* Re: [RFC 0/6] vDSO support for Hyper-V guest on ARM64
From: Marc Zyngier @ 2020-01-28 11:48 UTC (permalink / raw)
To: Boqun Feng
Cc: Vincenzo Frascino, Sasha Levin, linux-hyperv, Stefano Stabellini,
Stephen Hemminger, Catalin Marinas, Haiyang Zhang, linux-kernel,
Michael Kelley, xen-devel, Thomas Gleixner, K. Y. Srinivasan,
Will Deacon, linux-arm-kernel
In-Reply-To: <20200128055846.GA83200@debian-boqun.qqnc3lrjykvubdpftowmye0fmh.lx.internal.cloudapp.net>
On 2020-01-28 05:58, Boqun Feng wrote:
> On Fri, Jan 24, 2020 at 10:24:44AM +0000, Vincenzo Frascino wrote:
>> Hi Boqun Feng,
>>
>> On 24/01/2020 06:32, Boqun Feng wrote:
>> > Hi Vincenzo,
>> >
>>
>> [...]
>>
>> >>
>> >> I had a look to your patches and overall, I could not understand why we can't
>> >> use the arch_timer to do the same things you are doing with the one you
>> >> introduced in this series. What confuses me is that KVM works just fine with the
>> >> arch_timer which was designed with virtualization in mind. Why do we need
>> >> another one? Could you please explain?
>> >>
>> >
>> > Please note that the guest VM on Hyper-V for ARM64 doesn't use
>> > arch_timer as the clocksource. See:
>> >
>> > https://lore.kernel.org/linux-arm-kernel/1570129355-16005-7-git-send-email-mikelley@microsoft.com/
>> >
>> > , ACPI_SIG_GTDT is used for setting up Hyper-V synthetic clocksource
>> > and other initialization work.
>> >
>>
>> I had a look a look at it and my question stands, why do we need
>> another timer
>> on arm64?
>>
>
> Sorry for the late response. It's weekend and Chinese New Year, so I
> got
> to spend some time making (and mostly eating) dumplings ;-)
And you haven't been sharing! ;-)
> After discussion with Michael, here is some explanation why we need
> another timer:
>
> The synthetic clocks that Hyper-V presents in a guest VM were
> originally
> created for the x86 architecture. They provide a level of abstraction
> that solves problems like continuity across live migrations where the
> hardware clock (i.e., TSC in the case x86) frequency may be different
> across the migration. When Hyper-V was brought to ARM64, this
> abstraction was maintained to provide consistency across the x86 and
> ARM64 architectures, and for both Windows and Linux guest VMs. The
> core Linux code for the Hyper-V clocks (in
> drivers/clocksource/hyperv_timer.c) is architecture neutral and works
> on
> both x86 and ARM64. As you can see, this part is done in Michael's
> patchset.
>
> Arguably, Hyper-V for ARM64 should have optimized for consistency with
> the ARM64 community rather with the existing x86 implementation and
> existing guest code in Windows. But at this point, it is what it is,
> and the Hyper-V clocks do solve problems like migration that aren’t
> addressed in ARM64 until v8.4 of the architecture with the addition of
> the counter hardware scaling feature. Hyper-V doesn’t currently map the
> ARM arch timer interrupts into guest VMs, so we need to use the
> existing
> Hyper-V clocks and the common code that already exists.
The migration thing is a bit of a red herring. Do you really anticipate
VM migration across systems that have their timers running at different
frequencies *today*? And even if you did, there are ways to deal with it
with the arch timers (patches to that effect were posted on the list,
and
there was even a bit of an ARM spec for it).
I find it odd to try and make arm64 "just another x86", while the
architecture
gives you most of what you need already. I guess I'm tainted.
Thanks,
M.
--
Who you jivin' with that Cosmik Debris?
^ permalink raw reply
* Re: [RFC 0/6] vDSO support for Hyper-V guest on ARM64
From: Boqun Feng @ 2020-01-28 5:58 UTC (permalink / raw)
To: Vincenzo Frascino
Cc: linux-hyperv, linux-arm-kernel, linux-kernel, Sasha Levin,
Stephen Hemminger, Catalin Marinas, Haiyang Zhang, Michael Kelley,
Stefano Stabellini, xen-devel, Thomas Gleixner, K. Y. Srinivasan,
Will Deacon
In-Reply-To: <4cdf2188-8909-4b90-ca78-92cef520b23d@arm.com>
On Fri, Jan 24, 2020 at 10:24:44AM +0000, Vincenzo Frascino wrote:
> Hi Boqun Feng,
>
> On 24/01/2020 06:32, Boqun Feng wrote:
> > Hi Vincenzo,
> >
>
> [...]
>
> >>
> >> I had a look to your patches and overall, I could not understand why we can't
> >> use the arch_timer to do the same things you are doing with the one you
> >> introduced in this series. What confuses me is that KVM works just fine with the
> >> arch_timer which was designed with virtualization in mind. Why do we need
> >> another one? Could you please explain?
> >>
> >
> > Please note that the guest VM on Hyper-V for ARM64 doesn't use
> > arch_timer as the clocksource. See:
> >
> > https://lore.kernel.org/linux-arm-kernel/1570129355-16005-7-git-send-email-mikelley@microsoft.com/
> >
> > , ACPI_SIG_GTDT is used for setting up Hyper-V synthetic clocksource
> > and other initialization work.
> >
>
> I had a look a look at it and my question stands, why do we need another timer
> on arm64?
>
Sorry for the late response. It's weekend and Chinese New Year, so I got
to spend some time making (and mostly eating) dumplings ;-)
After discussion with Michael, here is some explanation why we need
another timer:
The synthetic clocks that Hyper-V presents in a guest VM were originally
created for the x86 architecture. They provide a level of abstraction
that solves problems like continuity across live migrations where the
hardware clock (i.e., TSC in the case x86) frequency may be different
across the migration. When Hyper-V was brought to ARM64, this
abstraction was maintained to provide consistency across the x86 and
ARM64 architectures, and for both Windows and Linux guest VMs. The
core Linux code for the Hyper-V clocks (in
drivers/clocksource/hyperv_timer.c) is architecture neutral and works on
both x86 and ARM64. As you can see, this part is done in Michael's
patchset.
Arguably, Hyper-V for ARM64 should have optimized for consistency with
the ARM64 community rather with the existing x86 implementation and
existing guest code in Windows. But at this point, it is what it is,
and the Hyper-V clocks do solve problems like migration that aren’t
addressed in ARM64 until v8.4 of the architecture with the addition of
the counter hardware scaling feature. Hyper-V doesn’t currently map the
ARM arch timer interrupts into guest VMs, so we need to use the existing
Hyper-V clocks and the common code that already exists.
Does the above answer your question?
Regards,
Boqun
> > So just to be clear, your suggestion is
> >
> > 1) Hyper-V guest on ARM64 should use arch_timer as clocksource and vDSO
> > will just work.
> >
> > or
> >
> > 2) Even though arch_timer is not used as the clocksource, we can still
> > use it for vDSO.
> >
> > ?
> >
>
> Option #1 would be the preferred solution, unless there is a good reason against.
>
> > Regards,
> > Boqun
> >
>
> --
> Regards,
> Vincenzo
^ permalink raw reply
* RE: [PATCH v4 3/4] hv_utils: Support host-initiated hibernation request
From: Michael Kelley @ 2020-01-26 6:08 UTC (permalink / raw)
To: Dexuan Cui, KY Srinivasan, Haiyang Zhang, Stephen Hemminger,
sashal@kernel.org, linux-hyperv@vger.kernel.org,
linux-kernel@vger.kernel.org, vkuznets
In-Reply-To: <1580017784-103557-4-git-send-email-decui@microsoft.com>
From: Dexuan Cui <decui@microsoft.com> Sent: Saturday, January 25, 2020 9:50 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>
>
> ---
> 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.
>
> Changes in v4 (Thanks to Michael!):
> Used a compact way to handle the work item.
>
> drivers/hv/hv_util.c | 49 +++++++++++++++++++++++++++++++++++++++++++-
> 1 file changed, 48 insertions(+), 1 deletion(-)
>
Reviewed-by: Michael Kelley <mikelley@microsoft.com>
^ permalink raw reply
* RE: [PATCH v4 2/4] hv_utils: Support host-initiated restart request
From: Michael Kelley @ 2020-01-26 6:05 UTC (permalink / raw)
To: Dexuan Cui, KY Srinivasan, Haiyang Zhang, Stephen Hemminger,
sashal@kernel.org, linux-hyperv@vger.kernel.org,
linux-kernel@vger.kernel.org, vkuznets
In-Reply-To: <1580017784-103557-3-git-send-email-decui@microsoft.com>
From: Dexuan Cui <decui@microsoft.com> Sent: Saturday, January 25, 2020 9:50 PM
>
> 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 <decui@microsoft.com>
>
> ---
> 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.
>
> Changes in v4 (Thanks to Michael!):
> Used a compact way to handle the work items.
>
> drivers/hv/hv_util.c | 39 +++++++++++++++++++++++++++++++--------
> 1 file changed, 31 insertions(+), 8 deletions(-)
>
Reviewed-by: Michael Kelley <mikelley@microsoft.com>
^ permalink raw reply
* RE: [PATCH v4 1/4] Tools: hv: Reopen the devices if read() or write() returns errors
From: Michael Kelley @ 2020-01-26 6:02 UTC (permalink / raw)
To: Dexuan Cui, KY Srinivasan, Haiyang Zhang, Stephen Hemminger,
sashal@kernel.org, linux-hyperv@vger.kernel.org,
linux-kernel@vger.kernel.org, vkuznets
In-Reply-To: <1580017784-103557-2-git-send-email-decui@microsoft.com>
From: Dexuan Cui <decui@microsoft.com> Sent: Saturday, January 25, 2020 9:50 PM
>
> The state machine in the hv_utils driver can run out of order in some
> corner cases, e.g. if the kvp daemon doesn't call write() fast enough
> due to some reason, kvp_timeout_func() can run first and move the state
> to HVUTIL_READY; next, when kvp_on_msg() is called it returns -EINVAL
> since kvp_transaction.state is smaller than HVUTIL_USERSPACE_REQ; later,
> the daemon's write() gets an error -EINVAL, and the daemon will exit().
>
> We can reproduce the issue by sending a SIGSTOP signal to the daemon, wait
> for 1 minute, and send a SIGCONT signal to the daemon: the daemon will
> exit() quickly.
>
> We can fix the issue by forcing a reset of the device (which means the
> daemon can close() and open() the device again) and doing extra necessary
> clean-up.
>
> Signed-off-by: Dexuan Cui <decui@microsoft.com>
>
> ---
> Changes in v2:
> This is actually a new patch that makes the daemons more robust.
>
> Changes in v3 (I addressed Michael's comments):
> Don't reset target_fd, since that's unnecessary.
> Reset target_fname by: target_fname[0] = '\0';
> Added the missing "fs_frozen = true;" in vss_operate().
> Just after reopen_vss_fd: if vss_operate(VSS_OP_THAW) can not clear
> fs_frozen due to an error, we just exit().
> Added comments.
>
> Changes in v4 (Thanks to Michael!):
> Added the omitted "int fcopy_fd = -1" and
> "
> if (fcopy_fd != -1)
> close(fcopy_fd);
> "
>
> tools/hv/hv_fcopy_daemon.c | 37 ++++++++++++++++++++++++----
> tools/hv/hv_kvp_daemon.c | 36 ++++++++++++++++------------
> tools/hv/hv_vss_daemon.c | 49 +++++++++++++++++++++++++++++---------
> 3 files changed, 91 insertions(+), 31 deletions(-)
>
Reviewed-by: Michael Kelley <mikelley@microsoft.com>
^ permalink raw reply
* [PATCH v4 1/4] Tools: hv: Reopen the devices if read() or write() returns errors
From: Dexuan Cui @ 2020-01-26 5:49 UTC (permalink / raw)
To: kys, haiyangz, sthemmin, sashal, linux-hyperv, linux-kernel,
mikelley, vkuznets
Cc: Dexuan Cui
In-Reply-To: <1580017784-103557-1-git-send-email-decui@microsoft.com>
The state machine in the hv_utils driver can run out of order in some
corner cases, e.g. if the kvp daemon doesn't call write() fast enough
due to some reason, kvp_timeout_func() can run first and move the state
to HVUTIL_READY; next, when kvp_on_msg() is called it returns -EINVAL
since kvp_transaction.state is smaller than HVUTIL_USERSPACE_REQ; later,
the daemon's write() gets an error -EINVAL, and the daemon will exit().
We can reproduce the issue by sending a SIGSTOP signal to the daemon, wait
for 1 minute, and send a SIGCONT signal to the daemon: the daemon will
exit() quickly.
We can fix the issue by forcing a reset of the device (which means the
daemon can close() and open() the device again) and doing extra necessary
clean-up.
Signed-off-by: Dexuan Cui <decui@microsoft.com>
---
Changes in v2:
This is actually a new patch that makes the daemons more robust.
Changes in v3 (I addressed Michael's comments):
Don't reset target_fd, since that's unnecessary.
Reset target_fname by: target_fname[0] = '\0';
Added the missing "fs_frozen = true;" in vss_operate().
Just after reopen_vss_fd: if vss_operate(VSS_OP_THAW) can not clear
fs_frozen due to an error, we just exit().
Added comments.
Changes in v4 (Thanks to Michael!):
Added the omitted "int fcopy_fd = -1" and
"
if (fcopy_fd != -1)
close(fcopy_fd);
"
tools/hv/hv_fcopy_daemon.c | 37 ++++++++++++++++++++++++----
tools/hv/hv_kvp_daemon.c | 36 ++++++++++++++++------------
tools/hv/hv_vss_daemon.c | 49 +++++++++++++++++++++++++++++---------
3 files changed, 91 insertions(+), 31 deletions(-)
diff --git a/tools/hv/hv_fcopy_daemon.c b/tools/hv/hv_fcopy_daemon.c
index aea2d91ab364..16d629b22c25 100644
--- a/tools/hv/hv_fcopy_daemon.c
+++ b/tools/hv/hv_fcopy_daemon.c
@@ -80,6 +80,8 @@ static int hv_start_fcopy(struct hv_start_fcopy *smsg)
error = 0;
done:
+ if (error)
+ target_fname[0] = '\0';
return error;
}
@@ -108,15 +110,29 @@ static int hv_copy_data(struct hv_do_fcopy *cpmsg)
return ret;
}
+/*
+ * Reset target_fname to "" in the two below functions for hibernation: if
+ * the fcopy operation is aborted by hibernation, the daemon should remove the
+ * partially-copied file; to achieve this, the hv_utils driver always fakes a
+ * CANCEL_FCOPY message upon suspend, and later when the VM resumes back,
+ * the daemon calls hv_copy_cancel() to remove the file; if a file is copied
+ * successfully before suspend, hv_copy_finished() must reset target_fname to
+ * avoid that the file can be incorrectly removed upon resume, since the faked
+ * CANCEL_FCOPY message is spurious in this case.
+ */
static int hv_copy_finished(void)
{
close(target_fd);
+ target_fname[0] = '\0';
return 0;
}
static int hv_copy_cancel(void)
{
close(target_fd);
- unlink(target_fname);
+ if (strlen(target_fname) > 0) {
+ unlink(target_fname);
+ target_fname[0] = '\0';
+ }
return 0;
}
@@ -131,7 +147,7 @@ void print_usage(char *argv[])
int main(int argc, char *argv[])
{
- int fcopy_fd;
+ int fcopy_fd = -1;
int error;
int daemonize = 1, long_index = 0, opt;
int version = FCOPY_CURRENT_VERSION;
@@ -141,7 +157,7 @@ int main(int argc, char *argv[])
struct hv_do_fcopy copy;
__u32 kernel_modver;
} buffer = { };
- int in_handshake = 1;
+ int in_handshake;
static struct option long_options[] = {
{"help", no_argument, 0, 'h' },
@@ -170,6 +186,12 @@ int main(int argc, char *argv[])
openlog("HV_FCOPY", 0, LOG_USER);
syslog(LOG_INFO, "starting; pid is:%d", getpid());
+reopen_fcopy_fd:
+ if (fcopy_fd != -1)
+ close(fcopy_fd);
+ /* Remove any possible partially-copied file on error */
+ hv_copy_cancel();
+ in_handshake = 1;
fcopy_fd = open("/dev/vmbus/hv_fcopy", O_RDWR);
if (fcopy_fd < 0) {
@@ -196,7 +218,7 @@ int main(int argc, char *argv[])
len = pread(fcopy_fd, &buffer, sizeof(buffer), 0);
if (len < 0) {
syslog(LOG_ERR, "pread failed: %s", strerror(errno));
- exit(EXIT_FAILURE);
+ goto reopen_fcopy_fd;
}
if (in_handshake) {
@@ -231,9 +253,14 @@ int main(int argc, char *argv[])
}
+ /*
+ * pwrite() may return an error due to the faked CANCEL_FCOPY
+ * message upon hibernation. Ignore the error by resetting the
+ * dev file, i.e. closing and re-opening it.
+ */
if (pwrite(fcopy_fd, &error, sizeof(int), 0) != sizeof(int)) {
syslog(LOG_ERR, "pwrite failed: %s", strerror(errno));
- exit(EXIT_FAILURE);
+ goto reopen_fcopy_fd;
}
}
}
diff --git a/tools/hv/hv_kvp_daemon.c b/tools/hv/hv_kvp_daemon.c
index e9ef4ca6a655..ee9c1bb2293e 100644
--- a/tools/hv/hv_kvp_daemon.c
+++ b/tools/hv/hv_kvp_daemon.c
@@ -76,7 +76,7 @@ enum {
DNS
};
-static int in_hand_shake = 1;
+static int in_hand_shake;
static char *os_name = "";
static char *os_major = "";
@@ -1360,7 +1360,7 @@ void print_usage(char *argv[])
int main(int argc, char *argv[])
{
- int kvp_fd, len;
+ int kvp_fd = -1, len;
int error;
struct pollfd pfd;
char *p;
@@ -1400,14 +1400,6 @@ int main(int argc, char *argv[])
openlog("KVP", 0, LOG_USER);
syslog(LOG_INFO, "KVP starting; pid is:%d", getpid());
- kvp_fd = open("/dev/vmbus/hv_kvp", O_RDWR | O_CLOEXEC);
-
- if (kvp_fd < 0) {
- syslog(LOG_ERR, "open /dev/vmbus/hv_kvp failed; error: %d %s",
- errno, strerror(errno));
- exit(EXIT_FAILURE);
- }
-
/*
* Retrieve OS release information.
*/
@@ -1423,6 +1415,18 @@ int main(int argc, char *argv[])
exit(EXIT_FAILURE);
}
+reopen_kvp_fd:
+ if (kvp_fd != -1)
+ close(kvp_fd);
+ in_hand_shake = 1;
+ kvp_fd = open("/dev/vmbus/hv_kvp", O_RDWR | O_CLOEXEC);
+
+ if (kvp_fd < 0) {
+ syslog(LOG_ERR, "open /dev/vmbus/hv_kvp failed; error: %d %s",
+ errno, strerror(errno));
+ exit(EXIT_FAILURE);
+ }
+
/*
* Register ourselves with the kernel.
*/
@@ -1456,9 +1460,7 @@ int main(int argc, char *argv[])
if (len != sizeof(struct hv_kvp_msg)) {
syslog(LOG_ERR, "read failed; error:%d %s",
errno, strerror(errno));
-
- close(kvp_fd);
- return EXIT_FAILURE;
+ goto reopen_kvp_fd;
}
/*
@@ -1617,13 +1619,17 @@ int main(int argc, char *argv[])
break;
}
- /* Send the value back to the kernel. */
+ /*
+ * Send the value back to the kernel. Note: the write() may
+ * return an error due to hibernation; we can ignore the error
+ * by resetting the dev file, i.e. closing and re-opening it.
+ */
kvp_done:
len = write(kvp_fd, hv_msg, sizeof(struct hv_kvp_msg));
if (len != sizeof(struct hv_kvp_msg)) {
syslog(LOG_ERR, "write failed; error: %d %s", errno,
strerror(errno));
- exit(EXIT_FAILURE);
+ goto reopen_kvp_fd;
}
}
diff --git a/tools/hv/hv_vss_daemon.c b/tools/hv/hv_vss_daemon.c
index 92902a88f671..dd111870beee 100644
--- a/tools/hv/hv_vss_daemon.c
+++ b/tools/hv/hv_vss_daemon.c
@@ -28,6 +28,8 @@
#include <stdbool.h>
#include <dirent.h>
+static bool fs_frozen;
+
/* Don't use syslog() in the function since that can cause write to disk */
static int vss_do_freeze(char *dir, unsigned int cmd)
{
@@ -155,18 +157,27 @@ static int vss_operate(int operation)
continue;
}
error |= vss_do_freeze(ent->mnt_dir, cmd);
- if (error && operation == VSS_OP_FREEZE)
- goto err;
+ if (operation == VSS_OP_FREEZE) {
+ if (error)
+ goto err;
+ fs_frozen = true;
+ }
}
endmntent(mounts);
if (root_seen) {
error |= vss_do_freeze("/", cmd);
- if (error && operation == VSS_OP_FREEZE)
- goto err;
+ if (operation == VSS_OP_FREEZE) {
+ if (error)
+ goto err;
+ fs_frozen = true;
+ }
}
+ if (operation == VSS_OP_THAW && !error)
+ fs_frozen = false;
+
goto out;
err:
save_errno = errno;
@@ -175,6 +186,7 @@ static int vss_operate(int operation)
endmntent(mounts);
}
vss_operate(VSS_OP_THAW);
+ fs_frozen = false;
/* Call syslog after we thaw all filesystems */
if (ent)
syslog(LOG_ERR, "FREEZE of %s failed; error:%d %s",
@@ -196,13 +208,13 @@ void print_usage(char *argv[])
int main(int argc, char *argv[])
{
- int vss_fd, len;
+ int vss_fd = -1, len;
int error;
struct pollfd pfd;
int op;
struct hv_vss_msg vss_msg[1];
int daemonize = 1, long_index = 0, opt;
- int in_handshake = 1;
+ int in_handshake;
__u32 kernel_modver;
static struct option long_options[] = {
@@ -232,6 +244,18 @@ int main(int argc, char *argv[])
openlog("Hyper-V VSS", 0, LOG_USER);
syslog(LOG_INFO, "VSS starting; pid is:%d", getpid());
+reopen_vss_fd:
+ if (vss_fd != -1)
+ close(vss_fd);
+ if (fs_frozen) {
+ if (vss_operate(VSS_OP_THAW) || fs_frozen) {
+ syslog(LOG_ERR, "failed to thaw file system: err=%d",
+ errno);
+ exit(EXIT_FAILURE);
+ }
+ }
+
+ in_handshake = 1;
vss_fd = open("/dev/vmbus/hv_vss", O_RDWR);
if (vss_fd < 0) {
syslog(LOG_ERR, "open /dev/vmbus/hv_vss failed; error: %d %s",
@@ -284,8 +308,7 @@ int main(int argc, char *argv[])
if (len != sizeof(struct hv_vss_msg)) {
syslog(LOG_ERR, "read failed; error:%d %s",
errno, strerror(errno));
- close(vss_fd);
- return EXIT_FAILURE;
+ goto reopen_vss_fd;
}
op = vss_msg->vss_hdr.operation;
@@ -312,14 +335,18 @@ int main(int argc, char *argv[])
default:
syslog(LOG_ERR, "Illegal op:%d\n", op);
}
+
+ /*
+ * The write() may return an error due to the faked VSS_OP_THAW
+ * message upon hibernation. Ignore the error by resetting the
+ * dev file, i.e. closing and re-opening it.
+ */
vss_msg->error = error;
len = write(vss_fd, vss_msg, sizeof(struct hv_vss_msg));
if (len != sizeof(struct hv_vss_msg)) {
syslog(LOG_ERR, "write failed; error: %d %s", errno,
strerror(errno));
-
- if (op == VSS_OP_FREEZE)
- vss_operate(VSS_OP_THAW);
+ goto reopen_vss_fd;
}
}
--
2.19.1
^ permalink raw reply related
* [PATCH v4 2/4] hv_utils: Support host-initiated restart request
From: Dexuan Cui @ 2020-01-26 5:49 UTC (permalink / raw)
To: kys, haiyangz, sthemmin, sashal, linux-hyperv, linux-kernel,
mikelley, vkuznets
Cc: Dexuan Cui
In-Reply-To: <1580017784-103557-1-git-send-email-decui@microsoft.com>
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 <decui@microsoft.com>
---
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.
Changes in v4 (Thanks to Michael!):
Used a compact way to handle the work items.
drivers/hv/hv_util.c | 39 +++++++++++++++++++++++++++++++--------
1 file changed, 31 insertions(+), 8 deletions(-)
diff --git a/drivers/hv/hv_util.c b/drivers/hv/hv_util.c
index 766bd8457346..32d96af67522 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,27 @@ 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;
+ struct work_struct *work = NULL;
u32 recvlen;
u64 requestid;
- bool execute_shutdown = false;
u8 *shut_txf_buf = util_shutdown.recv_buffer;
struct shutdown_msg_data *shutdown_msg;
@@ -157,19 +170,29 @@ 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:
icmsghdrp->status = HV_S_OK;
- execute_shutdown = true;
-
+ work = &shutdown_work;
pr_info("Shutdown request received -"
" graceful shutdown initiated\n");
break;
+ case 2:
+ case 3:
+ icmsghdrp->status = HV_S_OK;
+ work = &restart_work;
+ pr_info("Restart request received -"
+ " graceful restart initiated\n");
+ break;
default:
icmsghdrp->status = HV_E_FAIL;
- execute_shutdown = false;
-
pr_info("Shutdown request received -"
" Invalid request\n");
break;
@@ -184,8 +207,8 @@ static void shutdown_onchannelcallback(void *context)
VM_PKT_DATA_INBAND, 0);
}
- if (execute_shutdown == true)
- schedule_work(&shutdown_work);
+ if (work)
+ schedule_work(work);
}
/*
--
2.19.1
^ permalink raw reply related
* [PATCH v4 3/4] hv_utils: Support host-initiated hibernation request
From: Dexuan Cui @ 2020-01-26 5:49 UTC (permalink / raw)
To: kys, haiyangz, sthemmin, sashal, linux-hyperv, linux-kernel,
mikelley, vkuznets
Cc: Dexuan Cui
In-Reply-To: <1580017784-103557-1-git-send-email-decui@microsoft.com>
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>
---
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.
Changes in v4 (Thanks to Michael!):
Used a compact way to handle the work item.
drivers/hv/hv_util.c | 49 +++++++++++++++++++++++++++++++++++++++++++-
1 file changed, 48 insertions(+), 1 deletion(-)
diff --git a/drivers/hv/hv_util.c b/drivers/hv/hv_util.c
index 32d96af67522..64fbf4ac80f9 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);
@@ -191,6 +230,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;
+ if (hibernation_supported)
+ work = &hibernate_context.work;
+ break;
default:
icmsghdrp->status = HV_E_FAIL;
pr_info("Shutdown request received -"
--
2.19.1
^ permalink raw reply related
* [PATCH v4 4/4] hv_utils: Add the support of hibernation
From: Dexuan Cui @ 2020-01-26 5:49 UTC (permalink / raw)
To: kys, haiyangz, sthemmin, sashal, linux-hyperv, linux-kernel,
mikelley, vkuznets
Cc: Dexuan Cui
In-Reply-To: <1580017784-103557-1-git-send-email-decui@microsoft.com>
Add util_pre_suspend() and util_pre_resume() for some hv_utils devices
(e.g. kvp/vss/fcopy), because they need special handling before
util_suspend() calls vmbus_close().
For kvp, all the possible pending work items should be cancelled.
For vss and fcopy, some extra clean-up needs to be done, i.e. fake a
THAW message for hv_vss_daemon and fake a CANCEL_FCOPY message for
hv_fcopy_daemon, otherwise when the VM resums back, the daemons
can end up in an inconsistent state (i.e. the file systems are
frozen but will never be thawed; the file transmitted via fcopy
may not be complete). Note: there is an extra patch for the daemons:
"Tools: hv: Reopen the devices if read() or write() returns errors",
because the hv_utils driver can not guarantee the whole transaction
finishes completely once util_suspend() starts to run (at this time,
all the userspace processes are frozen).
util_probe() disables channel->callback_event to avoid the race with
the channel callback.
Signed-off-by: Dexuan Cui <decui@microsoft.com>
Reviewed-by: Michael Kelley <mikelley@microsoft.com>
---
Changes in v2:
Handles fcopy/vss specially to avoid possible inconsistent states.
Changes in v3 (I addressed Michael's comments):
Removed unneeded blank lines.
Simplified the error handling logic by allocating memory earlier.
Added a comment before util_suspend(): when we're in the function,
all the userspace processes have been frozen.
Changes in v4:
Added Michael's Reviewed-by.
drivers/hv/hv_fcopy.c | 54 +++++++++++++++++++++++++++++++++-
drivers/hv/hv_kvp.c | 43 +++++++++++++++++++++++++--
drivers/hv/hv_snapshot.c | 55 ++++++++++++++++++++++++++++++++--
drivers/hv/hv_util.c | 62 ++++++++++++++++++++++++++++++++++++++-
drivers/hv/hyperv_vmbus.h | 6 ++++
include/linux/hyperv.h | 2 ++
6 files changed, 216 insertions(+), 6 deletions(-)
diff --git a/drivers/hv/hv_fcopy.c b/drivers/hv/hv_fcopy.c
index 08fa4a5de644..bb9ba3f7c794 100644
--- a/drivers/hv/hv_fcopy.c
+++ b/drivers/hv/hv_fcopy.c
@@ -346,9 +346,61 @@ int hv_fcopy_init(struct hv_util_service *srv)
return 0;
}
+static void hv_fcopy_cancel_work(void)
+{
+ cancel_delayed_work_sync(&fcopy_timeout_work);
+ cancel_work_sync(&fcopy_send_work);
+}
+
+int hv_fcopy_pre_suspend(void)
+{
+ struct vmbus_channel *channel = fcopy_transaction.recv_channel;
+ struct hv_fcopy_hdr *fcopy_msg;
+
+ /*
+ * Fake a CANCEL_FCOPY message for the user space daemon in case the
+ * daemon is in the middle of copying some file. It doesn't matter if
+ * there is already a message pending to be delivered to the user
+ * space since we force fcopy_transaction.state to be HVUTIL_READY, so
+ * the user space daemon's write() will fail with EINVAL (see
+ * fcopy_on_msg()), and the daemon will reset the device by closing
+ * and re-opening it.
+ */
+ fcopy_msg = kzalloc(sizeof(*fcopy_msg), GFP_KERNEL);
+ if (!fcopy_msg)
+ return -ENOMEM;
+
+ tasklet_disable(&channel->callback_event);
+
+ fcopy_msg->operation = CANCEL_FCOPY;
+
+ hv_fcopy_cancel_work();
+
+ /* We don't care about the return value. */
+ hvutil_transport_send(hvt, fcopy_msg, sizeof(*fcopy_msg), NULL);
+
+ kfree(fcopy_msg);
+
+ fcopy_transaction.state = HVUTIL_READY;
+
+ /* tasklet_enable() will be called in hv_fcopy_pre_resume(). */
+ return 0;
+}
+
+int hv_fcopy_pre_resume(void)
+{
+ struct vmbus_channel *channel = fcopy_transaction.recv_channel;
+
+ tasklet_enable(&channel->callback_event);
+
+ return 0;
+}
+
void hv_fcopy_deinit(void)
{
fcopy_transaction.state = HVUTIL_DEVICE_DYING;
- cancel_delayed_work_sync(&fcopy_timeout_work);
+
+ hv_fcopy_cancel_work();
+
hvutil_transport_destroy(hvt);
}
diff --git a/drivers/hv/hv_kvp.c b/drivers/hv/hv_kvp.c
index ae7c028dc5a8..e74b144b8f3d 100644
--- a/drivers/hv/hv_kvp.c
+++ b/drivers/hv/hv_kvp.c
@@ -758,11 +758,50 @@ hv_kvp_init(struct hv_util_service *srv)
return 0;
}
-void hv_kvp_deinit(void)
+static void hv_kvp_cancel_work(void)
{
- kvp_transaction.state = HVUTIL_DEVICE_DYING;
cancel_delayed_work_sync(&kvp_host_handshake_work);
cancel_delayed_work_sync(&kvp_timeout_work);
cancel_work_sync(&kvp_sendkey_work);
+}
+
+int hv_kvp_pre_suspend(void)
+{
+ struct vmbus_channel *channel = kvp_transaction.recv_channel;
+
+ tasklet_disable(&channel->callback_event);
+
+ /*
+ * If there is a pending transtion, it's unnecessary to tell the host
+ * that the transaction will fail, because that is implied when
+ * util_suspend() calls vmbus_close() later.
+ */
+ hv_kvp_cancel_work();
+
+ /*
+ * Forece the state to READY to handle the ICMSGTYPE_NEGOTIATE message
+ * later. The user space daemon may go out of order and its write()
+ * may fail with EINVAL: this doesn't matter since the daemon will
+ * reset the device by closing and re-opening it.
+ */
+ kvp_transaction.state = HVUTIL_READY;
+ return 0;
+}
+
+int hv_kvp_pre_resume(void)
+{
+ struct vmbus_channel *channel = kvp_transaction.recv_channel;
+
+ tasklet_enable(&channel->callback_event);
+
+ return 0;
+}
+
+void hv_kvp_deinit(void)
+{
+ kvp_transaction.state = HVUTIL_DEVICE_DYING;
+
+ hv_kvp_cancel_work();
+
hvutil_transport_destroy(hvt);
}
diff --git a/drivers/hv/hv_snapshot.c b/drivers/hv/hv_snapshot.c
index 03b6454268b3..1c75b38f0d6d 100644
--- a/drivers/hv/hv_snapshot.c
+++ b/drivers/hv/hv_snapshot.c
@@ -379,10 +379,61 @@ hv_vss_init(struct hv_util_service *srv)
return 0;
}
-void hv_vss_deinit(void)
+static void hv_vss_cancel_work(void)
{
- vss_transaction.state = HVUTIL_DEVICE_DYING;
cancel_delayed_work_sync(&vss_timeout_work);
cancel_work_sync(&vss_handle_request_work);
+}
+
+int hv_vss_pre_suspend(void)
+{
+ struct vmbus_channel *channel = vss_transaction.recv_channel;
+ struct hv_vss_msg *vss_msg;
+
+ /*
+ * Fake a THAW message for the user space daemon in case the daemon
+ * has frozen the file systems. It doesn't matter if there is already
+ * a message pending to be delivered to the user space since we force
+ * vss_transaction.state to be HVUTIL_READY, so the user space daemon's
+ * write() will fail with EINVAL (see vss_on_msg()), and the daemon
+ * will reset the device by closing and re-opening it.
+ */
+ vss_msg = kzalloc(sizeof(*vss_msg), GFP_KERNEL);
+ if (!vss_msg)
+ return -ENOMEM;
+
+ tasklet_disable(&channel->callback_event);
+
+ vss_msg->vss_hdr.operation = VSS_OP_THAW;
+
+ /* Cancel any possible pending work. */
+ hv_vss_cancel_work();
+
+ /* We don't care about the return value. */
+ hvutil_transport_send(hvt, vss_msg, sizeof(*vss_msg), NULL);
+
+ kfree(vss_msg);
+
+ vss_transaction.state = HVUTIL_READY;
+
+ /* tasklet_enable() will be called in hv_vss_pre_resume(). */
+ return 0;
+}
+
+int hv_vss_pre_resume(void)
+{
+ struct vmbus_channel *channel = vss_transaction.recv_channel;
+
+ tasklet_enable(&channel->callback_event);
+
+ return 0;
+}
+
+void hv_vss_deinit(void)
+{
+ vss_transaction.state = HVUTIL_DEVICE_DYING;
+
+ hv_vss_cancel_work();
+
hvutil_transport_destroy(hvt);
}
diff --git a/drivers/hv/hv_util.c b/drivers/hv/hv_util.c
index 64fbf4ac80f9..900b8a8af57c 100644
--- a/drivers/hv/hv_util.c
+++ b/drivers/hv/hv_util.c
@@ -123,12 +123,14 @@ static struct hv_util_service util_shutdown = {
};
static int hv_timesync_init(struct hv_util_service *srv);
+static int hv_timesync_pre_suspend(void);
static void hv_timesync_deinit(void);
static void timesync_onchannelcallback(void *context);
static struct hv_util_service util_timesynch = {
.util_cb = timesync_onchannelcallback,
.util_init = hv_timesync_init,
+ .util_pre_suspend = hv_timesync_pre_suspend,
.util_deinit = hv_timesync_deinit,
};
@@ -140,18 +142,24 @@ static struct hv_util_service util_heartbeat = {
static struct hv_util_service util_kvp = {
.util_cb = hv_kvp_onchannelcallback,
.util_init = hv_kvp_init,
+ .util_pre_suspend = hv_kvp_pre_suspend,
+ .util_pre_resume = hv_kvp_pre_resume,
.util_deinit = hv_kvp_deinit,
};
static struct hv_util_service util_vss = {
.util_cb = hv_vss_onchannelcallback,
.util_init = hv_vss_init,
+ .util_pre_suspend = hv_vss_pre_suspend,
+ .util_pre_resume = hv_vss_pre_resume,
.util_deinit = hv_vss_deinit,
};
static struct hv_util_service util_fcopy = {
.util_cb = hv_fcopy_onchannelcallback,
.util_init = hv_fcopy_init,
+ .util_pre_suspend = hv_fcopy_pre_suspend,
+ .util_pre_resume = hv_fcopy_pre_resume,
.util_deinit = hv_fcopy_deinit,
};
@@ -511,6 +519,44 @@ static int util_remove(struct hv_device *dev)
return 0;
}
+/*
+ * When we're in util_suspend(), all the userspace processes have been frozen
+ * (refer to hibernate() -> freeze_processes()). The userspace is thawed only
+ * after the whole resume procedure, including util_resume(), finishes.
+ */
+static int util_suspend(struct hv_device *dev)
+{
+ struct hv_util_service *srv = hv_get_drvdata(dev);
+ int ret = 0;
+
+ if (srv->util_pre_suspend) {
+ ret = srv->util_pre_suspend();
+ if (ret)
+ return ret;
+ }
+
+ vmbus_close(dev->channel);
+
+ return 0;
+}
+
+static int util_resume(struct hv_device *dev)
+{
+ struct hv_util_service *srv = hv_get_drvdata(dev);
+ int ret = 0;
+
+ if (srv->util_pre_resume) {
+ ret = srv->util_pre_resume();
+ if (ret)
+ return ret;
+ }
+
+ ret = vmbus_open(dev->channel, 4 * HV_HYP_PAGE_SIZE,
+ 4 * HV_HYP_PAGE_SIZE, NULL, 0, srv->util_cb,
+ dev->channel);
+ return ret;
+}
+
static const struct hv_vmbus_device_id id_table[] = {
/* Shutdown guid */
{ HV_SHUTDOWN_GUID,
@@ -547,6 +593,8 @@ static struct hv_driver util_drv = {
.id_table = id_table,
.probe = util_probe,
.remove = util_remove,
+ .suspend = util_suspend,
+ .resume = util_resume,
.driver = {
.probe_type = PROBE_PREFER_ASYNCHRONOUS,
},
@@ -616,11 +664,23 @@ static int hv_timesync_init(struct hv_util_service *srv)
return 0;
}
+static void hv_timesync_cancel_work(void)
+{
+ cancel_work_sync(&adj_time_work);
+}
+
+static int hv_timesync_pre_suspend(void)
+{
+ hv_timesync_cancel_work();
+ return 0;
+}
+
static void hv_timesync_deinit(void)
{
if (hv_ptp_clock)
ptp_clock_unregister(hv_ptp_clock);
- cancel_work_sync(&adj_time_work);
+
+ hv_timesync_cancel_work();
}
static int __init init_hyperv_utils(void)
diff --git a/drivers/hv/hyperv_vmbus.h b/drivers/hv/hyperv_vmbus.h
index 20edcfd3b96c..f5fa3b3c9baf 100644
--- a/drivers/hv/hyperv_vmbus.h
+++ b/drivers/hv/hyperv_vmbus.h
@@ -352,14 +352,20 @@ void vmbus_on_msg_dpc(unsigned long data);
int hv_kvp_init(struct hv_util_service *srv);
void hv_kvp_deinit(void);
+int hv_kvp_pre_suspend(void);
+int hv_kvp_pre_resume(void);
void hv_kvp_onchannelcallback(void *context);
int hv_vss_init(struct hv_util_service *srv);
void hv_vss_deinit(void);
+int hv_vss_pre_suspend(void);
+int hv_vss_pre_resume(void);
void hv_vss_onchannelcallback(void *context);
int hv_fcopy_init(struct hv_util_service *srv);
void hv_fcopy_deinit(void);
+int hv_fcopy_pre_suspend(void);
+int hv_fcopy_pre_resume(void);
void hv_fcopy_onchannelcallback(void *context);
void vmbus_initiate_unload(bool crash);
diff --git a/include/linux/hyperv.h b/include/linux/hyperv.h
index 41c58011431e..692c89ccf5df 100644
--- a/include/linux/hyperv.h
+++ b/include/linux/hyperv.h
@@ -1435,6 +1435,8 @@ struct hv_util_service {
void (*util_cb)(void *);
int (*util_init)(struct hv_util_service *);
void (*util_deinit)(void);
+ int (*util_pre_suspend)(void);
+ int (*util_pre_resume)(void);
};
struct vmbuspipe_hdr {
--
2.19.1
^ permalink raw reply related
* [PATCH v4 0/4] hv_utils: Add the support of hibernation
From: Dexuan Cui @ 2020-01-26 5:49 UTC (permalink / raw)
To: kys, haiyangz, sthemmin, sashal, linux-hyperv, linux-kernel,
mikelley, vkuznets
Cc: Dexuan Cui
Hi,
I addressed the new 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 | 148 ++++++++++++++++++++++++++++++++++---
drivers/hv/hyperv_vmbus.h | 6 ++
include/linux/hyperv.h | 2 +
tools/hv/hv_fcopy_daemon.c | 37 ++++++++--
tools/hv/hv_kvp_daemon.c | 36 +++++----
tools/hv/hv_vss_daemon.c | 49 +++++++++---
9 files changed, 385 insertions(+), 45 deletions(-)
--
2.19.1
^ permalink raw reply
* RE: [PATCH v3 2/4] hv_utils: Support host-initiated restart request
From: Dexuan Cui @ 2020-01-26 5:05 UTC (permalink / raw)
To: Michael Kelley, KY Srinivasan, Haiyang Zhang, Stephen Hemminger,
sashal@kernel.org, linux-hyperv@vger.kernel.org,
linux-kernel@vger.kernel.org, vkuznets
In-Reply-To: <MW2PR2101MB10527D5A3D4F7E52568D119DD7080@MW2PR2101MB1052.namprd21.prod.outlook.com>
> From: Michael Kelley <mikelley@microsoft.com>
> 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
^ permalink raw reply
* RE: [PATCH v3 1/4] Tools: hv: Reopen the devices if read() or write() returns errors
From: Dexuan Cui @ 2020-01-26 4:59 UTC (permalink / raw)
To: Michael Kelley, KY Srinivasan, Haiyang Zhang, Stephen Hemminger,
sashal@kernel.org, linux-hyperv@vger.kernel.org,
linux-kernel@vger.kernel.org, vkuznets
In-Reply-To: <MW2PR2101MB105256D6229F97FC16DC4735D7080@MW2PR2101MB1052.namprd21.prod.outlook.com>
> From: Michael Kelley <mikelley@microsoft.com>
> Sent: Saturday, January 25, 2020 8:49 PM
> > +reopen_fcopy_fd:
> > + /* Remove any possible partially-copied file on error */
> > + hv_copy_cancel();
>
> Since you have removed the calls to close(fcopy_fd) after a
> pread() or pwrite() failure that were in v2 of the patch, I was
> expecting to see
>
> if (fcopy_fd != -1)
> close(fcopy_fd)
I missed this... Thanks for catching it!
> here, like you've done with the kvp and vss code. And
> remember to initialize fcopy_fd to -1. :-)
>
> Michael
Will do it in v4.
Thanks,
-- Dexuan
^ permalink raw reply
* RE: [PATCH v3 4/4] hv_utils: Add the support of hibernation
From: Michael Kelley @ 2020-01-26 4:58 UTC (permalink / raw)
To: Dexuan Cui, KY Srinivasan, Haiyang Zhang, Stephen Hemminger,
sashal@kernel.org, linux-hyperv@vger.kernel.org,
linux-kernel@vger.kernel.org, vkuznets
In-Reply-To: <1579982036-121722-5-git-send-email-decui@microsoft.com>
From: Dexuan Cui <decui@microsoft.com> Sent: Saturday, January 25, 2020 11:54 AM
>
> Add util_pre_suspend() and util_pre_resume() for some hv_utils devices
> (e.g. kvp/vss/fcopy), because they need special handling before
> util_suspend() calls vmbus_close().
>
> For kvp, all the possible pending work items should be cancelled.
>
> For vss and fcopy, some extra clean-up needs to be done, i.e. fake a
> THAW message for hv_vss_daemon and fake a CANCEL_FCOPY message for
> hv_fcopy_daemon, otherwise when the VM resums back, the daemons
> can end up in an inconsistent state (i.e. the file systems are
> frozen but will never be thawed; the file transmitted via fcopy
> may not be complete). Note: there is an extra patch for the daemons:
> "Tools: hv: Reopen the devices if read() or write() returns errors",
> because the hv_utils driver can not guarantee the whole transaction
> finishes completely once util_suspend() starts to run (at this time,
> all the userspace processes are frozen).
>
> util_probe() disables channel->callback_event to avoid the race with
> the channel callback.
>
> Signed-off-by: Dexuan Cui <decui@microsoft.com>
>
> ---
> Changes in v2:
> Handles fcopy/vss specially to avoid possible inconsistent states.
>
> Changes in v3 (I addressed Michael's comments):
> Removed unneeded blank lines.
> Simplified the error handling logic by allocating memory earlier.
> Added a comment before util_suspend(): when we're in the function,
> all the userspace processes have been frozen.
>
> drivers/hv/hv_fcopy.c | 54 +++++++++++++++++++++++++++++++++-
> drivers/hv/hv_kvp.c | 43 +++++++++++++++++++++++++--
> drivers/hv/hv_snapshot.c | 55 ++++++++++++++++++++++++++++++++--
> drivers/hv/hv_util.c | 62 ++++++++++++++++++++++++++++++++++++++-
> drivers/hv/hyperv_vmbus.h | 6 ++++
> include/linux/hyperv.h | 2 ++
> 6 files changed, 216 insertions(+), 6 deletions(-)
>
Reviewed-by: Michael Kelley <mikelley@microsoft.com>
^ permalink raw reply
* RE: [PATCH v3 2/4] hv_utils: Support host-initiated restart request
From: Michael Kelley @ 2020-01-26 4:57 UTC (permalink / raw)
To: Dexuan Cui, KY Srinivasan, Haiyang Zhang, Stephen Hemminger,
sashal@kernel.org, linux-hyperv@vger.kernel.org,
linux-kernel@vger.kernel.org, vkuznets
In-Reply-To: <1579982036-121722-3-git-send-email-decui@microsoft.com>
From: Dexuan Cui <decui@microsoft.com> 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 <decui@microsoft.com>
>
> ---
> 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
^ permalink raw reply
* RE: [PATCH v3 1/4] Tools: hv: Reopen the devices if read() or write() returns errors
From: Michael Kelley @ 2020-01-26 4:49 UTC (permalink / raw)
To: Dexuan Cui, KY Srinivasan, Haiyang Zhang, Stephen Hemminger,
sashal@kernel.org, linux-hyperv@vger.kernel.org,
linux-kernel@vger.kernel.org, vkuznets
In-Reply-To: <1579982036-121722-2-git-send-email-decui@microsoft.com>
From: Dexuan Cui <decui@microsoft.com> Sent: Saturday, January 25, 2020 11:54 AM
>
> The state machine in the hv_utils driver can run out of order in some
> corner cases, e.g. if the kvp daemon doesn't call write() fast enough
> due to some reason, kvp_timeout_func() can run first and move the state
> to HVUTIL_READY; next, when kvp_on_msg() is called it returns -EINVAL
> since kvp_transaction.state is smaller than HVUTIL_USERSPACE_REQ; later,
> the daemon's write() gets an error -EINVAL, and the daemon will exit().
>
> We can reproduce the issue by sending a SIGSTOP signal to the daemon, wait
> for 1 minute, and send a SIGCONT signal to the daemon: the daemon will
> exit() quickly.
>
> We can fix the issue by forcing a reset of the device (which means the
> daemon can close() and open() the device again) and doing extra necessary
> clean-up.
>
> Signed-off-by: Dexuan Cui <decui@microsoft.com>
>
> ---
> Changes in v2:
> This is actually a new patch that makes the daemons more robust.
>
> Changes in v3 (I addressed Michael's comments):
> Don't reset target_fd, since that's unnecessary.
> Reset target_fname by: target_fname[0] = '\0';
> Added the missing "fs_frozen = true;" in vss_operate().
> After reopen_vss_fd: if vss_operate(VSS_OP_THAW) can not clear
> fs_frozen due to an error, we just exit().
> Added comments.
>
> tools/hv/hv_fcopy_daemon.c | 33 +++++++++++++++++++++----
> tools/hv/hv_kvp_daemon.c | 36 ++++++++++++++++------------
> tools/hv/hv_vss_daemon.c | 49 +++++++++++++++++++++++++++++---------
> 3 files changed, 88 insertions(+), 30 deletions(-)
>
> diff --git a/tools/hv/hv_fcopy_daemon.c b/tools/hv/hv_fcopy_daemon.c
> index aea2d91ab364..48cfa032432c 100644
> --- a/tools/hv/hv_fcopy_daemon.c
> +++ b/tools/hv/hv_fcopy_daemon.c
> @@ -80,6 +80,8 @@ static int hv_start_fcopy(struct hv_start_fcopy *smsg)
>
> error = 0;
> done:
> + if (error)
> + target_fname[0] = '\0';
> return error;
> }
>
> @@ -108,15 +110,29 @@ static int hv_copy_data(struct hv_do_fcopy *cpmsg)
> return ret;
> }
>
> +/*
> + * Reset target_fname to "" in the two below functions for hibernation: if
> + * the fcopy operation is aborted by hibernation, the daemon should remove the
> + * partially-copied file; to achieve this, the hv_utils driver always fakes a
> + * CANCEL_FCOPY message upon suspend, and later when the VM resumes back,
> + * the daemon calls hv_copy_cancel() to remove the file; if a file is copied
> + * successfully before suspend, hv_copy_finished() must reset target_fname to
> + * avoid that the file can be incorrectly removed upon resume, since the faked
> + * CANCEL_FCOPY message is spurious in this case.
> + */
> static int hv_copy_finished(void)
> {
> close(target_fd);
> + target_fname[0] = '\0';
> return 0;
> }
> static int hv_copy_cancel(void)
> {
> close(target_fd);
> - unlink(target_fname);
> + if (strlen(target_fname) > 0) {
> + unlink(target_fname);
> + target_fname[0] = '\0';
> + }
> return 0;
>
> }
> @@ -141,7 +157,7 @@ int main(int argc, char *argv[])
> struct hv_do_fcopy copy;
> __u32 kernel_modver;
> } buffer = { };
> - int in_handshake = 1;
> + int in_handshake;
>
> static struct option long_options[] = {
> {"help", no_argument, 0, 'h' },
> @@ -170,6 +186,10 @@ int main(int argc, char *argv[])
> openlog("HV_FCOPY", 0, LOG_USER);
> syslog(LOG_INFO, "starting; pid is:%d", getpid());
>
> +reopen_fcopy_fd:
> + /* Remove any possible partially-copied file on error */
> + hv_copy_cancel();
Since you have removed the calls to close(fcopy_fd) after a
pread() or pwrite() failure that were in v2 of the patch, I was
expecting to see
if (fcopy_fd != -1)
close(fcopy_fd)
here, like you've done with the kvp and vss code. And
remember to initialize fcopy_fd to -1. :-)
Michael
> + in_handshake = 1;
> fcopy_fd = open("/dev/vmbus/hv_fcopy", O_RDWR);
>
> if (fcopy_fd < 0) {
> @@ -196,7 +216,7 @@ int main(int argc, char *argv[])
> len = pread(fcopy_fd, &buffer, sizeof(buffer), 0);
> if (len < 0) {
> syslog(LOG_ERR, "pread failed: %s", strerror(errno));
> - exit(EXIT_FAILURE);
> + goto reopen_fcopy_fd;
> }
>
> if (in_handshake) {
> @@ -231,9 +251,14 @@ int main(int argc, char *argv[])
>
> }
>
> + /*
> + * pwrite() may return an error due to the faked CANCEL_FCOPY
> + * message upon hibernation. Ignore the error by resetting the
> + * dev file, i.e. closing and re-opening it.
> + */
> if (pwrite(fcopy_fd, &error, sizeof(int), 0) != sizeof(int)) {
> syslog(LOG_ERR, "pwrite failed: %s", strerror(errno));
> - exit(EXIT_FAILURE);
> + goto reopen_fcopy_fd;
> }
> }
> }
^ permalink raw reply
* Re: [PATCH][RESEND] Input: hyperv-keyboard: Add the support of hibernation
From: Sasha Levin @ 2020-01-25 21:45 UTC (permalink / raw)
To: Dexuan Cui
Cc: dmitry.torokhov@gmail.com, KY Srinivasan, Haiyang Zhang,
Stephen Hemminger, linux-hyperv@vger.kernel.org,
linux-input@vger.kernel.org, linux-kernel@vger.kernel.org,
Michael Kelley, Sasha Levin
In-Reply-To: <HK0P153MB01489C2ABCFF40822EC8B695BF090@HK0P153MB0148.APCP153.PROD.OUTLOOK.COM>
On Sat, Jan 25, 2020 at 01:59:03AM +0000, Dexuan Cui wrote:
>> From: linux-hyperv-owner@vger.kernel.org
>> <linux-hyperv-owner@vger.kernel.org> On Behalf Of Dexuan Cui
>> Sent: Wednesday, January 15, 2020 11:53 AM
>> > ...
>> > On Mon, Jan 06, 2020 at 02:40:44PM -0800, Dexuan Cui wrote:
>> > >Add suspend() and resume() functions so the Hyper-V virtual keyboard
>> > >can participate in VM hibernation.
>> > >
>> > >Note that the keyboard is a "wakeup" device that could abort an in-progress
>> > >hibernation if there is keyboard event. No attempt is made to suppress
>> > this
>> > >behavior. If desired, a sysadmin can disable the keyboard as a wakeup
>> > device
>> > >using standard mechanisms such as:
>> > >
>> > >echo disabled >
>> > /sys/bus/vmbus/drivers/hyperv_keyboard/XXX/power/wakeup
>> > >(where XXX is the device's GUID)
>> > >
>> > >Reviewed-by: Michael Kelley <mikelley@microsoft.com>
>> > >Signed-off-by: Dexuan Cui <decui@microsoft.com>
>> > >---
>> > >
>> > >This is a RESEND of https://lkml.org/lkml/2019/11/24/115 .
>> >
>> > >Please review.
>> > >
>> > >If it looks good, Sasha Levin, can you please pick it up via the
>> > >hyperv/linux.git tree, as you did last time for this driver?
>> >
>> > This will need an ack from the input driver maintainers, unless they
>> > want to give a blanket ack to this type of patches.
>> >
>> > Thanks,
>> > Sasha
>>
>> Hi Dmitry,
>> May I have your Acked-by for this patch?
>>
>> Thanks
>> -- Dexuan
>
>It looks Dmitry may be on leave and there is no ACK from any input driver
>maintainers. :-(
>
>Sasha, for this patch, I would suggest it may as well go directly through the
>Hyper-V tree because
>1. It's really small and it's a pure Hyper-V change.
>2. This is for a new functionality and it can not break any existing code.
>3. I'm pretty sure it can not cause any merge conflict or build failure.
>
>It's still wonderful to get an Ack or some comment from the input driver
>maintainer(s), so maybe let's wait more time (e.g. a week?) and then I
>suggust we proceed.
I'll queue it up via the hyperv tree, if anyone has any objections
please let me know.
--
Thanks,
Sasha
^ permalink raw reply
* Re: [PATCH v2] Drivers: hv: vmbus: Ignore CHANNELMSG_TL_CONNECT_RESULT(23)
From: Sasha Levin @ 2020-01-25 21:42 UTC (permalink / raw)
To: Michael Kelley
Cc: Dexuan Cui, KY Srinivasan, Haiyang Zhang, Stephen Hemminger,
linux-hyperv@vger.kernel.org, linux-kernel@vger.kernel.org,
Sasha Levin, Sunil Muthuswamy, Andrea Parri, Wei Hu
In-Reply-To: <MW2PR2101MB1052AEC27FF7287F5BA81C91D7320@MW2PR2101MB1052.namprd21.prod.outlook.com>
On Mon, Jan 20, 2020 at 12:51:51AM +0000, Michael Kelley wrote:
>From: Dexuan Cui <decui@microsoft.com> Sent: Sunday, January 19, 2020 3:29 PM
>>
>> When a Linux hv_sock app tries to connect to a Service GUID on which no
>> host app is listening, a recent host (RS3+) sends a
>> CHANNELMSG_TL_CONNECT_RESULT (23) message to Linux and this triggers such
>> a warning:
>>
>> unknown msgtype=23
>> WARNING: CPU: 2 PID: 0 at drivers/hv/vmbus_drv.c:1031 vmbus_on_msg_dpc
>>
>> Actually Linux can safely ignore the message because the Linux app's
>> connect() will time out in 2 seconds: see VSOCK_DEFAULT_CONNECT_TIMEOUT
>> and vsock_stream_connect(). We don't bother to make use of the message
>> because: 1) it's only supported on recent hosts; 2) a non-trivial effort
>> is required to use the message in Linux, but the benefit is small.
>>
>> So, let's not see the warning by silently ignoring the message.
>>
>> Signed-off-by: Dexuan Cui <decui@microsoft.com>
>> ---
>>
>> In v2 (followed Michael Kelley's suggestions):
>> Removed the redundant code in vmbus_onmessage()
>> Added the new enries into channel_message_table[].
>>
>> drivers/hv/channel_mgmt.c | 21 +++++++--------------
>> drivers/hv/vmbus_drv.c | 4 ++++
>> include/linux/hyperv.h | 2 ++
>> 3 files changed, 13 insertions(+), 14 deletions(-)
>>
>
>Reviewed-by: Michael Kelley <mikelley@microsoft.com>
Queued up, thanks!
--
Thanks,
Sasha
^ permalink raw reply
* Re: [PATCH V4] x86/Hyper-V: Balloon up according to request page number
From: Sasha Levin @ 2020-01-25 21:41 UTC (permalink / raw)
To: Michael Kelley
Cc: lantianyu1986@gmail.com, KY Srinivasan, Haiyang Zhang,
Stephen Hemminger, Tianyu Lan, linux-hyperv@vger.kernel.org,
linux-kernel@vger.kernel.org, vkuznets, stable@vger.kernel.org
In-Reply-To: <MW2PR2101MB1052B2248B0F9B92F98A2201D70D0@MW2PR2101MB1052.namprd21.prod.outlook.com>
On Tue, Jan 21, 2020 at 04:36:26AM +0000, Michael Kelley wrote:
>From: Tianyu Lan <Tianyu.Lan@microsoft.com> Sent: Monday, January 20, 2020 7:49 PM
>>
>> Current code has assumption that balloon request memory size aligns
>> with 2MB. But actually Hyper-V doesn't guarantee such alignment. When
>> balloon driver receives non-aligned balloon request, it produces warning
>> and balloon up more memory than requested in order to keep 2MB alignment.
>> Remove the warning and balloon up memory according to actual requested
>> memory size.
>>
>> Fixes: f6712238471a ("hv: hv_balloon: avoid memory leak on alloc_error of 2MB memory
>> block")
>> Cc: stable@vger.kernel.org
>> Reviewed-by: Vitaly Kuznetsov <vkuznets@redhat.com>
>> Signed-off-by: Tianyu Lan <Tianyu.Lan@microsoft.com>
>
>Reviewed-by: Michael Kelley <mikelley@microsoft.com>
Queued up, thanks!
--
Thanks,
Sasha
^ permalink raw reply
* [PATCH v3 1/4] Tools: hv: Reopen the devices if read() or write() returns errors
From: Dexuan Cui @ 2020-01-25 19:53 UTC (permalink / raw)
To: kys, haiyangz, sthemmin, sashal, linux-hyperv, linux-kernel,
mikelley, vkuznets
Cc: Dexuan Cui
In-Reply-To: <1579982036-121722-1-git-send-email-decui@microsoft.com>
The state machine in the hv_utils driver can run out of order in some
corner cases, e.g. if the kvp daemon doesn't call write() fast enough
due to some reason, kvp_timeout_func() can run first and move the state
to HVUTIL_READY; next, when kvp_on_msg() is called it returns -EINVAL
since kvp_transaction.state is smaller than HVUTIL_USERSPACE_REQ; later,
the daemon's write() gets an error -EINVAL, and the daemon will exit().
We can reproduce the issue by sending a SIGSTOP signal to the daemon, wait
for 1 minute, and send a SIGCONT signal to the daemon: the daemon will
exit() quickly.
We can fix the issue by forcing a reset of the device (which means the
daemon can close() and open() the device again) and doing extra necessary
clean-up.
Signed-off-by: Dexuan Cui <decui@microsoft.com>
---
Changes in v2:
This is actually a new patch that makes the daemons more robust.
Changes in v3 (I addressed Michael's comments):
Don't reset target_fd, since that's unnecessary.
Reset target_fname by: target_fname[0] = '\0';
Added the missing "fs_frozen = true;" in vss_operate().
After reopen_vss_fd: if vss_operate(VSS_OP_THAW) can not clear
fs_frozen due to an error, we just exit().
Added comments.
tools/hv/hv_fcopy_daemon.c | 33 +++++++++++++++++++++----
tools/hv/hv_kvp_daemon.c | 36 ++++++++++++++++------------
tools/hv/hv_vss_daemon.c | 49 +++++++++++++++++++++++++++++---------
3 files changed, 88 insertions(+), 30 deletions(-)
diff --git a/tools/hv/hv_fcopy_daemon.c b/tools/hv/hv_fcopy_daemon.c
index aea2d91ab364..48cfa032432c 100644
--- a/tools/hv/hv_fcopy_daemon.c
+++ b/tools/hv/hv_fcopy_daemon.c
@@ -80,6 +80,8 @@ static int hv_start_fcopy(struct hv_start_fcopy *smsg)
error = 0;
done:
+ if (error)
+ target_fname[0] = '\0';
return error;
}
@@ -108,15 +110,29 @@ static int hv_copy_data(struct hv_do_fcopy *cpmsg)
return ret;
}
+/*
+ * Reset target_fname to "" in the two below functions for hibernation: if
+ * the fcopy operation is aborted by hibernation, the daemon should remove the
+ * partially-copied file; to achieve this, the hv_utils driver always fakes a
+ * CANCEL_FCOPY message upon suspend, and later when the VM resumes back,
+ * the daemon calls hv_copy_cancel() to remove the file; if a file is copied
+ * successfully before suspend, hv_copy_finished() must reset target_fname to
+ * avoid that the file can be incorrectly removed upon resume, since the faked
+ * CANCEL_FCOPY message is spurious in this case.
+ */
static int hv_copy_finished(void)
{
close(target_fd);
+ target_fname[0] = '\0';
return 0;
}
static int hv_copy_cancel(void)
{
close(target_fd);
- unlink(target_fname);
+ if (strlen(target_fname) > 0) {
+ unlink(target_fname);
+ target_fname[0] = '\0';
+ }
return 0;
}
@@ -141,7 +157,7 @@ int main(int argc, char *argv[])
struct hv_do_fcopy copy;
__u32 kernel_modver;
} buffer = { };
- int in_handshake = 1;
+ int in_handshake;
static struct option long_options[] = {
{"help", no_argument, 0, 'h' },
@@ -170,6 +186,10 @@ int main(int argc, char *argv[])
openlog("HV_FCOPY", 0, LOG_USER);
syslog(LOG_INFO, "starting; pid is:%d", getpid());
+reopen_fcopy_fd:
+ /* Remove any possible partially-copied file on error */
+ hv_copy_cancel();
+ in_handshake = 1;
fcopy_fd = open("/dev/vmbus/hv_fcopy", O_RDWR);
if (fcopy_fd < 0) {
@@ -196,7 +216,7 @@ int main(int argc, char *argv[])
len = pread(fcopy_fd, &buffer, sizeof(buffer), 0);
if (len < 0) {
syslog(LOG_ERR, "pread failed: %s", strerror(errno));
- exit(EXIT_FAILURE);
+ goto reopen_fcopy_fd;
}
if (in_handshake) {
@@ -231,9 +251,14 @@ int main(int argc, char *argv[])
}
+ /*
+ * pwrite() may return an error due to the faked CANCEL_FCOPY
+ * message upon hibernation. Ignore the error by resetting the
+ * dev file, i.e. closing and re-opening it.
+ */
if (pwrite(fcopy_fd, &error, sizeof(int), 0) != sizeof(int)) {
syslog(LOG_ERR, "pwrite failed: %s", strerror(errno));
- exit(EXIT_FAILURE);
+ goto reopen_fcopy_fd;
}
}
}
diff --git a/tools/hv/hv_kvp_daemon.c b/tools/hv/hv_kvp_daemon.c
index e9ef4ca6a655..ee9c1bb2293e 100644
--- a/tools/hv/hv_kvp_daemon.c
+++ b/tools/hv/hv_kvp_daemon.c
@@ -76,7 +76,7 @@ enum {
DNS
};
-static int in_hand_shake = 1;
+static int in_hand_shake;
static char *os_name = "";
static char *os_major = "";
@@ -1360,7 +1360,7 @@ void print_usage(char *argv[])
int main(int argc, char *argv[])
{
- int kvp_fd, len;
+ int kvp_fd = -1, len;
int error;
struct pollfd pfd;
char *p;
@@ -1400,14 +1400,6 @@ int main(int argc, char *argv[])
openlog("KVP", 0, LOG_USER);
syslog(LOG_INFO, "KVP starting; pid is:%d", getpid());
- kvp_fd = open("/dev/vmbus/hv_kvp", O_RDWR | O_CLOEXEC);
-
- if (kvp_fd < 0) {
- syslog(LOG_ERR, "open /dev/vmbus/hv_kvp failed; error: %d %s",
- errno, strerror(errno));
- exit(EXIT_FAILURE);
- }
-
/*
* Retrieve OS release information.
*/
@@ -1423,6 +1415,18 @@ int main(int argc, char *argv[])
exit(EXIT_FAILURE);
}
+reopen_kvp_fd:
+ if (kvp_fd != -1)
+ close(kvp_fd);
+ in_hand_shake = 1;
+ kvp_fd = open("/dev/vmbus/hv_kvp", O_RDWR | O_CLOEXEC);
+
+ if (kvp_fd < 0) {
+ syslog(LOG_ERR, "open /dev/vmbus/hv_kvp failed; error: %d %s",
+ errno, strerror(errno));
+ exit(EXIT_FAILURE);
+ }
+
/*
* Register ourselves with the kernel.
*/
@@ -1456,9 +1460,7 @@ int main(int argc, char *argv[])
if (len != sizeof(struct hv_kvp_msg)) {
syslog(LOG_ERR, "read failed; error:%d %s",
errno, strerror(errno));
-
- close(kvp_fd);
- return EXIT_FAILURE;
+ goto reopen_kvp_fd;
}
/*
@@ -1617,13 +1619,17 @@ int main(int argc, char *argv[])
break;
}
- /* Send the value back to the kernel. */
+ /*
+ * Send the value back to the kernel. Note: the write() may
+ * return an error due to hibernation; we can ignore the error
+ * by resetting the dev file, i.e. closing and re-opening it.
+ */
kvp_done:
len = write(kvp_fd, hv_msg, sizeof(struct hv_kvp_msg));
if (len != sizeof(struct hv_kvp_msg)) {
syslog(LOG_ERR, "write failed; error: %d %s", errno,
strerror(errno));
- exit(EXIT_FAILURE);
+ goto reopen_kvp_fd;
}
}
diff --git a/tools/hv/hv_vss_daemon.c b/tools/hv/hv_vss_daemon.c
index 92902a88f671..dd111870beee 100644
--- a/tools/hv/hv_vss_daemon.c
+++ b/tools/hv/hv_vss_daemon.c
@@ -28,6 +28,8 @@
#include <stdbool.h>
#include <dirent.h>
+static bool fs_frozen;
+
/* Don't use syslog() in the function since that can cause write to disk */
static int vss_do_freeze(char *dir, unsigned int cmd)
{
@@ -155,18 +157,27 @@ static int vss_operate(int operation)
continue;
}
error |= vss_do_freeze(ent->mnt_dir, cmd);
- if (error && operation == VSS_OP_FREEZE)
- goto err;
+ if (operation == VSS_OP_FREEZE) {
+ if (error)
+ goto err;
+ fs_frozen = true;
+ }
}
endmntent(mounts);
if (root_seen) {
error |= vss_do_freeze("/", cmd);
- if (error && operation == VSS_OP_FREEZE)
- goto err;
+ if (operation == VSS_OP_FREEZE) {
+ if (error)
+ goto err;
+ fs_frozen = true;
+ }
}
+ if (operation == VSS_OP_THAW && !error)
+ fs_frozen = false;
+
goto out;
err:
save_errno = errno;
@@ -175,6 +186,7 @@ static int vss_operate(int operation)
endmntent(mounts);
}
vss_operate(VSS_OP_THAW);
+ fs_frozen = false;
/* Call syslog after we thaw all filesystems */
if (ent)
syslog(LOG_ERR, "FREEZE of %s failed; error:%d %s",
@@ -196,13 +208,13 @@ void print_usage(char *argv[])
int main(int argc, char *argv[])
{
- int vss_fd, len;
+ int vss_fd = -1, len;
int error;
struct pollfd pfd;
int op;
struct hv_vss_msg vss_msg[1];
int daemonize = 1, long_index = 0, opt;
- int in_handshake = 1;
+ int in_handshake;
__u32 kernel_modver;
static struct option long_options[] = {
@@ -232,6 +244,18 @@ int main(int argc, char *argv[])
openlog("Hyper-V VSS", 0, LOG_USER);
syslog(LOG_INFO, "VSS starting; pid is:%d", getpid());
+reopen_vss_fd:
+ if (vss_fd != -1)
+ close(vss_fd);
+ if (fs_frozen) {
+ if (vss_operate(VSS_OP_THAW) || fs_frozen) {
+ syslog(LOG_ERR, "failed to thaw file system: err=%d",
+ errno);
+ exit(EXIT_FAILURE);
+ }
+ }
+
+ in_handshake = 1;
vss_fd = open("/dev/vmbus/hv_vss", O_RDWR);
if (vss_fd < 0) {
syslog(LOG_ERR, "open /dev/vmbus/hv_vss failed; error: %d %s",
@@ -284,8 +308,7 @@ int main(int argc, char *argv[])
if (len != sizeof(struct hv_vss_msg)) {
syslog(LOG_ERR, "read failed; error:%d %s",
errno, strerror(errno));
- close(vss_fd);
- return EXIT_FAILURE;
+ goto reopen_vss_fd;
}
op = vss_msg->vss_hdr.operation;
@@ -312,14 +335,18 @@ int main(int argc, char *argv[])
default:
syslog(LOG_ERR, "Illegal op:%d\n", op);
}
+
+ /*
+ * The write() may return an error due to the faked VSS_OP_THAW
+ * message upon hibernation. Ignore the error by resetting the
+ * dev file, i.e. closing and re-opening it.
+ */
vss_msg->error = error;
len = write(vss_fd, vss_msg, sizeof(struct hv_vss_msg));
if (len != sizeof(struct hv_vss_msg)) {
syslog(LOG_ERR, "write failed; error: %d %s", errno,
strerror(errno));
-
- if (op == VSS_OP_FREEZE)
- vss_operate(VSS_OP_THAW);
+ goto reopen_vss_fd;
}
}
--
2.19.1
^ permalink raw reply related
* [PATCH v3 2/4] hv_utils: Support host-initiated restart request
From: Dexuan Cui @ 2020-01-25 19:53 UTC (permalink / raw)
To: kys, haiyangz, sthemmin, sashal, linux-hyperv, linux-kernel,
mikelley, vkuznets
Cc: Dexuan Cui
In-Reply-To: <1579982036-121722-1-git-send-email-decui@microsoft.com>
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 <decui@microsoft.com>
---
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
^ permalink raw reply related
* [PATCH v3 4/4] hv_utils: Add the support of hibernation
From: Dexuan Cui @ 2020-01-25 19:53 UTC (permalink / raw)
To: kys, haiyangz, sthemmin, sashal, linux-hyperv, linux-kernel,
mikelley, vkuznets
Cc: Dexuan Cui
In-Reply-To: <1579982036-121722-1-git-send-email-decui@microsoft.com>
Add util_pre_suspend() and util_pre_resume() for some hv_utils devices
(e.g. kvp/vss/fcopy), because they need special handling before
util_suspend() calls vmbus_close().
For kvp, all the possible pending work items should be cancelled.
For vss and fcopy, some extra clean-up needs to be done, i.e. fake a
THAW message for hv_vss_daemon and fake a CANCEL_FCOPY message for
hv_fcopy_daemon, otherwise when the VM resums back, the daemons
can end up in an inconsistent state (i.e. the file systems are
frozen but will never be thawed; the file transmitted via fcopy
may not be complete). Note: there is an extra patch for the daemons:
"Tools: hv: Reopen the devices if read() or write() returns errors",
because the hv_utils driver can not guarantee the whole transaction
finishes completely once util_suspend() starts to run (at this time,
all the userspace processes are frozen).
util_probe() disables channel->callback_event to avoid the race with
the channel callback.
Signed-off-by: Dexuan Cui <decui@microsoft.com>
---
Changes in v2:
Handles fcopy/vss specially to avoid possible inconsistent states.
Changes in v3 (I addressed Michael's comments):
Removed unneeded blank lines.
Simplified the error handling logic by allocating memory earlier.
Added a comment before util_suspend(): when we're in the function,
all the userspace processes have been frozen.
drivers/hv/hv_fcopy.c | 54 +++++++++++++++++++++++++++++++++-
drivers/hv/hv_kvp.c | 43 +++++++++++++++++++++++++--
drivers/hv/hv_snapshot.c | 55 ++++++++++++++++++++++++++++++++--
drivers/hv/hv_util.c | 62 ++++++++++++++++++++++++++++++++++++++-
drivers/hv/hyperv_vmbus.h | 6 ++++
include/linux/hyperv.h | 2 ++
6 files changed, 216 insertions(+), 6 deletions(-)
diff --git a/drivers/hv/hv_fcopy.c b/drivers/hv/hv_fcopy.c
index 08fa4a5de644..bb9ba3f7c794 100644
--- a/drivers/hv/hv_fcopy.c
+++ b/drivers/hv/hv_fcopy.c
@@ -346,9 +346,61 @@ int hv_fcopy_init(struct hv_util_service *srv)
return 0;
}
+static void hv_fcopy_cancel_work(void)
+{
+ cancel_delayed_work_sync(&fcopy_timeout_work);
+ cancel_work_sync(&fcopy_send_work);
+}
+
+int hv_fcopy_pre_suspend(void)
+{
+ struct vmbus_channel *channel = fcopy_transaction.recv_channel;
+ struct hv_fcopy_hdr *fcopy_msg;
+
+ /*
+ * Fake a CANCEL_FCOPY message for the user space daemon in case the
+ * daemon is in the middle of copying some file. It doesn't matter if
+ * there is already a message pending to be delivered to the user
+ * space since we force fcopy_transaction.state to be HVUTIL_READY, so
+ * the user space daemon's write() will fail with EINVAL (see
+ * fcopy_on_msg()), and the daemon will reset the device by closing
+ * and re-opening it.
+ */
+ fcopy_msg = kzalloc(sizeof(*fcopy_msg), GFP_KERNEL);
+ if (!fcopy_msg)
+ return -ENOMEM;
+
+ tasklet_disable(&channel->callback_event);
+
+ fcopy_msg->operation = CANCEL_FCOPY;
+
+ hv_fcopy_cancel_work();
+
+ /* We don't care about the return value. */
+ hvutil_transport_send(hvt, fcopy_msg, sizeof(*fcopy_msg), NULL);
+
+ kfree(fcopy_msg);
+
+ fcopy_transaction.state = HVUTIL_READY;
+
+ /* tasklet_enable() will be called in hv_fcopy_pre_resume(). */
+ return 0;
+}
+
+int hv_fcopy_pre_resume(void)
+{
+ struct vmbus_channel *channel = fcopy_transaction.recv_channel;
+
+ tasklet_enable(&channel->callback_event);
+
+ return 0;
+}
+
void hv_fcopy_deinit(void)
{
fcopy_transaction.state = HVUTIL_DEVICE_DYING;
- cancel_delayed_work_sync(&fcopy_timeout_work);
+
+ hv_fcopy_cancel_work();
+
hvutil_transport_destroy(hvt);
}
diff --git a/drivers/hv/hv_kvp.c b/drivers/hv/hv_kvp.c
index ae7c028dc5a8..e74b144b8f3d 100644
--- a/drivers/hv/hv_kvp.c
+++ b/drivers/hv/hv_kvp.c
@@ -758,11 +758,50 @@ hv_kvp_init(struct hv_util_service *srv)
return 0;
}
-void hv_kvp_deinit(void)
+static void hv_kvp_cancel_work(void)
{
- kvp_transaction.state = HVUTIL_DEVICE_DYING;
cancel_delayed_work_sync(&kvp_host_handshake_work);
cancel_delayed_work_sync(&kvp_timeout_work);
cancel_work_sync(&kvp_sendkey_work);
+}
+
+int hv_kvp_pre_suspend(void)
+{
+ struct vmbus_channel *channel = kvp_transaction.recv_channel;
+
+ tasklet_disable(&channel->callback_event);
+
+ /*
+ * If there is a pending transtion, it's unnecessary to tell the host
+ * that the transaction will fail, because that is implied when
+ * util_suspend() calls vmbus_close() later.
+ */
+ hv_kvp_cancel_work();
+
+ /*
+ * Forece the state to READY to handle the ICMSGTYPE_NEGOTIATE message
+ * later. The user space daemon may go out of order and its write()
+ * may fail with EINVAL: this doesn't matter since the daemon will
+ * reset the device by closing and re-opening it.
+ */
+ kvp_transaction.state = HVUTIL_READY;
+ return 0;
+}
+
+int hv_kvp_pre_resume(void)
+{
+ struct vmbus_channel *channel = kvp_transaction.recv_channel;
+
+ tasklet_enable(&channel->callback_event);
+
+ return 0;
+}
+
+void hv_kvp_deinit(void)
+{
+ kvp_transaction.state = HVUTIL_DEVICE_DYING;
+
+ hv_kvp_cancel_work();
+
hvutil_transport_destroy(hvt);
}
diff --git a/drivers/hv/hv_snapshot.c b/drivers/hv/hv_snapshot.c
index 03b6454268b3..1c75b38f0d6d 100644
--- a/drivers/hv/hv_snapshot.c
+++ b/drivers/hv/hv_snapshot.c
@@ -379,10 +379,61 @@ hv_vss_init(struct hv_util_service *srv)
return 0;
}
-void hv_vss_deinit(void)
+static void hv_vss_cancel_work(void)
{
- vss_transaction.state = HVUTIL_DEVICE_DYING;
cancel_delayed_work_sync(&vss_timeout_work);
cancel_work_sync(&vss_handle_request_work);
+}
+
+int hv_vss_pre_suspend(void)
+{
+ struct vmbus_channel *channel = vss_transaction.recv_channel;
+ struct hv_vss_msg *vss_msg;
+
+ /*
+ * Fake a THAW message for the user space daemon in case the daemon
+ * has frozen the file systems. It doesn't matter if there is already
+ * a message pending to be delivered to the user space since we force
+ * vss_transaction.state to be HVUTIL_READY, so the user space daemon's
+ * write() will fail with EINVAL (see vss_on_msg()), and the daemon
+ * will reset the device by closing and re-opening it.
+ */
+ vss_msg = kzalloc(sizeof(*vss_msg), GFP_KERNEL);
+ if (!vss_msg)
+ return -ENOMEM;
+
+ tasklet_disable(&channel->callback_event);
+
+ vss_msg->vss_hdr.operation = VSS_OP_THAW;
+
+ /* Cancel any possible pending work. */
+ hv_vss_cancel_work();
+
+ /* We don't care about the return value. */
+ hvutil_transport_send(hvt, vss_msg, sizeof(*vss_msg), NULL);
+
+ kfree(vss_msg);
+
+ vss_transaction.state = HVUTIL_READY;
+
+ /* tasklet_enable() will be called in hv_vss_pre_resume(). */
+ return 0;
+}
+
+int hv_vss_pre_resume(void)
+{
+ struct vmbus_channel *channel = vss_transaction.recv_channel;
+
+ tasklet_enable(&channel->callback_event);
+
+ return 0;
+}
+
+void hv_vss_deinit(void)
+{
+ vss_transaction.state = HVUTIL_DEVICE_DYING;
+
+ hv_vss_cancel_work();
+
hvutil_transport_destroy(hvt);
}
diff --git a/drivers/hv/hv_util.c b/drivers/hv/hv_util.c
index e07197dfb4a2..0fe5ff845d07 100644
--- a/drivers/hv/hv_util.c
+++ b/drivers/hv/hv_util.c
@@ -123,12 +123,14 @@ static struct hv_util_service util_shutdown = {
};
static int hv_timesync_init(struct hv_util_service *srv);
+static int hv_timesync_pre_suspend(void);
static void hv_timesync_deinit(void);
static void timesync_onchannelcallback(void *context);
static struct hv_util_service util_timesynch = {
.util_cb = timesync_onchannelcallback,
.util_init = hv_timesync_init,
+ .util_pre_suspend = hv_timesync_pre_suspend,
.util_deinit = hv_timesync_deinit,
};
@@ -140,18 +142,24 @@ static struct hv_util_service util_heartbeat = {
static struct hv_util_service util_kvp = {
.util_cb = hv_kvp_onchannelcallback,
.util_init = hv_kvp_init,
+ .util_pre_suspend = hv_kvp_pre_suspend,
+ .util_pre_resume = hv_kvp_pre_resume,
.util_deinit = hv_kvp_deinit,
};
static struct hv_util_service util_vss = {
.util_cb = hv_vss_onchannelcallback,
.util_init = hv_vss_init,
+ .util_pre_suspend = hv_vss_pre_suspend,
+ .util_pre_resume = hv_vss_pre_resume,
.util_deinit = hv_vss_deinit,
};
static struct hv_util_service util_fcopy = {
.util_cb = hv_fcopy_onchannelcallback,
.util_init = hv_fcopy_init,
+ .util_pre_suspend = hv_fcopy_pre_suspend,
+ .util_pre_resume = hv_fcopy_pre_resume,
.util_deinit = hv_fcopy_deinit,
};
@@ -521,6 +529,44 @@ static int util_remove(struct hv_device *dev)
return 0;
}
+/*
+ * When we're in util_suspend(), all the userspace processes have been frozen
+ * (refer to hibernate() -> freeze_processes()). The userspace is thawed only
+ * after the whole resume procedure, including util_resume(), finishes.
+ */
+static int util_suspend(struct hv_device *dev)
+{
+ struct hv_util_service *srv = hv_get_drvdata(dev);
+ int ret = 0;
+
+ if (srv->util_pre_suspend) {
+ ret = srv->util_pre_suspend();
+ if (ret)
+ return ret;
+ }
+
+ vmbus_close(dev->channel);
+
+ return 0;
+}
+
+static int util_resume(struct hv_device *dev)
+{
+ struct hv_util_service *srv = hv_get_drvdata(dev);
+ int ret = 0;
+
+ if (srv->util_pre_resume) {
+ ret = srv->util_pre_resume();
+ if (ret)
+ return ret;
+ }
+
+ ret = vmbus_open(dev->channel, 4 * HV_HYP_PAGE_SIZE,
+ 4 * HV_HYP_PAGE_SIZE, NULL, 0, srv->util_cb,
+ dev->channel);
+ return ret;
+}
+
static const struct hv_vmbus_device_id id_table[] = {
/* Shutdown guid */
{ HV_SHUTDOWN_GUID,
@@ -557,6 +603,8 @@ static struct hv_driver util_drv = {
.id_table = id_table,
.probe = util_probe,
.remove = util_remove,
+ .suspend = util_suspend,
+ .resume = util_resume,
.driver = {
.probe_type = PROBE_PREFER_ASYNCHRONOUS,
},
@@ -626,11 +674,23 @@ static int hv_timesync_init(struct hv_util_service *srv)
return 0;
}
+static void hv_timesync_cancel_work(void)
+{
+ cancel_work_sync(&adj_time_work);
+}
+
+static int hv_timesync_pre_suspend(void)
+{
+ hv_timesync_cancel_work();
+ return 0;
+}
+
static void hv_timesync_deinit(void)
{
if (hv_ptp_clock)
ptp_clock_unregister(hv_ptp_clock);
- cancel_work_sync(&adj_time_work);
+
+ hv_timesync_cancel_work();
}
static int __init init_hyperv_utils(void)
diff --git a/drivers/hv/hyperv_vmbus.h b/drivers/hv/hyperv_vmbus.h
index 20edcfd3b96c..f5fa3b3c9baf 100644
--- a/drivers/hv/hyperv_vmbus.h
+++ b/drivers/hv/hyperv_vmbus.h
@@ -352,14 +352,20 @@ void vmbus_on_msg_dpc(unsigned long data);
int hv_kvp_init(struct hv_util_service *srv);
void hv_kvp_deinit(void);
+int hv_kvp_pre_suspend(void);
+int hv_kvp_pre_resume(void);
void hv_kvp_onchannelcallback(void *context);
int hv_vss_init(struct hv_util_service *srv);
void hv_vss_deinit(void);
+int hv_vss_pre_suspend(void);
+int hv_vss_pre_resume(void);
void hv_vss_onchannelcallback(void *context);
int hv_fcopy_init(struct hv_util_service *srv);
void hv_fcopy_deinit(void);
+int hv_fcopy_pre_suspend(void);
+int hv_fcopy_pre_resume(void);
void hv_fcopy_onchannelcallback(void *context);
void vmbus_initiate_unload(bool crash);
diff --git a/include/linux/hyperv.h b/include/linux/hyperv.h
index 41c58011431e..692c89ccf5df 100644
--- a/include/linux/hyperv.h
+++ b/include/linux/hyperv.h
@@ -1435,6 +1435,8 @@ struct hv_util_service {
void (*util_cb)(void *);
int (*util_init)(struct hv_util_service *);
void (*util_deinit)(void);
+ int (*util_pre_suspend)(void);
+ int (*util_pre_resume)(void);
};
struct vmbuspipe_hdr {
--
2.19.1
^ permalink raw reply related
* [PATCH v3 3/4] hv_utils: Support host-initiated hibernation request
From: Dexuan Cui @ 2020-01-25 19:53 UTC (permalink / raw)
To: kys, haiyangz, sthemmin, sashal, linux-hyperv, linux-kernel,
mikelley, vkuznets
Cc: Dexuan Cui
In-Reply-To: <1579982036-121722-1-git-send-email-decui@microsoft.com>
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>
---
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
^ permalink raw reply related
* [PATCH v3 0/4] hv_utils: Add the support of hibernation
From: Dexuan Cui @ 2020-01-25 19:53 UTC (permalink / raw)
To: kys, haiyangz, sthemmin, sashal, linux-hyperv, linux-kernel,
mikelley, vkuznets
Cc: Dexuan Cui
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
^ permalink raw reply
* Re: [PATCH V4,net-next, 0/2] hv_netvsc: Add XDP support
From: David Miller @ 2020-01-25 9:43 UTC (permalink / raw)
To: haiyangz
Cc: sashal, linux-hyperv, netdev, kys, sthemmin, olaf, vkuznets,
linux-kernel
In-Reply-To: <1579816355-6933-1-git-send-email-haiyangz@microsoft.com>
From: Haiyang Zhang <haiyangz@microsoft.com>
Date: Thu, 23 Jan 2020 13:52:33 -0800
> Add XDP support and update related document.
Series applied, thakns.
^ permalink raw reply
* RE: [PATCH][RESEND] Input: hyperv-keyboard: Add the support of hibernation
From: Dexuan Cui @ 2020-01-25 1:59 UTC (permalink / raw)
To: Sasha Levin, dmitry.torokhov@gmail.com
Cc: KY Srinivasan, Haiyang Zhang, Stephen Hemminger,
linux-hyperv@vger.kernel.org, linux-input@vger.kernel.org,
linux-kernel@vger.kernel.org, Michael Kelley, Sasha Levin
In-Reply-To: <KU1P153MB01505A56BACA9DA4AF0E7BABBF370@KU1P153MB0150.APCP153.PROD.OUTLOOK.COM>
> From: linux-hyperv-owner@vger.kernel.org
> <linux-hyperv-owner@vger.kernel.org> On Behalf Of Dexuan Cui
> Sent: Wednesday, January 15, 2020 11:53 AM
> > ...
> > On Mon, Jan 06, 2020 at 02:40:44PM -0800, Dexuan Cui wrote:
> > >Add suspend() and resume() functions so the Hyper-V virtual keyboard
> > >can participate in VM hibernation.
> > >
> > >Note that the keyboard is a "wakeup" device that could abort an in-progress
> > >hibernation if there is keyboard event. No attempt is made to suppress
> > this
> > >behavior. If desired, a sysadmin can disable the keyboard as a wakeup
> > device
> > >using standard mechanisms such as:
> > >
> > >echo disabled >
> > /sys/bus/vmbus/drivers/hyperv_keyboard/XXX/power/wakeup
> > >(where XXX is the device's GUID)
> > >
> > >Reviewed-by: Michael Kelley <mikelley@microsoft.com>
> > >Signed-off-by: Dexuan Cui <decui@microsoft.com>
> > >---
> > >
> > >This is a RESEND of https://lkml.org/lkml/2019/11/24/115 .
> >
> > >Please review.
> > >
> > >If it looks good, Sasha Levin, can you please pick it up via the
> > >hyperv/linux.git tree, as you did last time for this driver?
> >
> > This will need an ack from the input driver maintainers, unless they
> > want to give a blanket ack to this type of patches.
> >
> > Thanks,
> > Sasha
>
> Hi Dmitry,
> May I have your Acked-by for this patch?
>
> Thanks
> -- Dexuan
It looks Dmitry may be on leave and there is no ACK from any input driver
maintainers. :-(
Sasha, for this patch, I would suggest it may as well go directly through the
Hyper-V tree because
1. It's really small and it's a pure Hyper-V change.
2. This is for a new functionality and it can not break any existing code.
3. I'm pretty sure it can not cause any merge conflict or build failure.
It's still wonderful to get an Ack or some comment from the input driver
maintainer(s), so maybe let's wait more time (e.g. a week?) and then I
suggust we proceed.
Thanks,
-- Dexuan
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox