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

[Duan, Zhenzhong]

>-----Original Message-----
>From: Nikolay Borisov <nik.borisov@xxxxxxxx>
>Subject: Re: [RFC PATCH 05/15] virt: tdx-guest: Add TDCM helpers and TEE-IO
>support check
>
>
>
>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@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,
>> +};
>> +
>> +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?

Will use SZ_64K.

>
>> +
>>   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?)

Will update:

+#define TDCM_POLL_DELAY_US	1000
+#define TDCM_POLL_TIMEOUT_US	(10 * TDCM_POLL_DELAY_US)

>
>> +
>> +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:

TDVMCALL_GET_QUOTE is another example of a request whose
completion is handled asynchronously.

Will add below to changelog:
a TDCM operation may take longer than is appropriate for a single
TD.VMCALL invocation. The host acknowledges submission and reports
completion through the shared status field. The guest waits for that status
with a bounded polling timeout and delay, rather than remaining in the
hypercall for the whole operation. This also avoids holding the vCPU in a
long-running hypercall that could trigger a guest soft-lockup warning.

>
>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.

"TDCM module" is GHCI terminology. In this series, the guest-side
implementation is in the TDX TSM driver; it is not an NRX module.

TDCM requests are forwarded to the host, which handles them using the
applicable TDX Connect operations. Some of those host-side operations
use the NRX facility for TDISP processing, but that is an implementation
detail of those operations, not how TDCM itself is implemented.

Thanks
Zhenzhong