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

[Aaron Ma <aaron.ma@canonical.com>]

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