linux-kernel - Re: 50 Watt idle power regression bisected to Linux-3.10

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Wed, 11 Dec 2013 15:42:38 +0100
From:	Ingo Molnar <mingo@...nel.org>
To:	Peter Zijlstra <peterz@...radead.org>
Cc:	Mike Galbraith <bitbucket@...ine.de>,
	Borislav Petkov <bp@...en8.de>,
	Thomas Gleixner <tglx@...utronix.de>,
	Len Brown <lenb@...nel.org>,
	Linux PM list <linux-pm@...r.kernel.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	Jeremy Eder <jeder@...hat.com>, x86@...nel.org
Subject: Re: 50 Watt idle power regression bisected to Linux-3.10


* Peter Zijlstra <peterz@...radead.org> wrote:

> On Wed, Dec 11, 2013 at 01:29:15PM +0100, Mike Galbraith wrote:
> > On Wed, 2013-12-11 at 12:52 +0100, Peter Zijlstra wrote: 
> > > On Wed, Dec 11, 2013 at 12:38:39PM +0100, Borislav Petkov wrote:
> > > > Right, if it turns out that this is really the case and that this
> > > > erratum hasn't been fixed for models later than 29 - we'd need the
> > > > additional model numbers to set X86_FEATURE_CLFLUSH_MONITOR correctly.
> > > 
> > > You also need: https://lkml.org/lkml/2013/11/19/143
> > > 
> > > Because obviously not all mwait idle loops check that cpu bit.
> > 
> > I had tried that patch, to see if it would magically make the thing
> > start working, nope.  I had also tried...
> 
> > +		if (this_cpu_has(X86_FEATURE_CLFLUSH_MONITOR))
> > +			clflush((void *)&current_thread_info()->flags);
> 
> Yeah, you need a bit extra to enable that feature bit for your CPU as
> bpetkov said.
> 
> Something like the below.. someone needs to double check and possibly
> add SNB/IVB EX parts if they're already available.
> 
> diff --git a/arch/x86/kernel/cpu/intel.c b/arch/x86/kernel/cpu/intel.c
> index dc1ec0dff939..015642eed045 100644
> --- a/arch/x86/kernel/cpu/intel.c
> +++ b/arch/x86/kernel/cpu/intel.c
> @@ -387,8 +387,15 @@ static void init_intel(struct cpuinfo_x86 *c)
>  			set_cpu_cap(c, X86_FEATURE_PEBS);
>  	}
>  
> -	if (c->x86 == 6 && c->x86_model == 29 && cpu_has_clflush)
> -		set_cpu_cap(c, X86_FEATURE_CLFLUSH_MONITOR);
> +	if (c->x86 == 6 && cpu_has_clflush) {
> +		switch (c->x86_model) {
> +		case 29: /* Core2 EX */
> +		case 46: /* NHM EX */
> +		case 47: /* WSM EX */
> +			set_cpu_cap(c, X86_FEATURE_CLFLUSH_MONITOR);
> +			break;
> +		}
> +	}

Another thing that is required I think is to issue a write barrier 
before CLFLUSH instruction. By my (possibly incorrect ...) reading of 
the documentation CLFLUSH does not appear to be ordered (at all), so 
it might execute before the modification to the affected memory?


So something like:

		if (this_cpu_has(X86_FEATURE_CLFLUSH_MONITOR)) {
			smp_wmb(); /* order CLFLUSH */
			clflush(&current_thread_info()->flags);
		}

possibly put behind some utility function, smp_clflush() or so, hiding 
the CPU feature bit check as well:

		smp_clflush(&current_thread_info()->flags);

or so?

Thanks,

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