Alexey Kardashevskiy wrote:
>
> On 27/8/25 13:51, Dan Williams wrote:
>
> [...]
>
> > +static int pci_tsm_connect(struct pci_dev *pdev, struct tsm_dev *tsm_dev)
> > +{
> > + int rc;
> > + struct pci_tsm_pf0 *tsm_pf0;
> > + const struct pci_tsm_ops *ops = tsm_pci_ops(tsm_dev);
> > + struct pci_tsm *pci_tsm __free(tsm_remove) = ops->probe(pdev);
> > +
> > + /* connect() mutually exclusive with subfunction pci_tsm_init() */
> > + lockdep_assert_held_write(&pci_tsm_rwsem);
> > +
> > + if (!pci_tsm)
> > + return -ENXIO;
> > +
> > + pdev->tsm = pci_tsm;
> > + tsm_pf0 = to_pci_tsm_pf0(pdev->tsm);
> > +
> > + /* mutex_intr assumes connect() is always sysfs/user driven */
> > + ACQUIRE(mutex_intr, lock)(&tsm_pf0->lock);
> > + if ((rc = ACQUIRE_ERR(mutex_intr, &lock)))
> > + return rc;
> > +
> > + rc = ops->connect(pdev);
> > + if (rc)
> > + return rc;
> > +
> > + pdev->tsm = no_free_ptr(pci_tsm);
> > +
> > + /*
> > + * Now that the DSM is established, probe() all the potential
> > + * dependent functions. Failure to probe a function is not fatal
> > + * to connect(), it just disables subsequent security operations
> > + * for that function.
> > + */
> > + pci_tsm_walk_fns(pdev, probe_fn, pdev);
> > + return 0;
> > +}
> > +
> > +static ssize_t connect_show(struct device *dev, struct device_attribute *attr,
> > + char *buf)
> > +{
> > + struct pci_dev *pdev = to_pci_dev(dev);
> > + int rc;
> > +
> > + ACQUIRE(rwsem_read_intr, lock)(&pci_tsm_rwsem);
> > + if ((rc = ACQUIRE_ERR(rwsem_read_intr, &lock)))
> > + return rc;
> > +
> > + if (!pdev->tsm)
> > + return sysfs_emit(buf, "\n");
> > +
> > + return sysfs_emit(buf, "%s\n", tsm_name(pdev->tsm->ops->owner));
> > +}
> > +
> > +/* Is @tsm_dev managing physical link / session properties... */
> > +static bool is_link_tsm(struct tsm_dev *tsm_dev)
> > +{
> > + const struct pci_tsm_ops *ops = tsm_pci_ops(tsm_dev);
> > +
> > + return ops && ops->link_ops.probe;
> > +}
> > +
> > +/* ...or is @tsm_dev managing device security state ? */
> > +static bool is_devsec_tsm(struct tsm_dev *tsm_dev)
> > +{
> > + const struct pci_tsm_ops *ops = tsm_pci_ops(tsm_dev);
> > +
> > + return ops && ops->devsec_ops.lock;
> > +}
> > +
> > +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);
> > + struct tsm_dev *tsm_dev;
> > + int rc, id;
> > +
> > + rc = sscanf(buf, "tsm%d\n", &id);
> > + if (rc != 1)
> > + return -EINVAL;
> > +
> > + ACQUIRE(rwsem_write_kill, lock)(&pci_tsm_rwsem);
> > + if ((rc = ACQUIRE_ERR(rwsem_write_kill, &lock)))
> > + return rc;
> > +
> > + if (pdev->tsm)
> > + return -EBUSY;
>
>
> In one of my previous RFC, I had an IDE key refresh call and it's been
> suggested [1] to ditch that and use connect() instead and the clause
> above prevents it. I am hacking something around this anyway (need to
> validate the PSP support for it) and may be this may be generalized
> now rather than later. Thanks,
When I recommended reuse "connect" I was thinking about kernel internal
helper calls ->connect() again and have the low-level TSM driver be
responsible for determining the difference. IDE Key Refresh deserves its
own follow-on patch set to layout assumptions and tradeoffs between:
* core helper that calls ->connect() again
* core helper that calls a new ->refresh()
* no core helper, TSM drivers handle locally. Local because the refresh
policy might by dynamically negotiated per TSM-arch/DSM-device pairing
and the core is not in a good position to drive that policy.
