Re: [PATCH v3 12/13] PCI/TSM: support TDI related operations for host TSM driver

[Dan Williams <dan.j.williams@intel.com>]

Alexey Kardashevskiy wrote:
> 
> 
> On 7/6/25 11:56, Dan Williams wrote:
> > Alexey Kardashevskiy wrote:
> > [..]
> >>> but the point is still that we
> >>> have not even got one implementation of a bus Device Security protocol
> >>> upstream, let alone multiple.
> >>
> >> And my point is that TSM does not actually do anything with PCI except
> >> SPDM/DOE which can happily live in a library or DOE (and called from
> >> CCP or TDX drivers) and the rest can be just "device", not "pci_dev".
> >> I wonder if+how nailing TSM to PCI makes your life somehow easier, it
> >> is not going to help my case. Thanks,
> > 
> > The goal is not to solve Alexey's case, the goal is to solve the TDISP
> > enabling problem in a way that all impacted subsystem owners (PCI,
> > Device core, DMA, IOMMU, VFIO/IOMMUFD, KVM, CPU arch/...), and all TSM
> > platform vendors are willing to accept.
> 
> Right, so I need to understand how this TSM makes your life easier.

The goal is make other subsystem owner's lives easier.

I read Bjorn's ack as "oh hey, you added new PCI/TSM functionality in
the same way all other new PCI device capabilities get added with
lifetime bounded by pci_init_capabilities() and pci_destroy_dev(), and
the sysfs attributes follow normal PCI device rules too". I also expect
I have an implied ack from Lukas who also does the same lifetime for
native authentication.

Now, they are free to change their mind if new information makes them
rethink this, but I have been operating under the assumption this
question is settled. I read ARM folks as tentatively on board as well.
Greg did not balk yet either.

I expect other subsystem maintainers want to see vendor consensus before
weighing. "Ongoing debate" almost always == "wait".

> I did show my complete solution, still waiting to see yours or any
> other really.

This is a fair criticism and I hope to share more soon in the meantime
Yilun and I have been working on the skeleton pieces and samples/devsec/
to unit test the proposals.

> For example, DOE bouncing.

The tsm operations and bind have been taking up all of the focus. What
are the specific concerns around DOE bouncing proposal? The only
criteria I currently have in mind is:

* if there is common boilerplate lets make that a library helper
* a TDISP operation / state change should be atomic regardless of how
  many DOE messages are involved.

> > "TSM" is literally a PCI-introduced term. It comes with a full
> > device-model and state machine for a protocol that we, OS practitioners,
> > have a chance to agree what it means. If another bus wants to do "Device
> > Security" ideally it would map as a strict subset of the TDISP model. If
> > / when that happens it is easy enough for userspace to see "oh hey, bus
> > $foo now has tsm/connect and tsm/accept mechanisms too".
> 
> Quite a chunk of it is in the SPDM specs which have all sorts of bindings. No strict PCI.

Agree, but most of the interesting potential for shared code there is
buried within the TSM implementations with TDISP we mostly sit behind
TSM calls.

I note that Lukas has spdm common code in lib/spdm/, but all of protocol
work there is not usable for TDISP since TSMs run that protocol.

> VFIO started as PCI and look at it now with all these platform and mediated devices.

Yes, and VFIO still has specific support for PCI semantics... but the
meat of your concern is below.

>   > Just like the evolution of the "new_id / remove_id, and authorized" bus
> > attributes, other buses add workalike functionality as a matter of
> > course. Other buses can add "TSM" mechanisms to their device model,
> > "TSM" is not a device model unto itself. Similarly, nothing stops
> > 'struct pci_dev' properties to be promoted to 'struct device' when
> > needed.
> > 
> > I note IOMMMUFD has the bulk of all the interesting cross-bus shared
> > work to do here and it already has a generic device object model for
> > that purpose. It is another example of "extend existing objects with
> > 'Device Security' properties", not "add a new tdi_dev object to manage".
> > 
> > I am frustrated that we are still spinning in this philosophical debate.
> 
> That's because I did not do very good job explaining my TSM, my bad,
> I'll do better, it is too bloated now, and violates sysfs, and should
> integrate with Lukas'es work, my bad.

No need to apologize, this stuff is complicated and I needed significant
changes from v1 to v2, getting better for v3, but v4 still needed.

> But having this all in a single built-in (1) with PCI nailed down (2),
> globals (3), one tsm_ops struct for both host and guest - this
> frustrates me.

These are good review comments, and need to be addressed.

> (1) means annoyingly many reboots vs rmmod+modprobe

Once you buy the idea that a PCI device capability should have lifetime
bounded by pci_init_capabilities()/pci_destroy_dev() and attributes
defined by pci_dev_attr_groups() then the built-in design comes along
for the ride.

I do not agree that this near term developer ergonomics issue, for this
core infrastructure piece that will eventually settle and be slow moving
thereafter, should affect the long term design.

That was also another motivation for samples/devsec/, i.e. stand up an
environment that can quickly target different corners of the TSM stack
by restarting QEMU.

> (2) TSM does not IDE (the platform calls the IDE library) and does not
> do DOE (the DOE library should, called by the platform)

The rationale for this organization is that IDE is not required. TSM may
have knowledge that the link is secure by other means.

> (3) bites every time when there are development bugs

I take this as par for the course for PCI capability development. I also
expect the bulk of the development complexity to live in the low-level
TSM driver, not the TSM skeleton.

> (4) leads to ugly "if (tvm_mode())" checks and bugs (when missed), been there, done that with my first TSM, did not like it.

Fair enough. I will take a look at replacing tvm_mode() with guest
specific ops structure. It likely still ends up being the same ops
signature though.

> 1/2/3/5 are not necessary, do not really make anything simpler and
> most likely will requite untangling later.
> 
> 
> Say, there are assumptions already made for IDE which I believe we do
> not have to make (like, same number association blocks in all streams)
> but it is internal IDE detail, can be changed later if needed, but the
> API is sane so I am ok with the limitations (thanks btw!). But the TSM
> just is not there yet imho. Hope it all makes sense. Trying now to
> move to v6.16-rc1 + dmabuf + this series as we speak

Which dmabuf series are you trying to integrate? There is yours,
Yilun's, and Aneesh was looking to publish a third.

> so you'll hear from me soon :) Thanks,

Sounds good, the whole goal of this is get to the point where 2 vendors
agree and they can drag the 3rd vendor along.