LKML Archive on lore.kernel.org
 help / Atom feed
* [PATCH 1/1] Drivers: HV: Send one page worth of kmsg dump over Hyper-V during panic
@ 2018-07-08  2:56 kys
  2018-07-11  1:04 ` Michael Kelley (EOSG)
  0 siblings, 1 reply; 4+ messages in thread
From: kys @ 2018-07-08  2:56 UTC (permalink / raw)
  To: gregkh, linux-kernel, devel, olaf, apw, jasowang, sthemmin,
	Michael.H.Kelley, vkuznets
  Cc: Sunil Muthuswamy, K . Y . Srinivasan

From: Sunil Muthuswamy <sunilmut@microsoft.com>

In the VM mode on Hyper-V, currently, when the kernel panics, an error
code and few register values are populated in an MSR and the Hypervisor
notified. This information is collected on the host. The amount of
information currently collected is found to be limited and not very
actionable. To gather more actionable data, such as stack trace, the
proposal is to write one page worth of kmsg data on an allocated page
and the Hypervisor notified of the page address through the MSR.

- Sysctl option to control the behavior, with ON by default.

Cc: K. Y. Srinivasan <kys@microsoft.com>
Cc: Stephen Hemminger <sthemmin@microsoft.com>
Signed-off-by: Sunil Muthuswamy <sunilmut@microsoft.com>
Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
---
 Documentation/sysctl/kernel.txt    |  11 +++
 arch/x86/hyperv/hv_init.c          |  27 +++++++
 arch/x86/include/asm/hyperv-tlfs.h |   5 +-
 arch/x86/include/asm/mshyperv.h    |   1 +
 drivers/hv/vmbus_drv.c             | 110 +++++++++++++++++++++++++++++
 5 files changed, 152 insertions(+), 2 deletions(-)

diff --git a/Documentation/sysctl/kernel.txt b/Documentation/sysctl/kernel.txt
index eded671d55eb..59585030cbaf 100644
--- a/Documentation/sysctl/kernel.txt
+++ b/Documentation/sysctl/kernel.txt
@@ -39,6 +39,7 @@ show up in /proc/sys/kernel:
 - hung_task_check_count
 - hung_task_timeout_secs
 - hung_task_warnings
+- hyperv_record_panic_msg
 - kexec_load_disabled
 - kptr_restrict
 - l2cr                        [ PPC only ]
@@ -374,6 +375,16 @@ This file shows up if CONFIG_DETECT_HUNG_TASK is enabled.
 
 ==============================================================
 
+hyperv_record_panic_msg:
+
+Controls whether the panic kmsg data should be reported to Hyper-V.
+
+0: do not report panic kmsg data.
+
+1: report the panic kmsg data. This is the default behavior.
+
+==============================================================
+
 kexec_load_disabled:
 
 A toggle indicating if the kexec_load syscall has been disabled. This
diff --git a/arch/x86/hyperv/hv_init.c b/arch/x86/hyperv/hv_init.c
index 595e44e8abaa..9c70018a9906 100644
--- a/arch/x86/hyperv/hv_init.c
+++ b/arch/x86/hyperv/hv_init.c
@@ -423,6 +423,33 @@ void hyperv_report_panic(struct pt_regs *regs, long err)
 }
 EXPORT_SYMBOL_GPL(hyperv_report_panic);
 
+/**
+ * hyperv_report_panic_msg - report panic message to Hyper-V
+ * @pa: physical address of the panic page containing the message
+ * @size: size of the message in the page
+ */
+void hyperv_report_panic_msg(phys_addr_t pa, size_t size)
+{
+	/*
+	 * P3 to contain the physical address of the panic page & P4 to
+	 * contain the size of the panic data in that page. Rest of the
+	 * registers are no-op when the NOTIFY_MSG flag is set.
+	 */
+	wrmsrl(HV_X64_MSR_CRASH_P0, 0);
+	wrmsrl(HV_X64_MSR_CRASH_P1, 0);
+	wrmsrl(HV_X64_MSR_CRASH_P2, 0);
+	wrmsrl(HV_X64_MSR_CRASH_P3, pa);
+	wrmsrl(HV_X64_MSR_CRASH_P4, size);
+
+	/*
+	 * Let Hyper-V know there is crash data available along with
+	 * the panic message.
+	 */
+	wrmsrl(HV_X64_MSR_CRASH_CTL,
+	       (HV_CRASH_CTL_CRASH_NOTIFY | HV_CRASH_CTL_CRASH_NOTIFY_MSG));
+}
+EXPORT_SYMBOL_GPL(hyperv_report_panic_msg);
+
 bool hv_is_hyperv_initialized(void)
 {
 	union hv_x64_msr_hypercall_contents hypercall_msr;
diff --git a/arch/x86/include/asm/hyperv-tlfs.h b/arch/x86/include/asm/hyperv-tlfs.h
index 96272e99b64e..6ced78af48da 100644
--- a/arch/x86/include/asm/hyperv-tlfs.h
+++ b/arch/x86/include/asm/hyperv-tlfs.h
@@ -176,9 +176,10 @@
 #define HV_X64_ENLIGHTENED_VMCS_RECOMMENDED    (1 << 14)
 
 /*
- * Crash notification flag.
+ * Crash notification flags.
  */
-#define HV_CRASH_CTL_CRASH_NOTIFY (1ULL << 63)
+#define HV_CRASH_CTL_CRASH_NOTIFY_MSG	BIT_ULL(62)
+#define HV_CRASH_CTL_CRASH_NOTIFY	BIT_ULL(63)
 
 /* MSR used to identify the guest OS. */
 #define HV_X64_MSR_GUEST_OS_ID			0x40000000
diff --git a/arch/x86/include/asm/mshyperv.h b/arch/x86/include/asm/mshyperv.h
index 81e768b8d9eb..156a2e5a97a9 100644
--- a/arch/x86/include/asm/mshyperv.h
+++ b/arch/x86/include/asm/mshyperv.h
@@ -299,6 +299,7 @@ static inline int cpumask_to_vpset(struct hv_vpset *vpset,
 void __init hyperv_init(void);
 void hyperv_setup_mmu_ops(void);
 void hyperv_report_panic(struct pt_regs *regs, long err);
+void hyperv_report_panic_msg(phys_addr_t pa, size_t size);
 bool hv_is_hyperv_initialized(void);
 void hyperv_cleanup(void);
 
diff --git a/drivers/hv/vmbus_drv.c b/drivers/hv/vmbus_drv.c
index b10fe26c4891..05e37283d7c3 100644
--- a/drivers/hv/vmbus_drv.c
+++ b/drivers/hv/vmbus_drv.c
@@ -56,6 +56,8 @@ static struct completion probe_event;
 
 static int hyperv_cpuhp_online;
 
+static void *hv_panic_page;
+
 static int hyperv_panic_event(struct notifier_block *nb, unsigned long val,
 			      void *args)
 {
@@ -1018,6 +1020,75 @@ 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.
+ */
+static void hv_kmsg_dump(struct kmsg_dumper *dumper,
+			 enum kmsg_dump_reason reason)
+{
+	size_t bytes_written;
+	phys_addr_t panic_pa;
+
+	/* We are only interested in panics. */
+	if ((reason != KMSG_DUMP_PANIC) || (!sysctl_record_panic_msg))
+		return;
+
+	panic_pa = virt_to_phys(hv_panic_page);
+
+	/*
+	 * Write dump contents to the page. No need to synchronize; panic should
+	 * be single-threaded.
+	 */
+	if (!kmsg_dump_get_buffer(dumper, true, hv_panic_page,
+				  PAGE_SIZE, &bytes_written)) {
+		pr_err("Hyper-V: Unable to get kmsg data for panic\n");
+		return;
+	}
+
+	hyperv_report_panic_msg(panic_pa, bytes_written);
+}
+
+static struct kmsg_dumper hv_kmsg_dumper = {
+	.dump = hv_kmsg_dump,
+};
+
+static struct ctl_table_header *hv_ctl_table_hdr;
+static int zero;
+static int one = 1;
+
+/*
+ * sysctl option to allow the user to control whether kmsg data should be
+ * reported to Hyper-V on panic.
+ */
+static struct ctl_table hv_ctl_table[] = {
+	{
+		.procname       = "hyperv_record_panic_msg",
+		.data           = &sysctl_record_panic_msg,
+		.maxlen         = sizeof(int),
+		.mode           = 0644,
+		.proc_handler   = proc_dointvec_minmax,
+		.extra1		= &zero,
+		.extra2		= &one
+	},
+	{}
+};
+
+static struct ctl_table hv_root_table[] = {
+	{
+		.procname	= "kernel",
+		.mode		= 0555,
+		.child		= hv_ctl_table
+	},
+	{}
+};
 
 /*
  * vmbus_bus_init -Main vmbus driver initialization routine.
@@ -1065,6 +1136,32 @@ static int vmbus_bus_init(void)
 	 * Only register if the crash MSRs are available
 	 */
 	if (ms_hyperv.misc_features & HV_FEATURE_GUEST_CRASH_MSR_AVAILABLE) {
+		u64 hyperv_crash_ctl;
+		/*
+		 * Sysctl registration is not fatal, since by default
+		 * reporting is enabled.
+		 */
+		hv_ctl_table_hdr = register_sysctl_table(hv_root_table);
+		if (!hv_ctl_table_hdr)
+			pr_err("Hyper-V: sysctl table register error");
+
+		/*
+		 * Register for panic kmsg callback only if the right
+		 * capability is supported by the hypervisor.
+		 */
+		rdmsrl(HV_X64_MSR_CRASH_CTL, hyperv_crash_ctl);
+		if (hyperv_crash_ctl & HV_CRASH_CTL_CRASH_NOTIFY_MSG) {
+			hv_panic_page = (void *)get_zeroed_page(GFP_KERNEL);
+			if (hv_panic_page) {
+				ret = kmsg_dump_register(&hv_kmsg_dumper);
+				if (ret)
+					pr_err("Hyper-V: kmsg dump register "
+						"error 0x%x\n", ret);
+			} else
+				pr_err("Hyper-V: panic message page memory "
+					"allocation failed");
+		}
+
 		register_die_notifier(&hyperv_die_block);
 		atomic_notifier_chain_register(&panic_notifier_list,
 					       &hyperv_panic_block);
@@ -1081,6 +1178,11 @@ static int vmbus_bus_init(void)
 	hv_remove_vmbus_irq();
 
 	bus_unregister(&hv_bus);
+	free_page((unsigned long)hv_panic_page);
+	if (!hv_ctl_table_hdr) {
+		unregister_sysctl_table(hv_ctl_table_hdr);
+		hv_ctl_table_hdr = NULL;
+	}
 
 	return ret;
 }
@@ -1785,10 +1887,18 @@ static void __exit vmbus_exit(void)
 	vmbus_free_channels();
 
 	if (ms_hyperv.misc_features & HV_FEATURE_GUEST_CRASH_MSR_AVAILABLE) {
+		kmsg_dump_unregister(&hv_kmsg_dumper);
 		unregister_die_notifier(&hyperv_die_block);
 		atomic_notifier_chain_unregister(&panic_notifier_list,
 						 &hyperv_panic_block);
 	}
+
+	free_page((unsigned long)hv_panic_page);
+	if (!hv_ctl_table_hdr) {
+		unregister_sysctl_table(hv_ctl_table_hdr);
+		hv_ctl_table_hdr = NULL;
+	}
+
 	bus_unregister(&hv_bus);
 
 	cpuhp_remove_state(hyperv_cpuhp_online);
-- 
2.17.1


^ permalink raw reply	[flat|nested] 4+ messages in thread

* RE: [PATCH 1/1] Drivers: HV: Send one page worth of kmsg dump over Hyper-V during panic
  2018-07-08  2:56 [PATCH 1/1] Drivers: HV: Send one page worth of kmsg dump over Hyper-V during panic kys
@ 2018-07-11  1:04 ` Michael Kelley (EOSG)
  2018-07-11 16:59   ` Sunil Muthuswamy
  0 siblings, 1 reply; 4+ messages in thread
From: Michael Kelley (EOSG) @ 2018-07-11  1:04 UTC (permalink / raw)
  To: KY Srinivasan, gregkh, linux-kernel, devel, olaf, apw, jasowang,
	Stephen Hemminger, vkuznets
  Cc: Sunil Muthuswamy

From kys@linuxonhyperv.com <kys@linuxonhyperv.com>  Sent: Saturday, July 7, 2018 7:57 PM
> 
> From: Sunil Muthuswamy <sunilmut@microsoft.com>
> 
> In the VM mode on Hyper-V, currently, when the kernel panics, an error
> code and few register values are populated in an MSR and the Hypervisor
> notified. This information is collected on the host. The amount of
> information currently collected is found to be limited and not very
> actionable. To gather more actionable data, such as stack trace, the
> proposal is to write one page worth of kmsg data on an allocated page
> and the Hypervisor notified of the page address through the MSR.
> 
> - Sysctl option to control the behavior, with ON by default.
> 
> Cc: K. Y. Srinivasan <kys@microsoft.com>
> Cc: Stephen Hemminger <sthemmin@microsoft.com>
> Signed-off-by: Sunil Muthuswamy <sunilmut@microsoft.com>
> Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
> ---

> +	/*
> +	 * Write dump contents to the page. No need to synchronize; panic should
> +	 * be single-threaded.
> +	 */
> +	if (!kmsg_dump_get_buffer(dumper, true, hv_panic_page,
> +				  PAGE_SIZE, &bytes_written)) {
> +		pr_err("Hyper-V: Unable to get kmsg data for panic\n");
> +		return;

From what I can see, the return value from kmsg_dump_get_buffer()
is not an indication of success or failure -- it's an indication of whether
there is more data available.   There's no reason to output an error
message.

> @@ -1065,6 +1136,32 @@ static int vmbus_bus_init(void)
>  	 * Only register if the crash MSRs are available
>  	 */
>  	if (ms_hyperv.misc_features & HV_FEATURE_GUEST_CRASH_MSR_AVAILABLE) {
> +		u64 hyperv_crash_ctl;
> +		/*
> +		 * Sysctl registration is not fatal, since by default
> +		 * reporting is enabled.
> +		 */
> +		hv_ctl_table_hdr = register_sysctl_table(hv_root_table);
> +		if (!hv_ctl_table_hdr)
> +			pr_err("Hyper-V: sysctl table register error");
> +
> +		/*
> +		 * Register for panic kmsg callback only if the right
> +		 * capability is supported by the hypervisor.
> +		 */
> +		rdmsrl(HV_X64_MSR_CRASH_CTL, hyperv_crash_ctl);
> +		if (hyperv_crash_ctl & HV_CRASH_CTL_CRASH_NOTIFY_MSG) {

vmbus_drv.c is architecture independent code, and should not be referencing
x86/x64 MSRs.   Reading the MSR (and maybe the test as well?) should go
in a separate function in an x86-specific source file.

And just to confirm, is this the right way to test for the feature?  Usually,
feature determination is based on one of the feature registers.  The
NOTIFY_MSG flag seems to have a dual meaning -- on read it indicates
the feature is present.  On write in hyperv_report_panic_msg(), it evidently
means that the guest is sending a full page of data to Hyper-V.

> @@ -1081,6 +1178,11 @@ static int vmbus_bus_init(void)
>  	bus_unregister(&hv_bus);
> +	free_page((unsigned long)hv_panic_page);
> +	if (!hv_ctl_table_hdr) {

The above test is backwards.  Remove the bang.

> @@ -1785,10 +1887,18 @@ static void __exit vmbus_exit(void)
> +	free_page((unsigned long)hv_panic_page);
> +	if (!hv_ctl_table_hdr) {

Same here.  Test is backwards.

Michael


^ permalink raw reply	[flat|nested] 4+ messages in thread

* RE: [PATCH 1/1] Drivers: HV: Send one page worth of kmsg dump over Hyper-V during panic
  2018-07-11  1:04 ` Michael Kelley (EOSG)
@ 2018-07-11 16:59   ` Sunil Muthuswamy
  2018-07-11 20:50     ` Michael Kelley (EOSG)
  0 siblings, 1 reply; 4+ messages in thread
From: Sunil Muthuswamy @ 2018-07-11 16:59 UTC (permalink / raw)
  To: Michael Kelley (EOSG),
	KY Srinivasan, gregkh, linux-kernel, devel, olaf, apw, jasowang,
	Stephen Hemminger, vkuznets

Thanks, Michael. In which branch should I fix these now that the changes have been
merged with the char-misc-next branch?

Comments inline.

> -----Original Message-----
> From: Michael Kelley (EOSG)
> Sent: Tuesday, July 10, 2018 6:05 PM
> To: KY Srinivasan <kys@microsoft.com>; gregkh@linuxfoundation.org; linux-
> kernel@vger.kernel.org; devel@linuxdriverproject.org; olaf@aepfle.de;
> apw@canonical.com; jasowang@redhat.com; Stephen Hemminger
> <sthemmin@microsoft.com>; vkuznets@redhat.com
> Cc: Sunil Muthuswamy <sunilmut@microsoft.com>
> Subject: RE: [PATCH 1/1] Drivers: HV: Send one page worth of kmsg dump
> over Hyper-V during panic
> 
> From kys@linuxonhyperv.com <kys@linuxonhyperv.com>  Sent: Saturday,
> July 7, 2018 7:57 PM
> >
> > From: Sunil Muthuswamy <sunilmut@microsoft.com>
> >
> > In the VM mode on Hyper-V, currently, when the kernel panics, an error
> > code and few register values are populated in an MSR and the Hypervisor
> > notified. This information is collected on the host. The amount of
> > information currently collected is found to be limited and not very
> > actionable. To gather more actionable data, such as stack trace, the
> > proposal is to write one page worth of kmsg data on an allocated page
> > and the Hypervisor notified of the page address through the MSR.
> >
> > - Sysctl option to control the behavior, with ON by default.
> >
> > Cc: K. Y. Srinivasan <kys@microsoft.com>
> > Cc: Stephen Hemminger <sthemmin@microsoft.com>
> > Signed-off-by: Sunil Muthuswamy <sunilmut@microsoft.com>
> > Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
> > ---
> 
> > +	/*
> > +	 * Write dump contents to the page. No need to synchronize; panic
> should
> > +	 * be single-threaded.
> > +	 */
> > +	if (!kmsg_dump_get_buffer(dumper, true, hv_panic_page,
> > +				  PAGE_SIZE, &bytes_written)) {
> > +		pr_err("Hyper-V: Unable to get kmsg data for panic\n");
> > +		return;
> 
> From what I can see, the return value from kmsg_dump_get_buffer()
> is not an indication of success or failure -- it's an indication of whether
> there is more data available.   There's no reason to output an error
> message.
> 
That seems correct. Will address this.
> > @@ -1065,6 +1136,32 @@ static int vmbus_bus_init(void)
> >  	 * Only register if the crash MSRs are available
> >  	 */
> >  	if (ms_hyperv.misc_features &
> HV_FEATURE_GUEST_CRASH_MSR_AVAILABLE) {
> > +		u64 hyperv_crash_ctl;
> > +		/*
> > +		 * Sysctl registration is not fatal, since by default
> > +		 * reporting is enabled.
> > +		 */
> > +		hv_ctl_table_hdr = register_sysctl_table(hv_root_table);
> > +		if (!hv_ctl_table_hdr)
> > +			pr_err("Hyper-V: sysctl table register error");
> > +
> > +		/*
> > +		 * Register for panic kmsg callback only if the right
> > +		 * capability is supported by the hypervisor.
> > +		 */
> > +		rdmsrl(HV_X64_MSR_CRASH_CTL, hyperv_crash_ctl);
> > +		if (hyperv_crash_ctl &
> HV_CRASH_CTL_CRASH_NOTIFY_MSG) {
> 
> vmbus_drv.c is architecture independent code, and should not be
> referencing
> x86/x64 MSRs.   Reading the MSR (and maybe the test as well?) should go
> in a separate function in an x86-specific source file.
> 
I will move the code.
> And just to confirm, is this the right way to test for the feature?  Usually,
> feature determination is based on one of the feature registers.  The
> NOTIFY_MSG flag seems to have a dual meaning -- on read it indicates
> the feature is present.  On write in hyperv_report_panic_msg(), it evidently
> means that the guest is sending a full page of data to Hyper-V.
> 
As per my conversation with John, this seems to be correct and something
also he suggested. The host sets these bits depending on whether it supports
these features or not.

> > @@ -1081,6 +1178,11 @@ static int vmbus_bus_init(void)
> >  	bus_unregister(&hv_bus);
> > +	free_page((unsigned long)hv_panic_page);
> > +	if (!hv_ctl_table_hdr) {
> 
> The above test is backwards.  Remove the bang.
Good call, will do.
> 
> > @@ -1785,10 +1887,18 @@ static void __exit vmbus_exit(void)
> > +	free_page((unsigned long)hv_panic_page);
> > +	if (!hv_ctl_table_hdr) {
> 
> Same here.  Test is backwards.
> 
Will fix.
> Michael


^ permalink raw reply	[flat|nested] 4+ messages in thread

* RE: [PATCH 1/1] Drivers: HV: Send one page worth of kmsg dump over Hyper-V during panic
  2018-07-11 16:59   ` Sunil Muthuswamy
@ 2018-07-11 20:50     ` Michael Kelley (EOSG)
  0 siblings, 0 replies; 4+ messages in thread
From: Michael Kelley (EOSG) @ 2018-07-11 20:50 UTC (permalink / raw)
  To: Sunil Muthuswamy, KY Srinivasan, gregkh, linux-kernel, devel,
	olaf, apw, jasowang, Stephen Hemminger, vkuznets

From: Sunil Muthuswamy Sent: Wednesday, July 11, 2018 9:59 AM
> Thanks, Michael. In which branch should I fix these now that the changes have been
> merged with the char-misc-next branch?

If the original code is already in char-misc-next, you should probably submit a
completely new patch for char-misc-next that just makes the fixes to the original code.

Michael

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, back to index

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-07-08  2:56 [PATCH 1/1] Drivers: HV: Send one page worth of kmsg dump over Hyper-V during panic kys
2018-07-11  1:04 ` Michael Kelley (EOSG)
2018-07-11 16:59   ` Sunil Muthuswamy
2018-07-11 20:50     ` Michael Kelley (EOSG)

LKML Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/lkml/0 lkml/git/0.git
	git clone --mirror https://lore.kernel.org/lkml/1 lkml/git/1.git
	git clone --mirror https://lore.kernel.org/lkml/2 lkml/git/2.git
	git clone --mirror https://lore.kernel.org/lkml/3 lkml/git/3.git
	git clone --mirror https://lore.kernel.org/lkml/4 lkml/git/4.git
	git clone --mirror https://lore.kernel.org/lkml/5 lkml/git/5.git
	git clone --mirror https://lore.kernel.org/lkml/6 lkml/git/6.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 lkml lkml/ https://lore.kernel.org/lkml \
		linux-kernel@vger.kernel.org linux-kernel@archiver.kernel.org
	public-inbox-index lkml


Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-kernel


AGPL code for this site: git clone https://public-inbox.org/ public-inbox