On Thu, 05 Dec 2024 14:23:45 -0800
Dan Williams <dan.j.williams@xxxxxxxxx> 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.
I don't follow the previous sentence. Perhaps consider a rewrite?
> 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>
A few minor things inline.
Jonathan
> ---
> 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 ++
> include/linux/pci-tsm.h | 83 +++++++++
> include/linux/pci.h | 3
> include/linux/tsm.h | 4
> include/uapi/linux/pci_regs.h | 1
> 14 files changed, 476 insertions(+), 3 deletions(-)
> create mode 100644 drivers/pci/tsm.c
> create mode 100644 include/linux/pci-tsm.h
> diff --git a/drivers/pci/tsm.c b/drivers/pci/tsm.c
> new file mode 100644
> index 000000000000..04e9257a6e41
> --- /dev/null
> +++ b/drivers/pci/tsm.c
> @@ -0,0 +1,293 @@
> +// 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/pci-doe.h>
> +#include <linux/sysfs.h>
> +#include <linux/xarray.h>
> +#include <linux/pci-tsm.h>
> +#include <linux/bitfield.h>
Odd header ordering. Anything consistent is fine by me but
this just feels random.
> +#include "pci.h"
> +
> +/*
> + * 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 pci_tsm_ops *tsm_ops;
> +
> +/* supplemental attributes to surface when pci_tsm_attr_group is active */
> +static const struct attribute_group *pci_tsm_owner_attr_group;
> +
> +static int pci_tsm_disconnect(struct pci_dev *pdev)
> +{
> + struct pci_tsm *pci_tsm = pdev->tsm;
> +
> + lockdep_assert_held(&pci_tsm_rwsem)
> + if_not_guard(mutex_intr, &pci_tsm->lock)
Sadly that got dropped.
> + return -EINTR;
> +
> + if (pci_tsm->state < PCI_TSM_CONNECT)
> + return 0;
> + if (pci_tsm->state < PCI_TSM_INIT)
> + return -ENXIO;
> +
> + tsm_ops->disconnect(pdev);
> + pci_tsm->state = PCI_TSM_INIT;
> +
> + return 0;
> +}
> +
> +static struct attribute *pci_tsm_attrs[] = {
> + &dev_attr_connect.attr,
> + NULL,
Trivia but no comma given it's a terminator and nothing will
ever come after it.
> +};
> +
> +const struct attribute_group pci_tsm_attr_group = {
> + .name = "tsm",
> + .attrs = pci_tsm_attrs,
> + .is_visible = SYSFS_GROUP_VISIBLE(pci_tsm),
> +};
> +
> +static ssize_t authenticated_show(struct device *dev,
> + struct device_attribute *attr, char *buf)
> +{
> + /*
> + * When device authentication is TSM owned, 'authenticated' is
> + * identical to the connect state.
> + */
> + return connect_show(dev, attr, buf);
> +}
> +static DEVICE_ATTR_RO(authenticated);
> +
> +static struct attribute *pci_tsm_auth_attrs[] = {
> + &dev_attr_authenticated.attr,
> + NULL,
no comma
> +};
> +void pci_tsm_destroy(struct pci_dev *pdev)
> +{
> + guard(rwsem_write)(&pci_tsm_rwsem);
> + __pci_tsm_destroy(pdev);
> +}
> +
> +void pci_tsm_unregister(const struct pci_tsm_ops *ops)
I'd try to name things so it is clearer when a function
is about the TSM coming and going vs a particular PCI
device coming and going after the TSM is loaded.
At least that's what I'm assuming is the difference between
pci_tsm_unregister() tsm going vs
pci_tsm_destroy() particular PCI device driver being unbound
(which I don't think gets called, so maybe drop for now?)
> +{
> + struct pci_dev *pdev = NULL;
> +
> + if (!ops)
> + return;
> + guard(rwsem_write)(&pci_tsm_rwsem);
> + if (ops != tsm_ops)
> + return;
> + for_each_pci_dev(pdev)
> + __pci_tsm_destroy(pdev);
> + tsm_ops = NULL;
> +}
> +EXPORT_SYMBOL_GPL(pci_tsm_unregister);
>
