Re: [PATCH 08/16] KVM: x86/mmu: Bug the VM if kvm_zap_gfn_range() is called for TDX

["Edgecombe, Rick P" <rick.p.edgecombe@intel.com>]

On Thu, 2024-05-16 at 11:38 +1200, Huang, Kai wrote:
> 
> 
> On 16/05/2024 11:14 am, Edgecombe, Rick P wrote:
> > On Thu, 2024-05-16 at 10:17 +1200, Huang, Kai wrote:
> > > > TDX has several aspects related to the TDP MMU.
> > > > 1) Based on the faulting GPA, determine which KVM page table to walk.
> > > >        (private-vs-shared)
> > > > 2) Need to call TDX SEAMCALL to operate on Secure-EPT instead of direct
> > > > memory
> > > >        load/store.  TDP MMU needs hooks for it.
> > > > 3) The tables must be zapped from the leaf. not the root or the middle.
> > > > 
> > > > For 1) and 2), what about something like this?  TDX backend code will
> > > > set
> > > > kvm->arch.has_mirrored_pt = true; I think we will use
> > > > kvm_gfn_shared_mask()
> > > > only
> > > > for address conversion (shared<->private).
> > 
> > 1 and 2 are not the same as "mirrored" though. You could have a design that
> > mirrors half of the EPT and doesn't track it with separate roots. In fact, 1
> > might be just a KVM design choice, even for TDX.
> 
> I am not sure whether I understand this correctly.  If they are not 
> tracked with separate roots, it means they use the same page table (root).

There are three roots, right? Shared, private and mirrored. Shared and mirrored
don't have to be different roots, but it makes some operations arguably easier
to have it that way.

> 
> So IIUC what you said is to support "mirror PT" at any sub-tree of the 
> page table?
> 
> That will only complicate things.  I don't think we should consider 
> this.  In reality, we only have TDX and SEV-SNP.  We should have a 
> simple solution to cover both of them.

Look at "bool is_private" in kvm_tdp_mmu_map(). Do you see how it switches
between different roots in the iterator? That is one use.

The second use is to decide whether to call out to the x86_ops. It happens via
the role bit in the sp, which is copied from the parent sp role. The root's bit
is set originally via a kvm_gfn_shared_mask() check.

BTW, the role bit is the thing I'm wondering if we really need, because we have
shared_mask. While the shared_mask is used for lots of things today, we need
still need it for masking GPAs. Where as the role bit is only needed to know if
a SP is for private (which we can tell from the GPA).