Re: kdb: add rdmsr and wrmsr commands for i386
Bernardo Innocenti (on Thu, 17 May 2007 02:36:21 -0400) wrote: >Keith Owens wrote: > >> Before using MSR, you must first check that the cpu supports the >> instruction, rd/wrmsr cause an oops on 486 or earlier. Also using an >> invalid msr number causes an oops, so use rd/wrmsr_safe(). > >I didn't bother implementing those checks because kdb recovers >nicely from GPF anyway. Yes and no. Yes, kdb will recover from a GPF. No, because if the system was already running correctly (i.e. manual entry into kdb), then taking a GPF and not recovering will flag the rest of the system as corrupt and can kill a running system. I try to avoid adding spurious system corruption. >It's the valid MSR writes that could >cause unrecoveable problems! :) Tell me about it :-( - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: kdb: add rdmsr and wrmsr commands for i386
Keith Owens wrote: > Before using MSR, you must first check that the cpu supports the > instruction, rd/wrmsr cause an oops on 486 or earlier. Also using an > invalid msr number causes an oops, so use rd/wrmsr_safe(). I didn't bother implementing those checks because kdb recovers nicely from GPF anyway. It's the valid MSR writes that could cause unrecoveable problems! :) -- // Bernardo Innocenti \X/ http://www.codewiz.org/ - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: kdb: add rdmsr and wrmsr commands for i386
Bernardo Innocenti (on Tue, 15 May 2007 23:03:55 -0400) wrote: >Jordan Crouse wrote: > >> Can you break this up with a : between the high dword and the low dword? >> That makes it easier to parse when debugging. > >Good idea, but I used "_" instead because it's what AMD uses in their >documentation and it looks better with a "0x" prefix. > >> Also, would it make sense to coordinate the order of the high and low >> dwords with the order they are specified with 'wrmsr'? > >Yeah, I did it as suggested by Mitch. Here's a thrid >revision of the patch with everything included: > >From 1850ca76585306e2484cf5e709434049f1df3c1f Mon Sep 17 00:00:00 2001 >From: Bernardo Innocenti <[EMAIL PROTECTED]> >Date: Tue, 15 May 2007 15:29:48 -0400 >Subject: [PATCH] kdb: add rdmsr and wrmsr commands for i386 (take 3) > >The syntax is: > rdmsr > wrmsr > >Signed-off-by: Bernardo Innocenti <[EMAIL PROTECTED]> >--- > arch/i386/kdb/kdbasupport.c | 47 +++--- > kdb/kdbmain.c |3 +- Thanks for the patch, but ... Before using MSR, you must first check that the cpu supports the instruction, rd/wrmsr cause an oops on 486 or earlier. Also using an invalid msr number causes an oops, so use rd/wrmsr_safe(). Finally, kdb on x86_64 needs the commands as well, so move it to kdb/modules/kdbm_x86.c (common i386/x86_64 code). Cleaned up patch + changelogs. --- arch/i386/kdb/ChangeLog |6 arch/i386/kdb/kdbasupport.c |7 ++-- arch/x86_64/kdb/ChangeLog |6 arch/x86_64/kdb/kdbasupport.c |7 ++-- kdb/ChangeLog |7 kdb/kdbmain.c |3 -- kdb/modules/kdbm_x86.c| 59 ++ 7 files changed, 85 insertions(+), 10 deletions(-) diff -u linux/arch/i386/kdb/ChangeLog linux/arch/i386/kdb/ChangeLog --- linux/arch/i386/kdb/ChangeLog +++ linux/arch/i386/kdb/ChangeLog @@ -1,3 +1,9 @@ +2007-05-17 Keith Owens <[EMAIL PROTECTED]> + + * Update dumpregs comments for rdmsr and wrmsr commands. + Bernardo Innocenti. + * kdb v4.4-2.6.21-i386-3. + 2007-05-15 Keith Owens <[EMAIL PROTECTED]> * Change kdba_late_init to kdba_arch_init so KDB_ENTER() can be used diff -u linux/arch/i386/kdb/kdbasupport.c linux/arch/i386/kdb/kdbasupport.c --- linux/arch/i386/kdb/kdbasupport.c +++ linux/arch/i386/kdb/kdbasupport.c @@ -474,13 +474,14 @@ * argument is NULL (struct pt_regs). The alternate register * set types supported by this function: * - * d Debug registers + * d Debug registers * c Control registers * u User registers at most recent entry to kernel * for the process currently selected with "pid" command. * Following not yet implemented: - * m Model Specific Registers (extra defines register #) * r Memory Type Range Registers (extra defines register) + * + * MSR on i386/x86_64 are handled by rdmsr/wrmsr commands. */ int @@ -546,8 +547,6 @@ cr[0], cr[1], cr[2], cr[3], cr[4]); return 0; } - case 'm': - break; case 'r': break; default: diff -u linux/arch/x86_64/kdb/ChangeLog linux/arch/x86_64/kdb/ChangeLog --- linux/arch/x86_64/kdb/ChangeLog +++ linux/arch/x86_64/kdb/ChangeLog @@ -1,3 +1,9 @@ +2007-05-17 Keith Owens <[EMAIL PROTECTED]> + + * Update dumpregs comments for rdmsr and wrmsr commands. + Bernardo Innocenti. + * kdb v4.4-2.6.21-x86_64-3. + 2007-05-15 Keith Owens <[EMAIL PROTECTED]> * Change kdba_late_init to kdba_arch_init so KDB_ENTER() can be used diff -u linux/arch/x86_64/kdb/kdbasupport.c linux/arch/x86_64/kdb/kdbasupport.c --- linux/arch/x86_64/kdb/kdbasupport.c +++ linux/arch/x86_64/kdb/kdbasupport.c @@ -470,12 +470,13 @@ * argument is NULL (struct pt_regs). The alternate register * set types supported by this function: * - * d Debug registers + * d Debug registers * c Control registers * u User registers at most recent entry to kernel * Following not yet implemented: - * m Model Specific Registers (extra defines register #) * r Memory Type Range Registers (extra defines register) + * + * MSR on i386/x86_64 are handled by rdmsr/wrmsr commands. */ int @@ -536,8 +537,6 @@ cr[0], cr[1], cr[2], cr[3], cr[4]); return 0; } - case 'm': - break; case 'r': break; default: diff -u linux/kdb/ChangeLog linux/kdb/ChangeLog --- linux/kdb/ChangeLog +++ linux/kdb/ChangeLog @@
Re: kdb: add rdmsr and wrmsr commands for i386
Bernardo Innocenti (on Tue, 15 May 2007 23:03:55 -0400) wrote: Jordan Crouse wrote: Can you break this up with a : between the high dword and the low dword? That makes it easier to parse when debugging. Good idea, but I used _ instead because it's what AMD uses in their documentation and it looks better with a 0x prefix. Also, would it make sense to coordinate the order of the high and low dwords with the order they are specified with 'wrmsr'? Yeah, I did it as suggested by Mitch. Here's a thrid revision of the patch with everything included: From 1850ca76585306e2484cf5e709434049f1df3c1f Mon Sep 17 00:00:00 2001 From: Bernardo Innocenti [EMAIL PROTECTED] Date: Tue, 15 May 2007 15:29:48 -0400 Subject: [PATCH] kdb: add rdmsr and wrmsr commands for i386 (take 3) The syntax is: rdmsr addr wrmsr addr h l Signed-off-by: Bernardo Innocenti [EMAIL PROTECTED] --- arch/i386/kdb/kdbasupport.c | 47 +++--- kdb/kdbmain.c |3 +- Thanks for the patch, but ... Before using MSR, you must first check that the cpu supports the instruction, rd/wrmsr cause an oops on 486 or earlier. Also using an invalid msr number causes an oops, so use rd/wrmsr_safe(). Finally, kdb on x86_64 needs the commands as well, so move it to kdb/modules/kdbm_x86.c (common i386/x86_64 code). Cleaned up patch + changelogs. --- arch/i386/kdb/ChangeLog |6 arch/i386/kdb/kdbasupport.c |7 ++-- arch/x86_64/kdb/ChangeLog |6 arch/x86_64/kdb/kdbasupport.c |7 ++-- kdb/ChangeLog |7 kdb/kdbmain.c |3 -- kdb/modules/kdbm_x86.c| 59 ++ 7 files changed, 85 insertions(+), 10 deletions(-) diff -u linux/arch/i386/kdb/ChangeLog linux/arch/i386/kdb/ChangeLog --- linux/arch/i386/kdb/ChangeLog +++ linux/arch/i386/kdb/ChangeLog @@ -1,3 +1,9 @@ +2007-05-17 Keith Owens [EMAIL PROTECTED] + + * Update dumpregs comments for rdmsr and wrmsr commands. + Bernardo Innocenti. + * kdb v4.4-2.6.21-i386-3. + 2007-05-15 Keith Owens [EMAIL PROTECTED] * Change kdba_late_init to kdba_arch_init so KDB_ENTER() can be used diff -u linux/arch/i386/kdb/kdbasupport.c linux/arch/i386/kdb/kdbasupport.c --- linux/arch/i386/kdb/kdbasupport.c +++ linux/arch/i386/kdb/kdbasupport.c @@ -474,13 +474,14 @@ * argument is NULL (struct pt_regs). The alternate register * set types supported by this function: * - * d Debug registers + * d Debug registers * c Control registers * u User registers at most recent entry to kernel * for the process currently selected with pid command. * Following not yet implemented: - * m Model Specific Registers (extra defines register #) * r Memory Type Range Registers (extra defines register) + * + * MSR on i386/x86_64 are handled by rdmsr/wrmsr commands. */ int @@ -546,8 +547,6 @@ cr[0], cr[1], cr[2], cr[3], cr[4]); return 0; } - case 'm': - break; case 'r': break; default: diff -u linux/arch/x86_64/kdb/ChangeLog linux/arch/x86_64/kdb/ChangeLog --- linux/arch/x86_64/kdb/ChangeLog +++ linux/arch/x86_64/kdb/ChangeLog @@ -1,3 +1,9 @@ +2007-05-17 Keith Owens [EMAIL PROTECTED] + + * Update dumpregs comments for rdmsr and wrmsr commands. + Bernardo Innocenti. + * kdb v4.4-2.6.21-x86_64-3. + 2007-05-15 Keith Owens [EMAIL PROTECTED] * Change kdba_late_init to kdba_arch_init so KDB_ENTER() can be used diff -u linux/arch/x86_64/kdb/kdbasupport.c linux/arch/x86_64/kdb/kdbasupport.c --- linux/arch/x86_64/kdb/kdbasupport.c +++ linux/arch/x86_64/kdb/kdbasupport.c @@ -470,12 +470,13 @@ * argument is NULL (struct pt_regs). The alternate register * set types supported by this function: * - * d Debug registers + * d Debug registers * c Control registers * u User registers at most recent entry to kernel * Following not yet implemented: - * m Model Specific Registers (extra defines register #) * r Memory Type Range Registers (extra defines register) + * + * MSR on i386/x86_64 are handled by rdmsr/wrmsr commands. */ int @@ -536,8 +537,6 @@ cr[0], cr[1], cr[2], cr[3], cr[4]); return 0; } - case 'm': - break; case 'r': break; default: diff -u linux/kdb/ChangeLog linux/kdb/ChangeLog --- linux/kdb/ChangeLog +++ linux/kdb/ChangeLog @@ -1,3 +1,10 @@ +2007-05-17 Keith Owens [EMAIL PROTECTED] + + * Add rdmsr and wrmsr commands for i386 and x86_64. Original patch by + Bernardo Innocenti for i386, reworked by Keith
Re: kdb: add rdmsr and wrmsr commands for i386
Keith Owens wrote: Before using MSR, you must first check that the cpu supports the instruction, rd/wrmsr cause an oops on 486 or earlier. Also using an invalid msr number causes an oops, so use rd/wrmsr_safe(). I didn't bother implementing those checks because kdb recovers nicely from GPF anyway. It's the valid MSR writes that could cause unrecoveable problems! :) -- // Bernardo Innocenti \X/ http://www.codewiz.org/ - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: kdb: add rdmsr and wrmsr commands for i386
Bernardo Innocenti (on Thu, 17 May 2007 02:36:21 -0400) wrote: Keith Owens wrote: Before using MSR, you must first check that the cpu supports the instruction, rd/wrmsr cause an oops on 486 or earlier. Also using an invalid msr number causes an oops, so use rd/wrmsr_safe(). I didn't bother implementing those checks because kdb recovers nicely from GPF anyway. Yes and no. Yes, kdb will recover from a GPF. No, because if the system was already running correctly (i.e. manual entry into kdb), then taking a GPF and not recovering will flag the rest of the system as corrupt and can kill a running system. I try to avoid adding spurious system corruption. It's the valid MSR writes that could cause unrecoveable problems! :) Tell me about it :-( - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: kdb: add rdmsr and wrmsr commands for i386
Jordan Crouse wrote:
> Can you break this up with a : between the high dword and the low dword?
> That makes it easier to parse when debugging.
Good idea, but I used "_" instead because it's what AMD uses in their
documentation and it looks better with a "0x" prefix.
> Also, would it make sense to coordinate the order of the high and low
> dwords with the order they are specified with 'wrmsr'?
Yeah, I did it as suggested by Mitch. Here's a thrid
revision of the patch with everything included:
>From 1850ca76585306e2484cf5e709434049f1df3c1f Mon Sep 17 00:00:00 2001
From: Bernardo Innocenti <[EMAIL PROTECTED]>
Date: Tue, 15 May 2007 15:29:48 -0400
Subject: [PATCH] kdb: add rdmsr and wrmsr commands for i386 (take 3)
The syntax is:
rdmsr
wrmsr
Signed-off-by: Bernardo Innocenti <[EMAIL PROTECTED]>
---
arch/i386/kdb/kdbasupport.c | 47 +++---
kdb/kdbmain.c |3 +-
2 files changed, 44 insertions(+), 6 deletions(-)
diff --git a/arch/i386/kdb/kdbasupport.c b/arch/i386/kdb/kdbasupport.c
index 482b319..7038dfb 100644
--- a/arch/i386/kdb/kdbasupport.c
+++ b/arch/i386/kdb/kdbasupport.c
@@ -223,6 +223,46 @@ kdba_removedbreg(kdb_bp_t *bp)
kdba_putdr7(dr7);
}
+static int
+kdba_rdmsr(int argc, const char **argv)
+{
+ unsigned long addr;
+ uint32_t l, h;
+ int diag;
+
+ if (argc != 1)
+ return KDB_ARGCOUNT;
+
+ if ((diag = kdbgetularg(argv[1], )))
+ return diag;
+
+ kdb_printf("msr(0x%lx) = ", addr);
+ rdmsr(addr, l, h);
+ kdb_printf("0x%08lx_%08lx\n", h, l);
+
+ return 0;
+}
+
+static int
+kdba_wrmsr(int argc, const char **argv)
+{
+ unsigned long addr;
+ unsigned long l, h;
+ int diag;
+
+ if (argc != 3)
+ return KDB_ARGCOUNT;
+
+ if ((diag = kdbgetularg(argv[1], ))
+ || (diag = kdbgetularg(argv[2], ))
+ || (diag = kdbgetularg(argv[3], )))
+ return diag;
+
+ wrmsr(addr, l, h);
+
+ return 0;
+}
+
/*
* kdba_getregcontents
@@ -474,12 +514,11 @@ kdba_setregcontents(const char *regname,
* argument is NULL (struct pt_regs). The alternate register
* set types supported by this function:
*
- * d Debug registers
+ * d Debug registers
* c Control registers
* u User registers at most recent entry to kernel
* for the process currently selected with "pid" command.
* Following not yet implemented:
- * m Model Specific Registers (extra defines register #)
* r Memory Type Range Registers (extra defines register)
*/
@@ -546,8 +585,6 @@ kdba_dumpregs(struct pt_regs *regs,
cr[0], cr[1], cr[2], cr[3], cr[4]);
return 0;
}
- case 'm':
- break;
case 'r':
break;
default:
@@ -899,6 +936,8 @@ kdba_init(void)
{
kdb_register("pt_regs", kdba_pt_regs, "address", "Format struct
pt_regs", 0);
kdb_register("stackdepth", kdba_stackdepth, "[percentage]", "Print
processes using >= stack percentage", 0);
+ kdb_register("rdmsr", kdba_rdmsr, "", "Display Model Specific
Register", 0);
+ kdb_register("wrmsr", kdba_wrmsr, " ", "Modify Model
Specific Register", 0);
return;
}
diff --git a/kdb/kdbmain.c b/kdb/kdbmain.c
index 0b2cb91..88bf14f 100644
--- a/kdb/kdbmain.c
+++ b/kdb/kdbmain.c
@@ -2596,8 +2596,7 @@ kdb_rd(int argc, const char **argv)
* none.
* Remarks:
* Currently doesn't allow modification of control or
- * debug registers, nor does it allow modification
- * of model-specific registers (MSR).
+ * debug registers.
*/
static int
--
1.5.0.2
--
// Bernardo Innocenti
\X/ http://www.codewiz.org/
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: kdb: add rdmsr and wrmsr commands for i386
Jordan Crouse wrote:
Can you break this up with a : between the high dword and the low dword?
That makes it easier to parse when debugging.
Good idea, but I used _ instead because it's what AMD uses in their
documentation and it looks better with a 0x prefix.
Also, would it make sense to coordinate the order of the high and low
dwords with the order they are specified with 'wrmsr'?
Yeah, I did it as suggested by Mitch. Here's a thrid
revision of the patch with everything included:
From 1850ca76585306e2484cf5e709434049f1df3c1f Mon Sep 17 00:00:00 2001
From: Bernardo Innocenti [EMAIL PROTECTED]
Date: Tue, 15 May 2007 15:29:48 -0400
Subject: [PATCH] kdb: add rdmsr and wrmsr commands for i386 (take 3)
The syntax is:
rdmsr addr
wrmsr addr h l
Signed-off-by: Bernardo Innocenti [EMAIL PROTECTED]
---
arch/i386/kdb/kdbasupport.c | 47 +++---
kdb/kdbmain.c |3 +-
2 files changed, 44 insertions(+), 6 deletions(-)
diff --git a/arch/i386/kdb/kdbasupport.c b/arch/i386/kdb/kdbasupport.c
index 482b319..7038dfb 100644
--- a/arch/i386/kdb/kdbasupport.c
+++ b/arch/i386/kdb/kdbasupport.c
@@ -223,6 +223,46 @@ kdba_removedbreg(kdb_bp_t *bp)
kdba_putdr7(dr7);
}
+static int
+kdba_rdmsr(int argc, const char **argv)
+{
+ unsigned long addr;
+ uint32_t l, h;
+ int diag;
+
+ if (argc != 1)
+ return KDB_ARGCOUNT;
+
+ if ((diag = kdbgetularg(argv[1], addr)))
+ return diag;
+
+ kdb_printf(msr(0x%lx) = , addr);
+ rdmsr(addr, l, h);
+ kdb_printf(0x%08lx_%08lx\n, h, l);
+
+ return 0;
+}
+
+static int
+kdba_wrmsr(int argc, const char **argv)
+{
+ unsigned long addr;
+ unsigned long l, h;
+ int diag;
+
+ if (argc != 3)
+ return KDB_ARGCOUNT;
+
+ if ((diag = kdbgetularg(argv[1], addr))
+ || (diag = kdbgetularg(argv[2], h))
+ || (diag = kdbgetularg(argv[3], l)))
+ return diag;
+
+ wrmsr(addr, l, h);
+
+ return 0;
+}
+
/*
* kdba_getregcontents
@@ -474,12 +514,11 @@ kdba_setregcontents(const char *regname,
* argument is NULL (struct pt_regs). The alternate register
* set types supported by this function:
*
- * d Debug registers
+ * d Debug registers
* c Control registers
* u User registers at most recent entry to kernel
* for the process currently selected with pid command.
* Following not yet implemented:
- * m Model Specific Registers (extra defines register #)
* r Memory Type Range Registers (extra defines register)
*/
@@ -546,8 +585,6 @@ kdba_dumpregs(struct pt_regs *regs,
cr[0], cr[1], cr[2], cr[3], cr[4]);
return 0;
}
- case 'm':
- break;
case 'r':
break;
default:
@@ -899,6 +936,8 @@ kdba_init(void)
{
kdb_register(pt_regs, kdba_pt_regs, address, Format struct
pt_regs, 0);
kdb_register(stackdepth, kdba_stackdepth, [percentage], Print
processes using = stack percentage, 0);
+ kdb_register(rdmsr, kdba_rdmsr, maddr, Display Model Specific
Register, 0);
+ kdb_register(wrmsr, kdba_wrmsr, maddr h l, Modify Model
Specific Register, 0);
return;
}
diff --git a/kdb/kdbmain.c b/kdb/kdbmain.c
index 0b2cb91..88bf14f 100644
--- a/kdb/kdbmain.c
+++ b/kdb/kdbmain.c
@@ -2596,8 +2596,7 @@ kdb_rd(int argc, const char **argv)
* none.
* Remarks:
* Currently doesn't allow modification of control or
- * debug registers, nor does it allow modification
- * of model-specific registers (MSR).
+ * debug registers.
*/
static int
--
1.5.0.2
--
// Bernardo Innocenti
\X/ http://www.codewiz.org/
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
