On Fri, May 22, 2020 at 01:48:28PM -0400, sven falempin wrote:
> After a few days ... (free size too small 288 < 1024 /2 )
>
> Maybe this can help make the driver better.
>
> printf '%x\n' $((0x350+0xf7)) ; grep -A2 'if_iwx.c:515' /tmp/iwx.dis
> 447
> /usr/src/sys/dev/pci/if_iwx.c:515
> 447: 41 c7 86 28 2f 05 00 movl $0x0,0x52f28(%r14)
> 44e: 00 00 00 00
>
> [0]-[current]-[~]
> # cat -n /usr/src/sys/dev/pci/if_iwx.c | grep -C5 -E ' 515'
> 510 /* free paging*/
> 511 for (i = 0; i < dram->paging_cnt; i++)
> 512 iwx_dma_contig_free(dram->paging);
> 513
> 514 free(dram->paging, M_DEVBUF, dram->paging_cnt *
> sizeof(*dram->paging));
> 515 dram->paging_cnt = 0;
> 516 dram->paging = NULL;
> 517 }
> 518
> 519 int
> 520 iwx_get_num_sections(const struct iwx_fw_sects *fws, int start
This should fix free with a wrong size in the error case, and avoids
re-allocating a chunk of DMA memory (sc->ctxt_info_dma) every time the
firmware gets loaded. Instead, this chunk is now allocated once at
attach time. This seems to be the allocation that failed in your case.
diff 66ecf2e2f524653126dce17a447a43b26ee90abb /usr/src
blob - c3ca08c7a726326e37cda8645596a176051b6cf4
file + sys/dev/pci/if_iwx.c
--- sys/dev/pci/if_iwx.c
+++ sys/dev/pci/if_iwx.c
@@ -230,7 +230,7 @@ int iwx_alloc_fw_monitor_block(struct iwx_softc *, uin
int iwx_alloc_fw_monitor(struct iwx_softc *, uint8_t);
int iwx_apply_debug_destination(struct iwx_softc *);
int iwx_ctxt_info_init(struct iwx_softc *, const struct iwx_fw_sects *);
-void iwx_ctxt_info_free(struct iwx_softc *);
+void iwx_ctxt_info_free_fw_img(struct iwx_softc *);
void iwx_ctxt_info_free_paging(struct iwx_softc *);
int iwx_init_fw_sec(struct iwx_softc *, const struct iwx_fw_sects *,
struct iwx_context_info_dram *);
@@ -535,52 +535,60 @@ iwx_init_fw_sec(struct iwx_softc *sc, const struct iwx
struct iwx_context_info_dram *ctxt_dram)
{
struct iwx_self_init_dram *dram = &sc->init_dram;
- int i, ret, lmac_cnt, umac_cnt, paging_cnt;
+ int i, ret, fw_cnt = 0;
KASSERT(dram->paging == NULL);
- lmac_cnt = iwx_get_num_sections(fws, 0);
+ dram->lmac_cnt = iwx_get_num_sections(fws, 0);
/* add 1 due to separator */
- umac_cnt = iwx_get_num_sections(fws, lmac_cnt + 1);
+ dram->umac_cnt = iwx_get_num_sections(fws, dram->lmac_cnt + 1);
/* add 2 due to separators */
- paging_cnt = iwx_get_num_sections(fws, lmac_cnt + umac_cnt + 2);
+ dram->paging_cnt = iwx_get_num_sections(fws,
+ dram->lmac_cnt + dram->umac_cnt + 2);
- dram->fw = mallocarray(umac_cnt + lmac_cnt, sizeof(*dram->fw),
- M_DEVBUF, M_ZERO | M_NOWAIT);
- if (!dram->fw)
+ dram->fw = mallocarray(dram->umac_cnt + dram->lmac_cnt,
+ sizeof(*dram->fw), M_DEVBUF, M_ZERO | M_NOWAIT);
+ if (!dram->fw) {
+ printf("%s: could not allocate memory for firmware sections\n",
+ DEVNAME(sc));
return ENOMEM;
- dram->paging = mallocarray(paging_cnt, sizeof(*dram->paging),
+ }
+
+ dram->paging = mallocarray(dram->paging_cnt, sizeof(*dram->paging),
M_DEVBUF, M_ZERO | M_NOWAIT);
- if (!dram->paging)
+ if (!dram->paging) {
+ printf("%s: could not allocate memory for firmware paging\n",
+ DEVNAME(sc));
return ENOMEM;
+ }
/* initialize lmac sections */
- for (i = 0; i < lmac_cnt; i++) {
+ for (i = 0; i < dram->lmac_cnt; i++) {
ret = iwx_ctxt_info_alloc_dma(sc, &fws->fw_sect[i],
- &dram->fw[dram->fw_cnt]);
+ &dram->fw[fw_cnt]);
if (ret)
return ret;
ctxt_dram->lmac_img[i] =
- htole64(dram->fw[dram->fw_cnt].paddr);
+ htole64(dram->fw[fw_cnt].paddr);
DPRINTF(("%s: firmware LMAC section %d at 0x%llx size %lld\n",
__func__, i,
- (unsigned long long)dram->fw[dram->fw_cnt].paddr,
- (unsigned long long)dram->fw[dram->fw_cnt].size));
- dram->fw_cnt++;
+ (unsigned long long)dram->fw[fw_cnt].paddr,
+ (unsigned long long)dram->fw[fw_cnt].size));
+ fw_cnt++;
}
/* initialize umac sections */
- for (i = 0; i < umac_cnt; i++) {
+ for (i = 0; i < dram->umac_cnt; i++) {
/* access FW with +1 to make up for lmac separator */
ret = iwx_ctxt_info_alloc_dma(sc,
- &fws->fw_sect[dram->fw_cnt + 1], &dram->fw[dram->fw_cnt]);
+ &fws->fw_sect[fw_cnt + 1], &dram->fw[fw_cnt]);
if (ret)
return ret;
ctxt_dram->umac_img[i] =
- htole64(dram->fw[dram->fw_cnt].paddr);
+ htole64(dram->fw[fw_cnt].paddr);
DPRINTF(("%s: firmware UMAC section %d at 0x%llx size %lld\n",
__func__, i,
- (unsigned long long)dram->fw[dram->fw_cnt].paddr,
- (unsigned long long)dram->fw[dram->fw_cnt].size));
- dram->fw_cnt++;
+ (unsigned long long)dram->fw[fw_cnt].paddr,
+ (unsigned long long)dram->fw[fw_cnt].size));
+ fw_cnt++;
}
/*
@@ -593,9 +601,9 @@ iwx_init_fw_sec(struct iwx_softc *sc, const struct iwx
* Given that, the logic here in accessing the fw image is a bit
* different - fw_cnt isn't changing so loop counter is added to it.
*/
- for (i = 0; i < paging_cnt; i++) {
+ for (i = 0; i < dram->paging_cnt; i++) {
/* access FW with +2 to make up for lmac & umac separators */
- int fw_idx = dram->fw_cnt + i + 2;
+ int fw_idx = fw_cnt + i + 2;
ret = iwx_ctxt_info_alloc_dma(sc,
&fws->fw_sect[fw_idx], &dram->paging[i]);
@@ -606,7 +614,6 @@ iwx_init_fw_sec(struct iwx_softc *sc, const struct iwx
DPRINTF(("%s: firmware paging section %d at 0x%llx size
%lld\n", __func__, i,
(unsigned long long)dram->paging[i].paddr,
(unsigned long long)dram->paging[i].size));
- dram->paging_cnt++;
}
return 0;
@@ -758,13 +765,6 @@ iwx_ctxt_info_init(struct iwx_softc *sc, const struct
uint32_t control_flags = 0, rb_size;
int err;
- err = iwx_dma_contig_alloc(sc->sc_dmat, &sc->ctxt_info_dma,
- sizeof(*ctxt_info), 0);
- if (err) {
- printf("%s: could not allocate context info DMA memory\n",
- DEVNAME(sc));
- return err;
- }
ctxt_info = sc->ctxt_info_dma.vaddr;
ctxt_info->version.version = 0;
@@ -800,15 +800,17 @@ iwx_ctxt_info_init(struct iwx_softc *sc, const struct
/* allocate ucode sections in dram and set addresses */
err = iwx_init_fw_sec(sc, fws, &ctxt_info->dram);
if (err) {
- iwx_ctxt_info_free(sc);
+ iwx_ctxt_info_free_fw_img(sc);
return err;
}
/* Configure debug, if exists */
if (sc->sc_fw.dbg_dest_tlv_v1) {
err = iwx_apply_debug_destination(sc);
- if (err)
+ if (err) {
+ iwx_ctxt_info_free_fw_img(sc);
return err;
+ }
}
/* kick FW self load */
@@ -829,26 +831,19 @@ iwx_ctxt_info_free_fw_img(struct iwx_softc *sc)
struct iwx_self_init_dram *dram = &sc->init_dram;
int i;
- if (!dram->fw) {
- KASSERT(dram->fw_cnt == 0);
+ if (!dram->fw)
return;
- }
- for (i = 0; i < dram->fw_cnt; i++)
+ for (i = 0; i < dram->lmac_cnt + dram->umac_cnt; i++)
iwx_dma_contig_free(&dram->fw[i]);
- free(dram->fw, M_DEVBUF, dram->fw_cnt * sizeof(dram->fw[0]));
- dram->fw_cnt = 0;
+ free(dram->fw, M_DEVBUF,
+ (dram->lmac_cnt + dram->umac_cnt) * sizeof(*dram->fw));
+ dram->lmac_cnt = 0;
+ dram->umac_cnt = 0;
dram->fw = NULL;
}
-void
-iwx_ctxt_info_free(struct iwx_softc *sc)
-{
- iwx_dma_contig_free(&sc->ctxt_info_dma);
- iwx_ctxt_info_free_fw_img(sc);
-}
-
int
iwx_firmware_store_section(struct iwx_softc *sc, enum iwx_ucode_type type,
uint8_t *data, size_t dlen)
@@ -2410,7 +2405,6 @@ void
iwx_post_alive(struct iwx_softc *sc)
{
iwx_ict_reset(sc);
- iwx_ctxt_info_free(sc);
}
/*
@@ -3113,6 +3107,9 @@ iwx_load_firmware(struct iwx_softc *sc)
}
if (err || !sc->sc_uc.uc_ok)
printf("%s: could not load firmware\n", DEVNAME(sc));
+
+ iwx_ctxt_info_free_fw_img(sc);
+
if (!sc->sc_uc.uc_ok)
return EINVAL;
@@ -7774,6 +7771,15 @@ iwx_attach(struct device *parent, struct device *self,
return;
}
+ /* Allocate DMA memory for loading firmware. */
+ err = iwx_dma_contig_alloc(sc->sc_dmat, &sc->ctxt_info_dma,
+ sizeof(struct iwx_context_info), 0);
+ if (err) {
+ printf("%s: could not allocate memory for loading firmware\n",
+ DEVNAME(sc));
+ return;
+ }
+
/*
* Allocate DMA memory for firmware transfers.
* Must be aligned on a 16-byte boundary.
@@ -7781,9 +7787,9 @@ iwx_attach(struct device *parent, struct device *self,
err = iwx_dma_contig_alloc(sc->sc_dmat, &sc->fw_dma,
sc->sc_fwdmasegsz, 16);
if (err) {
- printf("%s: could not allocate memory for firmware\n",
+ printf("%s: could not allocate memory for firmware transfers\n",
DEVNAME(sc));
- return;
+ goto fail0;
}
/* Allocate interrupt cause table (ICT).*/
@@ -7915,6 +7921,7 @@ fail3: if (sc->ict_dma.vaddr != NULL)
iwx_dma_contig_free(&sc->ict_dma);
fail1: iwx_dma_contig_free(&sc->fw_dma);
+fail0: iwx_dma_contig_free(&sc->ctxt_info_dma);
return;
}
blob - 7a0d1c61f771b3691baa72ea835940bcffb48c64
file + sys/dev/pci/if_iwxvar.h
--- sys/dev/pci/if_iwxvar.h
+++ sys/dev/pci/if_iwxvar.h
@@ -346,13 +346,15 @@ struct iwx_bf_data {
/**
* struct iwx_self_init_dram - dram data used by self init process
* @fw: lmac and umac dram data
- * @fw_cnt: total number of items in array
+ * @lmac_cnt: number of lmac sections in fw image
+ * @umac_cnt: number of umac sections in fw image
* @paging: paging dram data
- * @paging_cnt: total number of items in array
+ * @paging_cnt: number of paging sections needed by fw image
*/
struct iwx_self_init_dram {
struct iwx_dma_info *fw;
- int fw_cnt;
+ int lmac_cnt;
+ int umac_cnt;
struct iwx_dma_info *paging;
int paging_cnt;
};
