Xu Yilun wrote:
> > +static int pci_tsm_disconnect(struct pci_dev *pdev)
> > +{
> > + struct pci_tsm *pci_tsm = pdev->tsm;
> > +
> > + lockdep_assert_held(&pci_tsm_rwsem);
> > + if_not_guard(mutex_intr, &pci_tsm->lock)
> > + return -EINTR;
> > +
> > + if (pci_tsm->state < PCI_TSM_CONNECT)
> > + return 0;
> > + if (pci_tsm->state < PCI_TSM_INIT)
> > + return -ENXIO;
>
> Check PCI_TSM_INIT first, or this condition will never hit.
>
> if (pci_tsm->state < PCI_TSM_INIT)
> return -ENXIO;
> if (pci_tsm->state < PCI_TSM_CONNECT)
> return 0;
>
> I suggest the same sequence for pci_tsm_connect().
Good catch, fixed.
[..]
> > +
> > +static void __pci_tsm_init(struct pci_dev *pdev)
> > +{
> > + bool tee_cap;
> > +
> > + if (!is_physical_endpoint(pdev))
> > + return;
>
> This Filters out virtual functions, just because not ready for support,
> is it?
Do you see a need for PCI core to notify the TSM driver about the
arrival of VF devices?
My expectation is that a VF TDI communicates with the TSM driver
relative to its PF.
> > +
> > + tee_cap = pdev->devcap & PCI_EXP_DEVCAP_TEE;
> > +
> > + if (!(pdev->ide_cap || tee_cap))
> > + return;
> > +
> > + lockdep_assert_held_write(&pci_tsm_rwsem);
> > + if (!tsm_ops)
> > + return;
> > +
> > + struct pci_tsm *pci_tsm __free(kfree) = kzalloc(sizeof(*pci_tsm), GFP_KERNEL);
> > + if (!pci_tsm)
> > + return;
> > +
> > + /*
> > + * If a physical device has any security capabilities it may be
> > + * a candidate to connect with the platform TSM
> > + */
> > + struct pci_dsm *dsm __free(dsm_remove) = tsm_ops->probe(pdev);
>
> IIUC, pdev->tsm should be for every pci function (physical or virtual),
> pdev->tsm->dsm should be only for physical functions, is it?
Per above I was only expecting physical function, but the bind flow
might introduce the need for per function (phyiscal or virtual) TDI
context. I expect that is separate from the PF pdev->tsm context.
