[PATCH 2.6.7-mm3] cpuflags reviewed

[PATCH 2.6.7-mm3] cpuflags reviewed

[FabF]

[-- Attachment #1: Type: text/plain, Size: 190 bytes --]

Andrew,

	Here's cpuflags v2 (x86 version) :

[/proc/cpuflags snippet]
no  sse2
no  ss
no  ht
no  tm
no  ia64
no  pbe
yes syscall
yes mp
no  nx
yes mmxext
no  lm
yes 3dnowext

Regards,
FabF

[-- Attachment #2: cpuflags2.diff --]
[-- Type: text/x-patch, Size: 6127 bytes --]

diff -Naur orig~cpuflags/arch/i386/kernel/cpu/proc.c edited~cpuflags/arch/i386/kernel/cpu/proc.c
--- orig~cpuflags/arch/i386/kernel/cpu/proc.c	2004-06-25 00:52:24.000000000 +0200
+++ edited~cpuflags/arch/i386/kernel/cpu/proc.c	2004-06-28 10:47:42.000000000 +0200
@@ -4,57 +4,57 @@
 #include <asm/semaphore.h>
 #include <linux/seq_file.h>
 
+/* 
+ * These flag bits must match the definitions in <asm/cpufeature.h>.
+ * NULL means this bit is undefined or reserved; either way it doesn't
+ * have meaning as far as Linux is concerned.  Note that it's important
+ * to realize there is a difference between this table and CPUID -- if
+ * applications want to get the raw CPUID data, they should access
+ * /dev/cpu/<cpu_nr>/cpuid instead.
+ */
+static char *x86_cap_flags[] = {
+	/* Intel-defined */
+        "fpu", "vme", "de", "pse", "tsc", "msr", "pae", "mce",
+        "cx8", "apic", NULL, "sep", "mtrr", "pge", "mca", "cmov",
+        "pat", "pse36", "pn", "clflush", NULL, "dts", "acpi", "mmx",
+        "fxsr", "sse", "sse2", "ss", "ht", "tm", "ia64", "pbe",
+
+	/* AMD-defined */
+	NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL,
+	NULL, NULL, NULL, "syscall", NULL, NULL, NULL, NULL,
+	NULL, NULL, NULL, "mp", "nx", NULL, "mmxext", NULL,
+	NULL, NULL, NULL, NULL, NULL, "lm", "3dnowext", "3dnow",
+
+	/* Transmeta-defined */
+	"recovery", "longrun", NULL, "lrti", 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,
+
+	/* Other (Linux-defined) */
+	"cxmmx", "k6_mtrr", "cyrix_arr", "centaur_mcr",
+	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,
+
+	/* Intel-defined (#2) */
+	"pni", NULL, NULL, "monitor", "ds_cpl", NULL, NULL, "tm2",
+	"est", NULL, "cid", 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, "rng", "rng_en", NULL, NULL, "ace", "ace_en",
+	NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL,
+	NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL,
+	NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL,
+};
 /*
  *	Get CPU information for use by the procfs.
  */
 static int show_cpuinfo(struct seq_file *m, void *v)
 {
-	/* 
-	 * These flag bits must match the definitions in <asm/cpufeature.h>.
-	 * NULL means this bit is undefined or reserved; either way it doesn't
-	 * have meaning as far as Linux is concerned.  Note that it's important
-	 * to realize there is a difference between this table and CPUID -- if
-	 * applications want to get the raw CPUID data, they should access
-	 * /dev/cpu/<cpu_nr>/cpuid instead.
-	 */
-	static char *x86_cap_flags[] = {
-		/* Intel-defined */
-	        "fpu", "vme", "de", "pse", "tsc", "msr", "pae", "mce",
-	        "cx8", "apic", NULL, "sep", "mtrr", "pge", "mca", "cmov",
-	        "pat", "pse36", "pn", "clflush", NULL, "dts", "acpi", "mmx",
-	        "fxsr", "sse", "sse2", "ss", "ht", "tm", "ia64", "pbe",
-
-		/* AMD-defined */
-		NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL,
-		NULL, NULL, NULL, "syscall", NULL, NULL, NULL, NULL,
-		NULL, NULL, NULL, "mp", "nx", NULL, "mmxext", NULL,
-		NULL, NULL, NULL, NULL, NULL, "lm", "3dnowext", "3dnow",
-
-		/* Transmeta-defined */
-		"recovery", "longrun", NULL, "lrti", 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,
-
-		/* Other (Linux-defined) */
-		"cxmmx", "k6_mtrr", "cyrix_arr", "centaur_mcr",
-		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,
-
-		/* Intel-defined (#2) */
-		"pni", NULL, NULL, "monitor", "ds_cpl", NULL, NULL, "tm2",
-		"est", NULL, "cid", 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, "rng", "rng_en", NULL, NULL, "ace", "ace_en",
-		NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL,
-		NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL,
-		NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL,
-	};
 	struct cpuinfo_x86 *c = v;
 	int i, n = c - cpu_data;
 	int fpu_exception;
@@ -144,3 +144,15 @@
 	.stop	= c_stop,
 	.show	= show_cpuinfo,
 };
+int show_cpuflags(char *page)
+{
+	int i,len=0;
+	for ( i = 0 ; i < 32*NCAPINTS ; i++ )
+		if ( x86_cap_flags[i] != NULL ){
+			if ( test_bit(i, cpu_data[0].x86_capability) )
+				len+=sprintf(page+len, "yes");
+			  else  len+=sprintf(page+len, "no ");	
+			len+=sprintf(page+len, " %s\n", x86_cap_flags[i]);
+		}
+	return len;
+}
diff -Naur orig~cpuflags/fs/proc/proc_misc.c edited~cpuflags/fs/proc/proc_misc.c
--- orig~cpuflags/fs/proc/proc_misc.c	2004-06-25 00:53:20.000000000 +0200
+++ edited~cpuflags/fs/proc/proc_misc.c	2004-06-26 13:54:22.000000000 +0200
@@ -66,6 +66,7 @@
 extern int get_exec_domain_list(char *);
 extern int get_dma_list(char *);
 extern int get_locks_status (char *, char **, off_t, int);
+extern int show_cpuflags (char *);
 
 static int proc_calc_metrics(char *page, char **start, off_t off,
 				 int count, int *eof, int len)
@@ -149,6 +150,18 @@
 	return proc_calc_metrics(page, start, off, count, eof, len);
 }
 
+static int cpuflags_read_proc(char *page, char **start, off_t off,
+				 int count, int *eof, void *data)
+{
+	int len;
+#ifdef CONFIG_X86
+	len=show_cpuflags(page);
+	return proc_calc_metrics(page, start, off, count, eof, len);
+#endif
+	return 0;
+
+}
+
 static int meminfo_read_proc(char *page, char **start, off_t off,
 				 int count, int *eof, void *data)
 {
@@ -677,6 +690,7 @@
 		{"loadavg",     loadavg_read_proc},
 		{"uptime",	uptime_read_proc},
 		{"meminfo",	meminfo_read_proc},
+		{"cpuflags",	cpuflags_read_proc},
 		{"version",	version_read_proc},
 #ifdef CONFIG_PROC_HARDWARE
 		{"hardware",	hardware_read_proc},

Re: [PATCH 2.6.7-mm3] cpuflags reviewed

[Dave Jones]

On Mon, Jun 28, 2004 at 11:55:06AM +0200, FabF wrote:
 > Andrew,
 > 
 > 	Here's cpuflags v2 (x86 version) :
 > 
 > [/proc/cpuflags snippet]
 > no  sse2
 > no  ss
 > no  ht
 > no  tm
 > no  ia64
 > no  pbe
 > yes syscall
 > yes mp
 > no  nx
 > yes mmxext
 > no  lm
 > yes 3dnowext

This can be done just as easily (if not easier) in userspace.
I see no value in adding a duplicate flag mechanism to the kernel.

		Dave

Re: [PATCH 2.6.7-mm3] cpuflags reviewed

[Dave Jones]

On Tue, Jun 29, 2004 at 10:42:04AM +0200, fabian.frederick@skynet.be wrote:

 > I made this one in order to display flags in a clear way (ie better than
 > cpuinfo) and display \"what we have and what we don't\"
 > related to my arch. Kernel is authoritative there and gathers all
 > info.

How is this more authorative than a userspace app ?
If a new feature flag appears that the kernel doesn't know about,
it won't get picked up. Just as it won't in a userspace app.
The difference being, someone can grab the latest version of the
app without needing to update their whole kernel.

 > Having userland maintaining such stuff would be double-work
 > double-effort (ok, it's usual :) ).Let's say procps maintains flagging as
 > well, it would require all arch updates.

Updating userspace is less impact than kernel updates.

 > All this without the fact that kernel would be able to deliver such info
 > through a syscall or something I'm not aware of...

That would be even more overkill.  The cpuid driver does everything
you need, in userspace.  It has established userspace users.
Do you expect those users to switch over to /proc/cpuflags ?
It won't happen, and adding duplicate interfaces is just pointless.

 > IOW, userland requires more feed.Feed userland pls :))))

No, it does not.  All you patch brings afaics is the need to parse
ASCII (/proc/cpuflags) vs binary (/dev/cpu/n/cpuid), and we
already have a /proc/cpuinfo for that.

		Dave