[Bugfix] x86/apic: Fix xen IRQ allocation failure caused by commit b81975eade8c

[Bugfix] x86/apic: Fix xen IRQ allocation failure caused by commit b81975eade8c

[Jiang Liu]

Commit b81975eade8c ("x86, irq: Clean up irqdomain transition code")
breaks xen IRQ allocation because xen_smp_prepare_cpus() doesn't invoke
setup_IO_APIC(), so no irqdomains created for IOAPICs and
mp_map_pin_to_irq() fails at the very beginning.

Enhance xen_smp_prepare_cpus() to call setup_IO_APIC() to initialize
irqdomain for IOAPICs.

Signed-off-by: Jiang Liu <jiang.liu@linux.intel.com>
Reported-and-tested-by: Sander Eikelenboom <linux@eikelenboom.it>
Cc: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
---
Hi all,
	This patch should be backported to v3.17, but there are
conflicts. So I will send backported patch to 3.17/3.18 stable tree
once this patch has been merged into mainstream kernel.
Thanks!
Gerry
---
 arch/x86/include/asm/io_apic.h       |    2 +-
 arch/x86/include/asm/smpboot_hooks.h |    5 ++---
 arch/x86/kernel/apic/apic.c          |    5 ++---
 arch/x86/kernel/apic/io_apic.c       |   32 +++++++++++++++-----------------
 arch/x86/xen/smp.c                   |    3 +++
 5 files changed, 23 insertions(+), 24 deletions(-)

diff --git a/arch/x86/include/asm/io_apic.h b/arch/x86/include/asm/io_apic.h
index bf006cce9418..72a1298658bb 100644
--- a/arch/x86/include/asm/io_apic.h
+++ b/arch/x86/include/asm/io_apic.h
@@ -237,7 +237,7 @@ static inline void io_apic_modify(unsigned int apic, unsigned int reg, unsigned
 
 extern void io_apic_eoi(unsigned int apic, unsigned int vector);
 
-extern void setup_IO_APIC(void);
+extern void setup_IO_APIC(bool xen_smp);
 extern void enable_IO_APIC(void);
 extern void disable_IO_APIC(void);
 extern void setup_ioapic_dest(void);
diff --git a/arch/x86/include/asm/smpboot_hooks.h b/arch/x86/include/asm/smpboot_hooks.h
index 0da7409f0bec..e47df710a588 100644
--- a/arch/x86/include/asm/smpboot_hooks.h
+++ b/arch/x86/include/asm/smpboot_hooks.h
@@ -53,10 +53,9 @@ static inline void __init smpboot_setup_io_apic(void)
 	 * go and set it up:
 	 */
 	if (!skip_ioapic_setup && nr_ioapics)
-		setup_IO_APIC();
-	else {
+		setup_IO_APIC(false);
+	else
 		nr_ioapics = 0;
-	}
 #endif
 }
 
diff --git a/arch/x86/kernel/apic/apic.c b/arch/x86/kernel/apic/apic.c
index 29b5b18afa27..71b8a6cb7f0e 100644
--- a/arch/x86/kernel/apic/apic.c
+++ b/arch/x86/kernel/apic/apic.c
@@ -1913,10 +1913,9 @@ int __init APIC_init_uniprocessor(void)
 
 #ifdef CONFIG_X86_IO_APIC
 	if (smp_found_config && !skip_ioapic_setup && nr_ioapics)
-		setup_IO_APIC();
-	else {
+		setup_IO_APIC(false);
+	else
 		nr_ioapics = 0;
-	}
 #endif
 
 	x86_init.timers.setup_percpu_clockev();
diff --git a/arch/x86/kernel/apic/io_apic.c b/arch/x86/kernel/apic/io_apic.c
index 3f5f60406ab1..13cddc75e4c0 100644
--- a/arch/x86/kernel/apic/io_apic.c
+++ b/arch/x86/kernel/apic/io_apic.c
@@ -2369,31 +2369,29 @@ static void ioapic_destroy_irqdomain(int idx)
 	ioapics[idx].pin_info = NULL;
 }
 
-void __init setup_IO_APIC(void)
+void __init setup_IO_APIC(bool xen_smp)
 {
 	int ioapic;
 
-	/*
-	 * calling enable_IO_APIC() is moved to setup_local_APIC for BP
-	 */
-	io_apic_irqs = nr_legacy_irqs() ? ~PIC_IRQS : ~0UL;
+	if (!xen_smp) {
+		apic_printk(APIC_VERBOSE, "ENABLING IO-APIC IRQs\n");
+		io_apic_irqs = nr_legacy_irqs() ? ~PIC_IRQS : ~0UL;
+
+		/* Set up IO-APIC IRQ routing. */
+		x86_init.mpparse.setup_ioapic_ids();
+		sync_Arb_IDs();
+	}
 
-	apic_printk(APIC_VERBOSE, "ENABLING IO-APIC IRQs\n");
 	for_each_ioapic(ioapic)
 		BUG_ON(mp_irqdomain_create(ioapic));
-
-	/*
-         * Set up IO-APIC IRQ routing.
-         */
-	x86_init.mpparse.setup_ioapic_ids();
-
-	sync_Arb_IDs();
 	setup_IO_APIC_irqs();
-	init_IO_APIC_traps();
-	if (nr_legacy_irqs())
-		check_timer();
-
 	ioapic_initialized = 1;
+
+	if (!xen_smp) {
+		init_IO_APIC_traps();
+		if (nr_legacy_irqs())
+			check_timer();
+	}
 }
 
 /*
diff --git a/arch/x86/xen/smp.c b/arch/x86/xen/smp.c
index 4c071aeb8417..7eb0283901fa 100644
--- a/arch/x86/xen/smp.c
+++ b/arch/x86/xen/smp.c
@@ -326,7 +326,10 @@ static void __init xen_smp_prepare_cpus(unsigned int max_cpus)
 
 		xen_raw_printk(m);
 		panic(m);
+	} else {
+		setup_IO_APIC(true);
 	}
+
 	xen_init_lock_cpu(0);
 
 	smp_store_boot_cpu_info();
-- 
1.7.10.4

Re: [Bugfix] x86/apic: Fix xen IRQ allocation failure caused by commit b81975eade8c

[Konrad Rzeszutek Wilk]

On Wed, Jan 07, 2015 at 02:13:49PM +0800, Jiang Liu wrote:
> Commit b81975eade8c ("x86, irq: Clean up irqdomain transition code")
> breaks xen IRQ allocation because xen_smp_prepare_cpus() doesn't invoke
> setup_IO_APIC(), so no irqdomains created for IOAPICs and
> mp_map_pin_to_irq() fails at the very beginning.
> --- a/arch/x86/kernel/apic/io_apic.c
> +++ b/arch/x86/kernel/apic/io_apic.c
> @@ -2369,31 +2369,29 @@ static void ioapic_destroy_irqdomain(int idx)
>  	ioapics[idx].pin_info = NULL;
>  }
>  
> -void __init setup_IO_APIC(void)
> +void __init setup_IO_APIC(bool xen_smp)
>  {
>  	int ioapic;
>  
> -	/*
> -	 * calling enable_IO_APIC() is moved to setup_local_APIC for BP
> -	 */
> -	io_apic_irqs = nr_legacy_irqs() ? ~PIC_IRQS : ~0UL;
> +	if (!xen_smp) {
> +		apic_printk(APIC_VERBOSE, "ENABLING IO-APIC IRQs\n");
> +		io_apic_irqs = nr_legacy_irqs() ? ~PIC_IRQS : ~0UL;
> +
> +		/* Set up IO-APIC IRQ routing. */
> +		x86_init.mpparse.setup_ioapic_ids();
> +		sync_Arb_IDs();
> +	}


Is there a specific reason that this cannot run in all cases?

What I am asking is why are we doing a special case here? The description
at the top implied that we were just missing an call to
setup_IO_APIC.. 

>  
> -	apic_printk(APIC_VERBOSE, "ENABLING IO-APIC IRQs\n");
>  	for_each_ioapic(ioapic)
>  		BUG_ON(mp_irqdomain_create(ioapic));
> -
> -	/*
> -         * Set up IO-APIC IRQ routing.
> -         */
> -	x86_init.mpparse.setup_ioapic_ids();
> -
> -	sync_Arb_IDs();
>  	setup_IO_APIC_irqs();
> -	init_IO_APIC_traps();
> -	if (nr_legacy_irqs())
> -		check_timer();
> -
>  	ioapic_initialized = 1;
> +
> +	if (!xen_smp) {
> +		init_IO_APIC_traps();
> +		if (nr_legacy_irqs())
> +			check_timer();
> +	}
>  }
>  
>  /*
> diff --git a/arch/x86/xen/smp.c b/arch/x86/xen/smp.c
> index 4c071aeb8417..7eb0283901fa 100644
> --- a/arch/x86/xen/smp.c
> +++ b/arch/x86/xen/smp.c
> @@ -326,7 +326,10 @@ static void __init xen_smp_prepare_cpus(unsigned int max_cpus)
>  
>  		xen_raw_printk(m);
>  		panic(m);
> +	} else {
> +		setup_IO_APIC(true);
>  	}
> +
>  	xen_init_lock_cpu(0);
>  
>  	smp_store_boot_cpu_info();
> -- 
> 1.7.10.4
> 

Re: [Bugfix] x86/apic: Fix xen IRQ allocation failure caused by commit b81975eade8c

[Jiang Liu]

On 2015/1/7 22:50, Konrad Rzeszutek Wilk wrote:
> On Wed, Jan 07, 2015 at 02:13:49PM +0800, Jiang Liu wrote:
>> Commit b81975eade8c ("x86, irq: Clean up irqdomain transition code")
>> breaks xen IRQ allocation because xen_smp_prepare_cpus() doesn't invoke
>> setup_IO_APIC(), so no irqdomains created for IOAPICs and
>> mp_map_pin_to_irq() fails at the very beginning.
>> --- a/arch/x86/kernel/apic/io_apic.c
>> +++ b/arch/x86/kernel/apic/io_apic.c
>> @@ -2369,31 +2369,29 @@ static void ioapic_destroy_irqdomain(int idx)
>>  	ioapics[idx].pin_info = NULL;
>>  }
>>  
>> -void __init setup_IO_APIC(void)
>> +void __init setup_IO_APIC(bool xen_smp)
>>  {
>>  	int ioapic;
>>  
>> -	/*
>> -	 * calling enable_IO_APIC() is moved to setup_local_APIC for BP
>> -	 */
>> -	io_apic_irqs = nr_legacy_irqs() ? ~PIC_IRQS : ~0UL;
>> +	if (!xen_smp) {
>> +		apic_printk(APIC_VERBOSE, "ENABLING IO-APIC IRQs\n");
>> +		io_apic_irqs = nr_legacy_irqs() ? ~PIC_IRQS : ~0UL;
>> +
>> +		/* Set up IO-APIC IRQ routing. */
>> +		x86_init.mpparse.setup_ioapic_ids();
>> +		sync_Arb_IDs();
>> +	}
> 
> 
> Is there a specific reason that this cannot run in all cases?
> 
> What I am asking is why are we doing a special case here? The description
> at the top implied that we were just missing an call to
> setup_IO_APIC.. 
Hi Konrad,
	I'm not very familiar with Xen IRQ yet, so I just enabled the
code to create irqdomains for IOAPICs and keep other part as is. I will
try to check whether we could enable other part all together:)
Regards!
Gerry

> 
>>  
>> -	apic_printk(APIC_VERBOSE, "ENABLING IO-APIC IRQs\n");
>>  	for_each_ioapic(ioapic)
>>  		BUG_ON(mp_irqdomain_create(ioapic));
>> -
>> -	/*
>> -         * Set up IO-APIC IRQ routing.
>> -         */
>> -	x86_init.mpparse.setup_ioapic_ids();
>> -
>> -	sync_Arb_IDs();
>>  	setup_IO_APIC_irqs();
>> -	init_IO_APIC_traps();
>> -	if (nr_legacy_irqs())
>> -		check_timer();
>> -
>>  	ioapic_initialized = 1;
>> +
>> +	if (!xen_smp) {
>> +		init_IO_APIC_traps();
>> +		if (nr_legacy_irqs())
>> +			check_timer();
>> +	}
>>  }
>>  
>>  /*
>> diff --git a/arch/x86/xen/smp.c b/arch/x86/xen/smp.c
>> index 4c071aeb8417..7eb0283901fa 100644
>> --- a/arch/x86/xen/smp.c
>> +++ b/arch/x86/xen/smp.c
>> @@ -326,7 +326,10 @@ static void __init xen_smp_prepare_cpus(unsigned int max_cpus)
>>  
>>  		xen_raw_printk(m);
>>  		panic(m);
>> +	} else {
>> +		setup_IO_APIC(true);
>>  	}
>> +
>>  	xen_init_lock_cpu(0);
>>  
>>  	smp_store_boot_cpu_info();
>> -- 
>> 1.7.10.4
>>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/
> 

Re: [Bugfix] x86/apic: Fix xen IRQ allocation failure caused by commit b81975eade8c

[Konrad Rzeszutek Wilk]

On Wed, Jan 07, 2015 at 11:37:52PM +0800, Jiang Liu wrote:
> On 2015/1/7 22:50, Konrad Rzeszutek Wilk wrote:
> > On Wed, Jan 07, 2015 at 02:13:49PM +0800, Jiang Liu wrote:
> >> Commit b81975eade8c ("x86, irq: Clean up irqdomain transition code")
> >> breaks xen IRQ allocation because xen_smp_prepare_cpus() doesn't invoke
> >> setup_IO_APIC(), so no irqdomains created for IOAPICs and
> >> mp_map_pin_to_irq() fails at the very beginning.
> >> --- a/arch/x86/kernel/apic/io_apic.c
> >> +++ b/arch/x86/kernel/apic/io_apic.c
> >> @@ -2369,31 +2369,29 @@ static void ioapic_destroy_irqdomain(int idx)
> >>  	ioapics[idx].pin_info = NULL;
> >>  }
> >>  
> >> -void __init setup_IO_APIC(void)
> >> +void __init setup_IO_APIC(bool xen_smp)
> >>  {
> >>  	int ioapic;
> >>  
> >> -	/*
> >> -	 * calling enable_IO_APIC() is moved to setup_local_APIC for BP
> >> -	 */
> >> -	io_apic_irqs = nr_legacy_irqs() ? ~PIC_IRQS : ~0UL;
> >> +	if (!xen_smp) {
> >> +		apic_printk(APIC_VERBOSE, "ENABLING IO-APIC IRQs\n");
> >> +		io_apic_irqs = nr_legacy_irqs() ? ~PIC_IRQS : ~0UL;
> >> +
> >> +		/* Set up IO-APIC IRQ routing. */
> >> +		x86_init.mpparse.setup_ioapic_ids();
> >> +		sync_Arb_IDs();
> >> +	}
> > 
> > 
> > Is there a specific reason that this cannot run in all cases?
> > 
> > What I am asking is why are we doing a special case here? The description
> > at the top implied that we were just missing an call to
> > setup_IO_APIC.. 
> Hi Konrad,
> 	I'm not very familiar with Xen IRQ yet, so I just enabled the
> code to create irqdomains for IOAPICs and keep other part as is. I will
> try to check whether we could enable other part all together:)

If you want you can (privately) send me (or Sander) and patch that I would
be happy to test out. It is quite amusing that I have not used the power
button for quite a while  - I always rebooting the machine to install
a new kernel :-)

> Regards!
> Gerry
> 
> > 
> >>  
> >> -	apic_printk(APIC_VERBOSE, "ENABLING IO-APIC IRQs\n");
> >>  	for_each_ioapic(ioapic)
> >>  		BUG_ON(mp_irqdomain_create(ioapic));
> >> -
> >> -	/*
> >> -         * Set up IO-APIC IRQ routing.
> >> -         */
> >> -	x86_init.mpparse.setup_ioapic_ids();
> >> -
> >> -	sync_Arb_IDs();
> >>  	setup_IO_APIC_irqs();
> >> -	init_IO_APIC_traps();
> >> -	if (nr_legacy_irqs())
> >> -		check_timer();
> >> -
> >>  	ioapic_initialized = 1;
> >> +
> >> +	if (!xen_smp) {
> >> +		init_IO_APIC_traps();
> >> +		if (nr_legacy_irqs())
> >> +			check_timer();
> >> +	}
> >>  }
> >>  
> >>  /*
> >> diff --git a/arch/x86/xen/smp.c b/arch/x86/xen/smp.c
> >> index 4c071aeb8417..7eb0283901fa 100644
> >> --- a/arch/x86/xen/smp.c
> >> +++ b/arch/x86/xen/smp.c
> >> @@ -326,7 +326,10 @@ static void __init xen_smp_prepare_cpus(unsigned int max_cpus)
> >>  
> >>  		xen_raw_printk(m);
> >>  		panic(m);
> >> +	} else {
> >> +		setup_IO_APIC(true);
> >>  	}
> >> +
> >>  	xen_init_lock_cpu(0);
> >>  
> >>  	smp_store_boot_cpu_info();
> >> -- 
> >> 1.7.10.4
> >>
> > --
> > To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> > the body of a message to majordomo@vger.kernel.org
> > More majordomo info at  http://vger.kernel.org/majordomo-info.html
> > Please read the FAQ at  http://www.tux.org/lkml/
> > 

Re: [Bugfix] x86/apic: Fix xen IRQ allocation failure caused by commit b81975eade8c

[Jiang Liu]

On 2015/1/7 23:44, Konrad Rzeszutek Wilk wrote:
> On Wed, Jan 07, 2015 at 11:37:52PM +0800, Jiang Liu wrote:
>> On 2015/1/7 22:50, Konrad Rzeszutek Wilk wrote:
>>> On Wed, Jan 07, 2015 at 02:13:49PM +0800, Jiang Liu wrote:
>>>> Commit b81975eade8c ("x86, irq: Clean up irqdomain transition code")
>>>> breaks xen IRQ allocation because xen_smp_prepare_cpus() doesn't invoke
>>>> setup_IO_APIC(), so no irqdomains created for IOAPICs and
>>>> mp_map_pin_to_irq() fails at the very beginning.
>>>> --- a/arch/x86/kernel/apic/io_apic.c
>>>> +++ b/arch/x86/kernel/apic/io_apic.c
>>>> @@ -2369,31 +2369,29 @@ static void ioapic_destroy_irqdomain(int idx)
>>>>  	ioapics[idx].pin_info = NULL;
>>>>  }
>>>>  
>>>> -void __init setup_IO_APIC(void)
>>>> +void __init setup_IO_APIC(bool xen_smp)
>>>>  {
>>>>  	int ioapic;
>>>>  
>>>> -	/*
>>>> -	 * calling enable_IO_APIC() is moved to setup_local_APIC for BP
>>>> -	 */
>>>> -	io_apic_irqs = nr_legacy_irqs() ? ~PIC_IRQS : ~0UL;
>>>> +	if (!xen_smp) {
>>>> +		apic_printk(APIC_VERBOSE, "ENABLING IO-APIC IRQs\n");
>>>> +		io_apic_irqs = nr_legacy_irqs() ? ~PIC_IRQS : ~0UL;
>>>> +
>>>> +		/* Set up IO-APIC IRQ routing. */
>>>> +		x86_init.mpparse.setup_ioapic_ids();
>>>> +		sync_Arb_IDs();
>>>> +	}
Hi Konrad,
	Enabling above code for Xen dom0 will cause following warning
because it writes a special value to ICR register.
[    3.394981] ------------[ cut here ]------------
[    3.394985] WARNING: CPU: 0 PID: 1 at arch/x86/xen/enlighten.c:968
xen_apic_write+0x15/0x20()
[    3.394988] Modules linked in:
[    3.394991] CPU: 0 PID: 1 Comm: swapper/0 Not tainted 3.19.0-rc3+ #5
[    3.394993] Hardware name: Dell Inc. OptiPlex 9020/0DNKMN, BIOS A03
09/17/2013
[    3.394996]  00000000000003c8 ffff88003056bdd8 ffffffff817611bb
00000000000003c8
[    3.395000]  0000000000000000 ffff88003056be18 ffffffff8106f4ea
0000000000000008
[    3.395004]  ffffffff81fc1120 ffff880030561348 000000000000a108
000000000000a101
[    3.395008] Call Trace:
[    3.395012]  [<ffffffff817611bb>] dump_stack+0x4f/0x6c
[    3.395015]  [<ffffffff8106f4ea>] warn_slowpath_common+0xaa/0xd0
[    3.395018]  [<ffffffff8106f525>] warn_slowpath_null+0x15/0x20
[    3.395021]  [<ffffffff81003e25>] xen_apic_write+0x15/0x20
[    3.395026]  [<ffffffff81ef606b>] sync_Arb_IDs+0x84/0x86
[    3.395029]  [<ffffffff81ef7f7a>] setup_IO_APIC+0x7f/0x8e3
[    3.395033]  [<ffffffff810b275d>] ? trace_hardirqs_on+0xd/0x10
[    3.395036]  [<ffffffff8176858a>] ? _raw_spin_unlock_irqrestore+0x8a/0xa0
[    3.395040]  [<ffffffff81ee841b>] xen_smp_prepare_cpus+0x5d/0x184
[    3.395044]  [<ffffffff81ee1ba3>] kernel_init_freeable+0x149/0x293
[    3.395047]  [<ffffffff81758d49>] ? kernel_init+0x9/0xf0
[    3.395049]  [<ffffffff81758d40>] ? rest_init+0xd0/0xd0
[    3.395052]  [<ffffffff81758d49>] kernel_init+0x9/0xf0
[    3.395054]  [<ffffffff8176887c>] ret_from_fork+0x7c/0xb0
[    3.395057]  [<ffffffff81758d40>] ? rest_init+0xd0/0xd0
[    3.395066] ---[ end trace 7c4371c8ba33d5d0 ]---

<snit>
>>>>  	ioapic_initialized = 1;
>>>> +
>>>> +	if (!xen_smp) {
>>>> +		init_IO_APIC_traps();
>>>> +		if (nr_legacy_irqs())
>>>> +			check_timer();
>>>> +	}
>>>>  }
And enabling above code causes Xen dom0 reboots.
Haven't test HVM and PV kernel yet.
So seems we still need special treatment for xen here.
Regards!
Gerry

>>>>  
>>>>  /*
>>>> diff --git a/arch/x86/xen/smp.c b/arch/x86/xen/smp.c
>>>> index 4c071aeb8417..7eb0283901fa 100644
>>>> --- a/arch/x86/xen/smp.c
>>>> +++ b/arch/x86/xen/smp.c
>>>> @@ -326,7 +326,10 @@ static void __init xen_smp_prepare_cpus(unsigned int max_cpus)
>>>>  
>>>>  		xen_raw_printk(m);
>>>>  		panic(m);
>>>> +	} else {
>>>> +		setup_IO_APIC(true);
>>>>  	}
>>>> +
>>>>  	xen_init_lock_cpu(0);
>>>>  
>>>>  	smp_store_boot_cpu_info();
>>>> -- 
>>>> 1.7.10.4
>>>>
>>> --
>>> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
>>> the body of a message to majordomo@vger.kernel.org
>>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>>> Please read the FAQ at  http://www.tux.org/lkml/
>>>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/
> 

Re: [Bugfix] x86/apic: Fix xen IRQ allocation failure caused by commit b81975eade8c

[Konrad Rzeszutek Wilk]

On Thu, Jan 08, 2015 at 02:36:38PM +0800, Jiang Liu wrote:
> On 2015/1/7 23:44, Konrad Rzeszutek Wilk wrote:
> > On Wed, Jan 07, 2015 at 11:37:52PM +0800, Jiang Liu wrote:
> >> On 2015/1/7 22:50, Konrad Rzeszutek Wilk wrote:
> >>> On Wed, Jan 07, 2015 at 02:13:49PM +0800, Jiang Liu wrote:
> >>>> Commit b81975eade8c ("x86, irq: Clean up irqdomain transition code")
> >>>> breaks xen IRQ allocation because xen_smp_prepare_cpus() doesn't invoke
> >>>> setup_IO_APIC(), so no irqdomains created for IOAPICs and
> >>>> mp_map_pin_to_irq() fails at the very beginning.
> >>>> --- a/arch/x86/kernel/apic/io_apic.c
> >>>> +++ b/arch/x86/kernel/apic/io_apic.c
> >>>> @@ -2369,31 +2369,29 @@ static void ioapic_destroy_irqdomain(int idx)
> >>>>  	ioapics[idx].pin_info = NULL;
> >>>>  }
> >>>>  
> >>>> -void __init setup_IO_APIC(void)
> >>>> +void __init setup_IO_APIC(bool xen_smp)
> >>>>  {
> >>>>  	int ioapic;
> >>>>  
> >>>> -	/*
> >>>> -	 * calling enable_IO_APIC() is moved to setup_local_APIC for BP
> >>>> -	 */
> >>>> -	io_apic_irqs = nr_legacy_irqs() ? ~PIC_IRQS : ~0UL;
> >>>> +	if (!xen_smp) {
> >>>> +		apic_printk(APIC_VERBOSE, "ENABLING IO-APIC IRQs\n");
> >>>> +		io_apic_irqs = nr_legacy_irqs() ? ~PIC_IRQS : ~0UL;
> >>>> +
> >>>> +		/* Set up IO-APIC IRQ routing. */
> >>>> +		x86_init.mpparse.setup_ioapic_ids();
> >>>> +		sync_Arb_IDs();
> >>>> +	}
> Hi Konrad,
> 	Enabling above code for Xen dom0 will cause following warning
> because it writes a special value to ICR register.
> [    3.394981] ------------[ cut here ]------------
> [    3.394985] WARNING: CPU: 0 PID: 1 at arch/x86/xen/enlighten.c:968
> xen_apic_write+0x15/0x20()
> [    3.394988] Modules linked in:
> [    3.394991] CPU: 0 PID: 1 Comm: swapper/0 Not tainted 3.19.0-rc3+ #5
> [    3.394993] Hardware name: Dell Inc. OptiPlex 9020/0DNKMN, BIOS A03
> 09/17/2013
> [    3.394996]  00000000000003c8 ffff88003056bdd8 ffffffff817611bb
> 00000000000003c8
> [    3.395000]  0000000000000000 ffff88003056be18 ffffffff8106f4ea
> 0000000000000008
> [    3.395004]  ffffffff81fc1120 ffff880030561348 000000000000a108
> 000000000000a101
> [    3.395008] Call Trace:
> [    3.395012]  [<ffffffff817611bb>] dump_stack+0x4f/0x6c
> [    3.395015]  [<ffffffff8106f4ea>] warn_slowpath_common+0xaa/0xd0
> [    3.395018]  [<ffffffff8106f525>] warn_slowpath_null+0x15/0x20
> [    3.395021]  [<ffffffff81003e25>] xen_apic_write+0x15/0x20
> [    3.395026]  [<ffffffff81ef606b>] sync_Arb_IDs+0x84/0x86
> [    3.395029]  [<ffffffff81ef7f7a>] setup_IO_APIC+0x7f/0x8e3
> [    3.395033]  [<ffffffff810b275d>] ? trace_hardirqs_on+0xd/0x10
> [    3.395036]  [<ffffffff8176858a>] ? _raw_spin_unlock_irqrestore+0x8a/0xa0
> [    3.395040]  [<ffffffff81ee841b>] xen_smp_prepare_cpus+0x5d/0x184
> [    3.395044]  [<ffffffff81ee1ba3>] kernel_init_freeable+0x149/0x293
> [    3.395047]  [<ffffffff81758d49>] ? kernel_init+0x9/0xf0
> [    3.395049]  [<ffffffff81758d40>] ? rest_init+0xd0/0xd0
> [    3.395052]  [<ffffffff81758d49>] kernel_init+0x9/0xf0
> [    3.395054]  [<ffffffff8176887c>] ret_from_fork+0x7c/0xb0
> [    3.395057]  [<ffffffff81758d40>] ? rest_init+0xd0/0xd0
> [    3.395066] ---[ end trace 7c4371c8ba33d5d0 ]---
> 
> <snit>
> >>>>  	ioapic_initialized = 1;
> >>>> +
> >>>> +	if (!xen_smp) {
> >>>> +		init_IO_APIC_traps();
> >>>> +		if (nr_legacy_irqs())
> >>>> +			check_timer();
> >>>> +	}
> >>>>  }
> And enabling above code causes Xen dom0 reboots.


Which is due to the 'check_timer' trying to setup its timer and
failing and then moving under its feet the legacy_pic to the NULL one
and then hitting panic.

The 'check_timer' has the logic to swap the 'legacy_pic':

2186         legacy_pic->init(1);                                                    

which ends up executing:

317         new_val = inb(PIC_MASTER_IMR);                                          
318         if (new_val != probe_val) {                                             
319                 printk(KERN_INFO "Using NULL legacy PIC\n");                    
320                 legacy_pic = &null_legacy_pic;                                  
321                 raw_spin_unlock_irqrestore(&i8259A_lock, flags);                
322                 return;                                                         
323         }                                         

And the 'legacy_pic' has now be swapped over to the 'null_legacy_pic'
for which:

2393         if (nr_legacy_irqs())                                                   
2394                 check_timer();                                                  
2395                                                                                 

 70 static inline int nr_legacy_irqs(void)                                          
 71 {                                                                               
 72         return legacy_pic->nr_legacy_irqs;                                      
 73 }                                                                               
 74            

would return zero (and not invoke the 'check_timer'), but because
we do make the check inside the 'check_timer' we continue on.

Perhaps something like this?

diff --git a/arch/x86/kernel/apic/io_apic.c b/arch/x86/kernel/apic/io_apic.c
index 3f5f604..e474389 100644
--- a/arch/x86/kernel/apic/io_apic.c
+++ b/arch/x86/kernel/apic/io_apic.c
@@ -2184,6 +2184,14 @@ static inline void __init check_timer(void)
 	 */
 	apic_write(APIC_LVT0, APIC_LVT_MASKED | APIC_DM_EXTINT);
 	legacy_pic->init(1);
+	/*
+	 * The init swapped out the legacy_pic to point to the NULL one.
+	 * As such we should not even have entered this init routine
+	 * (which depends on ->nr_legacy_irqs having an non-zero value
+	 * and null_legacy_pic has zero.
+	 */
+	if (legacy_pic == null_legacy_pic)
+		goto out;
 
 	pin1  = find_isa_irq_pin(0, mp_INT);
 	apic1 = find_isa_irq_apic(0, mp_INT);
diff --git a/arch/x86/xen/smp.c b/arch/x86/xen/smp.c
index 4c071ae..9f404df 100644
--- a/arch/x86/xen/smp.c
+++ b/arch/x86/xen/smp.c
@@ -327,6 +327,7 @@ static void __init xen_smp_prepare_cpus(unsigned int max_cpus)
 		xen_raw_printk(m);
 		panic(m);
 	}
+	setup_IO_APIC();
 	xen_init_lock_cpu(0);
 
 	smp_store_boot_cpu_info();

The patch of course ignores the WARN which woudl need something
else.

> Haven't test HVM and PV kernel yet.
> So seems we still need special treatment for xen here.
> Regards!
> Gerry
> 
> >>>>  
> >>>>  /*
> >>>> diff --git a/arch/x86/xen/smp.c b/arch/x86/xen/smp.c
> >>>> index 4c071aeb8417..7eb0283901fa 100644
> >>>> --- a/arch/x86/xen/smp.c
> >>>> +++ b/arch/x86/xen/smp.c
> >>>> @@ -326,7 +326,10 @@ static void __init xen_smp_prepare_cpus(unsigned int max_cpus)
> >>>>  
> >>>>  		xen_raw_printk(m);
> >>>>  		panic(m);
> >>>> +	} else {
> >>>> +		setup_IO_APIC(true);
> >>>>  	}
> >>>> +
> >>>>  	xen_init_lock_cpu(0);
> >>>>  
> >>>>  	smp_store_boot_cpu_info();
> >>>> -- 
> >>>> 1.7.10.4
> >>>>
> >>> --
> >>> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> >>> the body of a message to majordomo@vger.kernel.org
> >>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> >>> Please read the FAQ at  http://www.tux.org/lkml/
> >>>
> > --
> > To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> > the body of a message to majordomo@vger.kernel.org
> > More majordomo info at  http://vger.kernel.org/majordomo-info.html
> > Please read the FAQ at  http://www.tux.org/lkml/
> > 

Re: [Bugfix] x86/apic: Fix xen IRQ allocation failure caused by commit b81975eade8c

[Sander Eikelenboom]

Wednesday, January 7, 2015, 7:13:49 AM, you wrote:

> Commit b81975eade8c ("x86, irq: Clean up irqdomain transition code")
> breaks xen IRQ allocation because xen_smp_prepare_cpus() doesn't invoke
> setup_IO_APIC(), so no irqdomains created for IOAPICs and
> mp_map_pin_to_irq() fails at the very beginning.

> Enhance xen_smp_prepare_cpus() to call setup_IO_APIC() to initialize
> irqdomain for IOAPICs.

> Signed-off-by: Jiang Liu <jiang.liu@linux.intel.com>
> Reported-and-tested-by: Sander Eikelenboom <linux@eikelenboom.it>
> Cc: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
> ---
> Hi all,
>         This patch should be backported to v3.17, but there are
> conflicts. So I will send backported patch to 3.17/3.18 stable tree
> once this patch has been merged into mainstream kernel.
> Thanks!
> Gerry

Hi Gerry / Konrad / Thomas,

This patch doesn't apply cleanly to current linux-tip.

Unfortunately the "Tested-by" seems only valid for the intel hardware i have (intel NUC).
Testing on AMD delivered some interesting results:

- Under Xen: Host freeze early in dom0 kernel boot, unfortunately no more info.

- On baremetal with iommu enabled and ivrs_ioapic[6]=00:14.0 ivrs_hpet[0]=00:14.0
  as commandline override for a borked bios:
  It doesn't boot and spits out:
  
  [    0.339811] AMD-Vi: Command-line override present for HPET id 0 - ignoring
  [    0.460563] ..TIMER: vector=0x30 apic1=0 pin1=2 apic2=-1 pin2=-1
  [    0.511535] Kernel panic - not syncing: timer doesn't work through Interrupt-remapped IO-APIC
  [    0.537042] CPU: 0 PID: 1 Comm: swapper/0 Not tainted 3.19.0-rc3-20150110-pciback-doflr-apic-fixed+ #1
  [    0.564887] Hardware name: MSI MS-7640/890FXA-GD70 (MS-7640)  , BIOS V1.8B1 09/13/2010
  [    0.588558]  ffff88054b00c000 ffff880547d8fda8 ffffffff81bb3fc9 ffff880547db0000
  [    0.610722]  ffffffff81f20070 ffff880547d8fe28 ffffffff81baeeb9 ffff880547d8fde8
  [    0.632886]  ffffffff00000008 ffff880547d8fe38 ffff880547d8fdd8 00000000fffea093
  [    0.655052] Call Trace:
  [    0.662349]  [<ffffffff81bb3fc9>] dump_stack+0x45/0x57
  [    0.677712]  [<ffffffff81baeeb9>] panic+0xcd/0x212
  [    0.692026]  [<ffffffff81583187>] panic_if_irq_remap+0x17/0x20
  [    0.709460]  [<ffffffff8233c634>] setup_IO_APIC+0x2bb/0x74c
  [    0.726112]  [<ffffffff8233a04a>] native_smp_prepare_cpus+0x2c9/0x35a
  [    0.745365]  [<ffffffff82328200>] kernel_init_freeable+0x153/0x298
  [    0.763839]  [<ffffffff81baa589>] ? kernel_init+0x9/0xf0
  [    0.779712]  [<ffffffff810e82bb>] ? finish_task_switch+0x8b/0x100
  [    0.797937]  [<ffffffff81baa580>] ? rest_init+0xc0/0xc0
  [    0.813549]  [<ffffffff81baa589>] kernel_init+0x9/0xf0
  [    0.828904]  [<ffffffff81bbf8bc>] ret_from_fork+0x7c/0xb0
  [    0.845036]  [<ffffffff81baa580>] ? rest_init+0xc0/0xc0
  [    0.860660] ---[ end Kernel panic - not syncing: timer doesn't work through Interrupt-remapped IO-APIC


- On baremetal with iommu enabled and without the commandline overrides:
  It boots, but iommu is disabled (as expected) but i also get this lockdep trace:

  [    0.339808] [Firmware Bug]: AMD-Vi: IOAPIC[6] not in IVRS table
  [    0.357519] [Firmware Bug]: AMD-Vi: No southbridge IOAPIC found
  [    0.375220] AMD-Vi: Disabling interrupt remapping
  [    0.389723] ..TIMER: vector=0x30 apic1=0 pin1=2 apic2=-1 pin2=-1
  [    0.440685] ..MP-BIOS bug: 8254 timer not connected to IO-APIC
  [    0.458128] ...trying to set up timer (IRQ0) through the 8259A ...
  [    0.476602] ..... (found apic 0 pin 2) ...
  [    0.488834] ------------[ cut here ]------------
  [    0.502631] WARNING: CPU: 0 PID: 1 at kernel/locking/lockdep.c:2744 lockdep_trace_alloc+0x12c/0x140()
  [    0.530215] DEBUG_LOCKS_WARN_ON(irqs_disabled_flags(flags))
  [    0.546347] Modules linked in:
  [    0.556006] CPU: 0 PID: 1 Comm: swapper/0 Not tainted 3.19.0-rc3-20150110-pciback-doflr-apic-fixed+ #1
  [    0.583839] Hardware name: MSI MS-7640/890FXA-GD70 (MS-7640)  , BIOS V1.8B1 09/13/2010
  [    0.607512]  ffffffff81f2408e ffff880547d8fcd8 ffffffff81bb3fc9 ffff880547db0000
  [    0.629675]  ffff880547d8fd28 ffff880547d8fd18 ffffffff810c738d ffff880547d8fd08
  [    0.651840]  ffff880547db0000 0000000000000086 0000000000000080 0000000000000000
  [    0.674004] Call Trace:
  [    0.681302]  [<ffffffff81bb3fc9>] dump_stack+0x45/0x57
  [    0.696653]  [<ffffffff810c738d>] warn_slowpath_common+0x8d/0xd0
  [    0.714605]  [<ffffffff810c7471>] warn_slowpath_fmt+0x41/0x50
  [    0.731780]  [<ffffffff8110e2c2>] ? vprintk_emit+0x312/0x5f0
  [    0.748692]  [<ffffffff8110229c>] lockdep_trace_alloc+0x12c/0x140
  [    0.766906]  [<ffffffff811b5a9f>] kmem_cache_alloc_node+0x3f/0x160
  [    0.785379]  [<ffffffff8104573c>] ? __add_pin_to_irq_node+0x6c/0xc0
  [    0.804112]  [<ffffffff8104573c>] __add_pin_to_irq_node+0x6c/0xc0
  [    0.822326]  [<ffffffff8233c6c8>] setup_IO_APIC+0x34f/0x74c
  [    0.838990]  [<ffffffff8233a04a>] native_smp_prepare_cpus+0x2c9/0x35a
  [    0.858243]  [<ffffffff82328200>] kernel_init_freeable+0x153/0x298
  [    0.876719]  [<ffffffff81baa589>] ? kernel_init+0x9/0xf0
  [    0.892602]  [<ffffffff810e82bb>] ? finish_task_switch+0x8b/0x100
  [    0.910815]  [<ffffffff81baa580>] ? rest_init+0xc0/0xc0
  [    0.926428]  [<ffffffff81baa589>] kernel_init+0x9/0xf0
  [    0.941782]  [<ffffffff81bbf8bc>] ret_from_fork+0x7c/0xb0
  [    0.957913]  [<ffffffff81baa580>] ? rest_init+0xc0/0xc0
  [    0.973531] ---[ end trace 5f14749f8239057a ]---
  [    1.020338] ....... works.

Without this patch applied i don't see these issues (with or without the 
override).

.config and dmesg of a baremetal boot without the commandline override is attached.

--
Sander







> ---
>  arch/x86/include/asm/io_apic.h       |    2 +-
>  arch/x86/include/asm/smpboot_hooks.h |    5 ++---
>  arch/x86/kernel/apic/apic.c          |    5 ++---
>  arch/x86/kernel/apic/io_apic.c       |   32 +++++++++++++++-----------------
>  arch/x86/xen/smp.c                   |    3 +++
>  5 files changed, 23 insertions(+), 24 deletions(-)

> diff --git a/arch/x86/include/asm/io_apic.h b/arch/x86/include/asm/io_apic.h
> index bf006cce9418..72a1298658bb 100644
> --- a/arch/x86/include/asm/io_apic.h
> +++ b/arch/x86/include/asm/io_apic.h
> @@ -237,7 +237,7 @@ static inline void io_apic_modify(unsigned int apic, unsigned int reg, unsigned
>  
>  extern void io_apic_eoi(unsigned int apic, unsigned int vector);
>  
> -extern void setup_IO_APIC(void);
> +extern void setup_IO_APIC(bool xen_smp);
>  extern void enable_IO_APIC(void);
>  extern void disable_IO_APIC(void);
>  extern void setup_ioapic_dest(void);
> diff --git a/arch/x86/include/asm/smpboot_hooks.h b/arch/x86/include/asm/smpboot_hooks.h
> index 0da7409f0bec..e47df710a588 100644
> --- a/arch/x86/include/asm/smpboot_hooks.h
> +++ b/arch/x86/include/asm/smpboot_hooks.h
> @@ -53,10 +53,9 @@ static inline void __init smpboot_setup_io_apic(void)
>          * go and set it up:
>          */
>         if (!skip_ioapic_setup && nr_ioapics)
> -               setup_IO_APIC();
> -       else {
> +               setup_IO_APIC(false);
> +       else
>                 nr_ioapics = 0;
> -       }
>  #endif
>  }
>  
> diff --git a/arch/x86/kernel/apic/apic.c b/arch/x86/kernel/apic/apic.c
> index 29b5b18afa27..71b8a6cb7f0e 100644
> --- a/arch/x86/kernel/apic/apic.c
> +++ b/arch/x86/kernel/apic/apic.c
> @@ -1913,10 +1913,9 @@ int __init APIC_init_uniprocessor(void)
>  
>  #ifdef CONFIG_X86_IO_APIC
>         if (smp_found_config && !skip_ioapic_setup && nr_ioapics)
> -               setup_IO_APIC();
> -       else {
> +               setup_IO_APIC(false);
> +       else
>                 nr_ioapics = 0;
> -       }
>  #endif
>  
>         x86_init.timers.setup_percpu_clockev();
> diff --git a/arch/x86/kernel/apic/io_apic.c b/arch/x86/kernel/apic/io_apic.c
> index 3f5f60406ab1..13cddc75e4c0 100644
> --- a/arch/x86/kernel/apic/io_apic.c
> +++ b/arch/x86/kernel/apic/io_apic.c
> @@ -2369,31 +2369,29 @@ static void ioapic_destroy_irqdomain(int idx)
>         ioapics[idx].pin_info = NULL;
>  }
>  
> -void __init setup_IO_APIC(void)
> +void __init setup_IO_APIC(bool xen_smp)
>  {
>         int ioapic;
>  
> -       /*
> -        * calling enable_IO_APIC() is moved to setup_local_APIC for BP
> -        */
> -       io_apic_irqs = nr_legacy_irqs() ? ~PIC_IRQS : ~0UL;
> +       if (!xen_smp) {
> +               apic_printk(APIC_VERBOSE, "ENABLING IO-APIC IRQs\n");
> +               io_apic_irqs = nr_legacy_irqs() ? ~PIC_IRQS : ~0UL;
> +
> +               /* Set up IO-APIC IRQ routing. */
> +               x86_init.mpparse.setup_ioapic_ids();
> +               sync_Arb_IDs();
> +       }
>  
> -       apic_printk(APIC_VERBOSE, "ENABLING IO-APIC IRQs\n");
>         for_each_ioapic(ioapic)
>                 BUG_ON(mp_irqdomain_create(ioapic));
> -
> -       /*
> -         * Set up IO-APIC IRQ routing.
> -         */
> -       x86_init.mpparse.setup_ioapic_ids();
> -
> -       sync_Arb_IDs();
>         setup_IO_APIC_irqs();
> -       init_IO_APIC_traps();
> -       if (nr_legacy_irqs())
> -               check_timer();
> -
>         ioapic_initialized = 1;
> +
> +       if (!xen_smp) {
> +               init_IO_APIC_traps();
> +               if (nr_legacy_irqs())
> +                       check_timer();
> +       }
>  }
>  
>  /*
> diff --git a/arch/x86/xen/smp.c b/arch/x86/xen/smp.c
> index 4c071aeb8417..7eb0283901fa 100644
> --- a/arch/x86/xen/smp.c
> +++ b/arch/x86/xen/smp.c
> @@ -326,7 +326,10 @@ static void __init xen_smp_prepare_cpus(unsigned int max_cpus)
>  
>                 xen_raw_printk(m);
>                 panic(m);
> +       } else {
> +               setup_IO_APIC(true);
>         }
> +
>         xen_init_lock_cpu(0);
>  
>         smp_store_boot_cpu_info();