> -----Original Message-----
> From: Aaron Ma <[email protected]>
> Sent: Monday, September 7, 2026 1:52 PM
> To: Nguyen, Anthony L <[email protected]>; Kitszel,
> Przemyslaw <[email protected]>; Andrew Lunn
> <[email protected]>; David S. Miller <[email protected]>; Eric
> Dumazet <[email protected]>; Jakub Kicinski <[email protected]>; Paolo
> Abeni <[email protected]>; [email protected]; linux-
> [email protected]
> Cc: Henry Tieman <[email protected]>; moderated list:INTEL
> ETHERNET DRIVERS <[email protected]>
> Subject: [PATCH v2 1/2] ice: restore DDP state during PFR recovery
>
> The firmware package and switch recipes are shared by all PFs of an
> adapter. However, each PF rebuilds independently after system resume.
> The resulting package downloads and recipe updates can interleave,
> causing firmware timeouts and leaving interfaces unusable.
>
> PFR also clears package-derived firmware state on affected devices.
> The existing PFR path only rebuilds the driver's block tables, so VLAN
> recipe programming can fail after the reset.
>
> The failure is reported as:
>
> ice 0000:04:00.0: Update pkg failed: err -5
> ice 0000:04:00.0: package load failed, -12
> ice 0000:04:00.0: Rebuild failed, unload and reload driver
>
> This was observed on an Intel E810-XXV-2 adapter (PCI ID 8086:159b,
> revision 02) with NVM package 1.0.0.18 and ICE OS Default DDP package
> 1.3.43.0.
>
> Serialize rebuilds across PFs of the same adapter, while allowing each
> PFR to complete independently. Reload the DDP package from its cached
> copy and restore the default DVM recipes before rebuilding the
> remaining PF state.
> Abort recovery if either operation fails.
>
> In addition, stop the service task before tearing down resources in
> ice_remove() so that an in-flight rebuild completes and cannot
> dereference
> pf->adapter after ice_adapter_put().
>
> Fixes: 462acf6aca85 ("ice: Enable DDP package download")
> Reviewed-by: Przemek Kitszel <[email protected]>
> Signed-off-by: Aaron Ma <[email protected]>
> ---
> v1 -> v2:
> - Stop service task early in ice_remove() to prevent use-after-free on
> pf->adapter if rebuild is in flight.
>
> drivers/net/ethernet/intel/ice/ice_adapter.c | 2 ++
> drivers/net/ethernet/intel/ice/ice_adapter.h | 3 ++
> drivers/net/ethernet/intel/ice/ice_main.c | 35 ++++++++++++++----
> -
> .../net/ethernet/intel/ice/ice_vlan_mode.c | 2 +-
> .../net/ethernet/intel/ice/ice_vlan_mode.h | 1 +
> 5 files changed, 34 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/net/ethernet/intel/ice/ice_adapter.c
> b/drivers/net/ethernet/intel/ice/ice_adapter.c
> index 2dc3629d6d0f6..7de9223e971a7 100644
> --- a/drivers/net/ethernet/intel/ice/ice_adapter.c
> +++ b/drivers/net/ethernet/intel/ice/ice_adapter.c
> @@ -64,6 +64,7 @@ static struct ice_adapter *ice_adapter_new(struct
> pci_dev *pdev)
> spin_lock_init(&adapter->txq_ctx_lock);
> for (int i = 0; i < ARRAY_SIZE(adapter->cpi_phy_lock); i++)
> mutex_init(&adapter->cpi_phy_lock[i]);
> + mutex_init(&adapter->rebuild_lock);
> refcount_set(&adapter->refcount, 1);
>
> mutex_init(&adapter->ports.lock);
> @@ -77,6 +78,7 @@ static void ice_adapter_free(struct ice_adapter
> *adapter)
> WARN_ON(!list_empty(&adapter->ports.ports));
> for (int i = 0; i < ARRAY_SIZE(adapter->cpi_phy_lock); i++)
> mutex_destroy(&adapter->cpi_phy_lock[i]);
> + mutex_destroy(&adapter->rebuild_lock);
> mutex_destroy(&adapter->ports.lock);
>
> kfree(adapter);
> diff --git a/drivers/net/ethernet/intel/ice/ice_adapter.h
> b/drivers/net/ethernet/intel/ice/ice_adapter.h
> index 4f695f32da3d8..f9265de973efc 100644
> --- a/drivers/net/ethernet/intel/ice/ice_adapter.h
> +++ b/drivers/net/ethernet/intel/ice/ice_adapter.h
> @@ -37,6 +37,7 @@ struct ice_port_list {
> * @cpi_phy_lock: Per-PHY mutex serializing CPI REQ/ACK transactions.
> * Index 0 = PHY0, index 1 = PHY1. Used on E825C
> devices.
> * @ctrl_pf: Control PF of the adapter
> + * @rebuild_lock: serialize PFR recovery across PFs of the same
> adapter
> * @ports: Ports list
> * @index: 64-bit index cached for collision detection on 32bit
> systems
> */
> @@ -48,6 +49,8 @@ struct ice_adapter {
> spinlock_t txq_ctx_lock;
> /* Serialize CPI REQ/ACK transactions per PHY (E825C only) */
> struct mutex cpi_phy_lock[ICE_E825_MAX_PHYS];
> + /* Serialize PFR recovery touching shared FW global state */
> + struct mutex rebuild_lock;
>
> struct ice_pf *ctrl_pf;
> struct ice_port_list ports;
> diff --git a/drivers/net/ethernet/intel/ice/ice_main.c
> b/drivers/net/ethernet/intel/ice/ice_main.c
> index d88835482d3aa..abb6e850ec09f 100644
> --- a/drivers/net/ethernet/intel/ice/ice_main.c
> +++ b/drivers/net/ethernet/intel/ice/ice_main.c
> @@ -659,7 +659,9 @@ static void ice_do_reset(struct ice_pf *pf, enum
> ice_reset_req reset_type)
> */
> if (reset_type == ICE_RESET_PFR) {
> pf->pfr_count++;
> + mutex_lock(&pf->adapter->rebuild_lock);
> ice_rebuild(pf, reset_type);
> + mutex_unlock(&pf->adapter->rebuild_lock);
> clear_bit(ICE_PREPARED_FOR_RESET, pf->state);
> clear_bit(ICE_PFR_REQ, pf->state);
> wake_up(&pf->reset_wait_queue);
> @@ -704,7 +706,9 @@ static void ice_reset_subtask(struct ice_pf *pf)
> } else {
> /* done with reset. start rebuild */
> pf->hw.reset_ongoing = false;
> + mutex_lock(&pf->adapter->rebuild_lock);
> ice_rebuild(pf, reset_type);
> + mutex_unlock(&pf->adapter->rebuild_lock);
> /* clear bit to resume normal operations, but
> * ICE_NEEDS_RESTART bit is set in case rebuild
> failed
> */
> @@ -5374,6 +5378,8 @@ static void ice_remove(struct pci_dev *pdev)
> return;
> }
>
> + ice_service_task_stop(pf);
> +
> if (test_bit(ICE_FLAG_SRIOV_ENA, pf->flags)) {
> set_bit(ICE_VF_RESETS_DISABLED, pf->state);
> ice_free_vfs(pf);
> @@ -7674,14 +7680,27 @@ static void ice_rebuild(struct ice_pf *pf,
> enum ice_reset_req reset_type)
> goto err_init_ctrlq;
> }
>
> - /* if DDP was previously loaded successfully */
> - if (!ice_is_safe_mode(pf)) {
> - /* reload the SW DB of filter tables */
> - if (reset_type == ICE_RESET_PFR)
> - ice_fill_blk_tbls(hw);
> - else
> - /* Reload DDP Package after CORER/GLOBR reset */
> - ice_load_pkg(NULL, pf);
> + if (!ice_is_safe_mode(pf) && reset_type == ICE_RESET_PFR) {
> + enum ice_ddp_state state;
> +
> + state = ice_init_pkg(hw, hw->pkg_copy, hw->pkg_size);
> + ice_log_pkg_init(hw, state);
> + if (!ice_is_init_pkg_successful(state))
> + goto err_init_ctrlq;
> + } else if (!ice_is_safe_mode(pf)) {
> + /* Reload DDP Package after CORER/GLOBR reset */
> + ice_load_pkg(NULL, pf);
> + }
> +
> + /* PFR can lose DVM recipes after system suspend. */
> + if (!ice_is_safe_mode(pf) && reset_type == ICE_RESET_PFR &&
> + ice_is_dvm_ena(hw)) {
> + err = ice_dvm_update_dflt_recipes(hw);
> + if (err) {
> + dev_err(dev, "failed to restore default DVM
> recipes: %d\n",
> + err);
> + goto err_init_ctrlq;
> + }
> }
>
> err = ice_clear_pf_cfg(hw);
> diff --git a/drivers/net/ethernet/intel/ice/ice_vlan_mode.c
> b/drivers/net/ethernet/intel/ice/ice_vlan_mode.c
> index fb526cb847764..58fb191903a75 100644
> --- a/drivers/net/ethernet/intel/ice/ice_vlan_mode.c
> +++ b/drivers/net/ethernet/intel/ice/ice_vlan_mode.c
> @@ -240,7 +240,7 @@ static struct ice_update_recipe_lkup_idx_params
> ice_dvm_dflt_recipes[] = {
> * ice_dvm_update_dflt_recipes - update default switch recipes in DVM
> * @hw: hardware structure used to update the recipes
> */
> -static int ice_dvm_update_dflt_recipes(struct ice_hw *hw)
> +int ice_dvm_update_dflt_recipes(struct ice_hw *hw)
> {
> unsigned long i;
>
> diff --git a/drivers/net/ethernet/intel/ice/ice_vlan_mode.h
> b/drivers/net/ethernet/intel/ice/ice_vlan_mode.h
> index a0fb743d08e20..5dc705435f56a 100644
> --- a/drivers/net/ethernet/intel/ice/ice_vlan_mode.h
> +++ b/drivers/net/ethernet/intel/ice/ice_vlan_mode.h
> @@ -8,6 +8,7 @@ struct ice_hw;
>
> bool ice_is_dvm_ena(struct ice_hw *hw); int ice_set_vlan_mode(struct
> ice_hw *hw);
> +int ice_dvm_update_dflt_recipes(struct ice_hw *hw);
> void ice_post_pkg_dwnld_vlan_mode_cfg(struct ice_hw *hw);
>
> #endif /* _ICE_VLAN_MODE_H */
> --
> 2.43.0
Reviewed-by: Aleksandr Loktionov <[email protected]>
