Messages in this thread | | | Subject | Re: [PATCH] KVM: vmx: Fix the broken usage of vmx_xsaves_supported | From | Xiaoyao Li <> | Date | Thu, 20 Jun 2019 14:46:50 +0800 |
| |
On 6/20/2019 2:40 PM, Wanpeng Li wrote: > Hi, > On Thu, 20 Jun 2019 at 13:06, Tao Xu <tao3.xu@intel.com> wrote: >> >> The helper vmx_xsaves_supported() returns the bit value of >> SECONDARY_EXEC_XSAVES in vmcs_config.cpu_based_2nd_exec_ctrl, which >> remains unchanged true if vmcs supports 1-setting of this bit after >> setup_vmcs_config(). It should check the guest's cpuid not this >> unchanged value when get/set msr. >> >> Besides, vmx_compute_secondary_exec_control() adjusts >> SECONDARY_EXEC_XSAVES bit based on guest cpuid's X86_FEATURE_XSAVE >> and X86_FEATURE_XSAVES, it should use updated value to decide whether >> set XSS_EXIT_BITMAP. >> >> Co-developed-by: Xiaoyao Li <xiaoyao.li@linux.intel.com> >> Signed-off-by: Xiaoyao Li <xiaoyao.li@linux.intel.com> >> Signed-off-by: Tao Xu <tao3.xu@intel.com> >> --- >> arch/x86/kvm/vmx/vmx.c | 8 +++++--- >> 1 file changed, 5 insertions(+), 3 deletions(-) >> >> diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c >> index b93e36ddee5e..935cf72439a9 100644 >> --- a/arch/x86/kvm/vmx/vmx.c >> +++ b/arch/x86/kvm/vmx/vmx.c >> @@ -1721,7 +1721,8 @@ static int vmx_get_msr(struct kvm_vcpu *vcpu, struct msr_data *msr_info) >> return vmx_get_vmx_msr(&vmx->nested.msrs, msr_info->index, >> &msr_info->data); >> case MSR_IA32_XSS: >> - if (!vmx_xsaves_supported()) >> + if (!guest_cpuid_has(vcpu, X86_FEATURE_XSAVE) || >> + !guest_cpuid_has(vcpu, X86_FEATURE_XSAVES)) >> return 1; >> msr_info->data = vcpu->arch.ia32_xss; >> break; >> @@ -1935,7 +1936,8 @@ static int vmx_set_msr(struct kvm_vcpu *vcpu, struct msr_data *msr_info) >> return 1; >> return vmx_set_vmx_msr(vcpu, msr_index, data); >> case MSR_IA32_XSS: >> - if (!vmx_xsaves_supported()) >> + if (!guest_cpuid_has(vcpu, X86_FEATURE_XSAVE) || >> + !guest_cpuid_has(vcpu, X86_FEATURE_XSAVES)) >> return 1; > > Not complete true. > >> /* >> * The only supported bit as of Skylake is bit 8, but >> @@ -4094,7 +4096,7 @@ static void vmx_vcpu_setup(struct vcpu_vmx *vmx) >> >> set_cr4_guest_host_mask(vmx); >> >> - if (vmx_xsaves_supported()) >> + if (vmx->secondary_exec_control & SECONDARY_EXEC_XSAVES) >> vmcs_write64(XSS_EXIT_BITMAP, VMX_XSS_EXIT_BITMAP); > > This is not true. > > SDM 24.6.20: > On processors that support the 1-setting of the “enable > XSAVES/XRSTORS” VM-execution control, the VM-execution control fields > include a 64-bit XSS-exiting bitmap. > > It depends on whether or not processors support the 1-setting instead > of “enable XSAVES/XRSTORS” is 1 in VM-exection control field. Anyway,
Yes, whether this field exist or not depends on whether processors support the 1-setting.
But if "enable XSAVES/XRSTORS" is clear to 0, XSS_EXIT_BITMAP doesn't work. I think in this case, there is no need to set this vmcs field?
> I will send a patch to fix the msr read/write for commit > 203000993de5(kvm: vmx: add MSR logic for XSAVES), thanks for the > report. > > Regards, > Wanpeng Li >
| |