All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v7 00/15] Introduce Architectural LBR for vPMU
@ 2021-08-06  7:42 Yang Weijiang
  2021-08-06  7:42 ` [PATCH v7 01/15] perf/x86/intel: Fix the comment about guest LBR support on KVM Yang Weijiang
                   ` (14 more replies)
  0 siblings, 15 replies; 28+ messages in thread
From: Yang Weijiang @ 2021-08-06  7:42 UTC (permalink / raw)
  To: pbonzini, jmattson, seanjc, vkuznets, wei.w.wang, like.xu.linux,
	kvm, linux-kernel
  Cc: root

From: root <root@984fee009f2b.jf.intel.com>

The Architectural Last Branch Records (LBRs) is published in the 319433-040
release of Intel Architecture Instruction Set Extensions and Future Features
Programming Reference[0].

The main advantages of Arch LBR are [1]:
- Faster context switching due to XSAVES support and faster reset of
  LBR MSRs via the new DEPTH MSR
- Faster LBR read for a non-PEBS event due to XSAVES support, which
  lowers the overhead of the NMI handler.
- Linux kernel can support the LBR features without knowing the model
  number of the current CPU.

From end user's point of view, the usage of Arch LBR is the same as
the Legacy LBR that has been merged in the mainline.

Note, there's one limitations for current guest Arch LBR implementation:
Guest can only use the same LBR record depth as host, this is due to
the special behavior of MSR_ARCH_LBR_DEPTH: a) On write to the MSR,
it'll reset all Arch LBR recording MSRs to 0s. b) XRSTORS will reset all
recording MSRs to 0s if the saved depth mismatches MSR_ARCH_LBR_DEPTH.

But this limitation won't impact guest perf tool usage.

[0] https://software.intel.com/content/www/us/en/develop/download/intel-architecture-instruction-set-extensions-and-future-features-programming-reference.html
[1] https://lore.kernel.org/lkml/1593780569-62993-1-git-send-email-kan.liang@linux.intel.com/


Previous version:
v6: https://lkml.kernel.org/kvm/1626425406-18582-1-git-send-email-weijiang.yang@intel.com/

Changes in v7:
1. Added fix patch for nested VM entry failure issue, also fix warnings from reported from L1.
2. Added patches to flip LBREn bit upon guest state changes, e.g., on #SMI, #DB, warm reset.
3. Added kvm unit-test application for Arch LBR.
4. Rebased v6 patches to kernel v5.14-rc4.


Like Xu (6):
  perf/x86/intel: Fix the comment about guest LBR support on KVM
  perf/x86/lbr: Simplify the exposure check for the LBR_INFO registers
  KVM: vmx/pmu: Emulate MSR_ARCH_LBR_DEPTH for guest Arch LBR
  KVM: vmx/pmu: Emulate MSR_ARCH_LBR_CTL for guest Arch LBR
  KVM: x86: Refine the matching and clearing logic for supported_xss
  KVM: x86: Add XSAVE Support for Architectural LBR

Sean Christopherson (1):
  KVM: x86: Report XSS as an MSR to be saved if there are supported
    features

Yang Weijiang (8):
  KVM: x86: Add Arch LBR MSRs to msrs_to_save_all list
  KVM: x86/pmu: Refactor code to support guest Arch LBR
  KVM: x86: Refresh CPUID on writes to MSR_IA32_XSS
  KVM: x86/vmx: Check Arch LBR config when return perf capabilities
  KVM: nVMX: Add necessary Arch LBR settings for nested VM
  KVM: x86/vmx: Clear Arch LBREn bit before inject #DB to guest
  KVM: x86/vmx: Flip Arch LBREn bit on guest state change
  KVM: x86/cpuid: Advise Arch LBR feature in CPUID

 arch/x86/events/intel/core.c     |   3 +-
 arch/x86/events/intel/lbr.c      |   6 +-
 arch/x86/include/asm/kvm_host.h  |   1 +
 arch/x86/include/asm/msr-index.h |   1 +
 arch/x86/include/asm/vmx.h       |   4 +
 arch/x86/kvm/cpuid.c             |  54 +++++++++++++-
 arch/x86/kvm/vmx/capabilities.h  |  25 +++++--
 arch/x86/kvm/vmx/nested.c        |   6 +-
 arch/x86/kvm/vmx/pmu_intel.c     | 122 +++++++++++++++++++++++++++----
 arch/x86/kvm/vmx/vmcs12.c        |   1 +
 arch/x86/kvm/vmx/vmcs12.h        |   3 +-
 arch/x86/kvm/vmx/vmx.c           |  54 ++++++++++++--
 arch/x86/kvm/x86.c               |  24 +++++-
 13 files changed, 261 insertions(+), 43 deletions(-)

-- 
2.25.1


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

* [PATCH v7 01/15] perf/x86/intel: Fix the comment about guest LBR support on KVM
  2021-08-06  7:42 [PATCH v7 00/15] Introduce Architectural LBR for vPMU Yang Weijiang
@ 2021-08-06  7:42 ` Yang Weijiang
  2021-08-06  7:42 ` [PATCH v7 02/15] perf/x86/lbr: Simplify the exposure check for the LBR_INFO registers Yang Weijiang
                   ` (13 subsequent siblings)
  14 siblings, 0 replies; 28+ messages in thread
From: Yang Weijiang @ 2021-08-06  7:42 UTC (permalink / raw)
  To: pbonzini, jmattson, seanjc, vkuznets, wei.w.wang, like.xu.linux,
	kvm, linux-kernel
  Cc: Like Xu, Peter Zijlstra, Yang Weijiang

From: Like Xu <like.xu@linux.intel.com>

Starting from v5.12, KVM reports guest LBR and extra_regs support
when the host has relevant support. Just delete this part of the
comment and fix a typo incidentally.

Cc: Peter Zijlstra <peterz@infradead.org>
Reviewed-by: Kan Liang <kan.liang@linux.intel.com>
Reviewed-by: Andi Kleen <ak@linux.intel.com>
Signed-off-by: Like Xu <like.xu@linux.intel.com>
Signed-off-by: Yang Weijiang <weijiang.yang@intel.com>
---
 arch/x86/events/intel/core.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/arch/x86/events/intel/core.c b/arch/x86/events/intel/core.c
index fca7a6e2242f..a30b3ae09386 100644
--- a/arch/x86/events/intel/core.c
+++ b/arch/x86/events/intel/core.c
@@ -6264,8 +6264,7 @@ __init int intel_pmu_init(void)
 					  x86_pmu.intel_ctrl);
 	/*
 	 * Access LBR MSR may cause #GP under certain circumstances.
-	 * E.g. KVM doesn't support LBR MSR
-	 * Check all LBT MSR here.
+	 * Check all LBR MSR here.
 	 * Disable LBR access if any LBR MSRs can not be accessed.
 	 */
 	if (x86_pmu.lbr_tos && !check_msr(x86_pmu.lbr_tos, 0x3UL))
-- 
2.25.1


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

* [PATCH v7 02/15] perf/x86/lbr: Simplify the exposure check for the LBR_INFO registers
  2021-08-06  7:42 [PATCH v7 00/15] Introduce Architectural LBR for vPMU Yang Weijiang
  2021-08-06  7:42 ` [PATCH v7 01/15] perf/x86/intel: Fix the comment about guest LBR support on KVM Yang Weijiang
@ 2021-08-06  7:42 ` Yang Weijiang
  2021-08-06  7:42 ` [PATCH v7 03/15] KVM: x86: Add Arch LBR MSRs to msrs_to_save_all list Yang Weijiang
                   ` (12 subsequent siblings)
  14 siblings, 0 replies; 28+ messages in thread
From: Yang Weijiang @ 2021-08-06  7:42 UTC (permalink / raw)
  To: pbonzini, jmattson, seanjc, vkuznets, wei.w.wang, like.xu.linux,
	kvm, linux-kernel
  Cc: Like Xu, Peter Zijlstra, Yang Weijiang

From: Like Xu <like.xu@linux.intel.com>

The x86_pmu.lbr_info is 0 unless explicitly initialized, so there's
no point checking x86_pmu.intel_cap.lbr_format.

Cc: Peter Zijlstra <peterz@infradead.org>
Reviewed-by: Kan Liang <kan.liang@linux.intel.com>
Reviewed-by: Andi Kleen <ak@linux.intel.com>
Signed-off-by: Like Xu <like.xu@linux.intel.com>
Signed-off-by: Yang Weijiang <weijiang.yang@intel.com>
---
 arch/x86/events/intel/lbr.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/arch/x86/events/intel/lbr.c b/arch/x86/events/intel/lbr.c
index 9e6d6eaeb4cb..ceff16eb4bcb 100644
--- a/arch/x86/events/intel/lbr.c
+++ b/arch/x86/events/intel/lbr.c
@@ -1848,12 +1848,10 @@ void __init intel_pmu_arch_lbr_init(void)
  */
 int x86_perf_get_lbr(struct x86_pmu_lbr *lbr)
 {
-	int lbr_fmt = x86_pmu.intel_cap.lbr_format;
-
 	lbr->nr = x86_pmu.lbr_nr;
 	lbr->from = x86_pmu.lbr_from;
 	lbr->to = x86_pmu.lbr_to;
-	lbr->info = (lbr_fmt == LBR_FORMAT_INFO) ? x86_pmu.lbr_info : 0;
+	lbr->info = x86_pmu.lbr_info;
 
 	return 0;
 }
-- 
2.25.1


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

* [PATCH v7 03/15] KVM: x86: Add Arch LBR MSRs to msrs_to_save_all list
  2021-08-06  7:42 [PATCH v7 00/15] Introduce Architectural LBR for vPMU Yang Weijiang
  2021-08-06  7:42 ` [PATCH v7 01/15] perf/x86/intel: Fix the comment about guest LBR support on KVM Yang Weijiang
  2021-08-06  7:42 ` [PATCH v7 02/15] perf/x86/lbr: Simplify the exposure check for the LBR_INFO registers Yang Weijiang
@ 2021-08-06  7:42 ` Yang Weijiang
  2021-08-09 13:07   ` Like Xu
  2021-08-06  7:42 ` [PATCH v7 04/15] KVM: vmx/pmu: Emulate MSR_ARCH_LBR_DEPTH for guest Arch LBR Yang Weijiang
                   ` (11 subsequent siblings)
  14 siblings, 1 reply; 28+ messages in thread
From: Yang Weijiang @ 2021-08-06  7:42 UTC (permalink / raw)
  To: pbonzini, jmattson, seanjc, vkuznets, wei.w.wang, like.xu.linux,
	kvm, linux-kernel
  Cc: Yang Weijiang

Arch LBR MSR_ARCH_LBR_DEPTH and MSR_ARCH_LBR_CTL are {saved|restored}
by userspace application if they're available.

Suggested-by: Jim Mattson <jmattson@google.com>
Signed-off-by: Yang Weijiang <weijiang.yang@intel.com>
Reviewed-by: Jim Mattson <jmattson@google.com>
---
 arch/x86/kvm/x86.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index 4116567f3d44..4ae173ab2208 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -1327,6 +1327,7 @@ static const u32 msrs_to_save_all[] = {
 	MSR_ARCH_PERFMON_EVENTSEL0 + 12, MSR_ARCH_PERFMON_EVENTSEL0 + 13,
 	MSR_ARCH_PERFMON_EVENTSEL0 + 14, MSR_ARCH_PERFMON_EVENTSEL0 + 15,
 	MSR_ARCH_PERFMON_EVENTSEL0 + 16, MSR_ARCH_PERFMON_EVENTSEL0 + 17,
+	MSR_ARCH_LBR_CTL, MSR_ARCH_LBR_DEPTH,
 };
 
 static u32 msrs_to_save[ARRAY_SIZE(msrs_to_save_all)];
@@ -6229,6 +6230,11 @@ static void kvm_init_msr_list(void)
 			    min(INTEL_PMC_MAX_GENERIC, x86_pmu.num_counters_gp))
 				continue;
 			break;
+		case MSR_ARCH_LBR_DEPTH:
+		case MSR_ARCH_LBR_CTL:
+			if (!kvm_cpu_cap_has(X86_FEATURE_ARCH_LBR))
+				continue;
+			break;
 		default:
 			break;
 		}
-- 
2.25.1


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

* [PATCH v7 04/15] KVM: vmx/pmu: Emulate MSR_ARCH_LBR_DEPTH for guest Arch LBR
  2021-08-06  7:42 [PATCH v7 00/15] Introduce Architectural LBR for vPMU Yang Weijiang
                   ` (2 preceding siblings ...)
  2021-08-06  7:42 ` [PATCH v7 03/15] KVM: x86: Add Arch LBR MSRs to msrs_to_save_all list Yang Weijiang
@ 2021-08-06  7:42 ` Yang Weijiang
  2021-08-09 13:16   ` Like Xu
  2021-08-06  7:42 ` [PATCH v7 05/15] KVM: vmx/pmu: Emulate MSR_ARCH_LBR_CTL " Yang Weijiang
                   ` (10 subsequent siblings)
  14 siblings, 1 reply; 28+ messages in thread
From: Yang Weijiang @ 2021-08-06  7:42 UTC (permalink / raw)
  To: pbonzini, jmattson, seanjc, vkuznets, wei.w.wang, like.xu.linux,
	kvm, linux-kernel
  Cc: Like Xu, Yang Weijiang

From: Like Xu <like.xu@linux.intel.com>

The number of Arch LBR entries available is determined by the value
in host MSR_ARCH_LBR_DEPTH.DEPTH. The supported LBR depth values are
enumerated in CPUID.(EAX=01CH, ECX=0):EAX[7:0]. For each bit "n" set
in this field, the MSR_ARCH_LBR_DEPTH.DEPTH value of "8*(n+1)" is
supported.

On a guest write to MSR_ARCH_LBR_DEPTH, all LBR entries are reset to 0.
KVM writes guest requested value to the native ARCH_LBR_DEPTH MSR
(this is safe because the two values will be the same) when the Arch LBR
records MSRs are pass-through to the guest.

Signed-off-by: Like Xu <like.xu@linux.intel.com>
Signed-off-by: Yang Weijiang <weijiang.yang@intel.com>
---
 arch/x86/kvm/vmx/pmu_intel.c | 35 ++++++++++++++++++++++++++++++++++-
 1 file changed, 34 insertions(+), 1 deletion(-)

diff --git a/arch/x86/kvm/vmx/pmu_intel.c b/arch/x86/kvm/vmx/pmu_intel.c
index 9efc1a6b8693..a4ef5bbce186 100644
--- a/arch/x86/kvm/vmx/pmu_intel.c
+++ b/arch/x86/kvm/vmx/pmu_intel.c
@@ -211,7 +211,7 @@ static bool intel_pmu_is_valid_lbr_msr(struct kvm_vcpu *vcpu, u32 index)
 static bool intel_is_valid_msr(struct kvm_vcpu *vcpu, u32 msr)
 {
 	struct kvm_pmu *pmu = vcpu_to_pmu(vcpu);
-	int ret;
+	int ret = 0;
 
 	switch (msr) {
 	case MSR_CORE_PERF_FIXED_CTR_CTRL:
@@ -220,6 +220,10 @@ static bool intel_is_valid_msr(struct kvm_vcpu *vcpu, u32 msr)
 	case MSR_CORE_PERF_GLOBAL_OVF_CTRL:
 		ret = pmu->version > 1;
 		break;
+	case MSR_ARCH_LBR_DEPTH:
+		if (kvm_cpu_cap_has(X86_FEATURE_ARCH_LBR))
+			ret = guest_cpuid_has(vcpu, X86_FEATURE_ARCH_LBR);
+		break;
 	default:
 		ret = get_gp_pmc(pmu, msr, MSR_IA32_PERFCTR0) ||
 			get_gp_pmc(pmu, msr, MSR_P6_EVNTSEL0) ||
@@ -348,10 +352,28 @@ static bool intel_pmu_handle_lbr_msrs_access(struct kvm_vcpu *vcpu,
 	return true;
 }
 
+/*
+ * Check if the requested depth value the same as that of host.
+ * When guest/host depth are different, the handling would be tricky,
+ * so now only max depth is supported for both host and guest.
+ */
+static bool arch_lbr_depth_is_valid(struct kvm_vcpu *vcpu, u64 depth)
+{
+	unsigned int eax, ebx, ecx, edx;
+
+	if (!kvm_cpu_cap_has(X86_FEATURE_ARCH_LBR))
+		return false;
+
+	cpuid_count(0x1c, 0, &eax, &ebx, &ecx, &edx);
+
+	return (depth == fls(eax & 0xff) * 8);
+}
+
 static int intel_pmu_get_msr(struct kvm_vcpu *vcpu, struct msr_data *msr_info)
 {
 	struct kvm_pmu *pmu = vcpu_to_pmu(vcpu);
 	struct kvm_pmc *pmc;
+	struct lbr_desc *lbr_desc = vcpu_to_lbr_desc(vcpu);
 	u32 msr = msr_info->index;
 
 	switch (msr) {
@@ -367,6 +389,9 @@ static int intel_pmu_get_msr(struct kvm_vcpu *vcpu, struct msr_data *msr_info)
 	case MSR_CORE_PERF_GLOBAL_OVF_CTRL:
 		msr_info->data = pmu->global_ovf_ctrl;
 		return 0;
+	case MSR_ARCH_LBR_DEPTH:
+		msr_info->data = lbr_desc->records.nr;
+		return 0;
 	default:
 		if ((pmc = get_gp_pmc(pmu, msr, MSR_IA32_PERFCTR0)) ||
 		    (pmc = get_gp_pmc(pmu, msr, MSR_IA32_PMC0))) {
@@ -393,6 +418,7 @@ static int intel_pmu_set_msr(struct kvm_vcpu *vcpu, struct msr_data *msr_info)
 {
 	struct kvm_pmu *pmu = vcpu_to_pmu(vcpu);
 	struct kvm_pmc *pmc;
+	struct lbr_desc *lbr_desc = vcpu_to_lbr_desc(vcpu);
 	u32 msr = msr_info->index;
 	u64 data = msr_info->data;
 
@@ -427,6 +453,13 @@ static int intel_pmu_set_msr(struct kvm_vcpu *vcpu, struct msr_data *msr_info)
 			return 0;
 		}
 		break;
+	case MSR_ARCH_LBR_DEPTH:
+		if (!arch_lbr_depth_is_valid(vcpu, data))
+			return 1;
+		lbr_desc->records.nr = data;
+		if (!msr_info->host_initiated)
+			wrmsrl(MSR_ARCH_LBR_DEPTH, lbr_desc->records.nr);
+		return 0;
 	default:
 		if ((pmc = get_gp_pmc(pmu, msr, MSR_IA32_PERFCTR0)) ||
 		    (pmc = get_gp_pmc(pmu, msr, MSR_IA32_PMC0))) {
-- 
2.25.1


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

* [PATCH v7 05/15] KVM: vmx/pmu: Emulate MSR_ARCH_LBR_CTL for guest Arch LBR
  2021-08-06  7:42 [PATCH v7 00/15] Introduce Architectural LBR for vPMU Yang Weijiang
                   ` (3 preceding siblings ...)
  2021-08-06  7:42 ` [PATCH v7 04/15] KVM: vmx/pmu: Emulate MSR_ARCH_LBR_DEPTH for guest Arch LBR Yang Weijiang
@ 2021-08-06  7:42 ` Yang Weijiang
  2021-08-06 16:26     ` kernel test robot
  2021-08-09 13:36   ` Like Xu
  2021-08-06  7:42 ` [PATCH v7 06/15] KVM: x86/pmu: Refactor code to support " Yang Weijiang
                   ` (9 subsequent siblings)
  14 siblings, 2 replies; 28+ messages in thread
From: Yang Weijiang @ 2021-08-06  7:42 UTC (permalink / raw)
  To: pbonzini, jmattson, seanjc, vkuznets, wei.w.wang, like.xu.linux,
	kvm, linux-kernel
  Cc: Like Xu, Yang Weijiang

From: Like Xu <like.xu@linux.intel.com>

Arch LBRs are enabled by setting MSR_ARCH_LBR_CTL.LBREn to 1. A new guest
state field named "Guest IA32_LBR_CTL" is added to enhance guest LBR usage.
When guest Arch LBR is enabled, a guest LBR event will be created like the
model-specific LBR does. Clear guest LBR enable bit on host PMI handling so
guest can see expected config.

On processors that support Arch LBR, MSR_IA32_DEBUGCTLMSR[bit 0] has no
meaning. It can be written to 0 or 1, but reads will always return 0.
Like IA32_DEBUGCTL, IA32_ARCH_LBR_CTL msr is also preserved on INIT.

Regardless of the Arch LBR or legacy LBR, when the LBR_EN bit 0 of the
corresponding control MSR is set to 1, LBR recording will be enabled.

Signed-off-by: Like Xu <like.xu@linux.intel.com>
Signed-off-by: Yang Weijiang <weijiang.yang@intel.com>
---
 arch/x86/events/intel/lbr.c      |  2 --
 arch/x86/include/asm/msr-index.h |  1 +
 arch/x86/include/asm/vmx.h       |  2 ++
 arch/x86/kvm/vmx/pmu_intel.c     | 48 ++++++++++++++++++++++++++++----
 arch/x86/kvm/vmx/vmx.c           |  9 ++++++
 5 files changed, 55 insertions(+), 7 deletions(-)

diff --git a/arch/x86/events/intel/lbr.c b/arch/x86/events/intel/lbr.c
index ceff16eb4bcb..d6773a200c70 100644
--- a/arch/x86/events/intel/lbr.c
+++ b/arch/x86/events/intel/lbr.c
@@ -168,8 +168,6 @@ enum {
 	 ARCH_LBR_RETURN		|\
 	 ARCH_LBR_OTHER_BRANCH)
 
-#define ARCH_LBR_CTL_MASK			0x7f000e
-
 static void intel_pmu_lbr_filter(struct cpu_hw_events *cpuc);
 
 static __always_inline bool is_lbr_call_stack_bit_set(u64 config)
diff --git a/arch/x86/include/asm/msr-index.h b/arch/x86/include/asm/msr-index.h
index a7c413432b33..04059e8ed115 100644
--- a/arch/x86/include/asm/msr-index.h
+++ b/arch/x86/include/asm/msr-index.h
@@ -169,6 +169,7 @@
 #define LBR_INFO_BR_TYPE		(0xfull << LBR_INFO_BR_TYPE_OFFSET)
 
 #define MSR_ARCH_LBR_CTL		0x000014ce
+#define ARCH_LBR_CTL_MASK		0x7f000e
 #define ARCH_LBR_CTL_LBREN		BIT(0)
 #define ARCH_LBR_CTL_CPL_OFFSET		1
 #define ARCH_LBR_CTL_CPL		(0x3ull << ARCH_LBR_CTL_CPL_OFFSET)
diff --git a/arch/x86/include/asm/vmx.h b/arch/x86/include/asm/vmx.h
index 0ffaa3156a4e..ea3be961cc8e 100644
--- a/arch/x86/include/asm/vmx.h
+++ b/arch/x86/include/asm/vmx.h
@@ -245,6 +245,8 @@ enum vmcs_field {
 	GUEST_BNDCFGS_HIGH              = 0x00002813,
 	GUEST_IA32_RTIT_CTL		= 0x00002814,
 	GUEST_IA32_RTIT_CTL_HIGH	= 0x00002815,
+	GUEST_IA32_LBR_CTL		= 0x00002816,
+	GUEST_IA32_LBR_CTL_HIGH		= 0x00002817,
 	HOST_IA32_PAT			= 0x00002c00,
 	HOST_IA32_PAT_HIGH		= 0x00002c01,
 	HOST_IA32_EFER			= 0x00002c02,
diff --git a/arch/x86/kvm/vmx/pmu_intel.c b/arch/x86/kvm/vmx/pmu_intel.c
index a4ef5bbce186..b2631fea5e6c 100644
--- a/arch/x86/kvm/vmx/pmu_intel.c
+++ b/arch/x86/kvm/vmx/pmu_intel.c
@@ -19,6 +19,7 @@
 #include "pmu.h"
 
 #define MSR_PMC_FULL_WIDTH_BIT      (MSR_IA32_PMC0 - MSR_IA32_PERFCTR0)
+#define KVM_ARCH_LBR_CTL_MASK  (ARCH_LBR_CTL_MASK | ARCH_LBR_CTL_LBREN)
 
 static struct kvm_event_hw_type_mapping intel_arch_events[] = {
 	/* Index must match CPUID 0x0A.EBX bit vector */
@@ -221,6 +222,7 @@ static bool intel_is_valid_msr(struct kvm_vcpu *vcpu, u32 msr)
 		ret = pmu->version > 1;
 		break;
 	case MSR_ARCH_LBR_DEPTH:
+	case MSR_ARCH_LBR_CTL:
 		if (kvm_cpu_cap_has(X86_FEATURE_ARCH_LBR))
 			ret = guest_cpuid_has(vcpu, X86_FEATURE_ARCH_LBR);
 		break;
@@ -369,6 +371,26 @@ static bool arch_lbr_depth_is_valid(struct kvm_vcpu *vcpu, u64 depth)
 	return (depth == fls(eax & 0xff) * 8);
 }
 
+#define ARCH_LBR_CTL_BRN_MASK   GENMASK_ULL(22, 16)
+
+static bool arch_lbr_ctl_is_valid(struct kvm_vcpu *vcpu, u64 ctl)
+{
+	unsigned int eax, ebx, ecx, edx;
+
+	if (!kvm_cpu_cap_has(X86_FEATURE_ARCH_LBR))
+		return false;
+
+	cpuid_count(0x1c, 0, &eax, &ebx, &ecx, &edx);
+	if (!(ebx & BIT(0)) && (ctl & ARCH_LBR_CTL_CPL))
+		return false;
+	if (!(ebx & BIT(2)) && (ctl & ARCH_LBR_CTL_STACK))
+		return false;
+	if (!(ebx & BIT(1)) && (ctl & ARCH_LBR_CTL_BRN_MASK))
+		return false;
+
+	return !(ctl & ~KVM_ARCH_LBR_CTL_MASK);
+}
+
 static int intel_pmu_get_msr(struct kvm_vcpu *vcpu, struct msr_data *msr_info)
 {
 	struct kvm_pmu *pmu = vcpu_to_pmu(vcpu);
@@ -392,6 +414,9 @@ static int intel_pmu_get_msr(struct kvm_vcpu *vcpu, struct msr_data *msr_info)
 	case MSR_ARCH_LBR_DEPTH:
 		msr_info->data = lbr_desc->records.nr;
 		return 0;
+	case MSR_ARCH_LBR_CTL:
+		msr_info->data = vmcs_read64(GUEST_IA32_LBR_CTL);
+		return 0;
 	default:
 		if ((pmc = get_gp_pmc(pmu, msr, MSR_IA32_PERFCTR0)) ||
 		    (pmc = get_gp_pmc(pmu, msr, MSR_IA32_PMC0))) {
@@ -460,6 +485,15 @@ static int intel_pmu_set_msr(struct kvm_vcpu *vcpu, struct msr_data *msr_info)
 		if (!msr_info->host_initiated)
 			wrmsrl(MSR_ARCH_LBR_DEPTH, lbr_desc->records.nr);
 		return 0;
+	case MSR_ARCH_LBR_CTL:
+		if (!arch_lbr_ctl_is_valid(vcpu, data))
+			break;
+
+		vmcs_write64(GUEST_IA32_LBR_CTL, data);
+		if (intel_pmu_lbr_is_enabled(vcpu) && !lbr_desc->event &&
+		    (data & ARCH_LBR_CTL_LBREN))
+			intel_pmu_create_guest_lbr_event(vcpu);
+		return 0;
 	default:
 		if ((pmc = get_gp_pmc(pmu, msr, MSR_IA32_PERFCTR0)) ||
 		    (pmc = get_gp_pmc(pmu, msr, MSR_IA32_PMC0))) {
@@ -637,12 +671,16 @@ static void intel_pmu_reset(struct kvm_vcpu *vcpu)
  */
 static void intel_pmu_legacy_freezing_lbrs_on_pmi(struct kvm_vcpu *vcpu)
 {
-	u64 data = vmcs_read64(GUEST_IA32_DEBUGCTL);
+	u32 lbr_ctl_field = GUEST_IA32_DEBUGCTL;
 
-	if (data & DEBUGCTLMSR_FREEZE_LBRS_ON_PMI) {
-		data &= ~DEBUGCTLMSR_LBR;
-		vmcs_write64(GUEST_IA32_DEBUGCTL, data);
-	}
+	if (!(vmcs_read64(GUEST_IA32_DEBUGCTL) & DEBUGCTLMSR_FREEZE_LBRS_ON_PMI))
+		return;
+
+	if (kvm_cpu_cap_has(X86_FEATURE_ARCH_LBR) &&
+	    guest_cpuid_has(vcpu, X86_FEATURE_ARCH_LBR))
+		lbr_ctl_field = GUEST_IA32_LBR_CTL;
+
+	vmcs_write64(lbr_ctl_field, vmcs_read64(lbr_ctl_field) & ~0x1ULL);
 }
 
 static void intel_pmu_deliver_pmi(struct kvm_vcpu *vcpu)
diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c
index 927a552393b9..e8df44faf900 100644
--- a/arch/x86/kvm/vmx/vmx.c
+++ b/arch/x86/kvm/vmx/vmx.c
@@ -2002,6 +2002,13 @@ static int vmx_set_msr(struct kvm_vcpu *vcpu, struct msr_data *msr_info)
 						VM_EXIT_SAVE_DEBUG_CONTROLS)
 			get_vmcs12(vcpu)->guest_ia32_debugctl = data;
 
+		/*
+		 * For Arch LBR, IA32_DEBUGCTL[bit 0] has no meaning.
+		 * It can be written to 0 or 1, but reads will always return 0.
+		 */
+		if (guest_cpuid_has(vcpu, X86_FEATURE_ARCH_LBR))
+			data &= ~DEBUGCTLMSR_LBR;
+
 		vmcs_write64(GUEST_IA32_DEBUGCTL, data);
 		if (intel_pmu_lbr_is_enabled(vcpu) && !to_vmx(vcpu)->lbr_desc.event &&
 		    (data & DEBUGCTLMSR_LBR))
@@ -4441,6 +4448,8 @@ static void vmx_vcpu_reset(struct kvm_vcpu *vcpu, bool init_event)
 		vmcs_writel(GUEST_SYSENTER_ESP, 0);
 		vmcs_writel(GUEST_SYSENTER_EIP, 0);
 		vmcs_write64(GUEST_IA32_DEBUGCTL, 0);
+		if (static_cpu_has(X86_FEATURE_ARCH_LBR))
+			vmcs_write64(GUEST_IA32_LBR_CTL, 0);
 	}
 
 	kvm_set_rflags(vcpu, X86_EFLAGS_FIXED);
-- 
2.25.1


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

* [PATCH v7 06/15] KVM: x86/pmu: Refactor code to support guest Arch LBR
  2021-08-06  7:42 [PATCH v7 00/15] Introduce Architectural LBR for vPMU Yang Weijiang
                   ` (4 preceding siblings ...)
  2021-08-06  7:42 ` [PATCH v7 05/15] KVM: vmx/pmu: Emulate MSR_ARCH_LBR_CTL " Yang Weijiang
@ 2021-08-06  7:42 ` Yang Weijiang
  2021-08-06  7:42 ` [PATCH v7 07/15] KVM: x86: Refresh CPUID on writes to MSR_IA32_XSS Yang Weijiang
                   ` (8 subsequent siblings)
  14 siblings, 0 replies; 28+ messages in thread
From: Yang Weijiang @ 2021-08-06  7:42 UTC (permalink / raw)
  To: pbonzini, jmattson, seanjc, vkuznets, wei.w.wang, like.xu.linux,
	kvm, linux-kernel
  Cc: Yang Weijiang

Take account of Arch LBR when do sanity checks before program
vPMU for guest. Pass through Arch LBR recording MSRs to guest
to gain better performance. Note, Arch LBR and Legacy LBR support
are mutually exclusive, i.e., they're not both available on one
platform.

Co-developed-by: Like Xu <like.xu@linux.intel.com>
Signed-off-by: Like Xu <like.xu@linux.intel.com>
Signed-off-by: Yang Weijiang <weijiang.yang@intel.com>
---
 arch/x86/kvm/vmx/pmu_intel.c | 37 +++++++++++++++++++++++++++++-------
 arch/x86/kvm/vmx/vmx.c       |  3 +++
 2 files changed, 33 insertions(+), 7 deletions(-)

diff --git a/arch/x86/kvm/vmx/pmu_intel.c b/arch/x86/kvm/vmx/pmu_intel.c
index b2631fea5e6c..35bcdc5357ee 100644
--- a/arch/x86/kvm/vmx/pmu_intel.c
+++ b/arch/x86/kvm/vmx/pmu_intel.c
@@ -176,12 +176,16 @@ static inline struct kvm_pmc *get_fw_gp_pmc(struct kvm_pmu *pmu, u32 msr)
 
 bool intel_pmu_lbr_is_compatible(struct kvm_vcpu *vcpu)
 {
+	if (kvm_cpu_cap_has(X86_FEATURE_ARCH_LBR))
+		return guest_cpuid_has(vcpu, X86_FEATURE_ARCH_LBR);
+
 	/*
 	 * As a first step, a guest could only enable LBR feature if its
 	 * cpu model is the same as the host because the LBR registers
 	 * would be pass-through to the guest and they're model specific.
 	 */
-	return boot_cpu_data.x86_model == guest_cpuid_model(vcpu);
+	return !boot_cpu_has(X86_FEATURE_ARCH_LBR) &&
+		boot_cpu_data.x86_model == guest_cpuid_model(vcpu);
 }
 
 bool intel_pmu_lbr_is_enabled(struct kvm_vcpu *vcpu)
@@ -199,12 +203,19 @@ static bool intel_pmu_is_valid_lbr_msr(struct kvm_vcpu *vcpu, u32 index)
 	if (!intel_pmu_lbr_is_enabled(vcpu))
 		return ret;
 
-	ret = (index == MSR_LBR_SELECT) || (index == MSR_LBR_TOS) ||
-		(index >= records->from && index < records->from + records->nr) ||
-		(index >= records->to && index < records->to + records->nr);
+	if (!guest_cpuid_has(vcpu, X86_FEATURE_ARCH_LBR))
+		ret = (index == MSR_LBR_SELECT) || (index == MSR_LBR_TOS);
+
+	if (!ret) {
+		ret = (index >= records->from &&
+		       index < records->from + records->nr) ||
+		      (index >= records->to &&
+		       index < records->to + records->nr);
+	}
 
 	if (!ret && records->info)
-		ret = (index >= records->info && index < records->info + records->nr);
+		ret = (index >= records->info &&
+		       index < records->info + records->nr);
 
 	return ret;
 }
@@ -706,6 +717,9 @@ static void vmx_update_intercept_for_lbr_msrs(struct kvm_vcpu *vcpu, bool set)
 			vmx_set_intercept_for_msr(vcpu, lbr->info + i, MSR_TYPE_RW, set);
 	}
 
+	if (guest_cpuid_has(vcpu, X86_FEATURE_ARCH_LBR))
+		return;
+
 	vmx_set_intercept_for_msr(vcpu, MSR_LBR_SELECT, MSR_TYPE_RW, set);
 	vmx_set_intercept_for_msr(vcpu, MSR_LBR_TOS, MSR_TYPE_RW, set);
 }
@@ -746,10 +760,13 @@ void vmx_passthrough_lbr_msrs(struct kvm_vcpu *vcpu)
 {
 	struct kvm_pmu *pmu = vcpu_to_pmu(vcpu);
 	struct lbr_desc *lbr_desc = vcpu_to_lbr_desc(vcpu);
+	bool lbr_enable = guest_cpuid_has(vcpu, X86_FEATURE_ARCH_LBR) ?
+		(vmcs_read64(GUEST_IA32_LBR_CTL) & ARCH_LBR_CTL_LBREN) :
+		(vmcs_read64(GUEST_IA32_DEBUGCTL) & DEBUGCTLMSR_LBR);
 
 	if (!lbr_desc->event) {
 		vmx_disable_lbr_msrs_passthrough(vcpu);
-		if (vmcs_read64(GUEST_IA32_DEBUGCTL) & DEBUGCTLMSR_LBR)
+		if (lbr_enable)
 			goto warn;
 		if (test_bit(INTEL_PMC_IDX_FIXED_VLBR, pmu->pmc_in_use))
 			goto warn;
@@ -766,13 +783,19 @@ void vmx_passthrough_lbr_msrs(struct kvm_vcpu *vcpu)
 	return;
 
 warn:
+	if (kvm_cpu_cap_has(X86_FEATURE_ARCH_LBR))
+		wrmsrl(MSR_ARCH_LBR_DEPTH, lbr_desc->records.nr);
 	pr_warn_ratelimited("kvm: vcpu-%d: fail to passthrough LBR.\n",
 		vcpu->vcpu_id);
 }
 
 static void intel_pmu_cleanup(struct kvm_vcpu *vcpu)
 {
-	if (!(vmcs_read64(GUEST_IA32_DEBUGCTL) & DEBUGCTLMSR_LBR))
+	bool lbr_enable = guest_cpuid_has(vcpu, X86_FEATURE_ARCH_LBR) ?
+		(vmcs_read64(GUEST_IA32_LBR_CTL) & ARCH_LBR_CTL_LBREN) :
+		(vmcs_read64(GUEST_IA32_DEBUGCTL) & DEBUGCTLMSR_LBR);
+
+	if (!lbr_enable)
 		intel_pmu_release_guest_lbr_event(vcpu);
 }
 
diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c
index e8df44faf900..c15be89d6619 100644
--- a/arch/x86/kvm/vmx/vmx.c
+++ b/arch/x86/kvm/vmx/vmx.c
@@ -576,6 +576,9 @@ static bool is_valid_passthrough_msr(u32 msr)
 	case MSR_LBR_NHM_TO ... MSR_LBR_NHM_TO + 31:
 	case MSR_LBR_CORE_FROM ... MSR_LBR_CORE_FROM + 8:
 	case MSR_LBR_CORE_TO ... MSR_LBR_CORE_TO + 8:
+	case MSR_ARCH_LBR_FROM_0 ... MSR_ARCH_LBR_FROM_0 + 31:
+	case MSR_ARCH_LBR_TO_0 ... MSR_ARCH_LBR_TO_0 + 31:
+	case MSR_ARCH_LBR_INFO_0 ... MSR_ARCH_LBR_INFO_0 + 31:
 		/* LBR MSRs. These are handled in vmx_update_intercept_for_lbr_msrs() */
 		return true;
 	}
-- 
2.25.1


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

* [PATCH v7 07/15] KVM: x86: Refresh CPUID on writes to MSR_IA32_XSS
  2021-08-06  7:42 [PATCH v7 00/15] Introduce Architectural LBR for vPMU Yang Weijiang
                   ` (5 preceding siblings ...)
  2021-08-06  7:42 ` [PATCH v7 06/15] KVM: x86/pmu: Refactor code to support " Yang Weijiang
@ 2021-08-06  7:42 ` Yang Weijiang
  2021-08-06  7:42 ` [PATCH v7 08/15] KVM: x86: Report XSS as an MSR to be saved if there are supported features Yang Weijiang
                   ` (7 subsequent siblings)
  14 siblings, 0 replies; 28+ messages in thread
From: Yang Weijiang @ 2021-08-06  7:42 UTC (permalink / raw)
  To: pbonzini, jmattson, seanjc, vkuznets, wei.w.wang, like.xu.linux,
	kvm, linux-kernel
  Cc: Yang Weijiang

Updated CPUID.0xD.0x1, which reports the current required storage size
of all features enabled via XCR0 | XSS, when the guest's XSS is modified.

Note, KVM does not yet support any XSS based features, i.e. supported_xss
is guaranteed to be zero at this time.

Co-developed-by: Zhang Yi Z <yi.z.zhang@linux.intel.com>
Signed-off-by: Zhang Yi Z <yi.z.zhang@linux.intel.com>
Signed-off-by: Yang Weijiang <weijiang.yang@intel.com>
Message-Id: <20210203113421.5759-3-weijiang.yang@intel.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 arch/x86/include/asm/kvm_host.h |  1 +
 arch/x86/kvm/cpuid.c            | 21 ++++++++++++++++++---
 arch/x86/kvm/x86.c              |  7 +++++--
 3 files changed, 24 insertions(+), 5 deletions(-)

diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
index 974cbfb1eefe..4a4fb99e8a99 100644
--- a/arch/x86/include/asm/kvm_host.h
+++ b/arch/x86/include/asm/kvm_host.h
@@ -692,6 +692,7 @@ struct kvm_vcpu_arch {
 
 	u64 xcr0;
 	u64 guest_supported_xcr0;
+	u64 guest_supported_xss;
 
 	struct kvm_pio_request pio;
 	void *pio_data;
diff --git a/arch/x86/kvm/cpuid.c b/arch/x86/kvm/cpuid.c
index 739be5da3bca..03025eea1524 100644
--- a/arch/x86/kvm/cpuid.c
+++ b/arch/x86/kvm/cpuid.c
@@ -131,9 +131,24 @@ void kvm_update_cpuid_runtime(struct kvm_vcpu *vcpu)
 		best->ebx = xstate_required_size(vcpu->arch.xcr0, false);
 
 	best = kvm_find_cpuid_entry(vcpu, 0xD, 1);
-	if (best && (cpuid_entry_has(best, X86_FEATURE_XSAVES) ||
-		     cpuid_entry_has(best, X86_FEATURE_XSAVEC)))
-		best->ebx = xstate_required_size(vcpu->arch.xcr0, true);
+	if (best) {
+		if (cpuid_entry_has(best, X86_FEATURE_XSAVES) ||
+		    cpuid_entry_has(best, X86_FEATURE_XSAVEC))  {
+			u64 xstate = vcpu->arch.xcr0 | vcpu->arch.ia32_xss;
+
+			best->ebx = xstate_required_size(xstate, true);
+		}
+
+		if (!cpuid_entry_has(best, X86_FEATURE_XSAVES)) {
+			best->ecx = 0;
+			best->edx = 0;
+		}
+		vcpu->arch.guest_supported_xss =
+			(((u64)best->edx << 32) | best->ecx) & supported_xss;
+
+	} else {
+		vcpu->arch.guest_supported_xss = 0;
+	}
 
 	best = kvm_find_cpuid_entry(vcpu, KVM_CPUID_FEATURES, 0);
 	if (kvm_hlt_in_guest(vcpu->kvm) && best &&
diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index 4ae173ab2208..08a58ef8bec2 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -3358,9 +3358,12 @@ int kvm_set_msr_common(struct kvm_vcpu *vcpu, struct msr_data *msr_info)
 		 * IA32_XSS[bit 8]. Guests have to use RDMSR/WRMSR rather than
 		 * XSAVES/XRSTORS to save/restore PT MSRs.
 		 */
-		if (data & ~supported_xss)
+		if (data & ~vcpu->arch.guest_supported_xss)
 			return 1;
-		vcpu->arch.ia32_xss = data;
+		if (vcpu->arch.ia32_xss != data) {
+			vcpu->arch.ia32_xss = data;
+			kvm_update_cpuid_runtime(vcpu);
+		}
 		break;
 	case MSR_SMI_COUNT:
 		if (!msr_info->host_initiated)
-- 
2.25.1


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

* [PATCH v7 08/15] KVM: x86: Report XSS as an MSR to be saved if there are supported features
  2021-08-06  7:42 [PATCH v7 00/15] Introduce Architectural LBR for vPMU Yang Weijiang
                   ` (6 preceding siblings ...)
  2021-08-06  7:42 ` [PATCH v7 07/15] KVM: x86: Refresh CPUID on writes to MSR_IA32_XSS Yang Weijiang
@ 2021-08-06  7:42 ` Yang Weijiang
  2021-08-06  7:42 ` [PATCH v7 09/15] KVM: x86: Refine the matching and clearing logic for supported_xss Yang Weijiang
                   ` (6 subsequent siblings)
  14 siblings, 0 replies; 28+ messages in thread
From: Yang Weijiang @ 2021-08-06  7:42 UTC (permalink / raw)
  To: pbonzini, jmattson, seanjc, vkuznets, wei.w.wang, like.xu.linux,
	kvm, linux-kernel
  Cc: Sean Christopherson, Yang Weijiang

From: Sean Christopherson <sean.j.christopherson@intel.com>

Add MSR_IA32_XSS to the list of MSRs reported to userspace if
supported_xss is non-zero, i.e. KVM supports at least one XSS based
feature.

Signed-off-by: Sean Christopherson <sean.j.christopherson@intel.com>
Signed-off-by: Yang Weijiang <weijiang.yang@intel.com>
Message-Id: <20210203113421.5759-2-weijiang.yang@intel.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 arch/x86/kvm/x86.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index 08a58ef8bec2..2ebb05212652 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -1328,6 +1328,7 @@ static const u32 msrs_to_save_all[] = {
 	MSR_ARCH_PERFMON_EVENTSEL0 + 14, MSR_ARCH_PERFMON_EVENTSEL0 + 15,
 	MSR_ARCH_PERFMON_EVENTSEL0 + 16, MSR_ARCH_PERFMON_EVENTSEL0 + 17,
 	MSR_ARCH_LBR_CTL, MSR_ARCH_LBR_DEPTH,
+	MSR_IA32_XSS,
 };
 
 static u32 msrs_to_save[ARRAY_SIZE(msrs_to_save_all)];
@@ -6238,6 +6239,10 @@ static void kvm_init_msr_list(void)
 			if (!kvm_cpu_cap_has(X86_FEATURE_ARCH_LBR))
 				continue;
 			break;
+		case MSR_IA32_XSS:
+			if (!supported_xss)
+				continue;
+			break;
 		default:
 			break;
 		}
-- 
2.25.1


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

* [PATCH v7 09/15] KVM: x86: Refine the matching and clearing logic for supported_xss
  2021-08-06  7:42 [PATCH v7 00/15] Introduce Architectural LBR for vPMU Yang Weijiang
                   ` (7 preceding siblings ...)
  2021-08-06  7:42 ` [PATCH v7 08/15] KVM: x86: Report XSS as an MSR to be saved if there are supported features Yang Weijiang
@ 2021-08-06  7:42 ` Yang Weijiang
  2021-08-06  7:42 ` [PATCH v7 10/15] KVM: x86: Add XSAVE Support for Architectural LBR Yang Weijiang
                   ` (5 subsequent siblings)
  14 siblings, 0 replies; 28+ messages in thread
From: Yang Weijiang @ 2021-08-06  7:42 UTC (permalink / raw)
  To: pbonzini, jmattson, seanjc, vkuznets, wei.w.wang, like.xu.linux,
	kvm, linux-kernel
  Cc: Like Xu, Yang Weijiang

From: Like Xu <like.xu@linux.intel.com>

Refine the code path of the existing clearing of supported_xss in this way:
initialize the supported_xss with the filter of KVM_SUPPORTED_XSS mask and
update its value in a bit clear manner (rather than bit setting).

Suggested-by: Sean Christopherson <seanjc@google.com>
Signed-off-by: Like Xu <like.xu@linux.intel.com>
Signed-off-by: Yang Weijiang <weijiang.yang@intel.com>
---
 arch/x86/kvm/vmx/vmx.c | 5 +++--
 arch/x86/kvm/x86.c     | 6 +++++-
 2 files changed, 8 insertions(+), 3 deletions(-)

diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c
index c15be89d6619..f4d228a905c8 100644
--- a/arch/x86/kvm/vmx/vmx.c
+++ b/arch/x86/kvm/vmx/vmx.c
@@ -7244,9 +7244,10 @@ static __init void vmx_set_cpu_caps(void)
 		kvm_cpu_cap_set(X86_FEATURE_UMIP);
 
 	/* CPUID 0xD.1 */
-	supported_xss = 0;
-	if (!cpu_has_vmx_xsaves())
+	if (!cpu_has_vmx_xsaves()) {
 		kvm_cpu_cap_clear(X86_FEATURE_XSAVES);
+		supported_xss = 0;
+	}
 
 	/* CPUID 0x80000001 and 0x7 (RDPID) */
 	if (!cpu_has_vmx_rdtscp()) {
diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index 2ebb05212652..ef80370d825b 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -210,6 +210,8 @@ static struct kvm_user_return_msrs __percpu *user_return_msrs;
 				| XFEATURE_MASK_BNDCSR | XFEATURE_MASK_AVX512 \
 				| XFEATURE_MASK_PKRU)
 
+#define KVM_SUPPORTED_XSS     0
+
 u64 __read_mostly host_efer;
 EXPORT_SYMBOL_GPL(host_efer);
 
@@ -11002,8 +11004,10 @@ int kvm_arch_hardware_setup(void *opaque)
 
 	rdmsrl_safe(MSR_EFER, &host_efer);
 
-	if (boot_cpu_has(X86_FEATURE_XSAVES))
+	if (boot_cpu_has(X86_FEATURE_XSAVES)) {
 		rdmsrl(MSR_IA32_XSS, host_xss);
+		supported_xss = host_xss & KVM_SUPPORTED_XSS;
+	}
 
 	r = ops->hardware_setup();
 	if (r != 0)
-- 
2.25.1


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

* [PATCH v7 10/15] KVM: x86: Add XSAVE Support for Architectural LBR
  2021-08-06  7:42 [PATCH v7 00/15] Introduce Architectural LBR for vPMU Yang Weijiang
                   ` (8 preceding siblings ...)
  2021-08-06  7:42 ` [PATCH v7 09/15] KVM: x86: Refine the matching and clearing logic for supported_xss Yang Weijiang
@ 2021-08-06  7:42 ` Yang Weijiang
  2021-08-06  7:42 ` [PATCH v7 11/15] KVM: x86/vmx: Check Arch LBR config when return perf capabilities Yang Weijiang
                   ` (4 subsequent siblings)
  14 siblings, 0 replies; 28+ messages in thread
From: Yang Weijiang @ 2021-08-06  7:42 UTC (permalink / raw)
  To: pbonzini, jmattson, seanjc, vkuznets, wei.w.wang, like.xu.linux,
	kvm, linux-kernel
  Cc: Like Xu, Yang Weijiang

From: Like Xu <like.xu@linux.intel.com>

On processors supporting XSAVES and XRSTORS, Architectural LBR XSAVE
support is enumerated from CPUID.(EAX=0DH, ECX=1):ECX[bit 15].
The detailed sub-leaf for Arch LBR is enumerated in CPUID.(0DH, 0FH).

XSAVES provides a faster means than RDMSR for guest to read all LBRs.
When guest IA32_XSS[bit 15] is set, the Arch LBR state can be saved using
XSAVES and restored by XRSTORS with the appropriate RFBM.

Signed-off-by: Like Xu <like.xu@linux.intel.com>
Signed-off-by: Yang Weijiang <weijiang.yang@intel.com>
---
 arch/x86/kvm/vmx/vmx.c | 4 ++++
 arch/x86/kvm/x86.c     | 2 +-
 2 files changed, 5 insertions(+), 1 deletion(-)

diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c
index f4d228a905c8..5f0545106d54 100644
--- a/arch/x86/kvm/vmx/vmx.c
+++ b/arch/x86/kvm/vmx/vmx.c
@@ -7232,6 +7232,10 @@ static __init void vmx_set_cpu_caps(void)
 		kvm_cpu_cap_clear(X86_FEATURE_INVPCID);
 	if (vmx_pt_mode_is_host_guest())
 		kvm_cpu_cap_check_and_set(X86_FEATURE_INTEL_PT);
+	if (!cpu_has_vmx_arch_lbr()) {
+		kvm_cpu_cap_clear(X86_FEATURE_ARCH_LBR);
+		supported_xss &= ~XFEATURE_MASK_LBR;
+	}
 
 	if (!enable_sgx) {
 		kvm_cpu_cap_clear(X86_FEATURE_SGX);
diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index ef80370d825b..62db124c62a5 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -210,7 +210,7 @@ static struct kvm_user_return_msrs __percpu *user_return_msrs;
 				| XFEATURE_MASK_BNDCSR | XFEATURE_MASK_AVX512 \
 				| XFEATURE_MASK_PKRU)
 
-#define KVM_SUPPORTED_XSS     0
+#define KVM_SUPPORTED_XSS     XFEATURE_MASK_LBR
 
 u64 __read_mostly host_efer;
 EXPORT_SYMBOL_GPL(host_efer);
-- 
2.25.1


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

* [PATCH v7 11/15] KVM: x86/vmx: Check Arch LBR config when return perf capabilities
  2021-08-06  7:42 [PATCH v7 00/15] Introduce Architectural LBR for vPMU Yang Weijiang
                   ` (9 preceding siblings ...)
  2021-08-06  7:42 ` [PATCH v7 10/15] KVM: x86: Add XSAVE Support for Architectural LBR Yang Weijiang
@ 2021-08-06  7:42 ` Yang Weijiang
  2021-08-06  7:42 ` [PATCH v7 12/15] KVM: nVMX: Add necessary Arch LBR settings for nested VM Yang Weijiang
                   ` (3 subsequent siblings)
  14 siblings, 0 replies; 28+ messages in thread
From: Yang Weijiang @ 2021-08-06  7:42 UTC (permalink / raw)
  To: pbonzini, jmattson, seanjc, vkuznets, wei.w.wang, like.xu.linux,
	kvm, linux-kernel
  Cc: Yang Weijiang

Two new bit fields(VM_EXIT_CLEAR_IA32_LBR_CTL, VM_ENTRY_LOAD_IA32_LBR_CTL)
are added to support guest Arch LBR. These two bits should be set in order
to make Arch LBR workable in both guest and host.

Co-developed-by: Like Xu <like.xu@linux.intel.com>
Signed-off-by: Like Xu <like.xu@linux.intel.com>
Signed-off-by: Yang Weijiang <weijiang.yang@intel.com>
---
 arch/x86/include/asm/vmx.h      |  2 ++
 arch/x86/kvm/vmx/capabilities.h | 25 +++++++++++++++++--------
 arch/x86/kvm/vmx/vmx.c          |  6 ++++--
 3 files changed, 23 insertions(+), 10 deletions(-)

diff --git a/arch/x86/include/asm/vmx.h b/arch/x86/include/asm/vmx.h
index ea3be961cc8e..d9b1dffc4638 100644
--- a/arch/x86/include/asm/vmx.h
+++ b/arch/x86/include/asm/vmx.h
@@ -95,6 +95,7 @@
 #define VM_EXIT_CLEAR_BNDCFGS                   0x00800000
 #define VM_EXIT_PT_CONCEAL_PIP			0x01000000
 #define VM_EXIT_CLEAR_IA32_RTIT_CTL		0x02000000
+#define VM_EXIT_CLEAR_IA32_LBR_CTL		0x04000000
 
 #define VM_EXIT_ALWAYSON_WITHOUT_TRUE_MSR	0x00036dff
 
@@ -108,6 +109,7 @@
 #define VM_ENTRY_LOAD_BNDCFGS                   0x00010000
 #define VM_ENTRY_PT_CONCEAL_PIP			0x00020000
 #define VM_ENTRY_LOAD_IA32_RTIT_CTL		0x00040000
+#define VM_ENTRY_LOAD_IA32_LBR_CTL		0x00200000
 
 #define VM_ENTRY_ALWAYSON_WITHOUT_TRUE_MSR	0x000011ff
 
diff --git a/arch/x86/kvm/vmx/capabilities.h b/arch/x86/kvm/vmx/capabilities.h
index 4705ad55abb5..51855a559a75 100644
--- a/arch/x86/kvm/vmx/capabilities.h
+++ b/arch/x86/kvm/vmx/capabilities.h
@@ -376,20 +376,29 @@ static inline bool vmx_pt_mode_is_host_guest(void)
 	return pt_mode == PT_MODE_HOST_GUEST;
 }
 
-static inline u64 vmx_get_perf_capabilities(void)
+static inline bool cpu_has_vmx_arch_lbr(void)
 {
-	u64 perf_cap = 0;
-
-	if (boot_cpu_has(X86_FEATURE_PDCM))
-		rdmsrl(MSR_IA32_PERF_CAPABILITIES, perf_cap);
-
-	perf_cap &= PMU_CAP_LBR_FMT;
+	return (vmcs_config.vmexit_ctrl & VM_EXIT_CLEAR_IA32_LBR_CTL) &&
+		(vmcs_config.vmentry_ctrl & VM_ENTRY_LOAD_IA32_LBR_CTL);
+}
 
+static inline u64 vmx_get_perf_capabilities(void)
+{
 	/*
 	 * Since counters are virtualized, KVM would support full
 	 * width counting unconditionally, even if the host lacks it.
 	 */
-	return PMU_CAP_FW_WRITES | perf_cap;
+	u64 perf_cap = PMU_CAP_FW_WRITES;
+	u64 host_perf_cap = 0;
+
+	if (boot_cpu_has(X86_FEATURE_PDCM))
+		rdmsrl(MSR_IA32_PERF_CAPABILITIES, host_perf_cap);
+
+	perf_cap |= host_perf_cap & PMU_CAP_LBR_FMT;
+	if (boot_cpu_has(X86_FEATURE_ARCH_LBR) && !cpu_has_vmx_arch_lbr())
+		perf_cap &= ~PMU_CAP_LBR_FMT;
+
+	return perf_cap;
 }
 
 static inline u64 vmx_supported_debugctl(void)
diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c
index 5f0545106d54..70314cd93340 100644
--- a/arch/x86/kvm/vmx/vmx.c
+++ b/arch/x86/kvm/vmx/vmx.c
@@ -2513,7 +2513,8 @@ static __init int setup_vmcs_config(struct vmcs_config *vmcs_conf,
 	      VM_EXIT_LOAD_IA32_EFER |
 	      VM_EXIT_CLEAR_BNDCFGS |
 	      VM_EXIT_PT_CONCEAL_PIP |
-	      VM_EXIT_CLEAR_IA32_RTIT_CTL;
+	      VM_EXIT_CLEAR_IA32_RTIT_CTL |
+	      VM_EXIT_CLEAR_IA32_LBR_CTL;
 	if (adjust_vmx_controls(min, opt, MSR_IA32_VMX_EXIT_CTLS,
 				&_vmexit_control) < 0)
 		return -EIO;
@@ -2537,7 +2538,8 @@ static __init int setup_vmcs_config(struct vmcs_config *vmcs_conf,
 	      VM_ENTRY_LOAD_IA32_EFER |
 	      VM_ENTRY_LOAD_BNDCFGS |
 	      VM_ENTRY_PT_CONCEAL_PIP |
-	      VM_ENTRY_LOAD_IA32_RTIT_CTL;
+	      VM_ENTRY_LOAD_IA32_RTIT_CTL |
+	      VM_ENTRY_LOAD_IA32_LBR_CTL;
 	if (adjust_vmx_controls(min, opt, MSR_IA32_VMX_ENTRY_CTLS,
 				&_vmentry_control) < 0)
 		return -EIO;
-- 
2.25.1


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

* [PATCH v7 12/15] KVM: nVMX: Add necessary Arch LBR settings for nested VM
  2021-08-06  7:42 [PATCH v7 00/15] Introduce Architectural LBR for vPMU Yang Weijiang
                   ` (10 preceding siblings ...)
  2021-08-06  7:42 ` [PATCH v7 11/15] KVM: x86/vmx: Check Arch LBR config when return perf capabilities Yang Weijiang
@ 2021-08-06  7:42 ` Yang Weijiang
  2021-08-06  7:42 ` [PATCH v7 13/15] KVM: x86/vmx: Clear Arch LBREn bit before inject #DB to guest Yang Weijiang
                   ` (2 subsequent siblings)
  14 siblings, 0 replies; 28+ messages in thread
From: Yang Weijiang @ 2021-08-06  7:42 UTC (permalink / raw)
  To: pbonzini, jmattson, seanjc, vkuznets, wei.w.wang, like.xu.linux,
	kvm, linux-kernel
  Cc: Yang Weijiang

Arch LBR is not supported in nested VM now. This patch is to add
necessary settings to make it pass host KVM checks before L2 VM is
launched and also to avoid some warnings reported from L1.

Signed-off-by: Yang Weijiang <weijiang.yang@intel.com>
---
 arch/x86/kvm/vmx/nested.c    | 6 ++++--
 arch/x86/kvm/vmx/pmu_intel.c | 2 ++
 arch/x86/kvm/vmx/vmcs12.c    | 1 +
 arch/x86/kvm/vmx/vmcs12.h    | 3 ++-
 4 files changed, 9 insertions(+), 3 deletions(-)

diff --git a/arch/x86/kvm/vmx/nested.c b/arch/x86/kvm/vmx/nested.c
index 1a52134b0c42..3fa5f9556cd1 100644
--- a/arch/x86/kvm/vmx/nested.c
+++ b/arch/x86/kvm/vmx/nested.c
@@ -6466,7 +6466,8 @@ void nested_vmx_setup_ctls_msrs(struct nested_vmx_msrs *msrs, u32 ept_caps)
 		VM_EXIT_HOST_ADDR_SPACE_SIZE |
 #endif
 		VM_EXIT_LOAD_IA32_PAT | VM_EXIT_SAVE_IA32_PAT |
-		VM_EXIT_CLEAR_BNDCFGS | VM_EXIT_LOAD_IA32_PERF_GLOBAL_CTRL;
+		VM_EXIT_CLEAR_BNDCFGS | VM_EXIT_LOAD_IA32_PERF_GLOBAL_CTRL |
+		VM_EXIT_CLEAR_IA32_LBR_CTL;
 	msrs->exit_ctls_high |=
 		VM_EXIT_ALWAYSON_WITHOUT_TRUE_MSR |
 		VM_EXIT_LOAD_IA32_EFER | VM_EXIT_SAVE_IA32_EFER |
@@ -6486,7 +6487,8 @@ void nested_vmx_setup_ctls_msrs(struct nested_vmx_msrs *msrs, u32 ept_caps)
 		VM_ENTRY_IA32E_MODE |
 #endif
 		VM_ENTRY_LOAD_IA32_PAT | VM_ENTRY_LOAD_BNDCFGS |
-		VM_ENTRY_LOAD_IA32_PERF_GLOBAL_CTRL;
+		VM_ENTRY_LOAD_IA32_PERF_GLOBAL_CTRL |
+		VM_ENTRY_LOAD_IA32_LBR_CTL;
 	msrs->entry_ctls_high |=
 		(VM_ENTRY_ALWAYSON_WITHOUT_TRUE_MSR | VM_ENTRY_LOAD_IA32_EFER);
 
diff --git a/arch/x86/kvm/vmx/pmu_intel.c b/arch/x86/kvm/vmx/pmu_intel.c
index 35bcdc5357ee..1dd0a176a716 100644
--- a/arch/x86/kvm/vmx/pmu_intel.c
+++ b/arch/x86/kvm/vmx/pmu_intel.c
@@ -234,6 +234,8 @@ static bool intel_is_valid_msr(struct kvm_vcpu *vcpu, u32 msr)
 		break;
 	case MSR_ARCH_LBR_DEPTH:
 	case MSR_ARCH_LBR_CTL:
+		if (is_guest_mode(vcpu))
+			break;
 		if (kvm_cpu_cap_has(X86_FEATURE_ARCH_LBR))
 			ret = guest_cpuid_has(vcpu, X86_FEATURE_ARCH_LBR);
 		break;
diff --git a/arch/x86/kvm/vmx/vmcs12.c b/arch/x86/kvm/vmx/vmcs12.c
index d9f5d7c56ae3..c39eda8adf5f 100644
--- a/arch/x86/kvm/vmx/vmcs12.c
+++ b/arch/x86/kvm/vmx/vmcs12.c
@@ -66,6 +66,7 @@ const unsigned short vmcs_field_to_offset_table[] = {
 	FIELD64(HOST_IA32_PAT, host_ia32_pat),
 	FIELD64(HOST_IA32_EFER, host_ia32_efer),
 	FIELD64(HOST_IA32_PERF_GLOBAL_CTRL, host_ia32_perf_global_ctrl),
+	FIELD64(GUEST_IA32_LBR_CTL, guest_lbr_ctl),
 	FIELD(PIN_BASED_VM_EXEC_CONTROL, pin_based_vm_exec_control),
 	FIELD(CPU_BASED_VM_EXEC_CONTROL, cpu_based_vm_exec_control),
 	FIELD(EXCEPTION_BITMAP, exception_bitmap),
diff --git a/arch/x86/kvm/vmx/vmcs12.h b/arch/x86/kvm/vmx/vmcs12.h
index 5e0e1b39f495..c75941425b4e 100644
--- a/arch/x86/kvm/vmx/vmcs12.h
+++ b/arch/x86/kvm/vmx/vmcs12.h
@@ -71,7 +71,7 @@ struct __packed vmcs12 {
 	u64 pml_address;
 	u64 encls_exiting_bitmap;
 	u64 tsc_multiplier;
-	u64 padding64[1]; /* room for future expansion */
+	u64 guest_lbr_ctl;
 	/*
 	 * To allow migration of L1 (complete with its L2 guests) between
 	 * machines of different natural widths (32 or 64 bit), we cannot have
@@ -254,6 +254,7 @@ static inline void vmx_check_vmcs12_offsets(void)
 	CHECK_OFFSET(pml_address, 312);
 	CHECK_OFFSET(encls_exiting_bitmap, 320);
 	CHECK_OFFSET(tsc_multiplier, 328);
+	CHECK_OFFSET(guest_lbr_ctl, 336);
 	CHECK_OFFSET(cr0_guest_host_mask, 344);
 	CHECK_OFFSET(cr4_guest_host_mask, 352);
 	CHECK_OFFSET(cr0_read_shadow, 360);
-- 
2.25.1


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

* [PATCH v7 13/15] KVM: x86/vmx: Clear Arch LBREn bit before inject #DB to guest
  2021-08-06  7:42 [PATCH v7 00/15] Introduce Architectural LBR for vPMU Yang Weijiang
                   ` (11 preceding siblings ...)
  2021-08-06  7:42 ` [PATCH v7 12/15] KVM: nVMX: Add necessary Arch LBR settings for nested VM Yang Weijiang
@ 2021-08-06  7:42 ` Yang Weijiang
  2021-08-09  5:08   ` Like Xu
  2021-08-06  7:42 ` [PATCH v7 14/15] KVM: x86/vmx: Flip Arch LBREn bit on guest state change Yang Weijiang
  2021-08-06  7:42 ` [PATCH v7 15/15] KVM: x86/cpuid: Advise Arch LBR feature in CPUID Yang Weijiang
  14 siblings, 1 reply; 28+ messages in thread
From: Yang Weijiang @ 2021-08-06  7:42 UTC (permalink / raw)
  To: pbonzini, jmattson, seanjc, vkuznets, wei.w.wang, like.xu.linux,
	kvm, linux-kernel
  Cc: Yang Weijiang

Per ISA spec, need to clear the bit before inject #DB.

Signed-off-by: Yang Weijiang <weijiang.yang@intel.com>
---
 arch/x86/kvm/vmx/vmx.c | 21 +++++++++++++++++++++
 1 file changed, 21 insertions(+)

diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c
index 70314cd93340..31b9c06c9b3b 100644
--- a/arch/x86/kvm/vmx/vmx.c
+++ b/arch/x86/kvm/vmx/vmx.c
@@ -1601,6 +1601,21 @@ static void vmx_clear_hlt(struct kvm_vcpu *vcpu)
 		vmcs_write32(GUEST_ACTIVITY_STATE, GUEST_ACTIVITY_ACTIVE);
 }
 
+static void flip_arch_lbr_ctl(struct kvm_vcpu *vcpu, bool on)
+{
+	if (vcpu_to_pmu(vcpu)->event_count > 0 &&
+	    kvm_cpu_cap_has(X86_FEATURE_ARCH_LBR)) {
+		u64 lbr_ctl = vmcs_read64(GUEST_IA32_LBR_CTL);
+
+		if (on)
+			lbr_ctl |= 1ULL;
+		else
+			lbr_ctl &= ~1ULL;
+
+		vmcs_write64(GUEST_IA32_LBR_CTL, lbr_ctl);
+	}
+}
+
 static void vmx_queue_exception(struct kvm_vcpu *vcpu)
 {
 	struct vcpu_vmx *vmx = to_vmx(vcpu);
@@ -1636,6 +1651,9 @@ static void vmx_queue_exception(struct kvm_vcpu *vcpu)
 	vmcs_write32(VM_ENTRY_INTR_INFO_FIELD, intr_info);
 
 	vmx_clear_hlt(vcpu);
+
+	if (nr == DB_VECTOR)
+		flip_arch_lbr_ctl(vcpu, false);
 }
 
 static void vmx_setup_uret_msr(struct vcpu_vmx *vmx, unsigned int msr,
@@ -4572,6 +4590,9 @@ static void vmx_inject_nmi(struct kvm_vcpu *vcpu)
 			INTR_TYPE_NMI_INTR | INTR_INFO_VALID_MASK | NMI_VECTOR);
 
 	vmx_clear_hlt(vcpu);
+
+	if (vcpu->arch.exception.nr == DB_VECTOR)
+		flip_arch_lbr_ctl(vcpu, false);
 }
 
 bool vmx_get_nmi_mask(struct kvm_vcpu *vcpu)
-- 
2.25.1


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

* [PATCH v7 14/15] KVM: x86/vmx: Flip Arch LBREn bit on guest state change
  2021-08-06  7:42 [PATCH v7 00/15] Introduce Architectural LBR for vPMU Yang Weijiang
                   ` (12 preceding siblings ...)
  2021-08-06  7:42 ` [PATCH v7 13/15] KVM: x86/vmx: Clear Arch LBREn bit before inject #DB to guest Yang Weijiang
@ 2021-08-06  7:42 ` Yang Weijiang
  2021-08-06  7:42 ` [PATCH v7 15/15] KVM: x86/cpuid: Advise Arch LBR feature in CPUID Yang Weijiang
  14 siblings, 0 replies; 28+ messages in thread
From: Yang Weijiang @ 2021-08-06  7:42 UTC (permalink / raw)
  To: pbonzini, jmattson, seanjc, vkuznets, wei.w.wang, like.xu.linux,
	kvm, linux-kernel
  Cc: Yang Weijiang

Clear Arch LBREn bit on #SMI and restore it on RSM per ISA spec.,
also clear the bit when guest does warm reset.

Signed-off-by: Yang Weijiang <weijiang.yang@intel.com>
---
 arch/x86/kvm/vmx/vmx.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c
index 31b9c06c9b3b..31969221db8c 100644
--- a/arch/x86/kvm/vmx/vmx.c
+++ b/arch/x86/kvm/vmx/vmx.c
@@ -4513,8 +4513,10 @@ static void vmx_vcpu_reset(struct kvm_vcpu *vcpu, bool init_event)
 	vmx_update_exception_bitmap(vcpu);
 
 	vpid_sync_context(vmx->vpid);
-	if (init_event)
+	if (init_event) {
 		vmx_clear_hlt(vcpu);
+		flip_arch_lbr_ctl(vcpu, false);
+	}
 }
 
 static void vmx_enable_irq_window(struct kvm_vcpu *vcpu)
@@ -7513,6 +7515,7 @@ static int vmx_enter_smm(struct kvm_vcpu *vcpu, char *smstate)
 	vmx->nested.smm.vmxon = vmx->nested.vmxon;
 	vmx->nested.vmxon = false;
 	vmx_clear_hlt(vcpu);
+	flip_arch_lbr_ctl(vcpu, false);
 	return 0;
 }
 
@@ -7533,6 +7536,7 @@ static int vmx_leave_smm(struct kvm_vcpu *vcpu, const char *smstate)
 
 		vmx->nested.smm.guest_mode = false;
 	}
+	flip_arch_lbr_ctl(vcpu, true);
 	return 0;
 }
 
-- 
2.25.1


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

* [PATCH v7 15/15] KVM: x86/cpuid: Advise Arch LBR feature in CPUID
  2021-08-06  7:42 [PATCH v7 00/15] Introduce Architectural LBR for vPMU Yang Weijiang
                   ` (13 preceding siblings ...)
  2021-08-06  7:42 ` [PATCH v7 14/15] KVM: x86/vmx: Flip Arch LBREn bit on guest state change Yang Weijiang
@ 2021-08-06  7:42 ` Yang Weijiang
  14 siblings, 0 replies; 28+ messages in thread
From: Yang Weijiang @ 2021-08-06  7:42 UTC (permalink / raw)
  To: pbonzini, jmattson, seanjc, vkuznets, wei.w.wang, like.xu.linux,
	kvm, linux-kernel
  Cc: Yang Weijiang

Add Arch LBR feature bit in CPU cap-mask to expose the feature.
Currently only max LBR depth is available for guest, and it's
consistent with host Arch LBR settings.

Co-developed-by: Like Xu <like.xu@linux.intel.com>
Signed-off-by: Like Xu <like.xu@linux.intel.com>
Signed-off-by: Yang Weijiang <weijiang.yang@intel.com>
---
 arch/x86/kvm/cpuid.c | 33 ++++++++++++++++++++++++++++++++-
 1 file changed, 32 insertions(+), 1 deletion(-)

diff --git a/arch/x86/kvm/cpuid.c b/arch/x86/kvm/cpuid.c
index 03025eea1524..1ee3014b8977 100644
--- a/arch/x86/kvm/cpuid.c
+++ b/arch/x86/kvm/cpuid.c
@@ -202,6 +202,16 @@ static void kvm_vcpu_after_set_cpuid(struct kvm_vcpu *vcpu)
 		best->ecx |= XFEATURE_MASK_FPSSE;
 	}
 
+	best = kvm_find_cpuid_entry(vcpu, 0x1c, 0);
+	if (best) {
+		unsigned int eax, ebx, ecx, edx;
+
+		/* Make sure guest only sees the supported arch lbr depth. */
+		cpuid_count(0x1c, 0, &eax, &ebx, &ecx, &edx);
+		best->eax &= ~0xff;
+		best->eax |= BIT(fls(eax & 0xff) - 1);
+	}
+
 	kvm_update_pv_runtime(vcpu);
 
 	vcpu->arch.maxphyaddr = cpuid_query_maxphyaddr(vcpu);
@@ -490,7 +500,7 @@ void kvm_set_cpu_caps(void)
 		F(AVX512_4VNNIW) | F(AVX512_4FMAPS) | F(SPEC_CTRL) |
 		F(SPEC_CTRL_SSBD) | F(ARCH_CAPABILITIES) | F(INTEL_STIBP) |
 		F(MD_CLEAR) | F(AVX512_VP2INTERSECT) | F(FSRM) |
-		F(SERIALIZE) | F(TSXLDTRK) | F(AVX512_FP16)
+		F(SERIALIZE) | F(TSXLDTRK) | F(AVX512_FP16) | F(ARCH_LBR)
 	);
 
 	/* TSC_ADJUST and ARCH_CAPABILITIES are emulated in software. */
@@ -903,6 +913,27 @@ static inline int __do_cpuid_func(struct kvm_cpuid_array *array, u32 function)
 				goto out;
 		}
 		break;
+	/* Architectural LBR */
+	case 0x1c: {
+		u32 lbr_depth_mask = entry->eax & 0xff;
+
+		if (!lbr_depth_mask ||
+		    !kvm_cpu_cap_has(X86_FEATURE_ARCH_LBR)) {
+			entry->eax = entry->ebx = entry->ecx = entry->edx = 0;
+			break;
+		}
+		/*
+		 * KVM only exposes the maximum supported depth, which is the
+		 * fixed value used on the host side.
+		 * KVM doesn't allow VMM userspace to adjust LBR depth because
+		 * guest LBR emulation depends on the configuration of host LBR
+		 * driver.
+		 */
+		lbr_depth_mask = BIT((fls(lbr_depth_mask) - 1));
+		entry->eax &= ~0xff;
+		entry->eax |= lbr_depth_mask;
+		break;
+	}
 	case KVM_CPUID_SIGNATURE: {
 		static const char signature[12] = "KVMKVMKVM\0\0";
 		const u32 *sigptr = (const u32 *)signature;
-- 
2.25.1


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

* Re: [PATCH v7 05/15] KVM: vmx/pmu: Emulate MSR_ARCH_LBR_CTL for guest Arch LBR
  2021-08-06  7:42 ` [PATCH v7 05/15] KVM: vmx/pmu: Emulate MSR_ARCH_LBR_CTL " Yang Weijiang
@ 2021-08-06 16:26     ` kernel test robot
  2021-08-09 13:36   ` Like Xu
  1 sibling, 0 replies; 28+ messages in thread
From: kernel test robot @ 2021-08-06 16:26 UTC (permalink / raw)
  To: Yang Weijiang, pbonzini, jmattson, seanjc, vkuznets, wei.w.wang,
	like.xu.linux, kvm, linux-kernel
  Cc: kbuild-all, Like Xu, Yang Weijiang

[-- Attachment #1: Type: text/plain, Size: 4311 bytes --]

Hi Yang,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on v5.14-rc4]
[also build test WARNING on next-20210805]
[cannot apply to kvm/queue tip/perf/core tip/x86/core]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Yang-Weijiang/Introduce-Architectural-LBR-for-vPMU/20210806-153154
base:    c500bee1c5b2f1d59b1081ac879d73268ab0ff17
config: x86_64-rhel-8.3-kselftests (attached as .config)
compiler: gcc-9 (Debian 9.3.0-22) 9.3.0
reproduce:
        # apt-get install sparse
        # sparse version: v0.6.3-348-gf0e6938b-dirty
        # https://github.com/0day-ci/linux/commit/1d5b4967a3c4553c3648638f1189dcbff631328e
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Yang-Weijiang/Introduce-Architectural-LBR-for-vPMU/20210806-153154
        git checkout 1d5b4967a3c4553c3648638f1189dcbff631328e
        # save the attached .config to linux build tree
        make W=1 C=1 CF='-fdiagnostic-prefix -D__CHECK_ENDIAN__' O=build_dir ARCH=x86_64 SHELL=/bin/bash arch/x86/kvm/

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>


sparse warnings: (new ones prefixed by >>)
   arch/x86/kvm/vmx/pmu_intel.c: note: in included file (through arch/x86/kvm/vmx/vmx_ops.h, arch/x86/kvm/vmx/vmx.h, arch/x86/kvm/vmx/nested.h):
>> arch/x86/kvm/vmx/evmcs.h:81:30: sparse: sparse: cast truncates bits from constant value (a058a becomes 58a)
>> arch/x86/kvm/vmx/evmcs.h:81:30: sparse: sparse: cast truncates bits from constant value (a058a becomes 58a)
   arch/x86/kvm/vmx/evmcs.h:81:30: sparse: sparse: cast truncates bits from constant value (a008a becomes 8a)
   arch/x86/kvm/vmx/evmcs.h:81:30: sparse: sparse: cast truncates bits from constant value (a008a becomes 8a)
   arch/x86/kvm/vmx/evmcs.h:81:30: sparse: sparse: cast truncates bits from constant value (a008a becomes 8a)

vim +81 arch/x86/kvm/vmx/evmcs.h

75edce8a45486f arch/x86/kvm/vmx/evmcs.h Sean Christopherson 2018-12-03  77  
773e8a0425c923 arch/x86/kvm/vmx_evmcs.h Vitaly Kuznetsov    2018-03-20  78  static __always_inline int get_evmcs_offset(unsigned long field,
773e8a0425c923 arch/x86/kvm/vmx_evmcs.h Vitaly Kuznetsov    2018-03-20  79  					    u16 *clean_field)
773e8a0425c923 arch/x86/kvm/vmx_evmcs.h Vitaly Kuznetsov    2018-03-20  80  {
773e8a0425c923 arch/x86/kvm/vmx_evmcs.h Vitaly Kuznetsov    2018-03-20 @81  	unsigned int index = ROL16(field, 6);
773e8a0425c923 arch/x86/kvm/vmx_evmcs.h Vitaly Kuznetsov    2018-03-20  82  	const struct evmcs_field *evmcs_field;
773e8a0425c923 arch/x86/kvm/vmx_evmcs.h Vitaly Kuznetsov    2018-03-20  83  
75edce8a45486f arch/x86/kvm/vmx/evmcs.h Sean Christopherson 2018-12-03  84  	if (unlikely(index >= nr_evmcs_1_fields)) {
773e8a0425c923 arch/x86/kvm/vmx_evmcs.h Vitaly Kuznetsov    2018-03-20  85  		WARN_ONCE(1, "KVM: accessing unsupported EVMCS field %lx\n",
773e8a0425c923 arch/x86/kvm/vmx_evmcs.h Vitaly Kuznetsov    2018-03-20  86  			  field);
773e8a0425c923 arch/x86/kvm/vmx_evmcs.h Vitaly Kuznetsov    2018-03-20  87  		return -ENOENT;
773e8a0425c923 arch/x86/kvm/vmx_evmcs.h Vitaly Kuznetsov    2018-03-20  88  	}
773e8a0425c923 arch/x86/kvm/vmx_evmcs.h Vitaly Kuznetsov    2018-03-20  89  
773e8a0425c923 arch/x86/kvm/vmx_evmcs.h Vitaly Kuznetsov    2018-03-20  90  	evmcs_field = &vmcs_field_to_evmcs_1[index];
773e8a0425c923 arch/x86/kvm/vmx_evmcs.h Vitaly Kuznetsov    2018-03-20  91  
773e8a0425c923 arch/x86/kvm/vmx_evmcs.h Vitaly Kuznetsov    2018-03-20  92  	if (clean_field)
773e8a0425c923 arch/x86/kvm/vmx_evmcs.h Vitaly Kuznetsov    2018-03-20  93  		*clean_field = evmcs_field->clean_field;
773e8a0425c923 arch/x86/kvm/vmx_evmcs.h Vitaly Kuznetsov    2018-03-20  94  
773e8a0425c923 arch/x86/kvm/vmx_evmcs.h Vitaly Kuznetsov    2018-03-20  95  	return evmcs_field->offset;
773e8a0425c923 arch/x86/kvm/vmx_evmcs.h Vitaly Kuznetsov    2018-03-20  96  }
773e8a0425c923 arch/x86/kvm/vmx_evmcs.h Vitaly Kuznetsov    2018-03-20  97  

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 42142 bytes --]

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

* Re: [PATCH v7 05/15] KVM: vmx/pmu: Emulate MSR_ARCH_LBR_CTL for guest Arch LBR
@ 2021-08-06 16:26     ` kernel test robot
  0 siblings, 0 replies; 28+ messages in thread
From: kernel test robot @ 2021-08-06 16:26 UTC (permalink / raw)
  To: kbuild-all

[-- Attachment #1: Type: text/plain, Size: 4377 bytes --]

Hi Yang,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on v5.14-rc4]
[also build test WARNING on next-20210805]
[cannot apply to kvm/queue tip/perf/core tip/x86/core]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Yang-Weijiang/Introduce-Architectural-LBR-for-vPMU/20210806-153154
base:    c500bee1c5b2f1d59b1081ac879d73268ab0ff17
config: x86_64-rhel-8.3-kselftests (attached as .config)
compiler: gcc-9 (Debian 9.3.0-22) 9.3.0
reproduce:
        # apt-get install sparse
        # sparse version: v0.6.3-348-gf0e6938b-dirty
        # https://github.com/0day-ci/linux/commit/1d5b4967a3c4553c3648638f1189dcbff631328e
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Yang-Weijiang/Introduce-Architectural-LBR-for-vPMU/20210806-153154
        git checkout 1d5b4967a3c4553c3648638f1189dcbff631328e
        # save the attached .config to linux build tree
        make W=1 C=1 CF='-fdiagnostic-prefix -D__CHECK_ENDIAN__' O=build_dir ARCH=x86_64 SHELL=/bin/bash arch/x86/kvm/

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>


sparse warnings: (new ones prefixed by >>)
   arch/x86/kvm/vmx/pmu_intel.c: note: in included file (through arch/x86/kvm/vmx/vmx_ops.h, arch/x86/kvm/vmx/vmx.h, arch/x86/kvm/vmx/nested.h):
>> arch/x86/kvm/vmx/evmcs.h:81:30: sparse: sparse: cast truncates bits from constant value (a058a becomes 58a)
>> arch/x86/kvm/vmx/evmcs.h:81:30: sparse: sparse: cast truncates bits from constant value (a058a becomes 58a)
   arch/x86/kvm/vmx/evmcs.h:81:30: sparse: sparse: cast truncates bits from constant value (a008a becomes 8a)
   arch/x86/kvm/vmx/evmcs.h:81:30: sparse: sparse: cast truncates bits from constant value (a008a becomes 8a)
   arch/x86/kvm/vmx/evmcs.h:81:30: sparse: sparse: cast truncates bits from constant value (a008a becomes 8a)

vim +81 arch/x86/kvm/vmx/evmcs.h

75edce8a45486f arch/x86/kvm/vmx/evmcs.h Sean Christopherson 2018-12-03  77  
773e8a0425c923 arch/x86/kvm/vmx_evmcs.h Vitaly Kuznetsov    2018-03-20  78  static __always_inline int get_evmcs_offset(unsigned long field,
773e8a0425c923 arch/x86/kvm/vmx_evmcs.h Vitaly Kuznetsov    2018-03-20  79  					    u16 *clean_field)
773e8a0425c923 arch/x86/kvm/vmx_evmcs.h Vitaly Kuznetsov    2018-03-20  80  {
773e8a0425c923 arch/x86/kvm/vmx_evmcs.h Vitaly Kuznetsov    2018-03-20 @81  	unsigned int index = ROL16(field, 6);
773e8a0425c923 arch/x86/kvm/vmx_evmcs.h Vitaly Kuznetsov    2018-03-20  82  	const struct evmcs_field *evmcs_field;
773e8a0425c923 arch/x86/kvm/vmx_evmcs.h Vitaly Kuznetsov    2018-03-20  83  
75edce8a45486f arch/x86/kvm/vmx/evmcs.h Sean Christopherson 2018-12-03  84  	if (unlikely(index >= nr_evmcs_1_fields)) {
773e8a0425c923 arch/x86/kvm/vmx_evmcs.h Vitaly Kuznetsov    2018-03-20  85  		WARN_ONCE(1, "KVM: accessing unsupported EVMCS field %lx\n",
773e8a0425c923 arch/x86/kvm/vmx_evmcs.h Vitaly Kuznetsov    2018-03-20  86  			  field);
773e8a0425c923 arch/x86/kvm/vmx_evmcs.h Vitaly Kuznetsov    2018-03-20  87  		return -ENOENT;
773e8a0425c923 arch/x86/kvm/vmx_evmcs.h Vitaly Kuznetsov    2018-03-20  88  	}
773e8a0425c923 arch/x86/kvm/vmx_evmcs.h Vitaly Kuznetsov    2018-03-20  89  
773e8a0425c923 arch/x86/kvm/vmx_evmcs.h Vitaly Kuznetsov    2018-03-20  90  	evmcs_field = &vmcs_field_to_evmcs_1[index];
773e8a0425c923 arch/x86/kvm/vmx_evmcs.h Vitaly Kuznetsov    2018-03-20  91  
773e8a0425c923 arch/x86/kvm/vmx_evmcs.h Vitaly Kuznetsov    2018-03-20  92  	if (clean_field)
773e8a0425c923 arch/x86/kvm/vmx_evmcs.h Vitaly Kuznetsov    2018-03-20  93  		*clean_field = evmcs_field->clean_field;
773e8a0425c923 arch/x86/kvm/vmx_evmcs.h Vitaly Kuznetsov    2018-03-20  94  
773e8a0425c923 arch/x86/kvm/vmx_evmcs.h Vitaly Kuznetsov    2018-03-20  95  	return evmcs_field->offset;
773e8a0425c923 arch/x86/kvm/vmx_evmcs.h Vitaly Kuznetsov    2018-03-20  96  }
773e8a0425c923 arch/x86/kvm/vmx_evmcs.h Vitaly Kuznetsov    2018-03-20  97  

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org

[-- Attachment #2: config.gz --]
[-- Type: application/gzip, Size: 42142 bytes --]

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

* Re: [PATCH v7 13/15] KVM: x86/vmx: Clear Arch LBREn bit before inject #DB to guest
  2021-08-06  7:42 ` [PATCH v7 13/15] KVM: x86/vmx: Clear Arch LBREn bit before inject #DB to guest Yang Weijiang
@ 2021-08-09  5:08   ` Like Xu
  2021-08-09  9:02     ` Yang Weijiang
  0 siblings, 1 reply; 28+ messages in thread
From: Like Xu @ 2021-08-09  5:08 UTC (permalink / raw)
  To: Yang Weijiang
  Cc: pbonzini, jmattson, seanjc, vkuznets, wei.w.wang, kvm, linux-kernel

On 6/8/2021 3:42 pm, Yang Weijiang wrote:
> Per ISA spec, need to clear the bit before inject #DB.

Please paste the SDM statement accurately so that the reviewers
can verify that the code is consistent with the documentation.

> 
> Signed-off-by: Yang Weijiang <weijiang.yang@intel.com>
> ---
>   arch/x86/kvm/vmx/vmx.c | 21 +++++++++++++++++++++
>   1 file changed, 21 insertions(+)
> 
> diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c
> index 70314cd93340..31b9c06c9b3b 100644
> --- a/arch/x86/kvm/vmx/vmx.c
> +++ b/arch/x86/kvm/vmx/vmx.c
> @@ -1601,6 +1601,21 @@ static void vmx_clear_hlt(struct kvm_vcpu *vcpu)
>   		vmcs_write32(GUEST_ACTIVITY_STATE, GUEST_ACTIVITY_ACTIVE);
>   }
>   
> +static void flip_arch_lbr_ctl(struct kvm_vcpu *vcpu, bool on)
> +{
> +	if (vcpu_to_pmu(vcpu)->event_count > 0 &&

Ugh, this check seems ridiculous/funny to me.

> +	    kvm_cpu_cap_has(X86_FEATURE_ARCH_LBR)) {
> +		u64 lbr_ctl = vmcs_read64(GUEST_IA32_LBR_CTL);
> +
> +		if (on)
> +			lbr_ctl |= 1ULL;
> +		else
> +			lbr_ctl &= ~1ULL;
> +
> +		vmcs_write64(GUEST_IA32_LBR_CTL, lbr_ctl);
> +	}
> +}

...

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

* Re: [PATCH v7 13/15] KVM: x86/vmx: Clear Arch LBREn bit before inject #DB to guest
  2021-08-09  5:08   ` Like Xu
@ 2021-08-09  9:02     ` Yang Weijiang
  0 siblings, 0 replies; 28+ messages in thread
From: Yang Weijiang @ 2021-08-09  9:02 UTC (permalink / raw)
  To: Like Xu
  Cc: Yang Weijiang, pbonzini, jmattson, seanjc, vkuznets, wei.w.wang,
	kvm, linux-kernel

On Mon, Aug 09, 2021 at 01:08:32PM +0800, Like Xu wrote:
> On 6/8/2021 3:42 pm, Yang Weijiang wrote:
> >Per ISA spec, need to clear the bit before inject #DB.
> 
> Please paste the SDM statement accurately so that the reviewers
> can verify that the code is consistent with the documentation.
>
Thanks Like! Sure, will add the description in commit message.

> >
> >Signed-off-by: Yang Weijiang <weijiang.yang@intel.com>
> >---
> >  arch/x86/kvm/vmx/vmx.c | 21 +++++++++++++++++++++
> >  1 file changed, 21 insertions(+)
> >
> >diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c
> >index 70314cd93340..31b9c06c9b3b 100644
> >--- a/arch/x86/kvm/vmx/vmx.c
> >+++ b/arch/x86/kvm/vmx/vmx.c
> >@@ -1601,6 +1601,21 @@ static void vmx_clear_hlt(struct kvm_vcpu *vcpu)
> >  		vmcs_write32(GUEST_ACTIVITY_STATE, GUEST_ACTIVITY_ACTIVE);
> >  }
> >+static void flip_arch_lbr_ctl(struct kvm_vcpu *vcpu, bool on)
> >+{
> >+	if (vcpu_to_pmu(vcpu)->event_count > 0 &&
> 
> Ugh, this check seems ridiculous/funny to me.
Do you expect aditional bit-check for INTEL_PMC_IDX_FIXED_VLBR in 
pmu->pmc_in_use?

> 
> >+	    kvm_cpu_cap_has(X86_FEATURE_ARCH_LBR)) {
> >+		u64 lbr_ctl = vmcs_read64(GUEST_IA32_LBR_CTL);
> >+
> >+		if (on)
> >+			lbr_ctl |= 1ULL;
> >+		else
> >+			lbr_ctl &= ~1ULL;
> >+
> >+		vmcs_write64(GUEST_IA32_LBR_CTL, lbr_ctl);
> >+	}
> >+}
> 
> ...

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

* Re: [PATCH v7 03/15] KVM: x86: Add Arch LBR MSRs to msrs_to_save_all list
  2021-08-06  7:42 ` [PATCH v7 03/15] KVM: x86: Add Arch LBR MSRs to msrs_to_save_all list Yang Weijiang
@ 2021-08-09 13:07   ` Like Xu
  0 siblings, 0 replies; 28+ messages in thread
From: Like Xu @ 2021-08-09 13:07 UTC (permalink / raw)
  To: Yang Weijiang, Jim Mattson
  Cc: pbonzini, seanjc, vkuznets, wei.w.wang, kvm, linux-kernel

On 6/8/2021 3:42 pm, Yang Weijiang wrote:
> Arch LBR MSR_ARCH_LBR_DEPTH and MSR_ARCH_LBR_CTL are {saved|restored}
> by userspace application if they're available.
> 
> Suggested-by: Jim Mattson <jmattson@google.com>
> Signed-off-by: Yang Weijiang <weijiang.yang@intel.com>
> Reviewed-by: Jim Mattson <jmattson@google.com>
> ---
>   arch/x86/kvm/x86.c | 6 ++++++
>   1 file changed, 6 insertions(+)
> 
> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> index 4116567f3d44..4ae173ab2208 100644
> --- a/arch/x86/kvm/x86.c
> +++ b/arch/x86/kvm/x86.c
> @@ -1327,6 +1327,7 @@ static const u32 msrs_to_save_all[] = {
>   	MSR_ARCH_PERFMON_EVENTSEL0 + 12, MSR_ARCH_PERFMON_EVENTSEL0 + 13,
>   	MSR_ARCH_PERFMON_EVENTSEL0 + 14, MSR_ARCH_PERFMON_EVENTSEL0 + 15,
>   	MSR_ARCH_PERFMON_EVENTSEL0 + 16, MSR_ARCH_PERFMON_EVENTSEL0 + 17,
> +	MSR_ARCH_LBR_CTL, MSR_ARCH_LBR_DEPTH,

Hi Jim,

Do you have a particular reason for not putting the set of
MSR_ARCH_LBR_{FROM, TO, INFO} into msrs_to_save_all[]
for {saved|restored} by user space ?

>   };
>   
>   static u32 msrs_to_save[ARRAY_SIZE(msrs_to_save_all)];
> @@ -6229,6 +6230,11 @@ static void kvm_init_msr_list(void)
>   			    min(INTEL_PMC_MAX_GENERIC, x86_pmu.num_counters_gp))
>   				continue;
>   			break;
> +		case MSR_ARCH_LBR_DEPTH:
> +		case MSR_ARCH_LBR_CTL:
> +			if (!kvm_cpu_cap_has(X86_FEATURE_ARCH_LBR))
> +				continue;
> +			break;
>   		default:
>   			break;
>   		}
> 

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

* Re: [PATCH v7 04/15] KVM: vmx/pmu: Emulate MSR_ARCH_LBR_DEPTH for guest Arch LBR
  2021-08-06  7:42 ` [PATCH v7 04/15] KVM: vmx/pmu: Emulate MSR_ARCH_LBR_DEPTH for guest Arch LBR Yang Weijiang
@ 2021-08-09 13:16   ` Like Xu
  2021-08-10  7:38     ` Yang Weijiang
  0 siblings, 1 reply; 28+ messages in thread
From: Like Xu @ 2021-08-09 13:16 UTC (permalink / raw)
  To: Yang Weijiang
  Cc: pbonzini, jmattson, seanjc, vkuznets, wei.w.wang, kvm, linux-kernel

On 6/8/2021 3:42 pm, Yang Weijiang wrote:
> From: Like Xu <like.xu@linux.intel.com>

...

> 
> The number of Arch LBR entries available is determined by the value
> in host MSR_ARCH_LBR_DEPTH.DEPTH. The supported LBR depth values are
> enumerated in CPUID.(EAX=01CH, ECX=0):EAX[7:0]. For each bit "n" set
> in this field, the MSR_ARCH_LBR_DEPTH.DEPTH value of "8*(n+1)" is
> supported.
> 
> On a guest write to MSR_ARCH_LBR_DEPTH, all LBR entries are reset to 0.
> KVM writes guest requested value to the native ARCH_LBR_DEPTH MSR
> (this is safe because the two values will be the same) when the Arch LBR
> records MSRs are pass-through to the guest.
> 
> Signed-off-by: Like Xu <like.xu@linux.intel.com>
> Signed-off-by: Yang Weijiang <weijiang.yang@intel.com>
> ---
>   arch/x86/kvm/vmx/pmu_intel.c | 35 ++++++++++++++++++++++++++++++++++-
>   1 file changed, 34 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/x86/kvm/vmx/pmu_intel.c b/arch/x86/kvm/vmx/pmu_intel.c
> index 9efc1a6b8693..a4ef5bbce186 100644
> --- a/arch/x86/kvm/vmx/pmu_intel.c
> +++ b/arch/x86/kvm/vmx/pmu_intel.c
> @@ -211,7 +211,7 @@ static bool intel_pmu_is_valid_lbr_msr(struct kvm_vcpu *vcpu, u32 index)
>   static bool intel_is_valid_msr(struct kvm_vcpu *vcpu, u32 msr)
>   {
>   	struct kvm_pmu *pmu = vcpu_to_pmu(vcpu);
> -	int ret;
> +	int ret = 0;
>   
>   	switch (msr) {
>   	case MSR_CORE_PERF_FIXED_CTR_CTRL:
> @@ -220,6 +220,10 @@ static bool intel_is_valid_msr(struct kvm_vcpu *vcpu, u32 msr)
>   	case MSR_CORE_PERF_GLOBAL_OVF_CTRL:
>   		ret = pmu->version > 1;
>   		break;
> +	case MSR_ARCH_LBR_DEPTH:
> +		if (kvm_cpu_cap_has(X86_FEATURE_ARCH_LBR))
> +			ret = guest_cpuid_has(vcpu, X86_FEATURE_ARCH_LBR);
> +		break;
>   	default:
>   		ret = get_gp_pmc(pmu, msr, MSR_IA32_PERFCTR0) ||
>   			get_gp_pmc(pmu, msr, MSR_P6_EVNTSEL0) ||
> @@ -348,10 +352,28 @@ static bool intel_pmu_handle_lbr_msrs_access(struct kvm_vcpu *vcpu,
>   	return true;
>   }
>   
> +/*
> + * Check if the requested depth value the same as that of host.
> + * When guest/host depth are different, the handling would be tricky,
> + * so now only max depth is supported for both host and guest.
> + */
> +static bool arch_lbr_depth_is_valid(struct kvm_vcpu *vcpu, u64 depth)
> +{
> +	unsigned int eax, ebx, ecx, edx;
> +
> +	if (!kvm_cpu_cap_has(X86_FEATURE_ARCH_LBR))
> +		return false;
> +
> +	cpuid_count(0x1c, 0, &eax, &ebx, &ecx, &edx);

I really don't understand why the sanity check of the
guest lbr depth needs to read the host's cpuid entry and it's pretty slow.

KVM has reported the maximum host LBR depth as the only supported value.

> +
> +	return (depth == fls(eax & 0xff) * 8);
> +}
> +
>   static int intel_pmu_get_msr(struct kvm_vcpu *vcpu, struct msr_data *msr_info)
>   {
>   	struct kvm_pmu *pmu = vcpu_to_pmu(vcpu);
>   	struct kvm_pmc *pmc;
> +	struct lbr_desc *lbr_desc = vcpu_to_lbr_desc(vcpu);
>   	u32 msr = msr_info->index;
>   
>   	switch (msr) {
> @@ -367,6 +389,9 @@ static int intel_pmu_get_msr(struct kvm_vcpu *vcpu, struct msr_data *msr_info)
>   	case MSR_CORE_PERF_GLOBAL_OVF_CTRL:
>   		msr_info->data = pmu->global_ovf_ctrl;
>   		return 0;
> +	case MSR_ARCH_LBR_DEPTH:
> +		msr_info->data = lbr_desc->records.nr;
> +		return 0;
>   	default:
>   		if ((pmc = get_gp_pmc(pmu, msr, MSR_IA32_PERFCTR0)) ||
>   		    (pmc = get_gp_pmc(pmu, msr, MSR_IA32_PMC0))) {
> @@ -393,6 +418,7 @@ static int intel_pmu_set_msr(struct kvm_vcpu *vcpu, struct msr_data *msr_info)
>   {
>   	struct kvm_pmu *pmu = vcpu_to_pmu(vcpu);
>   	struct kvm_pmc *pmc;
> +	struct lbr_desc *lbr_desc = vcpu_to_lbr_desc(vcpu);
>   	u32 msr = msr_info->index;
>   	u64 data = msr_info->data;
>   
> @@ -427,6 +453,13 @@ static int intel_pmu_set_msr(struct kvm_vcpu *vcpu, struct msr_data *msr_info)
>   			return 0;
>   		}
>   		break;
> +	case MSR_ARCH_LBR_DEPTH:
> +		if (!arch_lbr_depth_is_valid(vcpu, data))
> +			return 1;
> +		lbr_desc->records.nr = data;
> +		if (!msr_info->host_initiated)
> +			wrmsrl(MSR_ARCH_LBR_DEPTH, lbr_desc->records.nr);

Resetting the host msr here is dangerous,
what if the guest LBR event doesn't exist or isn't scheduled on?

> +		return 0;
>   	default:
>   		if ((pmc = get_gp_pmc(pmu, msr, MSR_IA32_PERFCTR0)) ||
>   		    (pmc = get_gp_pmc(pmu, msr, MSR_IA32_PMC0))) {
> 

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

* Re: [PATCH v7 05/15] KVM: vmx/pmu: Emulate MSR_ARCH_LBR_CTL for guest Arch LBR
  2021-08-06  7:42 ` [PATCH v7 05/15] KVM: vmx/pmu: Emulate MSR_ARCH_LBR_CTL " Yang Weijiang
  2021-08-06 16:26     ` kernel test robot
@ 2021-08-09 13:36   ` Like Xu
  2021-08-10  8:30     ` Yang Weijiang
  1 sibling, 1 reply; 28+ messages in thread
From: Like Xu @ 2021-08-09 13:36 UTC (permalink / raw)
  To: Yang Weijiang
  Cc: pbonzini, jmattson, seanjc, vkuznets, wei.w.wang, kvm, linux-kernel

On 6/8/2021 3:42 pm, Yang Weijiang wrote:
> From: Like Xu <like.xu@linux.intel.com>
> 
> Arch LBRs are enabled by setting MSR_ARCH_LBR_CTL.LBREn to 1. A new guest
> state field named "Guest IA32_LBR_CTL" is added to enhance guest LBR usage.
> When guest Arch LBR is enabled, a guest LBR event will be created like the
> model-specific LBR does. Clear guest LBR enable bit on host PMI handling so
> guest can see expected config.
> 
> On processors that support Arch LBR, MSR_IA32_DEBUGCTLMSR[bit 0] has no
> meaning. It can be written to 0 or 1, but reads will always return 0.
> Like IA32_DEBUGCTL, IA32_ARCH_LBR_CTL msr is also preserved on INIT.
> 
> Regardless of the Arch LBR or legacy LBR, when the LBR_EN bit 0 of the
> corresponding control MSR is set to 1, LBR recording will be enabled.
> 
> Signed-off-by: Like Xu <like.xu@linux.intel.com>
> Signed-off-by: Yang Weijiang <weijiang.yang@intel.com>
> ---
>   arch/x86/events/intel/lbr.c      |  2 --
>   arch/x86/include/asm/msr-index.h |  1 +
>   arch/x86/include/asm/vmx.h       |  2 ++
>   arch/x86/kvm/vmx/pmu_intel.c     | 48 ++++++++++++++++++++++++++++----
>   arch/x86/kvm/vmx/vmx.c           |  9 ++++++
>   5 files changed, 55 insertions(+), 7 deletions(-)
> 
> diff --git a/arch/x86/events/intel/lbr.c b/arch/x86/events/intel/lbr.c
> index ceff16eb4bcb..d6773a200c70 100644
> --- a/arch/x86/events/intel/lbr.c
> +++ b/arch/x86/events/intel/lbr.c
> @@ -168,8 +168,6 @@ enum {
>   	 ARCH_LBR_RETURN		|\
>   	 ARCH_LBR_OTHER_BRANCH)
>   
> -#define ARCH_LBR_CTL_MASK			0x7f000e
> -
>   static void intel_pmu_lbr_filter(struct cpu_hw_events *cpuc);
>   
>   static __always_inline bool is_lbr_call_stack_bit_set(u64 config)
> diff --git a/arch/x86/include/asm/msr-index.h b/arch/x86/include/asm/msr-index.h
> index a7c413432b33..04059e8ed115 100644
> --- a/arch/x86/include/asm/msr-index.h
> +++ b/arch/x86/include/asm/msr-index.h
> @@ -169,6 +169,7 @@
>   #define LBR_INFO_BR_TYPE		(0xfull << LBR_INFO_BR_TYPE_OFFSET)
>   
>   #define MSR_ARCH_LBR_CTL		0x000014ce
> +#define ARCH_LBR_CTL_MASK		0x7f000e
>   #define ARCH_LBR_CTL_LBREN		BIT(0)
>   #define ARCH_LBR_CTL_CPL_OFFSET		1
>   #define ARCH_LBR_CTL_CPL		(0x3ull << ARCH_LBR_CTL_CPL_OFFSET)
> diff --git a/arch/x86/include/asm/vmx.h b/arch/x86/include/asm/vmx.h
> index 0ffaa3156a4e..ea3be961cc8e 100644
> --- a/arch/x86/include/asm/vmx.h
> +++ b/arch/x86/include/asm/vmx.h
> @@ -245,6 +245,8 @@ enum vmcs_field {
>   	GUEST_BNDCFGS_HIGH              = 0x00002813,
>   	GUEST_IA32_RTIT_CTL		= 0x00002814,
>   	GUEST_IA32_RTIT_CTL_HIGH	= 0x00002815,
> +	GUEST_IA32_LBR_CTL		= 0x00002816,
> +	GUEST_IA32_LBR_CTL_HIGH		= 0x00002817,
>   	HOST_IA32_PAT			= 0x00002c00,
>   	HOST_IA32_PAT_HIGH		= 0x00002c01,
>   	HOST_IA32_EFER			= 0x00002c02,
> diff --git a/arch/x86/kvm/vmx/pmu_intel.c b/arch/x86/kvm/vmx/pmu_intel.c
> index a4ef5bbce186..b2631fea5e6c 100644
> --- a/arch/x86/kvm/vmx/pmu_intel.c
> +++ b/arch/x86/kvm/vmx/pmu_intel.c
> @@ -19,6 +19,7 @@
>   #include "pmu.h"
>   
>   #define MSR_PMC_FULL_WIDTH_BIT      (MSR_IA32_PMC0 - MSR_IA32_PERFCTR0)
> +#define KVM_ARCH_LBR_CTL_MASK  (ARCH_LBR_CTL_MASK | ARCH_LBR_CTL_LBREN)
>   
>   static struct kvm_event_hw_type_mapping intel_arch_events[] = {
>   	/* Index must match CPUID 0x0A.EBX bit vector */
> @@ -221,6 +222,7 @@ static bool intel_is_valid_msr(struct kvm_vcpu *vcpu, u32 msr)
>   		ret = pmu->version > 1;
>   		break;
>   	case MSR_ARCH_LBR_DEPTH:
> +	case MSR_ARCH_LBR_CTL:
>   		if (kvm_cpu_cap_has(X86_FEATURE_ARCH_LBR))
>   			ret = guest_cpuid_has(vcpu, X86_FEATURE_ARCH_LBR);
>   		break;
> @@ -369,6 +371,26 @@ static bool arch_lbr_depth_is_valid(struct kvm_vcpu *vcpu, u64 depth)
>   	return (depth == fls(eax & 0xff) * 8);
>   }
>   
> +#define ARCH_LBR_CTL_BRN_MASK   GENMASK_ULL(22, 16)
> +
> +static bool arch_lbr_ctl_is_valid(struct kvm_vcpu *vcpu, u64 ctl)
> +{
> +	unsigned int eax, ebx, ecx, edx;
> +
> +	if (!kvm_cpu_cap_has(X86_FEATURE_ARCH_LBR))
> +		return false;
> +
> +	cpuid_count(0x1c, 0, &eax, &ebx, &ecx, &edx);
> +	if (!(ebx & BIT(0)) && (ctl & ARCH_LBR_CTL_CPL))
> +		return false;
> +	if (!(ebx & BIT(2)) && (ctl & ARCH_LBR_CTL_STACK))
> +		return false;
> +	if (!(ebx & BIT(1)) && (ctl & ARCH_LBR_CTL_BRN_MASK))
> +		return false;
> +
> +	return !(ctl & ~KVM_ARCH_LBR_CTL_MASK);
> +}

Please check it with the *guest* cpuid entry.

And it should remove the bits that are not supported by x86_pmu.lbr_ctl_mask before
vmcs_write64(...) if the guest value is a superset of the host value with 
warning message.

> +
>   static int intel_pmu_get_msr(struct kvm_vcpu *vcpu, struct msr_data *msr_info)
>   {
>   	struct kvm_pmu *pmu = vcpu_to_pmu(vcpu);
> @@ -392,6 +414,9 @@ static int intel_pmu_get_msr(struct kvm_vcpu *vcpu, struct msr_data *msr_info)
>   	case MSR_ARCH_LBR_DEPTH:
>   		msr_info->data = lbr_desc->records.nr;
>   		return 0;
> +	case MSR_ARCH_LBR_CTL:
> +		msr_info->data = vmcs_read64(GUEST_IA32_LBR_CTL);
> +		return 0;
>   	default:
>   		if ((pmc = get_gp_pmc(pmu, msr, MSR_IA32_PERFCTR0)) ||
>   		    (pmc = get_gp_pmc(pmu, msr, MSR_IA32_PMC0))) {
> @@ -460,6 +485,15 @@ static int intel_pmu_set_msr(struct kvm_vcpu *vcpu, struct msr_data *msr_info)
>   		if (!msr_info->host_initiated)
>   			wrmsrl(MSR_ARCH_LBR_DEPTH, lbr_desc->records.nr);
>   		return 0;
> +	case MSR_ARCH_LBR_CTL:
> +		if (!arch_lbr_ctl_is_valid(vcpu, data))
> +			break;
> +
> +		vmcs_write64(GUEST_IA32_LBR_CTL, data);
> +		if (intel_pmu_lbr_is_enabled(vcpu) && !lbr_desc->event &&
> +		    (data & ARCH_LBR_CTL_LBREN))
> +			intel_pmu_create_guest_lbr_event(vcpu);
> +		return 0;
>   	default:
>   		if ((pmc = get_gp_pmc(pmu, msr, MSR_IA32_PERFCTR0)) ||
>   		    (pmc = get_gp_pmc(pmu, msr, MSR_IA32_PMC0))) {
> @@ -637,12 +671,16 @@ static void intel_pmu_reset(struct kvm_vcpu *vcpu)
>    */
>   static void intel_pmu_legacy_freezing_lbrs_on_pmi(struct kvm_vcpu *vcpu)
>   {
> -	u64 data = vmcs_read64(GUEST_IA32_DEBUGCTL);
> +	u32 lbr_ctl_field = GUEST_IA32_DEBUGCTL;
>   
> -	if (data & DEBUGCTLMSR_FREEZE_LBRS_ON_PMI) {
> -		data &= ~DEBUGCTLMSR_LBR;
> -		vmcs_write64(GUEST_IA32_DEBUGCTL, data);
> -	}
> +	if (!(vmcs_read64(GUEST_IA32_DEBUGCTL) & DEBUGCTLMSR_FREEZE_LBRS_ON_PMI))
> +		return;
> +
> +	if (kvm_cpu_cap_has(X86_FEATURE_ARCH_LBR) &&
> +	    guest_cpuid_has(vcpu, X86_FEATURE_ARCH_LBR))
> +		lbr_ctl_field = GUEST_IA32_LBR_CTL;
> +
> +	vmcs_write64(lbr_ctl_field, vmcs_read64(lbr_ctl_field) & ~0x1ULL);
>   }
>   
>   static void intel_pmu_deliver_pmi(struct kvm_vcpu *vcpu)
> diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c
> index 927a552393b9..e8df44faf900 100644
> --- a/arch/x86/kvm/vmx/vmx.c
> +++ b/arch/x86/kvm/vmx/vmx.c
> @@ -2002,6 +2002,13 @@ static int vmx_set_msr(struct kvm_vcpu *vcpu, struct msr_data *msr_info)
>   						VM_EXIT_SAVE_DEBUG_CONTROLS)
>   			get_vmcs12(vcpu)->guest_ia32_debugctl = data;
>   
> +		/*
> +		 * For Arch LBR, IA32_DEBUGCTL[bit 0] has no meaning.
> +		 * It can be written to 0 or 1, but reads will always return 0.
> +		 */
> +		if (guest_cpuid_has(vcpu, X86_FEATURE_ARCH_LBR))
> +			data &= ~DEBUGCTLMSR_LBR;
> +
>   		vmcs_write64(GUEST_IA32_DEBUGCTL, data);
>   		if (intel_pmu_lbr_is_enabled(vcpu) && !to_vmx(vcpu)->lbr_desc.event &&
>   		    (data & DEBUGCTLMSR_LBR))
> @@ -4441,6 +4448,8 @@ static void vmx_vcpu_reset(struct kvm_vcpu *vcpu, bool init_event)
>   		vmcs_writel(GUEST_SYSENTER_ESP, 0);
>   		vmcs_writel(GUEST_SYSENTER_EIP, 0);
>   		vmcs_write64(GUEST_IA32_DEBUGCTL, 0);
> +		if (static_cpu_has(X86_FEATURE_ARCH_LBR))
> +			vmcs_write64(GUEST_IA32_LBR_CTL, 0);

Please update dump_vmcs() to dump GUEST_IA32_LBR_CTL as well.

How about update the load_vmcs12_host_state() for GUEST_IA32_LBR_CTL
since you enabled the nested case in this patch set ?

>   	}
>   
>   	kvm_set_rflags(vcpu, X86_EFLAGS_FIXED);
> 

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

* Re: [PATCH v7 04/15] KVM: vmx/pmu: Emulate MSR_ARCH_LBR_DEPTH for guest Arch LBR
  2021-08-09 13:16   ` Like Xu
@ 2021-08-10  7:38     ` Yang Weijiang
  2021-08-10  7:54       ` Like Xu
  0 siblings, 1 reply; 28+ messages in thread
From: Yang Weijiang @ 2021-08-10  7:38 UTC (permalink / raw)
  To: Like Xu
  Cc: Yang Weijiang, pbonzini, jmattson, seanjc, vkuznets, wei.w.wang,
	kvm, linux-kernel

On Mon, Aug 09, 2021 at 09:16:47PM +0800, Like Xu wrote:
> On 6/8/2021 3:42 pm, Yang Weijiang wrote:
> >From: Like Xu <like.xu@linux.intel.com>
> 
> ...
> 
> >
> >The number of Arch LBR entries available is determined by the value
> >in host MSR_ARCH_LBR_DEPTH.DEPTH. The supported LBR depth values are
> >enumerated in CPUID.(EAX=01CH, ECX=0):EAX[7:0]. For each bit "n" set
> >in this field, the MSR_ARCH_LBR_DEPTH.DEPTH value of "8*(n+1)" is
> >supported.
> >
> >On a guest write to MSR_ARCH_LBR_DEPTH, all LBR entries are reset to 0.
> >KVM writes guest requested value to the native ARCH_LBR_DEPTH MSR
> >(this is safe because the two values will be the same) when the Arch LBR
> >records MSRs are pass-through to the guest.
> >
> >Signed-off-by: Like Xu <like.xu@linux.intel.com>
> >Signed-off-by: Yang Weijiang <weijiang.yang@intel.com>
> >---
> >  arch/x86/kvm/vmx/pmu_intel.c | 35 ++++++++++++++++++++++++++++++++++-
> >  1 file changed, 34 insertions(+), 1 deletion(-)
> >
> >diff --git a/arch/x86/kvm/vmx/pmu_intel.c b/arch/x86/kvm/vmx/pmu_intel.c
> >index 9efc1a6b8693..a4ef5bbce186 100644
> >--- a/arch/x86/kvm/vmx/pmu_intel.c
> >+++ b/arch/x86/kvm/vmx/pmu_intel.c
> >@@ -211,7 +211,7 @@ static bool intel_pmu_is_valid_lbr_msr(struct kvm_vcpu *vcpu, u32 index)
> >  static bool intel_is_valid_msr(struct kvm_vcpu *vcpu, u32 msr)
> >  {
> >  	struct kvm_pmu *pmu = vcpu_to_pmu(vcpu);
> >-	int ret;
> >+	int ret = 0;
> >  	switch (msr) {
> >  	case MSR_CORE_PERF_FIXED_CTR_CTRL:
> >@@ -220,6 +220,10 @@ static bool intel_is_valid_msr(struct kvm_vcpu *vcpu, u32 msr)
> >  	case MSR_CORE_PERF_GLOBAL_OVF_CTRL:
> >  		ret = pmu->version > 1;
> >  		break;
> >+	case MSR_ARCH_LBR_DEPTH:
> >+		if (kvm_cpu_cap_has(X86_FEATURE_ARCH_LBR))
> >+			ret = guest_cpuid_has(vcpu, X86_FEATURE_ARCH_LBR);
> >+		break;
> >  	default:
> >  		ret = get_gp_pmc(pmu, msr, MSR_IA32_PERFCTR0) ||
> >  			get_gp_pmc(pmu, msr, MSR_P6_EVNTSEL0) ||
> >@@ -348,10 +352,28 @@ static bool intel_pmu_handle_lbr_msrs_access(struct kvm_vcpu *vcpu,
> >  	return true;
> >  }
> >+/*
> >+ * Check if the requested depth value the same as that of host.
> >+ * When guest/host depth are different, the handling would be tricky,
> >+ * so now only max depth is supported for both host and guest.
> >+ */
> >+static bool arch_lbr_depth_is_valid(struct kvm_vcpu *vcpu, u64 depth)
> >+{
> >+	unsigned int eax, ebx, ecx, edx;
> >+
> >+	if (!kvm_cpu_cap_has(X86_FEATURE_ARCH_LBR))
> >+		return false;
> >+
> >+	cpuid_count(0x1c, 0, &eax, &ebx, &ecx, &edx);
> 
> I really don't understand why the sanity check of the
> guest lbr depth needs to read the host's cpuid entry and it's pretty slow.
>
This is to address a concern from Jim:
"Does this imply that, when restoring a vCPU, KVM_SET_CPUID2 must be called before
KVM_SET_MSRS, so that arch_lbr_depth_is_valid() knows what to do? Is this documented
anywhere?" 
anyway, setting depth MSR shouldn't be hot path.
 
> KVM has reported the maximum host LBR depth as the only supported value.
> 
> >+
> >+	return (depth == fls(eax & 0xff) * 8);
> >+}
> >+
> >  static int intel_pmu_get_msr(struct kvm_vcpu *vcpu, struct msr_data *msr_info)
> >  {
> >  	struct kvm_pmu *pmu = vcpu_to_pmu(vcpu);
> >  	struct kvm_pmc *pmc;
> >+	struct lbr_desc *lbr_desc = vcpu_to_lbr_desc(vcpu);
> >  	u32 msr = msr_info->index;
> >  	switch (msr) {
> >@@ -367,6 +389,9 @@ static int intel_pmu_get_msr(struct kvm_vcpu *vcpu, struct msr_data *msr_info)
> >  	case MSR_CORE_PERF_GLOBAL_OVF_CTRL:
> >  		msr_info->data = pmu->global_ovf_ctrl;
> >  		return 0;
> >+	case MSR_ARCH_LBR_DEPTH:
> >+		msr_info->data = lbr_desc->records.nr;
> >+		return 0;
> >  	default:
> >  		if ((pmc = get_gp_pmc(pmu, msr, MSR_IA32_PERFCTR0)) ||
> >  		    (pmc = get_gp_pmc(pmu, msr, MSR_IA32_PMC0))) {
> >@@ -393,6 +418,7 @@ static int intel_pmu_set_msr(struct kvm_vcpu *vcpu, struct msr_data *msr_info)
> >  {
> >  	struct kvm_pmu *pmu = vcpu_to_pmu(vcpu);
> >  	struct kvm_pmc *pmc;
> >+	struct lbr_desc *lbr_desc = vcpu_to_lbr_desc(vcpu);
> >  	u32 msr = msr_info->index;
> >  	u64 data = msr_info->data;
> >@@ -427,6 +453,13 @@ static int intel_pmu_set_msr(struct kvm_vcpu *vcpu, struct msr_data *msr_info)
> >  			return 0;
> >  		}
> >  		break;
> >+	case MSR_ARCH_LBR_DEPTH:
> >+		if (!arch_lbr_depth_is_valid(vcpu, data))
> >+			return 1;
> >+		lbr_desc->records.nr = data;
> >+		if (!msr_info->host_initiated)
> >+			wrmsrl(MSR_ARCH_LBR_DEPTH, lbr_desc->records.nr);
> 
> Resetting the host msr here is dangerous,
> what if the guest LBR event doesn't exist or isn't scheduled on?
Hmm, should be vmcs_write to the DEPTH field, thanks for pointing this
out!
> 
> >+		return 0;
> >  	default:
> >  		if ((pmc = get_gp_pmc(pmu, msr, MSR_IA32_PERFCTR0)) ||
> >  		    (pmc = get_gp_pmc(pmu, msr, MSR_IA32_PMC0))) {
> >

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

* Re: [PATCH v7 04/15] KVM: vmx/pmu: Emulate MSR_ARCH_LBR_DEPTH for guest Arch LBR
  2021-08-10  7:38     ` Yang Weijiang
@ 2021-08-10  7:54       ` Like Xu
  2021-08-10  9:08         ` Yang Weijiang
  0 siblings, 1 reply; 28+ messages in thread
From: Like Xu @ 2021-08-10  7:54 UTC (permalink / raw)
  To: Yang Weijiang
  Cc: pbonzini, jmattson, seanjc, vkuznets, wei.w.wang, kvm, linux-kernel

On 10/8/2021 3:38 pm, Yang Weijiang wrote:
> On Mon, Aug 09, 2021 at 09:16:47PM +0800, Like Xu wrote:
>> On 6/8/2021 3:42 pm, Yang Weijiang wrote:
>>> From: Like Xu <like.xu@linux.intel.com>
>>
>> ...
>>
>>>
>>> The number of Arch LBR entries available is determined by the value
>>> in host MSR_ARCH_LBR_DEPTH.DEPTH. The supported LBR depth values are
>>> enumerated in CPUID.(EAX=01CH, ECX=0):EAX[7:0]. For each bit "n" set
>>> in this field, the MSR_ARCH_LBR_DEPTH.DEPTH value of "8*(n+1)" is
>>> supported.
>>>
>>> On a guest write to MSR_ARCH_LBR_DEPTH, all LBR entries are reset to 0.
>>> KVM writes guest requested value to the native ARCH_LBR_DEPTH MSR
>>> (this is safe because the two values will be the same) when the Arch LBR
>>> records MSRs are pass-through to the guest.
>>>
>>> Signed-off-by: Like Xu <like.xu@linux.intel.com>
>>> Signed-off-by: Yang Weijiang <weijiang.yang@intel.com>
>>> ---
>>>   arch/x86/kvm/vmx/pmu_intel.c | 35 ++++++++++++++++++++++++++++++++++-
>>>   1 file changed, 34 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/arch/x86/kvm/vmx/pmu_intel.c b/arch/x86/kvm/vmx/pmu_intel.c
>>> index 9efc1a6b8693..a4ef5bbce186 100644
>>> --- a/arch/x86/kvm/vmx/pmu_intel.c
>>> +++ b/arch/x86/kvm/vmx/pmu_intel.c
>>> @@ -211,7 +211,7 @@ static bool intel_pmu_is_valid_lbr_msr(struct kvm_vcpu *vcpu, u32 index)
>>>   static bool intel_is_valid_msr(struct kvm_vcpu *vcpu, u32 msr)
>>>   {
>>>   	struct kvm_pmu *pmu = vcpu_to_pmu(vcpu);
>>> -	int ret;
>>> +	int ret = 0;
>>>   	switch (msr) {
>>>   	case MSR_CORE_PERF_FIXED_CTR_CTRL:
>>> @@ -220,6 +220,10 @@ static bool intel_is_valid_msr(struct kvm_vcpu *vcpu, u32 msr)
>>>   	case MSR_CORE_PERF_GLOBAL_OVF_CTRL:
>>>   		ret = pmu->version > 1;
>>>   		break;
>>> +	case MSR_ARCH_LBR_DEPTH:
>>> +		if (kvm_cpu_cap_has(X86_FEATURE_ARCH_LBR))
>>> +			ret = guest_cpuid_has(vcpu, X86_FEATURE_ARCH_LBR);
>>> +		break;
>>>   	default:
>>>   		ret = get_gp_pmc(pmu, msr, MSR_IA32_PERFCTR0) ||
>>>   			get_gp_pmc(pmu, msr, MSR_P6_EVNTSEL0) ||
>>> @@ -348,10 +352,28 @@ static bool intel_pmu_handle_lbr_msrs_access(struct kvm_vcpu *vcpu,
>>>   	return true;
>>>   }
>>> +/*
>>> + * Check if the requested depth value the same as that of host.
>>> + * When guest/host depth are different, the handling would be tricky,
>>> + * so now only max depth is supported for both host and guest.
>>> + */
>>> +static bool arch_lbr_depth_is_valid(struct kvm_vcpu *vcpu, u64 depth)
>>> +{
>>> +	unsigned int eax, ebx, ecx, edx;
>>> +
>>> +	if (!kvm_cpu_cap_has(X86_FEATURE_ARCH_LBR))
>>> +		return false;
>>> +
>>> +	cpuid_count(0x1c, 0, &eax, &ebx, &ecx, &edx);
>>
>> I really don't understand why the sanity check of the
>> guest lbr depth needs to read the host's cpuid entry and it's pretty slow.
>>
> This is to address a concern from Jim:
> "Does this imply that, when restoring a vCPU, KVM_SET_CPUID2 must be called before
> KVM_SET_MSRS, so that arch_lbr_depth_is_valid() knows what to do? Is this documented
> anywhere?"

What will KVM do if the #GP behaviour of msr does not match the CPUID it is set to?

For user space host_initiated path, we may check it with the host one but
for the guest emulation, it should rely on guest cpuid, just like what we do in 
the intel_is_valid_msr().

> anyway, setting depth MSR shouldn't be hot path.

Not at all, it will be used to reset LBR entries
and it's as frequent as task switching.

>   
>> KVM has reported the maximum host LBR depth as the only supported value.
>>
>>> +
>>> +	return (depth == fls(eax & 0xff) * 8);
>>> +}
>>> +
>>>   static int intel_pmu_get_msr(struct kvm_vcpu *vcpu, struct msr_data *msr_info)
>>>   {
>>>   	struct kvm_pmu *pmu = vcpu_to_pmu(vcpu);
>>>   	struct kvm_pmc *pmc;
>>> +	struct lbr_desc *lbr_desc = vcpu_to_lbr_desc(vcpu);
>>>   	u32 msr = msr_info->index;
>>>   	switch (msr) {
>>> @@ -367,6 +389,9 @@ static int intel_pmu_get_msr(struct kvm_vcpu *vcpu, struct msr_data *msr_info)
>>>   	case MSR_CORE_PERF_GLOBAL_OVF_CTRL:
>>>   		msr_info->data = pmu->global_ovf_ctrl;
>>>   		return 0;
>>> +	case MSR_ARCH_LBR_DEPTH:
>>> +		msr_info->data = lbr_desc->records.nr;
>>> +		return 0;
>>>   	default:
>>>   		if ((pmc = get_gp_pmc(pmu, msr, MSR_IA32_PERFCTR0)) ||
>>>   		    (pmc = get_gp_pmc(pmu, msr, MSR_IA32_PMC0))) {
>>> @@ -393,6 +418,7 @@ static int intel_pmu_set_msr(struct kvm_vcpu *vcpu, struct msr_data *msr_info)
>>>   {
>>>   	struct kvm_pmu *pmu = vcpu_to_pmu(vcpu);
>>>   	struct kvm_pmc *pmc;
>>> +	struct lbr_desc *lbr_desc = vcpu_to_lbr_desc(vcpu);
>>>   	u32 msr = msr_info->index;
>>>   	u64 data = msr_info->data;
>>> @@ -427,6 +453,13 @@ static int intel_pmu_set_msr(struct kvm_vcpu *vcpu, struct msr_data *msr_info)
>>>   			return 0;
>>>   		}
>>>   		break;
>>> +	case MSR_ARCH_LBR_DEPTH:
>>> +		if (!arch_lbr_depth_is_valid(vcpu, data))
>>> +			return 1;
>>> +		lbr_desc->records.nr = data;
>>> +		if (!msr_info->host_initiated)
>>> +			wrmsrl(MSR_ARCH_LBR_DEPTH, lbr_desc->records.nr);
>>
>> Resetting the host msr here is dangerous,
>> what if the guest LBR event doesn't exist or isn't scheduled on?
> Hmm, should be vmcs_write to the DEPTH field, thanks for pointing this
> out!

Seriously?

>>
>>> +		return 0;
>>>   	default:
>>>   		if ((pmc = get_gp_pmc(pmu, msr, MSR_IA32_PERFCTR0)) ||
>>>   		    (pmc = get_gp_pmc(pmu, msr, MSR_IA32_PMC0))) {
>>>

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

* Re: [PATCH v7 05/15] KVM: vmx/pmu: Emulate MSR_ARCH_LBR_CTL for guest Arch LBR
  2021-08-09 13:36   ` Like Xu
@ 2021-08-10  8:30     ` Yang Weijiang
  2021-08-10  8:37       ` Like Xu
  0 siblings, 1 reply; 28+ messages in thread
From: Yang Weijiang @ 2021-08-10  8:30 UTC (permalink / raw)
  To: Like Xu
  Cc: Yang Weijiang, pbonzini, jmattson, seanjc, vkuznets, wei.w.wang,
	kvm, linux-kernel

On Mon, Aug 09, 2021 at 09:36:49PM +0800, Like Xu wrote:
> On 6/8/2021 3:42 pm, Yang Weijiang wrote:
> >From: Like Xu <like.xu@linux.intel.com>
> >
> >Arch LBRs are enabled by setting MSR_ARCH_LBR_CTL.LBREn to 1. A new guest
> >state field named "Guest IA32_LBR_CTL" is added to enhance guest LBR usage.
> >When guest Arch LBR is enabled, a guest LBR event will be created like the
> >model-specific LBR does. Clear guest LBR enable bit on host PMI handling so
> >guest can see expected config.
> >
> >On processors that support Arch LBR, MSR_IA32_DEBUGCTLMSR[bit 0] has no
> >meaning. It can be written to 0 or 1, but reads will always return 0.
> >Like IA32_DEBUGCTL, IA32_ARCH_LBR_CTL msr is also preserved on INIT.
> >
> >Regardless of the Arch LBR or legacy LBR, when the LBR_EN bit 0 of the
> >corresponding control MSR is set to 1, LBR recording will be enabled.
> >
> >Signed-off-by: Like Xu <like.xu@linux.intel.com>
> >Signed-off-by: Yang Weijiang <weijiang.yang@intel.com>
> >---
> >  arch/x86/events/intel/lbr.c      |  2 --
> >  arch/x86/include/asm/msr-index.h |  1 +
> >  arch/x86/include/asm/vmx.h       |  2 ++
> >  arch/x86/kvm/vmx/pmu_intel.c     | 48 ++++++++++++++++++++++++++++----
> >  arch/x86/kvm/vmx/vmx.c           |  9 ++++++
> >  5 files changed, 55 insertions(+), 7 deletions(-)
> >

[...]

> >+static bool arch_lbr_ctl_is_valid(struct kvm_vcpu *vcpu, u64 ctl)
> >+{
> >+	unsigned int eax, ebx, ecx, edx;
> >+
> >+	if (!kvm_cpu_cap_has(X86_FEATURE_ARCH_LBR))
> >+		return false;
> >+
> >+	cpuid_count(0x1c, 0, &eax, &ebx, &ecx, &edx);
> >+	if (!(ebx & BIT(0)) && (ctl & ARCH_LBR_CTL_CPL))
> >+		return false;
> >+	if (!(ebx & BIT(2)) && (ctl & ARCH_LBR_CTL_STACK))
> >+		return false;
> >+	if (!(ebx & BIT(1)) && (ctl & ARCH_LBR_CTL_BRN_MASK))
> >+		return false;
> >+
> >+	return !(ctl & ~KVM_ARCH_LBR_CTL_MASK);
> >+}
> 
> Please check it with the *guest* cpuid entry.
If KVM "trusts" user-space, then check with guest cpuid is OK.
But if user-space enable excessive controls, then check against guest
cpuid could make things mess.

> 
> And it should remove the bits that are not supported by x86_pmu.lbr_ctl_mask before
> vmcs_write64(...) if the guest value is a superset of the host value with
> warning message.
Then I think it makes more sense to check against x86_pmu.lbr_xxx masks in above function
for compatibility. What do you think of it?
> 
> >+
> >  static int intel_pmu_get_msr(struct kvm_vcpu *vcpu, struct msr_data *msr_info)
> >  {
> >  	struct kvm_pmu *pmu = vcpu_to_pmu(vcpu);
> >@@ -392,6 +414,9 @@ static int intel_pmu_get_msr(struct kvm_vcpu *vcpu, struct msr_data *msr_info)
> >  	case MSR_ARCH_LBR_DEPTH:
> >  		msr_info->data = lbr_desc->records.nr;
> >  		return 0;
> >+	case MSR_ARCH_LBR_CTL:
> >+		msr_info->data = vmcs_read64(GUEST_IA32_LBR_CTL);
> >+		return 0;
> >  	default:
> >  		if ((pmc = get_gp_pmc(pmu, msr, MSR_IA32_PERFCTR0)) ||
> >  		    (pmc = get_gp_pmc(pmu, msr, MSR_IA32_PMC0))) {
> >  		vmcs_write64(GUEST_IA32_DEBUGCTL, data);

[...]

> >  		if (intel_pmu_lbr_is_enabled(vcpu) && !to_vmx(vcpu)->lbr_desc.event &&
> >  		    (data & DEBUGCTLMSR_LBR))
> >@@ -4441,6 +4448,8 @@ static void vmx_vcpu_reset(struct kvm_vcpu *vcpu, bool init_event)
> >  		vmcs_writel(GUEST_SYSENTER_ESP, 0);
> >  		vmcs_writel(GUEST_SYSENTER_EIP, 0);
> >  		vmcs_write64(GUEST_IA32_DEBUGCTL, 0);
> >+		if (static_cpu_has(X86_FEATURE_ARCH_LBR))
> >+			vmcs_write64(GUEST_IA32_LBR_CTL, 0);
> 
> Please update dump_vmcs() to dump GUEST_IA32_LBR_CTL as well.
OK, will add it.
> 
> How about update the load_vmcs12_host_state() for GUEST_IA32_LBR_CTL
> since you enabled the nested case in this patch set ?
No, I didn't enable nested Arch LBR but unblocked some issues for nested case.
> 
> >  	}
> >  	kvm_set_rflags(vcpu, X86_EFLAGS_FIXED);
> >

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

* Re: [PATCH v7 05/15] KVM: vmx/pmu: Emulate MSR_ARCH_LBR_CTL for guest Arch LBR
  2021-08-10  8:30     ` Yang Weijiang
@ 2021-08-10  8:37       ` Like Xu
  0 siblings, 0 replies; 28+ messages in thread
From: Like Xu @ 2021-08-10  8:37 UTC (permalink / raw)
  To: Yang Weijiang
  Cc: pbonzini, jmattson, seanjc, vkuznets, wei.w.wang, kvm, linux-kernel

On 10/8/2021 4:30 pm, Yang Weijiang wrote:
> On Mon, Aug 09, 2021 at 09:36:49PM +0800, Like Xu wrote:
>> On 6/8/2021 3:42 pm, Yang Weijiang wrote:
>>> From: Like Xu <like.xu@linux.intel.com>
>>>
>>> Arch LBRs are enabled by setting MSR_ARCH_LBR_CTL.LBREn to 1. A new guest
>>> state field named "Guest IA32_LBR_CTL" is added to enhance guest LBR usage.
>>> When guest Arch LBR is enabled, a guest LBR event will be created like the
>>> model-specific LBR does. Clear guest LBR enable bit on host PMI handling so
>>> guest can see expected config.
>>>
>>> On processors that support Arch LBR, MSR_IA32_DEBUGCTLMSR[bit 0] has no
>>> meaning. It can be written to 0 or 1, but reads will always return 0.
>>> Like IA32_DEBUGCTL, IA32_ARCH_LBR_CTL msr is also preserved on INIT.
>>>
>>> Regardless of the Arch LBR or legacy LBR, when the LBR_EN bit 0 of the
>>> corresponding control MSR is set to 1, LBR recording will be enabled.
>>>
>>> Signed-off-by: Like Xu <like.xu@linux.intel.com>
>>> Signed-off-by: Yang Weijiang <weijiang.yang@intel.com>
>>> ---
>>>   arch/x86/events/intel/lbr.c      |  2 --
>>>   arch/x86/include/asm/msr-index.h |  1 +
>>>   arch/x86/include/asm/vmx.h       |  2 ++
>>>   arch/x86/kvm/vmx/pmu_intel.c     | 48 ++++++++++++++++++++++++++++----
>>>   arch/x86/kvm/vmx/vmx.c           |  9 ++++++
>>>   5 files changed, 55 insertions(+), 7 deletions(-)
>>>
> 
> [...]
> 
>>> +static bool arch_lbr_ctl_is_valid(struct kvm_vcpu *vcpu, u64 ctl)
>>> +{
>>> +	unsigned int eax, ebx, ecx, edx;
>>> +
>>> +	if (!kvm_cpu_cap_has(X86_FEATURE_ARCH_LBR))
>>> +		return false;
>>> +
>>> +	cpuid_count(0x1c, 0, &eax, &ebx, &ecx, &edx);
>>> +	if (!(ebx & BIT(0)) && (ctl & ARCH_LBR_CTL_CPL))
>>> +		return false;
>>> +	if (!(ebx & BIT(2)) && (ctl & ARCH_LBR_CTL_STACK))
>>> +		return false;
>>> +	if (!(ebx & BIT(1)) && (ctl & ARCH_LBR_CTL_BRN_MASK))
>>> +		return false;
>>> +
>>> +	return !(ctl & ~KVM_ARCH_LBR_CTL_MASK);
>>> +}
>>
>> Please check it with the *guest* cpuid entry.
> If KVM "trusts" user-space, then check with guest cpuid is OK.
> But if user-space enable excessive controls, then check against guest
> cpuid could make things mess.

The user space should be aware of its own risk
if it sets the cpuid that exceeds the host's capabilities.

> 
>>
>> And it should remove the bits that are not supported by x86_pmu.lbr_ctl_mask before
>> vmcs_write64(...) if the guest value is a superset of the host value with
>> warning message.
> Then I think it makes more sense to check against x86_pmu.lbr_xxx masks in above function
> for compatibility. What do you think of it?

The host driver hard-codes x86_pmu.lbr_ctl_mask to ARCH_LBR_CTL_MASK
but the user space can mask out some of the capability based on its cpuid selection.
In that case, we need to check it with the guest cpuid entry.

If user space exceeds the KVM supported capabilities,
KVM could leave a warning before the vm-entry fails.

>>
>>> +
>>>   static int intel_pmu_get_msr(struct kvm_vcpu *vcpu, struct msr_data *msr_info)
>>>   {
>>>   	struct kvm_pmu *pmu = vcpu_to_pmu(vcpu);
>>> @@ -392,6 +414,9 @@ static int intel_pmu_get_msr(struct kvm_vcpu *vcpu, struct msr_data *msr_info)
>>>   	case MSR_ARCH_LBR_DEPTH:
>>>   		msr_info->data = lbr_desc->records.nr;
>>>   		return 0;
>>> +	case MSR_ARCH_LBR_CTL:
>>> +		msr_info->data = vmcs_read64(GUEST_IA32_LBR_CTL);
>>> +		return 0;
>>>   	default:
>>>   		if ((pmc = get_gp_pmc(pmu, msr, MSR_IA32_PERFCTR0)) ||
>>>   		    (pmc = get_gp_pmc(pmu, msr, MSR_IA32_PMC0))) {
>>>   		vmcs_write64(GUEST_IA32_DEBUGCTL, data);
> 
> [...]
> 
>>>   		if (intel_pmu_lbr_is_enabled(vcpu) && !to_vmx(vcpu)->lbr_desc.event &&
>>>   		    (data & DEBUGCTLMSR_LBR))
>>> @@ -4441,6 +4448,8 @@ static void vmx_vcpu_reset(struct kvm_vcpu *vcpu, bool init_event)
>>>   		vmcs_writel(GUEST_SYSENTER_ESP, 0);
>>>   		vmcs_writel(GUEST_SYSENTER_EIP, 0);
>>>   		vmcs_write64(GUEST_IA32_DEBUGCTL, 0);
>>> +		if (static_cpu_has(X86_FEATURE_ARCH_LBR))
>>> +			vmcs_write64(GUEST_IA32_LBR_CTL, 0);
>>
>> Please update dump_vmcs() to dump GUEST_IA32_LBR_CTL as well.
> OK, will add it.
>>
>> How about update the load_vmcs12_host_state() for GUEST_IA32_LBR_CTL
>> since you enabled the nested case in this patch set ?
> No, I didn't enable nested Arch LBR but unblocked some issues for nested case.

Would you like to explain more about the unblocked issue ?

>>
>>>   	}
>>>   	kvm_set_rflags(vcpu, X86_EFLAGS_FIXED);
>>>

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

* Re: [PATCH v7 04/15] KVM: vmx/pmu: Emulate MSR_ARCH_LBR_DEPTH for guest Arch LBR
  2021-08-10  7:54       ` Like Xu
@ 2021-08-10  9:08         ` Yang Weijiang
  0 siblings, 0 replies; 28+ messages in thread
From: Yang Weijiang @ 2021-08-10  9:08 UTC (permalink / raw)
  To: Like Xu
  Cc: Yang Weijiang, pbonzini, jmattson, seanjc, vkuznets, wei.w.wang,
	kvm, linux-kernel

On Tue, Aug 10, 2021 at 03:54:09PM +0800, Like Xu wrote:
> On 10/8/2021 3:38 pm, Yang Weijiang wrote:
> >On Mon, Aug 09, 2021 at 09:16:47PM +0800, Like Xu wrote:
> >>On 6/8/2021 3:42 pm, Yang Weijiang wrote:
> >>>From: Like Xu <like.xu@linux.intel.com>
> >>
> >>...
> >>
> >>>
> >>>The number of Arch LBR entries available is determined by the value
> >>>in host MSR_ARCH_LBR_DEPTH.DEPTH. The supported LBR depth values are
> >>>enumerated in CPUID.(EAX=01CH, ECX=0):EAX[7:0]. For each bit "n" set
> >>>in this field, the MSR_ARCH_LBR_DEPTH.DEPTH value of "8*(n+1)" is
> >>>supported.
> >>>
> >>>On a guest write to MSR_ARCH_LBR_DEPTH, all LBR entries are reset to 0.
> >>>KVM writes guest requested value to the native ARCH_LBR_DEPTH MSR
> >>>(this is safe because the two values will be the same) when the Arch LBR
> >>>records MSRs are pass-through to the guest.
> >>>
> >>>Signed-off-by: Like Xu <like.xu@linux.intel.com>
> >>>Signed-off-by: Yang Weijiang <weijiang.yang@intel.com>
> >>>---
> >>>  arch/x86/kvm/vmx/pmu_intel.c | 35 ++++++++++++++++++++++++++++++++++-
> >>>  1 file changed, 34 insertions(+), 1 deletion(-)
> >>>
> >>>diff --git a/arch/x86/kvm/vmx/pmu_intel.c b/arch/x86/kvm/vmx/pmu_intel.c
> >>>index 9efc1a6b8693..a4ef5bbce186 100644
> >>>--- a/arch/x86/kvm/vmx/pmu_intel.c
> >>>+++ b/arch/x86/kvm/vmx/pmu_intel.c
> >>>@@ -211,7 +211,7 @@ static bool intel_pmu_is_valid_lbr_msr(struct kvm_vcpu *vcpu, u32 index)
> >>>  static bool intel_is_valid_msr(struct kvm_vcpu *vcpu, u32 msr)
> >>>  {
> >>>  	struct kvm_pmu *pmu = vcpu_to_pmu(vcpu);
> >>>-	int ret;
> >>>+	int ret = 0;
> >>>  	switch (msr) {
> >>>  	case MSR_CORE_PERF_FIXED_CTR_CTRL:
> >>>@@ -220,6 +220,10 @@ static bool intel_is_valid_msr(struct kvm_vcpu *vcpu, u32 msr)
> >>>  	case MSR_CORE_PERF_GLOBAL_OVF_CTRL:
> >>>  		ret = pmu->version > 1;
> >>>  		break;
> >>>+	case MSR_ARCH_LBR_DEPTH:
> >>>+		if (kvm_cpu_cap_has(X86_FEATURE_ARCH_LBR))
> >>>+			ret = guest_cpuid_has(vcpu, X86_FEATURE_ARCH_LBR);
> >>>+		break;
> >>>  	default:
> >>>  		ret = get_gp_pmc(pmu, msr, MSR_IA32_PERFCTR0) ||
> >>>  			get_gp_pmc(pmu, msr, MSR_P6_EVNTSEL0) ||
> >>>@@ -348,10 +352,28 @@ static bool intel_pmu_handle_lbr_msrs_access(struct kvm_vcpu *vcpu,
> >>>  	return true;
> >>>  }
> >>>+/*
> >>>+ * Check if the requested depth value the same as that of host.
> >>>+ * When guest/host depth are different, the handling would be tricky,
> >>>+ * so now only max depth is supported for both host and guest.
> >>>+ */
> >>>+static bool arch_lbr_depth_is_valid(struct kvm_vcpu *vcpu, u64 depth)
> >>>+{
> >>>+	unsigned int eax, ebx, ecx, edx;
> >>>+
> >>>+	if (!kvm_cpu_cap_has(X86_FEATURE_ARCH_LBR))
> >>>+		return false;
> >>>+
> >>>+	cpuid_count(0x1c, 0, &eax, &ebx, &ecx, &edx);
> >>
> >>I really don't understand why the sanity check of the
> >>guest lbr depth needs to read the host's cpuid entry and it's pretty slow.
> >>
> >This is to address a concern from Jim:
> >"Does this imply that, when restoring a vCPU, KVM_SET_CPUID2 must be called before
> >KVM_SET_MSRS, so that arch_lbr_depth_is_valid() knows what to do? Is this documented
> >anywhere?"
> 
> What will KVM do if the #GP behaviour of msr does not match the CPUID it is set to?
> 
> For user space host_initiated path, we may check it with the host one but
> for the guest emulation, it should rely on guest cpuid, just like what we do
> in the intel_is_valid_msr().
I think the original consideration is to avoid a mis-match of host/guest depth
makes the host Arch LBR totally broken since a mis-match will clear all LBR MSRs.
But I added a check in kvm_vcpu_after_set_cpuid() when user-space KVM_SET_CPUID2 to
make sure user-space won't set unsupported depth. So maybe I can make
the check as simple as checking a fixed value.
> 
> >anyway, setting depth MSR shouldn't be hot path.
> 
> Not at all, it will be used to reset LBR entries
> and it's as frequent as task switching.

Yes, I saw this, if xsaves is not supported, writing depth msr is used as a short-cut.
> 
> >>KVM has reported the maximum host LBR depth as the only supported value.
> >>
> >>>+
> >>>+	return (depth == fls(eax & 0xff) * 8);
> >>>+}
> >>>+
> >>>  static int intel_pmu_get_msr(struct kvm_vcpu *vcpu, struct msr_data *msr_info)
> >>>  {
> >>>  	struct kvm_pmu *pmu = vcpu_to_pmu(vcpu);
> >>>  	struct kvm_pmc *pmc;
> >>>+	struct lbr_desc *lbr_desc = vcpu_to_lbr_desc(vcpu);
> >>>  	u32 msr = msr_info->index;
> >>>  	switch (msr) {
> >>>@@ -367,6 +389,9 @@ static int intel_pmu_get_msr(struct kvm_vcpu *vcpu, struct msr_data *msr_info)
> >>>  	case MSR_CORE_PERF_GLOBAL_OVF_CTRL:
> >>>  		msr_info->data = pmu->global_ovf_ctrl;
> >>>  		return 0;
> >>>+	case MSR_ARCH_LBR_DEPTH:
> >>>+		msr_info->data = lbr_desc->records.nr;
> >>>+		return 0;
> >>>  	default:
> >>>  		if ((pmc = get_gp_pmc(pmu, msr, MSR_IA32_PERFCTR0)) ||
> >>>  		    (pmc = get_gp_pmc(pmu, msr, MSR_IA32_PMC0))) {
> >>>@@ -393,6 +418,7 @@ static int intel_pmu_set_msr(struct kvm_vcpu *vcpu, struct msr_data *msr_info)
> >>>  {
> >>>  	struct kvm_pmu *pmu = vcpu_to_pmu(vcpu);
> >>>  	struct kvm_pmc *pmc;
> >>>+	struct lbr_desc *lbr_desc = vcpu_to_lbr_desc(vcpu);
> >>>  	u32 msr = msr_info->index;
> >>>  	u64 data = msr_info->data;
> >>>@@ -427,6 +453,13 @@ static int intel_pmu_set_msr(struct kvm_vcpu *vcpu, struct msr_data *msr_info)
> >>>  			return 0;
> >>>  		}
> >>>  		break;
> >>>+	case MSR_ARCH_LBR_DEPTH:
> >>>+		if (!arch_lbr_depth_is_valid(vcpu, data))
> >>>+			return 1;
> >>>+		lbr_desc->records.nr = data;
> >>>+		if (!msr_info->host_initiated)
> >>>+			wrmsrl(MSR_ARCH_LBR_DEPTH, lbr_desc->records.nr);
> >>
> >>Resetting the host msr here is dangerous,
> >>what if the guest LBR event doesn't exist or isn't scheduled on?
> >Hmm, should be vmcs_write to the DEPTH field, thanks for pointing this
> >out!
> 
> Seriously?
My gosh! we don't have the field :-/ Then more sanity checks are
required.
> 
> >>
> >>>+		return 0;
> >>>  	default:
> >>>  		if ((pmc = get_gp_pmc(pmu, msr, MSR_IA32_PERFCTR0)) ||
> >>>  		    (pmc = get_gp_pmc(pmu, msr, MSR_IA32_PMC0))) {
> >>>

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

end of thread, other threads:[~2021-08-10  8:55 UTC | newest]

Thread overview: 28+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-08-06  7:42 [PATCH v7 00/15] Introduce Architectural LBR for vPMU Yang Weijiang
2021-08-06  7:42 ` [PATCH v7 01/15] perf/x86/intel: Fix the comment about guest LBR support on KVM Yang Weijiang
2021-08-06  7:42 ` [PATCH v7 02/15] perf/x86/lbr: Simplify the exposure check for the LBR_INFO registers Yang Weijiang
2021-08-06  7:42 ` [PATCH v7 03/15] KVM: x86: Add Arch LBR MSRs to msrs_to_save_all list Yang Weijiang
2021-08-09 13:07   ` Like Xu
2021-08-06  7:42 ` [PATCH v7 04/15] KVM: vmx/pmu: Emulate MSR_ARCH_LBR_DEPTH for guest Arch LBR Yang Weijiang
2021-08-09 13:16   ` Like Xu
2021-08-10  7:38     ` Yang Weijiang
2021-08-10  7:54       ` Like Xu
2021-08-10  9:08         ` Yang Weijiang
2021-08-06  7:42 ` [PATCH v7 05/15] KVM: vmx/pmu: Emulate MSR_ARCH_LBR_CTL " Yang Weijiang
2021-08-06 16:26   ` kernel test robot
2021-08-06 16:26     ` kernel test robot
2021-08-09 13:36   ` Like Xu
2021-08-10  8:30     ` Yang Weijiang
2021-08-10  8:37       ` Like Xu
2021-08-06  7:42 ` [PATCH v7 06/15] KVM: x86/pmu: Refactor code to support " Yang Weijiang
2021-08-06  7:42 ` [PATCH v7 07/15] KVM: x86: Refresh CPUID on writes to MSR_IA32_XSS Yang Weijiang
2021-08-06  7:42 ` [PATCH v7 08/15] KVM: x86: Report XSS as an MSR to be saved if there are supported features Yang Weijiang
2021-08-06  7:42 ` [PATCH v7 09/15] KVM: x86: Refine the matching and clearing logic for supported_xss Yang Weijiang
2021-08-06  7:42 ` [PATCH v7 10/15] KVM: x86: Add XSAVE Support for Architectural LBR Yang Weijiang
2021-08-06  7:42 ` [PATCH v7 11/15] KVM: x86/vmx: Check Arch LBR config when return perf capabilities Yang Weijiang
2021-08-06  7:42 ` [PATCH v7 12/15] KVM: nVMX: Add necessary Arch LBR settings for nested VM Yang Weijiang
2021-08-06  7:42 ` [PATCH v7 13/15] KVM: x86/vmx: Clear Arch LBREn bit before inject #DB to guest Yang Weijiang
2021-08-09  5:08   ` Like Xu
2021-08-09  9:02     ` Yang Weijiang
2021-08-06  7:42 ` [PATCH v7 14/15] KVM: x86/vmx: Flip Arch LBREn bit on guest state change Yang Weijiang
2021-08-06  7:42 ` [PATCH v7 15/15] KVM: x86/cpuid: Advise Arch LBR feature in CPUID Yang Weijiang

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.