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

From: Binbin Wu
Date: Sat Apr 06 2024 - 21:32:48 EST




On 4/6/2024 8:58 AM, Huang, Kai wrote:
On Sat, 2024-04-06 at 00:09 +0000, Edgecombe, Rick P wrote:
On Wed, 2024-04-03 at 10:33 -0700, Isaku Yamahata wrote:
On Mon, Apr 01, 2024 at 11:49:43PM +0800,
Binbin Wu <binbin.wu@xxxxxxxxxxxxxxx> wrote:


On 2/26/2024 4:26 PM, isaku.yamahata@xxxxxxxxx wrote:
From: Sean Christopherson <sean.j.christopherson@xxxxxxxxx>

For virtual IO, the guest TD shares guest pages with VMM without
encryption.
Virtual IO is a use case of shared memory, it's better to use it
as a example instead of putting it at the beginning of the sentence.


   Shared EPT is used to map guest pages in unprotected way.

Add the VMCS field encoding for the shared EPTP, which will be used by
TDX to have separate EPT walks for private GPAs (existing EPTP) versus
shared GPAs (new shared EPTP).

Set shared EPT pointer value for the TDX guest to initialize TDX MMU.
May have a mention that the EPTP for priavet GPAs is set by TDX module.
Sure, let me update the commit message.
How about this?
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.

Typo, "simpy" -> "simply"

the SHARED_EPT_POINTER field.