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

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

[Vitaly Kuznetsov]

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(-)

-- 
2.33.1

[PATCH 1/5] KVM: SVM: Drop stale comment from svm_hv_vmcb_dirty_nested_enlightenments()

[Vitaly Kuznetsov]

Commit 3fa5e8fd0a0e4 ("KVM: SVM: delay svm_vcpu_init_msrpm after
svm->vmcb is initialized") re-arranged svm_vcpu_init_msrpm() call in
svm_create_vcpu() making the comment about vmcb being NULL
obsolete. Drop it.

While on it, drop superfluous vmcb_is_clean() check: vmcb_mark_dirty()
is a bit flip, an extra check is unlikely to bring any performance gain.
Drop now-unneeded vmcb_is_clean() helper as well.

Fixes: 3fa5e8fd0a0e4 ("KVM: SVM: delay svm_vcpu_init_msrpm after svm->vmcb is initialized")
Signed-off-by: Vitaly Kuznetsov <vkuznets@redhat.com>
---
 arch/x86/kvm/svm/svm.h          | 5 -----
 arch/x86/kvm/svm/svm_onhyperv.h | 9 +--------
 2 files changed, 1 insertion(+), 13 deletions(-)

diff --git a/arch/x86/kvm/svm/svm.h b/arch/x86/kvm/svm/svm.h
index daa8ca84afcc..5d197aae3a19 100644
--- a/arch/x86/kvm/svm/svm.h
+++ b/arch/x86/kvm/svm/svm.h
@@ -305,11 +305,6 @@ static inline void vmcb_mark_all_clean(struct vmcb *vmcb)
 			       & ~VMCB_ALWAYS_DIRTY_MASK;
 }
 
-static inline bool vmcb_is_clean(struct vmcb *vmcb, int bit)
-{
-	return (vmcb->control.clean & (1 << bit));
-}
-
 static inline void vmcb_mark_dirty(struct vmcb *vmcb, int bit)
 {
 	vmcb->control.clean &= ~(1 << bit);
diff --git a/arch/x86/kvm/svm/svm_onhyperv.h b/arch/x86/kvm/svm/svm_onhyperv.h
index c53b8bf8d013..cdbcfc63d171 100644
--- a/arch/x86/kvm/svm/svm_onhyperv.h
+++ b/arch/x86/kvm/svm/svm_onhyperv.h
@@ -83,14 +83,7 @@ static inline void svm_hv_vmcb_dirty_nested_enlightenments(
 	struct hv_enlightenments *hve =
 		(struct hv_enlightenments *)vmcb->control.reserved_sw;
 
-	/*
-	 * vmcb can be NULL if called during early vcpu init.
-	 * And its okay not to mark vmcb dirty during vcpu init
-	 * as we mark it dirty unconditionally towards end of vcpu
-	 * init phase.
-	 */
-	if (vmcb_is_clean(vmcb, VMCB_HV_NESTED_ENLIGHTENMENTS) &&
-	    hve->hv_enlightenments_control.msr_bitmap)
+	if (hve->hv_enlightenments_control.msr_bitmap)
 		vmcb_mark_dirty(vmcb, VMCB_HV_NESTED_ENLIGHTENMENTS);
 }
 
-- 
2.33.1

[PATCH 2/5] KVM: SVM: hyper-v: Enable Enlightened MSR-Bitmap support for real

[Vitaly Kuznetsov]

Commit c4327f15dfc7 ("KVM: SVM: hyper-v: Enlightened MSR-Bitmap support")
introduced enlightened MSR-Bitmap support for KVM-on-Hyper-V but it didn't
actually enable the support. Similar to enlightened NPT TLB flush and
direct TLB flush features, the guest (KVM) has to tell L0 (Hyper-V) that
it's using the feature by setting the appropriate feature fit in VMCB
control area (sw reserved fields).

Fixes: c4327f15dfc7 ("KVM: SVM: hyper-v: Enlightened MSR-Bitmap support")
Signed-off-by: Vitaly Kuznetsov <vkuznets@redhat.com>
---
 arch/x86/kvm/svm/svm_onhyperv.h | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/arch/x86/kvm/svm/svm_onhyperv.h b/arch/x86/kvm/svm/svm_onhyperv.h
index cdbcfc63d171..489ca56212c6 100644
--- a/arch/x86/kvm/svm/svm_onhyperv.h
+++ b/arch/x86/kvm/svm/svm_onhyperv.h
@@ -46,6 +46,9 @@ static inline void svm_hv_init_vmcb(struct vmcb *vmcb)
 	if (npt_enabled &&
 	    ms_hyperv.nested_features & HV_X64_NESTED_ENLIGHTENED_TLB)
 		hve->hv_enlightenments_control.enlightened_npt_tlb = 1;
+
+	if (ms_hyperv.nested_features & HV_X64_NESTED_MSR_BITMAP)
+		hve->hv_enlightenments_control.msr_bitmap = 1;
 }
 
 static inline void svm_hv_hardware_setup(void)
-- 
2.33.1

[PATCH 3/5] KVM: nSVM: Track whether changes in L0 require MSR bitmap for L2 to be rebuilt

[Vitaly Kuznetsov]

Similar to nVMX commit ed2a4800ae9d ("KVM: nVMX: Track whether changes in
L0 require MSR bitmap for L2 to be rebuilt"), introduce a flag to keep
track of whether MSR bitmap for L2 needs to be rebuilt due to changes in
MSR bitmap for L1 or switching to a different L2.

Signed-off-by: Vitaly Kuznetsov <vkuznets@redhat.com>
---
 arch/x86/kvm/svm/nested.c | 6 ++++++
 arch/x86/kvm/svm/svm.c    | 3 ++-
 arch/x86/kvm/svm/svm.h    | 9 +++++++++
 3 files changed, 17 insertions(+), 1 deletion(-)

diff --git a/arch/x86/kvm/svm/nested.c b/arch/x86/kvm/svm/nested.c
index cf206855ebf0..f27323728be2 100644
--- a/arch/x86/kvm/svm/nested.c
+++ b/arch/x86/kvm/svm/nested.c
@@ -193,6 +193,8 @@ static bool nested_svm_vmrun_msrpm(struct vcpu_svm *svm)
 		svm->nested.msrpm[p] = svm->msrpm[p] | value;
 	}
 
+	svm->nested.force_msr_bitmap_recalc = false;
+
 	svm->vmcb->control.msrpm_base_pa = __sme_set(__pa(svm->nested.msrpm));
 
 	return true;
@@ -494,6 +496,7 @@ static void nested_vmcb02_prepare_save(struct vcpu_svm *svm, struct vmcb *vmcb12
 	if (svm->nested.vmcb12_gpa != svm->nested.last_vmcb12_gpa) {
 		new_vmcb12 = true;
 		svm->nested.last_vmcb12_gpa = svm->nested.vmcb12_gpa;
+		svm->nested.force_msr_bitmap_recalc = true;
 	}
 
 	if (unlikely(new_vmcb12 || vmcb_is_dirty(vmcb12, VMCB_SEG))) {
@@ -1494,6 +1497,9 @@ static int svm_set_nested_state(struct kvm_vcpu *vcpu,
 
 	svm_switch_vmcb(svm, &svm->nested.vmcb02);
 	nested_vmcb02_prepare_control(svm);
+
+	svm->nested.force_msr_bitmap_recalc = true;
+
 	kvm_make_request(KVM_REQ_GET_NESTED_STATE_PAGES, vcpu);
 	ret = 0;
 out_free:
diff --git a/arch/x86/kvm/svm/svm.c b/arch/x86/kvm/svm/svm.c
index e57e6857e063..24cae7e43316 100644
--- a/arch/x86/kvm/svm/svm.c
+++ b/arch/x86/kvm/svm/svm.c
@@ -668,6 +668,7 @@ static bool msr_write_intercepted(struct kvm_vcpu *vcpu, u32 msr)
 static void set_msr_interception_bitmap(struct kvm_vcpu *vcpu, u32 *msrpm,
 					u32 msr, int read, int write)
 {
+	struct vcpu_svm *svm = to_svm(vcpu);
 	u8 bit_read, bit_write;
 	unsigned long tmp;
 	u32 offset;
@@ -698,7 +699,7 @@ static void set_msr_interception_bitmap(struct kvm_vcpu *vcpu, u32 *msrpm,
 	msrpm[offset] = tmp;
 
 	svm_hv_vmcb_dirty_nested_enlightenments(vcpu);
-
+	svm->nested.force_msr_bitmap_recalc = true;
 }
 
 void set_msr_interception(struct kvm_vcpu *vcpu, u32 *msrpm, u32 msr,
diff --git a/arch/x86/kvm/svm/svm.h b/arch/x86/kvm/svm/svm.h
index 5d197aae3a19..e5040daaa968 100644
--- a/arch/x86/kvm/svm/svm.h
+++ b/arch/x86/kvm/svm/svm.h
@@ -164,6 +164,15 @@ struct svm_nested_state {
 	struct vmcb_save_area_cached save;
 
 	bool initialized;
+
+	/*
+	 * Indicates whether MSR bitmap for L2 needs to be rebuilt due to
+	 * changes in MSR bitmap for L1 or switching to a different L2. Note,
+	 * this flag can only be used reliably in conjunction with a paravirt L1
+	 * which informs L0 whether any changes to MSR bitmap for L2 were done
+	 * on its side.
+	 */
+	bool force_msr_bitmap_recalc;
 };
 
 struct vcpu_sev_es_state {
-- 
2.33.1

[PATCH 4/5] KVM: x86: Make kvm_hv_hypercall_enabled() static inline

[Vitaly Kuznetsov]

In preparation for using kvm_hv_hypercall_enabled() from SVM code, make
it static inline to avoid the need to export it. The function is a
simple check with only two call sites currently.

Signed-off-by: Vitaly Kuznetsov <vkuznets@redhat.com>
---
 arch/x86/kvm/hyperv.c | 5 -----
 arch/x86/kvm/hyperv.h | 6 +++++-
 2 files changed, 5 insertions(+), 6 deletions(-)

diff --git a/arch/x86/kvm/hyperv.c b/arch/x86/kvm/hyperv.c
index a91424ed436d..c008522112f6 100644
--- a/arch/x86/kvm/hyperv.c
+++ b/arch/x86/kvm/hyperv.c
@@ -2014,11 +2014,6 @@ int kvm_hv_set_enforce_cpuid(struct kvm_vcpu *vcpu, bool enforce)
 	return ret;
 }
 
-bool kvm_hv_hypercall_enabled(struct kvm_vcpu *vcpu)
-{
-	return vcpu->arch.hyperv_enabled && to_kvm_hv(vcpu->kvm)->hv_guest_os_id;
-}
-
 static void kvm_hv_hypercall_set_result(struct kvm_vcpu *vcpu, u64 result)
 {
 	bool longmode;
diff --git a/arch/x86/kvm/hyperv.h b/arch/x86/kvm/hyperv.h
index ed1c4e546d04..e19c00ee9ab3 100644
--- a/arch/x86/kvm/hyperv.h
+++ b/arch/x86/kvm/hyperv.h
@@ -89,7 +89,11 @@ static inline u32 kvm_hv_get_vpindex(struct kvm_vcpu *vcpu)
 int kvm_hv_set_msr_common(struct kvm_vcpu *vcpu, u32 msr, u64 data, bool host);
 int kvm_hv_get_msr_common(struct kvm_vcpu *vcpu, u32 msr, u64 *pdata, bool host);
 
-bool kvm_hv_hypercall_enabled(struct kvm_vcpu *vcpu);
+static inline bool kvm_hv_hypercall_enabled(struct kvm_vcpu *vcpu)
+{
+	return vcpu->arch.hyperv_enabled && to_kvm_hv(vcpu->kvm)->hv_guest_os_id;
+}
+
 int kvm_hv_hypercall(struct kvm_vcpu *vcpu);
 
 void kvm_hv_irq_routing_update(struct kvm *kvm);
-- 
2.33.1

[PATCH 5/5] KVM: nSVM: Implement Enlightened MSR-Bitmap feature

[Vitaly Kuznetsov]

Similar to nVMX commit 502d2bf5f2fd ("KVM: nVMX: Implement Enlightened MSR
Bitmap feature"), add support for the feature for nSVM (Hyper-V on KVM).

Notable differences from nVMX implementation:
- As the feature uses SW reserved fields in VMCB control, KVM needs to
make sure it's dealing with a Hyper-V guest (kvm_hv_hypercall_enabled()).

- 'msrpm_base_pa' needs to be always be overwritten in
nested_svm_vmrun_msrpm(), even when the update is skipped. As an
optimization, nested_vmcb02_prepare_control() copies it from VMCB01
so when MSR-Bitmap feature for L2 is disabled nothing needs to be done.

- 'struct vmcb_ctrl_area_cached' needs to be extended with clean
fields/sw reserved data and __nested_copy_vmcb_control_to_cache() needs to
copy it so nested_svm_vmrun_msrpm() can use it later.

Signed-off-by: Vitaly Kuznetsov <vkuznets@redhat.com>
---
 arch/x86/kvm/hyperv.c     |  7 +------
 arch/x86/kvm/svm/nested.c | 41 ++++++++++++++++++++++++++++++++-------
 arch/x86/kvm/svm/svm.h    |  2 ++
 3 files changed, 37 insertions(+), 13 deletions(-)

diff --git a/arch/x86/kvm/hyperv.c b/arch/x86/kvm/hyperv.c
index c008522112f6..18af9e67be40 100644
--- a/arch/x86/kvm/hyperv.c
+++ b/arch/x86/kvm/hyperv.c
@@ -2409,10 +2409,6 @@ int kvm_get_hv_cpuid(struct kvm_vcpu *vcpu, struct kvm_cpuid2 *cpuid,
 	if (kvm_x86_ops.nested_ops->get_evmcs_version)
 		evmcs_ver = kvm_x86_ops.nested_ops->get_evmcs_version(vcpu);
 
-	/* Skip NESTED_FEATURES if eVMCS is not supported */
-	if (!evmcs_ver)
-		--nent;
-
 	if (cpuid->nent < nent)
 		return -E2BIG;
 
@@ -2512,8 +2508,7 @@ int kvm_get_hv_cpuid(struct kvm_vcpu *vcpu, struct kvm_cpuid2 *cpuid,
 
 		case HYPERV_CPUID_NESTED_FEATURES:
 			ent->eax = evmcs_ver;
-			if (evmcs_ver)
-				ent->eax |= HV_X64_NESTED_MSR_BITMAP;
+			ent->eax |= HV_X64_NESTED_MSR_BITMAP;
 
 			break;
 
diff --git a/arch/x86/kvm/svm/nested.c b/arch/x86/kvm/svm/nested.c
index f27323728be2..c975570188e9 100644
--- a/arch/x86/kvm/svm/nested.c
+++ b/arch/x86/kvm/svm/nested.c
@@ -28,6 +28,7 @@
 #include "cpuid.h"
 #include "lapic.h"
 #include "svm.h"
+#include "svm_onhyperv.h"
 
 #define CC KVM_NESTED_VMENTER_CONSISTENCY_CHECK
 
@@ -165,14 +166,30 @@ void recalc_intercepts(struct vcpu_svm *svm)
 	vmcb_set_intercept(c, INTERCEPT_VMSAVE);
 }
 
+/*
+ * Merge L0's (KVM) and L1's (Nested VMCB) MSR permission bitmaps. The function
+ * is optimized in that it only merges the parts where KVM MSR permission bitmap
+ * may contain zero bits.
+ */
 static bool nested_svm_vmrun_msrpm(struct vcpu_svm *svm)
 {
+	struct hv_enlightenments *hve =
+		(struct hv_enlightenments *)svm->nested.ctl.reserved_sw;
+	int i;
+
 	/*
-	 * This function merges the msr permission bitmaps of kvm and the
-	 * nested vmcb. It is optimized in that it only merges the parts where
-	 * the kvm msr permission bitmap may contain zero bits
+	 * MSR bitmap update can be skipped when:
+	 * - MSR bitmap for L1 hasn't changed.
+	 * - Nested hypervisor (L1) is attempting to launch the same L2 as
+	 *   before.
+	 * - Nested hypervisor (L1) is using Hyper-V emulation interface and
+	 * tells KVM (L0) there were no changes in MSR bitmap for L2.
 	 */
-	int i;
+	if (!svm->nested.force_msr_bitmap_recalc &&
+	    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;
@@ -195,6 +212,7 @@ 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;
@@ -300,7 +318,8 @@ static bool nested_vmcb_check_controls(struct kvm_vcpu *vcpu)
 }
 
 static
-void __nested_copy_vmcb_control_to_cache(struct vmcb_ctrl_area_cached *to,
+void __nested_copy_vmcb_control_to_cache(struct kvm_vcpu *vcpu,
+					 struct vmcb_ctrl_area_cached *to,
 					 struct vmcb_control_area *from)
 {
 	unsigned int i;
@@ -333,12 +352,19 @@ void __nested_copy_vmcb_control_to_cache(struct vmcb_ctrl_area_cached *to,
 	to->asid           = from->asid;
 	to->msrpm_base_pa &= ~0x0fffULL;
 	to->iopm_base_pa  &= ~0x0fffULL;
+
+	/* Hyper-V extensions (Enlightened VMCB) */
+	if (kvm_hv_hypercall_enabled(vcpu)) {
+		to->clean = from->clean;
+		memcpy(to->reserved_sw, from->reserved_sw,
+		       sizeof(struct hv_enlightenments));
+	}
 }
 
 void nested_copy_vmcb_control_to_cache(struct vcpu_svm *svm,
 				       struct vmcb_control_area *control)
 {
-	__nested_copy_vmcb_control_to_cache(&svm->nested.ctl, control);
+	__nested_copy_vmcb_control_to_cache(&svm->vcpu, &svm->nested.ctl, control);
 }
 
 static void __nested_copy_vmcb_save_to_cache(struct vmcb_save_area_cached *to,
@@ -1305,6 +1331,7 @@ static void nested_copy_vmcb_cache_to_control(struct vmcb_control_area *dst,
 	dst->virt_ext              = from->virt_ext;
 	dst->pause_filter_count   = from->pause_filter_count;
 	dst->pause_filter_thresh  = from->pause_filter_thresh;
+	/* 'clean' and 'reserved_sw' are not changed by KVM */
 }
 
 static int svm_get_nested_state(struct kvm_vcpu *vcpu,
@@ -1437,7 +1464,7 @@ static int svm_set_nested_state(struct kvm_vcpu *vcpu,
 		goto out_free;
 
 	ret = -EINVAL;
-	__nested_copy_vmcb_control_to_cache(&ctl_cached, ctl);
+	__nested_copy_vmcb_control_to_cache(vcpu, &ctl_cached, ctl);
 	if (!__nested_vmcb_check_controls(vcpu, &ctl_cached))
 		goto out_free;
 
diff --git a/arch/x86/kvm/svm/svm.h b/arch/x86/kvm/svm/svm.h
index e5040daaa968..40b0b92d4e3e 100644
--- a/arch/x86/kvm/svm/svm.h
+++ b/arch/x86/kvm/svm/svm.h
@@ -138,6 +138,8 @@ struct vmcb_ctrl_area_cached {
 	u32 event_inj_err;
 	u64 nested_cr3;
 	u64 virt_ext;
+	u32 clean;
+	u8 reserved_sw[32];
 };
 
 struct svm_nested_state {
-- 
2.33.1

Re: [PATCH 2/5] KVM: SVM: hyper-v: Enable Enlightened MSR-Bitmap support for real

[Paolo Bonzini]

On 12/20/21 16:21, Vitaly Kuznetsov wrote:
> Commit c4327f15dfc7 ("KVM: SVM: hyper-v: Enlightened MSR-Bitmap support")
> introduced enlightened MSR-Bitmap support for KVM-on-Hyper-V but it didn't
> actually enable the support. Similar to enlightened NPT TLB flush and
> direct TLB flush features, the guest (KVM) has to tell L0 (Hyper-V) that
> it's using the feature by setting the appropriate feature fit in VMCB
> control area (sw reserved fields).
> 
> Fixes: c4327f15dfc7 ("KVM: SVM: hyper-v: Enlightened MSR-Bitmap support")
> Signed-off-by: Vitaly Kuznetsov <vkuznets@redhat.com>
> ---
>   arch/x86/kvm/svm/svm_onhyperv.h | 3 +++
>   1 file changed, 3 insertions(+)
> 
> diff --git a/arch/x86/kvm/svm/svm_onhyperv.h b/arch/x86/kvm/svm/svm_onhyperv.h
> index cdbcfc63d171..489ca56212c6 100644
> --- a/arch/x86/kvm/svm/svm_onhyperv.h
> +++ b/arch/x86/kvm/svm/svm_onhyperv.h
> @@ -46,6 +46,9 @@ static inline void svm_hv_init_vmcb(struct vmcb *vmcb)
>   	if (npt_enabled &&
>   	    ms_hyperv.nested_features & HV_X64_NESTED_ENLIGHTENED_TLB)
>   		hve->hv_enlightenments_control.enlightened_npt_tlb = 1;
> +
> +	if (ms_hyperv.nested_features & HV_X64_NESTED_MSR_BITMAP)
> +		hve->hv_enlightenments_control.msr_bitmap = 1;
>   }
>   
>   static inline void svm_hv_hardware_setup(void)

Queued this one for now.

Paolo

Re: [PATCH 1/5] KVM: SVM: Drop stale comment from svm_hv_vmcb_dirty_nested_enlightenments()

[Paolo Bonzini]

On 12/20/21 16:21, Vitaly Kuznetsov wrote:
> Commit 3fa5e8fd0a0e4 ("KVM: SVM: delay svm_vcpu_init_msrpm after
> svm->vmcb is initialized") re-arranged svm_vcpu_init_msrpm() call in
> svm_create_vcpu() making the comment about vmcb being NULL
> obsolete. Drop it.
> 
> While on it, drop superfluous vmcb_is_clean() check: vmcb_mark_dirty()
> is a bit flip, an extra check is unlikely to bring any performance gain.
> Drop now-unneeded vmcb_is_clean() helper as well.
> 
> Fixes: 3fa5e8fd0a0e4 ("KVM: SVM: delay svm_vcpu_init_msrpm after svm->vmcb is initialized")
> Signed-off-by: Vitaly Kuznetsov <vkuznets@redhat.com>

Queued, but with subject changed to "KVM: SVM: clean up beginning of 
svm_hv_vmcb_dirty_nested_enlightenments()".

Paolo

>   arch/x86/kvm/svm/svm.h          | 5 -----
>   arch/x86/kvm/svm/svm_onhyperv.h | 9 +--------
>   2 files changed, 1 insertion(+), 13 deletions(-)
> 
> diff --git a/arch/x86/kvm/svm/svm.h b/arch/x86/kvm/svm/svm.h
> index daa8ca84afcc..5d197aae3a19 100644
> --- a/arch/x86/kvm/svm/svm.h
> +++ b/arch/x86/kvm/svm/svm.h
> @@ -305,11 +305,6 @@ static inline void vmcb_mark_all_clean(struct vmcb *vmcb)
>   			       & ~VMCB_ALWAYS_DIRTY_MASK;
>   }
>   
> -static inline bool vmcb_is_clean(struct vmcb *vmcb, int bit)
> -{
> -	return (vmcb->control.clean & (1 << bit));
> -}
> -
>   static inline void vmcb_mark_dirty(struct vmcb *vmcb, int bit)
>   {
>   	vmcb->control.clean &= ~(1 << bit);
> diff --git a/arch/x86/kvm/svm/svm_onhyperv.h b/arch/x86/kvm/svm/svm_onhyperv.h
> index c53b8bf8d013..cdbcfc63d171 100644
> --- a/arch/x86/kvm/svm/svm_onhyperv.h
> +++ b/arch/x86/kvm/svm/svm_onhyperv.h
> @@ -83,14 +83,7 @@ static inline void svm_hv_vmcb_dirty_nested_enlightenments(
>   	struct hv_enlightenments *hve =
>   		(struct hv_enlightenments *)vmcb->control.reserved_sw;
>   
> -	/*
> -	 * vmcb can be NULL if called during early vcpu init.
> -	 * And its okay not to mark vmcb dirty during vcpu init
> -	 * as we mark it dirty unconditionally towards end of vcpu
> -	 * init phase.
> -	 */
> -	if (vmcb_is_clean(vmcb, VMCB_HV_NESTED_ENLIGHTENMENTS) &&
> -	    hve->hv_enlightenments_control.msr_bitmap)
> +	if (hve->hv_enlightenments_control.msr_bitmap)
>   		vmcb_mark_dirty(vmcb, VMCB_HV_NESTED_ENLIGHTENMENTS);
>   }
>   

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

[Paolo Bonzini]

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.

Paolo

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

[Vitaly Kuznetsov]

Paolo Bonzini <pbonzini@redhat.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.
>

Thanks, indeed, I'll try to draft something up, both for nVMX and nSVM.

-- 
Vitaly

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

[Paolo Bonzini]

On 2/1/22 15:31, Vitaly Kuznetsov wrote:
> Paolo Bonzini <pbonzini@redhat.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

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

[Vitaly Kuznetsov]

Paolo Bonzini <pbonzini@redhat.com> writes:

> On 2/1/22 15:31, Vitaly Kuznetsov wrote:
>> Paolo Bonzini <pbonzini@redhat.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?
>

Hm, it does but honestly I did not anticipate this dependency --
CONFIG_HYPERV is needed for KVM-on-Hyper-V but this feature is for
Hyper-V-on-KVM. Let me take a look tomorrow.

> 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
>

-- 
Vitaly

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

[Paolo Bonzini]

On 2/1/22 19:58, Vitaly Kuznetsov wrote:
>> 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?
>>
> Hm, it does but honestly I did not anticipate this dependency --
> CONFIG_HYPERV is needed for KVM-on-Hyper-V but this feature is for
> Hyper-V-on-KVM. Let me take a look tomorrow.
> 

It's because, without it, the relevant structs are not defined by 
svm_onhyperv.h.  Go ahead and send a new version if you prefer, I can 
unqueue it (really, just not push to kvm/queue).

Paolo

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

[Vitaly Kuznetsov]

Paolo Bonzini <pbonzini@redhat.com> writes:

> On 2/1/22 19:58, Vitaly Kuznetsov wrote:
>>> 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?
>>>
>> Hm, it does but honestly I did not anticipate this dependency --
>> CONFIG_HYPERV is needed for KVM-on-Hyper-V but this feature is for
>> Hyper-V-on-KVM. Let me take a look tomorrow.
>> 
>
> It's because, without it, the relevant structs are not defined by 
> svm_onhyperv.h.  Go ahead and send a new version if you prefer

Should be fixed in v2. I still think it makes sense to keep this
KVM-on-Hyper-V and Hyper-V-on-KVM separation as it's really confusing to
an unprepared reader. There's still room for improvement in nVMX I
believe but it's orthogonal to this nSVM feature.

-- 
Vitaly