From: Dave Hansen <dave.hansen@linux.intel.com>
To: dave.hansen@linux.intel.com
Subject: Re: [PATCH v9 05/23] coco/tdx-host: Expose TDX module version
In-Reply-To: <20260513151045.1420990-6-chao.gao@intel.com>
References: <20260513151045.1420990-6-chao.gao@intel.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

[Draft review generated by x86-maintainer-agent applying Dave_Hansen.txt
 rules to the v9 series.  Strip this banner before sending.]

> +static ssize_t version_show(struct device *dev, struct device_attribute *attr,
> +			    char *buf)
> +{
> +	const struct tdx_sys_info *tdx_sysinfo = tdx_get_sysinfo();
> +	const struct tdx_sys_info_version *ver;
> +
> +	if (!tdx_sysinfo)
> +		return -ENXIO;

-ENXIO here is a one-shot value that patch 22 documents as "the TDX
module is dead".  Worth a one-line code comment that explicitly ties
the errno to that meaning -- the next person to touch this file will
not have patch 22's ABI documentation open.  RULE 2.

  /*
   * tdx_get_sysinfo() returns NULL once the TDX module has
   * been torn down (e.g. after a failed runtime update).  See
   * Documentation/ABI/testing/sysfs-devices-faux-tdx-host for the
   * userspace-visible meaning of this -ENXIO.
   */

The TDX_VERSION_FMT macro plus its zero-pad comment is the right call
-- thanks for spelling out the convention next to the definition
(RULE 16, "use existing helpers / make a helper if you'll need it
twice").

ABI documentation is present (RULE 13).  Good.

Reviewed-by: Dave Hansen <dave.hansen@linux.intel.com>  [with the
                                                        comment added]