Commit f1c5651f authored by Sean Christopherson's avatar Sean Christopherson Committed by Paolo Bonzini
Browse files

KVM: x86/pmu: Force reprogramming of all counters on PMU filter change 



Force vCPUs to reprogram all counters on a PMU filter change to provide
a sane ABI for userspace.  Use the existing KVM_REQ_PMU to do the
programming, and take advantage of the fact that the reprogram_pmi bitmap
fits in a u64 to set all bits in a single atomic update.  Note, setting
the bitmap and making the request needs to be done _after_ the SRCU
synchronization to ensure that vCPUs will reprogram using the new filter.

KVM's current "lazy" approach is confusing and non-deterministic.  It's
confusing because, from a developer perspective, the code is buggy as it
makes zero sense to let userspace modify the filter but then not actually
enforce the new filter.  The lazy approach is non-deterministic because
KVM enforces the filter whenever a counter is reprogrammed, not just on
guest WRMSRs, i.e. a guest might gain/lose access to an event at random
times depending on what is going on in the host.

Note, the resulting behavior is still non-determinstic while the filter
is in flux.  If userspace wants to guarantee deterministic behavior, all
vCPUs should be paused during the filter update.

Jim Mattson <jmattson@google.com>

Fixes: 66bb8a06 ("KVM: x86: PMU Event Filter")
Cc: Aaron Lewis <aaronlewis@google.com>
Signed-off-by: default avatarSean Christopherson <seanjc@google.com>
Message-Id: <20220923001355.3741194-2-seanjc@google.com>
Signed-off-by: default avatarPaolo Bonzini <pbonzini@redhat.com>
parent 3a056757

arch/x86/include/asm/kvm_host.h

+10 −1
Original line number Diff line number Diff line
@@ -527,7 +527,16 @@ struct kvm_pmu { 
	struct kvm_pmc gp_counters[KVM_INTEL_PMC_MAX_GENERIC];

	struct kvm_pmc fixed_counters[KVM_PMC_MAX_FIXED];

	struct irq_work irq_work;



	/*

	 * Overlay the bitmap with a 64-bit atomic so that all bits can be

	 * set in a single access, e.g. to reprogram all counters when the PMU

	 * filter changes.

	 */

	union {

		DECLARE_BITMAP(reprogram_pmi, X86_PMC_IDX_MAX);

		atomic64_t __reprogram_pmi;

	};

	DECLARE_BITMAP(all_valid_pmc_idx, X86_PMC_IDX_MAX);

	DECLARE_BITMAP(pmc_in_use, X86_PMC_IDX_MAX);

arch/x86/kvm/pmu.c

+12 −1
Original line number Diff line number Diff line
@@ -577,6 +577,8 @@ EXPORT_SYMBOL_GPL(kvm_pmu_trigger_event); 
int kvm_vm_ioctl_set_pmu_event_filter(struct kvm *kvm, void __user *argp)

{

	struct kvm_pmu_event_filter tmp, *filter;

	struct kvm_vcpu *vcpu;

	unsigned long i;

	size_t size;

	int r;
@@ -613,9 +615,18 @@ int kvm_vm_ioctl_set_pmu_event_filter(struct kvm *kvm, void __user *argp) 
	mutex_lock(&kvm->lock);

	filter = rcu_replace_pointer(kvm->arch.pmu_event_filter, filter,

				     mutex_is_locked(&kvm->lock));

	synchronize_srcu_expedited(&kvm->srcu);



	BUILD_BUG_ON(sizeof(((struct kvm_pmu *)0)->reprogram_pmi) >

		     sizeof(((struct kvm_pmu *)0)->__reprogram_pmi));



	kvm_for_each_vcpu(i, vcpu, kvm)

		atomic64_set(&vcpu_to_pmu(vcpu)->__reprogram_pmi, -1ull);



	kvm_make_all_cpus_request(kvm, KVM_REQ_PMU);



	mutex_unlock(&kvm->lock);



	synchronize_srcu_expedited(&kvm->srcu);

	r = 0;

cleanup:

	kfree(filter);