Re: PPC KGDB changes and some help?

[Tom Rini <trini@kernel.crashing.org>]

On Wed, Jan 21, 2004 at 03:05:54PM -0800, George Anzinger wrote:
> Tom Rini wrote:
> >On Wed, Jan 21, 2004 at 10:23:12PM +0530, Amit S. Kale wrote:
> >
> >
> >>Hi,
> >>
> >>Here it is: ppc kgdb from timesys kernel is available at
> >>http://kgdb.sourceforge.net/kgdb-2/linux-2.6.1-kgdb-2.1.0.tar.bz2
> >>
> >>This is my attempt at extracting kgdb from TimeSys kernel. It works well 
> >>in TimeSys kernel, so blame me if above patch doesn't work.
> >
> >
> >Okay, here's my first patch against this.
> >===== kernel/kgdbstub.c 1.1 vs edited =====
> >--- 1.1/kernel/kgdbstub.c	Wed Jan 21 10:13:17 2004
> >+++ edited/kernel/kgdbstub.c	Wed Jan 21 10:53:38 2004
> >@@ -1058,9 +1058,6 @@
> > 	kgdb_serial->write_char('+');
> > 
> > 	linux_debug_hook = kgdb_handle_exception;
> >-	
> >-	if (kgdb_ops->kgdb_init)
> >-		kgdb_ops->kgdb_init();
> > 
> > 	/* We can't do much if this fails */
> > 	register_module_notifier(&kgdb_module_load_nb);
> >@@ -1104,6 +1101,11 @@
> > 	if (!kgdb_enter) {
> > 		return;
> > 	}
> >+
> >+	/* Let the arch do any initalization it needs to */
> >+	if (kgdb_ops->kgdb_init)
> >+		kgdb_ops->kgdb_init();
> >+
> > 	if (!kgdb_serial) {
> > 		printk("KGDB: no gdb interface available.\n"
> > 		       "kgdb can't be enabled\n");
> >
> >I'm not sure why you were calling the arch-specific init so late in the
> >process, but since it's a nop on both i386 and x86_64 (so perhaps it
> >should be removed for both of these?), this change doesn't matter to
> >them.  But it does make the PPC code cleaner, IMHO.
> 
> I agree.  Lets dump all the init calls/code.  I have not seen anything yet 
> that can not be done as a side effect of the first call, or better yet, at 
> compile time.
> 
> I am willing to be shown a valid case, however.  Remember, I want to be 
> able to do a breakpoint() as the first line of C code in the kernel.  
> (works with the mm kgdb).

How would you propose handling what's done in ppc_kgdb_init ?  I could
make it a __setup, ala how kgdb_8250.c works, but that too won't allow
for 'first line of C'.  OTOH,  if breakpoint did:
if (!kgdb_initalized) {
   ... work of kgdb_entry() ...
}
... normal breakpoint() code ...

PPC would be fine, as would other arches which need to do some setup.

-- 
Tom Rini
http://gate.crashing.org/~trini/