[PATCH] pci, dmar: Update dmar units devices list during hotplug

[PATCH] pci, dmar: Update dmar units devices list during hotplug

[Yinghai Lu]

When do pci remove/rescan on system that have more iommus, got

[  894.089745] Set context mapping for c4:00.0
[  894.110890] mpt2sas3: Allocated physical memory: size(4293 kB)
[  894.112556] mpt2sas3: Current Controller Queue Depth(1883), Max Controller Queue Depth(2144)
[  894.127278] mpt2sas3: Scatter Gather Elements per IO(128)
[  894.361295] DRHD: handling fault status reg 2
[  894.364053] DMAR:[DMA Read] Request device [c4:00.0] fault addr fffbe000
[  894.364056] DMAR:[fault reason 02] Present bit in context entry is cl

it turns out when remove/rescan, pci dev will be freed and will get another new dev.
but drhd units still keep old one... so dmar_find_matched_drhd_unit will
return wrong drhd and iommu for the device that is not on first iommu.

So need to update devices in drhd_units during pci remove/rescan.

Could save domain/bus/device/function aside in the list and according that info
restore new dev to drhd_units later.
Then dmar_find_matched_drdh_unit and device_to_iommu could return right drhd and iommu.

Add remove_dev_from_drhd/restore_dev_to_drhd functions to do the real work.
call them in device ADD_DEVICE and UNBOUND_DRIVER

Need to do the samething to atsr.  (expose dmar_atsr_units and add atsru->segment)

After patch, will right iommu for the new dev and will not get DMAR error any more.

Signed-off-by: Yinghai Lu <yinghai@kernel.org>

---
 drivers/pci/dmar.c        |    7 -
 drivers/pci/intel-iommu.c |  177 ++++++++++++++++++++++++++++++++++++++++++++--
 include/linux/dmar.h      |    5 +
 3 files changed, 180 insertions(+), 9 deletions(-)

Index: linux-2.6/drivers/pci/dmar.c
===================================================================
--- linux-2.6.orig/drivers/pci/dmar.c
+++ linux-2.6/drivers/pci/dmar.c
@@ -263,7 +263,7 @@ rmrr_parse_dev(struct dmar_rmrr_unit *rm
 	return ret;
 }
 
-static LIST_HEAD(dmar_atsr_units);
+LIST_HEAD(dmar_atsr_units);
 
 static int __init dmar_parse_one_atsr(struct acpi_dmar_header *hdr)
 {
@@ -277,6 +277,7 @@ static int __init dmar_parse_one_atsr(st
 
 	atsru->hdr = hdr;
 	atsru->include_all = atsr->flags & 0x1;
+	atsru->segment = atsr->segment;
 
 	list_add(&atsru->list, &dmar_atsr_units);
 
@@ -308,14 +309,12 @@ int dmar_find_matched_atsr_unit(struct p
 {
 	int i;
 	struct pci_bus *bus;
-	struct acpi_dmar_atsr *atsr;
 	struct dmar_atsr_unit *atsru;
 
 	dev = pci_physfn(dev);
 
 	list_for_each_entry(atsru, &dmar_atsr_units, list) {
-		atsr = container_of(atsru->hdr, struct acpi_dmar_atsr, header);
-		if (atsr->segment == pci_domain_nr(dev->bus))
+		if (atsru->segment == pci_domain_nr(dev->bus))
 			goto found;
 	}
 
Index: linux-2.6/include/linux/dmar.h
===================================================================
--- linux-2.6.orig/include/linux/dmar.h
+++ linux-2.6/include/linux/dmar.h
@@ -219,14 +219,19 @@ struct dmar_rmrr_unit {
 #define for_each_rmrr_units(rmrr) \
 	list_for_each_entry(rmrr, &dmar_rmrr_units, list)
 
+extern struct list_head dmar_atsr_units;
 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 */
+	u16	segment;		/* PCI domain		*/
 	u8 include_all:1;		/* include all ports */
 };
 
+#define for_each_atsr_unit(atsr) \
+	list_for_each_entry(atsr, &dmar_atsr_units, list)
+
 extern int intel_iommu_init(void);
 #else /* !CONFIG_DMAR: */
 static inline int intel_iommu_init(void) { return -ENODEV; }
Index: linux-2.6/drivers/pci/intel-iommu.c
===================================================================
--- linux-2.6.orig/drivers/pci/intel-iommu.c
+++ linux-2.6/drivers/pci/intel-iommu.c
@@ -591,6 +591,157 @@ static struct intel_iommu *device_to_iom
 	return NULL;
 }
 
+#ifdef CONFIG_HOTPLUG
+struct dev_dmaru {
+	struct list_head list;
+	void *dmaru;
+	int index;
+	int segment;
+	unsigned char bus;
+	unsigned int devfn;
+};
+
+static int
+save_dev_dmaru(int segment, unsigned char bus, unsigned int devfn,
+		 void *dmaru, int index, struct list_head *lh)
+{
+	struct dev_dmaru *m;
+
+	m = kzalloc(sizeof(*m), GFP_KERNEL);
+	if (!m)
+		return -ENOMEM;
+
+	m->segment = segment;
+	m->bus     = bus;
+	m->devfn   = devfn;
+	m->dmaru   = dmaru;
+	m->index   = index;
+
+	list_add(&m->list, lh);
+
+	return 0;
+}
+
+static void
+*get_dev_dmaru(int segment, unsigned char bus, unsigned int devfn,
+		int *index, struct list_head *lh)
+{
+	struct dev_dmaru *m;
+	void *dmaru = NULL;
+
+	list_for_each_entry(m, lh, list) {
+		if (m->segment == segment &&
+		    m->bus == bus && m->devfn == devfn) {
+			*index = m->index;
+			dmaru  = m->dmaru;
+			list_del(&m->list);
+			kfree(m);
+			break;
+		}
+	}
+
+	return dmaru;
+}
+
+static LIST_HEAD(saved_dev_drhd_list);
+
+static void remove_dev_from_drhd(struct pci_dev *dev)
+{
+	struct dmar_drhd_unit *drhd = NULL;
+	int segment = pci_domain_nr(dev->bus);
+	int i;
+
+	for_each_drhd_unit(drhd) {
+		if (drhd->ignored)
+			continue;
+		if (segment != drhd->segment)
+			continue;
+
+		for (i = 0; i < drhd->devices_cnt; i++) {
+			if (drhd->devices[i] == dev) {
+				/* save it at first if it is in drhd */
+				save_dev_dmaru(segment, dev->bus->number,
+						dev->devfn, drhd, i,
+						&saved_dev_drhd_list);
+				/* always remove it */
+				drhd->devices[i] = NULL;
+				return;
+			}
+		}
+	}
+}
+
+static void restore_dev_to_drhd(struct pci_dev *dev)
+{
+	struct dmar_drhd_unit *drhd = NULL;
+	int i;
+
+	/* find the stored drhd */
+	drhd = get_dev_dmaru(pci_domain_nr(dev->bus), dev->bus->number,
+				 dev->devfn, &i, &saved_dev_drhd_list);
+	/* restore that into drhd */
+	if (drhd)
+		drhd->devices[i] = dev;
+}
+#else
+static void remove_dev_from_drhd(struct pci_dev *dev)
+{
+}
+
+static void restore_dev_to_drhd(struct pci_dev *dev)
+{
+}
+#endif
+
+#if defined(CONFIG_DMAR) && defined(CONFIG_HOTPLUG)
+static LIST_HEAD(saved_dev_atsr_list);
+
+static void remove_dev_from_atsr(struct pci_dev *dev)
+{
+	struct dmar_atsr_unit *atsr = NULL;
+	int segment = pci_domain_nr(dev->bus);
+	int i;
+
+	for_each_atsr_unit(atsr) {
+		if (segment != atsr->segment)
+			continue;
+
+		for (i = 0; i < atsr->devices_cnt; i++) {
+			if (atsr->devices[i] == dev) {
+				/* save it at first if it is in drhd */
+				save_dev_dmaru(segment, dev->bus->number,
+						dev->devfn, atsr, i,
+						&saved_dev_atsr_list);
+				/* always remove it */
+				atsr->devices[i] = NULL;
+				return;
+			}
+		}
+	}
+}
+
+static void restore_dev_to_atsr(struct pci_dev *dev)
+{
+	struct dmar_atsr_unit *atsr = NULL;
+	int i;
+
+	/* find the stored atsr */
+	atsr = get_dev_dmaru(pci_domain_nr(dev->bus), dev->bus->number,
+				 dev->devfn, &i, &saved_dev_atsr_list);
+	/* restore that into atsr */
+	if (atsr)
+		atsr->devices[i] = dev;
+}
+#else
+static void remove_dev_from_atsr(struct pci_dev *dev)
+{
+}
+
+static void restore_dev_to_atsr(struct pci_dev *dev)
+{
+}
+#endif
+
 static void domain_flush_cache(struct dmar_domain *domain,
 			       void *addr, int size)
 {
@@ -3246,20 +3397,36 @@ static int device_notifier(struct notifi
 	struct pci_dev *pdev = to_pci_dev(dev);
 	struct dmar_domain *domain;
 
-	if (iommu_no_mapping(dev))
+	if (unlikely(dev->bus != &pci_bus_type))
 		return 0;
 
-	domain = find_domain(pdev);
-	if (!domain)
-		return 0;
+	switch (action) {
+	case BUS_NOTIFY_UNBOUND_DRIVER:
+		if (iommu_no_mapping(dev))
+			goto out;
+
+		if (iommu_pass_through)
+			goto out;
+
+		domain = find_domain(pdev);
+		if (!domain)
+			goto out;
 
-	if (action == BUS_NOTIFY_UNBOUND_DRIVER && !iommu_pass_through) {
 		domain_remove_one_dev_info(domain, pdev);
 
 		if (!(domain->flags & DOMAIN_FLAG_VIRTUAL_MACHINE) &&
 		    !(domain->flags & DOMAIN_FLAG_STATIC_IDENTITY) &&
 		    list_empty(&domain->devices))
 			domain_exit(domain);
+out:
+		remove_dev_from_drhd(pdev);
+		remove_dev_from_atsr(pdev);
+
+		break;
+	case BUS_NOTIFY_ADD_DEVICE:
+		restore_dev_to_drhd(pdev);
+		restore_dev_to_atsr(pdev);
+		break;
 	}
 
 	return 0;

Re: [PATCH] pci, dmar: Update dmar units devices list during hotplug

[Alex Williamson]

On Sun, 2011-05-08 at 11:48 -0700, Yinghai Lu wrote:
> When do pci remove/rescan on system that have more iommus, got
> 
> [  894.089745] Set context mapping for c4:00.0
> [  894.110890] mpt2sas3: Allocated physical memory: size(4293 kB)
> [  894.112556] mpt2sas3: Current Controller Queue Depth(1883), Max Controller Queue Depth(2144)
> [  894.127278] mpt2sas3: Scatter Gather Elements per IO(128)
> [  894.361295] DRHD: handling fault status reg 2
> [  894.364053] DMAR:[DMA Read] Request device [c4:00.0] fault addr fffbe000
> [  894.364056] DMAR:[fault reason 02] Present bit in context entry is cl
> 
> it turns out when remove/rescan, pci dev will be freed and will get another new dev.
> but drhd units still keep old one... so dmar_find_matched_drhd_unit will
> return wrong drhd and iommu for the device that is not on first iommu.
> 
> So need to update devices in drhd_units during pci remove/rescan.
> 
> Could save domain/bus/device/function aside in the list and according that info
> restore new dev to drhd_units later.
> Then dmar_find_matched_drdh_unit and device_to_iommu could return right drhd and iommu.

I think I'd vote for saving some kind of representation of the bus
hierarchy, we probably don't need to list every possible individual
device.  Leaving a broken pointer around to be matched up and restored
later just seems like a continuation of an idea that was bad to begin
with.  Thanks,

Alex

Re: [PATCH] pci, dmar: Update dmar units devices list during hotplug

[David Woodhouse]

On Thu, 2011-05-19 at 16:15 -0600, Alex Williamson wrote:
> I think I'd vote for saving some kind of representation of the bus
> hierarchy, we probably don't need to list every possible individual
> device.  Leaving a broken pointer around to be matched up and restored
> later just seems like a continuation of an idea that was bad to begin
> with.  Thanks, 

I agree. We should just process the original ATSR information in
dmar_find_matched_drhd_unit(), rather than comparing with a list of
possibly stale pointers.

I don't quite understand why the list of PCI devices was *ever* done
like that.

-- 
David Woodhouse                            Open Source Technology Centre
David.Woodhouse@intel.com                              Intel Corporation

Re: [PATCH] pci, dmar: Update dmar units devices list during hotplug

[Alex Williamson]

On Tue, 2011-05-24 at 11:58 +0100, David Woodhouse wrote:
> On Thu, 2011-05-19 at 16:15 -0600, Alex Williamson wrote:
> > I think I'd vote for saving some kind of representation of the bus
> > hierarchy, we probably don't need to list every possible individual
> > device.  Leaving a broken pointer around to be matched up and restored
> > later just seems like a continuation of an idea that was bad to begin
> > with.  Thanks, 
> 
> I agree. We should just process the original ATSR information in
> dmar_find_matched_drhd_unit(), rather than comparing with a list of
> possibly stale pointers.
> 
> I don't quite understand why the list of PCI devices was *ever* done
> like that.

Yinghai,

I thought I might be running into something similar so spent some time
taking a different slant coding up the bug you found.  Turns out I
should have tested your patch first because I wasn't hitting that bug at
all.  The patch below is a work-in-progress that I think fixes the bug
by providing a quick means of re-parsing the scope as needed to match
current struct pci_devs.  It needs testing and cleanup, but feel free to
run with it (or ignore).  Just figured its better to post than waste the
code if you end up doing something similar.  Thanks,

Alex


Not for commit

Signed-off-by: Alex Williamson <alex.williamson@redhat.com>
---

diff --git a/drivers/pci/dmar.c b/drivers/pci/dmar.c
index 9cbeb0b..1d7bc58 100644
--- a/drivers/pci/dmar.c
+++ b/drivers/pci/dmar.c
@@ -59,8 +59,8 @@ static void __init dmar_register_drhd_unit(struct dmar_drhd_unit *drhd)
 		list_add(&drhd->list, &dmar_drhd_units);
 }
 
-static int __init dmar_parse_one_dev_scope(struct acpi_dmar_device_scope *scope,
-					   struct pci_dev **dev, u16 segment)
+struct pci_dev *dmar_get_scope_dev(struct acpi_dmar_device_scope *scope,
+				   u16 segment)
 {
 	struct pci_bus *bus;
 	struct pci_dev *pdev = NULL;
@@ -72,7 +72,7 @@ static int __init dmar_parse_one_dev_scope(struct acpi_dmar_device_scope *scope,
 	count = (scope->length - sizeof(struct acpi_dmar_device_scope))
 		/ sizeof(struct acpi_dmar_pci_path);
 
-	while (count) {
+	for (; count; path++, count--, bus = pdev->subordinate) {
 		if (pdev)
 			pci_dev_put(pdev);
 		/*
@@ -80,82 +80,103 @@ static int __init dmar_parse_one_dev_scope(struct acpi_dmar_device_scope *scope,
 		 * ignore it
 		 */
 		if (!bus) {
-			printk(KERN_WARNING
-			PREFIX "Device scope bus [%d] not found\n",
-			scope->bus);
-			break;
+			printk(KERN_WARNING PREFIX
+			       "Device scope bus [%d] not found\n", scope->bus);
+			return NULL;
 		}
 		pdev = pci_get_slot(bus, PCI_DEVFN(path->dev, path->fn));
 		if (!pdev) {
 			printk(KERN_WARNING PREFIX
-			"Device scope device [%04x:%02x:%02x.%02x] not found\n",
-				segment, bus->number, path->dev, path->fn);
-			break;
+			       "Device scope device [%04x:%02x:%02x.%02x] not found\n",
+			       segment, bus->number, path->dev, path->fn);
+			return NULL;
 		}
-		path ++;
-		count --;
-		bus = pdev->subordinate;
 	}
-	if (!pdev) {
-		printk(KERN_WARNING PREFIX
-		"Device scope device [%04x:%02x:%02x.%02x] not found\n",
-		segment, scope->bus, path->dev, path->fn);
-		*dev = NULL;
+
+	return pdev;
+}
+
+static int dmar_match_scope_one(struct acpi_dmar_device_scope *scope,
+				struct pci_dev *dev, u16 segment)
+{
+	struct pci_dev *pdev;
+	int ret = 0;
+
+	if (segment != pci_domain_nr(dev->bus))
 		return 0;
+
+	pdev = dmar_get_scope_dev(scope, segment);
+	if (!pdev)
+		return 0;
+
+	if (scope->entry_type == ACPI_DMAR_SCOPE_TYPE_ENDPOINT) {
+		if (dev == pdev)
+			ret = 1;
+	} else {
+		while (dev) {
+			if (dev == pdev) {
+				ret = 1;
+				break;
+			}
+			dev = dev->bus->self;
+		}
 	}
-	if ((scope->entry_type == ACPI_DMAR_SCOPE_TYPE_ENDPOINT && \
-			pdev->subordinate) || (scope->entry_type == \
-			ACPI_DMAR_SCOPE_TYPE_BRIDGE && !pdev->subordinate)) {
-		pci_dev_put(pdev);
-		printk(KERN_WARNING PREFIX
-			"Device scope type does not match for %s\n",
-			 pci_name(pdev));
-		return -EINVAL;
+
+	pci_dev_put(pdev);
+
+	return ret;
+}
+
+int dmar_match_scope(struct acpi_dmar_device_scope **scopes, int cnt,
+		     struct pci_dev *dev, u16 segment)
+{
+	int i;
+
+	for (i = 0; i < cnt; i++) {
+		if (dmar_match_scope_one(scopes[i], dev, segment))
+			return 1;
 	}
-	*dev = pdev;
 	return 0;
 }
 
 static int __init dmar_parse_dev_scope(void *start, void *end, int *cnt,
-				       struct pci_dev ***devices, u16 segment)
+				       struct acpi_dmar_device_scope ***scopes)
 {
 	struct acpi_dmar_device_scope *scope;
-	void * tmp = start;
-	int index;
-	int ret;
+	void *tmp = start;
+	int index = 0;
 
 	*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
 			printk(KERN_WARNING PREFIX
-				"Unsupported device scope\n");
+			       "Unsupported device scope\n");
+
 		start += scope->length;
 	}
+
 	if (*cnt == 0)
 		return 0;
 
-	*devices = kcalloc(*cnt, sizeof(struct pci_dev *), GFP_KERNEL);
-	if (!*devices)
+	*scopes = kcalloc(*cnt, sizeof(struct acpi_dmar_device_scope *),
+                          GFP_KERNEL);
+	if (!*scopes)
 		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);
-				return ret;
-			}
-			index ++;
-		}
+		    scope->entry_type == ACPI_DMAR_SCOPE_TYPE_BRIDGE)
+			(*scopes)[index++] = scope;
+
 		start += scope->length;
 	}
 
@@ -204,9 +225,8 @@ static int __init dmar_parse_dev(struct dmar_drhd_unit *dmaru)
 		return 0;
 
 	ret = dmar_parse_dev_scope((void *)(drhd + 1),
-				((void *)drhd) + drhd->header.length,
-				&dmaru->devices_cnt, &dmaru->devices,
-				drhd->segment);
+				   ((void *)drhd) + drhd->header.length,
+				   &dmaru->scopes_cnt, &dmaru->scopes);
 	if (ret) {
 		list_del(&dmaru->list);
 		kfree(dmaru);
@@ -250,10 +270,10 @@ 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);
+				   ((void *)rmrr) + rmrr->header.length,
+				   &rmrru->scopes_cnt, &rmrru->scopes);
 
-	if (ret || (rmrru->devices_cnt == 0)) {
+	if (ret || (rmrru->scopes_cnt == 0)) {
 		list_del(&rmrru->list);
 		kfree(rmrru);
 	}
@@ -290,10 +310,9 @@ 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) {
+				  (void *)atsr + atsr->header.length,
+				  &atsru->scopes_cnt, &atsru->scopes);
+	if (rc || !atsru->scopes_cnt) {
 		list_del(&atsru->list);
 		kfree(atsru);
 	}
@@ -307,6 +326,7 @@ int dmar_find_matched_atsr_unit(struct pci_dev *dev)
 	struct pci_bus *bus;
 	struct acpi_dmar_atsr *atsr;
 	struct dmar_atsr_unit *atsru;
+	struct pci_dev *pdev;
 
 	list_for_each_entry(atsru, &dmar_atsr_units, list) {
 		atsr = container_of(atsru->hdr, struct acpi_dmar_atsr, header);
@@ -325,10 +345,18 @@ found:
 			return 0;
 
 		if (bridge->pcie_type == PCI_EXP_TYPE_ROOT_PORT) {
-			for (i = 0; i < atsru->devices_cnt; i++)
-				if (atsru->devices[i] == bridge)
+			for (i = 0; i < atsru->scopes_cnt; i++) {
+				pdev = dmar_get_scope_dev(atsru->scopes[i],
+							  atsr->segment);
+				if (!pdev)
+					continue;
+
+				if (pdev == bridge) {
+					pci_dev_put(pdev);
 					return 1;
-			break;
+				}
+				pci_dev_put(pdev);
+			}
 		}
 	}
 
@@ -475,23 +503,6 @@ parse_dmar_table(void)
 	return ret;
 }
 
-int dmar_pci_device_match(struct pci_dev *devices[], int cnt,
-			  struct pci_dev *dev)
-{
-	int index;
-
-	while (dev) {
-		for (index = 0; index < cnt; index++)
-			if (dev == devices[index])
-				return 1;
-
-		/* Check our parent */
-		dev = dev->bus->self;
-	}
-
-	return 0;
-}
-
 struct dmar_drhd_unit *
 dmar_find_matched_drhd_unit(struct pci_dev *dev)
 {
@@ -504,11 +515,11 @@ dmar_find_matched_drhd_unit(struct pci_dev *dev)
 				    header);
 
 		if (dmaru->include_all &&
-		    drhd->segment == pci_domain_nr(dev->bus))
+		    dmaru->segment == pci_domain_nr(dev->bus))
 			return dmaru;
 
-		if (dmar_pci_device_match(dmaru->devices,
-					  dmaru->devices_cnt, dev))
+		if (dmar_match_scope(dmaru->scopes, dmaru->scopes_cnt,
+				     dev, dmaru->segment))
 			return dmaru;
 	}
 
diff --git a/drivers/pci/intel-iommu.c b/drivers/pci/intel-iommu.c
index e7ff5b8..7d0b095 100644
--- a/drivers/pci/intel-iommu.c
+++ b/drivers/pci/intel-iommu.c
@@ -541,32 +541,34 @@ 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_drhd_unit *dmaru = NULL;
+	struct pci_dev *pdev;
+	struct intel_iommu *found = NULL;
 
-	for_each_drhd_unit(drhd) {
-		if (drhd->ignored)
+	pdev = pci_get_domain_bus_and_slot(segment, bus, devfn);
+
+	for_each_drhd_unit(dmaru) {
+		if (dmaru->ignored)
 			continue;
-		if (segment != drhd->segment)
+		if (segment != dmaru->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->subordinate >= bus)
-				return drhd->iommu;
+		if (dmaru->include_all) {
+			found = dmaru->iommu;
+			break;
+		}
+
+		if (dmar_match_scope(dmaru->scopes, dmaru->scopes_cnt,
+				     pdev, dmaru->segment)) {
+			found = dmaru->iommu;
+			break;
 		}
 
-		if (drhd->include_all)
-			return drhd->iommu;
 	}
 
-	return NULL;
+	pci_dev_put(pdev);
+
+	return found;
 }
 
 static void domain_flush_cache(struct dmar_domain *domain,
@@ -2250,7 +2252,7 @@ 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
@@ -2397,18 +2399,22 @@ 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];
-			/*
-			 * some BIOS lists non-exist devices in DMAR
-			 * table.
-			 */
+		struct acpi_dmar_reserved_memory *rmrrh;
+		int i;
+
+		rmrrh = container_of(rmrr->hdr,
+				     struct acpi_dmar_reserved_memory, header);
+
+		for (i = 0; i < rmrr->scopes_cnt; i++) {
+			pdev = dmar_get_scope_dev(rmrr->scopes[i],
+						  rmrrh->segment);
 			if (!pdev)
 				continue;
-			ret = iommu_prepare_rmrr_dev(rmrr, pdev);
-			if (ret)
+
+			if (iommu_prepare_rmrr_dev(rmrr, pdev))
 				printk(KERN_ERR
 				       "IOMMU: mapping reserved region failed\n");
+			pci_dev_put(pdev);
 		}
 	}
 
@@ -3078,15 +3084,21 @@ 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 pci_dev *pdev;
 
 	for_each_drhd_unit(drhd) {
 		if (!drhd->include_all) {
 			int i;
-			for (i = 0; i < drhd->devices_cnt; i++)
-				if (drhd->devices[i] != NULL)
+			for (i = 0; i < drhd->scopes_cnt; i++) {
+				pdev = dmar_get_scope_dev(drhd->scopes[i],
+							  drhd->segment);
+				if (pdev) {
+					pci_dev_put(pdev);
 					break;
+				}
+			}
 			/* ignore DMAR unit if no pci devices exist */
-			if (i == drhd->devices_cnt)
+			if (i == drhd->scopes_cnt)
 				drhd->ignored = 1;
 		}
 	}
@@ -3099,20 +3111,28 @@ static void __init init_no_remapping_devices(void)
 		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]))
+		for (i = 0; i < drhd->scopes_cnt; i++) {
+			pdev = dmar_get_scope_dev(drhd->scopes[i],
+						  drhd->segment);
+			if (pdev && !IS_GFX_DEVICE(pdev)) {
+				pci_dev_put(pdev);
 				break;
+			}
+			pci_dev_put(pdev);
+		}
 
-		if (i < drhd->devices_cnt)
+		if (i < drhd->scopes_cnt)
 			continue;
 
 		/* bypass IOMMU if it is just for gfx devices */
 		drhd->ignored = 1;
-		for (i = 0; i < drhd->devices_cnt; i++) {
-			if (!drhd->devices[i])
+		for (i = 0; i < drhd->scopes_cnt; i++) {
+			pdev = dmar_get_scope_dev(drhd->scopes[i],
+						  drhd->segment);
+			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);
 		}
 	}
 }
diff --git a/include/linux/dmar.h b/include/linux/dmar.h
index 69a6fba..70b7e3c 100644
--- a/include/linux/dmar.h
+++ b/include/linux/dmar.h
@@ -32,8 +32,8 @@ 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	acpi_dmar_device_scope **scopes; /* target scope array	*/
+	int	scopes_cnt;		/* target scope count	*/
 	u16	segment;		/* PCI domain		*/
 	u8	ignored:1; 		/* ignore drhd		*/
 	u8	include_all:1;
@@ -55,6 +55,9 @@ extern struct list_head dmar_drhd_units;
 
 extern int dmar_table_init(void);
 extern int dmar_dev_scope_init(void);
+extern int dmar_match_scope(struct acpi_dmar_device_scope **, int,
+			    struct pci_dev *, u16);
+extern struct pci_dev *dmar_get_scope_dev(struct acpi_dmar_device_scope *, u16);
 
 /* Intel IOMMU detection */
 extern void detect_intel_iommu(void);
@@ -72,6 +75,20 @@ static inline int dmar_table_init(void)
 {
 	return -ENODEV;
 }
+
+static inline int dmar_match_scope(struct acpi_dmar_device_scope **scopes,
+				   int cnt, struct pci_dev *dev, u16 segment)
+{
+	return 0;
+}
+
+static inline struct pci_dev *dmar_get_scope_dev(
+					struct acpi_dmar_device_scope *scope,
+					u16 segment)
+{
+	return NULL;
+}
+
 static inline int enable_drhd_fault_handling(void)
 {
 	return -1;
@@ -203,8 +220,8 @@ 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	acpi_dmar_device_scope **scopes; /* target scope array */
+	int	scopes_cnt;		/* target scope count */
 };
 
 #define for_each_rmrr_units(rmrr) \
@@ -213,8 +230,8 @@ 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 */
+	struct acpi_dmar_device_scope **scopes;	/* target scope array */
+	int scopes_cnt;		/* target scope count */
 	u8 include_all:1;		/* include all ports */
 };
 

Re: [PATCH] pci, dmar: Update dmar units devices list during hotplug

[Yinghai Lu]

On 05/24/2011 10:42 AM, Alex Williamson wrote:
> On Tue, 2011-05-24 at 11:58 +0100, David Woodhouse wrote:
>> On Thu, 2011-05-19 at 16:15 -0600, Alex Williamson wrote:
>>> I think I'd vote for saving some kind of representation of the bus
>>> hierarchy, we probably don't need to list every possible individual
>>> device.  Leaving a broken pointer around to be matched up and restored
>>> later just seems like a continuation of an idea that was bad to begin
>>> with.  Thanks, 
>>
>> I agree. We should just process the original ATSR information in
>> dmar_find_matched_drhd_unit(), rather than comparing with a list of
>> possibly stale pointers.
>>
>> I don't quite understand why the list of PCI devices was *ever* done
>> like that.
> 
> Yinghai,
> 
> I thought I might be running into something similar so spent some time
> taking a different slant coding up the bug you found.  Turns out I
> should have tested your patch first because I wasn't hitting that bug at
> all.  The patch below is a work-in-progress that I think fixes the bug
> by providing a quick means of re-parsing the scope as needed to match
> current struct pci_devs.  It needs testing and cleanup, but feel free to
> run with it (or ignore).  Just figured its better to post than waste the
> code if you end up doing something similar.  Thanks,

yes, will try it.

Thanks

> 
> Alex
> 
> 
> Not for commit
> 
> Signed-off-by: Alex Williamson <alex.williamson@redhat.com>
> ---
> 
> diff --git a/drivers/pci/dmar.c b/drivers/pci/dmar.c
> index 9cbeb0b..1d7bc58 100644
> --- a/drivers/pci/dmar.c
> +++ b/drivers/pci/dmar.c
> @@ -59,8 +59,8 @@ static void __init dmar_register_drhd_unit(struct dmar_drhd_unit *drhd)
>  		list_add(&drhd->list, &dmar_drhd_units);
>  }
>  
> -static int __init dmar_parse_one_dev_scope(struct acpi_dmar_device_scope *scope,
> -					   struct pci_dev **dev, u16 segment)
> +struct pci_dev *dmar_get_scope_dev(struct acpi_dmar_device_scope *scope,
> +				   u16 segment)
>  {
>  	struct pci_bus *bus;
>  	struct pci_dev *pdev = NULL;
> @@ -72,7 +72,7 @@ static int __init dmar_parse_one_dev_scope(struct acpi_dmar_device_scope *scope,
>  	count = (scope->length - sizeof(struct acpi_dmar_device_scope))
>  		/ sizeof(struct acpi_dmar_pci_path);
>  
> -	while (count) {
> +	for (; count; path++, count--, bus = pdev->subordinate) {
>  		if (pdev)
>  			pci_dev_put(pdev);
>  		/*
> @@ -80,82 +80,103 @@ static int __init dmar_parse_one_dev_scope(struct acpi_dmar_device_scope *scope,
>  		 * ignore it
>  		 */
>  		if (!bus) {
> -			printk(KERN_WARNING
> -			PREFIX "Device scope bus [%d] not found\n",
> -			scope->bus);
> -			break;
> +			printk(KERN_WARNING PREFIX
> +			       "Device scope bus [%d] not found\n", scope->bus);
> +			return NULL;
>  		}
>  		pdev = pci_get_slot(bus, PCI_DEVFN(path->dev, path->fn));
>  		if (!pdev) {
>  			printk(KERN_WARNING PREFIX
> -			"Device scope device [%04x:%02x:%02x.%02x] not found\n",
> -				segment, bus->number, path->dev, path->fn);
> -			break;
> +			       "Device scope device [%04x:%02x:%02x.%02x] not found\n",
> +			       segment, bus->number, path->dev, path->fn);
> +			return NULL;
>  		}
> -		path ++;
> -		count --;
> -		bus = pdev->subordinate;
>  	}
> -	if (!pdev) {
> -		printk(KERN_WARNING PREFIX
> -		"Device scope device [%04x:%02x:%02x.%02x] not found\n",
> -		segment, scope->bus, path->dev, path->fn);
> -		*dev = NULL;
> +
> +	return pdev;
> +}
> +
> +static int dmar_match_scope_one(struct acpi_dmar_device_scope *scope,
> +				struct pci_dev *dev, u16 segment)
> +{
> +	struct pci_dev *pdev;
> +	int ret = 0;
> +
> +	if (segment != pci_domain_nr(dev->bus))
>  		return 0;
> +
> +	pdev = dmar_get_scope_dev(scope, segment);
> +	if (!pdev)
> +		return 0;
> +
> +	if (scope->entry_type == ACPI_DMAR_SCOPE_TYPE_ENDPOINT) {
> +		if (dev == pdev)
> +			ret = 1;
> +	} else {
> +		while (dev) {
> +			if (dev == pdev) {
> +				ret = 1;
> +				break;
> +			}
> +			dev = dev->bus->self;
> +		}
>  	}
> -	if ((scope->entry_type == ACPI_DMAR_SCOPE_TYPE_ENDPOINT && \
> -			pdev->subordinate) || (scope->entry_type == \
> -			ACPI_DMAR_SCOPE_TYPE_BRIDGE && !pdev->subordinate)) {
> -		pci_dev_put(pdev);
> -		printk(KERN_WARNING PREFIX
> -			"Device scope type does not match for %s\n",
> -			 pci_name(pdev));
> -		return -EINVAL;
> +
> +	pci_dev_put(pdev);
> +
> +	return ret;
> +}
> +
> +int dmar_match_scope(struct acpi_dmar_device_scope **scopes, int cnt,
> +		     struct pci_dev *dev, u16 segment)
> +{
> +	int i;
> +
> +	for (i = 0; i < cnt; i++) {
> +		if (dmar_match_scope_one(scopes[i], dev, segment))
> +			return 1;
>  	}
> -	*dev = pdev;
>  	return 0;
>  }
>  
>  static int __init dmar_parse_dev_scope(void *start, void *end, int *cnt,
> -				       struct pci_dev ***devices, u16 segment)
> +				       struct acpi_dmar_device_scope ***scopes)
>  {
>  	struct acpi_dmar_device_scope *scope;
> -	void * tmp = start;
> -	int index;
> -	int ret;
> +	void *tmp = start;
> +	int index = 0;
>  
>  	*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
>  			printk(KERN_WARNING PREFIX
> -				"Unsupported device scope\n");
> +			       "Unsupported device scope\n");
> +
>  		start += scope->length;
>  	}
> +
>  	if (*cnt == 0)
>  		return 0;
>  
> -	*devices = kcalloc(*cnt, sizeof(struct pci_dev *), GFP_KERNEL);
> -	if (!*devices)
> +	*scopes = kcalloc(*cnt, sizeof(struct acpi_dmar_device_scope *),
> +                          GFP_KERNEL);
> +	if (!*scopes)
>  		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);
> -				return ret;
> -			}
> -			index ++;
> -		}
> +		    scope->entry_type == ACPI_DMAR_SCOPE_TYPE_BRIDGE)
> +			(*scopes)[index++] = scope;
> +
>  		start += scope->length;
>  	}
>  
> @@ -204,9 +225,8 @@ static int __init dmar_parse_dev(struct dmar_drhd_unit *dmaru)
>  		return 0;
>  
>  	ret = dmar_parse_dev_scope((void *)(drhd + 1),
> -				((void *)drhd) + drhd->header.length,
> -				&dmaru->devices_cnt, &dmaru->devices,
> -				drhd->segment);
> +				   ((void *)drhd) + drhd->header.length,
> +				   &dmaru->scopes_cnt, &dmaru->scopes);
>  	if (ret) {
>  		list_del(&dmaru->list);
>  		kfree(dmaru);
> @@ -250,10 +270,10 @@ 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);
> +				   ((void *)rmrr) + rmrr->header.length,
> +				   &rmrru->scopes_cnt, &rmrru->scopes);
>  
> -	if (ret || (rmrru->devices_cnt == 0)) {
> +	if (ret || (rmrru->scopes_cnt == 0)) {
>  		list_del(&rmrru->list);
>  		kfree(rmrru);
>  	}
> @@ -290,10 +310,9 @@ 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) {
> +				  (void *)atsr + atsr->header.length,
> +				  &atsru->scopes_cnt, &atsru->scopes);
> +	if (rc || !atsru->scopes_cnt) {
>  		list_del(&atsru->list);
>  		kfree(atsru);
>  	}
> @@ -307,6 +326,7 @@ int dmar_find_matched_atsr_unit(struct pci_dev *dev)
>  	struct pci_bus *bus;
>  	struct acpi_dmar_atsr *atsr;
>  	struct dmar_atsr_unit *atsru;
> +	struct pci_dev *pdev;
>  
>  	list_for_each_entry(atsru, &dmar_atsr_units, list) {
>  		atsr = container_of(atsru->hdr, struct acpi_dmar_atsr, header);
> @@ -325,10 +345,18 @@ found:
>  			return 0;
>  
>  		if (bridge->pcie_type == PCI_EXP_TYPE_ROOT_PORT) {
> -			for (i = 0; i < atsru->devices_cnt; i++)
> -				if (atsru->devices[i] == bridge)
> +			for (i = 0; i < atsru->scopes_cnt; i++) {
> +				pdev = dmar_get_scope_dev(atsru->scopes[i],
> +							  atsr->segment);
> +				if (!pdev)
> +					continue;
> +
> +				if (pdev == bridge) {
> +					pci_dev_put(pdev);
>  					return 1;
> -			break;
> +				}
> +				pci_dev_put(pdev);
> +			}
>  		}
>  	}
>  
> @@ -475,23 +503,6 @@ parse_dmar_table(void)
>  	return ret;
>  }
>  
> -int dmar_pci_device_match(struct pci_dev *devices[], int cnt,
> -			  struct pci_dev *dev)
> -{
> -	int index;
> -
> -	while (dev) {
> -		for (index = 0; index < cnt; index++)
> -			if (dev == devices[index])
> -				return 1;
> -
> -		/* Check our parent */
> -		dev = dev->bus->self;
> -	}
> -
> -	return 0;
> -}
> -
>  struct dmar_drhd_unit *
>  dmar_find_matched_drhd_unit(struct pci_dev *dev)
>  {
> @@ -504,11 +515,11 @@ dmar_find_matched_drhd_unit(struct pci_dev *dev)
>  				    header);
>  
>  		if (dmaru->include_all &&
> -		    drhd->segment == pci_domain_nr(dev->bus))
> +		    dmaru->segment == pci_domain_nr(dev->bus))
>  			return dmaru;
>  
> -		if (dmar_pci_device_match(dmaru->devices,
> -					  dmaru->devices_cnt, dev))
> +		if (dmar_match_scope(dmaru->scopes, dmaru->scopes_cnt,
> +				     dev, dmaru->segment))
>  			return dmaru;
>  	}
>  
> diff --git a/drivers/pci/intel-iommu.c b/drivers/pci/intel-iommu.c
> index e7ff5b8..7d0b095 100644
> --- a/drivers/pci/intel-iommu.c
> +++ b/drivers/pci/intel-iommu.c
> @@ -541,32 +541,34 @@ 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_drhd_unit *dmaru = NULL;
> +	struct pci_dev *pdev;
> +	struct intel_iommu *found = NULL;
>  
> -	for_each_drhd_unit(drhd) {
> -		if (drhd->ignored)
> +	pdev = pci_get_domain_bus_and_slot(segment, bus, devfn);
> +
> +	for_each_drhd_unit(dmaru) {
> +		if (dmaru->ignored)
>  			continue;
> -		if (segment != drhd->segment)
> +		if (segment != dmaru->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->subordinate >= bus)
> -				return drhd->iommu;
> +		if (dmaru->include_all) {
> +			found = dmaru->iommu;
> +			break;
> +		}
> +
> +		if (dmar_match_scope(dmaru->scopes, dmaru->scopes_cnt,
> +				     pdev, dmaru->segment)) {
> +			found = dmaru->iommu;
> +			break;
>  		}
>  
> -		if (drhd->include_all)
> -			return drhd->iommu;
>  	}
>  
> -	return NULL;
> +	pci_dev_put(pdev);
> +
> +	return found;
>  }
>  
>  static void domain_flush_cache(struct dmar_domain *domain,
> @@ -2250,7 +2252,7 @@ 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
> @@ -2397,18 +2399,22 @@ 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];
> -			/*
> -			 * some BIOS lists non-exist devices in DMAR
> -			 * table.
> -			 */
> +		struct acpi_dmar_reserved_memory *rmrrh;
> +		int i;
> +
> +		rmrrh = container_of(rmrr->hdr,
> +				     struct acpi_dmar_reserved_memory, header);
> +
> +		for (i = 0; i < rmrr->scopes_cnt; i++) {
> +			pdev = dmar_get_scope_dev(rmrr->scopes[i],
> +						  rmrrh->segment);
>  			if (!pdev)
>  				continue;
> -			ret = iommu_prepare_rmrr_dev(rmrr, pdev);
> -			if (ret)
> +
> +			if (iommu_prepare_rmrr_dev(rmrr, pdev))
>  				printk(KERN_ERR
>  				       "IOMMU: mapping reserved region failed\n");
> +			pci_dev_put(pdev);
>  		}
>  	}
>  
> @@ -3078,15 +3084,21 @@ 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 pci_dev *pdev;
>  
>  	for_each_drhd_unit(drhd) {
>  		if (!drhd->include_all) {
>  			int i;
> -			for (i = 0; i < drhd->devices_cnt; i++)
> -				if (drhd->devices[i] != NULL)
> +			for (i = 0; i < drhd->scopes_cnt; i++) {
> +				pdev = dmar_get_scope_dev(drhd->scopes[i],
> +							  drhd->segment);
> +				if (pdev) {
> +					pci_dev_put(pdev);
>  					break;
> +				}
> +			}
>  			/* ignore DMAR unit if no pci devices exist */
> -			if (i == drhd->devices_cnt)
> +			if (i == drhd->scopes_cnt)
>  				drhd->ignored = 1;
>  		}
>  	}
> @@ -3099,20 +3111,28 @@ static void __init init_no_remapping_devices(void)
>  		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]))
> +		for (i = 0; i < drhd->scopes_cnt; i++) {
> +			pdev = dmar_get_scope_dev(drhd->scopes[i],
> +						  drhd->segment);
> +			if (pdev && !IS_GFX_DEVICE(pdev)) {
> +				pci_dev_put(pdev);
>  				break;
> +			}
> +			pci_dev_put(pdev);
> +		}
>  
> -		if (i < drhd->devices_cnt)
> +		if (i < drhd->scopes_cnt)
>  			continue;
>  
>  		/* bypass IOMMU if it is just for gfx devices */
>  		drhd->ignored = 1;
> -		for (i = 0; i < drhd->devices_cnt; i++) {
> -			if (!drhd->devices[i])
> +		for (i = 0; i < drhd->scopes_cnt; i++) {
> +			pdev = dmar_get_scope_dev(drhd->scopes[i],
> +						  drhd->segment);
> +			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);
>  		}
>  	}
>  }
> diff --git a/include/linux/dmar.h b/include/linux/dmar.h
> index 69a6fba..70b7e3c 100644
> --- a/include/linux/dmar.h
> +++ b/include/linux/dmar.h
> @@ -32,8 +32,8 @@ 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	acpi_dmar_device_scope **scopes; /* target scope array	*/
> +	int	scopes_cnt;		/* target scope count	*/
>  	u16	segment;		/* PCI domain		*/
>  	u8	ignored:1; 		/* ignore drhd		*/
>  	u8	include_all:1;
> @@ -55,6 +55,9 @@ extern struct list_head dmar_drhd_units;
>  
>  extern int dmar_table_init(void);
>  extern int dmar_dev_scope_init(void);
> +extern int dmar_match_scope(struct acpi_dmar_device_scope **, int,
> +			    struct pci_dev *, u16);
> +extern struct pci_dev *dmar_get_scope_dev(struct acpi_dmar_device_scope *, u16);
>  
>  /* Intel IOMMU detection */
>  extern void detect_intel_iommu(void);
> @@ -72,6 +75,20 @@ static inline int dmar_table_init(void)
>  {
>  	return -ENODEV;
>  }
> +
> +static inline int dmar_match_scope(struct acpi_dmar_device_scope **scopes,
> +				   int cnt, struct pci_dev *dev, u16 segment)
> +{
> +	return 0;
> +}
> +
> +static inline struct pci_dev *dmar_get_scope_dev(
> +					struct acpi_dmar_device_scope *scope,
> +					u16 segment)
> +{
> +	return NULL;
> +}
> +
>  static inline int enable_drhd_fault_handling(void)
>  {
>  	return -1;
> @@ -203,8 +220,8 @@ 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	acpi_dmar_device_scope **scopes; /* target scope array */
> +	int	scopes_cnt;		/* target scope count */
>  };
>  
>  #define for_each_rmrr_units(rmrr) \
> @@ -213,8 +230,8 @@ 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 */
> +	struct acpi_dmar_device_scope **scopes;	/* target scope array */
> +	int scopes_cnt;		/* target scope count */
>  	u8 include_all:1;		/* include all ports */
>  };
>  
> 

Re: [PATCH] pci, dmar: Update dmar units devices list during hotplug

[Yinghai Lu]

On 05/24/2011 10:42 AM, Alex Williamson wrote:
> On Tue, 2011-05-24 at 11:58 +0100, David Woodhouse wrote:
>> On Thu, 2011-05-19 at 16:15 -0600, Alex Williamson wrote:
>>> I think I'd vote for saving some kind of representation of the bus
>>> hierarchy, we probably don't need to list every possible individual
>>> device.  Leaving a broken pointer around to be matched up and restored
>>> later just seems like a continuation of an idea that was bad to begin
>>> with.  Thanks, 
>>
>> I agree. We should just process the original ATSR information in
>> dmar_find_matched_drhd_unit(), rather than comparing with a list of
>> possibly stale pointers.
>>
>> I don't quite understand why the list of PCI devices was *ever* done
>> like that.
> 
> Yinghai,
> 
> I thought I might be running into something similar so spent some time
> taking a different slant coding up the bug you found.  Turns out I
> should have tested your patch first because I wasn't hitting that bug at
> all.  The patch below is a work-in-progress that I think fixes the bug
> by providing a quick means of re-parsing the scope as needed to match
> current struct pci_devs.  It needs testing and cleanup, but feel free to
> run with it (or ignore).  Just figured its better to post than waste the
> code if you end up doing something similar.  Thanks,
> 
> Alex
> 

it does not apply to current linus tree cleanly.

> 
> Not for commit
> 
> Signed-off-by: Alex Williamson <alex.williamson@redhat.com>
> ---
> 
> diff --git a/drivers/pci/dmar.c b/drivers/pci/dmar.c
> index 9cbeb0b..1d7bc58 100644
> --- a/drivers/pci/dmar.c
> +++ b/drivers/pci/dmar.c
> @@ -59,8 +59,8 @@ static void __init dmar_register_drhd_unit(struct dmar_drhd_unit *drhd)
>  		list_add(&drhd->list, &dmar_drhd_units);
>  }
>  
> -static int __init dmar_parse_one_dev_scope(struct acpi_dmar_device_scope *scope,
> -					   struct pci_dev **dev, u16 segment)
> +struct pci_dev *dmar_get_scope_dev(struct acpi_dmar_device_scope *scope,
> +				   u16 segment)
>  {
>  	struct pci_bus *bus;
>  	struct pci_dev *pdev = NULL;
> @@ -72,7 +72,7 @@ static int __init dmar_parse_one_dev_scope(struct acpi_dmar_device_scope *scope,
>  	count = (scope->length - sizeof(struct acpi_dmar_device_scope))
>  		/ sizeof(struct acpi_dmar_pci_path);
>  
> -	while (count) {
> +	for (; count; path++, count--, bus = pdev->subordinate) {
>  		if (pdev)
>  			pci_dev_put(pdev);
>  		/*
> @@ -80,82 +80,103 @@ static int __init dmar_parse_one_dev_scope(struct acpi_dmar_device_scope *scope,
>  		 * ignore it
>  		 */
>  		if (!bus) {
> -			printk(KERN_WARNING
> -			PREFIX "Device scope bus [%d] not found\n",
> -			scope->bus);
> -			break;
> +			printk(KERN_WARNING PREFIX
> +			       "Device scope bus [%d] not found\n", scope->bus);
> +			return NULL;
>  		}
>  		pdev = pci_get_slot(bus, PCI_DEVFN(path->dev, path->fn));
>  		if (!pdev) {
>  			printk(KERN_WARNING PREFIX
> -			"Device scope device [%04x:%02x:%02x.%02x] not found\n",
> -				segment, bus->number, path->dev, path->fn);
> -			break;
> +			       "Device scope device [%04x:%02x:%02x.%02x] not found\n",
> +			       segment, bus->number, path->dev, path->fn);
> +			return NULL;
>  		}
> -		path ++;
> -		count --;
> -		bus = pdev->subordinate;
>  	}
> -	if (!pdev) {
> -		printk(KERN_WARNING PREFIX
> -		"Device scope device [%04x:%02x:%02x.%02x] not found\n",
> -		segment, scope->bus, path->dev, path->fn);
> -		*dev = NULL;
> +
> +	return pdev;
> +}
> +
> +static int dmar_match_scope_one(struct acpi_dmar_device_scope *scope,
> +				struct pci_dev *dev, u16 segment)
> +{
> +	struct pci_dev *pdev;
> +	int ret = 0;
> +
> +	if (segment != pci_domain_nr(dev->bus))
>  		return 0;
> +
> +	pdev = dmar_get_scope_dev(scope, segment);
> +	if (!pdev)
> +		return 0;
> +
> +	if (scope->entry_type == ACPI_DMAR_SCOPE_TYPE_ENDPOINT) {
> +		if (dev == pdev)
> +			ret = 1;
> +	} else {
> +		while (dev) {
> +			if (dev == pdev) {
> +				ret = 1;
> +				break;
> +			}
> +			dev = dev->bus->self;
> +		}
>  	}
> -	if ((scope->entry_type == ACPI_DMAR_SCOPE_TYPE_ENDPOINT && \
> -			pdev->subordinate) || (scope->entry_type == \
> -			ACPI_DMAR_SCOPE_TYPE_BRIDGE && !pdev->subordinate)) {
> -		pci_dev_put(pdev);
> -		printk(KERN_WARNING PREFIX
> -			"Device scope type does not match for %s\n",
> -			 pci_name(pdev));
> -		return -EINVAL;
> +
> +	pci_dev_put(pdev);
> +
> +	return ret;
> +}
> +
> +int dmar_match_scope(struct acpi_dmar_device_scope **scopes, int cnt,
> +		     struct pci_dev *dev, u16 segment)
> +{
> +	int i;
> +
> +	for (i = 0; i < cnt; i++) {
> +		if (dmar_match_scope_one(scopes[i], dev, segment))
> +			return 1;
>  	}
> -	*dev = pdev;
>  	return 0;
>  }
>  
>  static int __init dmar_parse_dev_scope(void *start, void *end, int *cnt,
> -				       struct pci_dev ***devices, u16 segment)
> +				       struct acpi_dmar_device_scope ***scopes)
>  {
>  	struct acpi_dmar_device_scope *scope;
> -	void * tmp = start;
> -	int index;
> -	int ret;
> +	void *tmp = start;
> +	int index = 0;
>  
>  	*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
>  			printk(KERN_WARNING PREFIX
> -				"Unsupported device scope\n");
> +			       "Unsupported device scope\n");
> +
>  		start += scope->length;
>  	}
> +
>  	if (*cnt == 0)
>  		return 0;
>  
> -	*devices = kcalloc(*cnt, sizeof(struct pci_dev *), GFP_KERNEL);
> -	if (!*devices)
> +	*scopes = kcalloc(*cnt, sizeof(struct acpi_dmar_device_scope *),
> +                          GFP_KERNEL);
> +	if (!*scopes)
>  		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);
> -				return ret;
> -			}
> -			index ++;
> -		}
> +		    scope->entry_type == ACPI_DMAR_SCOPE_TYPE_BRIDGE)
> +			(*scopes)[index++] = scope;
> +
>  		start += scope->length;
>  	}
>  
> @@ -204,9 +225,8 @@ static int __init dmar_parse_dev(struct dmar_drhd_unit *dmaru)
>  		return 0;
>  
>  	ret = dmar_parse_dev_scope((void *)(drhd + 1),
> -				((void *)drhd) + drhd->header.length,
> -				&dmaru->devices_cnt, &dmaru->devices,
> -				drhd->segment);
> +				   ((void *)drhd) + drhd->header.length,
> +				   &dmaru->scopes_cnt, &dmaru->scopes);
>  	if (ret) {
>  		list_del(&dmaru->list);
>  		kfree(dmaru);
> @@ -250,10 +270,10 @@ 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);
> +				   ((void *)rmrr) + rmrr->header.length,
> +				   &rmrru->scopes_cnt, &rmrru->scopes);
>  
> -	if (ret || (rmrru->devices_cnt == 0)) {
> +	if (ret || (rmrru->scopes_cnt == 0)) {
>  		list_del(&rmrru->list);
>  		kfree(rmrru);
>  	}
> @@ -290,10 +310,9 @@ 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) {
> +				  (void *)atsr + atsr->header.length,
> +				  &atsru->scopes_cnt, &atsru->scopes);
> +	if (rc || !atsru->scopes_cnt) {
>  		list_del(&atsru->list);
>  		kfree(atsru);
>  	}
> @@ -307,6 +326,7 @@ int dmar_find_matched_atsr_unit(struct pci_dev *dev)
>  	struct pci_bus *bus;
>  	struct acpi_dmar_atsr *atsr;
>  	struct dmar_atsr_unit *atsru;
> +	struct pci_dev *pdev;
>  
>  	list_for_each_entry(atsru, &dmar_atsr_units, list) {
>  		atsr = container_of(atsru->hdr, struct acpi_dmar_atsr, header);
> @@ -325,10 +345,18 @@ found:
>  			return 0;
>  
>  		if (bridge->pcie_type == PCI_EXP_TYPE_ROOT_PORT) {
> -			for (i = 0; i < atsru->devices_cnt; i++)
> -				if (atsru->devices[i] == bridge)
> +			for (i = 0; i < atsru->scopes_cnt; i++) {
> +				pdev = dmar_get_scope_dev(atsru->scopes[i],
> +							  atsr->segment);
> +				if (!pdev)
> +					continue;
> +
> +				if (pdev == bridge) {
> +					pci_dev_put(pdev);
>  					return 1;
> -			break;
> +				}
> +				pci_dev_put(pdev);
> +			}
>  		}
>  	}
>  
> @@ -475,23 +503,6 @@ parse_dmar_table(void)
>  	return ret;
>  }
>  
> -int dmar_pci_device_match(struct pci_dev *devices[], int cnt,
> -			  struct pci_dev *dev)
> -{
> -	int index;
> -
> -	while (dev) {
> -		for (index = 0; index < cnt; index++)
> -			if (dev == devices[index])
> -				return 1;
> -
> -		/* Check our parent */
> -		dev = dev->bus->self;
> -	}
> -
> -	return 0;
> -}
> -
>  struct dmar_drhd_unit *
>  dmar_find_matched_drhd_unit(struct pci_dev *dev)
>  {
> @@ -504,11 +515,11 @@ dmar_find_matched_drhd_unit(struct pci_dev *dev)
>  				    header);
>  
>  		if (dmaru->include_all &&
> -		    drhd->segment == pci_domain_nr(dev->bus))
> +		    dmaru->segment == pci_domain_nr(dev->bus))
>  			return dmaru;
>  
> -		if (dmar_pci_device_match(dmaru->devices,
> -					  dmaru->devices_cnt, dev))
> +		if (dmar_match_scope(dmaru->scopes, dmaru->scopes_cnt,
> +				     dev, dmaru->segment))
>  			return dmaru;
>  	}
>  
> diff --git a/drivers/pci/intel-iommu.c b/drivers/pci/intel-iommu.c
> index e7ff5b8..7d0b095 100644
> --- a/drivers/pci/intel-iommu.c
> +++ b/drivers/pci/intel-iommu.c
> @@ -541,32 +541,34 @@ 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_drhd_unit *dmaru = NULL;
> +	struct pci_dev *pdev;
> +	struct intel_iommu *found = NULL;
>  
> -	for_each_drhd_unit(drhd) {
> -		if (drhd->ignored)
> +	pdev = pci_get_domain_bus_and_slot(segment, bus, devfn);
> +
> +	for_each_drhd_unit(dmaru) {
> +		if (dmaru->ignored)
>  			continue;
> -		if (segment != drhd->segment)
> +		if (segment != dmaru->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->subordinate >= bus)
> -				return drhd->iommu;
> +		if (dmaru->include_all) {
> +			found = dmaru->iommu;
> +			break;
> +		}
> +
> +		if (dmar_match_scope(dmaru->scopes, dmaru->scopes_cnt,
> +				     pdev, dmaru->segment)) {
> +			found = dmaru->iommu;
> +			break;
>  		}
>  
> -		if (drhd->include_all)
> -			return drhd->iommu;
>  	}
>  
> -	return NULL;
> +	pci_dev_put(pdev);
> +
> +	return found;
>  }
>  
>  static void domain_flush_cache(struct dmar_domain *domain,
> @@ -2250,7 +2252,7 @@ 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
> @@ -2397,18 +2399,22 @@ 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];
> -			/*
> -			 * some BIOS lists non-exist devices in DMAR
> -			 * table.
> -			 */
> +		struct acpi_dmar_reserved_memory *rmrrh;
> +		int i;
> +
> +		rmrrh = container_of(rmrr->hdr,
> +				     struct acpi_dmar_reserved_memory, header);
> +
> +		for (i = 0; i < rmrr->scopes_cnt; i++) {
> +			pdev = dmar_get_scope_dev(rmrr->scopes[i],
> +						  rmrrh->segment);
>  			if (!pdev)
>  				continue;
> -			ret = iommu_prepare_rmrr_dev(rmrr, pdev);
> -			if (ret)
> +
> +			if (iommu_prepare_rmrr_dev(rmrr, pdev))
>  				printk(KERN_ERR
>  				       "IOMMU: mapping reserved region failed\n");
> +			pci_dev_put(pdev);
>  		}
>  	}
>  
> @@ -3078,15 +3084,21 @@ 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 pci_dev *pdev;
>  
>  	for_each_drhd_unit(drhd) {
>  		if (!drhd->include_all) {
>  			int i;
> -			for (i = 0; i < drhd->devices_cnt; i++)
> -				if (drhd->devices[i] != NULL)
> +			for (i = 0; i < drhd->scopes_cnt; i++) {
> +				pdev = dmar_get_scope_dev(drhd->scopes[i],
> +							  drhd->segment);
> +				if (pdev) {
> +					pci_dev_put(pdev);
>  					break;
> +				}
> +			}
>  			/* ignore DMAR unit if no pci devices exist */
> -			if (i == drhd->devices_cnt)
> +			if (i == drhd->scopes_cnt)
>  				drhd->ignored = 1;
>  		}
>  	}
> @@ -3099,20 +3111,28 @@ static void __init init_no_remapping_devices(void)
>  		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]))
> +		for (i = 0; i < drhd->scopes_cnt; i++) {
> +			pdev = dmar_get_scope_dev(drhd->scopes[i],
> +						  drhd->segment);
> +			if (pdev && !IS_GFX_DEVICE(pdev)) {
> +				pci_dev_put(pdev);
>  				break;
> +			}
> +			pci_dev_put(pdev);
> +		}
>  
> -		if (i < drhd->devices_cnt)
> +		if (i < drhd->scopes_cnt)
>  			continue;
>  
>  		/* bypass IOMMU if it is just for gfx devices */
>  		drhd->ignored = 1;
> -		for (i = 0; i < drhd->devices_cnt; i++) {
> -			if (!drhd->devices[i])
> +		for (i = 0; i < drhd->scopes_cnt; i++) {
> +			pdev = dmar_get_scope_dev(drhd->scopes[i],
> +						  drhd->segment);
> +			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);
>  		}
>  	}
>  }
> diff --git a/include/linux/dmar.h b/include/linux/dmar.h
> index 69a6fba..70b7e3c 100644
> --- a/include/linux/dmar.h
> +++ b/include/linux/dmar.h
> @@ -32,8 +32,8 @@ 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	acpi_dmar_device_scope **scopes; /* target scope array	*/
> +	int	scopes_cnt;		/* target scope count	*/
>  	u16	segment;		/* PCI domain		*/
>  	u8	ignored:1; 		/* ignore drhd		*/
>  	u8	include_all:1;
> @@ -55,6 +55,9 @@ extern struct list_head dmar_drhd_units;
>  
>  extern int dmar_table_init(void);
>  extern int dmar_dev_scope_init(void);
> +extern int dmar_match_scope(struct acpi_dmar_device_scope **, int,
> +			    struct pci_dev *, u16);
> +extern struct pci_dev *dmar_get_scope_dev(struct acpi_dmar_device_scope *, u16);
>  
>  /* Intel IOMMU detection */
>  extern void detect_intel_iommu(void);
> @@ -72,6 +75,20 @@ static inline int dmar_table_init(void)
>  {
>  	return -ENODEV;
>  }
> +
> +static inline int dmar_match_scope(struct acpi_dmar_device_scope **scopes,
> +				   int cnt, struct pci_dev *dev, u16 segment)
> +{
> +	return 0;
> +}
> +
> +static inline struct pci_dev *dmar_get_scope_dev(
> +					struct acpi_dmar_device_scope *scope,
> +					u16 segment)
> +{
> +	return NULL;
> +}
> +
>  static inline int enable_drhd_fault_handling(void)
>  {
>  	return -1;
> @@ -203,8 +220,8 @@ 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	acpi_dmar_device_scope **scopes; /* target scope array */
> +	int	scopes_cnt;		/* target scope count */
>  };
>  
>  #define for_each_rmrr_units(rmrr) \
> @@ -213,8 +230,8 @@ 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 */
> +	struct acpi_dmar_device_scope **scopes;	/* target scope array */
> +	int scopes_cnt;		/* target scope count */
>  	u8 include_all:1;		/* include all ports */
>  };
>  
> 

Re: [PATCH] pci, dmar: Update dmar units devices list during hotplug

[Alex Williamson]

On Tue, 2011-05-24 at 12:34 -0700, Yinghai Lu wrote:
> On 05/24/2011 10:42 AM, Alex Williamson wrote:
> > On Tue, 2011-05-24 at 11:58 +0100, David Woodhouse wrote:
> >> On Thu, 2011-05-19 at 16:15 -0600, Alex Williamson wrote:
> >>> I think I'd vote for saving some kind of representation of the bus
> >>> hierarchy, we probably don't need to list every possible individual
> >>> device.  Leaving a broken pointer around to be matched up and restored
> >>> later just seems like a continuation of an idea that was bad to begin
> >>> with.  Thanks, 
> >>
> >> I agree. We should just process the original ATSR information in
> >> dmar_find_matched_drhd_unit(), rather than comparing with a list of
> >> possibly stale pointers.
> >>
> >> I don't quite understand why the list of PCI devices was *ever* done
> >> like that.
> > 
> > Yinghai,
> > 
> > I thought I might be running into something similar so spent some time
> > taking a different slant coding up the bug you found.  Turns out I
> > should have tested your patch first because I wasn't hitting that bug at
> > all.  The patch below is a work-in-progress that I think fixes the bug
> > by providing a quick means of re-parsing the scope as needed to match
> > current struct pci_devs.  It needs testing and cleanup, but feel free to
> > run with it (or ignore).  Just figured its better to post than waste the
> > code if you end up doing something similar.  Thanks,
> > 
> > Alex
> > 
> 
> it does not apply to current linus tree cleanly.

Sorry, for some reason I started hacking on this against a rhel kernel.
Here's the compile tested-only forward port to 2.6.39 (plus the
domain_exit flush patch).  Thanks,

Alex


Not for commit

Signed-off-by: Alex Williamson <alex.williamson@redhat.com>
---

 drivers/pci/dmar.c        |  162 ++++++++++++++++++++++++---------------------
 drivers/pci/intel-iommu.c |   94 ++++++++++++++++----------
 include/linux/dmar.h      |   29 ++++++--
 3 files changed, 166 insertions(+), 119 deletions(-)


diff --git a/drivers/pci/dmar.c b/drivers/pci/dmar.c
index 12e02bf..47e4f09 100644
--- a/drivers/pci/dmar.c
+++ b/drivers/pci/dmar.c
@@ -61,8 +61,8 @@ static void __init dmar_register_drhd_unit(struct dmar_drhd_unit *drhd)
 		list_add(&drhd->list, &dmar_drhd_units);
 }
 
-static int __init dmar_parse_one_dev_scope(struct acpi_dmar_device_scope *scope,
-					   struct pci_dev **dev, u16 segment)
+struct pci_dev *dmar_get_scope_dev(struct acpi_dmar_device_scope *scope,
+				   u16 segment)
 {
 	struct pci_bus *bus;
 	struct pci_dev *pdev = NULL;
@@ -74,7 +74,7 @@ static int __init dmar_parse_one_dev_scope(struct acpi_dmar_device_scope *scope,
 	count = (scope->length - sizeof(struct acpi_dmar_device_scope))
 		/ sizeof(struct acpi_dmar_pci_path);
 
-	while (count) {
+	for (; count; path++, count--, bus = pdev->subordinate) {
 		if (pdev)
 			pci_dev_put(pdev);
 		/*
@@ -82,53 +82,77 @@ static int __init dmar_parse_one_dev_scope(struct acpi_dmar_device_scope *scope,
 		 * ignore it
 		 */
 		if (!bus) {
-			printk(KERN_WARNING
-			PREFIX "Device scope bus [%d] not found\n",
-			scope->bus);
-			break;
+			printk(KERN_WARNING PREFIX
+			       "Device scope bus [%d] not found\n", scope->bus);
+			return NULL;
 		}
 		pdev = pci_get_slot(bus, PCI_DEVFN(path->dev, path->fn));
 		if (!pdev) {
 			printk(KERN_WARNING PREFIX
-			"Device scope device [%04x:%02x:%02x.%02x] not found\n",
-				segment, bus->number, path->dev, path->fn);
-			break;
+			       "Device scope device [%04x:%02x:%02x.%02x] not found\n",
+			       segment, bus->number, path->dev, path->fn);
+			return NULL;
 		}
-		path ++;
-		count --;
-		bus = pdev->subordinate;
 	}
-	if (!pdev) {
-		printk(KERN_WARNING PREFIX
-		"Device scope device [%04x:%02x:%02x.%02x] not found\n",
-		segment, scope->bus, path->dev, path->fn);
-		*dev = NULL;
+
+	return pdev;
+}
+
+static int dmar_match_scope_one(struct acpi_dmar_device_scope *scope,
+				struct pci_dev *dev, u16 segment)
+{
+	struct pci_dev *pdev;
+	int ret = 0;
+
+	if (segment != pci_domain_nr(dev->bus))
+		return 0;
+
+	pdev = dmar_get_scope_dev(scope, segment);
+	if (!pdev)
 		return 0;
+
+	if (scope->entry_type == ACPI_DMAR_SCOPE_TYPE_ENDPOINT) {
+		if (dev == pdev)
+			ret = 1;
+	} else {
+		while (dev) {
+			if (dev == pdev) {
+				ret = 1;
+				break;
+			}
+			dev = dev->bus->self;
+		}
 	}
-	if ((scope->entry_type == ACPI_DMAR_SCOPE_TYPE_ENDPOINT && \
-			pdev->subordinate) || (scope->entry_type == \
-			ACPI_DMAR_SCOPE_TYPE_BRIDGE && !pdev->subordinate)) {
-		pci_dev_put(pdev);
-		printk(KERN_WARNING PREFIX
-			"Device scope type does not match for %s\n",
-			 pci_name(pdev));
-		return -EINVAL;
+
+	pci_dev_put(pdev);
+
+	return ret;
+}
+
+int dmar_match_scope(struct acpi_dmar_device_scope **scopes, int cnt,
+		     struct pci_dev *dev, u16 segment)
+{
+	int i;
+
+	for (i = 0; i < cnt; i++) {
+		if (dmar_match_scope_one(scopes[i], dev, segment))
+			return 1;
 	}
-	*dev = pdev;
 	return 0;
 }
 
 static int __init dmar_parse_dev_scope(void *start, void *end, int *cnt,
-				       struct pci_dev ***devices, u16 segment)
+				       struct acpi_dmar_device_scope ***scopes)
 {
 	struct acpi_dmar_device_scope *scope;
-	void * tmp = start;
-	int index;
-	int ret;
+	void *tmp = start;
+	int index = 0;
 
 	*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)++;
@@ -138,27 +162,23 @@ static int __init dmar_parse_dev_scope(void *start, void *end, int *cnt,
 		}
 		start += scope->length;
 	}
+
 	if (*cnt == 0)
 		return 0;
 
-	*devices = kcalloc(*cnt, sizeof(struct pci_dev *), GFP_KERNEL);
-	if (!*devices)
+	*scopes = kcalloc(*cnt, sizeof(struct acpi_dmar_device_scope *),
+                          GFP_KERNEL);
+	if (!*scopes)
 		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);
-				return ret;
-			}
-			index ++;
-		}
+		    scope->entry_type == ACPI_DMAR_SCOPE_TYPE_BRIDGE)
+			(*scopes)[index++] = scope;
+
 		start += scope->length;
 	}
 
@@ -207,9 +227,8 @@ static int __init dmar_parse_dev(struct dmar_drhd_unit *dmaru)
 		return 0;
 
 	ret = dmar_parse_dev_scope((void *)(drhd + 1),
-				((void *)drhd) + drhd->header.length,
-				&dmaru->devices_cnt, &dmaru->devices,
-				drhd->segment);
+				   ((void *)drhd) + drhd->header.length,
+				   &dmaru->scopes_cnt, &dmaru->scopes);
 	if (ret) {
 		list_del(&dmaru->list);
 		kfree(dmaru);
@@ -253,10 +272,10 @@ 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);
+				   ((void *)rmrr) + rmrr->header.length,
+				   &rmrru->scopes_cnt, &rmrru->scopes);
 
-	if (ret || (rmrru->devices_cnt == 0)) {
+	if (ret || (rmrru->scopes_cnt == 0)) {
 		list_del(&rmrru->list);
 		kfree(rmrru);
 	}
@@ -293,10 +312,9 @@ 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) {
+				  (void *)atsr + atsr->header.length,
+				  &atsru->scopes_cnt, &atsru->scopes);
+	if (rc || !atsru->scopes_cnt) {
 		list_del(&atsru->list);
 		kfree(atsru);
 	}
@@ -310,6 +328,7 @@ int dmar_find_matched_atsr_unit(struct pci_dev *dev)
 	struct pci_bus *bus;
 	struct acpi_dmar_atsr *atsr;
 	struct dmar_atsr_unit *atsru;
+	struct pci_dev *pdev;
 
 	dev = pci_physfn(dev);
 
@@ -330,10 +349,18 @@ found:
 			return 0;
 
 		if (bridge->pcie_type == PCI_EXP_TYPE_ROOT_PORT) {
-			for (i = 0; i < atsru->devices_cnt; i++)
-				if (atsru->devices[i] == bridge)
+			for (i = 0; i < atsru->scopes_cnt; i++) {
+				pdev = dmar_get_scope_dev(atsru->scopes[i],
+							  atsr->segment);
+				if (!pdev)
+					continue;
+
+				if (pdev == bridge) {
+					pci_dev_put(pdev);
 					return 1;
-			break;
+				}
+				pci_dev_put(pdev);
+			}
 		}
 	}
 
@@ -513,23 +540,6 @@ parse_dmar_table(void)
 	return ret;
 }
 
-static int dmar_pci_device_match(struct pci_dev *devices[], int cnt,
-			  struct pci_dev *dev)
-{
-	int index;
-
-	while (dev) {
-		for (index = 0; index < cnt; index++)
-			if (dev == devices[index])
-				return 1;
-
-		/* Check our parent */
-		dev = dev->bus->self;
-	}
-
-	return 0;
-}
-
 struct dmar_drhd_unit *
 dmar_find_matched_drhd_unit(struct pci_dev *dev)
 {
@@ -544,11 +554,11 @@ dmar_find_matched_drhd_unit(struct pci_dev *dev)
 				    header);
 
 		if (dmaru->include_all &&
-		    drhd->segment == pci_domain_nr(dev->bus))
+		    dmaru->segment == pci_domain_nr(dev->bus))
 			return dmaru;
 
-		if (dmar_pci_device_match(dmaru->devices,
-					  dmaru->devices_cnt, dev))
+		if (dmar_match_scope(dmaru->scopes, dmaru->scopes_cnt,
+				     dev, dmaru->segment))
 			return dmaru;
 	}
 
diff --git a/drivers/pci/intel-iommu.c b/drivers/pci/intel-iommu.c
index b04f84e..d1d542a 100644
--- a/drivers/pci/intel-iommu.c
+++ b/drivers/pci/intel-iommu.c
@@ -563,32 +563,34 @@ 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_drhd_unit *dmaru = NULL;
+	struct pci_dev *pdev;
+	struct intel_iommu *found = NULL;
 
-	for_each_drhd_unit(drhd) {
-		if (drhd->ignored)
+	pdev = pci_get_domain_bus_and_slot(segment, bus, devfn);
+
+	for_each_drhd_unit(dmaru) {
+		if (dmaru->ignored)
 			continue;
-		if (segment != drhd->segment)
+		if (segment != dmaru->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->subordinate >= bus)
-				return drhd->iommu;
+		if (dmaru->include_all) {
+			found = dmaru->iommu;
+			break;
+		}
+
+		if (dmar_match_scope(dmaru->scopes, dmaru->scopes_cnt,
+				     pdev, dmaru->segment)) {
+			found = dmaru->iommu;
+			break;
 		}
 
-		if (drhd->include_all)
-			return drhd->iommu;
 	}
 
-	return NULL;
+	pci_dev_put(pdev);
+
+	return found;
 }
 
 static void domain_flush_cache(struct dmar_domain *domain,
@@ -2227,7 +2229,7 @@ static int __init init_dmars(int force_on)
 	struct dmar_rmrr_unit *rmrr;
 	struct pci_dev *pdev;
 	struct intel_iommu *iommu;
-	int i, ret;
+	int ret;
 
 	/*
 	 * for each drhd
@@ -2376,18 +2378,22 @@ static int __init init_dmars(int force_on)
 	 */
 	printk(KERN_INFO "IOMMU: Setting RMRR:\n");
 	for_each_rmrr_units(rmrr) {
-		for (i = 0; i < rmrr->devices_cnt; i++) {
-			pdev = rmrr->devices[i];
-			/*
-			 * some BIOS lists non-exist devices in DMAR
-			 * table.
-			 */
+		struct acpi_dmar_reserved_memory *rmrrh;
+		int i;
+
+		rmrrh = container_of(rmrr->hdr,
+				     struct acpi_dmar_reserved_memory, header);
+
+		for (i = 0; i < rmrr->scopes_cnt; i++) {
+			pdev = dmar_get_scope_dev(rmrr->scopes[i],
+						  rmrrh->segment);
 			if (!pdev)
 				continue;
-			ret = iommu_prepare_rmrr_dev(rmrr, pdev);
-			if (ret)
+
+			if (iommu_prepare_rmrr_dev(rmrr, pdev))
 				printk(KERN_ERR
 				       "IOMMU: mapping reserved region failed\n");
+			pci_dev_put(pdev);
 		}
 	}
 
@@ -3072,15 +3078,21 @@ 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 pci_dev *pdev;
 
 	for_each_drhd_unit(drhd) {
 		if (!drhd->include_all) {
 			int i;
-			for (i = 0; i < drhd->devices_cnt; i++)
-				if (drhd->devices[i] != NULL)
+			for (i = 0; i < drhd->scopes_cnt; i++) {
+				pdev = dmar_get_scope_dev(drhd->scopes[i],
+							  drhd->segment);
+				if (pdev) {
+					pci_dev_put(pdev);
 					break;
+				}
+			}
 			/* ignore DMAR unit if no pci devices exist */
-			if (i == drhd->devices_cnt)
+			if (i == drhd->scopes_cnt)
 				drhd->ignored = 1;
 		}
 	}
@@ -3093,20 +3105,28 @@ static void __init init_no_remapping_devices(void)
 		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]))
+		for (i = 0; i < drhd->scopes_cnt; i++) {
+			pdev = dmar_get_scope_dev(drhd->scopes[i],
+						  drhd->segment);
+			if (pdev && !IS_GFX_DEVICE(pdev)) {
+				pci_dev_put(pdev);
 				break;
+			}
+			pci_dev_put(pdev);
+		}
 
-		if (i < drhd->devices_cnt)
+		if (i < drhd->scopes_cnt)
 			continue;
 
 		/* bypass IOMMU if it is just for gfx devices */
 		drhd->ignored = 1;
-		for (i = 0; i < drhd->devices_cnt; i++) {
-			if (!drhd->devices[i])
+		for (i = 0; i < drhd->scopes_cnt; i++) {
+			pdev = dmar_get_scope_dev(drhd->scopes[i],
+						  drhd->segment);
+			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);
 		}
 	}
 }
diff --git a/include/linux/dmar.h b/include/linux/dmar.h
index 7b776d7..cf071f9 100644
--- a/include/linux/dmar.h
+++ b/include/linux/dmar.h
@@ -32,8 +32,8 @@ 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	acpi_dmar_device_scope **scopes; /* target scope array	*/
+	int	scopes_cnt;		/* target scope count	*/
 	u16	segment;		/* PCI domain		*/
 	u8	ignored:1; 		/* ignore drhd		*/
 	u8	include_all:1;
@@ -55,6 +55,9 @@ extern struct list_head dmar_drhd_units;
 
 extern int dmar_table_init(void);
 extern int dmar_dev_scope_init(void);
+extern int dmar_match_scope(struct acpi_dmar_device_scope **, int,
+			    struct pci_dev *, u16);
+extern struct pci_dev *dmar_get_scope_dev(struct acpi_dmar_device_scope *, u16);
 
 /* Intel IOMMU detection */
 extern int detect_intel_iommu(void);
@@ -72,6 +75,20 @@ static inline int dmar_table_init(void)
 {
 	return -ENODEV;
 }
+
+static inline int dmar_match_scope(struct acpi_dmar_device_scope **scopes,
+				   int cnt, struct pci_dev *dev, u16 segment)
+{
+	return 0;
+}
+
+static inline struct pci_dev *dmar_get_scope_dev(
+					struct acpi_dmar_device_scope *scope,
+					u16 segment)
+{
+	return NULL;
+}
+
 static inline int enable_drhd_fault_handling(void)
 {
 	return -1;
@@ -212,8 +229,8 @@ 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	acpi_dmar_device_scope **scopes; /* target scope array */
+	int	scopes_cnt;		/* target scope count */
 };
 
 #define for_each_rmrr_units(rmrr) \
@@ -222,8 +239,8 @@ 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 */
+	struct acpi_dmar_device_scope **scopes;	/* target scope array */
+	int scopes_cnt;		/* target scope count */
 	u8 include_all:1;		/* include all ports */
 };
 

Re: [PATCH] pci, dmar: Update dmar units devices list during hotplug

[Yinghai Lu]

On 05/24/2011 01:07 PM, Alex Williamson wrote:
> On Tue, 2011-05-24 at 12:34 -0700, Yinghai Lu wrote:
>> On 05/24/2011 10:42 AM, Alex Williamson wrote:
>>> On Tue, 2011-05-24 at 11:58 +0100, David Woodhouse wrote:
>>>> On Thu, 2011-05-19 at 16:15 -0600, Alex Williamson wrote:
>>>>> I think I'd vote for saving some kind of representation of the bus
>>>>> hierarchy, we probably don't need to list every possible individual
>>>>> device.  Leaving a broken pointer around to be matched up and restored
>>>>> later just seems like a continuation of an idea that was bad to begin
>>>>> with.  Thanks, 
>>>>
>>>> I agree. We should just process the original ATSR information in
>>>> dmar_find_matched_drhd_unit(), rather than comparing with a list of
>>>> possibly stale pointers.
>>>>
>>>> I don't quite understand why the list of PCI devices was *ever* done
>>>> like that.
>>>
>>> Yinghai,
>>>
>>> I thought I might be running into something similar so spent some time
>>> taking a different slant coding up the bug you found.  Turns out I
>>> should have tested your patch first because I wasn't hitting that bug at
>>> all.  The patch below is a work-in-progress that I think fixes the bug
>>> by providing a quick means of re-parsing the scope as needed to match
>>> current struct pci_devs.  It needs testing and cleanup, but feel free to
>>> run with it (or ignore).  Just figured its better to post than waste the
>>> code if you end up doing something similar.  Thanks,
>>>
>>> Alex
>>>
>>
>> it does not apply to current linus tree cleanly.
> 
> Sorry, for some reason I started hacking on this against a rhel kernel.
> Here's the compile tested-only forward port to 2.6.39 (plus the
> domain_exit flush patch).  Thanks,

can not find anywhere to call flush_unmaps_timeout(0).

so what do you mean flush patch?

> 
> Alex
> 
> 
> Not for commit
> 
> Signed-off-by: Alex Williamson <alex.williamson@redhat.com>
> ---
> 
>  drivers/pci/dmar.c        |  162 ++++++++++++++++++++++++---------------------
>  drivers/pci/intel-iommu.c |   94 ++++++++++++++++----------
>  include/linux/dmar.h      |   29 ++++++--
>  3 files changed, 166 insertions(+), 119 deletions(-)
> 
> 
> diff --git a/drivers/pci/dmar.c b/drivers/pci/dmar.c
> index 12e02bf..47e4f09 100644
> --- a/drivers/pci/dmar.c
> +++ b/drivers/pci/dmar.c
> @@ -61,8 +61,8 @@ static void __init dmar_register_drhd_unit(struct dmar_drhd_unit *drhd)
>  		list_add(&drhd->list, &dmar_drhd_units);
>  }
>  
> -static int __init dmar_parse_one_dev_scope(struct acpi_dmar_device_scope *scope,
> -					   struct pci_dev **dev, u16 segment)
> +struct pci_dev *dmar_get_scope_dev(struct acpi_dmar_device_scope *scope,
> +				   u16 segment)
>  {
>  	struct pci_bus *bus;
>  	struct pci_dev *pdev = NULL;
> @@ -74,7 +74,7 @@ static int __init dmar_parse_one_dev_scope(struct acpi_dmar_device_scope *scope,
>  	count = (scope->length - sizeof(struct acpi_dmar_device_scope))
>  		/ sizeof(struct acpi_dmar_pci_path);
>  
> -	while (count) {
> +	for (; count; path++, count--, bus = pdev->subordinate) {
>  		if (pdev)
>  			pci_dev_put(pdev);
>  		/*
> @@ -82,53 +82,77 @@ static int __init dmar_parse_one_dev_scope(struct acpi_dmar_device_scope *scope,
>  		 * ignore it
>  		 */
>  		if (!bus) {
> -			printk(KERN_WARNING
> -			PREFIX "Device scope bus [%d] not found\n",
> -			scope->bus);
> -			break;
> +			printk(KERN_WARNING PREFIX
> +			       "Device scope bus [%d] not found\n", scope->bus);
> +			return NULL;
>  		}
>  		pdev = pci_get_slot(bus, PCI_DEVFN(path->dev, path->fn));
>  		if (!pdev) {
>  			printk(KERN_WARNING PREFIX
> -			"Device scope device [%04x:%02x:%02x.%02x] not found\n",
> -				segment, bus->number, path->dev, path->fn);
> -			break;
> +			       "Device scope device [%04x:%02x:%02x.%02x] not found\n",
> +			       segment, bus->number, path->dev, path->fn);
> +			return NULL;
>  		}
> -		path ++;
> -		count --;
> -		bus = pdev->subordinate;
>  	}
> -	if (!pdev) {
> -		printk(KERN_WARNING PREFIX
> -		"Device scope device [%04x:%02x:%02x.%02x] not found\n",
> -		segment, scope->bus, path->dev, path->fn);
> -		*dev = NULL;
> +
> +	return pdev;
> +}
> +
> +static int dmar_match_scope_one(struct acpi_dmar_device_scope *scope,
> +				struct pci_dev *dev, u16 segment)
> +{
> +	struct pci_dev *pdev;
> +	int ret = 0;
> +
> +	if (segment != pci_domain_nr(dev->bus))
> +		return 0;
> +
> +	pdev = dmar_get_scope_dev(scope, segment);
> +	if (!pdev)
>  		return 0;
> +
> +	if (scope->entry_type == ACPI_DMAR_SCOPE_TYPE_ENDPOINT) {
> +		if (dev == pdev)
> +			ret = 1;
> +	} else {
> +		while (dev) {
> +			if (dev == pdev) {
> +				ret = 1;
> +				break;
> +			}
> +			dev = dev->bus->self;
> +		}
>  	}
> -	if ((scope->entry_type == ACPI_DMAR_SCOPE_TYPE_ENDPOINT && \
> -			pdev->subordinate) || (scope->entry_type == \
> -			ACPI_DMAR_SCOPE_TYPE_BRIDGE && !pdev->subordinate)) {
> -		pci_dev_put(pdev);
> -		printk(KERN_WARNING PREFIX
> -			"Device scope type does not match for %s\n",
> -			 pci_name(pdev));
> -		return -EINVAL;
> +
> +	pci_dev_put(pdev);
> +
> +	return ret;
> +}
> +
> +int dmar_match_scope(struct acpi_dmar_device_scope **scopes, int cnt,
> +		     struct pci_dev *dev, u16 segment)
> +{
> +	int i;
> +
> +	for (i = 0; i < cnt; i++) {
> +		if (dmar_match_scope_one(scopes[i], dev, segment))
> +			return 1;
>  	}
> -	*dev = pdev;
>  	return 0;
>  }
>  
>  static int __init dmar_parse_dev_scope(void *start, void *end, int *cnt,
> -				       struct pci_dev ***devices, u16 segment)
> +				       struct acpi_dmar_device_scope ***scopes)
>  {
>  	struct acpi_dmar_device_scope *scope;
> -	void * tmp = start;
> -	int index;
> -	int ret;
> +	void *tmp = start;
> +	int index = 0;
>  
>  	*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)++;
> @@ -138,27 +162,23 @@ static int __init dmar_parse_dev_scope(void *start, void *end, int *cnt,
>  		}
>  		start += scope->length;
>  	}
> +
>  	if (*cnt == 0)
>  		return 0;
>  
> -	*devices = kcalloc(*cnt, sizeof(struct pci_dev *), GFP_KERNEL);
> -	if (!*devices)
> +	*scopes = kcalloc(*cnt, sizeof(struct acpi_dmar_device_scope *),
> +                          GFP_KERNEL);
> +	if (!*scopes)
>  		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);
> -				return ret;
> -			}
> -			index ++;
> -		}
> +		    scope->entry_type == ACPI_DMAR_SCOPE_TYPE_BRIDGE)
> +			(*scopes)[index++] = scope;
> +
>  		start += scope->length;
>  	}
>  
> @@ -207,9 +227,8 @@ static int __init dmar_parse_dev(struct dmar_drhd_unit *dmaru)
>  		return 0;
>  
>  	ret = dmar_parse_dev_scope((void *)(drhd + 1),
> -				((void *)drhd) + drhd->header.length,
> -				&dmaru->devices_cnt, &dmaru->devices,
> -				drhd->segment);
> +				   ((void *)drhd) + drhd->header.length,
> +				   &dmaru->scopes_cnt, &dmaru->scopes);
>  	if (ret) {
>  		list_del(&dmaru->list);
>  		kfree(dmaru);
> @@ -253,10 +272,10 @@ 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);
> +				   ((void *)rmrr) + rmrr->header.length,
> +				   &rmrru->scopes_cnt, &rmrru->scopes);
>  
> -	if (ret || (rmrru->devices_cnt == 0)) {
> +	if (ret || (rmrru->scopes_cnt == 0)) {
>  		list_del(&rmrru->list);
>  		kfree(rmrru);
>  	}
> @@ -293,10 +312,9 @@ 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) {
> +				  (void *)atsr + atsr->header.length,
> +				  &atsru->scopes_cnt, &atsru->scopes);
> +	if (rc || !atsru->scopes_cnt) {
>  		list_del(&atsru->list);
>  		kfree(atsru);
>  	}
> @@ -310,6 +328,7 @@ int dmar_find_matched_atsr_unit(struct pci_dev *dev)
>  	struct pci_bus *bus;
>  	struct acpi_dmar_atsr *atsr;
>  	struct dmar_atsr_unit *atsru;
> +	struct pci_dev *pdev;
>  
>  	dev = pci_physfn(dev);
>  
> @@ -330,10 +349,18 @@ found:
>  			return 0;
>  
>  		if (bridge->pcie_type == PCI_EXP_TYPE_ROOT_PORT) {
> -			for (i = 0; i < atsru->devices_cnt; i++)
> -				if (atsru->devices[i] == bridge)
> +			for (i = 0; i < atsru->scopes_cnt; i++) {
> +				pdev = dmar_get_scope_dev(atsru->scopes[i],
> +							  atsr->segment);
> +				if (!pdev)
> +					continue;
> +
> +				if (pdev == bridge) {
> +					pci_dev_put(pdev);
>  					return 1;
> -			break;
> +				}
> +				pci_dev_put(pdev);
> +			}
>  		}
>  	}
>  
> @@ -513,23 +540,6 @@ parse_dmar_table(void)
>  	return ret;
>  }
>  
> -static int dmar_pci_device_match(struct pci_dev *devices[], int cnt,
> -			  struct pci_dev *dev)
> -{
> -	int index;
> -
> -	while (dev) {
> -		for (index = 0; index < cnt; index++)
> -			if (dev == devices[index])
> -				return 1;
> -
> -		/* Check our parent */
> -		dev = dev->bus->self;
> -	}
> -
> -	return 0;
> -}
> -
>  struct dmar_drhd_unit *
>  dmar_find_matched_drhd_unit(struct pci_dev *dev)
>  {
> @@ -544,11 +554,11 @@ dmar_find_matched_drhd_unit(struct pci_dev *dev)
>  				    header);
>  
>  		if (dmaru->include_all &&
> -		    drhd->segment == pci_domain_nr(dev->bus))
> +		    dmaru->segment == pci_domain_nr(dev->bus))
>  			return dmaru;
>  
> -		if (dmar_pci_device_match(dmaru->devices,
> -					  dmaru->devices_cnt, dev))
> +		if (dmar_match_scope(dmaru->scopes, dmaru->scopes_cnt,
> +				     dev, dmaru->segment))
>  			return dmaru;
>  	}
>  
> diff --git a/drivers/pci/intel-iommu.c b/drivers/pci/intel-iommu.c
> index b04f84e..d1d542a 100644
> --- a/drivers/pci/intel-iommu.c
> +++ b/drivers/pci/intel-iommu.c
> @@ -563,32 +563,34 @@ 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_drhd_unit *dmaru = NULL;
> +	struct pci_dev *pdev;
> +	struct intel_iommu *found = NULL;
>  
> -	for_each_drhd_unit(drhd) {
> -		if (drhd->ignored)
> +	pdev = pci_get_domain_bus_and_slot(segment, bus, devfn);
> +
> +	for_each_drhd_unit(dmaru) {
> +		if (dmaru->ignored)
>  			continue;
> -		if (segment != drhd->segment)
> +		if (segment != dmaru->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->subordinate >= bus)
> -				return drhd->iommu;
> +		if (dmaru->include_all) {
> +			found = dmaru->iommu;
> +			break;
> +		}
> +
> +		if (dmar_match_scope(dmaru->scopes, dmaru->scopes_cnt,
> +				     pdev, dmaru->segment)) {
> +			found = dmaru->iommu;
> +			break;
>  		}
>  
> -		if (drhd->include_all)
> -			return drhd->iommu;
>  	}
>  
> -	return NULL;
> +	pci_dev_put(pdev);
> +
> +	return found;
>  }
>  
>  static void domain_flush_cache(struct dmar_domain *domain,
> @@ -2227,7 +2229,7 @@ static int __init init_dmars(int force_on)
>  	struct dmar_rmrr_unit *rmrr;
>  	struct pci_dev *pdev;
>  	struct intel_iommu *iommu;
> -	int i, ret;
> +	int ret;
>  
>  	/*
>  	 * for each drhd
> @@ -2376,18 +2378,22 @@ static int __init init_dmars(int force_on)
>  	 */
>  	printk(KERN_INFO "IOMMU: Setting RMRR:\n");
>  	for_each_rmrr_units(rmrr) {
> -		for (i = 0; i < rmrr->devices_cnt; i++) {
> -			pdev = rmrr->devices[i];
> -			/*
> -			 * some BIOS lists non-exist devices in DMAR
> -			 * table.
> -			 */
> +		struct acpi_dmar_reserved_memory *rmrrh;
> +		int i;
> +
> +		rmrrh = container_of(rmrr->hdr,
> +				     struct acpi_dmar_reserved_memory, header);
> +
> +		for (i = 0; i < rmrr->scopes_cnt; i++) {
> +			pdev = dmar_get_scope_dev(rmrr->scopes[i],
> +						  rmrrh->segment);
>  			if (!pdev)
>  				continue;
> -			ret = iommu_prepare_rmrr_dev(rmrr, pdev);
> -			if (ret)
> +
> +			if (iommu_prepare_rmrr_dev(rmrr, pdev))
>  				printk(KERN_ERR
>  				       "IOMMU: mapping reserved region failed\n");
> +			pci_dev_put(pdev);
>  		}
>  	}
>  
> @@ -3072,15 +3078,21 @@ 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 pci_dev *pdev;
>  
>  	for_each_drhd_unit(drhd) {
>  		if (!drhd->include_all) {
>  			int i;
> -			for (i = 0; i < drhd->devices_cnt; i++)
> -				if (drhd->devices[i] != NULL)
> +			for (i = 0; i < drhd->scopes_cnt; i++) {
> +				pdev = dmar_get_scope_dev(drhd->scopes[i],
> +							  drhd->segment);
> +				if (pdev) {
> +					pci_dev_put(pdev);
>  					break;
> +				}
> +			}
>  			/* ignore DMAR unit if no pci devices exist */
> -			if (i == drhd->devices_cnt)
> +			if (i == drhd->scopes_cnt)
>  				drhd->ignored = 1;
>  		}
>  	}
> @@ -3093,20 +3105,28 @@ static void __init init_no_remapping_devices(void)
>  		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]))
> +		for (i = 0; i < drhd->scopes_cnt; i++) {
> +			pdev = dmar_get_scope_dev(drhd->scopes[i],
> +						  drhd->segment);
> +			if (pdev && !IS_GFX_DEVICE(pdev)) {
> +				pci_dev_put(pdev);
>  				break;
> +			}
> +			pci_dev_put(pdev);
> +		}
>  
> -		if (i < drhd->devices_cnt)
> +		if (i < drhd->scopes_cnt)
>  			continue;
>  
>  		/* bypass IOMMU if it is just for gfx devices */
>  		drhd->ignored = 1;
> -		for (i = 0; i < drhd->devices_cnt; i++) {
> -			if (!drhd->devices[i])
> +		for (i = 0; i < drhd->scopes_cnt; i++) {
> +			pdev = dmar_get_scope_dev(drhd->scopes[i],
> +						  drhd->segment);
> +			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);
>  		}
>  	}
>  }
> diff --git a/include/linux/dmar.h b/include/linux/dmar.h
> index 7b776d7..cf071f9 100644
> --- a/include/linux/dmar.h
> +++ b/include/linux/dmar.h
> @@ -32,8 +32,8 @@ 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	acpi_dmar_device_scope **scopes; /* target scope array	*/
> +	int	scopes_cnt;		/* target scope count	*/
>  	u16	segment;		/* PCI domain		*/
>  	u8	ignored:1; 		/* ignore drhd		*/
>  	u8	include_all:1;
> @@ -55,6 +55,9 @@ extern struct list_head dmar_drhd_units;
>  
>  extern int dmar_table_init(void);
>  extern int dmar_dev_scope_init(void);
> +extern int dmar_match_scope(struct acpi_dmar_device_scope **, int,
> +			    struct pci_dev *, u16);
> +extern struct pci_dev *dmar_get_scope_dev(struct acpi_dmar_device_scope *, u16);
>  
>  /* Intel IOMMU detection */
>  extern int detect_intel_iommu(void);
> @@ -72,6 +75,20 @@ static inline int dmar_table_init(void)
>  {
>  	return -ENODEV;
>  }
> +
> +static inline int dmar_match_scope(struct acpi_dmar_device_scope **scopes,
> +				   int cnt, struct pci_dev *dev, u16 segment)
> +{
> +	return 0;
> +}
> +
> +static inline struct pci_dev *dmar_get_scope_dev(
> +					struct acpi_dmar_device_scope *scope,
> +					u16 segment)
> +{
> +	return NULL;
> +}
> +
>  static inline int enable_drhd_fault_handling(void)
>  {
>  	return -1;
> @@ -212,8 +229,8 @@ 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	acpi_dmar_device_scope **scopes; /* target scope array */
> +	int	scopes_cnt;		/* target scope count */
>  };
>  
>  #define for_each_rmrr_units(rmrr) \
> @@ -222,8 +239,8 @@ 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 */
> +	struct acpi_dmar_device_scope **scopes;	/* target scope array */
> +	int scopes_cnt;		/* target scope count */
>  	u8 include_all:1;		/* include all ports */
>  };
>  
> 

Re: [PATCH] pci, dmar: Update dmar units devices list during hotplug

[Alex Williamson]

On Tue, 2011-05-24 at 13:24 -0700, Yinghai Lu wrote:
> On 05/24/2011 01:07 PM, Alex Williamson wrote:
> > On Tue, 2011-05-24 at 12:34 -0700, Yinghai Lu wrote:
> >> On 05/24/2011 10:42 AM, Alex Williamson wrote:
> >>> On Tue, 2011-05-24 at 11:58 +0100, David Woodhouse wrote:
> >>>> On Thu, 2011-05-19 at 16:15 -0600, Alex Williamson wrote:
> >>>>> I think I'd vote for saving some kind of representation of the bus
> >>>>> hierarchy, we probably don't need to list every possible individual
> >>>>> device.  Leaving a broken pointer around to be matched up and restored
> >>>>> later just seems like a continuation of an idea that was bad to begin
> >>>>> with.  Thanks, 
> >>>>
> >>>> I agree. We should just process the original ATSR information in
> >>>> dmar_find_matched_drhd_unit(), rather than comparing with a list of
> >>>> possibly stale pointers.
> >>>>
> >>>> I don't quite understand why the list of PCI devices was *ever* done
> >>>> like that.
> >>>
> >>> Yinghai,
> >>>
> >>> I thought I might be running into something similar so spent some time
> >>> taking a different slant coding up the bug you found.  Turns out I
> >>> should have tested your patch first because I wasn't hitting that bug at
> >>> all.  The patch below is a work-in-progress that I think fixes the bug
> >>> by providing a quick means of re-parsing the scope as needed to match
> >>> current struct pci_devs.  It needs testing and cleanup, but feel free to
> >>> run with it (or ignore).  Just figured its better to post than waste the
> >>> code if you end up doing something similar.  Thanks,
> >>>
> >>> Alex
> >>>
> >>
> >> it does not apply to current linus tree cleanly.
> > 
> > Sorry, for some reason I started hacking on this against a rhel kernel.
> > Here's the compile tested-only forward port to 2.6.39 (plus the
> > domain_exit flush patch).  Thanks,
> 
> can not find anywhere to call flush_unmaps_timeout(0).
> 
> so what do you mean flush patch?

Take v2.6.39, add
http://git.infradead.org/iommu-2.6.git/commitdiff/7b668357810ecb5fdda4418689d50f5d95aea6a8
and the patch below should apply cleanly.  IIRC, it doesn't touch
anything near domain_exit, so it should apply with some fuzz to stock
2.6.39 too.  Thanks,

Alex

Re: [PATCH] pci, dmar: Update dmar units devices list during hotplug

[Yinghai Lu]

On 05/24/2011 01:07 PM, Alex Williamson wrote:
> On Tue, 2011-05-24 at 12:34 -0700, Yinghai Lu wrote:
>> On 05/24/2011 10:42 AM, Alex Williamson wrote:
>>> On Tue, 2011-05-24 at 11:58 +0100, David Woodhouse wrote:
>>>> On Thu, 2011-05-19 at 16:15 -0600, Alex Williamson wrote:
>>>>> I think I'd vote for saving some kind of representation of the bus
>>>>> hierarchy, we probably don't need to list every possible individual
>>>>> device.  Leaving a broken pointer around to be matched up and restored
>>>>> later just seems like a continuation of an idea that was bad to begin
>>>>> with.  Thanks, 
>>>>
>>>> I agree. We should just process the original ATSR information in
>>>> dmar_find_matched_drhd_unit(), rather than comparing with a list of
>>>> possibly stale pointers.
>>>>
>>>> I don't quite understand why the list of PCI devices was *ever* done
>>>> like that.
>>>
>>> Yinghai,
>>>
>>> I thought I might be running into something similar so spent some time
>>> taking a different slant coding up the bug you found.  Turns out I
>>> should have tested your patch first because I wasn't hitting that bug at
>>> all.  The patch below is a work-in-progress that I think fixes the bug
>>> by providing a quick means of re-parsing the scope as needed to match
>>> current struct pci_devs.  It needs testing and cleanup, but feel free to
>>> run with it (or ignore).  Just figured its better to post than waste the
>>> code if you end up doing something similar.  Thanks,
>>>
>>> Alex
>>>
>>
>> it does not apply to current linus tree cleanly.
> 
> Sorry, for some reason I started hacking on this against a rhel kernel.
> Here's the compile tested-only forward port to 2.6.39 (plus the
> domain_exit flush patch).  Thanks,
> 
> Alex
> 
> 
> Not for commit
> 
> Signed-off-by: Alex Williamson <alex.williamson@redhat.com>
> ---
> 
>  drivers/pci/dmar.c        |  162 ++++++++++++++++++++++++---------------------
>  drivers/pci/intel-iommu.c |   94 ++++++++++++++++----------
>  include/linux/dmar.h      |   29 ++++++--
>  3 files changed, 166 insertions(+), 119 deletions(-)
> 
> 
> diff --git a/drivers/pci/dmar.c b/drivers/pci/dmar.c
> index 12e02bf..47e4f09 100644
> --- a/drivers/pci/dmar.c
> +++ b/drivers/pci/dmar.c
> @@ -61,8 +61,8 @@ static void __init dmar_register_drhd_unit(struct dmar_drhd_unit *drhd)
>  		list_add(&drhd->list, &dmar_drhd_units);
>  }
>  
> -static int __init dmar_parse_one_dev_scope(struct acpi_dmar_device_scope *scope,
> -					   struct pci_dev **dev, u16 segment)
> +struct pci_dev *dmar_get_scope_dev(struct acpi_dmar_device_scope *scope,
> +				   u16 segment)
>  {
>  	struct pci_bus *bus;
>  	struct pci_dev *pdev = NULL;
> @@ -74,7 +74,7 @@ static int __init dmar_parse_one_dev_scope(struct acpi_dmar_device_scope *scope,
>  	count = (scope->length - sizeof(struct acpi_dmar_device_scope))
>  		/ sizeof(struct acpi_dmar_pci_path);
>  
> -	while (count) {
> +	for (; count; path++, count--, bus = pdev->subordinate) {
>  		if (pdev)
>  			pci_dev_put(pdev);
>  		/*
> @@ -82,53 +82,77 @@ static int __init dmar_parse_one_dev_scope(struct acpi_dmar_device_scope *scope,
>  		 * ignore it
>  		 */
>  		if (!bus) {
> -			printk(KERN_WARNING
> -			PREFIX "Device scope bus [%d] not found\n",
> -			scope->bus);
> -			break;
> +			printk(KERN_WARNING PREFIX
> +			       "Device scope bus [%d] not found\n", scope->bus);
> +			return NULL;
>  		}
>  		pdev = pci_get_slot(bus, PCI_DEVFN(path->dev, path->fn));
>  		if (!pdev) {
>  			printk(KERN_WARNING PREFIX
> -			"Device scope device [%04x:%02x:%02x.%02x] not found\n",
> -				segment, bus->number, path->dev, path->fn);
> -			break;
> +			       "Device scope device [%04x:%02x:%02x.%02x] not found\n",
> +			       segment, bus->number, path->dev, path->fn);
> +			return NULL;
>  		}
> -		path ++;
> -		count --;
> -		bus = pdev->subordinate;
>  	}
> -	if (!pdev) {
> -		printk(KERN_WARNING PREFIX
> -		"Device scope device [%04x:%02x:%02x.%02x] not found\n",
> -		segment, scope->bus, path->dev, path->fn);
> -		*dev = NULL;
> +
> +	return pdev;
> +}
> +
> +static int dmar_match_scope_one(struct acpi_dmar_device_scope *scope,
> +				struct pci_dev *dev, u16 segment)
> +{
> +	struct pci_dev *pdev;
> +	int ret = 0;
> +
> +	if (segment != pci_domain_nr(dev->bus))
> +		return 0;
> +
> +	pdev = dmar_get_scope_dev(scope, segment);
> +	if (!pdev)
>  		return 0;
> +
> +	if (scope->entry_type == ACPI_DMAR_SCOPE_TYPE_ENDPOINT) {
> +		if (dev == pdev)
> +			ret = 1;
> +	} else {
> +		while (dev) {
> +			if (dev == pdev) {
> +				ret = 1;
> +				break;
> +			}
> +			dev = dev->bus->self;
> +		}
>  	}
> -	if ((scope->entry_type == ACPI_DMAR_SCOPE_TYPE_ENDPOINT && \
> -			pdev->subordinate) || (scope->entry_type == \
> -			ACPI_DMAR_SCOPE_TYPE_BRIDGE && !pdev->subordinate)) {
> -		pci_dev_put(pdev);
> -		printk(KERN_WARNING PREFIX
> -			"Device scope type does not match for %s\n",
> -			 pci_name(pdev));
> -		return -EINVAL;
> +
> +	pci_dev_put(pdev);
> +
> +	return ret;
> +}
> +
> +int dmar_match_scope(struct acpi_dmar_device_scope **scopes, int cnt,
> +		     struct pci_dev *dev, u16 segment)
> +{
> +	int i;
> +
> +	for (i = 0; i < cnt; i++) {
> +		if (dmar_match_scope_one(scopes[i], dev, segment))
> +			return 1;
>  	}
> -	*dev = pdev;
>  	return 0;
>  }
>  
>  static int __init dmar_parse_dev_scope(void *start, void *end, int *cnt,
> -				       struct pci_dev ***devices, u16 segment)
> +				       struct acpi_dmar_device_scope ***scopes)
>  {
>  	struct acpi_dmar_device_scope *scope;
> -	void * tmp = start;
> -	int index;
> -	int ret;
> +	void *tmp = start;
> +	int index = 0;
>  
>  	*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)++;
> @@ -138,27 +162,23 @@ static int __init dmar_parse_dev_scope(void *start, void *end, int *cnt,
>  		}
>  		start += scope->length;
>  	}
> +
>  	if (*cnt == 0)
>  		return 0;
>  
> -	*devices = kcalloc(*cnt, sizeof(struct pci_dev *), GFP_KERNEL);
> -	if (!*devices)
> +	*scopes = kcalloc(*cnt, sizeof(struct acpi_dmar_device_scope *),
> +                          GFP_KERNEL);
> +	if (!*scopes)
>  		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);
> -				return ret;
> -			}
> -			index ++;
> -		}
> +		    scope->entry_type == ACPI_DMAR_SCOPE_TYPE_BRIDGE)
> +			(*scopes)[index++] = scope;
> +
>  		start += scope->length;
>  	}
>  
> @@ -207,9 +227,8 @@ static int __init dmar_parse_dev(struct dmar_drhd_unit *dmaru)
>  		return 0;
>  
>  	ret = dmar_parse_dev_scope((void *)(drhd + 1),
> -				((void *)drhd) + drhd->header.length,
> -				&dmaru->devices_cnt, &dmaru->devices,
> -				drhd->segment);
> +				   ((void *)drhd) + drhd->header.length,
> +				   &dmaru->scopes_cnt, &dmaru->scopes);
>  	if (ret) {
>  		list_del(&dmaru->list);
>  		kfree(dmaru);
> @@ -253,10 +272,10 @@ 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);
> +				   ((void *)rmrr) + rmrr->header.length,
> +				   &rmrru->scopes_cnt, &rmrru->scopes);
>  
> -	if (ret || (rmrru->devices_cnt == 0)) {
> +	if (ret || (rmrru->scopes_cnt == 0)) {
>  		list_del(&rmrru->list);
>  		kfree(rmrru);
>  	}
> @@ -293,10 +312,9 @@ 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) {
> +				  (void *)atsr + atsr->header.length,
> +				  &atsru->scopes_cnt, &atsru->scopes);
> +	if (rc || !atsru->scopes_cnt) {
>  		list_del(&atsru->list);
>  		kfree(atsru);
>  	}
> @@ -310,6 +328,7 @@ int dmar_find_matched_atsr_unit(struct pci_dev *dev)
>  	struct pci_bus *bus;
>  	struct acpi_dmar_atsr *atsr;
>  	struct dmar_atsr_unit *atsru;
> +	struct pci_dev *pdev;
>  
>  	dev = pci_physfn(dev);
>  
> @@ -330,10 +349,18 @@ found:
>  			return 0;
>  
>  		if (bridge->pcie_type == PCI_EXP_TYPE_ROOT_PORT) {
> -			for (i = 0; i < atsru->devices_cnt; i++)
> -				if (atsru->devices[i] == bridge)
> +			for (i = 0; i < atsru->scopes_cnt; i++) {
> +				pdev = dmar_get_scope_dev(atsru->scopes[i],
> +							  atsr->segment);
> +				if (!pdev)
> +					continue;
> +
> +				if (pdev == bridge) {
> +					pci_dev_put(pdev);
>  					return 1;
> -			break;
> +				}
> +				pci_dev_put(pdev);
> +			}
>  		}
>  	}
>  
> @@ -513,23 +540,6 @@ parse_dmar_table(void)
>  	return ret;
>  }
>  
> -static int dmar_pci_device_match(struct pci_dev *devices[], int cnt,
> -			  struct pci_dev *dev)
> -{
> -	int index;
> -
> -	while (dev) {
> -		for (index = 0; index < cnt; index++)
> -			if (dev == devices[index])
> -				return 1;
> -
> -		/* Check our parent */
> -		dev = dev->bus->self;
> -	}
> -
> -	return 0;
> -}
> -
>  struct dmar_drhd_unit *
>  dmar_find_matched_drhd_unit(struct pci_dev *dev)
>  {
> @@ -544,11 +554,11 @@ dmar_find_matched_drhd_unit(struct pci_dev *dev)
>  				    header);
>  
>  		if (dmaru->include_all &&
> -		    drhd->segment == pci_domain_nr(dev->bus))
> +		    dmaru->segment == pci_domain_nr(dev->bus))
>  			return dmaru;
>  
> -		if (dmar_pci_device_match(dmaru->devices,
> -					  dmaru->devices_cnt, dev))
> +		if (dmar_match_scope(dmaru->scopes, dmaru->scopes_cnt,
> +				     dev, dmaru->segment))
>  			return dmaru;
>  	}
>  
> diff --git a/drivers/pci/intel-iommu.c b/drivers/pci/intel-iommu.c
> index b04f84e..d1d542a 100644
> --- a/drivers/pci/intel-iommu.c
> +++ b/drivers/pci/intel-iommu.c
> @@ -563,32 +563,34 @@ 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_drhd_unit *dmaru = NULL;
> +	struct pci_dev *pdev;
> +	struct intel_iommu *found = NULL;
>  
> -	for_each_drhd_unit(drhd) {
> -		if (drhd->ignored)
> +	pdev = pci_get_domain_bus_and_slot(segment, bus, devfn);
> +
> +	for_each_drhd_unit(dmaru) {
> +		if (dmaru->ignored)
>  			continue;
> -		if (segment != drhd->segment)
> +		if (segment != dmaru->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->subordinate >= bus)
> -				return drhd->iommu;
> +		if (dmaru->include_all) {
> +			found = dmaru->iommu;
> +			break;
> +		}
> +
> +		if (dmar_match_scope(dmaru->scopes, dmaru->scopes_cnt,
> +				     pdev, dmaru->segment)) {
> +			found = dmaru->iommu;
> +			break;
>  		}
>  
> -		if (drhd->include_all)
> -			return drhd->iommu;
>  	}
>  
> -	return NULL;
> +	pci_dev_put(pdev);
> +
> +	return found;
>  }
>  
>  static void domain_flush_cache(struct dmar_domain *domain,
> @@ -2227,7 +2229,7 @@ static int __init init_dmars(int force_on)
>  	struct dmar_rmrr_unit *rmrr;
>  	struct pci_dev *pdev;
>  	struct intel_iommu *iommu;
> -	int i, ret;
> +	int ret;
>  
>  	/*
>  	 * for each drhd
> @@ -2376,18 +2378,22 @@ static int __init init_dmars(int force_on)
>  	 */
>  	printk(KERN_INFO "IOMMU: Setting RMRR:\n");
>  	for_each_rmrr_units(rmrr) {
> -		for (i = 0; i < rmrr->devices_cnt; i++) {
> -			pdev = rmrr->devices[i];
> -			/*
> -			 * some BIOS lists non-exist devices in DMAR
> -			 * table.
> -			 */
> +		struct acpi_dmar_reserved_memory *rmrrh;
> +		int i;
> +
> +		rmrrh = container_of(rmrr->hdr,
> +				     struct acpi_dmar_reserved_memory, header);
> +
> +		for (i = 0; i < rmrr->scopes_cnt; i++) {
> +			pdev = dmar_get_scope_dev(rmrr->scopes[i],
> +						  rmrrh->segment);
>  			if (!pdev)
>  				continue;
> -			ret = iommu_prepare_rmrr_dev(rmrr, pdev);
> -			if (ret)
> +
> +			if (iommu_prepare_rmrr_dev(rmrr, pdev))
>  				printk(KERN_ERR
>  				       "IOMMU: mapping reserved region failed\n");
> +			pci_dev_put(pdev);
>  		}
>  	}
>  
> @@ -3072,15 +3078,21 @@ 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 pci_dev *pdev;
>  
>  	for_each_drhd_unit(drhd) {
>  		if (!drhd->include_all) {
>  			int i;
> -			for (i = 0; i < drhd->devices_cnt; i++)
> -				if (drhd->devices[i] != NULL)
> +			for (i = 0; i < drhd->scopes_cnt; i++) {
> +				pdev = dmar_get_scope_dev(drhd->scopes[i],
> +							  drhd->segment);
> +				if (pdev) {
> +					pci_dev_put(pdev);
>  					break;
> +				}
> +			}
>  			/* ignore DMAR unit if no pci devices exist */
> -			if (i == drhd->devices_cnt)
> +			if (i == drhd->scopes_cnt)
>  				drhd->ignored = 1;
>  		}
>  	}
> @@ -3093,20 +3105,28 @@ static void __init init_no_remapping_devices(void)
>  		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]))
> +		for (i = 0; i < drhd->scopes_cnt; i++) {
> +			pdev = dmar_get_scope_dev(drhd->scopes[i],
> +						  drhd->segment);
> +			if (pdev && !IS_GFX_DEVICE(pdev)) {
> +				pci_dev_put(pdev);
>  				break;
> +			}
> +			pci_dev_put(pdev);
> +		}
>  
> -		if (i < drhd->devices_cnt)
> +		if (i < drhd->scopes_cnt)
>  			continue;
>  
>  		/* bypass IOMMU if it is just for gfx devices */
>  		drhd->ignored = 1;
> -		for (i = 0; i < drhd->devices_cnt; i++) {
> -			if (!drhd->devices[i])
> +		for (i = 0; i < drhd->scopes_cnt; i++) {
> +			pdev = dmar_get_scope_dev(drhd->scopes[i],
> +						  drhd->segment);
> +			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);
>  		}
>  	}
>  }
> diff --git a/include/linux/dmar.h b/include/linux/dmar.h
> index 7b776d7..cf071f9 100644
> --- a/include/linux/dmar.h
> +++ b/include/linux/dmar.h
> @@ -32,8 +32,8 @@ 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	acpi_dmar_device_scope **scopes; /* target scope array	*/
> +	int	scopes_cnt;		/* target scope count	*/
>  	u16	segment;		/* PCI domain		*/
>  	u8	ignored:1; 		/* ignore drhd		*/
>  	u8	include_all:1;
> @@ -55,6 +55,9 @@ extern struct list_head dmar_drhd_units;
>  
>  extern int dmar_table_init(void);
>  extern int dmar_dev_scope_init(void);
> +extern int dmar_match_scope(struct acpi_dmar_device_scope **, int,
> +			    struct pci_dev *, u16);
> +extern struct pci_dev *dmar_get_scope_dev(struct acpi_dmar_device_scope *, u16);
>  
>  /* Intel IOMMU detection */
>  extern int detect_intel_iommu(void);
> @@ -72,6 +75,20 @@ static inline int dmar_table_init(void)
>  {
>  	return -ENODEV;
>  }
> +
> +static inline int dmar_match_scope(struct acpi_dmar_device_scope **scopes,
> +				   int cnt, struct pci_dev *dev, u16 segment)
> +{
> +	return 0;
> +}
> +
> +static inline struct pci_dev *dmar_get_scope_dev(
> +					struct acpi_dmar_device_scope *scope,
> +					u16 segment)
> +{
> +	return NULL;
> +}
> +
>  static inline int enable_drhd_fault_handling(void)
>  {
>  	return -1;
> @@ -212,8 +229,8 @@ 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	acpi_dmar_device_scope **scopes; /* target scope array */
> +	int	scopes_cnt;		/* target scope count */
>  };
>  
>  #define for_each_rmrr_units(rmrr) \
> @@ -222,8 +239,8 @@ 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 */
> +	struct acpi_dmar_device_scope **scopes;	/* target scope array */
> +	int scopes_cnt;		/* target scope count */
>  	u8 include_all:1;		/* include all ports */
>  };
>  
> 


No, it does not work.

[  592.792864] BUG: unable to handle kernel NULL pointer dereference at 0000000000000010
[  592.793279] IP: [<ffffffff8136271a>] dmar_match_scope+0x27/0xb6
[  592.804488] PGD 0 
[  592.804629] Oops: 0000 [#1] SMP 
[  592.804849] CPU 1 
[  592.804947] Modules linked in:
[  592.824426] 
[  592.824521] Pid: 14498, comm: kworker/u:7 Tainted: G        W   2.6.39-tip-yh-06738-g5d55a15-dirty #1043 Oracle Corporation  Sun Fire X4800 M2 /     
[  592.844637] RIP: 0010:[<ffffffff8136271a>]  [<ffffffff8136271a>] dmar_match_scope+0x27/0xb6
[  592.864410] RSP: 0018:ffff881ffce45a70  EFLAGS: 00010293
[  592.864673] RAX: 0000000000000000 RBX: 0000000000000008 RCX: 0000000000000000
[  592.884403] RDX: 0000000000000000 RSI: 0000000000000008 RDI: ffff88fffeef6000
[  592.904205] RBP: ffff881ffce45ab0 R08: 0000000000000000 R09: 0000000000000000
[  592.904535] R10: 0000000000000000 R11: ffff88dffedda320 R12: 0000000000000000
[  592.924419] R13: 0000000000000000 R14: ffff88fffeef6000 R15: 0000000000000000
[  592.924748] FS:  0000000000000000(0000) GS:ffff88207d800000(0000) knlGS:0000000000000000
[  592.944447] CS:  0010 DS: 0000 ES: 0000 CR0: 000000008005003b
[  592.964196] CR2: 0000000000000010 CR3: 00000000023cb000 CR4: 00000000000006e0
[  592.964541] DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
[  592.984308] DR3: 0000000000000000 DR6: 00000000ffff0ff0 DR7: 0000000000000400
[  593.004091] Process kworker/u:7 (pid: 14498, threadinfo ffff881ffce44000, task ffff881ffce3c560)
[  593.004492] Stack:
[  593.024051]  ffff881ffce45a80 ffffffff8134bf6a ffff881ffce45ab0 ffff88407d002640
[  593.024479]  0000000000000000 0000000000000000 0000000000000000 0000000000000000
[  593.044272]  ffff881ffce45ae0 ffffffff81363e4a ffff881ffce45b00 ffff88dffe1c1d00
[  593.064027] Call Trace:
[  593.064156]  [<ffffffff8134bf6a>] ? pci_get_device+0x16/0x18
[  593.064441]  [<ffffffff81363e4a>] device_to_iommu+0x4c/0x77
[  593.084054]  [<ffffffff81364163>] domain_remove_one_dev_info+0x39/0x1fc
[  593.084361]  [<ffffffff8136637f>] device_notifier+0x52/0x78
[  593.104093]  [<ffffffff81c24cc3>] notifier_call_chain+0x68/0x9f
[  593.123814]  [<ffffffff81193710>] ? sysfs_schedule_callback+0x1df/0x1df
[  593.124125]  [<ffffffff8109f209>] __blocking_notifier_call_chain+0x4c/0x61
[  593.143899]  [<ffffffff8109f232>] blocking_notifier_call_chain+0x14/0x16
[  593.144207]  [<ffffffff81414909>] __device_release_driver+0xcd/0xd2
[  593.163934]  [<ffffffff81414933>] device_release_driver+0x25/0x32
[  593.164222]  [<ffffffff814144bd>] bus_remove_device+0x8e/0x9f
[  593.183934]  [<ffffffff8141255e>] device_del+0x130/0x17f
[  593.184186]  [<ffffffff814125c3>] device_unregister+0x16/0x23
[  593.203888]  [<ffffffff813473e5>] pci_stop_bus_device+0x61/0x83
[  593.223661]  [<ffffffff8134c88a>] ? remove_callback+0x1f/0x3c
[  593.223932]  [<ffffffff813473b4>] pci_stop_bus_device+0x30/0x83
[  593.243652]  [<ffffffff81347470>] pci_remove_bus_device+0x1a/0xba
[  593.243955]  [<ffffffff8134c896>] remove_callback+0x2b/0x3c
[  593.263631]  [<ffffffff8119372c>] sysfs_schedule_callback_work+0x1c/0x5f
[  593.263962]  [<ffffffff8109433b>] process_one_work+0x231/0x3e6
[  593.283667]  [<ffffffff810942ac>] ? process_one_work+0x1a2/0x3e6
[  593.283958]  [<ffffffff81094829>] worker_thread+0x17c/0x240
[  593.303651]  [<ffffffff810add32>] ? trace_hardirqs_on+0xd/0xf
[  593.303927]  [<ffffffff810946ad>] ? manage_workers+0xab/0xab
[  593.323617]  [<ffffffff81099ea5>] kthread+0xa0/0xa8
[  593.323845]  [<ffffffff810adbcc>] ? trace_hardirqs_on_caller+0x1f/0x178
[  593.343620]  [<ffffffff81c29614>] kernel_thread_helper+0x4/0x10
[  593.343888]  [<ffffffff81c218c4>] ? _raw_spin_unlock_irq+0x30/0x36
[  593.363642]  [<ffffffff810add32>] ? trace_hardirqs_on+0xd/0xf
[  593.383357]  [<ffffffff81c21bc0>] ? retint_restore_args+0xe/0xe
[  593.383624]  [<ffffffff81099e05>] ? __init_kthread_worker+0x5b/0x5b
[  593.403441]  [<ffffffff81c29610>] ? gs_change+0xb/0xb
[  593.403667] Code: 41 5f c9 c3 55 48 89 e5 41 57 41 56 41 55 41 54 53 48 83 ec 18 66 66 66 66 90 45 31 ed 89 f3 49 89 d4 49 89 fe 44 0f b7 f9 eb 72 
[  593.424146]  8b 44 24 10 49 8b 16 48 8b 80 88 00 00 00 44 3b 38 75 57 48 
[  593.443684] RIP  [<ffffffff8136271a>] dmar_match_scope+0x27/0xb6
[  593.443968]  RSP <ffff881ffce45a70>
[  593.463319] CR2: 0000000000000010
[  593.463510] ---[ end trace 75ddbb3d94414ea9 ]---
[  593.465841] BUG: unable to handle kernel 

Re: [PATCH] pci, dmar: Update dmar units devices list during hotplug

[Alex Williamson]

On Tue, 2011-05-24 at 14:45 -0700, Yinghai Lu wrote:
> No, it does not work.

I didn't say this wasn't without some effort, just thought it might give
you a jump start.

Alex

> [  592.792864] BUG: unable to handle kernel NULL pointer dereference at 0000000000000010
> [  592.793279] IP: [<ffffffff8136271a>] dmar_match_scope+0x27/0xb6
> [  592.804488] PGD 0 
> [  592.804629] Oops: 0000 [#1] SMP 
> [  592.804849] CPU 1 
> [  592.804947] Modules linked in:
> [  592.824426] 
> [  592.824521] Pid: 14498, comm: kworker/u:7 Tainted: G        W   2.6.39-tip-yh-06738-g5d55a15-dirty #1043 Oracle Corporation  Sun Fire X4800 M2 /     
> [  592.844637] RIP: 0010:[<ffffffff8136271a>]  [<ffffffff8136271a>] dmar_match_scope+0x27/0xb6
> [  592.864410] RSP: 0018:ffff881ffce45a70  EFLAGS: 00010293
> [  592.864673] RAX: 0000000000000000 RBX: 0000000000000008 RCX: 0000000000000000
> [  592.884403] RDX: 0000000000000000 RSI: 0000000000000008 RDI: ffff88fffeef6000
> [  592.904205] RBP: ffff881ffce45ab0 R08: 0000000000000000 R09: 0000000000000000
> [  592.904535] R10: 0000000000000000 R11: ffff88dffedda320 R12: 0000000000000000
> [  592.924419] R13: 0000000000000000 R14: ffff88fffeef6000 R15: 0000000000000000
> [  592.924748] FS:  0000000000000000(0000) GS:ffff88207d800000(0000) knlGS:0000000000000000
> [  592.944447] CS:  0010 DS: 0000 ES: 0000 CR0: 000000008005003b
> [  592.964196] CR2: 0000000000000010 CR3: 00000000023cb000 CR4: 00000000000006e0
> [  592.964541] DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
> [  592.984308] DR3: 0000000000000000 DR6: 00000000ffff0ff0 DR7: 0000000000000400
> [  593.004091] Process kworker/u:7 (pid: 14498, threadinfo ffff881ffce44000, task ffff881ffce3c560)
> [  593.004492] Stack:
> [  593.024051]  ffff881ffce45a80 ffffffff8134bf6a ffff881ffce45ab0 ffff88407d002640
> [  593.024479]  0000000000000000 0000000000000000 0000000000000000 0000000000000000
> [  593.044272]  ffff881ffce45ae0 ffffffff81363e4a ffff881ffce45b00 ffff88dffe1c1d00
> [  593.064027] Call Trace:
> [  593.064156]  [<ffffffff8134bf6a>] ? pci_get_device+0x16/0x18
> [  593.064441]  [<ffffffff81363e4a>] device_to_iommu+0x4c/0x77
> [  593.084054]  [<ffffffff81364163>] domain_remove_one_dev_info+0x39/0x1fc
> [  593.084361]  [<ffffffff8136637f>] device_notifier+0x52/0x78
> [  593.104093]  [<ffffffff81c24cc3>] notifier_call_chain+0x68/0x9f
> [  593.123814]  [<ffffffff81193710>] ? sysfs_schedule_callback+0x1df/0x1df
> [  593.124125]  [<ffffffff8109f209>] __blocking_notifier_call_chain+0x4c/0x61
> [  593.143899]  [<ffffffff8109f232>] blocking_notifier_call_chain+0x14/0x16
> [  593.144207]  [<ffffffff81414909>] __device_release_driver+0xcd/0xd2
> [  593.163934]  [<ffffffff81414933>] device_release_driver+0x25/0x32
> [  593.164222]  [<ffffffff814144bd>] bus_remove_device+0x8e/0x9f
> [  593.183934]  [<ffffffff8141255e>] device_del+0x130/0x17f
> [  593.184186]  [<ffffffff814125c3>] device_unregister+0x16/0x23
> [  593.203888]  [<ffffffff813473e5>] pci_stop_bus_device+0x61/0x83
> [  593.223661]  [<ffffffff8134c88a>] ? remove_callback+0x1f/0x3c
> [  593.223932]  [<ffffffff813473b4>] pci_stop_bus_device+0x30/0x83
> [  593.243652]  [<ffffffff81347470>] pci_remove_bus_device+0x1a/0xba
> [  593.243955]  [<ffffffff8134c896>] remove_callback+0x2b/0x3c
> [  593.263631]  [<ffffffff8119372c>] sysfs_schedule_callback_work+0x1c/0x5f
> [  593.263962]  [<ffffffff8109433b>] process_one_work+0x231/0x3e6
> [  593.283667]  [<ffffffff810942ac>] ? process_one_work+0x1a2/0x3e6
> [  593.283958]  [<ffffffff81094829>] worker_thread+0x17c/0x240
> [  593.303651]  [<ffffffff810add32>] ? trace_hardirqs_on+0xd/0xf
> [  593.303927]  [<ffffffff810946ad>] ? manage_workers+0xab/0xab
> [  593.323617]  [<ffffffff81099ea5>] kthread+0xa0/0xa8
> [  593.323845]  [<ffffffff810adbcc>] ? trace_hardirqs_on_caller+0x1f/0x178
> [  593.343620]  [<ffffffff81c29614>] kernel_thread_helper+0x4/0x10
> [  593.343888]  [<ffffffff81c218c4>] ? _raw_spin_unlock_irq+0x30/0x36
> [  593.363642]  [<ffffffff810add32>] ? trace_hardirqs_on+0xd/0xf
> [  593.383357]  [<ffffffff81c21bc0>] ? retint_restore_args+0xe/0xe
> [  593.383624]  [<ffffffff81099e05>] ? __init_kthread_worker+0x5b/0x5b
> [  593.403441]  [<ffffffff81c29610>] ? gs_change+0xb/0xb
> [  593.403667] Code: 41 5f c9 c3 55 48 89 e5 41 57 41 56 41 55 41 54 53 48 83 ec 18 66 66 66 66 90 45 31 ed 89 f3 49 89 d4 49 89 fe 44 0f b7 f9 eb 72 
> [  593.424146]  8b 44 24 10 49 8b 16 48 8b 80 88 00 00 00 44 3b 38 75 57 48 
> [  593.443684] RIP  [<ffffffff8136271a>] dmar_match_scope+0x27/0xb6
> [  593.443968]  RSP <ffff881ffce45a70>
> [  593.463319] CR2: 0000000000000010
> [  593.463510] ---[ end trace 75ddbb3d94414ea9 ]---
> [  593.465841] BUG: unable to handle kernel 
> 

Re: [PATCH] pci, dmar: Update dmar units devices list during hotplug

[Yinghai Lu]

On 05/24/2011 03:38 PM, Alex Williamson wrote:
> On Tue, 2011-05-24 at 14:45 -0700, Yinghai Lu wrote:
>> No, it does not work.
> 
> I didn't say this wasn't without some effort, just thought it might give
> you a jump start.

ok, let me debug it tonight.

looks like that pdev is not freed, but already get removed from the device tree.

may need to pass pci_dev pointer directly.

> 
> Alex
> 
>> [  592.792864] BUG: unable to handle kernel NULL pointer dereference at 0000000000000010
>> [  592.793279] IP: [<ffffffff8136271a>] dmar_match_scope+0x27/0xb6
>> [  592.804488] PGD 0 
>> [  592.804629] Oops: 0000 [#1] SMP 
>> [  592.804849] CPU 1 
>> [  592.804947] Modules linked in:
>> [  592.824426] 
>> [  592.824521] Pid: 14498, comm: kworker/u:7 Tainted: G        W   2.6.39-tip-yh-06738-g5d55a15-dirty #1043 Oracle Corporation  Sun Fire X4800 M2 /     
>> [  592.844637] RIP: 0010:[<ffffffff8136271a>]  [<ffffffff8136271a>] dmar_match_scope+0x27/0xb6
>> [  592.864410] RSP: 0018:ffff881ffce45a70  EFLAGS: 00010293
>> [  592.864673] RAX: 0000000000000000 RBX: 0000000000000008 RCX: 0000000000000000
>> [  592.884403] RDX: 0000000000000000 RSI: 0000000000000008 RDI: ffff88fffeef6000
>> [  592.904205] RBP: ffff881ffce45ab0 R08: 0000000000000000 R09: 0000000000000000
>> [  592.904535] R10: 0000000000000000 R11: ffff88dffedda320 R12: 0000000000000000
>> [  592.924419] R13: 0000000000000000 R14: ffff88fffeef6000 R15: 0000000000000000
>> [  592.924748] FS:  0000000000000000(0000) GS:ffff88207d800000(0000) knlGS:0000000000000000
>> [  592.944447] CS:  0010 DS: 0000 ES: 0000 CR0: 000000008005003b
>> [  592.964196] CR2: 0000000000000010 CR3: 00000000023cb000 CR4: 00000000000006e0
>> [  592.964541] DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
>> [  592.984308] DR3: 0000000000000000 DR6: 00000000ffff0ff0 DR7: 0000000000000400
>> [  593.004091] Process kworker/u:7 (pid: 14498, threadinfo ffff881ffce44000, task ffff881ffce3c560)
>> [  593.004492] Stack:
>> [  593.024051]  ffff881ffce45a80 ffffffff8134bf6a ffff881ffce45ab0 ffff88407d002640
>> [  593.024479]  0000000000000000 0000000000000000 0000000000000000 0000000000000000
>> [  593.044272]  ffff881ffce45ae0 ffffffff81363e4a ffff881ffce45b00 ffff88dffe1c1d00
>> [  593.064027] Call Trace:
>> [  593.064156]  [<ffffffff8134bf6a>] ? pci_get_device+0x16/0x18
>> [  593.064441]  [<ffffffff81363e4a>] device_to_iommu+0x4c/0x77
>> [  593.084054]  [<ffffffff81364163>] domain_remove_one_dev_info+0x39/0x1fc
>> [  593.084361]  [<ffffffff8136637f>] device_notifier+0x52/0x78
>> [  593.104093]  [<ffffffff81c24cc3>] notifier_call_chain+0x68/0x9f
>> [  593.123814]  [<ffffffff81193710>] ? sysfs_schedule_callback+0x1df/0x1df
>> [  593.124125]  [<ffffffff8109f209>] __blocking_notifier_call_chain+0x4c/0x61
>> [  593.143899]  [<ffffffff8109f232>] blocking_notifier_call_chain+0x14/0x16
>> [  593.144207]  [<ffffffff81414909>] __device_release_driver+0xcd/0xd2
>> [  593.163934]  [<ffffffff81414933>] device_release_driver+0x25/0x32
>> [  593.164222]  [<ffffffff814144bd>] bus_remove_device+0x8e/0x9f
>> [  593.183934]  [<ffffffff8141255e>] device_del+0x130/0x17f
>> [  593.184186]  [<ffffffff814125c3>] device_unregister+0x16/0x23
>> [  593.203888]  [<ffffffff813473e5>] pci_stop_bus_device+0x61/0x83
>> [  593.223661]  [<ffffffff8134c88a>] ? remove_callback+0x1f/0x3c
>> [  593.223932]  [<ffffffff813473b4>] pci_stop_bus_device+0x30/0x83
>> [  593.243652]  [<ffffffff81347470>] pci_remove_bus_device+0x1a/0xba
>> [  593.243955]  [<ffffffff8134c896>] remove_callback+0x2b/0x3c
>> [  593.263631]  [<ffffffff8119372c>] sysfs_schedule_callback_work+0x1c/0x5f
>> [  593.263962]  [<ffffffff8109433b>] process_one_work+0x231/0x3e6
>> [  593.283667]  [<ffffffff810942ac>] ? process_one_work+0x1a2/0x3e6
>> [  593.283958]  [<ffffffff81094829>] worker_thread+0x17c/0x240
>> [  593.303651]  [<ffffffff810add32>] ? trace_hardirqs_on+0xd/0xf
>> [  593.303927]  [<ffffffff810946ad>] ? manage_workers+0xab/0xab
>> [  593.323617]  [<ffffffff81099ea5>] kthread+0xa0/0xa8
>> [  593.323845]  [<ffffffff810adbcc>] ? trace_hardirqs_on_caller+0x1f/0x178
>> [  593.343620]  [<ffffffff81c29614>] kernel_thread_helper+0x4/0x10
>> [  593.343888]  [<ffffffff81c218c4>] ? _raw_spin_unlock_irq+0x30/0x36
>> [  593.363642]  [<ffffffff810add32>] ? trace_hardirqs_on+0xd/0xf
>> [  593.383357]  [<ffffffff81c21bc0>] ? retint_restore_args+0xe/0xe
>> [  593.383624]  [<ffffffff81099e05>] ? __init_kthread_worker+0x5b/0x5b
>> [  593.403441]  [<ffffffff81c29610>] ? gs_change+0xb/0xb
>> [  593.403667] Code: 41 5f c9 c3 55 48 89 e5 41 57 41 56 41 55 41 54 53 48 83 ec 18 66 66 66 66 90 45 31 ed 89 f3 49 89 d4 49 89 fe 44 0f b7 f9 eb 72 
>> [  593.424146]  8b 44 24 10 49 8b 16 48 8b 80 88 00 00 00 44 3b 38 75 57 48 
>> [  593.443684] RIP  [<ffffffff8136271a>] dmar_match_scope+0x27/0xb6
>> [  593.443968]  RSP <ffff881ffce45a70>
>> [  593.463319] CR2: 0000000000000010
>> [  593.463510] ---[ end trace 75ddbb3d94414ea9 ]---
>> [  593.465841] BUG: unable to handle kernel 
>>
> 
> 

Re: [PATCH] pci, dmar: Update dmar units devices list during hotplug

[Yinghai Lu]

On Tue, May 24, 2011 at 4:02 PM, Yinghai Lu <yinghai@kernel.org> wrote:
> On 05/24/2011 03:38 PM, Alex Williamson wrote:
>> On Tue, 2011-05-24 at 14:45 -0700, Yinghai Lu wrote:
>>> No, it does not work.
>>
>> I didn't say this wasn't without some effort, just thought it might give
>> you a jump start.
>
> ok, let me debug it tonight.
>
> looks like that pdev is not freed, but already get removed from the device tree.
>
> may need to pass pci_dev pointer directly.

keep getting:

[  784.364244] BUG: sleeping function called from invalid context at
kernel/rwsem.c:21
[  784.364253] in_atomic(): 0, irqs_disabled(): 1, pid: 29398, name:
work_for_cpu
[  784.364259] INFO: lockdep is turned off.
[  784.364265] irq event stamp: 0
[  784.364271] hardirqs last  enabled at (0): [<          (null)>]
      (null)
[  784.364282] hardirqs last disabled at (0): [<ffffffff8107ba5e>]
copy_process+0x43b/0xd95
[  784.364305] softirqs last  enabled at (0): [<ffffffff8107ba5e>]
copy_process+0x43b/0xd95
[  784.364318] softirqs last disabled at (0): [<          (null)>]
      (null)
[  784.364332] Pid: 29398, comm: work_for_cpu Not tainted
2.6.39-tip-yh-06791-gb282579-dirty #1047
[  784.364339] Call Trace:
[  784.364375]  [<ffffffff810ad12a>] ? print_irqtrace_events+0xd0/0xd4
[  784.364392]  [<ffffffff81071572>] __might_sleep+0xf2/0xf6
[  784.364410]  [<ffffffff81c20fc0>] down_read+0x26/0x91
[  784.364429]  [<ffffffff8134c089>] pci_find_next_bus+0x45/0x75
[  784.364442]  [<ffffffff8134c0fa>] pci_find_bus+0x41/0x54
[  784.364457]  [<ffffffff8136263f>] dmar_get_scope_dev+0x2f/0xe3
[  784.364474]  [<ffffffff813353ad>] ? random32+0x19/0x1b
[  784.364488]  [<ffffffff8136273d>] dmar_match_scope+0x4a/0xb6
[  784.364502]  [<ffffffff813628d4>] dmar_find_matched_drhd_unit+0x55/0x6f
[  784.364519]  [<ffffffff81367018>] get_domain_for_dev.clone.2+0x103/0x392
[  784.364533]  [<ffffffff81367459>] __get_valid_domain_for_dev+0x14/0x88
[  784.364546]  [<ffffffff813676c5>] __intel_map_single+0x58/0x174
[  784.364559]  [<ffffffff813678eb>] intel_alloc_coherent+0xc7/0xee
[  784.364575]  [<ffffffff811281dc>] pool_alloc_page.clone.0+0xc9/0x140
[  784.364588]  [<ffffffff811282d8>] dma_pool_alloc+0x85/0x131
[  784.364603]  [<ffffffff81134b8c>] ? should_failslab+0x44/0x48
[  784.364618]  [<ffffffff81132d62>] ? kmem_cache_alloc_trace+0x5e/0x123
[  784.364635]  [<ffffffff818429aa>] ehci_qh_alloc+0x59/0xd2
[  784.364649]  [<ffffffff81844451>] ehci_mem_init.clone.1+0x84/0x25c
[  784.364660]  [<ffffffff8184471b>] ehci_init+0xf2/0x245
[  784.364671]  [<ffffffff81844999>] ehci_pci_setup+0x12b/0x564
[  784.364687]  [<ffffffff818323d5>] usb_add_hcd+0x10f/0x318
[  784.364703]  [<ffffffff8183e31e>] usb_hcd_pci_probe+0x1e4/0x312
[  784.364722]  [<ffffffff8134ae25>] local_pci_probe+0x4d/0x96
[  784.364739]  [<ffffffff81093c4b>] ? cwq_dec_nr_in_flight+0x81/0x81
[  784.364754]  [<ffffffff81093c63>] do_work_for_cpu+0x18/0x2b
[  784.364770]  [<ffffffff81093c4b>] ? cwq_dec_nr_in_flight+0x81/0x81
[  784.364787]  [<ffffffff81099ea5>] kthread+0xa0/0xa8
[  784.364801]  [<ffffffff810adbcc>] ? trace_hardirqs_on_caller+0x1f/0x178
[  784.364818]  [<ffffffff81c2a214>] kernel_thread_helper+0x4/0x10
[  784.364832]  [<ffffffff81c224cc>] ? _raw_spin_unlock_irq+0x30/0x36
[  784.364846]  [<ffffffff810add32>] ? trace_hardirqs_on+0xd/0xf
[  784.364861]  [<ffffffff81c227c0>] ? retint_restore_args+0xe/0xe
[  784.364880]  [<ffffffff81099e05>] ? __init_kthread_worker+0x5b/0x5b
[  784.364893]  [<ffffffff81c2a210>] ? gs_change+0xb/0xb
[  784.364902] DMAR: Device scope device [0000:40:00.00] not found
[  784.364910] DMAR: Device scope device [0000:40:01.00] not found
[  784.364931] DMAR: Device scope device [0000:40:03.00] not found
[  784.364948] DMAR: Device scope device [0000:40:05.00] not found
[  784.364961] DMAR: Device scope device [0000:40:07.00] not found
[  784.364978] DMAR: Device scope device [0000:40:09.00] not found
[  784.365019] DMAR: Device scope device [0000:80:00.00] not found
[  784.365034] DMAR: Device scope device [0000:80:01.00] not found
[  784.365053] DMAR: Device scope device [0000:80:03.00] not found
[  784.365075] DMAR: Device scope device [0000:80:05.00] not found
[  784.365094] DMAR: Device scope device [0000:80:07.00] not found
[  784.365116] DMAR: Device scope device [0000:80:09.00] not found
[  784.365166] DMAR: Device scope device [0000:c0:00.00] not found
[  784.365193] DMAR: Device scope device [0000:c0:01.00] not found
[  784.365216] DMAR: Device scope device [0000:c0:03.00] not found
[  784.365243] DMAR: Device scope device [0000:c0:05.00] not found
[  784.365266] DMAR: Device scope device [0000:c0:07.00] not found
[  784.365284] DMAR: Device scope device [0000:c0:09.00] not found

Re: [PATCH] pci, dmar: Update dmar units devices list during hotplug

[Yinghai Lu]

On 05/24/2011 04:02 PM, Yinghai Lu wrote:
> On 05/24/2011 03:38 PM, Alex Williamson wrote:
>> On Tue, 2011-05-24 at 14:45 -0700, Yinghai Lu wrote:
>>> No, it does not work.
>>
>> I didn't say this wasn't without some effort, just thought it might give
>> you a jump start.
> 
> ok, let me debug it tonight.
> 
> looks like that pdev is not freed, but already get removed from the device tree.
> 
> may need to pass pci_dev pointer directly.
> 

keep getting:

[  784.364244] BUG: sleeping function called from invalid context at kernel/rwsem.c:21
[  784.364253] in_atomic(): 0, irqs_disabled(): 1, pid: 29398, name: work_for_cpu
[  784.364259] INFO: lockdep is turned off.
[  784.364265] irq event stamp: 0
[  784.364271] hardirqs last  enabled at (0): [<          (null)>]           (null)
[  784.364282] hardirqs last disabled at (0): [<ffffffff8107ba5e>] copy_process+0x43b/0xd95
[  784.364305] softirqs last  enabled at (0): [<ffffffff8107ba5e>] copy_process+0x43b/0xd95
[  784.364318] softirqs last disabled at (0): [<          (null)>]           (null)
[  784.364332] Pid: 29398, comm: work_for_cpu Not tainted 2.6.39-tip-yh-06791-gb282579-dirty #1047
[  784.364339] Call Trace:
[  784.364375]  [<ffffffff810ad12a>] ? print_irqtrace_events+0xd0/0xd4
[  784.364392]  [<ffffffff81071572>] __might_sleep+0xf2/0xf6
[  784.364410]  [<ffffffff81c20fc0>] down_read+0x26/0x91
[  784.364429]  [<ffffffff8134c089>] pci_find_next_bus+0x45/0x75
[  784.364442]  [<ffffffff8134c0fa>] pci_find_bus+0x41/0x54
[  784.364457]  [<ffffffff8136263f>] dmar_get_scope_dev+0x2f/0xe3
[  784.364474]  [<ffffffff813353ad>] ? random32+0x19/0x1b
[  784.364488]  [<ffffffff8136273d>] dmar_match_scope+0x4a/0xb6
[  784.364502]  [<ffffffff813628d4>] dmar_find_matched_drhd_unit+0x55/0x6f
[  784.364519]  [<ffffffff81367018>] get_domain_for_dev.clone.2+0x103/0x392
[  784.364533]  [<ffffffff81367459>] __get_valid_domain_for_dev+0x14/0x88
[  784.364546]  [<ffffffff813676c5>] __intel_map_single+0x58/0x174
[  784.364559]  [<ffffffff813678eb>] intel_alloc_coherent+0xc7/0xee
[  784.364575]  [<ffffffff811281dc>] pool_alloc_page.clone.0+0xc9/0x140
[  784.364588]  [<ffffffff811282d8>] dma_pool_alloc+0x85/0x131
[  784.364603]  [<ffffffff81134b8c>] ? should_failslab+0x44/0x48
[  784.364618]  [<ffffffff81132d62>] ? kmem_cache_alloc_trace+0x5e/0x123
[  784.364635]  [<ffffffff818429aa>] ehci_qh_alloc+0x59/0xd2
[  784.364649]  [<ffffffff81844451>] ehci_mem_init.clone.1+0x84/0x25c
[  784.364660]  [<ffffffff8184471b>] ehci_init+0xf2/0x245
[  784.364671]  [<ffffffff81844999>] ehci_pci_setup+0x12b/0x564
[  784.364687]  [<ffffffff818323d5>] usb_add_hcd+0x10f/0x318
[  784.364703]  [<ffffffff8183e31e>] usb_hcd_pci_probe+0x1e4/0x312
[  784.364722]  [<ffffffff8134ae25>] local_pci_probe+0x4d/0x96
[  784.364739]  [<ffffffff81093c4b>] ? cwq_dec_nr_in_flight+0x81/0x81
[  784.364754]  [<ffffffff81093c63>] do_work_for_cpu+0x18/0x2b
[  784.364770]  [<ffffffff81093c4b>] ? cwq_dec_nr_in_flight+0x81/0x81
[  784.364787]  [<ffffffff81099ea5>] kthread+0xa0/0xa8
[  784.364801]  [<ffffffff810adbcc>] ? trace_hardirqs_on_caller+0x1f/0x178
[  784.364818]  [<ffffffff81c2a214>] kernel_thread_helper+0x4/0x10
[  784.364832]  [<ffffffff81c224cc>] ? _raw_spin_unlock_irq+0x30/0x36
[  784.364846]  [<ffffffff810add32>] ? trace_hardirqs_on+0xd/0xf
[  784.364861]  [<ffffffff81c227c0>] ? retint_restore_args+0xe/0xe
[  784.364880]  [<ffffffff81099e05>] ? __init_kthread_worker+0x5b/0x5b
[  784.364893]  [<ffffffff81c2a210>] ? gs_change+0xb/0xb
[  784.364902] DMAR: Device scope device [0000:40:00.00] not found
[  784.364910] DMAR: Device scope device [0000:40:01.00] not found
[  784.364931] DMAR: Device scope device [0000:40:03.00] not found
[  784.364948] DMAR: Device scope device [0000:40:05.00] not found
[  784.364961] DMAR: Device scope device [0000:40:07.00] not found
[  784.364978] DMAR: Device scope device [0000:40:09.00] not found
[  784.365019] DMAR: Device scope device [0000:80:00.00] not found
[  784.365034] DMAR: Device scope device [0000:80:01.00] not found
[  784.365053] DMAR: Device scope device [0000:80:03.00] not found
[  784.365075] DMAR: Device scope device [0000:80:05.00] not found
[  784.365094] DMAR: Device scope device [0000:80:07.00] not found
[  784.365116] DMAR: Device scope device [0000:80:09.00] not found
[  784.365166] DMAR: Device scope device [0000:c0:00.00] not found
[  784.365193] DMAR: Device scope device [0000:c0:01.00] not found
[  784.365216] DMAR: Device scope device [0000:c0:03.00] not found
[  784.365243] DMAR: Device scope device [0000:c0:05.00] not found
[  784.365266] DMAR: Device scope device [0000:c0:07.00] not found
[  784.365284] DMAR: Device scope device [0000:c0:09.00] not found


updated patch:

---

 drivers/pci/dmar.c        |  164 ++++++++++++++++++++++++----------------------
 drivers/pci/intel-iommu.c |  161 +++++++++++++++++++++++++--------------------
 include/linux/dmar.h      |   29 ++++++--
 3 files changed, 202 insertions(+), 152 deletions(-)


Index: linux-2.6/drivers/pci/dmar.c
===================================================================
--- linux-2.6.orig/drivers/pci/dmar.c
+++ linux-2.6/drivers/pci/dmar.c
@@ -61,8 +61,8 @@ static void __init dmar_register_drhd_un
 		list_add(&drhd->list, &dmar_drhd_units);
 }
 
-static int __init dmar_parse_one_dev_scope(struct acpi_dmar_device_scope *scope,
-					   struct pci_dev **dev, u16 segment)
+struct pci_dev *dmar_get_scope_dev(struct acpi_dmar_device_scope *scope,
+				   u16 segment)
 {
 	struct pci_bus *bus;
 	struct pci_dev *pdev = NULL;
@@ -74,7 +74,7 @@ static int __init dmar_parse_one_dev_sco
 	count = (scope->length - sizeof(struct acpi_dmar_device_scope))
 		/ sizeof(struct acpi_dmar_pci_path);
 
-	while (count) {
+	for (; count; path++, count--, bus = pdev->subordinate) {
 		if (pdev)
 			pci_dev_put(pdev);
 		/*
@@ -82,53 +82,77 @@ static int __init dmar_parse_one_dev_sco
 		 * ignore it
 		 */
 		if (!bus) {
-			printk(KERN_WARNING
-			PREFIX "Device scope bus [%d] not found\n",
-			scope->bus);
-			break;
+			printk(KERN_WARNING PREFIX
+			       "Device scope bus [%d] not found\n", scope->bus);
+			return NULL;
 		}
 		pdev = pci_get_slot(bus, PCI_DEVFN(path->dev, path->fn));
 		if (!pdev) {
 			printk(KERN_WARNING PREFIX
-			"Device scope device [%04x:%02x:%02x.%02x] not found\n",
-				segment, bus->number, path->dev, path->fn);
-			break;
+			       "Device scope device [%04x:%02x:%02x.%02x] not found\n",
+			       segment, bus->number, path->dev, path->fn);
+			return NULL;
 		}
-		path ++;
-		count --;
-		bus = pdev->subordinate;
-	}
-	if (!pdev) {
-		printk(KERN_WARNING PREFIX
-		"Device scope device [%04x:%02x:%02x.%02x] not found\n",
-		segment, scope->bus, path->dev, path->fn);
-		*dev = NULL;
+	}
+
+	return pdev;
+}
+
+static int dmar_match_scope_one(struct acpi_dmar_device_scope *scope,
+				struct pci_dev *dev, u16 segment)
+{
+	struct pci_dev *pdev;
+	int ret = 0;
+
+	if (segment != pci_domain_nr(dev->bus))
+		return 0;
+
+	pdev = dmar_get_scope_dev(scope, segment);
+	if (!pdev)
 		return 0;
+
+	if (scope->entry_type == ACPI_DMAR_SCOPE_TYPE_ENDPOINT) {
+		if (dev == pdev)
+			ret = 1;
+	} else {
+		while (dev) {
+			if (dev == pdev) {
+				ret = 1;
+				break;
+			}
+			dev = dev->bus->self;
+		}
 	}
-	if ((scope->entry_type == ACPI_DMAR_SCOPE_TYPE_ENDPOINT && \
-			pdev->subordinate) || (scope->entry_type == \
-			ACPI_DMAR_SCOPE_TYPE_BRIDGE && !pdev->subordinate)) {
-		pci_dev_put(pdev);
-		printk(KERN_WARNING PREFIX
-			"Device scope type does not match for %s\n",
-			 pci_name(pdev));
-		return -EINVAL;
+
+	pci_dev_put(pdev);
+
+	return ret;
+}
+
+int dmar_match_scope(struct acpi_dmar_device_scope **scopes, int cnt,
+		     struct pci_dev *dev, u16 segment)
+{
+	int i;
+
+	for (i = 0; i < cnt; i++) {
+		if (dmar_match_scope_one(scopes[i], dev, segment))
+			return 1;
 	}
-	*dev = pdev;
 	return 0;
 }
 
 static int __init dmar_parse_dev_scope(void *start, void *end, int *cnt,
-				       struct pci_dev ***devices, u16 segment)
+				       struct acpi_dmar_device_scope ***scopes)
 {
 	struct acpi_dmar_device_scope *scope;
-	void * tmp = start;
-	int index;
-	int ret;
+	void *tmp = start;
+	int index = 0;
 
 	*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)++;
@@ -138,27 +162,23 @@ static int __init dmar_parse_dev_scope(v
 		}
 		start += scope->length;
 	}
+
 	if (*cnt == 0)
 		return 0;
 
-	*devices = kcalloc(*cnt, sizeof(struct pci_dev *), GFP_KERNEL);
-	if (!*devices)
+	*scopes = kcalloc(*cnt, sizeof(struct acpi_dmar_device_scope *),
+                          GFP_KERNEL);
+	if (!*scopes)
 		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);
-				return ret;
-			}
-			index ++;
-		}
+		    scope->entry_type == ACPI_DMAR_SCOPE_TYPE_BRIDGE)
+			(*scopes)[index++] = scope;
+
 		start += scope->length;
 	}
 
@@ -207,9 +227,8 @@ static int __init dmar_parse_dev(struct
 		return 0;
 
 	ret = dmar_parse_dev_scope((void *)(drhd + 1),
-				((void *)drhd) + drhd->header.length,
-				&dmaru->devices_cnt, &dmaru->devices,
-				drhd->segment);
+				   ((void *)drhd) + drhd->header.length,
+				   &dmaru->scopes_cnt, &dmaru->scopes);
 	if (ret) {
 		list_del(&dmaru->list);
 		kfree(dmaru);
@@ -253,10 +272,10 @@ rmrr_parse_dev(struct dmar_rmrr_unit *rm
 
 	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);
+				   ((void *)rmrr) + rmrr->header.length,
+				   &rmrru->scopes_cnt, &rmrru->scopes);
 
-	if (ret || (rmrru->devices_cnt == 0)) {
+	if (ret || (rmrru->scopes_cnt == 0)) {
 		list_del(&rmrru->list);
 		kfree(rmrru);
 	}
@@ -293,10 +312,9 @@ static int __init atsr_parse_dev(struct
 
 	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) {
+				  (void *)atsr + atsr->header.length,
+				  &atsru->scopes_cnt, &atsru->scopes);
+	if (rc || !atsru->scopes_cnt) {
 		list_del(&atsru->list);
 		kfree(atsru);
 	}
@@ -310,6 +328,7 @@ int dmar_find_matched_atsr_unit(struct p
 	struct pci_bus *bus;
 	struct acpi_dmar_atsr *atsr;
 	struct dmar_atsr_unit *atsru;
+	struct pci_dev *pdev;
 
 	dev = pci_physfn(dev);
 
@@ -330,10 +349,18 @@ found:
 			return 0;
 
 		if (bridge->pcie_type == PCI_EXP_TYPE_ROOT_PORT) {
-			for (i = 0; i < atsru->devices_cnt; i++)
-				if (atsru->devices[i] == bridge)
+			for (i = 0; i < atsru->scopes_cnt; i++) {
+				pdev = dmar_get_scope_dev(atsru->scopes[i],
+							  atsr->segment);
+				if (!pdev)
+					continue;
+
+				if (pdev == bridge) {
+					pci_dev_put(pdev);
 					return 1;
-			break;
+				}
+				pci_dev_put(pdev);
+			}
 		}
 	}
 
@@ -513,23 +540,6 @@ parse_dmar_table(void)
 	return ret;
 }
 
-static int dmar_pci_device_match(struct pci_dev *devices[], int cnt,
-			  struct pci_dev *dev)
-{
-	int index;
-
-	while (dev) {
-		for (index = 0; index < cnt; index++)
-			if (dev == devices[index])
-				return 1;
-
-		/* Check our parent */
-		dev = dev->bus->self;
-	}
-
-	return 0;
-}
-
 struct dmar_drhd_unit *
 dmar_find_matched_drhd_unit(struct pci_dev *dev)
 {
@@ -544,11 +554,11 @@ dmar_find_matched_drhd_unit(struct pci_d
 				    header);
 
 		if (dmaru->include_all &&
-		    drhd->segment == pci_domain_nr(dev->bus))
+		    dmaru->segment == pci_domain_nr(dev->bus))
 			return dmaru;
 
-		if (dmar_pci_device_match(dmaru->devices,
-					  dmaru->devices_cnt, dev))
+		if (dmar_match_scope(dmaru->scopes, dmaru->scopes_cnt,
+				     dev, dmaru->segment))
 			return dmaru;
 	}
 
Index: linux-2.6/drivers/pci/intel-iommu.c
===================================================================
--- linux-2.6.orig/drivers/pci/intel-iommu.c
+++ linux-2.6/drivers/pci/intel-iommu.c
@@ -562,34 +562,49 @@ static void domain_update_iommu_cap(stru
 	domain_update_iommu_snooping(domain);
 }
 
-static struct intel_iommu *device_to_iommu(int segment, u8 bus, u8 devfn)
+static struct intel_iommu *__device_to_iommu(struct pci_dev *pdev)
 {
-	struct dmar_drhd_unit *drhd = NULL;
-	int i;
+	struct dmar_drhd_unit *dmaru = NULL;
+	struct intel_iommu *found = NULL;
+	int segment = pci_domain_nr(pdev->bus);
 
-	for_each_drhd_unit(drhd) {
-		if (drhd->ignored)
+	for_each_drhd_unit(dmaru) {
+		if (dmaru->ignored)
 			continue;
-		if (segment != drhd->segment)
+		if (segment != dmaru->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->subordinate >= bus)
-				return drhd->iommu;
+		if (dmaru->include_all) {
+			found = dmaru->iommu;
+			break;
+		}
+
+		if (dmar_match_scope(dmaru->scopes, dmaru->scopes_cnt,
+				     pdev, dmaru->segment)) {
+			found = dmaru->iommu;
+			break;
 		}
 
-		if (drhd->include_all)
-			return drhd->iommu;
 	}
 
-	return NULL;
+	return found;
+}
+
+static struct intel_iommu *device_to_iommu(int segment, u8 bus, u8 devfn)
+{
+	struct pci_dev *pdev;
+	struct intel_iommu *found = NULL;
+
+	pdev = pci_get_domain_bus_and_slot(segment, bus, devfn);
+
+	if (!pdev)
+		return NULL;
+
+	found = __device_to_iommu(pdev);
+
+	pci_dev_put(pdev);
+
+	return found;
 }
 
 static void domain_flush_cache(struct dmar_domain *domain,
@@ -990,12 +1005,12 @@ static void __iommu_flush_iotlb(struct i
 }
 
 static struct device_domain_info *iommu_support_dev_iotlb(
-	struct dmar_domain *domain, int segment, u8 bus, u8 devfn)
+	struct dmar_domain *domain, struct intel_iommu *iommu,
+	int segment, u8 bus, u8 devfn)
 {
 	int found = 0;
 	unsigned long flags;
 	struct device_domain_info *info;
-	struct intel_iommu *iommu = device_to_iommu(segment, bus, devfn);
 
 	if (!ecap_dev_iotlb_support(iommu->ecap))
 		return NULL;
@@ -1441,8 +1456,8 @@ static void domain_exit(struct dmar_doma
 	free_domain_mem(domain);
 }
 
-static int domain_context_mapping_one(struct dmar_domain *domain, int segment,
-				 u8 bus, u8 devfn, int translation)
+static int domain_context_mapping_one(struct dmar_domain *domain,
+				      struct pci_dev *pdev, int translation)
 {
 	struct context_entry *context;
 	unsigned long flags;
@@ -1461,11 +1476,12 @@ static int domain_context_mapping_one(st
 	BUG_ON(translation != CONTEXT_TT_PASS_THROUGH &&
 	       translation != CONTEXT_TT_MULTI_LEVEL);
 
-	iommu = device_to_iommu(segment, bus, devfn);
+	iommu = __device_to_iommu(pdev);
 	if (!iommu)
 		return -ENODEV;
 
-	context = device_to_context_entry(iommu, bus, devfn);
+	context = device_to_context_entry(iommu, pdev->bus->number,
+						 pdev->devfn);
 	if (!context)
 		return -ENOMEM;
 	spin_lock_irqsave(&iommu->lock, flags);
@@ -1522,7 +1538,7 @@ static int domain_context_mapping_one(st
 	context_set_domain_id(context, id);
 
 	if (translation != CONTEXT_TT_PASS_THROUGH) {
-		info = iommu_support_dev_iotlb(domain, segment, bus, devfn);
+		info = iommu_support_dev_iotlb(domain, iommu, pdev);
 		translation = info ? CONTEXT_TT_DEV_IOTLB :
 				     CONTEXT_TT_MULTI_LEVEL;
 	}
@@ -1578,9 +1594,7 @@ domain_context_mapping(struct dmar_domai
 	int ret;
 	struct pci_dev *tmp, *parent;
 
-	ret = domain_context_mapping_one(domain, pci_domain_nr(pdev->bus),
-					 pdev->bus->number, pdev->devfn,
-					 translation);
+	ret = domain_context_mapping_one(domain, pdev, translation);
 	if (ret)
 		return ret;
 
@@ -1591,25 +1605,19 @@ domain_context_mapping(struct dmar_domai
 	/* Secondary interface's bus number and devfn 0 */
 	parent = pdev->bus->self;
 	while (parent != tmp) {
-		ret = domain_context_mapping_one(domain,
-						 pci_domain_nr(parent->bus),
-						 parent->bus->number,
-						 parent->devfn, translation);
+		ret = domain_context_mapping_one(domain, parent, translation);
 		if (ret)
 			return ret;
 		parent = parent->bus->self;
 	}
-	if (pci_is_pcie(tmp)) /* this is a PCIe-to-PCI bridge */
-		return domain_context_mapping_one(domain,
-					pci_domain_nr(tmp->subordinate),
-					tmp->subordinate->number, 0,
-					translation);
-	else /* this is a legacy PCI bridge */
-		return domain_context_mapping_one(domain,
-						  pci_domain_nr(tmp->bus),
-						  tmp->bus->number,
-						  tmp->devfn,
-						  translation);
+	if (pci_is_pcie(tmp)) {
+		 /* this is a PCIe-to-PCI bridge */
+		struct pci_dev *child = pci_get_slot(tmp->subordinate, 0);
+		ret = domain_context_mapping_one(domain, child, translation);
+		pci_dev_put(child);
+		return ret;
+	} else /* this is a legacy PCI bridge */
+		return domain_context_mapping_one(domain, tmp, translation);
 }
 
 static int domain_context_mapped(struct pci_dev *pdev)
@@ -1618,8 +1626,7 @@ static int domain_context_mapped(struct
 	struct pci_dev *tmp, *parent;
 	struct intel_iommu *iommu;
 
-	iommu = device_to_iommu(pci_domain_nr(pdev->bus), pdev->bus->number,
-				pdev->devfn);
+	iommu = __device_to_iommu(pdev);
 	if (!iommu)
 		return -ENODEV;
 
@@ -2233,7 +2240,7 @@ static int __init init_dmars(int force_o
 	struct dmar_rmrr_unit *rmrr;
 	struct pci_dev *pdev;
 	struct intel_iommu *iommu;
-	int i, ret;
+	int ret;
 
 	/*
 	 * for each drhd
@@ -2382,18 +2389,22 @@ static int __init init_dmars(int force_o
 	 */
 	printk(KERN_INFO "IOMMU: Setting RMRR:\n");
 	for_each_rmrr_units(rmrr) {
-		for (i = 0; i < rmrr->devices_cnt; i++) {
-			pdev = rmrr->devices[i];
-			/*
-			 * some BIOS lists non-exist devices in DMAR
-			 * table.
-			 */
+		struct acpi_dmar_reserved_memory *rmrrh;
+		int i;
+
+		rmrrh = container_of(rmrr->hdr,
+				     struct acpi_dmar_reserved_memory, header);
+
+		for (i = 0; i < rmrr->scopes_cnt; i++) {
+			pdev = dmar_get_scope_dev(rmrr->scopes[i],
+						  rmrrh->segment);
 			if (!pdev)
 				continue;
-			ret = iommu_prepare_rmrr_dev(rmrr, pdev);
-			if (ret)
+
+			if (iommu_prepare_rmrr_dev(rmrr, pdev))
 				printk(KERN_ERR
 				       "IOMMU: mapping reserved region failed\n");
+			pci_dev_put(pdev);
 		}
 	}
 
@@ -3075,15 +3086,21 @@ DECLARE_PCI_FIXUP_ENABLE(PCI_VENDOR_ID_I
 static void __init init_no_remapping_devices(void)
 {
 	struct dmar_drhd_unit *drhd;
+	struct pci_dev *pdev;
 
 	for_each_drhd_unit(drhd) {
 		if (!drhd->include_all) {
 			int i;
-			for (i = 0; i < drhd->devices_cnt; i++)
-				if (drhd->devices[i] != NULL)
+			for (i = 0; i < drhd->scopes_cnt; i++) {
+				pdev = dmar_get_scope_dev(drhd->scopes[i],
+							  drhd->segment);
+				if (pdev) {
+					pci_dev_put(pdev);
 					break;
+				}
+			}
 			/* ignore DMAR unit if no pci devices exist */
-			if (i == drhd->devices_cnt)
+			if (i == drhd->scopes_cnt)
 				drhd->ignored = 1;
 		}
 	}
@@ -3096,20 +3113,28 @@ static void __init init_no_remapping_dev
 		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]))
+		for (i = 0; i < drhd->scopes_cnt; i++) {
+			pdev = dmar_get_scope_dev(drhd->scopes[i],
+						  drhd->segment);
+			if (pdev && !IS_GFX_DEVICE(pdev)) {
+				pci_dev_put(pdev);
 				break;
+			}
+			pci_dev_put(pdev);
+		}
 
-		if (i < drhd->devices_cnt)
+		if (i < drhd->scopes_cnt)
 			continue;
 
 		/* bypass IOMMU if it is just for gfx devices */
 		drhd->ignored = 1;
-		for (i = 0; i < drhd->devices_cnt; i++) {
-			if (!drhd->devices[i])
+		for (i = 0; i < drhd->scopes_cnt; i++) {
+			pdev = dmar_get_scope_dev(drhd->scopes[i],
+						  drhd->segment);
+			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);
 		}
 	}
 }
@@ -3378,8 +3403,7 @@ static void domain_remove_one_dev_info(s
 	int found = 0;
 	struct list_head *entry, *tmp;
 
-	iommu = device_to_iommu(pci_domain_nr(pdev->bus), pdev->bus->number,
-				pdev->devfn);
+	iommu = __device_to_iommu(pdev);
 	if (!iommu)
 		return;
 
@@ -3622,8 +3646,7 @@ static int intel_iommu_attach_device(str
 		}
 	}
 
-	iommu = device_to_iommu(pci_domain_nr(pdev->bus), pdev->bus->number,
-				pdev->devfn);
+	iommu = __device_to_iommu(pdev);
 	if (!iommu)
 		return -ENODEV;
 
Index: linux-2.6/include/linux/dmar.h
===================================================================
--- linux-2.6.orig/include/linux/dmar.h
+++ linux-2.6/include/linux/dmar.h
@@ -32,8 +32,8 @@ 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	acpi_dmar_device_scope **scopes; /* target scope array	*/
+	int	scopes_cnt;		/* target scope count	*/
 	u16	segment;		/* PCI domain		*/
 	u8	ignored:1; 		/* ignore drhd		*/
 	u8	include_all:1;
@@ -55,6 +55,9 @@ extern struct list_head dmar_drhd_units;
 
 extern int dmar_table_init(void);
 extern int dmar_dev_scope_init(void);
+extern int dmar_match_scope(struct acpi_dmar_device_scope **, int,
+			    struct pci_dev *, u16);
+extern struct pci_dev *dmar_get_scope_dev(struct acpi_dmar_device_scope *, u16);
 
 /* Intel IOMMU detection */
 extern int detect_intel_iommu(void);
@@ -72,6 +75,20 @@ static inline int dmar_table_init(void)
 {
 	return -ENODEV;
 }
+
+static inline int dmar_match_scope(struct acpi_dmar_device_scope **scopes,
+				   int cnt, struct pci_dev *dev, u16 segment)
+{
+	return 0;
+}
+
+static inline struct pci_dev *dmar_get_scope_dev(
+					struct acpi_dmar_device_scope *scope,
+					u16 segment)
+{
+	return NULL;
+}
+
 static inline int enable_drhd_fault_handling(void)
 {
 	return -1;
@@ -212,8 +229,8 @@ 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	acpi_dmar_device_scope **scopes; /* target scope array */
+	int	scopes_cnt;		/* target scope count */
 };
 
 #define for_each_rmrr_units(rmrr) \
@@ -222,8 +239,8 @@ 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 */
+	struct acpi_dmar_device_scope **scopes;	/* target scope array */
+	int scopes_cnt;		/* target scope count */
 	u8 include_all:1;		/* include all ports */
 };
 

Re: [PATCH] pci, dmar: Update dmar units devices list during hotplug

[Alex Williamson]

On Tue, 2011-05-24 at 22:45 -0700, Yinghai Lu wrote:
> On 05/24/2011 04:02 PM, Yinghai Lu wrote:
> > On 05/24/2011 03:38 PM, Alex Williamson wrote:
> >> On Tue, 2011-05-24 at 14:45 -0700, Yinghai Lu wrote:
> >>> No, it does not work.
> >>
> >> I didn't say this wasn't without some effort, just thought it might give
> >> you a jump start.
> > 
> > ok, let me debug it tonight.
> > 
> > looks like that pdev is not freed, but already get removed from the device tree.
> > 
> > may need to pass pci_dev pointer directly.
> > 
> 
> keep getting:

Ugh, that's going to make it pretty difficult to use a dynamic lookup
approach.  Maybe we're stuck with caching the scope->pdev translation
somewhere.  We might have to turn devices into a list and include device
add/remove notification in dmar.c  I'm not sure what the best approach
is, I was just hoping to avoid leaving a known stale pointer in the
devices array with a kludgy fixup later.  Sorry if this patch was more
trouble than it's worth.  Thanks,

Alex

> [  784.364244] BUG: sleeping function called from invalid context at kernel/rwsem.c:21
> [  784.364253] in_atomic(): 0, irqs_disabled(): 1, pid: 29398, name: work_for_cpu
> [  784.364259] INFO: lockdep is turned off.
> [  784.364265] irq event stamp: 0
> [  784.364271] hardirqs last  enabled at (0): [<          (null)>]           (null)
> [  784.364282] hardirqs last disabled at (0): [<ffffffff8107ba5e>] copy_process+0x43b/0xd95
> [  784.364305] softirqs last  enabled at (0): [<ffffffff8107ba5e>] copy_process+0x43b/0xd95
> [  784.364318] softirqs last disabled at (0): [<          (null)>]           (null)
> [  784.364332] Pid: 29398, comm: work_for_cpu Not tainted 2.6.39-tip-yh-06791-gb282579-dirty #1047
> [  784.364339] Call Trace:
> [  784.364375]  [<ffffffff810ad12a>] ? print_irqtrace_events+0xd0/0xd4
> [  784.364392]  [<ffffffff81071572>] __might_sleep+0xf2/0xf6
> [  784.364410]  [<ffffffff81c20fc0>] down_read+0x26/0x91
> [  784.364429]  [<ffffffff8134c089>] pci_find_next_bus+0x45/0x75
> [  784.364442]  [<ffffffff8134c0fa>] pci_find_bus+0x41/0x54
> [  784.364457]  [<ffffffff8136263f>] dmar_get_scope_dev+0x2f/0xe3
> [  784.364474]  [<ffffffff813353ad>] ? random32+0x19/0x1b
> [  784.364488]  [<ffffffff8136273d>] dmar_match_scope+0x4a/0xb6
> [  784.364502]  [<ffffffff813628d4>] dmar_find_matched_drhd_unit+0x55/0x6f
> [  784.364519]  [<ffffffff81367018>] get_domain_for_dev.clone.2+0x103/0x392
> [  784.364533]  [<ffffffff81367459>] __get_valid_domain_for_dev+0x14/0x88
> [  784.364546]  [<ffffffff813676c5>] __intel_map_single+0x58/0x174
> [  784.364559]  [<ffffffff813678eb>] intel_alloc_coherent+0xc7/0xee
> [  784.364575]  [<ffffffff811281dc>] pool_alloc_page.clone.0+0xc9/0x140
> [  784.364588]  [<ffffffff811282d8>] dma_pool_alloc+0x85/0x131
> [  784.364603]  [<ffffffff81134b8c>] ? should_failslab+0x44/0x48
> [  784.364618]  [<ffffffff81132d62>] ? kmem_cache_alloc_trace+0x5e/0x123
> [  784.364635]  [<ffffffff818429aa>] ehci_qh_alloc+0x59/0xd2
> [  784.364649]  [<ffffffff81844451>] ehci_mem_init.clone.1+0x84/0x25c
> [  784.364660]  [<ffffffff8184471b>] ehci_init+0xf2/0x245
> [  784.364671]  [<ffffffff81844999>] ehci_pci_setup+0x12b/0x564
> [  784.364687]  [<ffffffff818323d5>] usb_add_hcd+0x10f/0x318
> [  784.364703]  [<ffffffff8183e31e>] usb_hcd_pci_probe+0x1e4/0x312
> [  784.364722]  [<ffffffff8134ae25>] local_pci_probe+0x4d/0x96
> [  784.364739]  [<ffffffff81093c4b>] ? cwq_dec_nr_in_flight+0x81/0x81
> [  784.364754]  [<ffffffff81093c63>] do_work_for_cpu+0x18/0x2b
> [  784.364770]  [<ffffffff81093c4b>] ? cwq_dec_nr_in_flight+0x81/0x81
> [  784.364787]  [<ffffffff81099ea5>] kthread+0xa0/0xa8
> [  784.364801]  [<ffffffff810adbcc>] ? trace_hardirqs_on_caller+0x1f/0x178
> [  784.364818]  [<ffffffff81c2a214>] kernel_thread_helper+0x4/0x10
> [  784.364832]  [<ffffffff81c224cc>] ? _raw_spin_unlock_irq+0x30/0x36
> [  784.364846]  [<ffffffff810add32>] ? trace_hardirqs_on+0xd/0xf
> [  784.364861]  [<ffffffff81c227c0>] ? retint_restore_args+0xe/0xe
> [  784.364880]  [<ffffffff81099e05>] ? __init_kthread_worker+0x5b/0x5b
> [  784.364893]  [<ffffffff81c2a210>] ? gs_change+0xb/0xb
> [  784.364902] DMAR: Device scope device [0000:40:00.00] not found
> [  784.364910] DMAR: Device scope device [0000:40:01.00] not found
> [  784.364931] DMAR: Device scope device [0000:40:03.00] not found
> [  784.364948] DMAR: Device scope device [0000:40:05.00] not found
> [  784.364961] DMAR: Device scope device [0000:40:07.00] not found
> [  784.364978] DMAR: Device scope device [0000:40:09.00] not found
> [  784.365019] DMAR: Device scope device [0000:80:00.00] not found
> [  784.365034] DMAR: Device scope device [0000:80:01.00] not found
> [  784.365053] DMAR: Device scope device [0000:80:03.00] not found
> [  784.365075] DMAR: Device scope device [0000:80:05.00] not found
> [  784.365094] DMAR: Device scope device [0000:80:07.00] not found
> [  784.365116] DMAR: Device scope device [0000:80:09.00] not found
> [  784.365166] DMAR: Device scope device [0000:c0:00.00] not found
> [  784.365193] DMAR: Device scope device [0000:c0:01.00] not found
> [  784.365216] DMAR: Device scope device [0000:c0:03.00] not found
> [  784.365243] DMAR: Device scope device [0000:c0:05.00] not found
> [  784.365266] DMAR: Device scope device [0000:c0:07.00] not found
> [  784.365284] DMAR: Device scope device [0000:c0:09.00] not found

Re: [PATCH] pci, dmar: Update dmar units devices list during hotplug

[David Woodhouse]

On Wed, 2011-05-25 at 06:43 -0600, Alex Williamson wrote:
> 
> Ugh, that's going to make it pretty difficult to use a dynamic lookup
> approach.  Maybe we're stuck with caching the scope->pdev translation
> somewhere. 

Can't you handle it on BUS_NOTIFY_ADD_DEVICE notification, and store it
in pdev->dev.archdata? Might as well do the ATSR the same way, so it
matches.

Hm, why the hell do we have device_to_iommu(), which seems to duplicate
the dmar_find_matched_drhd_unit() function? Introduced by Weidong in
commit c7151a8d in 2008... Weidong?

I have a *vague* recollection that we cannot assume that there will be a
matching extant pci_dev for every call to device_to_iommu(), which is
why it takes domai^H^H^H^H^Hsegment/bus/devfn arguments as it does. And
that would be a reasonable excuse for adding it in the first place, I
suppose. I note your patch will break if that's true...

-- 
dwmw2