Thread (3 messages) 3 messages, 3 authors, 8d ago

Re: [PATCH iwl] ice: acquire NVM lock around each flash read

From: Przemek Kitszel <przemyslaw.kitszel@intel.com>
Date: 2026-07-03 13:34:33
Also in: intel-wired-lan, lkml

On 7/3/26 12:32, Robert Malz wrote:
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 would overwrite sq_last_status, so the read's AQ error is preserved
across the release for callers such as ice_discover_flash_size() that
inspect it.

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 per-block locking in ice_devlink_nvm_snapshot() is now redundant
and dropped.

Fixes: e94509906d6b ("ice: create function to read a section of the NVM and Shadow RAM")
Signed-off-by: Robert Malz <redacted>
thank you for extra effort [1]
current fix looks elegant!
Reviewed-by: Przemek Kitszel <przemyslaw.kitszel@intel.com>

[1] for reference, this is previous attempt for the fix:
https://lore.kernel.org/intel-wired-lan/CADcc-bysA531q2Wh=TD_oFqxivLLdnCRNY5jy7mkZuO0cwJwvg@mail.gmail.com (local)

[...]
  /**
- * ice_read_sr_word - Reads Shadow RAM word and acquire NVM if necessary
+ * ice_read_sr_word - Reads Shadow RAM word
   * @hw: pointer to the HW structure
   * @offset: offset of the Shadow RAM word to read (0x000000 - 0x001FFF)
   * @data: word read from the Shadow RAM
   *
- * Reads one 16 bit word from the Shadow RAM using the ice_read_sr_word_aq.
+ * Reads one 16 bit word from the Shadow RAM using ice_read_sr_word_aq.
+ *
+ * The NVM lock is acquired and released internally by ice_read_flat_nvm()
+ * around the FW read, so this function must be called without the lock held.
   */
for future submissions would be great to "fix" kdoc warnings of touched
functions, here "Return: " section is missing.
I do not ask to fix this particular one (given there will be no ask for
v2 otherwise).
  int ice_read_sr_word(struct ice_hw *hw, u16 offset, u16 *data)
  {
  
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help