Re: [PATCH resend v6 04/10] PCI/TSM: Authenticate devices via platform TSM

[Alexey Kardashevskiy <aik@amd.com>]

On 20/9/25 06:15, dan.j.williams@intel.com wrote:
> Alexey Kardashevskiy wrote:
>>
>>
>> On 12/9/25 09:56, Dan Williams wrote:
>>> The PCIe 7.0 specification, section 11, defines the Trusted Execution
>>> Environment (TEE) Device Interface Security Protocol (TDISP).  This
>>> protocol 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. Consider that the TSM driver may itself be a PCI driver.
>>> Userspace can watch for the arrival of a "TSM" device,
>>> /sys/class/tsm/tsm0/uevent KOBJ_CHANGE, to know when the PCI core has
>>> initialized TSM services.
>>>
>>> The operations that can be executed against a PCI device are split into
>>> two mutually exclusive operation sets, "Link" and "Security" (struct
>>> pci_tsm_{link,security}_ops). The "Link" operations manage physical link
>>> security properties and communication with the device's Device Security
>>> Manager firmware. These are the host side operations in TDISP. The
>>> "Security" operations coordinate the security state of the assigned
>>> virtual device (TDI). These are the guest side operations in TDISP. Only
>>> link management operations are defined at this stage and placeholders
>>> provided for the security operations.
>>>
>>> The locking allows for multiple devices to be executing commands
>>> simultaneously, one outstanding command per-device and an rwsem
>>> synchronizes the implementation relative to TSM
>>> registration/unregistration events.
>>>
>>> Thanks to Wu Hao for his work on an early draft of this support.
>>>
>>> Cc: Lukas Wunner <lukas@wunner.de>
>>> Cc: Samuel Ortiz <sameo@rivosinc.com>
>>> Cc: Alexey Kardashevskiy <aik@amd.com>
>>> Acked-by: Bjorn Helgaas <bhelgaas@google.com>
>>> Co-developed-by: Xu Yilun <yilun.xu@linux.intel.com>
>>> Signed-off-by: Xu Yilun <yilun.xu@linux.intel.com>
>>> Signed-off-by: Dan Williams <dan.j.williams@intel.com>
>>> ---
>>>    Documentation/ABI/testing/sysfs-bus-pci |  51 ++
>>>    Documentation/driver-api/pci/index.rst  |   1 +
>>>    Documentation/driver-api/pci/tsm.rst    |  12 +
L>>>    MAINTAINERS                             |   4 +-
>>>    drivers/pci/Kconfig                     |  15 +
>>>    drivers/pci/Makefile                    |   1 +
>>>    drivers/pci/doe.c                       |   2 -
>>>    drivers/pci/pci-sysfs.c                 |   4 +
>>>    drivers/pci/pci.h                       |  10 +
>>>    drivers/pci/probe.c                     |   3 +
>>>    drivers/pci/remove.c                    |   6 +
>>>    drivers/pci/tsm.c                       | 627 ++++++++++++++++++++++++
>>>    drivers/virt/coco/tsm-core.c            |  40 +-
>>>    include/linux/pci-doe.h                 |   4 +
>>>    include/linux/pci-tsm.h                 | 159 ++++++
>>>    include/linux/pci.h                     |   3 +
>>>    include/linux/tsm.h                     |  11 +-
>>>    include/uapi/linux/pci_regs.h           |   1 +
>>
>>
>> A suggestion: "git format-patch -O ~/orderfile ..." produces
>> nicer-to-review order of files especially where there are new
>> interfaces being added.
> 
> Not the first time I have heard this recommendation, finally
> implementing in my flow.
> 
>> ===
>> *.txt
>> configure
>> Kconfig*
>> *Makefile*
>> *.json
>> *.h
>> *.c
>> ===
> 
> Went with this ordering instead:
> 
> Kconfig
> */Kconfig
> */Kconfig.*
> Makefile
> */Makefile
> */Makefile.*
> scripts/*
> Documentation/*
> *.h
> *.S
> *.c
> tools/testing/*
> 
> ...stolen from Kees:
> 
> https://github.com/kees/kernel-tools/commit/909db155

oh this is better, I was so sure I don't need paths. TIL.

  
> [ .. scrolls past pages of uncommented context .. ]
> 
>> It is still a rather global thing. May I suggest this?
> 
> I am not too keen on this.
> 
> Yes, it is global, but less often used compared to @ops, and I do not
> want both @ops and @tsm_dev in @pci_tsm.

Why exactly?

> So the options are lookup @ops
> from @tsm_dev or lookup @tsm_dev from @ops. Given @ops is used more
> often that is how I came up with the current arrangement.

I am looking at:
https://github.com/AMDESE/linux-kvm/commit/9e3caf921ad6ddd6bd860ec307b986649322a618
and not really sure "more often" applies here.

And do we have to check now if tsm_dev passed in probe() is the same as the owner? I struggle to find any other _ops doing the same owner caching easily. Or merge struct pci_tsm_ops into struct tsm_dev to stop pretending that pci_tsm_ops is an interface, and then we won't even need that @owner. Dunno. Aneesh? :)

Thanks,


-- 
Alexey