Received-SPF: pass (google.com: domain of linux-kernel+bounces-52090-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:4601:e00::3 as permitted sender) client-ip=2604:1380:4601:e00::3;
Message-ID: <a7457e6f-58b7-417c-8a91-c1cfc0d36437@intel.com>
Date: Mon, 5 Feb 2024 14:49:10 +0800
User-Agent: Mozilla Thunderbird
Subject: Re: [PATCH 3/4] x86/kexec(): Reset TDX private memory on platforms
 with TDX erratum
Content-Language: en-US
To: Dave Hansen <dave.hansen@intel.com>, "Kirill A. Shutemov"
	<kirill.shutemov@linux.intel.com>
CC: <linux-kernel@vger.kernel.org>, <x86@kernel.org>, <tglx@linutronix.de>,
	<bp@alien8.de>, <mingo@redhat.com>, <hpa@zytor.com>, <luto@kernel.org>,
	<peterz@infradead.org>, <thomas.lendacky@amd.com>, <chao.gao@intel.com>,
	<bhe@redhat.com>, <nik.borisov@suse.com>, <pbonzini@redhat.com>
References: <cover.1706698706.git.kai.huang@intel.com>
 <b52ed259e0d487b3a968b98da91cb4f55a28ae82.1706698706.git.kai.huang@intel.com>
 <12a927df-e437-40ff-ba4d-ceca5446c5b1@intel.com>
 <l3uo3n3li27czehll2wz34xxkcv5j2vcshgp5a6w7u4h4aidpu@4oe2cwye2e6z>
 <0c990c7d-4df9-45c1-8c03-980d9842b963@intel.com>
 <ixp6wj4763kc5vgpmipyrijddwnxblhdn2v7hio7ze756w5f2x@6uyc7hwtrbbn>
 <b2d1e806-5bb0-44f9-a1be-498ddadd37a3@intel.com>
From: "Huang, Kai" <kai.huang@intel.com>
In-Reply-To: <b2d1e806-5bb0-44f9-a1be-498ddadd37a3@intel.com>
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?ZU83czNTWU43OGdKYnVDY1VrTHV1SXZ0SzRRZnhlbUloams1STBvaHJGenNm?=
 =?utf-8?B?WWwyZDR3VFRMWk5Xa2VVRVdwcVp0YzIvTVJuQUlhL0RiQUZmSTFYYUl5TFV6?=
 =?utf-8?B?UHlBQzRhaG1ZNjBjM3gwRHZneUVVNm9yL3poQzFUbDVPb3JZdlROOUtjdmRo?=
 =?utf-8?B?ZitKM1UydUJBRzYwbzdRR0c4MWp1a3VkWGVXREt5ZEw0blRvSTg3YVBtZUxU?=
 =?utf-8?B?czRWVU45SGlwWUgyWnFoeGFWdTdGS25OUkRSY0w0OFYyb09CcFZHOGdwbVB0?=
 =?utf-8?B?WkVJak45TUNhQTBjZzVOM25jc3dCYVR3Q3Fia251cXJvYlFIY1BidjB1T1ZM?=
 =?utf-8?B?cjRFM0tRWXBTSVdpL25iazdhaHRJMjFVdUFDQktWMUNNK1JpVVdwNkNVUmhi?=
 =?utf-8?B?d3BCa2l6SVBpbE81Z1ZFSk9uWWxucUg2WXdaUFF6SWY1eUEyZTVaK2E5YWxH?=
 =?utf-8?B?cDZESVlPYkN0TW5LNkF4TExmR2gxQTJiSm1UVEU4MkhCR3BYd2dsVGYyRDVi?=
 =?utf-8?B?WjRmTGUwanJ3MzluUnE4TUxhVnhLR01SZEFydFQvcG1ZdkhRNnBrbE5DWStj?=
 =?utf-8?B?bE1ZSFllLzNwbmk5Y3B4dDB0aVJjdWZFNlVINXdmN3NOd0NXd1QzRE5oS00w?=
 =?utf-8?B?UXFQSFlIK25ka2dLWWd0emJ3UXZvNlRvazZKSlJVUTFGeURHRzRBTk5pMWlT?=
 =?utf-8?B?YWJZQysrcUhXVkN3bStGcHdjWU5Pdk1kYmZFRlp1Tm5aYXZuRGhrWmdXQTg3?=
 =?utf-8?B?cVBKOWRHQldUY0JpUW5IWUdodkJIWFlNZ3hIMXlxZFlxa3VhdlMzdEN0dFpm?=
 =?utf-8?B?NVQwc0NQRTJXRzlhUFBVN2p0SEltN0hJbzJBWGtBem15c29tb1gvODlMdEk2?=
 =?utf-8?B?Sjk0Uncyd0M4MTJCMmJOdm91SERZWmpoRE0xVzVXNWFhKzU5SXVEQVp2RXpo?=
 =?utf-8?B?aTl5ZFFVVEdqbnJ5aURxaDc5bEx3dS9taGVOcjdLMlFNWTRUbGtjWnJkeEs1?=
 =?utf-8?B?SHRJazQ2Z2dnN1FQd1YrVVpnQjdvZ3NVWDRqTlpmMVZRTjBhZEpyL2tkaVdU?=
 =?utf-8?B?QWxYdURZb0ZYOHRqa0NWcHlWRUQxbjNCSHNDZ1NrbVdZUEk4RnhJbmFxcjhW?=
 =?utf-8?B?N0R2bWlUcEpzR2h2UHFZS0czN1ZwdVJQTG1PQm5xRmcybWJxV0FJblRITjgx?=
 =?utf-8?B?a0c0SzlBR1V6SkJxZUwweVJ5a055QWpsN0RoZFFkZ3dHYnBOaDVRNG10UzBS?=
 =?utf-8?B?S3ZCVnBCdnl5dCtVUHN4alJZdXUzVnRKSkk4eUU4Z0F5cGUvMjh1bWkrSEdm?=
 =?utf-8?B?S1Zra2hQdE9IemhPOUpNeEErSlVkSHF3VkF4bzJhOElZR3JnM3ZQOFZNZFB1?=
 =?utf-8?B?clNvcU5sVm12SnNwNHd6RGxqaXVsUkNDejZOZHZLTTgyak5pZWw5ZVVjM2U0?=
 =?utf-8?B?S0hweU9KMWJDS2VjZ0pJOGNYNEZwaDZVYUhuR1VmdWdIRThDNmIyR1M4NUM0?=
 =?utf-8?B?OUtpbEczVFowaThweW14R3Z3NzJwZ1RzbkNOK200RXhqbHE1a0dtbDVrTUxB?=
 =?utf-8?B?MGhYa0RpdVJZOXQ5YWZJaGt3ZGYvT3lmSWdGdWltc3F2amIvK2o1TCtEc0RU?=
 =?utf-8?B?QzFkdmtpNmR0Uk9tcGJhWStlT3NvRGc0ankxekZSTWJwRlVydXFDd3kxajBC?=
 =?utf-8?B?RHZXVDVudWtFTzNXK3hSdk44cXZQZDE2eGpxQXQ1RTZmd0hFL1ZOeis2RmRM?=
 =?utf-8?B?YTNjaGttTU1vRmoxOEVabCtDVXhoU0RWTkhybjZCZS9iYXJXajZsTGRqWDUx?=
 =?utf-8?B?am5GVDdHVWpHdDBUT214NHM5WFFQanp3ZU1tVVd2THdVbTlQbURnbG52ckVp?=
 =?utf-8?B?Tmtjbkx3SGIrOHBkUnMrVFNCbmFrdWRzZkpUaFQ5bHUxbStHeFVmWlZQS0RU?=
 =?utf-8?B?c3Y5RXd2REtsRVNaVHlDVERsZ08rejNhR1U3alNyMSthZWhzY0xZUUFMUWtl?=
 =?utf-8?B?SDlEck4xYUhjNGdveTlpR1dISTl4VFRXaGFXYk9peFBOcmZCTXQycDJUc2dC?=
 =?utf-8?B?SGxwNVRQNkNyRTNvd1NGVlFXVXZ3MkhYUk5FcFhSMHgyRkJjN3h1eE1RR0RW?=
 =?utf-8?Q?gwY/JeDfKFVS10ohwlTL7v/YM?=
X-MS-Exchange-CrossTenant-Network-Message-Id: 3d083d41-c13a-4d86-1575-08dc2616968d
X-MS-Exchange-CrossTenant-AuthSource: BL1PR11MB5978.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 05 Feb 2024 06:49:24.0303
 (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: oKXopTeyohZZNS6i9gb95Tk35zvbCTd4tMD9+IKEZL3Q0EDHqbP9NeygmhrNVfaqKYixYqTggw3QcTljLQh+jg==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: DM4PR11MB5567
X-OriginatorOrg: intel.com


On 2/02/2024 12:57 am, Dave Hansen wrote:
> On 2/1/24 06:39, Kirill A. Shutemov wrote:
>>> On 1/02/2024 6:03 am, Kirill A. Shutemov wrote:
>>>> On Wed, Jan 31, 2024 at 01:21:39PM -0800, Dave Hansen wrote:
>>>>>>    #ifdef CONFIG_KEXEC_JUMP
>>>>>>    	if (image->preserve_context)
>>>>>>    		save_processor_state();
>>>>>> +	else
>>>>>> +		tdx_reset_memory();
>>>>>> +#else
>>>>>> +	tdx_reset_memory();
>>>>>>    #endif
> ...
>> +void save_processor_state(void);
>> +void restore_processor_state(void);
>> +
>>   #ifdef CONFIG_SUSPEND
>>   extern suspend_state_t pm_suspend_target_state;
>>   extern suspend_state_t mem_sleep_current;
>> @@ -491,8 +494,6 @@ static inline int is_hibernate_resume_dev(dev_t dev) { return 0; }
>>   extern struct mutex system_transition_mutex;
>>   
>>   #ifdef CONFIG_PM_SLEEP
>> -void save_processor_state(void);
>> -void restore_processor_state(void);
> 
> It's a little funky that we've got a #ifdef CONFIG_KEXEC_JUMP in the .c
> file and then we're dodging around an #ifdef CONFIG_PM_SLEEP in the
> header.  This is one of the reasons we shouldn't be putting #ifdefs in
> .c files in the first place.  But I digress...
> 
> Either way, if you focus on getting the dang #ifdef out of the main code
> flow, the rest will fall in place easily.  Heck, if you even do this in
> the x86 kexec code:
> 
> static void kexec_save_processor_start(image)
> {
> #ifdef CONFIG_KEXEC_JUMP
> 	if (image->preserve_context)
> 		save_processor_state();
> #endif
> }
> 
> it'll leave you with:
> 
> 	kexec_save_processor_start(image);
> 
> 	/* Give a good reason here */
> 	if (!image->preserve_context)
> 		tdx_reset_memory();
> 
> which is *FINE*.  No funky #ifdefs, indentation or else's dangling about.
> 
> 

Thanks for the insight and explanation!  I'll use this in the new version.