| Message ID | 1528851463-21140-18-git-send-email-ricardo.neri-calderon@linux.intel.com |
|---|---|
| State | Not Applicable |
| Delegated to: | David Miller |
| Headers | show |
| Series | Implement an HPET-based hardlockup detector | expand |
On Tue, Jun 12, 2018 at 05:57:37PM -0700, Ricardo Neri wrote:
+static bool is_hpet_wdt_interrupt(struct hpet_hld_data *hdata)
+{
+ unsigned long this_isr;
+ unsigned int lvl_trig;
+
+ this_isr = hpet_readl(HPET_STATUS) & BIT(hdata->num);
+
+ lvl_trig = hpet_readl(HPET_Tn_CFG(hdata->num)) & HPET_TN_LEVEL;
+
+ if (lvl_trig && this_isr)
+ return true;
+
+ return false;
+}
> +static int hardlockup_detector_nmi_handler(unsigned int val,
> + struct pt_regs *regs)
> +{
> + struct hpet_hld_data *hdata = hld_data;
> + unsigned int use_fsb;
> +
> + /*
> + * If FSB delivery mode is used, the timer interrupt is programmed as
> + * edge-triggered and there is no need to check the ISR register.
> + */
> + use_fsb = hdata->flags & HPET_DEV_FSB_CAP;
Please do explain.. That FSB thing basically means MSI. But there's only
a single NMI vector. How do we know this NMI came from the HPET?
> +
> + if (!use_fsb && !is_hpet_wdt_interrupt(hdata))
So you add _2_ HPET reads for every single NMI that gets triggered...
and IIRC HPET reads are _sllooooowwwwww_.
> + return NMI_DONE;
> +
> + inspect_for_hardlockups(regs);
> +
> + if (!(hdata->flags & HPET_DEV_PERI_CAP))
> + kick_timer(hdata);
> +
> + /* Acknowledge interrupt if in level-triggered mode */
> + if (!use_fsb)
> + hpet_writel(BIT(hdata->num), HPET_STATUS);
> +
> + return NMI_HANDLED;
So if I read this right, when in FSB/MSI mode, we'll basically _always_
claim every single NMI as handled?
That's broken.
> +}
--
To unsubscribe from this list: send the line "unsubscribe sparclinux" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tue, 12 Jun 2018, Ricardo Neri wrote: > @@ -183,6 +184,8 @@ static irqreturn_t hardlockup_detector_irq_handler(int irq, void *data) > if (!(hdata->flags & HPET_DEV_PERI_CAP)) > kick_timer(hdata); > > + pr_err("This interrupt should not have happened. Ensure delivery mode is NMI.\n"); Eeew. > /** > + * hardlockup_detector_nmi_handler() - NMI Interrupt handler > + * @val: Attribute associated with the NMI. Not used. > + * @regs: Register values as seen when the NMI was asserted > + * > + * When an NMI is issued, look for hardlockups. If the timer is not periodic, > + * kick it. The interrupt is always handled when if delivered via the > + * Front-Side Bus. > + * > + * Returns: > + * > + * NMI_DONE if the HPET timer did not cause the interrupt. NMI_HANDLED > + * otherwise. > + */ > +static int hardlockup_detector_nmi_handler(unsigned int val, > + struct pt_regs *regs) > +{ > + struct hpet_hld_data *hdata = hld_data; > + unsigned int use_fsb; > + > + /* > + * If FSB delivery mode is used, the timer interrupt is programmed as > + * edge-triggered and there is no need to check the ISR register. > + */ > + use_fsb = hdata->flags & HPET_DEV_FSB_CAP; > + > + if (!use_fsb && !is_hpet_wdt_interrupt(hdata)) > + return NMI_DONE; So for 'use_fsb == True' every single NMI will fall through into the watchdog code below. > + inspect_for_hardlockups(regs); > + > + if (!(hdata->flags & HPET_DEV_PERI_CAP)) > + kick_timer(hdata); And in case that the HPET does not support periodic mode this reprogramms the timer on every NMI which means that while perf is running the watchdog will never ever detect anything. Aside of that, reading TWO HPET registers for every NMI is insane. HPET access is horribly slow, so any high frequency perf monitoring will take a massive performance hit. Thanks, tglx -- To unsubscribe from this list: send the line "unsubscribe sparclinux" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wed, Jun 13, 2018 at 11:40:00AM +0200, Thomas Gleixner wrote: > On Tue, 12 Jun 2018, Ricardo Neri wrote: > > @@ -183,6 +184,8 @@ static irqreturn_t hardlockup_detector_irq_handler(int irq, void *data) > > if (!(hdata->flags & HPET_DEV_PERI_CAP)) > > kick_timer(hdata); > > > > + pr_err("This interrupt should not have happened. Ensure delivery mode is NMI.\n"); > > Eeew. If you don't mind me asking. What is the problem with this error message? > > > /** > > + * hardlockup_detector_nmi_handler() - NMI Interrupt handler > > + * @val: Attribute associated with the NMI. Not used. > > + * @regs: Register values as seen when the NMI was asserted > > + * > > + * When an NMI is issued, look for hardlockups. If the timer is not periodic, > > + * kick it. The interrupt is always handled when if delivered via the > > + * Front-Side Bus. > > + * > > + * Returns: > > + * > > + * NMI_DONE if the HPET timer did not cause the interrupt. NMI_HANDLED > > + * otherwise. > > + */ > > +static int hardlockup_detector_nmi_handler(unsigned int val, > > + struct pt_regs *regs) > > +{ > > + struct hpet_hld_data *hdata = hld_data; > > + unsigned int use_fsb; > > + > > + /* > > + * If FSB delivery mode is used, the timer interrupt is programmed as > > + * edge-triggered and there is no need to check the ISR register. > > + */ > > + use_fsb = hdata->flags & HPET_DEV_FSB_CAP; > > + > > + if (!use_fsb && !is_hpet_wdt_interrupt(hdata)) > > + return NMI_DONE; > > So for 'use_fsb == True' every single NMI will fall through into the > watchdog code below. > > > + inspect_for_hardlockups(regs); > > + > > + if (!(hdata->flags & HPET_DEV_PERI_CAP)) > > + kick_timer(hdata); > > And in case that the HPET does not support periodic mode this reprogramms > the timer on every NMI which means that while perf is running the watchdog > will never ever detect anything. Yes. I see that this is wrong. With MSI interrupts, as far as I can see, there is not a way to make sure that the HPET timer caused the NMI perhaps the only option is to use an IO APIC interrupt and read the interrupt status register. > > Aside of that, reading TWO HPET registers for every NMI is insane. HPET > access is horribly slow, so any high frequency perf monitoring will take a > massive performance hit. If an IO APIC interrupt is used, only HPET register (the status register) would need to be read for every NMI. Would that be more acceptable? Otherwise, there is no way to determine if the HPET cause the NMI. Alternatively, there could be a counter that skips reading the HPET status register (and the detection of hardlockups) for every X NMIs. This would reduce the overall frequency of HPET register reads. Is that more acceptable? Thanks and BR, Ricardo -- To unsubscribe from this list: send the line "unsubscribe sparclinux" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wed, Jun 13, 2018 at 11:07:20AM +0200, Peter Zijlstra wrote: > On Tue, Jun 12, 2018 at 05:57:37PM -0700, Ricardo Neri wrote: > > +static bool is_hpet_wdt_interrupt(struct hpet_hld_data *hdata) > +{ > + unsigned long this_isr; > + unsigned int lvl_trig; > + > + this_isr = hpet_readl(HPET_STATUS) & BIT(hdata->num); > + > + lvl_trig = hpet_readl(HPET_Tn_CFG(hdata->num)) & HPET_TN_LEVEL; > + > + if (lvl_trig && this_isr) > + return true; > + > + return false; > +} > > > +static int hardlockup_detector_nmi_handler(unsigned int val, > > + struct pt_regs *regs) > > +{ > > + struct hpet_hld_data *hdata = hld_data; > > + unsigned int use_fsb; > > + > > + /* > > + * If FSB delivery mode is used, the timer interrupt is programmed as > > + * edge-triggered and there is no need to check the ISR register. > > + */ > > + use_fsb = hdata->flags & HPET_DEV_FSB_CAP; > > Please do explain.. That FSB thing basically means MSI. But there's only > a single NMI vector. How do we know this NMI came from the HPET? Indeed, I see now that this is wrong. There is no way to know. The only way is to use an IO APIC interrupt and read the HPET status register. > > > + > > + if (!use_fsb && !is_hpet_wdt_interrupt(hdata)) > > So you add _2_ HPET reads for every single NMI that gets triggered... > and IIRC HPET reads are _sllooooowwwwww_. Since the trigger mode of the HPET timer is not expected to change, perhaps is_hpet_wdt_interrupt() can only need the interrupt status register. This would reduce the reads to one. Furthermore, the hardlockup detector can skip an X number of NMIs and reduce further the frequency of reads. Does this make sense? > > > + return NMI_DONE; > > + > > + inspect_for_hardlockups(regs); > > + > > + if (!(hdata->flags & HPET_DEV_PERI_CAP)) > > + kick_timer(hdata); > > + > > + /* Acknowledge interrupt if in level-triggered mode */ > > + if (!use_fsb) > > + hpet_writel(BIT(hdata->num), HPET_STATUS); > > + > > + return NMI_HANDLED; > > So if I read this right, when in FSB/MSI mode, we'll basically _always_ > claim every single NMI as handled? > > That's broken. Yes, this is not correct. I will drop the functionality to use FSB/MSI mode. Thanks and BR, Ricardo -- To unsubscribe from this list: send the line "unsubscribe sparclinux" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Thu, 14 Jun 2018, Ricardo Neri wrote: > On Wed, Jun 13, 2018 at 11:40:00AM +0200, Thomas Gleixner wrote: > > On Tue, 12 Jun 2018, Ricardo Neri wrote: > > > @@ -183,6 +184,8 @@ static irqreturn_t hardlockup_detector_irq_handler(int irq, void *data) > > > if (!(hdata->flags & HPET_DEV_PERI_CAP)) > > > kick_timer(hdata); > > > > > > + pr_err("This interrupt should not have happened. Ensure delivery mode is NMI.\n"); > > > > Eeew. > > If you don't mind me asking. What is the problem with this error message? The problem is not the error message. The problem is the abuse of request_irq() and the fact that this irq handler function exists in the first place for something which is NMI based. > > And in case that the HPET does not support periodic mode this reprogramms > > the timer on every NMI which means that while perf is running the watchdog > > will never ever detect anything. > > Yes. I see that this is wrong. With MSI interrupts, as far as I can > see, there is not a way to make sure that the HPET timer caused the NMI > perhaps the only option is to use an IO APIC interrupt and read the > interrupt status register. > > > Aside of that, reading TWO HPET registers for every NMI is insane. HPET > > access is horribly slow, so any high frequency perf monitoring will take a > > massive performance hit. > > If an IO APIC interrupt is used, only HPET register (the status register) > would need to be read for every NMI. Would that be more acceptable? Otherwise, > there is no way to determine if the HPET cause the NMI. You need level trigger for the HPET status register to be useful at all because in edge mode the interrupt status bits read always 0. That means you have to fiddle with the IOAPIC acknowledge magic from NMI context. Brilliant idea. If the NMI hits in the middle of a regular io_apic_read() then the interrupted code will endup with the wrong index register. Not to talk about the fun which the affinity rotation from NMI context would bring. Do not even think about using IOAPIC and level for this. > Alternatively, there could be a counter that skips reading the HPET status > register (and the detection of hardlockups) for every X NMIs. This would > reduce the overall frequency of HPET register reads. Great plan. So if the watchdog is the only NMI (because perf is off) then you delay the watchdog detection by that count. You neither can do a time based check, because time might be corrupted and then you end up in lala land as well. Thanks, tglx -- To unsubscribe from this list: send the line "unsubscribe sparclinux" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Fri, Jun 15, 2018 at 11:19:09AM +0200, Thomas Gleixner wrote: > On Thu, 14 Jun 2018, Ricardo Neri wrote: > > On Wed, Jun 13, 2018 at 11:40:00AM +0200, Thomas Gleixner wrote: > > > On Tue, 12 Jun 2018, Ricardo Neri wrote: > > > > @@ -183,6 +184,8 @@ static irqreturn_t hardlockup_detector_irq_handler(int irq, void *data) > > > > if (!(hdata->flags & HPET_DEV_PERI_CAP)) > > > > kick_timer(hdata); > > > > > > > > + pr_err("This interrupt should not have happened. Ensure delivery mode is NMI.\n"); > > > > > > Eeew. > > > > If you don't mind me asking. What is the problem with this error message? > > The problem is not the error message. The problem is the abuse of > request_irq() and the fact that this irq handler function exists in the > first place for something which is NMI based. I wanted to add this handler in case the interrupt was not configured correctly to be delivered as NMI (e.g., not supported by the hardware). I see your point. Perhaps this is not needed. There is code in place to complain when an interrupt that nobody was expecting happens. > > > > And in case that the HPET does not support periodic mode this reprogramms > > > the timer on every NMI which means that while perf is running the watchdog > > > will never ever detect anything. > > > > Yes. I see that this is wrong. With MSI interrupts, as far as I can > > see, there is not a way to make sure that the HPET timer caused the NMI > > perhaps the only option is to use an IO APIC interrupt and read the > > interrupt status register. > > > > > Aside of that, reading TWO HPET registers for every NMI is insane. HPET > > > access is horribly slow, so any high frequency perf monitoring will take a > > > massive performance hit. > > > > If an IO APIC interrupt is used, only HPET register (the status register) > > would need to be read for every NMI. Would that be more acceptable? Otherwise, > > there is no way to determine if the HPET cause the NMI. > > You need level trigger for the HPET status register to be useful at all > because in edge mode the interrupt status bits read always 0. Indeed. > > That means you have to fiddle with the IOAPIC acknowledge magic from NMI > context. Brilliant idea. If the NMI hits in the middle of a regular > io_apic_read() then the interrupted code will endup with the wrong index > register. Not to talk about the fun which the affinity rotation from NMI > context would bring. > > Do not even think about using IOAPIC and level for this. OK. I will stay away of it and focus on MSI. > > > Alternatively, there could be a counter that skips reading the HPET status > > register (and the detection of hardlockups) for every X NMIs. This would > > reduce the overall frequency of HPET register reads. > > Great plan. So if the watchdog is the only NMI (because perf is off) then > you delay the watchdog detection by that count. OK. This was a bad idea. Then, is it acceptable to have an read to an HPET register per NMI just to check in the status register if the HPET timer caused the NMI? Thanks and BR, Ricardo -- To unsubscribe from this list: send the line "unsubscribe sparclinux" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Fri, 15 Jun 2018, Ricardo Neri wrote: > On Fri, Jun 15, 2018 at 11:19:09AM +0200, Thomas Gleixner wrote: > > On Thu, 14 Jun 2018, Ricardo Neri wrote: > > > Alternatively, there could be a counter that skips reading the HPET status > > > register (and the detection of hardlockups) for every X NMIs. This would > > > reduce the overall frequency of HPET register reads. > > > > Great plan. So if the watchdog is the only NMI (because perf is off) then > > you delay the watchdog detection by that count. > > OK. This was a bad idea. Then, is it acceptable to have an read to an HPET > register per NMI just to check in the status register if the HPET timer > caused the NMI? The status register is useless in case of MSI. MSI is edge triggered .... The only register which gives you proper information is the counter register itself. That adds an massive overhead to each NMI, because the counter register access is synchronized to the HPET clock with hardware magic. Plus on larger systems, the HPET access is cross node and even slower. Thanks, tglx -- To unsubscribe from this list: send the line "unsubscribe sparclinux" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Sat, Jun 16, 2018 at 03:24:49PM +0200, Thomas Gleixner wrote: > On Fri, 15 Jun 2018, Ricardo Neri wrote: > > On Fri, Jun 15, 2018 at 11:19:09AM +0200, Thomas Gleixner wrote: > > > On Thu, 14 Jun 2018, Ricardo Neri wrote: > > > > Alternatively, there could be a counter that skips reading the HPET status > > > > register (and the detection of hardlockups) for every X NMIs. This would > > > > reduce the overall frequency of HPET register reads. > > > > > > Great plan. So if the watchdog is the only NMI (because perf is off) then > > > you delay the watchdog detection by that count. > > > > OK. This was a bad idea. Then, is it acceptable to have an read to an HPET > > register per NMI just to check in the status register if the HPET timer > > caused the NMI? > > The status register is useless in case of MSI. MSI is edge triggered .... > > The only register which gives you proper information is the counter > register itself. That adds an massive overhead to each NMI, because the > counter register access is synchronized to the HPET clock with hardware > magic. Plus on larger systems, the HPET access is cross node and even > slower. It starts to sound that the HPET is too slow to drive the hardlockup detector. Would it be possible to envision a variant of this implementation? In this variant, the HPET only targets a single CPU. The actual hardlockup detector is implemented by this single CPU sending interprocessor interrupts to the rest of the CPUs. In this manner only one CPU has to deal with the slowness of the HPET; the rest of the CPUs don't have to read or write any HPET registers. A sysfs entry could be added to configure which CPU will have to deal with the HPET timer. However, profiling could not be done accurately on such CPU. Thanks and BR, Ricardo -- To unsubscribe from this list: send the line "unsubscribe sparclinux" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 06/19/2018 05:15 PM, Ricardo Neri wrote: > On Sat, Jun 16, 2018 at 03:24:49PM +0200, Thomas Gleixner wrote: >> On Fri, 15 Jun 2018, Ricardo Neri wrote: >>> On Fri, Jun 15, 2018 at 11:19:09AM +0200, Thomas Gleixner wrote: >>>> On Thu, 14 Jun 2018, Ricardo Neri wrote: >>>>> Alternatively, there could be a counter that skips reading the HPET status >>>>> register (and the detection of hardlockups) for every X NMIs. This would >>>>> reduce the overall frequency of HPET register reads. >>>> >>>> Great plan. So if the watchdog is the only NMI (because perf is off) then >>>> you delay the watchdog detection by that count. >>> >>> OK. This was a bad idea. Then, is it acceptable to have an read to an HPET >>> register per NMI just to check in the status register if the HPET timer >>> caused the NMI? >> >> The status register is useless in case of MSI. MSI is edge triggered .... >> >> The only register which gives you proper information is the counter >> register itself. That adds an massive overhead to each NMI, because the >> counter register access is synchronized to the HPET clock with hardware >> magic. Plus on larger systems, the HPET access is cross node and even >> slower. > > It starts to sound that the HPET is too slow to drive the hardlockup detector. > > Would it be possible to envision a variant of this implementation? In this > variant, the HPET only targets a single CPU. The actual hardlockup detector > is implemented by this single CPU sending interprocessor interrupts to the > rest of the CPUs. > > In this manner only one CPU has to deal with the slowness of the HPET; the > rest of the CPUs don't have to read or write any HPET registers. A sysfs > entry could be added to configure which CPU will have to deal with the HPET > timer. However, profiling could not be done accurately on such CPU. Please forgive my simple question: What happens when this one CPU is the one that locks up? thnx,
On Tue, 19 Jun 2018, Ricardo Neri wrote: > On Sat, Jun 16, 2018 at 03:24:49PM +0200, Thomas Gleixner wrote: > > The status register is useless in case of MSI. MSI is edge triggered .... > > > > The only register which gives you proper information is the counter > > register itself. That adds an massive overhead to each NMI, because the > > counter register access is synchronized to the HPET clock with hardware > > magic. Plus on larger systems, the HPET access is cross node and even > > slower. > > It starts to sound that the HPET is too slow to drive the hardlockup detector. > > Would it be possible to envision a variant of this implementation? In this > variant, the HPET only targets a single CPU. The actual hardlockup detector > is implemented by this single CPU sending interprocessor interrupts to the > rest of the CPUs. And these IPIs must be NMIs which need to have a software based indicator that the watchdog needs to be checked, which is going to create yet another can of race conditions and in the worst case 'unknown NMI' splats. Not pretty either. Thanks, tglx -- To unsubscribe from this list: send the line "unsubscribe sparclinux" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tue, Jun 19, 2018 at 05:25:09PM -0700, Randy Dunlap wrote: > On 06/19/2018 05:15 PM, Ricardo Neri wrote: > > On Sat, Jun 16, 2018 at 03:24:49PM +0200, Thomas Gleixner wrote: > >> On Fri, 15 Jun 2018, Ricardo Neri wrote: > >>> On Fri, Jun 15, 2018 at 11:19:09AM +0200, Thomas Gleixner wrote: > >>>> On Thu, 14 Jun 2018, Ricardo Neri wrote: > >>>>> Alternatively, there could be a counter that skips reading the HPET status > >>>>> register (and the detection of hardlockups) for every X NMIs. This would > >>>>> reduce the overall frequency of HPET register reads. > >>>> > >>>> Great plan. So if the watchdog is the only NMI (because perf is off) then > >>>> you delay the watchdog detection by that count. > >>> > >>> OK. This was a bad idea. Then, is it acceptable to have an read to an HPET > >>> register per NMI just to check in the status register if the HPET timer > >>> caused the NMI? > >> > >> The status register is useless in case of MSI. MSI is edge triggered .... > >> > >> The only register which gives you proper information is the counter > >> register itself. That adds an massive overhead to each NMI, because the > >> counter register access is synchronized to the HPET clock with hardware > >> magic. Plus on larger systems, the HPET access is cross node and even > >> slower. > > > > It starts to sound that the HPET is too slow to drive the hardlockup detector. > > > > Would it be possible to envision a variant of this implementation? In this > > variant, the HPET only targets a single CPU. The actual hardlockup detector > > is implemented by this single CPU sending interprocessor interrupts to the > > rest of the CPUs. > > > > In this manner only one CPU has to deal with the slowness of the HPET; the > > rest of the CPUs don't have to read or write any HPET registers. A sysfs > > entry could be added to configure which CPU will have to deal with the HPET > > timer. However, profiling could not be done accurately on such CPU. > > Please forgive my simple question: > > What happens when this one CPU is the one that locks up? I think that in this particular case this one CPU would check for hardlockups on itself when it receives the NMI from the HPET timer. It would also issue NMIs to the other monitored processors. Thanks and BR, Ricardo -- To unsubscribe from this list: send the line "unsubscribe sparclinux" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/arch/x86/kernel/hpet.c b/arch/x86/kernel/hpet.c index fda6e19..5ca1953 100644 --- a/arch/x86/kernel/hpet.c +++ b/arch/x86/kernel/hpet.c @@ -205,7 +205,7 @@ int hpet_hardlockup_detector_assign_legacy_irq(struct hpet_hld_data *hdata) break; } - gsi = acpi_register_gsi(NULL, hwirq, ACPI_LEVEL_SENSITIVE, + gsi = acpi_register_gsi(NULL, hwirq, ACPI_EDGE_SENSITIVE, ACPI_ACTIVE_LOW); if (gsi > 0) break; diff --git a/kernel/watchdog_hld_hpet.c b/kernel/watchdog_hld_hpet.c index 8fa4e55..3bedffa 100644 --- a/kernel/watchdog_hld_hpet.c +++ b/kernel/watchdog_hld_hpet.c @@ -10,6 +10,7 @@ #include <linux/nmi.h> #include <linux/hpet.h> #include <asm/hpet.h> +#include <asm/irq_remapping.h> #undef pr_fmt #define pr_fmt(fmt) "NMI hpet watchdog: " fmt @@ -183,6 +184,8 @@ static irqreturn_t hardlockup_detector_irq_handler(int irq, void *data) if (!(hdata->flags & HPET_DEV_PERI_CAP)) kick_timer(hdata); + pr_err("This interrupt should not have happened. Ensure delivery mode is NMI.\n"); + /* Acknowledge interrupt if in level-triggered mode */ if (!use_fsb) hpet_writel(BIT(hdata->num), HPET_STATUS); @@ -191,6 +194,47 @@ static irqreturn_t hardlockup_detector_irq_handler(int irq, void *data) } /** + * hardlockup_detector_nmi_handler() - NMI Interrupt handler + * @val: Attribute associated with the NMI. Not used. + * @regs: Register values as seen when the NMI was asserted + * + * When an NMI is issued, look for hardlockups. If the timer is not periodic, + * kick it. The interrupt is always handled when if delivered via the + * Front-Side Bus. + * + * Returns: + * + * NMI_DONE if the HPET timer did not cause the interrupt. NMI_HANDLED + * otherwise. + */ +static int hardlockup_detector_nmi_handler(unsigned int val, + struct pt_regs *regs) +{ + struct hpet_hld_data *hdata = hld_data; + unsigned int use_fsb; + + /* + * If FSB delivery mode is used, the timer interrupt is programmed as + * edge-triggered and there is no need to check the ISR register. + */ + use_fsb = hdata->flags & HPET_DEV_FSB_CAP; + + if (!use_fsb && !is_hpet_wdt_interrupt(hdata)) + return NMI_DONE; + + inspect_for_hardlockups(regs); + + if (!(hdata->flags & HPET_DEV_PERI_CAP)) + kick_timer(hdata); + + /* Acknowledge interrupt if in level-triggered mode */ + if (!use_fsb) + hpet_writel(BIT(hdata->num), HPET_STATUS); + + return NMI_HANDLED; +} + +/** * setup_irq_msi_mode() - Configure the timer to deliver an MSI interrupt * @data: Data associated with the instance of the HPET timer to configure * @@ -282,11 +326,20 @@ static int setup_hpet_irq(struct hpet_hld_data *hdata) if (ret) return ret; + /* Register the NMI handler, which will be the actual handler we use. */ + ret = register_nmi_handler(NMI_LOCAL, hardlockup_detector_nmi_handler, + 0, "hpet_hld"); + if (ret) + return ret; + /* * Request an interrupt to activate the irq in all the needed domains. */ ret = request_irq(hwirq, hardlockup_detector_irq_handler, - IRQF_TIMER, "hpet_hld", hdata); + IRQF_TIMER | IRQF_DELIVER_AS_NMI, + "hpet_hld", hdata); + if (ret) + unregister_nmi_handler(NMI_LOCAL, "hpet_hld"); return ret; }
In order to detect hardlockups, it is necessary to have the ability to receive interrupts even when disabled: a non-maskable interrupt is required. Add the flag IRQF_DELIVER_AS_NMI to the arguments of request_irq() for this purpose. Note that the timer, when programmed to deliver interrupts via the IO APIC is programmed as level-triggered. This is to have an indication that the NMI comes from HPET timer as indicated in the General Status Interrupt Register. However, NMIs are always edge-triggered, thus a GSI edge- triggered interrupt is now requested. An NMI handler is also implemented. The handler looks for hardlockups and kicks the timer. Cc: Ashok Raj <ashok.raj@intel.com> Cc: Andi Kleen <andi.kleen@intel.com> Cc: Tony Luck <tony.luck@intel.com> Cc: Borislav Petkov <bp@suse.de> Cc: Jacob Pan <jacob.jun.pan@intel.com> Cc: "Rafael J. Wysocki" <rafael.j.wysocki@intel.com> Cc: Don Zickus <dzickus@redhat.com> Cc: Nicholas Piggin <npiggin@gmail.com> Cc: Michael Ellerman <mpe@ellerman.id.au> Cc: Frederic Weisbecker <frederic@kernel.org> Cc: Alexei Starovoitov <ast@kernel.org> Cc: Babu Moger <babu.moger@oracle.com> Cc: Mathieu Desnoyers <mathieu.desnoyers@efficios.com> Cc: Masami Hiramatsu <mhiramat@kernel.org> Cc: Peter Zijlstra <peterz@infradead.org> Cc: Andrew Morton <akpm@linux-foundation.org> Cc: Philippe Ombredanne <pombredanne@nexb.com> Cc: Colin Ian King <colin.king@canonical.com> Cc: Byungchul Park <byungchul.park@lge.com> Cc: "Paul E. McKenney" <paulmck@linux.vnet.ibm.com> Cc: "Luis R. Rodriguez" <mcgrof@kernel.org> Cc: Waiman Long <longman@redhat.com> Cc: Josh Poimboeuf <jpoimboe@redhat.com> Cc: Randy Dunlap <rdunlap@infradead.org> Cc: Davidlohr Bueso <dave@stgolabs.net> Cc: Christoffer Dall <cdall@linaro.org> Cc: Marc Zyngier <marc.zyngier@arm.com> Cc: Kai-Heng Feng <kai.heng.feng@canonical.com> Cc: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com> Cc: David Rientjes <rientjes@google.com> Cc: "Ravi V. Shankar" <ravi.v.shankar@intel.com> Cc: x86@kernel.org Cc: iommu@lists.linux-foundation.org Signed-off-by: Ricardo Neri <ricardo.neri-calderon@linux.intel.com> --- arch/x86/kernel/hpet.c | 2 +- kernel/watchdog_hld_hpet.c | 55 +++++++++++++++++++++++++++++++++++++++++++++- 2 files changed, 55 insertions(+), 2 deletions(-)