On Tue, Oct 28, 2025 at 01:46:03AM +0800, Edgecombe, Rick P wrote: > On Mon, 2025-10-27 at 17:26 +0800, Yan Zhao wrote: > > > Ugh, I'd rather not? Refresh me, what's the story with "v1"? Are we now > > > on > > > v2? > > No... We are now on v1. > > As in [1], I found that TDX module changed SEAMCALL TDH_VP_INIT behavior to > > require exclusive lock on resource TDR when leaf_opcode.version > 0. > > > > Therefore, we moved KVM_TDX_INIT_VCPU to tdx_vcpu_unlocked_ioctl() in patch > > 22. > > > > [1] > > https://lore.kernel.org/all/ala34qcjcxglk%[email protected]/ > > Looking at the PDF docs, TDR exclusive locking in version == 1 is called out > at > least back to the oldest ABI docs I have (March 2024). Not sure about the > assertion that the behavior changed, but if indeed this was documented, it's a > little bit our bad. We might consider being flexible around calling it a TDX > ABI > break? I apologize that the ABI documentation had already called this out earlier in March 2024.
I determined the locking behavior by reading the TDX module's source code, specifically, from public repo https://github.com/intel/tdx-module.git, branch tdx_1.5. I checked my git snapshot of that branch, and I think it's because back to my checking time, branch tdx_1.5 was pointing to TDX_1.5.01, which did not include the code for version 1. commit 72d8cffb214b74ae94d75afce36423020f74b47c (HEAD -> tdx_1.5, tag: TDX_1.5.01) Author: mvainer <[email protected]> Date: Thu Feb 22 15:36:58 2024 +0200 TDX 1.5.01 Signed-off-by: mvainer <[email protected]> In that repository, the latest tdx_1.5 branch points to tag TDX_1.5.16. The exclusive TDR lock in TDH.VP.INIT was introduced starting from TDX 1.5.05. commit 147ba2973479be63147048954f77a0d7248fcc35 Author: Vainer, Michael1 <[email protected]> Date: Mon Aug 11 11:53:07 2025 +0300 TDX 1.5.05 Signed-off-by: Vainer, Michael1 <[email protected]> diff --git a/src/vmm_dispatcher/api_calls/tdh_vp_init.c b/src/vmm_dispatcher/api_calls/tdh_vp_init.c index ccb6b9e..ee51a18 100644 --- a/src/vmm_dispatcher/api_calls/tdh_vp_init.c +++ b/src/vmm_dispatcher/api_calls/tdh_vp_init.c @@ -114,6 +114,17 @@ api_error_type tdh_vp_init(uint64_t target_tdvpr_pa, uint64_t td_vcpu_rcx) api_error_type return_val = UNINITIALIZE_ERROR; + tdx_leaf_and_version_t leaf_opcode; + leaf_opcode.raw = get_local_data()->vmm_regs.rax; + + uint64_t x2apic_id = get_local_data()->vmm_regs.r8; + + // TDH.VP.INIT supports version 1. Other version checks are done by the SEAMCALL dispatcher. + if (leaf_opcode.version > 1) + { + return_val = api_error_with_operand_id(TDX_OPERAND_INVALID, OPERAND_ID_RAX); + goto EXIT; + } // Check and lock the parent TDVPR page return_val = check_and_lock_explicit_4k_private_hpa(tdvpr_pa, @@ -129,11 +140,13 @@ api_error_type tdh_vp_init(uint64_t target_tdvpr_pa, uint64_t td_vcpu_rcx) goto EXIT; } + lock_type_t tdr_lock_type = (leaf_opcode.version > 0) ? TDX_LOCK_EXCLUSIVE : TDX_LOCK_SHARED; + // Lock and map the TDR page return_val = lock_and_map_implicit_tdr(get_pamt_entry_owner(tdvpr_pamt_entry_ptr), OPERAND_ID_TDR, TDX_RANGE_RO, - TDX_LOCK_SHARED, + tdr_lock_type, &tdr_pamt_entry_ptr, &tdr_locked_flag, &tdr_ptr); @@ -190,6 +203,32 @@ api_error_type tdh_vp_init(uint64_t target_tdvpr_pa, uint64_t td_vcpu_rcx) } tdvps_ptr->management.vcpu_index = vcpu_index; + if (leaf_opcode.version == 0) + { + // No x2APIC ID was provided + tdcs_ptr->executions_ctl_fields.topology_enum_configured = false; + } + else + { + // Check and save the configured x2APIC ID. Upper 32 bits must be 0. + if (x2apic_id > 0xFFFFFFFF) + { + (void)_lock_xadd_32b(&tdcs_ptr->management_fields.num_vcpus, (uint32_t)-1); + return_val = api_error_with_operand_id(TDX_OPERAND_INVALID, OPERAND_ID_R8); + goto EXIT; + } + + for (uint32_t i = 0; i < vcpu_index; i++) + { + if ((uint32_t)x2apic_id == tdcs_ptr->x2apic_ids[i]) + { + return_val = api_error_with_operand_id(TDX_X2APIC_ID_NOT_UNIQUE, tdcs_ptr->x2apic_ids[i]); + goto EXIT; + } + } + + tdcs_ptr->x2apic_ids[vcpu_index] = (uint32_t)x2apic_id; + } // We read TSC below. Compare IA32_TSC_ADJUST to the value sampled on TDHSYSINIT // to make sure the host VMM doesn't play any trick on us. */ @@ -282,7 +321,7 @@ EXIT: } if (tdr_locked_flag) { - pamt_implicit_release_lock(tdr_pamt_entry_ptr, TDX_LOCK_SHARED); + pamt_implicit_release_lock(tdr_pamt_entry_ptr, tdr_lock_type); free_la(tdr_ptr); } if (tdvpr_locked_flag)
