Skip to contentSkip to conversation

only print out DR registers if they are not power-on defaults.

x86 In review Dave Jones 4 participants 11 messages Jun 18, 2013 – Jun 20, 2013 x86

lore
Share & tools
lore

Dave Jones

author
#0 lore raw

The DR registers are rarely useful when decoding oopses.
With screen real estate during oopses at a premium, we can save two lines by only printing out these registers when they are set to something other than they power-on state.

arch/x86/kernel/process_64.c renamed from home/davej/src/kernel/git-trees/linux/arch/x86/kernel/process_64.c +8 -1
diff -durpN '--exclude-from=/home/davej/.exclude' /home/davej/src/kernel/git-trees/linux/arch/x86/kernel/process_64.c linux-dj/arch/x86/kernel/process_64.c
@@ -105,11 +105,18 @@ void __show_regs(struct pt_regs *regs, i
 	get_debugreg(d0, 0);
 	get_debugreg(d1, 1);
 	get_debugreg(d2, 2);
-	printk(KERN_DEFAULT "DR0: %016lx DR1: %016lx DR2: %016lx\n", d0, d1, d2);
 	get_debugreg(d3, 3);
 	get_debugreg(d6, 6);
 	get_debugreg(d7, 7);
+
+	/* Only print out debug registers if they are in their non-default state. */
+	if ((d0 == 0) && (d1 == 0) && (d2 == 0) && (d3 == 0) &&
+	    (d6 & ~DR6_RESERVED) && (d7 == 0x400))
+		return;
+
+	printk(KERN_DEFAULT "DR0: %016lx DR1: %016lx DR2: %016lx\n", d0, d1, d2);
 	printk(KERN_DEFAULT "DR3: %016lx DR6: %016lx DR7: %016lx\n", d3, d6, d7);
+
 }
 
 void release_thread(struct task_struct *dead_task)
arch/x86/kernel/process_32.c +8 -3
index 7305f7d..5905dc5 100644
@@ -110,11 +110,16 @@ void __show_regs(struct pt_regs *regs, int all)
 	get_debugreg(d1, 1);
 	get_debugreg(d2, 2);
 	get_debugreg(d3, 3);
-	printk(KERN_DEFAULT "DR0: %08lx DR1: %08lx DR2: %08lx DR3: %08lx\n",
-			d0, d1, d2, d3);
-
 	get_debugreg(d6, 6);
 	get_debugreg(d7, 7);
+
+	/* Only print out debug registers if they are in their non-default state. */
+	if ((d0 == 0) && (d1 == 0) && (d2 == 0) && (d3 == 0) &&
+	    (d6 & ~DR6_RESERVED) && (d7 == 0x400))
+		return;
+
+	printk(KERN_DEFAULT "DR0: %08lx DR1: %08lx DR2: %08lx DR3: %08lx\n",
+			d0, d1, d2, d3);
 	printk(KERN_DEFAULT "DR6: %08lx DR7: %08lx\n",
 			d6, d7);
 }
maintainer
#1 lore raw
Quoted 4 lines · Dave Jones

The DR registers are rarely useful when decoding oopses.
With screen real estate during oopses at a premium, we can save two lines by only printing out these registers when they are set to something other than they power-on state.

Makes sense, except...

Quoted 8 lines

diff -durpN '--exclude-from=/home/davej/.exclude' /home/davej/src/kernel/git-trees/linux/arch/x86/kernel/process_64.c linux-dj/arch/x86/kernel/process_64.c

arch/x86/kernel/process_64.c renamed from home/davej/src/kernel/git-trees/linux/arch/x86/kernel/process_64.c +0 -0
@@ -105,11 +105,18 @@ void __show_regs(struct pt_regs *regs, i
 	get_debugreg(d0, 0);

arch/x86/kernel/process_64.c void __show_regs(struct pt_regs *regs, i · line 114

Quoted 10 lines
arch/x86/kernel/process_64.c renamed from home/davej/src/kernel/git-trees/linux/arch/x86/kernel/process_64.c +4 -1
 	get_debugreg(d1, 1);
 	get_debugreg(d2, 2);
-	printk(KERN_DEFAULT "DR0: %016lx DR1: %016lx DR2: %016lx\n", d0, d1, d2);
 	get_debugreg(d3, 3);
 	get_debugreg(d6, 6);
 	get_debugreg(d7, 7);
+
+	/* Only print out debug registers if they are in their non-default state. */
+	if ((d0 == 0) && (d1 == 0) && (d2 == 0) && (d3 == 0) &&
+	    (d6 & ~DR6_RESERVED) && (d7 == 0x400))

... I'm not sure about %dr6. So we're not dumping %dr6 when, a.o. any of the bits in the bit slices [3:0], [15:13] are set (bit 12 is Read-As-Zero). Now those mean stuff like Breakpoint-Condition Detected or Debug-Register-Access Detected and so on and I think this is meaningful information.

So actually, we wouldn't want to dump %dr6 when its contents are its default contents, i.e, 0xffff0ff0, i.e. the test should be:

	(d6 == DR6_RESERVED)

no?

Quoted 16 lines
arch/x86/kernel/process_64.c renamed from home/davej/src/kernel/git-trees/linux/arch/x86/kernel/process_64.c +4 -0
+		return;
+
+	printk(KERN_DEFAULT "DR0: %016lx DR1: %016lx DR2: %016lx\n", d0, d1, d2);
 	printk(KERN_DEFAULT "DR3: %016lx DR6: %016lx DR7: %016lx\n", d3, d6, d7);
+
 }
 
 void release_thread(struct task_struct *dead_task)
arch/x86/kernel/process_32.c +0 -0
index 7305f7d..5905dc5 100644
@@ -110,11 +110,16 @@ void __show_regs(struct pt_regs *regs, int all)
 	get_debugreg(d1, 1);
 	get_debugreg(d2, 2);

arch/x86/kernel/process_32.c void __show_regs(struct pt_regs *regs, int all) · line 118

Quoted 10 lines
arch/x86/kernel/process_32.c +4 -3
 	get_debugreg(d3, 3);
-	printk(KERN_DEFAULT "DR0: %08lx DR1: %08lx DR2: %08lx DR3: %08lx\n",
-			d0, d1, d2, d3);
-
 	get_debugreg(d6, 6);
 	get_debugreg(d7, 7);
+
+	/* Only print out debug registers if they are in their non-default state. */
+	if ((d0 == 0) && (d1 == 0) && (d2 == 0) && (d3 == 0) &&
+	    (d6 & ~DR6_RESERVED) && (d7 == 0x400))

ditto.

Quoted 6 lines
arch/x86/kernel/process_32.c +4 -0
+		return;
+
+	printk(KERN_DEFAULT "DR0: %08lx DR1: %08lx DR2: %08lx DR3: %08lx\n",
+			d0, d1, d2, d3);
 	printk(KERN_DEFAULT "DR6: %08lx DR7: %08lx\n",
 			d6, d7);
Signature · 5 lines
Regards/Gruss,
    Boris.

Sent from a fat crate under my desk. Formatting is fine.
--
author
#2 lore raw
Quoted 33 lines · Borislav Petkov
Quoted 4 lines · Dave Jones

The DR registers are rarely useful when decoding oopses.
With screen real estate during oopses at a premium, we can save two lines by only printing out these registers when they are set to something other than they power-on state.

Makes sense, except...

Quoted 8 lines

diff -durpN '--exclude-from=/home/davej/.exclude' /home/davej/src/kernel/git-trees/linux/arch/x86/kernel/process_64.c linux-dj/arch/x86/kernel/process_64.c

arch/x86/kernel/process_64.c renamed from home/davej/src/kernel/git-trees/linux/arch/x86/kernel/process_64.c +0 -0
@@ -105,11 +105,18 @@ void __show_regs(struct pt_regs *regs, i
 	get_debugreg(d0, 0);

arch/x86/kernel/process_64.c void __show_regs(struct pt_regs *regs, i · line 114

Quoted 10 lines
arch/x86/kernel/process_64.c renamed from home/davej/src/kernel/git-trees/linux/arch/x86/kernel/process_64.c +4 -1
 	get_debugreg(d1, 1);
 	get_debugreg(d2, 2);
-	printk(KERN_DEFAULT "DR0: %016lx DR1: %016lx DR2: %016lx\n", d0, d1, d2);
 	get_debugreg(d3, 3);
 	get_debugreg(d6, 6);
 	get_debugreg(d7, 7);
+
+	/* Only print out debug registers if they are in their non-default state. */
+	if ((d0 == 0) && (d1 == 0) && (d2 == 0) && (d3 == 0) &&
+	    (d6 & ~DR6_RESERVED) && (d7 == 0x400))

... I'm not sure about %dr6. So we're not dumping %dr6 when, a.o. any of the bits in the bit slices [3:0], [15:13] are set (bit 12 is Read-As-Zero). Now those mean stuff like Breakpoint-Condition Detected or Debug-Register-Access Detected and so on and I think this is meaningful information.

Quoted 6 lines

So actually, we wouldn't want to dump %dr6 when its contents are its default contents, i.e, 0xffff0ff0, i.e. the test should be:

	(d6 == DR6_RESERVED)

no?

My intent here was to ignore cases where the reserved bits haven't been set. I occasionally see DR6: 00000000fffe0ff0 for eg.

But maybe you're right, and that is a clue and is worth printing ? I can't personally recall ever diagnosing a bug using those register dumps in the last 15 years.

Dave

maintainer
#3 lore raw
Quoted 2 lines · Dave Jones

My intent here was to ignore cases where the reserved bits haven't been set. I occasionally see DR6: 00000000fffe0ff0 for eg.

That's bit 16 which, according to the docs is read-as-1:

"All remaining bits in the DR6 register are reserved. Reserved bits 31:16 and 11:4 must all be set to 1, while reserved bit 12 must be cleared to 0. In 64-bit mode, the upper 32 bits of DR6 are reserved and must be written with zeros. Writing a 1 to any of the upper 32 bits results in a general-protection exception, #GP(0)."

This above if from AMD APM and Intel's SDM has a graphic showing the exact same thing:

[31:16] = set to 1; [12] = 0b, [11:4] = 1b

So if you see bit 16 cleared, then some BIOS or even hardware is doing funky things. I wouldn't wonder at all if BIOS dudes used reserved bits in registers as scratch space.

Quoted 3 lines

But maybe you're right, and that is a clue and is worth printing ? I can't personally recall ever diagnosing a bug using those register dumps in the last 15 years.

Right, I don't know whether it would always help but if you have an oops and see, say bit 0 in DR6 set, i.e. a debug exception was caused by address breakpoint condition in DR0, then that could be useful info, methinks.

Thanks.

Signature · 5 lines
Regards/Gruss,
    Boris.

Sent from a fat crate under my desk. Formatting is fine.
--
author
#4 lore raw

PATCH v2 1/1 only print out DR registers if they are not power-on defaults.

The DR registers are rarely useful when decoding oopses.
With screen real estate during oopses at a premium, we can save two lines by only printing out these registers when they are set to something other than they power-on state.

[v2: incorporate D6 change suggested by Borislav Petkov <[email protected]>]

arch/x86/kernel/process_64.c +8 -1
diff -durpN '--exclude-from=/home/davej/.exclude' /home/davej/src/kernel/git-trees/linux/arch/x86/kernel/process_64.c linux-dj/arch/x86/kernel/process_64.c
@@ -105,11 +105,18 @@ void __show_regs(struct pt_regs *regs, i
 	get_debugreg(d0, 0);
 	get_debugreg(d1, 1);
 	get_debugreg(d2, 2);
-	printk(KERN_DEFAULT "DR0: %016lx DR1: %016lx DR2: %016lx\n", d0, d1, d2);
 	get_debugreg(d3, 3);
 	get_debugreg(d6, 6);
 	get_debugreg(d7, 7);
+
+	/* Only print out debug registers if they are in their non-default state. */
+	if ((d0 == 0) && (d1 == 0) && (d2 == 0) && (d3 == 0) &&
+	    (d6 == DR6_RESERVED) && (d7 == 0x400))
+		return;
+
+	printk(KERN_DEFAULT "DR0: %016lx DR1: %016lx DR2: %016lx\n", d0, d1, d2);
 	printk(KERN_DEFAULT "DR3: %016lx DR6: %016lx DR7: %016lx\n", d3, d6, d7);
+
 }
 
 void release_thread(struct task_struct *dead_task)
arch/x86/kernel/process_32.c +8 -3
index 7305f7d..5905dc5 100644
@@ -110,11 +110,16 @@ void __show_regs(struct pt_regs *regs, int all)
 	get_debugreg(d1, 1);
 	get_debugreg(d2, 2);
 	get_debugreg(d3, 3);
-	printk(KERN_DEFAULT "DR0: %08lx DR1: %08lx DR2: %08lx DR3: %08lx\n",
-			d0, d1, d2, d3);
-
 	get_debugreg(d6, 6);
 	get_debugreg(d7, 7);
+
+	/* Only print out debug registers if they are in their non-default state. */
+	if ((d0 == 0) && (d1 == 0) && (d2 == 0) && (d3 == 0) &&
+	    (d6 == DR6_RESERVED) && (d7 == 0x400))
+		return;
+
+	printk(KERN_DEFAULT "DR0: %08lx DR1: %08lx DR2: %08lx DR3: %08lx\n",
+			d0, d1, d2, d3);
 	printk(KERN_DEFAULT "DR6: %08lx DR7: %08lx\n",
 			d6, d7);
 }
maintainer Acked-by on v2 1/1
#5 lore raw
Quoted 8 lines · Dave Jones

The DR registers are rarely useful when decoding oopses.
With screen real estate during oopses at a premium, we can save two lines by only printing out these registers when they are set to something other than they power-on state.

[v2: incorporate D6 change suggested by Borislav Petkov <[email protected]>]

Signature · 5 lines
Regards/Gruss,
    Boris.

Sent from a fat crate under my desk. Formatting is fine.
--
#6 lore raw

[tip:x86/debug] x86/debug: Only print out DR registers if they are not power-on defaults

In reply to Dave Jones
x86/debug: Only print out DR registers if they are not power-on defaults
Commit-ID:  4338774cd41a6abf72aa76585ce2184cea8ff8a2
Gitweb:     http://git.kernel.org/tip/4338774cd41a6abf72aa76585ce2184cea8ff8a2
Author:     Dave Jones <[email protected]>
AuthorDate: Tue, 18 Jun 2013 12:09:11 -0400
Committer:  Ingo Molnar <[email protected]>
CommitDate: Wed, 19 Jun 2013 14:33:59 +0200

x86/debug: Only print out DR registers if they are not power-on defaults

The DR registers are rarely useful when decoding oopses. With screen real estate during oopses at a premium, we can save two lines by only printing out these registers when they are set to something other than they power-on state.

---
 arch/x86/kernel/process_32.c | 11 ++++++++---
 arch/x86/kernel/process_64.c |  9 ++++++++-
 2 files changed, 16 insertions(+), 4 deletions(-)
arch/x86/kernel/process_32.c +8 -3
index 7305f7d..f84cfd1 100644
@@ -110,11 +110,16 @@ void __show_regs(struct pt_regs *regs, int all)
 	get_debugreg(d1, 1);
 	get_debugreg(d2, 2);
 	get_debugreg(d3, 3);
-	printk(KERN_DEFAULT "DR0: %08lx DR1: %08lx DR2: %08lx DR3: %08lx\n",
-			d0, d1, d2, d3);
-
 	get_debugreg(d6, 6);
 	get_debugreg(d7, 7);
+
+	/* Only print out debug registers if they are in their non-default state. */
+	if ((d0 == 0) && (d1 == 0) && (d2 == 0) && (d3 == 0) &&
+	    (d6 == DR6_RESERVED) && (d7 == 0x400))
+		return;
+
+	printk(KERN_DEFAULT "DR0: %08lx DR1: %08lx DR2: %08lx DR3: %08lx\n",
+			d0, d1, d2, d3);
 	printk(KERN_DEFAULT "DR6: %08lx DR7: %08lx\n",
 			d6, d7);
 }
arch/x86/kernel/process_64.c +8 -1
index 355ae06..a8b9abc 100644
@@ -105,11 +105,18 @@ void __show_regs(struct pt_regs *regs, int all)
 	get_debugreg(d0, 0);
 	get_debugreg(d1, 1);
 	get_debugreg(d2, 2);
-	printk(KERN_DEFAULT "DR0: %016lx DR1: %016lx DR2: %016lx\n", d0, d1, d2);
 	get_debugreg(d3, 3);
 	get_debugreg(d6, 6);
 	get_debugreg(d7, 7);
+
+	/* Only print out debug registers if they are in their non-default state. */
+	if ((d0 == 0) && (d1 == 0) && (d2 == 0) && (d3 == 0) &&
+	    (d6 == DR6_RESERVED) && (d7 == 0x400))
+		return;
+
+	printk(KERN_DEFAULT "DR0: %016lx DR1: %016lx DR2: %016lx\n", d0, d1, d2);
 	printk(KERN_DEFAULT "DR3: %016lx DR6: %016lx DR7: %016lx\n", d3, d6, d7);
+
 }
 
 void release_thread(struct task_struct *dead_task)
#7 lore raw
In reply to Borislav Petkov
Quoted 21 lines · Borislav Petkov
Quoted 2 lines · Dave Jones

My intent here was to ignore cases where the reserved bits haven't been set. I occasionally see DR6: 00000000fffe0ff0 for eg.

That's bit 16 which, according to the docs is read-as-1:

"All remaining bits in the DR6 register are reserved. Reserved bits 31:16 and 11:4 must all be set to 1, while reserved bit 12 must be cleared to 0. In 64-bit mode, the upper 32 bits of DR6 are reserved and must be written with zeros. Writing a 1 to any of the upper 32 bits results in a general-protection exception, #GP(0)."

This above if from AMD APM and Intel's SDM has a graphic showing the exact same thing:

[31:16] = set to 1; [12] = 0b, [11:4] = 1b

So if you see bit 16 cleared, then some BIOS or even hardware is doing funky things. I wouldn't wonder at all if BIOS dudes used reserved bits in registers as scratch space.

Quoted 9 lines
Quoted 3 lines

But maybe you're right, and that is a clue and is worth printing ? I can't personally recall ever diagnosing a bug using those register dumps in the last 15 years.

Right, I don't know whether it would always help but if you have an oops and see, say bit 0 in DR6 set, i.e. a debug exception was caused by address breakpoint condition in DR0, then that could be useful info, methinks.

There is serious confusion with regards to DR6 about the bits which are *fixed* (forced to 1) and the ones which are *reserved* (should always have a fixed value.)

There are some bits in DR6 which are used by hardware probes.

-hpa

maintainer
#8 lore raw
Quoted 5 lines · H. Peter Anvin

There is serious confusion with regards to DR6 about the bits which are *fixed* (forced to 1) and the ones which are *reserved* (should always have a fixed value.)

There are some bits in DR6 which are used by hardware probes.

Haha, I knew it! HW guys don't care about the architectural documents.

Maybe DR6 should probably be dropped altogether from the check before exiting __show_regs early?

Signature · 5 lines
Regards/Gruss,
    Boris.

Sent from a fat crate under my desk. Formatting is fine.
--
#9 lore raw
Quoted 5 lines · Borislav Petkov
Quoted 4 lines · H. Peter Anvin

There is serious confusion with regards to DR6 about the bits which are *fixed* (forced to 1) and the ones which are *reserved* (should always have a fixed value.)

Quoted 7 lines
Quoted 1 line

There are some bits in DR6 which are used by hardware probes.

Haha, I knew it! HW guys don't care about the architectural documents.

Maybe DR6 should probably be dropped altogether from the check before exiting __show_regs early?

Uhm... I think you're missing the point of architectural documents.

The architectural documents describe the contract between hardware and software. It doesn't mean that hardware can't go beyond it... it is in fact the norm. The architectural documentation states what software may rely on and within what bounds it can assume safe behavior to be compatible both now and with any future extensions.

-hpa

maintainer
#10 lore raw
Quoted 13 lines · H. Peter Anvin
Quoted 11 lines · Borislav Petkov
Quoted 5 lines · H. Peter Anvin

There is serious confusion with regards to DR6 about the bits which are *fixed* (forced to 1) and the ones which are *reserved* (should always have a fixed value.)

There are some bits in DR6 which are used by hardware probes.

Haha, I knew it! HW guys don't care about the architectural documents.

Maybe DR6 should probably be dropped altogether from the check before exiting __show_regs early?

Quoted 7 lines

Uhm... I think you're missing the point of architectural documents.

The architectural documents describe the contract between hardware and software. It doesn't mean that hardware can't go beyond it... it is in fact the norm. The architectural documentation states what software may rely on and within what bounds it can assume safe behavior to be compatible both now and with any future extensions.

Well, that I know :).

Quoted 1 line

From your comment above it sounded like we can't really rely on some of

the bits in DR6_RESERVED to be really always 1 as they cannot only be *fixed* (forced to 1) but also be reserved with a fixed value which is not necessarily 1.

And there's a section in the SDM "19.23.1 Differences in Debug Register DR6" which says that some families allow writes to 1b of bit 12 while others don't.

Now Dave stated earlier "I occasionally see DR6: 00000000fffe0ff0 for eg." which says that bit 16 can be 0b but the AMD spec says "read as 1s" and Intel's spec says "Reserved (set to 1)" which I only assume is the same thing: the default state of those is 1.

So maybe the DR6 check should be reversed to actually look at only the defined bits and ignore "volatile" reserved bits, whichever they might be :-):

if (!(dr6 & 0xe00f))

I.e., bit slices [15:13] and [3:0] should be 0.

Then we can assume no breakpoint condition has been detected nor the debug registers are being used in some way.

Signature · 5 lines
Regards/Gruss,
    Boris.

Sent from a fat crate under my desk. Formatting is fine.
--