| Message ID | 20241001050110.3643764-22-xin@zytor.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Enable FRED with KVM VMX | expand |
On Mon, Sep 30, 2024 at 10:01:04PM -0700, Xin Li (Intel) wrote: >From: Xin Li <xin3.li@intel.com> > >Set VMX CPU capabilities before initializing nested instead of after, >as it needs to check VMX CPU capabilities to setup the VMX basic MSR >for nested. Which VMX CPU capabilities are needed? after reading patch 25, I still don't get that. > >Signed-off-by: Xin Li <xin3.li@intel.com> >Signed-off-by: Xin Li (Intel) <xin@zytor.com> >Tested-by: Shan Kang <shan.kang@intel.com> >--- > arch/x86/kvm/vmx/vmx.c | 8 ++++++-- > 1 file changed, 6 insertions(+), 2 deletions(-) > >diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c >index ef807194ccbd..522ee27a4655 100644 >--- a/arch/x86/kvm/vmx/vmx.c >+++ b/arch/x86/kvm/vmx/vmx.c >@@ -8774,6 +8774,12 @@ __init int vmx_hardware_setup(void) > > setup_default_sgx_lepubkeyhash(); > >+ /* >+ * VMX CPU capabilities are required to setup the VMX basic MSR for >+ * nested, so this must be done before nested_vmx_setup_ctls_msrs(). >+ */ >+ vmx_set_cpu_caps(); >+ > if (nested) { > nested_vmx_setup_ctls_msrs(&vmcs_config, vmx_capability.ept); > >@@ -8782,8 +8788,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.46.2 > >
On 10/24/2024 12:49 AM, Chao Gao wrote: > On Mon, Sep 30, 2024 at 10:01:04PM -0700, Xin Li (Intel) wrote: >> From: Xin Li <xin3.li@intel.com> >> >> Set VMX CPU capabilities before initializing nested instead of after, >> as it needs to check VMX CPU capabilities to setup the VMX basic MSR >> for nested. > > Which VMX CPU capabilities are needed? after reading patch 25, I still > don't get that. Sigh, in v2 I had 'if (kvm_cpu_cap_has(X86_FEATURE_FRED))' in nested_vmx_setup_basic(), which is changed to 'if (cpu_has_vmx_fred())' in v3. So the reason for the change is gone. But I think logically the change is still needed; nested setup should be after VMX setup.
diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c index ef807194ccbd..522ee27a4655 100644 --- a/arch/x86/kvm/vmx/vmx.c +++ b/arch/x86/kvm/vmx/vmx.c @@ -8774,6 +8774,12 @@ __init int vmx_hardware_setup(void) setup_default_sgx_lepubkeyhash(); + /* + * VMX CPU capabilities are required to setup the VMX basic MSR for + * nested, so this must be done before nested_vmx_setup_ctls_msrs(). + */ + vmx_set_cpu_caps(); + if (nested) { nested_vmx_setup_ctls_msrs(&vmcs_config, vmx_capability.ept); @@ -8782,8 +8788,6 @@ __init int vmx_hardware_setup(void) return r; } - vmx_set_cpu_caps(); - r = alloc_kvm_area(); if (r && nested) nested_vmx_hardware_unsetup();