On Thu, Jan 26, 2023, David Matlack wrote:
> Replace sp->tdp_mmu_page with a bit in the page role. This reduces the
> size of struct kvm_mmu_page by a byte.
No it doesn't. I purposely squeezed the flag into an open byte in commit
ca41c34cab1f ("KVM: x86/mmu: Relocate kvm_mmu_page.tdp_mmu_page for better cache locality")
I double checked just to make sure: the size is 184 bytes before and after.
I'm not opposed to this change, but I also don't see the point. The common code
ends up with an arch hook in the appropriate place anyways[*], and I think we'll
want to pay careful attention to the cache locality of the struct as whole, e.g.
naively dumping the arch crud at the end of the common kvm_mmu_page structure may
impact performance, especially for shadow paging.
And just drop the WARN_ON() sanity check in kvm_tdp_mmu_put_root() .
Hmm, actually, if we invert the order for the shadow MMU, e.g. embed "struct
kvm_mmu_page" in a "struct kvm_shadow_mmu_page" or whatever, then the size of
TDP MMU pages should shrink substantially.
So my vote is to hold off for now and take a closer look at this in the common
MMU series proper.
[*] https://lore.kernel.org/all/20221208193857.4090582-20-dmatlack@xxxxxxxxxx
> Note that in tdp_mmu_init_sp() there is no need to explicitly set
> sp->role.tdp_mmu=1 for every SP since the role is already copied to all
> child SPs.
>
> Suggested-by: Paolo Bonzini <pbonzini@xxxxxxxxxx>
> Link: https://lore.kernel.org/kvm/b0e8eb55-c2ee-ce13-8806-9d0184678984@xxxxxxxxxx/
Drop the trailing slash, otherwise directly clicking the link goes sideways.
> Signed-off-by: David Matlack <dmatlack@xxxxxxxxxx>
> ---
