With NMI source reporting enabled, NMI handler can prioritize the
handling of sources reported explicitly. If the source is unknown, then
resume the existing processing flow. i.e. invoke all NMI handlers.

Signed-off-by: Jacob Pan <jacob.jun.pan@linux.intel.com>

---
v3:
   - Use a static flag to disable NMIs in case of HW failure
   - Optimize the case when unknown NMIs are mixed with known NMIs(HPA)
v2:
   - Disable NMI source reporting once garbage data is given in FRED
return stack. (HPA)
---
 arch/x86/kernel/nmi.c | 73 +++++++++++++++++++++++++++++++++++++++++--
 1 file changed, 70 insertions(+), 3 deletions(-)

diff --git a/arch/x86/kernel/nmi.c b/arch/x86/kernel/nmi.c
index 639a34e78bc9..c3a10af7f26b 100644
--- a/arch/x86/kernel/nmi.c
+++ b/arch/x86/kernel/nmi.c
@@ -149,23 +149,90 @@ static inline int do_handle_nmi(struct nmiaction *a, struct pt_regs *regs, unsig
 	return thishandled;
 }
 
+static int nmi_handle_src(unsigned int type, struct pt_regs *regs, unsigned long *handled_mask)
+{
+	static bool nmi_source_disabled;
+	bool has_unknown_src = false;
+	unsigned long source_bitmask;
+	struct nmiaction *a;
+	int handled = 0;
+	int vec = 1;
+
+	if (!cpu_feature_enabled(X86_FEATURE_NMI_SOURCE) ||
+	    type != NMI_LOCAL || nmi_source_disabled)
+		return 0;
+
+	source_bitmask = fred_event_data(regs);
+	if (!source_bitmask) {
+		pr_warn("NMI received without source information! Disable source reporting.\n");
+		nmi_source_disabled = true;
+		return 0;
+	}
+
+	/*
+	 * Per NMI source specification, there is no guarantee that a valid
+	 * NMI vector is always delivered, even when the source specified
+	 * one. It is software's responsibility to check all available NMI
+	 * sources when bit 0 is set in the NMI source bitmap. i.e. we have
+	 * to call every handler as if we have no NMI source.
+	 * On the other hand, if we do get non-zero vectors, we know exactly
+	 * what the sources are. So we only call the handlers with the bit set.
+	 */
+	if (source_bitmask & BIT(NMI_SOURCE_VEC_UNKNOWN)) {
+		pr_warn_ratelimited("NMI received with unknown source\n");
+		has_unknown_src = true;
+	}
+
+	rcu_read_lock();
+	/* Bit 0 is for unknown NMI sources, skip it. */
+	for_each_set_bit_from(vec, &source_bitmask, NR_NMI_SOURCE_VECTORS) {
+		a = rcu_dereference(nmiaction_src_table[vec]);
+		if (!a) {
+			pr_warn_ratelimited("NMI received %d no handler", vec);
+			continue;
+		}
+		handled += do_handle_nmi(a, regs, type);
+		/*
+		 * Needs polling if unknown source bit is set, handled_mask is
+		 * used to tell the polling code which NMIs can be skipped.
+		 */
+		if (has_unknown_src)
+			*handled_mask |= BIT(vec);
+	}
+	rcu_read_unlock();
+
+	return handled;
+}
+
 static int nmi_handle(unsigned int type, struct pt_regs *regs)
 {
 	struct nmi_desc *desc = nmi_to_desc(type);
+	unsigned long handled_mask = 0;
 	struct nmiaction *a;
 	int handled=0;
 
-	rcu_read_lock();
+	/*
+	 * Check if the NMI source handling is complete, otherwise polling is
+	 * still required. handled_mask is non-zero if NMI source handling is
+	 * partial due to unknown NMI sources.
+	 */
+	handled = nmi_handle_src(type, regs, &handled_mask);
+	if (handled && !handled_mask)
+		return handled;
 
+	rcu_read_lock();
 	/*
 	 * NMIs are edge-triggered, which means if you have enough
 	 * of them concurrently, you can lose some because only one
 	 * can be latched at any given time.  Walk the whole list
 	 * to handle those situations.
 	 */
-	list_for_each_entry_rcu(a, &desc->head, list)
+	list_for_each_entry_rcu(a, &desc->head, list) {
+		/* Skip NMIs handled earlier with source info */
+		if (BIT(a->source_vec) & handled_mask)
+			continue;
 		handled += do_handle_nmi(a, regs, type);
-
+	}
 	rcu_read_unlock();
 
 	/* return total number of NMI events handled */
-- 
2.25.1

On 28.06.24 г. 23:18 ч., Jacob Pan wrote:
> With NMI source reporting enabled, NMI handler can prioritize the
> handling of sources reported explicitly. If the source is unknown, then
> resume the existing processing flow. i.e. invoke all NMI handlers.
> 
> Signed-off-by: Jacob Pan <jacob.jun.pan@linux.intel.com>
> 
> ---
> v3:
>     - Use a static flag to disable NMIs in case of HW failure
>     - Optimize the case when unknown NMIs are mixed with known NMIs(HPA)
> v2:
>     - Disable NMI source reporting once garbage data is given in FRED
> return stack. (HPA)
> ---
>   arch/x86/kernel/nmi.c | 73 +++++++++++++++++++++++++++++++++++++++++--
>   1 file changed, 70 insertions(+), 3 deletions(-)
> 
> diff --git a/arch/x86/kernel/nmi.c b/arch/x86/kernel/nmi.c
> index 639a34e78bc9..c3a10af7f26b 100644
> --- a/arch/x86/kernel/nmi.c
> +++ b/arch/x86/kernel/nmi.c
> @@ -149,23 +149,90 @@ static inline int do_handle_nmi(struct nmiaction *a, struct pt_regs *regs, unsig
>   	return thishandled;
>   }
>   
> +static int nmi_handle_src(unsigned int type, struct pt_regs *regs, unsigned long *handled_mask)
> +{
> +	static bool nmi_source_disabled;
> +	bool has_unknown_src = false;
> +	unsigned long source_bitmask;
> +	struct nmiaction *a;
> +	int handled = 0;
> +	int vec = 1;
> +
> +	if (!cpu_feature_enabled(X86_FEATURE_NMI_SOURCE) ||
> +	    type != NMI_LOCAL || nmi_source_disabled)
> +		return 0;
> +
> +	source_bitmask = fred_event_data(regs);
> +	if (!source_bitmask) {
> +		pr_warn("NMI received without source information! Disable source reporting.\n");
> +		nmi_source_disabled = true;
> +		return 0;
> +	}
> +
> +	/*
> +	 * Per NMI source specification, there is no guarantee that a valid
> +	 * NMI vector is always delivered, even when the source specified
> +	 * one. It is software's responsibility to check all available NMI
> +	 * sources when bit 0 is set in the NMI source bitmap. i.e. we have
> +	 * to call every handler as if we have no NMI source.
> +	 * On the other hand, if we do get non-zero vectors, we know exactly
> +	 * what the sources are. So we only call the handlers with the bit set.
> +	 */
> +	if (source_bitmask & BIT(NMI_SOURCE_VEC_UNKNOWN)) {
> +		pr_warn_ratelimited("NMI received with unknown source\n");
> +		has_unknown_src = true;
> +	}
> +
> +	rcu_read_lock();
> +	/* Bit 0 is for unknown NMI sources, skip it. */
> +	for_each_set_bit_from(vec, &source_bitmask, NR_NMI_SOURCE_VECTORS) {
> +		a = rcu_dereference(nmiaction_src_table[vec]);
> +		if (!a) {
> +			pr_warn_ratelimited("NMI received %d no handler", vec);
> +			continue;
> +		}
> +		handled += do_handle_nmi(a, regs, type);
> +		/*
> +		 * Needs polling if unknown source bit is set, handled_mask is
> +		 * used to tell the polling code which NMIs can be skipped.
> +		 */
> +		if (has_unknown_src)
> +			*handled_mask |= BIT(vec);
> +	}
> +	rcu_read_unlock();
> +
> +	return handled;
> +}
> +
>   static int nmi_handle(unsigned int type, struct pt_regs *regs)
>   {
>   	struct nmi_desc *desc = nmi_to_desc(type);
> +	unsigned long handled_mask = 0;
>   	struct nmiaction *a;
>   	int handled=0;
>   
> -	rcu_read_lock();
> +	/*
> +	 * Check if the NMI source handling is complete, otherwise polling is
> +	 * still required. handled_mask is non-zero if NMI source handling is
> +	 * partial due to unknown NMI sources.
> +	 */
> +	handled = nmi_handle_src(type, regs, &handled_mask);
> +	if (handled && !handled_mask)
> +		return handled;

How about renaming handled_mask to "partial_handled_mask" ? Because in 
addition to it being a mask it's also used as a boolean to signal that 
an unknown NMI source was encountered.

>   
> +	rcu_read_lock();
>   	/*
>   	 * NMIs are edge-triggered, which means if you have enough
>   	 * of them concurrently, you can lose some because only one
>   	 * can be latched at any given time.  Walk the whole list
>   	 * to handle those situations.
>   	 */
> -	list_for_each_entry_rcu(a, &desc->head, list)
> +	list_for_each_entry_rcu(a, &desc->head, list) {
> +		/* Skip NMIs handled earlier with source info */
> +		if (BIT(a->source_vec) & handled_mask)
> +			continue;
>   		handled += do_handle_nmi(a, regs, type);
> -
> +	}
>   	rcu_read_unlock();
>   
>   	/* return total number of NMI events handled */

On Mon, 1 Jul 2024 17:31:48 +0300, Nikolay Borisov <nik.borisov@suse.com>
wrote:

> On 28.06.24 г. 23:18 ч., Jacob Pan wrote:
> > With NMI source reporting enabled, NMI handler can prioritize the
> > handling of sources reported explicitly. If the source is unknown, then
> > resume the existing processing flow. i.e. invoke all NMI handlers.
> > 
> > Signed-off-by: Jacob Pan <jacob.jun.pan@linux.intel.com>
> > 
> > ---
> > v3:
> >     - Use a static flag to disable NMIs in case of HW failure
> >     - Optimize the case when unknown NMIs are mixed with known NMIs(HPA)
> > v2:
> >     - Disable NMI source reporting once garbage data is given in FRED
> > return stack. (HPA)
> > ---
> >   arch/x86/kernel/nmi.c | 73 +++++++++++++++++++++++++++++++++++++++++--
> >   1 file changed, 70 insertions(+), 3 deletions(-)
> > 
> > diff --git a/arch/x86/kernel/nmi.c b/arch/x86/kernel/nmi.c
> > index 639a34e78bc9..c3a10af7f26b 100644
> > --- a/arch/x86/kernel/nmi.c
> > +++ b/arch/x86/kernel/nmi.c
> > @@ -149,23 +149,90 @@ static inline int do_handle_nmi(struct nmiaction
> > *a, struct pt_regs *regs, unsig return thishandled;
> >   }
> >   
> > +static int nmi_handle_src(unsigned int type, struct pt_regs *regs,
> > unsigned long *handled_mask) +{
> > +	static bool nmi_source_disabled;
> > +	bool has_unknown_src = false;
> > +	unsigned long source_bitmask;
> > +	struct nmiaction *a;
> > +	int handled = 0;
> > +	int vec = 1;
> > +
> > +	if (!cpu_feature_enabled(X86_FEATURE_NMI_SOURCE) ||
> > +	    type != NMI_LOCAL || nmi_source_disabled)
> > +		return 0;
> > +
> > +	source_bitmask = fred_event_data(regs);
> > +	if (!source_bitmask) {
> > +		pr_warn("NMI received without source information!
> > Disable source reporting.\n");
> > +		nmi_source_disabled = true;
> > +		return 0;
> > +	}
> > +
> > +	/*
> > +	 * Per NMI source specification, there is no guarantee that a
> > valid
> > +	 * NMI vector is always delivered, even when the source
> > specified
> > +	 * one. It is software's responsibility to check all available
> > NMI
> > +	 * sources when bit 0 is set in the NMI source bitmap. i.e. we
> > have
> > +	 * to call every handler as if we have no NMI source.
> > +	 * On the other hand, if we do get non-zero vectors, we know
> > exactly
> > +	 * what the sources are. So we only call the handlers with the
> > bit set.
> > +	 */
> > +	if (source_bitmask & BIT(NMI_SOURCE_VEC_UNKNOWN)) {
> > +		pr_warn_ratelimited("NMI received with unknown
> > source\n");
> > +		has_unknown_src = true;
> > +	}
> > +
> > +	rcu_read_lock();
> > +	/* Bit 0 is for unknown NMI sources, skip it. */
> > +	for_each_set_bit_from(vec, &source_bitmask,
> > NR_NMI_SOURCE_VECTORS) {
> > +		a = rcu_dereference(nmiaction_src_table[vec]);
> > +		if (!a) {
> > +			pr_warn_ratelimited("NMI received %d no
> > handler", vec);
> > +			continue;
> > +		}
> > +		handled += do_handle_nmi(a, regs, type);
> > +		/*
> > +		 * Needs polling if unknown source bit is set,
> > handled_mask is
> > +		 * used to tell the polling code which NMIs can be
> > skipped.
> > +		 */
> > +		if (has_unknown_src)
> > +			*handled_mask |= BIT(vec);
> > +	}
> > +	rcu_read_unlock();
> > +
> > +	return handled;
> > +}
> > +
> >   static int nmi_handle(unsigned int type, struct pt_regs *regs)
> >   {
> >   	struct nmi_desc *desc = nmi_to_desc(type);
> > +	unsigned long handled_mask = 0;
> >   	struct nmiaction *a;
> >   	int handled=0;
> >   
> > -	rcu_read_lock();
> > +	/*
> > +	 * Check if the NMI source handling is complete, otherwise
> > polling is
> > +	 * still required. handled_mask is non-zero if NMI source
> > handling is
> > +	 * partial due to unknown NMI sources.
> > +	 */
> > +	handled = nmi_handle_src(type, regs, &handled_mask);
> > +	if (handled && !handled_mask)
> > +		return handled;  
> 
> How about renaming handled_mask to "partial_handled_mask" ? Because in 
> addition to it being a mask it's also used as a boolean to signal that 
> an unknown NMI source was encountered.
yeah, that is better. will do.

> >   
> > +	rcu_read_lock();
> >   	/*
> >   	 * NMIs are edge-triggered, which means if you have enough
> >   	 * of them concurrently, you can lose some because only one
> >   	 * can be latched at any given time.  Walk the whole list
> >   	 * to handle those situations.
> >   	 */
> > -	list_for_each_entry_rcu(a, &desc->head, list)
> > +	list_for_each_entry_rcu(a, &desc->head, list) {
> > +		/* Skip NMIs handled earlier with source info */
> > +		if (BIT(a->source_vec) & handled_mask)
> > +			continue;
> >   		handled += do_handle_nmi(a, regs, type);
> > -
> > +	}
> >   	rcu_read_unlock();
> >   
> >   	/* return total number of NMI events handled */  


Thanks,

Jacob

On 6/28/2024 1:18 PM, Jacob Pan wrote:
> With NMI source reporting enabled, NMI handler can prioritize the
> handling of sources reported explicitly. If the source is unknown, then
> resume the existing processing flow. i.e. invoke all NMI handlers.
> 
> Signed-off-by: Jacob Pan <jacob.jun.pan@linux.intel.com>

The code looks good to me, however please improve coding styles and
comments, see below.

> 
> ---
> v3:
>     - Use a static flag to disable NMIs in case of HW failure
>     - Optimize the case when unknown NMIs are mixed with known NMIs(HPA)
> v2:
>     - Disable NMI source reporting once garbage data is given in FRED
> return stack. (HPA)
> ---
>   arch/x86/kernel/nmi.c | 73 +++++++++++++++++++++++++++++++++++++++++--
>   1 file changed, 70 insertions(+), 3 deletions(-)
> 
> diff --git a/arch/x86/kernel/nmi.c b/arch/x86/kernel/nmi.c
> index 639a34e78bc9..c3a10af7f26b 100644
> --- a/arch/x86/kernel/nmi.c
> +++ b/arch/x86/kernel/nmi.c
> @@ -149,23 +149,90 @@ static inline int do_handle_nmi(struct nmiaction *a, struct pt_regs *regs, unsig
>   	return thishandled;
>   }
>   
> +static int nmi_handle_src(unsigned int type, struct pt_regs *regs, unsigned long *handled_mask)
> +{
> +	static bool nmi_source_disabled;
> +	bool has_unknown_src = false;
> +	unsigned long source_bitmask;
> +	struct nmiaction *a;
> +	int handled = 0;
> +	int vec = 1;
> +
> +	if (!cpu_feature_enabled(X86_FEATURE_NMI_SOURCE) ||
> +	    type != NMI_LOCAL || nmi_source_disabled)

Harder to read, no need to break into 2 lines.

> +		return 0;
> +
> +	source_bitmask = fred_event_data(regs);
> +	if (!source_bitmask) {

unlikely()?

> +		pr_warn("NMI received without source information! Disable source reporting.\n");

It sounds you're disabling some hardware functionality. Better to say,
maybe:

Buggy hardware? Disable NMI source handling.

> +		nmi_source_disabled = true;
> +		return 0;
> +	}
> +
> +	/*
> +	 * Per NMI source specification, there is no guarantee that a valid
> +	 * NMI vector is always delivered, even when the source specified
> +	 * one. It is software's responsibility to check all available NMI
> +	 * sources when bit 0 is set in the NMI source bitmap. i.e. we have

s/i.e./I.e.,/

> +	 * to call every handler as if we have no NMI source.

This comment is misleading, because you do skip NMI handlers with source
bits set in polling.

And add an empty line to ease review.

> +	 * On the other hand, if we do get non-zero vectors, we know exactly
> +	 * what the sources are. So we only call the handlers with the bit set.
> +	 */
> +	if (source_bitmask & BIT(NMI_SOURCE_VEC_UNKNOWN)) {
> +		pr_warn_ratelimited("NMI received with unknown source\n");

s/source/sources/

> +		has_unknown_src = true;
> +	}
> +
> +	rcu_read_lock();

Add an empty line.

> +	/* Bit 0 is for unknown NMI sources, skip it. */

Put "vec = 1 " close to this comment.

> +	for_each_set_bit_from(vec, &source_bitmask, NR_NMI_SOURCE_VECTORS) {
> +		a = rcu_dereference(nmiaction_src_table[vec]);
> +		if (!a) {
> +			pr_warn_ratelimited("NMI received %d no handler", vec);

Use a better log message.

> +			continue;
> +		}

Empty line again.

> +		handled += do_handle_nmi(a, regs, type);

Ditto.

> +		/*
> +		 * Needs polling if unknown source bit is set, handled_mask is

                                    ^the

> +		 * used to tell the polling code which NMIs can be skipped.
> +		 */
> +		if (has_unknown_src)
> +			*handled_mask |= BIT(vec);
> +	}

empty line please.

> +	rcu_read_unlock();
> +
> +	return handled;
> +}
> +
>   static int nmi_handle(unsigned int type, struct pt_regs *regs)
>   {
>   	struct nmi_desc *desc = nmi_to_desc(type);
> +	unsigned long handled_mask = 0;
>   	struct nmiaction *a;
>   	int handled=0;
>   
> -	rcu_read_lock();
> +	/*
> +	 * Check if the NMI source handling is complete, otherwise polling is
> +	 * still required. handled_mask is non-zero if NMI source handling is
> +	 * partial due to unknown NMI sources.
> +	 */
> +	handled = nmi_handle_src(type, regs, &handled_mask);
> +	if (handled && !handled_mask)
> +		return handled;
>   
> +	rcu_read_lock();

keep original empty lines around it.

>   	/*
>   	 * NMIs are edge-triggered, which means if you have enough
>   	 * of them concurrently, you can lose some because only one
>   	 * can be latched at any given time.  Walk the whole list
>   	 * to handle those situations.
>   	 */
> -	list_for_each_entry_rcu(a, &desc->head, list)
> +	list_for_each_entry_rcu(a, &desc->head, list) {
> +		/* Skip NMIs handled earlier with source info */
> +		if (BIT(a->source_vec) & handled_mask)
> +			continue;
>   		handled += do_handle_nmi(a, regs, type);
> -
> +	}
>   	rcu_read_unlock();

keep original empty lines around it.

>   
>   	/* return total number of NMI events handled */

On Fri, 28 Jun 2024 20:39:24 -0700, Xin Li <xin@zytor.com> wrote:

> On 6/28/2024 1:18 PM, Jacob Pan wrote:
> > With NMI source reporting enabled, NMI handler can prioritize the
> > handling of sources reported explicitly. If the source is unknown, then
> > resume the existing processing flow. i.e. invoke all NMI handlers.
> > 
> > Signed-off-by: Jacob Pan <jacob.jun.pan@linux.intel.com>  
> 
> The code looks good to me, however please improve coding styles and
> comments, see below.
> 
> > 
> > ---
> > v3:
> >     - Use a static flag to disable NMIs in case of HW failure
> >     - Optimize the case when unknown NMIs are mixed with known NMIs(HPA)
> > v2:
> >     - Disable NMI source reporting once garbage data is given in FRED
> > return stack. (HPA)
> > ---
> >   arch/x86/kernel/nmi.c | 73 +++++++++++++++++++++++++++++++++++++++++--
> >   1 file changed, 70 insertions(+), 3 deletions(-)
> > 
> > diff --git a/arch/x86/kernel/nmi.c b/arch/x86/kernel/nmi.c
> > index 639a34e78bc9..c3a10af7f26b 100644
> > --- a/arch/x86/kernel/nmi.c
> > +++ b/arch/x86/kernel/nmi.c
> > @@ -149,23 +149,90 @@ static inline int do_handle_nmi(struct nmiaction
> > *a, struct pt_regs *regs, unsig return thishandled;
> >   }
> >   
> > +static int nmi_handle_src(unsigned int type, struct pt_regs *regs,
> > unsigned long *handled_mask) +{
> > +	static bool nmi_source_disabled;
> > +	bool has_unknown_src = false;
> > +	unsigned long source_bitmask;
> > +	struct nmiaction *a;
> > +	int handled = 0;
> > +	int vec = 1;
> > +
> > +	if (!cpu_feature_enabled(X86_FEATURE_NMI_SOURCE) ||
> > +	    type != NMI_LOCAL || nmi_source_disabled)  
> 
> Harder to read, no need to break into 2 lines.
ok.
 
> > +		return 0;
> > +
> > +	source_bitmask = fred_event_data(regs);
> > +	if (!source_bitmask) {  
> 
> unlikely()?
yes.

> 
> > +		pr_warn("NMI received without source information!
> > Disable source reporting.\n");  
> 
> It sounds you're disabling some hardware functionality. Better to say,
> maybe:
> 
> Buggy hardware? Disable NMI source handling.
> 
yes, this is more clear.

> > +		nmi_source_disabled = true;
> > +		return 0;
> > +	}
> > +
> > +	/*
> > +	 * Per NMI source specification, there is no guarantee that a
> > valid
> > +	 * NMI vector is always delivered, even when the source
> > specified
> > +	 * one. It is software's responsibility to check all available
> > NMI
> > +	 * sources when bit 0 is set in the NMI source bitmap. i.e. we
> > have  
> 
> s/i.e./I.e.,/
will fix

> 
> > +	 * to call every handler as if we have no NMI source.  
> 
> This comment is misleading, because you do skip NMI handlers with source
> bits set in polling.
At a high level, it is still accurate to say that every handler is invoked,
whether by the NMI-source handling code or subsequently in the polling code.

How about this:
	 * in the NMI-source reporting bitmap. I.e. we have to call every
	 * handler as if there is no NMI-source reporting feature enabled.
	 *
	 * In this case, handlers for the known NMI sources will be called
	 * first, followed by the remaining handlers, which are called
	 * during the subsequent polling code.


> 
> And add an empty line to ease review.
will do

> 
> > +	 * On the other hand, if we do get non-zero vectors, we know
> > exactly
> > +	 * what the sources are. So we only call the handlers with the
> > bit set.
> > +	 */
> > +	if (source_bitmask & BIT(NMI_SOURCE_VEC_UNKNOWN)) {
> > +		pr_warn_ratelimited("NMI received with unknown
> > source\n");  
> 
> s/source/sources/
> 
> > +		has_unknown_src = true;
> > +	}
> > +
> > +	rcu_read_lock();  
> 
> Add an empty line.
> 
> > +	/* Bit 0 is for unknown NMI sources, skip it. */  
> 
> Put "vec = 1 " close to this comment.
yes

> 
> > +	for_each_set_bit_from(vec, &source_bitmask,
> > NR_NMI_SOURCE_VECTORS) {
> > +		a = rcu_dereference(nmiaction_src_table[vec]);
> > +		if (!a) {
> > +			pr_warn_ratelimited("NMI received %d no
> > handler", vec);  
> 
> Use a better log message.
pr_warn_ratelimited("NMI-source vector %d has no handler!", vec);

> 
> > +			continue;
> > +		}  
> 
> Empty line again.
will add

> 
> > +		handled += do_handle_nmi(a, regs, type);  
> 
> Ditto.
yes

> 
> > +		/*
> > +		 * Needs polling if unknown source bit is set,
> > handled_mask is  
> 
>                                     ^the
got that.

> 
> > +		 * used to tell the polling code which NMIs can be
> > skipped.
> > +		 */
> > +		if (has_unknown_src)
> > +			*handled_mask |= BIT(vec);
> > +	}  
> 
> empty line please.
will add

> 
> > +	rcu_read_unlock();
> > +
> > +	return handled;
> > +}
> > +
> >   static int nmi_handle(unsigned int type, struct pt_regs *regs)
> >   {
> >   	struct nmi_desc *desc = nmi_to_desc(type);
> > +	unsigned long handled_mask = 0;
> >   	struct nmiaction *a;
> >   	int handled=0;
> >   
> > -	rcu_read_lock();
> > +	/*
> > +	 * Check if the NMI source handling is complete, otherwise
> > polling is
> > +	 * still required. handled_mask is non-zero if NMI source
> > handling is
> > +	 * partial due to unknown NMI sources.
> > +	 */
> > +	handled = nmi_handle_src(type, regs, &handled_mask);
> > +	if (handled && !handled_mask)
> > +		return handled;
> >   
> > +	rcu_read_lock();  
> 
> keep original empty lines around it.
ok

> 
> >   	/*
> >   	 * NMIs are edge-triggered, which means if you have enough
> >   	 * of them concurrently, you can lose some because only one
> >   	 * can be latched at any given time.  Walk the whole list
> >   	 * to handle those situations.
> >   	 */
> > -	list_for_each_entry_rcu(a, &desc->head, list)
> > +	list_for_each_entry_rcu(a, &desc->head, list) {
> > +		/* Skip NMIs handled earlier with source info */
> > +		if (BIT(a->source_vec) & handled_mask)
> > +			continue;
> >   		handled += do_handle_nmi(a, regs, type);
> > -
> > +	}
> >   	rcu_read_unlock();  
> 
> keep original empty lines around it.
ok

Thanks,

Jacob