Re: [PATCH v5 07/13] KVM: x86/pmu: Disable counters based on Host-Only/Guest-Only bits in SVM

Yosry Ahmed
2026-05-01T17:50:19Z

On Thu, Apr 30, 2026 at 08:34:59PM -0700, Yosry Ahmed wrote:
> On Thu, Apr 30, 2026 at 4:24 PM Yosry Ahmed <yosry@kernel.org> wrote:
> >
> > > diff --git a/arch/x86/kvm/pmu.h b/arch/x86/kvm/pmu.h
> > > index 0e99022168a85..0c372b9f8ed34 100644
> > > --- a/arch/x86/kvm/pmu.h
> > > +++ b/arch/x86/kvm/pmu.h
> > > @@ -36,6 +36,7 @@ struct kvm_pmu_ops {
> > > void (*reset)(struct kvm_vcpu *vcpu);
> > > void (*deliver_pmi)(struct kvm_vcpu *vcpu);
> > > void (*cleanup)(struct kvm_vcpu *vcpu);
> > > + void (*reprogram_counters)(struct kvm_vcpu *vcpu, u64 counters);
> > >
> > > bool (*is_mediated_pmu_supported)(struct x86_pmu_capability *host_pmu);
> > > void (*mediated_load)(struct kvm_vcpu *vcpu);
> > > diff --git a/arch/x86/kvm/svm/pmu.c b/arch/x86/kvm/svm/pmu.c
> > > index 7aa298eeb0721..fe6f2bb79ab83 100644
> > > --- a/arch/x86/kvm/svm/pmu.c
> > > +++ b/arch/x86/kvm/svm/pmu.c
> > > @@ -260,6 +260,48 @@ static void amd_mediated_pmu_put(struct kvm_vcpu *vcpu)
> > > wrmsrq(MSR_AMD64_PERF_CNTR_GLOBAL_STATUS_CLR, pmu->global_status);
> > > }
> > >
> > > +static void amd_mediated_pmu_handle_host_guest_bits(struct kvm_vcpu *vcpu,
> > > + struct kvm_pmc *pmc)
> > > +{
> > > + u64 host_guest_bits;
> > > +
> > > + if (!(pmc->eventsel & ARCH_PERFMON_EVENTSEL_ENABLE))
> > > + return;
> > > +
> > > + /* Count all events if both bits are cleared */
> > > + host_guest_bits = pmc->eventsel & AMD64_EVENTSEL_HOST_GUEST_MASK;
> > > + if (!host_guest_bits)
> > > + return;
> > > +
> > > + /*
> > > + * If EFER.SVME is set, the counter is disabledd if only one of the bits
> > > + * is set and it doesn't match the vCPU context. If EFER.SVME is
> > > + * cleared, the counter is disable if any of the bits is set.
> > > + */
> > > + if (vcpu->arch.efer & EFER_SVME) {
> > > + if (host_guest_bits == AMD64_EVENTSEL_HOST_GUEST_MASK)
> > > + return;
> > > +
> > > + if (!!(host_guest_bits & AMD64_EVENTSEL_GUESTONLY) == is_guest_mode(vcpu))
> > > + return;
> > > + }
> > > +
> > > + pmc->eventsel_hw &= ~ARCH_PERFMON_EVENTSEL_ENABLE;
> >
> > Sashiko raised a good point here. In the following patch, we reprogram
> > the counters synchronously on nested transitions to update the
> > counters' enablement before counting VMRUN or WRMSR(EFER.SVME).
> > However, this updates pmc->eventsel_hw while
> > kvm_pmu_recalc_pmc_emulation() checks pmc->eventsel to check if the
> > counter is enabled.
> >
> > I think either pmc_is_locally_enabled() needs to check
> > pmc->eventsel_hw or we need to update pmc->eventsel here.
> >
> > AFAICT, pmc->eventself has the value written to the MSR, so I think we
> > want to keep that as-is.
> >
> > On the other hand, ARCH_PERFMON_EVENTSEL_ENABLE is cleared in
> > pmc->eventsel_hw in kvm_mediated_pmu_refresh_event_filter() if the
> > event is not allowed, and kvm_pmu_recalc_pmc_emulation() has a comment
> > about intentionally ignoring event filters.
> >
> > We can also separately track enablement for nested purposes, but that
> > would add one more thing we need to check aside from general counter
> > enablement and event filtering.
> >
> > None of these options are ideal, perhaps directly clearing the bit in
> > pmc->eventsel would do the least damage as (pmc->eventsel &
> > ARCH_PERFMON_EVENTSEL_ENABLE) is only checked by
> > pmc_is_locally_enabled().
>
> No, we can't really clear the bit in pmc->eventsel as that's what the
> guest reads with RDMSR.
>
> The more I think about this, the more I think we should drop
> pmc->eventsel_hw. It serves two purposes AFAICT:
> 1. On AMD, we use it to clear HOSTONLY and set GUESTONLY in actual HW.
> 2. For event filtering, we use it to clear EVENTSEL_ENABLE.
>
> But maybe it's easier to explicitly track the changes we need to apply
> to eventsel rather than a HW version?
>
> (1) is trivial, we can just clear HOSTONLY and set GUESTONLY before
> doing the actual write as that's constant.
>
> For (2), instead of eventsel_hw, maybe add a boolean called 'filtered'
> to track if that PMC should be filtered out or not. Then, we can add
> another boolean to track if the counter is disabled due to
> nested/HG-bits (e.g. 'nested_disabled').
>
> With that, pmc_is_locally_enabled() would check:
> (pmc->eventsel & ARCH_PERFMON_EVENTSEL_ENABLE) && !pmc->nested_disabled
>
> , and then before writing to HW we check pmc->filtered,
> pmc->nested_disabled, as well as do the HOSTONLY/GUESTONLY changes for
> AMD.
>
> Actually, instead of a boolean, maybe add 'disabled_reasons' flags,
> with possible flags like PMC_DISABLED_FILTER and PMC_DISABLED_NESTED.
>
> This might all be unclear, I will draft some diff on top of the series
> tomorrow and send it in case it makes things clearer.

This is what I had in mind essentially:

diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
index b191967c9c1e4..9c38c47581e75 100644
--- a/arch/x86/include/asm/kvm_host.h
+++ b/arch/x86/include/asm/kvm_host.h
@@ -515,11 +515,21 @@ enum pmc_type {
KVM_PMC_FIXED,
};

+#define KVM_PMC_DISABLE_FILTERED BIT(0)
+#define KVM_PMC_DISABLE_NESTED BIT(1)
+
struct kvm_pmc {
enum pmc_type type;
u8 idx;
bool is_paused;
bool intr;
+
+ /*
+ * Reasons why the PMC should be disabled in eventsel when written to HW
+ * with the mediated vPMU.
+ */
+ u8 eventsel_hw_disable_reasons;
+
/*
* Base value of the PMC counter, relative to the *consumed* count in
* the associated perf_event. This value includes counter updates from
@@ -538,7 +548,6 @@ struct kvm_pmc {
*/
u64 emulated_counter;
u64 eventsel;
- u64 eventsel_hw;
struct perf_event *perf_event;
struct kvm_vcpu *vcpu;
/*
diff --git a/arch/x86/kvm/pmu.c b/arch/x86/kvm/pmu.c
index 7b2b4ce6bdad9..5128858610d83 100644
--- a/arch/x86/kvm/pmu.c
+++ b/arch/x86/kvm/pmu.c
@@ -536,10 +536,9 @@ static void kvm_mediated_pmu_refresh_event_filter(struct kvm_pmc *pmc)
struct kvm_pmu *pmu = pmc_to_pmu(pmc);

if (pmc_is_gp(pmc)) {
- pmc->eventsel_hw &= ~ARCH_PERFMON_EVENTSEL_ENABLE;
- if (allowed)
- pmc->eventsel_hw |= pmc->eventsel &
- ARCH_PERFMON_EVENTSEL_ENABLE;
+ pmc->eventsel_hw_disable_reasons &= ~KVM_PMC_DISABLE_FILTERED;
+ if (!allowed)
+ pmc->eventsel_hw_disable_reasons |= KVM_PMC_DISABLE_FILTERED;
} else {
u64 mask = intel_fixed_bits_by_idx(pmc->idx - KVM_FIXED_PMC_BASE_IDX, 0xf);

@@ -630,7 +629,7 @@ void kvm_pmu_recalc_pmc_emulation(struct kvm_pmu *pmu, struct kvm_pmc *pmc)
* omitting a PMC from a bitmap could result in a missed event if the
* filter is changed to allow counting the event.
*/
- if (!pmc_is_locally_enabled(pmc))
+ if (!pmc_is_locally_enabled(pmc) || pmc_is_nested_disabled(pmc))
return;

if (pmc_is_event_match(pmc, kvm_pmu_eventsel.INSTRUCTIONS_RETIRED))
@@ -944,7 +943,7 @@ static void kvm_pmu_reset(struct kvm_vcpu *vcpu)

if (pmc_is_gp(pmc)) {
pmc->eventsel = 0;
- pmc->eventsel_hw = 0;
+ pmc->eventsel_hw_disable_reasons = 0;
}
}

@@ -1313,6 +1312,19 @@ static __always_inline u32 gp_eventsel_msr(u32 idx)
return kvm_pmu_ops.GP_EVENTSEL_BASE + idx * kvm_pmu_ops.MSR_STRIDE;
}

+static __always_inline u64 gp_calc_eventsel_hw(struct kvm_pmc *pmc)
+{
+ u64 eventsel = pmc->eventsel;
+
+ if (pmc->eventsel_hw_disable_reasons)
+ eventsel &= ~ARCH_PERFMON_EVENTSEL_ENABLE;
+
+ eventsel |= kvm_pmu_ops.GP_EVENTSEL_ALWAYS_SET;
+ eventsel &= ~kvm_pmu_ops.GP_EVENTSEL_ALWAYS_CLR;
+
+ return eventsel;
+}
+
static void kvm_pmu_load_guest_pmcs(struct kvm_vcpu *vcpu)
{
struct kvm_pmu *pmu = vcpu_to_pmu(vcpu);
@@ -1329,7 +1341,7 @@ static void kvm_pmu_load_guest_pmcs(struct kvm_vcpu *vcpu)

if (pmc->counter != rdpmc(i))
wrmsrl(gp_counter_msr(i), pmc->counter);
- wrmsrl(gp_eventsel_msr(i), pmc->eventsel_hw);
+ wrmsrl(gp_eventsel_msr(i), gp_calc_eventsel_hw(pmc));
}
for (i = 0; i < pmu->nr_arch_fixed_counters; i++) {
pmc = &pmu->fixed_counters[i];
@@ -1387,7 +1399,7 @@ static void kvm_pmu_put_guest_pmcs(struct kvm_vcpu *vcpu)
pmc->counter = rdpmc(i);
if (pmc->counter)
wrmsrq(gp_counter_msr(i), 0);
- if (pmc->eventsel_hw)
+ if (gp_calc_eventsel_hw(pmc))
wrmsrq(gp_eventsel_msr(i), 0);
}

diff --git a/arch/x86/kvm/pmu.h b/arch/x86/kvm/pmu.h
index 4a9148cf779df..fcfce6a213aef 100644
--- a/arch/x86/kvm/pmu.h
+++ b/arch/x86/kvm/pmu.h
@@ -52,6 +52,9 @@ struct kvm_pmu_ops {
const u32 GP_COUNTER_BASE;
const u32 FIXED_COUNTER_BASE;
const u32 MSR_STRIDE;
+
+ const u64 GP_EVENTSEL_ALWAYS_SET;
+ const u64 GP_EVENTSEL_ALWAYS_CLR;
};

extern bool enable_pmu;
@@ -197,6 +200,11 @@ static inline bool pmc_is_locally_enabled(struct kvm_pmc *pmc)
return pmc->eventsel & ARCH_PERFMON_EVENTSEL_ENABLE;
}

+static inline bool pmc_is_nested_disabled(struct kvm_pmc *pmc)
+{
+ return pmc->eventsel_hw_disable_reasons & KVM_PMC_DISABLE_NESTED;
+}
+
extern struct x86_pmu_capability kvm_pmu_cap;

void kvm_init_pmu_capability(struct kvm_pmu_ops *pmu_ops);
diff --git a/arch/x86/kvm/svm/pmu.c b/arch/x86/kvm/svm/pmu.c
index b12c35b4fccbf..2ac00e729d04b 100644
--- a/arch/x86/kvm/svm/pmu.c
+++ b/arch/x86/kvm/svm/pmu.c
@@ -166,8 +166,6 @@ static int amd_pmu_set_msr(struct kvm_vcpu *vcpu, struct msr_data *msr_info)
data &= ~pmu->reserved_bits;
if (data != pmc->eventsel) {
pmc->eventsel = data;
- pmc->eventsel_hw = (data & ~AMD64_EVENTSEL_HOSTONLY) |
- AMD64_EVENTSEL_GUESTONLY;
kvm_pmu_request_counter_reprogram(pmc);
}
return 0;
@@ -270,6 +268,8 @@ static void amd_mediated_pmu_handle_host_guest_bits(struct kvm_vcpu *vcpu,
struct vcpu_svm *svm = to_svm(vcpu);
u64 host_guest_bits;

+ pmc->eventsel_hw_disable_reasons &= ~KVM_PMC_DISABLE_NESTED;
+
if (!(pmc->eventsel & ARCH_PERFMON_EVENTSEL_ENABLE))
return;

@@ -296,7 +296,7 @@ static void amd_mediated_pmu_handle_host_guest_bits(struct kvm_vcpu *vcpu,
return;
}

- pmc->eventsel_hw &= ~ARCH_PERFMON_EVENTSEL_ENABLE;
+ pmc->eventsel_hw_disable_reasons |= KVM_PMC_DISABLE_NESTED;
}

static void amd_pmu_reprogram_counters(struct kvm_vcpu *vcpu, u64 counters)
@@ -336,4 +336,7 @@ struct kvm_pmu_ops amd_pmu_ops __initdata = {
.GP_COUNTER_BASE = MSR_F15H_PERF_CTR0,
.FIXED_COUNTER_BASE = 0,
.MSR_STRIDE = 2,
+
+ .GP_EVENTSEL_ALWAYS_SET = AMD64_EVENTSEL_GUESTONLY,
+ .GP_EVENTSEL_ALWAYS_CLR = AMD64_EVENTSEL_HOSTONLY,
};
diff --git a/arch/x86/kvm/vmx/pmu_intel.c b/arch/x86/kvm/vmx/pmu_intel.c
index 9bd77843d8da2..05b68f40f189e 100644
--- a/arch/x86/kvm/vmx/pmu_intel.c
+++ b/arch/x86/kvm/vmx/pmu_intel.c
@@ -431,7 +431,6 @@ static int intel_pmu_set_msr(struct kvm_vcpu *vcpu, struct msr_data *msr_info)

if (data != pmc->eventsel) {
pmc->eventsel = data;
- pmc->eventsel_hw = data;
kvm_pmu_request_counter_reprogram(pmc);
}
break;