Date:	Tue, 5 Nov 2013 16:24:58 +0800
From:	Yijing Wang <wangyijing@...wei.com>
To:	Joerg Roedel <joro@...tes.org>
CC:	<linux-pci@...r.kernel.org>, David Woodhouse <dwmw2@...radead.org>,
	Vinod Koul <vinod.koul@...el.com>,
	Dan Williams <dan.j.williams@...el.com>,
	<dmaengine@...r.kernel.org>, <iommu@...ts.linux-foundation.org>,
	<linux-kernel@...r.kernel.org>,
	Yijing Wang <wangyijing@...wei.com>,
	"Hanjun Guo" <guohanjun@...wei.com>
Subject: [PATCH 1/1] IOMMU: Save pci device id instead of pci_dev* pointer for DMAR devices

Currently, DMAR driver save target pci devices pointers for drhd/rmrr/atsr
in (pci_dev *) array. This is not safe, because pci devices maybe
hot added or removed during system running. They will have new pci_dev *
pointer. So if there have two IOMMUs or more in system, these devices
will find a wrong drhd during DMA mapping. And DMAR faults will occur.
This patch save pci device id insted of (pci_dev *) to fix this issue,
Because DMAR table just provide pci device id under a specific IOMMU,
so there is no reason to bind IOMMU with the (pci_dev *). Other, here
use list to manage devices' id for IOMMU, we can easily use list helper
to manage device id.

after remove and rescan a pci device
[  611.857095] dmar: DRHD: handling fault status reg 2
[  611.857109] dmar: DMAR:[DMA Read] Request device [86:00.3] fault addr ffff7000
[  611.857109] DMAR:[fault reason 02] Present bit in context entry is clear
[  611.857524] dmar: DRHD: handling fault status reg 102
[  611.857534] dmar: DMAR:[DMA Read] Request device [86:00.3] fault addr ffff6000
[  611.857534] DMAR:[fault reason 02] Present bit in context entry is clear
[  611.857936] dmar: DRHD: handling fault status reg 202
[  611.857947] dmar: DMAR:[DMA Read] Request device [86:00.3] fault addr ffff5000
[  611.857947] DMAR:[fault reason 02] Present bit in context entry is clear
[  611.858351] dmar: DRHD: handling fault status reg 302
[  611.858362] dmar: DMAR:[DMA Read] Request device [86:00.3] fault addr ffff4000
[  611.858362] DMAR:[fault reason 02] Present bit in context entry is clear
[  611.860819] IPv6: ADDRCONF(NETDEV_UP): eth3: link is not ready
[  611.860983] dmar: DRHD: handling fault status reg 402
[  611.860995] dmar: INTR-REMAP: Request device [[86:00.3] fault index a4
[  611.860995] INTR-REMAP:[fault reason 34] Present field in the IRTE entry is clear

Signed-off-by: Yijing Wang <wangyijing@...wei.com>
---
 drivers/iommu/dmar.c        |   93 +++++++++++++-------------
 drivers/iommu/intel-iommu.c |  155 ++++++++++++++++++++++++++++---------------
 include/linux/dmar.h        |   20 ++++--
 3 files changed, 159 insertions(+), 109 deletions(-)

diff --git a/drivers/iommu/dmar.c b/drivers/iommu/dmar.c
index 785675a..9aa65a3 100644
--- a/drivers/iommu/dmar.c
+++ b/drivers/iommu/dmar.c
@@ -65,12 +65,13 @@ static void __init dmar_register_drhd_unit(struct dmar_drhd_unit *drhd)
 }
 
 static int __init dmar_parse_one_dev_scope(struct acpi_dmar_device_scope *scope,
-					   struct pci_dev **dev, u16 segment)
+					    u16 segment, struct list_head *head)
 {
 	struct pci_bus *bus;
 	struct pci_dev *pdev = NULL;
 	struct acpi_dmar_pci_path *path;
 	int count;
+	struct dmar_device *dmar_dev;
 
 	bus = pci_find_bus(segment, scope->bus);
 	path = (struct acpi_dmar_pci_path *)(scope + 1);
@@ -100,7 +101,6 @@ static int __init dmar_parse_one_dev_scope(struct acpi_dmar_device_scope *scope,
 	if (!pdev) {
 		pr_warn("Device scope device [%04x:%02x:%02x.%02x] not found\n",
 			segment, scope->bus, path->dev, path->fn);
-		*dev = NULL;
 		return 0;
 	}
 	if ((scope->entry_type == ACPI_DMAR_SCOPE_TYPE_ENDPOINT && \
@@ -111,54 +111,39 @@ static int __init dmar_parse_one_dev_scope(struct acpi_dmar_device_scope *scope,
 			pci_name(pdev));
 		return -EINVAL;
 	}
-	*dev = pdev;
+
+	dmar_dev = kzalloc(sizeof(struct dmar_device), GFP_KERNEL);
+	if (!dmar_dev) {
+		pci_dev_put(pdev);
+		return -ENOMEM;
+	}
+
+	dmar_dev->segment = segment;
+	dmar_dev->bus = pdev->bus->number;
+	dmar_dev->devfn = pdev->devfn;
+	list_add_tail(&dmar_dev->list, head);
+
+	pci_dev_put(pdev);
 	return 0;
 }
 
-int __init dmar_parse_dev_scope(void *start, void *end, int *cnt,
-				struct pci_dev ***devices, u16 segment)
+int __init dmar_parse_dev_scope(void *start, void *end, u16 segment, 
+	struct list_head *head)
 {
 	struct acpi_dmar_device_scope *scope;
-	void * tmp = start;
-	int index;
 	int ret;
 
-	*cnt = 0;
-	while (start < end) {
-		scope = start;
-		if (scope->entry_type == ACPI_DMAR_SCOPE_TYPE_ENDPOINT ||
-		    scope->entry_type == ACPI_DMAR_SCOPE_TYPE_BRIDGE)
-			(*cnt)++;
-		else if (scope->entry_type != ACPI_DMAR_SCOPE_TYPE_IOAPIC &&
-			scope->entry_type != ACPI_DMAR_SCOPE_TYPE_HPET) {
-			pr_warn("Unsupported device scope\n");
-		}
-		start += scope->length;
-	}
-	if (*cnt == 0)
-		return 0;
-
-	*devices = kcalloc(*cnt, sizeof(struct pci_dev *), GFP_KERNEL);
-	if (!*devices)
-		return -ENOMEM;
-
-	start = tmp;
-	index = 0;
 	while (start < end) {
 		scope = start;
 		if (scope->entry_type == ACPI_DMAR_SCOPE_TYPE_ENDPOINT ||
 		    scope->entry_type == ACPI_DMAR_SCOPE_TYPE_BRIDGE) {
-			ret = dmar_parse_one_dev_scope(scope,
-				&(*devices)[index], segment);
-			if (ret) {
-				kfree(*devices);
+			ret = dmar_parse_one_dev_scope(scope, segment, head);
+			if (ret)
 				return ret;
-			}
-			index ++;
 		}
 		start += scope->length;
 	}
-
+    
 	return 0;
 }
 
@@ -183,6 +168,7 @@ dmar_parse_one_drhd(struct acpi_dmar_header *header)
 	dmaru->reg_base_addr = drhd->address;
 	dmaru->segment = drhd->segment;
 	dmaru->include_all = drhd->flags & 0x1; /* BIT0: INCLUDE_ALL */
+	INIT_LIST_HEAD(&dmaru->head);
 
 	ret = alloc_iommu(dmaru);
 	if (ret) {
@@ -193,6 +179,19 @@ dmar_parse_one_drhd(struct acpi_dmar_header *header)
 	return 0;
 }
 
+static void drhd_free(struct dmar_drhd_unit *dmaru)
+{
+	struct dmar_device *dev, *tmp;
+
+	list_for_each_entry_safe(dev, tmp, &dmaru->head,
+	    list) {
+		list_del(&dev->list);
+		kfree(dev);
+	}
+
+	kfree(dmaru);
+}
+
 static int __init dmar_parse_dev(struct dmar_drhd_unit *dmaru)
 {
 	struct acpi_dmar_hardware_unit *drhd;
@@ -205,11 +204,10 @@ static int __init dmar_parse_dev(struct dmar_drhd_unit *dmaru)
 
 	ret = dmar_parse_dev_scope((void *)(drhd + 1),
 				((void *)drhd) + drhd->header.length,
-				&dmaru->devices_cnt, &dmaru->devices,
-				drhd->segment);
+				drhd->segment, &dmaru->head);
 	if (ret) {
 		list_del(&dmaru->list);
-		kfree(dmaru);
+		drhd_free(dmaru);
 	}
 	return ret;
 }
@@ -378,16 +376,18 @@ parse_dmar_table(void)
 	return ret;
 }
 
-static int dmar_pci_device_match(struct pci_dev *devices[], int cnt,
-			  struct pci_dev *dev)
+static int dmar_pci_device_match(struct pci_dev *dev, 
+	struct list_head *head)
 {
-	int index;
+	struct dmar_device *dmar_dev;
 
 	while (dev) {
-		for (index = 0; index < cnt; index++)
-			if (dev == devices[index])
-				return 1;
-
+		list_for_each_entry(dmar_dev, head, list)
+		    if (dmar_dev->segment == pci_domain_nr(dev->bus)
+			    && dmar_dev->bus == dev->bus->number
+			    && dmar_dev->devfn == dev->devfn)
+			return 1;
+		
 		/* Check our parent */
 		dev = dev->bus->self;
 	}
@@ -412,8 +412,7 @@ dmar_find_matched_drhd_unit(struct pci_dev *dev)
 		    drhd->segment == pci_domain_nr(dev->bus))
 			return dmaru;
 
-		if (dmar_pci_device_match(dmaru->devices,
-					  dmaru->devices_cnt, dev))
+		if (dmar_pci_device_match(dev, &dmaru->head))
 			return dmaru;
 	}
 
diff --git a/drivers/iommu/intel-iommu.c b/drivers/iommu/intel-iommu.c
index 15e9b57..b33fe0e 100644
--- a/drivers/iommu/intel-iommu.c
+++ b/drivers/iommu/intel-iommu.c
@@ -650,7 +650,8 @@ static void domain_update_iommu_cap(struct dmar_domain *domain)
 static struct intel_iommu *device_to_iommu(int segment, u8 bus, u8 devfn)
 {
 	struct dmar_drhd_unit *drhd = NULL;
-	int i;
+	struct dmar_device *dmar_dev;
+	struct pci_dev *pdev;
 
 	for_each_drhd_unit(drhd) {
 		if (drhd->ignored)
@@ -658,16 +659,22 @@ static struct intel_iommu *device_to_iommu(int segment, u8 bus, u8 devfn)
 		if (segment != drhd->segment)
 			continue;
 
-		for (i = 0; i < drhd->devices_cnt; i++) {
-			if (drhd->devices[i] &&
-			    drhd->devices[i]->bus->number == bus &&
-			    drhd->devices[i]->devfn == devfn)
-				return drhd->iommu;
-			if (drhd->devices[i] &&
-			    drhd->devices[i]->subordinate &&
-			    drhd->devices[i]->subordinate->number <= bus &&
-			    drhd->devices[i]->subordinate->busn_res.end >= bus)
-				return drhd->iommu;
+		list_for_each_entry(dmar_dev, &drhd->head, list) {
+		    if (dmar_dev->bus == bus && 
+			    dmar_dev->devfn == devfn)
+			return drhd->iommu;
+
+		    pdev = pci_get_domain_bus_and_slot(dmar_dev->segment, 
+			    dmar_dev->bus, dmar_dev->devfn);
+		    if (pdev->subordinate && 
+			    pdev->subordinate->number <= bus &&
+			    pdev->subordinate->busn_res.end >= bus) {
+			pci_dev_put(pdev);
+			return drhd->iommu;
+		    }
+
+		    if (pdev)
+			pci_dev_put(pdev);
 		}
 
 		if (drhd->include_all)
@@ -2331,18 +2338,20 @@ static int domain_add_dev_info(struct dmar_domain *domain,
 static bool device_has_rmrr(struct pci_dev *dev)
 {
 	struct dmar_rmrr_unit *rmrr;
-	int i;
+	struct dmar_device *dmar_dev;
 
 	for_each_rmrr_units(rmrr) {
-		for (i = 0; i < rmrr->devices_cnt; i++) {
-			/*
-			 * Return TRUE if this RMRR contains the device that
-			 * is passed in.
-			 */
-			if (rmrr->devices[i] == dev)
-				return true;
-		}
+	    list_for_each_entry(dmar_dev, &rmrr->head, list)
+		/*
+		 * Return TRUE if this RMRR contains the device that
+		 * is passed in.
+		 */
+	if (dmar_dev->segment == pci_domain_nr(dev->bus) && 
+			dmar_dev->bus == dev->bus->number && 
+			dmar_dev->devfn == dev->devfn)
+		return true;
 	}
+	
 	return false;
 }
 
@@ -2451,7 +2460,7 @@ static int __init init_dmars(void)
 	struct dmar_rmrr_unit *rmrr;
 	struct pci_dev *pdev;
 	struct intel_iommu *iommu;
-	int i, ret;
+	int ret;
 
 	/*
 	 * for each drhd
@@ -2605,8 +2614,10 @@ static int __init init_dmars(void)
 	 */
 	printk(KERN_INFO "IOMMU: Setting RMRR:\n");
 	for_each_rmrr_units(rmrr) {
-		for (i = 0; i < rmrr->devices_cnt; i++) {
-			pdev = rmrr->devices[i];
+		struct dmar_device *dmar_dev;
+	    list_for_each_entry(dmar_dev, &rmrr->head, list) {
+			pdev = pci_get_domain_bus_and_slot(dmar_dev->segment, 
+					dmar_dev->bus, dmar_dev->devfn);
 			/*
 			 * some BIOS lists non-exist devices in DMAR
 			 * table.
@@ -2615,9 +2626,11 @@ static int __init init_dmars(void)
 				continue;
 			ret = iommu_prepare_rmrr_dev(rmrr, pdev);
 			if (ret)
-				printk(KERN_ERR
-				       "IOMMU: mapping reserved region failed\n");
-		}
+				printk(KERN_ERR 
+					"IOMMU: mapping reserved region failed\n");
+		
+			pci_dev_put(pdev);
+	    }
 	}
 
 	iommu_prepare_isa();
@@ -3301,30 +3314,30 @@ DECLARE_PCI_FIXUP_ENABLE(PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_IOAT_SNB, quir
 static void __init init_no_remapping_devices(void)
 {
 	struct dmar_drhd_unit *drhd;
+	struct dmar_device *dmar_dev;
+	struct pci_dev *pdev = NULL;
 
-	for_each_drhd_unit(drhd) {
-		if (!drhd->include_all) {
-			int i;
-			for (i = 0; i < drhd->devices_cnt; i++)
-				if (drhd->devices[i] != NULL)
-					break;
+	for_each_drhd_unit(drhd) 
+		if (!drhd->include_all) 
 			/* ignore DMAR unit if no pci devices exist */
-			if (i == drhd->devices_cnt)
+			if (list_empty(&drhd->head))
 				drhd->ignored = 1;
-		}
-	}
-
+	
 	for_each_drhd_unit(drhd) {
-		int i;
 		if (drhd->ignored || drhd->include_all)
 			continue;
 
-		for (i = 0; i < drhd->devices_cnt; i++)
-			if (drhd->devices[i] &&
-			    !IS_GFX_DEVICE(drhd->devices[i]))
+		list_for_each_entry(dmar_dev, &drhd->head, list) {
+			pdev = pci_get_domain_bus_and_slot(dmar_dev->segment,
+				dmar_dev->bus, dmar_dev->devfn);
+			if (!IS_GFX_DEVICE(pdev)) {
+				pci_dev_put(pdev);
 				break;
+			}
+			pci_dev_put(pdev);
+		}
 
-		if (i < drhd->devices_cnt)
+		if (!IS_GFX_DEVICE(pdev))
 			continue;
 
 		/* This IOMMU has *only* gfx devices. Either bypass it or
@@ -3333,10 +3346,15 @@ static void __init init_no_remapping_devices(void)
 			intel_iommu_gfx_mapped = 1;
 		} else {
 			drhd->ignored = 1;
-			for (i = 0; i < drhd->devices_cnt; i++) {
-				if (!drhd->devices[i])
+			list_for_each_entry(dmar_dev, &drhd->head, list) {
+				pdev = pci_get_domain_bus_and_slot(
+						dmar_dev->segment, 
+						dmar_dev->bus, 
+						dmar_dev->devfn);
+				if (!pdev)
 					continue;
-				drhd->devices[i]->dev.archdata.iommu = DUMMY_DEVICE_DOMAIN_INFO;
+				pdev->dev.archdata.iommu = DUMMY_DEVICE_DOMAIN_INFO;
+				pci_dev_put(pdev);
 			}
 		}
 	}
@@ -3501,11 +3519,25 @@ int __init dmar_parse_one_rmrr(struct acpi_dmar_header *header)
 	rmrr = (struct acpi_dmar_reserved_memory *)header;
 	rmrru->base_address = rmrr->base_address;
 	rmrru->end_address = rmrr->end_address;
+	INIT_LIST_HEAD(&rmrru->head);
 
 	dmar_register_rmrr_unit(rmrru);
 	return 0;
 }
 
+static void rmrr_free(struct dmar_rmrr_unit *rmrru) 
+{
+	struct dmar_device *dev, *tmp;
+
+	list_for_each_entry_safe(dev, tmp, &rmrru->head,
+	    list) {
+		list_del(&dev->list);
+		kfree(dev);
+	}
+    
+	kfree(rmrru);
+}
+
 static int __init
 rmrr_parse_dev(struct dmar_rmrr_unit *rmrru)
 {
@@ -3515,11 +3547,11 @@ rmrr_parse_dev(struct dmar_rmrr_unit *rmrru)
 	rmrr = (struct acpi_dmar_reserved_memory *) rmrru->hdr;
 	ret = dmar_parse_dev_scope((void *)(rmrr + 1),
 		((void *)rmrr) + rmrr->header.length,
-		&rmrru->devices_cnt, &rmrru->devices, rmrr->segment);
+		rmrr->segment, &rmrru->head);
 
-	if (ret || (rmrru->devices_cnt == 0)) {
+	if (ret || list_empty(&rmrru->head)) {
 		list_del(&rmrru->list);
-		kfree(rmrru);
+		rmrr_free(rmrru);
 	}
 	return ret;
 }
@@ -3538,12 +3570,25 @@ int __init dmar_parse_one_atsr(struct acpi_dmar_header *hdr)
 
 	atsru->hdr = hdr;
 	atsru->include_all = atsr->flags & 0x1;
+	INIT_LIST_HEAD(&atsru->head);
 
 	list_add(&atsru->list, &dmar_atsr_units);
 
 	return 0;
 }
 
+static void atsr_free(struct dmar_atsr_unit *atsr) 
+{
+	struct dmar_device *dev, *tmp;
+    
+	list_for_each_entry_safe(dev, tmp, &atsr->head, list) {
+		list_del(&dev->list);
+		kfree(dev);
+	}
+
+	kfree(atsr);
+}
+
 static int __init atsr_parse_dev(struct dmar_atsr_unit *atsru)
 {
 	int rc;
@@ -3555,11 +3600,10 @@ static int __init atsr_parse_dev(struct dmar_atsr_unit *atsru)
 	atsr = container_of(atsru->hdr, struct acpi_dmar_atsr, header);
 	rc = dmar_parse_dev_scope((void *)(atsr + 1),
 				(void *)atsr + atsr->header.length,
-				&atsru->devices_cnt, &atsru->devices,
-				atsr->segment);
-	if (rc || !atsru->devices_cnt) {
+				atsr->segment, &atsru->head);
+	if (rc || list_empty(&atsru->head)) {
 		list_del(&atsru->list);
-		kfree(atsru);
+		atsr_free(atsru);
 	}
 
 	return rc;
@@ -3567,7 +3611,6 @@ static int __init atsr_parse_dev(struct dmar_atsr_unit *atsru)
 
 int dmar_find_matched_atsr_unit(struct pci_dev *dev)
 {
-	int i;
 	struct pci_bus *bus;
 	struct acpi_dmar_atsr *atsr;
 	struct dmar_atsr_unit *atsru;
@@ -3584,6 +3627,7 @@ int dmar_find_matched_atsr_unit(struct pci_dev *dev)
 
 found:
 	for (bus = dev->bus; bus; bus = bus->parent) {
+		struct dmar_device *dmar_dev;
 		struct pci_dev *bridge = bus->self;
 
 		if (!bridge || !pci_is_pcie(bridge) ||
@@ -3591,8 +3635,11 @@ found:
 			return 0;
 
 		if (pci_pcie_type(bridge) == PCI_EXP_TYPE_ROOT_PORT) {
-			for (i = 0; i < atsru->devices_cnt; i++)
-				if (atsru->devices[i] == bridge)
+			list_for_each_entry(dmar_dev, &atsru->head, list)
+				if (dmar_dev->segment == 
+					pci_domain_nr(bridge->bus) && 
+					dmar_dev->bus == bridge->bus->number &&
+					dmar_dev->devfn == bridge->devfn)
 					return 1;
 			break;
 		}
diff --git a/include/linux/dmar.h b/include/linux/dmar.h
index b029d1a..5317cb0 100644
--- a/include/linux/dmar.h
+++ b/include/linux/dmar.h
@@ -32,6 +32,13 @@ struct acpi_dmar_header;
 #define DMAR_INTR_REMAP		0x1
 #define DMAR_X2APIC_OPT_OUT	0x2
 
+struct dmar_device {
+	struct list_head list;
+	u8 segment;
+	u8 bus;
+	u8 devfn;
+};
+
 struct intel_iommu;
 #ifdef CONFIG_DMAR_TABLE
 extern struct acpi_table_header *dmar_tbl;
@@ -39,8 +46,7 @@ struct dmar_drhd_unit {
 	struct list_head list;		/* list of drhd units	*/
 	struct  acpi_dmar_header *hdr;	/* ACPI header		*/
 	u64	reg_base_addr;		/* register base address*/
-	struct	pci_dev **devices; 	/* target device array	*/
-	int	devices_cnt;		/* target device count	*/
+	struct list_head head;	/* target devices' list */
 	u16	segment;		/* PCI domain		*/
 	u8	ignored:1; 		/* ignore drhd		*/
 	u8	include_all:1;
@@ -139,8 +145,7 @@ struct dmar_rmrr_unit {
 	struct acpi_dmar_header *hdr;	/* ACPI header		*/
 	u64	base_address;		/* reserved base address*/
 	u64	end_address;		/* reserved end address */
-	struct pci_dev **devices;	/* target devices */
-	int	devices_cnt;		/* target device count */
+	struct list_head head;	/* target devices' list */
 };
 
 #define for_each_rmrr_units(rmrr) \
@@ -149,16 +154,15 @@ struct dmar_rmrr_unit {
 struct dmar_atsr_unit {
 	struct list_head list;		/* list of ATSR units */
 	struct acpi_dmar_header *hdr;	/* ACPI header */
-	struct pci_dev **devices;	/* target devices */
-	int devices_cnt;		/* target device count */
 	u8 include_all:1;		/* include all ports */
+	struct list_head head;	/* target devices' list */
 };
 
 int dmar_parse_rmrr_atsr_dev(void);
 extern int dmar_parse_one_rmrr(struct acpi_dmar_header *header);
 extern int dmar_parse_one_atsr(struct acpi_dmar_header *header);
-extern int dmar_parse_dev_scope(void *start, void *end, int *cnt,
-				struct pci_dev ***devices, u16 segment);
+extern int dmar_parse_dev_scope(void *start, void *end, u16 segment, 
+				struct list_head *head);
 extern int intel_iommu_init(void);
 #else /* !CONFIG_INTEL_IOMMU: */
 static inline int intel_iommu_init(void) { return -ENODEV; }
-- 
1.7.1


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Powered by blists - more mailing lists