Re: [PATCH v6 02/11] x86/virt/tdx: Allocate page bitmap for Dynamic PAMT
From: Sohil Mehta
Date: Tue Jul 07 2026 - 23:36:06 EST
On 7/7/2026 7:10 PM, Edgecombe, Rick P wrote:
> On Tue, 2026-07-07 at 19:07 -0700, Rick Edgecombe wrote:
>>> I think you can avoid the comment altogether in that case.
>>
>> I don't think it removes the need for a comment. The point is if
>> tdx_supports_dynamic_pamt() is not supported. The comment should be more about
>> "why", than what the code does, right?
>>
>> How about:
>>
>> if (ret)
>> return ret;
>>
>> /*
>> * The kernel supports using TDX without Dynamic PAMT, so
>> * avoid reporting failure if it's not supported.
>> */
>> if (tdx_supports_dynamic_pamt(sysinfo))
>> ret = get_tdx_sys_info_tdmr_dpamt(&sysinfo->tdmr);
>>
>> return ret;
>
> Actually this really stands out from the autogenerated code now, how about:
>
I thought that's a good thing. Eventually, we want to make this file
conform to the standard kernel style, right? The autogenerated pattern
in this file to "check any previous failure" before doing the current
step is odd.
> /*
> * The kernel supports using TDX without Dynamic PAMT, so
> * avoid reporting failure if it's not supported.
> */
> if (!ret && tdx_supports_dynamic_pamt(sysinfo))
> ret = get_tdx_sys_info_tdmr_dpamt(&sysinfo->tdmr);
>
> It why-ifys the comment more, but only gently moves from the auto-generated
> patterns.
Sure, that's fine if you want to continue the style in this file.
BTW, do you also want to clarify why we fail the init if
get_tdx_sys_info_tdmr_dpamt() fails?
Is it because:
If reading the DPAMT metadata fails, something is really wrong with the
TDX module. It is better to fail the initialization in that case.