Introduce a B_NO_LOCK_VECTOR flag to be used with install_io_interrupt_handler().

When specified it desigantes that the interrupt handler should not lock the
vector with a spinlock when executing the installed interrupt handlers. This
is necessary to allow the same interrupt vector to be handled in parallel on
different CPUs. And it is required for the CPU halt to work synchronously when
there is more than one AP CPU. Though the acquire_spinlock() should cause IPIs
to be processed, only this fixed the SMP_MSG_FLAG_SYNC problem for me.
Not locking is safe as long as it is guaranteed that no interrupt handler is
registered or removed while the interrupt handler is running. We can guarantee
this for the SMP interrupt handlers we install in arch_smp_init() as they are
never uninstalled. Probably this flag should be made private though.
Restored the SMP_MSG_FLAG_SYNC when entering the kernel debugger.

git-svn-id: file:///srv/svn/repos/haiku/haiku/trunk@23838 a95241bf-73f2-0310-859d-f6bbb57e9c96

headers/os/drivers/KernelExport.h

@@ -47,6 +47,7 @@ typedef int32 (*interrupt_handler)(void *data);
 
 /* Flags that can be passed to install_io_interrupt_handler() */
 #define B_NO_ENABLE_COUNTER		1
+#define B_NO_LOCK_VECTOR		2
 
 extern status_t		install_io_interrupt_handler(long interrupt_number,
 						interrupt_handler handler, void *data, ulong flags);

src/system/kernel/arch/x86/arch_smp.c

@@ -197,10 +197,10 @@ arch_smp_init(kernel_args *args)
 		arch_smp_per_cpu_init(args, 0);
 
 		// I/O interrupts start at ARCH_INTERRUPT_BASE, so all interrupts are shifted
-		install_io_interrupt_handler(0xfb - ARCH_INTERRUPT_BASE, &i386_timer_interrupt, NULL, 0);
-		install_io_interrupt_handler(0xfd - ARCH_INTERRUPT_BASE, &i386_ici_interrupt, NULL, 0);
-		install_io_interrupt_handler(0xfe - ARCH_INTERRUPT_BASE, &i386_smp_error_interrupt, NULL, 0);
-		install_io_interrupt_handler(0xff - ARCH_INTERRUPT_BASE, &i386_spurious_interrupt, NULL, 0);
+		install_io_interrupt_handler(0xfb - ARCH_INTERRUPT_BASE, &i386_timer_interrupt, NULL, B_NO_LOCK_VECTOR);
+		install_io_interrupt_handler(0xfd - ARCH_INTERRUPT_BASE, &i386_ici_interrupt, NULL, B_NO_LOCK_VECTOR);
+		install_io_interrupt_handler(0xfe - ARCH_INTERRUPT_BASE, &i386_smp_error_interrupt, NULL, B_NO_LOCK_VECTOR);
+		install_io_interrupt_handler(0xff - ARCH_INTERRUPT_BASE, &i386_spurious_interrupt, NULL, B_NO_LOCK_VECTOR);
 	}
 	return B_OK;
 }

src/system/kernel/debug/debug.cpp

@@ -1089,18 +1089,10 @@ kernel_debugger(const char *message)
 
 	if (sDebuggerOnCPU != smp_get_current_cpu() && smp_get_num_cpus() > 1) {
 		// First entry on a MP system, send a halt request to all of the other
-		// CPUs but don't block here if they currently can't process it.
-		// Should they try to enter the debugger they will be cought in the
-		// loop above.
-		// ToDo: investigate why we sometimes hang here with SMP_MSG_FLAG_SYNC
-		// and readd that flag and remove the spin below when it's fixed.
+		// CPUs. Should they try to enter the debugger they will be cought in
+		// the loop above.
 		smp_send_broadcast_ici(SMP_MSG_CPU_HALT, 0, 0, 0,
-			(void *)&inDebugger, 0);
-
-		// Delay a bit to give other CPUs the chance to process the halt
-		// request. Otherwise we could get debug output of other CPUs mixed
-		// into our own when enabling debug output below.
-		spin(50000);
+			(void *)&inDebugger, SMP_MSG_FLAG_SYNC);
 	}
 
 	if (sBlueScreenOutput) {

src/system/kernel/int.c

@@ -40,6 +40,7 @@ struct io_vector {
 	struct io_handler	handler_list;
 	spinlock			vector_lock;
 	int32				enable_count;
+	bool				no_lock_vector;
 #ifdef DEBUG_INT
 	int64				handled_count;
 	int64				unhandled_count;
@@ -124,6 +125,7 @@ int_init_post_vm(kernel_args *args)
 	for (i = 0; i < NUM_IO_VECTORS; i++) {
 		io_vectors[i].vector_lock = 0;			/* initialize spinlock */
 		io_vectors[i].enable_count = 0;
+		io_vectors[i].no_lock_vector = false;
 		#ifdef DEBUG_INT
 			io_vectors[i].handled_count = 0;
 			io_vectors[i].unhandled_count = 0;
@@ -189,6 +191,16 @@ install_io_interrupt_handler(long vector, interrupt_handler handler, void *data,
 			arch_int_enable_io_interrupt(vector);
 	}
 
+	// If B_NO_LOCK_VECTOR is specified this is a vector that is not supposed
+	// to have multiple handlers and does not require locking of the vector
+	// when entering the handler. For example this is used by internally
+	// registered interrupt handlers like for handling local APIC interrupts
+	// that may run concurently on multiple CPUs. Locking with a spinlock
+	// would in that case defeat the purpose as it would serialize calling the
+	// handlers in parallel on different CPUs.
+	if (flags & B_NO_LOCK_VECTOR)
+		io_vectors[vector].no_lock_vector = true;
+
 	release_spinlock(&io_vectors[vector].vector_lock);
 	restore_interrupts(state);
 
@@ -254,12 +266,14 @@ int_io_interrupt_handler(int vector, bool levelTriggered)
 	struct io_handler *io;
 	bool invokeScheduler = false, handled = false;
 
-	acquire_spinlock(&io_vectors[vector].vector_lock);
+	if (!io_vectors[vector].no_lock_vector)
+		acquire_spinlock(&io_vectors[vector].vector_lock);
 
 	// The list can be empty at this place
 	if (io_vectors[vector].handler_list.next == &io_vectors[vector].handler_list) {
 		dprintf("unhandled io interrupt %d\n", vector);
-		release_spinlock(&io_vectors[vector].vector_lock);
+		if (!io_vectors[vector].no_lock_vector)
+			release_spinlock(&io_vectors[vector].vector_lock);
 		return B_UNHANDLED_INTERRUPT;
 	}
 
@@ -306,7 +320,8 @@ int_io_interrupt_handler(int vector, bool levelTriggered)
 	}
 #endif
 
-	release_spinlock(&io_vectors[vector].vector_lock);
+	if (!io_vectors[vector].no_lock_vector)
+		release_spinlock(&io_vectors[vector].vector_lock);
 
 	if (levelTriggered)
 		return status;

src/system/kernel/smp.c

@@ -397,7 +397,7 @@ process_pending_ici(int32 currentCPU)
 
 		while (*haltValue != 0)
 			PAUSE();
-		
+
 		restore_interrupts(state);
 	}