On 10/18/2024 10:20 PM, Edgecombe, Rick P wrote:
On Fri, 2024-10-18 at 10:21 +0800, Xiaoyao Li wrote:
KVM usually leaves it up to userspace to not create nonsensical VMs. So I
think
we can skip the check in KVM.
It's not nonsensical unless KVM announces its own requirement for TD
guest that userspace VMM must provide valid CPUID leaf 0x1f value for
topology.
How about adding it to the docs?
OK for me.
It's architectural valid that userspace VMM creates a TD with legacy
topology, i.e., topology enumerated via CPUID 0x1 and 0x4.
In that case, do you see a need for the vanilla tdh_vp_init() SEAMCALL
wrapper?
The TDX module version we need already supports enum_topology, so the code:
if (modinfo->tdx_features0 & MD_FIELD_ID_FEATURES0_TOPOLOGY_ENUM)
err = tdh_vp_init_apicid(tdx, vcpu_rcx, vcpu->vcpu_id);
else
err = tdh_vp_init(tdx, vcpu_rcx);
The tdh_vp_init() branch shouldn't be hit.
We cannot know what version of TDX module user might use thus we cannot
assume enum_topology is always there unless we make it a hard
requirement in KVM that TDX fails being enabled when
!(modinfo->tdx_features0 & MD_FIELD_ID_FEATURES0_TOPOLOGY_ENUM)
We will depend on bugs that are fixed in TDX Modules after enum topology, so it
shouldn't be required in the normal case. So I think it would be simpler to add
this tdx_features0 conditional. We can then export one less SEAMCALL and will
have less configurations flows to worry about on the KVM side.
I'm a little bit confused. what does "add this tdx_feature0 conditional"
mean?
