linux-kernel - Re: [PATCH v4 07/16] x86/virt/tdx: Add tdx_alloc/free

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: <aTaXQSu0j/9Y0bsY@yzhao56-desk.sh.intel.com>
Date: Mon, 8 Dec 2025 17:15:45 +0800
From: Yan Zhao <yan.y.zhao@...el.com>
To: Rick Edgecombe <rick.p.edgecombe@...el.com>
CC: <bp@...en8.de>, <chao.gao@...el.com>, <dave.hansen@...el.com>,
	<isaku.yamahata@...el.com>, <kai.huang@...el.com>, <kas@...nel.org>,
	<kvm@...r.kernel.org>, <linux-coco@...ts.linux.dev>,
	<linux-kernel@...r.kernel.org>, <mingo@...hat.com>, <pbonzini@...hat.com>,
	<seanjc@...gle.com>, <tglx@...utronix.de>, <vannapurve@...gle.com>,
	<x86@...nel.org>, <xiaoyao.li@...el.com>, <binbin.wu@...el.com>, "Kirill A.
 Shutemov" <kirill.shutemov@...ux.intel.com>
Subject: Re: [PATCH v4 07/16] x86/virt/tdx: Add tdx_alloc/free_page() helpers

On Thu, Nov 20, 2025 at 04:51:16PM -0800, Rick Edgecombe wrote:
> +static int alloc_pamt_array(u64 *pa_array)
> +{
> +	struct page *page;
> +	int i;
> +
> +	for (i = 0; i < tdx_dpamt_entry_pages(); i++) {
> +		page = alloc_page(GFP_KERNEL_ACCOUNT);
> +		if (!page)
> +			goto err;
> +		pa_array[i] = page_to_phys(page);
> +	}
> +
> +	return 0;
> +err:
On failure to allocate the 2nd page, instead of zeroing out the rest of the
array and having the caller invoke free_pamt_array() to free the 1st page,
could we free the 1st page here directly?

Upon alloc_pamt_array() error, the pages shouldn't have been passed to the TDX
module, so there's no need to invoke tdx_quirk_reset_paddr() as in
free_pamt_array(), right?

It's also somewhat strange to have the caller to invoke free_pamt_array() after
alloc_pamt_array() fails.

> +	/*
> +	 * Zero the rest of the array to help with
> +	 * freeing in error paths.
> +	 */
> +	for (; i < tdx_dpamt_entry_pages(); i++)
> +		pa_array[i] = 0;
> +	return -ENOMEM;
> +}
> +
> +static void free_pamt_array(u64 *pa_array)
> +{
> +	for (int i = 0; i < tdx_dpamt_entry_pages(); i++) {
> +		if (!pa_array[i])
> +			break;
> +
> +		/*
> +		 * Reset pages unconditionally to cover cases
> +		 * where they were passed to the TDX module.
> +		 */
> +		tdx_quirk_reset_paddr(pa_array[i], PAGE_SIZE);
> +
> +		__free_page(phys_to_page(pa_array[i]));
> +	}
> +}
...
> +/* Bump PAMT refcount for the given page and allocate PAMT memory if needed */
> +int tdx_pamt_get(struct page *page)
> +{
> +	u64 pamt_pa_array[MAX_TDX_ARG_SIZE(rdx)];
> +	atomic_t *pamt_refcount;
> +	u64 tdx_status;
> +	int ret;
> +
> +	if (!tdx_supports_dynamic_pamt(&tdx_sysinfo))
> +		return 0;
> +
> +	ret = alloc_pamt_array(pamt_pa_array);
> +	if (ret)
> +		goto out_free;
> +
> +	pamt_refcount = tdx_find_pamt_refcount(page_to_pfn(page));
> +
> +	scoped_guard(spinlock, &pamt_lock) {
> +		/*
> +		 * If the pamt page is already added (i.e. refcount >= 1),
> +		 * then just increment the refcount.
> +		 */
> +		if (atomic_read(pamt_refcount)) {
> +			atomic_inc(pamt_refcount);
> +			goto out_free;
> +		}
> +
> +		/* Try to add the pamt page and take the refcount 0->1. */
> +
> +		tdx_status = tdh_phymem_pamt_add(page, pamt_pa_array);
> +		if (!IS_TDX_SUCCESS(tdx_status)) {
> +			pr_err("TDH_PHYMEM_PAMT_ADD failed: %#llx\n", tdx_status);
> +			goto out_free;
> +		}
> +
> +		atomic_inc(pamt_refcount);
> +	}
> +
> +	return ret;
> +out_free:
> +	/*
> +	 * pamt_pa_array is populated or zeroed up to tdx_dpamt_entry_pages()
> +	 * above. free_pamt_array() can handle either case.
> +	 */
> +	free_pamt_array(pamt_pa_array);
> +	return ret;
> +}
> +EXPORT_SYMBOL_GPL(tdx_pamt_get);