Received-SPF: pass (google.com: domain of linux-kernel+bounces-141654-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.199.223 as permitted sender) client-ip=147.75.199.223;
Message-ID: <f53ab4dc-20b6-4ada-83dc-89e2f7e8307a@intel.com>
Date: Fri, 12 Apr 2024 09:59:04 +1200
User-Agent: Mozilla Thunderbird
Subject: Re: [PATCH v3 2/5] x86/kexec: do unconditional WBINVD for bare-metal
 in relocate_kernel()
To: Tom Lendacky <thomas.lendacky@amd.com>, "linux-kernel@vger.kernel.org"
	<linux-kernel@vger.kernel.org>
CC: "x86@kernel.org" <x86@kernel.org>, "Hansen, Dave" <dave.hansen@intel.com>,
	"bp@alien8.de" <bp@alien8.de>, "kirill.shutemov@linux.intel.com"
	<kirill.shutemov@linux.intel.com>, "tglx@linutronix.de" <tglx@linutronix.de>,
	"mingo@redhat.com" <mingo@redhat.com>, "hpa@zytor.com" <hpa@zytor.com>,
	"luto@kernel.org" <luto@kernel.org>, "peterz@infradead.org"
	<peterz@infradead.org>, "Edgecombe, Rick P" <rick.p.edgecombe@intel.com>,
	"ashish.kalra@amd.com" <ashish.kalra@amd.com>, "Gao, Chao"
	<chao.gao@intel.com>, "bhe@redhat.com" <bhe@redhat.com>,
	"nik.borisov@suse.com" <nik.borisov@suse.com>, "pbonzini@redhat.com"
	<pbonzini@redhat.com>, "seanjc@google.com" <seanjc@google.com>
References: <cover.1712493366.git.kai.huang@intel.com>
 <5aca124fe0ca1c97bf4a2e925b3905485bf5b255.1712493366.git.kai.huang@intel.com>
 <f361d574-11d6-3730-c77f-f3aabe111454@amd.com>
 <a5948fe2-afbb-4b8c-a83c-0be55a823c53@intel.com>
 <533c2f6a-9cb2-6d25-c8a0-0643c6862d9b@amd.com>
Content-Language: en-US
From: "Huang, Kai" <kai.huang@intel.com>
In-Reply-To: <533c2f6a-9cb2-6d25-c8a0-0643c6862d9b@amd.com>
Content-Type: text/plain; charset="UTF-8"; format=flowed
Content-Transfer-Encoding: 8bit
Precedence: bulk
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?QUpmekU3aG5telhCUGVOeVI5RlBrb3FGUmR5YjFLa2xHaXE2SFVVM0dmNlBR?=
 =?utf-8?B?TmkxUDZLRmhFWVNYbHBhK216SWJnQmRzWklBWnRlbkFDN1VsT0VtRzZCbmtY?=
 =?utf-8?B?Y1RVT2drRjJwL3gvTGpQNTJmeHJramV4SzNySmRuRytsdlVHcE9kTTdET29m?=
 =?utf-8?B?NThLNTU4QzNpMjRLVmhlQkFVOWd1SmdrYnhtN3J3OVhvdlB2ZDZiWU4wNXFn?=
 =?utf-8?B?eFMrc05OeWlwT2U2VnkvZG1talRzZjVHdUhpd2xUd2dQMWRsQnZWazRrWGkr?=
 =?utf-8?B?VExCYnI0ajlXTGladVVwZnlsY1lwcTBlVmlxVWJOT3BWcWJ2VnFjWDlCekdl?=
 =?utf-8?B?YW9vZDdwd2I0ODBVdzY0K3MvZENXM21SZ0dSd2ZaeU9ZcGM4Z3ZORUJnd25I?=
 =?utf-8?B?VEk0QXlObFZsV1U0N1hRVWZKV1VNVUJ4ZlVDRndjVUJVUmdXTkFWMWNlTFFH?=
 =?utf-8?B?c2dFL2hZdGpQTGRMbTVKVlZncWY3MGRnOXNPb1g5MGxXc01MTGpwNCt2K3Nz?=
 =?utf-8?B?UGY5T2ZsOUlGcjdWNmp4US9JWksxb0tsRlJXN3dIMWpaMUw1c2JqUDA0M0di?=
 =?utf-8?B?WmhzdUtrQXZYdGFYVmlqT014NzA0c0Y1b3llTDhqeUpyYXpmc3NFK1F3MStJ?=
 =?utf-8?B?NDBRNGNrdEI1SDFOcnJnTTV5RjZQTkg0NDJzcUc0M1ZzYms4SlRlZkZRS1c5?=
 =?utf-8?B?MENSYzNEYVpiTUM3QzNnS3BxMVl1a1J4WFEvTlhXajFLZGZZbnFCVHNNVzNa?=
 =?utf-8?B?bXJXSlBNWWFzY05BK2ozZkhUemhaMWFoWnZPTlJVcm5LV1lVaEZraWZsTEQw?=
 =?utf-8?B?UHU2b2Q0YXNCOHBCaUFtaEhKT2hBK3ByWU5RQUVEVWZKbXU4WHZJU3lZbTQx?=
 =?utf-8?B?MnVXNVpFZVRvZTk5czhFY1ZCQUcxdGhZWm5iS0hhSmV5aVJQY2ZUZDlud0pN?=
 =?utf-8?B?aTkrL1p6cE1zUG8zWS9lUGZvRG0rNS8vaEoyTlkyTmQ0dncxUmhoYU82cStN?=
 =?utf-8?B?b1dvZTh4SmxJbUx5M2FrZ1VhMHQyZ0V6NmovRjdwSEUxeDBvR21FWnQ1bDVL?=
 =?utf-8?B?VGNlaTFHeWxMN2VwamRJaDNvV1BvOWRGcE9Za0pOdFZrcStjVzZqQWpXdjBq?=
 =?utf-8?B?MHBYa3BhZW16dnZKSXZNUHRwZUNsV1pUMUMzZ1ZJaXRvZHhaaDRvT3p0TjFp?=
 =?utf-8?B?bFhiQjFRV2NoRE1yaDdsVGllY2ZLdEc3R1BaZVhXbEkvNU5UQzFKRnh3NWZY?=
 =?utf-8?B?QzhkOTFnZXNRdHZPTzA2aE1DMy9ZcWt3TjFNN09JT0xBWE9FOUpJZUVLUFZw?=
 =?utf-8?B?ZGNCSHJHQ2hVODVFeDdDK09WRlRiYTVYbUw1QXFCOStqRVVHVk9sdk5DVE9V?=
 =?utf-8?B?M0hBTmRYOS96a0JMdmxXM0g0SW9mS2gzQmY2QnljOU5zaGJNMzVUTFZydTVC?=
 =?utf-8?B?R2loMnVGUmczTkMwY3JPUFN6ZzltcVFTaUY4eXhveHliMk51RUVLY09CQmhT?=
 =?utf-8?B?NmtwYnlBeGhRUG12S0VudGUyL0FUMnRuOFQvVDFUa0NMRVBiSUtBUlY0bDI1?=
 =?utf-8?B?Nmh6amRMUG5XTDZoODIySG5nQ2s3Zis0bmM5Vnh5ZzJwSm4yRGp3d3B4V1d0?=
 =?utf-8?B?VS9iaU9LaXNxY3oyUWVFR2R2OVBYS3JFckVrSnY3MjJCQmJmd0VJTmsyeVQx?=
 =?utf-8?B?UzhSa3BibjJZeitQeENQYkY0NGNBdURxb3RWKzFWb2FBeUk0Ly84eDlMZGxE?=
 =?utf-8?B?NGRpTG1HQi9FdEFRd29KSHJRSmJZM0EwQUZXU2dBRDV1U2ZUQXI0VzV5R2Z1?=
 =?utf-8?B?U1BiVkhjaFh0RWF6ajlTWjRKK2pnTVZIbzNxRGs5M2FQL2hmdTJVNTMrK3gr?=
 =?utf-8?B?cmlvL0oyaTJsTW9BRU04TkdDN3VFZVh6ZmQ2cE5mbW1OT3VCcGNKTkdKMjhV?=
 =?utf-8?B?MWh4MWNwR3IycVh0bmpUQko0aVBwYkw0cTYrWElnR2QvejFJWWxpbnZkKzNO?=
 =?utf-8?B?NlU2OE5MUFA1SmtYdzVBcklKVHRXM1huaFg3czRFTkY2VDFXeWJIV202WDV1?=
 =?utf-8?B?N0R1QXIxd01Na0d3L3V6eVR3N2VhQzM3MmdPMyt4K0ltT0hLamNLT3dYZlE2?=
 =?utf-8?Q?BZJkG2CYj2gNGQmTZ/rpQ4lfu?=
X-MS-Exchange-CrossTenant-Network-Message-Id: 034d0a6c-7495-419b-85ad-08dc5a72a272
X-MS-Exchange-CrossTenant-AuthSource: BL1PR11MB5978.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 11 Apr 2024 21:59:17.9276
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 46c98d88-e344-4ed4-8496-4ed7712e255d
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: 9DPGScZAxZOh1XEJ1PmA1asVVdquWnsVgRo30K0cQ+SQ/W068lI/gv53Sbl+eBboeMgE31XjqCK7D23vbnJZbw==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: BY1PR11MB8080
X-OriginatorOrg: 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!