On Thu, May 23, 2024 at 10:27:53AM +1200, Huang, Kai wrote: > > >On 22/05/2024 2:28 pm, Sean Christopherson wrote: >> Add an off-by-default module param, enable_virt_at_load, to let userspace >> force virtualization to be enabled in hardware when KVM is initialized, >> i.e. just before /dev/kvm is exposed to userspace. Enabling virtualization >> during KVM initialization allows userspace to avoid the additional latency >> when creating/destroying the first/last VM. Now that KVM uses the cpuhp >> framework to do per-CPU enabling, the latency could be non-trivial as the >> cpuhup bringup/teardown is serialized across CPUs, e.g. the latency could >> be problematic for use case that need to spin up VMs quickly. > >How about we defer this until there's a real complain that this isn't >acceptable? To me it doesn't sound "latency of creating the first VM" >matters a lot in the real CSP deployments. I suspect kselftest and kvm-unit-tests will be impacted a lot because hundreds of tests are run serially. And it looks clumsy to reload KVM module to set enable_virt_at_load to make tests run faster. I think the test slowdown is a more realistic problem than running an off-tree hypervisor, so I vote to make enabling virtualization at load time the default behavior and if we really want to support an off-tree hypervisor, we can add a new module param to opt in enabling virtualization at runtime. > >The concern of adding a new module param is once we add it, we need to >maintain it even it is no longer needed in the future for backward >compatibility. Especially this param is in kvm.ko, and for all ARCHs. > >E.g., I think _IF_ the core cpuhp code is enhanced to call those callbacks in >parallel in cpuhp_setup_state(), then this issue could be mitigated to an >unnoticeable level. > >Or we just still do: > > cpus_read_lock(); > on_each_cpu(hardware_enable_nolock, ...); > cpuhp_setup_state_nocalls_cpuslocked(...); > cpus_read_unlock(); > >I think the main benefit of series is to put all virtualization enabling >related things into one single function. Whether using cpuhp_setup_state() >or using on_each_cpu() shouldn't be the main point. >
