Make it possible to use ix(4) with MSI-X, currently disabled by default.

The current implementation still uses a single queue but already establishes
a different handler for link interrupts.  This is done in preparation for
multi-queues support.

The performance and i386 regression exposed by the last version of this
diff have been fixed thanks to the help of Hrvoje Popovski.

Based on a bigger diff from haesbaert@ and on the FreeBSD code.

Tested by Sigi Rudzio, Hrvoje Popovski and jmatthew@, ok jmatthew@

2 files changed, 184 insertions, 38 deletions

diff --git a/sys/dev/pci/if_ix.c b/sys/dev/pci/if_ix.c
index 8b49547519a..e74fbfdd2ae 100644
--- a/sys/dev/pci/if_ix.c
+++ b/sys/dev/pci/if_ix.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: if_ix.c,v 1.163 2020/03/25 17:20:46 mpi Exp $	*/
+/*	$OpenBSD: if_ix.c,v 1.164 2020/04/06 08:31:04 mpi Exp $	*/
 
 /******************************************************************************
 
@@ -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 ix_softc *);
 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, struct device *self, void *aux)
 	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 command, caddr_t data)
 			ixgbe_disable_intr(sc);
 			ixgbe_iff(sc);
 			ixgbe_enable_intr(sc);
+			ixgbe_enable_queues(sc);
 		}
 		error = 0;
 	}
@@ -819,6 +832,12 @@ ixgbe_init(void *arg)
 		itr |= IXGBE_EITR_LLI_MOD | IXGBE_EITR_CNT_WDIS;
 	IXGBE_WRITE_REG(&sc->hw, IXGBE_EITR(0), itr);
 
+	if (sc->msix > 1) {
+		/* 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 +853,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 +984,16 @@ ixgbe_enable_queue(struct ix_softc *sc, uint32_t vector)
 }
 
 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 +1012,41 @@ ixgbe_disable_queue(struct ix_softc *sc, uint32_t vector)
 	}
 }
 
+/*
+ * 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 +1054,24 @@ ixgbe_disable_queue(struct ix_softc *sc, uint32_t vector)
  **********************************************************************/
 
 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 +1080,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 +1167,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 +1700,7 @@ ixgbe_allocate_legacy(struct ix_softc *sc)
 
 	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 +1716,92 @@ ixgbe_allocate_legacy(struct ix_softc *sc)
 	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 +1826,8 @@ ixgbe_allocate_pci_resources(struct ix_softc *sc)
 	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 +3243,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 +3291,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 +3406,11 @@ ixgbe_set_ivar(struct ix_softc *sc, uint8_t entry, uint8_t vector, int8_t type)
 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 +3424,6 @@ ixgbe_configure_ivars(struct ix_softc *sc)
 
 	/* For the Link interrupt */
 	ixgbe_set_ivar(sc, 1, sc->linkvec, -1);
-#endif
 }
 
 /*
diff --git a/sys/dev/pci/if_ix.h b/sys/dev/pci/if_ix.h
index cc28e33976c..c8089740c19 100644
--- a/sys/dev/pci/if_ix.h
+++ b/sys/dev/pci/if_ix.h
@@ -1,4 +1,4 @@
-/*	$OpenBSD: if_ix.h,v 1.39 2020/03/25 17:20:46 mpi Exp $	*/
+/*	$OpenBSD: if_ix.h,v 1.40 2020/04/06 08:31:04 mpi Exp $	*/
 
 /******************************************************************************
 
@@ -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;