* Re: Announce: kdb v4.4 x86-64 updates for for kernel 2.6.6
[not found] <250er-7es-5@gated-at.bofh.it>
@ 2004-06-09 9:37 ` Andi Kleen
0 siblings, 0 replies; 2+ messages in thread
From: Andi Kleen @ 2004-06-09 9:37 UTC (permalink / raw)
To: Keith Owens, jfv, linux-kernel
Keith Owens <kaos@sgi.com> writes:
> KDB (Linux Kernel Debugger) has been updated.
>
> ftp://oss.sgi.com/projects/kdb/download/v4.4/
>
> kdb-v4.4-2.6.6-x86-64-2.bz2 is available. The x86-64 patch is still a
> work in progress, use with care. Changelog extract.
>
> 2004-05-15 Jack F. Vogel <jfv@bluesong.net>
> * port to 2.6.6 for x86_64
It uses the wrong interfaces. The x86-64 die notifier was exactly
designed to avoid putting KDB hooks all over the kernel, but you
added them anyways. Please fix that.
In theory with the die hooks interface kdb could be a loadable (although
not unloadable) module btw.
Some comments:
+#if defined(CONFIG_SMP) && defined(CONFIG_KDB)
+static void do_ack_apic_irq(void)
+{
+ ack_APIC_irq();
+}
+#endif
NMIs don't need to be ACKed in the x86 APIC. The only reason to ack
something is that a higher priority interrupt can run, but there is no
higher priority interrupt than an NMI.
+#if defined(CONFIG_SMP) && defined(CONFIG_KDB)
+ /*
+ * Call the debugger to see if this NMI is due
+ * to a KDB requested IPI. If so, it will handle
+ */
+ if (kdb_ipi(regs, do_ack_apic_irq)){
+ nmi_exit();
+ return;
+ }
+#endif
Please just grab DIE_NMI_IPI in the die chain, which you're already
using. The NMI handler can veto the NMI then using NOTIFY_BAD.
The ack is also not needed.
@@ -367,6 +376,10 @@
handle_BUG(regs);
__die(str, regs, err);
oops_end();
+#ifdef CONFIG_KDB
+ kdb_diemsg = str;
+ kdb(KDB_REASON_OOPS, err, regs);
+#endif /* CONFIG_KDB */
__die already has a die chain hook for that. Use that instead.
+#if !defined(CONFIG_KDB)
DO_ERROR( 3, SIGTRAP, "int3", int3);
+#endif
and
+#ifdef CONFIG_KDB
+/*
+ * KDB Breakpoint vector
+ */
+asmlinkage int do_int3(struct pt_regs * regs, long error_code)
+{
+ if (kdb(KDB_REASON_BREAK, error_code, regs))
+ return 0;
+ do_trap(3, SIGTRAP, "int3", regs, error_code, NULL);
+ return 0;
+}
Unnecessary when you have a die chain handler (which you have
already). Take a look at how DO_ERROR is defined.
- set_intr_gate_ist(18,&machine_check, MCE_STACK);
+#ifdef CONFIG_KDB
+ {
+ set_intr_gate(18, &machine_check);
+ }
+ kdb_enablehwfault();
and
+void
+kdba_enable_mce(void)
+{
+ /* No longer required, arch/x86_64/kernel/bluesmoke.c does the job now *
/
+}
kdb_enablehwfault only calls kdba_enable_mce and is a complete
nop. Can be just removed.
Overall about 90% of your changes to arch/x86_64/kernel/traps.c are
unnecessary and the rest could be easily moved into arch/x86_64/kdb,
giving an completely independent and low maintainance kdb.
-Andi
^ permalink raw reply [flat|nested] 2+ messages in thread