Re: [RFC PATCH 05/15] virt: tdx-guest: Add TDCM helpers and TEE-IO support check

[Nikolay Borisov <nik.borisov@suse.com>]

On 24.09.26 г. 7:10 ч., Zhenzhong Duan wrote:
> Implement internal helper wrappers tdx_tdcm_alloc(), tdx_tdcm_run(),
> and tdx_tdcm_free() to manage execution context lifecycle for TDX Connect
> TEE-IO Device Control and Management (TDCM) commands.
> 
> Use these infrastructures to deploy all TDCM commands to orchestrate
> trusted device management, beginning with the first command,
> TDCM_OP_CHECK_TEEIO_SUPP, executed during the secure device locking
> sequence.
> 
> Signed-off-by: Zhenzhong Duan <zhenzhong.duan@intel.com>
> ---
>   arch/x86/include/asm/tdx.h            |  48 +++++++++
>   drivers/virt/coco/tdx-guest/connect.c | 139 ++++++++++++++++++++++++++
>   2 files changed, 187 insertions(+)
> 
> diff --git a/arch/x86/include/asm/tdx.h b/arch/x86/include/asm/tdx.h
> index 4937e23d5483..71cd701d346f 100644
> --- a/arch/x86/include/asm/tdx.h
> +++ b/arch/x86/include/asm/tdx.h
> @@ -105,6 +105,54 @@ int tdx_mcall_extend_rtmr(u8 index, u8 *data);
>   u64 tdx_hcall_get_quote(u8 *buf, size_t size);
>   
>   #ifdef CONFIG_TDX_CONNECT_GUEST
> +enum tdcm_operation {
> +	TDCM_OP_CHECK_TEEIO_SUPP	= 1,
> +	TDCM_OP_BIND			= 2,
> +	TDCM_OP_GET_DEV_INFO		= 3,
> +	TDCM_OP_GET_TDI_REPORT		= 4,
> +	TDCM_OP_START_TDI		= 5,
> +	TDCM_OP_GET_TDI_STATE		= 6,
> +	TDCM_OP_UNBIND			= 7,
> +};
> +
> +enum tdcm_status {
> +	TDCM_STATUS_WAIT		= 0,
> +	TDCM_STATUS_COMPLETED		= 1,
> +	TDCM_STATUS_ERROR		= 2,
> +};
> +
> +enum tdcm_error {
> +	TDCM_ERROR_TDX_MODULE		= 10,
> +	TDCM_ERROR_TDXIO_DEVICE		= 11,
> +	TDCM_ERROR_SPDM_MESSAGE		= 12,
> +	TDCM_ERROR_IDE_KM_MESSAGE	= 13,
> +	TDCM_ERROR_TDISP_MESSAGE	= 14,
> +	TDCM_ERROR_INVALID_STATE	= 15,
> +};
> +
> +struct tdvmcall_tdcm {
> +	__u16 operation;  /* See enum tdcm_operation */
> +	__u8  version;
> +	__u8  rsvd0;
> +	__u32 rsvd1;
> +
> +	__u8  status;     /* See enum tdcm_status */
> +	__u8  error;      /* See enum tdcm_error */
> +	__u16 rsvd2;
> +	__u32 rsvd3;
> +
> +	__u32 data_length;
> +
> +	__u8  data[];
> +} __packed;
> +
> +struct tdcm_rsp_check_teeio_supp {
> +	__u8 is_supported;
> +	__u8  data[];
> +} __packed;
> +
> +#define MAX_TDI_REPORT_SIZE	(16 * PAGE_SIZE)

nit: You can use SZ_64K, no  or the report's size indeed scales with 
whether PAGE_SIZE is 4k or 2m for example?

> +
>   u64 tdx_hcall_tdcm(u16 devid, void *buf, size_t size, u8 vector);
>   int tdx_mcall_tdi_read(u64 func_id, u64 field, u64 *value);
>   #endif
> diff --git a/drivers/virt/coco/tdx-guest/connect.c b/drivers/virt/coco/tdx-guest/connect.c
> index 80ae7c19e179..1f52a187b30f 100644
> --- a/drivers/virt/coco/tdx-guest/connect.c
> +++ b/drivers/virt/coco/tdx-guest/connect.c
> @@ -7,13 +7,127 @@
>   
>   #define pr_fmt(fmt)		KBUILD_MODNAME ": tdx_connect: " fmt
>   
> +#include <linux/iopoll.h>
>   #include <linux/pci.h>
>   #include <linux/pci-tsm.h>
> +#include <linux/set_memory.h>
>   #include <linux/tsm.h>
>   #include <asm/tdx.h>
>   
>   #include "tdx-guest.h"
>   
> +/**
> + * struct tdcm_ctx - TEE-IO Device Configuration and Management (TDCM) context
> + * @buf: Pointer to 4KB-aligned shared memory (>= one page) used as the command
> + *       buffer on input and the response buffer on output.
> + * @buf_size: Total size of the shared memory buffer.
> + * @max_rsp_data_sz: Maximum expected size of the response data.
> + * @rsp_data_sz: Actual size of the response data populated by the VMM.
> + * @devid: Device Identifier.
> + */
> +struct tdcm_ctx {
> +	struct tdvmcall_tdcm *buf;
> +	size_t buf_size;
> +	u32 max_rsp_data_sz;
> +	u32 rsp_data_sz;
> +	u16 devid;
> +};
> +
> +#define to_tdcm_rsp_data(tdcm)	((tdcm)->buf->data)
> +
> +#define TDCM_POLL_DELAY_US	10000
> +#define TDCM_POLL_TIMEOUT_US	(10 * USEC_PER_SEC)

Shouldn't  POLL_TIMEOUT be a multiple of DELAY_US  for the sake of 
simplicity? I.e DELAY_US 1000 (wait for 1k usecs) TIME_OUT 10 * DELAY_US 
(try up to 10 times to get a response?)

> +
> +static int tdx_tdcm_run(struct tdcm_ctx *tdcm)
> +{
> +	struct tdvmcall_tdcm *buf = tdcm->buf;
> +	u8 status;
> +	int ret;
> +	u64 r;
> +
> +	r = tdx_hcall_tdcm(tdcm->devid, buf, tdcm->buf_size, 0);
> +	if (r)
> +		return -EFAULT;
> +
> +	ret = read_poll_timeout(READ_ONCE, status, status != TDCM_STATUS_WAIT,
> +				TDCM_POLL_DELAY_US, TDCM_POLL_TIMEOUT_US, false, buf->status);
> +	if (ret) {
> +		pr_err("TDCM request %d timed out\n", buf->operation);
> +		return ret;
> +	}

It's not entirely clear to me why do we have this asynchronous handling 
of TDCM, we don't have that for any of the other VP.VMCALL leafs? Can 
you explain this in the changelog. According to GHCI 2.0 TDCM is used:

This is used to allow TD guest TEE-IO Device Control and Management 
(TDCM) module to
manage the device. The TDCM leaf functions are defined in the following 
table

Is TDCM module implemented via the NRX facility ?  The TDX Connect spec 
never mentions the TDCM module. A bit more details are required in order 
to better understand why you've implemented this the way you have.



> +
> +	/* Ensure subsequent data reads are ordered after the status observation */
> +	virt_rmb();
> +
> +	/*
> +	 * Cache VMM response data length to avoid referencing buf->data_length
> +	 * directly in case a malicious VMM changes buf->data_length between the
> +	 * validation below and the subsequent reads.
> +	 */
> +	tdcm->rsp_data_sz = READ_ONCE(buf->data_length);
> +
> +	if (status != TDCM_STATUS_COMPLETED || tdcm->rsp_data_sz > tdcm->max_rsp_data_sz ||
> +	    (tdcm->max_rsp_data_sz && !tdcm->rsp_data_sz)) {
> +		pr_err("TDCM request %d failed: status 0x%x, error 0x%x, max_rsp_data_sz 0x%x, returned 0x%x\n",
> +		       buf->operation, status, buf->error, tdcm->max_rsp_data_sz,
> +		       tdcm->rsp_data_sz);
> +		return -EIO;
> +	}
> +
> +	return 0;
> +}

<snip>