From: Stefan Sperling Subject: Re: qwx(4) suspend/hibernate + resume To: Theo de Raadt Cc: tech@openbsd.org Date: Wed, 21 Feb 2024 22:28:05 +0100 On Wed, Feb 21, 2024 at 10:18:25AM -0700, Theo de Raadt wrote: > Works for multiple suspends on a x13s. In addition to the previous diff, I propose the following to uncouple qwx(4) resume from disk-device resume. I recall us going through similarly careful steps in iwx(4). Successfully tested with hibernate. ----------------------------------------------- cache qwx(4) firmware images in memory across suspend/resume cycles diff 6dc955176d769c398ad467a99b6434d9c54d57b8 2ccfb7ca303c6e380a8a8790fa14b1556eba4167 commit - 6dc955176d769c398ad467a99b6434d9c54d57b8 commit + 2ccfb7ca303c6e380a8a8790fa14b1556eba4167 blob - b1c0c1a5f3627951ac086c6d403fbbd57d8f8b1b blob + 43753d3d31405116391a7f706fb0735162b062b5 --- sys/dev/ic/qwx.c +++ sys/dev/ic/qwx.c @@ -299,6 +299,18 @@ qwx_stop(struct ifnet *ifp) splx(s); } +void +qwx_free_firmware(struct qwx_softc *sc) +{ + int i; + + for (i = 0; i < nitems(sc->fw_img); i++) { + free(sc->fw_img[i].data, M_DEVBUF, sc->fw_img[i].size); + sc->fw_img[i].data = NULL; + sc->fw_img[i].size = 0; + } +} + int qwx_ioctl(struct ifnet *ifp, u_long cmd, caddr_t data) { @@ -322,7 +334,7 @@ qwx_ioctl(struct ifnet *ifp, u_long cmd, caddr_t data) if (ifp->if_flags & IFF_UP) { if (!(ifp->if_flags & IFF_RUNNING)) { /* Force reload of firmware image from disk. */ - sc->have_firmware = 0; + qwx_free_firmware(sc); err = qwx_init(ifp); } } else { @@ -8374,21 +8386,35 @@ err_free_req: int qwx_qmi_load_bdf_qmi(struct qwx_softc *sc, int regdb) { - u_char *data; + u_char *data = NULL; const u_char *boardfw; - size_t len, boardfw_len; + size_t len = 0, boardfw_len; uint32_t fw_size; int ret = 0, bdf_type; #ifdef notyet const uint8_t *tmp; uint32_t file_type; #endif + int fw_idx = regdb ? QWX_FW_REGDB : QWX_FW_BOARD; - ret = qwx_core_fetch_bdf(sc, &data, &len, &boardfw, &boardfw_len, - regdb ? ATH11K_REGDB_FILE : ATH11K_BOARD_API2_FILE); - if (ret) - return ret; + if (sc->fw_img[fw_idx].data) { + boardfw = sc->fw_img[fw_idx].data; + boardfw_len = sc->fw_img[fw_idx].size; + } else { + ret = qwx_core_fetch_bdf(sc, &data, &len, + &boardfw, &boardfw_len, + regdb ? ATH11K_REGDB_FILE : ATH11K_BOARD_API2_FILE); + if (ret) + return ret; + sc->fw_img[fw_idx].data = malloc(boardfw_len, M_DEVBUF, + M_NOWAIT); + if (sc->fw_img[fw_idx].data) { + memcpy(sc->fw_img[fw_idx].data, boardfw, boardfw_len); + sc->fw_img[fw_idx].size = boardfw_len; + } + } + if (regdb) bdf_type = ATH11K_QMI_BDF_TYPE_REGDB; else if (boardfw_len >= QWX_SELFMAG && @@ -8506,16 +8532,24 @@ qwx_qmi_m3_load(struct qwx_softc *sc) char path[PATH_MAX]; int ret; - ret = snprintf(path, sizeof(path), "%s-%s-%s", - ATH11K_FW_DIR, sc->hw_params.fw.dir, ATH11K_M3_FILE); - if (ret < 0 || ret >= sizeof(path)) - return ENOSPC; + if (sc->fw_img[QWX_FW_M3].data) { + data = sc->fw_img[QWX_FW_M3].data; + len = sc->fw_img[QWX_FW_M3].size; + } else { + ret = snprintf(path, sizeof(path), "%s-%s-%s", + ATH11K_FW_DIR, sc->hw_params.fw.dir, ATH11K_M3_FILE); + if (ret < 0 || ret >= sizeof(path)) + return ENOSPC; - ret = loadfirmware(path, &data, &len); - if (ret) { - printf("%s: could not read %s (error %d)\n", - sc->sc_dev.dv_xname, path, ret); - return ret; + ret = loadfirmware(path, &data, &len); + if (ret) { + printf("%s: could not read %s (error %d)\n", + sc->sc_dev.dv_xname, path, ret); + return ret; + } + + sc->fw_img[QWX_FW_M3].data = data; + sc->fw_img[QWX_FW_M3].size = len; } if (sc->m3_mem == NULL || QWX_DMA_LEN(sc->m3_mem) < len) { @@ -8531,7 +8565,6 @@ qwx_qmi_m3_load(struct qwx_softc *sc) } memcpy(QWX_DMA_KVA(sc->m3_mem), data, len); - free(data, M_DEVBUF, len); return 0; } @@ -24709,6 +24742,8 @@ qwx_detach(struct qwx_softc *sc) qwx_dmamem_free(sc->sc_dmat, sc->m3_mem); sc->m3_mem = NULL; } + + qwx_free_firmware(sc); } struct qwx_dmamem * blob - f7f429fa27e3fc5f63cb9345a9e90f46d5fbf15d blob + bc97e1e56f9ff5873d23b3782193a2af97f76e00 --- sys/dev/ic/qwxvar.h +++ sys/dev/ic/qwxvar.h @@ -1782,7 +1782,14 @@ struct qwx_softc { struct qwx_survey_info survey[IEEE80211_CHAN_MAX]; int attached; - int have_firmware; + struct { + u_char *data; + size_t size; + } fw_img[4]; +#define QWX_FW_AMSS 0 +#define QWX_FW_BOARD 1 +#define QWX_FW_M3 2 +#define QWX_FW_REGDB 3 int sc_tx_timer; uint32_t qfullmsk; blob - 9fd14232402e064ea2c520f63ba8d6afd0b36431 blob + d09d91a129077f0a85e43035530676df61c3afd6 --- sys/dev/pci/if_qwx_pci.c +++ sys/dev/pci/if_qwx_pci.c @@ -371,7 +371,7 @@ struct qwx_pci_softc { struct taskq *mhi_taskq; /* - * DMA memory for AMMS.bin firmware image. + * DMA memory for AMSS.bin firmware image. * This memory must remain available to the device until * the device is powered down. */ @@ -3032,23 +3032,31 @@ qwx_mhi_fw_load_handler(struct qwx_pci_softc *psc) u_char *data; size_t len; - ret = snprintf(amss_path, sizeof(amss_path), "%s-%s-%s", - ATH11K_FW_DIR, sc->hw_params.fw.dir, ATH11K_AMSS_FILE); - if (ret < 0 || ret >= sizeof(amss_path)) - return ENOSPC; + if (sc->fw_img[QWX_FW_AMSS].data) { + data = sc->fw_img[QWX_FW_AMSS].data; + len = sc->fw_img[QWX_FW_AMSS].size; + } else { + ret = snprintf(amss_path, sizeof(amss_path), "%s-%s-%s", + ATH11K_FW_DIR, sc->hw_params.fw.dir, ATH11K_AMSS_FILE); + if (ret < 0 || ret >= sizeof(amss_path)) + return ENOSPC; - ret = loadfirmware(amss_path, &data, &len); - if (ret) { - printf("%s: could not read %s (error %d)\n", - sc->sc_dev.dv_xname, amss_path, ret); - return ret; - } + ret = loadfirmware(amss_path, &data, &len); + if (ret) { + printf("%s: could not read %s (error %d)\n", + sc->sc_dev.dv_xname, amss_path, ret); + return ret; + } - if (len < MHI_DMA_VEC_CHUNK_SIZE) { - printf("%s: %s is too short, have only %zu bytes\n", - sc->sc_dev.dv_xname, amss_path, len); - free(data, M_DEVBUF, len); - return EINVAL; + if (len < MHI_DMA_VEC_CHUNK_SIZE) { + printf("%s: %s is too short, have only %zu bytes\n", + sc->sc_dev.dv_xname, amss_path, len); + free(data, M_DEVBUF, len); + return EINVAL; + } + + sc->fw_img[QWX_FW_AMSS].data = data; + sc->fw_img[QWX_FW_AMSS].size = len; } /* Second-stage boot loader sits in the first 512 KB of image. */ @@ -3056,7 +3064,6 @@ qwx_mhi_fw_load_handler(struct qwx_pci_softc *psc) if (ret != 0) { printf("%s: could not load firmware %s\n", sc->sc_dev.dv_xname, amss_path); - free(data, M_DEVBUF, len); return ret; } @@ -3065,6 +3072,7 @@ qwx_mhi_fw_load_handler(struct qwx_pci_softc *psc) if (ret != 0) { printf("%s: could not load firmware %s\n", sc->sc_dev.dv_xname, amss_path); + return ret; } while (psc->bhi_ee < MHI_EE_AMSS) { @@ -3076,11 +3084,8 @@ qwx_mhi_fw_load_handler(struct qwx_pci_softc *psc) if (ret != 0) { printf("%s: device failed to enter AMSS EE\n", sc->sc_dev.dv_xname); - free(data, M_DEVBUF, len); - return ret; } - free(data, M_DEVBUF, len); return ret; }