Re: [PATCH v3 2/5] x86/kexec: do unconditional WBINVD for bare-metal in relocate_kernel()

["Huang, Kai" <kai.huang@intel.com>]

On 12/04/2024 2:25 am, Tom Lendacky wrote:
> On 4/10/24 17:55, Huang, Kai wrote:
>> On 11/04/2024 4:21 am, Tom Lendacky wrote:
>>> On 4/7/24 07:44, Kai Huang wrote:
> 
>>>> @@ -160,9 +160,13 @@ SYM_CODE_START_LOCAL_NOALIGN(identity_mapped)
>>>>        movq    %r9, %cr3
>>>>        /*
>>>> -     * If SME is active, there could be old encrypted cache line
>>>> -     * entries that will conflict with the now unencrypted memory
>>>> -     * used by kexec. Flush the caches before copying the kernel.
>>>> +     * The kernel could leave caches in incoherent state on SME/TDX
>>>> +     * capable platforms.  Just do unconditional WBINVD to avoid
>>>> +     * silent memory corruption to the new kernel for these platforms.
>>>> +     *
>>>> +     * But only do WBINVD for bare-metal because TDX guests and
>>>> +     * SEV-ES/SEV-SNP guests will get #VE which the kernel is unable
>>>> +     * to handle at this stage.
>>>
>>> Similar comment here about doing an unconditional WBINVD, but then
>>> qualifying that as only on guests. This is where talking about how
>>> exception handling has been torn down would be good.
>>>
>>
>> OK.
>>
>> Thinking again, also it might be a good idea to not lose the existing
>> comment for SME, because it somehow justifies why we do WBINVD _HERE_ I
>> suppose?
>>
>> How about below?
>>
>>       /*
>>        * The kernel could leave caches in incoherent state on SME/TDX
>>        * capable platforms.  Flush cache to avoid silent memory
>>        * corruption for these platforms.
>>        *
>>        * For SME, need to flush cache here before copying the kernel.
>>        * When it is active, there could be old encrypted cache line
>>        * entries that will conflict with the now unencrypted memory
>>        * used by kexec.
>>        *
>>        * Do WBINVD for bare-metal to cover both SME and TDX, as it's
>>        * not safe to do WBINVD for TDX and SEV-ES/SEV-SNP guests.
>>        * WBINVD results in exception (#VE or #VC) for these guests, and
>>        * at this stage kernel is not able to handle such exception any
>>        * more because the kernel has torn down IDT.
>>        */
> 
> Similar to my comment in the other patch, just adding something that
> indicates that the WBINVD isn't necessary for TDX and SEV-ES/SEV-SNP
> (and maybe guests in general) would help. Maybe something like:
> 
> 	 * Do WBINVD for bare-metal only to cover both SME and TDX. It
> 	 * isn't necessary to perform a WBINVD in a guest and performing
> 	 * one could result in an exception (#VE or #VC) for a TDX or
> 	 * SEV-ES/SEV-SNP guest that can crash the guest since, at this
> 	 * stage, the kernel has torn down the IDT.
> 

Ah sure.  I will replace the 3rd paragraph in my original comment with 
yours (assuming that is what you meant).

Thanks again for providing the feedback!