[PATCH 0/6] Improvements to Yinghai's x86 IOAPIC hotplug work

[PATCH 0/6] Improvements to Yinghai's x86 IOAPIC hotplug work

[Jiang Liu]

This patchset is based on Yinghua's work for IOAPIC hotplug on x86 systems,
please refer to http://lwn.net/Articles/483671/. It applies to:
94738bb PCI: Disable mem in the ioapic removing path
at
git://git.kernel.org/pub/scm/linux/kernel/git/yinghai/linux-yinghai.git for-x86-irq

Due to resource limitation, (only an Intel Atom system available for building
and testing), only the boot logic has been tested until now, so need more tests
for the hotplug logic when having access to systems with IOAPIC hotplug feature.

BTW, it's really a nightmare to build and test kernel on an Intel Atom system.

Jiang Liu (6):
  x86,IRQ: Fix possible invalid memory access after IOAPIC hot-plugging
  x86,IRQ: Mark unused entries in 'ioapics' array as free at startup
  x86,IRQ: Enhance irq allocation policy for hot-added IOAPICs
  x86,IRQ: split out function ioapic_setup_resource()
  x86,IRQ: Correctly manage MMIO resource used by IOAPIC when
    hot-plugging IOPAICs
  x86,IRQ: Use memory barriers to protect searching side code

 arch/x86/kernel/apic/io_apic.c |  150 +++++++++++++++++++++++++++++-----------
 1 files changed, 110 insertions(+), 40 deletions(-)

-- 
1.7.5.4

[PATCH 0/5] Improvements to Yinghai's IOAPIC hotplug work on x86

[Jiang Liu]

This patchset is based on Yinghua's work for IOAPIC hotplug on x86,
please refer to http://lwn.net/Articles/483671/.
The first two patches are minor bugfixes, the third and fourth are
enhancement to Yinghai's implementation, and the fifth is a RFC patch.

I only have an Atom system at hand to build and test the patchset,
so only the boot logic has been tested until now, need more tests
about the hotplug logic when having access to systems which support
IOAPIC hotplug feature.

BTW, it's really a nightmare to build and test kernel on an Atom system.

Jiang Liu (5):
  x86:IRQ: Fix possible invalid memory access when hot-plugging IOAPICs
  x86:IRQ: Mark unused entries in 'ioapics' array as free at startup
  x86:IRQ: Enhance irq allocation policy for hot-added IOAPICs
  x86:IRQ: Correctly manage MMIO resource used by IOAPIC when
    hot-plugging
  x86:IRQ: Use memory barriers to protect searching side code

 arch/x86/kernel/apic/io_apic.c |  159 ++++++++++++++++++++++++++++------------
 1 files changed, 113 insertions(+), 46 deletions(-)

-- 
1.7.5.4

[PATCH 1/6] x86,IRQ: Fix possible invalid memory access after IOAPIC hot-plugging

[Jiang Liu]

Function free_ioapic_saved_registers() should mark ioapic->saved_regiters
as NULL after the memory has been freed. Otherwise when hot-adding
another IOAPIC, function alloc_ioapic_saved_registers() may reuse the
stale pointer and cause memory corruption.

Signed-off-by: Jiang Liu <jiang.liu@huawei.com>
---
 arch/x86/kernel/apic/io_apic.c |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/arch/x86/kernel/apic/io_apic.c b/arch/x86/kernel/apic/io_apic.c
index da02320..7412eb8 100644
--- a/arch/x86/kernel/apic/io_apic.c
+++ b/arch/x86/kernel/apic/io_apic.c
@@ -362,6 +362,7 @@ static void alloc_ioapic_saved_registers(int idx)
 static void free_ioapic_saved_registers(int idx)
 {
 	kfree(ioapics[idx].saved_registers);
+	ioapics[idx].saved_registers = NULL;
 }
 
 int __init arch_early_irq_init(void)
-- 
1.7.5.4

[PATCH 2/6] x86,IRQ: Mark unused entries in 'ioapics' array as free at startup

[Jiang Liu]

Unused entries in ioapics array should be marked as free at startup,
so they could be used by hot-added IOAPICs.

Signed-off-by: Jiang Liu <jiang.liu@huawei.com>
---
 arch/x86/kernel/apic/io_apic.c |    4 ++++
 1 files changed, 4 insertions(+), 0 deletions(-)

diff --git a/arch/x86/kernel/apic/io_apic.c b/arch/x86/kernel/apic/io_apic.c
index 7412eb8..622374f 100644
--- a/arch/x86/kernel/apic/io_apic.c
+++ b/arch/x86/kernel/apic/io_apic.c
@@ -392,6 +392,10 @@ int __init arch_early_irq_init(void)
 		cpumask_set_cpu(0, cfg->domain);
 	}
 
+	/* Mark all left entries as free for IOAPIC hot-adding. */
+	for (i = nr_ioapics; i < MAX_IO_APICS; i++)
+		ioapics[i].mp_config.apicid = 0xff;
+
 	return 0;
 }
 
-- 
1.7.5.4

[PATCH 3/6] x86,IRQ: Enhance irq allocation policy for hot-added IOAPICs

[Jiang Liu]

For hot-added IOAPICs, try to allocate irqs starting from
ioapic->gsi_base first, and fallback to starting from 0.

Signed-off-by: Jiang Liu <jiang.liu@huawei.com>
---
 arch/x86/kernel/apic/io_apic.c |    9 ++++++---
 1 files changed, 6 insertions(+), 3 deletions(-)

diff --git a/arch/x86/kernel/apic/io_apic.c b/arch/x86/kernel/apic/io_apic.c
index 622374f..e5b6ca4 100644
--- a/arch/x86/kernel/apic/io_apic.c
+++ b/arch/x86/kernel/apic/io_apic.c
@@ -294,13 +294,16 @@ static struct irq_cfg *realloc_irq_and_cfg_at(unsigned int at, int node)
 	return alloc_irq_and_cfg_at(at, node);
 }
 
-static int reserve_ioapic_gsi_irq_base(int idx)
+static int reserve_ioapic_gsi_irq_base(int idx, bool hotadd)
 {
 	int irq;
 	struct mp_ioapic_gsi *gsi_cfg = mp_ioapic_gsi_routing(idx);
 	int cnt = gsi_cfg->gsi_end - gsi_cfg->gsi_base + 1;
 
 	irq = __irq_reserve_irqs(-1, gsi_cfg->gsi_base, cnt);
+	if (irq < 0 && hotadd)
+		irq = __irq_reserve_irqs(-1, 0, cnt);
+
 	if (irq >= 0) {
 		gsi_cfg->irq_base = irq;
 		printk(KERN_INFO
@@ -378,7 +381,7 @@ int __init arch_early_irq_init(void)
 		alloc_ioapic_saved_registers(i);
 
 	for (i = 0; i < nr_ioapics; i++)
-		reserve_ioapic_gsi_irq_base(i);
+		reserve_ioapic_gsi_irq_base(i, false);
 
 	reserve_ioapic_gsi_irq_extra();
 
@@ -4181,7 +4184,7 @@ int __mp_register_ioapic(int id, u32 address, u32 gsi_base, bool hotadd)
 		if (gsi_cfg->gsi_end >= gsi_top)
 			gsi_top = gsi_cfg->gsi_end + 1;
 	} else {
-		int irq = reserve_ioapic_gsi_irq_base(idx);
+		int irq = reserve_ioapic_gsi_irq_base(idx, true);
 
 		if (irq < 0)
 			goto failed;
-- 
1.7.5.4

[PATCH 4/6] x86,IRQ: split out function ioapic_setup_resource()

[Jiang Liu]

Split out function ioapic_setup_resource(), which will be used by IOAPIC
hotplug logic.

Signed-off-by: Jiang Liu <jiang.liu@huawei.com>
---
 arch/x86/kernel/apic/io_apic.c |   69 ++++++++++++++++++++++------------------
 1 files changed, 38 insertions(+), 31 deletions(-)

diff --git a/arch/x86/kernel/apic/io_apic.c b/arch/x86/kernel/apic/io_apic.c
index e5b6ca4..dfb6f45 100644
--- a/arch/x86/kernel/apic/io_apic.c
+++ b/arch/x86/kernel/apic/io_apic.c
@@ -85,6 +85,7 @@ static struct ioapic {
 	 * Saved state during suspend/resume, or while enabling intr-remap.
 	 */
 	struct IO_APIC_route_entry *saved_registers;
+	struct resource *resource;
 	/* I/O APIC config */
 	struct mpc_ioapic mp_config;
 	/* IO APIC gsi routing info */
@@ -3987,7 +3988,7 @@ void __init setup_ioapic_dest(void)
 
 static struct resource *ioapic_resources;
 
-static struct resource * __init ioapic_setup_resources(int nr_ioapics)
+static struct resource * __init ioapic_alloc_resources(int nr_ioapics)
 {
 	unsigned long n;
 	struct resource *res;
@@ -4010,6 +4011,7 @@ static struct resource * __init ioapic_setup_resources(int nr_ioapics)
 		res[i].flags = IORESOURCE_MEM | IORESOURCE_BUSY;
 		snprintf(mem, IOAPIC_RESOURCE_NAME_SIZE, "IOAPIC %u", i);
 		mem += IOAPIC_RESOURCE_NAME_SIZE;
+		ioapics[i].resource = &res[i];
 	}
 
 	ioapic_resources = res;
@@ -4017,43 +4019,48 @@ static struct resource * __init ioapic_setup_resources(int nr_ioapics)
 	return res;
 }
 
-void __init ioapic_and_gsi_init(void)
+static void ioapic_setup_resource(int idx, struct resource *res, bool hotadd)
 {
-	unsigned long ioapic_phys, idx = FIX_IO_APIC_BASE_0;
-	struct resource *ioapic_res;
-	int i;
+	unsigned long ioapic_phys, vaddr;
 
-	ioapic_res = ioapic_setup_resources(nr_ioapics);
-	for (i = 0; i < nr_ioapics; i++) {
-		if (smp_found_config) {
-			ioapic_phys = mpc_ioapic_addr(i);
+	if (hotadd) {
+		ioapic_phys = mpc_ioapic_addr(idx);
+	} else if (smp_found_config) {
+		ioapic_phys = mpc_ioapic_addr(idx);
 #ifdef CONFIG_X86_32
-			if (!ioapic_phys) {
-				printk(KERN_ERR
-				       "WARNING: bogus zero IO-APIC "
-				       "address found in MPTABLE, "
-				       "disabling IO/APIC support!\n");
-				smp_found_config = 0;
-				skip_ioapic_setup = 1;
-				goto fake_ioapic_page;
-			}
+		if (!ioapic_phys) {
+			printk(KERN_ERR
+			       "WARNING: bogus zero IO-APIC address found "
+			       "in MPTABLE, disabling IO/APIC support!\n");
+			smp_found_config = 0;
+			skip_ioapic_setup = 1;
+			goto fake_ioapic_page;
+		}
 #endif
-		} else {
+	} else {
 #ifdef CONFIG_X86_32
 fake_ioapic_page:
 #endif
-			ioapic_phys = (unsigned long)alloc_bootmem_pages(PAGE_SIZE);
-			ioapic_phys = __pa(ioapic_phys);
-		}
-		set_fixmap_nocache(idx, ioapic_phys);
-		apic_printk(APIC_VERBOSE, "mapped IOAPIC to %08lx (%08lx)\n",
-			__fix_to_virt(idx) + (ioapic_phys & ~PAGE_MASK),
-			ioapic_phys);
-		idx++;
-
-		ioapic_res->start = ioapic_phys;
-		ioapic_res->end = ioapic_phys + IO_APIC_SLOT_SIZE - 1;
-		ioapic_res++;
+		ioapic_phys = (unsigned long)alloc_bootmem_pages(PAGE_SIZE);
+		ioapic_phys = __pa(ioapic_phys);
+	}
+
+	vaddr = __fix_to_virt(FIX_IO_APIC_BASE_0 + idx);
+	vaddr += (ioapic_phys & ~PAGE_MASK);
+	apic_printk(APIC_VERBOSE, "mapped IOAPIC to %08lx (%08lx)\n",
+		vaddr, ioapic_phys);
+
+	res->start = ioapic_phys;
+	res->end = ioapic_phys + IO_APIC_SLOT_SIZE - 1;
+}
+
+void __init ioapic_and_gsi_init(void)
+{
+	int i;
+
+	ioapic_alloc_resources(nr_ioapics);
+	for (i = 0; i < nr_ioapics; i++) {
+		ioapic_setup_resource(i, ioapics[i].resource, false);
 	}
 
 	probe_nr_irqs_gsi();
-- 
1.7.5.4

[PATCH 5/6] x86,IRQ: Correctly manage MMIO resource used by IOAPIC when hot-plugging IOPAICs

[Jiang Liu]

Correctly manage MMIO resource used by IOAPIC when hot-plugging IOAPICs.

Signed-off-by: Jiang Liu <jiang.liu@huawei.com>
---
 arch/x86/kernel/apic/io_apic.c |   46 +++++++++++++++++++++++++++++++++++++++-
 1 files changed, 45 insertions(+), 1 deletions(-)

diff --git a/arch/x86/kernel/apic/io_apic.c b/arch/x86/kernel/apic/io_apic.c
index dfb6f45..9c2ac7b 100644
--- a/arch/x86/kernel/apic/io_apic.c
+++ b/arch/x86/kernel/apic/io_apic.c
@@ -4054,6 +4054,42 @@ fake_ioapic_page:
 	res->end = ioapic_phys + IO_APIC_SLOT_SIZE - 1;
 }
 
+static struct resource *ioapic_add_resource(int idx)
+{
+	char *name;
+	struct resource *res;
+	size_t sz = IOAPIC_RESOURCE_NAME_SIZE + sizeof(struct resource);
+
+	res = kzalloc(sz, GFP_KERNEL);
+	if (res) {
+		res->flags = IORESOURCE_MEM | IORESOURCE_BUSY;
+		res->name = name = (char *)(res + 1);
+		snprintf(name, IOAPIC_RESOURCE_NAME_SIZE, "IOAPIC %u", idx);
+		ioapic_setup_resource(idx, res, true);
+		ioapics[idx].resource = res;
+
+		if (insert_resource(&iomem_resource, res)) {
+			kfree(res);
+			res = NULL;
+		}
+	}
+
+	return res;
+}
+
+static void ioapic_remove_resource(int idx)
+{
+	struct resource *res = ioapics[idx].resource;
+
+	if (res) {
+		if (res->parent)
+			release_resource(res);
+		if (PageSlab(virt_to_page(res)))
+			kfree(res);
+		ioapics[idx].resource = NULL;
+	}
+}
+
 void __init ioapic_and_gsi_init(void)
 {
 	int i;
@@ -4191,8 +4227,12 @@ int __mp_register_ioapic(int id, u32 address, u32 gsi_base, bool hotadd)
 		if (gsi_cfg->gsi_end >= gsi_top)
 			gsi_top = gsi_cfg->gsi_end + 1;
 	} else {
-		int irq = reserve_ioapic_gsi_irq_base(idx, true);
+		int irq;
 
+		if (ioapic_add_resource(idx) == NULL)
+			goto failed;
+
+		irq = reserve_ioapic_gsi_irq_base(idx, true);
 		if (irq < 0)
 			goto failed;
 
@@ -4210,6 +4250,7 @@ int __mp_register_ioapic(int id, u32 address, u32 gsi_base, bool hotadd)
 	return 0;
 
 failed:
+	ioapic_remove_resource(idx);
 	clear_fixmap(FIX_IO_APIC_BASE_0 + idx);
 	memset(&ioapics[idx], 0, sizeof(struct ioapic));
 	ioapics[idx].mp_config.apicid = 0xff;
@@ -4234,6 +4275,9 @@ int mp_unregister_ioapic(u32 gsi_base)
 	free_ioapic_gsi_irq_base(idx);
 
 	clear_fixmap(FIX_IO_APIC_BASE_0 + idx);
+
+	ioapic_remove_resource(idx);
+
 	memset(&ioapics[idx], 0, sizeof(struct ioapic));
 	ioapics[idx].mp_config.apicid = 0xff;
 
-- 
1.7.5.4

[PATCH 6/6] x86,IRQ: Use memory barriers to protect searching side code

[Jiang Liu]

Use memory barriers to protect searching side code when hot-plugging
IOAPICs.

Signed-off-by: Jiang Liu <jiang.liu@huawei.com>
---
 arch/x86/kernel/apic/io_apic.c |   25 ++++++++++++++++++-------
 1 files changed, 18 insertions(+), 7 deletions(-)

diff --git a/arch/x86/kernel/apic/io_apic.c b/arch/x86/kernel/apic/io_apic.c
index 9c2ac7b..49c016a 100644
--- a/arch/x86/kernel/apic/io_apic.c
+++ b/arch/x86/kernel/apic/io_apic.c
@@ -352,12 +352,15 @@ static void free_ioapic_gsi_irq_base(int idx)
 
 static void alloc_ioapic_saved_registers(int idx)
 {
+	struct mp_ioapic_gsi *gsi_cfg = mp_ioapic_gsi_routing(idx);
+	int irq_cnt = gsi_cfg->gsi_end - gsi_cfg->gsi_base + 1;
+
 	if (ioapics[idx].saved_registers)
 		return;
 
 	ioapics[idx].saved_registers =
 			kzalloc(sizeof(struct IO_APIC_route_entry) *
-				ioapics[idx].nr_registers, GFP_KERNEL);
+				irq_cnt, GFP_KERNEL);
 
 	if (!ioapics[idx].saved_registers)
 		pr_err("IOAPIC %d: suspend/resume impossible!\n", idx);
@@ -1174,6 +1177,8 @@ int IO_APIC_get_PCI_irq_vector(int bus, int slot, int pin,
 		for (ioapic_idx = 0; ioapic_idx < nr_ioapics; ioapic_idx++) {
 			if (!ioapics[ioapic_idx].nr_registers)
 				continue;
+
+			smp_rmb();
 			if (mpc_ioapic_id(ioapic_idx) == mp_irqs[i].dstapic ||
 			    mp_irqs[i].dstapic == MP_APIC_ALL)
 				break;
@@ -2083,7 +2088,7 @@ void __init enable_IO_APIC(void)
 	if (!legacy_pic->nr_legacy_irqs)
 		return;
 
-	for(apic = 0; apic < nr_ioapics; apic++) {
+	for (apic = 0; apic < nr_ioapics; apic++) {
 		int pin;
 		/* See if any of the pins is in ExtINT mode */
 		for (pin = 0; pin < ioapics[apic].nr_registers; pin++) {
@@ -4134,6 +4139,7 @@ int mp_find_ioapic(u32 gsi)
 		if (!ioapics[i].nr_registers)
 			continue;
 
+		smp_rmb();
 		if ((gsi >= gsi_cfg->gsi_base) && (gsi <= gsi_cfg->gsi_end))
 			return i;
 	}
@@ -4214,11 +4220,6 @@ int __mp_register_ioapic(int id, u32 address, u32 gsi_base, bool hotadd)
 	gsi_cfg->gsi_base = gsi_base;
 	gsi_cfg->gsi_end = gsi_base + entries - 1;
 
-	/*
-	 * The number of IO-APIC IRQ registers (== #pins):
-	 */
-	ioapics[idx].nr_registers = entries;
-
 	if (!hotadd) {
 		/*
 		 * irqs will be reserved in arch_early_irq_init()
@@ -4247,6 +4248,13 @@ int __mp_register_ioapic(int id, u32 address, u32 gsi_base, bool hotadd)
 	if (idx == nr_ioapics)
 		nr_ioapics++;
 
+	smp_wmb();
+
+	/*
+	 * The number of IO-APIC IRQ registers (== #pins):
+	 */
+	ioapics[idx].nr_registers = entries;
+
 	return 0;
 
 failed:
@@ -4270,6 +4278,9 @@ int mp_unregister_ioapic(u32 gsi_base)
 	if (idx < 0)
 		return -EINVAL;
 
+	ioapics[idx].nr_registers = 0;
+	smp_wmb();
+
 	free_ioapic_saved_registers(idx);
 
 	free_ioapic_gsi_irq_base(idx);
-- 
1.7.5.4

Re: [PATCH 2/6] x86,IRQ: Mark unused entries in 'ioapics' array as free at startup

[Yinghai Lu]

On Tue, Mar 20, 2012 at 9:20 AM, Jiang Liu <liuj97@gmail.com> wrote:
> Unused entries in ioapics array should be marked as free at startup,
> so they could be used by hot-added IOAPICs.
>
> Signed-off-by: Jiang Liu <jiang.liu@huawei.com>
> ---
>  arch/x86/kernel/apic/io_apic.c |    4 ++++
>  1 files changed, 4 insertions(+), 0 deletions(-)
>
> diff --git a/arch/x86/kernel/apic/io_apic.c b/arch/x86/kernel/apic/io_apic.c
> index 7412eb8..622374f 100644
> --- a/arch/x86/kernel/apic/io_apic.c
> +++ b/arch/x86/kernel/apic/io_apic.c
> @@ -392,6 +392,10 @@ int __init arch_early_irq_init(void)
>                cpumask_set_cpu(0, cfg->domain);
>        }
>
> +       /* Mark all left entries as free for IOAPIC hot-adding. */
> +       for (i = nr_ioapics; i < MAX_IO_APICS; i++)
> +               ioapics[i].mp_config.apicid = 0xff;
> +
>        return 0;
>  }

this one looks like not needed, we did not search that after
nr_ioapics, and just use nr_ioapics if open spot is found.

Yinghai

Re: [PATCH 2/6] x86,IRQ: Mark unused entries in 'ioapics' array as free at startup

[Jiang Liu]

So seems another patch is needed to search all entries in the ioapics
array to find a free entry for real hot-added IOAPICs. Current
implementation can only support hot-replace scenario, but can't support
real hot-add. If needed, I will work out another patch to address that.

Gerry

On 2012-3-21 11:25, Yinghai Lu wrote:
> On Tue, Mar 20, 2012 at 9:20 AM, Jiang Liu<liuj97@gmail.com>  wrote:
>> Unused entries in ioapics array should be marked as free at startup,
>> so they could be used by hot-added IOAPICs.
>>
>> Signed-off-by: Jiang Liu<jiang.liu@huawei.com>
>> ---
>>   arch/x86/kernel/apic/io_apic.c |    4 ++++
>>   1 files changed, 4 insertions(+), 0 deletions(-)
>>
>> diff --git a/arch/x86/kernel/apic/io_apic.c b/arch/x86/kernel/apic/io_apic.c
>> index 7412eb8..622374f 100644
>> --- a/arch/x86/kernel/apic/io_apic.c
>> +++ b/arch/x86/kernel/apic/io_apic.c
>> @@ -392,6 +392,10 @@ int __init arch_early_irq_init(void)
>>                 cpumask_set_cpu(0, cfg->domain);
>>         }
>>
>> +       /* Mark all left entries as free for IOAPIC hot-adding. */
>> +       for (i = nr_ioapics; i<  MAX_IO_APICS; i++)
>> +               ioapics[i].mp_config.apicid = 0xff;
>> +
>>         return 0;
>>   }
>
> this one looks like not needed, we did not search that after
> nr_ioapics, and just use nr_ioapics if open spot is found.
>
> Yinghai
>
>

Re: [PATCH 4/6] x86,IRQ: split out function ioapic_setup_resource()

[Yinghai Lu]

On Tue, Mar 20, 2012 at 9:21 AM, Jiang Liu <liuj97@gmail.com> wrote:
> Split out function ioapic_setup_resource(), which will be used by IOAPIC
> hotplug logic.
>
> Signed-off-by: Jiang Liu <jiang.liu@huawei.com>
> ---
>  arch/x86/kernel/apic/io_apic.c |   69 ++++++++++++++++++++++------------------
>  1 files changed, 38 insertions(+), 31 deletions(-)
>
> diff --git a/arch/x86/kernel/apic/io_apic.c b/arch/x86/kernel/apic/io_apic.c
> index e5b6ca4..dfb6f45 100644
> --- a/arch/x86/kernel/apic/io_apic.c
> +++ b/arch/x86/kernel/apic/io_apic.c
> @@ -85,6 +85,7 @@ static struct ioapic {
>         * Saved state during suspend/resume, or while enabling intr-remap.
>         */
>        struct IO_APIC_route_entry *saved_registers;
> +       struct resource *resource;
>        /* I/O APIC config */
>        struct mpc_ioapic mp_config;
>        /* IO APIC gsi routing info */
> @@ -3987,7 +3988,7 @@ void __init setup_ioapic_dest(void)
>
>  static struct resource *ioapic_resources;
>
> -static struct resource * __init ioapic_setup_resources(int nr_ioapics)
> +static struct resource * __init ioapic_alloc_resources(int nr_ioapics)
>  {
>        unsigned long n;
>        struct resource *res;
> @@ -4010,6 +4011,7 @@ static struct resource * __init ioapic_setup_resources(int nr_ioapics)
>                res[i].flags = IORESOURCE_MEM | IORESOURCE_BUSY;
>                snprintf(mem, IOAPIC_RESOURCE_NAME_SIZE, "IOAPIC %u", i);
>                mem += IOAPIC_RESOURCE_NAME_SIZE;
> +               ioapics[i].resource = &res[i];
>        }
>
>        ioapic_resources = res;
> @@ -4017,43 +4019,48 @@ static struct resource * __init ioapic_setup_resources(int nr_ioapics)
>        return res;
>  }
>
> -void __init ioapic_and_gsi_init(void)
> +static void ioapic_setup_resource(int idx, struct resource *res, bool hotadd)
>  {
> -       unsigned long ioapic_phys, idx = FIX_IO_APIC_BASE_0;
> -       struct resource *ioapic_res;
> -       int i;
> +       unsigned long ioapic_phys, vaddr;
>
> -       ioapic_res = ioapic_setup_resources(nr_ioapics);
> -       for (i = 0; i < nr_ioapics; i++) {
> -               if (smp_found_config) {
> -                       ioapic_phys = mpc_ioapic_addr(i);
> +       if (hotadd) {
> +               ioapic_phys = mpc_ioapic_addr(idx);
> +       } else if (smp_found_config) {
> +               ioapic_phys = mpc_ioapic_addr(idx);
>  #ifdef CONFIG_X86_32
> -                       if (!ioapic_phys) {
> -                               printk(KERN_ERR
> -                                      "WARNING: bogus zero IO-APIC "
> -                                      "address found in MPTABLE, "
> -                                      "disabling IO/APIC support!\n");
> -                               smp_found_config = 0;
> -                               skip_ioapic_setup = 1;
> -                               goto fake_ioapic_page;
> -                       }
> +               if (!ioapic_phys) {
> +                       printk(KERN_ERR
> +                              "WARNING: bogus zero IO-APIC address found "
> +                              "in MPTABLE, disabling IO/APIC support!\n");
> +                       smp_found_config = 0;
> +                       skip_ioapic_setup = 1;
> +                       goto fake_ioapic_page;
> +               }
>  #endif
> -               } else {
> +       } else {
>  #ifdef CONFIG_X86_32
>  fake_ioapic_page:
>  #endif
> -                       ioapic_phys = (unsigned long)alloc_bootmem_pages(PAGE_SIZE);
> -                       ioapic_phys = __pa(ioapic_phys);
> -               }
> -               set_fixmap_nocache(idx, ioapic_phys);
> -               apic_printk(APIC_VERBOSE, "mapped IOAPIC to %08lx (%08lx)\n",
> -                       __fix_to_virt(idx) + (ioapic_phys & ~PAGE_MASK),
> -                       ioapic_phys);
> -               idx++;
> -
> -               ioapic_res->start = ioapic_phys;
> -               ioapic_res->end = ioapic_phys + IO_APIC_SLOT_SIZE - 1;
> -               ioapic_res++;
> +               ioapic_phys = (unsigned long)alloc_bootmem_pages(PAGE_SIZE);
> +               ioapic_phys = __pa(ioapic_phys);

where is calling set_fixmap_nocache for fake ioapic one?

> +       }
> +
> +       vaddr = __fix_to_virt(FIX_IO_APIC_BASE_0 + idx);
> +       vaddr += (ioapic_phys & ~PAGE_MASK);
> +       apic_printk(APIC_VERBOSE, "mapped IOAPIC to %08lx (%08lx)\n",
> +               vaddr, ioapic_phys);
> +
> +       res->start = ioapic_phys;
> +       res->end = ioapic_phys + IO_APIC_SLOT_SIZE - 1;
> +}
> +
> +void __init ioapic_and_gsi_init(void)
> +{
> +       int i;
> +
> +       ioapic_alloc_resources(nr_ioapics);
> +       for (i = 0; i < nr_ioapics; i++) {
> +               ioapic_setup_resource(i, ioapics[i].resource, false);
>        }

how about booting system with ioapic device and later hot remove it?

maybe we can just alloc one by one.

Yinghai

Re: [PATCH 4/6] x86,IRQ: split out function ioapic_setup_resource()

[Jiang Liu]

On 2012-3-21 11:34, Yinghai Lu wrote:
> On Tue, Mar 20, 2012 at 9:21 AM, Jiang Liu<liuj97@gmail.com>  wrote:
>> Split out function ioapic_setup_resource(), which will be used by IOAPIC
>> hotplug logic.
>>
>> Signed-off-by: Jiang Liu<jiang.liu@huawei.com>
>> ---
>>   arch/x86/kernel/apic/io_apic.c |   69 ++++++++++++++++++++++------------------
>>   1 files changed, 38 insertions(+), 31 deletions(-)
>>
>> diff --git a/arch/x86/kernel/apic/io_apic.c b/arch/x86/kernel/apic/io_apic.c
>> index e5b6ca4..dfb6f45 100644
>> --- a/arch/x86/kernel/apic/io_apic.c
>> +++ b/arch/x86/kernel/apic/io_apic.c
>> @@ -85,6 +85,7 @@ static struct ioapic {
>>          * Saved state during suspend/resume, or while enabling intr-remap.
>>          */
>>         struct IO_APIC_route_entry *saved_registers;
>> +       struct resource *resource;
>>         /* I/O APIC config */
>>         struct mpc_ioapic mp_config;
>>         /* IO APIC gsi routing info */
>> @@ -3987,7 +3988,7 @@ void __init setup_ioapic_dest(void)
>>
>>   static struct resource *ioapic_resources;
>>
>> -static struct resource * __init ioapic_setup_resources(int nr_ioapics)
>> +static struct resource * __init ioapic_alloc_resources(int nr_ioapics)
>>   {
>>         unsigned long n;
>>         struct resource *res;
>> @@ -4010,6 +4011,7 @@ static struct resource * __init ioapic_setup_resources(int nr_ioapics)
>>                 res[i].flags = IORESOURCE_MEM | IORESOURCE_BUSY;
>>                 snprintf(mem, IOAPIC_RESOURCE_NAME_SIZE, "IOAPIC %u", i);
>>                 mem += IOAPIC_RESOURCE_NAME_SIZE;
>> +               ioapics[i].resource =&res[i];
>>         }
>>
>>         ioapic_resources = res;
>> @@ -4017,43 +4019,48 @@ static struct resource * __init ioapic_setup_resources(int nr_ioapics)
>>         return res;
>>   }
>>
>> -void __init ioapic_and_gsi_init(void)
>> +static void ioapic_setup_resource(int idx, struct resource *res, bool hotadd)
>>   {
>> -       unsigned long ioapic_phys, idx = FIX_IO_APIC_BASE_0;
>> -       struct resource *ioapic_res;
>> -       int i;
>> +       unsigned long ioapic_phys, vaddr;
>>
>> -       ioapic_res = ioapic_setup_resources(nr_ioapics);
>> -       for (i = 0; i<  nr_ioapics; i++) {
>> -               if (smp_found_config) {
>> -                       ioapic_phys = mpc_ioapic_addr(i);
>> +       if (hotadd) {
>> +               ioapic_phys = mpc_ioapic_addr(idx);
>> +       } else if (smp_found_config) {
>> +               ioapic_phys = mpc_ioapic_addr(idx);
>>   #ifdef CONFIG_X86_32
>> -                       if (!ioapic_phys) {
>> -                               printk(KERN_ERR
>> -                                      "WARNING: bogus zero IO-APIC "
>> -                                      "address found in MPTABLE, "
>> -                                      "disabling IO/APIC support!\n");
>> -                               smp_found_config = 0;
>> -                               skip_ioapic_setup = 1;
>> -                               goto fake_ioapic_page;
>> -                       }
>> +               if (!ioapic_phys) {
>> +                       printk(KERN_ERR
>> +                              "WARNING: bogus zero IO-APIC address found "
>> +                              "in MPTABLE, disabling IO/APIC support!\n");
>> +                       smp_found_config = 0;
>> +                       skip_ioapic_setup = 1;
>> +                       goto fake_ioapic_page;
>> +               }
>>   #endif
>> -               } else {
>> +       } else {
>>   #ifdef CONFIG_X86_32
>>   fake_ioapic_page:
>>   #endif
>> -                       ioapic_phys = (unsigned long)alloc_bootmem_pages(PAGE_SIZE);
>> -                       ioapic_phys = __pa(ioapic_phys);
>> -               }
>> -               set_fixmap_nocache(idx, ioapic_phys);
>> -               apic_printk(APIC_VERBOSE, "mapped IOAPIC to %08lx (%08lx)\n",
>> -                       __fix_to_virt(idx) + (ioapic_phys&  ~PAGE_MASK),
>> -                       ioapic_phys);
>> -               idx++;
>> -
>> -               ioapic_res->start = ioapic_phys;
>> -               ioapic_res->end = ioapic_phys + IO_APIC_SLOT_SIZE - 1;
>> -               ioapic_res++;
>> +               ioapic_phys = (unsigned long)alloc_bootmem_pages(PAGE_SIZE);
>> +               ioapic_phys = __pa(ioapic_phys);
>
> where is calling set_fixmap_nocache for fake ioapic one?
My bad, doesn't notice the case for fake ioapic, will add back the
set_fixmap_nocache. I just noticed that set_fixmap_nocache() will
be called twice for real IOAPICs.

>
>> +       }
>> +
>> +       vaddr = __fix_to_virt(FIX_IO_APIC_BASE_0 + idx);
>> +       vaddr += (ioapic_phys&  ~PAGE_MASK);
>> +       apic_printk(APIC_VERBOSE, "mapped IOAPIC to %08lx (%08lx)\n",
>> +               vaddr, ioapic_phys);
>> +
>> +       res->start = ioapic_phys;
>> +       res->end = ioapic_phys + IO_APIC_SLOT_SIZE - 1;
>> +}
>> +
>> +void __init ioapic_and_gsi_init(void)
>> +{
>> +       int i;
>> +
>> +       ioapic_alloc_resources(nr_ioapics);
>> +       for (i = 0; i<  nr_ioapics; i++) {
>> +               ioapic_setup_resource(i, ioapics[i].resource, false);
>>         }
>
> how about booting system with ioapic device and later hot remove it?
>
> maybe we can just alloc one by one.
Here the issue is that, it uses bootmem allocator at boot time, which
is paged aligned. So seems a little waste to allocate one page for
each ioapic. The tradeoff here is to leak the memory if all IOAPICs
are removed at runtime. If needed, we could add counter to track the
allcoated bootmem and free it when counter reaches zero.

>
> Yinghai
>
> .
>

Re: [PATCH 2/6] x86,IRQ: Mark unused entries in 'ioapics' array as free at startup

[Jiang Liu]

Hi Yinghai,
	Get your points now and will drop this patch from the series.
	Thanks!

On 03/21/2012 11:32 AM, Jiang Liu wrote:
> So seems another patch is needed to search all entries in the ioapics
> array to find a free entry for real hot-added IOAPICs. Current
> implementation can only support hot-replace scenario, but can't support
> real hot-add. If needed, I will work out another patch to address that.
> 
> Gerry
> 
> On 2012-3-21 11:25, Yinghai Lu wrote:
>> On Tue, Mar 20, 2012 at 9:20 AM, Jiang Liu<liuj97@gmail.com>  wrote:
>>> Unused entries in ioapics array should be marked as free at startup,
>>> so they could be used by hot-added IOAPICs.
>>>
>>> Signed-off-by: Jiang Liu<jiang.liu@huawei.com>
>>> ---
>>>   arch/x86/kernel/apic/io_apic.c |    4 ++++
>>>   1 files changed, 4 insertions(+), 0 deletions(-)
>>>
>>> diff --git a/arch/x86/kernel/apic/io_apic.c b/arch/x86/kernel/apic/io_apic.c
>>> index 7412eb8..622374f 100644
>>> --- a/arch/x86/kernel/apic/io_apic.c
>>> +++ b/arch/x86/kernel/apic/io_apic.c
>>> @@ -392,6 +392,10 @@ int __init arch_early_irq_init(void)
>>>                 cpumask_set_cpu(0, cfg->domain);
>>>         }
>>>
>>> +       /* Mark all left entries as free for IOAPIC hot-adding. */
>>> +       for (i = nr_ioapics; i<  MAX_IO_APICS; i++)
>>> +               ioapics[i].mp_config.apicid = 0xff;
>>> +
>>>         return 0;
>>>   }
>>
>> this one looks like not needed, we did not search that after
>> nr_ioapics, and just use nr_ioapics if open spot is found.
>>
>> Yinghai
>>
>>
> 
>