Re: [PATCH v19 067/130] KVM: TDX: Add load_mmu_pgd method for TDX

[Edgecombe, Rick P]

On Sun, 2024-04-07 at 09:32 +0800, Binbin Wu wrote:
> > Looks good.  Some nits though:
> > 
> > > KVM: TDX: Add load_mmu_pgd method for TDX
> > > 
> > > TDX has uses two EPT pointers, one for the private half of the GPA
> > "TDX uses"
> > 
> > > space and one for the shared half. The private half used the normal
> > "used" -> "uses"
> > 
> > > EPT_POINTER vmcs field and is managed in a special way by the TDX module.
> > Perhaps add:
> > 
> > KVM is not allowed to operate on the EPT_POINTER directly.
> > 
> > > The shared half uses a new SHARED_EPT_POINTER field and will be managed by
> > > the conventional MMU management operations that operate directly on the
> > > EPT tables.
> > > 
> > I would like to explicitly call out KVM can update SHARED_EPT_POINTER directly:
> > 
> > The shared half uses a new SHARED_EPT_POINTER field.  KVM is allowed to set it
> > directly by the interface provided by the TDX module, and KVM is expected to
> > manage the shared half just like it manages the existing EPT page table today.
> > 
> > 
> > > This means for TDX the .load_mmu_pgd() operation will need to
> > > know to use the SHARED_EPT_POINTER field instead of the normal one. Add a
> > > new wrapper in x86 ops for load_mmu_pgd() that either directs the write to
> > > the existing vmx implementation or a TDX one.
> > > 
> > > For the TDX operation, EPT will always be used, so it can simpy write to
> 
> 
> Maybe remove "so"?  IMO, there is no causal relationship between the 
> first and second half of the sentence.
> 

I was trying to nod at why tdx_load_mmu_pgd() is so much simpler than vmx_load_mmu_pgd(). Here is a
new version with all the feedback:

KVM: TDX: Add load_mmu_pgd method for TDX

TDX uses two EPT pointers, one for the private half of the GPA space and one for the shared half.
The private half uses the normal EPT_POINTER vmcs field, which is managed in a special way by the
TDX module. For TDX, KVM is not allowed to operate on it directly. The shared half uses a new
SHARED_EPT_POINTER field and will be managed by the conventional MMU management operations that
operate directly on the EPT root. This means for TDX the .load_mmu_pgd() operation will need to know
to use the SHARED_EPT_POINTER field instead of the normal one. Add a new wrapper in x86 ops for
load_mmu_pgd() that either directs the write to the existing vmx implementation or a TDX one.

For the TDX mode of operation, EPT will always be used and KVM does not need to be involved in
virtualization of CR3 behavior. So tdx_load_mmu_pgd() can simply write to SHARED_EPT_POINTER.