Re: [PATCH v9 04/11] x86/virt/tdx: Allocate refcounts for Dynamic PAMT memory

[email protected]
Newsgroups org.kernel.vger.kvm
Message-ID <[email protected]>
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] vzalloc() lacks __GFP_RETRY_MAYFAIL, causing it to invoke the OOM killer and panic the kernel during boot on systems with sparse memory layouts.
--

--- Patch [4]: [PATCH v9 04/11] x86/virt/tdx: Allocate refcounts for Dynamic PAMT memory ---
commit b63d4342b3d95af2bbe50ca5c66ac1ecbf04dedc
Author: Kiryl Shutsemau <[email protected]>

x86/virt/tdx: Allocate refcounts for Dynamic PAMT memory

This commit allocates reference counters for every 2MB memory range to track
TDX memory usage. The allocation size is based on max_pfn and can be very 
large on platforms with sparse memory layouts. If allocation fails, it is
intended to return -ENOMEM and fail TDX initialization.

> diff --git a/arch/x86/virt/vmx/tdx/tdx.c b/arch/x86/virt/vmx/tdx/tdx.c
> index 4d3427b8a5b71..b0ebd74a69fe2 100644
> --- a/arch/x86/virt/vmx/tdx/tdx.c
> +++ b/arch/x86/virt/vmx/tdx/tdx.c
[ ... ]
> @@ -252,6 +261,42 @@ static struct syscore tdx_syscore = {
>  	.ops = &tdx_syscore_ops,
>  };
>  
> +/*
> + * Allocate PAMT reference counters for all physical memory.
> + *
> + * It consumes 2MB for every 1TB of physical memory.
> + */
> +static __init int init_pamt_refcounts(void)
> +{
> +	size_t size = DIV_ROUND_UP(max_pfn, PTRS_PER_PTE) * sizeof(*pamt_refcounts);
> +
> +	if (!tdx_supports_dynamic_pamt(&tdx_sysinfo))
> +		return 0;
> +
> +	pamt_refcounts = vzalloc(size);
> +	if (!pamt_refcounts)
> +		return -ENOMEM;

[Severity: High]
Could this vzalloc() call trigger a kernel panic on systems with sparse
memory layouts instead of gracefully disabling TDX?

The allocation size based on max_pfn can be extremely large (up to 8GB).
Because vzalloc() does not include __GFP_RETRY_MAYFAIL, an allocation
failure will invoke the page allocator slow path and the OOM killer. Since
this runs during early boot before any killable user processes exist, the
OOM killer will unconditionally call panic() instead of returning NULL.

Would it be safer to use __vmalloc() with GFP_KERNEL | __GFP_ZERO |
__GFP_RETRY_MAYFAIL | __GFP_NOWARN to ensure the allocator returns NULL
when memory is exhausted?

> +
> +	return 0;
> +}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=4
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.