On 7/3/2026 6:34 AM, Przemek Kitszel wrote: > 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. >>
Yikes. >> 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. >> Reviewed-by: Jacob Keller <[email protected]> >> Fixes: e94509906d6b ("ice: create function to read a section of the >> NVM and Shadow RAM") >> Signed-off-by: Robert Malz <[email protected]> > > thank you for extra effort [1] > current fix looks elegant! > Reviewed-by: Przemek Kitszel <[email protected]> > > [1] for reference, this is previous attempt for the fix: > https://lore.kernel.org/intel-wired-lan/CADcc- > [email protected] > > [...] > >> /** >> - * 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) >> { >
