On 10/17/2024 1:42 AM, Edgecombe, Rick P wrote:
On Wed, 2024-10-09 at 18:01 +0300, Adrian Hunter wrote:tdh_vp_init_apicid() passes x2APIC ID to TDH.VP.INIT which is one of the steps for the TDX Module to support topology information for the guest i.e. CPUID leaf 0xB and CPUID leaf 0x1F. If the host VMM does not provide CPUID leaf 0x1F values (i.e. the values are 0), then the TDX Module will use native values for both CPUID leaf 0x1F and CPUID leaf 0xB. To get 0x1F/0xB the guest must also opt-in by setting TDCS.TD_CTLS.ENUM_TOPOLOGY to 1. AFAICT currently Linux does not do that. In the tdh_vp_init() case, topology information will not be supported. If topology information is not supported CPUID leaf 0xB and CPUID leaf 0x1F will #VE, and a Linux guest will return zeros. So, yes, it seems like tdh_vp_init_apicid() should only be called if there is non-zero CPUID leaf 0x1F values provided by host VMM. e.g. add a helper function bool tdx_td_enum_topology(struct kvm_cpuid2 *cpuid) { const struct tdx_sys_info_features *modinfo = &tdx_sysinfo->features; const struct kvm_cpuid_entry2 *entry; if (!(modinfo->tdx_features0 & MD_FIELD_ID_FEATURES0_TOPOLOGY_ENUM)) return false; entry = kvm_find_cpuid_entry2(cpuid->entries, cpuid->nent, 0x1f, 0); if (!entry) return false; return entry->eax || entry->ebx || entry->ecx || entry->edx; }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.
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)
