[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <aQAeQV+xrQB5IFBF@yzhao56-desk.sh.intel.com>
Date: Tue, 28 Oct 2025 09:37:05 +0800
From: Yan Zhao <yan.y.zhao@...el.com>
To: "Edgecombe, Rick P" <rick.p.edgecombe@...el.com>
CC: "seanjc@...gle.com" <seanjc@...gle.com>, "borntraeger@...ux.ibm.com"
<borntraeger@...ux.ibm.com>, "kvm-riscv@...ts.infradead.org"
<kvm-riscv@...ts.infradead.org>, "kvm@...r.kernel.org" <kvm@...r.kernel.org>,
"pbonzini@...hat.com" <pbonzini@...hat.com>, "linux-mips@...r.kernel.org"
<linux-mips@...r.kernel.org>, "linux-riscv@...ts.infradead.org"
<linux-riscv@...ts.infradead.org>, "linuxppc-dev@...ts.ozlabs.org"
<linuxppc-dev@...ts.ozlabs.org>, "michael.roth@....com"
<michael.roth@....com>, "kvmarm@...ts.linux.dev" <kvmarm@...ts.linux.dev>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
"oliver.upton@...ux.dev" <oliver.upton@...ux.dev>, "palmer@...belt.com"
<palmer@...belt.com>, "x86@...nel.org" <x86@...nel.org>,
"chenhuacai@...nel.org" <chenhuacai@...nel.org>, "aou@...s.berkeley.edu"
<aou@...s.berkeley.edu>, "Annapurve, Vishal" <vannapurve@...gle.com>,
"binbin.wu@...ux.intel.com" <binbin.wu@...ux.intel.com>,
"maddy@...ux.ibm.com" <maddy@...ux.ibm.com>, "maobibo@...ngson.cn"
<maobibo@...ngson.cn>, "maz@...nel.org" <maz@...nel.org>,
"linux-coco@...ts.linux.dev" <linux-coco@...ts.linux.dev>,
"anup@...infault.org" <anup@...infault.org>, "Huang, Kai"
<kai.huang@...el.com>, "frankja@...ux.ibm.com" <frankja@...ux.ibm.com>,
"pjw@...nel.org" <pjw@...nel.org>, "zhaotianrui@...ngson.cn"
<zhaotianrui@...ngson.cn>, "ackerleytng@...gle.com" <ackerleytng@...gle.com>,
"linux-arm-kernel@...ts.infradead.org"
<linux-arm-kernel@...ts.infradead.org>, "Weiny, Ira" <ira.weiny@...el.com>,
"loongarch@...ts.linux.dev" <loongarch@...ts.linux.dev>,
"imbrenda@...ux.ibm.com" <imbrenda@...ux.ibm.com>, "kas@...nel.org"
<kas@...nel.org>
Subject: Re: [PATCH v3 24/25] KVM: TDX: Guard VM state transitions with "all"
the locks
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%2Ffl@yzhao56-desk.sh.intel.com/
>
> 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 <michael1.vainer@...el.com>
Date: Thu Feb 22 15:36:58 2024 +0200
TDX 1.5.01
Signed-off-by: mvainer <michael1.vainer@...el.com>
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 <michael1.vainer@...el.com>
Date: Mon Aug 11 11:53:07 2025 +0300
TDX 1.5.05
Signed-off-by: Vainer, Michael1 <michael1.vainer@...el.com>
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)
Powered by blists - more mailing lists