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?
-Vasant
>
> for_each_group_device(group, device) {
> - ret = domain->ops->set_dev_pasid(domain, device->dev,
> - pasid, NULL);
> + if (device->dev->iommu->max_pasids > 0)
> + ret = domain->ops->set_dev_pasid(domain, device->dev,
> + pasid, NULL);
> if (ret)
> goto err_revert;
> }
> @@ -3342,7 +3343,8 @@ static int __iommu_set_group_pasid(struct iommu_domain *domain,
> for_each_group_device(group, device) {
> if (device == last_gdev)
> break;
> - iommu_remove_dev_pasid(device->dev, pasid, domain);
> + if (device->dev->iommu->max_pasids > 0)
> + iommu_remove_dev_pasid(device->dev, pasid, domain);
> }
> return ret;
> }
> @@ -3353,8 +3355,10 @@ static void __iommu_remove_group_pasid(struct iommu_group *group,
> {
> struct group_device *device;
>
> - for_each_group_device(group, device)
> - iommu_remove_dev_pasid(device->dev, pasid, domain);
> + for_each_group_device(group, device) {
> + if (device->dev->iommu->max_pasids > 0)
> + iommu_remove_dev_pasid(device->dev, pasid, domain);
> + }
> }
>
> /*
> @@ -3391,7 +3395,13 @@ int iommu_attach_device_pasid(struct iommu_domain *domain,
>
> mutex_lock(&group->mutex);
> for_each_group_device(group, device) {
> - if (pasid >= device->dev->iommu->max_pasids) {
> + /*
> + * Skip PASID validation for devices without PASID support
> + * (max_pasids = 0). These devices cannot issue transactions
> + * with PASID, so they don't affect group's PASID usage.
> + */
> + if ((device->dev->iommu->max_pasids > 0) &&
> + (pasid >= device->dev->iommu->max_pasids)) {
> ret = -EINVAL;
> goto out_unlock;
> }
