Re: [Fwd: [PATCH] KDB: commandeer vector 0xfe for KDB_VECTOR]

Jay Lan (on Mon, 27 Oct 2008 19:00:28 -0700) wrote:
>From: Cliff Wickman <[email protected]>
>
>Hi Jay,
>
>Remember the conflict over interrupt vector 0xf8 between KDB and
>UV TLB shootdown?
>
>There didn't seem to be any clean way to resolve that.  The dynamic
>interrupt vector scheme that's going into the kernel does not allow
>assigning the same vector to all cpu's, or specifying a vector priority.
>And KDB needs to be higher priority than the timer interrupt, and UV TLB
>shootdown should be too.
>
>But since KDB only needs KDB_VECTOR to ping other cpu's it seems that
>it can use one of the system vectors temporarily (not the UV TLB shootdown's
>however, as it might be in-progress during the entry to KDB).  Dimitri
>suggested this approach.
>
>So I tried commandeering 0xfe (ERROR_APIC_VECTOR).
>According to arch/x86/kernel/apic_64.c:
> "This interrupt should never happen with our APIC/SMP architecture"
>
>When a cpu enters kdb, this patch causes it to commandeer that vector,
>send the IPI's, and wait till all other cpu's enter kdb.  Then restore
>the IDT to its previous state.
>
>It seems to work.  I tested on x86_64 and ia32.
>What do you think?
>
>Diffed against 2.6.27-rc8
>Signed-off-by: Cliff Wickman <[email protected]>
>---
> arch/x86/kdb/kdbasupport_32.c |   36 ++++++++++++++++++++++++++++++++----
> arch/x86/kdb/kdbasupport_64.c |   37 +++++++++++++++++++++++++++++++++----
> include/asm-x86/irq_vectors.h |   11 ++++++-----
> 3 files changed, 71 insertions(+), 13 deletions(-)
>
>Index: 081002.linus/arch/x86/kdb/kdbasupport_32.c
>===================================================================
>--- 081002.linus.orig/arch/x86/kdb/kdbasupport_32.c
>+++ 081002.linus/arch/x86/kdb/kdbasupport_32.c
>@@ -883,9 +883,6 @@ kdba_cpu_up(void)
> static int __init
> kdba_arch_init(void)
> {
>-#ifdef	CONFIG_SMP
>-	set_intr_gate(KDB_VECTOR, kdb_interrupt);
>-#endif
> 	set_intr_gate(KDBENTER_VECTOR, kdb_call);
> 	return 0;
> }
>@@ -1027,14 +1024,45 @@ kdba_verify_rw(unsigned long addr, size_
> 
> #include <mach_ipi.h>
> 
>+gate_desc save_idt[NR_VECTORS];
>+
>+void kdb_takeover_vector(int vector)
>+{
>+	memcpy(&save_idt[vector], &idt_table[vector], sizeof(gate_desc));
>+	set_intr_gate(KDB_VECTOR, kdb_interrupt);
>+	return;
>+}
>+
>+void kdb_giveback_vector(int vector)
>+{
>+	native_write_idt_entry(idt_table, vector, &save_idt[vector]);
>+	return;
>+}
>+
> /* When first entering KDB, try a normal IPI.  That reduces backtrace problems
>  * on the other cpus.
>  */
> void
> smp_kdb_stop(void)
> {
>-	if (!KDB_FLAG(NOIPI))
>+	int count;
>+	int cpu;
>+
>+	if (!KDB_FLAG(NOIPI)) {
>+		kdb_takeover_vector(KDB_VECTOR);
> 		send_IPI_allbutself(KDB_VECTOR);
>+		do {
>+			count = 0;
>+			for_each_possible_cpu(cpu) {
>+				if (cpu_isset(cpu, cpu_online_map)) {
>+					if (!KDB_STATE_CPU(KDB,cpu))
>+						/* count any cpus NOT in kdb */
>+					count++;
>+				}
>+			}
>+		} while (count != 0);
>+		kdb_giveback_vector(KDB_VECTOR);
>+	}
> }
> 
> /* The normal KDB IPI handler */

I don't see how this can work.  kdba_arch_init() currently maps
KDB_VECTOR to kdb_interrupt() on each cpu as that cpu is brought up.
IOW, KDB_VECTOR ends up being defined on ALL cpus before any KDB
interrupt is sent.

Your kdb_takeover_vector() only maps KDB_VECTOR to kdb_interrupt() on
the CURRENT cpu, then sends KDB_VECTOR to all the other cpus.  How do
the other cpus know what to do with KDB_VECTOR when they receive it?
They will have no definition for KDB_VECTOR so they will receive an
unexpected interrupt.

---------------------------
Use http://oss.sgi.com/ecartis to modify your settings or to unsubscribe.