git: 557866b899b6 - main - aq(4): harden the attach, detach, and reset error paths
- Go to: [ bottom of page ] [ top of archives ] [ this month ]
Date: Sun, 19 Jul 2026 16:49:13 UTC
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);
}