Re: [PATCH] kgdb-x86_64-support.patch for 2.6.2-rc1-mm3

Re: [PATCH] kgdb-x86_64-support.patch for 2.6.2-rc1-mm3

[Andrew Morton]

Jim Houston <jim.houston@comcast.net> wrote:
>
> The attached patch updates my kgdb-x86_64-support.patch to work
>  with linux-2.6.2-rc1-mm3.

Thanks.  Why does it relocate the call to trap_init() in start_kernel()?

Re: [PATCH] kgdb-x86_64-support.patch for 2.6.2-rc1-mm3

[Jim Houston]

On Mon, 2004-01-26 at 22:28, Andrew Morton wrote:
> Jim Houston <jim.houston@comcast.net> wrote:
> >
> > The attached patch updates my kgdb-x86_64-support.patch to work
> >  with linux-2.6.2-rc1-mm3.
> 
> Thanks.  Why does it relocate the call to trap_init() in start_kernel()?

Hi Andrew,

Moving trap_init() before parse_args() makes the "gdb" command line
option work.

On the i386 George has a few lines of code in breakpoint() which do 
enough setup to allow a break point trap to enter kgdb early in the
boot.  I played with similar code on x86_64, but it didn't work.
Handling a break point trap requires some of the initialization
done in cpu_init().  I suspect the difference is the per-cpu-data
referenced using %gs.

Jim Houston - Concurrent Computer Corp.

Re: [PATCH] kgdb-x86_64-support.patch for 2.6.2-rc1-mm3

[Andi Kleen]

On Mon, 26 Jan 2004 22:05:29 -0500 (EST)
Jim Houston <jim.houston@comcast.net> wrote:

> 
> Hi Andrew,
> 
> The attached patch updates my kgdb-x86_64-support.patch to work
> with linux-2.6.2-rc1-mm3.

Hi,

I already did this merge yesterday. Didn't you get mail? 


> 
> The conflicts seen with the old patch are the result of Andi Kleen
> pushing a portion of the patch to Linus.  In particular my 
> addition of .cfi directives to the x86_64 assembly files is
> now in Linus's tree.
> 
> This version has also been tested (and now works) with Matt Mackall's
> kgdb over ethernet.

Hmm, It didn't work for me.

-Andi

Re: [PATCH] kgdb-x86_64-support.patch for 2.6.2-rc1-mm3

[Jim Houston]

On Tue, 2004-01-27 at 09:56, Andi Kleen wrote:
> On Mon, 26 Jan 2004 22:05:29 -0500 (EST)
> Jim Houston <jim.houston@comcast.net> wrote:
> > The attached patch updates my kgdb-x86_64-support.patch to work
> > with linux-2.6.2-rc1-mm3.
> 
> I already did this merge yesterday. Didn't you get mail? 

Hi Andi,

No.  I didn't see your mail until this morning.

It looks like we were working in lock step.  I had been meaning to
update the patch so when I saw that Andrew had dropped it from 
2.6.2-rc1-mm3 it seemed like a good time.

I'll leave it to you and Andrew to decide how we should resolve our
conflicting patches.

I'm including my notes on the difference between our patches. 

Jim Houston - Concurrent Computer Corp.

--

arch/x86_64/kernel/kgdb_stub.c
	Lots of white space changes.  I assume these are my fault.

	I use the variable kgdb_enable to decide if the system should
	stop in kgdb on an oops or other failure.  My intention was to
	set this variable when the user connected.  I was doing this
	for serial but not for kgdboe.  

	I removed a \n from print_extra_info.

init/main.c
	This change puts trap_init before parse_args().  I needed this
	for the early entry into kgdb with the gdb command line argument
	to work.

arch/x86_64/boot/compressed/head.S
arch/x86_64/boot/compressed/misc.c
include/linux/config.h
	On the i386 asm/kgdb.h is included from config.h.  These changes
	make the x86_64 do the same.  I'm not a fan of globally
	included header files, but I wanted the x86_64 to work the same
	as the i386.  The asm/kgdb.h provides a stub for
	kgdb_process_breakpoint() avoiding the undefined symbol.

arch/x86_64/kernel/irq.c
	This change is not needed with the change above.


arch/x86_64/Kconfig
arch/x86_64/Kconfig.kgdb
	We used a different approach to selecting DEBUG_INFO.
	I was not really happy with the way select DEBUG_INFO worked.

Makefile
	I added -g to AFLAGS so the .S files get line number info.
	I have a problem where gdb identifies error_exit as being
	in elf_core.h.  I had hoped this would help.  It didn't,
	but I still like this change.

include/linux/bitops.h
	I dropped this one.  I suspect that this fixed a compile 
	warning in a forgotten Concurrent tree.

include/asm-x86_64/kgdb_local.h
	This file seems to be missing from your patch.  Maybe I'm 
	missing something.  In my patch it is a copy of the i386
	version.

Re: [PATCH] kgdb-x86_64-support.patch for 2.6.2-rc1-mm3

[Andi Kleen]

On 27 Jan 2004 12:43:20 -0500
Jim Houston <jim.houston@comcast.net> wrote:

.
> 
> It looks like we were working in lock step.  I had been meaning to
> update the patch so when I saw that Andrew had dropped it from 
> 2.6.2-rc1-mm3 it seemed like a good time.
> 
> I'll leave it to you and Andrew to decide how we should resolve our
> conflicting patches.

If yours works on ethernet please use yours. Mine didn't.


> arch/x86_64/Kconfig
> arch/x86_64/Kconfig.kgdb
> 	We used a different approach to selecting DEBUG_INFO.
> 	I was not really happy with the way select DEBUG_INFO worked.

You reverted it back? 

What I did was to change all not really kgdb specific CONFIG_KGDB uses in
the main kernel with CONFIG_DEBUG_INFO (mostly CFI support). I don't feel
strongly about it, but this way there is no reference to an unknown
config symbol in mainline. Also DEBUG_INFO including CFI makes sense I think.

Putting the kgdb options into a separate sourced file is a good idea. 
This should decrease future conflicts.

> include/asm-x86_64/kgdb_local.h
> 	This file seems to be missing from your patch.  Maybe I'm 
> 	missing something.  In my patch it is a copy of the i386
> 	version.

Probably my fault.

-Andi 

Re: [PATCH] kgdb-x86_64-support.patch for 2.6.2-rc1-mm3

[Jim Houston]

On Tue, 2004-01-27 at 13:02, Andi Kleen wrote:
> On 27 Jan 2004 12:43:20 -0500
> Jim Houston <jim.houston@comcast.net> wrote:

> > arch/x86_64/Kconfig
> > arch/x86_64/Kconfig.kgdb
> > 	We used a different approach to selecting DEBUG_INFO.
> > 	I was not really happy with the way select DEBUG_INFO worked.
> 
> You reverted it back? 
> 
> What I did was to change all not really kgdb specific CONFIG_KGDB uses in
> the main kernel with CONFIG_DEBUG_INFO (mostly CFI support). I don't feel
> strongly about it, but this way there is no reference to an unknown
> config symbol in mainline. Also DEBUG_INFO including CFI makes sense I think.

Hi Andi,

I'm using CONFIG_DEBUG_INFO, but I used a different mechanism to
select it when KGDB is selected.  I'm still learning to speak Kconfig.

My patch:

   config KGDB
        bool "Include kgdb kernel debugger"
        depends on DEBUG_KERNEL
        select DEBUG_INFO
        help
          If you say Y here, the system will be compiled with the debug

Your patch:

  config DEBUG_INFO
        bool "Compile kernel with debug information" if !KGDB
        default y

Using "select DEBUG_INFO" selects the option and makes the input box
on xconfig disappear.  The line describing the option remains, perhaps
leaving a user wondering why this line doesn't have an input box.

With your version, the DEBUG_INFO option disappears when KGDB forces
it on.

I was looking for a way to get the old behavior where the 
the effect was controlled by an OR of the two options.

Jim Houston - Concurrent Computer Corp.

Re: [PATCH] kgdb-x86_64-support.patch for 2.6.2-rc1-mm3

[Andi Kleen]

On 27 Jan 2004 14:35:17 -0500
Jim Houston <jim.houston@comcast.net> wrote:


> I was looking for a way to get the old behavior where the 
> the effect was controlled by an OR of the two options.


Hmm, probably something like (untested):

config DEBUG_INFO
	bool "Compile kernel with debug information"
	default y if KGDB
	help
		bla bla bla 

I have no strong preference to either way.

-Andi

Re: [PATCH] kgdb-x86_64-support.patch for 2.6.2-rc1-mm3

[George Anzinger]

Jim Houston wrote:
> On Tue, 2004-01-27 at 13:02, Andi Kleen wrote:
> 
>>On 27 Jan 2004 12:43:20 -0500
>>Jim Houston <jim.houston@comcast.net> wrote:
> 
> 
>>>arch/x86_64/Kconfig
>>>arch/x86_64/Kconfig.kgdb
>>>	We used a different approach to selecting DEBUG_INFO.
>>>	I was not really happy with the way select DEBUG_INFO worked.
>>
>>You reverted it back? 
>>
>>What I did was to change all not really kgdb specific CONFIG_KGDB uses in
>>the main kernel with CONFIG_DEBUG_INFO (mostly CFI support). I don't feel
>>strongly about it, but this way there is no reference to an unknown
>>config symbol in mainline. Also DEBUG_INFO including CFI makes sense I think.

If we are going to use DEBUG_INFO could we change the "-g" it produces to 
"-gdwarft-2", especially since you (and I) are using dwarft2 CFI stuff.

-g
> 
> 
> Hi Andi,
> 
> I'm using CONFIG_DEBUG_INFO, but I used a different mechanism to
> select it when KGDB is selected.  I'm still learning to speak Kconfig.
> 
> My patch:
> 
>    config KGDB
>         bool "Include kgdb kernel debugger"
>         depends on DEBUG_KERNEL
>         select DEBUG_INFO
>         help
>           If you say Y here, the system will be compiled with the debug
> 
> Your patch:
> 
>   config DEBUG_INFO
>         bool "Compile kernel with debug information" if !KGDB
>         default y
> 
> Using "select DEBUG_INFO" selects the option and makes the input box
> on xconfig disappear.  The line describing the option remains, perhaps
> leaving a user wondering why this line doesn't have an input box.
> 
> With your version, the DEBUG_INFO option disappears when KGDB forces
> it on.
> 
> I was looking for a way to get the old behavior where the 
> the effect was controlled by an OR of the two options.
> 
> Jim Houston - Concurrent Computer Corp.
> 
> -
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/
> 

-- 
George Anzinger   george@mvista.com
High-res-timers:  http://sourceforge.net/projects/high-res-timers/
Preemption patch: http://www.kernel.org/pub/linux/kernel/people/rml

Re: [PATCH] kgdb-x86_64-support.patch for 2.6.2-rc1-mm3

[Andi Kleen]

On Tue, 27 Jan 2004 12:37:48 -0800
George Anzinger <george@mvista.com> wrote:

> If we are going to use DEBUG_INFO could we change the "-g" it produces to 
> "-gdwarft-2", especially since you (and I) are using dwarft2 CFI stuff.

On x86-64 dwarf2 is default (and the only supported debug format anyways)
I believe any modern i386 gcc does that same. It would probably only 
make a difference to a few people still using ancient compilers.

-Andi