Re: [PATCH v2 09/15] KVM: x86/tdp_mmu: Support mirror root for TDP MMU

[Edgecombe, Rick P]

On Sat, 2024-06-08 at 11:13 +0200, Paolo Bonzini wrote:
> > I was not loving the amount of indirection here in the patch, but thought it
> > centralized the logic a bit better. This way seems good, given that the
> > actual
> > logic is not that complex.
> 
> My proposed implementation is a bit TDX-specific though... Something
> like this is more agnostic, and it exploits nicely the difference
> between fault->addr and fault->gfn:
> 
> if (!kvm_gfn_direct_mask(kvm) ||
>     (gpa_to_gfn(fault->addr) & kvm_gfn_direct_mask(kvm))
>   root_hpa = vcpu->arch.mmu->root.hpa;
> else
>   root_hpa = vcpu->arch.mmu->mirror_root_hpa;
> return root_to_sp(root_hpa);

Agreed that this is less TDX specific and it means that this part of the generic
MMU code doesn't need to know that the mirror/direct matches to private vs
shared. I don't love that it has such a complicated conditional for the normal
VM case, though. Just for readability.

The previous versions checked kvm_gfn_shared_mask() more readily in various open
coded spots. In this v2 we tried to reduce this and instead always rely on
the "private" concept to switch between the roots in the generic code. I think
it's arguably a little easier to understand if we stick to a single way of
deciding which root to use.

But I don't feel like any of these solutions discussed is perfectly clean. So
I'm ok taking the benefits you prefer. I guess doing bitwise operations when
possible is kind of the KVM way, haha. :)