Date: Tue, 25 Feb 2025 19:35:30 GMT From: Navdeep Parhar <np@FreeBSD.org> To: src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-main@FreeBSD.org Subject: git: e19d84979a18 - main - cxgbe(4): Block most access to the hardware as soon as the adapter stops. Message-ID: <202502251935.51PJZURu029257@gitrepo.freebsd.org>
next in thread | raw e-mail | index | archive | help
The branch main has been updated by np: URL: https://cgit.FreeBSD.org/src/commit/?id=e19d84979a183deb37ce6d7e385c3ccf02a3c8c7 commit e19d84979a183deb37ce6d7e385c3ccf02a3c8c7 Author: Navdeep Parhar <np@FreeBSD.org> AuthorDate: 2025-02-12 00:29:46 +0000 Commit: Navdeep Parhar <np@FreeBSD.org> CommitDate: 2025-02-25 19:33:40 +0000 cxgbe(4): Block most access to the hardware as soon as the adapter stops. Add a new hw_all_ok() routine and use it to avoid hardware access in the public control interfaces (ifnet ioctls, ifmedia calls, etc.). Continue to use hw_off_limits() in the private ioctls/sysctls and other debug code. Retire adapter_stopped() as it's of no use by itself. This fixes problems where ifnet slow-path operations would enter a synch_op just before set_adapter_hwstatus(false) and touch the hardware when it's not safe to do so. MFC after: 1 week Sponsored by: Chelsio Communications --- sys/dev/cxgbe/adapter.h | 10 ++++++---- sys/dev/cxgbe/t4_clip.c | 2 +- sys/dev/cxgbe/t4_filter.c | 8 ++++---- sys/dev/cxgbe/t4_main.c | 40 ++++++++++++++++++++-------------------- sys/dev/cxgbe/tom/t4_tom_l2t.c | 2 +- 5 files changed, 32 insertions(+), 30 deletions(-) diff --git a/sys/dev/cxgbe/adapter.h b/sys/dev/cxgbe/adapter.h index 8d10a07e0933..3bf4f666ce7d 100644 --- a/sys/dev/cxgbe/adapter.h +++ b/sys/dev/cxgbe/adapter.h @@ -1118,7 +1118,7 @@ forwarding_intr_to_fwq(struct adapter *sc) return (sc->intr_count == 1); } -/* Works reliably inside a sync_op or with reg_lock held. */ +/* Works reliably inside a synch_op or with reg_lock held. */ static inline bool hw_off_limits(struct adapter *sc) { @@ -1127,12 +1127,14 @@ hw_off_limits(struct adapter *sc) return (__predict_false(off_limits != 0)); } +/* Works reliably inside a synch_op or with reg_lock held. */ static inline bool -adapter_stopped(struct adapter *sc) +hw_all_ok(struct adapter *sc) { - const int stopped = atomic_load_int(&sc->error_flags) & ADAP_STOPPED; + const int not_ok = atomic_load_int(&sc->error_flags) & + (ADAP_STOPPED | HW_OFF_LIMITS); - return (__predict_false(stopped != 0)); + return (__predict_true(not_ok == 0)); } static inline int diff --git a/sys/dev/cxgbe/t4_clip.c b/sys/dev/cxgbe/t4_clip.c index 24f049f9dc06..e462a064847f 100644 --- a/sys/dev/cxgbe/t4_clip.c +++ b/sys/dev/cxgbe/t4_clip.c @@ -576,7 +576,7 @@ update_hw_clip_table(struct adapter *sc) rc = begin_synchronized_op(sc, NULL, HOLD_LOCK, "t4clip"); if (rc != 0) return (rc); - if (hw_off_limits(sc)) + if (!hw_all_ok(sc)) goto done; /* with rc = 0, we don't want to reschedule. */ while (!TAILQ_EMPTY(&sc->clip_pending)) { ce = TAILQ_FIRST(&sc->clip_pending); diff --git a/sys/dev/cxgbe/t4_filter.c b/sys/dev/cxgbe/t4_filter.c index 359aae6df24e..8d4552116d96 100644 --- a/sys/dev/cxgbe/t4_filter.c +++ b/sys/dev/cxgbe/t4_filter.c @@ -520,7 +520,7 @@ set_filter_mode(struct adapter *sc, uint32_t mode) if (rc) return (rc); - if (hw_off_limits(sc)) { + if (!hw_all_ok(sc)) { rc = ENXIO; goto done; } @@ -571,7 +571,7 @@ set_filter_mask(struct adapter *sc, uint32_t mode) if (rc) return (rc); - if (hw_off_limits(sc)) { + if (!hw_all_ok(sc)) { rc = ENXIO; goto done; } @@ -602,7 +602,7 @@ get_filter_hits(struct adapter *sc, uint32_t tid) tcb_addr = t4_read_reg(sc, A_TP_CMM_TCB_BASE) + tid * TCB_SIZE; mtx_lock(&sc->reg_lock); - if (hw_off_limits(sc)) + if (!hw_all_ok(sc)) hits = 0; else if (is_t4(sc)) { uint64_t t; @@ -976,7 +976,7 @@ set_filter(struct adapter *sc, struct t4_filter *t) if (rc) return (rc); - if (hw_off_limits(sc)) { + if (!hw_all_ok(sc)) { rc = ENXIO; goto done; } diff --git a/sys/dev/cxgbe/t4_main.c b/sys/dev/cxgbe/t4_main.c index 51ba6d94b5fa..6ee839151db0 100644 --- a/sys/dev/cxgbe/t4_main.c +++ b/sys/dev/cxgbe/t4_main.c @@ -1133,7 +1133,7 @@ t4_calibration(void *arg) sc = (struct adapter *)arg; - KASSERT((hw_off_limits(sc) == 0), ("hw_off_limits at t4_calibration")); + KASSERT(hw_all_ok(sc), ("!hw_all_ok at t4_calibration")); hw = t4_read_reg64(sc, A_SGE_TIMESTAMP_LO); sbt = sbinuptime(); @@ -2864,7 +2864,7 @@ cxgbe_ioctl(if_t ifp, unsigned long cmd, caddr_t data) if_setmtu(ifp, mtu); if (vi->flags & VI_INIT_DONE) { t4_update_fl_bufsize(ifp); - if (!hw_off_limits(sc) && + if (hw_all_ok(sc) && if_getdrvflags(ifp) & IFF_DRV_RUNNING) rc = update_mac_settings(ifp, XGMAC_MTU); } @@ -2876,7 +2876,7 @@ cxgbe_ioctl(if_t ifp, unsigned long cmd, caddr_t data) if (rc) return (rc); - if (hw_off_limits(sc)) { + if (!hw_all_ok(sc)) { rc = ENXIO; goto fail; } @@ -2904,7 +2904,7 @@ cxgbe_ioctl(if_t ifp, unsigned long cmd, caddr_t data) rc = begin_synchronized_op(sc, vi, SLEEP_OK | INTR_OK, "t4multi"); if (rc) return (rc); - if (!hw_off_limits(sc) && if_getdrvflags(ifp) & IFF_DRV_RUNNING) + if (hw_all_ok(sc) && if_getdrvflags(ifp) & IFF_DRV_RUNNING) rc = update_mac_settings(ifp, XGMAC_MCADDRS); end_synchronized_op(sc, 0); break; @@ -3079,7 +3079,7 @@ fail: rc = begin_synchronized_op(sc, vi, SLEEP_OK | INTR_OK, "t4i2c"); if (rc) return (rc); - if (hw_off_limits(sc)) + if (!hw_all_ok(sc)) rc = ENXIO; else rc = -t4_i2c_rd(sc, sc->mbox, pi->port_id, i2c.dev_addr, @@ -3356,7 +3356,7 @@ cxgbe_media_change(if_t ifp) if (IFM_OPTIONS(ifm->ifm_media) & IFM_ETH_TXPAUSE) lc->requested_fc |= PAUSE_TX; } - if (pi->up_vis > 0 && !hw_off_limits(sc)) { + if (pi->up_vis > 0 && hw_all_ok(sc)) { fixup_link_config(pi); rc = apply_link_config(pi); } @@ -3524,7 +3524,7 @@ cxgbe_media_status(if_t ifp, struct ifmediareq *ifmr) return; PORT_LOCK(pi); - if (pi->up_vis == 0 && !hw_off_limits(sc)) { + if (pi->up_vis == 0 && hw_all_ok(sc)) { /* * If all the interfaces are administratively down the firmware * does not report transceiver changes. Refresh port info here @@ -8272,7 +8272,7 @@ sysctl_btphy(SYSCTL_HANDLER_ARGS) rc = begin_synchronized_op(sc, &pi->vi[0], SLEEP_OK | INTR_OK, "t4btt"); if (rc) return (rc); - if (hw_off_limits(sc)) + if (!hw_all_ok(sc)) rc = ENXIO; else { /* XXX: magic numbers */ @@ -8329,7 +8329,7 @@ sysctl_tx_vm_wr(SYSCTL_HANDLER_ARGS) "t4txvm"); if (rc) return (rc); - if (hw_off_limits(sc)) + if (!hw_all_ok(sc)) rc = ENXIO; else if (if_getdrvflags(vi->ifp) & IFF_DRV_RUNNING) { /* @@ -8543,7 +8543,7 @@ sysctl_pause_settings(SYSCTL_HANDLER_ARGS) "t4PAUSE"); if (rc) return (rc); - if (!hw_off_limits(sc)) { + if (hw_all_ok(sc)) { PORT_LOCK(pi); lc->requested_fc = n; fixup_link_config(pi); @@ -8639,7 +8639,7 @@ sysctl_requested_fec(SYSCTL_HANDLER_ARGS) lc->requested_fec = n & (M_FW_PORT_CAP32_FEC | FEC_MODULE); } - if (!hw_off_limits(sc)) { + if (hw_all_ok(sc)) { fixup_link_config(pi); if (pi->up_vis > 0) { rc = apply_link_config(pi); @@ -8677,7 +8677,7 @@ sysctl_module_fec(SYSCTL_HANDLER_ARGS) rc = EBUSY; goto done; } - if (hw_off_limits(sc)) { + if (!hw_all_ok(sc)) { rc = ENXIO; goto done; } @@ -8743,7 +8743,7 @@ sysctl_autoneg(SYSCTL_HANDLER_ARGS) goto done; } lc->requested_aneg = val; - if (!hw_off_limits(sc)) { + if (hw_all_ok(sc)) { fixup_link_config(pi); if (pi->up_vis > 0) rc = apply_link_config(pi); @@ -8778,7 +8778,7 @@ sysctl_force_fec(SYSCTL_HANDLER_ARGS) return (rc); PORT_LOCK(pi); lc->force_fec = val; - if (!hw_off_limits(sc)) { + if (hw_all_ok(sc)) { fixup_link_config(pi); if (pi->up_vis > 0) rc = apply_link_config(pi); @@ -8818,7 +8818,7 @@ sysctl_temperature(SYSCTL_HANDLER_ARGS) rc = begin_synchronized_op(sc, NULL, SLEEP_OK | INTR_OK, "t4temp"); if (rc) return (rc); - if (hw_off_limits(sc)) + if (!hw_all_ok(sc)) rc = ENXIO; else { param = V_FW_PARAMS_MNEM(FW_PARAMS_MNEM_DEV) | @@ -8849,7 +8849,7 @@ sysctl_vdd(SYSCTL_HANDLER_ARGS) "t4vdd"); if (rc) return (rc); - if (hw_off_limits(sc)) + if (!hw_all_ok(sc)) rc = ENXIO; else { param = V_FW_PARAMS_MNEM(FW_PARAMS_MNEM_DEV) | @@ -8886,7 +8886,7 @@ sysctl_reset_sensor(SYSCTL_HANDLER_ARGS) rc = begin_synchronized_op(sc, NULL, SLEEP_OK | INTR_OK, "t4srst"); if (rc) return (rc); - if (hw_off_limits(sc)) + if (!hw_all_ok(sc)) rc = ENXIO; else { param = (V_FW_PARAMS_MNEM(FW_PARAMS_MNEM_DEV) | @@ -8912,7 +8912,7 @@ sysctl_loadavg(SYSCTL_HANDLER_ARGS) rc = begin_synchronized_op(sc, NULL, SLEEP_OK | INTR_OK, "t4lavg"); if (rc) return (rc); - if (hw_off_limits(sc)) + if (hw_all_ok(sc)) rc = ENXIO; else { param = V_FW_PARAMS_MNEM(FW_PARAMS_MNEM_DEV) | @@ -12366,7 +12366,7 @@ toe_capability(struct vi_info *vi, bool enable) if (!is_offload(sc)) return (ENODEV); - if (hw_off_limits(sc)) + if (!hw_all_ok(sc)) return (ENXIO); if (enable) { @@ -12629,7 +12629,7 @@ ktls_capability(struct adapter *sc, bool enable) return (ENODEV); if (!is_t6(sc)) return (0); - if (hw_off_limits(sc)) + if (!hw_all_ok(sc)) return (ENXIO); if (enable) { diff --git a/sys/dev/cxgbe/tom/t4_tom_l2t.c b/sys/dev/cxgbe/tom/t4_tom_l2t.c index 909abe793835..3fd0d5ca41d4 100644 --- a/sys/dev/cxgbe/tom/t4_tom_l2t.c +++ b/sys/dev/cxgbe/tom/t4_tom_l2t.c @@ -289,7 +289,7 @@ again: mtx_unlock(&e->lock); goto again; } - if (adapter_stopped(sc)) + if (!hw_all_ok(sc)) free(wr, M_CXGBE); else arpq_enqueue(e, wr);
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?202502251935.51PJZURu029257>