Re: [Kgdb-bugreport] [PATCH 1/5] KGDB: improve early init

From: Jan Kiszka
Date: Thu Jan 31 2008 - 18:06:40 EST

Jan Kiszka wrote:
> George Anzinger wrote:
>> On 01/31/2008 01:36 AM, Jan Kiszka was caught saying:
>>> BTW, do you know if EXCEPTION_STACK_READY fails for other archs in
>>> parse_early_param as well? It should, because my under standing of
>>> trap_init is that it's the functions to arm things like... exception
>>> handlers? And that raises the question of the deeper purpose of this
>>> check (and the invocation of kgdb_early_init from the argument parsing
>>> function). Sigh, KGDB is still a quite improvable piece of code.
>> Likely. Once you get it in the main line kernel, one would hope that
>> other arch code would be forth coming as many more "eyes" will be in play.
>
> Meanwhile I realized that there is early_trap_init - for x86-32 only! I
> assume now we are only lacking the same for x86-64 to get kgdb running
> there already during early_param-parsing.

Looks like that was the key. Thanks for pointing me at this, George.
Here the updated patch:

------snip-------

This cleans up the early entry of kgdb. It introduces early_trap_init
for x86-64, reloads the idt register also in the 32-bit variant, removes
the now unneeded EXCEPTION_STACK_READY construction, and matures the
init-state machine of kgdb.

Signed-off-by: Jan Kiszka <jan.kiszka@xxxxxx>

---
arch/x86/kernel/setup_64.c | 4 +++
arch/x86/kernel/traps_32.c | 3 +-
arch/x86/kernel/traps_64.c | 10 ++++++++-
include/asm-x86/kgdb.h | 3 --
include/linux/kgdb.h | 7 +-----
kernel/kgdb.c | 47 +++++++++++++++++++--------------------------
6 files changed, 37 insertions(+), 37 deletions(-)

Index: b/arch/x86/kernel/traps_32.c
===================================================================
--- a/arch/x86/kernel/traps_32.c
+++ b/arch/x86/kernel/traps_32.c
@@ -1137,12 +1137,13 @@ asmlinkage void math_emulate(long arg)

#endif /* CONFIG_MATH_EMULATION */

-/* Some traps need to be set early. */
+/* Set of traps needed for early debugging. */
void __init early_trap_init(void)
{
set_intr_gate(1, &debug);
set_system_intr_gate(3, &int3); /* int3 can be called from all */
set_intr_gate(14, &page_fault);
+ load_idt(&idt_descr);
}

void __init trap_init(void)
Index: b/arch/x86/kernel/traps_64.c
===================================================================
--- a/arch/x86/kernel/traps_64.c
+++ b/arch/x86/kernel/traps_64.c
@@ -1129,6 +1129,15 @@ asmlinkage void math_state_restore(void)
}
EXPORT_SYMBOL_GPL(math_state_restore);

+/* Set of traps needed for early debugging. */
+void __init early_trap_init(void)
+{
+ set_intr_gate(1, &debug);
+ set_intr_gate(3, &int3);
+ set_intr_gate(14, &page_fault);
+ load_idt((const struct desc_ptr *)&idt_descr);
+}
+
void __init trap_init(void)
{
set_intr_gate(0,&divide_error);
@@ -1145,7 +1154,6 @@ void __init trap_init(void)
set_intr_gate(11,&segment_not_present);
set_intr_gate_ist(12,&stack_segment,STACKFAULT_STACK);
set_intr_gate(13,&general_protection);
- set_intr_gate(14,&page_fault);
set_intr_gate(15,&spurious_interrupt_bug);
set_intr_gate(16,&coprocessor_error);
set_intr_gate(17,&alignment_check);
Index: b/include/linux/kgdb.h
===================================================================
--- a/include/linux/kgdb.h
+++ b/include/linux/kgdb.h
@@ -43,7 +43,8 @@ extern struct task_struct *kgdb_contthre

enum kgdb_initstate {
KGDB_UNINITIALIZED = 0,
- KGDB_SEMI_INITIALIZED,
+ KGDB_ARCH_INITIALIZED,
+ KGDB_DELAYED_CONNECTION,
KGDB_FULLY_INITIALIZED
};

@@ -290,10 +291,6 @@ int kgdb_nmihook(int cpu, void *regs);
extern int debugger_step;
extern atomic_t debugger_active;

-#ifndef EXCEPTION_STACK_READY
-# define EXCEPTION_STACK_READY() 1
-#endif
-
#else /* !CONFIG_KGDB */
static const atomic_t debugger_active = ATOMIC_INIT(0);
#endif /* !CONFIG_KGDB */
Index: b/kernel/kgdb.c
===================================================================
--- a/kernel/kgdb.c
+++ b/kernel/kgdb.c
@@ -2104,6 +2104,12 @@ void kgdb_unregister_io_module(struct kg
}
EXPORT_SYMBOL_GPL(kgdb_unregister_io_module);

+static void __init kgdb_initial_breakpoint(void)
+{
+ printk(KERN_CRIT "kgdb: Waiting for connection from remote gdb...\n");
+ breakpoint();
+}
+
/*
* This function can be called very early, either via early_param() or
* an explicit breakpoint() early on.
@@ -2112,25 +2118,15 @@ static void __init kgdb_early_entry(void
{
/* Let the architecture do any setup that it needs to. */
kgdb_arch_init();
-
- /*
- * Don't try and do anything until the architecture is able to
- * setup the exception stack. In this case, it is up to the
- * architecture to hook in and look at us when they are ready.
- */
- if (!EXCEPTION_STACK_READY()) {
- kgdb_state = KGDB_SEMI_INITIALIZED;
- /* any kind of break point is deferred to late_init */
- return;
- }
+ kgdb_state = KGDB_ARCH_INITIALIZED;

/*
* Now try the I/O.
* For early entry kgdb_io_ops.init must be defined
*/
if (!kgdb_io_ops.init || kgdb_io_ops.init()) {
- /* Try again later. */
- kgdb_state = KGDB_SEMI_INITIALIZED;
+ printk(KERN_ERR "kgdb: Could not setup core I/O for KGDB.\n");
+ printk(KERN_INFO "kgdb: Defering I/O setup to late init.\n");
return;
}

@@ -2155,14 +2151,16 @@ static void __init kgdb_early_entry(void
*/
static int __init kgdb_late_entry(void)
{
- int need_break = (kgdb_state == KGDB_SEMI_INITIALIZED);
+ int need_break = (kgdb_state == KGDB_DELAYED_CONNECTION);

/*
* If we haven't tried to initialize KGDB yet, we need to call
* kgdb_arch_init before moving onto the I/O.
*/
- if (kgdb_state == KGDB_UNINITIALIZED)
+ if (kgdb_state == KGDB_UNINITIALIZED) {
kgdb_arch_init();
+ kgdb_state = KGDB_ARCH_INITIALIZED;
+ }

if (kgdb_state != KGDB_FULLY_INITIALIZED) {
if (kgdb_io_ops.init && kgdb_io_ops.init()) {
@@ -2177,6 +2175,7 @@ static int __init kgdb_late_entry(void)
printk(KERN_INFO "kgdb: Defering I/O setup to kernel "
"module.\n");
memset(&kgdb_io_ops, 0, sizeof(struct kgdb_io));
+ need_break = 0;
}

kgdb_internal_init();
@@ -2198,11 +2197,8 @@ static int __init kgdb_late_entry(void)
if (kgdb_io_ops.late_init)
kgdb_io_ops.late_init();

- if (need_break) {
- printk(KERN_CRIT "kgdb: Waiting for connection from remote"
- " gdb...\n");
- breakpoint();
- }
+ if (need_break)
+ kgdb_initial_breakpoint();

return 0;
}
@@ -2290,14 +2286,11 @@ static int __init opt_kgdb_enter(char *s

kgdb_early_entry();
attachwait = 1;
- if (kgdb_state == KGDB_FULLY_INITIALIZED)
- printk(KERN_CRIT "Waiting for connection from remote gdb...\n");
- else {
- printk(KERN_CRIT "KGDB cannot initialize I/O yet.\n");
- return 0;
- }

- breakpoint();
+ if (kgdb_state == KGDB_FULLY_INITIALIZED)
+ kgdb_initial_breakpoint();
+ else
+ kgdb_state = KGDB_DELAYED_CONNECTION;

return 0;
}
Index: b/arch/x86/kernel/setup_64.c
===================================================================
--- a/arch/x86/kernel/setup_64.c
+++ b/arch/x86/kernel/setup_64.c
@@ -92,6 +92,8 @@ unsigned long saved_video_mode;

int force_mwait __cpuinitdata;

+void early_trap_init(void);
+
/*
* Early DMI memory
*/
@@ -264,6 +266,8 @@ void __init setup_arch(char **cmdline_p)
{
unsigned i;

+ early_trap_init();
+
printk(KERN_INFO "Command line: %s\n", boot_command_line);

ROOT_DEV = old_decode_dev(boot_params.hdr.root_dev);
Index: b/include/asm-x86/kgdb.h
===================================================================
--- a/include/asm-x86/kgdb.h
+++ b/include/asm-x86/kgdb.h
@@ -76,9 +76,6 @@ enum regnames { _AX, /* 0 */

#ifndef __ASSEMBLY__
#define BREAKPOINT() asm(" int $3");
-#ifndef CONFIG_X86_32
-#define EXCEPTION_STACK_READY() ((&__get_cpu_var(init_tss))[0].x86_tss.ist[0])
-#endif /* ! CONFIG_X86_32 */
#define BREAK_INSTR_SIZE 1
#define CACHE_FLUSH_IS_SAFE 1
#endif /* !__ASSEMBLY__ */
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/