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.
