linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/5] KVM: VMX: PT (processor trace) optimizations and fixes
@ 2021-08-24 11:07 Xiaoyao Li
  2021-08-24 11:07 ` [PATCH 1/5] KVM: VMX: Restore host's MSR_IA32_RTIT_CTL when it's not zero Xiaoyao Li
                   ` (4 more replies)
  0 siblings, 5 replies; 19+ messages in thread
From: Xiaoyao Li @ 2021-08-24 11:07 UTC (permalink / raw)
  To: Paolo Bonzini
  Cc: Xiaoyao Li, Sean Christopherson, Vitaly Kuznetsov, Wanpeng Li,
	Jim Mattson, Joerg Roedel, kvm, linux-kernel

Patch 1 and 2 are optimizations and cleanup.

Patch 3 - 5 are fixes for PT handling.

Xiaoyao Li (5):
  KVM: VMX: Restore host's MSR_IA32_RTIT_CTL when it's not zero
  KVM: VMX: Use cached vmx->pt_desc.addr_range
  KVM: VMX: RTIT_CTL_BRANCH_EN has no dependency on other CPUID bit
  KVM: VMX: Disallow PT MSRs accessing if PT is not exposed to guest
  KVM: VMX: Check Intel PT related CPUID leaves

 arch/x86/kvm/cpuid.c   | 25 +++++++++++++++++++++++++
 arch/x86/kvm/vmx/vmx.c | 34 +++++++++++++++++++++-------------
 2 files changed, 46 insertions(+), 13 deletions(-)

-- 
2.27.0


^ permalink raw reply	[flat|nested] 19+ messages in thread

* [PATCH 1/5] KVM: VMX: Restore host's MSR_IA32_RTIT_CTL when it's not zero
  2021-08-24 11:07 [PATCH 0/5] KVM: VMX: PT (processor trace) optimizations and fixes Xiaoyao Li
@ 2021-08-24 11:07 ` Xiaoyao Li
  2021-08-24 17:54   ` Sean Christopherson
  2021-08-24 11:07 ` [PATCH 2/5] KVM: VMX: Use cached vmx->pt_desc.addr_range Xiaoyao Li
                   ` (3 subsequent siblings)
  4 siblings, 1 reply; 19+ messages in thread
From: Xiaoyao Li @ 2021-08-24 11:07 UTC (permalink / raw)
  To: Paolo Bonzini
  Cc: Xiaoyao Li, Sean Christopherson, Vitaly Kuznetsov, Wanpeng Li,
	Jim Mattson, Joerg Roedel, kvm, linux-kernel

A minor optimation to WRMSR MSR_IA32_RTIT_CTL when necessary.

Signed-off-by: Xiaoyao Li <xiaoyao.li@intel.com>
---
 arch/x86/kvm/vmx/vmx.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c
index fada1055f325..e0a9460e4dab 100644
--- a/arch/x86/kvm/vmx/vmx.c
+++ b/arch/x86/kvm/vmx/vmx.c
@@ -1075,7 +1075,8 @@ static void pt_guest_exit(struct vcpu_vmx *vmx)
 	}
 
 	/* Reload host state (IA32_RTIT_CTL will be cleared on VM exit). */
-	wrmsrl(MSR_IA32_RTIT_CTL, vmx->pt_desc.host.ctl);
+	if (vmx->pt_desc.host.ctl)
+		wrmsrl(MSR_IA32_RTIT_CTL, vmx->pt_desc.host.ctl);
 }
 
 void vmx_set_host_fs_gs(struct vmcs_host_state *host, u16 fs_sel, u16 gs_sel,
-- 
2.27.0


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH 2/5] KVM: VMX: Use cached vmx->pt_desc.addr_range
  2021-08-24 11:07 [PATCH 0/5] KVM: VMX: PT (processor trace) optimizations and fixes Xiaoyao Li
  2021-08-24 11:07 ` [PATCH 1/5] KVM: VMX: Restore host's MSR_IA32_RTIT_CTL when it's not zero Xiaoyao Li
@ 2021-08-24 11:07 ` Xiaoyao Li
  2021-08-24 15:24   ` Sean Christopherson
  2021-08-24 11:07 ` [PATCH 3/5] KVM: VMX: RTIT_CTL_BRANCH_EN has no dependency on other CPUID bit Xiaoyao Li
                   ` (2 subsequent siblings)
  4 siblings, 1 reply; 19+ messages in thread
From: Xiaoyao Li @ 2021-08-24 11:07 UTC (permalink / raw)
  To: Paolo Bonzini
  Cc: Xiaoyao Li, Sean Christopherson, Vitaly Kuznetsov, Wanpeng Li,
	Jim Mattson, Joerg Roedel, kvm, linux-kernel

The number of guest's valid PT ADDR MSRs is cached in
vmx->pt_desc.addr_range. Use it instead of calculating it again.

Signed-off-by: Xiaoyao Li <xiaoyao.li@intel.com>
---
 arch/x86/kvm/vmx/vmx.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c
index e0a9460e4dab..7ed96c460661 100644
--- a/arch/x86/kvm/vmx/vmx.c
+++ b/arch/x86/kvm/vmx/vmx.c
@@ -2202,8 +2202,7 @@ static int vmx_set_msr(struct kvm_vcpu *vcpu, struct msr_data *msr_info)
 		if (!pt_can_write_msr(vmx))
 			return 1;
 		index = msr_info->index - MSR_IA32_RTIT_ADDR0_A;
-		if (index >= 2 * intel_pt_validate_cap(vmx->pt_desc.caps,
-						       PT_CAP_num_address_ranges))
+		if (index >= 2 * vmx->pt_desc.addr_range)
 			return 1;
 		if (is_noncanonical_address(data, vcpu))
 			return 1;
-- 
2.27.0


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH 3/5] KVM: VMX: RTIT_CTL_BRANCH_EN has no dependency on other CPUID bit
  2021-08-24 11:07 [PATCH 0/5] KVM: VMX: PT (processor trace) optimizations and fixes Xiaoyao Li
  2021-08-24 11:07 ` [PATCH 1/5] KVM: VMX: Restore host's MSR_IA32_RTIT_CTL when it's not zero Xiaoyao Li
  2021-08-24 11:07 ` [PATCH 2/5] KVM: VMX: Use cached vmx->pt_desc.addr_range Xiaoyao Li
@ 2021-08-24 11:07 ` Xiaoyao Li
  2021-08-25  3:30   ` Like Xu
  2021-08-24 11:07 ` [PATCH 4/5] KVM: VMX: Disallow PT MSRs accessing if PT is not exposed to guest Xiaoyao Li
  2021-08-24 11:07 ` [PATCH 5/5] KVM: VMX: Check Intel PT related CPUID leaves Xiaoyao Li
  4 siblings, 1 reply; 19+ messages in thread
From: Xiaoyao Li @ 2021-08-24 11:07 UTC (permalink / raw)
  To: Paolo Bonzini
  Cc: Xiaoyao Li, Sean Christopherson, Vitaly Kuznetsov, Wanpeng Li,
	Jim Mattson, Joerg Roedel, kvm, linux-kernel

Per Intel SDM, RTIT_CTL_BRANCH_EN bit has no dependency on any CPUID
leaf 0x14.

Signed-off-by: Xiaoyao Li <xiaoyao.li@intel.com>
---
 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 (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))
-- 
2.27.0


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH 4/5] KVM: VMX: Disallow PT MSRs accessing if PT is not exposed to guest
  2021-08-24 11:07 [PATCH 0/5] KVM: VMX: PT (processor trace) optimizations and fixes Xiaoyao Li
                   ` (2 preceding siblings ...)
  2021-08-24 11:07 ` [PATCH 3/5] KVM: VMX: RTIT_CTL_BRANCH_EN has no dependency on other CPUID bit Xiaoyao Li
@ 2021-08-24 11:07 ` Xiaoyao Li
  2021-08-24 14:20   ` Sean Christopherson
  2021-08-24 11:07 ` [PATCH 5/5] KVM: VMX: Check Intel PT related CPUID leaves Xiaoyao Li
  4 siblings, 1 reply; 19+ messages in thread
From: Xiaoyao Li @ 2021-08-24 11:07 UTC (permalink / raw)
  To: Paolo Bonzini
  Cc: Xiaoyao Li, Sean Christopherson, Vitaly Kuznetsov, Wanpeng Li,
	Jim Mattson, Joerg Roedel, kvm, linux-kernel

Per SDM, it triggers #GP for all the accessing of PT MSRs, if
X86_FEATURE_INTEL_PT is not available.

Signed-off-by: Xiaoyao Li <xiaoyao.li@intel.com>
---
 arch/x86/kvm/vmx/vmx.c | 20 ++++++++++++++------
 1 file changed, 14 insertions(+), 6 deletions(-)

diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c
index 4a70a6d2f442..1bbc4d84c623 100644
--- a/arch/x86/kvm/vmx/vmx.c
+++ b/arch/x86/kvm/vmx/vmx.c
@@ -1010,9 +1010,16 @@ static unsigned long segment_base(u16 selector)
 static inline bool pt_can_write_msr(struct vcpu_vmx *vmx)
 {
 	return vmx_pt_mode_is_host_guest() &&
+	       guest_cpuid_has(&vmx->vcpu, X86_FEATURE_INTEL_PT) &&
 	       !(vmx->pt_desc.guest.ctl & RTIT_CTL_TRACEEN);
 }
 
+static inline bool pt_can_read_msr(struct kvm_vcpu *vcpu)
+{
+	return vmx_pt_mode_is_host_guest() &&
+	       guest_cpuid_has(vcpu, X86_FEATURE_INTEL_PT);
+}
+
 static inline bool pt_output_base_valid(struct kvm_vcpu *vcpu, u64 base)
 {
 	/* The base must be 128-byte aligned and a legal physical address. */
@@ -1849,24 +1856,24 @@ static int vmx_get_msr(struct kvm_vcpu *vcpu, struct msr_data *msr_info)
 							&msr_info->data);
 		break;
 	case MSR_IA32_RTIT_CTL:
-		if (!vmx_pt_mode_is_host_guest())
+		if (!pt_can_read_msr(vcpu))
 			return 1;
 		msr_info->data = vmx->pt_desc.guest.ctl;
 		break;
 	case MSR_IA32_RTIT_STATUS:
-		if (!vmx_pt_mode_is_host_guest())
+		if (!pt_can_read_msr(vcpu))
 			return 1;
 		msr_info->data = vmx->pt_desc.guest.status;
 		break;
 	case MSR_IA32_RTIT_CR3_MATCH:
-		if (!vmx_pt_mode_is_host_guest() ||
+		if (!pt_can_read_msr(vcpu) ||
 			!intel_pt_validate_cap(vmx->pt_desc.caps,
 						PT_CAP_cr3_filtering))
 			return 1;
 		msr_info->data = vmx->pt_desc.guest.cr3_match;
 		break;
 	case MSR_IA32_RTIT_OUTPUT_BASE:
-		if (!vmx_pt_mode_is_host_guest() ||
+		if (!pt_can_read_msr(vcpu) ||
 			(!intel_pt_validate_cap(vmx->pt_desc.caps,
 					PT_CAP_topa_output) &&
 			 !intel_pt_validate_cap(vmx->pt_desc.caps,
@@ -1875,7 +1882,7 @@ static int vmx_get_msr(struct kvm_vcpu *vcpu, struct msr_data *msr_info)
 		msr_info->data = vmx->pt_desc.guest.output_base;
 		break;
 	case MSR_IA32_RTIT_OUTPUT_MASK:
-		if (!vmx_pt_mode_is_host_guest() ||
+		if (!pt_can_read_msr(vcpu) ||
 			(!intel_pt_validate_cap(vmx->pt_desc.caps,
 					PT_CAP_topa_output) &&
 			 !intel_pt_validate_cap(vmx->pt_desc.caps,
@@ -1885,7 +1892,7 @@ static int vmx_get_msr(struct kvm_vcpu *vcpu, struct msr_data *msr_info)
 		break;
 	case MSR_IA32_RTIT_ADDR0_A ... MSR_IA32_RTIT_ADDR3_B:
 		index = msr_info->index - MSR_IA32_RTIT_ADDR0_A;
-		if (!vmx_pt_mode_is_host_guest() ||
+		if (!pt_can_read_msr(vcpu) ||
 			(index >= 2 * intel_pt_validate_cap(vmx->pt_desc.caps,
 					PT_CAP_num_address_ranges)))
 			return 1;
@@ -2154,6 +2161,7 @@ static int vmx_set_msr(struct kvm_vcpu *vcpu, struct msr_data *msr_info)
 		return vmx_set_vmx_msr(vcpu, msr_index, data);
 	case MSR_IA32_RTIT_CTL:
 		if (!vmx_pt_mode_is_host_guest() ||
+		    !guest_cpuid_has(vcpu, X86_FEATURE_INTEL_PT) ||
 			vmx_rtit_ctl_check(vcpu, data) ||
 			vmx->nested.vmxon)
 			return 1;
-- 
2.27.0


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH 5/5] KVM: VMX: Check Intel PT related CPUID leaves
  2021-08-24 11:07 [PATCH 0/5] KVM: VMX: PT (processor trace) optimizations and fixes Xiaoyao Li
                   ` (3 preceding siblings ...)
  2021-08-24 11:07 ` [PATCH 4/5] KVM: VMX: Disallow PT MSRs accessing if PT is not exposed to guest Xiaoyao Li
@ 2021-08-24 11:07 ` Xiaoyao Li
  4 siblings, 0 replies; 19+ messages in thread
From: Xiaoyao Li @ 2021-08-24 11:07 UTC (permalink / raw)
  To: Paolo Bonzini
  Cc: Xiaoyao Li, Sean Christopherson, Vitaly Kuznetsov, Wanpeng Li,
	Jim Mattson, Joerg Roedel, kvm, linux-kernel

CPUID 0XD leaves reports the capabilities of Intel PT and decides which
bits are valid to be set in MSR_IA32_RTIT_CTL.

KVM needs to check the guest CPUID values set by userspace doesn't
enable any bit which is not supported by bare metal. Otherwise, it
allows guest to set corresponding bit in MSR_IA32_RTIT_CTL and it will
trigger vm-entry failure if unsupported bit is exposed to guest and
set by guest.

Signed-off-by: Xiaoyao Li <xiaoyao.li@intel.com>
---
There is bit 31 of CPUID(0xD, 0).ECX that doesn't restrict any bit in
MSR_IA32_RTIT_CTL. If guest has different value than host, it won't
cause any vm-entry failure, but guest will parse the PT packet with
wrong format.

I also check it to be same as host to ensure the virtualization correctness.
---
 arch/x86/kvm/cpuid.c | 25 +++++++++++++++++++++++++
 1 file changed, 25 insertions(+)

diff --git a/arch/x86/kvm/cpuid.c b/arch/x86/kvm/cpuid.c
index 739be5da3bca..0c8e06a24156 100644
--- a/arch/x86/kvm/cpuid.c
+++ b/arch/x86/kvm/cpuid.c
@@ -76,6 +76,7 @@ static inline struct kvm_cpuid_entry2 *cpuid_entry2_find(
 static int kvm_check_cpuid(struct kvm_cpuid_entry2 *entries, int nent)
 {
 	struct kvm_cpuid_entry2 *best;
+	u32 eax, ebx, ecx, edx;
 
 	/*
 	 * The existing code assumes virtual address is 48-bit or 57-bit in the
@@ -89,6 +90,30 @@ static int kvm_check_cpuid(struct kvm_cpuid_entry2 *entries, int nent)
 			return -EINVAL;
 	}
 
+	/*
+	 * CPUID 0xD leaves tell Intel PT capabilities, which decides
+	 * pt_desc.ctl_bitmask in later update_intel_pt_cfg().
+	 *
+	 * pt_desc.ctl_bitmask decides the legal value for guest
+	 * MSR_IA32_RTIT_CTL. KVM cannot support PT capabilities beyond native,
+	 * otherwise it will trigger vm-entry failure if guest sets native
+	 * unsupported bits in MSR_IA32_RTIT_CTL.
+	 */
+	best = cpuid_entry2_find(entries, nent, 0xD, 0);
+	if (best) {
+		cpuid_count(0xD, 0, &eax, &ebx, &ecx, &edx);
+		if (best->ebx & ~ebx || best->ecx & ~ecx)
+			return -EINVAL;
+	}
+	best = cpuid_entry2_find(entries, nent, 0xD, 1);
+	if (best) {
+		cpuid_count(0xD, 0, &eax, &ebx, &ecx, &edx);
+		if (((best->eax & 0x7) > (eax & 0x7)) ||
+		    ((best->eax & ~eax) >> 16) ||
+		    (best->ebx & ~ebx))
+			return -EINVAL;
+	}
+
 	return 0;
 }
 
-- 
2.27.0


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* Re: [PATCH 4/5] KVM: VMX: Disallow PT MSRs accessing if PT is not exposed to guest
  2021-08-24 11:07 ` [PATCH 4/5] KVM: VMX: Disallow PT MSRs accessing if PT is not exposed to guest Xiaoyao Li
@ 2021-08-24 14:20   ` Sean Christopherson
  2021-08-24 15:35     ` Xiaoyao Li
  0 siblings, 1 reply; 19+ messages in thread
From: Sean Christopherson @ 2021-08-24 14:20 UTC (permalink / raw)
  To: Xiaoyao Li
  Cc: Paolo Bonzini, Vitaly Kuznetsov, Wanpeng Li, Jim Mattson,
	Joerg Roedel, kvm, linux-kernel

On Tue, Aug 24, 2021, Xiaoyao Li wrote:
> Per SDM, it triggers #GP for all the accessing of PT MSRs, if
> X86_FEATURE_INTEL_PT is not available.
> 
> Signed-off-by: Xiaoyao Li <xiaoyao.li@intel.com>
> ---
>  arch/x86/kvm/vmx/vmx.c | 20 ++++++++++++++------
>  1 file changed, 14 insertions(+), 6 deletions(-)
> 
> diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c
> index 4a70a6d2f442..1bbc4d84c623 100644
> --- a/arch/x86/kvm/vmx/vmx.c
> +++ b/arch/x86/kvm/vmx/vmx.c
> @@ -1010,9 +1010,16 @@ static unsigned long segment_base(u16 selector)
>  static inline bool pt_can_write_msr(struct vcpu_vmx *vmx)
>  {
>  	return vmx_pt_mode_is_host_guest() &&
> +	       guest_cpuid_has(&vmx->vcpu, X86_FEATURE_INTEL_PT) &&
>  	       !(vmx->pt_desc.guest.ctl & RTIT_CTL_TRACEEN);
>  }
>  
> +static inline bool pt_can_read_msr(struct kvm_vcpu *vcpu)
> +{
> +	return vmx_pt_mode_is_host_guest() &&
> +	       guest_cpuid_has(vcpu, X86_FEATURE_INTEL_PT);
> +}
> +
>  static inline bool pt_output_base_valid(struct kvm_vcpu *vcpu, u64 base)
>  {
>  	/* The base must be 128-byte aligned and a legal physical address. */
> @@ -1849,24 +1856,24 @@ static int vmx_get_msr(struct kvm_vcpu *vcpu, struct msr_data *msr_info)
>  							&msr_info->data);
>  		break;
>  	case MSR_IA32_RTIT_CTL:
> -		if (!vmx_pt_mode_is_host_guest())
> +		if (!pt_can_read_msr(vcpu))

These all need to provide exemptions for accesses from the host.  KVM allows
access to MSRs that are not exposed to the guest so long as all the other checks
pass.  Same for the next patch.

Easiest thing is probably to pass in @msr_info to the helpers and do the check
there.

>  			return 1;
>  		msr_info->data = vmx->pt_desc.guest.ctl;
>  		break;

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 2/5] KVM: VMX: Use cached vmx->pt_desc.addr_range
  2021-08-24 11:07 ` [PATCH 2/5] KVM: VMX: Use cached vmx->pt_desc.addr_range Xiaoyao Li
@ 2021-08-24 15:24   ` Sean Christopherson
  2021-08-24 15:42     ` Xiaoyao Li
  0 siblings, 1 reply; 19+ messages in thread
From: Sean Christopherson @ 2021-08-24 15:24 UTC (permalink / raw)
  To: Xiaoyao Li
  Cc: Paolo Bonzini, Vitaly Kuznetsov, Wanpeng Li, Jim Mattson,
	Joerg Roedel, kvm, linux-kernel

On Tue, Aug 24, 2021, Xiaoyao Li wrote:
> The number of guest's valid PT ADDR MSRs is cached in

Can you do s/cached/precomputed in the shortlog and changelog?  Explanation below.

> vmx->pt_desc.addr_range. Use it instead of calculating it again.
> 
> Signed-off-by: Xiaoyao Li <xiaoyao.li@intel.com>
> ---
>  arch/x86/kvm/vmx/vmx.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c
> index e0a9460e4dab..7ed96c460661 100644
> --- a/arch/x86/kvm/vmx/vmx.c
> +++ b/arch/x86/kvm/vmx/vmx.c
> @@ -2202,8 +2202,7 @@ static int vmx_set_msr(struct kvm_vcpu *vcpu, struct msr_data *msr_info)
>  		if (!pt_can_write_msr(vmx))
>  			return 1;
>  		index = msr_info->index - MSR_IA32_RTIT_ADDR0_A;
> -		if (index >= 2 * intel_pt_validate_cap(vmx->pt_desc.caps,
> -						       PT_CAP_num_address_ranges))
> +		if (index >= 2 * vmx->pt_desc.addr_range)

Ugh, "validate" is a lie, a better name would be intel_pt_get_cap() or so.  There
is no validation, the helper is simply extracting the requested cap from the
passed in array of capabilities.

That matters in this case because the number of address ranges exposed to the
guest is not bounded by the number of address ranges present in hardware, i.e.
it's not "validated".  And that matters because KVM uses vmx->pt_desc.addr_range
to pass through the ADDRn_{A,B} MSRs when tracing enabled.  In other words,
userspace can expose MSRs to the guest that do not exist.

The bug shouldn't be a security issue, so long as Intel CPUs are bug free and
aren't doing silly things with MSR indexes.  The number of possible address ranges
is encoded in three bits, thus the theoretical max is 8 ranges.  So userspace can't
get access to arbitrary MSRs, just ADDR0_A -> ADDR7_B.

And since KVM would be modifying the "validated" value, it's more than just a
cache, hence the request to use "precomputed".

Finally, vmx_get_msr() should use the precomputed value as well.

P.S. If you want to introduce a bit of churn, s/addr_range/nr_addr_ranges would
     be a welcome change as well.

>  			return 1;
>  		if (is_noncanonical_address(data, vcpu))
>  			return 1;
> -- 
> 2.27.0
> 

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 4/5] KVM: VMX: Disallow PT MSRs accessing if PT is not exposed to guest
  2021-08-24 14:20   ` Sean Christopherson
@ 2021-08-24 15:35     ` Xiaoyao Li
  2021-08-24 16:48       ` Sean Christopherson
  0 siblings, 1 reply; 19+ messages in thread
From: Xiaoyao Li @ 2021-08-24 15:35 UTC (permalink / raw)
  To: Sean Christopherson
  Cc: Paolo Bonzini, Vitaly Kuznetsov, Wanpeng Li, Jim Mattson,
	Joerg Roedel, kvm, linux-kernel

On 8/24/2021 10:20 PM, Sean Christopherson wrote:
> On Tue, Aug 24, 2021, Xiaoyao Li wrote:
>> Per SDM, it triggers #GP for all the accessing of PT MSRs, if
>> X86_FEATURE_INTEL_PT is not available.
>>
>> Signed-off-by: Xiaoyao Li <xiaoyao.li@intel.com>
>> ---
>>   arch/x86/kvm/vmx/vmx.c | 20 ++++++++++++++------
>>   1 file changed, 14 insertions(+), 6 deletions(-)
>>
>> diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c
>> index 4a70a6d2f442..1bbc4d84c623 100644
>> --- a/arch/x86/kvm/vmx/vmx.c
>> +++ b/arch/x86/kvm/vmx/vmx.c
>> @@ -1010,9 +1010,16 @@ static unsigned long segment_base(u16 selector)
>>   static inline bool pt_can_write_msr(struct vcpu_vmx *vmx)
>>   {
>>   	return vmx_pt_mode_is_host_guest() &&
>> +	       guest_cpuid_has(&vmx->vcpu, X86_FEATURE_INTEL_PT) &&
>>   	       !(vmx->pt_desc.guest.ctl & RTIT_CTL_TRACEEN);
>>   }
>>   
>> +static inline bool pt_can_read_msr(struct kvm_vcpu *vcpu)
>> +{
>> +	return vmx_pt_mode_is_host_guest() &&
>> +	       guest_cpuid_has(vcpu, X86_FEATURE_INTEL_PT);
>> +}
>> +
>>   static inline bool pt_output_base_valid(struct kvm_vcpu *vcpu, u64 base)
>>   {
>>   	/* The base must be 128-byte aligned and a legal physical address. */
>> @@ -1849,24 +1856,24 @@ static int vmx_get_msr(struct kvm_vcpu *vcpu, struct msr_data *msr_info)
>>   							&msr_info->data);
>>   		break;
>>   	case MSR_IA32_RTIT_CTL:
>> -		if (!vmx_pt_mode_is_host_guest())
>> +		if (!pt_can_read_msr(vcpu))
> 
> These all need to provide exemptions for accesses from the host.  KVM allows
> access to MSRs that are not exposed to the guest so long as all the other checks
> pass. 

Not all the MSRs are allowed to be accessed from host regardless of 
whether it's exposed to guest. e.g., MSR_IA32_TSC_ADJUST, it checks 
guest CPUID first.

For me, for those PT MSRs, I cannot think of any reason that 
host/userspace would access them without PT being exposed to guest.

On the other hand, since this patch indeed breaks the existing userspace 
VMM who accesses those MSRs without checking guest CPUID.

So I will follow your advice to allow the host_initiated case in next 
version.

> Same for the next patch.

Sorry, I don't know how it matters next patch.

> Easiest thing is probably to pass in @msr_info to the helpers and do the check
> there.
> 
>>   			return 1;
>>   		msr_info->data = vmx->pt_desc.guest.ctl;
>>   		break;


^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 2/5] KVM: VMX: Use cached vmx->pt_desc.addr_range
  2021-08-24 15:24   ` Sean Christopherson
@ 2021-08-24 15:42     ` Xiaoyao Li
  0 siblings, 0 replies; 19+ messages in thread
From: Xiaoyao Li @ 2021-08-24 15:42 UTC (permalink / raw)
  To: Sean Christopherson
  Cc: Paolo Bonzini, Vitaly Kuznetsov, Wanpeng Li, Jim Mattson,
	Joerg Roedel, kvm, linux-kernel

On 8/24/2021 11:24 PM, Sean Christopherson wrote:
> On Tue, Aug 24, 2021, Xiaoyao Li wrote:
>> The number of guest's valid PT ADDR MSRs is cached in
> 
> Can you do s/cached/precomputed in the shortlog and changelog?  Explanation below.

OK.

>> vmx->pt_desc.addr_range. Use it instead of calculating it again.
>>
>> Signed-off-by: Xiaoyao Li <xiaoyao.li@intel.com>
>> ---
>>   arch/x86/kvm/vmx/vmx.c | 3 +--
>>   1 file changed, 1 insertion(+), 2 deletions(-)
>>
>> diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c
>> index e0a9460e4dab..7ed96c460661 100644
>> --- a/arch/x86/kvm/vmx/vmx.c
>> +++ b/arch/x86/kvm/vmx/vmx.c
>> @@ -2202,8 +2202,7 @@ static int vmx_set_msr(struct kvm_vcpu *vcpu, struct msr_data *msr_info)
>>   		if (!pt_can_write_msr(vmx))
>>   			return 1;
>>   		index = msr_info->index - MSR_IA32_RTIT_ADDR0_A;
>> -		if (index >= 2 * intel_pt_validate_cap(vmx->pt_desc.caps,
>> -						       PT_CAP_num_address_ranges))
>> +		if (index >= 2 * vmx->pt_desc.addr_range)
> 
> Ugh, "validate" is a lie, a better name would be intel_pt_get_cap() or so.  There
> is no validation, the helper is simply extracting the requested cap from the
> passed in array of capabilities.
> 
> That matters in this case because the number of address ranges exposed to the
> guest is not bounded by the number of address ranges present in hardware, i.e.
> it's not "validated".  And that matters because KVM uses vmx->pt_desc.addr_range
> to pass through the ADDRn_{A,B} MSRs when tracing enabled.  In other words,
> userspace can expose MSRs to the guest that do not exist.

That's why I provided patch 5.

> The bug shouldn't be a security issue, so long as Intel CPUs are bug free and
> aren't doing silly things with MSR indexes.  The number of possible address ranges
> is encoded in three bits, thus the theoretical max is 8 ranges.  So userspace can't
> get access to arbitrary MSRs, just ADDR0_A -> ADDR7_B.
> 
> And since KVM would be modifying the "validated" value, it's more than just a
> cache, hence the request to use "precomputed".
> 
> Finally, vmx_get_msr() should use the precomputed value as well.

Argh, I missed it.

> P.S. If you want to introduce a bit of churn, s/addr_range/nr_addr_ranges would
>       be a welcome change as well.

In a separate patch?

>>   			return 1;
>>   		if (is_noncanonical_address(data, vcpu))
>>   			return 1;
>> -- 
>> 2.27.0
>>


^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 4/5] KVM: VMX: Disallow PT MSRs accessing if PT is not exposed to guest
  2021-08-24 15:35     ` Xiaoyao Li
@ 2021-08-24 16:48       ` Sean Christopherson
  0 siblings, 0 replies; 19+ messages in thread
From: Sean Christopherson @ 2021-08-24 16:48 UTC (permalink / raw)
  To: Xiaoyao Li
  Cc: Paolo Bonzini, Vitaly Kuznetsov, Wanpeng Li, Jim Mattson,
	Joerg Roedel, kvm, linux-kernel

On Tue, Aug 24, 2021, Xiaoyao Li wrote:
> On 8/24/2021 10:20 PM, Sean Christopherson wrote:
> > On Tue, Aug 24, 2021, Xiaoyao Li wrote:
> > > Per SDM, it triggers #GP for all the accessing of PT MSRs, if
> > > X86_FEATURE_INTEL_PT is not available.
> > > 
> > > Signed-off-by: Xiaoyao Li <xiaoyao.li@intel.com>
> > > ---
> > >   arch/x86/kvm/vmx/vmx.c | 20 ++++++++++++++------
> > >   1 file changed, 14 insertions(+), 6 deletions(-)
> > > 
> > > diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c
> > > index 4a70a6d2f442..1bbc4d84c623 100644
> > > --- a/arch/x86/kvm/vmx/vmx.c
> > > +++ b/arch/x86/kvm/vmx/vmx.c
> > > @@ -1010,9 +1010,16 @@ static unsigned long segment_base(u16 selector)
> > >   static inline bool pt_can_write_msr(struct vcpu_vmx *vmx)
> > >   {
> > >   	return vmx_pt_mode_is_host_guest() &&
> > > +	       guest_cpuid_has(&vmx->vcpu, X86_FEATURE_INTEL_PT) &&
> > >   	       !(vmx->pt_desc.guest.ctl & RTIT_CTL_TRACEEN);
> > >   }
> > > +static inline bool pt_can_read_msr(struct kvm_vcpu *vcpu)
> > > +{
> > > +	return vmx_pt_mode_is_host_guest() &&
> > > +	       guest_cpuid_has(vcpu, X86_FEATURE_INTEL_PT);
> > > +}
> > > +
> > >   static inline bool pt_output_base_valid(struct kvm_vcpu *vcpu, u64 base)
> > >   {
> > >   	/* The base must be 128-byte aligned and a legal physical address. */
> > > @@ -1849,24 +1856,24 @@ static int vmx_get_msr(struct kvm_vcpu *vcpu, struct msr_data *msr_info)
> > >   							&msr_info->data);
> > >   		break;
> > >   	case MSR_IA32_RTIT_CTL:
> > > -		if (!vmx_pt_mode_is_host_guest())
> > > +		if (!pt_can_read_msr(vcpu))
> > 
> > These all need to provide exemptions for accesses from the host.  KVM allows
> > access to MSRs that are not exposed to the guest so long as all the other checks
> > pass.
> 
> Not all the MSRs are allowed to be accessed from host regardless of whether
> it's exposed to guest. e.g., MSR_IA32_TSC_ADJUST, it checks guest CPUID
> first.
> 
> For me, for those PT MSRs, I cannot think of any reason that host/userspace
> would access them without PT being exposed to guest.

Order of operations.  Userspace is allowed to do KVM_GET/SET_MSR before
KVM_SET_CPUID2.

> On the other hand, since this patch indeed breaks the existing userspace VMM
> who accesses those MSRs without checking guest CPUID.
> 
> So I will follow your advice to allow the host_initiated case in next
> version.
> 
> > Same for the next patch.
> 
> Sorry, I don't know how it matters next patch.

Me either.  Ignore that comment. :-)

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 1/5] KVM: VMX: Restore host's MSR_IA32_RTIT_CTL when it's not zero
  2021-08-24 11:07 ` [PATCH 1/5] KVM: VMX: Restore host's MSR_IA32_RTIT_CTL when it's not zero Xiaoyao Li
@ 2021-08-24 17:54   ` Sean Christopherson
  0 siblings, 0 replies; 19+ messages in thread
From: Sean Christopherson @ 2021-08-24 17:54 UTC (permalink / raw)
  To: Xiaoyao Li
  Cc: Paolo Bonzini, Vitaly Kuznetsov, Wanpeng Li, Jim Mattson,
	Joerg Roedel, kvm, linux-kernel

On Tue, Aug 24, 2021, Xiaoyao Li wrote:
> A minor optimation to WRMSR MSR_IA32_RTIT_CTL when necessary.
> 
> Signed-off-by: Xiaoyao Li <xiaoyao.li@intel.com>
> ---
>  arch/x86/kvm/vmx/vmx.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c
> index fada1055f325..e0a9460e4dab 100644
> --- a/arch/x86/kvm/vmx/vmx.c
> +++ b/arch/x86/kvm/vmx/vmx.c
> @@ -1075,7 +1075,8 @@ static void pt_guest_exit(struct vcpu_vmx *vmx)
>  	}
>  
>  	/* Reload host state (IA32_RTIT_CTL will be cleared on VM exit). */

Could you opportunistically update the comment to call out that KVM requires
VM_EXIT_CLEAR_IA32_RTIT_CTL to expose PT to the guest?  E.g. something like

	/*
	 * KVM's requires VM_EXIT_CLEAR_IA32_RTIT_CTL to expose PT to the guest,
	 * i.e. RTIT_CTL is always cleared on VM-Exit.  Restore it if necessary.
	 */

With that,

Reviewed-by: Sean Christopherson <seanjc@google.com> 

> -	wrmsrl(MSR_IA32_RTIT_CTL, vmx->pt_desc.host.ctl);
> +	if (vmx->pt_desc.host.ctl)
> +		wrmsrl(MSR_IA32_RTIT_CTL, vmx->pt_desc.host.ctl);
>  }
>  
>  void vmx_set_host_fs_gs(struct vmcs_host_state *host, u16 fs_sel, u16 gs_sel,
> -- 
> 2.27.0
> 

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 3/5] KVM: VMX: RTIT_CTL_BRANCH_EN has no dependency on other CPUID bit
  2021-08-24 11:07 ` [PATCH 3/5] KVM: VMX: RTIT_CTL_BRANCH_EN has no dependency on other CPUID bit Xiaoyao Li
@ 2021-08-25  3:30   ` Like Xu
  2021-08-25  4:19     ` Xiaoyao Li
  0 siblings, 1 reply; 19+ messages in thread
From: Like Xu @ 2021-08-25  3:30 UTC (permalink / raw)
  To: Xiaoyao Li, Alexander Shishkin (hwtracing + intel_th + stm + R:perf)
  Cc: Sean Christopherson, Vitaly Kuznetsov, Wanpeng Li, Jim Mattson,
	Joerg Roedel, kvm, linux-kernel, Paolo Bonzini

+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@intel.com>
> ---
>   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."

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 ?

>   	 */
>   	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))
> 

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 3/5] KVM: VMX: RTIT_CTL_BRANCH_EN has no dependency on other CPUID bit
  2021-08-25  3:30   ` Like Xu
@ 2021-08-25  4:19     ` Xiaoyao Li
  2021-08-25  6:08       ` Like Xu
  0 siblings, 1 reply; 19+ messages in thread
From: Xiaoyao Li @ 2021-08-25  4:19 UTC (permalink / raw)
  To: Like Xu, Alexander Shishkin (hwtracing + intel_th + stm + R:perf)
  Cc: Sean Christopherson, Vitaly Kuznetsov, Wanpeng Li, Jim Mattson,
	Joerg Roedel, kvm, linux-kernel, Paolo Bonzini

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@intel.com>
>> ---
>>   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.

Anyway, I think it's just a mistake on CPUID instruction document of SDM.

CPUD.(EAX=14H,ECX=0):EBX[3] should only indicates the MTC support.

BranchEn should be always supported if PT is available. Per "31.2.7.2 
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))
>>


^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 3/5] KVM: VMX: RTIT_CTL_BRANCH_EN has no dependency on other CPUID bit
  2021-08-25  4:19     ` Xiaoyao Li
@ 2021-08-25  6:08       ` Like Xu
  2021-08-25  6:33         ` Xiaoyao Li
  0 siblings, 1 reply; 19+ messages in thread
From: Like Xu @ 2021-08-25  6:08 UTC (permalink / raw)
  To: Xiaoyao Li
  Cc: Sean Christopherson, Vitaly Kuznetsov, Wanpeng Li, Jim Mattson,
	Joerg Roedel, kvm, linux-kernel, Paolo Bonzini,
	Alexander Shishkin (hwtracing + intel_th + stm + R:perf)

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@intel.com>
>>> ---
>>>   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))
>>>
> 

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 3/5] KVM: VMX: RTIT_CTL_BRANCH_EN has no dependency on other CPUID bit
  2021-08-25  6:08       ` Like Xu
@ 2021-08-25  6:33         ` Xiaoyao Li
  2021-08-25  8:14           ` Like Xu
  2021-08-25 11:53           ` Alexander Shishkin
  0 siblings, 2 replies; 19+ messages in thread
From: Xiaoyao Li @ 2021-08-25  6:33 UTC (permalink / raw)
  To: Like Xu
  Cc: Sean Christopherson, Vitaly Kuznetsov, Wanpeng Li, Jim Mattson,
	Joerg Roedel, kvm, linux-kernel, Paolo Bonzini,
	Alexander Shishkin (hwtracing + intel_th + stm + R:perf)

On 8/25/2021 2:08 PM, Like Xu wrote:
> 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@intel.com>
>>>> ---
>>>>   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.

This commit shows BranchEn is supported on BDW, and must be enabled on 
BDW. This doesn't conflict the description above that BranchEn should be 
always supported.

>> 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))
>>>>
>>


^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 3/5] KVM: VMX: RTIT_CTL_BRANCH_EN has no dependency on other CPUID bit
  2021-08-25  6:33         ` Xiaoyao Li
@ 2021-08-25  8:14           ` Like Xu
  2021-08-25  8:58             ` Xiaoyao Li
  2021-08-25 11:53           ` Alexander Shishkin
  1 sibling, 1 reply; 19+ messages in thread
From: Like Xu @ 2021-08-25  8:14 UTC (permalink / raw)
  To: Xiaoyao Li
  Cc: Sean Christopherson, Vitaly Kuznetsov, Wanpeng Li, Jim Mattson,
	Joerg Roedel, kvm, linux-kernel, Paolo Bonzini,
	Alexander Shishkin (hwtracing + intel_th + stm + R:perf)

On 25/8/2021 2:33 pm, Xiaoyao Li wrote:
> On 8/25/2021 2:08 PM, Like Xu wrote:
>> 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@intel.com>
>>>>> ---
>>>>>   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.
> 
> This commit shows BranchEn is supported on BDW, and must be enabled on BDW. This 
> doesn't conflict the description above that BranchEn should be always supported.

Per Vol. 4 Table 2-34. Additional MSRs Common to Processors Based the
Broadwell Microarchitectures, the BranchEn bit 13 is:

	"Reserved; writing 0 will #GP if also setting TraceEn"

on the Intel® Core™ M Processors.

My point is that we, especially software developers from hardware vendors,
should really focus on real hardware and fix real problems.

<EOM>

> 
>>> 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))
>>>>>
>>>
> 

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 3/5] KVM: VMX: RTIT_CTL_BRANCH_EN has no dependency on other CPUID bit
  2021-08-25  8:14           ` Like Xu
@ 2021-08-25  8:58             ` Xiaoyao Li
  0 siblings, 0 replies; 19+ messages in thread
From: Xiaoyao Li @ 2021-08-25  8:58 UTC (permalink / raw)
  To: Like Xu
  Cc: Sean Christopherson, Vitaly Kuznetsov, Wanpeng Li, Jim Mattson,
	Joerg Roedel, kvm, linux-kernel, Paolo Bonzini,
	Alexander Shishkin (hwtracing + intel_th + stm + R:perf)

On 8/25/2021 4:14 PM, Like Xu wrote:
> On 25/8/2021 2:33 pm, Xiaoyao Li wrote:
>> On 8/25/2021 2:08 PM, Like Xu wrote:
>>> 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@intel.com>
>>>>>> ---
>>>>>>   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.
>>
>> This commit shows BranchEn is supported on BDW, and must be enabled on 
>> BDW. This doesn't conflict the description above that BranchEn should 
>> be always supported.
> 
> Per Vol. 4 Table 2-34. Additional MSRs Common to Processors Based the
> Broadwell Microarchitectures, the BranchEn bit 13 is:
> 
>      "Reserved; writing 0 will #GP if also setting TraceEn"
> 
> on the Intel® Core™ M Processors.
> 
> My point is that we, especially software developers from hardware vendors,
> should really focus on real hardware and fix real problems.

Isn't this patch fixing real problem? Without it, it forbids guest to 
enable BranchEn if PT_MTC_cap not exposed to guest.



^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 3/5] KVM: VMX: RTIT_CTL_BRANCH_EN has no dependency on other CPUID bit
  2021-08-25  6:33         ` Xiaoyao Li
  2021-08-25  8:14           ` Like Xu
@ 2021-08-25 11:53           ` Alexander Shishkin
  1 sibling, 0 replies; 19+ messages in thread
From: Alexander Shishkin @ 2021-08-25 11:53 UTC (permalink / raw)
  To: Xiaoyao Li, Like Xu
  Cc: Sean Christopherson, Vitaly Kuznetsov, Wanpeng Li, Jim Mattson,
	Joerg Roedel, kvm, linux-kernel, Paolo Bonzini,
	alexander.shishkin

Xiaoyao Li <xiaoyao.li@intel.com> writes:

> On 8/25/2021 2:08 PM, Like Xu wrote:
>> On 25/8/2021 12:19 pm, Xiaoyao Li wrote:
>>> On 8/25/2021 11:30 AM, Like Xu wrote:
>>> BranchEn should be always supported if PT is available. Per "31.2.7.2 
>> 
>> Check d35869ba348d3f1ff3e6d8214fe0f674bb0e404e.
>
> This commit shows BranchEn is supported on BDW, and must be enabled on 
> BDW. This doesn't conflict the description above that BranchEn should be 
> always supported.

It's the *not* setting BranchEn that's not supported on BDW. The point
of BranchEn is to allow the user to not set it and filter out all the
branch trace related packets. The main point of PT, however, is the
branch trace, so in the first implementation BranchEn was reserved as
1.

IOW, it's always available, doesn't depend on CPUID, but on BDW,
BranchEn==0 should throw a #GP, if I remember right. Check BDM106 for
details.

Regards,
--
Alex

^ permalink raw reply	[flat|nested] 19+ messages in thread

end of thread, other threads:[~2021-08-25 11:53 UTC | newest]

Thread overview: 19+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-08-24 11:07 [PATCH 0/5] KVM: VMX: PT (processor trace) optimizations and fixes Xiaoyao Li
2021-08-24 11:07 ` [PATCH 1/5] KVM: VMX: Restore host's MSR_IA32_RTIT_CTL when it's not zero Xiaoyao Li
2021-08-24 17:54   ` Sean Christopherson
2021-08-24 11:07 ` [PATCH 2/5] KVM: VMX: Use cached vmx->pt_desc.addr_range Xiaoyao Li
2021-08-24 15:24   ` Sean Christopherson
2021-08-24 15:42     ` Xiaoyao Li
2021-08-24 11:07 ` [PATCH 3/5] KVM: VMX: RTIT_CTL_BRANCH_EN has no dependency on other CPUID bit Xiaoyao Li
2021-08-25  3:30   ` Like Xu
2021-08-25  4:19     ` Xiaoyao Li
2021-08-25  6:08       ` Like Xu
2021-08-25  6:33         ` Xiaoyao Li
2021-08-25  8:14           ` Like Xu
2021-08-25  8:58             ` Xiaoyao Li
2021-08-25 11:53           ` Alexander Shishkin
2021-08-24 11:07 ` [PATCH 4/5] KVM: VMX: Disallow PT MSRs accessing if PT is not exposed to guest Xiaoyao Li
2021-08-24 14:20   ` Sean Christopherson
2021-08-24 15:35     ` Xiaoyao Li
2021-08-24 16:48       ` Sean Christopherson
2021-08-24 11:07 ` [PATCH 5/5] KVM: VMX: Check Intel PT related CPUID leaves Xiaoyao Li

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).