The comment in unknown NMI handling is incorrect and misleading. There
is no longer a restriction on having a single Unknown NMI handler. Also,
nmi_handle() does not use the 'b2b' parameter anymore.
The changes that made the comment outdated are:
commit 0d443b70cc92 ("x86/platform: Remove warning message for duplicate
NMI handlers")
commit bf9f2ee28d47 ("x86/nmi: Remove the 'b2b' parameter from
nmi_handle()")
Remove the old comment and update it to reflect the current intention.
Signed-off-by: Sohil Mehta <sohil.mehta@intel.com>
---
arch/x86/kernel/nmi.c | 7 +++----
1 file changed, 3 insertions(+), 4 deletions(-)
diff --git a/arch/x86/kernel/nmi.c b/arch/x86/kernel/nmi.c
index cdfb3864d59a..2a07c9adc6a6 100644
--- a/arch/x86/kernel/nmi.c
+++ b/arch/x86/kernel/nmi.c
@@ -327,10 +327,9 @@ unknown_nmi_error(unsigned char reason, struct pt_regs *regs)
int handled;
/*
- * Use 'false' as back-to-back NMIs are dealt with one level up.
- * Of course this makes having multiple 'unknown' handlers useless
- * as only the first one is ever run (unless it can actually determine
- * if it caused the NMI)
+ * As a last resort, let the "unknown" handlers make a
+ * best-effort attempt to figure out if they can claim
+ * responsibility for this Unknown NMI.
*/
handled = nmi_handle(NMI_UNKNOWN, regs);
if (handled) {
--
2.43.0On Thu, 2025-03-27 at 23:46 +0000, Mehta, Sohil wrote:
> The comment in unknown NMI handling is incorrect and misleading. There
> is no longer a restriction on having a single Unknown NMI handler. Also,
> nmi_handle() does not use the 'b2b' parameter anymore.
>
> The changes that made the comment outdated are:
>
> commit 0d443b70cc92 ("x86/platform: Remove warning message for duplicate
> NMI handlers")
>
> commit bf9f2ee28d47 ("x86/nmi: Remove the 'b2b' parameter from
> nmi_handle()")
After some digging, IIUC, the 'b2b' parameter of the nmi_handle() was actually
never used when it was originally added in the
commit b227e23399dc ("x86, nmi: Add in logic to handle multiple events and
unknown NMIs")
.., so IIUC the comment was wrong when it was firstly added.
The above commit to remove the 'b2b' seems just a fixup patch but it didn't fix
the comment.
Not sure whether it is worth to mention in the changelog.
>
> Remove the old comment and update it to reflect the current intention.
>
> Signed-off-by: Sohil Mehta <sohil.mehta@intel.com>
>
Anyway:
Reviewed-by: Kai Huang <kai.huang@intel.com>
On March 31, 2025 5:17:55 PM PDT, "Huang, Kai" <kai.huang@intel.com> wrote:
>On Thu, 2025-03-27 at 23:46 +0000, Mehta, Sohil wrote:
>> The comment in unknown NMI handling is incorrect and misleading. There
>> is no longer a restriction on having a single Unknown NMI handler. Also,
>> nmi_handle() does not use the 'b2b' parameter anymore.
>>
>> The changes that made the comment outdated are:
>>
>> commit 0d443b70cc92 ("x86/platform: Remove warning message for duplicate
>> NMI handlers")
>>
>> commit bf9f2ee28d47 ("x86/nmi: Remove the 'b2b' parameter from
>> nmi_handle()")
>
>After some digging, IIUC, the 'b2b' parameter of the nmi_handle() was actually
>never used when it was originally added in the
>
> commit b227e23399dc ("x86, nmi: Add in logic to handle multiple events and
>unknown NMIs")
>
>.., so IIUC the comment was wrong when it was firstly added.
>
>The above commit to remove the 'b2b' seems just a fixup patch but it didn't fix
>the comment.
>
>Not sure whether it is worth to mention in the changelog.
>
>>
>> Remove the old comment and update it to reflect the current intention.
>>
>> Signed-off-by: Sohil Mehta <sohil.mehta@intel.com>
>>
>
>Anyway:
>
>Reviewed-by: Kai Huang <kai.huang@intel.com>
>
Ha! Any idea what it was *supposed* to do? I'm guessing it stood for "back to back" or some such?
On 3/31/2025 10:45 PM, H. Peter Anvin wrote: > Ha! Any idea what it was *supposed* to do? I'm guessing it stood for "back to back" or some such? Yes, it stands for back to back NMIs. I added additional comments to clarify how they are detected. https://lore.kernel.org/lkml/20250327234629.3953536-7-sohil.mehta@intel.com/ nmi_handle() still has special handling for back to back NMIs. But at some point, there was a b2b parameter that was passed to nmi_handle(). AFAICT, that parameter was never really used. Sohil
On 3/31/2025 5:17 PM, Huang, Kai wrote:
> On Thu, 2025-03-27 at 23:46 +0000, Mehta, Sohil wrote:
>> The comment in unknown NMI handling is incorrect and misleading. There
>> is no longer a restriction on having a single Unknown NMI handler. Also,
>> nmi_handle() does not use the 'b2b' parameter anymore.
>>
>> The changes that made the comment outdated are:
>>
>> commit 0d443b70cc92 ("x86/platform: Remove warning message for duplicate
>> NMI handlers")
>>
>> commit bf9f2ee28d47 ("x86/nmi: Remove the 'b2b' parameter from
>> nmi_handle()")
>
> After some digging, IIUC, the 'b2b' parameter of the nmi_handle() was actually
> never used when it was originally added in the
>
You are right. The b2b parameter was never really used. The fixup patch
says the same thing as well.
"x86/nmi: Remove the 'b2b' parameter from nmi_handle()
It has never had any effect. Remove it for comprehensibility."
> commit b227e23399dc ("x86, nmi: Add in logic to handle multiple events and
> unknown NMIs")
>
It's probably okay even if we don't mention this directly in the
changelog. It is indirectly implied through the above fixup patch.
> .., so IIUC the comment was wrong when it was firstly added.
> > The above commit to remove the 'b2b' seems just a fixup patch but it
didn't fix
> the comment.
>
Yup, that's what I meant. The fixup patch should have fixed the comment
as well.
> Not sure whether it is worth to mention in the changelog.
>
>
> Anyway:
>
> Reviewed-by: Kai Huang <kai.huang@intel.com>
Thanks!
The following commit has been merged into the x86/nmi branch of tip:
Commit-ID: b4bc3144c1eca9107f45018000a1e68bfd308d8c
Gitweb: https://git.kernel.org/tip/b4bc3144c1eca9107f45018000a1e68bfd308d8c
Author: Sohil Mehta <sohil.mehta@intel.com>
AuthorDate: Thu, 27 Mar 2025 23:46:25
Committer: Ingo Molnar <mingo@kernel.org>
CommitterDate: Tue, 01 Apr 2025 22:26:11 +02:00
x86/nmi: Fix comment in unknown_nmi_error()
The comment in unknown_nmi_error() is incorrect and misleading. There
is no longer a restriction on having a single Unknown NMI handler. Also,
nmi_handle() never used the 'b2b' parameter.
The commits that made the comment outdated are:
0d443b70cc92 ("x86/platform: Remove warning message for duplicate NMI handlers")
bf9f2ee28d47 ("x86/nmi: Remove the 'b2b' parameter from nmi_handle()")
Remove the old comment and update it to reflect the current logic.
Signed-off-by: Sohil Mehta <sohil.mehta@intel.com>
Signed-off-by: Ingo Molnar <mingo@kernel.org>
Reviewed-by: Kai Huang <kai.huang@intel.com>
Acked-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Reviewed-by: Nikolay Borisov <nik.borisov@suse.com>
Link: https://lore.kernel.org/r/20250327234629.3953536-6-sohil.mehta@intel.com
---
arch/x86/kernel/nmi.c | 7 +++----
1 file changed, 3 insertions(+), 4 deletions(-)
diff --git a/arch/x86/kernel/nmi.c b/arch/x86/kernel/nmi.c
index cdfb386..2a07c9a 100644
--- a/arch/x86/kernel/nmi.c
+++ b/arch/x86/kernel/nmi.c
@@ -327,10 +327,9 @@ unknown_nmi_error(unsigned char reason, struct pt_regs *regs)
int handled;
/*
- * Use 'false' as back-to-back NMIs are dealt with one level up.
- * Of course this makes having multiple 'unknown' handlers useless
- * as only the first one is ever run (unless it can actually determine
- * if it caused the NMI)
+ * As a last resort, let the "unknown" handlers make a
+ * best-effort attempt to figure out if they can claim
+ * responsibility for this Unknown NMI.
*/
handled = nmi_handle(NMI_UNKNOWN, regs);
if (handled) {© 2016 - 2025 Red Hat, Inc.