On 11/04/2024 3:29 am, Sean Christopherson wrote:
On Wed, Apr 10, 2024, Kai Huang wrote:
On Fri, 2024-03-22 at 14:23 -0700, Isaku Yamahata wrote:
+ r = atomic_read(&enable.err);
+ if (!r)
+ r = tdx_module_setup();
+ else
+ r = -EIO;
+ on_each_cpu(vmx_off, &enable.enabled, true);
+ cpus_read_unlock();
+ free_cpumask_var(enable.enabled);
+
+out:
+ return r;
+}
At last, I think there's one problem here:
KVM actually only registers CPU hotplug callback in kvm_init(), which happens
way after tdx_hardware_setup().
What happens if any CPU goes online *BETWEEN* tdx_hardware_setup() and
kvm_init()?
Looks we have two options:
1) move registering CPU hotplug callback before tdx_hardware_setup(), or
2) we need to disable CPU hotplug until callbacks have been registered.
This is all so dumb (not TDX, the current state of KVM). All of the hardware
enabling crud is pointless complex inherited from misguided, decade old paranoia
that led to the decision to enable VMX if and only if VMs are running. Enabling
VMX doesn't make the system less secure, and the insane dances we are doing to
do VMXON on-demand makes everything *more* fragile.
And all of this complexity really was driven by VMX, enabling virtualization for
every other vendor, including AMD/SVM, is completely uninteresting. Forcing other
architectures/vendors to take on yet more complexity doesn't make any sense.
Ah, I actually preferred this solution, but I was trying to follow your
suggestion here:
https://lore.kernel.org/lkml/ZW6FRBnOwYV-UCkY@xxxxxxxxxx/
form which I interpreted you didn't like always having VMX enabled when
KVM is present. :-)
Barely tested, and other architectures would need to be converted, but I don't
see any obvious reasons why we can't simply enable virtualization when the module
is loaded.
The diffstat pretty much says it all.
Thanks a lot for the code!
I can certainly follow up with this and generate a reviewable patchset
if I can confirm with you that this is what you want?
