Re: [PATCH 4/6] PCI/TSM: Add pci_tsm_bind() helper for instantiating TDIs

Aneesh Kumar K.V
2025-11-05T04:59:54Z

Dan Williams <dan.j.williams@intel.com> writes:

...

> +/**
> + * pci_tsm_bind() - Bind @pdev as a TDI for @kvm
> + * @pdev: PCI device function to bind
> + * @kvm: Private memory attach context
> + * @tdi_id: Identifier (virtual BDF) for the TDI as referenced by the TSM and DSM
> + *
> + * Returns 0 on success, or a negative error code on failure.
> + *
> + * Context: Caller is responsible for constraining the bind lifetime to the
> + * registered state of the device. For example, pci_tsm_bind() /
> + * pci_tsm_unbind() limited to the VFIO driver bound state of the device.
> + */
> +int pci_tsm_bind(struct pci_dev *pdev, struct kvm *kvm, u32 tdi_id)
> +{
> + struct pci_tsm_pf0 *tsm_pf0;
> + struct pci_tdi *tdi;
> +
> + if (!kvm)
> + return -EINVAL;
> +
> + guard(rwsem_read)(&pci_tsm_rwsem);
> +
> + if (!pdev->tsm)
> + return -EINVAL;
> +
> + if (!is_link_tsm(pdev->tsm->tsm_dev))
> + return -ENXIO;
> +
> + tsm_pf0 = to_pci_tsm_pf0(pdev->tsm);
> + guard(mutex)(&tsm_pf0->lock);
> +
> + /* Resolve races to bind a TDI */
> + if (pdev->tsm->tdi) {
> + if (pdev->tsm->tdi->kvm == kvm)
> + return 0;
> + else
> + return -EBUSY;
> + }
> +
> + tdi = to_pci_tsm_ops(pdev->tsm)->bind(pdev, kvm, tdi_id);
> + if (IS_ERR(tdi))
> + return PTR_ERR(tdi);
> +
> + pdev->tsm->tdi = tdi;
> +
> + return 0;
> +}
> +EXPORT_SYMBOL_GPL(pci_tsm_bind);

Can we set tdi in the constructor? I use it later as part of a bind()
callback. I’d prefer not to pass tdi as an argument to those functions,
since functions like do_dev_communicate() are reused in other contexts
where tdi isn't needed.

cca_tsm_bind -> cca_vdev_create -> vdev state transition to VDEV_UNLOCKED.

/* setup host_tdi before call to device communicate */
host_tdi = to_cca_host_tdi(pdev);
host_tdi->rmm_vdev = rmm_vdev;

ret = submit_vdev_state_transition_work(pdev, RMI_VDEV_UNLOCKED);
/* failure is treated as rmi_vdev_create failure */
if (ret)
goto err_vdev_comm;

static inline struct cca_host_tdi *to_cca_host_tdi(struct pci_dev *pdev)
{
struct pci_tsm *tsm = pdev->tsm;

if (!tsm || !tsm->tdi)
return NULL;

return container_of(tsm->tdi, struct cca_host_tdi, tdi);
}

modified drivers/pci/tsm.c
@@ -391,8 +391,6 @@ int pci_tsm_bind(struct pci_dev *pdev, struct kvm *kvm, u32 tdi_id)
if (IS_ERR(tdi))
return PTR_ERR(tdi);

- pdev->tsm->tdi = tdi;
-
return 0;
}
EXPORT_SYMBOL_GPL(pci_tsm_bind);
@@ -998,6 +996,7 @@ static struct pci_dev *find_dsm_dev(struct pci_dev *pdev)
void pci_tsm_tdi_constructor(struct pci_dev *pdev, struct pci_tdi *tdi,
struct kvm *kvm, u32 tdi_id)
{
+ pdev->tsm->tdi = tdi;
tdi->pdev = pdev;
tdi->kvm = kvm;
tdi->tdi_id = tdi_id;

We could do a pci_tsm_tdi_destructor() to reset pdev->tsm->tdi = NULL?

-aneesh