linux-kernel - Re: [PATCH v5 4/9] x86/nmi: Assign and register NMI-source vectors

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]

Message-ID: <3a9ebdf4-1a52-4659-84ad-2ee015b453f8@intel.com>
Date: Wed, 7 May 2025 13:43:24 -0700
From: Sohil Mehta <sohil.mehta@...el.com>
To: Peter Zijlstra <peterz@...radead.org>
CC: <x86@...nel.org>, <linux-kernel@...r.kernel.org>, Xin Li <xin@...or.com>,
	"H . Peter Anvin" <hpa@...or.com>, Andy Lutomirski <luto@...nel.org>, "Thomas
 Gleixner" <tglx@...utronix.de>, Ingo Molnar <mingo@...hat.com>, "Borislav
 Petkov" <bp@...en8.de>, Dave Hansen <dave.hansen@...ux.intel.com>, "Sean
 Christopherson" <seanjc@...gle.com>, Arnaldo Carvalho de Melo
	<acme@...nel.org>, Namhyung Kim <namhyung@...nel.org>, Mark Rutland
	<mark.rutland@....com>, Alexander Shishkin
	<alexander.shishkin@...ux.intel.com>, Jiri Olsa <jolsa@...nel.org>, "Ian
 Rogers" <irogers@...gle.com>, Adrian Hunter <adrian.hunter@...el.com>, "Kan
 Liang" <kan.liang@...ux.intel.com>, Tony Luck <tony.luck@...el.com>, "Paolo
 Bonzini" <pbonzini@...hat.com>, Vitaly Kuznetsov <vkuznets@...hat.com>,
	"Rafael J . Wysocki" <rafael@...nel.org>, Daniel Lezcano
	<daniel.lezcano@...aro.org>, Zhang Rui <rui.zhang@...el.com>, Lukasz Luba
	<lukasz.luba@....com>, Steven Rostedt <rostedt@...dmis.org>, Masami Hiramatsu
	<mhiramat@...nel.org>, Mathieu Desnoyers <mathieu.desnoyers@...icios.com>,
	Brian Gerst <brgerst@...il.com>, Andrew Cooper <andrew.cooper3@...rix.com>,
	"Kirill A . Shutemov" <kirill.shutemov@...ux.intel.com>, Jacob Pan
	<jacob.pan@...ux.microsoft.com>, Andi Kleen <ak@...ux.intel.com>, Kai Huang
	<kai.huang@...el.com>, Nikolay Borisov <nik.borisov@...e.com>,
	<linux-perf-users@...r.kernel.org>, <linux-edac@...r.kernel.org>,
	<kvm@...r.kernel.org>, <linux-pm@...r.kernel.org>,
	<linux-trace-kernel@...r.kernel.org>
Subject: Re: [PATCH v5 4/9] x86/nmi: Assign and register NMI-source vectors

On 5/7/2025 1:22 AM, Peter Zijlstra wrote:
> On Tue, May 06, 2025 at 06:21:40PM -0700, Sohil Mehta wrote:

>> + */
>> +#define NMIS_VECTOR_NONE	0	/* Reserved - Set for all unidentified sources */
>> +#define NMIS_VECTOR_TEST	1	/* NMI selftest */
>> +#define NMIS_VECTOR_EXTERNAL	2	/* Reserved - Match External NMI vector 2 */
>> +#define NMIS_VECTOR_SMP_STOP	3	/* Panic stop CPU */
>> +#define NMIS_VECTOR_BT		4	/* CPU backtrace */
>> +#define NMIS_VECTOR_KGDB	5	/* Kernel debugger */
>> +#define NMIS_VECTOR_MCE		6	/* MCE injection */
>> +#define NMIS_VECTOR_PMI		7	/* PerfMon counters */
>> +
>> +#define NMIS_VECTORS_MAX	16	/* Maximum number of NMI-source vectors */
> 
> Are these really independent NMI vectors, or simply NMI source reporting bits?
> 
> Because if they are not NMI vectors, naming them such is confusing.
> 
> Specifically, is there a latch per source?
> 

Yes, they are truly vectors, confirmed with HPA that there is one latch
per source. Also, while generating the NMIs these values are used in the
APIC code to program the ICR vector field as shown.

ICR[7:0]  — Vector         -> NMIS_VECTOR_BT (4)
ICR[10:8] — Delivery Mode  -> APIC_DM_NMI    (100)

>> diff --git a/arch/x86/kernel/nmi.c b/arch/x86/kernel/nmi.c
>> index be93ec7255bf..a1d672dcb6f0 100644
>> --- a/arch/x86/kernel/nmi.c
>> +++ b/arch/x86/kernel/nmi.c
>> @@ -184,6 +184,11 @@ int __register_nmi_handler(unsigned int type, struct nmiaction *action)
>>  
>>  	raw_spin_lock_irqsave(&desc->lock, flags);
>>  
>> +	WARN_ON_ONCE(action->source_vector >= NMIS_VECTORS_MAX);
>> +
>> +	if (!cpu_feature_enabled(X86_FEATURE_NMI_SOURCE) || type != NMI_LOCAL)
>> +		action->source_vector = 0;
> 
> How about:
> 
> 	WARN_ON_ONCE(type != NMI_LOCAL && action->source_vector);
> 

This should work fine as well.

I don't see any harm in storing the source_vector unconditionally even
if X86_FEATURE_NMI_SOURCE is disabled.