[PATCH 1/2] tg3: 5701 DMA corruption fix

[PATCH 1/2] tg3: 5701 DMA corruption fix

[Matt Carlson]

Herbert Xu's commit fb93134dfc2a6e6fbedc7c270a31da03fce88db9, entitled
"[TCP]: Fix size calculation in sk_stream_alloc_pskb", has triggered a
bug in the 5701 where the 5701 DMA engine will corrupt outgoing
packets.  This problem only happens when the starting address of the
packet matches a certain range of offsets and only when the 5701 is
placed downstream of a particular Intel bridge.

This patch detects the problematic bridge and if present, readjusts the
starting address of the packet data to a dword aligned boundary.

Signed-off-by: Matt Carlson <mcarlson@broadcom.com>
Signed-off-by: Michael Chan <mchan@broadcom.com>

diff --git a/drivers/net/tg3.c b/drivers/net/tg3.c
index 96043c5..b6637f0 100644
--- a/drivers/net/tg3.c
+++ b/drivers/net/tg3.c
@@ -4135,11 +4135,21 @@ static int tigon3_dma_hwbug_workaround(struct tg3 *tp, struct sk_buff *skb,
 				       u32 last_plus_one, u32 *start,
 				       u32 base_flags, u32 mss)
 {
-	struct sk_buff *new_skb = skb_copy(skb, GFP_ATOMIC);
+	struct sk_buff *new_skb;
 	dma_addr_t new_addr = 0;
 	u32 entry = *start;
 	int i, ret = 0;
 
+	if (GET_ASIC_REV(tp->pci_chip_rev_id) != ASIC_REV_5701)
+		new_skb = skb_copy(skb, GFP_ATOMIC);
+	else {
+		int more_headroom = 4 - ((unsigned long)skb->data & 3);
+
+		new_skb = skb_copy_expand(skb,
+					  skb_headroom(skb) + more_headroom,
+					  skb_tailroom(skb), GFP_ATOMIC);
+	}
+
 	if (!new_skb) {
 		ret = -1;
 	} else {
@@ -4462,7 +4472,9 @@ static int tg3_start_xmit_dma_bug(struct sk_buff *skb, struct net_device *dev)
 
 	would_hit_hwbug = 0;
 
-	if (tg3_4g_overflow_test(mapping, len))
+	if (tp->tg3_flags3 & TG3_FLG3_5701_DMA_BUG)
+		would_hit_hwbug = 1;
+	else if (tg3_4g_overflow_test(mapping, len))
 		would_hit_hwbug = 1;
 
 	tg3_set_txd(tp, entry, mapping, len, base_flags,
@@ -11339,6 +11351,38 @@ static int __devinit tg3_get_invariants(struct tg3 *tp)
 		}
 	}
 
+	if ((GET_ASIC_REV(tp->pci_chip_rev_id) == ASIC_REV_5701)) {
+		static struct tg3_dev_id {
+			u32	vendor;
+			u32	device;
+		} bridge_chipsets[] = {
+			{ PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_PXH_0 },
+			{ PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_PXH_1 },
+			{ },
+		};
+		struct tg3_dev_id *pci_id = &bridge_chipsets[0];
+		struct pci_dev *bridge = NULL;
+
+		while (pci_id->vendor != 0) {
+			bridge = pci_get_device(pci_id->vendor,
+						pci_id->device,
+						bridge);
+			if (!bridge) {
+				pci_id++;
+				continue;
+			}
+			if (bridge->subordinate &&
+			    (bridge->subordinate->number <=
+			     tp->pdev->bus->number) &&
+			    (bridge->subordinate->subordinate >=
+			     tp->pdev->bus->number)) {
+				tp->tg3_flags3 |= TG3_FLG3_5701_DMA_BUG;
+				pci_dev_put(bridge);
+				break;
+			}
+		}
+	}
+
 	/* The EPB bridge inside 5714, 5715, and 5780 cannot support
 	 * DMA addresses > 40-bit. This bridge may have other additional
 	 * 57xx devices behind it in some 4-port NIC designs for example.
diff --git a/drivers/net/tg3.h b/drivers/net/tg3.h
index c1075a7..c688c3a 100644
--- a/drivers/net/tg3.h
+++ b/drivers/net/tg3.h
@@ -2476,6 +2476,7 @@ struct tg3 {
 #define TG3_FLG3_NO_NVRAM_ADDR_TRANS	0x00000001
 #define TG3_FLG3_ENABLE_APE		0x00000002
 #define TG3_FLG3_5761_5784_AX_FIXES	0x00000004
+#define TG3_FLG3_5701_DMA_BUG		0x00000008
 
 	struct timer_list		timer;
 	u16				timer_counter;

Re: [PATCH 1/2] tg3: 5701 DMA corruption fix

[David Miller]

From: "Matt Carlson" <mcarlson@broadcom.com>
Date: Wed, 16 Apr 2008 15:27:41 -0700

> +		while (pci_id->vendor != 0) {
> +			bridge = pci_get_device(pci_id->vendor,
> +						pci_id->device,
> +						bridge);
> +			if (!bridge) {
> +				pci_id++;
> +				continue;
> +			}
> +			if (bridge->subordinate &&
> +			    (bridge->subordinate->number <=
> +			     tp->pdev->bus->number) &&
> +			    (bridge->subordinate->subordinate >=
> +			     tp->pdev->bus->number)) {
> +				tp->tg3_flags3 |= TG3_FLG3_5701_DMA_BUG;
> +				pci_dev_put(bridge);
> +				break;
> +			}
> +		}
> +	}

This code block will leak bridge device objects when the table
comparison check fails.

Probably the best thing to do is seperate the boolean check
from the other operations:

	bool match = false;
 ...
	if (bridge->subordinate &&
	    (bridge->subordinate->number <=
	     tp->pdev->bus->number) &&
	    (bridge->subordinate->subordinate >=
	     tp->pdev->bus->number))
		match = true;

	pci_dev_put_bridge(bridge);

	if (match) {
		tp->tg3_flags |= TG3_FLG3_5701_DMA_BUG;
		pci_dev_put(bridge);
	}

Please fix this up, and combine the version bump into this patch.

Thank you!

Re: [PATCH 1/2] tg3: 5701 DMA corruption fix

[Michael Chan]

On Thu, 2008-04-17 at 23:26 -0700, David Miller wrote:
> From: "Matt Carlson" <mcarlson@broadcom.com>
> Date: Wed, 16 Apr 2008 15:27:41 -0700
> 
> > +		while (pci_id->vendor != 0) {
> > +			bridge = pci_get_device(pci_id->vendor,
> > +						pci_id->device,
> > +						bridge);
> > +			if (!bridge) {
> > +				pci_id++;
> > +				continue;
> > +			}
> > +			if (bridge->subordinate &&
> > +			    (bridge->subordinate->number <=
> > +			     tp->pdev->bus->number) &&
> > +			    (bridge->subordinate->subordinate >=
> > +			     tp->pdev->bus->number)) {
> > +				tp->tg3_flags3 |= TG3_FLG3_5701_DMA_BUG;
> > +				pci_dev_put(bridge);
> > +				break;
> > +			}
> > +		}
> > +	}
> 
> This code block will leak bridge device objects when the table
> comparison check fails.

Do you mean having undecremented reference count on the bridge when it
doesn't match?

I think it is ok, because when we call pci_get_device() next time, we'll
pass in the old bridge so that it can find the next one.  pci_get_device
() will automatically decrement the reference count on the old bridge.

> 
> Probably the best thing to do is seperate the boolean check
> from the other operations:
> 
> 	bool match = false;
>  ...
> 	if (bridge->subordinate &&
> 	    (bridge->subordinate->number <=
> 	     tp->pdev->bus->number) &&
> 	    (bridge->subordinate->subordinate >=
> 	     tp->pdev->bus->number))
> 		match = true;
> 
> 	pci_dev_put_bridge(bridge);
> 
> 	if (match) {
> 		tp->tg3_flags |= TG3_FLG3_5701_DMA_BUG;
> 		pci_dev_put(bridge);
> 	}
> 
> Please fix this up, and combine the version bump into this patch.
> 
> Thank you!
> 

Re: [PATCH 1/2] tg3: 5701 DMA corruption fix

[David Miller]

From: "Michael Chan" <mchan@broadcom.com>
Date: Fri, 18 Apr 2008 10:45:35 -0700

> I think it is ok, because when we call pci_get_device() next time, we'll
> pass in the old bridge so that it can find the next one.  pci_get_device
> () will automatically decrement the reference count on the old bridge.

You're absolutely correct.

I still would appreciate the version bump being combined with
the bug fix patch, could someone please do that?

Thanks!