* Switch APIC (+nmi, +oprofile) to driver model
@ 2003-02-09 11:32 Pavel Machek
2003-02-09 11:54 ` John Levon
2003-02-09 12:03 ` John Levon
0 siblings, 2 replies; 27+ messages in thread
From: Pavel Machek @ 2003-02-09 11:32 UTC (permalink / raw)
To: levon, kernel list
Hi!
Here's next attempt at moving APIC (+nmi, +oprofile) to the driver
model. If it looks good I'l submit it to Linus.
Pavel
--- clean/arch/i386/kernel/apic.c 2003-01-05 22:58:18.000000000 +0100
+++ linux-swsusp/arch/i386/kernel/apic.c 2003-02-03 16:36:41.000000000 +0100
@@ -10,6 +10,7 @@
* for testing these extensively.
* Maciej W. Rozycki : Various updates and fixes.
* Mikael Pettersson : Power Management for UP-APIC.
+ * Pavel Machek : Converted to driver model.
*/
#include <linux/config.h>
@@ -23,6 +24,10 @@
#include <linux/interrupt.h>
#include <linux/mc146818rtc.h>
#include <linux/kernel_stat.h>
+#include <linux/delay.h>
+#include <linux/slab.h>
+#include <linux/device.h>
+#include <linux/pm.h>
#include <asm/atomic.h>
#include <asm/smp.h>
@@ -54,6 +59,8 @@
int prof_old_multiplier[NR_CPUS] = { 1, };
int prof_counter[NR_CPUS] = { 1, };
+unsigned long apic_phys;
+
int get_maxlvt(void)
{
unsigned int v, ver, maxlvt;
@@ -292,6 +299,27 @@
apic_write_around(APIC_LVT1, value);
}
+static struct {
+ /* 'active' is true if the local APIC was enabled by us and
+ not the BIOS; this signifies that we are also responsible
+ for disabling it before entering apm/acpi suspend */
+ int active;
+ /* r/w apic fields */
+ unsigned int apic_id;
+ unsigned int apic_taskpri;
+ unsigned int apic_ldr;
+ unsigned int apic_dfr;
+ unsigned int apic_spiv;
+ unsigned int apic_lvtt;
+ unsigned int apic_lvtpc;
+ unsigned int apic_lvt0;
+ unsigned int apic_lvt1;
+ unsigned int apic_lvterr;
+ unsigned int apic_tmict;
+ unsigned int apic_tdcr;
+ unsigned int apic_thmr;
+} apic_pm_state;
+
void __init setup_local_APIC (void)
{
unsigned long value, ver, maxlvt;
@@ -435,44 +463,19 @@
if (nmi_watchdog == NMI_LOCAL_APIC)
setup_apic_nmi_watchdog();
+
+ apic_pm_state.active = 1;
}
#ifdef CONFIG_PM
-
-#include <linux/slab.h>
-#include <linux/pm.h>
-
-static struct {
- /* 'active' is true if the local APIC was enabled by us and
- not the BIOS; this signifies that we are also responsible
- for disabling it before entering apm/acpi suspend */
- int active;
- /* 'perfctr_pmdev' is here because the current (2.4.1) PM
- callback system doesn't handle hierarchical dependencies */
- struct pm_dev *perfctr_pmdev;
- /* r/w apic fields */
- unsigned int apic_id;
- unsigned int apic_taskpri;
- unsigned int apic_ldr;
- unsigned int apic_dfr;
- unsigned int apic_spiv;
- unsigned int apic_lvtt;
- unsigned int apic_lvtpc;
- unsigned int apic_lvt0;
- unsigned int apic_lvt1;
- unsigned int apic_lvterr;
- unsigned int apic_tmict;
- unsigned int apic_tdcr;
- unsigned int apic_thmr;
-} apic_pm_state;
-
-static void apic_pm_suspend(void *data)
+static int apic_suspend(struct device *dev, u32 state, u32 level)
{
unsigned int l, h;
unsigned long flags;
- if (apic_pm_state.perfctr_pmdev)
- pm_send(apic_pm_state.perfctr_pmdev, PM_SUSPEND, data);
+ if (level != SUSPEND_DISABLE)
+ return 0;
+
apic_pm_state.apic_id = apic_read(APIC_ID);
apic_pm_state.apic_taskpri = apic_read(APIC_TASKPRI);
apic_pm_state.apic_ldr = apic_read(APIC_LDR);
@@ -493,13 +496,19 @@
l &= ~MSR_IA32_APICBASE_ENABLE;
wrmsr(MSR_IA32_APICBASE, l, h);
local_irq_restore(flags);
+ return 0;
}
-static void apic_pm_resume(void *data)
+static int apic_resume(struct device *dev, u32 level)
{
unsigned int l, h;
unsigned long flags;
+ if (level != RESUME_POWER_ON)
+ return 0;
+
+ set_fixmap_nocache(FIX_APIC_BASE, apic_phys); /* FIXME: this is needed for S3 resume, but why? */
+
local_irq_save(flags);
rdmsr(MSR_IA32_APICBASE, l, h);
l &= ~MSR_IA32_APICBASE_BASE;
@@ -524,74 +533,34 @@
apic_write(APIC_ESR, 0);
apic_read(APIC_ESR);
local_irq_restore(flags);
- if (apic_pm_state.perfctr_pmdev)
- pm_send(apic_pm_state.perfctr_pmdev, PM_RESUME, data);
-}
-
-static int apic_pm_callback(struct pm_dev *dev, pm_request_t rqst, void *data)
-{
- switch (rqst) {
- case PM_SUSPEND:
- apic_pm_suspend(data);
- break;
- case PM_RESUME:
- apic_pm_resume(data);
- break;
- }
return 0;
}
-/* perfctr driver should call this instead of pm_register() */
-struct pm_dev *apic_pm_register(pm_dev_t type,
- unsigned long id,
- pm_callback callback)
-{
- struct pm_dev *dev;
-
- if (!apic_pm_state.active)
- return pm_register(type, id, callback);
- if (apic_pm_state.perfctr_pmdev)
- return NULL; /* we're busy */
- dev = kmalloc(sizeof(struct pm_dev), GFP_KERNEL);
- if (dev) {
- memset(dev, 0, sizeof(*dev));
- dev->type = type;
- dev->id = id;
- dev->callback = callback;
- apic_pm_state.perfctr_pmdev = dev;
- }
- return dev;
-}
-
-/* perfctr driver should call this instead of pm_unregister() */
-void apic_pm_unregister(struct pm_dev *dev)
-{
- if (!apic_pm_state.active) {
- pm_unregister(dev);
- } else if (dev == apic_pm_state.perfctr_pmdev) {
- apic_pm_state.perfctr_pmdev = NULL;
- kfree(dev);
- }
-}
+static struct device_driver apic_driver = {
+ .name = "apic",
+ .bus = &system_bus_type,
+ .resume = apic_resume,
+ .suspend = apic_suspend,
+};
+
+struct sys_device device_apic = {
+ .name = "apic",
+ .id = 0,
+ .dev = {
+ .name = "APIC",
+ .driver = &apic_driver,
+ },
+};
-static void __init apic_pm_init1(void)
-{
- /* can't pm_register() at this early stage in the boot process
- (causes an immediate reboot), so just set the flag */
- apic_pm_state.active = 1;
-}
-
-static void __init apic_pm_init2(void)
+static int __init init_apic_devicefs(void)
{
+ driver_register(&apic_driver);
if (apic_pm_state.active)
- pm_register(PM_SYS_DEV, 0, apic_pm_callback);
+ return sys_device_register(&device_apic);
+ return 0;
}
-#else /* CONFIG_PM */
-
-static inline void apic_pm_init1(void) { }
-static inline void apic_pm_init2(void) { }
-
+device_initcall(init_apic_devicefs);
#endif /* CONFIG_PM */
/*
@@ -658,9 +627,6 @@
nmi_watchdog = NMI_LOCAL_APIC;
printk("Found and enabled local APIC!\n");
-
- apic_pm_init1();
-
return 0;
no_apic:
@@ -670,8 +636,6 @@
void __init init_apic_mappings(void)
{
- unsigned long apic_phys;
-
/*
* If no local APIC can be found then set up a fake all
* zeroes page to simulate the local APIC and another
@@ -1141,8 +1105,6 @@
phys_cpu_present_map = 1;
apic_write_around(APIC_ID, boot_cpu_physical_apicid);
- apic_pm_init2();
-
setup_local_APIC();
if (nmi_watchdog == NMI_LOCAL_APIC)
--- clean/arch/i386/kernel/apm.c 2003-01-09 22:16:11.000000000 +0100
+++ linux-swsusp/arch/i386/kernel/apm.c 2003-01-28 10:35:51.000000000 +0100
@@ -1263,6 +1263,11 @@
}
printk(KERN_CRIT "apm: suspend was vetoed, but suspending anyway.\n");
}
+
+ device_suspend(3, SUSPEND_NOTIFY);
+ device_suspend(3, SUSPEND_SAVE_STATE);
+ device_suspend(3, SUSPEND_DISABLE);
+
/* serialize with the timer interrupt */
write_lock_irq(&xtime_lock);
@@ -1283,6 +1288,8 @@
if (err != APM_SUCCESS)
apm_error("suspend", err);
err = (err == APM_SUCCESS) ? 0 : -EIO;
+ device_resume(RESUME_RESTORE_STATE);
+ device_resume(RESUME_ENABLE);
pm_send_all(PM_RESUME, (void *)0);
queue_event(APM_NORMAL_RESUME, NULL);
out:
@@ -1396,6 +1403,8 @@
write_lock_irq(&xtime_lock);
set_time();
write_unlock_irq(&xtime_lock);
+ device_resume(RESUME_RESTORE_STATE);
+ device_resume(RESUME_ENABLE);
pm_send_all(PM_RESUME, (void *)0);
queue_event(event, NULL);
}
--- clean/arch/i386/kernel/i386_ksyms.c 2003-01-17 23:09:51.000000000 +0100
+++ linux-swsusp/arch/i386/kernel/i386_ksyms.c 2003-01-19 19:58:34.000000000 +0100
@@ -161,10 +161,6 @@
EXPORT_SYMBOL(flush_tlb_page);
#endif
-#if defined(CONFIG_X86_LOCAL_APIC) && defined(CONFIG_PM)
-EXPORT_SYMBOL_GPL(set_nmi_pm_callback);
-EXPORT_SYMBOL_GPL(unset_nmi_pm_callback);
-#endif
#ifdef CONFIG_X86_IO_APIC
EXPORT_SYMBOL(IO_APIC_get_PCI_irq_vector);
#endif
--- clean/arch/i386/kernel/nmi.c 2003-01-05 22:58:19.000000000 +0100
+++ linux-swsusp/arch/i386/kernel/nmi.c 2003-02-09 11:43:29.000000000 +0100
@@ -9,6 +9,7 @@
* Mikael Pettersson : AMD K7 support for local APIC NMI watchdog.
* Mikael Pettersson : Power Management for local APIC NMI watchdog.
* Mikael Pettersson : Pentium 4 support for local APIC NMI watchdog.
+ * Pavel Machek : Driver model here, too.
*/
#include <linux/config.h>
@@ -20,6 +21,7 @@
#include <linux/interrupt.h>
#include <linux/mc146818rtc.h>
#include <linux/kernel_stat.h>
+#include <linux/device.h>
#include <asm/smp.h>
#include <asm/mtrr.h>
@@ -29,6 +31,7 @@
static unsigned int nmi_hz = HZ;
unsigned int nmi_perfctr_msr; /* the MSR to reset in NMI handler */
extern void show_registers(struct pt_regs *regs);
+static int nmi_active;
#define K7_EVNTSEL_ENABLE (1 << 22)
#define K7_EVNTSEL_INT (1 << 20)
@@ -118,10 +121,6 @@
* missing bits. Right now Intel P6/P4 and AMD K7 only.
*/
if ((nmi == NMI_LOCAL_APIC) &&
- (boot_cpu_data.x86_vendor == X86_VENDOR_INTEL) &&
- (boot_cpu_data.x86 == 6 || boot_cpu_data.x86 == 15))
- nmi_watchdog = nmi;
- if ((nmi == NMI_LOCAL_APIC) &&
(boot_cpu_data.x86_vendor == X86_VENDOR_AMD) &&
(boot_cpu_data.x86 == 6 || boot_cpu_data.x86 == 15))
nmi_watchdog = nmi;
@@ -136,14 +135,11 @@
__setup("nmi_watchdog=", setup_nmi_watchdog);
-#ifdef CONFIG_PM
-
-#include <linux/pm.h>
-struct pm_dev *nmi_pmdev;
-
-static void disable_apic_nmi_watchdog(void)
+void disable_apic_nmi_watchdog(void)
{
+ if (!nmi_active)
+ return;
switch (boot_cpu_data.x86_vendor) {
case X86_VENDOR_AMD:
wrmsr(MSR_K7_EVNTSEL0, 0, 0);
@@ -160,47 +156,58 @@
}
break;
}
+ nmi_active = 0;
}
-static int nmi_pm_callback(struct pm_dev *dev, pm_request_t rqst, void *data)
+#ifdef CONFIG_PM
+
+static int nmi_suspend(struct device *dev, u32 state, u32 level)
{
- switch (rqst) {
- case PM_SUSPEND:
- disable_apic_nmi_watchdog();
- break;
- case PM_RESUME:
- setup_apic_nmi_watchdog();
- break;
- }
+ if (level != SUSPEND_DISABLE)
+ return 0;
+
+ disable_apic_nmi_watchdog();
return 0;
}
-struct pm_dev * set_nmi_pm_callback(pm_callback callback)
+static int nmi_resume(struct device *dev, u32 level)
{
- apic_pm_unregister(nmi_pmdev);
- return apic_pm_register(PM_SYS_DEV, 0, callback);
-}
+ if (level != RESUME_POWER_ON)
+ return 0;
-void unset_nmi_pm_callback(struct pm_dev * dev)
-{
- apic_pm_unregister(dev);
- nmi_pmdev = apic_pm_register(PM_SYS_DEV, 0, nmi_pm_callback);
-}
-
-static void nmi_pm_init(void)
-{
- if (!nmi_pmdev)
- nmi_pmdev = apic_pm_register(PM_SYS_DEV, 0, nmi_pm_callback);
+ if (nmi_watchdog == NMI_LOCAL_APIC)
+ setup_apic_nmi_watchdog();
+
+ return 0;
}
-#define __pminit /*empty*/
-#else /* CONFIG_PM */
+static struct device_driver nmi_driver = {
+ .name = "nmi",
+ .bus = &system_bus_type,
+ .resume = nmi_resume,
+ .suspend = nmi_suspend,
+};
+
+static struct device device_nmi = {
+ .name = "NMI",
+ .bus_id = "NMI",
+ .driver = &nmi_driver,
+};
+
+extern struct sys_device device_apic;
-static inline void nmi_pm_init(void) { }
+static int __init init_nmi_devicefs(void)
+{
+ driver_register(&nmi_driver);
+
+ device_nmi.parent = &device_apic.dev;
+ return device_register(&device_nmi);
+}
-#define __pminit __init
+device_initcall(init_nmi_devicefs);
+#define __pminit
#endif /* CONFIG_PM */
/*
@@ -290,7 +297,7 @@
return 1;
}
-void __pminit setup_apic_nmi_watchdog (void)
+void setup_apic_nmi_watchdog (void)
{
switch (boot_cpu_data.x86_vendor) {
case X86_VENDOR_AMD:
@@ -314,7 +321,7 @@
default:
return;
}
- nmi_pm_init();
+ nmi_active = 1;
}
static spinlock_t nmi_print_lock = SPIN_LOCK_UNLOCKED;
--- clean/arch/i386/oprofile/nmi_int.c 2003-01-05 22:58:19.000000000 +0100
+++ linux-swsusp/arch/i386/oprofile/nmi_int.c 2003-02-09 12:16:52.000000000 +0100
@@ -11,6 +11,7 @@
#include <linux/notifier.h>
#include <linux/smp.h>
#include <linux/oprofile.h>
+#include <linux/device.h>
#include <asm/nmi.h>
#include <asm/msr.h>
#include <asm/apic.h>
@@ -24,27 +25,6 @@
static int nmi_start(void);
static void nmi_stop(void);
-
-static struct pm_dev * oprofile_pmdev;
-
-/* We're at risk of causing big trouble unless we
- * make sure to not cause any NMI interrupts when
- * suspended.
- */
-static int oprofile_pm_callback(struct pm_dev * dev,
- pm_request_t rqst, void * data)
-{
- switch (rqst) {
- case PM_SUSPEND:
- nmi_stop();
- break;
- case PM_RESUME:
- nmi_start();
- break;
- }
- return 0;
-}
-
static int nmi_callback(struct pt_regs * regs, int cpu)
{
@@ -86,7 +66,42 @@
saved_lvtpc[cpu] = apic_read(APIC_LVTPC);
apic_write(APIC_LVTPC, APIC_DM_NMI);
}
-
+
+static int nmi_enabled = 0; /* 0 == registered but off, 1 == registered and on */
+
+static int nmi_suspend(struct device *dev, u32 state, u32 level)
+{
+ if (level != SUSPEND_DISABLE)
+ return 0;
+ if (nmi_enabled == 1)
+ nmi_stop();
+ return 0;
+}
+
+static int nmi_resume(struct device *dev, u32 level)
+{
+ if (level != RESUME_POWER_ON)
+ return 0;
+ if (nmi_enabled == 1)
+ nmi_start();
+ return 0;
+}
+
+
+static struct device_driver nmi_driver = {
+ .name = "oprofile",
+ .bus = &system_bus_type,
+ .resume = nmi_resume,
+ .suspend = nmi_suspend,
+};
+
+static struct device device_nmi = {
+ .name = "oprofile",
+ .bus_id = "oprofile",
+ .driver = &nmi_driver,
+};
+
+extern struct sys_device device_apic;
static int nmi_setup(void)
{
@@ -95,13 +110,25 @@
* without actually triggering any NMIs as this will
* break the core code horrifically.
*/
+ if (nmi_watchdog == NMI_LOCAL_APIC) {
+ disable_apic_nmi_watchdog();
+ nmi_watchdog = NMI_LOCAL_APIC_SUSPENDED_BY_OPROFILE;
+ }
smp_call_function(nmi_cpu_setup, NULL, 0, 1);
nmi_cpu_setup(0);
set_nmi_callback(nmi_callback);
- oprofile_pmdev = set_nmi_pm_callback(oprofile_pm_callback);
- return 0;
+ nmi_enabled = 1;
+ return 0;
}
+static int __init init_nmi_driverfs(void)
+{
+ driver_register(&nmi_driver);
+ device_nmi.parent = &device_apic.dev;
+ device_register(&device_nmi);
+}
+
+device_initcall(init_nmi_driverfs);
static void nmi_restore_registers(struct op_msrs * msrs)
{
@@ -146,10 +173,14 @@
static void nmi_shutdown(void)
{
- unset_nmi_pm_callback(oprofile_pmdev);
+ nmi_enabled = 0;
unset_nmi_callback();
smp_call_function(nmi_cpu_shutdown, NULL, 0, 1);
nmi_cpu_shutdown(0);
+ if (nmi_watchdog == NMI_LOCAL_APIC_SUSPENDED_BY_OPROFILE) {
+ nmi_watchdog = NMI_LOCAL_APIC_SUSPENDED_BY_OPROFILE;
+ setup_apic_nmi_watchdog();
+ }
}
@@ -217,9 +248,9 @@
int __init nmi_init(struct oprofile_operations ** ops, enum oprofile_cpu * cpu)
{
- __u8 vendor = current_cpu_data.x86_vendor;
- __u8 family = current_cpu_data.x86;
- __u8 cpu_model = current_cpu_data.x86_model;
+ u8 vendor = current_cpu_data.x86_vendor;
+ u8 family = current_cpu_data.x86;
+ u8 cpu_model = current_cpu_data.x86_model;
if (!cpu_has_apic)
return 0;
--- clean/include/asm-i386/apic.h 2002-10-20 16:22:45.000000000 +0200
+++ linux-swsusp/include/asm-i386/apic.h 2003-02-09 11:46:09.000000000 +0100
@@ -95,6 +95,7 @@
#define NMI_IO_APIC 1
#define NMI_LOCAL_APIC 2
#define NMI_INVALID 3
+#define NMI_LOCAL_APIC_SUSPENDED_BY_OPROFILE 4
#endif /* CONFIG_X86_LOCAL_APIC */
--
Worst form of spam? Adding advertisment signatures ala sourceforge.net.
What goes next? Inserting advertisment *into* email?
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: Switch APIC (+nmi, +oprofile) to driver model
2003-02-09 11:32 Switch APIC (+nmi, +oprofile) to driver model Pavel Machek
@ 2003-02-09 11:54 ` John Levon
2003-02-09 12:03 ` John Levon
1 sibling, 0 replies; 27+ messages in thread
From: John Levon @ 2003-02-09 11:54 UTC (permalink / raw)
To: Pavel Machek; +Cc: kernel list
On Sun, Feb 09, 2003 at 12:32:01PM +0100, Pavel Machek wrote:
> + if (nmi_watchdog == NMI_LOCAL_APIC_SUSPENDED_BY_OPROFILE) {
> + nmi_watchdog = NMI_LOCAL_APIC_SUSPENDED_BY_OPROFILE;
This is obviously wrong...
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: Switch APIC (+nmi, +oprofile) to driver model
2003-02-09 11:32 Switch APIC (+nmi, +oprofile) to driver model Pavel Machek
2003-02-09 11:54 ` John Levon
@ 2003-02-09 12:03 ` John Levon
1 sibling, 0 replies; 27+ messages in thread
From: John Levon @ 2003-02-09 12:03 UTC (permalink / raw)
To: Pavel Machek; +Cc: kernel list
On Sun, Feb 09, 2003 at 12:32:01PM +0100, Pavel Machek wrote:
> static void nmi_shutdown(void)
> {
> - unset_nmi_pm_callback(oprofile_pmdev);
> + nmi_enabled = 0;
> unset_nmi_callback();
> smp_call_function(nmi_cpu_shutdown, NULL, 0, 1);
> nmi_cpu_shutdown(0);
> + if (nmi_watchdog == NMI_LOCAL_APIC_SUSPENDED_BY_OPROFILE) {
> + nmi_watchdog = NMI_LOCAL_APIC_SUSPENDED_BY_OPROFILE;
> + setup_apic_nmi_watchdog();
> + }
It looks to me like you'll end up enabilng the watchdog even if user
didn't enable the watchdog at boot up. Also, setup_apic_nmi_watchdog()
and the disable function need to exported to modules.
john
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: Switch APIC (+nmi, +oprofile) to driver model
2003-02-16 12:43 Mikael Pettersson
2003-02-16 16:01 ` Pavel Machek
@ 2003-02-26 21:59 ` Pavel Machek
1 sibling, 0 replies; 27+ messages in thread
From: Pavel Machek @ 2003-02-26 21:59 UTC (permalink / raw)
To: Mikael Pettersson; +Cc: levon, linux-kernel
Hi!
> Although I find it ugly, I could accept "lapic" as a shorter
> replacement for "local_apic" in identifiers.
Did local_apic -> lapic conversion and fixed code so that it does not
need changes at sysfs level. This is the result. Do you find it
acceptable?
Pavel
--- clean/arch/i386/kernel/apic.c 2003-02-25 21:13:43.000000000 +0100
+++ linux/arch/i386/kernel/apic.c 2003-02-26 21:19:24.000000000 +0100
@@ -10,6 +10,8 @@
* for testing these extensively.
* Maciej W. Rozycki : Various updates and fixes.
* Mikael Pettersson : Power Management for UP-APIC.
+ * Pavel Machek and
+ * Mikael Pettersson : Converted to driver model.
*/
#include <linux/config.h>
@@ -23,6 +25,10 @@
#include <linux/interrupt.h>
#include <linux/mc146818rtc.h>
#include <linux/kernel_stat.h>
+#include <linux/delay.h>
+#include <linux/slab.h>
+#include <linux/device.h>
+#include <linux/pm.h>
#include <asm/atomic.h>
#include <asm/smp.h>
@@ -77,7 +83,7 @@
return maxlvt;
}
-void clear_local_APIC(void)
+void clear_lapic(void)
{
int maxlvt;
unsigned long v;
@@ -143,7 +149,7 @@
/*
* Do not trust the local APIC being empty at bootup.
*/
- clear_local_APIC();
+ clear_lapic();
/*
* PIC mode, enable APIC mode in the IMCR, i.e.
* connect BSP's local APIC to INT and NMI lines.
@@ -169,11 +175,11 @@
}
}
-void disable_local_APIC(void)
+void disable_lapic(void)
{
unsigned long value;
- clear_local_APIC();
+ clear_lapic();
/*
* Disable APIC (implies clearing of registers
@@ -189,7 +195,7 @@
* Check these against your board if the CPUs aren't getting
* started for no apparent reason.
*/
-int __init verify_local_APIC(void)
+int __init verify_lapic(void)
{
unsigned int reg0, reg1;
@@ -279,7 +285,7 @@
/*
* Do not trust the local APIC being empty at bootup.
*/
- clear_local_APIC();
+ clear_lapic();
/*
* Enable APIC.
@@ -304,7 +310,28 @@
apic_write_around(APIC_LVT1, value);
}
-void __init setup_local_APIC (void)
+static struct {
+ /* 'active' is true if the local APIC was enabled by us and
+ not the BIOS; this signifies that we are also responsible
+ for disabling it before entering apm/acpi suspend */
+ int active;
+ /* r/w apic fields */
+ unsigned int apic_id;
+ unsigned int apic_taskpri;
+ unsigned int apic_ldr;
+ unsigned int apic_dfr;
+ unsigned int apic_spiv;
+ unsigned int apic_lvtt;
+ unsigned int apic_lvtpc;
+ unsigned int apic_lvt0;
+ unsigned int apic_lvt1;
+ unsigned int apic_lvterr;
+ unsigned int apic_tmict;
+ unsigned int apic_tdcr;
+ unsigned int apic_thmr;
+} apic_pm_state;
+
+void __init setup_lapic (void)
{
unsigned long value, ver, maxlvt;
@@ -445,46 +472,24 @@
printk("No ESR for 82489DX.\n");
}
- if (nmi_watchdog == NMI_LOCAL_APIC)
- setup_apic_nmi_watchdog();
+ if (nmi_watchdog == NMI_LOCAL_APIC) {
+ enable_lapic_nmi_watchdog();
+ }
+
+ apic_pm_state.active = 1;
}
#ifdef CONFIG_PM
-
-#include <linux/slab.h>
-#include <linux/pm.h>
-
-static struct {
- /* 'active' is true if the local APIC was enabled by us and
- not the BIOS; this signifies that we are also responsible
- for disabling it before entering apm/acpi suspend */
- int active;
- /* 'perfctr_pmdev' is here because the current (2.4.1) PM
- callback system doesn't handle hierarchical dependencies */
- struct pm_dev *perfctr_pmdev;
- /* r/w apic fields */
- unsigned int apic_id;
- unsigned int apic_taskpri;
- unsigned int apic_ldr;
- unsigned int apic_dfr;
- unsigned int apic_spiv;
- unsigned int apic_lvtt;
- unsigned int apic_lvtpc;
- unsigned int apic_lvt0;
- unsigned int apic_lvt1;
- unsigned int apic_lvterr;
- unsigned int apic_tmict;
- unsigned int apic_tdcr;
- unsigned int apic_thmr;
-} apic_pm_state;
-
-static void apic_pm_suspend(void *data)
+static int lapic_suspend(struct device *dev, u32 state, u32 level)
{
unsigned int l, h;
unsigned long flags;
- if (apic_pm_state.perfctr_pmdev)
- pm_send(apic_pm_state.perfctr_pmdev, PM_SUSPEND, data);
+ if (level != SUSPEND_POWER_DOWN)
+ return 0;
+ if (!apic_pm_state.active)
+ return 0;
+
apic_pm_state.apic_id = apic_read(APIC_ID);
apic_pm_state.apic_taskpri = apic_read(APIC_TASKPRI);
apic_pm_state.apic_ldr = apic_read(APIC_LDR);
@@ -500,18 +505,26 @@
apic_pm_state.apic_thmr = apic_read(APIC_LVTTHMR);
local_irq_save(flags);
- disable_local_APIC();
+ disable_lapic();
rdmsr(MSR_IA32_APICBASE, l, h);
l &= ~MSR_IA32_APICBASE_ENABLE;
wrmsr(MSR_IA32_APICBASE, l, h);
local_irq_restore(flags);
+ return 0;
}
-static void apic_pm_resume(void *data)
+static int lapic_resume(struct device *dev, u32 level)
{
unsigned int l, h;
unsigned long flags;
+ if (level != RESUME_POWER_ON)
+ return 0;
+ if (!apic_pm_state.active)
+ return 0;
+
+ set_fixmap_nocache(FIX_APIC_BASE, APIC_DEFAULT_PHYS_BASE); /* FIXME: this is needed for S3 resume, but why? */
+
local_irq_save(flags);
rdmsr(MSR_IA32_APICBASE, l, h);
l &= ~MSR_IA32_APICBASE_BASE;
@@ -536,74 +549,35 @@
apic_write(APIC_ESR, 0);
apic_read(APIC_ESR);
local_irq_restore(flags);
- if (apic_pm_state.perfctr_pmdev)
- pm_send(apic_pm_state.perfctr_pmdev, PM_RESUME, data);
-}
-
-static int apic_pm_callback(struct pm_dev *dev, pm_request_t rqst, void *data)
-{
- switch (rqst) {
- case PM_SUSPEND:
- apic_pm_suspend(data);
- break;
- case PM_RESUME:
- apic_pm_resume(data);
- break;
- }
return 0;
}
-/* perfctr driver should call this instead of pm_register() */
-struct pm_dev *apic_pm_register(pm_dev_t type,
- unsigned long id,
- pm_callback callback)
-{
- struct pm_dev *dev;
-
- if (!apic_pm_state.active)
- return pm_register(type, id, callback);
- if (apic_pm_state.perfctr_pmdev)
- return NULL; /* we're busy */
- dev = kmalloc(sizeof(struct pm_dev), GFP_KERNEL);
- if (dev) {
- memset(dev, 0, sizeof(*dev));
- dev->type = type;
- dev->id = id;
- dev->callback = callback;
- apic_pm_state.perfctr_pmdev = dev;
- }
- return dev;
-}
+static struct device_driver lapic_driver = {
+ .name = "apic",
+ .bus = &system_bus_type,
+ .resume = lapic_resume,
+ .suspend = lapic_suspend,
+};
+
+/* not static, needed by child devices */
+struct sys_device device_lapic = {
+ .name = "apic",
+ .id = 0,
+ .dev = {
+ .name = "lapic",
+ .driver = &lapic_driver,
+ },
+};
-/* perfctr driver should call this instead of pm_unregister() */
-void apic_pm_unregister(struct pm_dev *dev)
-{
- if (!apic_pm_state.active) {
- pm_unregister(dev);
- } else if (dev == apic_pm_state.perfctr_pmdev) {
- apic_pm_state.perfctr_pmdev = NULL;
- kfree(dev);
- }
-}
-
-static void __init apic_pm_init1(void)
-{
- /* can't pm_register() at this early stage in the boot process
- (causes an immediate reboot), so just set the flag */
- apic_pm_state.active = 1;
-}
-
-static void __init apic_pm_init2(void)
+static int __init init_apic_devicefs(void)
{
+ driver_register(&lapic_driver);
if (apic_pm_state.active)
- pm_register(PM_SYS_DEV, 0, apic_pm_callback);
+ return sys_device_register(&device_lapic);
+ return 0;
}
-#else /* CONFIG_PM */
-
-static inline void apic_pm_init1(void) { }
-static inline void apic_pm_init2(void) { }
-
+device_initcall(init_apic_devicefs);
#endif /* CONFIG_PM */
/*
@@ -670,9 +644,6 @@
nmi_watchdog = NMI_LOCAL_APIC;
printk("Found and enabled local APIC!\n");
-
- apic_pm_init1();
-
return 0;
no_apic:
@@ -683,7 +654,6 @@
void __init init_apic_mappings(void)
{
unsigned long apic_phys;
-
/*
* If no local APIC can be found then set up a fake all
* zeroes page to simulate the local APIC and another
@@ -1150,16 +1120,14 @@
return -1;
}
- verify_local_APIC();
+ verify_lapic();
connect_bsp_APIC();
phys_cpu_present_map = 1;
apic_write_around(APIC_ID, boot_cpu_physical_apicid);
- apic_pm_init2();
-
- setup_local_APIC();
+ setup_lapic();
if (nmi_watchdog == NMI_LOCAL_APIC)
check_nmi_watchdog();
--- clean/arch/i386/kernel/apm.c 2003-02-25 21:13:43.000000000 +0100
+++ linux/arch/i386/kernel/apm.c 2003-02-26 21:43:41.000000000 +0100
@@ -1237,6 +1237,10 @@
}
printk(KERN_CRIT "apm: suspend was vetoed, but suspending anyway.\n");
}
+
+
+ device_suspend(3, SUSPEND_POWER_DOWN);
+
/* serialize with the timer interrupt */
write_seqlock_irq(&xtime_lock);
@@ -1257,6 +1261,7 @@
if (err != APM_SUCCESS)
apm_error("suspend", err);
err = (err == APM_SUCCESS) ? 0 : -EIO;
+ device_resume(RESUME_POWER_ON);
pm_send_all(PM_RESUME, (void *)0);
queue_event(APM_NORMAL_RESUME, NULL);
out:
@@ -1370,6 +1375,7 @@
write_seqlock_irq(&xtime_lock);
set_time();
write_sequnlock_irq(&xtime_lock);
+ device_resume(RESUME_POWER_ON);
pm_send_all(PM_RESUME, (void *)0);
queue_event(event, NULL);
}
--- clean/arch/i386/kernel/nmi.c 2003-02-25 21:13:46.000000000 +0100
+++ linux/arch/i386/kernel/nmi.c 2003-02-26 21:19:31.000000000 +0100
@@ -9,6 +9,8 @@
* Mikael Pettersson : AMD K7 support for local APIC NMI watchdog.
* Mikael Pettersson : Power Management for local APIC NMI watchdog.
* Mikael Pettersson : Pentium 4 support for local APIC NMI watchdog.
+ * Pavel Machek and
+ * Mikael Pettersson : PM converted to driver model. disable/enable API.
*/
#include <linux/config.h>
@@ -20,6 +22,8 @@
#include <linux/interrupt.h>
#include <linux/mc146818rtc.h>
#include <linux/kernel_stat.h>
+#include <linux/device.h>
+#include <linux/module.h>
#include <asm/smp.h>
#include <asm/mtrr.h>
@@ -29,6 +33,7 @@
static unsigned int nmi_hz = HZ;
unsigned int nmi_perfctr_msr; /* the MSR to reset in NMI handler */
extern void show_registers(struct pt_regs *regs);
+static int nmi_active;
#define K7_EVNTSEL_ENABLE (1 << 22)
#define K7_EVNTSEL_INT (1 << 20)
@@ -117,7 +122,7 @@
*/
if ((nmi == NMI_LOCAL_APIC) &&
(boot_cpu_data.x86_vendor == X86_VENDOR_INTEL) &&
- (boot_cpu_data.x86 == 6 || boot_cpu_data.x86 == 15))
+ (boot_cpu_data.x86 == 6 || boot_cpu_data.x86 == 15))
nmi_watchdog = nmi;
if ((nmi == NMI_LOCAL_APIC) &&
(boot_cpu_data.x86_vendor == X86_VENDOR_AMD) &&
@@ -134,14 +139,11 @@
__setup("nmi_watchdog=", setup_nmi_watchdog);
-#ifdef CONFIG_PM
-
-#include <linux/pm.h>
-
-struct pm_dev *nmi_pmdev;
-static void disable_apic_nmi_watchdog(void)
+void disable_lapic_nmi_watchdog(void)
{
+ if (!nmi_active)
+ return;
switch (boot_cpu_data.x86_vendor) {
case X86_VENDOR_AMD:
wrmsr(MSR_K7_EVNTSEL0, 0, 0);
@@ -158,46 +160,60 @@
}
break;
}
+ nmi_active = 0;
}
-static int nmi_pm_callback(struct pm_dev *dev, pm_request_t rqst, void *data)
+#ifdef CONFIG_PM
+
+static int nmi_suspend(struct device *dev, u32 state, u32 level)
{
- switch (rqst) {
- case PM_SUSPEND:
- disable_apic_nmi_watchdog();
- break;
- case PM_RESUME:
- setup_apic_nmi_watchdog();
- break;
- }
+ if (level != SUSPEND_POWER_DOWN)
+ return 0;
+
+ disable_lapic_nmi_watchdog();
return 0;
}
-struct pm_dev * set_nmi_pm_callback(pm_callback callback)
+static int nmi_resume(struct device *dev, u32 level)
{
- apic_pm_unregister(nmi_pmdev);
- return apic_pm_register(PM_SYS_DEV, 0, callback);
-}
+ if (level != RESUME_POWER_ON)
+ return 0;
-void unset_nmi_pm_callback(struct pm_dev * dev)
-{
- apic_pm_unregister(dev);
- nmi_pmdev = apic_pm_register(PM_SYS_DEV, 0, nmi_pm_callback);
-}
-
-static void nmi_pm_init(void)
-{
- if (!nmi_pmdev)
- nmi_pmdev = apic_pm_register(PM_SYS_DEV, 0, nmi_pm_callback);
+ if (nmi_watchdog == NMI_LOCAL_APIC)
+ enable_lapic_nmi_watchdog();
+
+ return 0;
}
-#define __pminit /*empty*/
-#else /* CONFIG_PM */
+static struct device_driver nmi_driver = {
+ .name = "lapic_nmi",
+ .bus = &system_bus_type,
+ .resume = nmi_resume,
+ .suspend = nmi_suspend,
+};
+
+static struct sys_device device_nmi = {
+ .name = "lapic_nmi",
+ .id = 0,
+ .dev = {
+ .name = "lapic_nmi",
+ .driver = &nmi_driver,
+ .parent = &device_lapic.dev,
+ }
+};
+
+extern struct sys_device device_apic;
-static inline void nmi_pm_init(void) { }
+static int __init init_nmi_devicefs(void)
+{
+ if (nmi_watchdog == NMI_LOCAL_APIC) {
+ driver_register(&nmi_driver);
+ return sys_device_register(&device_nmi);
+ }
+}
-#define __pminit __init
+late_initcall(init_nmi_devicefs);
#endif /* CONFIG_PM */
@@ -206,7 +222,7 @@
* Original code written by Keith Owens.
*/
-static void __pminit clear_msr_range(unsigned int base, unsigned int n)
+static void clear_msr_range(unsigned int base, unsigned int n)
{
unsigned int i;
@@ -214,7 +230,7 @@
wrmsr(base+i, 0, 0);
}
-static void __pminit setup_k7_watchdog(void)
+static void setup_k7_watchdog(void)
{
unsigned int evntsel;
@@ -236,7 +252,7 @@
wrmsr(MSR_K7_EVNTSEL0, evntsel, 0);
}
-static void __pminit setup_p6_watchdog(void)
+static void setup_p6_watchdog(void)
{
unsigned int evntsel;
@@ -258,7 +274,7 @@
wrmsr(MSR_P6_EVNTSEL0, evntsel, 0);
}
-static int __pminit setup_p4_watchdog(void)
+static int setup_p4_watchdog(void)
{
unsigned int misc_enable, dummy;
@@ -288,7 +304,7 @@
return 1;
}
-void __pminit setup_apic_nmi_watchdog (void)
+void enable_lapic_nmi_watchdog (void)
{
switch (boot_cpu_data.x86_vendor) {
case X86_VENDOR_AMD:
@@ -312,7 +328,7 @@
default:
return;
}
- nmi_pm_init();
+ nmi_active = 1;
}
static spinlock_t nmi_print_lock = SPIN_LOCK_UNLOCKED;
@@ -400,3 +416,7 @@
wrmsr(nmi_perfctr_msr, -(cpu_khz/nmi_hz*1000), -1);
}
}
+
+EXPORT_SYMBOL(nmi_watchdog);
+EXPORT_SYMBOL(enable_lapic_nmi_watchdog);
+EXPORT_SYMBOL(disable_lapic_nmi_watchdog);
--- clean/arch/i386/oprofile/nmi_int.c 2003-02-25 21:14:03.000000000 +0100
+++ linux/arch/i386/oprofile/nmi_int.c 2003-02-26 21:19:10.000000000 +0100
@@ -11,6 +11,7 @@
#include <linux/notifier.h>
#include <linux/smp.h>
#include <linux/oprofile.h>
+#include <linux/device.h>
#include <asm/nmi.h>
#include <asm/msr.h>
#include <asm/apic.h>
@@ -24,27 +25,6 @@
static int nmi_start(void);
static void nmi_stop(void);
-
-static struct pm_dev * oprofile_pmdev;
-
-/* We're at risk of causing big trouble unless we
- * make sure to not cause any NMI interrupts when
- * suspended.
- */
-static int oprofile_pm_callback(struct pm_dev * dev,
- pm_request_t rqst, void * data)
-{
- switch (rqst) {
- case PM_SUSPEND:
- nmi_stop();
- break;
- case PM_RESUME:
- nmi_start();
- break;
- }
- return 0;
-}
-
static int nmi_callback(struct pt_regs * regs, int cpu)
{
@@ -86,7 +66,41 @@
saved_lvtpc[cpu] = apic_read(APIC_LVTPC);
apic_write(APIC_LVTPC, APIC_DM_NMI);
}
-
+
+static int nmi_enabled = 0; /* 0 == registered but off, 1 == registered and on */
+
+static int nmi_suspend(struct device *dev, u32 state, u32 level)
+{
+ if (level != SUSPEND_POWER_DOWN)
+ return 0;
+ if (nmi_enabled == 1)
+ nmi_stop();
+ return 0;
+}
+
+static int nmi_resume(struct device *dev, u32 level)
+{
+ if (level != RESUME_POWER_ON)
+ return 0;
+ if (nmi_enabled == 1)
+ nmi_start();
+ return 0;
+}
+
+
+static struct device_driver nmi_driver = {
+ .name = "oprofile",
+ .bus = &system_bus_type,
+ .resume = nmi_resume,
+ .suspend = nmi_suspend,
+};
+
+static struct device device_nmi = {
+ .name = "oprofile",
+ .bus_id = "oprofile",
+ .driver = &nmi_driver,
+ .parent = &device_lapic,
+};
static int nmi_setup(void)
{
@@ -95,13 +109,27 @@
* without actually triggering any NMIs as this will
* break the core code horrifically.
*/
+ if (nmi_watchdog == NMI_LOCAL_APIC) {
+ disable_lapic_nmi_watchdog();
+ nmi_watchdog = NMI_LOCAL_APIC_SUSPENDED_BY_OPROFILE;
+ }
smp_call_function(nmi_cpu_setup, NULL, 0, 1);
nmi_cpu_setup(0);
set_nmi_callback(nmi_callback);
- oprofile_pmdev = set_nmi_pm_callback(oprofile_pm_callback);
- return 0;
+ nmi_enabled = 1;
+ return 0;
+}
+
+static int __init init_nmi_driverfs(void)
+{
+ if (nmi_watchdog == NMI_LOCAL_APIC) {
+ printk("Registering oprofile-nmi\n");
+ driver_register(&nmi_driver);
+ device_register(&device_nmi);
+ }
}
+late_initcall(init_nmi_driverfs);
static void nmi_restore_registers(struct op_msrs * msrs)
{
@@ -146,10 +174,14 @@
static void nmi_shutdown(void)
{
- unset_nmi_pm_callback(oprofile_pmdev);
+ nmi_enabled = 0;
unset_nmi_callback();
smp_call_function(nmi_cpu_shutdown, NULL, 0, 1);
nmi_cpu_shutdown(0);
+ if (nmi_watchdog == NMI_LOCAL_APIC_SUSPENDED_BY_OPROFILE) {
+ nmi_watchdog = NMI_LOCAL_APIC;
+ enable_lapic_nmi_watchdog();
+ }
}
--- clean/include/asm-i386/apic.h 2003-02-25 21:21:10.000000000 +0100
+++ linux/include/asm-i386/apic.h 2003-02-18 07:46:02.000000000 +0100
@@ -78,7 +78,8 @@
extern void smp_local_timer_interrupt (struct pt_regs * regs);
extern void setup_boot_APIC_clock (void);
extern void setup_secondary_APIC_clock (void);
-extern void setup_apic_nmi_watchdog (void);
+extern void enable_lapic_nmi_watchdog (void);
+extern void disable_lapic_nmi_watchdog (void);
extern inline void nmi_watchdog_tick (struct pt_regs * regs);
extern int APIC_init_uniprocessor (void);
extern void disable_APIC_timer(void);
@@ -95,6 +96,9 @@
#define NMI_IO_APIC 1
#define NMI_LOCAL_APIC 2
#define NMI_INVALID 3
+#define NMI_LOCAL_APIC_SUSPENDED_BY_OPROFILE 4
+
+extern struct sys_device device_lapic;
#endif /* CONFIG_X86_LOCAL_APIC */
--
When do you have a heart between your knees?
[Johanka's followup: and *two* hearts?]
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: Switch APIC (+nmi, +oprofile) to driver model
2003-02-16 12:43 Mikael Pettersson
@ 2003-02-16 16:01 ` Pavel Machek
2003-02-26 21:59 ` Pavel Machek
1 sibling, 0 replies; 27+ messages in thread
From: Pavel Machek @ 2003-02-16 16:01 UTC (permalink / raw)
To: Mikael Pettersson; +Cc: levon, linux-kernel
Hi!
> >> @@ -1263,6 +1264,11 @@
> >> }
> >> printk(KERN_CRIT "apm: suspend was vetoed, but suspending anyway.\n");
> >> }
> >> +
> >> + device_suspend(3, SUSPEND_NOTIFY);
> >> + device_suspend(3, SUSPEND_SAVE_STATE);
> >
> >Comment these two lines... and all RESTORE_STATEs. System needs to be
> >stopped in order for SAVE_STATE to work, and it is not in apm case.
>
> What's the proper fix? apm must be able to initiate suspend &
> resume.
Not sure.
In theory, apm must be able to initiate suspend & resume. But by the
same theory, apm bios should be doing hardware save stating itself --
and it obviously does not.
freeze_processes() should not fail, anyway, so right fix is to wrap
this with freeze_processes() and resume_processes().
Alternate fix is to invent level 6 and use it for apm. Things like ide
would then ignore level 6 (as apm is likely to do the right thing in
that case).
> >Do you think it is neccessary to call it "*local_*apic_nmi_driver"? It
> >seems way too long.
>
> Local APIC != I/O APIC. I try to be a caretaker for the former,
> so I dislike the ambiguous term "APIC".
Ok.
> >Why did you convert device_apic_nmi to *sys_*device?
>
> Otherwise the NMI watchdog device wouldn't show up in /sys.
Strange, it was there for me, IIRC.
> >This is good, if we have disable_, we should have enable_, not setup_;
> >but I killed _local_ part as it is way too long, then.
>
> Note that there is also an I/O APIC NMI generator.
> If you drop the "local" qualifier, things get ambiguous.
>
> Although I find it ugly, I could accept "lapic" as a shorter
> replacement for "local_apic" in identifiers.
Yep, lapic seems okay.
Pavel
--
When do you have a heart between your knees?
[Johanka's followup: and *two* hearts?]
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: Switch APIC (+nmi, +oprofile) to driver model
@ 2003-02-16 12:43 Mikael Pettersson
2003-02-16 16:01 ` Pavel Machek
2003-02-26 21:59 ` Pavel Machek
0 siblings, 2 replies; 27+ messages in thread
From: Mikael Pettersson @ 2003-02-16 12:43 UTC (permalink / raw)
To: pavel; +Cc: levon, linux-kernel
On Sun, 16 Feb 2003 13:05:16 +0100, Pavel Machek wrote:
>> --- linux-2.5.60/arch/i386/kernel/apm.c.~1~ 2003-02-10 23:36:54.000000000 +0100
>> +++ linux-2.5.60/arch/i386/kernel/apm.c 2003-02-12 21:01:51.000000000 +0100
>> @@ -218,6 +218,7 @@
>> #include <linux/time.h>
>> #include <linux/sched.h>
>> #include <linux/pm.h>
>> +#include <linux/device.h>
>> #include <linux/kernel.h>
>> #include <linux/smp.h>
>> #include <linux/smp_lock.h>
>> @@ -1263,6 +1264,11 @@
>> }
>> printk(KERN_CRIT "apm: suspend was vetoed, but suspending anyway.\n");
>> }
>> +
>> + device_suspend(3, SUSPEND_NOTIFY);
>> + device_suspend(3, SUSPEND_SAVE_STATE);
>
>Comment these two lines... and all RESTORE_STATEs. System needs to be
>stopped in order for SAVE_STATE to work, and it is not in apm case.
What's the proper fix? apm must be able to initiate suspend & resume.
(This part is straight from your patch, BTW. I only rediffed and
and added #include <linux/device.h>.)
>> +static struct device_driver local_apic_nmi_driver = {
>> + .name = "local_apic_nmi",
>> + .bus = &system_bus_type,
>> + .resume = nmi_resume,
>> + .suspend = nmi_suspend,
>> +};
>
>Do you think it is neccessary to call it "*local_*apic_nmi_driver"? It
>seems way too long.
Local APIC != I/O APIC. I try to be a caretaker for the former,
so I dislike the ambiguous term "APIC".
>> +extern struct sys_device device_local_apic;
>> +
>> +static struct sys_device device_local_apic_nmi = {
>> + .name = "local_apic_nmi",
>> + .id = 0,
>> + .dev = {
>> + .name = "local_apic_nmi",
>> + .driver = &local_apic_nmi_driver,
>> + .parent = &device_local_apic.dev,
>> + },
>> +};
>
>Why did you convert device_apic_nmi to *sys_*device?
Otherwise the NMI watchdog device wouldn't show up in /sys.
>> +EXPORT_SYMBOL(nmi_watchdog);
>> +EXPORT_SYMBOL(disable_local_apic_nmi_watchdog);
>> +EXPORT_SYMBOL(enable_local_apic_nmi_watchdog);
>
>This is good, if we have disable_, we should have enable_, not setup_;
>but I killed _local_ part as it is way too long, then.
Note that there is also an I/O APIC NMI generator.
If you drop the "local" qualifier, things get ambiguous.
Although I find it ugly, I could accept "lapic" as a shorter
replacement for "local_apic" in identifiers.
/Mikael
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: Switch APIC (+nmi, +oprofile) to driver model
2003-02-13 13:39 ` Mikael Pettersson
2003-02-13 13:46 ` Pavel Machek
2003-02-14 9:08 ` Maciej W. Rozycki
@ 2003-02-16 12:05 ` Pavel Machek
2 siblings, 0 replies; 27+ messages in thread
From: Pavel Machek @ 2003-02-16 12:05 UTC (permalink / raw)
To: Mikael Pettersson; +Cc: John Levon, linux-kernel
Hi!
> Here is my modified version of Pavel's latest patch to convert
> apm/apic/nmi to the driver model. It's a minimalistic patch,
> intendended ONLY to convert the old-fashioned PM support code
> to the driver model. It seems to work for me, except that
> initiating a suspend (via apm --suspend) triggers a BUG_ON
> somewhere in ide-disk.c, which prevents the suspend and causes a
> hang at shutdown.
> --- linux-2.5.60/arch/i386/kernel/apm.c.~1~ 2003-02-10 23:36:54.000000000 +0100
> +++ linux-2.5.60/arch/i386/kernel/apm.c 2003-02-12 21:01:51.000000000 +0100
> @@ -218,6 +218,7 @@
> #include <linux/time.h>
> #include <linux/sched.h>
> #include <linux/pm.h>
> +#include <linux/device.h>
> #include <linux/kernel.h>
> #include <linux/smp.h>
> #include <linux/smp_lock.h>
> @@ -1263,6 +1264,11 @@
> }
> printk(KERN_CRIT "apm: suspend was vetoed, but suspending anyway.\n");
> }
> +
> + device_suspend(3, SUSPEND_NOTIFY);
> + device_suspend(3, SUSPEND_SAVE_STATE);
Comment these two lines... and all RESTORE_STATEs. System needs to be
stopped in order for SAVE_STATE to work, and it is not in apm case.
> +static struct device_driver local_apic_nmi_driver = {
> + .name = "local_apic_nmi",
> + .bus = &system_bus_type,
> + .resume = nmi_resume,
> + .suspend = nmi_suspend,
> +};
Do you think it is neccessary to call it "*local_*apic_nmi_driver"? It
seems way too long.
> +extern struct sys_device device_local_apic;
> +
> +static struct sys_device device_local_apic_nmi = {
> + .name = "local_apic_nmi",
> + .id = 0,
> + .dev = {
> + .name = "local_apic_nmi",
> + .driver = &local_apic_nmi_driver,
> + .parent = &device_local_apic.dev,
> + },
> +};
Why did you convert device_apic_nmi to *sys_*device?
> @@ -402,3 +423,7 @@
> wrmsr(nmi_perfctr_msr, -(cpu_khz/nmi_hz*1000), -1);
> }
> }
> +
> +EXPORT_SYMBOL(nmi_watchdog);
> +EXPORT_SYMBOL(disable_local_apic_nmi_watchdog);
> +EXPORT_SYMBOL(enable_local_apic_nmi_watchdog);
This is good, if we have disable_, we should have enable_, not setup_;
but I killed _local_ part as it is way too long, then.
Pavel
--
When do you have a heart between your knees?
[Johanka's followup: and *two* hearts?]
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: Switch APIC (+nmi, +oprofile) to driver model
2003-02-14 11:25 ` Mikael Pettersson
@ 2003-02-14 11:32 ` Maciej W. Rozycki
0 siblings, 0 replies; 27+ messages in thread
From: Maciej W. Rozycki @ 2003-02-14 11:32 UTC (permalink / raw)
To: Mikael Pettersson; +Cc: Pavel Machek, John Levon, linux-kernel
On Fri, 14 Feb 2003, Mikael Pettersson wrote:
> > OK, but then the question is: are the following calls:
> >
> > + driver_register(&local_apic_driver);
> > + return sys_device_register(&device_local_apic);
> >
> > for suspend/resume exclusively?
>
> Yes.
OK, then.
> We could register the device also in other cases (!PM, SMP)
> but the methods would then be nullified and we'd have a device
> node with a name but no operations. I could do that, I just
> think it's pointless.
Agreed if the interface is not going to be extended further, i.e. the
intent is to cover PM-capable devices only.
I'd prefer the discrete APIC support not to get broken accidentally as
such systems are rare thus testing is limited.
--
+ Maciej W. Rozycki, Technical University of Gdansk, Poland +
+--------------------------------------------------------------+
+ e-mail: macro@ds2.pg.gda.pl, PGP key available +
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: Switch APIC (+nmi, +oprofile) to driver model
2003-02-14 10:54 ` Maciej W. Rozycki
@ 2003-02-14 11:25 ` Mikael Pettersson
2003-02-14 11:32 ` Maciej W. Rozycki
0 siblings, 1 reply; 27+ messages in thread
From: Mikael Pettersson @ 2003-02-14 11:25 UTC (permalink / raw)
To: Maciej W. Rozycki; +Cc: Pavel Machek, John Levon, linux-kernel
Maciej W. Rozycki writes:
> OK, but then the question is: are the following calls:
>
> + driver_register(&local_apic_driver);
> + return sys_device_register(&device_local_apic);
>
> for suspend/resume exclusively?
Yes.
We could register the device also in other cases (!PM, SMP)
but the methods would then be nullified and we'd have a device
node with a name but no operations. I could do that, I just
think it's pointless.
Getting suspend to work on SMP is a separate project. This
patch is just about upgrading to the new infrastructure.
/Mikael
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: Switch APIC (+nmi, +oprofile) to driver model
2003-02-14 10:32 ` Mikael Pettersson
@ 2003-02-14 10:54 ` Maciej W. Rozycki
2003-02-14 11:25 ` Mikael Pettersson
0 siblings, 1 reply; 27+ messages in thread
From: Maciej W. Rozycki @ 2003-02-14 10:54 UTC (permalink / raw)
To: Mikael Pettersson; +Cc: Pavel Machek, John Levon, linux-kernel
On Fri, 14 Feb 2003, Mikael Pettersson wrote:
> My goal was to not change any behaviour from our current code, and from
> what I can tell, our current code does not support PM suspend and resume
> for old external-local-APIC machines. (They're mostly 486 MPs, right?)
Both i486 and Pentium (typically for quad support, etc.).
> The suspend/resume procedures only work on P6/K7 and up. There's a
> bug there in that we may try to run the suspend on a UP P5 with enabled
> local APIC, which won't work. So far, no one seems to have noticed :->
OK, but then the question is: are the following calls:
+ driver_register(&local_apic_driver);
+ return sys_device_register(&device_local_apic);
for suspend/resume exclusively?
--
+ Maciej W. Rozycki, Technical University of Gdansk, Poland +
+--------------------------------------------------------------+
+ e-mail: macro@ds2.pg.gda.pl, PGP key available +
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: Switch APIC (+nmi, +oprofile) to driver model
2003-02-14 9:08 ` Maciej W. Rozycki
@ 2003-02-14 10:32 ` Mikael Pettersson
2003-02-14 10:54 ` Maciej W. Rozycki
0 siblings, 1 reply; 27+ messages in thread
From: Mikael Pettersson @ 2003-02-14 10:32 UTC (permalink / raw)
To: Maciej W. Rozycki; +Cc: Pavel Machek, John Levon, linux-kernel
Maciej W. Rozycki writes:
> On Thu, 13 Feb 2003, Mikael Pettersson wrote:
>
> > +static int __init init_local_apic_devicefs(void)
> > {
> > - if (apic_pm_state.active)
> > - pm_register(PM_SYS_DEV, 0, apic_pm_callback);
> > + if (!cpu_has_apic)
>
> This looks broken -- what if an external local APIC is present?
My goal was to not change any behaviour from our current code, and from
what I can tell, our current code does not support PM suspend and resume
for old external-local-APIC machines. (They're mostly 486 MPs, right?)
The suspend/resume procedures only work on P6/K7 and up. There's a
bug there in that we may try to run the suspend on a UP P5 with enabled
local APIC, which won't work. So far, no one seems to have noticed :->
(Admittedly, the P5 errata sheets say "BIOS should disable the P5 local
APIC on UP" so I don't think this case is very likely.)
/Mikael
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: Switch APIC (+nmi, +oprofile) to driver model
2003-02-13 13:39 ` Mikael Pettersson
2003-02-13 13:46 ` Pavel Machek
@ 2003-02-14 9:08 ` Maciej W. Rozycki
2003-02-14 10:32 ` Mikael Pettersson
2003-02-16 12:05 ` Pavel Machek
2 siblings, 1 reply; 27+ messages in thread
From: Maciej W. Rozycki @ 2003-02-14 9:08 UTC (permalink / raw)
To: Mikael Pettersson; +Cc: Pavel Machek, John Levon, linux-kernel
On Thu, 13 Feb 2003, Mikael Pettersson wrote:
> +static int __init init_local_apic_devicefs(void)
> {
> - if (apic_pm_state.active)
> - pm_register(PM_SYS_DEV, 0, apic_pm_callback);
> + if (!cpu_has_apic)
This looks broken -- what if an external local APIC is present?
> + return 0;
> + if (!apic_pm_state.active) {
> + local_apic_driver.resume = NULL;
> + local_apic_driver.suspend = NULL;
> + }
> + driver_register(&local_apic_driver);
> + return sys_device_register(&device_local_apic);
> }
--
+ Maciej W. Rozycki, Technical University of Gdansk, Poland +
+--------------------------------------------------------------+
+ e-mail: macro@ds2.pg.gda.pl, PGP key available +
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: Switch APIC (+nmi, +oprofile) to driver model
2003-02-13 13:39 ` Mikael Pettersson
@ 2003-02-13 13:46 ` Pavel Machek
2003-02-14 9:08 ` Maciej W. Rozycki
2003-02-16 12:05 ` Pavel Machek
2 siblings, 0 replies; 27+ messages in thread
From: Pavel Machek @ 2003-02-13 13:46 UTC (permalink / raw)
To: Mikael Pettersson; +Cc: John Levon, linux-kernel
Hi!
> Here is my modified version of Pavel's latest patch to convert
> apm/apic/nmi to the driver model. It's a minimalistic patch,
It looks good.
> intendended ONLY to convert the old-fashioned PM support code
> to the driver model. It seems to work for me, except that
> initiating a suspend (via apm --suspend) triggers a BUG_ON
> somewhere in ide-disk.c, which prevents the suspend and causes a
> hang at shutdown.
If you want to workaround that hang,
> @@ -1263,6 +1264,11 @@
> }
> printk(KERN_CRIT "apm: suspend was vetoed, but suspending anyway.\n");
> }
> +
> + device_suspend(3, SUSPEND_NOTIFY);
> + device_suspend(3, SUSPEND_SAVE_STATE);
Kill these two lines ^
> + device_suspend(3, SUSPEND_DISABLE);
> +
> /* serialize with the timer interrupt */
> write_seqlock_irq(&xtime_lock);
>
> @@ -1283,6 +1289,8 @@
> if (err != APM_SUCCESS)
> apm_error("suspend", err);
> err = (err == APM_SUCCESS) ? 0 : -EIO;
> + device_resume(RESUME_RESTORE_STATE);
This line ^
> + device_resume(RESUME_ENABLE);
> pm_send_all(PM_RESUME, (void *)0);
> queue_event(APM_NORMAL_RESUME, NULL);
> out:
> @@ -1396,6 +1404,8 @@
> write_seqlock_irq(&xtime_lock);
> set_time();
> write_sequnlock_irq(&xtime_lock);
> + device_resume(RESUME_RESTORE_STATE);
And this line ^.
Its not right thing to do, but it should make it work for you.
Pavel
--
Casualities in World Trade Center: ~3k dead inside the building,
cryptography in U.S.A. and free speech in Czech Republic.
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: Switch APIC (+nmi, +oprofile) to driver model
2003-02-11 12:01 ` Pavel Machek
@ 2003-02-13 13:39 ` Mikael Pettersson
2003-02-13 13:46 ` Pavel Machek
` (2 more replies)
0 siblings, 3 replies; 27+ messages in thread
From: Mikael Pettersson @ 2003-02-13 13:39 UTC (permalink / raw)
To: Pavel Machek; +Cc: John Levon, linux-kernel
Here is my modified version of Pavel's latest patch to convert
apm/apic/nmi to the driver model. It's a minimalistic patch,
intendended ONLY to convert the old-fashioned PM support code
to the driver model. It seems to work for me, except that
initiating a suspend (via apm --suspend) triggers a BUG_ON
somewhere in ide-disk.c, which prevents the suspend and causes a
hang at shutdown.
John: I didn't change oprofile's nmi_int.c from Pavel's version so
it's broken in this patch. I did add enable_local_apic_nmi_watchdog()
and disable_local_apic_nmi_watchdog() and EXPORT_SYMBOL'd them,
so you should be able to use that interface.
/Mikael
--- linux-2.5.60/arch/i386/kernel/apic.c.~1~ 2003-01-02 14:27:54.000000000 +0100
+++ linux-2.5.60/arch/i386/kernel/apic.c 2003-02-12 21:52:28.000000000 +0100
@@ -10,6 +10,8 @@
* for testing these extensively.
* Maciej W. Rozycki : Various updates and fixes.
* Mikael Pettersson : Power Management for UP-APIC.
+ * Pavel Machek and
+ * Mikael Pettersson : PM converted to driver model.
*/
#include <linux/config.h>
@@ -439,17 +441,13 @@
#ifdef CONFIG_PM
-#include <linux/slab.h>
-#include <linux/pm.h>
+#include <linux/device.h>
static struct {
/* 'active' is true if the local APIC was enabled by us and
not the BIOS; this signifies that we are also responsible
for disabling it before entering apm/acpi suspend */
int active;
- /* 'perfctr_pmdev' is here because the current (2.4.1) PM
- callback system doesn't handle hierarchical dependencies */
- struct pm_dev *perfctr_pmdev;
/* r/w apic fields */
unsigned int apic_id;
unsigned int apic_taskpri;
@@ -466,13 +464,14 @@
unsigned int apic_thmr;
} apic_pm_state;
-static void apic_pm_suspend(void *data)
+static int local_apic_suspend(struct device *dev, u32 state, u32 level)
{
unsigned int l, h;
unsigned long flags;
- if (apic_pm_state.perfctr_pmdev)
- pm_send(apic_pm_state.perfctr_pmdev, PM_SUSPEND, data);
+ if (level != SUSPEND_DISABLE)
+ return 0;
+
apic_pm_state.apic_id = apic_read(APIC_ID);
apic_pm_state.apic_taskpri = apic_read(APIC_TASKPRI);
apic_pm_state.apic_ldr = apic_read(APIC_LDR);
@@ -493,13 +492,17 @@
l &= ~MSR_IA32_APICBASE_ENABLE;
wrmsr(MSR_IA32_APICBASE, l, h);
local_irq_restore(flags);
+ return 0;
}
-static void apic_pm_resume(void *data)
+static int local_apic_resume(struct device *dev, u32 level)
{
unsigned int l, h;
unsigned long flags;
+ if (level != RESUME_POWER_ON)
+ return 0;
+
local_irq_save(flags);
rdmsr(MSR_IA32_APICBASE, l, h);
l &= ~MSR_IA32_APICBASE_BASE;
@@ -524,73 +527,47 @@
apic_write(APIC_ESR, 0);
apic_read(APIC_ESR);
local_irq_restore(flags);
- if (apic_pm_state.perfctr_pmdev)
- pm_send(apic_pm_state.perfctr_pmdev, PM_RESUME, data);
-}
-
-static int apic_pm_callback(struct pm_dev *dev, pm_request_t rqst, void *data)
-{
- switch (rqst) {
- case PM_SUSPEND:
- apic_pm_suspend(data);
- break;
- case PM_RESUME:
- apic_pm_resume(data);
- break;
- }
return 0;
}
-/* perfctr driver should call this instead of pm_register() */
-struct pm_dev *apic_pm_register(pm_dev_t type,
- unsigned long id,
- pm_callback callback)
-{
- struct pm_dev *dev;
-
- if (!apic_pm_state.active)
- return pm_register(type, id, callback);
- if (apic_pm_state.perfctr_pmdev)
- return NULL; /* we're busy */
- dev = kmalloc(sizeof(struct pm_dev), GFP_KERNEL);
- if (dev) {
- memset(dev, 0, sizeof(*dev));
- dev->type = type;
- dev->id = id;
- dev->callback = callback;
- apic_pm_state.perfctr_pmdev = dev;
- }
- return dev;
-}
-
-/* perfctr driver should call this instead of pm_unregister() */
-void apic_pm_unregister(struct pm_dev *dev)
-{
- if (!apic_pm_state.active) {
- pm_unregister(dev);
- } else if (dev == apic_pm_state.perfctr_pmdev) {
- apic_pm_state.perfctr_pmdev = NULL;
- kfree(dev);
- }
-}
+static struct device_driver local_apic_driver = {
+ .name = "local_apic",
+ .bus = &system_bus_type,
+ .resume = local_apic_resume,
+ .suspend = local_apic_suspend,
+};
+
+/* not static, needed by child devices */
+struct sys_device device_local_apic = {
+ .name = "local_apic",
+ .id = 0,
+ .dev = {
+ .name = "local_apic",
+ .driver = &local_apic_driver,
+ },
+};
-static void __init apic_pm_init1(void)
+static void __init apic_pm_activate(void)
{
- /* can't pm_register() at this early stage in the boot process
- (causes an immediate reboot), so just set the flag */
apic_pm_state.active = 1;
}
-static void __init apic_pm_init2(void)
+static int __init init_local_apic_devicefs(void)
{
- if (apic_pm_state.active)
- pm_register(PM_SYS_DEV, 0, apic_pm_callback);
+ if (!cpu_has_apic)
+ return 0;
+ if (!apic_pm_state.active) {
+ local_apic_driver.resume = NULL;
+ local_apic_driver.suspend = NULL;
+ }
+ driver_register(&local_apic_driver);
+ return sys_device_register(&device_local_apic);
}
+device_initcall(init_local_apic_devicefs);
#else /* CONFIG_PM */
-static inline void apic_pm_init1(void) { }
-static inline void apic_pm_init2(void) { }
+static inline void apic_pm_activate(void) { }
#endif /* CONFIG_PM */
@@ -659,7 +636,7 @@
printk("Found and enabled local APIC!\n");
- apic_pm_init1();
+ apic_pm_activate();
return 0;
@@ -1141,8 +1118,6 @@
phys_cpu_present_map = 1;
apic_write_around(APIC_ID, boot_cpu_physical_apicid);
- apic_pm_init2();
-
setup_local_APIC();
if (nmi_watchdog == NMI_LOCAL_APIC)
--- linux-2.5.60/arch/i386/kernel/apm.c.~1~ 2003-02-10 23:36:54.000000000 +0100
+++ linux-2.5.60/arch/i386/kernel/apm.c 2003-02-12 21:01:51.000000000 +0100
@@ -218,6 +218,7 @@
#include <linux/time.h>
#include <linux/sched.h>
#include <linux/pm.h>
+#include <linux/device.h>
#include <linux/kernel.h>
#include <linux/smp.h>
#include <linux/smp_lock.h>
@@ -1263,6 +1264,11 @@
}
printk(KERN_CRIT "apm: suspend was vetoed, but suspending anyway.\n");
}
+
+ device_suspend(3, SUSPEND_NOTIFY);
+ device_suspend(3, SUSPEND_SAVE_STATE);
+ device_suspend(3, SUSPEND_DISABLE);
+
/* serialize with the timer interrupt */
write_seqlock_irq(&xtime_lock);
@@ -1283,6 +1289,8 @@
if (err != APM_SUCCESS)
apm_error("suspend", err);
err = (err == APM_SUCCESS) ? 0 : -EIO;
+ device_resume(RESUME_RESTORE_STATE);
+ device_resume(RESUME_ENABLE);
pm_send_all(PM_RESUME, (void *)0);
queue_event(APM_NORMAL_RESUME, NULL);
out:
@@ -1396,6 +1404,8 @@
write_seqlock_irq(&xtime_lock);
set_time();
write_sequnlock_irq(&xtime_lock);
+ device_resume(RESUME_RESTORE_STATE);
+ device_resume(RESUME_ENABLE);
pm_send_all(PM_RESUME, (void *)0);
queue_event(event, NULL);
}
--- linux-2.5.60/arch/i386/kernel/i386_ksyms.c.~1~ 2003-02-10 23:36:54.000000000 +0100
+++ linux-2.5.60/arch/i386/kernel/i386_ksyms.c 2003-02-12 21:01:51.000000000 +0100
@@ -162,10 +162,6 @@
EXPORT_SYMBOL(flush_tlb_page);
#endif
-#if defined(CONFIG_X86_LOCAL_APIC) && defined(CONFIG_PM)
-EXPORT_SYMBOL_GPL(set_nmi_pm_callback);
-EXPORT_SYMBOL_GPL(unset_nmi_pm_callback);
-#endif
#ifdef CONFIG_X86_IO_APIC
EXPORT_SYMBOL(IO_APIC_get_PCI_irq_vector);
#endif
--- linux-2.5.60/arch/i386/kernel/nmi.c.~1~ 2003-02-12 21:00:57.000000000 +0100
+++ linux-2.5.60/arch/i386/kernel/nmi.c 2003-02-12 22:10:31.000000000 +0100
@@ -9,6 +9,8 @@
* Mikael Pettersson : AMD K7 support for local APIC NMI watchdog.
* Mikael Pettersson : Power Management for local APIC NMI watchdog.
* Mikael Pettersson : Pentium 4 support for local APIC NMI watchdog.
+ * Pavel Machek and
+ * Mikael Pettersson : PM converted to driver model. disable/enable API.
*/
#include <linux/config.h>
@@ -20,6 +22,7 @@
#include <linux/interrupt.h>
#include <linux/mc146818rtc.h>
#include <linux/kernel_stat.h>
+#include <linux/module.h>
#include <asm/smp.h>
#include <asm/mtrr.h>
@@ -136,14 +139,12 @@
__setup("nmi_watchdog=", setup_nmi_watchdog);
-#ifdef CONFIG_PM
-
-#include <linux/pm.h>
-
-struct pm_dev *nmi_pmdev;
+static int nmi_active; /* +1: active, 0: never active, -1: was active */
-static void disable_apic_nmi_watchdog(void)
+void disable_local_apic_nmi_watchdog(void)
{
+ if (nmi_active <= 0)
+ return;
switch (boot_cpu_data.x86_vendor) {
case X86_VENDOR_AMD:
wrmsr(MSR_K7_EVNTSEL0, 0, 0);
@@ -160,46 +161,66 @@
}
break;
}
+ nmi_watchdog = NMI_NONE;
+ nmi_active = -1;
}
-static int nmi_pm_callback(struct pm_dev *dev, pm_request_t rqst, void *data)
+void enable_local_apic_nmi_watchdog(void)
{
- switch (rqst) {
- case PM_SUSPEND:
- disable_apic_nmi_watchdog();
- break;
- case PM_RESUME:
+ if (nmi_active < 0) {
+ nmi_watchdog = NMI_LOCAL_APIC;
setup_apic_nmi_watchdog();
- break;
}
- return 0;
}
-struct pm_dev * set_nmi_pm_callback(pm_callback callback)
-{
- apic_pm_unregister(nmi_pmdev);
- return apic_pm_register(PM_SYS_DEV, 0, callback);
-}
+#ifdef CONFIG_PM
+
+#include <linux/device.h>
-void unset_nmi_pm_callback(struct pm_dev * dev)
+static int nmi_suspend(struct device *dev, u32 state, u32 level)
{
- apic_pm_unregister(dev);
- nmi_pmdev = apic_pm_register(PM_SYS_DEV, 0, nmi_pm_callback);
+ if (level != SUSPEND_DISABLE)
+ return 0;
+ disable_local_apic_nmi_watchdog();
+ return 0;
}
-
-static void nmi_pm_init(void)
+
+static int nmi_resume(struct device *dev, u32 level)
{
- if (!nmi_pmdev)
- nmi_pmdev = apic_pm_register(PM_SYS_DEV, 0, nmi_pm_callback);
+ if (level != RESUME_POWER_ON)
+ return 0;
+ enable_local_apic_nmi_watchdog();
+ return 0;
}
-#define __pminit /*empty*/
-
-#else /* CONFIG_PM */
+static struct device_driver local_apic_nmi_driver = {
+ .name = "local_apic_nmi",
+ .bus = &system_bus_type,
+ .resume = nmi_resume,
+ .suspend = nmi_suspend,
+};
+
+extern struct sys_device device_local_apic;
+
+static struct sys_device device_local_apic_nmi = {
+ .name = "local_apic_nmi",
+ .id = 0,
+ .dev = {
+ .name = "local_apic_nmi",
+ .driver = &local_apic_nmi_driver,
+ .parent = &device_local_apic.dev,
+ },
+};
-static inline void nmi_pm_init(void) { }
-
-#define __pminit __init
+static int __init init_nmi_devicefs(void)
+{
+ if (nmi_active == 0)
+ return 0;
+ driver_register(&local_apic_nmi_driver);
+ return sys_device_register(&device_local_apic_nmi);
+}
+/* must come after the local APIC's device_initcall() */
+late_initcall(init_nmi_devicefs);
#endif /* CONFIG_PM */
@@ -208,7 +229,7 @@
* Original code written by Keith Owens.
*/
-static void __pminit clear_msr_range(unsigned int base, unsigned int n)
+static void clear_msr_range(unsigned int base, unsigned int n)
{
unsigned int i;
@@ -216,7 +237,7 @@
wrmsr(base+i, 0, 0);
}
-static void __pminit setup_k7_watchdog(void)
+static void setup_k7_watchdog(void)
{
unsigned int evntsel;
@@ -238,7 +259,7 @@
wrmsr(MSR_K7_EVNTSEL0, evntsel, 0);
}
-static void __pminit setup_p6_watchdog(void)
+static void setup_p6_watchdog(void)
{
unsigned int evntsel;
@@ -260,7 +281,7 @@
wrmsr(MSR_P6_EVNTSEL0, evntsel, 0);
}
-static int __pminit setup_p4_watchdog(void)
+static int setup_p4_watchdog(void)
{
unsigned int misc_enable, dummy;
@@ -290,7 +311,7 @@
return 1;
}
-void __pminit setup_apic_nmi_watchdog (void)
+void setup_apic_nmi_watchdog (void)
{
switch (boot_cpu_data.x86_vendor) {
case X86_VENDOR_AMD:
@@ -314,7 +335,7 @@
default:
return;
}
- nmi_pm_init();
+ nmi_active = 1;
}
static spinlock_t nmi_print_lock = SPIN_LOCK_UNLOCKED;
@@ -402,3 +423,7 @@
wrmsr(nmi_perfctr_msr, -(cpu_khz/nmi_hz*1000), -1);
}
}
+
+EXPORT_SYMBOL(nmi_watchdog);
+EXPORT_SYMBOL(disable_local_apic_nmi_watchdog);
+EXPORT_SYMBOL(enable_local_apic_nmi_watchdog);
--- linux-2.5.60/arch/i386/oprofile/nmi_int.c.~1~ 2003-01-02 14:27:54.000000000 +0100
+++ linux-2.5.60/arch/i386/oprofile/nmi_int.c 2003-02-12 21:01:51.000000000 +0100
@@ -11,6 +11,7 @@
#include <linux/notifier.h>
#include <linux/smp.h>
#include <linux/oprofile.h>
+#include <linux/device.h>
#include <asm/nmi.h>
#include <asm/msr.h>
#include <asm/apic.h>
@@ -24,27 +25,6 @@
static int nmi_start(void);
static void nmi_stop(void);
-
-static struct pm_dev * oprofile_pmdev;
-
-/* We're at risk of causing big trouble unless we
- * make sure to not cause any NMI interrupts when
- * suspended.
- */
-static int oprofile_pm_callback(struct pm_dev * dev,
- pm_request_t rqst, void * data)
-{
- switch (rqst) {
- case PM_SUSPEND:
- nmi_stop();
- break;
- case PM_RESUME:
- nmi_start();
- break;
- }
- return 0;
-}
-
static int nmi_callback(struct pt_regs * regs, int cpu)
{
@@ -86,7 +66,42 @@
saved_lvtpc[cpu] = apic_read(APIC_LVTPC);
apic_write(APIC_LVTPC, APIC_DM_NMI);
}
-
+
+static int nmi_enabled = 0; /* 0 == registered but off, 1 == registered and on */
+
+static int nmi_suspend(struct device *dev, u32 state, u32 level)
+{
+ if (level != SUSPEND_DISABLE)
+ return 0;
+ if (nmi_enabled == 1)
+ nmi_stop();
+ return 0;
+}
+
+static int nmi_resume(struct device *dev, u32 level)
+{
+ if (level != RESUME_POWER_ON)
+ return 0;
+ if (nmi_enabled == 1)
+ nmi_start();
+ return 0;
+}
+
+
+static struct device_driver nmi_driver = {
+ .name = "oprofile",
+ .bus = &system_bus_type,
+ .resume = nmi_resume,
+ .suspend = nmi_suspend,
+};
+
+static struct device device_nmi = {
+ .name = "oprofile",
+ .bus_id = "oprofile",
+ .driver = &nmi_driver,
+};
+
+extern struct sys_device device_apic;
static int nmi_setup(void)
{
@@ -95,13 +110,25 @@
* without actually triggering any NMIs as this will
* break the core code horrifically.
*/
+ if (nmi_watchdog == NMI_LOCAL_APIC) {
+ disable_apic_nmi_watchdog();
+ nmi_watchdog = NMI_LOCAL_APIC_SUSPENDED_BY_OPROFILE;
+ }
smp_call_function(nmi_cpu_setup, NULL, 0, 1);
nmi_cpu_setup(0);
set_nmi_callback(nmi_callback);
- oprofile_pmdev = set_nmi_pm_callback(oprofile_pm_callback);
- return 0;
+ nmi_enabled = 1;
+ return 0;
}
+static int __init init_nmi_driverfs(void)
+{
+ driver_register(&nmi_driver);
+ device_nmi.parent = &device_apic.dev;
+ device_register(&device_nmi);
+}
+
+late_initcall(init_nmi_driverfs);
static void nmi_restore_registers(struct op_msrs * msrs)
{
@@ -146,10 +173,14 @@
static void nmi_shutdown(void)
{
- unset_nmi_pm_callback(oprofile_pmdev);
+ nmi_enabled = 0;
unset_nmi_callback();
smp_call_function(nmi_cpu_shutdown, NULL, 0, 1);
nmi_cpu_shutdown(0);
+ if (nmi_watchdog == NMI_LOCAL_APIC_SUSPENDED_BY_OPROFILE) {
+ nmi_watchdog = NMI_LOCAL_APIC;
+ setup_apic_nmi_watchdog();
+ }
}
--- linux-2.5.60/include/asm-i386/apic.h.~1~ 2002-10-20 18:51:08.000000000 +0200
+++ linux-2.5.60/include/asm-i386/apic.h 2003-02-12 21:01:51.000000000 +0100
@@ -84,8 +84,8 @@
extern void disable_APIC_timer(void);
extern void enable_APIC_timer(void);
-extern struct pm_dev *apic_pm_register(pm_dev_t, unsigned long, pm_callback);
-extern void apic_pm_unregister(struct pm_dev*);
+extern void disable_local_apic_nmi_watchdog(void);
+extern void enable_local_apic_nmi_watchdog(void);
extern int check_nmi_watchdog (void);
extern void enable_NMI_through_LVT0 (void * dummy);
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: Switch APIC (+nmi, +oprofile) to driver model
2003-02-11 11:12 ` John Levon
@ 2003-02-11 12:01 ` Pavel Machek
2003-02-13 13:39 ` Mikael Pettersson
0 siblings, 1 reply; 27+ messages in thread
From: Pavel Machek @ 2003-02-11 12:01 UTC (permalink / raw)
To: John Levon; +Cc: linux-kernel
Hi!
> > > > Yes, whole oprofile/nmi interaction is ugly like hell. This way it is
> > > > at least explicit, so people *know* its ugly.
> > >
> > > That's no reason not do something like Mikael or I suggested.
> >
> > makes what happens there pretty explicit. Whole thing can be
> > controlled by single variable...
> >
> > Here's new version of diff...
>
> Are you going to continue ignoring me when I point out the bugs in your
> patch ? You're still not exporting nmi_watchdog,setup/disable_watchdog
> to modules.
I try not to ignore comments, but when there's too much of them... :-(
[I hope it is okay to EXPORT_SYMBOL() from normal .c file?]
Updated patch:
Pavel
--- clean/arch/i386/kernel/apic.c 2003-01-05 22:58:18.000000000 +0100
+++ linux/arch/i386/kernel/apic.c 2003-02-11 12:47:55.000000000 +0100
@@ -10,6 +10,7 @@
* for testing these extensively.
* Maciej W. Rozycki : Various updates and fixes.
* Mikael Pettersson : Power Management for UP-APIC.
+ * Pavel Machek : Converted to driver model.
*/
#include <linux/config.h>
@@ -23,6 +24,10 @@
#include <linux/interrupt.h>
#include <linux/mc146818rtc.h>
#include <linux/kernel_stat.h>
+#include <linux/delay.h>
+#include <linux/slab.h>
+#include <linux/device.h>
+#include <linux/pm.h>
#include <asm/atomic.h>
#include <asm/smp.h>
@@ -292,6 +297,27 @@
apic_write_around(APIC_LVT1, value);
}
+static struct {
+ /* 'active' is true if the local APIC was enabled by us and
+ not the BIOS; this signifies that we are also responsible
+ for disabling it before entering apm/acpi suspend */
+ int active;
+ /* r/w apic fields */
+ unsigned int apic_id;
+ unsigned int apic_taskpri;
+ unsigned int apic_ldr;
+ unsigned int apic_dfr;
+ unsigned int apic_spiv;
+ unsigned int apic_lvtt;
+ unsigned int apic_lvtpc;
+ unsigned int apic_lvt0;
+ unsigned int apic_lvt1;
+ unsigned int apic_lvterr;
+ unsigned int apic_tmict;
+ unsigned int apic_tdcr;
+ unsigned int apic_thmr;
+} apic_pm_state;
+
void __init setup_local_APIC (void)
{
unsigned long value, ver, maxlvt;
@@ -435,44 +461,21 @@
if (nmi_watchdog == NMI_LOCAL_APIC)
setup_apic_nmi_watchdog();
+
+ apic_pm_state.active = 1;
}
#ifdef CONFIG_PM
-
-#include <linux/slab.h>
-#include <linux/pm.h>
-
-static struct {
- /* 'active' is true if the local APIC was enabled by us and
- not the BIOS; this signifies that we are also responsible
- for disabling it before entering apm/acpi suspend */
- int active;
- /* 'perfctr_pmdev' is here because the current (2.4.1) PM
- callback system doesn't handle hierarchical dependencies */
- struct pm_dev *perfctr_pmdev;
- /* r/w apic fields */
- unsigned int apic_id;
- unsigned int apic_taskpri;
- unsigned int apic_ldr;
- unsigned int apic_dfr;
- unsigned int apic_spiv;
- unsigned int apic_lvtt;
- unsigned int apic_lvtpc;
- unsigned int apic_lvt0;
- unsigned int apic_lvt1;
- unsigned int apic_lvterr;
- unsigned int apic_tmict;
- unsigned int apic_tdcr;
- unsigned int apic_thmr;
-} apic_pm_state;
-
-static void apic_pm_suspend(void *data)
+static int apic_suspend(struct device *dev, u32 state, u32 level)
{
unsigned int l, h;
unsigned long flags;
- if (apic_pm_state.perfctr_pmdev)
- pm_send(apic_pm_state.perfctr_pmdev, PM_SUSPEND, data);
+ if (level != SUSPEND_DISABLE)
+ return 0;
+ if (!apic_pm_state.active)
+ return 0;
+
apic_pm_state.apic_id = apic_read(APIC_ID);
apic_pm_state.apic_taskpri = apic_read(APIC_TASKPRI);
apic_pm_state.apic_ldr = apic_read(APIC_LDR);
@@ -493,13 +496,21 @@
l &= ~MSR_IA32_APICBASE_ENABLE;
wrmsr(MSR_IA32_APICBASE, l, h);
local_irq_restore(flags);
+ return 0;
}
-static void apic_pm_resume(void *data)
+static int apic_resume(struct device *dev, u32 level)
{
unsigned int l, h;
unsigned long flags;
+ if (level != RESUME_POWER_ON)
+ return 0;
+ if (!apic_pm_state.active)
+ return 0;
+
+ set_fixmap_nocache(FIX_APIC_BASE, APIC_DEFAULT_PHYS_BASE); /* FIXME: this is needed for S3 resume, but why? */
+
local_irq_save(flags);
rdmsr(MSR_IA32_APICBASE, l, h);
l &= ~MSR_IA32_APICBASE_BASE;
@@ -524,74 +535,34 @@
apic_write(APIC_ESR, 0);
apic_read(APIC_ESR);
local_irq_restore(flags);
- if (apic_pm_state.perfctr_pmdev)
- pm_send(apic_pm_state.perfctr_pmdev, PM_RESUME, data);
-}
-
-static int apic_pm_callback(struct pm_dev *dev, pm_request_t rqst, void *data)
-{
- switch (rqst) {
- case PM_SUSPEND:
- apic_pm_suspend(data);
- break;
- case PM_RESUME:
- apic_pm_resume(data);
- break;
- }
return 0;
}
-/* perfctr driver should call this instead of pm_register() */
-struct pm_dev *apic_pm_register(pm_dev_t type,
- unsigned long id,
- pm_callback callback)
-{
- struct pm_dev *dev;
-
- if (!apic_pm_state.active)
- return pm_register(type, id, callback);
- if (apic_pm_state.perfctr_pmdev)
- return NULL; /* we're busy */
- dev = kmalloc(sizeof(struct pm_dev), GFP_KERNEL);
- if (dev) {
- memset(dev, 0, sizeof(*dev));
- dev->type = type;
- dev->id = id;
- dev->callback = callback;
- apic_pm_state.perfctr_pmdev = dev;
- }
- return dev;
-}
+static struct device_driver apic_driver = {
+ .name = "apic",
+ .bus = &system_bus_type,
+ .resume = apic_resume,
+ .suspend = apic_suspend,
+};
+
+struct sys_device device_apic = {
+ .name = "apic",
+ .id = 0,
+ .dev = {
+ .name = "APIC",
+ .driver = &apic_driver,
+ },
+};
-/* perfctr driver should call this instead of pm_unregister() */
-void apic_pm_unregister(struct pm_dev *dev)
-{
- if (!apic_pm_state.active) {
- pm_unregister(dev);
- } else if (dev == apic_pm_state.perfctr_pmdev) {
- apic_pm_state.perfctr_pmdev = NULL;
- kfree(dev);
- }
-}
-
-static void __init apic_pm_init1(void)
-{
- /* can't pm_register() at this early stage in the boot process
- (causes an immediate reboot), so just set the flag */
- apic_pm_state.active = 1;
-}
-
-static void __init apic_pm_init2(void)
+static int __init init_apic_devicefs(void)
{
+ driver_register(&apic_driver);
if (apic_pm_state.active)
- pm_register(PM_SYS_DEV, 0, apic_pm_callback);
+ return sys_device_register(&device_apic);
+ return 0;
}
-#else /* CONFIG_PM */
-
-static inline void apic_pm_init1(void) { }
-static inline void apic_pm_init2(void) { }
-
+device_initcall(init_apic_devicefs);
#endif /* CONFIG_PM */
/*
@@ -658,9 +629,6 @@
nmi_watchdog = NMI_LOCAL_APIC;
printk("Found and enabled local APIC!\n");
-
- apic_pm_init1();
-
return 0;
no_apic:
@@ -671,7 +639,6 @@
void __init init_apic_mappings(void)
{
unsigned long apic_phys;
-
/*
* If no local APIC can be found then set up a fake all
* zeroes page to simulate the local APIC and another
@@ -1141,8 +1108,6 @@
phys_cpu_present_map = 1;
apic_write_around(APIC_ID, boot_cpu_physical_apicid);
- apic_pm_init2();
-
setup_local_APIC();
if (nmi_watchdog == NMI_LOCAL_APIC)
--- clean/arch/i386/kernel/apm.c 2003-01-09 22:16:11.000000000 +0100
+++ linux/arch/i386/kernel/apm.c 2003-02-10 18:16:59.000000000 +0100
@@ -1263,6 +1263,11 @@
}
printk(KERN_CRIT "apm: suspend was vetoed, but suspending anyway.\n");
}
+
+ device_suspend(3, SUSPEND_NOTIFY);
+ device_suspend(3, SUSPEND_SAVE_STATE);
+ device_suspend(3, SUSPEND_DISABLE);
+
/* serialize with the timer interrupt */
write_lock_irq(&xtime_lock);
@@ -1283,6 +1288,8 @@
if (err != APM_SUCCESS)
apm_error("suspend", err);
err = (err == APM_SUCCESS) ? 0 : -EIO;
+ device_resume(RESUME_RESTORE_STATE);
+ device_resume(RESUME_ENABLE);
pm_send_all(PM_RESUME, (void *)0);
queue_event(APM_NORMAL_RESUME, NULL);
out:
@@ -1396,6 +1403,8 @@
write_lock_irq(&xtime_lock);
set_time();
write_unlock_irq(&xtime_lock);
+ device_resume(RESUME_RESTORE_STATE);
+ device_resume(RESUME_ENABLE);
pm_send_all(PM_RESUME, (void *)0);
queue_event(event, NULL);
}
--- clean/arch/i386/kernel/i386_ksyms.c 2003-01-17 23:09:51.000000000 +0100
+++ linux/arch/i386/kernel/i386_ksyms.c 2003-02-11 12:40:13.000000000 +0100
@@ -161,10 +161,6 @@
EXPORT_SYMBOL(flush_tlb_page);
#endif
-#if defined(CONFIG_X86_LOCAL_APIC) && defined(CONFIG_PM)
-EXPORT_SYMBOL_GPL(set_nmi_pm_callback);
-EXPORT_SYMBOL_GPL(unset_nmi_pm_callback);
-#endif
#ifdef CONFIG_X86_IO_APIC
EXPORT_SYMBOL(IO_APIC_get_PCI_irq_vector);
#endif
--- clean/arch/i386/kernel/nmi.c 2003-01-05 22:58:19.000000000 +0100
+++ linux/arch/i386/kernel/nmi.c 2003-02-11 12:42:14.000000000 +0100
@@ -9,6 +9,7 @@
* Mikael Pettersson : AMD K7 support for local APIC NMI watchdog.
* Mikael Pettersson : Power Management for local APIC NMI watchdog.
* Mikael Pettersson : Pentium 4 support for local APIC NMI watchdog.
+ * Pavel Machek : Driver model here, too.
*/
#include <linux/config.h>
@@ -20,6 +21,8 @@
#include <linux/interrupt.h>
#include <linux/mc146818rtc.h>
#include <linux/kernel_stat.h>
+#include <linux/device.h>
+#include <linux/module.h>
#include <asm/smp.h>
#include <asm/mtrr.h>
@@ -29,6 +32,7 @@
static unsigned int nmi_hz = HZ;
unsigned int nmi_perfctr_msr; /* the MSR to reset in NMI handler */
extern void show_registers(struct pt_regs *regs);
+static int nmi_active;
#define K7_EVNTSEL_ENABLE (1 << 22)
#define K7_EVNTSEL_INT (1 << 20)
@@ -119,7 +123,7 @@
*/
if ((nmi == NMI_LOCAL_APIC) &&
(boot_cpu_data.x86_vendor == X86_VENDOR_INTEL) &&
- (boot_cpu_data.x86 == 6 || boot_cpu_data.x86 == 15))
+ (boot_cpu_data.x86 == 6 || boot_cpu_data.x86 == 15))
nmi_watchdog = nmi;
if ((nmi == NMI_LOCAL_APIC) &&
(boot_cpu_data.x86_vendor == X86_VENDOR_AMD) &&
@@ -136,14 +140,11 @@
__setup("nmi_watchdog=", setup_nmi_watchdog);
-#ifdef CONFIG_PM
-
-#include <linux/pm.h>
-
-struct pm_dev *nmi_pmdev;
-static void disable_apic_nmi_watchdog(void)
+void disable_apic_nmi_watchdog(void)
{
+ if (!nmi_active)
+ return;
switch (boot_cpu_data.x86_vendor) {
case X86_VENDOR_AMD:
wrmsr(MSR_K7_EVNTSEL0, 0, 0);
@@ -160,46 +161,56 @@
}
break;
}
+ nmi_active = 0;
}
-static int nmi_pm_callback(struct pm_dev *dev, pm_request_t rqst, void *data)
+#ifdef CONFIG_PM
+
+static int nmi_suspend(struct device *dev, u32 state, u32 level)
{
- switch (rqst) {
- case PM_SUSPEND:
- disable_apic_nmi_watchdog();
- break;
- case PM_RESUME:
- setup_apic_nmi_watchdog();
- break;
- }
+ if (level != SUSPEND_DISABLE)
+ return 0;
+
+ disable_apic_nmi_watchdog();
return 0;
}
-struct pm_dev * set_nmi_pm_callback(pm_callback callback)
+static int nmi_resume(struct device *dev, u32 level)
{
- apic_pm_unregister(nmi_pmdev);
- return apic_pm_register(PM_SYS_DEV, 0, callback);
-}
+ if (level != RESUME_POWER_ON)
+ return 0;
-void unset_nmi_pm_callback(struct pm_dev * dev)
-{
- apic_pm_unregister(dev);
- nmi_pmdev = apic_pm_register(PM_SYS_DEV, 0, nmi_pm_callback);
-}
-
-static void nmi_pm_init(void)
-{
- if (!nmi_pmdev)
- nmi_pmdev = apic_pm_register(PM_SYS_DEV, 0, nmi_pm_callback);
+ if (nmi_watchdog == NMI_LOCAL_APIC)
+ setup_apic_nmi_watchdog();
+
+ return 0;
}
-#define __pminit /*empty*/
-#else /* CONFIG_PM */
+static struct device_driver nmi_driver = {
+ .name = "nmi",
+ .bus = &system_bus_type,
+ .resume = nmi_resume,
+ .suspend = nmi_suspend,
+};
+
+static struct device device_nmi = {
+ .name = "NMI",
+ .bus_id = "NMI",
+ .driver = &nmi_driver,
+};
-static inline void nmi_pm_init(void) { }
+extern struct sys_device device_apic;
-#define __pminit __init
+static int __init init_nmi_devicefs(void)
+{
+ driver_register(&nmi_driver);
+
+ device_nmi.parent = &device_apic.dev;
+ return device_register(&device_nmi);
+}
+
+late_initcall(init_nmi_devicefs);
#endif /* CONFIG_PM */
@@ -208,7 +219,7 @@
* Original code written by Keith Owens.
*/
-static void __pminit clear_msr_range(unsigned int base, unsigned int n)
+static void clear_msr_range(unsigned int base, unsigned int n)
{
unsigned int i;
@@ -216,7 +227,7 @@
wrmsr(base+i, 0, 0);
}
-static void __pminit setup_k7_watchdog(void)
+static void setup_k7_watchdog(void)
{
unsigned int evntsel;
@@ -238,7 +249,7 @@
wrmsr(MSR_K7_EVNTSEL0, evntsel, 0);
}
-static void __pminit setup_p6_watchdog(void)
+static void setup_p6_watchdog(void)
{
unsigned int evntsel;
@@ -260,7 +271,7 @@
wrmsr(MSR_P6_EVNTSEL0, evntsel, 0);
}
-static int __pminit setup_p4_watchdog(void)
+static int setup_p4_watchdog(void)
{
unsigned int misc_enable, dummy;
@@ -290,7 +301,7 @@
return 1;
}
-void __pminit setup_apic_nmi_watchdog (void)
+void setup_apic_nmi_watchdog (void)
{
switch (boot_cpu_data.x86_vendor) {
case X86_VENDOR_AMD:
@@ -314,7 +325,7 @@
default:
return;
}
- nmi_pm_init();
+ nmi_active = 1;
}
static spinlock_t nmi_print_lock = SPIN_LOCK_UNLOCKED;
@@ -402,3 +413,7 @@
wrmsr(nmi_perfctr_msr, -(cpu_khz/nmi_hz*1000), -1);
}
}
+
+EXPORT_SYMBOL(nmi_watchdog);
+EXPORT_SYMBOL(setup_apic_nmi_watchdog);
+EXPORT_SYMBOL(disable_apic_nmi_watchdog);
--- clean/arch/i386/oprofile/nmi_int.c 2003-01-05 22:58:19.000000000 +0100
+++ linux/arch/i386/oprofile/nmi_int.c 2003-02-10 18:17:00.000000000 +0100
@@ -11,6 +11,7 @@
#include <linux/notifier.h>
#include <linux/smp.h>
#include <linux/oprofile.h>
+#include <linux/device.h>
#include <asm/nmi.h>
#include <asm/msr.h>
#include <asm/apic.h>
@@ -24,27 +25,6 @@
static int nmi_start(void);
static void nmi_stop(void);
-
-static struct pm_dev * oprofile_pmdev;
-
-/* We're at risk of causing big trouble unless we
- * make sure to not cause any NMI interrupts when
- * suspended.
- */
-static int oprofile_pm_callback(struct pm_dev * dev,
- pm_request_t rqst, void * data)
-{
- switch (rqst) {
- case PM_SUSPEND:
- nmi_stop();
- break;
- case PM_RESUME:
- nmi_start();
- break;
- }
- return 0;
-}
-
static int nmi_callback(struct pt_regs * regs, int cpu)
{
@@ -86,7 +66,42 @@
saved_lvtpc[cpu] = apic_read(APIC_LVTPC);
apic_write(APIC_LVTPC, APIC_DM_NMI);
}
-
+
+static int nmi_enabled = 0; /* 0 == registered but off, 1 == registered and on */
+
+static int nmi_suspend(struct device *dev, u32 state, u32 level)
+{
+ if (level != SUSPEND_DISABLE)
+ return 0;
+ if (nmi_enabled == 1)
+ nmi_stop();
+ return 0;
+}
+
+static int nmi_resume(struct device *dev, u32 level)
+{
+ if (level != RESUME_POWER_ON)
+ return 0;
+ if (nmi_enabled == 1)
+ nmi_start();
+ return 0;
+}
+
+
+static struct device_driver nmi_driver = {
+ .name = "oprofile",
+ .bus = &system_bus_type,
+ .resume = nmi_resume,
+ .suspend = nmi_suspend,
+};
+
+static struct device device_nmi = {
+ .name = "oprofile",
+ .bus_id = "oprofile",
+ .driver = &nmi_driver,
+};
+
+extern struct sys_device device_apic;
static int nmi_setup(void)
{
@@ -95,13 +110,25 @@
* without actually triggering any NMIs as this will
* break the core code horrifically.
*/
+ if (nmi_watchdog == NMI_LOCAL_APIC) {
+ disable_apic_nmi_watchdog();
+ nmi_watchdog = NMI_LOCAL_APIC_SUSPENDED_BY_OPROFILE;
+ }
smp_call_function(nmi_cpu_setup, NULL, 0, 1);
nmi_cpu_setup(0);
set_nmi_callback(nmi_callback);
- oprofile_pmdev = set_nmi_pm_callback(oprofile_pm_callback);
- return 0;
+ nmi_enabled = 1;
+ return 0;
}
+static int __init init_nmi_driverfs(void)
+{
+ driver_register(&nmi_driver);
+ device_nmi.parent = &device_apic.dev;
+ device_register(&device_nmi);
+}
+
+late_initcall(init_nmi_driverfs);
static void nmi_restore_registers(struct op_msrs * msrs)
{
@@ -146,10 +173,14 @@
static void nmi_shutdown(void)
{
- unset_nmi_pm_callback(oprofile_pmdev);
+ nmi_enabled = 0;
unset_nmi_callback();
smp_call_function(nmi_cpu_shutdown, NULL, 0, 1);
nmi_cpu_shutdown(0);
+ if (nmi_watchdog == NMI_LOCAL_APIC_SUSPENDED_BY_OPROFILE) {
+ nmi_watchdog = NMI_LOCAL_APIC;
+ setup_apic_nmi_watchdog();
+ }
}
--- clean/include/asm-i386/apic.h 2002-10-20 16:22:45.000000000 +0200
+++ linux/include/asm-i386/apic.h 2003-02-10 18:17:01.000000000 +0100
@@ -95,6 +95,7 @@
#define NMI_IO_APIC 1
#define NMI_LOCAL_APIC 2
#define NMI_INVALID 3
+#define NMI_LOCAL_APIC_SUSPENDED_BY_OPROFILE 4
#endif /* CONFIG_X86_LOCAL_APIC */
--
When do you have a heart between your knees?
[Johanka's followup: and *two* hearts?]
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: Switch APIC (+nmi, +oprofile) to driver model
2003-02-10 19:05 Mikael Pettersson
2003-02-11 11:39 ` Pavel Machek
@ 2003-02-11 11:53 ` Pavel Machek
1 sibling, 0 replies; 27+ messages in thread
From: Pavel Machek @ 2003-02-11 11:53 UTC (permalink / raw)
To: Mikael Pettersson; +Cc: levon, linux-kernel
Hi!
> >So it seems to me it really is variable.
>
> The original code has the property that apic_pm_state.active is
> true if and only if detect_init_APIC() was called and succeeded,
> which implies that apic_phys == mp_lapic_addr == APIC_DEFAULT_PHYS_BASE.
> You can also see that apic_pm_resume() writes APIC_DEFAULT_PHYS_BASE
> to MSR_IA32_APICBASE, which only makes sense in this situation.
Ahha, sorry about previous comment. Yep, you are right. I wonder if I
should BUG_ON() there, but I just cleaned it up for now.
Pavel
--
When do you have a heart between your knees?
[Johanka's followup: and *two* hearts?]
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: Switch APIC (+nmi, +oprofile) to driver model
2003-02-10 16:10 ` Pavel Machek
@ 2003-02-11 11:43 ` Mikael Pettersson
0 siblings, 0 replies; 27+ messages in thread
From: Mikael Pettersson @ 2003-02-11 11:43 UTC (permalink / raw)
To: Pavel Machek; +Cc: levon, linux-kernel
Pavel Machek writes:
> cpu_has_apic test seems wrong: AFAICS, there are CPUs that have apic
> but linux does not know how to use it.
Please give a reference. I don't know of any x86-type CPU that
fits your description above.
/Mikael
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: Switch APIC (+nmi, +oprofile) to driver model
2003-02-10 19:05 Mikael Pettersson
@ 2003-02-11 11:39 ` Pavel Machek
2003-02-11 11:53 ` Pavel Machek
1 sibling, 0 replies; 27+ messages in thread
From: Pavel Machek @ 2003-02-11 11:39 UTC (permalink / raw)
To: Mikael Pettersson; +Cc: levon, linux-kernel
Hi!
> >> Also, apic_phys is (or should be) APIC_DEFAULT_PHYS_BASE, so
> >> you shouldn't need to make apic_phys global.
> >
> >Really?
> >
> > /*
> > * If no local APIC can be found then set up a fake all
> > * zeroes page to simulate the local APIC and another
> > * one for the IO-APIC.
> > */
> > if (!smp_found_config && detect_init_APIC()) {
> > apic_phys = (unsigned long) alloc_bootmem_pages(PAGE_SIZE);
> > apic_phys = __pa(apic_phys);
> > } else
> > apic_phys = mp_lapic_addr;
> >
> > set_fixmap_nocache(FIX_APIC_BASE, apic_phys);
> >
> >So it seems to me it really is variable.
>
> The original code has the property that apic_pm_state.active is
> true if and only if detect_init_APIC() was called and succeeded,
> which implies that apic_phys == mp_lapic_addr == APIC_DEFAULT_PHYS_BASE.
> You can also see that apic_pm_resume() writes APIC_DEFAULT_PHYS_BASE
> to MSR_IA32_APICBASE, which only makes sense in this situation.
Okay, I guess I do not rely on so complicated invariants...
> You moved the apic_pm_state.active = 1 assignment from detect_init_APIC(),
> which is specific to UP_APIC, to setup_local_APIC(), which is called
> also in the SMP case. Do you intend to do suspend and resume on SMP boxes
> as well? If this is intensional, shouldn't device_apic be per-cpu?
Eventually, I do want suspend working on SMP machines, but that's
still quite a long way to go: plan is to hot-unplug all but boot CPUs,
then do suspend, then hot-plug them back.
Pavel
--
When do you have a heart between your knees?
[Johanka's followup: and *two* hearts?]
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: Switch APIC (+nmi, +oprofile) to driver model
2003-02-10 20:06 ` Pavel Machek
@ 2003-02-11 11:12 ` John Levon
2003-02-11 12:01 ` Pavel Machek
0 siblings, 1 reply; 27+ messages in thread
From: John Levon @ 2003-02-11 11:12 UTC (permalink / raw)
To: linux-kernel; +Cc: pavel
On Mon, Feb 10, 2003 at 09:06:06PM +0100, Pavel Machek wrote:
> > > Yes, whole oprofile/nmi interaction is ugly like hell. This way it is
> > > at least explicit, so people *know* its ugly.
> >
> > That's no reason not do something like Mikael or I suggested.
>
> makes what happens there pretty explicit. Whole thing can be
> controlled by single variable...
>
> Here's new version of diff...
Are you going to continue ignoring me when I point out the bugs in your
patch ? You're still not exporting nmi_watchdog,setup/disable_watchdog
to modules.
Of course, exporting three things when you only need two is ugly, but
since you refuse to do it like that ...
john
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: Switch APIC (+nmi, +oprofile) to driver model
2003-02-10 11:50 ` John Levon
@ 2003-02-10 20:06 ` Pavel Machek
2003-02-11 11:12 ` John Levon
0 siblings, 1 reply; 27+ messages in thread
From: Pavel Machek @ 2003-02-10 20:06 UTC (permalink / raw)
To: John Levon; +Cc: linux-kernel
Hi!
> > Yes, whole oprofile/nmi interaction is ugly like hell. This way it is
> > at least explicit, so people *know* its ugly.
>
> That's no reason not do something like Mikael or I suggested.
Well, I believe that having
extern unsigned int nmi_watchdog;
#define NMI_NONE 0
#define NMI_IO_APIC 1
#define NMI_LOCAL_APIC 2
#define NMI_INVALID 3
#define NMI_LOCAL_APIC_SUSPENDED_BY_OPROFILE 4
makes what happens there pretty explicit. Whole thing can be
controlled by single variable...
Here's new version of diff...
Pavel
--- clean/arch/i386/kernel/apic.c 2003-01-05 22:58:18.000000000 +0100
+++ linux-swsusp/arch/i386/kernel/apic.c 2003-02-10 17:03:51.000000000 +0100
@@ -10,6 +10,7 @@
* for testing these extensively.
* Maciej W. Rozycki : Various updates and fixes.
* Mikael Pettersson : Power Management for UP-APIC.
+ * Pavel Machek : Converted to driver model.
*/
#include <linux/config.h>
@@ -23,6 +24,10 @@
#include <linux/interrupt.h>
#include <linux/mc146818rtc.h>
#include <linux/kernel_stat.h>
+#include <linux/delay.h>
+#include <linux/slab.h>
+#include <linux/device.h>
+#include <linux/pm.h>
#include <asm/atomic.h>
#include <asm/smp.h>
@@ -54,6 +59,8 @@
int prof_old_multiplier[NR_CPUS] = { 1, };
int prof_counter[NR_CPUS] = { 1, };
+unsigned long apic_phys;
+
int get_maxlvt(void)
{
unsigned int v, ver, maxlvt;
@@ -292,6 +299,27 @@
apic_write_around(APIC_LVT1, value);
}
+static struct {
+ /* 'active' is true if the local APIC was enabled by us and
+ not the BIOS; this signifies that we are also responsible
+ for disabling it before entering apm/acpi suspend */
+ int active;
+ /* r/w apic fields */
+ unsigned int apic_id;
+ unsigned int apic_taskpri;
+ unsigned int apic_ldr;
+ unsigned int apic_dfr;
+ unsigned int apic_spiv;
+ unsigned int apic_lvtt;
+ unsigned int apic_lvtpc;
+ unsigned int apic_lvt0;
+ unsigned int apic_lvt1;
+ unsigned int apic_lvterr;
+ unsigned int apic_tmict;
+ unsigned int apic_tdcr;
+ unsigned int apic_thmr;
+} apic_pm_state;
+
void __init setup_local_APIC (void)
{
unsigned long value, ver, maxlvt;
@@ -435,44 +463,21 @@
if (nmi_watchdog == NMI_LOCAL_APIC)
setup_apic_nmi_watchdog();
+
+ apic_pm_state.active = 1;
}
#ifdef CONFIG_PM
-
-#include <linux/slab.h>
-#include <linux/pm.h>
-
-static struct {
- /* 'active' is true if the local APIC was enabled by us and
- not the BIOS; this signifies that we are also responsible
- for disabling it before entering apm/acpi suspend */
- int active;
- /* 'perfctr_pmdev' is here because the current (2.4.1) PM
- callback system doesn't handle hierarchical dependencies */
- struct pm_dev *perfctr_pmdev;
- /* r/w apic fields */
- unsigned int apic_id;
- unsigned int apic_taskpri;
- unsigned int apic_ldr;
- unsigned int apic_dfr;
- unsigned int apic_spiv;
- unsigned int apic_lvtt;
- unsigned int apic_lvtpc;
- unsigned int apic_lvt0;
- unsigned int apic_lvt1;
- unsigned int apic_lvterr;
- unsigned int apic_tmict;
- unsigned int apic_tdcr;
- unsigned int apic_thmr;
-} apic_pm_state;
-
-static void apic_pm_suspend(void *data)
+static int apic_suspend(struct device *dev, u32 state, u32 level)
{
unsigned int l, h;
unsigned long flags;
- if (apic_pm_state.perfctr_pmdev)
- pm_send(apic_pm_state.perfctr_pmdev, PM_SUSPEND, data);
+ if (level != SUSPEND_DISABLE)
+ return 0;
+ if (!apic_pm_state.active)
+ return 0;
+
apic_pm_state.apic_id = apic_read(APIC_ID);
apic_pm_state.apic_taskpri = apic_read(APIC_TASKPRI);
apic_pm_state.apic_ldr = apic_read(APIC_LDR);
@@ -493,13 +498,21 @@
l &= ~MSR_IA32_APICBASE_ENABLE;
wrmsr(MSR_IA32_APICBASE, l, h);
local_irq_restore(flags);
+ return 0;
}
-static void apic_pm_resume(void *data)
+static int apic_resume(struct device *dev, u32 level)
{
unsigned int l, h;
unsigned long flags;
+ if (level != RESUME_POWER_ON)
+ return 0;
+ if (!apic_pm_state.active)
+ return 0;
+
+ set_fixmap_nocache(FIX_APIC_BASE, apic_phys); /* FIXME: this is needed for S3 resume, but why? */
+
local_irq_save(flags);
rdmsr(MSR_IA32_APICBASE, l, h);
l &= ~MSR_IA32_APICBASE_BASE;
@@ -524,74 +537,34 @@
apic_write(APIC_ESR, 0);
apic_read(APIC_ESR);
local_irq_restore(flags);
- if (apic_pm_state.perfctr_pmdev)
- pm_send(apic_pm_state.perfctr_pmdev, PM_RESUME, data);
-}
-
-static int apic_pm_callback(struct pm_dev *dev, pm_request_t rqst, void *data)
-{
- switch (rqst) {
- case PM_SUSPEND:
- apic_pm_suspend(data);
- break;
- case PM_RESUME:
- apic_pm_resume(data);
- break;
- }
return 0;
}
-/* perfctr driver should call this instead of pm_register() */
-struct pm_dev *apic_pm_register(pm_dev_t type,
- unsigned long id,
- pm_callback callback)
-{
- struct pm_dev *dev;
-
- if (!apic_pm_state.active)
- return pm_register(type, id, callback);
- if (apic_pm_state.perfctr_pmdev)
- return NULL; /* we're busy */
- dev = kmalloc(sizeof(struct pm_dev), GFP_KERNEL);
- if (dev) {
- memset(dev, 0, sizeof(*dev));
- dev->type = type;
- dev->id = id;
- dev->callback = callback;
- apic_pm_state.perfctr_pmdev = dev;
- }
- return dev;
-}
+static struct device_driver apic_driver = {
+ .name = "apic",
+ .bus = &system_bus_type,
+ .resume = apic_resume,
+ .suspend = apic_suspend,
+};
+
+struct sys_device device_apic = {
+ .name = "apic",
+ .id = 0,
+ .dev = {
+ .name = "APIC",
+ .driver = &apic_driver,
+ },
+};
-/* perfctr driver should call this instead of pm_unregister() */
-void apic_pm_unregister(struct pm_dev *dev)
-{
- if (!apic_pm_state.active) {
- pm_unregister(dev);
- } else if (dev == apic_pm_state.perfctr_pmdev) {
- apic_pm_state.perfctr_pmdev = NULL;
- kfree(dev);
- }
-}
-
-static void __init apic_pm_init1(void)
-{
- /* can't pm_register() at this early stage in the boot process
- (causes an immediate reboot), so just set the flag */
- apic_pm_state.active = 1;
-}
-
-static void __init apic_pm_init2(void)
+static int __init init_apic_devicefs(void)
{
+ driver_register(&apic_driver);
if (apic_pm_state.active)
- pm_register(PM_SYS_DEV, 0, apic_pm_callback);
+ return sys_device_register(&device_apic);
+ return 0;
}
-#else /* CONFIG_PM */
-
-static inline void apic_pm_init1(void) { }
-static inline void apic_pm_init2(void) { }
-
+device_initcall(init_apic_devicefs);
#endif /* CONFIG_PM */
/*
@@ -658,9 +631,6 @@
nmi_watchdog = NMI_LOCAL_APIC;
printk("Found and enabled local APIC!\n");
-
- apic_pm_init1();
-
return 0;
no_apic:
@@ -670,8 +640,6 @@
void __init init_apic_mappings(void)
{
- unsigned long apic_phys;
-
/*
* If no local APIC can be found then set up a fake all
* zeroes page to simulate the local APIC and another
@@ -1141,8 +1109,6 @@
phys_cpu_present_map = 1;
apic_write_around(APIC_ID, boot_cpu_physical_apicid);
- apic_pm_init2();
-
setup_local_APIC();
if (nmi_watchdog == NMI_LOCAL_APIC)
--- clean/arch/i386/kernel/apm.c 2003-01-09 22:16:11.000000000 +0100
+++ linux-swsusp/arch/i386/kernel/apm.c 2003-01-28 10:35:51.000000000 +0100
@@ -1263,6 +1263,11 @@
}
printk(KERN_CRIT "apm: suspend was vetoed, but suspending anyway.\n");
}
+
+ device_suspend(3, SUSPEND_NOTIFY);
+ device_suspend(3, SUSPEND_SAVE_STATE);
+ device_suspend(3, SUSPEND_DISABLE);
+
/* serialize with the timer interrupt */
write_lock_irq(&xtime_lock);
@@ -1283,6 +1288,8 @@
if (err != APM_SUCCESS)
apm_error("suspend", err);
err = (err == APM_SUCCESS) ? 0 : -EIO;
+ device_resume(RESUME_RESTORE_STATE);
+ device_resume(RESUME_ENABLE);
pm_send_all(PM_RESUME, (void *)0);
queue_event(APM_NORMAL_RESUME, NULL);
out:
@@ -1396,6 +1403,8 @@
write_lock_irq(&xtime_lock);
set_time();
write_unlock_irq(&xtime_lock);
+ device_resume(RESUME_RESTORE_STATE);
+ device_resume(RESUME_ENABLE);
pm_send_all(PM_RESUME, (void *)0);
queue_event(event, NULL);
}
--- clean/arch/i386/kernel/i386_ksyms.c 2003-01-17 23:09:51.000000000 +0100
+++ linux-swsusp/arch/i386/kernel/i386_ksyms.c 2003-01-19 19:58:34.000000000 +0100
@@ -161,10 +161,6 @@
EXPORT_SYMBOL(flush_tlb_page);
#endif
-#if defined(CONFIG_X86_LOCAL_APIC) && defined(CONFIG_PM)
-EXPORT_SYMBOL_GPL(set_nmi_pm_callback);
-EXPORT_SYMBOL_GPL(unset_nmi_pm_callback);
-#endif
#ifdef CONFIG_X86_IO_APIC
EXPORT_SYMBOL(IO_APIC_get_PCI_irq_vector);
#endif
--- clean/arch/i386/kernel/nmi.c 2003-01-05 22:58:19.000000000 +0100
+++ linux-swsusp/arch/i386/kernel/nmi.c 2003-02-10 17:09:14.000000000 +0100
@@ -9,6 +9,7 @@
* Mikael Pettersson : AMD K7 support for local APIC NMI watchdog.
* Mikael Pettersson : Power Management for local APIC NMI watchdog.
* Mikael Pettersson : Pentium 4 support for local APIC NMI watchdog.
+ * Pavel Machek : Driver model here, too.
*/
#include <linux/config.h>
@@ -20,6 +21,7 @@
#include <linux/interrupt.h>
#include <linux/mc146818rtc.h>
#include <linux/kernel_stat.h>
+#include <linux/device.h>
#include <asm/smp.h>
#include <asm/mtrr.h>
@@ -29,6 +31,7 @@
static unsigned int nmi_hz = HZ;
unsigned int nmi_perfctr_msr; /* the MSR to reset in NMI handler */
extern void show_registers(struct pt_regs *regs);
+static int nmi_active;
#define K7_EVNTSEL_ENABLE (1 << 22)
#define K7_EVNTSEL_INT (1 << 20)
@@ -119,7 +122,7 @@
*/
if ((nmi == NMI_LOCAL_APIC) &&
(boot_cpu_data.x86_vendor == X86_VENDOR_INTEL) &&
- (boot_cpu_data.x86 == 6 || boot_cpu_data.x86 == 15))
+ (boot_cpu_data.x86 == 6 || boot_cpu_data.x86 == 15))
nmi_watchdog = nmi;
if ((nmi == NMI_LOCAL_APIC) &&
(boot_cpu_data.x86_vendor == X86_VENDOR_AMD) &&
@@ -136,14 +139,11 @@
__setup("nmi_watchdog=", setup_nmi_watchdog);
-#ifdef CONFIG_PM
-
-#include <linux/pm.h>
-
-struct pm_dev *nmi_pmdev;
-static void disable_apic_nmi_watchdog(void)
+void disable_apic_nmi_watchdog(void)
{
+ if (!nmi_active)
+ return;
switch (boot_cpu_data.x86_vendor) {
case X86_VENDOR_AMD:
wrmsr(MSR_K7_EVNTSEL0, 0, 0);
@@ -160,46 +160,56 @@
}
break;
}
+ nmi_active = 0;
}
-static int nmi_pm_callback(struct pm_dev *dev, pm_request_t rqst, void *data)
+#ifdef CONFIG_PM
+
+static int nmi_suspend(struct device *dev, u32 state, u32 level)
{
- switch (rqst) {
- case PM_SUSPEND:
- disable_apic_nmi_watchdog();
- break;
- case PM_RESUME:
- setup_apic_nmi_watchdog();
- break;
- }
+ if (level != SUSPEND_DISABLE)
+ return 0;
+
+ disable_apic_nmi_watchdog();
return 0;
}
-struct pm_dev * set_nmi_pm_callback(pm_callback callback)
+static int nmi_resume(struct device *dev, u32 level)
{
- apic_pm_unregister(nmi_pmdev);
- return apic_pm_register(PM_SYS_DEV, 0, callback);
-}
+ if (level != RESUME_POWER_ON)
+ return 0;
-void unset_nmi_pm_callback(struct pm_dev * dev)
-{
- apic_pm_unregister(dev);
- nmi_pmdev = apic_pm_register(PM_SYS_DEV, 0, nmi_pm_callback);
-}
-
-static void nmi_pm_init(void)
-{
- if (!nmi_pmdev)
- nmi_pmdev = apic_pm_register(PM_SYS_DEV, 0, nmi_pm_callback);
+ if (nmi_watchdog == NMI_LOCAL_APIC)
+ setup_apic_nmi_watchdog();
+
+ return 0;
}
-#define __pminit /*empty*/
-#else /* CONFIG_PM */
+static struct device_driver nmi_driver = {
+ .name = "nmi",
+ .bus = &system_bus_type,
+ .resume = nmi_resume,
+ .suspend = nmi_suspend,
+};
+
+static struct device device_nmi = {
+ .name = "NMI",
+ .bus_id = "NMI",
+ .driver = &nmi_driver,
+};
+
+extern struct sys_device device_apic;
-static inline void nmi_pm_init(void) { }
+static int __init init_nmi_devicefs(void)
+{
+ driver_register(&nmi_driver);
+
+ device_nmi.parent = &device_apic.dev;
+ return device_register(&device_nmi);
+}
-#define __pminit __init
+late_initcall(init_nmi_devicefs);
#endif /* CONFIG_PM */
@@ -208,7 +218,7 @@
* Original code written by Keith Owens.
*/
-static void __pminit clear_msr_range(unsigned int base, unsigned int n)
+static void clear_msr_range(unsigned int base, unsigned int n)
{
unsigned int i;
@@ -216,7 +226,7 @@
wrmsr(base+i, 0, 0);
}
-static void __pminit setup_k7_watchdog(void)
+static void setup_k7_watchdog(void)
{
unsigned int evntsel;
@@ -238,7 +248,7 @@
wrmsr(MSR_K7_EVNTSEL0, evntsel, 0);
}
-static void __pminit setup_p6_watchdog(void)
+static void setup_p6_watchdog(void)
{
unsigned int evntsel;
@@ -260,7 +270,7 @@
wrmsr(MSR_P6_EVNTSEL0, evntsel, 0);
}
-static int __pminit setup_p4_watchdog(void)
+static int setup_p4_watchdog(void)
{
unsigned int misc_enable, dummy;
@@ -290,7 +300,7 @@
return 1;
}
-void __pminit setup_apic_nmi_watchdog (void)
+void setup_apic_nmi_watchdog (void)
{
switch (boot_cpu_data.x86_vendor) {
case X86_VENDOR_AMD:
@@ -314,7 +324,7 @@
default:
return;
}
- nmi_pm_init();
+ nmi_active = 1;
}
static spinlock_t nmi_print_lock = SPIN_LOCK_UNLOCKED;
--- clean/arch/i386/oprofile/nmi_int.c 2003-01-05 22:58:19.000000000 +0100
+++ linux-swsusp/arch/i386/oprofile/nmi_int.c 2003-02-10 17:34:38.000000000 +0100
@@ -11,6 +11,7 @@
#include <linux/notifier.h>
#include <linux/smp.h>
#include <linux/oprofile.h>
+#include <linux/device.h>
#include <asm/nmi.h>
#include <asm/msr.h>
#include <asm/apic.h>
@@ -24,27 +25,6 @@
static int nmi_start(void);
static void nmi_stop(void);
-
-static struct pm_dev * oprofile_pmdev;
-
-/* We're at risk of causing big trouble unless we
- * make sure to not cause any NMI interrupts when
- * suspended.
- */
-static int oprofile_pm_callback(struct pm_dev * dev,
- pm_request_t rqst, void * data)
-{
- switch (rqst) {
- case PM_SUSPEND:
- nmi_stop();
- break;
- case PM_RESUME:
- nmi_start();
- break;
- }
- return 0;
-}
-
static int nmi_callback(struct pt_regs * regs, int cpu)
{
@@ -86,7 +66,42 @@
saved_lvtpc[cpu] = apic_read(APIC_LVTPC);
apic_write(APIC_LVTPC, APIC_DM_NMI);
}
-
+
+static int nmi_enabled = 0; /* 0 == registered but off, 1 == registered and on */
+
+static int nmi_suspend(struct device *dev, u32 state, u32 level)
+{
+ if (level != SUSPEND_DISABLE)
+ return 0;
+ if (nmi_enabled == 1)
+ nmi_stop();
+ return 0;
+}
+
+static int nmi_resume(struct device *dev, u32 level)
+{
+ if (level != RESUME_POWER_ON)
+ return 0;
+ if (nmi_enabled == 1)
+ nmi_start();
+ return 0;
+}
+
+
+static struct device_driver nmi_driver = {
+ .name = "oprofile",
+ .bus = &system_bus_type,
+ .resume = nmi_resume,
+ .suspend = nmi_suspend,
+};
+
+static struct device device_nmi = {
+ .name = "oprofile",
+ .bus_id = "oprofile",
+ .driver = &nmi_driver,
+};
+
+extern struct sys_device device_apic;
static int nmi_setup(void)
{
@@ -95,13 +110,25 @@
* without actually triggering any NMIs as this will
* break the core code horrifically.
*/
+ if (nmi_watchdog == NMI_LOCAL_APIC) {
+ disable_apic_nmi_watchdog();
+ nmi_watchdog = NMI_LOCAL_APIC_SUSPENDED_BY_OPROFILE;
+ }
smp_call_function(nmi_cpu_setup, NULL, 0, 1);
nmi_cpu_setup(0);
set_nmi_callback(nmi_callback);
- oprofile_pmdev = set_nmi_pm_callback(oprofile_pm_callback);
- return 0;
+ nmi_enabled = 1;
+ return 0;
}
+static int __init init_nmi_driverfs(void)
+{
+ driver_register(&nmi_driver);
+ device_nmi.parent = &device_apic.dev;
+ device_register(&device_nmi);
+}
+
+late_initcall(init_nmi_driverfs);
static void nmi_restore_registers(struct op_msrs * msrs)
{
@@ -146,10 +173,14 @@
static void nmi_shutdown(void)
{
- unset_nmi_pm_callback(oprofile_pmdev);
+ nmi_enabled = 0;
unset_nmi_callback();
smp_call_function(nmi_cpu_shutdown, NULL, 0, 1);
nmi_cpu_shutdown(0);
+ if (nmi_watchdog == NMI_LOCAL_APIC_SUSPENDED_BY_OPROFILE) {
+ nmi_watchdog = NMI_LOCAL_APIC;
+ setup_apic_nmi_watchdog();
+ }
}
--- clean/include/asm-i386/apic.h 2002-10-20 16:22:45.000000000 +0200
+++ linux-swsusp/include/asm-i386/apic.h 2003-02-09 11:46:09.000000000 +0100
@@ -95,6 +95,7 @@
#define NMI_IO_APIC 1
#define NMI_LOCAL_APIC 2
#define NMI_INVALID 3
+#define NMI_LOCAL_APIC_SUSPENDED_BY_OPROFILE 4
#endif /* CONFIG_X86_LOCAL_APIC */
--
Worst form of spam? Adding advertisment signatures ala sourceforge.net.
What goes next? Inserting advertisment *into* email?
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: Switch APIC (+nmi, +oprofile) to driver model
@ 2003-02-10 19:05 Mikael Pettersson
2003-02-11 11:39 ` Pavel Machek
2003-02-11 11:53 ` Pavel Machek
0 siblings, 2 replies; 27+ messages in thread
From: Mikael Pettersson @ 2003-02-10 19:05 UTC (permalink / raw)
To: pavel; +Cc: levon, linux-kernel
On Mon, 10 Feb 2003 12:01:09 +0100, Pavel Machek wrote:
>> Also, apic_phys is (or should be) APIC_DEFAULT_PHYS_BASE, so
>> you shouldn't need to make apic_phys global.
>
>Really?
>
> /*
> * If no local APIC can be found then set up a fake all
> * zeroes page to simulate the local APIC and another
> * one for the IO-APIC.
> */
> if (!smp_found_config && detect_init_APIC()) {
> apic_phys = (unsigned long) alloc_bootmem_pages(PAGE_SIZE);
> apic_phys = __pa(apic_phys);
> } else
> apic_phys = mp_lapic_addr;
>
> set_fixmap_nocache(FIX_APIC_BASE, apic_phys);
>
>So it seems to me it really is variable.
The original code has the property that apic_pm_state.active is
true if and only if detect_init_APIC() was called and succeeded,
which implies that apic_phys == mp_lapic_addr == APIC_DEFAULT_PHYS_BASE.
You can also see that apic_pm_resume() writes APIC_DEFAULT_PHYS_BASE
to MSR_IA32_APICBASE, which only makes sense in this situation.
You moved the apic_pm_state.active = 1 assignment from detect_init_APIC(),
which is specific to UP_APIC, to setup_local_APIC(), which is called
also in the SMP case. Do you intend to do suspend and resume on SMP boxes
as well? If this is intensional, shouldn't device_apic be per-cpu?
/Mikael
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: Switch APIC (+nmi, +oprofile) to driver model
2003-02-09 14:07 Mikael Pettersson
2003-02-09 14:11 ` John Levon
2003-02-10 11:01 ` Pavel Machek
@ 2003-02-10 16:10 ` Pavel Machek
2003-02-11 11:43 ` Mikael Pettersson
2 siblings, 1 reply; 27+ messages in thread
From: Pavel Machek @ 2003-02-10 16:10 UTC (permalink / raw)
To: Mikael Pettersson; +Cc: levon, linux-kernel
Hi!
> >+ set_fixmap_nocache(FIX_APIC_BASE, apic_phys); /* FIXME: this is needed for S3 resume, but why? */
>
> This is horrible! The only reason this might be needed is if
> the page tables weren't restored properly at resume, and that
> indicates a bug somewhere else.
Pagetables should be restored okay, I do not know what is going on.
> >+static int __init init_apic_devicefs(void)
> > {
> >+ driver_register(&apic_driver);
> > if (apic_pm_state.active)
> >- pm_register(PM_SYS_DEV, 0, apic_pm_callback);
> >+ return sys_device_register(&device_apic);
> >+ return 0;
> ...
> >+device_initcall(init_apic_devicefs);
>
> init_apic_devicefs() registers apic_driver even if active is false.
> This probably breaks any machine where cpu_has_apic is false, since
> apic_suspend/resume will access non-existent APIC registers & MSRs.
>
> I don't like the idea of registering apic_driver when !cpu_has_apic,
> but it might be needed for the local-APIC NMI watchdog. A fix could
> be to guard apic_suspend/resume with "if(!cpu_has_apic)return;".
cpu_has_apic test seems wrong: AFAICS, there are CPUs that have apic
but linux does not know how to use it.
> > nmi_watchdog = nmi;
>
> You just killed NMI_LOCAL_APIC support on Intel.
Fixed.
> >+device_initcall(init_nmi_devicefs);
>
> Doesn't this require that init_apic_devicefs() has been done?
> Can you guarantee that?
Moved to late_initcall().
> >
> >+#define __pminit
> > #endif /* CONFIG_PM */
>
> __pminit is no longer defined when !CONFIG_PM, which will
> cause compile errors. Possibly the remaining uses of __pminit
> should be removed.
Fixed.
Pavel
--
Worst form of spam? Adding advertisment signatures ala sourceforge.net.
What goes next? Inserting advertisment *into* email?
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: Switch APIC (+nmi, +oprofile) to driver model
2003-02-10 11:01 ` Pavel Machek
@ 2003-02-10 11:50 ` John Levon
2003-02-10 20:06 ` Pavel Machek
0 siblings, 1 reply; 27+ messages in thread
From: John Levon @ 2003-02-10 11:50 UTC (permalink / raw)
To: linux-kernel
On Mon, Feb 10, 2003 at 12:01:09PM +0100, Pavel Machek wrote:
> Yes, whole oprofile/nmi interaction is ugly like hell. This way it is
> at least explicit, so people *know* its ugly.
That's no reason not do something like Mikael or I suggested.
john
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: Switch APIC (+nmi, +oprofile) to driver model
2003-02-09 14:07 Mikael Pettersson
2003-02-09 14:11 ` John Levon
@ 2003-02-10 11:01 ` Pavel Machek
2003-02-10 11:50 ` John Levon
2003-02-10 16:10 ` Pavel Machek
2 siblings, 1 reply; 27+ messages in thread
From: Pavel Machek @ 2003-02-10 11:01 UTC (permalink / raw)
To: Mikael Pettersson; +Cc: levon, linux-kernel
Hi!
> >Here's next attempt at moving APIC (+nmi, +oprofile) to the driver
> >model. If it looks good I'l submit it to Linus.
>
> I'm sorry to be a killjoy, but this still doesn't look right.
>
> >--- clean/arch/i386/kernel/apic.c 2003-01-05 22:58:18.000000000 +0100
> >+++ linux-swsusp/arch/i386/kernel/apic.c 2003-02-03 16:36:41.000000000 +0100
> >-static void apic_pm_resume(void *data)
> >+static int apic_resume(struct device *dev, u32 level)
> > {
> > unsigned int l, h;
> > unsigned long flags;
> >
> >+ if (level != RESUME_POWER_ON)
> >+ return 0;
> >+
> >+ set_fixmap_nocache(FIX_APIC_BASE, apic_phys); /* FIXME: this is needed for S3 resume, but why? */
>
> This is horrible! The only reason this might be needed is if
> the page tables weren't restored properly at resume, and that
> indicates a bug somewhere else.
>
> Also, apic_phys is (or should be) APIC_DEFAULT_PHYS_BASE, so
> you shouldn't need to make apic_phys global.
Really?
/*
* If no local APIC can be found then set up a fake all
* zeroes page to simulate the local APIC and another
* one for the IO-APIC.
*/
if (!smp_found_config && detect_init_APIC()) {
apic_phys = (unsigned long) alloc_bootmem_pages(PAGE_SIZE);
apic_phys = __pa(apic_phys);
} else
apic_phys = mp_lapic_addr;
set_fixmap_nocache(FIX_APIC_BASE, apic_phys);
So it seems to me it really is variable.
> >--- clean/arch/i386/kernel/nmi.c 2003-01-05 22:58:19.000000000 +0100
> >+++ linux-swsusp/arch/i386/kernel/nmi.c 2003-02-09 11:43:29.000000000 +0100
> >@@ -118,10 +121,6 @@
> > * missing bits. Right now Intel P6/P4 and AMD K7 only.
> > */
> > if ((nmi == NMI_LOCAL_APIC) &&
> >- (boot_cpu_data.x86_vendor == X86_VENDOR_INTEL) &&
> >- (boot_cpu_data.x86 == 6 || boot_cpu_data.x86 == 15))
> >- nmi_watchdog = nmi;
> >- if ((nmi == NMI_LOCAL_APIC) &&
> > (boot_cpu_data.x86_vendor == X86_VENDOR_AMD) &&
> > (boot_cpu_data.x86 == 6 || boot_cpu_data.x86 == 15))
> > nmi_watchdog = nmi;
>
> You just killed NMI_LOCAL_APIC support on Intel.
Oops, sorry, I seen two identical blocks of code... and they were not
identical. Sorry.
> >--- clean/arch/i386/oprofile/nmi_int.c 2003-01-05 22:58:19.000000000 +0100
> >+++ linux-swsusp/arch/i386/oprofile/nmi_int.c 2003-02-09 12:16:52.000000000 +0100
> ...
> >+ if (nmi_watchdog == NMI_LOCAL_APIC) {
> >+ disable_apic_nmi_watchdog();
> >+ nmi_watchdog = NMI_LOCAL_APIC_SUSPENDED_BY_OPROFILE;
> >+ }
> ...
> >+ if (nmi_watchdog == NMI_LOCAL_APIC_SUSPENDED_BY_OPROFILE) {
> >+ nmi_watchdog = NMI_LOCAL_APIC_SUSPENDED_BY_OPROFILE;
> >+ setup_apic_nmi_watchdog();
> >+ }
> ...
> >--- clean/include/asm-i386/apic.h 2002-10-20 16:22:45.000000000 +0200
> >+++ linux-swsusp/include/asm-i386/apic.h 2003-02-09 11:46:09.000000000 +0100
> >@@ -95,6 +95,7 @@
> > #define NMI_IO_APIC 1
> > #define NMI_LOCAL_APIC 2
> > #define NMI_INVALID 3
> >+#define NMI_LOCAL_APIC_SUSPENDED_BY_OPROFILE 4
>
> This is ugly like h*ll. Surely oprofile could just do:
Yes, whole oprofile/nmi interaction is ugly like hell. This way it is
at least explicit, so people *know* its ugly.
Pavel
--
Casualities in World Trade Center: ~3k dead inside the building,
cryptography in U.S.A. and free speech in Czech Republic.
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: Switch APIC (+nmi, +oprofile) to driver model
@ 2003-02-09 14:42 Mikael Pettersson
0 siblings, 0 replies; 27+ messages in thread
From: Mikael Pettersson @ 2003-02-09 14:42 UTC (permalink / raw)
To: levon; +Cc: linux-kernel, pavel
On Sun, 9 Feb 2003 14:11:42 +0000, John Levon wrote:
>On Sun, Feb 09, 2003 at 03:07:27PM +0100, Mikael Pettersson wrote:
>
>> I don't like the idea of registering apic_driver when !cpu_has_apic,
>> but it might be needed for the local-APIC NMI watchdog.
>
>Really ?
Pavel's patch changes nmi.c to include:
extern struct sys_device device_apic;
static int __init init_nmi_devicefs(void)
{
driver_register(&nmi_driver);
device_nmi.parent = &device_apic.dev;
return device_register(&device_nmi);
}
device_initcall(init_nmi_devicefs);
so nmi.c will unconditionally register a device with the local
APIC's device as parent. I strongly suspect this will break if
&device_apic hasn't itself been device_register():d.
The NMI driver's suspend/resume procedures check nmi_active before
doing anything, so registering the NMI device unconditionally
is _probably_ safe. (Although I personally think it should be
conditional.)
>as long as it's exported to modules. I'd probably prefer
>to just have :
>
> disable_nmi_watchdog();
> ...
> enable_nmi_watchdog();
>
>and have those do the right thing depending on a (nmi.c local)
>nmi_watchdog.
Yeah, that's nicer. disable_nmi_watchdog() could easily stash away
the previous state, and enable_nmi_watchdog() could check that copy
and conditionally call setup_apic_nmi_watchdog().
/Mikael
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: Switch APIC (+nmi, +oprofile) to driver model
2003-02-09 14:07 Mikael Pettersson
@ 2003-02-09 14:11 ` John Levon
2003-02-10 11:01 ` Pavel Machek
2003-02-10 16:10 ` Pavel Machek
2 siblings, 0 replies; 27+ messages in thread
From: John Levon @ 2003-02-09 14:11 UTC (permalink / raw)
To: Mikael Pettersson; +Cc: pavel, linux-kernel
On Sun, Feb 09, 2003 at 03:07:27PM +0100, Mikael Pettersson wrote:
> I don't like the idea of registering apic_driver when !cpu_has_apic,
> but it might be needed for the local-APIC NMI watchdog.
Really ?
> >+#define NMI_LOCAL_APIC_SUSPENDED_BY_OPROFILE 4
>
> This is ugly like h*ll. Surely oprofile could just do:
>
> static unsigned int prev_nmi_watchdog;
> ..
> prev_nmi_watchdog = nmi_watchdog;
as long as it's exported to modules. I'd probably prefer
to just have :
disable_nmi_watchdog();
...
enable_nmi_watchdog();
and have those do the right thing depending on a (nmi.c local)
nmi_watchdog.
regards,
john
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: Switch APIC (+nmi, +oprofile) to driver model
@ 2003-02-09 14:07 Mikael Pettersson
2003-02-09 14:11 ` John Levon
` (2 more replies)
0 siblings, 3 replies; 27+ messages in thread
From: Mikael Pettersson @ 2003-02-09 14:07 UTC (permalink / raw)
To: pavel; +Cc: levon, linux-kernel
On Sun, 9 Feb 2003 12:32:01 +0100, Pavel Machek wrote:
>Here's next attempt at moving APIC (+nmi, +oprofile) to the driver
>model. If it looks good I'l submit it to Linus.
I'm sorry to be a killjoy, but this still doesn't look right.
>--- clean/arch/i386/kernel/apic.c 2003-01-05 22:58:18.000000000 +0100
>+++ linux-swsusp/arch/i386/kernel/apic.c 2003-02-03 16:36:41.000000000 +0100
>-static void apic_pm_resume(void *data)
>+static int apic_resume(struct device *dev, u32 level)
> {
> unsigned int l, h;
> unsigned long flags;
>
>+ if (level != RESUME_POWER_ON)
>+ return 0;
>+
>+ set_fixmap_nocache(FIX_APIC_BASE, apic_phys); /* FIXME: this is needed for S3 resume, but why? */
This is horrible! The only reason this might be needed is if
the page tables weren't restored properly at resume, and that
indicates a bug somewhere else.
Also, apic_phys is (or should be) APIC_DEFAULT_PHYS_BASE, so
you shouldn't need to make apic_phys global.
>+static struct device_driver apic_driver = {
>+ .name = "apic",
>+ .bus = &system_bus_type,
>+ .resume = apic_resume,
>+ .suspend = apic_suspend,
>+};
...
>+static int __init init_apic_devicefs(void)
> {
>+ driver_register(&apic_driver);
> if (apic_pm_state.active)
>- pm_register(PM_SYS_DEV, 0, apic_pm_callback);
>+ return sys_device_register(&device_apic);
>+ return 0;
...
>+device_initcall(init_apic_devicefs);
init_apic_devicefs() registers apic_driver even if active is false.
This probably breaks any machine where cpu_has_apic is false, since
apic_suspend/resume will access non-existent APIC registers & MSRs.
I don't like the idea of registering apic_driver when !cpu_has_apic,
but it might be needed for the local-APIC NMI watchdog. A fix could
be to guard apic_suspend/resume with "if(!cpu_has_apic)return;".
>--- clean/arch/i386/kernel/nmi.c 2003-01-05 22:58:19.000000000 +0100
>+++ linux-swsusp/arch/i386/kernel/nmi.c 2003-02-09 11:43:29.000000000 +0100
>@@ -118,10 +121,6 @@
> * missing bits. Right now Intel P6/P4 and AMD K7 only.
> */
> if ((nmi == NMI_LOCAL_APIC) &&
>- (boot_cpu_data.x86_vendor == X86_VENDOR_INTEL) &&
>- (boot_cpu_data.x86 == 6 || boot_cpu_data.x86 == 15))
>- nmi_watchdog = nmi;
>- if ((nmi == NMI_LOCAL_APIC) &&
> (boot_cpu_data.x86_vendor == X86_VENDOR_AMD) &&
> (boot_cpu_data.x86 == 6 || boot_cpu_data.x86 == 15))
> nmi_watchdog = nmi;
You just killed NMI_LOCAL_APIC support on Intel.
>+static int __init init_nmi_devicefs(void)
>+{
>+ driver_register(&nmi_driver);
>+
>+ device_nmi.parent = &device_apic.dev;
>+ return device_register(&device_nmi);
>+}
>
>-#define __pminit __init
>+device_initcall(init_nmi_devicefs);
Doesn't this require that init_apic_devicefs() has been done?
Can you guarantee that?
>
>+#define __pminit
> #endif /* CONFIG_PM */
__pminit is no longer defined when !CONFIG_PM, which will
cause compile errors. Possibly the remaining uses of __pminit
should be removed.
>--- clean/arch/i386/oprofile/nmi_int.c 2003-01-05 22:58:19.000000000 +0100
>+++ linux-swsusp/arch/i386/oprofile/nmi_int.c 2003-02-09 12:16:52.000000000 +0100
...
>+ if (nmi_watchdog == NMI_LOCAL_APIC) {
>+ disable_apic_nmi_watchdog();
>+ nmi_watchdog = NMI_LOCAL_APIC_SUSPENDED_BY_OPROFILE;
>+ }
...
>+ if (nmi_watchdog == NMI_LOCAL_APIC_SUSPENDED_BY_OPROFILE) {
>+ nmi_watchdog = NMI_LOCAL_APIC_SUSPENDED_BY_OPROFILE;
>+ setup_apic_nmi_watchdog();
>+ }
...
>--- clean/include/asm-i386/apic.h 2002-10-20 16:22:45.000000000 +0200
>+++ linux-swsusp/include/asm-i386/apic.h 2003-02-09 11:46:09.000000000 +0100
>@@ -95,6 +95,7 @@
> #define NMI_IO_APIC 1
> #define NMI_LOCAL_APIC 2
> #define NMI_INVALID 3
>+#define NMI_LOCAL_APIC_SUSPENDED_BY_OPROFILE 4
This is ugly like h*ll. Surely oprofile could just do:
static unsigned int prev_nmi_watchdog;
..
prev_nmi_watchdog = nmi_watchdog;
if (prev_nmi_watchdog == NMI_LOCAL_APIC) {
disable_apic_nmi_watchdog();
nmi_watchdog = NMI_NONE;
}
...
if (prev_nmi_watchdog == NMI_LOCAL_APIC) {
nmi_watchdog = NMI_LOCAL_APIC;
setup_apic_nmi_watchdog();
}
without having to add crap to the global namespace?
/Mikael
^ permalink raw reply [flat|nested] 27+ messages in thread
end of thread, other threads:[~2003-02-26 22:20 UTC | newest]
Thread overview: 27+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2003-02-09 11:32 Switch APIC (+nmi, +oprofile) to driver model Pavel Machek
2003-02-09 11:54 ` John Levon
2003-02-09 12:03 ` John Levon
2003-02-09 14:07 Mikael Pettersson
2003-02-09 14:11 ` John Levon
2003-02-10 11:01 ` Pavel Machek
2003-02-10 11:50 ` John Levon
2003-02-10 20:06 ` Pavel Machek
2003-02-11 11:12 ` John Levon
2003-02-11 12:01 ` Pavel Machek
2003-02-13 13:39 ` Mikael Pettersson
2003-02-13 13:46 ` Pavel Machek
2003-02-14 9:08 ` Maciej W. Rozycki
2003-02-14 10:32 ` Mikael Pettersson
2003-02-14 10:54 ` Maciej W. Rozycki
2003-02-14 11:25 ` Mikael Pettersson
2003-02-14 11:32 ` Maciej W. Rozycki
2003-02-16 12:05 ` Pavel Machek
2003-02-10 16:10 ` Pavel Machek
2003-02-11 11:43 ` Mikael Pettersson
2003-02-09 14:42 Mikael Pettersson
2003-02-10 19:05 Mikael Pettersson
2003-02-11 11:39 ` Pavel Machek
2003-02-11 11:53 ` Pavel Machek
2003-02-16 12:43 Mikael Pettersson
2003-02-16 16:01 ` Pavel Machek
2003-02-26 21:59 ` Pavel Machek
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).