[PATCH] powerpc: add dmesg command to xmon

[PATCH] powerpc: add dmesg command to xmon

[Olaf Hering]

Based on work by Linas Vepstas:
dump dmesg buffer in xmon, with a new 'D' command.
Use kallsyms to lookup the symbols to avoid touching generic code in printk.c

Signed-off-by: Olaf Hering <olh@suse.de>

---
 arch/powerpc/xmon/xmon.c |  112 +++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 112 insertions(+)

Index: linux-2.6/arch/powerpc/xmon/xmon.c
===================================================================
--- linux-2.6.orig/arch/powerpc/xmon/xmon.c
+++ linux-2.6/arch/powerpc/xmon/xmon.c
@@ -143,6 +143,7 @@ void dump_segments(void);
 static void symbol_lookup(void);
 static void xmon_print_symbol(unsigned long address, const char *mid,
 			      const char *after);
+static void xmon_show_dmesg(void);
 static const char *getvecname(unsigned long vec);
 
 extern int print_insn_powerpc(unsigned long, unsigned long, int);
@@ -192,6 +193,7 @@ Commands:\n\
   df	dump float values\n\
   dd	dump double values\n\
   dr	dump stream of raw bytes\n\
+  D	show dmesg (printk) buffer\n\
   e	print exception information\n\
   f	flush cache\n\
   la	lookup symbol+offset of specified address\n\
@@ -781,6 +783,9 @@ cmds(struct pt_regs *excp)
 		case 'd':
 			dump();
 			break;
+		case 'D':
+			xmon_show_dmesg();
+			break;
 		case 'l':
 			symbol_lookup();
 			break;
@@ -2488,6 +2493,113 @@ static void xmon_print_symbol(unsigned l
 	printf("%s", after);
 }
 
+#ifdef CONFIG_KALLSYMS
+static const char *xmon_log_buf;
+static int xmon_log_buf_len;
+static unsigned long xmon_log_end, xmon_logged_chars;
+
+static int xmon_init_dmesg(void)
+{
+	const char **p;
+	const int *i;
+	const unsigned long *l;
+
+	if (!xmon_log_buf && setjmp(bus_error_jmp) == 0) {
+		catch_memory_errors = 1;
+		sync();
+
+		p = (const char **)kallsyms_lookup_name("log_buf");
+		if (p) {
+			xmon_log_buf = *p;
+			sync();
+			__delay(200);
+		}
+
+		i = (const int *)kallsyms_lookup_name("log_buf_len");
+		if (i) {
+			xmon_log_buf_len = *i;
+			sync();
+			__delay(200);
+		}
+
+		l = (const unsigned long *)kallsyms_lookup_name("log_end");
+		if (l) {
+			xmon_log_end = *l;
+			sync();
+			__delay(200);
+		}
+
+		l = (const unsigned long *)kallsyms_lookup_name("logged_chars");
+		if (l) {
+			xmon_logged_chars = *l;
+			sync();
+			__delay(200);
+		}
+
+	}
+	catch_memory_errors = 0;
+	return !!xmon_log_buf;
+}
+#else
+static inline int xmon_init_dmesg(void)
+{
+	return 0;
+}
+#endif
+
+static void xmon_show_dmesg(void)
+{
+	if (xmon_init_dmesg()) {
+		const char *p_start, *p_end, *l_start, *l_end, *start, *end;
+		char c;
+
+		p_start = xmon_log_buf;
+		p_end = p_start + xmon_log_buf_len;
+		l_start = p_start + xmon_log_end - (xmon_logged_chars < xmon_log_buf_len ? xmon_logged_chars : xmon_log_buf_len);
+		l_end = p_start + xmon_log_end;
+		if (l_start == l_end)
+			return;
+		start = p_start + (l_start - p_start) % xmon_log_buf_len;
+		end = p_start + (l_end - p_start) % xmon_log_buf_len;
+		c = '\0';
+		while (1) {
+			const char *p;
+			int chars = 0;
+			if (!*start) {
+				while (!*start) {
+					start++;
+					if (start < p_start)
+						start = p_end - 1;
+					else if (start >= p_end)
+						start = p_start;
+					if (start == end)
+						break;
+				}
+				if (start == end)
+					break;
+			}
+			p = start;
+			while (*start && chars < 200) {
+				c = *start;
+				chars++;
+				start++;
+				if (start < p_start)
+					start = p_end - 1;
+				else if (start >= p_end)
+					start = p_start;
+				if (start == end)
+					break;
+			}
+			if (chars)
+				printf("%.*s", chars, p);
+			if (start == end)
+				break;
+		}
+		if (c != '\n')
+			printf("\n");
+	}
+}
+
 #ifdef CONFIG_PPC64
 static void dump_slb(void)
 {

Re: [PATCH] powerpc: add dmesg command to xmon

[Arnd Bergmann]

T24gU3VuZGF5IDI4IE1heSAyMDA2IDEyOjQ2LCBPbGFmIEhlcmluZyB3cm90ZToKPiArc3RhdGlj
IGludCB4bW9uX2luaXRfZG1lc2codm9pZCkKPiArewo+ICugoKCgoKCgY29uc3QgY2hhciAqKnA7
Cj4gK6CgoKCgoKBjb25zdCBpbnQgKmk7Cj4gK6CgoKCgoKBjb25zdCB1bnNpZ25lZCBsb25nICps
Owo+ICsKPiAroKCgoKCgoGlmICgheG1vbl9sb2dfYnVmICYmIHNldGptcChidXNfZXJyb3Jfam1w
KSA9PSAwKSB7Cj4gK6CgoKCgoKCgoKCgoKCgoGNhdGNoX21lbW9yeV9lcnJvcnMgPSAxOwo+ICug
oKCgoKCgoKCgoKCgoKBzeW5jKCk7Cj4gKwo+ICugoKCgoKCgoKCgoKCgoKBwID0gKGNvbnN0IGNo
YXIgKiopa2FsbHN5bXNfbG9va3VwX25hbWUoImxvZ19idWYiKTsKPiAroKCgoKCgoKCgoKCgoKCg
aWYgKHApIHsKPiAroKCgoKCgoKCgoKCgoKCgoKCgoKCgoKB4bW9uX2xvZ19idWYgPSAqcDsKPiAr
oKCgoKCgoKCgoKCgoKCgoKCgoKCgoKBzeW5jKCk7Cj4gK6CgoKCgoKCgoKCgoKCgoKCgoKCgoKCg
X19kZWxheSgyMDApOwo+ICugoKCgoKCgoKCgoKCgoKB9Cj4gKwo+ICugoKCgoKCgoKCgoKCgoKBp
ID0gKGNvbnN0IGludCAqKWthbGxzeW1zX2xvb2t1cF9uYW1lKCJsb2dfYnVmX2xlbiIpOwo+ICug
oKCgoKCgoKCgoKCgoKBpZiAoaSkgewo+ICugoKCgoKCgoKCgoKCgoKCgoKCgoKCgoHhtb25fbG9n
X2J1Zl9sZW4gPSAqaTsKPiAroKCgoKCgoKCgoKCgoKCgoKCgoKCgoKBzeW5jKCk7Cj4gK6CgoKCg
oKCgoKCgoKCgoKCgoKCgoKCgX19kZWxheSgyMDApOwo+ICugoKCgoKCgoKCgoKCgoKB9Cj4gKwo+
ICugoKCgoKCgoKCgoKCgoKBsID0gKGNvbnN0IHVuc2lnbmVkIGxvbmcgKilrYWxsc3ltc19sb29r
dXBfbmFtZSgibG9nX2VuZCIpOwo+ICugoKCgoKCgoKCgoKCgoKBpZiAobCkgewo+ICugoKCgoKCg
oKCgoKCgoKCgoKCgoKCgoHhtb25fbG9nX2VuZCA9ICpsOwo+ICugoKCgoKCgoKCgoKCgoKCgoKCg
oKCgoHN5bmMoKTsKPiAroKCgoKCgoKCgoKCgoKCgoKCgoKCgoKBfX2RlbGF5KDIwMCk7Cj4gK6Cg
oKCgoKCgoKCgoKCgoH0KPiArCj4gK6CgoKCgoKCgoKCgoKCgoGwgPSAoY29uc3QgdW5zaWduZWQg
bG9uZyAqKWthbGxzeW1zX2xvb2t1cF9uYW1lKCJsb2dnZWRfY2hhcnMiKTsKPiAroKCgoKCgoKCg
oKCgoKCgaWYgKGwpIHsKPiAroKCgoKCgoKCgoKCgoKCgoKCgoKCgoKB4bW9uX2xvZ2dlZF9jaGFy
cyA9ICpsOwo+ICugoKCgoKCgoKCgoKCgoKCgoKCgoKCgoHN5bmMoKTsKPiAroKCgoKCgoKCgoKCg
oKCgoKCgoKCgoKBfX2RlbGF5KDIwMCk7Cj4gK6CgoKCgoKCgoKCgoKCgoH0KPiArCj4gK6CgoKCg
oKB9Cj4gK6CgoKCgoKBjYXRjaF9tZW1vcnlfZXJyb3JzID0gMDsKPiAroKCgoKCgoHJldHVybiAh
IXhtb25fbG9nX2J1ZjsKClRoaXMgd2hvbGUgZnVuY3Rpb24gbG9va3MgZmlzaHkgdG8gbWUuIEl0
IHNlZW1zIHVubmVjZXNzYXJpbHkgcnVkZSB0byBtZSB0bwp1c2Uga2FsbHN5bXNfbG9va3VwX25h
bWUgaW4gb3JkZXIgdG8gZ2V0IGF0IGEgc3RhdGljIHZhcmlhYmxlIGZyb20gYW5vdGhlcgpmaWxl
LiBDYW4ndCB5b3UgaW5zdGVhZCBhZGQgYSBnbG9iYWwgZnVuY3Rpb24gdG8ga2VybmVsL3ByaW50
ay5jIHRvIHJldHVybgp0aGUgYnVmZmVyPwoKQWxzbywgd2hhdCdzIHRoZSBwdXJwb3NlIG9mIHRo
ZSBzeW5jL19fZGVsYXkgY29kZSBpbiBoZXJlPyBJZiB5b3UgdXNlIHRoYXQKdG8gY2hlY2sgZm9y
IGEgcG9zc2libGUgbWFjaGluZSBjaGVjayB0aGF0IG1heSBoYXZlIGhhcHBlbmVkLCB3aHkgbm90
IGluCnRoZSBjYXNlIHdoZXJlIHRoZSBsb29rdXAgZmFpbHM/CgoJQXJuZCA8PjwK

Re: [PATCH] powerpc: add dmesg command to xmon

[Olaf Hering]

 On Sun, May 28, Arnd Bergmann wrote:

> This whole function looks fishy to me. It seems unnecessarily rude to me to
> use kallsyms_lookup_name in order to get at a static variable from another
> file. Can't you instead add a global function to kernel/printk.c to return
> the buffer?

Its possible, but it wont end up in Linus tree.

> Also, what's the purpose of the sync/__delay code in here? If you use that
> to check for a possible machine check that may have happened, why not in
> the case where the lookup fails?

I just copied the other usages. Accessing the dmesg buffer itself may
need similar checks.

Re: [PATCH] powerpc: add dmesg command to xmon

[Arnd Bergmann]

On Sunday 28 May 2006 14:07, Olaf Hering wrote:
> > This whole function looks fishy to me. It seems unnecessarily rude to me to
> > use kallsyms_lookup_name in order to get at a static variable from another
> > file. Can't you instead add a global function to kernel/printk.c to return
> > the buffer?
> 
> Its possible, but it wont end up in Linus tree.

Hmm, so the idea is that because Linus doesn't like kernel debuggers, we just
obfuscate the code for any improvements to xmon and hope he doesn't notice?

Don't get me wrong, I think having dmesg in xmon is a good idea, but this is
probably not the best strategy of getting it in there.

	Arnd <><

Re: [PATCH] powerpc: add dmesg command to xmon

[Olaf Hering]

 On Sun, May 28, Arnd Bergmann wrote:

> On Sunday 28 May 2006 14:07, Olaf Hering wrote:
> > > This whole function looks fishy to me. It seems unnecessarily rude to me to
> > > use kallsyms_lookup_name in order to get at a static variable from another
> > > file. Can't you instead add a global function to kernel/printk.c to return
> > > the buffer?
> > 
> > Its possible, but it wont end up in Linus tree.
> 
> Hmm, so the idea is that because Linus doesn't like kernel debuggers, we just
> obfuscate the code for any improvements to xmon and hope he doesn't notice?

So lets just ask again for support in generic code for a dmesg command in xmon:

http://lkml.org/lkml/2004/10/28/32

vs. these 2 patches

http://ozlabs.org/pipermail/linuxppc-dev/2006-May/023261.html
http://lkml.org/lkml/2006/5/29/55

or no dmesg at all.

Re: [PATCH] powerpc: add dmesg command to xmon

[Linas Vepstas]

On Mon, May 29, 2006 at 07:58:34PM +0200, Olaf Hering wrote:
>  On Sun, May 28, Arnd Bergmann wrote:
> 
> > On Sunday 28 May 2006 14:07, Olaf Hering wrote:
> > > > This whole function looks fishy to me. It seems unnecessarily rude to me to
> > > > use kallsyms_lookup_name in order to get at a static variable from another
> > > > file. Can't you instead add a global function to kernel/printk.c to return
> > > > the buffer?
> > > 
> > > Its possible, but it wont end up in Linus tree.
> > 
> > Hmm, so the idea is that because Linus doesn't like kernel debuggers, we just
> > obfuscate the code for any improvements to xmon and hope he doesn't notice?
> 
> So lets just ask again for support in generic code for a dmesg command in xmon:
> 
> http://lkml.org/lkml/2004/10/28/32

I'd been planning to pester Torvalds/LKML on a regular basis until the
patch (or something else suitable) went in. Clearly, I'd failed to do
this. 

--linas

p.s. Belated apologies for failing to mention KDB; I assumed it would be 
"obvious" to those who know, and that by not mentioning KDB explicitly, 
I thought that the patch would have a better chance of acceptance (which
I expected to be low). Silly me.