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
Hi H.,
On Wed, 29 May 2024 13:59:51 -0700, "H. Peter Anvin" <[email protected]> wrote:
> 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.
>
I thought about that as well, then I dropped that when I limit NMIS to
local interrupts. Not aware the hardcoded case as you mentioned.
I will reserve vector #2 to avoid future headache.
> 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.
>
Make sense, will set LINT1 vector to 2 and avoid polling known sources.
That is another optimization.
> 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].
Will sort them.
Thanks,
Jacob