On Thu, Jul 29, 2021, Yan Zhao wrote:
> On Thu, Jul 29, 2021 at 11:00:56AM +0800, Yu Zhang wrote:
> > >
> > > Ooof that's a lot of resets, though if there are only a handful of
> > > pages mapped, it might not be a noticeable performance impact. I think
> > > it'd be worth collecting some performance data to quantify the impact.
> >
> > Yes. Too many reset will definitely hurt the performance, though I did not see
> > obvious delay.
> >
>
> if I add below limits before unloading mmu, and with
> enable_unrestricted_guest=0, the boot time can be reduced to 31 secs
> from more than 5 minutes.
>
> void kvm_mmu_reset_context(struct kvm_vcpu *vcpu)
> {
> - kvm_mmu_unload(vcpu);
> - kvm_init_mmu(vcpu, true);
> + union kvm_mmu_role new_role =
> + kvm_calc_tdp_mmu_root_page_role(vcpu, false);
> + struct kvm_mmu *context = &vcpu->arch.root_mmu;
> + bool reset = false;
> +
> + if (new_role.as_u64 != context->mmu_role.as_u64) {
Aha! A clue!
This hack indicates that the call to kvm_mmu_reset_context() is spurious, i.e.
none of the MMU role bits in CR0/CR4/EFER are changing. Dollars to donuts says
this is due to the long-standing hack-a-fix in enter_rmode() that unconditionally
reset the MMU when the guest entered real mode.
Prior to commit 5babbb43a58a ("KVM: VMX: Remove explicit MMU reset in enter_rmode()").
(sitting in kvm/queue), enter_rmode() to deal with unrestricted_guest=0 would
unconditionally do kvm_mmu_reset_context(). Based on the above, it sounds like
your guest is going in and out of RM/PM, i.e. toggling CR0.PE. CR0.PE isn't a
MMU role bit, so the kvm_mmu_reset_context() is spurious unless CR0.PG is also
being changed.
TL;DR: Try the current kvm/queue, or at least after commit 5babbb43a58a
("KVM: VMX: Remove explicit MMU reset in enter_rmode()").
> + kvm_mmu_unload(vcpu);
> + reset = true;
> + }
> + kvm_init_mmu(vcpu, reset);
>
> But with enable_unrestricted_guest=0, if I further modify the limits to
> "if (new_role.base.word != context->mmu_role.base.word)", the VM would
> fail to boot.
> so, with mmu extended role changes, unload the mmu is necessary in some
> situation, or at least we need to zap related sptes.
>
> Thanks
> Yan
