| Message ID | 20220516232138.1783324-4-dmatlack@google.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | KVM: Extend Eager Page Splitting to the shadow MMU | expand |
On Mon, May 16, 2022, David Matlack wrote: > The argument @direct is vcpu->arch.mmu->root_role.direct, so just use > that. It's worth calling out that, unlike non-root page tables, it's impossible to have a direct root in an indirect MMU. I.e. provide a hint as to why there's a need to pass @direct in the first place. > Suggested-by: Lai Jiangshan <jiangshanlai@gmail.com> > Signed-off-by: David Matlack <dmatlack@google.com> > ---
On 6/16/22 20:47, Sean Christopherson wrote: >> The argument @direct is vcpu->arch.mmu->root_role.direct, so just use >> that. > It's worth calling out that, unlike non-root page tables, it's impossible to have > a direct root in an indirect MMU. I.e. provide a hint as to why there's a need to > pass @direct in the first place. > I suppose there's *no* need to pass direct? Also, there's the trivial (but less interesting) justification that kvm_mmu_load does if (vcpu->arch.mmu->root_role.direct) r = mmu_alloc_direct_roots(vcpu); else r = mmu_alloc_shadow_roots(vcpu); and those are the only callers of mmu_alloc_root. Paolo
On Wed, Jun 22, 2022, Paolo Bonzini wrote: > On 6/16/22 20:47, Sean Christopherson wrote: > > > The argument @direct is vcpu->arch.mmu->root_role.direct, so just use > > > that. > > It's worth calling out that, unlike non-root page tables, it's impossible to have > > a direct root in an indirect MMU. I.e. provide a hint as to why there's a need to > > pass @direct in the first place. > > > > I suppose there's *no* need to pass direct? Also, there's the trivial (but > less interesting) justification that kvm_mmu_load does > > if (vcpu->arch.mmu->root_role.direct) > r = mmu_alloc_direct_roots(vcpu); > else > r = mmu_alloc_shadow_roots(vcpu); > > and those are the only callers of mmu_alloc_root. Duh, you're right, grabbing root_role.direct in mmu_alloc_root() is much better.
diff --git a/arch/x86/kvm/mmu/mmu.c b/arch/x86/kvm/mmu/mmu.c index 34fb0cddff2b..a9d28bcabcbb 100644 --- a/arch/x86/kvm/mmu/mmu.c +++ b/arch/x86/kvm/mmu/mmu.c @@ -3370,8 +3370,9 @@ static int mmu_check_root(struct kvm_vcpu *vcpu, gfn_t root_gfn) } static hpa_t mmu_alloc_root(struct kvm_vcpu *vcpu, gfn_t gfn, gva_t gva, - u8 level, bool direct) + u8 level) { + bool direct = vcpu->arch.mmu->root_role.direct; struct kvm_mmu_page *sp; sp = kvm_mmu_get_page(vcpu, gfn, gva, level, direct, ACC_ALL); @@ -3397,7 +3398,7 @@ static int mmu_alloc_direct_roots(struct kvm_vcpu *vcpu) root = kvm_tdp_mmu_get_vcpu_root_hpa(vcpu); mmu->root.hpa = root; } else if (shadow_root_level >= PT64_ROOT_4LEVEL) { - root = mmu_alloc_root(vcpu, 0, 0, shadow_root_level, true); + root = mmu_alloc_root(vcpu, 0, 0, shadow_root_level); mmu->root.hpa = root; } else if (shadow_root_level == PT32E_ROOT_LEVEL) { if (WARN_ON_ONCE(!mmu->pae_root)) { @@ -3409,7 +3410,7 @@ static int mmu_alloc_direct_roots(struct kvm_vcpu *vcpu) WARN_ON_ONCE(IS_VALID_PAE_ROOT(mmu->pae_root[i])); root = mmu_alloc_root(vcpu, i << (30 - PAGE_SHIFT), - i << 30, PT32_ROOT_LEVEL, true); + i << 30, PT32_ROOT_LEVEL); mmu->pae_root[i] = root | PT_PRESENT_MASK | shadow_me_mask; } @@ -3533,7 +3534,7 @@ static int mmu_alloc_shadow_roots(struct kvm_vcpu *vcpu) */ if (mmu->cpu_role.base.level >= PT64_ROOT_4LEVEL) { root = mmu_alloc_root(vcpu, root_gfn, 0, - mmu->root_role.level, false); + mmu->root_role.level); mmu->root.hpa = root; goto set_root_pgd; } @@ -3579,7 +3580,7 @@ static int mmu_alloc_shadow_roots(struct kvm_vcpu *vcpu) } root = mmu_alloc_root(vcpu, root_gfn, i << 30, - PT32_ROOT_LEVEL, false); + PT32_ROOT_LEVEL); mmu->pae_root[i] = root | pm_mask; }
The argument @direct is vcpu->arch.mmu->root_role.direct, so just use that. Suggested-by: Lai Jiangshan <jiangshanlai@gmail.com> Signed-off-by: David Matlack <dmatlack@google.com> --- arch/x86/kvm/mmu/mmu.c | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-)