Linux kernel mirror (for testing) git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
kernel os linux

drivers: hv, hyperv_fb: Untangle and refactor Hyper-V panic notifiers

Currently Hyper-V guests are among the most relevant users of the panic
infrastructure, like panic notifiers, kmsg dumpers, etc. The reasons rely
both in cleaning-up procedures (closing hypervisor <-> guest connection,
disabling some paravirtualized timer) as well as to data collection
(sending panic information to the hypervisor) and framebuffer management.

The thing is: some notifiers are related to others, ordering matters, some
functionalities are duplicated and there are lots of conditionals behind
sending panic information to the hypervisor. As part of an effort to
clean-up the panic notifiers mechanism and better document things, we
hereby address some of the issues/complexities of Hyper-V panic handling
through the following changes:

(a) We have die and panic notifiers on vmbus_drv.c and both have goals of
sending panic information to the hypervisor, though the panic notifier is
also responsible for a cleaning-up procedure.

This commit clears the code by splitting the panic notifier in two, one
for closing the vmbus connection whereas the other is only for sending
panic info to hypervisor. With that, it was possible to merge the die and
panic notifiers in a single/well-documented function, and clear some
conditional complexities on sending such information to the hypervisor.

(b) There is a Hyper-V framebuffer panic notifier, which relies in doing
a vmbus operation that demands a valid connection. So, we must order this
notifier with the panic notifier from vmbus_drv.c, to guarantee that the
framebuffer code executes before the vmbus connection is unloaded.

Also, this commit removes a useless header.

Although there is code rework and re-ordering, we expect that this change
has no functional regressions but instead optimize the path and increase
panic reliability on Hyper-V. This was tested on Hyper-V with success.

Cc: Andrea Parri (Microsoft) <parri.andrea@gmail.com>
Cc: Dexuan Cui <decui@microsoft.com>
Cc: Haiyang Zhang <haiyangz@microsoft.com>
Cc: "K. Y. Srinivasan" <kys@microsoft.com>
Cc: Petr Mladek <pmladek@suse.com>
Cc: Stephen Hemminger <sthemmin@microsoft.com>
Cc: Tianyu Lan <Tianyu.Lan@microsoft.com>
Cc: Wei Liu <wei.liu@kernel.org>
Reviewed-by: Michael Kelley <mikelley@microsoft.com>
Tested-by: Fabio A M Martins <fabiomirmar@gmail.com>
Signed-off-by: Guilherme G. Piccoli <gpiccoli@igalia.com>
Tested-by: Michael Kelley <mikelley@microsoft.com>
Link: https://lore.kernel.org/r/20220819221731.480795-11-gpiccoli@igalia.com
Signed-off-by: Wei Liu <wei.liu@kernel.org>

authored by

Guilherme G. Piccoli and committed by
Wei Liu
d786e00d 1d044ca0

+72 -41
+64 -41
drivers/hv/vmbus_drv.c
··· 25 25 #include <linux/sched/task_stack.h> 26 26 27 27 #include <linux/delay.h> 28 - #include <linux/notifier.h> 29 28 #include <linux/panic_notifier.h> 30 29 #include <linux/ptrace.h> 31 30 #include <linux/screen_info.h> ··· 67 68 return !sysctl_record_panic_msg || !hv_panic_page; 68 69 } 69 70 70 - static int hyperv_panic_event(struct notifier_block *nb, unsigned long val, 71 + /* 72 + * The panic notifier below is responsible solely for unloading the 73 + * vmbus connection, which is necessary in a panic event. 74 + * 75 + * Notice an intrincate relation of this notifier with Hyper-V 76 + * framebuffer panic notifier exists - we need vmbus connection alive 77 + * there in order to succeed, so we need to order both with each other 78 + * [see hvfb_on_panic()] - this is done using notifiers' priorities. 79 + */ 80 + static int hv_panic_vmbus_unload(struct notifier_block *nb, unsigned long val, 71 81 void *args) 72 82 { 73 - struct pt_regs *regs; 74 - 75 83 vmbus_initiate_unload(true); 76 - 77 - /* 78 - * Hyper-V should be notified only once about a panic. If we will be 79 - * doing hv_kmsg_dump() with kmsg data later, don't do the notification 80 - * here. 81 - */ 82 - if (ms_hyperv.misc_features & HV_FEATURE_GUEST_CRASH_MSR_AVAILABLE 83 - && hyperv_report_reg()) { 84 - regs = current_pt_regs(); 85 - hyperv_report_panic(regs, val, false); 86 - } 87 84 return NOTIFY_DONE; 88 85 } 86 + static struct notifier_block hyperv_panic_vmbus_unload_block = { 87 + .notifier_call = hv_panic_vmbus_unload, 88 + .priority = INT_MIN + 1, /* almost the latest one to execute */ 89 + }; 89 90 90 - static int hyperv_die_event(struct notifier_block *nb, unsigned long val, 91 - void *args) 91 + static int hv_die_panic_notify_crash(struct notifier_block *self, 92 + unsigned long val, void *args); 93 + 94 + static struct notifier_block hyperv_die_report_block = { 95 + .notifier_call = hv_die_panic_notify_crash, 96 + }; 97 + static struct notifier_block hyperv_panic_report_block = { 98 + .notifier_call = hv_die_panic_notify_crash, 99 + }; 100 + 101 + /* 102 + * The following callback works both as die and panic notifier; its 103 + * goal is to provide panic information to the hypervisor unless the 104 + * kmsg dumper is used [see hv_kmsg_dump()], which provides more 105 + * information but isn't always available. 106 + * 107 + * Notice that both the panic/die report notifiers are registered only 108 + * if we have the capability HV_FEATURE_GUEST_CRASH_MSR_AVAILABLE set. 109 + */ 110 + static int hv_die_panic_notify_crash(struct notifier_block *self, 111 + unsigned long val, void *args) 92 112 { 93 - struct die_args *die = args; 94 - struct pt_regs *regs = die->regs; 113 + struct pt_regs *regs; 114 + bool is_die; 95 115 96 - /* Don't notify Hyper-V if the die event is other than oops */ 97 - if (val != DIE_OOPS) 98 - return NOTIFY_DONE; 116 + /* Don't notify Hyper-V unless we have a die oops event or panic. */ 117 + if (self == &hyperv_panic_report_block) { 118 + is_die = false; 119 + regs = current_pt_regs(); 120 + } else { /* die event */ 121 + if (val != DIE_OOPS) 122 + return NOTIFY_DONE; 123 + 124 + is_die = true; 125 + regs = ((struct die_args *)args)->regs; 126 + } 99 127 100 128 /* 101 - * Hyper-V should be notified only once about a panic. If we will be 102 - * doing hv_kmsg_dump() with kmsg data later, don't do the notification 103 - * here. 129 + * Hyper-V should be notified only once about a panic/die. If we will 130 + * be calling hv_kmsg_dump() later with kmsg data, don't do the 131 + * notification here. 104 132 */ 105 133 if (hyperv_report_reg()) 106 - hyperv_report_panic(regs, val, true); 134 + hyperv_report_panic(regs, val, is_die); 135 + 107 136 return NOTIFY_DONE; 108 137 } 109 - 110 - static struct notifier_block hyperv_die_block = { 111 - .notifier_call = hyperv_die_event, 112 - }; 113 - static struct notifier_block hyperv_panic_block = { 114 - .notifier_call = hyperv_panic_event, 115 - }; 116 138 117 139 static const char *fb_mmio_name = "fb_range"; 118 140 static struct resource *fb_mmio; ··· 1558 1538 if (hyperv_crash_ctl & HV_CRASH_CTL_CRASH_NOTIFY_MSG) 1559 1539 hv_kmsg_dump_register(); 1560 1540 1561 - register_die_notifier(&hyperv_die_block); 1541 + register_die_notifier(&hyperv_die_report_block); 1542 + atomic_notifier_chain_register(&panic_notifier_list, 1543 + &hyperv_panic_report_block); 1562 1544 } 1563 1545 1564 1546 /* 1565 - * Always register the panic notifier because we need to unload 1566 - * the VMbus channel connection to prevent any VMbus 1567 - * activity after the VM panics. 1547 + * Always register the vmbus unload panic notifier because we 1548 + * need to shut the VMbus channel connection on panic. 1568 1549 */ 1569 1550 atomic_notifier_chain_register(&panic_notifier_list, 1570 - &hyperv_panic_block); 1551 + &hyperv_panic_vmbus_unload_block); 1571 1552 1572 1553 vmbus_request_offers(); 1573 1554 ··· 2821 2800 2822 2801 if (ms_hyperv.misc_features & HV_FEATURE_GUEST_CRASH_MSR_AVAILABLE) { 2823 2802 kmsg_dump_unregister(&hv_kmsg_dumper); 2824 - unregister_die_notifier(&hyperv_die_block); 2803 + unregister_die_notifier(&hyperv_die_report_block); 2804 + atomic_notifier_chain_unregister(&panic_notifier_list, 2805 + &hyperv_panic_report_block); 2825 2806 } 2826 2807 2827 2808 /* 2828 - * The panic notifier is always registered, hence we should 2809 + * The vmbus panic notifier is always registered, hence we should 2829 2810 * also unconditionally unregister it here as well. 2830 2811 */ 2831 2812 atomic_notifier_chain_unregister(&panic_notifier_list, 2832 - &hyperv_panic_block); 2813 + &hyperv_panic_vmbus_unload_block); 2833 2814 2834 2815 free_page((unsigned long)hv_panic_page); 2835 2816 unregister_sysctl_table(hv_ctl_table_hdr);
+8
drivers/video/fbdev/hyperv_fb.c
··· 1214 1214 par->fb_ready = true; 1215 1215 1216 1216 par->synchronous_fb = false; 1217 + 1218 + /* 1219 + * We need to be sure this panic notifier runs _before_ the 1220 + * vmbus disconnect, so order it by priority. It must execute 1221 + * before the function hv_panic_vmbus_unload() [drivers/hv/vmbus_drv.c], 1222 + * which is almost at the end of list, with priority = INT_MIN + 1. 1223 + */ 1217 1224 par->hvfb_panic_nb.notifier_call = hvfb_on_panic; 1225 + par->hvfb_panic_nb.priority = INT_MIN + 10, 1218 1226 atomic_notifier_chain_register(&panic_notifier_list, 1219 1227 &par->hvfb_panic_nb); 1220 1228