1. Patchew
  2. linux
  3. [PATCH 0/6] Add support for NMI source reporting
  4. View patch

[PATCH 1/6] x86/irq: Add enumeration of NMI source reporting CPU feature

Jacob Pan posted 6 patches 1 year, 6 months ago
There is a newer version of this series
[PATCH 1/6] x86/irq: Add enumeration of NMI source reporting CPU feature
Posted by Jacob Pan 1 year, 6 months ago 
The lack of a mechanism to pinpoint the origins of Non-Maskable Interrupts
(NMIs) necessitates that the NMI vector 2 handler consults each NMI source
handler individually. This approach leads to inefficiencies, delays, and
the occurrence of unnecessary NMIs, thereby also constraining the potential
applications of NMIs.

A new CPU feature, known as NMI source reporting, has been introduced as
part of the Flexible Return and Event Delivery (FRED) spec. This feature
enables the NMI vector 2 handler to directly obtain information about the
NMI source from the FRED event data.

The functionality of NMI source reporting is tied to the FRED. Although it
is enumerated by a unique CPUID feature bit, it cannot be turned off
independently once FRED is activated.

Signed-off-by: Jacob Pan <jacob.jun.pan@linux.intel.com>
---
 arch/x86/Kconfig                         | 9 +++++++++
 arch/x86/include/asm/cpufeatures.h       | 1 +
 arch/x86/include/asm/disabled-features.h | 8 +++++++-
 arch/x86/kernel/cpu/cpuid-deps.c         | 1 +
 arch/x86/kernel/traps.c                  | 4 +++-
 5 files changed, 21 insertions(+), 2 deletions(-)

diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
index 1d7122a1883e..b8b15f20b94e 100644
--- a/arch/x86/Kconfig
+++ b/arch/x86/Kconfig
@@ -511,12 +511,21 @@ config X86_CPU_RESCTRL
 config X86_FRED
 	bool "Flexible Return and Event Delivery"
 	depends on X86_64
+	select X86_NMI_SOURCE
 	help
 	  When enabled, try to use Flexible Return and Event Delivery
 	  instead of the legacy SYSCALL/SYSENTER/IDT architecture for
 	  ring transitions and exception/interrupt handling if the
 	  system supports it.
 
+config X86_NMI_SOURCE
+	def_bool n
+	help
+	  Once enabled, information on NMI originator/source can be provided
+	  via FRED event data. This makes NMI processing more efficient in that
+	  NMI handler does not need to check for every possible source at
+	  runtime when NMI is delivered.
+
 config X86_BIGSMP
 	bool "Support for big SMP systems with more than 8 CPUs"
 	depends on SMP && X86_32
diff --git a/arch/x86/include/asm/cpufeatures.h b/arch/x86/include/asm/cpufeatures.h
index 3c7434329661..ec78d361e685 100644
--- a/arch/x86/include/asm/cpufeatures.h
+++ b/arch/x86/include/asm/cpufeatures.h
@@ -327,6 +327,7 @@
 #define X86_FEATURE_FRED		(12*32+17) /* Flexible Return and Event Delivery */
 #define X86_FEATURE_LKGS		(12*32+18) /* "" Load "kernel" (userspace) GS */
 #define X86_FEATURE_WRMSRNS		(12*32+19) /* "" Non-serializing WRMSR */
+#define X86_FEATURE_NMI_SOURCE		(12*32+20) /* NMI source reporting */
 #define X86_FEATURE_AMX_FP16		(12*32+21) /* "" AMX fp16 Support */
 #define X86_FEATURE_AVX_IFMA            (12*32+23) /* "" Support for VPMADD52[H,L]UQ */
 #define X86_FEATURE_LAM			(12*32+26) /* Linear Address Masking */
diff --git a/arch/x86/include/asm/disabled-features.h b/arch/x86/include/asm/disabled-features.h
index c492bdc97b05..3856c4737d65 100644
--- a/arch/x86/include/asm/disabled-features.h
+++ b/arch/x86/include/asm/disabled-features.h
@@ -123,6 +123,12 @@
 # define DISABLE_FRED	(1 << (X86_FEATURE_FRED & 31))
 #endif
 
+#ifdef CONFIG_X86_NMI_SOURCE
+# define DISABLE_NMI_SOURCE	0
+#else
+# define DISABLE_NMI_SOURCE	(1 << (X86_FEATURE_NMI_SOURCE & 31))
+#endif
+
 #ifdef CONFIG_KVM_AMD_SEV
 #define DISABLE_SEV_SNP		0
 #else
@@ -145,7 +151,7 @@
 #define DISABLED_MASK10	0
 #define DISABLED_MASK11	(DISABLE_RETPOLINE|DISABLE_RETHUNK|DISABLE_UNRET| \
 			 DISABLE_CALL_DEPTH_TRACKING|DISABLE_USER_SHSTK)
-#define DISABLED_MASK12	(DISABLE_FRED|DISABLE_LAM)
+#define DISABLED_MASK12	(DISABLE_FRED|DISABLE_LAM|DISABLE_NMI_SOURCE)
 #define DISABLED_MASK13	0
 #define DISABLED_MASK14	0
 #define DISABLED_MASK15	0
diff --git a/arch/x86/kernel/cpu/cpuid-deps.c b/arch/x86/kernel/cpu/cpuid-deps.c
index b7d9f530ae16..3f1a1a1961fa 100644
--- a/arch/x86/kernel/cpu/cpuid-deps.c
+++ b/arch/x86/kernel/cpu/cpuid-deps.c
@@ -84,6 +84,7 @@ static const struct cpuid_dep cpuid_deps[] = {
 	{ X86_FEATURE_SHSTK,			X86_FEATURE_XSAVES    },
 	{ X86_FEATURE_FRED,			X86_FEATURE_LKGS      },
 	{ X86_FEATURE_FRED,			X86_FEATURE_WRMSRNS   },
+	{ X86_FEATURE_FRED,			X86_FEATURE_NMI_SOURCE},
 	{}
 };
 
diff --git a/arch/x86/kernel/traps.c b/arch/x86/kernel/traps.c
index 4fa0b17e5043..465f04e4a79f 100644
--- a/arch/x86/kernel/traps.c
+++ b/arch/x86/kernel/traps.c
@@ -1427,8 +1427,10 @@ early_param("fred", fred_setup);
 
 void __init trap_init(void)
 {
-	if (cpu_feature_enabled(X86_FEATURE_FRED) && !enable_fred)
+	if (cpu_feature_enabled(X86_FEATURE_FRED) && !enable_fred) {
 		setup_clear_cpu_cap(X86_FEATURE_FRED);
+		setup_clear_cpu_cap(X86_FEATURE_NMI_SOURCE);
+	}
 
 	/* Init cpu_entry_area before IST entries are set up */
 	setup_cpu_entry_areas();
-- 
2.25.1
Re: [PATCH 1/6] x86/irq: Add enumeration of NMI source reporting CPU feature
Posted by H. Peter Anvin 1 year, 6 months ago 
On 5/29/24 13:33, Jacob Pan wrote:
> diff --git a/arch/x86/kernel/cpu/cpuid-deps.c b/arch/x86/kernel/cpu/cpuid-deps.c
> index b7d9f530ae16..3f1a1a1961fa 100644
> --- a/arch/x86/kernel/cpu/cpuid-deps.c
> +++ b/arch/x86/kernel/cpu/cpuid-deps.c
> @@ -84,6 +84,7 @@ static const struct cpuid_dep cpuid_deps[] = {
>   	{ X86_FEATURE_SHSTK,			X86_FEATURE_XSAVES    },
>   	{ X86_FEATURE_FRED,			X86_FEATURE_LKGS      },
>   	{ X86_FEATURE_FRED,			X86_FEATURE_WRMSRNS   },
> +	{ X86_FEATURE_FRED,			X86_FEATURE_NMI_SOURCE},
>   	{}
>   };
>   

This is incorrect. FRED does *not* inherently depend on NMI_SOURCE; the 
dependency is the reverse, but since it *also* depends on FRED being 
dynamically enabled, there is no need to add it to the static table; the 
dynamic test:

> diff --git a/arch/x86/kernel/traps.c b/arch/x86/kernel/traps.c
> index 4fa0b17e5043..465f04e4a79f 100644
> --- a/arch/x86/kernel/traps.c
> +++ b/arch/x86/kernel/traps.c
> @@ -1427,8 +1427,10 @@ early_param("fred", fred_setup);
>  
>  void __init trap_init(void)
>  {
> -	if (cpu_feature_enabled(X86_FEATURE_FRED) && !enable_fred)
> +	if (cpu_feature_enabled(X86_FEATURE_FRED) && !enable_fred) {
>  		setup_clear_cpu_cap(X86_FEATURE_FRED);
> +		setup_clear_cpu_cap(X86_FEATURE_NMI_SOURCE);
> +	}
>  
>  	/* Init cpu_entry_area before IST entries are set up */
>  	setup_cpu_entry_areas();

... suffices just fine on its own.

	-hpa
Re: [PATCH 1/6] x86/irq: Add enumeration of NMI source reporting CPU feature
Posted by Jacob Pan 1 year, 6 months ago 
Hi Peter,

On Wed, 29 May 2024 13:49:40 -0700, "H. Peter Anvin" <hpa@zytor.com> wrote:

> On 5/29/24 13:33, Jacob Pan wrote:
> > diff --git a/arch/x86/kernel/cpu/cpuid-deps.c
> > b/arch/x86/kernel/cpu/cpuid-deps.c index b7d9f530ae16..3f1a1a1961fa
> > 100644 --- a/arch/x86/kernel/cpu/cpuid-deps.c
> > +++ b/arch/x86/kernel/cpu/cpuid-deps.c
> > @@ -84,6 +84,7 @@ static const struct cpuid_dep cpuid_deps[] = {
> >   	{ X86_FEATURE_SHSTK,
> > X86_FEATURE_XSAVES    }, { X86_FEATURE_FRED,
> > X86_FEATURE_LKGS      }, { X86_FEATURE_FRED,
> > X86_FEATURE_WRMSRNS   },
> > +	{ X86_FEATURE_FRED,
> > X86_FEATURE_NMI_SOURCE}, {}
> >   };
> >     
> 
> This is incorrect. FRED does *not* inherently depend on NMI_SOURCE; the 
> dependency is the reverse, but since it *also* depends on FRED being 
> dynamically enabled, there is no need to add it to the static table; the 
> dynamic test:
> 
My misunderstanding of the dependency table, thanks for pointing it out.
Will remove.

> > diff --git a/arch/x86/kernel/traps.c b/arch/x86/kernel/traps.c
> > index 4fa0b17e5043..465f04e4a79f 100644
> > --- a/arch/x86/kernel/traps.c
> > +++ b/arch/x86/kernel/traps.c
> > @@ -1427,8 +1427,10 @@ early_param("fred", fred_setup);
> >  
> >  void __init trap_init(void)
> >  {
> > -	if (cpu_feature_enabled(X86_FEATURE_FRED) && !enable_fred)
> > +	if (cpu_feature_enabled(X86_FEATURE_FRED) && !enable_fred) {
> >  		setup_clear_cpu_cap(X86_FEATURE_FRED);
> > +		setup_clear_cpu_cap(X86_FEATURE_NMI_SOURCE);
> > +	}
> >  
> >  	/* Init cpu_entry_area before IST entries are set up */
> >  	setup_cpu_entry_areas();  
> 
> ... suffices just fine on its own.
I am not following, do you mean checking for FRED is sufficient for NMI
source? I think it works since NMI source cannot be disabled if FRED is on.
Just want to use the architectural CPUID bits to the fullest.


Thanks,

Jacob
Re: [PATCH 1/6] x86/irq: Add enumeration of NMI source reporting CPU feature
Posted by Jacob Pan 1 year, 6 months ago 
Hi Jacob,

On Thu, 30 May 2024 09:19:16 -0700, Jacob Pan
<jacob.jun.pan@linux.intel.com> wrote:

> Hi Peter,
> 
> On Wed, 29 May 2024 13:49:40 -0700, "H. Peter Anvin" <hpa@zytor.com>
> wrote:
> 
> > On 5/29/24 13:33, Jacob Pan wrote:  
> > > diff --git a/arch/x86/kernel/cpu/cpuid-deps.c
> > > b/arch/x86/kernel/cpu/cpuid-deps.c index b7d9f530ae16..3f1a1a1961fa
> > > 100644 --- a/arch/x86/kernel/cpu/cpuid-deps.c
> > > +++ b/arch/x86/kernel/cpu/cpuid-deps.c
> > > @@ -84,6 +84,7 @@ static const struct cpuid_dep cpuid_deps[] = {
> > >   	{ X86_FEATURE_SHSTK,
> > > X86_FEATURE_XSAVES    }, { X86_FEATURE_FRED,
> > > X86_FEATURE_LKGS      }, { X86_FEATURE_FRED,
> > > X86_FEATURE_WRMSRNS   },
> > > +	{ X86_FEATURE_FRED,
> > > X86_FEATURE_NMI_SOURCE}, {}
> > >   };
> > >       
> > 
> > This is incorrect. FRED does *not* inherently depend on NMI_SOURCE; the 
> > dependency is the reverse, but since it *also* depends on FRED being 
> > dynamically enabled, there is no need to add it to the static table;
> > the dynamic test:
> >   
> My misunderstanding of the dependency table, thanks for pointing it out.
> Will remove.
> 
> > > diff --git a/arch/x86/kernel/traps.c b/arch/x86/kernel/traps.c
> > > index 4fa0b17e5043..465f04e4a79f 100644
> > > --- a/arch/x86/kernel/traps.c
> > > +++ b/arch/x86/kernel/traps.c
> > > @@ -1427,8 +1427,10 @@ early_param("fred", fred_setup);
> > >  
> > >  void __init trap_init(void)
> > >  {
> > > -	if (cpu_feature_enabled(X86_FEATURE_FRED) && !enable_fred)
> > > +	if (cpu_feature_enabled(X86_FEATURE_FRED) && !enable_fred) {
> > >  		setup_clear_cpu_cap(X86_FEATURE_FRED);
> > > +		setup_clear_cpu_cap(X86_FEATURE_NMI_SOURCE);
> > > +	}
> > >  
> > >  	/* Init cpu_entry_area before IST entries are set up */
> > >  	setup_cpu_entry_areas();    
> > 
> > ... suffices just fine on its own.  
> I am not following, do you mean checking for FRED is sufficient for NMI
> source? I think it works since NMI source cannot be disabled if FRED is
> on. Just want to use the architectural CPUID bits to the fullest.
> 
Nevermind, I got it now, will keep the dynamic test.


Thanks,

Jacob

Patchew 2.1-devel-b67e4c2

© 2016 - 2025 Red Hat, Inc.