Received-SPF: pass (google.com: domain of linux-kernel+bounces-185489-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.199.223 as permitted sender) client-ip=147.75.199.223;
Message-ID: <00c59dce-e1e4-47cf-a109-722a033b00d8@intel.com>
Date: Wed, 22 May 2024 09:43:04 +1200
User-Agent: Mozilla Thunderbird
Subject: Re: [PATCH 1/4] x86/reboot: Unconditionally define
 cpu_emergency_virt_cb typedef
To: Sean Christopherson <seanjc@google.com>
CC: "pbonzini@redhat.com" <pbonzini@redhat.com>, "kvm@vger.kernel.org"
	<kvm@vger.kernel.org>, "linux-kernel@vger.kernel.org"
	<linux-kernel@vger.kernel.org>
References: <20240425233951.3344485-1-seanjc@google.com>
 <20240425233951.3344485-2-seanjc@google.com>
 <5dfc9eb860a587d1864371874bbf267fa0aa7922.camel@intel.com>
 <ZkI5WApAR6iqCgil@google.com>
 <6100e822-378b-422e-8ff8-f41b19785eea@intel.com>
 <1dbb09c5-35c7-49b9-8c6f-24b532511f0b@intel.com>
 <Zkz97y9VVAFgqNJB@google.com>
Content-Language: en-US
From: "Huang, Kai" <kai.huang@intel.com>
In-Reply-To: <Zkz97y9VVAFgqNJB@google.com>
Content-Type: text/plain; charset="UTF-8"; format=flowed
Content-Transfer-Encoding: 7bit
Precedence: bulk
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?M3JTQ0lUOFNnWmx1UUZrMmtpRGFEcWN6M1ltTkI3Zkc3RnF3cDJVdUE5S2ov?=
 =?utf-8?B?TktjRHVXZUdrbm9yT3dhMHZyUUs1MS9QMjZLNTlmQTcyZWwvSU8yUVphS25u?=
 =?utf-8?B?MENkVEJGc1B0dGVOeFg1TWpGUDR3QTU3RzhaNXV6TlBsWjBDdzJTc2tjamF0?=
 =?utf-8?B?aUcyaUMveER5ZFFpR29raTFOR0hzd2h5SlZTUTNoOTdYOWdiOWNOOUZnVU5J?=
 =?utf-8?B?MkN3Qk51ZHdBbkxiNmw4R2x3eUVLOEx5TUFmV0dwTkpPNnVVQ0xuUk9lVkxE?=
 =?utf-8?B?K2ZFMlNUeFlCTEloMG9PaVNRaTFWRUhPeWQ4QjlnblE2ZnB5WDVSVGRLMjV6?=
 =?utf-8?B?aTVCTC9Yam9OUEsxelNVeVorWGtwcGpuM1hVSDR2dTNVSjRYaEFVbUFUdGJG?=
 =?utf-8?B?TFVnNE9zemtqYVpUcDB6Ulg3REpLenJQbkkxWjR2Z28rWDVnTm5sWUl0dDFa?=
 =?utf-8?B?Y1ROcW5BK2dXQmhsRk9yOUxsWFdCeUhLdDRCalEwL215RGxVdENnY0tLVmd3?=
 =?utf-8?B?MmJHYU0zeE9Cd2Y2cXBLMUZQOUVpcTc2bUJMY2VpR1IrenpNOHJPUnZ5R2Mz?=
 =?utf-8?B?SDF4RmN1RDVIZVF1Qk9wZnpPQ2dvYXlLSXZZdmpvZlBNY2xpWnFzK2JjcVkr?=
 =?utf-8?B?MXI0UUtFdWMvblM0NDBYV1RENnp6M2ljUitTQnBIOGpZZDN5dTJldVd3Z3hy?=
 =?utf-8?B?ZDNTVnBvajA2cFdOZTd5MGgwRUkyTFgxOVNwMXJkbTZRU285dTVBU1NTSnFR?=
 =?utf-8?B?RFFXZ2JLTDQ1ZFBld25GRmY2bXlaamMwei9mQnJwaXR2d3N2WTh4K25Lb3Z2?=
 =?utf-8?B?ZmFSVVN4dnA5U045SU1FMWdSZEFyRytEWmtjVHhyTlRWbGtHSnRBajZJbDBN?=
 =?utf-8?B?UExob0ZxM1FxS1RIODJQYnhPTm5GdTJ6QzlaRWc2NzhHSzM5cTNXQTlLeEtj?=
 =?utf-8?B?M3FLREVQOTRZRlRKN01xaFd2L3VzdE42bHJkN2g5b3JTam9TekYrUVlkZGd4?=
 =?utf-8?B?OEovT2Ura2x1bkZoZzRIbkJXSUozN1VjTVJsVWFxaE1SQW9GbXpJeFg3djJT?=
 =?utf-8?B?RnkzRXllSm8rbmxwQ2RMRVczcE5KSDFVVytWNnJmdjJPRHNpS2g1dHBBc1FG?=
 =?utf-8?B?aVhlNC9zTk1QQ2tZdk92dVV0NndkQlZnTDg3Y3E0aE10MEV3YThEblVHZmto?=
 =?utf-8?B?aFFGTlFTWkZCcXRyVEUxd3BBclFUVnkxZ2pTaVhpN21IOEZhTm1BS2c5QzBw?=
 =?utf-8?B?bDBuKzFUS0xuYW5tQndHSmVnemZNQTd2UXRtWjc0MElZemZPcEdFQ3JrZmxY?=
 =?utf-8?B?cXhXMFdyaW5RZmxRaGZSM2NlK0Rsd2FYUHRnWjY0ZlpoUHJWak5NaWhweTh6?=
 =?utf-8?B?dzNzU1NCREkxd0U4UGR6WG5RbVptWnVTc1ppcEU0Q0s0akFYT3VlU0gwWmlX?=
 =?utf-8?B?OWhLNUVMVUhwbWZ0YUxLR0JPQnh2ODFsTzNXT2s0NkY3YU9UMnBOYXFzVk1v?=
 =?utf-8?B?VGtkdFIwME9FRGVubW5SN1diODJyMkh2VXRqVkQvbFU5cGtNUXFYbEZ0OEsw?=
 =?utf-8?B?ZVNodVBpSU92eDRkQ1h6S1pZZ3c5SFN4NEZlSHM3MklQakxYZUZWNEp5K3ZP?=
 =?utf-8?B?Mi9SN3RZZnBaZGFKU2prY3ZJK2EwRnNhODgvTXRIWmxKVGN3N255dVAvRzBr?=
 =?utf-8?B?emZ5Q2pJWU4rcFI5RFRKcWtMOHlkbTVYWk1PZ3EzL2xvVmhmajBwNEFqa0sr?=
 =?utf-8?B?ZkxHS3BZb0ZvMHpPYkhLWXRtWkQ2UWJXYzgybFJrRmw5cTkyZG5UV1NhTVlZ?=
 =?utf-8?B?U1ZWTGlYQWtWRk1BTElxNlRBTVU3Y3g5TVJXS0dGYnFMOU9iRWNZZWNxY2Fu?=
 =?utf-8?B?U0tGK2VDcHlDTVZtbUZjUGhXWEtPM25zdTliL0lmRTA3MFpwWjl0WDlVZFQ0?=
 =?utf-8?B?OTdkTVB3N3I4VklHS3A2dndhdko3YUx5dmxxRW1NOGE0NjQ0eUlCWkhDVmJq?=
 =?utf-8?B?cnNIRGZlMWZiYy9NekRpeVR5aHBrbzRHMFMxc1RHSnUyUHdQQk5UZW9IT1Jy?=
 =?utf-8?B?ZWNIRlJlNktxZFJXQzlEaVJpd0JRR28vSzhMOWdFUHZ3RnVrUjNrY2tFSmkv?=
 =?utf-8?Q?BYv1ZFG7G5vzE8lkuxmV5s8rO?=
X-MS-Exchange-CrossTenant-Network-Message-Id: dde6ea4e-7d6e-4567-9ec1-08dc79df02af
X-MS-Exchange-CrossTenant-AuthSource: BL1PR11MB5978.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 21 May 2024 21:43:11.1676
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 46c98d88-e344-4ed4-8496-4ed7712e255d
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: 8xN4BF/LabgkheWud7/47eO8cjaks8PwF7tB50ZZT/2RbhkUxkWw1tYKdIgj2C3bu/5yRLwuckDYZ71AMB5fmA==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: MW6PR11MB8440
X-OriginatorOrg: intel.com


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.