[PATCH V3 0/6] x86/Hyper-V: Panic code path fixes

[PATCH V3 0/6] x86/Hyper-V: Panic code path fixes

[ltykernel]

From: Tianyu Lan <Tianyu.Lan@microsoft.com>

This patchset fixes some issues in the Hyper-V panic code path.
Patch 1 resolves issue that panic system still responses network
packets.
Patch 2-3,5-6 resolves crash enlightenment issues.
Patch 4 is to set crash_kexec_post_notifiers to true for Hyper-V
VM in order to report crash data or kmsg to host before running
kdump kernel.

Tianyu Lan (6):
  x86/Hyper-V: Unload vmbus channel in hv panic callback
  x86/Hyper-V: Free hv_panic_page when fail to register kmsg dump
  x86/Hyper-V: Trigger crash enlightenment only once during  system
    crash.
  x86/Hyper-V: Report crash register data or ksmg before  running crash
    kernel
  x86/Hyper-V: Report crash register data when sysctl_record_panic_msg
    is not set
  x86/Hyper-V: Report crash data in die() when panic_on_oops is set

 arch/x86/kernel/cpu/mshyperv.c | 10 +++++++
 drivers/hv/channel_mgmt.c      |  3 +++
 drivers/hv/vmbus_drv.c         | 61 +++++++++++++++++++++++++++++-------------
 3 files changed, 56 insertions(+), 18 deletions(-)

-- 
2.14.5

[PATCH V3 1/6] x86/Hyper-V: Unload vmbus channel in hv panic callback

[ltykernel]

From: Tianyu Lan <Tianyu.Lan@microsoft.com>

When kdump is not configured, a Hyper-V VM might still respond to
network traffic after a kernel panic when kernel parameter panic=0.
The panic CPU goes into an infinite loop with interrupts enabled,
and the VMbus driver interrupt handler still works because the
VMbus connection is unloaded only in the kdump path.  The network
responses make the other end of the connection think the VM is
still functional even though it has panic'ed, which could affect any
failover actions that should be taken.

Fix this by unloading the VMbus connection during the panic process.
vmbus_initiate_unload() could then be called twice (e.g., by
hyperv_panic_event() and hv_crash_handler(), so reset the connection
state in vmbus_initiate_unload() to ensure the unload is done only
once.

Signed-off-by: Tianyu Lan <Tianyu.Lan@microsoft.com>
---
Change since v1:
	- Update change log
	- Use xchg() to change vmbus connection status
Change since v2:
	- Update comment of registering panic callback.
---
 drivers/hv/channel_mgmt.c |  3 +++
 drivers/hv/vmbus_drv.c    | 21 +++++++++++++--------
 2 files changed, 16 insertions(+), 8 deletions(-)

diff --git a/drivers/hv/channel_mgmt.c b/drivers/hv/channel_mgmt.c
index 0370364169c4..501c43c5851d 100644
--- a/drivers/hv/channel_mgmt.c
+++ b/drivers/hv/channel_mgmt.c
@@ -839,6 +839,9 @@ void vmbus_initiate_unload(bool crash)
 {
 	struct vmbus_channel_message_header hdr;
 
+	if (xchg(&vmbus_connection.conn_state, DISCONNECTED) == DISCONNECTED)
+		return;
+
 	/* Pre-Win2012R2 hosts don't support reconnect */
 	if (vmbus_proto_version < VERSION_WIN8_1)
 		return;
diff --git a/drivers/hv/vmbus_drv.c b/drivers/hv/vmbus_drv.c
index 029378c27421..6478240d11ab 100644
--- a/drivers/hv/vmbus_drv.c
+++ b/drivers/hv/vmbus_drv.c
@@ -53,9 +53,12 @@ static int hyperv_panic_event(struct notifier_block *nb, unsigned long val,
 {
 	struct pt_regs *regs;
 
-	regs = current_pt_regs();
+	vmbus_initiate_unload(true);
 
-	hyperv_report_panic(regs, val);
+	if (ms_hyperv.misc_features & HV_FEATURE_GUEST_CRASH_MSR_AVAILABLE) {
+		regs = current_pt_regs();
+		hyperv_report_panic(regs, val);
+	}
 	return NOTIFY_DONE;
 }
 
@@ -1391,10 +1394,16 @@ static int vmbus_bus_init(void)
 		}
 
 		register_die_notifier(&hyperv_die_block);
-		atomic_notifier_chain_register(&panic_notifier_list,
-					       &hyperv_panic_block);
 	}
 
+	/*
+	 * Always register the panic notifier because we need to unload
+	 * the VMbus channel connection to prevent any VMbus
+	 * activity after the VM panics.
+	 */
+	atomic_notifier_chain_register(&panic_notifier_list,
+			       &hyperv_panic_block);
+
 	vmbus_request_offers();
 
 	return 0;
@@ -2204,8 +2213,6 @@ static int vmbus_bus_suspend(struct device *dev)
 
 	vmbus_initiate_unload(false);
 
-	vmbus_connection.conn_state = DISCONNECTED;
-
 	/* Reset the event for the next resume. */
 	reinit_completion(&vmbus_connection.ready_for_resume_event);
 
@@ -2289,7 +2296,6 @@ static void hv_kexec_handler(void)
 {
 	hv_stimer_global_cleanup();
 	vmbus_initiate_unload(false);
-	vmbus_connection.conn_state = DISCONNECTED;
 	/* Make sure conn_state is set as hv_synic_cleanup checks for it */
 	mb();
 	cpuhp_remove_state(hyperv_cpuhp_online);
@@ -2306,7 +2312,6 @@ static void hv_crash_handler(struct pt_regs *regs)
 	 * doing the cleanup for current CPU only. This should be sufficient
 	 * for kdump.
 	 */
-	vmbus_connection.conn_state = DISCONNECTED;
 	cpu = smp_processor_id();
 	hv_stimer_cleanup(cpu);
 	hv_synic_disable_regs(cpu);
-- 
2.14.5

RE: [PATCH V3 1/6] x86/Hyper-V: Unload vmbus channel in hv panic callback

[Michael Kelley]

From: Tianyu Lan <Tianyu.Lan@microsoft.com> Sent: Tuesday, March 24, 2020 12:57 AM
> 
> When kdump is not configured, a Hyper-V VM might still respond to
> network traffic after a kernel panic when kernel parameter panic=0.
> The panic CPU goes into an infinite loop with interrupts enabled,
> and the VMbus driver interrupt handler still works because the
> VMbus connection is unloaded only in the kdump path.  The network
> responses make the other end of the connection think the VM is
> still functional even though it has panic'ed, which could affect any
> failover actions that should be taken.
> 
> Fix this by unloading the VMbus connection during the panic process.
> vmbus_initiate_unload() could then be called twice (e.g., by
> hyperv_panic_event() and hv_crash_handler(), so reset the connection
> state in vmbus_initiate_unload() to ensure the unload is done only
> once.
> 
> Signed-off-by: Tianyu Lan <Tianyu.Lan@microsoft.com>
> ---
> Change since v1:
> 	- Update change log
> 	- Use xchg() to change vmbus connection status
> Change since v2:
> 	- Update comment of registering panic callback.
> ---
>  drivers/hv/channel_mgmt.c |  3 +++
>  drivers/hv/vmbus_drv.c    | 21 +++++++++++++--------
>  2 files changed, 16 insertions(+), 8 deletions(-)
> 

Reviewed-by: Michael Kelley <mikelley@microsoft.com>

[PATCH V3 2/6] x86/Hyper-V: Free hv_panic_page when fail to register kmsg dump

[ltykernel]

From: Tianyu Lan <Tianyu.Lan@microsoft.com>

If kmsg_dump_register() fails, hv_panic_page will not be used
anywhere.  So free and reset it.

Signed-off-by: Tianyu Lan <Tianyu.Lan@microsoft.com>
---
Change since v1:
	- Update commit log
	- Remove hv_free_hyperv_page() in the error path
---
 drivers/hv/vmbus_drv.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/drivers/hv/vmbus_drv.c b/drivers/hv/vmbus_drv.c
index 6478240d11ab..00a511f15926 100644
--- a/drivers/hv/vmbus_drv.c
+++ b/drivers/hv/vmbus_drv.c
@@ -1385,9 +1385,13 @@ static int vmbus_bus_init(void)
 			hv_panic_page = (void *)hv_alloc_hyperv_zeroed_page();
 			if (hv_panic_page) {
 				ret = kmsg_dump_register(&hv_kmsg_dumper);
-				if (ret)
+				if (ret) {
 					pr_err("Hyper-V: kmsg dump register "
 						"error 0x%x\n", ret);
+					hv_free_hyperv_page(
+					    (unsigned long)hv_panic_page);
+					hv_panic_page = NULL;
+				}
 			} else
 				pr_err("Hyper-V: panic message page memory "
 					"allocation failed");
@@ -1416,7 +1420,6 @@ static int vmbus_bus_init(void)
 	hv_remove_vmbus_irq();
 
 	bus_unregister(&hv_bus);
-	hv_free_hyperv_page((unsigned long)hv_panic_page);
 	unregister_sysctl_table(hv_ctl_table_hdr);
 	hv_ctl_table_hdr = NULL;
 	return ret;
-- 
2.14.5

RE: [PATCH V3 2/6] x86/Hyper-V: Free hv_panic_page when fail to register kmsg dump

[Michael Kelley]

From: Tianyu Lan <Tianyu.Lan@microsoft.com> Sent: Tuesday, March 24, 2020 12:57 AM
>
> If kmsg_dump_register() fails, hv_panic_page will not be used
> anywhere.  So free and reset it.
> 
> Signed-off-by: Tianyu Lan <Tianyu.Lan@microsoft.com>
> ---
> Change since v1:
> 	- Update commit log
> 	- Remove hv_free_hyperv_page() in the error path
> ---
>  drivers/hv/vmbus_drv.c | 7 +++++--
>  1 file changed, 5 insertions(+), 2 deletions(-)
> 

Reviewed-by: Michael Kelley <mikelley@microsoft.com>

[PATCH V3 3/6] x86/Hyper-V: Trigger crash enlightenment only once during system crash.

[ltykernel]

From: Tianyu Lan <Tianyu.Lan@microsoft.com>

When a guest VM panics, Hyper-V should be notified only once via the
crash synthetic MSRs.  Current Linux code might write these crash MSRs
twice during a system panic:
1) hyperv_panic/die_event() calling hyperv_report_panic()
2) hv_kmsg_dump() calling hyperv_report_panic_msg()

Fix this by not calling hyperv_report_panic() if a kmsg dump has been
successfully registered.  The notification will happen later via
hyperv_report_panic_msg().

Signed-off-by: Tianyu Lan <Tianyu.Lan@microsoft.com>
---
Change since v1:
	- Update commit log

Change since v2:
        - Update comment
---
 drivers/hv/vmbus_drv.c | 16 ++++++++++++++--
 1 file changed, 14 insertions(+), 2 deletions(-)

diff --git a/drivers/hv/vmbus_drv.c b/drivers/hv/vmbus_drv.c
index 00a511f15926..333dad39b1c1 100644
--- a/drivers/hv/vmbus_drv.c
+++ b/drivers/hv/vmbus_drv.c
@@ -55,7 +55,13 @@ static int hyperv_panic_event(struct notifier_block *nb, unsigned long val,
 
 	vmbus_initiate_unload(true);
 
-	if (ms_hyperv.misc_features & HV_FEATURE_GUEST_CRASH_MSR_AVAILABLE) {
+	/*
+	 * Hyper-V should be notified only once about a panic.  If we will be
+	 * doing hyperv_report_panic_msg() later with kmsg data, don't do
+	 * the notification here.
+	 */
+	if (ms_hyperv.misc_features & HV_FEATURE_GUEST_CRASH_MSR_AVAILABLE
+	    && !hv_panic_page) {
 		regs = current_pt_regs();
 		hyperv_report_panic(regs, val);
 	}
@@ -68,7 +74,13 @@ static int hyperv_die_event(struct notifier_block *nb, unsigned long val,
 	struct die_args *die = (struct die_args *)args;
 	struct pt_regs *regs = die->regs;
 
-	hyperv_report_panic(regs, val);
+	/*
+	 * Hyper-V should be notified only once about a panic.  If we will be
+	 * doing hyperv_report_panic_msg() later with kmsg data, don't do
+	 * the notification here.
+	 */
+	if (!hv_panic_page)
+		hyperv_report_panic(regs, val);
 	return NOTIFY_DONE;
 }
 
-- 
2.14.5

RE: [PATCH V3 3/6] x86/Hyper-V: Trigger crash enlightenment only once during system crash.

[Michael Kelley]

From: Tianyu Lan <Tianyu.Lan@microsoft.com> Sent: Tuesday, March 24, 2020 12:57 AM
> 
> When a guest VM panics, Hyper-V should be notified only once via the
> crash synthetic MSRs.  Current Linux code might write these crash MSRs
> twice during a system panic:
> 1) hyperv_panic/die_event() calling hyperv_report_panic()
> 2) hv_kmsg_dump() calling hyperv_report_panic_msg()
> 
> Fix this by not calling hyperv_report_panic() if a kmsg dump has been
> successfully registered.  The notification will happen later via
> hyperv_report_panic_msg().
> 
> Signed-off-by: Tianyu Lan <Tianyu.Lan@microsoft.com>
> ---
> Change since v1:
> 	- Update commit log
> 
> Change since v2:
>         - Update comment
> ---
>  drivers/hv/vmbus_drv.c | 16 ++++++++++++++--
>  1 file changed, 14 insertions(+), 2 deletions(-)
> 

Reviewed-by: Michael Kelley <mikelley@microsoft.com>

[PATCH V3 4/6] x86/Hyper-V: Report crash register data or ksmg before running crash kernel

[ltykernel]

From: Tianyu Lan <Tianyu.Lan@microsoft.com>

We want to notify Hyper-V when a Linux guest VM crash occurs, so
there is a record of the crash even when kdump is enabled.   But
crash_kexec_post_notifiers defaults to "false", so the kdump kernel
runs before the notifiers and Hyper-V never gets notified.  Fix this by
always setting crash_kexec_post_notifiers to be true for Hyper-V VMs.

Signed-off-by: Tianyu Lan <Tianyu.Lan@microsoft.com>
---
Change since v1:
       Update commit log
---
 arch/x86/kernel/cpu/mshyperv.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/arch/x86/kernel/cpu/mshyperv.c b/arch/x86/kernel/cpu/mshyperv.c
index caa032ce3fe3..5e296a7e6036 100644
--- a/arch/x86/kernel/cpu/mshyperv.c
+++ b/arch/x86/kernel/cpu/mshyperv.c
@@ -263,6 +263,16 @@ static void __init ms_hyperv_init_platform(void)
 			cpuid_eax(HYPERV_CPUID_NESTED_FEATURES);
 	}
 
+	/*
+	 * Hyper-V expects to get crash register data or kmsg when
+	 * crash enlightment is available and system crashes. Set
+	 * crash_kexec_post_notifiers to be true to make sure that
+	 * calling crash enlightment interface before running kdump
+	 * kernel.
+	 */
+	if (ms_hyperv.misc_features & HV_FEATURE_GUEST_CRASH_MSR_AVAILABLE)
+		crash_kexec_post_notifiers = true;
+
 #ifdef CONFIG_X86_LOCAL_APIC
 	if (ms_hyperv.features & HV_X64_ACCESS_FREQUENCY_MSRS &&
 	    ms_hyperv.misc_features & HV_FEATURE_FREQUENCY_MSRS_AVAILABLE) {
-- 
2.14.5

RE: [PATCH V3 4/6] x86/Hyper-V: Report crash register data or ksmg before running crash kernel

[Michael Kelley]

From: Tianyu Lan <Tianyu.Lan@microsoft.com> Sent: Tuesday, March 24, 2020 12:57 AM
> 
> We want to notify Hyper-V when a Linux guest VM crash occurs, so
> there is a record of the crash even when kdump is enabled.   But
> crash_kexec_post_notifiers defaults to "false", so the kdump kernel
> runs before the notifiers and Hyper-V never gets notified.  Fix this by
> always setting crash_kexec_post_notifiers to be true for Hyper-V VMs.
> 
> Signed-off-by: Tianyu Lan <Tianyu.Lan@microsoft.com>
> ---
> Change since v1:
>        Update commit log
> ---
>  arch/x86/kernel/cpu/mshyperv.c | 10 ++++++++++
>  1 file changed, 10 insertions(+)
> 

Reviewed-by: Michael Kelley <mikelley@microsoft.com>

[PATCH V3 5/6] x86/Hyper-V: Report crash register data when sysctl_record_panic_msg is not set

[ltykernel]

From: Tianyu Lan <Tianyu.Lan@microsoft.com>

When sysctl_record_panic_msg is not set, the panic will
not be reported to Hyper-V via hyperv_report_panic_msg().
So the crash should be reported via hyperv_report_panic().

Signed-off-by: Tianyu Lan <Tianyu.Lan@microsoft.com>
---
 drivers/hv/vmbus_drv.c | 23 ++++++++++++++---------
 1 file changed, 14 insertions(+), 9 deletions(-)

diff --git a/drivers/hv/vmbus_drv.c b/drivers/hv/vmbus_drv.c
index 333dad39b1c1..172ceae69abb 100644
--- a/drivers/hv/vmbus_drv.c
+++ b/drivers/hv/vmbus_drv.c
@@ -48,6 +48,18 @@ static int hyperv_cpuhp_online;
 
 static void *hv_panic_page;
 
+/*
+ * Boolean to control whether to report panic messages over Hyper-V.
+ *
+ * It can be set via /proc/sys/kernel/hyperv/record_panic_msg
+ */
+static int sysctl_record_panic_msg = 1;
+
+static int hyperv_report_reg(void)
+{
+	return !sysctl_record_panic_msg || !hv_panic_page;
+}
+
 static int hyperv_panic_event(struct notifier_block *nb, unsigned long val,
 			      void *args)
 {
@@ -61,7 +73,7 @@ static int hyperv_panic_event(struct notifier_block *nb, unsigned long val,
 	 * the notification here.
 	 */
 	if (ms_hyperv.misc_features & HV_FEATURE_GUEST_CRASH_MSR_AVAILABLE
-	    && !hv_panic_page) {
+	    && hyperv_report_reg()) {
 		regs = current_pt_regs();
 		hyperv_report_panic(regs, val);
 	}
@@ -79,7 +91,7 @@ static int hyperv_die_event(struct notifier_block *nb, unsigned long val,
 	 * doing hyperv_report_panic_msg() later with kmsg data, don't do
 	 * the notification here.
 	 */
-	if (!hv_panic_page)
+	if (hyperv_report_reg())
 		hyperv_report_panic(regs, val);
 	return NOTIFY_DONE;
 }
@@ -1267,13 +1279,6 @@ static void vmbus_isr(void)
 	add_interrupt_randomness(HYPERVISOR_CALLBACK_VECTOR, 0);
 }
 
-/*
- * Boolean to control whether to report panic messages over Hyper-V.
- *
- * It can be set via /proc/sys/kernel/hyperv/record_panic_msg
- */
-static int sysctl_record_panic_msg = 1;
-
 /*
  * Callback from kmsg_dump. Grab as much as possible from the end of the kmsg
  * buffer and call into Hyper-V to transfer the data.
-- 
2.14.5

RE: [PATCH V3 5/6] x86/Hyper-V: Report crash register data when sysctl_record_panic_msg is not set

[Michael Kelley]

From: Tianyu Lan <Tianyu.Lan@microsoft.com> Sent: Tuesday, March 24, 2020 12:57 AM
> 
> When sysctl_record_panic_msg is not set, the panic will
> not be reported to Hyper-V via hyperv_report_panic_msg().
> So the crash should be reported via hyperv_report_panic().
> 
> Signed-off-by: Tianyu Lan <Tianyu.Lan@microsoft.com>
> ---
>  drivers/hv/vmbus_drv.c | 23 ++++++++++++++---------
>  1 file changed, 14 insertions(+), 9 deletions(-)
> 

Reviewed-by: Michael Kelley <mikelley@microsoft.com>

[PATCH V3 6/6] x86/Hyper-V: Report crash data in die() when panic_on_oops is set

[ltykernel]

From: Tianyu Lan <Tianyu.Lan@microsoft.com>

When oops happens with panic_on_oops unset, the oops
thread is killed by die() and system continues to run.
In such case, guest should not report crash register
data to host since system still runs. Fix it.

Signed-off-by: Tianyu Lan <Tianyu.Lan@microsoft.com>
---
 drivers/hv/vmbus_drv.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/hv/vmbus_drv.c b/drivers/hv/vmbus_drv.c
index 172ceae69abb..8baca2881596 100644
--- a/drivers/hv/vmbus_drv.c
+++ b/drivers/hv/vmbus_drv.c
@@ -91,7 +91,7 @@ static int hyperv_die_event(struct notifier_block *nb, unsigned long val,
 	 * doing hyperv_report_panic_msg() later with kmsg data, don't do
 	 * the notification here.
 	 */
-	if (hyperv_report_reg())
+	if (hyperv_report_reg() && panic_on_oops)
 		hyperv_report_panic(regs, val);
 	return NOTIFY_DONE;
 }
-- 
2.14.5

RE: [PATCH V3 6/6] x86/Hyper-V: Report crash data in die() when panic_on_oops is set

[Michael Kelley]

From: Tianyu Lan <Tianyu.Lan@microsoft.com> Sent: Tuesday, March 24, 2020 12:57 AM
> 
> When oops happens with panic_on_oops unset, the oops
> thread is killed by die() and system continues to run.
> In such case, guest should not report crash register
> data to host since system still runs. Fix it.
> 
> Signed-off-by: Tianyu Lan <Tianyu.Lan@microsoft.com>
> ---
>  drivers/hv/vmbus_drv.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 

Reviewed-by: Michael Kelley <mikelley@microsoft.com>

[PATCH V4 6/6] x86/Hyper-V: Report crash data in die() when panic_on_oops is set

[ltykernel]

From: Tianyu Lan <Tianyu.Lan@microsoft.com>

When oops happens with panic_on_oops unset, the oops
thread is killed by die() and system continues to run.
In such case, guest should not report crash register
data to host since system still runs. Fix it.

Reviewed-by: Michael Kelley <mikelley@microsoft.com>
Signed-off-by: Tianyu Lan <Tianyu.Lan@microsoft.com>
---
Change since v3:
	Fix compile error
---
 drivers/hv/vmbus_drv.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/hv/vmbus_drv.c b/drivers/hv/vmbus_drv.c
index 172ceae69abb..4bc02aea2098 100644
--- a/drivers/hv/vmbus_drv.c
+++ b/drivers/hv/vmbus_drv.c
@@ -31,6 +31,7 @@
 #include <linux/kdebug.h>
 #include <linux/efi.h>
 #include <linux/random.h>
+#include <linux/kernel.h>
 #include <linux/syscore_ops.h>
 #include <clocksource/hyperv_timer.h>
 #include "hyperv_vmbus.h"
@@ -91,7 +92,7 @@ static int hyperv_die_event(struct notifier_block *nb, unsigned long val,
 	 * doing hyperv_report_panic_msg() later with kmsg data, don't do
 	 * the notification here.
 	 */
-	if (hyperv_report_reg())
+	if (hyperv_report_reg() && panic_on_oops)
 		hyperv_report_panic(regs, val);
 	return NOTIFY_DONE;
 }
-- 
2.14.5

RE: [PATCH V4 6/6] x86/Hyper-V: Report crash data in die() when panic_on_oops is set

[Michael Kelley]

From: Tianyu Lan <Tianyu.Lan@microsoft.com> Sent: Tuesday, March 31, 2020 12:39 AM
> 
> When oops happens with panic_on_oops unset, the oops
> thread is killed by die() and system continues to run.
> In such case, guest should not report crash register
> data to host since system still runs. Fix it.
> 
> Reviewed-by: Michael Kelley <mikelley@microsoft.com>
> Signed-off-by: Tianyu Lan <Tianyu.Lan@microsoft.com>
> ---
> Change since v3:
> 	Fix compile error
> ---
>  drivers/hv/vmbus_drv.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/hv/vmbus_drv.c b/drivers/hv/vmbus_drv.c
> index 172ceae69abb..4bc02aea2098 100644
> --- a/drivers/hv/vmbus_drv.c
> +++ b/drivers/hv/vmbus_drv.c
> @@ -31,6 +31,7 @@
>  #include <linux/kdebug.h>
>  #include <linux/efi.h>
>  #include <linux/random.h>
> +#include <linux/kernel.h>

Unfortunately, adding the #include doesn't solve the problem.  The error occurs when
CONFIG_HYPERV=m, because panic_on_oops is not exported.  I haven't thought it
through, but hopefully there's a solution where panic_on_oops can be tested in
hyperv_report_panic() or some other Hyper-V specific function that's never in a
module, so that we don't need to export panic_on_oops.

Michael

>  #include <linux/syscore_ops.h>
>  #include <clocksource/hyperv_timer.h>
>  #include "hyperv_vmbus.h"
> @@ -91,7 +92,7 @@ static int hyperv_die_event(struct notifier_block *nb, unsigned long
> val,
>  	 * doing hyperv_report_panic_msg() later with kmsg data, don't do
>  	 * the notification here.
>  	 */
> -	if (hyperv_report_reg())
> +	if (hyperv_report_reg() && panic_on_oops)
>  		hyperv_report_panic(regs, val);
>  	return NOTIFY_DONE;
>  }
> --
> 2.14.5

Re: [PATCH V4 6/6] x86/Hyper-V: Report crash data in die() when panic_on_oops is set

[Tianyu Lan]

On 3/31/2020 9:51 PM, Michael Kelley wrote:
> From: Tianyu Lan <Tianyu.Lan@microsoft.com> Sent: Tuesday, March 31, 2020 12:39 AM
>>
>> When oops happens with panic_on_oops unset, the oops
>> thread is killed by die() and system continues to run.
>> In such case, guest should not report crash register
>> data to host since system still runs. Fix it.
>>
>> Reviewed-by: Michael Kelley <mikelley@microsoft.com>
>> Signed-off-by: Tianyu Lan <Tianyu.Lan@microsoft.com>
>> ---
>> Change since v3:
>> 	Fix compile error
>> ---
>>   drivers/hv/vmbus_drv.c | 3 ++-
>>   1 file changed, 2 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/hv/vmbus_drv.c b/drivers/hv/vmbus_drv.c
>> index 172ceae69abb..4bc02aea2098 100644
>> --- a/drivers/hv/vmbus_drv.c
>> +++ b/drivers/hv/vmbus_drv.c
>> @@ -31,6 +31,7 @@
>>   #include <linux/kdebug.h>
>>   #include <linux/efi.h>
>>   #include <linux/random.h>
>> +#include <linux/kernel.h>
> 
> Unfortunately, adding the #include doesn't solve the problem.  The error occurs when
> CONFIG_HYPERV=m, because panic_on_oops is not exported.  I haven't thought it
> through, but hopefully there's a solution where panic_on_oops can be tested in
> hyperv_report_panic() or some other Hyper-V specific function that's never in a
> module, so that we don't need to export panic_on_oops.

Yes, I don't consider modules case. I think we may introduce a check 
function of panic_on_oops in the mshyperv.c and expose it to module.

> 
> Michael
> 
>>   #include <linux/syscore_ops.h>
>>   #include <clocksource/hyperv_timer.h>
>>   #include "hyperv_vmbus.h"
>> @@ -91,7 +92,7 @@ static int hyperv_die_event(struct notifier_block *nb, unsigned long
>> val,
>>   	 * doing hyperv_report_panic_msg() later with kmsg data, don't do
>>   	 * the notification here.
>>   	 */
>> -	if (hyperv_report_reg())
>> +	if (hyperv_report_reg() && panic_on_oops)
>>   		hyperv_report_panic(regs, val);
>>   	return NOTIFY_DONE;
>>   }
>> --
>> 2.14.5
> 

Re: [PATCH V4 6/6] x86/Hyper-V: Report crash data in die() when panic_on_oops is set

[Wei Liu]

On Tue, Mar 31, 2020 at 10:26:06PM +0800, Tianyu Lan wrote:
> 
> 
> On 3/31/2020 9:51 PM, Michael Kelley wrote:
> > From: Tianyu Lan <Tianyu.Lan@microsoft.com> Sent: Tuesday, March 31, 2020 12:39 AM
> > > 
> > > When oops happens with panic_on_oops unset, the oops
> > > thread is killed by die() and system continues to run.
> > > In such case, guest should not report crash register
> > > data to host since system still runs. Fix it.
> > > 
> > > Reviewed-by: Michael Kelley <mikelley@microsoft.com>
> > > Signed-off-by: Tianyu Lan <Tianyu.Lan@microsoft.com>
> > > ---
> > > Change since v3:
> > > 	Fix compile error
> > > ---
> > >   drivers/hv/vmbus_drv.c | 3 ++-
> > >   1 file changed, 2 insertions(+), 1 deletion(-)
> > > 
> > > diff --git a/drivers/hv/vmbus_drv.c b/drivers/hv/vmbus_drv.c
> > > index 172ceae69abb..4bc02aea2098 100644
> > > --- a/drivers/hv/vmbus_drv.c
> > > +++ b/drivers/hv/vmbus_drv.c
> > > @@ -31,6 +31,7 @@
> > >   #include <linux/kdebug.h>
> > >   #include <linux/efi.h>
> > >   #include <linux/random.h>
> > > +#include <linux/kernel.h>
> > 
> > Unfortunately, adding the #include doesn't solve the problem.  The error occurs when
> > CONFIG_HYPERV=m, because panic_on_oops is not exported.  I haven't thought it
> > through, but hopefully there's a solution where panic_on_oops can be tested in
> > hyperv_report_panic() or some other Hyper-V specific function that's never in a
> > module, so that we don't need to export panic_on_oops.
> 
> Yes, I don't consider modules case. I think we may introduce a check
> function of panic_on_oops in the mshyperv.c and expose it to module.
> 

Why expose something new? You can just test panic_on_oops in
hyperv_report_panic and bail if it is false, right?

Something like the following (not compiled) diff:

diff --git a/arch/x86/hyperv/hv_init.c b/arch/x86/hyperv/hv_init.c
index b0da5320bcff..0dc229a9142c 100644
--- a/arch/x86/hyperv/hv_init.c
+++ b/arch/x86/hyperv/hv_init.c
@@ -424,6 +424,9 @@ void hyperv_report_panic(struct pt_regs *regs, long err)
        static bool panic_reported;
        u64 guest_id;

+       if (!panic_on_oops)
+               return;
+
        /*
         * We prefer to report panic on 'die' chain as we have proper
         * registers to report, but if we miss it (e.g. on BUG()) we need

I haven't checked all the error reporting paths and don't know if this
works or not though.

Wei.

Re: [PATCH V4 6/6] x86/Hyper-V: Report crash data in die() when panic_on_oops is set

[Tianyu Lan]

On 4/2/2020 2:53 AM, Wei Liu wrote:
> On Tue, Mar 31, 2020 at 10:26:06PM +0800, Tianyu Lan wrote:
>>
>>
>> On 3/31/2020 9:51 PM, Michael Kelley wrote:
>>> From: Tianyu Lan <Tianyu.Lan@microsoft.com> Sent: Tuesday, March 31, 2020 12:39 AM
>>>>
>>>> When oops happens with panic_on_oops unset, the oops
>>>> thread is killed by die() and system continues to run.
>>>> In such case, guest should not report crash register
>>>> data to host since system still runs. Fix it.
>>>>
>>>> Reviewed-by: Michael Kelley <mikelley@microsoft.com>
>>>> Signed-off-by: Tianyu Lan <Tianyu.Lan@microsoft.com>
>>>> ---
>>>> Change since v3:
>>>> 	Fix compile error
>>>> ---
>>>>    drivers/hv/vmbus_drv.c | 3 ++-
>>>>    1 file changed, 2 insertions(+), 1 deletion(-)
>>>>
>>>> diff --git a/drivers/hv/vmbus_drv.c b/drivers/hv/vmbus_drv.c
>>>> index 172ceae69abb..4bc02aea2098 100644
>>>> --- a/drivers/hv/vmbus_drv.c
>>>> +++ b/drivers/hv/vmbus_drv.c
>>>> @@ -31,6 +31,7 @@
>>>>    #include <linux/kdebug.h>
>>>>    #include <linux/efi.h>
>>>>    #include <linux/random.h>
>>>> +#include <linux/kernel.h>
>>>
>>> Unfortunately, adding the #include doesn't solve the problem.  The error occurs when
>>> CONFIG_HYPERV=m, because panic_on_oops is not exported.  I haven't thought it
>>> through, but hopefully there's a solution where panic_on_oops can be tested in
>>> hyperv_report_panic() or some other Hyper-V specific function that's never in a
>>> module, so that we don't need to export panic_on_oops.
>>
>> Yes, I don't consider modules case. I think we may introduce a check
>> function of panic_on_oops in the mshyperv.c and expose it to module.
>>
> 
> Why expose something new? You can just test panic_on_oops in
> hyperv_report_panic and bail if it is false, right?
> 
> Something like the following (not compiled) diff:
> 
> diff --git a/arch/x86/hyperv/hv_init.c b/arch/x86/hyperv/hv_init.c
> index b0da5320bcff..0dc229a9142c 100644
> --- a/arch/x86/hyperv/hv_init.c
> +++ b/arch/x86/hyperv/hv_init.c
> @@ -424,6 +424,9 @@ void hyperv_report_panic(struct pt_regs *regs, long err)
>          static bool panic_reported;
>          u64 guest_id;
> 
> +       if (!panic_on_oops)
> +               return;
> +
>          /*
>           * We prefer to report panic on 'die' chain as we have proper
>           * registers to report, but if we miss it (e.g. on BUG()) we need
> 
> I haven't checked all the error reporting paths and don't know if this
> works or not though.

Hi Wei:
     hyperv_report_panic() is also called in panic() which may not be
triggered by die(). In these cases, we still need to report crash
register data to host even when panic_on_oops is unset.
     Another approach is to add new parameter "in_die" for
hyperv_report_panic() and just check panic_on_oops when the function is
called in die notifier callback and "in_die" is set to true.

Re: [PATCH V3 0/6] x86/Hyper-V: Panic code path fixes

[Wei Liu]

On Tue, Mar 24, 2020 at 12:57:14AM -0700, ltykernel@gmail.com wrote:
> From: Tianyu Lan <Tianyu.Lan@microsoft.com>
> 
> This patchset fixes some issues in the Hyper-V panic code path.
> Patch 1 resolves issue that panic system still responses network
> packets.
> Patch 2-3,5-6 resolves crash enlightenment issues.
> Patch 4 is to set crash_kexec_post_notifiers to true for Hyper-V
> VM in order to report crash data or kmsg to host before running
> kdump kernel.
> 
> Tianyu Lan (6):
>   x86/Hyper-V: Unload vmbus channel in hv panic callback
>   x86/Hyper-V: Free hv_panic_page when fail to register kmsg dump
>   x86/Hyper-V: Trigger crash enlightenment only once during  system
>     crash.
>   x86/Hyper-V: Report crash register data or ksmg before  running crash
>     kernel
>   x86/Hyper-V: Report crash register data when sysctl_record_panic_msg
>     is not set
>   x86/Hyper-V: Report crash data in die() when panic_on_oops is set
> 

Queued to hyperv-next. Thanks.

Wei.