Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1762572AbXIVGt3 (ORCPT ); Sat, 22 Sep 2007 02:49:29 -0400 Received: (majordomo@vger.kernel.org) by vger.kernel.org id S1757184AbXIVGtV (ORCPT ); Sat, 22 Sep 2007 02:49:21 -0400 Received: from wa-out-1112.google.com ([209.85.146.179]:50124 "EHLO wa-out-1112.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1756659AbXIVGtU (ORCPT ); Sat, 22 Sep 2007 02:49:20 -0400 DomainKey-Signature: a=rsa-sha1; c=nofws; d=gmail.com; s=beta; h=received:message-id:date:from:to:subject:cc:in-reply-to:mime-version:content-type:content-transfer-encoding:content-disposition:references; b=LPN2I+8v1R4DQU9mwf0klhh/GsBZsDVlxLTNcE6SjMt5m8Grb/94GuCIlm2c1KOhQ0qTf3/PrQ9h7UU75S8hLFg3J/8pJq05QStvMkLYlnjBT1M9xVPQbcpVmllUK9EerTr8Hfpwaz1TVGUz/6UsIVhg+l3Bz1onI5z1may6l18= Message-ID: <86802c440709212349l3d968d1bgdc89e7c7415b54da@mail.gmail.com> Date: Fri, 21 Sep 2007 23:49:19 -0700 From: "Yinghai Lu" To: "Andi Kleen" Subject: Re: [PATCH] [9/50] i386: validate against ACPI motherboard resources Cc: hancockr@shaw.ca, rajesh.shah@intel.com, jbarnes@virtuousgeek.org, greg@kroah.com, patches@x86-64.org, linux-kernel@vger.kernel.org In-Reply-To: <20070921223207.7BBE71479D@wotan.suse.de> MIME-Version: 1.0 Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit Content-Disposition: inline References: <200709221231.836138000@suse.de> <20070921223207.7BBE71479D@wotan.suse.de> Sender: linux-kernel-owner@vger.kernel.org X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 12016 Lines: 311 On 9/21/07, Andi Kleen wrote: > > From: Robert Hancock > > This path adds validation of the MMCONFIG table against the ACPI reserved > motherboard resources. If the MMCONFIG table is found to be reserved in > ACPI, we don't bother checking the E820 table. The PCI Express firmware > spec apparently tells BIOS developers that reservation in ACPI is required > and E820 reservation is optional, so checking against ACPI first makes > sense. Many BIOSes don't reserve the MMCONFIG region in E820 even though > it is perfectly functional, the existing check needlessly disables MMCONFIG > in these cases. > > In order to do this, MMCONFIG setup has been split into two phases. If PCI > configuration type 1 is not available then MMCONFIG is enabled early as > before. Otherwise, it is enabled later after the ACPI interpreter is > enabled, since we need to be able to execute control methods in order to > check the ACPI reserved resources. Presently this is just triggered off > the end of ACPI interpreter initialization. > > There are a few other behavioral changes here: > > - Validate all MMCONFIG configurations provided, not just the first one. > > - Validate the entire required length of each configuration according to > the provided ending bus number is reserved, not just the minimum required > allocation. > > - Validate that the area is reserved even if we read it from the chipset > directly and not from the MCFG table. This catches the case where the > BIOS didn't set the location properly in the chipset and has mapped it > over other things it shouldn't have. > > This also cleans up the MMCONFIG initialization functions so that they > simply do nothing if MMCONFIG is not compiled in. > > Based on an original patch by Rajesh Shah from Intel. > > [akpm@linux-foundation.org: many fixes and cleanups] > Signed-off-by: Robert Hancock > Signed-off-by: Andi Kleen > Cc: Rajesh Shah > Cc: Jesse Barnes > Acked-by: Linus Torvalds > Cc: Andi Kleen > Cc: Greg KH > Signed-off-by: Andrew Morton > --- > > arch/i386/pci/init.c | 4 - > arch/i386/pci/mmconfig-shared.c | 151 +++++++++++++++++++++++++++++++++++----- > arch/i386/pci/pci.h | 1 > drivers/acpi/bus.c | 2 > include/linux/pci.h | 8 ++ > 5 files changed, 144 insertions(+), 22 deletions(-) > > Index: linux/arch/i386/pci/init.c > =================================================================== > --- linux.orig/arch/i386/pci/init.c > +++ linux/arch/i386/pci/init.c > @@ -11,9 +11,7 @@ static __init int pci_access_init(void) > #ifdef CONFIG_PCI_DIRECT > type = pci_direct_probe(); > #endif > -#ifdef CONFIG_PCI_MMCONFIG > - pci_mmcfg_init(type); > -#endif > + pci_mmcfg_early_init(type); > if (raw_pci_ops) > return 0; > #ifdef CONFIG_PCI_BIOS > Index: linux/arch/i386/pci/mmconfig-shared.c > =================================================================== > --- linux.orig/arch/i386/pci/mmconfig-shared.c > +++ linux/arch/i386/pci/mmconfig-shared.c > @@ -206,9 +206,78 @@ static void __init pci_mmcfg_insert_reso > pci_mmcfg_resources_inserted = 1; > } > > -static void __init pci_mmcfg_reject_broken(int type) > +static acpi_status __init check_mcfg_resource(struct acpi_resource *res, > + void *data) > +{ > + struct resource *mcfg_res = data; > + struct acpi_resource_address64 address; > + acpi_status status; > + > + if (res->type == ACPI_RESOURCE_TYPE_FIXED_MEMORY32) { > + struct acpi_resource_fixed_memory32 *fixmem32 = > + &res->data.fixed_memory32; > + if (!fixmem32) > + return AE_OK; > + if ((mcfg_res->start >= fixmem32->address) && > + (mcfg_res->end < (fixmem32->address + > + fixmem32->address_length))) { > + mcfg_res->flags = 1; > + return AE_CTRL_TERMINATE; > + } > + } > + if ((res->type != ACPI_RESOURCE_TYPE_ADDRESS32) && > + (res->type != ACPI_RESOURCE_TYPE_ADDRESS64)) > + return AE_OK; > + > + status = acpi_resource_to_address64(res, &address); > + if (ACPI_FAILURE(status) || > + (address.address_length <= 0) || > + (address.resource_type != ACPI_MEMORY_RANGE)) > + return AE_OK; > + > + if ((mcfg_res->start >= address.minimum) && > + (mcfg_res->end < (address.minimum + address.address_length))) { > + mcfg_res->flags = 1; > + return AE_CTRL_TERMINATE; > + } > + return AE_OK; > +} > + > +static acpi_status __init find_mboard_resource(acpi_handle handle, u32 lvl, > + void *context, void **rv) > +{ > + struct resource *mcfg_res = context; > + > + acpi_walk_resources(handle, METHOD_NAME__CRS, > + check_mcfg_resource, context); > + > + if (mcfg_res->flags) > + return AE_CTRL_TERMINATE; > + > + return AE_OK; > +} > + > +static int __init is_acpi_reserved(unsigned long start, unsigned long end) > +{ > + struct resource mcfg_res; > + > + mcfg_res.start = start; > + mcfg_res.end = end; > + mcfg_res.flags = 0; > + > + acpi_get_devices("PNP0C01", find_mboard_resource, &mcfg_res, NULL); > + > + if (!mcfg_res.flags) > + acpi_get_devices("PNP0C02", find_mboard_resource, &mcfg_res, > + NULL); > + > + return mcfg_res.flags; > +} > + > +static void __init pci_mmcfg_reject_broken(void) > { > typeof(pci_mmcfg_config[0]) *cfg; > + int i; > > if ((pci_mmcfg_config_num == 0) || > (pci_mmcfg_config == NULL) || > @@ -229,17 +298,37 @@ static void __init pci_mmcfg_reject_brok > goto reject; > } > > - /* > - * Only do this check when type 1 works. If it doesn't work > - * assume we run on a Mac and always use MCFG > - */ > - if (type == 1 && !e820_all_mapped(cfg->address, > - cfg->address + MMCONFIG_APER_MIN, > - E820_RESERVED)) { > - printk(KERN_ERR "PCI: BIOS Bug: MCFG area at %Lx is not" > - " E820-reserved\n", cfg->address); > - goto reject; > + for (i = 0; i < pci_mmcfg_config_num; i++) { > + u32 size = (cfg->end_bus_number + 1) << 20; > + cfg = &pci_mmcfg_config[i]; > + printk(KERN_NOTICE "PCI: MCFG configuration %d: base %lu " > + "segment %hu buses %u - %u\n", > + i, (unsigned long)cfg->address, cfg->pci_segment, > + (unsigned int)cfg->start_bus_number, > + (unsigned int)cfg->end_bus_number); > + if (is_acpi_reserved(cfg->address, cfg->address + size - 1)) { > + printk(KERN_NOTICE "PCI: MCFG area at %Lx reserved " > + "in ACPI motherboard resources\n", > + cfg->address); > + } else { > + printk(KERN_ERR "PCI: BIOS Bug: MCFG area at %Lx is not" > + " reserved in ACPI motherboard resources\n", > + cfg->address); > + /* Don't try to do this check unless configuration > + type 1 is available. */ > + if ((pci_probe & PCI_PROBE_CONF1) && > + e820_all_mapped(cfg->address, > + cfg->address + size - 1, > + E820_RESERVED)) > + printk(KERN_NOTICE > + "PCI: MCFG area at %Lx reserved in " > + "E820\n", > + cfg->address); > + else > + goto reject; > + } > } > + > return; > > reject: > @@ -249,20 +338,46 @@ reject: > pci_mmcfg_config_num = 0; > } > > -void __init pci_mmcfg_init(int type) > +void __init pci_mmcfg_early_init(int type) > +{ > + if ((pci_probe & PCI_PROBE_MMCONF) == 0) > + return; > + > + /* If type 1 access is available, no need to enable MMCONFIG yet, we can > + defer until later when the ACPI interpreter is available to better > + validate things. */ > + if (type == 1) > + return; > + > + acpi_table_parse(ACPI_SIG_MCFG, acpi_parse_mcfg); > + > + if ((pci_mmcfg_config_num == 0) || > + (pci_mmcfg_config == NULL) || > + (pci_mmcfg_config[0].address == 0)) > + return; > + > + if (pci_mmcfg_arch_init()) > + pci_probe = (pci_probe & ~PCI_PROBE_MASK) | PCI_PROBE_MMCONF; > +} > + > +void __init pci_mmcfg_late_init(void) > { > int known_bridge = 0; > > + /* MMCONFIG disabled */ > if ((pci_probe & PCI_PROBE_MMCONF) == 0) > return; > > - if (type == 1 && pci_mmcfg_check_hostbridge()) > - known_bridge = 1; > + /* MMCONFIG already enabled */ > + if (!(pci_probe & PCI_PROBE_MASK & ~PCI_PROBE_MMCONF)) > + return; > > - if (!known_bridge) { > + if ((pci_probe & PCI_PROBE_CONF1) && pci_mmcfg_check_hostbridge()) > + known_bridge = 1; > + else > acpi_table_parse(ACPI_SIG_MCFG, acpi_parse_mcfg); > - pci_mmcfg_reject_broken(type); > - } > + > + pci_mmcfg_reject_broken(); > > if ((pci_mmcfg_config_num == 0) || > (pci_mmcfg_config == NULL) || > @@ -270,7 +385,7 @@ void __init pci_mmcfg_init(int type) > return; > > if (pci_mmcfg_arch_init()) { > - if (type == 1) > + if (pci_probe & PCI_PROBE_CONF1) > unreachable_devices(); > if (known_bridge) > pci_mmcfg_insert_resources(IORESOURCE_BUSY); > Index: linux/arch/i386/pci/pci.h > =================================================================== > --- linux.orig/arch/i386/pci/pci.h > +++ linux/arch/i386/pci/pci.h > @@ -91,7 +91,6 @@ extern int pci_conf1_read(unsigned int s > extern int pci_direct_probe(void); > extern void pci_direct_init(int type); > extern void pci_pcbios_init(void); > -extern void pci_mmcfg_init(int type); > extern void pcibios_sort(void); > > /* pci-mmconfig.c */ > Index: linux/drivers/acpi/bus.c > =================================================================== > --- linux.orig/drivers/acpi/bus.c > +++ linux/drivers/acpi/bus.c > @@ -35,6 +35,7 @@ > #ifdef CONFIG_X86 > #include > #endif > +#include > #include > #include > > @@ -757,6 +758,7 @@ static int __init acpi_init(void) > result = acpi_bus_init(); > > if (!result) { > + pci_mmcfg_late_init(); No! MMCONFIG will not work with acpi=off any more. because acpi_init==>pci_mmcfg_late_init==>pci_mmcfg_check_hostbridge... can you move pci_mmcfg_later_init out of acpi_init and just call somewhere after acpi_init... or put pci_mmcfg_check_hostbridge back to pci_mmcfg_early_init? YH - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/