Re: iwx(4) firmware memory fixes

2021-09-09 Thread Mike Larkin
On Wed, Sep 08, 2021 at 02:08:36PM +0200, Stefan Sperling wrote:
> Add a missing call to iwx_ctxt_info_free_fw_img() in an error path
> of iwx_ctxt_info_init() which should always free on error.
>
> Also, free firmware paging DMA memory in case loading firmware has failed.
> If we don't free paging on error we hit KASSERT(dram->paging == NULL)
> in iwx_init_fw_sec() once we try to load firmware again.  I have hit
> this while debugging firmware load failures during suspend/resume.
>
> (Ideally, we would re-allocate firmware image and paging memory only
> after re-loading a potentially different fw image, but this can be
> fixed later.)
>
> ok?
>

ok mlarkin

> diff 50816b19557cd9c29c50f92eebbe32098a494bd3 
> 055f053850bb0f3af81ea3aa7c4f705a85cfcb76
> blob - f7d69707ed0a98dfcd7717c9c82faac3af4f39d7
> blob + 51063c862bfc0cf2dc9fbe3f41628bbdbdf3486e
> --- sys/dev/pci/if_iwx.c
> +++ sys/dev/pci/if_iwx.c
> @@ -914,8 +914,10 @@ iwx_ctxt_info_init(struct iwx_softc *sc, const struct
>   IWX_WRITE(sc, IWX_CSR_CTXT_INFO_BA + 4, paddr >> 32);
>
>   /* kick FW self load */
> - if (!iwx_nic_lock(sc))
> + if (!iwx_nic_lock(sc)) {
> + iwx_ctxt_info_free_fw_img(sc);
>   return EBUSY;
> + }
>   iwx_write_prph(sc, IWX_UREG_CPU_INIT_RUN, 1);
>   iwx_nic_unlock(sc);
>
> @@ -3364,8 +3366,10 @@ iwx_load_firmware(struct iwx_softc *sc)
>
>   /* wait for the firmware to load */
>   err = tsleep_nsec(&sc->sc_uc, 0, "iwxuc", SEC_TO_NSEC(1));
> - if (err || !sc->sc_uc.uc_ok)
> + if (err || !sc->sc_uc.uc_ok) {
>   printf("%s: could not load firmware, %d\n", DEVNAME(sc), err);
> + iwx_ctxt_info_free_paging(sc);
> + }
>
>   iwx_ctxt_info_free_fw_img(sc);
>
>



iwx(4) firmware memory fixes

2021-09-08 Thread Stefan Sperling
Add a missing call to iwx_ctxt_info_free_fw_img() in an error path
of iwx_ctxt_info_init() which should always free on error.

Also, free firmware paging DMA memory in case loading firmware has failed.
If we don't free paging on error we hit KASSERT(dram->paging == NULL)
in iwx_init_fw_sec() once we try to load firmware again.  I have hit
this while debugging firmware load failures during suspend/resume.

(Ideally, we would re-allocate firmware image and paging memory only
after re-loading a potentially different fw image, but this can be
fixed later.)

ok?
 
diff 50816b19557cd9c29c50f92eebbe32098a494bd3 
055f053850bb0f3af81ea3aa7c4f705a85cfcb76
blob - f7d69707ed0a98dfcd7717c9c82faac3af4f39d7
blob + 51063c862bfc0cf2dc9fbe3f41628bbdbdf3486e
--- sys/dev/pci/if_iwx.c
+++ sys/dev/pci/if_iwx.c
@@ -914,8 +914,10 @@ iwx_ctxt_info_init(struct iwx_softc *sc, const struct 
IWX_WRITE(sc, IWX_CSR_CTXT_INFO_BA + 4, paddr >> 32);
 
/* kick FW self load */
-   if (!iwx_nic_lock(sc))
+   if (!iwx_nic_lock(sc)) {
+   iwx_ctxt_info_free_fw_img(sc);
return EBUSY;
+   }
iwx_write_prph(sc, IWX_UREG_CPU_INIT_RUN, 1);
iwx_nic_unlock(sc);
 
@@ -3364,8 +3366,10 @@ iwx_load_firmware(struct iwx_softc *sc)
 
/* wait for the firmware to load */
err = tsleep_nsec(&sc->sc_uc, 0, "iwxuc", SEC_TO_NSEC(1));
-   if (err || !sc->sc_uc.uc_ok)
+   if (err || !sc->sc_uc.uc_ok) {
printf("%s: could not load firmware, %d\n", DEVNAME(sc), err);
+   iwx_ctxt_info_free_paging(sc);
+   }
 
iwx_ctxt_info_free_fw_img(sc);