Received-SPF: pass (google.com: domain of linux-kernel+bounces-130648-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.48.161 as permitted sender) client-ip=147.75.48.161;
Message-ID: <8e4e1d80-11ef-472f-befc-f61310281f18@intel.com>
Date: Thu, 4 Apr 2024 10:40:34 +1300
User-Agent: Mozilla Thunderbird
Subject: Re: [PATCHv9 05/17] x86/kexec: Keep CR4.MCE set during kexec for TDX
 guest
To: "Kirill A. Shutemov" <kirill.shutemov@linux.intel.com>, Xiaoyao Li
	<xiaoyao.li@intel.com>
CC: Thomas Gleixner <tglx@linutronix.de>, Ingo Molnar <mingo@redhat.com>,
	Borislav Petkov <bp@alien8.de>, Dave Hansen <dave.hansen@linux.intel.com>,
	<x86@kernel.org>, "Rafael J. Wysocki" <rafael@kernel.org>, Peter Zijlstra
	<peterz@infradead.org>, Adrian Hunter <adrian.hunter@intel.com>, "Kuppuswamy
 Sathyanarayanan" <sathyanarayanan.kuppuswamy@linux.intel.com>, Elena
 Reshetova <elena.reshetova@intel.com>, Jun Nakajima <jun.nakajima@intel.com>,
	"Rick Edgecombe" <rick.p.edgecombe@intel.com>, Tom Lendacky
	<thomas.lendacky@amd.com>, "Kalra, Ashish" <ashish.kalra@amd.com>, "Sean
 Christopherson" <seanjc@google.com>, Baoquan He <bhe@redhat.com>,
	<kexec@lists.infradead.org>, <linux-coco@lists.linux.dev>,
	<linux-kernel@vger.kernel.org>
References: <20240325103911.2651793-1-kirill.shutemov@linux.intel.com>
 <20240325103911.2651793-6-kirill.shutemov@linux.intel.com>
 <b94c81ef-50f2-4e66-9533-461791777d10@intel.com>
 <dlixf7bld4efdh5goir5nreabkgtcwfbvv2rovjqrvjttplqss@auuqnt4h2llj>
 <j4s4n5slt6a234w5o5r4zjmb3xj53odnmwekt6ihotfzotd7ur@ghyvphwcki57>
Content-Language: en-US
From: "Huang, Kai" <kai.huang@intel.com>
In-Reply-To: <j4s4n5slt6a234w5o5r4zjmb3xj53odnmwekt6ihotfzotd7ur@ghyvphwcki57>
Content-Type: text/plain; charset="UTF-8"; format=flowed
Content-Transfer-Encoding: 7bit
Precedence: bulk
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?OENlVVVtOXBNZDNsRW5TaGt2am1PZEgwN3p1TXZ6dkZZVE5WdFJqYndlbXRt?=
 =?utf-8?B?SGwzdzRGdzVxa0JVL3dpYlVKRFNDdFJFb3pZMzlXdmRXMUYxN3NPRHJGU2RC?=
 =?utf-8?B?U0t2VFl2NXpzcVFMaXBadUh3VUMrS29IUFN6a1BKSXJWazZtWExQNDJ5VlRu?=
 =?utf-8?B?Qk8ycmg0VnlQYkpycm80OGFyRGltVGlGVFNwRU9kRzZ4bmRsRDV4eHAzTHJP?=
 =?utf-8?B?VkYxVXZLZFUzM3hNZk96Y0w3S1p5TVk5cG14OG4zcy9sQjRiV1pCT0lsVzlI?=
 =?utf-8?B?dXg0czZWcFJWM1NNSVQyVjE1alJzdGhtbXNRVEF5Wk9keVF0dWNCVFhiczBH?=
 =?utf-8?B?Sm5SMC9lenBVSFlrZFZHZ05RU3B6cXRhZmR5N01xbVM2SDdubUdxUWJNMjZK?=
 =?utf-8?B?d3BFYkVrTnFtS21OdUxCalpPd0lFTlJyVEV5SGNyTTlKaG1VaGZLZVRuMi9R?=
 =?utf-8?B?Q2pYN1dKZTdkVm0yTkE4Y2RMS1EwaHB4UUM1VkcvY3BQRjdJb1hla0FtMkFM?=
 =?utf-8?B?Nms3QXc3ZDlQSEJXR1dHcGVLSXJCcUFDakhSL2xyZXRzdFEwKzlhKzNsUStU?=
 =?utf-8?B?OTcvZEZ6WDY0Q3ZxT1dHb3JGNFJIV21sZVlYRmFibGZDcWZ6eHRLS0x2RVlJ?=
 =?utf-8?B?Tk4zcXUwQXZVWE5MbEY1OVlxbGFIcUk5N0Y3OGo2c0IxczdIRFVKWXRNWUxz?=
 =?utf-8?B?elJISExQV0Z3d0FMa2VrYWxPMlZJU01sSTBUZGd0UzZBSnJRSXYrQW9lQnBz?=
 =?utf-8?B?dUdUcmhxenlwc1ZQY0JSSVJxdVZxdnE5U0o3M0k4QjRUTWRYMGNXYmw3bk1M?=
 =?utf-8?B?VTg0QVBaZHBLZFpySjdlT1JmS2s2ZHRVOGYwT2pvT3lMd3NBUDVKeHVBNmY3?=
 =?utf-8?B?c3BMME11Zmw0QlVmOWJtczB5bzIrdVR3eW8zL2k4ZXNuUkczRTBnNURsK0J6?=
 =?utf-8?B?eUlOS0pnVXFRSjdDL1lyRlhoMFJzWEowTFJZamJiQzdyNTBkS2Z2eW9qcEc5?=
 =?utf-8?B?Q0M2M3hBMmk1QnE4N2pnZms5U3NWY0JSNnpCSVZWK2drdXRod28vV0FnVHpM?=
 =?utf-8?B?T0hGeUxRSnBWS29Xc3BMREpSd3cyeCtRT1d6bk8vVXQ0Z0RSZDlxRjVybFcv?=
 =?utf-8?B?S2lVcG0yL3ZJZDAyaWZYdjZiYkVPbVVPTUNVeEIycHc0ejZDTmZOWGZQcnRQ?=
 =?utf-8?B?TXJ5L3lWWm1pTklRc2RaTXdiQzVucnZhNDMyak5QZys2UW9QRDgwVUsvNmZM?=
 =?utf-8?B?Nm1zbWpGU0tsQzVqU1Jxdm5YdWxiTzZKYjNSdmRmdk41cytsMGhnV0JaLzJQ?=
 =?utf-8?B?Rm1rZmJDWFBhd09OMXUxOXhuZEFqZFdKT2FsQ0luSlZmQ1FLWUVzTTUwZlJX?=
 =?utf-8?B?dHp3UTVVNm5PbTZueWVEZHV6ZnJ0Y1JFMlEvQkg2d2cwckJuY2hwMGNROUw5?=
 =?utf-8?B?cnFzTHk2Q2EyN3ZLZW1QTWJ1R2ZybWZRWTBCVFcvb0I5SGIwSklaRWM4Nk53?=
 =?utf-8?B?Ny9XM1dvZjlJZzU0S0RCY2lhL3JrYWZ0ZFFRS3BuU2xxc3gzaXFYZkIzb1d5?=
 =?utf-8?B?V2dRdUc5Zjk5ZVhTV3o3ZlNMSU4zYktBbVBRdTJrTTgvRzlMNURHZGZYTVd2?=
 =?utf-8?B?Z3JlVHFrdFp0RDBZOUZzbGxhOWlVY1NpWHM5R0NObml4dGhhZjJnb1FXY0RR?=
 =?utf-8?B?bWtzd3o2VVg3MWxYbHptSHl5bEE4d2tkeEtoOTd2VkRJQ241Z05BdEpnd3o3?=
 =?utf-8?B?NFcxZFRVNEhwUXh6bmpId2lDZ2tqZm9pTHRNTTQvelNpejFOVXFUMSsvSGlE?=
 =?utf-8?B?VW5qNHpuMkRzVlBhdTNBMHczTWNJQ0Y5Z1pvVHNUZlk1d1p1Y3dMbWgwSEl2?=
 =?utf-8?B?cGR3V0Yvc3dzaytBeDZ2NUtZaFppa0gyRzNFWWVhcDJhV3VWZ3FIbjNVQ1lB?=
 =?utf-8?B?MFYraE9VT2xwOFIvU09zbFpzejdZcmlNRmw4azFlQVlmTkFnbDRKUHNWMVlQ?=
 =?utf-8?B?Z0R5L2phLzBrWmt1YnVqTzlhZHhqdWY2U0ZLQnZaRlh2RHZrNGJXSmwvblFu?=
 =?utf-8?B?b1VsTHBPS0lZaG01MDVhaUJVYzg4emJ3bHV6UGg3OVJRZVAzNWNQODd2c0U0?=
 =?utf-8?Q?Lq4SxGTW5G5Utvgf6V3EC10pb?=
X-MS-Exchange-CrossTenant-Network-Message-Id: 51558196-0628-49b3-59ba-08dc5426b916
X-MS-Exchange-CrossTenant-AuthSource: BL1PR11MB5978.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 03 Apr 2024 21:40:47.2178
 (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: QE3sopmEjUj84JtseQxsF8ekkAWXk6nuBaF7WQ+M85n59sVW5FHMnUUExodyMbgJdv3mw9NaVldaioSOStD8HQ==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: CY5PR11MB6091
X-OriginatorOrg: intel.com


On 3/04/2024 4:42 am, Kirill A. Shutemov wrote:
> On Fri, Mar 29, 2024 at 06:48:21PM +0200, Kirill A. Shutemov wrote:
>> On Fri, Mar 29, 2024 at 11:21:32PM +0800, Xiaoyao Li wrote:
>>> On 3/25/2024 6:38 PM, Kirill A. Shutemov wrote:
>>>> TDX guests are not allowed to clear CR4.MCE. Attempt to clear it leads
>>>> to #VE.
>>>
>>> Will we consider making it more safe and compatible for future to guard
>>> against X86_FEATURE_MCE as well?
>>>
>>> If in the future, MCE becomes configurable for TD guest, then CR4.MCE might
>>> not be fixed1.
>>
>> Good point.
>>
>> I guess we can leave it clear if it was clear. This should be easy
>> enough. But we might want to clear even if was set if clearing is allowed.
>>
>> It would require some kind of indication that clearing MCE is fine. We
>> don't have such indication yet. Not sure we can reasonably future-proof
>> the code at this point.
>>
>> But let me think more.
> 
> I think I will go with the variant below.
> 
> diff --git a/arch/x86/kernel/relocate_kernel_64.S b/arch/x86/kernel/relocate_kernel_64.S
> index 56cab1bb25f5..8e2037d78a1f 100644
> --- a/arch/x86/kernel/relocate_kernel_64.S
> +++ b/arch/x86/kernel/relocate_kernel_64.S
> @@ -5,6 +5,8 @@
>    */
>   
>   #include <linux/linkage.h>
> +#include <linux/stringify.h>
> +#include <asm/alternative.h>
>   #include <asm/page_types.h>
>   #include <asm/kexec.h>
>   #include <asm/processor-flags.h>
> @@ -145,11 +147,17 @@ SYM_CODE_START_LOCAL_NOALIGN(identity_mapped)
>   	 * Set cr4 to a known state:
>   	 *  - physical address extension enabled
>   	 *  - 5-level paging, if it was enabled before
> +	 *  - Machine check exception on TDX guest, if it was enabled before.
> +	 *    Clearing MCE might not allowed in TDX guests, depending on setup.

Nit:  Perhaps we can just call out:

	Clearing MCE is not allowed if it _was_ enabled before.

Which is always true I suppose.

>   	 */
>   	movl	$X86_CR4_PAE, %eax
>   	testq	$X86_CR4_LA57, %r13
>   	jz	1f
>   	orl	$X86_CR4_LA57, %eax
> +1:
> +	testq	$X86_CR4_MCE, %r13
> +	jz	1f
> +	ALTERNATIVE "", __stringify(orl $X86_CR4_MCE, %eax), X86_FEATURE_TDX_GUEST
>   1:
>   	movq	%rax, %cr4
>   

Anyway,

Reviewed-by: Kai Huang <kai.huang@intel.com>