linux-kernel - Re: [PATCHv2 03/12] x86/virt/tdx: Allocate reference counters for PAMT 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: <104abe744282dba34456d467e4730058ec2e7d99.camel@intel.com>
Date: Thu, 26 Jun 2025 00:53:29 +0000
From: "Huang, Kai" <kai.huang@...el.com>
To: "kirill.shutemov@...ux.intel.com" <kirill.shutemov@...ux.intel.com>,
	"pbonzini@...hat.com" <pbonzini@...hat.com>, "seanjc@...gle.com"
	<seanjc@...gle.com>, "dave.hansen@...ux.intel.com"
	<dave.hansen@...ux.intel.com>
CC: "Gao, Chao" <chao.gao@...el.com>, "Edgecombe, Rick P"
	<rick.p.edgecombe@...el.com>, "bp@...en8.de" <bp@...en8.de>, "x86@...nel.org"
	<x86@...nel.org>, "mingo@...hat.com" <mingo@...hat.com>, "Zhao, Yan Y"
	<yan.y.zhao@...el.com>, "tglx@...utronix.de" <tglx@...utronix.de>,
	"kvm@...r.kernel.org" <kvm@...r.kernel.org>, "linux-coco@...ts.linux.dev"
	<linux-coco@...ts.linux.dev>, "Yamahata, Isaku" <isaku.yamahata@...el.com>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: [PATCHv2 03/12] x86/virt/tdx: Allocate reference counters for
 PAMT memory


> +static int init_pamt_metadata(void)
> +{
> +	size_t size = max_pfn / PTRS_PER_PTE * sizeof(*pamt_refcounts);
> +	struct vm_struct *area;
> +
> +	if (!tdx_supports_dynamic_pamt(&tdx_sysinfo))
> +		return 0;
> +
> +	/*
> +	 * Reserve vmalloc range for PAMT reference counters. It covers all
> +	 * physical address space up to max_pfn. It is going to be populated
> +	 * from init_tdmr() only for present memory that available for TDX use.
		^
		build_tdx_memlist()

> +	 */
> +	area = get_vm_area(size, VM_IOREMAP);

I am not sure why VM_IOREMAP is used?  Or should we just use vmalloc()?

> +	if (!area)
> +		return -ENOMEM;
> +
> +	pamt_refcounts = area->addr;
> +	return 0;
> +}
> +
> +static void free_pamt_metadata(void)
> +{
> +	size_t size = max_pfn / PTRS_PER_PTE * sizeof(*pamt_refcounts);
> +
> +	if (!tdx_supports_dynamic_pamt(&tdx_sysinfo))
> +		return;
> +
> +	size = round_up(size, PAGE_SIZE);
> +	apply_to_existing_page_range(&init_mm,
> +				     (unsigned long)pamt_refcounts,
> +				     size, pamt_refcount_depopulate,
> +				     NULL);
> +	vfree(pamt_refcounts);
> +	pamt_refcounts = NULL;
> +}
> +
>  /*
>   * Add a memory region as a TDX memory block.  The caller must make sure
>   * all memory regions are added in address ascending order and don't
> @@ -248,6 +347,10 @@ static int build_tdx_memlist(struct list_head *tmb_list)
>  		ret = add_tdx_memblock(tmb_list, start_pfn, end_pfn, nid);
>  		if (ret)
>  			goto err;
> +
> +		ret = alloc_pamt_refcount(start_pfn, end_pfn);
> +		if (ret)
> +			goto err;

So this would goto the error path, which only calls free_tdx_memlist(),
which frees all existing TDX memory blocks that have already created.

Logically, it would be great to also free PAMT refcount pages too, but they
all can be freed at free_pamt_metadata() eventually, so it's OK.

But I think it would still be helpful to put a comment before
free_tdx_memlist() in the error path to call out.  Something like:

err:
	/*
	 * This only frees all TDX memory blocks that have been created.
	 * All PAMT refcount pages will be freed when init_tdx_module() 
	 * calls free_pamt_metadata() eventually.
	 */
	free_tdx_memlist(tmb_list);
	return ret;

> 
>  
>  	return 0;
> @@ -1110,10 +1213,15 @@ static int init_tdx_module(void)
>  	 */
>  	get_online_mems();
>  
> -	ret = build_tdx_memlist(&tdx_memlist);
> +	/* Reserve vmalloc range for PAMT reference counters */
> +	ret = init_pamt_metadata();
>  	if (ret)
>  		goto out_put_tdxmem;
>  
> +	ret = build_tdx_memlist(&tdx_memlist);
> +	if (ret)
> +		goto err_free_pamt_metadata;
> +
>  	/* Allocate enough space for constructing TDMRs */
>  	ret = alloc_tdmr_list(&tdx_tdmr_list, &tdx_sysinfo.tdmr);
>  	if (ret)
> @@ -1171,6 +1279,8 @@ static int init_tdx_module(void)
>  	free_tdmr_list(&tdx_tdmr_list);
>  err_free_tdxmem:
>  	free_tdx_memlist(&tdx_memlist);
> +err_free_pamt_metadata:
> +	free_pamt_metadata();
>  	goto out_put_tdxmem;
>  }
>