Skip to contentSkip to conversation

Dump interesting arch/platform info

RFC In review Borislav Petkov 4 participants 16 messages Feb 1, 2016 – Feb 9, 2016 x86

lore
Share & tools
lore

Borislav Petkov

author maintainer
#0 lore raw

RFC 1/1 Dump interesting arch/platform info

Hi,

so I've been playing with this simple module below. The idea is to be able to dump interesting arch/platform information on the currently running system. For example, how does the GDT look like. It is supposed to be used as a debugging aid and the information it dumps is not easily accessible from userspace.

So the use case is you go, modprobe this thing and do

cat /sys/kernel/debug/x86/archinfo

Right now, it dumps only the GDT and even that is not fully done. But more interesting stuff should be added to it as need arises.

Thoughts, ideas, suggestions?

Thanks.

---
CPU0, GDT ffff88007b609000:
00:
[ base[31:24]:00 G:0 D:0 L:0 AVL:0 lim[19:16]:0 | P:0 DPL:0 S:0 C:0 base[23:16]:00 ] [ base[15:00]:0000 | lim[15:00]:0000 ]: System: (0x0) Reserved (illegal)

01:
[ base[31:24]:00 G:1 D:1 L:0 AVL:0 lim[19:16]:f | P:1 DPL:0 S:1 C:0 base[23:16]:00 ] [ base[15:00]:0000 | lim[15:00]:ffff ]: User: (0xb) Code, Execute/Readable - Accessed

02:
[ base[31:24]:00 G:1 D:0 L:1 AVL:0 lim[19:16]:f | P:1 DPL:0 S:1 C:0 base[23:16]:00 ] [ base[15:00]:0000 | lim[15:00]:ffff ]: User: (0xb) Code, Execute/Readable - Accessed

03:
[ base[31:24]:00 G:1 D:1 L:0 AVL:0 lim[19:16]:f | P:1 DPL:0 S:1 C:0 base[23:16]:00 ] [ base[15:00]:0000 | lim[15:00]:ffff ]: User: (0x3) Data, Read/Write - Accessed

04:
[ base[31:24]:00 G:1 D:1 L:0 AVL:0 lim[19:16]:f | P:1 DPL:3 S:1 C:0 base[23:16]:00 ] [ base[15:00]:0000 | lim[15:00]:ffff ]: User: (0xb) Code, Execute/Readable - Accessed

05:
[ base[31:24]:00 G:1 D:1 L:0 AVL:0 lim[19:16]:f | P:1 DPL:3 S:1 C:0 base[23:16]:00 ] [ base[15:00]:0000 | lim[15:00]:ffff ]: User: (0x3) Data, Read/Write - Accessed

06:
[ base[31:24]:00 G:1 D:0 L:1 AVL:0 lim[19:16]:f | P:1 DPL:3 S:1 C:0 base[23:16]:00 ] [ base[15:00]:0000 | lim[15:00]:ffff ]: User: (0xb) Code, Execute/Readable - Accessed

07:
[ base[31:24]:00 G:0 D:0 L:0 AVL:0 lim[19:16]:0 | P:0 DPL:0 S:0 C:0 base[23:16]:00 ] [ base[15:00]:0000 | lim[15:00]:0000 ]: System: (0x0) Reserved (illegal)

08:
[ base[31:24]:7b G:0 D:0 L:0 AVL:0 lim[19:16]:0 | P:1 DPL:0 S:0 C:0 base[23:16]:7d ] [ base[15:00]:2e00 | lim[15:00]:2087 ]: System: (0xb) Busy 32-bit TSS

09:
[ base[31:24]:00 G:0 D:0 L:0 AVL:0 lim[19:16]:0 | P:0 DPL:0 S:0 C:0 base[23:16]:00 ] [ base[15:00]:ffff | lim[15:00]:8800 ]: System: (0x0) Reserved (illegal)

10:
[ base[31:24]:00 G:0 D:0 L:0 AVL:0 lim[19:16]:0 | P:0 DPL:0 S:0 C:0 base[23:16]:00 ] [ base[15:00]:0000 | lim[15:00]:0000 ]: System: (0x0) Reserved (illegal)

11:
[ base[31:24]:00 G:0 D:0 L:0 AVL:0 lim[19:16]:0 | P:0 DPL:0 S:0 C:0 base[23:16]:00 ] [ base[15:00]:0000 | lim[15:00]:0000 ]: System: (0x0) Reserved (illegal)

12:
[ base[31:24]:00 G:0 D:0 L:0 AVL:0 lim[19:16]:0 | P:0 DPL:0 S:0 C:0 base[23:16]:00 ] [ base[15:00]:0000 | lim[15:00]:0000 ]: System: (0x0) Reserved (illegal)

13:
[ base[31:24]:00 G:0 D:0 L:0 AVL:0 lim[19:16]:0 | P:0 DPL:0 S:0 C:0 base[23:16]:00 ] [ base[15:00]:0000 | lim[15:00]:0000 ]: System: (0x0) Reserved (illegal)

14:
[ base[31:24]:00 G:0 D:0 L:0 AVL:0 lim[19:16]:0 | P:0 DPL:0 S:0 C:0 base[23:16]:00 ] [ base[15:00]:0000 | lim[15:00]:0000 ]: System: (0x0) Reserved (illegal)

15:
[ base[31:24]:00 G:0 D:1 L:0 AVL:0 lim[19:16]:0 | P:1 DPL:3 S:1 C:1 base[23:16]:00 ] [ base[15:00]:0000 | lim[15:00]:0000 ]: User: (0x5) Data, Expand-down, Read-Only - Accessed

...

----

Info:
base,limit,A,G,R: ignored in 64-bit mode.
G: granularity bit (23):
- 0b: segment limit is not scaled.
- 1b: segment limit scaled by 4K.
D/B: CS default operand size bit (22):
- 0b: 16-bit.
- 1b: 32-bit.
D=0b is the only allowed setting in long mode (L=1b). Called B in stack segments.
L: long mode bit (21):
- 0b: CPU in compat mode. Enables segmentation.
- 1b: CPU in long mode.
AVL: bit available to software (20).
P: present bit (15):
- 0b: seg. not present in mem => #NP.
- 1b: seg is present in memory.
DPL: Descriptor Privilege Level [14:13]:
- 0b: highest privilege level.
...
- 3b: lowest privilege level.
S+Type: decriptor types [12,11:8]:
Specify descriptor type and access characteristics.
S:
- 0b: System descriptor.
- 1b: User descriptor.
R: readable bit (9):
- 0b: code seg is executable, reads -> #GP
- 1b: code seg is both read/exec
A: accessed bit (8): set by CPU when desc copied into %cs; cleared only by sw.

---
From: Borislav Petkov <[email protected]>
Date: Mon, 30 Nov 2015 13:53:36 +0100
Subject: [PATCH] x86: Add an archinfo dumper module

Dump interesting/valuable information related to the x86 architecture of the current CPU and platform.

---
 arch/x86/Kconfig.debug     |   5 ++
 arch/x86/kernel/Makefile   |   1 +
 arch/x86/kernel/archinfo.c | 164 +++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 170 insertions(+)
 create mode 100644 arch/x86/kernel/archinfo.c
arch/x86/Kconfig.debug +5 -0
index 9b18ed97a8a2..c757d1ee9a7a 100644
@@ -383,4 +383,9 @@ config PUNIT_ATOM_DEBUG
 	  The current power state can be read from
 	  /sys/kernel/debug/punit_atom/dev_power_state
 
+config ARCHINFO
+	tristate "x86 archinfo dumper module"
+	---help---
+	  Dump interesting x86 arch stuff.
+
 endmenu
arch/x86/kernel/Makefile +1 -0
index b1b78ffe01d0..89972c1b9de6 100644
@@ -109,6 +109,7 @@ obj-$(CONFIG_EFI)			+= sysfb_efi.o
 
 obj-$(CONFIG_PERF_EVENTS)		+= perf_regs.o
 obj-$(CONFIG_TRACING)			+= tracepoint.o
+obj-$(CONFIG_ARCHINFO)			+= archinfo.o
 
 ###
 # 64 bit specific files
arch/x86/kernel/archinfo.c new file +164 -0

164 lines not on this page · raw message

Signature · 8 lines
2.3.5


-- 
Regards/Gruss,
    Boris.

ECO tip #101: Trim your mails when you reply.
#1 lore raw
Quoted 8 lines

So the use case is you go, modprobe this thing and do

cat /sys/kernel/debug/x86/archinfo

Right now, it dumps only the GDT and even that is not fully done. But more interesting stuff should be added to it as need arises.

Thoughts, ideas, suggestions?

On a high core count, multi-socket, machine I see almost 10K lines of output from this. If you add other things it will become a bit of a jumble to parse.

Perhaps the module name "archinfo" is fine, but each separate thing it shows should get its own file. Starting with:

/sys/kernel/debug/x86/gdtinfo

for the current output?

-Tony

author maintainer
#2 lore raw
Quoted 3 lines · Tony

On a high core count, multi-socket, machine I see almost 10K lines of output from this. If you add other things it will become a bit of a jumble to parse.

Always thinking big... :-)

Quoted 6 lines

Perhaps the module name "archinfo" is fine, but each separate thing it shows should get its own file. Starting with:

/sys/kernel/debug/x86/gdtinfo

for the current output?

Right, or put everything under a directory "archinfo":

/sys/kernel/debug/x86/archinfo/ ... gdt

				... msrs

				...

and so on.

Yap, sounds better. Thanks!

Signature · 4 lines
Regards/Gruss,
    Boris.

ECO tip #101: Trim your mails when you reply.
#3 lore raw
Quoted 16 lines · Borislav Petkov
Quoted 3 lines · Tony

On a high core count, multi-socket, machine I see almost 10K lines of output from this. If you add other things it will become a bit of a jumble to parse.

Always thinking big... :-)

Quoted 6 lines

Perhaps the module name "archinfo" is fine, but each separate thing it shows should get its own file. Starting with:

/sys/kernel/debug/x86/gdtinfo

for the current output?

Right, or put everything under a directory "archinfo":

Quoted 8 lines

/sys/kernel/debug/x86/archinfo/ ... gdt

				... msrs

				...

and so on.

Yap, sounds better. Thanks!

Agreed. We used to have something like this but then removed it due to various problems - but the concept itself is fine.

Note that these can be pretty security sensitive pieces of information, and can also expose ASLR/KASLR details, so the VFS interface needs to be a strictly root-only thing.

Thanks,

Ingo

author maintainer
#4 lore raw
Quoted 6 lines · Ingo Molnar

Agreed. We used to have something like this but then removed it due to various problems - but the concept itself is fine.

Note that these can be pretty security sensitive pieces of information, and can also expose ASLR/KASLR details, so the VFS interface needs to be a strictly root-only thing.

Oh sure, so the use case I have in mind is have this as a module, modprobe it on a system, get the whole info and delete the module .ko file again. As root.

Signature · 4 lines
Regards/Gruss,
    Boris.

ECO tip #101: Trim your mails when you reply.
author maintainer
#5 lore raw

PATCH 1/1 x86: Add an archinfo dumper module

Here's v2 with the stuff we talked about, implemented. I've added 'control_regs' file too so that you can do:

$ cat /sys/kernel/debug/x86/archinfo/control_regs
CR4: [-|-|SMEP|OSXSAVE|-|-|-|-|OSXMMEXCPT|OSFXSR|-|PGE|MCE|PAE|PSE|-|-|-|-]: 0x1406f0

for example. Yeah, only CR4 right now.

Off the top of my head, we would need "msrs" which dumps EFER and a bunch of other interesting MSRs along with the names of the set bits.

---
From: Borislav Petkov <[email protected]>
Date: Mon, 30 Nov 2015 13:53:36 +0100
Subject: [PATCH -v2] x86: Add an archinfo dumper module

Dump interesting/valuable information related to the x86 architecture of the current CPU and platform.

---
 arch/x86/Kconfig.debug     |   5 ++
 arch/x86/kernel/Makefile   |   1 +
 arch/x86/kernel/archinfo.c | 215 +++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 221 insertions(+)
 create mode 100644 arch/x86/kernel/archinfo.c
arch/x86/Kconfig.debug +5 -0
index 9b18ed97a8a2..c757d1ee9a7a 100644
@@ -383,4 +383,9 @@ config PUNIT_ATOM_DEBUG
 	  The current power state can be read from
 	  /sys/kernel/debug/punit_atom/dev_power_state
 
+config ARCHINFO
+	tristate "x86 archinfo dumper module"
+	---help---
+	  Dump interesting x86 arch stuff.
+
 endmenu
arch/x86/kernel/Makefile +1 -0
index b1b78ffe01d0..89972c1b9de6 100644
@@ -109,6 +109,7 @@ obj-$(CONFIG_EFI)			+= sysfb_efi.o
 
 obj-$(CONFIG_PERF_EVENTS)		+= perf_regs.o
 obj-$(CONFIG_TRACING)			+= tracepoint.o
+obj-$(CONFIG_ARCHINFO)			+= archinfo.o
 
 ###
 # 64 bit specific files
arch/x86/kernel/archinfo.c new file +215 -0

215 lines not on this page · raw message

Signature · 8 lines
2.3.5


-- 
Regards/Gruss,
    Boris.

ECO tip #101: Trim your mails when you reply.
#6 lore raw

Re: [PATCH -v2] x86: Add an archinfo dumper module

Quoted 10 lines · Borislav Petkov

Here's v2 with the stuff we talked about, implemented. I've added 'control_regs' file too so that you can do:

$ cat /sys/kernel/debug/x86/archinfo/control_regs
CR4: [-|-|SMEP|OSXSAVE|-|-|-|-|OSXMMEXCPT|OSFXSR|-|PGE|MCE|PAE|PSE|-|-|-|-]: 0x1406f0

for example. Yeah, only CR4 right now.

Off the top of my head, we would need "msrs" which dumps EFER and a bunch of other interesting MSRs along with the names of the set bits.

It would be nce to have some helper function that makes it easier for people to add new registers to this that just uses one string to describe the format of a register. E.g. if we have a register like this:

	 63 63 62  11 10  9 8    5 4   2 1   0
	+-----+------+-----+------+-----+-----+
	| VAL | rsvd | BAZ | rsvd | BAR | FOO |
	+-----+------+-----+------+-----+-----+

	BAZ bits mean:
		00 - nope
		01 - low
		10 - mid
		11 - high

we could describe it with:

"1fVAL|52r|2[nope,low,mid,high]BAZ|4r|3xBAR|2dFOO"

syntax is <width><fmt><name>

where width is the number of bits. Format is one of:

f - flag - print the name if the field is non-zero, else print nothing
(or "-" to match the format you used above for CR4)
r - reserved - skip this field
d - print name=%d
x - print name=0x%x
[str0,str1,...str{2^width - 1}] - print NAME=str[val]

-Tony

author maintainer
#7 lore raw

Re: [PATCH -v2] x86: Add an archinfo dumper module

Oh wow,

I'm back and reading emails and I can see that Tony has had fun parsing strings for a change!

:-)))

Quoted 34 lines · Tony
Quoted 10 lines · Borislav Petkov

Here's v2 with the stuff we talked about, implemented. I've added 'control_regs' file too so that you can do:

$ cat /sys/kernel/debug/x86/archinfo/control_regs
CR4: [-|-|SMEP|OSXSAVE|-|-|-|-|OSXMMEXCPT|OSFXSR|-|PGE|MCE|PAE|PSE|-|-|-|-]: 0x1406f0

for example. Yeah, only CR4 right now.

Off the top of my head, we would need "msrs" which dumps EFER and a bunch of other interesting MSRs along with the names of the set bits.

It would be nce to have some helper function that makes it easier for people to add new registers to this that just uses one string to describe the format of a register. E.g. if we have a register like this:

	 63 63 62  11 10  9 8    5 4   2 1   0
	+-----+------+-----+------+-----+-----+
	| VAL | rsvd | BAZ | rsvd | BAR | FOO |
	+-----+------+-----+------+-----+-----+

	BAZ bits mean:
		00 - nope
		01 - low
		10 - mid
		11 - high

we could describe it with:

"1fVAL|52r|2[nope,low,mid,high]BAZ|4r|3xBAR|2dFOO"

syntax is <width><fmt><name>

Quoted 8 lines

where width is the number of bits. Format is one of:

f - flag - print the name if the field is non-zero, else print nothing
(or "-" to match the format you used above for CR4)
r - reserved - skip this field
d - print name=%d
x - print name=0x%x
[str0,str1,...str{2^width - 1}] - print NAME=str[val]

Right, this sounds real nice.

What I was going to propose, though, was to simplify the parsing by doing this:

struct reg_range {
	const char * const names;
	unsigned flags;
	unsigned len;
};

which describes a bit slice of the register and then do this:

const struct reg_range reg_descriptor[] = {
	{ TYPE_FLAG,	  1,		{ "VAL" } },
	{ TYPE_RSVD,	  62 - 11 + 1,	{"rsvd" } },
	{ TYPE_ALTERNATE, 2,		{ "nope", "low", "mid", high" } },
	{ TYPE_HEX,	  3,		{ "BAR" } },
	{ TYPE_DEC,	  2,		{ "FOO" } },
};

Then, the parsing code would simply do:

	for (i = ARRAY_SIZE(reg_descriptor) - 1; i >= 0; i--)
		dump_range(reg_descriptor[i]);

with all the logic in dump_range().

The advantage is that you don't have to do any string parsing which might be problematic in some cases and when typing the register descriptor, you can be very easy exact on bit length and which bits by typing the values directly from the manuals. And you can do lazy stuff like "62-11+1" above in case you don't want to count reserved bits, especially if they're trailing nybbles and such fun...

Signature · 4 lines
Regards/Gruss,
    Boris.

ECO tip #101: Trim your mails when you reply.
#8 lore raw

Re: [PATCH -v2] x86: Add an archinfo dumper module

Quoted 25 lines

What I was going to propose, though, was to simplify the parsing by doing this:

struct reg_range {
	const char * const names;
	unsigned flags;
	unsigned len;
};

which describes a bit slice of the register and then do this:

const struct reg_range reg_descriptor[] = {
	{ TYPE_FLAG,	  1,		{ "VAL" } },
	{ TYPE_RSVD,	  62 - 11 + 1,	{"rsvd" } },
	{ TYPE_ALTERNATE, 2,		{ "nope", "low", "mid", high" } },
	{ TYPE_HEX,	  3,		{ "BAR" } },
	{ TYPE_DEC,	  2,		{ "FOO" } },
};

Then, the parsing code would simply do:

	for (i = ARRAY_SIZE(reg_descriptor) - 1; i >= 0; i--)
		dump_range(reg_descriptor[i]);
Quoted 8 lines

with all the logic in dump_range().

The advantage is that you don't have to do any string parsing which might be problematic in some cases and when typing the register descriptor, you can be very easy exact on bit length and which bits by typing the values directly from the manuals. And you can do lazy stuff like "62-11+1" above in case you don't want to count reserved bits, especially if they're trailing nybbles and such fun...

There is a lot of bit counting and typing either way. My string format is visually compact, and looks quite similar to the eventual output.

Your reg_range does allow you to pass counting to the compiler in the case that the documentation gives you highbit/lowbit ranges. But most fields are small enough that yuo don't even need to take your socks off to count ... so I don't see it as a huge deal.

Both formats allow for a sanity check that all the bitfields add up to 64 ... which will detect single errors (which your code for my example would fail because you missed the second reserved field) and only have 60 bits described).

-Tony

author maintainer
#9 lore raw

Re: [PATCH -v2] x86: Add an archinfo dumper module

Quoted 3 lines · Tony

There is a lot of bit counting and typing either way. My string format is visually compact, and looks quite similar to the eventual output.

Except if you have 64 all single bits and all defined. Then that thing:

+static char *cr4_format = +"41r|PKE|SMAP|SMEP|1r|OSXSAVE|PCIDE|FSGSBASE|1r|SMXE|VMXE|2r|OSXMMEXCPT|OSFXSR|PCE|PGE|MCE|PAE|PSE|DE|TSD|PVI|VME";

triples. The array approach is going to be long too but in the vertical and still visually parseable.

Quoted 10 lines

Your reg_range does allow you to pass counting to the compiler in the case that the documentation gives you highbit/lowbit ranges. But most fields are small enough that yuo don't even need to take your socks off to count ... so I don't see it as a huge deal.

Both formats allow for a sanity check that all the bitfields add up to 64 ... which will detect single errors (which your code for my example would fail because you missed the second reserved field) and only have 60 bits described).

The code iterating over reg_descriptor can check that, of course.

So the only thing I'm trying to avoid is string parsing - if you add all the corner cases handling and more field syntax, then the whole parsing game could become pretty complex and maybe even fragile.

Not with the range descriptors - that remains simple. And I like simple.

Signature · 4 lines
Regards/Gruss,
    Boris.

ECO tip #101: Trim your mails when you reply.
#10 lore raw

Re: [PATCH -v2] x86: Add an archinfo dumper module

In reply to Borislav Petkov

Patch on top of your v2 that defines a register priting function based on a single string format descriptor. CR4 changed to use it.

 arch/x86/kernel/archinfo.c |  135 ++++++++++++++++++++++++++++++++++++++-------
 1 file changed, 114 insertions(+), 21 deletions(-)

We could break even on code lines with about four more registers using this!

I think it could print the GDT (though the format string will be very long).

Probably could use some sanity checks and cleanup. Maybe there might be some more useful format cases for other registers.

-Tony

---

arch/x86/kernel/archinfo.c +114 -21
index c04e98625565..3374e1307a96 100644
@@ -132,31 +132,124 @@ static int gdt_open(struct inode *inode, struct file *filp)
 	return single_open(filp, gdt_show, NULL);
 }
 
+/*
+ * find the field name, and its length
+ */
+static char *get_fname(char *s, char *e, unsigned long val, int *flen)
+{
+	char *fe;
+
+	while (val--) {
+		s = strnchr(s, e - s, ',');
+		if (s >= e) {
+			*flen = 7;
+			return "unknown";
+		}
+		s++;
+	}
+	fe = strnchr(s, e - s, ',');
+	if (fe)
+		*flen = fe - s;
+	else
+		*flen = e - s;
+	return s;
+}
+
+/*
+ * print a value consisting of several bitfields in a human readable way
+ *
+ * Format string has a ":" separated set of descriptions from LSB to MSB
+ * interpreted like this:
+ *	<width>fmtNAME
+ *	width is the number of bits in this field
+ *	fmt is either a single character:
+ *		f - print the name of the field if the field value is non-zero
+ *		r - reserved field - skip it
+ *		d - Print NAME=%d
+ *		x - Print NAME=0x%x
+ *	or a set of comma separated strings inside [str0,str1,...strN]
+ *		print NAME=%s
+ *	If width is missing, default to "1f"
+ */
+static void show_reg_bits(struct seq_file *m, char *regname, char *fmt, unsigned long val)
+{
+	char *endptr, *s_fnames = NULL, *e_fnames = NULL, *f;
+	int nbits, totbits = 0;
+	int mode, flen, namelen;
+	unsigned long fval;
+	unsigned long orig_val = val;
+
+	seq_printf(m, "%s: [", regname);
+loop:
+	nbits = kstrtol(fmt, &endptr, 10);
+	if (endptr == fmt) {
+		nbits = 1;
+		mode = 'f';
+	} else {
+		switch (*endptr) {
+		case 'f': case 'r': case 'd': case 'x':
+			mode = *endptr;
+			endptr++;
+			break;
+		case '[':
+			mode = *endptr;
+			s_fnames = endptr + 1;
+			endptr = strchr(endptr, ']');
+			if (endptr) {
+				e_fnames = endptr;
+				endptr++;
+			} else {
+				seq_puts(m, "Missing ']'\n");
+				return;
+			}
+			break;
+		default:
+			seq_printf(m, "Unknown format '%c'\n", *endptr);
+			return;
+		}
+	}
+	totbits += nbits;
+	fmt = strchr(endptr, '|');
+	namelen = fmt ? fmt - endptr : strlen(endptr);
+
+	fval = val >> (64 - nbits);
+	switch (mode) {
+	case 'f':
+		if (fval)
+			seq_printf(m, "%.*s", namelen, endptr);
+		else
+			seq_puts(m, "-");
+		break;
+	case 'd':
+		seq_printf(m, "%.*s=%ld", namelen, endptr, fval);
+		break;
+	case 'x':
+		seq_printf(m, "%.*s=0x%lx", namelen, endptr, fval);
+		break;
+	case '[':
+		f = get_fname(s_fnames, e_fnames, fval, &flen);
+		seq_printf(m, "%.*s=%.*s", namelen, endptr, flen, f);
+	}
+
+	if (fmt) {
+		fmt++;
+		val <<= nbits;
+		if (mode != 'r')
+			seq_puts(m, "|");
+		goto loop;
+	}
+	if (totbits != 64)
+		seq_printf(m, "{format described %d bits}", totbits);
+	seq_printf(m, "]: 0x%lx\n", orig_val);
+}
+
+static char *cr4_format = "41r|PKE|SMAP|SMEP|1r|OSXSAVE|PCIDE|FSGSBASE|1r|SMXE|VMXE|2r|OSXMMEXCPT|OSFXSR|PCE|PGE|MCE|PAE|PSE|DE|TSD|PVI|VME";
+
 static int cr_show(struct seq_file *m, void *v)
 {
 	unsigned long cr4 = __read_cr4();
 
-	seq_printf(m, "CR4: [%s|%s|%s|%s|%s|%s|%s|%s|%s|%s|%s|%s|%s|%s|%s|%s|%s|%s|%s]: 0x%lx\n",
-		      (cr4 & BIT(22) ? "PKE"		: "-"),
-		      (cr4 & BIT(21) ? "SMAP"		: "-"),
-		      (cr4 & BIT(20) ? "SMEP"		: "-"),
-		      (cr4 & BIT(18) ? "OSXSAVE"	: "-"),
-		      (cr4 & BIT(17) ? "PCIDE"		: "-"),
-		      (cr4 & BIT(16) ? "FSGSBASE"	: "-"),
-		      (cr4 & BIT(14) ? "SMXE"		: "-"),
-		      (cr4 & BIT(13) ? "VMXE"		: "-"),
-		      (cr4 & BIT(10) ? "OSXMMEXCPT"	: "-"),
-		      (cr4 & BIT(9)  ? "OSFXSR"		: "-"),
-		      (cr4 & BIT(8)  ? "PCE"		: "-"),
-		      (cr4 & BIT(7)  ? "PGE"		: "-"),
-		      (cr4 & BIT(6)  ? "MCE"		: "-"),
-		      (cr4 & BIT(5)  ? "PAE"		: "-"),
-		      (cr4 & BIT(4)  ? "PSE"		: "-"),
-		      (cr4 & BIT(3)  ? "DE"		: "-"),
-		      (cr4 & BIT(2)  ? "TSD"		: "-"),
-		      (cr4 & BIT(1)  ? "PVI"		: "-"),
-		      (cr4 & BIT(0)  ? "VME"		: "-"),
-		      cr4);
+	show_reg_bits(m, "CR4", cr4_format, cr4);
 
 	return 0;
 }
Signature · 1 line
2.5.0
#11 lore raw

RE: [PATCH -v2] x86: Add an archinfo dumper module

+ show_reg_bits(m, "CR4", cr4_format, cr4);

Can %pXX formats use more than one argument? If so, we might be able to move all my code to lib/vsprintf.c and just write:

	seq_printf(m, "CR4: %pBITS: 0x%lx\n", cr4_format, cr4, cr4);

If they can't, we could bundle the format and value into a structure:

struct printbits {
	char *fmt;
	u64 val;
};
{
	struct printbits p;

	p.fmt = cr4_format;
	p.val = __read_cr4();
	seq_printf(m, "CR4: %pBITS: 0x%lx\n", &p, p.val);
}

-Tony

#12 lore raw

Re: [PATCH -v2] x86: Add an archinfo dumper module

In reply to Borislav Petkov
Quoted 10 lines · Borislav Petkov

Here's v2 with the stuff we talked about, implemented. I've added 'control_regs' file too so that you can do:

$ cat /sys/kernel/debug/x86/archinfo/control_regs
CR4: [-|-|SMEP|OSXSAVE|-|-|-|-|OSXMMEXCPT|OSFXSR|-|PGE|MCE|PAE|PSE|-|-|-|-]: 0x1406f0

for example. Yeah, only CR4 right now.

Off the top of my head, we would need "msrs" which dumps EFER and a bunch of other interesting MSRs along with the names of the set bits.

Is there a reason why all this parsing has to be done in kernel space?

-hpa

author maintainer
#13 lore raw

Re: [PATCH -v2] x86: Add an archinfo dumper module

Quoted 1 line · H. Peter Anvin

Is there a reason why all this parsing has to be done in kernel space?

Not at all. What do you have in mind?

Signature · 1 line
Sent from a small device: formatting sux and brevity is inevitable. 
#14 lore raw

Re: [PATCH -v2] x86: Add an archinfo dumper module

In reply to H. Peter Anvin
Quoted 7 lines · H. Peter Anvin
Quoted 6 lines · Borislav Petkov

Here's v2 with the stuff we talked about, implemented. I've added 'control_regs' file too so that you can do:

$ cat /sys/kernel/debug/x86/archinfo/control_regs
CR4: [-|-|SMEP|OSXSAVE|-|-|-|-|OSXMMEXCPT|OSFXSR|-|PGE|MCE|PAE|PSE|-|-|-|-]: 0x1406f0
Quoted 6 lines
Quoted 4 lines

for example. Yeah, only CR4 right now.

Off the top of my head, we would need "msrs" which dumps EFER and a bunch of other interesting MSRs along with the names of the set bits.

Is there a reason why all this parsing has to be done in kernel space?

Ease of use I suspect, no need to locate some separate utility. Also, no need to define an ABI for the information displayed - which keeps things simpler. Since it's a kernel module there's no real bloat argument.

Thanks,

Ingo

author maintainer
#15 lore raw

Re: [PATCH -v2] x86: Add an archinfo dumper module

Quoted 1 line · Ingo Molnar

Ease of use I suspect, no need to locate some separate utility.

Yeah, that is IMO the main reason which speaks against parsing in userspace.

Quoted 3 lines

Also, no need to define an ABI for the information displayed - which keeps things simpler. Since it's a kernel module there's no real bloat argument.

... and ontop it is a debugging aid module so won't be present on production systems.

Signature · 4 lines
Regards/Gruss,
    Boris.

ECO tip #101: Trim your mails when you reply.