linux-kernel - RE: [patch v2 3/8] KVM: x86: add Intel processor trace virtualization mode

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: <82D7661F83C1A047AF7DC287873BF1E167E375C6@SHSMSX101.ccr.corp.intel.com>
Date:   Tue, 14 Nov 2017 07:11:16 +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 3/8] KVM: x86: add Intel processor trace
 virtualization mode

> > +#define VM_EXIT_PT_SUPPRESS_PIP			0x01000000
> > +#define VM_EXIT_CLEAR_IA32_RTIT_CTL		0x02000000
> >
> >  #define VM_EXIT_ALWAYSON_WITHOUT_TRUE_MSR	0x00036dff
> >
> > @@ -108,6 +112,8 @@
> >  #define VM_ENTRY_LOAD_IA32_PAT			0x00004000
> >  #define VM_ENTRY_LOAD_IA32_EFER                 0x00008000
> >  #define VM_ENTRY_LOAD_BNDCFGS                   0x00010000
> > +#define VM_ENTRY_PT_SUPPRESS_PIP		0x00020000
> > +#define VM_ENTRY_LOAD_IA32_RTIT_CTL		0x00040000
> 
> 
> Please use PT_CONCEAL instead of PT_SUPPRESS_PIP, to better match the SDM (for both vmexit and vmentry controls).
> 
> > +	if (!enable_ept)
> > +		vmexit_control &= ~VM_EXIT_CLEAR_IA32_RTIT_CTL;
> > +
> 
> Why is this (and the similar bit-clear operation in vmx_vmentry_control) needed only for !enable_ept?
> 
> Shouldn't it be like
> 
> 	if (pt_mode == PT_MODE_SYSTEM) {
> 		vmexit_control &= ~VM_EXIT_PT_SUPPRESS_PIP;
> 		vmexit_control &= ~VM_EXIT_CLEAR_IA32_RTIT_CTL;
> 	}
> 
> and
> 
> 	if (pt_mode == PT_MODE_SYSTEM) {
> 		vmentry_control &= ~VM_ENTRY_PT_SUPPRESS_PIP;
> 		vmentry_control &= ~VM_ENTRY_LOAD_IA32_RTIT_CTL;
> 	}
> 

I think I have a misunderstand of " always set "use GPA for processor tracing" in secondary execution control if it can be ".
"use GPA for processor tracing" can't be set in SYSTEM mode even if hardware can set this bit. Because guest will still think this a GPA address and translate by EPT. In fact, RTIT_OUTPUT_BASE will always a HPA in SYSTEM mode.
Will fix in next version.

Thanks,
Luwei Kang