Re: [RFC v1 25/26] x86/tdx: Make DMA pages shared

From: Sean Christopherson <seanjc@google.com>
To: Dave Hansen <dave.hansen@intel.com>
Cc: "Kirill A. Shutemov" <kirill@shutemov.name>,
	Kuppuswamy Sathyanarayanan 
	<sathyanarayanan.kuppuswamy@linux.intel.com>,
	Peter Zijlstra <peterz@infradead.org>,
	Andy Lutomirski <luto@kernel.org>,
	Andi Kleen <ak@linux.intel.com>,
	Kirill Shutemov <kirill.shutemov@linux.intel.com>,
	Kuppuswamy Sathyanarayanan <knsathya@kernel.org>,
	Dan Williams <dan.j.williams@intel.com>,
	Raj Ashok <ashok.raj@intel.com>,
	linux-kernel@vger.kernel.org,
	Kai Huang <kai.huang@linux.intel.com>,
	Sean Christopherson <sean.j.christopherson@intel.com>
Subject: Re: [RFC v1 25/26] x86/tdx: Make DMA pages shared
Date: Tue, 6 Apr 2021 17:16:02 +0000	[thread overview]
Message-ID: <YGyXUkl4ftKi673H@google.com> (raw)
In-Reply-To: <6065a93b-6bcd-2038-e994-6ec7b7b4c891@intel.com>

On Tue, Apr 06, 2021, Dave Hansen wrote:
> On 4/6/21 9:31 AM, Kirill A. Shutemov wrote:
> > On Thu, Apr 01, 2021 at 02:01:15PM -0700, Dave Hansen wrote:
> >>> @@ -1999,7 +2006,8 @@ static int __set_memory_enc_dec(unsigned long addr, int numpages, bool enc)
> >>>  	/*
> >>>  	 * Before changing the encryption attribute, we need to flush caches.
> >>>  	 */
> >>> -	cpa_flush(&cpa, !this_cpu_has(X86_FEATURE_SME_COHERENT));
> >>> +	if (!enc || !is_tdx_guest())
> >>> +		cpa_flush(&cpa, !this_cpu_has(X86_FEATURE_SME_COHERENT));
> >>
> >> That "!enc" looks wrong to me.  Caches would need to be flushed whenever
> >> encryption attributes *change*, not just when they are set.
> >>
> >> Also, cpa_flush() flushes caches *AND* the TLB.  How does TDX manage to
> >> not need TLB flushes?
> > 
> > I will double-check everthing, but I think we can skip *both* cpa_flush()
> > for private->shared conversion. VMM and TDX module will take care about
> > TLB and cache flush in response to MapGPA TDVMCALL.

No, on both accounts.

The guest is always responsible for flushing so called "linear mappings", i.e.
the gva -> gpa translations.  The VMM / TDX Module are responsible for flushing
the "guest-physical mappings" and "combined mappings" when the shared EPT /
secure EPT tables are modified.  E.g. the VMM could choose to keep separate
memory pools for shared vs. private and not even touch EPT tables on conversion.
But, the guest would still need to invalidate its virt->phys translations so
that accesses from within the guest generate the correct gpa.

Regarding cache flushing, the guest is responsible for flushing the cache lines
when converting from private to shared, and the VMM is responsible for flushing
the cache lines when converting from shared to private.

For private->shared, the VMM _can't_ do a targeted flush, as it can't generate
the correct physical address since stuffing a private key into its page tables
will #PF.  The VMM could do a full WBINVD, but that's not the intended ABI.
Hopefully this is documented in the GHCI...

For shared->private, the VMM is responsible for flushing the caches, assuming it
reuses the same physical page.  The TDX module does not enforce this directly,
rather TDX relies on integrity checks to detect if stale data (with the shared
key) is written back to guest private memory.  I.e. if the VMM does not do the
necessary flushing, the guest will get a poisoned memory #MC and die (or crash
the host).

> Oh, interesting.  You might also want to double check if there are any
> more places where X86_FEATURE_SME_COHERENT and TDX have similar properties.