RE: [RFC PATCH 04/15] virt: tdx-guest: Support devsec TSM for secure devices

[Duan, Zhenzhong]

>-----Original Message-----
>From: Li, Xiaoyao <xiaoyao.li@xxxxxxxxx>
>Subject: Re: [RFC PATCH 04/15] virt: tdx-guest: Support devsec TSM for
>secure devices
>
>On 9/24/2026 12:10 PM, Zhenzhong Duan wrote:
>> Register Intel TDX secure device (devsec TSM) to the TSM framework and
>> supply the tdx_devsec_ops callback operations. This enables control and
>> management of the Trust Device Interface (TDI) through those hooks in
>> accordance with the TDISP protocol.
>
>It only implements the placeholders for these hooks. Please state them
>accurately.

Will do.

>
>> Check the TDCS configuration flags during driver initialization to
>> ensure the hardware interface is supported before attempting device
>> registration.
>>
>> Signed-off-by: Zhenzhong Duan <zhenzhong.duan@xxxxxxxxx>
>> ---
>>  arch/x86/include/asm/shared/tdx.h             |  1 +
>>  drivers/virt/coco/tdx-guest/Kconfig           | 15 ++++
>>  drivers/virt/coco/tdx-guest/Makefile          |  3 +
>>  drivers/virt/coco/tdx-guest/connect.c         | 81 +++++++++++++++++++
>>  .../coco/tdx-guest/{tdx-guest.c => main.c}    |  6 ++
>>  drivers/virt/coco/tdx-guest/tdx-guest.h       | 20 +++++
>>  6 files changed, 126 insertions(+)
>>  create mode 100644 drivers/virt/coco/tdx-guest/connect.c
>>  rename drivers/virt/coco/tdx-guest/{tdx-guest.c => main.c} (98%)
>>  create mode 100644 drivers/virt/coco/tdx-guest/tdx-guest.h
>>
>> diff --git a/arch/x86/include/asm/shared/tdx.h
>b/arch/x86/include/asm/shared/tdx.h
>> index 665c12925ff6..499103f7a01b 100644
>> --- a/arch/x86/include/asm/shared/tdx.h
>> +++ b/arch/x86/include/asm/shared/tdx.h
>> @@ -56,6 +56,7 @@
>>
>>  /* TDCS_CONFIG_FLAGS bits */
>>  #define TDCS_CONFIG_FLEXIBLE_PENDING_VE	BIT_ULL(1)
>> +#define TDCS_CONFIG_TDX_CONNECT		BIT_ULL(5)
>>
>>  /* TDCS_TD_CTLS bits */
>>  #define TD_CTLS_PENDING_VE_DISABLE_BIT	0
>> diff --git a/drivers/virt/coco/tdx-guest/Kconfig b/drivers/virt/coco/tdx-
>guest/Kconfig
>> index dbbdc14383b1..2e9407f8bc70 100644
>> --- a/drivers/virt/coco/tdx-guest/Kconfig
>> +++ b/drivers/virt/coco/tdx-guest/Kconfig
>> @@ -10,3 +10,18 @@ config TDX_GUEST_DRIVER
>>
>>  	  To compile this driver as module, choose M here. The module will
>>  	  be called tdx-guest.
>> +
>> +config TDX_CONNECT_GUEST
>> +	bool "Intel TDX Connect Guest Support"
>> +	depends on TDX_GUEST_DRIVER
>
>maybe select TDX_GUEST_DRIVER ?

My understanding is this will force enable TDX_GUEST_DRIVER without
checking its dependency. I'll keep it as is except you have strong
reason here.

>
>> +	depends on PCI_TSM
>> +	default y
>> +	help
>> +	  Support Trust Device Interface (TDI) feature enumeration,
>> +	  validation, and activation within an Intel TDX guest.
>> +
>> +	  This option enables the guest kernel to establish secure,
>> +	  isolated connections with trusted devices assigned to the TD,
>> +	  validating MMIO maps and authorizing DMA remapping tables.
>> +
>> +	  If unsure, say Y.
>> diff --git a/drivers/virt/coco/tdx-guest/Makefile
>b/drivers/virt/coco/tdx-guest/Makefile
>> index 775cb463f9c8..cfa991c7aeb5 100644
>> --- a/drivers/virt/coco/tdx-guest/Makefile
>> +++ b/drivers/virt/coco/tdx-guest/Makefile
>> @@ -1,2 +1,5 @@
>>  # SPDX-License-Identifier: GPL-2.0
>>  obj-$(CONFIG_TDX_GUEST_DRIVER) += tdx-guest.o
>> +
>> +tdx-guest-y := main.o
>> +tdx-guest-$(CONFIG_TDX_CONNECT_GUEST) += connect.o
>> diff --git a/drivers/virt/coco/tdx-guest/connect.c
>b/drivers/virt/coco/tdx-guest/connect.c
>> new file mode 100644
>> index 000000000000..80ae7c19e179
>> --- /dev/null
>> +++ b/drivers/virt/coco/tdx-guest/connect.c
>> @@ -0,0 +1,81 @@
>> +// SPDX-License-Identifier: GPL-2.0
>> +/*
>> + * TDX Connect guest driver
>> + *
>> + * Copyright (C) 2026 Intel Corporation
>> + */
>> +
>> +#define pr_fmt(fmt)		KBUILD_MODNAME ": tdx_connect: " fmt
>> +
>> +#include <linux/pci.h>
>> +#include <linux/pci-tsm.h>
>> +#include <linux/tsm.h>
>> +#include <asm/tdx.h>
>> +
>> +#include "tdx-guest.h"
>> +
>> +struct tdx_devsec {
>> +	struct pci_tsm_devsec pci;
>> +};
>> +
>> +static struct tdx_devsec *to_tdx_devsec(struct pci_tsm *tsm)
>> +{
>> +	return container_of(tsm, struct tdx_devsec, pci.base_tsm);
>> +}
>> +
>> +static struct pci_tsm *tdx_devsec_lock(struct tsm_dev *tsm_dev, struct
>pci_dev *pdev)
>> +{
>> +	int ret;
>> +
>> +	struct tdx_devsec *tdevsec __free(kfree) = kzalloc(sizeof(*tdevsec),
>GFP_KERNEL);
>> +	if (!tdevsec)
>> +		return ERR_PTR(-ENOMEM);
>> +
>> +	ret = pci_tsm_devsec_constructor(pdev, &tdevsec->pci, tsm_dev);
>> +	if (ret)
>> +		return ERR_PTR(ret);
>> +
>> +	return &no_free_ptr(tdevsec)->pci.base_tsm;
>> +}
>> +
>> +static void tdx_devsec_unlock(struct pci_tsm *tsm)
>> +{
>> +	struct tdx_devsec *tdevsec = to_tdx_devsec(tsm);
>> +
>> +	kfree(tdevsec);
>> +}
>> +
>> +static int tdx_devsec_run(struct pci_dev *pdev)
>> +{
>> +	return -EOPNOTSUPP;
>> +}
>> +
>> +static struct pci_tsm_ops tdx_devsec_ops = {
>> +	.lock = tdx_devsec_lock,
>> +	.unlock = tdx_devsec_unlock,
>> +	.run = tdx_devsec_run,
>> +};
>> +
>> +static void devsec_tsm_remove(void *tsm_dev)
>> +{
>> +	tsm_unregister(tsm_dev);
>> +}
>> +
>> +int tdx_connect_init(struct device *dev)
>> +{
>> +	struct tsm_dev *tsm_dev;
>> +	u64 config, ret;
>> +
>> +	ret = tdg_vm_rd(TDCS_CONFIG_FLAGS, &config);
>> +	if (ret)
>> +		return -EIO;
>
>The kernel already manadates and assumes TDCS_CONFIG_FLAGS is always
>supported
>in disable_sept_ve().
>
>Can we make them consistent? Maybe just treat config as 0 when reading
>TDCS_CONFIG_FLAGS fails?

I see existing code never check return value of tdg_vm_rd() no matter
what input, but tdg_vm_rd() is defined with return value. I don't know
why, but I'm fine to be consistent here.

>
>> +
>> +	if (!(config & TDCS_CONFIG_TDX_CONNECT))
>> +		return 0;
>> +
>> +	tsm_dev = tsm_register(dev, &tdx_devsec_ops);
>> +	if (IS_ERR(tsm_dev))
>> +		return PTR_ERR(tsm_dev);
>> +
>> +	return devm_add_action_or_reset(dev, devsec_tsm_remove, tsm_dev);
>> +}
>> diff --git a/drivers/virt/coco/tdx-guest/tdx-guest.c
>b/drivers/virt/coco/tdx-guest/main.c
>> similarity index 98%
>> rename from drivers/virt/coco/tdx-guest/tdx-guest.c
>> rename to drivers/virt/coco/tdx-guest/main.c
>> index d0303e31e816..801a1f2b5f3d 100644
>> --- a/drivers/virt/coco/tdx-guest/tdx-guest.c
>> +++ b/drivers/virt/coco/tdx-guest/main.c
>> @@ -25,6 +25,8 @@
>>  #include <asm/cpu_device_id.h>
>>  #include <asm/tdx.h>
>>
>> +#include "tdx-guest.h"
>> +
>>  /* TDREPORT buffer */
>>  static u8 *tdx_report_buf;
>>
>> @@ -428,6 +430,10 @@ static int __init tdx_guest_init(void)
>>  	if (ret)
>>  		goto free_quote;
>>
>> +	ret = tdx_connect_init(tdx_misc_dev.this_device);
>> +	if (ret)
>> +		pr_warn("Failed to enable TDX Connect: %d\n", ret);
>> +
>>  	return 0;
>>
>>  free_quote:
>> diff --git a/drivers/virt/coco/tdx-guest/tdx-guest.h
>b/drivers/virt/coco/tdx-guest/tdx-guest.h
>> new file mode 100644
>> index 000000000000..4218541c7f3b
>> --- /dev/null
>> +++ b/drivers/virt/coco/tdx-guest/tdx-guest.h
>> @@ -0,0 +1,20 @@
>> +/* SPDX-License-Identifier: GPL-2.0 */
>> +/*
>> + * TDX guest driver private declarations
>> + *
>> + * Copyright (C) 2026 Intel Corporation
>> + */
>> +
>> +#ifndef __TDX_GUEST_H
>> +#define __TDX_GUEST_H
>> +
>> +#include <linux/kernel.h>
>> +#include <linux/device.h>
>> +
>> +#ifdef CONFIG_TDX_CONNECT_GUEST
>> +extern int tdx_connect_init(struct device *dev);
>
>extern is not needed.	

Sure.

Thanks
Zhenzhong