Re: [PATCH] Don't map nested_vmcb on INTERCEPT_MSR_PROT

[Marcelo Tosatti <mtosatti@redhat.com>]

On Thu, Sep 03, 2009 at 04:51:52PM +0200, Alexander Graf wrote:
> Thanks to Joerg's previous series of cleanups, we now have almost all
> information we need to decide what to do on #VMEXIT because we get
> the variables from the VMCB on VMRUN.
> 
> Unfortunately there's one piece that slipped through the conversion,
> namely the MSR intercept which still tries to map the nested VMCB
> to find out if MSRs are intercepted.
> 
> So let's use the cached value, removing the need for two atomic maps
> (which breaks anyways) and fix an oops along the way.
> 
> CC: Joerg Roedel <joerg.roedel@amd.com>
> Signed-off-by: Alexander Graf <agraf@suse.de>

Applied, thanks.

BTW, why nested_svm_map takes mmap_sem? Thats looks wrong.

> 
> ---
> 
> v1 -> v2:
> 
>   - Don't break when MSR is not intercepted
> ---
>  arch/x86/kvm/svm.c |   15 ++++++---------
>  1 files changed, 6 insertions(+), 9 deletions(-)
> 
> diff --git a/arch/x86/kvm/svm.c b/arch/x86/kvm/svm.c
> index 2df9b45..a5f90c7 100644
> --- a/arch/x86/kvm/svm.c
> +++ b/arch/x86/kvm/svm.c
> @@ -1427,19 +1427,17 @@ static bool nested_svm_exit_handled_msr(struct vcpu_svm *svm)
>  {
>  	u32 param = svm->vmcb->control.exit_info_1 & 1;
>  	u32 msr = svm->vcpu.arch.regs[VCPU_REGS_RCX];
> -	struct vmcb *nested_vmcb;
>  	bool ret = false;
>  	u32 t0, t1;
>  	u8 *msrpm;
>  
> -	nested_vmcb = nested_svm_map(svm, svm->nested.vmcb, KM_USER0);
> -	msrpm       = nested_svm_map(svm, svm->nested.vmcb_msrpm, KM_USER1);
> +	if (!(svm->nested.intercept & (1ULL << INTERCEPT_MSR_PROT)))
> +		return false;
>  
> -	if (!nested_vmcb || !msrpm)
> -		goto out;
> +	msrpm = nested_svm_map(svm, svm->nested.vmcb_msrpm, KM_USER0);
>  
> -	if (!(nested_vmcb->control.intercept & (1ULL << INTERCEPT_MSR_PROT)))
> -		return 0;
> +	if (!msrpm)
> +		goto out;
>  
>  	switch (msr) {
>  	case 0 ... 0x1fff:
> @@ -1464,8 +1462,7 @@ static bool nested_svm_exit_handled_msr(struct vcpu_svm *svm)
>  	ret = msrpm[t1] & ((1 << param) << t0);
>  
>  out:
> -	nested_svm_unmap(nested_vmcb, KM_USER0);
> -	nested_svm_unmap(msrpm, KM_USER1);
> +	nested_svm_unmap(msrpm, KM_USER0);
>  
>  	return ret;
>  }
> -- 
> 1.6.0.2
> 
> --
> To unsubscribe from this list: send the line "unsubscribe kvm" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html