On Fri, Aug 25, 2023 at 04:13:35PM -0700, Sean Christopherson wrote:
> On Fri, Jul 14, 2023, Yan Zhao wrote:
> > void kvm_honors_guest_mtrrs_zap_on_cd_toggle(struct kvm_vcpu *vcpu)
> > {
> > - return kvm_mtrr_zap_gfn_range(vcpu, gpa_to_gfn(0), gpa_to_gfn(~0ULL));
> > + struct kvm_mtrr *mtrr_state = &vcpu->arch.mtrr_state;
> > + bool mtrr_enabled = mtrr_is_enabled(mtrr_state);
> > + u8 default_mtrr_type;
> > + bool cd_ipat;
> > + u8 cd_type;
> > +
> > + kvm_honors_guest_mtrrs_get_cd_memtype(vcpu, &cd_type, &cd_ipat);
> > +
> > + default_mtrr_type = mtrr_enabled ? mtrr_default_type(mtrr_state) :
> > + mtrr_disabled_type(vcpu);
> > +
> > + if (cd_type != default_mtrr_type || cd_ipat)
> > + return kvm_mtrr_zap_gfn_range(vcpu, gpa_to_gfn(0), gpa_to_gfn(~0ULL));
>
> Why does this use use the MTRR version but the failure path does not? Ah, because
> trying to allocate in the failure path will likely fail to allocate memory. With
> a statically sized array, we can just special case the 0 => -1 case. Actually,
> we can do that regardless, it just doesn't need a dedicated flag if we use an
> array.
>
> Using the MTRR version on failure (array is full) means that other vCPUs can see
> that everything is being zapped and go straight to waitin.
Yes, will convert to the way of using statially sized array in next version.
>
> > +
> > + /*
> > + * If mtrr is not enabled, it will go to zap all above if the default
>
> Pronouns again. Maybe this?
>
> /*
> * The default MTRR type has already been checked above, if MTRRs are
> * disabled there are no other MTRR types to consider.
> */
Yes, better :)
> > + * type does not equal to cd_type;
> > + * Or it has no need to zap if the default type equals to cd_type.
> > + */
> > + if (mtrr_enabled) {
>
> To save some indentation:
>
> if (!mtrr_enabled)
> return;
>
Got it!
> > + if (prepare_zaplist_fixed_mtrr_of_non_type(vcpu, default_mtrr_type))
> > + goto fail;
> > +
> > + if (prepare_zaplist_var_mtrr_of_non_type(vcpu, default_mtrr_type))
> > + goto fail;
> > +
> > + kvm_zap_or_wait_mtrr_zap_list(vcpu->kvm);
> > + }
> > + return;
> > +fail:
> > + kvm_clear_mtrr_zap_list(vcpu->kvm);
> > + /* resort to zapping all on failure*/
> > + kvm_zap_gfn_range(vcpu->kvm, gpa_to_gfn(0), gpa_to_gfn(~0ULL));
> > + return;
> > }
> > --
> > 2.17.1
> >
