Re: [PATCH v8 15/16] x86/virt/tdx: Flush cache in kexec() when TDX is enabled

[Dave Hansen]

On 1/10/23 03:29, Huang, Kai wrote:
> On Fri, 2023-01-06 at 16:35 -0800, Dave Hansen wrote:
>> On 12/8/22 22:52, Kai Huang wrote:
...
>>> However, this implementation doesn't convert TDX private pages back to
>>> normal in kexec() because of below considerations:
>>>
>>> 1) Neither the kernel nor the TDX module has existing infrastructure to
>>>    track which pages are TDX private pages.
>>> 2) The number of TDX private pages can be large, and converting all of
>>>    them (cache flush + using MOVDIR64B to clear the page) in kexec() can
>>>    be time consuming.
>>> 3) The new kernel will almost only use KeyID 0 to access memory.  KeyID
>>>    0 doesn't support integrity-check, so it's OK.
>>> 4) The kernel doesn't (and may never) support MKTME.  If any 3rd party
>>>    kernel ever supports MKTME, it can/should do MOVDIR64B to clear the
>>>    page with the new MKTME KeyID (just like TDX does) before using it.
>>
>> Yeah, why are we getting all worked up about MKTME when there is not
>> support?
> 
> I am not sure whether we need to consider 3rd party kernel case?

No, we don't.

>> The only thing that matters here is dirty cacheline writeback.  There
>> are two things the kernel needs to do to mitigate that:
>>
>>  1. Stop accessing TDX private memory mappings
>>   1a. Stop making TDX module calls (uses global private KeyID)
>>   1b. Stop TDX guests from running (uses per-guest KeyID)
>>  2. Flush any cachelines from previous private KeyID writes
>>
>> There are a couple of ways we can do #2.  We do *NOT* need to convert
>> *ANYTHING* back to KeyID 0.  Page conversion doesn't even come into play
>> in any way as far as I can tell.
> 
> May I ask why?  When I was writing this patch I was not sure whether kexec()
> should give the new kernel a clean slate.  SGX driver doesn't EREMOVE all EPC
> during kexec() but depends on the new kernel to do that too, but I don't know
> what's the general guide of supporting kexec().

Think about it this way: kexec() is modifying persistent (across kexec)
state to get the system ready for the new kernel.  The caches are
persistent state.  Devices have persistent state.  Memory state persists
across kexec().  The memory integrity metadata persists.

What persistent state does a conversion to KeyID-0 affect?  It resets
the integrity metadata and the memory contents.

Kexec leaves memory contents in place and doesn't zero them, so memory
contents don't matter.  The integrity metadata also doesn't matter
because the memory will be used as KeyID-0 and that KeyID doesn't read
the integrity metadata.

What practical impact does a conversion back to KeyID-0 serve?  What
persistent state does it affect that matters?

>> I think you're also saying that since all CPUs go through this path and
>> there is no TDX activity between the WBINVD and the native_halt() that
>> 1a and 1b basically happen for "free" without needing to do theme
>> explicitly.
> 
> Yes.  Should we mention this part in changelog?

That would be nice.