[PATCH iwl-next v1] ixgbe: fix eeprom_id staleness and non-fatal EMPR reload failure

[PATCH iwl-next v1] ixgbe: fix eeprom_id staleness and non-fatal EMPR reload failure

[Aleksandr Loktionov]

Three related bugs around FW version reporting after EMPR reset on E610:

1. ixgbe_refresh_fw_version() silently discarded the error return from
   ixgbe_get_flash_data(), so a failed NVM re-read left adapter->eeprom_id
   with whatever stale data it held before the reset.  Propagate the error
   and set eeprom_id to "unknown" on failure so that ethtool -i and
   devlink dev info never show a version that no longer reflects reality.

2. ixgbe_devlink_reload_empr_finish() returned 0 without ever refreshing
   the FW version after the EMPR completed.  Add the refresh call, but
   treat it as best-effort: a failure to re-read flash does not mean the
   EMPR itself failed.  Log a netdev_warn() and return 0 so devlink
   reports the correct reload outcome.

3. ixgbe_reinit_locked() never refreshed the FW version for E610.
   Because E610 has no FW event that notifies peer PFs when an EMPR
   triggered by another PF's devlink reload completes, any PF that
   subsequently goes through reinit would keep stale data in hw->flash
   and adapter->eeprom_id.  Add the same best-effort refresh here,
   gated on ixgbe_mac_e610, with a netdev_warn() on failure.

Signed-off-by: Aleksandr Loktionov <aleksandr.loktionov@intel.com>
---
 .../ethernet/intel/ixgbe/devlink/devlink.c    | 25 ++++++++++++++++---
 drivers/net/ethernet/intel/ixgbe/ixgbe.h      |  2 +-
 .../net/ethernet/intel/ixgbe/ixgbe_ethtool.c  | 22 ++++++++++++++--
 drivers/net/ethernet/intel/ixgbe/ixgbe_main.c | 15 +++++++++++
 4 files changed, 57 insertions(+), 7 deletions(-)

diff --git a/drivers/net/ethernet/intel/ixgbe/devlink/devlink.c b/drivers/net/ethernet/intel/ixgbe/devlink/devlink.c
index b0404f3..4581605 100644
--- a/drivers/net/ethernet/intel/ixgbe/devlink/devlink.c
+++ b/drivers/net/ethernet/intel/ixgbe/devlink/devlink.c
@@ -322,8 +322,13 @@ static int ixgbe_devlink_info_get(struct devlink *devlink,
 	if (!ctx)
 		return -ENOMEM;
 
-	if (hw->mac.type == ixgbe_mac_e610)
-		ixgbe_refresh_fw_version(adapter);
+	if (hw->mac.type == ixgbe_mac_e610) {
+		err = ixgbe_refresh_fw_version(adapter);
+		if (err)
+			netdev_warn(adapter->netdev,
+				    "Failed to refresh FW version for devlink info: %d\n",
+				    err);
+	}
 
 	ixgbe_info_get_dsn(adapter, ctx);
 	err = devlink_info_serial_number_put(req, ctx->buf);
@@ -442,7 +447,9 @@ static int ixgbe_devlink_reload_empr_start(struct devlink *devlink,
  *
  * Wait for new NVM to be loaded during EMP reset.
  *
- * Return: -ETIME when timer is exceeded, 0 on success.
+ * Return: -ETIME when timer is exceeded, 0 on success. A failure to re-read
+ * the FW version after the reset completes is non-fatal and does not cause
+ * this function to return an error.
  */
 static int ixgbe_devlink_reload_empr_finish(struct devlink *devlink,
 					    enum devlink_reload_action action,
@@ -452,7 +459,7 @@ static int ixgbe_devlink_reload_empr_finish(struct devlink *devlink,
 {
 	struct ixgbe_adapter *adapter = devlink_priv(devlink);
 	struct ixgbe_hw *hw = &adapter->hw;
-	int i = 0;
+	int i = 0, err;
 	u32 fwsm;
 
 	do {
@@ -474,6 +481,16 @@ static int ixgbe_devlink_reload_empr_finish(struct devlink *devlink,
 	adapter->flags2 &= ~(IXGBE_FLAG2_API_MISMATCH |
 			     IXGBE_FLAG2_FW_ROLLBACK);
 
+	/* Re-reading the FW version is best-effort; the EMPR itself completed
+	 * successfully.  Log any failure so the user knows eeprom_id has been
+	 * reset to "unknown".
+	 */
+	err = ixgbe_refresh_fw_version(adapter);
+	if (err)
+		netdev_warn(adapter->netdev,
+			    "Failed to refresh FW version after EMPR: %d\n",
+			    err);
+
 	return 0;
 }
 
diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe.h b/drivers/net/ethernet/intel/ixgbe/ixgbe.h
index 59a1cee4..9b82175 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe.h
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe.h
@@ -974,7 +974,7 @@ int ixgbe_init_interrupt_scheme(struct ixgbe_adapter *adapter);
 bool ixgbe_wol_supported(struct ixgbe_adapter *adapter, u16 device_id,
 			 u16 subdevice_id);
 void ixgbe_set_fw_version_e610(struct ixgbe_adapter *adapter);
-void ixgbe_refresh_fw_version(struct ixgbe_adapter *adapter);
+int ixgbe_refresh_fw_version(struct ixgbe_adapter *adapter);
 #ifdef CONFIG_PCI_IOV
 void ixgbe_full_sync_mac_table(struct ixgbe_adapter *adapter);
 #endif
diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_ethtool.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_ethtool.c
index 56aabaa..40d593b 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_ethtool.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_ethtool.c
@@ -1155,12 +1155,30 @@ static int ixgbe_set_eeprom(struct net_device *netdev,
 	return ret_val;
 }
 
-void ixgbe_refresh_fw_version(struct ixgbe_adapter *adapter)
+/**
+ * ixgbe_refresh_fw_version - re-read flash data and update eeprom_id cache
+ * @adapter: board private structure
+ *
+ * Re-reads the NVM/flash and refreshes the cached adapter->eeprom_id string.
+ * On failure the cache is set to "unknown" so that ethtool -i never shows a
+ * stale version string after a failed reset.
+ *
+ * Return: 0 on success, negative error code on failure.
+ */
+int ixgbe_refresh_fw_version(struct ixgbe_adapter *adapter)
 {
 	struct ixgbe_hw *hw = &adapter->hw;
+	int err;
+
+	err = ixgbe_get_flash_data(hw);
+	if (err) {
+		strscpy(adapter->eeprom_id, "unknown",
+			sizeof(adapter->eeprom_id));
+		return err;
+	}
 
-	ixgbe_get_flash_data(hw);
 	ixgbe_set_fw_version_e610(adapter);
+	return 0;
 }
 
 static void ixgbe_get_drvinfo(struct net_device *netdev,
diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
index 0bc806a..d209cfc 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
@@ -6289,6 +6289,21 @@ void ixgbe_reinit_locked(struct ixgbe_adapter *adapter)
 	if (adapter->flags & IXGBE_FLAG_SRIOV_ENABLED)
 		msleep(2000);
 	ixgbe_up(adapter);
+
+	/* E610 has no FW event to notify peer PFs that an EMPR reset occurred.
+	 * Refresh the cached FW version here so that a PF completing reinit
+	 * after an EMPR triggered by another PF's devlink reload picks up the
+	 * new version in adapter->eeprom_id.
+	 */
+	if (adapter->hw.mac.type == ixgbe_mac_e610) {
+		int err = ixgbe_refresh_fw_version(adapter);
+
+		if (err)
+			netdev_warn(adapter->netdev,
+				    "Failed to refresh FW version after reset: %d\n",
+				    err);
+	}
+
 	clear_bit(__IXGBE_RESETTING, &adapter->state);
 }
 
-- 
2.52.0

Re: [PATCH iwl-next v1] ixgbe: fix eeprom_id staleness and non-fatal EMPR reload failure

[Simon Horman]

On Fri, Mar 20, 2026 at 06:14:54AM +0100, Aleksandr Loktionov wrote:
> Three related bugs around FW version reporting after EMPR reset on E610:
> 
> 1. ixgbe_refresh_fw_version() silently discarded the error return from
>    ixgbe_get_flash_data(), so a failed NVM re-read left adapter->eeprom_id
>    with whatever stale data it held before the reset.  Propagate the error
>    and set eeprom_id to "unknown" on failure so that ethtool -i and
>    devlink dev info never show a version that no longer reflects reality.
> 
> 2. ixgbe_devlink_reload_empr_finish() returned 0 without ever refreshing
>    the FW version after the EMPR completed.  Add the refresh call, but
>    treat it as best-effort: a failure to re-read flash does not mean the
>    EMPR itself failed.  Log a netdev_warn() and return 0 so devlink
>    reports the correct reload outcome.
> 
> 3. ixgbe_reinit_locked() never refreshed the FW version for E610.
>    Because E610 has no FW event that notifies peer PFs when an EMPR
>    triggered by another PF's devlink reload completes, any PF that
>    subsequently goes through reinit would keep stale data in hw->flash
>    and adapter->eeprom_id.  Add the same best-effort refresh here,
>    gated on ixgbe_mac_e610, with a netdev_warn() on failure.
> 
> Signed-off-by: Aleksandr Loktionov <aleksandr.loktionov@intel.com>

Reviewed-by: Simon Horman <horms@kernel.org>

...

> diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_ethtool.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_ethtool.c
> index 56aabaa..40d593b 100644
> --- a/drivers/net/ethernet/intel/ixgbe/ixgbe_ethtool.c
> +++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_ethtool.c
> @@ -1155,12 +1155,30 @@ static int ixgbe_set_eeprom(struct net_device *netdev,
>  	return ret_val;
>  }
>  
> -void ixgbe_refresh_fw_version(struct ixgbe_adapter *adapter)
> +/**
> + * ixgbe_refresh_fw_version - re-read flash data and update eeprom_id cache
> + * @adapter: board private structure
> + *
> + * Re-reads the NVM/flash and refreshes the cached adapter->eeprom_id string.
> + * On failure the cache is set to "unknown" so that ethtool -i never shows a
> + * stale version string after a failed reset.
> + *
> + * Return: 0 on success, negative error code on failure.
> + */
> +int ixgbe_refresh_fw_version(struct ixgbe_adapter *adapter)
>  {
>  	struct ixgbe_hw *hw = &adapter->hw;
> +	int err;
> +
> +	err = ixgbe_get_flash_data(hw);
> +	if (err) {
> +		strscpy(adapter->eeprom_id, "unknown",
> +			sizeof(adapter->eeprom_id));

nit: I think you can omit the size argument to strscpy
     because eeprom_id is an array.

> +		return err;
> +	}
>  
> -	ixgbe_get_flash_data(hw);
>  	ixgbe_set_fw_version_e610(adapter);
> +	return 0;
>  }
>  
>  static void ixgbe_get_drvinfo(struct net_device *netdev,

...