linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [RFC] Dump interesting arch/platform info
@ 2016-02-01 11:56 Borislav Petkov
  2016-02-01 23:29 ` Luck, Tony
  0 siblings, 1 reply; 16+ messages in thread
From: Borislav Petkov @ 2016-02-01 11:56 UTC (permalink / raw)
  To: H. Peter Anvin, Ingo Molnar, Thomas Gleixner
  Cc: Andy Lutomirski, Peter Zijlstra, Steven Rostedt, Tony Luck,
	Brian Gerst, lkml

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 <bp@suse.de>
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.

Signed-off-by: Borislav Petkov <bp@suse.de>
---
 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

diff --git a/arch/x86/Kconfig.debug b/arch/x86/Kconfig.debug
index 9b18ed97a8a2..c757d1ee9a7a 100644
--- a/arch/x86/Kconfig.debug
+++ b/arch/x86/Kconfig.debug
@@ -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
diff --git a/arch/x86/kernel/Makefile b/arch/x86/kernel/Makefile
index b1b78ffe01d0..89972c1b9de6 100644
--- a/arch/x86/kernel/Makefile
+++ b/arch/x86/kernel/Makefile
@@ -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
diff --git a/arch/x86/kernel/archinfo.c b/arch/x86/kernel/archinfo.c
new file mode 100644
index 000000000000..790ba8413895
--- /dev/null
+++ b/arch/x86/kernel/archinfo.c
@@ -0,0 +1,164 @@
+#include <linux/debugfs.h>
+#include <linux/module.h>
+#include <linux/cpu.h>
+
+#include <asm/desc.h>
+
+static const char * const system_desc_types[] = {
+	[0] = "Reserved (illegal)",
+	[1] = "Available 16-bit TSS",
+	[2] = "LDT",
+	[3] = "Busy 16-bit TSS",
+	[4] = "16-bit Call Gate",
+	[5] = "Task Gate",
+	[6] = "16-bit Interrupt Gate",
+	[7] = "16-bit Trap Gate",
+	[8] = "Reserved (illegal)",
+	[9] = "Available 32-bit TSS",
+	[10] = "Reserved (illegal)",
+	[11] = "Busy 32-bit TSS",
+	[12] = "32-bit Call Gate",
+	[13] = "Reserved (illegal)",
+	[14] = "32-bit Interrupt Gate",
+	[15] = "32-bit Trap Gate",
+};
+
+static const char * const user_desc_types[] = {
+	[0] = "Read-Only",
+	[1] = "Read-only - Accessed",
+	[2] = "Read/Write",
+	[3] = "Read/Write - Accessed",
+	[4] = "Expand-down, Read-Only",
+	[5] = "Expand-down, Read-Only - Accessed",
+	[6] = "Expand-down, Read-Write",
+	[7] = "Expand-down, Read-Write - Accessed",
+	[8] = "Execute-Only",
+	[9] = "Execute-Only - Accessed",
+	[10] = "Execute/Readable",
+	[11] = "Execute/Readable - Accessed",
+	[12] = "Conforming, Execute-Only",
+	[13] = "Conforming, Execute-Only - Accessed",
+	[14] = "Conforming, Execute/Readable",
+	[15] = "Conforming, Execute/Readable - Accessed",
+};
+
+static void print_seg_desc(struct seq_file *m, struct desc_struct *d, int num)
+{
+	seq_printf(m, "%02d:\n", num);
+	seq_printf(m, "[ base[31:24]:%02x G:%x D:%x L:%x AVL:%x lim[19:16]:%x |",
+		   d->base2, d->g, d->d, d->l, d->avl, d->limit);
+	seq_printf(m, " P:%x DPL:%x S:%x C:%x base[23:16]:%02x ]\n",
+		   d->p, d->dpl, d->s, !!(d->type & BIT(2)), d->base1);
+	seq_printf(m, "[ base[15:00]:%04x | lim[15:00]:%04x ]: ",
+		   d->base0, d->limit0);
+
+	if (d->s)
+		seq_printf(m, "User: (0x%x) %s, %s\n",
+			    d->type,
+			   (d->type > 7 ? "Code" : "Data"),
+			   (user_desc_types[d->type]));
+	else
+		seq_printf(m, "System: (0x%x) %s\n", d->type, system_desc_types[d->type]);
+
+	seq_printf(m, "\n");
+}
+
+static void dump_gdt(void *info)
+{
+	struct gdt_page *g = this_cpu_ptr(&gdt_page);
+	struct seq_file *m = (struct seq_file *)info;
+	int i;
+
+	seq_printf(m, "CPU%d, GDT %p:\n", smp_processor_id(), &g->gdt);
+
+	for (i = 0; i < GDT_ENTRIES; i++)
+		print_seg_desc(m, &g->gdt[i], i);
+
+	seq_printf(m, "----\n");
+
+}
+
+static int archinfo_show(struct seq_file *m, void *v)
+{
+	int c;
+
+	/*
+	 * Using on_each_cpu() here fudges the output and we want it nicely
+	 * sorted by CPU.
+	 */
+	get_online_cpus();
+		for_each_online_cpu(c)
+			smp_call_function_single(c, dump_gdt, m, 1);
+	put_online_cpus();
+
+	seq_printf(m,
+		   "\nInfo:\n"
+		   "base,limit,A,G,R: ignored in 64-bit mode.\n"
+		   "G: granularity bit (23):\n"
+			"\t- 0b: segment limit is not scaled.\n"
+			"\t- 1b: segment limit scaled by 4K.\n"
+		   "D/B: CS default operand size bit (22):\n"
+			"\t- 0b: 16-bit.\n"
+			"\t- 1b: 32-bit.\n"
+			"\tD=0b is the only allowed setting in long mode (L=1b).\n"
+			"\tCalled B in stack segments.\n"
+		   "L: long mode bit (21):\n"
+			"\t- 0b: CPU in compat mode. Enables segmentation.\n"
+			"\t- 1b: CPU in long mode.\n"
+		   "AVL: bit available to software (20).\n"
+		   "P: present bit (15):\n"
+			"\t- 0b: seg. not present in mem => #NP.\n"
+			"\t- 1b: seg is present in memory.\n"
+		   "DPL: Descriptor Privilege Level [14:13]:\n"
+			"\t- 0b: highest privilege level.\n"
+			"    ...\n"
+			"\t- 3b: lowest privilege level.\n"
+		   "S+Type: decriptor types [12,11:8]:\n"
+			"\t Specify descriptor type and access characteristics.\n"
+		   " S:\n"
+			"\t- 0b: System descriptor.\n"
+			"\t- 1b: User descriptor.\n"
+		   " R: readable bit (9):\n"
+			"\t- 0b: code seg is executable, reads -> #GP\n"
+			"\t- 1b: code seg is both read/exec\n"
+		   " A: accessed bit (8): set by CPU when desc copied into %%cs; cleared only by sw.\n"
+			);
+
+	return 0;
+}
+
+static int archinfo_open(struct inode *inode, struct file *filp)
+{
+	return single_open(filp, archinfo_show, NULL);
+}
+
+static const struct file_operations archinfo_fops = {
+	.owner	 = THIS_MODULE,
+	.open	 = archinfo_open,
+	.read	 = seq_read,
+	.llseek	 = seq_lseek,
+	.release = single_release,
+};
+
+static struct dentry *dfs_entry;
+
+static int __init archinfo_init(void)
+{
+	dfs_entry = debugfs_create_file("archinfo", S_IRUSR,
+					arch_debugfs_dir, NULL, &archinfo_fops);
+	if (!dfs_entry)
+		return -ENOMEM;
+
+	return 0;
+}
+
+static void __exit archinfo_exit(void)
+{
+	debugfs_remove_recursive(dfs_entry);
+}
+
+module_init(archinfo_init);
+module_exit(archinfo_exit);
+MODULE_LICENSE("GPL");
+MODULE_AUTHOR("Borislav Petkov <bp@alien8.de>");
+MODULE_DESCRIPTION("x86 arch info dumper");
-- 
2.3.5


-- 
Regards/Gruss,
    Boris.

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

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* RE: [RFC] Dump interesting arch/platform info
  2016-02-01 11:56 [RFC] Dump interesting arch/platform info Borislav Petkov
@ 2016-02-01 23:29 ` Luck, Tony
  2016-02-02  9:41   ` Borislav Petkov
  0 siblings, 1 reply; 16+ messages in thread
From: Luck, Tony @ 2016-02-01 23:29 UTC (permalink / raw)
  To: Borislav Petkov, H. Peter Anvin, Ingo Molnar, Thomas Gleixner
  Cc: Andy Lutomirski, Peter Zijlstra, Steven Rostedt, Brian Gerst, lkml

> 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

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [RFC] Dump interesting arch/platform info
  2016-02-01 23:29 ` Luck, Tony
@ 2016-02-02  9:41   ` Borislav Petkov
  2016-02-03 10:48     ` Ingo Molnar
  0 siblings, 1 reply; 16+ messages in thread
From: Borislav Petkov @ 2016-02-02  9:41 UTC (permalink / raw)
  To: Luck, Tony
  Cc: H. Peter Anvin, Ingo Molnar, Thomas Gleixner, Andy Lutomirski,
	Peter Zijlstra, Steven Rostedt, Brian Gerst, lkml

On Mon, Feb 01, 2016 at 11:29:03PM +0000, Luck, Tony wrote:
> 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... :-)

> 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!

-- 
Regards/Gruss,
    Boris.

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

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [RFC] Dump interesting arch/platform info
  2016-02-02  9:41   ` Borislav Petkov
@ 2016-02-03 10:48     ` Ingo Molnar
  2016-02-03 11:00       ` Borislav Petkov
  0 siblings, 1 reply; 16+ messages in thread
From: Ingo Molnar @ 2016-02-03 10:48 UTC (permalink / raw)
  To: Borislav Petkov
  Cc: Luck, Tony, H. Peter Anvin, Thomas Gleixner, Andy Lutomirski,
	Peter Zijlstra, Steven Rostedt, Brian Gerst, lkml


* Borislav Petkov <bp@alien8.de> wrote:

> On Mon, Feb 01, 2016 at 11:29:03PM +0000, Luck, Tony wrote:
> > 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... :-)
> 
> > 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!

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

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [RFC] Dump interesting arch/platform info
  2016-02-03 10:48     ` Ingo Molnar
@ 2016-02-03 11:00       ` Borislav Petkov
  2016-02-04 15:22         ` [PATCH -v2] x86: Add an archinfo dumper module Borislav Petkov
  0 siblings, 1 reply; 16+ messages in thread
From: Borislav Petkov @ 2016-02-03 11:00 UTC (permalink / raw)
  To: Ingo Molnar
  Cc: Luck, Tony, H. Peter Anvin, Thomas Gleixner, Andy Lutomirski,
	Peter Zijlstra, Steven Rostedt, Brian Gerst, lkml

On Wed, Feb 03, 2016 at 11:48:23AM +0100, Ingo Molnar wrote:
> 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.

-- 
Regards/Gruss,
    Boris.

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

^ permalink raw reply	[flat|nested] 16+ messages in thread

* [PATCH -v2] x86: Add an archinfo dumper module
  2016-02-03 11:00       ` Borislav Petkov
@ 2016-02-04 15:22         ` Borislav Petkov
  2016-02-04 19:07           ` Luck, Tony
                             ` (2 more replies)
  0 siblings, 3 replies; 16+ messages in thread
From: Borislav Petkov @ 2016-02-04 15:22 UTC (permalink / raw)
  To: Ingo Molnar
  Cc: Luck, Tony, H. Peter Anvin, Thomas Gleixner, Andy Lutomirski,
	Peter Zijlstra, Steven Rostedt, Brian Gerst, lkml

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 <bp@suse.de>
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.

Signed-off-by: Borislav Petkov <bp@suse.de>
---
 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

diff --git a/arch/x86/Kconfig.debug b/arch/x86/Kconfig.debug
index 9b18ed97a8a2..c757d1ee9a7a 100644
--- a/arch/x86/Kconfig.debug
+++ b/arch/x86/Kconfig.debug
@@ -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
diff --git a/arch/x86/kernel/Makefile b/arch/x86/kernel/Makefile
index b1b78ffe01d0..89972c1b9de6 100644
--- a/arch/x86/kernel/Makefile
+++ b/arch/x86/kernel/Makefile
@@ -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
diff --git a/arch/x86/kernel/archinfo.c b/arch/x86/kernel/archinfo.c
new file mode 100644
index 000000000000..c04e98625565
--- /dev/null
+++ b/arch/x86/kernel/archinfo.c
@@ -0,0 +1,215 @@
+#include <linux/debugfs.h>
+#include <linux/module.h>
+#include <linux/cpu.h>
+
+#include <asm/desc.h>
+
+static const char * const system_desc_types[] = {
+	[0] = "Reserved (illegal)",
+	[1] = "Available 16-bit TSS",
+	[2] = "LDT",
+	[3] = "Busy 16-bit TSS",
+	[4] = "16-bit Call Gate",
+	[5] = "Task Gate",
+	[6] = "16-bit Interrupt Gate",
+	[7] = "16-bit Trap Gate",
+	[8] = "Reserved (illegal)",
+	[9] = "Available 32-bit TSS",
+	[10] = "Reserved (illegal)",
+	[11] = "Busy 32-bit TSS",
+	[12] = "32-bit Call Gate",
+	[13] = "Reserved (illegal)",
+	[14] = "32-bit Interrupt Gate",
+	[15] = "32-bit Trap Gate",
+};
+
+static const char * const user_desc_types[] = {
+	[0] = "Read-Only",
+	[1] = "Read-only - Accessed",
+	[2] = "Read/Write",
+	[3] = "Read/Write - Accessed",
+	[4] = "Expand-down, Read-Only",
+	[5] = "Expand-down, Read-Only - Accessed",
+	[6] = "Expand-down, Read-Write",
+	[7] = "Expand-down, Read-Write - Accessed",
+	[8] = "Execute-Only",
+	[9] = "Execute-Only - Accessed",
+	[10] = "Execute/Readable",
+	[11] = "Execute/Readable - Accessed",
+	[12] = "Conforming, Execute-Only",
+	[13] = "Conforming, Execute-Only - Accessed",
+	[14] = "Conforming, Execute/Readable",
+	[15] = "Conforming, Execute/Readable - Accessed",
+};
+
+static void print_seg_desc(struct seq_file *m, struct desc_struct *d, int num)
+{
+	seq_printf(m, "%02d:\n", num);
+	seq_printf(m, "[ base[31:24]:%02x G:%x D:%x L:%x AVL:%x lim[19:16]:%x |",
+		   d->base2, d->g, d->d, d->l, d->avl, d->limit);
+	seq_printf(m, " P:%x DPL:%x S:%x C:%x base[23:16]:%02x ]\n",
+		   d->p, d->dpl, d->s, !!(d->type & BIT(2)), d->base1);
+	seq_printf(m, "[ base[15:00]:%04x | lim[15:00]:%04x ]: ",
+		   d->base0, d->limit0);
+
+	if (d->s)
+		seq_printf(m, "User: (0x%x) %s, %s\n",
+			    d->type,
+			   (d->type > 7 ? "Code" : "Data"),
+			   (user_desc_types[d->type]));
+	else
+		seq_printf(m, "System: (0x%x) %s\n", d->type, system_desc_types[d->type]);
+
+	seq_printf(m, "\n");
+}
+
+static void dump_gdt(void *info)
+{
+	struct gdt_page *g = this_cpu_ptr(&gdt_page);
+	struct seq_file *m = (struct seq_file *)info;
+	int i;
+
+	seq_printf(m, "CPU%d, GDT %p:\n", smp_processor_id(), &g->gdt);
+
+	for (i = 0; i < GDT_ENTRIES; i++)
+		print_seg_desc(m, &g->gdt[i], i);
+
+	seq_printf(m, "----\n");
+
+}
+
+static int gdt_show(struct seq_file *m, void *v)
+{
+	int c;
+
+	/*
+	 * Using on_each_cpu() here fudges the output and we want it nicely
+	 * sorted by CPU.
+	 */
+	get_online_cpus();
+		for_each_online_cpu(c)
+			smp_call_function_single(c, dump_gdt, m, 1);
+	put_online_cpus();
+
+	seq_printf(m,
+		   "\nInfo:\n"
+		   "base,limit,A,G,R: ignored in 64-bit mode.\n"
+		   "G: granularity bit (23):\n"
+			"\t- 0b: segment limit is not scaled.\n"
+			"\t- 1b: segment limit scaled by 4K.\n"
+		   "D/B: CS default operand size bit (22):\n"
+			"\t- 0b: 16-bit.\n"
+			"\t- 1b: 32-bit.\n"
+			"\tD=0b is the only allowed setting in long mode (L=1b).\n"
+			"\tCalled B in stack segments.\n"
+		   "L: long mode bit (21):\n"
+			"\t- 0b: CPU in compat mode. Enables segmentation.\n"
+			"\t- 1b: CPU in long mode.\n"
+		   "AVL: bit available to software (20).\n"
+		   "P: present bit (15):\n"
+			"\t- 0b: seg. not present in mem => #NP.\n"
+			"\t- 1b: seg is present in memory.\n"
+		   "DPL: Descriptor Privilege Level [14:13]:\n"
+			"\t- 0b: highest privilege level.\n"
+			"    ...\n"
+			"\t- 3b: lowest privilege level.\n"
+		   "S+Type: decriptor types [12,11:8]:\n"
+			"\t Specify descriptor type and access characteristics.\n"
+		   " S:\n"
+			"\t- 0b: System descriptor.\n"
+			"\t- 1b: User descriptor.\n"
+		   " R: readable bit (9):\n"
+			"\t- 0b: code seg is executable, reads -> #GP\n"
+			"\t- 1b: code seg is both read/exec\n"
+		   " A: accessed bit (8): set by CPU when desc copied into %%cs; cleared only by sw.\n"
+			);
+
+	return 0;
+}
+
+static int gdt_open(struct inode *inode, struct file *filp)
+{
+	return single_open(filp, gdt_show, NULL);
+}
+
+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);
+
+	return 0;
+}
+
+static int cr_open(struct inode *inode, struct file *filp)
+{
+	return single_open(filp, cr_show, NULL);
+}
+
+static const struct file_operations cr_fops = {
+	.owner	 = THIS_MODULE,
+	.open	 = cr_open,
+	.read	 = seq_read,
+	.llseek	 = seq_lseek,
+	.release = single_release,
+};
+
+static const struct file_operations gdt_fops = {
+	.owner	 = THIS_MODULE,
+	.open	 = gdt_open,
+	.read	 = seq_read,
+	.llseek	 = seq_lseek,
+	.release = single_release,
+};
+
+static struct dentry *dfs_entry;
+
+static int __init archinfo_init(void)
+{
+	dfs_entry = debugfs_create_dir("archinfo", arch_debugfs_dir);
+	if (IS_ERR_OR_NULL(dfs_entry))
+		return -EINVAL;
+
+	if (!debugfs_create_file("gdt", S_IRUSR, dfs_entry, NULL, &gdt_fops)) {
+		debugfs_remove_recursive(dfs_entry);
+		return -EINVAL;
+	}
+
+	if (!debugfs_create_file("control_regs", S_IRUSR, dfs_entry, NULL, &cr_fops)) {
+		debugfs_remove_recursive(dfs_entry);
+		return -EINVAL;
+	}
+
+	return 0;
+}
+
+static void __exit archinfo_exit(void)
+{
+	debugfs_remove_recursive(dfs_entry);
+}
+
+module_init(archinfo_init);
+module_exit(archinfo_exit);
+MODULE_LICENSE("GPL");
+MODULE_AUTHOR("Borislav Petkov <bp@alien8.de>");
+MODULE_DESCRIPTION("x86 arch info dumper");
-- 
2.3.5


-- 
Regards/Gruss,
    Boris.

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

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* Re: [PATCH -v2] x86: Add an archinfo dumper module
  2016-02-04 15:22         ` [PATCH -v2] x86: Add an archinfo dumper module Borislav Petkov
@ 2016-02-04 19:07           ` Luck, Tony
  2016-02-07 10:51             ` Borislav Petkov
  2016-02-05 19:51           ` Luck, Tony
  2016-02-08  0:01           ` H. Peter Anvin
  2 siblings, 1 reply; 16+ messages in thread
From: Luck, Tony @ 2016-02-04 19:07 UTC (permalink / raw)
  To: Borislav Petkov
  Cc: Ingo Molnar, H. Peter Anvin, Thomas Gleixner, Andy Lutomirski,
	Peter Zijlstra, Steven Rostedt, Brian Gerst, lkml

On Thu, Feb 04, 2016 at 04:22:35PM +0100, Borislav Petkov wrote:
> 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

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH -v2] x86: Add an archinfo dumper module
  2016-02-04 15:22         ` [PATCH -v2] x86: Add an archinfo dumper module Borislav Petkov
  2016-02-04 19:07           ` Luck, Tony
@ 2016-02-05 19:51           ` Luck, Tony
  2016-02-05 22:24             ` Luck, Tony
  2016-02-08  0:01           ` H. Peter Anvin
  2 siblings, 1 reply; 16+ messages in thread
From: Luck, Tony @ 2016-02-05 19:51 UTC (permalink / raw)
  To: Borislav Petkov
  Cc: Ingo Molnar, H. Peter Anvin, Thomas Gleixner, Andy Lutomirski,
	Peter Zijlstra, Steven Rostedt, Brian Gerst, lkml

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


---

diff --git a/arch/x86/kernel/archinfo.c b/arch/x86/kernel/archinfo.c
index c04e98625565..3374e1307a96 100644
--- a/arch/x86/kernel/archinfo.c
+++ b/arch/x86/kernel/archinfo.c
@@ -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;
 }
-- 
2.5.0

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* RE: [PATCH -v2] x86: Add an archinfo dumper module
  2016-02-05 19:51           ` Luck, Tony
@ 2016-02-05 22:24             ` Luck, Tony
  0 siblings, 0 replies; 16+ messages in thread
From: Luck, Tony @ 2016-02-05 22:24 UTC (permalink / raw)
  To: Luck, Tony, Borislav Petkov
  Cc: Ingo Molnar, H. Peter Anvin, Thomas Gleixner, Andy Lutomirski,
	Peter Zijlstra, Steven Rostedt, Brian Gerst, lkml

+	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

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH -v2] x86: Add an archinfo dumper module
  2016-02-04 19:07           ` Luck, Tony
@ 2016-02-07 10:51             ` Borislav Petkov
  2016-02-09 19:17               ` Luck, Tony
  0 siblings, 1 reply; 16+ messages in thread
From: Borislav Petkov @ 2016-02-07 10:51 UTC (permalink / raw)
  To: Luck, Tony
  Cc: Ingo Molnar, H. Peter Anvin, Thomas Gleixner, Andy Lutomirski,
	Peter Zijlstra, Steven Rostedt, Brian Gerst, lkml

Oh wow,

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

:-)))

On Thu, Feb 04, 2016 at 11:07:57AM -0800, Luck, Tony wrote:
> On Thu, Feb 04, 2016 at 04:22:35PM +0100, Borislav Petkov wrote:
> > 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]

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...



-- 
Regards/Gruss,
    Boris.

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

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH -v2] x86: Add an archinfo dumper module
  2016-02-04 15:22         ` [PATCH -v2] x86: Add an archinfo dumper module Borislav Petkov
  2016-02-04 19:07           ` Luck, Tony
  2016-02-05 19:51           ` Luck, Tony
@ 2016-02-08  0:01           ` H. Peter Anvin
  2016-02-08  7:50             ` Boris Petkov
  2016-02-09 12:23             ` Ingo Molnar
  2 siblings, 2 replies; 16+ messages in thread
From: H. Peter Anvin @ 2016-02-08  0:01 UTC (permalink / raw)
  To: Borislav Petkov, Ingo Molnar
  Cc: Luck, Tony, Thomas Gleixner, Andy Lutomirski, Peter Zijlstra,
	Steven Rostedt, Brian Gerst, lkml

On 02/04/16 07:22, Borislav Petkov wrote:
> 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

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH -v2] x86: Add an archinfo dumper module
  2016-02-08  0:01           ` H. Peter Anvin
@ 2016-02-08  7:50             ` Boris Petkov
  2016-02-09 12:23             ` Ingo Molnar
  1 sibling, 0 replies; 16+ messages in thread
From: Boris Petkov @ 2016-02-08  7:50 UTC (permalink / raw)
  To: H. Peter Anvin, Ingo Molnar
  Cc: Luck, Tony, Thomas Gleixner, Andy Lutomirski, Peter Zijlstra,
	Steven Rostedt, Brian Gerst, lkml

"H. Peter Anvin" <hpa@zytor.com> wrote:
>Is there a reason why all this parsing has to be done in kernel space?

Not at all. What do you have in mind?


-- 
Sent from a small device: formatting sux and brevity is inevitable. 

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH -v2] x86: Add an archinfo dumper module
  2016-02-08  0:01           ` H. Peter Anvin
  2016-02-08  7:50             ` Boris Petkov
@ 2016-02-09 12:23             ` Ingo Molnar
  2016-02-09 14:01               ` Borislav Petkov
  1 sibling, 1 reply; 16+ messages in thread
From: Ingo Molnar @ 2016-02-09 12:23 UTC (permalink / raw)
  To: H. Peter Anvin
  Cc: Borislav Petkov, Luck, Tony, Thomas Gleixner, Andy Lutomirski,
	Peter Zijlstra, Steven Rostedt, Brian Gerst, lkml


* H. Peter Anvin <hpa@zytor.com> wrote:

> On 02/04/16 07:22, Borislav Petkov wrote:
> > 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?

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

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH -v2] x86: Add an archinfo dumper module
  2016-02-09 12:23             ` Ingo Molnar
@ 2016-02-09 14:01               ` Borislav Petkov
  0 siblings, 0 replies; 16+ messages in thread
From: Borislav Petkov @ 2016-02-09 14:01 UTC (permalink / raw)
  To: Ingo Molnar
  Cc: H. Peter Anvin, Luck, Tony, Thomas Gleixner, Andy Lutomirski,
	Peter Zijlstra, Steven Rostedt, Brian Gerst, lkml

On Tue, Feb 09, 2016 at 01:23:00PM +0100, Ingo Molnar wrote:
> 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.

> 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.

-- 
Regards/Gruss,
    Boris.

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

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH -v2] x86: Add an archinfo dumper module
  2016-02-07 10:51             ` Borislav Petkov
@ 2016-02-09 19:17               ` Luck, Tony
  2016-02-09 19:46                 ` Borislav Petkov
  0 siblings, 1 reply; 16+ messages in thread
From: Luck, Tony @ 2016-02-09 19:17 UTC (permalink / raw)
  To: Borislav Petkov
  Cc: Ingo Molnar, H. Peter Anvin, Thomas Gleixner, Andy Lutomirski,
	Peter Zijlstra, Steven Rostedt, Brian Gerst, lkml

> 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...

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

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH -v2] x86: Add an archinfo dumper module
  2016-02-09 19:17               ` Luck, Tony
@ 2016-02-09 19:46                 ` Borislav Petkov
  0 siblings, 0 replies; 16+ messages in thread
From: Borislav Petkov @ 2016-02-09 19:46 UTC (permalink / raw)
  To: Luck, Tony
  Cc: Ingo Molnar, H. Peter Anvin, Thomas Gleixner, Andy Lutomirski,
	Peter Zijlstra, Steven Rostedt, Brian Gerst, lkml

On Tue, Feb 09, 2016 at 11:17:59AM -0800, Luck, Tony wrote:
> 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.

> 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.

-- 
Regards/Gruss,
    Boris.

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

^ permalink raw reply	[flat|nested] 16+ messages in thread

end of thread, other threads:[~2016-02-09 19:47 UTC | newest]

Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-02-01 11:56 [RFC] Dump interesting arch/platform info Borislav Petkov
2016-02-01 23:29 ` Luck, Tony
2016-02-02  9:41   ` Borislav Petkov
2016-02-03 10:48     ` Ingo Molnar
2016-02-03 11:00       ` Borislav Petkov
2016-02-04 15:22         ` [PATCH -v2] x86: Add an archinfo dumper module Borislav Petkov
2016-02-04 19:07           ` Luck, Tony
2016-02-07 10:51             ` Borislav Petkov
2016-02-09 19:17               ` Luck, Tony
2016-02-09 19:46                 ` Borislav Petkov
2016-02-05 19:51           ` Luck, Tony
2016-02-05 22:24             ` Luck, Tony
2016-02-08  0:01           ` H. Peter Anvin
2016-02-08  7:50             ` Boris Petkov
2016-02-09 12:23             ` Ingo Molnar
2016-02-09 14:01               ` Borislav Petkov

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).