Messages in this thread | | | Date | Fri, 6 Jan 2023 11:18:10 -0800 | From | David Matlack <> | Subject | Re: [RFC 00/14] KVM: x86/MMU: Formalize the Shadow MMU |
| |
On Wed, Dec 21, 2022 at 10:24:04PM +0000, Ben Gardon wrote: > This series makes the Shadow MMU a distinct part of the KVM x86 MMU, > implemented in separate files, with a defined interface to common code.
Overall I really like the end result.
While looking through I found a few more bits of code that should probably be moved into shadow_mmu.c:
- kvm_mmu_zap_all(): Move the shadow MMU zapping to shadow_mmu.c (the active_mmu_pages loop + commit_zap_page).
- need_topup(), need_topup_split_caches_or_resched() topup_split_caches() should be static functions in shadow_mmu.c.
- Split out kvm_mmu_init/uninit_vm() functions for the shadow MMU. Notably, the split caches, active_mmu_pages, zapped_obsolete_pages, and other Shadow MMU-specific stuff can go in shadow_mmu.c.
- The Shadow MMU parts of walk_shadow_page_lockless_begin/end() should go in shadow_mmu.c. e.g. kvm_shadow_mmu_walk_lockless_begin/end().
> Patch 3 is an enormous change, and doing it all at once in a single > commit all but guarantees merge conflicts and makes it hard to review. I > don't have a good answer to this problem as there's no easy way to move > 3.5K lines between files. I tried moving the code bit-by-bit but the > intermediate steps added complexity and ultimately the 50+ patches it > created didn't seem any easier to review. > Doing the big move all at once at least makes it easier to get past when > doing Git archeology, and doing it at the beggining of the series allows the > rest of the commits to still show up in Git blame.
An alternative would be to rename mmu.c to shadow_mmu.c first and then move code in the opposite direction. That would preserve the git-blame history for shadow_mmu.c. But by the end of the series mmu.c and shadow_mmu.c are both ~3K LOC, so I don't think doing this is really any better. Either way, you have to move ~3K LOC.
| |