* [PATCH 1/7] x86/hyper-v: Suspend/resume the hypercall page for hibernation
2019-07-09 5:29 [PATCH 0/7] Enhance the hv_vmbus driver to support hibernation Dexuan Cui
@ 2019-07-09 5:29 ` Dexuan Cui
2019-07-30 22:18 ` Michael Kelley
2019-07-09 5:29 ` [PATCH 2/7] clocksource/drivers: Suspend/resume Hyper-V clocksource " Dexuan Cui
` (5 subsequent siblings)
6 siblings, 1 reply; 17+ messages in thread
From: Dexuan Cui @ 2019-07-09 5:29 UTC (permalink / raw)
To: linux-hyperv, gregkh, Stephen Hemminger, Sasha Levin, sashal,
Haiyang Zhang, KY Srinivasan, Michael Kelley, tglx
Cc: linux-kernel, Dexuan Cui
This is needed for hibernation, e.g. when we resume the old kernel, we need
to disable the "current" kernel's hypercall page and then resume the old
kernel's.
Signed-off-by: Dexuan Cui <decui@microsoft.com>
---
arch/x86/hyperv/hv_init.c | 34 ++++++++++++++++++++++++++++++++++
1 file changed, 34 insertions(+)
diff --git a/arch/x86/hyperv/hv_init.c b/arch/x86/hyperv/hv_init.c
index 0e033ef..3005871 100644
--- a/arch/x86/hyperv/hv_init.c
+++ b/arch/x86/hyperv/hv_init.c
@@ -20,6 +20,7 @@
#include <linux/hyperv.h>
#include <linux/slab.h>
#include <linux/cpuhotplug.h>
+#include <linux/syscore_ops.h>
#include <clocksource/hyperv_timer.h>
void *hv_hypercall_pg;
@@ -214,6 +215,34 @@ static int __init hv_pci_init(void)
return 1;
}
+static int hv_suspend(void)
+{
+ union hv_x64_msr_hypercall_contents hypercall_msr;
+
+ /* Reset the hypercall page */
+ rdmsrl(HV_X64_MSR_HYPERCALL, hypercall_msr.as_uint64);
+ hypercall_msr.enable = 0;
+ wrmsrl(HV_X64_MSR_HYPERCALL, hypercall_msr.as_uint64);
+
+ return 0;
+}
+
+static void hv_resume(void)
+{
+ union hv_x64_msr_hypercall_contents hypercall_msr;
+
+ /* Re-enable the hypercall page */
+ rdmsrl(HV_X64_MSR_HYPERCALL, hypercall_msr.as_uint64);
+ hypercall_msr.enable = 1;
+ hypercall_msr.guest_physical_address = vmalloc_to_pfn(hv_hypercall_pg);
+ wrmsrl(HV_X64_MSR_HYPERCALL, hypercall_msr.as_uint64);
+}
+
+static struct syscore_ops hv_syscore_ops = {
+ .suspend = hv_suspend,
+ .resume = hv_resume,
+};
+
/*
* This function is to be invoked early in the boot sequence after the
* hypervisor has been detected.
@@ -294,6 +323,9 @@ void __init hyperv_init(void)
/* Register Hyper-V specific clocksource */
hv_init_clocksource();
+
+ register_syscore_ops(&hv_syscore_ops);
+
return;
remove_cpuhp_state:
@@ -313,6 +345,8 @@ void hyperv_cleanup(void)
{
union hv_x64_msr_hypercall_contents hypercall_msr;
+ unregister_syscore_ops(&hv_syscore_ops);
+
/* Reset our OS id */
wrmsrl(HV_X64_MSR_GUEST_OS_ID, 0);
--
1.8.3.1
^ permalink raw reply related [flat|nested] 17+ messages in thread
* RE: [PATCH 1/7] x86/hyper-v: Suspend/resume the hypercall page for hibernation
2019-07-09 5:29 ` [PATCH 1/7] x86/hyper-v: Suspend/resume the hypercall page for hibernation Dexuan Cui
@ 2019-07-30 22:18 ` Michael Kelley
0 siblings, 0 replies; 17+ messages in thread
From: Michael Kelley @ 2019-07-30 22:18 UTC (permalink / raw)
To: Dexuan Cui, linux-hyperv, gregkh, Stephen Hemminger, Sasha Levin,
sashal, Haiyang Zhang, KY Srinivasan, tglx
Cc: linux-kernel
From: Dexuan Cui <decui@microsoft.com> Sent: Monday, July 8, 2019 10:29 PM
>
> This is needed for hibernation, e.g. when we resume the old kernel, we need
> to disable the "current" kernel's hypercall page and then resume the old
> kernel's.
>
> Signed-off-by: Dexuan Cui <decui@microsoft.com>
> ---
> arch/x86/hyperv/hv_init.c | 34 ++++++++++++++++++++++++++++++++++
> 1 file changed, 34 insertions(+)
>
> diff --git a/arch/x86/hyperv/hv_init.c b/arch/x86/hyperv/hv_init.c
> index 0e033ef..3005871 100644
> --- a/arch/x86/hyperv/hv_init.c
> +++ b/arch/x86/hyperv/hv_init.c
> @@ -20,6 +20,7 @@
> #include <linux/hyperv.h>
> #include <linux/slab.h>
> #include <linux/cpuhotplug.h>
> +#include <linux/syscore_ops.h>
> #include <clocksource/hyperv_timer.h>
>
> void *hv_hypercall_pg;
> @@ -214,6 +215,34 @@ static int __init hv_pci_init(void)
> return 1;
> }
>
> +static int hv_suspend(void)
> +{
> + union hv_x64_msr_hypercall_contents hypercall_msr;
> +
> + /* Reset the hypercall page */
> + rdmsrl(HV_X64_MSR_HYPERCALL, hypercall_msr.as_uint64);
> + hypercall_msr.enable = 0;
> + wrmsrl(HV_X64_MSR_HYPERCALL, hypercall_msr.as_uint64);
> +
> + return 0;
> +}
> +
> +static void hv_resume(void)
> +{
> + union hv_x64_msr_hypercall_contents hypercall_msr;
> +
> + /* Re-enable the hypercall page */
> + rdmsrl(HV_X64_MSR_HYPERCALL, hypercall_msr.as_uint64);
> + hypercall_msr.enable = 1;
> + hypercall_msr.guest_physical_address = vmalloc_to_pfn(hv_hypercall_pg);
> + wrmsrl(HV_X64_MSR_HYPERCALL, hypercall_msr.as_uint64);
> +}
> +
> +static struct syscore_ops hv_syscore_ops = {
> + .suspend = hv_suspend,
> + .resume = hv_resume,
> +};
> +
> /*
> * This function is to be invoked early in the boot sequence after the
> * hypervisor has been detected.
> @@ -294,6 +323,9 @@ void __init hyperv_init(void)
>
> /* Register Hyper-V specific clocksource */
> hv_init_clocksource();
> +
> + register_syscore_ops(&hv_syscore_ops);
> +
> return;
>
> remove_cpuhp_state:
> @@ -313,6 +345,8 @@ void hyperv_cleanup(void)
> {
> union hv_x64_msr_hypercall_contents hypercall_msr;
>
> + unregister_syscore_ops(&hv_syscore_ops);
> +
> /* Reset our OS id */
> wrmsrl(HV_X64_MSR_GUEST_OS_ID, 0);
>
> --
> 1.8.3.1
Reviewed-by: Michael Kelley <mikelley@microsoft.com>
^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH 2/7] clocksource/drivers: Suspend/resume Hyper-V clocksource for hibernation
2019-07-09 5:29 [PATCH 0/7] Enhance the hv_vmbus driver to support hibernation Dexuan Cui
2019-07-09 5:29 ` [PATCH 1/7] x86/hyper-v: Suspend/resume the hypercall page for hibernation Dexuan Cui
@ 2019-07-09 5:29 ` Dexuan Cui
2019-07-30 22:23 ` Michael Kelley
2019-07-09 5:29 ` [PATCH 3/7] Drivers: hv: vmbus: Split hv_synic_init/cleanup into regs and timer settings Dexuan Cui
` (4 subsequent siblings)
6 siblings, 1 reply; 17+ messages in thread
From: Dexuan Cui @ 2019-07-09 5:29 UTC (permalink / raw)
To: linux-hyperv, gregkh, Stephen Hemminger, Sasha Levin, sashal,
Haiyang Zhang, KY Srinivasan, Michael Kelley, tglx
Cc: linux-kernel, Dexuan Cui
This is needed for hibernation, e.g. when we resume the old kernel, we need
to disable the "current" kernel's TSC page and then resume the old kernel's.
Signed-off-by: Dexuan Cui <decui@microsoft.com>
---
drivers/clocksource/hyperv_timer.c | 25 +++++++++++++++++++++++++
1 file changed, 25 insertions(+)
diff --git a/drivers/clocksource/hyperv_timer.c b/drivers/clocksource/hyperv_timer.c
index ba2c79e6..41c31a7 100644
--- a/drivers/clocksource/hyperv_timer.c
+++ b/drivers/clocksource/hyperv_timer.c
@@ -237,12 +237,37 @@ static u64 read_hv_clock_tsc(struct clocksource *arg)
return read_hv_sched_clock_tsc();
}
+static void suspend_hv_clock_tsc(struct clocksource *arg)
+{
+ u64 tsc_msr;
+
+ /* Disable the TSC page */
+ hv_get_reference_tsc(tsc_msr);
+ tsc_msr &= ~BIT_ULL(0);
+ hv_set_reference_tsc(tsc_msr);
+}
+
+
+static void resume_hv_clock_tsc(struct clocksource *arg)
+{
+ phys_addr_t phys_addr = page_to_phys(vmalloc_to_page(tsc_pg));
+ u64 tsc_msr;
+
+ /* Re-enable the TSC page */
+ hv_get_reference_tsc(tsc_msr);
+ tsc_msr &= GENMASK_ULL(11, 0);
+ tsc_msr |= BIT_ULL(0) | (u64)phys_addr;
+ hv_set_reference_tsc(tsc_msr);
+}
+
static struct clocksource hyperv_cs_tsc = {
.name = "hyperv_clocksource_tsc_page",
.rating = 400,
.read = read_hv_clock_tsc,
.mask = CLOCKSOURCE_MASK(64),
.flags = CLOCK_SOURCE_IS_CONTINUOUS,
+ .suspend= suspend_hv_clock_tsc,
+ .resume = resume_hv_clock_tsc,
};
#endif
--
1.8.3.1
^ permalink raw reply related [flat|nested] 17+ messages in thread
* RE: [PATCH 2/7] clocksource/drivers: Suspend/resume Hyper-V clocksource for hibernation
2019-07-09 5:29 ` [PATCH 2/7] clocksource/drivers: Suspend/resume Hyper-V clocksource " Dexuan Cui
@ 2019-07-30 22:23 ` Michael Kelley
0 siblings, 0 replies; 17+ messages in thread
From: Michael Kelley @ 2019-07-30 22:23 UTC (permalink / raw)
To: Dexuan Cui, linux-hyperv, gregkh, Stephen Hemminger, Sasha Levin,
sashal, Haiyang Zhang, KY Srinivasan, tglx
Cc: linux-kernel
From: Dexuan Cui <decui@microsoft.com> Sent: Monday, July 8, 2019 10:29 PM
>
> This is needed for hibernation, e.g. when we resume the old kernel, we need
> to disable the "current" kernel's TSC page and then resume the old kernel's.
>
> Signed-off-by: Dexuan Cui <decui@microsoft.com>
> ---
> drivers/clocksource/hyperv_timer.c | 25 +++++++++++++++++++++++++
> 1 file changed, 25 insertions(+)
>
> diff --git a/drivers/clocksource/hyperv_timer.c b/drivers/clocksource/hyperv_timer.c
> index ba2c79e6..41c31a7 100644
> --- a/drivers/clocksource/hyperv_timer.c
> +++ b/drivers/clocksource/hyperv_timer.c
> @@ -237,12 +237,37 @@ static u64 read_hv_clock_tsc(struct clocksource *arg)
> return read_hv_sched_clock_tsc();
> }
>
> +static void suspend_hv_clock_tsc(struct clocksource *arg)
> +{
> + u64 tsc_msr;
> +
> + /* Disable the TSC page */
> + hv_get_reference_tsc(tsc_msr);
> + tsc_msr &= ~BIT_ULL(0);
> + hv_set_reference_tsc(tsc_msr);
> +}
> +
> +
> +static void resume_hv_clock_tsc(struct clocksource *arg)
> +{
> + phys_addr_t phys_addr = page_to_phys(vmalloc_to_page(tsc_pg));
> + u64 tsc_msr;
> +
> + /* Re-enable the TSC page */
> + hv_get_reference_tsc(tsc_msr);
> + tsc_msr &= GENMASK_ULL(11, 0);
> + tsc_msr |= BIT_ULL(0) | (u64)phys_addr;
> + hv_set_reference_tsc(tsc_msr);
> +}
> +
> static struct clocksource hyperv_cs_tsc = {
> .name = "hyperv_clocksource_tsc_page",
> .rating = 400,
> .read = read_hv_clock_tsc,
> .mask = CLOCKSOURCE_MASK(64),
> .flags = CLOCK_SOURCE_IS_CONTINUOUS,
> + .suspend= suspend_hv_clock_tsc,
> + .resume = resume_hv_clock_tsc,
> };
> #endif
>
> --
> 1.8.3.1
Reviewed-by: Michael Kelley <mikelley@microsoft.com>
^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH 3/7] Drivers: hv: vmbus: Split hv_synic_init/cleanup into regs and timer settings
2019-07-09 5:29 [PATCH 0/7] Enhance the hv_vmbus driver to support hibernation Dexuan Cui
2019-07-09 5:29 ` [PATCH 1/7] x86/hyper-v: Suspend/resume the hypercall page for hibernation Dexuan Cui
2019-07-09 5:29 ` [PATCH 2/7] clocksource/drivers: Suspend/resume Hyper-V clocksource " Dexuan Cui
@ 2019-07-09 5:29 ` Dexuan Cui
2019-07-30 22:35 ` Michael Kelley
2019-07-09 5:29 ` [PATCH 4/7] Drivers: hv: vmbus: Suspend/resume the synic for hibernation Dexuan Cui
` (3 subsequent siblings)
6 siblings, 1 reply; 17+ messages in thread
From: Dexuan Cui @ 2019-07-09 5:29 UTC (permalink / raw)
To: linux-hyperv, gregkh, Stephen Hemminger, Sasha Levin, sashal,
Haiyang Zhang, KY Srinivasan, Michael Kelley, tglx
Cc: linux-kernel, Dexuan Cui
There is only one functional change: the unnecessary check
"if (sctrl.enable != 1) return -EFAULT;" is removed, because when we're in
hv_synic_cleanup(), we're absolutely sure sctrl.enable must be 1.
The new functions hv_synic_disable/enable_regs() will be used by a later patch
to support hibernation.
Signed-off-by: Dexuan Cui <decui@microsoft.com>
---
drivers/hv/hv.c | 66 ++++++++++++++++++++++++++---------------------
drivers/hv/hyperv_vmbus.h | 2 ++
2 files changed, 39 insertions(+), 29 deletions(-)
diff --git a/drivers/hv/hv.c b/drivers/hv/hv.c
index 6188fb7..fcc5279 100644
--- a/drivers/hv/hv.c
+++ b/drivers/hv/hv.c
@@ -154,7 +154,7 @@ void hv_synic_free(void)
* retrieve the initialized message and event pages. Otherwise, we create and
* initialize the message and event pages.
*/
-int hv_synic_init(unsigned int cpu)
+void hv_synic_enable_regs(unsigned int cpu)
{
struct hv_per_cpu_context *hv_cpu
= per_cpu_ptr(hv_context.cpu_context, cpu);
@@ -196,6 +196,11 @@ int hv_synic_init(unsigned int cpu)
sctrl.enable = 1;
hv_set_synic_state(sctrl.as_uint64);
+}
+
+int hv_synic_init(unsigned int cpu)
+{
+ hv_synic_enable_regs(cpu);
hv_stimer_init(cpu);
@@ -205,20 +210,45 @@ int hv_synic_init(unsigned int cpu)
/*
* hv_synic_cleanup - Cleanup routine for hv_synic_init().
*/
-int hv_synic_cleanup(unsigned int cpu)
+void hv_synic_disable_regs(unsigned int cpu)
{
union hv_synic_sint shared_sint;
union hv_synic_simp simp;
union hv_synic_siefp siefp;
union hv_synic_scontrol sctrl;
+
+ hv_get_synint_state(VMBUS_MESSAGE_SINT, shared_sint.as_uint64);
+
+ shared_sint.masked = 1;
+
+ /* Need to correctly cleanup in the case of SMP!!! */
+ /* Disable the interrupt */
+ hv_set_synint_state(VMBUS_MESSAGE_SINT, shared_sint.as_uint64);
+
+ hv_get_simp(simp.as_uint64);
+ simp.simp_enabled = 0;
+ simp.base_simp_gpa = 0;
+
+ hv_set_simp(simp.as_uint64);
+
+ hv_get_siefp(siefp.as_uint64);
+ siefp.siefp_enabled = 0;
+ siefp.base_siefp_gpa = 0;
+
+ hv_set_siefp(siefp.as_uint64);
+
+ /* Disable the global synic bit */
+ hv_get_synic_state(sctrl.as_uint64);
+ sctrl.enable = 0;
+ hv_set_synic_state(sctrl.as_uint64);
+}
+
+int hv_synic_cleanup(unsigned int cpu)
+{
struct vmbus_channel *channel, *sc;
bool channel_found = false;
unsigned long flags;
- hv_get_synic_state(sctrl.as_uint64);
- if (sctrl.enable != 1)
- return -EFAULT;
-
/*
* Search for channels which are bound to the CPU we're about to
* cleanup. In case we find one and vmbus is still connected we need to
@@ -249,29 +279,7 @@ int hv_synic_cleanup(unsigned int cpu)
hv_stimer_cleanup(cpu);
- hv_get_synint_state(VMBUS_MESSAGE_SINT, shared_sint.as_uint64);
-
- shared_sint.masked = 1;
-
- /* Need to correctly cleanup in the case of SMP!!! */
- /* Disable the interrupt */
- hv_set_synint_state(VMBUS_MESSAGE_SINT, shared_sint.as_uint64);
-
- hv_get_simp(simp.as_uint64);
- simp.simp_enabled = 0;
- simp.base_simp_gpa = 0;
-
- hv_set_simp(simp.as_uint64);
-
- hv_get_siefp(siefp.as_uint64);
- siefp.siefp_enabled = 0;
- siefp.base_siefp_gpa = 0;
-
- hv_set_siefp(siefp.as_uint64);
-
- /* Disable the global synic bit */
- sctrl.enable = 0;
- hv_set_synic_state(sctrl.as_uint64);
+ hv_synic_disable_regs(cpu);
return 0;
}
diff --git a/drivers/hv/hyperv_vmbus.h b/drivers/hv/hyperv_vmbus.h
index 362e70e..26ea161 100644
--- a/drivers/hv/hyperv_vmbus.h
+++ b/drivers/hv/hyperv_vmbus.h
@@ -171,8 +171,10 @@ extern int hv_post_message(union hv_connection_id connection_id,
extern void hv_synic_free(void);
+extern void hv_synic_enable_regs(unsigned int cpu);
extern int hv_synic_init(unsigned int cpu);
+extern void hv_synic_disable_regs(unsigned int cpu);
extern int hv_synic_cleanup(unsigned int cpu);
/* Interface */
--
1.8.3.1
^ permalink raw reply related [flat|nested] 17+ messages in thread
* RE: [PATCH 3/7] Drivers: hv: vmbus: Split hv_synic_init/cleanup into regs and timer settings
2019-07-09 5:29 ` [PATCH 3/7] Drivers: hv: vmbus: Split hv_synic_init/cleanup into regs and timer settings Dexuan Cui
@ 2019-07-30 22:35 ` Michael Kelley
2019-07-30 23:18 ` Dexuan Cui
0 siblings, 1 reply; 17+ messages in thread
From: Michael Kelley @ 2019-07-30 22:35 UTC (permalink / raw)
To: Dexuan Cui, linux-hyperv, gregkh, Stephen Hemminger, Sasha Levin,
sashal, Haiyang Zhang, KY Srinivasan, tglx
Cc: linux-kernel
From: Dexuan Cui <decui@microsoft.com> Sent: Monday, July 8, 2019 10:29 PM
>
> There is only one functional change: the unnecessary check
> "if (sctrl.enable != 1) return -EFAULT;" is removed, because when we're in
> hv_synic_cleanup(), we're absolutely sure sctrl.enable must be 1.
>
> The new functions hv_synic_disable/enable_regs() will be used by a later patch
> to support hibernation.
Seems like this commit message doesn't really describe the main change.
How about:
Break out synic enable and disable operations into separate
hv_synic_disable_regs() and hv_synic_enable_regs() functions for use by a
later patch to support hibernation.
There is no functional change except the unnecessary check
"if (sctrl.enable != 1) return -EFAULT;" is removed, because when we're in
hv_synic_cleanup(), we're absolutely sure sctrl.enable must be 1.
Otherwise,
Reviewed-by: Michael Kelley <mikelley@microsoft.com>
>
> Signed-off-by: Dexuan Cui <decui@microsoft.com>
> ---
> drivers/hv/hv.c | 66 ++++++++++++++++++++++++++---------------------
> drivers/hv/hyperv_vmbus.h | 2 ++
> 2 files changed, 39 insertions(+), 29 deletions(-)
>
> diff --git a/drivers/hv/hv.c b/drivers/hv/hv.c
> index 6188fb7..fcc5279 100644
> --- a/drivers/hv/hv.c
> +++ b/drivers/hv/hv.c
> @@ -154,7 +154,7 @@ void hv_synic_free(void)
> * retrieve the initialized message and event pages. Otherwise, we create and
> * initialize the message and event pages.
> */
> -int hv_synic_init(unsigned int cpu)
> +void hv_synic_enable_regs(unsigned int cpu)
> {
> struct hv_per_cpu_context *hv_cpu
> = per_cpu_ptr(hv_context.cpu_context, cpu);
> @@ -196,6 +196,11 @@ int hv_synic_init(unsigned int cpu)
> sctrl.enable = 1;
>
> hv_set_synic_state(sctrl.as_uint64);
> +}
> +
> +int hv_synic_init(unsigned int cpu)
> +{
> + hv_synic_enable_regs(cpu);
>
> hv_stimer_init(cpu);
>
> @@ -205,20 +210,45 @@ int hv_synic_init(unsigned int cpu)
> /*
> * hv_synic_cleanup - Cleanup routine for hv_synic_init().
> */
> -int hv_synic_cleanup(unsigned int cpu)
> +void hv_synic_disable_regs(unsigned int cpu)
> {
> union hv_synic_sint shared_sint;
> union hv_synic_simp simp;
> union hv_synic_siefp siefp;
> union hv_synic_scontrol sctrl;
> +
> + hv_get_synint_state(VMBUS_MESSAGE_SINT, shared_sint.as_uint64);
> +
> + shared_sint.masked = 1;
> +
> + /* Need to correctly cleanup in the case of SMP!!! */
> + /* Disable the interrupt */
> + hv_set_synint_state(VMBUS_MESSAGE_SINT, shared_sint.as_uint64);
> +
> + hv_get_simp(simp.as_uint64);
> + simp.simp_enabled = 0;
> + simp.base_simp_gpa = 0;
> +
> + hv_set_simp(simp.as_uint64);
> +
> + hv_get_siefp(siefp.as_uint64);
> + siefp.siefp_enabled = 0;
> + siefp.base_siefp_gpa = 0;
> +
> + hv_set_siefp(siefp.as_uint64);
> +
> + /* Disable the global synic bit */
> + hv_get_synic_state(sctrl.as_uint64);
> + sctrl.enable = 0;
> + hv_set_synic_state(sctrl.as_uint64);
> +}
> +
> +int hv_synic_cleanup(unsigned int cpu)
> +{
> struct vmbus_channel *channel, *sc;
> bool channel_found = false;
> unsigned long flags;
>
> - hv_get_synic_state(sctrl.as_uint64);
> - if (sctrl.enable != 1)
> - return -EFAULT;
> -
> /*
> * Search for channels which are bound to the CPU we're about to
> * cleanup. In case we find one and vmbus is still connected we need to
> @@ -249,29 +279,7 @@ int hv_synic_cleanup(unsigned int cpu)
>
> hv_stimer_cleanup(cpu);
>
> - hv_get_synint_state(VMBUS_MESSAGE_SINT, shared_sint.as_uint64);
> -
> - shared_sint.masked = 1;
> -
> - /* Need to correctly cleanup in the case of SMP!!! */
> - /* Disable the interrupt */
> - hv_set_synint_state(VMBUS_MESSAGE_SINT, shared_sint.as_uint64);
> -
> - hv_get_simp(simp.as_uint64);
> - simp.simp_enabled = 0;
> - simp.base_simp_gpa = 0;
> -
> - hv_set_simp(simp.as_uint64);
> -
> - hv_get_siefp(siefp.as_uint64);
> - siefp.siefp_enabled = 0;
> - siefp.base_siefp_gpa = 0;
> -
> - hv_set_siefp(siefp.as_uint64);
> -
> - /* Disable the global synic bit */
> - sctrl.enable = 0;
> - hv_set_synic_state(sctrl.as_uint64);
> + hv_synic_disable_regs(cpu);
>
> return 0;
> }
> diff --git a/drivers/hv/hyperv_vmbus.h b/drivers/hv/hyperv_vmbus.h
> index 362e70e..26ea161 100644
> --- a/drivers/hv/hyperv_vmbus.h
> +++ b/drivers/hv/hyperv_vmbus.h
> @@ -171,8 +171,10 @@ extern int hv_post_message(union hv_connection_id
> connection_id,
>
> extern void hv_synic_free(void);
>
> +extern void hv_synic_enable_regs(unsigned int cpu);
> extern int hv_synic_init(unsigned int cpu);
>
> +extern void hv_synic_disable_regs(unsigned int cpu);
> extern int hv_synic_cleanup(unsigned int cpu);
>
> /* Interface */
> --
> 1.8.3.1
^ permalink raw reply [flat|nested] 17+ messages in thread
* RE: [PATCH 3/7] Drivers: hv: vmbus: Split hv_synic_init/cleanup into regs and timer settings
2019-07-30 22:35 ` Michael Kelley
@ 2019-07-30 23:18 ` Dexuan Cui
0 siblings, 0 replies; 17+ messages in thread
From: Dexuan Cui @ 2019-07-30 23:18 UTC (permalink / raw)
To: Michael Kelley, linux-hyperv, gregkh, Stephen Hemminger,
Sasha Levin, sashal, Haiyang Zhang, KY Srinivasan, tglx
Cc: linux-kernel
> From: Michael Kelley <mikelley@microsoft.com>
> Sent: Tuesday, July 30, 2019 3:36 PM
>
> From: Dexuan Cui <decui@microsoft.com> Sent: Monday, July 8, 2019 10:29
> PM
> >
> > There is only one functional change: the unnecessary check
> > "if (sctrl.enable != 1) return -EFAULT;" is removed, because when we're in
> > hv_synic_cleanup(), we're absolutely sure sctrl.enable must be 1.
> >
> > The new functions hv_synic_disable/enable_regs() will be used by a later
> patch
> > to support hibernation.
>
> Seems like this commit message doesn't really describe the main change.
> How about:
>
> Break out synic enable and disable operations into separate
> hv_synic_disable_regs() and hv_synic_enable_regs() functions for use by a
> later patch to support hibernation.
>
> There is no functional change except the unnecessary check
> "if (sctrl.enable != 1) return -EFAULT;" is removed, because when we're in
> hv_synic_cleanup(), we're absolutely sure sctrl.enable must be 1.
>
> Otherwise,
>
> Reviewed-by: Michael Kelley <mikelley@microsoft.com>
Thanks! I'll use your version as the changelog of v2. I'll change the
Subject accordingly.
Thanks,
-- Dexuan
^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH 4/7] Drivers: hv: vmbus: Suspend/resume the synic for hibernation
2019-07-09 5:29 [PATCH 0/7] Enhance the hv_vmbus driver to support hibernation Dexuan Cui
` (2 preceding siblings ...)
2019-07-09 5:29 ` [PATCH 3/7] Drivers: hv: vmbus: Split hv_synic_init/cleanup into regs and timer settings Dexuan Cui
@ 2019-07-09 5:29 ` Dexuan Cui
2019-07-09 5:29 ` [PATCH 5/7] Drivers: hv: vmbus: Ignore the offers when resuming from hibernation Dexuan Cui
` (2 subsequent siblings)
6 siblings, 0 replies; 17+ messages in thread
From: Dexuan Cui @ 2019-07-09 5:29 UTC (permalink / raw)
To: linux-hyperv, gregkh, Stephen Hemminger, Sasha Levin, sashal,
Haiyang Zhang, KY Srinivasan, Michael Kelley, tglx
Cc: linux-kernel, Dexuan Cui
This is needed when we resume the old kernel from the "current" kernel.
Signed-off-by: Dexuan Cui <decui@microsoft.com>
---
drivers/hv/vmbus_drv.c | 40 ++++++++++++++++++++++++++++++++++++++++
1 file changed, 40 insertions(+)
diff --git a/drivers/hv/vmbus_drv.c b/drivers/hv/vmbus_drv.c
index 72d5a7c..1c2d935 100644
--- a/drivers/hv/vmbus_drv.c
+++ b/drivers/hv/vmbus_drv.c
@@ -30,6 +30,7 @@
#include <linux/kdebug.h>
#include <linux/efi.h>
#include <linux/random.h>
+#include <linux/syscore_ops.h>
#include <clocksource/hyperv_timer.h>
#include "hyperv_vmbus.h"
@@ -2088,6 +2089,41 @@ static void hv_crash_handler(struct pt_regs *regs)
hyperv_cleanup();
};
+static int hv_synic_suspend(void)
+{
+ /*
+ * Here we only need to care about CPU0: when the other CPUs are
+ * offlined, hv_synic_cleanup() has been called for them, and the
+ * timers on them have been automatically disabled and deleted in
+ * tick_cleanup_dead_cpu().
+ */
+ hv_stimer_cleanup(0);
+
+ hv_synic_disable_regs(0);
+
+ return 0;
+}
+
+static void hv_synic_resume(void)
+{
+ /*
+ * Here we only need to care about CPU0: when the other CPUs are
+ * onlined, hv_synic_init() has been called, and the timers are
+ * added there.
+ *
+ * Note: we don't need to call hv_stimer_init() for stimer0, because
+ * it is not deleted before hibernation and it's resumed in
+ * timekeeping_resume().
+ */
+ hv_synic_enable_regs(0);
+}
+
+/* The callbacks run only on CPU0, with irqs_disabled. */
+static struct syscore_ops hv_synic_syscore_ops = {
+ .suspend = hv_synic_suspend,
+ .resume = hv_synic_resume,
+};
+
static int __init hv_acpi_init(void)
{
int ret, t;
@@ -2118,6 +2154,8 @@ static int __init hv_acpi_init(void)
hv_setup_kexec_handler(hv_kexec_handler);
hv_setup_crash_handler(hv_crash_handler);
+ register_syscore_ops(&hv_synic_syscore_ops);
+
return 0;
cleanup:
@@ -2130,6 +2168,8 @@ static void __exit vmbus_exit(void)
{
int cpu;
+ unregister_syscore_ops(&hv_synic_syscore_ops);
+
hv_remove_kexec_handler();
hv_remove_crash_handler();
vmbus_connection.conn_state = DISCONNECTED;
--
1.8.3.1
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH 5/7] Drivers: hv: vmbus: Ignore the offers when resuming from hibernation
2019-07-09 5:29 [PATCH 0/7] Enhance the hv_vmbus driver to support hibernation Dexuan Cui
` (3 preceding siblings ...)
2019-07-09 5:29 ` [PATCH 4/7] Drivers: hv: vmbus: Suspend/resume the synic for hibernation Dexuan Cui
@ 2019-07-09 5:29 ` Dexuan Cui
2019-07-30 23:07 ` Michael Kelley
2019-07-09 5:29 ` [PATCH 6/7] Drivers: hv: vmbus: Suspend/resume the vmbus itself for hibernation Dexuan Cui
2019-07-09 5:29 ` [PATCH 7/7] Drivers: hv: vmbus: Implement suspend/resume for VSC drivers " Dexuan Cui
6 siblings, 1 reply; 17+ messages in thread
From: Dexuan Cui @ 2019-07-09 5:29 UTC (permalink / raw)
To: linux-hyperv, gregkh, Stephen Hemminger, Sasha Levin, sashal,
Haiyang Zhang, KY Srinivasan, Michael Kelley, tglx
Cc: linux-kernel, Dexuan Cui
When the VM resumes, the host re-sends the offers. We should not add the
offers to the global vmbus_connection.chn_list again.
Added some debug code, in case the host screws up the exact info related to
the offers.
Signed-off-by: Dexuan Cui <decui@microsoft.com>
---
drivers/hv/channel_mgmt.c | 28 +++++++++++++++++++++++++++-
1 file changed, 27 insertions(+), 1 deletion(-)
diff --git a/drivers/hv/channel_mgmt.c b/drivers/hv/channel_mgmt.c
index addcef5..a9aeeab 100644
--- a/drivers/hv/channel_mgmt.c
+++ b/drivers/hv/channel_mgmt.c
@@ -854,12 +854,38 @@ void vmbus_initiate_unload(bool crash)
static void vmbus_onoffer(struct vmbus_channel_message_header *hdr)
{
struct vmbus_channel_offer_channel *offer;
- struct vmbus_channel *newchannel;
+ struct vmbus_channel *oldchannel, *newchannel;
+ size_t offer_sz;
offer = (struct vmbus_channel_offer_channel *)hdr;
trace_vmbus_onoffer(offer);
+ mutex_lock(&vmbus_connection.channel_mutex);
+ oldchannel = relid2channel(offer->child_relid);
+ mutex_unlock(&vmbus_connection.channel_mutex);
+
+ if (oldchannel != NULL) {
+ atomic_dec(&vmbus_connection.offer_in_progress);
+
+ /*
+ * We're resuming from hibernation: we expect the host to send
+ * exactly the same offers that we had before the hibernation.
+ */
+ offer_sz = sizeof(*offer);
+ if (memcmp(offer, &oldchannel->offermsg, offer_sz) == 0)
+ return;
+
+ pr_err("Mismatched offer from the host (relid=%d)!\n",
+ offer->child_relid);
+
+ print_hex_dump_debug("Old vmbus offer: ", DUMP_PREFIX_OFFSET, 4,
+ 4, &oldchannel->offermsg, offer_sz, false);
+ print_hex_dump_debug("New vmbus offer: ", DUMP_PREFIX_OFFSET, 4,
+ 4, offer, offer_sz, false);
+ return;
+ }
+
/* Allocate the channel object and save this offer. */
newchannel = alloc_channel();
if (!newchannel) {
--
1.8.3.1
^ permalink raw reply related [flat|nested] 17+ messages in thread
* RE: [PATCH 5/7] Drivers: hv: vmbus: Ignore the offers when resuming from hibernation
2019-07-09 5:29 ` [PATCH 5/7] Drivers: hv: vmbus: Ignore the offers when resuming from hibernation Dexuan Cui
@ 2019-07-30 23:07 ` Michael Kelley
2019-07-31 0:01 ` Dexuan Cui
0 siblings, 1 reply; 17+ messages in thread
From: Michael Kelley @ 2019-07-30 23:07 UTC (permalink / raw)
To: Dexuan Cui, linux-hyperv, gregkh, Stephen Hemminger, Sasha Levin,
sashal, Haiyang Zhang, KY Srinivasan, tglx
Cc: linux-kernel
From: Dexuan Cui <decui@microsoft.com> Sent: Monday, July 8, 2019 10:29 PM
>
> When the VM resumes, the host re-sends the offers. We should not add the
> offers to the global vmbus_connection.chn_list again.
>
> Added some debug code, in case the host screws up the exact info related to
> the offers.
>
> Signed-off-by: Dexuan Cui <decui@microsoft.com>
> ---
> drivers/hv/channel_mgmt.c | 28 +++++++++++++++++++++++++++-
> 1 file changed, 27 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/hv/channel_mgmt.c b/drivers/hv/channel_mgmt.c
> index addcef5..a9aeeab 100644
> --- a/drivers/hv/channel_mgmt.c
> +++ b/drivers/hv/channel_mgmt.c
> @@ -854,12 +854,38 @@ void vmbus_initiate_unload(bool crash)
> static void vmbus_onoffer(struct vmbus_channel_message_header *hdr)
> {
> struct vmbus_channel_offer_channel *offer;
> - struct vmbus_channel *newchannel;
> + struct vmbus_channel *oldchannel, *newchannel;
> + size_t offer_sz;
>
> offer = (struct vmbus_channel_offer_channel *)hdr;
>
> trace_vmbus_onoffer(offer);
>
> + mutex_lock(&vmbus_connection.channel_mutex);
> + oldchannel = relid2channel(offer->child_relid);
> + mutex_unlock(&vmbus_connection.channel_mutex);
> +
> + if (oldchannel != NULL) {
> + atomic_dec(&vmbus_connection.offer_in_progress);
> +
> + /*
> + * We're resuming from hibernation: we expect the host to send
> + * exactly the same offers that we had before the hibernation.
> + */
> + offer_sz = sizeof(*offer);
> + if (memcmp(offer, &oldchannel->offermsg, offer_sz) == 0)
> + return;
The offermsg contains "reserved" and "padding" fields. Does Hyper-V
guarantee that all these fields are the same in the new offer after resuming
from hibernation? Or should a less stringent check be made? For example,
I could imagine a newer version of Hyper-V allowing a VM that was
hibernated on an older version to be resumed. But one of the reserved fields
might be used in the newer version, and the comparison could fail
unnecessarily.
> +
> + pr_err("Mismatched offer from the host (relid=%d)!\n",
> + offer->child_relid);
> +
> + print_hex_dump_debug("Old vmbus offer: ", DUMP_PREFIX_OFFSET, 4,
> + 4, &oldchannel->offermsg, offer_sz, false);
> + print_hex_dump_debug("New vmbus offer: ", DUMP_PREFIX_OFFSET, 4,
> + 4, offer, offer_sz, false);
The third argument to print_hex_dump() is the rowsize and is specified as must
be 16 or 32.
> + return;
> + }
> +
> /* Allocate the channel object and save this offer. */
> newchannel = alloc_channel();
> if (!newchannel) {
> --
> 1.8.3.1
^ permalink raw reply [flat|nested] 17+ messages in thread
* RE: [PATCH 5/7] Drivers: hv: vmbus: Ignore the offers when resuming from hibernation
2019-07-30 23:07 ` Michael Kelley
@ 2019-07-31 0:01 ` Dexuan Cui
0 siblings, 0 replies; 17+ messages in thread
From: Dexuan Cui @ 2019-07-31 0:01 UTC (permalink / raw)
To: Michael Kelley, linux-hyperv, gregkh, Stephen Hemminger,
Sasha Levin, sashal, Haiyang Zhang, KY Srinivasan, tglx
Cc: linux-kernel
> From: Michael Kelley <mikelley@microsoft.com>
> Sent: Tuesday, July 30, 2019 4:07 PM
> > +
> > + if (oldchannel != NULL) {
> > + atomic_dec(&vmbus_connection.offer_in_progress);
> > +
> > + /*
> > + * We're resuming from hibernation: we expect the host to send
> > + * exactly the same offers that we had before the hibernation.
> > + */
> > + offer_sz = sizeof(*offer);
> > + if (memcmp(offer, &oldchannel->offermsg, offer_sz) == 0)
> > + return;
>
> The offermsg contains "reserved" and "padding" fields. Does Hyper-V
> guarantee that all these fields are the same in the new offer after resuming
> from hibernation?
Yes. I confirmed this with Hyper-V team. The reserved/padding fields don't change
across hiberantion. BTW, the fields are filled with zeros since they're not used.
> Or should a less stringent check be made? For example,
> I could imagine a newer version of Hyper-V allowing a VM that was
> hibernated on an older version to be resumed. But one of the reserved fields
> might be used in the newer version, and the comparison could fail
> unnecessarily.
Upon resume, Linux VM always uses the same version, which was used when the
VM firstly booted up before suspend, to re-negotiate with the host.
> > +
> > + pr_err("Mismatched offer from the host (relid=%d)!\n",
> > + offer->child_relid);
> > +
> > + print_hex_dump_debug("Old vmbus offer: ", DUMP_PREFIX_OFFSET,
> 4,
> > + 4, &oldchannel->offermsg, offer_sz, false);
> > + print_hex_dump_debug("New vmbus offer: ",
> DUMP_PREFIX_OFFSET, 4,
> > + 4, offer, offer_sz, false);
>
> The third argument to print_hex_dump() is the rowsize and is specified as must
> be 16 or 32.
Thanks! I misunderstood the argument. I'll change it to 16.
Thanks,
-- Dexuan
^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH 6/7] Drivers: hv: vmbus: Suspend/resume the vmbus itself for hibernation
2019-07-09 5:29 [PATCH 0/7] Enhance the hv_vmbus driver to support hibernation Dexuan Cui
` (4 preceding siblings ...)
2019-07-09 5:29 ` [PATCH 5/7] Drivers: hv: vmbus: Ignore the offers when resuming from hibernation Dexuan Cui
@ 2019-07-09 5:29 ` Dexuan Cui
2019-07-30 23:25 ` Michael Kelley
2019-07-09 5:29 ` [PATCH 7/7] Drivers: hv: vmbus: Implement suspend/resume for VSC drivers " Dexuan Cui
6 siblings, 1 reply; 17+ messages in thread
From: Dexuan Cui @ 2019-07-09 5:29 UTC (permalink / raw)
To: linux-hyperv, gregkh, Stephen Hemminger, Sasha Levin, sashal,
Haiyang Zhang, KY Srinivasan, Michael Kelley, tglx
Cc: linux-kernel, Dexuan Cui
This is needed when we resume the old kernel from the "current" kernel.
Signed-off-by: Dexuan Cui <decui@microsoft.com>
---
drivers/hv/connection.c | 3 +--
drivers/hv/hyperv_vmbus.h | 2 ++
drivers/hv/vmbus_drv.c | 40 ++++++++++++++++++++++++++++++++++++++++
3 files changed, 43 insertions(+), 2 deletions(-)
diff --git a/drivers/hv/connection.c b/drivers/hv/connection.c
index 09829e1..806319c 100644
--- a/drivers/hv/connection.c
+++ b/drivers/hv/connection.c
@@ -59,8 +59,7 @@ static __u32 vmbus_get_next_version(__u32 current_version)
}
}
-static int vmbus_negotiate_version(struct vmbus_channel_msginfo *msginfo,
- __u32 version)
+int vmbus_negotiate_version(struct vmbus_channel_msginfo *msginfo, u32 version)
{
int ret = 0;
unsigned int cur_cpu;
diff --git a/drivers/hv/hyperv_vmbus.h b/drivers/hv/hyperv_vmbus.h
index 26ea161..e657197 100644
--- a/drivers/hv/hyperv_vmbus.h
+++ b/drivers/hv/hyperv_vmbus.h
@@ -274,6 +274,8 @@ struct vmbus_msginfo {
extern struct vmbus_connection vmbus_connection;
+int vmbus_negotiate_version(struct vmbus_channel_msginfo *msginfo, u32 version);
+
static inline void vmbus_send_interrupt(u32 relid)
{
sync_set_bit(relid, vmbus_connection.send_int_page);
diff --git a/drivers/hv/vmbus_drv.c b/drivers/hv/vmbus_drv.c
index 1c2d935..1730e7b 100644
--- a/drivers/hv/vmbus_drv.c
+++ b/drivers/hv/vmbus_drv.c
@@ -2045,6 +2045,41 @@ static int vmbus_acpi_add(struct acpi_device *device)
return ret_val;
}
+static int vmbus_bus_suspend(struct device *dev)
+{
+ vmbus_initiate_unload(false);
+
+ vmbus_connection.conn_state = DISCONNECTED;
+
+ return 0;
+}
+
+static int vmbus_bus_resume(struct device *dev)
+{
+ struct vmbus_channel_msginfo *msginfo;
+ size_t msgsize;
+ int ret;
+
+ msgsize = sizeof(*msginfo) +
+ sizeof(struct vmbus_channel_initiate_contact);
+
+ msginfo = kzalloc(msgsize, GFP_KERNEL);
+
+ if (msginfo == NULL)
+ return -ENOMEM;
+
+ ret = vmbus_negotiate_version(msginfo, vmbus_proto_version);
+
+ kfree(msginfo);
+
+ if (ret != 0)
+ return ret;
+
+ vmbus_request_offers();
+
+ return 0;
+}
+
static const struct acpi_device_id vmbus_acpi_device_ids[] = {
{"VMBUS", 0},
{"VMBus", 0},
@@ -2052,6 +2087,10 @@ static int vmbus_acpi_add(struct acpi_device *device)
};
MODULE_DEVICE_TABLE(acpi, vmbus_acpi_device_ids);
+static const struct dev_pm_ops vmbus_bus_pm = {
+ SET_SYSTEM_SLEEP_PM_OPS(vmbus_bus_suspend, vmbus_bus_resume)
+};
+
static struct acpi_driver vmbus_acpi_driver = {
.name = "vmbus",
.ids = vmbus_acpi_device_ids,
@@ -2059,6 +2098,7 @@ static int vmbus_acpi_add(struct acpi_device *device)
.add = vmbus_acpi_add,
.remove = vmbus_acpi_remove,
},
+ .drv.pm = &vmbus_bus_pm,
};
static void hv_kexec_handler(void)
--
1.8.3.1
^ permalink raw reply related [flat|nested] 17+ messages in thread
* RE: [PATCH 6/7] Drivers: hv: vmbus: Suspend/resume the vmbus itself for hibernation
2019-07-09 5:29 ` [PATCH 6/7] Drivers: hv: vmbus: Suspend/resume the vmbus itself for hibernation Dexuan Cui
@ 2019-07-30 23:25 ` Michael Kelley
2019-07-31 0:16 ` Dexuan Cui
0 siblings, 1 reply; 17+ messages in thread
From: Michael Kelley @ 2019-07-30 23:25 UTC (permalink / raw)
To: Dexuan Cui, linux-hyperv, gregkh, Stephen Hemminger, Sasha Levin,
sashal, Haiyang Zhang, KY Srinivasan, tglx
Cc: linux-kernel
From: Dexuan Cui <decui@microsoft.com> Sent: Monday, July 8, 2019 10:30 PM
>
> This is needed when we resume the old kernel from the "current" kernel.
Perhaps a bit more descriptive commit message could be supplied?
>
> Signed-off-by: Dexuan Cui <decui@microsoft.com>
> ---
> drivers/hv/connection.c | 3 +--
> drivers/hv/hyperv_vmbus.h | 2 ++
> drivers/hv/vmbus_drv.c | 40 ++++++++++++++++++++++++++++++++++++++++
> 3 files changed, 43 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/hv/connection.c b/drivers/hv/connection.c
> index 09829e1..806319c 100644
> --- a/drivers/hv/connection.c
> +++ b/drivers/hv/connection.c
> @@ -59,8 +59,7 @@ static __u32 vmbus_get_next_version(__u32 current_version)
> }
> }
>
> -static int vmbus_negotiate_version(struct vmbus_channel_msginfo *msginfo,
> - __u32 version)
> +int vmbus_negotiate_version(struct vmbus_channel_msginfo *msginfo, u32 version)
> {
> int ret = 0;
> unsigned int cur_cpu;
> diff --git a/drivers/hv/hyperv_vmbus.h b/drivers/hv/hyperv_vmbus.h
> index 26ea161..e657197 100644
> --- a/drivers/hv/hyperv_vmbus.h
> +++ b/drivers/hv/hyperv_vmbus.h
> @@ -274,6 +274,8 @@ struct vmbus_msginfo {
>
> extern struct vmbus_connection vmbus_connection;
>
> +int vmbus_negotiate_version(struct vmbus_channel_msginfo *msginfo, u32 version);
> +
> static inline void vmbus_send_interrupt(u32 relid)
> {
> sync_set_bit(relid, vmbus_connection.send_int_page);
> diff --git a/drivers/hv/vmbus_drv.c b/drivers/hv/vmbus_drv.c
> index 1c2d935..1730e7b 100644
> --- a/drivers/hv/vmbus_drv.c
> +++ b/drivers/hv/vmbus_drv.c
> @@ -2045,6 +2045,41 @@ static int vmbus_acpi_add(struct acpi_device *device)
> return ret_val;
> }
>
> +static int vmbus_bus_suspend(struct device *dev)
> +{
> + vmbus_initiate_unload(false);
> +
> + vmbus_connection.conn_state = DISCONNECTED;
> +
> + return 0;
> +}
> +
> +static int vmbus_bus_resume(struct device *dev)
> +{
> + struct vmbus_channel_msginfo *msginfo;
> + size_t msgsize;
> + int ret;
> +
> + msgsize = sizeof(*msginfo) +
> + sizeof(struct vmbus_channel_initiate_contact);
> +
> + msginfo = kzalloc(msgsize, GFP_KERNEL);
> +
> + if (msginfo == NULL)
> + return -ENOMEM;
> +
> + ret = vmbus_negotiate_version(msginfo, vmbus_proto_version);
I think this code answers my earlier question: Upon resume, we negotiate the same
VMbus protocol version we had at time of hibernation, even if running on a newer
version of Hyper-V that might support newer protocol versions. Hence the offer
messages should not have any previously reserved fields now being used. A
comment to this effect would be useful.
> +
> + kfree(msginfo);
> +
> + if (ret != 0)
> + return ret;
> +
> + vmbus_request_offers();
> +
> + return 0;
> +}
> +
> static const struct acpi_device_id vmbus_acpi_device_ids[] = {
> {"VMBUS", 0},
> {"VMBus", 0},
> @@ -2052,6 +2087,10 @@ static int vmbus_acpi_add(struct acpi_device *device)
> };
> MODULE_DEVICE_TABLE(acpi, vmbus_acpi_device_ids);
>
> +static const struct dev_pm_ops vmbus_bus_pm = {
> + SET_SYSTEM_SLEEP_PM_OPS(vmbus_bus_suspend, vmbus_bus_resume)
> +};
> +
> static struct acpi_driver vmbus_acpi_driver = {
> .name = "vmbus",
> .ids = vmbus_acpi_device_ids,
> @@ -2059,6 +2098,7 @@ static int vmbus_acpi_add(struct acpi_device *device)
> .add = vmbus_acpi_add,
> .remove = vmbus_acpi_remove,
> },
> + .drv.pm = &vmbus_bus_pm,
> };
>
> static void hv_kexec_handler(void)
> --
> 1.8.3.1
^ permalink raw reply [flat|nested] 17+ messages in thread
* RE: [PATCH 6/7] Drivers: hv: vmbus: Suspend/resume the vmbus itself for hibernation
2019-07-30 23:25 ` Michael Kelley
@ 2019-07-31 0:16 ` Dexuan Cui
0 siblings, 0 replies; 17+ messages in thread
From: Dexuan Cui @ 2019-07-31 0:16 UTC (permalink / raw)
To: Michael Kelley, linux-hyperv, gregkh, Stephen Hemminger,
Sasha Levin, sashal, Haiyang Zhang, KY Srinivasan, tglx
Cc: linux-kernel
> From: Michael Kelley <mikelley@microsoft.com>
> Sent: Tuesday, July 30, 2019 4:26 PM
> From: Dexuan Cui <decui@microsoft.com> Sent: Monday, July 8, 2019 10:30
> PM
> >
> > This is needed when we resume the old kernel from the "current" kernel.
>
> Perhaps a bit more descriptive commit message could be supplied?
I'll use this as the new changelog:
Before Linux enters hibernation, it sends the CHANNELMSG_UNLOAD message to
the host so all the offers are gone. After hibernation, Linux needs to re-negotiate
with the host using the same vmbus protocol version (which was in use
before hibernation), and ask the host to re-offer the vmbus devices.
> > +
> > + ret = vmbus_negotiate_version(msginfo, vmbus_proto_version);
>
> I think this code answers my earlier question: Upon resume, we negotiate
> the same
> VMbus protocol version we had at time of hibernation, even if running on a
> newer
> version of Hyper-V that might support newer protocol versions. Hence the
> offer
> messages should not have any previously reserved fields now being used. A
> comment to this effect would be useful.
Ok, let me add a comment before the line.
I'll post a v2 of the patchset, including the "[PATCH 4/7]", which needs a small change.
Thanks,
-- Dexuan
^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH 7/7] Drivers: hv: vmbus: Implement suspend/resume for VSC drivers for hibernation
2019-07-09 5:29 [PATCH 0/7] Enhance the hv_vmbus driver to support hibernation Dexuan Cui
` (5 preceding siblings ...)
2019-07-09 5:29 ` [PATCH 6/7] Drivers: hv: vmbus: Suspend/resume the vmbus itself for hibernation Dexuan Cui
@ 2019-07-09 5:29 ` Dexuan Cui
2019-07-30 23:38 ` Michael Kelley
6 siblings, 1 reply; 17+ messages in thread
From: Dexuan Cui @ 2019-07-09 5:29 UTC (permalink / raw)
To: linux-hyperv, gregkh, Stephen Hemminger, Sasha Levin, sashal,
Haiyang Zhang, KY Srinivasan, Michael Kelley, tglx
Cc: linux-kernel, Dexuan Cui
The high-level VSC drivers will implement device-specific callbacks.
Signed-off-by: Dexuan Cui <decui@microsoft.com>
---
drivers/hv/vmbus_drv.c | 42 ++++++++++++++++++++++++++++++++++++++++++
include/linux/hyperv.h | 3 +++
2 files changed, 45 insertions(+)
diff --git a/drivers/hv/vmbus_drv.c b/drivers/hv/vmbus_drv.c
index 1730e7b..e29e2171 100644
--- a/drivers/hv/vmbus_drv.c
+++ b/drivers/hv/vmbus_drv.c
@@ -911,6 +911,43 @@ static void vmbus_shutdown(struct device *child_device)
drv->shutdown(dev);
}
+/*
+ * vmbus_suspend - Suspend a vmbus device
+ */
+static int vmbus_suspend(struct device *child_device)
+{
+ struct hv_driver *drv;
+ struct hv_device *dev = device_to_hv_device(child_device);
+
+ /* The device may not be attached yet */
+ if (!child_device->driver)
+ return 0;
+
+ drv = drv_to_hv_drv(child_device->driver);
+ if (!drv->suspend)
+ return -EOPNOTSUPP;
+
+ return drv->suspend(dev);
+}
+
+/*
+ * vmbus_resume - Resume a vmbus device
+ */
+static int vmbus_resume(struct device *child_device)
+{
+ struct hv_driver *drv;
+ struct hv_device *dev = device_to_hv_device(child_device);
+
+ /* The device may not be attached yet */
+ if (!child_device->driver)
+ return 0;
+
+ drv = drv_to_hv_drv(child_device->driver);
+ if (!drv->resume)
+ return -EOPNOTSUPP;
+
+ return drv->resume(dev);
+}
/*
* vmbus_device_release - Final callback release of the vmbus child device
@@ -926,6 +963,10 @@ static void vmbus_device_release(struct device *device)
kfree(hv_dev);
}
+static const struct dev_pm_ops vmbus_pm = {
+ SET_SYSTEM_SLEEP_PM_OPS(vmbus_suspend, vmbus_resume)
+};
+
/* The one and only one */
static struct bus_type hv_bus = {
.name = "vmbus",
@@ -936,6 +977,7 @@ static void vmbus_device_release(struct device *device)
.uevent = vmbus_uevent,
.dev_groups = vmbus_dev_groups,
.drv_groups = vmbus_drv_groups,
+ .pm = &vmbus_pm,
};
struct onmessage_work_context {
diff --git a/include/linux/hyperv.h b/include/linux/hyperv.h
index 6256cc3..94443c4 100644
--- a/include/linux/hyperv.h
+++ b/include/linux/hyperv.h
@@ -1149,6 +1149,9 @@ struct hv_driver {
int (*remove)(struct hv_device *);
void (*shutdown)(struct hv_device *);
+ int (*suspend)(struct hv_device *);
+ int (*resume)(struct hv_device *);
+
};
/* Base device object */
--
1.8.3.1
^ permalink raw reply related [flat|nested] 17+ messages in thread
* RE: [PATCH 7/7] Drivers: hv: vmbus: Implement suspend/resume for VSC drivers for hibernation
2019-07-09 5:29 ` [PATCH 7/7] Drivers: hv: vmbus: Implement suspend/resume for VSC drivers " Dexuan Cui
@ 2019-07-30 23:38 ` Michael Kelley
0 siblings, 0 replies; 17+ messages in thread
From: Michael Kelley @ 2019-07-30 23:38 UTC (permalink / raw)
To: Dexuan Cui, linux-hyperv, gregkh, Stephen Hemminger, Sasha Levin,
sashal, Haiyang Zhang, KY Srinivasan, tglx
Cc: linux-kernel
From: Dexuan Cui <decui@microsoft.com> Sent: Monday, July 8, 2019 10:30 PM
>
> The high-level VSC drivers will implement device-specific callbacks.
>
> Signed-off-by: Dexuan Cui <decui@microsoft.com>
> ---
> drivers/hv/vmbus_drv.c | 42 ++++++++++++++++++++++++++++++++++++++++++
> include/linux/hyperv.h | 3 +++
> 2 files changed, 45 insertions(+)
>
> diff --git a/drivers/hv/vmbus_drv.c b/drivers/hv/vmbus_drv.c
> index 1730e7b..e29e2171 100644
> --- a/drivers/hv/vmbus_drv.c
> +++ b/drivers/hv/vmbus_drv.c
> @@ -911,6 +911,43 @@ static void vmbus_shutdown(struct device *child_device)
> drv->shutdown(dev);
> }
>
> +/*
> + * vmbus_suspend - Suspend a vmbus device
> + */
> +static int vmbus_suspend(struct device *child_device)
> +{
> + struct hv_driver *drv;
> + struct hv_device *dev = device_to_hv_device(child_device);
> +
> + /* The device may not be attached yet */
> + if (!child_device->driver)
> + return 0;
> +
> + drv = drv_to_hv_drv(child_device->driver);
> + if (!drv->suspend)
> + return -EOPNOTSUPP;
> +
> + return drv->suspend(dev);
> +}
> +
> +/*
> + * vmbus_resume - Resume a vmbus device
> + */
> +static int vmbus_resume(struct device *child_device)
> +{
> + struct hv_driver *drv;
> + struct hv_device *dev = device_to_hv_device(child_device);
> +
> + /* The device may not be attached yet */
> + if (!child_device->driver)
> + return 0;
> +
> + drv = drv_to_hv_drv(child_device->driver);
> + if (!drv->resume)
> + return -EOPNOTSUPP;
> +
> + return drv->resume(dev);
> +}
>
> /*
> * vmbus_device_release - Final callback release of the vmbus child device
> @@ -926,6 +963,10 @@ static void vmbus_device_release(struct device *device)
> kfree(hv_dev);
> }
>
> +static const struct dev_pm_ops vmbus_pm = {
> + SET_SYSTEM_SLEEP_PM_OPS(vmbus_suspend, vmbus_resume)
> +};
> +
> /* The one and only one */
> static struct bus_type hv_bus = {
> .name = "vmbus",
> @@ -936,6 +977,7 @@ static void vmbus_device_release(struct device *device)
> .uevent = vmbus_uevent,
> .dev_groups = vmbus_dev_groups,
> .drv_groups = vmbus_drv_groups,
> + .pm = &vmbus_pm,
> };
>
> struct onmessage_work_context {
> diff --git a/include/linux/hyperv.h b/include/linux/hyperv.h
> index 6256cc3..94443c4 100644
> --- a/include/linux/hyperv.h
> +++ b/include/linux/hyperv.h
> @@ -1149,6 +1149,9 @@ struct hv_driver {
> int (*remove)(struct hv_device *);
> void (*shutdown)(struct hv_device *);
>
> + int (*suspend)(struct hv_device *);
> + int (*resume)(struct hv_device *);
> +
> };
>
> /* Base device object */
> --
> 1.8.3.1
Reviewed-by: Michael Kelley <mikelley@microsoft.com>
^ permalink raw reply [flat|nested] 17+ messages in thread