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

[Loktionov, Aleksandr]

> -----Original Message-----
> From: Aaron Ma <aaron.ma@xxxxxxxxxxxxx>
> Sent: Monday, October 19, 2026 12:52 PM
> To: Nguyen, Anthony L <anthony.l.nguyen@xxxxxxxxx>; Kitszel, Przemyslaw
> <przemyslaw.kitszel@xxxxxxxxx>; Andrew Lunn <andrew+netdev@xxxxxxx>;
> David S. Miller <davem@xxxxxxxxxxxxx>; Eric Dumazet
> <edumazet@xxxxxxxxxx>; Jakub Kicinski <kuba@xxxxxxxxxx>; Paolo Abeni
> <pabeni@xxxxxxxxxx>; Lobakin, Aleksander <aleksander.lobakin@xxxxxxxxx>;
> Richard Cochran <richardcochran@xxxxxxxxx>; netdev@xxxxxxxxxxxxxxx;
> linux-kernel@xxxxxxxxxxxxxxx
> Cc: Dan Nowlin <dan.nowlin@xxxxxxxxx>; Brett Creeley
> <brett.creeley@xxxxxxxxx>; Keller, Jacob E <jacob.e.keller@xxxxxxxxx>;
> moderated list:INTEL ETHERNET DRIVERS <intel-wired-lan@xxxxxxxxxxxxxxxx>
> 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 <aaron.ma@xxxxxxxxxxxxx>
> ---
>  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);

...

>  /**
>   * 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?

> +
> +	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,

...

>  	/* Since we triggered a CORER, re-initialize hardware */
>  	ice_deinit_hw(hw);
>  	if (ice_init_hw(hw)) {
> --
> 2.53.0