[PATCH RESEND] Enable full width counting for KVM: x86/pmu

[PATCH RESEND] Enable full width counting for KVM: x86/pmu

[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

[PATCH RESEND v4 1/2] KVM: x86/pmu: Tweak kvm_pmu_get_msr to pass 'struct msr_data' in

[Like Xu]

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

[PATCH RESEND v4 2/2] KVM: x86/pmu: Support full width counting

[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

[kvm-unit-tests PATCH] x86: pmu: Test full-width counter writes support

[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

Re: [kvm-unit-tests PATCH] x86: pmu: Test full-width counter writes support

[Thomas Huth]

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

Re: [kvm-unit-tests PATCH] x86: pmu: Test full-width counter writes support

[Paolo Bonzini]

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

Re: [kvm-unit-tests PATCH] x86: pmu: Test full-width counter writes support

[Nadav Amit]

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

Re: [kvm-unit-tests PATCH] x86: pmu: Test full-width counter writes support

[Jim Mattson]

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.

Re: [kvm-unit-tests PATCH] x86: pmu: Test full-width counter writes support

[Like Xu]

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

Re: [kvm-unit-tests PATCH] x86: pmu: Test full-width counter writes support

[Jim Mattson]

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.

Re: [kvm-unit-tests PATCH] x86: pmu: Test full-width counter writes support

[Like Xu]

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/

Re: [kvm-unit-tests PATCH] x86: pmu: Test full-width counter writes support

[Thomas Huth]

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

[Qemu-devel PATCH] target/i386: define a new MSR based feature word - FEAT_PERF_CAPABILITIES

[Like Xu]

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

Re: [Qemu-devel PATCH] target/i386: define a new MSR based feature word - FEAT_PERF_CAPABILITIES

[Paolo Bonzini]

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

Re: [PATCH RESEND] Enable full width counting for KVM: x86/pmu

[Paolo Bonzini]

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

Re: [PATCH RESEND] Enable full width counting for KVM: x86/pmu

[Xu, Like]

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
>