linux-kernel - Re: [PATCH 2/5] powernow-k8: Add core performance boost support

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 for Android: free password hash cracker in your pocket

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

Message-ID: <20100323115858.GC16493@aftab>
Date:	Tue, 23 Mar 2010 12:58:58 +0100
From:	Borislav Petkov <bp@...64.org>
To:	Thomas Renninger <trenn@...e.de>
Cc:	akpm@...ux-foundation.org, davej@...hat.com,
	cpufreq@...r.kernel.org, x86@...nel.org,
	linux-kernel@...r.kernel.org, "H. Peter Anvin" <hpa@...or.com>
Subject: Re: [PATCH 2/5] powernow-k8: Add core performance boost support

(Adding hpa to Cc)

From: Thomas Renninger <trenn@...e.de>
Date: Tue, Mar 23, 2010 at 12:17:16PM +0100

> > +/* core performance boost */
> > +static bool cpb_capable, cpb_disabled;
> Whatabout using a cpufeature (arch/x86/include/asm/cpufeature.h)
> instead of cpb_capable. Then people could see this feature in 
> /proc/cpuinfo and other code parts could check for it easily if
> needed later.

I don't have a problem with that per se. It's just that /proc/cpuinfo
is a widely used interface and, AFAIR, changing it is not taken that
lightly.

Peter, what do you think?

> It could already be set in arch/x86/kernel/cpu/amd.c and
> powernow-k8 could use cpu_has(cpu, X86_FEATURE_CPB);

I'd still like to cache the cpb_capable value locally instead of getting
x86_cpuinfo percpu var and querying it. Especially if this happens often
and not only at driver init.

> Instead of cpb_disabled, I'd use cpb_enabled. Checking for
> !cpb_disabled whether it's enabled, is ugly to read.

Fair enough, cpb_disabled reflects the bit semantics in the MSR but why
not, don't matter which to me.

[..]

> > +		_cpb_toggle_msrs(t);
> > +		printk(KERN_INFO PFX "Core Boosting enabled.\n");
> Always printk on every toggle?
> That should not happen often and a user might want to get noticed if
> an app does this behind his back -> should be fine w/ or w/o, just not 
> sure whether it's intended.

Well, actually, this should be on by default and the user or an app
shouldn't be fidgeting with it all the time. It's there only for
benchmarking purposes so that you can disable it when you really have
to. But I guess it actually is going to get used if its there so maybe
we should have to rethink our approach. Hmm...

-- 
Regards/Gruss,
Boris.

--
Advanced Micro Devices, Inc.
Operating Systems Research Center
--
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/