Re: [PATCH v3] x86/hyperv: Handle unknown NMIs on one CPU when unknown_nmi_panic
Thomas Gleixner writes: > Vitaly, > > On Thu, 1 Dec 2016, Vitaly Kuznetsov wrote: > >> There is a feature in Hyper-V (Debug-VM --InjectNonMaskableInterrupt) which >> injects NMI to the guest. Prior to WS2016 the NMI is injected to all CPUs >> of the guest and WS2016 injects it to CPU0 only. When unknown_nmi_panic is >> enabled and we'd like to do kdump we need to perform some minimal cleanup >> so the kdump kernel will be able to initialize VMBus devices, this cleanup >> includes sending CHANNELMSG_UNLOAD to the host waiting for >> CHANNELMSG_UNLOAD_RESPONSE to arrive. WS2012R2 always sends the response >> to the CPU which was used to send CHANNELMSG_REQUESTOFFERS on VMBus module >> load and not to the CPU which is sending CHANNELMSG_UNLOAD. As we can't do >> any cross-CPU work reliably on crash we have vmbus_wait_for_unload() >> function which tries to read CHANNELMSG_UNLOAD_RESPONSE on all CPUs message >> pages and this sometimes works. It was discovered that in case the host >> wants to send more than one message to a secondary CPU (not the CPU running >> vmbus_wait_for_unload()) we're unable to get it as after reading the first >> message we're supposed to do EOMing by doing wrmsrl(HV_X64_MSR_EOM, 0) but >> this is per-CPU. I have a feeling that this was working some time ago when >> I implemented vmbus_wait_for_unload(), the host was re-trying to deliver a >> message even without wrmsrl() but apparently this doesn't work any more. >> Unfortunately there is not that much we can do when all CPUs get NMI as >> all but the first one are getting blocked with interrupts disabled. What we >> can do is limit processing unknown interrupts to the first CPU which gets >> it in case we're about to crash. > > This is completely unreadable and I really tried hard to make sense of it. > > Please structure it in a way that people who are not familiar with the > inner workings of hyperv can at least understand the problem you are trying > to solve and the concept of the solution w/o needing to figure out what all > the acronyms and constants actually mean. > > Also visual structuring in paragraphs helps readability a lot. > Got it, I'll try to do my best to make it readable. > AFAICT this tries to deal with different problems of different hypervisor > versions, but even that is unclear as you talk about version WS2016, > versions prior to WS2016 and then about WS2012R2 in particular. > > Another issue I have with this is: > > " I have a feeling that this was working " > > Changes like this are not about feelings. We want to have changes based on > facts. > The thing is that Hyper-V is a (proprietary) software which gets updates and I don't remember which particular updates were installed when I was imlementing vmbus_wait_for_unload() but as far as I remember it was always working on WS2012R2. Now I observe a different behavior ... -- Vitaly
Re: [PATCH v3] x86/hyperv: Handle unknown NMIs on one CPU when unknown_nmi_panic
Vitaly, On Thu, 1 Dec 2016, Vitaly Kuznetsov wrote: > There is a feature in Hyper-V (Debug-VM --InjectNonMaskableInterrupt) which > injects NMI to the guest. Prior to WS2016 the NMI is injected to all CPUs > of the guest and WS2016 injects it to CPU0 only. When unknown_nmi_panic is > enabled and we'd like to do kdump we need to perform some minimal cleanup > so the kdump kernel will be able to initialize VMBus devices, this cleanup > includes sending CHANNELMSG_UNLOAD to the host waiting for > CHANNELMSG_UNLOAD_RESPONSE to arrive. WS2012R2 always sends the response > to the CPU which was used to send CHANNELMSG_REQUESTOFFERS on VMBus module > load and not to the CPU which is sending CHANNELMSG_UNLOAD. As we can't do > any cross-CPU work reliably on crash we have vmbus_wait_for_unload() > function which tries to read CHANNELMSG_UNLOAD_RESPONSE on all CPUs message > pages and this sometimes works. It was discovered that in case the host > wants to send more than one message to a secondary CPU (not the CPU running > vmbus_wait_for_unload()) we're unable to get it as after reading the first > message we're supposed to do EOMing by doing wrmsrl(HV_X64_MSR_EOM, 0) but > this is per-CPU. I have a feeling that this was working some time ago when > I implemented vmbus_wait_for_unload(), the host was re-trying to deliver a > message even without wrmsrl() but apparently this doesn't work any more. > Unfortunately there is not that much we can do when all CPUs get NMI as > all but the first one are getting blocked with interrupts disabled. What we > can do is limit processing unknown interrupts to the first CPU which gets > it in case we're about to crash. This is completely unreadable and I really tried hard to make sense of it. Please structure it in a way that people who are not familiar with the inner workings of hyperv can at least understand the problem you are trying to solve and the concept of the solution w/o needing to figure out what all the acronyms and constants actually mean. Also visual structuring in paragraphs helps readability a lot. AFAICT this tries to deal with different problems of different hypervisor versions, but even that is unclear as you talk about version WS2016, versions prior to WS2016 and then about WS2012R2 in particular. Another issue I have with this is: " I have a feeling that this was working " Changes like this are not about feelings. We want to have changes based on facts. Thanks, tglx
[PATCH v3] x86/hyperv: Handle unknown NMIs on one CPU when unknown_nmi_panic
There is a feature in Hyper-V (Debug-VM --InjectNonMaskableInterrupt) which injects NMI to the guest. Prior to WS2016 the NMI is injected to all CPUs of the guest and WS2016 injects it to CPU0 only. When unknown_nmi_panic is enabled and we'd like to do kdump we need to perform some minimal cleanup so the kdump kernel will be able to initialize VMBus devices, this cleanup includes sending CHANNELMSG_UNLOAD to the host waiting for CHANNELMSG_UNLOAD_RESPONSE to arrive. WS2012R2 always sends the response to the CPU which was used to send CHANNELMSG_REQUESTOFFERS on VMBus module load and not to the CPU which is sending CHANNELMSG_UNLOAD. As we can't do any cross-CPU work reliably on crash we have vmbus_wait_for_unload() function which tries to read CHANNELMSG_UNLOAD_RESPONSE on all CPUs message pages and this sometimes works. It was discovered that in case the host wants to send more than one message to a secondary CPU (not the CPU running vmbus_wait_for_unload()) we're unable to get it as after reading the first message we're supposed to do EOMing by doing wrmsrl(HV_X64_MSR_EOM, 0) but this is per-CPU. I have a feeling that this was working some time ago when I implemented vmbus_wait_for_unload(), the host was re-trying to deliver a message even without wrmsrl() but apparently this doesn't work any more. Unfortunately there is not that much we can do when all CPUs get NMI as all but the first one are getting blocked with interrupts disabled. What we can do is limit processing unknown interrupts to the first CPU which gets it in case we're about to crash. Signed-off-by: Vitaly Kuznetsov Acked-by: K. Y. Srinivasan --- Changes since v2: - Supply NMI_UNKNOWN instead of NMI_LOCAL for register_nmi_handler() - Retain KY's ACK (hopefully stands with the above mentioned change) Changes since v1: - Put everything under #ifdef CONFIG_X86_LOCAL_APIC as unknown_nmi_panic is otherwise undefined. [kbuild test robot] - Add KY's Ack (hopefully stands with the above mentioned change) --- arch/x86/kernel/cpu/mshyperv.c | 24 1 file changed, 24 insertions(+) diff --git a/arch/x86/kernel/cpu/mshyperv.c b/arch/x86/kernel/cpu/mshyperv.c index 8f44c5a..f228f74 100644 --- a/arch/x86/kernel/cpu/mshyperv.c +++ b/arch/x86/kernel/cpu/mshyperv.c @@ -31,6 +31,7 @@ #include #include #include +#include struct ms_hyperv_info ms_hyperv; EXPORT_SYMBOL_GPL(ms_hyperv); @@ -158,6 +159,26 @@ static unsigned char hv_get_nmi_reason(void) return 0; } +#ifdef CONFIG_X86_LOCAL_APIC +/* + * Prior to WS2016 Debug-VM sends NMIs to all CPUs which makes + * it dificult to process CHANNELMSG_UNLOAD in case of crash. Handle + * unknown NMI on the first CPU which gets it. + */ +static int hv_nmi_unknown(unsigned int val, struct pt_regs *regs) +{ + static atomic_t nmi_cpu = ATOMIC_INIT(-1); + + if (!unknown_nmi_panic) + return NMI_DONE; + + if (atomic_cmpxchg(&nmi_cpu, -1, raw_smp_processor_id()) != -1) + return NMI_HANDLED; + + return NMI_DONE; +} +#endif + static void __init ms_hyperv_init_platform(void) { /* @@ -183,6 +204,9 @@ static void __init ms_hyperv_init_platform(void) pr_info("HyperV: LAPIC Timer Frequency: %#x\n", lapic_timer_frequency); } + + register_nmi_handler(NMI_UNKNOWN, hv_nmi_unknown, NMI_FLAG_FIRST, +"hv_nmi_unknown"); #endif if (ms_hyperv.features & HV_X64_MSR_TIME_REF_COUNT_AVAILABLE) -- 2.9.3