Bug 219683

Comment 1 Mark Linimon 2017-05-31 17:57:34 UTC

Over to committer of r318215.

Hi,

this is error is caused because I am using a 32 bit kernel (and world).

Booting the machine with a 64 bit kernel[1] via USB stick and doing k"ldload
if_iwm" is working fine. Booting the machine with a 32 bit kernel [2] shows
the known error message. Both kernel have the same release.

The used kernel are
[1] FreeBSD-12.0-CURRENT-amd64-20170703-r320599-memstick.img
[2] FreeBSD-12.0-CURRENT-i386-20170703-r320599-memstick.img

Ralf

Comment 3 commit-hook 2019-01-15 22:32:05 UTC

A commit references this bug:

Author: bz
Date: Tue Jan 15 22:31:54 UTC 2019
New revision: 343065
URL: https://svnweb.freebsd.org/changeset/base/343065

Log:
  With the sync from Dragonfly BSD in r318216 a bug slipped in (also still present
  upstream it seems).

  The tlv variable was changed to a pointer but the advancement of the data pointer
  was left as sizeof(tlv).  While the sizeof the (now) pointer equals the
  sizeof 2 x uint32_t (size of the struct) on 64bit platforms, on 32bit platforms
  the size of the advancement of the data pointer was wrong leading to
  firmware load issues.

  Correctly advance the data pointer by the size of the structure and not by
  the size of a pointer.

  PR:		219683
  Submitted by:	waddlesplash gamil.com (Haiku) on irc
  MFC after:	1 week

Changes:
  head/sys/dev/iwm/if_iwm.c

I can confirm that r343065 fixes the bug on the original hardware as reported.
Thank you.

Comment 5 commit-hook 2019-04-08 18:15:50 UTC

A commit references this bug:

Author: kevans
Date: Mon Apr  8 18:15:11 UTC 2019
New revision: 346037
URL: https://svnweb.freebsd.org/changeset/base/346037

Log:
  MFC r343065, r343373-r343390, r343477

  r343065:
  With the sync from Dragonfly BSD in r318216 a bug slipped in (also still present
  upstream it seems).

  The tlv variable was changed to a pointer but the advancement of the data pointer
  was left as sizeof(tlv).  While the sizeof the (now) pointer equals the
  sizeof 2 x uint32_t (size of the struct) on 64bit platforms, on 32bit platforms
  the size of the advancement of the data pointer was wrong leading to
  firmware load issues.

  Correctly advance the data pointer by the size of the structure and not by
  the size of a pointer.

  r343373:
  if_iwm - Update firmware rs table, instead of indexing the table in tx cmds.

  * Rather than providing a non-zero index into the firmware RS table,
  we should always use index 0 and update the firmware RS table whenever
  our chosen tx rate for data-frames changes.

  * Send IWM_LQ_CMD updates when the tx rate gets updated by the net80211
  rate control (which is after we tell the tx status to the net80211
  rate-control in iwm_mvm_rx_tx_cmd_single()).

  * Disregard frames transferred with a different tx rate than the currently
  selected rate for the rate-control calculations. This way we avoid
  counting management frames (which are sent at a slow, and fixed rate),
  as well as frames we added to the tx queue just before a new IWM_LQ_CMD
  update took effect.

  r343374:
  if_iwm - The iwm_prepare_card_hw() in iwm_attach() is only needed on 8K hw.

  * Doing the iwm_prepare_card_hw() call in iwm_attach() only on Family 8000
  hardware matches the code in Linux iwlwifi.

  * While there remove DEFAULT_MAX_TX_POWER definition which is unused, and
  has a value different from IWL_DEFAULT_MAX_TX_POWER in iwlwifi.

  r343375:
  if_iwm - Move iwm_read_firmware() call into iwm_attach().

  * We should load the firmware exactly once before the driver really
  initializes the hardware the first time, and unload it at detach time.
  There is no need to retrieve the firmware during execution of
  iwm_mvm_load_ucode_wait_alive(), we should make sure we already have the
  firmware data at hand before that.

  * The existing sc_preinit_hook code fails to deal with the case where
  if_iwm is loaded by the loader (or is statically linked) and the
  firmware needs to be loaded from disk. So we can just call
  iwm_read_firmware() from iwm_attach() directly.

  * A separate solution will have to be added to properly defer the firmware
  loading during bootup, until the necessary filesystem is mounted.

  r343376:
  if_iwm - Check sc->sc_attached flag in suspend/resume callbacks.

  * There is (almost) nothing to do in suspend/resume if if_iwm has failed
  during initialization (e.g. because of firmware load failure) and was
  already uninitialized by iwm_detach_local().

  r343377:
  iwm - Reduce gratuitous differences with Linux iwlwifi in struct naming.

  * Rename some structs and struct members for firmware handling.

  r343378:
  if_iwm - Update struct iwm_scan_results_notif. Remove old/unused definitions

  * Remove outdated notifications IWM_SCAN_ABORT_CMD,
  IWM_SCAN_START_NOTIFICATION and IWM_SCAN_RESULTS_NOTIFICATION.

  * Remove unused enum iwm_scan_complete_status.

  * Use the updated FW Api version 3 of struct iwm_scan_results_notif.

  * No functional change, since struct iwm_scan_results_notif is never
  accessed in iwm at the moment.

  Taken-From: Linux iwlwifi commits 1083fd7391e989be52022f0f338e9dadc048b063
  	and 75118fdb63496e4611ab50380499ddd62b9de69f.

  r343379:
  if_iwm - Configure the PCIe LTR, fix PCI express capability accesses.

  Taken-From: Linux iwlwifi

  r343380:
  if_iwm - Add firmware API definitions for TX power commands.

  * While there remove unused IWM_UCODE_TLV_CAPA_LMAC_UPLOAD definition,
  which isn't defined in iwlwifi.

  Taken-From: Linux iwlwifi

  r343381:
  iwm - Track firmware state better, and improve handling in iwm_newstate().

  * This avoids firmware resets in all the cases in iwm_newstate(). Instead
  iwm_bring_down_firmware() is called, which tears down all the STA
  connection state, according to the sc->sc_firmware_state value.

  * Improve the behaviour of the LED blinking a bit, so it only blinks when
  there really is a wireless scan going on.

  * Print the newstate arg in debug output of iwm_newstate(), to help in
  debugging.

  This is inspired by the firmware state maintaining change in OpenBSD's iwm,
  by stsp@openbsd.org (OpenBSD Git 0ddb056fb7370664b1d4b84392697cb17d1a414a).

  r343382:
  iwm - Avoid Tx watchdog timeout, when dropping a connection.

  r343383:
  iwm - Improve firmware Time Event handling.

  * This is a mix of the OpenBSD Git 7fd9664469d1b717a307eebd74aeececbd3c41cc
  change, and syncing with the Linux iwlwifi code.

  Taken-From: Linux iwlwifi, and OpenBSD

  r343384:
  iwm - Clear Time Event active state, when receiving End Notification.

  * This hopefully avoids some firmware panics, I was occasionally seeing,
  when iwm disconnects upon losing signal to an access point at some point.

  * This is synchronizing the if_iwm_time_event.c file a bit more from the
  corresponding Linux iwlwifi/mvm/time-event.c.

  Taken-From:     Linux iwlwifi

  r343385:
  iwm - Always clear watchdog timer, when bringing down firmware state.

  r343386:
  if_iwm - Stop iwm_watchdog callout when idle.

  r343387:
  iwm - Fix race during detach, where a callout is left after driver is gone.

  r343388:
  iwm - Update alive response handling, add v4 and remove old versions.

  r343389:
  iwm - Remove unused REPLY_MAX

  Taken-From: Linux git e4eb275ac5cfe71686612d929a9829345b2a4ada

  r343390:
  iwm - Remove unused TX_CMD_NEXT_FRAME_*

  Taken-From: Linux git b1e06c65fb69c5e3fddcd91987561e225eaa9bfa

  r343477:
  Fix logic errors in iwm_pcie_load_firmware_chunk introduced in r314065.

   * There's no reason to have a while() loop here, because:
      - if msleep returns 0, that means we were woken up by the interrupt handler,
        and we are going to exit immediately as sc_fw_chunk_done will now be 1
        (there is nothing else that sleeps on sc_fw.)
      - if msleep doesn't return 0 (i.e. it returned ETIMEDOUT) then we will
        exit immediately because of the if-test.
     So, just use a single msleep() and then check sc_fw_chunk_done as before.
   * The comment said we were sleeping for 5 seconds, but the msleep was only
     for 1. Before r314065, this was 1 second and so was the comment,
     and in that commit the comment was changed and the function call wasn't.

  Possibly fixes failures to initialize uCode on certain devices.

  PR:		219683

Changes:
_U  stable/12/
  stable/12/sys/dev/iwm/if_iwm.c
  stable/12/sys/dev/iwm/if_iwm_config.h
  stable/12/sys/dev/iwm/if_iwm_debug.h
  stable/12/sys/dev/iwm/if_iwm_fw.c
  stable/12/sys/dev/iwm/if_iwm_fw.h
  stable/12/sys/dev/iwm/if_iwm_led.c
  stable/12/sys/dev/iwm/if_iwm_mac_ctxt.c
  stable/12/sys/dev/iwm/if_iwm_pcie_trans.c
  stable/12/sys/dev/iwm/if_iwm_phy_db.c
  stable/12/sys/dev/iwm/if_iwm_scan.c
  stable/12/sys/dev/iwm/if_iwm_sta.c
  stable/12/sys/dev/iwm/if_iwm_time_event.c
  stable/12/sys/dev/iwm/if_iwm_time_event.h
  stable/12/sys/dev/iwm/if_iwmreg.h
  stable/12/sys/dev/iwm/if_iwmvar.h