[Patch v4 0/8] Consolidate ACPI PCI root common code into ACPI core

[Patch v4 0/8] 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. And a patch set based on this to support ACPI based
PCIe host bridge on ARM64 has been posted at:

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

V3->V4:
1) Add patch[05/08] support solve building issue on ARM64
2) Solve an implicitly pointer cast issue.
3) Rebase to latest mainstream kernel

V2->V3:
1. Move memory allocation/free from ACPI core into arch
2. Kill the field 'segment' in struct pci_root_info on x86

Thanks!
Gerry

Hanjun Guo (1):
  ARM64 / PCI: introduce struct pci_controller for ACPI

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/arm64/include/asm/pci.h  |   10 ++
 arch/ia64/include/asm/pci.h   |    5 -
 arch/ia64/pci/pci.c           |  364 +++++++++++------------------------------
 arch/x86/include/asm/pci.h    |   13 +-
 arch/x86/include/asm/pci_64.h |    4 +-
 arch/x86/pci/acpi.c           |  295 ++++++++++-----------------------
 arch/x86/pci/common.c         |    2 +-
 drivers/acpi/pci_root.c       |  200 ++++++++++++++++++++++
 drivers/acpi/resource.c       |    9 +-
 include/linux/ioport.h        |    1 +
 include/linux/pci-acpi.h      |   23 +++
 11 files changed, 432 insertions(+), 494 deletions(-)

-- 
1.7.10.4

[Patch v4 1/8] 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>
Reviewed-by: Hanjun Guo <hanjun.guo@linaro.org>
---
 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

[Patch v4 2/8] 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>
Reviewed-by: Hanjun Guo <hanjun.guo@linaro.org>
---
 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 7cc3be9fa7c6..d20db9e48014 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;
 	}
-- 
1.7.10.4

[Patch v4 3/8] 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>
Reviewed-by: Hanjun Guo <hanjun.guo@linaro.org>
---
 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 d20db9e48014..b1846b891ea5 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

[Patch v4 4/8] 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           |    9 +++++----
 arch/x86/pci/common.c         |    2 +-
 4 files changed, 15 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 14a63ed6fe09..a46fff030c09 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;
 
@@ -489,9 +489,10 @@ int pcibios_root_bridge_prepare(struct pci_host_bridge *bridge)
 	 * that case.
 	 */
 	if (!bridge->dev.parent) {
-		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

[Patch v4 5/8] ARM64/PCI/ACPI: Introduce struct pci_controller for ACPI

[Jiang Liu]

From: Hanjun Guo <hanjun.guo@linaro.org>

ARM64 ACPI based PCI host bridge init needs a arch dependent
struct pci_controller to accommodate common PCI host bridge
code which is introduced later, or it will lead to compile
errors on ARM64.

Signed-off-by: Hanjun Guo <hanjun.guo@linaro.org>
Tested-by: Suravee Suthikulpanit <Suravee.Suthikulpanit@amd.com>
CC: Arnd Bergmann <arnd@arndb.de>
CC: Catalin Marinas <catalin.marinas@arm.com>
CC: Liviu Dudau <Liviu.Dudau@arm.com>
CC: Lorenzo Pieralisi <Lorenzo.Pieralisi@arm.com>
CC: Will Deacon <will.deacon@arm.com>
Signed-off-by: Jiang Liu <jiang.liu@linux.intel.com>
---
 arch/arm64/include/asm/pci.h |   10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/arch/arm64/include/asm/pci.h b/arch/arm64/include/asm/pci.h
index b008a72f8bc0..70884957f253 100644
--- a/arch/arm64/include/asm/pci.h
+++ b/arch/arm64/include/asm/pci.h
@@ -10,6 +10,16 @@
 #include <asm-generic/pci-bridge.h>
 #include <asm-generic/pci-dma-compat.h>
 
+struct acpi_device;
+
+struct pci_controller {
+#ifdef CONFIG_ACPI
+	struct acpi_device *companion;	/* ACPI companion device */
+#endif
+	int		segment;	/* PCI domain */
+	int		node;		/* NUMA node */
+};
+
 #define PCIBIOS_MIN_IO		0x1000
 #define PCIBIOS_MIN_MEM		0
 
-- 
1.7.10.4

[Patch v4 6/8] 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  |  200 ++++++++++++++++++++++++++++++++++++++++++++++
 include/linux/pci-acpi.h |   23 ++++++
 2 files changed, 223 insertions(+)

diff --git a/drivers/acpi/pci_root.c b/drivers/acpi/pci_root.c
index 1b5569c092c6..22d0cb799ef1 100644
--- a/drivers/acpi/pci_root.c
+++ b/drivers/acpi/pci_root.c
@@ -656,6 +656,206 @@ 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 *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 *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 *info)
+{
+	struct resource *res;
+	struct resource_entry *entry, *tmp;
+
+	if (!info)
+		return;
+
+	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);
+	}
+
+	info->ops->release_info(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,
+				     struct acpi_pci_root_info *info)
+{
+	int ret, busnum = root->secondary.start;
+	struct acpi_device *device = root->device;
+	struct pci_bus *bus;
+
+	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))
+		goto out_release_info;
+	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..685ccc4c3170 100644
--- a/include/linux/pci-acpi.h
+++ b/include/linux/pci-acpi.h
@@ -52,6 +52,29 @@ 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 {
+	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 *info);
+	void (*release_info)(struct acpi_pci_root_info *info);
+	int (*prepare_resources)(struct acpi_pci_root_info *info, int status);
+};
+
+extern struct pci_bus *acpi_pci_root_create(struct acpi_pci_root *root,
+					    struct acpi_pci_root_ops *ops,
+					    struct acpi_pci_root_info *info);
+
 void acpi_pci_add_bus(struct pci_bus *bus);
 void acpi_pci_remove_bus(struct pci_bus *bus);
 
-- 
1.7.10.4

[Patch v4 7/8] 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>
Reviewed-by: Hanjun Guo <hanjun.guo@linaro.org>
---
 arch/x86/pci/acpi.c |  292 +++++++++++++++------------------------------------
 1 file changed, 85 insertions(+), 207 deletions(-)

diff --git a/arch/x86/pci/acpi.c b/arch/x86/pci/acpi.c
index a46fff030c09..58877e810d51 100644
--- a/arch/x86/pci/acpi.c
+++ b/arch/x86/pci/acpi.c
@@ -4,16 +4,14 @@
 #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;
 #ifdef	CONFIG_PCI_MMCONFIG
 	bool mcfg_added;
-	u16 segment;
 	u8 start_bus;
 	u8 end_bus;
 #endif
@@ -165,14 +163,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 *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 +183,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,134 +197,59 @@ 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 *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);
+		pci_mmconfig_delete(ci->controller.segment,
+				    info->start_bus, info->end_bus);
 		info->mcfg_added = false;
 	}
 }
 #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 *ci)
 {
 	return 0;
 }
-static void teardown_mcfg_map(struct pci_root_info *info)
+
+static void teardown_mcfg_map(struct acpi_pci_root_info *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 *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 *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);
+	kfree(container_of(ci, struct pci_root_info, common));
 }
 
 /*
@@ -345,47 +272,43 @@ 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 *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);
+	int node = pci_acpi_root_get_node(root);
 	struct pci_bus *bus;
-	struct pci_controller *sd;
-	int node;
 
 	if (pci_ignore_seg)
 		domain = 0;
@@ -397,72 +320,30 @@ 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 = node,
+			.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);
-		}
+		info = kzalloc_node(sizeof(*info), GFP_KERNEL, node);
+		if (!info)
+			dev_err(&root->device->dev,
+				"pci_bus %04x:%02x: ignored (out of memory)\n",
+				domain, busnum);
+		else
+			bus = acpi_pci_root_create(root, &acpi_pci_root_ops,
+						   &info->common);
 	}
 
 	/* After the PCI-E bus has been walked and all devices discovered,
@@ -474,9 +355,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

[Patch v4 8/8] 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 |  235 ++++++++++-----------------------------------------
 1 file changed, 45 insertions(+), 190 deletions(-)

diff --git a/arch/ia64/pci/pci.c b/arch/ia64/pci/pci.c
index b1846b891ea5..769bf80b4fc9 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;
 	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,76 @@ 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 *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;
-		}
-
-	return ret;
-}
-
-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;
-		}
-
-		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;
+	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);
 			}
 		}
-
-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);
-		}
-	}
+	return status;
 }
 
-static void __release_pci_root_info(struct pci_root_info *info)
+static void pci_acpi_root_release_info(struct acpi_pci_root_info *ci)
 {
-	struct resource *res;
-	struct resource_entry *entry, *tentry;
+	struct pci_root_info *info;
+	struct resource_entry *entry, *tmp;
 
-	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,
+	.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,
+		dev_err(&root->device->dev,
 			"pci_bus %04x:%02x: ignored (out of memory)\n",
-			domain, bus);
+			root->segment, (int)root->secondary.start);
 		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, &info->common);
 }
 
 int pcibios_root_bridge_prepare(struct pci_host_bridge *bridge)
-- 
1.7.10.4

Re: [Patch v4 0/8] Consolidate ACPI PCI root common code into ACPI core

[Hanjun Guo]

On 2015/6/2 14:12, 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. And a patch set based on this to support ACPI based
> PCIe host bridge on ARM64 has been posted at:

https://lkml.org/lkml/2015/5/26/207

And yes, it works on ARM64 too with legacy interrupts (INT#A/B/C/D) with ACPI _PRT
method (MSI is in developing).

Thanks
Hanjun

>
> It's based on latest mainstream kernel. It passes Fengguang's 0day test
> suite and has been tested on two IA64 platforms and one x86 platform.
>
> V3->V4:
> 1) Add patch[05/08] support solve building issue on ARM64
> 2) Solve an implicitly pointer cast issue.
> 3) Rebase to latest mainstream kernel
>
> V2->V3:
> 1. Move memory allocation/free from ACPI core into arch
> 2. Kill the field 'segment' in struct pci_root_info on x86
>
> Thanks!
> Gerry
>
> Hanjun Guo (1):
>   ARM64 / PCI: introduce struct pci_controller for ACPI
>
> 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/arm64/include/asm/pci.h  |   10 ++
>  arch/ia64/include/asm/pci.h   |    5 -
>  arch/ia64/pci/pci.c           |  364 +++++++++++------------------------------
>  arch/x86/include/asm/pci.h    |   13 +-
>  arch/x86/include/asm/pci_64.h |    4 +-
>  arch/x86/pci/acpi.c           |  295 ++++++++++-----------------------
>  arch/x86/pci/common.c         |    2 +-
>  drivers/acpi/pci_root.c       |  200 ++++++++++++++++++++++
>  drivers/acpi/resource.c       |    9 +-
>  include/linux/ioport.h        |    1 +
>  include/linux/pci-acpi.h      |   23 +++
>  11 files changed, 432 insertions(+), 494 deletions(-)
>

Re: [Patch v4 5/8] ARM64/PCI/ACPI: Introduce struct pci_controller for ACPI

[Lorenzo Pieralisi]

On Tue, Jun 02, 2015 at 07:12:53AM +0100, Jiang Liu wrote:
> From: Hanjun Guo <hanjun.guo@linaro.org>
> 
> ARM64 ACPI based PCI host bridge init needs a arch dependent
> struct pci_controller to accommodate common PCI host bridge
> code which is introduced later, or it will lead to compile
> errors on ARM64.
> 
> Signed-off-by: Hanjun Guo <hanjun.guo@linaro.org>
> Tested-by: Suravee Suthikulpanit <Suravee.Suthikulpanit@amd.com>
> CC: Arnd Bergmann <arnd@arndb.de>
> CC: Catalin Marinas <catalin.marinas@arm.com>
> CC: Liviu Dudau <Liviu.Dudau@arm.com>
> CC: Lorenzo Pieralisi <Lorenzo.Pieralisi@arm.com>
> CC: Will Deacon <will.deacon@arm.com>
> Signed-off-by: Jiang Liu <jiang.liu@linux.intel.com>
> ---
>  arch/arm64/include/asm/pci.h |   10 ++++++++++
>  1 file changed, 10 insertions(+)
> 
> diff --git a/arch/arm64/include/asm/pci.h b/arch/arm64/include/asm/pci.h
> index b008a72f8bc0..70884957f253 100644
> --- a/arch/arm64/include/asm/pci.h
> +++ b/arch/arm64/include/asm/pci.h
> @@ -10,6 +10,16 @@
>  #include <asm-generic/pci-bridge.h>
>  #include <asm-generic/pci-dma-compat.h>
>  
> +struct acpi_device;
> +
> +struct pci_controller {
> +#ifdef CONFIG_ACPI
> +	struct acpi_device *companion;	/* ACPI companion device */
> +#endif
> +	int		segment;	/* PCI domain */
> +	int		node;		/* NUMA node */
> +};

There is nothing ARM64 specific in this structure. The only
reason I see you want to keep it arch specific is the iommu
pointer on x86, but I think we should find a way to make
the common bits shared across archs (ie the struct above) and
add (maybe a void*) to the generic struct to cater for arch
specific data.

Thoughts ?

Lorenzo

> +
>  #define PCIBIOS_MIN_IO		0x1000
>  #define PCIBIOS_MIN_MEM		0
>  
> -- 
> 1.7.10.4
> 

Re: [Patch v4 5/8] ARM64/PCI/ACPI: Introduce struct pci_controller for ACPI

[Hanjun Guo]

On 2015年06月02日 17:35, Lorenzo Pieralisi wrote:
> On Tue, Jun 02, 2015 at 07:12:53AM +0100, Jiang Liu wrote:
>> From: Hanjun Guo <hanjun.guo@linaro.org>
>>
>> ARM64 ACPI based PCI host bridge init needs a arch dependent
>> struct pci_controller to accommodate common PCI host bridge
>> code which is introduced later, or it will lead to compile
>> errors on ARM64.
>>
>> Signed-off-by: Hanjun Guo <hanjun.guo@linaro.org>
>> Tested-by: Suravee Suthikulpanit <Suravee.Suthikulpanit@amd.com>
>> CC: Arnd Bergmann <arnd@arndb.de>
>> CC: Catalin Marinas <catalin.marinas@arm.com>
>> CC: Liviu Dudau <Liviu.Dudau@arm.com>
>> CC: Lorenzo Pieralisi <Lorenzo.Pieralisi@arm.com>
>> CC: Will Deacon <will.deacon@arm.com>
>> Signed-off-by: Jiang Liu <jiang.liu@linux.intel.com>
>> ---
>>   arch/arm64/include/asm/pci.h |   10 ++++++++++
>>   1 file changed, 10 insertions(+)
>>
>> diff --git a/arch/arm64/include/asm/pci.h b/arch/arm64/include/asm/pci.h
>> index b008a72f8bc0..70884957f253 100644
>> --- a/arch/arm64/include/asm/pci.h
>> +++ b/arch/arm64/include/asm/pci.h
>> @@ -10,6 +10,16 @@
>>   #include <asm-generic/pci-bridge.h>
>>   #include <asm-generic/pci-dma-compat.h>
>>
>> +struct acpi_device;
>> +
>> +struct pci_controller {
>> +#ifdef CONFIG_ACPI
>> +	struct acpi_device *companion;	/* ACPI companion device */
>> +#endif
>> +	int		segment;	/* PCI domain */
>> +	int		node;		/* NUMA node */
>> +};
>
> There is nothing ARM64 specific in this structure. The only
> reason I see you want to keep it arch specific is the iommu
> pointer on x86,

And also plarform_data for IA64 too.

> but I think we should find a way to make
> the common bits shared across archs (ie the struct above) and
> add (maybe a void*) to the generic struct to cater for arch
> specific data.
>
> Thoughts ?

We discussed this already, it has limitations to make it
common to all archs, I think the limitation are:

   - struct pci_controller are also used for other archs
     such as PowerPC and Tile, they will not use it for
     ACPI purpose, so we can not used for all archs.

   - if we let struct pci_controller defined only for archs
     using ACPI, such as introduce it in linux/acpi.h, we still
     can not satisfy that the struct pci_controller is not
     only used for ACPI case on x86, it will be used for
     non-ACPI too.

So it's pretty difficult to share it with across archs to me,
any more ideas?

Thanks
Hanjun

Re: [Patch v4 5/8] ARM64/PCI/ACPI: Introduce struct pci_controller for ACPI

[Jiang Liu]

On 2015/6/3 16:44, Hanjun Guo wrote:
> On 2015年06月02日 17:35, Lorenzo Pieralisi wrote:
>> On Tue, Jun 02, 2015 at 07:12:53AM +0100, Jiang Liu wrote:
>>> From: Hanjun Guo <hanjun.guo@linaro.org>
>>>
>>> ARM64 ACPI based PCI host bridge init needs a arch dependent
>>> struct pci_controller to accommodate common PCI host bridge
>>> code which is introduced later, or it will lead to compile
>>> errors on ARM64.
>>>
>>> Signed-off-by: Hanjun Guo <hanjun.guo@linaro.org>
>>> Tested-by: Suravee Suthikulpanit <Suravee.Suthikulpanit@amd.com>
>>> CC: Arnd Bergmann <arnd@arndb.de>
>>> CC: Catalin Marinas <catalin.marinas@arm.com>
>>> CC: Liviu Dudau <Liviu.Dudau@arm.com>
>>> CC: Lorenzo Pieralisi <Lorenzo.Pieralisi@arm.com>
>>> CC: Will Deacon <will.deacon@arm.com>
>>> Signed-off-by: Jiang Liu <jiang.liu@linux.intel.com>
>>> ---
>>>   arch/arm64/include/asm/pci.h |   10 ++++++++++
>>>   1 file changed, 10 insertions(+)
>>>
>>> diff --git a/arch/arm64/include/asm/pci.h b/arch/arm64/include/asm/pci.h
>>> index b008a72f8bc0..70884957f253 100644
>>> --- a/arch/arm64/include/asm/pci.h
>>> +++ b/arch/arm64/include/asm/pci.h
>>> @@ -10,6 +10,16 @@
>>>   #include <asm-generic/pci-bridge.h>
>>>   #include <asm-generic/pci-dma-compat.h>
>>>
>>> +struct acpi_device;
>>> +
>>> +struct pci_controller {
>>> +#ifdef CONFIG_ACPI
>>> +    struct acpi_device *companion;    /* ACPI companion device */
>>> +#endif
>>> +    int        segment;    /* PCI domain */
>>> +    int        node;        /* NUMA node */
>>> +};
>>
>> There is nothing ARM64 specific in this structure. The only
>> reason I see you want to keep it arch specific is the iommu
>> pointer on x86,
> 
> And also plarform_data for IA64 too.
> 
>> but I think we should find a way to make
>> the common bits shared across archs (ie the struct above) and
>> add (maybe a void*) to the generic struct to cater for arch
>> specific data.
>>
>> Thoughts ?
> 
> We discussed this already, it has limitations to make it
> common to all archs, I think the limitation are:
> 
>   - struct pci_controller are also used for other archs
>     such as PowerPC and Tile, they will not use it for
>     ACPI purpose, so we can not used for all archs.
> 
>   - if we let struct pci_controller defined only for archs
>     using ACPI, such as introduce it in linux/acpi.h, we still
>     can not satisfy that the struct pci_controller is not
>     only used for ACPI case on x86, it will be used for
>     non-ACPI too.
> 
> So it's pretty difficult to share it with across archs to me,
> any more ideas?
Hi Hanjun and Lorenzo,
	As mentioned by Hanjun, I have no idea yet about how to
consolidating "struct pci_controller" further. One possible
way is to move "struct pci_controller" related code into
arch, but apparently that will reduce code reusing.
Thanks!
Gerry

> 
> Thanks
> Hanjun

Re: [Patch v4 5/8] ARM64/PCI/ACPI: Introduce struct pci_controller for ACPI

[Lorenzo Pieralisi]

On Wed, Jun 03, 2015 at 10:36:19AM +0100, Jiang Liu wrote:
> On 2015/6/3 16:44, Hanjun Guo wrote:
> > On 2015???06???02??? 17:35, Lorenzo Pieralisi wrote:
> >> On Tue, Jun 02, 2015 at 07:12:53AM +0100, Jiang Liu wrote:
> >>> From: Hanjun Guo <hanjun.guo@linaro.org>
> >>>
> >>> ARM64 ACPI based PCI host bridge init needs a arch dependent
> >>> struct pci_controller to accommodate common PCI host bridge
> >>> code which is introduced later, or it will lead to compile
> >>> errors on ARM64.
> >>>
> >>> Signed-off-by: Hanjun Guo <hanjun.guo@linaro.org>
> >>> Tested-by: Suravee Suthikulpanit <Suravee.Suthikulpanit@amd.com>
> >>> CC: Arnd Bergmann <arnd@arndb.de>
> >>> CC: Catalin Marinas <catalin.marinas@arm.com>
> >>> CC: Liviu Dudau <Liviu.Dudau@arm.com>
> >>> CC: Lorenzo Pieralisi <Lorenzo.Pieralisi@arm.com>
> >>> CC: Will Deacon <will.deacon@arm.com>
> >>> Signed-off-by: Jiang Liu <jiang.liu@linux.intel.com>
> >>> ---
> >>>   arch/arm64/include/asm/pci.h |   10 ++++++++++
> >>>   1 file changed, 10 insertions(+)
> >>>
> >>> diff --git a/arch/arm64/include/asm/pci.h b/arch/arm64/include/asm/pci.h
> >>> index b008a72f8bc0..70884957f253 100644
> >>> --- a/arch/arm64/include/asm/pci.h
> >>> +++ b/arch/arm64/include/asm/pci.h
> >>> @@ -10,6 +10,16 @@
> >>>   #include <asm-generic/pci-bridge.h>
> >>>   #include <asm-generic/pci-dma-compat.h>
> >>>
> >>> +struct acpi_device;
> >>> +
> >>> +struct pci_controller {
> >>> +#ifdef CONFIG_ACPI
> >>> +    struct acpi_device *companion;    /* ACPI companion device */
> >>> +#endif
> >>> +    int        segment;    /* PCI domain */
> >>> +    int        node;        /* NUMA node */
> >>> +};
> >>
> >> There is nothing ARM64 specific in this structure. The only
> >> reason I see you want to keep it arch specific is the iommu
> >> pointer on x86,
> > 
> > And also plarform_data for IA64 too.
> > 
> >> but I think we should find a way to make
> >> the common bits shared across archs (ie the struct above) and
> >> add (maybe a void*) to the generic struct to cater for arch
> >> specific data.
> >>
> >> Thoughts ?
> > 
> > We discussed this already, it has limitations to make it
> > common to all archs, I think the limitation are:
> > 
> >   - struct pci_controller are also used for other archs
> >     such as PowerPC and Tile, they will not use it for
> >     ACPI purpose, so we can not used for all archs.
> > 
> >   - if we let struct pci_controller defined only for archs
> >     using ACPI, such as introduce it in linux/acpi.h, we still
> >     can not satisfy that the struct pci_controller is not
> >     only used for ACPI case on x86, it will be used for
> >     non-ACPI too.
> > 
> > So it's pretty difficult to share it with across archs to me,
> > any more ideas?
> Hi Hanjun and Lorenzo,
> 	As mentioned by Hanjun, I have no idea yet about how to
> consolidating "struct pci_controller" further. One possible
> way is to move "struct pci_controller" related code into
> arch, but apparently that will reduce code reusing.

I guess you can't move that struct pci_controller to generic code
since it is present on other archs too (with completely different
members).

What you can do is creating a new struct (ie same purpose of pci_controller
with a different name) common to all archs that contains the common bits
+ a void* data that contains arch specific data, and convert x86 and ia64
to using it.

It is weird to be forced to declare a pci_controller structure in arm64
code with 0 arch specific data in it.

Lorenzo

Re: [Patch v4 5/8] ARM64/PCI/ACPI: Introduce struct pci_controller for ACPI

[Jiang Liu]

On 2015/6/3 18:03, Lorenzo Pieralisi wrote:
> On Wed, Jun 03, 2015 at 10:36:19AM +0100, Jiang Liu wrote:
>> On 2015/6/3 16:44, Hanjun Guo wrote:
>>> On 2015???06???02??? 17:35, Lorenzo Pieralisi wrote:
>>>> On Tue, Jun 02, 2015 at 07:12:53AM +0100, Jiang Liu wrote:
>>>>> From: Hanjun Guo <hanjun.guo@linaro.org>
>>>>>
>>>>> ARM64 ACPI based PCI host bridge init needs a arch dependent
>>>>> struct pci_controller to accommodate common PCI host bridge
>>>>> code which is introduced later, or it will lead to compile
>>>>> errors on ARM64.
>>>>>
>>>>> Signed-off-by: Hanjun Guo <hanjun.guo@linaro.org>
>>>>> Tested-by: Suravee Suthikulpanit <Suravee.Suthikulpanit@amd.com>
>>>>> CC: Arnd Bergmann <arnd@arndb.de>
>>>>> CC: Catalin Marinas <catalin.marinas@arm.com>
>>>>> CC: Liviu Dudau <Liviu.Dudau@arm.com>
>>>>> CC: Lorenzo Pieralisi <Lorenzo.Pieralisi@arm.com>
>>>>> CC: Will Deacon <will.deacon@arm.com>
>>>>> Signed-off-by: Jiang Liu <jiang.liu@linux.intel.com>
>>>>> ---
>>>>>   arch/arm64/include/asm/pci.h |   10 ++++++++++
>>>>>   1 file changed, 10 insertions(+)
>>>>>
>>>>> diff --git a/arch/arm64/include/asm/pci.h b/arch/arm64/include/asm/pci.h
>>>>> index b008a72f8bc0..70884957f253 100644
>>>>> --- a/arch/arm64/include/asm/pci.h
>>>>> +++ b/arch/arm64/include/asm/pci.h
>>>>> @@ -10,6 +10,16 @@
>>>>>   #include <asm-generic/pci-bridge.h>
>>>>>   #include <asm-generic/pci-dma-compat.h>
>>>>>
>>>>> +struct acpi_device;
>>>>> +
>>>>> +struct pci_controller {
>>>>> +#ifdef CONFIG_ACPI
>>>>> +    struct acpi_device *companion;    /* ACPI companion device */
>>>>> +#endif
>>>>> +    int        segment;    /* PCI domain */
>>>>> +    int        node;        /* NUMA node */
>>>>> +};
>>>>
>>>> There is nothing ARM64 specific in this structure. The only
>>>> reason I see you want to keep it arch specific is the iommu
>>>> pointer on x86,
>>>
>>> And also plarform_data for IA64 too.
>>>
>>>> but I think we should find a way to make
>>>> the common bits shared across archs (ie the struct above) and
>>>> add (maybe a void*) to the generic struct to cater for arch
>>>> specific data.
>>>>
>>>> Thoughts ?
>>>
>>> We discussed this already, it has limitations to make it
>>> common to all archs, I think the limitation are:
>>>
>>>   - struct pci_controller are also used for other archs
>>>     such as PowerPC and Tile, they will not use it for
>>>     ACPI purpose, so we can not used for all archs.
>>>
>>>   - if we let struct pci_controller defined only for archs
>>>     using ACPI, such as introduce it in linux/acpi.h, we still
>>>     can not satisfy that the struct pci_controller is not
>>>     only used for ACPI case on x86, it will be used for
>>>     non-ACPI too.
>>>
>>> So it's pretty difficult to share it with across archs to me,
>>> any more ideas?
>> Hi Hanjun and Lorenzo,
>> 	As mentioned by Hanjun, I have no idea yet about how to
>> consolidating "struct pci_controller" further. One possible
>> way is to move "struct pci_controller" related code into
>> arch, but apparently that will reduce code reusing.
> 
> I guess you can't move that struct pci_controller to generic code
> since it is present on other archs too (with completely different
> members).
> 
> What you can do is creating a new struct (ie same purpose of pci_controller
> with a different name) common to all archs that contains the common bits
> + a void* data that contains arch specific data, and convert x86 and ia64
> to using it.
> 
> It is weird to be forced to declare a pci_controller structure in arm64
> code with 0 arch specific data in it.

Hi Lorenzo,
	I have thought to consolidate pci_controller for x86 and ia64,
but that will make the change set much more bigger. How about to
consolidate pci_controller by another patch set. That will be easier
for review.
Thanks!
Gerry

> 
> Lorenzo
> 

Re: [Patch v4 5/8] ARM64/PCI/ACPI: Introduce struct pci_controller for ACPI

[Lorenzo Pieralisi]

On Wed, Jun 03, 2015 at 11:21:16AM +0100, Jiang Liu wrote:
> On 2015/6/3 18:03, Lorenzo Pieralisi wrote:
> > On Wed, Jun 03, 2015 at 10:36:19AM +0100, Jiang Liu wrote:
> >> On 2015/6/3 16:44, Hanjun Guo wrote:
> >>> On 2015???06???02??? 17:35, Lorenzo Pieralisi wrote:
> >>>> On Tue, Jun 02, 2015 at 07:12:53AM +0100, Jiang Liu wrote:
> >>>>> From: Hanjun Guo <hanjun.guo@linaro.org>
> >>>>>
> >>>>> ARM64 ACPI based PCI host bridge init needs a arch dependent
> >>>>> struct pci_controller to accommodate common PCI host bridge
> >>>>> code which is introduced later, or it will lead to compile
> >>>>> errors on ARM64.
> >>>>>
> >>>>> Signed-off-by: Hanjun Guo <hanjun.guo@linaro.org>
> >>>>> Tested-by: Suravee Suthikulpanit <Suravee.Suthikulpanit@amd.com>
> >>>>> CC: Arnd Bergmann <arnd@arndb.de>
> >>>>> CC: Catalin Marinas <catalin.marinas@arm.com>
> >>>>> CC: Liviu Dudau <Liviu.Dudau@arm.com>
> >>>>> CC: Lorenzo Pieralisi <Lorenzo.Pieralisi@arm.com>
> >>>>> CC: Will Deacon <will.deacon@arm.com>
> >>>>> Signed-off-by: Jiang Liu <jiang.liu@linux.intel.com>
> >>>>> ---
> >>>>>   arch/arm64/include/asm/pci.h |   10 ++++++++++
> >>>>>   1 file changed, 10 insertions(+)
> >>>>>
> >>>>> diff --git a/arch/arm64/include/asm/pci.h b/arch/arm64/include/asm/pci.h
> >>>>> index b008a72f8bc0..70884957f253 100644
> >>>>> --- a/arch/arm64/include/asm/pci.h
> >>>>> +++ b/arch/arm64/include/asm/pci.h
> >>>>> @@ -10,6 +10,16 @@
> >>>>>   #include <asm-generic/pci-bridge.h>
> >>>>>   #include <asm-generic/pci-dma-compat.h>
> >>>>>
> >>>>> +struct acpi_device;
> >>>>> +
> >>>>> +struct pci_controller {
> >>>>> +#ifdef CONFIG_ACPI
> >>>>> +    struct acpi_device *companion;    /* ACPI companion device */
> >>>>> +#endif
> >>>>> +    int        segment;    /* PCI domain */
> >>>>> +    int        node;        /* NUMA node */
> >>>>> +};
> >>>>
> >>>> There is nothing ARM64 specific in this structure. The only
> >>>> reason I see you want to keep it arch specific is the iommu
> >>>> pointer on x86,
> >>>
> >>> And also plarform_data for IA64 too.
> >>>
> >>>> but I think we should find a way to make
> >>>> the common bits shared across archs (ie the struct above) and
> >>>> add (maybe a void*) to the generic struct to cater for arch
> >>>> specific data.
> >>>>
> >>>> Thoughts ?
> >>>
> >>> We discussed this already, it has limitations to make it
> >>> common to all archs, I think the limitation are:
> >>>
> >>>   - struct pci_controller are also used for other archs
> >>>     such as PowerPC and Tile, they will not use it for
> >>>     ACPI purpose, so we can not used for all archs.
> >>>
> >>>   - if we let struct pci_controller defined only for archs
> >>>     using ACPI, such as introduce it in linux/acpi.h, we still
> >>>     can not satisfy that the struct pci_controller is not
> >>>     only used for ACPI case on x86, it will be used for
> >>>     non-ACPI too.
> >>>
> >>> So it's pretty difficult to share it with across archs to me,
> >>> any more ideas?
> >> Hi Hanjun and Lorenzo,
> >> 	As mentioned by Hanjun, I have no idea yet about how to
> >> consolidating "struct pci_controller" further. One possible
> >> way is to move "struct pci_controller" related code into
> >> arch, but apparently that will reduce code reusing.
> > 
> > I guess you can't move that struct pci_controller to generic code
> > since it is present on other archs too (with completely different
> > members).
> > 
> > What you can do is creating a new struct (ie same purpose of pci_controller
> > with a different name) common to all archs that contains the common bits
> > + a void* data that contains arch specific data, and convert x86 and ia64
> > to using it.
> > 
> > It is weird to be forced to declare a pci_controller structure in arm64
> > code with 0 arch specific data in it.
> 
> Hi Lorenzo,
> 	I have thought to consolidate pci_controller for x86 and ia64,
> but that will make the change set much more bigger. How about to
> consolidate pci_controller by another patch set. That will be easier
> for review.

Agreed, but with this set you are forcing arm64 to define pci_controller
as pci_bus sysdata and I am not really keen on that, there are already
function calls in the arm64 pci layer that are there to make ACPI
compile and it is a bit annoying, instead of removing them we are
adding arch stuff on top.

How about passing a void* pointer (ie that is what pci_create_root_bus
expects) to acpi_pci_root_create through a member in acpi_pci_root_info
(I mean acpi_pci_root_info replaces the controller member with a void*
where you can add x86/ia64 pci_controller) ?

I understand this forces you to alloc the pci_controller in arch code,
but that's not a big deal right ? This way you can drop the
pci_controller struct from arm64 (I do not even know if it will ever
be needed, by looking at Hanjun's code, the bits of code that
need the pci_controller can be moved to generic PCI layer).

https://lkml.org/lkml/2015/5/26/215

This way we can add the generic struct we discussed later
(pci_controller refactoring), I agree it is going to be a bigger
change but at least you do not force something into arm64 that we do
not even know if it is required.

Thanks anyway for putting this series together.
Lorenzo

Re: [Patch v4 0/8] Consolidate ACPI PCI root common code into ACPI core

[Al Stone]

On 06/02/2015 12:12 AM, 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. And a patch set based on this to support ACPI based
> PCIe host bridge on ARM64 has been posted at:

Link is missing (or it's a typo of some flavor).

> It's based on latest mainstream kernel. It passes Fengguang's 0day test
> suite and has been tested on two IA64 platforms and one x86 platform.
> 
> V3->V4:
> 1) Add patch[05/08] support solve building issue on ARM64
> 2) Solve an implicitly pointer cast issue.
> 3) Rebase to latest mainstream kernel
> 
> V2->V3:
> 1. Move memory allocation/free from ACPI core into arch
> 2. Kill the field 'segment' in struct pci_root_info on x86
> 
> Thanks!
> Gerry
> 
> Hanjun Guo (1):
>   ARM64 / PCI: introduce struct pci_controller for ACPI
> 
> 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/arm64/include/asm/pci.h  |   10 ++
>  arch/ia64/include/asm/pci.h   |    5 -
>  arch/ia64/pci/pci.c           |  364 +++++++++++------------------------------
>  arch/x86/include/asm/pci.h    |   13 +-
>  arch/x86/include/asm/pci_64.h |    4 +-
>  arch/x86/pci/acpi.c           |  295 ++++++++++-----------------------
>  arch/x86/pci/common.c         |    2 +-
>  drivers/acpi/pci_root.c       |  200 ++++++++++++++++++++++
>  drivers/acpi/resource.c       |    9 +-
>  include/linux/ioport.h        |    1 +
>  include/linux/pci-acpi.h      |   23 +++
>  11 files changed, 432 insertions(+), 494 deletions(-)
> 


-- 
ciao,
al
-----------------------------------
Al Stone
Software Engineer
Red Hat, Inc.
ahs3@redhat.com
-----------------------------------

Re: [Patch v4 0/8] Consolidate ACPI PCI root common code into ACPI core

[Jiang Liu]

On 2015/6/4 4:27, Al Stone wrote:
> On 06/02/2015 12:12 AM, 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. And a patch set based on this to support ACPI based
>> PCIe host bridge on ARM64 has been posted at:
> 
> Link is missing (or it's a typo of some flavor).
HI Al,
	Sorry, I missed the link. It has been posted at:
https://lkml.org/lkml/2015/5/26/207
Thanks!
Gerry

Re: [Patch v4 0/8] Consolidate ACPI PCI root common code into ACPI core

[Hanjun Guo]

Hi Jiang,

On 2015年06月04日 09:54, Jiang Liu wrote:
> On 2015/6/4 4:27, Al Stone wrote:
>> On 06/02/2015 12:12 AM, 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. And a patch set based on this to support ACPI based
>>> PCIe host bridge on ARM64 has been posted at:
>>
>> Link is missing (or it's a typo of some flavor).
> HI Al,
> 	Sorry, I missed the link. It has been posted at:
> https://lkml.org/lkml/2015/5/26/207

I failed to get io resources for PCI hostbridge  when I was testing PCI
on ARM64 QEMU, I debugged this for quite a while, and finally found out
that ACPI resource parsing for IO is not suitable for ARM64, because io
space for x86 is 64K, but 16M for ARM64.

This issue is only found when the firmware representing the io resource
using the type ACPI_RESOURCE_TYPE_ADDRESS32, so the io address will
greater than 64k.

In drivers/acpi/resource.c:

static void acpi_dev_ioresource_flags(struct resource *res, u64 len,
                                       u8 io_decode, u8 translation_type)
{
         res->flags = IORESOURCE_IO;

[...]

         if (res->end >= 0x10003)
                 res->flags |= IORESOURCE_DISABLED | IORESOURCE_UNSET;

[...]
}

so the code will filter out res->end >= 0x10003, and in my case, it will
more than 64K, so we can't get the IO resources.

I got a question, why we use if (res->end >= 0x10003) here?
I mean 64k will be 0x10000, and in that case, we should use
if (res->end >= 0x10000) here, not 0x10003, any history behind that?

This is not the problem of this patch set, but need updating
the core ACPI resource parsing code, I'm working on that. I'm
just wondering there is no special IO space on IA64, how this works
on IA64?

Thanks
Hanjun

Re: [Patch v4 0/8] Consolidate ACPI PCI root common code into ACPI core

[Jiang Liu]

On 2015/6/4 14:31, Hanjun Guo wrote:
> Hi Jiang,
> 
> On 2015年06月04日 09:54, Jiang Liu wrote:
>> On 2015/6/4 4:27, Al Stone wrote:
>>> On 06/02/2015 12:12 AM, 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. And a patch set based on this to support ACPI
>>>> based
>>>> PCIe host bridge on ARM64 has been posted at:
>>>
>>> Link is missing (or it's a typo of some flavor).
>> HI Al,
>>     Sorry, I missed the link. It has been posted at:
>> https://lkml.org/lkml/2015/5/26/207
> 
> I failed to get io resources for PCI hostbridge  when I was testing PCI
> on ARM64 QEMU, I debugged this for quite a while, and finally found out
> that ACPI resource parsing for IO is not suitable for ARM64, because io
> space for x86 is 64K, but 16M for ARM64.
> 
> This issue is only found when the firmware representing the io resource
> using the type ACPI_RESOURCE_TYPE_ADDRESS32, so the io address will
> greater than 64k.
> 
> In drivers/acpi/resource.c:
> 
> static void acpi_dev_ioresource_flags(struct resource *res, u64 len,
>                                       u8 io_decode, u8 translation_type)
> {
>         res->flags = IORESOURCE_IO;
> 
> [...]
> 
>         if (res->end >= 0x10003)
>                 res->flags |= IORESOURCE_DISABLED | IORESOURCE_UNSET;
> 
> [...]
> }
> 
> so the code will filter out res->end >= 0x10003, and in my case, it will
> more than 64K, so we can't get the IO resources.
> 
> I got a question, why we use if (res->end >= 0x10003) here?
> I mean 64k will be 0x10000, and in that case, we should use
> if (res->end >= 0x10000) here, not 0x10003, any history behind that?

Hi Hanjun,
This is a special tricky for x86. You may read a dword(four bytes) from
IO port 0xffff, so the effective io port space is 0x10003 bytes.

> 
> This is not the problem of this patch set, but need updating
> the core ACPI resource parsing code, I'm working on that. I'm
> just wondering there is no special IO space on IA64, how this works
> on IA64?
There is special handling for IO port on IA64. IA64 io ports are
actually memory-mapped, and there may be multiple 64K IO port spaces.
For example, each PCI domain may have its own 64k memory-mapped
IO space.
Thanks!
Gerry
> 
> Thanks
> Hanjun

Re: [Patch v4 0/8] Consolidate ACPI PCI root common code into ACPI core

[Hanjun Guo]

On 2015年06月04日 14:41, Jiang Liu wrote:
> On 2015/6/4 14:31, Hanjun Guo wrote:
>> Hi Jiang,
>>
>> On 2015年06月04日 09:54, Jiang Liu wrote:
>>> On 2015/6/4 4:27, Al Stone wrote:
>>>> On 06/02/2015 12:12 AM, 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. And a patch set based on this to support ACPI
>>>>> based
>>>>> PCIe host bridge on ARM64 has been posted at:
>>>>
>>>> Link is missing (or it's a typo of some flavor).
>>> HI Al,
>>>      Sorry, I missed the link. It has been posted at:
>>> https://lkml.org/lkml/2015/5/26/207
>>
>> I failed to get io resources for PCI hostbridge  when I was testing PCI
>> on ARM64 QEMU, I debugged this for quite a while, and finally found out
>> that ACPI resource parsing for IO is not suitable for ARM64, because io
>> space for x86 is 64K, but 16M for ARM64.
>>
>> This issue is only found when the firmware representing the io resource
>> using the type ACPI_RESOURCE_TYPE_ADDRESS32, so the io address will
>> greater than 64k.
>>
>> In drivers/acpi/resource.c:
>>
>> static void acpi_dev_ioresource_flags(struct resource *res, u64 len,
>>                                        u8 io_decode, u8 translation_type)
>> {
>>          res->flags = IORESOURCE_IO;
>>
>> [...]
>>
>>          if (res->end >= 0x10003)
>>                  res->flags |= IORESOURCE_DISABLED | IORESOURCE_UNSET;
>>
>> [...]
>> }
>>
>> so the code will filter out res->end >= 0x10003, and in my case, it will
>> more than 64K, so we can't get the IO resources.
>>
>> I got a question, why we use if (res->end >= 0x10003) here?
>> I mean 64k will be 0x10000, and in that case, we should use
>> if (res->end >= 0x10000) here, not 0x10003, any history behind that?
>
> Hi Hanjun,
> This is a special tricky for x86. You may read a dword(four bytes) from
> IO port 0xffff, so the effective io port space is 0x10003 bytes.

Thanks for the explanation, how about add a patch to comment on it?
if it's ok to you and will improve the code readability, I can prepare
one.

>
>>
>> This is not the problem of this patch set, but need updating
>> the core ACPI resource parsing code, I'm working on that. I'm
>> just wondering there is no special IO space on IA64, how this works
>> on IA64?
> There is special handling for IO port on IA64. IA64 io ports are
> actually memory-mapped, and there may be multiple 64K IO port spaces.
> For example, each PCI domain may have its own 64k memory-mapped
> IO space.

That's the case for ARM64 too, I will review the IA64 code for
reference, great thanks for the help and explanation :)

Thanks
Hanjun

Re: [Patch v4 0/8] Consolidate ACPI PCI root common code into ACPI core

[Mark Salter]

On Thu, 2015-06-04 at 14:41 +0800, Jiang Liu wrote:
> On 2015/6/4 14:31, Hanjun Guo wrote:
> > Hi Jiang,
> > 
> > On 2015年06月04日 09:54, Jiang Liu wrote:
> >> On 2015/6/4 4:27, Al Stone wrote:
> >>> On 06/02/2015 12:12 AM, 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. And a patch set based on this to support ACPI
> >>>> based
> >>>> PCIe host bridge on ARM64 has been posted at:
> >>>
> >>> Link is missing (or it's a typo of some flavor).
> >> HI Al,
> >>     Sorry, I missed the link. It has been posted at:
> >> https://lkml.org/lkml/2015/5/26/207
> > 
> > I failed to get io resources for PCI hostbridge  when I was testing PCI
> > on ARM64 QEMU, I debugged this for quite a while, and finally found out
> > that ACPI resource parsing for IO is not suitable for ARM64, because io
> > space for x86 is 64K, but 16M for ARM64.
> > 
> > This issue is only found when the firmware representing the io resource
> > using the type ACPI_RESOURCE_TYPE_ADDRESS32, so the io address will
> > greater than 64k.
> > 
> > In drivers/acpi/resource.c:
> > 
> > static void acpi_dev_ioresource_flags(struct resource *res, u64 len,
> >                                       u8 io_decode, u8 translation_type)
> > {
> >         res->flags = IORESOURCE_IO;
> > 
> > [...]
> > 
> >         if (res->end >= 0x10003)
> >                 res->flags |= IORESOURCE_DISABLED | IORESOURCE_UNSET;
> > 
> > [...]
> > }
> > 
> > so the code will filter out res->end >= 0x10003, and in my case, it will
> > more than 64K, so we can't get the IO resources.
> > 
> > I got a question, why we use if (res->end >= 0x10003) here?
> > I mean 64k will be 0x10000, and in that case, we should use
> > if (res->end >= 0x10000) here, not 0x10003, any history behind that?
> 
> Hi Hanjun,
> This is a special tricky for x86. You may read a dword(four bytes) from
> IO port 0xffff, so the effective io port space is 0x10003 bytes.
> 

Is there something in ACPI spec which would limit PCI IO space to 64K?
PCI itself allows 32-bit IO addresses and at least some arm64 platforms
use PCI bus addresses above 64K for IO transactions. From a PCI view,
the (res->end >= 0x10003) check doesn't make sense. Am I missing
something?

Re: [Patch v4 0/8] Consolidate ACPI PCI root common code into ACPI core

[Jiang Liu]

On 2015/6/4 23:51, Mark Salter wrote:
> On Thu, 2015-06-04 at 14:41 +0800, Jiang Liu wrote:
>> On 2015/6/4 14:31, Hanjun Guo wrote:
>>> Hi Jiang,
>>>
>>> On 2015年06月04日 09:54, Jiang Liu wrote:
>>>> On 2015/6/4 4:27, Al Stone wrote:
>>>>> On 06/02/2015 12:12 AM, 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. And a patch set based on this to support ACPI
>>>>>> based
>>>>>> PCIe host bridge on ARM64 has been posted at:
>>>>>
>>>>> Link is missing (or it's a typo of some flavor).
>>>> HI Al,
>>>>     Sorry, I missed the link. It has been posted at:
>>>> https://lkml.org/lkml/2015/5/26/207
>>>
>>> I failed to get io resources for PCI hostbridge  when I was testing PCI
>>> on ARM64 QEMU, I debugged this for quite a while, and finally found out
>>> that ACPI resource parsing for IO is not suitable for ARM64, because io
>>> space for x86 is 64K, but 16M for ARM64.
>>>
>>> This issue is only found when the firmware representing the io resource
>>> using the type ACPI_RESOURCE_TYPE_ADDRESS32, so the io address will
>>> greater than 64k.
>>>
>>> In drivers/acpi/resource.c:
>>>
>>> static void acpi_dev_ioresource_flags(struct resource *res, u64 len,
>>>                                       u8 io_decode, u8 translation_type)
>>> {
>>>         res->flags = IORESOURCE_IO;
>>>
>>> [...]
>>>
>>>         if (res->end >= 0x10003)
>>>                 res->flags |= IORESOURCE_DISABLED | IORESOURCE_UNSET;
>>>
>>> [...]
>>> }
>>>
>>> so the code will filter out res->end >= 0x10003, and in my case, it will
>>> more than 64K, so we can't get the IO resources.
>>>
>>> I got a question, why we use if (res->end >= 0x10003) here?
>>> I mean 64k will be 0x10000, and in that case, we should use
>>> if (res->end >= 0x10000) here, not 0x10003, any history behind that?
>>
>> Hi Hanjun,
>> This is a special tricky for x86. You may read a dword(four bytes) from
>> IO port 0xffff, so the effective io port space is 0x10003 bytes.
>>
> 
> Is there something in ACPI spec which would limit PCI IO space to 64K?
> PCI itself allows 32-bit IO addresses and at least some arm64 platforms
> use PCI bus addresses above 64K for IO transactions. From a PCI view,
> the (res->end >= 0x10003) check doesn't make sense. Am I missing
> something?
HI Mark,
	Something interesting here. According to my understanding,
the actually limitations are
1) the maximum size for each IO port space is 64k,
2) each PCI segment may only have one IO port space assigned at most.

Other than those, it's flexible for system designer to:
1) have multiple IO port spaces, each is 64K at most.
2) CPU may use MMIO transactions to access PCI IO space, and PCI host
   bridge will do the translation from CPU side MMIO address to PCI side
   IO port address.

For example, we may have following configuration on IA64 platforms:
1) CPU side physical address [0x100000000-0x100010000] maps to IO space
   [0x00000-0x10000] on PCI segment 0
2) CPU side physical address [0x100010000-0x100020000] maps to IO space
   [0x00000-0x10000] on PCI segment 1
And ACPI resource descriptor provides 'translation_offset' to support
such an usage case. Hope this helps:)
Thanks!
Gerry
> 
> 
> 

Re: [Patch v4 0/8] Consolidate ACPI PCI root common code into ACPI core

[Mark Salter]

On Fri, 2015-06-05 at 00:29 +0800, Jiang Liu wrote:
> On 2015/6/4 23:51, Mark Salter wrote:
> > On Thu, 2015-06-04 at 14:41 +0800, Jiang Liu wrote:
> >> On 2015/6/4 14:31, Hanjun Guo wrote:
> >>> Hi Jiang,
> >>>
> >>> On 2015年06月04日 09:54, Jiang Liu wrote:
> >>>> On 2015/6/4 4:27, Al Stone wrote:
> >>>>> On 06/02/2015 12:12 AM, 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. And a patch set based on this to support ACPI
> >>>>>> based
> >>>>>> PCIe host bridge on ARM64 has been posted at:
> >>>>>
> >>>>> Link is missing (or it's a typo of some flavor).
> >>>> HI Al,
> >>>>     Sorry, I missed the link. It has been posted at:
> >>>> https://lkml.org/lkml/2015/5/26/207
> >>>
> >>> I failed to get io resources for PCI hostbridge  when I was testing PCI
> >>> on ARM64 QEMU, I debugged this for quite a while, and finally found out
> >>> that ACPI resource parsing for IO is not suitable for ARM64, because io
> >>> space for x86 is 64K, but 16M for ARM64.
> >>>
> >>> This issue is only found when the firmware representing the io resource
> >>> using the type ACPI_RESOURCE_TYPE_ADDRESS32, so the io address will
> >>> greater than 64k.
> >>>
> >>> In drivers/acpi/resource.c:
> >>>
> >>> static void acpi_dev_ioresource_flags(struct resource *res, u64 len,
> >>>                                       u8 io_decode, u8 translation_type)
> >>> {
> >>>         res->flags = IORESOURCE_IO;
> >>>
> >>> [...]
> >>>
> >>>         if (res->end >= 0x10003)
> >>>                 res->flags |= IORESOURCE_DISABLED | IORESOURCE_UNSET;
> >>>
> >>> [...]
> >>> }
> >>>
> >>> so the code will filter out res->end >= 0x10003, and in my case, it will
> >>> more than 64K, so we can't get the IO resources.
> >>>
> >>> I got a question, why we use if (res->end >= 0x10003) here?
> >>> I mean 64k will be 0x10000, and in that case, we should use
> >>> if (res->end >= 0x10000) here, not 0x10003, any history behind that?
> >>
> >> Hi Hanjun,
> >> This is a special tricky for x86. You may read a dword(four bytes) from
> >> IO port 0xffff, so the effective io port space is 0x10003 bytes.
> >>
> > 
> > Is there something in ACPI spec which would limit PCI IO space to 64K?
> > PCI itself allows 32-bit IO addresses and at least some arm64 platforms
> > use PCI bus addresses above 64K for IO transactions. From a PCI view,
> > the (res->end >= 0x10003) check doesn't make sense. Am I missing
> > something?
> HI Mark,
> 	Something interesting here. According to my understanding,
> the actually limitations are
> 1) the maximum size for each IO port space is 64k,

PCI bridges may support a full 4GiB IO space on a single bus.

See drivers/pci/probe.c:pci_read_bridge_io() where it checks to see if
bridge supports 32-bit IO space.

> 2) each PCI segment may only have one IO port space assigned at most.
> 
> Other than those, it's flexible for system designer to:
> 1) have multiple IO port spaces, each is 64K at most.
> 2) CPU may use MMIO transactions to access PCI IO space, and PCI host
>    bridge will do the translation from CPU side MMIO address to PCI side
>    IO port address.


> For example, we may have following configuration on IA64 platforms:
> 1) CPU side physical address [0x100000000-0x100010000] maps to IO space
>    [0x00000-0x10000] on PCI segment 0
> 2) CPU side physical address [0x100010000-0x100020000] maps to IO space
>    [0x00000-0x10000] on PCI segment 1
> And ACPI resource descriptor provides 'translation_offset' to support
> such an usage case. Hope this helps:)
> Thanks!
> Gerry
> > 
> > 
> > 

Re: [Patch v4 0/8] Consolidate ACPI PCI root common code into ACPI core

[Hanjun Guo]

On 2015年06月05日 00:29, Jiang Liu wrote:
> On 2015/6/4 23:51, Mark Salter wrote:
>> On Thu, 2015-06-04 at 14:41 +0800, Jiang Liu wrote:
>>> On 2015/6/4 14:31, Hanjun Guo wrote:
>>>> Hi Jiang,
>>>>
>>>> On 2015年06月04日 09:54, Jiang Liu wrote:
>>>>> On 2015/6/4 4:27, Al Stone wrote:
>>>>>> On 06/02/2015 12:12 AM, 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. And a patch set based on this to support ACPI
>>>>>>> based
>>>>>>> PCIe host bridge on ARM64 has been posted at:
>>>>>>
>>>>>> Link is missing (or it's a typo of some flavor).
>>>>> HI Al,
>>>>>      Sorry, I missed the link. It has been posted at:
>>>>> https://lkml.org/lkml/2015/5/26/207
>>>>
>>>> I failed to get io resources for PCI hostbridge  when I was testing PCI
>>>> on ARM64 QEMU, I debugged this for quite a while, and finally found out
>>>> that ACPI resource parsing for IO is not suitable for ARM64, because io
>>>> space for x86 is 64K, but 16M for ARM64.
>>>>
>>>> This issue is only found when the firmware representing the io resource
>>>> using the type ACPI_RESOURCE_TYPE_ADDRESS32, so the io address will
>>>> greater than 64k.
>>>>
>>>> In drivers/acpi/resource.c:
>>>>
>>>> static void acpi_dev_ioresource_flags(struct resource *res, u64 len,
>>>>                                        u8 io_decode, u8 translation_type)
>>>> {
>>>>          res->flags = IORESOURCE_IO;
>>>>
>>>> [...]
>>>>
>>>>          if (res->end >= 0x10003)
>>>>                  res->flags |= IORESOURCE_DISABLED | IORESOURCE_UNSET;
>>>>
>>>> [...]
>>>> }
>>>>
>>>> so the code will filter out res->end >= 0x10003, and in my case, it will
>>>> more than 64K, so we can't get the IO resources.
>>>>
>>>> I got a question, why we use if (res->end >= 0x10003) here?
>>>> I mean 64k will be 0x10000, and in that case, we should use
>>>> if (res->end >= 0x10000) here, not 0x10003, any history behind that?
>>>
>>> Hi Hanjun,
>>> This is a special tricky for x86. You may read a dword(four bytes) from
>>> IO port 0xffff, so the effective io port space is 0x10003 bytes.
>>>
>>
>> Is there something in ACPI spec which would limit PCI IO space to 64K?
>> PCI itself allows 32-bit IO addresses and at least some arm64 platforms
>> use PCI bus addresses above 64K for IO transactions. From a PCI view,
>> the (res->end >= 0x10003) check doesn't make sense. Am I missing
>> something?
> HI Mark,
> 	Something interesting here. According to my understanding,
> the actually limitations are
> 1) the maximum size for each IO port space is 64k,
> 2) each PCI segment may only have one IO port space assigned at most.
>
> Other than those, it's flexible for system designer to:
> 1) have multiple IO port spaces, each is 64K at most.
> 2) CPU may use MMIO transactions to access PCI IO space, and PCI host
>     bridge will do the translation from CPU side MMIO address to PCI side
>     IO port address.
>
> For example, we may have following configuration on IA64 platforms:
> 1) CPU side physical address [0x100000000-0x100010000] maps to IO space
>     [0x00000-0x10000] on PCI segment 0
> 2) CPU side physical address [0x100010000-0x100020000] maps to IO space
>     [0x00000-0x10000] on PCI segment 1
> And ACPI resource descriptor provides 'translation_offset' to support
> such an usage case. Hope this helps:)

Hi Jiang,

It seems the translation_offset for IA64 is assuming to zero in this
patch set, which have regressions if we use DworldIO() with offset
to report io port resources, am I missing something?

Thanks
Hanjun