Changeset View
Standalone View
sys/dev/e1000/if_em.c
Show First 20 Lines • Show All 1,498 Lines • ▼ Show 20 Lines | |||||
* MSI-X Link Fast Interrupt Service routine | * MSI-X Link Fast Interrupt Service routine | ||||
* | * | ||||
**********************************************************************/ | **********************************************************************/ | ||||
static int | static int | ||||
em_msix_link(void *arg) | em_msix_link(void *arg) | ||||
{ | { | ||||
struct adapter *adapter = arg; | struct adapter *adapter = arg; | ||||
u32 reg_icr; | u32 reg_icr; | ||||
bool notlink = false; | |||||
++adapter->link_irq; | ++adapter->link_irq; | ||||
MPASS(adapter->hw.back != NULL); | MPASS(adapter->hw.back != NULL); | ||||
reg_icr = E1000_READ_REG(&adapter->hw, E1000_ICR); | reg_icr = E1000_READ_REG(&adapter->hw, E1000_ICR); | ||||
if (reg_icr & E1000_ICR_RXO) | if (reg_icr & E1000_ICR_RXO) | ||||
adapter->rx_overruns++; | adapter->rx_overruns++; | ||||
if (reg_icr & (E1000_ICR_RXSEQ | E1000_ICR_LSC)) { | if (reg_icr & (E1000_ICR_RXSEQ | E1000_ICR_LSC)) | ||||
em_handle_link(adapter->ctx); | em_handle_link(adapter->ctx); | ||||
} else if (adapter->hw.mac.type == e1000_82574) { | else | ||||
kbowling: @marius I'm not really sure why this would be necessary, under what circumstance will the admin… | |||||
Done Inline Actions@marius I added this back because I'm not sure under what conditions it will fire. RXSEQ above doesn't seem to be a defined bit on 82574, is that related to the issue this works around? kbowling: @marius I added this back because I'm not sure under what conditions it will fire. RXSEQ above… | |||||
/* Only re-arm 82574 if em_if_update_admin_status() won't. */ | notlink = true; | ||||
E1000_WRITE_REG(&adapter->hw, E1000_IMS, EM_MSIX_LINK | | |||||
E1000_IMS_LSC); | |||||
} | |||||
if (adapter->hw.mac.type == e1000_82574) { | /* Re-arm for other/spurious interrupts */ | ||||
if (notlink && adapter->hw.mac.type >= igb_mac_min) { | |||||
E1000_WRITE_REG(&adapter->hw, E1000_IMS, E1000_IMS_LSC); | |||||
E1000_WRITE_REG(&adapter->hw, E1000_EIMS, adapter->link_mask); | |||||
} else if (adapter->hw.mac.type == e1000_82574) { | |||||
if (notlink) | |||||
E1000_WRITE_REG(&adapter->hw, E1000_IMS, E1000_IMS_LSC | | |||||
E1000_IMS_OTHER); | |||||
/* | /* | ||||
* Because we must read the ICR for this interrupt it may | * Because we must read the ICR for this interrupt it may | ||||
* clear other causes using autoclear, for this reason we | * clear other causes using autoclear, for this reason we | ||||
* simply create a soft interrupt for all these vectors. | * simply create a soft interrupt for all these vectors. | ||||
*/ | */ | ||||
if (reg_icr) | if (reg_icr) | ||||
E1000_WRITE_REG(&adapter->hw, E1000_ICS, adapter->ims); | E1000_WRITE_REG(&adapter->hw, E1000_ICS, adapter->ims); | ||||
} else { | |||||
/* Re-arm unconditionally */ | |||||
E1000_WRITE_REG(&adapter->hw, E1000_IMS, E1000_IMS_LSC); | |||||
E1000_WRITE_REG(&adapter->hw, E1000_EIMS, adapter->link_mask); | |||||
Done Inline Actionsem-class devices don't have EIMS kbowling: em-class devices don't have EIMS | |||||
} | } | ||||
Done Inline ActionsI think this is dead code, the only MSIX-capable devices are igb and 82574, and those are handled in the two cases above. markj: I think this is dead code, the only MSIX-capable devices are igb and 82574, and those are… | |||||
return (FILTER_HANDLED); | return (FILTER_HANDLED); | ||||
} | } | ||||
static void | static void | ||||
em_handle_link(void *context) | em_handle_link(void *context) | ||||
{ | { | ||||
if_ctx_t ctx = context; | if_ctx_t ctx = context; | ||||
▲ Show 20 Lines • Show All 327 Lines • ▼ Show 20 Lines | em_if_update_admin_status(if_ctx_t ctx) | ||||
em_update_stats_counters(adapter); | em_update_stats_counters(adapter); | ||||
/* Reset LAA into RAR[0] on 82571 */ | /* Reset LAA into RAR[0] on 82571 */ | ||||
if (hw->mac.type == e1000_82571 && e1000_get_laa_state_82571(hw)) | if (hw->mac.type == e1000_82571 && e1000_get_laa_state_82571(hw)) | ||||
e1000_rar_set(hw, hw->mac.addr, 0); | e1000_rar_set(hw, hw->mac.addr, 0); | ||||
if (hw->mac.type < em_mac_min) | if (hw->mac.type < em_mac_min) | ||||
lem_smartspeed(adapter); | lem_smartspeed(adapter); | ||||
else if (hw->mac.type == e1000_82574 && | else if (hw->mac.type >= igb_mac_min && | ||||
adapter->intr_type == IFLIB_INTR_MSIX) { | |||||
Not Done Inline ActionsThe indentation looks off here. markj: The indentation looks off here. | |||||
E1000_WRITE_REG(&adapter->hw, E1000_IMS, E1000_IMS_LSC); | |||||
E1000_WRITE_REG(&adapter->hw, E1000_EIMS, adapter->link_mask); | |||||
} else if (hw->mac.type == e1000_82574 && | |||||
Done Inline Actionskbowling: @markj @marius for my education, do we defer this re-arm because re-arming in em_msix_link may… | |||||
Done Inline ActionsSo one difference between the iflib'ed em and others (pre-iflib em and intel's em) is that em_msix_link() runs as an interrupt filter rather than a threaded interrupt handler. That being the case, I don't understand why em_msix_link() re-arms the LSC at all. markj: So one difference between the iflib'ed em and others (pre-iflib em and intel's em) is that… | |||||
adapter->intr_type == IFLIB_INTR_MSIX) | adapter->intr_type == IFLIB_INTR_MSIX) | ||||
E1000_WRITE_REG(hw, E1000_IMS, EM_MSIX_LINK | E1000_IMS_LSC); | E1000_WRITE_REG(hw, E1000_IMS, E1000_IMS_LSC | E1000_IMS_OTHER); | ||||
Done Inline ActionsThis is functionally equivalent i.e. #include <stdio.h> #define EM_MSIX_LINK 0x01000000 /* For 82574 use */ int main () { unsigned int x = 0; x |= 1 << 24; if (x == EM_MSIX_LINK) printf("%08x == %08x\n", x, EM_MSIX_LINK); return 0; } 01000000 == 01000000 kbowling: This is functionally equivalent i.e.
```
#include <stdio.h>
#define EM_MSIX_LINK… | |||||
} | } | ||||
static void | static void | ||||
em_if_watchdog_reset(if_ctx_t ctx) | em_if_watchdog_reset(if_ctx_t ctx) | ||||
{ | { | ||||
struct adapter *adapter = iflib_get_softc(ctx); | struct adapter *adapter = iflib_get_softc(ctx); | ||||
/* | /* | ||||
▲ Show 20 Lines • Show All 195 Lines • ▼ Show 20 Lines | em_if_msix_intr_assign(if_ctx_t ctx, int msix) | ||||
if (error) { | if (error) { | ||||
device_printf(iflib_get_dev(ctx), "Failed to register admin handler"); | device_printf(iflib_get_dev(ctx), "Failed to register admin handler"); | ||||
goto fail; | goto fail; | ||||
} | } | ||||
adapter->linkvec = rx_vectors; | adapter->linkvec = rx_vectors; | ||||
if (adapter->hw.mac.type < igb_mac_min) { | if (adapter->hw.mac.type < igb_mac_min) { | ||||
adapter->ivars |= (8 | rx_vectors) << 16; | adapter->ivars |= (8 | rx_vectors) << 16; | ||||
adapter->ivars |= 0x80000000; | adapter->ivars |= 0x80000000; | ||||
/* Enable the "Other" interrupt type for link status change */ | |||||
adapter->ims |= E1000_IMS_OTHER; | |||||
} | } | ||||
return (0); | return (0); | ||||
fail: | fail: | ||||
Done Inline ActionsE1000_IMS_OTHER? markj: `E1000_IMS_OTHER`? | |||||
Done Inline ActionsThanks. I will do some reading tonight and see if EM_MSIX_MASK can go away altogether. kbowling: Thanks. I will do some reading tonight and see if EM_MSIX_MASK can go away altogether. | |||||
iflib_irq_free(ctx, &adapter->irq); | iflib_irq_free(ctx, &adapter->irq); | ||||
rx_que = adapter->rx_queues; | rx_que = adapter->rx_queues; | ||||
for (int i = 0; i < adapter->rx_num_queues; i++, rx_que++) | for (int i = 0; i < adapter->rx_num_queues; i++, rx_que++) | ||||
iflib_irq_free(ctx, &rx_que->que_irq); | iflib_irq_free(ctx, &rx_que->que_irq); | ||||
return (error); | return (error); | ||||
} | } | ||||
static void | static void | ||||
▲ Show 20 Lines • Show All 1,358 Lines • ▼ Show 20 Lines | |||||
static void | static void | ||||
em_if_intr_enable(if_ctx_t ctx) | em_if_intr_enable(if_ctx_t ctx) | ||||
{ | { | ||||
struct adapter *adapter = iflib_get_softc(ctx); | struct adapter *adapter = iflib_get_softc(ctx); | ||||
struct e1000_hw *hw = &adapter->hw; | struct e1000_hw *hw = &adapter->hw; | ||||
u32 ims_mask = IMS_ENABLE_MASK; | u32 ims_mask = IMS_ENABLE_MASK; | ||||
if (hw->mac.type == e1000_82574) { | if (adapter->intr_type == IFLIB_INTR_MSIX) { | ||||
E1000_WRITE_REG(hw, EM_EIAC, EM_MSIX_MASK); | E1000_WRITE_REG(hw, EM_EIAC, adapter->ims); | ||||
Done Inline ActionsThis is functionally equivalent i.e. #include <stdio.h> #define EM_MSIX_MASK 0x01F00000 /* For 82574 use */ int main () { unsigned int x = 0; x |= 1 << 20; // RxQ0 x |= 1 << 21; // RxQ1 x |= 1 << 22; // TxQ0 x |= 1 << 23; // TxQ1 x |= 1 << 24; // Other if (x == EM_MSIX_MASK) printf("%08x == %08x\n", x, EM_MSIX_MASK); return 0; } 01f00000 == 01f00000 kbowling: This is functionally equivalent i.e.
```
#include <stdio.h>
#define EM_MSIX_MASK 0x01F00000 /*… | |||||
Done Inline ActionsI agree. markj: I agree. | |||||
ims_mask |= adapter->ims; | ims_mask |= adapter->ims; | ||||
} | } | ||||
E1000_WRITE_REG(hw, E1000_IMS, ims_mask); | E1000_WRITE_REG(hw, E1000_IMS, ims_mask); | ||||
} | } | ||||
static void | static void | ||||
em_if_intr_disable(if_ctx_t ctx) | em_if_intr_disable(if_ctx_t ctx) | ||||
{ | { | ||||
struct adapter *adapter = iflib_get_softc(ctx); | struct adapter *adapter = iflib_get_softc(ctx); | ||||
struct e1000_hw *hw = &adapter->hw; | struct e1000_hw *hw = &adapter->hw; | ||||
if (hw->mac.type == e1000_82574) | if (adapter->intr_type == IFLIB_INTR_MSIX) | ||||
Not Done Inline Actionskbowling: @marius had changed this to the mac type citing rS206437, but I don't agree. iflib… | |||||
E1000_WRITE_REG(hw, EM_EIAC, 0); | E1000_WRITE_REG(hw, EM_EIAC, 0); | ||||
E1000_WRITE_REG(hw, E1000_IMC, 0xffffffff); | E1000_WRITE_REG(hw, E1000_IMC, 0xffffffff); | ||||
} | } | ||||
static void | static void | ||||
igb_if_intr_enable(if_ctx_t ctx) | igb_if_intr_enable(if_ctx_t ctx) | ||||
{ | { | ||||
struct adapter *adapter = iflib_get_softc(ctx); | struct adapter *adapter = iflib_get_softc(ctx); | ||||
▲ Show 20 Lines • Show All 1,174 Lines • Show Last 20 Lines |
@marius I'm not really sure why this would be necessary, under what circumstance will the admin task not re-arm link changes? It seems like this might have been because we didn't set the adapter's ims up as this diff will now do.