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.
> +
> + /*
> + * 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.
*/
> + * 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;
> + 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
>
