Re: [PATCH rc v7 5/6] iommu: Fix ATS invalidation timeouts during __iommu_remove_group_pasid()

[Baolu Lu <baolu.lu@linux.intel.com>]

On 4/19/26 07:41, Nicolin Chen wrote:
> If a device is blocked, its PASID domains are already detached. Repeating
> iommu_remove_dev_pasid() is unnecessary and might trigger ATS invalidation
> timeouts.
> 
> Skip the iommu_remove_dev_pasid() call upon the blocked flag.
> 
> Fixes: c279e83953d9 ("iommu: Introduce pci_dev_reset_iommu_prepare/done()")
> Cc: stable@vger.kernel.org
> Reported-by: Kevin Tian <kevin.tian@intel.com>
> Closes: https://lore.kernel.org/all/BN9PR11MB5276D60096EBF15C5753C4BB8C202@BN9PR11MB5276.namprd11.prod.outlook.com/
> Signed-off-by: Nicolin Chen <nicolinc@nvidia.com>
> ---
>   drivers/iommu/iommu.c | 3 ++-
>   1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
> index ff181db687bbf..30ba18e613faa 100644
> --- a/drivers/iommu/iommu.c
> +++ b/drivers/iommu/iommu.c
> @@ -3554,7 +3554,8 @@ static void __iommu_remove_group_pasid(struct iommu_group *group,
>   	struct group_device *device;
>   
>   	for_each_group_device(group, device) {
> -		if (device->dev->iommu->max_pasids > 0)
> +		/* Device might be already detached for a device recovery */
> +		if (!device->blocked && device->dev->iommu->max_pasids > 0)
>   			iommu_remove_dev_pasid(device->dev, pasid, domain);
>   	}
>   }

If that is the case, why not add the same logic to the set_group_pasid
path? For example:

diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
index 61c12ba78206..42a979ee0ced 100644
--- a/drivers/iommu/iommu.c
+++ b/drivers/iommu/iommu.c
@@ -3532,7 +3532,7 @@ static int __iommu_set_group_pasid(struct 
iommu_domain *domain,
         int ret;

         for_each_group_device(group, device) {
-               if (device->dev->iommu->max_pasids > 0) {
+               if (!device->blocked && device->dev->iommu->max_pasids > 
0) {
                         ret = domain->ops->set_dev_pasid(domain, 
device->dev,
                                                          pasid, old);
                         if (ret)

By the way, now that I see the device->dev->iommu->max_pasids > 0 check
in the set_pasid path, I understand why you added the same check to the
remove path in patch 3/6. :-)

Thanks,
baolu