| Message ID | 20220209170020.1775368-11-pbonzini@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | KVM: MMU: do not unload MMU roots on all role changes | expand |
On Wed, Feb 09, 2022, Paolo Bonzini wrote: > Now that __kvm_mmu_new_pgd does not look at the MMU's root_level and > shadow_root_level anymore, pull the PGD load after the initialization of > the shadow MMUs. > > Besides being more intuitive, this enables future simplifications > and optimizations because it's not necessary anymore to compute the > role outside kvm_init_mmu. In particular, kvm_mmu_reset_context was not > attempting to use a cached PGD to avoid having to figure out the new role. > It will soon be able to follow what nested_{vmx,svm}_load_cr3 are doing, > and avoid unloading all the cached roots. > > Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> > --- If you add a sanity check as described in the other thread[*], Reviewed-by: Sean Christopherson <seanjc@google.com> [*] https://lore.kernel.org/all/1e8c38eb-d66a-60e7-9432-eb70e7ec1dd4@redhat.com
On 2/11/22 18:45, Sean Christopherson wrote: > On Wed, Feb 09, 2022, Paolo Bonzini wrote: >> Now that __kvm_mmu_new_pgd does not look at the MMU's root_level and >> shadow_root_level anymore, pull the PGD load after the initialization of >> the shadow MMUs. >> >> Besides being more intuitive, this enables future simplifications >> and optimizations because it's not necessary anymore to compute the >> role outside kvm_init_mmu. In particular, kvm_mmu_reset_context was not >> attempting to use a cached PGD to avoid having to figure out the new role. >> It will soon be able to follow what nested_{vmx,svm}_load_cr3 are doing, >> and avoid unloading all the cached roots. >> >> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> >> --- > > If you add a sanity check as described in the other thread[*], > > Reviewed-by: Sean Christopherson <seanjc@google.com> It's not as easy as I thought, but it becomes almost trivial with the CPU/MMU role refactoring. I'll get that posted as soon as I can push a final-ish version of this one to kvm/queue. Paolo
diff --git a/arch/x86/kvm/mmu/mmu.c b/arch/x86/kvm/mmu/mmu.c index f61208ccce43..df9e0a43513c 100644 --- a/arch/x86/kvm/mmu/mmu.c +++ b/arch/x86/kvm/mmu/mmu.c @@ -4882,9 +4882,8 @@ void kvm_init_shadow_npt_mmu(struct kvm_vcpu *vcpu, unsigned long cr0, new_role = kvm_calc_shadow_npt_root_page_role(vcpu, ®s); - __kvm_mmu_new_pgd(vcpu, nested_cr3, new_role.base); - shadow_mmu_init_context(vcpu, context, ®s, new_role); + __kvm_mmu_new_pgd(vcpu, nested_cr3, new_role.base); } EXPORT_SYMBOL_GPL(kvm_init_shadow_npt_mmu); @@ -4922,27 +4921,25 @@ void kvm_init_shadow_ept_mmu(struct kvm_vcpu *vcpu, bool execonly, kvm_calc_shadow_ept_root_page_role(vcpu, accessed_dirty, execonly, level); - __kvm_mmu_new_pgd(vcpu, new_eptp, new_role.base); - - if (new_role.as_u64 == context->mmu_role.as_u64) - return; - - context->mmu_role.as_u64 = new_role.as_u64; + if (new_role.as_u64 != context->mmu_role.as_u64) { + context->mmu_role.as_u64 = new_role.as_u64; - context->shadow_root_level = level; + context->shadow_root_level = level; - context->ept_ad = accessed_dirty; - context->page_fault = ept_page_fault; - context->gva_to_gpa = ept_gva_to_gpa; - context->sync_page = ept_sync_page; - context->invlpg = ept_invlpg; - context->root_level = level; - context->direct_map = false; + context->ept_ad = accessed_dirty; + context->page_fault = ept_page_fault; + context->gva_to_gpa = ept_gva_to_gpa; + context->sync_page = ept_sync_page; + context->invlpg = ept_invlpg; + context->root_level = level; + context->direct_map = false; + update_permission_bitmask(context, true); + context->pkru_mask = 0; + reset_rsvds_bits_mask_ept(vcpu, context, execonly, huge_page_level); + reset_ept_shadow_zero_bits_mask(context, execonly); + } - update_permission_bitmask(context, true); - context->pkru_mask = 0; - reset_rsvds_bits_mask_ept(vcpu, context, execonly, huge_page_level); - reset_ept_shadow_zero_bits_mask(context, execonly); + __kvm_mmu_new_pgd(vcpu, new_eptp, new_role.base); } EXPORT_SYMBOL_GPL(kvm_init_shadow_ept_mmu); diff --git a/arch/x86/kvm/svm/nested.c b/arch/x86/kvm/svm/nested.c index f284e61451c8..96bab464967f 100644 --- a/arch/x86/kvm/svm/nested.c +++ b/arch/x86/kvm/svm/nested.c @@ -492,14 +492,14 @@ static int nested_svm_load_cr3(struct kvm_vcpu *vcpu, unsigned long cr3, CC(!load_pdptrs(vcpu, cr3))) return -EINVAL; - if (!nested_npt) - kvm_mmu_new_pgd(vcpu, cr3); - vcpu->arch.cr3 = cr3; /* Re-initialize the MMU, e.g. to pick up CR4 MMU role changes. */ kvm_init_mmu(vcpu); + if (!nested_npt) + kvm_mmu_new_pgd(vcpu, cr3); + return 0; } diff --git a/arch/x86/kvm/vmx/nested.c b/arch/x86/kvm/vmx/nested.c index 29289ecca223..abfcd71f787f 100644 --- a/arch/x86/kvm/vmx/nested.c +++ b/arch/x86/kvm/vmx/nested.c @@ -1126,15 +1126,15 @@ static int nested_vmx_load_cr3(struct kvm_vcpu *vcpu, unsigned long cr3, return -EINVAL; } - if (!nested_ept) - kvm_mmu_new_pgd(vcpu, cr3); - vcpu->arch.cr3 = cr3; kvm_register_mark_dirty(vcpu, VCPU_EXREG_CR3); /* Re-initialize the MMU, e.g. to pick up CR4 MMU role changes. */ kvm_init_mmu(vcpu); + if (!nested_ept) + kvm_mmu_new_pgd(vcpu, cr3); + return 0; }
Now that __kvm_mmu_new_pgd does not look at the MMU's root_level and shadow_root_level anymore, pull the PGD load after the initialization of the shadow MMUs. Besides being more intuitive, this enables future simplifications and optimizations because it's not necessary anymore to compute the role outside kvm_init_mmu. In particular, kvm_mmu_reset_context was not attempting to use a cached PGD to avoid having to figure out the new role. It will soon be able to follow what nested_{vmx,svm}_load_cr3 are doing, and avoid unloading all the cached roots. Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> --- arch/x86/kvm/mmu/mmu.c | 37 +++++++++++++++++-------------------- arch/x86/kvm/svm/nested.c | 6 +++--- arch/x86/kvm/vmx/nested.c | 6 +++--- 3 files changed, 23 insertions(+), 26 deletions(-)