Received-SPF: pass (google.com: domain of linux-kernel+bounces-146031-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.48.161 as permitted sender) client-ip=147.75.48.161;
Message-ID: <a552a48d-81b6-441a-88cf-63301f6968a2@intel.com>
Date: Tue, 16 Apr 2024 12:05:31 +1200
User-Agent: Mozilla Thunderbird
Subject: Re: [PATCH v19 087/130] KVM: TDX: handle vcpu migration over logical
 processor
To: "Yamahata, Isaku" <isaku.yamahata@intel.com>, Sean Christopherson
	<seanjc@google.com>
CC: "Chatre, Reinette" <reinette.chatre@intel.com>, "kvm@vger.kernel.org"
	<kvm@vger.kernel.org>, "linux-kernel@vger.kernel.org"
	<linux-kernel@vger.kernel.org>, "isaku.yamahata@gmail.com"
	<isaku.yamahata@gmail.com>, Paolo Bonzini <pbonzini@redhat.com>, "Aktas,
 Erdem" <erdemaktas@google.com>, Sagi Shahar <sagis@google.com>, "Chen, Bo2"
	<chen.bo@intel.com>, "Yuan, Hang" <hang.yuan@intel.com>, "Zhang, Tina"
	<tina.zhang@intel.com>, "isaku.yamahata@linux.intel.com"
	<isaku.yamahata@linux.intel.com>
References: <cover.1708933498.git.isaku.yamahata@intel.com>
 <b9fe57ceeaabe650f0aecb21db56ef2b1456dcfe.1708933498.git.isaku.yamahata@intel.com>
 <0c3efffa-8dd5-4231-8e90-e0241f058a20@intel.com>
 <20240412214201.GO3039520@ls.amr.corp.intel.com>
 <Zhm5rYA8eSWIUi36@google.com>
 <20240413004031.GQ3039520@ls.amr.corp.intel.com>
 <Zh0wGQ_FfPRENgb0@google.com>
 <20240415224828.GS3039520@ls.amr.corp.intel.com>
Content-Language: en-US
From: "Huang, Kai" <kai.huang@intel.com>
In-Reply-To: <20240415224828.GS3039520@ls.amr.corp.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?MWJZS1U4SGk4Zm11cGc0T3UzQnhJTDg1SW5tWjZ5czk5Mzh3OXF1bmVGMk91?=
 =?utf-8?B?eVJuUGFkVGt4QnNaSE84RHZUeDFaVnBoMFZrU1RNNHd0S09zaXo4amRobkI2?=
 =?utf-8?B?enpveFphb3ZoSEZiNFhmZ3ZJVWFIdXZnZlhBNm9ZY01sTXZERklTRDRUTG1Y?=
 =?utf-8?B?K2IrdnRzenpwVytwazJzV1ZURjNWU01zTjFuV0xua1lub0Zkb2lWR1RhbjFj?=
 =?utf-8?B?MnVnSUp5RkxyR1Y2R2tKZXl1R1l2Y2JyTUx1M3IrZlN4ZS9xWTJyVjVMYk1C?=
 =?utf-8?B?RGYwcG42cFZYWS9obm5mR0dpUGswbTdQUk9QdnJyaWtkY1pLTzh0OHBlR3dj?=
 =?utf-8?B?V2pGL2ZaOTBIWWsyMG9ZbThNZnJvcEdnWUx6dWNSMU4rWW5kN1J1UE0yMTM0?=
 =?utf-8?B?Q1VReHNMUm01YjhuMitIQ29xWWNHNFp2anhBS3FoTEcvbEZNK1NUSG1FWCt6?=
 =?utf-8?B?S0Yvc2VvQkN6TEJNVUR1TFIwU080Z0loU2xOaWVEWXRXVEFQK2lpVE1sWTU3?=
 =?utf-8?B?Tm4xeHhZbW1FRFhsZ25VWDcrNVE3T1JCSElVWk9JVW80V25TaGpsZm85K1hI?=
 =?utf-8?B?YmlKcHRLQ3BqOEQzYXhBYjZrWXd3TWFyYms5eWd3UzY4U3VJalFqdlFkcXhl?=
 =?utf-8?B?TFVZU1Q1Y25kYTJMWVRtdklpZWRiRkZMYk9xcktRMmhCK2hPUktZcENFVTl3?=
 =?utf-8?B?TUlRaXU2eDBIdkY5ZDcyS1Brbi9iVUhiT3ZnWXhsRnJsNXZGYllVcXpod0Fj?=
 =?utf-8?B?UklqY2xqNmlEVzc1QU1JUk51ZDhONWkvR2dBcVcyVmhTNHRqeDUzS0tFeUVD?=
 =?utf-8?B?YWx6MTY4SnlJRndrNEFuY211YWQyUFg2dHVaWFprUU0yU01TM2o5Y3l0WEd0?=
 =?utf-8?B?ZHNXZXgxYnZJY1FseTlrQklyWEUrOGR5UWNmWEt5QTRXOEJybGovTGpuNEYz?=
 =?utf-8?B?WnVIUjhhK1VvSzczMmVVRTZnV0lHMkRiZGN4bnhCRXEzcURXbW9jSG5HYUpU?=
 =?utf-8?B?VjVMTW01SkRMd3ZiUmsrNnRJRkZtbFVvdmtEN0FLbTg2TGU1ZTdDS1BXZkZB?=
 =?utf-8?B?NWxYUllNLy8wTW0vVC9jYmIwNWdmWVRjR0x6d3c3OHY2VVEzUlU5Ync4cXl5?=
 =?utf-8?B?RjVTYktCUzJlVXZEamtXUzJLdlg3U3NCTXhnS211SmI1cGdMajBNQ0ZnaUJT?=
 =?utf-8?B?VkVPSWRQKzd3SGN1MUwvNXErd2Vzc0xzcWliS3hzYjNqeGl1ZUNuSjRKaVAz?=
 =?utf-8?B?MlZyZDF6Q3BDbGIxMm45cFFZcU5ESm9vc2FPMmdjOWJBMmdCQmFBbHZEWDcy?=
 =?utf-8?B?RWlpOXNXK3hnSXBkVVFHMkVwaXlCTlY1UjVBeGNwWHVGbEh0UGN5VTdUbkFa?=
 =?utf-8?B?QXI2b3BkdndPSWlzbzJWdDVuRFNncFQ0QXFxd0hsMHc4SDBxVDBUMlBNcXdV?=
 =?utf-8?B?TnlhTGFxV0loSU81OU4wSnlYenhSM1ZyMGh4LzF3VTcwYVhYUWp1RjdaUmls?=
 =?utf-8?B?UDRGeVJuK2QxZUlVZng1U0VKTWduU1dXSTJoY1dHbElZRE9yeVMvQVVYZyt6?=
 =?utf-8?B?YU11THdrbmc5eGJGNDR4cnBvSExqRVpOKzBzbFhDRmlYSmpZQk5abWQrdUQ3?=
 =?utf-8?B?ek1SZzlxYTloWjF5dGc0VFpyYkdneGZJc013S2tLQ0g2YWs5ZS9JQjc0cXU5?=
 =?utf-8?B?UWE3L0dNaFRkTWFTdTRnanBtdTF6akE2RlNkQkNIYnZnMldmekNxYWVJUDBt?=
 =?utf-8?B?b0hJM2IvdWhuVk1FQldKM21tcVQzbzdQK2dSM2p0Nkdha0ZiOFI0WE43bStp?=
 =?utf-8?B?OWJYckJHeGdKZ3RMNmtmTWJ1MmFUUjJMa1VGQTMwdHVoRlJkS3djMWZQRDVo?=
 =?utf-8?B?aDBiSm55bXc3aCtHZC9YSDFUdGVKVmFXQ1cvT0UvdXRSblQyL2ExY00rSVpS?=
 =?utf-8?B?OWEzTm5HcGsyb2xZZHJtZ2Q3RjZjaVFhTjNTbUt2NTJzRWJxOFA5UlVSR2E2?=
 =?utf-8?B?cEdYWmZaSTFVd3R1S1VJWUJUamRhUVRYcUROcTJqUFRhNXB3QjRLWUlpZUM3?=
 =?utf-8?B?Vm5oY3cxdXFacm0zcmRyMGZWSXU3cG9IZmk0VGtDTGQzOFE4dlIxdnZ2ZVBu?=
 =?utf-8?Q?yxLJvquTdaMKtrP3F4y/6y1kO?=
X-MS-Exchange-CrossTenant-Network-Message-Id: 86b4fde2-e062-4954-1be1-08dc5da8f45b
X-MS-Exchange-CrossTenant-AuthSource: PH7PR11MB5983.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 16 Apr 2024 00:05:41.6886
 (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: SgbFQ2LPSP+N6wFi6bM47a02DUHqhD6MXdawn/j1HGcCnTo26rK/JQwmlRsu+KL4QOhAEO+IDlLsT3SOUU5OpA==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: MW3PR11MB4699
X-OriginatorOrg: intel.com


On 16/04/2024 10:48 am, Yamahata, Isaku wrote:
> On Mon, Apr 15, 2024 at 06:49:35AM -0700,
> Sean Christopherson <seanjc@google.com> wrote:
> 
>> On Fri, Apr 12, 2024, Isaku Yamahata wrote:
>>> On Fri, Apr 12, 2024 at 03:46:05PM -0700,
>>> Sean Christopherson <seanjc@google.com> wrote:
>>>
>>>> On Fri, Apr 12, 2024, Isaku Yamahata wrote:
>>>>> On Fri, Apr 12, 2024 at 09:15:29AM -0700, Reinette Chatre <reinette.chatre@intel.com> wrote:
>>>>>>> +void tdx_mmu_release_hkid(struct kvm *kvm)
>>>>>>> +{
>>>>>>> +	while (__tdx_mmu_release_hkid(kvm) == -EBUSY)
>>>>>>> +		;
>>>>>>>   }
>>>>>>
>>>>>> As I understand, __tdx_mmu_release_hkid() returns -EBUSY
>>>>>> after TDH.VP.FLUSH has been sent for every vCPU followed by
>>>>>> TDH.MNG.VPFLUSHDONE, which returns TDX_FLUSHVP_NOT_DONE.
>>>>>>
>>>>>> Considering earlier comment that a retry of TDH.VP.FLUSH is not
>>>>>> needed, why is this while() loop here that sends the
>>>>>> TDH.VP.FLUSH again to all vCPUs instead of just a loop within
>>>>>> __tdx_mmu_release_hkid() to _just_ resend TDH.MNG.VPFLUSHDONE?
>>>>>>
>>>>>> Could it be possible for a vCPU to appear during this time, thus
>>>>>> be missed in one TDH.VP.FLUSH cycle, to require a new cycle of
>>>>>> TDH.VP.FLUSH?
>>>>>
>>>>> Yes. There is a race between closing KVM vCPU fd and MMU notifier release hook.
>>>>> When KVM vCPU fd is closed, vCPU context can be loaded again.
>>>>
>>>> But why is _loading_ a vCPU context problematic?
>>>
>>> It's nothing problematic.  It becomes a bit harder to understand why
>>> tdx_mmu_release_hkid() issues IPI on each loop.  I think it's reasonable
>>> to make the normal path easy and to complicate/penalize the destruction path.
>>> Probably I should've added comment on the function.
>>
>> By "problematic", I meant, why can that result in a "missed in one TDH.VP.FLUSH
>> cycle"?  AFAICT, loading a vCPU shouldn't cause that vCPU to be associated from
>> the TDX module's perspective, and thus shouldn't trigger TDX_FLUSHVP_NOT_DONE.
>>
>> I.e. looping should be unnecessary, no?
> 
> The loop is unnecessary with the current code.
> 
> The possible future optimization is to reduce destruction time of Secure-EPT
> somehow.  One possible option is to release HKID while vCPUs are still alive and
> destruct Secure-EPT with multiple vCPU context.  Because that's future
> optimization, we can ignore it at this phase.

I kinda lost here.

I thought in the current v19 code, you have already implemented this 
optimization?

Or is this optimization totally different from what we discussed in an 
earlier patch?

https://lore.kernel.org/lkml/8feaba8f8ef249950b629f3a8300ddfb4fbcf11c.camel@intel.com/