[PATCH v16 29/51] KVM: VMX: Configure nested capabilities after CPU capabilities

Sean Christopherson posted 51 patches 1 week, 5 days ago
[PATCH v16 29/51] KVM: VMX: Configure nested capabilities after CPU capabilities
Posted by Sean Christopherson 1 week, 5 days ago
Swap the order between configuring nested VMX capabilities and base CPU
capabilities, so that nested VMX support can be conditioned on core KVM
support, e.g. to allow conditioning support for LOAD_CET_STATE on the
presence of IBT or SHSTK.  Because the sanity checks on nested VMX config
performed by vmx_check_processor_compat() run _after_ vmx_hardware_setup(),
any use of kvm_cpu_cap_has() when configuring nested VMX support will lead
to failures in vmx_check_processor_compat().

While swapping the order of two (or more) configuration flows can lead to
a game of whack-a-mole, in this case nested support inarguably should be
done after base support.  KVM should never condition base support on nested
support, because nested support is fully optional, while obviously it's
desirable to condition nested support on base support.  And there's zero
evidence the current ordering was intentional, e.g. commit 66a6950f9995
("KVM: x86: Introduce kvm_cpu_caps to replace runtime CPUID masking")
likely placed the call to kvm_set_cpu_caps() after nested setup because it
looked pretty.

Signed-off-by: Sean Christopherson <seanjc@google.com>
---
 arch/x86/kvm/vmx/vmx.c | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c
index 69e35440cee7..29e1bc118479 100644
--- a/arch/x86/kvm/vmx/vmx.c
+++ b/arch/x86/kvm/vmx/vmx.c
@@ -8602,6 +8602,13 @@ __init int vmx_hardware_setup(void)
 
 	setup_default_sgx_lepubkeyhash();
 
+	vmx_set_cpu_caps();
+
+	/*
+	 * Configure nested capabilities after core CPU capabilities so that
+	 * nested support can be conditional on base support, e.g. so that KVM
+	 * can hide/show features based on kvm_cpu_cap_has().
+	 */
 	if (nested) {
 		nested_vmx_setup_ctls_msrs(&vmcs_config, vmx_capability.ept);
 
@@ -8610,8 +8617,6 @@ __init int vmx_hardware_setup(void)
 			return r;
 	}
 
-	vmx_set_cpu_caps();
-
 	r = alloc_kvm_area();
 	if (r && nested)
 		nested_vmx_hardware_unsetup();
-- 
2.51.0.470.ga7dc726c21-goog
Re: [PATCH v16 29/51] KVM: VMX: Configure nested capabilities after CPU capabilities
Posted by Chao Gao 1 week, 2 days ago
On Fri, Sep 19, 2025 at 03:32:36PM -0700, Sean Christopherson wrote:
>Swap the order between configuring nested VMX capabilities and base CPU
>capabilities, so that nested VMX support can be conditioned on core KVM
>support, e.g. to allow conditioning support for LOAD_CET_STATE on the
>presence of IBT or SHSTK.  Because the sanity checks on nested VMX config
>performed by vmx_check_processor_compat() run _after_ vmx_hardware_setup(),
>any use of kvm_cpu_cap_has() when configuring nested VMX support will lead
>to failures in vmx_check_processor_compat().
>
>While swapping the order of two (or more) configuration flows can lead to
>a game of whack-a-mole, in this case nested support inarguably should be
>done after base support.  KVM should never condition base support on nested
>support, because nested support is fully optional, while obviously it's
>desirable to condition nested support on base support.  And there's zero
>evidence the current ordering was intentional, e.g. commit 66a6950f9995
>("KVM: x86: Introduce kvm_cpu_caps to replace runtime CPUID masking")
>likely placed the call to kvm_set_cpu_caps() after nested setup because it
>looked pretty.
>
>Signed-off-by: Sean Christopherson <seanjc@google.com>

Reviewed-by: Chao Gao <chao.gao@intel.com>

I had a feeling I'd seen this patch before :). After some searching in lore, I
tracked it down:
https://lore.kernel.org/kvm/20241001050110.3643764-22-xin@zytor.com/
Re: [PATCH v16 29/51] KVM: VMX: Configure nested capabilities after CPU capabilities
Posted by Sean Christopherson 1 week, 1 day ago
On Tue, Sep 23, 2025, Chao Gao wrote:
> On Fri, Sep 19, 2025 at 03:32:36PM -0700, Sean Christopherson wrote:
> >Swap the order between configuring nested VMX capabilities and base CPU
> >capabilities, so that nested VMX support can be conditioned on core KVM
> >support, e.g. to allow conditioning support for LOAD_CET_STATE on the
> >presence of IBT or SHSTK.  Because the sanity checks on nested VMX config
> >performed by vmx_check_processor_compat() run _after_ vmx_hardware_setup(),
> >any use of kvm_cpu_cap_has() when configuring nested VMX support will lead
> >to failures in vmx_check_processor_compat().
> >
> >While swapping the order of two (or more) configuration flows can lead to
> >a game of whack-a-mole, in this case nested support inarguably should be
> >done after base support.  KVM should never condition base support on nested
> >support, because nested support is fully optional, while obviously it's
> >desirable to condition nested support on base support.  And there's zero
> >evidence the current ordering was intentional, e.g. commit 66a6950f9995
> >("KVM: x86: Introduce kvm_cpu_caps to replace runtime CPUID masking")
> >likely placed the call to kvm_set_cpu_caps() after nested setup because it
> >looked pretty.
> >
> >Signed-off-by: Sean Christopherson <seanjc@google.com>
> 
> Reviewed-by: Chao Gao <chao.gao@intel.com>
> 
> I had a feeling I'd seen this patch before :). After some searching in lore, I
> tracked it down:
> https://lore.kernel.org/kvm/20241001050110.3643764-22-xin@zytor.com/

Gah, sorry Xin :-/
Re: [PATCH v16 29/51] KVM: VMX: Configure nested capabilities after CPU capabilities
Posted by Xin Li 1 week, 1 day ago
On 9/23/2025 9:24 AM, Sean Christopherson wrote:
>> I had a feeling I'd seen this patch before 🙂. After some searching in lore, I
>> tracked it down:
>> https://lore.kernel.org/kvm/20241001050110.3643764-22-xin@zytor.com/
> Gah, sorry Xin :-/

Oh, Chao really has a good memory.