[RFC PATCH 0/5] Towards a shared TSM sysfs-ABI for Confidential Computing

[RFC PATCH 0/5] Towards a shared TSM sysfs-ABI for Confidential Computing

[Dan Williams]

As you may recall the configfs-tsm proposal [1] for conveying
attestation reports, accepted into v6.7, made the case that shared
cross-vendor concepts demand shared infrastructure. Whereas configfs-tsm
only shares guest-side transport / blob-marshaling infrastructure, the
PCIe device-security interactions with a TSM are more standardized (PCIe
TDISP) and as a result present a wider opportunity for cross-vendor
shared-semantics, not just transport infrastructure.

The proposals in this set are an attempt to intercept and reconcile
details of 4 different patchsets in various stages of making their way
upstream, TDX host patches [2], TIO preview implementation [3], SPDM/CMA
support [4], and TDX Connect preview implementation (not posted). The
guiding principle for this RFC, beyond "common concepts demand shared
infrastructure", is that PCI core concepts, defined by the PCI spec,
should live in and interoperate with the PCI core (e.g. CMA <-> TSM
interaction). The result is a /sys/bus/virtual/devices/tsm0 singleton
class device proposal with a pci/ attribute directory and a
/sys/bus/pci/devices/$pdev/tsm/ attribute directory added to each PCI
capable device that in turn coordinates with
/sys/bus/pci/devices/$pdev/authenticated.

This gives TDX, and other software-module based TSMs like RISCV COVE, a
common place to host their module attributes in
/sys/bus/virtual/devices/tsm0/host, while for SEV-SNP, and other
hardware based TSMs like ARM CCA, that /sys/bus/virtual/tsm0/host is a
symlink back to /sys/bus/pci/devices/$tsm_dev. Beyond the sysfs
device-model, for TIO, COVE-IO, and TDX Connect this gives an initial
set of library calls, locking model, and operations to support the low
level implementations.

This set has only been verified to compile, it is not functional. It is
only meant to convey the scaffolding for low level implementations to
build upon. Expect it to change as low level implementations try to
adopt it.

Otherwise, this is the first step in demonstrating that we have our act
together in terms of unifying on a common-core interface language and
low-level interface verbs. I.e. "we" the device-security kernel
developer community regarding our conversation at the BoF at Plumbers
[5] where the outcome was to unify on a set of verbs and adopt the
semantics of the low level implementation that exports the least amount
of complexity to Linux while still meeting user expectations (as simple
as possible, but no simpler).

Note that this depends on a new core-sysfs capability (patch4), with
trending positive reaction from Greg [6], to hide entire sysfs-group
attribute directories when not applicable. This further lets
implementations get away from dynamic device-attribute creation, or
worse dynamic kobject creation, that makes device ABI subsystems hard to
reason about [7].

[1]: http://lore.kernel.org/r/654438f4ca604_3f6029413@dwillia2-mobl3.amr.corp.intel.com.notmuch
[2]: http://lore.kernel.org/r/20230331154432.00001373@gmail.com
[3]: https://github.com/aik/linux/tree/tio
[4]: http://lore.kernel.org/r/cover.1695921656.git.lukas@wunner.de
[5]: https://lpc.events/event/17/contributions/1635/
[6]: http://lore.kernel.org/r/2024012855-limb-monument-cd86@gregkh
[7]: http://lore.kernel.org/r/20231019200110.GA1410324@bhelgaas

---

Dan Williams (5):
      PCI/CMA: Prepare to interoperate with TSM authentication
      coco/tsm: Establish a new coco/tsm subdirectory
      coco/tsm: Introduce a shared class device for TSMs
      sysfs: Introduce a mechanism to hide static attribute_groups
      PCI/TSM: Authenticate devices via platform TSM


 Documentation/ABI/testing/sysfs-bus-pci   |   53 ++++
 Documentation/ABI/testing/sysfs-class-tsm |   35 +++
 drivers/pci/Kconfig                       |   15 +
 drivers/pci/Makefile                      |    2 
 drivers/pci/cma.c                         |   12 +
 drivers/pci/pci-sysfs.c                   |    3 
 drivers/pci/pci.h                         |   14 +
 drivers/pci/probe.c                       |    1 
 drivers/pci/remove.c                      |    1 
 drivers/pci/tsm.c                         |  346 +++++++++++++++++++++++++++++
 drivers/virt/coco/Kconfig                 |    6 -
 drivers/virt/coco/Makefile                |    4 
 drivers/virt/coco/sev-guest/sev-guest.c   |    8 -
 drivers/virt/coco/tdx-guest/tdx-guest.c   |    8 -
 drivers/virt/coco/tsm/Kconfig             |   14 +
 drivers/virt/coco/tsm/Makefile            |   10 +
 drivers/virt/coco/tsm/class.c             |  112 +++++++++
 drivers/virt/coco/tsm/pci.c               |   83 +++++++
 drivers/virt/coco/tsm/reports.c           |   24 +-
 drivers/virt/coco/tsm/tsm.h               |   28 ++
 fs/sysfs/group.c                          |   45 +++-
 include/linux/pci.h                       |    3 
 include/linux/sysfs.h                     |   63 ++++-
 include/linux/tsm.h                       |  109 ++++++++-
 include/uapi/linux/pci_regs.h             |    3 
 25 files changed, 935 insertions(+), 67 deletions(-)
 create mode 100644 Documentation/ABI/testing/sysfs-class-tsm
 create mode 100644 drivers/pci/tsm.c
 create mode 100644 drivers/virt/coco/tsm/Kconfig
 create mode 100644 drivers/virt/coco/tsm/Makefile
 create mode 100644 drivers/virt/coco/tsm/class.c
 create mode 100644 drivers/virt/coco/tsm/pci.c
 rename drivers/virt/coco/{tsm.c => tsm/reports.c} (94%)
 create mode 100644 drivers/virt/coco/tsm/tsm.h

The unstable baseline of this series is a merge of
https://github.com/l1k/linux/commit/f3dc1e0eb451 and v6.8-rc2.

[RFC PATCH 1/5] PCI/CMA: Prepare to interoperate with TSM authentication

[Dan Williams]

A TSM (TEE Security Manager) is a platform agent that facilitates TEE
I/O (device assignment for confidential VMs). It uses PCI CMA, IDE, and
TDISP to authenticate, encrypt/integrity-protect the link, and bind
device-virtual-functions capable of accessing private memory to
confidential VMs (TVMs).

Unlike native PCI CMA many of the details of establishing a connection
between a device (DSM) and the TSM are abstracted through platform APIs.
I.e. in the native case Linux picks the keys and validates the
certificates, in the TSM case Linux just sees a "success" from invoking
a "connect" API with the TSM.

SPDM only allows for one session-owner per transport (DOE), so the
expectation is that authentication will only ever be in the "native"
established case, or the "tsm" established case.

Convert the "authenticated" attribute to reflect {"none", "native"}
rather than {"0", "1"} in preparation for a follow-on {"none", "native",
"tsm"} possibility.

Note: Expect this patch gets folded into "PCI/CMA: Expose in sysfs
      whether devices are authenticated" and assume Linux never ships
      the binary authenticated ABI.

Cc: Wu Hao <hao.wu@intel.com>
Cc: Yilun Xu <yilun.xu@intel.com>
Cc: Lukas Wunner <lukas@wunner.de>
Cc: Samuel Ortiz <sameo@rivosinc.com>
Cc: Alexey Kardashevskiy <aik@amd.com>
Cc: Bjorn Helgaas <bhelgaas@google.com>
Signed-off-by: Dan Williams <dan.j.williams@intel.com>
---
 Documentation/ABI/testing/sysfs-bus-pci |   14 ++++++++------
 drivers/pci/cma.c                       |    7 ++++++-
 2 files changed, 14 insertions(+), 7 deletions(-)

diff --git a/Documentation/ABI/testing/sysfs-bus-pci b/Documentation/ABI/testing/sysfs-bus-pci
index bec7c197451e..35b0e11fd0e6 100644
--- a/Documentation/ABI/testing/sysfs-bus-pci
+++ b/Documentation/ABI/testing/sysfs-bus-pci
@@ -505,12 +505,14 @@ What:		/sys/bus/pci/devices/.../authenticated
 Date:		November 2023
 Contact:	Lukas Wunner <lukas@wunner.de>
 Description:
-		This file contains 1 if the device authenticated successfully
-		with CMA-SPDM (PCIe r6.1 sec 6.31).  It contains 0 if the
-		device failed authentication (and may thus be malicious).
-
-		Writing anything to this file causes reauthentication.
-		That may be opportune after updating the .cma keyring.
+		This file contains "native" if the device authenticated
+		successfully with CMA-SPDM (PCIe r6.1 sec 6.31). It contains
+		"none" if the device failed authentication (and may thus be
+		malicious).
+
+		Writing "native" to this file causes reauthentication with
+		kernel-selected keys and the kernel's certificate chain.  That
+		may be opportune after updating the .cma keyring.
 
 		The file is not visible if authentication is unsupported
 		by the device.
diff --git a/drivers/pci/cma.c b/drivers/pci/cma.c
index fb9bb5a637a5..be7d2bb21b4c 100644
--- a/drivers/pci/cma.c
+++ b/drivers/pci/cma.c
@@ -36,6 +36,9 @@ static ssize_t authenticated_store(struct device *dev,
 	    (pdev->cma_init_failed || pdev->doe_init_failed))
 		return -ENOTTY;
 
+	if (!sysfs_streq(buf, "native"))
+		return -EINVAL;
+
 	rc = pci_cma_reauthenticate(pdev);
 	if (rc)
 		return rc;
@@ -52,7 +55,9 @@ static ssize_t authenticated_show(struct device *dev,
 	    (pdev->cma_init_failed || pdev->doe_init_failed))
 		return -ENOTTY;
 
-	return sysfs_emit(buf, "%u\n", spdm_authenticated(pdev->spdm_state));
+	if (spdm_authenticated(pdev->spdm_state))
+		return sysfs_emit(buf, "native\n");
+	return sysfs_emit(buf, "none\n");
 }
 static DEVICE_ATTR_RW(authenticated);
 

[RFC PATCH 2/5] coco/tsm: Establish a new coco/tsm subdirectory

[Dan Williams]

In preparation for new + common TSM infrastructure, establish
drivers/virt/coco/tsm/. The tsm.ko module is renamed to tsm_reports.ko,
and some of its symbols moved to the "tsm_report_" namespace to separate
it from more generic "tsm" objects / symbols. The old tsm.ko module was
only ever demand loaded by kernel internal dependencies, so it should
not affect existing userspace module install scripts.

Cc: Wu Hao <hao.wu@intel.com>
Cc: Yilun Xu <yilun.xu@intel.com>
Cc: Samuel Ortiz <sameo@rivosinc.com>
Cc: Alexey Kardashevskiy <aik@amd.com>
Signed-off-by: Dan Williams <dan.j.williams@intel.com>
---
 drivers/virt/coco/Kconfig               |    6 ++----
 drivers/virt/coco/Makefile              |    4 ++--
 drivers/virt/coco/sev-guest/sev-guest.c |    8 ++++----
 drivers/virt/coco/tdx-guest/tdx-guest.c |    8 ++++----
 drivers/virt/coco/tsm/Kconfig           |    7 +++++++
 drivers/virt/coco/tsm/Makefile          |    6 ++++++
 drivers/virt/coco/tsm/reports.c         |   24 ++++++++++++------------
 include/linux/tsm.h                     |   24 ++++++++++++------------
 8 files changed, 49 insertions(+), 38 deletions(-)
 create mode 100644 drivers/virt/coco/tsm/Kconfig
 create mode 100644 drivers/virt/coco/tsm/Makefile
 rename drivers/virt/coco/{tsm.c => tsm/reports.c} (94%)

diff --git a/drivers/virt/coco/Kconfig b/drivers/virt/coco/Kconfig
index 87d142c1f932..1e2eb5e768f9 100644
--- a/drivers/virt/coco/Kconfig
+++ b/drivers/virt/coco/Kconfig
@@ -3,12 +3,10 @@
 # Confidential computing related collateral
 #
 
-config TSM_REPORTS
-	select CONFIGFS_FS
-	tristate
-
 source "drivers/virt/coco/efi_secret/Kconfig"
 
 source "drivers/virt/coco/sev-guest/Kconfig"
 
 source "drivers/virt/coco/tdx-guest/Kconfig"
+
+source "drivers/virt/coco/tsm/Kconfig"
diff --git a/drivers/virt/coco/Makefile b/drivers/virt/coco/Makefile
index 18c1aba5edb7..2c9d0a178678 100644
--- a/drivers/virt/coco/Makefile
+++ b/drivers/virt/coco/Makefile
@@ -1,8 +1,8 @@
 # SPDX-License-Identifier: GPL-2.0-only
 #
 # Confidential computing related collateral
-#
-obj-$(CONFIG_TSM_REPORTS)	+= tsm.o
+
 obj-$(CONFIG_EFI_SECRET)	+= efi_secret/
 obj-$(CONFIG_SEV_GUEST)		+= sev-guest/
 obj-$(CONFIG_INTEL_TDX_GUEST)	+= tdx-guest/
+obj-y				+= tsm/
diff --git a/drivers/virt/coco/sev-guest/sev-guest.c b/drivers/virt/coco/sev-guest/sev-guest.c
index 87f241825bc3..d058cb8f9708 100644
--- a/drivers/virt/coco/sev-guest/sev-guest.c
+++ b/drivers/virt/coco/sev-guest/sev-guest.c
@@ -786,7 +786,7 @@ struct snp_msg_cert_entry {
 static int sev_report_new(struct tsm_report *report, void *data)
 {
 	struct snp_msg_cert_entry *cert_table;
-	struct tsm_desc *desc = &report->desc;
+	struct tsm_report_desc *desc = &report->desc;
 	struct snp_guest_dev *snp_dev = data;
 	struct snp_msg_report_resp_hdr hdr;
 	const u32 report_size = SZ_4K;
@@ -885,14 +885,14 @@ static int sev_report_new(struct tsm_report *report, void *data)
 	return 0;
 }
 
-static const struct tsm_ops sev_tsm_ops = {
+static const struct tsm_report_ops sev_tsm_ops = {
 	.name = KBUILD_MODNAME,
 	.report_new = sev_report_new,
 };
 
 static void unregister_sev_tsm(void *data)
 {
-	tsm_unregister(&sev_tsm_ops);
+	tsm_report_unregister(&sev_tsm_ops);
 }
 
 static int __init sev_guest_probe(struct platform_device *pdev)
@@ -968,7 +968,7 @@ static int __init sev_guest_probe(struct platform_device *pdev)
 	snp_dev->input.resp_gpa = __pa(snp_dev->response);
 	snp_dev->input.data_gpa = __pa(snp_dev->certs_data);
 
-	ret = tsm_register(&sev_tsm_ops, snp_dev, &tsm_report_extra_type);
+	ret = tsm_report_register(&sev_tsm_ops, snp_dev, &tsm_report_extra_type);
 	if (ret)
 		goto e_free_cert_data;
 
diff --git a/drivers/virt/coco/tdx-guest/tdx-guest.c b/drivers/virt/coco/tdx-guest/tdx-guest.c
index 1253bf76b570..904f16461492 100644
--- a/drivers/virt/coco/tdx-guest/tdx-guest.c
+++ b/drivers/virt/coco/tdx-guest/tdx-guest.c
@@ -163,7 +163,7 @@ static int tdx_report_new(struct tsm_report *report, void *data)
 {
 	u8 *buf, *reportdata = NULL, *tdreport = NULL;
 	struct tdx_quote_buf *quote_buf = quote_data;
-	struct tsm_desc *desc = &report->desc;
+	struct tsm_report_desc *desc = &report->desc;
 	int ret;
 	u64 err;
 
@@ -278,7 +278,7 @@ static const struct x86_cpu_id tdx_guest_ids[] = {
 };
 MODULE_DEVICE_TABLE(x86cpu, tdx_guest_ids);
 
-static const struct tsm_ops tdx_tsm_ops = {
+static const struct tsm_report_ops tdx_tsm_ops = {
 	.name = KBUILD_MODNAME,
 	.report_new = tdx_report_new,
 };
@@ -301,7 +301,7 @@ static int __init tdx_guest_init(void)
 		goto free_misc;
 	}
 
-	ret = tsm_register(&tdx_tsm_ops, NULL, NULL);
+	ret = tsm_report_register(&tdx_tsm_ops, NULL, NULL);
 	if (ret)
 		goto free_quote;
 
@@ -318,7 +318,7 @@ module_init(tdx_guest_init);
 
 static void __exit tdx_guest_exit(void)
 {
-	tsm_unregister(&tdx_tsm_ops);
+	tsm_report_unregister(&tdx_tsm_ops);
 	free_quote_buf(quote_data);
 	misc_deregister(&tdx_misc_dev);
 }
diff --git a/drivers/virt/coco/tsm/Kconfig b/drivers/virt/coco/tsm/Kconfig
new file mode 100644
index 000000000000..69f04461c83e
--- /dev/null
+++ b/drivers/virt/coco/tsm/Kconfig
@@ -0,0 +1,7 @@
+# SPDX-License-Identifier: GPL-2.0-only
+#
+# TSM (TEE Security Manager) Common infrastructure
+
+config TSM_REPORTS
+	select CONFIGFS_FS
+	tristate
diff --git a/drivers/virt/coco/tsm/Makefile b/drivers/virt/coco/tsm/Makefile
new file mode 100644
index 000000000000..b48504a3ccfd
--- /dev/null
+++ b/drivers/virt/coco/tsm/Makefile
@@ -0,0 +1,6 @@
+# SPDX-License-Identifier: GPL-2.0-only
+#
+# TSM (TEE Security Manager) Common infrastructure
+
+obj-$(CONFIG_TSM_REPORTS) += tsm_reports.o
+tsm_reports-y := reports.o
diff --git a/drivers/virt/coco/tsm.c b/drivers/virt/coco/tsm/reports.c
similarity index 94%
rename from drivers/virt/coco/tsm.c
rename to drivers/virt/coco/tsm/reports.c
index d1c2db83a8ca..6cb0a0e6783d 100644
--- a/drivers/virt/coco/tsm.c
+++ b/drivers/virt/coco/tsm/reports.c
@@ -13,7 +13,7 @@
 #include <linux/configfs.h>
 
 static struct tsm_provider {
-	const struct tsm_ops *ops;
+	const struct tsm_report_ops *ops;
 	const struct config_item_type *type;
 	void *data;
 } provider;
@@ -98,7 +98,7 @@ static ssize_t tsm_report_privlevel_store(struct config_item *cfg,
 	 * SEV-SNP GHCB) and a minimum of a TSM selected floor value no less
 	 * than 0.
 	 */
-	if (provider.ops->privlevel_floor > val || val > TSM_PRIVLEVEL_MAX)
+	if (provider.ops->privlevel_floor > val || val > TSM_REPORT_PRIVLEVEL_MAX)
 		return -EINVAL;
 
 	guard(rwsem_write)(&tsm_rwsem);
@@ -134,7 +134,7 @@ static ssize_t tsm_report_inblob_write(struct config_item *cfg,
 	memcpy(report->desc.inblob, buf, count);
 	return count;
 }
-CONFIGFS_BIN_ATTR_WO(tsm_report_, inblob, NULL, TSM_INBLOB_MAX);
+CONFIGFS_BIN_ATTR_WO(tsm_report_, inblob, NULL, TSM_REPORT_INBLOB_MAX);
 
 static ssize_t tsm_report_generation_show(struct config_item *cfg, char *buf)
 {
@@ -201,7 +201,7 @@ static ssize_t tsm_report_read(struct tsm_report *report, void *buf,
 			       size_t count, enum tsm_data_select select)
 {
 	struct tsm_report_state *state = to_state(report);
-	const struct tsm_ops *ops;
+	const struct tsm_report_ops *ops;
 	ssize_t rc;
 
 	/* try to read from the existing report if present and valid... */
@@ -241,7 +241,7 @@ static ssize_t tsm_report_outblob_read(struct config_item *cfg, void *buf,
 
 	return tsm_report_read(report, buf, count, TSM_REPORT);
 }
-CONFIGFS_BIN_ATTR_RO(tsm_report_, outblob, NULL, TSM_OUTBLOB_MAX);
+CONFIGFS_BIN_ATTR_RO(tsm_report_, outblob, NULL, TSM_REPORT_OUTBLOB_MAX);
 
 static ssize_t tsm_report_auxblob_read(struct config_item *cfg, void *buf,
 				       size_t count)
@@ -250,7 +250,7 @@ static ssize_t tsm_report_auxblob_read(struct config_item *cfg, void *buf,
 
 	return tsm_report_read(report, buf, count, TSM_CERTS);
 }
-CONFIGFS_BIN_ATTR_RO(tsm_report_, auxblob, NULL, TSM_OUTBLOB_MAX);
+CONFIGFS_BIN_ATTR_RO(tsm_report_, auxblob, NULL, TSM_REPORT_OUTBLOB_MAX);
 
 #define TSM_DEFAULT_ATTRS() \
 	&tsm_report_attr_generation, \
@@ -353,10 +353,10 @@ static struct configfs_subsystem tsm_configfs = {
 	.su_mutex = __MUTEX_INITIALIZER(tsm_configfs.su_mutex),
 };
 
-int tsm_register(const struct tsm_ops *ops, void *priv,
-		 const struct config_item_type *type)
+int tsm_report_register(const struct tsm_report_ops *ops, void *priv,
+			const struct config_item_type *type)
 {
-	const struct tsm_ops *conflict;
+	const struct tsm_report_ops *conflict;
 
 	if (!type)
 		type = &tsm_report_default_type;
@@ -375,9 +375,9 @@ int tsm_register(const struct tsm_ops *ops, void *priv,
 	provider.type = type;
 	return 0;
 }
-EXPORT_SYMBOL_GPL(tsm_register);
+EXPORT_SYMBOL_GPL(tsm_report_register);
 
-int tsm_unregister(const struct tsm_ops *ops)
+int tsm_report_unregister(const struct tsm_report_ops *ops)
 {
 	guard(rwsem_write)(&tsm_rwsem);
 	if (ops != provider.ops)
@@ -387,7 +387,7 @@ int tsm_unregister(const struct tsm_ops *ops)
 	provider.type = NULL;
 	return 0;
 }
-EXPORT_SYMBOL_GPL(tsm_unregister);
+EXPORT_SYMBOL_GPL(tsm_report_unregister);
 
 static struct config_group *tsm_report_group;
 
diff --git a/include/linux/tsm.h b/include/linux/tsm.h
index de8324a2223c..28753608fcf5 100644
--- a/include/linux/tsm.h
+++ b/include/linux/tsm.h
@@ -5,25 +5,25 @@
 #include <linux/sizes.h>
 #include <linux/types.h>
 
-#define TSM_INBLOB_MAX 64
-#define TSM_OUTBLOB_MAX SZ_32K
+#define TSM_REPORT_INBLOB_MAX 64
+#define TSM_REPORT_OUTBLOB_MAX SZ_32K
 
 /*
  * Privilege level is a nested permission concept to allow confidential
  * guests to partition address space, 4-levels are supported.
  */
-#define TSM_PRIVLEVEL_MAX 3
+#define TSM_REPORT_PRIVLEVEL_MAX 3
 
 /**
- * struct tsm_desc - option descriptor for generating tsm report blobs
+ * struct tsm_report_desc - option descriptor for generating tsm report blobs
  * @privlevel: optional privilege level to associate with @outblob
  * @inblob_len: sizeof @inblob
  * @inblob: arbitrary input data
  */
-struct tsm_desc {
+struct tsm_report_desc {
 	unsigned int privlevel;
 	size_t inblob_len;
-	u8 inblob[TSM_INBLOB_MAX];
+	u8 inblob[TSM_REPORT_INBLOB_MAX];
 };
 
 /**
@@ -35,7 +35,7 @@ struct tsm_desc {
  * @auxblob: (optional) auxiliary data to the report (e.g. certificate data)
  */
 struct tsm_report {
-	struct tsm_desc desc;
+	struct tsm_report_desc desc;
 	size_t outblob_len;
 	u8 *outblob;
 	size_t auxblob_len;
@@ -43,7 +43,7 @@ struct tsm_report {
 };
 
 /**
- * struct tsm_ops - attributes and operations for tsm instances
+ * struct tsm_report_ops - attributes and operations for tsm instances
  * @name: tsm id reflected in /sys/kernel/config/tsm/report/$report/provider
  * @privlevel_floor: convey base privlevel for nested scenarios
  * @report_new: Populate @report with the report blob and auxblob
@@ -52,7 +52,7 @@ struct tsm_report {
  * Implementation specific ops, only one is expected to be registered at
  * a time i.e. only one of "sev-guest", "tdx-guest", etc.
  */
-struct tsm_ops {
+struct tsm_report_ops {
 	const char *name;
 	const unsigned int privlevel_floor;
 	int (*report_new)(struct tsm_report *report, void *data);
@@ -63,7 +63,7 @@ extern const struct config_item_type tsm_report_default_type;
 /* publish @privlevel, @privlevel_floor, and @auxblob attributes */
 extern const struct config_item_type tsm_report_extra_type;
 
-int tsm_register(const struct tsm_ops *ops, void *priv,
-		 const struct config_item_type *type);
-int tsm_unregister(const struct tsm_ops *ops);
+int tsm_report_register(const struct tsm_report_ops *ops, void *priv,
+			const struct config_item_type *type);
+int tsm_report_unregister(const struct tsm_report_ops *ops);
 #endif /* __TSM_H */

[RFC PATCH 3/5] coco/tsm: Introduce a shared class device for TSMs

[Dan Williams]

A "tsm" is a platform component that provides an API for securely
provisioning resources for a confidential guest (TVM) to consume. "TSM"
also happens to be the acronym the PCI specification uses to define the
platform agent that carries out device-security operations. That
platform capability is commonly called TEE I/O. It is this arrival of
TEE I/O platforms that requires the "tsm" concept to grow from a
low-level arch-specific detail of TVM instantiation, to a frontend
interface to mediate device setup and interact with general purpose
kernel subsystems outside of arch/ like the PCI core.

Provide a virtual (as in /sys/devices/virtual) class device interface to
front all of the aspects of a TSM and TEE I/O that are
cross-architecture common. This includes mechanisms like enumerating
available platform TEE I/O capabilities and provisioning connections
between the platform TSM and device DSMs.

It is expected to handle hardware TSMs, like AMD SEV-SNP and ARM CCA
where there is a physical TEE coprocessor device running firmware, as
well as software TSMs like Intel TDX and RISC-V COVE, where there is a
privileged software module loaded at runtime.

For now this is just the scaffolding for registering a TSM device and/or
TSM-specific attribute groups.

Cc: Xiaoyao Li <xiaoyao.li@intel.com>
Cc: Isaku Yamahata <isaku.yamahata@intel.com>
Cc: Alexey Kardashevskiy <aik@amd.com>
Cc: Wu Hao <hao.wu@intel.com>
Cc: Yilun Xu <yilun.xu@intel.com>
Cc: Tom Lendacky <thomas.lendacky@amd.com>
Cc: John Allen <john.allen@amd.com>
Signed-off-by: Dan Williams <dan.j.williams@intel.com>
---
 Documentation/ABI/testing/sysfs-class-tsm |   12 +++
 drivers/virt/coco/tsm/Kconfig             |    7 ++
 drivers/virt/coco/tsm/Makefile            |    3 +
 drivers/virt/coco/tsm/class.c             |  100 +++++++++++++++++++++++++++++
 include/linux/tsm.h                       |    8 ++
 5 files changed, 130 insertions(+)
 create mode 100644 Documentation/ABI/testing/sysfs-class-tsm
 create mode 100644 drivers/virt/coco/tsm/class.c

diff --git a/Documentation/ABI/testing/sysfs-class-tsm b/Documentation/ABI/testing/sysfs-class-tsm
new file mode 100644
index 000000000000..304b50b53e65
--- /dev/null
+++ b/Documentation/ABI/testing/sysfs-class-tsm
@@ -0,0 +1,12 @@
+What:		/sys/class/tsm/tsm0/host
+Date:		January 2024
+Contact:	linux-coco@lists.linux.dev
+Description:
+		(RO) For hardware TSMs represented by a device in /sys/devices,
+		@host is a link to that device.
+		Links to hardware TSM sysfs ABIs:
+		- Documentation/ABI/testing/sysfs-driver-ccp
+
+		For software TSMs instantiated by a software module, @host is a
+		directory with attributes for that TSM, and those attributes are
+		documented below.
diff --git a/drivers/virt/coco/tsm/Kconfig b/drivers/virt/coco/tsm/Kconfig
index 69f04461c83e..595d86917462 100644
--- a/drivers/virt/coco/tsm/Kconfig
+++ b/drivers/virt/coco/tsm/Kconfig
@@ -5,3 +5,10 @@
 config TSM_REPORTS
 	select CONFIGFS_FS
 	tristate
+
+config ARCH_HAS_TSM
+	bool
+
+config TSM
+	depends on ARCH_HAS_TSM && SYSFS
+	tristate
diff --git a/drivers/virt/coco/tsm/Makefile b/drivers/virt/coco/tsm/Makefile
index b48504a3ccfd..f7561169faed 100644
--- a/drivers/virt/coco/tsm/Makefile
+++ b/drivers/virt/coco/tsm/Makefile
@@ -4,3 +4,6 @@
 
 obj-$(CONFIG_TSM_REPORTS) += tsm_reports.o
 tsm_reports-y := reports.o
+
+obj-$(CONFIG_TSM) += tsm.o
+tsm-y := class.o
diff --git a/drivers/virt/coco/tsm/class.c b/drivers/virt/coco/tsm/class.c
new file mode 100644
index 000000000000..a569fa6b09eb
--- /dev/null
+++ b/drivers/virt/coco/tsm/class.c
@@ -0,0 +1,100 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/* Copyright(c) 2024 Intel Corporation. All rights reserved. */
+
+#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
+
+#include <linux/tsm.h>
+#include <linux/rwsem.h>
+#include <linux/device.h>
+#include <linux/module.h>
+#include <linux/cleanup.h>
+
+static DECLARE_RWSEM(tsm_core_rwsem);
+struct class *tsm_class;
+struct tsm_subsys {
+	struct device dev;
+	const struct tsm_info *info;
+} *tsm_subsys;
+
+int tsm_register(const struct tsm_info *info)
+{
+	struct device *dev __free(put_device) = NULL;
+	struct tsm_subsys *subsys;
+	int rc;
+
+	guard(rwsem_write)(&tsm_core_rwsem);
+	if (tsm_subsys) {
+		pr_warn("failed to register: \"%s\", \"%s\" already registered\n",
+			info->name, tsm_subsys->info->name);
+		return -EBUSY;
+	}
+
+	subsys = kzalloc(sizeof(*subsys), GFP_KERNEL);
+	if (!subsys)
+		return -ENOMEM;
+
+	subsys->info = info;
+	dev = &subsys->dev;
+	dev->class = tsm_class;
+	dev->groups = info->groups;
+	dev_set_name(dev, "tsm0");
+	rc = device_register(dev);
+	if (rc)
+		return rc;
+
+	if (info->host) {
+		rc = sysfs_create_link(&dev->kobj, &info->host->kobj, "host");
+		if (rc)
+			return rc;
+	}
+
+	/* don't auto-free @dev */
+	dev = NULL;
+	tsm_subsys = subsys;
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(tsm_register);
+
+void tsm_unregister(const struct tsm_info *info)
+{
+	guard(rwsem_write)(&tsm_core_rwsem);
+	if (!tsm_subsys || info != tsm_subsys->info) {
+		pr_warn("failed to unregister: \"%s\", not currently registered\n",
+			info->name);
+		return;
+	}
+
+	if (info->host)
+		sysfs_remove_link(&tsm_subsys->dev.kobj, "host");
+	device_unregister(&tsm_subsys->dev);
+	tsm_subsys = NULL;
+}
+EXPORT_SYMBOL_GPL(tsm_unregister);
+
+static void tsm_release(struct device *dev)
+{
+	struct tsm_subsys *subsys = container_of(dev, typeof(*subsys), dev);
+
+	kfree(subsys);
+}
+
+static int __init tsm_init(void)
+{
+	tsm_class = class_create("tsm");
+	if (IS_ERR(tsm_class))
+		return PTR_ERR(tsm_class);
+
+	tsm_class->dev_release = tsm_release;
+	return 0;
+}
+module_init(tsm_init)
+
+static void __exit tsm_exit(void)
+{
+	class_destroy(tsm_class);
+}
+module_exit(tsm_exit)
+
+MODULE_LICENSE("GPL");
+MODULE_DESCRIPTION("Trusted Security Module core device model");
diff --git a/include/linux/tsm.h b/include/linux/tsm.h
index 28753608fcf5..8cb8a661ba41 100644
--- a/include/linux/tsm.h
+++ b/include/linux/tsm.h
@@ -5,6 +5,12 @@
 #include <linux/sizes.h>
 #include <linux/types.h>
 
+struct tsm_info {
+	const char *name;
+	struct device *host;
+	const struct attribute_group **groups;
+};
+
 #define TSM_REPORT_INBLOB_MAX 64
 #define TSM_REPORT_OUTBLOB_MAX SZ_32K
 
@@ -66,4 +72,6 @@ extern const struct config_item_type tsm_report_extra_type;
 int tsm_report_register(const struct tsm_report_ops *ops, void *priv,
 			const struct config_item_type *type);
 int tsm_report_unregister(const struct tsm_report_ops *ops);
+int tsm_register(const struct tsm_info *info);
+void tsm_unregister(const struct tsm_info *info);
 #endif /* __TSM_H */

[RFC PATCH 4/5] sysfs: Introduce a mechanism to hide static attribute_groups

[Dan Williams]

Add a mechanism for named attribute_groups to hide their directory at
sysfs_update_group() time, or otherwise skip emitting the group
directory when the group is first registered. It piggybacks on
is_visible() in a similar manner as SYSFS_PREALLOC, i.e. special flags
in the upper bits of the returned mode. To use it, specify a symbol
prefix to DEFINE_SYSFS_GROUP_VISIBLE(), and then pass that same prefix
to SYSFS_GROUP_VISIBLE() when assigning the @is_visible() callback:

	DEFINE_SYSFS_GROUP_VISIBLE($prefix)

	struct attribute_group $prefix_group = {
		.name = $name,
		.is_visible = SYSFS_GROUP_VISIBLE($prefix),
	};

SYSFS_GROUP_VISIBLE() expects a definition of $prefix_group_visible()
and $prefix_attr_visible(), where $prefix_group_visible() just returns
true / false and $prefix_attr_visible() behaves as normal.

The motivation for this capability is to centralize PCI device
authentication in the PCI core with a named sysfs group while keeping
that group hidden for devices and platforms that do not meet the
requirements. In a PCI topology, most devices will not support
authentication, a small subset will support just PCI CMA (Component
Measurement and Authentication), a smaller subset will support PCI CMA +
PCIe IDE (Link Integrity and Encryption), and only next generation
server hosts will start to include a platform TSM (TEE Security
Manager).

Without this capability the alternatives are:

* Check if all attributes are invisible and if so, hide the directory.
  Beyond trouble getting this to work [1], this is an ABI change for
  scenarios if userspace happens to depend on group visibility absent any
  attributes. I.e. this new capability avoids regression since it does
  not retroactively apply to existing cases.

* Publish an empty /sys/bus/pci/devices/$pdev/tsm/ directory for all PCI
  devices (i.e. for the case when TSM platform support is present, but
  device support is absent). Unfortunate that this will be a vestigial
  empty directory in the vast majority of cases.

* Reintroduce usage of runtime calls to sysfs_{create,remove}_group()
  in the PCI core. Bjorn has already indicated that he does not want to
  see any growth of pci_sysfs_init() [2].

* Drop the named group and simulate a directory by prefixing all
  TSM-related attributes with "tsm_". Unfortunate to not use the naming
  capability of a sysfs group as intended.

In comparison, there is a small potential for regression if for some
reason an @is_visible() callback had dependencies on how many times it
was called. Additionally, it is no longer an error to update a group
that does not have its directory already present, and it is no longer a
WARN() to remove a group that was never visible.

Link: https://lore.kernel.org/all/2024012321-envious-procedure-4a58@gregkh/ [1]
Link: https://lore.kernel.org/linux-pci/20231019200110.GA1410324@bhelgaas/ [2]
Signed-off-by: Dan Williams <dan.j.williams@intel.com>
---
 fs/sysfs/group.c      |   45 ++++++++++++++++++++++++++++-------
 include/linux/sysfs.h |   63 ++++++++++++++++++++++++++++++++++++++++---------
 2 files changed, 87 insertions(+), 21 deletions(-)

diff --git a/fs/sysfs/group.c b/fs/sysfs/group.c
index 138676463336..ccb275cdabcb 100644
--- a/fs/sysfs/group.c
+++ b/fs/sysfs/group.c
@@ -31,6 +31,17 @@ static void remove_files(struct kernfs_node *parent,
 			kernfs_remove_by_name(parent, (*bin_attr)->attr.name);
 }
 
+static umode_t __first_visible(const struct attribute_group *grp, struct kobject *kobj)
+{
+	if (grp->attrs && grp->is_visible)
+		return grp->is_visible(kobj, grp->attrs[0], 0);
+
+	if (grp->bin_attrs && grp->is_bin_visible)
+		return grp->is_bin_visible(kobj, grp->bin_attrs[0], 0);
+
+	return 0;
+}
+
 static int create_files(struct kernfs_node *parent, struct kobject *kobj,
 			kuid_t uid, kgid_t gid,
 			const struct attribute_group *grp, int update)
@@ -52,6 +63,7 @@ static int create_files(struct kernfs_node *parent, struct kobject *kobj,
 				kernfs_remove_by_name(parent, (*attr)->name);
 			if (grp->is_visible) {
 				mode = grp->is_visible(kobj, *attr, i);
+				mode &= ~SYSFS_GROUP_INVISIBLE;
 				if (!mode)
 					continue;
 			}
@@ -81,6 +93,7 @@ static int create_files(struct kernfs_node *parent, struct kobject *kobj,
 						(*bin_attr)->attr.name);
 			if (grp->is_bin_visible) {
 				mode = grp->is_bin_visible(kobj, *bin_attr, i);
+				mode &= ~SYSFS_GROUP_INVISIBLE;
 				if (!mode)
 					continue;
 			}
@@ -127,16 +140,31 @@ static int internal_create_group(struct kobject *kobj, int update,
 
 	kobject_get_ownership(kobj, &uid, &gid);
 	if (grp->name) {
+		umode_t mode = __first_visible(grp, kobj);
+
+		if (mode & SYSFS_GROUP_INVISIBLE)
+			mode = 0;
+		else
+			mode = S_IRWXU | S_IRUGO | S_IXUGO;
+
 		if (update) {
 			kn = kernfs_find_and_get(kobj->sd, grp->name);
 			if (!kn) {
-				pr_warn("Can't update unknown attr grp name: %s/%s\n",
-					kobj->name, grp->name);
-				return -EINVAL;
+				pr_debug("attr grp %s/%s not created yet\n",
+					 kobj->name, grp->name);
+				/* may have been invisible prior to this update */
+				update = 0;
+			} else if (!mode) {
+				sysfs_remove_group(kobj, grp);
+				kernfs_put(kn);
+				return 0;
 			}
-		} else {
-			kn = kernfs_create_dir_ns(kobj->sd, grp->name,
-						  S_IRWXU | S_IRUGO | S_IXUGO,
+		}
+
+		if (!update) {
+			if (!mode)
+				return 0;
+			kn = kernfs_create_dir_ns(kobj->sd, grp->name, mode,
 						  uid, gid, kobj, NULL);
 			if (IS_ERR(kn)) {
 				if (PTR_ERR(kn) == -EEXIST)
@@ -279,9 +307,8 @@ void sysfs_remove_group(struct kobject *kobj,
 	if (grp->name) {
 		kn = kernfs_find_and_get(parent, grp->name);
 		if (!kn) {
-			WARN(!kn, KERN_WARNING
-			     "sysfs group '%s' not found for kobject '%s'\n",
-			     grp->name, kobject_name(kobj));
+			pr_debug("sysfs group '%s' not found for kobject '%s'\n",
+				 grp->name, kobject_name(kobj));
 			return;
 		}
 	} else {
diff --git a/include/linux/sysfs.h b/include/linux/sysfs.h
index b717a70219f6..a42642b277dd 100644
--- a/include/linux/sysfs.h
+++ b/include/linux/sysfs.h
@@ -61,22 +61,32 @@ do {							\
 /**
  * struct attribute_group - data structure used to declare an attribute group.
  * @name:	Optional: Attribute group name
- *		If specified, the attribute group will be created in
- *		a new subdirectory with this name.
+ *		If specified, the attribute group will be created in a
+ *		new subdirectory with this name. Additionally when a
+ *		group is named, @is_visible and @is_bin_visible may
+ *		return SYSFS_GROUP_INVISIBLE to control visibility of
+ *		the directory itself.
  * @is_visible:	Optional: Function to return permissions associated with an
- *		attribute of the group. Will be called repeatedly for each
- *		non-binary attribute in the group. Only read/write
+ *		attribute of the group. Will be called repeatedly for
+ *		each non-binary attribute in the group. Only read/write
  *		permissions as well as SYSFS_PREALLOC are accepted. Must
- *		return 0 if an attribute is not visible. The returned value
- *		will replace static permissions defined in struct attribute.
+ *		return 0 if an attribute is not visible. The returned
+ *		value will replace static permissions defined in struct
+ *		attribute. Use SYSFS_GROUP_VISIBLE() when assigning this
+ *		callback to specify separate _group_visible() and
+ *		_attr_visible() handlers.
  * @is_bin_visible:
  *		Optional: Function to return permissions associated with a
  *		binary attribute of the group. Will be called repeatedly
  *		for each binary attribute in the group. Only read/write
- *		permissions as well as SYSFS_PREALLOC are accepted. Must
- *		return 0 if a binary attribute is not visible. The returned
- *		value will replace static permissions defined in
- *		struct bin_attribute.
+ *		permissions as well as SYSFS_PREALLOC (and the
+ *		visibility flags for named groups) are accepted. Must
+ *		return 0 if a binary attribute is not visible. The
+ *		returned value will replace static permissions defined
+ *		in struct bin_attribute. If @is_visible is not set, Use
+ *		SYSFS_GROUP_VISIBLE() when assigning this callback to
+ *		specify separate _group_visible() and _attr_visible()
+ *		handlers.
  * @attrs:	Pointer to NULL terminated list of attributes.
  * @bin_attrs:	Pointer to NULL terminated list of binary attributes.
  *		Either attrs or bin_attrs or both must be provided.
@@ -91,13 +101,42 @@ struct attribute_group {
 	struct bin_attribute	**bin_attrs;
 };
 
+#define SYSFS_PREALLOC		010000
+#define SYSFS_GROUP_INVISIBLE	020000
+
+/*
+ * The first call to is_visible() in the create / update path may
+ * indicate visibility for the entire group
+ */
+#define DEFINE_SYSFS_GROUP_VISIBLE(name)                             \
+	static inline umode_t sysfs_group_visible_##name(            \
+		struct kobject *kobj, struct attribute *attr, int n) \
+	{                                                            \
+		if (n == 0 && !name##_group_visible(kobj))           \
+			return SYSFS_GROUP_INVISIBLE;                \
+		return name##_attr_visible(kobj, attr, n);           \
+	}
+
+/*
+ * Same as DEFINE_SYSFS_GROUP_VISIBLE, but for groups with only binary
+ * attributes
+ */
+#define DEFINE_SYSFS_BIN_GROUP_VISIBLE(name)                             \
+	static inline umode_t sysfs_group_visible_##name(                \
+		struct kobject *kobj, struct bin_attribute *attr, int n) \
+	{                                                                \
+		if (n == 0 && !name##_group_visible(kobj))               \
+			return SYSFS_GROUP_INVISIBLE;                    \
+		return name##_attr_visible(kobj, attr, n);               \
+	}
+
+#define SYSFS_GROUP_VISIBLE(fn) sysfs_group_visible_##fn
+
 /*
  * Use these macros to make defining attributes easier.
  * See include/linux/device.h for examples..
  */
 
-#define SYSFS_PREALLOC 010000
-
 #define __ATTR(_name, _mode, _show, _store) {				\
 	.attr = {.name = __stringify(_name),				\
 		 .mode = VERIFY_OCTAL_PERMISSIONS(_mode) },		\

[RFC PATCH 5/5] PCI/TSM: Authenticate devices via platform TSM

[Dan Williams]

The PCIe 6.1 specification, section 11, introduces the Trusted
Execution Environment (TEE) Device Interface Security Protocol (TDISP).
This interface definition builds upon CMA, component measurement and
authentication, and IDE, link integrity and data encryption. It adds
support for establishing virtual functions within a device that can be
assigned to a confidential VM such that the assigned device is enabled
to access guest private memory protected by technologies like Intel TDX,
AMD SEV-SNP, RISCV COVE, or ARM CCA.

The "TSM" (TEE Security Manager) is a concept in the TDISP specification
of an agent that mediates between a device security manager (DSM) and
system software in both a VMM and a VM. From a Linux perspective the TSM
abstracts many of the details of TDISP, IDE, and CMA. Some of those
details leak through at times, but for the most part TDISP is an
internal implementation detail of the TSM.

Similar to the PCI core extensions to support CONFIG_PCI_CMA,
CONFIG_PCI_TSM builds upon that to reuse the "authenticated" sysfs
attribute, and add more properties + controls in a tsm/ subdirectory of
the PCI device sysfs interface. Unlike CMA that can depend on a local to
the PCI core implementation, PCI_TSM needs to be prepared for late
loading of the platform TSM driver. Consider that the TSM driver may
itself be a PCI driver. Userspace can depend on the common TSM device
uevent to know when the PCI core has TSM services enabled. The PCI
device tsm/ subdirectory is supplemented by the TSM device pci/
directory for platform global TSM properties + controls.

All vendor TSM implementations share the property of asking the VMM to
perform DOE mailbox operations on behalf of the TSM. That common
capability is centralized in PCI core code that invokes an ->exec()
operation callback potentially multiple times to service a given request
(struct pci_tsm_req). Future operations / verbs will be handled
similarly with the "request + exec" model. For now, only "connect" and
"disconnect" are implemented which at a minimum is expected to establish
IDE for the link.

In addition to requests the low-level TSM implementation is notified of
device arrival and departure events so that it can filter devices that
the TSM is not prepared to support, or otherwise setup and teardown
per-device context.

Cc: Wu Hao <hao.wu@intel.com>
Cc: Yilun Xu <yilun.xu@intel.com>
Cc: Lukas Wunner <lukas@wunner.de>
Cc: Samuel Ortiz <sameo@rivosinc.com>
Cc: Alexey Kardashevskiy <aik@amd.com>
Cc: Bjorn Helgaas <bhelgaas@google.com>
Signed-off-by: Dan Williams <dan.j.williams@intel.com>
---
 Documentation/ABI/testing/sysfs-bus-pci   |   43 +++-
 Documentation/ABI/testing/sysfs-class-tsm |   23 ++
 drivers/pci/Kconfig                       |   15 +
 drivers/pci/Makefile                      |    2 
 drivers/pci/cma.c                         |    5 
 drivers/pci/pci-sysfs.c                   |    3 
 drivers/pci/pci.h                         |   14 +
 drivers/pci/probe.c                       |    1 
 drivers/pci/remove.c                      |    1 
 drivers/pci/tsm.c                         |  346 +++++++++++++++++++++++++++++
 drivers/virt/coco/tsm/Makefile            |    1 
 drivers/virt/coco/tsm/class.c             |   22 +-
 drivers/virt/coco/tsm/pci.c               |   83 +++++++
 drivers/virt/coco/tsm/tsm.h               |   28 ++
 include/linux/pci.h                       |    3 
 include/linux/tsm.h                       |   77 ++++++
 include/uapi/linux/pci_regs.h             |    3 
 17 files changed, 662 insertions(+), 8 deletions(-)
 create mode 100644 drivers/pci/tsm.c
 create mode 100644 drivers/virt/coco/tsm/pci.c
 create mode 100644 drivers/virt/coco/tsm/tsm.h

diff --git a/Documentation/ABI/testing/sysfs-bus-pci b/Documentation/ABI/testing/sysfs-bus-pci
index 35b0e11fd0e6..0eef2128cf09 100644
--- a/Documentation/ABI/testing/sysfs-bus-pci
+++ b/Documentation/ABI/testing/sysfs-bus-pci
@@ -508,11 +508,16 @@ Description:
 		This file contains "native" if the device authenticated
 		successfully with CMA-SPDM (PCIe r6.1 sec 6.31). It contains
 		"none" if the device failed authentication (and may thus be
-		malicious).
+		malicious). It transitions from "native" to "tsm" after
+		successful connection to a tsm, see the "connect" attribute
+		below.
 
 		Writing "native" to this file causes reauthentication with
 		kernel-selected keys and the kernel's certificate chain.  That
-		may be opportune after updating the .cma keyring.
+		may be opportune after updating the .cma keyring. Note
+		that once connected to a tsm this returns -EBUSY to attempts to
+		write "native", i.e. first disconnect from the tsm to retrigger
+		native authentication.
 
 		The file is not visible if authentication is unsupported
 		by the device.
@@ -529,3 +534,37 @@ Description:
 		The reason why authentication support could not be determined
 		is apparent from "dmesg".  To probe for authentication support
 		again, exercise the "remove" and "rescan" attributes.
+
+What:		/sys/bus/pci/devices/.../tsm/
+Date:		January 2024
+Contact:	linux-coco@lists.linux.dev
+Description:
+		This directory only appears if a device supports CMA and IDE,
+		and only after a TSM driver has loaded and accepted / setup this
+		PCI device. Similar to the 'authenticated' attribute, trigger
+		"remove" and "rescan" to retry the initialization. See
+		Documentation/ABI/testing/sysfs-class-tsm for enumerating the
+		platform's TSM capabilities.
+
+What:		/sys/bus/pci/devices/.../tsm/connect
+Date:		January 2024
+Contact:	linux-coco@lists.linux.dev
+Description:
+		(RW) Writing "1" to this file triggers the TSM to establish a
+		secure connection with the device. This typically includes an
+		SPDM (DMTF Security Protocols and Data Models) session over PCIe
+		DOE (Data Object Exchange) and PCIe IDE (Integrity and Data
+		Encryption) establishment. For TSMs and devices that support
+		both modes of IDE ("link" and "selective") the "connect_mode"
+		attribute selects the mode.
+
+What:		/sys/bus/pci/devices/.../tsm/connect_mode
+Date:		January 2024
+Contact:	linux-coco@lists.linux.dev
+Description:
+		(RO) Returns the available connection modes optionally with
+		brackets around the currently active mode if the device is
+		connected. For example it may show "link selective" for a
+		disconnected device, "link [selective]" for a selective
+		connected device, and it may hide a mode that is not supported
+		by the device or TSM.
diff --git a/Documentation/ABI/testing/sysfs-class-tsm b/Documentation/ABI/testing/sysfs-class-tsm
index 304b50b53e65..77957882738a 100644
--- a/Documentation/ABI/testing/sysfs-class-tsm
+++ b/Documentation/ABI/testing/sysfs-class-tsm
@@ -10,3 +10,26 @@ Description:
 		For software TSMs instantiated by a software module, @host is a
 		directory with attributes for that TSM, and those attributes are
 		documented below.
+
+
+What:		/sys/class/tsm/tsm0/pci/link_capable
+Date:		January, 2024
+Contact:	linux-coco@lists.linux.dev
+Description:
+		(RO) When present this returns "1\n" to indicate that the TSM
+		supports establishing Link IDE with a given root-port attached
+		device. See "tsm/connect_mode" in
+		Documentation/ABI/testing/sysfs-bus-pci
+
+
+What:		/sys/class/tsm/tsm0/pci/selective_streams
+Date:		January, 2024
+Contact:	linux-coco@lists.linux.dev
+Description:
+		(RO) When present this returns the number of currently available
+		selective IDE streams available to the TSM. When a stream id is
+		allocated this number is decremented and a link to the PCI
+		device(s) consuming the stream(s) appears alonside this
+		attribute in the /sys/class/tsm/tsm0/pci/ directory. See
+		"tsm/connect" and "tsm/connect_mode" in
+		Documentation/ABI/testing/sysfs-bus-pci.
diff --git a/drivers/pci/Kconfig b/drivers/pci/Kconfig
index a5c3cadddd6f..11d788038d19 100644
--- a/drivers/pci/Kconfig
+++ b/drivers/pci/Kconfig
@@ -129,6 +129,21 @@ config PCI_CMA
 	  A PCI DOE mailbox is used as transport for DMTF SPDM based
 	  authentication, measurement and secure channel establishment.
 
+config PCI_TSM
+	bool "TEE Security Manager for Device Security"
+	depends on PCI_CMA
+	depends on TSM
+	help
+	  The TEE (Trusted Execution Environment) Device Interface
+	  Security Protocol (TDISP) defines a "TSM" as a platform agent
+	  that manages device authentication, link encryption, link
+	  integrity protection, and assignment of PCI device functions
+	  (virtual or physical) to confidential computing VMs that can
+	  access (DMA) guest private memory.
+
+	  Say Y to enable the PCI subsystem to enable the IDE and
+	  TDISP capabilities of devices via TSM semantics.
+
 config PCI_DOE
 	bool
 
diff --git a/drivers/pci/Makefile b/drivers/pci/Makefile
index cc8b5d1d15b9..c4117d67ea83 100644
--- a/drivers/pci/Makefile
+++ b/drivers/pci/Makefile
@@ -38,6 +38,8 @@ obj-$(CONFIG_PCI_CMA)		+= cma.o cma.asn1.o
 $(obj)/cma.o:			$(obj)/cma.asn1.h
 $(obj)/cma.asn1.o:		$(obj)/cma.asn1.c $(obj)/cma.asn1.h
 
+obj-$(CONFIG_PCI_TSM)		+= tsm.o
+
 # Endpoint library must be initialized before its users
 obj-$(CONFIG_PCI_ENDPOINT)	+= endpoint/
 
diff --git a/drivers/pci/cma.c b/drivers/pci/cma.c
index be7d2bb21b4c..5a69e9919589 100644
--- a/drivers/pci/cma.c
+++ b/drivers/pci/cma.c
@@ -39,6 +39,9 @@ static ssize_t authenticated_store(struct device *dev,
 	if (!sysfs_streq(buf, "native"))
 		return -EINVAL;
 
+	if (pci_tsm_authenticated(pdev))
+		return -EBUSY;
+
 	rc = pci_cma_reauthenticate(pdev);
 	if (rc)
 		return rc;
@@ -55,6 +58,8 @@ static ssize_t authenticated_show(struct device *dev,
 	    (pdev->cma_init_failed || pdev->doe_init_failed))
 		return -ENOTTY;
 
+	if (pci_tsm_authenticated(pdev))
+		return sysfs_emit(buf, "tsm\n");
 	if (spdm_authenticated(pdev->spdm_state))
 		return sysfs_emit(buf, "native\n");
 	return sysfs_emit(buf, "none\n");
diff --git a/drivers/pci/pci-sysfs.c b/drivers/pci/pci-sysfs.c
index 368c4f71cc55..4327f8c2e6b5 100644
--- a/drivers/pci/pci-sysfs.c
+++ b/drivers/pci/pci-sysfs.c
@@ -1654,6 +1654,9 @@ const struct attribute_group *pci_dev_attr_groups[] = {
 #endif
 #ifdef CONFIG_PCI_CMA
 	&pci_cma_attr_group,
+#endif
+#ifdef CONFIG_PCI_TSM
+	&pci_tsm_attr_group,
 #endif
 	NULL,
 };
diff --git a/drivers/pci/pci.h b/drivers/pci/pci.h
index 2b7d8d0b2e21..daa20866bc90 100644
--- a/drivers/pci/pci.h
+++ b/drivers/pci/pci.h
@@ -350,6 +350,20 @@ static inline int pci_cma_reauthenticate(struct pci_dev *pdev)
 }
 #endif
 
+#ifdef CONFIG_PCI_TSM
+void pci_tsm_init(struct pci_dev *pdev);
+void pci_tsm_destroy(struct pci_dev *pdev);
+extern const struct attribute_group pci_tsm_attr_group;
+bool pci_tsm_authenticated(struct pci_dev *pdev);
+#else
+static inline void pci_tsm_init(struct pci_dev *pdev) { }
+static inline void pci_tsm_destroy(struct pci_dev *pdev) { }
+static inline bool pci_tsm_authenticated(struct pci_dev *pdev)
+{
+	return false;
+}
+#endif
+
 /**
  * pci_dev_set_io_state - Set the new error state if possible.
  *
diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
index 6b09c962c0b8..f60d6c3c8c48 100644
--- a/drivers/pci/probe.c
+++ b/drivers/pci/probe.c
@@ -2542,6 +2542,7 @@ static void pci_init_capabilities(struct pci_dev *dev)
 	pci_rcec_init(dev);		/* Root Complex Event Collector */
 	pci_doe_init(dev);		/* Data Object Exchange */
 	pci_cma_init(dev);		/* Component Measurement & Auth */
+	pci_tsm_init(dev);		/* TEE Security Manager connection */
 
 	pcie_report_downtraining(dev);
 	pci_init_reset_methods(dev);
diff --git a/drivers/pci/remove.c b/drivers/pci/remove.c
index f009ac578997..228fa6ccf911 100644
--- a/drivers/pci/remove.c
+++ b/drivers/pci/remove.c
@@ -39,6 +39,7 @@ static void pci_destroy_dev(struct pci_dev *dev)
 	list_del(&dev->bus_list);
 	up_write(&pci_bus_sem);
 
+	pci_tsm_destroy(dev);
 	pci_cma_destroy(dev);
 	pci_doe_destroy(dev);
 	pcie_aspm_exit_link_state(dev);
diff --git a/drivers/pci/tsm.c b/drivers/pci/tsm.c
new file mode 100644
index 000000000000..f74de0ee49a0
--- /dev/null
+++ b/drivers/pci/tsm.c
@@ -0,0 +1,346 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * TEE Security Manager for the TEE Device Interface Security Protocol
+ * (TDISP, PCIe r6.1 sec 11)
+ *
+ * Copyright(c) 2024 Intel Corporation. All rights reserved.
+ */
+
+#define dev_fmt(fmt) "TSM: " fmt
+
+#include <linux/pci.h>
+#include <linux/tsm.h>
+#include <linux/sysfs.h>
+#include <linux/xarray.h>
+#include "pci.h"
+
+/* collect tsm capable devices to rendezvous with the tsm driver */
+static DEFINE_XARRAY(pci_tsm_devs);
+
+/*
+ * Provide a read/write lock against the init / exit of pdev tsm
+ * capabilities and arrival/departure of a tsm instance
+ */
+static DECLARE_RWSEM(pci_tsm_rwsem);
+static const struct tsm_pci_ops *tsm_ops;
+
+void generic_pci_tsm_req_free(struct pci_tsm_req *req)
+{
+	kfree(req);
+}
+EXPORT_SYMBOL_GPL(generic_pci_tsm_req_free);
+
+struct pci_tsm_req *generic_pci_tsm_req_alloc(struct pci_dev *pdev, enum pci_tsm_op op)
+{
+	struct pci_tsm_req *req = kzalloc(sizeof(*req), GFP_KERNEL);
+
+	if (!req)
+		return NULL;
+	req->op = op;
+	return req;
+}
+EXPORT_SYMBOL_GPL(generic_pci_tsm_req_alloc);
+
+DEFINE_FREE(req_free, struct pci_tsm_req *, if (_T) tsm_ops->req_free(_T))
+
+static int pci_tsm_disconnect(struct pci_dev *pdev)
+{
+	struct pci_tsm_req *req __free(req_free) = NULL;
+
+	/* opportunistic state checks to skip allocating a request */
+	if (pdev->tsm->state < PCI_TSM_CONNECT)
+		return 0;
+
+	req = tsm_ops->req_alloc(pdev, PCI_TSM_OP_DISCONNECT);
+	if (!req)
+		return -ENOMEM;
+
+	scoped_cond_guard(mutex_intr, return -EINTR, tsm_ops->lock) {
+		enum pci_tsm_op_status status;
+
+		/* revalidate state */
+		if (pdev->tsm->state < PCI_TSM_CONNECT)
+			return 0;
+		if (pdev->tsm->state < PCI_TSM_INIT)
+			return -ENXIO;
+
+		do {
+			status = tsm_ops->exec(pdev, req);
+			req->seq++;
+			/* TODO: marshal SPDM request */
+		} while (status == PCI_TSM_SPDM_REQ);
+
+		if (status == PCI_TSM_FAIL)
+			return -EIO;
+		pdev->tsm->state = PCI_TSM_INIT;
+	}
+	return 0;
+}
+
+static int pci_tsm_connect(struct pci_dev *pdev)
+{
+	struct pci_tsm_req *req __free(req_free) = NULL;
+
+	/* opportunistic state checks to skip allocating a request */
+	if (pdev->tsm->state >= PCI_TSM_CONNECT)
+		return 0;
+
+	req = tsm_ops->req_alloc(pdev, PCI_TSM_OP_CONNECT);
+	if (!req)
+		return -ENOMEM;
+
+	scoped_cond_guard(mutex_intr, return -EINTR, tsm_ops->lock) {
+		enum pci_tsm_op_status status;
+
+		/* revalidate state */
+		if (pdev->tsm->state >= PCI_TSM_CONNECT)
+			return 0;
+		if (pdev->tsm->state < PCI_TSM_INIT)
+			return -ENXIO;
+
+		do {
+			status = tsm_ops->exec(pdev, req);
+			req->seq++;
+		} while (status == PCI_TSM_SPDM_REQ);
+
+		if (status == PCI_TSM_FAIL)
+			return -EIO;
+		pdev->tsm->state = PCI_TSM_CONNECT;
+	}
+	return 0;
+}
+
+static ssize_t connect_store(struct device *dev, struct device_attribute *attr,
+			     const char *buf, size_t len)
+{
+	bool connect;
+	int rc = kstrtobool(buf, &connect);
+	struct pci_dev *pdev = to_pci_dev(dev);
+
+	if (rc)
+		return rc;
+
+	if (connect) {
+		if (!spdm_authenticated(pdev->spdm_state)) {
+			pci_dbg(pdev, "SPDM authentication pre-requisite not met.\n");
+			return -ENXIO;
+		}
+		rc = pci_tsm_connect(pdev);
+		if (rc)
+			return rc;
+		return len;
+	}
+
+	rc = pci_tsm_disconnect(pdev);
+	if (rc)
+		return rc;
+	return len;
+}
+
+static ssize_t connect_show(struct device *dev, struct device_attribute *attr,
+			    char *buf)
+{
+	struct pci_dev *pdev = to_pci_dev(dev);
+
+	return sysfs_emit(buf, "%d\n", pdev->tsm->state >= PCI_TSM_CONNECT);
+}
+static DEVICE_ATTR_RW(connect);
+
+static const char *const pci_tsm_modes[] = {
+	[PCI_TSM_MODE_LINK] = "link",
+	[PCI_TSM_MODE_SELECTIVE] = "selective",
+};
+
+static ssize_t connect_mode_store(struct device *dev,
+				  struct device_attribute *attr,
+				  const char *buf, size_t len)
+{
+	struct pci_dev *pdev = to_pci_dev(dev);
+	int i;
+
+	guard(mutex)(tsm_ops->lock);
+	if (pdev->tsm->state >= PCI_TSM_CONNECT)
+		return -EBUSY;
+	for (i = 0; i < ARRAY_SIZE(pci_tsm_modes); i++)
+		if (sysfs_streq(buf, pci_tsm_modes[i]))
+			break;
+	if (i == PCI_TSM_MODE_LINK) {
+		if (pdev->tsm->link_capable)
+			pdev->tsm->mode = PCI_TSM_MODE_LINK;
+		return -EOPNOTSUPP;
+	} else if (i == PCI_TSM_MODE_SELECTIVE) {
+		if (pdev->tsm->selective_capable)
+			pdev->tsm->mode = PCI_TSM_MODE_SELECTIVE;
+		return -EOPNOTSUPP;
+	} else
+		return -EINVAL;
+	return len;
+}
+
+static ssize_t connect_mode_show(struct device *dev,
+				 struct device_attribute *attr, char *buf)
+{
+	struct pci_dev *pdev = to_pci_dev(dev);
+	ssize_t count = 0;
+	int i;
+
+	for (i = 0; i < ARRAY_SIZE(pci_tsm_modes); i++) {
+		if (i == PCI_TSM_MODE_LINK) {
+			if (!pdev->tsm->link_capable)
+				continue;
+		} else if (i == PCI_TSM_MODE_SELECTIVE) {
+			if (!pdev->tsm->selective_capable)
+				continue;
+		}
+
+		if (i == pdev->tsm->mode)
+			count += sysfs_emit_at(buf, count, "[%s] ",
+					       pci_tsm_modes[i]);
+		else
+			count += sysfs_emit_at(buf, count, "%s ",
+					       pci_tsm_modes[i]);
+	}
+
+	if (count)
+		buf[count - 1] = '\n';
+
+	return count;
+}
+static DEVICE_ATTR_RW(connect_mode);
+
+static umode_t pci_tsm_attr_visible(struct kobject *kobj, struct attribute *a, int n)
+{
+	struct device *dev = kobj_to_dev(kobj);
+	struct pci_dev *pdev = to_pci_dev(dev);
+
+	if (a == &dev_attr_connect_mode.attr) {
+		if (pdev->tsm->link_capable || pdev->tsm->selective_capable)
+			return a->mode;
+		return 0;
+	}
+
+	return a->mode;
+}
+
+static bool pci_tsm_group_visible(struct kobject *kobj)
+{
+	struct device *dev = kobj_to_dev(kobj);
+	struct pci_dev *pdev = to_pci_dev(dev);
+
+	if (pdev->tsm && pdev->tsm->state > PCI_TSM_IDLE)
+		return true;
+	return false;
+}
+DEFINE_SYSFS_GROUP_VISIBLE(pci_tsm);
+
+static struct attribute *pci_tsm_attrs[] = {
+	&dev_attr_connect.attr,
+	&dev_attr_connect_mode.attr,
+	NULL,
+};
+
+const struct attribute_group pci_tsm_attr_group = {
+	.name = "tsm",
+	.attrs = pci_tsm_attrs,
+	.is_visible = SYSFS_GROUP_VISIBLE(pci_tsm),
+};
+
+static int pci_tsm_add(struct pci_dev *pdev)
+{
+	lockdep_assert_held(&pci_tsm_rwsem);
+	if (!tsm_ops)
+		return 0;
+	scoped_guard(mutex, tsm_ops->lock) {
+		if (pdev->tsm->state < PCI_TSM_INIT) {
+			int rc = tsm_ops->add(pdev);
+
+			if (rc)
+				return rc;
+		}
+		pdev->tsm->state = PCI_TSM_INIT;
+	}
+	return sysfs_update_group(&pdev->dev.kobj, &pci_tsm_attr_group);
+}
+
+static void pci_tsm_del(struct pci_dev *pdev)
+{
+	lockdep_assert_held(&pci_tsm_rwsem);
+	/* shutdown sysfs operations before tsm delete */
+	pdev->tsm->state = PCI_TSM_IDLE;
+	sysfs_update_group(&pdev->dev.kobj, &pci_tsm_attr_group);
+	guard(mutex)(tsm_ops->lock);
+	tsm_ops->del(pdev);
+}
+
+int pci_tsm_register(const struct tsm_pci_ops *ops)
+{
+	struct pci_dev *pdev;
+	unsigned long index;
+
+	guard(rwsem_write)(&pci_tsm_rwsem);
+	if (tsm_ops)
+		return -EBUSY;
+	tsm_ops = ops;
+	xa_for_each(&pci_tsm_devs, index, pdev)
+		pci_tsm_add(pdev);
+	return 0;
+}
+EXPORT_SYMBOL_GPL(pci_tsm_register);
+
+void pci_tsm_unregister(const struct tsm_pci_ops *ops)
+{
+	struct pci_dev *pdev;
+	unsigned long index;
+
+	guard(rwsem_write)(&pci_tsm_rwsem);
+	if (ops != tsm_ops)
+		return;
+	xa_for_each(&pci_tsm_devs, index, pdev)
+		pci_tsm_del(pdev);
+	tsm_ops = NULL;
+}
+EXPORT_SYMBOL_GPL(pci_tsm_unregister);
+
+void pci_tsm_init(struct pci_dev *pdev)
+{
+	u16 ide_cap;
+	int rc;
+
+	if (!pdev->cma_capable)
+		return;
+
+	ide_cap = pci_find_ext_capability(pdev, PCI_EXT_CAP_ID_IDE);
+	if (!ide_cap)
+		return;
+
+	struct pci_tsm *tsm __free(kfree) = kzalloc(sizeof(*tsm), GFP_KERNEL);
+	if (!tsm)
+		return;
+
+	tsm->ide_cap = ide_cap;
+
+	rc = xa_insert(&pci_tsm_devs, (unsigned long)pdev, pdev, GFP_KERNEL);
+	if (rc) {
+		pci_dbg(pdev, "failed to register tsm capable device\n");
+		return;
+	}
+
+	guard(rwsem_write)(&pci_tsm_rwsem);
+	pdev->tsm = no_free_ptr(tsm);
+	pci_tsm_add(pdev);
+}
+
+void pci_tsm_destroy(struct pci_dev *pdev)
+{
+	guard(rwsem_write)(&pci_tsm_rwsem);
+	pci_tsm_del(pdev);
+	xa_erase(&pci_tsm_devs, (unsigned long)pdev);
+	kfree(pdev->tsm);
+	pdev->tsm = NULL;
+}
+
+bool pci_tsm_authenticated(struct pci_dev *pdev)
+{
+	guard(rwsem_read)(&pci_tsm_rwsem);
+	return pdev->tsm && pdev->tsm->state >= PCI_TSM_CONNECT;
+}
diff --git a/drivers/virt/coco/tsm/Makefile b/drivers/virt/coco/tsm/Makefile
index f7561169faed..a4f0d07d7d97 100644
--- a/drivers/virt/coco/tsm/Makefile
+++ b/drivers/virt/coco/tsm/Makefile
@@ -7,3 +7,4 @@ tsm_reports-y := reports.o
 
 obj-$(CONFIG_TSM) += tsm.o
 tsm-y := class.o
+tsm-$(CONFIG_PCI_TSM) += pci.o
diff --git a/drivers/virt/coco/tsm/class.c b/drivers/virt/coco/tsm/class.c
index a569fa6b09eb..a459e51c0892 100644
--- a/drivers/virt/coco/tsm/class.c
+++ b/drivers/virt/coco/tsm/class.c
@@ -8,13 +8,11 @@
 #include <linux/device.h>
 #include <linux/module.h>
 #include <linux/cleanup.h>
+#include "tsm.h"
 
 static DECLARE_RWSEM(tsm_core_rwsem);
-struct class *tsm_class;
-struct tsm_subsys {
-	struct device dev;
-	const struct tsm_info *info;
-} *tsm_subsys;
+static struct class *tsm_class;
+static struct tsm_subsys *tsm_subsys;
 
 int tsm_register(const struct tsm_info *info)
 {
@@ -52,6 +50,10 @@ int tsm_register(const struct tsm_info *info)
 	dev = NULL;
 	tsm_subsys = subsys;
 
+	rc = tsm_pci_init(info);
+	if (rc)
+		pr_err("PCI initialization failure: %d\n", rc);
+
 	return 0;
 }
 EXPORT_SYMBOL_GPL(tsm_register);
@@ -65,6 +67,8 @@ void tsm_unregister(const struct tsm_info *info)
 		return;
 	}
 
+	tsm_pci_destroy(info);
+
 	if (info->host)
 		sysfs_remove_link(&tsm_subsys->dev.kobj, "host");
 	device_unregister(&tsm_subsys->dev);
@@ -79,6 +83,13 @@ static void tsm_release(struct device *dev)
 	kfree(subsys);
 }
 
+static const struct attribute_group *tsm_attr_groups[] = {
+#ifdef CONFIG_PCI_TSM
+	&tsm_pci_attr_group,
+#endif
+	NULL,
+};
+
 static int __init tsm_init(void)
 {
 	tsm_class = class_create("tsm");
@@ -86,6 +97,7 @@ static int __init tsm_init(void)
 		return PTR_ERR(tsm_class);
 
 	tsm_class->dev_release = tsm_release;
+	tsm_class->dev_groups = tsm_attr_groups;
 	return 0;
 }
 module_init(tsm_init)
diff --git a/drivers/virt/coco/tsm/pci.c b/drivers/virt/coco/tsm/pci.c
new file mode 100644
index 000000000000..b3684ad7114f
--- /dev/null
+++ b/drivers/virt/coco/tsm/pci.c
@@ -0,0 +1,83 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/* Copyright(c) 2024 Intel Corporation. All rights reserved. */
+
+#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
+
+#include <linux/tsm.h>
+#include <linux/device.h>
+#include "tsm.h"
+
+static ssize_t link_capable_show(struct device *dev,
+				 struct device_attribute *attr, char *buf)
+{
+	struct tsm_subsys *subsys = container_of(dev, typeof(*subsys), dev);
+
+	return sysfs_emit(buf, "%u\n", subsys->info->link_stream_capable);
+}
+static DEVICE_ATTR_RO(link_capable);
+
+static ssize_t selective_streams_show(struct device *dev,
+				      struct device_attribute *attr, char *buf)
+{
+	struct tsm_subsys *subsys = container_of(dev, typeof(*subsys), dev);
+
+	return sysfs_emit(buf, "%u\n", subsys->info->nr_selective_streams);
+}
+static DEVICE_ATTR_RO(selective_streams);
+
+static umode_t tsm_pci_attr_visible(struct kobject *kobj, struct attribute *a, int n)
+{
+	struct device *dev = kobj_to_dev(kobj);
+	struct tsm_subsys *subsys = container_of(dev, typeof(*subsys), dev);
+	const struct tsm_info *info = subsys->info;
+
+	if (a == &dev_attr_link_capable.attr) {
+		if (info->link_stream_capable)
+			return a->mode;
+		return 0;
+	}
+
+	if (a == &dev_attr_selective_streams.attr) {
+		if (info->nr_selective_streams)
+			return a->mode;
+		return 0;
+	}
+
+	return a->mode;
+}
+
+static bool tsm_pci_group_visible(struct kobject *kobj)
+{
+	struct device *dev = kobj_to_dev(kobj);
+	struct tsm_subsys *subsys = container_of(dev, typeof(*subsys), dev);
+
+	if (subsys->info->pci_ops)
+		return true;
+	return false;
+}
+DEFINE_SYSFS_GROUP_VISIBLE(tsm_pci);
+
+static struct attribute *tsm_pci_attrs[] = {
+	&dev_attr_link_capable.attr,
+	&dev_attr_selective_streams.attr,
+	NULL,
+};
+
+const struct attribute_group tsm_pci_attr_group = {
+	.name = "pci",
+	.attrs = tsm_pci_attrs,
+	.is_visible = SYSFS_GROUP_VISIBLE(tsm_pci),
+};
+
+int tsm_pci_init(const struct tsm_info *info)
+{
+	if (!info->pci_ops)
+		return 0;
+
+	return pci_tsm_register(info->pci_ops);
+}
+
+void tsm_pci_destroy(const struct tsm_info *info)
+{
+	pci_tsm_unregister(info->pci_ops);
+}
diff --git a/drivers/virt/coco/tsm/tsm.h b/drivers/virt/coco/tsm/tsm.h
new file mode 100644
index 000000000000..407c388a109b
--- /dev/null
+++ b/drivers/virt/coco/tsm/tsm.h
@@ -0,0 +1,28 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#ifndef __TSM_CORE_H
+#define __TSM_CORE_H
+
+#include <linux/device.h>
+
+struct tsm_info;
+struct tsm_subsys {
+	struct device dev;
+	const struct tsm_info *info;
+};
+
+#ifdef CONFIG_PCI_TSM
+int tsm_pci_init(const struct tsm_info *info);
+void tsm_pci_destroy(const struct tsm_info *info);
+extern const struct attribute_group tsm_pci_attr_group;
+#else
+static inline int tsm_pci_init(const struct tsm_info *info)
+{
+	return 0;
+}
+static inline void tsm_pci_destroy(const struct tsm_info *info)
+{
+}
+#endif
+
+#endif /* TSM_CORE_H */
+
diff --git a/include/linux/pci.h b/include/linux/pci.h
index 4a04ce7685e7..132962b21e04 100644
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -522,6 +522,9 @@ struct pci_dev {
 	struct spdm_state *spdm_state;	/* Security Protocol and Data Model */
 	unsigned int	cma_capable:1;	/* Authentication supported */
 	unsigned int	cma_init_failed:1;
+#endif
+#ifdef CONFIG_PCI_TSM
+	struct pci_tsm *tsm;		/* TSM operation state */
 #endif
 	u16		acs_cap;	/* ACS Capability offset */
 	phys_addr_t	rom;		/* Physical address if not from BAR */
diff --git a/include/linux/tsm.h b/include/linux/tsm.h
index 8cb8a661ba41..f5dbdfa65d8d 100644
--- a/include/linux/tsm.h
+++ b/include/linux/tsm.h
@@ -4,11 +4,15 @@
 
 #include <linux/sizes.h>
 #include <linux/types.h>
+#include <linux/mutex.h>
 
 struct tsm_info {
 	const char *name;
 	struct device *host;
 	const struct attribute_group **groups;
+	const struct tsm_pci_ops *pci_ops;
+	unsigned int nr_selective_streams;
+	unsigned int link_stream_capable:1;
 };
 
 #define TSM_REPORT_INBLOB_MAX 64
@@ -74,4 +78,77 @@ int tsm_report_register(const struct tsm_report_ops *ops, void *priv,
 int tsm_report_unregister(const struct tsm_report_ops *ops);
 int tsm_register(const struct tsm_info *info);
 void tsm_unregister(const struct tsm_info *info);
+
+enum pci_tsm_op_status {
+	PCI_TSM_FAIL = -1,
+	PCI_TSM_OK,
+	PCI_TSM_SPDM_REQ,
+};
+
+enum pci_tsm_op {
+	PCI_TSM_OP_CONNECT,
+	PCI_TSM_OP_DISCONNECT,
+};
+
+struct pci_tsm_req {
+	enum pci_tsm_op op;
+	unsigned int seq;
+};
+
+struct pci_dev;
+/**
+ * struct tsm_pci_ops - Low-level TSM-exported interface to the PCI core
+ * @add: accept device for tsm operation, locked
+ * @del: teardown tsm context for @pdev, locked
+ * @req_alloc: setup context for given operation, unlocked
+ * @req_free: teardown context for given request, unlocked
+ * @exec: run @req, may be invoked multiple times per @req, locked
+ * @lock: tsm work is one device and one op at a time
+ */
+struct tsm_pci_ops {
+	int (*add)(struct pci_dev *pdev);
+	void (*del)(struct pci_dev *pdev);
+	struct pci_tsm_req *(*req_alloc)(struct pci_dev *pdev,
+					 enum pci_tsm_op op);
+	struct pci_tsm_req *(*req_free)(struct pci_tsm_req *req);
+	enum pci_tsm_op_status (*exec)(struct pci_dev *pdev,
+				       struct pci_tsm_req *req);
+	struct mutex *lock;
+};
+
+enum pci_tsm_state {
+	PCI_TSM_IDLE,
+	PCI_TSM_INIT,
+	PCI_TSM_CONNECT,
+};
+
+enum pci_tsm_mode {
+	PCI_TSM_MODE_LINK,
+	PCI_TSM_MODE_SELECTIVE,
+};
+
+struct pci_tsm {
+	enum pci_tsm_state state;
+	enum pci_tsm_mode mode;
+	u16 ide_cap;
+	unsigned int link_capable:1;
+	unsigned int selective_capable:1;
+	void *tsm_data;
+};
+
+#ifdef CONFIG_PCI_TSM
+int pci_tsm_register(const struct tsm_pci_ops *ops);
+void pci_tsm_unregister(const struct tsm_pci_ops *ops);
+void generic_pci_tsm_req_free(struct pci_tsm_req *req);
+struct pci_tsm_req *generic_pci_tsm_req_alloc(struct pci_dev *pdev,
+					      enum pci_tsm_op op);
+#else
+static inline int pci_tsm_register(const struct tsm_pci_ops *ops)
+{
+	return 0;
+}
+static inline void pci_tsm_unregister(const struct tsm_pci_ops *ops)
+{
+}
+#endif
 #endif /* __TSM_H */
diff --git a/include/uapi/linux/pci_regs.h b/include/uapi/linux/pci_regs.h
index a39193213ff2..1219d50f8e89 100644
--- a/include/uapi/linux/pci_regs.h
+++ b/include/uapi/linux/pci_regs.h
@@ -742,7 +742,8 @@
 #define PCI_EXT_CAP_ID_PL_16GT	0x26	/* Physical Layer 16.0 GT/s */
 #define PCI_EXT_CAP_ID_PL_32GT  0x2A    /* Physical Layer 32.0 GT/s */
 #define PCI_EXT_CAP_ID_DOE	0x2E	/* Data Object Exchange */
-#define PCI_EXT_CAP_ID_MAX	PCI_EXT_CAP_ID_DOE
+#define PCI_EXT_CAP_ID_IDE	0x30	/* Integrity and Data Encryption */
+#define PCI_EXT_CAP_ID_MAX	PCI_EXT_CAP_ID_IDE
 
 #define PCI_EXT_CAP_DSN_SIZEOF	12
 #define PCI_EXT_CAP_MCAST_ENDPOINT_SIZEOF 40

Re: [RFC PATCH 4/5] sysfs: Introduce a mechanism to hide static attribute_groups

[Greg KH]

On Tue, Jan 30, 2024 at 01:24:08AM -0800, Dan Williams wrote:
> Add a mechanism for named attribute_groups to hide their directory at
> sysfs_update_group() time, or otherwise skip emitting the group
> directory when the group is first registered. It piggybacks on
> is_visible() in a similar manner as SYSFS_PREALLOC, i.e. special flags
> in the upper bits of the returned mode. To use it, specify a symbol
> prefix to DEFINE_SYSFS_GROUP_VISIBLE(), and then pass that same prefix
> to SYSFS_GROUP_VISIBLE() when assigning the @is_visible() callback:
> 
> 	DEFINE_SYSFS_GROUP_VISIBLE($prefix)
> 
> 	struct attribute_group $prefix_group = {
> 		.name = $name,
> 		.is_visible = SYSFS_GROUP_VISIBLE($prefix),
> 	};
> 
> SYSFS_GROUP_VISIBLE() expects a definition of $prefix_group_visible()
> and $prefix_attr_visible(), where $prefix_group_visible() just returns
> true / false and $prefix_attr_visible() behaves as normal.
> 
> The motivation for this capability is to centralize PCI device
> authentication in the PCI core with a named sysfs group while keeping
> that group hidden for devices and platforms that do not meet the
> requirements. In a PCI topology, most devices will not support
> authentication, a small subset will support just PCI CMA (Component
> Measurement and Authentication), a smaller subset will support PCI CMA +
> PCIe IDE (Link Integrity and Encryption), and only next generation
> server hosts will start to include a platform TSM (TEE Security
> Manager).
> 
> Without this capability the alternatives are:
> 
> * Check if all attributes are invisible and if so, hide the directory.
>   Beyond trouble getting this to work [1], this is an ABI change for
>   scenarios if userspace happens to depend on group visibility absent any
>   attributes. I.e. this new capability avoids regression since it does
>   not retroactively apply to existing cases.
> 
> * Publish an empty /sys/bus/pci/devices/$pdev/tsm/ directory for all PCI
>   devices (i.e. for the case when TSM platform support is present, but
>   device support is absent). Unfortunate that this will be a vestigial
>   empty directory in the vast majority of cases.
> 
> * Reintroduce usage of runtime calls to sysfs_{create,remove}_group()
>   in the PCI core. Bjorn has already indicated that he does not want to
>   see any growth of pci_sysfs_init() [2].
> 
> * Drop the named group and simulate a directory by prefixing all
>   TSM-related attributes with "tsm_". Unfortunate to not use the naming
>   capability of a sysfs group as intended.
> 
> In comparison, there is a small potential for regression if for some
> reason an @is_visible() callback had dependencies on how many times it
> was called. Additionally, it is no longer an error to update a group
> that does not have its directory already present, and it is no longer a
> WARN() to remove a group that was never visible.
> 
> Link: https://lore.kernel.org/all/2024012321-envious-procedure-4a58@gregkh/ [1]
> Link: https://lore.kernel.org/linux-pci/20231019200110.GA1410324@bhelgaas/ [2]
> Signed-off-by: Dan Williams <dan.j.williams@intel.com>
> ---
>  fs/sysfs/group.c      |   45 ++++++++++++++++++++++++++++-------
>  include/linux/sysfs.h |   63 ++++++++++++++++++++++++++++++++++++++++---------
>  2 files changed, 87 insertions(+), 21 deletions(-)

You beat me to this again :)

I have tested this patch, and it looks good, I'll send out my series
that uses it for a different subsystem as well.

I guess I can take this as a static tag for others to pull from for this
rc development cycle?

thanks,

greg k-h

Re: [RFC PATCH 4/5] sysfs: Introduce a mechanism to hide static attribute_groups

[Dan Williams]

Greg KH wrote:
> On Tue, Jan 30, 2024 at 01:24:08AM -0800, Dan Williams wrote:
> > Add a mechanism for named attribute_groups to hide their directory at
> > sysfs_update_group() time, or otherwise skip emitting the group
> > directory when the group is first registered. It piggybacks on
> > is_visible() in a similar manner as SYSFS_PREALLOC, i.e. special flags
> > in the upper bits of the returned mode. To use it, specify a symbol
> > prefix to DEFINE_SYSFS_GROUP_VISIBLE(), and then pass that same prefix
> > to SYSFS_GROUP_VISIBLE() when assigning the @is_visible() callback:
> > 
> > 	DEFINE_SYSFS_GROUP_VISIBLE($prefix)
> > 
> > 	struct attribute_group $prefix_group = {
> > 		.name = $name,
> > 		.is_visible = SYSFS_GROUP_VISIBLE($prefix),
> > 	};
> > 
> > SYSFS_GROUP_VISIBLE() expects a definition of $prefix_group_visible()
> > and $prefix_attr_visible(), where $prefix_group_visible() just returns
> > true / false and $prefix_attr_visible() behaves as normal.
> > 
> > The motivation for this capability is to centralize PCI device
> > authentication in the PCI core with a named sysfs group while keeping
> > that group hidden for devices and platforms that do not meet the
> > requirements. In a PCI topology, most devices will not support
> > authentication, a small subset will support just PCI CMA (Component
> > Measurement and Authentication), a smaller subset will support PCI CMA +
> > PCIe IDE (Link Integrity and Encryption), and only next generation
> > server hosts will start to include a platform TSM (TEE Security
> > Manager).
> > 
> > Without this capability the alternatives are:
> > 
> > * Check if all attributes are invisible and if so, hide the directory.
> >   Beyond trouble getting this to work [1], this is an ABI change for
> >   scenarios if userspace happens to depend on group visibility absent any
> >   attributes. I.e. this new capability avoids regression since it does
> >   not retroactively apply to existing cases.
> > 
> > * Publish an empty /sys/bus/pci/devices/$pdev/tsm/ directory for all PCI
> >   devices (i.e. for the case when TSM platform support is present, but
> >   device support is absent). Unfortunate that this will be a vestigial
> >   empty directory in the vast majority of cases.
> > 
> > * Reintroduce usage of runtime calls to sysfs_{create,remove}_group()
> >   in the PCI core. Bjorn has already indicated that he does not want to
> >   see any growth of pci_sysfs_init() [2].
> > 
> > * Drop the named group and simulate a directory by prefixing all
> >   TSM-related attributes with "tsm_". Unfortunate to not use the naming
> >   capability of a sysfs group as intended.
> > 
> > In comparison, there is a small potential for regression if for some
> > reason an @is_visible() callback had dependencies on how many times it
> > was called. Additionally, it is no longer an error to update a group
> > that does not have its directory already present, and it is no longer a
> > WARN() to remove a group that was never visible.
> > 
> > Link: https://lore.kernel.org/all/2024012321-envious-procedure-4a58@gregkh/ [1]
> > Link: https://lore.kernel.org/linux-pci/20231019200110.GA1410324@bhelgaas/ [2]
> > Signed-off-by: Dan Williams <dan.j.williams@intel.com>
> > ---
> >  fs/sysfs/group.c      |   45 ++++++++++++++++++++++++++++-------
> >  include/linux/sysfs.h |   63 ++++++++++++++++++++++++++++++++++++++++---------
> >  2 files changed, 87 insertions(+), 21 deletions(-)
> 
> You beat me to this again :)

Pardon the spam, was just showing it in context of the patchset I was
developing.

> I have tested this patch, and it looks good, I'll send out my series
> that uses it for a different subsystem as well.
> 
> I guess I can take this as a static tag for others to pull from for this
> rc development cycle?

That works for me. Thanks Greg!

Re: [RFC PATCH 4/5] sysfs: Introduce a mechanism to hide static attribute_groups

[Greg KH]

On Tue, Jan 30, 2024 at 08:48:26AM -0800, Dan Williams wrote:
> Greg KH wrote:
> > On Tue, Jan 30, 2024 at 01:24:08AM -0800, Dan Williams wrote:
> > > Add a mechanism for named attribute_groups to hide their directory at
> > > sysfs_update_group() time, or otherwise skip emitting the group
> > > directory when the group is first registered. It piggybacks on
> > > is_visible() in a similar manner as SYSFS_PREALLOC, i.e. special flags
> > > in the upper bits of the returned mode. To use it, specify a symbol
> > > prefix to DEFINE_SYSFS_GROUP_VISIBLE(), and then pass that same prefix
> > > to SYSFS_GROUP_VISIBLE() when assigning the @is_visible() callback:
> > > 
> > > 	DEFINE_SYSFS_GROUP_VISIBLE($prefix)
> > > 
> > > 	struct attribute_group $prefix_group = {
> > > 		.name = $name,
> > > 		.is_visible = SYSFS_GROUP_VISIBLE($prefix),
> > > 	};
> > > 
> > > SYSFS_GROUP_VISIBLE() expects a definition of $prefix_group_visible()
> > > and $prefix_attr_visible(), where $prefix_group_visible() just returns
> > > true / false and $prefix_attr_visible() behaves as normal.
> > > 
> > > The motivation for this capability is to centralize PCI device
> > > authentication in the PCI core with a named sysfs group while keeping
> > > that group hidden for devices and platforms that do not meet the
> > > requirements. In a PCI topology, most devices will not support
> > > authentication, a small subset will support just PCI CMA (Component
> > > Measurement and Authentication), a smaller subset will support PCI CMA +
> > > PCIe IDE (Link Integrity and Encryption), and only next generation
> > > server hosts will start to include a platform TSM (TEE Security
> > > Manager).
> > > 
> > > Without this capability the alternatives are:
> > > 
> > > * Check if all attributes are invisible and if so, hide the directory.
> > >   Beyond trouble getting this to work [1], this is an ABI change for
> > >   scenarios if userspace happens to depend on group visibility absent any
> > >   attributes. I.e. this new capability avoids regression since it does
> > >   not retroactively apply to existing cases.
> > > 
> > > * Publish an empty /sys/bus/pci/devices/$pdev/tsm/ directory for all PCI
> > >   devices (i.e. for the case when TSM platform support is present, but
> > >   device support is absent). Unfortunate that this will be a vestigial
> > >   empty directory in the vast majority of cases.
> > > 
> > > * Reintroduce usage of runtime calls to sysfs_{create,remove}_group()
> > >   in the PCI core. Bjorn has already indicated that he does not want to
> > >   see any growth of pci_sysfs_init() [2].
> > > 
> > > * Drop the named group and simulate a directory by prefixing all
> > >   TSM-related attributes with "tsm_". Unfortunate to not use the naming
> > >   capability of a sysfs group as intended.
> > > 
> > > In comparison, there is a small potential for regression if for some
> > > reason an @is_visible() callback had dependencies on how many times it
> > > was called. Additionally, it is no longer an error to update a group
> > > that does not have its directory already present, and it is no longer a
> > > WARN() to remove a group that was never visible.
> > > 
> > > Link: https://lore.kernel.org/all/2024012321-envious-procedure-4a58@gregkh/ [1]
> > > Link: https://lore.kernel.org/linux-pci/20231019200110.GA1410324@bhelgaas/ [2]
> > > Signed-off-by: Dan Williams <dan.j.williams@intel.com>
> > > ---
> > >  fs/sysfs/group.c      |   45 ++++++++++++++++++++++++++++-------
> > >  include/linux/sysfs.h |   63 ++++++++++++++++++++++++++++++++++++++++---------
> > >  2 files changed, 87 insertions(+), 21 deletions(-)
> > 
> > You beat me to this again :)
> 
> Pardon the spam, was just showing it in context of the patchset I was
> developing.

Patches are never spam!  :)

Re: [RFC PATCH 1/5] PCI/CMA: Prepare to interoperate with TSM authentication

[Bjorn Helgaas]

On Tue, Jan 30, 2024 at 01:23:51AM -0800, Dan Williams wrote:
> A TSM (TEE Security Manager) is a platform agent that facilitates TEE
> I/O (device assignment for confidential VMs). It uses PCI CMA, IDE, and
> TDISP to authenticate, encrypt/integrity-protect the link, and bind
> device-virtual-functions capable of accessing private memory to
> confidential VMs (TVMs).
> 
> Unlike native PCI CMA many of the details of establishing a connection
> between a device (DSM) and the TSM are abstracted through platform APIs.
> I.e. in the native case Linux picks the keys and validates the
> certificates, in the TSM case Linux just sees a "success" from invoking
> a "connect" API with the TSM.
> 
> SPDM only allows for one session-owner per transport (DOE), so the
> expectation is that authentication will only ever be in the "native"
> established case, or the "tsm" established case.

Holy cow, this is tasty nested acronym soup.  TEE, CMA, IDE, TDISP,
TVM, DSM, SPDM, DOE?  I know these will all become common knowledge in
a few years, but this is a big mouthful right now.  Is there any
overview or glossary in Documentation/ or similar?

Bjorn

Re: [RFC PATCH 5/5] PCI/TSM: Authenticate devices via platform TSM

[Bjorn Helgaas]

On Tue, Jan 30, 2024 at 01:24:14AM -0800, Dan Williams wrote:
> The PCIe 6.1 specification, section 11, introduces the Trusted
> Execution Environment (TEE) Device Interface Security Protocol (TDISP).
> This interface definition builds upon CMA, component measurement and
> authentication, and IDE, link integrity and data encryption. It adds
> support for establishing virtual functions within a device that can be
> assigned to a confidential VM such that the assigned device is enabled
> to access guest private memory protected by technologies like Intel TDX,
> AMD SEV-SNP, RISCV COVE, or ARM CCA.
> 
> The "TSM" (TEE Security Manager) is a concept in the TDISP specification
> of an agent that mediates between a device security manager (DSM) and
> system software in both a VMM and a VM. From a Linux perspective the TSM
> abstracts many of the details of TDISP, IDE, and CMA. Some of those
> details leak through at times, but for the most part TDISP is an
> internal implementation detail of the TSM.
> 
> Similar to the PCI core extensions to support CONFIG_PCI_CMA,
> CONFIG_PCI_TSM builds upon that to reuse the "authenticated" sysfs
> attribute, and add more properties + controls in a tsm/ subdirectory of
> the PCI device sysfs interface. Unlike CMA that can depend on a local to
> the PCI core implementation, PCI_TSM needs to be prepared for late
> loading of the platform TSM driver. Consider that the TSM driver may
> itself be a PCI driver. Userspace can depend on the common TSM device
> uevent to know when the PCI core has TSM services enabled. The PCI
> device tsm/ subdirectory is supplemented by the TSM device pci/
> directory for platform global TSM properties + controls.
> 
> All vendor TSM implementations share the property of asking the VMM to
> perform DOE mailbox operations on behalf of the TSM. That common
> capability is centralized in PCI core code that invokes an ->exec()
> operation callback potentially multiple times to service a given request
> (struct pci_tsm_req). Future operations / verbs will be handled
> similarly with the "request + exec" model. For now, only "connect" and
> "disconnect" are implemented which at a minimum is expected to establish
> IDE for the link.
> 
> In addition to requests the low-level TSM implementation is notified of
> device arrival and departure events so that it can filter devices that
> the TSM is not prepared to support, or otherwise setup and teardown
> per-device context.

Gulp, this is a good start and covers a lot of what I asked about
[1/5].  Should have read the whole series first ;)

Re: [RFC PATCH 2/5] coco/tsm: Establish a new coco/tsm subdirectory

[Kuppuswamy Sathyanarayanan]

On 1/30/24 1:23 AM, Dan Williams wrote:
> In preparation for new + common TSM infrastructure, establish
> drivers/virt/coco/tsm/. The tsm.ko module is renamed to tsm_reports.ko,
> and some of its symbols moved to the "tsm_report_" namespace to separate
> it from more generic "tsm" objects / symbols. The old tsm.ko module was
> only ever demand loaded by kernel internal dependencies, so it should
> not affect existing userspace module install scripts.

Since host related common code is also going be here, do you
want push reports.c under tsm/guest dir?

>
> Cc: Wu Hao <hao.wu@intel.com>
> Cc: Yilun Xu <yilun.xu@intel.com>
> Cc: Samuel Ortiz <sameo@rivosinc.com>
> Cc: Alexey Kardashevskiy <aik@amd.com>
> Signed-off-by: Dan Williams <dan.j.williams@intel.com>
> ---
>  drivers/virt/coco/Kconfig               |    6 ++----
>  drivers/virt/coco/Makefile              |    4 ++--
>  drivers/virt/coco/sev-guest/sev-guest.c |    8 ++++----
>  drivers/virt/coco/tdx-guest/tdx-guest.c |    8 ++++----
>  drivers/virt/coco/tsm/Kconfig           |    7 +++++++
>  drivers/virt/coco/tsm/Makefile          |    6 ++++++
>  drivers/virt/coco/tsm/reports.c         |   24 ++++++++++++------------
>  include/linux/tsm.h                     |   24 ++++++++++++------------
>  8 files changed, 49 insertions(+), 38 deletions(-)
>  create mode 100644 drivers/virt/coco/tsm/Kconfig
>  create mode 100644 drivers/virt/coco/tsm/Makefile
>  rename drivers/virt/coco/{tsm.c => tsm/reports.c} (94%)
>
> diff --git a/drivers/virt/coco/Kconfig b/drivers/virt/coco/Kconfig
> index 87d142c1f932..1e2eb5e768f9 100644
> --- a/drivers/virt/coco/Kconfig
> +++ b/drivers/virt/coco/Kconfig
> @@ -3,12 +3,10 @@
>  # Confidential computing related collateral
>  #
>  
> -config TSM_REPORTS
> -	select CONFIGFS_FS
> -	tristate
> -
>  source "drivers/virt/coco/efi_secret/Kconfig"
>  
>  source "drivers/virt/coco/sev-guest/Kconfig"
>  
>  source "drivers/virt/coco/tdx-guest/Kconfig"
> +
> +source "drivers/virt/coco/tsm/Kconfig"
> diff --git a/drivers/virt/coco/Makefile b/drivers/virt/coco/Makefile
> index 18c1aba5edb7..2c9d0a178678 100644
> --- a/drivers/virt/coco/Makefile
> +++ b/drivers/virt/coco/Makefile
> @@ -1,8 +1,8 @@
>  # SPDX-License-Identifier: GPL-2.0-only
>  #
>  # Confidential computing related collateral
> -#
> -obj-$(CONFIG_TSM_REPORTS)	+= tsm.o
> +
>  obj-$(CONFIG_EFI_SECRET)	+= efi_secret/
>  obj-$(CONFIG_SEV_GUEST)		+= sev-guest/
>  obj-$(CONFIG_INTEL_TDX_GUEST)	+= tdx-guest/
> +obj-y				+= tsm/
> diff --git a/drivers/virt/coco/sev-guest/sev-guest.c b/drivers/virt/coco/sev-guest/sev-guest.c
> index 87f241825bc3..d058cb8f9708 100644
> --- a/drivers/virt/coco/sev-guest/sev-guest.c
> +++ b/drivers/virt/coco/sev-guest/sev-guest.c
> @@ -786,7 +786,7 @@ struct snp_msg_cert_entry {
>  static int sev_report_new(struct tsm_report *report, void *data)
>  {
>  	struct snp_msg_cert_entry *cert_table;
> -	struct tsm_desc *desc = &report->desc;
> +	struct tsm_report_desc *desc = &report->desc;
>  	struct snp_guest_dev *snp_dev = data;
>  	struct snp_msg_report_resp_hdr hdr;
>  	const u32 report_size = SZ_4K;
> @@ -885,14 +885,14 @@ static int sev_report_new(struct tsm_report *report, void *data)
>  	return 0;
>  }
>  
> -static const struct tsm_ops sev_tsm_ops = {
> +static const struct tsm_report_ops sev_tsm_ops = {
>  	.name = KBUILD_MODNAME,
>  	.report_new = sev_report_new,
>  };
>  
>  static void unregister_sev_tsm(void *data)
>  {
> -	tsm_unregister(&sev_tsm_ops);
> +	tsm_report_unregister(&sev_tsm_ops);
>  }
>  
>  static int __init sev_guest_probe(struct platform_device *pdev)
> @@ -968,7 +968,7 @@ static int __init sev_guest_probe(struct platform_device *pdev)
>  	snp_dev->input.resp_gpa = __pa(snp_dev->response);
>  	snp_dev->input.data_gpa = __pa(snp_dev->certs_data);
>  
> -	ret = tsm_register(&sev_tsm_ops, snp_dev, &tsm_report_extra_type);
> +	ret = tsm_report_register(&sev_tsm_ops, snp_dev, &tsm_report_extra_type);
>  	if (ret)
>  		goto e_free_cert_data;
>  
> diff --git a/drivers/virt/coco/tdx-guest/tdx-guest.c b/drivers/virt/coco/tdx-guest/tdx-guest.c
> index 1253bf76b570..904f16461492 100644
> --- a/drivers/virt/coco/tdx-guest/tdx-guest.c
> +++ b/drivers/virt/coco/tdx-guest/tdx-guest.c
> @@ -163,7 +163,7 @@ static int tdx_report_new(struct tsm_report *report, void *data)
>  {
>  	u8 *buf, *reportdata = NULL, *tdreport = NULL;
>  	struct tdx_quote_buf *quote_buf = quote_data;
> -	struct tsm_desc *desc = &report->desc;
> +	struct tsm_report_desc *desc = &report->desc;
>  	int ret;
>  	u64 err;
>  
> @@ -278,7 +278,7 @@ static const struct x86_cpu_id tdx_guest_ids[] = {
>  };
>  MODULE_DEVICE_TABLE(x86cpu, tdx_guest_ids);
>  
> -static const struct tsm_ops tdx_tsm_ops = {
> +static const struct tsm_report_ops tdx_tsm_ops = {
>  	.name = KBUILD_MODNAME,
>  	.report_new = tdx_report_new,
>  };
> @@ -301,7 +301,7 @@ static int __init tdx_guest_init(void)
>  		goto free_misc;
>  	}
>  
> -	ret = tsm_register(&tdx_tsm_ops, NULL, NULL);
> +	ret = tsm_report_register(&tdx_tsm_ops, NULL, NULL);
>  	if (ret)
>  		goto free_quote;
>  
> @@ -318,7 +318,7 @@ module_init(tdx_guest_init);
>  
>  static void __exit tdx_guest_exit(void)
>  {
> -	tsm_unregister(&tdx_tsm_ops);
> +	tsm_report_unregister(&tdx_tsm_ops);
>  	free_quote_buf(quote_data);
>  	misc_deregister(&tdx_misc_dev);
>  }
> diff --git a/drivers/virt/coco/tsm/Kconfig b/drivers/virt/coco/tsm/Kconfig
> new file mode 100644
> index 000000000000..69f04461c83e
> --- /dev/null
> +++ b/drivers/virt/coco/tsm/Kconfig
> @@ -0,0 +1,7 @@
> +# SPDX-License-Identifier: GPL-2.0-only
> +#
> +# TSM (TEE Security Manager) Common infrastructure
> +
> +config TSM_REPORTS
> +	select CONFIGFS_FS
> +	tristate
> diff --git a/drivers/virt/coco/tsm/Makefile b/drivers/virt/coco/tsm/Makefile
> new file mode 100644
> index 000000000000..b48504a3ccfd
> --- /dev/null
> +++ b/drivers/virt/coco/tsm/Makefile
> @@ -0,0 +1,6 @@
> +# SPDX-License-Identifier: GPL-2.0-only
> +#
> +# TSM (TEE Security Manager) Common infrastructure
> +
> +obj-$(CONFIG_TSM_REPORTS) += tsm_reports.o
> +tsm_reports-y := reports.o
> diff --git a/drivers/virt/coco/tsm.c b/drivers/virt/coco/tsm/reports.c
> similarity index 94%
> rename from drivers/virt/coco/tsm.c
> rename to drivers/virt/coco/tsm/reports.c
> index d1c2db83a8ca..6cb0a0e6783d 100644
> --- a/drivers/virt/coco/tsm.c
> +++ b/drivers/virt/coco/tsm/reports.c
> @@ -13,7 +13,7 @@
>  #include <linux/configfs.h>
>  
>  static struct tsm_provider {
> -	const struct tsm_ops *ops;
> +	const struct tsm_report_ops *ops;
>  	const struct config_item_type *type;
>  	void *data;
>  } provider;
> @@ -98,7 +98,7 @@ static ssize_t tsm_report_privlevel_store(struct config_item *cfg,
>  	 * SEV-SNP GHCB) and a minimum of a TSM selected floor value no less
>  	 * than 0.
>  	 */
> -	if (provider.ops->privlevel_floor > val || val > TSM_PRIVLEVEL_MAX)
> +	if (provider.ops->privlevel_floor > val || val > TSM_REPORT_PRIVLEVEL_MAX)
>  		return -EINVAL;
>  
>  	guard(rwsem_write)(&tsm_rwsem);
> @@ -134,7 +134,7 @@ static ssize_t tsm_report_inblob_write(struct config_item *cfg,
>  	memcpy(report->desc.inblob, buf, count);
>  	return count;
>  }
> -CONFIGFS_BIN_ATTR_WO(tsm_report_, inblob, NULL, TSM_INBLOB_MAX);
> +CONFIGFS_BIN_ATTR_WO(tsm_report_, inblob, NULL, TSM_REPORT_INBLOB_MAX);
>  
>  static ssize_t tsm_report_generation_show(struct config_item *cfg, char *buf)
>  {
> @@ -201,7 +201,7 @@ static ssize_t tsm_report_read(struct tsm_report *report, void *buf,
>  			       size_t count, enum tsm_data_select select)
>  {
>  	struct tsm_report_state *state = to_state(report);
> -	const struct tsm_ops *ops;
> +	const struct tsm_report_ops *ops;
>  	ssize_t rc;
>  
>  	/* try to read from the existing report if present and valid... */
> @@ -241,7 +241,7 @@ static ssize_t tsm_report_outblob_read(struct config_item *cfg, void *buf,
>  
>  	return tsm_report_read(report, buf, count, TSM_REPORT);
>  }
> -CONFIGFS_BIN_ATTR_RO(tsm_report_, outblob, NULL, TSM_OUTBLOB_MAX);
> +CONFIGFS_BIN_ATTR_RO(tsm_report_, outblob, NULL, TSM_REPORT_OUTBLOB_MAX);
>  
>  static ssize_t tsm_report_auxblob_read(struct config_item *cfg, void *buf,
>  				       size_t count)
> @@ -250,7 +250,7 @@ static ssize_t tsm_report_auxblob_read(struct config_item *cfg, void *buf,
>  
>  	return tsm_report_read(report, buf, count, TSM_CERTS);
>  }
> -CONFIGFS_BIN_ATTR_RO(tsm_report_, auxblob, NULL, TSM_OUTBLOB_MAX);
> +CONFIGFS_BIN_ATTR_RO(tsm_report_, auxblob, NULL, TSM_REPORT_OUTBLOB_MAX);
>  
>  #define TSM_DEFAULT_ATTRS() \

Don't you plan to rename  TSM_DEFAULT_ATTRS & TSM_DEFAULT_BIN_ATTRS to
TSM_REPORT_* variants?

>  	&tsm_report_attr_generation, \
> @@ -353,10 +353,10 @@ static struct configfs_subsystem tsm_configfs = {
>  	.su_mutex = __MUTEX_INITIALIZER(tsm_configfs.su_mutex),
>  };
>  
> -int tsm_register(const struct tsm_ops *ops, void *priv,
> -		 const struct config_item_type *type)
> +int tsm_report_register(const struct tsm_report_ops *ops, void *priv,
> +			const struct config_item_type *type)
>  {
> -	const struct tsm_ops *conflict;
> +	const struct tsm_report_ops *conflict;
>  
>  	if (!type)
>  		type = &tsm_report_default_type;
> @@ -375,9 +375,9 @@ int tsm_register(const struct tsm_ops *ops, void *priv,
>  	provider.type = type;
>  	return 0;
>  }
> -EXPORT_SYMBOL_GPL(tsm_register);
> +EXPORT_SYMBOL_GPL(tsm_report_register);
>  
> -int tsm_unregister(const struct tsm_ops *ops)
> +int tsm_report_unregister(const struct tsm_report_ops *ops)
>  {
>  	guard(rwsem_write)(&tsm_rwsem);
>  	if (ops != provider.ops)
> @@ -387,7 +387,7 @@ int tsm_unregister(const struct tsm_ops *ops)
>  	provider.type = NULL;
>  	return 0;
>  }
> -EXPORT_SYMBOL_GPL(tsm_unregister);
> +EXPORT_SYMBOL_GPL(tsm_report_unregister);
>  
>  static struct config_group *tsm_report_group;
>  
> diff --git a/include/linux/tsm.h b/include/linux/tsm.h
> index de8324a2223c..28753608fcf5 100644
> --- a/include/linux/tsm.h
> +++ b/include/linux/tsm.h
> @@ -5,25 +5,25 @@
>  #include <linux/sizes.h>
>  #include <linux/types.h>
>  
> -#define TSM_INBLOB_MAX 64
> -#define TSM_OUTBLOB_MAX SZ_32K
> +#define TSM_REPORT_INBLOB_MAX 64
> +#define TSM_REPORT_OUTBLOB_MAX SZ_32K
>  
>  /*
>   * Privilege level is a nested permission concept to allow confidential
>   * guests to partition address space, 4-levels are supported.
>   */
> -#define TSM_PRIVLEVEL_MAX 3
> +#define TSM_REPORT_PRIVLEVEL_MAX 3
>  
>  /**
> - * struct tsm_desc - option descriptor for generating tsm report blobs
> + * struct tsm_report_desc - option descriptor for generating tsm report blobs
>   * @privlevel: optional privilege level to associate with @outblob
>   * @inblob_len: sizeof @inblob
>   * @inblob: arbitrary input data
>   */
> -struct tsm_desc {
> +struct tsm_report_desc {
>  	unsigned int privlevel;
>  	size_t inblob_len;
> -	u8 inblob[TSM_INBLOB_MAX];
> +	u8 inblob[TSM_REPORT_INBLOB_MAX];
>  };
>  
>  /**
> @@ -35,7 +35,7 @@ struct tsm_desc {
>   * @auxblob: (optional) auxiliary data to the report (e.g. certificate data)
>   */
>  struct tsm_report {
> -	struct tsm_desc desc;
> +	struct tsm_report_desc desc;
>  	size_t outblob_len;
>  	u8 *outblob;
>  	size_t auxblob_len;
> @@ -43,7 +43,7 @@ struct tsm_report {
>  };
>  
>  /**
> - * struct tsm_ops - attributes and operations for tsm instances
> + * struct tsm_report_ops - attributes and operations for tsm instances
I think it is tsm report instances
>   * @name: tsm id reflected in /sys/kernel/config/tsm/report/$report/provider
>   * @privlevel_floor: convey base privlevel for nested scenarios
>   * @report_new: Populate @report with the report blob and auxblob
> @@ -52,7 +52,7 @@ struct tsm_report {
>   * Implementation specific ops, only one is expected to be registered at
>   * a time i.e. only one of "sev-guest", "tdx-guest", etc.
>   */
> -struct tsm_ops {
> +struct tsm_report_ops {
>  	const char *name;
>  	const unsigned int privlevel_floor;
>  	int (*report_new)(struct tsm_report *report, void *data);
> @@ -63,7 +63,7 @@ extern const struct config_item_type tsm_report_default_type;
>  /* publish @privlevel, @privlevel_floor, and @auxblob attributes */
>  extern const struct config_item_type tsm_report_extra_type;
>  
> -int tsm_register(const struct tsm_ops *ops, void *priv,
> -		 const struct config_item_type *type);
> -int tsm_unregister(const struct tsm_ops *ops);
> +int tsm_report_register(const struct tsm_report_ops *ops, void *priv,
> +			const struct config_item_type *type);
> +int tsm_report_unregister(const struct tsm_report_ops *ops);
>  #endif /* __TSM_H */
>
>
-- 
Sathyanarayanan Kuppuswamy
Linux Kernel Developer

Re: [RFC PATCH 5/5] PCI/TSM: Authenticate devices via platform TSM

[Dan Williams]

Bjorn Helgaas wrote:
> On Tue, Jan 30, 2024 at 01:24:14AM -0800, Dan Williams wrote:
> > The PCIe 6.1 specification, section 11, introduces the Trusted
> > Execution Environment (TEE) Device Interface Security Protocol (TDISP).
> > This interface definition builds upon CMA, component measurement and
> > authentication, and IDE, link integrity and data encryption. It adds
> > support for establishing virtual functions within a device that can be
> > assigned to a confidential VM such that the assigned device is enabled
> > to access guest private memory protected by technologies like Intel TDX,
> > AMD SEV-SNP, RISCV COVE, or ARM CCA.
> > 
> > The "TSM" (TEE Security Manager) is a concept in the TDISP specification
> > of an agent that mediates between a device security manager (DSM) and
> > system software in both a VMM and a VM. From a Linux perspective the TSM
> > abstracts many of the details of TDISP, IDE, and CMA. Some of those
> > details leak through at times, but for the most part TDISP is an
> > internal implementation detail of the TSM.
> > 
> > Similar to the PCI core extensions to support CONFIG_PCI_CMA,
> > CONFIG_PCI_TSM builds upon that to reuse the "authenticated" sysfs
> > attribute, and add more properties + controls in a tsm/ subdirectory of
> > the PCI device sysfs interface. Unlike CMA that can depend on a local to
> > the PCI core implementation, PCI_TSM needs to be prepared for late
> > loading of the platform TSM driver. Consider that the TSM driver may
> > itself be a PCI driver. Userspace can depend on the common TSM device
> > uevent to know when the PCI core has TSM services enabled. The PCI
> > device tsm/ subdirectory is supplemented by the TSM device pci/
> > directory for platform global TSM properties + controls.
> > 
> > All vendor TSM implementations share the property of asking the VMM to
> > perform DOE mailbox operations on behalf of the TSM. That common
> > capability is centralized in PCI core code that invokes an ->exec()
> > operation callback potentially multiple times to service a given request
> > (struct pci_tsm_req). Future operations / verbs will be handled
> > similarly with the "request + exec" model. For now, only "connect" and
> > "disconnect" are implemented which at a minimum is expected to establish
> > IDE for the link.
> > 
> > In addition to requests the low-level TSM implementation is notified of
> > device arrival and departure events so that it can filter devices that
> > the TSM is not prepared to support, or otherwise setup and teardown
> > per-device context.
> 
> Gulp, this is a good start and covers a lot of what I asked about
> [1/5].  Should have read the whole series first ;)

I should have at least left a breadcrumb that the acronym soup is better
described in patch5. However, the specifications are certainly dense and
it is funny, in a laugh to keep from crying sort of way, that the
acronyms nest. "What do you mean the T in TDISP stands for TEE!?"

One interesting sidebar I had with Lukas about the naming was an
assertion that "Bjorn will want to call this the TDISP layer not the TSM
layer". The rationale being that the name of the PCIe specification
chapter that talks about the role of a 'TSM' is "Section 11. TEE Device
Interface Security Protocol (TDISP)". However, if there is one point I
want to get across in this posting is that TDISP is a protocol that is
spoken between the platform and the endpoint, i.e. between the TSM and
the DSM. A protocol abstracted away from Linux's view. Everything that
Linux needs to worry about is behind by the OS-to-TSM interface, and the
TDISP specification says next to nothing about what that OS-to-TSM
interface looks like. If it had standardized that, the job would be so
much easier.

So, I think Linux's role here is to act as a "standards body of last
resort" and try to hold platform definitions to a common understanding
of how much complexity is reasonable to export to Linux. Assert that the
per-platform portions intersect Linux at the same level of abstraction.
Basically, please no vendor-specific layering violations sprinkled
around the kernel, and enlighten the PCI core for core concepts (like
authentication). Do not build sidecars.

Re: [RFC PATCH 3/5] coco/tsm: Introduce a shared class device for TSMs

[Alexey Kardashevskiy]

On 30/1/24 20:24, Dan Williams wrote:
> A "tsm" is a platform component that provides an API for securely
> provisioning resources for a confidential guest (TVM) to consume. "TSM"
> also happens to be the acronym the PCI specification uses to define the
> platform agent that carries out device-security operations. That
> platform capability is commonly called TEE I/O. It is this arrival of
> TEE I/O platforms that requires the "tsm" concept to grow from a
> low-level arch-specific detail of TVM instantiation, to a frontend
> interface to mediate device setup and interact with general purpose
> kernel subsystems outside of arch/ like the PCI core.
> 
> Provide a virtual (as in /sys/devices/virtual) class device interface to
> front all of the aspects of a TSM and TEE I/O that are
> cross-architecture common. This includes mechanisms like enumerating
> available platform TEE I/O capabilities and provisioning connections
> between the platform TSM and device DSMs.
> 
> It is expected to handle hardware TSMs, like AMD SEV-SNP and ARM CCA
> where there is a physical TEE coprocessor device running firmware, as
> well as software TSMs like Intel TDX and RISC-V COVE, where there is a
> privileged software module loaded at runtime.
> 
> For now this is just the scaffolding for registering a TSM device and/or
> TSM-specific attribute groups.
> 
> Cc: Xiaoyao Li <xiaoyao.li@intel.com>
> Cc: Isaku Yamahata <isaku.yamahata@intel.com>
> Cc: Alexey Kardashevskiy <aik@amd.com>
> Cc: Wu Hao <hao.wu@intel.com>
> Cc: Yilun Xu <yilun.xu@intel.com>
> Cc: Tom Lendacky <thomas.lendacky@amd.com>
> Cc: John Allen <john.allen@amd.com>
> Signed-off-by: Dan Williams <dan.j.williams@intel.com>
> ---
>   Documentation/ABI/testing/sysfs-class-tsm |   12 +++
>   drivers/virt/coco/tsm/Kconfig             |    7 ++
>   drivers/virt/coco/tsm/Makefile            |    3 +
>   drivers/virt/coco/tsm/class.c             |  100 +++++++++++++++++++++++++++++
>   include/linux/tsm.h                       |    8 ++
>   5 files changed, 130 insertions(+)
>   create mode 100644 Documentation/ABI/testing/sysfs-class-tsm
>   create mode 100644 drivers/virt/coco/tsm/class.c
> 
> diff --git a/Documentation/ABI/testing/sysfs-class-tsm b/Documentation/ABI/testing/sysfs-class-tsm
> new file mode 100644
> index 000000000000..304b50b53e65
> --- /dev/null
> +++ b/Documentation/ABI/testing/sysfs-class-tsm
> @@ -0,0 +1,12 @@
> +What:		/sys/class/tsm/tsm0/host
> +Date:		January 2024
> +Contact:	linux-coco@lists.linux.dev
> +Description:
> +		(RO) For hardware TSMs represented by a device in /sys/devices,
> +		@host is a link to that device.
> +		Links to hardware TSM sysfs ABIs:
> +		- Documentation/ABI/testing/sysfs-driver-ccp
> +
> +		For software TSMs instantiated by a software module, @host is a
> +		directory with attributes for that TSM, and those attributes are
> +		documented below.
> diff --git a/drivers/virt/coco/tsm/Kconfig b/drivers/virt/coco/tsm/Kconfig
> index 69f04461c83e..595d86917462 100644
> --- a/drivers/virt/coco/tsm/Kconfig
> +++ b/drivers/virt/coco/tsm/Kconfig
> @@ -5,3 +5,10 @@
>   config TSM_REPORTS
>   	select CONFIGFS_FS
>   	tristate
> +
> +config ARCH_HAS_TSM
> +	bool
> +
> +config TSM
> +	depends on ARCH_HAS_TSM && SYSFS
> +	tristate
> diff --git a/drivers/virt/coco/tsm/Makefile b/drivers/virt/coco/tsm/Makefile
> index b48504a3ccfd..f7561169faed 100644
> --- a/drivers/virt/coco/tsm/Makefile
> +++ b/drivers/virt/coco/tsm/Makefile
> @@ -4,3 +4,6 @@
>   
>   obj-$(CONFIG_TSM_REPORTS) += tsm_reports.o
>   tsm_reports-y := reports.o
> +
> +obj-$(CONFIG_TSM) += tsm.o
> +tsm-y := class.o
> diff --git a/drivers/virt/coco/tsm/class.c b/drivers/virt/coco/tsm/class.c
> new file mode 100644
> index 000000000000..a569fa6b09eb
> --- /dev/null
> +++ b/drivers/virt/coco/tsm/class.c
> @@ -0,0 +1,100 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/* Copyright(c) 2024 Intel Corporation. All rights reserved. */
> +
> +#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
> +
> +#include <linux/tsm.h>
> +#include <linux/rwsem.h>
> +#include <linux/device.h>
> +#include <linux/module.h>
> +#include <linux/cleanup.h>
> +
> +static DECLARE_RWSEM(tsm_core_rwsem);
> +struct class *tsm_class;
> +struct tsm_subsys {
> +	struct device dev;
> +	const struct tsm_info *info;
> +} *tsm_subsys;
> +
> +int tsm_register(const struct tsm_info *info)
> +{
> +	struct device *dev __free(put_device) = NULL;
> +	struct tsm_subsys *subsys;
> +	int rc;
> +
> +	guard(rwsem_write)(&tsm_core_rwsem);
> +	if (tsm_subsys) {
> +		pr_warn("failed to register: \"%s\", \"%s\" already registered\n",
> +			info->name, tsm_subsys->info->name);
> +		return -EBUSY;
> +	}
> +
> +	subsys = kzalloc(sizeof(*subsys), GFP_KERNEL);
> +	if (!subsys)
> +		return -ENOMEM;
> +
> +	subsys->info = info;
> +	dev = &subsys->dev;
> +	dev->class = tsm_class;
> +	dev->groups = info->groups;
> +	dev_set_name(dev, "tsm0");
> +	rc = device_register(dev);
> +	if (rc)
> +		return rc;


no kfree(subsys) ? Thanks,

> +
> +	if (info->host) {
> +		rc = sysfs_create_link(&dev->kobj, &info->host->kobj, "host");
> +		if (rc)
> +			return rc;
> +	}
> +
> +	/* don't auto-free @dev */
> +	dev = NULL;
> +	tsm_subsys = subsys;
> +
> +	return 0;
> +}
> +EXPORT_SYMBOL_GPL(tsm_register);
> +
> +void tsm_unregister(const struct tsm_info *info)
> +{
> +	guard(rwsem_write)(&tsm_core_rwsem);
> +	if (!tsm_subsys || info != tsm_subsys->info) {
> +		pr_warn("failed to unregister: \"%s\", not currently registered\n",
> +			info->name);
> +		return;
> +	}
> +
> +	if (info->host)
> +		sysfs_remove_link(&tsm_subsys->dev.kobj, "host");
> +	device_unregister(&tsm_subsys->dev);
> +	tsm_subsys = NULL;
> +}
> +EXPORT_SYMBOL_GPL(tsm_unregister);
> +
> +static void tsm_release(struct device *dev)
> +{
> +	struct tsm_subsys *subsys = container_of(dev, typeof(*subsys), dev);
> +
> +	kfree(subsys);
> +}
> +
> +static int __init tsm_init(void)
> +{
> +	tsm_class = class_create("tsm");
> +	if (IS_ERR(tsm_class))
> +		return PTR_ERR(tsm_class);
> +
> +	tsm_class->dev_release = tsm_release;
> +	return 0;
> +}
> +module_init(tsm_init)
> +
> +static void __exit tsm_exit(void)
> +{
> +	class_destroy(tsm_class);
> +}
> +module_exit(tsm_exit)
> +
> +MODULE_LICENSE("GPL");
> +MODULE_DESCRIPTION("Trusted Security Module core device model");
> diff --git a/include/linux/tsm.h b/include/linux/tsm.h
> index 28753608fcf5..8cb8a661ba41 100644
> --- a/include/linux/tsm.h
> +++ b/include/linux/tsm.h
> @@ -5,6 +5,12 @@
>   #include <linux/sizes.h>
>   #include <linux/types.h>
>   
> +struct tsm_info {
> +	const char *name;
> +	struct device *host;
> +	const struct attribute_group **groups;
> +};
> +
>   #define TSM_REPORT_INBLOB_MAX 64
>   #define TSM_REPORT_OUTBLOB_MAX SZ_32K
>   
> @@ -66,4 +72,6 @@ extern const struct config_item_type tsm_report_extra_type;
>   int tsm_report_register(const struct tsm_report_ops *ops, void *priv,
>   			const struct config_item_type *type);
>   int tsm_report_unregister(const struct tsm_report_ops *ops);
> +int tsm_register(const struct tsm_info *info);
> +void tsm_unregister(const struct tsm_info *info);
>   #endif /* __TSM_H */
> 

-- 
Alexey

Re: [RFC PATCH 5/5] PCI/TSM: Authenticate devices via platform TSM

[Alexey Kardashevskiy]

On 30/1/24 20:24, Dan Williams wrote:
> The PCIe 6.1 specification, section 11, introduces the Trusted
> Execution Environment (TEE) Device Interface Security Protocol (TDISP).
> This interface definition builds upon CMA, component measurement and
> authentication, and IDE, link integrity and data encryption. It adds
> support for establishing virtual functions within a device that can be
> assigned to a confidential VM such that the assigned device is enabled
> to access guest private memory protected by technologies like Intel TDX,
> AMD SEV-SNP, RISCV COVE, or ARM CCA.
> 
> The "TSM" (TEE Security Manager) is a concept in the TDISP specification
> of an agent that mediates between a device security manager (DSM) and
> system software in both a VMM and a VM. From a Linux perspective the TSM
> abstracts many of the details of TDISP, IDE, and CMA. Some of those
> details leak through at times, but for the most part TDISP is an
> internal implementation detail of the TSM.
> 
> Similar to the PCI core extensions to support CONFIG_PCI_CMA,
> CONFIG_PCI_TSM builds upon that to reuse the "authenticated" sysfs
> attribute, and add more properties + controls in a tsm/ subdirectory of
> the PCI device sysfs interface. Unlike CMA that can depend on a local to
> the PCI core implementation, PCI_TSM needs to be prepared for late
> loading of the platform TSM driver. Consider that the TSM driver may
> itself be a PCI driver. Userspace can depend on the common TSM device
> uevent to know when the PCI core has TSM services enabled. The PCI
> device tsm/ subdirectory is supplemented by the TSM device pci/
> directory for platform global TSM properties + controls.
> 
> All vendor TSM implementations share the property of asking the VMM to
> perform DOE mailbox operations on behalf of the TSM. That common
> capability is centralized in PCI core code that invokes an ->exec()
> operation callback potentially multiple times to service a given request
> (struct pci_tsm_req). Future operations / verbs will be handled
> similarly with the "request + exec" model. For now, only "connect" and
> "disconnect" are implemented which at a minimum is expected to establish
> IDE for the link.
> 
> In addition to requests the low-level TSM implementation is notified of
> device arrival and departure events so that it can filter devices that
> the TSM is not prepared to support, or otherwise setup and teardown
> per-device context.


It's a good start but I am still digesting this scaffolding.

> 
> Cc: Wu Hao <hao.wu@intel.com>
> Cc: Yilun Xu <yilun.xu@intel.com>
> Cc: Lukas Wunner <lukas@wunner.de>
> Cc: Samuel Ortiz <sameo@rivosinc.com>
> Cc: Alexey Kardashevskiy <aik@amd.com>
> Cc: Bjorn Helgaas <bhelgaas@google.com>
> Signed-off-by: Dan Williams <dan.j.williams@intel.com>
> ---
>   Documentation/ABI/testing/sysfs-bus-pci   |   43 +++-
>   Documentation/ABI/testing/sysfs-class-tsm |   23 ++
>   drivers/pci/Kconfig                       |   15 +
>   drivers/pci/Makefile                      |    2
>   drivers/pci/cma.c                         |    5
>   drivers/pci/pci-sysfs.c                   |    3
>   drivers/pci/pci.h                         |   14 +
>   drivers/pci/probe.c                       |    1
>   drivers/pci/remove.c                      |    1
>   drivers/pci/tsm.c                         |  346 +++++++++++++++++++++++++++++
>   drivers/virt/coco/tsm/Makefile            |    1
>   drivers/virt/coco/tsm/class.c             |   22 +-
>   drivers/virt/coco/tsm/pci.c               |   83 +++++++
>   drivers/virt/coco/tsm/tsm.h               |   28 ++
>   include/linux/pci.h                       |    3
>   include/linux/tsm.h                       |   77 ++++++
>   include/uapi/linux/pci_regs.h             |    3
>   17 files changed, 662 insertions(+), 8 deletions(-)
>   create mode 100644 drivers/pci/tsm.c
>   create mode 100644 drivers/virt/coco/tsm/pci.c
>   create mode 100644 drivers/virt/coco/tsm/tsm.h
> 
> diff --git a/Documentation/ABI/testing/sysfs-bus-pci b/Documentation/ABI/testing/sysfs-bus-pci
> index 35b0e11fd0e6..0eef2128cf09 100644
> --- a/Documentation/ABI/testing/sysfs-bus-pci
> +++ b/Documentation/ABI/testing/sysfs-bus-pci
> @@ -508,11 +508,16 @@ Description:
>   		This file contains "native" if the device authenticated
>   		successfully with CMA-SPDM (PCIe r6.1 sec 6.31). It contains
>   		"none" if the device failed authentication (and may thus be
> -		malicious).
> +		malicious). It transitions from "native" to "tsm" after
> +		successful connection to a tsm, see the "connect" attribute
> +		below.
>   
>   		Writing "native" to this file causes reauthentication with
>   		kernel-selected keys and the kernel's certificate chain.  That
> -		may be opportune after updating the .cma keyring.
> +		may be opportune after updating the .cma keyring. Note
> +		that once connected to a tsm this returns -EBUSY to attempts to
> +		write "native", i.e. first disconnect from the tsm to retrigger
> +		native authentication.
>   
>   		The file is not visible if authentication is unsupported
>   		by the device.
> @@ -529,3 +534,37 @@ Description:
>   		The reason why authentication support could not be determined
>   		is apparent from "dmesg".  To probe for authentication support
>   		again, exercise the "remove" and "rescan" attributes.
> +
> +What:		/sys/bus/pci/devices/.../tsm/
> +Date:		January 2024
> +Contact:	linux-coco@lists.linux.dev
> +Description:
> +		This directory only appears if a device supports CMA and IDE,
> +		and only after a TSM driver has loaded and accepted / setup this
> +		PCI device. Similar to the 'authenticated' attribute, trigger
> +		"remove" and "rescan" to retry the initialization. See
> +		Documentation/ABI/testing/sysfs-class-tsm for enumerating the
> +		platform's TSM capabilities.
> +
> +What:		/sys/bus/pci/devices/.../tsm/connect
> +Date:		January 2024
> +Contact:	linux-coco@lists.linux.dev
> +Description:
> +		(RW) Writing "1" to this file triggers the TSM to establish a
> +		secure connection with the device. This typically includes an
> +		SPDM (DMTF Security Protocols and Data Models) session over PCIe
> +		DOE (Data Object Exchange) and PCIe IDE (Integrity and Data
> +		Encryption) establishment. For TSMs and devices that support
> +		both modes of IDE ("link" and "selective") the "connect_mode"
> +		attribute selects the mode.
> +
> +What:		/sys/bus/pci/devices/.../tsm/connect_mode
> +Date:		January 2024
> +Contact:	linux-coco@lists.linux.dev
> +Description:
> +		(RO) Returns the available connection modes optionally with
> +		brackets around the currently active mode if the device is
> +		connected. For example it may show "link selective" for a
> +		disconnected device, "link [selective]" for a selective
> +		connected device, and it may hide a mode that is not supported
> +		by the device or TSM.
> diff --git a/Documentation/ABI/testing/sysfs-class-tsm b/Documentation/ABI/testing/sysfs-class-tsm
> index 304b50b53e65..77957882738a 100644
> --- a/Documentation/ABI/testing/sysfs-class-tsm
> +++ b/Documentation/ABI/testing/sysfs-class-tsm
> @@ -10,3 +10,26 @@ Description:
>   		For software TSMs instantiated by a software module, @host is a
>   		directory with attributes for that TSM, and those attributes are
>   		documented below.
> +
> +
> +What:		/sys/class/tsm/tsm0/pci/link_capable
> +Date:		January, 2024
> +Contact:	linux-coco@lists.linux.dev
> +Description:
> +		(RO) When present this returns "1\n" to indicate that the TSM
> +		supports establishing Link IDE with a given root-port attached
> +		device. See "tsm/connect_mode" in
> +		Documentation/ABI/testing/sysfs-bus-pci

I am struggling to make sense of "a given root-port attached device".
There is one CCP device on AMD SEV and therefore one /sys/class/tsm/tsmX 
but still many root ports. How do root ports relate to /sys/class/tsm/tsm0 ?


> +
> +
> +What:		/sys/class/tsm/tsm0/pci/selective_streams
> +Date:		January, 2024
> +Contact:	linux-coco@lists.linux.dev
> +Description:
> +		(RO) When present this returns the number of currently available
> +		selective IDE streams available to the TSM. When a stream id is
> +		allocated this number is decremented and a link to the PCI
> +		device(s) consuming the stream(s) appears alonside this

s/alonside/alongside/

> +		attribute in the /sys/class/tsm/tsm0/pci/ directory. See
> +		"tsm/connect" and "tsm/connect_mode" in
> +		Documentation/ABI/testing/sysfs-bus-pci.
> diff --git a/drivers/pci/Kconfig b/drivers/pci/Kconfig
> index a5c3cadddd6f..11d788038d19 100644
> --- a/drivers/pci/Kconfig
> +++ b/drivers/pci/Kconfig
> @@ -129,6 +129,21 @@ config PCI_CMA
>   	  A PCI DOE mailbox is used as transport for DMTF SPDM based
>   	  authentication, measurement and secure channel establishment.
>   
> +config PCI_TSM
> +	bool "TEE Security Manager for Device Security"

(discussed elsewhere, I'll rant here once more and then will shut up)

It is bool and not tristate :(
CMA, DOE are the same, quite annoying (as in these early days I am 
adding printks here and there and rmmod+modpbobe saves time but builtins 
mean reboot) and imho no really necessary as (from 4/5) "only next 
generation server hosts will start to include a platform TSM".


> +	depends on PCI_CMA
> +	depends on TSM
> +	help
> +	  The TEE (Trusted Execution Environment) Device Interface
> +	  Security Protocol (TDISP) defines a "TSM" as a platform agent
> +	  that manages device authentication, link encryption, link
> +	  integrity protection, and assignment of PCI device functions
> +	  (virtual or physical) to confidential computing VMs that can
> +	  access (DMA) guest private memory.
> +
> +	  Say Y to enable the PCI subsystem to enable the IDE and
> +	  TDISP capabilities of devices via TSM semantics.
> +
>   config PCI_DOE
>   	bool
>   
> diff --git a/drivers/pci/Makefile b/drivers/pci/Makefile
> index cc8b5d1d15b9..c4117d67ea83 100644
> --- a/drivers/pci/Makefile
> +++ b/drivers/pci/Makefile
> @@ -38,6 +38,8 @@ obj-$(CONFIG_PCI_CMA)		+= cma.o cma.asn1.o
>   $(obj)/cma.o:			$(obj)/cma.asn1.h
>   $(obj)/cma.asn1.o:		$(obj)/cma.asn1.c $(obj)/cma.asn1.h
>   
> +obj-$(CONFIG_PCI_TSM)		+= tsm.o
> +
>   # Endpoint library must be initialized before its users
>   obj-$(CONFIG_PCI_ENDPOINT)	+= endpoint/
>   
> diff --git a/drivers/pci/cma.c b/drivers/pci/cma.c
> index be7d2bb21b4c..5a69e9919589 100644
> --- a/drivers/pci/cma.c
> +++ b/drivers/pci/cma.c
> @@ -39,6 +39,9 @@ static ssize_t authenticated_store(struct device *dev,
>   	if (!sysfs_streq(buf, "native"))
>   		return -EINVAL;
>   
> +	if (pci_tsm_authenticated(pdev))
> +		return -EBUSY;
> +
>   	rc = pci_cma_reauthenticate(pdev);
>   	if (rc)
>   		return rc;
> @@ -55,6 +58,8 @@ static ssize_t authenticated_show(struct device *dev,
>   	    (pdev->cma_init_failed || pdev->doe_init_failed))
>   		return -ENOTTY;
>   
> +	if (pci_tsm_authenticated(pdev))
> +		return sysfs_emit(buf, "tsm\n");
>   	if (spdm_authenticated(pdev->spdm_state))
>   		return sysfs_emit(buf, "native\n");
>   	return sysfs_emit(buf, "none\n");
> diff --git a/drivers/pci/pci-sysfs.c b/drivers/pci/pci-sysfs.c
> index 368c4f71cc55..4327f8c2e6b5 100644
> --- a/drivers/pci/pci-sysfs.c
> +++ b/drivers/pci/pci-sysfs.c
> @@ -1654,6 +1654,9 @@ const struct attribute_group *pci_dev_attr_groups[] = {
>   #endif
>   #ifdef CONFIG_PCI_CMA
>   	&pci_cma_attr_group,
> +#endif
> +#ifdef CONFIG_PCI_TSM
> +	&pci_tsm_attr_group,
>   #endif
>   	NULL,
>   };
> diff --git a/drivers/pci/pci.h b/drivers/pci/pci.h
> index 2b7d8d0b2e21..daa20866bc90 100644
> --- a/drivers/pci/pci.h
> +++ b/drivers/pci/pci.h
> @@ -350,6 +350,20 @@ static inline int pci_cma_reauthenticate(struct pci_dev *pdev)
>   }
>   #endif
>   
> +#ifdef CONFIG_PCI_TSM
> +void pci_tsm_init(struct pci_dev *pdev);
> +void pci_tsm_destroy(struct pci_dev *pdev);
> +extern const struct attribute_group pci_tsm_attr_group;
> +bool pci_tsm_authenticated(struct pci_dev *pdev);
> +#else
> +static inline void pci_tsm_init(struct pci_dev *pdev) { }
> +static inline void pci_tsm_destroy(struct pci_dev *pdev) { }
> +static inline bool pci_tsm_authenticated(struct pci_dev *pdev)
> +{
> +	return false;
> +}
> +#endif
> +
>   /**
>    * pci_dev_set_io_state - Set the new error state if possible.
>    *
> diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
> index 6b09c962c0b8..f60d6c3c8c48 100644
> --- a/drivers/pci/probe.c
> +++ b/drivers/pci/probe.c
> @@ -2542,6 +2542,7 @@ static void pci_init_capabilities(struct pci_dev *dev)
>   	pci_rcec_init(dev);		/* Root Complex Event Collector */
>   	pci_doe_init(dev);		/* Data Object Exchange */
>   	pci_cma_init(dev);		/* Component Measurement & Auth */
> +	pci_tsm_init(dev);		/* TEE Security Manager connection */
>   
>   	pcie_report_downtraining(dev);
>   	pci_init_reset_methods(dev);
> diff --git a/drivers/pci/remove.c b/drivers/pci/remove.c
> index f009ac578997..228fa6ccf911 100644
> --- a/drivers/pci/remove.c
> +++ b/drivers/pci/remove.c
> @@ -39,6 +39,7 @@ static void pci_destroy_dev(struct pci_dev *dev)
>   	list_del(&dev->bus_list);
>   	up_write(&pci_bus_sem);
>   
> +	pci_tsm_destroy(dev);
>   	pci_cma_destroy(dev);
>   	pci_doe_destroy(dev);
>   	pcie_aspm_exit_link_state(dev);
> diff --git a/drivers/pci/tsm.c b/drivers/pci/tsm.c
> new file mode 100644
> index 000000000000..f74de0ee49a0
> --- /dev/null
> +++ b/drivers/pci/tsm.c
> @@ -0,0 +1,346 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * TEE Security Manager for the TEE Device Interface Security Protocol
> + * (TDISP, PCIe r6.1 sec 11)
> + *
> + * Copyright(c) 2024 Intel Corporation. All rights reserved.
> + */
> +
> +#define dev_fmt(fmt) "TSM: " fmt
> +
> +#include <linux/pci.h>
> +#include <linux/tsm.h>
> +#include <linux/sysfs.h>
> +#include <linux/xarray.h>
> +#include "pci.h"
> +
> +/* collect tsm capable devices to rendezvous with the tsm driver */
> +static DEFINE_XARRAY(pci_tsm_devs);

Not used anywhere.


> +
> +/*
> + * Provide a read/write lock against the init / exit of pdev tsm
> + * capabilities and arrival/departure of a tsm instance
> + */
> +static DECLARE_RWSEM(pci_tsm_rwsem);
> +static const struct tsm_pci_ops *tsm_ops;
> +
> +void generic_pci_tsm_req_free(struct pci_tsm_req *req)
> +{
> +	kfree(req);
> +}
> +EXPORT_SYMBOL_GPL(generic_pci_tsm_req_free);
> +
> +struct pci_tsm_req *generic_pci_tsm_req_alloc(struct pci_dev *pdev, enum pci_tsm_op op)
> +{
> +	struct pci_tsm_req *req = kzalloc(sizeof(*req), GFP_KERNEL);
> +
> +	if (!req)
> +		return NULL;
> +	req->op = op;
> +	return req;
> +}
> +EXPORT_SYMBOL_GPL(generic_pci_tsm_req_alloc);
> +
> +DEFINE_FREE(req_free, struct pci_tsm_req *, if (_T) tsm_ops->req_free(_T))
> +
> +static int pci_tsm_disconnect(struct pci_dev *pdev)
> +{
> +	struct pci_tsm_req *req __free(req_free) = NULL;
> +
> +	/* opportunistic state checks to skip allocating a request */
> +	if (pdev->tsm->state < PCI_TSM_CONNECT)
> +		return 0;
> +
> +	req = tsm_ops->req_alloc(pdev, PCI_TSM_OP_DISCONNECT);
> +	if (!req)
> +		return -ENOMEM;
> +
> +	scoped_cond_guard(mutex_intr, return -EINTR, tsm_ops->lock) {
> +		enum pci_tsm_op_status status;
> +
> +		/* revalidate state */
> +		if (pdev->tsm->state < PCI_TSM_CONNECT)
> +			return 0;
> +		if (pdev->tsm->state < PCI_TSM_INIT)
> +			return -ENXIO;
> +
> +		do {
> +			status = tsm_ops->exec(pdev, req);
> +			req->seq++;
> +			/* TODO: marshal SPDM request */
> +		} while (status == PCI_TSM_SPDM_REQ);
> +
> +		if (status == PCI_TSM_FAIL)
> +			return -EIO;
> +		pdev->tsm->state = PCI_TSM_INIT;
> +	}
> +	return 0;
> +}
> +
> +static int pci_tsm_connect(struct pci_dev *pdev)
> +{
> +	struct pci_tsm_req *req __free(req_free) = NULL;
> +
> +	/* opportunistic state checks to skip allocating a request */
> +	if (pdev->tsm->state >= PCI_TSM_CONNECT)
> +		return 0;
> +
> +	req = tsm_ops->req_alloc(pdev, PCI_TSM_OP_CONNECT);
> +	if (!req)
> +		return -ENOMEM;
> +
> +	scoped_cond_guard(mutex_intr, return -EINTR, tsm_ops->lock) {
> +		enum pci_tsm_op_status status;
> +
> +		/* revalidate state */
> +		if (pdev->tsm->state >= PCI_TSM_CONNECT)
> +			return 0;
> +		if (pdev->tsm->state < PCI_TSM_INIT)
> +			return -ENXIO;
> +
> +		do {
> +			status = tsm_ops->exec(pdev, req);
> +			req->seq++;
> +		} while (status == PCI_TSM_SPDM_REQ);
> +
> +		if (status == PCI_TSM_FAIL)
> +			return -EIO;
> +		pdev->tsm->state = PCI_TSM_CONNECT;
> +	}
> +	return 0;
> +}
> +
> +static ssize_t connect_store(struct device *dev, struct device_attribute *attr,
> +			     const char *buf, size_t len)
> +{
> +	bool connect;
> +	int rc = kstrtobool(buf, &connect);
> +	struct pci_dev *pdev = to_pci_dev(dev);
> +
> +	if (rc)
> +		return rc;
> +
> +	if (connect) {
> +		if (!spdm_authenticated(pdev->spdm_state)) {
> +			pci_dbg(pdev, "SPDM authentication pre-requisite not met.\n");
> +			return -ENXIO;
> +		}
> +		rc = pci_tsm_connect(pdev);
> +		if (rc)
> +			return rc;
> +		return len;
> +	}
> +
> +	rc = pci_tsm_disconnect(pdev);
> +	if (rc)
> +		return rc;
> +	return len;
> +}
> +
> +static ssize_t connect_show(struct device *dev, struct device_attribute *attr,
> +			    char *buf)
> +{
> +	struct pci_dev *pdev = to_pci_dev(dev);
> +
> +	return sysfs_emit(buf, "%d\n", pdev->tsm->state >= PCI_TSM_CONNECT);
> +}
> +static DEVICE_ATTR_RW(connect);
> +
> +static const char *const pci_tsm_modes[] = {
> +	[PCI_TSM_MODE_LINK] = "link",
> +	[PCI_TSM_MODE_SELECTIVE] = "selective",
> +};
> +
> +static ssize_t connect_mode_store(struct device *dev,
> +				  struct device_attribute *attr,
> +				  const char *buf, size_t len)
> +{
> +	struct pci_dev *pdev = to_pci_dev(dev);
> +	int i;
> +
> +	guard(mutex)(tsm_ops->lock);
> +	if (pdev->tsm->state >= PCI_TSM_CONNECT)
> +		return -EBUSY;
> +	for (i = 0; i < ARRAY_SIZE(pci_tsm_modes); i++)
> +		if (sysfs_streq(buf, pci_tsm_modes[i]))
> +			break;
> +	if (i == PCI_TSM_MODE_LINK) {
> +		if (pdev->tsm->link_capable)
> +			pdev->tsm->mode = PCI_TSM_MODE_LINK;
> +		return -EOPNOTSUPP;
> +	} else if (i == PCI_TSM_MODE_SELECTIVE) {
> +		if (pdev->tsm->selective_capable)
> +			pdev->tsm->mode = PCI_TSM_MODE_SELECTIVE;
> +		return -EOPNOTSUPP;
> +	} else
> +		return -EINVAL;
> +	return len;
> +}
> +
> +static ssize_t connect_mode_show(struct device *dev,
> +				 struct device_attribute *attr, char *buf)
> +{
> +	struct pci_dev *pdev = to_pci_dev(dev);
> +	ssize_t count = 0;
> +	int i;
> +
> +	for (i = 0; i < ARRAY_SIZE(pci_tsm_modes); i++) {
> +		if (i == PCI_TSM_MODE_LINK) {
> +			if (!pdev->tsm->link_capable)
> +				continue;
> +		} else if (i == PCI_TSM_MODE_SELECTIVE) {
> +			if (!pdev->tsm->selective_capable)
> +				continue;
> +		}
> +
> +		if (i == pdev->tsm->mode)
> +			count += sysfs_emit_at(buf, count, "[%s] ",
> +					       pci_tsm_modes[i]);
> +		else
> +			count += sysfs_emit_at(buf, count, "%s ",
> +					       pci_tsm_modes[i]);
> +	}
> +
> +	if (count)
> +		buf[count - 1] = '\n';
> +
> +	return count;
> +}
> +static DEVICE_ATTR_RW(connect_mode);
> +
> +static umode_t pci_tsm_attr_visible(struct kobject *kobj, struct attribute *a, int n)
> +{
> +	struct device *dev = kobj_to_dev(kobj);
> +	struct pci_dev *pdev = to_pci_dev(dev);
> +
> +	if (a == &dev_attr_connect_mode.attr) {
> +		if (pdev->tsm->link_capable || pdev->tsm->selective_capable)
> +			return a->mode;
> +		return 0;
> +	}
> +
> +	return a->mode;
> +}
> +
> +static bool pci_tsm_group_visible(struct kobject *kobj)
> +{
> +	struct device *dev = kobj_to_dev(kobj);
> +	struct pci_dev *pdev = to_pci_dev(dev);
> +
> +	if (pdev->tsm && pdev->tsm->state > PCI_TSM_IDLE)
> +		return true;
> +	return false;
> +}
> +DEFINE_SYSFS_GROUP_VISIBLE(pci_tsm);
> +
> +static struct attribute *pci_tsm_attrs[] = {
> +	&dev_attr_connect.attr,
> +	&dev_attr_connect_mode.attr,
> +	NULL,
> +};
> +
> +const struct attribute_group pci_tsm_attr_group = {
> +	.name = "tsm",
> +	.attrs = pci_tsm_attrs,
> +	.is_visible = SYSFS_GROUP_VISIBLE(pci_tsm),
> +};
> +
> +static int pci_tsm_add(struct pci_dev *pdev)

Nothing checks the returned value.

> +{
> +	lockdep_assert_held(&pci_tsm_rwsem);
> +	if (!tsm_ops)
> +		return 0;
> +	scoped_guard(mutex, tsm_ops->lock) {
> +		if (pdev->tsm->state < PCI_TSM_INIT) {
> +			int rc = tsm_ops->add(pdev);
> +
> +			if (rc)
> +				return rc;
> +		}
> +		pdev->tsm->state = PCI_TSM_INIT;
> +	}
> +	return sysfs_update_group(&pdev->dev.kobj, &pci_tsm_attr_group);
> +}
> +
> +static void pci_tsm_del(struct pci_dev *pdev)
> +{
> +	lockdep_assert_held(&pci_tsm_rwsem);
> +	/* shutdown sysfs operations before tsm delete */
> +	pdev->tsm->state = PCI_TSM_IDLE;
> +	sysfs_update_group(&pdev->dev.kobj, &pci_tsm_attr_group);
> +	guard(mutex)(tsm_ops->lock);
> +	tsm_ops->del(pdev);
> +}
> +
> +int pci_tsm_register(const struct tsm_pci_ops *ops)
> +{
> +	struct pci_dev *pdev;
> +	unsigned long index;
> +
> +	guard(rwsem_write)(&pci_tsm_rwsem);
> +	if (tsm_ops)
> +		return -EBUSY;
> +	tsm_ops = ops;
> +	xa_for_each(&pci_tsm_devs, index, pdev)
> +		pci_tsm_add(pdev);
> +	return 0;
> +}
> +EXPORT_SYMBOL_GPL(pci_tsm_register);
> +
> +void pci_tsm_unregister(const struct tsm_pci_ops *ops)
> +{
> +	struct pci_dev *pdev;
> +	unsigned long index;
> +
> +	guard(rwsem_write)(&pci_tsm_rwsem);
> +	if (ops != tsm_ops)
> +		return;
> +	xa_for_each(&pci_tsm_devs, index, pdev)
> +		pci_tsm_del(pdev);
> +	tsm_ops = NULL;
> +}
> +EXPORT_SYMBOL_GPL(pci_tsm_unregister);
> +
> +void pci_tsm_init(struct pci_dev *pdev)
> +{
> +	u16 ide_cap;
> +	int rc;
> +
> +	if (!pdev->cma_capable)
> +		return;
> +
> +	ide_cap = pci_find_ext_capability(pdev, PCI_EXT_CAP_ID_IDE);
> +	if (!ide_cap)
> +		return;
> +
> +	struct pci_tsm *tsm __free(kfree) = kzalloc(sizeof(*tsm), GFP_KERNEL);
> +	if (!tsm)
> +		return;
> +
> +	tsm->ide_cap = ide_cap;
> +
> +	rc = xa_insert(&pci_tsm_devs, (unsigned long)pdev, pdev, GFP_KERNEL);
> +	if (rc) {
> +		pci_dbg(pdev, "failed to register tsm capable device\n");
> +		return;
> +	}
> +
> +	guard(rwsem_write)(&pci_tsm_rwsem);
> +	pdev->tsm = no_free_ptr(tsm);
> +	pci_tsm_add(pdev);
> +}
> +
> +void pci_tsm_destroy(struct pci_dev *pdev)
> +{
> +	guard(rwsem_write)(&pci_tsm_rwsem);
> +	pci_tsm_del(pdev);
> +	xa_erase(&pci_tsm_devs, (unsigned long)pdev);
> +	kfree(pdev->tsm);
> +	pdev->tsm = NULL;
> +}
> +
> +bool pci_tsm_authenticated(struct pci_dev *pdev)
> +{
> +	guard(rwsem_read)(&pci_tsm_rwsem);
> +	return pdev->tsm && pdev->tsm->state >= PCI_TSM_CONNECT;
> +}
> diff --git a/drivers/virt/coco/tsm/Makefile b/drivers/virt/coco/tsm/Makefile
> index f7561169faed..a4f0d07d7d97 100644
> --- a/drivers/virt/coco/tsm/Makefile
> +++ b/drivers/virt/coco/tsm/Makefile
> @@ -7,3 +7,4 @@ tsm_reports-y := reports.o
>   
>   obj-$(CONFIG_TSM) += tsm.o
>   tsm-y := class.o
> +tsm-$(CONFIG_PCI_TSM) += pci.o
> diff --git a/drivers/virt/coco/tsm/class.c b/drivers/virt/coco/tsm/class.c
> index a569fa6b09eb..a459e51c0892 100644
> --- a/drivers/virt/coco/tsm/class.c
> +++ b/drivers/virt/coco/tsm/class.c
> @@ -8,13 +8,11 @@
>   #include <linux/device.h>
>   #include <linux/module.h>
>   #include <linux/cleanup.h>
> +#include "tsm.h"
>   
>   static DECLARE_RWSEM(tsm_core_rwsem);
> -struct class *tsm_class;
> -struct tsm_subsys {
> -	struct device dev;
> -	const struct tsm_info *info;
> -} *tsm_subsys;
> +static struct class *tsm_class;
> +static struct tsm_subsys *tsm_subsys;
>   
>   int tsm_register(const struct tsm_info *info)
>   {
> @@ -52,6 +50,10 @@ int tsm_register(const struct tsm_info *info)
>   	dev = NULL;
>   	tsm_subsys = subsys;
>   
> +	rc = tsm_pci_init(info);
> +	if (rc)
> +		pr_err("PCI initialization failure: %d\n", rc);
> +
>   	return 0;
>   }
>   EXPORT_SYMBOL_GPL(tsm_register);
> @@ -65,6 +67,8 @@ void tsm_unregister(const struct tsm_info *info)
>   		return;
>   	}
>   
> +	tsm_pci_destroy(info);
> +
>   	if (info->host)
>   		sysfs_remove_link(&tsm_subsys->dev.kobj, "host");
>   	device_unregister(&tsm_subsys->dev);
> @@ -79,6 +83,13 @@ static void tsm_release(struct device *dev)
>   	kfree(subsys);
>   }
>   
> +static const struct attribute_group *tsm_attr_groups[] = {
> +#ifdef CONFIG_PCI_TSM
> +	&tsm_pci_attr_group,
> +#endif
> +	NULL,
> +};
> +
>   static int __init tsm_init(void)
>   {
>   	tsm_class = class_create("tsm");
> @@ -86,6 +97,7 @@ static int __init tsm_init(void)
>   		return PTR_ERR(tsm_class);
>   
>   	tsm_class->dev_release = tsm_release;
> +	tsm_class->dev_groups = tsm_attr_groups;
>   	return 0;
>   }
>   module_init(tsm_init)
> diff --git a/drivers/virt/coco/tsm/pci.c b/drivers/virt/coco/tsm/pci.c
> new file mode 100644
> index 000000000000..b3684ad7114f
> --- /dev/null
> +++ b/drivers/virt/coco/tsm/pci.c
> @@ -0,0 +1,83 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/* Copyright(c) 2024 Intel Corporation. All rights reserved. */
> +
> +#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
> +
> +#include <linux/tsm.h>
> +#include <linux/device.h>
> +#include "tsm.h"
> +
> +static ssize_t link_capable_show(struct device *dev,
> +				 struct device_attribute *attr, char *buf)
> +{
> +	struct tsm_subsys *subsys = container_of(dev, typeof(*subsys), dev);
> +
> +	return sysfs_emit(buf, "%u\n", subsys->info->link_stream_capable);
> +}
> +static DEVICE_ATTR_RO(link_capable);
> +
> +static ssize_t selective_streams_show(struct device *dev,
> +				      struct device_attribute *attr, char *buf)
> +{
> +	struct tsm_subsys *subsys = container_of(dev, typeof(*subsys), dev);
> +
> +	return sysfs_emit(buf, "%u\n", subsys->info->nr_selective_streams);
> +}
> +static DEVICE_ATTR_RO(selective_streams);
> +
> +static umode_t tsm_pci_attr_visible(struct kobject *kobj, struct attribute *a, int n)
> +{
> +	struct device *dev = kobj_to_dev(kobj);
> +	struct tsm_subsys *subsys = container_of(dev, typeof(*subsys), dev);
> +	const struct tsm_info *info = subsys->info;
> +
> +	if (a == &dev_attr_link_capable.attr) {
> +		if (info->link_stream_capable)
> +			return a->mode;
> +		return 0;
> +	}
> +
> +	if (a == &dev_attr_selective_streams.attr) {
> +		if (info->nr_selective_streams)
> +			return a->mode;
> +		return 0;
> +	}
> +
> +	return a->mode;
> +}
> +
> +static bool tsm_pci_group_visible(struct kobject *kobj)
> +{
> +	struct device *dev = kobj_to_dev(kobj);
> +	struct tsm_subsys *subsys = container_of(dev, typeof(*subsys), dev);
> +
> +	if (subsys->info->pci_ops)
> +		return true;
> +	return false;
> +}
> +DEFINE_SYSFS_GROUP_VISIBLE(tsm_pci);
> +
> +static struct attribute *tsm_pci_attrs[] = {
> +	&dev_attr_link_capable.attr,
> +	&dev_attr_selective_streams.attr,
> +	NULL,
> +};
> +
> +const struct attribute_group tsm_pci_attr_group = {
> +	.name = "pci",
> +	.attrs = tsm_pci_attrs,
> +	.is_visible = SYSFS_GROUP_VISIBLE(tsm_pci),
> +};
> +
> +int tsm_pci_init(const struct tsm_info *info)
> +{
> +	if (!info->pci_ops)
> +		return 0;
> +
> +	return pci_tsm_register(info->pci_ops);
> +}
> +
> +void tsm_pci_destroy(const struct tsm_info *info)
> +{
> +	pci_tsm_unregister(info->pci_ops);
> +}
> diff --git a/drivers/virt/coco/tsm/tsm.h b/drivers/virt/coco/tsm/tsm.h
> new file mode 100644
> index 000000000000..407c388a109b
> --- /dev/null
> +++ b/drivers/virt/coco/tsm/tsm.h
> @@ -0,0 +1,28 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +#ifndef __TSM_CORE_H
> +#define __TSM_CORE_H
> +
> +#include <linux/device.h>
> +
> +struct tsm_info;
> +struct tsm_subsys {
> +	struct device dev;
> +	const struct tsm_info *info;
> +};

Have not you just defined this in 3/5? :)

> +
> +#ifdef CONFIG_PCI_TSM
> +int tsm_pci_init(const struct tsm_info *info);
> +void tsm_pci_destroy(const struct tsm_info *info);
> +extern const struct attribute_group tsm_pci_attr_group;
> +#else
> +static inline int tsm_pci_init(const struct tsm_info *info)
> +{
> +	return 0;
> +}
> +static inline void tsm_pci_destroy(const struct tsm_info *info)
> +{
> +}
> +#endif
> +
> +#endif /* TSM_CORE_H */
> +
> diff --git a/include/linux/pci.h b/include/linux/pci.h
> index 4a04ce7685e7..132962b21e04 100644
> --- a/include/linux/pci.h
> +++ b/include/linux/pci.h
> @@ -522,6 +522,9 @@ struct pci_dev {
>   	struct spdm_state *spdm_state;	/* Security Protocol and Data Model */
>   	unsigned int	cma_capable:1;	/* Authentication supported */
>   	unsigned int	cma_init_failed:1;
> +#endif
> +#ifdef CONFIG_PCI_TSM
> +	struct pci_tsm *tsm;		/* TSM operation state */
>   #endif
>   	u16		acs_cap;	/* ACS Capability offset */
>   	phys_addr_t	rom;		/* Physical address if not from BAR */
> diff --git a/include/linux/tsm.h b/include/linux/tsm.h
> index 8cb8a661ba41..f5dbdfa65d8d 100644
> --- a/include/linux/tsm.h
> +++ b/include/linux/tsm.h
> @@ -4,11 +4,15 @@
>   
>   #include <linux/sizes.h>
>   #include <linux/types.h>
> +#include <linux/mutex.h>
>   
>   struct tsm_info {
>   	const char *name;
>   	struct device *host;
>   	const struct attribute_group **groups;
> +	const struct tsm_pci_ops *pci_ops;
> +	unsigned int nr_selective_streams;
> +	unsigned int link_stream_capable:1;
>   };
>   
>   #define TSM_REPORT_INBLOB_MAX 64
> @@ -74,4 +78,77 @@ int tsm_report_register(const struct tsm_report_ops *ops, void *priv,
>   int tsm_report_unregister(const struct tsm_report_ops *ops);
>   int tsm_register(const struct tsm_info *info);
>   void tsm_unregister(const struct tsm_info *info);
> +
> +enum pci_tsm_op_status {
> +	PCI_TSM_FAIL = -1,
> +	PCI_TSM_OK,
> +	PCI_TSM_SPDM_REQ,

Secure SPDM is also needed here. In my toy TSM project [1] I am just 
using negatives for errors, 0 for "successfully finished" and positives 
for a DOE protocol (1 for SPDM, 2 for Secure SPDM), seems alright as it 
is all about PCI anyway (although "pci" is not always present in all 
these enums and structs).


> +};
> +
> +enum pci_tsm_op {
> +	PCI_TSM_OP_CONNECT,
> +	PCI_TSM_OP_DISCONNECT,
> +};
> +
> +struct pci_tsm_req {
> +	enum pci_tsm_op op;
> +	unsigned int seq;

@seq is not tested anywhere.

May be move (*req_free) here.


> +};
> +
> +struct pci_dev;
> +/**
> + * struct tsm_pci_ops - Low-level TSM-exported interface to the PCI core
> + * @add: accept device for tsm operation, locked
> + * @del: teardown tsm context for @pdev, locked
> + * @req_alloc: setup context for given operation, unlocked
> + * @req_free: teardown context for given request, unlocked
> + * @exec: run @req, may be invoked multiple times per @req, locked
> + * @lock: tsm work is one device and one op at a time
> + */
> +struct tsm_pci_ops {
> +	int (*add)(struct pci_dev *pdev);
> +	void (*del)(struct pci_dev *pdev);
> +	struct pci_tsm_req *(*req_alloc)(struct pci_dev *pdev,
> +					 enum pci_tsm_op op);
> +	struct pci_tsm_req *(*req_free)(struct pci_tsm_req *req);
> +	enum pci_tsm_op_status (*exec)(struct pci_dev *pdev,
> +				       struct pci_tsm_req *req);

The pci_tsm_req is just an @op, three hooks seems to be more than 
needed, could be just (*exec)(struct pci_dev *pdev, enum pci_tsm_op op).

Or the idea is to extend pci_tsm_req with some void *platform_req_data, 
is not it? There is one "op" in flight per a physical device allowed in 
SEV TIO, I suspect that is likely to be the case for others so such data 
can be managed by the platform code in the platform data of a TEE-IO device.


> +	struct mutex *lock;
> +};
> +
> +enum pci_tsm_state {
> +	PCI_TSM_IDLE,
> +	PCI_TSM_INIT,
> +	PCI_TSM_CONNECT,
> +};
> +
> +enum pci_tsm_mode {
> +	PCI_TSM_MODE_LINK,
> +	PCI_TSM_MODE_SELECTIVE,
> +};
> +
> +struct pci_tsm {
> +	enum pci_tsm_state state;
> +	enum pci_tsm_mode mode;

Does it have to be either mode and cannot be both?

> +	u16 ide_cap;
> +	unsigned int link_capable:1;
> +	unsigned int selective_capable:1;
> +	void *tsm_data;
> +};
> +
> +#ifdef CONFIG_PCI_TSM
> +int pci_tsm_register(const struct tsm_pci_ops *ops);
> +void pci_tsm_unregister(const struct tsm_pci_ops *ops);
> +void generic_pci_tsm_req_free(struct pci_tsm_req *req);
> +struct pci_tsm_req *generic_pci_tsm_req_alloc(struct pci_dev *pdev,
> +					      enum pci_tsm_op op);
> +#else
> +static inline int pci_tsm_register(const struct tsm_pci_ops *ops)
> +{
> +	return 0;
> +}
> +static inline void pci_tsm_unregister(const struct tsm_pci_ops *ops)
> +{
> +}
> +#endif
>   #endif /* __TSM_H */
> diff --git a/include/uapi/linux/pci_regs.h b/include/uapi/linux/pci_regs.h
> index a39193213ff2..1219d50f8e89 100644
> --- a/include/uapi/linux/pci_regs.h
> +++ b/include/uapi/linux/pci_regs.h
> @@ -742,7 +742,8 @@
>   #define PCI_EXT_CAP_ID_PL_16GT	0x26	/* Physical Layer 16.0 GT/s */
>   #define PCI_EXT_CAP_ID_PL_32GT  0x2A    /* Physical Layer 32.0 GT/s */
>   #define PCI_EXT_CAP_ID_DOE	0x2E	/* Data Object Exchange */
> -#define PCI_EXT_CAP_ID_MAX	PCI_EXT_CAP_ID_DOE
> +#define PCI_EXT_CAP_ID_IDE	0x30	/* Integrity and Data Encryption */
> +#define PCI_EXT_CAP_ID_MAX	PCI_EXT_CAP_ID_IDE
>   
>   #define PCI_EXT_CAP_DSN_SIZEOF	12
>   #define PCI_EXT_CAP_MCAST_ENDPOINT_SIZEOF 40
> 


[1] https://github.com/AMDESE/linux-kvm/commits/tio  <- this fella 
recently moved from my personal account
-- 
Alexey

Re: [RFC PATCH 5/5] PCI/TSM: Authenticate devices via platform TSM

[Alexey Kardashevskiy]

On 30/1/24 20:24, Dan Williams wrote:
> The PCIe 6.1 specification, section 11, introduces the Trusted
> Execution Environment (TEE) Device Interface Security Protocol (TDISP).
> This interface definition builds upon CMA, component measurement and
> authentication, and IDE, link integrity and data encryption. It adds
> support for establishing virtual functions within a device that can be
> assigned to a confidential VM such that the assigned device is enabled
> to access guest private memory protected by technologies like Intel TDX,
> AMD SEV-SNP, RISCV COVE, or ARM CCA.
> 
> The "TSM" (TEE Security Manager) is a concept in the TDISP specification
> of an agent that mediates between a device security manager (DSM) and
> system software in both a VMM and a VM. From a Linux perspective the TSM
> abstracts many of the details of TDISP, IDE, and CMA. Some of those
> details leak through at times, but for the most part TDISP is an
> internal implementation detail of the TSM.
> 
> Similar to the PCI core extensions to support CONFIG_PCI_CMA,
> CONFIG_PCI_TSM builds upon that to reuse the "authenticated" sysfs
> attribute, and add more properties + controls in a tsm/ subdirectory of
> the PCI device sysfs interface. Unlike CMA that can depend on a local to
> the PCI core implementation, PCI_TSM needs to be prepared for late
> loading of the platform TSM driver. Consider that the TSM driver may
> itself be a PCI driver. Userspace can depend on the common TSM device
> uevent to know when the PCI core has TSM services enabled. The PCI
> device tsm/ subdirectory is supplemented by the TSM device pci/
> directory for platform global TSM properties + controls.
> 
> All vendor TSM implementations share the property of asking the VMM to
> perform DOE mailbox operations on behalf of the TSM. That common
> capability is centralized in PCI core code that invokes an ->exec()
> operation callback potentially multiple times to service a given request
> (struct pci_tsm_req). Future operations / verbs will be handled
> similarly with the "request + exec" model. For now, only "connect" and
> "disconnect" are implemented which at a minimum is expected to establish
> IDE for the link.
> 
> In addition to requests the low-level TSM implementation is notified of
> device arrival and departure events so that it can filter devices that
> the TSM is not prepared to support, or otherwise setup and teardown
> per-device context.
> 
> Cc: Wu Hao <hao.wu@intel.com>
> Cc: Yilun Xu <yilun.xu@intel.com>
> Cc: Lukas Wunner <lukas@wunner.de>
> Cc: Samuel Ortiz <sameo@rivosinc.com>
> Cc: Alexey Kardashevskiy <aik@amd.com>
> Cc: Bjorn Helgaas <bhelgaas@google.com>
> Signed-off-by: Dan Williams <dan.j.williams@intel.com>
> ---
>   Documentation/ABI/testing/sysfs-bus-pci   |   43 +++-
>   Documentation/ABI/testing/sysfs-class-tsm |   23 ++
>   drivers/pci/Kconfig                       |   15 +
>   drivers/pci/Makefile                      |    2
>   drivers/pci/cma.c                         |    5
>   drivers/pci/pci-sysfs.c                   |    3
>   drivers/pci/pci.h                         |   14 +
>   drivers/pci/probe.c                       |    1
>   drivers/pci/remove.c                      |    1
>   drivers/pci/tsm.c                         |  346 +++++++++++++++++++++++++++++
>   drivers/virt/coco/tsm/Makefile            |    1
>   drivers/virt/coco/tsm/class.c             |   22 +-
>   drivers/virt/coco/tsm/pci.c               |   83 +++++++
>   drivers/virt/coco/tsm/tsm.h               |   28 ++
>   include/linux/pci.h                       |    3
>   include/linux/tsm.h                       |   77 ++++++
>   include/uapi/linux/pci_regs.h             |    3
>   17 files changed, 662 insertions(+), 8 deletions(-)
>   create mode 100644 drivers/pci/tsm.c
>   create mode 100644 drivers/virt/coco/tsm/pci.c
>   create mode 100644 drivers/virt/coco/tsm/tsm.h
> 
> diff --git a/Documentation/ABI/testing/sysfs-bus-pci b/Documentation/ABI/testing/sysfs-bus-pci
> index 35b0e11fd0e6..0eef2128cf09 100644
> --- a/Documentation/ABI/testing/sysfs-bus-pci
> +++ b/Documentation/ABI/testing/sysfs-bus-pci
> @@ -508,11 +508,16 @@ Description:
>   		This file contains "native" if the device authenticated
>   		successfully with CMA-SPDM (PCIe r6.1 sec 6.31). It contains
>   		"none" if the device failed authentication (and may thus be
> -		malicious).
> +		malicious). It transitions from "native" to "tsm" after
> +		successful connection to a tsm, see the "connect" attribute
> +		below.
>   
>   		Writing "native" to this file causes reauthentication with
>   		kernel-selected keys and the kernel's certificate chain.  That
> -		may be opportune after updating the .cma keyring.
> +		may be opportune after updating the .cma keyring. Note
> +		that once connected to a tsm this returns -EBUSY to attempts to
> +		write "native", i.e. first disconnect from the tsm to retrigger
> +		native authentication.
>   
>   		The file is not visible if authentication is unsupported
>   		by the device.
> @@ -529,3 +534,37 @@ Description:
>   		The reason why authentication support could not be determined
>   		is apparent from "dmesg".  To probe for authentication support
>   		again, exercise the "remove" and "rescan" attributes.
> +
> +What:		/sys/bus/pci/devices/.../tsm/
> +Date:		January 2024
> +Contact:	linux-coco@lists.linux.dev
> +Description:
> +		This directory only appears if a device supports CMA and IDE,
> +		and only after a TSM driver has loaded and accepted / setup this
> +		PCI device. Similar to the 'authenticated' attribute, trigger
> +		"remove" and "rescan" to retry the initialization. See
> +		Documentation/ABI/testing/sysfs-class-tsm for enumerating the
> +		platform's TSM capabilities.
> +
> +What:		/sys/bus/pci/devices/.../tsm/connect
> +Date:		January 2024
> +Contact:	linux-coco@lists.linux.dev
> +Description:
> +		(RW) Writing "1" to this file triggers the TSM to establish a
> +		secure connection with the device. This typically includes an
> +		SPDM (DMTF Security Protocols and Data Models) session over PCIe
> +		DOE (Data Object Exchange) and PCIe IDE (Integrity and Data
> +		Encryption) establishment. For TSMs and devices that support
> +		both modes of IDE ("link" and "selective") the "connect_mode"
> +		attribute selects the mode.
> +
> +What:		/sys/bus/pci/devices/.../tsm/connect_mode
> +Date:		January 2024
> +Contact:	linux-coco@lists.linux.dev
> +Description:
> +		(RO) Returns the available connection modes optionally with
> +		brackets around the currently active mode if the device is
> +		connected. For example it may show "link selective" for a
> +		disconnected device, "link [selective]" for a selective
> +		connected device, and it may hide a mode that is not supported
> +		by the device or TSM.
> diff --git a/Documentation/ABI/testing/sysfs-class-tsm b/Documentation/ABI/testing/sysfs-class-tsm
> index 304b50b53e65..77957882738a 100644
> --- a/Documentation/ABI/testing/sysfs-class-tsm
> +++ b/Documentation/ABI/testing/sysfs-class-tsm
> @@ -10,3 +10,26 @@ Description:
>   		For software TSMs instantiated by a software module, @host is a
>   		directory with attributes for that TSM, and those attributes are
>   		documented below.
> +
> +
> +What:		/sys/class/tsm/tsm0/pci/link_capable
> +Date:		January, 2024
> +Contact:	linux-coco@lists.linux.dev
> +Description:
> +		(RO) When present this returns "1\n" to indicate that the TSM
> +		supports establishing Link IDE with a given root-port attached
> +		device. See "tsm/connect_mode" in
> +		Documentation/ABI/testing/sysfs-bus-pci
> +
> +
> +What:		/sys/class/tsm/tsm0/pci/selective_streams
> +Date:		January, 2024
> +Contact:	linux-coco@lists.linux.dev
> +Description:
> +		(RO) When present this returns the number of currently available
> +		selective IDE streams available to the TSM. When a stream id is
> +		allocated this number is decremented and a link to the PCI
> +		device(s) consuming the stream(s) appears alonside this
> +		attribute in the /sys/class/tsm/tsm0/pci/ directory. See
> +		"tsm/connect" and "tsm/connect_mode" in
> +		Documentation/ABI/testing/sysfs-bus-pci.
> diff --git a/drivers/pci/Kconfig b/drivers/pci/Kconfig
> index a5c3cadddd6f..11d788038d19 100644
> --- a/drivers/pci/Kconfig
> +++ b/drivers/pci/Kconfig
> @@ -129,6 +129,21 @@ config PCI_CMA
>   	  A PCI DOE mailbox is used as transport for DMTF SPDM based
>   	  authentication, measurement and secure channel establishment.
>   
> +config PCI_TSM
> +	bool "TEE Security Manager for Device Security"
> +	depends on PCI_CMA
> +	depends on TSM
> +	help
> +	  The TEE (Trusted Execution Environment) Device Interface
> +	  Security Protocol (TDISP) defines a "TSM" as a platform agent
> +	  that manages device authentication, link encryption, link
> +	  integrity protection, and assignment of PCI device functions
> +	  (virtual or physical) to confidential computing VMs that can
> +	  access (DMA) guest private memory.
> +
> +	  Say Y to enable the PCI subsystem to enable the IDE and
> +	  TDISP capabilities of devices via TSM semantics.
> +
>   config PCI_DOE
>   	bool
>   
> diff --git a/drivers/pci/Makefile b/drivers/pci/Makefile
> index cc8b5d1d15b9..c4117d67ea83 100644
> --- a/drivers/pci/Makefile
> +++ b/drivers/pci/Makefile
> @@ -38,6 +38,8 @@ obj-$(CONFIG_PCI_CMA)		+= cma.o cma.asn1.o
>   $(obj)/cma.o:			$(obj)/cma.asn1.h
>   $(obj)/cma.asn1.o:		$(obj)/cma.asn1.c $(obj)/cma.asn1.h
>   
> +obj-$(CONFIG_PCI_TSM)		+= tsm.o
> +
>   # Endpoint library must be initialized before its users
>   obj-$(CONFIG_PCI_ENDPOINT)	+= endpoint/
>   
> diff --git a/drivers/pci/cma.c b/drivers/pci/cma.c
> index be7d2bb21b4c..5a69e9919589 100644
> --- a/drivers/pci/cma.c
> +++ b/drivers/pci/cma.c
> @@ -39,6 +39,9 @@ static ssize_t authenticated_store(struct device *dev,
>   	if (!sysfs_streq(buf, "native"))
>   		return -EINVAL;
>   
> +	if (pci_tsm_authenticated(pdev))
> +		return -EBUSY;
> +
>   	rc = pci_cma_reauthenticate(pdev);
>   	if (rc)
>   		return rc;

btw is this "native" CMA expected to migrate to tsm_pci_ops? Thanks,



-- 
Alexey

Re: [RFC PATCH 4/5] sysfs: Introduce a mechanism to hide static attribute_groups

[Greg KH]

On Tue, Jan 30, 2024 at 08:48:26AM -0800, Dan Williams wrote:
> Greg KH wrote:
> > On Tue, Jan 30, 2024 at 01:24:08AM -0800, Dan Williams wrote:
> > > Add a mechanism for named attribute_groups to hide their directory at
> > > sysfs_update_group() time, or otherwise skip emitting the group
> > > directory when the group is first registered. It piggybacks on
> > > is_visible() in a similar manner as SYSFS_PREALLOC, i.e. special flags
> > > in the upper bits of the returned mode. To use it, specify a symbol
> > > prefix to DEFINE_SYSFS_GROUP_VISIBLE(), and then pass that same prefix
> > > to SYSFS_GROUP_VISIBLE() when assigning the @is_visible() callback:
> > > 
> > > 	DEFINE_SYSFS_GROUP_VISIBLE($prefix)
> > > 
> > > 	struct attribute_group $prefix_group = {
> > > 		.name = $name,
> > > 		.is_visible = SYSFS_GROUP_VISIBLE($prefix),
> > > 	};
> > > 
> > > SYSFS_GROUP_VISIBLE() expects a definition of $prefix_group_visible()
> > > and $prefix_attr_visible(), where $prefix_group_visible() just returns
> > > true / false and $prefix_attr_visible() behaves as normal.
> > > 
> > > The motivation for this capability is to centralize PCI device
> > > authentication in the PCI core with a named sysfs group while keeping
> > > that group hidden for devices and platforms that do not meet the
> > > requirements. In a PCI topology, most devices will not support
> > > authentication, a small subset will support just PCI CMA (Component
> > > Measurement and Authentication), a smaller subset will support PCI CMA +
> > > PCIe IDE (Link Integrity and Encryption), and only next generation
> > > server hosts will start to include a platform TSM (TEE Security
> > > Manager).
> > > 
> > > Without this capability the alternatives are:
> > > 
> > > * Check if all attributes are invisible and if so, hide the directory.
> > >   Beyond trouble getting this to work [1], this is an ABI change for
> > >   scenarios if userspace happens to depend on group visibility absent any
> > >   attributes. I.e. this new capability avoids regression since it does
> > >   not retroactively apply to existing cases.
> > > 
> > > * Publish an empty /sys/bus/pci/devices/$pdev/tsm/ directory for all PCI
> > >   devices (i.e. for the case when TSM platform support is present, but
> > >   device support is absent). Unfortunate that this will be a vestigial
> > >   empty directory in the vast majority of cases.
> > > 
> > > * Reintroduce usage of runtime calls to sysfs_{create,remove}_group()
> > >   in the PCI core. Bjorn has already indicated that he does not want to
> > >   see any growth of pci_sysfs_init() [2].
> > > 
> > > * Drop the named group and simulate a directory by prefixing all
> > >   TSM-related attributes with "tsm_". Unfortunate to not use the naming
> > >   capability of a sysfs group as intended.
> > > 
> > > In comparison, there is a small potential for regression if for some
> > > reason an @is_visible() callback had dependencies on how many times it
> > > was called. Additionally, it is no longer an error to update a group
> > > that does not have its directory already present, and it is no longer a
> > > WARN() to remove a group that was never visible.
> > > 
> > > Link: https://lore.kernel.org/all/2024012321-envious-procedure-4a58@gregkh/ [1]
> > > Link: https://lore.kernel.org/linux-pci/20231019200110.GA1410324@bhelgaas/ [2]
> > > Signed-off-by: Dan Williams <dan.j.williams@intel.com>
> > > ---
> > >  fs/sysfs/group.c      |   45 ++++++++++++++++++++++++++++-------
> > >  include/linux/sysfs.h |   63 ++++++++++++++++++++++++++++++++++++++++---------
> > >  2 files changed, 87 insertions(+), 21 deletions(-)
> > 
> > You beat me to this again :)
> 
> Pardon the spam, was just showing it in context of the patchset I was
> developing.
> 
> > I have tested this patch, and it looks good, I'll send out my series
> > that uses it for a different subsystem as well.
> > 
> > I guess I can take this as a static tag for others to pull from for this
> > rc development cycle?
> 
> That works for me. Thanks Greg!

I've applied this to my testing branch right now, and if 0-day passes
everything (it's done so in other branches), I can create a static tag
for everyone to pull from.

thanks,

greg k-h

Re: [RFC PATCH 4/5] sysfs: Introduce a mechanism to hide static attribute_groups

[Greg KH]

On Tue, Jan 30, 2024 at 08:48:26AM -0800, Dan Williams wrote:
> Greg KH wrote:
> > On Tue, Jan 30, 2024 at 01:24:08AM -0800, Dan Williams wrote:
> > > Add a mechanism for named attribute_groups to hide their directory at
> > > sysfs_update_group() time, or otherwise skip emitting the group
> > > directory when the group is first registered. It piggybacks on
> > > is_visible() in a similar manner as SYSFS_PREALLOC, i.e. special flags
> > > in the upper bits of the returned mode. To use it, specify a symbol
> > > prefix to DEFINE_SYSFS_GROUP_VISIBLE(), and then pass that same prefix
> > > to SYSFS_GROUP_VISIBLE() when assigning the @is_visible() callback:
> > > 
> > > 	DEFINE_SYSFS_GROUP_VISIBLE($prefix)
> > > 
> > > 	struct attribute_group $prefix_group = {
> > > 		.name = $name,
> > > 		.is_visible = SYSFS_GROUP_VISIBLE($prefix),
> > > 	};
> > > 
> > > SYSFS_GROUP_VISIBLE() expects a definition of $prefix_group_visible()
> > > and $prefix_attr_visible(), where $prefix_group_visible() just returns
> > > true / false and $prefix_attr_visible() behaves as normal.
> > > 
> > > The motivation for this capability is to centralize PCI device
> > > authentication in the PCI core with a named sysfs group while keeping
> > > that group hidden for devices and platforms that do not meet the
> > > requirements. In a PCI topology, most devices will not support
> > > authentication, a small subset will support just PCI CMA (Component
> > > Measurement and Authentication), a smaller subset will support PCI CMA +
> > > PCIe IDE (Link Integrity and Encryption), and only next generation
> > > server hosts will start to include a platform TSM (TEE Security
> > > Manager).
> > > 
> > > Without this capability the alternatives are:
> > > 
> > > * Check if all attributes are invisible and if so, hide the directory.
> > >   Beyond trouble getting this to work [1], this is an ABI change for
> > >   scenarios if userspace happens to depend on group visibility absent any
> > >   attributes. I.e. this new capability avoids regression since it does
> > >   not retroactively apply to existing cases.
> > > 
> > > * Publish an empty /sys/bus/pci/devices/$pdev/tsm/ directory for all PCI
> > >   devices (i.e. for the case when TSM platform support is present, but
> > >   device support is absent). Unfortunate that this will be a vestigial
> > >   empty directory in the vast majority of cases.
> > > 
> > > * Reintroduce usage of runtime calls to sysfs_{create,remove}_group()
> > >   in the PCI core. Bjorn has already indicated that he does not want to
> > >   see any growth of pci_sysfs_init() [2].
> > > 
> > > * Drop the named group and simulate a directory by prefixing all
> > >   TSM-related attributes with "tsm_". Unfortunate to not use the naming
> > >   capability of a sysfs group as intended.
> > > 
> > > In comparison, there is a small potential for regression if for some
> > > reason an @is_visible() callback had dependencies on how many times it
> > > was called. Additionally, it is no longer an error to update a group
> > > that does not have its directory already present, and it is no longer a
> > > WARN() to remove a group that was never visible.
> > > 
> > > Link: https://lore.kernel.org/all/2024012321-envious-procedure-4a58@gregkh/ [1]
> > > Link: https://lore.kernel.org/linux-pci/20231019200110.GA1410324@bhelgaas/ [2]
> > > Signed-off-by: Dan Williams <dan.j.williams@intel.com>
> > > ---
> > >  fs/sysfs/group.c      |   45 ++++++++++++++++++++++++++++-------
> > >  include/linux/sysfs.h |   63 ++++++++++++++++++++++++++++++++++++++++---------
> > >  2 files changed, 87 insertions(+), 21 deletions(-)
> > 
> > You beat me to this again :)
> 
> Pardon the spam, was just showing it in context of the patchset I was
> developing.
> 
> > I have tested this patch, and it looks good, I'll send out my series
> > that uses it for a different subsystem as well.
> > 
> > I guess I can take this as a static tag for others to pull from for this
> > rc development cycle?
> 
> That works for me. Thanks Greg!

This is now here for anyone else to pull from, I've put it into my
driver-core-next branch as well:


The following changes since commit b401b621758e46812da61fa58a67c3fd8d91de0d:

  Linux 6.8-rc5 (2024-02-18 12:56:25 -0800)

are available in the Git repository at:

  git://git.kernel.org/pub/scm/linux/kernel/git/gregkh/driver-core.git tags/sysfs_hidden_attribute_groups-6.9-rc1

for you to fetch changes up to 70317fd24b419091aa0a6dc3ea3ec7bb50c37c32:

  sysfs: Introduce a mechanism to hide static attribute_groups (2024-02-20 10:20:21 +0100)

----------------------------------------------------------------
sysfs_hidden_attribute_groups persistent tag to pull from

Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

----------------------------------------------------------------
Dan Williams (1):
      sysfs: Introduce a mechanism to hide static attribute_groups

 fs/sysfs/group.c      | 45 ++++++++++++++++++++++++++++--------
 include/linux/sysfs.h | 63 +++++++++++++++++++++++++++++++++++++++++----------
 2 files changed, 87 insertions(+), 21 deletions(-)

Re: [RFC PATCH 5/5] PCI/TSM: Authenticate devices via platform TSM

[Zhi Wang]

On Tue, 30 Jan 2024 01:24:14 -0800
Dan Williams <dan.j.williams@intel.com> wrote:

> The PCIe 6.1 specification, section 11, introduces the Trusted
> Execution Environment (TEE) Device Interface Security Protocol
> (TDISP). This interface definition builds upon CMA, component
> measurement and authentication, and IDE, link integrity and data
> encryption. It adds support for establishing virtual functions within
> a device that can be assigned to a confidential VM such that the
> assigned device is enabled to access guest private memory protected
> by technologies like Intel TDX, AMD SEV-SNP, RISCV COVE, or ARM CCA.
> 
> The "TSM" (TEE Security Manager) is a concept in the TDISP
> specification of an agent that mediates between a device security
> manager (DSM) and system software in both a VMM and a VM. From a
> Linux perspective the TSM abstracts many of the details of TDISP,
> IDE, and CMA. Some of those details leak through at times, but for
> the most part TDISP is an internal implementation detail of the TSM.
> 
> Similar to the PCI core extensions to support CONFIG_PCI_CMA,
> CONFIG_PCI_TSM builds upon that to reuse the "authenticated" sysfs
> attribute, and add more properties + controls in a tsm/ subdirectory
> of the PCI device sysfs interface. Unlike CMA that can depend on a
> local to the PCI core implementation, PCI_TSM needs to be prepared
> for late loading of the platform TSM driver. Consider that the TSM
> driver may itself be a PCI driver. Userspace can depend on the common
> TSM device uevent to know when the PCI core has TSM services enabled.
> The PCI device tsm/ subdirectory is supplemented by the TSM device
> pci/ directory for platform global TSM properties + controls.
> 
> All vendor TSM implementations share the property of asking the VMM to
> perform DOE mailbox operations on behalf of the TSM. That common
> capability is centralized in PCI core code that invokes an ->exec()
> operation callback potentially multiple times to service a given
> request (struct pci_tsm_req). Future operations / verbs will be
> handled similarly with the "request + exec" model. For now, only
> "connect" and "disconnect" are implemented which at a minimum is
> expected to establish IDE for the link.
> 
> In addition to requests the low-level TSM implementation is notified
> of device arrival and departure events so that it can filter devices
> that the TSM is not prepared to support, or otherwise setup and
> teardown per-device context.
> 

Hey Dan,

1) What is the expectation of using the device connect and disconnect
in the guest-driven secure I/O enlightenment? In the last device
security meeting, you said the sysfs interface was mostly for higher
level software stacks, like virt-manager. I was wondering what would be
the picture looks like when coping these statement with the
guest-driven model. Are we expecting the device connect triggered by
QEMU when extracting the guest request from the secure channel in
this case?

2) How does the device-specific logic fit into the new TSM
services? E.g. when the TDISP connect is triggered by userspace, a
device needs to perform quirks before/after/inside the verbs, or a
device needs an interface to tell TSM service when it is able to
response to some verbs. Do you think we needs to have a set of
callbacks from the device side for the PCI TSM service to call?

Thanks,
Zhi.

> Cc: Wu Hao <hao.wu@intel.com>
> Cc: Yilun Xu <yilun.xu@intel.com>
> Cc: Lukas Wunner <lukas@wunner.de>
> Cc: Samuel Ortiz <sameo@rivosinc.com>
> Cc: Alexey Kardashevskiy <aik@amd.com>
> Cc: Bjorn Helgaas <bhelgaas@google.com>
> Signed-off-by: Dan Williams <dan.j.williams@intel.com>
> ---
>  Documentation/ABI/testing/sysfs-bus-pci   |   43 +++-
>  Documentation/ABI/testing/sysfs-class-tsm |   23 ++
>  drivers/pci/Kconfig                       |   15 +
>  drivers/pci/Makefile                      |    2 
>  drivers/pci/cma.c                         |    5 
>  drivers/pci/pci-sysfs.c                   |    3 
>  drivers/pci/pci.h                         |   14 +
>  drivers/pci/probe.c                       |    1 
>  drivers/pci/remove.c                      |    1 
>  drivers/pci/tsm.c                         |  346
> +++++++++++++++++++++++++++++ drivers/virt/coco/tsm/Makefile
>   |    1 drivers/virt/coco/tsm/class.c             |   22 +-
>  drivers/virt/coco/tsm/pci.c               |   83 +++++++
>  drivers/virt/coco/tsm/tsm.h               |   28 ++
>  include/linux/pci.h                       |    3 
>  include/linux/tsm.h                       |   77 ++++++
>  include/uapi/linux/pci_regs.h             |    3 
>  17 files changed, 662 insertions(+), 8 deletions(-)
>  create mode 100644 drivers/pci/tsm.c
>  create mode 100644 drivers/virt/coco/tsm/pci.c
>  create mode 100644 drivers/virt/coco/tsm/tsm.h
> 
> diff --git a/Documentation/ABI/testing/sysfs-bus-pci
> b/Documentation/ABI/testing/sysfs-bus-pci index
> 35b0e11fd0e6..0eef2128cf09 100644 ---
> a/Documentation/ABI/testing/sysfs-bus-pci +++
> b/Documentation/ABI/testing/sysfs-bus-pci @@ -508,11 +508,16 @@
> Description: This file contains "native" if the device authenticated
>  		successfully with CMA-SPDM (PCIe r6.1 sec 6.31). It
> contains "none" if the device failed authentication (and may thus be
> -		malicious).
> +		malicious). It transitions from "native" to "tsm"
> after
> +		successful connection to a tsm, see the "connect"
> attribute
> +		below.
>  
>  		Writing "native" to this file causes
> reauthentication with kernel-selected keys and the kernel's
> certificate chain.  That
> -		may be opportune after updating the .cma keyring.
> +		may be opportune after updating the .cma keyring.
> Note
> +		that once connected to a tsm this returns -EBUSY to
> attempts to
> +		write "native", i.e. first disconnect from the tsm
> to retrigger
> +		native authentication.
>  
>  		The file is not visible if authentication is
> unsupported by the device.
> @@ -529,3 +534,37 @@ Description:
>  		The reason why authentication support could not be
> determined is apparent from "dmesg".  To probe for authentication
> support again, exercise the "remove" and "rescan" attributes.
> +
> +What:		/sys/bus/pci/devices/.../tsm/
> +Date:		January 2024
> +Contact:	linux-coco@lists.linux.dev
> +Description:
> +		This directory only appears if a device supports CMA
> and IDE,
> +		and only after a TSM driver has loaded and accepted
> / setup this
> +		PCI device. Similar to the 'authenticated'
> attribute, trigger
> +		"remove" and "rescan" to retry the initialization.
> See
> +		Documentation/ABI/testing/sysfs-class-tsm for
> enumerating the
> +		platform's TSM capabilities.
> +
> +What:		/sys/bus/pci/devices/.../tsm/connect
> +Date:		January 2024
> +Contact:	linux-coco@lists.linux.dev
> +Description:
> +		(RW) Writing "1" to this file triggers the TSM to
> establish a
> +		secure connection with the device. This typically
> includes an
> +		SPDM (DMTF Security Protocols and Data Models)
> session over PCIe
> +		DOE (Data Object Exchange) and PCIe IDE (Integrity
> and Data
> +		Encryption) establishment. For TSMs and devices that
> support
> +		both modes of IDE ("link" and "selective") the
> "connect_mode"
> +		attribute selects the mode.
> +
> +What:		/sys/bus/pci/devices/.../tsm/connect_mode
> +Date:		January 2024
> +Contact:	linux-coco@lists.linux.dev
> +Description:
> +		(RO) Returns the available connection modes
> optionally with
> +		brackets around the currently active mode if the
> device is
> +		connected. For example it may show "link selective"
> for a
> +		disconnected device, "link [selective]" for a
> selective
> +		connected device, and it may hide a mode that is not
> supported
> +		by the device or TSM.
> diff --git a/Documentation/ABI/testing/sysfs-class-tsm
> b/Documentation/ABI/testing/sysfs-class-tsm index
> 304b50b53e65..77957882738a 100644 ---
> a/Documentation/ABI/testing/sysfs-class-tsm +++
> b/Documentation/ABI/testing/sysfs-class-tsm @@ -10,3 +10,26 @@
> Description: For software TSMs instantiated by a software module,
> @host is a directory with attributes for that TSM, and those
> attributes are documented below.
> +
> +
> +What:		/sys/class/tsm/tsm0/pci/link_capable
> +Date:		January, 2024
> +Contact:	linux-coco@lists.linux.dev
> +Description:
> +		(RO) When present this returns "1\n" to indicate
> that the TSM
> +		supports establishing Link IDE with a given
> root-port attached
> +		device. See "tsm/connect_mode" in
> +		Documentation/ABI/testing/sysfs-bus-pci
> +
> +
> +What:		/sys/class/tsm/tsm0/pci/selective_streams
> +Date:		January, 2024
> +Contact:	linux-coco@lists.linux.dev
> +Description:
> +		(RO) When present this returns the number of
> currently available
> +		selective IDE streams available to the TSM. When a
> stream id is
> +		allocated this number is decremented and a link to
> the PCI
> +		device(s) consuming the stream(s) appears alonside
> this
> +		attribute in the /sys/class/tsm/tsm0/pci/ directory.
> See
> +		"tsm/connect" and "tsm/connect_mode" in
> +		Documentation/ABI/testing/sysfs-bus-pci.
> diff --git a/drivers/pci/Kconfig b/drivers/pci/Kconfig
> index a5c3cadddd6f..11d788038d19 100644
> --- a/drivers/pci/Kconfig
> +++ b/drivers/pci/Kconfig
> @@ -129,6 +129,21 @@ config PCI_CMA
>  	  A PCI DOE mailbox is used as transport for DMTF SPDM based
>  	  authentication, measurement and secure channel
> establishment. 
> +config PCI_TSM
> +	bool "TEE Security Manager for Device Security"
> +	depends on PCI_CMA
> +	depends on TSM
> +	help
> +	  The TEE (Trusted Execution Environment) Device Interface
> +	  Security Protocol (TDISP) defines a "TSM" as a platform
> agent
> +	  that manages device authentication, link encryption, link
> +	  integrity protection, and assignment of PCI device
> functions
> +	  (virtual or physical) to confidential computing VMs that
> can
> +	  access (DMA) guest private memory.
> +
> +	  Say Y to enable the PCI subsystem to enable the IDE and
> +	  TDISP capabilities of devices via TSM semantics.
> +
>  config PCI_DOE
>  	bool
>  
> diff --git a/drivers/pci/Makefile b/drivers/pci/Makefile
> index cc8b5d1d15b9..c4117d67ea83 100644
> --- a/drivers/pci/Makefile
> +++ b/drivers/pci/Makefile
> @@ -38,6 +38,8 @@ obj-$(CONFIG_PCI_CMA)		+= cma.o
> cma.asn1.o $(obj)/cma.o:			$(obj)/cma.asn1.h
>  $(obj)/cma.asn1.o:		$(obj)/cma.asn1.c $(obj)/cma.asn1.h
>  
> +obj-$(CONFIG_PCI_TSM)		+= tsm.o
> +
>  # Endpoint library must be initialized before its users
>  obj-$(CONFIG_PCI_ENDPOINT)	+= endpoint/
>  
> diff --git a/drivers/pci/cma.c b/drivers/pci/cma.c
> index be7d2bb21b4c..5a69e9919589 100644
> --- a/drivers/pci/cma.c
> +++ b/drivers/pci/cma.c
> @@ -39,6 +39,9 @@ static ssize_t authenticated_store(struct device
> *dev, if (!sysfs_streq(buf, "native"))
>  		return -EINVAL;
>  
> +	if (pci_tsm_authenticated(pdev))
> +		return -EBUSY;
> +
>  	rc = pci_cma_reauthenticate(pdev);
>  	if (rc)
>  		return rc;
> @@ -55,6 +58,8 @@ static ssize_t authenticated_show(struct device
> *dev, (pdev->cma_init_failed || pdev->doe_init_failed))
>  		return -ENOTTY;
>  
> +	if (pci_tsm_authenticated(pdev))
> +		return sysfs_emit(buf, "tsm\n");
>  	if (spdm_authenticated(pdev->spdm_state))
>  		return sysfs_emit(buf, "native\n");
>  	return sysfs_emit(buf, "none\n");
> diff --git a/drivers/pci/pci-sysfs.c b/drivers/pci/pci-sysfs.c
> index 368c4f71cc55..4327f8c2e6b5 100644
> --- a/drivers/pci/pci-sysfs.c
> +++ b/drivers/pci/pci-sysfs.c
> @@ -1654,6 +1654,9 @@ const struct attribute_group
> *pci_dev_attr_groups[] = { #endif
>  #ifdef CONFIG_PCI_CMA
>  	&pci_cma_attr_group,
> +#endif
> +#ifdef CONFIG_PCI_TSM
> +	&pci_tsm_attr_group,
>  #endif
>  	NULL,
>  };
> diff --git a/drivers/pci/pci.h b/drivers/pci/pci.h
> index 2b7d8d0b2e21..daa20866bc90 100644
> --- a/drivers/pci/pci.h
> +++ b/drivers/pci/pci.h
> @@ -350,6 +350,20 @@ static inline int pci_cma_reauthenticate(struct
> pci_dev *pdev) }
>  #endif
>  
> +#ifdef CONFIG_PCI_TSM
> +void pci_tsm_init(struct pci_dev *pdev);
> +void pci_tsm_destroy(struct pci_dev *pdev);
> +extern const struct attribute_group pci_tsm_attr_group;
> +bool pci_tsm_authenticated(struct pci_dev *pdev);
> +#else
> +static inline void pci_tsm_init(struct pci_dev *pdev) { }
> +static inline void pci_tsm_destroy(struct pci_dev *pdev) { }
> +static inline bool pci_tsm_authenticated(struct pci_dev *pdev)
> +{
> +	return false;
> +}
> +#endif
> +
>  /**
>   * pci_dev_set_io_state - Set the new error state if possible.
>   *
> diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
> index 6b09c962c0b8..f60d6c3c8c48 100644
> --- a/drivers/pci/probe.c
> +++ b/drivers/pci/probe.c
> @@ -2542,6 +2542,7 @@ static void pci_init_capabilities(struct
> pci_dev *dev) pci_rcec_init(dev);		/* Root Complex
> Event Collector */ pci_doe_init(dev);		/* Data Object
> Exchange */ pci_cma_init(dev);		/* Component
> Measurement & Auth */
> +	pci_tsm_init(dev);		/* TEE Security Manager
> connection */ 
>  	pcie_report_downtraining(dev);
>  	pci_init_reset_methods(dev);
> diff --git a/drivers/pci/remove.c b/drivers/pci/remove.c
> index f009ac578997..228fa6ccf911 100644
> --- a/drivers/pci/remove.c
> +++ b/drivers/pci/remove.c
> @@ -39,6 +39,7 @@ static void pci_destroy_dev(struct pci_dev *dev)
>  	list_del(&dev->bus_list);
>  	up_write(&pci_bus_sem);
>  
> +	pci_tsm_destroy(dev);
>  	pci_cma_destroy(dev);
>  	pci_doe_destroy(dev);
>  	pcie_aspm_exit_link_state(dev);
> diff --git a/drivers/pci/tsm.c b/drivers/pci/tsm.c
> new file mode 100644
> index 000000000000..f74de0ee49a0
> --- /dev/null
> +++ b/drivers/pci/tsm.c
> @@ -0,0 +1,346 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * TEE Security Manager for the TEE Device Interface Security
> Protocol
> + * (TDISP, PCIe r6.1 sec 11)
> + *
> + * Copyright(c) 2024 Intel Corporation. All rights reserved.
> + */
> +
> +#define dev_fmt(fmt) "TSM: " fmt
> +
> +#include <linux/pci.h>
> +#include <linux/tsm.h>
> +#include <linux/sysfs.h>
> +#include <linux/xarray.h>
> +#include "pci.h"
> +
> +/* collect tsm capable devices to rendezvous with the tsm driver */
> +static DEFINE_XARRAY(pci_tsm_devs);
> +
> +/*
> + * Provide a read/write lock against the init / exit of pdev tsm
> + * capabilities and arrival/departure of a tsm instance
> + */
> +static DECLARE_RWSEM(pci_tsm_rwsem);
> +static const struct tsm_pci_ops *tsm_ops;
> +
> +void generic_pci_tsm_req_free(struct pci_tsm_req *req)
> +{
> +	kfree(req);
> +}
> +EXPORT_SYMBOL_GPL(generic_pci_tsm_req_free);
> +
> +struct pci_tsm_req *generic_pci_tsm_req_alloc(struct pci_dev *pdev,
> enum pci_tsm_op op) +{
> +	struct pci_tsm_req *req = kzalloc(sizeof(*req), GFP_KERNEL);
> +
> +	if (!req)
> +		return NULL;
> +	req->op = op;
> +	return req;
> +}
> +EXPORT_SYMBOL_GPL(generic_pci_tsm_req_alloc);
> +
> +DEFINE_FREE(req_free, struct pci_tsm_req *, if (_T)
> tsm_ops->req_free(_T)) +
> +static int pci_tsm_disconnect(struct pci_dev *pdev)
> +{
> +	struct pci_tsm_req *req __free(req_free) = NULL;
> +
> +	/* opportunistic state checks to skip allocating a request */
> +	if (pdev->tsm->state < PCI_TSM_CONNECT)
> +		return 0;
> +
> +	req = tsm_ops->req_alloc(pdev, PCI_TSM_OP_DISCONNECT);
> +	if (!req)
> +		return -ENOMEM;
> +
> +	scoped_cond_guard(mutex_intr, return -EINTR, tsm_ops->lock) {
> +		enum pci_tsm_op_status status;
> +
> +		/* revalidate state */
> +		if (pdev->tsm->state < PCI_TSM_CONNECT)
> +			return 0;
> +		if (pdev->tsm->state < PCI_TSM_INIT)
> +			return -ENXIO;
> +
> +		do {
> +			status = tsm_ops->exec(pdev, req);
> +			req->seq++;
> +			/* TODO: marshal SPDM request */
> +		} while (status == PCI_TSM_SPDM_REQ);
> +
> +		if (status == PCI_TSM_FAIL)
> +			return -EIO;
> +		pdev->tsm->state = PCI_TSM_INIT;
> +	}
> +	return 0;
> +}
> +
> +static int pci_tsm_connect(struct pci_dev *pdev)
> +{
> +	struct pci_tsm_req *req __free(req_free) = NULL;
> +
> +	/* opportunistic state checks to skip allocating a request */
> +	if (pdev->tsm->state >= PCI_TSM_CONNECT)
> +		return 0;
> +
> +	req = tsm_ops->req_alloc(pdev, PCI_TSM_OP_CONNECT);
> +	if (!req)
> +		return -ENOMEM;
> +
> +	scoped_cond_guard(mutex_intr, return -EINTR, tsm_ops->lock) {
> +		enum pci_tsm_op_status status;
> +
> +		/* revalidate state */
> +		if (pdev->tsm->state >= PCI_TSM_CONNECT)
> +			return 0;
> +		if (pdev->tsm->state < PCI_TSM_INIT)
> +			return -ENXIO;
> +
> +		do {
> +			status = tsm_ops->exec(pdev, req);
> +			req->seq++;
> +		} while (status == PCI_TSM_SPDM_REQ);
> +
> +		if (status == PCI_TSM_FAIL)
> +			return -EIO;
> +		pdev->tsm->state = PCI_TSM_CONNECT;
> +	}
> +	return 0;
> +}
> +
> +static ssize_t connect_store(struct device *dev, struct
> device_attribute *attr,
> +			     const char *buf, size_t len)
> +{
> +	bool connect;
> +	int rc = kstrtobool(buf, &connect);
> +	struct pci_dev *pdev = to_pci_dev(dev);
> +
> +	if (rc)
> +		return rc;
> +
> +	if (connect) {
> +		if (!spdm_authenticated(pdev->spdm_state)) {
> +			pci_dbg(pdev, "SPDM authentication
> pre-requisite not met.\n");
> +			return -ENXIO;
> +		}
> +		rc = pci_tsm_connect(pdev);
> +		if (rc)
> +			return rc;
> +		return len;
> +	}
> +
> +	rc = pci_tsm_disconnect(pdev);
> +	if (rc)
> +		return rc;
> +	return len;
> +}
> +
> +static ssize_t connect_show(struct device *dev, struct
> device_attribute *attr,
> +			    char *buf)
> +{
> +	struct pci_dev *pdev = to_pci_dev(dev);
> +
> +	return sysfs_emit(buf, "%d\n", pdev->tsm->state >=
> PCI_TSM_CONNECT); +}
> +static DEVICE_ATTR_RW(connect);
> +
> +static const char *const pci_tsm_modes[] = {
> +	[PCI_TSM_MODE_LINK] = "link",
> +	[PCI_TSM_MODE_SELECTIVE] = "selective",
> +};
> +
> +static ssize_t connect_mode_store(struct device *dev,
> +				  struct device_attribute *attr,
> +				  const char *buf, size_t len)
> +{
> +	struct pci_dev *pdev = to_pci_dev(dev);
> +	int i;
> +
> +	guard(mutex)(tsm_ops->lock);
> +	if (pdev->tsm->state >= PCI_TSM_CONNECT)
> +		return -EBUSY;
> +	for (i = 0; i < ARRAY_SIZE(pci_tsm_modes); i++)
> +		if (sysfs_streq(buf, pci_tsm_modes[i]))
> +			break;
> +	if (i == PCI_TSM_MODE_LINK) {
> +		if (pdev->tsm->link_capable)
> +			pdev->tsm->mode = PCI_TSM_MODE_LINK;
> +		return -EOPNOTSUPP;
> +	} else if (i == PCI_TSM_MODE_SELECTIVE) {
> +		if (pdev->tsm->selective_capable)
> +			pdev->tsm->mode = PCI_TSM_MODE_SELECTIVE;
> +		return -EOPNOTSUPP;
> +	} else
> +		return -EINVAL;
> +	return len;
> +}
> +
> +static ssize_t connect_mode_show(struct device *dev,
> +				 struct device_attribute *attr, char
> *buf) +{
> +	struct pci_dev *pdev = to_pci_dev(dev);
> +	ssize_t count = 0;
> +	int i;
> +
> +	for (i = 0; i < ARRAY_SIZE(pci_tsm_modes); i++) {
> +		if (i == PCI_TSM_MODE_LINK) {
> +			if (!pdev->tsm->link_capable)
> +				continue;
> +		} else if (i == PCI_TSM_MODE_SELECTIVE) {
> +			if (!pdev->tsm->selective_capable)
> +				continue;
> +		}
> +
> +		if (i == pdev->tsm->mode)
> +			count += sysfs_emit_at(buf, count, "[%s] ",
> +					       pci_tsm_modes[i]);
> +		else
> +			count += sysfs_emit_at(buf, count, "%s ",
> +					       pci_tsm_modes[i]);
> +	}
> +
> +	if (count)
> +		buf[count - 1] = '\n';
> +
> +	return count;
> +}
> +static DEVICE_ATTR_RW(connect_mode);
> +
> +static umode_t pci_tsm_attr_visible(struct kobject *kobj, struct
> attribute *a, int n) +{
> +	struct device *dev = kobj_to_dev(kobj);
> +	struct pci_dev *pdev = to_pci_dev(dev);
> +
> +	if (a == &dev_attr_connect_mode.attr) {
> +		if (pdev->tsm->link_capable ||
> pdev->tsm->selective_capable)
> +			return a->mode;
> +		return 0;
> +	}
> +
> +	return a->mode;
> +}
> +
> +static bool pci_tsm_group_visible(struct kobject *kobj)
> +{
> +	struct device *dev = kobj_to_dev(kobj);
> +	struct pci_dev *pdev = to_pci_dev(dev);
> +
> +	if (pdev->tsm && pdev->tsm->state > PCI_TSM_IDLE)
> +		return true;
> +	return false;
> +}
> +DEFINE_SYSFS_GROUP_VISIBLE(pci_tsm);
> +
> +static struct attribute *pci_tsm_attrs[] = {
> +	&dev_attr_connect.attr,
> +	&dev_attr_connect_mode.attr,
> +	NULL,
> +};
> +
> +const struct attribute_group pci_tsm_attr_group = {
> +	.name = "tsm",
> +	.attrs = pci_tsm_attrs,
> +	.is_visible = SYSFS_GROUP_VISIBLE(pci_tsm),
> +};
> +
> +static int pci_tsm_add(struct pci_dev *pdev)
> +{
> +	lockdep_assert_held(&pci_tsm_rwsem);
> +	if (!tsm_ops)
> +		return 0;
> +	scoped_guard(mutex, tsm_ops->lock) {
> +		if (pdev->tsm->state < PCI_TSM_INIT) {
> +			int rc = tsm_ops->add(pdev);
> +
> +			if (rc)
> +				return rc;
> +		}
> +		pdev->tsm->state = PCI_TSM_INIT;
> +	}
> +	return sysfs_update_group(&pdev->dev.kobj,
> &pci_tsm_attr_group); +}
> +
> +static void pci_tsm_del(struct pci_dev *pdev)
> +{
> +	lockdep_assert_held(&pci_tsm_rwsem);
> +	/* shutdown sysfs operations before tsm delete */
> +	pdev->tsm->state = PCI_TSM_IDLE;
> +	sysfs_update_group(&pdev->dev.kobj, &pci_tsm_attr_group);
> +	guard(mutex)(tsm_ops->lock);
> +	tsm_ops->del(pdev);
> +}
> +
> +int pci_tsm_register(const struct tsm_pci_ops *ops)
> +{
> +	struct pci_dev *pdev;
> +	unsigned long index;
> +
> +	guard(rwsem_write)(&pci_tsm_rwsem);
> +	if (tsm_ops)
> +		return -EBUSY;
> +	tsm_ops = ops;
> +	xa_for_each(&pci_tsm_devs, index, pdev)
> +		pci_tsm_add(pdev);
> +	return 0;
> +}
> +EXPORT_SYMBOL_GPL(pci_tsm_register);
> +
> +void pci_tsm_unregister(const struct tsm_pci_ops *ops)
> +{
> +	struct pci_dev *pdev;
> +	unsigned long index;
> +
> +	guard(rwsem_write)(&pci_tsm_rwsem);
> +	if (ops != tsm_ops)
> +		return;
> +	xa_for_each(&pci_tsm_devs, index, pdev)
> +		pci_tsm_del(pdev);
> +	tsm_ops = NULL;
> +}
> +EXPORT_SYMBOL_GPL(pci_tsm_unregister);
> +
> +void pci_tsm_init(struct pci_dev *pdev)
> +{
> +	u16 ide_cap;
> +	int rc;
> +
> +	if (!pdev->cma_capable)
> +		return;
> +
> +	ide_cap = pci_find_ext_capability(pdev, PCI_EXT_CAP_ID_IDE);
> +	if (!ide_cap)
> +		return;
> +
> +	struct pci_tsm *tsm __free(kfree) = kzalloc(sizeof(*tsm),
> GFP_KERNEL);
> +	if (!tsm)
> +		return;
> +
> +	tsm->ide_cap = ide_cap;
> +
> +	rc = xa_insert(&pci_tsm_devs, (unsigned long)pdev, pdev,
> GFP_KERNEL);
> +	if (rc) {
> +		pci_dbg(pdev, "failed to register tsm capable
> device\n");
> +		return;
> +	}
> +
> +	guard(rwsem_write)(&pci_tsm_rwsem);
> +	pdev->tsm = no_free_ptr(tsm);
> +	pci_tsm_add(pdev);
> +}
> +
> +void pci_tsm_destroy(struct pci_dev *pdev)
> +{
> +	guard(rwsem_write)(&pci_tsm_rwsem);
> +	pci_tsm_del(pdev);
> +	xa_erase(&pci_tsm_devs, (unsigned long)pdev);
> +	kfree(pdev->tsm);
> +	pdev->tsm = NULL;
> +}
> +
> +bool pci_tsm_authenticated(struct pci_dev *pdev)
> +{
> +	guard(rwsem_read)(&pci_tsm_rwsem);
> +	return pdev->tsm && pdev->tsm->state >= PCI_TSM_CONNECT;
> +}
> diff --git a/drivers/virt/coco/tsm/Makefile
> b/drivers/virt/coco/tsm/Makefile index f7561169faed..a4f0d07d7d97
> 100644 --- a/drivers/virt/coco/tsm/Makefile
> +++ b/drivers/virt/coco/tsm/Makefile
> @@ -7,3 +7,4 @@ tsm_reports-y := reports.o
>  
>  obj-$(CONFIG_TSM) += tsm.o
>  tsm-y := class.o
> +tsm-$(CONFIG_PCI_TSM) += pci.o
> diff --git a/drivers/virt/coco/tsm/class.c
> b/drivers/virt/coco/tsm/class.c index a569fa6b09eb..a459e51c0892
> 100644 --- a/drivers/virt/coco/tsm/class.c
> +++ b/drivers/virt/coco/tsm/class.c
> @@ -8,13 +8,11 @@
>  #include <linux/device.h>
>  #include <linux/module.h>
>  #include <linux/cleanup.h>
> +#include "tsm.h"
>  
>  static DECLARE_RWSEM(tsm_core_rwsem);
> -struct class *tsm_class;
> -struct tsm_subsys {
> -	struct device dev;
> -	const struct tsm_info *info;
> -} *tsm_subsys;
> +static struct class *tsm_class;
> +static struct tsm_subsys *tsm_subsys;
>  
>  int tsm_register(const struct tsm_info *info)
>  {
> @@ -52,6 +50,10 @@ int tsm_register(const struct tsm_info *info)
>  	dev = NULL;
>  	tsm_subsys = subsys;
>  
> +	rc = tsm_pci_init(info);
> +	if (rc)
> +		pr_err("PCI initialization failure: %d\n", rc);
> +
>  	return 0;
>  }
>  EXPORT_SYMBOL_GPL(tsm_register);
> @@ -65,6 +67,8 @@ void tsm_unregister(const struct tsm_info *info)
>  		return;
>  	}
>  
> +	tsm_pci_destroy(info);
> +
>  	if (info->host)
>  		sysfs_remove_link(&tsm_subsys->dev.kobj, "host");
>  	device_unregister(&tsm_subsys->dev);
> @@ -79,6 +83,13 @@ static void tsm_release(struct device *dev)
>  	kfree(subsys);
>  }
>  
> +static const struct attribute_group *tsm_attr_groups[] = {
> +#ifdef CONFIG_PCI_TSM
> +	&tsm_pci_attr_group,
> +#endif
> +	NULL,
> +};
> +
>  static int __init tsm_init(void)
>  {
>  	tsm_class = class_create("tsm");
> @@ -86,6 +97,7 @@ static int __init tsm_init(void)
>  		return PTR_ERR(tsm_class);
>  
>  	tsm_class->dev_release = tsm_release;
> +	tsm_class->dev_groups = tsm_attr_groups;
>  	return 0;
>  }
>  module_init(tsm_init)
> diff --git a/drivers/virt/coco/tsm/pci.c b/drivers/virt/coco/tsm/pci.c
> new file mode 100644
> index 000000000000..b3684ad7114f
> --- /dev/null
> +++ b/drivers/virt/coco/tsm/pci.c
> @@ -0,0 +1,83 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/* Copyright(c) 2024 Intel Corporation. All rights reserved. */
> +
> +#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
> +
> +#include <linux/tsm.h>
> +#include <linux/device.h>
> +#include "tsm.h"
> +
> +static ssize_t link_capable_show(struct device *dev,
> +				 struct device_attribute *attr, char
> *buf) +{
> +	struct tsm_subsys *subsys = container_of(dev,
> typeof(*subsys), dev); +
> +	return sysfs_emit(buf, "%u\n",
> subsys->info->link_stream_capable); +}
> +static DEVICE_ATTR_RO(link_capable);
> +
> +static ssize_t selective_streams_show(struct device *dev,
> +				      struct device_attribute *attr,
> char *buf) +{
> +	struct tsm_subsys *subsys = container_of(dev,
> typeof(*subsys), dev); +
> +	return sysfs_emit(buf, "%u\n",
> subsys->info->nr_selective_streams); +}
> +static DEVICE_ATTR_RO(selective_streams);
> +
> +static umode_t tsm_pci_attr_visible(struct kobject *kobj, struct
> attribute *a, int n) +{
> +	struct device *dev = kobj_to_dev(kobj);
> +	struct tsm_subsys *subsys = container_of(dev,
> typeof(*subsys), dev);
> +	const struct tsm_info *info = subsys->info;
> +
> +	if (a == &dev_attr_link_capable.attr) {
> +		if (info->link_stream_capable)
> +			return a->mode;
> +		return 0;
> +	}
> +
> +	if (a == &dev_attr_selective_streams.attr) {
> +		if (info->nr_selective_streams)
> +			return a->mode;
> +		return 0;
> +	}
> +
> +	return a->mode;
> +}
> +
> +static bool tsm_pci_group_visible(struct kobject *kobj)
> +{
> +	struct device *dev = kobj_to_dev(kobj);
> +	struct tsm_subsys *subsys = container_of(dev,
> typeof(*subsys), dev); +
> +	if (subsys->info->pci_ops)
> +		return true;
> +	return false;
> +}
> +DEFINE_SYSFS_GROUP_VISIBLE(tsm_pci);
> +
> +static struct attribute *tsm_pci_attrs[] = {
> +	&dev_attr_link_capable.attr,
> +	&dev_attr_selective_streams.attr,
> +	NULL,
> +};
> +
> +const struct attribute_group tsm_pci_attr_group = {
> +	.name = "pci",
> +	.attrs = tsm_pci_attrs,
> +	.is_visible = SYSFS_GROUP_VISIBLE(tsm_pci),
> +};
> +
> +int tsm_pci_init(const struct tsm_info *info)
> +{
> +	if (!info->pci_ops)
> +		return 0;
> +
> +	return pci_tsm_register(info->pci_ops);
> +}
> +
> +void tsm_pci_destroy(const struct tsm_info *info)
> +{
> +	pci_tsm_unregister(info->pci_ops);
> +}
> diff --git a/drivers/virt/coco/tsm/tsm.h b/drivers/virt/coco/tsm/tsm.h
> new file mode 100644
> index 000000000000..407c388a109b
> --- /dev/null
> +++ b/drivers/virt/coco/tsm/tsm.h
> @@ -0,0 +1,28 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +#ifndef __TSM_CORE_H
> +#define __TSM_CORE_H
> +
> +#include <linux/device.h>
> +
> +struct tsm_info;
> +struct tsm_subsys {
> +	struct device dev;
> +	const struct tsm_info *info;
> +};
> +
> +#ifdef CONFIG_PCI_TSM
> +int tsm_pci_init(const struct tsm_info *info);
> +void tsm_pci_destroy(const struct tsm_info *info);
> +extern const struct attribute_group tsm_pci_attr_group;
> +#else
> +static inline int tsm_pci_init(const struct tsm_info *info)
> +{
> +	return 0;
> +}
> +static inline void tsm_pci_destroy(const struct tsm_info *info)
> +{
> +}
> +#endif
> +
> +#endif /* TSM_CORE_H */
> +
> diff --git a/include/linux/pci.h b/include/linux/pci.h
> index 4a04ce7685e7..132962b21e04 100644
> --- a/include/linux/pci.h
> +++ b/include/linux/pci.h
> @@ -522,6 +522,9 @@ struct pci_dev {
>  	struct spdm_state *spdm_state;	/* Security Protocol
> and Data Model */ unsigned int	cma_capable:1;	/*
> Authentication supported */ unsigned int	cma_init_failed:1;
> +#endif
> +#ifdef CONFIG_PCI_TSM
> +	struct pci_tsm *tsm;		/* TSM operation state */
>  #endif
>  	u16		acs_cap;	/* ACS Capability offset
> */ phys_addr_t	rom;		/* Physical address if not
> from BAR */ diff --git a/include/linux/tsm.h b/include/linux/tsm.h
> index 8cb8a661ba41..f5dbdfa65d8d 100644
> --- a/include/linux/tsm.h
> +++ b/include/linux/tsm.h
> @@ -4,11 +4,15 @@
>  
>  #include <linux/sizes.h>
>  #include <linux/types.h>
> +#include <linux/mutex.h>
>  
>  struct tsm_info {
>  	const char *name;
>  	struct device *host;
>  	const struct attribute_group **groups;
> +	const struct tsm_pci_ops *pci_ops;
> +	unsigned int nr_selective_streams;
> +	unsigned int link_stream_capable:1;
>  };
>  
>  #define TSM_REPORT_INBLOB_MAX 64
> @@ -74,4 +78,77 @@ int tsm_report_register(const struct
> tsm_report_ops *ops, void *priv, int tsm_report_unregister(const
> struct tsm_report_ops *ops); int tsm_register(const struct tsm_info
> *info); void tsm_unregister(const struct tsm_info *info);
> +
> +enum pci_tsm_op_status {
> +	PCI_TSM_FAIL = -1,
> +	PCI_TSM_OK,
> +	PCI_TSM_SPDM_REQ,
> +};
> +
> +enum pci_tsm_op {
> +	PCI_TSM_OP_CONNECT,
> +	PCI_TSM_OP_DISCONNECT,
> +};
> +
> +struct pci_tsm_req {
> +	enum pci_tsm_op op;
> +	unsigned int seq;
> +};
> +
> +struct pci_dev;
> +/**
> + * struct tsm_pci_ops - Low-level TSM-exported interface to the PCI
> core
> + * @add: accept device for tsm operation, locked
> + * @del: teardown tsm context for @pdev, locked
> + * @req_alloc: setup context for given operation, unlocked
> + * @req_free: teardown context for given request, unlocked
> + * @exec: run @req, may be invoked multiple times per @req, locked
> + * @lock: tsm work is one device and one op at a time
> + */
> +struct tsm_pci_ops {
> +	int (*add)(struct pci_dev *pdev);
> +	void (*del)(struct pci_dev *pdev);
> +	struct pci_tsm_req *(*req_alloc)(struct pci_dev *pdev,
> +					 enum pci_tsm_op op);
> +	struct pci_tsm_req *(*req_free)(struct pci_tsm_req *req);
> +	enum pci_tsm_op_status (*exec)(struct pci_dev *pdev,
> +				       struct pci_tsm_req *req);
> +	struct mutex *lock;
> +};
> +
> +enum pci_tsm_state {
> +	PCI_TSM_IDLE,
> +	PCI_TSM_INIT,
> +	PCI_TSM_CONNECT,
> +};
> +
> +enum pci_tsm_mode {
> +	PCI_TSM_MODE_LINK,
> +	PCI_TSM_MODE_SELECTIVE,
> +};
> +
> +struct pci_tsm {
> +	enum pci_tsm_state state;
> +	enum pci_tsm_mode mode;
> +	u16 ide_cap;
> +	unsigned int link_capable:1;
> +	unsigned int selective_capable:1;
> +	void *tsm_data;
> +};
> +
> +#ifdef CONFIG_PCI_TSM
> +int pci_tsm_register(const struct tsm_pci_ops *ops);
> +void pci_tsm_unregister(const struct tsm_pci_ops *ops);
> +void generic_pci_tsm_req_free(struct pci_tsm_req *req);
> +struct pci_tsm_req *generic_pci_tsm_req_alloc(struct pci_dev *pdev,
> +					      enum pci_tsm_op op);
> +#else
> +static inline int pci_tsm_register(const struct tsm_pci_ops *ops)
> +{
> +	return 0;
> +}
> +static inline void pci_tsm_unregister(const struct tsm_pci_ops *ops)
> +{
> +}
> +#endif
>  #endif /* __TSM_H */
> diff --git a/include/uapi/linux/pci_regs.h
> b/include/uapi/linux/pci_regs.h index a39193213ff2..1219d50f8e89
> 100644 --- a/include/uapi/linux/pci_regs.h
> +++ b/include/uapi/linux/pci_regs.h
> @@ -742,7 +742,8 @@
>  #define PCI_EXT_CAP_ID_PL_16GT	0x26	/* Physical Layer
> 16.0 GT/s */ #define PCI_EXT_CAP_ID_PL_32GT  0x2A    /* Physical
> Layer 32.0 GT/s */ #define PCI_EXT_CAP_ID_DOE	0x2E	/*
> Data Object Exchange */ -#define PCI_EXT_CAP_ID_MAX
> PCI_EXT_CAP_ID_DOE +#define PCI_EXT_CAP_ID_IDE	0x30	/*
> Integrity and Data Encryption */ +#define PCI_EXT_CAP_ID_MAX
> PCI_EXT_CAP_ID_IDE 
>  #define PCI_EXT_CAP_DSN_SIZEOF	12
>  #define PCI_EXT_CAP_MCAST_ENDPOINT_SIZEOF 40
> 
> 

Re: [RFC PATCH 2/5] coco/tsm: Establish a new coco/tsm subdirectory

[Dan Williams]

Kuppuswamy Sathyanarayanan wrote:
> 
> On 1/30/24 1:23 AM, Dan Williams wrote:
> > In preparation for new + common TSM infrastructure, establish
> > drivers/virt/coco/tsm/. The tsm.ko module is renamed to tsm_reports.ko,
> > and some of its symbols moved to the "tsm_report_" namespace to separate
> > it from more generic "tsm" objects / symbols. The old tsm.ko module was
> > only ever demand loaded by kernel internal dependencies, so it should
> > not affect existing userspace module install scripts.
> 
> Since host related common code is also going be here, do you
> want push reports.c under tsm/guest dir?

Maybe... we can always cross that bridge later if it turns out to be
confusing that reports.c is only for guests.

For tsm.c it may end being used for both guest and host so it is not
clear that separation buys us anything yet.

Re: [RFC PATCH 3/5] coco/tsm: Introduce a shared class device for TSMs

[Dan Williams]

Alexey Kardashevskiy wrote:
> On 30/1/24 20:24, Dan Williams wrote:
> > A "tsm" is a platform component that provides an API for securely
> > provisioning resources for a confidential guest (TVM) to consume. "TSM"
> > also happens to be the acronym the PCI specification uses to define the
> > platform agent that carries out device-security operations. That
> > platform capability is commonly called TEE I/O. It is this arrival of
> > TEE I/O platforms that requires the "tsm" concept to grow from a
> > low-level arch-specific detail of TVM instantiation, to a frontend
> > interface to mediate device setup and interact with general purpose
> > kernel subsystems outside of arch/ like the PCI core.
> > 
> > Provide a virtual (as in /sys/devices/virtual) class device interface to
> > front all of the aspects of a TSM and TEE I/O that are
> > cross-architecture common. This includes mechanisms like enumerating
> > available platform TEE I/O capabilities and provisioning connections
> > between the platform TSM and device DSMs.
> > 
> > It is expected to handle hardware TSMs, like AMD SEV-SNP and ARM CCA
> > where there is a physical TEE coprocessor device running firmware, as
> > well as software TSMs like Intel TDX and RISC-V COVE, where there is a
> > privileged software module loaded at runtime.
> > 
> > For now this is just the scaffolding for registering a TSM device and/or
> > TSM-specific attribute groups.
> > 
> > Cc: Xiaoyao Li <xiaoyao.li@intel.com>
> > Cc: Isaku Yamahata <isaku.yamahata@intel.com>
> > Cc: Alexey Kardashevskiy <aik@amd.com>
> > Cc: Wu Hao <hao.wu@intel.com>
> > Cc: Yilun Xu <yilun.xu@intel.com>
> > Cc: Tom Lendacky <thomas.lendacky@amd.com>
> > Cc: John Allen <john.allen@amd.com>
> > Signed-off-by: Dan Williams <dan.j.williams@intel.com>
> > ---
> >   Documentation/ABI/testing/sysfs-class-tsm |   12 +++
> >   drivers/virt/coco/tsm/Kconfig             |    7 ++
> >   drivers/virt/coco/tsm/Makefile            |    3 +
> >   drivers/virt/coco/tsm/class.c             |  100 +++++++++++++++++++++++++++++
> >   include/linux/tsm.h                       |    8 ++
> >   5 files changed, 130 insertions(+)
> >   create mode 100644 Documentation/ABI/testing/sysfs-class-tsm
> >   create mode 100644 drivers/virt/coco/tsm/class.c
> > 
> > diff --git a/Documentation/ABI/testing/sysfs-class-tsm b/Documentation/ABI/testing/sysfs-class-tsm
> > new file mode 100644
> > index 000000000000..304b50b53e65
> > --- /dev/null
> > +++ b/Documentation/ABI/testing/sysfs-class-tsm
> > @@ -0,0 +1,12 @@
> > +What:		/sys/class/tsm/tsm0/host
> > +Date:		January 2024
> > +Contact:	linux-coco@lists.linux.dev
> > +Description:
> > +		(RO) For hardware TSMs represented by a device in /sys/devices,
> > +		@host is a link to that device.
> > +		Links to hardware TSM sysfs ABIs:
> > +		- Documentation/ABI/testing/sysfs-driver-ccp
> > +
> > +		For software TSMs instantiated by a software module, @host is a
> > +		directory with attributes for that TSM, and those attributes are
> > +		documented below.
> > diff --git a/drivers/virt/coco/tsm/Kconfig b/drivers/virt/coco/tsm/Kconfig
> > index 69f04461c83e..595d86917462 100644
> > --- a/drivers/virt/coco/tsm/Kconfig
> > +++ b/drivers/virt/coco/tsm/Kconfig
> > @@ -5,3 +5,10 @@
> >   config TSM_REPORTS
> >   	select CONFIGFS_FS
> >   	tristate
> > +
> > +config ARCH_HAS_TSM
> > +	bool
> > +
> > +config TSM
> > +	depends on ARCH_HAS_TSM && SYSFS
> > +	tristate
> > diff --git a/drivers/virt/coco/tsm/Makefile b/drivers/virt/coco/tsm/Makefile
> > index b48504a3ccfd..f7561169faed 100644
> > --- a/drivers/virt/coco/tsm/Makefile
> > +++ b/drivers/virt/coco/tsm/Makefile
> > @@ -4,3 +4,6 @@
> >   
> >   obj-$(CONFIG_TSM_REPORTS) += tsm_reports.o
> >   tsm_reports-y := reports.o
> > +
> > +obj-$(CONFIG_TSM) += tsm.o
> > +tsm-y := class.o
> > diff --git a/drivers/virt/coco/tsm/class.c b/drivers/virt/coco/tsm/class.c
> > new file mode 100644
> > index 000000000000..a569fa6b09eb
> > --- /dev/null
> > +++ b/drivers/virt/coco/tsm/class.c
> > @@ -0,0 +1,100 @@
> > +// SPDX-License-Identifier: GPL-2.0-only
> > +/* Copyright(c) 2024 Intel Corporation. All rights reserved. */
> > +
> > +#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
> > +
> > +#include <linux/tsm.h>
> > +#include <linux/rwsem.h>
> > +#include <linux/device.h>
> > +#include <linux/module.h>
> > +#include <linux/cleanup.h>
> > +
> > +static DECLARE_RWSEM(tsm_core_rwsem);
> > +struct class *tsm_class;
> > +struct tsm_subsys {
> > +	struct device dev;
> > +	const struct tsm_info *info;
> > +} *tsm_subsys;
> > +
> > +int tsm_register(const struct tsm_info *info)
> > +{
> > +	struct device *dev __free(put_device) = NULL;
> > +	struct tsm_subsys *subsys;
> > +	int rc;
> > +
> > +	guard(rwsem_write)(&tsm_core_rwsem);
> > +	if (tsm_subsys) {
> > +		pr_warn("failed to register: \"%s\", \"%s\" already registered\n",
> > +			info->name, tsm_subsys->info->name);
> > +		return -EBUSY;
> > +	}
> > +
> > +	subsys = kzalloc(sizeof(*subsys), GFP_KERNEL);
> > +	if (!subsys)
> > +		return -ENOMEM;
> > +
> > +	subsys->info = info;
> > +	dev = &subsys->dev;
> > +	dev->class = tsm_class;
> > +	dev->groups = info->groups;
> > +	dev_set_name(dev, "tsm0");
> > +	rc = device_register(dev);
> > +	if (rc)
> > +		return rc;
> 
> 
> no kfree(subsys) ? Thanks,

No, that's handled by __free(put_device), but I want to make this
pattern easier to read with something like.

	struct tsm_subsys *subsys __free(put_tsm_subsys) = tsm_subsys_alloc();
	if (!subsys)
		return -ENOMEM;
	rc = device_register(&subsys->dev);
	cond_no_free_ptr(rc == 0, return rc, subsys)

...stay tuned for whether Linus and/or Peter like the cond_no_free_ptr()
suggestion, will address that on a separate thread.

Re: [RFC PATCH 5/5] PCI/TSM: Authenticate devices via platform TSM

[Dan Williams]

Alexey Kardashevskiy wrote:
> On 30/1/24 20:24, Dan Williams wrote:
> > The PCIe 6.1 specification, section 11, introduces the Trusted
> > Execution Environment (TEE) Device Interface Security Protocol (TDISP).
> > This interface definition builds upon CMA, component measurement and
> > authentication, and IDE, link integrity and data encryption. It adds
> > support for establishing virtual functions within a device that can be
> > assigned to a confidential VM such that the assigned device is enabled
> > to access guest private memory protected by technologies like Intel TDX,
> > AMD SEV-SNP, RISCV COVE, or ARM CCA.
> > 
> > The "TSM" (TEE Security Manager) is a concept in the TDISP specification
> > of an agent that mediates between a device security manager (DSM) and
> > system software in both a VMM and a VM. From a Linux perspective the TSM
> > abstracts many of the details of TDISP, IDE, and CMA. Some of those
> > details leak through at times, but for the most part TDISP is an
> > internal implementation detail of the TSM.
> > 
> > Similar to the PCI core extensions to support CONFIG_PCI_CMA,
> > CONFIG_PCI_TSM builds upon that to reuse the "authenticated" sysfs
> > attribute, and add more properties + controls in a tsm/ subdirectory of
> > the PCI device sysfs interface. Unlike CMA that can depend on a local to
> > the PCI core implementation, PCI_TSM needs to be prepared for late
> > loading of the platform TSM driver. Consider that the TSM driver may
> > itself be a PCI driver. Userspace can depend on the common TSM device
> > uevent to know when the PCI core has TSM services enabled. The PCI
> > device tsm/ subdirectory is supplemented by the TSM device pci/
> > directory for platform global TSM properties + controls.
> > 
> > All vendor TSM implementations share the property of asking the VMM to
> > perform DOE mailbox operations on behalf of the TSM. That common
> > capability is centralized in PCI core code that invokes an ->exec()
> > operation callback potentially multiple times to service a given request
> > (struct pci_tsm_req). Future operations / verbs will be handled
> > similarly with the "request + exec" model. For now, only "connect" and
> > "disconnect" are implemented which at a minimum is expected to establish
> > IDE for the link.
> > 
> > In addition to requests the low-level TSM implementation is notified of
> > device arrival and departure events so that it can filter devices that
> > the TSM is not prepared to support, or otherwise setup and teardown
> > per-device context.
> 
> 
> It's a good start but I am still digesting this scaffolding.
> 
[..]
> > diff --git a/Documentation/ABI/testing/sysfs-class-tsm b/Documentation/ABI/testing/sysfs-class-tsm
> > index 304b50b53e65..77957882738a 100644
> > --- a/Documentation/ABI/testing/sysfs-class-tsm
> > +++ b/Documentation/ABI/testing/sysfs-class-tsm
> > @@ -10,3 +10,26 @@ Description:
> >   		For software TSMs instantiated by a software module, @host is a
> >   		directory with attributes for that TSM, and those attributes are
> >   		documented below.
> > +
> > +
> > +What:		/sys/class/tsm/tsm0/pci/link_capable
> > +Date:		January, 2024
> > +Contact:	linux-coco@lists.linux.dev
> > +Description:
> > +		(RO) When present this returns "1\n" to indicate that the TSM
> > +		supports establishing Link IDE with a given root-port attached
> > +		device. See "tsm/connect_mode" in
> > +		Documentation/ABI/testing/sysfs-bus-pci
> 
> I am struggling to make sense of "a given root-port attached device".
> There is one CCP device on AMD SEV and therefore one /sys/class/tsm/tsmX 
> but still many root ports. How do root ports relate to /sys/class/tsm/tsm0 ?

This is a property of the TSM for whether it supports Link IDE,
Selective IDE, or both. Since Link IDE is a point-to-point protocol, the
TSM can only report whether Link IDE is available for Link IDE capable
root-ports.

For example, I believe some platform vendors are only supporting
Selective IDE with their TSM, while others additionally support Link
IDE.

For simplicity, I will likely drop Link IDE details out of this proposal
as most known use cases specify Selective IDE. Link IDE complexity can
always be added back later.


> > +
> > +
> > +What:		/sys/class/tsm/tsm0/pci/selective_streams
> > +Date:		January, 2024
> > +Contact:	linux-coco@lists.linux.dev
> > +Description:
> > +		(RO) When present this returns the number of currently available
> > +		selective IDE streams available to the TSM. When a stream id is
> > +		allocated this number is decremented and a link to the PCI
> > +		device(s) consuming the stream(s) appears alonside this
> 
> s/alonside/alongside/

thx.

> > +		attribute in the /sys/class/tsm/tsm0/pci/ directory. See
> > +		"tsm/connect" and "tsm/connect_mode" in
> > +		Documentation/ABI/testing/sysfs-bus-pci.
> > diff --git a/drivers/pci/Kconfig b/drivers/pci/Kconfig
> > index a5c3cadddd6f..11d788038d19 100644
> > --- a/drivers/pci/Kconfig
> > +++ b/drivers/pci/Kconfig
> > @@ -129,6 +129,21 @@ config PCI_CMA
> >   	  A PCI DOE mailbox is used as transport for DMTF SPDM based
> >   	  authentication, measurement and secure channel establishment.
> >   
> > +config PCI_TSM
> > +	bool "TEE Security Manager for Device Security"
> 
> (discussed elsewhere, I'll rant here once more and then will shut up)
> 
> It is bool and not tristate :(

Yes.

> CMA, DOE are the same, quite annoying (as in these early days I am 
> adding printks here and there and rmmod+modpbobe saves time but builtins 
> mean reboot) and imho no really necessary as (from 4/5) "only next 
> generation server hosts will start to include a platform TSM".

A couple observations:

* A significant portion of the overall TSM driver complexity lies in
  low-level the platform vendor specific init paths. Those
  initializations paths happen on demand via a loadable module.
  Additionally the successful outcome of cross-vendor  collaboration is
  that the vendor specific runtime paths stay thin, and those will also be
  included the demand loaded vendor platform module.

* The vmlinux .text size increase to turn on CONFIG_PCI_CMA and
  CONFIG_PCI_TSM is in the noise (0.01% increase). The runtime memory
  utilization when the feature is not used is just a pointer in a 'struct
  pci_dev', i.e. only a few hundred bytes for a reasonable laptop.

So the cost of maintaining this facility in the PCI core is negligible
from a memory overhead overhead perspective and the bulk of the
complexity that would benefit from being runtime replaceable as a module
(low level TSM driver) remains modular.

For that small cost we gain a facility that is easy for the rest of the
kernel to reason about and audit. All authentication state linked off of
the PCI device, all sysfs attributes centrally defined. The fundamental
object lifetime and sysfs lifetime is still pci_init_capabilities() and
pci_destroy_dev(), and TSM facilities can be tightly coupled with CMA
which had zero concerns being added to the PCI core.

[..]
> > diff --git a/drivers/pci/tsm.c b/drivers/pci/tsm.c
> > new file mode 100644
> > index 000000000000..f74de0ee49a0
> > --- /dev/null
> > +++ b/drivers/pci/tsm.c
> > @@ -0,0 +1,346 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +/*
> > + * TEE Security Manager for the TEE Device Interface Security Protocol
> > + * (TDISP, PCIe r6.1 sec 11)
> > + *
> > + * Copyright(c) 2024 Intel Corporation. All rights reserved.
> > + */
> > +
> > +#define dev_fmt(fmt) "TSM: " fmt
> > +
> > +#include <linux/pci.h>
> > +#include <linux/tsm.h>
> > +#include <linux/sysfs.h>
> > +#include <linux/xarray.h>
> > +#include "pci.h"
> > +
> > +/* collect tsm capable devices to rendezvous with the tsm driver */
> > +static DEFINE_XARRAY(pci_tsm_devs);
> 
> Not used anywhere.

pci_tsm_register(), pci_tsm_unregister(), pci_tsm_init()

[..]
> > +static int pci_tsm_add(struct pci_dev *pdev)
> 
> Nothing checks the returned value.

Yeah, it turns out that if the tsm rejects the device for whatever
reason then its state will never advance past PCI_TSM_INIT. Likely I
will make pci_tsm_add() return void in the next round.

[..]
> > diff --git a/include/linux/tsm.h b/include/linux/tsm.h
> > index 8cb8a661ba41..f5dbdfa65d8d 100644
> > --- a/include/linux/tsm.h
> > +++ b/include/linux/tsm.h
[..]
> > @@ -74,4 +78,77 @@ int tsm_report_register(const struct tsm_report_ops *ops, void *priv,
> >   int tsm_report_unregister(const struct tsm_report_ops *ops);
> >   int tsm_register(const struct tsm_info *info);
> >   void tsm_unregister(const struct tsm_info *info);
> > +
> > +enum pci_tsm_op_status {
> > +	PCI_TSM_FAIL = -1,
> > +	PCI_TSM_OK,
> > +	PCI_TSM_SPDM_REQ,
> 
> Secure SPDM is also needed here. In my toy TSM project [1] I am just 
> using negatives for errors, 0 for "successfully finished" and positives 
> for a DOE protocol (1 for SPDM, 2 for Secure SPDM), seems alright as it 
> is all about PCI anyway (although "pci" is not always present in all 
> these enums and structs).

Oh, I thought the SPDM marshaling was opaque so the kernel would not
know or care if it was transporting clear-text or cipher text, I will
take a closer look at your implementation.

> > +};
> > +
> > +enum pci_tsm_op {
> > +	PCI_TSM_OP_CONNECT,
> > +	PCI_TSM_OP_DISCONNECT,
> > +};
> > +
> > +struct pci_tsm_req {
> > +	enum pci_tsm_op op;
> > +	unsigned int seq;
> 
> @seq is not tested anywhere.
> 
> May be move (*req_free) here.

Say more, why does each request needs its own custom way to free a
request? I am following the lead of proven mechanisms like bio_put() and
bio_free() where the bio does not have its own ->bio_free() callback.

> > +};
> > +
> > +struct pci_dev;
> > +/**
> > + * struct tsm_pci_ops - Low-level TSM-exported interface to the PCI core
> > + * @add: accept device for tsm operation, locked
> > + * @del: teardown tsm context for @pdev, locked
> > + * @req_alloc: setup context for given operation, unlocked
> > + * @req_free: teardown context for given request, unlocked
> > + * @exec: run @req, may be invoked multiple times per @req, locked
> > + * @lock: tsm work is one device and one op at a time
> > + */
> > +struct tsm_pci_ops {
> > +	int (*add)(struct pci_dev *pdev);
> > +	void (*del)(struct pci_dev *pdev);
> > +	struct pci_tsm_req *(*req_alloc)(struct pci_dev *pdev,
> > +					 enum pci_tsm_op op);
> > +	struct pci_tsm_req *(*req_free)(struct pci_tsm_req *req);
> > +	enum pci_tsm_op_status (*exec)(struct pci_dev *pdev,
> > +				       struct pci_tsm_req *req);
> 
> The pci_tsm_req is just an @op, three hooks seems to be more than 
> needed, could be just (*exec)(struct pci_dev *pdev, enum pci_tsm_op op).

The rationale for other ops beyond exec is to keep the locking context
clear.

That said I do need to rework the locking to make it finer grained, and
allow multiple physical functions to be acted upon at the same time.

> Or the idea is to extend pci_tsm_req with some void *platform_req_data, 
> is not it?

Exactly. req_alloc() is where the low level TSM driver gets to append
its data to the request before execution, and req_free() lets the low
level TSM driver unwind that.

> There is one "op" in flight per a physical device allowed in 
> SEV TIO, I suspect that is likely to be the case for others so such data 
> can be managed by the platform code in the platform data of a TEE-IO device.

Yes, I think one-op per device is a common trait that can be managed by
the core.

[..]
> > +struct pci_tsm {
> > +	enum pci_tsm_state state;
> > +	enum pci_tsm_mode mode;
> 
> Does it have to be either mode and cannot be both?

Yes, mode is "selective" vs "link" IDE. The TDISP spec also allows for a
third option of "TSM deems no IDE needed". However, in terms of what the
user can pick it needs to be "selective" if they want TEE I/O operation.

...and a TSM is free to not implement support for the Link IDE so that
capability can be hidden.

Re: [RFC PATCH 5/5] PCI/TSM: Authenticate devices via platform TSM

[Dan Williams]

Alexey Kardashevskiy wrote:
[..]
> > diff --git a/drivers/pci/cma.c b/drivers/pci/cma.c
> > index be7d2bb21b4c..5a69e9919589 100644
> > --- a/drivers/pci/cma.c
> > +++ b/drivers/pci/cma.c
> > @@ -39,6 +39,9 @@ static ssize_t authenticated_store(struct device *dev,
> >   	if (!sysfs_streq(buf, "native"))
> >   		return -EINVAL;
> >   
> > +	if (pci_tsm_authenticated(pdev))
> > +		return -EBUSY;
> > +
> >   	rc = pci_cma_reauthenticate(pdev);
> >   	if (rc)
> >   		return rc;
> 
> btw is this "native" CMA expected to migrate to tsm_pci_ops? Thanks,

No, CMA is independent from TSM enabling. So you have the option to only
ever authenticate devices via kernel-native CMA, and ignore TEE I/O and
the platform TSM completely. Or, once CMA authentication succeeds then
the kernel additionally allows transitioning the device to be TSM
authenticated / connected.

This keeps the Linux device-attestation ecosystem healthy, standards
compliant devices with managed certificate distribution.

Re: [RFC PATCH 5/5] PCI/TSM: Authenticate devices via platform TSM

[Dan Williams]

Zhi Wang wrote:
[..]
> Hey Dan,
> 
> 1) What is the expectation of using the device connect and disconnect
> in the guest-driven secure I/O enlightenment?

"Connect" is state of the link that can be automatically maintained over
events like reset and error recovery. The guest is responsible for Bind
/ Unbind.

Now, the host can optionally "Connect" in response to a "Bind" event,
but it is not clear that such a mechanism is needed. It likely is going
to depend on how error handling is implemented, and whether an event
that causes disconnect can be recovered. We may get there, but likely
not in the initial phases of the implementation.

> In the last device security meeting, you said the sysfs interface was
> mostly for higher level software stacks, like virt-manager. I was
> wondering what would be the picture looks like when coping these
> statement with the guest-driven model. Are we expecting the device
> connect triggered by QEMU when extracting the guest request from the
> secure channel in this case?

I think it is simplest for now if "Connect" is a pre-requisite for
guest-triggered "Bind".

> 2) How does the device-specific logic fit into the new TSM
> services? E.g. when the TDISP connect is triggered by userspace, a
> device needs to perform quirks before/after/inside the verbs, or a
> device needs an interface to tell TSM service when it is able to
> response to some verbs. Do you think we needs to have a set of
> callbacks from the device side for the PCI TSM service to call?

True "quirks" would be driven by bug reports. Outside of that likely the
attestation information needs to have multiple validation entry points
for userspace, PCI core, and endpoint drivers to each have a chance to
deploy some attestation policy. Some of these questions will need have
common answers developed between the native CMA implementation and the
TSM implementation since CMA needs to solve some of ABI issues of making
measurements available to attestation agents.

At Plumbers I had been thinking "golden measurements" injected into the
kernel ahead of interface validation gives the kernel the most autonomy,
but questions about measurement nonce and other concerns tempered my
thinking there. Plenty to still talk about and navigate.

Re: [RFC PATCH 5/5] PCI/TSM: Authenticate devices via platform TSM

[Zhi Wang]

On Mon, 26 Feb 2024 22:34:54 -0800
Dan Williams <dan.j.williams@intel.com> wrote:

> Zhi Wang wrote:
> [..]
> > Hey Dan,
> > 
> > 1) What is the expectation of using the device connect and
> > disconnect in the guest-driven secure I/O enlightenment?
> 
> "Connect" is state of the link that can be automatically maintained
> over events like reset and error recovery. The guest is responsible
> for Bind / Unbind.
> 
> Now, the host can optionally "Connect" in response to a "Bind" event,
> but it is not clear that such a mechanism is needed. It likely is
> going to depend on how error handling is implemented, and whether an
> event that causes disconnect can be recovered. We may get there, but
> likely not in the initial phases of the implementation.
> 
> > In the last device security meeting, you said the sysfs interface
> > was mostly for higher level software stacks, like virt-manager. I
> > was wondering what would be the picture looks like when coping these
> > statement with the guest-driven model. Are we expecting the device
> > connect triggered by QEMU when extracting the guest request from the
> > secure channel in this case?
> 
> I think it is simplest for now if "Connect" is a pre-requisite for
> guest-triggered "Bind".
>

Thanks so much for the reply. 

IIUC, it means a guest assumes the device is in "connect" state when
noticing a TDI, and it has the awareness that the "connect" state will
be taken care by host until it needs to step in error recovery?

I am more thinking of how device driver talks with the PCI core.

+static int pci_tsm_connect(struct pci_dev *pdev)
+{
+	struct pci_tsm_req *req __free(req_free) = NULL;
+
+	/* opportunistic state checks to skip allocating a request */
+	if (pdev->tsm->state >= PCI_TSM_CONNECT)
+		return 0;
+

As this patch is triggered by userspace through sysfs, I am wondering
would it be a good idea to let the device driver step in around the
device connect/disconnect flow in the future? as a device might needs to
be switched to different states before it is ready to handle SPDM and
IDE.

Maybe the PCI core (pci_tsm_{connect, disconnect, error_handling}())
should broadcast the event through a notifier when checking the connect
state. An example kernel user of that notifier can forward the event to
the userspace as udev events via PF_NETLINK.

+	req = tsm_ops->req_alloc(pdev, PCI_TSM_OP_CONNECT);
+	if (!req)
+		return -ENOMEM;
+
+	scoped_cond_guard(mutex_intr, return -EINTR, tsm_ops->lock) {
+		enum pci_tsm_op_status status;
+
+		/* revalidate state */
+		if (pdev->tsm->state >= PCI_TSM_CONNECT)
+			return 0;
+		if (pdev->tsm->state < PCI_TSM_INIT)
+			return -ENXIO;
+
+		do {
+			status = tsm_ops->exec(pdev, req);
+			req->seq++;
+		} while (status == PCI_TSM_SPDM_REQ);
+
+		if (status == PCI_TSM_FAIL)
+			return -EIO;
+		pdev->tsm->state = PCI_TSM_CONNECT;
+	}
+	return 0;
+}
 
> > 2) How does the device-specific logic fit into the new TSM
> > services? E.g. when the TDISP connect is triggered by userspace, a
> > device needs to perform quirks before/after/inside the verbs, or a
> > device needs an interface to tell TSM service when it is able to
> > response to some verbs. Do you think we needs to have a set of
> > callbacks from the device side for the PCI TSM service to call?
> 
> True "quirks" would be driven by bug reports. Outside of that likely

Yup, I was just thinking another approach for the device specific code
to step in for pci_tsm_{connect,disconnect}() without a driver and
pci-quirks just popped up. :)

> the attestation information needs to have multiple validation entry
> points for userspace, PCI core, and endpoint drivers to each have a
> chance to deploy some attestation policy. Some of these questions
> will need have common answers developed between the native CMA
> implementation and the TSM implementation since CMA needs to solve
> some of ABI issues of making measurements available to attestation
> agents.
> 
> At Plumbers I had been thinking "golden measurements" injected into
> the kernel ahead of interface validation gives the kernel the most
> autonomy, but questions about measurement nonce and other concerns
> tempered my thinking there. Plenty to still talk about and navigate.

Yes. We had been discussing that a lot. :) I also carefully watched the
playback of LPC Confidential MC, quite a lot nice discussions. Out
of curiosity, were folks think eBPF a good fit or an overkill here? :)

Thanks,
Zhi.

Re: [RFC PATCH 5/5] PCI/TSM: Authenticate devices via platform TSM

[Dan Williams]

Zhi Wang wrote:
> On Mon, 26 Feb 2024 22:34:54 -0800
> Dan Williams <dan.j.williams@intel.com> wrote:
> 
> > Zhi Wang wrote:
> > [..]
> > > Hey Dan,
> > > 
> > > 1) What is the expectation of using the device connect and
> > > disconnect in the guest-driven secure I/O enlightenment?
> > 
> > "Connect" is state of the link that can be automatically maintained
> > over events like reset and error recovery. The guest is responsible
> > for Bind / Unbind.
> > 
> > Now, the host can optionally "Connect" in response to a "Bind" event,
> > but it is not clear that such a mechanism is needed. It likely is
> > going to depend on how error handling is implemented, and whether an
> > event that causes disconnect can be recovered. We may get there, but
> > likely not in the initial phases of the implementation.
> > 
> > > In the last device security meeting, you said the sysfs interface
> > > was mostly for higher level software stacks, like virt-manager. I
> > > was wondering what would be the picture looks like when coping these
> > > statement with the guest-driven model. Are we expecting the device
> > > connect triggered by QEMU when extracting the guest request from the
> > > secure channel in this case?
> > 
> > I think it is simplest for now if "Connect" is a pre-requisite for
> > guest-triggered "Bind".
> >
> 
> Thanks so much for the reply. 
> 
> IIUC, it means a guest assumes the device is in "connect" state when
> noticing a TDI, and it has the awareness that the "connect" state will
> be taken care by host until it needs to step in error recovery?
> 
> I am more thinking of how device driver talks with the PCI core.
> 
> +static int pci_tsm_connect(struct pci_dev *pdev)
> +{
> +	struct pci_tsm_req *req __free(req_free) = NULL;
> +
> +	/* opportunistic state checks to skip allocating a request */
> +	if (pdev->tsm->state >= PCI_TSM_CONNECT)
> +		return 0;
> +
> 
> As this patch is triggered by userspace through sysfs, I am wondering
> would it be a good idea to let the device driver step in around the
> device connect/disconnect flow in the future? as a device might needs to
> be switched to different states before it is ready to handle SPDM and
> IDE.

This capability needs a definite use case for the kernel to onboard the
complexity, and notifiers in general increase the maintenance burden.
This PCI/TSM proposal is that "connect" can happen independent of any
driver being loaded. If a driver wants to trigger reconnect after it
loads it can, but it is not clear why it would need to get its fingers
into the "connect" process itself. I.e. I would hope that at maximum an
endpoint driver just needs to see the results of "connect", or trigger
reconnect, not intercept the connect flow.

If the connect state is treated like a link state then it is reasonable
to expect that the PCI core restores the link after reset. If that is
the case then the driver's mechanism to trigger reconnect is to call
pci_reset_function() which implicitly handles reconnecting the device.

> Maybe the PCI core (pci_tsm_{connect, disconnect, error_handling}())
> should broadcast the event through a notifier when checking the connect
> state. An example kernel user of that notifier can forward the event to
> the userspace as udev events via PF_NETLINK.

Drivers can already register for ->reset_prepare() and ->reset_done()
notifiers. It would be great to not need to invent new notification
infrastructure, at least without an exceedingly good reason.

> > the attestation information needs to have multiple validation entry
> > points for userspace, PCI core, and endpoint drivers to each have a
> > chance to deploy some attestation policy. Some of these questions
> > will need have common answers developed between the native CMA
> > implementation and the TSM implementation since CMA needs to solve
> > some of ABI issues of making measurements available to attestation
> > agents.
> > 
> > At Plumbers I had been thinking "golden measurements" injected into
> > the kernel ahead of interface validation gives the kernel the most
> > autonomy, but questions about measurement nonce and other concerns
> > tempered my thinking there. Plenty to still talk about and navigate.
> 
> Yes. We had been discussing that a lot. :) I also carefully watched the
> playback of LPC Confidential MC, quite a lot nice discussions. Out
> of curiosity, were folks think eBPF a good fit or an overkill here? :)

Probably too early to settle that, we are still trying to figure out how
to log native measurements [1] and the logging mechanism [2]. eBPF only
comes after memcmp() is deemed insufficient, but even memcmp() needs to
plumb the measurements through an attestation mechanism.

[1]: http://lore.kernel.org/r/20240222154529.GA9465@wunner.de
[2]: http://lore.kernel.org/r/20240128212532.2754325-1-sameo@rivosinc.com

Re: [RFC PATCH 3/5] coco/tsm: Introduce a shared class device for TSMs

[Jonathan Cameron]

On Tue, 30 Jan 2024 01:24:02 -0800
Dan Williams <dan.j.williams@intel.com> wrote:

> A "tsm" is a platform component that provides an API for securely
> provisioning resources for a confidential guest (TVM) to consume. "TSM"
> also happens to be the acronym the PCI specification uses to define the
> platform agent that carries out device-security operations. That
> platform capability is commonly called TEE I/O. It is this arrival of
> TEE I/O platforms that requires the "tsm" concept to grow from a
> low-level arch-specific detail of TVM instantiation, to a frontend
> interface to mediate device setup and interact with general purpose
> kernel subsystems outside of arch/ like the PCI core.
> 
> Provide a virtual (as in /sys/devices/virtual) class device interface to
> front all of the aspects of a TSM and TEE I/O that are
> cross-architecture common. This includes mechanisms like enumerating
> available platform TEE I/O capabilities and provisioning connections
> between the platform TSM and device DSMs.
> 
> It is expected to handle hardware TSMs, like AMD SEV-SNP and ARM CCA
> where there is a physical TEE coprocessor device running firmware, as
> well as software TSMs like Intel TDX and RISC-V COVE, where there is a
> privileged software module loaded at runtime.
> 
> For now this is just the scaffolding for registering a TSM device and/or
> TSM-specific attribute groups.
> 
> Cc: Xiaoyao Li <xiaoyao.li@intel.com>
> Cc: Isaku Yamahata <isaku.yamahata@intel.com>
> Cc: Alexey Kardashevskiy <aik@amd.com>
> Cc: Wu Hao <hao.wu@intel.com>
> Cc: Yilun Xu <yilun.xu@intel.com>
> Cc: Tom Lendacky <thomas.lendacky@amd.com>
> Cc: John Allen <john.allen@amd.com>
> Signed-off-by: Dan Williams <dan.j.williams@intel.com>

A few drive by comments because I was curious.


> diff --git a/drivers/virt/coco/tsm/class.c b/drivers/virt/coco/tsm/class.c
> new file mode 100644
> index 000000000000..a569fa6b09eb
> --- /dev/null
> +++ b/drivers/virt/coco/tsm/class.c
> @@ -0,0 +1,100 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/* Copyright(c) 2024 Intel Corporation. All rights reserved. */
> +
> +#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
> +
> +#include <linux/tsm.h>
> +#include <linux/rwsem.h>
> +#include <linux/device.h>
> +#include <linux/module.h>
> +#include <linux/cleanup.h>
> +
> +static DECLARE_RWSEM(tsm_core_rwsem);
> +struct class *tsm_class;
> +struct tsm_subsys {
> +	struct device dev;
> +	const struct tsm_info *info;
> +} *tsm_subsys;
> +
> +int tsm_register(const struct tsm_info *info)
> +{
> +	struct device *dev __free(put_device) = NULL;

I think it would be appropriate to move this down to where
you set dev so it is moderately obvious where it comes from.
This only becomes valid cleanup after the call to device_register()
with it's device_initialize - which is ugly. 
Maybe we should just use split device_initialize() / device_add()
when combining with __free(put_device);

The ideal would be something like.

	struct device *dev __free(put_device) = device_initialize(&subsys->dev);

with device_initialize() returning the dev parameter.

For the really adventurous maybe even the overkill option of the following
(I know it's currently pointless complexity - given no error paths between
 the kzalloc and device_initialize())

	struct tsm_subsys *subsys __free(kfree) = kzalloc(sizeof(*subsys), GFP_KERNEL);

//Now safe to exit here.

	struct device *dev __free(put_device) = device_initialize(&no_free_ptr(subsys)->dev);

// Also good to exit here with no double free etc though subsys is now inaccessible breaking
the one place it's used later ;)

Maybe I'm over thinking things and I doubt cleanup.h discussions
was really what you wanted from this RFC :)


> +	struct tsm_subsys *subsys;
> +	int rc;
> +
> +	guard(rwsem_write)(&tsm_core_rwsem);
> +	if (tsm_subsys) {
> +		pr_warn("failed to register: \"%s\", \"%s\" already registered\n",
> +			info->name, tsm_subsys->info->name);
> +		return -EBUSY;
> +	}
> +
> +	subsys = kzalloc(sizeof(*subsys), GFP_KERNEL);
> +	if (!subsys)
> +		return -ENOMEM;
> +
> +	subsys->info = info;
> +	dev = &subsys->dev;
> +	dev->class = tsm_class;
> +	dev->groups = info->groups;
> +	dev_set_name(dev, "tsm0");
> +	rc = device_register(dev);
> +	if (rc)
> +		return rc;
> +
> +	if (info->host) {
> +		rc = sysfs_create_link(&dev->kobj, &info->host->kobj, "host");

Why not parent it from there?  If it has a logical parent, that would be
nicer than using a virtual device.

> +		if (rc)
> +			return rc;
> +	}
> +
> +	/* don't auto-free @dev */
> +	dev = NULL;
> +	tsm_subsys = subsys;
> +
> +	return 0;
> +}
> +EXPORT_SYMBOL_GPL(tsm_register);

Seems appropriate to namespace these from the start.

> +
> +void tsm_unregister(const struct tsm_info *info)
> +{
> +	guard(rwsem_write)(&tsm_core_rwsem);
> +	if (!tsm_subsys || info != tsm_subsys->info) {
> +		pr_warn("failed to unregister: \"%s\", not currently registered\n",
> +			info->name);
> +		return;
> +	}
> +
> +	if (info->host)
> +		sysfs_remove_link(&tsm_subsys->dev.kobj, "host");
> +	device_unregister(&tsm_subsys->dev);
> +	tsm_subsys = NULL;
> +}
> +EXPORT_SYMBOL_GPL(tsm_unregister);

Re: [RFC PATCH 5/5] PCI/TSM: Authenticate devices via platform TSM

[Jonathan Cameron]

On Tue, 30 Jan 2024 01:24:14 -0800
Dan Williams <dan.j.williams@intel.com> wrote:

> The PCIe 6.1 specification, section 11, introduces the Trusted
> Execution Environment (TEE) Device Interface Security Protocol (TDISP).
> This interface definition builds upon CMA, component measurement and
> authentication, and IDE, link integrity and data encryption. It adds
> support for establishing virtual functions within a device that can be
> assigned to a confidential VM such that the assigned device is enabled
> to access guest private memory protected by technologies like Intel TDX,
> AMD SEV-SNP, RISCV COVE, or ARM CCA.
> 
> The "TSM" (TEE Security Manager) is a concept in the TDISP specification
> of an agent that mediates between a device security manager (DSM) and
> system software in both a VMM and a VM. From a Linux perspective the TSM
> abstracts many of the details of TDISP, IDE, and CMA. Some of those
> details leak through at times, but for the most part TDISP is an
> internal implementation detail of the TSM.
> 
> Similar to the PCI core extensions to support CONFIG_PCI_CMA,
> CONFIG_PCI_TSM builds upon that to reuse the "authenticated" sysfs
> attribute, and add more properties + controls in a tsm/ subdirectory of
> the PCI device sysfs interface. Unlike CMA that can depend on a local to
> the PCI core implementation, PCI_TSM needs to be prepared for late
> loading of the platform TSM driver. Consider that the TSM driver may
> itself be a PCI driver. Userspace can depend on the common TSM device
> uevent to know when the PCI core has TSM services enabled. The PCI
> device tsm/ subdirectory is supplemented by the TSM device pci/
> directory for platform global TSM properties + controls.
> 
> All vendor TSM implementations share the property of asking the VMM to
> perform DOE mailbox operations on behalf of the TSM. That common
> capability is centralized in PCI core code that invokes an ->exec()
> operation callback potentially multiple times to service a given request
> (struct pci_tsm_req). Future operations / verbs will be handled
> similarly with the "request + exec" model. For now, only "connect" and
> "disconnect" are implemented which at a minimum is expected to establish
> IDE for the link.
> 
> In addition to requests the low-level TSM implementation is notified of
> device arrival and departure events so that it can filter devices that
> the TSM is not prepared to support, or otherwise setup and teardown
> per-device context.
> 
> Cc: Wu Hao <hao.wu@intel.com>
> Cc: Yilun Xu <yilun.xu@intel.com>
> Cc: Lukas Wunner <lukas@wunner.de>
> Cc: Samuel Ortiz <sameo@rivosinc.com>
> Cc: Alexey Kardashevskiy <aik@amd.com>
> Cc: Bjorn Helgaas <bhelgaas@google.com>
> Signed-off-by: Dan Williams <dan.j.williams@intel.com>

Superficial comments inline - noticed whilst getting my head
around the basic structure.




> diff --git a/drivers/pci/tsm.c b/drivers/pci/tsm.c
> new file mode 100644
> index 000000000000..f74de0ee49a0
> --- /dev/null
> +++ b/drivers/pci/tsm.c
> @@ -0,0 +1,346 @@

> +
> +DEFINE_FREE(req_free, struct pci_tsm_req *, if (_T) tsm_ops->req_free(_T))
> +
> +static int pci_tsm_disconnect(struct pci_dev *pdev)
> +{
> +	struct pci_tsm_req *req __free(req_free) = NULL;

As below. I'll stop commenting on these.

> +
> +	/* opportunistic state checks to skip allocating a request */
> +	if (pdev->tsm->state < PCI_TSM_CONNECT)
> +		return 0;
> +
> +	req = tsm_ops->req_alloc(pdev, PCI_TSM_OP_DISCONNECT);
> +	if (!req)
> +		return -ENOMEM;
> +
> +	scoped_cond_guard(mutex_intr, return -EINTR, tsm_ops->lock) {
> +		enum pci_tsm_op_status status;
> +
> +		/* revalidate state */
> +		if (pdev->tsm->state < PCI_TSM_CONNECT)
> +			return 0;
> +		if (pdev->tsm->state < PCI_TSM_INIT)
> +			return -ENXIO;
> +
> +		do {
> +			status = tsm_ops->exec(pdev, req);
> +			req->seq++;
> +			/* TODO: marshal SPDM request */
> +		} while (status == PCI_TSM_SPDM_REQ);
> +
> +		if (status == PCI_TSM_FAIL)
> +			return -EIO;
> +		pdev->tsm->state = PCI_TSM_INIT;
> +	}
> +	return 0;
> +}
> +
> +static int pci_tsm_connect(struct pci_dev *pdev)
> +{
> +	struct pci_tsm_req *req __free(req_free) = NULL;

Push down a few lines to put it where the allocation happens.

> +
> +	/* opportunistic state checks to skip allocating a request */
> +	if (pdev->tsm->state >= PCI_TSM_CONNECT)
> +		return 0;
> +
> +	req = tsm_ops->req_alloc(pdev, PCI_TSM_OP_CONNECT);
> +	if (!req)
> +		return -ENOMEM;
> +
> +	scoped_cond_guard(mutex_intr, return -EINTR, tsm_ops->lock) {

What could possibly go wrong? Everyone loves scoped_cond_guard ;)

> +		enum pci_tsm_op_status status;
> +
> +		/* revalidate state */
> +		if (pdev->tsm->state >= PCI_TSM_CONNECT)
> +			return 0;
> +		if (pdev->tsm->state < PCI_TSM_INIT)
> +			return -ENXIO;
> +
> +		do {
> +			status = tsm_ops->exec(pdev, req);
> +			req->seq++;
> +		} while (status == PCI_TSM_SPDM_REQ);
> +
> +		if (status == PCI_TSM_FAIL)
> +			return -EIO;
> +		pdev->tsm->state = PCI_TSM_CONNECT;
> +	}
> +	return 0;
> +}

...

> + size_t connect_mode_store(struct device *dev,
> +				  struct device_attribute *attr,
> +				  const char *buf, size_t len)
> +{
> +	struct pci_dev *pdev = to_pci_dev(dev);
> +	int i;
> +
> +	guard(mutex)(tsm_ops->lock);
> +	if (pdev->tsm->state >= PCI_TSM_CONNECT)
> +		return -EBUSY;
> +	for (i = 0; i < ARRAY_SIZE(pci_tsm_modes); i++)
> +		if (sysfs_streq(buf, pci_tsm_modes[i]))
> +			break;
> +	if (i == PCI_TSM_MODE_LINK) {
> +		if (pdev->tsm->link_capable)
> +			pdev->tsm->mode = PCI_TSM_MODE_LINK;

Error in all real paths paths?
Also, maybe a switch will be more sensible here.

> +		return -EOPNOTSUPP;
> +	} else if (i == PCI_TSM_MODE_SELECTIVE) {
> +		if (pdev->tsm->selective_capable)
> +			pdev->tsm->mode = PCI_TSM_MODE_SELECTIVE;
> +		return -EOPNOTSUPP;
> +	} else
> +		return -EINVAL;
> +	return len;
> +}


> +
> +void pci_tsm_init(struct pci_dev *pdev)
> +{
> +	u16 ide_cap;
> +	int rc;
> +
> +	if (!pdev->cma_capable)
> +		return;
> +
> +	ide_cap = pci_find_ext_capability(pdev, PCI_EXT_CAP_ID_IDE);
> +	if (!ide_cap)
> +		return;
> +
> +	struct pci_tsm *tsm __free(kfree) = kzalloc(sizeof(*tsm), GFP_KERNEL);
> +	if (!tsm)
> +		return;
> +
> +	tsm->ide_cap = ide_cap;
> +
> +	rc = xa_insert(&pci_tsm_devs, (unsigned long)pdev, pdev, GFP_KERNEL);

This is an unusual pattern with the key and the value matching.
Feels like xarray might for once not be the best choice of structure.

> +	if (rc) {
> +		pci_dbg(pdev, "failed to register tsm capable device\n");
> +		return;
> +	}
> +
> +	guard(rwsem_write)(&pci_tsm_rwsem);
> +	pdev->tsm = no_free_ptr(tsm);
> +	pci_tsm_add(pdev);
> +}
> +
> +void pci_tsm_destroy(struct pci_dev *pdev)
> +{
> +	guard(rwsem_write)(&pci_tsm_rwsem);
> +	pci_tsm_del(pdev);
> +	xa_erase(&pci_tsm_devs, (unsigned long)pdev);
> +	kfree(pdev->tsm);
> +	pdev->tsm = NULL;
> +}



> diff --git a/drivers/virt/coco/tsm/class.c b/drivers/virt/coco/tsm/class.c
> index a569fa6b09eb..a459e51c0892 100644
> --- a/drivers/virt/coco/tsm/class.c
> +++ b/drivers/virt/coco/tsm/class.c

> +static const struct attribute_group *tsm_attr_groups[] = {
> +#ifdef CONFIG_PCI_TSM
> +	&tsm_pci_attr_group,
> +#endif
> +	NULL,
Trivial but, no point in that comma as we will never chase it with anything.
> +};
> +
>  static int __init tsm_init(void)
>  {
>  	tsm_class = class_create("tsm");
> @@ -86,6 +97,7 @@ static int __init tsm_init(void)
>  		return PTR_ERR(tsm_class);
>  
>  	tsm_class->dev_release = tsm_release;
> +	tsm_class->dev_groups = tsm_attr_groups;
>  	return 0;
>  }
>  module_init(tsm_init)
> diff --git a/drivers/virt/coco/tsm/pci.c b/drivers/virt/coco/tsm/pci.c
> new file mode 100644
> index 000000000000..b3684ad7114f
> --- /dev/null
> +++ b/drivers/virt/coco/tsm/pci.c

> +
> +static bool tsm_pci_group_visible(struct kobject *kobj)
> +{
> +	struct device *dev = kobj_to_dev(kobj);
> +	struct tsm_subsys *subsys = container_of(dev, typeof(*subsys), dev);
Give this a macro probably.
 dev_to_tsm_subsys(kobj_to_dev(kobj));

> +
> +	if (subsys->info->pci_ops)
> +		return true;

	return subsys->info->pci->ops;
maybe

> +	return false;
> +}
> +DEFINE_SYSFS_GROUP_VISIBLE(tsm_pci);
> +
> +static struct attribute *tsm_pci_attrs[] = {
> +	&dev_attr_link_capable.attr,
> +	&dev_attr_selective_streams.attr,
> +	NULL,
> +};
> +
> +const struct attribute_group tsm_pci_attr_group = {
> +	.name = "pci",
> +	.attrs = tsm_pci_attrs,
> +	.is_visible = SYSFS_GROUP_VISIBLE(tsm_pci),
> +};
> +
> +int tsm_pci_init(const struct tsm_info *info)
> +{
> +	if (!info->pci_ops)
> +		return 0;
> +
> +	return pci_tsm_register(info->pci_ops);
> +}
> +
> +void tsm_pci_destroy(const struct tsm_info *info)
> +{
> +	pci_tsm_unregister(info->pci_ops);
> +}
> diff --git a/drivers/virt/coco/tsm/tsm.h b/drivers/virt/coco/tsm/tsm.h
> new file mode 100644
> index 000000000000..407c388a109b
> --- /dev/null
> +++ b/drivers/virt/coco/tsm/tsm.h
> @@ -0,0 +1,28 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +#ifndef __TSM_CORE_H
> +#define __TSM_CORE_H
> +
> +#include <linux/device.h>
> +
> +struct tsm_info;
> +struct tsm_subsys {
> +	struct device dev;
> +	const struct tsm_info *info;
> +};

I know people like to build up patch sets, but I think defining this here
from patch 3 would just reduce noise.

> +
> +#ifdef CONFIG_PCI_TSM
> +int tsm_pci_init(const struct tsm_info *info);
> +void tsm_pci_destroy(const struct tsm_info *info);
> +extern const struct attribute_group tsm_pci_attr_group;
> +#else
> +static inline int tsm_pci_init(const struct tsm_info *info)
> +{
> +	return 0;
> +}
> +static inline void tsm_pci_destroy(const struct tsm_info *info)
> +{
> +}
> +#endif
> +
> +#endif /* TSM_CORE_H */

> diff --git a/include/linux/tsm.h b/include/linux/tsm.h
> index 8cb8a661ba41..f5dbdfa65d8d 100644
> --- a/include/linux/tsm.h
> +++ b/include/linux/tsm.h
> @@ -4,11 +4,15 @@
>  
>  #include <linux/sizes.h>
>  #include <linux/types.h>
> +#include <linux/mutex.h>

struct mutex;
instead given you only have a pointer I think.


>  
>  struct tsm_info {
>  	const char *name;
>  	struct device *host;
>  	const struct attribute_group **groups;
> +	const struct tsm_pci_ops *pci_ops;
> +	unsigned int nr_selective_streams;
> +	unsigned int link_stream_capable:1;
>  };


> +struct pci_dev;
> +/**
> + * struct tsm_pci_ops - Low-level TSM-exported interface to the PCI core
> + * @add: accept device for tsm operation, locked
> + * @del: teardown tsm context for @pdev, locked
> + * @req_alloc: setup context for given operation, unlocked
> + * @req_free: teardown context for given request, unlocked
> + * @exec: run @req, may be invoked multiple times per @req, locked
> + * @lock: tsm work is one device and one op at a time
> + */
> +struct tsm_pci_ops {
> +	int (*add)(struct pci_dev *pdev);
> +	void (*del)(struct pci_dev *pdev);
> +	struct pci_tsm_req *(*req_alloc)(struct pci_dev *pdev,
> +					 enum pci_tsm_op op);
> +	struct pci_tsm_req *(*req_free)(struct pci_tsm_req *req);
> +	enum pci_tsm_op_status (*exec)(struct pci_dev *pdev,
> +				       struct pci_tsm_req *req);
> +	struct mutex *lock;

Mixing data with what would otherwise be likely to be constant pointers
probably best avoided.  Maybe wrap the lock in another op instead?


> +};

Re: [RFC PATCH 3/5] coco/tsm: Introduce a shared class device for TSMs

[Dan Williams]

Jonathan Cameron wrote:
> On Tue, 30 Jan 2024 01:24:02 -0800
> Dan Williams <dan.j.williams@intel.com> wrote:
> 
> > A "tsm" is a platform component that provides an API for securely
> > provisioning resources for a confidential guest (TVM) to consume. "TSM"
> > also happens to be the acronym the PCI specification uses to define the
> > platform agent that carries out device-security operations. That
> > platform capability is commonly called TEE I/O. It is this arrival of
> > TEE I/O platforms that requires the "tsm" concept to grow from a
> > low-level arch-specific detail of TVM instantiation, to a frontend
> > interface to mediate device setup and interact with general purpose
> > kernel subsystems outside of arch/ like the PCI core.
> > 
> > Provide a virtual (as in /sys/devices/virtual) class device interface to
> > front all of the aspects of a TSM and TEE I/O that are
> > cross-architecture common. This includes mechanisms like enumerating
> > available platform TEE I/O capabilities and provisioning connections
> > between the platform TSM and device DSMs.
> > 
> > It is expected to handle hardware TSMs, like AMD SEV-SNP and ARM CCA
> > where there is a physical TEE coprocessor device running firmware, as
> > well as software TSMs like Intel TDX and RISC-V COVE, where there is a
> > privileged software module loaded at runtime.
> > 
> > For now this is just the scaffolding for registering a TSM device and/or
> > TSM-specific attribute groups.
> > 
> > Cc: Xiaoyao Li <xiaoyao.li@intel.com>
> > Cc: Isaku Yamahata <isaku.yamahata@intel.com>
> > Cc: Alexey Kardashevskiy <aik@amd.com>
> > Cc: Wu Hao <hao.wu@intel.com>
> > Cc: Yilun Xu <yilun.xu@intel.com>
> > Cc: Tom Lendacky <thomas.lendacky@amd.com>
> > Cc: John Allen <john.allen@amd.com>
> > Signed-off-by: Dan Williams <dan.j.williams@intel.com>
> 
> A few drive by comments because I was curious.
> 
> 
> > diff --git a/drivers/virt/coco/tsm/class.c b/drivers/virt/coco/tsm/class.c
> > new file mode 100644
> > index 000000000000..a569fa6b09eb
> > --- /dev/null
> > +++ b/drivers/virt/coco/tsm/class.c
> > @@ -0,0 +1,100 @@
> > +// SPDX-License-Identifier: GPL-2.0-only
> > +/* Copyright(c) 2024 Intel Corporation. All rights reserved. */
> > +
> > +#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
> > +
> > +#include <linux/tsm.h>
> > +#include <linux/rwsem.h>
> > +#include <linux/device.h>
> > +#include <linux/module.h>
> > +#include <linux/cleanup.h>
> > +
> > +static DECLARE_RWSEM(tsm_core_rwsem);
> > +struct class *tsm_class;
> > +struct tsm_subsys {
> > +	struct device dev;
> > +	const struct tsm_info *info;
> > +} *tsm_subsys;
> > +
> > +int tsm_register(const struct tsm_info *info)
> > +{
> > +	struct device *dev __free(put_device) = NULL;
> 
> I think it would be appropriate to move this down to where
> you set dev so it is moderately obvious where it comes from.
> This only becomes valid cleanup after the call to device_register()
> with it's device_initialize - which is ugly. 
> Maybe we should just use split device_initialize() / device_add()
> when combining with __free(put_device);
> 
> The ideal would be something like.
> 
> 	struct device *dev __free(put_device) = device_initialize(&subsys->dev);
> 
> with device_initialize() returning the dev parameter.
> 
> For the really adventurous maybe even the overkill option of the following
> (I know it's currently pointless complexity - given no error paths between
>  the kzalloc and device_initialize())
> 
> 	struct tsm_subsys *subsys __free(kfree) = kzalloc(sizeof(*subsys), GFP_KERNEL);
> 
> //Now safe to exit here.
> 
> 	struct device *dev __free(put_device) = device_initialize(&no_free_ptr(subsys)->dev);
> 
> // Also good to exit here with no double free etc though subsys is now inaccessible breaking
> the one place it's used later ;)
> 
> Maybe I'm over thinking things and I doubt cleanup.h discussions
> was really what you wanted from this RFC :)

Heh, useful review is always welcome, and yeah, I think every instance
of the "... __free(x) = NULL" pattern deserves to be scrutinized. Likely
what I will do is add a helper function which was the main takeaway
I got from the Linus review of cond_no_free_ptr(). Something like:

diff --git a/drivers/virt/coco/tsm/class.c b/drivers/virt/coco/tsm/class.c
index a569fa6b09eb..a60087905c72 100644
--- a/drivers/virt/coco/tsm/class.c
+++ b/drivers/virt/coco/tsm/class.c
@@ -16,10 +16,29 @@ struct tsm_subsys {
 	const struct tsm_info *info;
 } *tsm_subsys;
 
+static struct tsm_subsys *tsm_subsys_alloc(const struct tsm_info *info)
+{
+	struct device *dev;
+
+	struct tsm_subsys *subsys __free(kfree) =
+		kzalloc(sizeof(*subsys), GFP_KERNEL);
+	if (!subsys)
+		return NULL;
+
+	subsys->info = info;
+	dev = &subsys->dev;
+	dev->class = tsm_class;
+	dev->groups = info->groups;
+	if (dev_set_name(dev, "tsm0"))
+		return NULL;
+	device_initialize(dev);
+
+	return no_free_ptr(subsys);
+}
+DEFINE_FREE(tsm_subsys_put, struct tsm_subsys *, if (_T) put_device(&_T->dev))
+
 int tsm_register(const struct tsm_info *info)
 {
-	struct device *dev __free(put_device) = NULL;
-	struct tsm_subsys *subsys;
 	int rc;
 
 	guard(rwsem_write)(&tsm_core_rwsem);
@@ -29,16 +48,11 @@ int tsm_register(const struct tsm_info *info)
 		return -EBUSY;
 	}
 
-	subsys = kzalloc(sizeof(*subsys), GFP_KERNEL);
+	struct tsm_subsys *subsys __free(tsm_subsys_put) = tsm_subsys_alloc(info);
 	if (!subsys)
 		return -ENOMEM;
 
-	subsys->info = info;
-	dev = &subsys->dev;
-	dev->class = tsm_class;
-	dev->groups = info->groups;
-	dev_set_name(dev, "tsm0");
-	rc = device_register(dev);
+	rc = device_add(&subsys->dev);
 	if (rc)
 		return rc;
 
@@ -48,9 +62,7 @@ int tsm_register(const struct tsm_info *info)
 			return rc;
 	}
 
-	/* don't auto-free @dev */
-	dev = NULL;
-	tsm_subsys = subsys;
+	tsm_subsys = no_free_ptr(subsys);
 
 	return 0;
 }

> 
> 
> > +	struct tsm_subsys *subsys;
> > +	int rc;
> > +
> > +	guard(rwsem_write)(&tsm_core_rwsem);
> > +	if (tsm_subsys) {
> > +		pr_warn("failed to register: \"%s\", \"%s\" already registered\n",
> > +			info->name, tsm_subsys->info->name);
> > +		return -EBUSY;
> > +	}
> > +
> > +	subsys = kzalloc(sizeof(*subsys), GFP_KERNEL);
> > +	if (!subsys)
> > +		return -ENOMEM;
> > +
> > +	subsys->info = info;
> > +	dev = &subsys->dev;
> > +	dev->class = tsm_class;
> > +	dev->groups = info->groups;
> > +	dev_set_name(dev, "tsm0");
> > +	rc = device_register(dev);
> > +	if (rc)
> > +		return rc;
> > +
> > +	if (info->host) {
> > +		rc = sysfs_create_link(&dev->kobj, &info->host->kobj, "host");
> 
> Why not parent it from there?  If it has a logical parent, that would be
> nicer than using a virtual device.

Yeah at the time I was drafting this I was working around the lack of a
ready device parent for Intel TDX which can not lean on a PCI device TSM
like other archs. However, yes, that would be nice and it implies that
Intel TDX just needs to build a virtual device abstraction to hang this
interface.

> 
> > +		if (rc)
> > +			return rc;
> > +	}
> > +
> > +	/* don't auto-free @dev */
> > +	dev = NULL;
> > +	tsm_subsys = subsys;
> > +
> > +	return 0;
> > +}
> > +EXPORT_SYMBOL_GPL(tsm_register);
> 
> Seems appropriate to namespace these from the start.

Sure.

Re: [RFC PATCH 5/5] PCI/TSM: Authenticate devices via platform TSM

[Dan Williams]

Jonathan Cameron wrote:
> On Tue, 30 Jan 2024 01:24:14 -0800
> Dan Williams <dan.j.williams@intel.com> wrote:
> 
> > The PCIe 6.1 specification, section 11, introduces the Trusted
> > Execution Environment (TEE) Device Interface Security Protocol (TDISP).
> > This interface definition builds upon CMA, component measurement and
> > authentication, and IDE, link integrity and data encryption. It adds
> > support for establishing virtual functions within a device that can be
> > assigned to a confidential VM such that the assigned device is enabled
> > to access guest private memory protected by technologies like Intel TDX,
> > AMD SEV-SNP, RISCV COVE, or ARM CCA.
> > 
> > The "TSM" (TEE Security Manager) is a concept in the TDISP specification
> > of an agent that mediates between a device security manager (DSM) and
> > system software in both a VMM and a VM. From a Linux perspective the TSM
> > abstracts many of the details of TDISP, IDE, and CMA. Some of those
> > details leak through at times, but for the most part TDISP is an
> > internal implementation detail of the TSM.
> > 
> > Similar to the PCI core extensions to support CONFIG_PCI_CMA,
> > CONFIG_PCI_TSM builds upon that to reuse the "authenticated" sysfs
> > attribute, and add more properties + controls in a tsm/ subdirectory of
> > the PCI device sysfs interface. Unlike CMA that can depend on a local to
> > the PCI core implementation, PCI_TSM needs to be prepared for late
> > loading of the platform TSM driver. Consider that the TSM driver may
> > itself be a PCI driver. Userspace can depend on the common TSM device
> > uevent to know when the PCI core has TSM services enabled. The PCI
> > device tsm/ subdirectory is supplemented by the TSM device pci/
> > directory for platform global TSM properties + controls.
> > 
> > All vendor TSM implementations share the property of asking the VMM to
> > perform DOE mailbox operations on behalf of the TSM. That common
> > capability is centralized in PCI core code that invokes an ->exec()
> > operation callback potentially multiple times to service a given request
> > (struct pci_tsm_req). Future operations / verbs will be handled
> > similarly with the "request + exec" model. For now, only "connect" and
> > "disconnect" are implemented which at a minimum is expected to establish
> > IDE for the link.
> > 
> > In addition to requests the low-level TSM implementation is notified of
> > device arrival and departure events so that it can filter devices that
> > the TSM is not prepared to support, or otherwise setup and teardown
> > per-device context.
> > 
> > Cc: Wu Hao <hao.wu@intel.com>
> > Cc: Yilun Xu <yilun.xu@intel.com>
> > Cc: Lukas Wunner <lukas@wunner.de>
> > Cc: Samuel Ortiz <sameo@rivosinc.com>
> > Cc: Alexey Kardashevskiy <aik@amd.com>
> > Cc: Bjorn Helgaas <bhelgaas@google.com>
> > Signed-off-by: Dan Williams <dan.j.williams@intel.com>
> 
> Superficial comments inline - noticed whilst getting my head
> around the basic structure.
> 
> 
> 
> 
> > diff --git a/drivers/pci/tsm.c b/drivers/pci/tsm.c
> > new file mode 100644
> > index 000000000000..f74de0ee49a0
> > --- /dev/null
> > +++ b/drivers/pci/tsm.c
> > @@ -0,0 +1,346 @@
> 
> > +
> > +DEFINE_FREE(req_free, struct pci_tsm_req *, if (_T) tsm_ops->req_free(_T))
> > +
> > +static int pci_tsm_disconnect(struct pci_dev *pdev)
> > +{
> > +	struct pci_tsm_req *req __free(req_free) = NULL;
> 
> As below. I'll stop commenting on these.

Hey, they are fair game, will find a way to rework this and not use the
confusing pattern.

> > +
> > +	/* opportunistic state checks to skip allocating a request */
> > +	if (pdev->tsm->state < PCI_TSM_CONNECT)
> > +		return 0;
> > +
> > +	req = tsm_ops->req_alloc(pdev, PCI_TSM_OP_DISCONNECT);
> > +	if (!req)
> > +		return -ENOMEM;
> > +
> > +	scoped_cond_guard(mutex_intr, return -EINTR, tsm_ops->lock) {
> > +		enum pci_tsm_op_status status;
> > +
> > +		/* revalidate state */
> > +		if (pdev->tsm->state < PCI_TSM_CONNECT)
> > +			return 0;
> > +		if (pdev->tsm->state < PCI_TSM_INIT)
> > +			return -ENXIO;
> > +
> > +		do {
> > +			status = tsm_ops->exec(pdev, req);
> > +			req->seq++;
> > +			/* TODO: marshal SPDM request */
> > +		} while (status == PCI_TSM_SPDM_REQ);
> > +
> > +		if (status == PCI_TSM_FAIL)
> > +			return -EIO;
> > +		pdev->tsm->state = PCI_TSM_INIT;
> > +	}
> > +	return 0;
> > +}
> > +
> > +static int pci_tsm_connect(struct pci_dev *pdev)
> > +{
> > +	struct pci_tsm_req *req __free(req_free) = NULL;
> 
> Push down a few lines to put it where the allocation happens.
> 
> > +
> > +	/* opportunistic state checks to skip allocating a request */
> > +	if (pdev->tsm->state >= PCI_TSM_CONNECT)
> > +		return 0;
> > +
> > +	req = tsm_ops->req_alloc(pdev, PCI_TSM_OP_CONNECT);
> > +	if (!req)
> > +		return -ENOMEM;
> > +
> > +	scoped_cond_guard(mutex_intr, return -EINTR, tsm_ops->lock) {
> 
> What could possibly go wrong? Everyone loves scoped_cond_guard ;)

Indeed.

> 
> > +		enum pci_tsm_op_status status;
> > +
> > +		/* revalidate state */
> > +		if (pdev->tsm->state >= PCI_TSM_CONNECT)
> > +			return 0;
> > +		if (pdev->tsm->state < PCI_TSM_INIT)
> > +			return -ENXIO;
> > +
> > +		do {
> > +			status = tsm_ops->exec(pdev, req);
> > +			req->seq++;
> > +		} while (status == PCI_TSM_SPDM_REQ);
> > +
> > +		if (status == PCI_TSM_FAIL)
> > +			return -EIO;
> > +		pdev->tsm->state = PCI_TSM_CONNECT;
> > +	}
> > +	return 0;
> > +}
> 
> ...
> 
> > + size_t connect_mode_store(struct device *dev,
> > +				  struct device_attribute *attr,
> > +				  const char *buf, size_t len)
> > +{
> > +	struct pci_dev *pdev = to_pci_dev(dev);
> > +	int i;
> > +
> > +	guard(mutex)(tsm_ops->lock);
> > +	if (pdev->tsm->state >= PCI_TSM_CONNECT)
> > +		return -EBUSY;
> > +	for (i = 0; i < ARRAY_SIZE(pci_tsm_modes); i++)
> > +		if (sysfs_streq(buf, pci_tsm_modes[i]))
> > +			break;
> > +	if (i == PCI_TSM_MODE_LINK) {
> > +		if (pdev->tsm->link_capable)
> > +			pdev->tsm->mode = PCI_TSM_MODE_LINK;
> 
> Error in all real paths paths?

Uh... yeah, did I mention that this untested. Will fix.

> Also, maybe a switch will be more sensible here.

Sure.

> 
> > +		return -EOPNOTSUPP;
> > +	} else if (i == PCI_TSM_MODE_SELECTIVE) {
> > +		if (pdev->tsm->selective_capable)
> > +			pdev->tsm->mode = PCI_TSM_MODE_SELECTIVE;
> > +		return -EOPNOTSUPP;
> > +	} else
> > +		return -EINVAL;
> > +	return len;
> > +}
> 
> 
> > +
> > +void pci_tsm_init(struct pci_dev *pdev)
> > +{
> > +	u16 ide_cap;
> > +	int rc;
> > +
> > +	if (!pdev->cma_capable)
> > +		return;
> > +
> > +	ide_cap = pci_find_ext_capability(pdev, PCI_EXT_CAP_ID_IDE);
> > +	if (!ide_cap)
> > +		return;
> > +
> > +	struct pci_tsm *tsm __free(kfree) = kzalloc(sizeof(*tsm), GFP_KERNEL);
> > +	if (!tsm)
> > +		return;
> > +
> > +	tsm->ide_cap = ide_cap;
> > +
> > +	rc = xa_insert(&pci_tsm_devs, (unsigned long)pdev, pdev, GFP_KERNEL);
> 
> This is an unusual pattern with the key and the value matching.
> Feels like xarray might for once not be the best choice of structure.

This is the "use xarray instead of a linked list patterni". It would be
useful to maybe make the key be the Segment+BDF, but I did not take the
time to figure out if that can fit in an unsigned long. In the meantime
this saves needing to embed a linked list node in 'struct pci_dev'.

[..]
> > diff --git a/drivers/virt/coco/tsm/class.c b/drivers/virt/coco/tsm/class.c
> > index a569fa6b09eb..a459e51c0892 100644
> > --- a/drivers/virt/coco/tsm/class.c
> > +++ b/drivers/virt/coco/tsm/class.c
> 
> > +static const struct attribute_group *tsm_attr_groups[] = {
> > +#ifdef CONFIG_PCI_TSM
> > +	&tsm_pci_attr_group,
> > +#endif
> > +	NULL,
> Trivial but, no point in that comma as we will never chase it with anything.
> > +};
> > +
> >  static int __init tsm_init(void)
> >  {
> >  	tsm_class = class_create("tsm");
> > @@ -86,6 +97,7 @@ static int __init tsm_init(void)
> >  		return PTR_ERR(tsm_class);
> >  
> >  	tsm_class->dev_release = tsm_release;
> > +	tsm_class->dev_groups = tsm_attr_groups;
> >  	return 0;
> >  }
> >  module_init(tsm_init)
> > diff --git a/drivers/virt/coco/tsm/pci.c b/drivers/virt/coco/tsm/pci.c
> > new file mode 100644
> > index 000000000000..b3684ad7114f
> > --- /dev/null
> > +++ b/drivers/virt/coco/tsm/pci.c
> 
> > +
> > +static bool tsm_pci_group_visible(struct kobject *kobj)
> > +{
> > +	struct device *dev = kobj_to_dev(kobj);
> > +	struct tsm_subsys *subsys = container_of(dev, typeof(*subsys), dev);
> Give this a macro probably.
>  dev_to_tsm_subsys(kobj_to_dev(kobj));

Sure.

> 
> > +
> > +	if (subsys->info->pci_ops)
> > +		return true;
> 
> 	return subsys->info->pci->ops;
> maybe

True, maybe with a !! for good measure.

[..]
> > diff --git a/drivers/virt/coco/tsm/tsm.h b/drivers/virt/coco/tsm/tsm.h
> > new file mode 100644
> > index 000000000000..407c388a109b
> > --- /dev/null
> > +++ b/drivers/virt/coco/tsm/tsm.h
> > @@ -0,0 +1,28 @@
> > +/* SPDX-License-Identifier: GPL-2.0 */
> > +#ifndef __TSM_CORE_H
> > +#define __TSM_CORE_H
> > +
> > +#include <linux/device.h>
> > +
> > +struct tsm_info;
> > +struct tsm_subsys {
> > +	struct device dev;
> > +	const struct tsm_info *info;
> > +};
> 
> I know people like to build up patch sets, but I think defining this here
> from patch 3 would just reduce noise.

Ok.

> 
> > +
> > +#ifdef CONFIG_PCI_TSM
> > +int tsm_pci_init(const struct tsm_info *info);
> > +void tsm_pci_destroy(const struct tsm_info *info);
> > +extern const struct attribute_group tsm_pci_attr_group;
> > +#else
> > +static inline int tsm_pci_init(const struct tsm_info *info)
> > +{
> > +	return 0;
> > +}
> > +static inline void tsm_pci_destroy(const struct tsm_info *info)
> > +{
> > +}
> > +#endif
> > +
> > +#endif /* TSM_CORE_H */
> 
> > diff --git a/include/linux/tsm.h b/include/linux/tsm.h
> > index 8cb8a661ba41..f5dbdfa65d8d 100644
> > --- a/include/linux/tsm.h
> > +++ b/include/linux/tsm.h
> > @@ -4,11 +4,15 @@
> >  
> >  #include <linux/sizes.h>
> >  #include <linux/types.h>
> > +#include <linux/mutex.h>
> 
> struct mutex;
> instead given you only have a pointer I think.

True, but see below I expect this lock to move somewhere else in the
next version.

> 
> 
> >  
> >  struct tsm_info {
> >  	const char *name;
> >  	struct device *host;
> >  	const struct attribute_group **groups;
> > +	const struct tsm_pci_ops *pci_ops;
> > +	unsigned int nr_selective_streams;
> > +	unsigned int link_stream_capable:1;
> >  };
> 
> 
> > +struct pci_dev;
> > +/**
> > + * struct tsm_pci_ops - Low-level TSM-exported interface to the PCI core
> > + * @add: accept device for tsm operation, locked
> > + * @del: teardown tsm context for @pdev, locked
> > + * @req_alloc: setup context for given operation, unlocked
> > + * @req_free: teardown context for given request, unlocked
> > + * @exec: run @req, may be invoked multiple times per @req, locked
> > + * @lock: tsm work is one device and one op at a time
> > + */
> > +struct tsm_pci_ops {
> > +	int (*add)(struct pci_dev *pdev);
> > +	void (*del)(struct pci_dev *pdev);
> > +	struct pci_tsm_req *(*req_alloc)(struct pci_dev *pdev,
> > +					 enum pci_tsm_op op);
> > +	struct pci_tsm_req *(*req_free)(struct pci_tsm_req *req);
> > +	enum pci_tsm_op_status (*exec)(struct pci_dev *pdev,
> > +				       struct pci_tsm_req *req);
> > +	struct mutex *lock;
> 
> Mixing data with what would otherwise be likely to be constant pointers
> probably best avoided.  Maybe wrap the lock in another op instead?

After chatting with Alexey this lock is too coarse and will move to a
per device lock rather than locking the entire interface.