RE: [PATCH v8 10/15] PCI/TSM: Add vIOMMU-bound device contexts

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

"Tian, Kevin" <kevin.tian@intel.com> writes:

>> From: Aneesh Kumar K.V (Arm) <aneesh.kumar@kernel.org>
>> Sent: Thursday, November 5, 2026 12:16 AM
>> 
>> An IOMMUFD vDEVICE represents a physical device within a virtual IOMMU
>> (vIOMMU) for a guest. It must use the same TEE Security Manager as its
>> vIOMMU. Pinning the vIOMMU backend keeps its tsm_dev registered, but
>> does not preserve the connection between the physical function 0 (PF0)
>> PCI device and that TSM.
>> 
>> Add a per-vDEVICE PCI TSM context. Verify that the PCI device and the
>> vIOMMU use the same tsm_dev, and account active contexts on PF0. Reject
>> disconnect with -EBUSY until all dependent vDEVICEs release their
>> contexts.
>
> We really don't need so many vIOMMU/vDEVICE notations here,
> especially in following code/comments. It's just about a tsm context
> which holds the pf0 from disconnected. Then let's describe/code it
> from that angle.
>
>> +struct pci_tsm_context *pci_tsm_context_get(struct pci_dev *pdev,
>> +					    struct tsm_dev *viommu_tsm_dev)
>
> could clarify that each get returns a new context even on a same device.
>
> s/viommu_tsm_dev/tsm_dev/. It's just a parameter to be matched against.
>
>>  {
>> -	struct pci_tsm_pf0 *tsm_pf0;
>> -	struct pci_tdi *tdi;
>> -
>> -	if (!kvm)
>> -		return -EINVAL;
>> +	struct pci_tsm_context *context;
>> +	struct pci_tsm_pf0 *pf0;
>> 
>>  	guard(rwsem_read)(&pci_tsm_rwsem);
>> +	if (!pdev->tsm || !is_link_tsm(pdev->tsm->tsm_dev))
>> +		return ERR_PTR(-EOPNOTSUPP);
>> +	/*
>> +	 * The vdevice can use a different PCI function from VIOMMU_ALLOC.
>> The
>> +	 * module pin keeps the vIOMMU's TSM registered, but does not
>> freeze this
>> +	 * function's link. Match its current TSM before pinning the link below.
>> +	 */
>
> this is the example needing a rewrite...
>

I'll simplify this and document the vIOMMU tsm_dev reference in the CCA
code.

	/* The device must be connected to the TSM requested by the caller. */
	if (pdev->tsm->tsm_dev != tsm_dev)
		return ERR_PTR(-EXDEV);

>
>> +	if (pdev->tsm->tsm_dev != viommu_tsm_dev)
>> +		return ERR_PTR(-EXDEV);
>> 
>> -	if (!pdev->tsm)
>> -		return -EINVAL;
>> +	pf0 = to_pci_tsm_pf0(pdev->tsm);
>> +	if (!pf0)
>> +		return ERR_PTR(-ENXIO);
>
> No need to check the return error as other places do?

I didn't quite follow this feedback. Are you asking why we check for
!pf0 here but not in other paths? If so, I think we can drop this check
as well, since a valid pdev->tsm should imply a valid pf0.

-aneesh