Program designated NMI source vectors for all NMI delivered IPIs
such that their handlers can be selectively invoked.
Signed-off-by: Jacob Pan <jacob.jun.pan@linux.intel.com>
---
v4: Enhance comments, no functional changes (Li Xin)
---
arch/x86/include/asm/irq_vectors.h | 10 ++++++++++
arch/x86/kernel/apic/hw_nmi.c | 3 ++-
arch/x86/kernel/apic/ipi.c | 4 ++--
arch/x86/kernel/apic/local.h | 18 ++++++++++++------
arch/x86/kernel/cpu/mce/inject.c | 2 +-
arch/x86/kernel/kgdb.c | 2 +-
arch/x86/kernel/nmi_selftest.c | 2 +-
arch/x86/kernel/reboot.c | 2 +-
arch/x86/kernel/smp.c | 2 +-
9 files changed, 31 insertions(+), 14 deletions(-)
diff --git a/arch/x86/include/asm/irq_vectors.h b/arch/x86/include/asm/irq_vectors.h
index 4f767c3940d6..9b7241e7faa3 100644
--- a/arch/x86/include/asm/irq_vectors.h
+++ b/arch/x86/include/asm/irq_vectors.h
@@ -135,6 +135,16 @@
#define NMI_SOURCE_VEC_IPI_TEST 8 /* For remote and local IPIs */
#define NR_NMI_SOURCE_VECTORS 9
+/*
+ * When programming the local APIC, IDT NMI vector and NMI-source vector
+ * are encoded in a single 32 bit variable. The top 16 bits contain
+ * the NMI-source vector and the bottom 16 bits contain NMI_VECTOR (2)
+ * The top 16 bits are always zero when NMI-source reporting feature
+ * is not enabled or the caller does not use NMI-source reporting.
+ */
+#define NMI_VECTOR_WITH_SOURCE(src) (NMI_VECTOR | (src << 16))
+#define NMI_SOURCE_VEC_MASK GENMASK(15, 0)
+
#ifdef CONFIG_X86_LOCAL_APIC
#define FIRST_SYSTEM_VECTOR POSTED_MSI_NOTIFICATION_VECTOR
#else
diff --git a/arch/x86/kernel/apic/hw_nmi.c b/arch/x86/kernel/apic/hw_nmi.c
index 9f0125d3b8b0..f73ca95d961e 100644
--- a/arch/x86/kernel/apic/hw_nmi.c
+++ b/arch/x86/kernel/apic/hw_nmi.c
@@ -20,6 +20,7 @@
#include <linux/nmi.h>
#include <linux/init.h>
#include <linux/delay.h>
+#include <asm/irq_vectors.h>
#include "local.h"
@@ -33,7 +34,7 @@ u64 hw_nmi_get_sample_period(int watchdog_thresh)
#ifdef arch_trigger_cpumask_backtrace
static void nmi_raise_cpu_backtrace(cpumask_t *mask)
{
- __apic_send_IPI_mask(mask, NMI_VECTOR);
+ __apic_send_IPI_mask(mask, NMI_VECTOR_WITH_SOURCE(NMI_SOURCE_VEC_IPI_BT));
}
void arch_trigger_cpumask_backtrace(const cpumask_t *mask, int exclude_cpu)
diff --git a/arch/x86/kernel/apic/ipi.c b/arch/x86/kernel/apic/ipi.c
index 5da693d633b7..9d2b18e58758 100644
--- a/arch/x86/kernel/apic/ipi.c
+++ b/arch/x86/kernel/apic/ipi.c
@@ -157,7 +157,7 @@ static void __default_send_IPI_shortcut(unsigned int shortcut, int vector)
* issues where otherwise the system hangs when the panic CPU tries
* to stop the others before launching the kdump kernel.
*/
- if (unlikely(vector == NMI_VECTOR))
+ if (unlikely(is_nmi_vector(vector)))
apic_mem_wait_icr_idle_timeout();
else
apic_mem_wait_icr_idle();
@@ -174,7 +174,7 @@ void __default_send_IPI_dest_field(unsigned int dest_mask, int vector,
unsigned int dest_mode)
{
/* See comment in __default_send_IPI_shortcut() */
- if (unlikely(vector == NMI_VECTOR))
+ if (unlikely(is_nmi_vector(vector)))
apic_mem_wait_icr_idle_timeout();
else
apic_mem_wait_icr_idle();
diff --git a/arch/x86/kernel/apic/local.h b/arch/x86/kernel/apic/local.h
index 842fe28496be..60e90b7bf058 100644
--- a/arch/x86/kernel/apic/local.h
+++ b/arch/x86/kernel/apic/local.h
@@ -12,6 +12,7 @@
#include <asm/irq_vectors.h>
#include <asm/apic.h>
+#include <asm/nmi.h>
/* X2APIC */
void __x2apic_send_IPI_dest(unsigned int apicid, int vector, unsigned int dest);
@@ -26,19 +27,24 @@ extern u32 x2apic_max_apicid;
DECLARE_STATIC_KEY_FALSE(apic_use_ipi_shorthand);
+static inline bool is_nmi_vector(int vector)
+{
+ return (vector & NMI_SOURCE_VEC_MASK) == NMI_VECTOR;
+}
+
static inline unsigned int __prepare_ICR(unsigned int shortcut, int vector,
unsigned int dest)
{
unsigned int icr = shortcut | dest;
- switch (vector) {
- default:
- icr |= APIC_DM_FIXED | vector;
- break;
- case NMI_VECTOR:
+ if (is_nmi_vector(vector)) {
icr |= APIC_DM_NMI;
- break;
+ if (cpu_feature_enabled(X86_FEATURE_NMI_SOURCE))
+ icr |= vector >> 16;
+ } else {
+ icr |= APIC_DM_FIXED | vector;
}
+
return icr;
}
diff --git a/arch/x86/kernel/cpu/mce/inject.c b/arch/x86/kernel/cpu/mce/inject.c
index 365a03f11d06..07bc6c29bd83 100644
--- a/arch/x86/kernel/cpu/mce/inject.c
+++ b/arch/x86/kernel/cpu/mce/inject.c
@@ -270,7 +270,7 @@ static void __maybe_unused raise_mce(struct mce *m)
mce_irq_ipi, NULL, 0);
preempt_enable();
} else if (m->inject_flags & MCJ_NMI_BROADCAST)
- __apic_send_IPI_mask(mce_inject_cpumask, NMI_VECTOR);
+ __apic_send_IPI_mask(mce_inject_cpumask, NMI_VECTOR_WITH_SOURCE(NMI_SOURCE_VEC_IPI_MCE));
}
start = jiffies;
while (!cpumask_empty(mce_inject_cpumask)) {
diff --git a/arch/x86/kernel/kgdb.c b/arch/x86/kernel/kgdb.c
index d167eb23cf13..02198cf9fe21 100644
--- a/arch/x86/kernel/kgdb.c
+++ b/arch/x86/kernel/kgdb.c
@@ -416,7 +416,7 @@ static void kgdb_disable_hw_debug(struct pt_regs *regs)
*/
void kgdb_roundup_cpus(void)
{
- apic_send_IPI_allbutself(NMI_VECTOR);
+ apic_send_IPI_allbutself(NMI_VECTOR_WITH_SOURCE(NMI_SOURCE_VEC_IPI_KGDB));
}
#endif
diff --git a/arch/x86/kernel/nmi_selftest.c b/arch/x86/kernel/nmi_selftest.c
index f014c8a66b0c..5aa122d3368c 100644
--- a/arch/x86/kernel/nmi_selftest.c
+++ b/arch/x86/kernel/nmi_selftest.c
@@ -76,7 +76,7 @@ static void __init test_nmi_ipi(struct cpumask *mask)
/* sync above data before sending NMI */
wmb();
- __apic_send_IPI_mask(mask, NMI_VECTOR);
+ __apic_send_IPI_mask(mask, NMI_VECTOR_WITH_SOURCE(NMI_SOURCE_VEC_IPI_TEST));
/* Don't wait longer than a second */
timeout = USEC_PER_SEC;
diff --git a/arch/x86/kernel/reboot.c b/arch/x86/kernel/reboot.c
index acc19c1d3b4f..fb63bc0d6a0f 100644
--- a/arch/x86/kernel/reboot.c
+++ b/arch/x86/kernel/reboot.c
@@ -918,7 +918,7 @@ void nmi_shootdown_cpus(nmi_shootdown_cb callback)
*/
wmb();
- apic_send_IPI_allbutself(NMI_VECTOR);
+ apic_send_IPI_allbutself(NMI_VECTOR_WITH_SOURCE(NMI_SOURCE_VEC_IPI_REBOOT));
/* Kick CPUs looping in NMI context. */
WRITE_ONCE(crash_ipi_issued, 1);
diff --git a/arch/x86/kernel/smp.c b/arch/x86/kernel/smp.c
index f27469e40141..b79e78762a73 100644
--- a/arch/x86/kernel/smp.c
+++ b/arch/x86/kernel/smp.c
@@ -217,7 +217,7 @@ static void native_stop_other_cpus(int wait)
pr_emerg("Shutting down cpus with NMI\n");
for_each_cpu(cpu, &cpus_stop_mask)
- __apic_send_IPI(cpu, NMI_VECTOR);
+ __apic_send_IPI(cpu, NMI_VECTOR_WITH_SOURCE(NMI_SOURCE_VEC_IPI_SMP_STOP));
}
/*
* Don't wait longer than 10 ms if the caller didn't
--
2.25.1On Tue, Jul 09, 2024, Jacob Pan wrote:
> Program designated NMI source vectors for all NMI delivered IPIs
> such that their handlers can be selectively invoked.
>
> Signed-off-by: Jacob Pan <jacob.jun.pan@linux.intel.com>
> ---
> v4: Enhance comments, no functional changes (Li Xin)
> ---
> arch/x86/include/asm/irq_vectors.h | 10 ++++++++++
> arch/x86/kernel/apic/hw_nmi.c | 3 ++-
> arch/x86/kernel/apic/ipi.c | 4 ++--
> arch/x86/kernel/apic/local.h | 18 ++++++++++++------
> arch/x86/kernel/cpu/mce/inject.c | 2 +-
> arch/x86/kernel/kgdb.c | 2 +-
> arch/x86/kernel/nmi_selftest.c | 2 +-
> arch/x86/kernel/reboot.c | 2 +-
> arch/x86/kernel/smp.c | 2 +-
> 9 files changed, 31 insertions(+), 14 deletions(-)
>
> diff --git a/arch/x86/include/asm/irq_vectors.h b/arch/x86/include/asm/irq_vectors.h
> index 4f767c3940d6..9b7241e7faa3 100644
> --- a/arch/x86/include/asm/irq_vectors.h
> +++ b/arch/x86/include/asm/irq_vectors.h
> @@ -135,6 +135,16 @@
> #define NMI_SOURCE_VEC_IPI_TEST 8 /* For remote and local IPIs */
> #define NR_NMI_SOURCE_VECTORS 9
>
> +/*
> + * When programming the local APIC, IDT NMI vector and NMI-source vector
> + * are encoded in a single 32 bit variable. The top 16 bits contain
> + * the NMI-source vector and the bottom 16 bits contain NMI_VECTOR (2)
> + * The top 16 bits are always zero when NMI-source reporting feature
> + * is not enabled or the caller does not use NMI-source reporting.
This is *extremely* misleading, bordering on being an outright lie. The vectors
are not encoded in a single 32-bit variable when programming the _local APIC_,
that is 100% an arbitrary software construct. The actually write to APIC.ICR
morphs bits 15:0 into the TYPE, and the bits 31:16 into the VECTOR.
> + */
> +#define NMI_VECTOR_WITH_SOURCE(src) (NMI_VECTOR | (src << 16))
Why require callers to use NMI_VECTOR_WITH_SOURCE instead of having macros to
directly encode each source NMI, a la APIC_PERF_NMI?
To me, this:
__apic_send_IPI(cpu, NMI_VECTOR_WITH_SOURCE(NMI_SOURCE_VEC_IPI_SMP_STOP));
is *way* harder to read/parse than:
__apic_send_IPI(cpu, NMI_VECTOR_SMP_STOP);
especially since that first one blasts way past 80 chars (yeah, I know 80 is now
a soft limit, but it's still nice to keep line lengths short when possible).
> +#define NMI_SOURCE_VEC_MASK GENMASK(15, 0)
IMO, this is an absolutely awful encoding scheme. Vectors are 8-bit values, so
why on earth use 16 bits? And @vector is passed along as a _signed_ integer,
which means 32-bit kernels could end up observing negative values, which probably
isn't problematic in practice, but it's unnecessarily confusing.
All this FRED stuff is hard enough to follow given the specs have been rolled out
piecemeal (someone at Intel must get paid based on how many specs they publish),
using a software-defined scheme when FRED is already overloading a decades old
hardware-defined encoding is just mean.
Why not encode APIC_DM_NMI straightaway? You're already making it a hard
requirement that the backend (__prepare_ICR()) be able to handle a @vector that
has bits 31:8!=0. I don't see how the above scheme provides any value.
Side topic, APIC_DM_FIXED_MASK should really be APIC_DM_MASK, that "FIXED" part
is completely wrong.
E.g.
static inline unsigned int __prepare_ICR(unsigned int shortcut, int vector,
unsigned int dest)
{
unsigned int icr = shortcut | dest;
/*
* Callers are allowed to encode the NMI delivery mode directly, which
* allows using the vector field to provide the NMI source (FRED only).
*/
if ((vector & APIC_DM_MASK) == APIC_DM_NMI) {
icr |= vector;
/*
* Pre-FRED, the actual vector is ignored for NMIs, but zero it
* if NMI source reporting is unsupported so as not to risk
* breakage on misbehaving hardware/hypervisors.
*/
if (!cpu_feature_enabled(X86_FEATURE_NMI_SOURCE))
icr &= ~APIC_VECTOR_MASK;
} else if (vector == NMI_VECTOR) {
icr |= APIC_DM_NMI;
} else {
icr |= APIC_DM_FIXED | vector;
}
return icr;
}
and then NMI_VECTOR_SMP_STOP would be (APIC_DM_NMI | NMI_SOURCE_VEC_IPI_SMP_STOP).
© 2016 - 2025 Red Hat, Inc.