[RFC v2 0/7] Consolidate ACPI PCI root common code into ACPI core

[RFC v2 0/7] Consolidate ACPI PCI root common code into ACPI core

[Jiang Liu]

This patch set consolidates common code to support ACPI PCI root on x86
and IA64 platforms into ACPI core, to reproduce duplicated code and
simplify maintenance. The common code should also used to support PCI
host bridge on ARM64 too, but I'm lacking of knowledge about PCIe host
bridge implementation details on ARM64, so please help to review whether
this is suitable for ARM64 too.

It's based on latest mainstream kernel plus the patch to fix a regression
at: http://lkml.org/lkml/2015/4/30/13

It passes Fengguang's 0day test suite and has been tested on two IA64
platforms and one x86 platform.

The patch set is available at:
https://github.com/jiangliu/linux.git acpi_pci_root_v2

Thanks!
Gerry

Jiang Liu (7):
  ACPI/PCI: Enhance ACPI core to support sparse IO space
  ia64/PCI/ACPI: Use common ACPI resource parsing interface for host
    bridge
  ia64/PCI: Use common struct resource_entry to replace struct
    iospace_resource
  x86/PCI: Rename struct pci_sysdata as struct pci_controller
  PCI/ACPI: Consolidate common PCI host bridge code into ACPI core
  x86/PCI/ACPI: Use common interface to support PCI host bridge
  ia64/PCI/ACPI: Use common interface to support PCI host bridge

 arch/ia64/include/asm/pci.h   |    5 -
 arch/ia64/pci/pci.c           |  373 +++++++++++------------------------------
 arch/x86/include/asm/pci.h    |   13 +-
 arch/x86/include/asm/pci_64.h |    4 +-
 arch/x86/pci/acpi.c           |  281 +++++++++----------------------
 arch/x86/pci/common.c         |    2 +-
 drivers/acpi/pci_root.c       |  214 +++++++++++++++++++++++
 drivers/acpi/resource.c       |    9 +-
 include/linux/ioport.h        |    1 +
 include/linux/pci-acpi.h      |   24 +++
 10 files changed, 431 insertions(+), 495 deletions(-)

-- 
1.7.10.4

[RFC v2 1/7] ACPI/PCI: Enhance ACPI core to support sparse IO space

[Jiang Liu]

Enhance ACPI resource parsing interfaces to support sparse IO space,
which will be used to share common code between x86 and IA64 later.

Tested-by: Tony Luck <tony.luck@intel.com>
Signed-off-by: Jiang Liu <jiang.liu@linux.intel.com>
---
 drivers/acpi/resource.c |    9 ++++++---
 include/linux/ioport.h  |    1 +
 2 files changed, 7 insertions(+), 3 deletions(-)

diff --git a/drivers/acpi/resource.c b/drivers/acpi/resource.c
index 8244f013f210..fdcc73dad2c1 100644
--- a/drivers/acpi/resource.c
+++ b/drivers/acpi/resource.c
@@ -123,7 +123,7 @@ bool acpi_dev_resource_memory(struct acpi_resource *ares, struct resource *res)
 EXPORT_SYMBOL_GPL(acpi_dev_resource_memory);
 
 static void acpi_dev_ioresource_flags(struct resource *res, u64 len,
-				      u8 io_decode)
+				      u8 io_decode, u8 translation_type)
 {
 	res->flags = IORESOURCE_IO;
 
@@ -135,6 +135,8 @@ static void acpi_dev_ioresource_flags(struct resource *res, u64 len,
 
 	if (io_decode == ACPI_DECODE_16)
 		res->flags |= IORESOURCE_IO_16BIT_ADDR;
+	if (translation_type == ACPI_SPARSE_TRANSLATION)
+		res->flags |= IORESOURCE_IO_SPARSE;
 }
 
 static void acpi_dev_get_ioresource(struct resource *res, u64 start, u64 len,
@@ -142,7 +144,7 @@ static void acpi_dev_get_ioresource(struct resource *res, u64 start, u64 len,
 {
 	res->start = start;
 	res->end = start + len - 1;
-	acpi_dev_ioresource_flags(res, len, io_decode);
+	acpi_dev_ioresource_flags(res, len, io_decode, 0);
 }
 
 /**
@@ -227,7 +229,8 @@ static bool acpi_decode_space(struct resource_win *win,
 		acpi_dev_memresource_flags(res, len, wp);
 		break;
 	case ACPI_IO_RANGE:
-		acpi_dev_ioresource_flags(res, len, iodec);
+		acpi_dev_ioresource_flags(res, len, iodec,
+					  addr->info.io.translation_type);
 		break;
 	case ACPI_BUS_NUMBER_RANGE:
 		res->flags = IORESOURCE_BUS;
diff --git a/include/linux/ioport.h b/include/linux/ioport.h
index 388e3ae94f7a..24bea087e7af 100644
--- a/include/linux/ioport.h
+++ b/include/linux/ioport.h
@@ -94,6 +94,7 @@ struct resource {
 /* PnP I/O specific bits (IORESOURCE_BITS) */
 #define IORESOURCE_IO_16BIT_ADDR	(1<<0)
 #define IORESOURCE_IO_FIXED		(1<<1)
+#define IORESOURCE_IO_SPARSE		(1<<2)
 
 /* PCI ROM control bits (IORESOURCE_BITS) */
 #define IORESOURCE_ROM_ENABLE		(1<<0)	/* ROM is enabled, same as PCI_ROM_ADDRESS_ENABLE */
-- 
1.7.10.4

[RFC v2 1/7] ACPI/PCI: Enhance ACPI core to support sparse IO space

[Hanjun Guo]

On 2015?05?05? 10:46, Jiang Liu wrote:
> Enhance ACPI resource parsing interfaces to support sparse IO space,
> which will be used to share common code between x86 and IA64 later.
>
> Tested-by: Tony Luck <tony.luck@intel.com>
> Signed-off-by: Jiang Liu <jiang.liu@linux.intel.com>
> ---
>   drivers/acpi/resource.c |    9 ++++++---
>   include/linux/ioport.h  |    1 +
>   2 files changed, 7 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/acpi/resource.c b/drivers/acpi/resource.c
> index 8244f013f210..fdcc73dad2c1 100644
> --- a/drivers/acpi/resource.c
> +++ b/drivers/acpi/resource.c
> @@ -123,7 +123,7 @@ bool acpi_dev_resource_memory(struct acpi_resource *ares, struct resource *res)
>   EXPORT_SYMBOL_GPL(acpi_dev_resource_memory);
>
>   static void acpi_dev_ioresource_flags(struct resource *res, u64 len,
> -				      u8 io_decode)
> +				      u8 io_decode, u8 translation_type)
>   {
>   	res->flags = IORESOURCE_IO;
>
> @@ -135,6 +135,8 @@ static void acpi_dev_ioresource_flags(struct resource *res, u64 len,
>
>   	if (io_decode == ACPI_DECODE_16)
>   		res->flags |= IORESOURCE_IO_16BIT_ADDR;
> +	if (translation_type == ACPI_SPARSE_TRANSLATION)
> +		res->flags |= IORESOURCE_IO_SPARSE;
>   }
>
>   static void acpi_dev_get_ioresource(struct resource *res, u64 start, u64 len,
> @@ -142,7 +144,7 @@ static void acpi_dev_get_ioresource(struct resource *res, u64 start, u64 len,
>   {
>   	res->start = start;
>   	res->end = start + len - 1;
> -	acpi_dev_ioresource_flags(res, len, io_decode);
> +	acpi_dev_ioresource_flags(res, len, io_decode, 0);
>   }
>
>   /**
> @@ -227,7 +229,8 @@ static bool acpi_decode_space(struct resource_win *win,
>   		acpi_dev_memresource_flags(res, len, wp);
>   		break;
>   	case ACPI_IO_RANGE:
> -		acpi_dev_ioresource_flags(res, len, iodec);
> +		acpi_dev_ioresource_flags(res, len, iodec,
> +					  addr->info.io.translation_type);
>   		break;
>   	case ACPI_BUS_NUMBER_RANGE:
>   		res->flags = IORESOURCE_BUS;
> diff --git a/include/linux/ioport.h b/include/linux/ioport.h
> index 388e3ae94f7a..24bea087e7af 100644
> --- a/include/linux/ioport.h
> +++ b/include/linux/ioport.h
> @@ -94,6 +94,7 @@ struct resource {
>   /* PnP I/O specific bits (IORESOURCE_BITS) */
>   #define IORESOURCE_IO_16BIT_ADDR	(1<<0)
>   #define IORESOURCE_IO_FIXED		(1<<1)
> +#define IORESOURCE_IO_SPARSE		(1<<2)
>
>   /* PCI ROM control bits (IORESOURCE_BITS) */
>   #define IORESOURCE_ROM_ENABLE		(1<<0)	/* ROM is enabled, same as PCI_ROM_ADDRESS_ENABLE */

Reviewed-by: Hanjun Guo <hanjun.guo@linaro.org>

Thanks
hanjun

[RFC v2 2/7] ia64/PCI/ACPI: Use common ACPI resource parsing interface for host bridge

[Jiang Liu]

Use common ACPI resource parsing interface to parse ACPI resources for
PCI host bridge, so we could share more code between IA64 and x86.
Later we will consolidate arch specific implementations into ACPI core.

Tested-by: Tony Luck <tony.luck@intel.com>
Signed-off-by: Jiang Liu <jiang.liu@linux.intel.com>
---
 arch/ia64/pci/pci.c |  414 ++++++++++++++++++++++++---------------------------
 1 file changed, 193 insertions(+), 221 deletions(-)

diff --git a/arch/ia64/pci/pci.c b/arch/ia64/pci/pci.c
index d4e162d35b34..23689d4c37ae 100644
--- a/arch/ia64/pci/pci.c
+++ b/arch/ia64/pci/pci.c
@@ -115,29 +115,12 @@ struct pci_ops pci_root_ops = {
 	.write = pci_write,
 };
 
-/* Called by ACPI when it finds a new root bus.  */
-
-static struct pci_controller *alloc_pci_controller(int seg)
-{
-	struct pci_controller *controller;
-
-	controller = kzalloc(sizeof(*controller), GFP_KERNEL);
-	if (!controller)
-		return NULL;
-
-	controller->segment = seg;
-	return controller;
-}
-
 struct pci_root_info {
+	struct pci_controller controller;
 	struct acpi_device *bridge;
-	struct pci_controller *controller;
 	struct list_head resources;
-	struct resource *res;
-	resource_size_t *res_offset;
-	unsigned int res_num;
 	struct list_head io_resources;
-	char *name;
+	char name[16];
 };
 
 static unsigned int
@@ -168,11 +151,11 @@ new_space (u64 phys_base, int sparse)
 	return i;
 }
 
-static u64 add_io_space(struct pci_root_info *info,
-			struct acpi_resource_address64 *addr)
+static int add_io_space(struct device *dev, struct pci_root_info *info,
+			struct resource_entry *entry)
 {
 	struct iospace_resource *iospace;
-	struct resource *resource;
+	struct resource *resource, *res = entry->res;
 	char *name;
 	unsigned long base, min, max, base_port;
 	unsigned int sparse = 0, space_nr, len;
@@ -180,27 +163,24 @@ static u64 add_io_space(struct pci_root_info *info,
 	len = strlen(info->name) + 32;
 	iospace = kzalloc(sizeof(*iospace) + len, GFP_KERNEL);
 	if (!iospace) {
-		dev_err(&info->bridge->dev,
-				"PCI: No memory for %s I/O port space\n",
-				info->name);
-		goto out;
+		dev_err(dev, "PCI: No memory for %s I/O port space\n",
+			info->name);
+		return -ENOMEM;
 	}
 
-	name = (char *)(iospace + 1);
-
-	min = addr->address.minimum;
-	max = min + addr->address.address_length - 1;
-	if (addr->info.io.translation_type == ACPI_SPARSE_TRANSLATION)
+	if (res->flags & IORESOURCE_IO_SPARSE)
 		sparse = 1;
-
-	space_nr = new_space(addr->address.translation_offset, sparse);
+	space_nr = new_space(entry->offset, sparse);
 	if (space_nr == ~0)
 		goto free_resource;
 
+	name = (char *)(iospace + 1);
+	min = res->start - entry->offset;
+	max = res->end - entry->offset;
 	base = __pa(io_space[space_nr].mmio_base);
 	base_port = IO_SPACE_BASE(space_nr);
 	snprintf(name, len, "%s I/O Ports %08lx-%08lx", info->name,
-		base_port + min, base_port + max);
+		 base_port + min, base_port + max);
 
 	/*
 	 * The SDM guarantees the legacy 0-64K space is sparse, but if the
@@ -216,153 +196,195 @@ static u64 add_io_space(struct pci_root_info *info,
 	resource->start = base + (sparse ? IO_SPACE_SPARSE_ENCODING(min) : min);
 	resource->end   = base + (sparse ? IO_SPACE_SPARSE_ENCODING(max) : max);
 	if (insert_resource(&iomem_resource, resource)) {
-		dev_err(&info->bridge->dev,
-				"can't allocate host bridge io space resource  %pR\n",
-				resource);
+		dev_err(dev,
+			"can't allocate host bridge io space resource  %pR\n",
+			resource);
 		goto free_resource;
 	}
 
+	entry->offset = base_port;
+	res->start = min + base_port;
+	res->end = max + base_port;
 	list_add_tail(&iospace->list, &info->io_resources);
-	return base_port;
+
+	return 0;
 
 free_resource:
 	kfree(iospace);
-out:
-	return ~0;
+	return -ENOSPC;
+}
+
+/*
+ * An IO port or MMIO resource assigned to a PCI host bridge may be
+ * consumed by the host bridge itself or available to its child
+ * bus/devices. The ACPI specification defines a bit (Producer/Consumer)
+ * to tell whether the resource is consumed by the host bridge itself,
+ * but firmware hasn't used that bit consistently, so we can't rely on it.
+ *
+ * On x86 and IA64 platforms, all IO port and MMIO resources are assumed
+ * to be available to child bus/devices except one special case:
+ *     IO port [0xCF8-0xCFF] is consumed by the host bridge itself
+ *     to access PCI configuration space.
+ *
+ * So explicitly filter out PCI CFG IO ports[0xCF8-0xCFF].
+ */
+static bool resource_is_pcicfg_ioport(struct resource *res)
+{
+	return (res->flags & IORESOURCE_IO) &&
+		res->start == 0xCF8 && res->end == 0xCFF;
 }
 
-static acpi_status resource_to_window(struct acpi_resource *resource,
-				      struct acpi_resource_address64 *addr)
+static int
+probe_pci_root_info(struct pci_root_info *info, struct acpi_device *device,
+		    int busnum, int domain)
 {
-	acpi_status status;
+	int ret;
+	struct list_head *list = &info->resources;
+	struct resource_entry *entry, *tmp;
 
-	/*
-	 * We're only interested in _CRS descriptors that are
-	 *	- address space descriptors for memory or I/O space
-	 *	- non-zero size
-	 */
-	status = acpi_resource_to_address64(resource, addr);
-	if (ACPI_SUCCESS(status) &&
-	    (addr->resource_type == ACPI_MEMORY_RANGE ||
-	     addr->resource_type == ACPI_IO_RANGE) &&
-	    addr->address.address_length)
-		return AE_OK;
-
-	return AE_ERROR;
-}
-
-static acpi_status count_window(struct acpi_resource *resource, void *data)
-{
-	unsigned int *windows = (unsigned int *) data;
-	struct acpi_resource_address64 addr;
-	acpi_status status;
-
-	status = resource_to_window(resource, &addr);
-	if (ACPI_SUCCESS(status))
-		(*windows)++;
-
-	return AE_OK;
-}
-
-static acpi_status add_window(struct acpi_resource *res, void *data)
-{
-	struct pci_root_info *info = data;
-	struct resource *resource;
-	struct acpi_resource_address64 addr;
-	acpi_status status;
-	unsigned long flags, offset = 0;
-	struct resource *root;
-
-	/* Return AE_OK for non-window resources to keep scanning for more */
-	status = resource_to_window(res, &addr);
-	if (!ACPI_SUCCESS(status))
-		return AE_OK;
-
-	if (addr.resource_type == ACPI_MEMORY_RANGE) {
-		flags = IORESOURCE_MEM;
-		root = &iomem_resource;
-		offset = addr.address.translation_offset;
-	} else if (addr.resource_type == ACPI_IO_RANGE) {
-		flags = IORESOURCE_IO;
-		root = &ioport_resource;
-		offset = add_io_space(info, &addr);
-		if (offset == ~0)
-			return AE_OK;
-	} else
-		return AE_OK;
-
-	resource = &info->res[info->res_num];
-	resource->name = info->name;
-	resource->flags = flags;
-	resource->start = addr.address.minimum + offset;
-	resource->end = resource->start + addr.address.address_length - 1;
-	info->res_offset[info->res_num] = offset;
-
-	if (insert_resource(root, resource)) {
-		dev_err(&info->bridge->dev,
-			"can't allocate host bridge window %pR\n",
-			resource);
-	} else {
-		if (offset)
-			dev_info(&info->bridge->dev, "host bridge window %pR "
-				 "(PCI address [%#llx-%#llx])\n",
-				 resource,
-				 resource->start - offset,
-				 resource->end - offset);
-		else
-			dev_info(&info->bridge->dev,
-				 "host bridge window %pR\n", resource);
-	}
-	/* HP's firmware has a hack to work around a Windows bug.
-	 * Ignore these tiny memory ranges */
-	if (!((resource->flags & IORESOURCE_MEM) &&
-	      (resource->end - resource->start < 16)))
-		pci_add_resource_offset(&info->resources, resource,
-					info->res_offset[info->res_num]);
+	ret = acpi_dev_get_resources(device, list,
+				     acpi_dev_filter_resource_type_cb,
+				     (void *)(IORESOURCE_IO | IORESOURCE_MEM));
+	if (ret < 0)
+		dev_warn(&device->dev,
+			 "failed to parse _CRS method, error code %d\n", ret);
+	else if (ret == 0)
+		dev_dbg(&device->dev,
+			"no IO and memory resources present in _CRS\n");
+	else
+		resource_list_for_each_entry_safe(entry, tmp, list) {
+			if ((entry->res->flags & IORESOURCE_DISABLED) ||
+			    resource_is_pcicfg_ioport(entry->res))
+				resource_list_destroy_entry(entry);
+			else
+				entry->res->name = info->name;
+		}
 
-	info->res_num++;
-	return AE_OK;
+	return ret;
 }
 
-static void free_pci_root_info_res(struct pci_root_info *info)
-{
-	struct iospace_resource *iospace, *tmp;
+static void validate_resources(struct device *dev, struct list_head *resources,
+			       unsigned long type)
+{
+	LIST_HEAD(list);
+	struct resource *res1, *res2, *root = NULL;
+	struct resource_entry *tmp, *entry, *entry2;
+
+	BUG_ON((type & (IORESOURCE_MEM | IORESOURCE_IO)) == 0);
+	root = (type & IORESOURCE_MEM) ? &iomem_resource : &ioport_resource;
+
+	list_splice_init(resources, &list);
+	resource_list_for_each_entry_safe(entry, tmp, &list) {
+		bool free = false;
+		resource_size_t end;
+
+		res1 = entry->res;
+		if (!(res1->flags & type))
+			goto next;
+
+		/* Exclude non-addressable range or non-addressable portion */
+		end = min(res1->end, root->end);
+		if (end <= res1->start) {
+			dev_info(dev, "host bridge window %pR (ignored, not CPU addressable)\n",
+				 res1);
+			free = true;
+			goto next;
+		} else if (res1->end != end) {
+			dev_info(dev, "host bridge window %pR ([%#llx-%#llx] ignored, not CPU addressable)\n",
+				 res1, (unsigned long long)end + 1,
+				 (unsigned long long)res1->end);
+			res1->end = end;
+		}
 
-	list_for_each_entry_safe(iospace, tmp, &info->io_resources, list)
-		kfree(iospace);
+		resource_list_for_each_entry(entry2, resources) {
+			res2 = entry2->res;
+			if (!(res2->flags & type))
+				continue;
+
+			/*
+			 * I don't like throwing away windows because then
+			 * our resources no longer match the ACPI _CRS, but
+			 * the kernel resource tree doesn't allow overlaps.
+			 */
+			if (resource_overlaps(res1, res2)) {
+				res2->start = min(res1->start, res2->start);
+				res2->end = max(res1->end, res2->end);
+				dev_info(dev, "host bridge window expanded to %pR; %pR ignored\n",
+					 res2, res1);
+				free = true;
+				goto next;
+			}
+		}
 
-	kfree(info->name);
-	kfree(info->res);
-	info->res = NULL;
-	kfree(info->res_offset);
-	info->res_offset = NULL;
-	info->res_num = 0;
-	kfree(info->controller);
-	info->controller = NULL;
+next:
+		resource_list_del(entry);
+		if (free)
+			resource_list_free_entry(entry);
+		else
+			resource_list_add_tail(entry, resources);
+	}
+}
+
+static void add_resources(struct pci_root_info *info, struct device *dev)
+{
+	struct resource_entry *entry, *tmp;
+	struct resource *res, *conflict, *root = NULL;
+	struct list_head *list = &info->resources;
+
+	validate_resources(dev, list, IORESOURCE_MEM);
+	validate_resources(dev, list, IORESOURCE_IO);
+
+	resource_list_for_each_entry_safe(entry, tmp, list) {
+		res = entry->res;
+		if (res->flags & IORESOURCE_MEM) {
+			root = &iomem_resource;
+			/*
+			 * HP's firmware has a hack to work around a Windows
+			 * bug. Ignore these tiny memory ranges.
+			 */
+			if (resource_size(res) <= 16) {
+				resource_list_destroy_entry(entry);
+				continue;
+			}
+		} else if (res->flags & IORESOURCE_IO) {
+			root = &ioport_resource;
+			if (add_io_space(&info->bridge->dev, info, entry)) {
+				resource_list_destroy_entry(entry);
+				continue;
+			}
+		} else {
+			BUG_ON(res);
+		}
+
+		conflict = insert_resource_conflict(root, res);
+		if (conflict) {
+			dev_info(dev,
+				 "ignoring host bridge window %pR (conflicts with %s %pR)\n",
+				 res, conflict->name, conflict);
+			resource_list_destroy_entry(entry);
+		}
+	}
 }
 
 static void __release_pci_root_info(struct pci_root_info *info)
 {
-	int i;
 	struct resource *res;
-	struct iospace_resource *iospace;
+	struct iospace_resource *iospace, *tmp;
+	struct resource_entry *entry, *tentry;
 
-	list_for_each_entry(iospace, &info->io_resources, list)
+	list_for_each_entry_safe(iospace, tmp, &info->io_resources, list) {
 		release_resource(&iospace->res);
+		kfree(iospace);
+	}
 
-	for (i = 0; i < info->res_num; i++) {
-		res = &info->res[i];
-
-		if (!res->parent)
-			continue;
-
-		if (!(res->flags & (IORESOURCE_MEM | IORESOURCE_IO)))
-			continue;
-
-		release_resource(res);
+	resource_list_for_each_entry_safe(entry, tentry, &info->resources) {
+		res = entry->res;
+		if (res->parent &&
+		    (res->flags & (IORESOURCE_MEM | IORESOURCE_IO)))
+			release_resource(res);
+		resource_list_destroy_entry(entry);
 	}
 
-	free_pci_root_info_res(info);
 	kfree(info);
 }
 
@@ -373,99 +395,49 @@ static void release_pci_root_info(struct pci_host_bridge *bridge)
 	__release_pci_root_info(info);
 }
 
-static int
-probe_pci_root_info(struct pci_root_info *info, struct acpi_device *device,
-		int busnum, int domain)
-{
-	char *name;
-
-	name = kmalloc(16, GFP_KERNEL);
-	if (!name)
-		return -ENOMEM;
-
-	sprintf(name, "PCI Bus %04x:%02x", domain, busnum);
-	info->bridge = device;
-	info->name = name;
-
-	acpi_walk_resources(device->handle, METHOD_NAME__CRS, count_window,
-			&info->res_num);
-	if (info->res_num) {
-		info->res =
-			kzalloc_node(sizeof(*info->res) * info->res_num,
-				     GFP_KERNEL, info->controller->node);
-		if (!info->res) {
-			kfree(name);
-			return -ENOMEM;
-		}
-
-		info->res_offset =
-			kzalloc_node(sizeof(*info->res_offset) * info->res_num,
-					GFP_KERNEL, info->controller->node);
-		if (!info->res_offset) {
-			kfree(name);
-			kfree(info->res);
-			info->res = NULL;
-			return -ENOMEM;
-		}
-
-		info->res_num = 0;
-		acpi_walk_resources(device->handle, METHOD_NAME__CRS,
-			add_window, info);
-	} else
-		kfree(name);
-
-	return 0;
-}
-
 struct pci_bus *pci_acpi_scan_root(struct acpi_pci_root *root)
 {
 	struct acpi_device *device = root->device;
 	int domain = root->segment;
 	int bus = root->secondary.start;
-	struct pci_controller *controller;
-	struct pci_root_info *info = NULL;
-	int busnum = root->secondary.start;
+	struct pci_root_info *info;
 	struct pci_bus *pbus;
 	int ret;
 
-	controller = alloc_pci_controller(domain);
-	if (!controller)
-		return NULL;
-
-	controller->companion = device;
-	controller->node = acpi_get_node(device->handle);
-
 	info = kzalloc(sizeof(*info), GFP_KERNEL);
 	if (!info) {
 		dev_err(&device->dev,
-				"pci_bus %04x:%02x: ignored (out of memory)\n",
-				domain, busnum);
-		kfree(controller);
+			"pci_bus %04x:%02x: ignored (out of memory)\n",
+			domain, bus);
 		return NULL;
 	}
 
-	info->controller = controller;
-	INIT_LIST_HEAD(&info->io_resources);
+	info->controller.segment = domain;
+	info->controller.companion = device;
+	info->controller.node = acpi_get_node(device->handle);
+	info->bridge = device;
 	INIT_LIST_HEAD(&info->resources);
+	INIT_LIST_HEAD(&info->io_resources);
+	snprintf(info->name, sizeof(info->name),
+		 "PCI Bus %04x:%02x", domain, bus);
 
-	ret = probe_pci_root_info(info, device, busnum, domain);
-	if (ret) {
-		kfree(info->controller);
+	ret = probe_pci_root_info(info, device, bus, domain);
+	if (ret <= 0) {
 		kfree(info);
 		return NULL;
 	}
-	/* insert busn resource@first */
+	add_resources(info, &info->bridge->dev);
 	pci_add_resource(&info->resources, &root->secondary);
+
 	/*
 	 * See arch/x86/pci/acpi.c.
 	 * The desired pci bus might already be scanned in a quirk. We
 	 * should handle the case here, but it appears that IA64 hasn't
 	 * such quirk. So we just ignore the case now.
 	 */
-	pbus = pci_create_root_bus(NULL, bus, &pci_root_ops, controller,
-				   &info->resources);
+	pbus = pci_create_root_bus(NULL, bus, &pci_root_ops,
+				   &info->controller, &info->resources);
 	if (!pbus) {
-		pci_free_resource_list(&info->resources);
 		__release_pci_root_info(info);
 		return NULL;
 	}
-- 
1.7.10.4

[RFC v2 2/7] ia64/PCI/ACPI: Use common ACPI resource parsing interface for host bridge

[Hanjun Guo]

On 2015?05?05? 10:46, Jiang Liu wrote:
> Use common ACPI resource parsing interface to parse ACPI resources for
> PCI host bridge, so we could share more code between IA64 and x86.
> Later we will consolidate arch specific implementations into ACPI core.
>
> Tested-by: Tony Luck <tony.luck@intel.com>
> Signed-off-by: Jiang Liu <jiang.liu@linux.intel.com>
> ---
>   arch/ia64/pci/pci.c |  414 ++++++++++++++++++++++++---------------------------
>   1 file changed, 193 insertions(+), 221 deletions(-)

Because the first version of ACPI base host bridge init for ARM64
is copied from  IA64, so I also familiar with this code, and
the changes in this patch is ok to me, so

Reviewed-by: Hanjun Guo <hanjun.guo@linaro.org>

Thanks
hanjun

>
> diff --git a/arch/ia64/pci/pci.c b/arch/ia64/pci/pci.c
> index d4e162d35b34..23689d4c37ae 100644
> --- a/arch/ia64/pci/pci.c
> +++ b/arch/ia64/pci/pci.c
> @@ -115,29 +115,12 @@ struct pci_ops pci_root_ops = {
>   	.write = pci_write,
>   };
>
> -/* Called by ACPI when it finds a new root bus.  */
> -
> -static struct pci_controller *alloc_pci_controller(int seg)
> -{
> -	struct pci_controller *controller;
> -
> -	controller = kzalloc(sizeof(*controller), GFP_KERNEL);
> -	if (!controller)
> -		return NULL;
> -
> -	controller->segment = seg;
> -	return controller;
> -}
> -
>   struct pci_root_info {
> +	struct pci_controller controller;
>   	struct acpi_device *bridge;
> -	struct pci_controller *controller;
>   	struct list_head resources;
> -	struct resource *res;
> -	resource_size_t *res_offset;
> -	unsigned int res_num;
>   	struct list_head io_resources;
> -	char *name;
> +	char name[16];
>   };
>
>   static unsigned int
> @@ -168,11 +151,11 @@ new_space (u64 phys_base, int sparse)
>   	return i;
>   }
>
> -static u64 add_io_space(struct pci_root_info *info,
> -			struct acpi_resource_address64 *addr)
> +static int add_io_space(struct device *dev, struct pci_root_info *info,
> +			struct resource_entry *entry)
>   {
>   	struct iospace_resource *iospace;
> -	struct resource *resource;
> +	struct resource *resource, *res = entry->res;
>   	char *name;
>   	unsigned long base, min, max, base_port;
>   	unsigned int sparse = 0, space_nr, len;
> @@ -180,27 +163,24 @@ static u64 add_io_space(struct pci_root_info *info,
>   	len = strlen(info->name) + 32;
>   	iospace = kzalloc(sizeof(*iospace) + len, GFP_KERNEL);
>   	if (!iospace) {
> -		dev_err(&info->bridge->dev,
> -				"PCI: No memory for %s I/O port space\n",
> -				info->name);
> -		goto out;
> +		dev_err(dev, "PCI: No memory for %s I/O port space\n",
> +			info->name);
> +		return -ENOMEM;
>   	}
>
> -	name = (char *)(iospace + 1);
> -
> -	min = addr->address.minimum;
> -	max = min + addr->address.address_length - 1;
> -	if (addr->info.io.translation_type == ACPI_SPARSE_TRANSLATION)
> +	if (res->flags & IORESOURCE_IO_SPARSE)
>   		sparse = 1;
> -
> -	space_nr = new_space(addr->address.translation_offset, sparse);
> +	space_nr = new_space(entry->offset, sparse);
>   	if (space_nr == ~0)
>   		goto free_resource;
>
> +	name = (char *)(iospace + 1);
> +	min = res->start - entry->offset;
> +	max = res->end - entry->offset;
>   	base = __pa(io_space[space_nr].mmio_base);
>   	base_port = IO_SPACE_BASE(space_nr);
>   	snprintf(name, len, "%s I/O Ports %08lx-%08lx", info->name,
> -		base_port + min, base_port + max);
> +		 base_port + min, base_port + max);
>
>   	/*
>   	 * The SDM guarantees the legacy 0-64K space is sparse, but if the
> @@ -216,153 +196,195 @@ static u64 add_io_space(struct pci_root_info *info,
>   	resource->start = base + (sparse ? IO_SPACE_SPARSE_ENCODING(min) : min);
>   	resource->end   = base + (sparse ? IO_SPACE_SPARSE_ENCODING(max) : max);
>   	if (insert_resource(&iomem_resource, resource)) {
> -		dev_err(&info->bridge->dev,
> -				"can't allocate host bridge io space resource  %pR\n",
> -				resource);
> +		dev_err(dev,
> +			"can't allocate host bridge io space resource  %pR\n",
> +			resource);
>   		goto free_resource;
>   	}
>
> +	entry->offset = base_port;
> +	res->start = min + base_port;
> +	res->end = max + base_port;
>   	list_add_tail(&iospace->list, &info->io_resources);
> -	return base_port;
> +
> +	return 0;
>
>   free_resource:
>   	kfree(iospace);
> -out:
> -	return ~0;
> +	return -ENOSPC;
> +}
> +
> +/*
> + * An IO port or MMIO resource assigned to a PCI host bridge may be
> + * consumed by the host bridge itself or available to its child
> + * bus/devices. The ACPI specification defines a bit (Producer/Consumer)
> + * to tell whether the resource is consumed by the host bridge itself,
> + * but firmware hasn't used that bit consistently, so we can't rely on it.
> + *
> + * On x86 and IA64 platforms, all IO port and MMIO resources are assumed
> + * to be available to child bus/devices except one special case:
> + *     IO port [0xCF8-0xCFF] is consumed by the host bridge itself
> + *     to access PCI configuration space.
> + *
> + * So explicitly filter out PCI CFG IO ports[0xCF8-0xCFF].
> + */
> +static bool resource_is_pcicfg_ioport(struct resource *res)
> +{
> +	return (res->flags & IORESOURCE_IO) &&
> +		res->start == 0xCF8 && res->end == 0xCFF;
>   }
>
> -static acpi_status resource_to_window(struct acpi_resource *resource,
> -				      struct acpi_resource_address64 *addr)
> +static int
> +probe_pci_root_info(struct pci_root_info *info, struct acpi_device *device,
> +		    int busnum, int domain)
>   {
> -	acpi_status status;
> +	int ret;
> +	struct list_head *list = &info->resources;
> +	struct resource_entry *entry, *tmp;
>
> -	/*
> -	 * We're only interested in _CRS descriptors that are
> -	 *	- address space descriptors for memory or I/O space
> -	 *	- non-zero size
> -	 */
> -	status = acpi_resource_to_address64(resource, addr);
> -	if (ACPI_SUCCESS(status) &&
> -	    (addr->resource_type == ACPI_MEMORY_RANGE ||
> -	     addr->resource_type == ACPI_IO_RANGE) &&
> -	    addr->address.address_length)
> -		return AE_OK;
> -
> -	return AE_ERROR;
> -}
> -
> -static acpi_status count_window(struct acpi_resource *resource, void *data)
> -{
> -	unsigned int *windows = (unsigned int *) data;
> -	struct acpi_resource_address64 addr;
> -	acpi_status status;
> -
> -	status = resource_to_window(resource, &addr);
> -	if (ACPI_SUCCESS(status))
> -		(*windows)++;
> -
> -	return AE_OK;
> -}
> -
> -static acpi_status add_window(struct acpi_resource *res, void *data)
> -{
> -	struct pci_root_info *info = data;
> -	struct resource *resource;
> -	struct acpi_resource_address64 addr;
> -	acpi_status status;
> -	unsigned long flags, offset = 0;
> -	struct resource *root;
> -
> -	/* Return AE_OK for non-window resources to keep scanning for more */
> -	status = resource_to_window(res, &addr);
> -	if (!ACPI_SUCCESS(status))
> -		return AE_OK;
> -
> -	if (addr.resource_type == ACPI_MEMORY_RANGE) {
> -		flags = IORESOURCE_MEM;
> -		root = &iomem_resource;
> -		offset = addr.address.translation_offset;
> -	} else if (addr.resource_type == ACPI_IO_RANGE) {
> -		flags = IORESOURCE_IO;
> -		root = &ioport_resource;
> -		offset = add_io_space(info, &addr);
> -		if (offset == ~0)
> -			return AE_OK;
> -	} else
> -		return AE_OK;
> -
> -	resource = &info->res[info->res_num];
> -	resource->name = info->name;
> -	resource->flags = flags;
> -	resource->start = addr.address.minimum + offset;
> -	resource->end = resource->start + addr.address.address_length - 1;
> -	info->res_offset[info->res_num] = offset;
> -
> -	if (insert_resource(root, resource)) {
> -		dev_err(&info->bridge->dev,
> -			"can't allocate host bridge window %pR\n",
> -			resource);
> -	} else {
> -		if (offset)
> -			dev_info(&info->bridge->dev, "host bridge window %pR "
> -				 "(PCI address [%#llx-%#llx])\n",
> -				 resource,
> -				 resource->start - offset,
> -				 resource->end - offset);
> -		else
> -			dev_info(&info->bridge->dev,
> -				 "host bridge window %pR\n", resource);
> -	}
> -	/* HP's firmware has a hack to work around a Windows bug.
> -	 * Ignore these tiny memory ranges */
> -	if (!((resource->flags & IORESOURCE_MEM) &&
> -	      (resource->end - resource->start < 16)))
> -		pci_add_resource_offset(&info->resources, resource,
> -					info->res_offset[info->res_num]);
> +	ret = acpi_dev_get_resources(device, list,
> +				     acpi_dev_filter_resource_type_cb,
> +				     (void *)(IORESOURCE_IO | IORESOURCE_MEM));
> +	if (ret < 0)
> +		dev_warn(&device->dev,
> +			 "failed to parse _CRS method, error code %d\n", ret);
> +	else if (ret == 0)
> +		dev_dbg(&device->dev,
> +			"no IO and memory resources present in _CRS\n");
> +	else
> +		resource_list_for_each_entry_safe(entry, tmp, list) {
> +			if ((entry->res->flags & IORESOURCE_DISABLED) ||
> +			    resource_is_pcicfg_ioport(entry->res))
> +				resource_list_destroy_entry(entry);
> +			else
> +				entry->res->name = info->name;
> +		}
>
> -	info->res_num++;
> -	return AE_OK;
> +	return ret;
>   }
>
> -static void free_pci_root_info_res(struct pci_root_info *info)
> -{
> -	struct iospace_resource *iospace, *tmp;
> +static void validate_resources(struct device *dev, struct list_head *resources,
> +			       unsigned long type)
> +{
> +	LIST_HEAD(list);
> +	struct resource *res1, *res2, *root = NULL;
> +	struct resource_entry *tmp, *entry, *entry2;
> +
> +	BUG_ON((type & (IORESOURCE_MEM | IORESOURCE_IO)) == 0);
> +	root = (type & IORESOURCE_MEM) ? &iomem_resource : &ioport_resource;
> +
> +	list_splice_init(resources, &list);
> +	resource_list_for_each_entry_safe(entry, tmp, &list) {
> +		bool free = false;
> +		resource_size_t end;
> +
> +		res1 = entry->res;
> +		if (!(res1->flags & type))
> +			goto next;
> +
> +		/* Exclude non-addressable range or non-addressable portion */
> +		end = min(res1->end, root->end);
> +		if (end <= res1->start) {
> +			dev_info(dev, "host bridge window %pR (ignored, not CPU addressable)\n",
> +				 res1);
> +			free = true;
> +			goto next;
> +		} else if (res1->end != end) {
> +			dev_info(dev, "host bridge window %pR ([%#llx-%#llx] ignored, not CPU addressable)\n",
> +				 res1, (unsigned long long)end + 1,
> +				 (unsigned long long)res1->end);
> +			res1->end = end;
> +		}
>
> -	list_for_each_entry_safe(iospace, tmp, &info->io_resources, list)
> -		kfree(iospace);
> +		resource_list_for_each_entry(entry2, resources) {
> +			res2 = entry2->res;
> +			if (!(res2->flags & type))
> +				continue;
> +
> +			/*
> +			 * I don't like throwing away windows because then
> +			 * our resources no longer match the ACPI _CRS, but
> +			 * the kernel resource tree doesn't allow overlaps.
> +			 */
> +			if (resource_overlaps(res1, res2)) {
> +				res2->start = min(res1->start, res2->start);
> +				res2->end = max(res1->end, res2->end);
> +				dev_info(dev, "host bridge window expanded to %pR; %pR ignored\n",
> +					 res2, res1);
> +				free = true;
> +				goto next;
> +			}
> +		}
>
> -	kfree(info->name);
> -	kfree(info->res);
> -	info->res = NULL;
> -	kfree(info->res_offset);
> -	info->res_offset = NULL;
> -	info->res_num = 0;
> -	kfree(info->controller);
> -	info->controller = NULL;
> +next:
> +		resource_list_del(entry);
> +		if (free)
> +			resource_list_free_entry(entry);
> +		else
> +			resource_list_add_tail(entry, resources);
> +	}
> +}
> +
> +static void add_resources(struct pci_root_info *info, struct device *dev)
> +{
> +	struct resource_entry *entry, *tmp;
> +	struct resource *res, *conflict, *root = NULL;
> +	struct list_head *list = &info->resources;
> +
> +	validate_resources(dev, list, IORESOURCE_MEM);
> +	validate_resources(dev, list, IORESOURCE_IO);
> +
> +	resource_list_for_each_entry_safe(entry, tmp, list) {
> +		res = entry->res;
> +		if (res->flags & IORESOURCE_MEM) {
> +			root = &iomem_resource;
> +			/*
> +			 * HP's firmware has a hack to work around a Windows
> +			 * bug. Ignore these tiny memory ranges.
> +			 */
> +			if (resource_size(res) <= 16) {
> +				resource_list_destroy_entry(entry);
> +				continue;
> +			}
> +		} else if (res->flags & IORESOURCE_IO) {
> +			root = &ioport_resource;
> +			if (add_io_space(&info->bridge->dev, info, entry)) {
> +				resource_list_destroy_entry(entry);
> +				continue;
> +			}
> +		} else {
> +			BUG_ON(res);
> +		}
> +
> +		conflict = insert_resource_conflict(root, res);
> +		if (conflict) {
> +			dev_info(dev,
> +				 "ignoring host bridge window %pR (conflicts with %s %pR)\n",
> +				 res, conflict->name, conflict);
> +			resource_list_destroy_entry(entry);
> +		}
> +	}
>   }
>
>   static void __release_pci_root_info(struct pci_root_info *info)
>   {
> -	int i;
>   	struct resource *res;
> -	struct iospace_resource *iospace;
> +	struct iospace_resource *iospace, *tmp;
> +	struct resource_entry *entry, *tentry;
>
> -	list_for_each_entry(iospace, &info->io_resources, list)
> +	list_for_each_entry_safe(iospace, tmp, &info->io_resources, list) {
>   		release_resource(&iospace->res);
> +		kfree(iospace);
> +	}
>
> -	for (i = 0; i < info->res_num; i++) {
> -		res = &info->res[i];
> -
> -		if (!res->parent)
> -			continue;
> -
> -		if (!(res->flags & (IORESOURCE_MEM | IORESOURCE_IO)))
> -			continue;
> -
> -		release_resource(res);
> +	resource_list_for_each_entry_safe(entry, tentry, &info->resources) {
> +		res = entry->res;
> +		if (res->parent &&
> +		    (res->flags & (IORESOURCE_MEM | IORESOURCE_IO)))
> +			release_resource(res);
> +		resource_list_destroy_entry(entry);
>   	}
>
> -	free_pci_root_info_res(info);
>   	kfree(info);
>   }
>
> @@ -373,99 +395,49 @@ static void release_pci_root_info(struct pci_host_bridge *bridge)
>   	__release_pci_root_info(info);
>   }
>
> -static int
> -probe_pci_root_info(struct pci_root_info *info, struct acpi_device *device,
> -		int busnum, int domain)
> -{
> -	char *name;
> -
> -	name = kmalloc(16, GFP_KERNEL);
> -	if (!name)
> -		return -ENOMEM;
> -
> -	sprintf(name, "PCI Bus %04x:%02x", domain, busnum);
> -	info->bridge = device;
> -	info->name = name;
> -
> -	acpi_walk_resources(device->handle, METHOD_NAME__CRS, count_window,
> -			&info->res_num);
> -	if (info->res_num) {
> -		info->res =
> -			kzalloc_node(sizeof(*info->res) * info->res_num,
> -				     GFP_KERNEL, info->controller->node);
> -		if (!info->res) {
> -			kfree(name);
> -			return -ENOMEM;
> -		}
> -
> -		info->res_offset =
> -			kzalloc_node(sizeof(*info->res_offset) * info->res_num,
> -					GFP_KERNEL, info->controller->node);
> -		if (!info->res_offset) {
> -			kfree(name);
> -			kfree(info->res);
> -			info->res = NULL;
> -			return -ENOMEM;
> -		}
> -
> -		info->res_num = 0;
> -		acpi_walk_resources(device->handle, METHOD_NAME__CRS,
> -			add_window, info);
> -	} else
> -		kfree(name);
> -
> -	return 0;
> -}
> -
>   struct pci_bus *pci_acpi_scan_root(struct acpi_pci_root *root)
>   {
>   	struct acpi_device *device = root->device;
>   	int domain = root->segment;
>   	int bus = root->secondary.start;
> -	struct pci_controller *controller;
> -	struct pci_root_info *info = NULL;
> -	int busnum = root->secondary.start;
> +	struct pci_root_info *info;
>   	struct pci_bus *pbus;
>   	int ret;
>
> -	controller = alloc_pci_controller(domain);
> -	if (!controller)
> -		return NULL;
> -
> -	controller->companion = device;
> -	controller->node = acpi_get_node(device->handle);
> -
>   	info = kzalloc(sizeof(*info), GFP_KERNEL);
>   	if (!info) {
>   		dev_err(&device->dev,
> -				"pci_bus %04x:%02x: ignored (out of memory)\n",
> -				domain, busnum);
> -		kfree(controller);
> +			"pci_bus %04x:%02x: ignored (out of memory)\n",
> +			domain, bus);
>   		return NULL;
>   	}
>
> -	info->controller = controller;
> -	INIT_LIST_HEAD(&info->io_resources);
> +	info->controller.segment = domain;
> +	info->controller.companion = device;
> +	info->controller.node = acpi_get_node(device->handle);
> +	info->bridge = device;
>   	INIT_LIST_HEAD(&info->resources);
> +	INIT_LIST_HEAD(&info->io_resources);
> +	snprintf(info->name, sizeof(info->name),
> +		 "PCI Bus %04x:%02x", domain, bus);
>
> -	ret = probe_pci_root_info(info, device, busnum, domain);
> -	if (ret) {
> -		kfree(info->controller);
> +	ret = probe_pci_root_info(info, device, bus, domain);
> +	if (ret <= 0) {
>   		kfree(info);
>   		return NULL;
>   	}
> -	/* insert busn resource at first */
> +	add_resources(info, &info->bridge->dev);
>   	pci_add_resource(&info->resources, &root->secondary);
> +
>   	/*
>   	 * See arch/x86/pci/acpi.c.
>   	 * The desired pci bus might already be scanned in a quirk. We
>   	 * should handle the case here, but it appears that IA64 hasn't
>   	 * such quirk. So we just ignore the case now.
>   	 */
> -	pbus = pci_create_root_bus(NULL, bus, &pci_root_ops, controller,
> -				   &info->resources);
> +	pbus = pci_create_root_bus(NULL, bus, &pci_root_ops,
> +				   &info->controller, &info->resources);
>   	if (!pbus) {
> -		pci_free_resource_list(&info->resources);
>   		__release_pci_root_info(info);
>   		return NULL;
>   	}
>

[RFC v2 3/7] ia64/PCI: Use common struct resource_entry to replace struct iospace_resource

[Jiang Liu]

Use common struct resource_entry to replace private
struct iospace_resource.

Signed-off-by: Jiang Liu <jiang.liu@linux.intel.com>
---
 arch/ia64/include/asm/pci.h |    5 -----
 arch/ia64/pci/pci.c         |   17 ++++++++---------
 2 files changed, 8 insertions(+), 14 deletions(-)

diff --git a/arch/ia64/include/asm/pci.h b/arch/ia64/include/asm/pci.h
index 52af5ed9f60b..5c10e0ec48d4 100644
--- a/arch/ia64/include/asm/pci.h
+++ b/arch/ia64/include/asm/pci.h
@@ -83,11 +83,6 @@ extern int pci_mmap_legacy_page_range(struct pci_bus *bus,
 #define pci_legacy_read platform_pci_legacy_read
 #define pci_legacy_write platform_pci_legacy_write
 
-struct iospace_resource {
-	struct list_head list;
-	struct resource res;
-};
-
 struct pci_controller {
 	struct acpi_device *companion;
 	void *iommu;
diff --git a/arch/ia64/pci/pci.c b/arch/ia64/pci/pci.c
index 23689d4c37ae..4af1b52c7a44 100644
--- a/arch/ia64/pci/pci.c
+++ b/arch/ia64/pci/pci.c
@@ -154,14 +154,14 @@ new_space (u64 phys_base, int sparse)
 static int add_io_space(struct device *dev, struct pci_root_info *info,
 			struct resource_entry *entry)
 {
-	struct iospace_resource *iospace;
+	struct resource_entry *iospace;
 	struct resource *resource, *res = entry->res;
 	char *name;
 	unsigned long base, min, max, base_port;
 	unsigned int sparse = 0, space_nr, len;
 
 	len = strlen(info->name) + 32;
-	iospace = kzalloc(sizeof(*iospace) + len, GFP_KERNEL);
+	iospace = resource_list_create_entry(NULL, len);
 	if (!iospace) {
 		dev_err(dev, "PCI: No memory for %s I/O port space\n",
 			info->name);
@@ -190,7 +190,7 @@ static int add_io_space(struct device *dev, struct pci_root_info *info,
 	if (space_nr == 0)
 		sparse = 1;
 
-	resource = &iospace->res;
+	resource = iospace->res;
 	resource->name  = name;
 	resource->flags = IORESOURCE_MEM;
 	resource->start = base + (sparse ? IO_SPACE_SPARSE_ENCODING(min) : min);
@@ -205,12 +205,12 @@ static int add_io_space(struct device *dev, struct pci_root_info *info,
 	entry->offset = base_port;
 	res->start = min + base_port;
 	res->end = max + base_port;
-	list_add_tail(&iospace->list, &info->io_resources);
+	resource_list_add_tail(iospace, &info->io_resources);
 
 	return 0;
 
 free_resource:
-	kfree(iospace);
+	resource_list_free_entry(iospace);
 	return -ENOSPC;
 }
 
@@ -369,12 +369,11 @@ static void add_resources(struct pci_root_info *info, struct device *dev)
 static void __release_pci_root_info(struct pci_root_info *info)
 {
 	struct resource *res;
-	struct iospace_resource *iospace, *tmp;
 	struct resource_entry *entry, *tentry;
 
-	list_for_each_entry_safe(iospace, tmp, &info->io_resources, list) {
-		release_resource(&iospace->res);
-		kfree(iospace);
+	resource_list_for_each_entry_safe(entry, tentry, &info->io_resources) {
+		release_resource(entry->res);
+		resource_list_destroy_entry(entry);
 	}
 
 	resource_list_for_each_entry_safe(entry, tentry, &info->resources) {
-- 
1.7.10.4

[RFC v2 4/7] x86/PCI: Rename struct pci_sysdata as struct pci_controller

[Jiang Liu]

Rename struct pci_sysdata as struct pci_controller, so we could share
common code between IA64 and x86 later.

Signed-off-by: Jiang Liu <jiang.liu@linux.intel.com>
---
 arch/x86/include/asm/pci.h    |   13 +++++++------
 arch/x86/include/asm/pci_64.h |    4 ++--
 arch/x86/pci/acpi.c           |    8 ++++----
 arch/x86/pci/common.c         |    2 +-
 4 files changed, 14 insertions(+), 13 deletions(-)

diff --git a/arch/x86/include/asm/pci.h b/arch/x86/include/asm/pci.h
index 4e370a5d8117..243dafd86f87 100644
--- a/arch/x86/include/asm/pci.h
+++ b/arch/x86/include/asm/pci.h
@@ -11,15 +11,15 @@
 
 #ifdef __KERNEL__
 
-struct pci_sysdata {
-	int		domain;		/* PCI domain */
-	int		node;		/* NUMA node */
+struct pci_controller {
 #ifdef CONFIG_ACPI
 	struct acpi_device *companion;	/* ACPI companion device */
 #endif
 #ifdef CONFIG_X86_64
 	void		*iommu;		/* IOMMU private data */
 #endif
+	int		segment;	/* PCI domain */
+	int		node;		/* NUMA node */
 };
 
 extern int pci_routeirq;
@@ -31,8 +31,9 @@ extern int noioapicreroute;
 #ifdef CONFIG_PCI_DOMAINS
 static inline int pci_domain_nr(struct pci_bus *bus)
 {
-	struct pci_sysdata *sd = bus->sysdata;
-	return sd->domain;
+	struct pci_controller *sd = bus->sysdata;
+
+	return sd->segment;
 }
 
 static inline int pci_proc_domain(struct pci_bus *bus)
@@ -127,7 +128,7 @@ int setup_msi_irq(struct pci_dev *dev, struct msi_desc *msidesc,
 /* Returns the node based on pci bus */
 static inline int __pcibus_to_node(const struct pci_bus *bus)
 {
-	const struct pci_sysdata *sd = bus->sysdata;
+	const struct pci_controller *sd = bus->sysdata;
 
 	return sd->node;
 }
diff --git a/arch/x86/include/asm/pci_64.h b/arch/x86/include/asm/pci_64.h
index fe15cfb21b9b..dcbb6b52d4fd 100644
--- a/arch/x86/include/asm/pci_64.h
+++ b/arch/x86/include/asm/pci_64.h
@@ -6,13 +6,13 @@
 #ifdef CONFIG_CALGARY_IOMMU
 static inline void *pci_iommu(struct pci_bus *bus)
 {
-	struct pci_sysdata *sd = bus->sysdata;
+	struct pci_controller *sd = bus->sysdata;
 	return sd->iommu;
 }
 
 static inline void set_pci_iommu(struct pci_bus *bus, void *val)
 {
-	struct pci_sysdata *sd = bus->sysdata;
+	struct pci_controller *sd = bus->sysdata;
 	sd->iommu = val;
 }
 #endif /* CONFIG_CALGARY_IOMMU */
diff --git a/arch/x86/pci/acpi.c b/arch/x86/pci/acpi.c
index d93963340c3c..b34a2b660de3 100644
--- a/arch/x86/pci/acpi.c
+++ b/arch/x86/pci/acpi.c
@@ -10,7 +10,7 @@
 struct pci_root_info {
 	struct acpi_device *bridge;
 	char name[16];
-	struct pci_sysdata sd;
+	struct pci_controller sd;
 #ifdef	CONFIG_PCI_MMCONFIG
 	bool mcfg_added;
 	u16 segment;
@@ -384,7 +384,7 @@ struct pci_bus *pci_acpi_scan_root(struct acpi_pci_root *root)
 	LIST_HEAD(crs_res);
 	LIST_HEAD(resources);
 	struct pci_bus *bus;
-	struct pci_sysdata *sd;
+	struct pci_controller *sd;
 	int node;
 
 	if (pci_ignore_seg)
@@ -416,7 +416,7 @@ struct pci_bus *pci_acpi_scan_root(struct acpi_pci_root *root)
 	}
 
 	sd = &info->sd;
-	sd->domain = domain;
+	sd->segment = domain;
 	sd->node = node;
 	sd->companion = device;
 
@@ -482,7 +482,7 @@ struct pci_bus *pci_acpi_scan_root(struct acpi_pci_root *root)
 
 int pcibios_root_bridge_prepare(struct pci_host_bridge *bridge)
 {
-	struct pci_sysdata *sd = bridge->bus->sysdata;
+	struct pci_controller *sd = bridge->bus->sysdata;
 
 	ACPI_COMPANION_SET(&bridge->dev, sd->companion);
 	return 0;
diff --git a/arch/x86/pci/common.c b/arch/x86/pci/common.c
index 8fd6f44aee83..10f37d0ce5d8 100644
--- a/arch/x86/pci/common.c
+++ b/arch/x86/pci/common.c
@@ -475,7 +475,7 @@ void __init dmi_check_pciprobe(void)
 void pcibios_scan_root(int busnum)
 {
 	struct pci_bus *bus;
-	struct pci_sysdata *sd;
+	struct pci_controller *sd;
 	LIST_HEAD(resources);
 
 	sd = kzalloc(sizeof(*sd), GFP_KERNEL);
-- 
1.7.10.4

[RFC v2 5/7] PCI/ACPI: Consolidate common PCI host bridge code into ACPI core

[Jiang Liu]

Introduce common interface acpi_pci_root_create() and related data
structures to create PCI root bus for ACPI PCI host bridges. It will
be used to kill duplicated arch specific code for IA64 and x86. It may
also help ARM64 in future.

Tested-by: Tony Luck <tony.luck@intel.com>
Signed-off-by: Jiang Liu <jiang.liu@linux.intel.com>
---
 drivers/acpi/pci_root.c  |  214 ++++++++++++++++++++++++++++++++++++++++++++++
 include/linux/pci-acpi.h |   24 ++++++
 2 files changed, 238 insertions(+)

diff --git a/drivers/acpi/pci_root.c b/drivers/acpi/pci_root.c
index 1b5569c092c6..97c260959a54 100644
--- a/drivers/acpi/pci_root.c
+++ b/drivers/acpi/pci_root.c
@@ -656,6 +656,220 @@ static void acpi_pci_root_remove(struct acpi_device *device)
 	kfree(root);
 }
 
+static void acpi_pci_root_validate_resources(struct device *dev,
+					     struct list_head *resources,
+					     unsigned long type)
+{
+	LIST_HEAD(list);
+	struct resource *res1, *res2, *root = NULL;
+	struct resource_entry *tmp, *entry, *entry2;
+
+	BUG_ON((type & (IORESOURCE_MEM | IORESOURCE_IO)) == 0);
+	root = (type & IORESOURCE_MEM) ? &iomem_resource : &ioport_resource;
+
+	list_splice_init(resources, &list);
+	resource_list_for_each_entry_safe(entry, tmp, &list) {
+		bool free = false;
+		resource_size_t end;
+
+		res1 = entry->res;
+		if (!(res1->flags & type))
+			goto next;
+
+		/* Exclude non-addressable range or non-addressable portion */
+		end = min(res1->end, root->end);
+		if (end <= res1->start) {
+			dev_info(dev, "host bridge window %pR (ignored, not CPU addressable)\n",
+				 res1);
+			free = true;
+			goto next;
+		} else if (res1->end != end) {
+			dev_info(dev, "host bridge window %pR ([%#llx-%#llx] ignored, not CPU addressable)\n",
+				 res1, (unsigned long long)end + 1,
+				 (unsigned long long)res1->end);
+			res1->end = end;
+		}
+
+		resource_list_for_each_entry(entry2, resources) {
+			res2 = entry2->res;
+			if (!(res2->flags & type))
+				continue;
+
+			/*
+			 * I don't like throwing away windows because then
+			 * our resources no longer match the ACPI _CRS, but
+			 * the kernel resource tree doesn't allow overlaps.
+			 */
+			if (resource_overlaps(res1, res2)) {
+				res2->start = min(res1->start, res2->start);
+				res2->end = max(res1->end, res2->end);
+				dev_info(dev, "host bridge window expanded to %pR; %pR ignored\n",
+					 res2, res1);
+				free = true;
+				goto next;
+			}
+		}
+
+next:
+		resource_list_del(entry);
+		if (free)
+			resource_list_free_entry(entry);
+		else
+			resource_list_add_tail(entry, resources);
+	}
+}
+
+static int acpi_pci_probe_root_resources(struct acpi_pci_root_info_common *info)
+{
+	int ret;
+	struct list_head *list = &info->resources;
+	struct acpi_device *device = info->bridge;
+	struct resource_entry *entry, *tmp;
+	unsigned long flags;
+
+	flags = IORESOURCE_IO | IORESOURCE_MEM | IORESOURCE_MEM_8AND16BIT;
+	ret = acpi_dev_get_resources(device, list,
+				     acpi_dev_filter_resource_type_cb,
+				     (void *)flags);
+	if (ret < 0)
+		dev_warn(&device->dev,
+			 "failed to parse _CRS method, error code %d\n", ret);
+	else if (ret == 0)
+		dev_dbg(&device->dev,
+			"no IO and memory resources present in _CRS\n");
+	else {
+		resource_list_for_each_entry_safe(entry, tmp, list) {
+			if (entry->res->flags & IORESOURCE_DISABLED)
+				resource_list_destroy_entry(entry);
+			else
+				entry->res->name = info->name;
+		}
+		acpi_pci_root_validate_resources(&device->dev, list,
+						 IORESOURCE_MEM);
+		acpi_pci_root_validate_resources(&device->dev, list,
+						 IORESOURCE_IO);
+	}
+
+	return ret;
+}
+
+static void pci_acpi_root_add_resources(struct acpi_pci_root_info_common *info)
+{
+	struct resource_entry *entry, *tmp;
+	struct resource *res, *conflict, *root = NULL;
+
+	resource_list_for_each_entry_safe(entry, tmp, &info->resources) {
+		res = entry->res;
+		if (res->flags & IORESOURCE_MEM)
+			root = &iomem_resource;
+		else if (res->flags & IORESOURCE_IO)
+			root = &ioport_resource;
+		else
+			continue;
+
+		conflict = insert_resource_conflict(root, res);
+		if (conflict) {
+			dev_info(&info->bridge->dev,
+				 "ignoring host bridge window %pR (conflicts with %s %pR)\n",
+				 res, conflict->name, conflict);
+			resource_list_destroy_entry(entry);
+		}
+	}
+}
+
+static void __acpi_pci_root_release_info(struct acpi_pci_root_info_common *info)
+{
+	struct resource *res;
+	struct resource_entry *entry, *tmp;
+
+	if (!info)
+		return;
+
+	if (info->ops && info->ops->release_info)
+		info->ops->release_info(info);
+
+	resource_list_for_each_entry_safe(entry, tmp, &info->resources) {
+		res = entry->res;
+		if (res->parent &&
+		    (res->flags & (IORESOURCE_MEM | IORESOURCE_IO)))
+			release_resource(res);
+		resource_list_destroy_entry(entry);
+	}
+
+	kfree(info);
+}
+
+static void acpi_pci_root_release_info(struct pci_host_bridge *bridge)
+{
+	struct resource *res;
+	struct resource_entry *entry;
+
+	resource_list_for_each_entry(entry, &bridge->windows) {
+		res = entry->res;
+		if (res->parent &&
+		    (res->flags & (IORESOURCE_MEM | IORESOURCE_IO)))
+			release_resource(res);
+	}
+	__acpi_pci_root_release_info(bridge->release_data);
+}
+
+struct pci_bus *acpi_pci_root_create(struct acpi_pci_root *root,
+				     struct acpi_pci_root_ops *ops,
+				     size_t extra_size)
+{
+	int ret, busnum = root->secondary.start;
+	struct acpi_device *device = root->device;
+	struct acpi_pci_root_info_common *info;
+	struct pci_bus *bus;
+
+	info = kzalloc(sizeof(*info) + extra_size, GFP_KERNEL);
+	if (!info) {
+		dev_err(&device->dev,
+			"pci_bus %04x:%02x: ignored (out of memory)\n",
+			root->segment, busnum);
+		return NULL;
+	}
+
+	info->controller.segment = root->segment;
+	info->controller.node = acpi_get_node(device->handle);
+	info->controller.companion = device;
+	info->root = root;
+	info->bridge = device;
+	info->ops = ops;
+	INIT_LIST_HEAD(&info->resources);
+	snprintf(info->name, sizeof(info->name), "PCI Bus %04x:%02x",
+		 info->controller.segment, busnum);
+	if (ops->init_info && ops->init_info(info)) {
+		kfree(info);
+		return NULL;
+	}
+
+	ret = acpi_pci_probe_root_resources(info);
+	if (ops->prepare_resources)
+		ret = ops->prepare_resources(info, ret);
+	if (ret < 0)
+		goto out_release_info;
+	else if (ret > 0)
+		pci_acpi_root_add_resources(info);
+	pci_add_resource(&info->resources, &root->secondary);
+
+	bus = pci_create_root_bus(NULL, busnum, ops->pci_ops,
+				  &info->controller, &info->resources);
+	if (bus) {
+		pci_scan_child_bus(bus);
+		pci_set_host_bridge_release(to_pci_host_bridge(bus->bridge),
+					    acpi_pci_root_release_info, info);
+		if (info->controller.node != NUMA_NO_NODE)
+			dev_printk(KERN_DEBUG, &bus->dev, "on NUMA node %d\n",
+				   info->controller.node);
+		return bus;
+	}
+
+out_release_info:
+	__acpi_pci_root_release_info(info);
+	return NULL;
+}
+
 void __init acpi_pci_root_init(void)
 {
 	acpi_hest_init();
diff --git a/include/linux/pci-acpi.h b/include/linux/pci-acpi.h
index a965efa52152..a292ee33d74b 100644
--- a/include/linux/pci-acpi.h
+++ b/include/linux/pci-acpi.h
@@ -52,6 +52,30 @@ static inline acpi_handle acpi_pci_get_bridge_handle(struct pci_bus *pbus)
 	return ACPI_HANDLE(dev);
 }
 
+struct acpi_pci_root;
+struct acpi_pci_root_ops;
+
+struct acpi_pci_root_info_common {
+	struct pci_controller		controller;
+	struct acpi_pci_root		*root;
+	struct acpi_device		*bridge;
+	struct acpi_pci_root_ops	*ops;
+	struct list_head		resources;
+	char				name[16];
+};
+
+struct acpi_pci_root_ops {
+	struct pci_ops *pci_ops;
+	int (*init_info)(struct acpi_pci_root_info_common *info);
+	void (*release_info)(struct acpi_pci_root_info_common *info);
+	int (*prepare_resources)(struct acpi_pci_root_info_common *info,
+				 int status);
+};
+
+extern struct pci_bus *acpi_pci_root_create(struct acpi_pci_root *root,
+					    struct acpi_pci_root_ops *ops,
+					    size_t extra_size);
+
 void acpi_pci_add_bus(struct pci_bus *bus);
 void acpi_pci_remove_bus(struct pci_bus *bus);
 
-- 
1.7.10.4

[RFC v2 5/7] PCI/ACPI: Consolidate common PCI host bridge code into ACPI core

[Hanjun Guo]

Hi Jiang,

I have comments inline.

On 2015?05?05? 10:46, Jiang Liu wrote:
> Introduce common interface acpi_pci_root_create() and related data
> structures to create PCI root bus for ACPI PCI host bridges. It will
> be used to kill duplicated arch specific code for IA64 and x86. It may
> also help ARM64 in future.
>
> Tested-by: Tony Luck <tony.luck@intel.com>
> Signed-off-by: Jiang Liu <jiang.liu@linux.intel.com>
> ---
>   drivers/acpi/pci_root.c  |  214 ++++++++++++++++++++++++++++++++++++++++++++++
>   include/linux/pci-acpi.h |   24 ++++++
>   2 files changed, 238 insertions(+)
>
> diff --git a/drivers/acpi/pci_root.c b/drivers/acpi/pci_root.c
> index 1b5569c092c6..97c260959a54 100644
> --- a/drivers/acpi/pci_root.c
> +++ b/drivers/acpi/pci_root.c
> @@ -656,6 +656,220 @@ static void acpi_pci_root_remove(struct acpi_device *device)
>   	kfree(root);
>   }
>
> +static void acpi_pci_root_validate_resources(struct device *dev,
> +					     struct list_head *resources,
> +					     unsigned long type)
> +{
> +	LIST_HEAD(list);
> +	struct resource *res1, *res2, *root = NULL;
> +	struct resource_entry *tmp, *entry, *entry2;
> +
> +	BUG_ON((type & (IORESOURCE_MEM | IORESOURCE_IO)) == 0);
> +	root = (type & IORESOURCE_MEM) ? &iomem_resource : &ioport_resource;
> +
> +	list_splice_init(resources, &list);
> +	resource_list_for_each_entry_safe(entry, tmp, &list) {
> +		bool free = false;
> +		resource_size_t end;
> +
> +		res1 = entry->res;
> +		if (!(res1->flags & type))
> +			goto next;
> +
> +		/* Exclude non-addressable range or non-addressable portion */
> +		end = min(res1->end, root->end);
> +		if (end <= res1->start) {
> +			dev_info(dev, "host bridge window %pR (ignored, not CPU addressable)\n",
> +				 res1);
> +			free = true;
> +			goto next;
> +		} else if (res1->end != end) {
> +			dev_info(dev, "host bridge window %pR ([%#llx-%#llx] ignored, not CPU addressable)\n",
> +				 res1, (unsigned long long)end + 1,
> +				 (unsigned long long)res1->end);
> +			res1->end = end;
> +		}
> +
> +		resource_list_for_each_entry(entry2, resources) {
> +			res2 = entry2->res;
> +			if (!(res2->flags & type))
> +				continue;
> +
> +			/*
> +			 * I don't like throwing away windows because then
> +			 * our resources no longer match the ACPI _CRS, but
> +			 * the kernel resource tree doesn't allow overlaps.
> +			 */
> +			if (resource_overlaps(res1, res2)) {
> +				res2->start = min(res1->start, res2->start);
> +				res2->end = max(res1->end, res2->end);
> +				dev_info(dev, "host bridge window expanded to %pR; %pR ignored\n",
> +					 res2, res1);
> +				free = true;
> +				goto next;
> +			}
> +		}
> +
> +next:
> +		resource_list_del(entry);
> +		if (free)
> +			resource_list_free_entry(entry);
> +		else
> +			resource_list_add_tail(entry, resources);
> +	}
> +}
> +
> +static int acpi_pci_probe_root_resources(struct acpi_pci_root_info_common *info)
> +{
> +	int ret;
> +	struct list_head *list = &info->resources;
> +	struct acpi_device *device = info->bridge;
> +	struct resource_entry *entry, *tmp;
> +	unsigned long flags;
> +
> +	flags = IORESOURCE_IO | IORESOURCE_MEM | IORESOURCE_MEM_8AND16BIT;
> +	ret = acpi_dev_get_resources(device, list,
> +				     acpi_dev_filter_resource_type_cb,
> +				     (void *)flags);
> +	if (ret < 0)
> +		dev_warn(&device->dev,
> +			 "failed to parse _CRS method, error code %d\n", ret);
> +	else if (ret == 0)
> +		dev_dbg(&device->dev,
> +			"no IO and memory resources present in _CRS\n");
> +	else {
> +		resource_list_for_each_entry_safe(entry, tmp, list) {
> +			if (entry->res->flags & IORESOURCE_DISABLED)
> +				resource_list_destroy_entry(entry);
> +			else
> +				entry->res->name = info->name;
> +		}
> +		acpi_pci_root_validate_resources(&device->dev, list,
> +						 IORESOURCE_MEM);
> +		acpi_pci_root_validate_resources(&device->dev, list,
> +						 IORESOURCE_IO);
> +	}
> +
> +	return ret;
> +}
> +
> +static void pci_acpi_root_add_resources(struct acpi_pci_root_info_common *info)
> +{
> +	struct resource_entry *entry, *tmp;
> +	struct resource *res, *conflict, *root = NULL;
> +
> +	resource_list_for_each_entry_safe(entry, tmp, &info->resources) {
> +		res = entry->res;
> +		if (res->flags & IORESOURCE_MEM)
> +			root = &iomem_resource;
> +		else if (res->flags & IORESOURCE_IO)
> +			root = &ioport_resource;
> +		else
> +			continue;
> +
> +		conflict = insert_resource_conflict(root, res);
> +		if (conflict) {
> +			dev_info(&info->bridge->dev,
> +				 "ignoring host bridge window %pR (conflicts with %s %pR)\n",
> +				 res, conflict->name, conflict);
> +			resource_list_destroy_entry(entry);
> +		}
> +	}
> +}
> +
> +static void __acpi_pci_root_release_info(struct acpi_pci_root_info_common *info)
> +{
> +	struct resource *res;
> +	struct resource_entry *entry, *tmp;
> +
> +	if (!info)
> +		return;
> +
> +	if (info->ops && info->ops->release_info)
> +		info->ops->release_info(info);
> +
> +	resource_list_for_each_entry_safe(entry, tmp, &info->resources) {
> +		res = entry->res;
> +		if (res->parent &&
> +		    (res->flags & (IORESOURCE_MEM | IORESOURCE_IO)))
> +			release_resource(res);
> +		resource_list_destroy_entry(entry);
> +	}
> +
> +	kfree(info);
> +}
> +
> +static void acpi_pci_root_release_info(struct pci_host_bridge *bridge)
> +{
> +	struct resource *res;
> +	struct resource_entry *entry;
> +
> +	resource_list_for_each_entry(entry, &bridge->windows) {
> +		res = entry->res;
> +		if (res->parent &&
> +		    (res->flags & (IORESOURCE_MEM | IORESOURCE_IO)))
> +			release_resource(res);
> +	}
> +	__acpi_pci_root_release_info(bridge->release_data);
> +}
> +
> +struct pci_bus *acpi_pci_root_create(struct acpi_pci_root *root,
> +				     struct acpi_pci_root_ops *ops,
> +				     size_t extra_size)
> +{
> +	int ret, busnum = root->secondary.start;
> +	struct acpi_device *device = root->device;
> +	struct acpi_pci_root_info_common *info;
> +	struct pci_bus *bus;
> +
> +	info = kzalloc(sizeof(*info) + extra_size, GFP_KERNEL);

For x86, the memory is allocated on its local numa node if memory
is available by using kzalloc_node(), and if
node = acpi_get_node(device->handle) is NUMA_NO_NODE, the code will
get the numa node info by using x86_pci_root_bus_node() (which you
consolidate them as a function pci_acpi_root_get_node() in later
patch).

but the code here just ignore that information, so the code
here has functional change for x86 code since you didn't use numa
information.

I'm not sure how frequently used for the info after init, so
not sure about the performance impact, but I think we should
keep consistence with the code behavior as before.

Further more, there is a implicit cast for
struct acpi_pci_root_info_common *info to arch specific
struct pci_root_info *info by using extra size, it's not
easy to understand (at least for me :) ), so how about
alloc the memory in arch specific function, and pass
struct acpi_pci_root_info_common *info fr this function?

other than that, this code is pretty good, I reworked ARM64
ACPI based PCI host bridge init code, and this patch simplified
the coed a lot, will have a test tomorrow and let you know
the result.

Thanks
Hanjun

[RFC v2 5/7] PCI/ACPI: Consolidate common PCI host bridge code into ACPI core

[Jiang Liu]

On 2015/5/11 21:36, Hanjun Guo wrote:
> Hi Jiang,
> 
> I have comments inline.
> 
> On 2015?05?05? 10:46, Jiang Liu wrote:
>> Introduce common interface acpi_pci_root_create() and related data
>> structures to create PCI root bus for ACPI PCI host bridges. It will
>> be used to kill duplicated arch specific code for IA64 and x86. It may
>> also help ARM64 in future.
>>
>> Tested-by: Tony Luck <tony.luck@intel.com>
>> Signed-off-by: Jiang Liu <jiang.liu@linux.intel.com>
>> ---
>> +struct pci_bus *acpi_pci_root_create(struct acpi_pci_root *root,
>> +                     struct acpi_pci_root_ops *ops,
>> +                     size_t extra_size)
>> +{
>> +    int ret, busnum = root->secondary.start;
>> +    struct acpi_device *device = root->device;
>> +    struct acpi_pci_root_info_common *info;
>> +    struct pci_bus *bus;
>> +
>> +    info = kzalloc(sizeof(*info) + extra_size, GFP_KERNEL);
> 
> For x86, the memory is allocated on its local numa node if memory
> is available by using kzalloc_node(), and if
> node = acpi_get_node(device->handle) is NUMA_NO_NODE, the code will
> get the numa node info by using x86_pci_root_bus_node() (which you
> consolidate them as a function pci_acpi_root_get_node() in later
> patch).
> 
> but the code here just ignore that information, so the code
> here has functional change for x86 code since you didn't use numa
> information.
> 
> I'm not sure how frequently used for the info after init, so
> not sure about the performance impact, but I think we should
> keep consistence with the code behavior as before.
Hi Hanjun,
	Good catch, will change code to respect NUMA node
info when allocating memory.

> 
> Further more, there is a implicit cast for
> struct acpi_pci_root_info_common *info to arch specific
> struct pci_root_info *info by using extra size, it's not
> easy to understand (at least for me :) ), so how about
> alloc the memory in arch specific function, and pass
> struct acpi_pci_root_info_common *info fr this function?
Good suggestion.
Thanks!
Gerry

> other than that, this code is pretty good, I reworked ARM64
> ACPI based PCI host bridge init code, and this patch simplified
> the coed a lot, will have a test tomorrow and let you know
> the result.
> 
> Thanks
> Hanjun

[RFC v2 5/7] PCI/ACPI: Consolidate common PCI host bridge code into ACPI core

[Hanjun Guo]

Hi Jiang,

On 2015?05?05? 10:46, Jiang Liu wrote:
> Introduce common interface acpi_pci_root_create() and related data
> structures to create PCI root bus for ACPI PCI host bridges. It will
> be used to kill duplicated arch specific code for IA64 and x86. It may
> also help ARM64 in future.
>
[...]
> diff --git a/include/linux/pci-acpi.h b/include/linux/pci-acpi.h
> index a965efa52152..a292ee33d74b 100644
> --- a/include/linux/pci-acpi.h
> +++ b/include/linux/pci-acpi.h
> @@ -52,6 +52,30 @@ static inline acpi_handle acpi_pci_get_bridge_handle(struct pci_bus *pbus)
>   	return ACPI_HANDLE(dev);
>   }
>
> +struct acpi_pci_root;
> +struct acpi_pci_root_ops;
> +
> +struct acpi_pci_root_info_common {
> +	struct pci_controller		controller;

There is another problem that this patch will lead to
compile error on ARM64 since ARM64 has basic ACPI support
in 4.1.

struct pci_controller		controller is not available
on ARM64, that's the reason why compile errors happens on ARM64.

How about move struct pci_controller to this head file?

because all the related file you changed in this patch set
are only compiled when CONFI_ACPI=y, so for x86,

struct pci_controller {
#ifdef CONFIG_ACPI
         struct acpi_device *companion;  /* ACPI companion device */
#endif
#ifdef CONFIG_X86_64
         void            *iommu;         /* IOMMU private data */
#endif
         int             segment;        /* PCI domain */
         int             node;           /* NUMA node */
};

I'm sure #ifdef CONFIG_ACPI .. #endif can be removed
with no harm, and for *iommu, we can remove the #ifdef CONFIG_X86_64
with introducing little more memory on x86_32, after
that, the struct pci_controller is almost the same as ia64:

struct pci_controller {
         struct acpi_device *companion;
         void *iommu;
         int segment;
         int node;               /* nearest node with memory or 
NUMA_NO_NODE for global allocation */

         void *platform_data;
};

except void *platform_data;

On ARM64, the structure is almost the same, so how about
introduce

struct pci_controller {
         struct acpi_device *companion;  /* ACPI companion device */
         void            *iommu;         /* IOMMU private data */
         int             segment;        /* PCI domain */
         int             node;           /* NUMA node */
#ifdef CONFIG_IA64	
	void *platform_data;
#endif
};

in this file, then can be used for all architectures?

Thanks
Hanjun

[RFC v2 5/7] PCI/ACPI: Consolidate common PCI host bridge code into ACPI core

[Jiang Liu]

On 2015/5/13 17:29, Hanjun Guo wrote:
> Hi Jiang,
> 
> On 2015?05?05? 10:46, Jiang Liu wrote:
>> Introduce common interface acpi_pci_root_create() and related data
>> structures to create PCI root bus for ACPI PCI host bridges. It will
>> be used to kill duplicated arch specific code for IA64 and x86. It may
>> also help ARM64 in future.
>>
> [...]
>> diff --git a/include/linux/pci-acpi.h b/include/linux/pci-acpi.h
>> index a965efa52152..a292ee33d74b 100644
>> --- a/include/linux/pci-acpi.h
>> +++ b/include/linux/pci-acpi.h
>> @@ -52,6 +52,30 @@ static inline acpi_handle
>> acpi_pci_get_bridge_handle(struct pci_bus *pbus)
>>       return ACPI_HANDLE(dev);
>>   }
>>
>> +struct acpi_pci_root;
>> +struct acpi_pci_root_ops;
>> +
>> +struct acpi_pci_root_info_common {
>> +    struct pci_controller        controller;
> 
> There is another problem that this patch will lead to
> compile error on ARM64 since ARM64 has basic ACPI support
> in 4.1.
> 
> struct pci_controller        controller is not available
> on ARM64, that's the reason why compile errors happens on ARM64.
> 
> How about move struct pci_controller to this head file?
> 
> because all the related file you changed in this patch set
> are only compiled when CONFI_ACPI=y, so for x86,
> 
> struct pci_controller {
> #ifdef CONFIG_ACPI
>         struct acpi_device *companion;  /* ACPI companion device */
> #endif
> #ifdef CONFIG_X86_64
>         void            *iommu;         /* IOMMU private data */
> #endif
>         int             segment;        /* PCI domain */
>         int             node;           /* NUMA node */
> };
> 
> I'm sure #ifdef CONFIG_ACPI .. #endif can be removed
> with no harm, and for *iommu, we can remove the #ifdef CONFIG_X86_64
> with introducing little more memory on x86_32, after
> that, the struct pci_controller is almost the same as ia64:

On x86, struct pci_controller may be used when CONFIG_ACPI is disabled.
So we can't move it into pci-acpi.h

> 
> struct pci_controller {
>         struct acpi_device *companion;
>         void *iommu;
>         int segment;
>         int node;               /* nearest node with memory or
> NUMA_NO_NODE for global allocation */
> 
>         void *platform_data;
> };
> 
> except void *platform_data;
> 
> On ARM64, the structure is almost the same, so how about
> introduce
> 
> struct pci_controller {
>         struct acpi_device *companion;  /* ACPI companion device */
>         void            *iommu;         /* IOMMU private data */
>         int             segment;        /* PCI domain */
>         int             node;           /* NUMA node */
> #ifdef CONFIG_IA64   
>     void *platform_data;
> #endif
> };
> 
> in this file, then can be used for all architectures?
Current mode is that architecture defines its own version of
struct pci_controller. It would be better to keep this pattern.
Thanks!
Gerry

> 
> Thanks
> Hanjun

[RFC v2 5/7] PCI/ACPI: Consolidate common PCI host bridge code into ACPI core

[Hanjun Guo]

On 2015?05?13? 20:24, Jiang Liu wrote:
> On 2015/5/13 17:29, Hanjun Guo wrote:
>> Hi Jiang,
>>
>> On 2015?05?05? 10:46, Jiang Liu wrote:
>>> Introduce common interface acpi_pci_root_create() and related data
>>> structures to create PCI root bus for ACPI PCI host bridges. It will
>>> be used to kill duplicated arch specific code for IA64 and x86. It may
>>> also help ARM64 in future.
>>>
>> [...]
>>> diff --git a/include/linux/pci-acpi.h b/include/linux/pci-acpi.h
>>> index a965efa52152..a292ee33d74b 100644
>>> --- a/include/linux/pci-acpi.h
>>> +++ b/include/linux/pci-acpi.h
>>> @@ -52,6 +52,30 @@ static inline acpi_handle
>>> acpi_pci_get_bridge_handle(struct pci_bus *pbus)
>>>        return ACPI_HANDLE(dev);
>>>    }
>>>
>>> +struct acpi_pci_root;
>>> +struct acpi_pci_root_ops;
>>> +
>>> +struct acpi_pci_root_info_common {
>>> +    struct pci_controller        controller;
>>
>> There is another problem that this patch will lead to
>> compile error on ARM64 since ARM64 has basic ACPI support
>> in 4.1.
>>
>> struct pci_controller        controller is not available
>> on ARM64, that's the reason why compile errors happens on ARM64.
>>
>> How about move struct pci_controller to this head file?
>>
>> because all the related file you changed in this patch set
>> are only compiled when CONFI_ACPI=y, so for x86,
>>
>> struct pci_controller {
>> #ifdef CONFIG_ACPI
>>          struct acpi_device *companion;  /* ACPI companion device */
>> #endif
>> #ifdef CONFIG_X86_64
>>          void            *iommu;         /* IOMMU private data */
>> #endif
>>          int             segment;        /* PCI domain */
>>          int             node;           /* NUMA node */
>> };
>>
>> I'm sure #ifdef CONFIG_ACPI .. #endif can be removed
>> with no harm, and for *iommu, we can remove the #ifdef CONFIG_X86_64
>> with introducing little more memory on x86_32, after
>> that, the struct pci_controller is almost the same as ia64:
>
> On x86, struct pci_controller may be used when CONFIG_ACPI is disabled.
> So we can't move it into pci-acpi.h

Ah, ok, I missed this part.

>
>>
>> struct pci_controller {
>>          struct acpi_device *companion;
>>          void *iommu;
>>          int segment;
>>          int node;               /* nearest node with memory or
>> NUMA_NO_NODE for global allocation */
>>
>>          void *platform_data;
>> };
>>
>> except void *platform_data;
>>
>> On ARM64, the structure is almost the same, so how about
>> introduce
>>
>> struct pci_controller {
>>          struct acpi_device *companion;  /* ACPI companion device */
>>          void            *iommu;         /* IOMMU private data */
>>          int             segment;        /* PCI domain */
>>          int             node;           /* NUMA node */
>> #ifdef CONFIG_IA64
>>      void *platform_data;
>> #endif
>> };
>>
>> in this file, then can be used for all architectures?
> Current mode is that architecture defines its own version of
> struct pci_controller. It would be better to keep this pattern.

OK, thanks for the clarify :) So how about add my basic
PCI support patch for ARM64 on top of you patch set to fix
this problem?

Thanks
Hanjun

[RFC v2 5/7] PCI/ACPI: Consolidate common PCI host bridge code into ACPI core

[Jiang Liu]

On 2015/5/13 21:25, Hanjun Guo wrote:
> On 2015?05?13? 20:24, Jiang Liu wrote:
>> On 2015/5/13 17:29, Hanjun Guo wrote:
>>> Hi Jiang,
>>>
>>> On 2015?05?05? 10:46, Jiang Liu wrote:
>>>
>>> struct pci_controller {
>>>          struct acpi_device *companion;
>>>          void *iommu;
>>>          int segment;
>>>          int node;               /* nearest node with memory or
>>> NUMA_NO_NODE for global allocation */
>>>
>>>          void *platform_data;
>>> };
>>>
>>> except void *platform_data;
>>>
>>> On ARM64, the structure is almost the same, so how about
>>> introduce
>>>
>>> struct pci_controller {
>>>          struct acpi_device *companion;  /* ACPI companion device */
>>>          void            *iommu;         /* IOMMU private data */
>>>          int             segment;        /* PCI domain */
>>>          int             node;           /* NUMA node */
>>> #ifdef CONFIG_IA64
>>>      void *platform_data;
>>> #endif
>>> };
>>>
>>> in this file, then can be used for all architectures?
>> Current mode is that architecture defines its own version of
>> struct pci_controller. It would be better to keep this pattern.
> 
> OK, thanks for the clarify :) So how about add my basic
> PCI support patch for ARM64 on top of you patch set to fix
> this problem?

Sure, please send me the patches and I will send out v3 to
cover your review comments.
Thanks!

> 
> Thanks
> Hanjun
> 
> 

[RFC v2 5/7] PCI/ACPI: Consolidate common PCI host bridge code into ACPI core

[Hanjun Guo]

On 2015?05?14? 09:09, Jiang Liu wrote:
> On 2015/5/13 21:25, Hanjun Guo wrote:
>> On 2015?05?13? 20:24, Jiang Liu wrote:
>>> On 2015/5/13 17:29, Hanjun Guo wrote:
>>>> Hi Jiang,
>>>>
>>>> On 2015?05?05? 10:46, Jiang Liu wrote:
>>>>
>>>> struct pci_controller {
>>>>           struct acpi_device *companion;
>>>>           void *iommu;
>>>>           int segment;
>>>>           int node;               /* nearest node with memory or
>>>> NUMA_NO_NODE for global allocation */
>>>>
>>>>           void *platform_data;
>>>> };
>>>>
>>>> except void *platform_data;
>>>>
>>>> On ARM64, the structure is almost the same, so how about
>>>> introduce
>>>>
>>>> struct pci_controller {
>>>>           struct acpi_device *companion;  /* ACPI companion device */
>>>>           void            *iommu;         /* IOMMU private data */
>>>>           int             segment;        /* PCI domain */
>>>>           int             node;           /* NUMA node */
>>>> #ifdef CONFIG_IA64
>>>>       void *platform_data;
>>>> #endif
>>>> };
>>>>
>>>> in this file, then can be used for all architectures?
>>> Current mode is that architecture defines its own version of
>>> struct pci_controller. It would be better to keep this pattern.
>>
>> OK, thanks for the clarify :) So how about add my basic
>> PCI support patch for ARM64 on top of you patch set to fix
>> this problem?
>
> Sure, please send me the patches and I will send out v3 to
> cover your review comments.

OK, I need to rework my patches because my patch set is dependent
on top of another MMCFG refactor patch set [1], so I need to remove
MMCONFIG first then will speed up the upstream process of your patch
set, will send you the patches soon.

[1]: https://lkml.org/lkml/2015/4/17/29

Thanks
Hanjun

[RFC v2 5/7] PCI/ACPI: Consolidate common PCI host bridge code into ACPI core

[Jiang Liu]

On 2015/5/14 12:05, Hanjun Guo wrote:
> On 2015?05?14? 09:09, Jiang Liu wrote:
>> On 2015/5/13 21:25, Hanjun Guo wrote:
>>> On 2015?05?13? 20:24, Jiang Liu wrote:
>>>> On 2015/5/13 17:29, Hanjun Guo wrote:
>>>>> Hi Jiang,
>>>>>
>>>>> On 2015?05?05? 10:46, Jiang Liu wrote:
>>>>>
>>>>> struct pci_controller {
>>>>>           struct acpi_device *companion;
>>>>>           void *iommu;
>>>>>           int segment;
>>>>>           int node;               /* nearest node with memory or
>>>>> NUMA_NO_NODE for global allocation */
>>>>>
>>>>>           void *platform_data;
>>>>> };
>>>>>
>>>>> except void *platform_data;
>>>>>
>>>>> On ARM64, the structure is almost the same, so how about
>>>>> introduce
>>>>>
>>>>> struct pci_controller {
>>>>>           struct acpi_device *companion;  /* ACPI companion device */
>>>>>           void            *iommu;         /* IOMMU private data */
>>>>>           int             segment;        /* PCI domain */
>>>>>           int             node;           /* NUMA node */
>>>>> #ifdef CONFIG_IA64
>>>>>       void *platform_data;
>>>>> #endif
>>>>> };
>>>>>
>>>>> in this file, then can be used for all architectures?
>>>> Current mode is that architecture defines its own version of
>>>> struct pci_controller. It would be better to keep this pattern.
>>>
>>> OK, thanks for the clarify :) So how about add my basic
>>> PCI support patch for ARM64 on top of you patch set to fix
>>> this problem?
>>
>> Sure, please send me the patches and I will send out v3 to
>> cover your review comments.
> 
> OK, I need to rework my patches because my patch set is dependent
> on top of another MMCFG refactor patch set [1], so I need to remove
> MMCONFIG first then will speed up the upstream process of your patch
> set, will send you the patches soon.
Hi Hanjun,
	I will send out v3 soon , so you could rebase onto v3.
There are changes which will affect your rebase.
Thanks!
Gerry
> 
> [1]: https://lkml.org/lkml/2015/4/17/29
> 
> Thanks
> Hanjun

[RFC v2 6/7] x86/PCI/ACPI: Use common interface to support PCI host bridge

[Jiang Liu]

Use common interface to simplify ACPI PCI host bridge implementation.

Signed-off-by: Jiang Liu <jiang.liu@linux.intel.com>
---
 arch/x86/pci/acpi.c |  279 ++++++++++++++-------------------------------------
 1 file changed, 75 insertions(+), 204 deletions(-)

diff --git a/arch/x86/pci/acpi.c b/arch/x86/pci/acpi.c
index b34a2b660de3..91f852d87bfd 100644
--- a/arch/x86/pci/acpi.c
+++ b/arch/x86/pci/acpi.c
@@ -4,13 +4,12 @@
 #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>
 
 struct pci_root_info {
-	struct acpi_device *bridge;
-	char name[16];
-	struct pci_controller sd;
+	struct acpi_pci_root_info_common common;
 #ifdef	CONFIG_PCI_MMCONFIG
 	bool mcfg_added;
 	u16 segment;
@@ -165,14 +164,17 @@ static int check_segment(u16 seg, struct device *dev, char *estr)
 	return 0;
 }
 
-static int setup_mcfg_map(struct pci_root_info *info, u16 seg, u8 start,
-			  u8 end, phys_addr_t addr)
+static int setup_mcfg_map(struct acpi_pci_root_info_common *ci)
 {
 	int result;
-	struct device *dev = &info->bridge->dev;
+	struct pci_root_info *info;
+	struct acpi_pci_root *root = ci->root;
+	struct device *dev = &ci->bridge->dev;
+	int seg = ci->controller.segment;
 
-	info->start_bus = start;
-	info->end_bus = end;
+	info = container_of(ci, struct pci_root_info, common);
+	info->start_bus = (u8)root->secondary.start;
+	info->end_bus = (u8)root->secondary.end;
 	info->mcfg_added = false;
 
 	/* return success if MMCFG is not in use */
@@ -182,7 +184,8 @@ static int setup_mcfg_map(struct pci_root_info *info, u16 seg, u8 start,
 	if (!(pci_probe & PCI_PROBE_MMCONF))
 		return check_segment(seg, dev, "MMCONFIG is disabled,");
 
-	result = pci_mmconfig_insert(dev, seg, start, end, addr);
+	result = pci_mmconfig_insert(dev, seg, info->start_bus, info->end_bus,
+				     root->mcfg_addr);
 	if (result == 0) {
 		/* enable MMCFG if it hasn't been enabled yet */
 		if (raw_pci_ext_ops == NULL)
@@ -195,8 +198,11 @@ static int setup_mcfg_map(struct pci_root_info *info, u16 seg, u8 start,
 	return 0;
 }
 
-static void teardown_mcfg_map(struct pci_root_info *info)
+static void teardown_mcfg_map(struct acpi_pci_root_info_common *ci)
 {
+	struct pci_root_info *info;
+
+	info = container_of(ci, struct pci_root_info, common);
 	if (info->mcfg_added) {
 		pci_mmconfig_delete(info->segment, info->start_bus,
 				    info->end_bus);
@@ -204,125 +210,46 @@ static void teardown_mcfg_map(struct pci_root_info *info)
 	}
 }
 #else
-static int setup_mcfg_map(struct pci_root_info *info,
-				    u16 seg, u8 start, u8 end,
-				    phys_addr_t addr)
+static int setup_mcfg_map(struct acpi_pci_root_info_common *ci)
 {
 	return 0;
 }
-static void teardown_mcfg_map(struct pci_root_info *info)
+
+static void teardown_mcfg_map(struct acpi_pci_root_info_common *ci)
 {
 }
 #endif
 
-static void validate_resources(struct device *dev, struct list_head *crs_res,
-			       unsigned long type)
+static int pci_acpi_root_get_node(struct acpi_pci_root *root)
 {
-	LIST_HEAD(list);
-	struct resource *res1, *res2, *root = NULL;
-	struct resource_entry *tmp, *entry, *entry2;
-
-	BUG_ON((type & (IORESOURCE_MEM | IORESOURCE_IO)) == 0);
-	root = (type & IORESOURCE_MEM) ? &iomem_resource : &ioport_resource;
-
-	list_splice_init(crs_res, &list);
-	resource_list_for_each_entry_safe(entry, tmp, &list) {
-		bool free = false;
-		resource_size_t end;
-
-		res1 = entry->res;
-		if (!(res1->flags & type))
-			goto next;
-
-		/* Exclude non-addressable range or non-addressable portion */
-		end = min(res1->end, root->end);
-		if (end <= res1->start) {
-			dev_info(dev, "host bridge window %pR (ignored, not CPU addressable)\n",
-				 res1);
-			free = true;
-			goto next;
-		} else if (res1->end != end) {
-			dev_info(dev, "host bridge window %pR ([%#llx-%#llx] ignored, not CPU addressable)\n",
-				 res1, (unsigned long long)end + 1,
-				 (unsigned long long)res1->end);
-			res1->end = end;
-		}
-
-		resource_list_for_each_entry(entry2, crs_res) {
-			res2 = entry2->res;
-			if (!(res2->flags & type))
-				continue;
-
-			/*
-			 * I don't like throwing away windows because then
-			 * our resources no longer match the ACPI _CRS, but
-			 * the kernel resource tree doesn't allow overlaps.
-			 */
-			if (resource_overlaps(res1, res2)) {
-				res2->start = min(res1->start, res2->start);
-				res2->end = max(res1->end, res2->end);
-				dev_info(dev, "host bridge window expanded to %pR; %pR ignored\n",
-					 res2, res1);
-				free = true;
-				goto next;
-			}
-		}
-
-next:
-		resource_list_del(entry);
-		if (free)
-			resource_list_free_entry(entry);
-		else
-			resource_list_add_tail(entry, crs_res);
+	int busnum = root->secondary.start;
+	struct acpi_device *device = root->device;
+	int node = acpi_get_node(device->handle);
+
+	if (node == NUMA_NO_NODE) {
+		node = x86_pci_root_bus_node(busnum);
+		if (node != 0 && node != NUMA_NO_NODE)
+			dev_info(&device->dev, FW_BUG "no _PXM; falling back to node %d from hardware (may be inconsistent with ACPI node numbers)\n",
+				node);
 	}
+	if (node != NUMA_NO_NODE && !node_online(node))
+		node = NUMA_NO_NODE;
+
+	return node;
 }
 
-static void add_resources(struct pci_root_info *info,
-			  struct list_head *resources,
-			  struct list_head *crs_res)
+static int pci_acpi_root_init_info(struct acpi_pci_root_info_common *ci)
 {
-	struct resource_entry *entry, *tmp;
-	struct resource *res, *conflict, *root = NULL;
-
-	validate_resources(&info->bridge->dev, crs_res, IORESOURCE_MEM);
-	validate_resources(&info->bridge->dev, crs_res, IORESOURCE_IO);
-
-	resource_list_for_each_entry_safe(entry, tmp, crs_res) {
-		res = entry->res;
-		if (res->flags & IORESOURCE_MEM)
-			root = &iomem_resource;
-		else if (res->flags & IORESOURCE_IO)
-			root = &ioport_resource;
-		else
-			BUG_ON(res);
-
-		conflict = insert_resource_conflict(root, res);
-		if (conflict) {
-			dev_info(&info->bridge->dev,
-				 "ignoring host bridge window %pR (conflicts with %s %pR)\n",
-				 res, conflict->name, conflict);
-			resource_list_destroy_entry(entry);
-		}
-	}
+	ci->controller.node = pci_acpi_root_get_node(ci->root);
+	if (pci_ignore_seg)
+		ci->controller.segment = 0;
 
-	list_splice_tail(crs_res, resources);
+	return setup_mcfg_map(ci);
 }
 
-static void release_pci_root_info(struct pci_host_bridge *bridge)
+static void pci_acpi_root_release_info(struct acpi_pci_root_info_common *ci)
 {
-	struct resource *res;
-	struct resource_entry *entry;
-	struct pci_root_info *info = bridge->release_data;
-
-	resource_list_for_each_entry(entry, &bridge->windows) {
-		res = entry->res;
-		if (res->parent &&
-		    (res->flags & (IORESOURCE_MEM | IORESOURCE_IO)))
-			release_resource(res);
-	}
-
-	teardown_mcfg_map(info);
-	kfree(info);
+	teardown_mcfg_map(ci);
 }
 
 /*
@@ -345,47 +272,42 @@ static bool resource_is_pcicfg_ioport(struct resource *res)
 		res->start == 0xCF8 && res->end == 0xCFF;
 }
 
-static void probe_pci_root_info(struct pci_root_info *info,
-				struct acpi_device *device,
-				int busnum, int domain,
-				struct list_head *list)
+static int pci_acpi_root_prepare_resources(struct acpi_pci_root_info_common *ci,
+					   int status)
 {
-	int ret;
+	struct acpi_device *device = ci->bridge;
+	int busnum = ci->root->secondary.start;
 	struct resource_entry *entry, *tmp;
 
-	sprintf(info->name, "PCI Bus %04x:%02x", domain, busnum);
-	info->bridge = device;
-	ret = acpi_dev_get_resources(device, list,
-				     acpi_dev_filter_resource_type_cb,
-				     (void *)(IORESOURCE_IO | IORESOURCE_MEM));
-	if (ret < 0)
-		dev_warn(&device->dev,
-			 "failed to parse _CRS method, error code %d\n", ret);
-	else if (ret == 0)
-		dev_dbg(&device->dev,
-			"no IO and memory resources present in _CRS\n");
-	else
-		resource_list_for_each_entry_safe(entry, tmp, list) {
-			if ((entry->res->flags & IORESOURCE_DISABLED) ||
-			    resource_is_pcicfg_ioport(entry->res))
+	if (pci_use_crs) {
+		resource_list_for_each_entry_safe(entry, tmp, &ci->resources)
+			if (resource_is_pcicfg_ioport(entry->res))
 				resource_list_destroy_entry(entry);
-			else
-				entry->res->name = info->name;
-		}
+		return status;
+	}
+
+	resource_list_for_each_entry_safe(entry, tmp, &ci->resources) {
+		dev_printk(KERN_DEBUG, &device->dev,
+			   "host bridge window %pR (ignored)\n", entry->res);
+		resource_list_destroy_entry(entry);
+	}
+	x86_pci_root_bus_resources(busnum, &ci->resources);
+
+	return 0;
 }
 
+static struct acpi_pci_root_ops acpi_pci_root_ops = {
+	.pci_ops = &pci_root_ops,
+	.init_info = pci_acpi_root_init_info,
+	.release_info = pci_acpi_root_release_info,
+	.prepare_resources = pci_acpi_root_prepare_resources,
+};
+
 struct pci_bus *pci_acpi_scan_root(struct acpi_pci_root *root)
 {
-	struct acpi_device *device = root->device;
-	struct pci_root_info *info;
 	int domain = root->segment;
 	int busnum = root->secondary.start;
-	struct resource_entry *res_entry;
-	LIST_HEAD(crs_res);
-	LIST_HEAD(resources);
 	struct pci_bus *bus;
-	struct pci_controller *sd;
-	int node;
 
 	if (pci_ignore_seg)
 		domain = 0;
@@ -397,72 +319,24 @@ struct pci_bus *pci_acpi_scan_root(struct acpi_pci_root *root)
 		return NULL;
 	}
 
-	node = acpi_get_node(device->handle);
-	if (node == NUMA_NO_NODE) {
-		node = x86_pci_root_bus_node(busnum);
-		if (node != 0 && node != NUMA_NO_NODE)
-			dev_info(&device->dev, FW_BUG "no _PXM; falling back to node %d from hardware (may be inconsistent with ACPI node numbers)\n",
-				node);
-	}
-
-	if (node != NUMA_NO_NODE && !node_online(node))
-		node = NUMA_NO_NODE;
-
-	info = kzalloc_node(sizeof(*info), GFP_KERNEL, node);
-	if (!info) {
-		printk(KERN_WARNING "pci_bus %04x:%02x: "
-		       "ignored (out of memory)\n", domain, busnum);
-		return NULL;
-	}
-
-	sd = &info->sd;
-	sd->segment = domain;
-	sd->node = node;
-	sd->companion = device;
-
 	bus = pci_find_bus(domain, busnum);
 	if (bus) {
 		/*
 		 * If the desired bus has been scanned already, replace
 		 * its bus->sysdata.
 		 */
-		memcpy(bus->sysdata, sd, sizeof(*sd));
-		kfree(info);
+		struct pci_controller sd = {
+			.segment = domain,
+			.node = pci_acpi_root_get_node(root),
+			.companion = root->device
+		};
+
+		memcpy(bus->sysdata, &sd, sizeof(sd));
 	} else {
-		/* insert busn res at first */
-		pci_add_resource(&resources,  &root->secondary);
+		struct pci_root_info *info;
 
-		/*
-		 * _CRS with no apertures is normal, so only fall back to
-		 * defaults or native bridge info if we're ignoring _CRS.
-		 */
-		probe_pci_root_info(info, device, busnum, domain, &crs_res);
-		if (pci_use_crs) {
-			add_resources(info, &resources, &crs_res);
-		} else {
-			resource_list_for_each_entry(res_entry, &crs_res)
-				dev_printk(KERN_DEBUG, &device->dev,
-					   "host bridge window %pR (ignored)\n",
-					   res_entry->res);
-			resource_list_free(&crs_res);
-			x86_pci_root_bus_resources(busnum, &resources);
-		}
-
-		if (!setup_mcfg_map(info, domain, (u8)root->secondary.start,
-				    (u8)root->secondary.end, root->mcfg_addr))
-			bus = pci_create_root_bus(NULL, busnum, &pci_root_ops,
-						  sd, &resources);
-
-		if (bus) {
-			pci_scan_child_bus(bus);
-			pci_set_host_bridge_release(
-				to_pci_host_bridge(bus->bridge),
-				release_pci_root_info, info);
-		} else {
-			resource_list_free(&resources);
-			teardown_mcfg_map(info);
-			kfree(info);
-		}
+		bus = acpi_pci_root_create(root, &acpi_pci_root_ops,
+					sizeof(*info) - sizeof(info->common));
 	}
 
 	/* After the PCI-E bus has been walked and all devices discovered,
@@ -474,9 +348,6 @@ struct pci_bus *pci_acpi_scan_root(struct acpi_pci_root *root)
 			pcie_bus_configure_settings(child);
 	}
 
-	if (bus && node != NUMA_NO_NODE)
-		dev_printk(KERN_DEBUG, &bus->dev, "on NUMA node %d\n", node);
-
 	return bus;
 }
 
-- 
1.7.10.4

[RFC v2 6/7] x86/PCI/ACPI: Use common interface to support PCI host bridge

[Hanjun Guo]

[...]
>   struct pci_root_info {
> -	struct acpi_device *bridge;
> -	char name[16];
> -	struct pci_controller sd;
> +	struct acpi_pci_root_info_common common;
>   #ifdef	CONFIG_PCI_MMCONFIG
>   	bool mcfg_added;
>   	u16 segment;
> @@ -165,14 +164,17 @@ static int check_segment(u16 seg, struct device *dev, char *estr)
>   	return 0;
>   }
>
> -static int setup_mcfg_map(struct pci_root_info *info, u16 seg, u8 start,
> -			  u8 end, phys_addr_t addr)
> +static int setup_mcfg_map(struct acpi_pci_root_info_common *ci)
>   {
>   	int result;
> -	struct device *dev = &info->bridge->dev;
> +	struct pci_root_info *info;
> +	struct acpi_pci_root *root = ci->root;
> +	struct device *dev = &ci->bridge->dev;
> +	int seg = ci->controller.segment;

...

>
> -	info->start_bus = start;
> -	info->end_bus = end;
> +	info = container_of(ci, struct pci_root_info, common);
> +	info->start_bus = (u8)root->secondary.start;
> +	info->end_bus = (u8)root->secondary.end;
>   	info->mcfg_added = false;

info->segment is not initialized here

>
>   	/* return success if MMCFG is not in use */
> @@ -182,7 +184,8 @@ static int setup_mcfg_map(struct pci_root_info *info, u16 seg, u8 start,
>   	if (!(pci_probe & PCI_PROBE_MMCONF))
>   		return check_segment(seg, dev, "MMCONFIG is disabled,");
>
> -	result = pci_mmconfig_insert(dev, seg, start, end, addr);
> +	result = pci_mmconfig_insert(dev, seg, info->start_bus, info->end_bus,
> +				     root->mcfg_addr);
>   	if (result == 0) {
>   		/* enable MMCFG if it hasn't been enabled yet */
>   		if (raw_pci_ext_ops == NULL)
> @@ -195,8 +198,11 @@ static int setup_mcfg_map(struct pci_root_info *info, u16 seg, u8 start,
>   	return 0;
>   }
>
> -static void teardown_mcfg_map(struct pci_root_info *info)
> +static void teardown_mcfg_map(struct acpi_pci_root_info_common *ci)
>   {
> +	struct pci_root_info *info;
> +
> +	info = container_of(ci, struct pci_root_info, common);
>   	if (info->mcfg_added) {
>   		pci_mmconfig_delete(info->segment, info->start_bus,

But the segment is used here to delete mmconfig entry, it seems that
the domain is 0 so there will be no problem, did I miss something?

Thanks
Hanjun

[RFC v2 6/7] x86/PCI/ACPI: Use common interface to support PCI host bridge

[Jiang Liu]

On 2015/5/12 20:19, Hanjun Guo wrote:
> [...]
>>   struct pci_root_info {
>> -    struct acpi_device *bridge;
>> -    char name[16];
>> -    struct pci_controller sd;
>> +    struct acpi_pci_root_info_common common;
>>   #ifdef    CONFIG_PCI_MMCONFIG
>>       bool mcfg_added;
>>       u16 segment;
>> @@ -165,14 +164,17 @@ static int check_segment(u16 seg, struct device
>> *dev, char *estr)
>>       return 0;
>>   }
>>
>> -static int setup_mcfg_map(struct pci_root_info *info, u16 seg, u8 start,
>> -              u8 end, phys_addr_t addr)
>> +static int setup_mcfg_map(struct acpi_pci_root_info_common *ci)
>>   {
>>       int result;
>> -    struct device *dev = &info->bridge->dev;
>> +    struct pci_root_info *info;
>> +    struct acpi_pci_root *root = ci->root;
>> +    struct device *dev = &ci->bridge->dev;
>> +    int seg = ci->controller.segment;
> 
> ...
> 
>>
>> -    info->start_bus = start;
>> -    info->end_bus = end;
>> +    info = container_of(ci, struct pci_root_info, common);
>> +    info->start_bus = (u8)root->secondary.start;
>> +    info->end_bus = (u8)root->secondary.end;
>>       info->mcfg_added = false;
> 
> info->segment is not initialized here
> 
>>
>>       /* return success if MMCFG is not in use */
>> @@ -182,7 +184,8 @@ static int setup_mcfg_map(struct pci_root_info
>> *info, u16 seg, u8 start,
>>       if (!(pci_probe & PCI_PROBE_MMCONF))
>>           return check_segment(seg, dev, "MMCONFIG is disabled,");
>>
>> -    result = pci_mmconfig_insert(dev, seg, start, end, addr);
>> +    result = pci_mmconfig_insert(dev, seg, info->start_bus,
>> info->end_bus,
>> +                     root->mcfg_addr);
>>       if (result == 0) {
>>           /* enable MMCFG if it hasn't been enabled yet */
>>           if (raw_pci_ext_ops == NULL)
>> @@ -195,8 +198,11 @@ static int setup_mcfg_map(struct pci_root_info
>> *info, u16 seg, u8 start,
>>       return 0;
>>   }
>>
>> -static void teardown_mcfg_map(struct pci_root_info *info)
>> +static void teardown_mcfg_map(struct acpi_pci_root_info_common *ci)
>>   {
>> +    struct pci_root_info *info;
>> +
>> +    info = container_of(ci, struct pci_root_info, common);
>>       if (info->mcfg_added) {
>>           pci_mmconfig_delete(info->segment, info->start_bus,
> 
> But the segment is used here to delete mmconfig entry, it seems that
> the domain is 0 so there will be no problem, did I miss something?

Good catch, we should use ci->controller.segment and kill info->segment.

> 
> Thanks
> Hanjun

[RFC v2 7/7] ia64/PCI/ACPI: Use common interface to support PCI host bridge

[Jiang Liu]

Use common interface to simplify PCI host bridge implementation.

Tested-by: Tony Luck <tony.luck@intel.com>
Signed-off-by: Jiang Liu <jiang.liu@linux.intel.com>
---
 arch/ia64/pci/pci.c |  246 +++++++++++----------------------------------------
 1 file changed, 51 insertions(+), 195 deletions(-)

diff --git a/arch/ia64/pci/pci.c b/arch/ia64/pci/pci.c
index 4af1b52c7a44..0819d6274e82 100644
--- a/arch/ia64/pci/pci.c
+++ b/arch/ia64/pci/pci.c
@@ -116,15 +116,11 @@ struct pci_ops pci_root_ops = {
 };
 
 struct pci_root_info {
-	struct pci_controller controller;
-	struct acpi_device *bridge;
-	struct list_head resources;
+	struct acpi_pci_root_info_common common;
 	struct list_head io_resources;
-	char name[16];
 };
 
-static unsigned int
-new_space (u64 phys_base, int sparse)
+static unsigned int new_space(u64 phys_base, int sparse)
 {
 	u64 mmio_base;
 	int i;
@@ -160,11 +156,11 @@ static int add_io_space(struct device *dev, struct pci_root_info *info,
 	unsigned long base, min, max, base_port;
 	unsigned int sparse = 0, space_nr, len;
 
-	len = strlen(info->name) + 32;
+	len = strlen(info->common.name) + 32;
 	iospace = resource_list_create_entry(NULL, len);
 	if (!iospace) {
 		dev_err(dev, "PCI: No memory for %s I/O port space\n",
-			info->name);
+			info->common.name);
 		return -ENOMEM;
 	}
 
@@ -179,7 +175,7 @@ static int add_io_space(struct device *dev, struct pci_root_info *info,
 	max = res->end - entry->offset;
 	base = __pa(io_space[space_nr].mmio_base);
 	base_port = IO_SPACE_BASE(space_nr);
-	snprintf(name, len, "%s I/O Ports %08lx-%08lx", info->name,
+	snprintf(name, len, "%s I/O Ports %08lx-%08lx", info->common.name,
 		 base_port + min, base_port + max);
 
 	/*
@@ -234,217 +230,77 @@ static bool resource_is_pcicfg_ioport(struct resource *res)
 		res->start == 0xCF8 && res->end == 0xCFF;
 }
 
-static int
-probe_pci_root_info(struct pci_root_info *info, struct acpi_device *device,
-		    int busnum, int domain)
+static int pci_acpi_root_prepare_resources(struct acpi_pci_root_info_common *ci,
+					   int status)
 {
-	int ret;
-	struct list_head *list = &info->resources;
+	struct device *dev = &ci->bridge->dev;
+	struct pci_root_info *info;
+	struct resource *res;
 	struct resource_entry *entry, *tmp;
 
-	ret = acpi_dev_get_resources(device, list,
-				     acpi_dev_filter_resource_type_cb,
-				     (void *)(IORESOURCE_IO | IORESOURCE_MEM));
-	if (ret < 0)
-		dev_warn(&device->dev,
-			 "failed to parse _CRS method, error code %d\n", ret);
-	else if (ret == 0)
-		dev_dbg(&device->dev,
-			"no IO and memory resources present in _CRS\n");
-	else
-		resource_list_for_each_entry_safe(entry, tmp, list) {
-			if ((entry->res->flags & IORESOURCE_DISABLED) ||
-			    resource_is_pcicfg_ioport(entry->res))
-				resource_list_destroy_entry(entry);
-			else
-				entry->res->name = info->name;
+	if (status > 0) {
+		info = container_of(ci, struct pci_root_info, common);
+		resource_list_for_each_entry_safe(entry, tmp, &ci->resources) {
+			res = entry->res;
+			if (res->flags & IORESOURCE_MEM) {
+				/*
+				 * HP's firmware has a hack to work around a
+				 * Windows bug. Ignore these tiny memory ranges.
+				 */
+				if (resource_size(res) <= 16) {
+					resource_list_del(entry);
+					insert_resource(&iomem_resource,
+							entry->res);
+					resource_list_add_tail(entry,
+							&info->io_resources);
+				}
+			} else if (res->flags & IORESOURCE_IO) {
+				if (resource_is_pcicfg_ioport(entry->res))
+					resource_list_destroy_entry(entry);
+				else if (add_io_space(dev, info, entry))
+					resource_list_destroy_entry(entry);
+			}
 		}
+	}
 
-	return ret;
+	return status;
 }
 
-static void validate_resources(struct device *dev, struct list_head *resources,
-			       unsigned long type)
+static int pci_acpi_root_init_info(struct acpi_pci_root_info_common *ci)
 {
-	LIST_HEAD(list);
-	struct resource *res1, *res2, *root = NULL;
-	struct resource_entry *tmp, *entry, *entry2;
-
-	BUG_ON((type & (IORESOURCE_MEM | IORESOURCE_IO)) == 0);
-	root = (type & IORESOURCE_MEM) ? &iomem_resource : &ioport_resource;
-
-	list_splice_init(resources, &list);
-	resource_list_for_each_entry_safe(entry, tmp, &list) {
-		bool free = false;
-		resource_size_t end;
-
-		res1 = entry->res;
-		if (!(res1->flags & type))
-			goto next;
-
-		/* Exclude non-addressable range or non-addressable portion */
-		end = min(res1->end, root->end);
-		if (end <= res1->start) {
-			dev_info(dev, "host bridge window %pR (ignored, not CPU addressable)\n",
-				 res1);
-			free = true;
-			goto next;
-		} else if (res1->end != end) {
-			dev_info(dev, "host bridge window %pR ([%#llx-%#llx] ignored, not CPU addressable)\n",
-				 res1, (unsigned long long)end + 1,
-				 (unsigned long long)res1->end);
-			res1->end = end;
-		}
+	struct pci_root_info *info;
 
-		resource_list_for_each_entry(entry2, resources) {
-			res2 = entry2->res;
-			if (!(res2->flags & type))
-				continue;
-
-			/*
-			 * I don't like throwing away windows because then
-			 * our resources no longer match the ACPI _CRS, but
-			 * the kernel resource tree doesn't allow overlaps.
-			 */
-			if (resource_overlaps(res1, res2)) {
-				res2->start = min(res1->start, res2->start);
-				res2->end = max(res1->end, res2->end);
-				dev_info(dev, "host bridge window expanded to %pR; %pR ignored\n",
-					 res2, res1);
-				free = true;
-				goto next;
-			}
-		}
+	info = container_of(ci, struct pci_root_info, common);
+	INIT_LIST_HEAD(&info->io_resources);
 
-next:
-		resource_list_del(entry);
-		if (free)
-			resource_list_free_entry(entry);
-		else
-			resource_list_add_tail(entry, resources);
-	}
+	return 0;
 }
 
-static void add_resources(struct pci_root_info *info, struct device *dev)
+static void pci_acpi_root_release_info(struct acpi_pci_root_info_common *ci)
 {
+	struct pci_root_info *info;
 	struct resource_entry *entry, *tmp;
-	struct resource *res, *conflict, *root = NULL;
-	struct list_head *list = &info->resources;
-
-	validate_resources(dev, list, IORESOURCE_MEM);
-	validate_resources(dev, list, IORESOURCE_IO);
-
-	resource_list_for_each_entry_safe(entry, tmp, list) {
-		res = entry->res;
-		if (res->flags & IORESOURCE_MEM) {
-			root = &iomem_resource;
-			/*
-			 * HP's firmware has a hack to work around a Windows
-			 * bug. Ignore these tiny memory ranges.
-			 */
-			if (resource_size(res) <= 16) {
-				resource_list_destroy_entry(entry);
-				continue;
-			}
-		} else if (res->flags & IORESOURCE_IO) {
-			root = &ioport_resource;
-			if (add_io_space(&info->bridge->dev, info, entry)) {
-				resource_list_destroy_entry(entry);
-				continue;
-			}
-		} else {
-			BUG_ON(res);
-		}
-
-		conflict = insert_resource_conflict(root, res);
-		if (conflict) {
-			dev_info(dev,
-				 "ignoring host bridge window %pR (conflicts with %s %pR)\n",
-				 res, conflict->name, conflict);
-			resource_list_destroy_entry(entry);
-		}
-	}
-}
 
-static void __release_pci_root_info(struct pci_root_info *info)
-{
-	struct resource *res;
-	struct resource_entry *entry, *tentry;
-
-	resource_list_for_each_entry_safe(entry, tentry, &info->io_resources) {
+	info = container_of(ci, struct pci_root_info, common);
+	resource_list_for_each_entry_safe(entry, tmp, &info->io_resources) {
 		release_resource(entry->res);
 		resource_list_destroy_entry(entry);
 	}
-
-	resource_list_for_each_entry_safe(entry, tentry, &info->resources) {
-		res = entry->res;
-		if (res->parent &&
-		    (res->flags & (IORESOURCE_MEM | IORESOURCE_IO)))
-			release_resource(res);
-		resource_list_destroy_entry(entry);
-	}
-
-	kfree(info);
 }
 
-static void release_pci_root_info(struct pci_host_bridge *bridge)
-{
-	struct pci_root_info *info = bridge->release_data;
-
-	__release_pci_root_info(info);
-}
+static struct acpi_pci_root_ops pci_acpi_root_ops = {
+	.pci_ops = &pci_root_ops,
+	.init_info = pci_acpi_root_init_info,
+	.release_info = pci_acpi_root_release_info,
+	.prepare_resources = pci_acpi_root_prepare_resources,
+};
 
 struct pci_bus *pci_acpi_scan_root(struct acpi_pci_root *root)
 {
-	struct acpi_device *device = root->device;
-	int domain = root->segment;
-	int bus = root->secondary.start;
 	struct pci_root_info *info;
-	struct pci_bus *pbus;
-	int ret;
-
-	info = kzalloc(sizeof(*info), GFP_KERNEL);
-	if (!info) {
-		dev_err(&device->dev,
-			"pci_bus %04x:%02x: ignored (out of memory)\n",
-			domain, bus);
-		return NULL;
-	}
-
-	info->controller.segment = domain;
-	info->controller.companion = device;
-	info->controller.node = acpi_get_node(device->handle);
-	info->bridge = device;
-	INIT_LIST_HEAD(&info->resources);
-	INIT_LIST_HEAD(&info->io_resources);
-	snprintf(info->name, sizeof(info->name),
-		 "PCI Bus %04x:%02x", domain, bus);
-
-	ret = probe_pci_root_info(info, device, bus, domain);
-	if (ret <= 0) {
-		kfree(info);
-		return NULL;
-	}
-	add_resources(info, &info->bridge->dev);
-	pci_add_resource(&info->resources, &root->secondary);
-
-	/*
-	 * See arch/x86/pci/acpi.c.
-	 * The desired pci bus might already be scanned in a quirk. We
-	 * should handle the case here, but it appears that IA64 hasn't
-	 * such quirk. So we just ignore the case now.
-	 */
-	pbus = pci_create_root_bus(NULL, bus, &pci_root_ops,
-				   &info->controller, &info->resources);
-	if (!pbus) {
-		__release_pci_root_info(info);
-		return NULL;
-	}
 
-	pci_set_host_bridge_release(to_pci_host_bridge(pbus->bridge),
-			release_pci_root_info, info);
-	pci_scan_child_bus(pbus);
-	return pbus;
+	return acpi_pci_root_create(root, &pci_acpi_root_ops,
+				    sizeof(*info) - sizeof(info->common));
 }
 
 int pcibios_root_bridge_prepare(struct pci_host_bridge *bridge)
-- 
1.7.10.4

[RFC v2 0/7] Consolidate ACPI PCI root common code into ACPI core

[Hanjun Guo]

Hi Gerry,

On 2015?05?05? 10:46, Jiang Liu wrote:
> This patch set consolidates common code to support ACPI PCI root on x86
> and IA64 platforms into ACPI core, to reproduce duplicated code and
> simplify maintenance. The common code should also used to support PCI
> host bridge on ARM64 too, but I'm lacking of knowledge about PCIe host
> bridge implementation details on ARM64, so please help to review whether
> this is suitable for ARM64 too.

I like the idea and direction of this patch set a lot. :)

I will rebase our ARM64 ACPI PCI host bridge patch on top of your
patchset this week, then I will come up with more detail review
comments and test results, for now, I'm pretty busy with ACPI GIC
patches to support your stacked irqdomains :)

Thanks
Hanjun