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

[Yosry Ahmed]

On Tue, May 5, 2026 at 11:11 AM Sean Christopherson <seanjc@xxxxxxxxxx> wrote:
>
> On Fri, May 01, 2026, Yosry Ahmed wrote:
> > 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@xxxxxxxxxx> wrote:
> > > > > +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.
>
> Hmm.  I don't think either of those is the correct approach.  Unlike the MSR filter
> case, the H/G stuff is architectural.  I.e. KVM doesn't just need to disable the
> counter in hardware, KVM _always_ needs to treat the counter as disabled.

Yeah I agree both are not correct.

>
> So I think we actually want to handle this in pmc_is_locally_enabled(), because
> the host/guest bits are "local" controls.  One option would be to add the guest/host
> masks as constants in kvm_pmu_ops, and bleed the logic into pmc_is_locally_enabled(),
> e.g. to avoid the CALL+RET overhead.  But if make the callback a "negative", then
> we can make the static call OPTIONAL_RET0, which will turn the call into a glorified
> nop for everything except AMD with a mediated PMU.  E.g.
>
> diff --git arch/x86/kvm/pmu.h arch/x86/kvm/pmu.h
> index 0925246731cb..d8ce0938fcbe 100644
> --- arch/x86/kvm/pmu.h
> +++ arch/x86/kvm/pmu.h
> @@ -190,7 +190,8 @@ static inline bool pmc_is_locally_enabled(struct kvm_pmc *pmc)
>                                         pmc->idx - KVM_FIXED_PMC_BASE_IDX) &
>                                         (INTEL_FIXED_0_KERNEL | INTEL_FIXED_0_USER);
>
> -       return pmc->eventsel & ARCH_PERFMON_EVENTSEL_ENABLE;
> +       return (pmc->eventsel & ARCH_PERFMON_EVENTSEL_ENABLE) &&
> +              !kvm_pmu_call(pmc_is_locally_disabled(pmc));

We still get the overhead on AMD with mediated PMU enabled, but more
importantly, I am not sure what pmc_is_locally_disabled() would test
for here? Do we re-check EFER, guest mode, etc to figure it out? I
don't think this is what you mean as it would be redundant, but I am
not sure what else.

Did you see my other replies and code snippet tracking disabling
reasons? I think the code snippet would still work, I just need to
move the pmc_is_nested_disabled() check into pmc_is_locally_enabled().

>  }
>
>  extern struct x86_pmu_capability kvm_pmu_cap;
>