Re: [PATCH 5/6] From: Andrew Morton <akpm@osdl.org>

Re: [PATCH 5/6] From: Andrew Morton <akpm@osdl.org>

[Paul Mackerras]

Jeremy Fitzhardinge writes:

> When my test machine hits a BUG, it simply returns from the exception handler
> after a second or so and reexecutes the bug.
> 
> This is independent of the use-generic-bug changes and might be related to
> XMON.
> 
> So it's some unknonw bug, and this change makes the powerpc kernel behave
> better when that bug hits.

NACK as to this part:

> +			if (btt == BUG_TRAP_TYPE_BUG)
> +				do_exit(SIGSEGV);

since it makes the kernel behave distinctly *worse* for me.

Paul.

Re: [PATCH 5/6] From: Andrew Morton <akpm@osdl.org>

[Andrew Morton]

On Tue, 3 Oct 2006 14:23:06 +1000
Paul Mackerras <paulus@samba.org> wrote:

> Jeremy Fitzhardinge writes:
> 
> > When my test machine hits a BUG, it simply returns from the exception handler
> > after a second or so and reexecutes the bug.
> > 
> > This is independent of the use-generic-bug changes and might be related to
> > XMON.
> > 
> > So it's some unknonw bug, and this change makes the powerpc kernel behave
> > better when that bug hits.
> 
> NACK as to this part:
> 
> > +			if (btt == BUG_TRAP_TYPE_BUG)
> > +				do_exit(SIGSEGV);
> 
> since it makes the kernel behave distinctly *worse* for me.
> 

It makes it heaps better here.

Did you try my .config on the g5?

Re: [PATCH 5/6] From: Andrew Morton <akpm@osdl.org>

[Paul Mackerras]

Andrew Morton writes:

> It makes it heaps better here.

The patch below should fix it properly.

> Did you try my .config on the g5?

Not yet; I can't find your .config in my inbox - maybe I deleted that
message by mistake.  Resend?

Paul.

diff --git a/arch/powerpc/xmon/xmon.c b/arch/powerpc/xmon/xmon.c
index 8adad14..40e5e0d 100644
--- a/arch/powerpc/xmon/xmon.c
+++ b/arch/powerpc/xmon/xmon.c
@@ -503,7 +503,7 @@ #endif
 
 	mtmsr(msr);		/* restore interrupt enable */
 
-	return cmd != 'X';
+	return cmd != 'X' && cmd != EOF;
 }
 
 int xmon(struct pt_regs *excp)

Re: [PATCH 5/6] From: Andrew Morton <akpm@osdl.org>

[Andrew Morton]

On Tue, 3 Oct 2006 15:19:50 +1000
Paul Mackerras <paulus@samba.org> wrote:

> --- a/arch/powerpc/xmon/xmon.c
> +++ b/arch/powerpc/xmon/xmon.c
> @@ -503,7 +503,7 @@ #endif
>  
>  	mtmsr(msr);		/* restore interrupt enable */
>  
> -	return cmd != 'X';
> +	return cmd != 'X' && cmd != EOF;
>  }
>  
>  int xmon(struct pt_regs *excp)

That fixes it.

I still get crap all over the screen when xmon is tring to print something
(at least, I assume that's what causes it).  See
http://userweb.kernel.org/~akpm/s5000334.jpg

Re: [PATCH 5/6] From: Andrew Morton <akpm@osdl.org>

[Paul Mackerras]

Andrew Morton writes:

> I still get crap all over the screen when xmon is tring to print something
> (at least, I assume that's what causes it).  See
> http://userweb.kernel.org/~akpm/s5000334.jpg

That is because the nvidia console driver has changed the line pitch
from what the firmware set it to.  This should fix it by making the
nvidia driver inform the btext engine (which xmon uses if the screen
is its output device) about changes to display resolution.

Signed-off-by: Paul Mackerras <paulus@samba.org>
---
diff --git a/drivers/video/nvidia/nvidia.c b/drivers/video/nvidia/nvidia.c
index d4f8501..18b9101 100644
--- a/drivers/video/nvidia/nvidia.c
+++ b/drivers/video/nvidia/nvidia.c
@@ -28,6 +28,9 @@ #ifdef CONFIG_PPC_OF
 #include <asm/prom.h>
 #include <asm/pci-bridge.h>
 #endif
+#ifdef CONFIG_BOOTX_TEXT
+#include <asm/btext.h>
+#endif
 
 #include "nv_local.h"
 #include "nv_type.h"
@@ -681,6 +684,13 @@ #endif
 
 	nvidia_vga_protect(par, 0);
 
+#ifdef CONFIG_BOOTX_TEXT
+	/* Update debug text engine */
+	btext_update_display(info->fix.smem_start,
+			     info->var.xres, info->var.yres,
+			     info->var.bits_per_pixel, info->fix.line_length);
+#endif
+
 	NVTRACE_LEAVE();
 	return 0;
 }

Re: [PATCH 5/6] From: Andrew Morton <akpm@osdl.org>

[Andrew Morton]

On Tue, 3 Oct 2006 16:55:52 +1000
Paul Mackerras <paulus@samba.org> wrote:

> That is because the nvidia console driver has changed the line pitch
> from what the firmware set it to.  This should fix it by making the
> nvidia driver inform the btext engine (which xmon uses if the screen
> is its output device) about changes to display resolution.
> 
> Signed-off-by: Paul Mackerras <paulus@samba.org>
> ---
> diff --git a/drivers/video/nvidia/nvidia.c b/drivers/video/nvidia/nvidia.c
> index d4f8501..18b9101 100644
> --- a/drivers/video/nvidia/nvidia.c
> +++ b/drivers/video/nvidia/nvidia.c
> @@ -28,6 +28,9 @@ #ifdef CONFIG_PPC_OF
>  #include <asm/prom.h>
>  #include <asm/pci-bridge.h>
>  #endif
> +#ifdef CONFIG_BOOTX_TEXT
> +#include <asm/btext.h>
> +#endif
>  
>  #include "nv_local.h"
>  #include "nv_type.h"
> @@ -681,6 +684,13 @@ #endif
>  
>  	nvidia_vga_protect(par, 0);
>  
> +#ifdef CONFIG_BOOTX_TEXT
> +	/* Update debug text engine */
> +	btext_update_display(info->fix.smem_start,
> +			     info->var.xres, info->var.yres,
> +			     info->var.bits_per_pixel, info->fix.line_length);
> +#endif

yup, that fixed it.  xmon apparently doesn't know where fbcon's output
cursor is, but the characters are now readable.

Re: [PATCH 5/6] From: Andrew Morton <akpm@osdl.org>

[Benjamin Herrenschmidt]

> yup, that fixed it.  xmon apparently doesn't know where fbcon's output
> cursor is, but the characters are now readable.

The low level text engine used by xmon (and my some early console/debug
stuff on powerpc as well) really doesn't have any link to fbcon... doing
that would be hard and ugly and take the risk of making it fragile,
which isn't what we want to do... usually, when you fall into xmon, you
don't care about your console cursor :) (In fact, that stuff can even
blast on top of X if the later has the UseFBDev option).

Ben.

Re: [PATCH 1/6] Generic implemenatation of BUG.

[Andi Kleen]

> Because powerpc also records the function name, I added this to i386 and
> x86-64 for consistency.  Strictly speaking the function name is redundant with
> kallsyms, so perhaps it can be dropped from powerpc.

It would be good to change it to use kallsyms() then.

>  
>  #ifdef CONFIG_BUG
> +
> +#ifdef CONFIG_GENERIC_BUG
> +struct bug_entry {
> +	unsigned long	bug_addr;
> +#ifdef CONFIG_DEBUG_BUGVERBOSE
> +	const char	*file;
> +	unsigned short	line;
> +#endif
> +	unsigned short	flags;

Can't you put the flags into the high bits of the line? I don't think
we have any 64kLOC files.

-Andi

Re: [PATCH 1/6] Generic implemenatation of BUG.

[Jeremy Fitzhardinge]

Andi Kleen wrote:
>> Because powerpc also records the function name, I added this to i386 and
>> x86-64 for consistency.  Strictly speaking the function name is redundant with
>> kallsyms, so perhaps it can be dropped from powerpc.
>>     
>
> It would be good to change it to use kallsyms() then.
>   

It does, in effect, when it prints the oops message and backtrace.

>>  
>>  #ifdef CONFIG_BUG
>> +
>> +#ifdef CONFIG_GENERIC_BUG
>> +struct bug_entry {
>> +	unsigned long	bug_addr;
>> +#ifdef CONFIG_DEBUG_BUGVERBOSE
>> +	const char	*file;
>> +	unsigned short	line;
>> +#endif
>> +	unsigned short	flags;
>>     
>
> Can't you put the flags into the high bits of the line? I don't think
> we have any 64kLOC files.

I thought about it, but it would still be padded out to 12 bytes on 
i386, and more on 64-bit platforms.  And it doesn't matter that much.


    J