[PATCH] x86: enable x2apic early at the first point

[PATCH] x86: enable x2apic early at the first point

[Yinghai Lu]

Impact: fix bug.

otherwise will get panic from early_acpi_boot_init()
also make disable_x2apic global, so could use it it x2_apic_xxx.c
and can get warning if preenabled system using nox2apic.

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

---
 arch/x86/kernel/apic/apic.c           |    3 +--
 arch/x86/kernel/apic/x2apic_cluster.c |    5 ++++-
 arch/x86/kernel/apic/x2apic_phys.c    |    5 ++++-
 arch/x86/kernel/apic/x2apic_uv_x.c    |    4 +++-
 drivers/pci/dmar.c                    |    3 ++-
 5 files changed, 14 insertions(+), 6 deletions(-)

Index: linux-2.6/arch/x86/kernel/apic/x2apic_cluster.c
===================================================================
--- linux-2.6.orig/arch/x86/kernel/apic/x2apic_cluster.c
+++ linux-2.6/arch/x86/kernel/apic/x2apic_cluster.c
@@ -14,8 +14,11 @@ DEFINE_PER_CPU(u32, x86_cpu_to_logical_a
 
 static int x2apic_acpi_madt_oem_check(char *oem_id, char *oem_table_id)
 {
-	if (cpu_has_x2apic)
+	if (cpu_has_x2apic && !disable_x2apic) {
+		x2apic = 1;
+		enable_x2apic();
 		return 1;
+	}
 
 	return 0;
 }
Index: linux-2.6/arch/x86/kernel/apic/x2apic_phys.c
===================================================================
--- linux-2.6.orig/arch/x86/kernel/apic/x2apic_phys.c
+++ linux-2.6/arch/x86/kernel/apic/x2apic_phys.c
@@ -21,8 +21,11 @@ early_param("x2apic_phys", set_x2apic_ph
 
 static int x2apic_acpi_madt_oem_check(char *oem_id, char *oem_table_id)
 {
-	if (cpu_has_x2apic && x2apic_phys)
+	if (cpu_has_x2apic && !disable_x2apic && x2apic_phys) {
+		x2apic = 1;
+		enable_x2apic();
 		return 1;
+	}
 
 	return 0;
 }
Index: linux-2.6/arch/x86/kernel/apic/x2apic_uv_x.c
===================================================================
--- linux-2.6.orig/arch/x86/kernel/apic/x2apic_uv_x.c
+++ linux-2.6/arch/x86/kernel/apic/x2apic_uv_x.c
@@ -41,8 +41,10 @@ static int uv_acpi_madt_oem_check(char *
 			uv_system_type = UV_LEGACY_APIC;
 		else if (!strcmp(oem_table_id, "UVX"))
 			uv_system_type = UV_X2APIC;
-		else if (!strcmp(oem_table_id, "UVH")) {
+		else if (!strcmp(oem_table_id, "UVH") && !disable_x2apic) {
 			uv_system_type = UV_NON_UNIQUE_APIC;
+			x2apic = 1;
+			enable_x2apic();
 			return 1;
 		}
 	}
Index: linux-2.6/arch/x86/kernel/apic/apic.c
===================================================================
--- linux-2.6.orig/arch/x86/kernel/apic/apic.c
+++ linux-2.6/arch/x86/kernel/apic/apic.c
@@ -116,11 +116,10 @@ __setup("apicpmtimer", setup_apicpmtimer
 int x2apic;
 /* x2apic enabled before OS handover */
 static int x2apic_preenabled;
-static int disable_x2apic;
+int disable_x2apic;
 static __init int setup_nox2apic(char *str)
 {
 	disable_x2apic = 1;
-	setup_clear_cpu_cap(X86_FEATURE_X2APIC);
 	return 0;
 }
 early_param("nox2apic", setup_nox2apic);
Index: linux-2.6/drivers/pci/dmar.c
===================================================================
--- linux-2.6.orig/drivers/pci/dmar.c
+++ linux-2.6/drivers/pci/dmar.c
@@ -472,7 +472,8 @@ void __init detect_intel_iommu(void)
 		 * is added, we will not need this any more.
 		 */
 		dmar = (struct acpi_table_dmar *) dmar_tbl;
-		if (ret && cpu_has_x2apic && dmar->flags & 0x1)
+		if (ret && cpu_has_x2apic && !disable_x2apic &&
+			 dmar->flags & 0x1)
 			printk(KERN_INFO
 			       "Queued invalidation will be enabled to support "
 			       "x2apic and Intr-remapping.\n");

Re: [PATCH] x86: enable x2apic early at the first point

[Suresh Siddha]

On Thu, 2009-02-19 at 13:50 -0800, Yinghai Lu wrote:
> Impact: fix bug.
> 
> otherwise will get panic from early_acpi_boot_init()
> also make disable_x2apic global, so could use it it x2_apic_xxx.c
> and can get warning if preenabled system using nox2apic.
> 
> Signed-off-by: Yinghai Lu <yinghai@kernel.org>
> 
> ---
>  arch/x86/kernel/apic/apic.c           |    3 +--
>  arch/x86/kernel/apic/x2apic_cluster.c |    5 ++++-
>  arch/x86/kernel/apic/x2apic_phys.c    |    5 ++++-
>  arch/x86/kernel/apic/x2apic_uv_x.c    |    4 +++-
>  drivers/pci/dmar.c                    |    3 ++-
>  5 files changed, 14 insertions(+), 6 deletions(-)
> 
> Index: linux-2.6/arch/x86/kernel/apic/x2apic_cluster.c
> ===================================================================
> --- linux-2.6.orig/arch/x86/kernel/apic/x2apic_cluster.c
> +++ linux-2.6/arch/x86/kernel/apic/x2apic_cluster.c
> @@ -14,8 +14,11 @@ DEFINE_PER_CPU(u32, x86_cpu_to_logical_a
> 
>  static int x2apic_acpi_madt_oem_check(char *oem_id, char *oem_table_id)
>  {
> -       if (cpu_has_x2apic)
> +       if (cpu_has_x2apic && !disable_x2apic) {
> +               x2apic = 1;
> +               enable_x2apic();

Yinghai, I also ran into couple of x2apic issues on the latest tip and
this issue is one of them. I don't like this patch mainly because we are
enabling x2apic mode in the cpu, with out checking the intr-remapping
support. We should fall back to xapic mode (in a clean fashion) if there
is any issue with intr-remapping. So I am planning to post another patch
to fix this early hang issue that we both encountered.

thanks,
suresh

>                 return 1;
> +       }
> 
>         return 0;
>  }
> Index: linux-2.6/arch/x86/kernel/apic/x2apic_phys.c
> ===================================================================
> --- linux-2.6.orig/arch/x86/kernel/apic/x2apic_phys.c
> +++ linux-2.6/arch/x86/kernel/apic/x2apic_phys.c
> @@ -21,8 +21,11 @@ early_param("x2apic_phys", set_x2apic_ph
> 
>  static int x2apic_acpi_madt_oem_check(char *oem_id, char *oem_table_id)
>  {
> -       if (cpu_has_x2apic && x2apic_phys)
> +       if (cpu_has_x2apic && !disable_x2apic && x2apic_phys) {
> +               x2apic = 1;
> +               enable_x2apic();
>                 return 1;
> +       }
> 
>         return 0;
>  }
> Index: linux-2.6/arch/x86/kernel/apic/x2apic_uv_x.c
> ===================================================================
> --- linux-2.6.orig/arch/x86/kernel/apic/x2apic_uv_x.c
> +++ linux-2.6/arch/x86/kernel/apic/x2apic_uv_x.c
> @@ -41,8 +41,10 @@ static int uv_acpi_madt_oem_check(char *
>                         uv_system_type = UV_LEGACY_APIC;
>                 else if (!strcmp(oem_table_id, "UVX"))
>                         uv_system_type = UV_X2APIC;
> -               else if (!strcmp(oem_table_id, "UVH")) {
> +               else if (!strcmp(oem_table_id, "UVH") && !disable_x2apic) {
>                         uv_system_type = UV_NON_UNIQUE_APIC;
> +                       x2apic = 1;
> +                       enable_x2apic();
>                         return 1;
>                 }
>         }
> Index: linux-2.6/arch/x86/kernel/apic/apic.c
> ===================================================================
> --- linux-2.6.orig/arch/x86/kernel/apic/apic.c
> +++ linux-2.6/arch/x86/kernel/apic/apic.c
> @@ -116,11 +116,10 @@ __setup("apicpmtimer", setup_apicpmtimer
>  int x2apic;
>  /* x2apic enabled before OS handover */
>  static int x2apic_preenabled;
> -static int disable_x2apic;
> +int disable_x2apic;
>  static __init int setup_nox2apic(char *str)
>  {
>         disable_x2apic = 1;
> -       setup_clear_cpu_cap(X86_FEATURE_X2APIC);
>         return 0;
>  }
>  early_param("nox2apic", setup_nox2apic);
> Index: linux-2.6/drivers/pci/dmar.c
> ===================================================================
> --- linux-2.6.orig/drivers/pci/dmar.c
> +++ linux-2.6/drivers/pci/dmar.c
> @@ -472,7 +472,8 @@ void __init detect_intel_iommu(void)
>                  * is added, we will not need this any more.
>                  */
>                 dmar = (struct acpi_table_dmar *) dmar_tbl;
> -               if (ret && cpu_has_x2apic && dmar->flags & 0x1)
> +               if (ret && cpu_has_x2apic && !disable_x2apic &&
> +                        dmar->flags & 0x1)
>                         printk(KERN_INFO
>                                "Queued invalidation will be enabled to support "
>                                "x2apic and Intr-remapping.\n");

Re: [PATCH] x86: enable x2apic early at the first point

[Yinghai Lu]

Suresh Siddha wrote:
> On Thu, 2009-02-19 at 13:50 -0800, Yinghai Lu wrote:
>> Impact: fix bug.
>>
>> otherwise will get panic from early_acpi_boot_init()
>> also make disable_x2apic global, so could use it it x2_apic_xxx.c
>> and can get warning if preenabled system using nox2apic.
>>
>> Signed-off-by: Yinghai Lu <yinghai@kernel.org>
>>
>> ---
>>  arch/x86/kernel/apic/apic.c           |    3 +--
>>  arch/x86/kernel/apic/x2apic_cluster.c |    5 ++++-
>>  arch/x86/kernel/apic/x2apic_phys.c    |    5 ++++-
>>  arch/x86/kernel/apic/x2apic_uv_x.c    |    4 +++-
>>  drivers/pci/dmar.c                    |    3 ++-
>>  5 files changed, 14 insertions(+), 6 deletions(-)
>>
>> Index: linux-2.6/arch/x86/kernel/apic/x2apic_cluster.c
>> ===================================================================
>> --- linux-2.6.orig/arch/x86/kernel/apic/x2apic_cluster.c
>> +++ linux-2.6/arch/x86/kernel/apic/x2apic_cluster.c
>> @@ -14,8 +14,11 @@ DEFINE_PER_CPU(u32, x86_cpu_to_logical_a
>>
>>  static int x2apic_acpi_madt_oem_check(char *oem_id, char *oem_table_id)
>>  {
>> -       if (cpu_has_x2apic)
>> +       if (cpu_has_x2apic && !disable_x2apic) {
>> +               x2apic = 1;
>> +               enable_x2apic();
> 
> Yinghai, I also ran into couple of x2apic issues on the latest tip and
> this issue is one of them. I don't like this patch mainly because we are
> enabling x2apic mode in the cpu, with out checking the intr-remapping
> support. We should fall back to xapic mode (in a clean fashion) if there
> is any issue with intr-remapping. So I am planning to post another patch
> to fix this early hang issue that we both encountered.

Ingo want to decouple that x2apic and intr_remapping.
it seems it does work with x2apic without intr_remapping in one of setup.

YH

Re: [PATCH] x86: enable x2apic early at the first point

[Suresh Siddha]

On Thu, 2009-02-19 at 14:42 -0800, Yinghai Lu wrote:
> Ingo want to decouple that x2apic and intr_remapping.
> it seems it does work with x2apic without intr_remapping in one of setup.

x2apic with out intr-remapping is not architectural. Even when we have <
255 logical cpu's, logical x2apic id's will be greater than 16 bits even
on a single/two socket systems and this will break interrupt delivery.
Please look at the x2apic logical destination mode definition in the SDM
(Section 9.7.2.3 and 9.7.2.4 in my copy of SDM Vol3a)

While it might work in certain configurations (for example, physical
mode with < 8 bit apicids), it is not architectural and implementation
dependent, which may break in future generations.

And also, logical x2apic mode has more advantages compared to physical
mode (like using lowest priority delivery mode etc).

I will post couple of patches, which revert's Gleb's patch and another
fix for the early boot failure issue, tomorrow.

thanks,
suresh

Re: [PATCH] x86: enable x2apic early at the first point

[Ingo Molnar]

* Suresh Siddha <suresh.b.siddha@intel.com> wrote:

> On Thu, 2009-02-19 at 14:42 -0800, Yinghai Lu wrote:
> > Ingo want to decouple that x2apic and intr_remapping.
> > it seems it does work with x2apic without intr_remapping in one of setup.
> 
> x2apic with out intr-remapping is not architectural. Even when 
> we have < 255 logical cpu's, logical x2apic id's will be 
> greater than 16 bits even on a single/two socket systems and 
> this will break interrupt delivery. Please look at the x2apic 
> logical destination mode definition in the SDM (Section 
> 9.7.2.3 and 9.7.2.4 in my copy of SDM Vol3a)
> 
> While it might work in certain configurations (for example, 
> physical mode with < 8 bit apicids), it is not architectural 
> and implementation dependent, which may break in future 
> generations.
> 
> And also, logical x2apic mode has more advantages compared to 
> physical mode (like using lowest priority delivery mode etc).
> 
> I will post couple of patches, which revert's Gleb's patch and 
> another fix for the early boot failure issue, tomorrow.

Instead of the revert, we can zap this patch from tip:x86/apic:

  d13d2c3: x86, apic: decouple x2apic from interrupt remapping

But it will all need more cleanups. For example the mandatory 
coupling between x2apic and intr-remap is not clearly spelled 
out.

Also, some of the code has also become rather ugly. For example 
could you please work on eliminating this #ifdef-fest:

earth4:~/tip> grep INTR_REMAP arch/x86/kernel/apic/io_apic.c 
#ifdef CONFIG_INTR_REMAP
#ifdef CONFIG_INTR_REMAP
#ifdef CONFIG_INTR_REMAP
#ifdef CONFIG_INTR_REMAP
#ifdef CONFIG_INTR_REMAP
#ifdef CONFIG_INTR_REMAP
#ifdef CONFIG_INTR_REMAP
#ifdef CONFIG_INTR_REMAP
#ifdef CONFIG_INTR_REMAP
#ifdef CONFIG_INTR_REMAP
#ifdef CONFIG_INTR_REMAP
#ifdef CONFIG_INTR_REMAP
#ifdef CONFIG_INTR_REMAP
#ifdef CONFIG_INTR_REMAP
#ifdef CONFIG_INTR_REMAP
#ifdef CONFIG_INTR_REMAP
#ifdef CONFIG_INTR_REMAP
#ifdef CONFIG_INTR_REMAP
#ifdef CONFIG_INTR_REMAP

for example all the #ifdefs around irq_remapped() look bogus to 
me - dmar.h already defines irq_remapped() to always-0 when 
!CONFIG_INTR_REMAP.

	Ingo

Re: [PATCH] x86: enable x2apic early at the first point

[Gleb Natapov]

On Fri, Feb 20, 2009 at 09:35:28AM +0100, Ingo Molnar wrote:
> 
> * Suresh Siddha <suresh.b.siddha@intel.com> wrote:
> 
> > On Thu, 2009-02-19 at 14:42 -0800, Yinghai Lu wrote:
> > > Ingo want to decouple that x2apic and intr_remapping.
> > > it seems it does work with x2apic without intr_remapping in one of setup.
> > 
> > x2apic with out intr-remapping is not architectural. Even when 
> > we have < 255 logical cpu's, logical x2apic id's will be 
> > greater than 16 bits even on a single/two socket systems and 
> > this will break interrupt delivery. Please look at the x2apic 
> > logical destination mode definition in the SDM (Section 
> > 9.7.2.3 and 9.7.2.4 in my copy of SDM Vol3a)
> >
This is what Intel doc says:
 It is likely that processor implementations may choose to support less
 than 16 bits of the cluster ID or less than 16-bits of the Logical ID
 in the Logical Destination Register.  However system software should be
 agnostic to the number of bits implemented in the cluster ID and logical
 ID sub-fields. The x2APIC hardware initialization will ensure that the
 appropriately initialized logical x2APIC IDs are available to system
 software and reads of non-implemented bits return zero.

KVM will implement 0 bits of cluster ID and will want to use x2apic without
implementing IR.

> > While it might work in certain configurations (for example, 
> > physical mode with < 8 bit apicids), it is not architectural 
> > and implementation dependent, which may break in future 
> > generations.
> > 
It will not break in future generation of KVM though.

> > And also, logical x2apic mode has more advantages compared to 
> > physical mode (like using lowest priority delivery mode etc).
> > 
> > I will post couple of patches, which revert's Gleb's patch and 
> > another fix for the early boot failure issue, tomorrow.
> 
If you'll revert my patch it will not be possible to use x2apic in
KVM (at least without KVM implementing interrupt remapping which is
unneeded otherwise) and x2apic interface is much better for vitalization.
Instead of reverting the patch it will be better to add check if x2apic
can be used without intr-remmaping (all CPUs belong to cluster 0) or
allow enabling of x2apic without IR if running as a guest.

--
			Gleb.

Re: [PATCH] x86: enable x2apic early at the first point

[Ingo Molnar]

* Gleb Natapov <gleb@redhat.com> wrote:

> > > I will post couple of patches, which revert's Gleb's patch 
> > > and another fix for the early boot failure issue, 
> > > tomorrow.
> 
> If you'll revert my patch it will not be possible to use 
> x2apic in KVM (at least without KVM implementing interrupt 
> remapping which is unneeded otherwise) and x2apic interface is 
> much better for vitalization. Instead of reverting the patch 
> it will be better to add check if x2apic can be used without 
> intr-remmaping (all CPUs belong to cluster 0) or allow 
> enabling of x2apic without IR if running as a guest.

yep, that would be required - because your patch can break real 
systems right now. Mind sending me a fix for it?

I've applied Yinghai's fix as well, so please base it on latest 
tip:master.

	Ingo

Re: [PATCH] x86: enable x2apic early at the first point

[Ingo Molnar]

* Yinghai Lu <yinghai@kernel.org> wrote:

> Impact: fix bug.
> 
> otherwise will get panic from early_acpi_boot_init()
> also make disable_x2apic global, so could use it it x2_apic_xxx.c
> and can get warning if preenabled system using nox2apic.
> 
> Signed-off-by: Yinghai Lu <yinghai@kernel.org>
> 
> ---
>  arch/x86/kernel/apic/apic.c           |    3 +--
>  arch/x86/kernel/apic/x2apic_cluster.c |    5 ++++-
>  arch/x86/kernel/apic/x2apic_phys.c    |    5 ++++-
>  arch/x86/kernel/apic/x2apic_uv_x.c    |    4 +++-
>  drivers/pci/dmar.c                    |    3 ++-
>  5 files changed, 14 insertions(+), 6 deletions(-)

I've applied it because it fixes a real bug, but this code 
really needs a cleanup. Look at the repeat patterns:

> Index: linux-2.6/arch/x86/kernel/apic/x2apic_cluster.c
> ===================================================================
> --- linux-2.6.orig/arch/x86/kernel/apic/x2apic_cluster.c
> +++ linux-2.6/arch/x86/kernel/apic/x2apic_cluster.c
> @@ -14,8 +14,11 @@ DEFINE_PER_CPU(u32, x86_cpu_to_logical_a
>  
>  static int x2apic_acpi_madt_oem_check(char *oem_id, char *oem_table_id)
>  {
> -	if (cpu_has_x2apic)
> +	if (cpu_has_x2apic && !disable_x2apic) {
> +		x2apic = 1;
> +		enable_x2apic();
>  		return 1;
> +	}
>  
>  	return 0;
>  }
> Index: linux-2.6/arch/x86/kernel/apic/x2apic_phys.c
> ===================================================================
> --- linux-2.6.orig/arch/x86/kernel/apic/x2apic_phys.c
> +++ linux-2.6/arch/x86/kernel/apic/x2apic_phys.c
> @@ -21,8 +21,11 @@ early_param("x2apic_phys", set_x2apic_ph
>  
>  static int x2apic_acpi_madt_oem_check(char *oem_id, char *oem_table_id)
>  {
> -	if (cpu_has_x2apic && x2apic_phys)
> +	if (cpu_has_x2apic && !disable_x2apic && x2apic_phys) {
> +		x2apic = 1;
> +		enable_x2apic();
>  		return 1;
> +	}
>  
>  	return 0;
>  }
> Index: linux-2.6/arch/x86/kernel/apic/x2apic_uv_x.c
> ===================================================================
> --- linux-2.6.orig/arch/x86/kernel/apic/x2apic_uv_x.c
> +++ linux-2.6/arch/x86/kernel/apic/x2apic_uv_x.c
> @@ -41,8 +41,10 @@ static int uv_acpi_madt_oem_check(char *
>  			uv_system_type = UV_LEGACY_APIC;
>  		else if (!strcmp(oem_table_id, "UVX"))
>  			uv_system_type = UV_X2APIC;
> -		else if (!strcmp(oem_table_id, "UVH")) {
> +		else if (!strcmp(oem_table_id, "UVH") && !disable_x2apic) {
>  			uv_system_type = UV_NON_UNIQUE_APIC;
> +			x2apic = 1;
> +			enable_x2apic();
>  			return 1;
>  		}
>  	}

Such repeat patterns with small variations are always the sign 
of an inefficient code structure.

The clean approach would be to have one generic helper function 
concentrated into enable_x2apic(). So instead of:

 static int x2apic_acpi_madt_oem_check(char *oem_id, char *oem_table_id)
 {
	if (cpu_has_x2apic && !disable_x2apic && x2apic_phys) {
		x2apic = 1;
		enable_x2apic();
		return 1;
	}

	return 0;
 }

We'd have:

 static int x2apic_acpi_madt_oem_check(char *oem_id, char *oem_table_id)
 {
	if (!x2apic_phys)
		return 0;

	return x86_enable_x2apic();
 }

That way all the repeat and common functionality (and the return 
code logic) is concentrated into enable_x2apic(), and the 
x2apic_acpi_madt_oem_check() function has _only_ its own special 
check open-coded. (namely, whether physical ID delivery mode is 
forced off.)

Okay?

	Ingo

Re: [PATCH] x86: enable x2apic early at the first point

[Ingo Molnar]

* Ingo Molnar <mingo@elte.hu> wrote:

> >  arch/x86/kernel/apic/apic.c           |    3 +--
> >  arch/x86/kernel/apic/x2apic_cluster.c |    5 ++++-
> >  arch/x86/kernel/apic/x2apic_phys.c    |    5 ++++-
> >  arch/x86/kernel/apic/x2apic_uv_x.c    |    4 +++-
> >  drivers/pci/dmar.c                    |    3 ++-
> >  5 files changed, 14 insertions(+), 6 deletions(-)
> 
> I've applied it because it fixes a real bug, but this code 
> really needs a cleanup. Look at the repeat patterns:

unapplied it again as it breaks the build:

arch/x86/kernel/apic/x2apic_cluster.c: In function 'x2apic_acpi_madt_oem_check':
arch/x86/kernel/apic/x2apic_cluster.c:17: error: 'disable_x2apic' undeclared (first use in this function)
arch/x86/kernel/apic/x2apic_cluster.c:17: error: (Each undeclared identifier is reported only once
arch/x86/kernel/apic/x2apic_cluster.c:17: error: for each function it appears in.)

so please resend the fixed and cleaned up version.

Thanks,

	Ingo

Re: [PATCH] x86: enable x2apic early at the first point

[Gleb Natapov]

On Fri, Feb 20, 2009 at 10:41:23AM +0100, Ingo Molnar wrote:
> 
> * Gleb Natapov <gleb@redhat.com> wrote:
> 
> > > > I will post couple of patches, which revert's Gleb's patch 
> > > > and another fix for the early boot failure issue, 
> > > > tomorrow.
> > 
> > If you'll revert my patch it will not be possible to use 
> > x2apic in KVM (at least without KVM implementing interrupt 
> > remapping which is unneeded otherwise) and x2apic interface is 
> > much better for vitalization. Instead of reverting the patch 
> > it will be better to add check if x2apic can be used without 
> > intr-remmaping (all CPUs belong to cluster 0) or allow 
> > enabling of x2apic without IR if running as a guest.
> 
> yep, that would be required - because your patch can break real 
> systems right now. Mind sending me a fix for it?
> 
> I've applied Yinghai's fix as well, so please base it on latest 
> tip:master.
> 
OK. Will send next week.

--
			Gleb.

Re: [PATCH] x86: enable x2apic early at the first point

[Gleb Natapov]

On Fri, Feb 20, 2009 at 12:06:51PM +0100, Ingo Molnar wrote:
> 
> * Gleb Natapov <gleb@redhat.com> wrote:
> 
> > On Fri, Feb 20, 2009 at 10:41:23AM +0100, Ingo Molnar wrote:
> > > 
> > > * Gleb Natapov <gleb@redhat.com> wrote:
> > > 
> > > > > > I will post couple of patches, which revert's Gleb's patch 
> > > > > > and another fix for the early boot failure issue, 
> > > > > > tomorrow.
> > > > 
> > > > If you'll revert my patch it will not be possible to use 
> > > > x2apic in KVM (at least without KVM implementing interrupt 
> > > > remapping which is unneeded otherwise) and x2apic interface is 
> > > > much better for vitalization. Instead of reverting the patch 
> > > > it will be better to add check if x2apic can be used without 
> > > > intr-remmaping (all CPUs belong to cluster 0) or allow 
> > > > enabling of x2apic without IR if running as a guest.
> > > 
> > > yep, that would be required - because your patch can break real 
> > > systems right now. Mind sending me a fix for it?
> > > 
> > > I've applied Yinghai's fix as well, so please base it on latest 
> > > tip:master.
> > > 
> > OK. Will send next week.
> 
> ok, that's too long of a breakage window - i'll revert it then, 
> please send a new version once you have it.
> 
Deal.

--
			Gleb.

Re: [PATCH] x86: enable x2apic early at the first point

[Ingo Molnar]

* Gleb Natapov <gleb@redhat.com> wrote:

> On Fri, Feb 20, 2009 at 10:41:23AM +0100, Ingo Molnar wrote:
> > 
> > * Gleb Natapov <gleb@redhat.com> wrote:
> > 
> > > > > I will post couple of patches, which revert's Gleb's patch 
> > > > > and another fix for the early boot failure issue, 
> > > > > tomorrow.
> > > 
> > > If you'll revert my patch it will not be possible to use 
> > > x2apic in KVM (at least without KVM implementing interrupt 
> > > remapping which is unneeded otherwise) and x2apic interface is 
> > > much better for vitalization. Instead of reverting the patch 
> > > it will be better to add check if x2apic can be used without 
> > > intr-remmaping (all CPUs belong to cluster 0) or allow 
> > > enabling of x2apic without IR if running as a guest.
> > 
> > yep, that would be required - because your patch can break real 
> > systems right now. Mind sending me a fix for it?
> > 
> > I've applied Yinghai's fix as well, so please base it on latest 
> > tip:master.
> > 
> OK. Will send next week.

ok, that's too long of a breakage window - i'll revert it then, 
please send a new version once you have it.

Thanks,

	Ingo

Re: [PATCH] x86: enable x2apic early at the first point

[Gleb Natapov]

On Fri, Feb 20, 2009 at 12:06:51PM +0100, Ingo Molnar wrote:
> > > > If you'll revert my patch it will not be possible to use 
> > > > x2apic in KVM (at least without KVM implementing interrupt 
> > > > remapping which is unneeded otherwise) and x2apic interface is 
> > > > much better for vitalization. Instead of reverting the patch 
> > > > it will be better to add check if x2apic can be used without 
> > > > intr-remmaping (all CPUs belong to cluster 0) or allow 
> > > > enabling of x2apic without IR if running as a guest.
> > > 
> > > yep, that would be required - because your patch can break real 
> > > systems right now. Mind sending me a fix for it?
> > > 
> > > I've applied Yinghai's fix as well, so please base it on latest 
> > > tip:master.
> > > 
> > OK. Will send next week.
> 
> ok, that's too long of a breakage window - i'll revert it then, 
> please send a new version once you have it.
> 
What approach you actually prefer? Enable x2apic without intr-remapping
only when running in KVM, or even if running on real HW but all CPUs are
in cluster 0? If former then below is updated patch (tested only in KVM)
if later then it's definitely for next week :)

From: Gleb Natapov <gleb@redhat.com>
Subject: x86, apic: decouple x2apic from interrupt remapping

Currently x2apic is used only if interrupt remapping can be enabled,
and although there is not real HW that has x2apic but does not have
intr-remapping  the decoupling is useful since we want to emulate x2apic
interface in KVM (it is more vitalization friendly), but we don't want
to emulate intr-remapping just for that. Attached patch enables x2apic
when running in kvm guest even if intr-remapping cannot be enabled.

Signed-off-by: Gleb Natapov <gleb@redhat.com>
diff --git a/arch/x86/kernel/apic/apic.c b/arch/x86/kernel/apic/apic.c
index c3acf90..98b9a0b 100644
--- a/arch/x86/kernel/apic/apic.c
+++ b/arch/x86/kernel/apic/apic.c
@@ -48,6 +48,7 @@
 #include <asm/idle.h>
 #include <asm/mtrr.h>
 #include <asm/smp.h>
+#include <asm/kvm_para.h>
 
 unsigned int num_processors;
 
@@ -1296,10 +1297,44 @@ void enable_x2apic(void)
 	}
 }
 
-void __init enable_IR_x2apic(void)
+static int __init enable_IR(void)
 {
 #ifdef CONFIG_INTR_REMAP
 	int ret;
+
+	ret = dmar_table_init();
+	if (ret) {
+		pr_info("dmar_table_init() failed with %d:\n", ret);
+		return ret;
+	}
+
+	ret = save_mask_IO_APIC_setup();
+	if (ret) {
+		pr_info("Saving IO-APIC state failed: %d\n", ret);
+		return ret;
+	}
+
+	ret = enable_intr_remapping(1);
+	if (ret) {
+		restore_IO_APIC_setup();
+		return ret;
+	}
+
+	reinit_intr_remapped_IO_APIC(x2apic_preenabled);
+
+	return 0;
+#else
+	if (!kvm_para_available()) {
+		local_irq_restore(flags);
+		panic("x2apic enabled prior OS handover,"
+				" enable CONFIG_INTR_REMAP");
+	}
+	return -ENODEV;
+#endif
+}
+
+void __init enable_IR_x2apic(void)
+{
 	unsigned long flags;
 
 	if (!cpu_has_x2apic)
@@ -1320,72 +1355,33 @@ void __init enable_IR_x2apic(void)
 		return;
 	}
 
-	ret = dmar_table_init();
-	if (ret) {
-		pr_info("dmar_table_init() failed with %d:\n", ret);
-
-		if (x2apic_preenabled)
-			panic("x2apic enabled by bios. But IR enabling failed");
-		else
-			pr_info("Not enabling x2apic,Intr-remapping\n");
-		return;
-	}
-
 	local_irq_save(flags);
 	mask_8259A();
 
-	ret = save_mask_IO_APIC_setup();
-	if (ret) {
-		pr_info("Saving IO-APIC state failed: %d\n", ret);
-		goto end;
-	}
-
-	ret = enable_intr_remapping(1);
-
-	if (ret && x2apic_preenabled) {
-		local_irq_restore(flags);
-		panic("x2apic enabled by bios. But IR enabling failed");
+	if (enable_IR()) {
+		if (!kvm_para_available()) {
+			if (x2apic_preenabled) {
+				local_irq_restore(flags);
+				panic("x2apic enabled by bios. "
+						"But IR enabling failed");
+			}
+			goto no_x2apic;
+		}
+	} else {
+		pr_info("Enabled interrupt remapping for x2apic\n");
 	}
 
-	if (ret)
-		goto end_restore;
-
 	if (!x2apic) {
 		x2apic = 1;
 		enable_x2apic();
 	}
 
-end_restore:
-	if (ret)
-		/*
-		 * IR enabling failed
-		 */
-		restore_IO_APIC_setup();
-	else
-		reinit_intr_remapped_IO_APIC(x2apic_preenabled);
-
-end:
+no_x2apic:
 	unmask_8259A();
 	local_irq_restore(flags);
 
-	if (!ret) {
-		if (!x2apic_preenabled)
-			pr_info("Enabled x2apic and interrupt-remapping\n");
-		else
-			pr_info("Enabled Interrupt-remapping\n");
-	} else
-		pr_err("Failed to enable Interrupt-remapping and x2apic\n");
-#else
-	if (!cpu_has_x2apic)
-		return;
-
-	if (x2apic_preenabled)
-		panic("x2apic enabled prior OS handover,"
-		      " enable CONFIG_INTR_REMAP");
-
-	pr_info("Enable CONFIG_INTR_REMAP for enabling intr-remapping "
-		" and x2apic\n");
-#endif
+	if (x2apic && !x2apic_preenabled)
+		pr_info("Enabled x2apic\n");
 
 	return;
 }
diff --git a/arch/x86/kernel/apic/probe_64.c b/arch/x86/kernel/apic/probe_64.c
index 70935dd..8f11f6d 100644
--- a/arch/x86/kernel/apic/probe_64.c
+++ b/arch/x86/kernel/apic/probe_64.c
@@ -22,6 +22,7 @@
 #include <asm/apic.h>
 #include <asm/ipi.h>
 #include <asm/setup.h>
+#include <asm/kvm_para.h>
 
 extern struct apic apic_flat;
 extern struct apic apic_physflat;
@@ -51,7 +52,7 @@ void __init default_setup_apic_routing(void)
 {
 #ifdef CONFIG_X86_X2APIC
 	if (apic == &apic_x2apic_phys || apic == &apic_x2apic_cluster) {
-		if (!intr_remapping_enabled)
+		if (!intr_remapping_enabled && !kvm_para_available())
 			apic = &apic_flat;
 	}
 #endif
diff --git a/arch/x86/kernel/apic/x2apic_cluster.c b/arch/x86/kernel/apic/x2apic_cluster.c
index 4e39d9a..3f7df23 100644
--- a/arch/x86/kernel/apic/x2apic_cluster.c
+++ b/arch/x86/kernel/apic/x2apic_cluster.c
@@ -24,7 +24,10 @@ static int x2apic_acpi_madt_oem_check(char *oem_id, char *oem_table_id)
 
 static const struct cpumask *x2apic_target_cpus(void)
 {
-	return cpumask_of(0);
+	if (intr_remapping_enabled)
+		return cpumask_of(0);
+	else
+		return cpu_online_mask;
 }
 
 /*
@@ -116,37 +119,46 @@ static int x2apic_apic_id_registered(void)
 
 static unsigned int x2apic_cpu_mask_to_apicid(const struct cpumask *cpumask)
 {
-	/*
-	 * We're using fixed IRQ delivery, can only return one logical APIC ID.
-	 * May as well be the first.
-	 */
-	int cpu = cpumask_first(cpumask);
+	if (intr_remapping_enabled) {
+		/*
+		 * We're using fixed IRQ delivery, can only return one
+		 * logical APIC ID. May as well be the first.
+		 */
+		int cpu = cpumask_first(cpumask);
 
-	if ((unsigned)cpu < nr_cpu_ids)
-		return per_cpu(x86_cpu_to_logical_apicid, cpu);
-	else
-		return BAD_APICID;
+		if ((unsigned)cpu < nr_cpu_ids)
+			return per_cpu(x86_cpu_to_logical_apicid, cpu);
+		else
+			return BAD_APICID;
+	} else
+		return cpumask_bits(cpumask)[0] & APIC_ALL_CPUS;
 }
 
 static unsigned int
 x2apic_cpu_mask_to_apicid_and(const struct cpumask *cpumask,
 			      const struct cpumask *andmask)
 {
-	int cpu;
+	if (intr_remapping_enabled) {
+		int cpu;
 
-	/*
-	 * We're using fixed IRQ delivery, can only return one logical APIC ID.
-	 * May as well be the first.
-	 */
-	for_each_cpu_and(cpu, cpumask, andmask) {
-		if (cpumask_test_cpu(cpu, cpu_online_mask))
-			break;
-	}
+		/*
+		 * We're using fixed IRQ delivery, can only return one
+		 * logical APIC ID. May as well be the first.
+		 */
+		for_each_cpu_and(cpu, cpumask, andmask) {
+			if (cpumask_test_cpu(cpu, cpu_online_mask))
+				break;
+		}
 
-	if (cpu < nr_cpu_ids)
-		return per_cpu(x86_cpu_to_logical_apicid, cpu);
+		if (cpu < nr_cpu_ids)
+			return per_cpu(x86_cpu_to_logical_apicid, cpu);
 
-	return BAD_APICID;
+		return BAD_APICID;
+	} else {
+		unsigned long mask1 = cpumask_bits(cpumask)[0] & APIC_ALL_CPUS;
+		unsigned long mask2 = cpumask_bits(andmask)[0] & APIC_ALL_CPUS;
+		return mask1 & mask2;
+	}
 }
 
 static unsigned int x2apic_cluster_phys_get_apic_id(unsigned long x)
--
			Gleb.

Re: [PATCH] x86: enable x2apic early at the first point

[Ingo Molnar]

* Gleb Natapov <gleb@redhat.com> wrote:

> On Fri, Feb 20, 2009 at 12:06:51PM +0100, Ingo Molnar wrote:
> > > > > If you'll revert my patch it will not be possible to use 
> > > > > x2apic in KVM (at least without KVM implementing interrupt 
> > > > > remapping which is unneeded otherwise) and x2apic interface is 
> > > > > much better for vitalization. Instead of reverting the patch 
> > > > > it will be better to add check if x2apic can be used without 
> > > > > intr-remmaping (all CPUs belong to cluster 0) or allow 
> > > > > enabling of x2apic without IR if running as a guest.
> > > > 
> > > > yep, that would be required - because your patch can break real 
> > > > systems right now. Mind sending me a fix for it?
> > > > 
> > > > I've applied Yinghai's fix as well, so please base it on latest 
> > > > tip:master.
> > > > 
> > > OK. Will send next week.
> > 
> > ok, that's too long of a breakage window - i'll revert it then, 
> > please send a new version once you have it.
> > 
>
> What approach you actually prefer? Enable x2apic without 
> intr-remapping only when running in KVM, or even if running on 
> real HW but all CPUs are in cluster 0? If former then below is 
> updated patch (tested only in KVM) if later then it's 
> definitely for next week :)

The latter would be the ideal solution - i.e. next week :)

x2apic is a rare feature, and the more exposure we give it the 
more tested it becomes. It's also obviously good for general 
code structure if core CPU features are separated from 
chipset/iommu features.

Plus, x2apic accesses could actually be faster than UC accesses 
to the lapic, so whenever we can we should enter x2apic mode - 
even if intr-remap is not turned on (because not needed).

	Ingo

Re: [PATCH] x86: enable x2apic early at the first point

[Suresh Siddha]

On Fri, 2009-02-20 at 01:55 -0800, Ingo Molnar wrote:
> * Ingo Molnar <mingo@elte.hu> wrote:
> 
> > >  arch/x86/kernel/apic/apic.c           |    3 +--
> > >  arch/x86/kernel/apic/x2apic_cluster.c |    5 ++++-
> > >  arch/x86/kernel/apic/x2apic_phys.c    |    5 ++++-
> > >  arch/x86/kernel/apic/x2apic_uv_x.c    |    4 +++-
> > >  drivers/pci/dmar.c                    |    3 ++-
> > >  5 files changed, 14 insertions(+), 6 deletions(-)
> > 
> > I've applied it because it fixes a real bug, but this code 
> > really needs a cleanup. Look at the repeat patterns:
> 
> unapplied it again as it breaks the build:
> 
> arch/x86/kernel/apic/x2apic_cluster.c: In function 'x2apic_acpi_madt_oem_check':
> arch/x86/kernel/apic/x2apic_cluster.c:17: error: 'disable_x2apic' undeclared (first use in this function)
> arch/x86/kernel/apic/x2apic_cluster.c:17: error: (Each undeclared identifier is reported only once
> arch/x86/kernel/apic/x2apic_cluster.c:17: error: for each function it appears in.)
> 
> so please resend the fixed and cleaned up version.

From: Suresh Siddha <suresh.b.siddha@intel.com>
Subject: x86: select x2apic ops in early apic probe only if x2apic mode is enabled

If BIOS hands over the control to OS in legacy xapic mode, select legacy xapic
related ops in the early apic probe and shift to x2apic ops later in the boot
sequence, only after enabling x2apic mode.

If BIOS hands over the control in x2apic mode, select x2apic related ops
in the early apic probe.

This fixes the early boot panic, where we were selecting x2apic ops,
while the cpu is still in legacy xapic mode.

Signed-off-by: Suresh Siddha <suresh.b.siddha@intel.com>
---

Index: tip/arch/x86/include/asm/apic.h
===================================================================
--- tip/arch/x86/include/asm/apic.h.orig
+++ tip/arch/x86/include/asm/apic.h
@@ -146,7 +146,7 @@ static inline u64 native_x2apic_icr_read
 	return val;
 }
 
-extern int x2apic;
+extern int x2apic, x2apic_phys;
 extern void check_x2apic(void);
 extern void enable_x2apic(void);
 extern void enable_IR_x2apic(void);
Index: tip/arch/x86/kernel/apic/probe_64.c
===================================================================
--- tip/arch/x86/kernel/apic/probe_64.c.orig
+++ tip/arch/x86/kernel/apic/probe_64.c
@@ -50,9 +50,16 @@ static struct apic *apic_probe[] __initd
 void __init default_setup_apic_routing(void)
 {
 #ifdef CONFIG_X86_X2APIC
-	if (apic == &apic_x2apic_phys || apic == &apic_x2apic_cluster) {
-		if (!intr_remapping_enabled)
-			apic = &apic_flat;
+	if (x2apic && (apic != &apic_x2apic_phys &&
+#ifdef CONFIG_X86_UV
+		       apic != &apic_x2apic_uv_x &&
+#endif
+		       apic != &apic_x2apic_cluster)) {
+		if (x2apic_phys)
+			apic = &apic_x2apic_phys;
+		else
+			apic = &apic_x2apic_cluster;
+		printk(KERN_INFO "Setting APIC routing to %s\n", apic->name);
 	}
 #endif
 
Index: tip/arch/x86/kernel/apic/x2apic_cluster.c
===================================================================
--- tip/arch/x86/kernel/apic/x2apic_cluster.c.orig
+++ tip/arch/x86/kernel/apic/x2apic_cluster.c
@@ -14,10 +14,7 @@ DEFINE_PER_CPU(u32, x86_cpu_to_logical_a
 
 static int x2apic_acpi_madt_oem_check(char *oem_id, char *oem_table_id)
 {
-	if (cpu_has_x2apic)
-		return 1;
-
-	return 0;
+	return x2apic_enabled();
 }
 
 /* Start with all IRQs pointing to boot CPU.  IRQ balancing will shift them. */
Index: tip/arch/x86/kernel/apic/x2apic_phys.c
===================================================================
--- tip/arch/x86/kernel/apic/x2apic_phys.c.orig
+++ tip/arch/x86/kernel/apic/x2apic_phys.c
@@ -10,7 +10,7 @@
 #include <asm/apic.h>
 #include <asm/ipi.h>
 
-static int x2apic_phys;
+int x2apic_phys;
 
 static int set_x2apic_phys_mode(char *arg)
 {
@@ -21,10 +21,10 @@ early_param("x2apic_phys", set_x2apic_ph
 
 static int x2apic_acpi_madt_oem_check(char *oem_id, char *oem_table_id)
 {
-	if (cpu_has_x2apic && x2apic_phys)
-		return 1;
-
-	return 0;
+	if (x2apic_phys)
+		return x2apic_enabled();
+	else
+		return 0;
 }
 
 /* Start with all IRQs pointing to boot CPU.  IRQ balancing will shift them. */
Index: tip/arch/x86/kernel/apic/apic.c
===================================================================
--- tip/arch/x86/kernel/apic/apic.c.orig
+++ tip/arch/x86/kernel/apic/apic.c
@@ -1269,14 +1269,7 @@ void __cpuinit end_local_APIC_setup(void
 #ifdef CONFIG_X86_X2APIC
 void check_x2apic(void)
 {
-	int msr, msr2;
-
-	if (!cpu_has_x2apic)
-		return;
-
-	rdmsr(MSR_IA32_APICBASE, msr, msr2);
-
-	if (msr & X2APIC_ENABLE) {
+	if (x2apic_enabled()) {
 		pr_info("x2apic enabled by BIOS, switching to x2apic ops\n");
 		x2apic_preenabled = x2apic = 1;
 	}

Re: [PATCH] x86: enable x2apic early at the first point

[Yinghai Lu]

Suresh Siddha wrote:
> On Fri, 2009-02-20 at 01:55 -0800, Ingo Molnar wrote:
>> * Ingo Molnar <mingo@elte.hu> wrote:
>>
>>>>  arch/x86/kernel/apic/apic.c           |    3 +--
>>>>  arch/x86/kernel/apic/x2apic_cluster.c |    5 ++++-
>>>>  arch/x86/kernel/apic/x2apic_phys.c    |    5 ++++-
>>>>  arch/x86/kernel/apic/x2apic_uv_x.c    |    4 +++-
>>>>  drivers/pci/dmar.c                    |    3 ++-
>>>>  5 files changed, 14 insertions(+), 6 deletions(-)
>>> I've applied it because it fixes a real bug, but this code 
>>> really needs a cleanup. Look at the repeat patterns:
>> unapplied it again as it breaks the build:
>>
>> arch/x86/kernel/apic/x2apic_cluster.c: In function 'x2apic_acpi_madt_oem_check':
>> arch/x86/kernel/apic/x2apic_cluster.c:17: error: 'disable_x2apic' undeclared (first use in this function)
>> arch/x86/kernel/apic/x2apic_cluster.c:17: error: (Each undeclared identifier is reported only once
>> arch/x86/kernel/apic/x2apic_cluster.c:17: error: for each function it appears in.)
>>
>> so please resend the fixed and cleaned up version.
> 
> From: Suresh Siddha <suresh.b.siddha@intel.com>
> Subject: x86: select x2apic ops in early apic probe only if x2apic mode is enabled
> 
> If BIOS hands over the control to OS in legacy xapic mode, select legacy xapic
> related ops in the early apic probe and shift to x2apic ops later in the boot
> sequence, only after enabling x2apic mode.
> 
> If BIOS hands over the control in x2apic mode, select x2apic related ops
> in the early apic probe.
> 
> This fixes the early boot panic, where we were selecting x2apic ops,
> while the cpu is still in legacy xapic mode.

good, other than that.

for x2apic preenabled system,
when nox2apic is used, cpu_has_x2apic will be cleared, apic will be xapic phys_flat or flat.
is that expected?

should
1. ignore nox2apic
2. or try to disable x2apic?

YH

Re: [PATCH] x86: enable x2apic early at the first point

[Suresh Siddha]

On Sat, Feb 21, 2009 at 02:43:46PM -0800, Yinghai Lu wrote:
> for x2apic preenabled system,
> when nox2apic is used, cpu_has_x2apic will be cleared, apic will be xapic phys_flat or flat.
> is that expected?
> 
> should
> 1. ignore nox2apic
> 2. or try to disable x2apic?

This scenario might be useful for debug purposes? But it might
not be simple/straight fwd in OS to implement this, as we need to do two things.

1. Go back to xapic mode using the state transition diagram in SDM.

2. And also, we need to disable the interrupt-remapping setup by the bios,
so that chipset and cpu's are in same mode.

If BIOS has enabled x2apic, it is for a reason (mostly platform
has more logical cpus and hence need x2apic to brinup all the AP's etc).
And typically other than very high end platforms, I don't expect bios
to turn on x2apic.

thanks,
suresh

Re: [PATCH] x86: enable x2apic early at the first point

[Ingo Molnar]

* Suresh Siddha <suresh.b.siddha@intel.com> wrote:

> On Fri, 2009-02-20 at 01:55 -0800, Ingo Molnar wrote:
> > * Ingo Molnar <mingo@elte.hu> wrote:
> > 
> > > >  arch/x86/kernel/apic/apic.c           |    3 +--
> > > >  arch/x86/kernel/apic/x2apic_cluster.c |    5 ++++-
> > > >  arch/x86/kernel/apic/x2apic_phys.c    |    5 ++++-
> > > >  arch/x86/kernel/apic/x2apic_uv_x.c    |    4 +++-
> > > >  drivers/pci/dmar.c                    |    3 ++-
> > > >  5 files changed, 14 insertions(+), 6 deletions(-)
> > > 
> > > I've applied it because it fixes a real bug, but this code 
> > > really needs a cleanup. Look at the repeat patterns:
> > 
> > unapplied it again as it breaks the build:
> > 
> > arch/x86/kernel/apic/x2apic_cluster.c: In function 'x2apic_acpi_madt_oem_check':
> > arch/x86/kernel/apic/x2apic_cluster.c:17: error: 'disable_x2apic' undeclared (first use in this function)
> > arch/x86/kernel/apic/x2apic_cluster.c:17: error: (Each undeclared identifier is reported only once
> > arch/x86/kernel/apic/x2apic_cluster.c:17: error: for each function it appears in.)
> > 
> > so please resend the fixed and cleaned up version.
> 
> From: Suresh Siddha <suresh.b.siddha@intel.com>
> Subject: x86: select x2apic ops in early apic probe only if x2apic mode is enabled

Applied to tip:x86/apic, thanks Suresh!

	Ingo