Previous diff had a bug in the interrupt handler refactoring that showed
up on i386. The version below has been tested on i386 and doesn't seem
to cause any regression.
Here's the original explanation, diff below has msix toggle to "off":
> Diff below allows ix(4) to establish two MSI-X handlers. Multiqueue
> support is intentionally not included in this diff.
>
> One handler is for the single queue (index ":0") and one for the
> link interrupt:
>
> # vmstat -i |grep ix0
> irq114/ix0:0 73178178 3758
> irq115/ix0 2 0
>
> A per-driver toggle allows to switch MSI-X support on/off. My plan is
> to commit with the switch "off" for the moment. Switching it to "on"
> should be better done in the beginning of a release cycle. However it
> is "on" in the diff below for testing purposes.
I appreciate more tests and oks :)
Index: dev/pci/if_ix.c
===================================================================
RCS file: /cvs/src/sys/dev/pci/if_ix.c,v
retrieving revision 1.163
diff -u -p -r1.163 if_ix.c
--- dev/pci/if_ix.c 25 Mar 2020 17:20:46 -0000 1.163
+++ dev/pci/if_ix.c 26 Mar 2020 10:37:41 -0000
@@ -114,6 +114,8 @@ int ixgbe_media_change(struct ifnet *);
void ixgbe_identify_hardware(struct ix_softc *);
int ixgbe_allocate_pci_resources(struct ix_softc *);
int ixgbe_allocate_legacy(struct ix_softc *);
+int ixgbe_allocate_msix(struct ix_softc *);
+int ixgbe_setup_msix(struct ix_softc *);
int ixgbe_allocate_queues(struct ix_softc *);
void ixgbe_free_pci_resources(struct ix_softc *);
void ixgbe_local_timer(void *);
@@ -140,6 +142,7 @@ void ixgbe_initialize_rss_mapping(struct
int ixgbe_rxfill(struct rx_ring *);
void ixgbe_rxrefill(void *);
+int ixgbe_intr(struct ix_softc *sc);
void ixgbe_enable_intr(struct ix_softc *);
void ixgbe_disable_intr(struct ix_softc *);
void ixgbe_update_stats_counters(struct ix_softc *);
@@ -172,11 +175,16 @@ void ixgbe_handle_msf(struct ix_softc *)
void ixgbe_handle_phy(struct ix_softc *);
/* Legacy (single vector interrupt handler */
-int ixgbe_intr(void *);
+int ixgbe_legacy_intr(void *);
void ixgbe_enable_queue(struct ix_softc *, uint32_t);
+void ixgbe_enable_queues(struct ix_softc *);
void ixgbe_disable_queue(struct ix_softc *, uint32_t);
void ixgbe_rearm_queue(struct ix_softc *, uint32_t);
+/* MSI-X (multiple vectors interrupt handlers) */
+int ixgbe_link_intr(void *);
+int ixgbe_queue_intr(void *);
+
/*********************************************************************
* OpenBSD Device Interface Entry Points
*********************************************************************/
@@ -190,6 +198,7 @@ struct cfattach ix_ca = {
};
int ixgbe_smart_speed = ixgbe_smart_speed_on;
+int ixgbe_enable_msix = 0;
/*********************************************************************
* Device identification routine
@@ -292,7 +301,10 @@ ixgbe_attach(struct device *parent, stru
bcopy(sc->hw.mac.addr, sc->arpcom.ac_enaddr,
IXGBE_ETH_LENGTH_OF_ADDRESS);
- error = ixgbe_allocate_legacy(sc);
+ if (sc->msix > 1)
+ error = ixgbe_allocate_msix(sc);
+ else
+ error = ixgbe_allocate_legacy(sc);
if (error)
goto err_late;
@@ -524,6 +536,7 @@ ixgbe_ioctl(struct ifnet * ifp, u_long c
ixgbe_disable_intr(sc);
ixgbe_iff(sc);
ixgbe_enable_intr(sc);
+ ixgbe_enable_queues(sc);
}
error = 0;
}
@@ -819,6 +832,9 @@ ixgbe_init(void *arg)
itr |= IXGBE_EITR_LLI_MOD | IXGBE_EITR_CNT_WDIS;
IXGBE_WRITE_REG(&sc->hw, IXGBE_EITR(0), itr);
+ /* Set moderation on the Link interrupt */
+ IXGBE_WRITE_REG(&sc->hw, IXGBE_EITR(sc->linkvec), IXGBE_LINK_ITR);
+
/* Enable power to the phy */
if (sc->hw.phy.ops.set_phy_power)
sc->hw.phy.ops.set_phy_power(&sc->hw, TRUE);
@@ -834,6 +850,7 @@ ixgbe_init(void *arg)
/* And now turn on interrupts */
ixgbe_enable_intr(sc);
+ ixgbe_enable_queues(sc);
/* Now inform the stack we're ready */
ifp->if_flags |= IFF_RUNNING;
@@ -964,6 +981,16 @@ ixgbe_enable_queue(struct ix_softc *sc,
}
void
+ixgbe_enable_queues(struct ix_softc *sc)
+{
+ struct ix_queue *que;
+ int i;
+
+ for (i = 0, que = sc->queues; i < sc->num_queues; i++, que++)
+ ixgbe_enable_queue(sc, que->msix);
+}
+
+void
ixgbe_disable_queue(struct ix_softc *sc, uint32_t vector)
{
uint64_t queue = 1ULL << vector;
@@ -982,6 +1009,41 @@ ixgbe_disable_queue(struct ix_softc *sc,
}
}
+/*
+ * MSIX Interrupt Handlers
+ */
+int
+ixgbe_link_intr(void *vsc)
+{
+ struct ix_softc *sc = (struct ix_softc *)vsc;
+
+ return ixgbe_intr(sc);
+}
+
+int
+ixgbe_queue_intr(void *vque)
+{
+ struct ix_queue *que = vque;
+ struct ix_softc *sc = que->sc;
+ struct ifnet *ifp = &sc->arpcom.ac_if;
+ struct tx_ring *txr = sc->tx_rings;
+
+ if (ISSET(ifp->if_flags, IFF_RUNNING)) {
+ ixgbe_rxeof(que);
+ ixgbe_txeof(txr);
+ if (ixgbe_rxfill(que->rxr)) {
+ /* Advance the Rx Queue "Tail Pointer" */
+ IXGBE_WRITE_REG(&sc->hw, IXGBE_RDT(que->rxr->me),
+ que->rxr->last_desc_filled);
+ } else
+ timeout_add(&sc->rx_refill, 1);
+ }
+
+ ixgbe_enable_queue(sc, que->msix);
+
+ return (1);
+}
+
/*********************************************************************
*
* Legacy Interrupt Service routine
@@ -989,29 +1051,24 @@ ixgbe_disable_queue(struct ix_softc *sc,
**********************************************************************/
int
-ixgbe_intr(void *arg)
+ixgbe_legacy_intr(void *arg)
{
struct ix_softc *sc = (struct ix_softc *)arg;
- struct ix_queue *que = sc->queues;
struct ifnet *ifp = &sc->arpcom.ac_if;
struct tx_ring *txr = sc->tx_rings;
- struct ixgbe_hw *hw = &sc->hw;
- uint32_t reg_eicr, mod_mask, msf_mask;
- int i, refill = 0;
+ int rv;
- reg_eicr = IXGBE_READ_REG(&sc->hw, IXGBE_EICR);
- if (reg_eicr == 0) {
- ixgbe_enable_intr(sc);
+ rv = ixgbe_intr(sc);
+ if (rv == 0) {
+ ixgbe_enable_queues(sc);
return (0);
}
if (ISSET(ifp->if_flags, IFF_RUNNING)) {
+ struct ix_queue *que = sc->queues;
+
ixgbe_rxeof(que);
ixgbe_txeof(txr);
- refill = 1;
- }
-
- if (refill) {
if (ixgbe_rxfill(que->rxr)) {
/* Advance the Rx Queue "Tail Pointer" */
IXGBE_WRITE_REG(&sc->hw, IXGBE_RDT(que->rxr->me),
@@ -1020,6 +1077,23 @@ ixgbe_intr(void *arg)
timeout_add(&sc->rx_refill, 1);
}
+ ixgbe_enable_queues(sc);
+ return (rv);
+}
+
+int
+ixgbe_intr(struct ix_softc *sc)
+{
+ struct ifnet *ifp = &sc->arpcom.ac_if;
+ struct ixgbe_hw *hw = &sc->hw;
+ uint32_t reg_eicr, mod_mask, msf_mask;
+
+ reg_eicr = IXGBE_READ_REG(&sc->hw, IXGBE_EICR);
+ if (reg_eicr == 0) {
+ ixgbe_enable_intr(sc);
+ return (0);
+ }
+
/* Link status change */
if (reg_eicr & IXGBE_EICR_LSC) {
KERNEL_LOCK();
@@ -1090,9 +1164,6 @@ ixgbe_intr(void *arg)
KERNEL_UNLOCK();
}
- for (i = 0; i < sc->num_queues; i++, que++)
- ixgbe_enable_queue(sc, que->msix);
-
return (1);
}
@@ -1626,7 +1697,7 @@ ixgbe_allocate_legacy(struct ix_softc *s
intrstr = pci_intr_string(pc, ih);
sc->tag = pci_intr_establish(pc, ih, IPL_NET | IPL_MPSAFE,
- ixgbe_intr, sc, sc->dev.dv_xname);
+ ixgbe_legacy_intr, sc, sc->dev.dv_xname);
if (sc->tag == NULL) {
printf(": couldn't establish interrupt");
if (intrstr != NULL)
@@ -1642,6 +1713,92 @@ ixgbe_allocate_legacy(struct ix_softc *s
return (0);
}
+/*********************************************************************
+ *
+ * Setup the MSI-X Interrupt handlers
+ *
+ **********************************************************************/
+int
+ixgbe_allocate_msix(struct ix_softc *sc)
+{
+ struct ixgbe_osdep *os = &sc->osdep;
+ struct pci_attach_args *pa = &os->os_pa;
+ int vec, error = 0;
+ struct ix_queue *que = sc->queues;
+ const char *intrstr = NULL;
+ pci_chipset_tag_t pc = pa->pa_pc;
+ pci_intr_handle_t ih;
+
+ vec = 0;
+ if (pci_intr_map_msix(pa, vec, &ih)) {
+ printf(": couldn't map interrupt\n");
+ return (ENXIO);
+ }
+
+ que->msix = vec;
+ snprintf(que->name, sizeof(que->name), "%s:%d", sc->dev.dv_xname, vec);
+
+ intrstr = pci_intr_string(pc, ih);
+ que->tag = pci_intr_establish(pc, ih, IPL_NET | IPL_MPSAFE,
+ ixgbe_queue_intr, que, que->name);
+ if (que->tag == NULL) {
+ printf(": couldn't establish interrupt");
+ if (intrstr != NULL)
+ printf(" at %s", intrstr);
+ printf("\n");
+ return (ENXIO);
+ }
+
+ /* Now the link status/control last MSI-X vector */
+ vec++;
+ if (pci_intr_map_msix(pa, vec, &ih)) {
+ printf(": couldn't map link vector\n");
+ error = ENXIO;
+ goto fail;
+ }
+
+ intrstr = pci_intr_string(pc, ih);
+ sc->tag = pci_intr_establish(pc, ih, IPL_NET | IPL_MPSAFE,
+ ixgbe_link_intr, sc, sc->dev.dv_xname);
+ if (sc->tag == NULL) {
+ printf(": couldn't establish interrupt");
+ if (intrstr != NULL)
+ printf(" at %s", intrstr);
+ printf("\n");
+ error = ENXIO;
+ goto fail;
+ }
+
+ sc->linkvec = vec;
+ printf(", %s, %d queue%s", intrstr, vec, (vec > 1) ? "s" : "");
+
+ return (0);
+fail:
+ pci_intr_disestablish(pc, que->tag);
+ que->tag = NULL;
+ return (error);
+}
+
+int
+ixgbe_setup_msix(struct ix_softc *sc)
+{
+ struct ixgbe_osdep *os = &sc->osdep;
+ struct pci_attach_args *pa = &os->os_pa;
+ pci_intr_handle_t dummy;
+
+ if (!ixgbe_enable_msix)
+ return (0);
+
+ /*
+ * Try a dummy map, maybe this bus doesn't like MSI, this function
+ * has no side effects.
+ */
+ if (pci_intr_map_msix(pa, 0, &dummy))
+ return (0);
+
+ return (2); /* queue vector + link vector */
+}
+
int
ixgbe_allocate_pci_resources(struct ix_softc *sc)
{
@@ -1666,10 +1823,8 @@ ixgbe_allocate_pci_resources(struct ix_s
sc->num_queues = 1;
sc->hw.back = os;
-#ifdef notyet
/* Now setup MSI or MSI/X, return us the number of supported vectors. */
sc->msix = ixgbe_setup_msix(sc);
-#endif
return (0);
}
@@ -3085,9 +3240,7 @@ void
ixgbe_enable_intr(struct ix_softc *sc)
{
struct ixgbe_hw *hw = &sc->hw;
- struct ix_queue *que = sc->queues;
uint32_t mask, fwsm;
- int i;
mask = (IXGBE_EIMS_ENABLE_MASK & ~IXGBE_EIMS_RTX_QUEUE);
/* Enable Fan Failure detection */
@@ -3135,14 +3288,6 @@ ixgbe_enable_intr(struct ix_softc *sc)
IXGBE_WRITE_REG(hw, IXGBE_EIAC, mask);
}
- /*
- * Now enable all queues, this is done separately to
- * allow for handling the extended (beyond 32) MSIX
- * vectors that can be used by 82599
- */
- for (i = 0; i < sc->num_queues; i++, que++)
- ixgbe_enable_queue(sc, que->msix);
-
IXGBE_WRITE_FLUSH(hw);
}
@@ -3258,15 +3403,11 @@ ixgbe_set_ivar(struct ix_softc *sc, uint
void
ixgbe_configure_ivars(struct ix_softc *sc)
{
-#if notyet
struct ix_queue *que = sc->queues;
uint32_t newitr;
int i;
- if (ixgbe_max_interrupt_rate > 0)
- newitr = (4000000 / ixgbe_max_interrupt_rate) & 0x0FF8;
- else
- newitr = 0;
+ newitr = (4000000 / IXGBE_INTS_PER_SEC) & 0x0FF8;
for (i = 0; i < sc->num_queues; i++, que++) {
/* First the RX queue entry */
@@ -3280,7 +3421,6 @@ ixgbe_configure_ivars(struct ix_softc *s
/* For the Link interrupt */
ixgbe_set_ivar(sc, 1, sc->linkvec, -1);
-#endif
}
/*
Index: dev/pci/if_ix.h
===================================================================
RCS file: /cvs/src/sys/dev/pci/if_ix.h,v
retrieving revision 1.39
diff -u -p -r1.39 if_ix.h
--- dev/pci/if_ix.h 25 Mar 2020 17:20:46 -0000 1.39
+++ dev/pci/if_ix.h 26 Mar 2020 10:36:39 -0000
@@ -120,6 +120,7 @@
* Interrupt Moderation parameters
*/
#define IXGBE_INTS_PER_SEC 8000
+#define IXGBE_LINK_ITR 1000
struct ixgbe_tx_buf {
uint32_t eop_index;
@@ -154,6 +155,8 @@ struct ix_queue {
uint32_t msix; /* This queue's MSIX vector */
uint32_t eims; /* This queue's EIMS bit */
uint32_t eitr_setting;
+ char name[8];
+ pci_intr_handle_t ih;
void *tag;
struct tx_ring *txr;
struct rx_ring *rxr;
