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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <Yul/DapNdokpeN36@google.com>
Date:   Tue, 2 Aug 2022 19:46:21 +0000
From:   Sean Christopherson <seanjc@...gle.com>
To:     isaku.yamahata@...el.com
Cc:     kvm@...r.kernel.org, linux-kernel@...r.kernel.org,
        isaku.yamahata@...il.com, Paolo Bonzini <pbonzini@...hat.com>,
        Kai Huang <kai.huang@...el.com>, Sagi Shahar <sagis@...gle.com>
Subject: Re: [PATCH v7 022/102] KVM: TDX: create/destroy VM structure

On Mon, Jun 27, 2022, isaku.yamahata@...el.com 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.

> +
> +	ret = tdx_alloc_td_page(&kvm_tdx->tdr);
> +	if (ret)
> +		goto free_hkid;
> +
> +	kvm_tdx->tdcs = kcalloc(tdx_caps.tdcs_nr_pages, sizeof(*kvm_tdx->tdcs),
> +				GFP_KERNEL_ACCOUNT);
> +	if (!kvm_tdx->tdcs)
> +		goto free_tdr;
> +	for (i = 0; i < tdx_caps.tdcs_nr_pages; i++) {
> +		ret = tdx_alloc_td_page(&kvm_tdx->tdcs[i]);
> +		if (ret)
> +			goto free_tdcs;
> +	}