2017-11-14 07:01:20

by Luwei Kang

[permalink] [raw]
Subject: RE: [patch v2 7/8] KVM: x86: add Intel PT msr RTIT_CTL read/write

> > static DEFINE_PER_CPU(struct vmcs *, vmxarea); static
> > DEFINE_PER_CPU(struct vmcs *, current_vmcs); @@ -3384,6 +3385,11 @@
> > static int vmx_get_msr(struct kvm_vcpu *vcpu, struct msr_data *msr_info)
> > return 1;
> > msr_info->data = vcpu->arch.ia32_xss;
> > break;
> > + case MSR_IA32_RTIT_CTL:
> > + if (!vmx_pt_supported())
> > + return 1;
> > + msr_info->data = vmcs_read64(GUEST_IA32_RTIT_CTL);
> > + break;
> > case MSR_TSC_AUX:
> > if (!msr_info->host_initiated &&
> > !guest_cpuid_has(vcpu, X86_FEATURE_RDTSCP)) @@ -3508,6 +3514,11
> > @@ static int vmx_set_msr(struct kvm_vcpu *vcpu, struct msr_data *msr_info)
> > else
> > clear_atomic_switch_msr(vmx, MSR_IA32_XSS);
> > break;
> > + case MSR_IA32_RTIT_CTL:
> > + if (!vmx_pt_supported() || to_vmx(vcpu)->nested.vmxon)
> > + return 1;
>
> VMXON must also clear TraceEn bit (see 23.4 in the SDM).

Will clear TraceEn bit in handle_vmon() function.

>
> You also need to support R/W of all the other MSRs, in order to make them accessible to userspace, and add them in msrs_to_save and kvm_init_msr_list.
>

Will add it in next version. This is use for live migration, is that right?

> Regarding the save/restore of the state, I am now wondering if you could also use XSAVES and XRSTORS instead of multiple rdmsr/wrmsr in a loop.
> The cost is two MSR writes on vmenter (because the guest must run with IA32_XSS=0) and two on vmexit.
>

If use XSAVES and XRSTORS for context switch.
1. Before kvm_load_guest_fpu(vcpu), we need to save host RTIT_CTL, disable PT and restore the value of "vmx->pt_desc.guest.ctl" to GUEST_IA32_RTIT_CTL. Is that right?
2. After VM-exit (step out from kvm_x86_ops->run(vcpu)), we need to save the status of GUEST_IA32_RTIT_CTL . TRACEEN=0 and others MSRs are still in guest status. Where to enable PT if in host-guest mode. I think we should enable PT after vm-exit but it may cause #GP. " If XRSTORS would restore (or initialize) PT state and IA32_RTIT_CTL.TraceEn = 1, the instruction causes a general-protection exception. SDM 13.5.6". if enable after kvm_put_guest_fpu() I think it too late.)

Thanks,
Luwei Kang
>
> > + vmcs_write64(GUEST_IA32_RTIT_CTL, data);
> > + break;
> > case MSR_TSC_AUX:
> > if (!msr_info->host_initiated &&
> > !guest_cpuid_has(vcpu, X86_FEATURE_RDTSCP))
> >