linux-kernel - Re: [PATCH v3 15/25] KVM: TDX: ADD pages to the TD image while populating mirror EPT entries

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <c2a34f05801ec5e3a9c51f0c585b368c82c4ea43.camel@intel.com>
Date: Fri, 24 Oct 2025 07:18:52 +0000
From: "Huang, Kai" <kai.huang@...el.com>
To: "chenhuacai@...nel.org" <chenhuacai@...nel.org>, "frankja@...ux.ibm.com"
	<frankja@...ux.ibm.com>, "maz@...nel.org" <maz@...nel.org>,
	"borntraeger@...ux.ibm.com" <borntraeger@...ux.ibm.com>, "pjw@...nel.org"
	<pjw@...nel.org>, "aou@...s.berkeley.edu" <aou@...s.berkeley.edu>,
	"kas@...nel.org" <kas@...nel.org>, "seanjc@...gle.com" <seanjc@...gle.com>,
	"maobibo@...ngson.cn" <maobibo@...ngson.cn>, "pbonzini@...hat.com"
	<pbonzini@...hat.com>, "maddy@...ux.ibm.com" <maddy@...ux.ibm.com>,
	"palmer@...belt.com" <palmer@...belt.com>, "imbrenda@...ux.ibm.com"
	<imbrenda@...ux.ibm.com>, "zhaotianrui@...ngson.cn"
	<zhaotianrui@...ngson.cn>, "anup@...infault.org" <anup@...infault.org>,
	"oliver.upton@...ux.dev" <oliver.upton@...ux.dev>
CC: "kvm@...r.kernel.org" <kvm@...r.kernel.org>, "linux-coco@...ts.linux.dev"
	<linux-coco@...ts.linux.dev>, "Zhao, Yan Y" <yan.y.zhao@...el.com>,
	"michael.roth@....com" <michael.roth@....com>, "linux-kernel@...r.kernel.org"
	<linux-kernel@...r.kernel.org>, "Weiny, Ira" <ira.weiny@...el.com>,
	"loongarch@...ts.linux.dev" <loongarch@...ts.linux.dev>,
	"binbin.wu@...ux.intel.com" <binbin.wu@...ux.intel.com>,
	"ackerleytng@...gle.com" <ackerleytng@...gle.com>, "kvmarm@...ts.linux.dev"
	<kvmarm@...ts.linux.dev>, "kvm-riscv@...ts.infradead.org"
	<kvm-riscv@...ts.infradead.org>, "Annapurve, Vishal" <vannapurve@...gle.com>,
	"linuxppc-dev@...ts.ozlabs.org" <linuxppc-dev@...ts.ozlabs.org>, "Edgecombe,
 Rick P" <rick.p.edgecombe@...el.com>, "linux-mips@...r.kernel.org"
	<linux-mips@...r.kernel.org>, "linux-arm-kernel@...ts.infradead.org"
	<linux-arm-kernel@...ts.infradead.org>, "linux-riscv@...ts.infradead.org"
	<linux-riscv@...ts.infradead.org>, "x86@...nel.org" <x86@...nel.org>
Subject: Re: [PATCH v3 15/25] KVM: TDX: ADD pages to the TD image while
 populating mirror EPT entries

On Thu, 2025-10-16 at 17:32 -0700, Sean Christopherson wrote:
> When populating the initial memory image for a TDX guest, ADD pages to the
> TD as part of establishing the mappings in the mirror EPT, as opposed to
> creating the mappings and then doing ADD after the fact.  Doing ADD in the
> S-EPT callbacks eliminates the need to track "premapped" pages, as the
> mirror EPT (M-EPT) and S-EPT are always synchronized, e.g. if ADD fails,
> KVM reverts to the previous M-EPT entry (guaranteed to be !PRESENT).
> 
> Eliminating the hole where the M-EPT can have a mapping that doesn't exist
> in the S-EPT in turn obviates the need to handle errors that are unique to
> encountering a missing S-EPT entry (see tdx_is_sept_zap_err_due_to_premap()).
> 
> Keeping the M-EPT and S-EPT synchronized also eliminates the need to check
> for unconsumed "premap" entries during tdx_td_finalize(), as there simply
> can't be any such entries.  Dropping that check in particular reduces the
> overall cognitive load, as the managemented of nr_premapped with respect
				 ^
				 management

> to removal of S-EPT is _very_ subtle.  E.g. successful removal of an S-EPT
> entry after it completed ADD doesn't adjust nr_premapped, but it's not
> clear why that's "ok" but having half-baked entries is not (it's not truly
> "ok" in that removing pages from the image will likely prevent the guest
> from booting, but from KVM's perspective it's "ok").
> 
> Doing ADD in the S-EPT path requires passing an argument via a scratch
> field, but the current approach of tracking the number of "premapped"
> pages effectively does the same.  And the "premapped" counter is much more
> dangerous, as it doesn't have a singular lock to protect its usage, since
> nr_premapped can be modified as soon as mmu_lock is dropped, at least in
> theory.  I.e. nr_premapped is guarded by slots_lock, but only for "happy"
> paths.
> 
> Note, this approach was used/tried at various points in TDX development,
> but was ultimately discarded due to a desire to avoid stashing temporary
> state in kvm_tdx.  But as above, KVM ended up with such state anyways,
> and fully committing to using temporary state provides better access
> rules (100% guarded by slots_lock), and makes several edge cases flat out
> impossible.
> 
> Note #2, continue to extend the measurement outside of mmu_lock, as it's
> a slow operation (typically 16 SEAMCALLs per page whose data is included
> in the measurement), and doesn't *need* to be done under mmu_lock, e.g.
> for consistency purposes.  However, MR.EXTEND isn't _that_ slow, e.g.
> ~1ms latency to measure a full page, so if it needs to be done under
> mmu_lock in the future, e.g. because KVM gains a flow that can remove
> S-EPT entries uring KVM_TDX_INIT_MEM_REGION, then extending the
		^
		during

> measurement can also be moved into the S-EPT mapping path (again, only if
> absolutely necessary).  P.S. _If_ MR.EXTEND is moved into the S-EPT path,
> take care not to return an error up the stack if TDH_MR_EXTEND fails, as
> removing the M-EPT entry but not the S-EPT entry would result in
> inconsistent state!
> 
> Reviewed-by: Rick Edgecombe <rick.p.edgecombe@...el.com>
> Signed-off-by: Sean Christopherson <seanjc@...gle.com>

Reviewed-by: Kai Huang <kai.huang@...el.com>