[PATCH 0/4] AMD IOMMU: Assign/de-assign complete alias groups

[PATCH 0/4] AMD IOMMU: Assign/de-assign complete alias groups

[Joerg Roedel]

Hi,

Here is a patch-set to fix the following issue:

When a device is added to an iommu domain, the current amd
iommu code will add the device itself and the device it
aliases to to the domain. But this alias relation is
uni-directional.

If, for example, device 03:00.1 aliases to 03:00.0 and is
added to a domain, the iommu code will add both devices.
But if 03.00.0 is added, 03.00.1 will be not.

This is a problem with some broken devices that appear with
function 0 on the bus but use function 1 as the request-id.
These devices will not work with the current implementation.

This patch-set changes the behavior of the code by
introducing alias-groups (similar to iommu-groups, but
support non-existent pci devices) and always
assigns/de-assigns complete alias groups to/from a domain.

This makes the alias relation bi-directional and fixes the
issues with broken devices.

Thanks,

	Joerg

Joerg Roedel (4):
  iommu/amd: Move struct iommu_dev_data to amd_iommu.c
  iommu/amd: Keep a list of devices in an alias group
  iommu/amd: Attach and detach complete alias group
  iommu/amd: Remove device binding reference count

 drivers/iommu/amd_iommu.c       | 74 +++++++++++++++++++++++++----------------
 drivers/iommu/amd_iommu_types.h | 21 ------------
 2 files changed, 46 insertions(+), 49 deletions(-)

-- 
1.9.1

[PATCH 1/4] iommu/amd: Move struct iommu_dev_data to amd_iommu.c

[Joerg Roedel]

From: Joerg Roedel <jroedel@suse.de>

The struct is only used there, so it doesn't need to be in
the header file.

Signed-off-by: Joerg Roedel <jroedel@suse.de>
---
 drivers/iommu/amd_iommu.c       | 21 +++++++++++++++++++++
 drivers/iommu/amd_iommu_types.h | 21 ---------------------
 2 files changed, 21 insertions(+), 21 deletions(-)

diff --git a/drivers/iommu/amd_iommu.c b/drivers/iommu/amd_iommu.c
index ecb0109..fe6d7cc 100644
--- a/drivers/iommu/amd_iommu.c
+++ b/drivers/iommu/amd_iommu.c
@@ -88,6 +88,27 @@ int amd_iommu_max_glx_val = -1;
 static struct dma_map_ops amd_iommu_dma_ops;
 
 /*
+ * This struct contains device specific data for the IOMMU
+ */
+struct iommu_dev_data {
+	struct list_head list;		  /* For domain->dev_list */
+	struct list_head dev_data_list;	  /* For global dev_data_list */
+	struct iommu_dev_data *alias_data;/* The alias dev_data */
+	struct protection_domain *domain; /* Domain the device is bound to */
+	atomic_t bind;			  /* Domain attach reference count */
+	u16 devid;			  /* PCI Device ID */
+	bool iommu_v2;			  /* Device can make use of IOMMUv2 */
+	bool passthrough;		  /* Default for device is pt_domain */
+	struct {
+		bool enabled;
+		int qdep;
+	} ats;				  /* ATS state */
+	bool pri_tlp;			  /* PASID TLB required for
+					     PPR completions */
+	u32 errata;			  /* Bitmap for errata to apply */
+};
+
+/*
  * general struct to manage commands send to an IOMMU
  */
 struct iommu_cmd {
diff --git a/drivers/iommu/amd_iommu_types.h b/drivers/iommu/amd_iommu_types.h
index 8e43b7c..cec51a8 100644
--- a/drivers/iommu/amd_iommu_types.h
+++ b/drivers/iommu/amd_iommu_types.h
@@ -418,27 +418,6 @@ struct protection_domain {
 };
 
 /*
- * This struct contains device specific data for the IOMMU
- */
-struct iommu_dev_data {
-	struct list_head list;		  /* For domain->dev_list */
-	struct list_head dev_data_list;	  /* For global dev_data_list */
-	struct iommu_dev_data *alias_data;/* The alias dev_data */
-	struct protection_domain *domain; /* Domain the device is bound to */
-	atomic_t bind;			  /* Domain attach reference count */
-	u16 devid;			  /* PCI Device ID */
-	bool iommu_v2;			  /* Device can make use of IOMMUv2 */
-	bool passthrough;		  /* Default for device is pt_domain */
-	struct {
-		bool enabled;
-		int qdep;
-	} ats;				  /* ATS state */
-	bool pri_tlp;			  /* PASID TLB required for
-					     PPR completions */
-	u32 errata;			  /* Bitmap for errata to apply */
-};
-
-/*
  * For dynamic growth the aperture size is split into ranges of 128MB of
  * DMA address space each. This struct represents one such range.
  */
-- 
1.9.1

[PATCH 2/4] iommu/amd: Keep a list of devices in an alias group

[Joerg Roedel]

From: Joerg Roedel <jroedel@suse.de>

Some broken devices might use any request-id from the alias
group, so we need to set a DTE entry for every device in
there. This patch adds creation of those lists.

Signed-off-by: Joerg Roedel <jroedel@suse.de>
---
 drivers/iommu/amd_iommu.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/drivers/iommu/amd_iommu.c b/drivers/iommu/amd_iommu.c
index fe6d7cc..8a15256 100644
--- a/drivers/iommu/amd_iommu.c
+++ b/drivers/iommu/amd_iommu.c
@@ -93,6 +93,7 @@ static struct dma_map_ops amd_iommu_dma_ops;
 struct iommu_dev_data {
 	struct list_head list;		  /* For domain->dev_list */
 	struct list_head dev_data_list;	  /* For global dev_data_list */
+	struct list_head alias_list;      /* Link alias-groups together */
 	struct iommu_dev_data *alias_data;/* The alias dev_data */
 	struct protection_domain *domain; /* Domain the device is bound to */
 	atomic_t bind;			  /* Domain attach reference count */
@@ -135,6 +136,8 @@ static struct iommu_dev_data *alloc_dev_data(u16 devid)
 	if (!dev_data)
 		return NULL;
 
+	INIT_LIST_HEAD(&dev_data->alias_list);
+
 	dev_data->devid = devid;
 	atomic_set(&dev_data->bind, 0);
 
@@ -383,6 +386,9 @@ static int iommu_init_device(struct device *dev)
 			return -ENOTSUPP;
 		}
 		dev_data->alias_data = alias_data;
+
+		/* Add device to the alias_list */
+		list_add(&dev_data->alias_list, &alias_data->alias_list);
 	}
 
 	ret = init_iommu_group(dev);
-- 
1.9.1

[PATCH 3/4] iommu/amd: Attach and detach complete alias group

[Joerg Roedel]

From: Joerg Roedel <jroedel@suse.de>

Change tha device attach and detach semantic to apply to all
devices in an alias group. This means all devices in an
alias group are now attached and detached at the same time.

Signed-off-by: Joerg Roedel <jroedel@suse.de>
---
 drivers/iommu/amd_iommu.c | 46 ++++++++++++++++++++--------------------------
 1 file changed, 20 insertions(+), 26 deletions(-)

diff --git a/drivers/iommu/amd_iommu.c b/drivers/iommu/amd_iommu.c
index 8a15256..a5e6b0a 100644
--- a/drivers/iommu/amd_iommu.c
+++ b/drivers/iommu/amd_iommu.c
@@ -2149,35 +2149,29 @@ static void do_detach(struct iommu_dev_data *dev_data)
 static int __attach_device(struct iommu_dev_data *dev_data,
 			   struct protection_domain *domain)
 {
+	struct iommu_dev_data *head, *entry;
 	int ret;
 
 	/* lock domain */
 	spin_lock(&domain->lock);
 
-	if (dev_data->alias_data != NULL) {
-		struct iommu_dev_data *alias_data = dev_data->alias_data;
-
-		/* Some sanity checks */
-		ret = -EBUSY;
-		if (alias_data->domain != NULL &&
-				alias_data->domain != domain)
-			goto out_unlock;
+	head = dev_data;
 
-		if (dev_data->domain != NULL &&
-				dev_data->domain != domain)
-			goto out_unlock;
+	if (head->alias_data != NULL)
+		head = head->alias_data;
 
-		/* Do real assignment */
-		if (alias_data->domain == NULL)
-			do_attach(alias_data, domain);
+	/* Now we have the root of the alias group, if any */
 
-		atomic_inc(&alias_data->bind);
-	}
+	ret = -EBUSY;
+	if (head->domain != NULL)
+		goto out_unlock;
 
-	if (dev_data->domain == NULL)
-		do_attach(dev_data, domain);
+	/* Attach alias group root */
+	do_attach(head, domain);
 
-	atomic_inc(&dev_data->bind);
+	/* Attach other devices in the alias group */
+	list_for_each_entry(entry, &head->alias_list, alias_list)
+		do_attach(entry, domain);
 
 	ret = 0;
 
@@ -2325,6 +2319,7 @@ static int attach_device(struct device *dev,
  */
 static void __detach_device(struct iommu_dev_data *dev_data)
 {
+	struct iommu_dev_data *head, *entry;
 	struct protection_domain *domain;
 	unsigned long flags;
 
@@ -2334,15 +2329,14 @@ static void __detach_device(struct iommu_dev_data *dev_data)
 
 	spin_lock_irqsave(&domain->lock, flags);
 
-	if (dev_data->alias_data != NULL) {
-		struct iommu_dev_data *alias_data = dev_data->alias_data;
+	head = dev_data;
+	if (head->alias_data != NULL)
+		head = head->alias_data;
 
-		if (atomic_dec_and_test(&alias_data->bind))
-			do_detach(alias_data);
-	}
+	list_for_each_entry(entry, &head->alias_list, alias_list)
+		do_detach(entry);
 
-	if (atomic_dec_and_test(&dev_data->bind))
-		do_detach(dev_data);
+	do_detach(head);
 
 	spin_unlock_irqrestore(&domain->lock, flags);
 
-- 
1.9.1

[PATCH 4/4] iommu/amd: Remove device binding reference count

[Joerg Roedel]

From: Joerg Roedel <jroedel@suse.de>

This reference count is not used anymore, as all devices in
an alias group are now attached and detached together.

Signed-off-by: Joerg Roedel <jroedel@suse.de>
---
 drivers/iommu/amd_iommu.c | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/drivers/iommu/amd_iommu.c b/drivers/iommu/amd_iommu.c
index a5e6b0a..989c1ae 100644
--- a/drivers/iommu/amd_iommu.c
+++ b/drivers/iommu/amd_iommu.c
@@ -96,7 +96,6 @@ struct iommu_dev_data {
 	struct list_head alias_list;      /* Link alias-groups together */
 	struct iommu_dev_data *alias_data;/* The alias dev_data */
 	struct protection_domain *domain; /* Domain the device is bound to */
-	atomic_t bind;			  /* Domain attach reference count */
 	u16 devid;			  /* PCI Device ID */
 	bool iommu_v2;			  /* Device can make use of IOMMUv2 */
 	bool passthrough;		  /* Default for device is pt_domain */
@@ -139,7 +138,6 @@ static struct iommu_dev_data *alloc_dev_data(u16 devid)
 	INIT_LIST_HEAD(&dev_data->alias_list);
 
 	dev_data->devid = devid;
-	atomic_set(&dev_data->bind, 0);
 
 	spin_lock_irqsave(&dev_data_list_lock, flags);
 	list_add_tail(&dev_data->dev_data_list, &dev_data_list);
@@ -3179,7 +3177,6 @@ static void cleanup_domain(struct protection_domain *domain)
 		entry = list_first_entry(&domain->dev_list,
 					 struct iommu_dev_data, list);
 		__detach_device(entry);
-		atomic_set(&entry->bind, 0);
 	}
 
 	write_unlock_irqrestore(&amd_iommu_devtable_lock, flags);
-- 
1.9.1