On Thu, Feb 03, 2022, David Matlack wrote: > Decompose kvm_mmu_get_page() into separate helper functions to increase > readability and prepare for allocating shadow pages without a vcpu > pointer. > > Specifically, pull the guts of kvm_mmu_get_page() into 3 helper > functions: > > kvm_mmu_get_existing_sp_mabye_unsync() - Heh, this ain't Java. Just add two underscores to whatever it's primary caller ends up being named; that succinctly documents the relationship _and_ suggests that there's some "danger" in using the inner helper. > Walks the page hash checking for any existing mmu pages that match the > given gfn and role. Does not attempt to synchronize the page if it is > unsync. > > kvm_mmu_get_existing_sp() - Meh. We should really be able to distill this down to something like kvm_mmu_find_sp(). I'm also tempted to say we go with shadow_page instead of "sp" for these helpers, so long as the line lengths don't get too brutal. KVM uses "sp" and "spte" in lots of places, but I suspect it would be helpful to KVM newbies if the core routines actually spell out shadow_page, a la to_shadow_page(). > Gets an existing page from the page hash if it exists and guarantees > the page, if one is returned, is synced. Implemented as a thin wrapper > around kvm_mmu_get_existing_page_mabye_unsync. Requres access to a vcpu > pointer in order to sync the page. > > kvm_mmu_create_sp() Probably prefer s/create/alloc to match existing terminology for allocating roots. Though looking through the series, there's going to be a lot of juggling of names. It probably makes sense to figure out what names we want to end up with and then work back from there. I'll be back next week for a proper bikeshed session. :-)