RE: [Intel-wired-lan] [PATCH] e1000e: add option not to verify NVM checksum

[Loktionov, Aleksandr]

> -----Original Message-----
> From: Intel-wired-lan <intel-wired-lan-bounces@xxxxxxxxxx> On Behalf Of
> Jacek Kowalski
> Sent: Tuesday, March 18, 2025 9:47 PM
> To: Nguyen, Anthony L <anthony.l.nguyen@xxxxxxxxx>; Kitszel, Przemyslaw
> <przemyslaw.kitszel@xxxxxxxxx>; Andrew Lunn <andrew+netdev@xxxxxxx>;
> David S. Miller <davem@xxxxxxxxxxxxx>; Dumazet, Eric
> <edumazet@xxxxxxxxxx>; Jakub Kicinski <kuba@xxxxxxxxxx>; Paolo Abeni
> <pabeni@xxxxxxxxxx>
> Cc: intel-wired-lan@xxxxxxxxxxxxxxxx; netdev@xxxxxxxxxxxxxxx; linux-
> kernel@xxxxxxxxxxxxxxx
> Subject: [Intel-wired-lan] [PATCH] e1000e: add option not to verify NVM
> checksum
> 
> Many laptops and motherboards including I219-V network card have invalid
> NVM checksum. While in most instances checksum is fixed by e1000e module
> or by using bootutil, some setups are resistant to NVM modifications. This
> result in the network card being completely unusable.
> 
> It seems to be the case on Dell Latitude 5420 where UEFI firmware corrupts (in
> this module's sense) checksums on each boot. No set of BIOS options seems
> to help.
> 
> This commit adds e1000e module option called VerifyNVMChecksum
> (defaults to 1) that allows advanced users to skip checkum verification by
> setting it to 0.
> 
> Signed-off-by: Jacek Kowalski <Jacek@xxxxxxxxxxx>
> Cc: stable@xxxxxxxxxxxxxxx
Reviewed-by: Aleksandr Loktionov <aleksandr.loktionov@xxxxxxxxx>

> ---
>  drivers/net/ethernet/intel/e1000e/e1000.h  |  1 +
> drivers/net/ethernet/intel/e1000e/netdev.c | 22 ++++++++--------
> drivers/net/ethernet/intel/e1000e/param.c  | 30
> ++++++++++++++++++++++
>  3 files changed, 43 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/net/ethernet/intel/e1000e/e1000.h
> b/drivers/net/ethernet/intel/e1000e/e1000.h
> index ba9c19e6994c..61dcc88dd2ff 100644
> --- a/drivers/net/ethernet/intel/e1000e/e1000.h
> +++ b/drivers/net/ethernet/intel/e1000e/e1000.h
> @@ -461,6 +461,7 @@ s32 e1000e_get_base_timinca(struct e1000_adapter
> *adapter, u32 *timinca);
>  #define FLAG2_CHECK_RX_HWTSTAMP           BIT(13)
>  #define FLAG2_CHECK_SYSTIM_OVERFLOW       BIT(14)
>  #define FLAG2_ENABLE_S0IX_FLOWS           BIT(15)
> +#define FLAG2_VERIFY_NVM_CHECKSUM         BIT(16)
> 
>  #define E1000_RX_DESC_PS(R, i)	    \
>  	(&(((union e1000_rx_desc_packet_split *)((R).desc))[i])) diff --git
> a/drivers/net/ethernet/intel/e1000e/netdev.c
> b/drivers/net/ethernet/intel/e1000e/netdev.c
> index 286155efcedf..b99b22dcaba4 100644
> --- a/drivers/net/ethernet/intel/e1000e/netdev.c
> +++ b/drivers/net/ethernet/intel/e1000e/netdev.c
> @@ -7567,16 +7567,18 @@ static int e1000_probe(struct pci_dev *pdev,
> const struct pci_device_id *ent)
>  	 */
>  	adapter->hw.mac.ops.reset_hw(&adapter->hw);
> 
> -	/* systems with ASPM and others may see the checksum fail on the
> first
> -	 * attempt. Let's give it a few tries
> -	 */
> -	for (i = 0;; i++) {
> -		if (e1000_validate_nvm_checksum(&adapter->hw) >= 0)
> -			break;
> -		if (i == 2) {
> -			dev_err(&pdev->dev, "The NVM Checksum Is Not
> Valid\n");
> -			err = -EIO;
> -			goto err_eeprom;
> +	if (adapter->flags2 & FLAG2_VERIFY_NVM_CHECKSUM) {
> +		/* systems with ASPM and others may see the checksum fail
> on the first
> +		* attempt. Let's give it a few tries
> +		*/
> +		for (i = 0;; i++) {
> +			if (e1000_validate_nvm_checksum(&adapter->hw) >=
> 0)
> +				break;
> +			if (i == 2) {
> +				dev_err(&pdev->dev, "The NVM Checksum Is
> Not Valid\n");
> +				err = -EIO;
> +				goto err_eeprom;
> +			}
>  		}
>  	}
> 
> diff --git a/drivers/net/ethernet/intel/e1000e/param.c
> b/drivers/net/ethernet/intel/e1000e/param.c
> index 3132d8f2f207..8711eb10dd11 100644
> --- a/drivers/net/ethernet/intel/e1000e/param.c
> +++ b/drivers/net/ethernet/intel/e1000e/param.c
> @@ -127,6 +127,15 @@ E1000_PARAM(KumeranLockLoss, "Enable
> Kumeran lock loss workaround");  E1000_PARAM(WriteProtectNVM,
>  	    "Write-protect NVM [WARNING: disabling this can lead to corrupted
> NVM]");
> 
> +/* Verify NVM Checksum
> + *
> + * Valid Range: 0, 1
> + *
> + * Default Value: 1 (enabled)
> + */
> +E1000_PARAM(VerifyNVMChecksum,
> +	    "Verify NVM checksum [WARNING: disabling can cause invalid
> +behavior]");
> +
>  /* Enable CRC Stripping
>   *
>   * Valid Range: 0, 1
> @@ -524,4 +533,25 @@ void e1000e_check_options(struct e1000_adapter
> *adapter)
>  			}
>  		}
>  	}
> +	/* Verify NVM checksum */
> +	{
> +		static const struct e1000_option opt = {
> +			.type = enable_option,
> +			.name = "Verify NVM checksum",
> +			.err  = "defaulting to Enabled",
> +			.def  = OPTION_ENABLED
> +		};
> +
> +		if (num_VerifyNVMChecksum > bd) {
> +			unsigned int verify_nvm_checksum =
> +				VerifyNVMChecksum[bd];
> +			e1000_validate_option(&verify_nvm_checksum,
> &opt,
> +						adapter);
> +			if (verify_nvm_checksum)
> +				adapter->flags2 |=
> FLAG2_VERIFY_NVM_CHECKSUM;
> +		} else {
> +			if (opt.def)
> +				adapter->flags2 |=
> FLAG2_VERIFY_NVM_CHECKSUM;
> +		}
> +	}
>  }
> --
> 2.39.5