On 14/8/25 11:40, dan.j.williams@xxxxxxxxx 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@xxxxxxxxxxxxxxxxxxxxxxxxx.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".
[2]: http://lore.kernel.org/b281b714-5097-4b3a-9809-7bdcb9e004dc@xxxxxxx
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?
For now, I want to just focus on the mechanics of the getting a
TDI into the run state. The attestation flow is a separate design debate
one there is consensus on getting the TDI up and running.
The pci_dev struct itself has *tsm now so this child device is not
that. Hm.
This tsm_dev is not a child device it is the common class representation
of a platform capability that can establish SPDM and optionally IDE.
Yeah, I realized that soon after I hit "send".
+ if (rc != 1)
+ return -EINVAL;
+
+ ACQUIRE(rwsem_read_intr, lock)(&pci_tsm_rwsem);
+ if ((rc = ACQUIRE_ERR(rwsem_read_intr, &lock)))
+ return rc;
+
+ if (pdev->tsm)
+ return -EBUSY;
+
+ tsm_dev = find_tsm_dev(id);
When PCI TSM loads, all it does is add "connect" nodes. And when write
to "connect" happens, this find_tsm_dev() is expected to find a
tsm_dev but what is going to add those in the real PCI?
sev_tsm_init() calls tsm_register(). Userspace catches the tsm_dev
KOBJECT_ADD event to run:
echo $TSM_DEV > /sys/bus/pci/devices/$PDEV/tsm/connect
[..]
imho "echo 0 > connect" is more descriptive than "echo 1 > disconnect", and one less sysfs node.
That makes it a bit too ambiguous for my taste as connect is "connect to
a tsm of the following identifier", so, for example, "is '0' a shorthand
for 'tsm0'?"
Nah, ignore my "imho" then. Thanks,
...and as I say that I realize disconnect as the same problem. I will
update disconnect to take the tsm device name just like connect for
symmetry, this ambiguity concern, and in case multiple TSM connections
per device might ever happen way down the road.
[..]
+/**
+ * pci_tsm_constructor() - base 'struct pci_tsm' initialization
+ * @pdev: The PCI device
+ * @tsm: context to initialize
+ * @ops: PCI operations provided by the TSM
+ */
+int pci_tsm_constructor(struct pci_dev *pdev, struct pci_tsm *tsm,
+ const struct pci_tsm_ops *ops)
+{
+ tsm->pdev = pdev;
+ tsm->ops = ops;
These should go down, right before "return 0". Thanks,
Sure, makes sense.
In practice @tsm will be unwound, but might as well not make it a
valid object while it is awaiting to be freed.
--
Alexey
