* [Bugfix] x86/apic: Fix xen IRQ allocation failure caused by commit b81975eade8c
@ 2015-01-07 6:13 Jiang Liu
2015-01-07 14:50 ` Konrad Rzeszutek Wilk
2015-01-10 19:12 ` Sander Eikelenboom
0 siblings, 2 replies; 7+ messages in thread
From: Jiang Liu @ 2015-01-07 6:13 UTC (permalink / raw)
To: linux-arm-kernel
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
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [Bugfix] x86/apic: Fix xen IRQ allocation failure caused by commit b81975eade8c
2015-01-07 6:13 [Bugfix] x86/apic: Fix xen IRQ allocation failure caused by commit b81975eade8c Jiang Liu
@ 2015-01-07 14:50 ` Konrad Rzeszutek Wilk
2015-01-07 15:37 ` Jiang Liu
2015-01-10 19:12 ` Sander Eikelenboom
1 sibling, 1 reply; 7+ messages in thread
From: Konrad Rzeszutek Wilk @ 2015-01-07 14:50 UTC (permalink / raw)
To: linux-arm-kernel
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
>
^ permalink raw reply [flat|nested] 7+ messages in thread
* [Bugfix] x86/apic: Fix xen IRQ allocation failure caused by commit b81975eade8c
2015-01-07 14:50 ` Konrad Rzeszutek Wilk
@ 2015-01-07 15:37 ` Jiang Liu
2015-01-07 15:44 ` Konrad Rzeszutek Wilk
0 siblings, 1 reply; 7+ messages in thread
From: Jiang Liu @ 2015-01-07 15:37 UTC (permalink / raw)
To: linux-arm-kernel
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 at vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/
>
^ permalink raw reply [flat|nested] 7+ messages in thread
* [Bugfix] x86/apic: Fix xen IRQ allocation failure caused by commit b81975eade8c
2015-01-07 15:37 ` Jiang Liu
@ 2015-01-07 15:44 ` Konrad Rzeszutek Wilk
2015-01-08 6:36 ` Jiang Liu
0 siblings, 1 reply; 7+ messages in thread
From: Konrad Rzeszutek Wilk @ 2015-01-07 15:44 UTC (permalink / raw)
To: linux-arm-kernel
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 at vger.kernel.org
> > More majordomo info at http://vger.kernel.org/majordomo-info.html
> > Please read the FAQ at http://www.tux.org/lkml/
> >
^ permalink raw reply [flat|nested] 7+ messages in thread
* [Bugfix] x86/apic: Fix xen IRQ allocation failure caused by commit b81975eade8c
2015-01-07 15:44 ` Konrad Rzeszutek Wilk
@ 2015-01-08 6:36 ` Jiang Liu
2015-01-09 21:15 ` Konrad Rzeszutek Wilk
0 siblings, 1 reply; 7+ messages in thread
From: Jiang Liu @ 2015-01-08 6:36 UTC (permalink / raw)
To: linux-arm-kernel
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 at 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 at vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/
>
^ permalink raw reply [flat|nested] 7+ messages in thread
* [Bugfix] x86/apic: Fix xen IRQ allocation failure caused by commit b81975eade8c
2015-01-08 6:36 ` Jiang Liu
@ 2015-01-09 21:15 ` Konrad Rzeszutek Wilk
0 siblings, 0 replies; 7+ messages in thread
From: Konrad Rzeszutek Wilk @ 2015-01-09 21:15 UTC (permalink / raw)
To: linux-arm-kernel
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 at 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 at vger.kernel.org
> > More majordomo info at http://vger.kernel.org/majordomo-info.html
> > Please read the FAQ at http://www.tux.org/lkml/
> >
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [Bugfix] x86/apic: Fix xen IRQ allocation failure caused by commit b81975eade8c
2015-01-07 6:13 [Bugfix] x86/apic: Fix xen IRQ allocation failure caused by commit b81975eade8c Jiang Liu
2015-01-07 14:50 ` Konrad Rzeszutek Wilk
@ 2015-01-10 19:12 ` Sander Eikelenboom
1 sibling, 0 replies; 7+ messages in thread
From: Sander Eikelenboom @ 2015-01-10 19:12 UTC (permalink / raw)
To: linux-arm-kernel
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();
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2015-01-10 19:12 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2015-01-07 6:13 [Bugfix] x86/apic: Fix xen IRQ allocation failure caused by commit b81975eade8c Jiang Liu
2015-01-07 14:50 ` Konrad Rzeszutek Wilk
2015-01-07 15:37 ` Jiang Liu
2015-01-07 15:44 ` Konrad Rzeszutek Wilk
2015-01-08 6:36 ` Jiang Liu
2015-01-09 21:15 ` Konrad Rzeszutek Wilk
2015-01-10 19:12 ` Sander Eikelenboom
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).