[Patch v4 00/16] Enable support of IOAPIC hotplug on x86 platforms

[Patch v4 00/16] Enable support of IOAPIC hotplug on x86 platforms

[Jiang Liu]

This patch set enhances IOAPIC core and ACPI drivers to support IOAPIC
hotplug on x86 platforms. It's based on latest mainstream kernel at
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git

You may pull it from 
https://github.com/jiangliu/linux.git ioapic/hotplug_v4

We have pick up several patches from Yinghai's original IOAPIC hotplug
patch set and reimplemented IOAPIC driver as an ACPI driver instead of
a PCI driver.

It has been tested on a 4-socket Intel SDV with socket hot-addition
capability. Any suggestions are welcomed!

Patch 1-5 are bugfixes and enhancements to ACPI subsystem
Patch 6-14 enhances IOAPIC core to support IOAPIC hotplug
Patch 15 killes PCI IOAPIC driver
Patch 16 reimplements ACPI IOAPIC driver and enables IOAPIC hotplug

V3->V4:
1) Fix a bug in manage IOAPIC reference count
2) Rebase to v3.17-rc2
3) Refine commit messages
V2->V3:
1) Refine ACPI resource walk functions for PCI root bus and IOAPIC
2) Improve commit messages
3) Reorder patch order for better maintenence

Jiang Liu (13):
  x86, PCI, ACPI: Kill private function resource_to_addr() in
    arch/x86/pci/acpi.c
  ACPI: Correct return value of acpi_dev_resource_address_space()
  ACPI: Fix minor syntax issues in processor_core.c
  ACPI: Rename processor_core.c as apic_id.c
  x86, irq: Remove __init marker for functions will be used by IOAPIC
    hotplug
  x86, irq: Keep balance of IOAPIC pin reference count
  x86, irq: Refine mp_register_ioapic() to prepare for IOAPIC hotplug
  x86, irq, ACPI: Introduce a rwsem to protect IOAPIC operations from
    hotplug
  x86, irq, ACPI: Implement interface to support ACPI based IOAPIC
    hot-addition
  x86, irq, ACPI: Implement interfaces to support ACPI based IOAPIC
    hot-removal
  x86, irq: Introduce helper to check whether an IOAPIC has been
    registered
  PCI: Remove PCI ioapic driver
  x86, irq, ACPI: Implement ACPI driver to support IOAPIC hotplug

Yinghai Lu (3):
  ACPI: Add interfaces to parse IOAPIC ID for IOAPIC hotplug
  x86, irq: Split out alloc_ioapic_save_registers()
  x86, irq: Prefer assigned ID in APIC ID register for x86_64

 arch/x86/include/asm/io_apic.h |    6 +-
 arch/x86/kernel/acpi/boot.c    |   68 +++++++++-
 arch/x86/kernel/apic/io_apic.c |  235 +++++++++++++++++++++++++-------
 arch/x86/pci/acpi.c            |  142 +++++++------------
 arch/x86/pci/intel_mid_pci.c   |    9 +-
 arch/x86/pci/irq.c             |    7 +-
 drivers/acpi/Kconfig           |    6 +
 drivers/acpi/Makefile          |    3 +-
 drivers/acpi/apic_id.c         |  294 ++++++++++++++++++++++++++++++++++++++++
 drivers/acpi/internal.h        |    7 +
 drivers/acpi/ioapic.c          |  236 ++++++++++++++++++++++++++++++++
 drivers/acpi/pci_irq.c         |   11 +-
 drivers/acpi/pci_root.c        |    3 +
 drivers/acpi/processor_core.c  |  206 ----------------------------
 drivers/acpi/resource.c        |    2 +-
 drivers/pci/Kconfig            |    7 -
 drivers/pci/Makefile           |    2 -
 drivers/pci/ioapic.c           |  121 -----------------
 include/acpi/processor.h       |    3 -
 include/linux/acpi.h           |    8 ++
 include/linux/pci.h            |    1 +
 21 files changed, 885 insertions(+), 492 deletions(-)
 create mode 100644 drivers/acpi/apic_id.c
 create mode 100644 drivers/acpi/ioapic.c
 delete mode 100644 drivers/acpi/processor_core.c
 delete mode 100644 drivers/pci/ioapic.c

-- 
1.7.10.4

[Patch v4 01/16] x86, PCI, ACPI: Kill private function resource_to_addr() in arch/x86/pci/acpi.c

[Jiang Liu]

Private function resource_to_addr() is used to parse ACPI resources
for PCI host bridge. There are public interfaces available for that
purpose, so replace resource_to_addr() with public interfaces.

Reviewed-by: Bjorn Helgaas <bhelgaas@google.com>
Signed-off-by: Jiang Liu <jiang.liu@linux.intel.com>
---
 arch/x86/pci/acpi.c |  142 ++++++++++++++++++---------------------------------
 1 file changed, 51 insertions(+), 91 deletions(-)

diff --git a/arch/x86/pci/acpi.c b/arch/x86/pci/acpi.c
index cfd1b132b8e3..0e716fa56ae5 100644
--- a/arch/x86/pci/acpi.c
+++ b/arch/x86/pci/acpi.c
@@ -218,114 +218,74 @@ static void teardown_mcfg_map(struct pci_root_info *info)
 }
 #endif
 
-static acpi_status resource_to_addr(struct acpi_resource *resource,
-				    struct acpi_resource_address64 *addr)
-{
-	acpi_status status;
-	struct acpi_resource_memory24 *memory24;
-	struct acpi_resource_memory32 *memory32;
-	struct acpi_resource_fixed_memory32 *fixed_memory32;
-
-	memset(addr, 0, sizeof(*addr));
-	switch (resource->type) {
-	case ACPI_RESOURCE_TYPE_MEMORY24:
-		memory24 = &resource->data.memory24;
-		addr->resource_type = ACPI_MEMORY_RANGE;
-		addr->minimum = memory24->minimum;
-		addr->address_length = memory24->address_length;
-		addr->maximum = addr->minimum + addr->address_length - 1;
-		return AE_OK;
-	case ACPI_RESOURCE_TYPE_MEMORY32:
-		memory32 = &resource->data.memory32;
-		addr->resource_type = ACPI_MEMORY_RANGE;
-		addr->minimum = memory32->minimum;
-		addr->address_length = memory32->address_length;
-		addr->maximum = addr->minimum + addr->address_length - 1;
-		return AE_OK;
-	case ACPI_RESOURCE_TYPE_FIXED_MEMORY32:
-		fixed_memory32 = &resource->data.fixed_memory32;
-		addr->resource_type = ACPI_MEMORY_RANGE;
-		addr->minimum = fixed_memory32->address;
-		addr->address_length = fixed_memory32->address_length;
-		addr->maximum = addr->minimum + addr->address_length - 1;
-		return AE_OK;
-	case ACPI_RESOURCE_TYPE_ADDRESS16:
-	case ACPI_RESOURCE_TYPE_ADDRESS32:
-	case ACPI_RESOURCE_TYPE_ADDRESS64:
-		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_length > 0) {
-			return AE_OK;
-		}
-		break;
-	}
-	return AE_ERROR;
-}
-
 static acpi_status count_resource(struct acpi_resource *acpi_res, void *data)
 {
 	struct pci_root_info *info = data;
-	struct acpi_resource_address64 addr;
-	acpi_status status;
+	struct resource r = {
+		.flags = 0
+	};
 
-	status = resource_to_addr(acpi_res, &addr);
-	if (ACPI_SUCCESS(status))
+	if (!acpi_dev_resource_memory(acpi_res, &r) &&
+	    !acpi_dev_resource_address_space(acpi_res, &r))
+		return AE_OK;
+
+	if ((r.flags & (IORESOURCE_IO | IORESOURCE_MEM)) && resource_size(&r))
 		info->res_num++;
+
 	return AE_OK;
 }
 
 static acpi_status setup_resource(struct acpi_resource *acpi_res, void *data)
 {
 	struct pci_root_info *info = data;
-	struct resource *res;
-	struct acpi_resource_address64 addr;
-	acpi_status status;
-	unsigned long flags;
-	u64 start, orig_end, end;
+	struct resource *res = &info->res[info->res_num];
+	u64 translation_offset = 0;
+
+	memset(res, 0, sizeof(*res));
+	if (acpi_dev_resource_memory(acpi_res, res)) {
+		res->flags &= IORESOURCE_MEM | IORESOURCE_IO;
+	} else if (acpi_dev_resource_address_space(acpi_res, res)) {
+		u64 orig_end;
+		struct acpi_resource_address64 addr;
+
+		res->flags &= IORESOURCE_MEM | IORESOURCE_IO;
+		if (res->flags == 0)
+			return AE_OK;
 
-	status = resource_to_addr(acpi_res, &addr);
-	if (!ACPI_SUCCESS(status))
-		return AE_OK;
+		if (ACPI_FAILURE(acpi_resource_to_address64(acpi_res, &addr)))
+			return AE_OK;
 
-	if (addr.resource_type == ACPI_MEMORY_RANGE) {
-		flags = IORESOURCE_MEM;
-		if (addr.info.mem.caching == ACPI_PREFETCHABLE_MEMORY)
-			flags |= IORESOURCE_PREFETCH;
-	} else if (addr.resource_type == ACPI_IO_RANGE) {
-		flags = IORESOURCE_IO;
-	} else
-		return AE_OK;
+		if (addr.resource_type == ACPI_MEMORY_RANGE &&
+		    addr.info.mem.caching == ACPI_PREFETCHABLE_MEMORY)
+			res->flags |= IORESOURCE_PREFETCH;
 
-	start = addr.minimum + addr.translation_offset;
-	orig_end = end = addr.maximum + addr.translation_offset;
+		translation_offset = addr.translation_offset;
+		orig_end = res->end;
+		res->start += translation_offset;
+		res->end += translation_offset;
 
-	/* Exclude non-addressable range or non-addressable portion of range */
-	end = min(end, (u64)iomem_resource.end);
-	if (end <= start) {
-		dev_info(&info->bridge->dev,
-			"host bridge window [%#llx-%#llx] "
-			"(ignored, not CPU addressable)\n", start, orig_end);
-		return AE_OK;
-	} else if (orig_end != end) {
-		dev_info(&info->bridge->dev,
-			"host bridge window [%#llx-%#llx] "
-			"([%#llx-%#llx] ignored, not CPU addressable)\n", 
-			start, orig_end, end + 1, orig_end);
+		/* Exclude non-addressable range or non-addressable portion of range */
+		res->end = min(res->end, (u64)iomem_resource.end);
+		if (res->end <= res->start) {
+			dev_info(&info->bridge->dev,
+				"host bridge window [%#llx-%#llx] (ignored, not CPU addressable)\n",
+				 res->start, orig_end);
+			return AE_OK;
+		} else if (orig_end != res->end) {
+			dev_info(&info->bridge->dev,
+				"host bridge window [%#llx-%#llx] ([%#llx-%#llx] ignored, not CPU addressable)\n",
+				res->start, orig_end, res->end + 1, orig_end);
+		}
 	}
 
-	res = &info->res[info->res_num];
-	res->name = info->name;
-	res->flags = flags;
-	res->start = start;
-	res->end = end;
-	info->res_offset[info->res_num] = addr.translation_offset;
-	info->res_num++;
-
-	if (!pci_use_crs)
-		dev_printk(KERN_DEBUG, &info->bridge->dev,
-			   "host bridge window %pR (ignored)\n", res);
+	if (res->flags && resource_size(res)) {
+		res->name = info->name;
+		info->res_offset[info->res_num] = translation_offset;
+		info->res_num++;
+		if (!pci_use_crs)
+			dev_printk(KERN_DEBUG, &info->bridge->dev,
+				   "host bridge window %pR (ignored)\n", res);
+	}
 
 	return AE_OK;
 }
-- 
1.7.10.4

[Patch v4 02/16] ACPI: Correct return value of acpi_dev_resource_address_space()

[Jiang Liu]

Change acpi_dev_resource_address_space() to return failure if the
acpi_resource structure can't be converted to an ACPI address64
structure, so caller could correctly detect failure.

Signed-off-by: Jiang Liu <jiang.liu@linux.intel.com>
---
 drivers/acpi/resource.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/acpi/resource.c b/drivers/acpi/resource.c
index 2ba8f02ced36..782a0d15c25f 100644
--- a/drivers/acpi/resource.c
+++ b/drivers/acpi/resource.c
@@ -200,7 +200,7 @@ bool acpi_dev_resource_address_space(struct acpi_resource *ares,
 
 	status = acpi_resource_to_address64(ares, &addr);
 	if (ACPI_FAILURE(status))
-		return true;
+		return false;
 
 	res->start = addr.minimum;
 	res->end = addr.maximum;
-- 
1.7.10.4

[Patch v4 03/16] ACPI: Fix minor syntax issues in processor_core.c

[Jiang Liu]

Fix minor syntax issues in processor.c to follow coding styles.

Signed-off-by: Jiang Liu <jiang.liu@linux.intel.com>
---
 drivers/acpi/processor_core.c |    9 ++++-----
 1 file changed, 4 insertions(+), 5 deletions(-)

diff --git a/drivers/acpi/processor_core.c b/drivers/acpi/processor_core.c
index e32321ce9a5c..b048f3752c2b 100644
--- a/drivers/acpi/processor_core.c
+++ b/drivers/acpi/processor_core.c
@@ -125,13 +125,12 @@ static int map_mat_entry(acpi_handle handle, int type, u32 acpi_id)
 	}
 
 	header = (struct acpi_subtable_header *)obj->buffer.pointer;
-	if (header->type == ACPI_MADT_TYPE_LOCAL_APIC) {
+	if (header->type == ACPI_MADT_TYPE_LOCAL_APIC)
 		map_lapic_id(header, acpi_id, &apic_id);
-	} else if (header->type == ACPI_MADT_TYPE_LOCAL_SAPIC) {
+	else if (header->type == ACPI_MADT_TYPE_LOCAL_SAPIC)
 		map_lsapic_id(header, type, acpi_id, &apic_id);
-	} else if (header->type == ACPI_MADT_TYPE_LOCAL_X2APIC) {
+	else if (header->type == ACPI_MADT_TYPE_LOCAL_X2APIC)
 		map_x2apic_id(header, type, acpi_id, &apic_id);
-	}
 
 exit:
 	kfree(buffer.pointer);
@@ -164,7 +163,7 @@ int acpi_map_cpuid(int apic_id, u32 acpi_id)
 		 * For example,
 		 *
 		 * Scope (_PR)
-                 * {
+		 * {
 		 *     Processor (CPU0, 0x00, 0x00000410, 0x06) {}
 		 *     Processor (CPU1, 0x01, 0x00000410, 0x06) {}
 		 *     Processor (CPU2, 0x02, 0x00000410, 0x06) {}
-- 
1.7.10.4

[Patch v4 04/16] ACPI: Rename processor_core.c as apic_id.c

[Jiang Liu]

Now all code in processor_core.c is APIC ID related, so rename it as
apic_id.c. Later IOAPIC ID related code will be added into apic_id.c.

Signed-off-by: Jiang Liu <jiang.liu@linux.intel.com>
---
 drivers/acpi/Makefile         |    2 +-
 drivers/acpi/apic_id.c        |  202 ++++++++++++++++++++++++++++++++++++++++
 drivers/acpi/processor_core.c |  205 -----------------------------------------
 include/acpi/processor.h      |    3 -
 include/linux/acpi.h          |    3 +
 5 files changed, 206 insertions(+), 209 deletions(-)
 create mode 100644 drivers/acpi/apic_id.c
 delete mode 100644 drivers/acpi/processor_core.c

diff --git a/drivers/acpi/Makefile b/drivers/acpi/Makefile
index 505d4d79fe3e..03ddd03f2bcd 100644
--- a/drivers/acpi/Makefile
+++ b/drivers/acpi/Makefile
@@ -35,7 +35,7 @@ acpi-y				+= bus.o glue.o
 acpi-y				+= scan.o
 acpi-y				+= resource.o
 acpi-y				+= acpi_processor.o
-acpi-y				+= processor_core.o
+acpi-y				+= apic_id.o
 acpi-$(CONFIG_ARCH_MIGHT_HAVE_ACPI_PDC) += processor_pdc.o
 acpi-y				+= ec.o
 acpi-$(CONFIG_ACPI_DOCK)	+= dock.o
diff --git a/drivers/acpi/apic_id.c b/drivers/acpi/apic_id.c
new file mode 100644
index 000000000000..ada5fd48bad4
--- /dev/null
+++ b/drivers/acpi/apic_id.c
@@ -0,0 +1,202 @@
+/*
+ * Copyright (C) 2005 Intel Corporation
+ * Copyright (C) 2009 Hewlett-Packard Development Company, L.P.
+ *
+ *	Alex Chiang <achiang@hp.com>
+ *	- Unified x86/ia64 implementations
+ */
+#include <linux/export.h>
+#include <linux/acpi.h>
+#include "internal.h"
+
+static int map_lapic_id(struct acpi_subtable_header *entry,
+		 u32 acpi_id, int *apic_id)
+{
+	struct acpi_madt_local_apic *lapic =
+		(struct acpi_madt_local_apic *)entry;
+
+	if (!(lapic->lapic_flags & ACPI_MADT_ENABLED))
+		return -ENODEV;
+
+	if (lapic->processor_id != acpi_id)
+		return -EINVAL;
+
+	*apic_id = lapic->id;
+	return 0;
+}
+
+static int map_x2apic_id(struct acpi_subtable_header *entry,
+			 int device_declaration, u32 acpi_id, int *apic_id)
+{
+	struct acpi_madt_local_x2apic *apic =
+		(struct acpi_madt_local_x2apic *)entry;
+
+	if (!(apic->lapic_flags & ACPI_MADT_ENABLED))
+		return -ENODEV;
+
+	if (device_declaration && (apic->uid == acpi_id)) {
+		*apic_id = apic->local_apic_id;
+		return 0;
+	}
+
+	return -EINVAL;
+}
+
+static int map_lsapic_id(struct acpi_subtable_header *entry,
+		int device_declaration, u32 acpi_id, int *apic_id)
+{
+	struct acpi_madt_local_sapic *lsapic =
+		(struct acpi_madt_local_sapic *)entry;
+
+	if (!(lsapic->lapic_flags & ACPI_MADT_ENABLED))
+		return -ENODEV;
+
+	if (device_declaration) {
+		if ((entry->length < 16) || (lsapic->uid != acpi_id))
+			return -EINVAL;
+	} else if (lsapic->processor_id != acpi_id)
+		return -EINVAL;
+
+	*apic_id = (lsapic->id << 8) | lsapic->eid;
+	return 0;
+}
+
+static int map_madt_entry(int type, u32 acpi_id)
+{
+	unsigned long madt_end, entry;
+	static struct acpi_table_madt *madt;
+	static int read_madt;
+	int apic_id = -1;
+
+	if (!read_madt) {
+		if (ACPI_FAILURE(acpi_get_table(ACPI_SIG_MADT, 0,
+					(struct acpi_table_header **)&madt)))
+			madt = NULL;
+		read_madt++;
+	}
+
+	if (!madt)
+		return apic_id;
+
+	entry = (unsigned long)madt;
+	madt_end = entry + madt->header.length;
+
+	/* Parse all entries looking for a match. */
+
+	entry += sizeof(struct acpi_table_madt);
+	while (entry + sizeof(struct acpi_subtable_header) < madt_end) {
+		struct acpi_subtable_header *header =
+			(struct acpi_subtable_header *)entry;
+		if (header->type == ACPI_MADT_TYPE_LOCAL_APIC) {
+			if (!map_lapic_id(header, acpi_id, &apic_id))
+				break;
+		} else if (header->type == ACPI_MADT_TYPE_LOCAL_X2APIC) {
+			if (!map_x2apic_id(header, type, acpi_id, &apic_id))
+				break;
+		} else if (header->type == ACPI_MADT_TYPE_LOCAL_SAPIC) {
+			if (!map_lsapic_id(header, type, acpi_id, &apic_id))
+				break;
+		}
+		entry += header->length;
+	}
+	return apic_id;
+}
+
+static int map_mat_entry(acpi_handle handle, int type, u32 acpi_id)
+{
+	struct acpi_buffer buffer = { ACPI_ALLOCATE_BUFFER, NULL };
+	union acpi_object *obj;
+	struct acpi_subtable_header *header;
+	int apic_id = -1;
+
+	if (ACPI_FAILURE(acpi_evaluate_object(handle, "_MAT", NULL, &buffer)))
+		goto exit;
+
+	if (!buffer.length || !buffer.pointer)
+		goto exit;
+
+	obj = buffer.pointer;
+	if (obj->type != ACPI_TYPE_BUFFER ||
+	    obj->buffer.length < sizeof(struct acpi_subtable_header)) {
+		goto exit;
+	}
+
+	header = (struct acpi_subtable_header *)obj->buffer.pointer;
+	if (header->type == ACPI_MADT_TYPE_LOCAL_APIC)
+		map_lapic_id(header, acpi_id, &apic_id);
+	else if (header->type == ACPI_MADT_TYPE_LOCAL_SAPIC)
+		map_lsapic_id(header, type, acpi_id, &apic_id);
+	else if (header->type == ACPI_MADT_TYPE_LOCAL_X2APIC)
+		map_x2apic_id(header, type, acpi_id, &apic_id);
+
+exit:
+	kfree(buffer.pointer);
+	return apic_id;
+}
+
+int acpi_get_apicid(acpi_handle handle, int type, u32 acpi_id)
+{
+	int apic_id;
+
+	apic_id = map_mat_entry(handle, type, acpi_id);
+	if (apic_id == -1)
+		apic_id = map_madt_entry(type, acpi_id);
+
+	return apic_id;
+}
+
+int acpi_map_cpuid(int apic_id, u32 acpi_id)
+{
+#ifdef CONFIG_SMP
+	int i;
+#endif
+
+	if (apic_id == -1) {
+		/*
+		 * On UP processor, there is no _MAT or MADT table.
+		 * So above apic_id is always set to -1.
+		 *
+		 * BIOS may define multiple CPU handles even for UP processor.
+		 * For example,
+		 *
+		 * Scope (_PR)
+		 * {
+		 *     Processor (CPU0, 0x00, 0x00000410, 0x06) {}
+		 *     Processor (CPU1, 0x01, 0x00000410, 0x06) {}
+		 *     Processor (CPU2, 0x02, 0x00000410, 0x06) {}
+		 *     Processor (CPU3, 0x03, 0x00000410, 0x06) {}
+		 * }
+		 *
+		 * Ignores apic_id and always returns 0 for the processor
+		 * handle with acpi id 0 if nr_cpu_ids is 1.
+		 * This should be the case if SMP tables are not found.
+		 * Return -1 for other CPU's handle.
+		 */
+		if (nr_cpu_ids <= 1 && acpi_id == 0)
+			return acpi_id;
+		else
+			return apic_id;
+	}
+
+#ifdef CONFIG_SMP
+	for_each_possible_cpu(i) {
+		if (cpu_physical_id(i) == apic_id)
+			return i;
+	}
+#else
+	/* In UP kernel, only processor 0 is valid */
+	if (apic_id == 0)
+		return apic_id;
+#endif
+	return -1;
+}
+
+int acpi_get_cpuid(acpi_handle handle, int type, u32 acpi_id)
+{
+	int apic_id;
+
+	apic_id = acpi_get_apicid(handle, type, acpi_id);
+
+	return acpi_map_cpuid(apic_id, acpi_id);
+}
+EXPORT_SYMBOL_GPL(acpi_get_cpuid);
diff --git a/drivers/acpi/processor_core.c b/drivers/acpi/processor_core.c
deleted file mode 100644
index b048f3752c2b..000000000000
--- a/drivers/acpi/processor_core.c
+++ /dev/null
@@ -1,205 +0,0 @@
-/*
- * Copyright (C) 2005 Intel Corporation
- * Copyright (C) 2009 Hewlett-Packard Development Company, L.P.
- *
- *	Alex Chiang <achiang@hp.com>
- *	- Unified x86/ia64 implementations
- */
-#include <linux/export.h>
-#include <linux/acpi.h>
-#include <acpi/processor.h>
-
-#define _COMPONENT		ACPI_PROCESSOR_COMPONENT
-ACPI_MODULE_NAME("processor_core");
-
-static int map_lapic_id(struct acpi_subtable_header *entry,
-		 u32 acpi_id, int *apic_id)
-{
-	struct acpi_madt_local_apic *lapic =
-		(struct acpi_madt_local_apic *)entry;
-
-	if (!(lapic->lapic_flags & ACPI_MADT_ENABLED))
-		return -ENODEV;
-
-	if (lapic->processor_id != acpi_id)
-		return -EINVAL;
-
-	*apic_id = lapic->id;
-	return 0;
-}
-
-static int map_x2apic_id(struct acpi_subtable_header *entry,
-			 int device_declaration, u32 acpi_id, int *apic_id)
-{
-	struct acpi_madt_local_x2apic *apic =
-		(struct acpi_madt_local_x2apic *)entry;
-
-	if (!(apic->lapic_flags & ACPI_MADT_ENABLED))
-		return -ENODEV;
-
-	if (device_declaration && (apic->uid == acpi_id)) {
-		*apic_id = apic->local_apic_id;
-		return 0;
-	}
-
-	return -EINVAL;
-}
-
-static int map_lsapic_id(struct acpi_subtable_header *entry,
-		int device_declaration, u32 acpi_id, int *apic_id)
-{
-	struct acpi_madt_local_sapic *lsapic =
-		(struct acpi_madt_local_sapic *)entry;
-
-	if (!(lsapic->lapic_flags & ACPI_MADT_ENABLED))
-		return -ENODEV;
-
-	if (device_declaration) {
-		if ((entry->length < 16) || (lsapic->uid != acpi_id))
-			return -EINVAL;
-	} else if (lsapic->processor_id != acpi_id)
-		return -EINVAL;
-
-	*apic_id = (lsapic->id << 8) | lsapic->eid;
-	return 0;
-}
-
-static int map_madt_entry(int type, u32 acpi_id)
-{
-	unsigned long madt_end, entry;
-	static struct acpi_table_madt *madt;
-	static int read_madt;
-	int apic_id = -1;
-
-	if (!read_madt) {
-		if (ACPI_FAILURE(acpi_get_table(ACPI_SIG_MADT, 0,
-					(struct acpi_table_header **)&madt)))
-			madt = NULL;
-		read_madt++;
-	}
-
-	if (!madt)
-		return apic_id;
-
-	entry = (unsigned long)madt;
-	madt_end = entry + madt->header.length;
-
-	/* Parse all entries looking for a match. */
-
-	entry += sizeof(struct acpi_table_madt);
-	while (entry + sizeof(struct acpi_subtable_header) < madt_end) {
-		struct acpi_subtable_header *header =
-			(struct acpi_subtable_header *)entry;
-		if (header->type == ACPI_MADT_TYPE_LOCAL_APIC) {
-			if (!map_lapic_id(header, acpi_id, &apic_id))
-				break;
-		} else if (header->type == ACPI_MADT_TYPE_LOCAL_X2APIC) {
-			if (!map_x2apic_id(header, type, acpi_id, &apic_id))
-				break;
-		} else if (header->type == ACPI_MADT_TYPE_LOCAL_SAPIC) {
-			if (!map_lsapic_id(header, type, acpi_id, &apic_id))
-				break;
-		}
-		entry += header->length;
-	}
-	return apic_id;
-}
-
-static int map_mat_entry(acpi_handle handle, int type, u32 acpi_id)
-{
-	struct acpi_buffer buffer = { ACPI_ALLOCATE_BUFFER, NULL };
-	union acpi_object *obj;
-	struct acpi_subtable_header *header;
-	int apic_id = -1;
-
-	if (ACPI_FAILURE(acpi_evaluate_object(handle, "_MAT", NULL, &buffer)))
-		goto exit;
-
-	if (!buffer.length || !buffer.pointer)
-		goto exit;
-
-	obj = buffer.pointer;
-	if (obj->type != ACPI_TYPE_BUFFER ||
-	    obj->buffer.length < sizeof(struct acpi_subtable_header)) {
-		goto exit;
-	}
-
-	header = (struct acpi_subtable_header *)obj->buffer.pointer;
-	if (header->type == ACPI_MADT_TYPE_LOCAL_APIC)
-		map_lapic_id(header, acpi_id, &apic_id);
-	else if (header->type == ACPI_MADT_TYPE_LOCAL_SAPIC)
-		map_lsapic_id(header, type, acpi_id, &apic_id);
-	else if (header->type == ACPI_MADT_TYPE_LOCAL_X2APIC)
-		map_x2apic_id(header, type, acpi_id, &apic_id);
-
-exit:
-	kfree(buffer.pointer);
-	return apic_id;
-}
-
-int acpi_get_apicid(acpi_handle handle, int type, u32 acpi_id)
-{
-	int apic_id;
-
-	apic_id = map_mat_entry(handle, type, acpi_id);
-	if (apic_id == -1)
-		apic_id = map_madt_entry(type, acpi_id);
-
-	return apic_id;
-}
-
-int acpi_map_cpuid(int apic_id, u32 acpi_id)
-{
-#ifdef CONFIG_SMP
-	int i;
-#endif
-
-	if (apic_id == -1) {
-		/*
-		 * On UP processor, there is no _MAT or MADT table.
-		 * So above apic_id is always set to -1.
-		 *
-		 * BIOS may define multiple CPU handles even for UP processor.
-		 * For example,
-		 *
-		 * Scope (_PR)
-		 * {
-		 *     Processor (CPU0, 0x00, 0x00000410, 0x06) {}
-		 *     Processor (CPU1, 0x01, 0x00000410, 0x06) {}
-		 *     Processor (CPU2, 0x02, 0x00000410, 0x06) {}
-		 *     Processor (CPU3, 0x03, 0x00000410, 0x06) {}
-		 * }
-		 *
-		 * Ignores apic_id and always returns 0 for the processor
-		 * handle with acpi id 0 if nr_cpu_ids is 1.
-		 * This should be the case if SMP tables are not found.
-		 * Return -1 for other CPU's handle.
-		 */
-		if (nr_cpu_ids <= 1 && acpi_id == 0)
-			return acpi_id;
-		else
-			return apic_id;
-	}
-
-#ifdef CONFIG_SMP
-	for_each_possible_cpu(i) {
-		if (cpu_physical_id(i) == apic_id)
-			return i;
-	}
-#else
-	/* In UP kernel, only processor 0 is valid */
-	if (apic_id == 0)
-		return apic_id;
-#endif
-	return -1;
-}
-
-int acpi_get_cpuid(acpi_handle handle, int type, u32 acpi_id)
-{
-	int apic_id;
-
-	apic_id = acpi_get_apicid(handle, type, acpi_id);
-
-	return acpi_map_cpuid(apic_id, acpi_id);
-}
-EXPORT_SYMBOL_GPL(acpi_get_cpuid);
diff --git a/include/acpi/processor.h b/include/acpi/processor.h
index 9b9b6f29bbf3..99fc22c9e61f 100644
--- a/include/acpi/processor.h
+++ b/include/acpi/processor.h
@@ -314,9 +314,6 @@ static inline int acpi_processor_get_bios_limit(int cpu, unsigned int *limit)
 
 /* in processor_core.c */
 void acpi_processor_set_pdc(acpi_handle handle);
-int acpi_get_apicid(acpi_handle, int type, u32 acpi_id);
-int acpi_map_cpuid(int apic_id, u32 acpi_id);
-int acpi_get_cpuid(acpi_handle, int type, u32 acpi_id);
 
 /* in processor_throttling.c */
 int acpi_processor_tstate_has_changed(struct acpi_processor *pr);
diff --git a/include/linux/acpi.h b/include/linux/acpi.h
index 807cbc46d73e..05ed6886f1f8 100644
--- a/include/linux/acpi.h
+++ b/include/linux/acpi.h
@@ -140,6 +140,9 @@ void acpi_numa_x2apic_affinity_init(struct acpi_srat_x2apic_cpu_affinity *pa);
 int acpi_numa_memory_affinity_init (struct acpi_srat_mem_affinity *ma);
 void acpi_numa_arch_fixup(void);
 
+int acpi_get_apicid(acpi_handle, int type, u32 acpi_id);
+int acpi_map_cpuid(int apic_id, u32 acpi_id);
+int acpi_get_cpuid(acpi_handle, int type, u32 acpi_id);
 #ifdef CONFIG_ACPI_HOTPLUG_CPU
 /* Arch dependent functions for cpu hotplug support */
 int acpi_map_lsapic(acpi_handle handle, int physid, int *pcpu);
-- 
1.7.10.4

Re: [Patch v4 04/16] ACPI: Rename processor_core.c as apic_id.c

[Rafael J. Wysocki]

On Thursday, August 28, 2014 10:22:29 AM Jiang Liu wrote:
> Now all code in processor_core.c is APIC ID related, so rename it as
> apic_id.c. Later IOAPIC ID related code will be added into apic_id.c.

Actually, I'm not sure about this one.

Renames like this make it difficult to backport things in general
and kind of break "git blame", so do we have to do that?

What's wrong with leaving the name as is and adding a comment
about the contents being related to IOAPIC ID?

> Signed-off-by: Jiang Liu <jiang.liu@linux.intel.com>
> ---
>  drivers/acpi/Makefile         |    2 +-
>  drivers/acpi/apic_id.c        |  202 ++++++++++++++++++++++++++++++++++++++++
>  drivers/acpi/processor_core.c |  205 -----------------------------------------

And BTW this doesn't seem to be an exact rename, does it?

>  include/acpi/processor.h      |    3 -
>  include/linux/acpi.h          |    3 +
>  5 files changed, 206 insertions(+), 209 deletions(-)
>  create mode 100644 drivers/acpi/apic_id.c
>  delete mode 100644 drivers/acpi/processor_core.c
> 
> diff --git a/drivers/acpi/Makefile b/drivers/acpi/Makefile
> index 505d4d79fe3e..03ddd03f2bcd 100644
> --- a/drivers/acpi/Makefile
> +++ b/drivers/acpi/Makefile
> @@ -35,7 +35,7 @@ acpi-y				+= bus.o glue.o
>  acpi-y				+= scan.o
>  acpi-y				+= resource.o
>  acpi-y				+= acpi_processor.o
> -acpi-y				+= processor_core.o
> +acpi-y				+= apic_id.o
>  acpi-$(CONFIG_ARCH_MIGHT_HAVE_ACPI_PDC) += processor_pdc.o
>  acpi-y				+= ec.o
>  acpi-$(CONFIG_ACPI_DOCK)	+= dock.o
> diff --git a/drivers/acpi/apic_id.c b/drivers/acpi/apic_id.c
> new file mode 100644
> index 000000000000..ada5fd48bad4
> --- /dev/null
> +++ b/drivers/acpi/apic_id.c
> @@ -0,0 +1,202 @@
> +/*
> + * Copyright (C) 2005 Intel Corporation
> + * Copyright (C) 2009 Hewlett-Packard Development Company, L.P.
> + *
> + *	Alex Chiang <achiang@hp.com>
> + *	- Unified x86/ia64 implementations
> + */
> +#include <linux/export.h>
> +#include <linux/acpi.h>
> +#include "internal.h"
> +
> +static int map_lapic_id(struct acpi_subtable_header *entry,
> +		 u32 acpi_id, int *apic_id)
> +{
> +	struct acpi_madt_local_apic *lapic =
> +		(struct acpi_madt_local_apic *)entry;
> +
> +	if (!(lapic->lapic_flags & ACPI_MADT_ENABLED))
> +		return -ENODEV;
> +
> +	if (lapic->processor_id != acpi_id)
> +		return -EINVAL;
> +
> +	*apic_id = lapic->id;
> +	return 0;
> +}
> +
> +static int map_x2apic_id(struct acpi_subtable_header *entry,
> +			 int device_declaration, u32 acpi_id, int *apic_id)
> +{
> +	struct acpi_madt_local_x2apic *apic =
> +		(struct acpi_madt_local_x2apic *)entry;
> +
> +	if (!(apic->lapic_flags & ACPI_MADT_ENABLED))
> +		return -ENODEV;
> +
> +	if (device_declaration && (apic->uid == acpi_id)) {
> +		*apic_id = apic->local_apic_id;
> +		return 0;
> +	}
> +
> +	return -EINVAL;
> +}
> +
> +static int map_lsapic_id(struct acpi_subtable_header *entry,
> +		int device_declaration, u32 acpi_id, int *apic_id)
> +{
> +	struct acpi_madt_local_sapic *lsapic =
> +		(struct acpi_madt_local_sapic *)entry;
> +
> +	if (!(lsapic->lapic_flags & ACPI_MADT_ENABLED))
> +		return -ENODEV;
> +
> +	if (device_declaration) {
> +		if ((entry->length < 16) || (lsapic->uid != acpi_id))
> +			return -EINVAL;
> +	} else if (lsapic->processor_id != acpi_id)
> +		return -EINVAL;
> +
> +	*apic_id = (lsapic->id << 8) | lsapic->eid;
> +	return 0;
> +}
> +
> +static int map_madt_entry(int type, u32 acpi_id)
> +{
> +	unsigned long madt_end, entry;
> +	static struct acpi_table_madt *madt;
> +	static int read_madt;
> +	int apic_id = -1;
> +
> +	if (!read_madt) {
> +		if (ACPI_FAILURE(acpi_get_table(ACPI_SIG_MADT, 0,
> +					(struct acpi_table_header **)&madt)))
> +			madt = NULL;
> +		read_madt++;
> +	}
> +
> +	if (!madt)
> +		return apic_id;
> +
> +	entry = (unsigned long)madt;
> +	madt_end = entry + madt->header.length;
> +
> +	/* Parse all entries looking for a match. */
> +
> +	entry += sizeof(struct acpi_table_madt);
> +	while (entry + sizeof(struct acpi_subtable_header) < madt_end) {
> +		struct acpi_subtable_header *header =
> +			(struct acpi_subtable_header *)entry;
> +		if (header->type == ACPI_MADT_TYPE_LOCAL_APIC) {
> +			if (!map_lapic_id(header, acpi_id, &apic_id))
> +				break;
> +		} else if (header->type == ACPI_MADT_TYPE_LOCAL_X2APIC) {
> +			if (!map_x2apic_id(header, type, acpi_id, &apic_id))
> +				break;
> +		} else if (header->type == ACPI_MADT_TYPE_LOCAL_SAPIC) {
> +			if (!map_lsapic_id(header, type, acpi_id, &apic_id))
> +				break;
> +		}
> +		entry += header->length;
> +	}
> +	return apic_id;
> +}
> +
> +static int map_mat_entry(acpi_handle handle, int type, u32 acpi_id)
> +{
> +	struct acpi_buffer buffer = { ACPI_ALLOCATE_BUFFER, NULL };
> +	union acpi_object *obj;
> +	struct acpi_subtable_header *header;
> +	int apic_id = -1;
> +
> +	if (ACPI_FAILURE(acpi_evaluate_object(handle, "_MAT", NULL, &buffer)))
> +		goto exit;
> +
> +	if (!buffer.length || !buffer.pointer)
> +		goto exit;
> +
> +	obj = buffer.pointer;
> +	if (obj->type != ACPI_TYPE_BUFFER ||
> +	    obj->buffer.length < sizeof(struct acpi_subtable_header)) {
> +		goto exit;
> +	}
> +
> +	header = (struct acpi_subtable_header *)obj->buffer.pointer;
> +	if (header->type == ACPI_MADT_TYPE_LOCAL_APIC)
> +		map_lapic_id(header, acpi_id, &apic_id);
> +	else if (header->type == ACPI_MADT_TYPE_LOCAL_SAPIC)
> +		map_lsapic_id(header, type, acpi_id, &apic_id);
> +	else if (header->type == ACPI_MADT_TYPE_LOCAL_X2APIC)
> +		map_x2apic_id(header, type, acpi_id, &apic_id);
> +
> +exit:
> +	kfree(buffer.pointer);
> +	return apic_id;
> +}
> +
> +int acpi_get_apicid(acpi_handle handle, int type, u32 acpi_id)
> +{
> +	int apic_id;
> +
> +	apic_id = map_mat_entry(handle, type, acpi_id);
> +	if (apic_id == -1)
> +		apic_id = map_madt_entry(type, acpi_id);
> +
> +	return apic_id;
> +}
> +
> +int acpi_map_cpuid(int apic_id, u32 acpi_id)
> +{
> +#ifdef CONFIG_SMP
> +	int i;
> +#endif
> +
> +	if (apic_id == -1) {
> +		/*
> +		 * On UP processor, there is no _MAT or MADT table.
> +		 * So above apic_id is always set to -1.
> +		 *
> +		 * BIOS may define multiple CPU handles even for UP processor.
> +		 * For example,
> +		 *
> +		 * Scope (_PR)
> +		 * {
> +		 *     Processor (CPU0, 0x00, 0x00000410, 0x06) {}
> +		 *     Processor (CPU1, 0x01, 0x00000410, 0x06) {}
> +		 *     Processor (CPU2, 0x02, 0x00000410, 0x06) {}
> +		 *     Processor (CPU3, 0x03, 0x00000410, 0x06) {}
> +		 * }
> +		 *
> +		 * Ignores apic_id and always returns 0 for the processor
> +		 * handle with acpi id 0 if nr_cpu_ids is 1.
> +		 * This should be the case if SMP tables are not found.
> +		 * Return -1 for other CPU's handle.
> +		 */
> +		if (nr_cpu_ids <= 1 && acpi_id == 0)
> +			return acpi_id;
> +		else
> +			return apic_id;
> +	}
> +
> +#ifdef CONFIG_SMP
> +	for_each_possible_cpu(i) {
> +		if (cpu_physical_id(i) == apic_id)
> +			return i;
> +	}
> +#else
> +	/* In UP kernel, only processor 0 is valid */
> +	if (apic_id == 0)
> +		return apic_id;
> +#endif
> +	return -1;
> +}
> +
> +int acpi_get_cpuid(acpi_handle handle, int type, u32 acpi_id)
> +{
> +	int apic_id;
> +
> +	apic_id = acpi_get_apicid(handle, type, acpi_id);
> +
> +	return acpi_map_cpuid(apic_id, acpi_id);
> +}
> +EXPORT_SYMBOL_GPL(acpi_get_cpuid);
> diff --git a/drivers/acpi/processor_core.c b/drivers/acpi/processor_core.c
> deleted file mode 100644
> index b048f3752c2b..000000000000
> --- a/drivers/acpi/processor_core.c
> +++ /dev/null
> @@ -1,205 +0,0 @@
> -/*
> - * Copyright (C) 2005 Intel Corporation
> - * Copyright (C) 2009 Hewlett-Packard Development Company, L.P.
> - *
> - *	Alex Chiang <achiang@hp.com>
> - *	- Unified x86/ia64 implementations
> - */
> -#include <linux/export.h>
> -#include <linux/acpi.h>
> -#include <acpi/processor.h>
> -
> -#define _COMPONENT		ACPI_PROCESSOR_COMPONENT
> -ACPI_MODULE_NAME("processor_core");
> -
> -static int map_lapic_id(struct acpi_subtable_header *entry,
> -		 u32 acpi_id, int *apic_id)
> -{
> -	struct acpi_madt_local_apic *lapic =
> -		(struct acpi_madt_local_apic *)entry;
> -
> -	if (!(lapic->lapic_flags & ACPI_MADT_ENABLED))
> -		return -ENODEV;
> -
> -	if (lapic->processor_id != acpi_id)
> -		return -EINVAL;
> -
> -	*apic_id = lapic->id;
> -	return 0;
> -}
> -
> -static int map_x2apic_id(struct acpi_subtable_header *entry,
> -			 int device_declaration, u32 acpi_id, int *apic_id)
> -{
> -	struct acpi_madt_local_x2apic *apic =
> -		(struct acpi_madt_local_x2apic *)entry;
> -
> -	if (!(apic->lapic_flags & ACPI_MADT_ENABLED))
> -		return -ENODEV;
> -
> -	if (device_declaration && (apic->uid == acpi_id)) {
> -		*apic_id = apic->local_apic_id;
> -		return 0;
> -	}
> -
> -	return -EINVAL;
> -}
> -
> -static int map_lsapic_id(struct acpi_subtable_header *entry,
> -		int device_declaration, u32 acpi_id, int *apic_id)
> -{
> -	struct acpi_madt_local_sapic *lsapic =
> -		(struct acpi_madt_local_sapic *)entry;
> -
> -	if (!(lsapic->lapic_flags & ACPI_MADT_ENABLED))
> -		return -ENODEV;
> -
> -	if (device_declaration) {
> -		if ((entry->length < 16) || (lsapic->uid != acpi_id))
> -			return -EINVAL;
> -	} else if (lsapic->processor_id != acpi_id)
> -		return -EINVAL;
> -
> -	*apic_id = (lsapic->id << 8) | lsapic->eid;
> -	return 0;
> -}
> -
> -static int map_madt_entry(int type, u32 acpi_id)
> -{
> -	unsigned long madt_end, entry;
> -	static struct acpi_table_madt *madt;
> -	static int read_madt;
> -	int apic_id = -1;
> -
> -	if (!read_madt) {
> -		if (ACPI_FAILURE(acpi_get_table(ACPI_SIG_MADT, 0,
> -					(struct acpi_table_header **)&madt)))
> -			madt = NULL;
> -		read_madt++;
> -	}
> -
> -	if (!madt)
> -		return apic_id;
> -
> -	entry = (unsigned long)madt;
> -	madt_end = entry + madt->header.length;
> -
> -	/* Parse all entries looking for a match. */
> -
> -	entry += sizeof(struct acpi_table_madt);
> -	while (entry + sizeof(struct acpi_subtable_header) < madt_end) {
> -		struct acpi_subtable_header *header =
> -			(struct acpi_subtable_header *)entry;
> -		if (header->type == ACPI_MADT_TYPE_LOCAL_APIC) {
> -			if (!map_lapic_id(header, acpi_id, &apic_id))
> -				break;
> -		} else if (header->type == ACPI_MADT_TYPE_LOCAL_X2APIC) {
> -			if (!map_x2apic_id(header, type, acpi_id, &apic_id))
> -				break;
> -		} else if (header->type == ACPI_MADT_TYPE_LOCAL_SAPIC) {
> -			if (!map_lsapic_id(header, type, acpi_id, &apic_id))
> -				break;
> -		}
> -		entry += header->length;
> -	}
> -	return apic_id;
> -}
> -
> -static int map_mat_entry(acpi_handle handle, int type, u32 acpi_id)
> -{
> -	struct acpi_buffer buffer = { ACPI_ALLOCATE_BUFFER, NULL };
> -	union acpi_object *obj;
> -	struct acpi_subtable_header *header;
> -	int apic_id = -1;
> -
> -	if (ACPI_FAILURE(acpi_evaluate_object(handle, "_MAT", NULL, &buffer)))
> -		goto exit;
> -
> -	if (!buffer.length || !buffer.pointer)
> -		goto exit;
> -
> -	obj = buffer.pointer;
> -	if (obj->type != ACPI_TYPE_BUFFER ||
> -	    obj->buffer.length < sizeof(struct acpi_subtable_header)) {
> -		goto exit;
> -	}
> -
> -	header = (struct acpi_subtable_header *)obj->buffer.pointer;
> -	if (header->type == ACPI_MADT_TYPE_LOCAL_APIC)
> -		map_lapic_id(header, acpi_id, &apic_id);
> -	else if (header->type == ACPI_MADT_TYPE_LOCAL_SAPIC)
> -		map_lsapic_id(header, type, acpi_id, &apic_id);
> -	else if (header->type == ACPI_MADT_TYPE_LOCAL_X2APIC)
> -		map_x2apic_id(header, type, acpi_id, &apic_id);
> -
> -exit:
> -	kfree(buffer.pointer);
> -	return apic_id;
> -}
> -
> -int acpi_get_apicid(acpi_handle handle, int type, u32 acpi_id)
> -{
> -	int apic_id;
> -
> -	apic_id = map_mat_entry(handle, type, acpi_id);
> -	if (apic_id == -1)
> -		apic_id = map_madt_entry(type, acpi_id);
> -
> -	return apic_id;
> -}
> -
> -int acpi_map_cpuid(int apic_id, u32 acpi_id)
> -{
> -#ifdef CONFIG_SMP
> -	int i;
> -#endif
> -
> -	if (apic_id == -1) {
> -		/*
> -		 * On UP processor, there is no _MAT or MADT table.
> -		 * So above apic_id is always set to -1.
> -		 *
> -		 * BIOS may define multiple CPU handles even for UP processor.
> -		 * For example,
> -		 *
> -		 * Scope (_PR)
> -		 * {
> -		 *     Processor (CPU0, 0x00, 0x00000410, 0x06) {}
> -		 *     Processor (CPU1, 0x01, 0x00000410, 0x06) {}
> -		 *     Processor (CPU2, 0x02, 0x00000410, 0x06) {}
> -		 *     Processor (CPU3, 0x03, 0x00000410, 0x06) {}
> -		 * }
> -		 *
> -		 * Ignores apic_id and always returns 0 for the processor
> -		 * handle with acpi id 0 if nr_cpu_ids is 1.
> -		 * This should be the case if SMP tables are not found.
> -		 * Return -1 for other CPU's handle.
> -		 */
> -		if (nr_cpu_ids <= 1 && acpi_id == 0)
> -			return acpi_id;
> -		else
> -			return apic_id;
> -	}
> -
> -#ifdef CONFIG_SMP
> -	for_each_possible_cpu(i) {
> -		if (cpu_physical_id(i) == apic_id)
> -			return i;
> -	}
> -#else
> -	/* In UP kernel, only processor 0 is valid */
> -	if (apic_id == 0)
> -		return apic_id;
> -#endif
> -	return -1;
> -}
> -
> -int acpi_get_cpuid(acpi_handle handle, int type, u32 acpi_id)
> -{
> -	int apic_id;
> -
> -	apic_id = acpi_get_apicid(handle, type, acpi_id);
> -
> -	return acpi_map_cpuid(apic_id, acpi_id);
> -}
> -EXPORT_SYMBOL_GPL(acpi_get_cpuid);
> diff --git a/include/acpi/processor.h b/include/acpi/processor.h
> index 9b9b6f29bbf3..99fc22c9e61f 100644
> --- a/include/acpi/processor.h
> +++ b/include/acpi/processor.h
> @@ -314,9 +314,6 @@ static inline int acpi_processor_get_bios_limit(int cpu, unsigned int *limit)
>  
>  /* in processor_core.c */
>  void acpi_processor_set_pdc(acpi_handle handle);
> -int acpi_get_apicid(acpi_handle, int type, u32 acpi_id);
> -int acpi_map_cpuid(int apic_id, u32 acpi_id);
> -int acpi_get_cpuid(acpi_handle, int type, u32 acpi_id);
>  
>  /* in processor_throttling.c */
>  int acpi_processor_tstate_has_changed(struct acpi_processor *pr);
> diff --git a/include/linux/acpi.h b/include/linux/acpi.h
> index 807cbc46d73e..05ed6886f1f8 100644
> --- a/include/linux/acpi.h
> +++ b/include/linux/acpi.h
> @@ -140,6 +140,9 @@ void acpi_numa_x2apic_affinity_init(struct acpi_srat_x2apic_cpu_affinity *pa);
>  int acpi_numa_memory_affinity_init (struct acpi_srat_mem_affinity *ma);
>  void acpi_numa_arch_fixup(void);
>  
> +int acpi_get_apicid(acpi_handle, int type, u32 acpi_id);
> +int acpi_map_cpuid(int apic_id, u32 acpi_id);
> +int acpi_get_cpuid(acpi_handle, int type, u32 acpi_id);
>  #ifdef CONFIG_ACPI_HOTPLUG_CPU
>  /* Arch dependent functions for cpu hotplug support */
>  int acpi_map_lsapic(acpi_handle handle, int physid, int *pcpu);
> 

-- 
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.

Re: [Patch v4 04/16] ACPI: Rename processor_core.c as apic_id.c

[Hanjun Guo]

Hi Gerry, Rafael,

On 2014年09月08日 06:37, Rafael J. Wysocki wrote:
> On Thursday, August 28, 2014 10:22:29 AM Jiang Liu wrote:
>> Now all code in processor_core.c is APIC ID related, so rename it as
>> apic_id.c. Later IOAPIC ID related code will be added into apic_id.c.
> Actually, I'm not sure about this one.
>
> Renames like this make it difficult to backport things in general
> and kind of break "git blame", so do we have to do that?
>
> What's wrong with leaving the name as is and adding a comment
> about the contents being related to IOAPIC ID?

It will be thankful for not renaming the file into apic_id.c, because apic id is x86 specific,
and platform like ARM will also add some code to get cpu hardware id via GICC structure
in MADT table in that file, apic id is not a generic name for both x86 and ARM, I prefer to
keep it as it is :)

Thanks
Hanjun

Re: [Patch v4 04/16] ACPI: Rename processor_core.c as apic_id.c

[Rafael J. Wysocki]

On Monday, September 08, 2014 08:51:31 PM Hanjun Guo wrote:
> Hi Gerry, Rafael,
> 
> On 2014年09月08日 06:37, Rafael J. Wysocki wrote:
> > On Thursday, August 28, 2014 10:22:29 AM Jiang Liu wrote:
> >> Now all code in processor_core.c is APIC ID related, so rename it as
> >> apic_id.c. Later IOAPIC ID related code will be added into apic_id.c.
> > Actually, I'm not sure about this one.
> >
> > Renames like this make it difficult to backport things in general
> > and kind of break "git blame", so do we have to do that?
> >
> > What's wrong with leaving the name as is and adding a comment
> > about the contents being related to IOAPIC ID?
> 
> It will be thankful for not renaming the file into apic_id.c, because apic id is x86 specific,
> and platform like ARM will also add some code to get cpu hardware id via GICC structure
> in MADT table in that file, apic id is not a generic name for both x86 and ARM, I prefer to
> keep it as it is :)

Well, that's a good argument too.

Rafael

Re: [Patch v4 04/16] ACPI: Rename processor_core.c as apic_id.c

[Jiang Liu]

Hi Rafael and Hanjun,
	Thanks for your review. I will keep the original file name
in next version.
Regards!
Gerry

On 2014/9/9 5:00, Rafael J. Wysocki wrote:
> On Monday, September 08, 2014 08:51:31 PM Hanjun Guo wrote:
>> Hi Gerry, Rafael,
>>
>> On 2014年09月08日 06:37, Rafael J. Wysocki wrote:
>>> On Thursday, August 28, 2014 10:22:29 AM Jiang Liu wrote:
>>>> Now all code in processor_core.c is APIC ID related, so rename it as
>>>> apic_id.c. Later IOAPIC ID related code will be added into apic_id.c.
>>> Actually, I'm not sure about this one.
>>>
>>> Renames like this make it difficult to backport things in general
>>> and kind of break "git blame", so do we have to do that?
>>>
>>> What's wrong with leaving the name as is and adding a comment
>>> about the contents being related to IOAPIC ID?
>>
>> It will be thankful for not renaming the file into apic_id.c, because apic id is x86 specific,
>> and platform like ARM will also add some code to get cpu hardware id via GICC structure
>> in MADT table in that file, apic id is not a generic name for both x86 and ARM, I prefer to
>> keep it as it is :)
> 
> Well, that's a good argument too.
> 
> Rafael
> 

[Patch v4 05/16] ACPI: Add interfaces to parse IOAPIC ID for IOAPIC hotplug

[Jiang Liu]

From: Yinghai Lu <yinghai@kernel.org>

We need to parse APIC ID for IOAPIC registration for IOAPIC hotplug.
ACPI _MAT method and MADT table are used to figure out IOAPIC ID, just
like parsing CPU APIC ID for CPU hotplug.

Signed-off-by: Yinghai Lu <yinghai@kernel.org>
Signed-off-by: Jiang Liu <jiang.liu@linux.intel.com>
---
 drivers/acpi/apic_id.c |   96 +++++++++++++++++++++++++++++++++++++++++++++++-
 include/linux/acpi.h   |    4 ++
 2 files changed, 98 insertions(+), 2 deletions(-)

diff --git a/drivers/acpi/apic_id.c b/drivers/acpi/apic_id.c
index ada5fd48bad4..ad2e2679c104 100644
--- a/drivers/acpi/apic_id.c
+++ b/drivers/acpi/apic_id.c
@@ -4,11 +4,18 @@
  *
  *	Alex Chiang <achiang@hp.com>
  *	- Unified x86/ia64 implementations
+ *
+ * I/O APIC hotplug support
+ *	Yinghai Lu <yinghai@kernel.org>
+ *	Jiang Liu <jiang.liu@intel.com>
  */
 #include <linux/export.h>
 #include <linux/acpi.h>
 #include "internal.h"
 
+static struct acpi_table_madt *madt;
+static int read_madt;
+
 static int map_lapic_id(struct acpi_subtable_header *entry,
 		 u32 acpi_id, int *apic_id)
 {
@@ -64,8 +71,6 @@ static int map_lsapic_id(struct acpi_subtable_header *entry,
 static int map_madt_entry(int type, u32 acpi_id)
 {
 	unsigned long madt_end, entry;
-	static struct acpi_table_madt *madt;
-	static int read_madt;
 	int apic_id = -1;
 
 	if (!read_madt) {
@@ -200,3 +205,90 @@ int acpi_get_cpuid(acpi_handle handle, int type, u32 acpi_id)
 	return acpi_map_cpuid(apic_id, acpi_id);
 }
 EXPORT_SYMBOL_GPL(acpi_get_cpuid);
+
+#ifdef CONFIG_ACPI_HOTPLUG_IOAPIC
+static int map_ioapic_id(struct acpi_subtable_header *entry, u32 gsi_base,
+			 u64 *phys_addr, int *ioapic_id)
+{
+	struct acpi_madt_io_apic *ioapic = (struct acpi_madt_io_apic *)entry;
+
+	if (ioapic->global_irq_base != gsi_base)
+		return 0;
+
+	*phys_addr = ioapic->address;
+	*ioapic_id = ioapic->id;
+	return 1;
+}
+
+static int map_madt_ioapic_entry(u32 gsi_base, u64 *phys_addr)
+{
+	struct acpi_subtable_header *hdr;
+	unsigned long madt_end, entry;
+	int apic_id = -1;
+
+	if (!read_madt) {
+		if (ACPI_FAILURE(acpi_get_table(ACPI_SIG_MADT, 0,
+					(struct acpi_table_header **)&madt)))
+			madt = NULL;
+		read_madt++;
+	}
+
+	if (!madt)
+		return apic_id;
+
+	entry = (unsigned long)madt;
+	madt_end = entry + madt->header.length;
+
+	/* Parse all entries looking for a match. */
+	entry += sizeof(struct acpi_table_madt);
+	while (entry + sizeof(struct acpi_subtable_header) < madt_end) {
+		hdr = (struct acpi_subtable_header *)entry;
+		if (hdr->type == ACPI_MADT_TYPE_IO_APIC &&
+		    map_ioapic_id(hdr, gsi_base, phys_addr, &apic_id))
+			break;
+		else
+			entry += hdr->length;
+	}
+
+	return apic_id;
+}
+
+static int map_mat_ioapic_entry(acpi_handle handle, u32 gsi_base,
+				u64 *phys_addr)
+{
+	struct acpi_buffer buffer = { ACPI_ALLOCATE_BUFFER, NULL };
+	union acpi_object *obj;
+	struct acpi_subtable_header *header;
+	int apic_id = -1;
+
+	if (ACPI_FAILURE(acpi_evaluate_object(handle, "_MAT", NULL, &buffer)))
+		goto exit;
+
+	if (!buffer.length || !buffer.pointer)
+		goto exit;
+
+	obj = buffer.pointer;
+	if (obj->type != ACPI_TYPE_BUFFER ||
+	    obj->buffer.length < sizeof(struct acpi_subtable_header))
+		goto exit;
+
+	header = (struct acpi_subtable_header *)obj->buffer.pointer;
+	if (header->type == ACPI_MADT_TYPE_IO_APIC)
+		map_ioapic_id(header, gsi_base, phys_addr, &apic_id);
+
+exit:
+	kfree(buffer.pointer);
+	return apic_id;
+}
+
+int acpi_get_ioapic_id(acpi_handle handle, u32 gsi_base, u64 *phys_addr)
+{
+	int apic_id;
+
+	apic_id = map_mat_ioapic_entry(handle, gsi_base, phys_addr);
+	if (apic_id == -1)
+		apic_id = map_madt_ioapic_entry(gsi_base, phys_addr);
+
+	return apic_id;
+}
+#endif /* CONFIG_ACPI_HOTPLUG_IOAPIC */
diff --git a/include/linux/acpi.h b/include/linux/acpi.h
index 05ed6886f1f8..b41c5aef5336 100644
--- a/include/linux/acpi.h
+++ b/include/linux/acpi.h
@@ -149,6 +149,10 @@ int acpi_map_lsapic(acpi_handle handle, int physid, int *pcpu);
 int acpi_unmap_lsapic(int cpu);
 #endif /* CONFIG_ACPI_HOTPLUG_CPU */
 
+#ifdef CONFIG_ACPI_HOTPLUG_IOAPIC
+int acpi_get_ioapic_id(acpi_handle handle, u32 gsi_base, u64 *phys_addr);
+#endif
+
 int acpi_register_ioapic(acpi_handle handle, u64 phys_addr, u32 gsi_base);
 int acpi_unregister_ioapic(acpi_handle handle, u32 gsi_base);
 void acpi_irq_stats_init(void);
-- 
1.7.10.4

Re: [Patch v4 05/16] ACPI: Add interfaces to parse IOAPIC ID for IOAPIC hotplug

[Thomas Gleixner]

On Thu, 28 Aug 2014, Jiang Liu wrote:
>  
> +static struct acpi_table_madt *madt;
> +static int read_madt;

Pretty lousy file visible variable names.

So we end up with two copies of the butt ugly 

   if (!read_madt) {
      .....
   }

stuff instead of creating a helper function which hides that and do

      madr = get_madt();
      if (!madt)
		return ...;

at the call sites.

>  static int map_lapic_id(struct acpi_subtable_header *entry,
>  		 u32 acpi_id, int *apic_id)
>  {
> @@ -64,8 +71,6 @@ static int map_lsapic_id(struct acpi_subtable_header *entry,
>  static int map_madt_entry(int type, u32 acpi_id)
>  {
>  	unsigned long madt_end, entry;
> -	static struct acpi_table_madt *madt;
> -	static int read_madt;
>  	int apic_id = -1;
>  
>  	if (!read_madt) {
> @@ -200,3 +205,90 @@ int acpi_get_cpuid(acpi_handle handle, int type, u32 acpi_id)
>  	return acpi_map_cpuid(apic_id, acpi_id);
>  }
>  EXPORT_SYMBOL_GPL(acpi_get_cpuid);
> +
> +#ifdef CONFIG_ACPI_HOTPLUG_IOAPIC
> +static int map_ioapic_id(struct acpi_subtable_header *entry, u32 gsi_base,
> +			 u64 *phys_addr, int *ioapic_id)
> +{
> +	struct acpi_madt_io_apic *ioapic = (struct acpi_madt_io_apic *)entry;
> +
> +	if (ioapic->global_irq_base != gsi_base)
> +		return 0;
> +
> +	*phys_addr = ioapic->address;
> +	*ioapic_id = ioapic->id;
> +	return 1;
> +}
> +
> +static int map_madt_ioapic_entry(u32 gsi_base, u64 *phys_addr)
> +{
> +	struct acpi_subtable_header *hdr;
> +	unsigned long madt_end, entry;
> +	int apic_id = -1;
> +
> +	if (!read_madt) {
> +		if (ACPI_FAILURE(acpi_get_table(ACPI_SIG_MADT, 0,
> +					(struct acpi_table_header **)&madt)))
> +			madt = NULL;
> +		read_madt++;
> +	}
> +
> +	if (!madt)
> +		return apic_id;
> +
> +	entry = (unsigned long)madt;
> +	madt_end = entry + madt->header.length;
> +
> +	/* Parse all entries looking for a match. */
> +	entry += sizeof(struct acpi_table_madt);
> +	while (entry + sizeof(struct acpi_subtable_header) < madt_end) {
> +		hdr = (struct acpi_subtable_header *)entry;
> +		if (hdr->type == ACPI_MADT_TYPE_IO_APIC &&
> +		    map_ioapic_id(hdr, gsi_base, phys_addr, &apic_id))
> +			break;
> +		else
> +			entry += hdr->length;
> +	}
> +
> +	return apic_id;
> +}
> +
> +static int map_mat_ioapic_entry(acpi_handle handle, u32 gsi_base,
> +				u64 *phys_addr)
> +{
> +	struct acpi_buffer buffer = { ACPI_ALLOCATE_BUFFER, NULL };
> +	union acpi_object *obj;
> +	struct acpi_subtable_header *header;
> +	int apic_id = -1;
> +
> +	if (ACPI_FAILURE(acpi_evaluate_object(handle, "_MAT", NULL, &buffer)))
> +		goto exit;
> +
> +	if (!buffer.length || !buffer.pointer)
> +		goto exit;
> +
> +	obj = buffer.pointer;
> +	if (obj->type != ACPI_TYPE_BUFFER ||
> +	    obj->buffer.length < sizeof(struct acpi_subtable_header))
> +		goto exit;
> +
> +	header = (struct acpi_subtable_header *)obj->buffer.pointer;
> +	if (header->type == ACPI_MADT_TYPE_IO_APIC)
> +		map_ioapic_id(header, gsi_base, phys_addr, &apic_id);
> +
> +exit:
> +	kfree(buffer.pointer);
> +	return apic_id;
> +}


This stuff really wants proper documentation. The lack of comments is
just annoying.

> +int acpi_get_ioapic_id(acpi_handle handle, u32 gsi_base, u64 *phys_addr)
> +{
> +	int apic_id;
> +
> +	apic_id = map_mat_ioapic_entry(handle, gsi_base, phys_addr);
> +	if (apic_id == -1)
> +		apic_id = map_madt_ioapic_entry(gsi_base, phys_addr);
> +
> +	return apic_id;
> +}

Thanks,

	tglx

Re: [Patch v4 05/16] ACPI: Add interfaces to parse IOAPIC ID for IOAPIC hotplug

[Jiang Liu]

On 2014/9/9 18:54, Thomas Gleixner wrote:
> On Thu, 28 Aug 2014, Jiang Liu wrote:
>>  
>> +static struct acpi_table_madt *madt;
>> +static int read_madt;
> 
> Pretty lousy file visible variable names.
> 
> So we end up with two copies of the butt ugly 
> 
>    if (!read_madt) {
>       .....
>    }
> 
> stuff instead of creating a helper function which hides that and do
> 
>       madr = get_madt();
>       if (!madt)
> 		return ...;
> 
> at the call sites.
Thanks for suggestion, will improve in that way.

> 
>>  static int map_lapic_id(struct acpi_subtable_header *entry,
>>  		 u32 acpi_id, int *apic_id)
>>  {
>> @@ -64,8 +71,6 @@ static int map_lsapic_id(struct acpi_subtable_header *entry,
>>  static int map_madt_entry(int type, u32 acpi_id)
>>  {
>>  	unsigned long madt_end, entry;
>> -	static struct acpi_table_madt *madt;
>> -	static int read_madt;
>>  	int apic_id = -1;
>>  
>>  	if (!read_madt) {
>> @@ -200,3 +205,90 @@ int acpi_get_cpuid(acpi_handle handle, int type, u32 acpi_id)
>>  	return acpi_map_cpuid(apic_id, acpi_id);
>>  }
>>  EXPORT_SYMBOL_GPL(acpi_get_cpuid);
>> +
>> +#ifdef CONFIG_ACPI_HOTPLUG_IOAPIC
>> +static int map_ioapic_id(struct acpi_subtable_header *entry, u32 gsi_base,
>> +			 u64 *phys_addr, int *ioapic_id)
>> +{
>> +	struct acpi_madt_io_apic *ioapic = (struct acpi_madt_io_apic *)entry;
>> +
>> +	if (ioapic->global_irq_base != gsi_base)
>> +		return 0;
>> +
>> +	*phys_addr = ioapic->address;
>> +	*ioapic_id = ioapic->id;
>> +	return 1;
>> +}
>> +
>> +static int map_madt_ioapic_entry(u32 gsi_base, u64 *phys_addr)
>> +{
>> +	struct acpi_subtable_header *hdr;
>> +	unsigned long madt_end, entry;
>> +	int apic_id = -1;
>> +
>> +	if (!read_madt) {
>> +		if (ACPI_FAILURE(acpi_get_table(ACPI_SIG_MADT, 0,
>> +					(struct acpi_table_header **)&madt)))
>> +			madt = NULL;
>> +		read_madt++;
>> +	}
>> +
>> +	if (!madt)
>> +		return apic_id;
>> +
>> +	entry = (unsigned long)madt;
>> +	madt_end = entry + madt->header.length;
>> +
>> +	/* Parse all entries looking for a match. */
>> +	entry += sizeof(struct acpi_table_madt);
>> +	while (entry + sizeof(struct acpi_subtable_header) < madt_end) {
>> +		hdr = (struct acpi_subtable_header *)entry;
>> +		if (hdr->type == ACPI_MADT_TYPE_IO_APIC &&
>> +		    map_ioapic_id(hdr, gsi_base, phys_addr, &apic_id))
>> +			break;
>> +		else
>> +			entry += hdr->length;
>> +	}
>> +
>> +	return apic_id;
>> +}
>> +
>> +static int map_mat_ioapic_entry(acpi_handle handle, u32 gsi_base,
>> +				u64 *phys_addr)
>> +{
>> +	struct acpi_buffer buffer = { ACPI_ALLOCATE_BUFFER, NULL };
>> +	union acpi_object *obj;
>> +	struct acpi_subtable_header *header;
>> +	int apic_id = -1;
>> +
>> +	if (ACPI_FAILURE(acpi_evaluate_object(handle, "_MAT", NULL, &buffer)))
>> +		goto exit;
>> +
>> +	if (!buffer.length || !buffer.pointer)
>> +		goto exit;
>> +
>> +	obj = buffer.pointer;
>> +	if (obj->type != ACPI_TYPE_BUFFER ||
>> +	    obj->buffer.length < sizeof(struct acpi_subtable_header))
>> +		goto exit;
>> +
>> +	header = (struct acpi_subtable_header *)obj->buffer.pointer;
>> +	if (header->type == ACPI_MADT_TYPE_IO_APIC)
>> +		map_ioapic_id(header, gsi_base, phys_addr, &apic_id);
>> +
>> +exit:
>> +	kfree(buffer.pointer);
>> +	return apic_id;
>> +}
> 
> 
> This stuff really wants proper documentation. The lack of comments is
> just annoying.
Sure, will improve comments in next version.

Regards!
Gerry

> 
>> +int acpi_get_ioapic_id(acpi_handle handle, u32 gsi_base, u64 *phys_addr)
>> +{
>> +	int apic_id;
>> +
>> +	apic_id = map_mat_ioapic_entry(handle, gsi_base, phys_addr);
>> +	if (apic_id == -1)
>> +		apic_id = map_madt_ioapic_entry(gsi_base, phys_addr);
>> +
>> +	return apic_id;
>> +}
> 
> Thanks,
> 
> 	tglx
> 

[Patch v4 06/16] x86, irq: Split out alloc_ioapic_save_registers()

[Jiang Liu]

From: Yinghai Lu <yinghai@kernel.org>

Split alloc_ioapic_save_registers() from early_irq_init(),
so it will be used per ioapic.

Will call that later for hot-added ioapic controller.

Signed-off-by: Yinghai Lu <yinghai@kernel.org>
Signed-off-by: Jiang Liu <jiang.liu@linux.intel.com>
Cc: Joerg Roedel <joro@8bytes.org>
Cc: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
Cc: Sebastian Andrzej Siewior <sebastian@breakpoint.cc>
---
 arch/x86/kernel/apic/io_apic.c |   22 +++++++++++++++-------
 1 file changed, 15 insertions(+), 7 deletions(-)

diff --git a/arch/x86/kernel/apic/io_apic.c b/arch/x86/kernel/apic/io_apic.c
index 40a4aa3f4061..3faf9599ff29 100644
--- a/arch/x86/kernel/apic/io_apic.c
+++ b/arch/x86/kernel/apic/io_apic.c
@@ -237,6 +237,19 @@ static struct irq_pin_list *alloc_irq_pin_list(int node)
 	return kzalloc_node(sizeof(struct irq_pin_list), GFP_KERNEL, node);
 }
 
+static void alloc_ioapic_saved_registers(int idx)
+{
+	size_t size;
+
+	if (ioapics[idx].saved_registers)
+		return;
+
+	size = sizeof(struct IO_APIC_route_entry) * ioapics[idx].nr_registers;
+	ioapics[idx].saved_registers = kzalloc(size, GFP_KERNEL);
+	if (!ioapics[idx].saved_registers)
+		pr_err("IOAPIC %d: suspend/resume impossible!\n", idx);
+}
+
 int __init arch_early_irq_init(void)
 {
 	struct irq_cfg *cfg;
@@ -245,13 +258,8 @@ int __init arch_early_irq_init(void)
 	if (!nr_legacy_irqs())
 		io_apic_irqs = ~0UL;
 
-	for_each_ioapic(i) {
-		ioapics[i].saved_registers =
-			kzalloc(sizeof(struct IO_APIC_route_entry) *
-				ioapics[i].nr_registers, GFP_KERNEL);
-		if (!ioapics[i].saved_registers)
-			pr_err("IOAPIC %d: suspend/resume impossible!\n", i);
-	}
+	for_each_ioapic(i)
+		alloc_ioapic_saved_registers(i);
 
 	/*
 	 * For legacy IRQ's, start with assigning irq0 to irq15 to
-- 
1.7.10.4

[Patch v4 07/16] x86, irq: Prefer assigned ID in APIC ID register for x86_64

[Jiang Liu]

From: Yinghai Lu <yinghai@kernel.org>

Perfer the assigned ID in APIC ID register for x86_64 if it's still
available.

Signed-off-by: Yinghai Lu <yinghai@kernel.org>
Cc: Joerg Roedel <joro@8bytes.org>
Cc: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
Cc: Sebastian Andrzej Siewior <sebastian@breakpoint.cc>
Signed-off-by: Jiang Liu <jiang.liu@linux.intel.com>
---
 arch/x86/kernel/apic/io_apic.c |   38 +++++++++++++++++++++++++++++++++-----
 1 file changed, 33 insertions(+), 5 deletions(-)

diff --git a/arch/x86/kernel/apic/io_apic.c b/arch/x86/kernel/apic/io_apic.c
index 3faf9599ff29..196d9c15fdec 100644
--- a/arch/x86/kernel/apic/io_apic.c
+++ b/arch/x86/kernel/apic/io_apic.c
@@ -3575,26 +3575,54 @@ static int __init io_apic_get_unique_id(int ioapic, int apic_id)
 	return apic_id;
 }
 
-static u8 __init io_apic_unique_id(u8 id)
+static u8 io_apic_unique_id(int idx, u8 id)
 {
 	if ((boot_cpu_data.x86_vendor == X86_VENDOR_INTEL) &&
 	    !APIC_XAPIC(apic_version[boot_cpu_physical_apicid]))
-		return io_apic_get_unique_id(nr_ioapics, id);
+		return io_apic_get_unique_id(idx, id);
 	else
 		return id;
 }
 #else
-static u8 __init io_apic_unique_id(u8 id)
+static u8 io_apic_unique_id(int idx, u8 id)
 {
 	int i;
+	u8 new_id;
+	unsigned long flags;
 	DECLARE_BITMAP(used, 256);
+	union IO_APIC_reg_00 reg_00;
 
 	bitmap_zero(used, 256);
 	for_each_ioapic(i)
 		__set_bit(mpc_ioapic_id(i), used);
 	if (!test_bit(id, used))
 		return id;
-	return find_first_zero_bit(used, 256);
+
+	/* check register at first */
+	raw_spin_lock_irqsave(&ioapic_lock, flags);
+	reg_00.raw = io_apic_read(idx, 0);
+	raw_spin_unlock_irqrestore(&ioapic_lock, flags);
+	new_id = reg_00.bits.ID;
+	if (!test_bit(new_id, used)) {
+		apic_printk(APIC_VERBOSE, KERN_INFO
+			"IOAPIC[%d]: Using reg apic_id %d instead of %d\n",
+			 idx, new_id, id);
+		return new_id;
+	}
+
+	new_id = find_first_zero_bit(used, 256);
+	reg_00.bits.ID = new_id;
+	raw_spin_lock_irqsave(&ioapic_lock, flags);
+	io_apic_write(idx, 0, reg_00.raw);
+	reg_00.raw = io_apic_read(idx, 0);
+	raw_spin_unlock_irqrestore(&ioapic_lock, flags);
+
+	/* Sanity check */
+	if (reg_00.bits.ID != new_id)
+		pr_warn("IOAPIC[%d]: Unable to change apic_id to %d!\n",
+			idx, new_id);
+
+	return new_id;
 }
 #endif
 
@@ -3860,7 +3888,7 @@ void __init mp_register_ioapic(int id, u32 address, u32 gsi_base,
 		return;
 	}
 
-	ioapics[idx].mp_config.apicid = io_apic_unique_id(id);
+	ioapics[idx].mp_config.apicid = io_apic_unique_id(idx, id);
 	ioapics[idx].mp_config.apicver = io_apic_get_version(idx);
 
 	/*
-- 
1.7.10.4

Re: [Patch v4 07/16] x86, irq: Prefer assigned ID in APIC ID register for x86_64

[Thomas Gleixner]

On Thu, 28 Aug 2014, Jiang Liu wrote:

> From: Yinghai Lu <yinghai@kernel.org>
> 
> Perfer the assigned ID in APIC ID register for x86_64 if it's still
> available.
> 
> Signed-off-by: Yinghai Lu <yinghai@kernel.org>
> Cc: Joerg Roedel <joro@8bytes.org>
> Cc: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
> Cc: Sebastian Andrzej Siewior <sebastian@breakpoint.cc>
> Signed-off-by: Jiang Liu <jiang.liu@linux.intel.com>
> ---
>  arch/x86/kernel/apic/io_apic.c |   38 +++++++++++++++++++++++++++++++++-----
>  1 file changed, 33 insertions(+), 5 deletions(-)
> 
> diff --git a/arch/x86/kernel/apic/io_apic.c b/arch/x86/kernel/apic/io_apic.c
> index 3faf9599ff29..196d9c15fdec 100644
> --- a/arch/x86/kernel/apic/io_apic.c
> +++ b/arch/x86/kernel/apic/io_apic.c
> @@ -3575,26 +3575,54 @@ static int __init io_apic_get_unique_id(int ioapic, int apic_id)
>  	return apic_id;
>  }
>  
> -static u8 __init io_apic_unique_id(u8 id)
> +static u8 io_apic_unique_id(int idx, u8 id)

How is that change related to the changelog?

>  {
>  	if ((boot_cpu_data.x86_vendor == X86_VENDOR_INTEL) &&
>  	    !APIC_XAPIC(apic_version[boot_cpu_physical_apicid]))
> -		return io_apic_get_unique_id(nr_ioapics, id);
> +		return io_apic_get_unique_id(idx, id);
>  	else
>  		return id;
>  }
>  #else
> -static u8 __init io_apic_unique_id(u8 id)
> +static u8 io_apic_unique_id(int idx, u8 id)
>  {
>  	int i;
> +	u8 new_id;
> +	unsigned long flags;
>  	DECLARE_BITMAP(used, 256);
> +	union IO_APIC_reg_00 reg_00;
>  
>  	bitmap_zero(used, 256);
>  	for_each_ioapic(i)
>  		__set_bit(mpc_ioapic_id(i), used);
>  	if (!test_bit(id, used))
>  		return id;
> -	return find_first_zero_bit(used, 256);
> +
> +	/* check register at first */
> +	raw_spin_lock_irqsave(&ioapic_lock, flags);
> +	reg_00.raw = io_apic_read(idx, 0);
> +	raw_spin_unlock_irqrestore(&ioapic_lock, flags);
> +	new_id = reg_00.bits.ID;
> +	if (!test_bit(new_id, used)) {
> +		apic_printk(APIC_VERBOSE, KERN_INFO
> +			"IOAPIC[%d]: Using reg apic_id %d instead of %d\n",
> +			 idx, new_id, id);
> +		return new_id;
> +	}
> +
> +	new_id = find_first_zero_bit(used, 256);
> +	reg_00.bits.ID = new_id;
> +	raw_spin_lock_irqsave(&ioapic_lock, flags);
> +	io_apic_write(idx, 0, reg_00.raw);
> +	reg_00.raw = io_apic_read(idx, 0);
> +	raw_spin_unlock_irqrestore(&ioapic_lock, flags);
> +
> +	/* Sanity check */
> +	if (reg_00.bits.ID != new_id)
> +		pr_warn("IOAPIC[%d]: Unable to change apic_id to %d!\n",
> +			idx, new_id);

So we detect, that the ID could not be changed and we return it
nevertheless?

Thanks,

	tglx

Re: [Patch v4 07/16] x86, irq: Prefer assigned ID in APIC ID register for x86_64

[Jiang Liu]

On 2014/9/9 19:04, Thomas Gleixner wrote:
> On Thu, 28 Aug 2014, Jiang Liu wrote:
> 
>> From: Yinghai Lu <yinghai@kernel.org>
>>
>> Perfer the assigned ID in APIC ID register for x86_64 if it's still
>> available.
>>
>> Signed-off-by: Yinghai Lu <yinghai@kernel.org>
>> Cc: Joerg Roedel <joro@8bytes.org>
>> Cc: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
>> Cc: Sebastian Andrzej Siewior <sebastian@breakpoint.cc>
>> Signed-off-by: Jiang Liu <jiang.liu@linux.intel.com>
>> ---
>>  arch/x86/kernel/apic/io_apic.c |   38 +++++++++++++++++++++++++++++++++-----
>>  1 file changed, 33 insertions(+), 5 deletions(-)
>>
>> diff --git a/arch/x86/kernel/apic/io_apic.c b/arch/x86/kernel/apic/io_apic.c
>> index 3faf9599ff29..196d9c15fdec 100644
>> --- a/arch/x86/kernel/apic/io_apic.c
>> +++ b/arch/x86/kernel/apic/io_apic.c
>> @@ -3575,26 +3575,54 @@ static int __init io_apic_get_unique_id(int ioapic, int apic_id)
>>  	return apic_id;
>>  }
>>  
>> -static u8 __init io_apic_unique_id(u8 id)
>> +static u8 io_apic_unique_id(int idx, u8 id)
> 
> How is that change related to the changelog?
My bad, removal of __init marker belongs to next patch.

> 
>>  {
>>  	if ((boot_cpu_data.x86_vendor == X86_VENDOR_INTEL) &&
>>  	    !APIC_XAPIC(apic_version[boot_cpu_physical_apicid]))
>> -		return io_apic_get_unique_id(nr_ioapics, id);
>> +		return io_apic_get_unique_id(idx, id);
>>  	else
>>  		return id;
>>  }
>>  #else
>> -static u8 __init io_apic_unique_id(u8 id)
>> +static u8 io_apic_unique_id(int idx, u8 id)
>>  {
>>  	int i;
>> +	u8 new_id;
>> +	unsigned long flags;
>>  	DECLARE_BITMAP(used, 256);
>> +	union IO_APIC_reg_00 reg_00;
>>  
>>  	bitmap_zero(used, 256);
>>  	for_each_ioapic(i)
>>  		__set_bit(mpc_ioapic_id(i), used);
>>  	if (!test_bit(id, used))
>>  		return id;
>> -	return find_first_zero_bit(used, 256);
>> +
>> +	/* check register at first */
>> +	raw_spin_lock_irqsave(&ioapic_lock, flags);
>> +	reg_00.raw = io_apic_read(idx, 0);
>> +	raw_spin_unlock_irqrestore(&ioapic_lock, flags);
>> +	new_id = reg_00.bits.ID;
>> +	if (!test_bit(new_id, used)) {
>> +		apic_printk(APIC_VERBOSE, KERN_INFO
>> +			"IOAPIC[%d]: Using reg apic_id %d instead of %d\n",
>> +			 idx, new_id, id);
>> +		return new_id;
>> +	}
>> +
>> +	new_id = find_first_zero_bit(used, 256);
>> +	reg_00.bits.ID = new_id;
>> +	raw_spin_lock_irqsave(&ioapic_lock, flags);
>> +	io_apic_write(idx, 0, reg_00.raw);
>> +	reg_00.raw = io_apic_read(idx, 0);
>> +	raw_spin_unlock_irqrestore(&ioapic_lock, flags);
>> +
>> +	/* Sanity check */
>> +	if (reg_00.bits.ID != new_id)
>> +		pr_warn("IOAPIC[%d]: Unable to change apic_id to %d!\n",
>> +			idx, new_id);
> 
> So we detect, that the ID could not be changed and we return it
> nevertheless?
Seems it deserves a BUG_ON here. We have tried the ID from ACPI table,
the ID from hardware register, they all fails. And we can't change the
hardware register too. Seems no way out here.
Regards!
Gerry
> 
> Thanks,
> 
> 	tglx
> 

[Patch v4 08/16] x86, irq: Remove __init marker for functions will be used by IOAPIC hotplug

[Jiang Liu]

Remove __init marker for functions which will be used by IOAPIC hotplug
at runtime.

Signed-off-by: Jiang Liu <jiang.liu@linux.intel.com>
---
 arch/x86/include/asm/io_apic.h |    4 ++--
 arch/x86/kernel/apic/io_apic.c |   14 +++++++-------
 2 files changed, 9 insertions(+), 9 deletions(-)

diff --git a/arch/x86/include/asm/io_apic.h b/arch/x86/include/asm/io_apic.h
index 0aeed5ca356e..0b31aebd9405 100644
--- a/arch/x86/include/asm/io_apic.h
+++ b/arch/x86/include/asm/io_apic.h
@@ -188,8 +188,8 @@ extern int mp_find_ioapic_pin(int ioapic, u32 gsi);
 extern u32 mp_pin_to_gsi(int ioapic, int pin);
 extern int mp_map_gsi_to_irq(u32 gsi, unsigned int flags);
 extern void mp_unmap_irq(int irq);
-extern void __init mp_register_ioapic(int id, u32 address, u32 gsi_base,
-				      struct ioapic_domain_cfg *cfg);
+extern void mp_register_ioapic(int id, u32 address, u32 gsi_base,
+			       struct ioapic_domain_cfg *cfg);
 extern int mp_irqdomain_map(struct irq_domain *domain, unsigned int virq,
 			    irq_hw_number_t hwirq);
 extern void mp_irqdomain_unmap(struct irq_domain *domain, unsigned int virq);
diff --git a/arch/x86/kernel/apic/io_apic.c b/arch/x86/kernel/apic/io_apic.c
index 196d9c15fdec..1ac868128a61 100644
--- a/arch/x86/kernel/apic/io_apic.c
+++ b/arch/x86/kernel/apic/io_apic.c
@@ -3454,7 +3454,7 @@ io_apic_setup_irq_pin(unsigned int irq, int node, struct io_apic_irq_attr *attr)
 	return ret;
 }
 
-static int __init io_apic_get_redir_entries(int ioapic)
+static int io_apic_get_redir_entries(int ioapic)
 {
 	union IO_APIC_reg_01	reg_01;
 	unsigned long flags;
@@ -3500,7 +3500,7 @@ int __init arch_probe_nr_irqs(void)
 }
 
 #ifdef CONFIG_X86_32
-static int __init io_apic_get_unique_id(int ioapic, int apic_id)
+static int io_apic_get_unique_id(int ioapic, int apic_id)
 {
 	union IO_APIC_reg_00 reg_00;
 	static physid_mask_t apic_id_map = PHYSID_MASK_NONE;
@@ -3626,7 +3626,7 @@ static u8 io_apic_unique_id(int idx, u8 id)
 }
 #endif
 
-static int __init io_apic_get_version(int ioapic)
+static int io_apic_get_version(int ioapic)
 {
 	union IO_APIC_reg_01	reg_01;
 	unsigned long flags;
@@ -3830,7 +3830,7 @@ int mp_find_ioapic_pin(int ioapic, u32 gsi)
 	return gsi - gsi_cfg->gsi_base;
 }
 
-static __init int bad_ioapic(unsigned long address)
+static int bad_ioapic(unsigned long address)
 {
 	if (nr_ioapics >= MAX_IO_APICS) {
 		pr_warn("WARNING: Max # of I/O APICs (%d) exceeded (found %d), skipping\n",
@@ -3844,7 +3844,7 @@ static __init int bad_ioapic(unsigned long address)
 	return 0;
 }
 
-static __init int bad_ioapic_register(int idx)
+static int bad_ioapic_register(int idx)
 {
 	union IO_APIC_reg_00 reg_00;
 	union IO_APIC_reg_01 reg_01;
@@ -3863,8 +3863,8 @@ static __init int bad_ioapic_register(int idx)
 	return 0;
 }
 
-void __init mp_register_ioapic(int id, u32 address, u32 gsi_base,
-			       struct ioapic_domain_cfg *cfg)
+void mp_register_ioapic(int id, u32 address, u32 gsi_base,
+			struct ioapic_domain_cfg *cfg)
 {
 	int idx = 0;
 	int entries;
-- 
1.7.10.4

[Patch v4 09/16] x86, irq: Keep balance of IOAPIC pin reference count

[Jiang Liu]

To keep balance of IOAPIC pin reference count, we need to protect
pirq_enable_irq(), acpi_pci_irq_enable() and intel_mid_pci_irq_enable()
from reentrance. There are two cases which will cause reentrance.

The first case is caused by suspend/hibernation. If pcibios_disable_irq
is called during suspending/hibernating, we don't release the assigned
IRQ number, otherwise it may break the suspend/hibernation. So late when
pcibios_enable_irq is called during resume, we shouldn't allocate IRQ
number again.

The second case is that function acpi_pci_irq_enable() may be called
twice for PCI devices present at boot time as below:
1) pci_acpi_init()
	--> acpi_pci_irq_enable() if pci_routeirq is true
2) pci_enable_device()
	--> pcibios_enable_device()
		--> acpi_pci_irq_enable()
We can't kill kernel parameter pci_routeirq yet because it's still
needed for debugging purpose.

So flag irq_managed is introduced to track whether IRQ number is
assigned by OS and to protect pirq_enable_irq(), acpi_pci_irq_enable()
and intel_mid_pci_irq_enable() from reentrance.

Signed-off-by: Jiang Liu <jiang.liu@linux.intel.com>
---
 arch/x86/pci/intel_mid_pci.c |    9 ++++++++-
 arch/x86/pci/irq.c           |    7 ++++++-
 drivers/acpi/pci_irq.c       |   11 +++++++++--
 include/linux/pci.h          |    1 +
 4 files changed, 24 insertions(+), 4 deletions(-)

diff --git a/arch/x86/pci/intel_mid_pci.c b/arch/x86/pci/intel_mid_pci.c
index 3865116c51fb..6a855fef78fc 100644
--- a/arch/x86/pci/intel_mid_pci.c
+++ b/arch/x86/pci/intel_mid_pci.c
@@ -210,6 +210,9 @@ static int intel_mid_pci_irq_enable(struct pci_dev *dev)
 {
 	int polarity;
 
+	if (dev->irq_managed && dev->irq > 0)
+		return 0;
+
 	if (intel_mid_identify_cpu() == INTEL_MID_CPU_CHIP_TANGIER)
 		polarity = 0; /* active high */
 	else
@@ -224,13 +227,17 @@ static int intel_mid_pci_irq_enable(struct pci_dev *dev)
 	if (mp_map_gsi_to_irq(dev->irq, IOAPIC_MAP_ALLOC) < 0)
 		return -EBUSY;
 
+	dev->irq_managed = 1;
+
 	return 0;
 }
 
 static void intel_mid_pci_irq_disable(struct pci_dev *dev)
 {
-	if (!dev->dev.power.is_prepared && dev->irq > 0)
+	if (!dev->dev.power.is_prepared && dev->irq_managed && dev->irq > 0) {
 		mp_unmap_irq(dev->irq);
+		dev->irq_managed = 0;
+	}
 }
 
 struct pci_ops intel_mid_pci_ops = {
diff --git a/arch/x86/pci/irq.c b/arch/x86/pci/irq.c
index bc1a2c341891..dd1369dbcc42 100644
--- a/arch/x86/pci/irq.c
+++ b/arch/x86/pci/irq.c
@@ -1202,6 +1202,9 @@ static int pirq_enable_irq(struct pci_dev *dev)
 			int irq;
 			struct io_apic_irq_attr irq_attr;
 
+			if (dev->irq_managed && dev->irq > 0)
+				return 0;
+
 			irq = IO_APIC_get_PCI_irq_vector(dev->bus->number,
 						PCI_SLOT(dev->devfn),
 						pin - 1, &irq_attr);
@@ -1228,6 +1231,7 @@ static int pirq_enable_irq(struct pci_dev *dev)
 			}
 			dev = temp_dev;
 			if (irq >= 0) {
+				dev->irq_managed = 1;
 				dev->irq = irq;
 				dev_info(&dev->dev, "PCI->APIC IRQ transform: "
 					 "INT %c -> IRQ %d\n", 'A' + pin - 1, irq);
@@ -1257,8 +1261,9 @@ static int pirq_enable_irq(struct pci_dev *dev)
 static void pirq_disable_irq(struct pci_dev *dev)
 {
 	if (io_apic_assign_pci_irqs && !dev->dev.power.is_prepared &&
-	    dev->irq) {
+	    dev->irq_managed && dev->irq > 0) {
 		mp_unmap_irq(dev->irq);
 		dev->irq = 0;
+		dev->irq_managed = 0;
 	}
 }
diff --git a/drivers/acpi/pci_irq.c b/drivers/acpi/pci_irq.c
index c96887d5289e..4a89701dfe36 100644
--- a/drivers/acpi/pci_irq.c
+++ b/drivers/acpi/pci_irq.c
@@ -413,6 +413,9 @@ int acpi_pci_irq_enable(struct pci_dev *dev)
 		return 0;
 	}
 
+	if (dev->irq_managed && dev->irq > 0)
+		return 0;
+
 	entry = acpi_pci_irq_lookup(dev, pin);
 	if (!entry) {
 		/*
@@ -456,6 +459,7 @@ int acpi_pci_irq_enable(struct pci_dev *dev)
 		return rc;
 	}
 	dev->irq = rc;
+	dev->irq_managed = 1;
 
 	if (link)
 		snprintf(link_desc, sizeof(link_desc), " -> Link[%s]", link);
@@ -478,7 +482,7 @@ void acpi_pci_irq_disable(struct pci_dev *dev)
 	u8 pin;
 
 	pin = dev->pin;
-	if (!pin)
+	if (!pin || !dev->irq_managed || dev->irq <= 0)
 		return;
 
 	/* Keep IOAPIC pin configuration when suspending */
@@ -502,6 +506,9 @@ void acpi_pci_irq_disable(struct pci_dev *dev)
 	 */
 
 	dev_dbg(&dev->dev, "PCI INT %c disabled\n", pin_name(pin));
-	if (gsi >= 0 && dev->irq > 0)
+	if (gsi >= 0) {
 		acpi_unregister_gsi(gsi);
+		dev->irq = 0;
+		dev->irq_managed = 0;
+	}
 }
diff --git a/include/linux/pci.h b/include/linux/pci.h
index 61978a460841..3e0b92715512 100644
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -347,6 +347,7 @@ struct pci_dev {
 	unsigned int	__aer_firmware_first:1;
 	unsigned int	broken_intx_masking:1;
 	unsigned int	io_window_1k:1;	/* Intel P2P bridge 1K I/O windows */
+	unsigned int	irq_managed:1;
 	pci_dev_flags_t dev_flags;
 	atomic_t	enable_cnt;	/* pci_enable_device has been called */
 
-- 
1.7.10.4

[Patch v4 10/16] x86, irq: Refine mp_register_ioapic() to prepare for IOAPIC hotplug

[Jiang Liu]

Refine mp_register_ioapic() to prepare for IOAPIC hotplug by:
1) change return value from void to int.
2) check for gsi range conflicts
3) check for IOAPIC physical address conflicts
4) enhance the way to allocate IOAPIC index

Signed-off-by: Jiang Liu <jiang.liu@linux.intel.com>
---
 arch/x86/include/asm/io_apic.h |    4 +-
 arch/x86/kernel/apic/io_apic.c |   80 ++++++++++++++++++++++++----------------
 2 files changed, 51 insertions(+), 33 deletions(-)

diff --git a/arch/x86/include/asm/io_apic.h b/arch/x86/include/asm/io_apic.h
index 0b31aebd9405..94d05bd6586f 100644
--- a/arch/x86/include/asm/io_apic.h
+++ b/arch/x86/include/asm/io_apic.h
@@ -188,8 +188,8 @@ extern int mp_find_ioapic_pin(int ioapic, u32 gsi);
 extern u32 mp_pin_to_gsi(int ioapic, int pin);
 extern int mp_map_gsi_to_irq(u32 gsi, unsigned int flags);
 extern void mp_unmap_irq(int irq);
-extern void mp_register_ioapic(int id, u32 address, u32 gsi_base,
-			       struct ioapic_domain_cfg *cfg);
+extern int mp_register_ioapic(int id, u32 address, u32 gsi_base,
+			      struct ioapic_domain_cfg *cfg);
 extern int mp_irqdomain_map(struct irq_domain *domain, unsigned int virq,
 			    irq_hw_number_t hwirq);
 extern void mp_irqdomain_unmap(struct irq_domain *domain, unsigned int virq);
diff --git a/arch/x86/kernel/apic/io_apic.c b/arch/x86/kernel/apic/io_apic.c
index 1ac868128a61..6e67af0c5f99 100644
--- a/arch/x86/kernel/apic/io_apic.c
+++ b/arch/x86/kernel/apic/io_apic.c
@@ -3830,20 +3830,6 @@ int mp_find_ioapic_pin(int ioapic, u32 gsi)
 	return gsi - gsi_cfg->gsi_base;
 }
 
-static int bad_ioapic(unsigned long address)
-{
-	if (nr_ioapics >= MAX_IO_APICS) {
-		pr_warn("WARNING: Max # of I/O APICs (%d) exceeded (found %d), skipping\n",
-			MAX_IO_APICS, nr_ioapics);
-		return 1;
-	}
-	if (!address) {
-		pr_warn("WARNING: Bogus (zero) I/O APIC address found in table, skipping!\n");
-		return 1;
-	}
-	return 0;
-}
-
 static int bad_ioapic_register(int idx)
 {
 	union IO_APIC_reg_00 reg_00;
@@ -3863,29 +3849,44 @@ static int bad_ioapic_register(int idx)
 	return 0;
 }
 
-void mp_register_ioapic(int id, u32 address, u32 gsi_base,
-			struct ioapic_domain_cfg *cfg)
+static int find_free_ioapic_entry(void)
 {
-	int idx = 0;
-	int entries;
+	return nr_ioapics;
+}
+
+int mp_register_ioapic(int id, u32 address, u32 gsi_base,
+		       struct ioapic_domain_cfg *cfg)
+{
+	u32 gsi_end;
+	int idx, ioapic, entries;
 	struct mp_ioapic_gsi *gsi_cfg;
 
-	if (bad_ioapic(address))
-		return;
+	if (!address) {
+		pr_warn("Bogus (zero) I/O APIC address found, skipping!\n");
+		return -EINVAL;
+	}
+	for_each_ioapic(ioapic)
+		if (ioapics[ioapic].mp_config.apicaddr == address) {
+			pr_warn("address 0x%x conflicts with IOAPIC%d\n",
+				address, ioapic);
+			return -EEXIST;
+		}
 
-	idx = nr_ioapics;
+	idx = find_free_ioapic_entry();
+	if (idx >= MAX_IO_APICS) {
+		pr_warn("Max # of I/O APICs (%d) exceeded (found %d), skipping\n",
+			MAX_IO_APICS, idx);
+		return -ENOSPC;
+	}
 
 	ioapics[idx].mp_config.type = MP_IOAPIC;
 	ioapics[idx].mp_config.flags = MPC_APIC_USABLE;
 	ioapics[idx].mp_config.apicaddr = address;
-	ioapics[idx].irqdomain = NULL;
-	ioapics[idx].irqdomain_cfg = *cfg;
 
 	set_fixmap_nocache(FIX_IO_APIC_BASE_0 + idx, address);
-
 	if (bad_ioapic_register(idx)) {
 		clear_fixmap(FIX_IO_APIC_BASE_0 + idx);
-		return;
+		return -ENODEV;
 	}
 
 	ioapics[idx].mp_config.apicid = io_apic_unique_id(idx, id);
@@ -3896,24 +3897,41 @@ void mp_register_ioapic(int id, u32 address, u32 gsi_base,
 	 * and to prevent reprogramming of IOAPIC pins (PCI GSIs).
 	 */
 	entries = io_apic_get_redir_entries(idx);
+	gsi_end = gsi_base + entries - 1;
+	for_each_ioapic(ioapic) {
+		gsi_cfg = mp_ioapic_gsi_routing(ioapic);
+		if ((gsi_base >= gsi_cfg->gsi_base &&
+		     gsi_base <= gsi_cfg->gsi_end) ||
+		    (gsi_end >= gsi_cfg->gsi_base &&
+		     gsi_end <= gsi_cfg->gsi_end)) {
+			pr_warn("GSI range [%u-%u] for new IOAPIC conflicts with GSI[%u-%u]\n",
+				gsi_base, gsi_end,
+				gsi_cfg->gsi_base, gsi_cfg->gsi_end);
+			clear_fixmap(FIX_IO_APIC_BASE_0 + idx);
+			return -ENOSPC;
+		}
+	}
 	gsi_cfg = mp_ioapic_gsi_routing(idx);
 	gsi_cfg->gsi_base = gsi_base;
-	gsi_cfg->gsi_end = gsi_base + entries - 1;
+	gsi_cfg->gsi_end = gsi_end;
 
-	/*
-	 * The number of IO-APIC IRQ registers (== #pins):
-	 */
-	ioapics[idx].nr_registers = entries;
+	ioapics[idx].irqdomain = NULL;
+	ioapics[idx].irqdomain_cfg = *cfg;
 
 	if (gsi_cfg->gsi_end >= gsi_top)
 		gsi_top = gsi_cfg->gsi_end + 1;
+	if (nr_ioapics <= idx)
+		nr_ioapics = idx + 1;
+
+	/* Set nr_registers to mark entry present */
+	ioapics[idx].nr_registers = entries;
 
 	pr_info("IOAPIC[%d]: apic_id %d, version %d, address 0x%x, GSI %d-%d\n",
 		idx, mpc_ioapic_id(idx),
 		mpc_ioapic_ver(idx), mpc_ioapic_addr(idx),
 		gsi_cfg->gsi_base, gsi_cfg->gsi_end);
 
-	nr_ioapics++;
+	return 0;
 }
 
 int mp_irqdomain_map(struct irq_domain *domain, unsigned int virq,
-- 
1.7.10.4

[Patch v4 11/16] x86, irq, ACPI: Introduce a rwsem to protect IOAPIC operations from hotplug

[Jiang Liu]

We are going to support ACPI based IOAPIC hotplug, so introduce a rwsem
to protect IOAPIC data structures from IOAPIC hotplug. We choose to
serialize in ACPI instead of in the IOAPIC core because:
1) currently we are only plan to support ACPI based IOAPIC hotplug
2) it's much more cleaner and easier
3) It does't affect IOAPIC discovered by devicetree, SFI and mpparse.

Signed-off-by: Jiang Liu <jiang.liu@linux.intel.com>
---
 arch/x86/kernel/acpi/boot.c |   13 ++++++++++++-
 1 file changed, 12 insertions(+), 1 deletion(-)

diff --git a/arch/x86/kernel/acpi/boot.c b/arch/x86/kernel/acpi/boot.c
index b436fc735aa4..e23f7460c3f8 100644
--- a/arch/x86/kernel/acpi/boot.c
+++ b/arch/x86/kernel/acpi/boot.c
@@ -76,6 +76,8 @@ int acpi_fix_pin2_polarity __initdata;
 static u64 acpi_lapic_addr __initdata = APIC_DEFAULT_PHYS_BASE;
 #endif
 
+static DECLARE_RWSEM(acpi_ioapic_rwsem);
+
 /* --------------------------------------------------------------------------
                               Boot-time Configuration
    -------------------------------------------------------------------------- */
@@ -604,8 +606,11 @@ void __init acpi_pic_sci_set_trigger(unsigned int irq, u16 trigger)
 
 int acpi_gsi_to_irq(u32 gsi, unsigned int *irqp)
 {
-	int irq = mp_map_gsi_to_irq(gsi, IOAPIC_MAP_ALLOC | IOAPIC_MAP_CHECK);
+	int irq;
 
+	down_read(&acpi_ioapic_rwsem);
+	irq = mp_map_gsi_to_irq(gsi, IOAPIC_MAP_ALLOC | IOAPIC_MAP_CHECK);
+	up_read(&acpi_ioapic_rwsem);
 	if (irq >= 0) {
 		*irqp = irq;
 		return 0;
@@ -646,7 +651,9 @@ static int acpi_register_gsi_ioapic(struct device *dev, u32 gsi,
 	int irq = gsi;
 
 #ifdef CONFIG_X86_IO_APIC
+	down_read(&acpi_ioapic_rwsem);
 	irq = mp_register_gsi(dev, gsi, trigger, polarity);
+	up_read(&acpi_ioapic_rwsem);
 #endif
 
 	return irq;
@@ -655,7 +662,9 @@ static int acpi_register_gsi_ioapic(struct device *dev, u32 gsi,
 static void acpi_unregister_gsi_ioapic(u32 gsi)
 {
 #ifdef CONFIG_X86_IO_APIC
+	down_read(&acpi_ioapic_rwsem);
 	mp_unregister_gsi(gsi);
+	up_read(&acpi_ioapic_rwsem);
 #endif
 }
 
@@ -1181,7 +1190,9 @@ static void __init acpi_process_madt(void)
 			/*
 			 * Parse MADT IO-APIC entries
 			 */
+			down_write(&acpi_ioapic_rwsem);
 			error = acpi_parse_madt_ioapic_entries();
+			up_write(&acpi_ioapic_rwsem);
 			if (!error) {
 				acpi_set_irq_model_ioapic();
 
-- 
1.7.10.4

[Patch v4 12/16] x86, irq, ACPI: Implement interface to support ACPI based IOAPIC hot-addition

[Jiang Liu]

Implement acpi_register_ioapic() and enhance mp_register_ioapic()
to support ACPI based IOAPIC hot-addition.

Signed-off-by: Jiang Liu <jiang.liu@linux.intel.com>
---
 arch/x86/kernel/acpi/boot.c    |   31 +++++++++++++++++++++++++++++--
 arch/x86/kernel/apic/io_apic.c |   27 ++++++++++++++++++++++++---
 2 files changed, 53 insertions(+), 5 deletions(-)

diff --git a/arch/x86/kernel/acpi/boot.c b/arch/x86/kernel/acpi/boot.c
index e23f7460c3f8..796cd9e31ef3 100644
--- a/arch/x86/kernel/acpi/boot.c
+++ b/arch/x86/kernel/acpi/boot.c
@@ -764,8 +764,35 @@ EXPORT_SYMBOL(acpi_unmap_lsapic);
 
 int acpi_register_ioapic(acpi_handle handle, u64 phys_addr, u32 gsi_base)
 {
-	/* TBD */
-	return -EINVAL;
+	int ret = -ENOSYS;
+#ifdef CONFIG_ACPI_HOTPLUG_IOAPIC
+	int ioapic_id;
+	u64 addr;
+	struct ioapic_domain_cfg cfg = {
+		.type = IOAPIC_DOMAIN_DYNAMIC,
+		.ops = &acpi_irqdomain_ops,
+	};
+
+	ioapic_id = acpi_get_ioapic_id(handle, gsi_base, &addr);
+	if (ioapic_id < 0) {
+		unsigned long long uid;
+		acpi_status status;
+
+		status = acpi_evaluate_integer(handle, METHOD_NAME__UID,
+					       NULL, &uid);
+		if (ACPI_FAILURE(status)) {
+			acpi_handle_warn(handle, "failed to get IOAPIC ID.\n");
+			return -EINVAL;
+		}
+		ioapic_id = (int)uid;
+	}
+
+	down_write(&acpi_ioapic_rwsem);
+	ret  = mp_register_ioapic(ioapic_id, phys_addr, gsi_base, &cfg);
+	up_write(&acpi_ioapic_rwsem);
+#endif
+
+	return ret;
 }
 
 EXPORT_SYMBOL(acpi_register_ioapic);
diff --git a/arch/x86/kernel/apic/io_apic.c b/arch/x86/kernel/apic/io_apic.c
index 6e67af0c5f99..b286461cabf9 100644
--- a/arch/x86/kernel/apic/io_apic.c
+++ b/arch/x86/kernel/apic/io_apic.c
@@ -3851,7 +3851,13 @@ static int bad_ioapic_register(int idx)
 
 static int find_free_ioapic_entry(void)
 {
-	return nr_ioapics;
+	int idx;
+
+	for (idx = 0; idx < MAX_IO_APICS; idx++)
+		if (ioapics[idx].nr_registers == 0)
+			return idx;
+
+	return MAX_IO_APICS;
 }
 
 int mp_register_ioapic(int id, u32 address, u32 gsi_base,
@@ -3867,8 +3873,15 @@ int mp_register_ioapic(int id, u32 address, u32 gsi_base,
 	}
 	for_each_ioapic(ioapic)
 		if (ioapics[ioapic].mp_config.apicaddr == address) {
-			pr_warn("address 0x%x conflicts with IOAPIC%d\n",
-				address, ioapic);
+			/*
+			 * IOAPIC unit may also be visible in PCI scope.
+			 * When ioapic PCI driver's probe() is called,
+			 * the IOAPIC unit may have already been initialized
+			 * at boot time.
+			 */
+			if (!ioapic_initialized)
+				pr_warn("address 0x%x conflicts with IOAPIC%d\n",
+					address, ioapic);
 			return -EEXIST;
 		}
 
@@ -3918,6 +3931,14 @@ int mp_register_ioapic(int id, u32 address, u32 gsi_base,
 	ioapics[idx].irqdomain = NULL;
 	ioapics[idx].irqdomain_cfg = *cfg;
 
+	if (ioapic_initialized) {
+		if (mp_irqdomain_create(idx)) {
+			clear_fixmap(FIX_IO_APIC_BASE_0 + idx);
+			return -ENOMEM;
+		}
+		alloc_ioapic_saved_registers(idx);
+	}
+
 	if (gsi_cfg->gsi_end >= gsi_top)
 		gsi_top = gsi_cfg->gsi_end + 1;
 	if (nr_ioapics <= idx)
-- 
1.7.10.4

Re: [Patch v4 12/16] x86, irq, ACPI: Implement interface to support ACPI based IOAPIC hot-addition

[Thomas Gleixner]

On Thu, 28 Aug 2014, Jiang Liu wrote:
>  EXPORT_SYMBOL(acpi_register_ioapic);
> diff --git a/arch/x86/kernel/apic/io_apic.c b/arch/x86/kernel/apic/io_apic.c
> index 6e67af0c5f99..b286461cabf9 100644
> --- a/arch/x86/kernel/apic/io_apic.c
> +++ b/arch/x86/kernel/apic/io_apic.c
> @@ -3851,7 +3851,13 @@ static int bad_ioapic_register(int idx)
>  
>  static int find_free_ioapic_entry(void)
>  {
> -	return nr_ioapics;
> +	int idx;
> +
> +	for (idx = 0; idx < MAX_IO_APICS; idx++)
> +		if (ioapics[idx].nr_registers == 0)
> +			return idx;
> +
> +	return MAX_IO_APICS;
>  }
>  
>  int mp_register_ioapic(int id, u32 address, u32 gsi_base,
> @@ -3867,8 +3873,15 @@ int mp_register_ioapic(int id, u32 address, u32 gsi_base,
>  	}
>  	for_each_ioapic(ioapic)
>  		if (ioapics[ioapic].mp_config.apicaddr == address) {
> -			pr_warn("address 0x%x conflicts with IOAPIC%d\n",
> -				address, ioapic);
> +			/*
> +			 * IOAPIC unit may also be visible in PCI scope.
> +			 * When ioapic PCI driver's probe() is called,
> +			 * the IOAPIC unit may have already been initialized
> +			 * at boot time.
> +			 */
> +			if (!ioapic_initialized)
> +				pr_warn("address 0x%x conflicts with IOAPIC%d\n",
> +					address, ioapic);

Hmm. This smells fishy. Why do we allow multiple initializations of
the same IOAPIC in the first place. Either it's done via ACPI or via
PCI, but not both.

>  			return -EEXIST;
>  		}
>  
> @@ -3918,6 +3931,14 @@ int mp_register_ioapic(int id, u32 address, u32 gsi_base,
>  	ioapics[idx].irqdomain = NULL;
>  	ioapics[idx].irqdomain_cfg = *cfg;
>  
> +	if (ioapic_initialized) {

I have a hard time to understand this conditional. Why can't we do
that unconditionally?

> +		if (mp_irqdomain_create(idx)) {
> +			clear_fixmap(FIX_IO_APIC_BASE_0 + idx);
> +			return -ENOMEM;
> +		}
> +		alloc_ioapic_saved_registers(idx);
> +	}
> +
>  	if (gsi_cfg->gsi_end >= gsi_top)
>  		gsi_top = gsi_cfg->gsi_end + 1;
>  	if (nr_ioapics <= idx)

Thanks,

	tglx

Re: [Patch v4 12/16] x86, irq, ACPI: Implement interface to support ACPI based IOAPIC hot-addition

[Jiang Liu]

On 2014/9/9 20:20, Thomas Gleixner wrote:
> On Thu, 28 Aug 2014, Jiang Liu wrote:
>>  EXPORT_SYMBOL(acpi_register_ioapic);
>> diff --git a/arch/x86/kernel/apic/io_apic.c b/arch/x86/kernel/apic/io_apic.c
>> index 6e67af0c5f99..b286461cabf9 100644
>> --- a/arch/x86/kernel/apic/io_apic.c
>> +++ b/arch/x86/kernel/apic/io_apic.c
>> @@ -3851,7 +3851,13 @@ static int bad_ioapic_register(int idx)
>>  
>>  static int find_free_ioapic_entry(void)
>>  {
>> -	return nr_ioapics;
>> +	int idx;
>> +
>> +	for (idx = 0; idx < MAX_IO_APICS; idx++)
>> +		if (ioapics[idx].nr_registers == 0)
>> +			return idx;
>> +
>> +	return MAX_IO_APICS;
>>  }
>>  
>>  int mp_register_ioapic(int id, u32 address, u32 gsi_base,
>> @@ -3867,8 +3873,15 @@ int mp_register_ioapic(int id, u32 address, u32 gsi_base,
>>  	}
>>  	for_each_ioapic(ioapic)
>>  		if (ioapics[ioapic].mp_config.apicaddr == address) {
>> -			pr_warn("address 0x%x conflicts with IOAPIC%d\n",
>> -				address, ioapic);
>> +			/*
>> +			 * IOAPIC unit may also be visible in PCI scope.
>> +			 * When ioapic PCI driver's probe() is called,
>> +			 * the IOAPIC unit may have already been initialized
>> +			 * at boot time.
>> +			 */
>> +			if (!ioapic_initialized)
>> +				pr_warn("address 0x%x conflicts with IOAPIC%d\n",
>> +					address, ioapic);
> 
> Hmm. This smells fishy. Why do we allow multiple initializations of
> the same IOAPIC in the first place. Either it's done via ACPI or via
> PCI, but not both.
The ACPI subsystem will register and initialize all IOAPICs when walking
ACPI MADT table during boot, before initializing PCI subsystem.
Later when binding ioapic PCI driver to IOAPIC PCI device, it will try
to register the IOAPIC device again.

After this patchset is applied, we could remove the !ioapic_initialized
check. We check acpi_ioapic_register() before calling
acpi_register_ioapic(). So the check becomes redundant.
Or we could remove the temporary code from this patch.

> 
>>  			return -EEXIST;
>>  		}
>>  
>> @@ -3918,6 +3931,14 @@ int mp_register_ioapic(int id, u32 address, u32 gsi_base,
>>  	ioapics[idx].irqdomain = NULL;
>>  	ioapics[idx].irqdomain_cfg = *cfg;
>>  
>> +	if (ioapic_initialized) {
> 
> I have a hard time to understand this conditional. Why can't we do
> that unconditionally?
How about following comments?
/*
 * If mp_register_ioapic() is called during early boot stage when
 * walking ACPI/SFI/DT tables, it's too early to create irqdomain,
 * we are still using bootmem allocator. So delay it to setup_IO_APIC().
 */
Regards!
Gerry
> 
>> +		if (mp_irqdomain_create(idx)) {
>> +			clear_fixmap(FIX_IO_APIC_BASE_0 + idx);
>> +			return -ENOMEM;
>> +		}
>> +		alloc_ioapic_saved_registers(idx);
>> +	}
>> +
>>  	if (gsi_cfg->gsi_end >= gsi_top)
>>  		gsi_top = gsi_cfg->gsi_end + 1;
>>  	if (nr_ioapics <= idx)
> 
> Thanks,
> 
> 	tglx
> 

Re: [Patch v4 12/16] x86, irq, ACPI: Implement interface to support ACPI based IOAPIC hot-addition

[Thomas Gleixner]

On Wed, 10 Sep 2014, Jiang Liu wrote:
> >>  int mp_register_ioapic(int id, u32 address, u32 gsi_base,
> >> @@ -3867,8 +3873,15 @@ int mp_register_ioapic(int id, u32 address, u32 gsi_base,
> >>  	}
> >>  	for_each_ioapic(ioapic)
> >>  		if (ioapics[ioapic].mp_config.apicaddr == address) {
> >> -			pr_warn("address 0x%x conflicts with IOAPIC%d\n",
> >> -				address, ioapic);
> >> +			/*
> >> +			 * IOAPIC unit may also be visible in PCI scope.
> >> +			 * When ioapic PCI driver's probe() is called,
> >> +			 * the IOAPIC unit may have already been initialized
> >> +			 * at boot time.
> >> +			 */
> >> +			if (!ioapic_initialized)
> >> +				pr_warn("address 0x%x conflicts with IOAPIC%d\n",
> >> +					address, ioapic);
> > 
> > Hmm. This smells fishy. Why do we allow multiple initializations of
> > the same IOAPIC in the first place. Either it's done via ACPI or via
> > PCI, but not both.
> The ACPI subsystem will register and initialize all IOAPICs when walking
> ACPI MADT table during boot, before initializing PCI subsystem.
> Later when binding ioapic PCI driver to IOAPIC PCI device, it will try
> to register the IOAPIC device again.
> 
> After this patchset is applied, we could remove the !ioapic_initialized
> check. We check acpi_ioapic_register() before calling
> acpi_register_ioapic(). So the check becomes redundant.
> Or we could remove the temporary code from this patch.

How about removing the disfunctional ioapic PCI driver first and then
implementing the whole thing cleanly?
 
> > 
> >>  			return -EEXIST;
> >>  		}
> >>  
> >> @@ -3918,6 +3931,14 @@ int mp_register_ioapic(int id, u32 address, u32 gsi_base,
> >>  	ioapics[idx].irqdomain = NULL;
> >>  	ioapics[idx].irqdomain_cfg = *cfg;
> >>  
> >> +	if (ioapic_initialized) {
> > 
> > I have a hard time to understand this conditional. Why can't we do
> > that unconditionally?
> How about following comments?
> /*
>  * If mp_register_ioapic() is called during early boot stage when
>  * walking ACPI/SFI/DT tables, it's too early to create irqdomain,
>  * we are still using bootmem allocator. So delay it to setup_IO_APIC().
>  */

Fine, but then the "if (ioapic_initialized)" conditional still does
not make sense. We surely have some global non ioapic specific
indicator that bootmem is done and the proper memory allocator is
available, right?

Aside of that is there a point to walk those tables before we actually
can make any use of their content?

Thanks,

	tglx

Re: [Patch v4 12/16] x86, irq, ACPI: Implement interface to support ACPI based IOAPIC hot-addition

[Jiang Liu]

On 2014/9/11 4:06, Thomas Gleixner wrote:
> On Wed, 10 Sep 2014, Jiang Liu wrote:
>>>>  int mp_register_ioapic(int id, u32 address, u32 gsi_base,
>>>> @@ -3867,8 +3873,15 @@ int mp_register_ioapic(int id, u32 address, u32 gsi_base,
>>>>  	}
>>>>  	for_each_ioapic(ioapic)
>>>>  		if (ioapics[ioapic].mp_config.apicaddr == address) {
>>>> -			pr_warn("address 0x%x conflicts with IOAPIC%d\n",
>>>> -				address, ioapic);
>>>> +			/*
>>>> +			 * IOAPIC unit may also be visible in PCI scope.
>>>> +			 * When ioapic PCI driver's probe() is called,
>>>> +			 * the IOAPIC unit may have already been initialized
>>>> +			 * at boot time.
>>>> +			 */
>>>> +			if (!ioapic_initialized)
>>>> +				pr_warn("address 0x%x conflicts with IOAPIC%d\n",
>>>> +					address, ioapic);
>>>
>>> Hmm. This smells fishy. Why do we allow multiple initializations of
>>> the same IOAPIC in the first place. Either it's done via ACPI or via
>>> PCI, but not both.
>> The ACPI subsystem will register and initialize all IOAPICs when walking
>> ACPI MADT table during boot, before initializing PCI subsystem.
>> Later when binding ioapic PCI driver to IOAPIC PCI device, it will try
>> to register the IOAPIC device again.
>>
>> After this patchset is applied, we could remove the !ioapic_initialized
>> check. We check acpi_ioapic_register() before calling
>> acpi_register_ioapic(). So the check becomes redundant.
>> Or we could remove the temporary code from this patch.
> 
> How about removing the disfunctional ioapic PCI driver first and then
> implementing the whole thing cleanly?
Good suggestion:)

>  
>>>
>>>>  			return -EEXIST;
>>>>  		}
>>>>  
>>>> @@ -3918,6 +3931,14 @@ int mp_register_ioapic(int id, u32 address, u32 gsi_base,
>>>>  	ioapics[idx].irqdomain = NULL;
>>>>  	ioapics[idx].irqdomain_cfg = *cfg;
>>>>  
>>>> +	if (ioapic_initialized) {
>>>
>>> I have a hard time to understand this conditional. Why can't we do
>>> that unconditionally?
>> How about following comments?
>> /*
>>  * If mp_register_ioapic() is called during early boot stage when
>>  * walking ACPI/SFI/DT tables, it's too early to create irqdomain,
>>  * we are still using bootmem allocator. So delay it to setup_IO_APIC().
>>  */
> 
> Fine, but then the "if (ioapic_initialized)" conditional still does
> not make sense. We surely have some global non ioapic specific
> indicator that bootmem is done and the proper memory allocator is
> available, right?
Flag ioapic_initialized will be used to check whether we have created
irqdomains for IOAPICs. Currently function arch_dynirq_lower_bound()
uses that flag, and alloc_irq_from_domain() will use it too later.

> 
> Aside of that is there a point to walk those tables before we actually
> can make any use of their content?
At least we depend on walking those tables to detect whether system has
IOAPICs available:)

Regards!
Gerry
> 
> Thanks,
> 
> 	tglx
> 

Re: [Patch v4 12/16] x86, irq, ACPI: Implement interface to support ACPI based IOAPIC hot-addition

[Jiang Liu]

On 2014/9/11 4:06, Thomas Gleixner wrote:
> On Wed, 10 Sep 2014, Jiang Liu wrote:
>>>>  int mp_register_ioapic(int id, u32 address, u32 gsi_base,
>>>> @@ -3867,8 +3873,15 @@ int mp_register_ioapic(int id, u32 address, u32 gsi_base,
>>>>  	}
>>>>  	for_each_ioapic(ioapic)
>>>>  		if (ioapics[ioapic].mp_config.apicaddr == address) {
>>>> -			pr_warn("address 0x%x conflicts with IOAPIC%d\n",
>>>> -				address, ioapic);
>>>> +			/*
>>>> +			 * IOAPIC unit may also be visible in PCI scope.
>>>> +			 * When ioapic PCI driver's probe() is called,
>>>> +			 * the IOAPIC unit may have already been initialized
>>>> +			 * at boot time.
>>>> +			 */
>>>> +			if (!ioapic_initialized)
>>>> +				pr_warn("address 0x%x conflicts with IOAPIC%d\n",
>>>> +					address, ioapic);
>>>
>>> Hmm. This smells fishy. Why do we allow multiple initializations of
>>> the same IOAPIC in the first place. Either it's done via ACPI or via
>>> PCI, but not both.
>> The ACPI subsystem will register and initialize all IOAPICs when walking
>> ACPI MADT table during boot, before initializing PCI subsystem.
>> Later when binding ioapic PCI driver to IOAPIC PCI device, it will try
>> to register the IOAPIC device again.
>>
>> After this patchset is applied, we could remove the !ioapic_initialized
>> check. We check acpi_ioapic_register() before calling
>> acpi_register_ioapic(). So the check becomes redundant.
>> Or we could remove the temporary code from this patch.
> 
> How about removing the disfunctional ioapic PCI driver first and then
> implementing the whole thing cleanly?
>  
>>>
>>>>  			return -EEXIST;
>>>>  		}
>>>>  
>>>> @@ -3918,6 +3931,14 @@ int mp_register_ioapic(int id, u32 address, u32 gsi_base,
>>>>  	ioapics[idx].irqdomain = NULL;
>>>>  	ioapics[idx].irqdomain_cfg = *cfg;
>>>>  
>>>> +	if (ioapic_initialized) {
>>>
>>> I have a hard time to understand this conditional. Why can't we do
>>> that unconditionally?
>> How about following comments?
>> /*
>>  * If mp_register_ioapic() is called during early boot stage when
>>  * walking ACPI/SFI/DT tables, it's too early to create irqdomain,
>>  * we are still using bootmem allocator. So delay it to setup_IO_APIC().
>>  */
> 
> Fine, but then the "if (ioapic_initialized)" conditional still does
> not make sense. We surely have some global non ioapic specific
> indicator that bootmem is done and the proper memory allocator is
> available, right?
Maybe a good name helps here. How about
bool hotplug = !!ioapic_initialized;

if (hotplug)
	mp_irqdomain_create(idx);

Regards!
Gerry	

> 
> Aside of that is there a point to walk those tables before we actually
> can make any use of their content?
> 
> Thanks,
> 
> 	tglx
> 

[Patch v4 13/16] x86, irq, ACPI: Implement interfaces to support ACPI based IOAPIC hot-removal

[Jiang Liu]

Implement acpi_unregister_ioapic() to support ACPI based IOAPIC hot-removal.
An IOAPIC could only be removed when all its pins are unused.

Signed-off-by: Jiang Liu <jiang.liu@linux.intel.com>
---
 arch/x86/include/asm/io_apic.h |    1 +
 arch/x86/kernel/acpi/boot.c    |   13 +++++++---
 arch/x86/kernel/apic/io_apic.c |   55 ++++++++++++++++++++++++++++++++++++++++
 3 files changed, 65 insertions(+), 4 deletions(-)

diff --git a/arch/x86/include/asm/io_apic.h b/arch/x86/include/asm/io_apic.h
index 94d05bd6586f..ce63cf34c93c 100644
--- a/arch/x86/include/asm/io_apic.h
+++ b/arch/x86/include/asm/io_apic.h
@@ -190,6 +190,7 @@ extern int mp_map_gsi_to_irq(u32 gsi, unsigned int flags);
 extern void mp_unmap_irq(int irq);
 extern int mp_register_ioapic(int id, u32 address, u32 gsi_base,
 			      struct ioapic_domain_cfg *cfg);
+extern int mp_unregister_ioapic(u32 gsi_base);
 extern int mp_irqdomain_map(struct irq_domain *domain, unsigned int virq,
 			    irq_hw_number_t hwirq);
 extern void mp_irqdomain_unmap(struct irq_domain *domain, unsigned int virq);
diff --git a/arch/x86/kernel/acpi/boot.c b/arch/x86/kernel/acpi/boot.c
index 796cd9e31ef3..560a6d1cb0f4 100644
--- a/arch/x86/kernel/acpi/boot.c
+++ b/arch/x86/kernel/acpi/boot.c
@@ -794,15 +794,20 @@ int acpi_register_ioapic(acpi_handle handle, u64 phys_addr, u32 gsi_base)
 
 	return ret;
 }
-
 EXPORT_SYMBOL(acpi_register_ioapic);
 
 int acpi_unregister_ioapic(acpi_handle handle, u32 gsi_base)
 {
-	/* TBD */
-	return -EINVAL;
-}
+	int ret = -ENOSYS;
+
+#ifdef CONFIG_ACPI_HOTPLUG_IOAPIC
+	down_write(&acpi_ioapic_rwsem);
+	ret  = mp_unregister_ioapic(gsi_base);
+	up_write(&acpi_ioapic_rwsem);
+#endif
 
+	return ret;
+}
 EXPORT_SYMBOL(acpi_unregister_ioapic);
 
 static int __init acpi_parse_sbf(struct acpi_table_header *table)
diff --git a/arch/x86/kernel/apic/io_apic.c b/arch/x86/kernel/apic/io_apic.c
index b286461cabf9..fc1e1f9567bc 100644
--- a/arch/x86/kernel/apic/io_apic.c
+++ b/arch/x86/kernel/apic/io_apic.c
@@ -112,6 +112,7 @@ static struct ioapic {
 	struct ioapic_domain_cfg irqdomain_cfg;
 	struct irq_domain *irqdomain;
 	struct mp_pin_info *pin_info;
+	struct resource *iomem_res;
 } ioapics[MAX_IO_APICS];
 
 #define mpc_ioapic_ver(ioapic_idx)	ioapics[ioapic_idx].mp_config.apicver
@@ -250,6 +251,12 @@ static void alloc_ioapic_saved_registers(int idx)
 		pr_err("IOAPIC %d: suspend/resume impossible!\n", idx);
 }
 
+static void free_ioapic_saved_registers(int idx)
+{
+	kfree(ioapics[idx].saved_registers);
+	ioapics[idx].saved_registers = NULL;
+}
+
 int __init arch_early_irq_init(void)
 {
 	struct irq_cfg *cfg;
@@ -2972,6 +2979,16 @@ static int mp_irqdomain_create(int ioapic)
 	return 0;
 }
 
+static void ioapic_destroy_irqdomain(int idx)
+{
+	if (ioapics[idx].irqdomain) {
+		irq_domain_remove(ioapics[idx].irqdomain);
+		ioapics[idx].irqdomain = NULL;
+	}
+	kfree(ioapics[idx].pin_info);
+	ioapics[idx].pin_info = NULL;
+}
+
 void __init setup_IO_APIC(void)
 {
 	int ioapic;
@@ -3733,6 +3750,7 @@ static struct resource * __init ioapic_setup_resources(void)
 		snprintf(mem, IOAPIC_RESOURCE_NAME_SIZE, "IOAPIC %u", i);
 		mem += IOAPIC_RESOURCE_NAME_SIZE;
 		num++;
+		ioapics[i].iomem_res = res;
 	}
 
 	ioapic_resources = res;
@@ -3955,6 +3973,43 @@ int mp_register_ioapic(int id, u32 address, u32 gsi_base,
 	return 0;
 }
 
+int mp_unregister_ioapic(u32 gsi_base)
+{
+	int ioapic, pin;
+	int found = 0;
+	struct mp_pin_info *pin_info;
+
+	for_each_ioapic(ioapic)
+		if (ioapics[ioapic].gsi_config.gsi_base == gsi_base) {
+			found = 1;
+			break;
+		}
+	if (!found) {
+		pr_warn("can't find IOAPIC for GSI %d\n", gsi_base);
+		return -ENODEV;
+	}
+
+	for_each_pin(ioapic, pin) {
+		pin_info = mp_pin_info(ioapic, pin);
+		if (pin_info->count) {
+			pr_warn("pin%d on IOAPIC%d is still in use.\n",
+				pin, ioapic);
+			return -EBUSY;
+		}
+	}
+
+	/* Mark entry not present */
+	ioapics[ioapic].nr_registers  = 0;
+	ioapic_destroy_irqdomain(ioapic);
+	free_ioapic_saved_registers(ioapic);
+	if (ioapics[ioapic].iomem_res)
+		release_resource(ioapics[ioapic].iomem_res);
+	clear_fixmap(FIX_IO_APIC_BASE_0 + ioapic);
+	memset(&ioapics[ioapic], 0, sizeof(ioapics[ioapic]));
+
+	return 0;
+}
+
 int mp_irqdomain_map(struct irq_domain *domain, unsigned int virq,
 		     irq_hw_number_t hwirq)
 {
-- 
1.7.10.4

[Patch v4 14/16] x86, irq: Introduce helper to check whether an IOAPIC has been registered

[Jiang Liu]

Introduce acpi_ioapic_registered() to check whether an IOAPIC has already
been registered, it will be used when enabling IOAPIC hotplug.

Signed-off-by: Jiang Liu <jiang.liu@linux.intel.com>
---
 arch/x86/include/asm/io_apic.h |    1 +
 arch/x86/kernel/acpi/boot.c    |   11 +++++++++++
 arch/x86/kernel/apic/io_apic.c |   11 +++++++++++
 include/linux/acpi.h           |    1 +
 4 files changed, 24 insertions(+)

diff --git a/arch/x86/include/asm/io_apic.h b/arch/x86/include/asm/io_apic.h
index ce63cf34c93c..0db2b7037e4b 100644
--- a/arch/x86/include/asm/io_apic.h
+++ b/arch/x86/include/asm/io_apic.h
@@ -191,6 +191,7 @@ extern void mp_unmap_irq(int irq);
 extern int mp_register_ioapic(int id, u32 address, u32 gsi_base,
 			      struct ioapic_domain_cfg *cfg);
 extern int mp_unregister_ioapic(u32 gsi_base);
+extern int mp_ioapic_registered(u32 gsi_base);
 extern int mp_irqdomain_map(struct irq_domain *domain, unsigned int virq,
 			    irq_hw_number_t hwirq);
 extern void mp_irqdomain_unmap(struct irq_domain *domain, unsigned int virq);
diff --git a/arch/x86/kernel/acpi/boot.c b/arch/x86/kernel/acpi/boot.c
index 560a6d1cb0f4..6aa796f77b71 100644
--- a/arch/x86/kernel/acpi/boot.c
+++ b/arch/x86/kernel/acpi/boot.c
@@ -810,6 +810,17 @@ int acpi_unregister_ioapic(acpi_handle handle, u32 gsi_base)
 }
 EXPORT_SYMBOL(acpi_unregister_ioapic);
 
+int acpi_ioapic_registered(acpi_handle handle, u32 gsi_base)
+{
+	int ret;
+
+	down_write(&acpi_ioapic_rwsem);
+	ret  = mp_ioapic_registered(gsi_base);
+	up_write(&acpi_ioapic_rwsem);
+
+	return ret;
+}
+
 static int __init acpi_parse_sbf(struct acpi_table_header *table)
 {
 	struct acpi_table_boot *sb;
diff --git a/arch/x86/kernel/apic/io_apic.c b/arch/x86/kernel/apic/io_apic.c
index fc1e1f9567bc..e104993a2394 100644
--- a/arch/x86/kernel/apic/io_apic.c
+++ b/arch/x86/kernel/apic/io_apic.c
@@ -4010,6 +4010,17 @@ int mp_unregister_ioapic(u32 gsi_base)
 	return 0;
 }
 
+int mp_ioapic_registered(u32 gsi_base)
+{
+	int ioapic;
+
+	for_each_ioapic(ioapic)
+		if (ioapics[ioapic].gsi_config.gsi_base == gsi_base)
+			return 1;
+
+	return 0;
+}
+
 int mp_irqdomain_map(struct irq_domain *domain, unsigned int virq,
 		     irq_hw_number_t hwirq)
 {
diff --git a/include/linux/acpi.h b/include/linux/acpi.h
index b41c5aef5336..754d99d5f258 100644
--- a/include/linux/acpi.h
+++ b/include/linux/acpi.h
@@ -155,6 +155,7 @@ int acpi_get_ioapic_id(acpi_handle handle, u32 gsi_base, u64 *phys_addr);
 
 int acpi_register_ioapic(acpi_handle handle, u64 phys_addr, u32 gsi_base);
 int acpi_unregister_ioapic(acpi_handle handle, u32 gsi_base);
+int acpi_ioapic_registered(acpi_handle handle, u32 gsi_base);
 void acpi_irq_stats_init(void);
 extern u32 acpi_irq_handled;
 extern u32 acpi_irq_not_handled;
-- 
1.7.10.4

Re: [Patch v4 14/16] x86, irq: Introduce helper to check whether an IOAPIC has been registered

[Thomas Gleixner]

On Thu, 28 Aug 2014, Jiang Liu wrote:

> Introduce acpi_ioapic_registered() to check whether an IOAPIC has already
> been registered, it will be used when enabling IOAPIC hotplug.
> 
> Signed-off-by: Jiang Liu <jiang.liu@linux.intel.com>
> ---
>  arch/x86/include/asm/io_apic.h |    1 +
>  arch/x86/kernel/acpi/boot.c    |   11 +++++++++++
>  arch/x86/kernel/apic/io_apic.c |   11 +++++++++++
>  include/linux/acpi.h           |    1 +
>  4 files changed, 24 insertions(+)
> 
> diff --git a/arch/x86/include/asm/io_apic.h b/arch/x86/include/asm/io_apic.h
> index ce63cf34c93c..0db2b7037e4b 100644
> --- a/arch/x86/include/asm/io_apic.h
> +++ b/arch/x86/include/asm/io_apic.h
> @@ -191,6 +191,7 @@ extern void mp_unmap_irq(int irq);
>  extern int mp_register_ioapic(int id, u32 address, u32 gsi_base,
>  			      struct ioapic_domain_cfg *cfg);
>  extern int mp_unregister_ioapic(u32 gsi_base);
> +extern int mp_ioapic_registered(u32 gsi_base);
>  extern int mp_irqdomain_map(struct irq_domain *domain, unsigned int virq,
>  			    irq_hw_number_t hwirq);
>  extern void mp_irqdomain_unmap(struct irq_domain *domain, unsigned int virq);
> diff --git a/arch/x86/kernel/acpi/boot.c b/arch/x86/kernel/acpi/boot.c
> index 560a6d1cb0f4..6aa796f77b71 100644
> --- a/arch/x86/kernel/acpi/boot.c
> +++ b/arch/x86/kernel/acpi/boot.c
> @@ -810,6 +810,17 @@ int acpi_unregister_ioapic(acpi_handle handle, u32 gsi_base)
>  }
>  EXPORT_SYMBOL(acpi_unregister_ioapic);
>  
> +int acpi_ioapic_registered(acpi_handle handle, u32 gsi_base)
> +{
> +	int ret;
> +
> +	down_write(&acpi_ioapic_rwsem);

Why down_write? You are merily checking whether the thing is
registered already.

> +	ret  = mp_ioapic_registered(gsi_base);
> +	up_write(&acpi_ioapic_rwsem);
> +
> +	return ret;
> +}

So I assume that this is for a particular caller in the apci ioapic
hotplug code and that call site has its own serialization. Otherwise
the return value is not protected at all.

Please add a Docbook comment to that function, and document the
locking rules as thats pretty non obvious.

The such is missing in some other patches as well.

Thanks,

	tglx

Re: [Patch v4 14/16] x86, irq: Introduce helper to check whether an IOAPIC has been registered

[Jiang Liu]

On 2014/9/9 20:37, Thomas Gleixner wrote:
> On Thu, 28 Aug 2014, Jiang Liu wrote:
> 
>> Introduce acpi_ioapic_registered() to check whether an IOAPIC has already
>> been registered, it will be used when enabling IOAPIC hotplug.
>>
>> Signed-off-by: Jiang Liu <jiang.liu@linux.intel.com>
>> ---
>>  arch/x86/include/asm/io_apic.h |    1 +
>>  arch/x86/kernel/acpi/boot.c    |   11 +++++++++++
>>  arch/x86/kernel/apic/io_apic.c |   11 +++++++++++
>>  include/linux/acpi.h           |    1 +
>>  4 files changed, 24 insertions(+)
>>
>> diff --git a/arch/x86/include/asm/io_apic.h b/arch/x86/include/asm/io_apic.h
>> index ce63cf34c93c..0db2b7037e4b 100644
>> --- a/arch/x86/include/asm/io_apic.h
>> +++ b/arch/x86/include/asm/io_apic.h
>> @@ -191,6 +191,7 @@ extern void mp_unmap_irq(int irq);
>>  extern int mp_register_ioapic(int id, u32 address, u32 gsi_base,
>>  			      struct ioapic_domain_cfg *cfg);
>>  extern int mp_unregister_ioapic(u32 gsi_base);
>> +extern int mp_ioapic_registered(u32 gsi_base);
>>  extern int mp_irqdomain_map(struct irq_domain *domain, unsigned int virq,
>>  			    irq_hw_number_t hwirq);
>>  extern void mp_irqdomain_unmap(struct irq_domain *domain, unsigned int virq);
>> diff --git a/arch/x86/kernel/acpi/boot.c b/arch/x86/kernel/acpi/boot.c
>> index 560a6d1cb0f4..6aa796f77b71 100644
>> --- a/arch/x86/kernel/acpi/boot.c
>> +++ b/arch/x86/kernel/acpi/boot.c
>> @@ -810,6 +810,17 @@ int acpi_unregister_ioapic(acpi_handle handle, u32 gsi_base)
>>  }
>>  EXPORT_SYMBOL(acpi_unregister_ioapic);
>>  
>> +int acpi_ioapic_registered(acpi_handle handle, u32 gsi_base)
>> +{
>> +	int ret;
>> +
>> +	down_write(&acpi_ioapic_rwsem);
> 
> Why down_write? You are merily checking whether the thing is
> registered already.
Yeah, a down_read() should be enough:)

>> +	ret  = mp_ioapic_registered(gsi_base);
>> +	up_write(&acpi_ioapic_rwsem);
>> +
>> +	return ret;
>> +}
> 
> So I assume that this is for a particular caller in the apci ioapic
> hotplug code and that call site has its own serialization. Otherwise
> the return value is not protected at all.
> 
> Please add a Docbook comment to that function, and document the
> locking rules as thats pretty non obvious.
How about this comments about locking rules?
/*
 * Locks related to IOAPIC hotplug
 * Hotplug side:
 *      ->lock_device_hotplug() //device_hotplug_lock
 *              ->acpi_ioapic_rwsem
 *                      ->ioapic_lock
 * Interrupt mapping side:
 *      ->acpi_ioapic_rwsem
 *              ->ioapic_mutex
 *                      ->ioapic_lock
 */
Regards!
Gerry
> 
> The such is missing in some other patches as well.
> 
> Thanks,
> 
> 	tglx
> 

Re: [Patch v4 14/16] x86, irq: Introduce helper to check whether an IOAPIC has been registered

[Thomas Gleixner]

On Wed, 10 Sep 2014, Jiang Liu wrote:
> On 2014/9/9 20:37, Thomas Gleixner wrote:
> >> +	ret  = mp_ioapic_registered(gsi_base);
> >> +	up_write(&acpi_ioapic_rwsem);
> >> +
> >> +	return ret;
> >> +}
> > 
> > So I assume that this is for a particular caller in the apci ioapic
> > hotplug code and that call site has its own serialization. Otherwise
> > the return value is not protected at all.
> > 
> > Please add a Docbook comment to that function, and document the
> > locking rules as thats pretty non obvious.
> How about this comments about locking rules?
> /*
>  * Locks related to IOAPIC hotplug
>  * Hotplug side:
>  *      ->lock_device_hotplug() //device_hotplug_lock
>  *              ->acpi_ioapic_rwsem
>  *                      ->ioapic_lock
>  * Interrupt mapping side:
>  *      ->acpi_ioapic_rwsem
>  *              ->ioapic_mutex
>  *                      ->ioapic_lock
>  */

That looks asymetric. Why is the hotplug side not taking ioapic_mutex?

Thanks,

	tglx

Re: [Patch v4 14/16] x86, irq: Introduce helper to check whether an IOAPIC has been registered

[Jiang Liu]

On 2014/9/11 4:08, Thomas Gleixner wrote:
> On Wed, 10 Sep 2014, Jiang Liu wrote:
>> On 2014/9/9 20:37, Thomas Gleixner wrote:
>>>> +	ret  = mp_ioapic_registered(gsi_base);
>>>> +	up_write(&acpi_ioapic_rwsem);
>>>> +
>>>> +	return ret;
>>>> +}
>>>
>>> So I assume that this is for a particular caller in the apci ioapic
>>> hotplug code and that call site has its own serialization. Otherwise
>>> the return value is not protected at all.
>>>
>>> Please add a Docbook comment to that function, and document the
>>> locking rules as thats pretty non obvious.
>> How about this comments about locking rules?
>> /*
>>  * Locks related to IOAPIC hotplug
>>  * Hotplug side:
>>  *      ->lock_device_hotplug() //device_hotplug_lock
>>  *              ->acpi_ioapic_rwsem
>>  *                      ->ioapic_lock
>>  * Interrupt mapping side:
>>  *      ->acpi_ioapic_rwsem
>>  *              ->ioapic_mutex
>>  *                      ->ioapic_lock
>>  */
> 
> That looks asymetric. Why is the hotplug side not taking ioapic_mutex?
Hi Thomas,
	We decide to protect system from IOAPIC hotplug in the ACPI
layer. For ACPI enumerated IOAPICs, ioapic_mutex is redundant and
it's used to protect MPPARSE, SFI, OF enumerated IOAPICs.
Regards!
Gerry

> 
> Thanks,
> 
> 	tglx
> 

[Patch v4 15/16] PCI: Remove PCI ioapic driver

[Jiang Liu]

To support IOAPIC hotplug on x86 and IA64 platforms, OS needs to figure
out global interrupt source number(GSI) and IOAPIC enumeration ID
through ACPI interfaces. So BIOS must implement an ACPI IOAPIC device
with _GSB/_UID or _MAT method to support IOAPIC hotplug. OS also needs
to figure out base physical address to access IOAPIC registers. OS may
get the base physical address through PCI BARs if IOAPIC device is
visible in PCI domain, otherwise OS may get the address by ACPI _CRS
method if IOAPIC device is hidden from PCI domain by BIOS.

When adding a PCI subtree, we need to add IOAPIC devices before enabling
all other PCI devices because other PCI devices may use the IOAPIC to
allocate PCI interrupts.

So we plan to reimplement IOAPIC driver as an ACPI instead of PCI driver
due to:
1) hot-pluggable IOAPIC devices are always visible in ACPI domain,
   but may or may not be visible in PCI domain.
2) we could explicitly control the order between IOAPIC and other PCI
   devices.

We also have another choice to use a PCI driver to manage IOAPIC device
if it's visible in PCI domain and use an ACPI driver if it's only
visible in ACPI domain. But this solution is a little complex.

It shouldn't cause serious backward compatibility issues because:
1) IOAPIC hotplug is never supported on x86 yet because it hasn't
   implemented the required acpi_register_ioapic() and
   acpi_unregister_ioapic().
2) Currently only ACPI based IOAPIC hotplug is possible on x86 and
   IA64, we don't know other specifications and interfaces to support
   IOAPIC hotplug yet.
3) We will reimplement an ACPI IOAPICｔdriver support IOAPIC hotplug.

This change also helps to get rid of the false alarm on all current
Linux distributions:
[    6.952497] ioapic: probe of 0000:00:05.4 failed with error -22
[    6.959542] ioapic: probe of 0000:80:05.4 failed with error -22

Signed-off-by: Jiang Liu <jiang.liu@linux.intel.com>
---
 drivers/pci/Kconfig  |    7 ---
 drivers/pci/Makefile |    2 -
 drivers/pci/ioapic.c |  121 --------------------------------------------------
 3 files changed, 130 deletions(-)
 delete mode 100644 drivers/pci/ioapic.c

diff --git a/drivers/pci/Kconfig b/drivers/pci/Kconfig
index 893503fa1782..39866d18004e 100644
--- a/drivers/pci/Kconfig
+++ b/drivers/pci/Kconfig
@@ -104,13 +104,6 @@ config PCI_PASID
 
 	  If unsure, say N.
 
-config PCI_IOAPIC
-	bool "PCI IO-APIC hotplug support" if X86
-	depends on PCI
-	depends on ACPI
-	depends on X86_IO_APIC
-	default !X86
-
 config PCI_LABEL
 	def_bool y if (DMI || ACPI)
 	select NLS
diff --git a/drivers/pci/Makefile b/drivers/pci/Makefile
index e04fe2d9df3b..73e4af400a5a 100644
--- a/drivers/pci/Makefile
+++ b/drivers/pci/Makefile
@@ -13,8 +13,6 @@ obj-$(CONFIG_PCI_QUIRKS) += quirks.o
 # Build PCI Express stuff if needed
 obj-$(CONFIG_PCIEPORTBUS) += pcie/
 
-obj-$(CONFIG_PCI_IOAPIC) += ioapic.o
-
 # Build the PCI Hotplug drivers if we were asked to
 obj-$(CONFIG_HOTPLUG_PCI) += hotplug/
 ifdef CONFIG_HOTPLUG_PCI
diff --git a/drivers/pci/ioapic.c b/drivers/pci/ioapic.c
deleted file mode 100644
index f6219d36227f..000000000000
--- a/drivers/pci/ioapic.c
+++ /dev/null
@@ -1,121 +0,0 @@
-/*
- * IOAPIC/IOxAPIC/IOSAPIC driver
- *
- * Copyright (C) 2009 Fujitsu Limited.
- * (c) Copyright 2009 Hewlett-Packard Development Company, L.P.
- *
- * This program is free software; you can redistribute it and/or modify
- * it under the terms of the GNU General Public License version 2 as
- * published by the Free Software Foundation.
- */
-
-/*
- * This driver manages PCI I/O APICs added by hotplug after boot.  We try to
- * claim all I/O APIC PCI devices, but those present at boot were registered
- * when we parsed the ACPI MADT, so we'll fail when we try to re-register
- * them.
- */
-
-#include <linux/pci.h>
-#include <linux/module.h>
-#include <linux/acpi.h>
-#include <linux/slab.h>
-
-struct ioapic {
-	acpi_handle	handle;
-	u32		gsi_base;
-};
-
-static int ioapic_probe(struct pci_dev *dev, const struct pci_device_id *ent)
-{
-	acpi_handle handle;
-	acpi_status status;
-	unsigned long long gsb;
-	struct ioapic *ioapic;
-	int ret;
-	char *type;
-	struct resource *res;
-
-	handle = ACPI_HANDLE(&dev->dev);
-	if (!handle)
-		return -EINVAL;
-
-	status = acpi_evaluate_integer(handle, "_GSB", NULL, &gsb);
-	if (ACPI_FAILURE(status))
-		return -EINVAL;
-
-	/*
-	 * The previous code in acpiphp evaluated _MAT if _GSB failed, but
-	 * ACPI spec 4.0 sec 6.2.2 requires _GSB for hot-pluggable I/O APICs.
-	 */
-
-	ioapic = kzalloc(sizeof(*ioapic), GFP_KERNEL);
-	if (!ioapic)
-		return -ENOMEM;
-
-	ioapic->handle = handle;
-	ioapic->gsi_base = (u32) gsb;
-
-	if (dev->class == PCI_CLASS_SYSTEM_PIC_IOAPIC)
-		type = "IOAPIC";
-	else
-		type = "IOxAPIC";
-
-	ret = pci_enable_device(dev);
-	if (ret < 0)
-		goto exit_free;
-
-	pci_set_master(dev);
-
-	if (pci_request_region(dev, 0, type))
-		goto exit_disable;
-
-	res = &dev->resource[0];
-	if (acpi_register_ioapic(ioapic->handle, res->start, ioapic->gsi_base))
-		goto exit_release;
-
-	pci_set_drvdata(dev, ioapic);
-	dev_info(&dev->dev, "%s at %pR, GSI %u\n", type, res, ioapic->gsi_base);
-	return 0;
-
-exit_release:
-	pci_release_region(dev, 0);
-exit_disable:
-	pci_disable_device(dev);
-exit_free:
-	kfree(ioapic);
-	return -ENODEV;
-}
-
-static void ioapic_remove(struct pci_dev *dev)
-{
-	struct ioapic *ioapic = pci_get_drvdata(dev);
-
-	acpi_unregister_ioapic(ioapic->handle, ioapic->gsi_base);
-	pci_release_region(dev, 0);
-	pci_disable_device(dev);
-	kfree(ioapic);
-}
-
-
-static const struct pci_device_id ioapic_devices[] = {
-	{ PCI_DEVICE_CLASS(PCI_CLASS_SYSTEM_PIC_IOAPIC, ~0) },
-	{ PCI_DEVICE_CLASS(PCI_CLASS_SYSTEM_PIC_IOXAPIC, ~0) },
-	{ }
-};
-MODULE_DEVICE_TABLE(pci, ioapic_devices);
-
-static struct pci_driver ioapic_driver = {
-	.name		= "ioapic",
-	.id_table	= ioapic_devices,
-	.probe		= ioapic_probe,
-	.remove		= ioapic_remove,
-};
-
-static int __init ioapic_init(void)
-{
-	return pci_register_driver(&ioapic_driver);
-}
-module_init(ioapic_init);
-
-MODULE_LICENSE("GPL");
-- 
1.7.10.4

[Patch v4 16/16] x86, irq, ACPI: Implement ACPI driver to support IOAPIC hotplug

[Jiang Liu]

Enable support of IOAPIC hotplug by:
1) reintroducing ACPI based IOAPIC driver
2) enhance pci_root driver to hook hotplug events

The ACPI IOAPIC driver is always enabled if all of ACPI, PCI and IOAPIC
are enabled.

Signed-off-by: Jiang Liu <jiang.liu@linux.intel.com>
---
 drivers/acpi/Kconfig    |    6 ++
 drivers/acpi/Makefile   |    1 +
 drivers/acpi/internal.h |    7 ++
 drivers/acpi/ioapic.c   |  236 +++++++++++++++++++++++++++++++++++++++++++++++
 drivers/acpi/pci_root.c |    3 +
 5 files changed, 253 insertions(+)
 create mode 100644 drivers/acpi/ioapic.c

diff --git a/drivers/acpi/Kconfig b/drivers/acpi/Kconfig
index d0f3265fb85d..b7b87a1a2252 100644
--- a/drivers/acpi/Kconfig
+++ b/drivers/acpi/Kconfig
@@ -315,6 +315,12 @@ config ACPI_HOTPLUG_MEMORY
 	  To compile this driver as a module, choose M here:
 	  the module will be called acpi_memhotplug.
 
+config ACPI_HOTPLUG_IOAPIC
+	bool
+	depends on PCI
+	depends on X86_IO_APIC
+	default y
+
 config ACPI_SBS
 	tristate "Smart Battery System"
 	depends on X86
diff --git a/drivers/acpi/Makefile b/drivers/acpi/Makefile
index 03ddd03f2bcd..4189023fb777 100644
--- a/drivers/acpi/Makefile
+++ b/drivers/acpi/Makefile
@@ -68,6 +68,7 @@ obj-$(CONFIG_ACPI_PROCESSOR)	+= processor.o
 obj-y				+= container.o
 obj-$(CONFIG_ACPI_THERMAL)	+= thermal.o
 obj-y				+= acpi_memhotplug.o
+obj-$(CONFIG_ACPI_HOTPLUG_IOAPIC) += ioapic.o
 obj-$(CONFIG_ACPI_BATTERY)	+= battery.o
 obj-$(CONFIG_ACPI_SBS)		+= sbshc.o
 obj-$(CONFIG_ACPI_SBS)		+= sbs.o
diff --git a/drivers/acpi/internal.h b/drivers/acpi/internal.h
index 4c5cf77e7576..817ab2d5975c 100644
--- a/drivers/acpi/internal.h
+++ b/drivers/acpi/internal.h
@@ -34,6 +34,13 @@ void acpi_pnp_init(void);
 int acpi_sysfs_init(void);
 void acpi_container_init(void);
 void acpi_memory_hotplug_init(void);
+#ifdef	CONFIG_ACPI_HOTPLUG_IOAPIC
+int acpi_ioapic_add(struct acpi_pci_root *root);
+int acpi_ioapic_remove(struct acpi_pci_root *root);
+#else
+static inline int acpi_ioapic_add(struct acpi_pci_root *root) { return 0; }
+static inline int acpi_ioapic_remove(struct acpi_pci_root *root) { return 0; }
+#endif
 #ifdef CONFIG_ACPI_DOCK
 void register_dock_dependent_device(struct acpi_device *adev,
 				    acpi_handle dshandle);
diff --git a/drivers/acpi/ioapic.c b/drivers/acpi/ioapic.c
new file mode 100644
index 000000000000..66d87a61ee5c
--- /dev/null
+++ b/drivers/acpi/ioapic.c
@@ -0,0 +1,236 @@
+/*
+ * IOAPIC/IOxAPIC/IOSAPIC driver
+ *
+ * Copyright (C) 2009 Fujitsu Limited.
+ * (c) Copyright 2009 Hewlett-Packard Development Company, L.P.
+ *
+ * Copyright (C) 2014 Intel Corporation
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ *
+ * Based on original drivers/pci/ioapic.c
+ *	Yinghai Lu <yinghai@kernel.org>
+ *	Jiang Liu <jiang.liu@intel.com>
+ */
+
+/*
+ * This driver manages I/O APICs added by hotplug after boot.
+ * We try to claim all I/O APIC devices, but those present at boot were
+ * registered when we parsed the ACPI MADT, so we'll fail when we try to
+ * re-register them.
+ */
+
+#define pr_fmt(fmt) "ACPI : IOAPIC: " fmt
+
+#include <linux/slab.h>
+#include <linux/acpi.h>
+#include <linux/pci.h>
+#include <acpi/acpi.h>
+
+struct acpi_pci_ioapic {
+	acpi_handle	root_handle;
+	acpi_handle	handle;
+	u32		gsi_base;
+	struct resource	res;
+	struct pci_dev	*pdev;
+	struct list_head list;
+};
+
+static LIST_HEAD(ioapic_list);
+static DEFINE_MUTEX(ioapic_list_lock);
+
+static acpi_status setup_res(struct acpi_resource *acpi_res, void *data)
+{
+	struct resource *res = data;
+
+	memset(res, 0, sizeof(*res));
+	if (acpi_dev_resource_memory(acpi_res, res)) {
+		res->flags &= IORESOURCE_MEM;
+		if (res->flags)
+			return AE_OK;
+	} else if (acpi_dev_resource_address_space(acpi_res, res)) {
+		struct acpi_resource_address64 addr;
+
+		res->flags &= IORESOURCE_MEM;
+		if (res->flags &&
+		    ACPI_SUCCESS(acpi_resource_to_address64(acpi_res, &addr)) &&
+		    addr.info.mem.caching != ACPI_PREFETCHABLE_MEMORY) {
+			res->start += addr.translation_offset;
+			res->end += addr.translation_offset;
+			return AE_OK;
+		}
+	}
+	res->flags = 0;
+
+	return AE_OK;
+}
+
+static bool acpi_is_ioapic(acpi_handle handle, char **type)
+{
+	acpi_status status;
+	struct acpi_device_info *info;
+	char *hid = NULL;
+	bool match = false;
+
+	if (!acpi_has_method(handle, "_GSB"))
+		return false;
+
+	status = acpi_get_object_info(handle, &info);
+	if (ACPI_SUCCESS(status)) {
+		if (info->valid & ACPI_VALID_HID)
+			hid = info->hardware_id.string;
+		if (hid) {
+			if (strcmp(hid, "ACPI0009") == 0) {
+				*type = "IOxAPIC";
+				match = true;
+			} else if (strcmp(hid, "ACPI000A") == 0) {
+				*type = "IOAPIC";
+				match = true;
+			}
+		}
+		kfree(info);
+	}
+
+	return match;
+}
+
+static acpi_status handle_ioapic_add(acpi_handle handle, u32 lvl,
+				     void *context, void **rv)
+{
+	acpi_status status;
+	unsigned long long gsi_base;
+	struct acpi_pci_ioapic *ioapic;
+	struct pci_dev *dev = NULL;
+	struct resource *res = NULL;
+	char *type = NULL;
+
+	if (!acpi_is_ioapic(handle, &type))
+		return AE_OK;
+
+	mutex_lock(&ioapic_list_lock);
+	list_for_each_entry(ioapic, &ioapic_list, list)
+		if (ioapic->handle == handle) {
+			mutex_unlock(&ioapic_list_lock);
+			return AE_OK;
+		}
+
+	status = acpi_evaluate_integer(handle, "_GSB", NULL, &gsi_base);
+	if (ACPI_FAILURE(status)) {
+		acpi_handle_warn(handle, "failed to evaluate _GSB method\n");
+		goto exit;
+	}
+
+	ioapic = kzalloc(sizeof(*ioapic), GFP_KERNEL);
+	if (!ioapic) {
+		pr_err("cannot allocate memory for new IOAPIC\n");
+		goto exit;
+	} else {
+		ioapic->root_handle = (acpi_handle)context;
+		ioapic->handle = handle;
+		ioapic->gsi_base = (u32)gsi_base;
+		ioapic->res.flags = IORESOURCE_UNSET;
+	}
+
+	if (acpi_ioapic_registered(handle, (u32)gsi_base))
+		goto done;
+
+	dev = acpi_get_pci_dev(handle);
+	if (dev && pci_resource_len(dev, 0)) {
+		if (pci_enable_device(dev) < 0)
+			goto exit_put;
+		pci_set_master(dev);
+		if (pci_request_region(dev, 0, type))
+			goto exit_disable;
+		res = &dev->resource[0];
+		ioapic->pdev = dev;
+	} else {
+		pci_dev_put(dev);
+		dev = NULL;
+
+		res = &ioapic->res;
+		acpi_walk_resources(handle, METHOD_NAME__CRS, setup_res, res);
+		if (res->flags == IORESOURCE_UNSET) {
+			acpi_handle_warn(handle, "failed to get resource\n");
+			goto exit_free;
+		} else if (request_resource(&iomem_resource, res)) {
+			acpi_handle_warn(handle, "failed to insert resource\n");
+			goto exit_free;
+		}
+	}
+
+	if (acpi_register_ioapic(handle, res->start, (u32)gsi_base)) {
+		acpi_handle_warn(handle, "failed to register IOAPIC\n");
+		goto exit_release;
+	}
+done:
+	list_add(&ioapic->list, &ioapic_list);
+	mutex_unlock(&ioapic_list_lock);
+
+	if (dev)
+		dev_info(&dev->dev, "%s at %pR, GSI %u\n",
+			 type, res, (u32)gsi_base);
+	else
+		acpi_handle_info(handle, "%s at %pR, GSI %u\n",
+				 type, res, (u32)gsi_base);
+
+	return AE_OK;
+
+exit_release:
+	if (dev)
+		pci_release_region(dev, 0);
+	else
+		release_resource(res);
+exit_disable:
+	if (dev)
+		pci_disable_device(dev);
+exit_put:
+	if (dev)
+		pci_dev_put(dev);
+exit_free:
+	kfree(ioapic);
+exit:
+	mutex_unlock(&ioapic_list_lock);
+	*(acpi_status *)rv = AE_ERROR;
+	return AE_OK;
+}
+
+int acpi_ioapic_add(struct acpi_pci_root *root)
+{
+	acpi_status status, retval = AE_OK;
+
+	status = acpi_walk_namespace(ACPI_TYPE_DEVICE, root->device->handle,
+				     UINT_MAX, handle_ioapic_add, NULL,
+				     root->device->handle, (void **)&retval);
+
+	return ACPI_SUCCESS(status) && ACPI_SUCCESS(retval) ? 0 : -ENODEV;
+}
+
+int acpi_ioapic_remove(struct acpi_pci_root *root)
+{
+	int retval = 0;
+	struct acpi_pci_ioapic *ioapic, *tmp;
+
+	mutex_lock(&ioapic_list_lock);
+	list_for_each_entry_safe(ioapic, tmp, &ioapic_list, list) {
+		if (root->device->handle != ioapic->root_handle)
+			continue;
+
+		if (acpi_unregister_ioapic(ioapic->handle, ioapic->gsi_base))
+			retval = -EBUSY;
+
+		if (ioapic->pdev) {
+			pci_release_region(ioapic->pdev, 0);
+			pci_disable_device(ioapic->pdev);
+			pci_dev_put(ioapic->pdev);
+		} else if (ioapic->res.flags != IORESOURCE_UNSET) {
+			release_resource(&ioapic->res);
+		}
+		list_del(&ioapic->list);
+		kfree(ioapic);
+	}
+	mutex_unlock(&ioapic_list_lock);
+
+	return retval;
+}
diff --git a/drivers/acpi/pci_root.c b/drivers/acpi/pci_root.c
index e6ae603ed1a1..d83980b18f49 100644
--- a/drivers/acpi/pci_root.c
+++ b/drivers/acpi/pci_root.c
@@ -600,6 +600,7 @@ static int acpi_pci_root_add(struct acpi_device *device,
 	if (system_state != SYSTEM_BOOTING) {
 		pcibios_resource_survey_bus(root->bus);
 		pci_assign_unassigned_root_bus_resources(root->bus);
+		acpi_ioapic_add(root);
 	}
 
 	pci_lock_rescan_remove();
@@ -620,6 +621,8 @@ static void acpi_pci_root_remove(struct acpi_device *device)
 
 	pci_stop_root_bus(root->bus);
 
+	WARN_ON(acpi_ioapic_remove(root));
+
 	device_set_run_wake(root->bus->bridge, false);
 	pci_acpi_remove_bus_pm_notifier(device);
 
-- 
1.7.10.4

Re: [Patch v4 00/16] Enable support of IOAPIC hotplug on x86 platforms

[Rafael J. Wysocki]

On Thursday, August 28, 2014 10:22:25 AM Jiang Liu wrote:
> This patch set enhances IOAPIC core and ACPI drivers to support IOAPIC
> hotplug on x86 platforms. It's based on latest mainstream kernel at
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
> 
> You may pull it from 
> https://github.com/jiangliu/linux.git ioapic/hotplug_v4
> 
> We have pick up several patches from Yinghai's original IOAPIC hotplug
> patch set and reimplemented IOAPIC driver as an ACPI driver instead of
> a PCI driver.
> 
> It has been tested on a 4-socket Intel SDV with socket hot-addition
> capability. Any suggestions are welcomed!
> 
> Patch 1-5 are bugfixes and enhancements to ACPI subsystem
> Patch 6-14 enhances IOAPIC core to support IOAPIC hotplug
> Patch 15 killes PCI IOAPIC driver
> Patch 16 reimplements ACPI IOAPIC driver and enables IOAPIC hotplug
> 
> V3->V4:
> 1) Fix a bug in manage IOAPIC reference count
> 2) Rebase to v3.17-rc2
> 3) Refine commit messages
> V2->V3:
> 1) Refine ACPI resource walk functions for PCI root bus and IOAPIC
> 2) Improve commit messages
> 3) Reorder patch order for better maintenence
> 
> Jiang Liu (13):
>   x86, PCI, ACPI: Kill private function resource_to_addr() in
>     arch/x86/pci/acpi.c
>   ACPI: Correct return value of acpi_dev_resource_address_space()
>   ACPI: Fix minor syntax issues in processor_core.c
>   ACPI: Rename processor_core.c as apic_id.c
>   x86, irq: Remove __init marker for functions will be used by IOAPIC
>     hotplug
>   x86, irq: Keep balance of IOAPIC pin reference count
>   x86, irq: Refine mp_register_ioapic() to prepare for IOAPIC hotplug
>   x86, irq, ACPI: Introduce a rwsem to protect IOAPIC operations from
>     hotplug
>   x86, irq, ACPI: Implement interface to support ACPI based IOAPIC
>     hot-addition
>   x86, irq, ACPI: Implement interfaces to support ACPI based IOAPIC
>     hot-removal
>   x86, irq: Introduce helper to check whether an IOAPIC has been
>     registered
>   PCI: Remove PCI ioapic driver
>   x86, irq, ACPI: Implement ACPI driver to support IOAPIC hotplug
> 
> Yinghai Lu (3):
>   ACPI: Add interfaces to parse IOAPIC ID for IOAPIC hotplug
>   x86, irq: Split out alloc_ioapic_save_registers()
>   x86, irq: Prefer assigned ID in APIC ID register for x86_64

I don't see any major problems in this patchset, but I'd prefer it to
go through the tip tree if possible.

-- 
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.