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

[Kang, Luwei]

> >  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))
> >