linux-kernel - Re: [RFC PATCH 17/23] watchdog/hardlockup/hpet: Convert the timer's interrupt to NMI

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Wed, 13 Jun 2018 11:40:00 +0200 (CEST)
From:   Thomas Gleixner <tglx@...utronix.de>
To:     Ricardo Neri <ricardo.neri-calderon@...ux.intel.com>
cc:     Ingo Molnar <mingo@...nel.org>, "H. Peter Anvin" <hpa@...or.com>,
        Andi Kleen <andi.kleen@...el.com>,
        Ashok Raj <ashok.raj@...el.com>, Borislav Petkov <bp@...e.de>,
        Tony Luck <tony.luck@...el.com>,
        "Ravi V. Shankar" <ravi.v.shankar@...el.com>, x86@...nel.org,
        sparclinux@...r.kernel.org, linuxppc-dev@...ts.ozlabs.org,
        linux-kernel@...r.kernel.org, Jacob Pan <jacob.jun.pan@...el.com>,
        "Rafael J. Wysocki" <rafael.j.wysocki@...el.com>,
        Don Zickus <dzickus@...hat.com>,
        Nicholas Piggin <npiggin@...il.com>,
        Michael Ellerman <mpe@...erman.id.au>,
        Frederic Weisbecker <frederic@...nel.org>,
        Alexei Starovoitov <ast@...nel.org>,
        Babu Moger <babu.moger@...cle.com>,
        Mathieu Desnoyers <mathieu.desnoyers@...icios.com>,
        Masami Hiramatsu <mhiramat@...nel.org>,
        Peter Zijlstra <peterz@...radead.org>,
        Andrew Morton <akpm@...ux-foundation.org>,
        Philippe Ombredanne <pombredanne@...b.com>,
        Colin Ian King <colin.king@...onical.com>,
        Byungchul Park <byungchul.park@....com>,
        "Paul E. McKenney" <paulmck@...ux.vnet.ibm.com>,
        "Luis R. Rodriguez" <mcgrof@...nel.org>,
        Waiman Long <longman@...hat.com>,
        Josh Poimboeuf <jpoimboe@...hat.com>,
        Randy Dunlap <rdunlap@...radead.org>,
        Davidlohr Bueso <dave@...olabs.net>,
        Christoffer Dall <cdall@...aro.org>,
        Marc Zyngier <marc.zyngier@....com>,
        Kai-Heng Feng <kai.heng.feng@...onical.com>,
        Konrad Rzeszutek Wilk <konrad.wilk@...cle.com>,
        David Rientjes <rientjes@...gle.com>,
        iommu@...ts.linux-foundation.org
Subject: Re: [RFC PATCH 17/23] watchdog/hardlockup/hpet: Convert the timer's
 interrupt to NMI

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