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

[Duan, Zhenzhong]

>-----Original Message-----
>From: Li, Xiaoyao <xiaoyao.li@xxxxxxxxx>
>Subject: Re: [RFC PATCH 05/15] virt: tdx-guest: Add TDCM helpers and TEE-IO
>support check
>
>On 9/24/2026 12:10 PM, 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.
>
>I think it needs to be split into 2 patches:
>- implement tdx_tdcm_* and justify why poll mode in chosen instread of
>notification.
>
>- Add TDCM_OP_CHECK_TEEIO_SUPP check to tdx_devsec_lock(). We need explain
>what
>it does, why we need it, and why it needs to be in tdx_devsec_lock() as the
>first step.

Hmm, that may break build because we define some static functions without user.

>
>> Signed-off-by: Zhenzhong Duan <zhenzhong.duan@xxxxxxxxx>
>> ---
>>  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,
>
>why not naming them TDCM_xxx_ERROR as GHCI definitions?
>
>TDCM_ERROR_TDX_MODULE reads like lacking some information at the end. For
>example, it can be TDCM_ERROR_TDX_MODULE_DURING_UPDATE.

Make sense, will do.

>
>> +};
>> +
>> +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[];
>
>since data is not used in tdx_is_dev_teeio_support(), is it really needed?

Will drop it.

>
>> +} __packed;
>> +
>> +#define MAX_TDI_REPORT_SIZE	(16 * PAGE_SIZE)
>
>Add some info in the changelog of where the 16 comes?

OK, I remember it's a hard limitation in TDISP spec.

>
>>  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.
>
>As I commented in patch 3, can we add more info of devid. What is it
>supposed to be?

We will use pci_dev pointer here instead of devid.

Thanks
Zhenzhong