Re: [PATCH 1/7] PCI/TSM: Add pci_tsm_{bind,unbind}() methods for instantiating TDIs

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

Alexey Kardashevskiy wrote:
> 
> 
> On 27/8/25 13:52, Dan Williams wrote:
> 
> 
> I suggest changing "pci_tsm_{bind,unbind}()" to
> "pci_tsm_bind/pci_tsm_unbind" or "pci_tsm_bind/_unbind" as otherwise
> cannot grep for pci_tsm_bind in git log.

Easy enough, and will change, but FWIW:

$ git log --grep pci_tsm_bind
commit a8c148ed753d640b5e5f8d1043d9f9d6188436b4 (HEAD -> for-6.18/devsec)
Author: Dan Williams <dan.j.williams@intel.com>
Date:   Mon Aug 18 13:59:26 2025 -0700

    PCI/TSM: Add pci_tsm_{bind,unbind}() methods for instantiating TDIs

...due to pci_tsm_bind() in the changelog.

[..]
> > diff --git a/drivers/pci/tsm.c b/drivers/pci/tsm.c
> > index 092e81c5208c..302a974f3632 100644
> > --- a/drivers/pci/tsm.c
> > +++ b/drivers/pci/tsm.c
> > @@ -251,6 +251,99 @@ static int remove_fn(struct pci_dev *pdev, void *data)
> >   	return 0;
> >   }
> >   
> > +/*
> > + * Note, this helper only returns an error code and takes an argument for
> > + * compatibility with the pci_walk_bus() callback prototype. pci_tsm_unbind()
> > + * always succeeds.
> > + */
> > +static int __pci_tsm_unbind(struct pci_dev *pdev, void *data)
> > +{
> > +	struct pci_tdi *tdi;
> > +	struct pci_tsm_pf0 *tsm_pf0;
> > +
> > +	lockdep_assert_held(&pci_tsm_rwsem);
> > +
> > +	if (!pdev->tsm)
> > +		return 0;
> > +
> > +	tsm_pf0 = to_pci_tsm_pf0(pdev->tsm);
> 
> What is expected to be passed to __pci_tsm_unbind/pci_tsm_bind as pdev - PF0 or TEE-IO VF? I guess the latter.

Yes, the latter.

> But to_pci_tsm_pf0() casts the pdev's tsm to pci_tsm_pf0 which makes sense for PF0 but not for VFs.

Yes, that it is a bug. Incremental fixup I will push in v6 below.

> What do I miss and how does this work for you? Thanks,

It works if only testing direct-device assignment of PF0.

-- 8< --
diff --git a/drivers/pci/tsm.c b/drivers/pci/tsm.c
index 4688ddbc0b33..59458e894251 100644
--- a/drivers/pci/tsm.c
+++ b/drivers/pci/tsm.c
@@ -36,16 +36,16 @@ static inline bool is_dsm(struct pci_dev *pdev)
 }
 
 /* 'struct pci_tsm_pf0' wraps 'struct pci_tsm' when ->dsm_dev == ->pdev (self) */
-static struct pci_tsm_pf0 *to_pci_tsm_pf0(struct pci_tsm *pci_tsm)
+static struct pci_tsm_pf0 *to_pci_tsm_pf0(struct pci_tsm *tsm)
 {
-	struct pci_dev *pdev = pci_tsm->pdev;
+	struct pci_dev *pdev = tsm->dsm_dev;
 
 	if (!is_pci_tsm_pf0(pdev) || !is_dsm(pdev)) {
-		dev_WARN_ONCE(&pdev->dev, 1, "invalid context object\n");
+		pci_WARN_ONCE(tsm->pdev, 1, "invalid context object\n");
 		return NULL;
 	}
 
-	return container_of(pci_tsm, struct pci_tsm_pf0, base_tsm);
+	return container_of(tsm, struct pci_tsm_pf0, base_tsm);
 }
 
 static void tsm_remove(struct pci_tsm *tsm)
diff --git a/include/linux/pci-tsm.h b/include/linux/pci-tsm.h
index 61c947ff8735..d26d6e128d83 100644
--- a/include/linux/pci-tsm.h
+++ b/include/linux/pci-tsm.h
@@ -121,6 +121,9 @@ struct pci_tsm_pf0 {
 /* physical function0 and capable of 'connect' */
 static inline bool is_pci_tsm_pf0(struct pci_dev *pdev)
 {
+	if (!pdev)
+		return false;
+
 	if (!pci_is_pcie(pdev))
 		return false;