linux-kernel - RE: [patch v2 7/8] KVM: x86: add Intel PT msr RTIT

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <82D7661F83C1A047AF7DC287873BF1E167E37575@SHSMSX101.ccr.corp.intel.com>
Date:   Tue, 14 Nov 2017 06:59:51 +0000
From:   "Kang, Luwei" <luwei.kang@...el.com>
To:     Paolo Bonzini <pbonzini@...hat.com>,
        "kvm@...r.kernel.org" <kvm@...r.kernel.org>
CC:     "rkrcmar@...hat.com" <rkrcmar@...hat.com>,
        "tglx@...utronix.de" <tglx@...utronix.de>,
        "mingo@...hat.com" <mingo@...hat.com>,
        "hpa@...or.com" <hpa@...or.com>, "x86@...nel.org" <x86@...nel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        Chao Peng <chao.p.peng@...ux.intel.com>
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))
> >