It is currently Fri, 23 Oct 2020 18:14:59 GMT



 
Author Message
 Switch APIC (+nmi, +oprofile) to driver model
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 * ...

read more »



 Thu, 28 Jul 2005 12:40:12 GMT   
 Switch APIC (+nmi, +oprofile) to driver model

This is obviously wrong...

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/



 Thu, 28 Jul 2005 13:00:13 GMT   
 Switch APIC (+nmi, +oprofile) to driver model

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
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/



 Thu, 28 Jul 2005 13:10:10 GMT   
 Switch APIC (+nmi, +oprofile) to driver model

I'm sorry to be a killjoy, but this still doesn't look right.

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.

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;".

You just killed NMI_LOCAL_APIC support on Intel.

Doesn't this require that init_apic_devicefs() has been done?
Can you guarantee that?

__pminit is no longer defined when !CONFIG_PM, which will
cause compile errors. Possibly the remaining uses of __pminit
should be removed.

- Show quoted text -

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 {*filter*}to the global namespace?

/Mikael
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at   http://www.**-**.com/
Please read the FAQ at   http://www.**-**.com/



 Thu, 28 Jul 2005 15:20:05 GMT   
 Switch APIC (+nmi, +oprofile) to driver model

Really ?

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
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/



 Thu, 28 Jul 2005 15:20:07 GMT   
 Switch APIC (+nmi, +oprofile) to driver model

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.)

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
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/



 Thu, 28 Jul 2005 15:50:13 GMT   
 Switch APIC (+nmi, +oprofile) to driver model
Hi!

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.

Oops, sorry, I seen two identical blocks of code... and they were not
identical. Sorry.

- Show quoted text -

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.
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


 Fri, 29 Jul 2005 12:10:08 GMT   
 Switch APIC (+nmi, +oprofile) to driver model

That's no reason not do something like Mikael or I suggested.

john
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/



 Fri, 29 Jul 2005 13:00:12 GMT   
 Switch APIC (+nmi, +oprofile) to driver model

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
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/



 Fri, 29 Jul 2005 20:10:16 GMT   
 Switch APIC (+nmi, +oprofile) to driver model
Hi!

Pagetables should be restored okay, I do not know what is going on.

cpu_has_apic test seems wrong: AFAICS, there are CPUs that have apic
but linux does not know how to use it.

Fixed.

Moved to late_initcall().

Fixed.
                                                                Pavel

--
Worst form of spam? Adding advertisment signatures ala sourceforge.net.
What goes next? Inserting advertisment *into* email?
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/



 Sat, 30 Jul 2005 12:20:07 GMT   
 Switch APIC (+nmi, +oprofile) to driver model
Hi!

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 ...

read more »



 Sat, 30 Jul 2005 12:20:12 GMT   
 Switch APIC (+nmi, +oprofile) to driver model

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
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/



 Sat, 30 Jul 2005 12:20:16 GMT   
 Switch APIC (+nmi, +oprofile) to driver model
 > 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
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/



 Sat, 30 Jul 2005 12:50:09 GMT   
 Switch APIC (+nmi, +oprofile) to driver model
Hi!

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?]
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/



 Sat, 30 Jul 2005 13:20:08 GMT   
 Switch APIC (+nmi, +oprofile) to driver model
Hi!

Okay, I guess I do not rely on so complicated invariants...

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?]
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/



 Sat, 30 Jul 2005 13:20:09 GMT   
 
   [ 25 post ]  Go to page: [1] [2]

Similar Threads

1. Switch APIC (+nmi, +oprofile) to driver model

2. Switch APIC to driver model (and make S3 sleep with APIC on)

3. [2.5.74] x86_64 apic/nmi driver model conversion cleanups

4. Switch APIC to driver model

5. Convert APIC to driver model: now it works with SMP

6. Convert APIC to driver model

7. Driver model for APIC

8. OProfile: small NMI shutdown fix

9. [4/7] oprofile - NMI hook

10. OProfile: IO-APIC fixup


 
Powered by phpBB © 2000, 2002, 2005, 2007 phpBB Group.
Designed by ST Software