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

[Aneesh Kumar K.V <aneesh.kumar@kernel.org>]

Alexey Kardashevskiy <aik@amd.com> writes:

> 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.
>
>
>> After a PCIe device has established a secure link and session between a TEE
>> Security Manager (TSM) and its local Device Security Manager (DSM), the
>> device or its subfunctions are candidates to be bound to a private memory
>> context, a TVM. A PCIe device function interface assigned to a TVM is a TEE
>> Device Interface (TDI).
>> 
>> The pci_tsm_bind() requests the low-level TSM driver to associate the
>> device with private MMIO and private IOMMU context resources of a given TVM
>> represented by a @kvm argument. A device in the bound state corresponds to
>> the TDISP protocol LOCKED state and awaits validation by the TVM. It is a
>> 'struct pci_tsm_link_ops' operation because, similar to IDE establishment,
>> it involves host side resource establishment and context setup on behalf of
>> the guest. It is also expected to be performed lazily to allow for
>> operation of the device in non-confidential "shared" context for pre-lock
>> configuration.
>> 
>
>
>
>> Co-developed-by: Xu Yilun <yilun.xu@linux.intel.com>
>> Signed-off-by: Xu Yilun <yilun.xu@linux.intel.com>
>> Signed-off-by: Dan Williams <dan.j.williams@intel.com>
>> ---
>>   drivers/pci/tsm.c       | 95 +++++++++++++++++++++++++++++++++++++++++
>>   include/linux/pci-tsm.h | 30 +++++++++++++
>>   2 files changed, 125 insertions(+)
>> 
>> 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.
>
> But to_pci_tsm_pf0() casts the pdev's tsm to pci_tsm_pf0 which makes sense for PF0 but not for VFs.
>
> What do I miss and how does this work for you? Thanks,
>

I guess this needs

modified   drivers/pci/tsm.c
@@ -44,7 +44,7 @@ static inline bool has_tee(struct pci_dev *pdev)
 /* 'struct pci_tsm_pf0' wraps 'struct pci_tsm' when ->dsm == ->pdev (self) */
 static struct pci_tsm_pf0 *to_pci_tsm_pf0(struct pci_tsm *pci_tsm)
 {
-	struct pci_dev *pdev = pci_tsm->pdev;
+	struct pci_dev *pdev = pci_tsm->dsm;
 
 	if (!is_pci_tsm_pf0(pdev) || !is_dsm(pdev)) {
 		dev_WARN_ONCE(&pdev->dev, 1, "invalid context object\n");

-aneesh