Re: [RFC PATCH 01/27] coco/tdx-host: Introduce a "tdx_host" device

[Jonathan Cameron <jonathan.cameron@huawei.com>]

On Fri, 19 Sep 2025 07:22:10 -0700
Dan Williams <dan.j.williams@intel.com> wrote:

> From: Chao Gao <chao.gao@intel.com>
> 
> TDX depends on a platform firmware module that is invoked via instructions
> similar to vmenter (i.e. enter into a new privileged "root-mode" context to
> manage private memory and private device mechanisms). It is a software
> construct that depends on the CPU vmxon state to enable invocation of
> TDX-module ABIs. Unlike other Trusted Execution Environment (TEE) platform
> implementations that employ a firmware module running on a PCI device with
> an MMIO mailbox for communication, TDX has no hardware device to point to
> as the TEE Secure Manager (TSM).
> 
> Create a virtual device not only to align with other implementations but
> also to make it easier to
> 
>  - expose metadata (e.g., TDX module version, seamldr version etc) to
>    the userspace as device attributes
> 
>  - implement firmware uploader APIs which are tied to a device. This is
>    needed to support TDX module runtime updates
> 
>  - enable TDX Connect which will share a common infrastructure with other
>    platform implementations. In the TDX Connect context, every
>    architecture has a TSM, represented by a PCIe or virtual device. The
>    new "tdx_host" device will serve the TSM role.
> 
> A faux device is used as for TDX because the TDX module is singular within
> the system and lacks associated platform resources. Using a faux device
> eliminates the need to create a stub bus.
> 
> The call to tdx_enable() makes the new module independent of kvm_intel.ko.
> For example, TDX Connect may be used to established to PCIe link encryption
> even if a TVM is never launched.  For now, just create the common loading
> infrastructure.
> 
> Co-developed-by: Xu Yilun <yilun.xu@linux.intel.com>
> Signed-off-by: Xu Yilun <yilun.xu@linux.intel.com>
> Signed-off-by: Chao Gao <chao.gao@intel.com>
> Signed-off-by: Dan Williams <dan.j.williams@intel.com>

I'm only taking a look to see a second example of how the core
code is used as I care mostly about the ARM one.  Anyhow, a
few passing comments inline.

> diff --git a/drivers/virt/coco/tdx-host/tdx-host.c b/drivers/virt/coco/tdx-host/tdx-host.c
> new file mode 100644
> index 000000000000..49c205913ef6
> --- /dev/null
> +++ b/drivers/virt/coco/tdx-host/tdx-host.c
> @@ -0,0 +1,52 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * TDX host user interface driver
> + *
> + * Copyright (C) 2025 Intel Corporation
> + */
> +
> +#include <linux/kernel.h>

There is general rework ongoing to stop including kernel.h except
where strictly necessary.  Please check exactly what you need and
see if one or more more specific headers is appropriate.

> +#include <linux/module.h>
> +#include <linux/mod_devicetable.h>
> +#include <linux/sysfs.h>

Bring headers in with the patch that first uses them. I'm not immediately
spotting anything from this one in this patch.  Doing that just makes
it easier to see if there are excess headers included at the end of
building up the driver across a series.

> +#include <linux/device/faux.h>
> +#include <asm/cpu_device_id.h>
> +#include <asm/tdx.h>
> +#include <asm/tdx_global_metadata.h>
> +
> +static const struct x86_cpu_id tdx_host_ids[] = {
> +	X86_MATCH_FEATURE(X86_FEATURE_TDX_HOST_PLATFORM, NULL),
> +	{}
> +};
> +MODULE_DEVICE_TABLE(x86cpu, tdx_host_ids);
> +
> +static struct faux_device *fdev;
> +
> +static int __init tdx_host_init(void)
> +{
> +	int r;
> +
> +	if (!x86_match_cpu(tdx_host_ids))
> +		return -ENODEV;
> +
> +	/* Enable the usage of SEAMCALLs */

What's the logic behind doing that here rather than in probe
for the faux device?  Perhaps add something to this comment.

> +	r = tdx_enable();
> +	if (r)
> +		return r;
> +
> +	fdev = faux_device_create(KBUILD_MODNAME, NULL, NULL);
> +	if (!fdev)
> +		return -ENODEV;
> +
> +	return 0;
> +}
> +module_init(tdx_host_init);
> +
> +static void __exit tdx_host_exit(void)
> +{
> +	faux_device_destroy(fdev);
> +}
> +module_exit(tdx_host_exit);
> +
> +MODULE_DESCRIPTION("TDX Host Services");
> +MODULE_LICENSE("GPL");
> 
> base-commit: 0d1fbc1f1b7a3c8b14a643303dd89bcc82d3fbd0