Re: [RFC PATCH v2 09/10] x86/virt/tdx: Use early returns in get_tdx_sys_info()

[Edgecombe, Rick P]

On Fri, 2026-09-18 at 06:29 -0700, Chao Gao wrote:
> get_tdx_sys_info() was generated by a script. It chains its metadata reads
> with:
> 
>       ret = ret ?: get_tdx_sys_info_foo(...);
> 
> The function is maintained by hand now. Use conventional early returns
> instead.
> 
> AI was used under supervision to review code and workshop logs.
> 
> Signed-off-by: Chao Gao <chao.gao@xxxxxxxxx>
> ---
>  arch/x86/virt/vmx/tdx/tdx.c | 33 +++++++++++++++++++++++++--------
>  1 file changed, 25 insertions(+), 8 deletions(-)
> 
> diff --git a/arch/x86/virt/vmx/tdx/tdx.c b/arch/x86/virt/vmx/tdx/tdx.c
> index 9f9f2ef80f55..9d5a3296d3c0 100644
> --- a/arch/x86/virt/vmx/tdx/tdx.c
> +++ b/arch/x86/virt/vmx/tdx/tdx.c
> @@ -564,19 +564,33 @@ static __init int get_tdx_sys_info_td_conf(struct tdx_sys_info_td_conf *td_conf)
>  
>  static __init int get_tdx_sys_info(struct tdx_sys_info *sysinfo)
>  {
> -	int ret = 0;
> +	int ret;
>  
> -	ret = ret ?: get_tdx_sys_info_version(&sysinfo->version);
> +	ret = get_tdx_sys_info_version(&sysinfo->version);
>  
>  	pr_info("Module version: " TDX_VERSION_FMT "\n",
>  		sysinfo->version.major_version,
>  		sysinfo->version.minor_version,
>  		sysinfo->version.update_version);
>  
> -	ret = ret ?: get_tdx_sys_info_features(&sysinfo->features);
> -	ret = ret ?: get_tdx_sys_info_tdmr(&sysinfo->tdmr);
> -	ret = ret ?: get_tdx_sys_info_td_ctrl(&sysinfo->td_ctrl);
> -	ret = ret ?: get_tdx_sys_info_td_conf(&sysinfo->td_conf);
> +	if (ret)
> +		return ret;

It should not print the module versions if it failed to read them, right? I
think the reason why it was done this way was to fit into the existing pattern
and because printing zeros is not a horrible outcome. But that code pattern
caused problem no longer exists. So I'd think it's ok to fix it in this patch.
Before it kind of blended in. But now it easily stands out as buggy. 

> +
> +	ret = get_tdx_sys_info_features(&sysinfo->features);
> +	if (ret)
> +		return ret;
> +
> +	ret = get_tdx_sys_info_tdmr(&sysinfo->tdmr);
> +	if (ret)
> +		return ret;
> +
> +	ret = get_tdx_sys_info_td_ctrl(&sysinfo->td_ctrl);
> +	if (ret)
> +		return ret;
> +
> +	ret = get_tdx_sys_info_td_conf(&sysinfo->td_conf);
> +	if (ret)
> +		return ret;
>  
>  	/*
>  	 * The kernel supports using TDX without DPAMT, so
> @@ -584,10 +598,13 @@ static __init int get_tdx_sys_info(struct tdx_sys_info *sysinfo)
>  	 * try to support buggy TDX modules that advertise
>  	 * DPAMT but don't expose the metadata.
>  	 */
> -	if (!ret && tdx_supports_dynamic_pamt(sysinfo))
> +	if (tdx_supports_dynamic_pamt(sysinfo)) {
>  		ret = get_tdx_sys_info_tdmr_dpamt(&sysinfo->tdmr);
> +		if (ret)
> +			return ret;
> +	}
>  
> -	return ret;
> +	return 0;
>  }
>  
>  static __init int check_features(struct tdx_sys_info *sysinfo)