The original reply was sent to Sean only by mistake. Add others back. On Fri, Apr 01, 2022 at 11:27:42AM +0800, Chao Gao wrote: >On Thu, Mar 31, 2022 at 07:34:12PM +0000, Sean Christopherson wrote: >>+Chao Gao >> >>On Thu, Mar 31, 2022, Isaku Yamahata wrote: >>> On Thu, Mar 31, 2022 at 12:03:15AM +0000, Sean Christopherson <seanjc@xxxxxxxxxx> wrote: >>> > On Mon, Mar 14, 2022, Isaku Yamahata wrote: >>> > > - VMXON on all pCPUs: The TDX module initialization requires to enable VMX >>> > > (VMXON) on all present pCPUs. vmx_hardware_enable() which is called on creating >>> > > guest does it. It naturally fits with the TDX module initialization at creating >>> > > first TD. I wanted to avoid code to enable VMXON on loading the kvm_intel.ko. >>> > >>> > That's a solvable problem, though making it work without exporting hardware_enable_all() >>> > could get messy. >>> >>> Could you please explain any reason why it's bad idea to export it? >> >>I'd really prefer to keep the hardware enable/disable logic internal to kvm_main.c >>so that all architectures share a common flow, and so that kvm_main.c is the sole >>owner. I'm worried that exposing the helper will lead to other arch/vendor usage, >>and that will end up with what is effectively duplicate flows. Deduplicating arch >>code into generic KVM is usually very difficult. >> >>This might also be a good opportunity to make KVM slightly more robust. Ooh, and >>we can kill two birds with one stone. There's an in-flight series to add compatibility >>checks to hotplug[*]. But rather than special case hotplug, what if we instead do >>hardware enable/disable during module load, and move the compatibility check into >>the hardware_enable path? That fixes the hotplug issue, gives TDX a window for running >>post-VMXON code in kvm_init(), and makes the broadcast IPI less wasteful on architectures >>that don't have compatiblity checks. > >Sounds good. But more time is wasted on compat checks on architectures >that have them because they are done each time of enabling hardware. >A solution for this is caching the result of kvm_arch_check_processor_compat(). > >> >>I'm thinking something like this, maybe as a modificatyion to patch 6 in Chao's >>series, or more likely as a patch 7 so that the hotplug compat checks still get >>in even > >>if the early hardware enable doesn't work on all architectures for some >>reason. > >By "early", do you mean hardware enable during module loading or during CPU hotplug? > >And if below change is put into my series, kvm_arch_post_hardware_enable_setup() >will be an empty function for all architectures until TDX series gets merged. >So, I prefer to drop kvm_arch_post_hardware_enable_setup() and let TDX series >introduce it.
