Re: [PATCH 1/3] iommu/s390: Fix duplicate domain attachments

[Jason Gunthorpe]

On Thu, Sep 15, 2022 at 05:14:00PM +0200, Niklas Schnelle wrote:
> Since commit fa7e9ecc5e1c ("iommu/s390: Tolerate repeat attach_dev
> calls") we can end up with duplicates in the list of devices attached to
> a domain. This is inefficient and confusing since only one domain can
> actually be in control of the IOMMU translations for a device. Fix this
> by detaching the device from the previous domain, if any, on attach.
> This also makes the restore behavior analogous between IOMMU and DMA API
> control.
> 
> Fixes: fa7e9ecc5e1c ("iommu/s390: Tolerate repeat attach_dev calls")
> Signed-off-by: Niklas Schnelle <schnelle@xxxxxxxxxxxxx>
> ---
>  drivers/iommu/s390-iommu.c | 82 ++++++++++++++++++++++----------------
>  1 file changed, 47 insertions(+), 35 deletions(-)
> 
> diff --git a/drivers/iommu/s390-iommu.c b/drivers/iommu/s390-iommu.c
> index c898bcbbce11..de8f76775240 100644
> --- a/drivers/iommu/s390-iommu.c
> +++ b/drivers/iommu/s390-iommu.c
> @@ -83,14 +83,41 @@ static void s390_domain_free(struct iommu_domain *domain)
>  	kfree(s390_domain);
>  }
>  
> +static bool __s390_iommu_detach_device(struct s390_domain *s390_domain,
> +				     struct zpci_dev *zdev)
> +{
> +	struct s390_domain_device *domain_device, *tmp;
> +	unsigned long flags;
> +	bool found = false;
> +
> +	spin_lock_irqsave(&s390_domain->list_lock, flags);
> +	list_for_each_entry_safe(domain_device, tmp, &s390_domain->devices,
> +				 list) {
> +		if (domain_device->zdev == zdev) {

Why all this searching? The domain argument is only being provided to
help drivers find their data structures, in most cases I would expect
it to be mostly unused.

After patch 3 the struct is gone, so isn't this just

 spin_lock_irqsave(&s390_domain->list_lock, flags);
 list_del_init(&zdev->iommu_list)
 spin_unlock_irqsave(&s390_domain->list_lock, flags);

?

Jason