[patch 1/5] x2apic, IR: cleanup X86_X2APIC and INTR_REMAP config checks

[patch 1/5] x2apic, IR: cleanup X86_X2APIC and INTR_REMAP config checks

[Suresh Siddha]

[-- Attachment #1: fix_ifdef_x2apic_intr_remap.patch --]
[-- Type: text/plain, Size: 7314 bytes --]

Add x2apic_supported() to cleanup CONFIG_X86_X2APIC checks.

Fix CONFIG_INTR_REMAP checks.

Signed-off-by: Suresh Siddha <suresh.b.siddha@intel.com>
Cc: Weidong Han <weidong.han@intel.com>
---

Index: tip/arch/x86/include/asm/apic.h
===================================================================
--- tip.orig/arch/x86/include/asm/apic.h
+++ tip/arch/x86/include/asm/apic.h
@@ -107,8 +107,7 @@ extern u32 native_safe_apic_wait_icr_idl
 extern void native_apic_icr_write(u32 low, u32 id);
 extern u64 native_apic_icr_read(void);
 
-#define EIM_8BIT_APIC_ID	0
-#define EIM_32BIT_APIC_ID	1
+extern int x2apic_mode;
 
 #ifdef CONFIG_X86_X2APIC
 /*
@@ -166,7 +165,7 @@ static inline u64 native_x2apic_icr_read
 	return val;
 }
 
-extern int x2apic, x2apic_phys;
+extern int x2apic_phys;
 extern void check_x2apic(void);
 extern void enable_x2apic(void);
 extern void x2apic_icr_write(u32 low, u32 id);
@@ -182,6 +181,8 @@ static inline int x2apic_enabled(void)
 		return 1;
 	return 0;
 }
+
+#define x2apic_supported()	(cpu_has_x2apic)
 #else
 static inline void check_x2apic(void)
 {
@@ -194,9 +195,8 @@ static inline int x2apic_enabled(void)
 	return 0;
 }
 
-#define	x2apic	0
 #define	x2apic_preenabled 0
-
+#define	x2apic_supported()	0
 #endif
 
 extern void enable_IR_x2apic(void);
Index: tip/arch/x86/include/asm/irq_remapping.h
===================================================================
--- tip.orig/arch/x86/include/asm/irq_remapping.h
+++ tip/arch/x86/include/asm/irq_remapping.h
@@ -1,6 +1,6 @@
 #ifndef _ASM_X86_IRQ_REMAPPING_H
 #define _ASM_X86_IRQ_REMAPPING_H
 
-#define IRTE_DEST(dest) ((x2apic) ? dest : dest << 8)
+#define IRTE_DEST(dest) ((x2apic_mode) ? dest : dest << 8)
 
 #endif	/* _ASM_X86_IRQ_REMAPPING_H */
Index: tip/arch/x86/kernel/apic/apic.c
===================================================================
--- tip.orig/arch/x86/kernel/apic/apic.c
+++ tip/arch/x86/kernel/apic/apic.c
@@ -135,8 +135,8 @@ static __init int setup_apicpmtimer(char
 __setup("apicpmtimer", setup_apicpmtimer);
 #endif
 
+int x2apic_mode;
 #ifdef CONFIG_X86_X2APIC
-int x2apic;
 /* x2apic enabled before OS handover */
 static int x2apic_preenabled;
 static int disable_x2apic;
@@ -861,7 +861,7 @@ void clear_local_APIC(void)
 	u32 v;
 
 	/* APIC hasn't been mapped yet */
-	if (!x2apic && !apic_phys)
+	if (!x2apic_mode && !apic_phys)
 		return;
 
 	maxlvt = lapic_get_maxlvt();
@@ -1334,7 +1334,7 @@ void check_x2apic(void)
 {
 	if (x2apic_enabled()) {
 		pr_info("x2apic enabled by BIOS, switching to x2apic ops\n");
-		x2apic_preenabled = x2apic = 1;
+		x2apic_preenabled = x2apic_mode = 1;
 	}
 }
 
@@ -1342,7 +1342,7 @@ void enable_x2apic(void)
 {
 	int msr, msr2;
 
-	if (!x2apic)
+	if (!x2apic_mode)
 		return;
 
 	rdmsr(MSR_IA32_APICBASE, msr, msr2);
@@ -1394,25 +1394,17 @@ void __init enable_IR_x2apic(void)
 	mask_IO_APIC_setup(ioapic_entries);
 	mask_8259A();
 
-#ifdef CONFIG_X86_X2APIC
-	if (cpu_has_x2apic)
-		ret = enable_intr_remapping(EIM_32BIT_APIC_ID);
-	else
-#endif
-		ret = enable_intr_remapping(EIM_8BIT_APIC_ID);
-
+	ret = enable_intr_remapping(x2apic_supported());
 	if (ret)
 		goto end_restore;
 
 	pr_info("Enabled Interrupt-remapping\n");
 
-#ifdef CONFIG_X86_X2APIC
-	if (cpu_has_x2apic && !x2apic) {
-		x2apic = 1;
+	if (x2apic_supported() && !x2apic_mode) {
+		x2apic_mode = 1;
 		enable_x2apic();
 		pr_info("Enabled x2apic\n");
 	}
-#endif
 
 end_restore:
 	if (ret)
@@ -1580,7 +1572,7 @@ void __init early_init_lapic_mapping(voi
  */
 void __init init_apic_mappings(void)
 {
-	if (x2apic) {
+	if (x2apic_mode) {
 		boot_cpu_physical_apicid = read_apic_id();
 		return;
 	}
@@ -2014,10 +2006,10 @@ static int lapic_suspend(struct sys_devi
 
 	local_irq_save(flags);
 	disable_local_APIC();
-#ifdef CONFIG_INTR_REMAP
+
 	if (intr_remapping_enabled)
 		disable_intr_remapping();
-#endif
+
 	local_irq_restore(flags);
 	return 0;
 }
@@ -2027,8 +2019,6 @@ static int lapic_resume(struct sys_devic
 	unsigned int l, h;
 	unsigned long flags;
 	int maxlvt;
-
-#ifdef CONFIG_INTR_REMAP
 	int ret;
 	struct IO_APIC_route_entry **ioapic_entries = NULL;
 
@@ -2054,17 +2044,8 @@ static int lapic_resume(struct sys_devic
 		mask_8259A();
 	}
 
-	if (x2apic)
+	if (x2apic_mode)
 		enable_x2apic();
-#else
-	if (!apic_pm_state.active)
-		return 0;
-
-	local_irq_save(flags);
-	if (x2apic)
-		enable_x2apic();
-#endif
-
 	else {
 		/*
 		 * Make sure the APICBASE points to the right address
@@ -2102,18 +2083,12 @@ static int lapic_resume(struct sys_devic
 	apic_write(APIC_ESR, 0);
 	apic_read(APIC_ESR);
 
-#ifdef CONFIG_INTR_REMAP
 	if (intr_remapping_enabled) {
-		if (x2apic)
-			reenable_intr_remapping(EIM_32BIT_APIC_ID);
-		else
-			reenable_intr_remapping(EIM_8BIT_APIC_ID);
-
+		reenable_intr_remapping(x2apic_mode);
 		unmask_8259A();
 		restore_IO_APIC_setup(ioapic_entries);
 		free_ioapic_entries(ioapic_entries);
 	}
-#endif
 
 	local_irq_restore(flags);
 
Index: tip/arch/x86/kernel/apic/probe_64.c
===================================================================
--- tip.orig/arch/x86/kernel/apic/probe_64.c
+++ tip/arch/x86/kernel/apic/probe_64.c
@@ -50,7 +50,7 @@ static struct apic *apic_probe[] __initd
 void __init default_setup_apic_routing(void)
 {
 #ifdef CONFIG_X86_X2APIC
-	if (x2apic && (apic != &apic_x2apic_phys &&
+	if (x2apic_mode && (apic != &apic_x2apic_phys &&
 #ifdef CONFIG_X86_UV
 		       apic != &apic_x2apic_uv_x &&
 #endif
Index: tip/include/linux/dmar.h
===================================================================
--- tip.orig/include/linux/dmar.h
+++ tip/include/linux/dmar.h
@@ -158,6 +158,8 @@ static inline struct intel_iommu *map_io
 }
 #define irq_remapped(irq)		(0)
 #define enable_intr_remapping(mode)	(-1)
+#define disable_intr_remapping()	(0)
+#define reenable_intr_remapping(mode)	(0)
 #define intr_remapping_enabled		(0)
 #endif
 
Index: tip/arch/x86/kernel/apic/io_apic.c
===================================================================
--- tip.orig/arch/x86/kernel/apic/io_apic.c
+++ tip/arch/x86/kernel/apic/io_apic.c
@@ -737,7 +737,6 @@ static int __init ioapic_pirq_setup(char
 __setup("pirq=", ioapic_pirq_setup);
 #endif /* CONFIG_X86_32 */
 
-#ifdef CONFIG_INTR_REMAP
 struct IO_APIC_route_entry **alloc_ioapic_entries(void)
 {
 	int apic;
@@ -858,7 +857,6 @@ void free_ioapic_entries(struct IO_APIC_
 
 	kfree(ioapic_entries);
 }
-#endif
 
 /*
  * Find the IRQ entry number of a certain pin.
Index: tip/arch/x86/include/asm/io_apic.h
===================================================================
--- tip.orig/arch/x86/include/asm/io_apic.h
+++ tip/arch/x86/include/asm/io_apic.h
@@ -161,7 +161,6 @@ extern int io_apic_set_pci_routing(int i
 extern int (*ioapic_renumber_irq)(int ioapic, int irq);
 extern void ioapic_init_mappings(void);
 
-#ifdef CONFIG_X86_64
 extern struct IO_APIC_route_entry **alloc_ioapic_entries(void);
 extern void free_ioapic_entries(struct IO_APIC_route_entry **ioapic_entries);
 extern int save_IO_APIC_setup(struct IO_APIC_route_entry **ioapic_entries);
@@ -169,7 +168,6 @@ extern void mask_IO_APIC_setup(struct IO
 extern int restore_IO_APIC_setup(struct IO_APIC_route_entry **ioapic_entries);
 extern void reinit_intr_remapped_IO_APIC(int intr_remapping,
 	struct IO_APIC_route_entry **ioapic_entries);
-#endif
 
 extern void probe_nr_irqs_gsi(void);
 

-- 

[patch 2/5] x2apic, IR: Fix compiler warning with !CONFIG_INTR_REMAP

[Suresh Siddha]

[-- Attachment #1: fix_ioapic_warning.patch --]
[-- Type: text/plain, Size: 1978 bytes --]

Fix complier warning:
arch/x86/kernel/apic/io_apic.c:2543: warning: `eoi_ioapic_irq' defined but not used

Signed-off-by: Suresh Siddha <suresh.b.siddha@intel.com>
Cc: Weidong Han <weidong.han@intel.com>
---

Index: tip/arch/x86/kernel/apic/io_apic.c
===================================================================
--- tip.orig/arch/x86/kernel/apic/io_apic.c
+++ tip/arch/x86/kernel/apic/io_apic.c
@@ -2518,39 +2518,6 @@ static void irq_complete_move(struct irq
 static inline void irq_complete_move(struct irq_desc **descp) {}
 #endif
 
-static void __eoi_ioapic_irq(unsigned int irq, struct irq_cfg *cfg)
-{
-	int apic, pin;
-	struct irq_pin_list *entry;
-
-	entry = cfg->irq_2_pin;
-	for (;;) {
-
-		if (!entry)
-			break;
-
-		apic = entry->apic;
-		pin = entry->pin;
-		io_apic_eoi(apic, pin);
-		entry = entry->next;
-	}
-}
-
-static void
-eoi_ioapic_irq(struct irq_desc *desc)
-{
-	struct irq_cfg *cfg;
-	unsigned long flags;
-	unsigned int irq;
-
-	irq = desc->irq;
-	cfg = desc->chip_data;
-
-	spin_lock_irqsave(&ioapic_lock, flags);
-	__eoi_ioapic_irq(irq, cfg);
-	spin_unlock_irqrestore(&ioapic_lock, flags);
-}
-
 static void ack_apic_edge(unsigned int irq)
 {
 	struct irq_desc *desc = irq_to_desc(irq);
@@ -2660,6 +2627,39 @@ static void ack_apic_level(unsigned int 
 }
 
 #ifdef CONFIG_INTR_REMAP
+static void __eoi_ioapic_irq(unsigned int irq, struct irq_cfg *cfg)
+{
+	int apic, pin;
+	struct irq_pin_list *entry;
+
+	entry = cfg->irq_2_pin;
+	for (;;) {
+
+		if (!entry)
+			break;
+
+		apic = entry->apic;
+		pin = entry->pin;
+		io_apic_eoi(apic, pin);
+		entry = entry->next;
+	}
+}
+
+static void
+eoi_ioapic_irq(struct irq_desc *desc)
+{
+	struct irq_cfg *cfg;
+	unsigned long flags;
+	unsigned int irq;
+
+	irq = desc->irq;
+	cfg = desc->chip_data;
+
+	spin_lock_irqsave(&ioapic_lock, flags);
+	__eoi_ioapic_irq(irq, cfg);
+	spin_unlock_irqrestore(&ioapic_lock, flags);
+}
+
 static void ir_ack_apic_edge(unsigned int irq)
 {
 	ack_APIC_irq();

-- 

[patch 3/5] x2apic, IR: cleanup panic() with nox2apic boot option

[Suresh Siddha]

[-- Attachment #1: fix_panic_msg.patch --]
[-- Type: text/plain, Size: 823 bytes --]

Instead of panic() ignore the "nox2apic" boot option when BIOS has
already enabled x2apic prior to OS handover.

Signed-off-by: Suresh Siddha <suresh.b.siddha@intel.com>
Cc: Weidong Han <weidong.han@intel.com>
---

Index: tip/arch/x86/kernel/apic/apic.c
===================================================================
--- tip.orig/arch/x86/kernel/apic/apic.c
+++ tip/arch/x86/kernel/apic/apic.c
@@ -142,8 +142,12 @@ static int x2apic_preenabled;
 static int disable_x2apic;
 static __init int setup_nox2apic(char *str)
 {
-	if (x2apic_enabled())
-		panic("Bios already enabled x2apic, can't enforce nox2apic");
+	if (x2apic_enabled()) {
+		pr_warning("Bios already enabled x2apic, "
+			   "can't enforce nox2apic");
+		return 0;
+	}
+
 	disable_x2apic = 1;
 	setup_clear_cpu_cap(X86_FEATURE_X2APIC);
 	return 0;

-- 

[patch 4/5] x2apic, IR: remove reinit_intr_remapped_IO_APIC()

[Suresh Siddha]

[-- Attachment #1: remove_reinit_intr_remapped_ioapic.patch --]
[-- Type: text/plain, Size: 2369 bytes --]


When interrupt-remapping is enabled, We are relying on setup_IO_APIC_irqs()
to configure remapped entries in the IO-APIC, which comes little bit later
after enabling interrupt-remapping.

Meanwhile, Restore of old io-apic entries after enabling interrupt-remapping
will not make the interrupts through io-apic functional anyway.

So remove unnecessary reinit_intr_remapped_IO_APIC().

Signed-off-by: Suresh Siddha <suresh.b.siddha@intel.com>
Cc: Weidong Han <weidong.han@intel.com>
---

Index: tip/arch/x86/include/asm/io_apic.h
===================================================================
--- tip.orig/arch/x86/include/asm/io_apic.h
+++ tip/arch/x86/include/asm/io_apic.h
@@ -166,8 +166,6 @@ extern void free_ioapic_entries(struct I
 extern int save_IO_APIC_setup(struct IO_APIC_route_entry **ioapic_entries);
 extern void mask_IO_APIC_setup(struct IO_APIC_route_entry **ioapic_entries);
 extern int restore_IO_APIC_setup(struct IO_APIC_route_entry **ioapic_entries);
-extern void reinit_intr_remapped_IO_APIC(int intr_remapping,
-	struct IO_APIC_route_entry **ioapic_entries);
 
 extern void probe_nr_irqs_gsi(void);
 
Index: tip/arch/x86/kernel/apic/apic.c
===================================================================
--- tip.orig/arch/x86/kernel/apic/apic.c
+++ tip/arch/x86/kernel/apic/apic.c
@@ -1416,8 +1416,6 @@ end_restore:
 		 * IR enabling failed
 		 */
 		restore_IO_APIC_setup(ioapic_entries);
-	else
-		reinit_intr_remapped_IO_APIC(x2apic_preenabled, ioapic_entries);
 
 	unmask_8259A();
 	local_irq_restore(flags);
Index: tip/arch/x86/kernel/apic/io_apic.c
===================================================================
--- tip.orig/arch/x86/kernel/apic/io_apic.c
+++ tip/arch/x86/kernel/apic/io_apic.c
@@ -834,20 +834,6 @@ int restore_IO_APIC_setup(struct IO_APIC
 	return 0;
 }
 
-void reinit_intr_remapped_IO_APIC(int intr_remapping,
-	struct IO_APIC_route_entry **ioapic_entries)
-
-{
-	/*
-	 * for now plain restore of previous settings.
-	 * TBD: In the case of OS enabling interrupt-remapping,
-	 * IO-APIC RTE's need to be setup to point to interrupt-remapping
-	 * table entries. for now, do a plain restore, and wait for
-	 * the setup_IO_APIC_irqs() to do proper initialization.
-	 */
-	restore_IO_APIC_setup(ioapic_entries);
-}
-
 void free_ioapic_entries(struct IO_APIC_route_entry **ioapic_entries)
 {
 	int apic;

-- 

[patch 5/5] x2apic, IR: Make config X86_UV dependent on X86_X2APIC

[Suresh Siddha]

[-- Attachment #1: fix_uv_x_config.patch --]
[-- Type: text/plain, Size: 750 bytes --]

Instead of selecting X86_X2APIC, make config X86_UV dependent on X86_X2APIC.
This will eliminate enabling CONFIG_X86_X2APIC with out enabling
CONFIG_INTR_REMAP.

Signed-off-by: Suresh Siddha <suresh.b.siddha@intel.com>
Cc: Weidong Han <weidong.han@intel.com>
Cc: Jack Steiner <steiner@sgi.com>
---

Index: tip/arch/x86/Kconfig
===================================================================
--- tip.orig/arch/x86/Kconfig
+++ tip/arch/x86/Kconfig
@@ -356,7 +356,7 @@ config X86_UV
 	depends on X86_64
 	depends on X86_EXTENDED_PLATFORM
 	depends on NUMA
-	select X86_X2APIC
+	depends on X86_X2APIC
 	---help---
 	  This option is needed in order to support SGI Ultraviolet systems.
 	  If you don't have one of these, you should say N here.

-- 

Re: [patch 5/5] x2apic, IR: Make config X86_UV dependent on X86_X2APIC

[Jack Steiner]

On Mon, Apr 20, 2009 at 01:02:31PM -0700, Suresh Siddha wrote:
> Instead of selecting X86_X2APIC, make config X86_UV dependent on X86_X2APIC.
> This will eliminate enabling CONFIG_X86_X2APIC with out enabling
> CONFIG_INTR_REMAP.
> 
> Signed-off-by: Suresh Siddha <suresh.b.siddha@intel.com>
> Cc: Weidong Han <weidong.han@intel.com>
> Cc: Jack Steiner <steiner@sgi.com>

Acked-by: Jack Steiner <steiner@sgi.com>

> ---
> 
> Index: tip/arch/x86/Kconfig
> ===================================================================
> --- tip.orig/arch/x86/Kconfig
> +++ tip/arch/x86/Kconfig
> @@ -356,7 +356,7 @@ config X86_UV
>  	depends on X86_64
>  	depends on X86_EXTENDED_PLATFORM
>  	depends on NUMA
> -	select X86_X2APIC
> +	depends on X86_X2APIC
>  	---help---
>  	  This option is needed in order to support SGI Ultraviolet systems.
>  	  If you don't have one of these, you should say N here.
> 
> -- 

RE: [patch 4/5] x2apic, IR: remove reinit_intr_remapped_IO_APIC()

[Han, Weidong]

Siddha, Suresh B wrote:
> When interrupt-remapping is enabled, We are relying on
> setup_IO_APIC_irqs() 
> to configure remapped entries in the IO-APIC, which comes little bit
> later after enabling interrupt-remapping.
> 
> Meanwhile, Restore of old io-apic entries after enabling
> interrupt-remapping will not make the interrupts through io-apic
> functional anyway. 
> 
> So remove unnecessary reinit_intr_remapped_IO_APIC().
> 
> Signed-off-by: Suresh Siddha <suresh.b.siddha@intel.com>
> Cc: Weidong Han <weidong.han@intel.com>
> ---
> 
> Index: tip/arch/x86/include/asm/io_apic.h
> ===================================================================
> --- tip.orig/arch/x86/include/asm/io_apic.h
> +++ tip/arch/x86/include/asm/io_apic.h
> @@ -166,8 +166,6 @@ extern void free_ioapic_entries(struct I
>  extern int save_IO_APIC_setup(struct IO_APIC_route_entry
>  **ioapic_entries); extern void mask_IO_APIC_setup(struct
>  IO_APIC_route_entry **ioapic_entries); extern int
> restore_IO_APIC_setup(struct IO_APIC_route_entry **ioapic_entries); 
> -extern void reinit_intr_remapped_IO_APIC(int intr_remapping,
> -	struct IO_APIC_route_entry **ioapic_entries);
> 
>  extern void probe_nr_irqs_gsi(void);
> 
> Index: tip/arch/x86/kernel/apic/apic.c
> ===================================================================
> --- tip.orig/arch/x86/kernel/apic/apic.c
> +++ tip/arch/x86/kernel/apic/apic.c
> @@ -1416,8 +1416,6 @@ end_restore:
>  		 * IR enabling failed
>  		 */
>  		restore_IO_APIC_setup(ioapic_entries);
> -	else
> -		reinit_intr_remapped_IO_APIC(x2apic_preenabled, ioapic_entries);

Whether IR enabling succeeds or fails, it always needs to restore old IOAPIC entries. Due to removing reinit_intr_remapped_IO_APIC here, it needs to also remove the "if (ret)" before restore_IO_APIC_setup(ioapic_entries);

Regards,
Weidong

Re: [patch 1/5] x2apic, IR: cleanup X86_X2APIC and INTR_REMAP config checks

[Ingo Molnar]

* Suresh Siddha <suresh.b.siddha@intel.com> wrote:

> Add x2apic_supported() to cleanup CONFIG_X86_X2APIC checks.
> 
> Fix CONFIG_INTR_REMAP checks.
> 
> Signed-off-by: Suresh Siddha <suresh.b.siddha@intel.com>
> Cc: Weidong Han <weidong.han@intel.com>
> ---
> 
> Index: tip/arch/x86/include/asm/apic.h

Ob'nit'pick: please generate diffstats for future patches as it 
makes in-email review much easier.

The diffstat is:

 arch/x86/include/asm/apic.h          |   10 +++----
 arch/x86/include/asm/io_apic.h       |    2 -
 arch/x86/include/asm/irq_remapping.h |    2 -
 arch/x86/kernel/apic/apic.c          |   49 ++++++++---------------------------
 arch/x86/kernel/apic/io_apic.c       |    2 -
 arch/x86/kernel/apic/probe_64.c      |    2 -
 include/linux/dmar.h                 |    2 +
 7 files changed, 21 insertions(+), 48 deletions(-)

Which already shows that the patch has a very nice cleanup factor.

I've added an "[ Impact: cleanup ]" tag to it - the patch is not 
supposed to have any functional side-effects, right? (i.e. has no 
fixes embedded in it.)

a question:

>  	if (intr_remapping_enabled)
>  		disable_intr_remapping();

Could the intr_remapping_enabled check be moved inside 
disable_intr_remapping(), and thus the ugly 'if (flag)' construct 
could go away?

>  	if (intr_remapping_enabled) {
> +		reenable_intr_remapping(x2apic_mode);
>  		unmask_8259A();
>  		restore_IO_APIC_setup(ioapic_entries);
>  		free_ioapic_entries(ioapic_entries);
>  	}

Ditto - shouldnt this sequence move into reenable_intr_remapping() - 
or into a new helper function? (or is it too single-purpose)

Anyway, this already looks good as-is so i can apply it if you do 
delta patches on top of it to improve it further.

	Ingo

Re: [patch 3/5] x2apic, IR: cleanup panic() with nox2apic boot option

[Ingo Molnar]

* Suresh Siddha <suresh.b.siddha@intel.com> wrote:

> Instead of panic() ignore the "nox2apic" boot option when BIOS has
> already enabled x2apic prior to OS handover.
> 
> Signed-off-by: Suresh Siddha <suresh.b.siddha@intel.com>
> Cc: Weidong Han <weidong.han@intel.com>
> ---
> 
> Index: tip/arch/x86/kernel/apic/apic.c
> ===================================================================
> --- tip.orig/arch/x86/kernel/apic/apic.c
> +++ tip/arch/x86/kernel/apic/apic.c
> @@ -142,8 +142,12 @@ static int x2apic_preenabled;
>  static int disable_x2apic;
>  static __init int setup_nox2apic(char *str)
>  {
> -	if (x2apic_enabled())
> -		panic("Bios already enabled x2apic, can't enforce nox2apic");
> +	if (x2apic_enabled()) {
> +		pr_warning("Bios already enabled x2apic, "
> +			   "can't enforce nox2apic");

Btw., in such cases we dont break printk strings in the middle but 
ignore the checkpatch warning. I fixed it up in your patch.

The advantage is that if someone, based on a dmesg line, types:

   git grep Bios already enabled x2apic, can't enforce nox2apic

the right source code line will be found. In the broken-up-line case 
this wont work. (grep is fundamentally single-line)

	Ingo

Re: [patch 4/5] x2apic, IR: remove reinit_intr_remapped_IO_APIC()

[Ingo Molnar]

* Han, Weidong <weidong.han@intel.com> wrote:

> Siddha, Suresh B wrote:
> > When interrupt-remapping is enabled, We are relying on
> > setup_IO_APIC_irqs() 
> > to configure remapped entries in the IO-APIC, which comes little bit
> > later after enabling interrupt-remapping.
> > 
> > Meanwhile, Restore of old io-apic entries after enabling
> > interrupt-remapping will not make the interrupts through io-apic
> > functional anyway. 
> > 
> > So remove unnecessary reinit_intr_remapped_IO_APIC().
> > 
> > Signed-off-by: Suresh Siddha <suresh.b.siddha@intel.com>
> > Cc: Weidong Han <weidong.han@intel.com>
> > ---
> > 
> > Index: tip/arch/x86/include/asm/io_apic.h
> > ===================================================================
> > --- tip.orig/arch/x86/include/asm/io_apic.h
> > +++ tip/arch/x86/include/asm/io_apic.h
> > @@ -166,8 +166,6 @@ extern void free_ioapic_entries(struct I
> >  extern int save_IO_APIC_setup(struct IO_APIC_route_entry
> >  **ioapic_entries); extern void mask_IO_APIC_setup(struct
> >  IO_APIC_route_entry **ioapic_entries); extern int
> > restore_IO_APIC_setup(struct IO_APIC_route_entry **ioapic_entries); 
> > -extern void reinit_intr_remapped_IO_APIC(int intr_remapping,
> > -	struct IO_APIC_route_entry **ioapic_entries);
> > 
> >  extern void probe_nr_irqs_gsi(void);
> > 
> > Index: tip/arch/x86/kernel/apic/apic.c
> > ===================================================================
> > --- tip.orig/arch/x86/kernel/apic/apic.c
> > +++ tip/arch/x86/kernel/apic/apic.c
> > @@ -1416,8 +1416,6 @@ end_restore:
> >  		 * IR enabling failed
> >  		 */
> >  		restore_IO_APIC_setup(ioapic_entries);
> > -	else
> > -		reinit_intr_remapped_IO_APIC(x2apic_preenabled, ioapic_entries);
> 
> Whether IR enabling succeeds or fails, it always needs to restore 
> old IOAPIC entries. Due to removing reinit_intr_remapped_IO_APIC 
> here, it needs to also remove the "if (ret)" before 
> restore_IO_APIC_setup(ioapic_entries);

Ok - i skipped this patch for now.

	Ingo

[tip:x86/apic] x86: x2apic, IR: Clean up X86_X2APIC and INTR_REMAP config checks

[tip-bot for Suresh Siddha]

Commit-ID:  fc1edaf9e7cc4d4696f83dee495b8f158d01c4eb
Gitweb:     http://git.kernel.org/tip/fc1edaf9e7cc4d4696f83dee495b8f158d01c4eb
Author:     Suresh Siddha <suresh.b.siddha@intel.com>
AuthorDate: Mon, 20 Apr 2009 13:02:27 -0700
Committer:  Ingo Molnar <mingo@elte.hu>
CommitDate: Tue, 21 Apr 2009 09:08:25 +0200

x86: x2apic, IR: Clean up X86_X2APIC and INTR_REMAP config checks

Add x2apic_supported() to clean up CONFIG_X86_X2APIC checks.

Fix CONFIG_INTR_REMAP checks.

[ Impact: cleanup ]

Signed-off-by: Suresh Siddha <suresh.b.siddha@intel.com>
Cc: dwmw2@infradead.org
Cc: Suresh Siddha <suresh.b.siddha@intel.com>
Cc: Weidong Han <weidong.han@intel.com>
LKML-Reference: <20090420200450.128993000@linux-os.sc.intel.com>
Signed-off-by: Ingo Molnar <mingo@elte.hu>


---
 arch/x86/include/asm/apic.h          |   10 +++---
 arch/x86/include/asm/io_apic.h       |    2 -
 arch/x86/include/asm/irq_remapping.h |    2 +-
 arch/x86/kernel/apic/apic.c          |   49 ++++++++-------------------------
 arch/x86/kernel/apic/io_apic.c       |    2 -
 arch/x86/kernel/apic/probe_64.c      |    2 +-
 include/linux/dmar.h                 |    2 +
 7 files changed, 21 insertions(+), 48 deletions(-)

diff --git a/arch/x86/include/asm/apic.h b/arch/x86/include/asm/apic.h
index fbdd654..3738438 100644
--- a/arch/x86/include/asm/apic.h
+++ b/arch/x86/include/asm/apic.h
@@ -107,8 +107,7 @@ extern u32 native_safe_apic_wait_icr_idle(void);
 extern void native_apic_icr_write(u32 low, u32 id);
 extern u64 native_apic_icr_read(void);
 
-#define EIM_8BIT_APIC_ID	0
-#define EIM_32BIT_APIC_ID	1
+extern int x2apic_mode;
 
 #ifdef CONFIG_X86_X2APIC
 /*
@@ -166,7 +165,7 @@ static inline u64 native_x2apic_icr_read(void)
 	return val;
 }
 
-extern int x2apic, x2apic_phys;
+extern int x2apic_phys;
 extern void check_x2apic(void);
 extern void enable_x2apic(void);
 extern void x2apic_icr_write(u32 low, u32 id);
@@ -182,6 +181,8 @@ static inline int x2apic_enabled(void)
 		return 1;
 	return 0;
 }
+
+#define x2apic_supported()	(cpu_has_x2apic)
 #else
 static inline void check_x2apic(void)
 {
@@ -194,9 +195,8 @@ static inline int x2apic_enabled(void)
 	return 0;
 }
 
-#define	x2apic	0
 #define	x2apic_preenabled 0
-
+#define	x2apic_supported()	0
 #endif
 
 extern void enable_IR_x2apic(void);
diff --git a/arch/x86/include/asm/io_apic.h b/arch/x86/include/asm/io_apic.h
index 9d826e4..34eaa37 100644
--- a/arch/x86/include/asm/io_apic.h
+++ b/arch/x86/include/asm/io_apic.h
@@ -161,7 +161,6 @@ extern int io_apic_set_pci_routing(int ioapic, int pin, int irq,
 extern int (*ioapic_renumber_irq)(int ioapic, int irq);
 extern void ioapic_init_mappings(void);
 
-#ifdef CONFIG_X86_64
 extern struct IO_APIC_route_entry **alloc_ioapic_entries(void);
 extern void free_ioapic_entries(struct IO_APIC_route_entry **ioapic_entries);
 extern int save_IO_APIC_setup(struct IO_APIC_route_entry **ioapic_entries);
@@ -169,7 +168,6 @@ extern void mask_IO_APIC_setup(struct IO_APIC_route_entry **ioapic_entries);
 extern int restore_IO_APIC_setup(struct IO_APIC_route_entry **ioapic_entries);
 extern void reinit_intr_remapped_IO_APIC(int intr_remapping,
 	struct IO_APIC_route_entry **ioapic_entries);
-#endif
 
 extern void probe_nr_irqs_gsi(void);
 
diff --git a/arch/x86/include/asm/irq_remapping.h b/arch/x86/include/asm/irq_remapping.h
index 0396760..f275e22 100644
--- a/arch/x86/include/asm/irq_remapping.h
+++ b/arch/x86/include/asm/irq_remapping.h
@@ -1,6 +1,6 @@
 #ifndef _ASM_X86_IRQ_REMAPPING_H
 #define _ASM_X86_IRQ_REMAPPING_H
 
-#define IRTE_DEST(dest) ((x2apic) ? dest : dest << 8)
+#define IRTE_DEST(dest) ((x2apic_mode) ? dest : dest << 8)
 
 #endif	/* _ASM_X86_IRQ_REMAPPING_H */
diff --git a/arch/x86/kernel/apic/apic.c b/arch/x86/kernel/apic/apic.c
index 7b41a32..2b30e52 100644
--- a/arch/x86/kernel/apic/apic.c
+++ b/arch/x86/kernel/apic/apic.c
@@ -134,8 +134,8 @@ static __init int setup_apicpmtimer(char *s)
 __setup("apicpmtimer", setup_apicpmtimer);
 #endif
 
+int x2apic_mode;
 #ifdef CONFIG_X86_X2APIC
-int x2apic;
 /* x2apic enabled before OS handover */
 static int x2apic_preenabled;
 static int disable_x2apic;
@@ -858,7 +858,7 @@ void clear_local_APIC(void)
 	u32 v;
 
 	/* APIC hasn't been mapped yet */
-	if (!x2apic && !apic_phys)
+	if (!x2apic_mode && !apic_phys)
 		return;
 
 	maxlvt = lapic_get_maxlvt();
@@ -1330,7 +1330,7 @@ void check_x2apic(void)
 {
 	if (x2apic_enabled()) {
 		pr_info("x2apic enabled by BIOS, switching to x2apic ops\n");
-		x2apic_preenabled = x2apic = 1;
+		x2apic_preenabled = x2apic_mode = 1;
 	}
 }
 
@@ -1338,7 +1338,7 @@ void enable_x2apic(void)
 {
 	int msr, msr2;
 
-	if (!x2apic)
+	if (!x2apic_mode)
 		return;
 
 	rdmsr(MSR_IA32_APICBASE, msr, msr2);
@@ -1390,25 +1390,17 @@ void __init enable_IR_x2apic(void)
 	mask_IO_APIC_setup(ioapic_entries);
 	mask_8259A();
 
-#ifdef CONFIG_X86_X2APIC
-	if (cpu_has_x2apic)
-		ret = enable_intr_remapping(EIM_32BIT_APIC_ID);
-	else
-#endif
-		ret = enable_intr_remapping(EIM_8BIT_APIC_ID);
-
+	ret = enable_intr_remapping(x2apic_supported());
 	if (ret)
 		goto end_restore;
 
 	pr_info("Enabled Interrupt-remapping\n");
 
-#ifdef CONFIG_X86_X2APIC
-	if (cpu_has_x2apic && !x2apic) {
-		x2apic = 1;
+	if (x2apic_supported() && !x2apic_mode) {
+		x2apic_mode = 1;
 		enable_x2apic();
 		pr_info("Enabled x2apic\n");
 	}
-#endif
 
 end_restore:
 	if (ret)
@@ -1576,7 +1568,7 @@ void __init early_init_lapic_mapping(void)
  */
 void __init init_apic_mappings(void)
 {
-	if (x2apic) {
+	if (x2apic_mode) {
 		boot_cpu_physical_apicid = read_apic_id();
 		return;
 	}
@@ -2010,10 +2002,10 @@ static int lapic_suspend(struct sys_device *dev, pm_message_t state)
 
 	local_irq_save(flags);
 	disable_local_APIC();
-#ifdef CONFIG_INTR_REMAP
+
 	if (intr_remapping_enabled)
 		disable_intr_remapping();
-#endif
+
 	local_irq_restore(flags);
 	return 0;
 }
@@ -2023,8 +2015,6 @@ static int lapic_resume(struct sys_device *dev)
 	unsigned int l, h;
 	unsigned long flags;
 	int maxlvt;
-
-#ifdef CONFIG_INTR_REMAP
 	int ret;
 	struct IO_APIC_route_entry **ioapic_entries = NULL;
 
@@ -2050,17 +2040,8 @@ static int lapic_resume(struct sys_device *dev)
 		mask_8259A();
 	}
 
-	if (x2apic)
+	if (x2apic_mode)
 		enable_x2apic();
-#else
-	if (!apic_pm_state.active)
-		return 0;
-
-	local_irq_save(flags);
-	if (x2apic)
-		enable_x2apic();
-#endif
-
 	else {
 		/*
 		 * Make sure the APICBASE points to the right address
@@ -2098,18 +2079,12 @@ static int lapic_resume(struct sys_device *dev)
 	apic_write(APIC_ESR, 0);
 	apic_read(APIC_ESR);
 
-#ifdef CONFIG_INTR_REMAP
 	if (intr_remapping_enabled) {
-		if (x2apic)
-			reenable_intr_remapping(EIM_32BIT_APIC_ID);
-		else
-			reenable_intr_remapping(EIM_8BIT_APIC_ID);
-
+		reenable_intr_remapping(x2apic_mode);
 		unmask_8259A();
 		restore_IO_APIC_setup(ioapic_entries);
 		free_ioapic_entries(ioapic_entries);
 	}
-#endif
 
 	local_irq_restore(flags);
 
diff --git a/arch/x86/kernel/apic/io_apic.c b/arch/x86/kernel/apic/io_apic.c
index ea22a86..3a45d2e 100644
--- a/arch/x86/kernel/apic/io_apic.c
+++ b/arch/x86/kernel/apic/io_apic.c
@@ -736,7 +736,6 @@ static int __init ioapic_pirq_setup(char *str)
 __setup("pirq=", ioapic_pirq_setup);
 #endif /* CONFIG_X86_32 */
 
-#ifdef CONFIG_INTR_REMAP
 struct IO_APIC_route_entry **alloc_ioapic_entries(void)
 {
 	int apic;
@@ -857,7 +856,6 @@ void free_ioapic_entries(struct IO_APIC_route_entry **ioapic_entries)
 
 	kfree(ioapic_entries);
 }
-#endif
 
 /*
  * Find the IRQ entry number of a certain pin.
diff --git a/arch/x86/kernel/apic/probe_64.c b/arch/x86/kernel/apic/probe_64.c
index 1783652..bc3e880 100644
--- a/arch/x86/kernel/apic/probe_64.c
+++ b/arch/x86/kernel/apic/probe_64.c
@@ -50,7 +50,7 @@ static struct apic *apic_probe[] __initdata = {
 void __init default_setup_apic_routing(void)
 {
 #ifdef CONFIG_X86_X2APIC
-	if (x2apic && (apic != &apic_x2apic_phys &&
+	if (x2apic_mode && (apic != &apic_x2apic_phys &&
 #ifdef CONFIG_X86_UV
 		       apic != &apic_x2apic_uv_x &&
 #endif
diff --git a/include/linux/dmar.h b/include/linux/dmar.h
index 06f592a..10ff5c4 100644
--- a/include/linux/dmar.h
+++ b/include/linux/dmar.h
@@ -158,6 +158,8 @@ static inline struct intel_iommu *map_ioapic_to_ir(int apic)
 }
 #define irq_remapped(irq)		(0)
 #define enable_intr_remapping(mode)	(-1)
+#define disable_intr_remapping()	(0)
+#define reenable_intr_remapping(mode)	(0)
 #define intr_remapping_enabled		(0)
 #endif
 

[tip:x86/apic] x86: x2apic, IR: Move eoi_ioapic_irq() into a CONFIG_INTR_REMAP section

[tip-bot for Suresh Siddha]

Commit-ID:  25629d810a52176758401184d9b437fbb7f79195
Gitweb:     http://git.kernel.org/tip/25629d810a52176758401184d9b437fbb7f79195
Author:     Suresh Siddha <suresh.b.siddha@intel.com>
AuthorDate: Mon, 20 Apr 2009 13:02:28 -0700
Committer:  Ingo Molnar <mingo@elte.hu>
CommitDate: Tue, 21 Apr 2009 09:08:26 +0200

x86: x2apic, IR: Move eoi_ioapic_irq() into a CONFIG_INTR_REMAP section

Address the following complier warning:

   arch/x86/kernel/apic/io_apic.c:2543: warning: `eoi_ioapic_irq' defined but not used

By moving that function (and eoi_ioapic_irq()) into an existing
#ifdef CONFIG_INTR_REMAP section of the code.

[ Impact: cleanup ]

Signed-off-by: Suresh Siddha <suresh.b.siddha@intel.com>
Cc: dwmw2@infradead.org
Cc: Weidong Han <weidong.han@intel.com>
LKML-Reference: <20090420200450.271099000@linux-os.sc.intel.com>
Signed-off-by: Ingo Molnar <mingo@elte.hu>
Cc: Weidong Han <weidong.han@intel.com>


---
 arch/x86/kernel/apic/io_apic.c |   66 ++++++++++++++++++++--------------------
 1 files changed, 33 insertions(+), 33 deletions(-)

diff --git a/arch/x86/kernel/apic/io_apic.c b/arch/x86/kernel/apic/io_apic.c
index 3a45d2e..4baa9cb 100644
--- a/arch/x86/kernel/apic/io_apic.c
+++ b/arch/x86/kernel/apic/io_apic.c
@@ -2517,39 +2517,6 @@ static void irq_complete_move(struct irq_desc **descp)
 static inline void irq_complete_move(struct irq_desc **descp) {}
 #endif
 
-static void __eoi_ioapic_irq(unsigned int irq, struct irq_cfg *cfg)
-{
-	int apic, pin;
-	struct irq_pin_list *entry;
-
-	entry = cfg->irq_2_pin;
-	for (;;) {
-
-		if (!entry)
-			break;
-
-		apic = entry->apic;
-		pin = entry->pin;
-		io_apic_eoi(apic, pin);
-		entry = entry->next;
-	}
-}
-
-static void
-eoi_ioapic_irq(struct irq_desc *desc)
-{
-	struct irq_cfg *cfg;
-	unsigned long flags;
-	unsigned int irq;
-
-	irq = desc->irq;
-	cfg = desc->chip_data;
-
-	spin_lock_irqsave(&ioapic_lock, flags);
-	__eoi_ioapic_irq(irq, cfg);
-	spin_unlock_irqrestore(&ioapic_lock, flags);
-}
-
 static void ack_apic_edge(unsigned int irq)
 {
 	struct irq_desc *desc = irq_to_desc(irq);
@@ -2659,6 +2626,39 @@ static void ack_apic_level(unsigned int irq)
 }
 
 #ifdef CONFIG_INTR_REMAP
+static void __eoi_ioapic_irq(unsigned int irq, struct irq_cfg *cfg)
+{
+	int apic, pin;
+	struct irq_pin_list *entry;
+
+	entry = cfg->irq_2_pin;
+	for (;;) {
+
+		if (!entry)
+			break;
+
+		apic = entry->apic;
+		pin = entry->pin;
+		io_apic_eoi(apic, pin);
+		entry = entry->next;
+	}
+}
+
+static void
+eoi_ioapic_irq(struct irq_desc *desc)
+{
+	struct irq_cfg *cfg;
+	unsigned long flags;
+	unsigned int irq;
+
+	irq = desc->irq;
+	cfg = desc->chip_data;
+
+	spin_lock_irqsave(&ioapic_lock, flags);
+	__eoi_ioapic_irq(irq, cfg);
+	spin_unlock_irqrestore(&ioapic_lock, flags);
+}
+
 static void ir_ack_apic_edge(unsigned int irq)
 {
 	ack_APIC_irq();

[tip:x86/apic] x86: x2apic, IR: Clean up panic() with nox2apic boot option

[tip-bot for Suresh Siddha]

Commit-ID:  39d83a5d684a457046aa2a6dac60f105966e78e9
Gitweb:     http://git.kernel.org/tip/39d83a5d684a457046aa2a6dac60f105966e78e9
Author:     Suresh Siddha <suresh.b.siddha@intel.com>
AuthorDate: Mon, 20 Apr 2009 13:02:29 -0700
Committer:  Ingo Molnar <mingo@elte.hu>
CommitDate: Tue, 21 Apr 2009 09:08:26 +0200

x86: x2apic, IR: Clean up panic() with nox2apic boot option

Instead of panic() ignore the "nox2apic" boot option when BIOS
has already enabled x2apic prior to OS handover.

[ Impact: printk warning instead of panic() when BIOS has enabled x2apic already ]

Signed-off-by: Suresh Siddha <suresh.b.siddha@intel.com>
Cc: dwmw2@infradead.org
Cc: Weidong Han <weidong.han@intel.com>
LKML-Reference: <20090420200450.425091000@linux-os.sc.intel.com>
Signed-off-by: Ingo Molnar <mingo@elte.hu>


---
 arch/x86/kernel/apic/apic.c |    8 ++++++--
 1 files changed, 6 insertions(+), 2 deletions(-)

diff --git a/arch/x86/kernel/apic/apic.c b/arch/x86/kernel/apic/apic.c
index 2b30e52..d32f558 100644
--- a/arch/x86/kernel/apic/apic.c
+++ b/arch/x86/kernel/apic/apic.c
@@ -141,8 +141,12 @@ static int x2apic_preenabled;
 static int disable_x2apic;
 static __init int setup_nox2apic(char *str)
 {
-	if (x2apic_enabled())
-		panic("Bios already enabled x2apic, can't enforce nox2apic");
+	if (x2apic_enabled()) {
+		pr_warning("Bios already enabled x2apic, "
+			   "can't enforce nox2apic");
+		return 0;
+	}
+
 	disable_x2apic = 1;
 	setup_clear_cpu_cap(X86_FEATURE_X2APIC);
 	return 0;

[tip:x86/apic] x86: x2apic, IR: Make config X86_UV dependent on X86_X2APIC

[tip-bot for Suresh Siddha]

Commit-ID:  9d6c26e73bd248c286bb3597aaf788716e8fcceb
Gitweb:     http://git.kernel.org/tip/9d6c26e73bd248c286bb3597aaf788716e8fcceb
Author:     Suresh Siddha <suresh.b.siddha@intel.com>
AuthorDate: Mon, 20 Apr 2009 13:02:31 -0700
Committer:  Ingo Molnar <mingo@elte.hu>
CommitDate: Tue, 21 Apr 2009 09:08:27 +0200

x86: x2apic, IR: Make config X86_UV dependent on X86_X2APIC

Instead of selecting X86_X2APIC, make config X86_UV dependent
on X86_X2APIC.

This will eliminate enabling CONFIG_X86_X2APIC with out
enabling CONFIG_INTR_REMAP.

[ Impact: cleanup ]

Signed-off-by: Suresh Siddha <suresh.b.siddha@intel.com>
Acked-by: Jack Steiner <steiner@sgi.com>
Cc: dwmw2@infradead.org
Cc: Suresh Siddha <suresh.b.siddha@intel.com>
Cc: Weidong Han <weidong.han@intel.com>
LKML-Reference: <20090420200450.694598000@linux-os.sc.intel.com>
Signed-off-by: Ingo Molnar <mingo@elte.hu>


---
 arch/x86/Kconfig |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
index c9086e6..58fb7b3 100644
--- a/arch/x86/Kconfig
+++ b/arch/x86/Kconfig
@@ -354,7 +354,7 @@ config X86_UV
 	depends on X86_64
 	depends on X86_EXTENDED_PLATFORM
 	depends on NUMA
-	select X86_X2APIC
+	depends on X86_X2APIC
 	---help---
 	  This option is needed in order to support SGI Ultraviolet systems.
 	  If you don't have one of these, you should say N here.

Re: [tip:x86/apic] x86: x2apic, IR: Move eoi_ioapic_irq() into a CONFIG_INTR_REMAP section

[Cyrill Gorcunov]

[tip-bot for Suresh Siddha - Tue, Apr 21, 2009 at 08:07:00AM +0000]
| Commit-ID:  25629d810a52176758401184d9b437fbb7f79195
| Gitweb:     http://git.kernel.org/tip/25629d810a52176758401184d9b437fbb7f79195
| Author:     Suresh Siddha <suresh.b.siddha@intel.com>
| AuthorDate: Mon, 20 Apr 2009 13:02:28 -0700
| Committer:  Ingo Molnar <mingo@elte.hu>
| CommitDate: Tue, 21 Apr 2009 09:08:26 +0200
| 
| x86: x2apic, IR: Move eoi_ioapic_irq() into a CONFIG_INTR_REMAP section
| 
| Address the following complier warning:
| 
|    arch/x86/kernel/apic/io_apic.c:2543: warning: `eoi_ioapic_irq' defined but not used
| 
| By moving that function (and eoi_ioapic_irq()) into an existing
| #ifdef CONFIG_INTR_REMAP section of the code.
| 
| [ Impact: cleanup ]
| 
| Signed-off-by: Suresh Siddha <suresh.b.siddha@intel.com>
| Cc: dwmw2@infradead.org
| Cc: Weidong Han <weidong.han@intel.com>
| LKML-Reference: <20090420200450.271099000@linux-os.sc.intel.com>
| Signed-off-by: Ingo Molnar <mingo@elte.hu>
| Cc: Weidong Han <weidong.han@intel.com>
| 
| 
| ---
|  arch/x86/kernel/apic/io_apic.c |   66 ++++++++++++++++++++--------------------
|  1 files changed, 33 insertions(+), 33 deletions(-)
| 
...  
|  #ifdef CONFIG_INTR_REMAP
| +static void __eoi_ioapic_irq(unsigned int irq, struct irq_cfg *cfg)
| +{
| +	int apic, pin;
| +	struct irq_pin_list *entry;
| +
| +	entry = cfg->irq_2_pin;
| +	for (;;) {
| +
| +		if (!entry)
| +			break;
| +
| +		apic = entry->apic;
| +		pin = entry->pin;
| +		io_apic_eoi(apic, pin);
| +		entry = entry->next;
| +	}
| +}
| +
...

Btw, maybe we better could use plain "while(entry) {...}" here?

        Cyrill

Re: [patch 4/5] x2apic, IR: remove reinit_intr_remapped_IO_APIC()

[Suresh Siddha]

On Tue, 2009-04-21 at 00:01 -0700, Ingo Molnar wrote:
> * Han, Weidong <weidong.han@intel.com> wrote:
> 
> > Siddha, Suresh B wrote:
> > > When interrupt-remapping is enabled, We are relying on
> > > setup_IO_APIC_irqs() 
> > > to configure remapped entries in the IO-APIC, which comes little bit
> > > later after enabling interrupt-remapping.
> > > 
> > > Meanwhile, Restore of old io-apic entries after enabling
> > > interrupt-remapping will not make the interrupts through io-apic
> > > functional anyway. 
> > > 
> > > So remove unnecessary reinit_intr_remapped_IO_APIC().
> > > 
> > > Signed-off-by: Suresh Siddha <suresh.b.siddha@intel.com>
> > > Cc: Weidong Han <weidong.han@intel.com>
> > > ---
> > > 
> > > Index: tip/arch/x86/include/asm/io_apic.h
> > > ===================================================================
> > > --- tip.orig/arch/x86/include/asm/io_apic.h
> > > +++ tip/arch/x86/include/asm/io_apic.h
> > > @@ -166,8 +166,6 @@ extern void free_ioapic_entries(struct I
> > >  extern int save_IO_APIC_setup(struct IO_APIC_route_entry
> > >  **ioapic_entries); extern void mask_IO_APIC_setup(struct
> > >  IO_APIC_route_entry **ioapic_entries); extern int
> > > restore_IO_APIC_setup(struct IO_APIC_route_entry **ioapic_entries); 
> > > -extern void reinit_intr_remapped_IO_APIC(int intr_remapping,
> > > -	struct IO_APIC_route_entry **ioapic_entries);
> > > 
> > >  extern void probe_nr_irqs_gsi(void);
> > > 
> > > Index: tip/arch/x86/kernel/apic/apic.c
> > > ===================================================================
> > > --- tip.orig/arch/x86/kernel/apic/apic.c
> > > +++ tip/arch/x86/kernel/apic/apic.c
> > > @@ -1416,8 +1416,6 @@ end_restore:
> > >  		 * IR enabling failed
> > >  		 */
> > >  		restore_IO_APIC_setup(ioapic_entries);
> > > -	else
> > > -		reinit_intr_remapped_IO_APIC(x2apic_preenabled, ioapic_entries);
> > 
> > Whether IR enabling succeeds or fails, it always needs to restore 
> > old IOAPIC entries. Due to removing reinit_intr_remapped_IO_APIC 
> > here, it needs to also remove the "if (ret)" before 
> > restore_IO_APIC_setup(ioapic_entries);
> 
> Ok - i skipped this patch for now.

Let me clarify what I am doing in this patch:

When interrupt-remapping is enabled, IO-APIC entries need to be setup in
the re-mappable format (pointing to interrupt-remapping table entries
setup by the OS). This remapping configuration is happening in the same
place where we traditionally configure IO-APIC (i.e., in
setup_IO_APIC_irqs()).

So when we enable interrupt-remapping successfully, there is no need to
restore old io-apic RTE entries before we actually do a complete
configuration shortly in setup_IO_APIC_irqs(). Old IO-APIC RTE's may be
in traditional format (non re-mappable) or in re-mappable format
pointing to interrupt-remapping table entries setup by BIOS. Restoring
both of these will not make IO-APIC functional. We have to rely on
setup_IO_APIC_irqs() for proper configuration by OS.

So I am removing this unnecessary and broken step.

When enabling interrupt-remapping is not successful, we are doing plain
restore of old RTE's (which will still work as we went back to original
no-remapping state). Complete IO-APIC configuration will be done shortly
in setup_IO_APIC_irqs().

thanks,
suresh

RE: [patch 4/5] x2apic, IR: remove reinit_intr_remapped_IO_APIC()

[Han, Weidong]

Siddha, Suresh B wrote:
> On Tue, 2009-04-21 at 00:01 -0700, Ingo Molnar wrote:
>> * Han, Weidong <weidong.han@intel.com> wrote:
>> 
>>> Siddha, Suresh B wrote:
>>>> When interrupt-remapping is enabled, We are relying on
>>>> setup_IO_APIC_irqs() to configure remapped entries in the IO-APIC,
>>>> which comes little bit later after enabling interrupt-remapping.
>>>> 
>>>> Meanwhile, Restore of old io-apic entries after enabling
>>>> interrupt-remapping will not make the interrupts through io-apic
>>>> functional anyway. 
>>>> 
>>>> So remove unnecessary reinit_intr_remapped_IO_APIC().
>>>> 
>>>> Signed-off-by: Suresh Siddha <suresh.b.siddha@intel.com>
>>>> Cc: Weidong Han <weidong.han@intel.com>
>>>> ---
>>>> 
>>>> Index: tip/arch/x86/include/asm/io_apic.h
>>>> ===================================================================
>>>> --- tip.orig/arch/x86/include/asm/io_apic.h
>>>> +++ tip/arch/x86/include/asm/io_apic.h
>>>> @@ -166,8 +166,6 @@ extern void free_ioapic_entries(struct I
>>>>  extern int save_IO_APIC_setup(struct IO_APIC_route_entry
>>>>  **ioapic_entries); extern void mask_IO_APIC_setup(struct
>>>>  IO_APIC_route_entry **ioapic_entries); extern int
>>>> restore_IO_APIC_setup(struct IO_APIC_route_entry **ioapic_entries);
>>>> -extern void reinit_intr_remapped_IO_APIC(int intr_remapping,
>>>> -	struct IO_APIC_route_entry **ioapic_entries);
>>>> 
>>>>  extern void probe_nr_irqs_gsi(void);
>>>> 
>>>> Index: tip/arch/x86/kernel/apic/apic.c
>>>> ===================================================================
>>>> --- tip.orig/arch/x86/kernel/apic/apic.c
>>>> +++ tip/arch/x86/kernel/apic/apic.c
>>>> @@ -1416,8 +1416,6 @@ end_restore:
>>>>  		 * IR enabling failed
>>>>  		 */
>>>>  		restore_IO_APIC_setup(ioapic_entries);
>>>> -	else
>>>> -		reinit_intr_remapped_IO_APIC(x2apic_preenabled, ioapic_entries);
>>> 
>>> Whether IR enabling succeeds or fails, it always needs to restore
>>> old IOAPIC entries. Due to removing reinit_intr_remapped_IO_APIC
>>> here, it needs to also remove the "if (ret)" before
>>> restore_IO_APIC_setup(ioapic_entries);
>> 
>> Ok - i skipped this patch for now.
> 
> Let me clarify what I am doing in this patch:
> 
> When interrupt-remapping is enabled, IO-APIC entries need to be setup
> in the re-mappable format (pointing to interrupt-remapping table
> entries setup by the OS). This remapping configuration is happening
> in the same place where we traditionally configure IO-APIC (i.e., in
> setup_IO_APIC_irqs()).
> 
> So when we enable interrupt-remapping successfully, there is no need
> to restore old io-apic RTE entries before we actually do a complete
> configuration shortly in setup_IO_APIC_irqs(). Old IO-APIC RTE's may
> be in traditional format (non re-mappable) or in re-mappable format
> pointing to interrupt-remapping table entries setup by BIOS. Restoring
> both of these will not make IO-APIC functional. We have to rely on
> setup_IO_APIC_irqs() for proper configuration by OS.
> 
> So I am removing this unnecessary and broken step.
> 
> When enabling interrupt-remapping is not successful, we are doing
> plain restore of old RTE's (which will still work as we went back to
> original no-remapping state). Complete IO-APIC configuration will be
> done shortly in setup_IO_APIC_irqs().
> 

Clear explanation. I misunderstood it. This patch is fine.

Regards,
Weidong

> thanks,
> suresh

Re: [patch 4/5] x2apic, IR: remove reinit_intr_remapped_IO_APIC()

[Ingo Molnar]

* Han, Weidong <weidong.han@intel.com> wrote:

> Siddha, Suresh B wrote:
> > On Tue, 2009-04-21 at 00:01 -0700, Ingo Molnar wrote:
> >> * Han, Weidong <weidong.han@intel.com> wrote:
> >> 
> >>> Siddha, Suresh B wrote:
> >>>> When interrupt-remapping is enabled, We are relying on
> >>>> setup_IO_APIC_irqs() to configure remapped entries in the IO-APIC,
> >>>> which comes little bit later after enabling interrupt-remapping.
> >>>> 
> >>>> Meanwhile, Restore of old io-apic entries after enabling
> >>>> interrupt-remapping will not make the interrupts through io-apic
> >>>> functional anyway. 
> >>>> 
> >>>> So remove unnecessary reinit_intr_remapped_IO_APIC().
> >>>> 
> >>>> Signed-off-by: Suresh Siddha <suresh.b.siddha@intel.com>
> >>>> Cc: Weidong Han <weidong.han@intel.com>
> >>>> ---
> >>>> 
> >>>> Index: tip/arch/x86/include/asm/io_apic.h
> >>>> ===================================================================
> >>>> --- tip.orig/arch/x86/include/asm/io_apic.h
> >>>> +++ tip/arch/x86/include/asm/io_apic.h
> >>>> @@ -166,8 +166,6 @@ extern void free_ioapic_entries(struct I
> >>>>  extern int save_IO_APIC_setup(struct IO_APIC_route_entry
> >>>>  **ioapic_entries); extern void mask_IO_APIC_setup(struct
> >>>>  IO_APIC_route_entry **ioapic_entries); extern int
> >>>> restore_IO_APIC_setup(struct IO_APIC_route_entry **ioapic_entries);
> >>>> -extern void reinit_intr_remapped_IO_APIC(int intr_remapping,
> >>>> -	struct IO_APIC_route_entry **ioapic_entries);
> >>>> 
> >>>>  extern void probe_nr_irqs_gsi(void);
> >>>> 
> >>>> Index: tip/arch/x86/kernel/apic/apic.c
> >>>> ===================================================================
> >>>> --- tip.orig/arch/x86/kernel/apic/apic.c
> >>>> +++ tip/arch/x86/kernel/apic/apic.c
> >>>> @@ -1416,8 +1416,6 @@ end_restore:
> >>>>  		 * IR enabling failed
> >>>>  		 */
> >>>>  		restore_IO_APIC_setup(ioapic_entries);
> >>>> -	else
> >>>> -		reinit_intr_remapped_IO_APIC(x2apic_preenabled, ioapic_entries);
> >>> 
> >>> Whether IR enabling succeeds or fails, it always needs to restore
> >>> old IOAPIC entries. Due to removing reinit_intr_remapped_IO_APIC
> >>> here, it needs to also remove the "if (ret)" before
> >>> restore_IO_APIC_setup(ioapic_entries);
> >> 
> >> Ok - i skipped this patch for now.
> > 
> > Let me clarify what I am doing in this patch:
> > 
> > When interrupt-remapping is enabled, IO-APIC entries need to be setup
> > in the re-mappable format (pointing to interrupt-remapping table
> > entries setup by the OS). This remapping configuration is happening
> > in the same place where we traditionally configure IO-APIC (i.e., in
> > setup_IO_APIC_irqs()).
> > 
> > So when we enable interrupt-remapping successfully, there is no need
> > to restore old io-apic RTE entries before we actually do a complete
> > configuration shortly in setup_IO_APIC_irqs(). Old IO-APIC RTE's may
> > be in traditional format (non re-mappable) or in re-mappable format
> > pointing to interrupt-remapping table entries setup by BIOS. Restoring
> > both of these will not make IO-APIC functional. We have to rely on
> > setup_IO_APIC_irqs() for proper configuration by OS.
> > 
> > So I am removing this unnecessary and broken step.
> > 
> > When enabling interrupt-remapping is not successful, we are doing
> > plain restore of old RTE's (which will still work as we went back to
> > original no-remapping state). Complete IO-APIC configuration will be
> > done shortly in setup_IO_APIC_irqs().
> > 
> 
> Clear explanation. I misunderstood it. This patch is fine.

Applied to tip:x86/apic - thanks guys,

	Ingo

[tip:x86/apic] x86: x2apic, IR: remove reinit_intr_remapped_IO_APIC()

[tip-bot for Suresh Siddha]

Commit-ID:  ff166cb57a17124af75714a9c11f448f56f1a4a3
Gitweb:     http://git.kernel.org/tip/ff166cb57a17124af75714a9c11f448f56f1a4a3
Author:     Suresh Siddha <suresh.b.siddha@intel.com>
AuthorDate: Mon, 20 Apr 2009 13:02:30 -0700
Committer:  Ingo Molnar <mingo@elte.hu>
CommitDate: Wed, 22 Apr 2009 12:03:04 +0200

x86: x2apic, IR: remove reinit_intr_remapped_IO_APIC()

When interrupt-remapping is enabled, we are relying on
setup_IO_APIC_irqs() to configure remapped entries in the
IO-APIC, which comes little bit later after enabling
interrupt-remapping.

Meanwhile, restoration of old io-apic entries after enabling
interrupt-remapping will not make the interrupts through
io-apic functional anyway.

So remove the unnecessary reinit_intr_remapped_IO_APIC() step.

The longer story:

When interrupt-remapping is enabled, IO-APIC entries need to be
setup in the re-mappable format (pointing to
interrupt-remapping table entries setup by the OS). This
remapping configuration is happening in the same place where we
traditionally configure IO-APIC (i.e., in
setup_IO_APIC_irqs()).

So when we enable interrupt-remapping successfully, there is no
need to restore old io-apic RTE entries before we actually do a
complete configuration shortly in setup_IO_APIC_irqs(). Old
IO-APIC RTE's may be in traditional format (non re-mappable) or
in re-mappable format pointing to interrupt-remapping table
entries setup by BIOS. Restoring both of these will not make
IO-APIC functional. We have to rely on setup_IO_APIC_irqs() for
proper configuration by OS.

So I am removing this unnecessary and broken step.

[ Impact: remove unnecessary/broken IO-APIC setup step ]

Signed-off-by: Suresh Siddha <suresh.b.siddha@intel.com>
Acked-by: Weidong Han <weidong.han@intel.com>
Cc: dwmw2@infradead.org
LKML-Reference: <20090420200450.552359000@linux-os.sc.intel.com>
Signed-off-by: Ingo Molnar <mingo@elte.hu>


---
 arch/x86/include/asm/io_apic.h |    2 --
 arch/x86/kernel/apic/apic.c    |    2 --
 arch/x86/kernel/apic/io_apic.c |   14 --------------
 3 files changed, 0 insertions(+), 18 deletions(-)

diff --git a/arch/x86/include/asm/io_apic.h b/arch/x86/include/asm/io_apic.h
index 34eaa37..1cf1450 100644
--- a/arch/x86/include/asm/io_apic.h
+++ b/arch/x86/include/asm/io_apic.h
@@ -166,8 +166,6 @@ extern void free_ioapic_entries(struct IO_APIC_route_entry **ioapic_entries);
 extern int save_IO_APIC_setup(struct IO_APIC_route_entry **ioapic_entries);
 extern void mask_IO_APIC_setup(struct IO_APIC_route_entry **ioapic_entries);
 extern int restore_IO_APIC_setup(struct IO_APIC_route_entry **ioapic_entries);
-extern void reinit_intr_remapped_IO_APIC(int intr_remapping,
-	struct IO_APIC_route_entry **ioapic_entries);
 
 extern void probe_nr_irqs_gsi(void);
 
diff --git a/arch/x86/kernel/apic/apic.c b/arch/x86/kernel/apic/apic.c
index d32f558..1386dbe 100644
--- a/arch/x86/kernel/apic/apic.c
+++ b/arch/x86/kernel/apic/apic.c
@@ -1412,8 +1412,6 @@ end_restore:
 		 * IR enabling failed
 		 */
 		restore_IO_APIC_setup(ioapic_entries);
-	else
-		reinit_intr_remapped_IO_APIC(x2apic_preenabled, ioapic_entries);
 
 	unmask_8259A();
 	local_irq_restore(flags);
diff --git a/arch/x86/kernel/apic/io_apic.c b/arch/x86/kernel/apic/io_apic.c
index 4baa9cb..8aef5f9 100644
--- a/arch/x86/kernel/apic/io_apic.c
+++ b/arch/x86/kernel/apic/io_apic.c
@@ -833,20 +833,6 @@ int restore_IO_APIC_setup(struct IO_APIC_route_entry **ioapic_entries)
 	return 0;
 }
 
-void reinit_intr_remapped_IO_APIC(int intr_remapping,
-	struct IO_APIC_route_entry **ioapic_entries)
-
-{
-	/*
-	 * for now plain restore of previous settings.
-	 * TBD: In the case of OS enabling interrupt-remapping,
-	 * IO-APIC RTE's need to be setup to point to interrupt-remapping
-	 * table entries. for now, do a plain restore, and wait for
-	 * the setup_IO_APIC_irqs() to do proper initialization.
-	 */
-	restore_IO_APIC_setup(ioapic_entries);
-}
-
 void free_ioapic_entries(struct IO_APIC_route_entry **ioapic_entries)
 {
 	int apic;