>From kys@xxxxxxxxxxxxxxxxx <kys@xxxxxxxxxxxxxxxxx> Sent: Saturday, July 7, 2018 7:57 PM
>
> From: Sunil Muthuswamy <sunilmut@xxxxxxxxxxxxx>
>
> 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@xxxxxxxxxxxxx>
> Cc: Stephen Hemminger <sthemmin@xxxxxxxxxxxxx>
> Signed-off-by: Sunil Muthuswamy <sunilmut@xxxxxxxxxxxxx>
> Signed-off-by: K. Y. Srinivasan <kys@xxxxxxxxxxxxx>
> ---
> + /*
> + * 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
_______________________________________________
devel mailing list
devel@xxxxxxxxxxxxxxxxxxxxxx
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
