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

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

Alexey Kardashevskiy wrote:
> 
> 
> On 14/8/25 11:40, dan.j.williams@intel.com wrote:
> > Alexey Kardashevskiy wrote:
> >> On 18/7/25 04:33, Dan Williams wrote:
> > [..]
> >>> diff --git a/drivers/pci/tsm.c b/drivers/pci/tsm.c
> >>> new file mode 100644
> >>> index 000000000000..0784cc436dd3
> >>> --- /dev/null
> >>> +++ b/drivers/pci/tsm.c
> > [..]
> >>> +static ssize_t connect_store(struct device *dev, struct device_attribute *attr,
> >>> +			     const char *buf, size_t len)
> >>> +{
> >>> +	struct pci_dev *pdev = to_pci_dev(dev);
> >>> +	const struct pci_tsm_ops *ops;
> >>> +	struct tsm_dev *tsm_dev;
> >>> +	int rc, id;
> >>> +
> >>> +	rc = sscanf(buf, "tsm%d\n", &id);
> >>
> >> Why is id needed here? Are there going to be multiple DSMs per a PCI
> >> device?
> > 
> > The implementation allows for multiple TSMs per platform [1], and you
> > acknowledged this earlier [2] (at least the "no globals" bit).
> > 
> > [1]: http://lore.kernel.org/683f9e141f1b1_1626e1009@dwillia2-xfh.jf.intel.com.notmuch
> 
> Right but I'd think that devices (or, more precisely, PCIe slots) are
> statically assigned to TSMs. A bit hard to imagine 2 TSMs in a system
> and ability to connect some PCI device to either of those. It is not
> impossible but not exactly "painfully simple".

The simple case is the typical case, single TSM. If a platform invents
multiple TSMs then it needs to define a protocol for userspace to figure
out the rules, like "match TSMs to devices by PCIe Segment Number", or
something similar. "Painfully simple" also means not pre-constraining
the ABI just to mitigate future userspace complexity. In the end the
kernel is allowed to not need / have an opinion about this detail.

> > [2]: http://lore.kernel.org/b281b714-5097-4b3a-9809-7bdcb9e004dc@amd.com
> > 
> > One of the nice properties of multiple tsm_devs is the ability to unit test
> > host and guest side TSM flows in the same kernel image.
> > 
> >> I am missing the point of tsm_dev. It does not have sysfs nodes (the
> >> pci_dev parent does)
> > 
> > The resource accounting symlinks for each each IDE stream point to the
> > tsm_dev, see tsm_ide_stream_register().
> > 
> >> tsm_register() takes attribute_group but what would posibbly go there?
> > 
> > Any vendor specific implementation of commonly named attributes.
> > Contrast that with vendor specific attributes with vendor specific names
> > that the vendor specific device publishes.
> > 
> >> certificates/meas/report blobs?
> > 
> > Perhaps.
> 
> Hm. Those groups are per a TSM so no device's certificates/meas/report blobs there, right?

True, I was thinking of a per-device TSM driver supplied attributes
similar to 'struct device_driver'::dev_groups. Both that, and this
@groups parameter to tsm_register() can wait until a solid use case
arrives.