On 5/2/2025 2:30 AM, Tushar Dave wrote:
>
>
> On 5/1/25 03:58, Vasant Hegde wrote:
>> On 4/30/2025 8:24 AM, Tushar Dave wrote:
>>> Generally PASID support requires ACS settings that usually create
>>> single device groups, but there are some niche cases where we can get
>>> multi-device groups and still have working PASID support. The primary
>>> issue is that PCI switches are not required to treat PASID tagged TLPs
>>> specially so appropriate ACS settings are required to route all TLPs to
>>> the host bridge if PASID is going to work properly.
>>>
>>> pci_enable_pasid() does check that each device that will use PASID has
>>> the proper ACS settings to achieve this routing.
>>>
>>> However, no-PASID devices can be combined with PASID capable devices
>>> within the same topology using non-uniform ACS settings. In this case
>>> the no-PASID devices may not have strict route to host ACS flags and
>>> end up being grouped with the PASID devices.
>>>
>>> This configuration fails to allow use of the PASID within the iommu
>>> core code which wrongly checks if the no-PASID device supports PASID.
>>>
>>> Fix this by ignoring no-PASID devices during the PASID validation. They
>>> will never issue a PASID TLP anyhow so they can be ignored.
>>>
>>> Fixes: c404f55c26fc ("iommu: Validate the PASID in iommu_attach_device_pasid()")
>>> Cc: stable@xxxxxxxxxxxxxxx
>>> Signed-off-by: Tushar Dave <tdave@xxxxxxxxxx>
>>> ---
>>>
>>> changes in v2:
>>> - added no-pasid check in __iommu_set_group_pasid and __iommu_remove_group_pasid
>>>
>>> drivers/iommu/iommu.c | 22 ++++++++++++++++------
>>> 1 file changed, 16 insertions(+), 6 deletions(-)
>>>
>>> diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
>>> index 60aed01e54f2..8251b07f4022 100644
>>> --- a/drivers/iommu/iommu.c
>>> +++ b/drivers/iommu/iommu.c
>>> @@ -3329,8 +3329,9 @@ static int __iommu_set_group_pasid(struct iommu_domain
>>> *domain,
>>> int ret;
>>
>> initialize ret to zero?
>
> Thanks Vasant.
>
> How about:
>
> for_each_group_device(group, device) {
> - ret = domain->ops->set_dev_pasid(domain, device->dev,
> - pasid, NULL);
> - if (ret)
> - goto err_revert;
> + if (device->dev->iommu->max_pasids > 0) {
> + ret = domain->ops->set_dev_pasid(domain, device->dev,
> + pasid, NULL);
> + if (ret)
> + goto err_revert;
> + }
> }
>
> Let me know.
Looks good.
-Vasant
