linux-kernel - Re: [PATCH 1/9] x86/nmi: Simplify unknown NMI panic 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: <88198f82-5a45-4a3d-af94-8ac1a4745b86@intel.com>
Date: Tue, 1 Apr 2025 08:54:55 -0700
From: Sohil Mehta <sohil.mehta@...el.com>
To: Nikolay Borisov <nik.borisov@...e.com>
CC: Borislav Petkov <bp@...en8.de>, Dave Hansen <dave.hansen@...ux.intel.com>,
	"H . Peter Anvin" <hpa@...or.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>, <x86@...nel.org>, Thomas
 Gleixner <tglx@...utronix.de>, Ingo Molnar <mingo@...hat.com>
Subject: Re: [PATCH 1/9] x86/nmi: Simplify unknown NMI panic handling

On 4/1/2025 8:00 AM, Nikolay Borisov wrote:

>> +#include <asm/realmode.h>
>>   #include <asm/thermal.h>
>>   #include <asm/unwind.h>
>>   #include <asm/vsyscall.h>
>> -#include <linux/vmalloc.h>
>> -#if defined(CONFIG_X86_LOCAL_APIC)
>> -#include <asm/nmi.h>
>> -#endif
> 
> As far as headers are concerned only this change falls under the 
> "simplify nmi handling code" the others while nice cleanups should 
> ideally be in a separate patch.
> 

You are right. The simplification is only for the #ifdef. I could have
just removed the CONFIG check and left the rest of the headers as-is.

But the <linux/vmalloc.h> include there was completely out of place and
needed to be moved. This eventually led to the reshuffle.

I feel header reordering by itself may not be considered worthwhile as a
patch. Since this series is focussed on cleanups, I tried to keep
cleanups in the same vicinity together. It's mainly to avoid bloating
the patch count but still keep the reviewing of these changes manageable.


>>   
>>   /*
>>    * max_low_pfn_mapped: highest directly mapped pfn < 4 GB
>> @@ -150,6 +147,13 @@ static size_t ima_kexec_buffer_size;
>>   int bootloader_type, bootloader_version;
>>   
>>   static const struct ctl_table x86_sysctl_table[] = {
>> +	{
>> +		.procname       = "unknown_nmi_panic",
>> +		.data           = &unknown_nmi_panic,
>> +		.maxlen         = sizeof(int),
>> +		.mode           = 0644,
>> +		.proc_handler   = proc_dointvec,
>> +	},
>>   	{
>>   		.procname	= "panic_on_unrecovered_nmi",
>>   		.data		= &panic_on_unrecovered_nmi,
>> @@ -185,15 +189,6 @@ static const struct ctl_table x86_sysctl_table[] = {
>>   		.mode		= 0644,
>>   		.proc_handler	= proc_dointvec,
>>   	},
>> -#if defined(CONFIG_X86_LOCAL_APIC)
>> -	{
>> -		.procname       = "unknown_nmi_panic",
>> -		.data           = &unknown_nmi_panic,
>> -		.maxlen         = sizeof(int),
>> -		.mode           = 0644,
>> -		.proc_handler   = proc_dointvec,
>> -	},
>> -#endif
> 
> Why move the definition and not just delete the #ifdef ?
> 

Mainly for consistency. This keeps all the NMI panic options together in
x86_sysctl_table[].

>>   #if defined(CONFIG_ACPI_SLEEP)
>>   	{
>>   		.procname	= "acpi_video_flags",
>