Re: [PATCH v2] Drivers: HV: Send one page worth of kmsg dump over Hyper-V during panic

2018-05-19 Thread Greg KH 
On Fri, May 18, 2018 at 07:09:10PM +, Sunil Muthuswamy wrote:
> 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.
> 
> - Added a sysctl option to control the behavior, with ON by default.
> 
> CC: k...@microsoft.com
> CC: sthem...@microsoft.com
> Signed-off-by: Sunil Muthuswamy 
> ---
>  arch/x86/hyperv/hv_init.c  |  35 ++
>  arch/x86/include/asm/hyperv-tlfs.h |   5 +-
>  arch/x86/include/asm/mshyperv.h|   1 +
>  drivers/hv/vmbus_drv.c | 128 
> +
>  4 files changed, 167 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/x86/hyperv/hv_init.c b/arch/x86/hyperv/hv_init.c
> index cfecc22..fc1e3cb 100644
> --- a/arch/x86/hyperv/hv_init.c
> +++ b/arch/x86/hyperv/hv_init.c
> @@ -395,6 +395,41 @@ 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
> + *
> + */
> +

The blank lines are not needed, please drop.

> +void hyperv_report_panic_msg(phys_addr_t pa, size_t size)
> +{
> + static bool panic_msg_reported;
> +
> + if (panic_msg_reported)
> + return;
> + panic_msg_reported = true;
> +
> + /*
> +  * 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 416cb0e..fc2932c 100644
> --- a/arch/x86/include/asm/hyperv-tlfs.h
> +++ b/arch/x86/include/asm/hyperv-tlfs.h
> @@ -171,9 +171,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 (1ULL << 63)
> +#define HV_CRASH_CTL_CRASH_NOTIFY_MSG (1ULL << 62)

BIT_ULL()?

And again, why are they not in numerical order?

>  /* MSR used to identify the guest OS. */
>  #define HV_X64_MSR_GUEST_OS_ID   0x4000
> diff --git a/arch/x86/include/asm/mshyperv.h b/arch/x86/include/asm/mshyperv.h
> index b90e796..ac83f2d 100644
> --- a/arch/x86/include/asm/mshyperv.h
> +++ b/arch/x86/include/asm/mshyperv.h
> @@ -262,6 +262,7 @@ void hyperv_init(void);
>  void hyperv_setup_mmu_ops(void);
>  void hyper_alloc_mmu(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 b10fe26..7b04f7f 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,86 @@ 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
> + */
> +int sysctl_record_panic_msg = 1;

Why was a sysctl chosen?  I'm not objecting, but you have to document
new user/kernel apis when you create them.

> +/*
> + * 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;
> +

Re: [PATCH v2] Drivers: HV: Send one page worth of kmsg dump over Hyper-V during panic

2018-05-19 Thread Greg KH 
On Fri, May 18, 2018 at 07:09:10PM +, Sunil Muthuswamy wrote:
> 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.
> 
> - Added a sysctl option to control the behavior, with ON by default.
> 
> CC: k...@microsoft.com
> CC: sthem...@microsoft.com
> Signed-off-by: Sunil Muthuswamy 
> ---
>  arch/x86/hyperv/hv_init.c  |  35 ++
>  arch/x86/include/asm/hyperv-tlfs.h |   5 +-
>  arch/x86/include/asm/mshyperv.h|   1 +
>  drivers/hv/vmbus_drv.c | 128 
> +
>  4 files changed, 167 insertions(+), 2 deletions(-)

What changed from v1?  Always put that below the --- line as the
documentation says to.

thanks,

greg k-h
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH v2] Drivers: HV: Send one page worth of kmsg dump over Hyper-V during panic

2018-05-18 Thread Stephen Hemminger 
On Fri, 18 May 2018 19:09:10 +
Sunil Muthuswamy  wrote:

>  
> +/*
> + * Boolean to control whether to report panic messages over Hyper-V.
> + *
> + * It can be set via /proc/sys/kernel/hyperv/record_panic_msg
> + */
> +int sysctl_record_panic_msg = 1;
> +

Looks good, this will help.

Please create new documentation to Documentation/sysctl/ for the new
sysctl.
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH v2] Drivers: HV: Send one page worth of kmsg dump over Hyper-V during panic

2018-05-18 Thread Sunil Muthuswamy 
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.

- Added a sysctl option to control the behavior, with ON by default.

CC: k...@microsoft.com
CC: sthem...@microsoft.com
Signed-off-by: Sunil Muthuswamy 
---
 arch/x86/hyperv/hv_init.c  |  35 ++
 arch/x86/include/asm/hyperv-tlfs.h |   5 +-
 arch/x86/include/asm/mshyperv.h|   1 +
 drivers/hv/vmbus_drv.c | 128 +
 4 files changed, 167 insertions(+), 2 deletions(-)

diff --git a/arch/x86/hyperv/hv_init.c b/arch/x86/hyperv/hv_init.c
index cfecc22..fc1e3cb 100644
--- a/arch/x86/hyperv/hv_init.c
+++ b/arch/x86/hyperv/hv_init.c
@@ -395,6 +395,41 @@ 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)
+{
+   static bool panic_msg_reported;
+
+   if (panic_msg_reported)
+   return;
+   panic_msg_reported = true;
+
+   /*
+* 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 416cb0e..fc2932c 100644
--- a/arch/x86/include/asm/hyperv-tlfs.h
+++ b/arch/x86/include/asm/hyperv-tlfs.h
@@ -171,9 +171,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 (1ULL << 63)
+#define HV_CRASH_CTL_CRASH_NOTIFY_MSG (1ULL << 62)
 
 /* MSR used to identify the guest OS. */
 #define HV_X64_MSR_GUEST_OS_ID 0x4000
diff --git a/arch/x86/include/asm/mshyperv.h b/arch/x86/include/asm/mshyperv.h
index b90e796..ac83f2d 100644
--- a/arch/x86/include/asm/mshyperv.h
+++ b/arch/x86/include/asm/mshyperv.h
@@ -262,6 +262,7 @@ void hyperv_init(void);
 void hyperv_setup_mmu_ops(void);
 void hyper_alloc_mmu(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 b10fe26..7b04f7f 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,86 @@ 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
+ */
+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)
+   return;
+
+   if (!sysctl_record_panic_msg)
+   return;
+
+   if (!hv_panic_page)
+   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,