On Tue, Jan 11, 2005 at 03:37:56PM -0500, Stephan Uphoff wrote:
> I think that sending IPIs needs to be protected with splclock() since
> some interrupts may send IPIs.
> x86_broadcast_ipi should also call x86_ipi to benefit from
> i82489_icr_wait.

Good point. splclock should stop IPIs from the clock interrupt as well
as those from interrupt handlers that cause TLB shootdown IPIs by doing
VM operations.

I assume you mean something like.. this (diff obfuscated a little because
I deleted the unused x86_self_ipi function):


Index: ipi.c
===================================================================
RCS file: /cvsroot/src/sys/arch/x86/x86/ipi.c,v
retrieving revision 1.5
diff -c -r1.5 ipi.c
*** ipi.c	13 Feb 2004 11:36:20 -0000	1.5
--- ipi.c	12 Jan 2005 03:53:15 -0000
***************
*** 55,67 ****
  int
  x86_send_ipi(struct cpu_info *ci, int ipimask)
  {
! 	int ret;
  
  	x86_atomic_setbits_l(&ci->ci_ipis, ipimask);
  
  	/* Don't send IPI to CPU which isn't (yet) running. */
! 	if (!(ci->ci_flags & CPUF_RUNNING))
  		return ENOENT;
  
  	ret = x86_ipi(LAPIC_IPI_VECTOR, ci->ci_apicid, LAPIC_DLMODE_FIXED);
  	if (ret != 0) {
--- 55,71 ----
  int
  x86_send_ipi(struct cpu_info *ci, int ipimask)
  {
! 	int ret, s;
! 
! 	s = splclock();
  
  	x86_atomic_setbits_l(&ci->ci_ipis, ipimask);
  
  	/* Don't send IPI to CPU which isn't (yet) running. */
! 	if (!(ci->ci_flags & CPUF_RUNNING)) {
! 		splx(s);
  		return ENOENT;
+ 	}
  
  	ret = x86_ipi(LAPIC_IPI_VECTOR, ci->ci_apicid, LAPIC_DLMODE_FIXED);
  	if (ret != 0) {
***************
*** 71,95 ****
  		    ci->ci_dev->dv_xname);
  	}
  
! 	return ret;
! }
  
! void
! x86_self_ipi (int vector)
! {
! 	i82489_writereg(LAPIC_ICRLO,
! 	    vector | LAPIC_DLMODE_FIXED | LAPIC_LVL_ASSERT | LAPIC_DEST_SELF);
  }
  
- 
  void
  x86_broadcast_ipi (int ipimask)
  {
  	struct cpu_info *ci, *self = curcpu();
! 	int count = 0;
! 
  	CPU_INFO_ITERATOR cii;
  
  	for (CPU_INFO_FOREACH(cii, ci)) {
  		if (ci == self)
  			continue;
--- 75,94 ----
  		    ci->ci_dev->dv_xname);
  	}
  
! 	splx(s);
  
! 	return ret;
  }
  
  void
  x86_broadcast_ipi (int ipimask)
  {
  	struct cpu_info *ci, *self = curcpu();
! 	int count = 0, s;
  	CPU_INFO_ITERATOR cii;
  
+ 	s = splclock();
+ 
  	for (CPU_INFO_FOREACH(cii, ci)) {
  		if (ci == self)
  			continue;
***************
*** 98,109 ****
  		x86_atomic_setbits_l(&ci->ci_ipis, ipimask);
  		count++;
  	}
! 	if (!count)
  		return;
  
! 	i82489_writereg(LAPIC_ICRLO,
! 	    LAPIC_IPI_VECTOR | LAPIC_DLMODE_FIXED | LAPIC_LVL_ASSERT |
! 	    LAPIC_DEST_ALLEXCL);
  }
  
  void
--- 97,110 ----
  		x86_atomic_setbits_l(&ci->ci_ipis, ipimask);
  		count++;
  	}
! 	if (!count) {
! 		splx(s);
  		return;
+ 	}
+ 
+ 	x86_ipi(LAPIC_IPI_VECTOR, LAPIC_DEST_ALLEXCL, LAPIC_DLMODE_FIXED);
  
! 	splx(s);
  }
  
  void