| Message ID | 20211220152139.418372-1-vkuznets@redhat.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | KVM: SVM: nSVM: Implement Enlightened MSR-Bitmap for Hyper-V-on-KVM and fix it for KVM-on-Hyper-V | expand |
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
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.
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
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 >
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
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.
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(-)