svn commit: r199670 - head/sys/dev/bge

[Pyun YongHyeon]

On Sun, Nov 22, 2009 at 04:48:18PM -0700, Scott Long wrote:
> By definition, PCIe can't transfer across 4GB boundaries.  It's not a  
> bug specific to Broadcom.  If you're loading dynamic buffers (i.e.  
> mbufs), setting an appropriate boundary value in the tag will allow  
> busdma to take care of this.  If you're allocating static buffers,  
> busdma won't honor this.  But what you've done here is best anyways;  

Last time I tried boundary value for static buffer in dma tag it
didn't honor the boundary value so I blindly thought boundary value
also is not honored for dynamic buffers. It would be great if this
can be documented in man page.

> control buffers that are going to be frequently transferred are best  
> kept in the lower 4GB of the address space.  It simplifies PCIe  
> handling, and it's significantly faster on PCI/PCI-X.  So I'd suggest  
> making this the rule rather than the exception in the driver.
> 

If this is the case it would also affect other drivers(em(4) and
igb(4) etc). As John said it would be even better this could be
automatically handled in bus_dma. Patching every driver in tree to
reflect this would be pain, I guess.

Anyway I've made a new patch. Would you review this?
-------------- next part --------------
--- if_bge.c.orig	2009-11-22 13:45:55.000000000 -0800
+++ if_bge.c	2009-11-23 12:20:07.000000000 -0800
@@ -2028,7 +2028,8 @@
 		bus_dma_tag_destroy(sc->bge_cdata.bge_rx_mtag);
 	if (sc->bge_cdata.bge_tx_mtag)
 		bus_dma_tag_destroy(sc->bge_cdata.bge_tx_mtag);
-
+	if (sc->bge_cdata.bge_mbuf_tag)
+		bus_dma_tag_destroy(sc->bge_cdata.bge_mbuf_tag);
 
 	/* Destroy standard RX ring. */
 	if (sc->bge_cdata.bge_rx_std_ring_map)
@@ -2125,16 +2126,24 @@
 
 	sc = device_get_softc(dev);
 
-	lowaddr = BUS_SPACE_MAXADDR;
-	if ((sc->bge_flags & BGE_FLAG_40BIT_BUG) != 0)
-		lowaddr = BGE_DMA_MAXADDR;
-	if ((sc->bge_flags & BGE_FLAG_4G_BNDRY_BUG) != 0)
-		lowaddr = BUS_SPACE_MAXADDR_32BIT;
 	/*
 	 * Allocate the parent bus DMA tag appropriate for PCI.
+	 * All controllers that are not 5755 or higher have 4GB
+	 * boundary DMA bug.
+	 * Whenever an address crosses a multiple of the 4GB boundary
+	 * (including 4GB, 8Gb, 12Gb, etc.) and makes the transition
+	 * from 0xX_FFFF_FFFF to 0x(X+1)_0000_0000 an internal DMA
+	 * state machine will lockup and cause the device to hang.
+	 * According to Scott Long, PCIe can't transfer across 4GB
+	 * boundaries by definition. And control buffers that are
+	 * going to frequently transferred are best kept in the lower
+	 * 4GB of the address space. It simplifies PCIe handling and
+	 * it's significantly faster on PCI/PCI-X. So limit the
+	 * DMA address to be within 4GB address for all control
+	 * buffers(TX/RX/RX return/status block/statistics block etc).
 	 */
 	error = bus_dma_tag_create(bus_get_dma_tag(sc->bge_dev),
-	    1, 0, lowaddr, BUS_SPACE_MAXADDR, NULL,
+	    1, 0, BUS_SPACE_MAXADDR_32BIT, BUS_SPACE_MAXADDR, NULL,
 	    NULL, BUS_SPACE_MAXSIZE_32BIT, 0, BUS_SPACE_MAXSIZE_32BIT,
 	    0, NULL, NULL, &sc->bge_cdata.bge_parent_tag);
 
@@ -2143,6 +2152,23 @@
 		    "could not allocate parent dma tag\n");
 		return (ENOMEM);
 	}
+	/*
+	 * Set 4GB boundary for mbufs such that it workarounds DMA bug
+	 * of pre BCM5755 controllers.
+	 */
+	lowaddr = BUS_SPACE_MAXADDR;
+	if ((sc->bge_flags & BGE_FLAG_40BIT_BUG) != 0)
+		lowaddr = BGE_DMA_MAXADDR;
+	error = bus_dma_tag_create(bus_get_dma_tag(sc->bge_dev),
+	    1, BUS_SPACE_MAXSIZE_32BIT, lowaddr, BUS_SPACE_MAXADDR,
+	    NULL, NULL, BUS_SPACE_MAXSIZE_32BIT, 0, BUS_SPACE_MAXSIZE_32BIT,
+	    0, NULL, NULL, &sc->bge_cdata.bge_mbuf_tag);
+
+	if (error != 0) {
+		device_printf(sc->bge_dev,
+		    "could not allocate mbuf dma tag\n");
+		return (ENOMEM);
+	}
 
 	/*
 	 * Create tag for Tx mbufs.
@@ -2154,7 +2180,7 @@
 		txsegsz = MCLBYTES;
 		txmaxsegsz = MCLBYTES * BGE_NSEG_NEW;
 	}
-	error = bus_dma_tag_create(sc->bge_cdata.bge_parent_tag, 1,
+	error = bus_dma_tag_create(sc->bge_cdata.bge_mbuf_tag, 1,
 	    0, BUS_SPACE_MAXADDR, BUS_SPACE_MAXADDR, NULL, NULL,
 	    txmaxsegsz, BGE_NSEG_NEW, txsegsz, 0, NULL, NULL,
 	    &sc->bge_cdata.bge_tx_mtag);
@@ -2167,7 +2193,7 @@
 	/*
 	 * Create tag for Rx mbufs.
 	 */
-	error = bus_dma_tag_create(sc->bge_cdata.bge_parent_tag, 1, 0,
+	error = bus_dma_tag_create(sc->bge_cdata.bge_mbuf_tag, 1, 0,
 	    BUS_SPACE_MAXADDR, BUS_SPACE_MAXADDR, NULL, NULL, MCLBYTES, 1,
 	    MCLBYTES, 0, NULL, NULL, &sc->bge_cdata.bge_rx_mtag);
 
@@ -2607,17 +2633,6 @@
 	}
 
 	/*
-	 * All controllers that are not 5755 or higher have 4GB
-	 * boundary DMA bug.
-	 * Whenever an address crosses a multiple of the 4GB boundary
-	 * (including 4GB, 8Gb, 12Gb, etc.) and makes the transition
-	 * from 0xX_FFFF_FFFF to 0x(X+1)_0000_0000 an internal DMA
-	 * state machine will lockup and cause the device to hang.
-	 */
-	if (BGE_IS_5755_PLUS(sc) == 0)
-		sc->bge_flags |= BGE_FLAG_4G_BNDRY_BUG;
-
-	/*
 	 * We could possibly check for BCOM_DEVICEID_BCM5788 in bge_probe()
 	 * but I do not know the DEVICEID for the 5788M.
 	 */
--- if_bgereg.h.orig	2009-11-22 13:16:30.000000000 -0800
+++ if_bgereg.h	2009-11-23 11:53:56.000000000 -0800
@@ -2537,6 +2537,7 @@
  */
 struct bge_chain_data {
 	bus_dma_tag_t		bge_parent_tag;
+	bus_dma_tag_t		bge_mbuf_tag;
 	bus_dma_tag_t		bge_rx_std_ring_tag;
 	bus_dma_tag_t		bge_rx_jumbo_ring_tag;
 	bus_dma_tag_t		bge_rx_return_ring_tag;
@@ -2612,7 +2613,6 @@
 #define	BGE_FLAG_575X_PLUS	0x00008000
 #define	BGE_FLAG_5755_PLUS	0x00010000
 #define	BGE_FLAG_40BIT_BUG	0x00020000
-#define	BGE_FLAG_4G_BNDRY_BUG	0x00040000
 #define	BGE_FLAG_RX_ALIGNBUG	0x00100000
 #define	BGE_FLAG_NO_3LED	0x00200000
 #define	BGE_FLAG_ADC_BUG	0x00400000