APIC is not properly suspending in 2.5.67 on UP

APIC is not properly suspending in 2.5.67 on UP

[Pavel Machek]

Hi!

This is needed otherwise APIC thinks it is not active, does not
suspend properly, and kills machine. Extra whitespace killed (looks
ugly). Please apply,
								Pavel

%patch
Index: linux/arch/i386/kernel/apic.c
===================================================================
--- linux.orig/arch/i386/kernel/apic.c	2003-04-08 13:19:44.000000000 +0200
+++ linux/arch/i386/kernel/apic.c	2003-04-12 23:38:21.000000000 +0200
@@ -449,6 +449,7 @@
 
 	if (nmi_watchdog == NMI_LOCAL_APIC)
 		setup_apic_nmi_watchdog();
+	apic_pm_activate();
 }
 
 #ifdef CONFIG_PM
@@ -652,9 +653,7 @@
 		nmi_watchdog = NMI_LOCAL_APIC;
 
 	printk("Found and enabled local APIC!\n");
-
 	apic_pm_activate();
-
 	return 0;
 
 no_apic:
@@ -1133,11 +1132,8 @@
 	}
 
 	verify_local_APIC();
-
 	connect_bsp_APIC();
-
 	phys_cpu_present_map = 1 << boot_cpu_physical_apicid;
-
 	setup_local_APIC();
 
 	if (nmi_watchdog == NMI_LOCAL_APIC)
@@ -1148,6 +1144,5 @@
 			setup_IO_APIC();
 #endif
 	setup_boot_APIC_clock();
-
 	return 0;
 }

%diffstat
 apic.c |    7 +------
 1 files changed, 1 insertion(+), 6 deletions(-)


-- 
When do you have a heart between your knees?
[Johanka's followup: and *two* hearts?]

Re: APIC is not properly suspending in 2.5.67 on UP

[mikpe]

On Sat, 12 Apr 2003 23:47:01 +0200, Pavel Machek wrote:
>This is needed otherwise APIC thinks it is not active, does not
>suspend properly, and kills machine.

This can only happen with UP if the machine boots with local
APIC enabled and the BIOS announces an MP table.

If this is the case, then yes apic_pm_activate() needs to be done.

> Extra whitespace killed (looks
>ugly). Please apply,

I think some fixes are needed first:
- You're calling apic_pm_activate() from setup_local_APIC(), which
  is before its definition. This will cause a compile warning, and
  a linkage error if CONFIG_PM=n.
- While calling apic_pm_activate() from setup_local_APIC() sort of
  works in the UP case, it's wrong since setup_local_APIC() is called
  for each CPU in SMP, and we must not run the suspend and resume
  code if there is more than one CPU in the machine.
  I don't have a good solution for this right now: I don't think
  cpu_online_map is valid when init_lapi_devicefs() runs, and I
  don't know how else to check the number of CPUs.
  Changing the #ifdef CONFIG_PM block to be #if defined(CONFIG_PM)
  && !defined(CONFIG_SMP) would fix UP kernels, but SMP kernels on
  UP HW would lose PM. Adding "if (num_online_cpus() > 1) return;"
  to the suspend & resume procedures is ugly but should work.

/Mikael

Re: APIC is not properly suspending in 2.5.67 on UP

[Pavel Machek]

Hi!

> >This is needed otherwise APIC thinks it is not active, does not
> >suspend properly, and kills machine.
> 
> This can only happen with UP if the machine boots with local
> APIC enabled and the BIOS announces an MP table.
> 
> If this is the case, then yes apic_pm_activate() needs to be done.
> 
> > Extra whitespace killed (looks
> >ugly). Please apply,
> 
> I think some fixes are needed first:
> - You're calling apic_pm_activate() from setup_local_APIC(), which
>   is before its definition. This will cause a compile warning, and
>   a linkage error if CONFIG_PM=n.

Okay, this needs to be fixed.

> - While calling apic_pm_activate() from setup_local_APIC() sort of
>   works in the UP case, it's wrong since setup_local_APIC() is called
>   for each CPU in SMP, and we must not run the suspend and resume
>   code if there is more than one CPU in the machine.
>   I don't have a good solution for this right now: I don't think
>   cpu_online_map is valid when init_lapi_devicefs() runs, and I
>   don't know how else to check the number of CPUs.
>   Changing the #ifdef CONFIG_PM block to be #if defined(CONFIG_PM)
>   && !defined(CONFIG_SMP) would fix UP kernels, but SMP kernels on
>   UP HW would lose PM. Adding "if (num_online_cpus() > 1) return;"
>   to the suspend & resume procedures is ugly but should work.

That's future work. As soon as I get SMP machine I'll do something
about SMP. (Most probably hot unplugging all but one CPUs before
suspend and hot plugging them all back after resume.) I'd ignore SMP
problems in suspendresume for now: apm just refuses to do it on SMP,
and acpi is broken in more than one way with regard to that.

							Pavel
-- 
Horseback riding is like software...
...vgf orggre jura vgf serr.

Re: APIC is not properly suspending in 2.5.67 on UP

[Pavel Machek]

Hi!

> >This is needed otherwise APIC thinks it is not active, does not
> >suspend properly, and kills machine.
> 
> This can only happen with UP if the machine boots with local
> APIC enabled and the BIOS announces an MP table.
> 
> If this is the case, then yes apic_pm_activate() needs to be done.
> 
> > Extra whitespace killed (looks
> >ugly). Please apply,
> 
> I think some fixes are needed first:
> - You're calling apic_pm_activate() from setup_local_APIC(), which
>   is before its definition. This will cause a compile warning, and
>   a linkage error if CONFIG_PM=n.

Fixed (by adding function prototype). Please apply,
								Pavel

%patch
Index: linux/arch/i386/kernel/apic.c
===================================================================
--- linux.orig/arch/i386/kernel/apic.c	2003-04-08 13:19:44.000000000 +0200
+++ linux/arch/i386/kernel/apic.c	2003-04-13 20:21:39.000000000 +0200
@@ -36,6 +36,8 @@
 
 #include <mach_apic.h>
 
+static void apic_pm_activate(void);
+
 void __init apic_intr_init(void)
 {
 #ifdef CONFIG_SMP
@@ -449,6 +451,7 @@
 
 	if (nmi_watchdog == NMI_LOCAL_APIC)
 		setup_apic_nmi_watchdog();
+	apic_pm_activate();
 }
 
 #ifdef CONFIG_PM
@@ -585,7 +588,7 @@
 
 #else	/* CONFIG_PM */
 
-static inline void apic_pm_activate(void) { }
+static void apic_pm_activate(void) { }
 
 #endif	/* CONFIG_PM */
 
@@ -652,9 +655,7 @@
 		nmi_watchdog = NMI_LOCAL_APIC;
 
 	printk("Found and enabled local APIC!\n");
-
 	apic_pm_activate();
-
 	return 0;
 
 no_apic:
@@ -1133,11 +1134,8 @@
 	}
 
 	verify_local_APIC();
-
 	connect_bsp_APIC();
-
 	phys_cpu_present_map = 1 << boot_cpu_physical_apicid;
-
 	setup_local_APIC();
 
 	if (nmi_watchdog == NMI_LOCAL_APIC)
@@ -1148,6 +1146,5 @@
 			setup_IO_APIC();
 #endif
 	setup_boot_APIC_clock();
-
 	return 0;
 }

%diffstat
 apic.c |   11 ++++-------
 1 files changed, 4 insertions(+), 7 deletions(-)



-- 
When do you have heart between your knees?