[PATCH v4 00/28] x86, irq: Support ioapic controller hotplug

[PATCH v4 00/28] x86, irq: Support ioapic controller hotplug

[Yinghai Lu]

Hi,

Intel cpu (from IVB) include cpu cores, mem controller, and IIO.
When hotplug cpu physically, it will involve cpu hotplug, mem hotplug,
and pci root hotplug.

IIO includes pci host bridge, ioapic controller and iommu etc.
pci devices will need to use ioapic and iommu.
So to make pci root bus hotplug working, we need ioapic hotplug support.

Now ioapics are with ACPI MADT table. During booting, kernel will parse
MADT and put info into ioapic arrays.
Also Bjorn added one pci device based driver, but it is not wired in yet,
as it need to call acpi_register_ioapic, and that is TBD.

This patchset will
1. extend genirq to support reserve/alloc_reserved method.
   because we want irqs for one ioapic controller to be linear mapping
   to the gsi range.
2. change ioapic array operation code so we could insert new ioapic and
   remove one leave the blank slot.
3. record irq_base in gsi_config in ioapic struct, and use it to convert gsi
   to irq for pci device using that ioapic controller
4. make static ioapic path (from MADT) use same code as hot-add path,
   with reserve/alloc_reserved method.
5. change ioapic add/remove to use acpi way, as that is only needed when
   pci root bus hotplug.
6. also support the case that ioapic controller is hiding in pci
   config space or ioapic address is not managed by pci reallocation subsys.

Also include some cleanups
1. print MSI-X clearly in /proc/interrupts and dmesg.
2. convert ioapic irq_2_pin to be generic list.

It is based on linus's tree of 2013/08/10 aka v3.11-rc4.

could get them from:
        git://git.kernel.org/pub/scm/linux/kernel/git/yinghai/linux-yinghai.git for-x86-irq

-v3: split last patch to 4 patches per request from Bjorn.
     drop one patch that will put pci device name in proc/interrupts
     update changelog.
-v4: address review from tglx, change to realloc to alloc_reserved.
     separate x86 code and core code into different patches.

Yinghai Lu (28):
  genirq: Split __irq_reserve_irqs from irq_alloc_descs
  genirq: Add irq_alloc_reserved_desc()
  genirq: Do not free unallocated irq descriptors
  x86, irq: Change irq_remap_modify_chip_defaults() to static
  x86, irq: Modify irq chip once for irq remapping
  x86, irq: Show MSI-X clearly in debug message
  x86, irq: Show MSI-X in /proc/interrupt
  x86, irq: Make dmar_msi/hpet_msi irq_chip name consistent
  ia64, irq: Add dummy create_irq_nr()
  iommu, irq: Allocate irq_desc for dmar_msi with local node
  x86, irq: Kill create_irq()
  x86, irq: Convert irq_2_pin list to generic list
  x86, irq: Add alloc_reserved_irq_and_cfg_at()
  x86, irq: Move down arch_early_irq_init()
  x86, irq: Split out alloc_ioapic_save_registers()
  xen, irq: Call irq_alloc_reserved_desc_at() at first
  x86, irq: Reserve irq range and alloc_reserved for booting path
  x86, irq: Add ioapic_gsi_to_irq
  x86, irq: Add for_each_ioapic helper
  x86, irq: More strict checking about registering ioapic
  x86, irq: Make mp_register_ioapic handle hot-added ioapic
  x86, irq: Add mp_unregister_ioapic to handle hot-remove ioapic
  x86, ioapic: Find usable ioapic id for 64bit.
  x86: Move declaration for mp_register_ioapic()
  PCI, x86: Make ioapic hotplug support built-in
  PCI, x86, ACPI: Link acpi ioapic register to ioapic
  PCI, x86, ACPI: Enable ioapic hotplug support with acpi host bridge.
  PCI, x86, ACPI: get ioapic address from acpi device

 arch/ia64/kernel/irq_ia64.c          |  10 +
 arch/x86/include/asm/hw_irq.h        |   2 +-
 arch/x86/include/asm/io_apic.h       |  13 +
 arch/x86/include/asm/irq_remapping.h |  14 +-
 arch/x86/include/asm/mpspec.h        |  21 +-
 arch/x86/kernel/acpi/boot.c          |  32 +--
 arch/x86/kernel/apic/apic.c          |   9 +-
 arch/x86/kernel/apic/io_apic.c       | 480 +++++++++++++++++++++++++----------
 arch/x86/kernel/devicetree.c         |   2 +-
 drivers/acpi/pci_root.c              |   4 +
 drivers/iommu/amd_iommu_init.c       |   2 +-
 drivers/iommu/dmar.c                 |   2 +-
 drivers/iommu/intel_irq_remapping.c  |   2 +-
 drivers/iommu/irq_remapping.c        |  18 +-
 drivers/pci/Kconfig                  |   3 +-
 drivers/pci/ioapic.c                 | 229 ++++++++++++-----
 drivers/xen/events.c                 |   8 +-
 include/linux/irq.h                  |   4 +
 include/linux/pci-acpi.h             |   8 +
 kernel/irq/irqdesc.c                 |  80 ++++--
 20 files changed, 679 insertions(+), 264 deletions(-)

-- 
1.8.1.4

[PATCH v4 01/28] genirq: Split __irq_reserve_irqs from irq_alloc_descs

[Yinghai Lu]

irq_alloc_descs and irq_reserve_irqs are almost the same.
Separate code out to __irq_reserved_irqs, and other two reuse
__irq_reserve_irqs.

We will use __irq_reserve_irqs for coming ioapic hotplug support.

-v2: fix func docbook related comments, pointed out by tglx.

Signed-off-by: Yinghai Lu <yinghai@kernel.org>
Cc: Alexander Gordeev <agordeev@redhat.com>
Reviewed-by: Alexander Gordeev <agordeev@redhat.com>
---
 include/linux/irq.h  |  1 +
 kernel/irq/irqdesc.c | 54 ++++++++++++++++++++++++++++++----------------------
 2 files changed, 32 insertions(+), 23 deletions(-)

diff --git a/include/linux/irq.h b/include/linux/irq.h
index f04d3ba..8c46cb2 100644
--- a/include/linux/irq.h
+++ b/include/linux/irq.h
@@ -611,6 +611,7 @@ int __irq_alloc_descs(int irq, unsigned int from, unsigned int cnt, int node,
 
 void irq_free_descs(unsigned int irq, unsigned int cnt);
 int irq_reserve_irqs(unsigned int from, unsigned int cnt);
+int __irq_reserve_irqs(int irq, unsigned int from, unsigned int cnt);
 
 static inline void irq_free_desc(unsigned int irq)
 {
diff --git a/kernel/irq/irqdesc.c b/kernel/irq/irqdesc.c
index 192a302..a151db6 100644
--- a/kernel/irq/irqdesc.c
+++ b/kernel/irq/irqdesc.c
@@ -340,18 +340,15 @@ void irq_free_descs(unsigned int from, unsigned int cnt)
 EXPORT_SYMBOL_GPL(irq_free_descs);
 
 /**
- * irq_alloc_descs - allocate and initialize a range of irq descriptors
- * @irq:	Allocate for specific irq number if irq >= 0
+ * __irq_reserve_irqs - reserve a range of irqs
+ * @irq:	Reserve for specific irq number if irq >= 0
  * @from:	Start the search from this irq number
- * @cnt:	Number of consecutive irqs to allocate.
- * @node:	Preferred node on which the irq descriptor should be allocated
- * @owner:	Owning module (can be NULL)
+ * @cnt:	Number of consecutive irqs to reserve.
  *
  * Returns the first irq number or error code
  */
 int __ref
-__irq_alloc_descs(int irq, unsigned int from, unsigned int cnt, int node,
-		  struct module *owner)
+__irq_reserve_irqs(int irq, unsigned int from, unsigned int cnt)
 {
 	int start, ret;
 
@@ -369,7 +366,7 @@ __irq_alloc_descs(int irq, unsigned int from, unsigned int cnt, int node,
 	start = bitmap_find_next_zero_area(allocated_irqs, IRQ_BITMAP_BITS,
 					   from, cnt, 0);
 	ret = -EEXIST;
-	if (irq >=0 && start != irq)
+	if (irq >= 0 && start != irq)
 		goto err;
 
 	if (start + cnt > nr_irqs) {
@@ -380,12 +377,36 @@ __irq_alloc_descs(int irq, unsigned int from, unsigned int cnt, int node,
 
 	bitmap_set(allocated_irqs, start, cnt);
 	mutex_unlock(&sparse_irq_lock);
-	return alloc_descs(start, cnt, node, owner);
+	return start;
 
 err:
 	mutex_unlock(&sparse_irq_lock);
 	return ret;
 }
+
+/**
+ * __irq_alloc_descs - allocate and initialize a range of irq descriptors
+ * @irq:	Allocate for specific irq number if irq >= 0
+ * @from:	Start the search from this irq number
+ * @cnt:	Number of consecutive irqs to allocate.
+ * @node:	Preferred node on which the irq descriptor should be allocated
+ * @owner:	Owning module (can be NULL)
+ *
+ * Returns the first irq number or error code
+ */
+int __ref
+__irq_alloc_descs(int irq, unsigned int from, unsigned int cnt, int node,
+		  struct module *owner)
+{
+	int start;
+
+	start = __irq_reserve_irqs(irq, from, cnt);
+
+	if (start < 0)
+		return start;
+
+	return alloc_descs(start, cnt, node, owner);
+}
 EXPORT_SYMBOL_GPL(__irq_alloc_descs);
 
 /**
@@ -397,20 +418,7 @@ EXPORT_SYMBOL_GPL(__irq_alloc_descs);
  */
 int irq_reserve_irqs(unsigned int from, unsigned int cnt)
 {
-	unsigned int start;
-	int ret = 0;
-
-	if (!cnt || (from + cnt) > nr_irqs)
-		return -EINVAL;
-
-	mutex_lock(&sparse_irq_lock);
-	start = bitmap_find_next_zero_area(allocated_irqs, nr_irqs, from, cnt, 0);
-	if (start == from)
-		bitmap_set(allocated_irqs, start, cnt);
-	else
-		ret = -EEXIST;
-	mutex_unlock(&sparse_irq_lock);
-	return ret;
+	return __irq_reserve_irqs(from, from, cnt);
 }
 
 /**
-- 
1.8.1.4

[PATCH v4 02/28] genirq: Add irq_alloc_reserved_desc()

[Yinghai Lu]

For ioapic hot-add support, it would be easy if we have continuous
irq numbers for hot added ioapic controller.

We can reserve irq range at first, and later allocate desc for those
pre-reserved irqs when they are needed.

The reasons for not allocating them during reserving:
1. only several pins of one ioapic are used, allocate for all pins, will
   waste memory for not used pins.
2. allocate later when is needed could make sure irq_desc is allocated
   on local node ram, as dev->node is set at that point.

-v2: update changelog by adding reasons, requested by Konrad.
-v3: according to tglx:
       separate core code change with arch code change.
       change function name to irq_alloc_reserved_desc.
       kill __irq_is_reserved().
       remove not need exports.
     according to Sebastian:
       spare one comments by put two functions together.

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>
---
 include/linux/irq.h  |  3 +++
 kernel/irq/irqdesc.c | 23 +++++++++++++++++++++++
 2 files changed, 26 insertions(+)

diff --git a/include/linux/irq.h b/include/linux/irq.h
index 8c46cb2..b998ea7 100644
--- a/include/linux/irq.h
+++ b/include/linux/irq.h
@@ -592,10 +592,13 @@ static inline u32 irq_get_trigger_type(unsigned int irq)
 
 int __irq_alloc_descs(int irq, unsigned int from, unsigned int cnt, int node,
 		struct module *owner);
+int __irq_alloc_reserved_desc(int at, int node, struct module *owner);
 
 /* use macros to avoid needing export.h for THIS_MODULE */
 #define irq_alloc_descs(irq, from, cnt, node)	\
 	__irq_alloc_descs(irq, from, cnt, node, THIS_MODULE)
+#define irq_alloc_reserved_desc_at(at, node)	\
+	__irq_alloc_reserved_desc(at, node, THIS_MODULE)
 
 #define irq_alloc_desc(node)			\
 	irq_alloc_descs(-1, 0, 1, node)
diff --git a/kernel/irq/irqdesc.c b/kernel/irq/irqdesc.c
index a151db6..1166545 100644
--- a/kernel/irq/irqdesc.c
+++ b/kernel/irq/irqdesc.c
@@ -410,6 +410,29 @@ __irq_alloc_descs(int irq, unsigned int from, unsigned int cnt, int node,
 EXPORT_SYMBOL_GPL(__irq_alloc_descs);
 
 /**
+ * __irq_alloc_reserved_desc - allocate irq descriptor for irq that is already reserved
+ * @irq:	Allocate for specific irq number if irq >= 0
+ * @node:	Preferred node on which the irq descriptor should be allocated
+ * @owner:	Owning module (can be NULL)
+ *
+ * Returns the irq number or error code
+ */
+int __ref __irq_alloc_reserved_desc(int irq, int node, struct module *owner)
+{
+	mutex_lock(&sparse_irq_lock);
+	if (!test_bit(irq, allocated_irqs)) {
+		mutex_unlock(&sparse_irq_lock);
+		return -EINVAL;
+	}
+	mutex_unlock(&sparse_irq_lock);
+
+	if (irq_to_desc(irq))
+		return irq;
+
+	return alloc_descs(irq, 1, node, owner);
+}
+
+/**
  * irq_reserve_irqs - mark irqs allocated
  * @from:	mark from irq number
  * @cnt:	number of irqs to mark
-- 
1.8.1.4

[PATCH v4 03/28] genirq: Do not free unallocated irq descriptors

[Yinghai Lu]

Hot-added interrupt controllers can reserve a range of interrupt
numbers, but only allocate some of them. To simplify the release on
hot-remove allow them to iterate over the reserved range, let the
free_desc() code return early when the descriptor does not exist

-v2: changelog from tglx

Signed-off-by: Yinghai Lu <yinghai@kernel.org>
---
 kernel/irq/irqdesc.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/kernel/irq/irqdesc.c b/kernel/irq/irqdesc.c
index 1166545..bbf0601 100644
--- a/kernel/irq/irqdesc.c
+++ b/kernel/irq/irqdesc.c
@@ -165,6 +165,9 @@ static void free_desc(unsigned int irq)
 {
 	struct irq_desc *desc = irq_to_desc(irq);
 
+	if (!desc)
+		return;
+
 	unregister_irq_proc(irq, desc);
 
 	mutex_lock(&sparse_irq_lock);
-- 
1.8.1.4

[PATCH v4 04/28] x86, irq: Change irq_remap_modify_chip_defaults() to static

[Yinghai Lu]

Change irq_remap_modify_chip_defaults() to static, as we have no
outside user.

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>
---
 arch/x86/include/asm/irq_remapping.h | 6 ------
 drivers/iommu/irq_remapping.c        | 2 +-
 2 files changed, 1 insertion(+), 7 deletions(-)

diff --git a/arch/x86/include/asm/irq_remapping.h b/arch/x86/include/asm/irq_remapping.h
index d806b22..606f42e 100644
--- a/arch/x86/include/asm/irq_remapping.h
+++ b/arch/x86/include/asm/irq_remapping.h
@@ -56,8 +56,6 @@ extern bool setup_remapped_irq(int irq,
 			       struct irq_cfg *cfg,
 			       struct irq_chip *chip);
 
-void irq_remap_modify_chip_defaults(struct irq_chip *chip);
-
 #else  /* CONFIG_IRQ_REMAP */
 
 static inline void setup_irq_remapping_ops(void) { }
@@ -91,10 +89,6 @@ static inline void panic_if_irq_remap(const char *msg)
 {
 }
 
-static inline void irq_remap_modify_chip_defaults(struct irq_chip *chip)
-{
-}
-
 static inline bool setup_remapped_irq(int irq,
 				      struct irq_cfg *cfg,
 				      struct irq_chip *chip)
diff --git a/drivers/iommu/irq_remapping.c b/drivers/iommu/irq_remapping.c
index 39f81ae..85ae2cf 100644
--- a/drivers/iommu/irq_remapping.c
+++ b/drivers/iommu/irq_remapping.c
@@ -375,7 +375,7 @@ static void ir_print_prefix(struct irq_data *data, struct seq_file *p)
 	seq_printf(p, " IR-%s", data->chip->name);
 }
 
-void irq_remap_modify_chip_defaults(struct irq_chip *chip)
+static void irq_remap_modify_chip_defaults(struct irq_chip *chip)
 {
 	chip->irq_print_chip = ir_print_prefix;
 	chip->irq_ack = ir_ack_apic_edge;
-- 
1.8.1.4

[PATCH v4 05/28] x86, irq: Modify irq chip once for irq remapping

[Yinghai Lu]

Current code: after irq remapping is enabled, irq_chip fields are modified
during every irq setup.
	mp_register_gsi
		io_apic_set_pci_routing
			io_apic_setup_irq_pin
				setup_ioapic_irq
					ioapic_register_intr
						setup_remapped_irq
	native_setup_msi_irqs
		setup_msi_irq
			setup_remapped_irq
	default_setup_hpet_msi
		setup_remapped_irq
that is not efficient.

We only need to modify those irq chip one time after we enable
irq remapping.

Change irq_remap_modify_chip_defaults() to __init as it only gets
called during booting stage, via irq_remap_modify_chips().

Affected irq_chip: ioapic_chip, msi_chip, hpet_msi_type.
We don't need to use #ifdef in irq_remap_modify_chips():
IRQ_REMAP only support x86_64 && X86_IO_APIC && PCI_MSI.
HPET_TIMER is selected when x86_64 is set.
When we have IRQ_REMAP enabled, al three chips are defined and
used.

-v3: Remove irq_chip in setup_remapped_irq according to Sebastian.
     also add missing "\n" in printk.

Signed-off-by: Yinghai Lu <yinghai@kernel.org>
Cc: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
Cc: Sebastian Andrzej Siewior <sebastian@breakpoint.cc>
Cc: Joerg Roedel <joro@8bytes.org>
---
 arch/x86/include/asm/io_apic.h       |  5 +++++
 arch/x86/include/asm/irq_remapping.h |  8 ++------
 arch/x86/kernel/apic/apic.c          |  9 ++++++++-
 arch/x86/kernel/apic/io_apic.c       | 14 ++++++--------
 drivers/iommu/irq_remapping.c        | 13 ++++++++++---
 5 files changed, 31 insertions(+), 18 deletions(-)

diff --git a/arch/x86/include/asm/io_apic.h b/arch/x86/include/asm/io_apic.h
index 459e50a..eaff3ad 100644
--- a/arch/x86/include/asm/io_apic.h
+++ b/arch/x86/include/asm/io_apic.h
@@ -215,6 +215,11 @@ static inline void io_apic_modify(unsigned int apic, unsigned int reg, unsigned
 
 extern void io_apic_eoi(unsigned int apic, unsigned int vector);
 
+extern struct irq_chip ioapic_chip;
+extern struct irq_chip msi_chip;
+extern struct irq_chip hpet_msi_type;
+void irq_remap_modify_chips(void);
+
 #else  /* !CONFIG_X86_IO_APIC */
 
 #define io_apic_assign_pci_irqs 0
diff --git a/arch/x86/include/asm/irq_remapping.h b/arch/x86/include/asm/irq_remapping.h
index 606f42e..304c93f 100644
--- a/arch/x86/include/asm/irq_remapping.h
+++ b/arch/x86/include/asm/irq_remapping.h
@@ -52,9 +52,7 @@ extern void compose_remapped_msi_msg(struct pci_dev *pdev,
 				     struct msi_msg *msg, u8 hpet_id);
 extern int setup_hpet_msi_remapped(unsigned int irq, unsigned int id);
 extern void panic_if_irq_remap(const char *msg);
-extern bool setup_remapped_irq(int irq,
-			       struct irq_cfg *cfg,
-			       struct irq_chip *chip);
+extern bool setup_remapped_irq(int irq, struct irq_cfg *cfg);
 
 #else  /* CONFIG_IRQ_REMAP */
 
@@ -89,9 +87,7 @@ static inline void panic_if_irq_remap(const char *msg)
 {
 }
 
-static inline bool setup_remapped_irq(int irq,
-				      struct irq_cfg *cfg,
-				      struct irq_chip *chip)
+static inline bool setup_remapped_irq(int irq, struct irq_cfg *cfg)
 {
 	return false;
 }
diff --git a/arch/x86/kernel/apic/apic.c b/arch/x86/kernel/apic/apic.c
index eca89c5..9b64203 100644
--- a/arch/x86/kernel/apic/apic.c
+++ b/arch/x86/kernel/apic/apic.c
@@ -1571,6 +1571,8 @@ void enable_x2apic(void)
 int __init enable_IR(void)
 {
 #ifdef CONFIG_IRQ_REMAP
+	int ret;
+
 	if (!irq_remapping_supported()) {
 		pr_debug("intr-remapping not supported\n");
 		return -1;
@@ -1582,7 +1584,12 @@ int __init enable_IR(void)
 		return -1;
 	}
 
-	return irq_remapping_enable();
+	ret = irq_remapping_enable();
+
+	if (ret >= 0)
+		irq_remap_modify_chips();
+
+	return ret;
 #endif
 	return -1;
 }
diff --git a/arch/x86/kernel/apic/io_apic.c b/arch/x86/kernel/apic/io_apic.c
index 9ed796c..eda56ad 100644
--- a/arch/x86/kernel/apic/io_apic.c
+++ b/arch/x86/kernel/apic/io_apic.c
@@ -1238,8 +1238,6 @@ void __setup_vector_irq(int cpu)
 	raw_spin_unlock(&vector_lock);
 }
 
-static struct irq_chip ioapic_chip;
-
 #ifdef CONFIG_X86_32
 static inline int IO_APIC_irq_trigger(int irq)
 {
@@ -1280,7 +1278,7 @@ static void ioapic_register_intr(unsigned int irq, struct irq_cfg *cfg,
 		fasteoi = false;
 	}
 
-	if (setup_remapped_irq(irq, cfg, chip))
+	if (setup_remapped_irq(irq, cfg))
 		fasteoi = trigger != 0;
 
 	hdl = fasteoi ? handle_fasteoi_irq : handle_edge_irq;
@@ -2511,7 +2509,7 @@ static void ack_apic_level(struct irq_data *data)
 	ioapic_irqd_unmask(data, cfg, masked);
 }
 
-static struct irq_chip ioapic_chip __read_mostly = {
+struct irq_chip ioapic_chip __read_mostly = {
 	.name			= "IO-APIC",
 	.irq_startup		= startup_ioapic_irq,
 	.irq_mask		= mask_ioapic_irq,
@@ -3093,7 +3091,7 @@ msi_set_affinity(struct irq_data *data, const struct cpumask *mask, bool force)
  * IRQ Chip for MSI PCI/PCI-X/PCI-Express Devices,
  * which implement the MSI or MSI-X Capability Structure.
  */
-static struct irq_chip msi_chip = {
+struct irq_chip msi_chip = {
 	.name			= "PCI-MSI",
 	.irq_unmask		= unmask_msi_irq,
 	.irq_mask		= mask_msi_irq,
@@ -3123,7 +3121,7 @@ int setup_msi_irq(struct pci_dev *dev, struct msi_desc *msidesc,
 	if (!irq_offset)
 		write_msi_msg(irq, &msg);
 
-	setup_remapped_irq(irq, irq_get_chip_data(irq), chip);
+	setup_remapped_irq(irq, irq_get_chip_data(irq));
 
 	irq_set_chip_and_handler_name(irq, chip, handle_edge_irq, "edge");
 
@@ -3240,7 +3238,7 @@ static int hpet_msi_set_affinity(struct irq_data *data,
 	return IRQ_SET_MASK_OK_NOCOPY;
 }
 
-static struct irq_chip hpet_msi_type = {
+struct irq_chip hpet_msi_type = {
 	.name = "HPET_MSI",
 	.irq_unmask = hpet_msi_unmask,
 	.irq_mask = hpet_msi_mask,
@@ -3261,7 +3259,7 @@ int default_setup_hpet_msi(unsigned int irq, unsigned int id)
 
 	hpet_msi_write(irq_get_handler_data(irq), &msg);
 	irq_set_status_flags(irq, IRQ_MOVE_PCNTXT);
-	setup_remapped_irq(irq, irq_get_chip_data(irq), chip);
+	setup_remapped_irq(irq, irq_get_chip_data(irq));
 
 	irq_set_chip_and_handler_name(irq, chip, handle_edge_irq, "edge");
 	return 0;
diff --git a/drivers/iommu/irq_remapping.c b/drivers/iommu/irq_remapping.c
index 85ae2cf..8e4e3af 100644
--- a/drivers/iommu/irq_remapping.c
+++ b/drivers/iommu/irq_remapping.c
@@ -375,19 +375,26 @@ static void ir_print_prefix(struct irq_data *data, struct seq_file *p)
 	seq_printf(p, " IR-%s", data->chip->name);
 }
 
-static void irq_remap_modify_chip_defaults(struct irq_chip *chip)
+static void __init irq_remap_modify_chip_defaults(struct irq_chip *chip)
 {
+	printk(KERN_DEBUG "irq_chip: %s ==> IR-%s\n", chip->name, chip->name);
 	chip->irq_print_chip = ir_print_prefix;
 	chip->irq_ack = ir_ack_apic_edge;
 	chip->irq_eoi = ir_ack_apic_level;
 	chip->irq_set_affinity = x86_io_apic_ops.set_affinity;
 }
 
-bool setup_remapped_irq(int irq, struct irq_cfg *cfg, struct irq_chip *chip)
+void __init irq_remap_modify_chips(void)
+{
+	irq_remap_modify_chip_defaults(&ioapic_chip);
+	irq_remap_modify_chip_defaults(&msi_chip);
+	irq_remap_modify_chip_defaults(&hpet_msi_type);
+}
+
+bool setup_remapped_irq(int irq, struct irq_cfg *cfg)
 {
 	if (!irq_remapped(cfg))
 		return false;
 	irq_set_status_flags(irq, IRQ_MOVE_PCNTXT);
-	irq_remap_modify_chip_defaults(chip);
 	return true;
 }
-- 
1.8.1.4

[PATCH v4 06/28] x86, irq: Show MSI-X clearly in debug message

[Yinghai Lu]

Print out exact MSI or MSI-X instead of MSI/MSI-X.

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>
---
 arch/x86/kernel/apic/io_apic.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/arch/x86/kernel/apic/io_apic.c b/arch/x86/kernel/apic/io_apic.c
index eda56ad..119fdf8 100644
--- a/arch/x86/kernel/apic/io_apic.c
+++ b/arch/x86/kernel/apic/io_apic.c
@@ -3125,7 +3125,8 @@ int setup_msi_irq(struct pci_dev *dev, struct msi_desc *msidesc,
 
 	irq_set_chip_and_handler_name(irq, chip, handle_edge_irq, "edge");
 
-	dev_printk(KERN_DEBUG, &dev->dev, "irq %d for MSI/MSI-X\n", irq);
+	dev_printk(KERN_DEBUG, &dev->dev, "irq %d for MSI%s\n", irq,
+			 msidesc->msi_attrib.is_msix ? "-X" : "");
 
 	return 0;
 }
-- 
1.8.1.4

[PATCH v4 07/28] x86, irq: Show MSI-X in /proc/interrupt

[Yinghai Lu]

Now MSI-X is shown as MSI in /proc/interrupt.

We could use new added irq_print_chip() interface to append -X for MSI-X.

After this patch, we will have PCI-MSI-X-edge IR-PCI-MSI-X-edge for MSI-X
instead of PCI-MSI-edge IR-PCI-MSI-edge in the /proc/interrupt.

-v2: do not need to check if msi_desc is null in msi_irq_print_chip().

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>
---
 arch/x86/kernel/apic/io_apic.c | 7 +++++++
 drivers/iommu/irq_remapping.c  | 5 ++++-
 2 files changed, 11 insertions(+), 1 deletion(-)

diff --git a/arch/x86/kernel/apic/io_apic.c b/arch/x86/kernel/apic/io_apic.c
index 119fdf8..cf320fb 100644
--- a/arch/x86/kernel/apic/io_apic.c
+++ b/arch/x86/kernel/apic/io_apic.c
@@ -3087,6 +3087,12 @@ msi_set_affinity(struct irq_data *data, const struct cpumask *mask, bool force)
 	return IRQ_SET_MASK_OK_NOCOPY;
 }
 
+static void msi_irq_print_chip(struct irq_data *data, struct seq_file *p)
+{
+	seq_printf(p, " %s%s", data->chip->name,
+			data->msi_desc->msi_attrib.is_msix ? "-X" : "");
+}
+
 /*
  * IRQ Chip for MSI PCI/PCI-X/PCI-Express Devices,
  * which implement the MSI or MSI-X Capability Structure.
@@ -3098,6 +3104,7 @@ struct irq_chip msi_chip = {
 	.irq_ack		= ack_apic_edge,
 	.irq_set_affinity	= msi_set_affinity,
 	.irq_retrigger		= ioapic_retrigger_irq,
+	.irq_print_chip		= msi_irq_print_chip,
 };
 
 int setup_msi_irq(struct pci_dev *dev, struct msi_desc *msidesc,
diff --git a/drivers/iommu/irq_remapping.c b/drivers/iommu/irq_remapping.c
index 8e4e3af..6893d75 100644
--- a/drivers/iommu/irq_remapping.c
+++ b/drivers/iommu/irq_remapping.c
@@ -372,7 +372,10 @@ static void ir_ack_apic_level(struct irq_data *data)
 
 static void ir_print_prefix(struct irq_data *data, struct seq_file *p)
 {
-	seq_printf(p, " IR-%s", data->chip->name);
+	seq_printf(p, " IR-%s%s", data->chip->name,
+			data->msi_desc ?
+			 (data->msi_desc->msi_attrib.is_msix ? "-X" : "")
+			 : "");
 }
 
 static void __init irq_remap_modify_chip_defaults(struct irq_chip *chip)
-- 
1.8.1.4

[PATCH v4 08/28] x86, irq: Make dmar_msi/hpet_msi irq_chip name consistent

[Yinghai Lu]

All others are using "-" instead of "_".

Change dmar_msi and hpet_msi to use "-".

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>
---
 arch/x86/kernel/apic/io_apic.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/x86/kernel/apic/io_apic.c b/arch/x86/kernel/apic/io_apic.c
index cf320fb..47add1a 100644
--- a/arch/x86/kernel/apic/io_apic.c
+++ b/arch/x86/kernel/apic/io_apic.c
@@ -3199,7 +3199,7 @@ dmar_msi_set_affinity(struct irq_data *data, const struct cpumask *mask,
 }
 
 static struct irq_chip dmar_msi_type = {
-	.name			= "DMAR_MSI",
+	.name			= "DMAR-MSI",
 	.irq_unmask		= dmar_msi_unmask,
 	.irq_mask		= dmar_msi_mask,
 	.irq_ack		= ack_apic_edge,
@@ -3247,7 +3247,7 @@ static int hpet_msi_set_affinity(struct irq_data *data,
 }
 
 struct irq_chip hpet_msi_type = {
-	.name = "HPET_MSI",
+	.name = "HPET-MSI",
 	.irq_unmask = hpet_msi_unmask,
 	.irq_mask = hpet_msi_mask,
 	.irq_ack = ack_apic_edge,
-- 
1.8.1.4

[PATCH v4 09/28] ia64, irq: Add dummy create_irq_nr()

[Yinghai Lu]

create_irq() will return -1 when fail to allocate.
create_irq_nr() will return 0 when fail to allocate.

Will use it to fix one return value checking for dmar_msi irq.

Signed-off-by: Yinghai Lu <yinghai@kernel.org>
Cc: Fenghua Yu <fenghua.yu@intel.com>
Cc: linux-ia64@vger.kernel.org
Cc: stable@vger.kernel.org
---
 arch/ia64/kernel/irq_ia64.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/arch/ia64/kernel/irq_ia64.c b/arch/ia64/kernel/irq_ia64.c
index 1034884..38e46df 100644
--- a/arch/ia64/kernel/irq_ia64.c
+++ b/arch/ia64/kernel/irq_ia64.c
@@ -429,6 +429,16 @@ int create_irq(void)
 	return irq;
 }
 
+unsigned int create_irq_nr(unsigned int from, int node)
+{
+	int irq = create_irq();
+
+	if (irq < 0)
+		irq = 0;
+
+	return irq;
+}
+
 void destroy_irq(unsigned int irq)
 {
 	dynamic_irq_cleanup(irq);
-- 
1.8.1.4

[PATCH v4 10/28] iommu, irq: Allocate irq_desc for dmar_msi with local node

[Yinghai Lu]

iommu irq's irq_desc should be on local node ram.

Fix the return value checking problem.
  create_irq() will return -1 when fail to allocate.
  create_irq_nr() will return 0 when fail to allocate.
here only check !irq, so need to change it to use create_irq_nr instead.

-v2: According to Sebastian, add cc to stable.

Signed-off-by: Yinghai Lu <yinghai@kernel.org>
Cc: Joerg Roedel <joro@8bytes.org>
Cc: Donald Dutile <ddutile@redhat.com>
Acked-by: Donald Dutile <ddutile@redhat.com>
Cc: Sebastian Andrzej Siewior <sebastian@breakpoint.cc>
Cc: stable@vger.kernel.org
---
 drivers/iommu/dmar.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/iommu/dmar.c b/drivers/iommu/dmar.c
index 785675a..db41e36 100644
--- a/drivers/iommu/dmar.c
+++ b/drivers/iommu/dmar.c
@@ -1277,7 +1277,7 @@ int dmar_set_interrupt(struct intel_iommu *iommu)
 	if (iommu->irq)
 		return 0;
 
-	irq = create_irq();
+	irq = create_irq_nr(0, iommu->node);
 	if (!irq) {
 		pr_err("IOMMU: no free vectors\n");
 		return -EINVAL;
-- 
1.8.1.4

[PATCH v4 11/28] x86, irq: Kill create_irq()

[Yinghai Lu]

create_irq() will return -1 when failing to allocate.
create_irq_nr() will return 0 when failing to allocate.

It only causes confusion.

Now we have no user for create_irq(), so remove create_irq() for x86.

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>
---
 arch/x86/kernel/apic/io_apic.c | 15 ---------------
 1 file changed, 15 deletions(-)

diff --git a/arch/x86/kernel/apic/io_apic.c b/arch/x86/kernel/apic/io_apic.c
index 47add1a..ccfb1d3 100644
--- a/arch/x86/kernel/apic/io_apic.c
+++ b/arch/x86/kernel/apic/io_apic.c
@@ -2968,21 +2968,6 @@ unsigned int create_irq_nr(unsigned int from, int node)
 	return __create_irqs(from, 1, node);
 }
 
-int create_irq(void)
-{
-	int node = cpu_to_node(0);
-	unsigned int irq_want;
-	int irq;
-
-	irq_want = nr_irqs_gsi;
-	irq = create_irq_nr(irq_want, node);
-
-	if (irq == 0)
-		irq = -1;
-
-	return irq;
-}
-
 void destroy_irq(unsigned int irq)
 {
 	struct irq_cfg *cfg = irq_get_chip_data(irq);
-- 
1.8.1.4

[PATCH v4 12/28] x86, irq: Convert irq_2_pin list to generic list

[Yinghai Lu]

Now irq_2_pin list is own grown list for ioapic irq/pin tracking.
We can use generic list to replace it so we could use generic helper
functions to operate it.

Also make free_irq_cfg() free irq_2_pin list to support coming ioapic
hotplug.

Signed-off-by: Yinghai Lu <yinghai@kernel.org>
Cc: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
Cc: Sebastian Andrzej Siewior <sebastian@breakpoint.cc>
Cc: Joerg Roedel <joro@8bytes.org>
---
 arch/x86/include/asm/hw_irq.h  |  2 +-
 arch/x86/kernel/apic/io_apic.c | 22 ++++++++++++----------
 2 files changed, 13 insertions(+), 11 deletions(-)

diff --git a/arch/x86/include/asm/hw_irq.h b/arch/x86/include/asm/hw_irq.h
index e4ac559..8504333 100644
--- a/arch/x86/include/asm/hw_irq.h
+++ b/arch/x86/include/asm/hw_irq.h
@@ -139,7 +139,7 @@ struct irq_2_irte {
  * Most irqs are mapped 1:1 with pins.
  */
 struct irq_cfg {
-	struct irq_pin_list	*irq_2_pin;
+	struct list_head	irq_2_pin;
 	cpumask_var_t		domain;
 	cpumask_var_t		old_domain;
 	u8			vector;
diff --git a/arch/x86/kernel/apic/io_apic.c b/arch/x86/kernel/apic/io_apic.c
index ccfb1d3..ef2d530 100644
--- a/arch/x86/kernel/apic/io_apic.c
+++ b/arch/x86/kernel/apic/io_apic.c
@@ -66,7 +66,7 @@
 #define __apicdebuginit(type) static type __init
 
 #define for_each_irq_pin(entry, head) \
-	for (entry = head; entry; entry = entry->next)
+	list_for_each_entry(entry, &head, list)
 
 /*
  *      Is the SiS APIC rmw bug present ?
@@ -176,8 +176,8 @@ void mp_save_irq(struct mpc_intsrc *m)
 }
 
 struct irq_pin_list {
+	struct list_head list;
 	int apic, pin;
-	struct irq_pin_list *next;
 };
 
 static struct irq_pin_list *alloc_irq_pin_list(int node)
@@ -213,6 +213,7 @@ int __init arch_early_irq_init(void)
 	irq_reserve_irqs(0, legacy_pic->nr_legacy_irqs);
 
 	for (i = 0; i < count; i++) {
+		INIT_LIST_HEAD(&cfg[i].irq_2_pin);
 		irq_set_chip_data(i, &cfg[i]);
 		zalloc_cpumask_var_node(&cfg[i].domain, GFP_KERNEL, node);
 		zalloc_cpumask_var_node(&cfg[i].old_domain, GFP_KERNEL, node);
@@ -245,6 +246,7 @@ static struct irq_cfg *alloc_irq_cfg(unsigned int irq, int node)
 		goto out_cfg;
 	if (!zalloc_cpumask_var_node(&cfg->old_domain, GFP_KERNEL, node))
 		goto out_domain;
+	INIT_LIST_HEAD(&cfg->irq_2_pin);
 	return cfg;
 out_domain:
 	free_cpumask_var(cfg->domain);
@@ -255,11 +257,15 @@ out_cfg:
 
 static void free_irq_cfg(unsigned int at, struct irq_cfg *cfg)
 {
+	struct irq_pin_list *entry, *tmp;
+
 	if (!cfg)
 		return;
 	irq_set_chip_data(at, NULL);
 	free_cpumask_var(cfg->domain);
 	free_cpumask_var(cfg->old_domain);
+	list_for_each_entry_safe(entry, tmp, &cfg->irq_2_pin, list)
+		kfree(entry);
 	kfree(cfg);
 }
 
@@ -420,15 +426,12 @@ static void ioapic_mask_entry(int apic, int pin)
  */
 static int __add_pin_to_irq_node(struct irq_cfg *cfg, int node, int apic, int pin)
 {
-	struct irq_pin_list **last, *entry;
+	struct irq_pin_list *entry;
 
 	/* don't allow duplicates */
-	last = &cfg->irq_2_pin;
-	for_each_irq_pin(entry, cfg->irq_2_pin) {
+	for_each_irq_pin(entry, cfg->irq_2_pin)
 		if (entry->apic == apic && entry->pin == pin)
 			return 0;
-		last = &entry->next;
-	}
 
 	entry = alloc_irq_pin_list(node);
 	if (!entry) {
@@ -439,7 +442,7 @@ static int __add_pin_to_irq_node(struct irq_cfg *cfg, int node, int apic, int pi
 	entry->apic = apic;
 	entry->pin = pin;
 
-	*last = entry;
+	list_add_tail(&entry->list, &cfg->irq_2_pin);
 	return 0;
 }
 
@@ -1622,8 +1625,7 @@ __apicdebuginit(void) print_IO_APICs(void)
 		cfg = irq_get_chip_data(irq);
 		if (!cfg)
 			continue;
-		entry = cfg->irq_2_pin;
-		if (!entry)
+		if (list_empty(&cfg->irq_2_pin))
 			continue;
 		printk(KERN_DEBUG "IRQ%d ", irq);
 		for_each_irq_pin(entry, cfg->irq_2_pin)
-- 
1.8.1.4

[PATCH v4 13/28] x86, irq: Add alloc_reserved_irq_and_cfg_at()

[Yinghai Lu]

For ioapic hot-add support, it would be easy if we have continuous
irq numbers for new added ioapic controller.

We can reserve irq range at first, then allocate those
pre-reserved one when it is needed.

Add alloc_reserved_irq_and_cfg_at() to really allocate irq_desc and cfg,
because pre-reserved only mark bits in allocate_irqs bit maps.

Also fallback to alloc_irq_and_cfg_at(), and it is needed as
we do not actually reserved irq for io apic irq yet.

This patch is x86 arch code.

-v2: update changelog by adding reasons, requested by Konrad.
-v3: according to tglx, separate to another patch, and use
     alloc_reserved_... instead of realloc_...
     also add handling for one possible path in
       alloc_reserved_irq_and_cfg_at().

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>
---
 arch/x86/kernel/apic/io_apic.c | 25 ++++++++++++++++++++++++-
 1 file changed, 24 insertions(+), 1 deletion(-)

diff --git a/arch/x86/kernel/apic/io_apic.c b/arch/x86/kernel/apic/io_apic.c
index ef2d530..c883da9 100644
--- a/arch/x86/kernel/apic/io_apic.c
+++ b/arch/x86/kernel/apic/io_apic.c
@@ -301,6 +301,29 @@ static void free_irq_at(unsigned int at, struct irq_cfg *cfg)
 	irq_free_desc(at);
 }
 
+static struct irq_cfg *alloc_reserved_irq_and_cfg_at(unsigned int at, int node)
+{
+	struct irq_cfg *cfg;
+	int res;
+
+	res = irq_alloc_reserved_desc_at(at, node);
+
+	if (res >= 0) {
+		cfg = irq_desc_get_chip_data(irq_to_desc(at));
+		if (cfg)
+			return cfg;
+
+		cfg = alloc_irq_cfg(at, node);
+		if (cfg)
+			irq_set_chip_data(at, cfg);
+		else
+			irq_free_desc(at);
+
+		return cfg;
+	}
+
+	return alloc_irq_and_cfg_at(at, node);
+}
 
 struct io_apic {
 	unsigned int index;
@@ -3352,7 +3375,7 @@ int arch_setup_ht_irq(unsigned int irq, struct pci_dev *dev)
 static int
 io_apic_setup_irq_pin(unsigned int irq, int node, struct io_apic_irq_attr *attr)
 {
-	struct irq_cfg *cfg = alloc_irq_and_cfg_at(irq, node);
+	struct irq_cfg *cfg = alloc_reserved_irq_and_cfg_at(irq, node);
 	int ret;
 
 	if (!cfg)
-- 
1.8.1.4

[PATCH v4 14/28] x86, irq: Move down arch_early_irq_init()

[Yinghai Lu]

Change position only.

Prepare to update arch_early_irq_init() that needs to call some static
functions.

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>
---
 arch/x86/kernel/apic/io_apic.c | 89 +++++++++++++++++++++---------------------
 1 file changed, 44 insertions(+), 45 deletions(-)

diff --git a/arch/x86/kernel/apic/io_apic.c b/arch/x86/kernel/apic/io_apic.c
index c883da9..18b123b 100644
--- a/arch/x86/kernel/apic/io_apic.c
+++ b/arch/x86/kernel/apic/io_apic.c
@@ -185,51 +185,6 @@ static struct irq_pin_list *alloc_irq_pin_list(int node)
 	return kzalloc_node(sizeof(struct irq_pin_list), GFP_KERNEL, node);
 }
 
-
-/* irq_cfg is indexed by the sum of all RTEs in all I/O APICs. */
-static struct irq_cfg irq_cfgx[NR_IRQS_LEGACY];
-
-int __init arch_early_irq_init(void)
-{
-	struct irq_cfg *cfg;
-	int count, node, i;
-
-	if (!legacy_pic->nr_legacy_irqs)
-		io_apic_irqs = ~0UL;
-
-	for (i = 0; i < nr_ioapics; 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);
-	}
-
-	cfg = irq_cfgx;
-	count = ARRAY_SIZE(irq_cfgx);
-	node = cpu_to_node(0);
-
-	/* Make sure the legacy interrupts are marked in the bitmap */
-	irq_reserve_irqs(0, legacy_pic->nr_legacy_irqs);
-
-	for (i = 0; i < count; i++) {
-		INIT_LIST_HEAD(&cfg[i].irq_2_pin);
-		irq_set_chip_data(i, &cfg[i]);
-		zalloc_cpumask_var_node(&cfg[i].domain, GFP_KERNEL, node);
-		zalloc_cpumask_var_node(&cfg[i].old_domain, GFP_KERNEL, node);
-		/*
-		 * For legacy IRQ's, start with assigning irq0 to irq15 to
-		 * IRQ0_VECTOR to IRQ15_VECTOR for all cpu's.
-		 */
-		if (i < legacy_pic->nr_legacy_irqs) {
-			cfg[i].vector = IRQ0_VECTOR + i;
-			cpumask_setall(cfg[i].domain);
-		}
-	}
-
-	return 0;
-}
-
 static struct irq_cfg *irq_cfg(unsigned int irq)
 {
 	return irq_get_chip_data(irq);
@@ -325,6 +280,50 @@ static struct irq_cfg *alloc_reserved_irq_and_cfg_at(unsigned int at, int node)
 	return alloc_irq_and_cfg_at(at, node);
 }
 
+/* irq_cfg is indexed by the sum of all RTEs in all I/O APICs. */
+static struct irq_cfg irq_cfgx[NR_IRQS_LEGACY];
+
+int __init arch_early_irq_init(void)
+{
+	struct irq_cfg *cfg;
+	int count, node, i;
+
+	if (!legacy_pic->nr_legacy_irqs)
+		io_apic_irqs = ~0UL;
+
+	for (i = 0; i < nr_ioapics; 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);
+	}
+
+	cfg = irq_cfgx;
+	count = ARRAY_SIZE(irq_cfgx);
+	node = cpu_to_node(0);
+
+	/* Make sure the legacy interrupts are marked in the bitmap */
+	irq_reserve_irqs(0, legacy_pic->nr_legacy_irqs);
+
+	for (i = 0; i < count; i++) {
+		INIT_LIST_HEAD(&cfg[i].irq_2_pin);
+		irq_set_chip_data(i, &cfg[i]);
+		zalloc_cpumask_var_node(&cfg[i].domain, GFP_KERNEL, node);
+		zalloc_cpumask_var_node(&cfg[i].old_domain, GFP_KERNEL, node);
+		/*
+		 * For legacy IRQ's, start with assigning irq0 to irq15 to
+		 * IRQ0_VECTOR to IRQ15_VECTOR for all cpu's.
+		 */
+		if (i < legacy_pic->nr_legacy_irqs) {
+			cfg[i].vector = IRQ0_VECTOR + i;
+			cpumask_setall(cfg[i].domain);
+		}
+	}
+
+	return 0;
+}
+
 struct io_apic {
 	unsigned int index;
 	unsigned int unused[3];
-- 
1.8.1.4

[PATCH v4 15/28] x86, irq: Split out alloc_ioapic_save_registers()

[Yinghai Lu]

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>
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 18b123b..3e0530c 100644
--- a/arch/x86/kernel/apic/io_apic.c
+++ b/arch/x86/kernel/apic/io_apic.c
@@ -283,6 +283,19 @@ static struct irq_cfg *alloc_reserved_irq_and_cfg_at(unsigned int at, int node)
 /* irq_cfg is indexed by the sum of all RTEs in all I/O APICs. */
 static struct irq_cfg irq_cfgx[NR_IRQS_LEGACY];
 
+static void alloc_ioapic_saved_registers(int idx)
+{
+	if (ioapics[idx].saved_registers)
+		return;
+
+	ioapics[idx].saved_registers =
+			kzalloc(sizeof(struct IO_APIC_route_entry) *
+				ioapics[idx].nr_registers, 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;
@@ -291,13 +304,8 @@ int __init arch_early_irq_init(void)
 	if (!legacy_pic->nr_legacy_irqs)
 		io_apic_irqs = ~0UL;
 
-	for (i = 0; i < nr_ioapics; 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 (i = 0; i < nr_ioapics; i++)
+		alloc_ioapic_saved_registers(i);
 
 	cfg = irq_cfgx;
 	count = ARRAY_SIZE(irq_cfgx);
-- 
1.8.1.4

[PATCH v4 16/28] xen, irq: Call irq_alloc_reserved_desc_at() at first

[Yinghai Lu]

To make x86 irq allocation to be same with booting path and ioapic
hot add path, We will pre-reserve irq for all gsi at first.
We have to use alloc_reserved here, otherwise irq_alloc_desc_at will fail
because bit is already get marked for pre-reserved in irq bitmaps.

Signed-off-by: Yinghai Lu <yinghai@kernel.org>
Cc: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
Cc: xen-devel@lists.xensource.com
---
 drivers/xen/events.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/drivers/xen/events.c b/drivers/xen/events.c
index a58ac43..1a3647c 100644
--- a/drivers/xen/events.c
+++ b/drivers/xen/events.c
@@ -507,8 +507,12 @@ static int __must_check xen_allocate_irq_gsi(unsigned gsi)
 	/* Legacy IRQ descriptors are already allocated by the arch. */
 	if (gsi < NR_IRQS_LEGACY)
 		irq = gsi;
-	else
-		irq = irq_alloc_desc_at(gsi, -1);
+	else {
+		/* for x86, irq already get reserved for gsi */
+		irq = irq_alloc_reserved_desc_at(gsi, -1);
+		if (irq < 0)
+			irq = irq_alloc_desc_at(gsi, -1);
+	}
 
 	xen_irq_init(irq);
 
-- 
1.8.1.4

[PATCH v4 17/28] x86, irq: Reserve irq range and alloc_reserved for booting path

[Yinghai Lu]

We will use reserve and alloc_reserved_irq_and_cfg_at for hotplug ioapic path.

To make thing simple, we could make booting path use same code.

All irq range for all GSIs will be reserved at first, and alloc_reserved
will really allocate those irq_desc/cfg when it is used.

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>
---
 arch/x86/include/asm/io_apic.h |  1 +
 arch/x86/kernel/apic/io_apic.c | 80 +++++++++++++++++++++++++++++++-----------
 2 files changed, 60 insertions(+), 21 deletions(-)

diff --git a/arch/x86/include/asm/io_apic.h b/arch/x86/include/asm/io_apic.h
index eaff3ad..8181fd8 100644
--- a/arch/x86/include/asm/io_apic.h
+++ b/arch/x86/include/asm/io_apic.h
@@ -176,6 +176,7 @@ extern void setup_ioapic_ids_from_mpc_nocheck(void);
 struct mp_ioapic_gsi{
 	u32 gsi_base;
 	u32 gsi_end;
+	u32 irq_base;
 };
 extern struct mp_ioapic_gsi  mp_gsi_routing[];
 extern u32 gsi_top;
diff --git a/arch/x86/kernel/apic/io_apic.c b/arch/x86/kernel/apic/io_apic.c
index 3e0530c..cb272c9 100644
--- a/arch/x86/kernel/apic/io_apic.c
+++ b/arch/x86/kernel/apic/io_apic.c
@@ -280,8 +280,47 @@ static struct irq_cfg *alloc_reserved_irq_and_cfg_at(unsigned int at, int node)
 	return alloc_irq_and_cfg_at(at, node);
 }
 
-/* irq_cfg is indexed by the sum of all RTEs in all I/O APICs. */
-static struct irq_cfg irq_cfgx[NR_IRQS_LEGACY];
+static int reserve_ioapic_gsi_irq_base(int idx)
+{
+	int irq;
+	struct mp_ioapic_gsi *gsi_cfg = mp_ioapic_gsi_routing(idx);
+	int cnt = gsi_cfg->gsi_end - gsi_cfg->gsi_base + 1;
+
+	irq = __irq_reserve_irqs(-1, gsi_cfg->gsi_base, cnt);
+	if (irq >= 0) {
+		gsi_cfg->irq_base = irq;
+		apic_printk(APIC_VERBOSE, KERN_INFO
+			"IOAPIC[%d]: apic_id %d, GSI %d-%d ==> irq %d-%d reserved\n",
+			idx, mpc_ioapic_id(idx),
+			gsi_cfg->gsi_base, gsi_cfg->gsi_end,
+			irq, irq + cnt - 1);
+	} else
+		apic_printk(APIC_VERBOSE, KERN_WARNING
+			"IOAPIC[%d]: apic_id %d, GSI %d-%d ==> irq reserve failed\n",
+			idx, mpc_ioapic_id(idx),
+			gsi_cfg->gsi_base, gsi_cfg->gsi_end);
+
+	return irq;
+}
+
+static void __init reserve_ioapic_gsi_irq_extra(void)
+{
+	int irq;
+
+	/* to prevent hot add ioapic taking those slots */
+	if (gsi_top) {
+		irq = irq_reserve_irqs(gsi_top, NR_IRQS_LEGACY);
+		if (irq >= 0)
+			apic_printk(APIC_VERBOSE, KERN_INFO
+				"IOAPIC[extra]: GSI %d-%d ==> irq %d-%d reserved\n",
+				gsi_top, gsi_top + NR_IRQS_LEGACY - 1,
+				irq, irq + NR_IRQS_LEGACY - 1);
+		else
+			apic_printk(APIC_VERBOSE, KERN_WARNING
+				"IOAPIC[extra]: GSI %d-%d ==> irq reserve failed\n",
+				gsi_top, gsi_top + NR_IRQS_LEGACY - 1);
+	}
+}
 
 static void alloc_ioapic_saved_registers(int idx)
 {
@@ -298,8 +337,9 @@ static void alloc_ioapic_saved_registers(int idx)
 
 int __init arch_early_irq_init(void)
 {
+	int node = cpu_to_node(0);
 	struct irq_cfg *cfg;
-	int count, node, i;
+	int i;
 
 	if (!legacy_pic->nr_legacy_irqs)
 		io_apic_irqs = ~0UL;
@@ -307,26 +347,23 @@ int __init arch_early_irq_init(void)
 	for (i = 0; i < nr_ioapics; i++)
 		alloc_ioapic_saved_registers(i);
 
-	cfg = irq_cfgx;
-	count = ARRAY_SIZE(irq_cfgx);
-	node = cpu_to_node(0);
+	for (i = 0; i < nr_ioapics; i++)
+		reserve_ioapic_gsi_irq_base(i);
 
-	/* Make sure the legacy interrupts are marked in the bitmap */
-	irq_reserve_irqs(0, legacy_pic->nr_legacy_irqs);
+	reserve_ioapic_gsi_irq_extra();
 
-	for (i = 0; i < count; i++) {
-		INIT_LIST_HEAD(&cfg[i].irq_2_pin);
-		irq_set_chip_data(i, &cfg[i]);
-		zalloc_cpumask_var_node(&cfg[i].domain, GFP_KERNEL, node);
-		zalloc_cpumask_var_node(&cfg[i].old_domain, GFP_KERNEL, node);
-		/*
-		 * For legacy IRQ's, start with assigning irq0 to irq15 to
-		 * IRQ0_VECTOR to IRQ15_VECTOR for all cpu's.
-		 */
-		if (i < legacy_pic->nr_legacy_irqs) {
-			cfg[i].vector = IRQ0_VECTOR + i;
-			cpumask_setall(cfg[i].domain);
+	/*
+	 * For legacy IRQ's, start with assigning irq0 to irq15 to
+	 * IRQ0_VECTOR to IRQ15_VECTOR for all cpu's.
+	 */
+	for (i = 0; i < legacy_pic->nr_legacy_irqs; i++) {
+		cfg = alloc_reserved_irq_and_cfg_at(i, node);
+		if (!cfg) {
+			pr_warn("can not allocate irq_desc/cfg for %d\n", i);
+			continue;
 		}
+		cfg->vector = IRQ0_VECTOR + i;
+		cpumask_setall(cfg->domain);
 	}
 
 	return 0;
@@ -3460,7 +3497,8 @@ int __init arch_probe_nr_irqs(void)
 	if (nr < nr_irqs)
 		nr_irqs = nr;
 
-	return NR_IRQS_LEGACY;
+	/* x86 arch code will allocate irq_desc/cfg */
+	return 0;
 }
 
 int io_apic_set_pci_routing(struct device *dev, int irq,
-- 
1.8.1.4

[PATCH v4 18/28] x86, irq: Add ioapic_gsi_to_irq

[Yinghai Lu]

For hot add ioapic, irq_base is not equal to gsi_base.
We need a way to do mapping between gsi and irq.

Also remove irq_to_gsi() that is confusing, just use that array
directly as we only have one caller and it already check
input irq before use it.

Signed-off-by: Yinghai Lu <yinghai@kernel.org>
Cc: Pavel Machek <pavel@ucw.cz>
Cc: Joerg Roedel <joro@8bytes.org>
Cc: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
Cc: Sebastian Andrzej Siewior <sebastian@breakpoint.cc>
---
 arch/x86/include/asm/io_apic.h |  1 +
 arch/x86/kernel/acpi/boot.c    | 22 +++++-----------------
 arch/x86/kernel/apic/io_apic.c | 29 ++++++++++++++++++++++++++++-
 3 files changed, 34 insertions(+), 18 deletions(-)

diff --git a/arch/x86/include/asm/io_apic.h b/arch/x86/include/asm/io_apic.h
index 8181fd8..02ac411 100644
--- a/arch/x86/include/asm/io_apic.h
+++ b/arch/x86/include/asm/io_apic.h
@@ -180,6 +180,7 @@ struct mp_ioapic_gsi{
 };
 extern struct mp_ioapic_gsi  mp_gsi_routing[];
 extern u32 gsi_top;
+int ioapic_gsi_to_irq(u32 gsi);
 int mp_find_ioapic(u32 gsi);
 int mp_find_ioapic_pin(int ioapic, u32 gsi);
 void __init mp_register_ioapic(int id, u32 address, u32 gsi_base);
diff --git a/arch/x86/kernel/acpi/boot.c b/arch/x86/kernel/acpi/boot.c
index 2627a81..5be15d1 100644
--- a/arch/x86/kernel/acpi/boot.c
+++ b/arch/x86/kernel/acpi/boot.c
@@ -112,6 +112,10 @@ static unsigned int gsi_to_irq(unsigned int gsi)
 		}
 	}
 
+#ifdef CONFIG_X86_IO_APIC
+	if (acpi_irq_model == ACPI_IRQ_MODEL_IOAPIC)
+		return ioapic_gsi_to_irq(gsi);
+#endif
 	/* Provide an identity mapping of gsi == irq
 	 * except on truly weird platforms that have
 	 * non isa irqs in the first 16 gsis.
@@ -124,22 +128,6 @@ static unsigned int gsi_to_irq(unsigned int gsi)
 	return irq;
 }
 
-static u32 irq_to_gsi(int irq)
-{
-	unsigned int gsi;
-
-	if (irq < NR_IRQS_LEGACY)
-		gsi = isa_irq_to_gsi[irq];
-	else if (irq < gsi_top)
-		gsi = irq;
-	else if (irq < (gsi_top + NR_IRQS_LEGACY))
-		gsi = irq - gsi_top;
-	else
-		gsi = 0xffffffff;
-
-	return gsi;
-}
-
 /*
  * Temporarily use the virtual area starting from FIX_IO_APIC_BASE_END,
  * to map the target physical address. The problem is that set_fixmap()
@@ -529,7 +517,7 @@ int acpi_isa_irq_to_gsi(unsigned isa_irq, u32 *gsi)
 {
 	if (isa_irq >= 16)
 		return -1;
-	*gsi = irq_to_gsi(isa_irq);
+	*gsi = isa_irq_to_gsi[isa_irq];
 	return 0;
 }
 
diff --git a/arch/x86/kernel/apic/io_apic.c b/arch/x86/kernel/apic/io_apic.c
index cb272c9..8d087f1 100644
--- a/arch/x86/kernel/apic/io_apic.c
+++ b/arch/x86/kernel/apic/io_apic.c
@@ -1041,13 +1041,16 @@ static int pin_2_irq(int idx, int apic, int pin)
 
 	if (test_bit(bus, mp_bus_not_pci)) {
 		irq = mp_irqs[idx].srcbusirq;
-	} else {
+	} else if (gsi_cfg->gsi_base == gsi_cfg->irq_base) {
 		u32 gsi = gsi_cfg->gsi_base + pin;
 
 		if (gsi >= NR_IRQS_LEGACY)
 			irq = gsi;
 		else
 			irq = gsi_top + gsi;
+	} else {
+		/* hotadd ioapic */
+		irq = gsi_cfg->irq_base + pin;
 	}
 
 #ifdef CONFIG_X86_32
@@ -1473,6 +1476,30 @@ static void __init setup_IO_APIC_irqs(void)
 		__io_apic_setup_irqs(ioapic_idx);
 }
 
+int ioapic_gsi_to_irq(u32 gsi)
+{
+	int ioapic_idx = 0, irq = gsi;
+	struct mp_ioapic_gsi *gsi_cfg;
+
+	ioapic_idx = mp_find_ioapic(gsi);
+	if (ioapic_idx < 0)
+		return -1;
+
+	gsi_cfg = mp_ioapic_gsi_routing(ioapic_idx);
+	if (gsi_cfg->gsi_base == gsi_cfg->irq_base) {
+		if (gsi < NR_IRQS_LEGACY)
+			irq = gsi_top + gsi;
+	} else {
+		int pin = mp_find_ioapic_pin(ioapic_idx, gsi);
+
+		if (pin < 0)
+			return -1;
+		/* hotadd ioapic */
+		irq = gsi_cfg->irq_base + pin;
+	}
+
+	return irq;
+}
 /*
  * for the gsit that is not in first ioapic
  * but could not use acpi_register_gsi()
-- 
1.8.1.4

[PATCH v4 19/28] x86, irq: Add for_each_ioapic helper

[Yinghai Lu]

For hotadd and hotremove ioapic controller, we leave blank
slots in ioapics array.

So we can not use for (i=...) to loop them any more.

Introdue ioapics_mask bitmap to track used ioapics, and use
for_each_ioapic to loop them.

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>
Cc: Grant Likely <grant.likely@linaro.org>
Cc: Paul Gortmaker <paul.gortmaker@windriver.com>
Cc: Andy Lutomirski <luto@amacapital.net>
Cc: iommu@lists.linux-foundation.org
---
 arch/x86/include/asm/io_apic.h      |  6 ++++
 arch/x86/kernel/apic/io_apic.c      | 64 ++++++++++++++++++++-----------------
 arch/x86/kernel/devicetree.c        |  2 +-
 drivers/iommu/amd_iommu_init.c      |  2 +-
 drivers/iommu/intel_irq_remapping.c |  2 +-
 5 files changed, 43 insertions(+), 33 deletions(-)

diff --git a/arch/x86/include/asm/io_apic.h b/arch/x86/include/asm/io_apic.h
index 02ac411..a25dcf1 100644
--- a/arch/x86/include/asm/io_apic.h
+++ b/arch/x86/include/asm/io_apic.h
@@ -105,6 +105,12 @@ struct IR_IO_APIC_route_entry {
  * # of IO-APICs and # of IRQ routing registers
  */
 extern int nr_ioapics;
+extern DECLARE_BITMAP(ioapics_mask, MAX_IO_APICS);
+
+#define for_each_ioapic(i)						\
+	for ((i) = -1;							\
+		(i) = find_next_bit(ioapics_mask, MAX_IO_APICS, (i)+1), \
+		(i) < MAX_IO_APICS;)
 
 extern int mpc_ioapic_id(int ioapic);
 extern unsigned int mpc_ioapic_addr(int ioapic);
diff --git a/arch/x86/kernel/apic/io_apic.c b/arch/x86/kernel/apic/io_apic.c
index 8d087f1..b026cc7 100644
--- a/arch/x86/kernel/apic/io_apic.c
+++ b/arch/x86/kernel/apic/io_apic.c
@@ -93,6 +93,8 @@ static struct ioapic {
 	DECLARE_BITMAP(pin_programmed, MP_MAX_IOAPIC_PIN + 1);
 } ioapics[MAX_IO_APICS];
 
+DECLARE_BITMAP(ioapics_mask, MAX_IO_APICS);
+
 #define mpc_ioapic_ver(ioapic_idx)	ioapics[ioapic_idx].mp_config.apicver
 
 int mpc_ioapic_id(int ioapic_idx)
@@ -344,10 +346,10 @@ int __init arch_early_irq_init(void)
 	if (!legacy_pic->nr_legacy_irqs)
 		io_apic_irqs = ~0UL;
 
-	for (i = 0; i < nr_ioapics; i++)
+	for_each_ioapic(i)
 		alloc_ioapic_saved_registers(i);
 
-	for (i = 0; i < nr_ioapics; i++)
+	for_each_ioapic(i)
 		reserve_ioapic_gsi_irq_base(i);
 
 	reserve_ioapic_gsi_irq_extra();
@@ -715,7 +717,7 @@ static void clear_IO_APIC (void)
 {
 	int apic, pin;
 
-	for (apic = 0; apic < nr_ioapics; apic++)
+	for_each_ioapic(apic)
 		for (pin = 0; pin < ioapics[apic].nr_registers; pin++)
 			clear_IO_APIC_pin(apic, pin);
 }
@@ -766,7 +768,7 @@ int save_ioapic_entries(void)
 	int apic, pin;
 	int err = 0;
 
-	for (apic = 0; apic < nr_ioapics; apic++) {
+	for_each_ioapic(apic) {
 		if (!ioapics[apic].saved_registers) {
 			err = -ENOMEM;
 			continue;
@@ -787,7 +789,7 @@ void mask_ioapic_entries(void)
 {
 	int apic, pin;
 
-	for (apic = 0; apic < nr_ioapics; apic++) {
+	for_each_ioapic(apic) {
 		if (!ioapics[apic].saved_registers)
 			continue;
 
@@ -810,7 +812,7 @@ int restore_ioapic_entries(void)
 {
 	int apic, pin;
 
-	for (apic = 0; apic < nr_ioapics; apic++) {
+	for_each_ioapic(apic) {
 		if (!ioapics[apic].saved_registers)
 			continue;
 
@@ -873,7 +875,7 @@ static int __init find_isa_irq_apic(int irq, int type)
 	if (i < mp_irq_entries) {
 		int ioapic_idx;
 
-		for (ioapic_idx = 0; ioapic_idx < nr_ioapics; ioapic_idx++)
+		for_each_ioapic(ioapic_idx)
 			if (mpc_ioapic_id(ioapic_idx) == mp_irqs[i].dstapic)
 				return ioapic_idx;
 	}
@@ -1095,7 +1097,7 @@ int IO_APIC_get_PCI_irq_vector(int bus, int slot, int pin,
 	for (i = 0; i < mp_irq_entries; i++) {
 		int lbus = mp_irqs[i].srcbus;
 
-		for (ioapic_idx = 0; ioapic_idx < nr_ioapics; ioapic_idx++)
+		for_each_ioapic(ioapic_idx)
 			if (mpc_ioapic_id(ioapic_idx) == mp_irqs[i].dstapic ||
 			    mp_irqs[i].dstapic == MP_APIC_ALL)
 				break;
@@ -1316,7 +1318,7 @@ static inline int IO_APIC_irq_trigger(int irq)
 {
 	int apic, idx, pin;
 
-	for (apic = 0; apic < nr_ioapics; apic++) {
+	for_each_ioapic(apic) {
 		for (pin = 0; pin < ioapics[apic].nr_registers; pin++) {
 			idx = find_irq_entry(apic, pin, mp_INT);
 			if ((idx != -1) && (irq == pin_2_irq(idx, apic, pin)))
@@ -1472,7 +1474,7 @@ static void __init setup_IO_APIC_irqs(void)
 
 	apic_printk(APIC_VERBOSE, KERN_DEBUG "init IO_APIC IRQs\n");
 
-	for (ioapic_idx = 0; ioapic_idx < nr_ioapics; ioapic_idx++)
+	for_each_ioapic(ioapic_idx)
 		__io_apic_setup_irqs(ioapic_idx);
 }
 
@@ -1694,7 +1696,7 @@ __apicdebuginit(void) print_IO_APICs(void)
 	struct irq_chip *chip;
 
 	printk(KERN_DEBUG "number of MP IRQ sources: %d.\n", mp_irq_entries);
-	for (ioapic_idx = 0; ioapic_idx < nr_ioapics; ioapic_idx++)
+	for_each_ioapic(ioapic_idx)
 		printk(KERN_DEBUG "number of IO-APIC #%d registers: %d.\n",
 		       mpc_ioapic_id(ioapic_idx),
 		       ioapics[ioapic_idx].nr_registers);
@@ -1705,7 +1707,7 @@ __apicdebuginit(void) print_IO_APICs(void)
 	 */
 	printk(KERN_INFO "testing the IO APIC.......................\n");
 
-	for (ioapic_idx = 0; ioapic_idx < nr_ioapics; ioapic_idx++)
+	for_each_ioapic(ioapic_idx)
 		print_IO_APIC(ioapic_idx);
 
 	printk(KERN_DEBUG "IRQ to pin mappings:\n");
@@ -1940,7 +1942,7 @@ void __init enable_IO_APIC(void)
 	if (!legacy_pic->nr_legacy_irqs)
 		return;
 
-	for(apic = 0; apic < nr_ioapics; apic++) {
+	for_each_ioapic(apic) {
 		int pin;
 		/* See if any of the pins is in ExtINT mode */
 		for (pin = 0; pin < ioapics[apic].nr_registers; pin++) {
@@ -2057,7 +2059,7 @@ void __init setup_ioapic_ids_from_mpc_nocheck(void)
 	/*
 	 * Set the IOAPIC ID to the value stored in the MPC table.
 	 */
-	for (ioapic_idx = 0; ioapic_idx < nr_ioapics; ioapic_idx++) {
+	for_each_ioapic(ioapic_idx) {
 		/* Read the register 0 value */
 		raw_spin_lock_irqsave(&ioapic_lock, flags);
 		reg_00.raw = io_apic_read(ioapic_idx, 0);
@@ -2987,7 +2989,8 @@ static void ioapic_resume(void)
 {
 	int ioapic_idx;
 
-	for (ioapic_idx = nr_ioapics - 1; ioapic_idx >= 0; ioapic_idx--)
+	/* really do that in reverse order ? */
+	for_each_ioapic(ioapic_idx)
 		resume_ioapic_id(ioapic_idx);
 
 	restore_ioapic_entries();
@@ -3635,9 +3638,9 @@ static u8 __init io_apic_unique_id(u8 id)
 	DECLARE_BITMAP(used, 256);
 
 	bitmap_zero(used, 256);
-	for (i = 0; i < nr_ioapics; i++) {
+	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);
@@ -3695,7 +3698,7 @@ void __init setup_ioapic_dest(void)
 	if (skip_ioapic_setup == 1)
 		return;
 
-	for (ioapic = 0; ioapic < nr_ioapics; ioapic++)
+	for_each_ioapic(ioapic)
 	for (pin = 0; pin < ioapics[ioapic].nr_registers; pin++) {
 		irq_entry = find_irq_entry(ioapic, pin, mp_INT);
 		if (irq_entry == -1)
@@ -3743,7 +3746,7 @@ static struct resource * __init ioapic_setup_resources(int nr_ioapics)
 
 	mem += sizeof(struct resource) * nr_ioapics;
 
-	for (i = 0; i < nr_ioapics; i++) {
+	for_each_ioapic(i) {
 		res[i].name = mem;
 		res[i].flags = IORESOURCE_MEM | IORESOURCE_BUSY;
 		snprintf(mem, IOAPIC_RESOURCE_NAME_SIZE, "IOAPIC %u", i);
@@ -3762,7 +3765,7 @@ void __init native_io_apic_init_mappings(void)
 	int i;
 
 	ioapic_res = ioapic_setup_resources(nr_ioapics);
-	for (i = 0; i < nr_ioapics; i++) {
+	for_each_ioapic(i) {
 		if (smp_found_config) {
 			ioapic_phys = mpc_ioapic_addr(i);
 #ifdef CONFIG_X86_32
@@ -3809,7 +3812,7 @@ void __init ioapic_insert_resources(void)
 		return;
 	}
 
-	for (i = 0; i < nr_ioapics; i++) {
+	for_each_ioapic(i) {
 		insert_resource(&iomem_resource, r);
 		r++;
 	}
@@ -3823,7 +3826,7 @@ int mp_find_ioapic(u32 gsi)
 		return -1;
 
 	/* Find the IOAPIC that manages this GSI. */
-	for (i = 0; i < nr_ioapics; i++) {
+	for_each_ioapic(i) {
 		struct mp_ioapic_gsi *gsi_cfg = mp_ioapic_gsi_routing(i);
 		if ((gsi >= gsi_cfg->gsi_base)
 		    && (gsi <= gsi_cfg->gsi_end))
@@ -3850,11 +3853,6 @@ int mp_find_ioapic_pin(int ioapic, u32 gsi)
 
 static __init 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;
@@ -3883,14 +3881,19 @@ static __init int bad_ioapic_register(int idx)
 
 void __init mp_register_ioapic(int id, u32 address, u32 gsi_base)
 {
-	int idx = 0;
+	int idx;
 	int entries;
 	struct mp_ioapic_gsi *gsi_cfg;
 
 	if (bad_ioapic(address))
 		return;
 
-	idx = nr_ioapics;
+	idx = find_first_zero_bit(ioapics_mask, MAX_IO_APICS);
+	if (idx >= MAX_IO_APICS) {
+		pr_warn("WARNING: Max # of I/O APICs (%d) exceeded, skipping\n",
+			MAX_IO_APICS);
+		return;
+	}
 
 	ioapics[idx].mp_config.type = MP_IOAPIC;
 	ioapics[idx].mp_config.flags = MPC_APIC_USABLE;
@@ -3928,7 +3931,8 @@ void __init mp_register_ioapic(int id, u32 address, u32 gsi_base)
 		mpc_ioapic_ver(idx), mpc_ioapic_addr(idx),
 		gsi_cfg->gsi_base, gsi_cfg->gsi_end);
 
-	nr_ioapics++;
+	set_bit(idx, ioapics_mask);
+	nr_ioapics = bitmap_weight(ioapics_mask, MAX_IO_APICS);
 }
 
 /* Enable IOAPIC early just for system timer */
diff --git a/arch/x86/kernel/devicetree.c b/arch/x86/kernel/devicetree.c
index 69eb2fa..c811e2f 100644
--- a/arch/x86/kernel/devicetree.c
+++ b/arch/x86/kernel/devicetree.c
@@ -393,7 +393,7 @@ static void __init ioapic_add_ofnode(struct device_node *np)
 		return;
 	}
 
-	for (i = 0; i < nr_ioapics; i++) {
+	for_each_ioapic(i) {
 		if (r.start == mpc_ioapic_addr(i)) {
 			dt_add_ioapic_domain(i, np);
 			return;
diff --git a/drivers/iommu/amd_iommu_init.c b/drivers/iommu/amd_iommu_init.c
index 7acbf35..7bbe9d2 100644
--- a/drivers/iommu/amd_iommu_init.c
+++ b/drivers/iommu/amd_iommu_init.c
@@ -1743,7 +1743,7 @@ static bool __init check_ioapic_information(void)
 	if (cmdline_maps)
 		fw_bug = "";
 
-	for (idx = 0; idx < nr_ioapics; idx++) {
+	for_each_ioapic(idx) {
 		int devid, id = mpc_ioapic_id(idx);
 
 		devid = get_ioapic_devid(id);
diff --git a/drivers/iommu/intel_irq_remapping.c b/drivers/iommu/intel_irq_remapping.c
index f71673d..84a6807 100644
--- a/drivers/iommu/intel_irq_remapping.c
+++ b/drivers/iommu/intel_irq_remapping.c
@@ -793,7 +793,7 @@ int __init parse_ioapics_under_ir(void)
 	if (!ir_supported)
 		return 0;
 
-	for (ioapic_idx = 0; ioapic_idx < nr_ioapics; ioapic_idx++) {
+	for_each_ioapic(ioapic_idx) {
 		int ioapic_id = mpc_ioapic_id(ioapic_idx);
 		if (!map_ioapic_to_ir(ioapic_id)) {
 			pr_err(FW_BUG "ioapic %d has no mapping iommu, "
-- 
1.8.1.4

[PATCH v4 20/28] x86, irq: More strict checking about registering ioapic

[Yinghai Lu]

1. check overlaping gsi range
for hot-add ioapic case, BIOS may have some entries in MADT
and also have setting in pci root bus with _GSB of DSDT.

2. check if entries is in right range.

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>
---
 arch/x86/kernel/apic/io_apic.c | 30 ++++++++++++++++++++++++------
 1 file changed, 24 insertions(+), 6 deletions(-)

diff --git a/arch/x86/kernel/apic/io_apic.c b/arch/x86/kernel/apic/io_apic.c
index b026cc7..60c6706 100644
--- a/arch/x86/kernel/apic/io_apic.c
+++ b/arch/x86/kernel/apic/io_apic.c
@@ -3818,12 +3818,9 @@ void __init ioapic_insert_resources(void)
 	}
 }
 
-int mp_find_ioapic(u32 gsi)
+static int __mp_find_ioapic(u32 gsi)
 {
-	int i = 0;
-
-	if (nr_ioapics == 0)
-		return -1;
+	int i;
 
 	/* Find the IOAPIC that manages this GSI. */
 	for_each_ioapic(i) {
@@ -3833,10 +3830,19 @@ int mp_find_ioapic(u32 gsi)
 			return i;
 	}
 
-	printk(KERN_ERR "ERROR: Unable to locate IOAPIC for GSI %d\n", gsi);
 	return -1;
 }
 
+int mp_find_ioapic(u32 gsi)
+{
+	int ret = __mp_find_ioapic(gsi);
+
+	if (ret == -1)
+		pr_err("ERROR: Unable to locate IOAPIC for GSI %d\n", gsi);
+
+	return ret;
+}
+
 int mp_find_ioapic_pin(int ioapic, u32 gsi)
 {
 	struct mp_ioapic_gsi *gsi_cfg;
@@ -3888,6 +3894,11 @@ void __init mp_register_ioapic(int id, u32 address, u32 gsi_base)
 	if (bad_ioapic(address))
 		return;
 
+	/* already registered ? */
+	idx = __mp_find_ioapic(gsi_base);
+	if (idx >= 0)
+		return;
+
 	idx = find_first_zero_bit(ioapics_mask, MAX_IO_APICS);
 	if (idx >= MAX_IO_APICS) {
 		pr_warn("WARNING: Max # of I/O APICs (%d) exceeded, skipping\n",
@@ -3914,6 +3925,13 @@ void __init 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);
+
+	if (!entries || entries > MP_MAX_IOAPIC_PIN) {
+		clear_fixmap(FIX_IO_APIC_BASE_0 + idx);
+		memset(&ioapics[idx], 0, sizeof(struct ioapic));
+		return;
+	}
+
 	gsi_cfg = mp_ioapic_gsi_routing(idx);
 	gsi_cfg->gsi_base = gsi_base;
 	gsi_cfg->gsi_end = gsi_base + entries - 1;
-- 
1.8.1.4

Re: [PATCH v4 20/28] x86, irq: More strict checking about registering ioapic

[Lan Tianyu]

2013/8/11 Yinghai Lu <yinghai@kernel.org>
>
> 1. check overlaping gsi range
> for hot-add ioapic case, BIOS may have some entries in MADT
> and also have setting in pci root bus with _GSB of DSDT.
>
> 2. check if entries is in right range.
>
> 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>
> ---
>  arch/x86/kernel/apic/io_apic.c | 30 ++++++++++++++++++++++++------
>  1 file changed, 24 insertions(+), 6 deletions(-)
>
> diff --git a/arch/x86/kernel/apic/io_apic.c b/arch/x86/kernel/apic/io_apic.c
> index b026cc7..60c6706 100644
> --- a/arch/x86/kernel/apic/io_apic.c
> +++ b/arch/x86/kernel/apic/io_apic.c
> @@ -3818,12 +3818,9 @@ void __init ioapic_insert_resources(void)
>         }
>  }
>
> -int mp_find_ioapic(u32 gsi)
> +static int __mp_find_ioapic(u32 gsi)
>  {
> -       int i = 0;
> -
> -       if (nr_ioapics == 0)
> -               return -1;
> +       int i;
>
>         /* Find the IOAPIC that manages this GSI. */
>         for_each_ioapic(i) {
> @@ -3833,10 +3830,19 @@ int mp_find_ioapic(u32 gsi)
>                         return i;
>         }
>
> -       printk(KERN_ERR "ERROR: Unable to locate IOAPIC for GSI %d\n", gsi);
>         return -1;
>  }
>
> +int mp_find_ioapic(u32 gsi)
> +{
> +       int ret = __mp_find_ioapic(gsi);
> +
> +       if (ret == -1)
> +               pr_err("ERROR: Unable to locate IOAPIC for GSI %d\n", gsi);
> +
> +       return ret;
> +}
> +
>  int mp_find_ioapic_pin(int ioapic, u32 gsi)
>  {
>         struct mp_ioapic_gsi *gsi_cfg;
> @@ -3888,6 +3894,11 @@ void __init mp_register_ioapic(int id, u32 address, u32 gsi_base)
>         if (bad_ioapic(address))
>                 return;
>
> +       /* already registered ? */
> +       idx = __mp_find_ioapic(gsi_base);
> +       if (idx >= 0)
> +               return;
> +
>         idx = find_first_zero_bit(ioapics_mask, MAX_IO_APICS);
>         if (idx >= MAX_IO_APICS) {
>                 pr_warn("WARNING: Max # of I/O APICs (%d) exceeded, skipping\n",
> @@ -3914,6 +3925,13 @@ void __init 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);

 "entries" is possible to be 0?  io_apic_get_redir_entries() returns
"reg_01.bits.entries + 1"
as "entries".

> +
> +       if (!entries || entries > MP_MAX_IOAPIC_PIN) {
> +               clear_fixmap(FIX_IO_APIC_BASE_0 + idx);
> +               memset(&ioapics[idx], 0, sizeof(struct ioapic));
> +               return;
> +       }
> +
>         gsi_cfg = mp_ioapic_gsi_routing(idx);
>         gsi_cfg->gsi_base = gsi_base;
>         gsi_cfg->gsi_end = gsi_base + entries - 1;
> --
> 1.8.1.4
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html




-- 
Best regards
Tianyu Lan

Re: [PATCH v4 20/28] x86, irq: More strict checking about registering ioapic

[Yijing Wang]

>  int mp_find_ioapic_pin(int ioapic, u32 gsi)
>  {
>  	struct mp_ioapic_gsi *gsi_cfg;
> @@ -3888,6 +3894,11 @@ void __init mp_register_ioapic(int id, u32 address, u32 gsi_base)
>  	if (bad_ioapic(address))
>  		return;
>  
> +	/* already registered ? */
> +	idx = __mp_find_ioapic(gsi_base);
> +	if (idx >= 0)
> +		return;

Hi Yinghai,
   If you add whether ioapic was already registered check here, so we will fail to add ioapic into global ioapic_list.
Then we also can not hot remove this ioapic.


1. We will call mp_register_ioapic during analyze MADT first.
2. We will try to add hotplug ioapic(which contain _GSB) to ioapic_list in acpi_pci_ioapic_add(), but will found this
ioapic has been registered.

Thanks!
Yijing.


> +
>  	idx = find_first_zero_bit(ioapics_mask, MAX_IO_APICS);
>  	if (idx >= MAX_IO_APICS) {
>  		pr_warn("WARNING: Max # of I/O APICs (%d) exceeded, skipping\n",
> @@ -3914,6 +3925,13 @@ void __init 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);
> +
> +	if (!entries || entries > MP_MAX_IOAPIC_PIN) {
> +		clear_fixmap(FIX_IO_APIC_BASE_0 + idx);
> +		memset(&ioapics[idx], 0, sizeof(struct ioapic));
> +		return;
> +	}
> +
>  	gsi_cfg = mp_ioapic_gsi_routing(idx);
>  	gsi_cfg->gsi_base = gsi_base;
>  	gsi_cfg->gsi_end = gsi_base + entries - 1;
> 


-- 
Thanks!
Yijing

[PATCH v4 21/28] x86, irq: Make mp_register_ioapic handle hot-added ioapic

[Yinghai Lu]

It needs to reserve irq range in allocated_irqs bitmaps
and irq_base will be used to get right irq for ioapic/pin or gsi.

also need to use lock to protect ioapics_mask to avoid racing.

Signed-off-by: Yinghai Lu <yinghai@kernel.org>
Cc: Paul Gortmaker <paul.gortmaker@windriver.com>
Cc: Joerg Roedel <joro@8bytes.org>
Cc: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
Cc: Sebastian Andrzej Siewior <sebastian@breakpoint.cc>
---
 arch/x86/include/asm/mpspec.h  |  1 +
 arch/x86/kernel/apic/io_apic.c | 68 ++++++++++++++++++++++++++++++++----------
 2 files changed, 54 insertions(+), 15 deletions(-)

diff --git a/arch/x86/include/asm/mpspec.h b/arch/x86/include/asm/mpspec.h
index 626cf70..13cf60c 100644
--- a/arch/x86/include/asm/mpspec.h
+++ b/arch/x86/include/asm/mpspec.h
@@ -96,6 +96,7 @@ static inline void early_reserve_e820_mpc_new(void) { }
 
 void generic_processor_info(int apicid, int version);
 #ifdef CONFIG_ACPI
+int __mp_register_ioapic(int id, u32 address, u32 gsi_base, bool hot);
 extern void mp_register_ioapic(int id, u32 address, u32 gsi_base);
 extern void mp_override_legacy_irq(u8 bus_irq, u8 polarity, u8 trigger,
 				   u32 gsi);
diff --git a/arch/x86/kernel/apic/io_apic.c b/arch/x86/kernel/apic/io_apic.c
index 60c6706..1616a35 100644
--- a/arch/x86/kernel/apic/io_apic.c
+++ b/arch/x86/kernel/apic/io_apic.c
@@ -94,6 +94,16 @@ static struct ioapic {
 } ioapics[MAX_IO_APICS];
 
 DECLARE_BITMAP(ioapics_mask, MAX_IO_APICS);
+static DEFINE_MUTEX(ioapics_mask_lock);
+
+static void lock_ioapics(void)
+{
+	mutex_lock(&ioapics_mask_lock);
+}
+static void unlock_ioapics(void)
+{
+	mutex_unlock(&ioapics_mask_lock);
+}
 
 #define mpc_ioapic_ver(ioapic_idx)	ioapics[ioapic_idx].mp_config.apicver
 
@@ -3885,25 +3895,28 @@ static __init int bad_ioapic_register(int idx)
 	return 0;
 }
 
-void __init mp_register_ioapic(int id, u32 address, u32 gsi_base)
+int __mp_register_ioapic(int id, u32 address, u32 gsi_base, bool hotadd)
 {
 	int idx;
 	int entries;
 	struct mp_ioapic_gsi *gsi_cfg;
+	int ret = -EINVAL;
 
 	if (bad_ioapic(address))
-		return;
+		return ret;
+
+	lock_ioapics();
 
 	/* already registered ? */
 	idx = __mp_find_ioapic(gsi_base);
 	if (idx >= 0)
-		return;
+		goto out;
 
 	idx = find_first_zero_bit(ioapics_mask, MAX_IO_APICS);
 	if (idx >= MAX_IO_APICS) {
 		pr_warn("WARNING: Max # of I/O APICs (%d) exceeded, skipping\n",
 			MAX_IO_APICS);
-		return;
+		goto out;
 	}
 
 	ioapics[idx].mp_config.type = MP_IOAPIC;
@@ -3912,10 +3925,8 @@ void __init mp_register_ioapic(int id, u32 address, u32 gsi_base)
 
 	set_fixmap_nocache(FIX_IO_APIC_BASE_0 + idx, address);
 
-	if (bad_ioapic_register(idx)) {
-		clear_fixmap(FIX_IO_APIC_BASE_0 + idx);
-		return;
-	}
+	if (bad_ioapic_register(idx))
+		goto clear_out;
 
 	ioapics[idx].mp_config.apicid = io_apic_unique_id(id);
 	ioapics[idx].mp_config.apicver = io_apic_get_version(idx);
@@ -3926,11 +3937,8 @@ void __init mp_register_ioapic(int id, u32 address, u32 gsi_base)
 	 */
 	entries = io_apic_get_redir_entries(idx);
 
-	if (!entries || entries > MP_MAX_IOAPIC_PIN) {
-		clear_fixmap(FIX_IO_APIC_BASE_0 + idx);
-		memset(&ioapics[idx], 0, sizeof(struct ioapic));
-		return;
-	}
+	if (!entries || entries > MP_MAX_IOAPIC_PIN)
+		goto clear_out;
 
 	gsi_cfg = mp_ioapic_gsi_routing(idx);
 	gsi_cfg->gsi_base = gsi_base;
@@ -3941,8 +3949,21 @@ void __init mp_register_ioapic(int id, u32 address, u32 gsi_base)
 	 */
 	ioapics[idx].nr_registers = entries;
 
-	if (gsi_cfg->gsi_end >= gsi_top)
-		gsi_top = gsi_cfg->gsi_end + 1;
+	if (!hotadd) {
+		/*
+		 * irqs will be reserved in arch_early_irq_init()
+		 * don't need to update gsi_top for hot add case
+		 */
+		if (gsi_cfg->gsi_end >= gsi_top)
+			gsi_top = gsi_cfg->gsi_end + 1;
+	} else {
+		int irq = reserve_ioapic_gsi_irq_base(idx);
+
+		if (irq < 0)
+			goto clear_out;
+
+		alloc_ioapic_saved_registers(idx);
+	}
 
 	pr_info("IOAPIC[%d]: apic_id %d, version %d, address 0x%x, GSI %d-%d\n",
 		idx, mpc_ioapic_id(idx),
@@ -3951,6 +3972,23 @@ void __init mp_register_ioapic(int id, u32 address, u32 gsi_base)
 
 	set_bit(idx, ioapics_mask);
 	nr_ioapics = bitmap_weight(ioapics_mask, MAX_IO_APICS);
+
+	unlock_ioapics();
+
+	return 0;
+
+clear_out:
+	clear_fixmap(FIX_IO_APIC_BASE_0 + idx);
+	memset(&ioapics[idx], 0, sizeof(struct ioapic));
+out:
+	unlock_ioapics();
+
+	return ret;
+}
+
+void mp_register_ioapic(int id, u32 address, u32 gsi_base)
+{
+	__mp_register_ioapic(id, address, gsi_base, false);
 }
 
 /* Enable IOAPIC early just for system timer */
-- 
1.8.1.4

[PATCH v4 22/28] x86, irq: Add mp_unregister_ioapic to handle hot-remove ioapic

[Yinghai Lu]

It will free ioapic related irq_desc and also clear allocated_irqs bits.

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>
---
 arch/x86/include/asm/mpspec.h  |  1 +
 arch/x86/kernel/apic/io_apic.c | 44 ++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 45 insertions(+)

diff --git a/arch/x86/include/asm/mpspec.h b/arch/x86/include/asm/mpspec.h
index 13cf60c..49f6ecc 100644
--- a/arch/x86/include/asm/mpspec.h
+++ b/arch/x86/include/asm/mpspec.h
@@ -97,6 +97,7 @@ static inline void early_reserve_e820_mpc_new(void) { }
 void generic_processor_info(int apicid, int version);
 #ifdef CONFIG_ACPI
 int __mp_register_ioapic(int id, u32 address, u32 gsi_base, bool hot);
+int mp_unregister_ioapic(u32 gsi_base);
 extern void mp_register_ioapic(int id, u32 address, u32 gsi_base);
 extern void mp_override_legacy_irq(u8 bus_irq, u8 polarity, u8 trigger,
 				   u32 gsi);
diff --git a/arch/x86/kernel/apic/io_apic.c b/arch/x86/kernel/apic/io_apic.c
index 1616a35..a75e9d8 100644
--- a/arch/x86/kernel/apic/io_apic.c
+++ b/arch/x86/kernel/apic/io_apic.c
@@ -334,6 +334,20 @@ static void __init reserve_ioapic_gsi_irq_extra(void)
 	}
 }
 
+static void free_ioapic_gsi_irq_base(int idx)
+{
+	int i;
+	struct mp_ioapic_gsi *gsi_cfg = mp_ioapic_gsi_routing(idx);
+	int irq_base = gsi_cfg->irq_base;
+	int irq_cnt = gsi_cfg->gsi_end - gsi_cfg->gsi_base + 1;
+
+	if (irq_base < 0)
+		return;
+
+	for (i = irq_base; i < irq_base + irq_cnt; i++)
+		free_irq_at(i, irq_get_chip_data(i));
+}
+
 static void alloc_ioapic_saved_registers(int idx)
 {
 	if (ioapics[idx].saved_registers)
@@ -347,6 +361,11 @@ 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);
+}
+
 int __init arch_early_irq_init(void)
 {
 	int node = cpu_to_node(0);
@@ -3991,6 +4010,31 @@ void mp_register_ioapic(int id, u32 address, u32 gsi_base)
 	__mp_register_ioapic(id, address, gsi_base, false);
 }
 
+int mp_unregister_ioapic(u32 gsi_base)
+{
+	int idx;
+
+	lock_ioapics();
+
+	idx = __mp_find_ioapic(gsi_base);
+	if (idx < 0) {
+		unlock_ioapics();
+		return -EINVAL;
+	}
+
+	free_ioapic_saved_registers(idx);
+
+	free_ioapic_gsi_irq_base(idx);
+
+	clear_fixmap(FIX_IO_APIC_BASE_0 + idx);
+	memset(&ioapics[idx], 0, sizeof(struct ioapic));
+	clear_bit(idx, ioapics_mask);
+
+	unlock_ioapics();
+
+	return 0;
+}
+
 /* Enable IOAPIC early just for system timer */
 void __init pre_init_apic_IRQ0(void)
 {
-- 
1.8.1.4

[PATCH v4 23/28] x86, ioapic: Find usable ioapic id for 64bit.

[Yinghai Lu]

Checking the id in register, if that is duplicated, will pick one and
update id register.

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>
---
 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 a75e9d8..fb9bf06 100644
--- a/arch/x86/kernel/apic/io_apic.c
+++ b/arch/x86/kernel/apic/io_apic.c
@@ -3652,19 +3652,22 @@ 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 __init 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)
@@ -3672,7 +3675,32 @@ static u8 __init io_apic_unique_id(u8 id)
 
 	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
 
@@ -3947,7 +3975,7 @@ int __mp_register_ioapic(int id, u32 address, u32 gsi_base, bool hotadd)
 	if (bad_ioapic_register(idx))
 		goto clear_out;
 
-	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.8.1.4

[PATCH v4 24/28] x86: Move declaration for mp_register_ioapic()

[Yinghai Lu]

Address compiling problem that Fengguang report.

Reported-by: Fengguang Wu <wfg@linux.intel.com>
Signed-off-by: Yinghai Lu <yinghai@kernel.org>
---
 arch/x86/include/asm/mpspec.h | 19 ++++++++++++++++++-
 1 file changed, 18 insertions(+), 1 deletion(-)

diff --git a/arch/x86/include/asm/mpspec.h b/arch/x86/include/asm/mpspec.h
index 49f6ecc..a57f113 100644
--- a/arch/x86/include/asm/mpspec.h
+++ b/arch/x86/include/asm/mpspec.h
@@ -95,10 +95,27 @@ static inline void early_reserve_e820_mpc_new(void) { }
 #endif
 
 void generic_processor_info(int apicid, int version);
-#ifdef CONFIG_ACPI
+
+#ifdef CONFIG_X86_IO_APIC
 int __mp_register_ioapic(int id, u32 address, u32 gsi_base, bool hot);
 int mp_unregister_ioapic(u32 gsi_base);
 extern void mp_register_ioapic(int id, u32 address, u32 gsi_base);
+#else
+static inline int __mp_register_ioapic(int id, u32 address, u32 gsi_base,
+					 bool hot)
+{
+	return 0;
+}
+static inline int mp_unregister_ioapic(u32 gsi_base)
+{
+	return 0;
+}
+static inline void mp_register_ioapic(int id, u32 address, u32 gsi_base)
+{
+}
+#endif
+
+#ifdef CONFIG_ACPI
 extern void mp_override_legacy_irq(u8 bus_irq, u8 polarity, u8 trigger,
 				   u32 gsi);
 extern void mp_config_acpi_legacy_irqs(void);
-- 
1.8.1.4

[PATCH v4 25/28] PCI, x86: Make ioapic hotplug support built-in

[Yinghai Lu]

ioapic hotplug should be built-in like pci root bus hotplug.

Also need to make it depends on X86_IO_APIC.

Signed-off-by: Yinghai Lu <yinghai@kernel.org>
---
 drivers/pci/Kconfig  | 3 ++-
 drivers/pci/ioapic.c | 6 +++++-
 2 files changed, 7 insertions(+), 2 deletions(-)

diff --git a/drivers/pci/Kconfig b/drivers/pci/Kconfig
index 81944fb..6c9ede2 100644
--- a/drivers/pci/Kconfig
+++ b/drivers/pci/Kconfig
@@ -109,9 +109,10 @@ config PCI_PASID
 	  If unsure, say N.
 
 config PCI_IOAPIC
-	tristate "PCI IO-APIC hotplug support" if X86
+	bool "PCI IO-APIC hotplug support" if X86
 	depends on PCI
 	depends on ACPI
+	depends on X86_IO_APIC
 	default !X86
 
 config PCI_LABEL
diff --git a/drivers/pci/ioapic.c b/drivers/pci/ioapic.c
index 1b90579..7d6b157 100644
--- a/drivers/pci/ioapic.c
+++ b/drivers/pci/ioapic.c
@@ -113,6 +113,10 @@ static struct pci_driver ioapic_driver = {
 	.remove		= ioapic_remove,
 };
 
-module_pci_driver(ioapic_driver);
+static int __init ioapic_init(void)
+{
+	return pci_register_driver(&ioapic_driver);
+}
+module_init(ioapic_init);
 
 MODULE_LICENSE("GPL");
-- 
1.8.1.4

Re: [PATCH v4 25/28] PCI, x86: Make ioapic hotplug support built-in

[Bjorn Helgaas]

On Sat, Aug 10, 2013 at 07:48:11PM -0700, Yinghai Lu wrote:
> ioapic hotplug should be built-in like pci root bus hotplug.
> 
> Also need to make it depends on X86_IO_APIC.
> 
> Signed-off-by: Yinghai Lu <yinghai@kernel.org>

Hi Yinghai,

What's the status of these?  It looks like the last three or four
could go via my PCI tree, but I don't know whether they depend on
the earlier patches.  If they could be applied separately, I could
merge them and we could make *some* progress.

Also, it looks like you planned some rework related to _UID and
resource parsing, so I've been waiting for that to show up before
doing much more.

Bjorn

> ---
>  drivers/pci/Kconfig  | 3 ++-
>  drivers/pci/ioapic.c | 6 +++++-
>  2 files changed, 7 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/pci/Kconfig b/drivers/pci/Kconfig
> index 81944fb..6c9ede2 100644
> --- a/drivers/pci/Kconfig
> +++ b/drivers/pci/Kconfig
> @@ -109,9 +109,10 @@ config PCI_PASID
>  	  If unsure, say N.
>  
>  config PCI_IOAPIC
> -	tristate "PCI IO-APIC hotplug support" if X86
> +	bool "PCI IO-APIC hotplug support" if X86
>  	depends on PCI
>  	depends on ACPI
> +	depends on X86_IO_APIC
>  	default !X86
>  
>  config PCI_LABEL
> diff --git a/drivers/pci/ioapic.c b/drivers/pci/ioapic.c
> index 1b90579..7d6b157 100644
> --- a/drivers/pci/ioapic.c
> +++ b/drivers/pci/ioapic.c
> @@ -113,6 +113,10 @@ static struct pci_driver ioapic_driver = {
>  	.remove		= ioapic_remove,
>  };
>  
> -module_pci_driver(ioapic_driver);
> +static int __init ioapic_init(void)
> +{
> +	return pci_register_driver(&ioapic_driver);
> +}
> +module_init(ioapic_init);
>  
>  MODULE_LICENSE("GPL");
> -- 
> 1.8.1.4
> 

Re: [PATCH v4 25/28] PCI, x86: Make ioapic hotplug support built-in

[Yinghai Lu]

On Tue, Oct 1, 2013 at 9:00 AM, Bjorn Helgaas <bhelgaas@google.com> wrote:
> On Sat, Aug 10, 2013 at 07:48:11PM -0700, Yinghai Lu wrote:
>> ioapic hotplug should be built-in like pci root bus hotplug.
>>
>> Also need to make it depends on X86_IO_APIC.
>>
>> Signed-off-by: Yinghai Lu <yinghai@kernel.org>
>
> Hi Yinghai,
>
> What's the status of these?  It looks like the last three or four
> could go via my PCI tree, but I don't know whether they depend on
> the earlier patches.  If they could be applied separately, I could
> merge them and we could make *some* progress.

they are rely on other patches.

would be better if you give them ack, and let them via tip/x86/irq etc.

or in two parts: after first part go to upstream via tip, and we can
push left pci related via pci tree.

>
> Also, it looks like you planned some rework related to _UID and
> resource parsing, so I've been waiting for that to show up before
> doing much more.

yes, I have that ready. will post them shortly.

Thanks

Yinghai

[PATCH v4 26/28] PCI, x86, ACPI: Link acpi ioapic register to ioapic

[Yinghai Lu]

During ioapic hotplug, acpi_register_ioapic will be called.
Now for x86, that function is blank.
Fill that will update __mp_register_ioapic to use those ioapic.

Signed-off-by: Yinghai Lu <yinghai@kernel.org>
---
 arch/x86/kernel/acpi/boot.c | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/arch/x86/kernel/acpi/boot.c b/arch/x86/kernel/acpi/boot.c
index 5be15d1..4a5336d 100644
--- a/arch/x86/kernel/acpi/boot.c
+++ b/arch/x86/kernel/acpi/boot.c
@@ -707,16 +707,18 @@ EXPORT_SYMBOL(acpi_unmap_lsapic);
 
 int acpi_register_ioapic(acpi_handle handle, u64 phys_addr, u32 gsi_base)
 {
-	/* TBD */
-	return -EINVAL;
+	unsigned long long id = 0;
+
+	acpi_evaluate_integer(handle, "_UID", NULL, &id);
+
+	return	__mp_register_ioapic(id, phys_addr, gsi_base, true);
 }
 
 EXPORT_SYMBOL(acpi_register_ioapic);
 
 int acpi_unregister_ioapic(acpi_handle handle, u32 gsi_base)
 {
-	/* TBD */
-	return -EINVAL;
+	return mp_unregister_ioapic(gsi_base);
 }
 
 EXPORT_SYMBOL(acpi_unregister_ioapic);
-- 
1.8.1.4

Re: [PATCH v4 26/28] PCI, x86, ACPI: Link acpi ioapic register to ioapic

[rui wang]

On 8/11/13, Yinghai Lu <yinghai@kernel.org> wrote:
> During ioapic hotplug, acpi_register_ioapic will be called.
> Now for x86, that function is blank.
> Fill that will update __mp_register_ioapic to use those ioapic.
>
> Signed-off-by: Yinghai Lu <yinghai@kernel.org>
> ---
>  arch/x86/kernel/acpi/boot.c | 10 ++++++----
>  1 file changed, 6 insertions(+), 4 deletions(-)
>
> diff --git a/arch/x86/kernel/acpi/boot.c b/arch/x86/kernel/acpi/boot.c
> index 5be15d1..4a5336d 100644
> --- a/arch/x86/kernel/acpi/boot.c
> +++ b/arch/x86/kernel/acpi/boot.c
> @@ -707,16 +707,18 @@ EXPORT_SYMBOL(acpi_unmap_lsapic);
>
>  int acpi_register_ioapic(acpi_handle handle, u64 phys_addr, u32 gsi_base)
>  {
> -	/* TBD */
> -	return -EINVAL;
> +	unsigned long long id = 0;
> +
> +	acpi_evaluate_integer(handle, "_UID", NULL, &id);

We cannot assume that "_UID" is an integer. "_UID" can be either an
integer or a string. Need to consider the case when it is a string.

Thanks
Rui

> +
> +	return	__mp_register_ioapic(id, phys_addr, gsi_base, true);
>  }
>
>  EXPORT_SYMBOL(acpi_register_ioapic);
>
>  int acpi_unregister_ioapic(acpi_handle handle, u32 gsi_base)
>  {
> -	/* TBD */
> -	return -EINVAL;
> +	return mp_unregister_ioapic(gsi_base);
>  }
>
>  EXPORT_SYMBOL(acpi_unregister_ioapic);
> --
> 1.8.1.4
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/
>

Re: [PATCH v4 26/28] PCI, x86, ACPI: Link acpi ioapic register to ioapic

[Yinghai Lu]

On Fri, Aug 23, 2013 at 5:51 PM, rui wang <ruiv.wang@gmail.com> wrote:
> On 8/11/13, Yinghai Lu <yinghai@kernel.org> wrote:
>> During ioapic hotplug, acpi_register_ioapic will be called.
>> Now for x86, that function is blank.
>> Fill that will update __mp_register_ioapic to use those ioapic.
>>
>> Signed-off-by: Yinghai Lu <yinghai@kernel.org>
>> ---
>>  arch/x86/kernel/acpi/boot.c | 10 ++++++----
>>  1 file changed, 6 insertions(+), 4 deletions(-)
>>
>> diff --git a/arch/x86/kernel/acpi/boot.c b/arch/x86/kernel/acpi/boot.c
>> index 5be15d1..4a5336d 100644
>> --- a/arch/x86/kernel/acpi/boot.c
>> +++ b/arch/x86/kernel/acpi/boot.c
>> @@ -707,16 +707,18 @@ EXPORT_SYMBOL(acpi_unmap_lsapic);
>>
>>  int acpi_register_ioapic(acpi_handle handle, u64 phys_addr, u32 gsi_base)
>>  {
>> -     /* TBD */
>> -     return -EINVAL;
>> +     unsigned long long id = 0;
>> +
>> +     acpi_evaluate_integer(handle, "_UID", NULL, &id);
>
> We cannot assume that "_UID" is an integer. "_UID" can be either an
> integer or a string. Need to consider the case when it is a string.

Yes.

But do you have test case for ioapic that _UID is not integer?

Thanks

Yinghai

Re: [PATCH v4 26/28] PCI, x86, ACPI: Link acpi ioapic register to ioapic

[rui wang]

On 8/27/13, Yinghai Lu <yinghai@kernel.org> wrote:
> On Fri, Aug 23, 2013 at 5:51 PM, rui wang <ruiv.wang@gmail.com> wrote:
>> On 8/11/13, Yinghai Lu <yinghai@kernel.org> wrote:
>>> During ioapic hotplug, acpi_register_ioapic will be called.
>>> Now for x86, that function is blank.
>>> Fill that will update __mp_register_ioapic to use those ioapic.
>>>
>>> Signed-off-by: Yinghai Lu <yinghai@kernel.org>
>>> ---
>>>  arch/x86/kernel/acpi/boot.c | 10 ++++++----
>>>  1 file changed, 6 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/arch/x86/kernel/acpi/boot.c b/arch/x86/kernel/acpi/boot.c
>>> index 5be15d1..4a5336d 100644
>>> --- a/arch/x86/kernel/acpi/boot.c
>>> +++ b/arch/x86/kernel/acpi/boot.c
>>> @@ -707,16 +707,18 @@ EXPORT_SYMBOL(acpi_unmap_lsapic);
>>>
>>>  int acpi_register_ioapic(acpi_handle handle, u64 phys_addr, u32
>>> gsi_base)
>>>  {
>>> -     /* TBD */
>>> -     return -EINVAL;
>>> +     unsigned long long id = 0;
>>> +
>>> +     acpi_evaluate_integer(handle, "_UID", NULL, &id);
>>
>> We cannot assume that "_UID" is an integer. "_UID" can be either an
>> integer or a string. Need to consider the case when it is a string.
>
> Yes.
>
> But do you have test case for ioapic that _UID is not integer?
>

Yes I have an IOAPIC with an _UID of:

        Name (_UID, "IOAPIC01")  // _UID: Unique ID

It's a bit challenge how you'll conjure unique identifiers from both
integers and strings. If you have a new patch I can help try it out.

Thanks
Rui

> Thanks
>
> Yinghai
>

Re: [PATCH v4 26/28] PCI, x86, ACPI: Link acpi ioapic register to ioapic

[Yinghai Lu]

On Mon, Aug 26, 2013 at 11:44 PM, rui wang <ruiv.wang@gmail.com> wrote:
> On 8/27/13, Yinghai Lu <yinghai@kernel.org> wrote:
>> On Fri, Aug 23, 2013 at 5:51 PM, rui wang <ruiv.wang@gmail.com> wrote:
>>> On 8/11/13, Yinghai Lu <yinghai@kernel.org> wrote:
>>>> During ioapic hotplug, acpi_register_ioapic will be called.
>>>> Now for x86, that function is blank.
>>>> Fill that will update __mp_register_ioapic to use those ioapic.
>>>>
>>>> Signed-off-by: Yinghai Lu <yinghai@kernel.org>
>>>> ---
>>>>  arch/x86/kernel/acpi/boot.c | 10 ++++++----
>>>>  1 file changed, 6 insertions(+), 4 deletions(-)
>>>>
>>>> diff --git a/arch/x86/kernel/acpi/boot.c b/arch/x86/kernel/acpi/boot.c
>>>> index 5be15d1..4a5336d 100644
>>>> --- a/arch/x86/kernel/acpi/boot.c
>>>> +++ b/arch/x86/kernel/acpi/boot.c
>>>> @@ -707,16 +707,18 @@ EXPORT_SYMBOL(acpi_unmap_lsapic);
>>>>
>>>>  int acpi_register_ioapic(acpi_handle handle, u64 phys_addr, u32
>>>> gsi_base)
>>>>  {
>>>> -     /* TBD */
>>>> -     return -EINVAL;
>>>> +     unsigned long long id = 0;
>>>> +
>>>> +     acpi_evaluate_integer(handle, "_UID", NULL, &id);
>>>
>>> We cannot assume that "_UID" is an integer. "_UID" can be either an
>>> integer or a string. Need to consider the case when it is a string.
>>
>> Yes.
>>
>> But do you have test case for ioapic that _UID is not integer?
>>
>
> Yes I have an IOAPIC with an _UID of:
>
>         Name (_UID, "IOAPIC01")  // _UID: Unique ID
>
> It's a bit challenge how you'll conjure unique identifiers from both
> integers and strings. If you have a new patch I can help try it out.

Then do you have _MAT with that ioapic device or else?

need to find the _MAT entry with GSM matching to find out ioapic id.


Thanks

Yinghai

Re: [PATCH v4 26/28] PCI, x86, ACPI: Link acpi ioapic register to ioapic

[rui wang]

On 8/27/13, Yinghai Lu <yinghai@kernel.org> wrote:
> On Mon, Aug 26, 2013 at 11:44 PM, rui wang <ruiv.wang@gmail.com> wrote:
>> On 8/27/13, Yinghai Lu <yinghai@kernel.org> wrote:
>>> On Fri, Aug 23, 2013 at 5:51 PM, rui wang <ruiv.wang@gmail.com> wrote:
>>>> On 8/11/13, Yinghai Lu <yinghai@kernel.org> wrote:
>>>>> During ioapic hotplug, acpi_register_ioapic will be called.
>>>>> Now for x86, that function is blank.
>>>>> Fill that will update __mp_register_ioapic to use those ioapic.
>>>>>
>>>>> Signed-off-by: Yinghai Lu <yinghai@kernel.org>
>>>>> ---
>>>>>  arch/x86/kernel/acpi/boot.c | 10 ++++++----
>>>>>  1 file changed, 6 insertions(+), 4 deletions(-)
>>>>>
>>>>> diff --git a/arch/x86/kernel/acpi/boot.c b/arch/x86/kernel/acpi/boot.c
>>>>> index 5be15d1..4a5336d 100644
>>>>> --- a/arch/x86/kernel/acpi/boot.c
>>>>> +++ b/arch/x86/kernel/acpi/boot.c
>>>>> @@ -707,16 +707,18 @@ EXPORT_SYMBOL(acpi_unmap_lsapic);
>>>>>
>>>>>  int acpi_register_ioapic(acpi_handle handle, u64 phys_addr, u32
>>>>> gsi_base)
>>>>>  {
>>>>> -     /* TBD */
>>>>> -     return -EINVAL;
>>>>> +     unsigned long long id = 0;
>>>>> +
>>>>> +     acpi_evaluate_integer(handle, "_UID", NULL, &id);
>>>>
>>>> We cannot assume that "_UID" is an integer. "_UID" can be either an
>>>> integer or a string. Need to consider the case when it is a string.
>>>
>>> Yes.
>>>
>>> But do you have test case for ioapic that _UID is not integer?
>>>
>>
>> Yes I have an IOAPIC with an _UID of:
>>
>>         Name (_UID, "IOAPIC01")  // _UID: Unique ID
>>
>> It's a bit challenge how you'll conjure unique identifiers from both
>> integers and strings. If you have a new patch I can help try it out.
>
> Then do you have _MAT with that ioapic device or else?
>
> need to find the _MAT entry with GSM matching to find out ioapic id.
>

There's no _MAT for the IOAPICs. I only find _MAT for the LAPICs. But
there's a static MADT in APIC.dsl which contains IOAPIC entries with
ioapic ids.

Thanks
Rui

>
> Thanks
>
> Yinghai
>

Re: [PATCH v4 26/28] PCI, x86, ACPI: Link acpi ioapic register to ioapic

[Yinghai Lu]

On Tue, Aug 27, 2013 at 3:08 AM, rui wang <ruiv.wang@gmail.com> wrote:
> On 8/27/13, Yinghai Lu <yinghai@kernel.org> wrote:
>>> It's a bit challenge how you'll conjure unique identifiers from both
>>> integers and strings. If you have a new patch I can help try it out.
>>
>> Then do you have _MAT with that ioapic device or else?
>>
>> need to find the _MAT entry with GSM matching to find out ioapic id.
>>
>
> There's no _MAT for the IOAPICs. I only find _MAT for the LAPICs. But
> there's a static MADT in APIC.dsl which contains IOAPIC entries with
> ioapic ids.

ok, will change to check _MAT at first and fall back to MADT.

Yinghai

[PATCH v4 27/28] PCI, x86, ACPI: Enable ioapic hotplug support with acpi host bridge.

[Yinghai Lu]

We need to have ioapic setup before normal pci drivers.
otherwise other pci driver can not setup irq.

So we should not treat them as normal pci devices.
Also we will need to support ioapic hotplug without pci device around.

We need to call ioapic add/remove during host-bridge add/remove.

Signed-off-by: Yinghai Lu <yinghai@kernel.org>
---
 drivers/acpi/pci_root.c  |   4 ++
 drivers/pci/ioapic.c     | 147 ++++++++++++++++++++++++++++++-----------------
 include/linux/pci-acpi.h |   8 +++
 3 files changed, 106 insertions(+), 53 deletions(-)

diff --git a/drivers/acpi/pci_root.c b/drivers/acpi/pci_root.c
index 5917839..7577175 100644
--- a/drivers/acpi/pci_root.c
+++ b/drivers/acpi/pci_root.c
@@ -532,6 +532,8 @@ static int acpi_pci_root_add(struct acpi_device *device,
 		pci_enable_bridges(root->bus);
 	}
 
+	acpi_pci_ioapic_add(root);
+
 	pci_bus_add_devices(root->bus);
 	return 1;
 
@@ -546,6 +548,8 @@ static void acpi_pci_root_remove(struct acpi_device *device)
 
 	pci_stop_root_bus(root->bus);
 
+	acpi_pci_ioapic_remove(root);
+
 	device_set_run_wake(root->bus->bridge, false);
 	pci_acpi_remove_bus_pm_notifier(device);
 
diff --git a/drivers/pci/ioapic.c b/drivers/pci/ioapic.c
index 7d6b157..60351b2 100644
--- a/drivers/pci/ioapic.c
+++ b/drivers/pci/ioapic.c
@@ -22,101 +22,142 @@
 #include <linux/slab.h>
 #include <acpi/acpi_bus.h>
 
-struct ioapic {
-	acpi_handle	handle;
+struct acpi_pci_ioapic {
+	acpi_handle	root_handle;
 	u32		gsi_base;
+	struct pci_dev *pdev;
+	struct list_head list;
 };
 
-static int ioapic_probe(struct pci_dev *dev, const struct pci_device_id *ent)
+static LIST_HEAD(ioapic_list);
+static DEFINE_MUTEX(ioapic_list_lock);
+
+static void handle_ioapic_add(acpi_handle handle, struct pci_dev **pdev,
+				 u32 *pgsi_base)
 {
-	acpi_handle handle;
 	acpi_status status;
 	unsigned long long gsb;
-	struct ioapic *ioapic;
+	struct pci_dev *dev;
+	u32 gsi_base;
 	int ret;
 	char *type;
-	struct resource *res;
+	struct resource r;
+	struct resource *res = &r;
+	char objname[64];
+	struct acpi_buffer buffer = {sizeof(objname), objname};
 
-	handle = DEVICE_ACPI_HANDLE(&dev->dev);
-	if (!handle)
-		return -EINVAL;
+	*pdev = NULL;
+	*pgsi_base = 0;
 
 	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.
-	 */
+	if (ACPI_FAILURE(status) || !gsb)
+		return;
 
-	ioapic = kzalloc(sizeof(*ioapic), GFP_KERNEL);
-	if (!ioapic)
-		return -ENOMEM;
+	dev = acpi_get_pci_dev(handle);
+	if (!dev)
+		return;
 
-	ioapic->handle = handle;
-	ioapic->gsi_base = (u32) gsb;
+	acpi_get_name(handle, ACPI_FULL_PATHNAME, &buffer);
 
-	if (dev->class == PCI_CLASS_SYSTEM_PIC_IOAPIC)
-		type = "IOAPIC";
-	else
-		type = "IOxAPIC";
+	gsi_base = gsb;
+	type = "IOxAPIC";
 
 	ret = pci_enable_device(dev);
 	if (ret < 0)
-		goto exit_free;
+		goto exit_put;
 
 	pci_set_master(dev);
 
+	if (dev->class == PCI_CLASS_SYSTEM_PIC_IOAPIC)
+		type = "IOAPIC";
+
 	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))
+
+	if (acpi_register_ioapic(handle, res->start, 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;
+	pr_info("%s %s %s at %pR, GSI %u\n",
+		dev_name(&dev->dev), objname, type,
+		res, gsi_base);
+
+	*pdev = dev;
+	*pgsi_base = gsi_base;
+	return;
 
 exit_release:
 	pci_release_region(dev, 0);
 exit_disable:
 	pci_disable_device(dev);
-exit_free:
-	kfree(ioapic);
-	return -ENODEV;
+exit_put:
+	pci_dev_put(dev);
 }
 
-static void ioapic_remove(struct pci_dev *dev)
+static void handle_ioapic_remove(acpi_handle handle, struct pci_dev *dev,
+				  u32 gsi_base)
 {
-	struct ioapic *ioapic = pci_get_drvdata(dev);
+	acpi_unregister_ioapic(handle, gsi_base);
 
-	acpi_unregister_ioapic(ioapic->handle, ioapic->gsi_base);
 	pci_release_region(dev, 0);
 	pci_disable_device(dev);
-	kfree(ioapic);
+	pci_dev_put(dev);
 }
 
+static acpi_status register_ioapic(acpi_handle handle, u32 lvl,
+					void *context, void **rv)
+{
+	acpi_handle root_handle = context;
+	struct pci_dev *pdev;
+	u32 gsi_base;
+	struct acpi_pci_ioapic *ioapic;
 
-static DEFINE_PCI_DEVICE_TABLE(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);
+	handle_ioapic_add(handle, &pdev, &gsi_base);
+	if (!gsi_base)
+		return AE_OK;
 
-static struct pci_driver ioapic_driver = {
-	.name		= "ioapic",
-	.id_table	= ioapic_devices,
-	.probe		= ioapic_probe,
-	.remove		= ioapic_remove,
-};
+	ioapic = kzalloc(sizeof(*ioapic), GFP_KERNEL);
+	if (!ioapic) {
+		pr_err("%s: cannot allocate memory\n", __func__);
+		handle_ioapic_remove(root_handle, pdev, gsi_base);
+		return AE_OK;
+	}
+	ioapic->root_handle = root_handle;
+	ioapic->pdev = pdev;
+	ioapic->gsi_base = gsi_base;
+
+	mutex_lock(&ioapic_list_lock);
+	list_add(&ioapic->list, &ioapic_list);
+	mutex_unlock(&ioapic_list_lock);
+
+	return AE_OK;
+}
 
-static int __init ioapic_init(void)
+void acpi_pci_ioapic_add(struct acpi_pci_root *root)
 {
-	return pci_register_driver(&ioapic_driver);
+	acpi_status status;
+
+	status = acpi_walk_namespace(ACPI_TYPE_DEVICE, root->device->handle,
+				     (u32)1, register_ioapic, NULL,
+				     root->device->handle,
+				     NULL);
+	if (ACPI_FAILURE(status))
+		pr_err("%s: register_ioapic failure - %d", __func__, status);
 }
-module_init(ioapic_init);
 
-MODULE_LICENSE("GPL");
+void acpi_pci_ioapic_remove(struct acpi_pci_root *root)
+{
+	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;
+		list_del(&ioapic->list);
+		handle_ioapic_remove(ioapic->root_handle, ioapic->pdev,
+					ioapic->gsi_base);
+		kfree(ioapic);
+	}
+	mutex_unlock(&ioapic_list_lock);
+}
diff --git a/include/linux/pci-acpi.h b/include/linux/pci-acpi.h
index 1704479..b2a2ced 100644
--- a/include/linux/pci-acpi.h
+++ b/include/linux/pci-acpi.h
@@ -69,6 +69,14 @@ static inline void acpiphp_remove_slots(struct pci_bus *bus) { }
 static inline void acpiphp_check_host_bridge(acpi_handle handle) { }
 #endif
 
+#ifdef CONFIG_PCI_IOAPIC
+void acpi_pci_ioapic_add(struct acpi_pci_root *root);
+void acpi_pci_ioapic_remove(struct acpi_pci_root *root);
+#else
+static inline void acpi_pci_ioapic_add(struct acpi_pci_root *root) { }
+static inline void acpi_pci_ioapic_remove(struct acpi_pci_root *root) { }
+#endif
+
 #else	/* CONFIG_ACPI */
 static inline void acpi_pci_add_bus(struct pci_bus *bus) { }
 static inline void acpi_pci_remove_bus(struct pci_bus *bus) { }
-- 
1.8.1.4

Re: [PATCH v4 27/28] PCI, x86, ACPI: Enable ioapic hotplug support with acpi host bridge.

[rui wang]

On 8/11/13, Yinghai Lu <yinghai@kernel.org> wrote:
> We need to have ioapic setup before normal pci drivers.
> otherwise other pci driver can not setup irq.
>
> So we should not treat them as normal pci devices.
> Also we will need to support ioapic hotplug without pci device around.
>
> We need to call ioapic add/remove during host-bridge add/remove.
>
> Signed-off-by: Yinghai Lu <yinghai@kernel.org>
> ---
>  drivers/acpi/pci_root.c  |   4 ++
>  drivers/pci/ioapic.c     | 147
> ++++++++++++++++++++++++++++++-----------------
>  include/linux/pci-acpi.h |   8 +++
>  3 files changed, 106 insertions(+), 53 deletions(-)
>
> diff --git a/drivers/acpi/pci_root.c b/drivers/acpi/pci_root.c
> index 5917839..7577175 100644
> --- a/drivers/acpi/pci_root.c
> +++ b/drivers/acpi/pci_root.c
> @@ -532,6 +532,8 @@ static int acpi_pci_root_add(struct acpi_device
> *device,
>  		pci_enable_bridges(root->bus);
>  	}
>
> +	acpi_pci_ioapic_add(root);
> +
>  	pci_bus_add_devices(root->bus);
>  	return 1;
>
> @@ -546,6 +548,8 @@ static void acpi_pci_root_remove(struct acpi_device
> *device)
>
>  	pci_stop_root_bus(root->bus);
>
> +	acpi_pci_ioapic_remove(root);
> +
>  	device_set_run_wake(root->bus->bridge, false);
>  	pci_acpi_remove_bus_pm_notifier(device);
>
> diff --git a/drivers/pci/ioapic.c b/drivers/pci/ioapic.c
> index 7d6b157..60351b2 100644
> --- a/drivers/pci/ioapic.c
> +++ b/drivers/pci/ioapic.c
> @@ -22,101 +22,142 @@
>  #include <linux/slab.h>
>  #include <acpi/acpi_bus.h>
>
> -struct ioapic {
> -	acpi_handle	handle;
> +struct acpi_pci_ioapic {
> +	acpi_handle	root_handle;
>  	u32		gsi_base;
> +	struct pci_dev *pdev;
> +	struct list_head list;
>  };
>
> -static int ioapic_probe(struct pci_dev *dev, const struct pci_device_id
> *ent)
> +static LIST_HEAD(ioapic_list);
> +static DEFINE_MUTEX(ioapic_list_lock);
> +
> +static void handle_ioapic_add(acpi_handle handle, struct pci_dev **pdev,
> +				 u32 *pgsi_base)
>  {
> -	acpi_handle handle;
>  	acpi_status status;
>  	unsigned long long gsb;
> -	struct ioapic *ioapic;
> +	struct pci_dev *dev;
> +	u32 gsi_base;
>  	int ret;
>  	char *type;
> -	struct resource *res;
> +	struct resource r;
> +	struct resource *res = &r;
> +	char objname[64];
> +	struct acpi_buffer buffer = {sizeof(objname), objname};
>
> -	handle = DEVICE_ACPI_HANDLE(&dev->dev);
> -	if (!handle)
> -		return -EINVAL;
> +	*pdev = NULL;
> +	*pgsi_base = 0;
>
>  	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.
> -	 */
> +	if (ACPI_FAILURE(status) || !gsb)
> +		return;
>
> -	ioapic = kzalloc(sizeof(*ioapic), GFP_KERNEL);
> -	if (!ioapic)
> -		return -ENOMEM;
> +	dev = acpi_get_pci_dev(handle);
> +	if (!dev)
> +		return;
>
> -	ioapic->handle = handle;
> -	ioapic->gsi_base = (u32) gsb;
> +	acpi_get_name(handle, ACPI_FULL_PATHNAME, &buffer);
>
> -	if (dev->class == PCI_CLASS_SYSTEM_PIC_IOAPIC)
> -		type = "IOAPIC";
> -	else
> -		type = "IOxAPIC";
> +	gsi_base = gsb;
> +	type = "IOxAPIC";
>
>  	ret = pci_enable_device(dev);
>  	if (ret < 0)
> -		goto exit_free;
> +		goto exit_put;
>
>  	pci_set_master(dev);
>
> +	if (dev->class == PCI_CLASS_SYSTEM_PIC_IOAPIC)
> +		type = "IOAPIC";
> +
>  	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))
> +
> +	if (acpi_register_ioapic(handle, res->start, gsi_base))
>  		goto exit_release;

Here acpi_register_ioapic() fails for IOAPICs already described by the
static MADT. So they won't be added to the ioapic_list. Subsequent
hot-removal will also fail because they're not found by
acpi_pci_ioapic_remove() on the ioapic_list.

Here's what I used to fix it:

>From 2fff3297b4c71c88d92b04ba9ad0a8931b3e99b8 Mon Sep 17 00:00:00 2001
From: Rui Wang <rui.y.wang@intel.com>
Date: Tue, 27 Aug 2013 11:23:43 -0400
Subject: [PATCH] Hotadd of IOAPICs described in static MADT

For IOAPICs described in static MADT, we already called __mp_register_ioapic()
in arch_early_irq_init(). During boot PCI root hotadd will call it again and
will find it already registered, thus register_ioapic() won't add it to the
ioapic_list. Subsequent hot-removal will also fail because it is not
found on the
ioapic_list.

Signed-off-by: Rui Wang <rui.y.wang@intel.com>
---
 arch/x86/kernel/apic/io_apic.c |    4 +++-
 drivers/pci/ioapic.c           |    3 ++-
 2 files changed, 5 insertions(+), 2 deletions(-)

diff --git a/arch/x86/kernel/apic/io_apic.c b/arch/x86/kernel/apic/io_apic.c
index fb9bf06..15ab9f1 100644
--- a/arch/x86/kernel/apic/io_apic.c
+++ b/arch/x86/kernel/apic/io_apic.c
@@ -3956,8 +3956,10 @@ int __mp_register_ioapic(int id, u32 address,
u32 gsi_base, bool hotadd)

        /* already registered ? */
        idx = __mp_find_ioapic(gsi_base);
-       if (idx >= 0)
+       if (idx >= 0) {
+               ret = -EEXIST;
                goto out;
+       }

        idx = find_first_zero_bit(ioapics_mask, MAX_IO_APICS);
        if (idx >= MAX_IO_APICS) {
diff --git a/drivers/pci/ioapic.c b/drivers/pci/ioapic.c
index 41f7c69..83b002b 100644
--- a/drivers/pci/ioapic.c
+++ b/drivers/pci/ioapic.c
@@ -126,7 +126,8 @@ static void handle_ioapic_add(acpi_handle handle,
struct pci_dev **pdev,
                }
        }

-       if (acpi_register_ioapic(handle, res->start, gsi_base)) {
+       ret = acpi_register_ioapic(handle, res->start, gsi_base);
+       if (ret && ret != -EEXIST) {
                if (dev)
                        goto exit_release;
                return;
-- 
1.7.3.4


Thanks
Rui


>
> -	pci_set_drvdata(dev, ioapic);
> -	dev_info(&dev->dev, "%s at %pR, GSI %u\n", type, res, ioapic->gsi_base);
> -	return 0;
> +	pr_info("%s %s %s at %pR, GSI %u\n",
> +		dev_name(&dev->dev), objname, type,
> +		res, gsi_base);
> +
> +	*pdev = dev;
> +	*pgsi_base = gsi_base;
> +	return;
>
>  exit_release:
>  	pci_release_region(dev, 0);
>  exit_disable:
>  	pci_disable_device(dev);
> -exit_free:
> -	kfree(ioapic);
> -	return -ENODEV;
> +exit_put:
> +	pci_dev_put(dev);
>  }
>
> -static void ioapic_remove(struct pci_dev *dev)
> +static void handle_ioapic_remove(acpi_handle handle, struct pci_dev *dev,
> +				  u32 gsi_base)
>  {
> -	struct ioapic *ioapic = pci_get_drvdata(dev);
> +	acpi_unregister_ioapic(handle, gsi_base);
>
> -	acpi_unregister_ioapic(ioapic->handle, ioapic->gsi_base);
>  	pci_release_region(dev, 0);
>  	pci_disable_device(dev);
> -	kfree(ioapic);
> +	pci_dev_put(dev);
>  }
>
> +static acpi_status register_ioapic(acpi_handle handle, u32 lvl,
> +					void *context, void **rv)
> +{
> +	acpi_handle root_handle = context;
> +	struct pci_dev *pdev;
> +	u32 gsi_base;
> +	struct acpi_pci_ioapic *ioapic;
>
> -static DEFINE_PCI_DEVICE_TABLE(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);
> +	handle_ioapic_add(handle, &pdev, &gsi_base);
> +	if (!gsi_base)
> +		return AE_OK;
>
> -static struct pci_driver ioapic_driver = {
> -	.name		= "ioapic",
> -	.id_table	= ioapic_devices,
> -	.probe		= ioapic_probe,
> -	.remove		= ioapic_remove,
> -};
> +	ioapic = kzalloc(sizeof(*ioapic), GFP_KERNEL);
> +	if (!ioapic) {
> +		pr_err("%s: cannot allocate memory\n", __func__);
> +		handle_ioapic_remove(root_handle, pdev, gsi_base);
> +		return AE_OK;
> +	}
> +	ioapic->root_handle = root_handle;
> +	ioapic->pdev = pdev;
> +	ioapic->gsi_base = gsi_base;
> +
> +	mutex_lock(&ioapic_list_lock);
> +	list_add(&ioapic->list, &ioapic_list);
> +	mutex_unlock(&ioapic_list_lock);
> +
> +	return AE_OK;
> +}
>
> -static int __init ioapic_init(void)
> +void acpi_pci_ioapic_add(struct acpi_pci_root *root)
>  {
> -	return pci_register_driver(&ioapic_driver);
> +	acpi_status status;
> +
> +	status = acpi_walk_namespace(ACPI_TYPE_DEVICE, root->device->handle,
> +				     (u32)1, register_ioapic, NULL,
> +				     root->device->handle,
> +				     NULL);
> +	if (ACPI_FAILURE(status))
> +		pr_err("%s: register_ioapic failure - %d", __func__, status);
>  }
> -module_init(ioapic_init);
>
> -MODULE_LICENSE("GPL");
> +void acpi_pci_ioapic_remove(struct acpi_pci_root *root)
> +{
> +	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;
> +		list_del(&ioapic->list);
> +		handle_ioapic_remove(ioapic->root_handle, ioapic->pdev,
> +					ioapic->gsi_base);
> +		kfree(ioapic);
> +	}
> +	mutex_unlock(&ioapic_list_lock);
> +}
> diff --git a/include/linux/pci-acpi.h b/include/linux/pci-acpi.h
> index 1704479..b2a2ced 100644
> --- a/include/linux/pci-acpi.h
> +++ b/include/linux/pci-acpi.h
> @@ -69,6 +69,14 @@ static inline void acpiphp_remove_slots(struct pci_bus
> *bus) { }
>  static inline void acpiphp_check_host_bridge(acpi_handle handle) { }
>  #endif
>
> +#ifdef CONFIG_PCI_IOAPIC
> +void acpi_pci_ioapic_add(struct acpi_pci_root *root);
> +void acpi_pci_ioapic_remove(struct acpi_pci_root *root);
> +#else
> +static inline void acpi_pci_ioapic_add(struct acpi_pci_root *root) { }
> +static inline void acpi_pci_ioapic_remove(struct acpi_pci_root *root) { }
> +#endif
> +
>  #else	/* CONFIG_ACPI */
>  static inline void acpi_pci_add_bus(struct pci_bus *bus) { }
>  static inline void acpi_pci_remove_bus(struct pci_bus *bus) { }
> --
> 1.8.1.4
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/
>

Re: [PATCH v4 27/28] PCI, x86, ACPI: Enable ioapic hotplug support with acpi host bridge.

[rui wang]

FWIW, I'm using my "I/O Hook" (http://lwn.net/Articles/561359/) as a
tool to test yinghai's IOAPIC patchset. It can be used to test any
kind of hotplug.

For people who don't yet have full hardware support and want to test
the hotplug of IOAPIC, CPU, Memory, IOH, PCIe root, etc., it is an
ideal tool. The latest version (v2) is at:
http://lwn.net/Articles/561779/


Thanks
Rui

Re: [PATCH v4 27/28] PCI, x86, ACPI: Enable ioapic hotplug support with acpi host bridge.

[Yinghai Lu]

On Tue, Aug 27, 2013 at 3:25 AM, rui wang <ruiv.wang@gmail.com> wrote:
> On 8/11/13, Yinghai Lu <yinghai@kernel.org> wrote:
>> We need to have ioapic setup before normal pci drivers.
>> otherwise other pci driver can not setup irq.
>>
>> So we should not treat them as normal pci devices.
>> Also we will need to support ioapic hotplug without pci device around.
>>
>> We need to call ioapic add/remove during host-bridge add/remove.
>>
>> Signed-off-by: Yinghai Lu <yinghai@kernel.org>
>> ---
>>  drivers/acpi/pci_root.c  |   4 ++
>>  drivers/pci/ioapic.c     | 147
>> ++++++++++++++++++++++++++++++-----------------
>>  include/linux/pci-acpi.h |   8 +++
>>  3 files changed, 106 insertions(+), 53 deletions(-)
>>
>> diff --git a/drivers/acpi/pci_root.c b/drivers/acpi/pci_root.c
>> index 5917839..7577175 100644
>> --- a/drivers/acpi/pci_root.c
>> +++ b/drivers/acpi/pci_root.c
>> @@ -532,6 +532,8 @@ static int acpi_pci_root_add(struct acpi_device
>> *device,
>>               pci_enable_bridges(root->bus);
>>       }
>>
>> +     acpi_pci_ioapic_add(root);
>> +
>>       pci_bus_add_devices(root->bus);
>>       return 1;
>>
>> @@ -546,6 +548,8 @@ static void acpi_pci_root_remove(struct acpi_device
>> *device)
>>
>>       pci_stop_root_bus(root->bus);
>>
>> +     acpi_pci_ioapic_remove(root);
>> +
>>       device_set_run_wake(root->bus->bridge, false);
>>       pci_acpi_remove_bus_pm_notifier(device);
>>
>> diff --git a/drivers/pci/ioapic.c b/drivers/pci/ioapic.c
>> index 7d6b157..60351b2 100644
>> --- a/drivers/pci/ioapic.c
>> +++ b/drivers/pci/ioapic.c
>> @@ -22,101 +22,142 @@
>>  #include <linux/slab.h>
>>  #include <acpi/acpi_bus.h>
>>
>> -struct ioapic {
>> -     acpi_handle     handle;
>> +struct acpi_pci_ioapic {
>> +     acpi_handle     root_handle;
>>       u32             gsi_base;
>> +     struct pci_dev *pdev;
>> +     struct list_head list;
>>  };
>>
>> -static int ioapic_probe(struct pci_dev *dev, const struct pci_device_id
>> *ent)
>> +static LIST_HEAD(ioapic_list);
>> +static DEFINE_MUTEX(ioapic_list_lock);
>> +
>> +static void handle_ioapic_add(acpi_handle handle, struct pci_dev **pdev,
>> +                              u32 *pgsi_base)
>>  {
>> -     acpi_handle handle;
>>       acpi_status status;
>>       unsigned long long gsb;
>> -     struct ioapic *ioapic;
>> +     struct pci_dev *dev;
>> +     u32 gsi_base;
>>       int ret;
>>       char *type;
>> -     struct resource *res;
>> +     struct resource r;
>> +     struct resource *res = &r;
>> +     char objname[64];
>> +     struct acpi_buffer buffer = {sizeof(objname), objname};
>>
>> -     handle = DEVICE_ACPI_HANDLE(&dev->dev);
>> -     if (!handle)
>> -             return -EINVAL;
>> +     *pdev = NULL;
>> +     *pgsi_base = 0;
>>
>>       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.
>> -      */
>> +     if (ACPI_FAILURE(status) || !gsb)
>> +             return;
>>
>> -     ioapic = kzalloc(sizeof(*ioapic), GFP_KERNEL);
>> -     if (!ioapic)
>> -             return -ENOMEM;
>> +     dev = acpi_get_pci_dev(handle);
>> +     if (!dev)
>> +             return;
>>
>> -     ioapic->handle = handle;
>> -     ioapic->gsi_base = (u32) gsb;
>> +     acpi_get_name(handle, ACPI_FULL_PATHNAME, &buffer);
>>
>> -     if (dev->class == PCI_CLASS_SYSTEM_PIC_IOAPIC)
>> -             type = "IOAPIC";
>> -     else
>> -             type = "IOxAPIC";
>> +     gsi_base = gsb;
>> +     type = "IOxAPIC";
>>
>>       ret = pci_enable_device(dev);
>>       if (ret < 0)
>> -             goto exit_free;
>> +             goto exit_put;
>>
>>       pci_set_master(dev);
>>
>> +     if (dev->class == PCI_CLASS_SYSTEM_PIC_IOAPIC)
>> +             type = "IOAPIC";
>> +
>>       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))
>> +
>> +     if (acpi_register_ioapic(handle, res->start, gsi_base))
>>               goto exit_release;
>
> Here acpi_register_ioapic() fails for IOAPICs already described by the
> static MADT. So they won't be added to the ioapic_list. Subsequent
> hot-removal will also fail because they're not found by
> acpi_pci_ioapic_remove() on the ioapic_list.
>
> Here's what I used to fix it:
>
> From 2fff3297b4c71c88d92b04ba9ad0a8931b3e99b8 Mon Sep 17 00:00:00 2001
> From: Rui Wang <rui.y.wang@intel.com>
> Date: Tue, 27 Aug 2013 11:23:43 -0400
> Subject: [PATCH] Hotadd of IOAPICs described in static MADT
>
> For IOAPICs described in static MADT, we already called __mp_register_ioapic()
> in arch_early_irq_init(). During boot PCI root hotadd will call it again and
> will find it already registered, thus register_ioapic() won't add it to the
> ioapic_list. Subsequent hot-removal will also fail because it is not
> found on the
> ioapic_list.
>
> Signed-off-by: Rui Wang <rui.y.wang@intel.com>
> ---
>  arch/x86/kernel/apic/io_apic.c |    4 +++-
>  drivers/pci/ioapic.c           |    3 ++-
>  2 files changed, 5 insertions(+), 2 deletions(-)
>
> diff --git a/arch/x86/kernel/apic/io_apic.c b/arch/x86/kernel/apic/io_apic.c
> index fb9bf06..15ab9f1 100644
> --- a/arch/x86/kernel/apic/io_apic.c
> +++ b/arch/x86/kernel/apic/io_apic.c
> @@ -3956,8 +3956,10 @@ int __mp_register_ioapic(int id, u32 address,
> u32 gsi_base, bool hotadd)
>
>         /* already registered ? */
>         idx = __mp_find_ioapic(gsi_base);
> -       if (idx >= 0)
> +       if (idx >= 0) {
> +               ret = -EEXIST;
>                 goto out;
> +       }
>
>         idx = find_first_zero_bit(ioapics_mask, MAX_IO_APICS);
>         if (idx >= MAX_IO_APICS) {
> diff --git a/drivers/pci/ioapic.c b/drivers/pci/ioapic.c
> index 41f7c69..83b002b 100644
> --- a/drivers/pci/ioapic.c
> +++ b/drivers/pci/ioapic.c
> @@ -126,7 +126,8 @@ static void handle_ioapic_add(acpi_handle handle,
> struct pci_dev **pdev,
>                 }
>         }
>
> -       if (acpi_register_ioapic(handle, res->start, gsi_base)) {
> +       ret = acpi_register_ioapic(handle, res->start, gsi_base);
> +       if (ret && ret != -EEXIST) {
>                 if (dev)
>                         goto exit_release;
>                 return;
> --

ok, will put it my series.

Thanks

Yinghai

Re: [PATCH v4 27/28] PCI, x86, ACPI: Enable ioapic hotplug support with acpi host bridge.

[rui wang]

On 8/28/13, Yinghai Lu <yinghai@kernel.org> wrote:
> On Tue, Aug 27, 2013 at 3:25 AM, rui wang <ruiv.wang@gmail.com> wrote:
>> On 8/11/13, Yinghai Lu <yinghai@kernel.org> wrote:
>>> We need to have ioapic setup before normal pci drivers.
>>> otherwise other pci driver can not setup irq.
>>>
>>> So we should not treat them as normal pci devices.
>>> Also we will need to support ioapic hotplug without pci device around.
>>>
>>> We need to call ioapic add/remove during host-bridge add/remove.
>>>
>>> Signed-off-by: Yinghai Lu <yinghai@kernel.org>
>>> ---
>>>  drivers/acpi/pci_root.c  |   4 ++
>>>  drivers/pci/ioapic.c     | 147
>>> ++++++++++++++++++++++++++++++-----------------
>>>  include/linux/pci-acpi.h |   8 +++
>>>  3 files changed, 106 insertions(+), 53 deletions(-)
>>>
>>> diff --git a/drivers/acpi/pci_root.c b/drivers/acpi/pci_root.c
>>> index 5917839..7577175 100644
>>> --- a/drivers/acpi/pci_root.c
>>> +++ b/drivers/acpi/pci_root.c
>>> @@ -532,6 +532,8 @@ static int acpi_pci_root_add(struct acpi_device
>>> *device,
>>>               pci_enable_bridges(root->bus);
>>>       }
>>>
>>> +     acpi_pci_ioapic_add(root);
>>> +
>>>       pci_bus_add_devices(root->bus);
>>>       return 1;
>>>
>>> @@ -546,6 +548,8 @@ static void acpi_pci_root_remove(struct acpi_device
>>> *device)
>>>
>>>       pci_stop_root_bus(root->bus);
>>>
>>> +     acpi_pci_ioapic_remove(root);
>>> +
>>>       device_set_run_wake(root->bus->bridge, false);
>>>       pci_acpi_remove_bus_pm_notifier(device);
>>>
>>> diff --git a/drivers/pci/ioapic.c b/drivers/pci/ioapic.c
>>> index 7d6b157..60351b2 100644
>>> --- a/drivers/pci/ioapic.c
>>> +++ b/drivers/pci/ioapic.c
>>> @@ -22,101 +22,142 @@
>>>  #include <linux/slab.h>
>>>  #include <acpi/acpi_bus.h>
>>>
>>> -struct ioapic {
>>> -     acpi_handle     handle;
>>> +struct acpi_pci_ioapic {
>>> +     acpi_handle     root_handle;
>>>       u32             gsi_base;
>>> +     struct pci_dev *pdev;
>>> +     struct list_head list;
>>>  };
>>>
>>> -static int ioapic_probe(struct pci_dev *dev, const struct pci_device_id
>>> *ent)
>>> +static LIST_HEAD(ioapic_list);
>>> +static DEFINE_MUTEX(ioapic_list_lock);
>>> +
>>> +static void handle_ioapic_add(acpi_handle handle, struct pci_dev
>>> **pdev,
>>> +                              u32 *pgsi_base)
>>>  {
>>> -     acpi_handle handle;
>>>       acpi_status status;
>>>       unsigned long long gsb;
>>> -     struct ioapic *ioapic;
>>> +     struct pci_dev *dev;
>>> +     u32 gsi_base;
>>>       int ret;
>>>       char *type;
>>> -     struct resource *res;
>>> +     struct resource r;
>>> +     struct resource *res = &r;
>>> +     char objname[64];
>>> +     struct acpi_buffer buffer = {sizeof(objname), objname};
>>>
>>> -     handle = DEVICE_ACPI_HANDLE(&dev->dev);
>>> -     if (!handle)
>>> -             return -EINVAL;
>>> +     *pdev = NULL;
>>> +     *pgsi_base = 0;
>>>
>>>       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.
>>> -      */
>>> +     if (ACPI_FAILURE(status) || !gsb)
>>> +             return;
>>>
>>> -     ioapic = kzalloc(sizeof(*ioapic), GFP_KERNEL);
>>> -     if (!ioapic)
>>> -             return -ENOMEM;
>>> +     dev = acpi_get_pci_dev(handle);
>>> +     if (!dev)
>>> +             return;
>>>
>>> -     ioapic->handle = handle;
>>> -     ioapic->gsi_base = (u32) gsb;
>>> +     acpi_get_name(handle, ACPI_FULL_PATHNAME, &buffer);
>>>
>>> -     if (dev->class == PCI_CLASS_SYSTEM_PIC_IOAPIC)
>>> -             type = "IOAPIC";
>>> -     else
>>> -             type = "IOxAPIC";
>>> +     gsi_base = gsb;
>>> +     type = "IOxAPIC";
>>>
>>>       ret = pci_enable_device(dev);
>>>       if (ret < 0)
>>> -             goto exit_free;
>>> +             goto exit_put;
>>>
>>>       pci_set_master(dev);
>>>
>>> +     if (dev->class == PCI_CLASS_SYSTEM_PIC_IOAPIC)
>>> +             type = "IOAPIC";
>>> +
>>>       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))
>>> +
>>> +     if (acpi_register_ioapic(handle, res->start, gsi_base))
>>>               goto exit_release;
>>
>> Here acpi_register_ioapic() fails for IOAPICs already described by the
>> static MADT. So they won't be added to the ioapic_list. Subsequent
>> hot-removal will also fail because they're not found by
>> acpi_pci_ioapic_remove() on the ioapic_list.
>>
>> Here's what I used to fix it:
>>
>> From 2fff3297b4c71c88d92b04ba9ad0a8931b3e99b8 Mon Sep 17 00:00:00 2001
>> From: Rui Wang <rui.y.wang@intel.com>
>> Date: Tue, 27 Aug 2013 11:23:43 -0400
>> Subject: [PATCH] Hotadd of IOAPICs described in static MADT
>>
>> For IOAPICs described in static MADT, we already called
>> __mp_register_ioapic()
>> in arch_early_irq_init(). During boot PCI root hotadd will call it again
>> and
>> will find it already registered, thus register_ioapic() won't add it to
>> the
>> ioapic_list. Subsequent hot-removal will also fail because it is not
>> found on the
>> ioapic_list.
>>
>> Signed-off-by: Rui Wang <rui.y.wang@intel.com>
>> ---
>>  arch/x86/kernel/apic/io_apic.c |    4 +++-
>>  drivers/pci/ioapic.c           |    3 ++-
>>  2 files changed, 5 insertions(+), 2 deletions(-)
>>
>> diff --git a/arch/x86/kernel/apic/io_apic.c
>> b/arch/x86/kernel/apic/io_apic.c
>> index fb9bf06..15ab9f1 100644
>> --- a/arch/x86/kernel/apic/io_apic.c
>> +++ b/arch/x86/kernel/apic/io_apic.c
>> @@ -3956,8 +3956,10 @@ int __mp_register_ioapic(int id, u32 address,
>> u32 gsi_base, bool hotadd)
>>
>>         /* already registered ? */
>>         idx = __mp_find_ioapic(gsi_base);
>> -       if (idx >= 0)
>> +       if (idx >= 0) {
>> +               ret = -EEXIST;
>>                 goto out;
>> +       }
>>
>>         idx = find_first_zero_bit(ioapics_mask, MAX_IO_APICS);
>>         if (idx >= MAX_IO_APICS) {
>> diff --git a/drivers/pci/ioapic.c b/drivers/pci/ioapic.c
>> index 41f7c69..83b002b 100644
>> --- a/drivers/pci/ioapic.c
>> +++ b/drivers/pci/ioapic.c
>> @@ -126,7 +126,8 @@ static void handle_ioapic_add(acpi_handle handle,
>> struct pci_dev **pdev,
>>                 }
>>         }
>>
>> -       if (acpi_register_ioapic(handle, res->start, gsi_base)) {
>> +       ret = acpi_register_ioapic(handle, res->start, gsi_base);
>> +       if (ret && ret != -EEXIST) {
>>                 if (dev)
>>                         goto exit_release;
>>                 return;
>> --
>
> ok, will put it my series.
>

That would be great. Thanks.

Rui

> Thanks
>
> Yinghai
>

Re: [PATCH v4 27/28] PCI, x86, ACPI: Enable ioapic hotplug support with acpi host bridge.

[Yijing Wang]

> diff --git a/drivers/acpi/pci_root.c b/drivers/acpi/pci_root.c
> index 5917839..7577175 100644
> --- a/drivers/acpi/pci_root.c
> +++ b/drivers/acpi/pci_root.c
> @@ -532,6 +532,8 @@ static int acpi_pci_root_add(struct acpi_device *device,
>  		pci_enable_bridges(root->bus);
>  	}

Hi Yinghai,
   This like a mistake, pci_enable_bridges() should be called after hot added ioapic registered,
as the comments mention.


527     if (system_state != SYSTEM_BOOTING) {
528         pcibios_resource_survey_bus(root->bus);
529         pci_assign_unassigned_bus_resources(root->bus);
530
531         /* need to after hot-added ioapic is registered */   <--------------
532         pci_enable_bridges(root->bus);
533     }
534
535     acpi_pci_ioapic_add(root);

In my platform, some errors found like:

  614.332730] mp_register_gsi: gsi 71
[  614.332733] ERROR: Unable to locate IOAPIC for GSI 71
[  614.332734] No IOAPIC for GSI 71
[  614.332736] ERROR: Unable to locate IOAPIC for GSI 71
[  614.332738] acpi_register_gsi: gsi 71, irq -1
[  614.332740] pci 0000:80:00.0: PCI INT A: failed to register GSI
[  614.332743] pci 0000:80:00.0: Error enabling bridge (-1), continuing
[  614.332857] mp_register_gsi: gsi 52
[  614.332859] ERROR: Unable to locate IOAPIC for GSI 52
[  614.332860] No IOAPIC for GSI 52
[  614.332861] ERROR: Unable to locate IOAPIC for GSI 52
[  614.332863] acpi_register_gsi: gsi 52, irq -1
[  614.332865] pci 0000:80:01.0: PCI INT A: failed to register GSI
[  614.332867] pci 0000:80:01.0: Error enabling bridge (-1), continuing
[  614.332979] mp_register_gsi: gsi 48
[  614.332980] ERROR: Unable to locate IOAPIC for GSI 48
[  614.332982] No IOAPIC for GSI 48
[  614.332983] ERROR: Unable to locate IOAPIC for GSI 48
[  614.332985] acpi_register_gsi: gsi 48, irq -1
[  614.332987] pci 0000:80:03.0: PCI INT A: failed to register GSI
[  614.332989] pci 0000:80:03.0: Error enabling bridge (-1), continuing
[  614.333101] mp_register_gsi: gsi 50
[  614.333102] ERROR: Unable to locate IOAPIC for GSI 50
[  614.333104] No IOAPIC for GSI 50


>  
> +	acpi_pci_ioapic_add(root);
> +
>  	pci_bus_add_devices(root->bus);
>  	return 1;
>  
> @@ -546,6 +548,8 @@ static void acpi_pci_root_remove(struct acpi_device *device)
>  
>  	pci_stop_root_bus(root->bus);
>  
> +	acpi_pci_ioapic_remove(root);
> +
>  	device_set_run_wake(root->bus->bridge, false);
>  	pci_acpi_remove_bus_pm_notifier(device);
>  
> diff --git a/drivers/pci/ioapic.c b/drivers/pci/ioapic.c
> index 7d6b157..60351b2 100644
> --- a/drivers/pci/ioapic.c
> +++ b/drivers/pci/ioapic.c
> @@ -22,101 +22,142 @@
>  #include <linux/slab.h>
>  #include <acpi/acpi_bus.h>
>  
> -struct ioapic {
> -	acpi_handle	handle;
> +struct acpi_pci_ioapic {
> +	acpi_handle	root_handle;
>  	u32		gsi_base;
> +	struct pci_dev *pdev;
> +	struct list_head list;
>  };
>  
> -static int ioapic_probe(struct pci_dev *dev, const struct pci_device_id *ent)
> +static LIST_HEAD(ioapic_list);
> +static DEFINE_MUTEX(ioapic_list_lock);
> +
> +static void handle_ioapic_add(acpi_handle handle, struct pci_dev **pdev,
> +				 u32 *pgsi_base)
>  {
> -	acpi_handle handle;
>  	acpi_status status;
>  	unsigned long long gsb;
> -	struct ioapic *ioapic;
> +	struct pci_dev *dev;
> +	u32 gsi_base;
>  	int ret;
>  	char *type;
> -	struct resource *res;
> +	struct resource r;
> +	struct resource *res = &r;
> +	char objname[64];
> +	struct acpi_buffer buffer = {sizeof(objname), objname};
>  
> -	handle = DEVICE_ACPI_HANDLE(&dev->dev);
> -	if (!handle)
> -		return -EINVAL;
> +	*pdev = NULL;
> +	*pgsi_base = 0;
>  
>  	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.
> -	 */
> +	if (ACPI_FAILURE(status) || !gsb)
> +		return;
>  
> -	ioapic = kzalloc(sizeof(*ioapic), GFP_KERNEL);
> -	if (!ioapic)
> -		return -ENOMEM;
> +	dev = acpi_get_pci_dev(handle);
> +	if (!dev)
> +		return;
>  
> -	ioapic->handle = handle;
> -	ioapic->gsi_base = (u32) gsb;
> +	acpi_get_name(handle, ACPI_FULL_PATHNAME, &buffer);
>  
> -	if (dev->class == PCI_CLASS_SYSTEM_PIC_IOAPIC)
> -		type = "IOAPIC";
> -	else
> -		type = "IOxAPIC";
> +	gsi_base = gsb;
> +	type = "IOxAPIC";
>  
>  	ret = pci_enable_device(dev);
>  	if (ret < 0)
> -		goto exit_free;
> +		goto exit_put;
>  
>  	pci_set_master(dev);
>  
> +	if (dev->class == PCI_CLASS_SYSTEM_PIC_IOAPIC)
> +		type = "IOAPIC";
> +
>  	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))
> +
> +	if (acpi_register_ioapic(handle, res->start, 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;
> +	pr_info("%s %s %s at %pR, GSI %u\n",
> +		dev_name(&dev->dev), objname, type,
> +		res, gsi_base);
> +
> +	*pdev = dev;
> +	*pgsi_base = gsi_base;
> +	return;
>  
>  exit_release:
>  	pci_release_region(dev, 0);
>  exit_disable:
>  	pci_disable_device(dev);
> -exit_free:
> -	kfree(ioapic);
> -	return -ENODEV;
> +exit_put:
> +	pci_dev_put(dev);
>  }
>  
> -static void ioapic_remove(struct pci_dev *dev)
> +static void handle_ioapic_remove(acpi_handle handle, struct pci_dev *dev,
> +				  u32 gsi_base)
>  {
> -	struct ioapic *ioapic = pci_get_drvdata(dev);
> +	acpi_unregister_ioapic(handle, gsi_base);
>  
> -	acpi_unregister_ioapic(ioapic->handle, ioapic->gsi_base);
>  	pci_release_region(dev, 0);
>  	pci_disable_device(dev);
> -	kfree(ioapic);
> +	pci_dev_put(dev);
>  }
>  
> +static acpi_status register_ioapic(acpi_handle handle, u32 lvl,
> +					void *context, void **rv)
> +{
> +	acpi_handle root_handle = context;
> +	struct pci_dev *pdev;
> +	u32 gsi_base;
> +	struct acpi_pci_ioapic *ioapic;
>  
> -static DEFINE_PCI_DEVICE_TABLE(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);
> +	handle_ioapic_add(handle, &pdev, &gsi_base);
> +	if (!gsi_base)
> +		return AE_OK;
>  
> -static struct pci_driver ioapic_driver = {
> -	.name		= "ioapic",
> -	.id_table	= ioapic_devices,
> -	.probe		= ioapic_probe,
> -	.remove		= ioapic_remove,
> -};
> +	ioapic = kzalloc(sizeof(*ioapic), GFP_KERNEL);
> +	if (!ioapic) {
> +		pr_err("%s: cannot allocate memory\n", __func__);
> +		handle_ioapic_remove(root_handle, pdev, gsi_base);
> +		return AE_OK;
> +	}
> +	ioapic->root_handle = root_handle;
> +	ioapic->pdev = pdev;
> +	ioapic->gsi_base = gsi_base;
> +
> +	mutex_lock(&ioapic_list_lock);
> +	list_add(&ioapic->list, &ioapic_list);
> +	mutex_unlock(&ioapic_list_lock);
> +
> +	return AE_OK;
> +}
>  
> -static int __init ioapic_init(void)
> +void acpi_pci_ioapic_add(struct acpi_pci_root *root)
>  {
> -	return pci_register_driver(&ioapic_driver);
> +	acpi_status status;
> +
> +	status = acpi_walk_namespace(ACPI_TYPE_DEVICE, root->device->handle,
> +				     (u32)1, register_ioapic, NULL,
> +				     root->device->handle,
> +				     NULL);
> +	if (ACPI_FAILURE(status))
> +		pr_err("%s: register_ioapic failure - %d", __func__, status);
>  }
> -module_init(ioapic_init);
>  
> -MODULE_LICENSE("GPL");
> +void acpi_pci_ioapic_remove(struct acpi_pci_root *root)
> +{
> +	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;
> +		list_del(&ioapic->list);
> +		handle_ioapic_remove(ioapic->root_handle, ioapic->pdev,
> +					ioapic->gsi_base);
> +		kfree(ioapic);
> +	}
> +	mutex_unlock(&ioapic_list_lock);
> +}
> diff --git a/include/linux/pci-acpi.h b/include/linux/pci-acpi.h
> index 1704479..b2a2ced 100644
> --- a/include/linux/pci-acpi.h
> +++ b/include/linux/pci-acpi.h
> @@ -69,6 +69,14 @@ static inline void acpiphp_remove_slots(struct pci_bus *bus) { }
>  static inline void acpiphp_check_host_bridge(acpi_handle handle) { }
>  #endif
>  
> +#ifdef CONFIG_PCI_IOAPIC
> +void acpi_pci_ioapic_add(struct acpi_pci_root *root);
> +void acpi_pci_ioapic_remove(struct acpi_pci_root *root);
> +#else
> +static inline void acpi_pci_ioapic_add(struct acpi_pci_root *root) { }
> +static inline void acpi_pci_ioapic_remove(struct acpi_pci_root *root) { }
> +#endif
> +
>  #else	/* CONFIG_ACPI */
>  static inline void acpi_pci_add_bus(struct pci_bus *bus) { }
>  static inline void acpi_pci_remove_bus(struct pci_bus *bus) { }
> 


-- 
Thanks!
Yijing

[PATCH v4 28/28] PCI, x86, ACPI: get ioapic address from acpi device

[Yinghai Lu]

Some ioapic controllers do not show up on pci config space,
or pci device is there but no bar is used and is set by firmware in
other non standard registers.

We can get ioapic address from ACPI0009's _CRS.

Signed-off-by: Yinghai Lu <yinghai@kernel.org>
---
 drivers/pci/ioapic.c | 86 +++++++++++++++++++++++++++++++++++++++++++---------
 1 file changed, 71 insertions(+), 15 deletions(-)

diff --git a/drivers/pci/ioapic.c b/drivers/pci/ioapic.c
index 60351b2..41f7c69 100644
--- a/drivers/pci/ioapic.c
+++ b/drivers/pci/ioapic.c
@@ -32,6 +32,36 @@ struct acpi_pci_ioapic {
 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;
+	struct acpi_resource_address64 addr;
+	acpi_status status;
+	unsigned long flags;
+	u64 start, end;
+
+	status = acpi_resource_to_address64(acpi_res, &addr);
+	if (!ACPI_SUCCESS(status))
+		return AE_OK;
+
+	if (addr.resource_type == ACPI_MEMORY_RANGE) {
+		if (addr.info.mem.caching == ACPI_PREFETCHABLE_MEMORY)
+			return AE_OK;
+		flags = IORESOURCE_MEM;
+	} else
+		return AE_OK;
+
+	start = addr.minimum + addr.translation_offset;
+	end = addr.maximum + addr.translation_offset;
+
+	res = data;
+	res->flags = flags;
+	res->start = start;
+	res->end = end;
+
+	return AE_OK;
+}
+
 static void handle_ioapic_add(acpi_handle handle, struct pci_dev **pdev,
 				 u32 *pgsi_base)
 {
@@ -54,33 +84,56 @@ static void handle_ioapic_add(acpi_handle handle, struct pci_dev **pdev,
 		return;
 
 	dev = acpi_get_pci_dev(handle);
-	if (!dev)
-		return;
+	if (!dev || !pci_resource_len(dev, 0)) {
+		struct acpi_device_info *info;
+		char *hid = NULL;
+
+		status = acpi_get_object_info(handle, &info);
+		if (ACPI_FAILURE(status))
+			goto exit_put;
+		if (info->valid & ACPI_VALID_HID)
+			hid = info->hardware_id.string;
+		if (!hid || strcmp(hid, "ACPI0009")) {
+			kfree(info);
+			goto exit_put;
+		}
+		kfree(info);
+		memset(res, 0, sizeof(*res));
+		acpi_walk_resources(handle, METHOD_NAME__CRS, setup_res, res);
+		if (!res->flags)
+			goto exit_put;
+	}
 
 	acpi_get_name(handle, ACPI_FULL_PATHNAME, &buffer);
 
 	gsi_base = gsb;
 	type = "IOxAPIC";
+	if (dev) {
+		ret = pci_enable_device(dev);
+		if (ret < 0)
+			goto exit_put;
 
-	ret = pci_enable_device(dev);
-	if (ret < 0)
-		goto exit_put;
-
-	pci_set_master(dev);
+		pci_set_master(dev);
 
-	if (dev->class == PCI_CLASS_SYSTEM_PIC_IOAPIC)
-		type = "IOAPIC";
+		if (dev->class == PCI_CLASS_SYSTEM_PIC_IOAPIC)
+			type = "IOAPIC";
 
-	if (pci_request_region(dev, 0, type))
-		goto exit_disable;
+		if (pci_resource_len(dev, 0)) {
+			if (pci_request_region(dev, 0, type))
+				goto exit_disable;
 
-	res = &dev->resource[0];
+			res = &dev->resource[0];
+		}
+	}
 
-	if (acpi_register_ioapic(handle, res->start, gsi_base))
-		goto exit_release;
+	if (acpi_register_ioapic(handle, res->start, gsi_base)) {
+		if (dev)
+			goto exit_release;
+		return;
+	}
 
 	pr_info("%s %s %s at %pR, GSI %u\n",
-		dev_name(&dev->dev), objname, type,
+		dev ? dev_name(&dev->dev) : "", objname, type,
 		res, gsi_base);
 
 	*pdev = dev;
@@ -100,6 +153,9 @@ static void handle_ioapic_remove(acpi_handle handle, struct pci_dev *dev,
 {
 	acpi_unregister_ioapic(handle, gsi_base);
 
+	if (!dev)
+		return;
+
 	pci_release_region(dev, 0);
 	pci_disable_device(dev);
 	pci_dev_put(dev);
-- 
1.8.1.4

Re: [PATCH v4 28/28] PCI, x86, ACPI: get ioapic address from acpi device

[rui wang]

On 8/11/13, Yinghai Lu <yinghai@kernel.org> wrote:
> Some ioapic controllers do not show up on pci config space,
> or pci device is there but no bar is used and is set by firmware in
> other non standard registers.
>
> We can get ioapic address from ACPI0009's _CRS.
>
> Signed-off-by: Yinghai Lu <yinghai@kernel.org>
> ---
>  drivers/pci/ioapic.c | 86
> +++++++++++++++++++++++++++++++++++++++++++---------
>  1 file changed, 71 insertions(+), 15 deletions(-)
>
> diff --git a/drivers/pci/ioapic.c b/drivers/pci/ioapic.c
> index 60351b2..41f7c69 100644
> --- a/drivers/pci/ioapic.c
> +++ b/drivers/pci/ioapic.c
> @@ -32,6 +32,36 @@ struct acpi_pci_ioapic {
>  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;
> +	struct acpi_resource_address64 addr;
> +	acpi_status status;
> +	unsigned long flags;
> +	u64 start, end;
> +
> +	status = acpi_resource_to_address64(acpi_res, &addr);

The above function fails on my machine because it does not handle
Memory32Fixed resources. Is the following _CRS declaration allowed for
IOAPIC?

            Device (APIC)
            {
                Name (_HID, "ACPI0009")  // _HID: Hardware ID
                Name (_CRS, ResourceTemplate ()  // _CRS: Current
Resource Settings
                {
                    Memory32Fixed (ReadOnly,
                        0xFEC01000,         // Address Base
                        0x00001000,         // Address Length
                        )
                })
                Method(_GSB) {
                        return (0x18)
                }
            }


Thanks
Rui

> +	if (!ACPI_SUCCESS(status))
> +		return AE_OK;
> +
> +	if (addr.resource_type == ACPI_MEMORY_RANGE) {
> +		if (addr.info.mem.caching == ACPI_PREFETCHABLE_MEMORY)
> +			return AE_OK;
> +		flags = IORESOURCE_MEM;
> +	} else
> +		return AE_OK;
> +
> +	start = addr.minimum + addr.translation_offset;
> +	end = addr.maximum + addr.translation_offset;
> +
> +	res = data;
> +	res->flags = flags;
> +	res->start = start;
> +	res->end = end;
> +
> +	return AE_OK;
> +}
> +
>  static void handle_ioapic_add(acpi_handle handle, struct pci_dev **pdev,
>  				 u32 *pgsi_base)
>  {
> @@ -54,33 +84,56 @@ static void handle_ioapic_add(acpi_handle handle, struct
> pci_dev **pdev,
>  		return;
>
>  	dev = acpi_get_pci_dev(handle);
> -	if (!dev)
> -		return;
> +	if (!dev || !pci_resource_len(dev, 0)) {
> +		struct acpi_device_info *info;
> +		char *hid = NULL;
> +
> +		status = acpi_get_object_info(handle, &info);
> +		if (ACPI_FAILURE(status))
> +			goto exit_put;
> +		if (info->valid & ACPI_VALID_HID)
> +			hid = info->hardware_id.string;
> +		if (!hid || strcmp(hid, "ACPI0009")) {
> +			kfree(info);
> +			goto exit_put;
> +		}
> +		kfree(info);
> +		memset(res, 0, sizeof(*res));
> +		acpi_walk_resources(handle, METHOD_NAME__CRS, setup_res, res);
> +		if (!res->flags)
> +			goto exit_put;
> +	}
>
>  	acpi_get_name(handle, ACPI_FULL_PATHNAME, &buffer);
>
>  	gsi_base = gsb;
>  	type = "IOxAPIC";
> +	if (dev) {
> +		ret = pci_enable_device(dev);
> +		if (ret < 0)
> +			goto exit_put;
>
> -	ret = pci_enable_device(dev);
> -	if (ret < 0)
> -		goto exit_put;
> -
> -	pci_set_master(dev);
> +		pci_set_master(dev);
>
> -	if (dev->class == PCI_CLASS_SYSTEM_PIC_IOAPIC)
> -		type = "IOAPIC";
> +		if (dev->class == PCI_CLASS_SYSTEM_PIC_IOAPIC)
> +			type = "IOAPIC";
>
> -	if (pci_request_region(dev, 0, type))
> -		goto exit_disable;
> +		if (pci_resource_len(dev, 0)) {
> +			if (pci_request_region(dev, 0, type))
> +				goto exit_disable;
>
> -	res = &dev->resource[0];
> +			res = &dev->resource[0];
> +		}
> +	}
>
> -	if (acpi_register_ioapic(handle, res->start, gsi_base))
> -		goto exit_release;
> +	if (acpi_register_ioapic(handle, res->start, gsi_base)) {
> +		if (dev)
> +			goto exit_release;
> +		return;
> +	}
>
>  	pr_info("%s %s %s at %pR, GSI %u\n",
> -		dev_name(&dev->dev), objname, type,
> +		dev ? dev_name(&dev->dev) : "", objname, type,
>  		res, gsi_base);
>
>  	*pdev = dev;
> @@ -100,6 +153,9 @@ static void handle_ioapic_remove(acpi_handle handle,
> struct pci_dev *dev,
>  {
>  	acpi_unregister_ioapic(handle, gsi_base);
>
> +	if (!dev)
> +		return;
> +
>  	pci_release_region(dev, 0);
>  	pci_disable_device(dev);
>  	pci_dev_put(dev);
> --
> 1.8.1.4
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/
>

Re: [PATCH v4 28/28] PCI, x86, ACPI: get ioapic address from acpi device

[rui wang]

On 8/22/13, rui wang <ruiv.wang@gmail.com> wrote:
> On 8/11/13, Yinghai Lu <yinghai@kernel.org> wrote:
>> Some ioapic controllers do not show up on pci config space,
>> or pci device is there but no bar is used and is set by firmware in
>> other non standard registers.
>>
>> We can get ioapic address from ACPI0009's _CRS.
>>
>> Signed-off-by: Yinghai Lu <yinghai@kernel.org>
>> ---
>>  drivers/pci/ioapic.c | 86
>> +++++++++++++++++++++++++++++++++++++++++++---------
>>  1 file changed, 71 insertions(+), 15 deletions(-)
>>
>> diff --git a/drivers/pci/ioapic.c b/drivers/pci/ioapic.c
>> index 60351b2..41f7c69 100644
>> --- a/drivers/pci/ioapic.c
>> +++ b/drivers/pci/ioapic.c
>> @@ -32,6 +32,36 @@ struct acpi_pci_ioapic {
>>  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;
>> +	struct acpi_resource_address64 addr;
>> +	acpi_status status;
>> +	unsigned long flags;
>> +	u64 start, end;
>> +
>> +	status = acpi_resource_to_address64(acpi_res, &addr);

I worked around the problem by replacing acpi_resource_to_address64()
with resource_to_addr().  But resource_to_addr() is a static function
in arch/x86/pci/acpi.c, not very convenient to use. Here's what I did:

diff --git a/arch/x86/pci/acpi.c b/arch/x86/pci/acpi.c
index d641897..cb5940a 100644
--- a/arch/x86/pci/acpi.c
+++ b/arch/x86/pci/acpi.c
@@ -218,7 +218,7 @@ static void teardown_mcfg_map(struct pci_root_info *info)
 }
 #endif

-static acpi_status
+acpi_status
 resource_to_addr(struct acpi_resource *resource,
                        struct acpi_resource_address64 *addr)
 {
diff --git a/drivers/pci/ioapic.c b/drivers/pci/ioapic.c
index 41f7c69..68d7395 100644
--- a/drivers/pci/ioapic.c
+++ b/drivers/pci/ioapic.c
@@ -40,7 +40,7 @@ static acpi_status setup_res(struct acpi_resource
*acpi_res, void *data)
        unsigned long flags;
        u64 start, end;

-       status = acpi_resource_to_address64(acpi_res, &addr);
+       status = resource_to_addr(acpi_res, &addr);
        if (!ACPI_SUCCESS(status))
                return AE_OK;
diff --git a/include/acpi/acpi_bus.h b/include/acpi/acpi_bus.h
index 94383a7..ae7f759 100644
--- a/include/acpi/acpi_bus.h
+++ b/include/acpi/acpi_bus.h
@@ -354,6 +354,10 @@ extern int unregister_acpi_notifier(struct
notifier_block *);

 extern int register_acpi_bus_notifier(struct notifier_block *nb);
 extern void unregister_acpi_bus_notifier(struct notifier_block *nb);
+extern acpi_status
+resource_to_addr(struct acpi_resource *resource,
+                        struct acpi_resource_address64 *addr);
+

Thanks
Rui


>
> The above function fails on my machine because it does not handle
> Memory32Fixed resources. Is the following _CRS declaration allowed for
> IOAPIC?
>
>             Device (APIC)
>             {
>                 Name (_HID, "ACPI0009")  // _HID: Hardware ID
>                 Name (_CRS, ResourceTemplate ()  // _CRS: Current
> Resource Settings
>                 {
>                     Memory32Fixed (ReadOnly,
>                         0xFEC01000,         // Address Base
>                         0x00001000,         // Address Length
>                         )
>                 })
>                 Method(_GSB) {
>                         return (0x18)
>                 }
>             }
>
>
> Thanks
> Rui
>
>> +	if (!ACPI_SUCCESS(status))
>> +		return AE_OK;
>> +
>> +	if (addr.resource_type == ACPI_MEMORY_RANGE) {
>> +		if (addr.info.mem.caching == ACPI_PREFETCHABLE_MEMORY)
>> +			return AE_OK;
>> +		flags = IORESOURCE_MEM;
>> +	} else
>> +		return AE_OK;
>> +
>> +	start = addr.minimum + addr.translation_offset;
>> +	end = addr.maximum + addr.translation_offset;
>> +
>> +	res = data;
>> +	res->flags = flags;
>> +	res->start = start;
>> +	res->end = end;
>> +
>> +	return AE_OK;
>> +}
>> +
>>  static void handle_ioapic_add(acpi_handle handle, struct pci_dev **pdev,
>>  				 u32 *pgsi_base)
>>  {
>> @@ -54,33 +84,56 @@ static void handle_ioapic_add(acpi_handle handle,
>> struct
>> pci_dev **pdev,
>>  		return;
>>
>>  	dev = acpi_get_pci_dev(handle);
>> -	if (!dev)
>> -		return;
>> +	if (!dev || !pci_resource_len(dev, 0)) {
>> +		struct acpi_device_info *info;
>> +		char *hid = NULL;
>> +
>> +		status = acpi_get_object_info(handle, &info);
>> +		if (ACPI_FAILURE(status))
>> +			goto exit_put;
>> +		if (info->valid & ACPI_VALID_HID)
>> +			hid = info->hardware_id.string;
>> +		if (!hid || strcmp(hid, "ACPI0009")) {
>> +			kfree(info);
>> +			goto exit_put;
>> +		}
>> +		kfree(info);
>> +		memset(res, 0, sizeof(*res));
>> +		acpi_walk_resources(handle, METHOD_NAME__CRS, setup_res, res);
>> +		if (!res->flags)
>> +			goto exit_put;
>> +	}
>>
>>  	acpi_get_name(handle, ACPI_FULL_PATHNAME, &buffer);
>>
>>  	gsi_base = gsb;
>>  	type = "IOxAPIC";
>> +	if (dev) {
>> +		ret = pci_enable_device(dev);
>> +		if (ret < 0)
>> +			goto exit_put;
>>
>> -	ret = pci_enable_device(dev);
>> -	if (ret < 0)
>> -		goto exit_put;
>> -
>> -	pci_set_master(dev);
>> +		pci_set_master(dev);
>>
>> -	if (dev->class == PCI_CLASS_SYSTEM_PIC_IOAPIC)
>> -		type = "IOAPIC";
>> +		if (dev->class == PCI_CLASS_SYSTEM_PIC_IOAPIC)
>> +			type = "IOAPIC";
>>
>> -	if (pci_request_region(dev, 0, type))
>> -		goto exit_disable;
>> +		if (pci_resource_len(dev, 0)) {
>> +			if (pci_request_region(dev, 0, type))
>> +				goto exit_disable;
>>
>> -	res = &dev->resource[0];
>> +			res = &dev->resource[0];
>> +		}
>> +	}
>>
>> -	if (acpi_register_ioapic(handle, res->start, gsi_base))
>> -		goto exit_release;
>> +	if (acpi_register_ioapic(handle, res->start, gsi_base)) {
>> +		if (dev)
>> +			goto exit_release;
>> +		return;
>> +	}
>>
>>  	pr_info("%s %s %s at %pR, GSI %u\n",
>> -		dev_name(&dev->dev), objname, type,
>> +		dev ? dev_name(&dev->dev) : "", objname, type,
>>  		res, gsi_base);
>>
>>  	*pdev = dev;
>> @@ -100,6 +153,9 @@ static void handle_ioapic_remove(acpi_handle handle,
>> struct pci_dev *dev,
>>  {
>>  	acpi_unregister_ioapic(handle, gsi_base);
>>
>> +	if (!dev)
>> +		return;
>> +
>>  	pci_release_region(dev, 0);
>>  	pci_disable_device(dev);
>>  	pci_dev_put(dev);
>> --
>> 1.8.1.4
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-kernel"
>> in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>> Please read the FAQ at  http://www.tux.org/lkml/
>>
>

Re: [PATCH v4 28/28] PCI, x86, ACPI: get ioapic address from acpi device

[Lan Tianyu]

2013/8/23 rui wang <ruiv.wang@gmail.com>:
> On 8/22/13, rui wang <ruiv.wang@gmail.com> wrote:
>> On 8/11/13, Yinghai Lu <yinghai@kernel.org> wrote:
>>> Some ioapic controllers do not show up on pci config space,
>>> or pci device is there but no bar is used and is set by firmware in
>>> other non standard registers.
>>>
>>> We can get ioapic address from ACPI0009's _CRS.
>>>
>>> Signed-off-by: Yinghai Lu <yinghai@kernel.org>
>>> ---
>>>  drivers/pci/ioapic.c | 86
>>> +++++++++++++++++++++++++++++++++++++++++++---------
>>>  1 file changed, 71 insertions(+), 15 deletions(-)
>>>
>>> diff --git a/drivers/pci/ioapic.c b/drivers/pci/ioapic.c
>>> index 60351b2..41f7c69 100644
>>> --- a/drivers/pci/ioapic.c
>>> +++ b/drivers/pci/ioapic.c
>>> @@ -32,6 +32,36 @@ struct acpi_pci_ioapic {
>>>  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;
>>> +    struct acpi_resource_address64 addr;
>>> +    acpi_status status;
>>> +    unsigned long flags;
>>> +    u64 start, end;
>>> +
>>> +    status = acpi_resource_to_address64(acpi_res, &addr);
>
> I worked around the problem by replacing acpi_resource_to_address64()
> with resource_to_addr().  But resource_to_addr() is a static function
> in arch/x86/pci/acpi.c, not very convenient to use. Here's what I did:
>

Hi Rui&Yinghai:
           How about using the following code to translate struct
 acpi_resource to struct resouce in this setup_res()?

         if (acpi_dev_resource_address_space(...)
                || acpi_dev_resource_memory(..))
              return AE_OK;




> diff --git a/arch/x86/pci/acpi.c b/arch/x86/pci/acpi.c
> index d641897..cb5940a 100644
> --- a/arch/x86/pci/acpi.c
> +++ b/arch/x86/pci/acpi.c
> @@ -218,7 +218,7 @@ static void teardown_mcfg_map(struct pci_root_info *info)
>  }
>  #endif
>
> -static acpi_status
> +acpi_status
>  resource_to_addr(struct acpi_resource *resource,
>                         struct acpi_resource_address64 *addr)
>  {
> diff --git a/drivers/pci/ioapic.c b/drivers/pci/ioapic.c
> index 41f7c69..68d7395 100644
> --- a/drivers/pci/ioapic.c
> +++ b/drivers/pci/ioapic.c
> @@ -40,7 +40,7 @@ static acpi_status setup_res(struct acpi_resource
> *acpi_res, void *data)
>         unsigned long flags;
>         u64 start, end;
>
> -       status = acpi_resource_to_address64(acpi_res, &addr);
> +       status = resource_to_addr(acpi_res, &addr);
>         if (!ACPI_SUCCESS(status))
>                 return AE_OK;
> diff --git a/include/acpi/acpi_bus.h b/include/acpi/acpi_bus.h
> index 94383a7..ae7f759 100644
> --- a/include/acpi/acpi_bus.h
> +++ b/include/acpi/acpi_bus.h
> @@ -354,6 +354,10 @@ extern int unregister_acpi_notifier(struct
> notifier_block *);
>
>  extern int register_acpi_bus_notifier(struct notifier_block *nb);
>  extern void unregister_acpi_bus_notifier(struct notifier_block *nb);
> +extern acpi_status
> +resource_to_addr(struct acpi_resource *resource,
> +                        struct acpi_resource_address64 *addr);
> +
>
> Thanks
> Rui
>
>
>>
>> The above function fails on my machine because it does not handle
>> Memory32Fixed resources. Is the following _CRS declaration allowed for
>> IOAPIC?
>>
>>             Device (APIC)
>>             {
>>                 Name (_HID, "ACPI0009")  // _HID: Hardware ID
>>                 Name (_CRS, ResourceTemplate ()  // _CRS: Current
>> Resource Settings
>>                 {
>>                     Memory32Fixed (ReadOnly,
>>                         0xFEC01000,         // Address Base
>>                         0x00001000,         // Address Length
>>                         )
>>                 })
>>                 Method(_GSB) {
>>                         return (0x18)
>>                 }
>>             }
>>
>>
>> Thanks
>> Rui
>>
>>> +    if (!ACPI_SUCCESS(status))
>>> +            return AE_OK;
>>> +
>>> +    if (addr.resource_type == ACPI_MEMORY_RANGE) {
>>> +            if (addr.info.mem.caching == ACPI_PREFETCHABLE_MEMORY)
>>> +                    return AE_OK;
>>> +            flags = IORESOURCE_MEM;
>>> +    } else
>>> +            return AE_OK;
>>> +
>>> +    start = addr.minimum + addr.translation_offset;
>>> +    end = addr.maximum + addr.translation_offset;
>>> +
>>> +    res = data;
>>> +    res->flags = flags;
>>> +    res->start = start;
>>> +    res->end = end;
>>> +
>>> +    return AE_OK;
>>> +}
>>> +
>>>  static void handle_ioapic_add(acpi_handle handle, struct pci_dev **pdev,
>>>                               u32 *pgsi_base)
>>>  {
>>> @@ -54,33 +84,56 @@ static void handle_ioapic_add(acpi_handle handle,
>>> struct
>>> pci_dev **pdev,
>>>              return;
>>>
>>>      dev = acpi_get_pci_dev(handle);
>>> -    if (!dev)
>>> -            return;
>>> +    if (!dev || !pci_resource_len(dev, 0)) {
>>> +            struct acpi_device_info *info;
>>> +            char *hid = NULL;
>>> +
>>> +            status = acpi_get_object_info(handle, &info);
>>> +            if (ACPI_FAILURE(status))
>>> +                    goto exit_put;
>>> +            if (info->valid & ACPI_VALID_HID)
>>> +                    hid = info->hardware_id.string;
>>> +            if (!hid || strcmp(hid, "ACPI0009")) {
>>> +                    kfree(info);
>>> +                    goto exit_put;
>>> +            }
>>> +            kfree(info);
>>> +            memset(res, 0, sizeof(*res));
>>> +            acpi_walk_resources(handle, METHOD_NAME__CRS, setup_res, res);
>>> +            if (!res->flags)
>>> +                    goto exit_put;
>>> +    }
>>>
>>>      acpi_get_name(handle, ACPI_FULL_PATHNAME, &buffer);
>>>
>>>      gsi_base = gsb;
>>>      type = "IOxAPIC";
>>> +    if (dev) {
>>> +            ret = pci_enable_device(dev);
>>> +            if (ret < 0)
>>> +                    goto exit_put;
>>>
>>> -    ret = pci_enable_device(dev);
>>> -    if (ret < 0)
>>> -            goto exit_put;
>>> -
>>> -    pci_set_master(dev);
>>> +            pci_set_master(dev);
>>>
>>> -    if (dev->class == PCI_CLASS_SYSTEM_PIC_IOAPIC)
>>> -            type = "IOAPIC";
>>> +            if (dev->class == PCI_CLASS_SYSTEM_PIC_IOAPIC)
>>> +                    type = "IOAPIC";
>>>
>>> -    if (pci_request_region(dev, 0, type))
>>> -            goto exit_disable;
>>> +            if (pci_resource_len(dev, 0)) {
>>> +                    if (pci_request_region(dev, 0, type))
>>> +                            goto exit_disable;
>>>
>>> -    res = &dev->resource[0];
>>> +                    res = &dev->resource[0];
>>> +            }
>>> +    }
>>>
>>> -    if (acpi_register_ioapic(handle, res->start, gsi_base))
>>> -            goto exit_release;
>>> +    if (acpi_register_ioapic(handle, res->start, gsi_base)) {
>>> +            if (dev)
>>> +                    goto exit_release;
>>> +            return;
>>> +    }
>>>
>>>      pr_info("%s %s %s at %pR, GSI %u\n",
>>> -            dev_name(&dev->dev), objname, type,
>>> +            dev ? dev_name(&dev->dev) : "", objname, type,
>>>              res, gsi_base);
>>>
>>>      *pdev = dev;
>>> @@ -100,6 +153,9 @@ static void handle_ioapic_remove(acpi_handle handle,
>>> struct pci_dev *dev,
>>>  {
>>>      acpi_unregister_ioapic(handle, gsi_base);
>>>
>>> +    if (!dev)
>>> +            return;
>>> +
>>>      pci_release_region(dev, 0);
>>>      pci_disable_device(dev);
>>>      pci_dev_put(dev);
>>> --
>>> 1.8.1.4
>>>
>>> --
>>> To unsubscribe from this list: send the line "unsubscribe linux-kernel"
>>> in
>>> the body of a message to majordomo@vger.kernel.org
>>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>>> Please read the FAQ at  http://www.tux.org/lkml/
>>>
>>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html



-- 
Best regards
Tianyu Lan

Re: [PATCH v4 28/28] PCI, x86, ACPI: get ioapic address from acpi device

[Yinghai Lu]

On Fri, Aug 23, 2013 at 8:34 AM, Lan Tianyu <lantianyu1986@gmail.com> wrote:
>> I worked around the problem by replacing acpi_resource_to_address64()
>> with resource_to_addr().  But resource_to_addr() is a static function
>> in arch/x86/pci/acpi.c, not very convenient to use. Here's what I did:
>>
>
> Hi Rui&Yinghai:
>            How about using the following code to translate struct
>  acpi_resource to struct resouce in this setup_res()?
>
>          if (acpi_dev_resource_address_space(...)
>                 || acpi_dev_resource_memory(..))
>               return AE_OK;

Yest, that could be better, will update that.

Also can you submit patch that will use that in res_to_addr of
arch/x86/pci/acpi.c?

Thanks

Yinghai

Re: [PATCH v4 28/28] PCI, x86, ACPI: get ioapic address from acpi device

[Yinghai Lu]

On Fri, Aug 23, 2013 at 11:38 AM, Yinghai Lu <yinghai@kernel.org> wrote:
> On Fri, Aug 23, 2013 at 8:34 AM, Lan Tianyu <lantianyu1986@gmail.com> wrote:
>>> I worked around the problem by replacing acpi_resource_to_address64()
>>> with resource_to_addr().  But resource_to_addr() is a static function
>>> in arch/x86/pci/acpi.c, not very convenient to use. Here's what I did:
>>>
>>
>> Hi Rui&Yinghai:
>>            How about using the following code to translate struct
>>  acpi_resource to struct resouce in this setup_res()?
>>
>>          if (acpi_dev_resource_address_space(...)
>>                 || acpi_dev_resource_memory(..))
>>               return AE_OK;
>
> Yest, that could be better, will update that.
>
> Also can you submit patch that will use that in res_to_addr of
> arch/x86/pci/acpi.c?

looks acpi_dev_resource_address_space... does not handle
PREFTCH and translation offset.

So now i have to use res_to_addr alike one.

Thanks

Yinghai

Re: [PATCH v4 28/28] PCI, x86, ACPI: get ioapic address from acpi device

[Yinghai Lu]

[-- Attachment #1: Type: text/plain, Size: 1168 bytes --]

On Fri, Aug 23, 2013 at 12:04 PM, Yinghai Lu <yinghai@kernel.org> wrote:
> On Fri, Aug 23, 2013 at 11:38 AM, Yinghai Lu <yinghai@kernel.org> wrote:
>> On Fri, Aug 23, 2013 at 8:34 AM, Lan Tianyu <lantianyu1986@gmail.com> wrote:
>>>> I worked around the problem by replacing acpi_resource_to_address64()
>>>> with resource_to_addr().  But resource_to_addr() is a static function
>>>> in arch/x86/pci/acpi.c, not very convenient to use. Here's what I did:
>>>>
>>>
>>> Hi Rui&Yinghai:
>>>            How about using the following code to translate struct
>>>  acpi_resource to struct resouce in this setup_res()?
>>>
>>>          if (acpi_dev_resource_address_space(...)
>>>                 || acpi_dev_resource_memory(..))
>>>               return AE_OK;
>>
>> Yest, that could be better, will update that.
>>
>> Also can you submit patch that will use that in res_to_addr of
>> arch/x86/pci/acpi.c?
>
> looks acpi_dev_resource_address_space... does not handle
> PREFTCH and translation offset.
>
> So now i have to use res_to_addr alike one.

Raphael,

Maybe we should move resource_to_addr to acpi generic.

Please check if you are ok with attached.

Thanks

Yinghai

[-- Attachment #2: move_res_to_addr.patch --]
[-- Type: application/octet-stream, Size: 5747 bytes --]

Subject: [PATCH] ACPI, x86/PCI: Move resource_to_addr() to acpi generic

Rui wang reported ioapic hot-add does not work on his machine
that _CRS have MEMORY_FIXED.
He also proposed to expose resource_to_addr() and use it for ioapic.

We should move it to acpi generic, as ioapi.c current is in
drivers/pci/.

Or should move it to acpica?

Signed-off-by: Yinghai Lu <yinghai@kernel.org>

---
 arch/x86/pci/acpi.c     |   51 +-----------------------------------------------
 drivers/acpi/resource.c |   46 +++++++++++++++++++++++++++++++++++++++++++
 include/linux/acpi.h    |    3 ++
 3 files changed, 51 insertions(+), 49 deletions(-)

Index: linux-2.6/arch/x86/pci/acpi.c
===================================================================
--- linux-2.6.orig/arch/x86/pci/acpi.c
+++ linux-2.6/arch/x86/pci/acpi.c
@@ -219,60 +219,13 @@ static void teardown_mcfg_map(struct pci
 #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;
 
-	status = resource_to_addr(acpi_res, &addr);
+	status = acpi_mem_addr_resource_to_address64(acpi_res, &addr);
 	if (ACPI_SUCCESS(status))
 		info->res_num++;
 	return AE_OK;
@@ -288,7 +241,7 @@ setup_resource(struct acpi_resource *acp
 	unsigned long flags;
 	u64 start, orig_end, end;
 
-	status = resource_to_addr(acpi_res, &addr);
+	status = acpi_mem_addr_resource_to_address64(acpi_res, &addr);
 	if (!ACPI_SUCCESS(status))
 		return AE_OK;
 
Index: linux-2.6/drivers/acpi/resource.c
===================================================================
--- linux-2.6.orig/drivers/acpi/resource.c
+++ linux-2.6/drivers/acpi/resource.c
@@ -414,6 +414,52 @@ void acpi_dev_free_resource_list(struct
 }
 EXPORT_SYMBOL_GPL(acpi_dev_free_resource_list);
 
+acpi_status acpi_mem_addr_resource_to_address64(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;
+}
+
 struct res_proc_context {
 	struct list_head *list;
 	int (*preproc)(struct acpi_resource *, void *);
Index: linux-2.6/include/linux/acpi.h
===================================================================
--- linux-2.6.orig/include/linux/acpi.h
+++ linux-2.6/include/linux/acpi.h
@@ -259,6 +259,9 @@ unsigned long acpi_dev_irq_flags(u8 trig
 bool acpi_dev_resource_interrupt(struct acpi_resource *ares, int index,
 				 struct resource *res);
 
+acpi_status acpi_mem_addr_resource_to_address64(struct acpi_resource *resource,
+			struct acpi_resource_address64 *addr);
+
 struct resource_list_entry {
 	struct list_head node;
 	struct resource res;

Re: [PATCH v4 28/28] PCI, x86, ACPI: get ioapic address from acpi device

[Lan Tianyu]

2013/8/24 Yinghai Lu <yinghai@kernel.org>:
> On Fri, Aug 23, 2013 at 12:04 PM, Yinghai Lu <yinghai@kernel.org> wrote:
>> On Fri, Aug 23, 2013 at 11:38 AM, Yinghai Lu <yinghai@kernel.org> wrote:
>>> On Fri, Aug 23, 2013 at 8:34 AM, Lan Tianyu <lantianyu1986@gmail.com> wrote:
>>>>> I worked around the problem by replacing acpi_resource_to_address64()
>>>>> with resource_to_addr().  But resource_to_addr() is a static function
>>>>> in arch/x86/pci/acpi.c, not very convenient to use. Here's what I did:
>>>>>
>>>>
>>>> Hi Rui&Yinghai:
>>>>            How about using the following code to translate struct
>>>>  acpi_resource to struct resouce in this setup_res()?
>>>>
>>>>          if (acpi_dev_resource_address_space(...)
>>>>                 || acpi_dev_resource_memory(..))
>>>>               return AE_OK;
>>>
>>> Yest, that could be better, will update that.
>>>
>>> Also can you submit patch that will use that in res_to_addr of
>>> arch/x86/pci/acpi.c?
>>
>> looks acpi_dev_resource_address_space... does not handle
>> PREFTCH and translation offset.
>>
>> So now i have to use res_to_addr alike one.
>
> Raphael,
>
> Maybe we should move resource_to_addr to acpi generic.
>
> Please check if you are ok with attached.
>
> Thanks
>
> Yinghai

Hi Rafael & Yinghai:
            I wrote 4 proposal patches to try to make acpi resource
functions to replace
 resource_to_addr() in the arch/x86/pci/acpi.c. From my opinion,
resource_to_addr()
does most work which acpi resource functions have done. So please have
a look. Thanks.
The following patches just passed through compiling test.

Patch 1
-------------------
commit 1800bc9dda7318314607fbae7afda8be38e056dc
Author: Lan Tianyu <tianyu.lan@intel.com>
Date:   Mon Aug 26 14:40:39 2013 +0800

    ACPI/Resource: Add setting PREFETCH flag support

diff --git a/drivers/acpi/resource.c b/drivers/acpi/resource.c
index b7201fc..23a560b 100644
--- a/drivers/acpi/resource.c
+++ b/drivers/acpi/resource.c
@@ -35,7 +35,7 @@
 #endif

 static unsigned long acpi_dev_memresource_flags(u64 len, u8 write_protect,
-                        bool window)
+                        bool window, bool prefetchable)
 {
     unsigned long flags = IORESOURCE_MEM;

@@ -48,6 +48,9 @@ static unsigned long acpi_dev_memresource_flags(u64
len, u8 write_protect,
     if (window)
         flags |= IORESOURCE_WINDOW;

+    if (prefetchable)
+        flags |= IORESOURCE_PREFETCH;
+
     return flags;
 }

@@ -56,7 +59,8 @@ static void acpi_dev_get_memresource(struct resource
*res, u64 start, u64 len,
 {
     res->start = start;
     res->end = start + len - 1;
-    res->flags = acpi_dev_memresource_flags(len, write_protect, false);
+    res->flags = acpi_dev_memresource_flags(len, write_protect,
+                        false, false);
 }

 /**
@@ -175,7 +179,7 @@ bool acpi_dev_resource_address_space(struct
acpi_resource *ares,
 {
     acpi_status status;
     struct acpi_resource_address64 addr;
-    bool window;
+    bool window, prefetchable;
     u64 len;
     u8 io_decode;

@@ -199,9 +203,11 @@ bool acpi_dev_resource_address_space(struct
acpi_resource *ares,
     switch(addr.resource_type) {
     case ACPI_MEMORY_RANGE:
         len = addr.maximum - addr.minimum + 1;
+        prefetchable =
+            addr.info.mem.caching == ACPI_PREFETCHABLE_MEMORY;
         res->flags = acpi_dev_memresource_flags(len,
                         addr.info.mem.write_protect,
-                        window);
+                        window, prefetchable);
         break;
     case ACPI_IO_RANGE:
         io_decode = addr.granularity == 0xfff ?
@@ -252,7 +258,7 @@ bool acpi_dev_resource_ext_address_space(struct
acpi_resource *ares,
         len = ext_addr->maximum - ext_addr->minimum + 1;
         res->flags = acpi_dev_memresource_flags(len,
                     ext_addr->info.mem.write_protect,
-                    window);
+                    window, false);
         break;
     case ACPI_IO_RANGE:
         io_decode = ext_addr->granularity == 0xfff ?

Patch 2
-----------
commit a3ff8345ffb51885b216ba0ae231252cd88d4e76
Author: Lan Tianyu <tianyu.lan@intel.com>
Date:   Mon Aug 26 15:57:14 2013 +0800

    ACPI/Resource: Add address translation support

diff --git a/drivers/acpi/resource.c b/drivers/acpi/resource.c
index 23a560b..439ee44 100644
--- a/drivers/acpi/resource.c
+++ b/drivers/acpi/resource.c
@@ -196,8 +196,8 @@ bool acpi_dev_resource_address_space(struct
acpi_resource *ares,
     if (ACPI_FAILURE(status))
         return true;

-    res->start = addr.minimum;
-    res->end = addr.maximum;
+    res->start = addr.minimum + addr.translation_offset;
+    res->end = addr.maximum + addr.translation_offset;
     window = addr.producer_consumer == ACPI_PRODUCER;

     switch(addr.resource_type) {


Patch 3
---------------
commit 68bfefe619b49baa5d372e0664a6cb6d5138fad1
Author: Lan Tianyu <tianyu.lan@intel.com>
Date:   Mon Aug 26 21:45:32 2013 +0800

    ACPI: Add new acpi_dev_resource_address_space_with_addr() function

    Add new function which can return the converted struct
acpi_resource_address64.
This will be used to get transaction offset in the setup_resource().

diff --git a/drivers/acpi/resource.c b/drivers/acpi/resource.c
index 439ee44..98a6c35 100644
--- a/drivers/acpi/resource.c
+++ b/drivers/acpi/resource.c
@@ -174,11 +174,11 @@ EXPORT_SYMBOL_GPL(acpi_dev_resource_io);
  * and if that's the case, use the information in it to populate the generic
  * resource object pointed to by @res.
  */
-bool acpi_dev_resource_address_space(struct acpi_resource *ares,
+bool acpi_dev_resource_address_space_with_addr(struct acpi_resource *ares,
+                     struct acpi_resource_address64 *addr,
                      struct resource *res)
 {
     acpi_status status;
-    struct acpi_resource_address64 addr;
     bool window, prefetchable;
     u64 len;
     u8 io_decode;
@@ -192,28 +192,28 @@ bool acpi_dev_resource_address_space(struct
acpi_resource *ares,
         return false;
     }

-    status = acpi_resource_to_address64(ares, &addr);
+    status = acpi_resource_to_address64(ares, addr);
     if (ACPI_FAILURE(status))
         return true;

-    res->start = addr.minimum + addr.translation_offset;
-    res->end = addr.maximum + addr.translation_offset;
-    window = addr.producer_consumer == ACPI_PRODUCER;
+    res->start = addr->minimum + addr->translation_offset;
+    res->end = addr->maximum + addr->translation_offset;
+    window = addr->producer_consumer == ACPI_PRODUCER;

-    switch(addr.resource_type) {
+    switch (addr->resource_type) {
     case ACPI_MEMORY_RANGE:
-        len = addr.maximum - addr.minimum + 1;
+        len = addr->maximum - addr->minimum + 1;
         prefetchable =
-            addr.info.mem.caching == ACPI_PREFETCHABLE_MEMORY;
+            addr->info.mem.caching == ACPI_PREFETCHABLE_MEMORY;
         res->flags = acpi_dev_memresource_flags(len,
-                        addr.info.mem.write_protect,
+                        addr->info.mem.write_protect,
                         window, prefetchable);
         break;
     case ACPI_IO_RANGE:
-        io_decode = addr.granularity == 0xfff ?
+        io_decode = addr->granularity == 0xfff ?
                 ACPI_DECODE_10 : ACPI_DECODE_16;
-        res->flags = acpi_dev_ioresource_flags(addr.minimum,
-                               addr.maximum,
+        res->flags = acpi_dev_ioresource_flags(addr->minimum,
+                               addr->maximum,
                                io_decode, window);
         break;
     case ACPI_BUS_NUMBER_RANGE:
@@ -225,6 +225,16 @@ bool acpi_dev_resource_address_space(struct
acpi_resource *ares,

     return true;
 }
+EXPORT_SYMBOL_GPL(acpi_dev_resource_address_space_with_addr);
+
+bool acpi_dev_resource_address_space(struct acpi_resource *ares,
+                     struct resource *res)
+{
+    struct acpi_resource_address64 addr;
+
+    return acpi_dev_resource_address_space_with_addr(ares, &addr,
+                             res);
+}
 EXPORT_SYMBOL_GPL(acpi_dev_resource_address_space);

 /**
diff --git a/include/linux/acpi.h b/include/linux/acpi.h
index a5db4ae..9f5c0d5 100644
--- a/include/linux/acpi.h
+++ b/include/linux/acpi.h
@@ -260,6 +260,9 @@ bool acpi_dev_resource_memory(struct acpi_resource
*ares, struct resource *res);
 bool acpi_dev_resource_io(struct acpi_resource *ares, struct resource *res);
 bool acpi_dev_resource_address_space(struct acpi_resource *ares,
                      struct resource *res);
+bool acpi_dev_resource_address_space_with_addr(struct acpi_resource *ares,
+                struct acpi_resource_address64 *addr,
+                struct resource *res);
 bool acpi_dev_resource_ext_address_space(struct acpi_resource *ares,
                      struct resource *res);
 unsigned long acpi_dev_irq_flags(u8 triggering, u8 polarity, u8 shareable);

Patch 4
--------------
commit a8733a1da756a257d55e68994268174b84b33670
Author: Lan Tianyu <tianyu.lan@intel.com>
Date:   Mon Aug 26 22:53:38 2013 +0800

    X86/PCI/ACPI: Rework setup_resource() via functions acpi resource functions

diff --git a/arch/x86/pci/acpi.c b/arch/x86/pci/acpi.c
index d641897..d4f85a1 100644
--- a/arch/x86/pci/acpi.c
+++ b/arch/x86/pci/acpi.c
@@ -219,62 +219,15 @@ 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 res;

-    status = resource_to_addr(acpi_res, &addr);
-    if (ACPI_SUCCESS(status))
+    if (acpi_dev_resource_address_space(acpi_res, &res)
+        || acpi_dev_resource_memory(acpi_res, &res))
         info->res_num++;
+
     return AE_OK;
 }

@@ -282,27 +235,18 @@ static acpi_status
 setup_resource(struct acpi_resource *acpi_res, void *data)
 {
     struct pci_root_info *info = data;
-    struct resource *res;
+    struct resource *res = &info->res[info->res_num];
     struct acpi_resource_address64 addr;
-    acpi_status status;
-    unsigned long flags;
     u64 start, orig_end, end;

-    status = resource_to_addr(acpi_res, &addr);
-    if (!ACPI_SUCCESS(status))
-        return AE_OK;
+    memset(&addr, 0x00, sizeof(addr));

-    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
+    if (!(acpi_dev_resource_address_space_with_addr(acpi_res, &addr, res)
+        || acpi_dev_resource_memory(acpi_res, res)))
         return AE_OK;

-    start = addr.minimum + addr.translation_offset;
-    orig_end = end = addr.maximum + addr.translation_offset;
+    start = res->start;
+    orig_end = end = res->end;

     /* Exclude non-addressable range or non-addressable portion of range */
     end = min(end, (u64)iomem_resource.end);
@@ -310,6 +254,7 @@ setup_resource(struct acpi_resource *acpi_res, void *data)
         dev_info(&info->bridge->dev,
             "host bridge window [%#llx-%#llx] "
             "(ignored, not CPU addressable)\n", start, orig_end);
+        memset(&info->res[info->res_num], 0x00, sizeof(*res));
         return AE_OK;
     } else if (orig_end != end) {
         dev_info(&info->bridge->dev,
@@ -318,11 +263,9 @@ setup_resource(struct acpi_resource *acpi_res, void *data)
             start, orig_end, 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++;



-- 
Best regards
Tianyu Lan

Re: [PATCH v4 28/28] PCI, x86, ACPI: get ioapic address from acpi device

[Yinghai Lu]

On Mon, Aug 26, 2013 at 8:46 AM, Lan Tianyu <lantianyu1986@gmail.com> wrote:
> 2013/8/24 Yinghai Lu <yinghai@kernel.org>:
>> On Fri, Aug 23, 2013 at 12:04 PM, Yinghai Lu <yinghai@kernel.org> wrote:
>>> On Fri, Aug 23, 2013 at 11:38 AM, Yinghai Lu <yinghai@kernel.org> wrote:
>>>> On Fri, Aug 23, 2013 at 8:34 AM, Lan Tianyu <lantianyu1986@gmail.com> wrote:
>>>>>> I worked around the problem by replacing acpi_resource_to_address64()
>>>>>> with resource_to_addr().  But resource_to_addr() is a static function
>>>>>> in arch/x86/pci/acpi.c, not very convenient to use. Here's what I did:
>>>>>>
>>>>>
>>>>> Hi Rui&Yinghai:
>>>>>            How about using the following code to translate struct
>>>>>  acpi_resource to struct resouce in this setup_res()?
>>>>>
>>>>>          if (acpi_dev_resource_address_space(...)
>>>>>                 || acpi_dev_resource_memory(..))
>>>>>               return AE_OK;
>>>>
>>>> Yest, that could be better, will update that.
>>>>
>>>> Also can you submit patch that will use that in res_to_addr of
>>>> arch/x86/pci/acpi.c?
>>>
>>> looks acpi_dev_resource_address_space... does not handle
>>> PREFTCH and translation offset.
>>>
>>> So now i have to use res_to_addr alike one.
>>
>> Raphael,
>>
>> Maybe we should move resource_to_addr to acpi generic.
>>
>> Please check if you are ok with attached.
>>
>> Thanks
>>
>> Yinghai
>
> Hi Rafael & Yinghai:
>             I wrote 4 proposal patches to try to make acpi resource
> functions to replace
>  resource_to_addr() in the arch/x86/pci/acpi.c. From my opinion,
> resource_to_addr()
> does most work which acpi resource functions have done. So please have
> a look. Thanks.
> The following patches just passed through compiling test.

Thanks for doing that.

Overall it's Good to me.

some comments inline.

>
> Patch 1
> -------------------
> commit 1800bc9dda7318314607fbae7afda8be38e056dc
> Author: Lan Tianyu <tianyu.lan@intel.com>
> Date:   Mon Aug 26 14:40:39 2013 +0800
>
>     ACPI/Resource: Add setting PREFETCH flag support
>
> diff --git a/drivers/acpi/resource.c b/drivers/acpi/resource.c
> index b7201fc..23a560b 100644
> --- a/drivers/acpi/resource.c
> +++ b/drivers/acpi/resource.c
> @@ -35,7 +35,7 @@
>  #endif
>
>  static unsigned long acpi_dev_memresource_flags(u64 len, u8 write_protect,
> -                        bool window)
> +                        bool window, bool prefetchable)
>  {
>      unsigned long flags = IORESOURCE_MEM;
>
> @@ -48,6 +48,9 @@ static unsigned long acpi_dev_memresource_flags(u64
> len, u8 write_protect,
>      if (window)
>          flags |= IORESOURCE_WINDOW;
>
> +    if (prefetchable)
> +        flags |= IORESOURCE_PREFETCH;
> +
>      return flags;
>  }
>
> @@ -56,7 +59,8 @@ static void acpi_dev_get_memresource(struct resource
> *res, u64 start, u64 len,
>  {
>      res->start = start;
>      res->end = start + len - 1;
> -    res->flags = acpi_dev_memresource_flags(len, write_protect, false);
> +    res->flags = acpi_dev_memresource_flags(len, write_protect,
> +                        false, false);
>  }

passing write_protect, window then pref looks silly.

may use | IO_REOURCE_... etc outside of acpi_dev_memresource_flags directly.

>
>  /**
> @@ -175,7 +179,7 @@ bool acpi_dev_resource_address_space(struct
> acpi_resource *ares,
>  {
>      acpi_status status;
>      struct acpi_resource_address64 addr;
> -    bool window;
> +    bool window, prefetchable;
>      u64 len;
>      u8 io_decode;
>
> @@ -199,9 +203,11 @@ bool acpi_dev_resource_address_space(struct
> acpi_resource *ares,
>      switch(addr.resource_type) {
>      case ACPI_MEMORY_RANGE:
>          len = addr.maximum - addr.minimum + 1;
> +        prefetchable =
> +            addr.info.mem.caching == ACPI_PREFETCHABLE_MEMORY;
>          res->flags = acpi_dev_memresource_flags(len,
>                          addr.info.mem.write_protect,
> -                        window);
> +                        window, prefetchable);
>          break;
>      case ACPI_IO_RANGE:
>          io_decode = addr.granularity == 0xfff ?
> @@ -252,7 +258,7 @@ bool acpi_dev_resource_ext_address_space(struct
> acpi_resource *ares,
>          len = ext_addr->maximum - ext_addr->minimum + 1;
>          res->flags = acpi_dev_memresource_flags(len,
>                      ext_addr->info.mem.write_protect,
> -                    window);
> +                    window, false);
>          break;
>      case ACPI_IO_RANGE:
>          io_decode = ext_addr->granularity == 0xfff ?
>
> Patch 2
> -----------
> commit a3ff8345ffb51885b216ba0ae231252cd88d4e76
> Author: Lan Tianyu <tianyu.lan@intel.com>
> Date:   Mon Aug 26 15:57:14 2013 +0800
>
>     ACPI/Resource: Add address translation support
>
> diff --git a/drivers/acpi/resource.c b/drivers/acpi/resource.c
> index 23a560b..439ee44 100644
> --- a/drivers/acpi/resource.c
> +++ b/drivers/acpi/resource.c
> @@ -196,8 +196,8 @@ bool acpi_dev_resource_address_space(struct
> acpi_resource *ares,
>      if (ACPI_FAILURE(status))
>          return true;
>
> -    res->start = addr.minimum;
> -    res->end = addr.maximum;
> +    res->start = addr.minimum + addr.translation_offset;
> +    res->end = addr.maximum + addr.translation_offset;
>      window = addr.producer_consumer == ACPI_PRODUCER;
>
>      switch(addr.resource_type) {
>
>
> Patch 3
> ---------------
> commit 68bfefe619b49baa5d372e0664a6cb6d5138fad1
> Author: Lan Tianyu <tianyu.lan@intel.com>
> Date:   Mon Aug 26 21:45:32 2013 +0800
>
>     ACPI: Add new acpi_dev_resource_address_space_with_addr() function
>
>     Add new function which can return the converted struct
> acpi_resource_address64.
> This will be used to get transaction offset in the setup_resource().
>
> diff --git a/drivers/acpi/resource.c b/drivers/acpi/resource.c
> index 439ee44..98a6c35 100644
> --- a/drivers/acpi/resource.c
> +++ b/drivers/acpi/resource.c
> @@ -174,11 +174,11 @@ EXPORT_SYMBOL_GPL(acpi_dev_resource_io);
>   * and if that's the case, use the information in it to populate the generic
>   * resource object pointed to by @res.
>   */
> -bool acpi_dev_resource_address_space(struct acpi_resource *ares,
> +bool acpi_dev_resource_address_space_with_addr(struct acpi_resource *ares,
> +                     struct acpi_resource_address64 *addr,
>                       struct resource *res)
>  {
>      acpi_status status;
> -    struct acpi_resource_address64 addr;
>      bool window, prefetchable;
>      u64 len;
>      u8 io_decode;
> @@ -192,28 +192,28 @@ bool acpi_dev_resource_address_space(struct
> acpi_resource *ares,
>          return false;
>      }
>
> -    status = acpi_resource_to_address64(ares, &addr);
> +    status = acpi_resource_to_address64(ares, addr);
>      if (ACPI_FAILURE(status))
>          return true;
>
> -    res->start = addr.minimum + addr.translation_offset;
> -    res->end = addr.maximum + addr.translation_offset;
> -    window = addr.producer_consumer == ACPI_PRODUCER;
> +    res->start = addr->minimum + addr->translation_offset;
> +    res->end = addr->maximum + addr->translation_offset;
> +    window = addr->producer_consumer == ACPI_PRODUCER;
>
> -    switch(addr.resource_type) {
> +    switch (addr->resource_type) {
>      case ACPI_MEMORY_RANGE:
> -        len = addr.maximum - addr.minimum + 1;
> +        len = addr->maximum - addr->minimum + 1;
>          prefetchable =
> -            addr.info.mem.caching == ACPI_PREFETCHABLE_MEMORY;
> +            addr->info.mem.caching == ACPI_PREFETCHABLE_MEMORY;
>          res->flags = acpi_dev_memresource_flags(len,
> -                        addr.info.mem.write_protect,
> +                        addr->info.mem.write_protect,
>                          window, prefetchable);
>          break;
>      case ACPI_IO_RANGE:
> -        io_decode = addr.granularity == 0xfff ?
> +        io_decode = addr->granularity == 0xfff ?
>                  ACPI_DECODE_10 : ACPI_DECODE_16;
> -        res->flags = acpi_dev_ioresource_flags(addr.minimum,
> -                               addr.maximum,
> +        res->flags = acpi_dev_ioresource_flags(addr->minimum,
> +                               addr->maximum,
>                                 io_decode, window);
>          break;
>      case ACPI_BUS_NUMBER_RANGE:
> @@ -225,6 +225,16 @@ bool acpi_dev_resource_address_space(struct
> acpi_resource *ares,
>
>      return true;
>  }
> +EXPORT_SYMBOL_GPL(acpi_dev_resource_address_space_with_addr);
> +
> +bool acpi_dev_resource_address_space(struct acpi_resource *ares,
> +                     struct resource *res)
> +{
> +    struct acpi_resource_address64 addr;
> +
> +    return acpi_dev_resource_address_space_with_addr(ares, &addr,
> +                             res);
> +}
>  EXPORT_SYMBOL_GPL(acpi_dev_resource_address_space);
>
>  /**
> diff --git a/include/linux/acpi.h b/include/linux/acpi.h
> index a5db4ae..9f5c0d5 100644
> --- a/include/linux/acpi.h
> +++ b/include/linux/acpi.h
> @@ -260,6 +260,9 @@ bool acpi_dev_resource_memory(struct acpi_resource
> *ares, struct resource *res);
>  bool acpi_dev_resource_io(struct acpi_resource *ares, struct resource *res);
>  bool acpi_dev_resource_address_space(struct acpi_resource *ares,
>                       struct resource *res);
> +bool acpi_dev_resource_address_space_with_addr(struct acpi_resource *ares,
> +                struct acpi_resource_address64 *addr,
> +                struct resource *res);
>  bool acpi_dev_resource_ext_address_space(struct acpi_resource *ares,
>                       struct resource *res);
>  unsigned long acpi_dev_irq_flags(u8 triggering, u8 polarity, u8 shareable);
>
> Patch 4
> --------------
> commit a8733a1da756a257d55e68994268174b84b33670
> Author: Lan Tianyu <tianyu.lan@intel.com>
> Date:   Mon Aug 26 22:53:38 2013 +0800
>
>     X86/PCI/ACPI: Rework setup_resource() via functions acpi resource functions
>
> diff --git a/arch/x86/pci/acpi.c b/arch/x86/pci/acpi.c
> index d641897..d4f85a1 100644
> --- a/arch/x86/pci/acpi.c
> +++ b/arch/x86/pci/acpi.c
> @@ -219,62 +219,15 @@ 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 res;
>
> -    status = resource_to_addr(acpi_res, &addr);
> -    if (ACPI_SUCCESS(status))
> +    if (acpi_dev_resource_address_space(acpi_res, &res)
> +        || acpi_dev_resource_memory(acpi_res, &res))
>          info->res_num++;
> +
>      return AE_OK;
>  }
>
> @@ -282,27 +235,18 @@ static acpi_status
>  setup_resource(struct acpi_resource *acpi_res, void *data)
>  {
>      struct pci_root_info *info = data;
> -    struct resource *res;
> +    struct resource *res = &info->res[info->res_num];
>      struct acpi_resource_address64 addr;
> -    acpi_status status;
> -    unsigned long flags;
>      u64 start, orig_end, end;
>
> -    status = resource_to_addr(acpi_res, &addr);
> -    if (!ACPI_SUCCESS(status))
> -        return AE_OK;
> +    memset(&addr, 0x00, sizeof(addr));
>
> -    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
> +    if (!(acpi_dev_resource_address_space_with_addr(acpi_res, &addr, res)
> +        || acpi_dev_resource_memory(acpi_res, res)))

acpi_dev_resource_address_space_with_addr(acpi_res, &addr, res)

passing extra addr, just for translation_offset?

maybe pass res_offset directly?




>          return AE_OK;
>
> -    start = addr.minimum + addr.translation_offset;
> -    orig_end = end = addr.maximum + addr.translation_offset;
> +    start = res->start;
> +    orig_end = end = res->end;

Yinghai

Re: [PATCH v4 28/28] PCI, x86, ACPI: get ioapic address from acpi device

[Lan Tianyu]

2013/8/27 Yinghai Lu <yinghai@kernel.org>:
> On Mon, Aug 26, 2013 at 8:46 AM, Lan Tianyu <lantianyu1986@gmail.com> wrote:
>> 2013/8/24 Yinghai Lu <yinghai@kernel.org>:
>>> On Fri, Aug 23, 2013 at 12:04 PM, Yinghai Lu <yinghai@kernel.org> wrote:
>>>> On Fri, Aug 23, 2013 at 11:38 AM, Yinghai Lu <yinghai@kernel.org> wrote:
>>>>> On Fri, Aug 23, 2013 at 8:34 AM, Lan Tianyu <lantianyu1986@gmail.com> wrote:
>>>>>>> I worked around the problem by replacing acpi_resource_to_address64()
>>>>>>> with resource_to_addr().  But resource_to_addr() is a static function
>>>>>>> in arch/x86/pci/acpi.c, not very convenient to use. Here's what I did:
>>>>>>>
>>>>>>
>>>>>> Hi Rui&Yinghai:
>>>>>>            How about using the following code to translate struct
>>>>>>  acpi_resource to struct resouce in this setup_res()?
>>>>>>
>>>>>>          if (acpi_dev_resource_address_space(...)
>>>>>>                 || acpi_dev_resource_memory(..))
>>>>>>               return AE_OK;
>>>>>
>>>>> Yest, that could be better, will update that.
>>>>>
>>>>> Also can you submit patch that will use that in res_to_addr of
>>>>> arch/x86/pci/acpi.c?
>>>>
>>>> looks acpi_dev_resource_address_space... does not handle
>>>> PREFTCH and translation offset.
>>>>
>>>> So now i have to use res_to_addr alike one.
>>>
>>> Raphael,
>>>
>>> Maybe we should move resource_to_addr to acpi generic.
>>>
>>> Please check if you are ok with attached.
>>>
>>> Thanks
>>>
>>> Yinghai
>>
>> Hi Rafael & Yinghai:
>>             I wrote 4 proposal patches to try to make acpi resource
>> functions to replace
>>  resource_to_addr() in the arch/x86/pci/acpi.c. From my opinion,
>> resource_to_addr()
>> does most work which acpi resource functions have done. So please have
>> a look. Thanks.
>> The following patches just passed through compiling test.
>
> Thanks for doing that.
>
> Overall it's Good to me.
>
> some comments inline.
>
>>
>> Patch 1
>> -------------------
>> commit 1800bc9dda7318314607fbae7afda8be38e056dc
>> Author: Lan Tianyu <tianyu.lan@intel.com>
>> Date:   Mon Aug 26 14:40:39 2013 +0800
>>
>>     ACPI/Resource: Add setting PREFETCH flag support
>>
>> diff --git a/drivers/acpi/resource.c b/drivers/acpi/resource.c
>> index b7201fc..23a560b 100644
>> --- a/drivers/acpi/resource.c
>> +++ b/drivers/acpi/resource.c
>> @@ -35,7 +35,7 @@
>>  #endif
>>
>>  static unsigned long acpi_dev_memresource_flags(u64 len, u8 write_protect,
>> -                        bool window)
>> +                        bool window, bool prefetchable)
>>  {
>>      unsigned long flags = IORESOURCE_MEM;
>>
>> @@ -48,6 +48,9 @@ static unsigned long acpi_dev_memresource_flags(u64
>> len, u8 write_protect,
>>      if (window)
>>          flags |= IORESOURCE_WINDOW;
>>
>> +    if (prefetchable)
>> +        flags |= IORESOURCE_PREFETCH;
>> +
>>      return flags;
>>  }
>>
>> @@ -56,7 +59,8 @@ static void acpi_dev_get_memresource(struct resource
>> *res, u64 start, u64 len,
>>  {
>>      res->start = start;
>>      res->end = start + len - 1;
>> -    res->flags = acpi_dev_memresource_flags(len, write_protect, false);
>> +    res->flags = acpi_dev_memresource_flags(len, write_protect,
>> +                        false, false);
>>  }
>
> passing write_protect, window then pref looks silly.
>
> may use | IO_REOURCE_... etc outside of acpi_dev_memresource_flags directly.
>

Yes, this looks better.

>>
>>  /**
>> @@ -175,7 +179,7 @@ bool acpi_dev_resource_address_space(struct
>> acpi_resource *ares,
>>  {
>>      acpi_status status;
>>      struct acpi_resource_address64 addr;
>> -    bool window;
>> +    bool window, prefetchable;
>>      u64 len;
>>      u8 io_decode;
>>
>> @@ -199,9 +203,11 @@ bool acpi_dev_resource_address_space(struct
>> acpi_resource *ares,
>>      switch(addr.resource_type) {
>>      case ACPI_MEMORY_RANGE:
>>          len = addr.maximum - addr.minimum + 1;
>> +        prefetchable =
>> +            addr.info.mem.caching == ACPI_PREFETCHABLE_MEMORY;
>>          res->flags = acpi_dev_memresource_flags(len,
>>                          addr.info.mem.write_protect,
>> -                        window);
>> +                        window, prefetchable);
>>          break;
>>      case ACPI_IO_RANGE:
>>          io_decode = addr.granularity == 0xfff ?
>> @@ -252,7 +258,7 @@ bool acpi_dev_resource_ext_address_space(struct
>> acpi_resource *ares,
>>          len = ext_addr->maximum - ext_addr->minimum + 1;
>>          res->flags = acpi_dev_memresource_flags(len,
>>                      ext_addr->info.mem.write_protect,
>> -                    window);
>> +                    window, false);
>>          break;
>>      case ACPI_IO_RANGE:
>>          io_decode = ext_addr->granularity == 0xfff ?
>>
>> Patch 2
>> -----------
>> commit a3ff8345ffb51885b216ba0ae231252cd88d4e76
>> Author: Lan Tianyu <tianyu.lan@intel.com>
>> Date:   Mon Aug 26 15:57:14 2013 +0800
>>
>>     ACPI/Resource: Add address translation support
>>
>> diff --git a/drivers/acpi/resource.c b/drivers/acpi/resource.c
>> index 23a560b..439ee44 100644
>> --- a/drivers/acpi/resource.c
>> +++ b/drivers/acpi/resource.c
>> @@ -196,8 +196,8 @@ bool acpi_dev_resource_address_space(struct
>> acpi_resource *ares,
>>      if (ACPI_FAILURE(status))
>>          return true;
>>
>> -    res->start = addr.minimum;
>> -    res->end = addr.maximum;
>> +    res->start = addr.minimum + addr.translation_offset;
>> +    res->end = addr.maximum + addr.translation_offset;
>>      window = addr.producer_consumer == ACPI_PRODUCER;
>>
>>      switch(addr.resource_type) {
>>
>>
>> Patch 3
>> ---------------
>> commit 68bfefe619b49baa5d372e0664a6cb6d5138fad1
>> Author: Lan Tianyu <tianyu.lan@intel.com>
>> Date:   Mon Aug 26 21:45:32 2013 +0800
>>
>>     ACPI: Add new acpi_dev_resource_address_space_with_addr() function
>>
>>     Add new function which can return the converted struct
>> acpi_resource_address64.
>> This will be used to get transaction offset in the setup_resource().
>>
>> diff --git a/drivers/acpi/resource.c b/drivers/acpi/resource.c
>> index 439ee44..98a6c35 100644
>> --- a/drivers/acpi/resource.c
>> +++ b/drivers/acpi/resource.c
>> @@ -174,11 +174,11 @@ EXPORT_SYMBOL_GPL(acpi_dev_resource_io);
>>   * and if that's the case, use the information in it to populate the generic
>>   * resource object pointed to by @res.
>>   */
>> -bool acpi_dev_resource_address_space(struct acpi_resource *ares,
>> +bool acpi_dev_resource_address_space_with_addr(struct acpi_resource *ares,
>> +                     struct acpi_resource_address64 *addr,
>>                       struct resource *res)
>>  {
>>      acpi_status status;
>> -    struct acpi_resource_address64 addr;
>>      bool window, prefetchable;
>>      u64 len;
>>      u8 io_decode;
>> @@ -192,28 +192,28 @@ bool acpi_dev_resource_address_space(struct
>> acpi_resource *ares,
>>          return false;
>>      }
>>
>> -    status = acpi_resource_to_address64(ares, &addr);
>> +    status = acpi_resource_to_address64(ares, addr);
>>      if (ACPI_FAILURE(status))
>>          return true;
>>
>> -    res->start = addr.minimum + addr.translation_offset;
>> -    res->end = addr.maximum + addr.translation_offset;
>> -    window = addr.producer_consumer == ACPI_PRODUCER;
>> +    res->start = addr->minimum + addr->translation_offset;
>> +    res->end = addr->maximum + addr->translation_offset;
>> +    window = addr->producer_consumer == ACPI_PRODUCER;
>>
>> -    switch(addr.resource_type) {
>> +    switch (addr->resource_type) {
>>      case ACPI_MEMORY_RANGE:
>> -        len = addr.maximum - addr.minimum + 1;
>> +        len = addr->maximum - addr->minimum + 1;
>>          prefetchable =
>> -            addr.info.mem.caching == ACPI_PREFETCHABLE_MEMORY;
>> +            addr->info.mem.caching == ACPI_PREFETCHABLE_MEMORY;
>>          res->flags = acpi_dev_memresource_flags(len,
>> -                        addr.info.mem.write_protect,
>> +                        addr->info.mem.write_protect,
>>                          window, prefetchable);
>>          break;
>>      case ACPI_IO_RANGE:
>> -        io_decode = addr.granularity == 0xfff ?
>> +        io_decode = addr->granularity == 0xfff ?
>>                  ACPI_DECODE_10 : ACPI_DECODE_16;
>> -        res->flags = acpi_dev_ioresource_flags(addr.minimum,
>> -                               addr.maximum,
>> +        res->flags = acpi_dev_ioresource_flags(addr->minimum,
>> +                               addr->maximum,
>>                                 io_decode, window);
>>          break;
>>      case ACPI_BUS_NUMBER_RANGE:
>> @@ -225,6 +225,16 @@ bool acpi_dev_resource_address_space(struct
>> acpi_resource *ares,
>>
>>      return true;
>>  }
>> +EXPORT_SYMBOL_GPL(acpi_dev_resource_address_space_with_addr);
>> +
>> +bool acpi_dev_resource_address_space(struct acpi_resource *ares,
>> +                     struct resource *res)
>> +{
>> +    struct acpi_resource_address64 addr;
>> +
>> +    return acpi_dev_resource_address_space_with_addr(ares, &addr,
>> +                             res);
>> +}
>>  EXPORT_SYMBOL_GPL(acpi_dev_resource_address_space);
>>
>>  /**
>> diff --git a/include/linux/acpi.h b/include/linux/acpi.h
>> index a5db4ae..9f5c0d5 100644
>> --- a/include/linux/acpi.h
>> +++ b/include/linux/acpi.h
>> @@ -260,6 +260,9 @@ bool acpi_dev_resource_memory(struct acpi_resource
>> *ares, struct resource *res);
>>  bool acpi_dev_resource_io(struct acpi_resource *ares, struct resource *res);
>>  bool acpi_dev_resource_address_space(struct acpi_resource *ares,
>>                       struct resource *res);
>> +bool acpi_dev_resource_address_space_with_addr(struct acpi_resource *ares,
>> +                struct acpi_resource_address64 *addr,
>> +                struct resource *res);
>>  bool acpi_dev_resource_ext_address_space(struct acpi_resource *ares,
>>                       struct resource *res);
>>  unsigned long acpi_dev_irq_flags(u8 triggering, u8 polarity, u8 shareable);
>>
>> Patch 4
>> --------------
>> commit a8733a1da756a257d55e68994268174b84b33670
>> Author: Lan Tianyu <tianyu.lan@intel.com>
>> Date:   Mon Aug 26 22:53:38 2013 +0800
>>
>>     X86/PCI/ACPI: Rework setup_resource() via functions acpi resource functions
>>
>> diff --git a/arch/x86/pci/acpi.c b/arch/x86/pci/acpi.c
>> index d641897..d4f85a1 100644
>> --- a/arch/x86/pci/acpi.c
>> +++ b/arch/x86/pci/acpi.c
>> @@ -219,62 +219,15 @@ 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 res;
>>
>> -    status = resource_to_addr(acpi_res, &addr);
>> -    if (ACPI_SUCCESS(status))
>> +    if (acpi_dev_resource_address_space(acpi_res, &res)
>> +        || acpi_dev_resource_memory(acpi_res, &res))
>>          info->res_num++;
>> +
>>      return AE_OK;
>>  }
>>
>> @@ -282,27 +235,18 @@ static acpi_status
>>  setup_resource(struct acpi_resource *acpi_res, void *data)
>>  {
>>      struct pci_root_info *info = data;
>> -    struct resource *res;
>> +    struct resource *res = &info->res[info->res_num];
>>      struct acpi_resource_address64 addr;
>> -    acpi_status status;
>> -    unsigned long flags;
>>      u64 start, orig_end, end;
>>
>> -    status = resource_to_addr(acpi_res, &addr);
>> -    if (!ACPI_SUCCESS(status))
>> -        return AE_OK;
>> +    memset(&addr, 0x00, sizeof(addr));
>>
>> -    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
>> +    if (!(acpi_dev_resource_address_space_with_addr(acpi_res, &addr, res)
>> +        || acpi_dev_resource_memory(acpi_res, res)))
>
> acpi_dev_resource_address_space_with_addr(acpi_res, &addr, res)
>
> passing extra addr, just for translation_offset?
>

For this case,  it's "Yes", . For some other cases(e.g
arch/ia64/pci/pci.c add_window()),
whole addr maybe necessary. To be more general, I hope it can return more infos

> maybe pass res_offset directly?
>
>
>
>
>>          return AE_OK;
>>
>> -    start = addr.minimum + addr.translation_offset;
>> -    orig_end = end = addr.maximum + addr.translation_offset;
>> +    start = res->start;
>> +    orig_end = end = res->end;
>
> Yinghai



-- 
Best regards
Tianyu Lan