RE: [Intel-wired-lan] [PATCH iwl v4] ice: acquire NVM lock around each flash read

[Rinitha, SX]

> -----Original Message-----
> From: Intel-wired-lan <intel-wired-lan-bounces@xxxxxxxxxx> On Behalf Of Robert Malz via Intel-wired-lan
> Sent: 04 August 2026 14:06
> 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>; Jesse Brandeburg <jbrandeb@xxxxxxxxxx>; Keller, Jacob E <jacob.e.keller@xxxxxxxxx>
> Cc: Robert Malz <robert.malz@xxxxxxxxxxxxx>; intel-wired-lan@xxxxxxxxxxxxxxxx; netdev@xxxxxxxxxxxxxxx; linux-kernel@xxxxxxxxxxxxxxx
> Subject: [Intel-wired-lan] [PATCH iwl v4] ice: acquire NVM lock around each flash read
>
> FW caps the NVM read lock at a maximum of 3000ms regardless of the timeout requested via ice_acquire_nvm(). ice_read_flat_nvm() splits a read into multiple ice_aq_read_nvm() commands, one per 4KB sector, all issued under a single lock taken by the caller. Reading a large region can exceed 3000ms, so FW reclaims the lock mid-read and the remaining commands might fail.
>
> Move the lock acquire/release into ice_read_flat_nvm() so it brackets each individual ice_aq_read_nvm() command, ensuring the lock is never held across more than one FW read.
>
> ice_release_nvm() issues its own AQ command and overwrites
> hw->adminq.sq_last_status, which some callers inspect after a failed read.
> Add an optional read_aq_err output parameter to ice_read_flat_nvm() to capture the failing read's AQ error before the release; callers that need it (ice_discover_flash_size() and the ethtool/devlink log paths) use it instead of sq_last_status, others pass NULL.
>
> Callers that previously took the lock around ice_read_flat_nvm(),
> ice_read_sr_word() or ice_read_flash_module() now call them without it.
> The now-redundant per-block locking in ice_devlink_nvm_snapshot() is dropped. ice_read_sr_word() is now a thin wrapper, so ice_read_sr_word_aq() is folded into it.
>
> Fixes: e94509906d6b ("ice: create function to read a section of the NVM and Shadow RAM")
> Signed-off-by: Robert Malz <robert.malz@xxxxxxxxxxxxx>
> ---
> v4:
> - Fold ice_read_sr_word_aq() into ice_read_sr_word() now that the latter is
>   only a wrapper.
> - Reduce the scope of read_aq_err in ice_devlink_nvm_snapshot() to the read
>   loop.
> - Fix reverse christmas tree ordering of the added read_aq_err declarations.
> v3:
> - Log the failure via ice_debug() when ice_acquire_nvm() fails inside
>   ice_read_flat_nvm(), rather than silently aborting the read.
> v2:
> - Replace the save/restore of sq_last_status across ice_release_nvm(),
>   which could race with a concurrent AdminQ command, with a new optional
>   read_aq_err output parameter.
> - Add missing "Return:" kdoc to ice_read_sr_word().
> ---
> .../net/ethernet/intel/ice/devlink/devlink.c  | 32 ++-----  drivers/net/ethernet/intel/ice/ice_ethtool.c  | 16 +---
> drivers/net/ethernet/intel/ice/ice_nvm.c      | 90 ++++++++++---------
> drivers/net/ethernet/intel/ice/ice_nvm.h      |  2 +-
> 4 files changed, 58 insertions(+), 82 deletions(-)
>

Tested-by: Rinitha S <sx.rinitha@xxxxxxxxx> (A Contingent worker at Intel)