[PATCH 1/6] x86: move ioapic_irq_destination_types

[PATCH 1/6] x86: move ioapic_irq_destination_types

[Henrik Kretzschmar]

This enum is used also by non-ioapic code, e.g apic_noop,
so its better kept in apicdef.h.

Signed-off-by: Henrik Kretzschmar <henne@nachtwindheim.de>
---
 arch/x86/include/asm/apicdef.h |   12 ++++++++++++
 arch/x86/include/asm/io_apic.h |   11 -----------
 2 files changed, 12 insertions(+), 11 deletions(-)

diff --git a/arch/x86/include/asm/apicdef.h b/arch/x86/include/asm/apicdef.h
index 47a30ff..2de3e95 100644
--- a/arch/x86/include/asm/apicdef.h
+++ b/arch/x86/include/asm/apicdef.h
@@ -426,4 +426,16 @@ struct local_apic {
 #else
  #define BAD_APICID 0xFFFFu
 #endif
+
+enum ioapic_irq_destination_types {
+	dest_Fixed = 0,
+	dest_LowestPrio = 1,
+	dest_SMI = 2,
+	dest__reserved_1 = 3,
+	dest_NMI = 4,
+	dest_INIT = 5,
+	dest__reserved_2 = 6,
+	dest_ExtINT = 7
+};
+
 #endif /* _ASM_X86_APICDEF_H */
diff --git a/arch/x86/include/asm/io_apic.h b/arch/x86/include/asm/io_apic.h
index f327d38..e1a9b0e 100644
--- a/arch/x86/include/asm/io_apic.h
+++ b/arch/x86/include/asm/io_apic.h
@@ -63,17 +63,6 @@ union IO_APIC_reg_03 {
 	} __attribute__ ((packed)) bits;
 };
 
-enum ioapic_irq_destination_types {
-	dest_Fixed = 0,
-	dest_LowestPrio = 1,
-	dest_SMI = 2,
-	dest__reserved_1 = 3,
-	dest_NMI = 4,
-	dest_INIT = 5,
-	dest__reserved_2 = 6,
-	dest_ExtINT = 7
-};
-
 struct IO_APIC_route_entry {
 	__u32	vector		:  8,
 		delivery_mode	:  3,	/* 000: FIXED
-- 
1.7.2.3

[PATCH 2/6] x86: ifdef enable_IR_x2apic() out

[Henrik Kretzschmar]

The only caller of enable_IR_x2apic() is probe_64.c, which is only
compiled on x86-64 bit machines.
This function causes compilation problems on 32-bit machines with no
io-apic enabled, so we remove it from 32s and keep the way it was on 64s.

Signed-off-by: Henrik Kretzschmar <henne@nachtwindheim.de>
---
 arch/x86/include/asm/apic.h |    2 ++
 arch/x86/kernel/apic/apic.c |    2 +-
 2 files changed, 3 insertions(+), 1 deletions(-)

diff --git a/arch/x86/include/asm/apic.h b/arch/x86/include/asm/apic.h
index b8a3484..b1d77e1 100644
--- a/arch/x86/include/asm/apic.h
+++ b/arch/x86/include/asm/apic.h
@@ -216,7 +216,9 @@ static inline void x2apic_force_phys(void)
 #define	x2apic_supported()	0
 #endif
 
+#ifdef CONFIG_X86_64
 extern void enable_IR_x2apic(void);
+#endif
 
 extern int get_physical_broadcast(void);
 
diff --git a/arch/x86/kernel/apic/apic.c b/arch/x86/kernel/apic/apic.c
index 306386f..27a7497 100644
--- a/arch/x86/kernel/apic/apic.c
+++ b/arch/x86/kernel/apic/apic.c
@@ -1467,6 +1467,7 @@ int __init enable_IR(void)
 	return 0;
 }
 
+#ifdef CONFIG_X86_64
 void __init enable_IR_x2apic(void)
 {
 	unsigned long flags;
@@ -1540,7 +1541,6 @@ out:
 		pr_info("Not enabling x2apic, Intr-remapping init failed.\n");
 }
 
-#ifdef CONFIG_X86_64
 /*
  * Detect and enable local APICs on non-SMP boards.
  * Original code written by Keir Fraser.
-- 
1.7.2.3

Re: [PATCH 2/6] x86: ifdef enable_IR_x2apic() out

[Ingo Molnar]

* Henrik Kretzschmar <henne@nachtwindheim.de> wrote:

> +#ifdef CONFIG_X86_64
>  extern void enable_IR_x2apic(void);
> +#endif

Cannot we use the CONFIG_X86_X2APIC Kconfig switch here, instead of CONFIG_X86_64?

enable_IR_x2apic() is not a 64-bit CPU feature.

Thanks,

	Ingo

Re: [PATCH 2/6] x86: ifdef enable_IR_x2apic() out

[Henrik Kretzschmar]

Am 14.02.2011 12:03, schrieb Ingo Molnar:
> 
> 
> * Henrik Kretzschmar <henne@nachtwindheim.de> wrote:
> 
>> +#ifdef CONFIG_X86_64
>>  extern void enable_IR_x2apic(void);
>> +#endif
> 
> Cannot we use the CONFIG_X86_X2APIC Kconfig switch here, instead of CONFIG_X86_64?
> 
> enable_IR_x2apic() is not a 64-bit CPU feature.
> 
> Thanks,
> 
> 	Ingo
> 
> 

Thats what I had liked also.

At the time you moved the apic code from x86/kernel it was exactly that way.
But after that, in commit 937582382c71b75b29fbb92615629494e1a05ac0, it was explicitely moved out of
CONFIG_X86_X2APIC, which made that function also compile on 32bit machines, even if its not used there.

Also commit ce69a784504222c3ab6f1b3c357d09ec5772127a enabled the x2apic without interrupt remapping,
which means that CONFIG_X86_X2APIC depends on CONFIG_INTR_REMAP has been bypassed and so Kconfig is
broken/not representing what happens to the code.

So since I dont have an x2apic and therefore cannot test changes,
I decided for CONFIG_X86_64 without touching code which seems to work.

I wish I could offer something better here.

Greets,
Henrik Kretzschmar

Re: [PATCH 2/6] x86: ifdef enable_IR_x2apic() out

[Gleb Natapov]

On Tue, Feb 15, 2011 at 02:47:50PM +0100, Henrik Kretzschmar wrote:
> Am 14.02.2011 12:03, schrieb Ingo Molnar:
> > 
> > 
> > * Henrik Kretzschmar <henne@nachtwindheim.de> wrote:
> > 
> >> +#ifdef CONFIG_X86_64
> >>  extern void enable_IR_x2apic(void);
> >> +#endif
> > 
> > Cannot we use the CONFIG_X86_X2APIC Kconfig switch here, instead of CONFIG_X86_64?
> > 
> > enable_IR_x2apic() is not a 64-bit CPU feature.
> > 
> > Thanks,
> > 
> > 	Ingo
> > 
> > 
> 
> Thats what I had liked also.
> 
> At the time you moved the apic code from x86/kernel it was exactly that way.
> But after that, in commit 937582382c71b75b29fbb92615629494e1a05ac0, it was explicitely moved out of
> CONFIG_X86_X2APIC, which made that function also compile on 32bit machines, even if its not used there.
> 
> Also commit ce69a784504222c3ab6f1b3c357d09ec5772127a enabled the x2apic without interrupt remapping,
> which means that CONFIG_X86_X2APIC depends on CONFIG_INTR_REMAP has been bypassed and so Kconfig is
> broken/not representing what happens to the code.
> 
Intel asked to leave Kconfig dependency in place.

> So since I dont have an x2apic and therefore cannot test changes,
> I decided for CONFIG_X86_64 without touching code which seems to work.
> 
> I wish I could offer something better here.
> 
> Greets,
> Henrik Kretzschmar
> 
> 
> 

--
			Gleb.

[PATCH 3/6] x86: ifdef INTR_REMAP code out

[Henrik Kretzschmar]

Interrupt remapping is only available on 64-bit machines,
so it has no place in lapic_resume() for 32bit machines.

Compilation on 32bit machines would produce errors here.

Signed-off-by: Henrik Kretzschmar <henne@nachtwindheim.de>
---
 arch/x86/kernel/apic/apic.c |    8 +++++++-
 1 files changed, 7 insertions(+), 1 deletions(-)

diff --git a/arch/x86/kernel/apic/apic.c b/arch/x86/kernel/apic/apic.c
index 27a7497..999c531 100644
--- a/arch/x86/kernel/apic/apic.c
+++ b/arch/x86/kernel/apic/apic.c
@@ -2109,12 +2109,15 @@ static int lapic_resume(struct sys_device *dev)
 	unsigned long flags;
 	int maxlvt;
 	int ret = 0;
-	struct IO_APIC_route_entry **ioapic_entries = NULL;
 
 	if (!apic_pm_state.active)
 		return 0;
 
 	local_irq_save(flags);
+
+#ifdef CONFIG_INTR_REMAP
+	struct IO_APIC_route_entry **ioapic_entries = NULL;
+
 	if (intr_remapping_enabled) {
 		ioapic_entries = alloc_ioapic_entries();
 		if (!ioapic_entries) {
@@ -2133,6 +2136,7 @@ static int lapic_resume(struct sys_device *dev)
 		mask_IO_APIC_setup(ioapic_entries);
 		legacy_pic->mask_all();
 	}
+#endif
 
 	if (x2apic_mode)
 		enable_x2apic();
@@ -2173,6 +2177,7 @@ 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) {
 		reenable_intr_remapping(x2apic_mode);
 		legacy_pic->restore_mask();
@@ -2180,6 +2185,7 @@ static int lapic_resume(struct sys_device *dev)
 		free_ioapic_entries(ioapic_entries);
 	}
 restore:
+#endif
 	local_irq_restore(flags);
 
 	return ret;
-- 
1.7.2.3

Re: [PATCH 3/6] x86: ifdef INTR_REMAP code out

[Ingo Molnar]

* Henrik Kretzschmar <henne@nachtwindheim.de> wrote:

> +#ifdef CONFIG_INTR_REMAP
> +	struct IO_APIC_route_entry **ioapic_entries = NULL;
> +
>  	if (intr_remapping_enabled) {
>  		ioapic_entries = alloc_ioapic_entries();
>  		if (!ioapic_entries) {
> @@ -2133,6 +2136,7 @@ static int lapic_resume(struct sys_device *dev)
>  		mask_IO_APIC_setup(ioapic_entries);
>  		legacy_pic->mask_all();
>  	}
> +#endif
>  
>  	if (x2apic_mode)
>  		enable_x2apic();
> @@ -2173,6 +2177,7 @@ 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) {
>  		reenable_intr_remapping(x2apic_mode);
>  		legacy_pic->restore_mask();
> @@ -2180,6 +2185,7 @@ static int lapic_resume(struct sys_device *dev)
>  		free_ioapic_entries(ioapic_entries);
>  	}
>  restore:
> +#endif

Hm, these bits should be factored out in a cleaner fashion - by adding helper 
functions, etc. The x2apic code's integration into the lapic code was done in a 
pretty ugly fashion so it's not your fault - but if we want to reintroduce UP-IOAPIC 
we need to do it cleanly.

Do you still want to do it? :-)

Thanks,

	Ingo

[PATCH 4/6] x86: add dummy mp_save_irq()

[Henrik Kretzschmar]

mmparse.c needs a mp_save_irq() function, which is only available
when CONFIG_X86_IO_APIC is defined. So here we give it a dummy one.

Signed-off-by: Henrik Kretzschmar <henne@nachtwindheim.de>
---
 arch/x86/include/asm/io_apic.h |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/arch/x86/include/asm/io_apic.h b/arch/x86/include/asm/io_apic.h
index e1a9b0e..7af0f5f 100644
--- a/arch/x86/include/asm/io_apic.h
+++ b/arch/x86/include/asm/io_apic.h
@@ -188,6 +188,7 @@ static inline int mp_find_ioapic(u32 gsi) { return 0; }
 struct io_apic_irq_attr;
 static inline int io_apic_set_pci_routing(struct device *dev, int irq,
 		 struct io_apic_irq_attr *irq_attr) { return 0; }
+static inline void mp_save_irq(struct mpc_intsrc *m) { }
 #endif
 
 #endif /* _ASM_X86_IO_APIC_H */
-- 
1.7.2.3

[PATCH 5/6] x86: ifdef ioapic related function out

[Henrik Kretzschmar]

arch_disable_smp_config() is an IO-APIC related function on x86,
and should only be needed if SMP is enabled.
But the IO-APIC code calls it when the parameter "noapic" is given to
the kernel, which doesn't mean SMP is enabled.

Anyway this fixes compilation on x86_32 UP systems with APIC and no IO-APIC.

Signed-off-by: Henrik Kretzschmar <henne@nachtwindheim.de>
---
 arch/x86/kernel/apic/apic.c |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/arch/x86/kernel/apic/apic.c b/arch/x86/kernel/apic/apic.c
index 999c531..4998f0a 100644
--- a/arch/x86/kernel/apic/apic.c
+++ b/arch/x86/kernel/apic/apic.c
@@ -1218,7 +1218,9 @@ void __cpuinit setup_local_APIC(void)
 		rdtscll(tsc);
 
 	if (disable_apic) {
+#ifdef CONFIG_X86_IO_APIC
 		arch_disable_smp_support();
+#endif
 		return;
 	}
 
-- 
1.7.2.3

Re: [PATCH 5/6] x86: ifdef ioapic related function out

[Ingo Molnar]

* Henrik Kretzschmar <henne@nachtwindheim.de> wrote:

> arch_disable_smp_config() is an IO-APIC related function on x86,
> and should only be needed if SMP is enabled.
> But the IO-APIC code calls it when the parameter "noapic" is given to
> the kernel, which doesn't mean SMP is enabled.
> 
> Anyway this fixes compilation on x86_32 UP systems with APIC and no IO-APIC.
> 
> Signed-off-by: Henrik Kretzschmar <henne@nachtwindheim.de>
> ---
>  arch/x86/kernel/apic/apic.c |    2 ++
>  1 files changed, 2 insertions(+), 0 deletions(-)
> 
> diff --git a/arch/x86/kernel/apic/apic.c b/arch/x86/kernel/apic/apic.c
> index 999c531..4998f0a 100644
> --- a/arch/x86/kernel/apic/apic.c
> +++ b/arch/x86/kernel/apic/apic.c
> @@ -1218,7 +1218,9 @@ void __cpuinit setup_local_APIC(void)
>  		rdtscll(tsc);
>  
>  	if (disable_apic) {
> +#ifdef CONFIG_X86_IO_APIC
>  		arch_disable_smp_support();
> +#endif

Why not make the arch_disable_smp_support() call generic in the 
arch/x86/include/asm/smp.h file (via an inline helper) and thus
avoid an ugly #ifdef in the .c file?

Thanks,

	Ingo

[PATCH 6/6] x86: makes X86_UP_IOAPIC work again

[Henrik Kretzschmar]

This fixes a typo, which made CONFIG_X86_UP_IOAPIC defunctional,
in commit 7cd92366a593246650cc7d6198e2c7d3af8c1d8a.

This has been successfully tested.

Signed-off-by: Henrik Kretzschmar <henne@nachtwindheim.de>
---
 arch/x86/Kconfig |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
index 95c36c4..66c6801 100644
--- a/arch/x86/Kconfig
+++ b/arch/x86/Kconfig
@@ -811,7 +811,7 @@ config X86_LOCAL_APIC
 
 config X86_IO_APIC
 	def_bool y
-	depends on X86_64 || SMP || X86_32_NON_STANDARD || X86_UP_APIC
+	depends on X86_64 || SMP || X86_32_NON_STANDARD || X86_UP_IOAPIC
 
 config X86_VISWS_APIC
 	def_bool y
-- 
1.7.2.3

Re: [PATCH 1/6] x86: move ioapic_irq_destination_types

[Ingo Molnar]

* Henrik Kretzschmar <henne@nachtwindheim.de> wrote:

> +++ b/arch/x86/include/asm/apicdef.h
> @@ -426,4 +426,16 @@ struct local_apic {
>  #else
>   #define BAD_APICID 0xFFFFu
>  #endif
> +
> +enum ioapic_irq_destination_types {
> +	dest_Fixed = 0,
> +	dest_LowestPrio = 1,
> +	dest_SMI = 2,
> +	dest__reserved_1 = 3,
> +	dest_NMI = 4,
> +	dest_INIT = 5,
> +	dest__reserved_2 = 6,
> +	dest_ExtINT = 7
> +};

one very small request, while we are moving it could you please align the value
enumeration vertically? Something like:

enum ioapic_irq_destination_types {

	dest_Fixed		= 0,
	dest_LowestPrio		= 1,
	dest_SMI		= 2,
	dest__reserved_1	= 3,
	dest_NMI		= 4,
	dest_INIT		= 5,
	dest__reserved_2	= 6,
	dest_ExtINT		= 7
};

... would be much more readable, right?

Thanks,

	Ingo