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

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);
 }