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

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 <[email protected]>
---
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


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

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 <[email protected]>
---
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

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

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 <[email protected]>
---
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

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

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 <[email protected]>
---
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

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

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

This has been successfully tested.

Signed-off-by: Henrik Kretzschmar <[email protected]>
---
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

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

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 <[email protected]>
---
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

2011-02-14 11:00:57

by Ingo Molnar

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


* Henrik Kretzschmar <[email protected]> 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 <[email protected]>
> ---
> 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

2011-02-14 11:02:43

by Ingo Molnar

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


* Henrik Kretzschmar <[email protected]> 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

2011-02-14 11:03:55

by Ingo Molnar

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



* Henrik Kretzschmar <[email protected]> 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

2011-02-14 11:05:46

by Ingo Molnar

[permalink] [raw]
Subject: Re: [PATCH 1/6] x86: move ioapic_irq_destination_types


* Henrik Kretzschmar <[email protected]> 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

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

Am 14.02.2011 12:03, schrieb Ingo Molnar:
>
>
> * Henrik Kretzschmar <[email protected]> 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



2011-02-15 14:16:18

by Gleb Natapov

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

On Tue, Feb 15, 2011 at 02:47:50PM +0100, Henrik Kretzschmar wrote:
> Am 14.02.2011 12:03, schrieb Ingo Molnar:
> >
> >
> > * Henrik Kretzschmar <[email protected]> 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.