| Message ID | 1672321493.8765712.1592320839082.JavaMail.zimbra@cert.pl (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Implement support for external IPT monitoring | expand |
On Tue, Jun 16, 2020 at 05:20:39PM +0200, Michał Leszczyński wrote: > Check if Intel Processor Trace feature is supported by current > processor. Define hvm_ipt_supported function. > > Signed-off-by: Michal Leszczynski <michal.leszczynski@cert.pl> > --- > xen/arch/x86/hvm/vmx/vmx.c | 24 +++++++++++++++++++++ > xen/include/asm-x86/cpufeature.h | 1 + > xen/include/asm-x86/hvm/hvm.h | 9 ++++++++ > xen/include/asm-x86/hvm/vmx/vmcs.h | 1 + > xen/include/public/arch-x86/cpufeatureset.h | 1 + > 5 files changed, 36 insertions(+) > > diff --git a/xen/arch/x86/hvm/vmx/vmx.c b/xen/arch/x86/hvm/vmx/vmx.c > index ab19d9424e..a91bbdb798 100644 > --- a/xen/arch/x86/hvm/vmx/vmx.c > +++ b/xen/arch/x86/hvm/vmx/vmx.c > @@ -2484,6 +2484,7 @@ static bool __init has_if_pschange_mc(void) > > const struct hvm_function_table * __init start_vmx(void) > { > + u64 _vmx_misc_cap; Please use uint64_t, and you can drop the leading _vmx prefix, this is already vmx specific. Also add a newline between variable definition and code. > set_in_cr4(X86_CR4_VMXE); > > if ( vmx_vmcs_init() ) > @@ -2557,6 +2558,29 @@ const struct hvm_function_table * __init start_vmx(void) > vmx_function_table.get_guest_bndcfgs = vmx_get_guest_bndcfgs; > } > > + /* Check whether IPT is supported in VMX operation */ > + vmx_function_table.ipt_supported = 1; > + > + if ( !cpu_has_ipt ) > + { > + vmx_function_table.ipt_supported = 0; > + printk("VMX: Missing support for Intel Processor Trace x86 feature.\n"); > + } > + > + rdmsrl(MSR_IA32_VMX_MISC, _vmx_misc_cap); > + > + if ( !( _vmx_misc_cap & VMX_MISC_PT_SUPPORTED ) ) > + { > + vmx_function_table.ipt_supported = 0; > + printk("VMX: Missing support for Intel Processor Trace in VMX operation, VMX_MISC caps: %llx\n", > + (unsigned long long)_vmx_misc_cap); > + } > + > + if (vmx_function_table.ipt_supported) > + { > + printk("VMX: Intel Processor Trace is SUPPORTED"); > + } I think you could simplify this as: vmx_function_table.ipt_supported = cpu_has_ipt && (misc_cap & VMX_MISC_PT_SUPPORTED); Also the code is too chatty IMO. Looking at how other VMX features are detected, I think you should move the checks to vmx_init_vmcs_config and set the relevant bits in the VM control registers that you can then evaluate in vmx_display_features in order to print if the feature is supported? > + > lbr_tsx_fixup_check(); > ler_to_fixup_check(); > > diff --git a/xen/include/asm-x86/cpufeature.h b/xen/include/asm-x86/cpufeature.h > index f790d5c1f8..8d7955dd87 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_IPT) > #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/hvm.h b/xen/include/asm-x86/hvm/hvm.h > index 1eb377dd82..48465b6067 100644 > --- a/xen/include/asm-x86/hvm/hvm.h > +++ b/xen/include/asm-x86/hvm/hvm.h > @@ -96,6 +96,9 @@ struct hvm_function_table { > /* Necessary hardware support for alternate p2m's? */ > bool altp2m_supported; > > + /* Hardware support for IPT? */ > + bool ipt_supported; We might want to name this pt_supported, since it's possible for other vendors to also introduce a processor tracing feature in the future? Thanks, Roger.
On 16.06.2020 18:30, Roger Pau Monné wrote: > On Tue, Jun 16, 2020 at 05:20:39PM +0200, Michał Leszczyński wrote: >> Check if Intel Processor Trace feature is supported by current >> processor. Define hvm_ipt_supported function. >> >> Signed-off-by: Michal Leszczynski <michal.leszczynski@cert.pl> >> --- >> xen/arch/x86/hvm/vmx/vmx.c | 24 +++++++++++++++++++++ >> xen/include/asm-x86/cpufeature.h | 1 + >> xen/include/asm-x86/hvm/hvm.h | 9 ++++++++ >> xen/include/asm-x86/hvm/vmx/vmcs.h | 1 + >> xen/include/public/arch-x86/cpufeatureset.h | 1 + >> 5 files changed, 36 insertions(+) >> >> diff --git a/xen/arch/x86/hvm/vmx/vmx.c b/xen/arch/x86/hvm/vmx/vmx.c >> index ab19d9424e..a91bbdb798 100644 >> --- a/xen/arch/x86/hvm/vmx/vmx.c >> +++ b/xen/arch/x86/hvm/vmx/vmx.c >> @@ -2484,6 +2484,7 @@ static bool __init has_if_pschange_mc(void) >> >> const struct hvm_function_table * __init start_vmx(void) >> { >> + u64 _vmx_misc_cap; > > Please use uint64_t, and you can drop the leading _vmx prefix, this is > already vmx specific. Actually, all of _vmx_ should be dropped (i.e. in particular local variables shouldn't start with an underscore). Jan
diff --git a/xen/arch/x86/hvm/vmx/vmx.c b/xen/arch/x86/hvm/vmx/vmx.c index ab19d9424e..a91bbdb798 100644 --- a/xen/arch/x86/hvm/vmx/vmx.c +++ b/xen/arch/x86/hvm/vmx/vmx.c @@ -2484,6 +2484,7 @@ static bool __init has_if_pschange_mc(void) const struct hvm_function_table * __init start_vmx(void) { + u64 _vmx_misc_cap; set_in_cr4(X86_CR4_VMXE); if ( vmx_vmcs_init() ) @@ -2557,6 +2558,29 @@ const struct hvm_function_table * __init start_vmx(void) vmx_function_table.get_guest_bndcfgs = vmx_get_guest_bndcfgs; } + /* Check whether IPT is supported in VMX operation */ + vmx_function_table.ipt_supported = 1; + + if ( !cpu_has_ipt ) + { + vmx_function_table.ipt_supported = 0; + printk("VMX: Missing support for Intel Processor Trace x86 feature.\n"); + } + + rdmsrl(MSR_IA32_VMX_MISC, _vmx_misc_cap); + + if ( !( _vmx_misc_cap & VMX_MISC_PT_SUPPORTED ) ) + { + vmx_function_table.ipt_supported = 0; + printk("VMX: Missing support for Intel Processor Trace in VMX operation, VMX_MISC caps: %llx\n", + (unsigned long long)_vmx_misc_cap); + } + + if (vmx_function_table.ipt_supported) + { + printk("VMX: Intel Processor Trace is SUPPORTED"); + } + lbr_tsx_fixup_check(); ler_to_fixup_check(); diff --git a/xen/include/asm-x86/cpufeature.h b/xen/include/asm-x86/cpufeature.h index f790d5c1f8..8d7955dd87 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_IPT) #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/hvm.h b/xen/include/asm-x86/hvm/hvm.h index 1eb377dd82..48465b6067 100644 --- a/xen/include/asm-x86/hvm/hvm.h +++ b/xen/include/asm-x86/hvm/hvm.h @@ -96,6 +96,9 @@ struct hvm_function_table { /* Necessary hardware support for alternate p2m's? */ bool altp2m_supported; + /* Hardware support for IPT? */ + bool ipt_supported; + /* Hardware virtual interrupt delivery enable? */ bool virtual_intr_delivery_enabled; @@ -630,6 +633,12 @@ static inline bool hvm_altp2m_supported(void) return hvm_funcs.altp2m_supported; } +/* returns true if hardware supports Intel Processor Trace */ +static inline bool hvm_ipt_supported(void) +{ + return hvm_funcs.ipt_supported; +} + /* updates the current hardware p2m */ static inline void altp2m_vcpu_update_p2m(struct vcpu *v) { diff --git a/xen/include/asm-x86/hvm/vmx/vmcs.h b/xen/include/asm-x86/hvm/vmx/vmcs.h index 906810592f..4c81093aba 100644 --- a/xen/include/asm-x86/hvm/vmx/vmcs.h +++ b/xen/include/asm-x86/hvm/vmx/vmcs.h @@ -285,6 +285,7 @@ extern u64 vmx_ept_vpid_cap; #define VMX_MISC_CR3_TARGET 0x01ff0000 #define VMX_MISC_VMWRITE_ALL 0x20000000 +#define VMX_MISC_PT_SUPPORTED 0x00004000 #define VMX_TSC_MULTIPLIER_MAX 0xffffffffffffffffULL diff --git a/xen/include/public/arch-x86/cpufeatureset.h b/xen/include/public/arch-x86/cpufeatureset.h index 5ca35d9d97..7cfcac451d 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(IPT, 5*32+25) /*H Intel Processor Trace */ 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 */
Check if Intel Processor Trace feature is supported by current processor. Define hvm_ipt_supported function. Signed-off-by: Michal Leszczynski <michal.leszczynski@cert.pl> --- xen/arch/x86/hvm/vmx/vmx.c | 24 +++++++++++++++++++++ xen/include/asm-x86/cpufeature.h | 1 + xen/include/asm-x86/hvm/hvm.h | 9 ++++++++ xen/include/asm-x86/hvm/vmx/vmcs.h | 1 + xen/include/public/arch-x86/cpufeatureset.h | 1 + 5 files changed, 36 insertions(+)