Re: [PATCH v6 01/11] x86/virt/tdx: Simplify tdmr_get_pamt_sz()

[Sohil Mehta <sohil.mehta@intel.com>]

On 5/25/2026 7:35 PM, Rick Edgecombe wrote:
> For each memory region that the TDX module might use (called TDMR), three
> separate traditional PAMT allocations are needed. 

> One for each supported page size (1GB, 2MB, 4KB).
Missing verb in this sentence. Maybe use a '-' to merge it with the
previous sentence.


> There are some commonalities in the math needed to calculate the base and
> size for each smaller allocation, and so an effort was made to share logic
> across the three. Unfortunately doing this turned out unnaturally tortured,
> with a loop iterating over the three page sizes, only to call into a
> function with cases statement for each page size. 

Did you mean "..with a case statement for each.."?



> diff --git a/arch/x86/virt/vmx/tdx/tdx.c b/arch/x86/virt/vmx/tdx/tdx.c
> index 967482ae3c801..487f389f52f4b 100644
> --- a/arch/x86/virt/vmx/tdx/tdx.c
> +++ b/arch/x86/virt/vmx/tdx/tdx.c
> @@ -516,31 +516,21 @@ static __init int fill_out_tdmrs(struct list_head *tmb_list,
>   * Calculate PAMT size given a TDMR and a page size.  The returned
>   * PAMT size is always aligned up to 4K page boundary.
>   */
> -static __init unsigned long tdmr_get_pamt_sz(struct tdmr_info *tdmr, int pgsz,
> -					     u16 pamt_entry_size)
> +static __init unsigned long tdmr_get_pamt_sz(struct tdmr_info *tdmr, int pgsz)
>  {
>  	unsigned long pamt_sz, nr_pamt_entries;
> +	const int tdx_pg_size_shift[] = { PAGE_SHIFT, PMD_SHIFT, PUD_SHIFT };

Both of these consts go hand-in-hand, right? I would write it as
tdx_pg_size_shift[TDX_PS_NR] to make the connection obvious.

Just curious, why is TDX_PS_NR defined as (TDX_PS_1G + 1)? I don't think
we are planning to add TDX_PS_256G anytime soon. But, should
TDX_PS_4K..TDX_PS_NR be an enum?

> +	const u16 pamt_entry_size[TDX_PS_NR] = {
> +		tdx_sysinfo.tdmr.pamt_4k_entry_size,
> +		tdx_sysinfo.tdmr.pamt_2m_entry_size,
> +		tdx_sysinfo.tdmr.pamt_1g_entry_size,
> +	};
>