linux-kernel - Re: [PATCH 1/4] x86/reboot: Unconditionally define cpu_emergency_virt

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: <00c59dce-e1e4-47cf-a109-722a033b00d8@intel.com>
Date: Wed, 22 May 2024 09:43:04 +1200
From: "Huang, Kai" <kai.huang@...el.com>
To: Sean Christopherson <seanjc@...gle.com>
CC: "pbonzini@...hat.com" <pbonzini@...hat.com>, "kvm@...r.kernel.org"
	<kvm@...r.kernel.org>, "linux-kernel@...r.kernel.org"
	<linux-kernel@...r.kernel.org>
Subject: Re: [PATCH 1/4] x86/reboot: Unconditionally define
 cpu_emergency_virt_cb typedef



On 22/05/2024 8:02 am, Sean Christopherson wrote:
> On Wed, May 15, 2024, Kai Huang wrote:
>> How about we just make all emergency virtualization disable code
>> unconditional but not guided by CONFIG_KVM_INTEL || CONFIG_KVM_AMD, i.e.,
>> revert commit
>>
>>     261cd5ed934e ("x86/reboot: Expose VMCS crash hooks if and only if
>> KVM_{INTEL,AMD} is enabled")
>>
>> It makes sense anyway from the perspective that it allows the out-of-tree
>> kernel module hypervisor to use this mechanism w/o needing to have the
>> kernel built with KVM enabled in Kconfig.  Otherwise, strictly speaking,
>> IIUC, the kernel won't be able to support out-of-tree module hypervisor as
>> there's no other way the module can intercept emergency reboot.
> 
> Practically speaking, no one is running an out-of-tree hypervisor without either
> (a) KVM being enabled in the .config, or (b) non-trivial changes to the kernel.

Just for curiosity: why b) is required to support out-of-tree hypervisor 
when KVM is disabled in Kconfig?  I am probably missing something.

> 
> Exposing/exporting select APIs and symbols if and only if KVM is enabled is a
> a well-established pattern, and there are concrete benefits to doing so.  E.g.
> it allows minimizing the kernel footprint for use cases that don't want/need KVM.
> 
>> This approach avoids the weirdness of the unconditional define for only
>> cpu_emergency_virt_cb.
> 
> I genuinely don't understand why you find it weird to unconditionally define
> cpu_emergency_virt_cb.  There are myriad examples throughout the kernel where a
> typedef, struct, enum, etc. is declared/defined even though support for its sole
> end consumer is disabled.  E.g. include/linux/mm_types.h declares "struct mem_cgroup"
> for pretty much the exact same reason, even though the structure is only fully
> defined if CONFIG_MEMCG=y.
> 
> The only oddity here is that the API that the #ifdef that guards the usage happens
> to be right below the typedef, but it shouldn't take that much brain power to
> figure out why a typedef exists outside of an #ifdef.

OK.  No more arguments.  :-)

Thanks for this series anyway.