linux-kernel - Re: [PATCH 0/9] x86: Cleanup NMI handling

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <F5D36889-A868-46D2-A678-8EE26E28556D@zytor.com>
Date: Wed, 02 Apr 2025 08:13:24 -0700
From: "H. Peter Anvin" <hpa@...or.com>
To: Sohil Mehta <sohil.mehta@...el.com>, x86@...nel.org,
        Thomas Gleixner <tglx@...utronix.de>, Ingo Molnar <mingo@...hat.com>
CC: Dave Hansen <dave.hansen@...ux.intel.com>,
        Josh Poimboeuf <jpoimboe@...nel.org>,
        Peter Zijlstra <peterz@...radead.org>,
        "Kirill A . Shutemov" <kirill.shutemov@...ux.intel.com>,
        Kai Huang <kai.huang@...el.com>,
        Sebastian Andrzej Siewior <bigeasy@...utronix.de>,
        Mike Rapoport <rppt@...nel.org>, Petr Mladek <pmladek@...e.com>,
        Jani Nikula <jani.nikula@...el.com>, Tony Luck <tony.luck@...el.com>,
        Xin Li <xin@...or.com>, linux-kernel@...r.kernel.org
Subject: Re: [PATCH 0/9] x86: Cleanup NMI handling

On March 27, 2025 4:46:20 PM PDT, Sohil Mehta <sohil.mehta@...el.com> wrote:
>While reworking the NMI-source patches[1], I spent quite a few days
>understanding the existing NMI handling code. The intention of this series is
>to fix the inconsistencies and redundancies that I encountered.
>
>This series also includes improved documentation to make the code easier to
>follow. It does not introduce any significant functional changes.
>
>I have tried to split the patches logically to make them easier to review. Let
>me know if some of them should be combined. The patches are in no particular
>order and can be picked up independently. 
>
>The documentation updates in patches (5, 6, 7) are to the best of my ability.
>However, I would really appreciate extra eyes to ensure it is precise.
>
>The updated NMI-source patches will follow this series sometime later.
>
>@Ingo, I tried to format the commit references the way you prefer:
>
>  Commit:
>    feeaf5512947 ("x86: Move sysctls into arch/x86")
>
>However, checkpatch seems to dislike it, so I reformatted them as below:
>
>  Commit feeaf5512947 ("x86: Move sysctls into arch/x86")
>
>Is there a specific guideline for x86?
>
>[1]: https://lore.kernel.org/lkml/20240709143906.1040477-1-jacob.jun.pan@linux.intel.com/
>
>Sohil Mehta (9):
>  x86/nmi: Simplify unknown NMI panic handling
>  x86/nmi: Consolidate NMI panic variables
>  x86/nmi: Use a macro to initialize NMI descriptors
>  x86/nmi: Remove export of local_touch_nmi()
>  x86/nmi: Fix comment in unknown NMI handling
>  x86/nmi: Improve and relocate NMI handler comments
>  x86/nmi: Improve NMI header documentation
>  x86/nmi: Clean up NMI selftest
>  x86/nmi: Improve NMI duration console print
>
> arch/x86/include/asm/nmi.h      | 49 +++++++++++++++++--
> arch/x86/include/asm/x86_init.h |  1 +
> arch/x86/kernel/dumpstack.c     |  2 -
> arch/x86/kernel/nmi.c           | 87 ++++++++++++++++-----------------
> arch/x86/kernel/nmi_selftest.c  | 52 ++++++--------------
> arch/x86/kernel/setup.c         | 37 ++++++--------
> include/linux/panic.h           |  2 -
> 7 files changed, 122 insertions(+), 108 deletions(-)
>

Talking about NMI, one thing I would like to see done would be to explicitly enable NMI at the point where we are ready to take them. 

If we enter the kernel with NMIs disabled, using IDT they will get spuriously reenabled the first time we do IRET, but using FRED they could stay disabled until we enter user space.

It also seems "cleaner".

We ought to already be doing something with the RTC NMI gate register, so that ought to be in the same logical spot, too.