linux-kernel - Re: [RFC PATCH v5 16/45] x86/virt/tdx: Add tdx_alloc/free_control

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: <ebd424718bb0b2754b7cbacb277746a3076faea3.camel@intel.com>
Date: Tue, 10 Feb 2026 22:15:42 +0000
From: "Edgecombe, Rick P" <rick.p.edgecombe@...el.com>
To: "Hansen, Dave" <dave.hansen@...el.com>, "seanjc@...gle.com"
	<seanjc@...gle.com>, "x86@...nel.org" <x86@...nel.org>,
	"dave.hansen@...ux.intel.com" <dave.hansen@...ux.intel.com>, "kas@...nel.org"
	<kas@...nel.org>, "bp@...en8.de" <bp@...en8.de>, "mingo@...hat.com"
	<mingo@...hat.com>, "tglx@...nel.org" <tglx@...nel.org>,
	"pbonzini@...hat.com" <pbonzini@...hat.com>
CC: "Huang, Kai" <kai.huang@...el.com>, "ackerleytng@...gle.com"
	<ackerleytng@...gle.com>, "sagis@...gle.com" <sagis@...gle.com>, "Annapurve,
 Vishal" <vannapurve@...gle.com>, "linux-kernel@...r.kernel.org"
	<linux-kernel@...r.kernel.org>, "Zhao, Yan Y" <yan.y.zhao@...el.com>, "Li,
 Xiaoyao" <xiaoyao.li@...el.com>, "kvm@...r.kernel.org" <kvm@...r.kernel.org>,
	"linux-coco@...ts.linux.dev" <linux-coco@...ts.linux.dev>, "Yamahata, Isaku"
	<isaku.yamahata@...el.com>, "binbin.wu@...ux.intel.com"
	<binbin.wu@...ux.intel.com>
Subject: Re: [RFC PATCH v5 16/45] x86/virt/tdx: Add
 tdx_alloc/free_control_page() helpers

On Tue, 2026-02-10 at 09:44 -0800, Dave Hansen wrote:
> This looks funky.
> 
> Right now, this is:
> 
> 	spin_lock(pamt_lock)
> 	atomic_inc/dec(fine-grained-refcount)
> 	tdcall_blah_blah()
> 	spin_unlock(pamt_lock)
> 
> Where it *always* acquires the global lock when DPAMT is supported.
> Couldn't we optimize it so that it only acquires it when it has to keep
> the refcount stable at zero?
> 
> Roughly:
> 
> 	slow_path = atomic_dec_and_lock(fine-grained-refcount,
> 					pamt_lock)
> 	if (!slow_path)
> 		goto out;
> 
> 	// fine-grained-refcount==0 and must stay that way with
> 	// pamt_lock held. Remove the DPAMT pages:
> 	tdh_phymem_pamt_remove(page, pamt_pa_array)
> out:	
> 	spin_unlock(pamt_lock)
> 
> On the acquire side, you do:
> 
> 	fast_path = atomic_inc_not_zero(fine-grained-refcount)
> 	if (fast_path)
> 		return;
> 
> 	// slow path:
> 	spin_lock(pamt_lock)
> 
> 	// Was the race lost with another 0=>1 increment?
> 	if (atomic_read(fine-grained-refcount) > 0)
> 		goto out_inc
> 
> 	tdh_phymem_pamt_add(page, pamt_pa_array)
> 	// Inc after the TDCALL so another thread won't race ahead of us
> 	// and try to use a non-existent PAMT entry
> out_inc:
> 	atomic_inc(fine-grained-refcount)
> 	spin_unlock(pamt_lock)
> 
> Then, at least only the 0=>1 and 1=>0 transitions need the global lock.
> The fast paths only touch the refcount which isn't shared nearly as much
> as the global lock.


This is pretty much what the next patch does "x86/virt/tdx: Optimize
tdx_alloc/free_control_page() helpers", although it doesn't use the
atomic_dec_and_lock() helpers. There are a few extra considerations. The get/put
fast paths can race, so inside the lock it has to double check that work or
otherwise handle the race. This lead the code to be complex enough that it was
split into too patches "dumb and correct" and "smart and complicated".

I'm wasn't familiar with atomic_dec_and_lock(). I'm guess the atomic part
doesn't cover both decrementing *and* taking the lock?