[PATCH v6 0/3] KVM: x86: Provide per VM capability for disabling PMU virtualization

[PATCH v6 0/3] KVM: x86: Provide per VM capability for disabling PMU virtualization

[David Dunn]

This patch set allows usermode to disable PMU virtualization on
individual x86 VMs.  When disabled, the PMU is not advertised to
or accessible from the guest.

v5: https://lore.kernel.org/kvm/20220123184541.993212-1-daviddunn@google.com/

v5 -> v6
 * resolve minor conflicts that were queued after v5 was reviewed

v4 -> v5
 * Remove automatic CPUID adjustment when PMU disabled [Like]
 * Update documentation and changelog to reflect above.
 * Update documentation to document arg[0] and return values.  [Like].

David Dunn (3):
  KVM: x86: Provide per VM capability for disabling PMU virtualization
  KVM: selftests: Allow creation of selftest VM without vcpus
  KVM: selftests: Verify disabling PMU virtualization via
    KVM_CAP_CONFIG_PMU

 Documentation/virt/kvm/api.rst                | 22 ++++++++++++
 arch/x86/include/asm/kvm_host.h               |  1 +
 arch/x86/kvm/svm/pmu.c                        |  2 +-
 arch/x86/kvm/vmx/pmu_intel.c                  |  2 +-
 arch/x86/kvm/x86.c                            | 12 +++++++
 include/uapi/linux/kvm.h                      |  4 +++
 tools/include/uapi/linux/kvm.h                |  4 +++
 .../selftests/kvm/include/kvm_util_base.h     |  3 ++
 tools/testing/selftests/kvm/lib/kvm_util.c    | 35 +++++++++++++++----
 .../kvm/x86_64/pmu_event_filter_test.c        | 35 +++++++++++++++++++
 10 files changed, 112 insertions(+), 8 deletions(-)

-- 
2.35.0.263.gb82422642f-goog

[PATCH v6 1/3] KVM: x86: Provide per VM capability for disabling PMU virtualization

[David Dunn]

KVM_CAP_PMU_DISABLE is used to disable PMU virtualization on individual
x86 VMs.  PMU configuration must be done prior to creating VCPUs.

To enable future extension, KVM_CAP_PMU_CAPABILITY reports available
settings via bitmask when queried via check_extension.

For VMs that have PMU virtualization disabled, usermode will need to
clear CPUID leaf 0xA to notify guests.

Signed-off-by: David Dunn <daviddunn@google.com>
---
 Documentation/virt/kvm/api.rst  | 22 ++++++++++++++++++++++
 arch/x86/include/asm/kvm_host.h |  1 +
 arch/x86/kvm/svm/pmu.c          |  2 +-
 arch/x86/kvm/vmx/pmu_intel.c    |  2 +-
 arch/x86/kvm/x86.c              | 12 ++++++++++++
 include/uapi/linux/kvm.h        |  4 ++++
 tools/include/uapi/linux/kvm.h  |  4 ++++
 7 files changed, 45 insertions(+), 2 deletions(-)

diff --git a/Documentation/virt/kvm/api.rst b/Documentation/virt/kvm/api.rst
index a4267104db50..df836965b347 100644
--- a/Documentation/virt/kvm/api.rst
+++ b/Documentation/virt/kvm/api.rst
@@ -7561,3 +7561,25 @@ The argument to KVM_ENABLE_CAP is also a bitmask, and must be a subset
 of the result of KVM_CHECK_EXTENSION.  KVM will forward to userspace
 the hypercalls whose corresponding bit is in the argument, and return
 ENOSYS for the others.
+
+8.35 KVM_CAP_PMU_CAPABILITY
+---------------------------
+
+:Capability KVM_CAP_PMU_CAPABILITY
+:Architectures: x86
+:Type: vm
+:Parameters: arg[0] is bitmask of PMU virtualization capabilities.
+:Returns 0 on success, -EINVAL when arg[0] contains invalid bits
+
+This capability alters PMU virtualization in KVM.
+
+Calling KVM_CHECK_EXTENSION for this capability returns a bitmask of
+PMU virtualization capabilities that can be adjusted on a VM.
+
+The argument to KVM_ENABLE_CAP is also a bitmask and selects specific
+PMU virtualization capabilities to be applied to the VM.  This can
+only be invoked on a VM prior to the creation of VCPUs.
+
+At this time, KVM_CAP_PMU_DISABLE is the only capability.  Setting
+this capability will disable PMU virtualization for that VM.  Usermode
+should adjust CPUID leaf 0xA to reflect that the PMU is disabled.
diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
index c371ee7e45f7..f832cd0f9b27 100644
--- a/arch/x86/include/asm/kvm_host.h
+++ b/arch/x86/include/asm/kvm_host.h
@@ -1232,6 +1232,7 @@ struct kvm_arch {
 	hpa_t	hv_root_tdp;
 	spinlock_t hv_root_tdp_lock;
 #endif
+	bool enable_pmu;
 };
 
 struct kvm_vm_stat {
diff --git a/arch/x86/kvm/svm/pmu.c b/arch/x86/kvm/svm/pmu.c
index 5aa45f13b16d..d4de52409335 100644
--- a/arch/x86/kvm/svm/pmu.c
+++ b/arch/x86/kvm/svm/pmu.c
@@ -101,7 +101,7 @@ static inline struct kvm_pmc *get_gp_pmc_amd(struct kvm_pmu *pmu, u32 msr,
 {
 	struct kvm_vcpu *vcpu = pmu_to_vcpu(pmu);
 
-	if (!enable_pmu)
+	if (!vcpu->kvm->arch.enable_pmu)
 		return NULL;
 
 	switch (msr) {
diff --git a/arch/x86/kvm/vmx/pmu_intel.c b/arch/x86/kvm/vmx/pmu_intel.c
index 03fab48b149c..4e5b1eeeb77c 100644
--- a/arch/x86/kvm/vmx/pmu_intel.c
+++ b/arch/x86/kvm/vmx/pmu_intel.c
@@ -487,7 +487,7 @@ static void intel_pmu_refresh(struct kvm_vcpu *vcpu)
 	pmu->reserved_bits = 0xffffffff00200000ull;
 
 	entry = kvm_find_cpuid_entry(vcpu, 0xa, 0);
-	if (!entry || !enable_pmu)
+	if (!entry || !vcpu->kvm->arch.enable_pmu)
 		return;
 	eax.full = entry->eax;
 	edx.full = entry->edx;
diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index 6f69f3e3635e..c35a6a193bf4 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -4329,6 +4329,9 @@ int kvm_vm_ioctl_check_extension(struct kvm *kvm, long ext)
 		if (r < sizeof(struct kvm_xsave))
 			r = sizeof(struct kvm_xsave);
 		break;
+	case KVM_CAP_PMU_CAPABILITY:
+		r = enable_pmu ? KVM_CAP_PMU_MASK : 0;
+		break;
 	}
 	default:
 		break;
@@ -6003,6 +6006,14 @@ int kvm_vm_ioctl_enable_cap(struct kvm *kvm,
 		kvm->arch.exit_on_emulation_error = cap->args[0];
 		r = 0;
 		break;
+	case KVM_CAP_PMU_CAPABILITY:
+		r = -EINVAL;
+		if (!enable_pmu || kvm->created_vcpus > 0 ||
+		    cap->args[0] & ~KVM_CAP_PMU_MASK)
+			break;
+		kvm->arch.enable_pmu = !(cap->args[0] & KVM_CAP_PMU_DISABLE);
+		r = 0;
+		break;
 	default:
 		r = -EINVAL;
 		break;
@@ -11630,6 +11641,7 @@ int kvm_arch_init_vm(struct kvm *kvm, unsigned long type)
 	raw_spin_unlock_irqrestore(&kvm->arch.tsc_write_lock, flags);
 
 	kvm->arch.guest_can_read_msr_platform_info = true;
+	kvm->arch.enable_pmu = enable_pmu;
 
 #if IS_ENABLED(CONFIG_HYPERV)
 	spin_lock_init(&kvm->arch.hv_root_tdp_lock);
diff --git a/include/uapi/linux/kvm.h b/include/uapi/linux/kvm.h
index 5191b57e1562..cf6774cc18ef 100644
--- a/include/uapi/linux/kvm.h
+++ b/include/uapi/linux/kvm.h
@@ -1134,6 +1134,7 @@ struct kvm_ppc_resize_hpt {
 #define KVM_CAP_VM_GPA_BITS 207
 #define KVM_CAP_XSAVE2 208
 #define KVM_CAP_SYS_ATTRIBUTES 209
+#define KVM_CAP_PMU_CAPABILITY 210
 
 #ifdef KVM_CAP_IRQ_ROUTING
 
@@ -1970,6 +1971,9 @@ struct kvm_dirty_gfn {
 #define KVM_BUS_LOCK_DETECTION_OFF             (1 << 0)
 #define KVM_BUS_LOCK_DETECTION_EXIT            (1 << 1)
 
+#define KVM_CAP_PMU_DISABLE                    (1 << 0)
+#define KVM_CAP_PMU_MASK                       (KVM_CAP_PMU_DISABLE)
+
 /**
  * struct kvm_stats_header - Header of per vm/vcpu binary statistics data.
  * @flags: Some extra information for header, always 0 for now.
diff --git a/tools/include/uapi/linux/kvm.h b/tools/include/uapi/linux/kvm.h
index b46bcdb0cab1..00fb392e1742 100644
--- a/tools/include/uapi/linux/kvm.h
+++ b/tools/include/uapi/linux/kvm.h
@@ -1134,6 +1134,7 @@ struct kvm_ppc_resize_hpt {
 #define KVM_CAP_VM_GPA_BITS 207
 #define KVM_CAP_XSAVE2 208
 #define KVM_CAP_SYS_ATTRIBUTES 209
+#define KVM_CAP_PMU_CAPABILITY 210
 
 #ifdef KVM_CAP_IRQ_ROUTING
 
@@ -1973,6 +1974,9 @@ struct kvm_dirty_gfn {
 #define KVM_BUS_LOCK_DETECTION_OFF             (1 << 0)
 #define KVM_BUS_LOCK_DETECTION_EXIT            (1 << 1)
 
+#define KVM_CAP_PMU_DISABLE                    (1 << 0)
+#define KVM_CAP_PMU_MASK                       (KVM_PMU_CONFIG_DISABLE)
+
 /**
  * struct kvm_stats_header - Header of per vm/vcpu binary statistics data.
  * @flags: Some extra information for header, always 0 for now.
-- 
2.35.0.263.gb82422642f-goog

Re: [PATCH v6 1/3] KVM: x86: Provide per VM capability for disabling PMU virtualization

[Sean Christopherson]

On Wed, Feb 09, 2022, David Dunn wrote:
> KVM_CAP_PMU_DISABLE is used to disable PMU virtualization on individual
> x86 VMs.  PMU configuration must be done prior to creating VCPUs.

Please use imperative mood to state what the patch is doing/adding, as opposed to
describing the ABI.  E.g.

  Add a new capability, KVM_CAP_PMU_CAPABILITY, that takes a bitmask of
  settings/features to allow userspace to configure PMU virtualization on
  a per-VM basis.  For now, support a single flag, KVM_CAP_PMU_DISABLE,
  to allow disabling PMU virtualization for a VM even when KVM is configured
  with enable_pmu=true a module level.

  To keep KVM simple, disallow changing VM's PMU configuration after vCPUs
  have been created.

> To enable future extension, KVM_CAP_PMU_CAPABILITY reports available
> settings via bitmask when queried via check_extension.

This can be omitted, the motivation is self-explanatory and the pattern is well
established.  The desire for future expensions can be alluded to by describing
KVM_CAP_PMU_DISABLE as the first/initial flag or whatever.

> For VMs that have PMU virtualization disabled, usermode will need to
> clear CPUID leaf 0xA to notify guests.

Eh, I'd just omit this, it won't age well if there's more that userspace "needs"
to do, and strictly speaking KVM doesn't care if userspace presents a bogus vCPU
model to the guest.

> Signed-off-by: David Dunn <daviddunn@google.com>
> ---
>  Documentation/virt/kvm/api.rst  | 22 ++++++++++++++++++++++
>  arch/x86/include/asm/kvm_host.h |  1 +
>  arch/x86/kvm/svm/pmu.c          |  2 +-
>  arch/x86/kvm/vmx/pmu_intel.c    |  2 +-
>  arch/x86/kvm/x86.c              | 12 ++++++++++++
>  include/uapi/linux/kvm.h        |  4 ++++
>  tools/include/uapi/linux/kvm.h  |  4 ++++
>  7 files changed, 45 insertions(+), 2 deletions(-)
> 
> diff --git a/Documentation/virt/kvm/api.rst b/Documentation/virt/kvm/api.rst
> index a4267104db50..df836965b347 100644
> --- a/Documentation/virt/kvm/api.rst
> +++ b/Documentation/virt/kvm/api.rst
> @@ -7561,3 +7561,25 @@ The argument to KVM_ENABLE_CAP is also a bitmask, and must be a subset
>  of the result of KVM_CHECK_EXTENSION.  KVM will forward to userspace
>  the hypercalls whose corresponding bit is in the argument, and return
>  ENOSYS for the others.
> +
> +8.35 KVM_CAP_PMU_CAPABILITY
> +---------------------------
> +
> +:Capability KVM_CAP_PMU_CAPABILITY
> +:Architectures: x86
> +:Type: vm
> +:Parameters: arg[0] is bitmask of PMU virtualization capabilities.
> +:Returns 0 on success, -EINVAL when arg[0] contains invalid bits
> +
> +This capability alters PMU virtualization in KVM.
> +
> +Calling KVM_CHECK_EXTENSION for this capability returns a bitmask of
> +PMU virtualization capabilities that can be adjusted on a VM.
> +
> +The argument to KVM_ENABLE_CAP is also a bitmask and selects specific
> +PMU virtualization capabilities to be applied to the VM.  This can
> +only be invoked on a VM prior to the creation of VCPUs.
> +
> +At this time, KVM_CAP_PMU_DISABLE is the only capability.  Setting
> +this capability will disable PMU virtualization for that VM.  Usermode
> +should adjust CPUID leaf 0xA to reflect that the PMU is disabled.
> diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
> index c371ee7e45f7..f832cd0f9b27 100644
> --- a/arch/x86/include/asm/kvm_host.h
> +++ b/arch/x86/include/asm/kvm_host.h
> @@ -1232,6 +1232,7 @@ struct kvm_arch {
>  	hpa_t	hv_root_tdp;
>  	spinlock_t hv_root_tdp_lock;
>  #endif
> +	bool enable_pmu;
>  };
>  
>  struct kvm_vm_stat {
> diff --git a/arch/x86/kvm/svm/pmu.c b/arch/x86/kvm/svm/pmu.c
> index 5aa45f13b16d..d4de52409335 100644
> --- a/arch/x86/kvm/svm/pmu.c
> +++ b/arch/x86/kvm/svm/pmu.c
> @@ -101,7 +101,7 @@ static inline struct kvm_pmc *get_gp_pmc_amd(struct kvm_pmu *pmu, u32 msr,
>  {
>  	struct kvm_vcpu *vcpu = pmu_to_vcpu(pmu);
>  
> -	if (!enable_pmu)
> +	if (!vcpu->kvm->arch.enable_pmu)
>  		return NULL;
>  
>  	switch (msr) {
> diff --git a/arch/x86/kvm/vmx/pmu_intel.c b/arch/x86/kvm/vmx/pmu_intel.c
> index 03fab48b149c..4e5b1eeeb77c 100644
> --- a/arch/x86/kvm/vmx/pmu_intel.c
> +++ b/arch/x86/kvm/vmx/pmu_intel.c
> @@ -487,7 +487,7 @@ static void intel_pmu_refresh(struct kvm_vcpu *vcpu)
>  	pmu->reserved_bits = 0xffffffff00200000ull;
>  
>  	entry = kvm_find_cpuid_entry(vcpu, 0xa, 0);
> -	if (!entry || !enable_pmu)
> +	if (!entry || !vcpu->kvm->arch.enable_pmu)
>  		return;
>  	eax.full = entry->eax;
>  	edx.full = entry->edx;
> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> index 6f69f3e3635e..c35a6a193bf4 100644
> --- a/arch/x86/kvm/x86.c
> +++ b/arch/x86/kvm/x86.c
> @@ -4329,6 +4329,9 @@ int kvm_vm_ioctl_check_extension(struct kvm *kvm, long ext)
>  		if (r < sizeof(struct kvm_xsave))
>  			r = sizeof(struct kvm_xsave);
>  		break;
> +	case KVM_CAP_PMU_CAPABILITY:
> +		r = enable_pmu ? KVM_CAP_PMU_MASK : 0;
> +		break;
>  	}
>  	default:
>  		break;
> @@ -6003,6 +6006,14 @@ int kvm_vm_ioctl_enable_cap(struct kvm *kvm,
>  		kvm->arch.exit_on_emulation_error = cap->args[0];
>  		r = 0;
>  		break;
> +	case KVM_CAP_PMU_CAPABILITY:
> +		r = -EINVAL;
> +		if (!enable_pmu || kvm->created_vcpus > 0 ||

This needs to take kvm->lock when checking kvm->created_vcpus and setting the
per-VM enable_pmu.  And preferred kernel style is to omit the "> 0" when checking
for a non-zero value.  Despite being an int, created_vcpus can't go negative.

> +		    cap->args[0] & ~KVM_CAP_PMU_MASK)
> +			break;
> +		kvm->arch.enable_pmu = !(cap->args[0] & KVM_CAP_PMU_DISABLE);
> +		r = 0;
> +		break;
>  	default:
>  		r = -EINVAL;
>  		break;
> @@ -11630,6 +11641,7 @@ int kvm_arch_init_vm(struct kvm *kvm, unsigned long type)
>  	raw_spin_unlock_irqrestore(&kvm->arch.tsc_write_lock, flags);
>  
>  	kvm->arch.guest_can_read_msr_platform_info = true;
> +	kvm->arch.enable_pmu = enable_pmu;
>  
>  #if IS_ENABLED(CONFIG_HYPERV)
>  	spin_lock_init(&kvm->arch.hv_root_tdp_lock);
> diff --git a/include/uapi/linux/kvm.h b/include/uapi/linux/kvm.h
> index 5191b57e1562..cf6774cc18ef 100644
> --- a/include/uapi/linux/kvm.h
> +++ b/include/uapi/linux/kvm.h
> @@ -1134,6 +1134,7 @@ struct kvm_ppc_resize_hpt {
>  #define KVM_CAP_VM_GPA_BITS 207
>  #define KVM_CAP_XSAVE2 208
>  #define KVM_CAP_SYS_ATTRIBUTES 209
> +#define KVM_CAP_PMU_CAPABILITY 210
>  
>  #ifdef KVM_CAP_IRQ_ROUTING
>  
> @@ -1970,6 +1971,9 @@ struct kvm_dirty_gfn {
>  #define KVM_BUS_LOCK_DETECTION_OFF             (1 << 0)
>  #define KVM_BUS_LOCK_DETECTION_EXIT            (1 << 1)
>  
> +#define KVM_CAP_PMU_DISABLE                    (1 << 0)
> +#define KVM_CAP_PMU_MASK                       (KVM_CAP_PMU_DISABLE)

KVM_CAP_PMU_MASK shouldn't be in the uapi header, expanding it in the future could
theoretically break userspace

[PATCH v6 2/3] KVM: selftests: Allow creation of selftest VM without vcpus

[David Dunn]

Break out portion of vm_create_with_vcpus so that selftests can modify
the VM prior to creating vcpus.

Signed-off-by: David Dunn <daviddunn@google.com>
---
 .../selftests/kvm/include/kvm_util_base.h     |  3 ++
 tools/testing/selftests/kvm/lib/kvm_util.c    | 35 +++++++++++++++----
 2 files changed, 32 insertions(+), 6 deletions(-)

diff --git a/tools/testing/selftests/kvm/include/kvm_util_base.h b/tools/testing/selftests/kvm/include/kvm_util_base.h
index 4ed6aa049a91..2bdf96f520aa 100644
--- a/tools/testing/selftests/kvm/include/kvm_util_base.h
+++ b/tools/testing/selftests/kvm/include/kvm_util_base.h
@@ -336,6 +336,9 @@ struct kvm_vm *vm_create_with_vcpus(enum vm_guest_mode mode, uint32_t nr_vcpus,
 				    uint32_t num_percpu_pages, void *guest_code,
 				    uint32_t vcpuids[]);
 
+/* First phase of vm_create_with_vcpus, allows customization before vcpu add */
+struct kvm_vm *vm_create_without_vcpus(enum vm_guest_mode mode, uint64_t pages);
+
 /*
  * Adds a vCPU with reasonable defaults (e.g. a stack)
  *
diff --git a/tools/testing/selftests/kvm/lib/kvm_util.c b/tools/testing/selftests/kvm/lib/kvm_util.c
index d8cf851ab119..52f1f530564e 100644
--- a/tools/testing/selftests/kvm/lib/kvm_util.c
+++ b/tools/testing/selftests/kvm/lib/kvm_util.c
@@ -362,6 +362,34 @@ struct kvm_vm *vm_create(enum vm_guest_mode mode, uint64_t phy_pages, int perm)
 	return vm;
 }
 
+/*
+ * VM Create without creating VCPUs
+ *
+ * Input Args:
+ *   mode - VM Mode (e.g. VM_MODE_P52V48_4K)
+ *   pages - pages of memory required for VM
+ *
+ * Output Args: None
+ *
+ * Return:
+ *   Pointer to opaque structure that describes the created VM.
+ *
+ * Creates a VM with the mode specified by mode (e.g. VM_MODE_P52V48_4K).
+ */
+struct kvm_vm *vm_create_without_vcpus(enum vm_guest_mode mode, uint64_t pages)
+{
+	struct kvm_vm *vm;
+
+	vm = vm_create(mode, pages, O_RDWR);
+
+	kvm_vm_elf_load(vm, program_invocation_name);
+
+#ifdef __x86_64__
+	vm_create_irqchip(vm);
+#endif
+	return vm;
+}
+
 /*
  * VM Create with customized parameters
  *
@@ -412,13 +440,8 @@ struct kvm_vm *vm_create_with_vcpus(enum vm_guest_mode mode, uint32_t nr_vcpus,
 		    nr_vcpus, kvm_check_cap(KVM_CAP_MAX_VCPUS));
 
 	pages = vm_adjust_num_guest_pages(mode, pages);
-	vm = vm_create(mode, pages, O_RDWR);
-
-	kvm_vm_elf_load(vm, program_invocation_name);
 
-#ifdef __x86_64__
-	vm_create_irqchip(vm);
-#endif
+	vm = vm_create_without_vcpus(mode, pages);
 
 	for (i = 0; i < nr_vcpus; ++i) {
 		uint32_t vcpuid = vcpuids ? vcpuids[i] : i;
-- 
2.35.0.263.gb82422642f-goog

Re: [PATCH v6 2/3] KVM: selftests: Allow creation of selftest VM without vcpus

[Sean Christopherson]

Shortlog and new function name are a bit confusing.  The framework already supports
creating VMs without vCPUs, what it doesn't provide is a helper to load the guest
code and do the other "default" stuff.

That said, the framework is such an absolute mess that I'm fine going with
vm_create_without_vcpus() for now, carving out a more appropriate name will be an
exercise in futility without a large-scale renaming and refactoring of the other
crud.

So just a different shortlog I supposed, though even that seems doomed to be
contradictory.  Maybe something like this?

  KVM: selftests: Carve out helper to create "default" VM without vCPUs

Default in quotes because the selftests already have a goofy interpretation of
"default".

On Wed, Feb 09, 2022, David Dunn wrote:
> Break out portion of vm_create_with_vcpus so that selftests can modify
> the VM prior to creating vcpus.
> 
> Signed-off-by: David Dunn <daviddunn@google.com>
> ---
>  .../selftests/kvm/include/kvm_util_base.h     |  3 ++
>  tools/testing/selftests/kvm/lib/kvm_util.c    | 35 +++++++++++++++----
>  2 files changed, 32 insertions(+), 6 deletions(-)
> 
> diff --git a/tools/testing/selftests/kvm/include/kvm_util_base.h b/tools/testing/selftests/kvm/include/kvm_util_base.h
> index 4ed6aa049a91..2bdf96f520aa 100644
> --- a/tools/testing/selftests/kvm/include/kvm_util_base.h
> +++ b/tools/testing/selftests/kvm/include/kvm_util_base.h
> @@ -336,6 +336,9 @@ struct kvm_vm *vm_create_with_vcpus(enum vm_guest_mode mode, uint32_t nr_vcpus,
>  				    uint32_t num_percpu_pages, void *guest_code,
>  				    uint32_t vcpuids[]);
>  
> +/* First phase of vm_create_with_vcpus, allows customization before vcpu add */

Eh, drop the comment, the association is obvious from the code, and it's just one
more thing that needs to be updated when this stuff finally gets cleaned up.

> +struct kvm_vm *vm_create_without_vcpus(enum vm_guest_mode mode, uint64_t pages);
> +
>  /*
>   * Adds a vCPU with reasonable defaults (e.g. a stack)
>   *

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

[David Dunn]

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)
+		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.");
+
+	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

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

[Sean Christopherson]

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
>