[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

[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
> 

[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

[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/
> 

[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 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/
> 

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/
> > 

[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 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/
> > 

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/
> 

[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 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/
> 

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/
> > 

[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 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/
> > 

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();

[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();

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();

[PATCH v2] [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>
---
Hi Sander,
	Could you please help to test this patch against 3.19-rc3?
I have reworked it based Konrad's suggestions.
Thanks!
Gerry
---
 arch/x86/kernel/apic/io_apic.c |   31 +++++++++++++++++++++++--------
 arch/x86/xen/smp.c             |    1 +
 2 files changed, 24 insertions(+), 8 deletions(-)

diff --git a/arch/x86/kernel/apic/io_apic.c b/arch/x86/kernel/apic/io_apic.c
index 3f5f60406ab1..81d4faeb8040 100644
--- a/arch/x86/kernel/apic/io_apic.c
+++ b/arch/x86/kernel/apic/io_apic.c
@@ -38,6 +38,8 @@
 #include <linux/slab.h>
 #include <linux/bootmem.h>
 
+#include <xen/xen.h>
+
 #include <asm/idle.h>
 #include <asm/io.h>
 #include <asm/smp.h>
@@ -2165,6 +2167,9 @@ static inline void __init check_timer(void)
 	unsigned long flags;
 	int no_pin1 = 0;
 
+	if (!nr_legacy_irqs())
+		return;
+
 	local_irq_save(flags);
 
 	/*
@@ -2185,6 +2190,13 @@ static inline void __init check_timer(void)
 	apic_write(APIC_LVT0, APIC_LVT_MASKED | APIC_DM_EXTINT);
 	legacy_pic->init(1);
 
+	/*
+	 * legacy_pic will be changed to null_legacy_pic if init() fails to
+	 * to detect legacy PIC hardware. Recheck again.
+	 */
+	if (!nr_legacy_irqs())
+		goto out;
+
 	pin1  = find_isa_irq_pin(0, mp_INT);
 	apic1 = find_isa_irq_apic(0, mp_INT);
 	pin2  = ioapic_i8259.pin;
@@ -2369,6 +2381,15 @@ static void ioapic_destroy_irqdomain(int idx)
 	ioapics[idx].pin_info = NULL;
 }
 
+static void setup_IO_APIC_IDs(void)
+{
+	if (xen_domain())
+		return;
+
+	x86_init.mpparse.setup_ioapic_ids();
+	sync_Arb_IDs();
+}
+
 void __init setup_IO_APIC(void)
 {
 	int ioapic;
@@ -2382,16 +2403,10 @@ void __init setup_IO_APIC(void)
 	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_IDs();
 	setup_IO_APIC_irqs();
 	init_IO_APIC_traps();
-	if (nr_legacy_irqs())
-		check_timer();
+	check_timer();
 
 	ioapic_initialized = 1;
 }
diff --git a/arch/x86/xen/smp.c b/arch/x86/xen/smp.c
index 4c071aeb8417..9f404df64422 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();
-- 
1.7.10.4

[PATCH v2] [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>
---
Hi Sander,
	Could you please help to test this patch against 3.19-rc3?
I have reworked it based Konrad's suggestions.
Thanks!
Gerry
---
 arch/x86/kernel/apic/io_apic.c |   31 +++++++++++++++++++++++--------
 arch/x86/xen/smp.c             |    1 +
 2 files changed, 24 insertions(+), 8 deletions(-)

diff --git a/arch/x86/kernel/apic/io_apic.c b/arch/x86/kernel/apic/io_apic.c
index 3f5f60406ab1..81d4faeb8040 100644
--- a/arch/x86/kernel/apic/io_apic.c
+++ b/arch/x86/kernel/apic/io_apic.c
@@ -38,6 +38,8 @@
 #include <linux/slab.h>
 #include <linux/bootmem.h>
 
+#include <xen/xen.h>
+
 #include <asm/idle.h>
 #include <asm/io.h>
 #include <asm/smp.h>
@@ -2165,6 +2167,9 @@ static inline void __init check_timer(void)
 	unsigned long flags;
 	int no_pin1 = 0;
 
+	if (!nr_legacy_irqs())
+		return;
+
 	local_irq_save(flags);
 
 	/*
@@ -2185,6 +2190,13 @@ static inline void __init check_timer(void)
 	apic_write(APIC_LVT0, APIC_LVT_MASKED | APIC_DM_EXTINT);
 	legacy_pic->init(1);
 
+	/*
+	 * legacy_pic will be changed to null_legacy_pic if init() fails to
+	 * to detect legacy PIC hardware. Recheck again.
+	 */
+	if (!nr_legacy_irqs())
+		goto out;
+
 	pin1  = find_isa_irq_pin(0, mp_INT);
 	apic1 = find_isa_irq_apic(0, mp_INT);
 	pin2  = ioapic_i8259.pin;
@@ -2369,6 +2381,15 @@ static void ioapic_destroy_irqdomain(int idx)
 	ioapics[idx].pin_info = NULL;
 }
 
+static void setup_IO_APIC_IDs(void)
+{
+	if (xen_domain())
+		return;
+
+	x86_init.mpparse.setup_ioapic_ids();
+	sync_Arb_IDs();
+}
+
 void __init setup_IO_APIC(void)
 {
 	int ioapic;
@@ -2382,16 +2403,10 @@ void __init setup_IO_APIC(void)
 	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_IDs();
 	setup_IO_APIC_irqs();
 	init_IO_APIC_traps();
-	if (nr_legacy_irqs())
-		check_timer();
+	check_timer();
 
 	ioapic_initialized = 1;
 }
diff --git a/arch/x86/xen/smp.c b/arch/x86/xen/smp.c
index 4c071aeb8417..9f404df64422 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();
-- 
1.7.10.4

Re: [Xen-devel] [PATCH v2] [Bugfix] x86/apic: Fix xen IRQ allocation failure caused by commit b81975eade8c

[David Vrabel]

On 12/01/15 13:39, 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.
> 
> Enhance xen_smp_prepare_cpus() to call setup_IO_APIC() to initialize
> irqdomain for IOAPICs.

Having Xen call setup_IO_APIC() to initialize the irq domains then having to
add special cases to it is just wrong.

The bits of init deferred by mp_register_apic() are also deferred to
two different places which looks odd.

What about something like the following (untested) patch?

diff --git a/arch/x86/kernel/apic/io_apic.c b/arch/x86/kernel/apic/io_apic.c
index 3f5f604..e180680 100644
--- a/arch/x86/kernel/apic/io_apic.c
+++ b/arch/x86/kernel/apic/io_apic.c
@@ -253,8 +253,10 @@ int __init arch_early_ioapic_init(void)
 	if (!nr_legacy_irqs())
 		io_apic_irqs = ~0UL;
 
-	for_each_ioapic(i)
+	for_each_ioapic(i) {
+		BUG_ON(mp_irqdomain_create(ioapic));
 		alloc_ioapic_saved_registers(i);
+	}
 
 	/*
 	 * For legacy IRQ's, start with assigning irq0 to irq15 to
@@ -2379,8 +2381,6 @@ void __init setup_IO_APIC(void)
 	io_apic_irqs = nr_legacy_irqs() ? ~PIC_IRQS : ~0UL;
 
 	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.
@@ -2929,7 +2929,8 @@ int mp_register_ioapic(int id, u32 address, u32 gsi_base,
 	/*
 	 * If mp_register_ioapic() is called during early boot stage when
 	 * walking ACPI/SFI/DT tables, it's too early to create irqdomain,
-	 * we are still using bootmem allocator. So delay it to setup_IO_APIC().
+	 * we are still using bootmem allocator. So delay it to
+	 * arch_early_ioapic_init().
 	 */
 	if (hotplug) {
 		if (mp_irqdomain_create(idx)) {

> --- a/arch/x86/kernel/apic/io_apic.c
> +++ b/arch/x86/kernel/apic/io_apic.c
> @@ -2369,6 +2381,15 @@ static void ioapic_destroy_irqdomain(int idx)
>  	ioapics[idx].pin_info = NULL;
>  }
>  
> +static void setup_IO_APIC_IDs(void)
> +{
> +	if (xen_domain())
> +		return;

This would have to xen_pv_domain().

David

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

[David Vrabel]

On 12/01/15 13:39, 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.
> 
> Enhance xen_smp_prepare_cpus() to call setup_IO_APIC() to initialize
> irqdomain for IOAPICs.

Having Xen call setup_IO_APIC() to initialize the irq domains then having to
add special cases to it is just wrong.

The bits of init deferred by mp_register_apic() are also deferred to
two different places which looks odd.

What about something like the following (untested) patch?

diff --git a/arch/x86/kernel/apic/io_apic.c b/arch/x86/kernel/apic/io_apic.c
index 3f5f604..e180680 100644
--- a/arch/x86/kernel/apic/io_apic.c
+++ b/arch/x86/kernel/apic/io_apic.c
@@ -253,8 +253,10 @@ int __init arch_early_ioapic_init(void)
 	if (!nr_legacy_irqs())
 		io_apic_irqs = ~0UL;
 
-	for_each_ioapic(i)
+	for_each_ioapic(i) {
+		BUG_ON(mp_irqdomain_create(ioapic));
 		alloc_ioapic_saved_registers(i);
+	}
 
 	/*
 	 * For legacy IRQ's, start with assigning irq0 to irq15 to
@@ -2379,8 +2381,6 @@ void __init setup_IO_APIC(void)
 	io_apic_irqs = nr_legacy_irqs() ? ~PIC_IRQS : ~0UL;
 
 	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.
@@ -2929,7 +2929,8 @@ int mp_register_ioapic(int id, u32 address, u32 gsi_base,
 	/*
 	 * If mp_register_ioapic() is called during early boot stage when
 	 * walking ACPI/SFI/DT tables, it's too early to create irqdomain,
-	 * we are still using bootmem allocator. So delay it to setup_IO_APIC().
+	 * we are still using bootmem allocator. So delay it to
+	 * arch_early_ioapic_init().
 	 */
 	if (hotplug) {
 		if (mp_irqdomain_create(idx)) {

> --- a/arch/x86/kernel/apic/io_apic.c
> +++ b/arch/x86/kernel/apic/io_apic.c
> @@ -2369,6 +2381,15 @@ static void ioapic_destroy_irqdomain(int idx)
>  	ioapics[idx].pin_info = NULL;
>  }
>  
> +static void setup_IO_APIC_IDs(void)
> +{
> +	if (xen_domain())
> +		return;

This would have to xen_pv_domain().

David

Re: [Xen-devel] [PATCH v2] [Bugfix] x86/apic: Fix xen IRQ allocation failure caused by commit b81975eade8c

[Sander Eikelenboom]

Monday, January 12, 2015, 4:01:00 PM, you wrote:

> On 12/01/15 13:39, 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.
>> 
>> Enhance xen_smp_prepare_cpus() to call setup_IO_APIC() to initialize
>> irqdomain for IOAPICs.

> Having Xen call setup_IO_APIC() to initialize the irq domains then having to
> add special cases to it is just wrong.

> The bits of init deferred by mp_register_apic() are also deferred to
> two different places which looks odd.

> What about something like the following (untested) patch?

Hi David / Gerry,

David's patch (after fixing a few compile issues) fixes the problem.

The power button now works for me on:
- intel baremetal
- intel xen
- amd baremetal (no issues with the override anymore)
- amd xen       (no freeze issues anymore)

Big thanks David !

--
Sander


> diff --git a/arch/x86/kernel/apic/io_apic.c b/arch/x86/kernel/apic/io_apic.c
> index 3f5f604..e180680 100644
> --- a/arch/x86/kernel/apic/io_apic.c
> +++ b/arch/x86/kernel/apic/io_apic.c
> @@ -253,8 +253,10 @@ int __init arch_early_ioapic_init(void)
>         if (!nr_legacy_irqs())
>                 io_apic_irqs = ~0UL;
>  
> -       for_each_ioapic(i)
> +       for_each_ioapic(i) {
> +               BUG_ON(mp_irqdomain_create(ioapic));
>                 alloc_ioapic_saved_registers(i);
> +       }
>  
>         /*
>          * For legacy IRQ's, start with assigning irq0 to irq15 to
> @@ -2379,8 +2381,6 @@ void __init setup_IO_APIC(void)
>         io_apic_irqs = nr_legacy_irqs() ? ~PIC_IRQS : ~0UL;
>  
>         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.
> @@ -2929,7 +2929,8 @@ int mp_register_ioapic(int id, u32 address, u32 gsi_base,
>         /*
>          * If mp_register_ioapic() is called during early boot stage when
>          * walking ACPI/SFI/DT tables, it's too early to create irqdomain,
> -        * we are still using bootmem allocator. So delay it to setup_IO_APIC().
> +        * we are still using bootmem allocator. So delay it to
> +        * arch_early_ioapic_init().
>          */
>         if (hotplug) {
>                 if (mp_irqdomain_create(idx)) {

>> --- a/arch/x86/kernel/apic/io_apic.c
>> +++ b/arch/x86/kernel/apic/io_apic.c
>> @@ -2369,6 +2381,15 @@ static void ioapic_destroy_irqdomain(int idx)
>>       ioapics[idx].pin_info = NULL;
>>  }
>>  
>> +static void setup_IO_APIC_IDs(void)
>> +{
>> +     if (xen_domain())
>> +             return;

> This would have to xen_pv_domain().

> David

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

[Sander Eikelenboom]

Monday, January 12, 2015, 4:01:00 PM, you wrote:

> On 12/01/15 13:39, 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.
>> 
>> Enhance xen_smp_prepare_cpus() to call setup_IO_APIC() to initialize
>> irqdomain for IOAPICs.

> Having Xen call setup_IO_APIC() to initialize the irq domains then having to
> add special cases to it is just wrong.

> The bits of init deferred by mp_register_apic() are also deferred to
> two different places which looks odd.

> What about something like the following (untested) patch?

Hi David / Gerry,

David's patch (after fixing a few compile issues) fixes the problem.

The power button now works for me on:
- intel baremetal
- intel xen
- amd baremetal (no issues with the override anymore)
- amd xen       (no freeze issues anymore)

Big thanks David !

--
Sander


> diff --git a/arch/x86/kernel/apic/io_apic.c b/arch/x86/kernel/apic/io_apic.c
> index 3f5f604..e180680 100644
> --- a/arch/x86/kernel/apic/io_apic.c
> +++ b/arch/x86/kernel/apic/io_apic.c
> @@ -253,8 +253,10 @@ int __init arch_early_ioapic_init(void)
>         if (!nr_legacy_irqs())
>                 io_apic_irqs = ~0UL;
>  
> -       for_each_ioapic(i)
> +       for_each_ioapic(i) {
> +               BUG_ON(mp_irqdomain_create(ioapic));
>                 alloc_ioapic_saved_registers(i);
> +       }
>  
>         /*
>          * For legacy IRQ's, start with assigning irq0 to irq15 to
> @@ -2379,8 +2381,6 @@ void __init setup_IO_APIC(void)
>         io_apic_irqs = nr_legacy_irqs() ? ~PIC_IRQS : ~0UL;
>  
>         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.
> @@ -2929,7 +2929,8 @@ int mp_register_ioapic(int id, u32 address, u32 gsi_base,
>         /*
>          * If mp_register_ioapic() is called during early boot stage when
>          * walking ACPI/SFI/DT tables, it's too early to create irqdomain,
> -        * we are still using bootmem allocator. So delay it to setup_IO_APIC().
> +        * we are still using bootmem allocator. So delay it to
> +        * arch_early_ioapic_init().
>          */
>         if (hotplug) {
>                 if (mp_irqdomain_create(idx)) {

>> --- a/arch/x86/kernel/apic/io_apic.c
>> +++ b/arch/x86/kernel/apic/io_apic.c
>> @@ -2369,6 +2381,15 @@ static void ioapic_destroy_irqdomain(int idx)
>>       ioapics[idx].pin_info = NULL;
>>  }
>>  
>> +static void setup_IO_APIC_IDs(void)
>> +{
>> +     if (xen_domain())
>> +             return;

> This would have to xen_pv_domain().

> David

Re: [Xen-devel] [PATCH v2] [Bugfix] x86/apic: Fix xen IRQ allocation failure caused by commit b81975eade8c

[Jiang Liu]

On 2015/1/12 23:01, David Vrabel wrote:
> On 12/01/15 13:39, 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.
>>
>> Enhance xen_smp_prepare_cpus() to call setup_IO_APIC() to initialize
>> irqdomain for IOAPICs.
> 
> Having Xen call setup_IO_APIC() to initialize the irq domains then having to
> add special cases to it is just wrong.
> 
> The bits of init deferred by mp_register_apic() are also deferred to
> two different places which looks odd.
> 
> What about something like the following (untested) patch?
Hi David,
	Really appreciate your suggestions about Xen IOAPIC:)
Your patch solves the issue, but it will break a coming patch
set to enable hierarchy irqdomain on x86.
	To enable hierarchy irqdomain on x86, need to build
irqdomains in following order:
1) create irqdomain for CPU vector domain
2) create irqdomains for interrupt remapping unit if interrupt
   remapping is enabled.
3) create irqdomains for IOAPIC/MSI/HPET etc.

Function arch_early_ioapic_init() is called before initializing
interrupt remapping subsystem. So creating IOAPIC irqdomains in
arch_early_ioapic_init() will break hierarchy irqdomain with
interrupt remapping enabled. I will post the x86 hierarchy
irqdomain patch soon.

So could you please help to advice on:
1) Is IOAPIC irqdomain needed for PV?
2) Is IOAPIC irqdomain needed for HVM?
3) Is IOAPIC irqdomain needed for Dom0?
4) What's the proper hook point to create IOAPIC irqdomain for Xen
   UP systems?
I really need to get more understanding of Xen interrupt subsystem:)
Thanks!
Gerry

> 
> diff --git a/arch/x86/kernel/apic/io_apic.c b/arch/x86/kernel/apic/io_apic.c
> index 3f5f604..e180680 100644
> --- a/arch/x86/kernel/apic/io_apic.c
> +++ b/arch/x86/kernel/apic/io_apic.c
> @@ -253,8 +253,10 @@ int __init arch_early_ioapic_init(void)
>  	if (!nr_legacy_irqs())
>  		io_apic_irqs = ~0UL;
>  
> -	for_each_ioapic(i)
> +	for_each_ioapic(i) {
> +		BUG_ON(mp_irqdomain_create(ioapic));
>  		alloc_ioapic_saved_registers(i);
> +	}
>  
>  	/*
>  	 * For legacy IRQ's, start with assigning irq0 to irq15 to
> @@ -2379,8 +2381,6 @@ void __init setup_IO_APIC(void)
>  	io_apic_irqs = nr_legacy_irqs() ? ~PIC_IRQS : ~0UL;
>  
>  	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.
> @@ -2929,7 +2929,8 @@ int mp_register_ioapic(int id, u32 address, u32 gsi_base,
>  	/*
>  	 * If mp_register_ioapic() is called during early boot stage when
>  	 * walking ACPI/SFI/DT tables, it's too early to create irqdomain,
> -	 * we are still using bootmem allocator. So delay it to setup_IO_APIC().
> +	 * we are still using bootmem allocator. So delay it to
> +	 * arch_early_ioapic_init().
>  	 */
>  	if (hotplug) {
>  		if (mp_irqdomain_create(idx)) {
> 
>> --- a/arch/x86/kernel/apic/io_apic.c
>> +++ b/arch/x86/kernel/apic/io_apic.c
>> @@ -2369,6 +2381,15 @@ static void ioapic_destroy_irqdomain(int idx)
>>  	ioapics[idx].pin_info = NULL;
>>  }
>>  
>> +static void setup_IO_APIC_IDs(void)
>> +{
>> +	if (xen_domain())
>> +		return;
> 
> This would have to xen_pv_domain().
> 
> David
> --
> 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: [PATCH v2] [Bugfix] x86/apic: Fix xen IRQ allocation failure caused by commit b81975eade8c

[Jiang Liu]

On 2015/1/12 23:01, David Vrabel wrote:
> On 12/01/15 13:39, 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.
>>
>> Enhance xen_smp_prepare_cpus() to call setup_IO_APIC() to initialize
>> irqdomain for IOAPICs.
> 
> Having Xen call setup_IO_APIC() to initialize the irq domains then having to
> add special cases to it is just wrong.
> 
> The bits of init deferred by mp_register_apic() are also deferred to
> two different places which looks odd.
> 
> What about something like the following (untested) patch?
Hi David,
	Really appreciate your suggestions about Xen IOAPIC:)
Your patch solves the issue, but it will break a coming patch
set to enable hierarchy irqdomain on x86.
	To enable hierarchy irqdomain on x86, need to build
irqdomains in following order:
1) create irqdomain for CPU vector domain
2) create irqdomains for interrupt remapping unit if interrupt
   remapping is enabled.
3) create irqdomains for IOAPIC/MSI/HPET etc.

Function arch_early_ioapic_init() is called before initializing
interrupt remapping subsystem. So creating IOAPIC irqdomains in
arch_early_ioapic_init() will break hierarchy irqdomain with
interrupt remapping enabled. I will post the x86 hierarchy
irqdomain patch soon.

So could you please help to advice on:
1) Is IOAPIC irqdomain needed for PV?
2) Is IOAPIC irqdomain needed for HVM?
3) Is IOAPIC irqdomain needed for Dom0?
4) What's the proper hook point to create IOAPIC irqdomain for Xen
   UP systems?
I really need to get more understanding of Xen interrupt subsystem:)
Thanks!
Gerry

> 
> diff --git a/arch/x86/kernel/apic/io_apic.c b/arch/x86/kernel/apic/io_apic.c
> index 3f5f604..e180680 100644
> --- a/arch/x86/kernel/apic/io_apic.c
> +++ b/arch/x86/kernel/apic/io_apic.c
> @@ -253,8 +253,10 @@ int __init arch_early_ioapic_init(void)
>  	if (!nr_legacy_irqs())
>  		io_apic_irqs = ~0UL;
>  
> -	for_each_ioapic(i)
> +	for_each_ioapic(i) {
> +		BUG_ON(mp_irqdomain_create(ioapic));
>  		alloc_ioapic_saved_registers(i);
> +	}
>  
>  	/*
>  	 * For legacy IRQ's, start with assigning irq0 to irq15 to
> @@ -2379,8 +2381,6 @@ void __init setup_IO_APIC(void)
>  	io_apic_irqs = nr_legacy_irqs() ? ~PIC_IRQS : ~0UL;
>  
>  	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.
> @@ -2929,7 +2929,8 @@ int mp_register_ioapic(int id, u32 address, u32 gsi_base,
>  	/*
>  	 * If mp_register_ioapic() is called during early boot stage when
>  	 * walking ACPI/SFI/DT tables, it's too early to create irqdomain,
> -	 * we are still using bootmem allocator. So delay it to setup_IO_APIC().
> +	 * we are still using bootmem allocator. So delay it to
> +	 * arch_early_ioapic_init().
>  	 */
>  	if (hotplug) {
>  		if (mp_irqdomain_create(idx)) {
> 
>> --- a/arch/x86/kernel/apic/io_apic.c
>> +++ b/arch/x86/kernel/apic/io_apic.c
>> @@ -2369,6 +2381,15 @@ static void ioapic_destroy_irqdomain(int idx)
>>  	ioapics[idx].pin_info = NULL;
>>  }
>>  
>> +static void setup_IO_APIC_IDs(void)
>> +{
>> +	if (xen_domain())
>> +		return;
> 
> This would have to xen_pv_domain().
> 
> David
> --
> 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: [Xen-devel] [PATCH v2] [Bugfix] x86/apic: Fix xen IRQ allocation failure caused by commit b81975eade8c

[Thomas Gleixner]

On Tue, 13 Jan 2015, Sander Eikelenboom wrote:

> 
> Monday, January 12, 2015, 4:01:00 PM, you wrote:
> 
> > On 12/01/15 13:39, 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.
> >> 
> >> Enhance xen_smp_prepare_cpus() to call setup_IO_APIC() to initialize
> >> irqdomain for IOAPICs.
> 
> > Having Xen call setup_IO_APIC() to initialize the irq domains then having to
> > add special cases to it is just wrong.
> 
> > The bits of init deferred by mp_register_apic() are also deferred to
> > two different places which looks odd.
> 
> > What about something like the following (untested) patch?
> 
> Hi David / Gerry,
> 
> David's patch (after fixing a few compile issues) fixes the problem.
> 
> The power button now works for me on:
> - intel baremetal
> - intel xen
> - amd baremetal (no issues with the override anymore)
> - amd xen       (no freeze issues anymore)

Can someone please send a proper patch with changelog?

Thanks,

	tglx

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

[Thomas Gleixner]

On Tue, 13 Jan 2015, Sander Eikelenboom wrote:

> 
> Monday, January 12, 2015, 4:01:00 PM, you wrote:
> 
> > On 12/01/15 13:39, 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.
> >> 
> >> Enhance xen_smp_prepare_cpus() to call setup_IO_APIC() to initialize
> >> irqdomain for IOAPICs.
> 
> > Having Xen call setup_IO_APIC() to initialize the irq domains then having to
> > add special cases to it is just wrong.
> 
> > The bits of init deferred by mp_register_apic() are also deferred to
> > two different places which looks odd.
> 
> > What about something like the following (untested) patch?
> 
> Hi David / Gerry,
> 
> David's patch (after fixing a few compile issues) fixes the problem.
> 
> The power button now works for me on:
> - intel baremetal
> - intel xen
> - amd baremetal (no issues with the override anymore)
> - amd xen       (no freeze issues anymore)

Can someone please send a proper patch with changelog?

Thanks,

	tglx

Re: [Xen-devel] [PATCH v2] [Bugfix] x86/apic: Fix xen IRQ allocation failure caused by commit b81975eade8c

[David Vrabel]

On 13/01/15 02:23, Jiang Liu wrote:
> On 2015/1/12 23:01, David Vrabel wrote:
>> On 12/01/15 13:39, 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.
>>>
>>> Enhance xen_smp_prepare_cpus() to call setup_IO_APIC() to initialize
>>> irqdomain for IOAPICs.
>>
>> Having Xen call setup_IO_APIC() to initialize the irq domains then having to
>> add special cases to it is just wrong.
>>
>> The bits of init deferred by mp_register_apic() are also deferred to
>> two different places which looks odd.
>>
>> What about something like the following (untested) patch?
> Hi David,
> 	Really appreciate your suggestions about Xen IOAPIC:)
> Your patch solves the issue, but it will break a coming patch
> set to enable hierarchy irqdomain on x86.

For 3.19 I would recommend this patch.  Since it's been tested and it's
pretty small.  Can you spin a version of this with a proper changelog.
please?

> 	To enable hierarchy irqdomain on x86, need to build
> irqdomains in following order:
> 1) create irqdomain for CPU vector domain
> 2) create irqdomains for interrupt remapping unit if interrupt
>    remapping is enabled.
> 3) create irqdomains for IOAPIC/MSI/HPET etc.
> 
> Function arch_early_ioapic_init() is called before initializing
> interrupt remapping subsystem. So creating IOAPIC irqdomains in
> arch_early_ioapic_init() will break hierarchy irqdomain with
> interrupt remapping enabled. I will post the x86 hierarchy
> irqdomain patch soon.
> 
> So could you please help to advice on:

I don't know enough about this area to comment on these authoritatively
and I don't have time to look into this in more detail right now, but to
the best of my understanding.

> 1) Is IOAPIC irqdomain needed for PV?

PV dom0 only requires a mechanism to obtain properties of the IOAPIC
interrupt lines.  The actual physical interrupts are delivered to the
hypervisor.  It doesn't require any of the irq setup.

> 2) Is IOAPIC irqdomain needed for HVM?

Yes.

> 3) Is IOAPIC irqdomain needed for Dom0?


See (1) above.

> 4) What's the proper hook point to create IOAPIC irqdomain for Xen
>    UP systems?

Don't know.

David

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

[David Vrabel]

On 13/01/15 02:23, Jiang Liu wrote:
> On 2015/1/12 23:01, David Vrabel wrote:
>> On 12/01/15 13:39, 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.
>>>
>>> Enhance xen_smp_prepare_cpus() to call setup_IO_APIC() to initialize
>>> irqdomain for IOAPICs.
>>
>> Having Xen call setup_IO_APIC() to initialize the irq domains then having to
>> add special cases to it is just wrong.
>>
>> The bits of init deferred by mp_register_apic() are also deferred to
>> two different places which looks odd.
>>
>> What about something like the following (untested) patch?
> Hi David,
> 	Really appreciate your suggestions about Xen IOAPIC:)
> Your patch solves the issue, but it will break a coming patch
> set to enable hierarchy irqdomain on x86.

For 3.19 I would recommend this patch.  Since it's been tested and it's
pretty small.  Can you spin a version of this with a proper changelog.
please?

> 	To enable hierarchy irqdomain on x86, need to build
> irqdomains in following order:
> 1) create irqdomain for CPU vector domain
> 2) create irqdomains for interrupt remapping unit if interrupt
>    remapping is enabled.
> 3) create irqdomains for IOAPIC/MSI/HPET etc.
> 
> Function arch_early_ioapic_init() is called before initializing
> interrupt remapping subsystem. So creating IOAPIC irqdomains in
> arch_early_ioapic_init() will break hierarchy irqdomain with
> interrupt remapping enabled. I will post the x86 hierarchy
> irqdomain patch soon.
> 
> So could you please help to advice on:

I don't know enough about this area to comment on these authoritatively
and I don't have time to look into this in more detail right now, but to
the best of my understanding.

> 1) Is IOAPIC irqdomain needed for PV?

PV dom0 only requires a mechanism to obtain properties of the IOAPIC
interrupt lines.  The actual physical interrupts are delivered to the
hypervisor.  It doesn't require any of the irq setup.

> 2) Is IOAPIC irqdomain needed for HVM?

Yes.

> 3) Is IOAPIC irqdomain needed for Dom0?


See (1) above.

> 4) What's the proper hook point to create IOAPIC irqdomain for Xen
>    UP systems?

Don't know.

David

Re: [Xen-devel] [PATCH v2] [Bugfix] x86/apic: Fix xen IRQ allocation failure caused by commit b81975eade8c

[Jiang Liu]

On 2015/1/13 18:09, David Vrabel wrote:
> On 13/01/15 02:23, Jiang Liu wrote:
>> On 2015/1/12 23:01, David Vrabel wrote:
>>> On 12/01/15 13:39, 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.
>>>>
>>>> Enhance xen_smp_prepare_cpus() to call setup_IO_APIC() to initialize
>>>> irqdomain for IOAPICs.
>>>
>>> Having Xen call setup_IO_APIC() to initialize the irq domains then having to
>>> add special cases to it is just wrong.
>>>
>>> The bits of init deferred by mp_register_apic() are also deferred to
>>> two different places which looks odd.
>>>
>>> What about something like the following (untested) patch?
>> Hi David,
>> 	Really appreciate your suggestions about Xen IOAPIC:)
>> Your patch solves the issue, but it will break a coming patch
>> set to enable hierarchy irqdomain on x86.
> 
> For 3.19 I would recommend this patch.  Since it's been tested and it's
> pretty small.  Can you spin a version of this with a proper changelog.
> please?
Hi David,
	Good suggestion, I will send out the patch tomorrow.
Regards!
Gerry

> 
>> 	To enable hierarchy irqdomain on x86, need to build
>> irqdomains in following order:
>> 1) create irqdomain for CPU vector domain
>> 2) create irqdomains for interrupt remapping unit if interrupt
>>    remapping is enabled.
>> 3) create irqdomains for IOAPIC/MSI/HPET etc.
>>
>> Function arch_early_ioapic_init() is called before initializing
>> interrupt remapping subsystem. So creating IOAPIC irqdomains in
>> arch_early_ioapic_init() will break hierarchy irqdomain with
>> interrupt remapping enabled. I will post the x86 hierarchy
>> irqdomain patch soon.
>>
>> So could you please help to advice on:
> 
> I don't know enough about this area to comment on these authoritatively
> and I don't have time to look into this in more detail right now, but to
> the best of my understanding.
> 
>> 1) Is IOAPIC irqdomain needed for PV?
> 
> PV dom0 only requires a mechanism to obtain properties of the IOAPIC
> interrupt lines.  The actual physical interrupts are delivered to the
> hypervisor.  It doesn't require any of the irq setup.
> 
>> 2) Is IOAPIC irqdomain needed for HVM?
> 
> Yes.
> 
>> 3) Is IOAPIC irqdomain needed for Dom0?
> 
> 
> See (1) above.
> 
>> 4) What's the proper hook point to create IOAPIC irqdomain for Xen
>>    UP systems?
> 
> Don't know.
> 
> David
> 

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

[Jiang Liu]

On 2015/1/13 18:09, David Vrabel wrote:
> On 13/01/15 02:23, Jiang Liu wrote:
>> On 2015/1/12 23:01, David Vrabel wrote:
>>> On 12/01/15 13:39, 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.
>>>>
>>>> Enhance xen_smp_prepare_cpus() to call setup_IO_APIC() to initialize
>>>> irqdomain for IOAPICs.
>>>
>>> Having Xen call setup_IO_APIC() to initialize the irq domains then having to
>>> add special cases to it is just wrong.
>>>
>>> The bits of init deferred by mp_register_apic() are also deferred to
>>> two different places which looks odd.
>>>
>>> What about something like the following (untested) patch?
>> Hi David,
>> 	Really appreciate your suggestions about Xen IOAPIC:)
>> Your patch solves the issue, but it will break a coming patch
>> set to enable hierarchy irqdomain on x86.
> 
> For 3.19 I would recommend this patch.  Since it's been tested and it's
> pretty small.  Can you spin a version of this with a proper changelog.
> please?
Hi David,
	Good suggestion, I will send out the patch tomorrow.
Regards!
Gerry

> 
>> 	To enable hierarchy irqdomain on x86, need to build
>> irqdomains in following order:
>> 1) create irqdomain for CPU vector domain
>> 2) create irqdomains for interrupt remapping unit if interrupt
>>    remapping is enabled.
>> 3) create irqdomains for IOAPIC/MSI/HPET etc.
>>
>> Function arch_early_ioapic_init() is called before initializing
>> interrupt remapping subsystem. So creating IOAPIC irqdomains in
>> arch_early_ioapic_init() will break hierarchy irqdomain with
>> interrupt remapping enabled. I will post the x86 hierarchy
>> irqdomain patch soon.
>>
>> So could you please help to advice on:
> 
> I don't know enough about this area to comment on these authoritatively
> and I don't have time to look into this in more detail right now, but to
> the best of my understanding.
> 
>> 1) Is IOAPIC irqdomain needed for PV?
> 
> PV dom0 only requires a mechanism to obtain properties of the IOAPIC
> interrupt lines.  The actual physical interrupts are delivered to the
> hypervisor.  It doesn't require any of the irq setup.
> 
>> 2) Is IOAPIC irqdomain needed for HVM?
> 
> Yes.
> 
>> 3) Is IOAPIC irqdomain needed for Dom0?
> 
> 
> See (1) above.
> 
>> 4) What's the proper hook point to create IOAPIC irqdomain for Xen
>>    UP systems?
> 
> Don't know.
> 
> David
> 

[PATCH v3] [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.

So move creating of IOAPIC irqdomains from setup_IO_APIC() into
arch_early_ioapic_init().

Signed-off-by: Jiang Liu <jiang.liu@linux.intel.com>
Signed-off-by: David Vrabel <david.vrabel@citrix.com>
Reported-and-tested-by: Sander Eikelenboom <linux@eikelenboom.it>
Cc: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
---
 arch/x86/kernel/apic/io_apic.c |   13 +++++++------
 1 file changed, 7 insertions(+), 6 deletions(-)

diff --git a/arch/x86/kernel/apic/io_apic.c b/arch/x86/kernel/apic/io_apic.c
index 3f5f60406ab1..1117c84cefe4 100644
--- a/arch/x86/kernel/apic/io_apic.c
+++ b/arch/x86/kernel/apic/io_apic.c
@@ -245,6 +245,8 @@ static void free_ioapic_saved_registers(int idx)
 	ioapics[idx].saved_registers = NULL;
 }
 
+static int mp_irqdomain_create(int ioapic);
+
 int __init arch_early_ioapic_init(void)
 {
 	struct irq_cfg *cfg;
@@ -253,8 +255,10 @@ int __init arch_early_ioapic_init(void)
 	if (!nr_legacy_irqs())
 		io_apic_irqs = ~0UL;
 
-	for_each_ioapic(i)
+	for_each_ioapic(i) {
+		BUG_ON(mp_irqdomain_create(i));
 		alloc_ioapic_saved_registers(i);
+	}
 
 	/*
 	 * For legacy IRQ's, start with assigning irq0 to irq15 to
@@ -2371,16 +2375,12 @@ static void ioapic_destroy_irqdomain(int idx)
 
 void __init setup_IO_APIC(void)
 {
-	int ioapic;
-
 	/*
 	 * calling enable_IO_APIC() is moved to setup_local_APIC for BP
 	 */
 	io_apic_irqs = nr_legacy_irqs() ? ~PIC_IRQS : ~0UL;
 
 	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.
@@ -2929,7 +2929,8 @@ int mp_register_ioapic(int id, u32 address, u32 gsi_base,
 	/*
 	 * If mp_register_ioapic() is called during early boot stage when
 	 * walking ACPI/SFI/DT tables, it's too early to create irqdomain,
-	 * we are still using bootmem allocator. So delay it to setup_IO_APIC().
+	 * we are still using bootmem allocator.So delay it to
+	 * arch_early_ioapic_init().
 	 */
 	if (hotplug) {
 		if (mp_irqdomain_create(idx)) {
-- 
1.7.10.4

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

[Sander Eikelenboom]

Wednesday, January 14, 2015, 4:09:35 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.

> So move creating of IOAPIC irqdomains from setup_IO_APIC() into
> arch_early_ioapic_init().

> Signed-off-by: Jiang Liu <jiang.liu@linux.intel.com>
> Signed-off-by: David Vrabel <david.vrabel@citrix.com>
> Reported-and-tested-by: Sander Eikelenboom <linux@eikelenboom.it>
> Cc: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>

Thanks Gerry !
Will you send (a backport) to stable for 3.17 and 3.18 when 
it's applied to -tip ?

--
Sander

> ---
>  arch/x86/kernel/apic/io_apic.c |   13 +++++++------
>  1 file changed, 7 insertions(+), 6 deletions(-)

> diff --git a/arch/x86/kernel/apic/io_apic.c b/arch/x86/kernel/apic/io_apic.c
> index 3f5f60406ab1..1117c84cefe4 100644
> --- a/arch/x86/kernel/apic/io_apic.c
> +++ b/arch/x86/kernel/apic/io_apic.c
> @@ -245,6 +245,8 @@ static void free_ioapic_saved_registers(int idx)
>         ioapics[idx].saved_registers = NULL;
>  }
>  
> +static int mp_irqdomain_create(int ioapic);
> +
>  int __init arch_early_ioapic_init(void)
>  {
>         struct irq_cfg *cfg;
> @@ -253,8 +255,10 @@ int __init arch_early_ioapic_init(void)
>         if (!nr_legacy_irqs())
>                 io_apic_irqs = ~0UL;
>  
> -       for_each_ioapic(i)
> +       for_each_ioapic(i) {
> +               BUG_ON(mp_irqdomain_create(i));
>                 alloc_ioapic_saved_registers(i);
> +       }
>  
>         /*
>          * For legacy IRQ's, start with assigning irq0 to irq15 to
> @@ -2371,16 +2375,12 @@ static void ioapic_destroy_irqdomain(int idx)
>  
>  void __init setup_IO_APIC(void)
>  {
> -       int ioapic;
> -
>         /*
>          * calling enable_IO_APIC() is moved to setup_local_APIC for BP
>          */
>         io_apic_irqs = nr_legacy_irqs() ? ~PIC_IRQS : ~0UL;
>  
>         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.
> @@ -2929,7 +2929,8 @@ int mp_register_ioapic(int id, u32 address, u32 gsi_base,
>         /*
>          * If mp_register_ioapic() is called during early boot stage when
>          * walking ACPI/SFI/DT tables, it's too early to create irqdomain,
> -        * we are still using bootmem allocator. So delay it to setup_IO_APIC().
> +        * we are still using bootmem allocator.So delay it to
> +        * arch_early_ioapic_init().
>          */
>         if (hotplug) {
>                 if (mp_irqdomain_create(idx)) {

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

[Jiang Liu]

On 2015/1/14 20:23, Sander Eikelenboom wrote:
> 
> Wednesday, January 14, 2015, 4:09:35 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.
> 
>> So move creating of IOAPIC irqdomains from setup_IO_APIC() into
>> arch_early_ioapic_init().
> 
>> Signed-off-by: Jiang Liu <jiang.liu@linux.intel.com>
>> Signed-off-by: David Vrabel <david.vrabel@citrix.com>
>> Reported-and-tested-by: Sander Eikelenboom <linux@eikelenboom.it>
>> Cc: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
> 
> Thanks Gerry !
> Will you send (a backport) to stable for 3.17 and 3.18 when 
> it's applied to -tip ?
Sure, will do it once it reaches mainstream kernel.

> 
> --
> Sander
> 
>> ---
>>  arch/x86/kernel/apic/io_apic.c |   13 +++++++------
>>  1 file changed, 7 insertions(+), 6 deletions(-)
> 
>> diff --git a/arch/x86/kernel/apic/io_apic.c b/arch/x86/kernel/apic/io_apic.c
>> index 3f5f60406ab1..1117c84cefe4 100644
>> --- a/arch/x86/kernel/apic/io_apic.c
>> +++ b/arch/x86/kernel/apic/io_apic.c
>> @@ -245,6 +245,8 @@ static void free_ioapic_saved_registers(int idx)
>>         ioapics[idx].saved_registers = NULL;
>>  }
>>  
>> +static int mp_irqdomain_create(int ioapic);
>> +
>>  int __init arch_early_ioapic_init(void)
>>  {
>>         struct irq_cfg *cfg;
>> @@ -253,8 +255,10 @@ int __init arch_early_ioapic_init(void)
>>         if (!nr_legacy_irqs())
>>                 io_apic_irqs = ~0UL;
>>  
>> -       for_each_ioapic(i)
>> +       for_each_ioapic(i) {
>> +               BUG_ON(mp_irqdomain_create(i));
>>                 alloc_ioapic_saved_registers(i);
>> +       }
>>  
>>         /*
>>          * For legacy IRQ's, start with assigning irq0 to irq15 to
>> @@ -2371,16 +2375,12 @@ static void ioapic_destroy_irqdomain(int idx)
>>  
>>  void __init setup_IO_APIC(void)
>>  {
>> -       int ioapic;
>> -
>>         /*
>>          * calling enable_IO_APIC() is moved to setup_local_APIC for BP
>>          */
>>         io_apic_irqs = nr_legacy_irqs() ? ~PIC_IRQS : ~0UL;
>>  
>>         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.
>> @@ -2929,7 +2929,8 @@ int mp_register_ioapic(int id, u32 address, u32 gsi_base,
>>         /*
>>          * If mp_register_ioapic() is called during early boot stage when
>>          * walking ACPI/SFI/DT tables, it's too early to create irqdomain,
>> -        * we are still using bootmem allocator. So delay it to setup_IO_APIC().
>> +        * we are still using bootmem allocator.So delay it to
>> +        * arch_early_ioapic_init().
>>          */
>>         if (hotplug) {
>>                 if (mp_irqdomain_create(idx)) {
> 

[Bugfix v4 0/2] Fix xen IRQ allocation failure caused by commit b81975eade8c

[Jiang Liu]

With more knowledge of Xen interrupt manangement subsytem, I realized
previous three versions to fix https://lkml.org/lkml/2014/12/19/178 are
just plainly wrong. Those patches try to fix the issue by creating
irqdomain for IOAPICs for PV domains, which effectively let native
IOAPIC driver and Xen PV interrupt management subsystem to manage
IOAPIC irqs concurrently, sounds unpredictable.

Sorry for those wrong fixes. The good news is that the new fix does
make code simpler and easier to maintain.

I have tested the patchset on Intel platform with bare metal and Dom0
kernels.

Hi Sander,
	Could you please help to test it again?

Regards!
Gerry

Jiang Liu (2):
  xen/pci: Kill function xen_setup_acpi_sci()
  xen/pci: Simplify x86/pci/xen.c by killing gsi_override related code

 arch/x86/kernel/acpi/boot.c |   26 ++++++++---------
 arch/x86/pci/xen.c          |   68 ++++---------------------------------------
 2 files changed, 19 insertions(+), 75 deletions(-)

-- 
1.7.10.4

[Bugfix v4 1/2] xen/pci: 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.

Currently Xen Domain0 has special treatment for ACPI SCI interrupt,
that is initialize irq for ACPI SCI at early stage in a special way as:
xen_init_IRQ()
	->pci_xen_initial_domain()
		->xen_setup_acpi_sci()
			Allocate and initialize irq for ACPI SCI

Function xen_setup_acpi_sci() calls acpi_gsi_to_irq() to get an irq
number for ACPI SCI. But unfortunately acpi_gsi_to_irq() depends on
IOAPIC irqdomains through following path
acpi_gsi_to_irq()
	->mp_map_gsi_to_irq()
		->mp_map_pin_to_irq()
			->check IOAPIC irqdomain

For PV domains, it uses Xen event based interrupt manangement and
doesn't make uses of native IOAPIC, so no irqdomains created for IOAPIC.
This causes Xen domain0 fails to install interrupt handler for ACPI SCI
and all ACPI events will be lost. Please refer to:
https://lkml.org/lkml/2014/12/19/178

So the fix is to get rid of special treatment for ACPI SCI, just treat
ACPI SCI as normal GSI interrupt as:
acpi_gsi_to_irq()
	->acpi_register_gsi()
		->acpi_register_gsi_xen()
			->xen_register_gsi()

With above change, there's no need for xen_setup_acpi_sci() anymore.
The above change also works with bare metal kernel too.

Note: we need to test this patch on those special AMD systems which
override normal SCI GSI (9) with strange GSI (20).

Signed-off-by: Jiang Liu <jiang.liu@linux.intel.com>
---
 arch/x86/kernel/acpi/boot.c |   26 ++++++++++++------------
 arch/x86/pci/xen.c          |   47 -------------------------------------------
 2 files changed, 13 insertions(+), 60 deletions(-)

diff --git a/arch/x86/kernel/acpi/boot.c b/arch/x86/kernel/acpi/boot.c
index 4433a4be8171..395c4c615293 100644
--- a/arch/x86/kernel/acpi/boot.c
+++ b/arch/x86/kernel/acpi/boot.c
@@ -611,20 +611,20 @@ void __init acpi_pic_sci_set_trigger(unsigned int irq, u16 trigger)
 
 int acpi_gsi_to_irq(u32 gsi, unsigned int *irqp)
 {
-	int irq;
-
-	if (acpi_irq_model == ACPI_IRQ_MODEL_PIC) {
-		*irqp = gsi;
-	} else {
-		mutex_lock(&acpi_ioapic_lock);
-		irq = mp_map_gsi_to_irq(gsi,
-					IOAPIC_MAP_ALLOC | IOAPIC_MAP_CHECK);
-		mutex_unlock(&acpi_ioapic_lock);
-		if (irq < 0)
-			return -1;
-		*irqp = irq;
+	int rc, irq, trigger, polarity;
+
+	rc = acpi_get_override_irq(gsi, &trigger, &polarity);
+	if (rc == 0) {
+		trigger = trigger ? ACPI_LEVEL_SENSITIVE : ACPI_EDGE_SENSITIVE;
+		polarity = polarity ? ACPI_ACTIVE_LOW : ACPI_ACTIVE_HIGH;
+		irq = acpi_register_gsi(NULL, gsi, trigger, polarity);
+		if (irq >= 0) {
+			*irqp = irq;
+			return 0;
+		}
 	}
-	return 0;
+
+	return -1;
 }
 EXPORT_SYMBOL_GPL(acpi_gsi_to_irq);
 
diff --git a/arch/x86/pci/xen.c b/arch/x86/pci/xen.c
index c489ef2c1a39..6e5e89c3c644 100644
--- a/arch/x86/pci/xen.c
+++ b/arch/x86/pci/xen.c
@@ -471,52 +471,6 @@ int __init pci_xen_hvm_init(void)
 }
 
 #ifdef CONFIG_XEN_DOM0
-static __init void xen_setup_acpi_sci(void)
-{
-	int rc;
-	int trigger, polarity;
-	int gsi = acpi_sci_override_gsi;
-	int irq = -1;
-	int gsi_override = -1;
-
-	if (!gsi)
-		return;
-
-	rc = acpi_get_override_irq(gsi, &trigger, &polarity);
-	if (rc) {
-		printk(KERN_WARNING "xen: acpi_get_override_irq failed for acpi"
-				" sci, rc=%d\n", rc);
-		return;
-	}
-	trigger = trigger ? ACPI_LEVEL_SENSITIVE : ACPI_EDGE_SENSITIVE;
-	polarity = polarity ? ACPI_ACTIVE_LOW : ACPI_ACTIVE_HIGH;
-
-	printk(KERN_INFO "xen: sci override: global_irq=%d trigger=%d "
-			"polarity=%d\n", gsi, trigger, polarity);
-
-	/* Before we bind the GSI to a Linux IRQ, check whether
-	 * we need to override it with bus_irq (IRQ) value. Usually for
-	 * IRQs below IRQ_LEGACY_IRQ this holds IRQ == GSI, as so:
-	 *  ACPI: INT_SRC_OVR (bus 0 bus_irq 9 global_irq 9 low level)
-	 * but there are oddballs where the IRQ != GSI:
-	 *  ACPI: INT_SRC_OVR (bus 0 bus_irq 9 global_irq 20 low level)
-	 * which ends up being: gsi_to_irq[9] == 20
-	 * (which is what acpi_gsi_to_irq ends up calling when starting the
-	 * the ACPI interpreter and keels over since IRQ 9 has not been
-	 * setup as we had setup IRQ 20 for it).
-	 */
-	if (acpi_gsi_to_irq(gsi, &irq) == 0) {
-		/* Use the provided value if it's valid. */
-		if (irq >= 0)
-			gsi_override = irq;
-	}
-
-	gsi = xen_register_gsi(gsi, gsi_override, trigger, polarity);
-	printk(KERN_INFO "xen: acpi sci %d\n", gsi);
-
-	return;
-}
-
 int __init pci_xen_initial_domain(void)
 {
 	int irq;
@@ -527,7 +481,6 @@ int __init pci_xen_initial_domain(void)
 	x86_msi.restore_msi_irqs = xen_initdom_restore_msi_irqs;
 	pci_msi_ignore_mask = 1;
 #endif
-	xen_setup_acpi_sci();
 	__acpi_register_gsi = acpi_register_gsi_xen;
 	/* Pre-allocate legacy irqs */
 	for (irq = 0; irq < nr_legacy_irqs(); irq++) {
-- 
1.7.10.4

[Bugfix v4 1/2] xen/pci: 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.

Currently Xen Domain0 has special treatment for ACPI SCI interrupt,
that is initialize irq for ACPI SCI at early stage in a special way as:
xen_init_IRQ()
	->pci_xen_initial_domain()
		->xen_setup_acpi_sci()
			Allocate and initialize irq for ACPI SCI

Function xen_setup_acpi_sci() calls acpi_gsi_to_irq() to get an irq
number for ACPI SCI. But unfortunately acpi_gsi_to_irq() depends on
IOAPIC irqdomains through following path
acpi_gsi_to_irq()
	->mp_map_gsi_to_irq()
		->mp_map_pin_to_irq()
			->check IOAPIC irqdomain

For PV domains, it uses Xen event based interrupt manangement and
doesn't make uses of native IOAPIC, so no irqdomains created for IOAPIC.
This causes Xen domain0 fails to install interrupt handler for ACPI SCI
and all ACPI events will be lost. Please refer to:
https://lkml.org/lkml/2014/12/19/178

So the fix is to get rid of special treatment for ACPI SCI, just treat
ACPI SCI as normal GSI interrupt as:
acpi_gsi_to_irq()
	->acpi_register_gsi()
		->acpi_register_gsi_xen()
			->xen_register_gsi()

With above change, there's no need for xen_setup_acpi_sci() anymore.
The above change also works with bare metal kernel too.

Note: we need to test this patch on those special AMD systems which
override normal SCI GSI (9) with strange GSI (20).

Signed-off-by: Jiang Liu <jiang.liu@linux.intel.com>
---
 arch/x86/kernel/acpi/boot.c |   26 ++++++++++++------------
 arch/x86/pci/xen.c          |   47 -------------------------------------------
 2 files changed, 13 insertions(+), 60 deletions(-)

diff --git a/arch/x86/kernel/acpi/boot.c b/arch/x86/kernel/acpi/boot.c
index 4433a4be8171..395c4c615293 100644
--- a/arch/x86/kernel/acpi/boot.c
+++ b/arch/x86/kernel/acpi/boot.c
@@ -611,20 +611,20 @@ void __init acpi_pic_sci_set_trigger(unsigned int irq, u16 trigger)
 
 int acpi_gsi_to_irq(u32 gsi, unsigned int *irqp)
 {
-	int irq;
-
-	if (acpi_irq_model == ACPI_IRQ_MODEL_PIC) {
-		*irqp = gsi;
-	} else {
-		mutex_lock(&acpi_ioapic_lock);
-		irq = mp_map_gsi_to_irq(gsi,
-					IOAPIC_MAP_ALLOC | IOAPIC_MAP_CHECK);
-		mutex_unlock(&acpi_ioapic_lock);
-		if (irq < 0)
-			return -1;
-		*irqp = irq;
+	int rc, irq, trigger, polarity;
+
+	rc = acpi_get_override_irq(gsi, &trigger, &polarity);
+	if (rc == 0) {
+		trigger = trigger ? ACPI_LEVEL_SENSITIVE : ACPI_EDGE_SENSITIVE;
+		polarity = polarity ? ACPI_ACTIVE_LOW : ACPI_ACTIVE_HIGH;
+		irq = acpi_register_gsi(NULL, gsi, trigger, polarity);
+		if (irq >= 0) {
+			*irqp = irq;
+			return 0;
+		}
 	}
-	return 0;
+
+	return -1;
 }
 EXPORT_SYMBOL_GPL(acpi_gsi_to_irq);
 
diff --git a/arch/x86/pci/xen.c b/arch/x86/pci/xen.c
index c489ef2c1a39..6e5e89c3c644 100644
--- a/arch/x86/pci/xen.c
+++ b/arch/x86/pci/xen.c
@@ -471,52 +471,6 @@ int __init pci_xen_hvm_init(void)
 }
 
 #ifdef CONFIG_XEN_DOM0
-static __init void xen_setup_acpi_sci(void)
-{
-	int rc;
-	int trigger, polarity;
-	int gsi = acpi_sci_override_gsi;
-	int irq = -1;
-	int gsi_override = -1;
-
-	if (!gsi)
-		return;
-
-	rc = acpi_get_override_irq(gsi, &trigger, &polarity);
-	if (rc) {
-		printk(KERN_WARNING "xen: acpi_get_override_irq failed for acpi"
-				" sci, rc=%d\n", rc);
-		return;
-	}
-	trigger = trigger ? ACPI_LEVEL_SENSITIVE : ACPI_EDGE_SENSITIVE;
-	polarity = polarity ? ACPI_ACTIVE_LOW : ACPI_ACTIVE_HIGH;
-
-	printk(KERN_INFO "xen: sci override: global_irq=%d trigger=%d "
-			"polarity=%d\n", gsi, trigger, polarity);
-
-	/* Before we bind the GSI to a Linux IRQ, check whether
-	 * we need to override it with bus_irq (IRQ) value. Usually for
-	 * IRQs below IRQ_LEGACY_IRQ this holds IRQ == GSI, as so:
-	 *  ACPI: INT_SRC_OVR (bus 0 bus_irq 9 global_irq 9 low level)
-	 * but there are oddballs where the IRQ != GSI:
-	 *  ACPI: INT_SRC_OVR (bus 0 bus_irq 9 global_irq 20 low level)
-	 * which ends up being: gsi_to_irq[9] == 20
-	 * (which is what acpi_gsi_to_irq ends up calling when starting the
-	 * the ACPI interpreter and keels over since IRQ 9 has not been
-	 * setup as we had setup IRQ 20 for it).
-	 */
-	if (acpi_gsi_to_irq(gsi, &irq) == 0) {
-		/* Use the provided value if it's valid. */
-		if (irq >= 0)
-			gsi_override = irq;
-	}
-
-	gsi = xen_register_gsi(gsi, gsi_override, trigger, polarity);
-	printk(KERN_INFO "xen: acpi sci %d\n", gsi);
-
-	return;
-}
-
 int __init pci_xen_initial_domain(void)
 {
 	int irq;
@@ -527,7 +481,6 @@ int __init pci_xen_initial_domain(void)
 	x86_msi.restore_msi_irqs = xen_initdom_restore_msi_irqs;
 	pci_msi_ignore_mask = 1;
 #endif
-	xen_setup_acpi_sci();
 	__acpi_register_gsi = acpi_register_gsi_xen;
 	/* Pre-allocate legacy irqs */
 	for (irq = 0; irq < nr_legacy_irqs(); irq++) {
-- 
1.7.10.4

[Bugfix v4 2/2] xen/pci: Simplify x86/pci/xen.c by killing gsi_override related code

[Jiang Liu]

There's no use of gsi_override related logic anymore, so kill code
related to gsi_override.

Signed-off-by: Jiang Liu <jiang.liu@linux.intel.com>
---
 arch/x86/pci/xen.c |   21 ++++++---------------
 1 file changed, 6 insertions(+), 15 deletions(-)

diff --git a/arch/x86/pci/xen.c b/arch/x86/pci/xen.c
index 6e5e89c3c644..a73c3bd71243 100644
--- a/arch/x86/pci/xen.c
+++ b/arch/x86/pci/xen.c
@@ -59,8 +59,7 @@ static int xen_pcifront_enable_irq(struct pci_dev *dev)
 }
 
 #ifdef CONFIG_ACPI
-static int xen_register_pirq(u32 gsi, int gsi_override, int triggering,
-			     bool set_pirq)
+static int xen_register_pirq(u32 gsi, int triggering, bool set_pirq)
 {
 	int rc, pirq = -1, irq = -1;
 	struct physdev_map_pirq map_irq;
@@ -93,9 +92,6 @@ static int xen_register_pirq(u32 gsi, int gsi_override, int triggering,
 		name = "ioapic-level";
 	}
 
-	if (gsi_override >= 0)
-		gsi = gsi_override;
-
 	irq = xen_bind_pirq_gsi_to_irq(gsi, map_irq.pirq, shareable, name);
 	if (irq < 0)
 		goto out;
@@ -111,12 +107,13 @@ static int acpi_register_gsi_xen_hvm(struct device *dev, u32 gsi,
 	if (!xen_hvm_domain())
 		return -1;
 
-	return xen_register_pirq(gsi, -1 /* no GSI override */, trigger,
+	return xen_register_pirq(gsi, trigger,
 				 false /* no mapping of GSI to PIRQ */);
 }
 
 #ifdef CONFIG_XEN_DOM0
-static int xen_register_gsi(u32 gsi, int gsi_override, int triggering, int polarity)
+static int acpi_register_gsi_xen(struct device *dev, u32 gsi,
+				 int triggering, int polarity)
 {
 	int rc, irq;
 	struct physdev_setup_gsi setup_gsi;
@@ -127,7 +124,7 @@ static int xen_register_gsi(u32 gsi, int gsi_override, int triggering, int polar
 	printk(KERN_DEBUG "xen: registering gsi %u triggering %d polarity %d\n",
 			gsi, triggering, polarity);
 
-	irq = xen_register_pirq(gsi, gsi_override, triggering, true);
+	irq = xen_register_pirq(gsi, triggering, true);
 
 	setup_gsi.gsi = gsi;
 	setup_gsi.triggering = (triggering == ACPI_EDGE_SENSITIVE ? 0 : 1);
@@ -143,12 +140,6 @@ static int xen_register_gsi(u32 gsi, int gsi_override, int triggering, int polar
 
 	return irq;
 }
-
-static int acpi_register_gsi_xen(struct device *dev, u32 gsi,
-				 int trigger, int polarity)
-{
-	return xen_register_gsi(gsi, -1 /* no GSI override */, trigger, polarity);
-}
 #endif
 #endif
 
@@ -489,7 +480,7 @@ int __init pci_xen_initial_domain(void)
 		if (acpi_get_override_irq(irq, &trigger, &polarity) == -1)
 			continue;
 
-		xen_register_pirq(irq, -1 /* no GSI override */,
+		xen_register_pirq(irq,
 			trigger ? ACPI_LEVEL_SENSITIVE : ACPI_EDGE_SENSITIVE,
 			true /* Map GSI to PIRQ */);
 	}
-- 
1.7.10.4

[Bugfix v4 2/2] xen/pci: Simplify x86/pci/xen.c by killing gsi_override related code

[Jiang Liu]

There's no use of gsi_override related logic anymore, so kill code
related to gsi_override.

Signed-off-by: Jiang Liu <jiang.liu@linux.intel.com>
---
 arch/x86/pci/xen.c |   21 ++++++---------------
 1 file changed, 6 insertions(+), 15 deletions(-)

diff --git a/arch/x86/pci/xen.c b/arch/x86/pci/xen.c
index 6e5e89c3c644..a73c3bd71243 100644
--- a/arch/x86/pci/xen.c
+++ b/arch/x86/pci/xen.c
@@ -59,8 +59,7 @@ static int xen_pcifront_enable_irq(struct pci_dev *dev)
 }
 
 #ifdef CONFIG_ACPI
-static int xen_register_pirq(u32 gsi, int gsi_override, int triggering,
-			     bool set_pirq)
+static int xen_register_pirq(u32 gsi, int triggering, bool set_pirq)
 {
 	int rc, pirq = -1, irq = -1;
 	struct physdev_map_pirq map_irq;
@@ -93,9 +92,6 @@ static int xen_register_pirq(u32 gsi, int gsi_override, int triggering,
 		name = "ioapic-level";
 	}
 
-	if (gsi_override >= 0)
-		gsi = gsi_override;
-
 	irq = xen_bind_pirq_gsi_to_irq(gsi, map_irq.pirq, shareable, name);
 	if (irq < 0)
 		goto out;
@@ -111,12 +107,13 @@ static int acpi_register_gsi_xen_hvm(struct device *dev, u32 gsi,
 	if (!xen_hvm_domain())
 		return -1;
 
-	return xen_register_pirq(gsi, -1 /* no GSI override */, trigger,
+	return xen_register_pirq(gsi, trigger,
 				 false /* no mapping of GSI to PIRQ */);
 }
 
 #ifdef CONFIG_XEN_DOM0
-static int xen_register_gsi(u32 gsi, int gsi_override, int triggering, int polarity)
+static int acpi_register_gsi_xen(struct device *dev, u32 gsi,
+				 int triggering, int polarity)
 {
 	int rc, irq;
 	struct physdev_setup_gsi setup_gsi;
@@ -127,7 +124,7 @@ static int xen_register_gsi(u32 gsi, int gsi_override, int triggering, int polar
 	printk(KERN_DEBUG "xen: registering gsi %u triggering %d polarity %d\n",
 			gsi, triggering, polarity);
 
-	irq = xen_register_pirq(gsi, gsi_override, triggering, true);
+	irq = xen_register_pirq(gsi, triggering, true);
 
 	setup_gsi.gsi = gsi;
 	setup_gsi.triggering = (triggering == ACPI_EDGE_SENSITIVE ? 0 : 1);
@@ -143,12 +140,6 @@ static int xen_register_gsi(u32 gsi, int gsi_override, int triggering, int polar
 
 	return irq;
 }
-
-static int acpi_register_gsi_xen(struct device *dev, u32 gsi,
-				 int trigger, int polarity)
-{
-	return xen_register_gsi(gsi, -1 /* no GSI override */, trigger, polarity);
-}
 #endif
 #endif
 
@@ -489,7 +480,7 @@ int __init pci_xen_initial_domain(void)
 		if (acpi_get_override_irq(irq, &trigger, &polarity) == -1)
 			continue;
 
-		xen_register_pirq(irq, -1 /* no GSI override */,
+		xen_register_pirq(irq,
 			trigger ? ACPI_LEVEL_SENSITIVE : ACPI_EDGE_SENSITIVE,
 			true /* Map GSI to PIRQ */);
 	}
-- 
1.7.10.4

Re: [Bugfix v4 0/2] Fix xen IRQ allocation failure caused by commit b81975eade8c

[Sander Eikelenboom]

Thursday, January 15, 2015, 2:04:34 PM, you wrote:

> With more knowledge of Xen interrupt manangement subsytem, I realized
> previous three versions to fix https://lkml.org/lkml/2014/12/19/178 are
> just plainly wrong. Those patches try to fix the issue by creating
> irqdomain for IOAPICs for PV domains, which effectively let native
> IOAPIC driver and Xen PV interrupt management subsystem to manage
> IOAPIC irqs concurrently, sounds unpredictable.

> Sorry for those wrong fixes. The good news is that the new fix does
> make code simpler and easier to maintain.

> I have tested the patchset on Intel platform with bare metal and Dom0
> kernels.

> Hi Sander,
>         Could you please help to test it again?

> Regards!
> Gerry

Hi Gerry,

Sure, i will test this afternoon and report back.

Thanks

--
Sander

> Jiang Liu (2):
>   xen/pci: Kill function xen_setup_acpi_sci()
>   xen/pci: Simplify x86/pci/xen.c by killing gsi_override related code

>  arch/x86/kernel/acpi/boot.c |   26 ++++++++---------
>  arch/x86/pci/xen.c          |   68 ++++---------------------------------------
>  2 files changed, 19 insertions(+), 75 deletions(-)

Re: [Bugfix v4 0/2] Fix xen IRQ allocation failure caused by commit b81975eade8c

[Sander Eikelenboom]

Thursday, January 15, 2015, 2:04:34 PM, you wrote:

> With more knowledge of Xen interrupt manangement subsytem, I realized
> previous three versions to fix https://lkml.org/lkml/2014/12/19/178 are
> just plainly wrong. Those patches try to fix the issue by creating
> irqdomain for IOAPICs for PV domains, which effectively let native
> IOAPIC driver and Xen PV interrupt management subsystem to manage
> IOAPIC irqs concurrently, sounds unpredictable.

> Sorry for those wrong fixes. The good news is that the new fix does
> make code simpler and easier to maintain.

> I have tested the patchset on Intel platform with bare metal and Dom0
> kernels.

> Hi Sander,
>         Could you please help to test it again?

> Regards!
> Gerry

Hi Gerry,

These patches fix the first symptom of the powerbutton not working.

Unfortunately it doesn't fix the second symptoms with pci-passthrough,
the device still doesn't receive irq's on intel and the video device still haas
issues on AMD.

What i have tested extensively and works stable for me is:
david's patch + revert of cffe0a2b5a34c95a4dadc9ec7132690a5b0f6687 "x86, irq: Keep balance of IOAPIC pin reference count" 

Hope that helps in finding the solution.

--
Sander


> Jiang Liu (2):
>   xen/pci: Kill function xen_setup_acpi_sci()
>   xen/pci: Simplify x86/pci/xen.c by killing gsi_override related code

>  arch/x86/kernel/acpi/boot.c |   26 ++++++++---------
>  arch/x86/pci/xen.c          |   68 ++++---------------------------------------
>  2 files changed, 19 insertions(+), 75 deletions(-)

Re: [Bugfix v4 0/2] Fix xen IRQ allocation failure caused by commit b81975eade8c

[Jiang Liu]

On 2015/1/15 22:22, Sander Eikelenboom wrote:
> 
> Thursday, January 15, 2015, 2:04:34 PM, you wrote:
> 
>> With more knowledge of Xen interrupt manangement subsytem, I realized
>> previous three versions to fix https://lkml.org/lkml/2014/12/19/178 are
>> just plainly wrong. Those patches try to fix the issue by creating
>> irqdomain for IOAPICs for PV domains, which effectively let native
>> IOAPIC driver and Xen PV interrupt management subsystem to manage
>> IOAPIC irqs concurrently, sounds unpredictable.
> 
>> Sorry for those wrong fixes. The good news is that the new fix does
>> make code simpler and easier to maintain.
> 
>> I have tested the patchset on Intel platform with bare metal and Dom0
>> kernels.
> 
>> Hi Sander,
>>         Could you please help to test it again?
> 
>> Regards!
>> Gerry
> 
> Hi Gerry,
> 
> These patches fix the first symptom of the powerbutton not working.
> 
> Unfortunately it doesn't fix the second symptoms with pci-passthrough,
> the device still doesn't receive irq's on intel and the video device still haas
> issues on AMD.
> 
> What i have tested extensively and works stable for me is:
> david's patch + revert of cffe0a2b5a34c95a4dadc9ec7132690a5b0f6687 "x86, irq: Keep balance of IOAPIC pin reference count" 
> 
> Hope that helps in finding the solution.
Hi Sander,
	Yes, this is only a fix for power button(ACPI SCI allocation).
I'm still reading xen-pciback code to figure out the possible cause
and fix for the second issue.
Thanks!
Gerry.

> 
> --
> Sander
> 
> 
>> Jiang Liu (2):
>>   xen/pci: Kill function xen_setup_acpi_sci()
>>   xen/pci: Simplify x86/pci/xen.c by killing gsi_override related code
> 
>>  arch/x86/kernel/acpi/boot.c |   26 ++++++++---------
>>  arch/x86/pci/xen.c          |   68 ++++---------------------------------------
>>  2 files changed, 19 insertions(+), 75 deletions(-)
> 
> 

Re: [Bugfix v4 1/2] xen/pci: Fix xen IRQ allocation failure caused by commit b81975eade8c

[Konrad Rzeszutek Wilk]

On Thu, Jan 15, 2015 at 09:04:35PM +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.
> 
> Currently Xen Domain0 has special treatment for ACPI SCI interrupt,
> that is initialize irq for ACPI SCI at early stage in a special way as:
> xen_init_IRQ()
> 	->pci_xen_initial_domain()
> 		->xen_setup_acpi_sci()
> 			Allocate and initialize irq for ACPI SCI
> 
> Function xen_setup_acpi_sci() calls acpi_gsi_to_irq() to get an irq
> number for ACPI SCI. But unfortunately acpi_gsi_to_irq() depends on
> IOAPIC irqdomains through following path
> acpi_gsi_to_irq()
> 	->mp_map_gsi_to_irq()
> 		->mp_map_pin_to_irq()
> 			->check IOAPIC irqdomain
> 
> For PV domains, it uses Xen event based interrupt manangement and
> doesn't make uses of native IOAPIC, so no irqdomains created for IOAPIC.
> This causes Xen domain0 fails to install interrupt handler for ACPI SCI
> and all ACPI events will be lost. Please refer to:
> https://lkml.org/lkml/2014/12/19/178
> 
> So the fix is to get rid of special treatment for ACPI SCI, just treat
> ACPI SCI as normal GSI interrupt as:
> acpi_gsi_to_irq()
> 	->acpi_register_gsi()
> 		->acpi_register_gsi_xen()
> 			->xen_register_gsi()
> 
> With above change, there's no need for xen_setup_acpi_sci() anymore.
> The above change also works with bare metal kernel too.
> 
> Note: we need to test this patch on those special AMD systems which
> override normal SCI GSI (9) with strange GSI (20).

[    0.000000] DMI: Supermicro X8DTN/X8DTN, BIOS 2.1c       10/28/2011
..
     0.000000] ACPI: INT_SRC_OVR (bus 0 bus_irq 0 global_irq 2 dfl dfl)
[    0.000000] ACPI: INT_SRC_OVR (bus 0 bus_irq 9 global_irq 20 low level)
[    0.000000] ACPI: IRQ0 used by override.
..
    6.358737] xen: registering gsi 20 triggering 0 polarity 1
[    6.358756] xen: --> pirq=20 -> irq=20 (gsi=20)
...
-bash-4.1# cat /proc/interrupts |grep acpi
 20:          0          0          0  xen-pirq-ioapic-level  acpi

Re: [Bugfix v4 1/2] xen/pci: Fix xen IRQ allocation failure caused by commit b81975eade8c

[Konrad Rzeszutek Wilk]

On Thu, Jan 15, 2015 at 09:04:35PM +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.
> 
> Currently Xen Domain0 has special treatment for ACPI SCI interrupt,
> that is initialize irq for ACPI SCI at early stage in a special way as:
> xen_init_IRQ()
> 	->pci_xen_initial_domain()
> 		->xen_setup_acpi_sci()
> 			Allocate and initialize irq for ACPI SCI
> 
> Function xen_setup_acpi_sci() calls acpi_gsi_to_irq() to get an irq
> number for ACPI SCI. But unfortunately acpi_gsi_to_irq() depends on
> IOAPIC irqdomains through following path
> acpi_gsi_to_irq()
> 	->mp_map_gsi_to_irq()
> 		->mp_map_pin_to_irq()
> 			->check IOAPIC irqdomain
> 
> For PV domains, it uses Xen event based interrupt manangement and
> doesn't make uses of native IOAPIC, so no irqdomains created for IOAPIC.
> This causes Xen domain0 fails to install interrupt handler for ACPI SCI
> and all ACPI events will be lost. Please refer to:
> https://lkml.org/lkml/2014/12/19/178
> 
> So the fix is to get rid of special treatment for ACPI SCI, just treat
> ACPI SCI as normal GSI interrupt as:
> acpi_gsi_to_irq()
> 	->acpi_register_gsi()
> 		->acpi_register_gsi_xen()
> 			->xen_register_gsi()
> 
> With above change, there's no need for xen_setup_acpi_sci() anymore.
> The above change also works with bare metal kernel too.
> 
> Note: we need to test this patch on those special AMD systems which
> override normal SCI GSI (9) with strange GSI (20).

[    0.000000] DMI: Supermicro X8DTN/X8DTN, BIOS 2.1c       10/28/2011
..
     0.000000] ACPI: INT_SRC_OVR (bus 0 bus_irq 0 global_irq 2 dfl dfl)
[    0.000000] ACPI: INT_SRC_OVR (bus 0 bus_irq 9 global_irq 20 low level)
[    0.000000] ACPI: IRQ0 used by override.
..
    6.358737] xen: registering gsi 20 triggering 0 polarity 1
[    6.358756] xen: --> pirq=20 -> irq=20 (gsi=20)
...
-bash-4.1# cat /proc/interrupts |grep acpi
 20:          0          0          0  xen-pirq-ioapic-level  acpi

Re: [Bugfix v4 1/2] xen/pci: Fix xen IRQ allocation failure caused by commit b81975eade8c

[Jiang Liu]

On 2015/1/17 2:13, Konrad Rzeszutek Wilk wrote:
> On Thu, Jan 15, 2015 at 09:04:35PM +0800, Jiang Liu wrote:
<snit>
>>
>> Note: we need to test this patch on those special AMD systems which
>> override normal SCI GSI (9) with strange GSI (20).
> 
> [    0.000000] DMI: Supermicro X8DTN/X8DTN, BIOS 2.1c       10/28/2011
> ..
>      0.000000] ACPI: INT_SRC_OVR (bus 0 bus_irq 0 global_irq 2 dfl dfl)
> [    0.000000] ACPI: INT_SRC_OVR (bus 0 bus_irq 9 global_irq 20 low level)
> [    0.000000] ACPI: IRQ0 used by override.
> ..
>     6.358737] xen: registering gsi 20 triggering 0 polarity 1
> [    6.358756] xen: --> pirq=20 -> irq=20 (gsi=20)
> ...
> -bash-4.1# cat /proc/interrupts |grep acpi
>  20:          0          0          0  xen-pirq-ioapic-level  acpi
> 
Thanks Konrad, it works as expected. Previously IRQ9 was assigned
to ACPI SCI GSI 20, now it assigns IRQ20. But that's OK, ACPI has
no special requirement about the assigned IRQ number, we could
assign any valid IRQ number for ACPI SCI GSI.
Thanks!
Gerry

Re: [Bugfix v4 1/2] xen/pci: Fix xen IRQ allocation failure caused by commit b81975eade8c

[Jiang Liu]

On 2015/1/17 2:13, Konrad Rzeszutek Wilk wrote:
> On Thu, Jan 15, 2015 at 09:04:35PM +0800, Jiang Liu wrote:
<snit>
>>
>> Note: we need to test this patch on those special AMD systems which
>> override normal SCI GSI (9) with strange GSI (20).
> 
> [    0.000000] DMI: Supermicro X8DTN/X8DTN, BIOS 2.1c       10/28/2011
> ..
>      0.000000] ACPI: INT_SRC_OVR (bus 0 bus_irq 0 global_irq 2 dfl dfl)
> [    0.000000] ACPI: INT_SRC_OVR (bus 0 bus_irq 9 global_irq 20 low level)
> [    0.000000] ACPI: IRQ0 used by override.
> ..
>     6.358737] xen: registering gsi 20 triggering 0 polarity 1
> [    6.358756] xen: --> pirq=20 -> irq=20 (gsi=20)
> ...
> -bash-4.1# cat /proc/interrupts |grep acpi
>  20:          0          0          0  xen-pirq-ioapic-level  acpi
> 
Thanks Konrad, it works as expected. Previously IRQ9 was assigned
to ACPI SCI GSI 20, now it assigns IRQ20. But that's OK, ACPI has
no special requirement about the assigned IRQ number, we could
assign any valid IRQ number for ACPI SCI GSI.
Thanks!
Gerry

Re: [Bugfix v4 1/2] xen/pci: Fix xen IRQ allocation failure caused by commit b81975eade8c

[Jiang Liu]

On 2015/1/17 2:13, Konrad Rzeszutek Wilk wrote:
> On Thu, Jan 15, 2015 at 09:04:35PM +0800, Jiang Liu wrote:
<snit>
>>
>> Note: we need to test this patch on those special AMD systems which
>> override normal SCI GSI (9) with strange GSI (20).
> 
> [    0.000000] DMI: Supermicro X8DTN/X8DTN, BIOS 2.1c       10/28/2011
> ..
>      0.000000] ACPI: INT_SRC_OVR (bus 0 bus_irq 0 global_irq 2 dfl dfl)
> [    0.000000] ACPI: INT_SRC_OVR (bus 0 bus_irq 9 global_irq 20 low level)
> [    0.000000] ACPI: IRQ0 used by override.
> ..
>     6.358737] xen: registering gsi 20 triggering 0 polarity 1
> [    6.358756] xen: --> pirq=20 -> irq=20 (gsi=20)
> ...
> -bash-4.1# cat /proc/interrupts |grep acpi
>  20:          0          0          0  xen-pirq-ioapic-level  acpi
Hi Konrad,
	May I assume an Tested-by from you here?
Thanks!
Gerry

Re: [Bugfix v4 1/2] xen/pci: Fix xen IRQ allocation failure caused by commit b81975eade8c

[Jiang Liu]

On 2015/1/17 2:13, Konrad Rzeszutek Wilk wrote:
> On Thu, Jan 15, 2015 at 09:04:35PM +0800, Jiang Liu wrote:
<snit>
>>
>> Note: we need to test this patch on those special AMD systems which
>> override normal SCI GSI (9) with strange GSI (20).
> 
> [    0.000000] DMI: Supermicro X8DTN/X8DTN, BIOS 2.1c       10/28/2011
> ..
>      0.000000] ACPI: INT_SRC_OVR (bus 0 bus_irq 0 global_irq 2 dfl dfl)
> [    0.000000] ACPI: INT_SRC_OVR (bus 0 bus_irq 9 global_irq 20 low level)
> [    0.000000] ACPI: IRQ0 used by override.
> ..
>     6.358737] xen: registering gsi 20 triggering 0 polarity 1
> [    6.358756] xen: --> pirq=20 -> irq=20 (gsi=20)
> ...
> -bash-4.1# cat /proc/interrupts |grep acpi
>  20:          0          0          0  xen-pirq-ioapic-level  acpi
Hi Konrad,
	May I assume an Tested-by from you here?
Thanks!
Gerry

Re: [Bugfix v4 1/2] xen/pci: Fix xen IRQ allocation failure caused by commit b81975eade8c

[Konrad Rzeszutek Wilk]

On Mon, Jan 19, 2015 at 12:41:24PM +0800, Jiang Liu wrote:
> 
> On 2015/1/17 2:13, Konrad Rzeszutek Wilk wrote:
> > On Thu, Jan 15, 2015 at 09:04:35PM +0800, Jiang Liu wrote:
> <snit>
> >>
> >> Note: we need to test this patch on those special AMD systems which
> >> override normal SCI GSI (9) with strange GSI (20).
> > 
> > [    0.000000] DMI: Supermicro X8DTN/X8DTN, BIOS 2.1c       10/28/2011
> > ..
> >      0.000000] ACPI: INT_SRC_OVR (bus 0 bus_irq 0 global_irq 2 dfl dfl)
> > [    0.000000] ACPI: INT_SRC_OVR (bus 0 bus_irq 9 global_irq 20 low level)
> > [    0.000000] ACPI: IRQ0 used by override.
> > ..
> >     6.358737] xen: registering gsi 20 triggering 0 polarity 1
> > [    6.358756] xen: --> pirq=20 -> irq=20 (gsi=20)
> > ...
> > -bash-4.1# cat /proc/interrupts |grep acpi
> >  20:          0          0          0  xen-pirq-ioapic-level  acpi
> Hi Konrad,

> 	May I assume an Tested-by from you here?

Yes for this particular use case. I hadn't yet tried to push the ACPI buttons
(this printout was from an server in a basement).
> Thanks!
> Gerry

Re: [Bugfix v4 1/2] xen/pci: Fix xen IRQ allocation failure caused by commit b81975eade8c

[Konrad Rzeszutek Wilk]

On Mon, Jan 19, 2015 at 12:41:24PM +0800, Jiang Liu wrote:
> 
> On 2015/1/17 2:13, Konrad Rzeszutek Wilk wrote:
> > On Thu, Jan 15, 2015 at 09:04:35PM +0800, Jiang Liu wrote:
> <snit>
> >>
> >> Note: we need to test this patch on those special AMD systems which
> >> override normal SCI GSI (9) with strange GSI (20).
> > 
> > [    0.000000] DMI: Supermicro X8DTN/X8DTN, BIOS 2.1c       10/28/2011
> > ..
> >      0.000000] ACPI: INT_SRC_OVR (bus 0 bus_irq 0 global_irq 2 dfl dfl)
> > [    0.000000] ACPI: INT_SRC_OVR (bus 0 bus_irq 9 global_irq 20 low level)
> > [    0.000000] ACPI: IRQ0 used by override.
> > ..
> >     6.358737] xen: registering gsi 20 triggering 0 polarity 1
> > [    6.358756] xen: --> pirq=20 -> irq=20 (gsi=20)
> > ...
> > -bash-4.1# cat /proc/interrupts |grep acpi
> >  20:          0          0          0  xen-pirq-ioapic-level  acpi
> Hi Konrad,

> 	May I assume an Tested-by from you here?

Yes for this particular use case. I hadn't yet tried to push the ACPI buttons
(this printout was from an server in a basement).
> Thanks!
> Gerry

[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