* [PATCH v11 01/16] perf/x86/intel: Fix the comment about guest LBR support on KVM
2022-05-06 3:32 [PATCH v11 00/16] Introduce Architectural LBR for vPMU Yang Weijiang
@ 2022-05-06 3:32 ` Yang Weijiang
2022-05-06 3:32 ` [PATCH v11 02/16] perf/x86/lbr: Simplify the exposure check for the LBR_INFO registers Yang Weijiang
` (15 subsequent siblings)
16 siblings, 0 replies; 37+ messages in thread
From: Yang Weijiang @ 2022-05-06 3:32 UTC (permalink / raw)
To: pbonzini, jmattson, seanjc, kan.liang, like.xu.linux, vkuznets,
wei.w.wang, kvm, linux-kernel
Cc: Like Xu, Peter Zijlstra, Andi Kleen, 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 fc7f458eb3de..b4a0ae3eea27 100644
--- a/arch/x86/events/intel/core.c
+++ b/arch/x86/events/intel/core.c
@@ -6396,8 +6396,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.27.0
^ permalink raw reply related [flat|nested] 37+ messages in thread* [PATCH v11 02/16] perf/x86/lbr: Simplify the exposure check for the LBR_INFO registers
2022-05-06 3:32 [PATCH v11 00/16] Introduce Architectural LBR for vPMU Yang Weijiang
2022-05-06 3:32 ` [PATCH v11 01/16] perf/x86/intel: Fix the comment about guest LBR support on KVM Yang Weijiang
@ 2022-05-06 3:32 ` Yang Weijiang
2022-05-06 3:32 ` [PATCH v11 03/16] KVM: x86: Report XSS as an MSR to be saved if there are supported features Yang Weijiang
` (14 subsequent siblings)
16 siblings, 0 replies; 37+ messages in thread
From: Yang Weijiang @ 2022-05-06 3:32 UTC (permalink / raw)
To: pbonzini, jmattson, seanjc, kan.liang, like.xu.linux, vkuznets,
wei.w.wang, kvm, linux-kernel
Cc: Like Xu, Peter Zijlstra, Andi Kleen, 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 fe1742c4ca49..4529ce448b2e 100644
--- a/arch/x86/events/intel/lbr.c
+++ b/arch/x86/events/intel/lbr.c
@@ -1874,12 +1874,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.27.0
^ permalink raw reply related [flat|nested] 37+ messages in thread* [PATCH v11 03/16] KVM: x86: Report XSS as an MSR to be saved if there are supported features
2022-05-06 3:32 [PATCH v11 00/16] Introduce Architectural LBR for vPMU Yang Weijiang
2022-05-06 3:32 ` [PATCH v11 01/16] perf/x86/intel: Fix the comment about guest LBR support on KVM Yang Weijiang
2022-05-06 3:32 ` [PATCH v11 02/16] perf/x86/lbr: Simplify the exposure check for the LBR_INFO registers Yang Weijiang
@ 2022-05-06 3:32 ` Yang Weijiang
2022-05-06 3:32 ` [PATCH v11 04/16] KVM: x86: Refresh CPUID on writes to MSR_IA32_XSS Yang Weijiang
` (13 subsequent siblings)
16 siblings, 0 replies; 37+ messages in thread
From: Yang Weijiang @ 2022-05-06 3:32 UTC (permalink / raw)
To: pbonzini, jmattson, seanjc, kan.liang, like.xu.linux, vkuznets,
wei.w.wang, 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>
---
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 4790f0d7d40b..396b576f8c42 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -1428,6 +1428,7 @@ static const u32 msrs_to_save_all[] = {
MSR_F15H_PERF_CTR0, MSR_F15H_PERF_CTR1, MSR_F15H_PERF_CTR2,
MSR_F15H_PERF_CTR3, MSR_F15H_PERF_CTR4, MSR_F15H_PERF_CTR5,
MSR_IA32_XFD, MSR_IA32_XFD_ERR,
+ MSR_IA32_XSS,
};
static u32 msrs_to_save[ARRAY_SIZE(msrs_to_save_all)];
@@ -6653,6 +6654,10 @@ static void kvm_init_msr_list(void)
if (!kvm_cpu_cap_has(X86_FEATURE_XFD))
continue;
break;
+ case MSR_IA32_XSS:
+ if (!supported_xss)
+ continue;
+ break;
default:
break;
}
--
2.27.0
^ permalink raw reply related [flat|nested] 37+ messages in thread* [PATCH v11 04/16] KVM: x86: Refresh CPUID on writes to MSR_IA32_XSS
2022-05-06 3:32 [PATCH v11 00/16] Introduce Architectural LBR for vPMU Yang Weijiang
` (2 preceding siblings ...)
2022-05-06 3:32 ` [PATCH v11 03/16] KVM: x86: Report XSS as an MSR to be saved if there are supported features Yang Weijiang
@ 2022-05-06 3:32 ` Yang Weijiang
2022-05-06 3:32 ` [PATCH v11 05/16] KVM: x86: Add Arch LBR MSRs to msrs_to_save_all list Yang Weijiang
` (12 subsequent siblings)
16 siblings, 0 replies; 37+ messages in thread
From: Yang Weijiang @ 2022-05-06 3:32 UTC (permalink / raw)
To: pbonzini, jmattson, seanjc, kan.liang, like.xu.linux, vkuznets,
wei.w.wang, kvm, linux-kernel
Cc: Yang Weijiang, Zhang Yi Z
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>
---
arch/x86/kvm/cpuid.c | 16 +++++++++++++---
arch/x86/kvm/x86.c | 6 ++++--
2 files changed, 17 insertions(+), 5 deletions(-)
diff --git a/arch/x86/kvm/cpuid.c b/arch/x86/kvm/cpuid.c
index 598334ed5fbc..63870f78ed16 100644
--- a/arch/x86/kvm/cpuid.c
+++ b/arch/x86/kvm/cpuid.c
@@ -239,9 +239,19 @@ static void __kvm_update_cpuid_runtime(struct kvm_vcpu *vcpu, struct kvm_cpuid_e
best->ebx = xstate_required_size(vcpu->arch.xcr0, false);
best = cpuid_entry2_find(entries, nent, 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;
+ }
+ }
best = __kvm_find_kvm_cpuid_features(vcpu, entries, nent);
if (kvm_hlt_in_guest(vcpu->kvm) && best &&
diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index 396b576f8c42..f78fc014aed3 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -3576,8 +3576,10 @@ int kvm_set_msr_common(struct kvm_vcpu *vcpu, struct msr_data *msr_info)
*/
if (data & ~supported_xss)
return 1;
- vcpu->arch.ia32_xss = data;
- kvm_update_cpuid_runtime(vcpu);
+ 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.27.0
^ permalink raw reply related [flat|nested] 37+ messages in thread* [PATCH v11 05/16] KVM: x86: Add Arch LBR MSRs to msrs_to_save_all list
2022-05-06 3:32 [PATCH v11 00/16] Introduce Architectural LBR for vPMU Yang Weijiang
` (3 preceding siblings ...)
2022-05-06 3:32 ` [PATCH v11 04/16] KVM: x86: Refresh CPUID on writes to MSR_IA32_XSS Yang Weijiang
@ 2022-05-06 3:32 ` Yang Weijiang
2022-05-06 3:32 ` [PATCH v11 06/16] KVM: vmx/pmu: Emulate MSR_ARCH_LBR_DEPTH for guest Arch LBR Yang Weijiang
` (11 subsequent siblings)
16 siblings, 0 replies; 37+ messages in thread
From: Yang Weijiang @ 2022-05-06 3:32 UTC (permalink / raw)
To: pbonzini, jmattson, seanjc, kan.liang, like.xu.linux, vkuznets,
wei.w.wang, kvm, linux-kernel
Cc: Yang Weijiang
Arch LBR MSR_ARCH_LBR_DEPTH and MSR_ARCH_LBR_CTL are queried by
userspace application before it wants to {save|restore} the Arch LBR
data. Other LBR related data MSRs are omitted here intentionally due
to lengthy list(32*3). Userspace can still use KVM_{GET|SET}_MSRS to
access them if necessary.
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 f78fc014aed3..8437d0c9804f 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -1429,6 +1429,7 @@ static const u32 msrs_to_save_all[] = {
MSR_F15H_PERF_CTR3, MSR_F15H_PERF_CTR4, MSR_F15H_PERF_CTR5,
MSR_IA32_XFD, MSR_IA32_XFD_ERR,
MSR_IA32_XSS,
+ MSR_ARCH_LBR_CTL, MSR_ARCH_LBR_DEPTH,
};
static u32 msrs_to_save[ARRAY_SIZE(msrs_to_save_all)];
@@ -6660,6 +6661,11 @@ static void kvm_init_msr_list(void)
if (!supported_xss)
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.27.0
^ permalink raw reply related [flat|nested] 37+ messages in thread* [PATCH v11 06/16] KVM: vmx/pmu: Emulate MSR_ARCH_LBR_DEPTH for guest Arch LBR
2022-05-06 3:32 [PATCH v11 00/16] Introduce Architectural LBR for vPMU Yang Weijiang
` (4 preceding siblings ...)
2022-05-06 3:32 ` [PATCH v11 05/16] KVM: x86: Add Arch LBR MSRs to msrs_to_save_all list Yang Weijiang
@ 2022-05-06 3:32 ` Yang Weijiang
2022-05-06 14:39 ` Liang, Kan
2022-05-06 3:32 ` [PATCH v11 07/16] KVM: vmx/pmu: Emulate MSR_ARCH_LBR_CTL " Yang Weijiang
` (10 subsequent siblings)
16 siblings, 1 reply; 37+ messages in thread
From: Yang Weijiang @ 2022-05-06 3:32 UTC (permalink / raw)
To: pbonzini, jmattson, seanjc, kan.liang, like.xu.linux, vkuznets,
wei.w.wang, 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. In the first generation of Arch LBR, max entry size is 32,
host configures the max size and guest always honors the setting.
Write to MSR_ARCH_LBR_DEPTH has side-effect, all LBR entries are reset
to 0. Kernel PMU driver can leverage this effect to do fask reset to
LBR record MSRs. KVM allows guest to achieve it when Arch LBR records
MSRs are passed through to the guest.
Signed-off-by: Like Xu <like.xu@linux.intel.com>
Co-developed-by: Yang Weijiang <weijiang.yang@intel.com>
Signed-off-by: Yang Weijiang <weijiang.yang@intel.com>
---
arch/x86/include/asm/kvm_host.h | 3 ++
arch/x86/kvm/vmx/pmu_intel.c | 50 ++++++++++++++++++++++++++++++++-
2 files changed, 52 insertions(+), 1 deletion(-)
diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
index 4ff36610af6a..753e3ecac1a1 100644
--- a/arch/x86/include/asm/kvm_host.h
+++ b/arch/x86/include/asm/kvm_host.h
@@ -534,6 +534,9 @@ struct kvm_pmu {
* redundant check before cleanup if guest don't use vPMU at all.
*/
u8 event_count;
+
+ /* Guest arch lbr depth supported by KVM. */
+ u64 kvm_arch_lbr_depth;
};
struct kvm_pmu_ops;
diff --git a/arch/x86/kvm/vmx/pmu_intel.c b/arch/x86/kvm/vmx/pmu_intel.c
index b82b6709d7a8..e2b5fc1f4f1a 100644
--- a/arch/x86/kvm/vmx/pmu_intel.c
+++ b/arch/x86/kvm/vmx/pmu_intel.c
@@ -192,6 +192,12 @@ static bool intel_pmu_is_valid_lbr_msr(struct kvm_vcpu *vcpu, u32 index)
if (!intel_pmu_lbr_is_enabled(vcpu))
return ret;
+ if (index == MSR_ARCH_LBR_DEPTH) {
+ if (kvm_cpu_cap_has(X86_FEATURE_ARCH_LBR))
+ ret = guest_cpuid_has(vcpu, X86_FEATURE_ARCH_LBR);
+ 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);
@@ -205,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:
@@ -342,10 +348,26 @@ 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)
+{
+ struct kvm_pmu *pmu = vcpu_to_pmu(vcpu);
+
+ if (!kvm_cpu_cap_has(X86_FEATURE_ARCH_LBR))
+ return false;
+
+ return (depth == pmu->kvm_arch_lbr_depth);
+}
+
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) {
@@ -361,6 +383,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 = 0;
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))) {
@@ -387,6 +412,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;
u64 reserved_bits;
@@ -421,6 +447,16 @@ 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;
+ /*
+ * Writing depth MSR from guest could either setting the
+ * MSR or resetting the LBR records with the side-effect.
+ */
+ 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))) {
@@ -555,6 +591,18 @@ static void intel_pmu_refresh(struct kvm_vcpu *vcpu)
if (lbr_desc->records.nr)
bitmap_set(pmu->all_valid_pmc_idx, INTEL_PMC_IDX_FIXED_VLBR, 1);
+
+ if (!kvm_cpu_cap_has(X86_FEATURE_ARCH_LBR))
+ return;
+
+ entry = kvm_find_cpuid_entry(vcpu, 28, 0);
+ if (entry) {
+ /*
+ * The depth mask in CPUID is fixed to host supported
+ * value when userspace sets guest CPUID.
+ */
+ pmu->kvm_arch_lbr_depth = fls(entry->eax & 0xff) * 8;
+ }
}
static void intel_pmu_init(struct kvm_vcpu *vcpu)
--
2.27.0
^ permalink raw reply related [flat|nested] 37+ messages in thread* Re: [PATCH v11 06/16] KVM: vmx/pmu: Emulate MSR_ARCH_LBR_DEPTH for guest Arch LBR
2022-05-06 3:32 ` [PATCH v11 06/16] KVM: vmx/pmu: Emulate MSR_ARCH_LBR_DEPTH for guest Arch LBR Yang Weijiang
@ 2022-05-06 14:39 ` Liang, Kan
0 siblings, 0 replies; 37+ messages in thread
From: Liang, Kan @ 2022-05-06 14:39 UTC (permalink / raw)
To: Yang Weijiang, pbonzini, jmattson, seanjc, like.xu.linux,
vkuznets, wei.w.wang, kvm, linux-kernel
On 5/5/2022 11:32 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. In the first generation of Arch LBR, max entry size is 32,
> host configures the max size and guest always honors the setting.
>
> Write to MSR_ARCH_LBR_DEPTH has side-effect, all LBR entries are reset
> to 0. Kernel PMU driver can leverage this effect to do fask reset to
> LBR record MSRs. KVM allows guest to achieve it when Arch LBR records
> MSRs are passed through to the guest.
>
> Signed-off-by: Like Xu <like.xu@linux.intel.com>
> Co-developed-by: Yang Weijiang <weijiang.yang@intel.com>
> Signed-off-by: Yang Weijiang <weijiang.yang@intel.com>
> ---
> arch/x86/include/asm/kvm_host.h | 3 ++
> arch/x86/kvm/vmx/pmu_intel.c | 50 ++++++++++++++++++++++++++++++++-
> 2 files changed, 52 insertions(+), 1 deletion(-)
>
> diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
> index 4ff36610af6a..753e3ecac1a1 100644
> --- a/arch/x86/include/asm/kvm_host.h
> +++ b/arch/x86/include/asm/kvm_host.h
> @@ -534,6 +534,9 @@ struct kvm_pmu {
> * redundant check before cleanup if guest don't use vPMU at all.
> */
> u8 event_count;
> +
> + /* Guest arch lbr depth supported by KVM. */
> + u64 kvm_arch_lbr_depth;
> };
>
> struct kvm_pmu_ops;
> diff --git a/arch/x86/kvm/vmx/pmu_intel.c b/arch/x86/kvm/vmx/pmu_intel.c
> index b82b6709d7a8..e2b5fc1f4f1a 100644
> --- a/arch/x86/kvm/vmx/pmu_intel.c
> +++ b/arch/x86/kvm/vmx/pmu_intel.c
> @@ -192,6 +192,12 @@ static bool intel_pmu_is_valid_lbr_msr(struct kvm_vcpu *vcpu, u32 index)
> if (!intel_pmu_lbr_is_enabled(vcpu))
> return ret;
>
> + if (index == MSR_ARCH_LBR_DEPTH) {
> + if (kvm_cpu_cap_has(X86_FEATURE_ARCH_LBR))
> + ret = guest_cpuid_has(vcpu, X86_FEATURE_ARCH_LBR);
> + 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);
> @@ -205,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;
>
I don't think you need this change anymore, since the MSR_ARCH_LBR_DEPTH
has been moved to the other place.
After the above is removed, the patch looks good to me.
Reviewed-by: Kan Liang <kan.liang@linux.intel.com>
Thanks,
Kan
> switch (msr) {
> case MSR_CORE_PERF_FIXED_CTR_CTRL:
> @@ -342,10 +348,26 @@ 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)
> +{
> + struct kvm_pmu *pmu = vcpu_to_pmu(vcpu);
> +
> + if (!kvm_cpu_cap_has(X86_FEATURE_ARCH_LBR))
> + return false;
> +
> + return (depth == pmu->kvm_arch_lbr_depth);
> +}
> +
> 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) {
> @@ -361,6 +383,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 = 0;
> 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))) {
> @@ -387,6 +412,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;
> u64 reserved_bits;
> @@ -421,6 +447,16 @@ 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;
> + /*
> + * Writing depth MSR from guest could either setting the
> + * MSR or resetting the LBR records with the side-effect.
> + */
> + 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))) {
> @@ -555,6 +591,18 @@ static void intel_pmu_refresh(struct kvm_vcpu *vcpu)
>
> if (lbr_desc->records.nr)
> bitmap_set(pmu->all_valid_pmc_idx, INTEL_PMC_IDX_FIXED_VLBR, 1);
> +
> + if (!kvm_cpu_cap_has(X86_FEATURE_ARCH_LBR))
> + return;
> +
> + entry = kvm_find_cpuid_entry(vcpu, 28, 0);
> + if (entry) {
> + /*
> + * The depth mask in CPUID is fixed to host supported
> + * value when userspace sets guest CPUID.
> + */
> + pmu->kvm_arch_lbr_depth = fls(entry->eax & 0xff) * 8;
> + }
> }
>
> static void intel_pmu_init(struct kvm_vcpu *vcpu)
^ permalink raw reply [flat|nested] 37+ messages in thread
* [PATCH v11 07/16] KVM: vmx/pmu: Emulate MSR_ARCH_LBR_CTL for guest Arch LBR
2022-05-06 3:32 [PATCH v11 00/16] Introduce Architectural LBR for vPMU Yang Weijiang
` (5 preceding siblings ...)
2022-05-06 3:32 ` [PATCH v11 06/16] KVM: vmx/pmu: Emulate MSR_ARCH_LBR_DEPTH for guest Arch LBR Yang Weijiang
@ 2022-05-06 3:32 ` Yang Weijiang
2022-05-06 14:42 ` Liang, Kan
2022-05-06 3:32 ` [PATCH v11 08/16] KVM: x86/pmu: Refactor code to support " Yang Weijiang
` (9 subsequent siblings)
16 siblings, 1 reply; 37+ messages in thread
From: Yang Weijiang @ 2022-05-06 3:32 UTC (permalink / raw)
To: pbonzini, jmattson, seanjc, kan.liang, like.xu.linux, vkuznets,
wei.w.wang, kvm, linux-kernel
Cc: Like Xu, Yang Weijiang
From: Like Xu <like.xu@linux.intel.com>
Arch LBR is 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>
Co-developed-by: Yang Weijiang <weijiang.yang@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 | 57 ++++++++++++++++++++++++++++----
arch/x86/kvm/vmx/vmx.c | 12 +++++++
5 files changed, 66 insertions(+), 8 deletions(-)
diff --git a/arch/x86/events/intel/lbr.c b/arch/x86/events/intel/lbr.c
index 4529ce448b2e..4fe6c3b50fc3 100644
--- a/arch/x86/events/intel/lbr.c
+++ b/arch/x86/events/intel/lbr.c
@@ -160,8 +160,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 ee15311b6be1..fdf0e3097e0b 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 e2b5fc1f4f1a..aa36d2072b91 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[] = {
[0] = { 0x3c, 0x00, PERF_COUNT_HW_CPU_CYCLES },
@@ -192,7 +193,7 @@ static bool intel_pmu_is_valid_lbr_msr(struct kvm_vcpu *vcpu, u32 index)
if (!intel_pmu_lbr_is_enabled(vcpu))
return ret;
- if (index == MSR_ARCH_LBR_DEPTH) {
+ if (index == MSR_ARCH_LBR_DEPTH || index == MSR_ARCH_LBR_CTL) {
if (kvm_cpu_cap_has(X86_FEATURE_ARCH_LBR))
ret = guest_cpuid_has(vcpu, X86_FEATURE_ARCH_LBR);
return ret;
@@ -363,6 +364,33 @@ static bool arch_lbr_depth_is_valid(struct kvm_vcpu *vcpu, u64 depth)
return (depth == pmu->kvm_arch_lbr_depth);
}
+static bool arch_lbr_ctl_is_valid(struct kvm_vcpu *vcpu, u64 ctl)
+{
+ struct kvm_cpuid_entry2 *entry;
+
+ if (!kvm_cpu_cap_has(X86_FEATURE_ARCH_LBR))
+ return false;
+
+ if (ctl & ~KVM_ARCH_LBR_CTL_MASK)
+ goto warn;
+
+ entry = kvm_find_cpuid_entry(vcpu, 0x1c, 0);
+ if (!entry)
+ return false;
+
+ if (!(entry->ebx & BIT(0)) && (ctl & ARCH_LBR_CTL_CPL))
+ return false;
+ if (!(entry->ebx & BIT(2)) && (ctl & ARCH_LBR_CTL_STACK))
+ return false;
+ if (!(entry->ebx & BIT(1)) && (ctl & ARCH_LBR_CTL_FILTER))
+ return false;
+ return true;
+warn:
+ pr_warn_ratelimited("kvm: vcpu-%d: invalid arch lbr ctl.\n",
+ vcpu->vcpu_id);
+ return false;
+}
+
static int intel_pmu_get_msr(struct kvm_vcpu *vcpu, struct msr_data *msr_info)
{
struct kvm_pmu *pmu = vcpu_to_pmu(vcpu);
@@ -386,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))) {
@@ -457,6 +488,16 @@ static int intel_pmu_set_msr(struct kvm_vcpu *vcpu, struct msr_data *msr_info)
*/
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))) {
@@ -666,12 +707,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 d58b763df855..b6bc7d97e4b4 100644
--- a/arch/x86/kvm/vmx/vmx.c
+++ b/arch/x86/kvm/vmx/vmx.c
@@ -2022,6 +2022,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))
@@ -4553,6 +4560,11 @@ static void vmx_vcpu_reset(struct kvm_vcpu *vcpu, bool init_event)
kvm_make_request(KVM_REQ_APIC_PAGE_RELOAD, vcpu);
vpid_sync_context(vmx->vpid);
+
+ if (!init_event) {
+ if (static_cpu_has(X86_FEATURE_ARCH_LBR))
+ vmcs_write64(GUEST_IA32_LBR_CTL, 0);
+ }
}
static void vmx_enable_irq_window(struct kvm_vcpu *vcpu)
--
2.27.0
^ permalink raw reply related [flat|nested] 37+ messages in thread* Re: [PATCH v11 07/16] KVM: vmx/pmu: Emulate MSR_ARCH_LBR_CTL for guest Arch LBR
2022-05-06 3:32 ` [PATCH v11 07/16] KVM: vmx/pmu: Emulate MSR_ARCH_LBR_CTL " Yang Weijiang
@ 2022-05-06 14:42 ` Liang, Kan
0 siblings, 0 replies; 37+ messages in thread
From: Liang, Kan @ 2022-05-06 14:42 UTC (permalink / raw)
To: Yang Weijiang, pbonzini, jmattson, seanjc, like.xu.linux,
vkuznets, wei.w.wang, kvm, linux-kernel
On 5/5/2022 11:32 PM, Yang Weijiang wrote:
> From: Like Xu <like.xu@linux.intel.com>
>
> Arch LBR is 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>
> Co-developed-by: Yang Weijiang <weijiang.yang@intel.com>
> Signed-off-by: Yang Weijiang <weijiang.yang@intel.com>
Reviewed-by: Kan Liang <kan.liang@linux.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 | 57 ++++++++++++++++++++++++++++----
> arch/x86/kvm/vmx/vmx.c | 12 +++++++
> 5 files changed, 66 insertions(+), 8 deletions(-)
>
> diff --git a/arch/x86/events/intel/lbr.c b/arch/x86/events/intel/lbr.c
> index 4529ce448b2e..4fe6c3b50fc3 100644
> --- a/arch/x86/events/intel/lbr.c
> +++ b/arch/x86/events/intel/lbr.c
> @@ -160,8 +160,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 ee15311b6be1..fdf0e3097e0b 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 e2b5fc1f4f1a..aa36d2072b91 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[] = {
> [0] = { 0x3c, 0x00, PERF_COUNT_HW_CPU_CYCLES },
> @@ -192,7 +193,7 @@ static bool intel_pmu_is_valid_lbr_msr(struct kvm_vcpu *vcpu, u32 index)
> if (!intel_pmu_lbr_is_enabled(vcpu))
> return ret;
>
> - if (index == MSR_ARCH_LBR_DEPTH) {
> + if (index == MSR_ARCH_LBR_DEPTH || index == MSR_ARCH_LBR_CTL) {
> if (kvm_cpu_cap_has(X86_FEATURE_ARCH_LBR))
> ret = guest_cpuid_has(vcpu, X86_FEATURE_ARCH_LBR);
> return ret;
> @@ -363,6 +364,33 @@ static bool arch_lbr_depth_is_valid(struct kvm_vcpu *vcpu, u64 depth)
> return (depth == pmu->kvm_arch_lbr_depth);
> }
>
> +static bool arch_lbr_ctl_is_valid(struct kvm_vcpu *vcpu, u64 ctl)
> +{
> + struct kvm_cpuid_entry2 *entry;
> +
> + if (!kvm_cpu_cap_has(X86_FEATURE_ARCH_LBR))
> + return false;
> +
> + if (ctl & ~KVM_ARCH_LBR_CTL_MASK)
> + goto warn;
> +
> + entry = kvm_find_cpuid_entry(vcpu, 0x1c, 0);
> + if (!entry)
> + return false;
> +
> + if (!(entry->ebx & BIT(0)) && (ctl & ARCH_LBR_CTL_CPL))
> + return false;
> + if (!(entry->ebx & BIT(2)) && (ctl & ARCH_LBR_CTL_STACK))
> + return false;
> + if (!(entry->ebx & BIT(1)) && (ctl & ARCH_LBR_CTL_FILTER))
> + return false;
> + return true;
> +warn:
> + pr_warn_ratelimited("kvm: vcpu-%d: invalid arch lbr ctl.\n",
> + vcpu->vcpu_id);
> + return false;
> +}
> +
> static int intel_pmu_get_msr(struct kvm_vcpu *vcpu, struct msr_data *msr_info)
> {
> struct kvm_pmu *pmu = vcpu_to_pmu(vcpu);
> @@ -386,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))) {
> @@ -457,6 +488,16 @@ static int intel_pmu_set_msr(struct kvm_vcpu *vcpu, struct msr_data *msr_info)
> */
> 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))) {
> @@ -666,12 +707,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 d58b763df855..b6bc7d97e4b4 100644
> --- a/arch/x86/kvm/vmx/vmx.c
> +++ b/arch/x86/kvm/vmx/vmx.c
> @@ -2022,6 +2022,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))
> @@ -4553,6 +4560,11 @@ static void vmx_vcpu_reset(struct kvm_vcpu *vcpu, bool init_event)
> kvm_make_request(KVM_REQ_APIC_PAGE_RELOAD, vcpu);
>
> vpid_sync_context(vmx->vpid);
> +
> + if (!init_event) {
> + if (static_cpu_has(X86_FEATURE_ARCH_LBR))
> + vmcs_write64(GUEST_IA32_LBR_CTL, 0);
> + }
> }
>
> static void vmx_enable_irq_window(struct kvm_vcpu *vcpu)
^ permalink raw reply [flat|nested] 37+ messages in thread
* [PATCH v11 08/16] KVM: x86/pmu: Refactor code to support guest Arch LBR
2022-05-06 3:32 [PATCH v11 00/16] Introduce Architectural LBR for vPMU Yang Weijiang
` (6 preceding siblings ...)
2022-05-06 3:32 ` [PATCH v11 07/16] KVM: vmx/pmu: Emulate MSR_ARCH_LBR_CTL " Yang Weijiang
@ 2022-05-06 3:32 ` Yang Weijiang
2022-05-06 15:03 ` Liang, Kan
2022-05-06 3:32 ` [PATCH v11 09/16] KVM: x86: Refine the matching and clearing logic for supported_xss Yang Weijiang
` (8 subsequent siblings)
16 siblings, 1 reply; 37+ messages in thread
From: Yang Weijiang @ 2022-05-06 3:32 UTC (permalink / raw)
To: pbonzini, jmattson, seanjc, kan.liang, like.xu.linux, vkuznets,
wei.w.wang, kvm, linux-kernel
Cc: Yang Weijiang, Like Xu
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 | 40 ++++++++++++++++++++++++++++--------
arch/x86/kvm/vmx/vmx.c | 3 +++
2 files changed, 35 insertions(+), 8 deletions(-)
diff --git a/arch/x86/kvm/vmx/pmu_intel.c b/arch/x86/kvm/vmx/pmu_intel.c
index aa36d2072b91..bd4ddf63ba8f 100644
--- a/arch/x86/kvm/vmx/pmu_intel.c
+++ b/arch/x86/kvm/vmx/pmu_intel.c
@@ -170,12 +170,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,20 @@ static bool intel_pmu_is_valid_lbr_msr(struct kvm_vcpu *vcpu, u32 index)
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);
+ if (!ret && records->info) {
+ ret = (index >= records->info &&
+ index < records->info + records->nr);
+ }
return ret;
}
@@ -742,6 +754,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);
}
@@ -782,10 +797,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;
@@ -802,13 +820,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 b6bc7d97e4b4..98e56a909c01 100644
--- a/arch/x86/kvm/vmx/vmx.c
+++ b/arch/x86/kvm/vmx/vmx.c
@@ -573,6 +573,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.27.0
^ permalink raw reply related [flat|nested] 37+ messages in thread* Re: [PATCH v11 08/16] KVM: x86/pmu: Refactor code to support guest Arch LBR
2022-05-06 3:32 ` [PATCH v11 08/16] KVM: x86/pmu: Refactor code to support " Yang Weijiang
@ 2022-05-06 15:03 ` Liang, Kan
2022-05-07 2:32 ` Yang, Weijiang
0 siblings, 1 reply; 37+ messages in thread
From: Liang, Kan @ 2022-05-06 15:03 UTC (permalink / raw)
To: Yang Weijiang, pbonzini, jmattson, seanjc, like.xu.linux,
vkuznets, wei.w.wang, kvm, linux-kernel
On 5/5/2022 11:32 PM, Yang Weijiang wrote:
> 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 | 40 ++++++++++++++++++++++++++++--------
> arch/x86/kvm/vmx/vmx.c | 3 +++
> 2 files changed, 35 insertions(+), 8 deletions(-)
>
> diff --git a/arch/x86/kvm/vmx/pmu_intel.c b/arch/x86/kvm/vmx/pmu_intel.c
> index aa36d2072b91..bd4ddf63ba8f 100644
> --- a/arch/x86/kvm/vmx/pmu_intel.c
> +++ b/arch/x86/kvm/vmx/pmu_intel.c
> @@ -170,12 +170,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,20 @@ static bool intel_pmu_is_valid_lbr_msr(struct kvm_vcpu *vcpu, u32 index)
> 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);
> +
Shouldn't we return immediately if (ret == true)?
Keep checking if (!ret) looks uncommon.
Actually we probably don't need the ret in this function.
if (!guest_cpuid_has(vcpu, X86_FEATURE_ARCH_LBR) &&
((index == MSR_LBR_SELECT) || (index == MSR_LBR_TOS)))
return true;
> + if (!ret) {
> + ret = (index >= records->from &&
> + index < records->from + records->nr) ||
> + (index >= records->to &&
> + index < records->to + records->nr);
> + }
if ((index >= records->from &&
index < records->from + records->nr) ||
(index >= records->to &&
index < records->to + records->nr))
return true;
>
> - if (!ret && records->info)
> - ret = (index >= records->info && index < records->info + records->nr);
> + if (!ret && records->info) {
> + ret = (index >= records->info &&
> + index < records->info + records->nr);
> + }
if (records->info &&
(index >= records->info && index < records->info + records->nr)
return true;
return false;
Sorry, I didn't notice it in the previous review.
Thanks,
Kan
>
> return ret;
> }
> @@ -742,6 +754,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);
> }
> @@ -782,10 +797,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;
> @@ -802,13 +820,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 b6bc7d97e4b4..98e56a909c01 100644
> --- a/arch/x86/kvm/vmx/vmx.c
> +++ b/arch/x86/kvm/vmx/vmx.c
> @@ -573,6 +573,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;
> }
^ permalink raw reply [flat|nested] 37+ messages in thread* Re: [PATCH v11 08/16] KVM: x86/pmu: Refactor code to support guest Arch LBR
2022-05-06 15:03 ` Liang, Kan
@ 2022-05-07 2:32 ` Yang, Weijiang
2022-05-09 14:06 ` Liang, Kan
0 siblings, 1 reply; 37+ messages in thread
From: Yang, Weijiang @ 2022-05-07 2:32 UTC (permalink / raw)
To: Liang, Kan, pbonzini, jmattson, seanjc, like.xu.linux, vkuznets,
Wang, Wei W, kvm, linux-kernel
On 5/6/2022 11:03 PM, Liang, Kan wrote:
> On 5/5/2022 11:32 PM, Yang Weijiang wrote:
>
> bool intel_pmu_lbr_is_enabled(struct kvm_vcpu *vcpu)
> @@ -199,12 +203,20 @@ static bool intel_pmu_is_valid_lbr_msr(struct kvm_vcpu *vcpu, u32 index)
> 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);
> +
> Shouldn't we return immediately if (ret == true)?
> Keep checking if (!ret) looks uncommon.
>
> Actually we probably don't need the ret in this function.
>
> if (!guest_cpuid_has(vcpu, X86_FEATURE_ARCH_LBR) &&
> ((index == MSR_LBR_SELECT) || (index == MSR_LBR_TOS)))
> return true;
>
>> + if (!ret) {
>> + ret = (index >= records->from &&
>> + index < records->from + records->nr) ||
>> + (index >= records->to &&
>> + index < records->to + records->nr);
>> + }
> if ((index >= records->from &&
> index < records->from + records->nr) ||
> (index >= records->to &&
> index < records->to + records->nr))
> return true;
>
>>
>> - if (!ret && records->info)
>> - ret = (index >= records->info && index < records->info + records->nr);
>> + if (!ret && records->info) {
>> + ret = (index >= records->info &&
>> + index < records->info + records->nr);
>> + }
> if (records->info &&
> (index >= records->info && index < records->info + records->nr)
> return true;
>
> return false;
> Sorry, I didn't notice it in the previous review.
Thanks Kan, so I'll modify this function as below (keeping other part
unchanged):
From 642d5e05e8a8578e75531632d714cec5976ab9ac Mon Sep 17 00:00:00 2001
From: Yang Weijiang <weijiang.yang@intel.com>
Date: Thu, 8 Jul 2021 23:51:02 +0800
Subject: [PATCH] KVM: x86/pmu: Refactor code to support guest Arch LBR
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 | 47 +++++++++++++++++++++++++-----------
arch/x86/kvm/vmx/vmx.c | 3 +++
2 files changed, 36 insertions(+), 14 deletions(-)
diff --git a/arch/x86/kvm/vmx/pmu_intel.c b/arch/x86/kvm/vmx/pmu_intel.c
index aa36d2072b91..306ce7ac9934 100644
--- a/arch/x86/kvm/vmx/pmu_intel.c
+++ b/arch/x86/kvm/vmx/pmu_intel.c
@@ -170,12 +170,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)
@@ -188,25 +192,28 @@ bool intel_pmu_lbr_is_enabled(struct kvm_vcpu *vcpu)
static bool intel_pmu_is_valid_lbr_msr(struct kvm_vcpu *vcpu, u32 index)
{
struct x86_pmu_lbr *records = vcpu_to_lbr_records(vcpu);
- bool ret = false;
if (!intel_pmu_lbr_is_enabled(vcpu))
- return ret;
+ return false;
if (index == MSR_ARCH_LBR_DEPTH || index == MSR_ARCH_LBR_CTL) {
- if (kvm_cpu_cap_has(X86_FEATURE_ARCH_LBR))
- ret = guest_cpuid_has(vcpu, X86_FEATURE_ARCH_LBR);
- return ret;
+ return kvm_cpu_cap_has(X86_FEATURE_ARCH_LBR) &&
+ guest_cpuid_has(vcpu, X86_FEATURE_ARCH_LBR);
}
- 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) &&
+ (index == MSR_LBR_SELECT || index == MSR_LBR_TOS))
+ return true;
- if (!ret && records->info)
- ret = (index >= records->info && index < records->info +
records->nr);
+ if ((index >= records->from && index < records->from +
records->nr) ||
+ (index >= records->to && index < records->to + records->nr))
+ return true;
- return ret;
+ if (records->info && index >= records->info &&
+ index < records->info + records->nr)
+ return true;
+
+ return false;
}
static bool intel_is_valid_msr(struct kvm_vcpu *vcpu, u32 msr)
@@ -742,6 +749,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);
}
@@ -782,10 +792,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;
@@ -802,13 +815,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 b6bc7d97e4b4..98e56a909c01 100644
--- a/arch/x86/kvm/vmx/vmx.c
+++ b/arch/x86/kvm/vmx/vmx.c
@@ -573,6 +573,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.27.0
> Thanks,
> Kan
>
^ permalink raw reply related [flat|nested] 37+ messages in thread* Re: [PATCH v11 08/16] KVM: x86/pmu: Refactor code to support guest Arch LBR
2022-05-07 2:32 ` Yang, Weijiang
@ 2022-05-09 14:06 ` Liang, Kan
0 siblings, 0 replies; 37+ messages in thread
From: Liang, Kan @ 2022-05-09 14:06 UTC (permalink / raw)
To: Yang, Weijiang, pbonzini, jmattson, seanjc, like.xu.linux,
vkuznets, Wang, Wei W, kvm, linux-kernel
On 5/6/2022 10:32 PM, Yang, Weijiang wrote:
>
> On 5/6/2022 11:03 PM, Liang, Kan wrote:
>> On 5/5/2022 11:32 PM, Yang Weijiang wrote:
>>
>> bool intel_pmu_lbr_is_enabled(struct kvm_vcpu *vcpu)
>> @@ -199,12 +203,20 @@ static bool intel_pmu_is_valid_lbr_msr(struct
>> kvm_vcpu *vcpu, u32 index)
>> 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);
>> +
>> Shouldn't we return immediately if (ret == true)?
>> Keep checking if (!ret) looks uncommon.
>>
>> Actually we probably don't need the ret in this function.
>>
>> if (!guest_cpuid_has(vcpu, X86_FEATURE_ARCH_LBR) &&
>> ((index == MSR_LBR_SELECT) || (index == MSR_LBR_TOS)))
>> return true;
>>
>>> + if (!ret) {
>>> + ret = (index >= records->from &&
>>> + index < records->from + records->nr) ||
>>> + (index >= records->to &&
>>> + index < records->to + records->nr);
>>> + }
>> if ((index >= records->from &&
>> index < records->from + records->nr) ||
>> (index >= records->to &&
>> index < records->to + records->nr))
>> return true;
>>
>>> - if (!ret && records->info)
>>> - ret = (index >= records->info && index < records->info +
>>> records->nr);
>>> + if (!ret && records->info) {
>>> + ret = (index >= records->info &&
>>> + index < records->info + records->nr);
>>> + }
>> if (records->info &&
>> (index >= records->info && index < records->info + records->nr)
>> return true;
>>
>> return false;
>> Sorry, I didn't notice it in the previous review.
>
> Thanks Kan, so I'll modify this function as below (keeping other part
> unchanged):
>
> From 642d5e05e8a8578e75531632d714cec5976ab9ac Mon Sep 17 00:00:00 2001
> From: Yang Weijiang <weijiang.yang@intel.com>
> Date: Thu, 8 Jul 2021 23:51:02 +0800
> Subject: [PATCH] KVM: x86/pmu: Refactor code to support guest Arch LBR
>
> 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>
> ---
This one looks good to me.
Reviewed-by: Kan Liang <kan.liang@linux.intel.com>
Thanks,
Kan
> arch/x86/kvm/vmx/pmu_intel.c | 47 +++++++++++++++++++++++++-----------
> arch/x86/kvm/vmx/vmx.c | 3 +++
> 2 files changed, 36 insertions(+), 14 deletions(-)
>
> diff --git a/arch/x86/kvm/vmx/pmu_intel.c b/arch/x86/kvm/vmx/pmu_intel.c
> index aa36d2072b91..306ce7ac9934 100644
> --- a/arch/x86/kvm/vmx/pmu_intel.c
> +++ b/arch/x86/kvm/vmx/pmu_intel.c
> @@ -170,12 +170,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)
> @@ -188,25 +192,28 @@ bool intel_pmu_lbr_is_enabled(struct kvm_vcpu *vcpu)
> static bool intel_pmu_is_valid_lbr_msr(struct kvm_vcpu *vcpu, u32 index)
> {
> struct x86_pmu_lbr *records = vcpu_to_lbr_records(vcpu);
> - bool ret = false;
>
> if (!intel_pmu_lbr_is_enabled(vcpu))
> - return ret;
> + return false;
>
> if (index == MSR_ARCH_LBR_DEPTH || index == MSR_ARCH_LBR_CTL) {
> - if (kvm_cpu_cap_has(X86_FEATURE_ARCH_LBR))
> - ret = guest_cpuid_has(vcpu, X86_FEATURE_ARCH_LBR);
> - return ret;
> + return kvm_cpu_cap_has(X86_FEATURE_ARCH_LBR) &&
> + guest_cpuid_has(vcpu, X86_FEATURE_ARCH_LBR);
> }
>
> - 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) &&
> + (index == MSR_LBR_SELECT || index == MSR_LBR_TOS))
> + return true;
>
> - if (!ret && records->info)
> - ret = (index >= records->info && index < records->info +
> records->nr);
> + if ((index >= records->from && index < records->from +
> records->nr) ||
> + (index >= records->to && index < records->to + records->nr))
> + return true;
>
> - return ret;
> + if (records->info && index >= records->info &&
> + index < records->info + records->nr)
> + return true;
> +
> + return false;
> }
>
> static bool intel_is_valid_msr(struct kvm_vcpu *vcpu, u32 msr)
> @@ -742,6 +749,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);
> }
> @@ -782,10 +792,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;
> @@ -802,13 +815,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 b6bc7d97e4b4..98e56a909c01 100644
> --- a/arch/x86/kvm/vmx/vmx.c
> +++ b/arch/x86/kvm/vmx/vmx.c
> @@ -573,6 +573,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.27.0
>
>> Thanks,
>> Kan
>>
^ permalink raw reply [flat|nested] 37+ messages in thread
* [PATCH v11 09/16] KVM: x86: Refine the matching and clearing logic for supported_xss
2022-05-06 3:32 [PATCH v11 00/16] Introduce Architectural LBR for vPMU Yang Weijiang
` (7 preceding siblings ...)
2022-05-06 3:32 ` [PATCH v11 08/16] KVM: x86/pmu: Refactor code to support " Yang Weijiang
@ 2022-05-06 3:32 ` Yang Weijiang
2022-05-06 3:32 ` [PATCH v11 10/16] KVM: x86: Add XSAVE Support for Architectural LBR Yang Weijiang
` (7 subsequent siblings)
16 siblings, 0 replies; 37+ messages in thread
From: Yang Weijiang @ 2022-05-06 3:32 UTC (permalink / raw)
To: pbonzini, jmattson, seanjc, kan.liang, like.xu.linux, vkuznets,
wei.w.wang, 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 98e56a909c01..379d13aa65c0 100644
--- a/arch/x86/kvm/vmx/vmx.c
+++ b/arch/x86/kvm/vmx/vmx.c
@@ -7444,9 +7444,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 8437d0c9804f..7a181bd34b82 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -224,6 +224,8 @@ static struct kvm_user_return_msrs __percpu *user_return_msrs;
| XFEATURE_MASK_BNDCSR | XFEATURE_MASK_AVX512 \
| XFEATURE_MASK_PKRU | XFEATURE_MASK_XTILE)
+#define KVM_SUPPORTED_XSS 0
+
u64 __read_mostly host_efer;
EXPORT_SYMBOL_GPL(host_efer);
@@ -11601,8 +11603,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.27.0
^ permalink raw reply related [flat|nested] 37+ messages in thread* [PATCH v11 10/16] KVM: x86: Add XSAVE Support for Architectural LBR
2022-05-06 3:32 [PATCH v11 00/16] Introduce Architectural LBR for vPMU Yang Weijiang
` (8 preceding siblings ...)
2022-05-06 3:32 ` [PATCH v11 09/16] KVM: x86: Refine the matching and clearing logic for supported_xss Yang Weijiang
@ 2022-05-06 3:32 ` Yang Weijiang
2022-05-06 3:33 ` [PATCH v11 11/16] KVM: x86/vmx: Check Arch LBR config when return perf capabilities Yang Weijiang
` (6 subsequent siblings)
16 siblings, 0 replies; 37+ messages in thread
From: Yang Weijiang @ 2022-05-06 3:32 UTC (permalink / raw)
To: pbonzini, jmattson, seanjc, kan.liang, like.xu.linux, vkuznets,
wei.w.wang, 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 379d13aa65c0..97b123b18e57 100644
--- a/arch/x86/kvm/vmx/vmx.c
+++ b/arch/x86/kvm/vmx/vmx.c
@@ -7432,6 +7432,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 7a181bd34b82..a722fe6e18ff 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -224,7 +224,7 @@ static struct kvm_user_return_msrs __percpu *user_return_msrs;
| XFEATURE_MASK_BNDCSR | XFEATURE_MASK_AVX512 \
| XFEATURE_MASK_PKRU | XFEATURE_MASK_XTILE)
-#define KVM_SUPPORTED_XSS 0
+#define KVM_SUPPORTED_XSS XFEATURE_MASK_LBR
u64 __read_mostly host_efer;
EXPORT_SYMBOL_GPL(host_efer);
--
2.27.0
^ permalink raw reply related [flat|nested] 37+ messages in thread* [PATCH v11 11/16] KVM: x86/vmx: Check Arch LBR config when return perf capabilities
2022-05-06 3:32 [PATCH v11 00/16] Introduce Architectural LBR for vPMU Yang Weijiang
` (9 preceding siblings ...)
2022-05-06 3:32 ` [PATCH v11 10/16] KVM: x86: Add XSAVE Support for Architectural LBR Yang Weijiang
@ 2022-05-06 3:33 ` Yang Weijiang
2022-05-06 3:33 ` [PATCH v11 12/16] KVM: nVMX: Add necessary Arch LBR settings for nested VM Yang Weijiang
` (5 subsequent siblings)
16 siblings, 0 replies; 37+ messages in thread
From: Yang Weijiang @ 2022-05-06 3:33 UTC (permalink / raw)
To: pbonzini, jmattson, seanjc, kan.liang, like.xu.linux, vkuznets,
wei.w.wang, kvm, linux-kernel
Cc: Yang Weijiang, Like Xu
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 | 8 ++++++++
arch/x86/kvm/vmx/vmx.c | 10 ++++++++--
3 files changed, 18 insertions(+), 2 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 3f430e218375..68fbb76ba439 100644
--- a/arch/x86/kvm/vmx/capabilities.h
+++ b/arch/x86/kvm/vmx/capabilities.h
@@ -385,6 +385,12 @@ static inline bool vmx_pt_mode_is_host_guest(void)
return pt_mode == PT_MODE_HOST_GUEST;
}
+static inline bool cpu_has_vmx_arch_lbr(void)
+{
+ 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)
{
u64 perf_cap = 0;
@@ -396,6 +402,8 @@ static inline u64 vmx_get_perf_capabilities(void)
rdmsrl(MSR_IA32_PERF_CAPABILITIES, perf_cap);
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;
/*
* Since counters are virtualized, KVM would support full
diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c
index 97b123b18e57..e6384ef1d115 100644
--- a/arch/x86/kvm/vmx/vmx.c
+++ b/arch/x86/kvm/vmx/vmx.c
@@ -2532,7 +2532,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;
@@ -2556,7 +2557,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;
@@ -5930,6 +5932,10 @@ void dump_vmcs(struct kvm_vcpu *vcpu)
vmentry_ctl & VM_ENTRY_LOAD_IA32_PERF_GLOBAL_CTRL)
pr_err("PerfGlobCtl = 0x%016llx\n",
vmcs_read64(GUEST_IA32_PERF_GLOBAL_CTRL));
+ if (kvm_cpu_cap_has(X86_FEATURE_ARCH_LBR) &&
+ vmentry_ctl & VM_ENTRY_LOAD_IA32_LBR_CTL)
+ pr_err("ArchLBRCtl = 0x%016llx\n",
+ vmcs_read64(GUEST_IA32_LBR_CTL));
if (vmentry_ctl & VM_ENTRY_LOAD_BNDCFGS)
pr_err("BndCfgS = 0x%016llx\n", vmcs_read64(GUEST_BNDCFGS));
pr_err("Interruptibility = %08x ActivityState = %08x\n",
--
2.27.0
^ permalink raw reply related [flat|nested] 37+ messages in thread* [PATCH v11 12/16] KVM: nVMX: Add necessary Arch LBR settings for nested VM
2022-05-06 3:32 [PATCH v11 00/16] Introduce Architectural LBR for vPMU Yang Weijiang
` (10 preceding siblings ...)
2022-05-06 3:33 ` [PATCH v11 11/16] KVM: x86/vmx: Check Arch LBR config when return perf capabilities Yang Weijiang
@ 2022-05-06 3:33 ` Yang Weijiang
2022-05-06 3:33 ` [PATCH v11 13/16] KVM: x86/vmx: Clear Arch LBREn bit before inject #DB to guest Yang Weijiang
` (4 subsequent siblings)
16 siblings, 0 replies; 37+ messages in thread
From: Yang Weijiang @ 2022-05-06 3:33 UTC (permalink / raw)
To: pbonzini, jmattson, seanjc, kan.liang, like.xu.linux, vkuznets,
wei.w.wang, 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 | 7 +++++--
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, 10 insertions(+), 3 deletions(-)
diff --git a/arch/x86/kvm/vmx/nested.c b/arch/x86/kvm/vmx/nested.c
index 856c87563883..2434d8bd5e30 100644
--- a/arch/x86/kvm/vmx/nested.c
+++ b/arch/x86/kvm/vmx/nested.c
@@ -6563,7 +6563,9 @@ 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 |
@@ -6583,7 +6585,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 bd4ddf63ba8f..3adc8f28d142 100644
--- a/arch/x86/kvm/vmx/pmu_intel.c
+++ b/arch/x86/kvm/vmx/pmu_intel.c
@@ -198,6 +198,8 @@ static bool intel_pmu_is_valid_lbr_msr(struct kvm_vcpu *vcpu, u32 index)
return ret;
if (index == MSR_ARCH_LBR_DEPTH || index == MSR_ARCH_LBR_CTL) {
+ if (is_guest_mode(vcpu))
+ return ret;
if (kvm_cpu_cap_has(X86_FEATURE_ARCH_LBR))
ret = guest_cpuid_has(vcpu, X86_FEATURE_ARCH_LBR);
return ret;
diff --git a/arch/x86/kvm/vmx/vmcs12.c b/arch/x86/kvm/vmx/vmcs12.c
index 2251b60920f8..bcda664e4d26 100644
--- a/arch/x86/kvm/vmx/vmcs12.c
+++ b/arch/x86/kvm/vmx/vmcs12.c
@@ -65,6 +65,7 @@ const unsigned short vmcs12_field_offsets[] = {
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 746129ddd5ae..bf50227fe401 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.27.0
^ permalink raw reply related [flat|nested] 37+ messages in thread* [PATCH v11 13/16] KVM: x86/vmx: Clear Arch LBREn bit before inject #DB to guest
2022-05-06 3:32 [PATCH v11 00/16] Introduce Architectural LBR for vPMU Yang Weijiang
` (11 preceding siblings ...)
2022-05-06 3:33 ` [PATCH v11 12/16] KVM: nVMX: Add necessary Arch LBR settings for nested VM Yang Weijiang
@ 2022-05-06 3:33 ` Yang Weijiang
2022-05-06 15:08 ` Liang, Kan
2022-05-06 3:33 ` [PATCH v11 14/16] KVM: x86/vmx: Flip Arch LBREn bit on guest state change Yang Weijiang
` (3 subsequent siblings)
16 siblings, 1 reply; 37+ messages in thread
From: Yang Weijiang @ 2022-05-06 3:33 UTC (permalink / raw)
To: pbonzini, jmattson, seanjc, kan.liang, like.xu.linux, vkuznets,
wei.w.wang, kvm, linux-kernel
Cc: Yang Weijiang
On a debug breakpoint event (#DB), IA32_LBR_CTL.LBREn is cleared.
So need to clear the bit manually before inject #DB.
Signed-off-by: Yang Weijiang <weijiang.yang@intel.com>
---
arch/x86/kvm/vmx/vmx.c | 27 +++++++++++++++++++++++++++
1 file changed, 27 insertions(+)
diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c
index e6384ef1d115..6d6ee9cf82f5 100644
--- a/arch/x86/kvm/vmx/vmx.c
+++ b/arch/x86/kvm/vmx/vmx.c
@@ -1605,6 +1605,27 @@ 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)
+{
+ struct lbr_desc *lbr_desc = vcpu_to_lbr_desc(vcpu);
+ struct kvm_pmu *pmu = vcpu_to_pmu(vcpu);
+
+ if (kvm_cpu_cap_has(X86_FEATURE_ARCH_LBR) &&
+ test_bit(INTEL_PMC_IDX_FIXED_VLBR, pmu->pmc_in_use) &&
+ lbr_desc->event) {
+ u64 old = vmcs_read64(GUEST_IA32_LBR_CTL);
+ u64 new;
+
+ if (on)
+ new = old | ARCH_LBR_CTL_LBREN;
+ else
+ new = old & ~ARCH_LBR_CTL_LBREN;
+
+ if (old != new)
+ vmcs_write64(GUEST_IA32_LBR_CTL, new);
+ }
+}
+
static void vmx_queue_exception(struct kvm_vcpu *vcpu)
{
struct vcpu_vmx *vmx = to_vmx(vcpu);
@@ -1640,6 +1661,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,
@@ -4645,6 +4669,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.27.0
^ permalink raw reply related [flat|nested] 37+ messages in thread* Re: [PATCH v11 13/16] KVM: x86/vmx: Clear Arch LBREn bit before inject #DB to guest
2022-05-06 3:33 ` [PATCH v11 13/16] KVM: x86/vmx: Clear Arch LBREn bit before inject #DB to guest Yang Weijiang
@ 2022-05-06 15:08 ` Liang, Kan
0 siblings, 0 replies; 37+ messages in thread
From: Liang, Kan @ 2022-05-06 15:08 UTC (permalink / raw)
To: Yang Weijiang, pbonzini, jmattson, seanjc, like.xu.linux,
vkuznets, wei.w.wang, kvm, linux-kernel
On 5/5/2022 11:33 PM, Yang Weijiang wrote:
> On a debug breakpoint event (#DB), IA32_LBR_CTL.LBREn is cleared.
> So need to clear the bit manually before inject #DB.
>
> Signed-off-by: Yang Weijiang <weijiang.yang@intel.com>
Reviewed-by: Kan Liang <kan.liang@linux.intel.com>
> ---
> arch/x86/kvm/vmx/vmx.c | 27 +++++++++++++++++++++++++++
> 1 file changed, 27 insertions(+)
>
> diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c
> index e6384ef1d115..6d6ee9cf82f5 100644
> --- a/arch/x86/kvm/vmx/vmx.c
> +++ b/arch/x86/kvm/vmx/vmx.c
> @@ -1605,6 +1605,27 @@ 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)
> +{
> + struct lbr_desc *lbr_desc = vcpu_to_lbr_desc(vcpu);
> + struct kvm_pmu *pmu = vcpu_to_pmu(vcpu);
> +
> + if (kvm_cpu_cap_has(X86_FEATURE_ARCH_LBR) &&
> + test_bit(INTEL_PMC_IDX_FIXED_VLBR, pmu->pmc_in_use) &&
> + lbr_desc->event) {
> + u64 old = vmcs_read64(GUEST_IA32_LBR_CTL);
> + u64 new;
> +
> + if (on)
> + new = old | ARCH_LBR_CTL_LBREN;
> + else
> + new = old & ~ARCH_LBR_CTL_LBREN;
> +
> + if (old != new)
> + vmcs_write64(GUEST_IA32_LBR_CTL, new);
> + }
> +}
> +
> static void vmx_queue_exception(struct kvm_vcpu *vcpu)
> {
> struct vcpu_vmx *vmx = to_vmx(vcpu);
> @@ -1640,6 +1661,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,
> @@ -4645,6 +4669,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)
^ permalink raw reply [flat|nested] 37+ messages in thread
* [PATCH v11 14/16] KVM: x86/vmx: Flip Arch LBREn bit on guest state change
2022-05-06 3:32 [PATCH v11 00/16] Introduce Architectural LBR for vPMU Yang Weijiang
` (12 preceding siblings ...)
2022-05-06 3:33 ` [PATCH v11 13/16] KVM: x86/vmx: Clear Arch LBREn bit before inject #DB to guest Yang Weijiang
@ 2022-05-06 3:33 ` Yang Weijiang
2022-05-06 15:08 ` Liang, Kan
2022-05-10 15:51 ` Paolo Bonzini
2022-05-06 3:33 ` [PATCH v11 15/16] KVM: x86: Add Arch LBR data MSR access interface Yang Weijiang
` (2 subsequent siblings)
16 siblings, 2 replies; 37+ messages in thread
From: Yang Weijiang @ 2022-05-06 3:33 UTC (permalink / raw)
To: pbonzini, jmattson, seanjc, kan.liang, like.xu.linux, vkuznets,
wei.w.wang, kvm, linux-kernel
Cc: Yang Weijiang
Per spec:"IA32_LBR_CTL.LBREn is saved and cleared on #SMI, and restored
on RSM. On a warm reset, all LBR MSRs, including IA32_LBR_DEPTH, have their
values preserved. However, IA32_LBR_CTL.LBREn is cleared to 0, disabling
LBRs." So clear Arch LBREn bit on #SMI and restore it on RSM manully, also
clear the bit when guest does warm reset.
Signed-off-by: Yang Weijiang <weijiang.yang@intel.com>
---
arch/x86/kvm/vmx/vmx.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c
index 6d6ee9cf82f5..b38f58868905 100644
--- a/arch/x86/kvm/vmx/vmx.c
+++ b/arch/x86/kvm/vmx/vmx.c
@@ -4593,6 +4593,8 @@ static void vmx_vcpu_reset(struct kvm_vcpu *vcpu, bool init_event)
if (!init_event) {
if (static_cpu_has(X86_FEATURE_ARCH_LBR))
vmcs_write64(GUEST_IA32_LBR_CTL, 0);
+ } else {
+ flip_arch_lbr_ctl(vcpu, false);
}
}
@@ -7704,6 +7706,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;
}
@@ -7725,6 +7728,7 @@ static int vmx_leave_smm(struct kvm_vcpu *vcpu, const char *smstate)
vmx->nested.nested_run_pending = 1;
vmx->nested.smm.guest_mode = false;
}
+ flip_arch_lbr_ctl(vcpu, true);
return 0;
}
--
2.27.0
^ permalink raw reply related [flat|nested] 37+ messages in thread* Re: [PATCH v11 14/16] KVM: x86/vmx: Flip Arch LBREn bit on guest state change
2022-05-06 3:33 ` [PATCH v11 14/16] KVM: x86/vmx: Flip Arch LBREn bit on guest state change Yang Weijiang
@ 2022-05-06 15:08 ` Liang, Kan
2022-05-10 15:51 ` Paolo Bonzini
1 sibling, 0 replies; 37+ messages in thread
From: Liang, Kan @ 2022-05-06 15:08 UTC (permalink / raw)
To: Yang Weijiang, pbonzini, jmattson, seanjc, like.xu.linux,
vkuznets, wei.w.wang, kvm, linux-kernel
On 5/5/2022 11:33 PM, Yang Weijiang wrote:
> Per spec:"IA32_LBR_CTL.LBREn is saved and cleared on #SMI, and restored
> on RSM. On a warm reset, all LBR MSRs, including IA32_LBR_DEPTH, have their
> values preserved. However, IA32_LBR_CTL.LBREn is cleared to 0, disabling
> LBRs." So clear Arch LBREn bit on #SMI and restore it on RSM manully, also
> clear the bit when guest does warm reset.
>
> Signed-off-by: Yang Weijiang <weijiang.yang@intel.com>
Reviewed-by: Kan Liang <kan.liang@linux.intel.com>
> ---
> arch/x86/kvm/vmx/vmx.c | 4 ++++
> 1 file changed, 4 insertions(+)
>
> diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c
> index 6d6ee9cf82f5..b38f58868905 100644
> --- a/arch/x86/kvm/vmx/vmx.c
> +++ b/arch/x86/kvm/vmx/vmx.c
> @@ -4593,6 +4593,8 @@ static void vmx_vcpu_reset(struct kvm_vcpu *vcpu, bool init_event)
> if (!init_event) {
> if (static_cpu_has(X86_FEATURE_ARCH_LBR))
> vmcs_write64(GUEST_IA32_LBR_CTL, 0);
> + } else {
> + flip_arch_lbr_ctl(vcpu, false);
> }
> }
>
> @@ -7704,6 +7706,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;
> }
>
> @@ -7725,6 +7728,7 @@ static int vmx_leave_smm(struct kvm_vcpu *vcpu, const char *smstate)
> vmx->nested.nested_run_pending = 1;
> vmx->nested.smm.guest_mode = false;
> }
> + flip_arch_lbr_ctl(vcpu, true);
> return 0;
> }
>
^ permalink raw reply [flat|nested] 37+ messages in thread* Re: [PATCH v11 14/16] KVM: x86/vmx: Flip Arch LBREn bit on guest state change
2022-05-06 3:33 ` [PATCH v11 14/16] KVM: x86/vmx: Flip Arch LBREn bit on guest state change Yang Weijiang
2022-05-06 15:08 ` Liang, Kan
@ 2022-05-10 15:51 ` Paolo Bonzini
2022-05-11 7:43 ` Yang, Weijiang
2022-05-12 6:44 ` Yang, Weijiang
1 sibling, 2 replies; 37+ messages in thread
From: Paolo Bonzini @ 2022-05-10 15:51 UTC (permalink / raw)
To: Yang Weijiang, jmattson, seanjc, kan.liang, like.xu.linux,
vkuznets, wei.w.wang, kvm, linux-kernel
On 5/6/22 05:33, Yang Weijiang wrote:
> Per spec:"IA32_LBR_CTL.LBREn is saved and cleared on #SMI, and restored
> on RSM. On a warm reset, all LBR MSRs, including IA32_LBR_DEPTH, have their
> values preserved. However, IA32_LBR_CTL.LBREn is cleared to 0, disabling
> LBRs." So clear Arch LBREn bit on #SMI and restore it on RSM manully, also
> clear the bit when guest does warm reset.
>
> Signed-off-by: Yang Weijiang <weijiang.yang@intel.com>
> ---
> arch/x86/kvm/vmx/vmx.c | 4 ++++
> 1 file changed, 4 insertions(+)
>
> diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c
> index 6d6ee9cf82f5..b38f58868905 100644
> --- a/arch/x86/kvm/vmx/vmx.c
> +++ b/arch/x86/kvm/vmx/vmx.c
> @@ -4593,6 +4593,8 @@ static void vmx_vcpu_reset(struct kvm_vcpu *vcpu, bool init_event)
> if (!init_event) {
> if (static_cpu_has(X86_FEATURE_ARCH_LBR))
> vmcs_write64(GUEST_IA32_LBR_CTL, 0);
> + } else {
> + flip_arch_lbr_ctl(vcpu, false);
> }
> }
>
> @@ -7704,6 +7706,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;
> }
>
> @@ -7725,6 +7728,7 @@ static int vmx_leave_smm(struct kvm_vcpu *vcpu, const char *smstate)
> vmx->nested.nested_run_pending = 1;
> vmx->nested.smm.guest_mode = false;
> }
> + flip_arch_lbr_ctl(vcpu, true);
> return 0;
> }
>
This is incorrect, you hare not saving/restoring the actual value of
LBREn (which is "lbr_desc->event != NULL"). Therefore, a migration
while in SMM would lose the value of LBREn = true.
Instead of using flip_arch_lbr_ctl, SMM should save the value of the MSR
in kvm_x86_ops->enter_smm, and restore it in kvm_x86_ops->leave_smm
(feel free to do it only if guest_cpuid_has(vcpu, X86_FEATURE_LM), i.e.
the 32-bit case can be ignored).
Paolo
^ permalink raw reply [flat|nested] 37+ messages in thread* Re: [PATCH v11 14/16] KVM: x86/vmx: Flip Arch LBREn bit on guest state change
2022-05-10 15:51 ` Paolo Bonzini
@ 2022-05-11 7:43 ` Yang, Weijiang
2022-05-12 13:18 ` Paolo Bonzini
2022-05-12 6:44 ` Yang, Weijiang
1 sibling, 1 reply; 37+ messages in thread
From: Yang, Weijiang @ 2022-05-11 7:43 UTC (permalink / raw)
To: Paolo Bonzini, jmattson, seanjc, kan.liang, like.xu.linux,
vkuznets, Wang, Wei W, kvm, linux-kernel
On 5/10/2022 11:51 PM, Paolo Bonzini wrote:
> On 5/6/22 05:33, Yang Weijiang wrote:
>> Per spec:"IA32_LBR_CTL.LBREn is saved and cleared on #SMI, and restored
>> on RSM. On a warm reset, all LBR MSRs, including IA32_LBR_DEPTH, have their
>> values preserved. However, IA32_LBR_CTL.LBREn is cleared to 0, disabling
>> LBRs." So clear Arch LBREn bit on #SMI and restore it on RSM manully, also
>> clear the bit when guest does warm reset.
>>
>> Signed-off-by: Yang Weijiang <weijiang.yang@intel.com>
>> ---
>> arch/x86/kvm/vmx/vmx.c | 4 ++++
>> 1 file changed, 4 insertions(+)
>>
>> diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c
>> index 6d6ee9cf82f5..b38f58868905 100644
>> --- a/arch/x86/kvm/vmx/vmx.c
>> +++ b/arch/x86/kvm/vmx/vmx.c
>> @@ -4593,6 +4593,8 @@ static void vmx_vcpu_reset(struct kvm_vcpu *vcpu, bool init_event)
>> if (!init_event) {
>> if (static_cpu_has(X86_FEATURE_ARCH_LBR))
>> vmcs_write64(GUEST_IA32_LBR_CTL, 0);
>> + } else {
>> + flip_arch_lbr_ctl(vcpu, false);
>> }
>> }
>>
>> @@ -7704,6 +7706,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;
>> }
>>
>> @@ -7725,6 +7728,7 @@ static int vmx_leave_smm(struct kvm_vcpu *vcpu, const char *smstate)
>> vmx->nested.nested_run_pending = 1;
>> vmx->nested.smm.guest_mode = false;
>> }
>> + flip_arch_lbr_ctl(vcpu, true);
>> return 0;
>> }
>>
> This is incorrect, you hare not saving/restoring the actual value of
> LBREn (which is "lbr_desc->event != NULL"). Therefore, a migration
> while in SMM would lose the value of LBREn = true.
>
> Instead of using flip_arch_lbr_ctl, SMM should save the value of the MSR
> in kvm_x86_ops->enter_smm, and restore it in kvm_x86_ops->leave_smm
> (feel free to do it only if guest_cpuid_has(vcpu, X86_FEATURE_LM), i.e.
> the 32-bit case can be ignored).
In the case of migration in SMM, I assume kvm_x86_ops->enter_smm()
called in source side
and kvm_x86_ops->leave_smm() is called at destination, then should the
saved LBREn be transferred
to destination too? The destination can rely on the bit to defer setting
LBREn bit in
VMCS until kvm_x86_ops->leave_smm() is called. is it good? thanks!
>
>
> Paolo
^ permalink raw reply [flat|nested] 37+ messages in thread* Re: [PATCH v11 14/16] KVM: x86/vmx: Flip Arch LBREn bit on guest state change
2022-05-11 7:43 ` Yang, Weijiang
@ 2022-05-12 13:18 ` Paolo Bonzini
2022-05-12 14:38 ` Yang, Weijiang
2022-05-13 4:02 ` Yang, Weijiang
0 siblings, 2 replies; 37+ messages in thread
From: Paolo Bonzini @ 2022-05-12 13:18 UTC (permalink / raw)
To: Yang, Weijiang, jmattson, seanjc, kan.liang, like.xu.linux,
vkuznets, Wang, Wei W, kvm, linux-kernel
On 5/11/22 09:43, Yang, Weijiang wrote:
>>
>> Instead of using flip_arch_lbr_ctl, SMM should save the value of the MSR
>> in kvm_x86_ops->enter_smm, and restore it in kvm_x86_ops->leave_smm
>> (feel free to do it only if guest_cpuid_has(vcpu, X86_FEATURE_LM), i.e.
>> the 32-bit case can be ignored).
>
> In the case of migration in SMM, I assume kvm_x86_ops->enter_smm()
> called in source side
>
> and kvm_x86_ops->leave_smm() is called at destination, then should the
> saved LBREn be transferred
>
> to destination too? The destination can rely on the bit to defer setting
> LBREn bit in
Hi, it's transferred automatically if the MSR is saved in the SMM save
state area. Both enter_smm and leave_smm can access the save state area.
The enter_smm callback is called after saving "normal" state, and it has
to save the state + clear the bit; likewise, the leave_smm callback is
called before saving "normal" state and will restore the old value of
the MSR.
Thanks,
Paolo
> VMCS until kvm_x86_ops->leave_smm() is called. is it good? thanks!
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [PATCH v11 14/16] KVM: x86/vmx: Flip Arch LBREn bit on guest state change
2022-05-12 13:18 ` Paolo Bonzini
@ 2022-05-12 14:38 ` Yang, Weijiang
2022-05-13 4:02 ` Yang, Weijiang
1 sibling, 0 replies; 37+ messages in thread
From: Yang, Weijiang @ 2022-05-12 14:38 UTC (permalink / raw)
To: Paolo Bonzini, jmattson, seanjc, kan.liang, like.xu.linux,
vkuznets, Wang, Wei W, kvm, linux-kernel
On 5/12/2022 9:18 PM, Paolo Bonzini wrote:
> On 5/11/22 09:43, Yang, Weijiang wrote:
>>> Instead of using flip_arch_lbr_ctl, SMM should save the value of the MSR
>>> in kvm_x86_ops->enter_smm, and restore it in kvm_x86_ops->leave_smm
>>> (feel free to do it only if guest_cpuid_has(vcpu, X86_FEATURE_LM), i.e.
>>> the 32-bit case can be ignored).
>> In the case of migration in SMM, I assume kvm_x86_ops->enter_smm()
>> called in source side
>>
>> and kvm_x86_ops->leave_smm() is called at destination, then should the
>> saved LBREn be transferred
>>
>> to destination too? The destination can rely on the bit to defer setting
>> LBREn bit in
> Hi, it's transferred automatically if the MSR is saved in the SMM save
> state area. Both enter_smm and leave_smm can access the save state area.
>
> The enter_smm callback is called after saving "normal" state, and it has
> to save the state + clear the bit; likewise, the leave_smm callback is
> called before saving "normal" state and will restore the old value of
> the MSR.
Got it thanks!
But there's no such slot for MSR_ARCH_LBR_CTL in SMRAM, do you still suggest
using this mechanism to implement the LBREn clear/restore logic?
> Thanks,
>
> Paolo
>
>> VMCS until kvm_x86_ops->leave_smm() is called. is it good? thanks!
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [PATCH v11 14/16] KVM: x86/vmx: Flip Arch LBREn bit on guest state change
2022-05-12 13:18 ` Paolo Bonzini
2022-05-12 14:38 ` Yang, Weijiang
@ 2022-05-13 4:02 ` Yang, Weijiang
2022-05-17 8:56 ` Yang, Weijiang
1 sibling, 1 reply; 37+ messages in thread
From: Yang, Weijiang @ 2022-05-13 4:02 UTC (permalink / raw)
To: Paolo Bonzini, jmattson, seanjc, kan.liang, like.xu.linux,
vkuznets, Wang, Wei W, kvm, linux-kernel
On 5/12/2022 9:18 PM, Paolo Bonzini wrote:
> On 5/11/22 09:43, Yang, Weijiang wrote:
>>> Instead of using flip_arch_lbr_ctl, SMM should save the value of the MSR
>>> in kvm_x86_ops->enter_smm, and restore it in kvm_x86_ops->leave_smm
>>> (feel free to do it only if guest_cpuid_has(vcpu, X86_FEATURE_LM), i.e.
>>> the 32-bit case can be ignored).
>> In the case of migration in SMM, I assume kvm_x86_ops->enter_smm()
>> called in source side
>>
>> and kvm_x86_ops->leave_smm() is called at destination, then should the
>> saved LBREn be transferred
>>
>> to destination too? The destination can rely on the bit to defer setting
>> LBREn bit in
> Hi, it's transferred automatically if the MSR is saved in the SMM save
> state area. Both enter_smm and leave_smm can access the save state area.
>
> The enter_smm callback is called after saving "normal" state, and it has
> to save the state + clear the bit; likewise, the leave_smm callback is
> called before saving "normal" state and will restore the old value of
> the MSR.
Hi, I modified this patch to consolidate your suggestion above, see
below patch.
I added more things to ease migration handling in SMM because: 1) qemu
checks
LBREn before transfer Arch LBR MSRs. 2)Perf event is created when LBREn
is being
set. Two things are not certain: 1) IA32_LBR_CTL doesn't have
corresponding slot in
SMRAM,not sure if we need to rely on it to transfer the MSR. 2) I chose
0x7f10 as
the offset(CET takes 0x7f08) for storage, need you double check if it's
free or used.
Thanks a lot!
====================================================================
From ceba1527fd87cdc789b38fce454058fca6582b0a Mon Sep 17 00:00:00 2001
From: Yang Weijiang <weijiang.yang@intel.com>
Date: Thu, 5 Aug 2021 20:48:39 +0800
Subject: [PATCH] KVM: x86/vmx: Flip Arch LBREn bit on guest state change
Per spec:"IA32_LBR_CTL.LBREn is saved and cleared on #SMI, and restored
on RSM. On a warm reset, all LBR MSRs, including IA32_LBR_DEPTH, have their
values preserved. However, IA32_LBR_CTL.LBREn is cleared to 0, disabling
LBRs." Given migration in SMM, use a reserved bit(63) of the MSR to mirror
LBREn bit, it facilitates Arch LBR specific handling during live migration
because user space will check LBREn to decide whether it's necessary to
migrate the Arch LBR related MSRs. When the mirrored bit and LBREn bit are
both set, it means Arch LBR was active in SMM, so only create perf event
and defer the LBREn bit restoring to leave_smm callback.
Also clear LBREn at warm reset.
Signed-off-by: Yang Weijiang <weijiang.yang@intel.com>
---
arch/x86/kvm/vmx/pmu_intel.c | 16 +++++++++++++---
arch/x86/kvm/vmx/vmx.c | 29 +++++++++++++++++++++++++++++
arch/x86/kvm/vmx/vmx.h | 1 +
3 files changed, 43 insertions(+), 3 deletions(-)
diff --git a/arch/x86/kvm/vmx/pmu_intel.c b/arch/x86/kvm/vmx/pmu_intel.c
index 038fdb788ccd..652601ad99ea 100644
--- a/arch/x86/kvm/vmx/pmu_intel.c
+++ b/arch/x86/kvm/vmx/pmu_intel.c
@@ -373,6 +373,8 @@ static bool arch_lbr_depth_is_valid(struct kvm_vcpu
*vcpu, u64 depth)
return (depth == pmu->kvm_arch_lbr_depth);
}
+#define ARCH_LBR_IN_SMM BIT(63)
+
static bool arch_lbr_ctl_is_valid(struct kvm_vcpu *vcpu, u64 ctl)
{
struct kvm_cpuid_entry2 *entry;
@@ -380,7 +382,7 @@ static bool arch_lbr_ctl_is_valid(struct kvm_vcpu
*vcpu, u64 ctl)
if (!kvm_cpu_cap_has(X86_FEATURE_ARCH_LBR))
return false;
- if (ctl & ~KVM_ARCH_LBR_CTL_MASK)
+ if (ctl & ~(KVM_ARCH_LBR_CTL_MASK | ARCH_LBR_IN_SMM))
goto warn;
entry = kvm_find_cpuid_entry(vcpu, 0x1c, 0);
@@ -425,6 +427,10 @@ static int intel_pmu_get_msr(struct kvm_vcpu *vcpu,
struct msr_data *msr_info)
return 0;
case MSR_ARCH_LBR_CTL:
msr_info->data = vmcs_read64(GUEST_IA32_LBR_CTL);
+ if (to_vmx(vcpu)->lbr_in_smm) {
+ msr_info->data |= ARCH_LBR_CTL_LBREN;
+ msr_info->data |= ARCH_LBR_IN_SMM;
+ }
return 0;
default:
if ((pmc = get_gp_pmc(pmu, msr, MSR_IA32_PERFCTR0)) ||
@@ -501,11 +507,15 @@ static int intel_pmu_set_msr(struct kvm_vcpu
*vcpu, struct msr_data *msr_info)
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);
+
+ if (data & ARCH_LBR_IN_SMM) {
+ data &= ~ARCH_LBR_CTL_LBREN;
+ data &= ~ARCH_LBR_IN_SMM;
+ }
+ vmcs_write64(GUEST_IA32_LBR_CTL, data);
return 0;
default:
if ((pmc = get_gp_pmc(pmu, msr, MSR_IA32_PERFCTR0)) ||
diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c
index 6d6ee9cf82f5..f754b9400151 100644
--- a/arch/x86/kvm/vmx/vmx.c
+++ b/arch/x86/kvm/vmx/vmx.c
@@ -4543,6 +4543,7 @@ static void vmx_vcpu_reset(struct kvm_vcpu *vcpu,
bool init_event)
vmx->rmode.vm86_active = 0;
vmx->spec_ctrl = 0;
+ vmx->lbr_in_smm = false;
vmx->msr_ia32_umwait_control = 0;
@@ -4593,6 +4594,8 @@ static void vmx_vcpu_reset(struct kvm_vcpu *vcpu,
bool init_event)
if (!init_event) {
if (static_cpu_has(X86_FEATURE_ARCH_LBR))
vmcs_write64(GUEST_IA32_LBR_CTL, 0);
+ } else {
+ flip_arch_lbr_ctl(vcpu, false);
}
}
@@ -7695,6 +7698,8 @@ static int vmx_smi_allowed(struct kvm_vcpu *vcpu,
bool for_injection)
static int vmx_enter_smm(struct kvm_vcpu *vcpu, char *smstate)
{
+ struct lbr_desc *lbr_desc = vcpu_to_lbr_desc(vcpu);
+ struct kvm_pmu *pmu = vcpu_to_pmu(vcpu);
struct vcpu_vmx *vmx = to_vmx(vcpu);
vmx->nested.smm.guest_mode = is_guest_mode(vcpu);
@@ -7704,12 +7709,26 @@ 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);
+
+ if (kvm_cpu_cap_has(X86_FEATURE_ARCH_LBR) &&
+ test_bit(INTEL_PMC_IDX_FIXED_VLBR, pmu->pmc_in_use) &&
+ lbr_desc->event && guest_cpuid_has(vcpu, X86_FEATURE_LM)) {
+ u64 ctl = vmcs_read64(GUEST_IA32_LBR_CTL);
+
+ put_smstate(u64, smstate, 0x7f10, ctl);
+ vmcs_write64(GUEST_IA32_LBR_CTL, ctl & ~ARCH_LBR_CTL_LBREN);
+ vmx->lbr_in_smm = true;
+ }
+
return 0;
}
static int vmx_leave_smm(struct kvm_vcpu *vcpu, const char *smstate)
{
+ struct lbr_desc *lbr_desc = vcpu_to_lbr_desc(vcpu);
+ struct kvm_pmu *pmu = vcpu_to_pmu(vcpu);
struct vcpu_vmx *vmx = to_vmx(vcpu);
+
int ret;
if (vmx->nested.smm.vmxon) {
@@ -7725,6 +7744,16 @@ static int vmx_leave_smm(struct kvm_vcpu *vcpu,
const char *smstate)
vmx->nested.nested_run_pending = 1;
vmx->nested.smm.guest_mode = false;
}
+
+ if (kvm_cpu_cap_has(X86_FEATURE_ARCH_LBR) &&
+ test_bit(INTEL_PMC_IDX_FIXED_VLBR, pmu->pmc_in_use) &&
+ lbr_desc->event && guest_cpuid_has(vcpu, X86_FEATURE_LM)) {
+ u64 ctl = GET_SMSTATE(u64, smstate, 0x7f10);
+
+ vmcs_write64(GUEST_IA32_LBR_CTL, ctl | ARCH_LBR_CTL_LBREN);
+ vmx->lbr_in_smm = false;
+ }
+
return 0;
}
diff --git a/arch/x86/kvm/vmx/vmx.h b/arch/x86/kvm/vmx/vmx.h
index b98c7e96697a..a227fe8bf288 100644
--- a/arch/x86/kvm/vmx/vmx.h
+++ b/arch/x86/kvm/vmx/vmx.h
@@ -351,6 +351,7 @@ struct vcpu_vmx {
struct pt_desc pt_desc;
struct lbr_desc lbr_desc;
+ bool lbr_in_smm;
/* Save desired MSR intercept (read: pass-through) state */
#define MAX_POSSIBLE_PASSTHROUGH_MSRS 15
--
2.27.0
>
> Thanks,
>
> Paolo
>
>> VMCS until kvm_x86_ops->leave_smm() is called. is it good? thanks!
^ permalink raw reply related [flat|nested] 37+ messages in thread* Re: [PATCH v11 14/16] KVM: x86/vmx: Flip Arch LBREn bit on guest state change
2022-05-13 4:02 ` Yang, Weijiang
@ 2022-05-17 8:56 ` Yang, Weijiang
2022-05-17 9:01 ` Paolo Bonzini
0 siblings, 1 reply; 37+ messages in thread
From: Yang, Weijiang @ 2022-05-17 8:56 UTC (permalink / raw)
To: Paolo Bonzini
Cc: jmattson, seanjc, kan.liang, like.xu.linux, vkuznets, Wang,
Wei W, kvm, linux-kernel
On 5/13/2022 12:02 PM, Yang, Weijiang wrote:
>
> On 5/12/2022 9:18 PM, Paolo Bonzini wrote:
>> On 5/11/22 09:43, Yang, Weijiang wrote:
>>>> Instead of using flip_arch_lbr_ctl, SMM should save the value of
>>>> the MSR
>>>> in kvm_x86_ops->enter_smm, and restore it in kvm_x86_ops->leave_smm
>>>> (feel free to do it only if guest_cpuid_has(vcpu, X86_FEATURE_LM),
>>>> i.e.
>>>> the 32-bit case can be ignored).
>>> In the case of migration in SMM, I assume kvm_x86_ops->enter_smm()
>>> called in source side
>>>
>>> and kvm_x86_ops->leave_smm() is called at destination, then should the
>>> saved LBREn be transferred
>>>
>>> to destination too? The destination can rely on the bit to defer
>>> setting
>>> LBREn bit in
>> Hi, it's transferred automatically if the MSR is saved in the SMM save
>> state area. Both enter_smm and leave_smm can access the save state
>> area.
>>
>> The enter_smm callback is called after saving "normal" state, and it has
>> to save the state + clear the bit; likewise, the leave_smm callback is
>> called before saving "normal" state and will restore the old value of
>> the MSR.
>
> Hi, I modified this patch to consolidate your suggestion above, see
> below patch.
>
> I added more things to ease migration handling in SMM because: 1) qemu
> checks
>
> LBREn before transfer Arch LBR MSRs. 2)Perf event is created when
> LBREn is being
>
> set. Two things are not certain: 1) IA32_LBR_CTL doesn't have
> corresponding slot in
>
> SMRAM,not sure if we need to rely on it to transfer the MSR. 2) I
> chose 0x7f10 as
>
> the offset(CET takes 0x7f08) for storage, need you double check if
> it's free or used.
>
> Thanks a lot!
Hi, Paolo,
I found there're some rebase conflicts between this series and your kvm
queue branch
due to PEBS patches, I can re-post a new version based on your queue
branch if necessary.
Waiting for your comments on this patch...
>
> ====================================================================
>
> From ceba1527fd87cdc789b38fce454058fca6582b0a Mon Sep 17 00:00:00 2001
> From: Yang Weijiang <weijiang.yang@intel.com>
> Date: Thu, 5 Aug 2021 20:48:39 +0800
> Subject: [PATCH] KVM: x86/vmx: Flip Arch LBREn bit on guest state change
>
> Per spec:"IA32_LBR_CTL.LBREn is saved and cleared on #SMI, and restored
> on RSM. On a warm reset, all LBR MSRs, including IA32_LBR_DEPTH, have
> their
> values preserved. However, IA32_LBR_CTL.LBREn is cleared to 0, disabling
> LBRs." Given migration in SMM, use a reserved bit(63) of the MSR to
> mirror
> LBREn bit, it facilitates Arch LBR specific handling during live
> migration
> because user space will check LBREn to decide whether it's necessary to
> migrate the Arch LBR related MSRs. When the mirrored bit and LBREn bit
> are
> both set, it means Arch LBR was active in SMM, so only create perf event
> and defer the LBREn bit restoring to leave_smm callback.
> Also clear LBREn at warm reset.
>
> Signed-off-by: Yang Weijiang <weijiang.yang@intel.com>
> ---
> arch/x86/kvm/vmx/pmu_intel.c | 16 +++++++++++++---
> arch/x86/kvm/vmx/vmx.c | 29 +++++++++++++++++++++++++++++
> arch/x86/kvm/vmx/vmx.h | 1 +
> 3 files changed, 43 insertions(+), 3 deletions(-)
>
> diff --git a/arch/x86/kvm/vmx/pmu_intel.c b/arch/x86/kvm/vmx/pmu_intel.c
> index 038fdb788ccd..652601ad99ea 100644
> --- a/arch/x86/kvm/vmx/pmu_intel.c
> +++ b/arch/x86/kvm/vmx/pmu_intel.c
> @@ -373,6 +373,8 @@ static bool arch_lbr_depth_is_valid(struct
> kvm_vcpu *vcpu, u64 depth)
> return (depth == pmu->kvm_arch_lbr_depth);
> }
>
> +#define ARCH_LBR_IN_SMM BIT(63)
> +
> static bool arch_lbr_ctl_is_valid(struct kvm_vcpu *vcpu, u64 ctl)
> {
> struct kvm_cpuid_entry2 *entry;
> @@ -380,7 +382,7 @@ static bool arch_lbr_ctl_is_valid(struct kvm_vcpu
> *vcpu, u64 ctl)
> if (!kvm_cpu_cap_has(X86_FEATURE_ARCH_LBR))
> return false;
>
> - if (ctl & ~KVM_ARCH_LBR_CTL_MASK)
> + if (ctl & ~(KVM_ARCH_LBR_CTL_MASK | ARCH_LBR_IN_SMM))
> goto warn;
>
> entry = kvm_find_cpuid_entry(vcpu, 0x1c, 0);
> @@ -425,6 +427,10 @@ static int intel_pmu_get_msr(struct kvm_vcpu
> *vcpu, struct msr_data *msr_info)
> return 0;
> case MSR_ARCH_LBR_CTL:
> msr_info->data = vmcs_read64(GUEST_IA32_LBR_CTL);
> + if (to_vmx(vcpu)->lbr_in_smm) {
> + msr_info->data |= ARCH_LBR_CTL_LBREN;
> + msr_info->data |= ARCH_LBR_IN_SMM;
> + }
> return 0;
> default:
> if ((pmc = get_gp_pmc(pmu, msr, MSR_IA32_PERFCTR0)) ||
> @@ -501,11 +507,15 @@ static int intel_pmu_set_msr(struct kvm_vcpu
> *vcpu, struct msr_data *msr_info)
> 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);
> +
> + if (data & ARCH_LBR_IN_SMM) {
> + data &= ~ARCH_LBR_CTL_LBREN;
> + data &= ~ARCH_LBR_IN_SMM;
> + }
> + vmcs_write64(GUEST_IA32_LBR_CTL, data);
> return 0;
> default:
> if ((pmc = get_gp_pmc(pmu, msr, MSR_IA32_PERFCTR0)) ||
> diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c
> index 6d6ee9cf82f5..f754b9400151 100644
> --- a/arch/x86/kvm/vmx/vmx.c
> +++ b/arch/x86/kvm/vmx/vmx.c
> @@ -4543,6 +4543,7 @@ static void vmx_vcpu_reset(struct kvm_vcpu
> *vcpu, bool init_event)
>
> vmx->rmode.vm86_active = 0;
> vmx->spec_ctrl = 0;
> + vmx->lbr_in_smm = false;
>
> vmx->msr_ia32_umwait_control = 0;
>
> @@ -4593,6 +4594,8 @@ static void vmx_vcpu_reset(struct kvm_vcpu
> *vcpu, bool init_event)
> if (!init_event) {
> if (static_cpu_has(X86_FEATURE_ARCH_LBR))
> vmcs_write64(GUEST_IA32_LBR_CTL, 0);
> + } else {
> + flip_arch_lbr_ctl(vcpu, false);
> }
> }
>
> @@ -7695,6 +7698,8 @@ static int vmx_smi_allowed(struct kvm_vcpu
> *vcpu, bool for_injection)
>
> static int vmx_enter_smm(struct kvm_vcpu *vcpu, char *smstate)
> {
> + struct lbr_desc *lbr_desc = vcpu_to_lbr_desc(vcpu);
> + struct kvm_pmu *pmu = vcpu_to_pmu(vcpu);
> struct vcpu_vmx *vmx = to_vmx(vcpu);
>
> vmx->nested.smm.guest_mode = is_guest_mode(vcpu);
> @@ -7704,12 +7709,26 @@ 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);
> +
> + if (kvm_cpu_cap_has(X86_FEATURE_ARCH_LBR) &&
> + test_bit(INTEL_PMC_IDX_FIXED_VLBR, pmu->pmc_in_use) &&
> + lbr_desc->event && guest_cpuid_has(vcpu, X86_FEATURE_LM)) {
> + u64 ctl = vmcs_read64(GUEST_IA32_LBR_CTL);
> +
> + put_smstate(u64, smstate, 0x7f10, ctl);
> + vmcs_write64(GUEST_IA32_LBR_CTL, ctl & ~ARCH_LBR_CTL_LBREN);
> + vmx->lbr_in_smm = true;
> + }
> +
> return 0;
> }
>
> static int vmx_leave_smm(struct kvm_vcpu *vcpu, const char *smstate)
> {
> + struct lbr_desc *lbr_desc = vcpu_to_lbr_desc(vcpu);
> + struct kvm_pmu *pmu = vcpu_to_pmu(vcpu);
> struct vcpu_vmx *vmx = to_vmx(vcpu);
> +
> int ret;
>
> if (vmx->nested.smm.vmxon) {
> @@ -7725,6 +7744,16 @@ static int vmx_leave_smm(struct kvm_vcpu *vcpu,
> const char *smstate)
> vmx->nested.nested_run_pending = 1;
> vmx->nested.smm.guest_mode = false;
> }
> +
> + if (kvm_cpu_cap_has(X86_FEATURE_ARCH_LBR) &&
> + test_bit(INTEL_PMC_IDX_FIXED_VLBR, pmu->pmc_in_use) &&
> + lbr_desc->event && guest_cpuid_has(vcpu, X86_FEATURE_LM)) {
> + u64 ctl = GET_SMSTATE(u64, smstate, 0x7f10);
> +
> + vmcs_write64(GUEST_IA32_LBR_CTL, ctl | ARCH_LBR_CTL_LBREN);
> + vmx->lbr_in_smm = false;
> + }
> +
> return 0;
> }
>
> diff --git a/arch/x86/kvm/vmx/vmx.h b/arch/x86/kvm/vmx/vmx.h
> index b98c7e96697a..a227fe8bf288 100644
> --- a/arch/x86/kvm/vmx/vmx.h
> +++ b/arch/x86/kvm/vmx/vmx.h
> @@ -351,6 +351,7 @@ struct vcpu_vmx {
>
> struct pt_desc pt_desc;
> struct lbr_desc lbr_desc;
> + bool lbr_in_smm;
>
> /* Save desired MSR intercept (read: pass-through) state */
> #define MAX_POSSIBLE_PASSTHROUGH_MSRS 15
^ permalink raw reply [flat|nested] 37+ messages in thread* Re: [PATCH v11 14/16] KVM: x86/vmx: Flip Arch LBREn bit on guest state change
2022-05-17 8:56 ` Yang, Weijiang
@ 2022-05-17 9:01 ` Paolo Bonzini
2022-05-17 11:31 ` Yang, Weijiang
0 siblings, 1 reply; 37+ messages in thread
From: Paolo Bonzini @ 2022-05-17 9:01 UTC (permalink / raw)
To: Yang, Weijiang
Cc: jmattson, seanjc, kan.liang, like.xu.linux, vkuznets, Wang,
Wei W, kvm, linux-kernel
On 5/17/22 10:56, Yang, Weijiang wrote:
>> I added more things to ease migration handling in SMM because: 1) qemu
>> checks LBREn before transfer Arch LBR MSRs.
I think it should always transfer them instead? There's time to post a
fixup patch.
>> 2) Perf event is created when
>> LBREn is being set. Two things are not certain: 1) IA32_LBR_CTL doesn't have
>> corresponding slot in SMRAM,not sure if we need to rely on it to transfer the MSR.
>> I chose 0x7f10 as the offset(CET takes 0x7f08) for storage, need you double check if
>> it's free or used.
0x7f10 sounds good.
> Hi, Paolo,
>
> I found there're some rebase conflicts between this series and your kvm
> queue branch due to PEBS patches, I can re-post a new version based on
> your queue branch if necessary.
Yes, please.
> Waiting for your comments on this patch...
I already commented that using bit 63 is not good, didn't I?
Paolo
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [PATCH v11 14/16] KVM: x86/vmx: Flip Arch LBREn bit on guest state change
2022-05-17 9:01 ` Paolo Bonzini
@ 2022-05-17 11:31 ` Yang, Weijiang
0 siblings, 0 replies; 37+ messages in thread
From: Yang, Weijiang @ 2022-05-17 11:31 UTC (permalink / raw)
To: Paolo Bonzini
Cc: jmattson, seanjc, kan.liang, like.xu.linux, vkuznets, Wang,
Wei W, kvm, linux-kernel
On 5/17/2022 5:01 PM, Paolo Bonzini wrote:
> On 5/17/22 10:56, Yang, Weijiang wrote:
>>> I added more things to ease migration handling in SMM because: 1) qemu
>>> checks LBREn before transfer Arch LBR MSRs.
> I think it should always transfer them instead? There's time to post a
> fixup patch.
OK, I'll send a fix patch.
>
>>> 2) Perf event is created when
>>> LBREn is being set. Two things are not certain: 1) IA32_LBR_CTL doesn't have
>>> corresponding slot in SMRAM,not sure if we need to rely on it to transfer the MSR.
>>> I chose 0x7f10 as the offset(CET takes 0x7f08) for storage, need you double check if
>>> it's free or used.
> 0x7f10 sounds good.
>
>> Hi, Paolo,
>>
>> I found there're some rebase conflicts between this series and your kvm
>> queue branch due to PEBS patches, I can re-post a new version based on
>> your queue branch if necessary.
> Yes, please.
Sure, I'll post v12 soon.
>
>> Waiting for your comments on this patch...
> I already commented that using bit 63 is not good, didn't I?
Clear :-D, thanks!
>
> Paolo
>
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [PATCH v11 14/16] KVM: x86/vmx: Flip Arch LBREn bit on guest state change
2022-05-10 15:51 ` Paolo Bonzini
2022-05-11 7:43 ` Yang, Weijiang
@ 2022-05-12 6:44 ` Yang, Weijiang
1 sibling, 0 replies; 37+ messages in thread
From: Yang, Weijiang @ 2022-05-12 6:44 UTC (permalink / raw)
To: Paolo Bonzini, jmattson, seanjc, kan.liang, like.xu.linux,
vkuznets, Wang, Wei W, kvm, linux-kernel
On 5/10/2022 11:51 PM, Paolo Bonzini wrote:
> On 5/6/22 05:33, Yang Weijiang wrote:
>> Per spec:"IA32_LBR_CTL.LBREn is saved and cleared on #SMI, and restored
>> on RSM. On a warm reset, all LBR MSRs, including IA32_LBR_DEPTH, have their
>> values preserved. However, IA32_LBR_CTL.LBREn is cleared to 0, disabling
>> LBRs." So clear Arch LBREn bit on #SMI and restore it on RSM manully, also
>> clear the bit when guest does warm reset.
>>
>> Signed-off-by: Yang Weijiang <weijiang.yang@intel.com>
>> ---
>> arch/x86/kvm/vmx/vmx.c | 4 ++++
>> 1 file changed, 4 insertions(+)
>>
>> diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c
>> index 6d6ee9cf82f5..b38f58868905 100644
>> --- a/arch/x86/kvm/vmx/vmx.c
>> +++ b/arch/x86/kvm/vmx/vmx.c
>> @@ -4593,6 +4593,8 @@ static void vmx_vcpu_reset(struct kvm_vcpu *vcpu, bool init_event)
>> if (!init_event) {
>> if (static_cpu_has(X86_FEATURE_ARCH_LBR))
>> vmcs_write64(GUEST_IA32_LBR_CTL, 0);
>> + } else {
>> + flip_arch_lbr_ctl(vcpu, false);
>> }
>> }
>>
>> @@ -7704,6 +7706,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;
>> }
>>
>> @@ -7725,6 +7728,7 @@ static int vmx_leave_smm(struct kvm_vcpu *vcpu, const char *smstate)
>> vmx->nested.nested_run_pending = 1;
>> vmx->nested.smm.guest_mode = false;
>> }
>> + flip_arch_lbr_ctl(vcpu, true);
>> return 0;
>> }
>>
> This is incorrect, you hare not saving/restoring the actual value of
> LBREn (which is "lbr_desc->event != NULL"). Therefore, a migration
> while in SMM would lose the value of LBREn = true.
>
> Instead of using flip_arch_lbr_ctl, SMM should save the value of the MSR
> in kvm_x86_ops->enter_smm, and restore it in kvm_x86_ops->leave_smm
> (feel free to do it only if guest_cpuid_has(vcpu, X86_FEATURE_LM), i.e.
> the 32-bit case can be ignored).
Hi, Paolo,
I re-factored this patch as below to enclose your above suggestion,
could you
kindly check? If it's OK then I'll refresh this series with v12, thanks!
======================================================================
From dad3abc7fe96022dd3dcee8f958960bbd4f68b95 Mon Sep 17 00:00:00 2001
From: Yang Weijiang <weijiang.yang@intel.com>
Date: Thu, 5 Aug 2021 20:48:39 +0800
Subject: [PATCH] KVM: x86/vmx: Flip Arch LBREn bit on guest state change
Per spec:"IA32_LBR_CTL.LBREn is saved and cleared on #SMI, and restored
on RSM. On a warm reset, all LBR MSRs, including IA32_LBR_DEPTH, have their
values preserved. However, IA32_LBR_CTL.LBREn is cleared to 0, disabling
LBRs." Use a reserved bit(63) of the MSR to hide LBREn bit on #SMI and
restore it to LBREn on RSM manully, also clear the bit when guest does
warm reset.
Signed-off-by: Yang Weijiang <weijiang.yang@intel.com>
---
arch/x86/kvm/vmx/pmu_intel.c | 16 +++++++++++++---
arch/x86/kvm/vmx/vmx.c | 24 ++++++++++++++++++++++++
arch/x86/kvm/vmx/vmx.h | 1 +
3 files changed, 38 insertions(+), 3 deletions(-)
diff --git a/arch/x86/kvm/vmx/pmu_intel.c b/arch/x86/kvm/vmx/pmu_intel.c
index 038fdb788ccd..652601ad99ea 100644
--- a/arch/x86/kvm/vmx/pmu_intel.c
+++ b/arch/x86/kvm/vmx/pmu_intel.c
@@ -373,6 +373,8 @@ static bool arch_lbr_depth_is_valid(struct kvm_vcpu
*vcpu, u64 depth)
return (depth == pmu->kvm_arch_lbr_depth);
}
+#define ARCH_LBR_IN_SMM BIT(63)
+
static bool arch_lbr_ctl_is_valid(struct kvm_vcpu *vcpu, u64 ctl)
{
struct kvm_cpuid_entry2 *entry;
@@ -380,7 +382,7 @@ static bool arch_lbr_ctl_is_valid(struct kvm_vcpu
*vcpu, u64 ctl)
if (!kvm_cpu_cap_has(X86_FEATURE_ARCH_LBR))
return false;
- if (ctl & ~KVM_ARCH_LBR_CTL_MASK)
+ if (ctl & ~(KVM_ARCH_LBR_CTL_MASK | ARCH_LBR_IN_SMM))
goto warn;
entry = kvm_find_cpuid_entry(vcpu, 0x1c, 0);
@@ -425,6 +427,10 @@ static int intel_pmu_get_msr(struct kvm_vcpu *vcpu,
struct msr_data *msr_info)
return 0;
case MSR_ARCH_LBR_CTL:
msr_info->data = vmcs_read64(GUEST_IA32_LBR_CTL);
+ if (to_vmx(vcpu)->lbr_in_smm) {
+ msr_info->data |= ARCH_LBR_CTL_LBREN;
+ msr_info->data |= ARCH_LBR_IN_SMM;
+ }
return 0;
default:
if ((pmc = get_gp_pmc(pmu, msr, MSR_IA32_PERFCTR0)) ||
@@ -501,11 +507,15 @@ static int intel_pmu_set_msr(struct kvm_vcpu
*vcpu, struct msr_data *msr_info)
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);
+
+ if (data & ARCH_LBR_IN_SMM) {
+ data &= ~ARCH_LBR_CTL_LBREN;
+ data &= ~ARCH_LBR_IN_SMM;
+ }
+ vmcs_write64(GUEST_IA32_LBR_CTL, data);
return 0;
default:
if ((pmc = get_gp_pmc(pmu, msr, MSR_IA32_PERFCTR0)) ||
diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c
index 6d6ee9cf82f5..eadad24a68e6 100644
--- a/arch/x86/kvm/vmx/vmx.c
+++ b/arch/x86/kvm/vmx/vmx.c
@@ -4543,6 +4543,7 @@ static void vmx_vcpu_reset(struct kvm_vcpu *vcpu,
bool init_event)
vmx->rmode.vm86_active = 0;
vmx->spec_ctrl = 0;
+ vmx->lbr_in_smm = false;
vmx->msr_ia32_umwait_control = 0;
@@ -4593,6 +4594,8 @@ static void vmx_vcpu_reset(struct kvm_vcpu *vcpu,
bool init_event)
if (!init_event) {
if (static_cpu_has(X86_FEATURE_ARCH_LBR))
vmcs_write64(GUEST_IA32_LBR_CTL, 0);
+ } else {
+ flip_arch_lbr_ctl(vcpu, false);
}
}
@@ -7695,6 +7698,8 @@ static int vmx_smi_allowed(struct kvm_vcpu *vcpu,
bool for_injection)
static int vmx_enter_smm(struct kvm_vcpu *vcpu, char *smstate)
{
+ struct lbr_desc *lbr_desc = vcpu_to_lbr_desc(vcpu);
+ struct kvm_pmu *pmu = vcpu_to_pmu(vcpu);
struct vcpu_vmx *vmx = to_vmx(vcpu);
vmx->nested.smm.guest_mode = is_guest_mode(vcpu);
@@ -7704,12 +7709,21 @@ 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);
+
+ if (kvm_cpu_cap_has(X86_FEATURE_ARCH_LBR) &&
+ test_bit(INTEL_PMC_IDX_FIXED_VLBR, pmu->pmc_in_use) &&
+ lbr_desc->event)
+ vmx->lbr_in_smm = true;
+
return 0;
}
static int vmx_leave_smm(struct kvm_vcpu *vcpu, const char *smstate)
{
+ struct lbr_desc *lbr_desc = vcpu_to_lbr_desc(vcpu);
+ struct kvm_pmu *pmu = vcpu_to_pmu(vcpu);
struct vcpu_vmx *vmx = to_vmx(vcpu);
+
int ret;
if (vmx->nested.smm.vmxon) {
@@ -7725,6 +7739,16 @@ static int vmx_leave_smm(struct kvm_vcpu *vcpu,
const char *smstate)
vmx->nested.nested_run_pending = 1;
vmx->nested.smm.guest_mode = false;
}
+
+ if (kvm_cpu_cap_has(X86_FEATURE_ARCH_LBR) &&
+ test_bit(INTEL_PMC_IDX_FIXED_VLBR, pmu->pmc_in_use) &&
+ lbr_desc->event && vmx->lbr_in_smm) {
+ u64 ctl = vmcs_read64(GUEST_IA32_LBR_CTL);
+
+ vmcs_write64(GUEST_IA32_LBR_CTL, ctl | ARCH_LBR_CTL_LBREN);
+ vmx->lbr_in_smm = false;
+ }
+
return 0;
}
diff --git a/arch/x86/kvm/vmx/vmx.h b/arch/x86/kvm/vmx/vmx.h
index b98c7e96697a..a227fe8bf288 100644
--- a/arch/x86/kvm/vmx/vmx.h
+++ b/arch/x86/kvm/vmx/vmx.h
@@ -351,6 +351,7 @@ struct vcpu_vmx {
struct pt_desc pt_desc;
struct lbr_desc lbr_desc;
+ bool lbr_in_smm;
/* Save desired MSR intercept (read: pass-through) state */
#define MAX_POSSIBLE_PASSTHROUGH_MSRS 15
--
2.27.0
>
> Paolo
^ permalink raw reply related [flat|nested] 37+ messages in thread
* [PATCH v11 15/16] KVM: x86: Add Arch LBR data MSR access interface
2022-05-06 3:32 [PATCH v11 00/16] Introduce Architectural LBR for vPMU Yang Weijiang
` (13 preceding siblings ...)
2022-05-06 3:33 ` [PATCH v11 14/16] KVM: x86/vmx: Flip Arch LBREn bit on guest state change Yang Weijiang
@ 2022-05-06 3:33 ` Yang Weijiang
2022-05-06 15:11 ` Liang, Kan
2022-05-06 3:33 ` [PATCH v11 16/16] KVM: x86/cpuid: Advertise Arch LBR feature in CPUID Yang Weijiang
2022-05-10 15:55 ` [PATCH v11 00/16] Introduce Architectural LBR for vPMU Paolo Bonzini
16 siblings, 1 reply; 37+ messages in thread
From: Yang Weijiang @ 2022-05-06 3:33 UTC (permalink / raw)
To: pbonzini, jmattson, seanjc, kan.liang, like.xu.linux, vkuznets,
wei.w.wang, kvm, linux-kernel
Cc: Yang Weijiang
Arch LBR MSRs are xsave-supported, but they're operated as "independent"
xsave feature by PMU code, i.e., during thread/process context switch,
the MSRs are saved/restored with perf_event_task_sched_{in|out} instead
of generic kernel fpu switch code, i.e.,save_fpregs_to_fpstate() and
restore_fpregs_from_fpstate(). When vcpu guest/host fpu state swap happens,
Arch LBR MSRs are retained so they can be accessed directly.
Signed-off-by: Yang Weijiang <weijiang.yang@intel.com>
---
arch/x86/kvm/vmx/pmu_intel.c | 10 ++++++++++
1 file changed, 10 insertions(+)
diff --git a/arch/x86/kvm/vmx/pmu_intel.c b/arch/x86/kvm/vmx/pmu_intel.c
index 3adc8f28d142..c2eab6272b35 100644
--- a/arch/x86/kvm/vmx/pmu_intel.c
+++ b/arch/x86/kvm/vmx/pmu_intel.c
@@ -431,6 +431,11 @@ static int intel_pmu_get_msr(struct kvm_vcpu *vcpu, struct msr_data *msr_info)
case MSR_ARCH_LBR_CTL:
msr_info->data = vmcs_read64(GUEST_IA32_LBR_CTL);
return 0;
+ 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:
+ rdmsrl(msr_info->index, msr_info->data);
+ return 0;
default:
if ((pmc = get_gp_pmc(pmu, msr, MSR_IA32_PERFCTR0)) ||
(pmc = get_gp_pmc(pmu, msr, MSR_IA32_PMC0))) {
@@ -512,6 +517,11 @@ static int intel_pmu_set_msr(struct kvm_vcpu *vcpu, struct msr_data *msr_info)
(data & ARCH_LBR_CTL_LBREN))
intel_pmu_create_guest_lbr_event(vcpu);
return 0;
+ 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:
+ wrmsrl(msr_info->index, msr_info->data);
+ return 0;
default:
if ((pmc = get_gp_pmc(pmu, msr, MSR_IA32_PERFCTR0)) ||
(pmc = get_gp_pmc(pmu, msr, MSR_IA32_PMC0))) {
--
2.27.0
^ permalink raw reply related [flat|nested] 37+ messages in thread* Re: [PATCH v11 15/16] KVM: x86: Add Arch LBR data MSR access interface
2022-05-06 3:33 ` [PATCH v11 15/16] KVM: x86: Add Arch LBR data MSR access interface Yang Weijiang
@ 2022-05-06 15:11 ` Liang, Kan
0 siblings, 0 replies; 37+ messages in thread
From: Liang, Kan @ 2022-05-06 15:11 UTC (permalink / raw)
To: Yang Weijiang, pbonzini, jmattson, seanjc, like.xu.linux,
vkuznets, wei.w.wang, kvm, linux-kernel
On 5/5/2022 11:33 PM, Yang Weijiang wrote:
> Arch LBR MSRs are xsave-supported, but they're operated as "independent"
> xsave feature by PMU code, i.e., during thread/process context switch,
> the MSRs are saved/restored with perf_event_task_sched_{in|out} instead
> of generic kernel fpu switch code, i.e.,save_fpregs_to_fpstate() and
> restore_fpregs_from_fpstate(). When vcpu guest/host fpu state swap happens,
> Arch LBR MSRs are retained so they can be accessed directly.
>
> Signed-off-by: Yang Weijiang <weijiang.yang@intel.com>
Reviewed-by: Kan Liang <kan.liang@linux.intel.com>
> ---
> arch/x86/kvm/vmx/pmu_intel.c | 10 ++++++++++
> 1 file changed, 10 insertions(+)
>
> diff --git a/arch/x86/kvm/vmx/pmu_intel.c b/arch/x86/kvm/vmx/pmu_intel.c
> index 3adc8f28d142..c2eab6272b35 100644
> --- a/arch/x86/kvm/vmx/pmu_intel.c
> +++ b/arch/x86/kvm/vmx/pmu_intel.c
> @@ -431,6 +431,11 @@ static int intel_pmu_get_msr(struct kvm_vcpu *vcpu, struct msr_data *msr_info)
> case MSR_ARCH_LBR_CTL:
> msr_info->data = vmcs_read64(GUEST_IA32_LBR_CTL);
> return 0;
> + 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:
> + rdmsrl(msr_info->index, msr_info->data);
> + return 0;
> default:
> if ((pmc = get_gp_pmc(pmu, msr, MSR_IA32_PERFCTR0)) ||
> (pmc = get_gp_pmc(pmu, msr, MSR_IA32_PMC0))) {
> @@ -512,6 +517,11 @@ static int intel_pmu_set_msr(struct kvm_vcpu *vcpu, struct msr_data *msr_info)
> (data & ARCH_LBR_CTL_LBREN))
> intel_pmu_create_guest_lbr_event(vcpu);
> return 0;
> + 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:
> + wrmsrl(msr_info->index, msr_info->data);
> + 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] 37+ messages in thread
* [PATCH v11 16/16] KVM: x86/cpuid: Advertise Arch LBR feature in CPUID
2022-05-06 3:32 [PATCH v11 00/16] Introduce Architectural LBR for vPMU Yang Weijiang
` (14 preceding siblings ...)
2022-05-06 3:33 ` [PATCH v11 15/16] KVM: x86: Add Arch LBR data MSR access interface Yang Weijiang
@ 2022-05-06 3:33 ` Yang Weijiang
2022-05-06 15:13 ` Liang, Kan
2022-05-10 15:55 ` [PATCH v11 00/16] Introduce Architectural LBR for vPMU Paolo Bonzini
16 siblings, 1 reply; 37+ messages in thread
From: Yang Weijiang @ 2022-05-06 3:33 UTC (permalink / raw)
To: pbonzini, jmattson, seanjc, kan.liang, like.xu.linux, vkuznets,
wei.w.wang, kvm, linux-kernel
Cc: Yang Weijiang, Like Xu
Add Arch LBR feature bit in CPU cap-mask to expose the feature.
Only max LBR depth is supported 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 63870f78ed16..be4eb4e5e1fc 100644
--- a/arch/x86/kvm/cpuid.c
+++ b/arch/x86/kvm/cpuid.c
@@ -102,6 +102,16 @@ static int kvm_check_cpuid(struct kvm_vcpu *vcpu,
if (vaddr_bits != 48 && vaddr_bits != 57 && vaddr_bits != 0)
return -EINVAL;
}
+ best = cpuid_entry2_find(entries, nent, 0x1c, 0);
+ if (best) {
+ unsigned int eax, ebx, ecx, edx;
+
+ /* Reject user-space CPUID if depth is different from host's.*/
+ cpuid_count(0x1c, 0, &eax, &ebx, &ecx, &edx);
+
+ if ((best->eax & 0xff) != BIT(fls(eax & 0xff) - 1))
+ return -EINVAL;
+ }
/*
* Exposing dynamic xfeatures to the guest requires additional
@@ -598,7 +608,7 @@ void kvm_set_cpu_caps(void)
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(AMX_TILE) | F(AMX_INT8) | F(AMX_BF16)
+ F(AMX_TILE) | F(AMX_INT8) | F(AMX_BF16) | F(ARCH_LBR)
);
/* TSC_ADJUST and ARCH_CAPABILITIES are emulated in software. */
@@ -1044,6 +1054,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;
+ }
/* Intel AMX TILE */
case 0x1d:
if (!kvm_cpu_cap_has(X86_FEATURE_AMX_TILE)) {
--
2.27.0
^ permalink raw reply related [flat|nested] 37+ messages in thread* Re: [PATCH v11 16/16] KVM: x86/cpuid: Advertise Arch LBR feature in CPUID
2022-05-06 3:33 ` [PATCH v11 16/16] KVM: x86/cpuid: Advertise Arch LBR feature in CPUID Yang Weijiang
@ 2022-05-06 15:13 ` Liang, Kan
0 siblings, 0 replies; 37+ messages in thread
From: Liang, Kan @ 2022-05-06 15:13 UTC (permalink / raw)
To: Yang Weijiang, pbonzini, jmattson, seanjc, like.xu.linux,
vkuznets, wei.w.wang, kvm, linux-kernel
On 5/5/2022 11:33 PM, Yang Weijiang wrote:
> Add Arch LBR feature bit in CPU cap-mask to expose the feature.
> Only max LBR depth is supported 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>
Reviewed-by: Kan Liang <kan.liang@linux.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 63870f78ed16..be4eb4e5e1fc 100644
> --- a/arch/x86/kvm/cpuid.c
> +++ b/arch/x86/kvm/cpuid.c
> @@ -102,6 +102,16 @@ static int kvm_check_cpuid(struct kvm_vcpu *vcpu,
> if (vaddr_bits != 48 && vaddr_bits != 57 && vaddr_bits != 0)
> return -EINVAL;
> }
> + best = cpuid_entry2_find(entries, nent, 0x1c, 0);
> + if (best) {
> + unsigned int eax, ebx, ecx, edx;
> +
> + /* Reject user-space CPUID if depth is different from host's.*/
> + cpuid_count(0x1c, 0, &eax, &ebx, &ecx, &edx);
> +
> + if ((best->eax & 0xff) != BIT(fls(eax & 0xff) - 1))
> + return -EINVAL;
> + }
>
> /*
> * Exposing dynamic xfeatures to the guest requires additional
> @@ -598,7 +608,7 @@ void kvm_set_cpu_caps(void)
> 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(AMX_TILE) | F(AMX_INT8) | F(AMX_BF16)
> + F(AMX_TILE) | F(AMX_INT8) | F(AMX_BF16) | F(ARCH_LBR)
> );
>
> /* TSC_ADJUST and ARCH_CAPABILITIES are emulated in software. */
> @@ -1044,6 +1054,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;
> + }
> /* Intel AMX TILE */
> case 0x1d:
> if (!kvm_cpu_cap_has(X86_FEATURE_AMX_TILE)) {
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [PATCH v11 00/16] Introduce Architectural LBR for vPMU
2022-05-06 3:32 [PATCH v11 00/16] Introduce Architectural LBR for vPMU Yang Weijiang
` (15 preceding siblings ...)
2022-05-06 3:33 ` [PATCH v11 16/16] KVM: x86/cpuid: Advertise Arch LBR feature in CPUID Yang Weijiang
@ 2022-05-10 15:55 ` Paolo Bonzini
2022-05-11 0:29 ` Yang, Weijiang
16 siblings, 1 reply; 37+ messages in thread
From: Paolo Bonzini @ 2022-05-10 15:55 UTC (permalink / raw)
To: Yang Weijiang, jmattson, seanjc, kan.liang, like.xu.linux,
vkuznets, wei.w.wang, kvm, linux-kernel
On 5/6/22 05:32, Yang Weijiang wrote:
> [0] https://software.intel.com/sites/default/files/managed/c5/15/architecture-instruction-set-extensions-programming-reference.pdf
> [1] https://lore.kernel.org/lkml/1593780569-62993-1-git-send-email-kan.liang@linux.intel.com/
>
> Qemu patch:
> https://patchwork.ozlabs.org/project/qemu-devel/cover/20220215195258.29149-1-weijiang.yang@intel.com/
>
> Previous version:
> v10: https://lore.kernel.org/all/20220422075509.353942-1-weijiang.yang@intel.com/
>
> Changes in v11:
> 1. Moved MSR_ARCH_LBR_DEPTH/CTL check code to a unified function.[Kan]
> 2. Modified some commit messages per Kan's feedback.
> 3. Rebased the patch series to 5.18-rc5.
Thanks, this is mostly okay; the only remaining issues are Kan's
feedback and saving/restoring on SMM enter/exit.
The QEMU patches look good too.
Paolo
^ permalink raw reply [flat|nested] 37+ messages in thread* Re: [PATCH v11 00/16] Introduce Architectural LBR for vPMU
2022-05-10 15:55 ` [PATCH v11 00/16] Introduce Architectural LBR for vPMU Paolo Bonzini
@ 2022-05-11 0:29 ` Yang, Weijiang
0 siblings, 0 replies; 37+ messages in thread
From: Yang, Weijiang @ 2022-05-11 0:29 UTC (permalink / raw)
To: Paolo Bonzini, jmattson, seanjc, kan.liang, like.xu.linux,
vkuznets, Wang, Wei W, kvm, linux-kernel
On 5/10/2022 11:55 PM, Paolo Bonzini wrote:
> On 5/6/22 05:32, Yang Weijiang wrote:
>> [0] https://software.intel.com/sites/default/files/managed/c5/15/architecture-instruction-set-extensions-programming-reference.pdf
>> [1] https://lore.kernel.org/lkml/1593780569-62993-1-git-send-email-kan.liang@linux.intel.com/
>>
>> Qemu patch:
>> https://patchwork.ozlabs.org/project/qemu-devel/cover/20220215195258.29149-1-weijiang.yang@intel.com/
>>
>> Previous version:
>> v10: https://lore.kernel.org/all/20220422075509.353942-1-weijiang.yang@intel.com/
>>
>> Changes in v11:
>> 1. Moved MSR_ARCH_LBR_DEPTH/CTL check code to a unified function.[Kan]
>> 2. Modified some commit messages per Kan's feedback.
>> 3. Rebased the patch series to 5.18-rc5.
> Thanks, this is mostly okay; the only remaining issues are Kan's
> feedback and saving/restoring on SMM enter/exit.
Thanks Paolo, I'll fix Kan's feedback and the issue you mentioned in
next version.
>
> The QEMU patches look good too.
>
> Paolo
^ permalink raw reply [flat|nested] 37+ messages in thread