linux-kernel - Re: [PATCH v4 06/16] x86/virt/tdx: Improve PAMT refcounts allocation for sparse memory

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: Windows password security audit tool. GUI, reports in PDF.

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

Message-ID: <69a2dee2-f6a5-4c1b-9daa-8c32ff7c3956@suse.com>
Date: Thu, 27 Nov 2025 09:36:00 +0200
From: Nikolay Borisov <nik.borisov@...e.com>
To: "Edgecombe, Rick P" <rick.p.edgecombe@...el.com>,
 "kvm@...r.kernel.org" <kvm@...r.kernel.org>,
 "linux-coco@...ts.linux.dev" <linux-coco@...ts.linux.dev>,
 "Huang, Kai" <kai.huang@...el.com>, "Li, Xiaoyao" <xiaoyao.li@...el.com>,
 "Hansen, Dave" <dave.hansen@...el.com>, "Zhao, Yan Y"
 <yan.y.zhao@...el.com>, "Wu, Binbin" <binbin.wu@...el.com>,
 "kas@...nel.org" <kas@...nel.org>, "seanjc@...gle.com" <seanjc@...gle.com>,
 "mingo@...hat.com" <mingo@...hat.com>,
 "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
 "tglx@...utronix.de" <tglx@...utronix.de>,
 "Yamahata, Isaku" <isaku.yamahata@...el.com>,
 "pbonzini@...hat.com" <pbonzini@...hat.com>,
 "Annapurve, Vishal" <vannapurve@...gle.com>, "Gao, Chao"
 <chao.gao@...el.com>, "bp@...en8.de" <bp@...en8.de>,
 "x86@...nel.org" <x86@...nel.org>
Cc: "kirill.shutemov@...ux.intel.com" <kirill.shutemov@...ux.intel.com>
Subject: Re: [PATCH v4 06/16] x86/virt/tdx: Improve PAMT refcounts allocation
 for sparse memory



On 26.11.25 г. 22:47 ч., Edgecombe, Rick P wrote:
> Kiryl, curious if you have any comments on the below...
> 
> On Wed, 2025-11-26 at 16:45 +0200, Nikolay Borisov wrote:
>>> +static int pamt_refcount_populate(pte_t *pte, unsigned long addr, void
>>> *data)
>>> +{
>>> +	struct page *page;
>>> +	pte_t entry;
>>> +
>>> +	page = alloc_page(GFP_KERNEL | __GFP_ZERO);
>>> +	if (!page)
>>>     		return -ENOMEM;
>>>     
>>> +	entry = mk_pte(page, PAGE_KERNEL);
>>> +
>>> +	spin_lock(&init_mm.page_table_lock);
>>> +	/*
>>> +	 * PAMT refcount populations can overlap due to rounding of the
>>> +	 * start/end pfn. Make sure the PAMT range is only populated once.
>>> +	 */
>>> +	if (pte_none(ptep_get(pte)))
>>> +		set_pte_at(&init_mm, addr, pte, entry);
>>> +	else
>>> +		__free_page(page);
>>> +	spin_unlock(&init_mm.page_table_lock);
>>
>> nit: Wouldn't it be better to perform the pte_none() check before doing
>> the allocation thus avoiding needless allocations? I.e do the
>> alloc/mk_pte only after we are 100% sure we are going to use this entry.
> 
> Yes, but I'm also wondering why it needs init_mm.page_table_lock at all. Here is
> my reasoning for why it doesn't:
> 
> apply_to_page_range() takes init_mm.page_table_lock internally when it modified
> page tables in the address range (vmalloc). It needs to do this to avoid races
> with other allocations that share the upper level page tables, which could be on
> the ends of area that TDX reserves.
 > > But pamt_refcount_populate() is only operating on the PTE's for the 
address
> range that TDX code already controls. Vmalloc should not free the PMD underneath
> the PTE operation because there is an allocation in any page tables it covers.
> So we can skip the lock and also do the pte_none() check before the page
> allocation as Nikolay suggests.

I agree with your analysis but this needs to be described not only in 
the commit message but also as a code comment because you intentionally 
omit locking since that particular pte (at that point) can only have a 
single user so no race conditions are possible.

> 
> Same for the depopulate path.