| Message ID | 4d6eac657d082efaa0e7d141b5c9a07791b31f94.1594150543.git.michal.leszczynski@cert.pl (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Implement support for external IPT monitoring | expand |
On Tue, Jul 07, 2020 at 09:39:42PM +0200, Michał Leszczyński wrote: > From: Michal Leszczynski <michal.leszczynski@cert.pl> > > Check if Intel Processor Trace feature is supported by current > processor. Define vmtrace_supported global variable. IIRC there was some discussion in previous versions about whether vmtrace_supported should be globally exposed to all arches, since I see the symbol is defined in a common file I assume Arm maintainers agree to this approach, and hence it would be helpful to add a note to the commit message, ie: "The vmtrace_supported global variable is defined in common code with the expectation that other arches also supporting processor tracing can make use of it." > Signed-off-by: Michal Leszczynski <michal.leszczynski@cert.pl> LGTM: Reviewed-by: Roger Pau Monné <roger.pau@citrix.com> > --- > xen/arch/x86/hvm/vmx/vmcs.c | 15 ++++++++++++++- > xen/common/domain.c | 2 ++ > xen/include/asm-x86/cpufeature.h | 1 + > xen/include/asm-x86/hvm/vmx/vmcs.h | 1 + > xen/include/public/arch-x86/cpufeatureset.h | 1 + > xen/include/xen/domain.h | 2 ++ > 6 files changed, 21 insertions(+), 1 deletion(-) > > diff --git a/xen/arch/x86/hvm/vmx/vmcs.c b/xen/arch/x86/hvm/vmx/vmcs.c > index ca94c2bedc..3a53553f10 100644 > --- a/xen/arch/x86/hvm/vmx/vmcs.c > +++ b/xen/arch/x86/hvm/vmx/vmcs.c > @@ -291,6 +291,20 @@ static int vmx_init_vmcs_config(void) > _vmx_cpu_based_exec_control &= > ~(CPU_BASED_CR8_LOAD_EXITING | CPU_BASED_CR8_STORE_EXITING); > > + rdmsrl(MSR_IA32_VMX_MISC, _vmx_misc_cap); > + > + /* Check whether IPT is supported in VMX operation. */ > + if ( !smp_processor_id() ) > + vmtrace_supported = cpu_has_ipt && > + (_vmx_misc_cap & VMX_MISC_PROC_TRACE); Andrew also suggested to set vmtrace_supported to the value of cpu_has_ipt during CPU identification and then clear it here if VMX doesn't support IPT. Since this implementation won't add support for PV guests I'm not specially trilled and I think this approach is fine for the time being. If/When PV support is added we will have to re-arrange this a bit. Thanks.
On 07.07.2020 21:39, Michał Leszczyński wrote: > --- a/xen/arch/x86/hvm/vmx/vmcs.c > +++ b/xen/arch/x86/hvm/vmx/vmcs.c > @@ -291,6 +291,20 @@ static int vmx_init_vmcs_config(void) > _vmx_cpu_based_exec_control &= > ~(CPU_BASED_CR8_LOAD_EXITING | CPU_BASED_CR8_STORE_EXITING); > > + rdmsrl(MSR_IA32_VMX_MISC, _vmx_misc_cap); > + > + /* Check whether IPT is supported in VMX operation. */ > + if ( !smp_processor_id() ) Despite us not supporting offlining and re-onlining of the BSP (i.e. CPU 0) yet, I'm trying hard to keep new code from making assumptions like this one. Please don't base this on CPU number, but some other property (system_state, if nothing else can be found). Jan
diff --git a/xen/arch/x86/hvm/vmx/vmcs.c b/xen/arch/x86/hvm/vmx/vmcs.c index ca94c2bedc..3a53553f10 100644 --- a/xen/arch/x86/hvm/vmx/vmcs.c +++ b/xen/arch/x86/hvm/vmx/vmcs.c @@ -291,6 +291,20 @@ static int vmx_init_vmcs_config(void) _vmx_cpu_based_exec_control &= ~(CPU_BASED_CR8_LOAD_EXITING | CPU_BASED_CR8_STORE_EXITING); + rdmsrl(MSR_IA32_VMX_MISC, _vmx_misc_cap); + + /* Check whether IPT is supported in VMX operation. */ + if ( !smp_processor_id() ) + vmtrace_supported = cpu_has_ipt && + (_vmx_misc_cap & VMX_MISC_PROC_TRACE); + else if ( vmtrace_supported && + !(_vmx_misc_cap & VMX_MISC_PROC_TRACE) ) + { + printk("VMX: IPT capabilities fatally differ between CPU%u and CPU0\n", + smp_processor_id()); + return -EINVAL; + } + if ( _vmx_cpu_based_exec_control & CPU_BASED_ACTIVATE_SECONDARY_CONTROLS ) { min = 0; @@ -305,7 +319,6 @@ static int vmx_init_vmcs_config(void) SECONDARY_EXEC_ENABLE_VIRT_EXCEPTIONS | SECONDARY_EXEC_XSAVES | SECONDARY_EXEC_TSC_SCALING); - rdmsrl(MSR_IA32_VMX_MISC, _vmx_misc_cap); if ( _vmx_misc_cap & VMX_MISC_VMWRITE_ALL ) opt |= SECONDARY_EXEC_ENABLE_VMCS_SHADOWING; if ( opt_vpid_enabled ) diff --git a/xen/common/domain.c b/xen/common/domain.c index 7cc9526139..a45cf023f7 100644 --- a/xen/common/domain.c +++ b/xen/common/domain.c @@ -82,6 +82,8 @@ struct vcpu *idle_vcpu[NR_CPUS] __read_mostly; vcpu_info_t dummy_vcpu_info; +bool vmtrace_supported __read_mostly; + static void __domain_finalise_shutdown(struct domain *d) { struct vcpu *v; diff --git a/xen/include/asm-x86/cpufeature.h b/xen/include/asm-x86/cpufeature.h index f790d5c1f8..555f696a26 100644 --- a/xen/include/asm-x86/cpufeature.h +++ b/xen/include/asm-x86/cpufeature.h @@ -104,6 +104,7 @@ #define cpu_has_clwb boot_cpu_has(X86_FEATURE_CLWB) #define cpu_has_avx512er boot_cpu_has(X86_FEATURE_AVX512ER) #define cpu_has_avx512cd boot_cpu_has(X86_FEATURE_AVX512CD) +#define cpu_has_ipt boot_cpu_has(X86_FEATURE_PROC_TRACE) #define cpu_has_sha boot_cpu_has(X86_FEATURE_SHA) #define cpu_has_avx512bw boot_cpu_has(X86_FEATURE_AVX512BW) #define cpu_has_avx512vl boot_cpu_has(X86_FEATURE_AVX512VL) diff --git a/xen/include/asm-x86/hvm/vmx/vmcs.h b/xen/include/asm-x86/hvm/vmx/vmcs.h index 906810592f..6153ba6769 100644 --- a/xen/include/asm-x86/hvm/vmx/vmcs.h +++ b/xen/include/asm-x86/hvm/vmx/vmcs.h @@ -283,6 +283,7 @@ extern u32 vmx_secondary_exec_control; #define VMX_VPID_INVVPID_SINGLE_CONTEXT_RETAINING_GLOBAL 0x80000000000ULL extern u64 vmx_ept_vpid_cap; +#define VMX_MISC_PROC_TRACE 0x00004000 #define VMX_MISC_CR3_TARGET 0x01ff0000 #define VMX_MISC_VMWRITE_ALL 0x20000000 diff --git a/xen/include/public/arch-x86/cpufeatureset.h b/xen/include/public/arch-x86/cpufeatureset.h index fe7492a225..2c91862f2d 100644 --- a/xen/include/public/arch-x86/cpufeatureset.h +++ b/xen/include/public/arch-x86/cpufeatureset.h @@ -217,6 +217,7 @@ XEN_CPUFEATURE(SMAP, 5*32+20) /*S Supervisor Mode Access Prevention */ XEN_CPUFEATURE(AVX512_IFMA, 5*32+21) /*A AVX-512 Integer Fused Multiply Add */ XEN_CPUFEATURE(CLFLUSHOPT, 5*32+23) /*A CLFLUSHOPT instruction */ XEN_CPUFEATURE(CLWB, 5*32+24) /*A CLWB instruction */ +XEN_CPUFEATURE(PROC_TRACE, 5*32+25) /* Processor Tracing feature */ XEN_CPUFEATURE(AVX512PF, 5*32+26) /*A AVX-512 Prefetch Instructions */ XEN_CPUFEATURE(AVX512ER, 5*32+27) /*A AVX-512 Exponent & Reciprocal Instrs */ XEN_CPUFEATURE(AVX512CD, 5*32+28) /*A AVX-512 Conflict Detection Instrs */ diff --git a/xen/include/xen/domain.h b/xen/include/xen/domain.h index 7e51d361de..61ebc6c24d 100644 --- a/xen/include/xen/domain.h +++ b/xen/include/xen/domain.h @@ -130,4 +130,6 @@ struct vnuma_info { void vnuma_destroy(struct vnuma_info *vnuma); +extern bool vmtrace_supported; + #endif /* __XEN_DOMAIN_H__ */