Re: [PATCH 3/3] KVM: x86: Don't deflect MSRs to userspace that can't be filtered

[Sean Christopherson <seanjc@google.com>]

On Sun, Jul 03, 2022, Aaron Lewis wrote:
> If an MSR is not permitted to be filtered and deflected to userspace,
> don't then allow it to be deflected to userspace by other means.  If an
> MSR that cannot be filtered #GP's, and KVM is configured to send all
> MSRs that #GP to userspace, that MSR will be sent to userspace as well.
> Prevent that from happening by filtering out disallowed MSRs from being
> deflected to userspace.

Why?  Honest question.  KVM doesn't allow filtering x2APIC accesses because
supporting that would be messy, and there's no sane use case for intercepting
x2APIC accesses if userspace has enabled the in-kernel local APIC.

I can't think of a meaningful use case for intercepting faults on x2APIC MSRs,
but I also don't see anything inherently broken with allowing userspace to intercept
such faults.

> Signed-off-by: Aaron Lewis <aaronlewis@google.com>
> ---
>  arch/x86/kvm/x86.c | 16 ++++++++++++++--
>  1 file changed, 14 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> index 031678eff28e..a84741f7d254 100644
> --- a/arch/x86/kvm/x86.c
> +++ b/arch/x86/kvm/x86.c
> @@ -1712,6 +1712,15 @@ void kvm_enable_efer_bits(u64 mask)
>  }
>  EXPORT_SYMBOL_GPL(kvm_enable_efer_bits);
>  
> +bool kvm_msr_filtering_disallowed(u32 index)

Should be static, per the test bot.

> +{
> +	/* x2APIC MSRs do not support filtering. */
> +	if (index >= 0x800 && index <= 0x8ff)
> +		return true;
> +
> +	return false;
> +}
> +
>  bool kvm_msr_allowed(struct kvm_vcpu *vcpu, u32 index, u32 type)
>  {
>  	struct kvm_x86_msr_filter *msr_filter;
> @@ -1721,8 +1730,8 @@ bool kvm_msr_allowed(struct kvm_vcpu *vcpu, u32 index, u32 type)
>  	int idx;
>  	u32 i;
>  
> -	/* x2APIC MSRs do not support filtering. */
> -	if (index >= 0x800 && index <= 0x8ff)
> +	/* Prevent certain MSRs from using MSR Filtering. */
> +	if (kvm_msr_filtering_disallowed(index))
>  		return true;
>  
>  	idx = srcu_read_lock(&kvm->srcu);
> @@ -1962,6 +1971,9 @@ static int kvm_msr_user_space(struct kvm_vcpu *vcpu, u32 index,
>  	if (!(vcpu->kvm->arch.user_space_msr_mask & msr_reason))
>  		return 0;
>  
> +	if (kvm_msr_filtering_disallowed(index))
> +		return 0;
> +
>  	vcpu->run->exit_reason = exit_reason;
>  	vcpu->run->msr.error = 0;
>  	memset(vcpu->run->msr.pad, 0, sizeof(vcpu->run->msr.pad));
> -- 
> 2.37.0.rc0.161.g10f37bed90-goog
>