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

[Sunil Muthuswamy]

The V3 patch should address the concerns/comments below. Happy to address any additional comments, if there are any.

- Sunil

> -----Original Message-----
> From: Sunil Muthuswamy
> Sent: Monday, May 21, 2018 9:48 AM
> To: 'Greg KH' <gregkh at linuxfoundation.org>
> Cc: devel at linuxdriverproject.org; Haiyang Zhang
> <haiyangz at microsoft.com>; Stephen Hemminger
> <sthemmin at microsoft.com>
> Subject: RE: [PATCH v1] Drivers: HV: Send one page worth of kmsg dump
> over Hyper-V during panic
> 
> The v2 patch that I sent was prior to responding to this thread. I am sending
> out v3 which addresses the comments here.
> 
> > -----Original Message-----
> > From: Greg KH <gregkh at linuxfoundation.org>
> > Sent: Saturday, May 19, 2018 12:01 AM
> > To: Sunil Muthuswamy <sunilmut at microsoft.com>
> > Cc: devel at linuxdriverproject.org; Haiyang Zhang
> > <haiyangz at microsoft.com>; Stephen Hemminger
> > <sthemmin at microsoft.com>
> > Subject: Re: [PATCH v1] Drivers: HV: Send one page worth of kmsg dump
> > over Hyper-V during panic
> >
> > On Fri, May 18, 2018 at 09:00:51PM +0000, Sunil Muthuswamy wrote:
> > > Thanks, Greg.
> > >
> > > My first patch to the Linux kernel. Still making mistakes, but, learning
> > through the documented process.
> > >
> > > > -----Original Message-----
> > > > From: Greg KH <gregkh at linuxfoundation.org>
> > > > Sent: Wednesday, May 9, 2018 11:51 PM
> > > > To: Sunil Muthuswamy <sunilmut at microsoft.com>
> > > > Cc: Haiyang Zhang <haiyangz at microsoft.com>;
> > > > devel at linuxdriverproject.org; Stephen Hemminger
> > > > <sthemmin at microsoft.com>
> > > > Subject: Re: [PATCH v1] Drivers: HV: Send one page worth of kmsg
> dump
> > > > over Hyper-V during panic
> > > >
> > > > On Wed, May 09, 2018 at 07:19:24PM +0000, 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.
> > > >
> > > > Odd indentation, what editor made you do that?  Please move it all to
> the
> > > > left.
> > > I inserted them. Will fix.
> > > >
> > > > >
> > > > > CC: kys at microsoft.com
> > > > > CC: sthemmin at microsoft.com
> > > > > Signed-off-by: Sunil Muthuswamy <sunilmut at microsoft.com>
> > > > > ---
> > > > >  arch/x86/hyperv/hv_init.c          | 28 +++++++++++++++++
> > > > >  arch/x86/include/asm/hyperv-tlfs.h |  5 ++--
> > > > >  arch/x86/include/asm/mshyperv.h    |  1 +
> > > > >  drivers/hv/vmbus_drv.c             | 61
> > > > ++++++++++++++++++++++++++++++++++++++
> > > > >  4 files changed, 93 insertions(+), 2 deletions(-)
> > > > >
> > > > > diff --git a/arch/x86/hyperv/hv_init.c b/arch/x86/hyperv/hv_init.c
> > > > > index cfecc22..88ee90d 100644
> > > > > --- a/arch/x86/hyperv/hv_init.c
> > > > > +++ b/arch/x86/hyperv/hv_init.c
> > > > > @@ -395,6 +395,34 @@ void hyperv_report_panic(struct pt_regs
> > *regs,
> > > > > long err)  }  EXPORT_SYMBOL_GPL(hyperv_report_panic);
> > > > >
> > > > > +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;
> > > >
> > > > Why do you only care about the first message?
> > > It is following the general direction from ' hyperv_report_panic', but, I
> don't
> > think it needs to. Will change.
> > > >
> > > > > +
> > > > > +	/*
> > > > > +	 * 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)
> > > >
> > > > Not in numerical order?
> > > >
> > > > And can you use the BIT() macro here instead?  Not a requirement, just
> a
> > > > general question.
> > > Will change in the next version.
> >
> > You didn't do that :(