Re: CPU local things [was Re: Nested suspends; messages vs. states]

Patrick Mochel
2005-03-24T15:59:21Z

[-- Attachment #1: Type: TEXT/PLAIN, Size: 2281 bytes --]

On Thu, 24 Mar 2005, Pavel Machek wrote:

> Hi!
>
> > > Just for clarity's sake, what are you thinking should happen to MTRR
> > > support?
> >
> > Become more componentized. We need a better way to represent optional
> > features of any device, but in particular CPUs. We should never be calling
> > it directly; we should be looping over the feature drivers bound to a 'CPU
> > Driver' and suspending them.
>
> Actually, no.
>
> mtrr's are cpu local. That means they need to be handled by CPU
> hotplug framework. I guess we should just drop them from "normal"
> device trees, and create something per-CPU.

Sorry, it was late and that explanation sucked.

- Every CPU has a set of optional features that it supports.

- MTRRs are an optional feature that a CPU may support.

- When the MTRR driver is loaded, a data structure should be allocated for
each CPU and added to a list.

- The list that the per-CPU MTRR data structure is added to could be part
of a 'CPU driver'.

- We should be looping over the set of optional features that a CPU
supports to suspend/resume them, rather than calling them directly.

Agree?

I agree that the CPU hotplug framework should help, since the save/restore
of MTRRs is needed for that feature as well.

I don't think they could/should go into normal device tree. They are part
of the special class of 'system' devices that don't support the notion of
a traditional driver and have to be handled at special times (when
interrupts are disabled) during the suspend/resume process.

I do think that the current model of system + platform devices kinda
stinks, but I do not think that merging them into the normal device tree
is the right solution.

> Perhaps plain old notification list is enough for this one.

It's possible, but notification lists present some problems. Like the fact
they use a hard-coded set of events in a global header file. They are good
only for a certain set of events.

It's damn simple to create a struct type for CPU features and a method
contained in each one for cpu offline/online. I would suggest adding a
list_head to struct cpu (include/linux/cpu.h) called 'features', then
having things like MTRR add themselves to that list.

But, that's just my $0.02 (which is greatly devalued these days :)

Pat