Skip to contentSkip to conversation

cpu/hw_random cleanups

PATCH In review Jeff Garzik 4 participants 6 messages Mar 13, 2003 Char and misc drivers

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 cpu/hw_random cleanups

Here are the requested cleanups to cpu capabilities and hw_random.c.

For x86 experts, the new cpu cap words are what needs looking over. For example, I wonder if storing Intel's cpuid(0x00000001) ecx register output is wise on older Intel cpus. I worry about garbage appearing there. Is that a false worry?

This has only been tested on Via Nehemiah CPUs... I have a laptop with an Intel RNG on it at home, and will be testing with that later on tonight to make sure nothing is broken. I'm hoping Alan or somebody can test AMD RNG... if not I'll poke around work and see if there are any boxes I can test on.

WRT hw_random, the main change there is to allow multiple independent openers to read(2) simultaneously, by removing the semaphore that limited userspace to a single open(2)er.

Comments welcome. This patch is to be considered as a fourth patch in the previously-posted hw_random series.

Jeff

===== arch/i386/kernel/cpu/centaur.c 1.7 vs edited =====

arch/i386/kernel/cpu/centaur.c +4 -3
@@ -256,9 +256,10 @@
 	if (cpuid_eax(0xC0000000) >= 0xC0000001) {
 		set_bit(X86_FEATURE_CENTAUR_EFF, c->x86_capability);
 
-		/* test for on-chip RNG */
-		if (cpuid_edx(0xC0000001) & (1 << 2))
-			set_bit(X86_FEATURE_XSTORE, c->x86_capability);
+		/* store Centaur Extended Feature Flags as
+		 * word 5 of the CPU capability bit array
+		 */
+		c->x86_capability[5] = cpuid_edx(0xC0000001);
 	}
 
 	switch (c->x86_model) {

===== arch/i386/kernel/cpu/common.c 1.19 vs edited =====

arch/i386/kernel/cpu/common.c +3 -2
@@ -211,9 +211,10 @@
 	
 		/* Intel-defined flags: level 0x00000001 */
 		if ( c->cpuid_level >= 0x00000001 ) {
-			u32 capability;
-			cpuid(0x00000001, &tfms, &junk, &junk, &capability);
+			u32 capability, excap;
+			cpuid(0x00000001, &tfms, &junk, &excap, &capability);
 			c->x86_capability[0] = capability;
+			c->x86_capability[4] = excap;
 			c->x86 = (tfms >> 8) & 15;
 			c->x86_model = (tfms >> 4) & 15;
 			c->x86_mask = tfms & 15;

===== arch/i386/kernel/cpu/proc.c 1.9 vs edited =====

arch/i386/kernel/cpu/proc.c +13 -1
@@ -38,7 +38,19 @@
 
 		/* Other (Linux-defined) */
 		"cxmmx", "k6_mtrr", "cyrix_arr", "centaur_mcr",
-		"centaur_eff", "xstore", NULL, NULL,
+		"centaur_eff", NULL, NULL, NULL,
+		NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL,
+		NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL,
+		NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL,
+
+		/* Intel-defined (#2) */
+		"pni", NULL, NULL, "monitor", "ds_cpl", NULL, NULL, NULL,
+		"tm2", NULL, "cnxt_id", NULL, NULL, NULL, NULL, NULL,
+		NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL,
+		NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL,
+
+		/* VIA/Cyrix/Centaur-defined */
+		NULL, NULL, "xstore", NULL, NULL, NULL, NULL, NULL,
 		NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL,
 		NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL,
 		NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL,

===== drivers/char/Kconfig 1.10 vs edited =====

drivers/char/Kconfig +2 -2
@@ -710,12 +710,12 @@
 	  If you're not sure, say N.
 
 config HW_RANDOM
-	tristate "Intel/AMD/Via H/W Random Number Generator support"
+	tristate "Intel/AMD/VIA HW Random Number Generator support"
 	depends on (X86 || IA64) && PCI
 	---help---
 	  This driver provides kernel-side support for the Random Number
 	  Generator hardware found on Intel i8xx-based motherboards,
-	  and AMD 76x-based motherboards.
+	  AMD 76x-based motherboards, and Via Nehemiah CPUs.
 
 	  Provides a character driver, used to read() entropy data.
 

===== drivers/char/hw_random.c 1.6 vs edited =====

drivers/char/hw_random.c +11 -31

126 lines not on this page · raw message

===== include/asm-i386/cpufeature.h 1.7 vs edited =====

include/asm-i386/cpufeature.h +7 -3
@@ -9,9 +9,9 @@
 
 #include <linux/bitops.h>
 
-#define NCAPINTS	4	/* Currently we have 4 32-bit words worth of info */
+#define NCAPINTS	6	/* Currently we have 6 32-bit words worth of info */
 
-/* Intel-defined CPU features, CPUID level 0x00000001, word 0 */
+/* Intel-defined CPU features, CPUID level 0x00000001 (edx), word 0 */
 #define X86_FEATURE_FPU		(0*32+ 0) /* Onboard FPU */
 #define X86_FEATURE_VME		(0*32+ 1) /* Virtual Mode Extensions */
 #define X86_FEATURE_DE		(0*32+ 2) /* Debugging Extensions */
@@ -64,7 +64,11 @@
 #define X86_FEATURE_CYRIX_ARR	(3*32+ 2) /* Cyrix ARRs (= MTRRs) */
 #define X86_FEATURE_CENTAUR_MCR	(3*32+ 3) /* Centaur MCRs (= MTRRs) */
 #define X86_FEATURE_CENTAUR_EFF	(3*32+ 4) /* Centaur Extended Feature Flags */
-#define X86_FEATURE_XSTORE	(3*32+ 5) /* on-CPU RNG present (xstore insn) */
+
+/* Intel-defined CPU features, CPUID level 0x00000001 (ecx), word 4 */
+
+/* VIA/Cyrix/Centaur-defined CPU features, CPUID level 0xC0000001, word 5 */
+#define X86_FEATURE_XSTORE	(5*32+ 2) /* on-CPU RNG present (xstore insn) */
 
 
 #define cpu_has(c, bit)		test_bit(bit, (c)->x86_capability)
#1 lore raw
Quoted 7 lines · Jeff Garzik

Here are the requested cleanups to cpu capabilities and hw_random.c.

For x86 experts, the new cpu cap words are what needs looking over. For example, I wonder if storing Intel's cpuid(0x00000001) ecx register output is wise on older Intel cpus. I worry about garbage appearing there. Is that a false worry?

Yes; it should be completely safe.

Quoted 18 lines

This has only been tested on Via Nehemiah CPUs... I have a laptop with an Intel RNG on it at home, and will be testing with that later on tonight to make sure nothing is broken. I'm hoping Alan or somebody can test AMD RNG... if not I'll poke around work and see if there are any boxes I can test on.

WRT hw_random, the main change there is to allow multiple independent openers to read(2) simultaneously, by removing the semaphore that limited userspace to a single open(2)er.

Comments welcome. This patch is to be considered as a fourth patch in the previously-posted hw_random series.

===== arch/i386/kernel/cpu/centaur.c 1.7 vs edited =====

arch/i386/kernel/cpu/centaur.c line 258

Quoted 3 lines
arch/i386/kernel/cpu/centaur.c +0 -0
@@ -256,9 +256,10 @@
 	if (cpuid_eax(0xC0000000) >= 0xC0000001) {
 		set_bit(X86_FEATURE_CENTAUR_EFF, c->x86_capability);
 

There is also no need to set a special feature bit for the existence of the feature flags. If they are not present the additional capability word will simply be zero.

#2 lore raw
Quoted 8 lines · H. Peter Anvin
Quoted 5 lines · Jeff Garzik

For x86 experts, the new cpu cap words are what needs looking over. For example, I wonder if storing Intel's cpuid(0x00000001) ecx register output is wise on older Intel cpus. I worry about garbage appearing there. Is that a false worry?

Yes; it should be completely safe.

noted

Quoted 11 lines

arch/i386/kernel/cpu/centaur.c line 258

Quoted 7 lines

===== arch/i386/kernel/cpu/centaur.c 1.7 vs edited =====

arch/i386/kernel/cpu/centaur.c +0 -0
@@ -256,9 +256,10 @@
 	if (cpuid_eax(0xC0000000) >= 0xC0000001) {
 		set_bit(X86_FEATURE_CENTAUR_EFF, c->x86_capability);
 

There is also no need to set a special feature bit for the existence of the feature flags. If they are not present the additional capability word will simply be zero.

Thanks, fixed.

Jeff

#3 lore raw
In reply to H. Peter Anvin
Quoted 6 lines
Quoted 4 lines

For example, I wonder if storing Intel's cpuid(0x00000001) ecx register output is wise on older Intel cpus. I worry about garbage appearing there. Is that a false worry?

Yes; it should be completely safe.

I have to admit I'd be more comfortable if we only set those bits IFF we know they are valid to check, not so much because we need to right now but out of a desire to make less mistakes possible

#4 lore raw
Quoted 11 lines · Alan Cox
Quoted 6 lines
Quoted 4 lines

For example, I wonder if storing Intel's cpuid(0x00000001) ecx register output is wise on older Intel cpus. I worry about garbage appearing there. Is that a false worry?

Yes; it should be completely safe.

I have to admit I'd be more comfortable if we only set those bits IFF we know they are valid to check, not so much because we need to right now but out of a desire to make less mistakes possible

The problem is that you risk exactly the opposite mistake -- it's called "anticompetitive feature lockout." Intel would happily tell you to do it; in fact, if you follow Intel guidelines or use their sample code it will treat anything that isn't an Intel chip like a 486.

If any chip that claims to support CPUID level 00000001h reports anything other than zero in this field, that chip is broken and we should deal with it as a chip-specific bug. At this point there are no chips which are known to have this bug (and there are, after all, only a finite number of chips out there.)

-hpa

#5 lore raw
In reply to Jeff Garzik
Quoted 2 lines · Jeff Garzik

I'm hoping Alan or somebody can test AMD RNG... if not I'll poke around work and see if there are any boxes I can test on.

My roommate has a 760MPX machine. If we can get the thing working (its been a big hassle) I am sure he would be happy to try out the new RNG. We can test it thoroughly.

It might not be until next week..

Robert Love