From: Dave Hansen <dave.hansen@linux.intel.com>
To: dave.hansen@linux.intel.com
Subject: Re: [PATCH v9 04/23] coco/tdx-host: Introduce a "tdx_host" device
In-Reply-To: <20260513151045.1420990-5-chao.gao@intel.com>
References: <20260513151045.1420990-5-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 int __init tdx_host_init(void)
> +{
> +	if (!x86_match_cpu(tdx_host_ids) || !tdx_get_sysinfo())
> +		return -ENODEV;
> +
> +	fdev = faux_device_create(KBUILD_MODNAME, NULL, NULL);
> +	if (!fdev)
> +		return -ENODEV;

Two ergonomic issues that will save somebody triage time later
(Dave_Hansen.txt RULE 2, RULE 12):

1. Both failure paths return -ENODEV silently.  When this driver
   doesn't load on a system that the user *thinks* should support TDX,
   they have no way to tell which branch fired without instrumenting
   the kernel.  At minimum:

     - if (!x86_match_cpu(...)) { pr_info("CPU lacks TDX host platform feature\n"); return -ENODEV; }
     - if (!tdx_get_sysinfo())  { pr_info("TDX module not initialized\n"); return -ENODEV; }

   Use pr_fmt + pr_info_once so this lands as "tdx_host: ..." in dmesg
   once per boot and doesn't spam.

2. faux_device_create() returning NULL is a black box.  If the API
   itself loses the underlying errno, file a follow-up to make it
   return an ERR_PTR.  Not blocking on this patch.

> -EXPORT_SYMBOL_FOR_KVM(tdx_get_sysinfo);
> +EXPORT_SYMBOL_FOR_MODULES(tdx_get_sysinfo, "kvm-intel,tdx-host");

Make sure this is the right namespace string for tdx-host (KBUILD_MODNAME
is "tdx_host" / hyphen-or-underscore).  Worth a one-line comment above
the EXPORT explaining the allow-list since this is the first user of
the new form in this subtree.
