Hi,

While I checked OpenBSD's bge(4) driver, I notice
our current bge(4) interrupt handler doesn't check
whether the interrupt is for bge(4) itself or not:
---
bge_intr(void *xsc)

 :

#ifdef notdef
	/* Avoid this for now -- checking this register is expensive. */
	/* Make sure this is really our interrupt. */
	if (!(CSR_READ_4(sc, BGE_MISC_LOCAL_CTL) & BGE_MLC_INTR_STATE))
		return (0);
#endif
---

I'm not sure how much expensive to access the register,
but this means each functions in the handlers are called
even on shared interrupts by other hardware.

OpenBSD and Broadcom drivers check bge_status block in
DMA descriptors and then BGE_PCI_PCISTAT register instead.
It seems reasonable for me but I can't test it right now
on a machine which shares the interrupt line.

Could anyone test the attached patch with any bge(4)?
---
Izumi Tsutsui


Index: if_bge.c
===================================================================
RCS file: /cvsroot/src/sys/dev/pci/if_bge.c,v
retrieving revision 1.122
diff -u -r1.122 if_bge.c
--- if_bge.c	26 Nov 2006 06:09:09 -0000	1.122
+++ if_bge.c	26 Nov 2006 16:45:50 -0000
@@ -3086,21 +3086,38 @@
 {
 	struct bge_softc *sc;
 	struct ifnet *ifp;
+	uint32_t bge_status;
 
 	sc = xsc;
-	ifp = &sc->ethercom.ec_if;
 
 #ifdef notdef
 	/* Avoid this for now -- checking this register is expensive. */
 	/* Make sure this is really our interrupt. */
 	if (!(CSR_READ_4(sc, BGE_MISC_LOCAL_CTL) & BGE_MLC_INTR_STATE))
 		return (0);
+#else
+	bge_status = sc->bge_rdata->bge_status_block.bge_status;
+
+	/*
+	 * From OpenBSD bge(4) driver:
+	 * It is possible for the interrupt to arrive before
+	 * the status block is updated prior to the interrupt.
+	 * Reading the PCI State register will confirm whether the
+	 * interrupt is ours and will flush the status block.
+	 */
+	if ((bge_status & BGE_STATFLAG_UPDATED) == 0 &&
+	    (CSR_READ_4(sc, BGE_PCI_PCISTATE) & BGE_PCISTATE_INTR_STATE) != 0)
+		return 0;
 #endif
+
 	/* Ack interrupt and stop others from occuring. */
 	CSR_WRITE_4(sc, BGE_MBX_IRQ0_LO, 1);
 
 	BGE_EVCNT_INCR(sc->bge_ev_intr);
 
+	/* clear status word */
+	sc->bge_rdata->bge_status_block.bge_status = 0;
+
 	/*
 	 * Process link state changes.
 	 * Grrr. The link status word in the status block does
@@ -3129,8 +3146,7 @@
 			    BRGPHY_INTRS);
 		}
 	} else {
-		if (sc->bge_rdata->bge_status_block.bge_status &
-		    BGE_STATFLAG_LINKSTATE_CHANGED) {
+		if (bge_status & BGE_STATFLAG_LINKSTATE_CHANGED) {
 			sc->bge_link = 0;
 			callout_stop(&sc->bge_timeout);
 			bge_tick(sc);
@@ -3141,6 +3157,7 @@
 		}
 	}
 
+	ifp = &sc->ethercom.ec_if;
 	if (ifp->if_flags & IFF_RUNNING) {
 		/* Check RX return ring producer/consumer */
 		bge_rxeof(sc);