[PATCH 0/6][RFC] pci, x86: change the way to add MMCFG region on x86

[PATCH 0/6][RFC] pci, x86: change the way to add MMCFG region on x86

[Taku Izumi]

 This patchset is based on the following Jiang Liu's patchset:
   http://marc.info/?l=linux-pci&m=133410334617298&w=2

 This changes the way to add MMCFG region on x86.
 MMCFG region information provided by ACPI MCFG table 
 or _CBA method will be added at acpi_pci_root_add()
 of pci_root driver.
 
 patch1-4 is exactly same as Liu's.

  *[PATCH 1/6] PCI, x86: split out pci_mmcfg_check_reserved() for code reuse
  *[PATCH 2/6] PCI, x86: split out pci_mmconfig_alloc() for code reuse
  *[PATCH 3/6] PCI, x86: use RCU list to protect mmconfig list
  *[PATCH 4/6] PCI, x86: introduce pci_mmcfg_arch_map()/pci_mmcfg_arch_unmap()
  *[PATCH 5/6] PCI, x86: introduce pci_mmcongi_add() / pci_mmconfig_delete()
  *[PATCH 6/6] PCI, x86: change the way to add MMCFG region on x86
  
-
Best regards,
Taku Izumi

[PATCH 1/6] PCI, x86: split out pci_mmcfg_check_reserved() for code reuse

[Taku Izumi]

Split out pci_mmcfg_check_reserved() for code reuse, which will be used
when supporting PCI host bridge hotplug.

Signed-off-by: Jiang Liu <jiang.liu@huawei.com>
---
 arch/x86/pci/mmconfig-shared.c |   51 ++++++++++++++++++++---------------------
 1 file changed, 25 insertions(+), 26 deletions(-)

Index: bjorn-next/arch/x86/pci/mmconfig-shared.c
===================================================================
--- bjorn-next.orig/arch/x86/pci/mmconfig-shared.c
+++ bjorn-next/arch/x86/pci/mmconfig-shared.c
@@ -474,39 +474,38 @@ static int __init is_mmconf_reserved(che
 	return valid;
 }
 
+static int __devinit pci_mmcfg_check_reserved(struct pci_mmcfg_region *cfg,
+					      int early)
+{
+	if (!early && !acpi_disabled) {
+		if (is_mmconf_reserved(is_acpi_reserved, cfg, 0))
+			return true;
+		else
+			printk(KERN_ERR FW_BUG PREFIX
+			       "MMCONFIG at %pR not reserved in "
+			       "ACPI motherboard resources\n",
+			       &cfg->res);
+	}
+
+	/* Don't try to do this check unless configuration
+	   type 1 is available. how about type 2 ?*/
+	if (raw_pci_ops)
+		return is_mmconf_reserved(e820_all_mapped, cfg, 1);
+
+	return false;
+}
+
 static void __init pci_mmcfg_reject_broken(int early)
 {
 	struct pci_mmcfg_region *cfg;
 
 	list_for_each_entry(cfg, &pci_mmcfg_list, list) {
-		int valid = 0;
-
-		if (!early && !acpi_disabled) {
-			valid = is_mmconf_reserved(is_acpi_reserved, cfg, 0);
-
-			if (valid)
-				continue;
-			else
-				printk(KERN_ERR FW_BUG PREFIX
-				       "MMCONFIG at %pR not reserved in "
-				       "ACPI motherboard resources\n",
-				       &cfg->res);
+		if (pci_mmcfg_check_reserved(cfg, early) == false) {
+			printk(KERN_INFO PREFIX "not using MMCONFIG\n");
+			free_all_mmcfg();
+			return;
 		}
-
-		/* Don't try to do this check unless configuration
-		   type 1 is available. how about type 2 ?*/
-		if (raw_pci_ops)
-			valid = is_mmconf_reserved(e820_all_mapped, cfg, 1);
-
-		if (!valid)
-			goto reject;
 	}
-
-	return;
-
-reject:
-	printk(KERN_INFO PREFIX "not using MMCONFIG\n");
-	free_all_mmcfg();
 }
 
 static int __initdata known_bridge;

Re: [PATCH 1/6] PCI, x86: split out pci_mmcfg_check_reserved() for code reuse

[Yinghai Lu]

On Thu, Apr 26, 2012 at 3:39 AM, Taku Izumi <izumi.taku@jp.fujitsu.com> wrote:
>
> Split out pci_mmcfg_check_reserved() for code reuse, which will be used
> when supporting PCI host bridge hotplug.
>
> Signed-off-by: Jiang Liu <jiang.liu@huawei.com>

Is the patch from You or Jiang?

Please put

From: Jiang Liu <jiang.liu@huawei.com>

at beginning if it is from Jiang.

Thanks

Yinghai

[PATCH 2/6] PCI, x86: split out pci_mmconfig_alloc() for code reuse

[Taku Izumi]

Split out pci_mmconfig_alloc() for code reuse.

Signed-off-by: Jiang Liu <jiang.liu@huawei.com>
---
 arch/x86/pci/mmconfig-shared.c |   19 +++++++++++++++----
 1 file changed, 15 insertions(+), 4 deletions(-)

Index: bjorn-next/arch/x86/pci/mmconfig-shared.c
===================================================================
--- bjorn-next.orig/arch/x86/pci/mmconfig-shared.c
+++ bjorn-next/arch/x86/pci/mmconfig-shared.c
@@ -61,8 +61,9 @@ static __init void list_add_sorted(struc
 	list_add_tail(&new->list, &pci_mmcfg_list);
 }
 
-static __init struct pci_mmcfg_region *pci_mmconfig_add(int segment, int start,
-							int end, u64 addr)
+static __devinit struct pci_mmcfg_region *pci_mmconfig_alloc(int segment,
+							     int start,
+							     int end, u64 addr)
 {
 	struct pci_mmcfg_region *new;
 	struct resource *res;
@@ -79,8 +80,6 @@ static __init struct pci_mmcfg_region *p
 	new->start_bus = start;
 	new->end_bus = end;
 
-	list_add_sorted(new);
-
 	res = &new->res;
 	res->start = addr + PCI_MMCFG_BUS_OFFSET(start);
 	res->end = addr + PCI_MMCFG_BUS_OFFSET(end + 1) - 1;
@@ -96,6 +95,18 @@ static __init struct pci_mmcfg_region *p
 	return new;
 }
 
+static __init struct pci_mmcfg_region *pci_mmconfig_add(int segment, int start,
+							int end, u64 addr)
+{
+	struct pci_mmcfg_region *new;
+
+	new = pci_mmconfig_alloc(segment, start, end, addr);
+	if (new)
+		list_add_sorted(new);
+
+	return new;
+}
+
 struct pci_mmcfg_region *pci_mmconfig_lookup(int segment, int bus)
 {
 	struct pci_mmcfg_region *cfg;

[PATCH 3/6] PCI, x86: use RCU list to protect mmconfig list

[Taku Izumi]

Use RCU list to protect mmconfig list from dynamic change
when supporting PCI host bridge hotplug.

Signed-off-by: Jiang Liu <jiang.liu@huawei.com>
---
 arch/x86/pci/mmconfig-shared.c |   11 ++++++-----
 arch/x86/pci/mmconfig_32.c     |   13 +++++++++++--
 arch/x86/pci/mmconfig_64.c     |   13 +++++++++++--
 3 files changed, 28 insertions(+), 9 deletions(-)

diff --git a/arch/x86/pci/mmconfig-shared.c b/arch/x86/pci/mmconfig-shared.c
index 5e2cd2a..3bcc361 100644
--- a/arch/x86/pci/mmconfig-shared.c
+++ b/arch/x86/pci/mmconfig-shared.c
@@ -17,6 +17,7 @@
 #include <linux/bitmap.h>
 #include <linux/dmi.h>
 #include <linux/slab.h>
+#include <linux/rculist.h>
 #include <asm/e820.h>
 #include <asm/pci_x86.h>
 #include <asm/acpi.h>
@@ -45,20 +46,20 @@ static __init void free_all_mmcfg(void)
 		pci_mmconfig_remove(cfg);
 }
 
-static __init void list_add_sorted(struct pci_mmcfg_region *new)
+static __devinit void list_add_sorted(struct pci_mmcfg_region *new)
 {
 	struct pci_mmcfg_region *cfg;
 
 	/* keep list sorted by segment and starting bus number */
-	list_for_each_entry(cfg, &pci_mmcfg_list, list) {
+	list_for_each_entry_rcu(cfg, &pci_mmcfg_list, list) {
 		if (cfg->segment > new->segment ||
 		    (cfg->segment == new->segment &&
 		     cfg->start_bus >= new->start_bus)) {
-			list_add_tail(&new->list, &cfg->list);
+			list_add_tail_rcu(&new->list, &cfg->list);
 			return;
 		}
 	}
-	list_add_tail(&new->list, &pci_mmcfg_list);
+	list_add_tail_rcu(&new->list, &pci_mmcfg_list);
 }
 
 static __devinit struct pci_mmcfg_region *pci_mmconfig_alloc(int segment,
@@ -111,7 +112,7 @@ struct pci_mmcfg_region *pci_mmconfig_lookup(int segment, int bus)
 {
 	struct pci_mmcfg_region *cfg;
 
-	list_for_each_entry(cfg, &pci_mmcfg_list, list)
+	list_for_each_entry_rcu(cfg, &pci_mmcfg_list, list)
 		if (cfg->segment == segment &&
 		    cfg->start_bus <= bus && bus <= cfg->end_bus)
 			return cfg;
diff --git a/arch/x86/pci/mmconfig_32.c b/arch/x86/pci/mmconfig_32.c
index 5372e86..5dad04a 100644
--- a/arch/x86/pci/mmconfig_32.c
+++ b/arch/x86/pci/mmconfig_32.c
@@ -11,6 +11,7 @@
 
 #include <linux/pci.h>
 #include <linux/init.h>
+#include <linux/rcupdate.h>
 #include <asm/e820.h>
 #include <asm/pci_x86.h>
 #include <acpi/acpi.h>
@@ -60,9 +61,12 @@ err:		*value = -1;
 		return -EINVAL;
 	}
 
+	rcu_read_lock();
 	base = get_base_addr(seg, bus, devfn);
-	if (!base)
+	if (!base) {
+		rcu_read_unlock();
 		goto err;
+	}
 
 	raw_spin_lock_irqsave(&pci_config_lock, flags);
 
@@ -80,6 +84,7 @@ err:		*value = -1;
 		break;
 	}
 	raw_spin_unlock_irqrestore(&pci_config_lock, flags);
+	rcu_read_unlock();
 
 	return 0;
 }
@@ -93,9 +98,12 @@ static int pci_mmcfg_write(unsigned int seg, unsigned int bus,
 	if ((bus > 255) || (devfn > 255) || (reg > 4095))
 		return -EINVAL;
 
+	rcu_read_lock();
 	base = get_base_addr(seg, bus, devfn);
-	if (!base)
+	if (!base) {
+		rcu_read_unlock();
 		return -EINVAL;
+	}
 
 	raw_spin_lock_irqsave(&pci_config_lock, flags);
 
@@ -113,6 +121,7 @@ static int pci_mmcfg_write(unsigned int seg, unsigned int bus,
 		break;
 	}
 	raw_spin_unlock_irqrestore(&pci_config_lock, flags);
+	rcu_read_unlock();
 
 	return 0;
 }
diff --git a/arch/x86/pci/mmconfig_64.c b/arch/x86/pci/mmconfig_64.c
index 915a493..acc48c5 100644
--- a/arch/x86/pci/mmconfig_64.c
+++ b/arch/x86/pci/mmconfig_64.c
@@ -9,6 +9,7 @@
 #include <linux/init.h>
 #include <linux/acpi.h>
 #include <linux/bitmap.h>
+#include <linux/rcupdate.h>
 #include <asm/e820.h>
 #include <asm/pci_x86.h>
 
@@ -34,9 +35,12 @@ err:		*value = -1;
 		return -EINVAL;
 	}
 
+	rcu_read_lock();
 	addr = pci_dev_base(seg, bus, devfn);
-	if (!addr)
+	if (!addr) {
+		rcu_read_unlock();
 		goto err;
+	}
 
 	switch (len) {
 	case 1:
@@ -49,6 +53,7 @@ err:		*value = -1;
 		*value = mmio_config_readl(addr + reg);
 		break;
 	}
+	rcu_read_unlock();
 
 	return 0;
 }
@@ -62,9 +67,12 @@ static int pci_mmcfg_write(unsigned int seg, unsigned int bus,
 	if (unlikely((bus > 255) || (devfn > 255) || (reg > 4095)))
 		return -EINVAL;
 
+	rcu_read_lock();
 	addr = pci_dev_base(seg, bus, devfn);
-	if (!addr)
+	if (!addr) {
+		rcu_read_unlock();
 		return -EINVAL;
+	}
 
 	switch (len) {
 	case 1:
@@ -77,6 +85,7 @@ static int pci_mmcfg_write(unsigned int seg, unsigned int bus,
 		mmio_config_writel(addr + reg, value);
 		break;
 	}
+	rcu_read_unlock();
 
 	return 0;
 }
-- 
1.7.5.4

--
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

[PATCH 4/6] PCI, x86: introduce pci_mmcfg_arch_map()/pci_mmcfg_arch_unmap()

[Taku Izumi]

Introduce pci_mmcfg_arch_map()/pci_mmcfg_arch_unmap(), which will be used
when supporting PCI root bridge hotplug.

Signed-off-by: Jiang Liu <jiang.liu@huawei.com>
---
 arch/x86/include/asm/pci_x86.h |    2 ++
 arch/x86/pci/mmconfig_32.c     |   15 +++++++++++++++
 arch/x86/pci/mmconfig_64.c     |   22 +++++++++++++++++++++-
 3 files changed, 38 insertions(+), 1 deletions(-)

diff --git a/arch/x86/include/asm/pci_x86.h b/arch/x86/include/asm/pci_x86.h
index b3a5317..df898ce 100644
--- a/arch/x86/include/asm/pci_x86.h
+++ b/arch/x86/include/asm/pci_x86.h
@@ -135,6 +135,8 @@ struct pci_mmcfg_region {
 
 extern int __init pci_mmcfg_arch_init(void);
 extern void __init pci_mmcfg_arch_free(void);
+extern int __devinit pci_mmcfg_arch_map(struct pci_mmcfg_region *cfg);
+extern void pci_mmcfg_arch_unmap(struct pci_mmcfg_region *cfg);
 extern struct pci_mmcfg_region *pci_mmconfig_lookup(int segment, int bus);
 
 extern struct list_head pci_mmcfg_list;
diff --git a/arch/x86/pci/mmconfig_32.c b/arch/x86/pci/mmconfig_32.c
index 5dad04a..a22785d 100644
--- a/arch/x86/pci/mmconfig_32.c
+++ b/arch/x86/pci/mmconfig_32.c
@@ -141,3 +141,18 @@ int __init pci_mmcfg_arch_init(void)
 void __init pci_mmcfg_arch_free(void)
 {
 }
+
+int __devinit pci_mmcfg_arch_map(struct pci_mmcfg_region *cfg)
+{
+	return 0;
+}
+
+void pci_mmcfg_arch_unmap(struct pci_mmcfg_region *cfg)
+{
+	unsigned long flags;
+
+	/* Invalidate the cached mmcfg map entry. */
+	raw_spin_lock_irqsave(&pci_config_lock, flags);
+	mmcfg_last_accessed_device = 0;
+	raw_spin_unlock_irqrestore(&pci_config_lock, flags);
+}
diff --git a/arch/x86/pci/mmconfig_64.c b/arch/x86/pci/mmconfig_64.c
index acc48c5..4e05779 100644
--- a/arch/x86/pci/mmconfig_64.c
+++ b/arch/x86/pci/mmconfig_64.c
@@ -95,7 +95,7 @@ static const struct pci_raw_ops pci_mmcfg = {
 	.write =	pci_mmcfg_write,
 };
 
-static void __iomem * __init mcfg_ioremap(struct pci_mmcfg_region *cfg)
+static void __iomem * __devinit mcfg_ioremap(struct pci_mmcfg_region *cfg)
 {
 	void __iomem *addr;
 	u64 start, size;
@@ -138,3 +138,23 @@ void __init pci_mmcfg_arch_free(void)
 		}
 	}
 }
+
+int __devinit pci_mmcfg_arch_map(struct pci_mmcfg_region *cfg)
+{
+	cfg->virt = mcfg_ioremap(cfg);
+	if (!cfg->virt) {
+		printk(KERN_ERR PREFIX "can't map MMCONFIG at %pR\n",
+		       &cfg->res);
+		return -ENOMEM;
+	}
+
+	return 0;
+}
+
+void pci_mmcfg_arch_unmap(struct pci_mmcfg_region *cfg)
+{
+	if (cfg && cfg->virt) {
+		iounmap(cfg->virt + PCI_MMCFG_BUS_OFFSET(cfg->start_bus));
+		cfg->virt = NULL;
+	}
+}
-- 
1.7.5.4

[PATCH 5/6] PCI, x86: introduce pci_mmcongi_add() / pci_mmconfig_delete()

[Taku Izumi]

This patch replaces pci_mmconfig_add() and introduces pci_mmconfig_delte()
for adding or deleting MCFG region.

The newly pci_mmconfig_add() does the following:
  - region check
  - insert_resource
  - ioremap
  - add new pci_mmcfg_region entry to pci_mmcfg_list

Signed-off-by: Taku Izumi <izumi.taku@jp.fujitsu.com>
---
 arch/x86/include/asm/pci_x86.h |    3 
 arch/x86/pci/mmconfig-shared.c |  142 ++++++++++++++++++++++++++++++-----------
 arch/x86/pci/mmconfig_64.c     |   11 ---
 3 files changed, 109 insertions(+), 47 deletions(-)

Index: bjorn-next/arch/x86/include/asm/pci_x86.h
===================================================================
--- bjorn-next.orig/arch/x86/include/asm/pci_x86.h
+++ bjorn-next/arch/x86/include/asm/pci_x86.h
@@ -138,7 +138,8 @@ extern void __init pci_mmcfg_arch_free(v
 extern int __devinit pci_mmcfg_arch_map(struct pci_mmcfg_region *cfg);
 extern void pci_mmcfg_arch_unmap(struct pci_mmcfg_region *cfg);
 extern struct pci_mmcfg_region *pci_mmconfig_lookup(int segment, int bus);
-
+extern int __devinit pci_mmconfig_add(int seg, int start, int end, u64 addr);
+extern int pci_mmconfig_delete(int seg, int start, int end);
 extern struct list_head pci_mmcfg_list;
 
 #define PCI_MMCFG_BUS_OFFSET(bus)      ((bus) << 20)
Index: bjorn-next/arch/x86/pci/mmconfig-shared.c
===================================================================
--- bjorn-next.orig/arch/x86/pci/mmconfig-shared.c
+++ bjorn-next/arch/x86/pci/mmconfig-shared.c
@@ -17,6 +17,7 @@
 #include <linux/bitmap.h>
 #include <linux/dmi.h>
 #include <linux/slab.h>
+#include <linux/mutex.h>
 #include <linux/rculist.h>
 #include <asm/e820.h>
 #include <asm/pci_x86.h>
@@ -96,18 +97,6 @@ static __devinit struct pci_mmcfg_region
 	return new;
 }
 
-static __init struct pci_mmcfg_region *pci_mmconfig_add(int segment, int start,
-							int end, u64 addr)
-{
-	struct pci_mmcfg_region *new;
-
-	new = pci_mmconfig_alloc(segment, start, end, addr);
-	if (new)
-		list_add_sorted(new);
-
-	return new;
-}
-
 struct pci_mmcfg_region *pci_mmconfig_lookup(int segment, int bus)
 {
 	struct pci_mmcfg_region *cfg;
@@ -129,7 +118,7 @@ static const char __init *pci_mmcfg_e752
 	if (win == 0x0000 || win == 0xf000)
 		return NULL;
 
-	if (pci_mmconfig_add(0, 0, 255, win << 16) == NULL)
+	if (pci_mmconfig_add(0, 0, 255, win << 16))
 		return NULL;
 
 	return "Intel Corporation E7520 Memory Controller Hub";
@@ -173,7 +162,7 @@ static const char __init *pci_mmcfg_inte
 	if ((pciexbar & mask) >= 0xf0000000U)
 		return NULL;
 
-	if (pci_mmconfig_add(0, 0, (len >> 20) - 1, pciexbar & mask) == NULL)
+	if (pci_mmconfig_add(0, 0, (len >> 20) - 1, pciexbar & mask))
 		return NULL;
 
 	return "Intel Corporation 945G/GZ/P/PL Express Memory Controller Hub";
@@ -221,7 +210,7 @@ static const char __init *pci_mmcfg_amd_
 	end_bus = (1 << busnbits) - 1;
 	for (i = 0; i < (1 << segnbits); i++)
 		if (pci_mmconfig_add(i, 0, end_bus,
-				     base + (1<<28) * i) == NULL) {
+				     base + (1<<28) * i)) {
 			free_all_mmcfg();
 			return NULL;
 		}
@@ -278,7 +267,7 @@ static const char __init *pci_mmcfg_nvid
 		base <<= extcfg_base_lshift;
 		start = (extcfg & extcfg_start_mask) >> extcfg_start_shift;
 		end = start + extcfg_sizebus[size_index] - 1;
-		if (pci_mmconfig_add(0, start, end, base) == NULL)
+		if (pci_mmconfig_add(0, start, end, base))
 			continue;
 		mcp55_mmconf_found++;
 	}
@@ -376,7 +365,7 @@ static void __init pci_mmcfg_insert_reso
 	pci_mmcfg_resources_inserted = 1;
 }
 
-static acpi_status __init check_mcfg_resource(struct acpi_resource *res,
+static acpi_status __devinit check_mcfg_resource(struct acpi_resource *res,
 					      void *data)
 {
 	struct resource *mcfg_res = data;
@@ -413,7 +402,7 @@ static acpi_status __init check_mcfg_res
 	return AE_OK;
 }
 
-static acpi_status __init find_mboard_resource(acpi_handle handle, u32 lvl,
+static acpi_status __devinit find_mboard_resource(acpi_handle handle, u32 lvl,
 		void *context, void **rv)
 {
 	struct resource *mcfg_res = context;
@@ -427,7 +416,7 @@ static acpi_status __init find_mboard_re
 	return AE_OK;
 }
 
-static int __init is_acpi_reserved(u64 start, u64 end, unsigned not_used)
+static int __devinit is_acpi_reserved(u64 start, u64 end, unsigned not_used)
 {
 	struct resource mcfg_res;
 
@@ -446,7 +435,7 @@ static int __init is_acpi_reserved(u64 s
 
 typedef int (*check_reserved_t)(u64 start, u64 end, unsigned type);
 
-static int __init is_mmconf_reserved(check_reserved_t is_reserved,
+static int __devinit is_mmconf_reserved(check_reserved_t is_reserved,
 				    struct pci_mmcfg_region *cfg, int with_e820)
 {
 	u64 addr = cfg->res.start;
@@ -507,19 +496,6 @@ static int __devinit pci_mmcfg_check_res
 	return false;
 }
 
-static void __init pci_mmcfg_reject_broken(int early)
-{
-	struct pci_mmcfg_region *cfg;
-
-	list_for_each_entry(cfg, &pci_mmcfg_list, list) {
-		if (pci_mmcfg_check_reserved(cfg, early) == false) {
-			printk(KERN_INFO PREFIX "not using MMCONFIG\n");
-			free_all_mmcfg();
-			return;
-		}
-	}
-}
-
 static int __initdata known_bridge;
 
 static int __init acpi_mcfg_check_entry(struct acpi_table_mcfg *mcfg,
@@ -613,8 +589,6 @@ static void __init __pci_mmcfg_init(int 
 	if (!known_bridge)
 		acpi_sfi_table_parse(ACPI_SIG_MCFG, pci_parse_mcfg);
 
-	pci_mmcfg_reject_broken(early);
-
 	if (list_empty(&pci_mmcfg_list))
 		return;
 
@@ -676,3 +650,101 @@ static int __init pci_mmcfg_late_insert_
  * with other system resources.
  */
 late_initcall(pci_mmcfg_late_insert_resources);
+
+static DEFINE_MUTEX(pci_mmcfg_lock);
+
+int __devinit pci_mmconfig_add(int segment, int start, int end, u64 addr)
+{
+	int rc = 0;
+	struct pci_mmcfg_region *cfg, *new = NULL;
+
+	if (segment < 0 || segment > USHRT_MAX ||
+	    start < 0 || start > 255 || end < start || end > 255)
+		return -EINVAL;
+
+	mutex_lock(&pci_mmcfg_lock);
+	cfg = pci_mmconfig_lookup(segment, start);
+	if (cfg) {
+		if (cfg->start_bus <= start && cfg->end_bus >= end) {
+			rc = -EEXIST;
+		} else {
+			rc = -EINVAL;
+			printk(KERN_WARNING PREFIX
+			       "MMCONFIG for domain %04x [bus %02x-%02x] "
+			       "conflicts with domain %04x [bus %02x-%02x]\n",
+			       segment, start, end,
+			       cfg->segment, cfg->start_bus, cfg->end_bus);
+		}
+		goto out;
+	}
+
+	new = pci_mmconfig_alloc(segment, start, end, addr);
+	if (new == NULL) {
+		rc = -ENOMEM;
+		goto out;
+	}
+
+	if (!pci_mmcfg_check_reserved(new, 0)) {
+		rc = -EINVAL;
+		printk(KERN_WARNING PREFIX
+		       "MMCONFIG for domain %04x [bus %02x-%02x] "
+		       "isn't reserved\n", segment, start, end);
+		goto out;
+	}
+
+	if (pci_mmcfg_resources_inserted &&
+	    insert_resource(&iomem_resource, &new->res)) {
+		rc = -EBUSY;
+		printk(KERN_WARNING PREFIX
+		       "failed to insert resource for domain "
+		       "%04x [bus %02x-%02x]\n", segment, start, end);
+		goto out;
+	}
+
+	if (pci_mmcfg_arch_map(new)) {
+		rc = -EBUSY;
+		printk(KERN_WARNING PREFIX
+		       "failed to map resource for domain "
+		       "%04x [bus %02x-%02x]\n", segment, start, end);
+		goto out;
+	}
+
+	list_add_sorted(new);
+	new = NULL;
+
+out:
+	if (new) {
+		if (new->res.parent)
+			release_resource(&new->res);
+		kfree(new);
+	}
+
+	mutex_unlock(&pci_mmcfg_lock);
+
+	return rc;
+}
+
+/* Delete MMCFG information at runtime */
+int pci_mmconfig_delete(int segment, int start, int end)
+{
+	struct pci_mmcfg_region *cfg;
+
+	mutex_lock(&pci_mmcfg_lock);
+	list_for_each_entry_rcu(cfg, &pci_mmcfg_list, list) {
+		if (cfg->segment == segment && cfg->start_bus == start &&
+		    cfg->end_bus == end) {
+			list_del_rcu(&cfg->list);
+			synchronize_rcu();
+			pci_mmcfg_arch_unmap(cfg);
+			if (cfg->res.parent)
+				release_resource(&cfg->res);
+			mutex_unlock(&pci_mmcfg_lock);
+			kfree(cfg);
+			return 0;
+		}
+	}
+	mutex_unlock(&pci_mmcfg_lock);
+
+	return -ENOENT;
+}
+
Index: bjorn-next/arch/x86/pci/mmconfig_64.c
===================================================================
--- bjorn-next.orig/arch/x86/pci/mmconfig_64.c
+++ bjorn-next/arch/x86/pci/mmconfig_64.c
@@ -112,17 +112,6 @@ static void __iomem * __devinit mcfg_ior
 
 int __init pci_mmcfg_arch_init(void)
 {
-	struct pci_mmcfg_region *cfg;
-
-	list_for_each_entry(cfg, &pci_mmcfg_list, list) {
-		cfg->virt = mcfg_ioremap(cfg);
-		if (!cfg->virt) {
-			printk(KERN_ERR PREFIX "can't map MMCONFIG at %pR\n",
-			       &cfg->res);
-			pci_mmcfg_arch_free();
-			return 0;
-		}
-	}
 	raw_pci_ext_ops = &pci_mmcfg;
 	return 1;
 }

Re: [PATCH 5/6] PCI, x86: introduce pci_mmcongi_add() / pci_mmconfig_delete()

[Jiang Liu]

On 04/26/2012 06:42 PM, Taku Izumi wrote:
> 
> This patch replaces pci_mmconfig_add() and introduces pci_mmconfig_delte()
> for adding or deleting MCFG region.
> 
> The newly pci_mmconfig_add() does the following:
>   - region check
>   - insert_resource
>   - ioremap
>   - add new pci_mmcfg_region entry to pci_mmcfg_list
> 
> Signed-off-by: Taku Izumi <izumi.taku@jp.fujitsu.com>
> ---
>  arch/x86/include/asm/pci_x86.h |    3 
>  arch/x86/pci/mmconfig-shared.c |  142 ++++++++++++++++++++++++++++++-----------
>  arch/x86/pci/mmconfig_64.c     |   11 ---
>  3 files changed, 109 insertions(+), 47 deletions(-)
> 
> Index: bjorn-next/arch/x86/include/asm/pci_x86.h
> ===================================================================
> --- bjorn-next.orig/arch/x86/include/asm/pci_x86.h
> +++ bjorn-next/arch/x86/include/asm/pci_x86.h
> @@ -138,7 +138,8 @@ extern void __init pci_mmcfg_arch_free(v
>  extern int __devinit pci_mmcfg_arch_map(struct pci_mmcfg_region *cfg);
>  extern void pci_mmcfg_arch_unmap(struct pci_mmcfg_region *cfg);
>  extern struct pci_mmcfg_region *pci_mmconfig_lookup(int segment, int bus);
> -
> +extern int __devinit pci_mmconfig_add(int seg, int start, int end, u64 addr);
> +extern int pci_mmconfig_delete(int seg, int start, int end);
>  extern struct list_head pci_mmcfg_list;
>  
>  #define PCI_MMCFG_BUS_OFFSET(bus)      ((bus) << 20)
> Index: bjorn-next/arch/x86/pci/mmconfig-shared.c
> ===================================================================
> --- bjorn-next.orig/arch/x86/pci/mmconfig-shared.c
> +++ bjorn-next/arch/x86/pci/mmconfig-shared.c
> @@ -17,6 +17,7 @@
>  #include <linux/bitmap.h>
>  #include <linux/dmi.h>
>  #include <linux/slab.h>
> +#include <linux/mutex.h>
>  #include <linux/rculist.h>
>  #include <asm/e820.h>
>  #include <asm/pci_x86.h>
> @@ -96,18 +97,6 @@ static __devinit struct pci_mmcfg_region
>  	return new;
>  }
>  
> -static __init struct pci_mmcfg_region *pci_mmconfig_add(int segment, int start,
> -							int end, u64 addr)
> -{
> -	struct pci_mmcfg_region *new;
> -
> -	new = pci_mmconfig_alloc(segment, start, end, addr);
> -	if (new)
> -		list_add_sorted(new);
> -
> -	return new;
> -}
> -
>  struct pci_mmcfg_region *pci_mmconfig_lookup(int segment, int bus)
>  {
>  	struct pci_mmcfg_region *cfg;
> @@ -129,7 +118,7 @@ static const char __init *pci_mmcfg_e752
>  	if (win == 0x0000 || win == 0xf000)
>  		return NULL;
>  
> -	if (pci_mmconfig_add(0, 0, 255, win << 16) == NULL)
> +	if (pci_mmconfig_add(0, 0, 255, win << 16))
>  		return NULL;
>  
>  	return "Intel Corporation E7520 Memory Controller Hub";
> @@ -173,7 +162,7 @@ static const char __init *pci_mmcfg_inte
>  	if ((pciexbar & mask) >= 0xf0000000U)
>  		return NULL;
>  
> -	if (pci_mmconfig_add(0, 0, (len >> 20) - 1, pciexbar & mask) == NULL)
> +	if (pci_mmconfig_add(0, 0, (len >> 20) - 1, pciexbar & mask))
>  		return NULL;
>  
>  	return "Intel Corporation 945G/GZ/P/PL Express Memory Controller Hub";
> @@ -221,7 +210,7 @@ static const char __init *pci_mmcfg_amd_
>  	end_bus = (1 << busnbits) - 1;
>  	for (i = 0; i < (1 << segnbits); i++)
>  		if (pci_mmconfig_add(i, 0, end_bus,
> -				     base + (1<<28) * i) == NULL) {
> +				     base + (1<<28) * i)) {
>  			free_all_mmcfg();
>  			return NULL;
>  		}
> @@ -278,7 +267,7 @@ static const char __init *pci_mmcfg_nvid
>  		base <<= extcfg_base_lshift;
>  		start = (extcfg & extcfg_start_mask) >> extcfg_start_shift;
>  		end = start + extcfg_sizebus[size_index] - 1;
> -		if (pci_mmconfig_add(0, start, end, base) == NULL)
> +		if (pci_mmconfig_add(0, start, end, base))
>  			continue;
>  		mcp55_mmconf_found++;
>  	}
> @@ -376,7 +365,7 @@ static void __init pci_mmcfg_insert_reso
>  	pci_mmcfg_resources_inserted = 1;
>  }
>  
> -static acpi_status __init check_mcfg_resource(struct acpi_resource *res,
> +static acpi_status __devinit check_mcfg_resource(struct acpi_resource *res,
>  					      void *data)
Please pay attention to indent for above line.

>  {
>  	struct resource *mcfg_res = data;
> @@ -413,7 +402,7 @@ static acpi_status __init check_mcfg_res
>  	return AE_OK;
>  }
>  
> -static acpi_status __init find_mboard_resource(acpi_handle handle, u32 lvl,
> +static acpi_status __devinit find_mboard_resource(acpi_handle handle, u32 lvl,
>  		void *context, void **rv)
>  {
>  	struct resource *mcfg_res = context;
> @@ -427,7 +416,7 @@ static acpi_status __init find_mboard_re
>  	return AE_OK;
>  }
>  
> -static int __init is_acpi_reserved(u64 start, u64 end, unsigned not_used)
> +static int __devinit is_acpi_reserved(u64 start, u64 end, unsigned not_used)
>  {
>  	struct resource mcfg_res;
>  
> @@ -446,7 +435,7 @@ static int __init is_acpi_reserved(u64 s
>  
>  typedef int (*check_reserved_t)(u64 start, u64 end, unsigned type);
>  
> -static int __init is_mmconf_reserved(check_reserved_t is_reserved,
> +static int __devinit is_mmconf_reserved(check_reserved_t is_reserved,
>  				    struct pci_mmcfg_region *cfg, int with_e820)
Please pay attention to indent for above line.

>  {
>  	u64 addr = cfg->res.start;
> @@ -507,19 +496,6 @@ static int __devinit pci_mmcfg_check_res
>  	return false;
>  }
>  
> -static void __init pci_mmcfg_reject_broken(int early)
> -{
> -	struct pci_mmcfg_region *cfg;
> -
> -	list_for_each_entry(cfg, &pci_mmcfg_list, list) {
> -		if (pci_mmcfg_check_reserved(cfg, early) == false) {
> -			printk(KERN_INFO PREFIX "not using MMCONFIG\n");
> -			free_all_mmcfg();
> -			return;
> -		}
> -	}
> -}
> -
pci_mmcfg_check_reserved() performs different checks according to the parameter 'early'.
The patch has changed the behavior of original code. 

At early time when mmcfg_early_init() is called, the ACPICA subsystem is still
uninitialized yet and is_acpi_reserved() shouldn't be called yet. So at early
stage, pci_mmcfg_check_reserved() should be called with parameter early as 1(true).
But this patch always call pci_mmcfg_check_reserved() with parameter as 0.

So suggest to keep pci_mmconfig_add() and pci_mmcfg_reject_broken(), but change return
value of pci_mmconfig_add() from "struct pci_mmcfg_region *" to "int".

>  static int __initdata known_bridge;
>  
>  static int __init acpi_mcfg_check_entry(struct acpi_table_mcfg *mcfg,
> @@ -613,8 +589,6 @@ static void __init __pci_mmcfg_init(int 
>  	if (!known_bridge)
>  		acpi_sfi_table_parse(ACPI_SIG_MCFG, pci_parse_mcfg);
>  
> -	pci_mmcfg_reject_broken(early);
> -
>  	if (list_empty(&pci_mmcfg_list))
>  		return;
>  
> @@ -676,3 +650,101 @@ static int __init pci_mmcfg_late_insert_
>   * with other system resources.
>   */
>  late_initcall(pci_mmcfg_late_insert_resources);
> +
> +static DEFINE_MUTEX(pci_mmcfg_lock);
> +
> +int __devinit pci_mmconfig_add(int segment, int start, int end, u64 addr)
> +{
> +	int rc = 0;
> +	struct pci_mmcfg_region *cfg, *new = NULL;
> +
> +	if (segment < 0 || segment > USHRT_MAX ||
> +	    start < 0 || start > 255 || end < start || end > 255)
> +		return -EINVAL;
> +
> +	mutex_lock(&pci_mmcfg_lock);
> +	cfg = pci_mmconfig_lookup(segment, start);
> +	if (cfg) {
> +		if (cfg->start_bus <= start && cfg->end_bus >= end) {
> +			rc = -EEXIST;
> +		} else {
> +			rc = -EINVAL;
> +			printk(KERN_WARNING PREFIX
> +			       "MMCONFIG for domain %04x [bus %02x-%02x] "
> +			       "conflicts with domain %04x [bus %02x-%02x]\n",
> +			       segment, start, end,
> +			       cfg->segment, cfg->start_bus, cfg->end_bus);
> +		}
> +		goto out;
> +	}
> +
> +	new = pci_mmconfig_alloc(segment, start, end, addr);
> +	if (new == NULL) {
> +		rc = -ENOMEM;
> +		goto out;
> +	}
> +
> +	if (!pci_mmcfg_check_reserved(new, 0)) {
> +		rc = -EINVAL;
> +		printk(KERN_WARNING PREFIX
> +		       "MMCONFIG for domain %04x [bus %02x-%02x] "
> +		       "isn't reserved\n", segment, start, end);
> +		goto out;
> +	}
> +
> +	if (pci_mmcfg_resources_inserted &&
> +	    insert_resource(&iomem_resource, &new->res)) {
> +		rc = -EBUSY;
> +		printk(KERN_WARNING PREFIX
> +		       "failed to insert resource for domain "
> +		       "%04x [bus %02x-%02x]\n", segment, start, end);
> +		goto out;
> +	}
> +
> +	if (pci_mmcfg_arch_map(new)) {
> +		rc = -EBUSY;
> +		printk(KERN_WARNING PREFIX
> +		       "failed to map resource for domain "
> +		       "%04x [bus %02x-%02x]\n", segment, start, end);
> +		goto out;
> +	}
> +
> +	list_add_sorted(new);
> +	new = NULL;
> +
> +out:
> +	if (new) {
> +		if (new->res.parent)
> +			release_resource(&new->res);
> +		kfree(new);
> +	}
> +
> +	mutex_unlock(&pci_mmcfg_lock);
> +
> +	return rc;
> +}
> +
> +/* Delete MMCFG information at runtime */
> +int pci_mmconfig_delete(int segment, int start, int end)
> +{
> +	struct pci_mmcfg_region *cfg;
> +
> +	mutex_lock(&pci_mmcfg_lock);
> +	list_for_each_entry_rcu(cfg, &pci_mmcfg_list, list) {
> +		if (cfg->segment == segment && cfg->start_bus == start &&
> +		    cfg->end_bus == end) {
> +			list_del_rcu(&cfg->list);
> +			synchronize_rcu();
> +			pci_mmcfg_arch_unmap(cfg);
> +			if (cfg->res.parent)
> +				release_resource(&cfg->res);
> +			mutex_unlock(&pci_mmcfg_lock);
> +			kfree(cfg);
> +			return 0;
> +		}
> +	}
> +	mutex_unlock(&pci_mmcfg_lock);
> +
> +	return -ENOENT;
> +}
> +
> Index: bjorn-next/arch/x86/pci/mmconfig_64.c
> ===================================================================
> --- bjorn-next.orig/arch/x86/pci/mmconfig_64.c
> +++ bjorn-next/arch/x86/pci/mmconfig_64.c
> @@ -112,17 +112,6 @@ static void __iomem * __devinit mcfg_ior
>  
>  int __init pci_mmcfg_arch_init(void)
>  {
> -	struct pci_mmcfg_region *cfg;
> -
> -	list_for_each_entry(cfg, &pci_mmcfg_list, list) {
> -		cfg->virt = mcfg_ioremap(cfg);
> -		if (!cfg->virt) {
> -			printk(KERN_ERR PREFIX "can't map MMCONFIG at %pR\n",
> -			       &cfg->res);
> -			pci_mmcfg_arch_free();
> -			return 0;
> -		}
> -	}
>  	raw_pci_ext_ops = &pci_mmcfg;
>  	return 1;
>  }
> 

Re: [PATCH 5/6] PCI, x86: introduce pci_mmcongi_add() / pci_mmconfig_delete()

[Jiang Liu]

On 04/26/2012 06:42 PM, Taku Izumi wrote:
> 
> This patch replaces pci_mmconfig_add() and introduces pci_mmconfig_delte()
> for adding or deleting MCFG region.
> 
> The newly pci_mmconfig_add() does the following:
>   - region check
>   - insert_resource
>   - ioremap
>   - add new pci_mmcfg_region entry to pci_mmcfg_list
> 
> Signed-off-by: Taku Izumi <izumi.taku@jp.fujitsu.com>
> ---
>  arch/x86/include/asm/pci_x86.h |    3 
>  arch/x86/pci/mmconfig-shared.c |  142 ++++++++++++++++++++++++++++++-----------
>  arch/x86/pci/mmconfig_64.c     |   11 ---
>  3 files changed, 109 insertions(+), 47 deletions(-)
> 
> Index: bjorn-next/arch/x86/include/asm/pci_x86.h
> ===================================================================
> --- bjorn-next.orig/arch/x86/include/asm/pci_x86.h
> +++ bjorn-next/arch/x86/include/asm/pci_x86.h
> @@ -138,7 +138,8 @@ extern void __init pci_mmcfg_arch_free(v
>  extern int __devinit pci_mmcfg_arch_map(struct pci_mmcfg_region *cfg);
>  extern void pci_mmcfg_arch_unmap(struct pci_mmcfg_region *cfg);
>  extern struct pci_mmcfg_region *pci_mmconfig_lookup(int segment, int bus);
> -
> +extern int __devinit pci_mmconfig_add(int seg, int start, int end, u64 addr);
> +extern int pci_mmconfig_delete(int seg, int start, int end);
>  extern struct list_head pci_mmcfg_list;
>  
>  #define PCI_MMCFG_BUS_OFFSET(bus)      ((bus) << 20)
> Index: bjorn-next/arch/x86/pci/mmconfig-shared.c
> ===================================================================
> --- bjorn-next.orig/arch/x86/pci/mmconfig-shared.c
> +++ bjorn-next/arch/x86/pci/mmconfig-shared.c
> @@ -17,6 +17,7 @@
>  #include <linux/bitmap.h>
>  #include <linux/dmi.h>
>  #include <linux/slab.h>
> +#include <linux/mutex.h>
>  #include <linux/rculist.h>
>  #include <asm/e820.h>
>  #include <asm/pci_x86.h>
> @@ -96,18 +97,6 @@ static __devinit struct pci_mmcfg_region
>  	return new;
>  }
>  
> -static __init struct pci_mmcfg_region *pci_mmconfig_add(int segment, int start,
> -							int end, u64 addr)
> -{
> -	struct pci_mmcfg_region *new;
> -
> -	new = pci_mmconfig_alloc(segment, start, end, addr);
> -	if (new)
> -		list_add_sorted(new);
> -
> -	return new;
> -}
> -
>  struct pci_mmcfg_region *pci_mmconfig_lookup(int segment, int bus)
>  {
>  	struct pci_mmcfg_region *cfg;
> @@ -129,7 +118,7 @@ static const char __init *pci_mmcfg_e752
>  	if (win == 0x0000 || win == 0xf000)
>  		return NULL;
>  
> -	if (pci_mmconfig_add(0, 0, 255, win << 16) == NULL)
> +	if (pci_mmconfig_add(0, 0, 255, win << 16))
>  		return NULL;
>  
>  	return "Intel Corporation E7520 Memory Controller Hub";
> @@ -173,7 +162,7 @@ static const char __init *pci_mmcfg_inte
>  	if ((pciexbar & mask) >= 0xf0000000U)
>  		return NULL;
>  
> -	if (pci_mmconfig_add(0, 0, (len >> 20) - 1, pciexbar & mask) == NULL)
> +	if (pci_mmconfig_add(0, 0, (len >> 20) - 1, pciexbar & mask))
>  		return NULL;
>  
>  	return "Intel Corporation 945G/GZ/P/PL Express Memory Controller Hub";
> @@ -221,7 +210,7 @@ static const char __init *pci_mmcfg_amd_
>  	end_bus = (1 << busnbits) - 1;
>  	for (i = 0; i < (1 << segnbits); i++)
>  		if (pci_mmconfig_add(i, 0, end_bus,
> -				     base + (1<<28) * i) == NULL) {
> +				     base + (1<<28) * i)) {
>  			free_all_mmcfg();
>  			return NULL;
>  		}
> @@ -278,7 +267,7 @@ static const char __init *pci_mmcfg_nvid
>  		base <<= extcfg_base_lshift;
>  		start = (extcfg & extcfg_start_mask) >> extcfg_start_shift;
>  		end = start + extcfg_sizebus[size_index] - 1;
> -		if (pci_mmconfig_add(0, start, end, base) == NULL)
> +		if (pci_mmconfig_add(0, start, end, base))
>  			continue;
>  		mcp55_mmconf_found++;
>  	}
> @@ -376,7 +365,7 @@ static void __init pci_mmcfg_insert_reso
>  	pci_mmcfg_resources_inserted = 1;
>  }
>  
> -static acpi_status __init check_mcfg_resource(struct acpi_resource *res,
> +static acpi_status __devinit check_mcfg_resource(struct acpi_resource *res,
>  					      void *data)
>  {
>  	struct resource *mcfg_res = data;
> @@ -413,7 +402,7 @@ static acpi_status __init check_mcfg_res
>  	return AE_OK;
>  }
>  
> -static acpi_status __init find_mboard_resource(acpi_handle handle, u32 lvl,
> +static acpi_status __devinit find_mboard_resource(acpi_handle handle, u32 lvl,
>  		void *context, void **rv)
>  {
>  	struct resource *mcfg_res = context;
> @@ -427,7 +416,7 @@ static acpi_status __init find_mboard_re
>  	return AE_OK;
>  }
>  
> -static int __init is_acpi_reserved(u64 start, u64 end, unsigned not_used)
> +static int __devinit is_acpi_reserved(u64 start, u64 end, unsigned not_used)
>  {
>  	struct resource mcfg_res;
>  
> @@ -446,7 +435,7 @@ static int __init is_acpi_reserved(u64 s
>  
>  typedef int (*check_reserved_t)(u64 start, u64 end, unsigned type);
>  
> -static int __init is_mmconf_reserved(check_reserved_t is_reserved,
> +static int __devinit is_mmconf_reserved(check_reserved_t is_reserved,
>  				    struct pci_mmcfg_region *cfg, int with_e820)
>  {
>  	u64 addr = cfg->res.start;
> @@ -507,19 +496,6 @@ static int __devinit pci_mmcfg_check_res
>  	return false;
>  }
>  
> -static void __init pci_mmcfg_reject_broken(int early)
> -{
> -	struct pci_mmcfg_region *cfg;
> -
> -	list_for_each_entry(cfg, &pci_mmcfg_list, list) {
> -		if (pci_mmcfg_check_reserved(cfg, early) == false) {
> -			printk(KERN_INFO PREFIX "not using MMCONFIG\n");
> -			free_all_mmcfg();
> -			return;
> -		}
> -	}
> -}
> -
>  static int __initdata known_bridge;
>  
>  static int __init acpi_mcfg_check_entry(struct acpi_table_mcfg *mcfg,
> @@ -613,8 +589,6 @@ static void __init __pci_mmcfg_init(int 
>  	if (!known_bridge)
>  		acpi_sfi_table_parse(ACPI_SIG_MCFG, pci_parse_mcfg);
>  
> -	pci_mmcfg_reject_broken(early);
> -
This also change the original behavior too. With original implementation, mmcfg
mechanism won't be enabled if any MMCFG item is invalid. With the change, mmcfg
mechanism will be enabled if any MMCFG is valid.


>  	if (list_empty(&pci_mmcfg_list))
>  		return;
>  
> @@ -676,3 +650,101 @@ static int __init pci_mmcfg_late_insert_
>   * with other system resources.
>   */
>  late_initcall(pci_mmcfg_late_insert_resources);
> +
> +static DEFINE_MUTEX(pci_mmcfg_lock);
> +
> +int __devinit pci_mmconfig_add(int segment, int start, int end, u64 addr)
> +{
> +	int rc = 0;
> +	struct pci_mmcfg_region *cfg, *new = NULL;
> +
> +	if (segment < 0 || segment > USHRT_MAX ||
> +	    start < 0 || start > 255 || end < start || end > 255)
> +		return -EINVAL;
> +
> +	mutex_lock(&pci_mmcfg_lock);
> +	cfg = pci_mmconfig_lookup(segment, start);
> +	if (cfg) {
> +		if (cfg->start_bus <= start && cfg->end_bus >= end) {
> +			rc = -EEXIST;
> +		} else {
> +			rc = -EINVAL;
> +			printk(KERN_WARNING PREFIX
> +			       "MMCONFIG for domain %04x [bus %02x-%02x] "
> +			       "conflicts with domain %04x [bus %02x-%02x]\n",
> +			       segment, start, end,
> +			       cfg->segment, cfg->start_bus, cfg->end_bus);
> +		}
> +		goto out;
> +	}
> +
> +	new = pci_mmconfig_alloc(segment, start, end, addr);
> +	if (new == NULL) {
> +		rc = -ENOMEM;
> +		goto out;
> +	}
> +
> +	if (!pci_mmcfg_check_reserved(new, 0)) {
> +		rc = -EINVAL;
> +		printk(KERN_WARNING PREFIX
> +		       "MMCONFIG for domain %04x [bus %02x-%02x] "
> +		       "isn't reserved\n", segment, start, end);
> +		goto out;
> +	}
> +
> +	if (pci_mmcfg_resources_inserted &&
> +	    insert_resource(&iomem_resource, &new->res)) {
> +		rc = -EBUSY;
> +		printk(KERN_WARNING PREFIX
> +		       "failed to insert resource for domain "
> +		       "%04x [bus %02x-%02x]\n", segment, start, end);
> +		goto out;
> +	}
> +
> +	if (pci_mmcfg_arch_map(new)) {
> +		rc = -EBUSY;
> +		printk(KERN_WARNING PREFIX
> +		       "failed to map resource for domain "
> +		       "%04x [bus %02x-%02x]\n", segment, start, end);
> +		goto out;
> +	}
> +
> +	list_add_sorted(new);
> +	new = NULL;
> +
> +out:
> +	if (new) {
> +		if (new->res.parent)
> +			release_resource(&new->res);
> +		kfree(new);
> +	}
> +
> +	mutex_unlock(&pci_mmcfg_lock);
> +
> +	return rc;
> +}
> +
> +/* Delete MMCFG information at runtime */
> +int pci_mmconfig_delete(int segment, int start, int end)
> +{
> +	struct pci_mmcfg_region *cfg;
> +
> +	mutex_lock(&pci_mmcfg_lock);
> +	list_for_each_entry_rcu(cfg, &pci_mmcfg_list, list) {
> +		if (cfg->segment == segment && cfg->start_bus == start &&
> +		    cfg->end_bus == end) {
> +			list_del_rcu(&cfg->list);
> +			synchronize_rcu();
> +			pci_mmcfg_arch_unmap(cfg);
> +			if (cfg->res.parent)
> +				release_resource(&cfg->res);
> +			mutex_unlock(&pci_mmcfg_lock);
> +			kfree(cfg);
> +			return 0;
> +		}
> +	}
> +	mutex_unlock(&pci_mmcfg_lock);
> +
> +	return -ENOENT;
> +}
> +
> Index: bjorn-next/arch/x86/pci/mmconfig_64.c
> ===================================================================
> --- bjorn-next.orig/arch/x86/pci/mmconfig_64.c
> +++ bjorn-next/arch/x86/pci/mmconfig_64.c
> @@ -112,17 +112,6 @@ static void __iomem * __devinit mcfg_ior
>  
>  int __init pci_mmcfg_arch_init(void)
>  {
> -	struct pci_mmcfg_region *cfg;
> -
> -	list_for_each_entry(cfg, &pci_mmcfg_list, list) {
> -		cfg->virt = mcfg_ioremap(cfg);
> -		if (!cfg->virt) {
> -			printk(KERN_ERR PREFIX "can't map MMCONFIG at %pR\n",
> -			       &cfg->res);
> -			pci_mmcfg_arch_free();
> -			return 0;
> -		}
> -	}
>  	raw_pci_ext_ops = &pci_mmcfg;
>  	return 1;
>  }
> 

[PATCH 6/6] PCI, x86: change the way to add MMCFG region on x86

[Taku Izumi]

This patch changes the way to add MMCFG region on x86.
MMCFG region information provided by ACPI MCFG table 
or _CBA method will be added at acpi_pci_root_add()
of pci_root driver.

Signed-off-by: Taku Izumi <izumi.taku@jp.fujitsu.com>
---
 arch/x86/include/asm/pci_x86.h |    2 +
 arch/x86/pci/acpi.c            |   45 +++++++++++++++++++++++++++++++++++++
 arch/x86/pci/mmconfig-shared.c |   49 ++++++++++++++++++++++++++++++-----------
 arch/x86/pci/mmconfig_32.c     |    2 -
 arch/x86/pci/mmconfig_64.c     |    2 -
 drivers/acpi/pci_root.c        |   15 ++++++++++++
 include/acpi/acnames.h         |    1 
 include/linux/pci-acpi.h       |    3 ++
 8 files changed, 105 insertions(+), 14 deletions(-)

Index: bjorn-next/arch/x86/include/asm/pci_x86.h
===================================================================
--- bjorn-next.orig/arch/x86/include/asm/pci_x86.h
+++ bjorn-next/arch/x86/include/asm/pci_x86.h
@@ -100,6 +100,7 @@ struct pci_raw_ops {
 extern const struct pci_raw_ops *raw_pci_ops;
 extern const struct pci_raw_ops *raw_pci_ext_ops;
 
+extern const struct pci_raw_ops pci_mmcfg;
 extern const struct pci_raw_ops pci_direct_conf1;
 extern bool port_cf9_safe;
 
@@ -140,6 +141,7 @@ extern void pci_mmcfg_arch_unmap(struct 
 extern struct pci_mmcfg_region *pci_mmconfig_lookup(int segment, int bus);
 extern int __devinit pci_mmconfig_add(int seg, int start, int end, u64 addr);
 extern int pci_mmconfig_delete(int seg, int start, int end);
+extern u64 query_acpi_mcfg_table(int segment, int start);
 extern struct list_head pci_mmcfg_list;
 
 #define PCI_MMCFG_BUS_OFFSET(bus)      ((bus) << 20)
Index: bjorn-next/arch/x86/pci/acpi.c
===================================================================
--- bjorn-next.orig/arch/x86/pci/acpi.c
+++ bjorn-next/arch/x86/pci/acpi.c
@@ -4,6 +4,7 @@
 #include <linux/irq.h>
 #include <linux/dmi.h>
 #include <linux/slab.h>
+#include <linux/pci-acpi.h>
 #include <asm/numa.h>
 #include <asm/pci_x86.h>
 
@@ -461,6 +462,50 @@ struct pci_bus * __devinit pci_acpi_scan
 	return bus;
 }
 
+
+int __devinit arch_acpi_pci_root_add(struct acpi_pci_root *root)
+{
+	int result = 0;
+	acpi_status status;
+	unsigned long long base_addr = 0;
+
+	/* MMCFG not in use */
+	if (raw_pci_ext_ops && raw_pci_ext_ops != &pci_mmcfg)
+		return result;
+
+	/* Evaluate _CBA */
+	status = acpi_evaluate_integer(root->device->handle, METHOD_NAME__CBA,
+				       NULL, &base_addr);
+	if (ACPI_FAILURE(status)) {
+		/* search MCFG table */
+		base_addr = query_acpi_mcfg_table(root->segment,
+						  root->secondary.start);
+
+	}
+
+	/* add MMCFG region */
+	if (base_addr) {
+		if (pci_mmconfig_add(root->segment, root->secondary.start,
+					root->secondary.end, base_addr)) {
+			printk(KERN_ERR
+				"can't add MMCFG information for Bus "
+				"%04x:%02x\n",
+				root->segment,
+				(unsigned int)root->secondary.start);
+		}
+	} else {
+		result = -ENODEV;
+	}
+
+	return result;
+}
+
+void arch_acpi_pci_root_remove(struct acpi_pci_root *root)
+{
+	pci_mmconfig_delete(root->segment, root->secondary.start,
+				root->secondary.end);
+}
+
 int __init pci_acpi_init(void)
 {
 	struct pci_dev *dev = NULL;
Index: bjorn-next/arch/x86/pci/mmconfig_32.c
===================================================================
--- bjorn-next.orig/arch/x86/pci/mmconfig_32.c
+++ bjorn-next/arch/x86/pci/mmconfig_32.c
@@ -126,7 +126,7 @@ static int pci_mmcfg_write(unsigned int 
 	return 0;
 }
 
-static const struct pci_raw_ops pci_mmcfg = {
+const struct pci_raw_ops pci_mmcfg = {
 	.read =		pci_mmcfg_read,
 	.write =	pci_mmcfg_write,
 };
Index: bjorn-next/arch/x86/pci/mmconfig_64.c
===================================================================
--- bjorn-next.orig/arch/x86/pci/mmconfig_64.c
+++ bjorn-next/arch/x86/pci/mmconfig_64.c
@@ -90,7 +90,7 @@ static int pci_mmcfg_write(unsigned int 
 	return 0;
 }
 
-static const struct pci_raw_ops pci_mmcfg = {
+const struct pci_raw_ops pci_mmcfg = {
 	.read =		pci_mmcfg_read,
 	.write =	pci_mmcfg_write,
 };
Index: bjorn-next/drivers/acpi/pci_root.c
===================================================================
--- bjorn-next.orig/drivers/acpi/pci_root.c
+++ bjorn-next/drivers/acpi/pci_root.c
@@ -449,6 +449,15 @@ out:
 }
 EXPORT_SYMBOL(acpi_pci_osc_control_set);
 
+int __weak arch_acpi_pci_root_add(struct acpi_pci_root *root)
+{
+	return 0;
+}
+
+void __weak arch_acpi_pci_root_remove(struct acpi_pci_root *root)
+{
+}
+
 static int __devinit acpi_pci_root_add(struct acpi_device *device)
 {
 	unsigned long long segment, bus;
@@ -505,6 +514,11 @@ static int __devinit acpi_pci_root_add(s
 	strcpy(acpi_device_class(device), ACPI_PCI_ROOT_CLASS);
 	device->driver_data = root;
 
+	if (arch_acpi_pci_root_add(root)) {
+		result = -ENODEV;
+		goto end;
+	}
+
 	/*
 	 * All supported architectures that use ACPI have support for
 	 * PCI domains, so we indicate this in _OSC support capabilities.
@@ -627,6 +641,7 @@ static int __devinit acpi_pci_root_add(s
 end:
 	if (!list_empty(&root->node))
 		list_del(&root->node);
+	arch_acpi_pci_root_remove(root);
 	kfree(root);
 	return result;
 }
Index: bjorn-next/include/acpi/acnames.h
===================================================================
--- bjorn-next.orig/include/acpi/acnames.h
+++ bjorn-next/include/acpi/acnames.h
@@ -62,6 +62,7 @@
 #define METHOD_NAME__AEI        "_AEI"
 #define METHOD_NAME__PRW        "_PRW"
 #define METHOD_NAME__SRS        "_SRS"
+#define METHOD_NAME__CBA	"_CBA"
 
 /* Method names - these methods must appear at the namespace root */
 
Index: bjorn-next/include/linux/pci-acpi.h
===================================================================
--- bjorn-next.orig/include/linux/pci-acpi.h
+++ bjorn-next/include/linux/pci-acpi.h
@@ -18,6 +18,9 @@ extern acpi_status pci_acpi_add_pm_notif
 					     struct pci_dev *pci_dev);
 extern acpi_status pci_acpi_remove_pm_notifier(struct acpi_device *dev);
 
+int arch_acpi_pci_root_add(struct acpi_pci_root *root);
+void arch_acpi_pci_root_remove(struct acpi_pci_root *root);
+
 static inline acpi_handle acpi_find_root_bridge_handle(struct pci_dev *pdev)
 {
 	struct pci_bus *pbus = pdev->bus;
Index: bjorn-next/arch/x86/pci/mmconfig-shared.c
===================================================================
--- bjorn-next.orig/arch/x86/pci/mmconfig-shared.c
+++ bjorn-next/arch/x86/pci/mmconfig-shared.c
@@ -28,6 +28,9 @@
 /* Indicate if the mmcfg resources have been placed into the resource table. */
 static int __initdata pci_mmcfg_resources_inserted;
 
+static struct acpi_mcfg_allocation __initdata *mcfg_table;
+static int __initdata mcfg_table_entry_size;
+
 LIST_HEAD(pci_mmcfg_list);
 
 static __init void pci_mmconfig_remove(struct pci_mmcfg_region *cfg)
@@ -522,6 +525,25 @@ static int __init acpi_mcfg_check_entry(
 	return -EINVAL;
 }
 
+u64 query_acpi_mcfg_table(int segment, int start)
+{
+	u64 base_addr = 0;
+	int i;
+
+	if (!mcfg_table)
+		return 0;
+
+	for (i = 0; i < mcfg_table_entry_size; i++) {
+		if (segment == mcfg_table[i].pci_segment &&
+			start == mcfg_table[i].start_bus_number) {
+			base_addr = mcfg_table[i].address;
+			break;
+		}
+	}
+
+	return base_addr;
+}
+
 static int __init pci_parse_mcfg(struct acpi_table_header *header)
 {
 	struct acpi_table_mcfg *mcfg;
@@ -551,19 +573,19 @@ static int __init pci_parse_mcfg(struct 
 	for (i = 0; i < entries; i++) {
 		cfg = &cfg_table[i];
 		if (acpi_mcfg_check_entry(mcfg, cfg)) {
-			free_all_mmcfg();
 			return -ENODEV;
 		}
-
-		if (pci_mmconfig_add(cfg->pci_segment, cfg->start_bus_number,
-				   cfg->end_bus_number, cfg->address) == NULL) {
-			printk(KERN_WARNING PREFIX
-			       "no memory for MCFG entries\n");
-			free_all_mmcfg();
-			return -ENOMEM;
-		}
 	}
 
+	/* populate mcfg_table */
+	mcfg_table = kzalloc(sizeof(struct acpi_mcfg_allocation) * entries,
+			     GFP_KERNEL);
+	if (!mcfg_table)
+		return -ENOMEM;
+	memcpy(mcfg_table, cfg_table,
+	sizeof(struct acpi_mcfg_allocation) * entries);
+	mcfg_table_entry_size = entries;
+
 	return 0;
 }
 
@@ -589,9 +611,6 @@ static void __init __pci_mmcfg_init(int 
 	if (!known_bridge)
 		acpi_sfi_table_parse(ACPI_SIG_MCFG, pci_parse_mcfg);
 
-	if (list_empty(&pci_mmcfg_list))
-		return;
-
 	if (pcibios_last_bus < 0) {
 		const struct pci_mmcfg_region *cfg;
 
@@ -641,6 +660,12 @@ static int __init pci_mmcfg_late_insert_
 	 */
 	pci_mmcfg_insert_resources();
 
+	/*
+	 * MCFG table should not be reffered to any longer
+	 */
+	kfree(mcfg_table);
+	mcfg_table = NULL;
+
 	return 0;
 }
 

Re: [PATCH 6/6] PCI, x86: change the way to add MMCFG region on x86

[Jiang Liu]

Hi Taku,
	Thanks for following this up. Please refer to inline comments below.
I feel there are still some concerns from Don about the change proposed by
Bjorn. And I haven't found a solution to address Don's concerns yet. So it
would be better for us to hold on this for a while.

	BTW, is it OK to split the task into two steps? 
1) enhance mmcfg to support host bridge hotplug.
2) refine the mmcfg setup logic.
Seems it's more realistic for us to reach an agreement for task 1 in a short time,
but there is still much work ahead for task 2.

On 04/26/2012 06:42 PM, Taku Izumi wrote:
> 
> This patch changes the way to add MMCFG region on x86.
> MMCFG region information provided by ACPI MCFG table 
> or _CBA method will be added at acpi_pci_root_add()
> of pci_root driver.
> 
> Signed-off-by: Taku Izumi <izumi.taku@jp.fujitsu.com>
> ---
>  arch/x86/include/asm/pci_x86.h |    2 +
>  arch/x86/pci/acpi.c            |   45 +++++++++++++++++++++++++++++++++++++
>  arch/x86/pci/mmconfig-shared.c |   49 ++++++++++++++++++++++++++++++-----------
>  arch/x86/pci/mmconfig_32.c     |    2 -
>  arch/x86/pci/mmconfig_64.c     |    2 -
>  drivers/acpi/pci_root.c        |   15 ++++++++++++
>  include/acpi/acnames.h         |    1 
>  include/linux/pci-acpi.h       |    3 ++
>  8 files changed, 105 insertions(+), 14 deletions(-)
> 
> Index: bjorn-next/arch/x86/include/asm/pci_x86.h
> ===================================================================
> --- bjorn-next.orig/arch/x86/include/asm/pci_x86.h
> +++ bjorn-next/arch/x86/include/asm/pci_x86.h
> @@ -100,6 +100,7 @@ struct pci_raw_ops {
>  extern const struct pci_raw_ops *raw_pci_ops;
>  extern const struct pci_raw_ops *raw_pci_ext_ops;
>  
> +extern const struct pci_raw_ops pci_mmcfg;
>  extern const struct pci_raw_ops pci_direct_conf1;
>  extern bool port_cf9_safe;
>  
> @@ -140,6 +141,7 @@ extern void pci_mmcfg_arch_unmap(struct 
>  extern struct pci_mmcfg_region *pci_mmconfig_lookup(int segment, int bus);
>  extern int __devinit pci_mmconfig_add(int seg, int start, int end, u64 addr);
>  extern int pci_mmconfig_delete(int seg, int start, int end);
> +extern u64 query_acpi_mcfg_table(int segment, int start);
>  extern struct list_head pci_mmcfg_list;
>  
>  #define PCI_MMCFG_BUS_OFFSET(bus)      ((bus) << 20)
> Index: bjorn-next/arch/x86/pci/acpi.c
> ===================================================================
> --- bjorn-next.orig/arch/x86/pci/acpi.c
> +++ bjorn-next/arch/x86/pci/acpi.c
> @@ -4,6 +4,7 @@
>  #include <linux/irq.h>
>  #include <linux/dmi.h>
>  #include <linux/slab.h>
> +#include <linux/pci-acpi.h>
>  #include <asm/numa.h>
>  #include <asm/pci_x86.h>
>  
> @@ -461,6 +462,50 @@ struct pci_bus * __devinit pci_acpi_scan
>  	return bus;
>  }
>  
> +
> +int __devinit arch_acpi_pci_root_add(struct acpi_pci_root *root)
> +{
> +	int result = 0;
> +	acpi_status status;
> +	unsigned long long base_addr = 0;
> +
> +	/* MMCFG not in use */
> +	if (raw_pci_ext_ops && raw_pci_ext_ops != &pci_mmcfg)
> +		return result;
> +
> +	/* Evaluate _CBA */
> +	status = acpi_evaluate_integer(root->device->handle, METHOD_NAME__CBA,
> +				       NULL, &base_addr);
> +	if (ACPI_FAILURE(status)) {
> +		/* search MCFG table */
> +		base_addr = query_acpi_mcfg_table(root->segment,
> +						  root->secondary.start);
> +
> +	}
> +
> +	/* add MMCFG region */
> +	if (base_addr) {
> +		if (pci_mmconfig_add(root->segment, root->secondary.start,
> +					root->secondary.end, base_addr)) {
> +			printk(KERN_ERR
> +				"can't add MMCFG information for Bus "
> +				"%04x:%02x\n",
> +				root->segment,
> +				(unsigned int)root->secondary.start);
Should set 'result' to suitable error code here.

> +		}
raw_pci_ext_ops should be set to &pci_mmcfg if raw_pci_ext_ops is still NULL
and pci_mmconfig_add succeeds.

> +	} else {
> +		result = -ENODEV;
> +	}
> +
> +	return result;
> +}
> +
> +void arch_acpi_pci_root_remove(struct acpi_pci_root *root)
> +{
> +	pci_mmconfig_delete(root->segment, root->secondary.start,
> +				root->secondary.end);
> +}
> +
>  int __init pci_acpi_init(void)
>  {
>  	struct pci_dev *dev = NULL;
> Index: bjorn-next/arch/x86/pci/mmconfig_32.c
> ===================================================================
> --- bjorn-next.orig/arch/x86/pci/mmconfig_32.c
> +++ bjorn-next/arch/x86/pci/mmconfig_32.c
> @@ -126,7 +126,7 @@ static int pci_mmcfg_write(unsigned int 
>  	return 0;
>  }
>  
> -static const struct pci_raw_ops pci_mmcfg = {
> +const struct pci_raw_ops pci_mmcfg = {
>  	.read =		pci_mmcfg_read,
>  	.write =	pci_mmcfg_write,
>  };
> Index: bjorn-next/arch/x86/pci/mmconfig_64.c
> ===================================================================
> --- bjorn-next.orig/arch/x86/pci/mmconfig_64.c
> +++ bjorn-next/arch/x86/pci/mmconfig_64.c
> @@ -90,7 +90,7 @@ static int pci_mmcfg_write(unsigned int 
>  	return 0;
>  }
>  
> -static const struct pci_raw_ops pci_mmcfg = {
> +const struct pci_raw_ops pci_mmcfg = {
>  	.read =		pci_mmcfg_read,
>  	.write =	pci_mmcfg_write,
>  };
> Index: bjorn-next/drivers/acpi/pci_root.c
> ===================================================================
> --- bjorn-next.orig/drivers/acpi/pci_root.c
> +++ bjorn-next/drivers/acpi/pci_root.c
> @@ -449,6 +449,15 @@ out:
>  }
>  EXPORT_SYMBOL(acpi_pci_osc_control_set);
>  
> +int __weak arch_acpi_pci_root_add(struct acpi_pci_root *root)
> +{
> +	return 0;
> +}
> +
> +void __weak arch_acpi_pci_root_remove(struct acpi_pci_root *root)
> +{
> +}
> +
>  static int __devinit acpi_pci_root_add(struct acpi_device *device)
>  {
>  	unsigned long long segment, bus;
> @@ -505,6 +514,11 @@ static int __devinit acpi_pci_root_add(s
>  	strcpy(acpi_device_class(device), ACPI_PCI_ROOT_CLASS);
>  	device->driver_data = root;
>  
> +	if (arch_acpi_pci_root_add(root)) {
> +		result = -ENODEV;
> +		goto end;
> +	}
> +
>  	/*
>  	 * All supported architectures that use ACPI have support for
>  	 * PCI domains, so we indicate this in _OSC support capabilities.
> @@ -627,6 +641,7 @@ static int __devinit acpi_pci_root_add(s
>  end:
>  	if (!list_empty(&root->node))
>  		list_del(&root->node);
> +	arch_acpi_pci_root_remove(root);
>  	kfree(root);
>  	return result;
>  }
> Index: bjorn-next/include/acpi/acnames.h
> ===================================================================
> --- bjorn-next.orig/include/acpi/acnames.h
> +++ bjorn-next/include/acpi/acnames.h
> @@ -62,6 +62,7 @@
>  #define METHOD_NAME__AEI        "_AEI"
>  #define METHOD_NAME__PRW        "_PRW"
>  #define METHOD_NAME__SRS        "_SRS"
> +#define METHOD_NAME__CBA	"_CBA"
>  
>  /* Method names - these methods must appear at the namespace root */
>  
> Index: bjorn-next/include/linux/pci-acpi.h
> ===================================================================
> --- bjorn-next.orig/include/linux/pci-acpi.h
> +++ bjorn-next/include/linux/pci-acpi.h
> @@ -18,6 +18,9 @@ extern acpi_status pci_acpi_add_pm_notif
>  					     struct pci_dev *pci_dev);
>  extern acpi_status pci_acpi_remove_pm_notifier(struct acpi_device *dev);
>  
> +int arch_acpi_pci_root_add(struct acpi_pci_root *root);
> +void arch_acpi_pci_root_remove(struct acpi_pci_root *root);
> +
>  static inline acpi_handle acpi_find_root_bridge_handle(struct pci_dev *pdev)
>  {
>  	struct pci_bus *pbus = pdev->bus;
> Index: bjorn-next/arch/x86/pci/mmconfig-shared.c
> ===================================================================
> --- bjorn-next.orig/arch/x86/pci/mmconfig-shared.c
> +++ bjorn-next/arch/x86/pci/mmconfig-shared.c
> @@ -28,6 +28,9 @@
>  /* Indicate if the mmcfg resources have been placed into the resource table. */
>  static int __initdata pci_mmcfg_resources_inserted;
>  
> +static struct acpi_mcfg_allocation __initdata *mcfg_table;
> +static int __initdata mcfg_table_entry_size;
> +
Yinghai is working on PCI root host bridge hotplug. One of his work is to provide
online/offline support to non-hotplug-capable PCI host bridges. Here online/offline
means enable/disable PCI host bridge without physical hotplug operations.

With that change, query_acpi_mcfg_table() may be called at runtime for host bridges
without _CBA method, so mcfg_table shouldn't be __initdata.

Suggest to call acpi_sfi_table_parse() directly for simplicity in query_acpi_mcfg_table()
instead of cache MMCFG info in mcfg_table.

>  LIST_HEAD(pci_mmcfg_list);
>  
>  static __init void pci_mmconfig_remove(struct pci_mmcfg_region *cfg)
> @@ -522,6 +525,25 @@ static int __init acpi_mcfg_check_entry(
>  	return -EINVAL;
>  }
>  
> +u64 query_acpi_mcfg_table(int segment, int start)
> +{
> +	u64 base_addr = 0;
> +	int i;
> +
> +	if (!mcfg_table)
> +		return 0;
> +
> +	for (i = 0; i < mcfg_table_entry_size; i++) {
> +		if (segment == mcfg_table[i].pci_segment &&
> +			start == mcfg_table[i].start_bus_number) {
> +			base_addr = mcfg_table[i].address;
> +			break;
> +		}
> +	}
> +
> +	return base_addr;
> +}
It should check the MMCFG item contains [(segment, start), (segment, end)]
instead of "start == mcfg_table[i].start_bus_number".
We have observed many systems which have several PCI host bridges in segment 0
and MMCFG table only reports one entry for bus 0-255 on segment 0.
The above code can't support such a system.

> +
>  static int __init pci_parse_mcfg(struct acpi_table_header *header)
>  {
>  	struct acpi_table_mcfg *mcfg;
> @@ -551,19 +573,19 @@ static int __init pci_parse_mcfg(struct 
>  	for (i = 0; i < entries; i++) {
>  		cfg = &cfg_table[i];
>  		if (acpi_mcfg_check_entry(mcfg, cfg)) {
> -			free_all_mmcfg();
>  			return -ENODEV;
>  		}
> -
> -		if (pci_mmconfig_add(cfg->pci_segment, cfg->start_bus_number,
> -				   cfg->end_bus_number, cfg->address) == NULL) {
> -			printk(KERN_WARNING PREFIX
> -			       "no memory for MCFG entries\n");
> -			free_all_mmcfg();
> -			return -ENOMEM;
> -		}
>  	}
>  
> +	/* populate mcfg_table */
> +	mcfg_table = kzalloc(sizeof(struct acpi_mcfg_allocation) * entries,
> +			     GFP_KERNEL);
> +	if (!mcfg_table)
> +		return -ENOMEM;
> +	memcpy(mcfg_table, cfg_table,
> +	sizeof(struct acpi_mcfg_allocation) * entries);
> +	mcfg_table_entry_size = entries;
> +
The above change will break the pcibios_last_bus logic. The MMCFG initialization sequence
is something like:
1) pci_mmcfg_early_init()
	->__pci_mmcfg_init()
		->pci_parse_mcfg()
2) initialize ACPICA subsystem
3) pci_mmcfg_late_init()
	->__pci_mmcfg_init()
		->pci_parse_mcfg()
4) bind pci_root driver to pci host bridges
	pci_mmconfig_add()

If pci_parse_mcfg() doesn't call pci_mmconfig_add(), pci_mmcfg_list will always be
NULL until step 4. Then function __pci_mmcfg_init() can't set pcibios_last_bus to
the correct value. As Don has mentioned, it's high risky to cause compatibility issues
if pcibios_last_bus hasn't been setup correctly.

>  	return 0;
>  }
>  
> @@ -589,9 +611,6 @@ static void __init __pci_mmcfg_init(int 
>  	if (!known_bridge)
>  		acpi_sfi_table_parse(ACPI_SIG_MCFG, pci_parse_mcfg);
>  
> -	if (list_empty(&pci_mmcfg_list))
> -		return;
> -
This change will always use MMCFG as the mechanism to access PCI extended configuration
space, then it will break some AMD platforms which should use pci_direct_conf1 to access
extend configuration space. Please refer to pci_direct_init() in arch/x86/pci/direct.c

>  	if (pcibios_last_bus < 0) {
>  		const struct pci_mmcfg_region *cfg;
>  
> @@ -641,6 +660,12 @@ static int __init pci_mmcfg_late_insert_
>  	 */
>  	pci_mmcfg_insert_resources();
>  
> +	/*
> +	 * MCFG table should not be reffered to any longer
> +	 */
> +	kfree(mcfg_table);
> +	mcfg_table = NULL;
> +
>  	return 0;
>  }
>  
> 

Re: [PATCH 6/6] PCI, x86: change the way to add MMCFG region on x86

[Taku Izumi]

Hi Jian, Yinghai,

 Thank you your comment and review.

On Thu, 26 Apr 2012 23:04:33 +0800
Jiang Liu <liuj97@gmail.com> wrote:

> Hi Taku,
> 	Thanks for following this up. Please refer to inline comments below.
> I feel there are still some concerns from Don about the change proposed by
> Bjorn. And I haven't found a solution to address Don's concerns yet. So it
> would be better for us to hold on this for a while.
> 
> 	BTW, is it OK to split the task into two steps? 

   Sure.

> 1) enhance mmcfg to support host bridge hotplug.
> 2) refine the mmcfg setup logic.
> Seems it's more realistic for us to reach an agreement for task 1 in a short time,
> but there is still much work ahead for task 2.

  I agree with you.
  To achive task 1, your V4 should is good enough, but
  I found a problem in your patchset.

  When a hotpluggable hostbridge is found at boot time, its MCFG information should be 
  provided via _CBA method, and will added at the following timing:

         arch_acpi_pci_root_add()
            + pci_mmconfig_insert()
              + insert_resource()

  But at boot time, all entries in pci_mmcfg_list collection will added at the following timing:

        late_initcall
           + pci_mmcfg_late_insert_resources()
             + pci_mmcfg_insert_resources()
               + insert_resource()

  So, MCFG information of hotpluggable hostbridges which exist at boot time will fail to 
  add at this timing.

+int __devinit pci_mmconfig_insert(int segment, int start, int end, u64 addr)
+{
+	int rc = -EINVAL;
+	struct pci_mmcfg_region *cfg = NULL;
+
+	if (segment < 0 || segment > USHRT_MAX ||
+	    start < 0 || start > 255 || end < start || end > 255)
+		return rc;
+
+	mutex_lock(&pci_mmcfg_lock);
+	cfg = pci_mmconfig_lookup(segment, start);
+	if (cfg) {
+		if (cfg->start_bus <= start && cfg->end_bus >= end) {
+			rc = -EEXIST;
+		} else {
+			printk(KERN_WARNING PREFIX
+			       "MMCONFIG for domain %04x [bus %02x-%02x] "
+			       "conflicts with domain %04x [bus %02x-%02x]\n",
+			       segment, start, end,
+			       cfg->segment, cfg->start_bus, cfg->end_bus);
+		}
+		goto out;
+	}
+	if (!addr)
+		goto out;
+
+	cfg = pci_mmconfig_alloc(segment, start, end, addr);
+	if (cfg == NULL) {
+		rc = -ENOMEM;
+	} else if (!pci_mmcfg_check_reserved(cfg, 0)) {
+		printk(KERN_WARNING PREFIX
+		       "MMCONFIG for domain %04x [bus %02x-%02x] "
+		       "isn't reserved\n", segment, start, end);
+	} else if (insert_resource(&iomem_resource, &cfg->res)) {

          I think the following is better.

          else if (pci_mmcfg_resources_inserted &&
                   insert_resource(&iomem_resource, &cfg->res)) {


+		rc = -EBUSY;
+		printk(KERN_WARNING PREFIX
+		       "failed to insert resource for domain "
+		       "%04x [bus %02x-%02x]\n", segment, start, end);
+	} else if (pci_mmcfg_arch_map(cfg)) {
+		rc = -EBUSY;


  Could you please fix this?


Best regards,
Taku Izumi

[PATCH] PCI, x86: avoid redundant insert_resource() call in pci_mmcfg_insert_resources()

[Jiang Liu]

From: Jiang Liu <jiang.liu@huawei.com>

For hot-pluggable host bridges present at boot time, pci_mmconfig_insert() will
insert resources used by MMCFG items for those host bridges. So avoid redundant
call to insert_resource() when pci_mmcfg_insert_resources() is called later.

Reported-By: Taku Izumi <izumi.taku@jp.fujitsu.com>
Signed-off-by: Jiang Liu <liuj97@gmail.com>
---
 arch/x86/pci/mmconfig-shared.c |    7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/arch/x86/pci/mmconfig-shared.c b/arch/x86/pci/mmconfig-shared.c
index f4d9ff9..7bba722 100644
--- a/arch/x86/pci/mmconfig-shared.c
+++ b/arch/x86/pci/mmconfig-shared.c
@@ -371,8 +371,13 @@ static void __init pci_mmcfg_insert_resources(void)
 {
 	struct pci_mmcfg_region *cfg;
 
+	/*
+	 * Insert resources for MMCFG items if the resource hasn't been
+	 * inserted by pci_mmconfig_insert() yet.
+	 */
 	list_for_each_entry(cfg, &pci_mmcfg_list, list)
-		insert_resource(&iomem_resource, &cfg->res);
+		if (!cfg->res.parent)
+			insert_resource(&iomem_resource, &cfg->res);
 
 	/* Mark that the resources have been inserted. */
 	pci_mmcfg_resources_inserted = 1;
-- 
1.7.9.5

Re: [PATCH 0/6][RFC] pci, x86: change the way to add MMCFG region on x86

[Yinghai Lu]

On Thu, Apr 26, 2012 at 3:37 AM, Taku Izumi <izumi.taku@jp.fujitsu.com> wrote:
>
>  This patchset is based on the following Jiang Liu's patchset:
>   http://marc.info/?l=linux-pci&m=133410334617298&w=2
>
>  This changes the way to add MMCFG region on x86.
>  MMCFG region information provided by ACPI MCFG table
>  or _CBA method will be added at acpi_pci_root_add()
>  of pci_root driver.
>
>  patch1-4 is exactly same as Liu's.
>
>  *[PATCH 1/6] PCI, x86: split out pci_mmcfg_check_reserved() for code reuse
>  *[PATCH 2/6] PCI, x86: split out pci_mmconfig_alloc() for code reuse
>  *[PATCH 3/6] PCI, x86: use RCU list to protect mmconfig list
>  *[PATCH 4/6] PCI, x86: introduce pci_mmcfg_arch_map()/pci_mmcfg_arch_unmap()
>  *[PATCH 5/6] PCI, x86: introduce pci_mmcongi_add() / pci_mmconfig_delete()
>  *[PATCH 6/6] PCI, x86: change the way to add MMCFG region on x86

what happened ?

Jiang's V4 should be good enough.  I have used them in my local tree
for a while.

Please consider to post addon patches for that patchset if you have
other corner case that is not handled.

Thanks

Yinghai Lu

Re: [PATCH 0/6][RFC] pci, x86: change the way to add MMCFG region on x86

[Bjorn Helgaas]

On Thu, Apr 26, 2012 at 4:37 AM, Taku Izumi <izumi.taku@jp.fujitsu.com> wrote:
>
>  This patchset is based on the following Jiang Liu's patchset:
>   http://marc.info/?l=linux-pci&m=133410334617298&w=2
>
>  This changes the way to add MMCFG region on x86.
>  MMCFG region information provided by ACPI MCFG table
>  or _CBA method will be added at acpi_pci_root_add()
>  of pci_root driver.
>
>  patch1-4 is exactly same as Liu's.
>
>  *[PATCH 1/6] PCI, x86: split out pci_mmcfg_check_reserved() for code reuse
>  *[PATCH 2/6] PCI, x86: split out pci_mmconfig_alloc() for code reuse
>  *[PATCH 3/6] PCI, x86: use RCU list to protect mmconfig list
>  *[PATCH 4/6] PCI, x86: introduce pci_mmcfg_arch_map()/pci_mmcfg_arch_unmap()
>  *[PATCH 5/6] PCI, x86: introduce pci_mmcongi_add() / pci_mmconfig_delete()
>  *[PATCH 6/6] PCI, x86: change the way to add MMCFG region on x86

I can't keep track of what's going on here, with you and Gerry posting
the same patches.  You can post additional patches that apply on top
of Gerry's, or work with Gerry to get your changes incorporated in his
patches.  But it's too hard to sort out the combination with different
versions of the same thing coming from both of you.

Bjorn