Re: [PATCH v4 03/24] coco/tdx-host: Expose TDX Module version

[Huang, Kai]

On Thu, 2026-02-12 at 06:35 -0800, Chao Gao wrote:
> For TDX Module updates, userspace needs to select compatible update
> versions based on the current module version. This design delegates
> module selection complexity to userspace because TDX Module update
> policies are complex and version series are platform-specific.
> 
> For example, the 1.5.x series is for certain platform generations, while
> the 2.0.x series is intended for others. And TDX Module 1.5.x may be
> updated to 1.5.y but not to 1.5.y+1.
> 
> Expose the TDX Module version to userspace via sysfs to aid module
> selection. Since the TDX faux device will drive module updates, expose
> the version as its attribute.
> 
> One bonus of exposing TDX Module version via sysfs is: TDX Module
> version information remains available even after dmesg logs are cleared.
> 
> == Background ==
> 
> The "faux device + device attribute" approach compares to other update
> mechanisms as follows:

This "faux device + device attribute" approach seems to be a wider design
choice instead of how to expose module version (which is the scope of this
patch).  Overall, shouldn't this be in the changelog of the previous patch
which actually introduces "faux device" (albeit no attribute is introduced
in that patch)?
 
> 
> 1. AMD SEV leverages an existing PCI device for the PSP to expose
>    metadata. TDX uses a faux device as it doesn't have PCI device
>    in its architecture.

E.g., this sounds to justify "why to use faux device for TDX", but not "to
expose module version via faux device attributes".

> 
> 2. Microcode uses per-CPU virtual devices to report microcode revisions
>    because CPUs can have different revisions. But, there is only a
>    single TDX Module, so exposing the TDX Module version through a global
>    TDX faux device is appropriate

This is related to exposing module version, but to me "there's only a single
TDX module" is also more like a justification to use "one faux device",
which should belong to changelog of previous patch too.

With "there's only a single TDX module" being said in previous patch
changelog, I think we can safely deduce that there's only "one module
version" but not per-cpu (thus I don't think we even need to call this out
in _this_ patch).

> 
> 3. ARM's CCA implementation isn't in-tree yet, but will likely follow a
>    similar faux device approach [1], though it's unclear whether they need
>    to expose firmware version information

Again, I don't feel "follow a similar faux device approach" for ARM CCA
should be a justification of "exposing module version via faux attributes".
It should be a justification of "using faux device for TDX".

> 
> Signed-off-by: Chao Gao <chao.gao@xxxxxxxxx>
> Reviewed-by: Binbin Wu <binbin.wu@xxxxxxxxxxxxxxx>
> Reviewed-by: Tony Lindgren <tony.lindgren@xxxxxxxxxxxxxxx>
> Reviewed-by: Xu Yilun <yilun.xu@xxxxxxxxxxxxxxx>
> Link: https://lore.kernel.org/all/2025073035-bulginess-rematch-b92e@gregkh/ # [1]
> 

[...]

> +Description:	(RO) Report the version of the loaded TDX Module. The TDX Module
> +		version is formatted as x.y.z, where "x" is the major version,
> +		"y" is the minor version and "z" is the update version. Versions
> +		are used for bug reporting, TDX Module updates and etc.
							       ^

Nit: No need to use "and" before "etc".

Consulting google:

  No, it is not correct to say or write "and etc." Because etc. is an 
  abbreviation for the Latin phrase et cetera, which translates to "and
  other things" or "and the rest," including "and" makes the phrase 
  redundant. Using "and etc." is equivalent to saying "and and the rest".


> diff --git a/drivers/virt/coco/tdx-host/tdx-host.c b/drivers/virt/coco/tdx-host/tdx-host.c
> index c77885392b09..0424933b2560 100644
> --- a/drivers/virt/coco/tdx-host/tdx-host.c
> +++ b/drivers/virt/coco/tdx-host/tdx-host.c
> 

[...]

The actual code LGTM.