* [PATCH 2/5] ice: retain the adapter throughout hardware initialization and teardown
2026-10-19 10:51 [PATCH 0/5] ice: fix reset and suspend paths Aaron Ma
2026-10-19 10:51 ` [PATCH 1/5] ice: check the full PF reset polling limit Aaron Ma
@ 2026-10-19 10:51 ` Aaron Ma
2026-10-19 11:29 ` Temerkhanov, Sergey
2026-10-19 10:51 ` [PATCH 3/5] ice: scope global configuration lock to the adapter Aaron Ma
` (2 subsequent siblings)
4 siblings, 1 reply; 9+ messages in thread
From: Aaron Ma @ 2026-10-19 10:51 UTC (permalink / raw)
To: Tony Nguyen, Przemek Kitszel, Andrew Lunn, David S. Miller,
Eric Dumazet, Jakub Kicinski, Paolo Abeni, Alexander Lobakin,
Richard Cochran, netdev, linux-kernel
Cc: Dan Nowlin, Brett Creeley, Jacob Keller,
moderated list:INTEL ETHERNET DRIVERS
Adapter-scoped synchronization must be available to Admin Queue users
from initial queue creation through the last teardown command. Probe
currently obtains its adapter reference only after hardware and package
initialization, and error cleanup and removal drop it before hardware
teardown.
Acquire the reference before either normal hardware initialization or
firmware recovery-mode initialization. Release it after hardware and
device cleanup, balancing recovery-mode probe failures and removal too.
This establishes the lifetime prerequisite for moving package ownership
exclusion from a module-global mutex to the shared adapter.
Signed-off-by: Aaron Ma <aaron.ma@canonical.com>
---
drivers/net/ethernet/intel/ice/ice_main.c | 31 +++++++++++++----------
1 file changed, 17 insertions(+), 14 deletions(-)
diff --git a/drivers/net/ethernet/intel/ice/ice_main.c b/drivers/net/ethernet/intel/ice/ice_main.c
index 164428ba116a0..ddebb28b2454f 100644
--- a/drivers/net/ethernet/intel/ice/ice_main.c
+++ b/drivers/net/ethernet/intel/ice/ice_main.c
@@ -5242,27 +5242,29 @@ ice_probe(struct pci_dev *pdev, const struct pci_device_id __always_unused *ent)
hw->debug_mask = debug;
#endif
- if (ice_is_recovery_mode(hw))
- return ice_probe_recovery_mode(pf);
+ adapter = ice_adapter_get(pdev);
+ if (IS_ERR(adapter))
+ return PTR_ERR(adapter);
+ pf->adapter = adapter;
+
+ if (ice_is_recovery_mode(hw)) {
+ err = ice_probe_recovery_mode(pf);
+ if (err)
+ ice_adapter_put(pdev);
+ return err;
+ }
err = ice_init_hw(hw);
if (err) {
dev_err(dev, "ice_init_hw failed: %d\n", err);
- return err;
+ goto unroll_adapter;
}
ice_init_dev_hw(pf);
- adapter = ice_adapter_get(pdev);
- if (IS_ERR(adapter)) {
- err = PTR_ERR(adapter);
- goto unroll_hw_init;
- }
- pf->adapter = adapter;
-
err = ice_init_dev(pf);
if (err)
- goto unroll_adapter;
+ goto unroll_hw_init;
err = ice_init(pf);
if (err)
@@ -5287,12 +5289,12 @@ ice_probe(struct pci_dev *pdev, const struct pci_device_id __always_unused *ent)
ice_deinit(pf);
unroll_dev_init:
need_dev_deinit = true;
-unroll_adapter:
- ice_adapter_put(pdev);
unroll_hw_init:
ice_deinit_hw(hw);
if (need_dev_deinit)
ice_deinit_dev(pf);
+unroll_adapter:
+ ice_adapter_put(pdev);
return err;
}
@@ -5377,6 +5379,7 @@ static void ice_remove(struct pci_dev *pdev)
scoped_guard(devl, priv_to_devlink(pf)) {
ice_deinit_devlink(pf);
}
+ ice_adapter_put(pdev);
return;
}
@@ -5403,11 +5406,11 @@ static void ice_remove(struct pci_dev *pdev)
ice_setup_mc_magic_wake(pf);
ice_set_wake(pf);
- ice_adapter_put(pdev);
ice_deinit_hw(&pf->hw);
ice_deinit_dev(pf);
ice_aq_cancel_waiting_tasks(pf);
+ ice_adapter_put(pdev);
set_bit(ICE_DOWN, pf->state);
}
--
2.53.0
^ permalink raw reply [flat|nested] 9+ messages in thread* [PATCH 3/5] ice: scope global configuration lock to the adapter
2026-10-19 10:51 [PATCH 0/5] ice: fix reset and suspend paths Aaron Ma
2026-10-19 10:51 ` [PATCH 1/5] ice: check the full PF reset polling limit Aaron Ma
2026-10-19 10:51 ` [PATCH 2/5] ice: retain the adapter throughout hardware initialization and teardown Aaron Ma
@ 2026-10-19 10:51 ` Aaron Ma
2026-10-19 10:51 ` [PATCH 4/5] ice: clear DVM boost table before scanning package hints Aaron Ma
2026-10-19 10:51 ` [PATCH 5/5] ice: quiesce PTP periodic work before system suspend Aaron Ma
4 siblings, 0 replies; 9+ messages in thread
From: Aaron Ma @ 2026-10-19 10:51 UTC (permalink / raw)
To: Tony Nguyen, Przemek Kitszel, Andrew Lunn, David S. Miller,
Eric Dumazet, Jakub Kicinski, Paolo Abeni, Alexander Lobakin,
Richard Cochran, netdev, linux-kernel
Cc: Dan Nowlin, Brett Creeley, Jacob Keller,
moderated list:INTEL ETHERNET DRIVERS
ice_global_cfg_lock_sw is a module-global mutex, so a package download
on one E810 adapter blocks Admin Queue commands on unrelated adapters in
the same system. In addition, the mutex is acquired only after firmware
grants the Global Configuration Lock and dropped before firmware
releases it, while the opcode exemption list in ice_aq_send_cmd() does
not check whether the caller is the lock owner. Non-owning PFs on the
same adapter can therefore submit exempted Admin Queue commands or
sideband requests while another PF holds firmware ownership.
Move the software mutex to struct ice_adapter and record the owning task
across the entire Global Configuration Lock acquisition and release
sequence, including the core reset path in ice_cfg_tx_topo(). In
ice_aq_send_cmd(), require non-owning tasks to wait on the adapter mutex
while allowing the owning task to issue the permitted configuration
commands. Route the Admin Queue fallback in ice_sbq_send_cmd() through
ice_aq_send_cmd() as well.
This patch depends on the previous patch in this series.
Fixes: c76488109616 ("ice: Implement Dynamic Device Personalization (DDP) download")
Signed-off-by: Aaron Ma <aaron.ma@canonical.com>
---
drivers/net/ethernet/intel/ice/ice_adapter.c | 2 +
drivers/net/ethernet/intel/ice/ice_adapter.h | 6 ++
drivers/net/ethernet/intel/ice/ice_common.c | 63 +++++++++++++-------
drivers/net/ethernet/intel/ice/ice_common.h | 3 +-
drivers/net/ethernet/intel/ice/ice_ddp.c | 7 +--
5 files changed, 55 insertions(+), 26 deletions(-)
diff --git a/drivers/net/ethernet/intel/ice/ice_adapter.c b/drivers/net/ethernet/intel/ice/ice_adapter.c
index 7de9223e971a7..17819c3a172fa 100644
--- a/drivers/net/ethernet/intel/ice/ice_adapter.c
+++ b/drivers/net/ethernet/intel/ice/ice_adapter.c
@@ -65,6 +65,7 @@ static struct ice_adapter *ice_adapter_new(struct pci_dev *pdev)
for (int i = 0; i < ARRAY_SIZE(adapter->cpi_phy_lock); i++)
mutex_init(&adapter->cpi_phy_lock[i]);
mutex_init(&adapter->rebuild_lock);
+ mutex_init(&adapter->global_cfg_lock);
refcount_set(&adapter->refcount, 1);
mutex_init(&adapter->ports.lock);
@@ -79,6 +80,7 @@ static void ice_adapter_free(struct ice_adapter *adapter)
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->global_cfg_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 f9265de973efc..6557fa0ee0ec3 100644
--- a/drivers/net/ethernet/intel/ice/ice_adapter.h
+++ b/drivers/net/ethernet/intel/ice/ice_adapter.h
@@ -13,6 +13,7 @@
struct pci_dev;
struct ice_pf;
+struct task_struct;
/**
* struct ice_port_list - data used to store the list of adapter ports
@@ -38,6 +39,8 @@ struct ice_port_list {
* 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
+ * @global_cfg_lock: serialize AQ access during global configuration
+ * @global_cfg_owner: task holding the global configuration lock
* @ports: Ports list
* @index: 64-bit index cached for collision detection on 32bit systems
*/
@@ -51,6 +54,9 @@ struct ice_adapter {
struct mutex cpi_phy_lock[ICE_E825_MAX_PHYS];
/* Serialize PFR recovery touching shared FW global state */
struct mutex rebuild_lock;
+ /* Keep other PFs off the AQ until firmware releases ownership */
+ struct mutex global_cfg_lock;
+ struct task_struct *global_cfg_owner;
struct ice_pf *ctrl_pf;
struct ice_port_list ports;
diff --git a/drivers/net/ethernet/intel/ice/ice_common.c b/drivers/net/ethernet/intel/ice/ice_common.c
index 82b803e74aac0..1793abba8d2b2 100644
--- a/drivers/net/ethernet/intel/ice/ice_common.c
+++ b/drivers/net/ethernet/intel/ice/ice_common.c
@@ -7,6 +7,7 @@
#include "ice_flow.h"
#include "ice_ptp_hw.h"
#include <linux/packing.h>
+#include <linux/sched.h>
#define ICE_PF_RESET_WAIT_COUNT 300
#define ICE_MAX_NETLIST_SIZE 10
@@ -1751,6 +1752,10 @@ static int
ice_sbq_send_cmd(struct ice_hw *hw, struct ice_sbq_cmd_desc *desc,
void *buf, u16 buf_size, struct ice_sq_cd *cd)
{
+ if (!ice_is_sbq_supported(hw))
+ return ice_aq_send_cmd(hw, (struct libie_aq_desc *)desc,
+ buf, buf_size, cd);
+
return ice_sq_send_cmd(hw, ice_get_sbq(hw),
(struct libie_aq_desc *)desc, buf, buf_size, cd);
}
@@ -1797,12 +1802,6 @@ int ice_sbq_rw_reg(struct ice_hw *hw, struct ice_sbq_msg_input *in, u16 flags)
/* FW Admin Queue command wrappers */
-/* Software lock/mutex that is meant to be held while the Global Config Lock
- * in firmware is acquired by the software to prevent most (but not all) types
- * of AQ commands from being sent to FW
- */
-DEFINE_MUTEX(ice_global_cfg_lock_sw);
-
/**
* ice_should_retry_sq_send_cmd
* @opcode: AQ opcode
@@ -1895,17 +1894,24 @@ ice_aq_send_cmd(struct ice_hw *hw, struct libie_aq_desc *desc, void *buf,
u16 buf_size, struct ice_sq_cd *cd)
{
struct libie_aqc_req_res *cmd = libie_aq_raw(desc);
- bool lock_acquired = false;
- int status;
+ struct ice_pf *pf = hw->back;
+ struct ice_adapter *adapter = pf->adapter;
+
+ if (READ_ONCE(adapter->global_cfg_owner) != current) {
+ guard(mutex)(&adapter->global_cfg_lock);
+ return ice_sq_send_cmd_retry(hw, &hw->adminq, desc, buf,
+ buf_size, cd);
+ }
/* When a package download is in process (i.e. when the firmware's
* Global Configuration Lock resource is held), only the Download
* Package, Get Version, Get Package Info List, Upload Section,
* Update Package, Set Port Parameters, Get/Set VLAN Mode Parameters,
- * Add Recipe, Set Recipes to Profile Association, Get Recipe, and Get
- * Recipes to Profile Association, and Release Resource (with resource
- * ID set to Global Config Lock) AdminQ commands are allowed; all others
- * must block until the package download completes and the Global Config
+ * Get/Set Tx Scheduler Topology, Add Recipe, Set Recipes to Profile
+ * Association, Get Recipe, Get Recipes to Profile Association, and
+ * Request/Release Resource (with resource ID set to Global Config
+ * Lock) AdminQ commands are allowed for the owner; all others must
+ * block until the package download completes and the Global Config
* Lock is released. See also ice_acquire_global_cfg_lock().
*/
switch (le16_to_cpu(desc->opcode)) {
@@ -1924,21 +1930,16 @@ ice_aq_send_cmd(struct ice_hw *hw, struct libie_aq_desc *desc, void *buf,
case ice_aqc_opc_get_recipe:
case ice_aqc_opc_get_recipe_to_profile:
break;
+ case ice_aqc_opc_req_res:
case ice_aqc_opc_release_res:
if (le16_to_cpu(cmd->res_id) == LIBIE_AQC_RES_ID_GLBL_LOCK)
break;
fallthrough;
default:
- mutex_lock(&ice_global_cfg_lock_sw);
- lock_acquired = true;
- break;
+ return -EBUSY;
}
- status = ice_sq_send_cmd_retry(hw, &hw->adminq, desc, buf, buf_size, cd);
- if (lock_acquired)
- mutex_unlock(&ice_global_cfg_lock_sw);
-
- return status;
+ return ice_sq_send_cmd_retry(hw, &hw->adminq, desc, buf, buf_size, cd);
}
/**
@@ -2161,10 +2162,17 @@ ice_acquire_res(struct ice_hw *hw, enum ice_aq_res_ids res,
enum ice_aq_res_access_type access, u32 timeout)
{
#define ICE_RES_POLLING_DELAY_MS 10
+ struct ice_pf *pf = hw->back;
+ struct ice_adapter *adapter = pf->adapter;
u32 delay = ICE_RES_POLLING_DELAY_MS;
u32 time_left = timeout;
int status;
+ if (res == ICE_GLOBAL_CFG_LOCK_RES_ID) {
+ mutex_lock(&adapter->global_cfg_lock);
+ WRITE_ONCE(adapter->global_cfg_owner, current);
+ }
+
status = ice_aq_req_res(hw, res, access, 0, &time_left, NULL);
/* A return code of -EALREADY means that another driver has
@@ -2197,6 +2205,9 @@ ice_acquire_res(struct ice_hw *hw, enum ice_aq_res_ids res,
ice_debug(hw, ICE_DBG_RES, "resource acquire timed out.\n");
ice_acquire_res_exit:
+ if (res == ICE_GLOBAL_CFG_LOCK_RES_ID && status)
+ ice_global_cfg_unlock(hw);
+
if (status == -EALREADY) {
if (access == ICE_RES_WRITE)
ice_debug(hw, ICE_DBG_RES, "resource indicates no work to do.\n");
@@ -2228,6 +2239,18 @@ void ice_release_res(struct ice_hw *hw, enum ice_aq_res_ids res)
break;
usleep_range(1000, 2000);
} while (time_before(jiffies, timeout));
+
+ if (res == ICE_GLOBAL_CFG_LOCK_RES_ID)
+ ice_global_cfg_unlock(hw);
+}
+
+void ice_global_cfg_unlock(struct ice_hw *hw)
+{
+ struct ice_pf *pf = hw->back;
+ struct ice_adapter *adapter = pf->adapter;
+
+ WRITE_ONCE(adapter->global_cfg_owner, NULL);
+ mutex_unlock(&adapter->global_cfg_lock);
}
/**
diff --git a/drivers/net/ethernet/intel/ice/ice_common.h b/drivers/net/ethernet/intel/ice/ice_common.h
index d1d674ca644f6..48b53f83aed3c 100644
--- a/drivers/net/ethernet/intel/ice/ice_common.h
+++ b/drivers/net/ethernet/intel/ice/ice_common.h
@@ -98,6 +98,7 @@ int
ice_acquire_res(struct ice_hw *hw, enum ice_aq_res_ids res,
enum ice_aq_res_access_type access, u32 timeout);
void ice_release_res(struct ice_hw *hw, enum ice_aq_res_ids res);
+void ice_global_cfg_unlock(struct ice_hw *hw);
int
ice_alloc_hw_res(struct ice_hw *hw, u16 type, u16 num, bool btm, u16 *res);
int
@@ -142,8 +143,6 @@ void ice_fill_dflt_direct_cmd_desc(struct libie_aq_desc *desc, u16 opcode);
void ice_pack_txq_ctx(const struct ice_tlan_ctx *ctx, ice_txq_ctx_buf_t *buf);
-extern struct mutex ice_global_cfg_lock_sw;
-
int
ice_aq_send_cmd(struct ice_hw *hw, struct libie_aq_desc *desc,
void *buf, u16 buf_size, struct ice_sq_cd *cd);
diff --git a/drivers/net/ethernet/intel/ice/ice_ddp.c b/drivers/net/ethernet/intel/ice/ice_ddp.c
index 3b2d9c4369792..b71f3fb791db7 100644
--- a/drivers/net/ethernet/intel/ice/ice_ddp.c
+++ b/drivers/net/ethernet/intel/ice/ice_ddp.c
@@ -1141,9 +1141,7 @@ static int ice_acquire_global_cfg_lock(struct ice_hw *hw,
status = ice_acquire_res(hw, ICE_GLOBAL_CFG_LOCK_RES_ID, access,
ICE_GLOBAL_CFG_LOCK_TIMEOUT);
- if (!status)
- mutex_lock(&ice_global_cfg_lock_sw);
- else if (status == -EALREADY)
+ if (status == -EALREADY)
ice_debug(hw, ICE_DBG_PKG,
"Global config lock: No work to do\n");
@@ -1158,7 +1156,6 @@ static int ice_acquire_global_cfg_lock(struct ice_hw *hw,
*/
static void ice_release_global_cfg_lock(struct ice_hw *hw)
{
- mutex_unlock(&ice_global_cfg_lock_sw);
ice_release_res(hw, ICE_GLOBAL_CFG_LOCK_RES_ID);
}
@@ -2536,6 +2533,8 @@ int ice_cfg_tx_topo(struct ice_hw *hw, const void *buf, u32 len)
ice_check_reset(hw);
reinit_hw:
+ ice_global_cfg_unlock(hw);
+
/* Since we triggered a CORER, re-initialize hardware */
ice_deinit_hw(hw);
if (ice_init_hw(hw)) {
--
2.53.0
^ permalink raw reply [flat|nested] 9+ messages in thread