All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH RESEND] Enable full width counting for KVM: x86/pmu
@ 2020-05-29  7:43 Like Xu
  2020-05-29  7:43 ` [PATCH RESEND v4 1/2] KVM: x86/pmu: Tweak kvm_pmu_get_msr to pass 'struct msr_data' in Like Xu
                   ` (4 more replies)
  0 siblings, 5 replies; 19+ messages in thread
From: Like Xu @ 2020-05-29  7:43 UTC (permalink / raw)
  To: Paolo Bonzini
  Cc: Sean Christopherson, Vitaly Kuznetsov, Wanpeng Li, Jim Mattson,
	Joerg Roedel, kvm, linux-kernel, Like Xu

Hi Paolo,

As you said, you will queue the v3 of KVM patch, but it looks like we
are missing that part at the top of the kvm/queue tree.

For your convenience, let me resend v4 so that we can upstream this
feature in the next merged window. Also this patch series includes
patches for qemu and kvm-unit-tests. Please help review.

Previous:
https://lore.kernel.org/kvm/f1c77c79-7ff8-c5f3-e011-9874a4336217@redhat.com/

Like Xu (1):
  KVM: x86/pmu: Support full width counting
  [kvm-unit-tests] x86: pmu: Test full-width counter writes 
  [Qemu-devel] target/i386: define a new MSR based feature
 word - FEAT_PERF_CAPABILITIES

Wei Wang (1):
  KVM: x86/pmu: Tweak kvm_pmu_get_msr to pass 'struct msr_data' in

 arch/x86/include/asm/kvm_host.h |  1 +
 arch/x86/kvm/cpuid.c            |  2 +-
 arch/x86/kvm/pmu.c              |  4 +-
 arch/x86/kvm/pmu.h              |  4 +-
 arch/x86/kvm/svm/pmu.c          |  7 ++--
 arch/x86/kvm/vmx/capabilities.h | 11 +++++
 arch/x86/kvm/vmx/pmu_intel.c    | 71 +++++++++++++++++++++++++++------
 arch/x86/kvm/vmx/vmx.c          |  3 ++
 arch/x86/kvm/x86.c              |  6 ++-
 9 files changed, 87 insertions(+), 22 deletions(-)

-- 
2.21.3


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

* [PATCH RESEND v4 1/2] KVM: x86/pmu: Tweak kvm_pmu_get_msr to pass 'struct msr_data' in
  2020-05-29  7:43 [PATCH RESEND] Enable full width counting for KVM: x86/pmu Like Xu
@ 2020-05-29  7:43 ` Like Xu
  2020-05-29  7:43 ` [PATCH RESEND v4 2/2] KVM: x86/pmu: Support full width counting Like Xu
                   ` (3 subsequent siblings)
  4 siblings, 0 replies; 19+ messages in thread
From: Like Xu @ 2020-05-29  7:43 UTC (permalink / raw)
  To: Paolo Bonzini
  Cc: Sean Christopherson, Vitaly Kuznetsov, Wanpeng Li, Jim Mattson,
	Joerg Roedel, kvm, linux-kernel, Wei Wang

From: Wei Wang <wei.w.wang@intel.com>

Change kvm_pmu_get_msr() to get the msr_data struct, as the host_initiated
field from the struct could be used by get_msr. This also makes this API
consistent with kvm_pmu_set_msr. No functional changes.

Signed-off-by: Wei Wang <wei.w.wang@intel.com>
---
 arch/x86/kvm/pmu.c           |  4 ++--
 arch/x86/kvm/pmu.h           |  4 ++--
 arch/x86/kvm/svm/pmu.c       |  7 ++++---
 arch/x86/kvm/vmx/pmu_intel.c | 19 +++++++++++--------
 arch/x86/kvm/x86.c           |  4 ++--
 5 files changed, 21 insertions(+), 17 deletions(-)

diff --git a/arch/x86/kvm/pmu.c b/arch/x86/kvm/pmu.c
index a5078841bdac..b86346903f2e 100644
--- a/arch/x86/kvm/pmu.c
+++ b/arch/x86/kvm/pmu.c
@@ -397,9 +397,9 @@ static void kvm_pmu_mark_pmc_in_use(struct kvm_vcpu *vcpu, u32 msr)
 		__set_bit(pmc->idx, pmu->pmc_in_use);
 }
 
-int kvm_pmu_get_msr(struct kvm_vcpu *vcpu, u32 msr, u64 *data)
+int kvm_pmu_get_msr(struct kvm_vcpu *vcpu, struct msr_data *msr_info)
 {
-	return kvm_x86_ops.pmu_ops->get_msr(vcpu, msr, data);
+	return kvm_x86_ops.pmu_ops->get_msr(vcpu, msr_info);
 }
 
 int kvm_pmu_set_msr(struct kvm_vcpu *vcpu, struct msr_data *msr_info)
diff --git a/arch/x86/kvm/pmu.h b/arch/x86/kvm/pmu.h
index a6c78a797cb1..ab85eed8a6cc 100644
--- a/arch/x86/kvm/pmu.h
+++ b/arch/x86/kvm/pmu.h
@@ -32,7 +32,7 @@ struct kvm_pmu_ops {
 	struct kvm_pmc *(*msr_idx_to_pmc)(struct kvm_vcpu *vcpu, u32 msr);
 	int (*is_valid_rdpmc_ecx)(struct kvm_vcpu *vcpu, unsigned int idx);
 	bool (*is_valid_msr)(struct kvm_vcpu *vcpu, u32 msr);
-	int (*get_msr)(struct kvm_vcpu *vcpu, u32 msr, u64 *data);
+	int (*get_msr)(struct kvm_vcpu *vcpu, struct msr_data *msr_info);
 	int (*set_msr)(struct kvm_vcpu *vcpu, struct msr_data *msr_info);
 	void (*refresh)(struct kvm_vcpu *vcpu);
 	void (*init)(struct kvm_vcpu *vcpu);
@@ -147,7 +147,7 @@ void kvm_pmu_handle_event(struct kvm_vcpu *vcpu);
 int kvm_pmu_rdpmc(struct kvm_vcpu *vcpu, unsigned pmc, u64 *data);
 int kvm_pmu_is_valid_rdpmc_ecx(struct kvm_vcpu *vcpu, unsigned int idx);
 bool kvm_pmu_is_valid_msr(struct kvm_vcpu *vcpu, u32 msr);
-int kvm_pmu_get_msr(struct kvm_vcpu *vcpu, u32 msr, u64 *data);
+int kvm_pmu_get_msr(struct kvm_vcpu *vcpu, struct msr_data *msr_info);
 int kvm_pmu_set_msr(struct kvm_vcpu *vcpu, struct msr_data *msr_info);
 void kvm_pmu_refresh(struct kvm_vcpu *vcpu);
 void kvm_pmu_reset(struct kvm_vcpu *vcpu);
diff --git a/arch/x86/kvm/svm/pmu.c b/arch/x86/kvm/svm/pmu.c
index ce0b10fe5e2b..035da07500e8 100644
--- a/arch/x86/kvm/svm/pmu.c
+++ b/arch/x86/kvm/svm/pmu.c
@@ -215,21 +215,22 @@ static struct kvm_pmc *amd_msr_idx_to_pmc(struct kvm_vcpu *vcpu, u32 msr)
 	return pmc;
 }
 
-static int amd_pmu_get_msr(struct kvm_vcpu *vcpu, u32 msr, u64 *data)
+static int amd_pmu_get_msr(struct kvm_vcpu *vcpu, struct msr_data *msr_info)
 {
 	struct kvm_pmu *pmu = vcpu_to_pmu(vcpu);
 	struct kvm_pmc *pmc;
+	u32 msr = msr_info->index;
 
 	/* MSR_PERFCTRn */
 	pmc = get_gp_pmc_amd(pmu, msr, PMU_TYPE_COUNTER);
 	if (pmc) {
-		*data = pmc_read_counter(pmc);
+		msr_info->data = pmc_read_counter(pmc);
 		return 0;
 	}
 	/* MSR_EVNTSELn */
 	pmc = get_gp_pmc_amd(pmu, msr, PMU_TYPE_EVNTSEL);
 	if (pmc) {
-		*data = pmc->eventsel;
+		msr_info->data = pmc->eventsel;
 		return 0;
 	}
 
diff --git a/arch/x86/kvm/vmx/pmu_intel.c b/arch/x86/kvm/vmx/pmu_intel.c
index 7c857737b438..e1a303fefc16 100644
--- a/arch/x86/kvm/vmx/pmu_intel.c
+++ b/arch/x86/kvm/vmx/pmu_intel.c
@@ -184,35 +184,38 @@ static struct kvm_pmc *intel_msr_idx_to_pmc(struct kvm_vcpu *vcpu, u32 msr)
 	return pmc;
 }
 
-static int intel_pmu_get_msr(struct kvm_vcpu *vcpu, u32 msr, u64 *data)
+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;
+	u32 msr = msr_info->index;
 
 	switch (msr) {
 	case MSR_CORE_PERF_FIXED_CTR_CTRL:
-		*data = pmu->fixed_ctr_ctrl;
+		msr_info->data = pmu->fixed_ctr_ctrl;
 		return 0;
 	case MSR_CORE_PERF_GLOBAL_STATUS:
-		*data = pmu->global_status;
+		msr_info->data = pmu->global_status;
 		return 0;
 	case MSR_CORE_PERF_GLOBAL_CTRL:
-		*data = pmu->global_ctrl;
+		msr_info->data = pmu->global_ctrl;
 		return 0;
 	case MSR_CORE_PERF_GLOBAL_OVF_CTRL:
-		*data = pmu->global_ovf_ctrl;
+		msr_info->data = pmu->global_ovf_ctrl;
 		return 0;
 	default:
 		if ((pmc = get_gp_pmc(pmu, msr, MSR_IA32_PERFCTR0))) {
 			u64 val = pmc_read_counter(pmc);
-			*data = val & pmu->counter_bitmask[KVM_PMC_GP];
+			msr_info->data =
+				val & pmu->counter_bitmask[KVM_PMC_GP];
 			return 0;
 		} else if ((pmc = get_fixed_pmc(pmu, msr))) {
 			u64 val = pmc_read_counter(pmc);
-			*data = val & pmu->counter_bitmask[KVM_PMC_FIXED];
+			msr_info->data =
+				val & pmu->counter_bitmask[KVM_PMC_FIXED];
 			return 0;
 		} else if ((pmc = get_gp_pmc(pmu, msr, MSR_P6_EVNTSEL0))) {
-			*data = pmc->eventsel;
+			msr_info->data = pmc->eventsel;
 			return 0;
 		}
 	}
diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index d4aa7dc662d5..9f77776f1479 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -3113,7 +3113,7 @@ int kvm_get_msr_common(struct kvm_vcpu *vcpu, struct msr_data *msr_info)
 	case MSR_P6_PERFCTR0 ... MSR_P6_PERFCTR1:
 	case MSR_P6_EVNTSEL0 ... MSR_P6_EVNTSEL1:
 		if (kvm_pmu_is_valid_msr(vcpu, msr_info->index))
-			return kvm_pmu_get_msr(vcpu, msr_info->index, &msr_info->data);
+			return kvm_pmu_get_msr(vcpu, msr_info);
 		msr_info->data = 0;
 		break;
 	case MSR_IA32_UCODE_REV:
@@ -3275,7 +3275,7 @@ int kvm_get_msr_common(struct kvm_vcpu *vcpu, struct msr_data *msr_info)
 		break;
 	default:
 		if (kvm_pmu_is_valid_msr(vcpu, msr_info->index))
-			return kvm_pmu_get_msr(vcpu, msr_info->index, &msr_info->data);
+			return kvm_pmu_get_msr(vcpu, msr_info);
 		if (!ignore_msrs) {
 			vcpu_debug_ratelimited(vcpu, "unhandled rdmsr: 0x%x\n",
 					       msr_info->index);
-- 
2.21.3


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

* [PATCH RESEND v4 2/2] KVM: x86/pmu: Support full width counting
  2020-05-29  7:43 [PATCH RESEND] Enable full width counting for KVM: x86/pmu Like Xu
  2020-05-29  7:43 ` [PATCH RESEND v4 1/2] KVM: x86/pmu: Tweak kvm_pmu_get_msr to pass 'struct msr_data' in Like Xu
@ 2020-05-29  7:43 ` Like Xu
  2020-05-29  7:43 ` [kvm-unit-tests PATCH] x86: pmu: Test full-width counter writes support Like Xu
                   ` (2 subsequent siblings)
  4 siblings, 0 replies; 19+ messages in thread
From: Like Xu @ 2020-05-29  7:43 UTC (permalink / raw)
  To: Paolo Bonzini
  Cc: Sean Christopherson, Vitaly Kuznetsov, Wanpeng Li, Jim Mattson,
	Joerg Roedel, kvm, linux-kernel, Like Xu

Intel CPUs have a new alternative MSR range (starting from MSR_IA32_PMC0)
for GP counters that allows writing the full counter width. Enable this
range from a new capability bit (IA32_PERF_CAPABILITIES.FW_WRITE[bit 13]).

The guest would query CPUID to get the counter width, and sign extends
the counter values as needed. The traditional MSRs always limit to 32bit,
even though the counter internally is larger (48 or 57 bits).

When the new capability is set, use the alternative range which do not
have these restrictions. This lowers the overhead of perf stat slightly
because it has to do less interrupts to accumulate the counter value.

Signed-off-by: Like Xu <like.xu@linux.intel.com>
---
 arch/x86/include/asm/kvm_host.h |  1 +
 arch/x86/kvm/cpuid.c            |  2 +-
 arch/x86/kvm/vmx/capabilities.h | 11 +++++++
 arch/x86/kvm/vmx/pmu_intel.c    | 52 ++++++++++++++++++++++++++++++---
 arch/x86/kvm/vmx/vmx.c          |  3 ++
 arch/x86/kvm/x86.c              |  2 ++
 6 files changed, 66 insertions(+), 5 deletions(-)

diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
index 3485f8454088..452e1bfea4c1 100644
--- a/arch/x86/include/asm/kvm_host.h
+++ b/arch/x86/include/asm/kvm_host.h
@@ -600,6 +600,7 @@ struct kvm_vcpu_arch {
 	u64 ia32_xss;
 	u64 microcode_version;
 	u64 arch_capabilities;
+	u64 perf_capabilities;
 
 	/*
 	 * Paging state of the vcpu
diff --git a/arch/x86/kvm/cpuid.c b/arch/x86/kvm/cpuid.c
index cd708b0b460a..9d781308b872 100644
--- a/arch/x86/kvm/cpuid.c
+++ b/arch/x86/kvm/cpuid.c
@@ -296,7 +296,7 @@ void kvm_set_cpu_caps(void)
 		F(XMM3) | F(PCLMULQDQ) | 0 /* DTES64, MONITOR */ |
 		0 /* DS-CPL, VMX, SMX, EST */ |
 		0 /* TM2 */ | F(SSSE3) | 0 /* CNXT-ID */ | 0 /* Reserved */ |
-		F(FMA) | F(CX16) | 0 /* xTPR Update, PDCM */ |
+		F(FMA) | F(CX16) | 0 /* xTPR Update */ | F(PDCM) |
 		F(PCID) | 0 /* Reserved, DCA */ | F(XMM4_1) |
 		F(XMM4_2) | F(X2APIC) | F(MOVBE) | F(POPCNT) |
 		0 /* Reserved*/ | F(AES) | F(XSAVE) | 0 /* OSXSAVE */ | F(AVX) |
diff --git a/arch/x86/kvm/vmx/capabilities.h b/arch/x86/kvm/vmx/capabilities.h
index 8903475f751e..4bbd8b448d22 100644
--- a/arch/x86/kvm/vmx/capabilities.h
+++ b/arch/x86/kvm/vmx/capabilities.h
@@ -18,6 +18,8 @@ extern int __read_mostly pt_mode;
 #define PT_MODE_SYSTEM		0
 #define PT_MODE_HOST_GUEST	1
 
+#define PMU_CAP_FW_WRITES	(1ULL << 13)
+
 struct nested_vmx_msrs {
 	/*
 	 * We only store the "true" versions of the VMX capability MSRs. We
@@ -367,4 +369,13 @@ static inline bool vmx_pt_mode_is_host_guest(void)
 	return pt_mode == PT_MODE_HOST_GUEST;
 }
 
+static inline u64 vmx_get_perf_capabilities(void)
+{
+	/*
+	 * Since counters are virtualized, KVM would support full
+	 * width counting unconditionally, even if the host lacks it.
+	 */
+	return PMU_CAP_FW_WRITES;
+}
+
 #endif /* __KVM_X86_VMX_CAPS_H */
diff --git a/arch/x86/kvm/vmx/pmu_intel.c b/arch/x86/kvm/vmx/pmu_intel.c
index e1a303fefc16..d33d890b605f 100644
--- a/arch/x86/kvm/vmx/pmu_intel.c
+++ b/arch/x86/kvm/vmx/pmu_intel.c
@@ -18,6 +18,8 @@
 #include "nested.h"
 #include "pmu.h"
 
+#define MSR_PMC_FULL_WIDTH_BIT      (MSR_IA32_PMC0 - MSR_IA32_PERFCTR0)
+
 static struct kvm_event_hw_type_mapping intel_arch_events[] = {
 	/* Index must match CPUID 0x0A.EBX bit vector */
 	[0] = { 0x3c, 0x00, PERF_COUNT_HW_CPU_CYCLES },
@@ -150,6 +152,22 @@ static struct kvm_pmc *intel_rdpmc_ecx_to_pmc(struct kvm_vcpu *vcpu,
 	return &counters[array_index_nospec(idx, num_counters)];
 }
 
+static inline bool fw_writes_is_enabled(struct kvm_vcpu *vcpu)
+{
+	if (!guest_cpuid_has(vcpu, X86_FEATURE_PDCM))
+		return false;
+
+	return vcpu->arch.perf_capabilities & PMU_CAP_FW_WRITES;
+}
+
+static inline struct kvm_pmc *get_fw_gp_pmc(struct kvm_pmu *pmu, u32 msr)
+{
+	if (!fw_writes_is_enabled(pmu_to_vcpu(pmu)))
+		return NULL;
+
+	return get_gp_pmc(pmu, msr, MSR_IA32_PMC0);
+}
+
 static bool intel_is_valid_msr(struct kvm_vcpu *vcpu, u32 msr)
 {
 	struct kvm_pmu *pmu = vcpu_to_pmu(vcpu);
@@ -162,10 +180,13 @@ static bool intel_is_valid_msr(struct kvm_vcpu *vcpu, u32 msr)
 	case MSR_CORE_PERF_GLOBAL_OVF_CTRL:
 		ret = pmu->version > 1;
 		break;
+	case MSR_IA32_PERF_CAPABILITIES:
+		ret = guest_cpuid_has(vcpu, X86_FEATURE_PDCM);
+		break;
 	default:
 		ret = get_gp_pmc(pmu, msr, MSR_IA32_PERFCTR0) ||
 			get_gp_pmc(pmu, msr, MSR_P6_EVNTSEL0) ||
-			get_fixed_pmc(pmu, msr);
+			get_fixed_pmc(pmu, msr) || get_fw_gp_pmc(pmu, msr);
 		break;
 	}
 
@@ -203,8 +224,15 @@ static int intel_pmu_get_msr(struct kvm_vcpu *vcpu, struct msr_data *msr_info)
 	case MSR_CORE_PERF_GLOBAL_OVF_CTRL:
 		msr_info->data = pmu->global_ovf_ctrl;
 		return 0;
+	case MSR_IA32_PERF_CAPABILITIES:
+		if (!msr_info->host_initiated &&
+		    !guest_cpuid_has(vcpu, X86_FEATURE_PDCM))
+			return 1;
+		msr_info->data = vcpu->arch.perf_capabilities;
+		return 0;
 	default:
-		if ((pmc = get_gp_pmc(pmu, msr, MSR_IA32_PERFCTR0))) {
+		if ((pmc = get_gp_pmc(pmu, msr, MSR_IA32_PERFCTR0)) ||
+		    (pmc = get_gp_pmc(pmu, msr, MSR_IA32_PMC0))) {
 			u64 val = pmc_read_counter(pmc);
 			msr_info->data =
 				val & pmu->counter_bitmask[KVM_PMC_GP];
@@ -261,9 +289,22 @@ static int intel_pmu_set_msr(struct kvm_vcpu *vcpu, struct msr_data *msr_info)
 			return 0;
 		}
 		break;
+	case MSR_IA32_PERF_CAPABILITIES:
+		if (!msr_info->host_initiated)
+			return 1;
+		if (guest_cpuid_has(vcpu, X86_FEATURE_PDCM) ?
+			(data & ~vmx_get_perf_capabilities()) : data)
+			return 1;
+		vcpu->arch.perf_capabilities = data;
+		return 0;
 	default:
-		if ((pmc = get_gp_pmc(pmu, msr, MSR_IA32_PERFCTR0))) {
-			if (!msr_info->host_initiated)
+		if ((pmc = get_gp_pmc(pmu, msr, MSR_IA32_PERFCTR0)) ||
+		    (pmc = get_gp_pmc(pmu, msr, MSR_IA32_PMC0))) {
+			if ((msr & MSR_PMC_FULL_WIDTH_BIT) &&
+			    (data & ~pmu->counter_bitmask[KVM_PMC_GP]))
+				return 1;
+			if (!msr_info->host_initiated &&
+			    !(msr & MSR_PMC_FULL_WIDTH_BIT))
 				data = (s64)(s32)data;
 			pmc->counter += data - pmc_read_counter(pmc);
 			if (pmc->perf_event)
@@ -303,6 +344,7 @@ static void intel_pmu_refresh(struct kvm_vcpu *vcpu)
 	pmu->counter_bitmask[KVM_PMC_FIXED] = 0;
 	pmu->version = 0;
 	pmu->reserved_bits = 0xffffffff00200000ull;
+	vcpu->arch.perf_capabilities = 0;
 
 	entry = kvm_find_cpuid_entry(vcpu, 0xa, 0);
 	if (!entry)
@@ -315,6 +357,8 @@ static void intel_pmu_refresh(struct kvm_vcpu *vcpu)
 		return;
 
 	perf_get_x86_pmu_capability(&x86_pmu);
+	if (guest_cpuid_has(vcpu, X86_FEATURE_PDCM))
+		vcpu->arch.perf_capabilities = vmx_get_perf_capabilities();
 
 	pmu->nr_arch_gp_counters = min_t(int, eax.split.num_counters,
 					 x86_pmu.num_counters_gp);
diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c
index 7b55dc6230a9..d5b7c3b55ae4 100644
--- a/arch/x86/kvm/vmx/vmx.c
+++ b/arch/x86/kvm/vmx/vmx.c
@@ -1788,6 +1788,9 @@ static int vmx_get_msr_feature(struct kvm_msr_entry *msr)
 		if (!nested)
 			return 1;
 		return vmx_get_vmx_msr(&vmcs_config.nested, msr->index, &msr->data);
+	case MSR_IA32_PERF_CAPABILITIES:
+		msr->data = vmx_get_perf_capabilities();
+		return 0;
 	default:
 		return 1;
 	}
diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index 9f77776f1479..791d5955d1b8 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -1253,6 +1253,7 @@ static const u32 emulated_msrs_all[] = {
 	MSR_IA32_TSC_ADJUST,
 	MSR_IA32_TSCDEADLINE,
 	MSR_IA32_ARCH_CAPABILITIES,
+	MSR_IA32_PERF_CAPABILITIES,
 	MSR_IA32_MISC_ENABLE,
 	MSR_IA32_MCG_STATUS,
 	MSR_IA32_MCG_CTL,
@@ -1319,6 +1320,7 @@ static const u32 msr_based_features_all[] = {
 	MSR_F10H_DECFG,
 	MSR_IA32_UCODE_REV,
 	MSR_IA32_ARCH_CAPABILITIES,
+	MSR_IA32_PERF_CAPABILITIES,
 };
 
 static u32 msr_based_features[ARRAY_SIZE(msr_based_features_all)];
-- 
2.21.3


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

* [kvm-unit-tests PATCH] x86: pmu: Test full-width counter writes support
  2020-05-29  7:43 [PATCH RESEND] Enable full width counting for KVM: x86/pmu Like Xu
  2020-05-29  7:43 ` [PATCH RESEND v4 1/2] KVM: x86/pmu: Tweak kvm_pmu_get_msr to pass 'struct msr_data' in Like Xu
  2020-05-29  7:43 ` [PATCH RESEND v4 2/2] KVM: x86/pmu: Support full width counting Like Xu
@ 2020-05-29  7:43 ` Like Xu
  2020-06-16 10:49   ` Thomas Huth
  2021-05-11 21:27   ` Jim Mattson
  2020-05-29  7:43   ` Like Xu
  2020-05-29  8:47 ` [PATCH RESEND] Enable full width counting for KVM: x86/pmu Paolo Bonzini
  4 siblings, 2 replies; 19+ messages in thread
From: Like Xu @ 2020-05-29  7:43 UTC (permalink / raw)
  To: Paolo Bonzini
  Cc: Sean Christopherson, Vitaly Kuznetsov, Wanpeng Li, Jim Mattson,
	Joerg Roedel, kvm, linux-kernel, Like Xu

When the full-width writes capability is set, use the alternative MSR
range to write larger sign counter values (up to GP counter width).

Signed-off-by: Like Xu <like.xu@linux.intel.com>
---
 lib/x86/msr.h |   1 +
 x86/pmu.c     | 125 ++++++++++++++++++++++++++++++++++++++++----------
 2 files changed, 102 insertions(+), 24 deletions(-)

diff --git a/lib/x86/msr.h b/lib/x86/msr.h
index 8dca964..6ef5502 100644
--- a/lib/x86/msr.h
+++ b/lib/x86/msr.h
@@ -35,6 +35,7 @@
 #define MSR_IA32_SPEC_CTRL              0x00000048
 #define MSR_IA32_PRED_CMD               0x00000049
 
+#define MSR_IA32_PMC0                  0x000004c1
 #define MSR_IA32_PERFCTR0		0x000000c1
 #define MSR_IA32_PERFCTR1		0x000000c2
 #define MSR_FSB_FREQ			0x000000cd
diff --git a/x86/pmu.c b/x86/pmu.c
index f45621a..fb9bf0a 100644
--- a/x86/pmu.c
+++ b/x86/pmu.c
@@ -91,6 +91,9 @@ struct pmu_event {
 	{"fixed 3", MSR_CORE_PERF_FIXED_CTR0 + 2, 0.1*N, 30*N}
 };
 
+#define PMU_CAP_FW_WRITES	(1ULL << 13)
+static u64 gp_counter_base = MSR_IA32_PERFCTR0;
+
 static int num_counters;
 
 char *buf;
@@ -125,12 +128,13 @@ static bool check_irq(void)
 
 static bool is_gp(pmu_counter_t *evt)
 {
-	return evt->ctr < MSR_CORE_PERF_FIXED_CTR0;
+	return evt->ctr < MSR_CORE_PERF_FIXED_CTR0 ||
+		evt->ctr >= MSR_IA32_PMC0;
 }
 
 static int event_to_global_idx(pmu_counter_t *cnt)
 {
-	return cnt->ctr - (is_gp(cnt) ? MSR_IA32_PERFCTR0 :
+	return cnt->ctr - (is_gp(cnt) ? gp_counter_base :
 		(MSR_CORE_PERF_FIXED_CTR0 - FIXED_CNT_INDEX));
 }
 
@@ -226,7 +230,7 @@ static bool verify_counter(pmu_counter_t *cnt)
 static void check_gp_counter(struct pmu_event *evt)
 {
 	pmu_counter_t cnt = {
-		.ctr = MSR_IA32_PERFCTR0,
+		.ctr = gp_counter_base,
 		.config = EVNTSEL_OS | EVNTSEL_USR | evt->unit_sel,
 	};
 	int i;
@@ -276,7 +280,7 @@ static void check_counters_many(void)
 			continue;
 
 		cnt[n].count = 0;
-		cnt[n].ctr = MSR_IA32_PERFCTR0 + n;
+		cnt[n].ctr = gp_counter_base + n;
 		cnt[n].config = EVNTSEL_OS | EVNTSEL_USR |
 			gp_events[i % ARRAY_SIZE(gp_events)].unit_sel;
 		n++;
@@ -302,7 +306,7 @@ static void check_counter_overflow(void)
 	uint64_t count;
 	int i;
 	pmu_counter_t cnt = {
-		.ctr = MSR_IA32_PERFCTR0,
+		.ctr = gp_counter_base,
 		.config = EVNTSEL_OS | EVNTSEL_USR | gp_events[1].unit_sel /* instructions */,
 		.count = 0,
 	};
@@ -319,6 +323,8 @@ static void check_counter_overflow(void)
 		int idx;
 
 		cnt.count = 1 - count;
+		if (gp_counter_base == MSR_IA32_PMC0)
+			cnt.count &= (1ul << eax.split.bit_width) - 1;
 
 		if (i == num_counters) {
 			cnt.ctr = fixed_events[0].unit_sel;
@@ -346,7 +352,7 @@ static void check_counter_overflow(void)
 static void check_gp_counter_cmask(void)
 {
 	pmu_counter_t cnt = {
-		.ctr = MSR_IA32_PERFCTR0,
+		.ctr = gp_counter_base,
 		.config = EVNTSEL_OS | EVNTSEL_USR | gp_events[1].unit_sel /* instructions */,
 		.count = 0,
 	};
@@ -369,7 +375,7 @@ static void do_rdpmc_fast(void *ptr)
 
 static void check_rdpmc(void)
 {
-	uint64_t val = 0x1f3456789ull;
+	uint64_t val = 0xff0123456789ull;
 	bool exc;
 	int i;
 
@@ -378,20 +384,23 @@ static void check_rdpmc(void)
 	for (i = 0; i < num_counters; i++) {
 		uint64_t x;
 		pmu_counter_t cnt = {
-			.ctr = MSR_IA32_PERFCTR0 + i,
+			.ctr = gp_counter_base + i,
 			.idx = i
 		};
 
-		/*
-		 * Only the low 32 bits are writable, and the value is
-		 * sign-extended.
-		 */
-		x = (uint64_t)(int64_t)(int32_t)val;
+	        /*
+	         * Without full-width writes, only the low 32 bits are writable,
+	         * and the value is sign-extended.
+	         */
+		if (gp_counter_base == MSR_IA32_PERFCTR0)
+			x = (uint64_t)(int64_t)(int32_t)val;
+		else
+			x = (uint64_t)(int64_t)val;
 
 		/* Mask according to the number of supported bits */
 		x &= (1ull << eax.split.bit_width) - 1;
 
-		wrmsr(MSR_IA32_PERFCTR0 + i, val);
+		wrmsr(gp_counter_base + i, val);
 		report(rdpmc(i) == x, "cntr-%d", i);
 
 		exc = test_for_exception(GP_VECTOR, do_rdpmc_fast, &cnt);
@@ -423,8 +432,9 @@ static void check_rdpmc(void)
 static void check_running_counter_wrmsr(void)
 {
 	uint64_t status;
+	uint64_t count;
 	pmu_counter_t evt = {
-		.ctr = MSR_IA32_PERFCTR0,
+		.ctr = gp_counter_base,
 		.config = EVNTSEL_OS | EVNTSEL_USR | gp_events[1].unit_sel,
 		.count = 0,
 	};
@@ -433,7 +443,7 @@ static void check_running_counter_wrmsr(void)
 
 	start_event(&evt);
 	loop();
-	wrmsr(MSR_IA32_PERFCTR0, 0);
+	wrmsr(gp_counter_base, 0);
 	stop_event(&evt);
 	report(evt.count < gp_events[1].min, "cntr");
 
@@ -443,7 +453,13 @@ static void check_running_counter_wrmsr(void)
 
 	evt.count = 0;
 	start_event(&evt);
-	wrmsr(MSR_IA32_PERFCTR0, -1);
+
+	count = -1;
+	if (gp_counter_base == MSR_IA32_PMC0)
+		count &= (1ul << eax.split.bit_width) - 1;
+
+	wrmsr(gp_counter_base, count);
+
 	loop();
 	stop_event(&evt);
 	status = rdmsr(MSR_CORE_PERF_GLOBAL_STATUS);
@@ -452,6 +468,66 @@ static void check_running_counter_wrmsr(void)
 	report_prefix_pop();
 }
 
+static void check_counters(void)
+{
+	check_gp_counters();
+	check_fixed_counters();
+	check_rdpmc();
+	check_counters_many();
+	check_counter_overflow();
+	check_gp_counter_cmask();
+	check_running_counter_wrmsr();
+}
+
+static void do_unsupported_width_counter_write(void *index)
+{
+	wrmsr(MSR_IA32_PMC0 + *((int *) index), 0xffffff0123456789ull);
+}
+
+static void  check_gp_counters_write_width(void)
+{
+	u64 val_64 = 0xffffff0123456789ull;
+	u64 val_32 = val_64 & ((1ul << 32) - 1);
+	u64 val_max_width = val_64 & ((1ul << eax.split.bit_width) - 1);
+	int i;
+
+	/*
+	 * MSR_IA32_PERFCTRn supports 64-bit writes,
+	 * but only the lowest 32 bits are valid.
+	 */
+	for (i = 0; i < num_counters; i++) {
+		wrmsr(MSR_IA32_PERFCTR0 + i, val_32);
+		assert(rdmsr(MSR_IA32_PERFCTR0 + i) == val_32);
+		assert(rdmsr(MSR_IA32_PMC0 + i) == val_32);
+
+		wrmsr(MSR_IA32_PERFCTR0 + i, val_max_width);
+		assert(rdmsr(MSR_IA32_PERFCTR0 + i) == val_32);
+		assert(rdmsr(MSR_IA32_PMC0 + i) == val_32);
+
+		wrmsr(MSR_IA32_PERFCTR0 + i, val_64);
+		assert(rdmsr(MSR_IA32_PERFCTR0 + i) == val_32);
+		assert(rdmsr(MSR_IA32_PMC0 + i) == val_32);
+	}
+
+	/*
+	 * MSR_IA32_PMCn supports writing values ​​up to GP counter width,
+	 * and only the lowest bits of GP counter width are valid.
+	 */
+	for (i = 0; i < num_counters; i++) {
+		wrmsr(MSR_IA32_PMC0 + i, val_32);
+		assert(rdmsr(MSR_IA32_PMC0 + i) == val_32);
+		assert(rdmsr(MSR_IA32_PERFCTR0 + i) == val_32);
+
+		wrmsr(MSR_IA32_PMC0 + i, val_max_width);
+		assert(rdmsr(MSR_IA32_PMC0 + i) == val_max_width);
+		assert(rdmsr(MSR_IA32_PERFCTR0 + i) == val_max_width);
+
+		report(test_for_exception(GP_VECTOR,
+			do_unsupported_width_counter_write, &i),
+		"writing unsupported width to MSR_IA32_PMC%d raises #GP", i);
+	}
+}
+
 int main(int ac, char **av)
 {
 	struct cpuid id = cpuid(10);
@@ -480,13 +556,14 @@ int main(int ac, char **av)
 
 	apic_write(APIC_LVTPC, PC_VECTOR);
 
-	check_gp_counters();
-	check_fixed_counters();
-	check_rdpmc();
-	check_counters_many();
-	check_counter_overflow();
-	check_gp_counter_cmask();
-	check_running_counter_wrmsr();
+	check_counters();
+
+	if (rdmsr(MSR_IA32_PERF_CAPABILITIES) & PMU_CAP_FW_WRITES) {
+		gp_counter_base = MSR_IA32_PMC0;
+		report_prefix_push("full-width writes");
+		check_counters();
+		check_gp_counters_write_width();
+	}
 
 	return report_summary();
 }
-- 
2.21.3


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

* [Qemu-devel PATCH] target/i386: define a new MSR based feature word - FEAT_PERF_CAPABILITIES
  2020-05-29  7:43 [PATCH RESEND] Enable full width counting for KVM: x86/pmu Like Xu
@ 2020-05-29  7:43   ` Like Xu
  2020-05-29  7:43 ` [PATCH RESEND v4 2/2] KVM: x86/pmu: Support full width counting Like Xu
                     ` (3 subsequent siblings)
  4 siblings, 0 replies; 19+ messages in thread
From: Like Xu @ 2020-05-29  7:43 UTC (permalink / raw)
  To: Paolo Bonzini
  Cc: Sean Christopherson, Vitaly Kuznetsov, Wanpeng Li, Jim Mattson,
	Joerg Roedel, kvm, linux-kernel, Like Xu, Richard Henderson,
	Eduardo Habkost, Marcelo Tosatti, qemu-devel

The Perfmon and Debug Capability MSR named IA32_PERF_CAPABILITIES is
a feature-enumerating MSR, which only enumerates the feature full-width
write (via bit 13) by now which indicates the processor supports IA32_A_PMCx
interface for updating bits 32 and above of IA32_PMCx.

The existence of MSR IA32_PERF_CAPABILITIES is enumerated by CPUID.1:ECX[15].

Cc: Paolo Bonzini <pbonzini@redhat.com>
Cc: Richard Henderson <rth@twiddle.net>
Cc: Eduardo Habkost <ehabkost@redhat.com>
Cc: Marcelo Tosatti <mtosatti@redhat.com>
Cc: qemu-devel@nongnu.org
Signed-off-by: Like Xu <like.xu@linux.intel.com>
---
 target/i386/cpu.c | 29 +++++++++++++++++++++++++++++
 target/i386/cpu.h |  3 +++
 target/i386/kvm.c | 20 ++++++++++++++++++++
 3 files changed, 52 insertions(+)

diff --git a/target/i386/cpu.c b/target/i386/cpu.c
index 3733d9a279..be56966bb0 100644
--- a/target/i386/cpu.c
+++ b/target/i386/cpu.c
@@ -1139,6 +1139,22 @@ static FeatureWordInfo feature_word_info[FEATURE_WORDS] = {
             .index = MSR_IA32_CORE_CAPABILITY,
         },
     },
+    [FEAT_PERF_CAPABILITIES] = {
+        .type = MSR_FEATURE_WORD,
+        .feat_names = {
+            NULL, NULL, NULL, NULL,
+            NULL, NULL, NULL, NULL,
+            NULL, NULL, NULL, NULL,
+            NULL, "full-width-write", NULL, NULL,
+            NULL, NULL, NULL, NULL,
+            NULL, NULL, NULL, NULL,
+            NULL, NULL, NULL, NULL,
+            NULL, NULL, NULL, NULL,
+        },
+        .msr = {
+            .index = MSR_IA32_PERF_CAPABILITIES,
+        },
+    },
 
     [FEAT_VMX_PROCBASED_CTLS] = {
         .type = MSR_FEATURE_WORD,
@@ -1316,6 +1332,10 @@ static FeatureDep feature_dependencies[] = {
         .from = { FEAT_7_0_EDX,             CPUID_7_0_EDX_CORE_CAPABILITY },
         .to = { FEAT_CORE_CAPABILITY,       ~0ull },
     },
+    {
+        .from = { FEAT_1_ECX,             CPUID_EXT_PDCM },
+        .to = { FEAT_PERF_CAPABILITIES,       ~0ull },
+    },
     {
         .from = { FEAT_1_ECX,               CPUID_EXT_VMX },
         .to = { FEAT_VMX_PROCBASED_CTLS,    ~0ull },
@@ -5488,6 +5508,9 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t index, uint32_t count,
             *ebx |= (cs->nr_cores * cs->nr_threads) << 16;
             *edx |= CPUID_HT;
         }
+        if (!cpu->enable_pmu) {
+            *ecx &= ~CPUID_EXT_PDCM;
+        }
         break;
     case 2:
         /* cache info: needed for Pentium Pro compatibility */
@@ -6505,6 +6528,12 @@ static void x86_cpu_realizefn(DeviceState *dev, Error **errp)
         }
     }
 
+    if (kvm_enabled() && cpu->enable_pmu &&
+        (kvm_arch_get_supported_cpuid(kvm_state, 1, 0, R_ECX) &
+         CPUID_EXT_PDCM)) {
+        env->features[FEAT_1_ECX] |= CPUID_EXT_PDCM;
+    }
+
     if (cpu->ucode_rev == 0) {
         /* The default is the same as KVM's.  */
         if (IS_AMD_CPU(env)) {
diff --git a/target/i386/cpu.h b/target/i386/cpu.h
index 408392dbf6..fad2f874bd 100644
--- a/target/i386/cpu.h
+++ b/target/i386/cpu.h
@@ -356,6 +356,8 @@ typedef enum X86Seg {
 #define MSR_IA32_ARCH_CAPABILITIES      0x10a
 #define ARCH_CAP_TSX_CTRL_MSR		(1<<7)
 
+#define MSR_IA32_PERF_CAPABILITIES      0x345
+
 #define MSR_IA32_TSX_CTRL		0x122
 #define MSR_IA32_TSCDEADLINE            0x6e0
 
@@ -529,6 +531,7 @@ typedef enum FeatureWord {
     FEAT_XSAVE_COMP_HI, /* CPUID[EAX=0xd,ECX=0].EDX */
     FEAT_ARCH_CAPABILITIES,
     FEAT_CORE_CAPABILITY,
+    FEAT_PERF_CAPABILITIES,
     FEAT_VMX_PROCBASED_CTLS,
     FEAT_VMX_SECONDARY_CTLS,
     FEAT_VMX_PINBASED_CTLS,
diff --git a/target/i386/kvm.c b/target/i386/kvm.c
index 34f838728d..9be6f76b2c 100644
--- a/target/i386/kvm.c
+++ b/target/i386/kvm.c
@@ -106,6 +106,7 @@ static bool has_msr_core_capabs;
 static bool has_msr_vmx_vmfunc;
 static bool has_msr_ucode_rev;
 static bool has_msr_vmx_procbased_ctls2;
+static bool has_msr_perf_capabs;
 
 static uint32_t has_architectural_pmu_version;
 static uint32_t num_architectural_pmu_gp_counters;
@@ -2027,6 +2028,9 @@ static int kvm_get_supported_msrs(KVMState *s)
             case MSR_IA32_CORE_CAPABILITY:
                 has_msr_core_capabs = true;
                 break;
+            case MSR_IA32_PERF_CAPABILITIES:
+                has_msr_perf_capabs = true;
+                break;
             case MSR_IA32_VMX_VMFUNC:
                 has_msr_vmx_vmfunc = true;
                 break;
@@ -2643,6 +2647,18 @@ static void kvm_msr_entry_add_vmx(X86CPU *cpu, FeatureWordArray f)
                       VMCS12_MAX_FIELD_INDEX << 1);
 }
 
+static void kvm_msr_entry_add_perf(X86CPU *cpu, FeatureWordArray f)
+{
+    uint64_t kvm_perf_cap =
+        kvm_arch_get_supported_msr_feature(kvm_state,
+                                           MSR_IA32_PERF_CAPABILITIES);
+
+    if (kvm_perf_cap) {
+        kvm_msr_entry_add(cpu, MSR_IA32_PERF_CAPABILITIES,
+                        kvm_perf_cap & f[FEAT_PERF_CAPABILITIES]);
+    }
+}
+
 static int kvm_buf_set_msrs(X86CPU *cpu)
 {
     int ret = kvm_vcpu_ioctl(CPU(cpu), KVM_SET_MSRS, cpu->kvm_msr_buf);
@@ -2675,6 +2691,10 @@ static void kvm_init_msrs(X86CPU *cpu)
                           env->features[FEAT_CORE_CAPABILITY]);
     }
 
+    if (has_msr_perf_capabs && cpu->enable_pmu) {
+        kvm_msr_entry_add_perf(cpu, env->features);
+    }
+
     if (has_msr_ucode_rev) {
         kvm_msr_entry_add(cpu, MSR_IA32_UCODE_REV, cpu->ucode_rev);
     }
-- 
2.21.3


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

* [Qemu-devel PATCH] target/i386: define a new MSR based feature word - FEAT_PERF_CAPABILITIES
@ 2020-05-29  7:43   ` Like Xu
  0 siblings, 0 replies; 19+ messages in thread
From: Like Xu @ 2020-05-29  7:43 UTC (permalink / raw)
  To: Paolo Bonzini
  Cc: Wanpeng Li, Like Xu, kvm, Joerg Roedel, Marcelo Tosatti,
	linux-kernel, Sean Christopherson, qemu-devel, Eduardo Habkost,
	Vitaly Kuznetsov, Richard Henderson, Jim Mattson

The Perfmon and Debug Capability MSR named IA32_PERF_CAPABILITIES is
a feature-enumerating MSR, which only enumerates the feature full-width
write (via bit 13) by now which indicates the processor supports IA32_A_PMCx
interface for updating bits 32 and above of IA32_PMCx.

The existence of MSR IA32_PERF_CAPABILITIES is enumerated by CPUID.1:ECX[15].

Cc: Paolo Bonzini <pbonzini@redhat.com>
Cc: Richard Henderson <rth@twiddle.net>
Cc: Eduardo Habkost <ehabkost@redhat.com>
Cc: Marcelo Tosatti <mtosatti@redhat.com>
Cc: qemu-devel@nongnu.org
Signed-off-by: Like Xu <like.xu@linux.intel.com>
---
 target/i386/cpu.c | 29 +++++++++++++++++++++++++++++
 target/i386/cpu.h |  3 +++
 target/i386/kvm.c | 20 ++++++++++++++++++++
 3 files changed, 52 insertions(+)

diff --git a/target/i386/cpu.c b/target/i386/cpu.c
index 3733d9a279..be56966bb0 100644
--- a/target/i386/cpu.c
+++ b/target/i386/cpu.c
@@ -1139,6 +1139,22 @@ static FeatureWordInfo feature_word_info[FEATURE_WORDS] = {
             .index = MSR_IA32_CORE_CAPABILITY,
         },
     },
+    [FEAT_PERF_CAPABILITIES] = {
+        .type = MSR_FEATURE_WORD,
+        .feat_names = {
+            NULL, NULL, NULL, NULL,
+            NULL, NULL, NULL, NULL,
+            NULL, NULL, NULL, NULL,
+            NULL, "full-width-write", NULL, NULL,
+            NULL, NULL, NULL, NULL,
+            NULL, NULL, NULL, NULL,
+            NULL, NULL, NULL, NULL,
+            NULL, NULL, NULL, NULL,
+        },
+        .msr = {
+            .index = MSR_IA32_PERF_CAPABILITIES,
+        },
+    },
 
     [FEAT_VMX_PROCBASED_CTLS] = {
         .type = MSR_FEATURE_WORD,
@@ -1316,6 +1332,10 @@ static FeatureDep feature_dependencies[] = {
         .from = { FEAT_7_0_EDX,             CPUID_7_0_EDX_CORE_CAPABILITY },
         .to = { FEAT_CORE_CAPABILITY,       ~0ull },
     },
+    {
+        .from = { FEAT_1_ECX,             CPUID_EXT_PDCM },
+        .to = { FEAT_PERF_CAPABILITIES,       ~0ull },
+    },
     {
         .from = { FEAT_1_ECX,               CPUID_EXT_VMX },
         .to = { FEAT_VMX_PROCBASED_CTLS,    ~0ull },
@@ -5488,6 +5508,9 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t index, uint32_t count,
             *ebx |= (cs->nr_cores * cs->nr_threads) << 16;
             *edx |= CPUID_HT;
         }
+        if (!cpu->enable_pmu) {
+            *ecx &= ~CPUID_EXT_PDCM;
+        }
         break;
     case 2:
         /* cache info: needed for Pentium Pro compatibility */
@@ -6505,6 +6528,12 @@ static void x86_cpu_realizefn(DeviceState *dev, Error **errp)
         }
     }
 
+    if (kvm_enabled() && cpu->enable_pmu &&
+        (kvm_arch_get_supported_cpuid(kvm_state, 1, 0, R_ECX) &
+         CPUID_EXT_PDCM)) {
+        env->features[FEAT_1_ECX] |= CPUID_EXT_PDCM;
+    }
+
     if (cpu->ucode_rev == 0) {
         /* The default is the same as KVM's.  */
         if (IS_AMD_CPU(env)) {
diff --git a/target/i386/cpu.h b/target/i386/cpu.h
index 408392dbf6..fad2f874bd 100644
--- a/target/i386/cpu.h
+++ b/target/i386/cpu.h
@@ -356,6 +356,8 @@ typedef enum X86Seg {
 #define MSR_IA32_ARCH_CAPABILITIES      0x10a
 #define ARCH_CAP_TSX_CTRL_MSR		(1<<7)
 
+#define MSR_IA32_PERF_CAPABILITIES      0x345
+
 #define MSR_IA32_TSX_CTRL		0x122
 #define MSR_IA32_TSCDEADLINE            0x6e0
 
@@ -529,6 +531,7 @@ typedef enum FeatureWord {
     FEAT_XSAVE_COMP_HI, /* CPUID[EAX=0xd,ECX=0].EDX */
     FEAT_ARCH_CAPABILITIES,
     FEAT_CORE_CAPABILITY,
+    FEAT_PERF_CAPABILITIES,
     FEAT_VMX_PROCBASED_CTLS,
     FEAT_VMX_SECONDARY_CTLS,
     FEAT_VMX_PINBASED_CTLS,
diff --git a/target/i386/kvm.c b/target/i386/kvm.c
index 34f838728d..9be6f76b2c 100644
--- a/target/i386/kvm.c
+++ b/target/i386/kvm.c
@@ -106,6 +106,7 @@ static bool has_msr_core_capabs;
 static bool has_msr_vmx_vmfunc;
 static bool has_msr_ucode_rev;
 static bool has_msr_vmx_procbased_ctls2;
+static bool has_msr_perf_capabs;
 
 static uint32_t has_architectural_pmu_version;
 static uint32_t num_architectural_pmu_gp_counters;
@@ -2027,6 +2028,9 @@ static int kvm_get_supported_msrs(KVMState *s)
             case MSR_IA32_CORE_CAPABILITY:
                 has_msr_core_capabs = true;
                 break;
+            case MSR_IA32_PERF_CAPABILITIES:
+                has_msr_perf_capabs = true;
+                break;
             case MSR_IA32_VMX_VMFUNC:
                 has_msr_vmx_vmfunc = true;
                 break;
@@ -2643,6 +2647,18 @@ static void kvm_msr_entry_add_vmx(X86CPU *cpu, FeatureWordArray f)
                       VMCS12_MAX_FIELD_INDEX << 1);
 }
 
+static void kvm_msr_entry_add_perf(X86CPU *cpu, FeatureWordArray f)
+{
+    uint64_t kvm_perf_cap =
+        kvm_arch_get_supported_msr_feature(kvm_state,
+                                           MSR_IA32_PERF_CAPABILITIES);
+
+    if (kvm_perf_cap) {
+        kvm_msr_entry_add(cpu, MSR_IA32_PERF_CAPABILITIES,
+                        kvm_perf_cap & f[FEAT_PERF_CAPABILITIES]);
+    }
+}
+
 static int kvm_buf_set_msrs(X86CPU *cpu)
 {
     int ret = kvm_vcpu_ioctl(CPU(cpu), KVM_SET_MSRS, cpu->kvm_msr_buf);
@@ -2675,6 +2691,10 @@ static void kvm_init_msrs(X86CPU *cpu)
                           env->features[FEAT_CORE_CAPABILITY]);
     }
 
+    if (has_msr_perf_capabs && cpu->enable_pmu) {
+        kvm_msr_entry_add_perf(cpu, env->features);
+    }
+
     if (has_msr_ucode_rev) {
         kvm_msr_entry_add(cpu, MSR_IA32_UCODE_REV, cpu->ucode_rev);
     }
-- 
2.21.3



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

* Re: [PATCH RESEND] Enable full width counting for KVM: x86/pmu
  2020-05-29  7:43 [PATCH RESEND] Enable full width counting for KVM: x86/pmu Like Xu
                   ` (3 preceding siblings ...)
  2020-05-29  7:43   ` Like Xu
@ 2020-05-29  8:47 ` Paolo Bonzini
  2020-05-29  8:52   ` Xu, Like
  4 siblings, 1 reply; 19+ messages in thread
From: Paolo Bonzini @ 2020-05-29  8:47 UTC (permalink / raw)
  To: Like Xu
  Cc: Sean Christopherson, Vitaly Kuznetsov, Wanpeng Li, Jim Mattson,
	Joerg Roedel, kvm, linux-kernel

On 29/05/20 09:43, Like Xu wrote:
> Hi Paolo,
> 
> As you said, you will queue the v3 of KVM patch, but it looks like we
> are missing that part at the top of the kvm/queue tree.
> 
> For your convenience, let me resend v4 so that we can upstream this
> feature in the next merged window. Also this patch series includes
> patches for qemu and kvm-unit-tests. Please help review.
> 
> Previous:
> https://lore.kernel.org/kvm/f1c77c79-7ff8-c5f3-e011-9874a4336217@redhat.com/
> 
> Like Xu (1):
>   KVM: x86/pmu: Support full width counting
>   [kvm-unit-tests] x86: pmu: Test full-width counter writes 
>   [Qemu-devel] target/i386: define a new MSR based feature
>  word - FEAT_PERF_CAPABILITIES
> 
> Wei Wang (1):
>   KVM: x86/pmu: Tweak kvm_pmu_get_msr to pass 'struct msr_data' in
> 
>  arch/x86/include/asm/kvm_host.h |  1 +
>  arch/x86/kvm/cpuid.c            |  2 +-
>  arch/x86/kvm/pmu.c              |  4 +-
>  arch/x86/kvm/pmu.h              |  4 +-
>  arch/x86/kvm/svm/pmu.c          |  7 ++--
>  arch/x86/kvm/vmx/capabilities.h | 11 +++++
>  arch/x86/kvm/vmx/pmu_intel.c    | 71 +++++++++++++++++++++++++++------
>  arch/x86/kvm/vmx/vmx.c          |  3 ++
>  arch/x86/kvm/x86.c              |  6 ++-
>  9 files changed, 87 insertions(+), 22 deletions(-)
> 

Thanks, I was busy with AMD stuff as you saw. :)  I've queued it now.

Paolo


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

* Re: [PATCH RESEND] Enable full width counting for KVM: x86/pmu
  2020-05-29  8:47 ` [PATCH RESEND] Enable full width counting for KVM: x86/pmu Paolo Bonzini
@ 2020-05-29  8:52   ` Xu, Like
  0 siblings, 0 replies; 19+ messages in thread
From: Xu, Like @ 2020-05-29  8:52 UTC (permalink / raw)
  To: Paolo Bonzini, Like Xu
  Cc: Sean Christopherson, Vitaly Kuznetsov, Wanpeng Li, Jim Mattson,
	Joerg Roedel, kvm, linux-kernel

On 2020/5/29 16:47, Paolo Bonzini wrote:
> On 29/05/20 09:43, Like Xu wrote:
>> Hi Paolo,
>>
>> As you said, you will queue the v3 of KVM patch, but it looks like we
>> are missing that part at the top of the kvm/queue tree.
>>
>> For your convenience, let me resend v4 so that we can upstream this
>> feature in the next merged window. Also this patch series includes
>> patches for qemu and kvm-unit-tests. Please help review.
>>
>> Previous:
>> https://lore.kernel.org/kvm/f1c77c79-7ff8-c5f3-e011-9874a4336217@redhat.com/
>>
>> Like Xu (1):
>>    KVM: x86/pmu: Support full width counting
>>    [kvm-unit-tests] x86: pmu: Test full-width counter writes
>>    [Qemu-devel] target/i386: define a new MSR based feature
>>   word - FEAT_PERF_CAPABILITIES
>>
>> Wei Wang (1):
>>    KVM: x86/pmu: Tweak kvm_pmu_get_msr to pass 'struct msr_data' in
>>
>>   arch/x86/include/asm/kvm_host.h |  1 +
>>   arch/x86/kvm/cpuid.c            |  2 +-
>>   arch/x86/kvm/pmu.c              |  4 +-
>>   arch/x86/kvm/pmu.h              |  4 +-
>>   arch/x86/kvm/svm/pmu.c          |  7 ++--
>>   arch/x86/kvm/vmx/capabilities.h | 11 +++++
>>   arch/x86/kvm/vmx/pmu_intel.c    | 71 +++++++++++++++++++++++++++------
>>   arch/x86/kvm/vmx/vmx.c          |  3 ++
>>   arch/x86/kvm/x86.c              |  6 ++-
>>   9 files changed, 87 insertions(+), 22 deletions(-)
>>
> Thanks, I was busy with AMD stuff as you saw. :)  I've queued it now.
Yes, we all know you're busy.

I will be very grateful if you could comment the KVM part for the LBR 
feature. :D

https://lore.kernel.org/kvm/20200514083054.62538-1-like.xu@linux.intel.com/

Thanks,
Like Xu
>
> Paolo
>


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

* Re: [Qemu-devel PATCH] target/i386: define a new MSR based feature word - FEAT_PERF_CAPABILITIES
  2020-05-29  7:43   ` Like Xu
@ 2020-05-29 10:23     ` Paolo Bonzini
  -1 siblings, 0 replies; 19+ messages in thread
From: Paolo Bonzini @ 2020-05-29 10:23 UTC (permalink / raw)
  To: Like Xu
  Cc: Sean Christopherson, Vitaly Kuznetsov, Wanpeng Li, Jim Mattson,
	Joerg Roedel, kvm, linux-kernel, Richard Henderson,
	Eduardo Habkost, Marcelo Tosatti, qemu-devel

On 29/05/20 09:43, Like Xu wrote:
> +        if (!cpu->enable_pmu) {
> +            *ecx &= ~CPUID_EXT_PDCM;
> +        }
>          break;
>      case 2:
>          /* cache info: needed for Pentium Pro compatibility */
> @@ -6505,6 +6528,12 @@ static void x86_cpu_realizefn(DeviceState *dev, Error **errp)
>          }
>      }
>  
> +    if (kvm_enabled() && cpu->enable_pmu &&
> +        (kvm_arch_get_supported_cpuid(kvm_state, 1, 0, R_ECX) &
> +         CPUID_EXT_PDCM)) {
> +        env->features[FEAT_1_ECX] |= CPUID_EXT_PDCM;
> +    }
> +

I'm dropping this hunk two hunks because it's going to break live
migration with e.g. "-cpu IvyBridge,pmu=on".  We will have to add PDCM
by default only to future CPU models, but "-cpu host,pmu=on" will pick
it up automatically.

Paolo


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

* Re: [Qemu-devel PATCH] target/i386: define a new MSR based feature word - FEAT_PERF_CAPABILITIES
@ 2020-05-29 10:23     ` Paolo Bonzini
  0 siblings, 0 replies; 19+ messages in thread
From: Paolo Bonzini @ 2020-05-29 10:23 UTC (permalink / raw)
  To: Like Xu
  Cc: Wanpeng Li, Eduardo Habkost, kvm, Joerg Roedel, Marcelo Tosatti,
	linux-kernel, Sean Christopherson, qemu-devel, Vitaly Kuznetsov,
	Richard Henderson, Jim Mattson

On 29/05/20 09:43, Like Xu wrote:
> +        if (!cpu->enable_pmu) {
> +            *ecx &= ~CPUID_EXT_PDCM;
> +        }
>          break;
>      case 2:
>          /* cache info: needed for Pentium Pro compatibility */
> @@ -6505,6 +6528,12 @@ static void x86_cpu_realizefn(DeviceState *dev, Error **errp)
>          }
>      }
>  
> +    if (kvm_enabled() && cpu->enable_pmu &&
> +        (kvm_arch_get_supported_cpuid(kvm_state, 1, 0, R_ECX) &
> +         CPUID_EXT_PDCM)) {
> +        env->features[FEAT_1_ECX] |= CPUID_EXT_PDCM;
> +    }
> +

I'm dropping this hunk two hunks because it's going to break live
migration with e.g. "-cpu IvyBridge,pmu=on".  We will have to add PDCM
by default only to future CPU models, but "-cpu host,pmu=on" will pick
it up automatically.

Paolo



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

* Re: [kvm-unit-tests PATCH] x86: pmu: Test full-width counter writes support
  2020-05-29  7:43 ` [kvm-unit-tests PATCH] x86: pmu: Test full-width counter writes support Like Xu
@ 2020-06-16 10:49   ` Thomas Huth
  2020-06-16 12:28     ` Paolo Bonzini
  2021-05-11 21:27   ` Jim Mattson
  1 sibling, 1 reply; 19+ messages in thread
From: Thomas Huth @ 2020-06-16 10:49 UTC (permalink / raw)
  To: Like Xu, Paolo Bonzini
  Cc: Sean Christopherson, Vitaly Kuznetsov, Wanpeng Li, Jim Mattson,
	Joerg Roedel, kvm, linux-kernel

On 29/05/2020 09.43, Like Xu wrote:
> When the full-width writes capability is set, use the alternative MSR
> range to write larger sign counter values (up to GP counter width).
> 
> Signed-off-by: Like Xu <like.xu@linux.intel.com>
> ---
>  lib/x86/msr.h |   1 +
>  x86/pmu.c     | 125 ++++++++++++++++++++++++++++++++++++++++----------
>  2 files changed, 102 insertions(+), 24 deletions(-)
[...]
> @@ -452,6 +468,66 @@ static void check_running_counter_wrmsr(void)
>  	report_prefix_pop();
>  }
>  
> +static void check_counters(void)
> +{
> +	check_gp_counters();
> +	check_fixed_counters();
> +	check_rdpmc();
> +	check_counters_many();
> +	check_counter_overflow();
> +	check_gp_counter_cmask();
> +	check_running_counter_wrmsr();
> +}
> +
> +static void do_unsupported_width_counter_write(void *index)
> +{
> +	wrmsr(MSR_IA32_PMC0 + *((int *) index), 0xffffff0123456789ull);
> +}
> +
> +static void  check_gp_counters_write_width(void)
> +{
> +	u64 val_64 = 0xffffff0123456789ull;
> +	u64 val_32 = val_64 & ((1ul << 32) - 1);
 Hi,

this broke compilation on 32-bit hosts:

 https://travis-ci.com/github/huth/kvm-unit-tests/jobs/349654654#L710

Fix should be easy, I guess - either use 1ull or specify the mask
0xffffffff directly.

 Thomas


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

* Re: [kvm-unit-tests PATCH] x86: pmu: Test full-width counter writes support
  2020-06-16 10:49   ` Thomas Huth
@ 2020-06-16 12:28     ` Paolo Bonzini
  2020-06-19 19:46       ` Nadav Amit
  0 siblings, 1 reply; 19+ messages in thread
From: Paolo Bonzini @ 2020-06-16 12:28 UTC (permalink / raw)
  To: Thomas Huth, Like Xu
  Cc: Sean Christopherson, Vitaly Kuznetsov, Wanpeng Li, Jim Mattson,
	Joerg Roedel, kvm, linux-kernel

On 16/06/20 12:49, Thomas Huth wrote:
> On 29/05/2020 09.43, Like Xu wrote:
>> When the full-width writes capability is set, use the alternative MSR
>> range to write larger sign counter values (up to GP counter width).
>>
>> Signed-off-by: Like Xu <like.xu@linux.intel.com>
>> ---
>>  lib/x86/msr.h |   1 +
>>  x86/pmu.c     | 125 ++++++++++++++++++++++++++++++++++++++++----------
>>  2 files changed, 102 insertions(+), 24 deletions(-)
> [...]
>> @@ -452,6 +468,66 @@ static void check_running_counter_wrmsr(void)
>>  	report_prefix_pop();
>>  }
>>  
>> +static void check_counters(void)
>> +{
>> +	check_gp_counters();
>> +	check_fixed_counters();
>> +	check_rdpmc();
>> +	check_counters_many();
>> +	check_counter_overflow();
>> +	check_gp_counter_cmask();
>> +	check_running_counter_wrmsr();
>> +}
>> +
>> +static void do_unsupported_width_counter_write(void *index)
>> +{
>> +	wrmsr(MSR_IA32_PMC0 + *((int *) index), 0xffffff0123456789ull);
>> +}
>> +
>> +static void  check_gp_counters_write_width(void)
>> +{
>> +	u64 val_64 = 0xffffff0123456789ull;
>> +	u64 val_32 = val_64 & ((1ul << 32) - 1);
>  Hi,
> 
> this broke compilation on 32-bit hosts:
> 
>  https://travis-ci.com/github/huth/kvm-unit-tests/jobs/349654654#L710
> 
> Fix should be easy, I guess - either use 1ull or specify the mask
> 0xffffffff directly.

Or

u64 val_32 = (u64)(u32) val_64;

I'll send a patch.

Paolo


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

* Re: [kvm-unit-tests PATCH] x86: pmu: Test full-width counter writes support
  2020-06-16 12:28     ` Paolo Bonzini
@ 2020-06-19 19:46       ` Nadav Amit
  0 siblings, 0 replies; 19+ messages in thread
From: Nadav Amit @ 2020-06-19 19:46 UTC (permalink / raw)
  To: Paolo Bonzini
  Cc: Thomas Huth, Like Xu, Sean Christopherson, Vitaly Kuznetsov,
	Wanpeng Li, Jim Mattson, Joerg Roedel, kvm, linux-kernel

> On Jun 16, 2020, at 5:28 AM, Paolo Bonzini <pbonzini@redhat.com> wrote:
> 
> On 16/06/20 12:49, Thomas Huth wrote:
>> On 29/05/2020 09.43, Like Xu wrote:
>>> When the full-width writes capability is set, use the alternative MSR
>>> range to write larger sign counter values (up to GP counter width).
>>> 
>>> Signed-off-by: Like Xu <like.xu@linux.intel.com>
>>> ---
>>> lib/x86/msr.h |   1 +
>>> x86/pmu.c     | 125 ++++++++++++++++++++++++++++++++++++++++----------
>>> 2 files changed, 102 insertions(+), 24 deletions(-)
>> [...]
>>> @@ -452,6 +468,66 @@ static void check_running_counter_wrmsr(void)
>>> 	report_prefix_pop();
>>> }
>>> 
>>> +static void check_counters(void)
>>> +{
>>> +	check_gp_counters();
>>> +	check_fixed_counters();
>>> +	check_rdpmc();
>>> +	check_counters_many();
>>> +	check_counter_overflow();
>>> +	check_gp_counter_cmask();
>>> +	check_running_counter_wrmsr();
>>> +}
>>> +
>>> +static void do_unsupported_width_counter_write(void *index)
>>> +{
>>> +	wrmsr(MSR_IA32_PMC0 + *((int *) index), 0xffffff0123456789ull);
>>> +}
>>> +
>>> +static void  check_gp_counters_write_width(void)
>>> +{
>>> +	u64 val_64 = 0xffffff0123456789ull;
>>> +	u64 val_32 = val_64 & ((1ul << 32) - 1);
>> Hi,
>> 
>> this broke compilation on 32-bit hosts:
>> 
>> https://travis-ci.com/github/huth/kvm-unit-tests/jobs/349654654#L710
>> 
>> Fix should be easy, I guess - either use 1ull or specify the mask
>> 0xffffffff directly.
> 
> Or
> 
> u64 val_32 = (u64)(u32) val_64;
> 
> I'll send a patch.

I missed this correspondence, but while running the tests on 32-bit I
encountered two additional cases of wrong masks that caused failures.

I have sent a separate patch for your consideration.


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

* Re: [kvm-unit-tests PATCH] x86: pmu: Test full-width counter writes support
  2020-05-29  7:43 ` [kvm-unit-tests PATCH] x86: pmu: Test full-width counter writes support Like Xu
  2020-06-16 10:49   ` Thomas Huth
@ 2021-05-11 21:27   ` Jim Mattson
  2021-05-12  6:33     ` Like Xu
  1 sibling, 1 reply; 19+ messages in thread
From: Jim Mattson @ 2021-05-11 21:27 UTC (permalink / raw)
  To: Like Xu; +Cc: Paolo Bonzini, kvm list

On Fri, May 29, 2020 at 12:44 AM Like Xu <like.xu@linux.intel.com> wrote:
>
> When the full-width writes capability is set, use the alternative MSR
> range to write larger sign counter values (up to GP counter width).
>
> Signed-off-by: Like Xu <like.xu@linux.intel.com>
> ---

> +       /*
> +        * MSR_IA32_PMCn supports writing values ​​up to GP counter width,
> +        * and only the lowest bits of GP counter width are valid.
> +        */

Could you rewrite this comment in ASCII, please? I would do it, but
I'm not sure what the correct translation is.

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

* Re: [kvm-unit-tests PATCH] x86: pmu: Test full-width counter writes support
  2021-05-11 21:27   ` Jim Mattson
@ 2021-05-12  6:33     ` Like Xu
  2022-01-08  0:06       ` Jim Mattson
  0 siblings, 1 reply; 19+ messages in thread
From: Like Xu @ 2021-05-12  6:33 UTC (permalink / raw)
  To: Jim Mattson; +Cc: Paolo Bonzini, kvm list, Thomas Huth, Andrew Jones

On 2021/5/12 5:27, Jim Mattson wrote:
> On Fri, May 29, 2020 at 12:44 AM Like Xu <like.xu@linux.intel.com> wrote:
>>
>> When the full-width writes capability is set, use the alternative MSR
>> range to write larger sign counter values (up to GP counter width).
>>
>> Signed-off-by: Like Xu <like.xu@linux.intel.com>
>> ---
> 
>> +       /*
>> +        * MSR_IA32_PMCn supports writing values ​​up to GP counter width,
>> +        * and only the lowest bits of GP counter width are valid.
>> +        */
> 
> Could you rewrite this comment in ASCII, please? I would do it, but
> I'm not sure what the correct translation is.
> 

My first submitted patch says that
they are just Unicode "ZERO WIDTH SPACE".

https://lore.kernel.org/kvm/20200508083218.120559-2-like.xu@linux.intel.com/

Here you go:

---

 From 1b058846aabcd7a85b5c5f41cb2b63b6a348bdc4 Mon Sep 17 00:00:00 2001
From: Like Xu <like.xu@linux.intel.com>
Date: Wed, 12 May 2021 14:26:40 +0800
Subject: [PATCH] x86: pmu: Fix a comment about full-width counter writes
  support

Remove two Unicode characters 'ZERO WIDTH SPACE' (U+200B).

Fixes: 22f2901a0e ("x86: pmu: Test full-width counter writes support")
Reported-by: Jim Mattson <jmattson@google.com>
Signed-off-by: Like Xu <like.xu@linux.intel.com>
---
  x86/pmu.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/x86/pmu.c b/x86/pmu.c
index 5a3d55b..6cb3506 100644
--- a/x86/pmu.c
+++ b/x86/pmu.c
@@ -510,7 +510,7 @@ static void  check_gp_counters_write_width(void)
         }

         /*
-        * MSR_IA32_PMCn supports writing values ​​up to GP 
counter width,
+        * MSR_IA32_PMCn supports writing values up to GP counter width,
          * and only the lowest bits of GP counter width are valid.
          */
         for (i = 0; i < num_counters; i++) {
--
2.31.1

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

* Re: [kvm-unit-tests PATCH] x86: pmu: Test full-width counter writes support
  2021-05-12  6:33     ` Like Xu
@ 2022-01-08  0:06       ` Jim Mattson
  2022-01-10  6:34         ` Like Xu
  2022-03-11 16:36         ` Thomas Huth
  0 siblings, 2 replies; 19+ messages in thread
From: Jim Mattson @ 2022-01-08  0:06 UTC (permalink / raw)
  To: Paolo Bonzini; +Cc: kvm list, Thomas Huth, Andrew Jones, Like Xu

On Tue, May 11, 2021 at 11:33 PM Like Xu <like.xu@linux.intel.com> wrote:
>
> On 2021/5/12 5:27, Jim Mattson wrote:
> > On Fri, May 29, 2020 at 12:44 AM Like Xu <like.xu@linux.intel.com> wrote:
> >>
> >> When the full-width writes capability is set, use the alternative MSR
> >> range to write larger sign counter values (up to GP counter width).
> >>
> >> Signed-off-by: Like Xu <like.xu@linux.intel.com>
> >> ---
> >
> >> +       /*
> >> +        * MSR_IA32_PMCn supports writing values ​​up to GP counter width,
> >> +        * and only the lowest bits of GP counter width are valid.
> >> +        */
> >
> > Could you rewrite this comment in ASCII, please? I would do it, but
> > I'm not sure what the correct translation is.
> >
>
> My first submitted patch says that
> they are just Unicode "ZERO WIDTH SPACE".
>
> https://lore.kernel.org/kvm/20200508083218.120559-2-like.xu@linux.intel.com/
>
> Here you go:
>
> ---
>
>  From 1b058846aabcd7a85b5c5f41cb2b63b6a348bdc4 Mon Sep 17 00:00:00 2001
> From: Like Xu <like.xu@linux.intel.com>
> Date: Wed, 12 May 2021 14:26:40 +0800
> Subject: [PATCH] x86: pmu: Fix a comment about full-width counter writes
>   support
>
> Remove two Unicode characters 'ZERO WIDTH SPACE' (U+200B).
>
> Fixes: 22f2901a0e ("x86: pmu: Test full-width counter writes support")
> Reported-by: Jim Mattson <jmattson@google.com>
> Signed-off-by: Like Xu <like.xu@linux.intel.com>
> ---
>   x86/pmu.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/x86/pmu.c b/x86/pmu.c
> index 5a3d55b..6cb3506 100644
> --- a/x86/pmu.c
> +++ b/x86/pmu.c
> @@ -510,7 +510,7 @@ static void  check_gp_counters_write_width(void)
>          }
>
>          /*
> -        * MSR_IA32_PMCn supports writing values ​​up to GP
> counter width,
> +        * MSR_IA32_PMCn supports writing values up to GP counter width,
>           * and only the lowest bits of GP counter width are valid.
>           */
>          for (i = 0; i < num_counters; i++) {
> --
> 2.31.1

Paolo:

Did this patch get overlooked? I'm still seeing the unicode characters
in this comment.

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

* Re: [kvm-unit-tests PATCH] x86: pmu: Test full-width counter writes support
  2022-01-08  0:06       ` Jim Mattson
@ 2022-01-10  6:34         ` Like Xu
  2022-03-11 16:36         ` Thomas Huth
  1 sibling, 0 replies; 19+ messages in thread
From: Like Xu @ 2022-01-10  6:34 UTC (permalink / raw)
  To: Jim Mattson, Paolo Bonzini; +Cc: kvm list, Thomas Huth, Andrew Jones

On 8/1/2022 8:06 am, Jim Mattson wrote:
> On Tue, May 11, 2021 at 11:33 PM Like Xu <like.xu@linux.intel.com> wrote:
>>
>> On 2021/5/12 5:27, Jim Mattson wrote:
>>> On Fri, May 29, 2020 at 12:44 AM Like Xu <like.xu@linux.intel.com> wrote:
>>>>
>>>> When the full-width writes capability is set, use the alternative MSR
>>>> range to write larger sign counter values (up to GP counter width).
>>>>
>>>> Signed-off-by: Like Xu <like.xu@linux.intel.com>
>>>> ---
>>>
>>>> +       /*
>>>> +        * MSR_IA32_PMCn supports writing values ​​up to GP counter width,
>>>> +        * and only the lowest bits of GP counter width are valid.
>>>> +        */
>>>
>>> Could you rewrite this comment in ASCII, please? I would do it, but
>>> I'm not sure what the correct translation is.
>>>
>>
>> My first submitted patch says that
>> they are just Unicode "ZERO WIDTH SPACE".
>>
>> https://lore.kernel.org/kvm/20200508083218.120559-2-like.xu@linux.intel.com/
>>
>> Here you go:
>>
>> ---
>>
>>   From 1b058846aabcd7a85b5c5f41cb2b63b6a348bdc4 Mon Sep 17 00:00:00 2001
>> From: Like Xu <like.xu@linux.intel.com>
>> Date: Wed, 12 May 2021 14:26:40 +0800
>> Subject: [PATCH] x86: pmu: Fix a comment about full-width counter writes
>>    support
>>
>> Remove two Unicode characters 'ZERO WIDTH SPACE' (U+200B).
>>
>> Fixes: 22f2901a0e ("x86: pmu: Test full-width counter writes support")
>> Reported-by: Jim Mattson <jmattson@google.com>
>> Signed-off-by: Like Xu <like.xu@linux.intel.com>
>> ---
>>    x86/pmu.c | 2 +-
>>    1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/x86/pmu.c b/x86/pmu.c
>> index 5a3d55b..6cb3506 100644
>> --- a/x86/pmu.c
>> +++ b/x86/pmu.c
>> @@ -510,7 +510,7 @@ static void  check_gp_counters_write_width(void)
>>           }
>>
>>           /*
>> -        * MSR_IA32_PMCn supports writing values ​​up to GP
>> counter width,
>> +        * MSR_IA32_PMCn supports writing values up to GP counter width,
>>            * and only the lowest bits of GP counter width are valid.
>>            */
>>           for (i = 0; i < num_counters; i++) {
>> --
>> 2.31.1
> 
> Paolo:
> 
> Did this patch get overlooked? I'm still seeing the unicode characters
> in this comment.
> 

Hi Paolo, please help review this one as well:
https://lore.kernel.org/kvm/20211122115758.46504-1-likexu@tencent.com/


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

* Re: [kvm-unit-tests PATCH] x86: pmu: Test full-width counter writes support
  2022-01-08  0:06       ` Jim Mattson
  2022-01-10  6:34         ` Like Xu
@ 2022-03-11 16:36         ` Thomas Huth
  1 sibling, 0 replies; 19+ messages in thread
From: Thomas Huth @ 2022-03-11 16:36 UTC (permalink / raw)
  To: Jim Mattson, Paolo Bonzini; +Cc: kvm list, Andrew Jones, Like Xu

On 08/01/2022 01.06, Jim Mattson wrote:
> On Tue, May 11, 2021 at 11:33 PM Like Xu <like.xu@linux.intel.com> wrote:
>>
>> On 2021/5/12 5:27, Jim Mattson wrote:
>>> On Fri, May 29, 2020 at 12:44 AM Like Xu <like.xu@linux.intel.com> wrote:
>>>>
>>>> When the full-width writes capability is set, use the alternative MSR
>>>> range to write larger sign counter values (up to GP counter width).
>>>>
>>>> Signed-off-by: Like Xu <like.xu@linux.intel.com>
>>>> ---
>>>
>>>> +       /*
>>>> +        * MSR_IA32_PMCn supports writing values ​​up to GP counter width,
>>>> +        * and only the lowest bits of GP counter width are valid.
>>>> +        */
>>>
>>> Could you rewrite this comment in ASCII, please? I would do it, but
>>> I'm not sure what the correct translation is.
>>>
>>
>> My first submitted patch says that
>> they are just Unicode "ZERO WIDTH SPACE".
>>
>> https://lore.kernel.org/kvm/20200508083218.120559-2-like.xu@linux.intel.com/
>>
>> Here you go:
>>
>> ---
>>
>>   From 1b058846aabcd7a85b5c5f41cb2b63b6a348bdc4 Mon Sep 17 00:00:00 2001
>> From: Like Xu <like.xu@linux.intel.com>
>> Date: Wed, 12 May 2021 14:26:40 +0800
>> Subject: [PATCH] x86: pmu: Fix a comment about full-width counter writes
>>    support
>>
>> Remove two Unicode characters 'ZERO WIDTH SPACE' (U+200B).
>>
>> Fixes: 22f2901a0e ("x86: pmu: Test full-width counter writes support")
>> Reported-by: Jim Mattson <jmattson@google.com>
>> Signed-off-by: Like Xu <like.xu@linux.intel.com>
>> ---
>>    x86/pmu.c | 2 +-
>>    1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/x86/pmu.c b/x86/pmu.c
>> index 5a3d55b..6cb3506 100644
>> --- a/x86/pmu.c
>> +++ b/x86/pmu.c
>> @@ -510,7 +510,7 @@ static void  check_gp_counters_write_width(void)
>>           }
>>
>>           /*
>> -        * MSR_IA32_PMCn supports writing values ​​up to GP
>> counter width,
>> +        * MSR_IA32_PMCn supports writing values up to GP counter width,
>>            * and only the lowest bits of GP counter width are valid.
>>            */
>>           for (i = 0; i < num_counters; i++) {
>> --
>> 2.31.1
> 
> Paolo:
> 
> Did this patch get overlooked? I'm still seeing the unicode characters
> in this comment.

Yes, seems like it felt through the cracks. It's better to send patches as a 
new mail thread instead of posting them in a reply, otherwise they might be 
overlooked. Anyway, I've pushed this patch now to the repo.

  Thomas


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

* [kvm-unit-tests PATCH] x86: pmu: Test full-width counter writes support
  2020-05-08  8:32 [PATCH v3] KVM: x86/pmu: Support full width counting Like Xu
@ 2020-05-08  8:32 ` Like Xu
  0 siblings, 0 replies; 19+ messages in thread
From: Like Xu @ 2020-05-08  8:32 UTC (permalink / raw)
  To: Paolo Bonzini
  Cc: Sean Christopherson, Vitaly Kuznetsov, Wanpeng Li, Jim Mattson,
	Joerg Roedel, kvm, linux-kernel, Like Xu

When the full-width writes capability is set, use the alternative MSR
range to write larger sign counter values (up to GP counter width).

Signed-off-by: Like Xu <like.xu@linux.intel.com>
---
 lib/x86/msr.h |   1 +
 x86/pmu.c     | 125 ++++++++++++++++++++++++++++++++++++++++----------
 2 files changed, 102 insertions(+), 24 deletions(-)

diff --git a/lib/x86/msr.h b/lib/x86/msr.h
index 8dca964..6ef5502 100644
--- a/lib/x86/msr.h
+++ b/lib/x86/msr.h
@@ -35,6 +35,7 @@
 #define MSR_IA32_SPEC_CTRL              0x00000048
 #define MSR_IA32_PRED_CMD               0x00000049
 
+#define MSR_IA32_PMC0                  0x000004c1
 #define MSR_IA32_PERFCTR0		0x000000c1
 #define MSR_IA32_PERFCTR1		0x000000c2
 #define MSR_FSB_FREQ			0x000000cd
diff --git a/x86/pmu.c b/x86/pmu.c
index f45621a..8644f90 100644
--- a/x86/pmu.c
+++ b/x86/pmu.c
@@ -91,6 +91,9 @@ struct pmu_event {
 	{"fixed 3", MSR_CORE_PERF_FIXED_CTR0 + 2, 0.1*N, 30*N}
 };
 
+#define PMU_CAP_FW_WRITES	(1ULL << 13)
+static u64 gp_counter_base = MSR_IA32_PERFCTR0;
+
 static int num_counters;
 
 char *buf;
@@ -125,12 +128,13 @@ static bool check_irq(void)
 
 static bool is_gp(pmu_counter_t *evt)
 {
-	return evt->ctr < MSR_CORE_PERF_FIXED_CTR0;
+	return evt->ctr < MSR_CORE_PERF_FIXED_CTR0 ||
+		evt->ctr >= MSR_IA32_PMC0;
 }
 
 static int event_to_global_idx(pmu_counter_t *cnt)
 {
-	return cnt->ctr - (is_gp(cnt) ? MSR_IA32_PERFCTR0 :
+	return cnt->ctr - (is_gp(cnt) ? gp_counter_base :
 		(MSR_CORE_PERF_FIXED_CTR0 - FIXED_CNT_INDEX));
 }
 
@@ -226,7 +230,7 @@ static bool verify_counter(pmu_counter_t *cnt)
 static void check_gp_counter(struct pmu_event *evt)
 {
 	pmu_counter_t cnt = {
-		.ctr = MSR_IA32_PERFCTR0,
+		.ctr = gp_counter_base,
 		.config = EVNTSEL_OS | EVNTSEL_USR | evt->unit_sel,
 	};
 	int i;
@@ -276,7 +280,7 @@ static void check_counters_many(void)
 			continue;
 
 		cnt[n].count = 0;
-		cnt[n].ctr = MSR_IA32_PERFCTR0 + n;
+		cnt[n].ctr = gp_counter_base + n;
 		cnt[n].config = EVNTSEL_OS | EVNTSEL_USR |
 			gp_events[i % ARRAY_SIZE(gp_events)].unit_sel;
 		n++;
@@ -302,7 +306,7 @@ static void check_counter_overflow(void)
 	uint64_t count;
 	int i;
 	pmu_counter_t cnt = {
-		.ctr = MSR_IA32_PERFCTR0,
+		.ctr = gp_counter_base,
 		.config = EVNTSEL_OS | EVNTSEL_USR | gp_events[1].unit_sel /* instructions */,
 		.count = 0,
 	};
@@ -319,6 +323,8 @@ static void check_counter_overflow(void)
 		int idx;
 
 		cnt.count = 1 - count;
+		if (gp_counter_base == MSR_IA32_PMC0)
+			cnt.count &= (1ul << eax.split.bit_width) - 1;
 
 		if (i == num_counters) {
 			cnt.ctr = fixed_events[0].unit_sel;
@@ -346,7 +352,7 @@ static void check_counter_overflow(void)
 static void check_gp_counter_cmask(void)
 {
 	pmu_counter_t cnt = {
-		.ctr = MSR_IA32_PERFCTR0,
+		.ctr = gp_counter_base,
 		.config = EVNTSEL_OS | EVNTSEL_USR | gp_events[1].unit_sel /* instructions */,
 		.count = 0,
 	};
@@ -369,7 +375,7 @@ static void do_rdpmc_fast(void *ptr)
 
 static void check_rdpmc(void)
 {
-	uint64_t val = 0x1f3456789ull;
+	uint64_t val = 0xff0123456789ull;
 	bool exc;
 	int i;
 
@@ -378,20 +384,23 @@ static void check_rdpmc(void)
 	for (i = 0; i < num_counters; i++) {
 		uint64_t x;
 		pmu_counter_t cnt = {
-			.ctr = MSR_IA32_PERFCTR0 + i,
+			.ctr = gp_counter_base + i,
 			.idx = i
 		};
 
-		/*
-		 * Only the low 32 bits are writable, and the value is
-		 * sign-extended.
-		 */
-		x = (uint64_t)(int64_t)(int32_t)val;
+	        /*
+	         * Without full-width writes, only the low 32 bits are writable,
+	         * and the value is sign-extended.
+	         */
+		if (gp_counter_base == MSR_IA32_PERFCTR0)
+			x = (uint64_t)(int64_t)(int32_t)val;
+		else
+			x = (uint64_t)(int64_t)val;
 
 		/* Mask according to the number of supported bits */
 		x &= (1ull << eax.split.bit_width) - 1;
 
-		wrmsr(MSR_IA32_PERFCTR0 + i, val);
+		wrmsr(gp_counter_base + i, val);
 		report(rdpmc(i) == x, "cntr-%d", i);
 
 		exc = test_for_exception(GP_VECTOR, do_rdpmc_fast, &cnt);
@@ -423,8 +432,9 @@ static void check_rdpmc(void)
 static void check_running_counter_wrmsr(void)
 {
 	uint64_t status;
+	uint64_t count;
 	pmu_counter_t evt = {
-		.ctr = MSR_IA32_PERFCTR0,
+		.ctr = gp_counter_base,
 		.config = EVNTSEL_OS | EVNTSEL_USR | gp_events[1].unit_sel,
 		.count = 0,
 	};
@@ -433,7 +443,7 @@ static void check_running_counter_wrmsr(void)
 
 	start_event(&evt);
 	loop();
-	wrmsr(MSR_IA32_PERFCTR0, 0);
+	wrmsr(gp_counter_base, 0);
 	stop_event(&evt);
 	report(evt.count < gp_events[1].min, "cntr");
 
@@ -443,7 +453,13 @@ static void check_running_counter_wrmsr(void)
 
 	evt.count = 0;
 	start_event(&evt);
-	wrmsr(MSR_IA32_PERFCTR0, -1);
+
+	count = -1;
+	if (gp_counter_base == MSR_IA32_PMC0)
+		count &= (1ul << eax.split.bit_width) - 1;
+
+	wrmsr(gp_counter_base, count);
+
 	loop();
 	stop_event(&evt);
 	status = rdmsr(MSR_CORE_PERF_GLOBAL_STATUS);
@@ -452,6 +468,66 @@ static void check_running_counter_wrmsr(void)
 	report_prefix_pop();
 }
 
+static void check_counters(void)
+{
+	check_gp_counters();
+	check_fixed_counters();
+	check_rdpmc();
+	check_counters_many();
+	check_counter_overflow();
+	check_gp_counter_cmask();
+	check_running_counter_wrmsr();
+}
+
+static void do_unsupported_width_counter_write(void *index)
+{
+	wrmsr(MSR_IA32_PMC0 + *((int *) index), 0xffffff0123456789ull);
+}
+
+static void  check_gp_counters_write_width(void)
+{
+	u64 val_64 = 0xffffff0123456789ull;
+	u64 val_32 = val_64 & ((1ul << 32) - 1);
+	u64 val_max_width = val_64 & ((1ul << eax.split.bit_width) - 1);
+	int i;
+
+	/*
+	 * MSR_IA32_PERFCTRn supports 64-bit writes,
+	 * but only the lowest 32 bits are valid.
+	 */
+	for (i = 0; i < num_counters; i++) {
+		wrmsr(MSR_IA32_PERFCTR0 + i, val_32);
+		assert(rdmsr(MSR_IA32_PERFCTR0 + i) == val_32);
+		assert(rdmsr(MSR_IA32_PMC0 + i) == val_32);
+
+		wrmsr(MSR_IA32_PERFCTR0 + i, val_max_width);
+		assert(rdmsr(MSR_IA32_PERFCTR0 + i) == val_32);
+		assert(rdmsr(MSR_IA32_PMC0 + i) == val_32);
+
+		wrmsr(MSR_IA32_PERFCTR0 + i, val_64);
+		assert(rdmsr(MSR_IA32_PERFCTR0 + i) == val_32);
+		assert(rdmsr(MSR_IA32_PMC0 + i) == val_32);
+	}
+
+	/*
+	 * MSR_IA32_PMCn supports writing values ​​up to GP counter width,
+	 * and only the lowest bits of GP counter width are valid.
+	 */
+	for (i = 0; i < num_counters; i++) {
+		wrmsr(MSR_IA32_PMC0 + i, val_32);
+		assert(rdmsr(MSR_IA32_PMC0 + i) == val_32);
+		assert(rdmsr(MSR_IA32_PERFCTR0 + i) == val_32);
+
+		wrmsr(MSR_IA32_PMC0 + i, val_max_width);
+		assert(rdmsr(MSR_IA32_PMC0 + i) == val_max_width);
+		assert(rdmsr(MSR_IA32_PERFCTR0 + i) == val_max_width);
+
+		report(test_for_exception(GP_VECTOR,
+			do_unsupported_width_counter_write, &i),
+		"writing unsupported width to MSR_IA32_PMC%d raises #GP", i);
+	}
+}
+
 int main(int ac, char **av)
 {
 	struct cpuid id = cpuid(10);
@@ -480,13 +556,14 @@ int main(int ac, char **av)
 
 	apic_write(APIC_LVTPC, PC_VECTOR);
 
-	check_gp_counters();
-	check_fixed_counters();
-	check_rdpmc();
-	check_counters_many();
-	check_counter_overflow();
-	check_gp_counter_cmask();
-	check_running_counter_wrmsr();
+	check_counters();
+
+	if (rdmsr(MSR_IA32_PERF_CAPABILITIES) & PMU_CAP_FW_WRITES) {
+		gp_counter_base = MSR_IA32_PMC0;
+		report_prefix_push("full-width writes");
+		check_counters();
+		check_gp_counters_write_width();
+	}
 
 	return report_summary();
 }
-- 
2.21.1


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

end of thread, other threads:[~2022-03-11 16:37 UTC | newest]

Thread overview: 19+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-05-29  7:43 [PATCH RESEND] Enable full width counting for KVM: x86/pmu Like Xu
2020-05-29  7:43 ` [PATCH RESEND v4 1/2] KVM: x86/pmu: Tweak kvm_pmu_get_msr to pass 'struct msr_data' in Like Xu
2020-05-29  7:43 ` [PATCH RESEND v4 2/2] KVM: x86/pmu: Support full width counting Like Xu
2020-05-29  7:43 ` [kvm-unit-tests PATCH] x86: pmu: Test full-width counter writes support Like Xu
2020-06-16 10:49   ` Thomas Huth
2020-06-16 12:28     ` Paolo Bonzini
2020-06-19 19:46       ` Nadav Amit
2021-05-11 21:27   ` Jim Mattson
2021-05-12  6:33     ` Like Xu
2022-01-08  0:06       ` Jim Mattson
2022-01-10  6:34         ` Like Xu
2022-03-11 16:36         ` Thomas Huth
2020-05-29  7:43 ` [Qemu-devel PATCH] target/i386: define a new MSR based feature word - FEAT_PERF_CAPABILITIES Like Xu
2020-05-29  7:43   ` Like Xu
2020-05-29 10:23   ` Paolo Bonzini
2020-05-29 10:23     ` Paolo Bonzini
2020-05-29  8:47 ` [PATCH RESEND] Enable full width counting for KVM: x86/pmu Paolo Bonzini
2020-05-29  8:52   ` Xu, Like
  -- strict thread matches above, loose matches on Subject: below --
2020-05-08  8:32 [PATCH v3] KVM: x86/pmu: Support full width counting Like Xu
2020-05-08  8:32 ` [kvm-unit-tests PATCH] x86: pmu: Test full-width counter writes support Like Xu

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