From patchwork Tue May 24 20:07:19 2011 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Alex Williamson X-Patchwork-Id: 813572 Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by demeter1.kernel.org (8.14.4/8.14.3) with ESMTP id p4OK82lk017086 for ; Tue, 24 May 2011 20:08:03 GMT Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754241Ab1EXUHf (ORCPT ); Tue, 24 May 2011 16:07:35 -0400 Received: from mx1.redhat.com ([209.132.183.28]:33426 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753288Ab1EXUHe (ORCPT ); Tue, 24 May 2011 16:07:34 -0400 Received: from int-mx01.intmail.prod.int.phx2.redhat.com (int-mx01.intmail.prod.int.phx2.redhat.com [10.5.11.11]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id p4OK7LTv024395 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK); Tue, 24 May 2011 16:07:21 -0400 Received: from [10.3.113.42] (ovpn-113-42.phx2.redhat.com [10.3.113.42]) by int-mx01.intmail.prod.int.phx2.redhat.com (8.13.8/8.13.8) with ESMTP id p4OK7JQK011207; Tue, 24 May 2011 16:07:20 -0400 Subject: Re: [PATCH] pci, dmar: Update dmar units devices list during hotplug From: Alex Williamson To: Yinghai Lu Cc: David Woodhouse , Vinod Koul , linux-pci@vger.kernel.org, linux-kernel@vger.kernel.org, Jesse Barnes , iommu@lists.linux-foundation.org, Dan Williams , Andrew Morton In-Reply-To: <4DDC0858.3090406@kernel.org> References: <4DC6E569.1060808@kernel.org> <1305843308.3100.10.camel@x201> <1306234696.7218.14.camel@i7.infradead.org> <1306258959.3100.79.camel@x201> <4DDC0858.3090406@kernel.org> Date: Tue, 24 May 2011 14:07:19 -0600 Message-ID: <1306267639.3100.102.camel@x201> Mime-Version: 1.0 X-Scanned-By: MIMEDefang 2.67 on 10.5.11.11 Sender: linux-pci-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-pci@vger.kernel.org X-Greylist: IP, sender and recipient auto-whitelisted, not delayed by milter-greylist-4.2.6 (demeter1.kernel.org [140.211.167.41]); Tue, 24 May 2011 20:08:03 +0000 (UTC) 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 --- drivers/pci/dmar.c | 162 ++++++++++++++++++++++++--------------------- drivers/pci/intel-iommu.c | 94 ++++++++++++++++---------- include/linux/dmar.h | 29 ++++++-- 3 files changed, 166 insertions(+), 119 deletions(-) -- To unsubscribe from this list: send the line "unsubscribe linux-pci" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html 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 */ };