Re: [PATCH v6 3/3] KVM: selftests: Verify disabling PMU virtualization via KVM_CAP_CONFIG_PMU

[Sean Christopherson <seanjc@google.com>]

On Wed, Feb 09, 2022, David Dunn wrote:
> On a VM with PMU disabled via KVM_CAP_PMU_CONFIG, the PMU will not be
> usable by the guest.  On Intel, this causes a #GP.  And on AMD, the
> counters no longer increment.
> 
> KVM_CAP_PMU_CONFIG must be invoked on a VM prior to creating VCPUs.
> 
> Signed-off-by: David Dunn <daviddunn@google.com>
> ---
>  .../kvm/x86_64/pmu_event_filter_test.c        | 35 +++++++++++++++++++
>  1 file changed, 35 insertions(+)
> 
> diff --git a/tools/testing/selftests/kvm/x86_64/pmu_event_filter_test.c b/tools/testing/selftests/kvm/x86_64/pmu_event_filter_test.c
> index c715adcbd487..7a4b99684d9d 100644
> --- a/tools/testing/selftests/kvm/x86_64/pmu_event_filter_test.c
> +++ b/tools/testing/selftests/kvm/x86_64/pmu_event_filter_test.c
> @@ -325,6 +325,39 @@ static void test_not_member_allow_list(struct kvm_vm *vm)
>  	TEST_ASSERT(!count, "Disallowed PMU Event is counting");
>  }
>  
> +/*
> + * Verify KVM_CAP_PMU_DISABLE prevents the use of the PMU.
> + *
> + * Note that KVM_CAP_PMU_CAPABILITY must be invoked prior to creating VCPUs.
> + */
> +static void test_pmu_config_disable(void (*guest_code)(void))
> +{
> +	int r;
> +	struct kvm_vm *vm;
> +	struct kvm_enable_cap cap = { 0 };
> +	bool sane;
> +
> +	r = kvm_check_cap(KVM_CAP_PMU_CAPABILITY);
> +	if ((r & KVM_CAP_PMU_DISABLE) == 0)

Preferred style is

	if (!(r & KVM_CAP_PMU_DISABLE))
		return;

Bonus points if you a helper to allow retrieving module params, then this could
be:

	TEST_ASSERT(!!(r & KVM_CAP_PMU_DISABLE) == enable_pmu);

	if (!(r & KVM_CAP_PMU_DISABLE))
		return;

> +		return;
> +
> +	vm = vm_create_without_vcpus(VM_MODE_DEFAULT, DEFAULT_GUEST_PHY_PAGES);
> +
> +	cap.cap = KVM_CAP_PMU_CAPABILITY;
> +	cap.args[0] = KVM_CAP_PMU_DISABLE;
> +	r = vm_enable_cap(vm, &cap);
> +	TEST_ASSERT(r == 0, "Failed KVM_CAP_PMU_DISABLE.");
> +
> +	vm_vcpu_add_default(vm, VCPU_ID, guest_code);
> +	vm_init_descriptor_tables(vm);
> +	vcpu_init_descriptor_tables(vm, VCPU_ID);
> +
> +	sane = sanity_check_pmu(vm);
> +	TEST_ASSERT(!sane, "Guest should not be able to use disabled PMU.");

Using a local boolean loses context, e.g.

	TEST_ASSERT(!sanity_check_pmu(vm),
		    "...");

will show exactly what failed in the error messages, where as "!sane" doesn't
provide much help to the user.

> +
> +	kvm_vm_free(vm);
> +}
> +
>  /*
>   * Check for a non-zero PMU version, at least one general-purpose
>   * counter per logical processor, an EBX bit vector of length greater
> @@ -430,5 +463,7 @@ int main(int argc, char *argv[])
>  
>  	kvm_vm_free(vm);
>  
> +	test_pmu_config_disable(guest_code);
> +
>  	return 0;
>  }
> -- 
> 2.35.0.263.gb82422642f-goog
>