Re: [PATCH v2 00/11] PCI/TSM: Core infrastructure for PCI device security (TDISP)

[Zhi Wang <zhiw@nvidia.com>]

On Mon, 03 Mar 2025 23:14:14 -0800
Dan Williams <dan.j.williams@intel.com> wrote:

Hi Dan,

Thanks for the patches!

I tested this patch series along with the example devices you provided,
and it works well. The current design and interface reflect the outcomes
of our numerous discussions during kernel SIG meetings with Gobi and
our brainstorm sessions at tech summits.

From my perspective, it should fit most modern TDISP devices, such as
GPUs and other accelerators, although the TDI bind interfaces are
still under discussion.

It's great to see this patch series progressing from the early RFC stage
to a formal merge request, definitely a significant step for bringing
TDISP support is in the mainline and benefits end-users.

It would be helpful if a minimal (my gut feeling would be it could be
very much basically functional, e.g. support one device and one IDE
stream, no advanced features) Intel TSM driver could be added later. So
that we can validate this framework basically on the Intel platform, and
confidently gives an Acked-by:

Z.

> Changes since v1 [1]:
>  - [configfs-tsm: Namespace TSM report symbols]
>    - collect tags
>  - [coco/guest: Move shared guest CC infrastructure to
> drivers/virt/coco/guest/]
>    - collect tags
>  - [coco/tsm: Introduce a core device for TEE Security Managers]
>    - Rename 'tsm_subsys' => 'tsm_core_dev' (Jonathan)
>  - [PCI/IDE: Enumerate Selective Stream IDE capabilities]
>    - Fix the reference PCIe 6.2 specification chapter to 7.9.26
> (Bjoen)
>    - Treat all specification terms as proper nouns, like "Stream ID"
> (Bjorn)
>    - Rename PCI_IDE_LINK_STREAM to PCI_IDE_LINK_STREAM_0 to indicate
>      first of a series (Jonathan)
>    - Stop saving sel_ide_cap in pci_dev as it is not a capability
> block (Jonathan)
>    - Add support for the "Configuration cycles over Selective Stream"
>      mechanism (Alexey, Jonathan)
>    - Cache the number of Link Stream register blocks in pci_dev to
> save IDE capability re-reads
>    - Clarify 'from Endpoint to Root Port' comment in pci_ide_init()
>      (Jonathan)
>    - Fix "Number of Selective IDE Streams Supported" 1-based field
>      interpretation (Aneesh, Yilun, Jonathan)
>    - Switch all register mask definitions to use __GENMASK() to fix
>      bugs, cleanup readability, and support usage of
> FIELD_{PREP,GET}() in ide.c (Alexey, Jonathan, Yilun, Aneesh)
>  - [PCI/TSM: Authenticate devices via platform TSM]
>    - Line wrap documentation, and fixup fidelity to specification
>      terminology (Bjorn)
>    - Prepare for calling tsm_ops->probe() for Physical Functions
> beyond 0 and Virtual Functions, introduce 'struct pci_tsm_pf0' as the
>      object to wrap 'struct pci_tsm' in the Physical Function 0 case.
>      Call tsm_ops->probe() and tsm_ops->remove() for all functions on
> a device if Physical Function 0 sets pdev->tsm. (Yilun, Aneesh)
>    - Drop the complicated 'struct pci_dsm' scheme (Alexey)
>    - Fix tsm->state validation, 'init before connect' (Yilun)
>    - Move on from if_not_guard(), but not onto the whitespace column
>      pressure of scoped_cond_guard() (Jonathan)
>    - Rename pci_tsm_register() pci_tsm_core_register() to disambiguate
>      from device init in pci_tsm_init() (Jonathan)
>  - [samples/devsec: Introduce a PCI device-security bus + endpoint
> sample]
>    - Fix CONFIG_VIRT_DRIVERS=n compilation dependency (0day Kbuild
> Robot)
>    - Switch from a single devm action to remove emulated devices and
>      ports to a per-device / per-port scheme (Jonathan)
>    - Fix "Number of Selective IDE Streams Supported"
>    - Use devm_gen_pool_create() (Jonathan)
>  - [PCI: Add PCIe Device 3 Extended Capability enumeration]
>  - [PCI/IDE: Add IDE establishment helpers]
>    - Drop PCI_IDE_SETUP_ROOT_PORT and its related complications. Push
>      Root Port programming responsibility to leaf drivers. (Alexey,
>      Jonathan, Bjorn)
>    - Clarify that some TSM technologies do not allow system-software
> to allocate the Stream ID (Aneesh)
>    - Fundamentally rework the API to stop tying the Stream ID to the
>      Endpoint register block index, the Root Port register block
> index, and the platform stream slot. Add pci_ide_strem_alloc() to grab
>      those resources and clarify that Stream IDs only need to be
> unique within a Partner Port pairing. The 'struct pci_ide' object is
>      updated accordingly to carry all the Partner Port details.
> (Alexey, Jonathan, Aneesh)
>    - Add kernel-doc commentary for all exported APIs (Bjorn)
>    - Miscellaneous specific terminology fixups and pci.h comment
>      cleanups (Bjorn)
>    - Drop address association setup for now given the questions around
>      its value (Alexey, Yilun)
>    - Switch from "devid" to "RID" to match specification language,
> add a comment to address the discrepancy in Linux terms vs PCIe spec
>      terms (Bjorn)
>    - Setup RID association registers relative to which RIDs are seen
> at either Partner Port (Yilun, Alexey)
>  - [PCI/IDE: Report available IDE streams]
>    - Rename pci_set_nr_ide_streams() to pci_ide_init_nr_streams() to
>      clarify why this one symbols is in the "PCI_IDE" symbol namespace
>      since PCI init code is typically built-in. (Alexey)
>    - Fix missing quotes in usage of EXPORT_SYMBOL_NS_GPL() and
>      MODULE_IMPORT() (Alexey)
>  - [PCI/TSM: Report active IDE streams]
>    - Documentation fixups (Bjorn)
>    - Rename tsm_register_ide_stream() to tsm_ide_stream_register() for
>      naming consistency
>    - Reflect that the format of the stream link changed from:
>      pciDDDD:BB/streamN:DDDD:BB:DD:F
>      ...to:
>      pciDDDD:BB/streamH.R.E:DDDD:BB:DD:F
>  - [samples/devsec: Add sample IDE establishment]
>    - Mirror the devsec_tsm_disconnect() sequence in the
>      devsec_tsm_connect() error unwind path (Jonathan)
>    - Other miscellaneous symmetry on error unwind fixups (Jonathan)
> 
> [1]:
> http://lore.kernel.org/173343739517.1074769.13134786548545925484.stgit@dwillia2-xfh.jf.intel.com
> 
> ---
> Towards devsec-next:
> 
> As evidenced by a full page of change notes from v1 to v2 there is
> multi-party interest in this core infrastructure, and more
> importantly, many small details to negotiate. That number of details
> to negotiate only increases with the follow-on "device bind" flows
> and the interactions across VFIO, IOMMUFD and KVM.
> 
> I expect it will continue to be the case that the mainline ingestion
> rate of all this infrastructure results in several more cycles before
> mainline ships a complete solution for one or more vendors. In the
> meantime, I am looking to run a devsec-next integration tree for
> kernel and QEMU. That is, a supplemental staging tree to enable
> end-to-end testing while proposals make their way upstream. For now,
> consider sending a branch and I will aim to do periodic octopus
> merges of submitted branches on top of a kvm-coco-queue + devsec-core
> baseline.
> 
> The main motivation for a "devsec-next" tree, as I mentioned to some
> in the hallway track at Plumbers, is to wrangle private hacks and
> workarounds in vendor trees to coalesce if not mature.  An example of
> multiple vendors solving the same problem in different ways in their
> vendor trees is: [2] vs [3]. Note that devsec-next is not intended to
> replace vendor trees, and instead reflect the snapshot state of
> cross-vendor consensus before topics are ready for linux-next /
> mainline.
> 
> I will send out more details as a follow up.
> 
> [2]: https://github.com/aik/qemu/commit/5256c41f
> [3]:
> http://lore.kernel.org/20250217081833.21568-1-chenyi.qiang@intel.com
> 
> ---
> Original Cover letter:
> 
> Trusted execution environment (TEE) Device Interface Security Protocol
> (TDISP) is a chapter name in the PCI specification. It describes an
> alphabet soup of mechanisms, SPDM, CMA, IDE, TSM/DSM, that system
> software uses to establish trust in a device and assign it to a
> confidential virtual machine (CVM). It is protocol for dynamically
> extending the trusted computing boundary (TCB) of a CVM with a PCI
> device interface that can issue DMA to CVM private memory.
>    
> The acronym soup problem is enhanced by every major platform vendor
> having distinct TEE Security Manager (TSM) API implementations /
> capabilities, and to a lesser extent, every potential endpoint Device
> Security Manager (DSM) having its own idiosyncratic behaviors around
> TDISP state transitions. 
>      
> Despite all that opportunity for differentiation, there is a
> significant portion of the implementation that is cross-vendor
> common. However, it is difficult to develop, debate, test and settle
> all those pieces absent a low level TSM driver implementation to pull
> it all together. 
> The proposal is incrementally develop the shared infrastructure on top
> of a sample TSM driver implementation to enable clean vendor agnostic
> discussions about the commons. "samples/devsec/" is meant to be: just
> enough emulation to exercise all the core infrastructure, a reference
> implementation, and a simple unit test. The sample also enables
> coordination with the native PCI device security effort [4].
>    
> The devsec_tsm driver already yielding benefits as it drove many of
> the fixes and enhancements of this patch-kit relative to the last RFC
> [1]. Future development would either reuse established devsec_tsm
> paths, or extend the sample alongside the vendor-specific
> implementation. 
> This first batch is just enough infrastructure for IDE (link Integrity
> and Data Encryption) establishment via TSM APIs. It is based on a
> review and curation of the IDE establishment flows from the SEV-TIO
> RFC [5] and a work-in-progress TDX Connect RFC (see the
> Co-developed-by and thanks yous in the changelogs for where code was
> copied).
> 
> It deliberately avoids SPDM details and does not touch upon the "bind"
> flows, or guest-side flows, simply to allow for upstream digestion of
> all the assumptions and tradeoffs for the "simple" IDE establishment
> baseline.
> 
> Note that devsec_tsm is for near term staging of vendor TSM
> implementations. The expectation is that every piece of new core
> infrastructure that devsec_tsm consumes must also have a vendor TSM
> driver consumer within 1 to 2 kernel development cycles.
> 
> The full series is available via devsec/tsm.git [6].
> 
> [4]: http://lore.kernel.org/cover.1719771133.git.lukas@wunner.de
> [5]: http://lore.kernel.org/20240823132137.336874-1-aik@amd.com
> [6]:
> https://git.kernel.org/pub/scm/linux/kernel/git/devsec/tsm.git/log/?h=devsec-20250303
> 
> ---
> 
> Dan Williams (11):
>       configfs-tsm: Namespace TSM report symbols
>       coco/guest: Move shared guest CC infrastructure to
> drivers/virt/coco/guest/ coco/tsm: Introduce a core device for TEE
> Security Managers PCI/IDE: Enumerate Selective Stream IDE capabilities
>       PCI/TSM: Authenticate devices via platform TSM
>       samples/devsec: Introduce a PCI device-security bus + endpoint
> sample PCI: Add PCIe Device 3 Extended Capability enumeration
>       PCI/IDE: Add IDE establishment helpers
>       PCI/IDE: Report available IDE streams
>       PCI/TSM: Report active IDE streams
>       samples/devsec: Add sample IDE establishment
> 
> 
>  Documentation/ABI/testing/configfs-tsm-report      |    0 
>  Documentation/ABI/testing/sysfs-bus-pci            |   45 +
>  Documentation/ABI/testing/sysfs-class-tsm          |   20 +
>  .../ABI/testing/sysfs-devices-pci-host-bridge      |   44 +
>  MAINTAINERS                                        |   10 
>  drivers/pci/Kconfig                                |   37 +
>  drivers/pci/Makefile                               |    2 
>  drivers/pci/ide.c                                  |  499
> ++++++++++++++ drivers/pci/pci-sysfs.c                            |
>  4 drivers/pci/pci.h                                  |   19 +
>  drivers/pci/probe.c                                |   26 +
>  drivers/pci/remove.c                               |    3 
>  drivers/pci/tsm.c                                  |  377 +++++++++++
>  drivers/virt/coco/Kconfig                          |    8 
>  drivers/virt/coco/Makefile                         |    3 
>  drivers/virt/coco/arm-cca-guest/arm-cca-guest.c    |    8 
>  drivers/virt/coco/guest/Kconfig                    |    7 
>  drivers/virt/coco/guest/Makefile                   |    3 
>  drivers/virt/coco/guest/report.c                   |   32 -
>  drivers/virt/coco/host/Kconfig                     |    6 
>  drivers/virt/coco/host/Makefile                    |    6 
>  drivers/virt/coco/host/tsm-core.c                  |  144 ++++
>  drivers/virt/coco/sev-guest/sev-guest.c            |   12 
>  drivers/virt/coco/tdx-guest/tdx-guest.c            |    8 
>  include/linux/pci-ide.h                            |   60 ++
>  include/linux/pci-tsm.h                            |  135 ++++
>  include/linux/pci.h                                |   25 +
>  include/linux/tsm.h                                |   33 +
>  include/uapi/linux/pci_regs.h                      |   89 +++
>  samples/Kconfig                                    |   16 
>  samples/Makefile                                   |    1 
>  samples/devsec/Makefile                            |   10 
>  samples/devsec/bus.c                               |  698
> ++++++++++++++++++++ samples/devsec/common.c
>   |   26 + samples/devsec/devsec.h                            |    7 
>  samples/devsec/tsm.c                               |  192 ++++++
>  36 files changed, 2564 insertions(+), 51 deletions(-)
>  rename Documentation/ABI/testing/{configfs-tsm =>
> configfs-tsm-report} (100%) create mode 100644
> Documentation/ABI/testing/sysfs-class-tsm create mode 100644
> Documentation/ABI/testing/sysfs-devices-pci-host-bridge create mode
> 100644 drivers/pci/ide.c create mode 100644 drivers/pci/tsm.c
>  create mode 100644 drivers/virt/coco/guest/Kconfig
>  create mode 100644 drivers/virt/coco/guest/Makefile
>  rename drivers/virt/coco/{tsm.c => guest/report.c} (93%)
>  create mode 100644 drivers/virt/coco/host/Kconfig
>  create mode 100644 drivers/virt/coco/host/Makefile
>  create mode 100644 drivers/virt/coco/host/tsm-core.c
>  create mode 100644 include/linux/pci-ide.h
>  create mode 100644 include/linux/pci-tsm.h
>  create mode 100644 samples/devsec/Makefile
>  create mode 100644 samples/devsec/bus.c
>  create mode 100644 samples/devsec/common.c
>  create mode 100644 samples/devsec/devsec.h
>  create mode 100644 samples/devsec/tsm.c
> 
> base-commit: 7eb172143d5508b4da468ed59ee857c6e5e01da6
>