linux-kernel - Re: [PATCH 08/30] KVM: nSVM: move map argument out of enter_svm_guest

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Fri, 29 May 2020 11:10:10 -0700
From:   Krish Sadhukhan <krish.sadhukhan@...cle.com>
To:     Paolo Bonzini <pbonzini@...hat.com>, linux-kernel@...r.kernel.org,
        kvm@...r.kernel.org
Subject: Re: [PATCH 08/30] KVM: nSVM: move map argument out of
 enter_svm_guest_mode


On 5/29/20 8:39 AM, Paolo Bonzini wrote:
> Unmapping the nested VMCB in enter_svm_guest_mode is a bit of a wart,
> since the map is not used elsewhere in the function.  There are
> just two calls, so move it there.


The last sentence sounds bit incomplete.

Also, does it make sense to mention the reason why unmapping is not 
required before we enter guest mode ?

>
> Signed-off-by: Paolo Bonzini <pbonzini@...hat.com>
> ---
>   arch/x86/kvm/svm/nested.c | 14 ++++++--------
>   arch/x86/kvm/svm/svm.c    |  3 ++-
>   arch/x86/kvm/svm/svm.h    |  2 +-
>   3 files changed, 9 insertions(+), 10 deletions(-)
>
> diff --git a/arch/x86/kvm/svm/nested.c b/arch/x86/kvm/svm/nested.c
> index 8756c9f463fd..8e98d5e420a5 100644
> --- a/arch/x86/kvm/svm/nested.c
> +++ b/arch/x86/kvm/svm/nested.c
> @@ -229,7 +229,7 @@ static bool nested_vmcb_checks(struct vmcb *vmcb)
>   }
>   
>   void enter_svm_guest_mode(struct vcpu_svm *svm, u64 vmcb_gpa,
> -			  struct vmcb *nested_vmcb, struct kvm_host_map *map)
> +			  struct vmcb *nested_vmcb)
>   {
>   	bool evaluate_pending_interrupts =
>   		is_intercept(svm, INTERCEPT_VINTR) ||
> @@ -304,8 +304,6 @@ void enter_svm_guest_mode(struct vcpu_svm *svm, u64 vmcb_gpa,
>   	svm->vmcb->control.pause_filter_thresh =
>   		nested_vmcb->control.pause_filter_thresh;
>   
> -	kvm_vcpu_unmap(&svm->vcpu, map, true);
> -
>   	/* Enter Guest-Mode */
>   	enter_guest_mode(&svm->vcpu);
>   
> @@ -368,10 +366,7 @@ int nested_svm_vmrun(struct vcpu_svm *svm)
>   		nested_vmcb->control.exit_code_hi = 0;
>   		nested_vmcb->control.exit_info_1  = 0;
>   		nested_vmcb->control.exit_info_2  = 0;
> -
> -		kvm_vcpu_unmap(&svm->vcpu, &map, true);
> -
> -		return ret;
> +		goto out;
>   	}
>   
>   	trace_kvm_nested_vmrun(svm->vmcb->save.rip, vmcb_gpa,
> @@ -414,7 +409,7 @@ int nested_svm_vmrun(struct vcpu_svm *svm)
>   	copy_vmcb_control_area(hsave, vmcb);
>   
>   	svm->nested.nested_run_pending = 1;
> -	enter_svm_guest_mode(svm, vmcb_gpa, nested_vmcb, &map);
> +	enter_svm_guest_mode(svm, vmcb_gpa, nested_vmcb);
>   
>   	if (!nested_svm_vmrun_msrpm(svm)) {
>   		svm->vmcb->control.exit_code    = SVM_EXIT_ERR;
> @@ -425,6 +420,9 @@ int nested_svm_vmrun(struct vcpu_svm *svm)
>   		nested_svm_vmexit(svm);
>   	}
>   
> +out:
> +	kvm_vcpu_unmap(&svm->vcpu, &map, true);
> +
>   	return ret;
>   }
>   
> diff --git a/arch/x86/kvm/svm/svm.c b/arch/x86/kvm/svm/svm.c
> index feb96a410f2d..76b3f553815e 100644
> --- a/arch/x86/kvm/svm/svm.c
> +++ b/arch/x86/kvm/svm/svm.c
> @@ -3814,7 +3814,8 @@ static int svm_pre_leave_smm(struct kvm_vcpu *vcpu, const char *smstate)
>   		if (kvm_vcpu_map(&svm->vcpu, gpa_to_gfn(vmcb), &map) == -EINVAL)
>   			return 1;
>   		nested_vmcb = map.hva;
> -		enter_svm_guest_mode(svm, vmcb, nested_vmcb, &map);
> +		enter_svm_guest_mode(svm, vmcb, nested_vmcb);
> +		kvm_vcpu_unmap(&svm->vcpu, &map, true);
>   	}
>   	return 0;
>   }
> diff --git a/arch/x86/kvm/svm/svm.h b/arch/x86/kvm/svm/svm.h
> index 89fab75dd4f5..33e3f09d7a8e 100644
> --- a/arch/x86/kvm/svm/svm.h
> +++ b/arch/x86/kvm/svm/svm.h
> @@ -395,7 +395,7 @@ static inline bool nested_exit_on_nmi(struct vcpu_svm *svm)
>   }
>   
>   void enter_svm_guest_mode(struct vcpu_svm *svm, u64 vmcb_gpa,
> -			  struct vmcb *nested_vmcb, struct kvm_host_map *map);
> +			  struct vmcb *nested_vmcb);
>   int nested_svm_vmrun(struct vcpu_svm *svm);
>   void nested_svm_vmloadsave(struct vmcb *from_vmcb, struct vmcb *to_vmcb);
>   int nested_svm_vmexit(struct vcpu_svm *svm);