Re: [PATCH 05/11] PCI/TSM: Authenticate devices via platform TSM

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 





On 22/2/25 07:42, Dan Williams wrote:
Alexey Kardashevskiy wrote:
On 6/12/24 09:23, 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 Component Measurement and
Authentication (CMA), and link Integrity and Data Encryption (IDE). It
adds support for assigning devices (PCI physical or virtual function) 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 "DSM" (Device Security Manager) and
system software in both a VMM and a confidential VM. A VMM uses TSM ABIs
to setup link security and assign devices. A confidential VM uses TSM
ABIs to transition an assigned device into the TDISP "RUN" state and
validate its configuration. 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.

CONFIG_PCI_TSM adds an "authenticated" attribute and "tsm/" subdirectory
to pci-sysfs. The work in progress CONFIG_PCI_CMA (software
kernel-native PCI authentication) that can depend on a local to the PCI
core implementation, CONFIG_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 watch /sys/class/tsm/tsm0/uevent
to know when the PCI core has TSM services enabled.

The common verbs that the low-level TSM drivers implement are defined by
'struct pci_tsm_ops'. For now only 'connect' and 'disconnect' are
defined for secure session and IDE establishment. The 'probe' and
'remove' operations setup per-device context representing the device's
security manager (DSM). Note that there is only one DSM expected per
physical PCI function, and that coordinates a variable number of
assignable interfaces to CVMs.

The locking allows for multiple devices to be executing commands
simultaneously, one outstanding command per-device and an rwsem flushes
all in-flight commands when a TSM low-level driver/device is removed.

Thanks to Wu Hao for his work on an early draft of this support.

Cc: Lukas Wunner <lukas@xxxxxxxxx>
Cc: Samuel Ortiz <sameo@xxxxxxxxxxxx>
Cc: Alexey Kardashevskiy <aik@xxxxxxx>
Acked-by: Bjorn Helgaas <bhelgaas@xxxxxxxxxx>
Co-developed-by: Xu Yilun <yilun.xu@xxxxxxxxxxxxxxx>
Signed-off-by: Xu Yilun <yilun.xu@xxxxxxxxxxxxxxx>
Signed-off-by: Dan Williams <dan.j.williams@xxxxxxxxx>
---
   Documentation/ABI/testing/sysfs-bus-pci |   42 ++++
   MAINTAINERS                             |    2
   drivers/pci/Kconfig                     |   13 +
   drivers/pci/Makefile                    |    1
   drivers/pci/pci-sysfs.c                 |    4
   drivers/pci/pci.h                       |   10 +
   drivers/pci/probe.c                     |    1
   drivers/pci/remove.c                    |    3
   drivers/pci/tsm.c                       |  293 +++++++++++++++++++++++++++++++
   drivers/virt/coco/host/tsm-core.c       |   19 ++

It is sooo small, make me wonder why we need it at all...

I expect it to grow as more common cross-vendor host TSM functionality
is added.

+static int pci_tsm_connect(struct pci_dev *pdev)
+{
+	struct pci_tsm *pci_tsm = pdev->tsm;
+	int rc;
+
+	lockdep_assert_held(&pci_tsm_rwsem);
+	if_not_guard(mutex_intr, &pci_tsm->lock)
+		return -EINTR;
+
+	if (pci_tsm->state >= PCI_TSM_CONNECT)
+		return 0;
+	if (pci_tsm->state < PCI_TSM_INIT)
+		return -ENXIO;
+
+	rc = tsm_ops->connect(pdev);

I thought ages ago it was suggested that DOE/SPDM loop happens in a
common place and not in the platform driver implementing
tsm_ops->connect() (but I may have missed the point then).

That's still the plan, but I would expect that to be a common helper
that TSM drivers can use and does not need to be enforced as a midlayer
detail in pci/tsm.c. We can add that to pci/doe.c or somewhere more
appropriate for SPDM transport helpers.

[..]
diff --git a/include/linux/pci-tsm.h b/include/linux/pci-tsm.h
new file mode 100644
index 000000000000..beb0d68129bc
--- /dev/null
+++ b/include/linux/pci-tsm.h
@@ -0,0 +1,83 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#ifndef __PCI_TSM_H
+#define __PCI_TSM_H
+#include <linux/mutex.h>
+
+struct pci_dev;
+
+/**
+ * struct pci_dsm - Device Security Manager context
+ * @pdev: physical device back pointer
+ */
+struct pci_dsm {
+	struct pci_dev *pdev;
+};
+
+enum pci_tsm_state {
+	PCI_TSM_ERR = -1,
+	PCI_TSM_INIT,
+	PCI_TSM_CONNECT,
+};
+
+/**
+ * struct pci_tsm - Platform TSM transport context
+ * @state: reflect device initialized, connected, or bound
+ * @lock: protect @state vs pci_tsm_ops invocation
+ * @doe_mb: PCIe Data Object Exchange mailbox
+ * @dsm: TSM driver device context established by pci_tsm_ops.probe
+ */
+struct pci_tsm {
+	enum pci_tsm_state state;
+	struct mutex lock;
+	struct pci_doe_mb *doe_mb;
+	struct pci_dsm *dsm;
+};

doe_mb and state look are device's attribures so will look more
appropriate in pci_dsm ("d" from "dsm" is "device"), and pci_tsm would
be some intimate knowledge of the ccp.ko (==PSP) about PCI PFs ("t" ==
"TEE" == TCB == PSP). Or I got it all wrong?

I typed up a long reply only to realize I think this can be made simpler
by only having one common context and drop this subtle 'struct pci_dsm'
distinction.

So, 'struct pci_tsm' is just the common core context / handle for
drivers/pci/tsm.c to communicate with low level TSM driver
implementation. It is allocated by pci_tsm_ops->probe() and freed by
pci_tsm_ops->remove().

A low-level TSM driver can optionally wrap that core context with its
own data, i.e. enforce a container_of() relationship between the core
context and the low level context.

My sketch evolved much since I wrote this comment :) I've put the low level TSM bits into CCP.


[..]
diff --git a/include/linux/pci.h b/include/linux/pci.h
index 50811b7655dd..a0900e7d2012 100644
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -535,6 +535,9 @@ struct pci_dev {
   	u16		ide_cap;	/* Link Integrity & Data Encryption */
   	u16		sel_ide_cap;	/* - Selective Stream register block */
   	int		nr_ide_mem;	/* - Address range limits for streams */
+#endif
+#ifdef CONFIG_PCI_TSM
+	struct pci_tsm *tsm;		/* TSM operation state */
   #endif
   	u16		acs_cap;	/* ACS Capability offset */
   	u8		supported_speeds; /* Supported Link Speeds Vector */
diff --git a/include/linux/tsm.h b/include/linux/tsm.h
index 1a97459fc23e..46b9a0c6ea4e 100644
--- a/include/linux/tsm.h
+++ b/include/linux/tsm.h
@@ -111,7 +111,9 @@ struct tsm_report_ops {
   int tsm_report_register(const struct tsm_report_ops *ops, void *priv);
   int tsm_report_unregister(const struct tsm_report_ops *ops);
   struct tsm_subsys;
+struct pci_tsm_ops;
   struct tsm_subsys *tsm_register(struct device *parent,
-				const struct attribute_group **groups);
+				const struct attribute_group **groups,
+				const struct pci_tsm_ops *ops);
   void tsm_unregister(struct tsm_subsys *subsys);
   #endif /* __TSM_H */
diff --git a/include/uapi/linux/pci_regs.h b/include/uapi/linux/pci_regs.h
index 9635b27d2485..19bba65a262c 100644
--- a/include/uapi/linux/pci_regs.h
+++ b/include/uapi/linux/pci_regs.h
@@ -499,6 +499,7 @@
   #define  PCI_EXP_DEVCAP_PWR_VAL	0x03fc0000 /* Slot Power Limit Value */
   #define  PCI_EXP_DEVCAP_PWR_SCL	0x0c000000 /* Slot Power Limit Scale */
   #define  PCI_EXP_DEVCAP_FLR     0x10000000 /* Function Level Reset */
+#define  PCI_EXP_DEVCAP_TEE     0x40000000 /* TEE I/O (TDISP) Support */
   #define PCI_EXP_DEVCTL		0x08	/* Device Control */
   #define  PCI_EXP_DEVCTL_CERE	0x0001	/* Correctable Error Reporting En. */
   #define  PCI_EXP_DEVCTL_NFERE	0x0002	/* Non-Fatal Error Reporting Enable */



I am trying to wrap my head around your tsm. here is what I got in my tree:
https://github.com/aik/linux/blob/tsm/include/linux/tsm.h

Shortly:

drivers/virt/coco/tsm.ko does sysfs (including "connect" and "bind" to
control and "certs"/"report" to attest) and implements tsm_dev/tsm_tdi,
it does not know pci_dev;

drivers/pci/tsm-pci.ko creates/destroys tsm_dev/tsm_dev using tsm.ko;

drivers/crypto/ccp/ccp.ko (the PSP guy) registers:
- tsm_subsys in tsm.ko (which does "connect" and "bind" and
- tsm_bus_subsys in tsm-pci.ko (which does "spdm_forward")
ccp.ko knows about pci_dev and whatever else comes in the future, and
ccp.ko's "connect" implementation calls the IDE library (I am adopting
yours now, with some tweaks).

tsm-dev and tsm-tdi embed struct dev each and are added as children to
PCI devices: no hide/show attrs, no additional TSM pointer in struct
device or pci_dev, looks like:

The motivation for building awareness of device-security properties
natively into 'struct pci_dev' is the recognition that TSM-based
security is not the only model that Linux needs to contend. The TSM
flow is a superset of PCI-CMA and maybe PCI-IDE in the future (although
Intel seems to be the only architecture that has a concept of allowing
IDE establishment without a TSM).

I understand your motivations to make all of TSM functionality bolted
onto the side of the PCI core. It has some nice properties. However, I
think that is a SEV-TIO centric view of the world.

Very true.

PCI device security
attributes are PCI device attributes and have reason to exist with and
without a TSM. In other words, certificates and measurements should not
be placed behind a TSM ABI because certificates and measurements are
expected to have a native PCI-CMA ABI.

The Lukas'es CMA ABI should just work on AMD, without any TSM. I am not sure if anyone wants CMA bits from the PSP when can be obtained from the device directly.


It would be a useful property if software written to retrieve
measurement and certificate chains did that relative to the PCI dev
independent of TSM presence.

aik@sc ~> ls  /sys/bus/pci/devices/0000:e1:04.0/tsm-tdi/tdi:0000:e1:04.0/
device  power  subsystem  tsm_report  tsm_report_user  tsm_tdi_bind
tsm_tdi_status  tsm_tdi_status_user  uevent

aik@sc ~> ls  /sys/bus/pci/devices/0000:e1:04.0/tsm_dev/
device  power  subsystem  tsm_certs  tsm_cert_slot  tsm_certs_user
tsm_dev_connect  tsm_dev_status  tsm_meas  tsm_meas_user  uevent

aik@sc ~> ls /sys/class/tsm/tsm0/
device  power  stream0:0000:e1:00.0  subsystem  uevent

aik@sc ~> ls /sys/class/tsm-dev/
tdev:0000:c0:01.1  tdev:0000:e0:01.1  tdev:0000:e1:00.0

aik@sc ~> ls /sys/class/tsm-tdi/
tdi:0000:c0:01.1  tdi:0000:e0:01.1  tdi:0000:e1:00.0  tdi:0000:e1:04.0
tdi:0000:e1:04.1  tdi:0000:e1:04.2  tdi:0000:e1:04.3

Right, so I remain unconvinced that Linux needs to contend with new "tsm"
class devs vs PCI device objects with security properties especially
when those security properties have a "TSM" and non-"TSM" flavor.

One of the thoughts was - when we start adding this TDISP to CXL (which is but also is not PCI), this may be handy, may be. Or, I dunno, USB-TDISP. The security objects are described in the PCIe spec now but the concept is generic really. Thanks,


--
Alexey





[Index of Archives]     [DMA Engine]     [Linux Coverity]     [Linux USB]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Greybus]

  Powered by Linux