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

Jason Gunthorpe
2022-09-20T14:23:18Z

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@linux.ibm.com>
> ---
> 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