x2APIC and many-APIC systems...

Ingo, Suresh,

Commit a35fd28256e7736cc84af8931a16224f0bfaaf6c prevents x2APIC structures being used from the ACPI MADT if the cores don't advertise the x2APIC feature. Commit c284b42abadbb22083bfde24d308899c08d44ffa prevents onlining cores with APIC ID >255 in non-x2APIC mode. Since NumaChip/NumaConnect uses x2APIC structures to describe non-x2APIC systems (AMD Opteron) with lots of APICs, we thus now can't boot all the cores.

We are able to set the x2APIC bit in the processor feature flags, so get caught by the second patch. Is there an appropriate approach to use in these circumstances? Otherwise, would a patch that separates the APIC ID handover and future x2APIC MSR access be appropriate?

Quoted 9 lines · Daniel J Blueman

Ingo, Suresh,

Commit a35fd28256e7736cc84af8931a16224f0bfaaf6c prevents x2APIC structures being used from the ACPI MADT if the cores don't advertise the x2APIC feature. Commit c284b42abadbb22083bfde24d308899c08d44ffa prevents onlining cores with APIC ID >255 in non-x2APIC mode. Since NumaChip/NumaConnect uses x2APIC structures to describe non-x2APIC systems (AMD Opteron) with lots of APICs, we thus now can't boot all the cores.

Quoted 5 lines

We are able to set the x2APIC bit in the processor feature flags, so get caught by the second patch. Is there an appropriate approach to use in these circumstances? Otherwise, would a patch that separates the APIC ID handover and future x2APIC MSR access be appropriate?

add is_apicid_valid() in struct apic?

Move APIC ID validity check into platform APIC code, so it can be overridden when needed. For NumaChip systems, always trust MADT, as it's constructed with high APIC IDs.

Behaviour verifies on standard x86 systems and on NumaChip systems with this, and compile-tested with allyesconfig.

Signed-off-by Daniel J Blueman <[email protected]>

Reviewed-by Steffen Persvold <[email protected]>

---
 arch/x86/include/asm/apic.h           |    6 ++++++ arch/x86/kernel/apic/apic_flat_64.c   |    2 ++ arch/x86/kernel/apic/apic_noop.c      |    1 + arch/x86/kernel/apic/apic_numachip.c  |   10 +++++++++-

 arch/x86/kernel/apic/bigsmp_32.c      |    1 +
 arch/x86/kernel/apic/es7000_32.c      |    2 ++
 arch/x86/kernel/apic/numaq_32.c       |    1 +
 arch/x86/kernel/apic/probe_32.c       |    1 +
 arch/x86/kernel/apic/summit_32.c      |    1 +

 arch/x86/kernel/apic/x2apic_cluster.c |    1 +
 arch/x86/kernel/apic/x2apic_phys.c    |    1 +
 arch/x86/kernel/apic/x2apic_uv_x.c    |    1 +
 arch/x86/kernel/smpboot.c             |    2 +-

 13 files changed, 28 insertions(+), 2 deletions(-)

arch/x86/include/asm/apic.h +6 -0

index 3ab9bdd..a9371c9 100644
@@ -288,6 +288,7 @@ struct apic {
 
        int (*probe)(void);
        int (*acpi_madt_oem_check)(char *oem_id, char *oem_table_id);
+       int (*apic_id_valid)(int apicid);
        int (*apic_id_registered)(void);
 
        u32 irq_delivery_mode;

@@ -532,6 +533,11 @@ static inline unsigned int read_apic_id(void)
        return apic->get_apic_id(reg);
  }
 
+static inline int default_apic_id_valid(int apicid)
+{
+       return x2apic_mode || (apicid < 255);
+}
+
  extern void default_setup_apic_routing(void);
 
  extern struct apic apic_noop;

arch/x86/kernel/apic/apic_flat_64.c +2 -0

index 8c3cdde..359b689 100644
@@ -180,6 +180,7 @@ static struct apic apic_flat =  {
        .name                           = "flat",
        .probe                          = flat_probe,
        .acpi_madt_oem_check            = flat_acpi_madt_oem_check,
+       .apic_id_valid                  = default_apic_id_valid,
        .apic_id_registered             = flat_apic_id_registered,
 
        .irq_delivery_mode              = dest_LowestPrio,

@@ -337,6 +338,7 @@ static struct apic apic_physflat =  {
        .name                           = "physical flat",
        .probe                          = physflat_probe,
        .acpi_madt_oem_check            = physflat_acpi_madt_oem_check,
+       .apic_id_valid                  = default_apic_id_valid,
        .apic_id_registered             = flat_apic_id_registered,
 
        .irq_delivery_mode              = dest_Fixed,

arch/x86/kernel/apic/apic_noop.c +1 -0

index 775b82b..634ae6c 100644
@@ -124,6 +124,7 @@ struct apic apic_noop = {
        .probe                          = noop_probe,
        .acpi_madt_oem_check            = NULL,
 
+       .apic_id_valid                  = default_apic_id_valid,
        .apic_id_registered             = noop_apic_id_registered,
 
        .irq_delivery_mode              = dest_LowestPrio,

arch/x86/kernel/apic/apic_numachip.c +6 -0

index 09d3d8c..d9ea5f3 100644
@@ -56,6 +56,12 @@ static unsigned int read_xapic_id(void)
        return get_apic_id(apic_read(APIC_ID));
  }
 
+static int numachip_apic_id_valid(int apicid)
+{
+       /* Trust what bootloader passes in MADT */
+       return 1;
+}
+
  static int numachip_apic_id_registered(void)
  {
        return physid_isset(read_xapic_id(), phys_cpu_present_map);

@@ -223,10 +229,11 @@ static int __init numachip_system_init(void)
  }

On Wed, Mar 14, 2012 at 12:17 AM, Daniel J Blueman

can you check if you can update

!cpu_has_x2apic&& (apic_id>= 0xff)&& enabled

in arch/x86/kernel/acpi/boot.c::acpi_parse_x2apic()

to use some kind of apic_id_valid()

so you could avoid setting that feature bit.

the checking in SRAT could be removed.

which, as you can see, checks against "x2apic_mode".

My question is; Is checking for "x2apic_mode" going to do the trick in the arch/x86/kernel/acpi/boot.c::acpi_parse_x2apic() ?

If the answer is yes, the patch is going to be very simple. But we can't verify that the code in arch/x86/kernel/acpi/boot.c::acpi_parse_x2apic() actually triggers for the case you wanted it to trigger for because then it will check against "x2apic_mode" and not "cpu_has_x2apic".

Use x2apic_supported() in the default_apic_id_valid() function. If x2apic mode is disabled (via nox2apic for example), x2apic_supported() will return false.

This allows us to substitute the check in arch/x86/kernel/acpi/boot.c::acpi_parse_x2apic and avoid feigning the x2apic cpu feature in the NumaChip apic code.

Signed-off-by Steffen Persvold <[email protected]>

Reviewed-by Daniel J Blueman <[email protected]>

Quoted 4 lines · Steffen Persvold

Use x2apic_supported() in the default_apic_id_valid() function. If x2apic mode is disabled (via nox2apic for example), x2apic_supported() will return false.

This allows us to substitute the check in arch/x86/kernel/acpi/boot.c::acpi_parse_x2apic and avoid feigning the x2apic cpu feature in the NumaChip apic code.

Quoted 2 lines

Signed-off-by Steffen Persvold <[email protected]>

Reviewed-by Daniel J Blueman <[email protected]>

I double checked on system with x2apic preenabled, nox2apic in boot command line still works well and it skips starting APs with apic id > 255.

Acked-by Yinghai Lu <[email protected]>

Quoted 5 lines · Yinghai Lu

Quoted 4 lines · Steffen Persvold

Use x2apic_supported() in the default_apic_id_valid() function. If x2apic mode is disabled (via nox2apic for example), x2apic_supported() will return false.

This allows us to substitute the check in arch/x86/kernel/acpi/boot.c::acpi_parse_x2apic and avoid feigning the x2apic cpu feature in the NumaChip apic code.

Quoted 8 lines

Quoted 2 lines

Signed-off-by Steffen Persvold<[email protected]>

Reviewed-by Daniel J Blueman<[email protected]>

I double checked on system with x2apic preenabled, nox2apic in boot command line still works well and it skips starting APs with apic id> 255.

Acked-by Yinghai Lu<[email protected]>

This breaks the smpboot check if enabling interrupt-remapping/x2apic fails on a platform. We will be in xapic mode and we don't clear the x2apic cpufeature bit in this case and as such smpboot check will fail.

So this change breaks the commit
c284b42abadbb22083bfde24d308899c08d44ffa.

I think the right thing is to have two different apid_id_valid checks one for xapic driver (apic_flat_64.c) and another for x2apic driver (x2apic_phys/cluster.c) and that way, x2apic MADT entries will be parsed only if bios has handed over the OS in x2apic mode or if we have selected the numachip model.

Quoted 11 lines · Suresh Siddha

Quoted 10 lines · Yinghai Lu

Quoted 8 lines · Steffen Persvold

Use x2apic_supported() in the default_apic_id_valid() function. If x2apic mode is disabled (via nox2apic for example), x2apic_supported() will return false.

This allows us to substitute the check in arch/x86/kernel/acpi/boot.c::acpi_parse_x2apic and avoid feigning the x2apic cpu feature in the NumaChip apic code.

Quoted 9 lines

Quoted 8 lines

Quoted 2 lines

Signed-off-by Steffen Persvold<[email protected]>

Reviewed-by Daniel J Blueman<[email protected]>

I double checked on system with x2apic preenabled, nox2apic in boot command line still works well and it skips starting APs with apic id> 255.

Acked-by Yinghai Lu<[email protected]>

Suresh,

Quoted 7 lines

This breaks the smpboot check if enabling interrupt-remapping/x2apic fails on a platform. We will be in xapic mode and we don't clear the x2apic cpufeature bit in this case and as such smpboot check will fail.

So this change breaks the commit
c284b42abadbb22083bfde24d308899c08d44ffa.

I was afraid of that.

Quoted 6 lines

I think the right thing is to have two different apid_id_valid checks one for xapic driver (apic_flat_64.c) and another for x2apic driver (x2apic_phys/cluster.c) and that way, x2apic MADT entries will be parsed only if bios has handed over the OS in x2apic mode or if we have selected the numachip model.

Is my understanding of your suggestion correct that in x2apic_phys/cluster.c we add the following apic_id_valid() function :

static int x2apic_apic_id_valid(int apicid)
{
return x2apic_mode || (apicid < 255);
}

?

Considering that this function (apic->apic_id_valid()) is called already in the acpi/boot.c::acpi_parse_x2apic() function is it sufficient enough to test for x2apic_mode ? Yinghai indicated that x2apic_mode was not set at this point, thus it was testing cpu_has_x2apic instead ?

I must admit that I am not familiar enough with the APIC/ACPI code base to determine the sequence of events here (i.e MADT parsing, enabling of x2apic mode etc. etc.).

Is my understanding of your suggestion correct that in x2apic_phys/cluster.c we add the following apic_id_valid() function :

static int x2apic_apic_id_valid(int apicid)
{
return x2apic_mode || (apicid < 255);
}

Considering that this function (apic->apic_id_valid()) is called already in the acpi/boot.c::acpi_parse_x2apic() function is it sufficient enough to test for x2apic_mode ? Yinghai indicated that x2apic_mode was not set at this point, thus it was testing cpu_has_x2apic instead ?

Quoted 7 lines · Steffen Persvold

Is my understanding of your suggestion correct that in x2apic_phys/cluster.c we add the following apic_id_valid() function :

static int x2apic_apic_id_valid(int apicid)

{
   	return x2apic_mode || (apicid<  255);
}

Steffen, We can have something like:

static int x2apic_apic_id_valid(int apicid)

{
    	return 1;
}

and

static int xapic_apic_id_valid(int apicid)

{
    	return apicid<  255;
}

If we have selected x2apic driver, then we know we are already in x2apic mode. And also x2apic_uv_x need to use the x2apic version above.

Quoted 4 lines

Considering that this function (apic->apic_id_valid()) is called already in the acpi/boot.c::acpi_parse_x2apic() function is it sufficient enough to test for x2apic_mode ? Yinghai indicated that x2apic_mode was not set at this point, thus it was testing cpu_has_x2apic instead ?

If the bios has handed over to us in x2apic mode (or if it is a numachip platform), then by this point apic driver is already set to the corresponding x2apic/numachip driver etc. so we should be fine.

When we are in xapic mode, typically there should be no x2apic MADT entries. And even if there are any (bios not following x2apic spec), the above xapic_apic_id_valid() check will consider only those x2apic MADT entries whose id's are less than 255. xapic mode can go into x2apic mode later but that flow is not supposed to bring up any cpu with apic id>
255. So parsing only entries with apic id< 255 here should be fine.

Hope this clarifies.

Quoted 7 lines · Steffen Persvold

Is my understanding of your suggestion correct that in x2apic_phys/cluster.c we add the following apic_id_valid() function :

static int x2apic_apic_id_valid(int apicid)

{
   	return x2apic_mode || (apicid<  255);
}

Steffen, We can have something like:

static int x2apic_apic_id_valid(int apicid)

{
    	return 1;
}

and

static int xapic_apic_id_valid(int apicid)

{
    	return apicid<  255;
}

If we have selected x2apic driver, then we know we are already in x2apic mode. And also x2apic_uv_x need to use the x2apic version above.

Quoted 23 lines · Suresh Siddha

Quoted 7 lines · Steffen Persvold

Is my understanding of your suggestion correct that in x2apic_phys/cluster.c we add the following apic_id_valid() function :

static int x2apic_apic_id_valid(int apicid)
{
return x2apic_mode || (apicid< 255);
}

Steffen, We can have something like:

static int x2apic_apic_id_valid(int apicid)
{
return 1;
}

and

static int xapic_apic_id_valid(int apicid)
{
return apicid< 255;
}

Quoted 3 lines

If we have selected x2apic driver, then we know we are already in x2apic mode. And also x2apic_uv_x need to use the x2apic version above.

If you specify "nox2apic" option, will it choose the xapic driver instead (and early enough) ? Otherwise I think we might break Yinghai's commit (a35fd28256e7736cc84af8931a16224f0bfaaf6c).

arch/x86/mm/srat.c +1 -4

-		printk(KERN_INFO "SRAT: PXM %u -> X2APIC 0x%04x ignored\n",
-			 pxm, apic_id);
-		return;
-	}
 	node = setup_node(pxm);
 	if (node < 0) {
 		printk(KERN_ERR "SRAT: Too many proximity domains %x\n", pxm);

@@ -82,6 +76,7 @@ acpi_numa_x2apic_affinity_init(struct acpi_srat_x2apic_cpu_affinity *pa)
 		return;
 	}
 
+	apic_id = pa->apic_id;
 	if (apic_id >= MAX_LOCAL_APIC) {
 		printk(KERN_INFO "SRAT: PXM %u -> APIC 0x%04x -> Node %u skipped apicid that is too big\n", pxm, apic_id, node);
 		return;

arch/x86/mm/srat.c acpi_numa_x2apic_affinity_init(struct acpi_srat_x2apic_cpu_affinity *pa) · line 73

Quoted 10 lines · Steffen Persvold

arch/x86/mm/srat.c +0 -2

index 1c1c4f4..7efd0c6 100644
@@ -69,12 +69,6 @@ acpi_numa_x2apic_affinity_init(struct acpi_srat_x2apic_cpu_affinity *pa)
       if ((pa->flags & ACPI_SRAT_CPU_ENABLED) == 0)
               return;
       pxm = pa->proximity_domain;
-     apic_id = pa->apic_id;
-     if (!cpu_has_x2apic && (apic_id >= 0xff)) {

not sure why you removed this. Shouldn't this be replaced with apic->apic_id_valid() check?

arch/x86/mm/srat.c acpi_numa_x2apic_affinity_init(struct acpi_srat_x2apic_cpu_affinity *pa) · line 73

Quoted 10 lines · Steffen Persvold

arch/x86/mm/srat.c +0 -2

index 1c1c4f4..7efd0c6 100644
@@ -69,12 +69,6 @@ acpi_numa_x2apic_affinity_init(struct acpi_srat_x2apic_cpu_affinity *pa)
  	if ((pa->flags&  ACPI_SRAT_CPU_ENABLED) == 0)
  		return;
  	pxm = pa->proximity_domain;
-	apic_id = pa->apic_id;
-	if (!cpu_has_x2apic&&  (apic_id>= 0xff)) {

not sure why you removed this. Shouldn't this be replaced with apic->apic_id_valid() check?

Quoted 3 lines

Quoted 3 lines

the checking in SRAT could be removed.

Yinghai

Quoted 12 lines · Yinghai Lu

Quoted 6 lines · Steffen Persvold

Use x2apic_supported() in the default_apic_id_valid() function. If x2apic mode is disabled (via nox2apic for example), x2apic_supported() will return false.

This allows us to substitute the check in arch/x86/kernel/acpi/boot.c::acpi_parse_x2apic and avoid feigning the x2apic cpu feature in the NumaChip apic code.

Signed-off-by Steffen Persvold <[email protected]>

Reviewed-by Daniel J Blueman <[email protected]>

I double checked on system with x2apic preenabled, nox2apic in boot command line still works well and it skips starting APs with apic id > 255.

Quoted 1 line

Acked-by Yinghai Lu <[email protected]>

This breaks the smpboot check if enabling interrupt-remapping/x2apic fails on a platform. We will be in xapic mode and we don't clear the x2apic cpufeature bit in this case and as such smpboot check will fail.

So this change breaks the commit
c284b42abadbb22083bfde24d308899c08d44ffa.

I think the right thing is to have two different apid_id_valid checks one for xapic driver (apic_flat_64.c) and another for x2apic driver (x2apic_phys/cluster.c) and that way, x2apic MADT entries will be parsed only if bios has handed over the OS in x2apic mode or if we have selected the numachip model.

Quoted 13 lines · Suresh Siddha

Quoted 12 lines · Yinghai Lu

Quoted 6 lines · Steffen Persvold

Use x2apic_supported() in the default_apic_id_valid() function. If x2apic mode is disabled (via nox2apic for example), x2apic_supported() will return false.

This allows us to substitute the check in arch/x86/kernel/acpi/boot.c::acpi_parse_x2apic and avoid feigning the x2apic cpu feature in the NumaChip apic code.

Signed-off-by Steffen Persvold <[email protected]>

Reviewed-by Daniel J Blueman <[email protected]>

I double checked on system with x2apic preenabled, nox2apic in boot command line still works well and it skips starting APs with apic id > 255.

Quoted 5 lines

Quoted 1 line

Acked-by Yinghai Lu <[email protected]>

This breaks the smpboot check if enabling interrupt-remapping/x2apic fails on a platform. We will be in xapic mode and we don't clear the x2apic cpufeature bit in this case and as such smpboot check will fail.

two cases:
A: x2apic is pre-enabled, and some apic id > 255
B: x2apic is not pre-enabled, but x2apic capable, suppose all apic id < 255.

in case A: if nox2apic is passed, or dmar table is not there or intr-remap can not be enabled.
  that feature bit will be cleared. So check cpu_has_x2apic is enough there.

in case B: if can not enable x2apic, the feature is not cleared, but apic id already < 255.
  so that check will pass too.

So it should not break the smpboot check.

Quoted 3 lines

So this change breaks the commit
c284b42abadbb22083bfde24d308899c08d44ffa.

Quoted 8 lines

So this change breaks the commit
c284b42abadbb22083bfde24d308899c08d44ffa.

I think the right thing is to have two different apid_id_valid checks one for xapic driver (apic_flat_64.c) and another for x2apic driver (x2apic_phys/cluster.c) and that way, x2apic MADT entries will be parsed only if bios has handed over the OS in x2apic mode or if we have selected the numachip model.

that looks like more clear.

Quoted 10 lines · Yinghai Lu

Quoted 8 lines

So this change breaks the commit
c284b42abadbb22083bfde24d308899c08d44ffa.

I think the right thing is to have two different apid_id_valid checks one for xapic driver (apic_flat_64.c) and another for x2apic driver (x2apic_phys/cluster.c) and that way, x2apic MADT entries will be parsed only if bios has handed over the OS in x2apic mode or if we have selected the numachip model.

that looks like more clear.

after more thinking, I think We should still use cpu_has_x2apic checking.

one maybe invalid case:

System have some cpus apic id< 255, and some cpu apic id> 255. BSP apic id< 255.
those cpus apic id< 255 will be put into xapic mode, cpus> 255 will be put into x2apic mode.
and DMAR table intr-remapping will be working.

So if we check x2apic_mode early, will skip cpu with apic id> 255, even switch to x2apic later.

Which one of the two patches I sent, do you (Suresh/Yinghai/others) believe is the best/cleanest and works in all cases. I, unfortunately, can't test the Intel case as I don't have any available to test on :/

Either patch works fine for NumaChip enabled systems.

If desired I will re-post the patch with the approach you find best, but add the apic->apic_id_valid() check in the SRAT code aswell.

Quoted 8 lines · Steffen Persvold

Which one of the two patches I sent, do you (Suresh/Yinghai/others) believe is the best/cleanest and works in all cases. I, unfortunately, can't test the Intel case as I don't have any available to test on :/

Either patch works fine for NumaChip enabled systems.

If desired I will re-post the patch with the approach you find best, but add the apic->apic_id_valid() check in the SRAT code aswell.

Ok, let use second way that Suresh proposed, please repost that without removing checking in srat.

should like:

From: Steffen Persvold <[email protected]>
Subject: [PATCH] Added separate apic_id_valid() functions for selected apic drivers

This allows us to substitute the check in arch/x86/kernel/acpi/boot.c::acpi_parse_x2apic and avoid feigning the x2apic cpu feature in the NumaChip apic code.

The following apic drivers have separate apic_id_valid() functions which will accept x2apic type IDs :

x2apic_phys
x2apic_cluster
x2apic_uv_x
apic_numachip

Signed-off-by Steffen Persvold <[email protected]>

---
 arch/x86/include/asm/apic.h           |    2 +-
 arch/x86/include/asm/x2apic.h         |    5 +++++
 arch/x86/kernel/acpi/boot.c           |    2 +-
 arch/x86/kernel/apic/apic_numachip.c  |    3 +--
 arch/x86/kernel/apic/x2apic_cluster.c |    2 +-
 arch/x86/kernel/apic/x2apic_phys.c    |    2 +-
 arch/x86/kernel/apic/x2apic_uv_x.c    |    7 ++++++-
 arch/x86/mm/srat.c                    |    2 +-
 8 files changed, 17 insertions(+), 8 deletions(-)

Index: linux-2.6/arch/x86/include/asm/apic.h
===================================================================

arch/x86/include/asm/apic.h +1 -1

@@ -535,7 +535,7 @@ static inline unsigned int read_apic_id(
 
 static inline int default_apic_id_valid(int apicid)
 {
-	return x2apic_mode || (apicid < 255);
+	return (apicid < 255);
 }
 
 extern void default_setup_apic_routing(void);

Index: linux-2.6/arch/x86/include/asm/x2apic.h
===================================================================

arch/x86/include/asm/x2apic.h +5 -0

@@ -18,6 +18,11 @@ static const struct cpumask *x2apic_targ
 	return cpu_online_mask;
 }
 
+static int x2apic_apic_id_valid(int apicid)
+{
+	return 1;
+}
+
 static int x2apic_apic_id_registered(void)
 {
 	return 1;

Index: linux-2.6/arch/x86/kernel/acpi/boot.c
===================================================================

arch/x86/kernel/acpi/boot.c +1 -1

@@ -227,7 +227,7 @@ acpi_parse_x2apic(struct acpi_subtable_h
 	 * to not preallocating memory for all NR_CPUS
 	 * when we use CPU hotplug.
 	 */
-	if (!cpu_has_x2apic && (apic_id >= 0xff) && enabled)
+	if (!apic->apic_id_valid(apic_id) && enabled)
 		printk(KERN_WARNING PREFIX "x2apic entry ignored\n");
 	else
 		acpi_register_lapic(apic_id, enabled);

Index: linux-2.6/arch/x86/kernel/apic/apic_numachip.c
===================================================================

arch/x86/kernel/apic/apic_numachip.c +1 -2

@@ -229,11 +229,10 @@ static int __init numachip_system_init(v
 }
 early_initcall(numachip_system_init);
 
-static int __cpuinit numachip_acpi_madt_oem_check(char *oem_id, char
 *oem_table_id)
+static int numachip_acpi_madt_oem_check(char *oem_id, char *oem_table_id)
 {
 	if (!strncmp(oem_id, "NUMASC", 6)) {
 		numachip_system = 1;
-		setup_force_cpu_cap(X86_FEATURE_X2APIC);
 		return 1;
 	}

Index: linux-2.6/arch/x86/kernel/apic/x2apic_cluster.c
===================================================================

arch/x86/kernel/apic/x2apic_cluster.c +1 -1

@@ -218,7 +218,7 @@ static struct apic apic_x2apic_cluster =
 	.name				= "cluster x2apic",
 	.probe				= x2apic_cluster_probe,
 	.acpi_madt_oem_check		= x2apic_acpi_madt_oem_check,
-	.apic_id_valid			= default_apic_id_valid,
+	.apic_id_valid			= x2apic_apic_id_valid,
 	.apic_id_registered		= x2apic_apic_id_registered,
 
 	.irq_delivery_mode		= dest_LowestPrio,

Index: linux-2.6/arch/x86/kernel/apic/x2apic_phys.c
===================================================================

arch/x86/kernel/apic/x2apic_phys.c +1 -1

@@ -119,7 +119,7 @@ static struct apic apic_x2apic_phys = {
 	.name				= "physical x2apic",
 	.probe				= x2apic_phys_probe,
 	.acpi_madt_oem_check		= x2apic_acpi_madt_oem_check,
-	.apic_id_valid			= default_apic_id_valid,
+	.apic_id_valid			= x2apic_apic_id_valid,
 	.apic_id_registered		= x2apic_apic_id_registered,
 
 	.irq_delivery_mode		= dest_Fixed,

Index: linux-2.6/arch/x86/kernel/apic/x2apic_uv_x.c
===================================================================

arch/x86/kernel/apic/x2apic_uv_x.c +6 -1

@@ -266,6 +266,11 @@ static void uv_send_IPI_all(int vector)
 	uv_send_IPI_mask(cpu_online_mask, vector);
 }
 
+static int uv_apic_id_valid(int apicid)
+{
+	return 1;
+}
+
 static int uv_apic_id_registered(void)
 {
 	return 1;

@@ -351,7 +356,7 @@ static struct apic __refdata apic_x2apic
 	.name				= "UV large system",
 	.probe				= uv_probe,
 	.acpi_madt_oem_check		= uv_acpi_madt_oem_check,
-	.apic_id_valid			= default_apic_id_valid,
+	.apic_id_valid			= uv_apic_id_valid,
 	.apic_id_registered		= uv_apic_id_registered,
 
 	.irq_delivery_mode		= dest_Fixed,

Index: linux-2.6/arch/x86/mm/srat.c
===================================================================

arch/x86/mm/srat.c +1 -1

@@ -70,7 +70,7 @@ acpi_numa_x2apic_affinity_init(struct ac
 		return;
 	pxm = pa->proximity_domain;
 	apic_id = pa->apic_id;
-	if (!cpu_has_x2apic && (apic_id >= 0xff)) {
+	if (!apic->apic_id_valid(apic_id)) {
 		printk(KERN_INFO "SRAT: PXM %u -> X2APIC 0x%04x ignored\n",
 			 pxm, apic_id);
 		return;

Quoted 31 lines · Yinghai Lu

Quoted 8 lines · Steffen Persvold

Which one of the two patches I sent, do you (Suresh/Yinghai/others) believe is the best/cleanest and works in all cases. I, unfortunately, can't test the Intel case as I don't have any available to test on :/

Either patch works fine for NumaChip enabled systems.

If desired I will re-post the patch with the approach you find best, but add the apic->apic_id_valid() check in the SRAT code aswell.

Ok, let use second way that Suresh proposed, please repost that without removing checking in srat.

should like:

From: Steffen Persvold <[email protected]>
Subject: [PATCH] Added separate apic_id_valid() functions for selected apic drivers

This allows us to substitute the check in arch/x86/kernel/acpi/boot.c::acpi_parse_x2apic and avoid feigning the x2apic cpu feature in the NumaChip apic code.

The following apic drivers have separate apic_id_valid() functions which will accept x2apic type IDs :

x2apic_phys
x2apic_cluster
x2apic_uv_x
apic_numachip

Quoted 1 line

Signed-off-by Steffen Persvold <[email protected]>

I have no complaints with this version ;)

Acked-by Suresh Siddha <[email protected]>

Quoted 136 lines

---
 arch/x86/include/asm/apic.h           |    2 +- arch/x86/include/asm/x2apic.h         |    5 +++++ arch/x86/kernel/acpi/boot.c           |    2 +- arch/x86/kernel/apic/apic_numachip.c  |    3 +-- arch/x86/kernel/apic/x2apic_cluster.c |    2 +- arch/x86/kernel/apic/x2apic_phys.c    |    2 +- arch/x86/kernel/apic/x2apic_uv_x.c    |    7 ++++++- arch/x86/mm/srat.c                    |    2 +-

 8 files changed, 17 insertions(+), 8 deletions(-)

Index: linux-2.6/arch/x86/include/asm/apic.h
===================================================================

arch/x86/include/asm/apic.h +1 -1

@@ -535,7 +535,7 @@ static inline unsigned int read_apic_id(
 
  static inline int default_apic_id_valid(int apicid)
  {
-     return x2apic_mode || (apicid < 255);
+     return (apicid < 255);
  }
 
  extern void default_setup_apic_routing(void);

Index: linux-2.6/arch/x86/include/asm/x2apic.h
===================================================================

arch/x86/include/asm/x2apic.h +5 -0

@@ -18,6 +18,11 @@ static const struct cpumask *x2apic_targ
       return cpu_online_mask;
  }
 
+static int x2apic_apic_id_valid(int apicid)
+{
+     return 1;
+}
+
  static int x2apic_apic_id_registered(void)
  {
       return 1;

Index: linux-2.6/arch/x86/kernel/acpi/boot.c
===================================================================

arch/x86/kernel/acpi/boot.c +1 -1

@@ -227,7 +227,7 @@ acpi_parse_x2apic(struct acpi_subtable_h
        * to not preallocating memory for all NR_CPUS
        * when we use CPU hotplug.
        */
-     if (!cpu_has_x2apic && (apic_id >= 0xff) && enabled)
+     if (!apic->apic_id_valid(apic_id) && enabled)
               printk(KERN_WARNING PREFIX "x2apic entry ignored\n");

      else
              acpi_register_lapic(apic_id, enabled);
Index: linux-2.6/arch/x86/kernel/apic/apic_numachip.c
===================================================================

arch/x86/kernel/apic/apic_numachip.c +1 -2

@@ -229,11 +229,10 @@ static int __init numachip_system_init(v
  }
  early_initcall(numachip_system_init);
 
-static int __cpuinit numachip_acpi_madt_oem_check(char *oem_id, char
 *oem_table_id)
+static int numachip_acpi_madt_oem_check(char *oem_id, char *oem_table_id)
  {
       if (!strncmp(oem_id, "NUMASC", 6)) {
               numachip_system = 1;
-             setup_force_cpu_cap(X86_FEATURE_X2APIC);
               return 1;
       }

Index: linux-2.6/arch/x86/kernel/apic/x2apic_cluster.c
===================================================================

arch/x86/kernel/apic/x2apic_cluster.c +1 -1

@@ -218,7 +218,7 @@ static struct apic apic_x2apic_cluster =
       .name                           = "cluster x2apic",
       .probe                          = x2apic_cluster_probe,
       .acpi_madt_oem_check            = x2apic_acpi_madt_oem_check,
-     .apic_id_valid                  = default_apic_id_valid,
+     .apic_id_valid                  = x2apic_apic_id_valid,
       .apic_id_registered             = x2apic_apic_id_registered,
 
       .irq_delivery_mode              = dest_LowestPrio,

Index: linux-2.6/arch/x86/kernel/apic/x2apic_phys.c
===================================================================

arch/x86/kernel/apic/x2apic_phys.c +1 -1

@@ -119,7 +119,7 @@ static struct apic apic_x2apic_phys = {
       .name                           = "physical x2apic",
       .probe                          = x2apic_phys_probe,
       .acpi_madt_oem_check            = x2apic_acpi_madt_oem_check,
-     .apic_id_valid                  = default_apic_id_valid,
+     .apic_id_valid                  = x2apic_apic_id_valid,
       .apic_id_registered             = x2apic_apic_id_registered,
 
       .irq_delivery_mode              = dest_Fixed,

Index: linux-2.6/arch/x86/kernel/apic/x2apic_uv_x.c
===================================================================

arch/x86/kernel/apic/x2apic_uv_x.c +6 -1

@@ -266,6 +266,11 @@ static void uv_send_IPI_all(int vector)
       uv_send_IPI_mask(cpu_online_mask, vector);
  }
 
+static int uv_apic_id_valid(int apicid)
+{
+     return 1;
+}
+
  static int uv_apic_id_registered(void)
  {
       return 1;

@@ -351,7 +356,7 @@ static struct apic __refdata apic_x2apic
       .name                           = "UV large system",
       .probe                          = uv_probe,
       .acpi_madt_oem_check            = uv_acpi_madt_oem_check,
-     .apic_id_valid                  = default_apic_id_valid,
+     .apic_id_valid                  = uv_apic_id_valid,
       .apic_id_registered             = uv_apic_id_registered,
 
       .irq_delivery_mode              = dest_Fixed,

Index: linux-2.6/arch/x86/mm/srat.c
===================================================================

arch/x86/mm/srat.c +1 -1

@@ -70,7 +70,7 @@ acpi_numa_x2apic_affinity_init(struct ac
               return;
       pxm = pa->proximity_domain;
       apic_id = pa->apic_id;
-     if (!cpu_has_x2apic && (apic_id >= 0xff)) {
+     if (!apic->apic_id_valid(apic_id)) {
               printk(KERN_INFO "SRAT: PXM %u -> X2APIC 0x%04x ignored\n",
                        pxm, apic_id);
               return;

As suggested by Suresh Siddha<[email protected]> and Yinghai Lu<[email protected]> :

For x2apic pre-enabled systems, apic driver is set already early through early_acpi_boot_init()/early_acpi_process_madt()/acpi_parse_madt()/default_acpi_madt_oem_check() so that apic_id_valid() checking will be sufficient during MADT and SRAT prasing. For non- x2apic pre-enabled systems, all apic ids should be less than 255.

This allows us to substitute the checks in arch/x86/kernel/acpi/boot.c::acpi_parse_x2apic() and arch/x86/mm/srat.c::acpi_numa_x2apic_affinity_init() with apic->apic_id_valid().

In addition we can avoid feigning the x2apic cpu feature in the NumaChip apic code.

The following apic drivers have separate apic_id_valid() functions which will accept x2apic type IDs :

x2apic_phys
x2apic_cluster
x2apic_uv_x
apic_numachip

Signed-off-by Steffen Persvold<[email protected]>

---
  arch/x86/include/asm/apic.h           |    2 +-
  arch/x86/include/asm/x2apic.h         |    5 +++++
  arch/x86/kernel/acpi/boot.c           |    2 +-
  arch/x86/kernel/apic/apic_numachip.c  |    3 +--
  arch/x86/kernel/apic/x2apic_cluster.c |    2 +-
  arch/x86/kernel/apic/x2apic_phys.c    |    2 +-
  arch/x86/kernel/apic/x2apic_uv_x.c    |    7 ++++++-
  arch/x86/mm/srat.c                    |    2 +-
  8 files changed, 17 insertions(+), 8 deletions(-)

arch/x86/include/asm/apic.h +1 -1

index a9371c9..d3eaac4 100644
@@ -535,7 +535,7 @@ static inline unsigned int read_apic_id(void)
 
  static inline int default_apic_id_valid(int apicid)
  {
-	return x2apic_mode || (apicid<  255);
+	return (apicid<  255);
  }
 
  extern void default_setup_apic_routing(void);

arch/x86/include/asm/x2apic.h +5 -0

index 6bf5b8e..92e54ab 100644
@@ -18,6 +18,11 @@ static const struct cpumask *x2apic_target_cpus(void)
  	return cpu_online_mask;
  }
 
+static int x2apic_apic_id_valid(int apicid)
+{
+	return 1;
+}
+
  static int x2apic_apic_id_registered(void)
  {
  	return 1;

arch/x86/kernel/acpi/boot.c +1 -1

index ce664f3..4b5059c 100644
@@ -239,7 +239,7 @@ acpi_parse_x2apic(struct acpi_subtable_header *header, const unsigned long end)
  	 * to not preallocating memory for all NR_CPUS
  	 * when we use CPU hotplug.
  	 */
-	if (!cpu_has_x2apic&&  (apic_id>= 0xff)&&  enabled)
+	if (!apic->apic_id_valid(apic_id)&&  enabled)
  		printk(KERN_WARNING PREFIX "x2apic entry ignored\n");
  	else
  		acpi_register_lapic(apic_id, enabled);

arch/x86/kernel/apic/apic_numachip.c +1 -2

index d9ea5f3..899803e 100644
@@ -229,11 +229,10 @@ static int __init numachip_system_init(void)
  }
  early_initcall(numachip_system_init);
 
-static int __cpuinit numachip_acpi_madt_oem_check(char *oem_id, char *oem_table_id)
+static int numachip_acpi_madt_oem_check(char *oem_id, char *oem_table_id)
  {
  	if (!strncmp(oem_id, "NUMASC", 6)) {
  		numachip_system = 1;
-		setup_force_cpu_cap(X86_FEATURE_X2APIC);
  		return 1;
  	}
 

arch/x86/kernel/apic/x2apic_cluster.c +1 -1

index 9193713..48f3103 100644
@@ -213,7 +213,7 @@ static struct apic apic_x2apic_cluster = {
  	.name				= "cluster x2apic",
  	.probe				= x2apic_cluster_probe,
  	.acpi_madt_oem_check		= x2apic_acpi_madt_oem_check,
-	.apic_id_valid			= default_apic_id_valid,
+	.apic_id_valid			= x2apic_apic_id_valid,
  	.apic_id_registered		= x2apic_apic_id_registered,
 
  	.irq_delivery_mode		= dest_LowestPrio,

arch/x86/kernel/apic/x2apic_phys.c +1 -1

index bcd1db6..8a778db 100644
@@ -119,7 +119,7 @@ static struct apic apic_x2apic_phys = {
  	.name				= "physical x2apic",
  	.probe				= x2apic_phys_probe,
  	.acpi_madt_oem_check		= x2apic_acpi_madt_oem_check,
-	.apic_id_valid			= default_apic_id_valid,
+	.apic_id_valid			= x2apic_apic_id_valid,
  	.apic_id_registered		= x2apic_apic_id_registered,
 
  	.irq_delivery_mode		= dest_Fixed,

arch/x86/kernel/apic/x2apic_uv_x.c +6 -1

index fc47714..87bfa69 100644
@@ -266,6 +266,11 @@ static void uv_send_IPI_all(int vector)
  	uv_send_IPI_mask(cpu_online_mask, vector);
  }
 
+static int uv_apic_id_valid(int apicid)
+{
+	return 1;
+}
+
  static int uv_apic_id_registered(void)
  {
  	return 1;

@@ -351,7 +356,7 @@ static struct apic __refdata apic_x2apic_uv_x = {
  	.name				= "UV large system",
  	.probe				= uv_probe,
  	.acpi_madt_oem_check		= uv_acpi_madt_oem_check,
-	.apic_id_valid			= default_apic_id_valid,
+	.apic_id_valid			= uv_apic_id_valid,
  	.apic_id_registered		= uv_apic_id_registered,
 
  	.irq_delivery_mode		= dest_Fixed,

arch/x86/mm/srat.c +1 -1

index 1c1c4f4..efb5b4b 100644
@@ -70,7 +70,7 @@ acpi_numa_x2apic_affinity_init(struct acpi_srat_x2apic_cpu_affinity *pa)
  		return;
  	pxm = pa->proximity_domain;
  	apic_id = pa->apic_id;
-	if (!cpu_has_x2apic&&  (apic_id>= 0xff)) {
+	if (!apic->apic_id_valid(apic_id)) {
  		printk(KERN_INFO "SRAT: PXM %u ->  X2APIC 0x%04x ignored\n",
  			 pxm, apic_id);
  		return;

Ingo, any comments on this approach ? It should cover both cases as suggested by Suresh and Yinghai.

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 <[email protected]>

---
 arch/x86/kernel/apic/io_apic.c |    4 ++++

 1 files changed, 4 insertions(+), 0 deletions(-)

arch/x86/kernel/apic/io_apic.c +0 -0

index 7412eb8..622374f 100644

Quoted 13 lines · 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<[email protected]>

---
  arch/x86/kernel/apic/io_apic.c |    4 ++++
  1 files changed, 4 insertions(+), 0 deletions(-)

arch/x86/kernel/apic/io_apic.c +0 -0

index 7412eb8..622374f 100644

arch/x86/kernel/apic/io_apic.c int __init arch_early_irq_init(void) · line 400

Quoted 9 lines

arch/x86/kernel/apic/io_apic.c +4 -0

@@ -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

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

Quoted 30 lines · Yinghai Lu

Quoted 13 lines · 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<[email protected]>

---
  arch/x86/kernel/apic/io_apic.c |    4 ++++
  1 files changed, 4 insertions(+), 0 deletions(-)

arch/x86/kernel/apic/io_apic.c +0 -0

index 7412eb8..622374f 100644

arch/x86/kernel/apic/io_apic.c int __init arch_early_irq_init(void) · line 400

Quoted 9 lines

arch/x86/kernel/apic/io_apic.c +4 -0

@@ -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

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

Signed-off-by Jiang Liu <[email protected]>

---
 arch/x86/kernel/apic/io_apic.c |   69 ++++++++++++++++++++++------------------

 1 files changed, 38 insertions(+), 31 deletions(-)

arch/x86/kernel/apic/io_apic.c +18 -19

index e5b6ca4..dfb6f45 100644
@@ -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);

+       }

arch/x86/kernel/apic/io_apic.c +17 -0

+
+       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);

       }

Quoted 89 lines · Jiang Liu

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

Signed-off-by Jiang Liu<[email protected]>

---
  arch/x86/kernel/apic/io_apic.c |   69 ++++++++++++++++++++++------------------
  1 files changed, 38 insertions(+), 31 deletions(-)

arch/x86/kernel/apic/io_apic.c +18 -27

index e5b6ca4..dfb6f45 100644
@@ -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++;

arch/x86/kernel/apic/io_apic.c static struct resource * __init ioapic_setup_resources(int nr_ioapics) · line 4045

Quoted 6 lines

arch/x86/kernel/apic/io_apic.c +2 -4

-
-               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?

Quoted 10 lines

arch/x86/kernel/apic/io_apic.c +9 -0

+       }
+
+       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;

arch/x86/kernel/apic/io_apic.c static struct resource * __init ioapic_setup_resources(int nr_ioapics) · line 4064

Quoted 10 lines

arch/x86/kernel/apic/io_apic.c +9 -0

+}
+
+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

.