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

[Ingo Molnar <mingo@elte.hu>]

* 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