Re: [PATCH v7 022/102] KVM: TDX: create/destroy VM structure

[Isaku Yamahata]

On Tue, Aug 02, 2022 at 07:46:21PM +0000,
Sean Christopherson <seanjc@xxxxxxxxxx> wrote:

> On Mon, Jun 27, 2022, isaku.yamahata@xxxxxxxxx wrote:
> > +int tdx_vm_init(struct kvm *kvm)
> > +{
> > +	struct kvm_tdx *kvm_tdx = to_kvm_tdx(kvm);
> > +	cpumask_var_t packages;
> > +	int ret, i;
> > +	u64 err;
> > +
> > +	/* vCPUs can't be created until after KVM_TDX_INIT_VM. */
> > +	kvm->max_vcpus = 0;
> > +
> > +	kvm_tdx->hkid = tdx_keyid_alloc();
> > +	if (kvm_tdx->hkid < 0)
> > +		return -EBUSY;
> 
> We (Google) have been working through potential flows for intrahost (copyless)
> migration, and one of the things that came up is that allocating the HKID during
> KVM_CREATE_VM will be problematic as HKID are a relatively scarce resource.  E.g.
> if all key IDs are in use, then creating a destination TDX VM will be impossible
> even though intrahost migration can create succeed since the "new" would reuse
> the source's HKID.
> 
> Allocating the various pages is also annoying, e.g. they'd have to be freed, but
> not as directly problematic.
> 
> SEV (all flavors) has a similar problem with ASIDs.  The solution for SEV was to
> not allocate an ASID during KVM_CREATE_VM and instead "activate" SEV during
> KVM_CAP_VM_MOVE_ENC_CONTEXT_FROM.
> 
> I think we should prepare for a similar future for TDX and move the HKID allocation
> and all dependent resource allocation to KVM_TDX_INIT_VM.  AFAICT (and remember),
> this should be a fairly simple code movement, but I'd prefer it be done before
> merging TDX so that if it's not so simple, e.g. requires another sub-ioctl, then
> we don't have to try and tweak KVM's ABI to enable intrahost migration.

The simple code movement works here.  The TDX related initialization/allocation
can simply be moved to KVM_TDX_INIT_VM and KVM_TDX_INIT_VCPU.

I'll update them with the next respin.

Thanks,
-- 
Isaku Yamahata <isaku.yamahata@xxxxxxxxx>