Re: [PATCH v2] kvm: svm: Only propagate next_rip when guest supports it

[Paolo Bonzini <pbonzini@redhat.com>]

On 14/10/2015 15:10, Joerg Roedel wrote:
> From 94ee662c527683c26ea5fa98a5a8f2c798c58470 Mon Sep 17 00:00:00 2001
> From: Joerg Roedel <jroedel@suse.de>
> Date: Wed, 7 Oct 2015 13:38:19 +0200
> Subject: [PATCH] kvm: svm: Only propagate next_rip when guest supports it
> 
> Currently we always write the next_rip of the shadow vmcb to
> the guests vmcb when we emulate a vmexit. This could confuse
> the guest when its cpuid indicated no support for the
> next_rip feature.
> 
> Fix this by only propagating next_rip if the guest actually
> supports it.
> 
> Cc: Bandan Das <bsd@redhat.com>
> Cc: Dirk Mueller <dmueller@suse.com>
> Tested-By: Dirk Mueller <dmueller@suse.com>
> Signed-off-by: Joerg Roedel <jroedel@suse.de>
> ---
>  arch/x86/kvm/cpuid.h | 21 +++++++++++++++++++++
>  arch/x86/kvm/svm.c   | 11 ++++++++++-
>  2 files changed, 31 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/x86/kvm/cpuid.h b/arch/x86/kvm/cpuid.h
> index dd05b9c..effca1f 100644
> --- a/arch/x86/kvm/cpuid.h
> +++ b/arch/x86/kvm/cpuid.h
> @@ -133,4 +133,25 @@ static inline bool guest_cpuid_has_mpx(struct kvm_vcpu *vcpu)
>  	best = kvm_find_cpuid_entry(vcpu, 7, 0);
>  	return best && (best->ebx & bit(X86_FEATURE_MPX));
>  }
> +
> +/*
> + * NRIPS is provided through cpuidfn 0x8000000a.edx bit 3
> + */
> +#define BIT_NRIPS	3
> +
> +static inline bool guest_cpuid_has_nrips(struct kvm_vcpu *vcpu)
> +{
> +	struct kvm_cpuid_entry2 *best;
> +
> +	best = kvm_find_cpuid_entry(vcpu, 0x8000000a, 0);
> +
> +	/*
> +	 * NRIPS is a scattered cpuid feature, so we can't use
> +	 * X86_FEATURE_NRIPS here (X86_FEATURE_NRIPS would be bit
> +	 * position 8, not 3).
> +	 */
> +	return best && (best->edx & bit(BIT_NRIPS));
> +}
> +#undef BIT_NRIPS
> +
>  #endif
> diff --git a/arch/x86/kvm/svm.c b/arch/x86/kvm/svm.c
> index 2f9ed1f..e9e3294 100644
> --- a/arch/x86/kvm/svm.c
> +++ b/arch/x86/kvm/svm.c
> @@ -159,6 +159,9 @@ struct vcpu_svm {
>  	u32 apf_reason;
>  
>  	u64  tsc_ratio;
> +
> +	/* cached guest cpuid flags for faster access */
> +	bool nrips_enabled	: 1;
>  };
>  
>  static DEFINE_PER_CPU(u64, current_tsc_ratio);
> @@ -2365,7 +2368,9 @@ static int nested_svm_vmexit(struct vcpu_svm *svm)
>  	nested_vmcb->control.exit_info_2       = vmcb->control.exit_info_2;
>  	nested_vmcb->control.exit_int_info     = vmcb->control.exit_int_info;
>  	nested_vmcb->control.exit_int_info_err = vmcb->control.exit_int_info_err;
> -	nested_vmcb->control.next_rip          = vmcb->control.next_rip;
> +
> +	if (svm->nrips_enabled)
> +		nested_vmcb->control.next_rip  = vmcb->control.next_rip;
>  
>  	/*
>  	 * If we emulate a VMRUN/#VMEXIT in the same host #vmexit cycle we have
> @@ -4098,6 +4103,10 @@ static u64 svm_get_mt_mask(struct kvm_vcpu *vcpu, gfn_t gfn, bool is_mmio)
>  
>  static void svm_cpuid_update(struct kvm_vcpu *vcpu)
>  {
> +	struct vcpu_svm *svm = to_svm(vcpu);
> +
> +	/* Update nrips enabled cache */
> +	svm->nrips_enabled = !!guest_cpuid_has_nrips(&svm->vcpu);
>  }
>  
>  static void svm_set_supported_cpuid(u32 func, struct kvm_cpuid_entry2 *entry)
> 

Applied to kvm/queue, thanks.

Paolo