From: "Michael Kelley (EOSG)" <Michael.H.Kelley@microsoft.com>
To: KY Srinivasan <kys@microsoft.com>,
"gregkh@linuxfoundation.org" <gregkh@linuxfoundation.org>,
"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
"devel@linuxdriverproject.org" <devel@linuxdriverproject.org>,
"olaf@aepfle.de" <olaf@aepfle.de>,
"apw@canonical.com" <apw@canonical.com>,
"jasowang@redhat.com" <jasowang@redhat.com>,
Stephen Hemminger <sthemmin@microsoft.com>,
"vkuznets@redhat.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
Date: Wed, 11 Jul 2018 01:04:36 +0000 [thread overview]
Message-ID: <SN6PR2101MB11207780F4859B196153FF2DDC5A0@SN6PR2101MB1120.namprd21.prod.outlook.com> (raw)
In-Reply-To: <20180708025651.26922-1-kys@linuxonhyperv.com>
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
next prev parent reply other threads:[~2018-07-11 1:06 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
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) [this message]
2018-07-11 16:59 ` Sunil Muthuswamy
2018-07-11 20:50 ` Michael Kelley (EOSG)
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=SN6PR2101MB11207780F4859B196153FF2DDC5A0@SN6PR2101MB1120.namprd21.prod.outlook.com \
--to=michael.h.kelley@microsoft.com \
--cc=apw@canonical.com \
--cc=devel@linuxdriverproject.org \
--cc=gregkh@linuxfoundation.org \
--cc=jasowang@redhat.com \
--cc=kys@microsoft.com \
--cc=linux-kernel@vger.kernel.org \
--cc=olaf@aepfle.de \
--cc=sthemmin@microsoft.com \
--cc=sunilmut@microsoft.com \
--cc=vkuznets@redhat.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).