RE: [patch 2/2] x2apic/intr-remap: decouple interrupt remapping from x2apic

[David Woodhouse <dwmw2@infradead.org>]

On Sat, 2009-04-04 at 11:37 +0800, Han, Weidong wrote:
> Siddha, Suresh B wrote:
> > On Fri, 2009-04-03 at 02:15 -0700, Han, Weidong wrote:
> >> 
> >> -#ifdef CONFIG_INTR_REMAP
> >> +#ifdef CONFIG_X86_X2APIC
> > 
> > Weidong, Can we also fix these config options?
> > 
> > Today, selecting CONFIG_INTR_REMAP automatically selects
> > CONFIG_X86_X2APIC. It should be the other way round.
> > 
> > i.e., turning on CONFIG_X86_X2APIC should automatically enable
> > CONFIG_INTR_REMAP and just enabling CONFIG_INTR_REMAP shouldn't enable
> > CONFIG_X86_X2APIC.
> > 
> > Also please make sure that the code compiles (and works :)) in
> > different combinations of these config settings.
> 
> 
> Thanks for your reminder. Following patch changes the config options
> selection. The code can compiles in different combinations.

Doesn't build for me.

Firstly, your patch 2/2 doesn't apply against git HEAD. I needed to
massage it to work. I think you're testing against an old kernel.

Then (with the massaged version in the iommu-2.6.git tree) if I apply
your third patch below I get this error when building with INTR_REMAP
&& !X2APIC:

arch/x86/kernel/apic/apic.c: In function ‘lapic_resume’:
arch/x86/kernel/apic/apic.c:2054: error: ‘EIM_32BIT_APIC_ID’ undeclared (first use in this function)
arch/x86/kernel/apic/apic.c:2054: error: (Each undeclared identifier is reported only once
arch/x86/kernel/apic/apic.c:2054: error: for each function it appears in.)
make[2]: *** [arch/x86/kernel/apic/apic.o] Error 1

If I apply the naïve fix of just changing a couple more #ifdef
CONFIG_INTR_REMAP to #ifdef CONFIG_X86_X2APIC (in enable_IR_x2apic() and
lapic_resume()), it builds but doesn't boot -- it just hangs after
reporting Queued Invalidation, at the code you added in dmar.c to call
enable_intr_remapping().

Please make sure you're testing against a current kernel, and test that
it builds and boots in all three configurations (no remapping, remapping
but no x2apic, and with both enabled).

-- 
David Woodhouse                            Open Source Technology Centre
David.Woodhouse@intel.com                              Intel Corporation