Re: [PATCH] x86: limit mwait_idle to Intel CPUs

["Andreas Herrmann" <andreas.herrmann3@amd.com>]

On Thu, Apr 05, 2007 at 05:37:17PM +0200, Andi Kleen wrote:
> 
> > > > This patch will enable default_idle for non-Intel
> > > > CPUs even if mwait is supported.
> > > 
> > > It would be better to clear MONITOR/MWAIT in the AMD specific
> > > CPU initialize code than add workarounds everywhere else.
> > 
> > Why is that?
> > MONITOR/MWAIT is usable.
> 
> If it doesn't save power it's not usable imho.

But you can also think of monitor/mwait as a power saving means
for fast synchronization.

> > And I think this should 
> > be indicated by cpuinfo.
> > It's just inappropriate to use it in pm_idle.
> 
> There are no other users anyways and user space can't use it.
> Ok in theory you could add a X86_FEATURE_MWAIT_DOESNT_SAVE_POWER
> and check that, but just clearing it seems simpler and equivalent.

It is not equivalent. Usually users check /proc/cpuinfo for their
CPU features. Deleting that flag is kind of obfuscation.

I guess some time ago people did not care about their "svm" or "vmx"
flags. Nowadays (e.g. with kvm) some people are quite happy
if one of those strings occurs in /proc/cpuinfo (and they are quite
disappointed if this feature was disabled by BIOS).

Why not state it positively for CPUs that are able to even enter
C1 with MWAIT, e.g. X86_FEATURE_MWAIT_ENTERS_CSTATE?

If you dislike this wording than I still prefer to have the MWAIT
flag visible and but introducing an MWAIT_DOESNT_ENTER_CSTATE thing.

> What would perhaps make sense is to add a idle=mwait command
> line option for this though. So that the benchmarkers who currently
> use idle=poll could migrate to idle=mwait. That option would need
> to check the real cpuid bit of course again, but that should be
> easy enough.

That sounds good. Except that I prefer to check for
X86_FEATURE_MWAIT.



Regards,

Andreas

-- 
AMD Saxony, Dresden, Germany
Operating System Research Center
email: andreas.herrmann3@amd.com
phone: +49-351-277-4919