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

[Aaron Ma]

On Wed, Oct 21, 2026 at 7:12 PM Loktionov, Aleksandr
<aleksandr.loktionov@xxxxxxxxx> wrote:
>
>
>
> > -----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?

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

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