Re: [PATCH 1/2] KVM: x86: Update vPMCs when retiring instructions

From: Like Xu
Date: Thu Nov 18 2021 - 06:28:58 EST

On 18/11/2021 11:37 am, Jim Mattson wrote:
On Wed, Nov 17, 2021 at 12:01 PM Jim Mattson <jmattson@xxxxxxxxxx> wrote:

On Tue, Nov 16, 2021 at 7:22 PM Like Xu <like.xu.linux@xxxxxxxxx> wrote:

On 17/11/2021 6:15 am, Jim Mattson wrote:
On Tue, Nov 16, 2021 at 4:44 AM Like Xu <like.xu.linux@xxxxxxxxx> wrote:

Hi Jim,

On 13/11/2021 7:52 am, Jim Mattson wrote:
When KVM retires a guest instruction through emulation, increment any
vPMCs that are configured to monitor "instructions retired," and
update the sample period of those counters so that they will overflow
at the right time.

Signed-off-by: Eric Hankland <ehankland@xxxxxxxxxx>
- Split the code to increment "branch instructions retired" into a
separate commit.
- Added 'static' to kvm_pmu_incr_counter() definition.
- Modified kvm_pmu_incr_counter() to check pmc->perf_event->state ==
Signed-off-by: Jim Mattson <jmattson@xxxxxxxxxx>
Fixes: f5132b01386b ("KVM: Expose a version 2 architectural PMU to a guests")
arch/x86/kvm/pmu.c | 31 +++++++++++++++++++++++++++++++
arch/x86/kvm/pmu.h | 1 +
arch/x86/kvm/x86.c | 3 +++
3 files changed, 35 insertions(+)

diff --git a/arch/x86/kvm/pmu.c b/arch/x86/kvm/pmu.c
index 09873f6488f7..153c488032a5 100644
--- a/arch/x86/kvm/pmu.c
+++ b/arch/x86/kvm/pmu.c
@@ -490,6 +490,37 @@ void kvm_pmu_destroy(struct kvm_vcpu *vcpu)

+static void kvm_pmu_incr_counter(struct kvm_pmc *pmc, u64 evt)
+ u64 counter_value, sample_period;
+ if (pmc->perf_event &&

We need to incr pmc->counter whether it has a perf_event or not.

+ pmc->perf_event->attr.type == PERF_TYPE_HARDWARE &&

We need to cover PERF_TYPE_RAW as well, for example,
it has the basic bits for "{ 0xc0, 0x00, PERF_COUNT_HW_INSTRUCTIONS },"

We just need to focus on checking the select and umask bits:

[What follows applies only to Intel CPUs. I haven't looked at AMD's
PMU implementation yet.]

x86 has the same bit definition and semantics on at least the select and umask bits.

Yes, but AMD supports 12 bits of event selector. AMD also has the
HG_ONLY bits, which affect whether or not to count the event based on

It looks like we already have an issue with event selector truncation
on the AMD side. It's not clear from the APM if AMD has always had a
12-bit event selector field, but it's 12 bits now. Milan, for example,
has at least 6 different events with selectors > 255. I don't see how
a guest could monitor those events with the existing KVM

Yes and I have reproduced the issue on a Milan.
Thanks for your input, and let me try to fix it.

Like Xu