On Mon, Jul 12, 2021 at 10:02:31AM -0700, Ben Gardon wrote: > On Wed, Jun 30, 2021 at 2:48 PM David Matlack <dmatlack@xxxxxxxxxx> wrote: > > > > Acquire the RCU read lock in walk_shadow_page_lockless_begin and release > > it in walk_shadow_page_lockless_end when the TDP MMU is enabled. This > > should not introduce any functional changes but is used in the following > > commit to make fast_page_fault interoperate with the TDP MMU. > > > > Signed-off-by: David Matlack <dmatlack@xxxxxxxxxx> > > Reviewed-by: Ben Gardon <bgardon@xxxxxxxxxx> > > This as I understand this, we're just lifting the rcu_lock/unlock out > of kvm_tdp_mmu_get_walk, and then putting all the TDP MMU specific > stuff down a level under walk_shadow_page_lockless_begin/end and > get_walk. > > Instead of moving kvm_tdp_mmu_get_walk_lockless into get_walk, it > could also be open-coded as: > > walk_shadow_page_lockless_begin > if (is_tdp_mmu(vcpu->arch.mmu)) > leaf = kvm_tdp_mmu_get_walk(vcpu, addr, sptes, &root); > else > leaf = get_walk(vcpu, addr, sptes, &root); > walk_shadow_page_lockless_end > > in get_mmio_spte, since get_walk isn't used anywhere else. Then > walk_shadow_page_lockless_begin/end could also be moved up out of > get_walk instead of having to add a goto to that function. > I don't have a strong preference either way, but the above feels like > a slightly simpler refactor. I don't have a strong preference either way as well. I'd be happy to switch to your suggestion in v3.