On Fri, Dec 02, 2022, Huang, Kai wrote:

On Wed, 2022-11-30 at 23:09 +0000, Sean Christopherson wrote:

quoted

--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c

@@ -11967,6 +11967,11 @@ int kvm_arch_hardware_enable(void)

 	bool stable, backwards_tsc = false;
 
 	kvm_user_return_msr_cpu_online();
+
+	ret = kvm_x86_check_processor_compatibility();
+	if (ret)
+		return ret;
+
 	ret = static_call(kvm_x86_hardware_enable)();
 	if (ret != 0)
 		return ret;

Thinking more, AFAICT, kvm_x86_vendor_init() so far still does the compatibility
check on all online cpus.  Since now kvm_arch_hardware_enable() also does the
compatibility check, IIUC the compatibility check will be done twice -- one in
kvm_x86_vendor_init() and one in hardware_enable_all() when creating the first
VM.

Do you think it's still worth to do compatibility check in vm_x86_vendor_init()?

The behaviour difference should be "KVM module fail to load" vs "failing to
create the first VM" IIUC.  I don't know whether the former is better than the
better, but it seems duplicated compatibility checking isn't needed?

It's not strictly needed, but I think it's worth keeping.  The duplicate checking
annoys me too, and I considered removing it multiple times when creating this
series.  But, if there is a hardware incompatibility for whatever reason, failing
to load and thus not instantiating /dev/kvm is friendlier to userspace, e.g.
userspace can immediately flag the platform as potentially flaky, whereas
detecting the likely hardware issue when VM creation fails would essentialy require
scraping the kernel logs.