Re: [RFC v1 21/26] x86/mm: Move force_dma_unencrypted() to common code

[Dave Hansen]

On 4/6/21 8:37 AM, Kirill A. Shutemov wrote:
> On Thu, Apr 01, 2021 at 01:06:29PM -0700, Dave Hansen wrote:
>> On 2/5/21 3:38 PM, Kuppuswamy Sathyanarayanan wrote:
>>> From: "Kirill A. Shutemov" <kirill.shutemov@xxxxxxxxxxxxxxx>
>>>
>>> Intel TDX doesn't allow VMM to access guest memory. Any memory that is
>>> required for communication with VMM suppose to be shared explicitly by
>>
>> s/suppose to/must/
> 
> Right.
> 
>>> setting the bit in page table entry. The shared memory is similar to
>>> unencrypted memory in AMD SME/SEV terminology.
>>
>> In addition to setting the page table bit, there's also a dance to go
>> through to convert the memory.  Please mention the procedure here at
>> least.  It's very different from SME.
> 
> "
>   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.
> "

Where does the TDX module fit in here?

>>> force_dma_unencrypted() has to return true for TDX guest. Move it out of
>>> AMD SME code.
>>
>> You lost me here.  What does force_dma_unencrypted() have to do with
>> host/guest shared memory?
> 
> "
>   AMD SEV makes force_dma_unencrypted() return true which triggers
>   set_memory_decrypted() calls on all DMA allocations. TDX will use the
>   same code path to make DMA allocations shared.
> "

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.

TDX is similar.  TDX architecturally prevents access to private guest
memory by anything other than the guest itself.  This means that any DMA
buffers must be shared.

Right?

>>> Introduce new config option X86_MEM_ENCRYPT_COMMON that has to be
>>> selected by all x86 memory encryption features.
>>
>> Please also mention what will set it.  I assume TDX guest support will
>> set this option.  It's probably also worth a sentence to say that
>> force_dma_unencrypted() will have TDX-specific code added to it.  (It
>> will, right??)
> 
> "
>   Only AMD_MEM_ENCRYPT uses the option now. TDX will be the second one.
> "
> 
>>> This is preparation for TDX changes in DMA code.
>>
>> Probably best to also mention that this effectively just moves code
>> around.  This patch should have no functional changes at runtime.
> 
> Isn't it what the subject says? :P

Yes, but please mention it explicitly.