Date: Wed, 27 Sep 2006 17:18:14 +0400 From: Gleb Smirnoff <glebius@FreeBSD.org> To: David Christensen <davidch@broadcom.com> Cc: brad@openbsd.org, oleg@FreeBSD.org, net@FreeBSD.org Subject: Re: bge(4) one packet wedge Message-ID: <20060927131814.GK59833@FreeBSD.org> In-Reply-To: <09BFF2FA5EAB4A45B6655E151BBDD90301D43002@NT-IRVA-0750.brcm.ad.broadcom.com> References: <20060823161649.GE76666@cell.sick.ru> <09BFF2FA5EAB4A45B6655E151BBDD90301D43002@NT-IRVA-0750.brcm.ad.broadcom.com>
next in thread | previous in thread | raw e-mail | index | archive | help
--v9Ux+11Zm5mwPlX6
Content-Type: text/plain; charset=koi8-r
Content-Disposition: inline
On Wed, Aug 23, 2006 at 12:53:49PM -0700, David Christensen wrote:
D> This "lost interrupt" type of problem is addressed by the use of the
D> status_tag
D> field in the status block. (Listed as bge_rsvd0 in the bge_status_block
D> structure).
D> Everytime the status block is updated a new tag value is written to the
D> status block.
D> When the ISR starts the driver should record the status_tag value. At
D> the end
D> of the ISR, the driver should compare the current status_tag value is
D> the status
D> block with the value recorded on entry to the ISR. If the values are
D> the same
D> then no additional status block updates have occurred so there shouldn't
D> be
D> any packets hanging around. If the values are different then additional
D> packets
D> or completions are waiting around so the ISR should loop around again.
D> At the
D> end of the ISR the driver will write the status_tag value it last
D> handled to a
D> mailbox register, letting the hardware know the last status block update
D> handled.
D> If necessary the hardware will generate a new interrupt and start the
D> process over
D> again.
D>
D> This entire process should be included in the Linux driver, I don't see
D> it being
D> used in the bge driver (bge_intr()).
Finally I got a NIC that has a chip that does tagged status block - 5701.
I've prepared a patch, that mimics Linux. If a chip can do status tag,
then we write it to mailbox register at end of ISR, as you have
described. If the chip can't, then we force coalescing once per second.
This should fix the problem correctly on the chips that support
status tag, and it is an ugly fix for chips that does not.
Unfortunately, the attached patch doesn't fix the problem on 5701. The
wedge occurs as before. And I see status tag updated, while the netperf
test has wedged:
(kgdb) p $sc->bge_last_tag
$45 = 239
(kgdb) p $sc->bge_last_tag
$46 = 240
(kgdb) p $sc->bge_last_tag
$47 = 241
(kgdb) p $sc->bge_last_tag
$48 = 242
I have no idea. :( May be you have one?
--
Totus tuus, Glebius.
GLEBIUS-RIPN GLEB-RIPE
--v9Ux+11Zm5mwPlX6
Content-Type: text/plain; charset=koi8-r
Content-Disposition: attachment; filename="bge.status_tag"
Index: if_bge.c
===================================================================
RCS file: /home/ncvs/src/sys/dev/bge/if_bge.c,v
retrieving revision 1.149
diff -u -p -r1.149 if_bge.c
--- if_bge.c 23 Sep 2006 18:55:49 -0000 1.149
+++ if_bge.c 27 Sep 2006 12:14:15 -0000
@@ -1094,7 +1094,7 @@ bge_chipinit(struct bge_softc *sc)
int i;
/* Set endianness before we access any non-PCI registers. */
- pci_write_config(sc->bge_dev, BGE_PCI_MISC_CTL, BGE_INIT, 4);
+ pci_write_config(sc->bge_dev, BGE_PCI_MISC_CTL, sc->bge_misc_ctl, 4);
/*
* Check the 'ROM failed' bit on the RX CPU to see if
@@ -2104,6 +2104,70 @@ bge_dma_alloc(device_t dev)
return (0);
}
+static void
+bge_recognize(struct bge_softc *sc)
+{
+ device_t dev = sc->bge_dev;
+ uint32_t misccfg;
+
+ /* Save ASIC rev. */
+ sc->bge_chipid =
+ pci_read_config(dev, BGE_PCI_MISC_CTL, 4) &
+ BGE_PCIMISCCTL_ASICREV;
+ sc->bge_asicrev = BGE_ASICREV(sc->bge_chipid);
+ sc->bge_chiprev = BGE_CHIPREV(sc->bge_chipid);
+
+ sc->bge_misc_ctl = (BGE_HIF_SWAP_OPTIONS |
+ BGE_PCIMISCCTL_CLEAR_INTA |
+ BGE_PCIMISCCTL_MASK_PCI_INTR |
+ BGE_PCIMISCCTL_INDIRECT_ACCESS);
+ sc->bge_hcc_mode = 0;
+
+ /*
+ * XXX: Broadcom Linux driver. Not in specs or eratta.
+ * PCI-Express?
+ */
+ if (BGE_IS_5705_OR_BEYOND(sc)) {
+ uint32_t v;
+
+ v = pci_read_config(dev, BGE_PCI_MSI_CAPID, 4);
+ if (((v >> 8) & 0xff) == BGE_PCIE_CAPID_REG) {
+ v = pci_read_config(dev, BGE_PCIE_CAPID_REG, 4);
+ if ((v & 0xff) == BGE_PCIE_CAPID)
+ sc->bge_flags |= BGE_FLAG_PCIE;
+ }
+ }
+
+ /*
+ * PCI-X?
+ */
+ if ((pci_read_config(sc->bge_dev, BGE_PCI_PCISTATE, 4) &
+ BGE_PCISTATE_PCI_BUSMODE) == 0)
+ sc->bge_flags |= BGE_FLAG_PCIX;
+
+ misccfg = CSR_READ_4(sc, BGE_MISC_CFG) & BGE_MISCCFG_BOARD_ID_MASK;
+ if (sc->bge_asicrev == BGE_ASICREV_BCM5705 &&
+ (misccfg == BGE_MISCCFG_BOARD_ID_5788 ||
+ misccfg == BGE_MISCCFG_BOARD_ID_5788M))
+ sc->bge_flags |= BGE_FLAG_5788;
+
+ if (sc->bge_chiprev != BGE_CHIPREV_5700_AX &&
+ sc->bge_chiprev != BGE_CHIPREV_5700_BX)
+ sc->bge_hcc_mode |= BGE_STATBLKSZ_32BYTE;
+
+ /*
+ * 5788 and 5700 does not support tagging the status block.
+ * This is workarounded in bge_tick_locked().
+ */
+ if (!(sc->bge_flags & BGE_FLAG_5788) &&
+ sc->bge_asicrev != BGE_ASICREV_BCM5700) {
+ sc->bge_flags |= BGE_FLAG_STATUSTAG;
+ sc->bge_hcc_mode |= (BGE_HCCMODE_CLRTICK_RXBD |
+ BGE_HCCMODE_CLRTICK_TXBD);
+ sc->bge_misc_ctl |= BGE_PCIMISCCTL_TAGGED_STATUS;
+ }
+}
+
static int
bge_attach(device_t dev)
{
@@ -2150,35 +2214,8 @@ bge_attach(device_t dev)
BGE_LOCK_INIT(sc, device_get_nameunit(dev));
- /* Save ASIC rev. */
-
- sc->bge_chipid =
- pci_read_config(dev, BGE_PCI_MISC_CTL, 4) &
- BGE_PCIMISCCTL_ASICREV;
- sc->bge_asicrev = BGE_ASICREV(sc->bge_chipid);
- sc->bge_chiprev = BGE_CHIPREV(sc->bge_chipid);
-
- /*
- * XXX: Broadcom Linux driver. Not in specs or eratta.
- * PCI-Express?
- */
- if (BGE_IS_5705_OR_BEYOND(sc)) {
- uint32_t v;
-
- v = pci_read_config(dev, BGE_PCI_MSI_CAPID, 4);
- if (((v >> 8) & 0xff) == BGE_PCIE_CAPID_REG) {
- v = pci_read_config(dev, BGE_PCIE_CAPID_REG, 4);
- if ((v & 0xff) == BGE_PCIE_CAPID)
- sc->bge_flags |= BGE_FLAG_PCIE;
- }
- }
-
- /*
- * PCI-X ?
- */
- if ((pci_read_config(sc->bge_dev, BGE_PCI_PCISTATE, 4) &
- BGE_PCISTATE_PCI_BUSMODE) == 0)
- sc->bge_flags |= BGE_FLAG_PCIX;
+ /* Fill in bge_flags, recognize features/bugs.*/
+ bge_recognize(sc);
/* Try to reset the chip. */
if (bge_reset(sc)) {
@@ -2866,16 +2903,13 @@ bge_poll(struct ifnet *ifp, enum poll_cm
static void
bge_intr(void *xsc)
{
- struct bge_softc *sc;
- struct ifnet *ifp;
+ struct bge_softc *sc = xsc;
+ struct ifnet *ifp = sc->bge_ifp;
+ struct bge_status_block *sblock = sc->bge_ldata.bge_status_block;
uint32_t statusword;
- sc = xsc;
-
BGE_LOCK(sc);
- ifp = sc->bge_ifp;
-
#ifdef DEVICE_POLLING
if (ifp->if_capenable & IFCAP_POLLING) {
BGE_UNLOCK(sc);
@@ -2883,6 +2917,15 @@ bge_intr(void *xsc)
}
#endif
+ if ((((sc->bge_flags & BGE_FLAG_STATUSTAG) &&
+ (sc->bge_last_tag == sblock->bge_tag)) ||
+ !(sblock->bge_status & BGE_STATUS_UPDATED)) &&
+ (pci_read_config(sc->bge_dev, BGE_PCI_PCISTATE, 4) &
+ BGE_PCISTATE_INTR_STATE)) {
+ /* Not our interrupt? */
+ BGE_UNLOCK(sc);
+ return;
+ }
/*
* Do the mandatory PCI flush as well as get the link status.
*/
@@ -2911,7 +2954,13 @@ bge_intr(void *xsc)
}
/* Re-enable interrupts. */
- CSR_WRITE_4(sc, BGE_MBX_IRQ0_LO, 0);
+ if (sc->bge_flags & BGE_FLAG_STATUSTAG) {
+ sc->bge_last_tag = sblock->bge_tag;
+ CSR_WRITE_4(sc, BGE_MBX_IRQ0_LO, (sc->bge_last_tag << 24));
+ } else {
+ sblock->bge_status &= ~BGE_STATUS_UPDATED;
+ CSR_WRITE_4(sc, BGE_MBX_IRQ0_LO, 0);
+ }
if (ifp->if_drv_flags & IFF_DRV_RUNNING &&
!IFQ_DRV_IS_EMPTY(&ifp->if_snd))
@@ -2974,6 +3023,16 @@ bge_tick_locked(struct bge_softc *sc)
bge_asf_driver_up(sc);
+ if (!(sc->bge_flags & BGE_FLAG_STATUSTAG)) {
+ struct bge_status_block *sblock = sc->bge_ldata.bge_status_block;
+
+ if (sblock->bge_status & BGE_STATUS_UPDATED)
+ BGE_SETBIT(sc, BGE_MISC_LOCAL_CTL, BGE_MLC_INTR_SET);
+ else
+ CSR_WRITE_4(sc, BGE_HCC_MODE, BGE_HCCMODE_ENABLE |
+ BGE_HCCMODE_COAL_NOW);
+ }
+
callout_reset(&sc->bge_stat_ch, hz, bge_tick, sc);
}
Index: if_bgereg.h
===================================================================
RCS file: /home/ncvs/src/sys/dev/bge/if_bgereg.h,v
retrieving revision 1.55
diff -u -p -r1.55 if_bgereg.h
--- if_bgereg.h 9 Sep 2006 03:36:57 -0000 1.55
+++ if_bgereg.h 27 Sep 2006 12:15:12 -0000
@@ -210,6 +210,7 @@
#define BGE_PCIMISCCTL_CLOCKCTL_RW 0x00000020
#define BGE_PCIMISCCTL_REG_WORDSWAP 0x00000040
#define BGE_PCIMISCCTL_INDIRECT_ACCESS 0x00000080
+#define BGE_PCIMISCCTL_TAGGED_STATUS 0x00000200
#define BGE_PCIMISCCTL_ASICREV 0xFFFF0000
#define BGE_HIF_SWAP_OPTIONS (BGE_PCIMISCCTL_ENDIAN_WORDSWAP)
@@ -223,10 +224,6 @@
BGE_MODECTL_BYTESWAP_DATA|BGE_MODECTL_WORDSWAP_DATA
#endif
-#define BGE_INIT \
- (BGE_HIF_SWAP_OPTIONS|BGE_PCIMISCCTL_CLEAR_INTA| \
- BGE_PCIMISCCTL_MASK_PCI_INTR|BGE_PCIMISCCTL_INDIRECT_ACCESS)
-
#define BGE_CHIPID_TIGON_I 0x40000000
#define BGE_CHIPID_TIGON_II 0x60000000
#define BGE_CHIPID_BCM5700_A0 0x70000000
@@ -1195,6 +1192,8 @@
#define BGE_STATBLKSZ_FULL 0x00000000
#define BGE_STATBLKSZ_64BYTE 0x00000080
#define BGE_STATBLKSZ_32BYTE 0x00000100
+#define BGE_HCCMODE_CLRTICK_RXBD 0x00000200
+#define BGE_HCCMODE_CLRTICK_TXBD 0x00000400
/* Host coalescing status register */
#define BGE_HCCSTAT_ERROR 0x00000004
@@ -1690,6 +1689,9 @@
/* Misc. config register */
#define BGE_MISCCFG_RESET_CORE_CLOCKS 0x00000001
#define BGE_MISCCFG_TIMER_PRESCALER 0x000000FE
+#define BGE_MISCCFG_BOARD_ID_MASK 0x0001e000
+#define BGE_MISCCFG_BOARD_ID_5788 0x00010000
+#define BGE_MISCCFG_BOARD_ID_5788M 0x00018000
#define BGE_32BITTIME_66MHZ (0x41 << 1)
@@ -1939,7 +1941,10 @@ struct bge_sts_idx {
struct bge_status_block {
uint32_t bge_status;
- uint32_t bge_rsvd0;
+#define BGE_STATUS_UPDATED 0x00000001
+#define BGE_STATUS_LINKEV 0x00000002
+#define BGE_STATUS_ERROR 0x00000004
+ uint32_t bge_tag;
#if BYTE_ORDER == LITTLE_ENDIAN
uint16_t bge_rx_jumbo_cons_idx;
uint16_t bge_rx_std_cons_idx;
@@ -2463,6 +2468,8 @@ struct bge_softc {
#define BGE_FLAG_NO3LED 0x00000008
#define BGE_FLAG_PCIX 0x00000010
#define BGE_FLAG_PCIE 0x00000020
+#define BGE_FLAG_5788 0x00000040
+#define BGE_FLAG_STATUSTAG 0x00000080
uint32_t bge_chipid;
uint8_t bge_asicrev;
uint8_t bge_chiprev;
@@ -2470,6 +2477,7 @@ struct bge_softc {
uint8_t bge_asf_count;
struct bge_ring_data bge_ldata; /* rings */
struct bge_chain_data bge_cdata; /* mbufs */
+ uint32_t bge_last_tag;
uint16_t bge_tx_saved_considx;
uint16_t bge_rx_saved_considx;
uint16_t bge_ev_saved_considx;
@@ -2488,6 +2496,9 @@ struct bge_softc {
int bge_link; /* link state */
int bge_link_evt; /* pending link event */
struct callout bge_stat_ch;
+ /* Masks/values for some registers. */
+ uint32_t bge_hcc_mode; /* BGE_HCC_MODE */
+ uint32_t bge_misc_ctl; /* BGE_PCI_MISC_CTL */
char *bge_vpd_prodname;
char *bge_vpd_readonly;
u_long bge_rx_discards;
--v9Ux+11Zm5mwPlX6--
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20060927131814.GK59833>