[PATCH] igb: restore EEPROM 16kB access limit

[PATCH] igb: restore EEPROM 16kB access limit

[Stefan Assmann]

The check limiting the EEPROM access up to 16kB was removed by
commit 4322e561a93ec7ee034b603a6c610e7be90d4e8a. Without this check
the kernel will try to checksum the EEPROM up to 2MB (observed with
a 8086:10c9 NIC) and fail.

igb 0000:03:00.0: 0 vfs allocated
igb 0000:03:00.0: The NVM Checksum Is Not Valid
ACPI: PCI interrupt for device 0000:03:00.0 disabled
igb: probe of 0000:03:00.0 failed with error -5

Reason for that being an overflow in u16 e1000_nvm_info->nvm
while doing "nvm->word_size = 1 << size;" with size == 21.
Putting the check back in place.

Signed-off-by: Stefan Assmann <sassmann@kpanic.de>
---
 drivers/net/igb/e1000_82575.c |    4 ++++
 1 files changed, 4 insertions(+), 0 deletions(-)

diff --git a/drivers/net/igb/e1000_82575.c b/drivers/net/igb/e1000_82575.c
index 6b256c2..5cfa37f 100644
--- a/drivers/net/igb/e1000_82575.c
+++ b/drivers/net/igb/e1000_82575.c
@@ -244,6 +244,10 @@ static s32 igb_get_invariants_82575(struct e1000_hw *hw)
 	 */
 	size += NVM_WORD_SIZE_BASE_SHIFT;
 
+	/* EEPROM access above 16k is unsupported */
+	if (size > 14)
+		size = 14;
+
 	nvm->word_size = 1 << size;
 	if (nvm->word_size == (1 << 15))
 		nvm->page_size = 128;
-- 
1.7.4


------------------------------------------------------------------------------
Xperia(TM) PLAY
It's a major breakthrough. An authentic gaming
smartphone on the nation's most reliable network.
And it wants your games.
http://p.sf.net/sfu/verizon-sfdev
_______________________________________________
E1000-devel mailing list
E1000-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/e1000-devel
To learn more about Intel&#174; Ethernet, visit http://communities.intel.com/community/wired

RE: [PATCH] igb: restore EEPROM 16kB access limit

[Wyborny, Carolyn]

>-----Original Message-----
>From: Stefan Assmann [mailto:sassmann@kpanic.de]
>Sent: Friday, April 08, 2011 6:35 AM
>To: netdev@vger.kernel.org
>Cc: e1000-devel@lists.sourceforge.net; Kirsher, Jeffrey T; Pieper,
>Jeffrey E; Wyborny, Carolyn; Ronciak, John
>Subject: [PATCH] igb: restore EEPROM 16kB access limit
>
>The check limiting the EEPROM access up to 16kB was removed by
>commit 4322e561a93ec7ee034b603a6c610e7be90d4e8a. Without this check
>the kernel will try to checksum the EEPROM up to 2MB (observed with
>a 8086:10c9 NIC) and fail.
>
>igb 0000:03:00.0: 0 vfs allocated
>igb 0000:03:00.0: The NVM Checksum Is Not Valid
>ACPI: PCI interrupt for device 0000:03:00.0 disabled
>igb: probe of 0000:03:00.0 failed with error -5
>
>Reason for that being an overflow in u16 e1000_nvm_info->nvm
>while doing "nvm->word_size = 1 << size;" with size == 21.
>Putting the check back in place.
>
>Signed-off-by: Stefan Assmann <sassmann@kpanic.de>
>---
> drivers/net/igb/e1000_82575.c |    4 ++++
> 1 files changed, 4 insertions(+), 0 deletions(-)
>
>diff --git a/drivers/net/igb/e1000_82575.c
>b/drivers/net/igb/e1000_82575.c
>index 6b256c2..5cfa37f 100644
>--- a/drivers/net/igb/e1000_82575.c
>+++ b/drivers/net/igb/e1000_82575.c
>@@ -244,6 +244,10 @@ static s32 igb_get_invariants_82575(struct e1000_hw
>*hw)
> 	 */
> 	size += NVM_WORD_SIZE_BASE_SHIFT;
>
>+	/* EEPROM access above 16k is unsupported */
>+	if (size > 14)
>+		size = 14;
>+
> 	nvm->word_size = 1 << size;
> 	if (nvm->word_size == (1 << 15))
> 		nvm->page_size = 128;
>--
>1.7.4
NACK

This doesn't apply against current upstream RC kernel.  There was more changed in that commit than just the removal of this.  There is a missing section of code that is needed, but not this.  This starts at line 251 in e1000_82575.c

--snip--
        /* NVM Function Pointers */
        nvm->ops.acquire = igb_acquire_nvm_82575;
        if (nvm->word_size < (1 << 15))
                nvm->ops.read = igb_read_nvm_eerd;
        else
                nvm->ops.read = igb_read_nvm_spi;
--snip--

Thanks,

Carolyn

Carolyn Wyborny
Linux Development
LAN Access Division
Intel Corporation

Re: [PATCH] igb: restore EEPROM 16kB access limit

[Stefan Assmann]

On 08.04.2011 18:40, Wyborny, Carolyn wrote:
> 
> 
>> -----Original Message-----
>> From: Stefan Assmann [mailto:sassmann@kpanic.de]
>> Sent: Friday, April 08, 2011 6:35 AM
>> To: netdev@vger.kernel.org
>> Cc: e1000-devel@lists.sourceforge.net; Kirsher, Jeffrey T; Pieper,
>> Jeffrey E; Wyborny, Carolyn; Ronciak, John
>> Subject: [PATCH] igb: restore EEPROM 16kB access limit
>>
>> The check limiting the EEPROM access up to 16kB was removed by
>> commit 4322e561a93ec7ee034b603a6c610e7be90d4e8a. Without this check
>> the kernel will try to checksum the EEPROM up to 2MB (observed with
>> a 8086:10c9 NIC) and fail.
>>
>> igb 0000:03:00.0: 0 vfs allocated
>> igb 0000:03:00.0: The NVM Checksum Is Not Valid
>> ACPI: PCI interrupt for device 0000:03:00.0 disabled
>> igb: probe of 0000:03:00.0 failed with error -5
>>
>> Reason for that being an overflow in u16 e1000_nvm_info->nvm
>> while doing "nvm->word_size = 1 << size;" with size == 21.
>> Putting the check back in place.
>>
>> Signed-off-by: Stefan Assmann <sassmann@kpanic.de>
>> ---
>> drivers/net/igb/e1000_82575.c |    4 ++++
>> 1 files changed, 4 insertions(+), 0 deletions(-)
>>
>> diff --git a/drivers/net/igb/e1000_82575.c
>> b/drivers/net/igb/e1000_82575.c
>> index 6b256c2..5cfa37f 100644
>> --- a/drivers/net/igb/e1000_82575.c
>> +++ b/drivers/net/igb/e1000_82575.c
>> @@ -244,6 +244,10 @@ static s32 igb_get_invariants_82575(struct e1000_hw
>> *hw)
>> 	 */
>> 	size += NVM_WORD_SIZE_BASE_SHIFT;
>>
>> +	/* EEPROM access above 16k is unsupported */
>> +	if (size > 14)
>> +		size = 14;
>> +
>> 	nvm->word_size = 1 << size;
>> 	if (nvm->word_size == (1 << 15))
>> 		nvm->page_size = 128;
>> --
>> 1.7.4
> NACK
> 
> This doesn't apply against current upstream RC kernel.  There was more changed in that commit than just the removal of this.  There is a missing section of code that is needed, but not this.  This starts at line 251 in e1000_82575.c

Carolyn,

the patch applies against latest net-next-2.6
(782d640afd15af7a1faf01cfe566ca4ac511319d).

How do you explain the behaviour observed in the patch description?

> 
> --snip--
>         /* NVM Function Pointers */
>         nvm->ops.acquire = igb_acquire_nvm_82575;
>         if (nvm->word_size < (1 << 15))
>                 nvm->ops.read = igb_read_nvm_eerd;
>         else
>                 nvm->ops.read = igb_read_nvm_spi;
> --snip--

Ok, so I assume some new NICs allow access beyond the 16k boundary.
In that case we should identify which NICs and treat them separately,
keeping the behaviour identical for the others.

  Stefan

RE: [PATCH] igb: restore EEPROM 16kB access limit

[Wyborny, Carolyn]

>-----Original Message-----
>From: Stefan Assmann [mailto:sassmann@kpanic.de]
>Sent: Friday, April 08, 2011 1:04 PM
>To: Wyborny, Carolyn
>Cc: netdev@vger.kernel.org; e1000-devel@lists.sourceforge.net; Kirsher,
>Jeffrey T; Pieper, Jeffrey E; Ronciak, John
>Subject: Re: [PATCH] igb: restore EEPROM 16kB access limit
>
>On 08.04.2011 18:40, Wyborny, Carolyn wrote:
>>
>>
>>> -----Original Message-----
>>> From: Stefan Assmann [mailto:sassmann@kpanic.de]
>>> Sent: Friday, April 08, 2011 6:35 AM
>>> To: netdev@vger.kernel.org
>>> Cc: e1000-devel@lists.sourceforge.net; Kirsher, Jeffrey T; Pieper,
>>> Jeffrey E; Wyborny, Carolyn; Ronciak, John
>>> Subject: [PATCH] igb: restore EEPROM 16kB access limit
>>>
>>> The check limiting the EEPROM access up to 16kB was removed by
>>> commit 4322e561a93ec7ee034b603a6c610e7be90d4e8a. Without this check
>>> the kernel will try to checksum the EEPROM up to 2MB (observed with
>>> a 8086:10c9 NIC) and fail.
>>>
>>> igb 0000:03:00.0: 0 vfs allocated
>>> igb 0000:03:00.0: The NVM Checksum Is Not Valid
>>> ACPI: PCI interrupt for device 0000:03:00.0 disabled
>>> igb: probe of 0000:03:00.0 failed with error -5
>>>
>>> Reason for that being an overflow in u16 e1000_nvm_info->nvm
>>> while doing "nvm->word_size = 1 << size;" with size == 21.
>>> Putting the check back in place.
>>>
>>> Signed-off-by: Stefan Assmann <sassmann@kpanic.de>
>>> ---
>>> drivers/net/igb/e1000_82575.c |    4 ++++
>>> 1 files changed, 4 insertions(+), 0 deletions(-)
>>>
>>> diff --git a/drivers/net/igb/e1000_82575.c
>>> b/drivers/net/igb/e1000_82575.c
>>> index 6b256c2..5cfa37f 100644
>>> --- a/drivers/net/igb/e1000_82575.c
>>> +++ b/drivers/net/igb/e1000_82575.c
>>> @@ -244,6 +244,10 @@ static s32 igb_get_invariants_82575(struct
>e1000_hw
>>> *hw)
>>> 	 */
>>> 	size += NVM_WORD_SIZE_BASE_SHIFT;
>>>
>>> +	/* EEPROM access above 16k is unsupported */
>>> +	if (size > 14)
>>> +		size = 14;
>>> +
>>> 	nvm->word_size = 1 << size;
>>> 	if (nvm->word_size == (1 << 15))
>>> 		nvm->page_size = 128;
>>> --
>>> 1.7.4
>> NACK
>>
>> This doesn't apply against current upstream RC kernel.  There was more
>changed in that commit than just the removal of this.  There is a
>missing section of code that is needed, but not this.  This starts at
>line 251 in e1000_82575.c
>
>Carolyn,
>
>the patch applies against latest net-next-2.6
>(782d640afd15af7a1faf01cfe566ca4ac511319d).
>
>How do you explain the behaviour observed in the patch description?
>
>>
>> --snip--
>>         /* NVM Function Pointers */
>>         nvm->ops.acquire = igb_acquire_nvm_82575;
>>         if (nvm->word_size < (1 << 15))
>>                 nvm->ops.read = igb_read_nvm_eerd;
>>         else
>>                 nvm->ops.read = igb_read_nvm_spi;
>> --snip--
>
>Ok, so I assume some new NICs allow access beyond the 16k boundary.
>In that case we should identify which NICs and treat them separately,
>keeping the behaviour identical for the others.
>
>  Stefan

Yes, there's more code changed than just the removal of what you're trying to add back.  The snip is the replacement but those function need to exist as well.  I believe that the commit referenced did not completely apply and you're missing some critical code.  The problem you are seeing should not occur with full patch.

The version of e1000_82575.c in 2.6.39-rc2 has all the changes needed for this to work correctly.

Carolyn

Re: [PATCH] igb: restore EEPROM 16kB access limit

[Andy Gospodarek]

On Fri, Apr 08, 2011 at 01:10:30PM -0700, Wyborny, Carolyn wrote:
[...]
> 
> Yes, there's more code changed than just the removal of what you're trying to add back.  The snip is the replacement but those function need to exist as well.  I believe that the commit referenced did not completely apply and you're missing some critical code.  The problem you are seeing should not occur with full patch.
> 
> The version of e1000_82575.c in 2.6.39-rc2 has all the changes needed for this to work correctly.
> 

I'm still seeing failures with today's net-next-2.6 ('git describe'
shows v2.6.39-rc1-1283-g64cad2a), so it would be really nice to get this
fixed.  I would rather not have to carry a patch like the one Stefan
posted or one like this crazy one I hacked up to try all sizes until
valid NVRAM is found.

It applies cleanly net-next-2.6, net-2.6, and linux-2.6 as all exhibit
the exact same problem.

diff --git a/drivers/net/igb/e1000_82575.c b/drivers/net/igb/e1000_82575.c
index 0cd41c4..f8677f2 100644
--- a/drivers/net/igb/e1000_82575.c
+++ b/drivers/net/igb/e1000_82575.c
@@ -243,7 +243,7 @@ static s32 igb_get_invariants_82575(struct e1000_hw *hw)
 	 * for setting word_size.
 	 */
 	size += NVM_WORD_SIZE_BASE_SHIFT;
-
+err_eeprom:
 	nvm->word_size = 1 << size;
 	if (nvm->word_size == (1 << 15))
 		nvm->page_size = 128;
@@ -271,6 +271,17 @@ static s32 igb_get_invariants_82575(struct e1000_hw *hw)
 	}
 	nvm->ops.write = igb_write_nvm_spi;
 
+        /* make sure the NVM is good */
+        if (hw->nvm.ops.validate(hw) < 0) {
+		if (size > 14)  {
+			size--;
+			printk(KERN_ERR "igb: The NVM size is not valid, trying %d\n", 1<<size);
+			goto err_eeprom;
+		}
+		printk(KERN_ERR "The NVM Checksum Is Not Valid\n");
+		return -E1000_ERR_MAC_INIT;
+        }
+
 	/* if part supports SR-IOV then initialize mailbox parameters */
 	switch (mac->type) {
 	case e1000_82576:
diff --git a/drivers/net/igb/igb_main.c b/drivers/net/igb/igb_main.c
index cdfd572..8e23ca2 100644
--- a/drivers/net/igb/igb_main.c
+++ b/drivers/net/igb/igb_main.c
@@ -1940,13 +1940,6 @@ static int __devinit igb_probe(struct pci_dev *pdev,
 	 * known good starting state */
 	hw->mac.ops.reset_hw(hw);
 
-	/* make sure the NVM is good */
-	if (hw->nvm.ops.validate(hw) < 0) {
-		dev_err(&pdev->dev, "The NVM Checksum Is Not Valid\n");
-		err = -EIO;
-		goto err_eeprom;
-	}
-
 	/* copy the MAC address out of the NVM */
 	if (hw->mac.ops.read_mac_addr(hw))
 		dev_err(&pdev->dev, "NVM Read Error\n");



------------------------------------------------------------------------------
WhatsUp Gold - Download Free Network Management Software
The most intuitive, comprehensive, and cost-effective network 
management toolset available today.  Delivers lowest initial 
acquisition cost and overall TCO of any competing solution.
http://p.sf.net/sfu/whatsupgold-sd
_______________________________________________
E1000-devel mailing list
E1000-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/e1000-devel
To learn more about Intel&#174; Ethernet, visit http://communities.intel.com/community/wired

Re: [PATCH] igb: restore EEPROM 16kB access limit

[Stefan Assmann]

New patch against net-next-2.6.

  Stefan

>From 67ce9e09e10f05054a26560306aa484ae3acc03f Mon Sep 17 00:00:00 2001
From: Stefan Assmann <sassmann@kpanic.de>
Date: Mon, 18 Apr 2011 15:22:19 +0200
Subject: [PATCH] igb: default to 32kB for EEPROMs reporting invalid size

The check that gracefully handled invalid EEPROM sizes was removed by
commit 4322e561a93ec7ee034b603a6c610e7be90d4e8a. Without this check the
EEPROM validation fails if the size is invalid and the NIC is not usable
by the OS. Observed with a 8086:10c9 NIC.

igb 0000:03:00.0: 0 vfs allocated
igb 0000:03:00.0: The NVM Checksum Is Not Valid
ACPI: PCI interrupt for device 0000:03:00.0 disabled
igb: probe of 0000:03:00.0 failed with error -5

Re-introducing the check with an additional dev_err() to report the problem.

Signed-off-by: Stefan Assmann <sassmann@kpanic.de>
---
 drivers/net/igb/e1000_82575.c |   10 ++++++++++
 1 files changed, 10 insertions(+), 0 deletions(-)

diff --git a/drivers/net/igb/e1000_82575.c b/drivers/net/igb/e1000_82575.c
index 0cd41c4..f3bdf29 100644
--- a/drivers/net/igb/e1000_82575.c
+++ b/drivers/net/igb/e1000_82575.c
@@ -31,9 +31,11 @@

 #include <linux/types.h>
 #include <linux/if_ether.h>
+#include <linux/pci.h>

 #include "e1000_mac.h"
 #include "e1000_82575.h"
+#include "igb.h"

 static s32  igb_get_invariants_82575(struct e1000_hw *);
 static s32  igb_acquire_phy_82575(struct e1000_hw *);
@@ -113,6 +115,7 @@ static s32 igb_get_invariants_82575(struct e1000_hw *hw)
 	struct e1000_nvm_info *nvm = &hw->nvm;
 	struct e1000_mac_info *mac = &hw->mac;
 	struct e1000_dev_spec_82575 * dev_spec = &hw->dev_spec._82575;
+	struct igb_adapter *adapter = hw->back;
 	u32 eecd;
 	s32 ret_val;
 	u16 size;
@@ -244,6 +247,13 @@ static s32 igb_get_invariants_82575(struct e1000_hw *hw)
 	 */
 	size += NVM_WORD_SIZE_BASE_SHIFT;

+	/* gracefully handle NICs reporting an invalid EEPROM size */
+	if (size > 15) {
+		dev_err(&adapter->pdev->dev,
+		        "NVM size is not valid, defaulting to 32kB\n");
+		size = 15;
+	}
+
 	nvm->word_size = 1 << size;
 	if (nvm->word_size == (1 << 15))
 		nvm->page_size = 128;
-- 
1.7.4

RE: [PATCH] igb: restore EEPROM 16kB access limit

[Wyborny, Carolyn]

>-----Original Message-----
>From: Stefan Assmann [mailto:sassmann@kpanic.de]
>Sent: Tuesday, April 26, 2011 8:07 AM
>To: Wyborny, Carolyn
>Cc: netdev@vger.kernel.org; e1000-devel@lists.sourceforge.net; Kirsher,
>Jeffrey T; Pieper, Jeffrey E; Ronciak, John; Andy Gospodarek
>Subject: Re: [PATCH] igb: restore EEPROM 16kB access limit
>
>New patch against net-next-2.6.
>
>  Stefan
>
>From 67ce9e09e10f05054a26560306aa484ae3acc03f Mon Sep 17 00:00:00 2001
>From: Stefan Assmann <sassmann@kpanic.de>
>Date: Mon, 18 Apr 2011 15:22:19 +0200
>Subject: [PATCH] igb: default to 32kB for EEPROMs reporting invalid size
>
>The check that gracefully handled invalid EEPROM sizes was removed by
>commit 4322e561a93ec7ee034b603a6c610e7be90d4e8a. Without this check the
>EEPROM validation fails if the size is invalid and the NIC is not usable
>by the OS. Observed with a 8086:10c9 NIC.
>
>igb 0000:03:00.0: 0 vfs allocated
>igb 0000:03:00.0: The NVM Checksum Is Not Valid
>ACPI: PCI interrupt for device 0000:03:00.0 disabled
>igb: probe of 0000:03:00.0 failed with error -5
>
>Re-introducing the check with an additional dev_err() to report the
>problem.
>
>Signed-off-by: Stefan Assmann <sassmann@kpanic.de>
>---
> drivers/net/igb/e1000_82575.c |   10 ++++++++++
> 1 files changed, 10 insertions(+), 0 deletions(-)
>
>diff --git a/drivers/net/igb/e1000_82575.c
>b/drivers/net/igb/e1000_82575.c
>index 0cd41c4..f3bdf29 100644
>--- a/drivers/net/igb/e1000_82575.c
>+++ b/drivers/net/igb/e1000_82575.c
>@@ -31,9 +31,11 @@
>
> #include <linux/types.h>
> #include <linux/if_ether.h>
>+#include <linux/pci.h>
>
> #include "e1000_mac.h"
> #include "e1000_82575.h"
>+#include "igb.h"
>
> static s32  igb_get_invariants_82575(struct e1000_hw *);
> static s32  igb_acquire_phy_82575(struct e1000_hw *);
>@@ -113,6 +115,7 @@ static s32 igb_get_invariants_82575(struct e1000_hw
>*hw)
> 	struct e1000_nvm_info *nvm = &hw->nvm;
> 	struct e1000_mac_info *mac = &hw->mac;
> 	struct e1000_dev_spec_82575 * dev_spec = &hw->dev_spec._82575;
>+	struct igb_adapter *adapter = hw->back;
> 	u32 eecd;
> 	s32 ret_val;
> 	u16 size;
>@@ -244,6 +247,13 @@ static s32 igb_get_invariants_82575(struct e1000_hw
>*hw)
> 	 */
> 	size += NVM_WORD_SIZE_BASE_SHIFT;
>
>+	/* gracefully handle NICs reporting an invalid EEPROM size */
>+	if (size > 15) {
>+		dev_err(&adapter->pdev->dev,
>+		        "NVM size is not valid, defaulting to 32kB\n");
>+		size = 15;
>+	}
>+
> 	nvm->word_size = 1 << size;
> 	if (nvm->word_size == (1 << 15))
> 		nvm->page_size = 128;
>--
>1.7.4

I have already submitted a patch to fix this in our internal process, but our maintainer has been ill.  It should be out shortly.  I thought you agreed with Alex to let us submit the patch as this code in our Shared Code and we need to retain the copyright.

I will get with Jeff to get it out as soon as possible.

Thanks,

Carolyn

Carolyn Wyborny
Linux Development
LAN Access Division
Intel Corporation

RE: [PATCH] igb: restore EEPROM 16kB access limit

[Wyborny, Carolyn]

>-----Original Message-----
>From: Andy Gospodarek [mailto:andy@greyhouse.net]
>Sent: Tuesday, April 26, 2011 8:07 AM
>To: Wyborny, Carolyn
>Cc: Stefan Assmann; netdev@vger.kernel.org; e1000-
>devel@lists.sourceforge.net; Kirsher, Jeffrey T; Pieper, Jeffrey E;
>Ronciak, John
>Subject: Re: [PATCH] igb: restore EEPROM 16kB access limit
>
>On Fri, Apr 08, 2011 at 01:10:30PM -0700, Wyborny, Carolyn wrote:
>[...]
>>
>> Yes, there's more code changed than just the removal of what you're
>trying to add back.  The snip is the replacement but those function need
>to exist as well.  I believe that the commit referenced did not
>completely apply and you're missing some critical code.  The problem you
>are seeing should not occur with full patch.
>>
>> The version of e1000_82575.c in 2.6.39-rc2 has all the changes needed
>for this to work correctly.
>>
>
>I'm still seeing failures with today's net-next-2.6 ('git describe'
>shows v2.6.39-rc1-1283-g64cad2a), so it would be really nice to get this
>fixed.  I would rather not have to carry a patch like the one Stefan
>posted or one like this crazy one I hacked up to try all sizes until
>valid NVRAM is found.
>
>It applies cleanly net-next-2.6, net-2.6, and linux-2.6 as all exhibit
>the exact same problem.
>
>diff --git a/drivers/net/igb/e1000_82575.c
>b/drivers/net/igb/e1000_82575.c
>index 0cd41c4..f8677f2 100644
>--- a/drivers/net/igb/e1000_82575.c
>+++ b/drivers/net/igb/e1000_82575.c
>@@ -243,7 +243,7 @@ static s32 igb_get_invariants_82575(struct e1000_hw
>*hw)
> 	 * for setting word_size.
> 	 */
> 	size += NVM_WORD_SIZE_BASE_SHIFT;
>-
>+err_eeprom:
> 	nvm->word_size = 1 << size;
> 	if (nvm->word_size == (1 << 15))
> 		nvm->page_size = 128;
>@@ -271,6 +271,17 @@ static s32 igb_get_invariants_82575(struct e1000_hw
>*hw)
> 	}
> 	nvm->ops.write = igb_write_nvm_spi;
>
>+        /* make sure the NVM is good */
>+        if (hw->nvm.ops.validate(hw) < 0) {
>+		if (size > 14)  {
>+			size--;
>+			printk(KERN_ERR "igb: The NVM size is not valid,
>trying %d\n", 1<<size);
>+			goto err_eeprom;
>+		}
>+		printk(KERN_ERR "The NVM Checksum Is Not Valid\n");
>+		return -E1000_ERR_MAC_INIT;
>+        }
>+
> 	/* if part supports SR-IOV then initialize mailbox parameters */
> 	switch (mac->type) {
> 	case e1000_82576:
>diff --git a/drivers/net/igb/igb_main.c b/drivers/net/igb/igb_main.c
>index cdfd572..8e23ca2 100644
>--- a/drivers/net/igb/igb_main.c
>+++ b/drivers/net/igb/igb_main.c
>@@ -1940,13 +1940,6 @@ static int __devinit igb_probe(struct pci_dev
>*pdev,
> 	 * known good starting state */
> 	hw->mac.ops.reset_hw(hw);
>
>-	/* make sure the NVM is good */
>-	if (hw->nvm.ops.validate(hw) < 0) {
>-		dev_err(&pdev->dev, "The NVM Checksum Is Not Valid\n");
>-		err = -EIO;
>-		goto err_eeprom;
>-	}
>-
> 	/* copy the MAC address out of the NVM */
> 	if (hw->mac.ops.read_mac_addr(hw))
> 		dev_err(&pdev->dev, "NVM Read Error\n");
>
Part of the problem you are seeing is an apparently widespread EEPROM problem where the size word in the EEPROM is invalid.  Since we didn't really check it before it didn't cause a problem.  I have a patch coming that addresses this by messaging the user that the size is invalid but setting it to a default and continuing.  

Thanks,

Carolyn

Carolyn Wyborny
Linux Development
LAN Access Division
Intel Corporation

Re: [PATCH] igb: restore EEPROM 16kB access limit

[Andy Gospodarek]

On Tue, Apr 26, 2011 at 08:12:20AM -0700, Wyborny, Carolyn wrote:
[...]
> Part of the problem you are seeing is an apparently widespread EEPROM problem where the size word in the EEPROM is invalid.  Since we didn't really check it before it didn't cause a problem.  I have a patch coming that addresses this by messaging the user that the size is invalid but setting it to a default and continuing.  
> 

It wasn't really a problem for me until the commit Stefan mentioned
4322e561a93ec7ee034b603a6c610e7be90d4e8a was applied.

I'm glad you are planning a fix for it, but I hope it can be out soon
and not held up for too long by other patches planned for the next
update.


------------------------------------------------------------------------------
WhatsUp Gold - Download Free Network Management Software
The most intuitive, comprehensive, and cost-effective network 
management toolset available today.  Delivers lowest initial 
acquisition cost and overall TCO of any competing solution.
http://p.sf.net/sfu/whatsupgold-sd
_______________________________________________
E1000-devel mailing list
E1000-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/e1000-devel
To learn more about Intel® Ethernet, visit http://communities.intel.com/community/wired

RE: [PATCH] igb: restore EEPROM 16kB access limit

[Wyborny, Carolyn]

>-----Original Message-----
>From: Andy Gospodarek [mailto:andy@greyhouse.net]
>Sent: Wednesday, April 27, 2011 7:16 AM
>To: Wyborny, Carolyn
>Cc: Andy Gospodarek; Stefan Assmann; netdev@vger.kernel.org; e1000-
>devel@lists.sourceforge.net; Kirsher, Jeffrey T; Pieper, Jeffrey E;
>Ronciak, John
>Subject: Re: [PATCH] igb: restore EEPROM 16kB access limit
>
>On Tue, Apr 26, 2011 at 08:12:20AM -0700, Wyborny, Carolyn wrote:
>[...]
>> Part of the problem you are seeing is an apparently widespread EEPROM
>problem where the size word in the EEPROM is invalid.  Since we didn't
>really check it before it didn't cause a problem.  I have a patch coming
>that addresses this by messaging the user that the size is invalid but
>setting it to a default and continuing.
>>
>
>It wasn't really a problem for me until the commit Stefan mentioned
>4322e561a93ec7ee034b603a6c610e7be90d4e8a was applied.
>
>I'm glad you are planning a fix for it, but I hope it can be out soon
>and not held up for too long by other patches planned for the next
>update.

Yes, the problem wasn't there before because of a bug in the code.  The bad EEPROM's have apparently been out there a while and are now being exposed, now that the code is fixed.  We didn't see one in our test of the fix originally or know there were out there until the reports starting coming in.

I'm pushing the fix through as soon as possible.  Its in test now.  I apologize for the delay.

Thanks,

Carolyn

Carolyn Wyborny
Linux Development
LAN Access Division
Intel Corporation