Re: [RFC v2-fix 1/1] x86/tdx: Make DMA pages shared

[Sean Christopherson]

On Mon, May 17, 2021, Kuppuswamy Sathyanarayanan wrote:
> From: "Kirill A. Shutemov" <kirill.shutemov@xxxxxxxxxxxxxxx>
> 
> Intel TDX doesn't allow VMM to access guest memory. Any memory
                                             ^
                                             |- private

And to be pedantic, the VMM can _access_ guest private memory all it wants, it
just can't decrypt guest private memory.

> that is required for communication with VMM must be shared
> explicitly by setting the bit in page table entry. And, after
> setting the shared bit, the conversion must be completed with
> MapGPA TDVMALL. The call informs VMM about the conversion and
> makes it remove the GPA from the S-EPT mapping.

The VMM is _not_ required to remove the GPA from the S-EPT.  E.g. if the VMM
wants to, it can leave a 2mb private page intact and create a 4kb shared page
translation within the same range (ignoring the shared bit).

> The shared memory is similar to unencrypted memory in AMD SME/SEV
> terminology but the underlying process of sharing/un-sharing the memory is
> different for Intel TDX guest platform.
> 
> SEV assumes that I/O devices can only do DMA to "decrypted"
> physical addresses without the C-bit set.  In order for the CPU
> to interact with this memory, the CPU needs a decrypted mapping.
> To add this support, AMD SME code forces force_dma_unencrypted()
> to return true for platforms that support AMD SEV feature. It will
> be used for DMA memory allocation API to trigger
> set_memory_decrypted() for platforms that support AMD SEV feature.
> 
> TDX is similar.  TDX architecturally prevents access to private

TDX doesn't prevent accesses.  If hardware _prevented_ accesses then we wouldn't
have to deal with the #MC mess.

> guest memory by anything other than the guest itself. This means
> that any DMA buffers must be shared.
> 
> So create a new file mem_encrypt_tdx.c to hold TDX specific memory
> initialization code, and re-define force_dma_unencrypted() for
> TDX guest and make it return true to get DMA pages mapped as shared.
> 
> __set_memory_enc_dec() is now aware about TDX and sets Shared bit
> accordingly following with relevant TDVMCALL.
> 
> Also, Do TDACCEPTPAGE on every 4k page after mapping the GPA range when

This should call out that the current TDX spec only supports 4kb AUG/ACCEPT.

On that topic... are there plans to support 2mb and/or 1gb TDH.MEM.PAGE.AUG?  If
so, will TDG.MEM.PAGE.ACCEPT also support 2mb/1gb granularity?

> converting memory to private.  If the VMM uses a common pool for private
> and shared memory, it will likely do TDAUGPAGE in response to MAP_GPA
> (or on the first access to the private GPA),

What the VMM does or does not do is irrelevant.  What matters is what the VMM is
_allowed_ to do without violating the GHCI.  Specifically, the VMM is allowed to
unmap a private page in response to MAP_GPA to convert to a shared page.

  If the GPA (range) was already mapped as an active, private page, the host
  VMM may remove the private page from the TD by following the “Removing TD
  Private Pages” sequence in the Intel TDX-module specification [3] to safely
  block the mapping(s), flush the TLB and cache, and remove the mapping(s).

That would also provide a nice segue into the "already accepted" error below.

> in which case TDX-Module will hold the page in a non-present "pending" state
> until it is explicitly accepted.
> 
> BUG() if TDACCEPTPAGE fails (except the above case)

What above case?  The code handles the case where the page was already accepted,
but the changelog doesn't talk about that at all.  

> as the guest is completely hosed if it can't access memory.