Re: [PATCH v3 24/25] KVM: TDX: Guard VM state transitions with "all" the locks
From: Yan Zhao <hidden>
Date: 2025-10-28 01:39:11
Also in:
kvm, kvm-riscv, kvmarm, linux-coco, linux-mips, linux-riscv, linuxppc-dev, lkml, loongarch
Subsystem:
the rest · Maintainer:
Linus Torvalds
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:quoted
quoted
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%2Ffl@yzhao56-desk.sh.intel.com/ (local)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 [off-list ref] Date: Thu Feb 22 15:36:58 2024 +0200 TDX 1.5.01 Signed-off-by: mvainer [off-list ref] 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 [off-list ref] Date: Mon Aug 11 11:53:07 2025 +0300 TDX 1.5.05 Signed-off-by: Vainer, Michael1 [off-list ref]
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)