Received-SPF: pass (google.com: domain of linux-kernel+bounces-123043-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.199.223 as permitted sender) client-ip=147.75.199.223;
Date: Thu, 28 Mar 2024 22:12:18 +0800
From: Chao Gao <chao.gao@intel.com>
To: "Huang, Kai" <kai.huang@intel.com>
CC: "Yamahata, Isaku" <isaku.yamahata@intel.com>, "Zhang, Tina"
	<tina.zhang@intel.com>, "isaku.yamahata@linux.intel.com"
	<isaku.yamahata@linux.intel.com>, "seanjc@google.com" <seanjc@google.com>,
	"Yuan, Hang" <hang.yuan@intel.com>, "sean.j.christopherson@intel.com"
	<sean.j.christopherson@intel.com>, "Chen, Bo2" <chen.bo@intel.com>,
	"sagis@google.com" <sagis@google.com>, "isaku.yamahata@gmail.com"
	<isaku.yamahata@gmail.com>, "Aktas, Erdem" <erdemaktas@google.com>,
	"kvm@vger.kernel.org" <kvm@vger.kernel.org>, "pbonzini@redhat.com"
	<pbonzini@redhat.com>, "linux-kernel@vger.kernel.org"
	<linux-kernel@vger.kernel.org>
Subject: Re: [PATCH v19 038/130] KVM: TDX: create/destroy VM structure
Message-ID: <ZgV6wnQcU37Jbp7W@chao-email>
References: <cover.1708933498.git.isaku.yamahata@intel.com>
 <7a508f88e8c8b5199da85b7a9959882ddf390796.1708933498.git.isaku.yamahata@intel.com>
 <a0627c0f-5c2d-4403-807f-fc800b43fd3b@intel.com>
 <20240327225337.GF2444378@ls.amr.corp.intel.com>
 <4d925a79-d3cf-4555-9c00-209be445310d@intel.com>
 <20240328053432.GO2444378@ls.amr.corp.intel.com>
 <65a1a35e0a3b9a6f0a123e50ec9ddb755f70da52.camel@intel.com>
Content-Type: text/plain; charset="us-ascii"
Content-Disposition: inline
In-Reply-To: <65a1a35e0a3b9a6f0a123e50ec9ddb755f70da52.camel@intel.com>
Precedence: bulk
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?us-ascii?Q?vct1llWLKX/vXdB65c0/jZkXcMQ6tqDF/UdMzmuAgzIrJzmS43Qelb97Vief?=
 =?us-ascii?Q?6zvznYJwcxAr+qrKyYl2S5vowix4w+lIwwusVCBp6XwxX9R5kDvjNGOZhre1?=
 =?us-ascii?Q?drzZePrF4n+9Xy1ch2C9iuZNpA5xKU8jHZXS4lXG812LxaNmqG0X5zxsKsYv?=
 =?us-ascii?Q?fECU+VkkGBc1HAt1tKf/5rvQD2MTM3HDnYTwSwKsnReDfGmZeq7qd58KfA5+?=
 =?us-ascii?Q?NWV4jLsvBW2FCEqEZg1121enPYyauZZUgQWBZ5lx0I4AIx/sRYSyRbo32bFn?=
 =?us-ascii?Q?/Y8+ik+0ObW7VhSXJRKr7buTtMuGKGRSX5FW5vVrTHfgUaZHkXNH94gqeHJl?=
 =?us-ascii?Q?F9EebJyqxfIIjcahZ+/OasB0eLk2rInhnk6SUIGZ4/R1/cRJN6pziY2bf6x3?=
 =?us-ascii?Q?03lCSFUx3+1oaREkUq1EUDgsQe1St/UhN2VkTeZC+UKMA4AkglWJgzRSouCM?=
 =?us-ascii?Q?gMolFSXyybBoN9AguUGTL7/8jHhzuESWuAt6rXQL5/joQy4B0ZWkaB2NzFNL?=
 =?us-ascii?Q?egRdop0T0IDUkALSSXWWmgffwDnkBNc4VU63kIE5Yf/9Sh/S4sz3Twwc4Dri?=
 =?us-ascii?Q?TrX2b8B7VS5YpeeZ/HvBhkHIyWW5zRIJE+QIcHsb7JdlL8FkKXvTC+ticAcc?=
 =?us-ascii?Q?SdUrLDF1yqqGUlCvaitLKG52zMzz8iW3RpmIFoiQ+s7GdIzKQizVUFTSIeRX?=
 =?us-ascii?Q?f8yaTuNV1/QYNjN00BJwphKiu05aUwFTzbd8TJFbBAtkZrCK9Z3bjCUWXCcX?=
 =?us-ascii?Q?uhvKIRXozRC9vcSH0NJsM5rMG6ceHDfsO1DULTny2YJu11f0IxRY5TClw7nA?=
 =?us-ascii?Q?YTzXBfEAhIItzmi6jU5bQr7ko1ecRnGH5mIeR6NPr4Ndibct8lCBl0svvDqR?=
 =?us-ascii?Q?3fdaP9PdV2MH++pemmGkPmYhlW5iQ/LfUKOuE450oCSSJeP+wzF5PvEnPDy1?=
 =?us-ascii?Q?59KfMMIxXzBghIgsyhqlqJwiLNLLPCjKmMWj4doTWWff69U1QtEbTg61xqnj?=
 =?us-ascii?Q?EZHdeQ3Ir8Rx/yjAi1s+qNU9E/5j7Ho6Z2FyWTTHECQkCSIixGRp3NyD6M9I?=
 =?us-ascii?Q?dKd6q+4jAR+Kdfx7hQ0126Xx2YUX9ZrgYDzf5/jw7aEZKuhOVyCol2j5RNlE?=
 =?us-ascii?Q?bjOf5xFbTMyWmh/axz581ErXurmFuUYBVYLRRpDhLt0le9eHD4GWvMKJZjWc?=
 =?us-ascii?Q?rKnzrJDDkIB6f+nEieVru3h7OyL7EBXfYPROq2iF+iKhfcl3H3pOHBL/Fx7k?=
 =?us-ascii?Q?9n7X/vWCbwXy1RT45pHhcakSJ2vIYr9wPu04Bg6lsGkVmuvx0X0KsEnf0jdS?=
 =?us-ascii?Q?ZhUEhIewhHJyayvJwyZE8klP94ePr6K69RgGaB/AjxBCa+M1qBWFLMJm8dWK?=
 =?us-ascii?Q?dgdlw67Hwiacc32h3HK2TUBTIQmZOlv6jZmvNlALp4D4C74JVdsiWxRYQtkP?=
 =?us-ascii?Q?BkUlPil7DEPBwCcZ7vbarfTdO1JejxzBlxpsZe3ul12VmcGTNG4TTgZOuhq3?=
 =?us-ascii?Q?TcmjLntmzjvwNMKkaADX8Kk1ZYbkE7OvGVB9v/ZVl7G1XLVziffS9l31yuI8?=
 =?us-ascii?Q?laV8msdv7QaIKa+6CNbtiGVJCpQ7prit11k9x51I?=
X-MS-Exchange-CrossTenant-Network-Message-Id: 1d5376fd-6ab5-4461-7a61-08dc4f3119af
X-MS-Exchange-CrossTenant-AuthSource: CH3PR11MB8660.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 28 Mar 2024 14:12:28.4265
 (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: mWwE1yjqPQB85lA2LD1spltLIYCIySMgey3FYKhtd1oIa4mJ/xx0IJj7t83mX7cZe1pp59ovZ+duOfM1I3siuQ==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: DM6PR11MB4545
X-OriginatorOrg: intel.com

On Thu, Mar 28, 2024 at 11:14:42AM +0000, Huang, Kai wrote:
>> > 
>> > [...]
>> > 
>> > > > > +
>> > > > > +void tdx_mmu_release_hkid(struct kvm *kvm)
>> > > > > +{
>> > > > > +	bool packages_allocated, targets_allocated;
>> > > > > +	struct kvm_tdx *kvm_tdx = to_kvm_tdx(kvm);
>> > > > > +	cpumask_var_t packages, targets;
>> > > > > +	u64 err;
>> > > > > +	int i;
>> > > > > +
>> > > > > +	if (!is_hkid_assigned(kvm_tdx))
>> > > > > +		return;
>> > > > > +
>> > > > > +	if (!is_td_created(kvm_tdx)) {
>> > > > > +		tdx_hkid_free(kvm_tdx);
>> > > > > +		return;
>> > > > > +	}
>> > > > 
>> > > > I lost tracking what does "td_created()" mean.
>> > > > 
>> > > > I guess it means: KeyID has been allocated to the TDX guest, but not yet
>> > > > programmed/configured.
>> > > > 
>> > > > Perhaps add a comment to remind the reviewer?
>> > > 
>> > > As Chao suggested, will introduce state machine for vm and vcpu.
>> > > 
>> > > https://lore.kernel.org/kvm/ZfvI8t7SlfIsxbmT@chao-email/
>> > 
>> > Could you elaborate what will the state machine look like?
>> > 
>> > I need to understand it.
>> 
>> Not yet. Chao only propose to introduce state machine. Right now it's just an
>> idea.
>
>Then why state machine is better?  I guess we need some concrete example to tell
>which is better?

Something like the TD Life Cycle State Machine (Section 9.1 of TDX module spec[1])

[1]: https://cdrdv2.intel.com/v1/dl/getContent/733568

I don't have the code. But using a few boolean variables to track the state of
TD and VCPU looks bad and hard to maintain and extend. At least, the state machine
is well-documented.