Date: Sun, 19 Jul 2026 16:49:13 +0000 From: Adrian Chadd <adrian@FreeBSD.org> To: src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-main@FreeBSD.org Cc: Nick Price <nick@spun.io> Subject: git: 557866b899b6 - main - aq(4): harden the attach, detach, and reset error paths Message-ID: <6a5d0009.310d9.4271c4f8@gitrepo.freebsd.org>
index | next in thread | raw e-mail
The branch main has been updated by adrian: URL: https://cgit.FreeBSD.org/src/commit/?id=557866b899b632ca4160953e5430cd9ccba8b570 commit 557866b899b632ca4160953e5430cd9ccba8b570 Author: Nick Price <nick@spun.io> AuthorDate: 2026-07-19 16:41:49 +0000 Commit: Adrian Chadd <adrian@FreeBSD.org> CommitDate: 2026-07-19 16:41:49 +0000 aq(4): harden the attach, detach, and reset error paths Correct several attach/detach/reset paths that either swallowed failures or acted on undefined state. MSI-X attach-failure double-free: aq_if_msix_intr_assign() freed the per-RX-ring interrupts in its failure path and then returned an error, so iflib's IFDI_DETACH freed the same irq structures again -- bus_teardown_intr() on a dangling tag and bus_release_resource() on an already-released IRQ, panicking a box that should have simply failed to attach. Let iflib own the teardown; drop the failure-path loop and the now-dead index bookkeeping. Detach loop bound: aq_if_detach() freed the per-ring interrupts looping to isc_nrxqsets while indexing rx_rings[], which is sized by rx_rings_count; index by rx_rings_count to match every other RX-ring loop. AQ_HW_WAIT_FOR final poll: the macro derived its result from the loop counter rather than the condition, so a condition that became true on the last iteration reported ETIMEDOUT. Worst for the acquire-on-read firmware RAM semaphore, which was acquired in hardware but reported as a timeout. Return based on the last evaluation of the condition. RBL MAC reset SPI cleanup: mac_soft_reset_rbl() fired the global reset without first tearing down the SPI/flash interface, so a flash burst in flight left the SPI bus wedged, the RBL could not re-read flash, and the reset returned EBUSY -- fatal at attach ("MAC reset failed: 16"). Set bit 4 of the SPI control register (0x53c) before the global reset, as the sibling FLB path and the Linux driver do. Reset failure propagation: aq_hw_reset() discarded fw_ops->reset()'s return, so a failed attach-time fw2x capability read left fw_caps == 0 permanently and stats silently froze. Propagate the error so the reset fails and is retried. aq_hw_init failure propagation: aq_hw_init() discarded aq_hw_init_tx_path()/aq_hw_init_rx_path() returns and reported success, bringing the interface up half-initialized; capture both and goto err_exit (mainly the Atlantic 2 RX action-resolver path, which returns EBUSY on ART semaphore timeout). Link-state outputs: aq_hw_get_link_state() left *link_speed and *fc_neg unwritten on early-return paths, and the caller acts on them uninitialized, so a transient firmware get_mode() failure could fabricate a phantom link-up at a garbage speed and program a garbage RX-pause bit. Initialize both to safe link-down values before calling get_mode(). Reviewed by: adrian Differential Revision: https://reviews.freebsd.org/D58138 --- sys/dev/aq/aq_common.h | 5 +++-- sys/dev/aq/aq_fw.c | 4 ++++ sys/dev/aq/aq_hw.c | 17 +++++++++++++---- sys/dev/aq/aq_main.c | 17 ++++++++++------- 4 files changed, 30 insertions(+), 13 deletions(-) diff --git a/sys/dev/aq/aq_common.h b/sys/dev/aq/aq_common.h index 5c497d071802..7fa245caeebb 100644 --- a/sys/dev/aq/aq_common.h +++ b/sys/dev/aq/aq_common.h @@ -42,9 +42,10 @@ #define AQ_HW_WAIT_FOR(_B_, _US_, _N_) ({ \ unsigned int _i; \ - for (_i = (_N_); !(_B_) && _i; --_i) \ + int _b; \ + for (_i = (_N_); !(_b = (_B_)) && _i; --_i) \ DELAY(_US_); \ - (_i == 0) ? ETIMEDOUT : 0; \ + _b ? 0 : ETIMEDOUT; \ }) diff --git a/sys/dev/aq/aq_fw.c b/sys/dev/aq/aq_fw.c index 72751c81b773..f2f75dd7d3de 100644 --- a/sys/dev/aq/aq_fw.c +++ b/sys/dev/aq/aq_fw.c @@ -306,6 +306,10 @@ mac_soft_reset_rbl(struct aq_hw* hw, enum aq_fw_bootloader_mode* mode) reg_glb_cpu_no_reset_scratchpad_set(hw, 0xDEAD, NO_RESET_SCRATCHPAD_RBL_STATUS); + /* Clear a stale flash burst so the global reset frees the SPI bus. */ + uint32_t spi = AQ_READ_REG(hw, 0x53c); + AQ_WRITE_REG(hw, 0x53c, spi | 0x10); + // Global software reset rx_rx_reg_res_dis_set(hw, 0); tx_tx_reg_res_dis_set(hw, 0); diff --git a/sys/dev/aq/aq_hw.c b/sys/dev/aq/aq_hw.c index e610f8b27ba7..7fc122482125 100644 --- a/sys/dev/aq/aq_hw.c +++ b/sys/dev/aq/aq_hw.c @@ -253,6 +253,10 @@ aq_hw_get_link_state(struct aq_hw *hw, uint32_t *link_speed, struct aq_hw_fc_inf enum aq_fw_link_speed speed = aq_fw_none; enum aq_fw_link_fc fc; + *link_speed = 0; + fc_neg->fc_rx = false; + fc_neg->fc_tx = false; + err = hw->fw_ops->get_mode(hw, &mode, &speed, &fc); if (err != 0) { @@ -260,7 +264,6 @@ aq_hw_get_link_state(struct aq_hw *hw, uint32_t *link_speed, struct aq_hw_fc_inf AQ_DBG_EXIT(err); return (err); } - *link_speed = 0; if (mode != MPI_INIT) return (0); @@ -376,7 +379,9 @@ aq_hw_reset(struct aq_hw *hw) goto err_exit; } - hw->fw_ops->reset(hw); + err = hw->fw_ops->reset(hw); + if (err != 0) + goto err_exit; err = aq_hw_err_from_flags(hw); @@ -644,8 +649,12 @@ aq_hw_init(struct aq_hw *hw, uint8_t *mac_addr, uint8_t adm_irq, bool msix) */ AQ_WRITE_REG(hw, AQ_HW_TX_DMA_TOTAL_REQ_LIMIT_ADR, 24); - aq_hw_init_tx_path(hw); - aq_hw_init_rx_path(hw); + err = aq_hw_init_tx_path(hw); + if (err != 0) + goto err_exit; + err = aq_hw_init_rx_path(hw); + if (err != 0) + goto err_exit; aq_hw_mac_addr_set(hw, mac_addr, AQ_HW_MAC); diff --git a/sys/dev/aq/aq_main.c b/sys/dev/aq/aq_main.c index 4c06447b55d3..c77575273e92 100644 --- a/sys/dev/aq/aq_main.c +++ b/sys/dev/aq/aq_main.c @@ -360,9 +360,14 @@ aq_if_attach_pre(if_ctx_t ctx) } if (hw->fast_start_enabled) - hw->fw_ops->reset(hw); + rc = hw->fw_ops->reset(hw); else - aq_hw_reset(&softc->hw); + rc = aq_hw_reset(&softc->hw); + if (rc != 0) { + device_printf(softc->dev, "%s: reset failed, err=%d\n", + __func__, rc); + goto fail; + } aq_hw_capabilities(softc); rc = aq_hw_get_mac_permanent(hw, hw->mac_addr); @@ -480,7 +485,7 @@ aq_if_detach(if_ctx_t ctx) aq_hw_deinit(&softc->hw); - for (i = 0; i < softc->scctx->isc_nrxqsets; i++) + for (i = 0; i < softc->rx_rings_count; i++) iflib_irq_free(ctx, &softc->rx_rings[i]->irq); iflib_irq_free(ctx, &softc->irq); @@ -707,6 +712,8 @@ aq_if_init(if_ctx_t ctx) softc->scctx->isc_intr == IFLIB_INTR_MSIX); if (err != 0) { device_printf(softc->dev, "atlantic: aq_hw_init: %d", err); + AQ_DBG_EXIT(err); + return; } aq_if_media_status(ctx, &ifmr); @@ -1029,7 +1036,6 @@ aq_if_msix_intr_assign(if_ctx_t ctx, int msix) if (rc) { device_printf(softc->dev, "failed to set up RX handler\n"); - i--; goto fail; } @@ -1057,15 +1063,12 @@ aq_if_msix_intr_assign(if_ctx_t ctx, int msix) if (rc) { device_printf(iflib_get_dev(ctx), "Failed to register admin handler"); - i = softc->rx_rings_count - 1; goto fail; } AQ_DBG_EXIT(0); return (0); fail: - for (; i >= 0; i--) - iflib_irq_free(ctx, &softc->rx_rings[i]->irq); AQ_DBG_EXIT(rc); return (rc); }home | help
Want to link to this message? Use this
URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?6a5d0009.310d9.4271c4f8>