linux-kernel - Re: [PATCH 0/5] KVM: SVM: nSVM: Implement Enlightened MSR-Bitmap for Hyper-V-on-KVM and fix it for KVM-on-Hyper-V

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]

Message-ID: <66bcd1bf-0df4-8f02-9c0d-f71cecef71f4@redhat.com>
Date:   Tue, 1 Feb 2022 19:30:23 +0100
From:   Paolo Bonzini <pbonzini@...hat.com>
To:     Vitaly Kuznetsov <vkuznets@...hat.com>, kvm@...r.kernel.org
Cc:     Sean Christopherson <seanjc@...gle.com>,
        Wanpeng Li <wanpengli@...cent.com>,
        Jim Mattson <jmattson@...gle.com>,
        Maxim Levitsky <mlevitsk@...hat.com>,
        Vineeth Pillai <viremana@...ux.microsoft.com>,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH 0/5] KVM: SVM: nSVM: Implement Enlightened MSR-Bitmap for
 Hyper-V-on-KVM and fix it for KVM-on-Hyper-V

On 2/1/22 15:31, Vitaly Kuznetsov wrote:
> Paolo Bonzini <pbonzini@...hat.com> writes:
> 
>> On 12/20/21 16:21, Vitaly Kuznetsov wrote:
>>> Enlightened MSR-Bitmap feature implements a PV protocol for L0 and L1
>>> hypervisors to collaborate and skip unneeded updates to MSR-Bitmap.
>>> KVM implements the feature for KVM-on-Hyper-V but it seems there was
>>> a flaw in the implementation and the feature may not be fully functional.
>>> PATCHes 1-2 fix the problem. The rest of the series implements the same
>>> feature for Hyper-V-on-KVM.
>>>
>>> Vitaly Kuznetsov (5):
>>>     KVM: SVM: Drop stale comment from
>>>       svm_hv_vmcb_dirty_nested_enlightenments()
>>>     KVM: SVM: hyper-v: Enable Enlightened MSR-Bitmap support for real
>>>     KVM: nSVM: Track whether changes in L0 require MSR bitmap for L2 to be
>>>       rebuilt
>>>     KVM: x86: Make kvm_hv_hypercall_enabled() static inline
>>>     KVM: nSVM: Implement Enlightened MSR-Bitmap feature
>>>
>>>    arch/x86/kvm/hyperv.c           | 12 +--------
>>>    arch/x86/kvm/hyperv.h           |  6 ++++-
>>>    arch/x86/kvm/svm/nested.c       | 47 ++++++++++++++++++++++++++++-----
>>>    arch/x86/kvm/svm/svm.c          |  3 ++-
>>>    arch/x86/kvm/svm/svm.h          | 16 +++++++----
>>>    arch/x86/kvm/svm/svm_onhyperv.h | 12 +++------
>>>    6 files changed, 63 insertions(+), 33 deletions(-)
>>>
>>
>> Queued 3-5 now, but it would be nice to have some testcases.

Hmm, it fails to compile with CONFIG_HYPERV disabled, and a trivial
#if also fails due to an unused goto label.  Does this look good to you?

diff --git a/arch/x86/kvm/svm/nested.c b/arch/x86/kvm/svm/nested.c
index e3759a79d39a..a2b5267b3e73 100644
--- a/arch/x86/kvm/svm/nested.c
+++ b/arch/x86/kvm/svm/nested.c
@@ -173,9 +173,16 @@ void recalc_intercepts(struct vcpu_svm *svm)
   */
  static bool nested_svm_vmrun_msrpm(struct vcpu_svm *svm)
  {
+	int i;
+
+	if (!(vmcb12_is_intercept(&svm->nested.ctl, INTERCEPT_MSR_PROT)))
+		return true;
+
+	svm->vmcb->control.msrpm_base_pa = __sme_set(__pa(svm->nested.msrpm));
+
+#if IS_ENABLED(CONFIG_HYPERV)
  	struct hv_enlightenments *hve =
  		(struct hv_enlightenments *)svm->nested.ctl.reserved_sw;
-	int i;
  
  	/*
  	 * MSR bitmap update can be skipped when:
@@ -185,10 +192,8 @@ static bool nested_svm_vmrun_msrpm(struct vcpu_svm *svm)
  	    kvm_hv_hypercall_enabled(&svm->vcpu) &&
  	    hve->hv_enlightenments_control.msr_bitmap &&
  	    (svm->nested.ctl.clean & VMCB_HV_NESTED_ENLIGHTENMENTS))
-		goto set_msrpm_base_pa;
-
-	if (!(vmcb12_is_intercept(&svm->nested.ctl, INTERCEPT_MSR_PROT)))
  		return true;
+#endif
  
  	for (i = 0; i < MSRPM_OFFSETS; i++) {
  		u32 value, p;
@@ -213,10 +216,6 @@ static bool nested_svm_vmrun_msrpm(struct vcpu_svm *svm)
  	}
  
  	svm->nested.force_msr_bitmap_recalc = false;
-
-set_msrpm_base_pa:
-	svm->vmcb->control.msrpm_base_pa = __sme_set(__pa(svm->nested.msrpm));
-
  	return true;
  }
  


Thanks,

Paolo