| 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
| ||
|
Message-ID: <aG1laKXYu7Uc4Tsb@google.com>
Date: Tue, 8 Jul 2025 11:37:28 -0700
From: Sean Christopherson <seanjc@...gle.com>
To: Sohil Mehta <sohil.mehta@...el.com>
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>, Peter Zijlstra <peterz@...radead.org>,
Adrian Hunter <adrian.hunter@...el.com>, Kan Liang <kan.liang@...ux.intel.com>,
Tony Luck <tony.luck@...el.com>, Zhang Rui <rui.zhang@...el.com>,
Steven Rostedt <rostedt@...dmis.org>, 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>,
Sandipan Das <sandipan.das@....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 v7 08/10] x86/nmi: Enable NMI-source for IPIs delivered as NMIs
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@...ux.intel.com>
> Suggested-by: Sean Christopherson <seanjc@...gle.com>
> Co-developed-by: Xin Li (Intel) <xin@...or.com>
> Signed-off-by: Xin Li (Intel) <xin@...or.com>
> Signed-off-by: Sohil Mehta <sohil.mehta@...el.com>
> ---
> 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)
Powered by blists - more mailing lists