tg3: 5701 DMA corruption fix
Subsystems (2)
PATCH 1/2 tg3: 5701 DMA corruption fix
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.
drivers/net/tg3.c +46 -2
@@ -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.
drivers/net/tg3.h +1 -0
@@ -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 0x00000008struct timer_list timer;u16 timer_counter;
David Miller
maintainerFrom: "Matt Carlson" <[email protected]>
Date: Wed, 16 Apr 2008 15:27:41 -0700
Quoted 9 lines
drivers/net/tg3.c +9
-0+while (pci_id->vendor !=0 ) { + bridge =pci_get_device (pci_id->vendor, + pci_id->device, + bridge); +if (!bridge) { + pci_id++; +continue ; + } + if (bridge->subordinate &&
drivers/net/tg3.c static int __devinit tg3_get_invariants(struct tg3 *tp) · line 11384
Quoted 10 lines
drivers/net/tg3.c +10
-0+ (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!
Quoted 17 lines · David Miller
From: "Matt Carlson" <[email protected]>
Date: Wed, 16 Apr 2008 15:27:41 -0700Quoted 14 lines
drivers/net/tg3.c +14
-0+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 ;
Quoted 8 lines
drivers/net/tg3.c static int __devinit tg3_get_invariants(struct tg3 *tp) · line 11384
Quoted 5 lines
drivers/net/tg3.c +5
-0+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.
Quoted 24 lines
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!
David Miller
maintainerFrom: "Michael Chan" <[email protected]>
Date: Fri, 18 Apr 2008 10:45:35 -0700
Quoted 3 lines
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!