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

[PATCH V4 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
    0;136;0c0;136;0c system crash.
  x86/Hyper-V: Report crash register data or kmsg 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/hyperv/hv_init.c      |  6 +++-
 arch/x86/kernel/cpu/mshyperv.c | 10 +++++++
 drivers/hv/channel_mgmt.c      |  3 ++
 drivers/hv/vmbus_drv.c         | 62 ++++++++++++++++++++++++++++++------------
 include/asm-generic/mshyperv.h |  2 +-
 5 files changed, 63 insertions(+), 20 deletions(-)

-- 
2.14.5

[PATCH V4 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.

Fixes: 81b18bce48af ("Drivers: HV: Send one page worth of kmsg dump over Hyper-V during panic")
Reviewed-by: Michael Kelley <mikelley@microsoft.com>
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.
Change since v3:
        - Add fix commit in the change log.
---
 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

[PATCH V4 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.

Fixes: 81b18bce48af ("Drivers: HV: Send one page worth of kmsg dump over Hyper-V during panic")
Reviewed-by: Michael Kelley <mikelley@microsoft.com>
Signed-off-by: Tianyu Lan <Tianyu.Lan@microsoft.com>
---
Change since v1:
	- Update commit log
	- Remove hv_free_hyperv_page() in the error path
Change since v3:
        - Add fix commit in the change log.	
---
 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

[PATCH V4 3/6] x86/Hyper-V: Trigger crash enlightenment only once during 0;136;0c0;136;0c 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().

Fixes: 7ed4325a44ea ("Drivers: hv: vmbus: Make panic reporting to be more useful")
Reviewed-by: Michael Kelley <mikelley@microsoft.com>
Signed-off-by: Tianyu Lan <Tianyu.Lan@microsoft.com>
---
Change since v1:
	- Update commit log

Change since v2:
        - Update comment
Change since v3:
        - Add fix commit in the change log.	
---
 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

[PATCH V4 4/6] x86/Hyper-V: Report crash register data or kmsg 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.

Fixes: 81b18bce48af ("Drivers: HV: Send one page worth of kmsg dump over Hyper-V during panic")
Reviewed-by: Michael Kelley <mikelley@microsoft.com>
Signed-off-by: Tianyu Lan <Tianyu.Lan@microsoft.com>
---
Change since v1:
       Update commit log
Change since v3:
        - Add fix commit in the change 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

[PATCH V4 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().

Fixes: 81b18bce48af ("Drivers: HV: Send one page worth of kmsg dump over Hyper-V during panic")
Reviewed-by: Michael Kelley <mikelley@microsoft.com>
Signed-off-by: Tianyu Lan <Tianyu.Lan@microsoft.com>
---
Change since v3:
        - Add fix commit in the change log
---
 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

[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. Check panic_on_oops
and return directly in hyperv_report_panic() when the function
is called in the die() and panic_on_oops is unset. Fix it.

Fixes: 7ed4325a44ea ("Drivers: hv: vmbus: Make panic reporting to be more useful")
Signed-off-by: Tianyu Lan <Tianyu.Lan@microsoft.com>
---
Change since v3:
	- Fix compile error.
        - Add fix commit in the change log
---
 arch/x86/hyperv/hv_init.c      | 6 +++++-
 drivers/hv/vmbus_drv.c         | 5 +++--
 include/asm-generic/mshyperv.h | 2 +-
 3 files changed, 9 insertions(+), 4 deletions(-)

diff --git a/arch/x86/hyperv/hv_init.c b/arch/x86/hyperv/hv_init.c
index b0da5320bcff..31e1d70f7e03 100644
--- a/arch/x86/hyperv/hv_init.c
+++ b/arch/x86/hyperv/hv_init.c
@@ -20,6 +20,7 @@
 #include <linux/mm.h>
 #include <linux/hyperv.h>
 #include <linux/slab.h>
+#include <linux/kernel.h>
 #include <linux/cpuhotplug.h>
 #include <linux/syscore_ops.h>
 #include <clocksource/hyperv_timer.h>
@@ -419,11 +420,14 @@ void hyperv_cleanup(void)
 }
 EXPORT_SYMBOL_GPL(hyperv_cleanup);
 
-void hyperv_report_panic(struct pt_regs *regs, long err)
+void hyperv_report_panic(struct pt_regs *regs, long err, bool in_die)
 {
 	static bool panic_reported;
 	u64 guest_id;
 
+	if (in_die && !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
diff --git a/drivers/hv/vmbus_drv.c b/drivers/hv/vmbus_drv.c
index 172ceae69abb..a68bce4d0ddb 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"
@@ -75,7 +76,7 @@ static int hyperv_panic_event(struct notifier_block *nb, unsigned long val,
 	if (ms_hyperv.misc_features & HV_FEATURE_GUEST_CRASH_MSR_AVAILABLE
 	    && hyperv_report_reg()) {
 		regs = current_pt_regs();
-		hyperv_report_panic(regs, val);
+		hyperv_report_panic(regs, val, false);
 	}
 	return NOTIFY_DONE;
 }
@@ -92,7 +93,7 @@ static int hyperv_die_event(struct notifier_block *nb, unsigned long val,
 	 * the notification here.
 	 */
 	if (hyperv_report_reg())
-		hyperv_report_panic(regs, val);
+		hyperv_report_panic(regs, val, true);
 	return NOTIFY_DONE;
 }
 
diff --git a/include/asm-generic/mshyperv.h b/include/asm-generic/mshyperv.h
index b3f1082cc435..1c4fd950f091 100644
--- a/include/asm-generic/mshyperv.h
+++ b/include/asm-generic/mshyperv.h
@@ -163,7 +163,7 @@ static inline int cpumask_to_vpset(struct hv_vpset *vpset,
 	return nr_bank;
 }
 
-void hyperv_report_panic(struct pt_regs *regs, long err);
+void hyperv_report_panic(struct pt_regs *regs, long err, bool in_die);
 void hyperv_report_panic_msg(phys_addr_t pa, size_t size);
 bool hv_is_hyperv_initialized(void);
 bool hv_is_hibernation_supported(void);
-- 
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: Monday, April 6, 2020 8:54 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. Check panic_on_oops
> and return directly in hyperv_report_panic() when the function
> is called in the die() and panic_on_oops is unset. Fix it.
> 
> Fixes: 7ed4325a44ea ("Drivers: hv: vmbus: Make panic reporting to be more useful")
> Signed-off-by: Tianyu Lan <Tianyu.Lan@microsoft.com>
> ---
> Change since v3:
> 	- Fix compile error.
>         - Add fix commit in the change log
> ---
>  arch/x86/hyperv/hv_init.c      | 6 +++++-
>  drivers/hv/vmbus_drv.c         | 5 +++--
>  include/asm-generic/mshyperv.h | 2 +-
>  3 files changed, 9 insertions(+), 4 deletions(-)

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

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

[Wei Liu]

On Wed, Apr 08, 2020 at 08:19:47PM +0000, Michael Kelley wrote:
> From: Tianyu Lan <Tianyu.Lan@microsoft.com> Sent: Monday, April 6, 2020 8:54 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. Check panic_on_oops
> > and return directly in hyperv_report_panic() when the function
> > is called in the die() and panic_on_oops is unset. Fix it.
> > 
> > Fixes: 7ed4325a44ea ("Drivers: hv: vmbus: Make panic reporting to be more useful")
> > Signed-off-by: Tianyu Lan <Tianyu.Lan@microsoft.com>
> > ---
> > Change since v3:
> > 	- Fix compile error.
> >         - Add fix commit in the change log
> > ---
> >  arch/x86/hyperv/hv_init.c      | 6 +++++-
> >  drivers/hv/vmbus_drv.c         | 5 +++--
> >  include/asm-generic/mshyperv.h | 2 +-
> >  3 files changed, 9 insertions(+), 4 deletions(-)
> 
> Reviewed-by: Michael Kelley <mikelley@microsoft.com>

It seems to me only the last patch is new, others are already in my
tree, so I only apply the last one.

Let me know if my understanding is wrong.

Wei.

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

[Michael Kelley]

From: Wei Liu <wei.liu@kernel.org>  Sent: Thursday, April 9, 2020 9:41 AM
> 
> On Wed, Apr 08, 2020 at 08:19:47PM +0000, Michael Kelley wrote:
> > From: Tianyu Lan <Tianyu.Lan@microsoft.com> Sent: Monday, April 6, 2020 8:54 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. Check panic_on_oops
> > > and return directly in hyperv_report_panic() when the function
> > > is called in the die() and panic_on_oops is unset. Fix it.
> > >
> > > Fixes: 7ed4325a44ea ("Drivers: hv: vmbus: Make panic reporting to be more useful")
> > > Signed-off-by: Tianyu Lan <Tianyu.Lan@microsoft.com>
> > > ---
> > > Change since v3:
> > > 	- Fix compile error.
> > >         - Add fix commit in the change log
> > > ---
> > >  arch/x86/hyperv/hv_init.c      | 6 +++++-
> > >  drivers/hv/vmbus_drv.c         | 5 +++--
> > >  include/asm-generic/mshyperv.h | 2 +-
> > >  3 files changed, 9 insertions(+), 4 deletions(-)
> >
> > Reviewed-by: Michael Kelley <mikelley@microsoft.com>
> 
> It seems to me only the last patch is new, others are already in my
> tree, so I only apply the last one.
> 
> Let me know if my understanding is wrong.
> 

Tianyu added "Fixes:" tags to some of the other patches in
the series.  It appears the version you had already queued doesn't
have those "Fixes:" tags.

Michael

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

[Wei Liu]

On Thu, Apr 09, 2020 at 11:06:13PM +0000, Michael Kelley wrote:
> From: Wei Liu <wei.liu@kernel.org>  Sent: Thursday, April 9, 2020 9:41 AM
> > 
> > On Wed, Apr 08, 2020 at 08:19:47PM +0000, Michael Kelley wrote:
> > > From: Tianyu Lan <Tianyu.Lan@microsoft.com> Sent: Monday, April 6, 2020 8:54 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. Check panic_on_oops
> > > > and return directly in hyperv_report_panic() when the function
> > > > is called in the die() and panic_on_oops is unset. Fix it.
> > > >
> > > > Fixes: 7ed4325a44ea ("Drivers: hv: vmbus: Make panic reporting to be more useful")
> > > > Signed-off-by: Tianyu Lan <Tianyu.Lan@microsoft.com>
> > > > ---
> > > > Change since v3:
> > > > 	- Fix compile error.
> > > >         - Add fix commit in the change log
> > > > ---
> > > >  arch/x86/hyperv/hv_init.c      | 6 +++++-
> > > >  drivers/hv/vmbus_drv.c         | 5 +++--
> > > >  include/asm-generic/mshyperv.h | 2 +-
> > > >  3 files changed, 9 insertions(+), 4 deletions(-)
> > >
> > > Reviewed-by: Michael Kelley <mikelley@microsoft.com>
> > 
> > It seems to me only the last patch is new, others are already in my
> > tree, so I only apply the last one.
> > 
> > Let me know if my understanding is wrong.
> > 
> 
> Tianyu added "Fixes:" tags to some of the other patches in
> the series.  It appears the version you had already queued doesn't
> have those "Fixes:" tags.

Oh, I only read the subject lines and checked the Reviewed-by tags. I
will queue the new series instead.

Wei.

> 
> Michael

[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

[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.