On 25/8/2021 12:19 pm, Xiaoyao Li wrote:
On 8/25/2021 11:30 AM, Like Xu wrote:
+Alexander
On 24/8/2021 7:07 pm, Xiaoyao Li wrote:
Per Intel SDM, RTIT_CTL_BRANCH_EN bit has no dependency on any CPUID
leaf 0x14.
Signed-off-by: Xiaoyao Li <xiaoyao.li@xxxxxxxxx>
---
arch/x86/kvm/vmx/vmx.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c
index 7ed96c460661..4a70a6d2f442 100644
--- a/arch/x86/kvm/vmx/vmx.c
+++ b/arch/x86/kvm/vmx/vmx.c
@@ -7116,7 +7116,8 @@ static void update_intel_pt_cfg(struct kvm_vcpu *vcpu)
/* Initialize and clear the no dependency bits */
vmx->pt_desc.ctl_bitmask = ~(RTIT_CTL_TRACEEN | RTIT_CTL_OS |
- RTIT_CTL_USR | RTIT_CTL_TSC_EN | RTIT_CTL_DISRETC);
+ RTIT_CTL_USR | RTIT_CTL_TSC_EN | RTIT_CTL_DISRETC |
+ RTIT_CTL_BRANCH_EN);
/*
* If CPUID.(EAX=14H,ECX=0):EBX[0]=1 CR3Filter can be set otherwise
@@ -7134,12 +7135,11 @@ static void update_intel_pt_cfg(struct kvm_vcpu *vcpu)
RTIT_CTL_CYC_THRESH | RTIT_CTL_PSB_FREQ);
/*
- * If CPUID.(EAX=14H,ECX=0):EBX[3]=1 MTCEn BranchEn and
- * MTCFreq can be set
+ * If CPUID.(EAX=14H,ECX=0):EBX[3]=1 MTCEn and MTCFreq can be set
If CPUID.(EAX=14H,ECX=0):EBX[3]=1,
"indicates support of MTC timing packet and suppression of COFI-based
packets."
I think it's a mistake of SDM in CPUID instruction.
If you read 31.3.1, table 31-11 of SDM 325462-075US,
It just says CPUID(0x14, 0):EBX[3]: MTC supprted.
It doesn't talk anything about COFI packets suppression.
Further as below.
Per 31.2.5.4 Branch Enable (BranchEn),
"If BranchEn is not set, then relevant COFI packets (TNT, TIP*, FUP,
MODE.*) are suppressed."
I think if the COFI capability is suppressed, the software can't set the
BranchEn bit, right ?
Based on your understanding, isn't it that
1. if CPUID.(EAX=14H,ECX=0):EBX[3]=0, it doesn't support "suppression of
COFI-based packets".
2. if it doesn't support "suppression of COFI-based packets", then it doens't
support "If BranchEn is not set, then relevant COFI packets (TNT, TIP*, FUP,
MODE.*) are suppressed", i.e. BranchEn must be 1.
That's it.
Anyway, I think it's just a mistake on CPUID instruction document of SDM.
Is this an ambiguity rather than a mistake ?
CPUD.(EAX=14H,ECX=0):EBX[3] should only indicates the MTC support.
Please do not make assertions that you do not confirm with hw.
BranchEn should be always supported if PT is available. Per "31.2.7.2
Check d35869ba348d3f1ff3e6d8214fe0f674bb0e404e.
IA32_RTIT_CTL MSR" on SDM:
When BranchEn is 1, it enables COFI-based packets.
When BranchEn is 0, it disables COFI-based packtes. i.e., COFI packets are
suppressed.
*/
if (intel_pt_validate_cap(vmx->pt_desc.caps, PT_CAP_mtc))
vmx->pt_desc.ctl_bitmask &= ~(RTIT_CTL_MTC_EN |
- RTIT_CTL_BRANCH_EN | RTIT_CTL_MTC_RANGE);
+ RTIT_CTL_MTC_RANGE);
/* If CPUID.(EAX=14H,ECX=0):EBX[4]=1 FUPonPTW and PTWEn can be set */
if (intel_pt_validate_cap(vmx->pt_desc.caps, PT_CAP_ptwrite))
