Re: [PATCH v19 038/130] KVM: TDX: create/destroy VM structure

[Chao Gao]

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.