On Thu, Jun 12, 2025, Sohil Mehta wrote:
> With the IPI handling APIs ready to support the new NMI encoding, encode
> the NMI delivery mode directly with the NMI-source vectors to trigger
> NMIs.
>
> Move most of the existing NMI-based IPIs to use the new NMI-source
> vectors, except for the microcode rendezvous NMI and the crash reboot
> NMI. NMI handling for them is special-cased in exc_nmi() and does not
> need NMI-source reporting.
>
> However, in the future, it might be useful to assign a source vector to
> all NMI sources to improve isolation and debuggability.
>
> Originally-by: Jacob Pan <jacob.jun.pan@xxxxxxxxxxxxxxx>
> Suggested-by: Sean Christopherson <seanjc@xxxxxxxxxx>
> Co-developed-by: Xin Li (Intel) <xin@xxxxxxxxx>
> Signed-off-by: Xin Li (Intel) <xin@xxxxxxxxx>
> Signed-off-by: Sohil Mehta <sohil.mehta@xxxxxxxxx>
> ---
> v7: No change.
>
> v6: Include asm/nmi.h to avoid compile errors. (LKP)
>
> v5: Encode APIC_DM_NMI directly with the NMI-source vector.
> ---
> arch/x86/include/asm/apic.h | 8 ++++++++
> arch/x86/kernel/apic/hw_nmi.c | 2 +-
> arch/x86/kernel/cpu/mce/inject.c | 2 +-
> arch/x86/kernel/kgdb.c | 2 +-
> arch/x86/kernel/nmi_selftest.c | 2 +-
> arch/x86/kernel/smp.c | 2 +-
> 6 files changed, 13 insertions(+), 5 deletions(-)
>
> diff --git a/arch/x86/include/asm/apic.h b/arch/x86/include/asm/apic.h
> index 32cdd81e5e45..5789df1708bd 100644
> --- a/arch/x86/include/asm/apic.h
> +++ b/arch/x86/include/asm/apic.h
> @@ -14,6 +14,7 @@
> #include <asm/msr.h>
> #include <asm/hardirq.h>
> #include <asm/io.h>
> +#include <asm/nmi.h>
> #include <asm/posted_intr.h>
>
> #define ARCH_APICTIMER_STOPS_ON_C3 1
> @@ -23,6 +24,13 @@
> #define APIC_EXTNMI_ALL 1
> #define APIC_EXTNMI_NONE 2
>
> +/* Trigger NMIs with source information */
> +#define TEST_NMI (APIC_DM_NMI | NMIS_VECTOR_TEST)
> +#define SMP_STOP_NMI (APIC_DM_NMI | NMIS_VECTOR_SMP_STOP)
> +#define BT_NMI (APIC_DM_NMI | NMIS_VECTOR_BT)
s/BT/BACKTRACE?
> +#define KGDB_NMI (APIC_DM_NMI | NMIS_VECTOR_KGDB)
> +#define MCE_NMI (APIC_DM_NMI | NMIS_VECTOR_MCE)
IMO, NMI_xxx reads better, e.g. it's easier to see that code is sending an NMI
at the call sites.
> +
> /*
> * Debugging macros
> */
> diff --git a/arch/x86/kernel/apic/hw_nmi.c b/arch/x86/kernel/apic/hw_nmi.c
> index 4e04f13d2de9..586f4b25feae 100644
> --- a/arch/x86/kernel/apic/hw_nmi.c
> +++ b/arch/x86/kernel/apic/hw_nmi.c
> @@ -33,7 +33,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, BT_NMI);
This patch is buggy. There are at least two implementations of ->send_IPI_mask()
that this breaks:
uv_send_IPI_mask() = > uv_send_IPI_one():
if (vector == NMI_VECTOR)
dmode = APIC_DELIVERY_MODE_NMI;
else
dmode = APIC_DELIVERY_MODE_FIXED;
and xen_send_IPI_mask() => xen_map_vector():
switch (vector) {
case RESCHEDULE_VECTOR:
xen_vector = XEN_RESCHEDULE_VECTOR;
break;
case CALL_FUNCTION_VECTOR:
xen_vector = XEN_CALL_FUNCTION_VECTOR;
break;
case CALL_FUNCTION_SINGLE_VECTOR:
xen_vector = XEN_CALL_FUNCTION_SINGLE_VECTOR;
break;
case IRQ_WORK_VECTOR:
xen_vector = XEN_IRQ_WORK_VECTOR;
break;
#ifdef CONFIG_X86_64
case NMI_VECTOR:
case APIC_DM_NMI: /* Some use that instead of NMI_VECTOR */
xen_vector = XEN_NMI_VECTOR;
break;
#endif
default:
xen_vector = -1;
printk(KERN_ERR "xen: vector 0x%x is not implemented\n",
vector);
}
return xen_vector;
Looking at all of this again, shoving the NMI source information into the @vector
is quite brittle. Nothing forces implementations to handle embedded delivery
mode information.
One thought would be to pass a small struct (by value), and then provide macros
to generate the structure for a specific vector. That provides some amount of
type safety and should make it a bit harder to pass in garbage, without making
the callers any less readable.
struct apic_ipi {
u8 vector;
u8 type;
};
#define APIC_IPI(v, t) ({ struct apic_ipi i = { .vector = v, .type = t }; i; })
#define APIC_IPI_IRQ(vector) APIC_IPI(vector, APIC_DELIVERY_MODE_FIXED)
#define APIC_IPI_NMI(vector) APIC_IPI(vector, APIC_DELIVERY_MODE_NMI)
#define IPI_IRQ_WORK APIC_IPI_IRQ(IRQ_WORK_VECTOR)
#define IPI_POSTED_INTR_WAKEUP APIC_IPI_IRQ(POSTED_INTR_WAKEUP_VECTOR)
#define IPI_NMI_BACKTRACE APIC_IPI_NMI(NMI_BACKTRACE_VECTOR)
static __always_inline void __apic_send_IPI_self(struct apic_ipi ipi)
