On Tue, Nov 30, 2021 at 4:45 PM Sean Christopherson <seanjc@xxxxxxxxxx> wrote: > > On Tue, Nov 30, 2021, David Matlack wrote: > > > I have a similar patch for the old MMU, but it was also replacing > > > shadow_root_level with shadow_root_role. I'll see if I can adapt it to > > > the TDP MMU, since the shadow_root_role is obviously the same for both. > > > > While I was writing this patch it got me wondering if we can do an > > even more general refactor and replace root_hpa and shadow_root_level > > with a pointer to the root kvm_mmu_page struct. But I didn't get a > > chance to look into it further. > > For TDP MUU, yes, as root_hpa == __pa(sp->spt) in all cases. For the legacy/full > MMU, not without additional refactoring since root_hpa doesn't point at a kvm_mmu_page > when KVM shadows a non-paging guest with PAE paging (uses pae_root), or when KVM > shadows nested NPT and the guest is using fewer paging levels that the host (uses > pml5_root or pml4_root). > > if (mmu->shadow_root_level == PT64_ROOT_5LEVEL) > mmu->root_hpa = __pa(mmu->pml5_root); > else if (mmu->shadow_root_level == PT64_ROOT_4LEVEL) > mmu->root_hpa = __pa(mmu->pml4_root); > else > mmu->root_hpa = __pa(mmu->pae_root); > > That's definitely a solvable problem, e.g. it wouldn't be a problem to burn a few > kvm_mmu_page for the special root. The biggest issue is probably the sheer amount > of code that would need to be updated. I do think it would be a good change, but > I think we'd want to do it in a release that isn't expected to have many other MMU > changes. Thanks for the explanation! I had a feeling this refactor would start getting hairy when I ventured outside of the TDP MMU. > > shadow_root_level can also be replaced by mmu_role.base.level. I've never bothered > to do the replacement because there's zero memory savings and it would undoubtedly > take me some time to retrain my brain :-)