Received-SPF: pass (google.com: domain of linux-kernel+bounces-194687-linux.lists.archive=gmail.com@vger.kernel.org designates 139.178.88.99 as permitted sender) client-ip=139.178.88.99;
Message-ID: <746fecd5-4c79-42f9-919e-912ec415e73f@zytor.com>
Date: Wed, 29 May 2024 13:59:51 -0700
Precedence: bulk
MIME-Version: 1.0
User-Agent: Mozilla Thunderbird
Subject: Re: [PATCH 2/6] x86/irq: Extend NMI handler registration interface to
 include source
To: Jacob Pan <jacob.jun.pan@linux.intel.com>, X86 Kernel <x86@kernel.org>,
        LKML <linux-kernel@vger.kernel.org>,
        Thomas Gleixner <tglx@linutronix.de>,
        Dave Hansen <dave.hansen@intel.com>, Ingo Molnar <mingo@redhat.com>,
        Borislav Petkov <bp@alien8.de>, linux-perf-users@vger.kernel.org,
        Peter Zijlstra <peterz@infradead.org>
Cc: Andi Kleen <andi.kleen@intel.com>, Xin Li <xin3.li@intel.com>
References: <20240529203325.3039243-1-jacob.jun.pan@linux.intel.com>
 <20240529203325.3039243-3-jacob.jun.pan@linux.intel.com>
Content-Language: en-US
From: "H. Peter Anvin" <hpa@zytor.com>
In-Reply-To: <20240529203325.3039243-3-jacob.jun.pan@linux.intel.com>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit

On 5/29/24 13:33, Jacob Pan wrote:
> diff --git a/arch/x86/include/asm/irq_vectors.h b/arch/x86/include/asm/irq_vectors.h
> index 13aea8fc3d45..b8388bc00cde 100644
> --- a/arch/x86/include/asm/irq_vectors.h
> +++ b/arch/x86/include/asm/irq_vectors.h
> @@ -105,6 +105,27 @@
>   
>   #define NR_VECTORS			 256
>   
> +/*
> + * The NMI senders specify the NMI source vector as an 8bit integer in their
> + * vector field with NMI delivery mode. A local APIC receiving an NMI will
> + * set the corresponding bit in a 16bit bitmask, which is accumulated until
> + * the NMI is delivered.
> + * When a sender didn't specify an NMI source vector the source vector will
> + * be 0, which will result in bit 0 of the bitmask being set. For out of
> + * bounds vectors >= 16 bit 0 will also be set.
> + * When bit 0 is set, system software must invoke all registered NMI handlers
> + * as if NMI source feature is not enabled.
> + */
> +#define NMI_SOURCE_VEC_UNKNOWN		0
> +#define NMI_SOURCE_VEC_PMI		1	/* PerfMon counters */
> +#define NMI_SOURCE_VEC_IPI_BT		2	/* CPU backtrace */
> +#define NMI_SOURCE_VEC_IPI_MCE		3	/* MCE injection */
> +#define NMI_SOURCE_VEC_IPI_KGDB		4
> +#define NMI_SOURCE_VEC_IPI_REBOOT	5	/* Crash reboot */
> +#define NMI_SOURCE_VEC_IPI_SMP_STOP	6	/* Panic stop CPU */
> +#define NMI_SOURCE_VEC_IPI_TEST		7	/* For remote and local IPIs*/
> +#define NR_NMI_SOURCE_VECTORS		8
> +

I would avoid using vector 2; it is at least remotely possible that some 
third-party chipset sends NMI messages with a hardcoded vector of 2. As 
long as you don't actively need that slot, it is better to avoid it.

Even better is to set the LINT1 (= external NMI) vector to 2 and treat 
bit 2 as "other"; use vector 2 for anything that you don't have an 
explicit vector for. You can treat a received bit 2 the same as bit 0, 
except that you can explicitly trust that any event assigned an explicit 
vector number is *NOT* triggered and so do not need to be polled.

I would also recommend sorting these in order of decreasing importance 
(other than 0 and 2). Although the current intent is there to be 16 
vectors indefinitely, defensive design would be to consider that number 
to be potentially variable (up to 64, obviously). Since any out-of-range 
vector will set bit 0, the code will still Just Work[TM].

	-hpa