Re: [PATCH 1/3] Provide VM capability to disable PMU virtualization for individual VMs

[Sean Christopherson <seanjc@google.com>]

On Wed, Jan 19, 2022, David Dunn wrote:
> When PMU virtualization is enabled via the module parameter, usermode
> can disable PMU virtualization on individual VMs using this new
> capability.
> 
> This provides a uniform way to disable PMU virtualization on x86.  Since
> AMD doesn't have a CPUID bit for PMU support, disabling PMU
> virtualization requires some other state to indicate whether the PMU
> related MSRs are ignored.
> 
> Since KVM_GET_SUPPORTED_CPUID reports the maximal CPUID information
> based on module parameters, usermode will need to adjust CPUID when
> disabling PMU virtualization on individual VMs.  On Intel CPUs, the
> change to PMU enablement will not alter existing until SET_CPUID2 is
> invoked.
> 
> Signed-off-by: David Dunn <daviddunn@google.com>
> ---

I'm not necessarily opposed to this capability, but can't userspace get the same
result by using MSR filtering to inject #GP on the PMU MSRs?

> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> index 55518b7d3b96..9b640c5bb4f6 100644
> --- a/arch/x86/kvm/x86.c
> +++ b/arch/x86/kvm/x86.c
> @@ -4326,6 +4326,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_ENABLE_PMU:
> +		r = enable_pmu;
> +		break;
>  	}
>  	default:
>  		break;
> @@ -5937,6 +5940,13 @@ int kvm_vm_ioctl_enable_cap(struct kvm *kvm,
>  		kvm->arch.exit_on_emulation_error = cap->args[0];
>  		r = 0;
>  		break;
> +	case KVM_CAP_ENABLE_PMU:
> +		r = -EINVAL;
> +		if (!enable_pmu || cap->args[0] & ~1)

Probably worth adding a #define in uapi/.../kvm.h for bit 0.

> +			break;
> +		kvm->arch.enable_pmu = cap->args[0];
> +		r = 0;
> +		break;
>  	default:
>  		r = -EINVAL;
>  		break;
> @@ -11562,6 +11572,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 = true;

Rather than default to "true", just capture the global "enable_pmu" and then all
the sites that check "enable_pmu" in VM context can check _only_ kvm->arch.enable_pmu.
enable_pmu is readonly, so there's no danger of it being toggled after the VM is
created.

>  #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 9563d294f181..37cbcdffe773 100644
> --- a/include/uapi/linux/kvm.h
> +++ b/include/uapi/linux/kvm.h
> @@ -1133,6 +1133,7 @@ struct kvm_ppc_resize_hpt {
>  #define KVM_CAP_VM_MOVE_ENC_CONTEXT_FROM 206
>  #define KVM_CAP_VM_GPA_BITS 207
>  #define KVM_CAP_XSAVE2 208
> +#define KVM_CAP_ENABLE_PMU 209
>  
>  #ifdef KVM_CAP_IRQ_ROUTING
>  
> diff --git a/tools/include/uapi/linux/kvm.h b/tools/include/uapi/linux/kvm.h
> index f066637ee206..e71712c71ab1 100644
> --- a/tools/include/uapi/linux/kvm.h
> +++ b/tools/include/uapi/linux/kvm.h
> @@ -1132,6 +1132,7 @@ struct kvm_ppc_resize_hpt {
>  #define KVM_CAP_ARM_MTE 205
>  #define KVM_CAP_VM_MOVE_ENC_CONTEXT_FROM 206
>  #define KVM_CAP_XSAVE2 207
> +#define KVM_CAP_ENABLE_PMU 209
>  
>  #ifdef KVM_CAP_IRQ_ROUTING
>  
> -- 
> 2.34.1.703.g22d0c6ccf7-goog
>