On Wed, 2022-03-09 at 14:12 +0100, Paolo Bonzini wrote:
> On 3/1/22 15:36, Maxim Levitsky wrote:
> > Allow L1 to use these settings if L0 disables PAUSE interception
> > (AKA cpu_pm=on)
> >
> > Signed-off-by: Maxim Levitsky <mlevitsk@xxxxxxxxxx>
> > ---
> > arch/x86/kvm/svm/nested.c | 6 ++++++
> > arch/x86/kvm/svm/svm.c | 17 +++++++++++++++++
> > arch/x86/kvm/svm/svm.h | 2 ++
> > 3 files changed, 25 insertions(+)
> >
> > diff --git a/arch/x86/kvm/svm/nested.c b/arch/x86/kvm/svm/nested.c
> > index 37510cb206190..4cb0bc49986d5 100644
> > --- a/arch/x86/kvm/svm/nested.c
> > +++ b/arch/x86/kvm/svm/nested.c
> > @@ -664,6 +664,12 @@ static void nested_vmcb02_prepare_control(struct vcpu_svm *svm)
> > if (!nested_vmcb_needs_vls_intercept(svm))
> > svm->vmcb->control.virt_ext |= VIRTUAL_VMLOAD_VMSAVE_ENABLE_MASK;
> >
> > + if (svm->pause_filter_enabled)
> > + svm->vmcb->control.pause_filter_count = svm->nested.ctl.pause_filter_count;
> > +
> > + if (svm->pause_threshold_enabled)
> > + svm->vmcb->control.pause_filter_thresh = svm->nested.ctl.pause_filter_thresh;
>
> I think this should be
>
> if (kvm_pause_in_guest(vcpu->kvm)) {
> /* copy from VMCB12 if guest has CPUID, else set to 0 */
> } else {
> /* copy from VMCB01, unconditionally */
> }
> and likewise it should be copied back to VMCB01 unconditionally on
> vmexit if !kvm_pause_in_guest(vcpu->kvm).
I did something different in the next version of the patches.
Please take a look.
>
> > nested_svm_transition_tlb_flush(vcpu);
> >
> > /* Enter Guest-Mode */
> > diff --git a/arch/x86/kvm/svm/svm.c b/arch/x86/kvm/svm/svm.c
> > index 6a571eed32ef4..52198e63c5fc4 100644
> > --- a/arch/x86/kvm/svm/svm.c
> > +++ b/arch/x86/kvm/svm/svm.c
> > @@ -4008,6 +4008,17 @@ static void svm_vcpu_after_set_cpuid(struct kvm_vcpu *vcpu)
> >
> > svm->v_vmload_vmsave_enabled = vls && guest_cpuid_has(vcpu, X86_FEATURE_V_VMSAVE_VMLOAD);
> >
> > + if (kvm_pause_in_guest(vcpu->kvm)) {
> > + svm->pause_filter_enabled = pause_filter_count > 0 &&
> > + guest_cpuid_has(vcpu, X86_FEATURE_PAUSEFILTER);
> > +
> > + svm->pause_threshold_enabled = pause_filter_thresh > 0 &&
> > + guest_cpuid_has(vcpu, X86_FEATURE_PFTHRESHOLD);
>
> Why only if the module parameters are >0? The module parameter is
> unused if pause-in-guest is active.
Agree, will do.
>
> > + } else {
> > + svm->pause_filter_enabled = false;
> > + svm->pause_threshold_enabled = false;
> > + }
> > +
> > svm_recalc_instruction_intercepts(vcpu, svm);
> >
> > /* For sev guests, the memory encryption bit is not reserved in CR3. */
> > @@ -4763,6 +4774,12 @@ static __init void svm_set_cpu_caps(void)
> > if (vls)
> > kvm_cpu_cap_set(X86_FEATURE_V_VMSAVE_VMLOAD);
> >
> > + if (pause_filter_count)
> > + kvm_cpu_cap_set(X86_FEATURE_PAUSEFILTER);
> > +
> > + if (pause_filter_thresh)
> > + kvm_cpu_cap_set(X86_FEATURE_PFTHRESHOLD);
>
> Likewise, this should be set using just boot_cpu_has, not the module
> parameters.
Agree as well + the check above is wrong - it should have been inverted.
>
> Paolo
>
> > /* Nested VM can receive #VMEXIT instead of triggering #GP */
> > kvm_cpu_cap_set(X86_FEATURE_SVME_ADDR_CHK);
> > }
> > diff --git a/arch/x86/kvm/svm/svm.h b/arch/x86/kvm/svm/svm.h
> > index a3c93f9c02847..6fa81eb3ffb78 100644
> > --- a/arch/x86/kvm/svm/svm.h
> > +++ b/arch/x86/kvm/svm/svm.h
> > @@ -234,6 +234,8 @@ struct vcpu_svm {
> > bool tsc_scaling_enabled : 1;
> > bool lbrv_enabled : 1;
> > bool v_vmload_vmsave_enabled : 1;
> > + bool pause_filter_enabled : 1;
> > + bool pause_threshold_enabled : 1;
> >
> > u32 ldr_reg;
> > u32 dfr_reg;
Thanks for the review!
Best regards,
Maxim Levitsky
