[patch v2 0/8] Intel Processor Trace virtulization enabling

[patch v2 0/8] Intel Processor Trace virtulization enabling

[Luwei Kang]

From: Chao Peng <chao.p.peng@linux.intel.com>

Hi All,

Here is a patch-series which adding Processor Trace enabling in KVM guest. You can get It's software developer manuals from:
https://software.intel.com/sites/default/files/managed/c5/15/architecture-instruction-set-extensions-programming-reference.pdf
In Chapter 5 INTEL PROCESSOR TRACE: VMX IMPROVEMENTS.

Introduction:
Intel Processor Trace (Intel PT) is an extension of Intel Architecture that captures information about software execution using dedicated hardware facilities that cause only minimal performance perturbation to the software being traced. Details on the Intel PT infrastructure and trace capabilities can be found in the Intel 64 and IA-32 Architectures Software Developer’s Manual, Volume 3C.

The suite of architecture changes serve to simplify the process of virtualizing Intel PT for use by a guest software. There are two primary elements to this new architecture support for VMX support improvements made for Intel PT.
1. Addition of a new guest IA32_RTIT_CTL value field to the VMCS.
  — This serves to speed and simplify the process of disabling trace on VM exit, and restoring it on VM entry.
2. Enabling use of EPT to redirect PT output.
  — This enables the VMM to elect to virtualize the PT output buffer using EPT. In this mode, the CPU will treat PT output addresses as Guest Physical Addresses (GPAs) and translate them using EPT. This means that Intel PT output reads (of the ToPA table) and writes (of trace output) can cause EPT violations, and other output events.

Processor Trace virtualization can be work in one of 3 possible modes by set new option "pt_mode". Default value is host guest mode.
 a. system-wide: trace both host/guest and output to host buffer;
 b. host-only: only trace host and output to host buffer;
 c. host-guest: trace host/guest simultaneous and output to their respective buffer.

>From v1:
 - remove guest-only mode because guest-only mode can be covered by host-guest mode;
 - always set "use GPA for processor tracing" in secondary execution control if it can be;
 - trap RTIT_CTL read/write. Forbid write this msr when VMXON in L1 hypervisor.

Chao Peng (8):
  perf/x86/intel/pt: Move Intel-PT MSR bit definitions to a public
    header
  perf/x86/intel/pt: change pt_cap_get() to a public function
  KVM: x86: add Intel processor trace virtualization mode
  KVM: x86: add Intel processor trace cpuid emulataion
  KVM: x86: add Intel processor trace context for each vcpu
  KVM: x86: Implement Intel processor trace context switch
  KVM: x86: add Intel PT msr RTIT_CTL read/write
  KVM: x86: Disable intercept for Intel processor trace MSRs

 arch/x86/events/intel/pt.c       |   3 +-
 arch/x86/events/intel/pt.h       |  55 -----------
 arch/x86/include/asm/intel_pt.h  |  26 ++++++
 arch/x86/include/asm/kvm_host.h  |   1 +
 arch/x86/include/asm/msr-index.h |  35 +++++++
 arch/x86/include/asm/vmx.h       |   8 ++
 arch/x86/kvm/cpuid.c             |  23 ++++-
 arch/x86/kvm/svm.c               |   6 ++
 arch/x86/kvm/vmx.c               | 195 ++++++++++++++++++++++++++++++++++++++-
 9 files changed, 290 insertions(+), 62 deletions(-)

-- 
1.8.3.1

[patch v2 1/8] perf/x86/intel/pt: Move Intel-PT MSR bit definitions to a public header

[Luwei Kang]

From: Chao Peng <chao.p.peng@linux.intel.com>

Intel processor trace virtualization enabling in guest need
to use these MSR bits, so move then to public header msr-index.h.

Signed-off-by: Chao Peng <chao.p.peng@linux.intel.com>
Signed-off-by: Luwei Kang <luwei.kang@intel.com>
---
 arch/x86/events/intel/pt.h       | 37 -------------------------------------
 arch/x86/include/asm/msr-index.h | 33 +++++++++++++++++++++++++++++++++
 2 files changed, 33 insertions(+), 37 deletions(-)

diff --git a/arch/x86/events/intel/pt.h b/arch/x86/events/intel/pt.h
index 0eb41d0..0050ca1 100644
--- a/arch/x86/events/intel/pt.h
+++ b/arch/x86/events/intel/pt.h
@@ -20,43 +20,6 @@
 #define __INTEL_PT_H__
 
 /*
- * PT MSR bit definitions
- */
-#define RTIT_CTL_TRACEEN		BIT(0)
-#define RTIT_CTL_CYCLEACC		BIT(1)
-#define RTIT_CTL_OS			BIT(2)
-#define RTIT_CTL_USR			BIT(3)
-#define RTIT_CTL_PWR_EVT_EN		BIT(4)
-#define RTIT_CTL_FUP_ON_PTW		BIT(5)
-#define RTIT_CTL_CR3EN			BIT(7)
-#define RTIT_CTL_TOPA			BIT(8)
-#define RTIT_CTL_MTC_EN			BIT(9)
-#define RTIT_CTL_TSC_EN			BIT(10)
-#define RTIT_CTL_DISRETC		BIT(11)
-#define RTIT_CTL_PTW_EN			BIT(12)
-#define RTIT_CTL_BRANCH_EN		BIT(13)
-#define RTIT_CTL_MTC_RANGE_OFFSET	14
-#define RTIT_CTL_MTC_RANGE		(0x0full << RTIT_CTL_MTC_RANGE_OFFSET)
-#define RTIT_CTL_CYC_THRESH_OFFSET	19
-#define RTIT_CTL_CYC_THRESH		(0x0full << RTIT_CTL_CYC_THRESH_OFFSET)
-#define RTIT_CTL_PSB_FREQ_OFFSET	24
-#define RTIT_CTL_PSB_FREQ      		(0x0full << RTIT_CTL_PSB_FREQ_OFFSET)
-#define RTIT_CTL_ADDR0_OFFSET		32
-#define RTIT_CTL_ADDR0      		(0x0full << RTIT_CTL_ADDR0_OFFSET)
-#define RTIT_CTL_ADDR1_OFFSET		36
-#define RTIT_CTL_ADDR1      		(0x0full << RTIT_CTL_ADDR1_OFFSET)
-#define RTIT_CTL_ADDR2_OFFSET		40
-#define RTIT_CTL_ADDR2      		(0x0full << RTIT_CTL_ADDR2_OFFSET)
-#define RTIT_CTL_ADDR3_OFFSET		44
-#define RTIT_CTL_ADDR3      		(0x0full << RTIT_CTL_ADDR3_OFFSET)
-#define RTIT_STATUS_FILTEREN		BIT(0)
-#define RTIT_STATUS_CONTEXTEN		BIT(1)
-#define RTIT_STATUS_TRIGGEREN		BIT(2)
-#define RTIT_STATUS_BUFFOVF		BIT(3)
-#define RTIT_STATUS_ERROR		BIT(4)
-#define RTIT_STATUS_STOPPED		BIT(5)
-
-/*
  * Single-entry ToPA: when this close to region boundary, switch
  * buffers to avoid losing data.
  */
diff --git a/arch/x86/include/asm/msr-index.h b/arch/x86/include/asm/msr-index.h
index 17f5c12..80b26e1 100644
--- a/arch/x86/include/asm/msr-index.h
+++ b/arch/x86/include/asm/msr-index.h
@@ -93,7 +93,40 @@
 #define MSR_PEBS_LD_LAT_THRESHOLD	0x000003f6
 
 #define MSR_IA32_RTIT_CTL		0x00000570
+#define RTIT_CTL_TRACEEN		BIT(0)
+#define RTIT_CTL_CYCLEACC		BIT(1)
+#define RTIT_CTL_OS			BIT(2)
+#define RTIT_CTL_USR			BIT(3)
+#define RTIT_CTL_PWR_EVT_EN		BIT(4)
+#define RTIT_CTL_FUP_ON_PTW		BIT(5)
+#define RTIT_CTL_CR3EN			BIT(7)
+#define RTIT_CTL_TOPA			BIT(8)
+#define RTIT_CTL_MTC_EN			BIT(9)
+#define RTIT_CTL_TSC_EN			BIT(10)
+#define RTIT_CTL_DISRETC		BIT(11)
+#define RTIT_CTL_PTW_EN			BIT(12)
+#define RTIT_CTL_BRANCH_EN		BIT(13)
+#define RTIT_CTL_MTC_RANGE_OFFSET	14
+#define RTIT_CTL_MTC_RANGE		(0x0full << RTIT_CTL_MTC_RANGE_OFFSET)
+#define RTIT_CTL_CYC_THRESH_OFFSET	19
+#define RTIT_CTL_CYC_THRESH		(0x0full << RTIT_CTL_CYC_THRESH_OFFSET)
+#define RTIT_CTL_PSB_FREQ_OFFSET	24
+#define RTIT_CTL_PSB_FREQ		(0x0full << RTIT_CTL_PSB_FREQ_OFFSET)
+#define RTIT_CTL_ADDR0_OFFSET		32
+#define RTIT_CTL_ADDR0			(0x0full << RTIT_CTL_ADDR0_OFFSET)
+#define RTIT_CTL_ADDR1_OFFSET		36
+#define RTIT_CTL_ADDR1			(0x0full << RTIT_CTL_ADDR1_OFFSET)
+#define RTIT_CTL_ADDR2_OFFSET		40
+#define RTIT_CTL_ADDR2			(0x0full << RTIT_CTL_ADDR2_OFFSET)
+#define RTIT_CTL_ADDR3_OFFSET		44
+#define RTIT_CTL_ADDR3			(0x0full << RTIT_CTL_ADDR3_OFFSET)
 #define MSR_IA32_RTIT_STATUS		0x00000571
+#define RTIT_STATUS_FILTEREN		BIT(0)
+#define RTIT_STATUS_CONTEXTEN		BIT(1)
+#define RTIT_STATUS_TRIGGEREN		BIT(2)
+#define RTIT_STATUS_BUFFOVF		BIT(3)
+#define RTIT_STATUS_ERROR		BIT(4)
+#define RTIT_STATUS_STOPPED		BIT(5)
 #define MSR_IA32_RTIT_ADDR0_A		0x00000580
 #define MSR_IA32_RTIT_ADDR0_B		0x00000581
 #define MSR_IA32_RTIT_ADDR1_A		0x00000582
-- 
1.8.3.1

[patch v2 2/8] perf/x86/intel/pt: change pt_cap_get() to a public function

[Luwei Kang]

From: Chao Peng <chao.p.peng@linux.intel.com>

Change pt_cap_get() to a public function so that KVM
can access it.

Signed-off-by: Chao Peng <chao.p.peng@linux.intel.com>
Signed-off-by: Luwei Kang <luwei.kang@intel.com>
---
 arch/x86/events/intel/pt.c      |  3 ++-
 arch/x86/events/intel/pt.h      | 18 ------------------
 arch/x86/include/asm/intel_pt.h | 20 ++++++++++++++++++++
 3 files changed, 22 insertions(+), 19 deletions(-)

diff --git a/arch/x86/events/intel/pt.c b/arch/x86/events/intel/pt.c
index 81fd41d..a5a7e44 100644
--- a/arch/x86/events/intel/pt.c
+++ b/arch/x86/events/intel/pt.c
@@ -75,7 +75,7 @@
 	PT_CAP(psb_periods,		1, CPUID_EBX, 0xffff0000),
 };
 
-static u32 pt_cap_get(enum pt_capabilities cap)
+u32 pt_cap_get(enum pt_capabilities cap)
 {
 	struct pt_cap_desc *cd = &pt_caps[cap];
 	u32 c = pt_pmu.caps[cd->leaf * PT_CPUID_REGS_NUM + cd->reg];
@@ -83,6 +83,7 @@ static u32 pt_cap_get(enum pt_capabilities cap)
 
 	return (c & cd->mask) >> shift;
 }
+EXPORT_SYMBOL_GPL(pt_cap_get);
 
 static ssize_t pt_cap_show(struct device *cdev,
 			   struct device_attribute *attr,
diff --git a/arch/x86/events/intel/pt.h b/arch/x86/events/intel/pt.h
index 0050ca1..d75c9f3 100644
--- a/arch/x86/events/intel/pt.h
+++ b/arch/x86/events/intel/pt.h
@@ -51,24 +51,6 @@ struct topa_entry {
 /* TSC to Core Crystal Clock Ratio */
 #define CPUID_TSC_LEAF		0x15
 
-enum pt_capabilities {
-	PT_CAP_max_subleaf = 0,
-	PT_CAP_cr3_filtering,
-	PT_CAP_psb_cyc,
-	PT_CAP_ip_filtering,
-	PT_CAP_mtc,
-	PT_CAP_ptwrite,
-	PT_CAP_power_event_trace,
-	PT_CAP_topa_output,
-	PT_CAP_topa_multiple_entries,
-	PT_CAP_single_range_output,
-	PT_CAP_payloads_lip,
-	PT_CAP_num_address_ranges,
-	PT_CAP_mtc_periods,
-	PT_CAP_cycle_thresholds,
-	PT_CAP_psb_periods,
-};
-
 struct pt_pmu {
 	struct pmu		pmu;
 	u32			caps[PT_CPUID_REGS_NUM * PT_CPUID_LEAVES];
diff --git a/arch/x86/include/asm/intel_pt.h b/arch/x86/include/asm/intel_pt.h
index e1a4117..73c8942 100644
--- a/arch/x86/include/asm/intel_pt.h
+++ b/arch/x86/include/asm/intel_pt.h
@@ -1,10 +1,30 @@
 #ifndef _ASM_X86_INTEL_PT_H
 #define _ASM_X86_INTEL_PT_H
 
+enum pt_capabilities {
+	PT_CAP_max_subleaf = 0,
+	PT_CAP_cr3_filtering,
+	PT_CAP_psb_cyc,
+	PT_CAP_ip_filtering,
+	PT_CAP_mtc,
+	PT_CAP_ptwrite,
+	PT_CAP_power_event_trace,
+	PT_CAP_topa_output,
+	PT_CAP_topa_multiple_entries,
+	PT_CAP_single_range_output,
+	PT_CAP_payloads_lip,
+	PT_CAP_num_address_ranges,
+	PT_CAP_mtc_periods,
+	PT_CAP_cycle_thresholds,
+	PT_CAP_psb_periods,
+};
+
 #if defined(CONFIG_PERF_EVENTS) && defined(CONFIG_CPU_SUP_INTEL)
 void cpu_emergency_stop_pt(void);
+extern u32 pt_cap_get(enum pt_capabilities cap);
 #else
 static inline void cpu_emergency_stop_pt(void) {}
+static inline u32 pt_cap_get(enum pt_capabilities cap) { return 0; }
 #endif
 
 #endif /* _ASM_X86_INTEL_PT_H */
-- 
1.8.3.1

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

[Luwei Kang]

From: Chao Peng <chao.p.peng@linux.intel.com>

PT virtualization can be work in one of 3 possible modes:
 a. system-wide: trace both host/guest and output to host buffer;
 b. host-only: only trace host and output to host buffer;
 c. host-guest: trace host/guest simultaneous and output to their
    respective buffer.

Signed-off-by: Chao Peng <chao.p.peng@linux.intel.com>
Signed-off-by: Luwei Kang <luwei.kang@intel.com>
---
 arch/x86/include/asm/intel_pt.h  |  6 ++++
 arch/x86/include/asm/msr-index.h |  1 +
 arch/x86/include/asm/vmx.h       |  6 ++++
 arch/x86/kvm/vmx.c               | 73 +++++++++++++++++++++++++++++++++++++---
 4 files changed, 82 insertions(+), 4 deletions(-)

diff --git a/arch/x86/include/asm/intel_pt.h b/arch/x86/include/asm/intel_pt.h
index 73c8942..9d6ba1f 100644
--- a/arch/x86/include/asm/intel_pt.h
+++ b/arch/x86/include/asm/intel_pt.h
@@ -1,6 +1,12 @@
 #ifndef _ASM_X86_INTEL_PT_H
 #define _ASM_X86_INTEL_PT_H
 
+enum pt_mode {
+	PT_MODE_SYSTEM = 0,
+	PT_MODE_HOST,
+	PT_MODE_HOST_GUEST,
+};
+
 enum pt_capabilities {
 	PT_CAP_max_subleaf = 0,
 	PT_CAP_cr3_filtering,
diff --git a/arch/x86/include/asm/msr-index.h b/arch/x86/include/asm/msr-index.h
index 80b26e1..57433e4 100644
--- a/arch/x86/include/asm/msr-index.h
+++ b/arch/x86/include/asm/msr-index.h
@@ -751,6 +751,7 @@
 #define VMX_BASIC_INOUT		0x0040000000000000LLU
 
 /* MSR_IA32_VMX_MISC bits */
+#define MSR_IA32_VMX_MISC_INTEL_PT                 (1ULL << 14)
 #define MSR_IA32_VMX_MISC_VMWRITE_SHADOW_RO_FIELDS (1ULL << 29)
 #define MSR_IA32_VMX_MISC_PREEMPTION_TIMER_SCALE   0x1F
 /* AMD-V MSRs */
diff --git a/arch/x86/include/asm/vmx.h b/arch/x86/include/asm/vmx.h
index caec841..80e3e22 100644
--- a/arch/x86/include/asm/vmx.h
+++ b/arch/x86/include/asm/vmx.h
@@ -76,7 +76,9 @@
 #define SECONDARY_EXEC_SHADOW_VMCS              0x00004000
 #define SECONDARY_EXEC_RDSEED			0x00010000
 #define SECONDARY_EXEC_ENABLE_PML               0x00020000
+#define SECONDARY_EXEC_PT_CONCEAL_VMX		0x00080000
 #define SECONDARY_EXEC_XSAVES			0x00100000
+#define SECONDARY_EXEC_PT_USE_GPA		0x01000000
 #define SECONDARY_EXEC_TSC_SCALING              0x02000000
 
 #define PIN_BASED_EXT_INTR_MASK                 0x00000001
@@ -97,6 +99,8 @@
 #define VM_EXIT_LOAD_IA32_EFER                  0x00200000
 #define VM_EXIT_SAVE_VMX_PREEMPTION_TIMER       0x00400000
 #define VM_EXIT_CLEAR_BNDCFGS                   0x00800000
+#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
 
 #define VM_ENTRY_ALWAYSON_WITHOUT_TRUE_MSR	0x000011ff
 
diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
index 95a0160..cb0928d 100644
--- a/arch/x86/kvm/vmx.c
+++ b/arch/x86/kvm/vmx.c
@@ -50,6 +50,7 @@
 #include <asm/apic.h>
 #include <asm/irq_remapping.h>
 #include <asm/mmu_context.h>
+#include <asm/intel_pt.h>
 
 #include "trace.h"
 #include "pmu.h"
@@ -178,6 +179,10 @@
 static int ple_window_max        = KVM_VMX_DEFAULT_PLE_WINDOW_MAX;
 module_param(ple_window_max, int, S_IRUGO);
 
+/* Default is host guest mode. */
+static int __read_mostly pt_mode = PT_MODE_HOST_GUEST;
+module_param(pt_mode, int, S_IRUGO);
+
 extern const ulong vmx_return;
 
 #define NR_AUTOLOAD_MSRS 8
@@ -1321,6 +1326,19 @@ static inline bool cpu_has_vmx_vmfunc(void)
 		SECONDARY_EXEC_ENABLE_VMFUNC;
 }
 
+static inline bool cpu_has_vmx_intel_pt(void)
+{
+	u64 vmx_msr;
+
+	rdmsrl(MSR_IA32_VMX_MISC, vmx_msr);
+	return vmx_msr & MSR_IA32_VMX_MISC_INTEL_PT;
+}
+
+static inline bool cpu_has_vmx_pt_use_gpa(void)
+{
+	return vmcs_config.cpu_based_2nd_exec_ctrl & SECONDARY_EXEC_PT_USE_GPA;
+}
+
 static inline bool report_flexpriority(void)
 {
 	return flexpriority_enabled;
@@ -3661,6 +3679,8 @@ static __init int setup_vmcs_config(struct vmcs_config *vmcs_conf)
 			SECONDARY_EXEC_RDRAND |
 			SECONDARY_EXEC_ENABLE_PML |
 			SECONDARY_EXEC_TSC_SCALING |
+			SECONDARY_EXEC_PT_USE_GPA |
+			SECONDARY_EXEC_PT_CONCEAL_VMX |
 			SECONDARY_EXEC_ENABLE_VMFUNC;
 		if (adjust_vmx_controls(min2, opt2,
 					MSR_IA32_VMX_PROCBASED_CTLS2,
@@ -3694,7 +3714,8 @@ static __init int setup_vmcs_config(struct vmcs_config *vmcs_conf)
 	min |= VM_EXIT_HOST_ADDR_SPACE_SIZE;
 #endif
 	opt = VM_EXIT_SAVE_IA32_PAT | VM_EXIT_LOAD_IA32_PAT |
-		VM_EXIT_CLEAR_BNDCFGS;
+		VM_EXIT_CLEAR_BNDCFGS | VM_EXIT_PT_SUPPRESS_PIP |
+		VM_EXIT_CLEAR_IA32_RTIT_CTL;
 	if (adjust_vmx_controls(min, opt, MSR_IA32_VMX_EXIT_CTLS,
 				&_vmexit_control) < 0)
 		return -EIO;
@@ -3713,11 +3734,20 @@ static __init int setup_vmcs_config(struct vmcs_config *vmcs_conf)
 		_pin_based_exec_control &= ~PIN_BASED_POSTED_INTR;
 
 	min = VM_ENTRY_LOAD_DEBUG_CONTROLS;
-	opt = VM_ENTRY_LOAD_IA32_PAT | VM_ENTRY_LOAD_BNDCFGS;
+	opt = VM_ENTRY_LOAD_IA32_PAT | VM_ENTRY_LOAD_BNDCFGS |
+		VM_ENTRY_PT_SUPPRESS_PIP | VM_ENTRY_LOAD_IA32_RTIT_CTL;
 	if (adjust_vmx_controls(min, opt, MSR_IA32_VMX_ENTRY_CTLS,
 				&_vmentry_control) < 0)
 		return -EIO;
 
+	if (!(_cpu_based_2nd_exec_control & SECONDARY_EXEC_PT_USE_GPA) ||
+		!(_vmexit_control & VM_EXIT_CLEAR_IA32_RTIT_CTL) ||
+		!(_vmentry_control & VM_ENTRY_LOAD_IA32_RTIT_CTL)) {
+		_cpu_based_2nd_exec_control &= ~SECONDARY_EXEC_PT_USE_GPA;
+		_vmexit_control &= ~VM_EXIT_CLEAR_IA32_RTIT_CTL;
+		_vmentry_control &= ~VM_ENTRY_LOAD_IA32_RTIT_CTL;
+	}
+
 	rdmsr(MSR_IA32_VMX_BASIC, vmx_msr_low, vmx_msr_high);
 
 	/* IA-32 SDM Vol 3B: VMCS size is never greater than 4kB. */
@@ -5279,6 +5309,32 @@ static u32 vmx_exec_control(struct vcpu_vmx *vmx)
 	return exec_control;
 }
 
+static u32 vmx_vmexit_control(struct vcpu_vmx *vmx)
+{
+	u32 vmexit_control = vmcs_config.vmexit_ctrl;
+
+	if (!enable_ept)
+		vmexit_control &= ~VM_EXIT_CLEAR_IA32_RTIT_CTL;
+
+	if (pt_mode == PT_MODE_SYSTEM)
+		vmexit_control &= ~VM_EXIT_PT_SUPPRESS_PIP;
+
+	return vmexit_control;
+}
+
+static u32 vmx_vmentry_control(struct vcpu_vmx *vmx)
+{
+	u32 vmentry_control = vmcs_config.vmentry_ctrl;
+
+	if (!enable_ept)
+		vmentry_control &= ~VM_ENTRY_LOAD_IA32_RTIT_CTL;
+
+	if (pt_mode == PT_MODE_SYSTEM)
+		vmentry_control &= ~VM_ENTRY_PT_SUPPRESS_PIP;
+
+	return vmentry_control;
+}
+
 static bool vmx_rdrand_supported(void)
 {
 	return vmcs_config.cpu_based_2nd_exec_ctrl &
@@ -5305,6 +5361,8 @@ static void vmx_compute_secondary_exec_control(struct vcpu_vmx *vmx)
 		enable_unrestricted_guest = 0;
 		/* Enable INVPCID for non-ept guests may cause performance regression. */
 		exec_control &= ~SECONDARY_EXEC_ENABLE_INVPCID;
+		/* Guest Intel PT uses GPA depend on EPT enabled. */
+		exec_control &= ~SECONDARY_EXEC_PT_USE_GPA;
 	}
 	if (!enable_unrestricted_guest)
 		exec_control &= ~SECONDARY_EXEC_UNRESTRICTED_GUEST;
@@ -5409,6 +5467,9 @@ static void vmx_compute_secondary_exec_control(struct vcpu_vmx *vmx)
 		}
 	}
 
+	if (pt_mode == PT_MODE_SYSTEM)
+		exec_control &= ~SECONDARY_EXEC_PT_CONCEAL_VMX;
+
 	vmx->secondary_exec_control = exec_control;
 }
 
@@ -5521,10 +5582,10 @@ static int vmx_vcpu_setup(struct vcpu_vmx *vmx)
 	}
 
 
-	vm_exit_controls_init(vmx, vmcs_config.vmexit_ctrl);
+	vm_exit_controls_init(vmx, vmx_vmexit_control(vmx));
 
 	/* 22.2.1, 20.8.1 */
-	vm_entry_controls_init(vmx, vmcs_config.vmentry_ctrl);
+	vm_entry_controls_init(vmx, vmx_vmentry_control(vmx));
 
 	vmx->vcpu.arch.cr0_guest_owned_bits = X86_CR0_TS;
 	vmcs_writel(CR0_GUEST_HOST_MASK, ~X86_CR0_TS);
@@ -6865,6 +6926,10 @@ static __init int hardware_setup(void)
 
 	kvm_mce_cap_supported |= MCG_LMCE_P;
 
+	if (!enable_ept || !pt_cap_get(PT_CAP_topa_output) ||
+		!cpu_has_vmx_intel_pt() || !cpu_has_vmx_pt_use_gpa())
+		pt_mode = PT_MODE_SYSTEM;
+
 	return alloc_kvm_area();
 
 out:
-- 
1.8.3.1

[patch v2 4/8] KVM: x86: add Intel processor trace cpuid emulataion

[Luwei Kang]

From: Chao Peng <chao.p.peng@linux.intel.com>

Expose Intel processor trace cpuid to guest.

Signed-off-by: Chao Peng <chao.p.peng@linux.intel.com>
Signed-off-by: Luwei Kang <luwei.kang@intel.com>
---
 arch/x86/include/asm/kvm_host.h |  1 +
 arch/x86/kvm/cpuid.c            | 23 +++++++++++++++++++++--
 arch/x86/kvm/svm.c              |  6 ++++++
 arch/x86/kvm/vmx.c              |  6 ++++++
 4 files changed, 34 insertions(+), 2 deletions(-)

diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
index c73e493..14f4b81 100644
--- a/arch/x86/include/asm/kvm_host.h
+++ b/arch/x86/include/asm/kvm_host.h
@@ -1004,6 +1004,7 @@ struct kvm_x86_ops {
 	void (*handle_external_intr)(struct kvm_vcpu *vcpu);
 	bool (*mpx_supported)(void);
 	bool (*xsaves_supported)(void);
+	bool (*pt_supported)(void);
 
 	int (*check_nested_events)(struct kvm_vcpu *vcpu, bool external_intr);
 
diff --git a/arch/x86/kvm/cpuid.c b/arch/x86/kvm/cpuid.c
index 0099e10..ef19a11 100644
--- a/arch/x86/kvm/cpuid.c
+++ b/arch/x86/kvm/cpuid.c
@@ -70,6 +70,7 @@ u64 kvm_supported_xcr0(void)
 /* These are scattered features in cpufeatures.h. */
 #define KVM_CPUID_BIT_AVX512_4VNNIW     2
 #define KVM_CPUID_BIT_AVX512_4FMAPS     3
+#define KVM_CPUID_BIT_INTEL_PT		25
 #define KF(x) bit(KVM_CPUID_BIT_##x)
 
 int kvm_update_cpuid(struct kvm_vcpu *vcpu)
@@ -327,6 +328,7 @@ static inline int __do_cpuid_ent(struct kvm_cpuid_entry2 *entry, u32 function,
 	unsigned f_invpcid = kvm_x86_ops->invpcid_supported() ? F(INVPCID) : 0;
 	unsigned f_mpx = kvm_mpx_supported() ? F(MPX) : 0;
 	unsigned f_xsaves = kvm_x86_ops->xsaves_supported() ? F(XSAVES) : 0;
+	unsigned f_intel_pt = kvm_x86_ops->pt_supported() ? KF(INTEL_PT) : 0;
 
 	/* cpuid 1.edx */
 	const u32 kvm_cpuid_1_edx_x86_features =
@@ -379,7 +381,7 @@ static inline int __do_cpuid_ent(struct kvm_cpuid_entry2 *entry, u32 function,
 		F(BMI2) | F(ERMS) | f_invpcid | F(RTM) | f_mpx | F(RDSEED) |
 		F(ADX) | F(SMAP) | F(AVX512IFMA) | F(AVX512F) | F(AVX512PF) |
 		F(AVX512ER) | F(AVX512CD) | F(CLFLUSHOPT) | F(CLWB) | F(AVX512DQ) |
-		F(SHA_NI) | F(AVX512BW) | F(AVX512VL);
+		F(SHA_NI) | F(AVX512BW) | F(AVX512VL) | f_intel_pt;
 
 	/* cpuid 0xD.1.eax */
 	const u32 kvm_cpuid_D_1_eax_x86_features =
@@ -407,7 +409,7 @@ static inline int __do_cpuid_ent(struct kvm_cpuid_entry2 *entry, u32 function,
 
 	switch (function) {
 	case 0:
-		entry->eax = min(entry->eax, (u32)0xd);
+		entry->eax = min(entry->eax, (u32)(f_intel_pt ? 0x14 : 0xd));
 		break;
 	case 1:
 		entry->edx &= kvm_cpuid_1_edx_x86_features;
@@ -578,6 +580,23 @@ static inline int __do_cpuid_ent(struct kvm_cpuid_entry2 *entry, u32 function,
 		}
 		break;
 	}
+	/* Intel PT */
+	case 0x14: {
+		int t, times = entry->eax & 0xffffffff;
+
+		if (!f_intel_pt)
+			break;
+
+		entry->flags |= KVM_CPUID_FLAG_SIGNIFCANT_INDEX;
+		for (t = 1; t <= times; ++t) {
+			if (*nent >= maxnent)
+				goto out;
+			do_cpuid_1_ent(&entry[t], function, t);
+			entry[t].flags |= KVM_CPUID_FLAG_SIGNIFCANT_INDEX;
+			++*nent;
+		}
+		break;
+	}
 	case KVM_CPUID_SIGNATURE: {
 		static const char signature[12] = "KVMKVMKVM\0\0";
 		const u32 *sigptr = (const u32 *)signature;
diff --git a/arch/x86/kvm/svm.c b/arch/x86/kvm/svm.c
index 0e68f0b..015c824 100644
--- a/arch/x86/kvm/svm.c
+++ b/arch/x86/kvm/svm.c
@@ -5174,6 +5174,11 @@ static bool svm_xsaves_supported(void)
 	return false;
 }
 
+static bool svm_pt_supported(void)
+{
+	return false;
+}
+
 static bool svm_has_wbinvd_exit(void)
 {
 	return true;
@@ -5485,6 +5490,7 @@ static void svm_setup_mce(struct kvm_vcpu *vcpu)
 	.invpcid_supported = svm_invpcid_supported,
 	.mpx_supported = svm_mpx_supported,
 	.xsaves_supported = svm_xsaves_supported,
+	.pt_supported = svm_pt_supported,
 
 	.set_supported_cpuid = svm_set_supported_cpuid,
 
diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
index cb0928d..26fe9e7 100644
--- a/arch/x86/kvm/vmx.c
+++ b/arch/x86/kvm/vmx.c
@@ -9163,6 +9163,11 @@ static bool vmx_xsaves_supported(void)
 		SECONDARY_EXEC_XSAVES;
 }
 
+static bool vmx_pt_supported(void)
+{
+	return (pt_mode == PT_MODE_HOST_GUEST);
+}
+
 static void vmx_recover_nmi_blocking(struct vcpu_vmx *vmx)
 {
 	u32 exit_intr_info;
@@ -12107,6 +12112,7 @@ static void vmx_setup_mce(struct kvm_vcpu *vcpu)
 	.handle_external_intr = vmx_handle_external_intr,
 	.mpx_supported = vmx_mpx_supported,
 	.xsaves_supported = vmx_xsaves_supported,
+	.pt_supported = vmx_pt_supported,
 
 	.check_nested_events = vmx_check_nested_events,
 
-- 
1.8.3.1

[patch v2 5/8] KVM: x86: add Intel processor trace context for each vcpu

[Luwei Kang]

From: Chao Peng <chao.p.peng@linux.intel.com>

Add a date structure to save Intel processor trace context.
It mainly include all the MSR of Intel processor trace.

Signed-off-by: Chao Peng <chao.p.peng@linux.intel.com>
Signed-off-by: Luwei Kang <luwei.kang@intel.com>
---
 arch/x86/include/asm/msr-index.h |  1 +
 arch/x86/include/asm/vmx.h       |  2 ++
 arch/x86/kvm/vmx.c               | 17 +++++++++++++++++
 3 files changed, 20 insertions(+)

diff --git a/arch/x86/include/asm/msr-index.h b/arch/x86/include/asm/msr-index.h
index 57433e4..96124c8 100644
--- a/arch/x86/include/asm/msr-index.h
+++ b/arch/x86/include/asm/msr-index.h
@@ -135,6 +135,7 @@
 #define MSR_IA32_RTIT_ADDR2_B		0x00000585
 #define MSR_IA32_RTIT_ADDR3_A		0x00000586
 #define MSR_IA32_RTIT_ADDR3_B		0x00000587
+#define MSR_IA32_RTIT_ADDR_COUNT	8
 #define MSR_IA32_RTIT_CR3_MATCH		0x00000572
 #define MSR_IA32_RTIT_OUTPUT_BASE	0x00000560
 #define MSR_IA32_RTIT_OUTPUT_MASK	0x00000561
diff --git a/arch/x86/include/asm/vmx.h b/arch/x86/include/asm/vmx.h
index 80e3e22..6293803 100644
--- a/arch/x86/include/asm/vmx.h
+++ b/arch/x86/include/asm/vmx.h
@@ -240,6 +240,8 @@ enum vmcs_field {
 	GUEST_PDPTR3_HIGH               = 0x00002811,
 	GUEST_BNDCFGS                   = 0x00002812,
 	GUEST_BNDCFGS_HIGH              = 0x00002813,
+	GUEST_IA32_RTIT_CTL		= 0x00002814,
+	GUEST_IA32_RTIT_CTL_HIGH	= 0x00002815,
 	HOST_IA32_PAT			= 0x00002c00,
 	HOST_IA32_PAT_HIGH		= 0x00002c01,
 	HOST_IA32_EFER			= 0x00002c02,
diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
index 26fe9e7..7eaf774 100644
--- a/arch/x86/kvm/vmx.c
+++ b/arch/x86/kvm/vmx.c
@@ -566,6 +566,21 @@ static inline int pi_test_sn(struct pi_desc *pi_desc)
 			(unsigned long *)&pi_desc->control);
 }
 
+struct pt_ctx {
+	u64 ctl;
+	u64 status;
+	u64 output_base;
+	u64 output_mask;
+	u64 cr3_match;
+	u64 addrs[MSR_IA32_RTIT_ADDR_COUNT];
+};
+
+struct pt_desc {
+	unsigned int addr_num;
+	struct pt_ctx host;
+	struct pt_ctx guest;
+};
+
 struct vcpu_vmx {
 	struct kvm_vcpu       vcpu;
 	unsigned long         host_rsp;
@@ -655,6 +670,8 @@ struct vcpu_vmx {
 	 */
 	u64 msr_ia32_feature_control;
 	u64 msr_ia32_feature_control_valid_bits;
+
+	struct pt_desc pt_desc;
 };
 
 enum segment_cache_field {
-- 
1.8.3.1

[patch v2 6/8] KVM: x86: Implement Intel processor trace context switch

[Luwei Kang]

From: Chao Peng <chao.p.peng@linux.intel.com>

Load/Store Intel processor trace register in context switch.
MSR IA32_RTIT_CTL is loaded/stored automatically from VMCS,
other MSRs are loaded/stored manaully.

Signed-off-by: Chao Peng <chao.p.peng@linux.intel.com>
Signed-off-by: Luwei Kang <luwei.kang@intel.com>
---
 arch/x86/kvm/vmx.c | 75 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 75 insertions(+)

diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
index 7eaf774..091120e 100644
--- a/arch/x86/kvm/vmx.c
+++ b/arch/x86/kvm/vmx.c
@@ -2131,6 +2131,61 @@ static unsigned long segment_base(u16 selector)
 }
 #endif
 
+static inline void pt_load_msr(struct pt_ctx *ctx, unsigned int addr_num)
+{
+	u32 i;
+
+	wrmsrl(MSR_IA32_RTIT_STATUS, ctx->status);
+	wrmsrl(MSR_IA32_RTIT_OUTPUT_BASE, ctx->output_base);
+	wrmsrl(MSR_IA32_RTIT_OUTPUT_MASK, ctx->output_mask);
+	wrmsrl(MSR_IA32_RTIT_CR3_MATCH, ctx->cr3_match);
+	for (i = 0; i < addr_num; i++)
+		wrmsrl(MSR_IA32_RTIT_ADDR0_A + i, ctx->addrs[i]);
+}
+
+static inline void pt_save_msr(struct pt_ctx *ctx, unsigned int addr_num)
+{
+	u32 i;
+
+	rdmsrl(MSR_IA32_RTIT_STATUS, ctx->status);
+	rdmsrl(MSR_IA32_RTIT_OUTPUT_BASE, ctx->output_base);
+	rdmsrl(MSR_IA32_RTIT_OUTPUT_MASK, ctx->output_mask);
+	rdmsrl(MSR_IA32_RTIT_CR3_MATCH, ctx->cr3_match);
+	for (i = 0; i < addr_num; i++)
+		rdmsrl(MSR_IA32_RTIT_ADDR0_A + i, ctx->addrs[i]);
+}
+
+static void pt_guest_enter(struct vcpu_vmx *vmx)
+{
+	u64 ctl;
+
+	if (pt_mode == PT_MODE_HOST || pt_mode == PT_MODE_HOST_GUEST) {
+		rdmsrl(MSR_IA32_RTIT_CTL, ctl);
+		vmx->pt_desc.host.ctl = ctl;
+		if (ctl & RTIT_CTL_TRACEEN) {
+			ctl &= ~RTIT_CTL_TRACEEN;
+			wrmsrl(MSR_IA32_RTIT_CTL, ctl);
+		}
+	}
+
+	if (pt_mode == PT_MODE_HOST_GUEST) {
+		pt_save_msr(&vmx->pt_desc.host, vmx->pt_desc.addr_num);
+		pt_load_msr(&vmx->pt_desc.guest, vmx->pt_desc.addr_num);
+	}
+}
+
+static void pt_guest_exit(struct vcpu_vmx *vmx)
+{
+	if (pt_mode == PT_MODE_HOST_GUEST) {
+		pt_save_msr(&vmx->pt_desc.guest, vmx->pt_desc.addr_num);
+		pt_load_msr(&vmx->pt_desc.host, vmx->pt_desc.addr_num);
+		wrmsrl(MSR_IA32_RTIT_CTL, vmx->pt_desc.host.ctl);
+	}
+
+	if (pt_mode == PT_MODE_HOST)
+		wrmsrl(MSR_IA32_RTIT_CTL, vmx->pt_desc.host.ctl);
+}
+
 static void vmx_save_host_state(struct kvm_vcpu *vcpu)
 {
 	struct vcpu_vmx *vmx = to_vmx(vcpu);
@@ -5618,6 +5673,13 @@ static int vmx_vcpu_setup(struct vcpu_vmx *vmx)
 		vmcs_write16(GUEST_PML_INDEX, PML_ENTITY_NUM - 1);
 	}
 
+	if (pt_mode == PT_MODE_HOST_GUEST) {
+		u32 eax, ebx, ecx, edx;
+
+		cpuid_count(0x14, 1, &eax, &ebx, &ecx, &edx);
+		vmx->pt_desc.addr_num = eax & 0x7;
+	}
+
 	return 0;
 }
 
@@ -5712,6 +5774,15 @@ static void vmx_vcpu_reset(struct kvm_vcpu *vcpu, bool init_event)
 	update_exception_bitmap(vcpu);
 
 	vpid_sync_context(vmx->vpid);
+
+	if (pt_mode == PT_MODE_HOST_GUEST) {
+		memset(&vmx->pt_desc.host, 0, sizeof(vmx->pt_desc.host));
+		memset(&vmx->pt_desc.guest, 0, sizeof(vmx->pt_desc.guest));
+		/* Bit[6~0] are forced to 1, writes are ignored. */
+		vmx->pt_desc.guest.output_mask = 0x7F;
+		/* Clear the status of IA32_RTIT_CTL in VMCS guest state. */
+		vmcs_write32(GUEST_IA32_RTIT_CTL, 0);
+	}
 }
 
 /*
@@ -9382,6 +9453,8 @@ static void __noclone vmx_vcpu_run(struct kvm_vcpu *vcpu)
 	    vcpu->arch.pkru != vmx->host_pkru)
 		__write_pkru(vcpu->arch.pkru);
 
+	pt_guest_enter(vmx);
+
 	atomic_switch_perf_msrs(vmx);
 	debugctlmsr = get_debugctlmsr();
 
@@ -9517,6 +9590,8 @@ static void __noclone vmx_vcpu_run(struct kvm_vcpu *vcpu)
 				  | (1 << VCPU_EXREG_CR3));
 	vcpu->arch.regs_dirty = 0;
 
+	pt_guest_exit(vmx);
+
 	/*
 	 * eager fpu is enabled if PKEY is supported and CR4 is switched
 	 * back on host, so it is safe to read guest PKRU from current
-- 
1.8.3.1

[patch v2 7/8] KVM: x86: add Intel PT msr RTIT_CTL read/write

[Luwei Kang]

From: Chao Peng <chao.p.peng@linux.intel.com>

L1 hypervisor can't get the capability of "TraceEn can be
set in VMX operation"(IA32_VMX_MISC[14]=0) and any attempt
to set IA32_RTIT_CTL.TraceEn in VMX operation using WRMSR
will cause a general-protection exception if IA32_VMX_MISC[14]
is 0. So we need to leave out write this msr in L1 VMX
operation.

Signed-off-by: Chao Peng <chao.p.peng@linux.intel.com>
Signed-off-by: Luwei Kang <luwei.kang@intel.com>
---
 arch/x86/kvm/vmx.c | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
index 091120e..8f61a8d 100644
--- a/arch/x86/kvm/vmx.c
+++ b/arch/x86/kvm/vmx.c
@@ -936,6 +936,7 @@ static void vmx_get_segment(struct kvm_vcpu *vcpu,
 static void vmx_set_nmi_mask(struct kvm_vcpu *vcpu, bool masked);
 static bool nested_vmx_is_page_fault_vmexit(struct vmcs12 *vmcs12,
 					    u16 error_code);
+static bool vmx_pt_supported(void);
 
 static DEFINE_PER_CPU(struct vmcs *, vmxarea);
 static DEFINE_PER_CPU(struct vmcs *, current_vmcs);
@@ -3384,6 +3385,11 @@ static int vmx_get_msr(struct kvm_vcpu *vcpu, struct msr_data *msr_info)
 			return 1;
 		msr_info->data = vcpu->arch.ia32_xss;
 		break;
+	case MSR_IA32_RTIT_CTL:
+		if (!vmx_pt_supported())
+			return 1;
+		msr_info->data = vmcs_read64(GUEST_IA32_RTIT_CTL);
+		break;
 	case MSR_TSC_AUX:
 		if (!msr_info->host_initiated &&
 		    !guest_cpuid_has(vcpu, X86_FEATURE_RDTSCP))
@@ -3508,6 +3514,11 @@ static int vmx_set_msr(struct kvm_vcpu *vcpu, struct msr_data *msr_info)
 		else
 			clear_atomic_switch_msr(vmx, MSR_IA32_XSS);
 		break;
+	case MSR_IA32_RTIT_CTL:
+		if (!vmx_pt_supported() || to_vmx(vcpu)->nested.vmxon)
+			return 1;
+		vmcs_write64(GUEST_IA32_RTIT_CTL, data);
+		break;
 	case MSR_TSC_AUX:
 		if (!msr_info->host_initiated &&
 		    !guest_cpuid_has(vcpu, X86_FEATURE_RDTSCP))
-- 
1.8.3.1

[patch v2 8/8] KVM: x86: Disable intercept for Intel processor trace MSRs

[Luwei Kang]

From: Chao Peng <chao.p.peng@linux.intel.com>

Pass through Intel processor trace to guest directly.

Signed-off-by: Chao Peng <chao.p.peng@linux.intel.com>
Signed-off-by: Luwei Kang <luwei.kang@intel.com>
---
 arch/x86/kvm/vmx.c | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
index 8f61a8d..01447e2 100644
--- a/arch/x86/kvm/vmx.c
+++ b/arch/x86/kvm/vmx.c
@@ -7029,6 +7029,19 @@ static __init int hardware_setup(void)
 		!cpu_has_vmx_intel_pt() || !cpu_has_vmx_pt_use_gpa())
 		pt_mode = PT_MODE_SYSTEM;
 
+	if (pt_mode == PT_MODE_HOST_GUEST) {
+		u32 i, eax, ebx, ecx, edx;
+
+		cpuid_count(0x14, 1, &eax, &ebx, &ecx, &edx);
+		vmx_disable_intercept_for_msr(MSR_IA32_RTIT_STATUS, false);
+		vmx_disable_intercept_for_msr(MSR_IA32_RTIT_OUTPUT_BASE, false);
+		vmx_disable_intercept_for_msr(MSR_IA32_RTIT_OUTPUT_MASK, false);
+		vmx_disable_intercept_for_msr(MSR_IA32_RTIT_CR3_MATCH, false);
+		for (i = 0; i < (eax & 0x7); i++)
+			vmx_disable_intercept_for_msr(MSR_IA32_RTIT_ADDR0_A + i,
+									false);
+	}
+
 	return alloc_kvm_area();
 
 out:
-- 
1.8.3.1

Re: [patch v2 0/8] Intel Processor Trace virtulization enabling

[Paolo Bonzini]

On 30/10/2017 23:05, Luwei Kang wrote:
> From v1:
>  - remove guest-only mode because guest-only mode can be covered by host-guest mode;
>  - always set "use GPA for processor tracing" in secondary execution control if it can be;
>  - trap RTIT_CTL read/write. Forbid write this msr when VMXON in L1 hypervisor.

Sounds fine.  However, it's likely that this will only be in Linux 4.16.

Paolo

RE: [patch v2 0/8] Intel Processor Trace virtulization enabling

[Kang, Luwei]

> > From v1:
> >  - remove guest-only mode because guest-only mode can be covered by host-guest mode;
> >  - always set "use GPA for processor tracing" in secondary execution control if it can be;
> >  - trap RTIT_CTL read/write. Forbid write this msr when VMXON in L1 hypervisor.
> 
> Sounds fine.  However, it's likely that this will only be in Linux 4.16.
> 
OK.  Hope can get more comments to these patches.  Thanks.

Luwei Kang

Re: [patch v2 4/8] KVM: x86: add Intel processor trace cpuid emulataion

[Paolo Bonzini]

On 30/10/2017 23:05, Luwei Kang wrote:
> diff --git a/arch/x86/kvm/cpuid.c b/arch/x86/kvm/cpuid.c
> index 0099e10..ef19a11 100644
> --- a/arch/x86/kvm/cpuid.c
> +++ b/arch/x86/kvm/cpuid.c
> @@ -70,6 +70,7 @@ u64 kvm_supported_xcr0(void)
>  /* These are scattered features in cpufeatures.h. */
>  #define KVM_CPUID_BIT_AVX512_4VNNIW     2
>  #define KVM_CPUID_BIT_AVX512_4FMAPS     3
> +#define KVM_CPUID_BIT_INTEL_PT		25

This is not necessary, because there is no need to place processor 
tracing in scattered features.  Can you replace this hunk, and the
KF usage below, with the following patch?

Thanks,

Paolo

------------ 8< -------------
From: Paolo Bonzini <pbonzini@redhat.com>
Subject: [PATCH] x86: cpufeature: move processor tracing out of scattered features

Processor tracing is already enumerated in word 9 (CPUID[7,0].EBX), so do not
duplicate it in the scattered features word.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 arch/x86/include/asm/cpufeatures.h | 3 ++-
 arch/x86/kernel/cpu/scattered.c    | 1 -
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/x86/include/asm/cpufeatures.h b/arch/x86/include/asm/cpufeatures.h
index 2519c6c801c9..839781e78763 100644
--- a/arch/x86/include/asm/cpufeatures.h
+++ b/arch/x86/include/asm/cpufeatures.h
@@ -199,7 +199,7 @@
 #define X86_FEATURE_SME		( 7*32+10) /* AMD Secure Memory Encryption */
 
 #define X86_FEATURE_INTEL_PPIN	( 7*32+14) /* Intel Processor Inventory Number */
-#define X86_FEATURE_INTEL_PT	( 7*32+15) /* Intel Processor Trace */
+
 #define X86_FEATURE_AVX512_4VNNIW (7*32+16) /* AVX-512 Neural Network Instructions */
 #define X86_FEATURE_AVX512_4FMAPS (7*32+17) /* AVX-512 Multiply Accumulation Single precision */
 
@@ -238,6 +238,7 @@
 #define X86_FEATURE_AVX512IFMA  ( 9*32+21) /* AVX-512 Integer Fused Multiply-Add instructions */
 #define X86_FEATURE_CLFLUSHOPT	( 9*32+23) /* CLFLUSHOPT instruction */
 #define X86_FEATURE_CLWB	( 9*32+24) /* CLWB instruction */
+#define X86_FEATURE_INTEL_PT	( 9*32+25) /* Intel Processor Trace */
 #define X86_FEATURE_AVX512PF	( 9*32+26) /* AVX-512 Prefetch */
 #define X86_FEATURE_AVX512ER	( 9*32+27) /* AVX-512 Exponential and Reciprocal */
 #define X86_FEATURE_AVX512CD	( 9*32+28) /* AVX-512 Conflict Detection */
diff --git a/arch/x86/kernel/cpu/scattered.c b/arch/x86/kernel/cpu/scattered.c
index 05459ad3db46..d0e69769abfd 100644
--- a/arch/x86/kernel/cpu/scattered.c
+++ b/arch/x86/kernel/cpu/scattered.c
@@ -21,7 +21,6 @@ struct cpuid_bit {
 static const struct cpuid_bit cpuid_bits[] = {
 	{ X86_FEATURE_APERFMPERF,       CPUID_ECX,  0, 0x00000006, 0 },
 	{ X86_FEATURE_EPB,		CPUID_ECX,  3, 0x00000006, 0 },
-	{ X86_FEATURE_INTEL_PT,		CPUID_EBX, 25, 0x00000007, 0 },
 	{ X86_FEATURE_AVX512_4VNNIW,    CPUID_EDX,  2, 0x00000007, 0 },
 	{ X86_FEATURE_AVX512_4FMAPS,    CPUID_EDX,  3, 0x00000007, 0 },
 	{ X86_FEATURE_CAT_L3,		CPUID_EBX,  1, 0x00000010, 0 },

>  #define KF(x) bit(KVM_CPUID_BIT_##x)
>  
>  int kvm_update_cpuid(struct kvm_vcpu *vcpu)
> @@ -327,6 +328,7 @@ static inline int __do_cpuid_ent(struct kvm_cpuid_entry2 *entry, u32 function,
>  	unsigned f_invpcid = kvm_x86_ops->invpcid_supported() ? F(INVPCID) : 0;
>  	unsigned f_mpx = kvm_mpx_supported() ? F(MPX) : 0;
>  	unsigned f_xsaves = kvm_x86_ops->xsaves_supported() ? F(XSAVES) : 0;
> +	unsigned f_intel_pt = kvm_x86_ops->pt_supported() ? KF(INTEL_PT) : 0;
>  
>  	/* cpuid 1.edx */
>  	const u32 kvm_cpuid_1_edx_x86_features =

Re: [patch v2 6/8] KVM: x86: Implement Intel processor trace context switch

[Paolo Bonzini]

On 30/10/2017 23:05, Luwei Kang wrote:
> +static void pt_guest_enter(struct vcpu_vmx *vmx)
> +{
> +	u64 ctl;
> +
> +	if (pt_mode == PT_MODE_HOST || pt_mode == PT_MODE_HOST_GUEST) {
> +		rdmsrl(MSR_IA32_RTIT_CTL, ctl);
> +		vmx->pt_desc.host.ctl = ctl;
> +		if (ctl & RTIT_CTL_TRACEEN) {
> +			ctl &= ~RTIT_CTL_TRACEEN;
> +			wrmsrl(MSR_IA32_RTIT_CTL, ctl);
> +		}

This "if" is only needed for PT_MODE_HOST_GUEST, I believe.
PT_MODE_HOST can just use the "load RTIT_CTL" vmentry control to disable
tracing.

> +	}
> +
> +	if (pt_mode == PT_MODE_HOST_GUEST) {
> +		pt_save_msr(&vmx->pt_desc.host, vmx->pt_desc.addr_num);
> +		pt_load_msr(&vmx->pt_desc.guest, vmx->pt_desc.addr_num);
> +	}
> +}
> +
> +static void pt_guest_exit(struct vcpu_vmx *vmx)
> +{
> +	if (pt_mode == PT_MODE_HOST_GUEST) {
> +		pt_save_msr(&vmx->pt_desc.guest, vmx->pt_desc.addr_num);
> +		pt_load_msr(&vmx->pt_desc.host, vmx->pt_desc.addr_num);
> +		wrmsrl(MSR_IA32_RTIT_CTL, vmx->pt_desc.host.ctl);
> +	}
> +
> +	if (pt_mode == PT_MODE_HOST)
> +		wrmsrl(MSR_IA32_RTIT_CTL, vmx->pt_desc.host.ctl);
> +}


Please use an

	if (pt_mode == PT_MODE_HOST || pt_mode == PT_MODE_HOST_GUEST)

for the write to RTIT_CTL, so that pt_guest_exit mirrors pt_guest_entry.

Also, we don't actually need to write the MSR if RTIT_CTL_TRACEEN is
false.  With these changes, the cost of the "host-only" mode is
acceptable, but for host-guest mode it is very expensive to read and
write the MSRs on all vmentries and vmexits is very expensive.

It would be much better to avoid writing the guest state if the guest
RTIT_CTL has TRACEEN=0.  This would require keeping the intercepts until
TRACEEN=1, but a lot of the work would be needed anyway---see my review
of patch 7.

Thanks,

Paolo

Re: [patch v2 7/8] KVM: x86: add Intel PT msr RTIT_CTL read/write

[Paolo Bonzini]

On 30/10/2017 23:05, Luwei Kang wrote:
> From: Chao Peng <chao.p.peng@linux.intel.com>
> 
> L1 hypervisor can't get the capability of "TraceEn can be
> set in VMX operation"(IA32_VMX_MISC[14]=0) and any attempt
> to set IA32_RTIT_CTL.TraceEn in VMX operation using WRMSR
> will cause a general-protection exception if IA32_VMX_MISC[14]
> is 0. So we need to leave out write this msr in L1 VMX
> operation.
> 
> Signed-off-by: Chao Peng <chao.p.peng@linux.intel.com>
> Signed-off-by: Luwei Kang <luwei.kang@intel.com>
> ---
>  arch/x86/kvm/vmx.c | 11 +++++++++++
>  1 file changed, 11 insertions(+)
> 
> diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
> index 091120e..8f61a8d 100644
> --- a/arch/x86/kvm/vmx.c
> +++ b/arch/x86/kvm/vmx.c
> @@ -936,6 +936,7 @@ static void vmx_get_segment(struct kvm_vcpu *vcpu,
>  static void vmx_set_nmi_mask(struct kvm_vcpu *vcpu, bool masked);
>  static bool nested_vmx_is_page_fault_vmexit(struct vmcs12 *vmcs12,
>  					    u16 error_code);
> +static bool vmx_pt_supported(void);
>  
>  static DEFINE_PER_CPU(struct vmcs *, vmxarea);
>  static DEFINE_PER_CPU(struct vmcs *, current_vmcs);
> @@ -3384,6 +3385,11 @@ static int vmx_get_msr(struct kvm_vcpu *vcpu, struct msr_data *msr_info)
>  			return 1;
>  		msr_info->data = vcpu->arch.ia32_xss;
>  		break;
> +	case MSR_IA32_RTIT_CTL:
> +		if (!vmx_pt_supported())
> +			return 1;
> +		msr_info->data = vmcs_read64(GUEST_IA32_RTIT_CTL);
> +		break;
>  	case MSR_TSC_AUX:
>  		if (!msr_info->host_initiated &&
>  		    !guest_cpuid_has(vcpu, X86_FEATURE_RDTSCP))
> @@ -3508,6 +3514,11 @@ static int vmx_set_msr(struct kvm_vcpu *vcpu, struct msr_data *msr_info)
>  		else
>  			clear_atomic_switch_msr(vmx, MSR_IA32_XSS);
>  		break;
> +	case MSR_IA32_RTIT_CTL:
> +		if (!vmx_pt_supported() || to_vmx(vcpu)->nested.vmxon)
> +			return 1;

VMXON must also clear TraceEn bit (see 23.4 in the SDM).

You also need to support R/W of all the other MSRs, in order to make
them accessible to userspace, and add them in msrs_to_save and
kvm_init_msr_list.

Regarding the save/restore of the state, I am now wondering if you could
also use XSAVES and XRSTORS instead of multiple rdmsr/wrmsr in a loop.
The cost is two MSR writes on vmenter (because the guest must run with
IA32_XSS=0) and two on vmexit.

Thanks,

Paolo

> +		vmcs_write64(GUEST_IA32_RTIT_CTL, data);
> +		break;
>  	case MSR_TSC_AUX:
>  		if (!msr_info->host_initiated &&
>  		    !guest_cpuid_has(vcpu, X86_FEATURE_RDTSCP))
> 

Re: [patch v2 8/8] KVM: x86: Disable intercept for Intel processor trace MSRs

[Paolo Bonzini]

On 30/10/2017 23:05, Luwei Kang wrote:
> From: Chao Peng <chao.p.peng@linux.intel.com>
> 
> Pass through Intel processor trace to guest directly.
> 
> Signed-off-by: Chao Peng <chao.p.peng@linux.intel.com>
> Signed-off-by: Luwei Kang <luwei.kang@intel.com>
> ---
>  arch/x86/kvm/vmx.c | 13 +++++++++++++
>  1 file changed, 13 insertions(+)
> 
> diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
> index 8f61a8d..01447e2 100644
> --- a/arch/x86/kvm/vmx.c
> +++ b/arch/x86/kvm/vmx.c
> @@ -7029,6 +7029,19 @@ static __init int hardware_setup(void)
>  		!cpu_has_vmx_intel_pt() || !cpu_has_vmx_pt_use_gpa())
>  		pt_mode = PT_MODE_SYSTEM;
>  
> +	if (pt_mode == PT_MODE_HOST_GUEST) {
> +		u32 i, eax, ebx, ecx, edx;
> +
> +		cpuid_count(0x14, 1, &eax, &ebx, &ecx, &edx);
> +		vmx_disable_intercept_for_msr(MSR_IA32_RTIT_STATUS, false);
> +		vmx_disable_intercept_for_msr(MSR_IA32_RTIT_OUTPUT_BASE, false);
> +		vmx_disable_intercept_for_msr(MSR_IA32_RTIT_OUTPUT_MASK, false);
> +		vmx_disable_intercept_for_msr(MSR_IA32_RTIT_CR3_MATCH, false);
> +		for (i = 0; i < (eax & 0x7); i++)
> +			vmx_disable_intercept_for_msr(MSR_IA32_RTIT_ADDR0_A + i,
> +									false);
> +	}
> +

As I mentioned earlier, this probably makes vmentry/vmexit too expensive
when guests are not using processor tracing.  I would do it only if
guest TRACEEN=1 (since anyway the values have to be correct if guest
TRACEEN=1, and a change in TRACEEN always causes a vmexit).

Paolo

>  	return alloc_kvm_area();
>  
>  out:
> 

Re: [patch v2 3/8] KVM: x86: add Intel processor trace virtualization mode

[Paolo Bonzini]

On 30/10/2017 23:05, Luwei Kang wrote:
> +#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;
	}

Thanks,

Paolo

Re: [patch v2 3/8] KVM: x86: add Intel processor trace virtualization mode

[Paolo Bonzini]

On 30/10/2017 23:05, Luwei Kang wrote:
> +	if (!(_cpu_based_2nd_exec_control & SECONDARY_EXEC_PT_USE_GPA) ||
> +		!(_vmexit_control & VM_EXIT_CLEAR_IA32_RTIT_CTL) ||
> +		!(_vmentry_control & VM_ENTRY_LOAD_IA32_RTIT_CTL)) {
> +		_cpu_based_2nd_exec_control &= ~SECONDARY_EXEC_PT_USE_GPA;

Also, you are not checking anywhere if the SUPPRESS_PIP controls are
available.  This is probably the best place.

> +		_vmexit_control &= ~VM_EXIT_CLEAR_IA32_RTIT_CTL;
> +		_vmentry_control &= ~VM_ENTRY_LOAD_IA32_RTIT_CTL;

These two are not needed; disabling SECONDARY_EXEC_PT_USE_GPA is enough.
The tracing mode will revert to PT_SYSTEM, which does not use the
load/clear RTIT_CTL controls.

Paolo

> +	}
> +

RE: [patch v2 4/8] KVM: x86: add Intel processor trace cpuid emulataion

[Kang, Luwei]

> > diff --git a/arch/x86/kvm/cpuid.c b/arch/x86/kvm/cpuid.c index
> > 0099e10..ef19a11 100644
> > --- a/arch/x86/kvm/cpuid.c
> > +++ b/arch/x86/kvm/cpuid.c
> > @@ -70,6 +70,7 @@ u64 kvm_supported_xcr0(void)
> >  /* These are scattered features in cpufeatures.h. */
> >  #define KVM_CPUID_BIT_AVX512_4VNNIW     2
> >  #define KVM_CPUID_BIT_AVX512_4FMAPS     3
> > +#define KVM_CPUID_BIT_INTEL_PT		25
> 
> This is not necessary, because there is no need to place processor tracing in scattered features.  Can you replace this hunk, and the KF usage below, with the following patch?
> 

Yes, this looks good to me. will fix in next version. 

Thanks,
Luwei Kang

> ------------ 8< -------------
> From: Paolo Bonzini <pbonzini@redhat.com>
> Subject: [PATCH] x86: cpufeature: move processor tracing out of scattered features
> 
> Processor tracing is already enumerated in word 9 (CPUID[7,0].EBX), so do not duplicate it in the scattered features word.
> 
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>  arch/x86/include/asm/cpufeatures.h | 3 ++-
>  arch/x86/kernel/cpu/scattered.c    | 1 -
>  2 files changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/x86/include/asm/cpufeatures.h b/arch/x86/include/asm/cpufeatures.h
> index 2519c6c801c9..839781e78763 100644
> --- a/arch/x86/include/asm/cpufeatures.h
> +++ b/arch/x86/include/asm/cpufeatures.h
> @@ -199,7 +199,7 @@
>  #define X86_FEATURE_SME		( 7*32+10) /* AMD Secure Memory Encryption */
> 
>  #define X86_FEATURE_INTEL_PPIN	( 7*32+14) /* Intel Processor Inventory Number */
> -#define X86_FEATURE_INTEL_PT	( 7*32+15) /* Intel Processor Trace */
> +
>  #define X86_FEATURE_AVX512_4VNNIW (7*32+16) /* AVX-512 Neural Network Instructions */  #define
> X86_FEATURE_AVX512_4FMAPS (7*32+17) /* AVX-512 Multiply Accumulation Single precision */
> 
> @@ -238,6 +238,7 @@
>  #define X86_FEATURE_AVX512IFMA  ( 9*32+21) /* AVX-512 Integer Fused Multiply-Add instructions */
>  #define X86_FEATURE_CLFLUSHOPT	( 9*32+23) /* CLFLUSHOPT instruction */
>  #define X86_FEATURE_CLWB	( 9*32+24) /* CLWB instruction */
> +#define X86_FEATURE_INTEL_PT	( 9*32+25) /* Intel Processor Trace */
>  #define X86_FEATURE_AVX512PF	( 9*32+26) /* AVX-512 Prefetch */
>  #define X86_FEATURE_AVX512ER	( 9*32+27) /* AVX-512 Exponential and Reciprocal */
>  #define X86_FEATURE_AVX512CD	( 9*32+28) /* AVX-512 Conflict Detection */
> diff --git a/arch/x86/kernel/cpu/scattered.c b/arch/x86/kernel/cpu/scattered.c index 05459ad3db46..d0e69769abfd 100644
> --- a/arch/x86/kernel/cpu/scattered.c
> +++ b/arch/x86/kernel/cpu/scattered.c
> @@ -21,7 +21,6 @@ struct cpuid_bit {
>  static const struct cpuid_bit cpuid_bits[] = {
>  	{ X86_FEATURE_APERFMPERF,       CPUID_ECX,  0, 0x00000006, 0 },
>  	{ X86_FEATURE_EPB,		CPUID_ECX,  3, 0x00000006, 0 },
> -	{ X86_FEATURE_INTEL_PT,		CPUID_EBX, 25, 0x00000007, 0 },
>  	{ X86_FEATURE_AVX512_4VNNIW,    CPUID_EDX,  2, 0x00000007, 0 },
>  	{ X86_FEATURE_AVX512_4FMAPS,    CPUID_EDX,  3, 0x00000007, 0 },
>  	{ X86_FEATURE_CAT_L3,		CPUID_EBX,  1, 0x00000010, 0 },
> 
> >  #define KF(x) bit(KVM_CPUID_BIT_##x)
> >
> >  int kvm_update_cpuid(struct kvm_vcpu *vcpu) @@ -327,6 +328,7 @@
> > static inline int __do_cpuid_ent(struct kvm_cpuid_entry2 *entry, u32 function,
> >  	unsigned f_invpcid = kvm_x86_ops->invpcid_supported() ? F(INVPCID) : 0;
> >  	unsigned f_mpx = kvm_mpx_supported() ? F(MPX) : 0;
> >  	unsigned f_xsaves = kvm_x86_ops->xsaves_supported() ? F(XSAVES) : 0;
> > +	unsigned f_intel_pt = kvm_x86_ops->pt_supported() ? KF(INTEL_PT) :
> > +0;
> >
> >  	/* cpuid 1.edx */
> >  	const u32 kvm_cpuid_1_edx_x86_features =

RE: [patch v2 7/8] KVM: x86: add Intel PT msr RTIT_CTL read/write

[Kang, Luwei]

> >  static DEFINE_PER_CPU(struct vmcs *, vmxarea);  static
> > DEFINE_PER_CPU(struct vmcs *, current_vmcs); @@ -3384,6 +3385,11 @@
> > static int vmx_get_msr(struct kvm_vcpu *vcpu, struct msr_data *msr_info)
> >  			return 1;
> >  		msr_info->data = vcpu->arch.ia32_xss;
> >  		break;
> > +	case MSR_IA32_RTIT_CTL:
> > +		if (!vmx_pt_supported())
> > +			return 1;
> > +		msr_info->data = vmcs_read64(GUEST_IA32_RTIT_CTL);
> > +		break;
> >  	case MSR_TSC_AUX:
> >  		if (!msr_info->host_initiated &&
> >  		    !guest_cpuid_has(vcpu, X86_FEATURE_RDTSCP)) @@ -3508,6 +3514,11
> > @@ static int vmx_set_msr(struct kvm_vcpu *vcpu, struct msr_data *msr_info)
> >  		else
> >  			clear_atomic_switch_msr(vmx, MSR_IA32_XSS);
> >  		break;
> > +	case MSR_IA32_RTIT_CTL:
> > +		if (!vmx_pt_supported() || to_vmx(vcpu)->nested.vmxon)
> > +			return 1;
> 
> VMXON must also clear TraceEn bit (see 23.4 in the SDM).

Will clear TraceEn bit in handle_vmon() function.

> 
> You also need to support R/W of all the other MSRs, in order to make them accessible to userspace, and add them in msrs_to_save and kvm_init_msr_list.
> 

Will add it in next version. This is use for live migration, is that right?

> Regarding the save/restore of the state, I am now wondering if you could also use XSAVES and XRSTORS instead of multiple rdmsr/wrmsr in a loop.
> The cost is two MSR writes on vmenter (because the guest must run with IA32_XSS=0) and two on vmexit.
> 

If use XSAVES and XRSTORS for context switch.
1. Before  kvm_load_guest_fpu(vcpu), we need to save host RTIT_CTL, disable PT and restore the value of  "vmx->pt_desc.guest.ctl" to GUEST_IA32_RTIT_CTL. Is that right?
2. After VM-exit (step out from kvm_x86_ops->run(vcpu)),  we need to save the status of GUEST_IA32_RTIT_CTL . TRACEEN=0 and others MSRs are still in guest status. Where to enable PT if in host-guest mode. I think we should enable PT after vm-exit but it may cause #GP. " If XRSTORS would restore (or initialize) PT state and IA32_RTIT_CTL.TraceEn = 1, the instruction causes a general-protection exception. SDM 13.5.6". if enable after kvm_put_guest_fpu() I think it too late.)

Thanks,
Luwei Kang
> 
> > +		vmcs_write64(GUEST_IA32_RTIT_CTL, data);
> > +		break;
> >  	case MSR_TSC_AUX:
> >  		if (!msr_info->host_initiated &&
> >  		    !guest_cpuid_has(vcpu, X86_FEATURE_RDTSCP))
> >

RE: [patch v2 8/8] KVM: x86: Disable intercept for Intel processor trace MSRs

[Kang, Luwei]

> > +	if (pt_mode == PT_MODE_HOST_GUEST) {
> > +		u32 i, eax, ebx, ecx, edx;
> > +
> > +		cpuid_count(0x14, 1, &eax, &ebx, &ecx, &edx);
> > +		vmx_disable_intercept_for_msr(MSR_IA32_RTIT_STATUS, false);
> > +		vmx_disable_intercept_for_msr(MSR_IA32_RTIT_OUTPUT_BASE, false);
> > +		vmx_disable_intercept_for_msr(MSR_IA32_RTIT_OUTPUT_MASK, false);
> > +		vmx_disable_intercept_for_msr(MSR_IA32_RTIT_CR3_MATCH, false);
> > +		for (i = 0; i < (eax & 0x7); i++)
> > +			vmx_disable_intercept_for_msr(MSR_IA32_RTIT_ADDR0_A + i,
> > +									false);
> > +	}
> > +
> 
> As I mentioned earlier, this probably makes vmentry/vmexit too expensive when guests are not using processor tracing.  I would do  it only if guest TRACEEN=1 (since anyway the values have to be correct if guest TRACEEN=1, and a change in TRACEEN always causes a vmexit).
> 

Will change in next version.

Thanks,
Luwei Kang

> 
> >  	return alloc_kvm_area();
> >
> >  out:
> >

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

[Kang, Luwei]

> > +#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

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

[Kang, Luwei]

> > +	if (!(_cpu_based_2nd_exec_control & SECONDARY_EXEC_PT_USE_GPA) ||
> > +		!(_vmexit_control & VM_EXIT_CLEAR_IA32_RTIT_CTL) ||
> > +		!(_vmentry_control & VM_ENTRY_LOAD_IA32_RTIT_CTL)) {
> > +		_cpu_based_2nd_exec_control &= ~SECONDARY_EXEC_PT_USE_GPA;
> 
> Also, you are not checking anywhere if the SUPPRESS_PIP controls are available.  This is probably the best place.

SUPPRESS_PIP(should be "CONCEAL", will fix it.) is use for control of  processor trace packet. 
I think we should clear it when in SYSTEM mode (For example, PIPs are generated on VM exit, with NonRoot=0. On VM exit to SMM, VMCS packets are additionally generated). Why need check this here?

> 
> > +		_vmexit_control &= ~VM_EXIT_CLEAR_IA32_RTIT_CTL;
> > +		_vmentry_control &= ~VM_ENTRY_LOAD_IA32_RTIT_CTL;
> 
> These two are not needed; disabling SECONDARY_EXEC_PT_USE_GPA is enough.
> The tracing mode will revert to PT_SYSTEM, which does not use the load/clear RTIT_CTL controls.
> 

The status of *_RTIT_CTL should be same with SECONDARY_EXEC_PT_USE_GPA or would cause VM-entry failed. (architecture-instruction-set-extensions-programming-reference  5.2.3)

Re: [patch v2 7/8] KVM: x86: add Intel PT msr RTIT_CTL read/write

[Paolo Bonzini]

On 14/11/2017 07:59, Kang, Luwei wrote:
> Will add it in next version. This is use for live migration, is that right?

Yes.

> If use XSAVES and XRSTORS for context switch.
> 1. Before  kvm_load_guest_fpu(vcpu), we need to save host RTIT_CTL, disable PT and restore the value of  "vmx->pt_desc.guest.ctl" to GUEST_IA32_RTIT_CTL. Is that right?

The idea is to make the MSR get/set handlers operate on an XSAVES state
separate from the guest FPU state.  RTIT_CTL in the XSAVES state is
special cased to always be zero.

Then you could do

  if (!system mode) {
    if (guest RTIT_CTL.TraceEn != 0) {
      set PT in IA32_XSS
      XSAVES the host PT state
      // RTIT_CTL.TraceEn is now zero, and remains zero after XRSTORS
      XRSTORS the guest PT state
      clear PT in IA32_XSS
    } else {
      save host RTIT_CTL
    }
    // guest RTIT_CTL.TraceEn will be loaded by vmentry
  }

on vmentry, and

  if (!system mode) {
    // RTIT_CTL.TraceEn is zero here
    if (guest RTIT_CTL.TraceEn != 0) {
      set PT in IA32_XSS
      // no need to XSAVES guest state, all MSR writes cause a vmexit
      XRSTORS the host PT state
      clear PT in IA32_XSS
    } else if (host RTIT_CTL.TraceEn != 0) {
      restore host RTIT_CTL
    }
  }

on vmexit.  It should still be cheaper than many rdmsr/wrmsr operations.

Paolo

> 2. After VM-exit (step out from kvm_x86_ops->run(vcpu)), we need to
> save the status of GUEST_IA32_RTIT_CTL . TRACEEN=0 and others MSRs are
> still in guest status. Where to enable PT if in host-guest mode. I think
> we should enable PT after vm-exit but it may cause #GP. " If XRSTORS
> would restore (or initialize) PT state and IA32_RTIT_CTL.TraceEn = 1,
> the instruction causes a general-protection exception. SDM 13.5.6". if
> enable after kvm_put_guest_fpu() I think it too late.)

Re: [patch v2 3/8] KVM: x86: add Intel processor trace virtualization mode

[Paolo Bonzini]

On 14/11/2017 08:31, Kang, Luwei wrote:
>>> +	if (!(_cpu_based_2nd_exec_control & SECONDARY_EXEC_PT_USE_GPA) ||
>>> +		!(_vmexit_control & VM_EXIT_CLEAR_IA32_RTIT_CTL) ||
>>> +		!(_vmentry_control & VM_ENTRY_LOAD_IA32_RTIT_CTL)) {
>>> +		_cpu_based_2nd_exec_control &= ~SECONDARY_EXEC_PT_USE_GPA;
>>
>> Also, you are not checking anywhere if the SUPPRESS_PIP controls are available.  This is probably the best place.
> 
> SUPPRESS_PIP(should be "CONCEAL", will fix it.) is use for control of  processor trace packet. 
> I think we should clear it when in SYSTEM mode (For example, PIPs are
> generated on VM exit, with NonRoot=0. On VM exit to SMM, VMCS packets
> are additionally generated). Why need check this here?

I agree.  But if SUPPRESS_PIP/CONCEAL is not available, you cannot
enable HOST_GUEST mode I think?

>>
>>> +		_vmexit_control &= ~VM_EXIT_CLEAR_IA32_RTIT_CTL;
>>> +		_vmentry_control &= ~VM_ENTRY_LOAD_IA32_RTIT_CTL;
>>
>> These two are not needed; disabling SECONDARY_EXEC_PT_USE_GPA is enough.
>> The tracing mode will revert to PT_SYSTEM, which does not use the load/clear RTIT_CTL controls.
> 
> The status of *_RTIT_CTL should be same with SECONDARY_EXEC_PT_USE_GPA
> or would cause VM-entry failed.
> (architecture-instruction-set-extensions-programming-reference 5.2.3)

Indeed:

- disable "use GPA" here

- hardware_setup notices !cpu_has_vmx_pt_use_gpa() and switches tracing
mode to system

- vmx_vcpu_setup (via vmx_vmentry_control and vmx_vmexit_control) turns
off the IA32_RIT_CTL vmentry/vmexit controls (and also turns off
SUPPRESS_PIP)

- everybody is happy

Paolo