On Thu, Jul 29, 2021 at 10:58:15AM +0800, 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) {
> + 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.
update to this enable_unrestricted_guest=0 scenario, the previous VM failure to
boot with restrict "if (new_role.base.word != context->mmu_role.base.word)"
is because pcid changes.
with tdp_enabled, below code with extra loading of pgd actually work without
unloading the mmu.
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.base.word != context->mmu_role.base.word) {
+ kvm_mmu_unload(vcpu);
+ reset = true;
+ }
+ if (new_role.ext.cr0_pg ^ context->mmu_role.ext.cr0_pg) {
+ kvm_make_all_cpus_request(vcpu->kvm, KVM_REQ_LOAD_MMU_PGD);
+ kvm_flush_remote_tlbs(vcpu->kvm);
+ }
+ kvm_init_mmu(vcpu, reset);
Thanks
Yan
> 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
