Re: Switch APIC (+nmi, +oprofile) to driver model

[Pavel Machek <pavel@ucw.cz>]

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?