Thread (17 messages) 17 messages, 5 authors, 6h ago

Re: [PATCH 3/5] ice: scope global configuration lock to the adapter

flat view

From: Aaron Ma <hidden>
Date: 2026-10-22 08:18:13
Also in: intel-wired-lan, lkml

On Wed, Oct 21, 2026 at 7:12 PM Loktionov, Aleksandr
[off-list ref] wrote:

quoted
-----Original Message-----
From: Aaron Ma <redacted>
Sent: Monday, October 19, 2026 12:52 PM
To: Nguyen, Anthony L <anthony.l.nguyen@intel.com>; Kitszel, Przemyslaw
[off-list ref]; Andrew Lunn [off-list ref];
David S. Miller [off-list ref]; Eric Dumazet
[off-list ref]; Jakub Kicinski [off-list ref]; Paolo Abeni
[off-list ref]; Lobakin, Aleksander [off-list ref];
Richard Cochran [off-list ref]; netdev@vger.kernel.org;
linux-kernel@vger.kernel.org
Cc: Dan Nowlin <redacted>; Brett Creeley
[off-list ref]; Keller, Jacob E [off-list ref];
moderated list:INTEL ETHERNET DRIVERS [off-list ref]
Subject: [PATCH 3/5] ice: scope global configuration lock to the adapter

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 <redacted>
---
 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);
...
quoted
 /**
  * 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;

pf->adapter can be NULL during ice_init_hw(), is it ok?
Patch 2/5 moves ice_adapter_get() and the pf->adapter assignment before
ice_init_hw() in ice_probe(), and retains the reference until after
hardware teardown. The reference is also retained across Tx-topology
reinitialization and devlink reload, so pf->adapter is valid during
ice_init_hw() with that patch applied.

Thanks for review,
Aaron
quoted
+
+     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,
...
quoted
      /* Since we triggered a CORER, re-initialize hardware */
      ice_deinit_hw(hw);
      if (ice_init_hw(hw)) {
--
2.53.0
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help