From 649e8ad140ebdbacfad649bd3ac6b78d954c37d5 Mon Sep 17 00:00:00 2001 From: erj Date: Thu, 31 Jan 2019 21:44:33 +0000 Subject: [PATCH] ix(4): Run {mod,msf,mbx,fdir,phy}_task in if_update_admin_status From Piotr: This patch introduces adapter->task_requests register responsible for recording requests for mod_task, msf_task, mbx_task, fdir_task and phy_task calls. Instead of enqueueing these tasks with GROUPTASK_ENQUEUE, handlers will be called directly from ixgbe_if_update_admin_status() while holding ctx lock. SIOCGIFXMEDIA ioctl() call reads adapter->media list. The list is deleted and rewritten in ixgbe_handle_msf() task without holding ctx lock. This change is needed to maintain data coherency when sharing adapter info via ioctl() calls. Patch co-authored by Krzysztof Galazka . PR: 221317 Submitted by: Piotr Pietruszewski Reviewed by: sbruno@, IntelNetworking Sponsored by: Intel Corporation Differential Revision: https://reviews.freebsd.org/D18468 --- sys/dev/ixgbe/if_ix.c | 104 ++++++++++++++++++------------------- sys/dev/ixgbe/ixgbe.h | 7 +-- sys/dev/ixgbe/ixgbe_type.h | 7 +++ 3 files changed, 59 insertions(+), 59 deletions(-) diff --git a/sys/dev/ixgbe/if_ix.c b/sys/dev/ixgbe/if_ix.c index ed93bf33661a..7f7902e29fdc 100644 --- a/sys/dev/ixgbe/if_ix.c +++ b/sys/dev/ixgbe/if_ix.c @@ -120,6 +120,7 @@ static int ixgbe_if_resume(if_ctx_t ctx); static void ixgbe_if_stop(if_ctx_t ctx); void ixgbe_if_enable_intr(if_ctx_t ctx); static void ixgbe_if_disable_intr(if_ctx_t ctx); +static void ixgbe_link_intr_enable(if_ctx_t ctx); static int ixgbe_if_rx_queue_intr_enable(if_ctx_t ctx, uint16_t qid); static void ixgbe_if_media_status(if_ctx_t ctx, struct ifmediareq * ifmr); static int ixgbe_if_media_change(if_ctx_t ctx); @@ -173,7 +174,7 @@ static void ixgbe_init_device_features(struct adapter *adapter); static void ixgbe_check_fan_failure(struct adapter *, u32, bool); static void ixgbe_add_media_types(if_ctx_t ctx); static void ixgbe_update_stats_counters(struct adapter *adapter); -static void ixgbe_config_link(struct adapter *adapter); +static void ixgbe_config_link(if_ctx_t ctx); static void ixgbe_get_slot_info(struct adapter *); static void ixgbe_check_wol_support(struct adapter *adapter); static void ixgbe_enable_rx_drop(struct adapter *); @@ -254,6 +255,7 @@ static device_method_t ixgbe_if_methods[] = { DEVMETHOD(ifdi_msix_intr_assign, ixgbe_if_msix_intr_assign), DEVMETHOD(ifdi_intr_enable, ixgbe_if_enable_intr), DEVMETHOD(ifdi_intr_disable, ixgbe_if_disable_intr), + DEVMETHOD(ifdi_link_intr_enable, ixgbe_link_intr_enable), DEVMETHOD(ifdi_tx_queue_intr_enable, ixgbe_if_rx_queue_intr_enable), DEVMETHOD(ifdi_rx_queue_intr_enable, ixgbe_if_rx_queue_intr_enable), DEVMETHOD(ifdi_tx_queues_alloc, ixgbe_if_tx_queues_alloc), @@ -446,19 +448,6 @@ ixgbe_if_tx_queues_alloc(if_ctx_t ctx, caddr_t *vaddrs, uint64_t *paddrs, } - iflib_config_gtask_init(ctx, &adapter->mod_task, ixgbe_handle_mod, - "mod_task"); - iflib_config_gtask_init(ctx, &adapter->msf_task, ixgbe_handle_msf, - "msf_task"); - iflib_config_gtask_init(ctx, &adapter->phy_task, ixgbe_handle_phy, - "phy_task"); - if (adapter->feat_cap & IXGBE_FEATURE_SRIOV) - iflib_config_gtask_init(ctx, &adapter->mbx_task, - ixgbe_handle_mbx, "mbx_task"); - if (adapter->feat_en & IXGBE_FEATURE_FDIR) - iflib_config_gtask_init(ctx, &adapter->fdir_task, - ixgbe_reinit_fdir, "fdir_task"); - device_printf(iflib_get_dev(ctx), "allocated for %d queues\n", adapter->num_tx_queues); @@ -1362,8 +1351,9 @@ ixgbe_is_sfp(struct ixgbe_hw *hw) * ixgbe_config_link ************************************************************************/ static void -ixgbe_config_link(struct adapter *adapter) +ixgbe_config_link(if_ctx_t ctx) { + struct adapter *adapter = iflib_get_softc(ctx); struct ixgbe_hw *hw = &adapter->hw; u32 autoneg, err = 0; bool sfp, negotiate; @@ -1371,7 +1361,8 @@ ixgbe_config_link(struct adapter *adapter) sfp = ixgbe_is_sfp(hw); if (sfp) { - GROUPTASK_ENQUEUE(&adapter->mod_task); + adapter->task_requests |= IXGBE_REQUEST_TASK_MOD; + iflib_admin_intr_deferred(ctx); } else { if (hw->mac.ops.check_link) err = ixgbe_check_link(hw, &adapter->link_speed, @@ -1388,7 +1379,6 @@ ixgbe_config_link(struct adapter *adapter) err = hw->mac.ops.setup_link(hw, autoneg, adapter->link_up); } - } /* ixgbe_config_link */ /************************************************************************ @@ -2096,8 +2086,6 @@ ixgbe_if_media_status(if_ctx_t ctx, struct ifmediareq * ifmr) INIT_DEBUGOUT("ixgbe_if_media_status: begin"); - iflib_admin_intr_deferred(ctx); - ifmr->ifm_status = IFM_AVALID; ifmr->ifm_active = IFM_ETHER; @@ -2386,7 +2374,7 @@ ixgbe_msix_link(void *arg) /* Link status change */ if (eicr & IXGBE_EICR_LSC) { IXGBE_WRITE_REG(hw, IXGBE_EIMC, IXGBE_EIMC_LSC); - iflib_admin_intr_deferred(adapter->ctx); + adapter->task_requests |= IXGBE_REQUEST_TASK_LSC; } if (adapter->hw.mac.type != ixgbe_mac_82598EB) { @@ -2397,7 +2385,7 @@ ixgbe_msix_link(void *arg) return (FILTER_HANDLED); /* Disable the interrupt */ IXGBE_WRITE_REG(hw, IXGBE_EIMC, IXGBE_EICR_FLOW_DIR); - GROUPTASK_ENQUEUE(&adapter->fdir_task); + adapter->task_requests |= IXGBE_REQUEST_TASK_FDIR; } else if (eicr & IXGBE_EICR_ECC) { device_printf(iflib_get_dev(adapter->ctx), @@ -2441,7 +2429,7 @@ ixgbe_msix_link(void *arg) /* Check for VF message */ if ((adapter->feat_en & IXGBE_FEATURE_SRIOV) && (eicr & IXGBE_EICR_MAILBOX)) - GROUPTASK_ENQUEUE(&adapter->mbx_task); + adapter->task_requests |= IXGBE_REQUEST_TASK_MBX; } if (ixgbe_is_sfp(hw)) { @@ -2453,16 +2441,14 @@ ixgbe_msix_link(void *arg) if (eicr & eicr_mask) { IXGBE_WRITE_REG(hw, IXGBE_EICR, eicr_mask); - if (atomic_cmpset_acq_int(&adapter->sfp_reinit, 0, 1)) - GROUPTASK_ENQUEUE(&adapter->mod_task); + adapter->task_requests |= IXGBE_REQUEST_TASK_MOD; } if ((hw->mac.type == ixgbe_mac_82599EB) && (eicr & IXGBE_EICR_GPI_SDP1_BY_MAC(hw))) { IXGBE_WRITE_REG(hw, IXGBE_EICR, IXGBE_EICR_GPI_SDP1_BY_MAC(hw)); - if (atomic_cmpset_acq_int(&adapter->sfp_reinit, 0, 1)) - GROUPTASK_ENQUEUE(&adapter->msf_task); + adapter->task_requests |= IXGBE_REQUEST_TASK_MSF; } } @@ -2476,13 +2462,10 @@ ixgbe_msix_link(void *arg) if ((hw->phy.type == ixgbe_phy_x550em_ext_t) && (eicr & IXGBE_EICR_GPI_SDP0_X540)) { IXGBE_WRITE_REG(hw, IXGBE_EICR, IXGBE_EICR_GPI_SDP0_X540); - GROUPTASK_ENQUEUE(&adapter->phy_task); + adapter->task_requests |= IXGBE_REQUEST_TASK_PHY; } - /* Re-enable other interrupts */ - IXGBE_WRITE_REG(hw, IXGBE_EIMS, IXGBE_EIMS_OTHER); - - return (FILTER_HANDLED); + return (adapter->task_requests != 0) ? FILTER_SCHEDULE_THREAD : FILTER_HANDLED; } /* ixgbe_msix_link */ /************************************************************************ @@ -2646,12 +2629,6 @@ ixgbe_if_detach(if_ctx_t ctx) return (EBUSY); } - iflib_config_gtask_deinit(&adapter->mod_task); - iflib_config_gtask_deinit(&adapter->msf_task); - iflib_config_gtask_deinit(&adapter->phy_task); - if (adapter->feat_cap & IXGBE_FEATURE_SRIOV) - iflib_config_gtask_deinit(&adapter->mbx_task); - ixgbe_setup_low_power_mode(ctx); /* let hardware know driver is unloading */ @@ -2910,6 +2887,12 @@ ixgbe_if_init(if_ctx_t ctx) /* Configure RX settings */ ixgbe_initialize_receive_units(ctx); + /* + * Initialize variable holding task enqueue requests + * from MSI-X interrupts + */ + adapter->task_requests = 0; + /* Enable SDP & MSI-X interrupts based on adapter */ ixgbe_config_gpie(adapter); @@ -3011,7 +2994,7 @@ ixgbe_if_init(if_ctx_t ctx) ixgbe_set_phy_power(hw, TRUE); /* Config/Enable Link */ - ixgbe_config_link(adapter); + ixgbe_config_link(ctx); /* Hardware Packet Buffer & Flow Control setup */ ixgbe_config_delay_values(adapter); @@ -3374,7 +3357,6 @@ ixgbe_handle_mod(void *context) device_t dev = iflib_get_dev(ctx); u32 err, cage_full = 0; - adapter->sfp_reinit = 1; if (adapter->hw.need_crosstalk_fix) { switch (hw->mac.type) { case ixgbe_mac_82599EB: @@ -3411,11 +3393,11 @@ ixgbe_handle_mod(void *context) "Setup failure - unsupported SFP+ module type.\n"); goto handle_mod_out; } - GROUPTASK_ENQUEUE(&adapter->msf_task); + adapter->task_requests |= IXGBE_REQUEST_TASK_MSF; return; handle_mod_out: - adapter->sfp_reinit = 0; + adapter->task_requests &= ~(IXGBE_REQUEST_TASK_MSF); } /* ixgbe_handle_mod */ @@ -3431,9 +3413,6 @@ ixgbe_handle_msf(void *context) u32 autoneg; bool negotiate; - if (adapter->sfp_reinit != 1) - return; - /* get_supported_phy_layer will call hw->phy.ops.identify_sfp() */ adapter->phy_layer = ixgbe_get_supported_physical_layer(hw); @@ -3447,8 +3426,6 @@ ixgbe_handle_msf(void *context) ifmedia_removeall(adapter->media); ixgbe_add_media_types(adapter->ctx); ifmedia_set(adapter->media, IFM_ETHER | IFM_AUTO); - - adapter->sfp_reinit = 0; } /* ixgbe_handle_msf */ /************************************************************************ @@ -3543,10 +3520,20 @@ ixgbe_if_update_admin_status(if_ctx_t ctx) } } - ixgbe_update_stats_counters(adapter); + /* Handle task requests from msix_link() */ + if (adapter->task_requests & IXGBE_REQUEST_TASK_MOD) + ixgbe_handle_mod(ctx); + if (adapter->task_requests & IXGBE_REQUEST_TASK_MSF) + ixgbe_handle_msf(ctx); + if (adapter->task_requests & IXGBE_REQUEST_TASK_MBX) + ixgbe_handle_mbx(ctx); + if (adapter->task_requests & IXGBE_REQUEST_TASK_FDIR) + ixgbe_reinit_fdir(ctx); + if (adapter->task_requests & IXGBE_REQUEST_TASK_PHY) + ixgbe_handle_phy(ctx); + adapter->task_requests = 0; - /* Re-enable link interrupts */ - IXGBE_WRITE_REG(&adapter->hw, IXGBE_EIMS, IXGBE_EIMS_LSC); + ixgbe_update_stats_counters(adapter); } /* ixgbe_if_update_admin_status */ /************************************************************************ @@ -3681,6 +3668,18 @@ ixgbe_if_disable_intr(if_ctx_t ctx) } /* ixgbe_if_disable_intr */ +/************************************************************************ + * ixgbe_link_intr_enable + ************************************************************************/ +static void +ixgbe_link_intr_enable(if_ctx_t ctx) +{ + struct ixgbe_hw *hw = &((struct adapter *)iflib_get_softc(ctx))->hw; + + /* Re-enable other interrupts */ + IXGBE_WRITE_REG(hw, IXGBE_EIMS, IXGBE_EIMS_OTHER | IXGBE_EIMS_LSC); +} /* ixgbe_link_intr_enable */ + /************************************************************************ * ixgbe_if_rx_queue_intr_enable ************************************************************************/ @@ -3784,22 +3783,21 @@ ixgbe_intr(void *arg) if (eicr & eicr_mask) { IXGBE_WRITE_REG(hw, IXGBE_EICR, eicr_mask); - GROUPTASK_ENQUEUE(&adapter->mod_task); + adapter->task_requests |= IXGBE_REQUEST_TASK_MOD; } if ((hw->mac.type == ixgbe_mac_82599EB) && (eicr & IXGBE_EICR_GPI_SDP1_BY_MAC(hw))) { IXGBE_WRITE_REG(hw, IXGBE_EICR, IXGBE_EICR_GPI_SDP1_BY_MAC(hw)); - if (atomic_cmpset_acq_int(&adapter->sfp_reinit, 0, 1)) - GROUPTASK_ENQUEUE(&adapter->msf_task); + adapter->task_requests |= IXGBE_REQUEST_TASK_MSF; } } /* External PHY interrupt */ if ((hw->phy.type == ixgbe_phy_x550em_ext_t) && (eicr & IXGBE_EICR_GPI_SDP0_X540)) - GROUPTASK_ENQUEUE(&adapter->phy_task); + adapter->task_requests |= IXGBE_REQUEST_TASK_PHY; return (FILTER_SCHEDULE_THREAD); } /* ixgbe_intr */ diff --git a/sys/dev/ixgbe/ixgbe.h b/sys/dev/ixgbe/ixgbe.h index 714c740a5ba5..afc55dccd4ef 100644 --- a/sys/dev/ixgbe/ixgbe.h +++ b/sys/dev/ixgbe/ixgbe.h @@ -428,16 +428,11 @@ struct adapter { /* Support for pluggable optics */ bool sfp_probe; - struct grouptask mod_task; /* SFP tasklet */ - struct grouptask msf_task; /* Multispeed Fiber */ - struct grouptask mbx_task; /* VF -> PF mailbox interrupt */ - int sfp_reinit; /* Flow Director */ int fdir_reinit; - struct grouptask fdir_task; - struct grouptask phy_task; /* PHY intr tasklet */ + u32 task_requests; /* * Queues: diff --git a/sys/dev/ixgbe/ixgbe_type.h b/sys/dev/ixgbe/ixgbe_type.h index 36101dac2961..fc5f191ee65e 100644 --- a/sys/dev/ixgbe/ixgbe_type.h +++ b/sys/dev/ixgbe/ixgbe_type.h @@ -4427,4 +4427,11 @@ struct ixgbe_bypass_eeprom { #define IXGBE_NW_MNG_IF_SEL_MDIO_PHY_ADD \ (0x1F << IXGBE_NW_MNG_IF_SEL_MDIO_PHY_ADD_SHIFT) +#define IXGBE_REQUEST_TASK_MOD 0x01 +#define IXGBE_REQUEST_TASK_MSF 0x02 +#define IXGBE_REQUEST_TASK_MBX 0x04 +#define IXGBE_REQUEST_TASK_FDIR 0x08 +#define IXGBE_REQUEST_TASK_PHY 0x10 +#define IXGBE_REQUEST_TASK_LSC 0x20 + #endif /* _IXGBE_TYPE_H_ */