[PATCH] add and handle NMI_VECTOR

[PATCH] add and handle NMI_VECTOR

[Jan Beulich]

[-- Attachment #1: Type: text/plain, Size: 1399 bytes --]

(Note: Patch also attached because the inline version is certain to get
line wrapped.)

Declare NMI_VECTOR and handle it in the IPI sending code.

Signed-off-by: Jan Beulich <jbeulich@novell.com>

diff -Npru 2.6.13/include/asm-x86_64/hw_irq.h
2.6.13-x86_64-nmi-ipi/include/asm-x86_64/hw_irq.h
--- 2.6.13/include/asm-x86_64/hw_irq.h	2005-08-29 01:41:01.000000000
+0200
+++ 2.6.13-x86_64-nmi-ipi/include/asm-x86_64/hw_irq.h	2005-03-16
12:24:42.000000000 +0100
@@ -26,6 +26,7 @@
 struct hw_interrupt_type;
 #endif
 
+#define NMI_VECTOR		0x02
 /*
  * IDT vectors usable for external interrupt sources start
  * at 0x20:
diff -Npru 2.6.13/include/asm-x86_64/ipi.h
2.6.13-x86_64-nmi-ipi/include/asm-x86_64/ipi.h
--- 2.6.13/include/asm-x86_64/ipi.h	2005-08-29 01:41:01.000000000
+0200
+++ 2.6.13-x86_64-nmi-ipi/include/asm-x86_64/ipi.h	2005-09-01
11:32:12.000000000 +0200
@@ -31,9 +31,20 @@
 
 static inline unsigned int __prepare_ICR (unsigned int shortcut, int
vector, unsigned int dest)
 {
-	unsigned int icr =  APIC_DM_FIXED | shortcut | vector | dest;
-	if (vector == KDB_VECTOR)
-		icr = (icr & (~APIC_VECTOR_MASK)) | APIC_DM_NMI;
+	unsigned int icr = shortcut | dest;
+
+	switch (vector) {
+	default:
+		icr |= APIC_DM_FIXED | vector;
+		break;
+	case NMI_VECTOR:
+		/*
+		 * Setup KDB IPI to be delivered as an NMI
+		 */
+	case KDB_VECTOR:
+		icr |= APIC_DM_NMI;
+		break;
+	}
 	return icr;
 }
 


[-- Attachment #2: linux-2.6.13-x86_64-nmi-ipi.patch --]
[-- Type: application/octet-stream, Size: 1398 bytes --]

(Note: Patch also attached because the inline version is certain to get
line wrapped.)

Declare NMI_VECTOR and handle it in the IPI sending code.

Signed-off-by: Jan Beulich <jbeulich@novell.com>

diff -Npru 2.6.13/include/asm-x86_64/hw_irq.h 2.6.13-x86_64-nmi-ipi/include/asm-x86_64/hw_irq.h
--- 2.6.13/include/asm-x86_64/hw_irq.h	2005-08-29 01:41:01.000000000 +0200
+++ 2.6.13-x86_64-nmi-ipi/include/asm-x86_64/hw_irq.h	2005-03-16 12:24:42.000000000 +0100
@@ -26,6 +26,7 @@
 struct hw_interrupt_type;
 #endif
 
+#define NMI_VECTOR		0x02
 /*
  * IDT vectors usable for external interrupt sources start
  * at 0x20:
diff -Npru 2.6.13/include/asm-x86_64/ipi.h 2.6.13-x86_64-nmi-ipi/include/asm-x86_64/ipi.h
--- 2.6.13/include/asm-x86_64/ipi.h	2005-08-29 01:41:01.000000000 +0200
+++ 2.6.13-x86_64-nmi-ipi/include/asm-x86_64/ipi.h	2005-09-01 11:32:12.000000000 +0200
@@ -31,9 +31,20 @@
 
 static inline unsigned int __prepare_ICR (unsigned int shortcut, int vector, unsigned int dest)
 {
-	unsigned int icr =  APIC_DM_FIXED | shortcut | vector | dest;
-	if (vector == KDB_VECTOR)
-		icr = (icr & (~APIC_VECTOR_MASK)) | APIC_DM_NMI;
+	unsigned int icr = shortcut | dest;
+
+	switch (vector) {
+	default:
+		icr |= APIC_DM_FIXED | vector;
+		break;
+	case NMI_VECTOR:
+		/*
+		 * Setup KDB IPI to be delivered as an NMI
+		 */
+	case KDB_VECTOR:
+		icr |= APIC_DM_NMI;
+		break;
+	}
 	return icr;
 }
 

Re: [discuss] [PATCH] add and handle NMI_VECTOR

[Andi Kleen]

On Thu, Sep 08, 2005 at 06:07:56PM +0200, Jan Beulich wrote:
> (Note: Patch also attached because the inline version is certain to get
> line wrapped.)
> 
> Declare NMI_VECTOR and handle it in the IPI sending code.

The earlier consensus was to just rename KDB_VECTOR to NMI vector.

I added the following patch.

-Andi


Rename KDB_VECTOR to NMI_VECTOR

As a generic NMI IPI vector to be used by debuggers.

And clean up the ICR setup for that slightly (code is equivalent, but cleaner 
now)

Signed-off-by: Andi Kleen <ak@suse.de>

Index: linux/include/asm-x86_64/hw_irq.h
===================================================================
--- linux.orig/include/asm-x86_64/hw_irq.h
+++ linux/include/asm-x86_64/hw_irq.h
@@ -52,7 +52,7 @@ struct hw_interrupt_type;
 #define ERROR_APIC_VECTOR	0xfe
 #define RESCHEDULE_VECTOR	0xfd
 #define CALL_FUNCTION_VECTOR	0xfc
-#define KDB_VECTOR		0xfb	/* reserved for KDB */
+#define NMI_VECTOR		0xfb	/* IPI NMIs for debugging */
 #define THERMAL_APIC_VECTOR	0xfa
 /* 0xf9 free */
 #define INVALIDATE_TLB_VECTOR_END	0xf8
Index: linux/include/asm-x86_64/ipi.h
===================================================================
--- linux.orig/include/asm-x86_64/ipi.h
+++ linux/include/asm-x86_64/ipi.h
@@ -29,11 +29,14 @@
  * We use 'broadcast', CPU->CPU IPIs and self-IPIs too.
  */
 
-static inline unsigned int __prepare_ICR (unsigned int shortcut, int vector, unsigned int dest)
+static inline unsigned int
+__prepare_ICR (unsigned int shortcut, int vector, unsigned int dest)
 {
-	unsigned int icr =  APIC_DM_FIXED | shortcut | vector | dest;
-	if (vector == KDB_VECTOR)
-		icr = (icr & (~APIC_VECTOR_MASK)) | APIC_DM_NMI;
+	unsigned int icr =  shortcut | dest;
+	if (vector == NMI_VECTOR)
+		icr |= APIC_DM_DMI;
+	else
+		icr |= APIC_DM_FIXED | vector;
 	return icr;
 }
 

Re: [discuss] [PATCH] add and handle NMI_VECTOR II

[Andi Kleen]

On Fri, Sep 09, 2005 at 02:20:45AM +0200, Andi Kleen wrote:
> On Thu, Sep 08, 2005 at 06:07:56PM +0200, Jan Beulich wrote:
> > (Note: Patch also attached because the inline version is certain to get
> > line wrapped.)
> > 
> > Declare NMI_VECTOR and handle it in the IPI sending code.
> 
> The earlier consensus was to just rename KDB_VECTOR to NMI vector.
> 
> I added the following patch.

[...]

The patch was obviously half backed. Here's one that actually
compiles.

-Andi


Rename KDB_VECTOR to NMI_VECTOR

As a generic NMI IPI vector to be used by debuggers.

And clean up the ICR setup for that slightly (code is equivalent, but cleaner 
now)

Signed-off-by: Andi Kleen <ak@suse.de>

Index: linux/include/asm-x86_64/hw_irq.h
===================================================================
--- linux.orig/include/asm-x86_64/hw_irq.h
+++ linux/include/asm-x86_64/hw_irq.h
@@ -52,7 +52,7 @@ struct hw_interrupt_type;
 #define ERROR_APIC_VECTOR	0xfe
 #define RESCHEDULE_VECTOR	0xfd
 #define CALL_FUNCTION_VECTOR	0xfc
-#define KDB_VECTOR		0xfb	/* reserved for KDB */
+#define NMI_VECTOR		0xfb	/* IPI NMIs for debugging */
 #define THERMAL_APIC_VECTOR	0xfa
 /* 0xf9 free */
 #define INVALIDATE_TLB_VECTOR_END	0xf8
Index: linux/include/asm-x86_64/ipi.h
===================================================================
--- linux.orig/include/asm-x86_64/ipi.h
+++ linux/include/asm-x86_64/ipi.h
@@ -29,11 +29,14 @@
  * We use 'broadcast', CPU->CPU IPIs and self-IPIs too.
  */
 
-static inline unsigned int __prepare_ICR (unsigned int shortcut, int vector, unsigned int dest)
+static inline unsigned int 
+__prepare_ICR (unsigned int shortcut, int vector, unsigned int dest)
 {
-	unsigned int icr =  APIC_DM_FIXED | shortcut | vector | dest;
-	if (vector == KDB_VECTOR)
-		icr = (icr & (~APIC_VECTOR_MASK)) | APIC_DM_NMI;
+	unsigned int icr =  shortcut | dest;
+	if (vector == NMI_VECTOR)
+		icr |= APIC_DM_NMI;
+	else
+		icr |= APIC_DM_FIXED | vector;
 	return icr;
 }
 
Index: linux/arch/x86_64/kernel/traps.c
===================================================================
--- linux.orig/arch/x86_64/kernel/traps.c
+++ linux/arch/x86_64/kernel/traps.c
@@ -931,7 +931,7 @@ void __init trap_init(void)
 	set_system_gate(IA32_SYSCALL_VECTOR, ia32_syscall);
 #endif
        
-	set_intr_gate(KDB_VECTOR, call_debug);
+	set_intr_gate(NMI_VECTOR, call_debug);
        
 	/*
 	 * Should be a barrier for any external CPU state.
Index: linux/arch/x86_64/kernel/i8259.c
===================================================================
--- linux.orig/arch/x86_64/kernel/i8259.c
+++ linux/arch/x86_64/kernel/i8259.c
@@ -551,7 +551,7 @@ void __init init_IRQ(void)
 		int vector = FIRST_EXTERNAL_VECTOR + i;
 		if (i >= NR_IRQS)
 			break;
-		if (vector != IA32_SYSCALL_VECTOR && vector != KDB_VECTOR) { 
+		if (vector != IA32_SYSCALL_VECTOR && vector != NMI_VECTOR) { 
 			set_intr_gate(vector, interrupt[i]);
 	}
 	}
Index: linux/arch/x86_64/kernel/smp.c
===================================================================
--- linux.orig/arch/x86_64/kernel/smp.c
+++ linux/arch/x86_64/kernel/smp.c
@@ -283,11 +283,6 @@ void flush_tlb_all(void)
 	on_each_cpu(do_flush_tlb_all, NULL, 1, 1);
 }
 
-void smp_kdb_stop(void)
-{
-	send_IPI_allbutself(KDB_VECTOR);
-}
-
 /*
  * this function sends a 'reschedule' IPI to another CPU.
  * it goes straight through and wastes no time serializing

Re: [discuss] [PATCH] add and handle NMI_VECTOR II

[Jan Beulich]

>Index: linux/include/asm-x86_64/hw_irq.h
>===================================================================
>--- linux.orig/include/asm-x86_64/hw_irq.h
>+++ linux/include/asm-x86_64/hw_irq.h
>@@ -52,7 +52,7 @@ struct hw_interrupt_type;
> #define ERROR_APIC_VECTOR	0xfe
> #define RESCHEDULE_VECTOR	0xfd
> #define CALL_FUNCTION_VECTOR	0xfc
>-#define KDB_VECTOR		0xfb	/* reserved for KDB */
>+#define NMI_VECTOR		0xfb	/* IPI NMIs for debugging */
> #define THERMAL_APIC_VECTOR	0xfa
> /* 0xf9 free */
> #define INVALIDATE_TLB_VECTOR_END	0xf8

This doesn't seem too good an idea: the NMI vector really is 0x02
(architecturally), so defining it to something else seems at least odd.

>Index: linux/arch/x86_64/kernel/traps.c
>===================================================================
>--- linux.orig/arch/x86_64/kernel/traps.c
>+++ linux/arch/x86_64/kernel/traps.c
>@@ -931,7 +931,7 @@ void __init trap_init(void)
> 	set_system_gate(IA32_SYSCALL_VECTOR, ia32_syscall);
> #endif
>        
>-	set_intr_gate(KDB_VECTOR, call_debug);
>+	set_intr_gate(NMI_VECTOR, call_debug);
>        
> 	/*
> 	 * Should be a barrier for any external CPU state.

I never understood what this does. If you deliver the IPI as an NMI,
it'll never arrive at this vector, and why would anyone want to put an
"int $NMI_VECTOR" anywhere?

Jan

Re: [discuss] [PATCH] add and handle NMI_VECTOR II

[Andi Kleen]

On Friday 09 September 2005 08:42, Jan Beulich wrote:
> >Index: linux/include/asm-x86_64/hw_irq.h
> >===================================================================
> >--- linux.orig/include/asm-x86_64/hw_irq.h
> >+++ linux/include/asm-x86_64/hw_irq.h
> >@@ -52,7 +52,7 @@ struct hw_interrupt_type;
> > #define ERROR_APIC_VECTOR	0xfe
> > #define RESCHEDULE_VECTOR	0xfd
> > #define CALL_FUNCTION_VECTOR	0xfc
> >-#define KDB_VECTOR		0xfb	/* reserved for KDB */
> >+#define NMI_VECTOR		0xfb	/* IPI NMIs for debugging */
> > #define THERMAL_APIC_VECTOR	0xfa
> > /* 0xf9 free */
> > #define INVALIDATE_TLB_VECTOR_END	0xf8
>
> This doesn't seem too good an idea: the NMI vector really is 0x02
> (architecturally), so defining it to something else seems at least odd.

Good point. Actually the consensus (or the patch from T.Rini) was 
to name it DEBUG_VECTOR, but I messed that up. Will fix.

>
> >Index: linux/arch/x86_64/kernel/traps.c
> >===================================================================
> >--- linux.orig/arch/x86_64/kernel/traps.c
> >+++ linux/arch/x86_64/kernel/traps.c
> >@@ -931,7 +931,7 @@ void __init trap_init(void)
> > 	set_system_gate(IA32_SYSCALL_VECTOR, ia32_syscall);
> > #endif
> >
> >-	set_intr_gate(KDB_VECTOR, call_debug);
> >+	set_intr_gate(NMI_VECTOR, call_debug);
> >
> > 	/*
> > 	 * Should be a barrier for any external CPU state.
>
> I never understood what this does. If you deliver the IPI as an NMI,
> it'll never arrive at this vector, and why would anyone want to put an
> "int $NMI_VECTOR" anywhere?

You can force an NMI when sending an IPI by setting the right bits
in ICR. That is what it is used for.

-Andi

Re: [discuss] [PATCH] add and handle NMI_VECTOR II

[Jan Beulich]

>> >Index: linux/arch/x86_64/kernel/traps.c
>>
>===================================================================
>> >--- linux.orig/arch/x86_64/kernel/traps.c
>> >+++ linux/arch/x86_64/kernel/traps.c
>> >@@ -931,7 +931,7 @@ void __init trap_init(void)
>> > 	set_system_gate(IA32_SYSCALL_VECTOR, ia32_syscall);
>> > #endif
>> >
>> >-	set_intr_gate(KDB_VECTOR, call_debug);
>> >+	set_intr_gate(NMI_VECTOR, call_debug);
>> >
>> > 	/*
>> > 	 * Should be a barrier for any external CPU state.
>>
>> I never understood what this does. If you deliver the IPI as an
NMI,
>> it'll never arrive at this vector, and why would anyone want to put
an
>> "int $NMI_VECTOR" anywhere?
>
>You can force an NMI when sending an IPI by setting the right bits
>in ICR. That is what it is used for.

??? This is what the code doing the setup does. But the question was -
what do you need the IDT entry for?

Jan

Re: [discuss] [PATCH] add and handle NMI_VECTOR II

[Andi Kleen]

> ??? This is what the code doing the setup does. But the question was -
> what do you need the IDT entry for?

Without an IDT entry you cannot receive it? 

-Andi

Re: [discuss] [PATCH] add and handle NMI_VECTOR II

[Jan Beulich]

>>> Andi Kleen <ak@suse.de> 09.09.05 09:14:07 >>>
>> ??? This is what the code doing the setup does. But the question was
-
>> what do you need the IDT entry for?
>
>Without an IDT entry you cannot receive it? 

But that's the point - if it's delivered as an NMI, it'll arrive
through vector 2 (the vector information specified is ignored).

Jan

Re: [discuss] [PATCH] add and handle NMI_VECTOR II

[Andi Kleen]

On Friday 09 September 2005 09:57, Jan Beulich wrote:
> >>> Andi Kleen <ak@suse.de> 09.09.05 09:14:07 >>>
> >>
> >> ??? This is what the code doing the setup does. But the question was
>
> -
>
> >> what do you need the IDT entry for?
> >
> >Without an IDT entry you cannot receive it?
>
> But that's the point - if it's delivered as an NMI, it'll arrive
> through vector 2 (the vector information specified is ignored).

Good point. I wonder how this ever worked. I'll remove it.

-Andi

Re: [discuss] [PATCH] add and handle NMI_VECTOR

[Andi Kleen]

On Thursday 08 September 2005 18:07, Jan Beulich wrote:
> (Note: Patch also attached because the inline version is certain to get
> line wrapped.)
>
> Declare NMI_VECTOR and handle it in the IPI sending code.

I would apply it now, but the attachment is empty. Can you resend?

-Andi

Re: [discuss] [PATCH] add and handle NMI_VECTOR

[Jan Beulich]

[-- Attachment #1: Type: text/plain, Size: 347 bytes --]

Here it is.

>>> Andi Kleen <ak@suse.de> 09.09.05 10:50:01 >>>
On Thursday 08 September 2005 18:07, Jan Beulich wrote:
> (Note: Patch also attached because the inline version is certain to
get
> line wrapped.)
>
> Declare NMI_VECTOR and handle it in the IPI sending code.

I would apply it now, but the attachment is empty. Can you resend?

-Andi

[-- Attachment #2: linux-2.6.13-x86_64-nmi-ipi.patch --]
[-- Type: application/octet-stream, Size: 1398 bytes --]

(Note: Patch also attached because the inline version is certain to get
line wrapped.)

Declare NMI_VECTOR and handle it in the IPI sending code.

Signed-off-by: Jan Beulich <jbeulich@novell.com>

diff -Npru 2.6.13/include/asm-x86_64/hw_irq.h 2.6.13-x86_64-nmi-ipi/include/asm-x86_64/hw_irq.h
--- 2.6.13/include/asm-x86_64/hw_irq.h	2005-08-29 01:41:01.000000000 +0200
+++ 2.6.13-x86_64-nmi-ipi/include/asm-x86_64/hw_irq.h	2005-03-16 12:24:42.000000000 +0100
@@ -26,6 +26,7 @@
 struct hw_interrupt_type;
 #endif
 
+#define NMI_VECTOR		0x02
 /*
  * IDT vectors usable for external interrupt sources start
  * at 0x20:
diff -Npru 2.6.13/include/asm-x86_64/ipi.h 2.6.13-x86_64-nmi-ipi/include/asm-x86_64/ipi.h
--- 2.6.13/include/asm-x86_64/ipi.h	2005-08-29 01:41:01.000000000 +0200
+++ 2.6.13-x86_64-nmi-ipi/include/asm-x86_64/ipi.h	2005-09-01 11:32:12.000000000 +0200
@@ -31,9 +31,20 @@
 
 static inline unsigned int __prepare_ICR (unsigned int shortcut, int vector, unsigned int dest)
 {
-	unsigned int icr =  APIC_DM_FIXED | shortcut | vector | dest;
-	if (vector == KDB_VECTOR)
-		icr = (icr & (~APIC_VECTOR_MASK)) | APIC_DM_NMI;
+	unsigned int icr = shortcut | dest;
+
+	switch (vector) {
+	default:
+		icr |= APIC_DM_FIXED | vector;
+		break;
+	case NMI_VECTOR:
+		/*
+		 * Setup KDB IPI to be delivered as an NMI
+		 */
+	case KDB_VECTOR:
+		icr |= APIC_DM_NMI;
+		break;
+	}
 	return icr;
 }