linux-kernel - Re: [PATCH 5/10 -tip] x86: check_powernow() for K8 and later user of Advanced Power Management features

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:	Tue, 12 May 2009 19:48:48 +0200
From:	Ingo Molnar <mingo@...e.hu>
To:	Jaswinder Singh Rajput <jaswinder@...nel.org>
Cc:	"H. Peter Anvin" <hpa@...nel.org>,
	Robert Richter <robert.richter@....com>,
	Dave Jones <davej@...hat.com>,
	LKML <linux-kernel@...r.kernel.org>,
	x86 maintainers <x86@...nel.org>
Subject: Re: [PATCH 5/10 -tip] x86: check_powernow() for K8 and later user
	of Advanced Power Management features


* Jaswinder Singh Rajput <jaswinder@...nel.org> wrote:

> -		eax = cpuid_eax(CPUID_GET_MAX_CAPABILITIES);
> -		if (eax < CPUID_FREQ_VOLT_CAPABILITIES) {
> +		/* Advanced Power Management capabilities */
> +		if (c->x86_capability[9]) {
>  			printk(KERN_INFO PFX
>  			       "No frequency change capabilities detected\n");
>  			goto out;
>  		}

How is the new check equivalent to the old one? It isnt and this is 
a bug.

Also, open-coding x86_capability[9] like that is quite unclean. Were 
we ever to reorder those bits internally, this could would break.

But i see what you are trying to do. A better method might be to add 
a new helper:

+#define X86_FEATURE_TS         (9*32+ 0) /* Temperatue sensor          */
+#define X86_FEATURE_FID                (9*32+ 1) /* Frequency ID control	*/
+#define X86_FEATURE_VID                (9*32+ 2) /* Voltage ID control		*/
+#define X86_FEATURE_TTP                (9*32+ 3) /* Thermal trip   	*/
+#define X86_FEATURE_HTC                (9*32+ 4) /* Hardware thermal control   */
+#define X86_FEATURE_STC                (9*32+ 5) /* Software thermal control   */
+#define X86_FEATURE_100MHZSTEPS        (9*32+ 6) /* 100 MHz multiplier control */
+#define X86_FEATURE_HWPSTATE   (9*32+ 7) /* Hardware P-State control   */
+#define X86_FEATURE_CONSTANT_TSC (9*32+ 8) /* Constant rate TSC ticks  */

... to represent the 'is any of these set' property.

[ btw., there's a typo in the X86_FEATURE_TS comment above ]

	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/