Skip to contentSkip to conversation

x86, irq: Support ioapic controller hotplug

PATCH In review Yinghai Lu 5 participants 56 messages Jan 3, 2014 – Feb 25, 2014

Subsystems (11)
Stable branch x86 ACPI PCI +7 more subsystems
lore
Share & tools
lore

Yinghai Lu

author
#0 lore raw

v5 cover x86, irq: Support ioapic controller hotplug

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.
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 2014/01/02 aka v3.11-rc6.

could get them from:

        git://git.kernel.org/pub/scm/linux/kernel/git/yinghai/linux-yinghai.git for-x86-irq-3.14

-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.
-v5: refresh to 3.13-rc6. also add patches to move resource_to_addr to generic code.

Rui Wang (1):
x86, ioapic: Hotadd of IOAPICs described in static MADT

Yinghai Lu (32):
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
ACPI: Make map_mat_entry handle x2apic entry
ACPI: Move acpi_get_cpuid() to separated file
ACPI, ioapic: Add acpi_get_ioapic_id()
PCI, x86, ACPI: Link acpi ioapic register to ioapic
PCI, x86, ACPI: Enable ioapic hotplug support with acpi host bridge. ACPI, x86/PCI: Move resource_to_addr() to acpi generic 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        |  20 +-
 arch/x86/kernel/acpi/boot.c          |  40 ++-
 arch/x86/kernel/apic/apic.c          |   9 +-
 arch/x86/kernel/apic/io_apic.c       | 485 +++++++++++++++++++++++++----------
 arch/x86/kernel/devicetree.c         |   2 +-
 arch/x86/pci/acpi.c                  |  51 +---
 drivers/acpi/Makefile                |   1 +
 drivers/acpi/apic_id.c               | 294 +++++++++++++++++++++
 drivers/acpi/pci_root.c              |   4 +
 drivers/acpi/processor_core.c        | 191 --------------
 drivers/acpi/resource.c              |  46 ++++
 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                 | 230 ++++++++++++-----
 drivers/xen/events.c                 |   8 +-
 include/acpi/processor.h             |   3 -
 include/linux/acpi.h                 |  16 ++
 include/linux/irq.h                  |   4 +
 include/linux/pci-acpi.h             |   8 +
 kernel/irq/irqdesc.c                 |  80 ++++--
 27 files changed, 1050 insertions(+), 508 deletions(-)
 create mode 100644 drivers/acpi/apic_id.c
Signature · 1 line
1.8.4
author
#1 lore raw

PATCH v5 1/33 genirq: Split __irq_reserve_irqs from irq_alloc_descs

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.

---
 include/linux/irq.h  |  1 +
 kernel/irq/irqdesc.c | 54 ++++++++++++++++++++++++++++++----------------------
 2 files changed, 32 insertions(+), 23 deletions(-)
include/linux/irq.h +1 -0
index 7dc1003..0229caf 100644
@@ -614,6 +614,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)
 {
kernel/irq/irqdesc.c +31 -23

88 lines not on this page · raw message

Signature · 1 line
1.8.4
author
#2 lore raw

PATCH v5 2/33 genirq: Add irq_alloc_reserved_desc()

In reply to 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.

Cc Joerg Roedel <[email protected]>
Cc Konrad Rzeszutek Wilk <[email protected]>
Cc Sebastian Andrzej Siewior <[email protected]>
---
 include/linux/irq.h  |  3 +++
 kernel/irq/irqdesc.c | 23 +++++++++++++++++++++++
 2 files changed, 26 insertions(+)
include/linux/irq.h +3 -0
index 0229caf..e5f6493 100644
@@ -595,10 +595,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)
kernel/irq/irqdesc.c +23 -0
index a151db6..1166545 100644
@@ -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
Signature · 1 line
1.8.4
#3 lore raw

Re: [PATCH v5 02/33] genirq: Add irq_alloc_reserved_desc()

Yinghai,

Quoted 2 lines · Yinghai Lu

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

I really don't care about easy. Easy to solve problems are for wimps.

What you really want to say is, that ioapic hot-add support requires a contiguous irq number space for a hotplugged ioapic to avoid expensive translations in the ioapic hotplug code.

That's a proper reason for making that change to the core code.

Review

Quoted 12 lines

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.

(quoted diff) +0 -2
-v2: update changelog by adding reasons, requested by Konrad.
-v3: according to tglx:
       separate core code change with arch code change.

Thanks for splitting the patches!

Now the scope of this change becomes more obvious and what I already suspected before becomes crystal clear.

The initial intention of irq_reserve_irqs() was to cope with the legacy interrupts to prevent the dynamic allocator from giving them out, but it was at least a misnomer if not even a misconception.

Did you notice that? No!

Did you even think why irq_reserve_irqs() exists? No!

You just hacked it into submission for your purpose. As usual, sigh!

What prevents a user of __irq_alloc_reserved_desc() to request something completely out of its range? Nothing as you happily return an existing interrupt via:

+ if (irq_to_desc(irq))
+ return irq;

which is true for all already existing interrupts. So some random off by one is going to cause a spurious and extremly hard to debug issue. Brilliant.

No, we are not going to play the "it works for Yinghai" game again. I wasted enough time with that already.

There is a clear step by step approach to get this done proper:

1) Get rid of the existing misconception/misnomer of irq_reserve_irqs().

Make it explicit that this is dealing with legacy irq spaces. It's not that hard as there are only two users in tree which are both trivial to fix.

2) Provide a proper reservation mechanism which does not piggypack blindly on the allocation bitmap.

So what you want is a reservation which:

A) Marks the irq range in the allocation bitmap

This prevents other code pathes to stomp on that range.

B) Stores a unique generated ID in a separate radix tree for that particular irq range.

The generated ID is returned to the caller as it is required for actually allocating an interrupt from that range.

We don't have to bother with making this conditional as the initial memory consumption of the radix tree is minimal and we only expand it when we actually use that hotplug feature.

3) Provide a proper alloc_reserved_irqdesc() function

This function verifies against the reservation ID which was handed out by the reservation function.

It's questionable whether we want to allow the reuse of already allocated irq descriptors. I'm leaning to avoid that. See #4

4) Provide a proper mechanism to free the registered irq descriptors and the reservation range when the physical device is removed from the system. So you don't have to preserve state in the ioapic code. Physical hotplug is not a high frequency hotpath operation.

5) Modify the x86 ioapic code to always use the reserve first and allocate later mechanism to avoid ifdeffery and pointless conditional code pathes. That also ensures proper test coverage.

TBH, I could not be bothered to look at your x86 related changes, but I expect they are from the "make it work for Yinghai" departement as well. I'll review them once the core code changes are in an acceptable shape.

Thanks,

tglx

author
#4 lore raw

Re: [PATCH v5 02/33] genirq: Add irq_alloc_reserved_desc()

Quoted 9 lines · Thomas Gleixner

There is a clear step by step approach to get this done proper:

1) Get rid of the existing misconception/misnomer of irq_reserve_irqs().

Make it explicit that this is dealing with legacy irq spaces. It's not that hard as there are only two users in tree which are both trivial to fix.

Hi, Thomas,

While going through the code for kill irq_reserve_irqs(), I found that there is irq_reserve_irq().

in include/linux/irq.h

static inline int irq_reserve_irq(unsigned int irq)

{
        return irq_reserve_irqs(irq, 1);
}

it is called via kernel/irq/chip.c::irq_set_chip().

/*
* For !CONFIG_SPARSE_IRQ make the irq show up in
* allocated_irqs. For the CONFIG_SPARSE_IRQ case, it is
* already marked, and this call is harmless. */
irq_reserve_irq(irq);

There are tens of irq_set_chip... calling for arches that does not support SPARSE_IRQ yet, and they does not use irq_alloc_desc() anywhere.

so how about change those irq_reserve_irq to irq_set_allocated_irqs() and leave them there?

Thanks

Yinghai

#5 lore raw

Re: [PATCH v5 02/33] genirq: Add irq_alloc_reserved_desc()

Quoted 32 lines · Yinghai Lu
Quoted 9 lines · Thomas Gleixner

There is a clear step by step approach to get this done proper:

1) Get rid of the existing misconception/misnomer of irq_reserve_irqs().

Make it explicit that this is dealing with legacy irq spaces. It's not that hard as there are only two users in tree which are both trivial to fix.

Hi, Thomas,

While going through the code for kill irq_reserve_irqs(), I found that there is irq_reserve_irq().

in include/linux/irq.h

static inline int irq_reserve_irq(unsigned int irq)

{
        return irq_reserve_irqs(irq, 1);
}

it is called via kernel/irq/chip.c::irq_set_chip().

/*
* For !CONFIG_SPARSE_IRQ make the irq show up in
* allocated_irqs. For the CONFIG_SPARSE_IRQ case, it is
* already marked, and this call is harmless. */
irq_reserve_irq(irq);

Quoted 6 lines

There are tens of irq_set_chip... calling for arches that does not support SPARSE_IRQ yet, and they does not use irq_alloc_desc() anywhere.

so how about change those irq_reserve_irq to irq_set_allocated_irqs() and leave them there?

As I said before irq_reserve_irq() is a misnomer and a misconception. Of course this needs to be fixed as well.

And you cannot just blindly change it because !SPARSE can use the allocation. We are not creating stupid corner cases just to support your sloppyness. Its not rocket science to do it the right way.

That said, it might be worthwhile to get rid of the !SPARSE case completely. That would probably make quite some stuff simpler.

Thanks,

tglx

author
#6 lore raw

Re: [PATCH v5 02/33] genirq: Add irq_alloc_reserved_desc()

Quoted 10 lines · Thomas Gleixner

As I said before irq_reserve_irq() is a misnomer and a misconception. Of course this needs to be fixed as well.

And you cannot just blindly change it because !SPARSE can use the allocation. We are not creating stupid corner cases just to support your sloppyness. Its not rocket science to do it the right way.

That said, it might be worthwhile to get rid of the !SPARSE case completely. That would probably make quite some stuff simpler.

So we need to make all arches support SPARSE_IRQ at first?

Now we have arm, arm64, c6x, metag, powerpc, sh, x86 support SPARSE_IRQ.

The following are not with SPARSE_IRQ yet:
alpha, arc, avr32, blackfin, cris, frv, hexagon, m32r, m68k, microblaze, mips, mn10300, openrisc, parisc, s390, score, sparc, tile, um, unicore32, xtensa.

Thanks

Yinghai

Thanks

Yinghai

author
#7 lore raw

Re: [PATCH v5 02/33] genirq: Add irq_alloc_reserved_desc()

Quoted 12 lines · Yinghai Lu
Quoted 10 lines · Thomas Gleixner

As I said before irq_reserve_irq() is a misnomer and a misconception. Of course this needs to be fixed as well.

And you cannot just blindly change it because !SPARSE can use the allocation. We are not creating stupid corner cases just to support your sloppyness. Its not rocket science to do it the right way.

That said, it might be worthwhile to get rid of the !SPARSE case completely. That would probably make quite some stuff simpler.

Quoted 8 lines

So we need to make all arches support SPARSE_IRQ at first?

Now we have arm, arm64, c6x, metag, powerpc, sh, x86 support SPARSE_IRQ.

The following are not with SPARSE_IRQ yet:
alpha, arc, avr32, blackfin, cris, frv, hexagon, m32r, m68k, microblaze, mips, mn10300, openrisc, parisc, s390, score, sparc, tile, um, unicore32, xtensa.

or add calling irq_alloc_desc_at() before irq_set_chip... for !SPARSE_IRQ.

Please check attached partial patch if you like it.

If you are happy with that, I will split it into pieces and add other irq_alloc_desc_at()
calling.

Thanks

Yinghai

arch/alpha/kernel/irq_alpha.c +1 -0
index 1c8625c..6c8728a 100644
@@ -221,6 +221,7 @@ struct irqaction timer_irqaction = {
 void __init
 init_rtc_irq(void)
 {
+	irq_alloc_desc_at(RTC_IRQ, 0);
 	irq_set_chip_and_handler_name(RTC_IRQ, &dummy_irq_chip,
 				      handle_percpu_irq, "RTC");
 	setup_irq(RTC_IRQ, &timer_irqaction);
arch/alpha/kernel/irq_i8259.c +1 -0
index e1861c7..c522359 100644
@@ -92,6 +92,7 @@ init_i8259a_irqs(void)
 	outb(0xff, 0xA1);	/* mask all of 8259A-2 */
 
 	for (i = 0; i < 16; i++) {
+		irq_alloc_desc_at(i, 0);
 		irq_set_chip_and_handler(i, &i8259a_irq_type, handle_level_irq);
 	}
 
arch/alpha/kernel/irq_pyxis.c +1 -0
index 13c97a5..b6da6d1 100644
@@ -102,6 +102,7 @@ init_pyxis_irqs(unsigned long ignore_mask)
 	for (i = 16; i < 48; ++i) {
 		if ((ignore_mask >> i) & 1)
 			continue;
+		irq_alloc_desc_at(i, 0);
 		irq_set_chip_and_handler(i, &pyxis_irq_type, handle_level_irq);
 		irq_set_status_flags(i, IRQ_LEVEL);
 	}
arch/s390/kernel/irq.c +5 -1
index bb27a26..2d1567b 100644
@@ -88,9 +88,13 @@ static const struct irq_class irqclass_sub_desc[NR_ARCH_IRQS] = {
 	[CPU_RST]    = {.name = "RST", .desc = "[CPU] CPU Restart"},
 };
 
+int arch_probe_early_allocate_nr_irqs(void)
+{
+	return THIN_INTERRUPT;
+}
+
 void __init init_IRQ(void)
 {
-	irq_reserve_irqs(0, THIN_INTERRUPT);
 	init_cio_interrupts();
 	init_airq_interrupts();
 	init_ext_interrupts();
arch/x86/kernel/apic/io_apic.c +0 -3
index 6ad4658..398f9c4 100644
@@ -206,9 +206,6 @@ int __init arch_early_irq_init(void)
 	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++) {
 		irq_set_chip_data(i, &cfg[i]);
 		zalloc_cpumask_var_node(&cfg[i].domain, GFP_KERNEL, node);
drivers/sh/intc/core.c +1 -4
index 8f32a13..05118f92 100644
@@ -81,11 +81,8 @@ static void __init intc_register_irq(struct intc_desc *desc,
 	unsigned long flags;
 
 	/*
-	 * Register the IRQ position with the global IRQ map, then insert
-	 * it in to the radix tree.
+	 * insert it in to the radix tree.
 	 */
-	irq_reserve_irq(irq);
-
 	raw_spin_lock_irqsave(&intc_big_lock, flags);
 	radix_tree_insert(&d->tree, enum_id, intc_irq_xlate_get(irq));
 	raw_spin_unlock_irqrestore(&intc_big_lock, flags);
include/linux/irq.h +1 -6
index 7dc1003..584f0d7 100644
@@ -613,18 +613,13 @@ int __irq_alloc_descs(int irq, unsigned int from, unsigned int cnt, int node,
 	irq_alloc_descs(-1, from, cnt, node)
 
 void irq_free_descs(unsigned int irq, unsigned int cnt);
-int irq_reserve_irqs(unsigned int from, unsigned int cnt);
+int arch_probe_early_allocate_nr_irqs(void);
 
 static inline void irq_free_desc(unsigned int irq)
 {
 	irq_free_descs(irq, 1);
 }
 
-static inline int irq_reserve_irq(unsigned int irq)
-{
-	return irq_reserve_irqs(irq, 1);
-}
-
 #ifndef irq_reg_writel
 # define irq_reg_writel(val, addr)	writel(val, addr)
 #endif
kernel/irq/chip.c +1 -6
index dc04c16..4d86cf5 100644
@@ -38,12 +38,7 @@ int irq_set_chip(unsigned int irq, struct irq_chip *chip)
 
 	desc->irq_data.chip = chip;
 	irq_put_desc_unlock(desc, flags);
-	/*
-	 * For !CONFIG_SPARSE_IRQ make the irq show up in
-	 * allocated_irqs. For the CONFIG_SPARSE_IRQ case, it is
-	 * already marked, and this call is harmless.
-	 */
-	irq_reserve_irq(irq);
+
 	return 0;
 }
 EXPORT_SYMBOL(irq_set_chip);
kernel/irq/irqdesc.c +10 -25
index 8ab8e93..4b2ab96 100644
@@ -248,10 +248,16 @@ struct irq_desc irq_desc[NR_IRQS] __cacheline_aligned_in_smp = {
 	}
 };
 
+int __weak arch_probe_early_allocate_nr_irqs(void)
+{
+        return 0;
+}
+
 int __init early_irq_init(void)
 {
 	int count, i, node = first_online_node;
 	struct irq_desc *desc;
+	int nr = arch_probe_early_allocate_nr_irqs();
 
 	init_irq_default_affinity();
 
@@ -267,6 +273,10 @@ int __init early_irq_init(void)
 		lockdep_set_class(&desc[i].lock, &irq_desc_lock_class);
 		desc_set_defaults(i, &desc[i], node, NULL);
 	}
+
+	for (i = 0; i < nr; i++)
+		set_bit(i, allocated_irqs);
+
 	return arch_early_irq_init();
 }
 
@@ -390,31 +400,6 @@ err:
 EXPORT_SYMBOL_GPL(__irq_alloc_descs);
 
 /**
- * irq_reserve_irqs - mark irqs allocated
- * @from:	mark from irq number
- * @cnt:	number of irqs to mark
- *
- * Returns 0 on success or an appropriate error code
- */
-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;
-}
-
-/**
  * irq_get_next_irq - get next allocated irq number
  * @offset:	where to start the search
  *
#8 lore raw

Re: [PATCH v5 02/33] genirq: Add irq_alloc_reserved_desc()

Quoted 17 lines · Yinghai Lu
Quoted 16 lines · Yinghai Lu
Quoted 10 lines · Thomas Gleixner

As I said before irq_reserve_irq() is a misnomer and a misconception. Of course this needs to be fixed as well.

And you cannot just blindly change it because !SPARSE can use the allocation. We are not creating stupid corner cases just to support your sloppyness. Its not rocket science to do it the right way.

That said, it might be worthwhile to get rid of the !SPARSE case completely. That would probably make quite some stuff simpler.

So we need to make all arches support SPARSE_IRQ at first?

Now we have arm, arm64, c6x, metag, powerpc, sh, x86 support SPARSE_IRQ.

Quoted 8 lines
Quoted 4 lines

The following are not with SPARSE_IRQ yet:
alpha, arc, avr32, blackfin, cris, frv, hexagon, m32r, m68k, microblaze, mips, mn10300, openrisc, parisc, s390, score, sparc, tile, um, unicore32, xtensa.

or add calling irq_alloc_desc_at() before irq_set_chip... for !SPARSE_IRQ.

Please check attached partial patch if you like it.

OMG, you really mean that:

+++ b/arch/alpha/kernel/irq_i8259.c
@@ -92,6 +92,7 @@ init_i8259a_irqs(void)
outb(0xff, 0xA1); /* mask all of 8259A-2 */

for (i = 0; i < 16; i++) {
+ irq_alloc_desc_at(i, 0);
irq_set_chip_and_handler(i, &i8259a_irq_type, handle_level_irq);
}

You can't be serious about that. There are tons of ways to call into the core and access an irq descriptor aside of irq_set_chip* before it is potentially allocated.

Are you going to analyze all of them and add an irq_alloc_desc_at() before that call?

HELL, NO!

I'm really tired of that.

Stay away from kernel/irq/* and wait for people who are competent enough and willing to spend the extra thoughts to come up with solutions which are not completely ass backwards.

Thanks,

tglx

author
#9 lore raw

Re: [PATCH v5 02/33] genirq: Add irq_alloc_reserved_desc()

Quoted 19 lines · Thomas Gleixner

OMG, you really mean that:

+++ b/arch/alpha/kernel/irq_i8259.c

(quoted diff) +1 -0
@@ -92,6 +92,7 @@ init_i8259a_irqs(void)
        outb(0xff, 0xA1);       /* mask all of 8259A-2 */
 
        for (i = 0; i < 16; i++) {
+               irq_alloc_desc_at(i, 0);
                irq_set_chip_and_handler(i, &i8259a_irq_type, handle_level_irq);
        }
 

You can't be serious about that. There are tons of ways to call into the core and access an irq descriptor aside of irq_set_chip* before it is potentially allocated.

Are you going to analyze all of them and add an irq_alloc_desc_at() before that call?

Quoted 7 lines

HELL, NO!

I'm really tired of that.

Stay away from kernel/irq/* and wait for people who are competent enough and willing to spend the extra thoughts to come up with solutions which are not completely ass backwards.

oh, no, sorry for annoying you again.

In the irq_alloc_desc_at() path for !SPARSE_IRQ, it should not touch anything except setting that bit in allocated_irqs bitmap.

Or could just replace irq_reserve_irq in irq_set_chip(), in that way will avoid touching arch codes.

like

kernel/irq/chip.c +7 -7
index dc04c16..02d6ad0 100644
@@ -28,8 +28,13 @@
 int irq_set_chip(unsigned int irq, struct irq_chip *chip)
 {
     unsigned long flags;
-    struct irq_desc *desc = irq_get_desc_lock(irq, &flags, 0);
+    struct irq_desc *desc;
+
+#ifndef CONFIG_SPARSE_IRQ
+    irq_alloc_desc_at(irq, 0);
+#endif
 
+    desc = irq_get_desc_lock(irq, &flags, 0);
     if (!desc)
         return -EINVAL;
 
@@ -38,12 +43,7 @@ int irq_set_chip(unsigned int irq, struct irq_chip *chip)
 
     desc->irq_data.chip = chip;
     irq_put_desc_unlock(desc, flags);
-    /*
-     * For !CONFIG_SPARSE_IRQ make the irq show up in
-     * allocated_irqs. For the CONFIG_SPARSE_IRQ case, it is
-     * already marked, and this call is harmless.
-     */
-    irq_reserve_irq(irq);
+
     return 0;
 }
 EXPORT_SYMBOL(irq_set_chip);
#10 lore raw

Re: [PATCH v5 02/33] genirq: Add irq_alloc_reserved_desc()

In reply to Yinghai Lu
Quoted 53 lines · 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.

(quoted diff) +0 -2
-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.
Cc Joerg Roedel <[email protected]>
Cc Konrad Rzeszutek Wilk <[email protected]>
Cc Sebastian Andrzej Siewior <[email protected]>
---
 include/linux/irq.h  |  3 +++
 kernel/irq/irqdesc.c | 23 +++++++++++++++++++++++
 2 files changed, 26 insertions(+)
include/linux/irq.h +3 -0
index 0229caf..e5f6493 100644
@@ -595,10 +595,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)
kernel/irq/irqdesc.c +0 -0
index a151db6..1166545 100644

kernel/irq/irqdesc.c __irq_alloc_descs(int irq, unsigned int from, unsigned int cnt, int node, · line 414

Quoted 5 lines
kernel/irq/irqdesc.c +2 -0
@@ -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

Hi Yinghai
Should we skip "if irq >= 0" here because irq should have already been
reserved? Or should we add range check for irq?
Thanks!
Gerry

Quoted 25 lines
kernel/irq/irqdesc.c +21 -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
author
#11 lore raw

PATCH v5 3/33 genirq: Do not free unallocated irq descriptors

In reply to 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

---
 kernel/irq/irqdesc.c | 3 +++
 1 file changed, 3 insertions(+)
kernel/irq/irqdesc.c +3 -0
index 1166545..bbf0601 100644
@@ -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);
Signature · 1 line
1.8.4
author
#12 lore raw

PATCH v5 4/33 x86, irq: Change irq_remap_modify_chip_defaults() to static

In reply to Yinghai Lu

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

Cc Joerg Roedel <[email protected]>
Cc Konrad Rzeszutek Wilk <[email protected]>
Cc Sebastian Andrzej Siewior <[email protected]>
---
 arch/x86/include/asm/irq_remapping.h | 6 ------
 drivers/iommu/irq_remapping.c        | 2 +-
 2 files changed, 1 insertion(+), 7 deletions(-)
arch/x86/include/asm/irq_remapping.h +0 -6
index d806b22..606f42e 100644
@@ -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)
drivers/iommu/irq_remapping.c +1 -1
index 39f81ae..85ae2cf 100644
@@ -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;
Signature · 1 line
1.8.4
author
#13 lore raw

PATCH v5 5/33 x86, irq: Modify irq chip once for irq remapping

In reply to 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.

Cc Konrad Rzeszutek Wilk <[email protected]>
Cc Sebastian Andrzej Siewior <[email protected]>
Cc Joerg Roedel <[email protected]>
---
 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(-)
arch/x86/include/asm/io_apic.h +5 -0
index 459e50a..eaff3ad 100644
@@ -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
arch/x86/include/asm/irq_remapping.h +2 -6
index 606f42e..304c93f 100644
@@ -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;
 }
arch/x86/kernel/apic/apic.c +8 -1
index d278736..37447b1 100644
@@ -1572,6 +1572,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;
@@ -1583,7 +1585,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;
 }
arch/x86/kernel/apic/io_apic.c +6 -8
index e63a5bd..6e9f55b 100644
@@ -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;
@@ -2516,7 +2514,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,
@@ -3098,7 +3096,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,
@@ -3128,7 +3126,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");
 
@@ -3245,7 +3243,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,
@@ -3266,7 +3264,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;
drivers/iommu/irq_remapping.c +10 -3
index 85ae2cf..8e4e3af 100644
@@ -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;
 }
Signature · 1 line
1.8.4
author
#14 lore raw

PATCH v5 6/33 x86, irq: Show MSI-X clearly in debug message

In reply to Yinghai Lu

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

Cc Joerg Roedel <[email protected]>
Cc Konrad Rzeszutek Wilk <[email protected]>
Cc Sebastian Andrzej Siewior <[email protected]>
---
 arch/x86/kernel/apic/io_apic.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)
arch/x86/kernel/apic/io_apic.c +2 -1
index 6e9f55b..8616517 100644
@@ -3130,7 +3130,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;
 }
Signature · 1 line
1.8.4
author
#15 lore raw

PATCH v5 7/33 x86, irq: Show MSI-X in /proc/interrupt

In reply to 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().

Cc Joerg Roedel <[email protected]>
Cc Konrad Rzeszutek Wilk <[email protected]>
Cc Sebastian Andrzej Siewior <[email protected]>
---
 arch/x86/kernel/apic/io_apic.c | 7 +++++++
 drivers/iommu/irq_remapping.c  | 5 ++++-
 2 files changed, 11 insertions(+), 1 deletion(-)
arch/x86/kernel/apic/io_apic.c +7 -0
index 8616517..c52085c 100644
@@ -3092,6 +3092,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.
@@ -3103,6 +3109,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,
drivers/iommu/irq_remapping.c +4 -1
index 8e4e3af..6893d75 100644
@@ -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)
Signature · 1 line
1.8.4
author
#16 lore raw

PATCH v5 8/33 x86, irq: Make dmar_msi/hpet_msi irq_chip name consistent

In reply to Yinghai Lu

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

Change dmar_msi and hpet_msi to use "-".

Cc Joerg Roedel <[email protected]>
Cc Konrad Rzeszutek Wilk <[email protected]>
Cc Sebastian Andrzej Siewior <[email protected]>
---
 arch/x86/kernel/apic/io_apic.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
arch/x86/kernel/apic/io_apic.c +2 -2
index c52085c..241325c 100644
@@ -3204,7 +3204,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,
@@ -3252,7 +3252,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,
Signature · 1 line
1.8.4
author
#17 lore raw

PATCH v5 9/33 ia64, irq: Add dummy create_irq_nr()

In reply to 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.

---
 arch/ia64/kernel/irq_ia64.c | 10 ++++++++++
 1 file changed, 10 insertions(+)
arch/ia64/kernel/irq_ia64.c +10 -0
index 1034884..38e46df 100644
@@ -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);
Signature · 1 line
1.8.4
author
#18 lore raw

PATCH v5 10/33 iommu, irq: Allocate irq_desc for dmar_msi with local node

In reply to 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.

Cc Joerg Roedel <[email protected]>
Cc Donald Dutile <[email protected]>
Cc Sebastian Andrzej Siewior <[email protected]>
---
 drivers/iommu/dmar.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/iommu/dmar.c +1 -1
index 8b452c9..f4eaa50 100644
@@ -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;
Signature · 1 line
1.8.4
#19 lore raw

Re: [PATCH v5 10/33] iommu, irq: Allocate irq_desc for dmar_msi with local node

Quoted 24 lines · 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.

Cc Joerg Roedel <[email protected]>
Cc Donald Dutile <[email protected]>
Cc Sebastian Andrzej Siewior <[email protected]>
---
 drivers/iommu/dmar.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/iommu/dmar.c +0 -0
index 8b452c9..f4eaa50 100644

drivers/iommu/dmar.c int dmar_set_interrupt(struct intel_iommu *iommu) · line 1280

Quoted 5 lines
drivers/iommu/dmar.c +1 -1
@@ -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);

Hi Yinghai,

	How about create_irq_nrq(1, iommu->node) here to clearly state

that 0 is not a valid irq number for dmar here?
Thanks!
Gerry

Quoted 4 lines
drivers/iommu/dmar.c +0 -0
 	if (!irq) {
 		pr_err("IOMMU: no free vectors\n");
 		return -EINVAL;
author
#20 lore raw

Re: [PATCH v5 10/33] iommu, irq: Allocate irq_desc for dmar_msi with local node

Quoted 30 lines · Jiang Liu
Quoted 27 lines · 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.

Cc Joerg Roedel <[email protected]>
Cc Donald Dutile <[email protected]>
Cc Sebastian Andrzej Siewior <[email protected]>
---
 drivers/iommu/dmar.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/iommu/dmar.c +0 -0
index 8b452c9..f4eaa50 100644
@@ -1277,7 +1277,7 @@ int dmar_set_interrupt(struct intel_iommu *iommu)
      if (iommu->irq)
              return 0;
 
Quoted 5 lines

drivers/iommu/dmar.c int dmar_set_interrupt(struct intel_iommu *iommu) · line 1280

Quoted 2 lines
drivers/iommu/dmar.c +1 -1
-     irq = create_irq();
+     irq = create_irq_nr(0, iommu->node);

Hi Yinghai,

        How about create_irq_nrq(1, iommu->node) here to clearly state

that 0 is not a valid irq number for dmar here?

Should be the same. that 0 or 1 will be from in __create_irqs().

And we have
if (from < nr_irqs_gsi)
from = nr_irqs_gsi;
in __create_irqs().

Thanks

Yinghai

#21 lore raw

Re: [PATCH v5 10/33] iommu, irq: Allocate irq_desc for dmar_msi with local node

Quoted 37 lines · Yinghai Lu
Quoted 30 lines · Jiang Liu
Quoted 27 lines · 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.

Cc Joerg Roedel <[email protected]>
Cc Donald Dutile <[email protected]>
Cc Sebastian Andrzej Siewior <[email protected]>
---
 drivers/iommu/dmar.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/iommu/dmar.c +0 -0
index 8b452c9..f4eaa50 100644
@@ -1277,7 +1277,7 @@ int dmar_set_interrupt(struct intel_iommu *iommu)
      if (iommu->irq)
              return 0;
 
Quoted 5 lines

drivers/iommu/dmar.c int dmar_set_interrupt(struct intel_iommu *iommu) · line 1280

Quoted 2 lines
drivers/iommu/dmar.c +1 -1
-     irq = create_irq();
+     irq = create_irq_nr(0, iommu->node);

Hi Yinghai,

        How about create_irq_nrq(1, iommu->node) here to clearly state

that 0 is not a valid irq number for dmar here?

Quoted 6 lines

Should be the same. that 0 or 1 will be from in __create_irqs().

And we have
if (from < nr_irqs_gsi)
from = nr_irqs_gsi;
in __create_irqs().

Hi Yinghai,
Seems there is no such protection on IA64 side. I feel create_irq_nr()
returns 0 for error is a little risky, 0 may be
a valid IRQ number on other platforms(no sure about this). Thanks!

Quoted 5 lines

Thanks

Yinghai

author
#22 lore raw

Re: [PATCH v5 10/33] iommu, irq: Allocate irq_desc for dmar_msi with local node

Quoted 4 lines · Jiang Liu

Seems there is no such protection on IA64 side. I feel create_irq_nr()
returns 0 for error is a little risky, 0 may be
a valid IRQ number on other platforms(no sure about this). Thanks!

in [PATCH] ia64, irq: Add dummy create_irq_nr()

+unsigned int create_irq_nr(unsigned int from, int node) +{
+ int irq = create_irq();
+
+ if (irq < 0)
+ irq = 0;
+
+ return irq;
+}
+

so that from is ignored.

create_irq_nr() is only with x86 and ia64.

Thanks

Yinghai

#23 lore raw

Re: [PATCH v5 10/33] iommu, irq: Allocate irq_desc for dmar_msi with local node

Hi Yinghai,
I have a plan to test this patch set once I have reviewed
all patches in coming weeks. Do you have a plan to update to latest kernel or is it ok to test this version?
Thanks!
Gerry

Quoted 28 lines · Yinghai Lu
Quoted 4 lines · Jiang Liu

Seems there is no such protection on IA64 side. I feel create_irq_nr()
returns 0 for error is a little risky, 0 may be
a valid IRQ number on other platforms(no sure about this). Thanks!

in [PATCH] ia64, irq: Add dummy create_irq_nr()

(quoted diff) +10 -0
+unsigned int create_irq_nr(unsigned int from, int node)
+{
+       int irq = create_irq();
+
+       if (irq < 0)
+               irq = 0;
+
+       return irq;
+}
+

so that from is ignored.

create_irq_nr() is only with x86 and ia64.

Thanks

Yinghai

author
#24 lore raw

Re: [PATCH v5 10/33] iommu, irq: Allocate irq_desc for dmar_msi with local node

Quoted 4 lines · Jiang Liu

Hi Yinghai,
I have a plan to test this patch set once I have reviewed
all patches in coming weeks. Do you have a plan to update to latest kernel or is it ok to test this version?

That will be great. I will refresh the branch on top of linus/tip/pci/acpi next week.

Thanks

Yinghai

author
#25 lore raw

PATCH v5 11/33 x86, irq: Kill create_irq()

In reply to 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.

Cc Joerg Roedel <[email protected]>
Cc Konrad Rzeszutek Wilk <[email protected]>
Cc Sebastian Andrzej Siewior <[email protected]>
---
 arch/x86/kernel/apic/io_apic.c | 15 ---------------
 1 file changed, 15 deletions(-)
arch/x86/kernel/apic/io_apic.c +0 -15
index 241325c..86d5466 100644
@@ -2973,21 +2973,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);
Signature · 1 line
1.8.4
author
#26 lore raw

PATCH v5 12/33 x86, irq: Convert irq_2_pin list to generic list

In reply to 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.

Cc Konrad Rzeszutek Wilk <[email protected]>
Cc Sebastian Andrzej Siewior <[email protected]>
Cc Joerg Roedel <[email protected]>
---
 arch/x86/include/asm/hw_irq.h  |  2 +-
 arch/x86/kernel/apic/io_apic.c | 22 ++++++++++++----------
 2 files changed, 13 insertions(+), 11 deletions(-)
arch/x86/include/asm/hw_irq.h +1 -1
index cba45d9..eb0907e 100644
@@ -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;
arch/x86/kernel/apic/io_apic.c +12 -10
index 86d5466..b943012 100644
@@ -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;
 }
 
@@ -1627,8 +1630,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)
Signature · 1 line
1.8.4
author
#27 lore raw

PATCH v5 13/33 x86, irq: Add alloc_reserved_irq_and_cfg_at()

In reply to 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().

Cc Joerg Roedel <[email protected]>
Cc Konrad Rzeszutek Wilk <[email protected]>
Cc Sebastian Andrzej Siewior <[email protected]>
---
 arch/x86/kernel/apic/io_apic.c | 25 ++++++++++++++++++++++++-
 1 file changed, 24 insertions(+), 1 deletion(-)
arch/x86/kernel/apic/io_apic.c +24 -1
index b943012..4b836ea 100644
@@ -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;
@@ -3357,7 +3380,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)
Signature · 1 line
1.8.4
author
#28 lore raw

PATCH v5 14/33 x86, irq: Move down arch_early_irq_init()

In reply to Yinghai Lu

Change position only.

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

Cc Joerg Roedel <[email protected]>
Cc Konrad Rzeszutek Wilk <[email protected]>
Cc Sebastian Andrzej Siewior <[email protected]>
---
 arch/x86/kernel/apic/io_apic.c | 89 +++++++++++++++++++++---------------------
 1 file changed, 44 insertions(+), 45 deletions(-)
arch/x86/kernel/apic/io_apic.c +44 -45
index 4b836ea..5822c37 100644
@@ -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];
Signature · 1 line
1.8.4
author
#29 lore raw

PATCH v5 15/33 x86, irq: Split out alloc_ioapic_save_registers()

In reply to 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.

Cc Joerg Roedel <[email protected]>
Cc Konrad Rzeszutek Wilk <[email protected]>
Cc Sebastian Andrzej Siewior <[email protected]>
---
 arch/x86/kernel/apic/io_apic.c | 22 +++++++++++++++-------
 1 file changed, 15 insertions(+), 7 deletions(-)
arch/x86/kernel/apic/io_apic.c +15 -7
index 5822c37..c94dd19 100644
@@ -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);
Signature · 1 line
1.8.4
author
#30 lore raw

PATCH v5 16/33 xen, irq: Call irq_alloc_reserved_desc_at() at first

In reply to 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.

---
 drivers/xen/events.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)
drivers/xen/events.c +6 -2
index 4035e83..020cd77 100644
@@ -508,8 +508,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);
 
Signature · 1 line
1.8.4
#31 lore raw

Re: [Xen-devel] [PATCH v5 16/33] xen, irq: Call irq_alloc_reserved_desc_at() at first

Quoted 18 lines · 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.

---
 drivers/xen/events.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)
drivers/xen/events.c +0 -0
index 4035e83..020cd77 100644
@@ -508,8 +508,12 @@ static int __must_check xen_allocate_irq_gsi(unsigned gsi)
 	/* Legacy IRQ descriptors are already allocated by the arch. */

drivers/xen/events.c static int __must_check xen_allocate_irq_gsi(unsigned gsi) · line 516

Quoted 10 lines
drivers/xen/events.c +6 -2
 	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);
+	}

This is common code. On ARM I get:

drivers/xen/events.c: In function 'xen_allocate_irq_gsi':
drivers/xen/events.c:513:3: error: implicit declaration of function 'irq_alloc_reserved_desc_at' [-Werror=implicit-function-declaration]
irq = irq_alloc_reserved_desc_at(gsi, -1);
^
cc1: some warnings being treated as errors

author
#32 lore raw

Re: [Xen-devel] [PATCH v5 16/33] xen, irq: Call irq_alloc_reserved_desc_at() at first

Quoted 22 lines · Stefano Stabellini
Quoted 9 lines
 drivers/xen/events.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)
drivers/xen/events.c +0 -0
index 4035e83..020cd77 100644
@@ -508,8 +508,12 @@ static int __must_check xen_allocate_irq_gsi(unsigned gsi)
      /* Legacy IRQ descriptors are already allocated by the arch. */

drivers/xen/events.c static int __must_check xen_allocate_irq_gsi(unsigned gsi) · line 516

Quoted 10 lines
drivers/xen/events.c +6 -2
      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);
+     }

This is common code. On ARM I get:

Quoted 5 lines

drivers/xen/events.c: In function 'xen_allocate_irq_gsi':
drivers/xen/events.c:513:3: error: implicit declaration of function 'irq_alloc_reserved_desc_at' [-Werror=implicit-function-declaration]

   irq = irq_alloc_reserved_desc_at(gsi, -1);
   ^

cc1: some warnings being treated as errors

It's strange.

that is defined with irq_alloc_desc_at in parallel in include/linux/irq.h and kernel/irq/irqdesc.c.

Did you try the whole tree, or just this patch?

Thanks

Yinghai

#33 lore raw

Re: [Xen-devel] [PATCH v5 16/33] xen, irq: Call irq_alloc_reserved_desc_at() at first

Quoted 31 lines · Yinghai Lu
Quoted 22 lines · Stefano Stabellini
Quoted 9 lines
 drivers/xen/events.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)
drivers/xen/events.c +0 -0
index 4035e83..020cd77 100644
@@ -508,8 +508,12 @@ static int __must_check xen_allocate_irq_gsi(unsigned gsi)
      /* Legacy IRQ descriptors are already allocated by the arch. */

drivers/xen/events.c static int __must_check xen_allocate_irq_gsi(unsigned gsi) · line 516

Quoted 10 lines
drivers/xen/events.c +6 -2
      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);
+     }

This is common code. On ARM I get:

Quoted 5 lines

drivers/xen/events.c: In function 'xen_allocate_irq_gsi':
drivers/xen/events.c:513:3: error: implicit declaration of function 'irq_alloc_reserved_desc_at' [-Werror=implicit-function-declaration]

   irq = irq_alloc_reserved_desc_at(gsi, -1);
   ^

cc1: some warnings being treated as errors

Quoted 6 lines

It's strange.

that is defined with irq_alloc_desc_at in parallel in include/linux/irq.h and kernel/irq/irqdesc.c.

Did you try the whole tree, or just this patch?

Just this patch.
The whole tree (yinghai/for-x86-irq-3.14) builds just fine. Thanks!

- Stefano

author
#34 lore raw

PATCH v5 17/33 x86, irq: Reserve irq range and alloc_reserved for booting path

In reply to 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.

Cc Joerg Roedel <[email protected]>
Cc Konrad Rzeszutek Wilk <[email protected]>
Cc Sebastian Andrzej Siewior <[email protected]>
---
 arch/x86/include/asm/io_apic.h |  1 +
 arch/x86/kernel/apic/io_apic.c | 80 +++++++++++++++++++++++++++++++-----------
 2 files changed, 60 insertions(+), 21 deletions(-)
arch/x86/include/asm/io_apic.h +1 -0
index eaff3ad..8181fd8 100644
@@ -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;
arch/x86/kernel/apic/io_apic.c +59 -21

108 lines not on this page · raw message

Signature · 1 line
1.8.4
author
#35 lore raw

PATCH v5 18/33 x86, irq: Add ioapic_gsi_to_irq

In reply to 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.

Cc Pavel Machek <[email protected]>
Cc Joerg Roedel <[email protected]>
Cc Konrad Rzeszutek Wilk <[email protected]>
Cc Sebastian Andrzej Siewior <[email protected]>
---
 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(-)
arch/x86/include/asm/io_apic.h +1 -0
index 8181fd8..02ac411 100644
@@ -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);
arch/x86/kernel/acpi/boot.c +5 -17
index 6c0b43b..0f9c133 100644
@@ -113,6 +113,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.
@@ -125,22 +129,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;
-}
-
 /*
  * This is just a simple wrapper around early_ioremap(),
  * with sanity checks for phys == 0 and size == 0.
@@ -530,7 +518,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;
 }
 
arch/x86/kernel/apic/io_apic.c +28 -1
index a1af837..066611f 100644
@@ -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()
Signature · 1 line
1.8.4
author
#36 lore raw

PATCH v5 19/33 x86, irq: Add for_each_ioapic helper

In reply to 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.

Cc Joerg Roedel <[email protected]>
Cc Konrad Rzeszutek Wilk <[email protected]>
Cc Sebastian Andrzej Siewior <[email protected]>
Cc Grant Likely <[email protected]>
Cc Paul Gortmaker <[email protected]>
Cc Andy Lutomirski <[email protected]>
---
 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(-)
arch/x86/include/asm/io_apic.h +6 -0
index 02ac411..a25dcf1 100644
@@ -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);
arch/x86/kernel/apic/io_apic.c +34 -30

217 lines not on this page · raw message

arch/x86/kernel/devicetree.c +1 -1
index d35078e..b353b1d 100644
@@ -361,7 +361,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;
drivers/iommu/amd_iommu_init.c +1 -1
index 8f798be..f3052cd 100644
@@ -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);
drivers/iommu/intel_irq_remapping.c +1 -1
index bab10b1..07ab6e8 100644
@@ -794,7 +794,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, "
Signature · 1 line
1.8.4
author
#37 lore raw

PATCH v5 20/33 x86, irq: More strict checking about registering ioapic

In reply to 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.

Cc Joerg Roedel <[email protected]>
Cc Konrad Rzeszutek Wilk <[email protected]>
Cc Sebastian Andrzej Siewior <[email protected]>
---
 arch/x86/kernel/apic/io_apic.c | 30 ++++++++++++++++++++++++------
 1 file changed, 24 insertions(+), 6 deletions(-)
arch/x86/kernel/apic/io_apic.c +24 -6
index 5dd3886..d43894e 100644
@@ -3826,12 +3826,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) {
@@ -3841,10 +3838,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;
@@ -3896,6 +3902,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",
@@ -3922,6 +3933,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;
Signature · 1 line
1.8.4
author
#38 lore raw

PATCH v5 21/33 x86, irq: Make mp_register_ioapic handle hot-added ioapic

In reply to 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.

Cc Paul Gortmaker <[email protected]>
Cc Joerg Roedel <[email protected]>
Cc Konrad Rzeszutek Wilk <[email protected]>
Cc Sebastian Andrzej Siewior <[email protected]>
---
 arch/x86/include/asm/mpspec.h  |  1 +
 arch/x86/kernel/apic/io_apic.c | 74 ++++++++++++++++++++++++++++++++----------
 2 files changed, 57 insertions(+), 18 deletions(-)
arch/x86/include/asm/mpspec.h +1 -0
index 3142a94..59d8465 100644
@@ -96,6 +96,7 @@ static inline void early_reserve_e820_mpc_new(void) { }
 
 int 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);
arch/x86/kernel/apic/io_apic.c +56 -18

143 lines not on this page · raw message

Signature · 1 line
1.8.4
author
#39 lore raw

PATCH v5 22/33 x86, irq: Add mp_unregister_ioapic to handle hot-remove ioapic

In reply to Yinghai Lu

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

Cc Paul Gortmaker <[email protected]>
Cc Jiang Liu <[email protected]>
Cc Joerg Roedel <[email protected]>
Cc Konrad Rzeszutek Wilk <[email protected]>
Cc Sebastian Andrzej Siewior <[email protected]>
---
 arch/x86/include/asm/mpspec.h  |  1 +
 arch/x86/kernel/apic/io_apic.c | 45 ++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 46 insertions(+)
arch/x86/include/asm/mpspec.h +1 -0
index 59d8465..b98665f 100644
@@ -97,6 +97,7 @@ static inline void early_reserve_e820_mpc_new(void) { }
 int 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);
arch/x86/kernel/apic/io_apic.c +45 -0

63 lines not on this page · raw message

Signature · 1 line
1.8.4
author
#40 lore raw

PATCH v5 23/33 x86, ioapic: Find usable ioapic id for 64bit.

In reply to Yinghai Lu

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

Cc Joerg Roedel <[email protected]>
Cc Konrad Rzeszutek Wilk <[email protected]>
Cc Sebastian Andrzej Siewior <[email protected]>
---
 arch/x86/kernel/apic/io_apic.c | 38 +++++++++++++++++++++++++++++++++-----
 1 file changed, 33 insertions(+), 5 deletions(-)
arch/x86/kernel/apic/io_apic.c +33 -5
index af6cbcd..248a21d 100644
@@ -3660,19 +3660,22 @@ static int io_apic_get_unique_id(int ioapic, int apic_id)
 	return apic_id;
 }
 
-static u8 io_apic_unique_id(u8 id)
+static u8 io_apic_unique_id(int idx, u8 id)
 {
 	if ((boot_cpu_data.x86_vendor == X86_VENDOR_INTEL) &&
 	    !APIC_XAPIC(apic_version[boot_cpu_physical_apicid]))
-		return io_apic_get_unique_id(nr_ioapics, id);
+		return io_apic_get_unique_id(idx, id);
 	else
 		return id;
 }
 #else
-static u8 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)
@@ -3680,7 +3683,32 @@ static u8 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
 
@@ -3955,7 +3983,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);
 
 	/*
Signature · 1 line
1.8.4
author
#41 lore raw

PATCH v5 24/33 x86: Move declaration for mp_register_ioapic()

In reply to Yinghai Lu

Address compiling problem that Fengguang report.

---
 arch/x86/include/asm/mpspec.h | 18 +++++++++++++++++-
 1 file changed, 17 insertions(+), 1 deletion(-)
arch/x86/include/asm/mpspec.h +17 -1
index b98665f..e9d850e 100644
@@ -95,10 +95,26 @@ static inline void early_reserve_e820_mpc_new(void) { }
 #endif
 
 int 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);
Signature · 1 line
1.8.4
maintainer
#42 lore raw

Re: [PATCH v5 24/33] x86: Move declaration for mp_register_ioapic()

Quoted 1 line · Yinghai Lu

Address compiling problem that Fengguang report.

Does this fix a problem that's already in the upstream tree?

If it merely fixes a problem that was added somewhere in your patches, why don't you fold this fix into the patch that added the problem?

If you included more details, like the exact problem report, I could probably have figured this out myself.

Bjorn

Quoted 43 lines
---
 arch/x86/include/asm/mpspec.h | 18 +++++++++++++++++-
 1 file changed, 17 insertions(+), 1 deletion(-)
arch/x86/include/asm/mpspec.h +17 -1
index b98665f..e9d850e 100644
@@ -95,10 +95,26 @@ static inline void early_reserve_e820_mpc_new(void) { }
 #endif
 
 int 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);
Signature · 1 line
1.8.4
author
#43 lore raw

Re: [PATCH v5 24/33] x86: Move declaration for mp_register_ioapic()

Quoted 10 lines · Bjorn Helgaas
Quoted 1 line · Yinghai Lu

Address compiling problem that Fengguang report.

Does this fix a problem that's already in the upstream tree?

If it merely fixes a problem that was added somewhere in your patches, why don't you fold this fix into the patch that added the problem?

If you included more details, like the exact problem report, I could probably have figured this out myself.

please check updated changelog:

---
Subject: [PATCH] x86: Move declaration for mp_register_ioapic()

Preparing patch for ioapic hotplug:
PCI, x86, ACPI: Link acpi ioapic register to ioapic
as it adds calling in

        arch/x86/kernel/acpi/boot.c::acpi_register_ioapic()
                                     acpi_unregister_ioapic()

Address compiling problem when
CONFIG_ACPI is defined
CONFIG_X86_IOAPIC is not defined.

Change condition to X86_IO_APIC instead of ACPI.

-v2: update changelog requested by Bjorn.

-----

Subject: [PATCH] x86: Move declaration for mp_register_ioapic()

Preparing patch for ioapic hotplug:
PCI, x86, ACPI: Link acpi ioapic register to ioapic
as it adds calling in

	arch/x86/kernel/acpi/boot.c::acpi_register_ioapic()
				     acpi_unregister_ioapic()

Address compiling problem when
CONFIG_ACPI is defined
CONFIG_X86_IOAPIC is not defined.

Change condition to X86_IO_APIC instead of ACPI.

-v2: update changelog requested by Bjorn.

---
 arch/x86/include/asm/io_apic.h |    1 -
 arch/x86/include/asm/mpspec.h  |   18 +++++++++++++++++-
 2 files changed, 17 insertions(+), 2 deletions(-)
arch/x86/include/asm/mpspec.h +17 -1
Index: linux-2.6/arch/x86/include/asm/mpspec.h
===================================================================
@@ -95,10 +95,26 @@ static inline void early_reserve_e820_mp
 #endif
 
 int 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);
arch/x86/include/asm/io_apic.h +0 -1
Index: linux-2.6/arch/x86/include/asm/io_apic.h
===================================================================
@@ -189,7 +189,6 @@ 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);
 extern void __init pre_init_apic_IRQ0(void);
 
 extern void mp_save_irq(struct mpc_intsrc *m);
author
#44 lore raw

PATCH v5 25/33 PCI, x86: Make ioapic hotplug support built-in

In reply to Yinghai Lu

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

Also need to make it depends on X86_IO_APIC.

---
 drivers/pci/Kconfig  | 3 ++-
 drivers/pci/ioapic.c | 6 +++++-
 2 files changed, 7 insertions(+), 2 deletions(-)
drivers/pci/Kconfig +2 -1
index b6a99f7..893503f 100644
@@ -105,9 +105,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
drivers/pci/ioapic.c +5 -1
index 50ce680..2c2930e 100644
@@ -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");
Signature · 1 line
1.8.4
maintainer
#45 lore raw

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

[fixed Rafael's email address; I imagine you got a few bounces :)]

Quoted 5 lines · Yinghai Lu

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

Also need to make it depends on X86_IO_APIC.

Most of this series is x86-specific and outside the scope of PCI, so I assume somebody else will deal with it.

This particular patch doesn't seem to depend on anything else, so I'll merge it.

Quoted 40 lines
---
 drivers/pci/Kconfig  | 3 ++-
 drivers/pci/ioapic.c | 6 +++++-
 2 files changed, 7 insertions(+), 2 deletions(-)
drivers/pci/Kconfig +2 -1
index b6a99f7..893503f 100644
@@ -105,9 +105,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
drivers/pci/ioapic.c +5 -1
index 50ce680..2c2930e 100644
@@ -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");
Signature · 1 line
1.8.4
author
#46 lore raw

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

Quoted 11 lines · Bjorn Helgaas

[fixed Rafael's email address; I imagine you got a few bounces :)]

Quoted 5 lines · Yinghai Lu

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

Also need to make it depends on X86_IO_APIC.

Most of this series is x86-specific and outside the scope of PCI, so I assume somebody else will deal with it.

Yes.

Quoted 3 lines

This particular patch doesn't seem to depend on anything else, so I'll merge it.

yes.

author
#47 lore raw

PATCH v5 26/33 ACPI: Make map_mat_entry handle x2apic entry

In reply to Yinghai Lu

Now we have map_madt_entry/MADT to handle x2apic, but does not do that with map_mat_entry _MAT.

For hotplug on system with x2apic only support, we need to add support for x2apic map in _MAT.

---
 drivers/acpi/processor_core.c | 2 ++
 1 file changed, 2 insertions(+)
drivers/acpi/processor_core.c +2 -0
index b3171f3..559e762 100644
@@ -153,6 +153,8 @@ static int map_mat_entry(acpi_handle handle, int type, u32 acpi_id)
 	header = (struct acpi_subtable_header *)obj->buffer.pointer;
 	if (header->type == ACPI_MADT_TYPE_LOCAL_APIC) {
 		map_lapic_id(header, acpi_id, &apic_id);
+	} else if (header->type == ACPI_MADT_TYPE_LOCAL_X2APIC) {
+		map_x2apic_id(header, type, acpi_id, &apic_id);
 	} else if (header->type == ACPI_MADT_TYPE_LOCAL_SAPIC) {
 		map_lsapic_id(header, type, acpi_id, &apic_id);
 	}
Signature · 1 line
1.8.4
author
#48 lore raw

PATCH v5 27/33 ACPI: Move acpi_get_cpuid() to separated file

In reply to Yinghai Lu

Will need to reuse searching MADT for ioapic, but should add related function like acpi_get_ioapic_id to processor_core.c to pollute the file.

Move related cpu apic id search to separated file.

---
 drivers/acpi/Makefile         |   1 +
 drivers/acpi/apic_id.c        | 201 ++++++++++++++++++++++++++++++++++++++++++
 drivers/acpi/processor_core.c | 193 ----------------------------------------
 include/acpi/processor.h      |   3 -
 include/linux/acpi.h          |   4 +
 5 files changed, 206 insertions(+), 196 deletions(-)
 create mode 100644 drivers/acpi/apic_id.c
drivers/acpi/Makefile +1 -0
index 0331f91..d922feb 100644
@@ -34,6 +34,7 @@ acpi-$(CONFIG_ACPI_SLEEP)	+= proc.o
 acpi-y				+= bus.o glue.o
 acpi-y				+= scan.o
 acpi-y				+= resource.o
+acpi-y				+= apic_id.o
 acpi-y				+= acpi_processor.o
 acpi-y				+= processor_core.o
 acpi-y				+= ec.o
drivers/acpi/apic_id.c new file +201 -0

201 lines not on this page · raw message

drivers/acpi/processor_core.c +0 -193

199 lines not on this page · raw message

include/acpi/processor.h +0 -3
index 6eb1d3c..337e731 100644
@@ -314,9 +314,6 @@ static inline int acpi_processor_get_bios_limit(int cpu, unsigned int *limit)
 
 /* in processor_core.c */
 void acpi_processor_set_pdc(acpi_handle handle);
-int acpi_get_apicid(acpi_handle, int type, u32 acpi_id);
-int acpi_map_cpuid(int apic_id, u32 acpi_id);
-int acpi_get_cpuid(acpi_handle, int type, u32 acpi_id);
 
 /* in processor_throttling.c */
 int acpi_processor_tstate_has_changed(struct acpi_processor *pr);
include/linux/acpi.h +4 -0
index d9099b1..2f73b81 100644
@@ -134,6 +134,10 @@ int acpi_map_lsapic(acpi_handle handle, int physid, int *pcpu);
 int acpi_unmap_lsapic(int cpu);
 #endif /* CONFIG_ACPI_HOTPLUG_CPU */
 
+int acpi_get_apicid(acpi_handle, int type, u32 acpi_id);
+int acpi_map_cpuid(int apic_id, u32 acpi_id);
+int acpi_get_cpuid(acpi_handle, int type, u32 acpi_id);
+
 int acpi_register_ioapic(acpi_handle handle, u64 phys_addr, u32 gsi_base);
 int acpi_unregister_ioapic(acpi_handle handle, u32 gsi_base);
 void acpi_irq_stats_init(void);
Signature · 1 line
1.8.4
author
#49 lore raw

PATCH v5 28/33 ACPI, ioapic: Add acpi_get_ioapic_id()

In reply to Yinghai Lu

For ioapic hotplug, we need to pass apic id for ioapic registration.

We could check _MAT and MADT to get io apic id, just like cpu hotplug path to get apic id.

---
 drivers/acpi/apic_id.c | 97 ++++++++++++++++++++++++++++++++++++++++++++++++--
 include/linux/acpi.h   |  9 +++++
 2 files changed, 104 insertions(+), 2 deletions(-)
drivers/acpi/apic_id.c +95 -2
index bf4e6de..724444e 100644
@@ -1,5 +1,7 @@
 /*
  * acpi_get_cpuid
+ * acpi_get_ioapic_id for ioapic hotplug support
+ *
  */
 #include <linux/export.h>
 #include <linux/slab.h>
@@ -59,11 +61,11 @@ static int map_lsapic_id(struct acpi_subtable_header *entry,
 	return 1;
 }
 
+static struct acpi_table_madt *madt;
+static int read_madt;
 static int map_madt_entry(int type, u32 acpi_id)
 {
 	unsigned long madt_end, entry;
-	static struct acpi_table_madt *madt;
-	static int read_madt;
 	int apic_id = -1;
 
 	if (!read_madt) {
@@ -199,3 +201,94 @@ int acpi_get_cpuid(acpi_handle handle, int type, u32 acpi_id)
 	return acpi_map_cpuid(apic_id, acpi_id);
 }
 EXPORT_SYMBOL_GPL(acpi_get_cpuid);
+
+#ifdef CONFIG_PCI_IOAPIC
+static int map_ioapic_id(struct acpi_subtable_header *entry, u32 gsi_base,
+		  u64 *phys_addr, int *ioapic_id)
+{
+	struct acpi_madt_io_apic *ioapic =
+		(struct acpi_madt_io_apic *)entry;
+
+	if (ioapic->global_irq_base != gsi_base)
+		return 0;
+
+	*phys_addr = ioapic->address;
+	*ioapic_id = ioapic->id;
+	return 1;
+}
+
+static int map_madt_ioapic_entry(u32 gsi_base, u64 *phys_addr)
+{
+	unsigned long madt_end, entry;
+	int apic_id = -1;
+
+	if (!read_madt) {
+		if (ACPI_FAILURE(acpi_get_table(ACPI_SIG_MADT, 0,
+					(struct acpi_table_header **)&madt)))
+			madt = NULL;
+		read_madt++;
+	}
+
+	if (!madt)
+		return apic_id;
+
+	entry = (unsigned long)madt;
+	madt_end = entry + madt->header.length;
+
+	/* Parse all entries looking for a match. */
+
+	entry += sizeof(struct acpi_table_madt);
+	while (entry + sizeof(struct acpi_subtable_header) < madt_end) {
+		struct acpi_subtable_header *header =
+			(struct acpi_subtable_header *)entry;
+		if (header->type == ACPI_MADT_TYPE_IO_APIC) {
+			if (map_ioapic_id(header, gsi_base, phys_addr,
+						 &apic_id))
+				break;
+		}
+		entry += header->length;
+	}
+	return apic_id;
+}
+
+static int map_mat_ioapic_entry(acpi_handle handle, u32 gsi_base,
+				u64 *phys_addr)
+{
+	struct acpi_buffer buffer = { ACPI_ALLOCATE_BUFFER, NULL };
+	union acpi_object *obj;
+	struct acpi_subtable_header *header;
+	int apic_id = -1;
+
+	if (ACPI_FAILURE(acpi_evaluate_object(handle, "_MAT", NULL, &buffer)))
+		goto exit;
+
+	if (!buffer.length || !buffer.pointer)
+		goto exit;
+
+	obj = buffer.pointer;
+	if (obj->type != ACPI_TYPE_BUFFER ||
+	    obj->buffer.length < sizeof(struct acpi_subtable_header)) {
+		goto exit;
+	}
+
+	header = (struct acpi_subtable_header *)obj->buffer.pointer;
+	if (header->type == ACPI_MADT_TYPE_IO_APIC)
+		map_ioapic_id(header, gsi_base, phys_addr, &apic_id);
+
+exit:
+	kfree(buffer.pointer);
+	return apic_id;
+}
+
+int acpi_get_ioapic_id(acpi_handle handle, u32 gsi_base, u64 *phys_addr)
+{
+	int apic_id = -1;
+
+	apic_id = map_mat_ioapic_entry(handle, gsi_base, phys_addr);
+	if (apic_id == -1)
+		apic_id = map_madt_ioapic_entry(gsi_base, phys_addr);
+
+	return apic_id;
+}
+
+#endif /* CONFIG_PCI_IOAPIC */
include/linux/acpi.h +9 -0
index 2f73b81..ee8058e 100644
@@ -137,6 +137,15 @@ int acpi_unmap_lsapic(int cpu);
 int acpi_get_apicid(acpi_handle, int type, u32 acpi_id);
 int acpi_map_cpuid(int apic_id, u32 acpi_id);
 int acpi_get_cpuid(acpi_handle, int type, u32 acpi_id);
+#ifdef CONFIG_PCI_IOAPIC
+int acpi_get_ioapic_id(acpi_handle handle, u32 gsi_base, u64 *phys_addr);
+#else
+static inline int acpi_get_ioapic_id(acpi_handle handle, u32 gsi_base,
+					 u64 *phys_addr)
+{
+	return -1;
+}
+#endif
 
 int acpi_register_ioapic(acpi_handle handle, u64 phys_addr, u32 gsi_base);
 int acpi_unregister_ioapic(acpi_handle handle, u32 gsi_base);
Signature · 1 line
1.8.4
author
#50 lore raw

PATCH v5 29/33 PCI, x86, ACPI: Link acpi ioapic register to ioapic

In reply to 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.

---
 arch/x86/kernel/acpi/boot.c | 18 ++++++++++++++----
 1 file changed, 14 insertions(+), 4 deletions(-)
arch/x86/kernel/acpi/boot.c +14 -4
index 0f9c133..77bf505 100644
@@ -651,16 +651,26 @@ EXPORT_SYMBOL(acpi_unmap_lsapic);
 
 int acpi_register_ioapic(acpi_handle handle, u64 phys_addr, u32 gsi_base)
 {
-	/* TBD */
-	return -EINVAL;
+	u64 addr = 0;
+	int apic_id;
+
+	apic_id = acpi_get_ioapic_id(handle, gsi_base, &addr);
+
+	if (apic_id < 0)
+		return -1;
+
+	if (phys_addr != addr)
+		pr_warn("ioapic 0x%02x address is not consistent _CRS/dev(%llx) _MAT/MADT(%llx) \n",
+			apic_id, phys_addr, addr);
+
+	return	__mp_register_ioapic(apic_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);
Signature · 1 line
1.8.4
author
#51 lore raw

PATCH v5 30/33 PCI, x86, ACPI: Enable ioapic hotplug support with acpi host bridge.

In reply to 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.

---
 drivers/acpi/pci_root.c  |   4 ++
 drivers/pci/ioapic.c     | 147 ++++++++++++++++++++++++++++++-----------------
 include/linux/pci-acpi.h |   8 +++
 3 files changed, 106 insertions(+), 53 deletions(-)
drivers/acpi/pci_root.c +4 -0
index 20360e4..e666be3 100644
@@ -599,6 +599,8 @@ static int acpi_pci_root_add(struct acpi_device *device,
 		pci_assign_unassigned_root_bus_resources(root->bus);
 	}
 
+	acpi_pci_ioapic_add(root);
+
 	pci_bus_add_devices(root->bus);
 	return 1;
 
@@ -613,6 +615,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);
 
drivers/pci/ioapic.c +94 -53

195 lines not on this page · raw message

include/linux/pci-acpi.h +8 -0
index 5a462c4..d2a976a 100644
@@ -67,6 +67,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) { }
Signature · 1 line
1.8.4
maintainer
#52 lore raw

Re: [PATCH v5 30/33] PCI, x86, ACPI: Enable ioapic hotplug support with acpi host bridge.

drivers/pci/ioapic.c

Quoted 41 lines · 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.

---
 drivers/acpi/pci_root.c  |   4 ++
 drivers/pci/ioapic.c     | 147 ++++++++++++++++++++++++++++++-----------------
 include/linux/pci-acpi.h |   8 +++
 3 files changed, 106 insertions(+), 53 deletions(-)
drivers/acpi/pci_root.c +4 -0
index 20360e4..e666be3 100644
@@ -599,6 +599,8 @@ static int acpi_pci_root_add(struct acpi_device *device,
 		pci_assign_unassigned_root_bus_resources(root->bus);
 	}
 
+	acpi_pci_ioapic_add(root);
+
 	pci_bus_add_devices(root->bus);
 	return 1;
 
@@ -613,6 +615,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);
 
drivers/pci/ioapic.c +0 -0
index 2c2930e..60351b2 100644

The file comment:

* This driver manages PCI I/O APICs added by hotplug after boot. We try to
* claim all I/O APIC PCI devices, but those present at boot were registered
* when we parsed the ACPI MADT, so we'll fail when we try to re-register
* them.

is now incorrect because this no longer claims all PCI I/O APICs. As far as I can tell, it now claims every ACPI Device below the host bridge that has a _GSB method and an associated PCI device. If there is a PCI I/O APIC that does not appear in the namespace, this driver will not claim it.

I don't have a problem with that, but the comment needs to be updated to reflect the change.

Quoted 23 lines
drivers/pci/ioapic.c +9 -4
@@ -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;

drivers/pci/ioapic.c line 47

Quoted 10 lines
drivers/pci/ioapic.c +6 -2
-	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};

You can avoid the hard-coded 64-byte buffer with:

    struct acpi_buffer buffer = {ACPI_ALLOCATE_BUFFER, NULL};

And of course the corresponding kfree in the exit path. The acpi_get_name() could be done after all possible error exits, so you'd only need the kfree in the success path.

Quoted 34 lines
drivers/pci/ioapic.c +10 -19
-	handle = 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";

drivers/pci/ioapic.c line 72

Quoted 10 lines
drivers/pci/ioapic.c +3 -1
 
 	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";

Why not keep the 'type = "IOxAPIC"' assignment here as it was before? Then all the "type" manipulation is in one place.

Quoted 5 lines
drivers/pci/ioapic.c +0 -1
 	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))

drivers/pci/ioapic.c line 84

Quoted 10 lines
drivers/pci/ioapic.c +5 -3
+
+	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);

Why did you change the dev_info() to pr_info()? They look equivalent for what you're doing here.

Quoted 46 lines
drivers/pci/ioapic.c +19 -13
+	*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;

drivers/pci/ioapic.c line 122

Quoted 10 lines
drivers/pci/ioapic.c +3 -6
 
-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__);

I don't know if this message is necessary, but if it is, please include device identification, e.g., with dev_info() on the struct pci_dev.

Quoted 16 lines
drivers/pci/ioapic.c +14 -1
+		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)

drivers/pci/ioapic.c line 146

Quoted 10 lines
drivers/pci/ioapic.c +8 -1
 {
-	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);

This message should include the part of the namespace where we had the problem, e.g., the pathname of the host bridge device.

Quoted 41 lines
drivers/pci/ioapic.c +15 -2
 }
-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);
+}
include/linux/pci-acpi.h +8 -0
index 5a462c4..d2a976a 100644
@@ -67,6 +67,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) { }
Signature · 1 line
1.8.4
author
#53 lore raw

PATCH v5 31/33 ACPI, x86/PCI: Move resource_to_addr() to acpi generic

In reply to Yinghai Lu

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?

---
 arch/x86/pci/acpi.c     | 51 ++-----------------------------------------------
 drivers/acpi/resource.c | 46 ++++++++++++++++++++++++++++++++++++++++++++
 include/linux/acpi.h    |  3 +++
 3 files changed, 51 insertions(+), 49 deletions(-)
arch/x86/pci/acpi.c +2 -49
index 4f25ec0..9dc42c6 100644
@@ -219,60 +219,13 @@ 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;
 
-	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 *acpi_res, void *data)
 	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;
 
drivers/acpi/resource.c +46 -0

52 lines not on this page · raw message

include/linux/acpi.h +3 -0
index ee8058e..762980a 100644
@@ -293,6 +293,9 @@ unsigned long acpi_dev_irq_flags(u8 triggering, u8 polarity, u8 shareable);
 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;
Signature · 1 line
1.8.4
author
#54 lore raw

PATCH v5 32/33 PCI, x86, ACPI: get ioapic address from acpi device

In reply to 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.

-v2: Use acpi_mem_addr_resource_to_address64() to address MEMORY in _CRS
of Rui Wang's machine.

---
 drivers/pci/ioapic.c | 86 +++++++++++++++++++++++++++++++++++++++++++---------
 1 file changed, 71 insertions(+), 15 deletions(-)
drivers/pci/ioapic.c +71 -15
index 60351b2..54de155 100644
@@ -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_mem_addr_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);
Signature · 1 line
1.8.4
author
#55 lore raw

PATCH v5 33/33 x86, ioapic: Hotadd of IOAPICs described in static MADT

In reply to Yinghai Lu

From: Rui Wang <[email protected]>

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.

---
 arch/x86/kernel/apic/io_apic.c | 4 +++-
 drivers/pci/ioapic.c           | 3 ++-
 2 files changed, 5 insertions(+), 2 deletions(-)
arch/x86/kernel/apic/io_apic.c +3 -1
index 248a21d..ad28933 100644
@@ -3964,8 +3964,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) {
drivers/pci/ioapic.c +2 -1
index 54de155..0a23295 100644
@@ -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;
Signature · 1 line
1.8.4