[PATCH net v2 0/3] Fix IRQ vectors

[PATCH net v2 0/3] Fix IRQ vectors

[Jiawen Wu]

The interrupt vector order was adjusted by [1]commit 937d46ecc5f9 ("net:
wangxun: add ethtool_ops for channel number") in Linux-6.8. Because at
that time, the MISC interrupt acts as the parent interrupt in the GPIO
IRQ chip. When the number of Rx/Tx ring changes, the last MISC
interrupt must be reallocated. Then the GPIO interrupt controller would
be corrupted. So the initial plan was to adjust the sequence of the
interrupt vectors, let MISC interrupt to be the first one and do not
free it.

Later, irq_domain was introduced in [2]commit aefd013624a1 ("net: txgbe:
use irq_domain for interrupt controller") to avoid this problem.
However, the vector sequence adjustment was not reverted. So there is
still one problem that has been left unresolved.

Due to hardware limitations of NGBE, queue IRQs can only be requested
on vector 0 to 7. When the number of queues is set to the maximum 8,
the PCI IRQ vectors are allocated from 0 to 8. The vector 0 is used by
MISC interrupt, and althrough the vector 8 is used by queue interrupt,
it is unable to receive packets. This will cause some packets to be
dropped when RSS is enabled and they are assigned to queue 8.

This patch set fix the above problems.

[1] https://git.kernel.org/netdev/net-next/c/937d46ecc5f9
[2] https://git.kernel.org/netdev/net-next/c/aefd013624a1

v1 -> v2:
- add a patch to fix the issue for ngbe sriov

Jiawen Wu (3):
  net: txgbe: request MISC IRQ in ndo_open
  net: wangxun: revert the adjustment of the IRQ vector sequence
  net: ngbe: specify IRQ vector when the number of VFs is 7

 drivers/net/ethernet/wangxun/libwx/wx_lib.c   | 26 ++++++++++++-------
 drivers/net/ethernet/wangxun/libwx/wx_sriov.c |  4 +++
 drivers/net/ethernet/wangxun/libwx/wx_type.h  |  3 ++-
 drivers/net/ethernet/wangxun/ngbe/ngbe_main.c |  4 +--
 drivers/net/ethernet/wangxun/ngbe/ngbe_type.h |  2 +-
 .../net/ethernet/wangxun/txgbe/txgbe_irq.c    |  8 +++---
 .../net/ethernet/wangxun/txgbe/txgbe_main.c   | 22 +++++++---------
 .../net/ethernet/wangxun/txgbe/txgbe_type.h   |  4 +--
 8 files changed, 42 insertions(+), 31 deletions(-)

-- 
2.48.1

[PATCH net v2 1/3] net: txgbe: request MISC IRQ in ndo_open

[Jiawen Wu]

Move the creating of irq_domain for MISC IRQ from .probe to .ndo_open,
and free it in .ndo_stop, to maintain consistency with the queue IRQs.
This it for subsequent adjustments to the IRQ vectors.

Fixes: aefd013624a1 ("net: txgbe: use irq_domain for interrupt controller")
Cc: stable@vger.kernel.org
Signed-off-by: Jiawen Wu <jiawenwu@trustnetic.com>
---
 .../net/ethernet/wangxun/txgbe/txgbe_irq.c    |  2 +-
 .../net/ethernet/wangxun/txgbe/txgbe_main.c   | 22 +++++++++----------
 2 files changed, 11 insertions(+), 13 deletions(-)

diff --git a/drivers/net/ethernet/wangxun/txgbe/txgbe_irq.c b/drivers/net/ethernet/wangxun/txgbe/txgbe_irq.c
index 20b9a28bcb55..dc468053bdf8 100644
--- a/drivers/net/ethernet/wangxun/txgbe/txgbe_irq.c
+++ b/drivers/net/ethernet/wangxun/txgbe/txgbe_irq.c
@@ -78,7 +78,6 @@ int txgbe_request_queue_irqs(struct wx *wx)
 		free_irq(wx->msix_q_entries[vector].vector,
 			 wx->q_vector[vector]);
 	}
-	wx_reset_interrupt_capability(wx);
 	return err;
 }
 
@@ -211,6 +210,7 @@ void txgbe_free_misc_irq(struct txgbe *txgbe)
 	free_irq(txgbe->link_irq, txgbe);
 	free_irq(txgbe->misc.irq, txgbe);
 	txgbe_del_irq_domain(txgbe);
+	txgbe->wx->misc_irq_domain = false;
 }
 
 int txgbe_setup_misc_irq(struct txgbe *txgbe)
diff --git a/drivers/net/ethernet/wangxun/txgbe/txgbe_main.c b/drivers/net/ethernet/wangxun/txgbe/txgbe_main.c
index f3d2778b8e35..a5867f3c93fc 100644
--- a/drivers/net/ethernet/wangxun/txgbe/txgbe_main.c
+++ b/drivers/net/ethernet/wangxun/txgbe/txgbe_main.c
@@ -458,10 +458,14 @@ static int txgbe_open(struct net_device *netdev)
 
 	wx_configure(wx);
 
-	err = txgbe_request_queue_irqs(wx);
+	err = txgbe_setup_misc_irq(wx->priv);
 	if (err)
 		goto err_free_resources;
 
+	err = txgbe_request_queue_irqs(wx);
+	if (err)
+		goto err_free_misc_irq;
+
 	/* Notify the stack of the actual queue counts. */
 	err = netif_set_real_num_tx_queues(netdev, wx->num_tx_queues);
 	if (err)
@@ -479,6 +483,9 @@ static int txgbe_open(struct net_device *netdev)
 
 err_free_irq:
 	wx_free_irq(wx);
+err_free_misc_irq:
+	txgbe_free_misc_irq(wx->priv);
+	wx_reset_interrupt_capability(wx);
 err_free_resources:
 	wx_free_resources(wx);
 err_reset:
@@ -519,6 +526,7 @@ static int txgbe_close(struct net_device *netdev)
 	wx_ptp_stop(wx);
 	txgbe_down(wx);
 	wx_free_irq(wx);
+	txgbe_free_misc_irq(wx->priv);
 	wx_free_resources(wx);
 	txgbe_fdir_filter_exit(wx);
 	wx_control_hw(wx, false);
@@ -564,7 +572,6 @@ static void txgbe_shutdown(struct pci_dev *pdev)
 int txgbe_setup_tc(struct net_device *dev, u8 tc)
 {
 	struct wx *wx = netdev_priv(dev);
-	struct txgbe *txgbe = wx->priv;
 
 	/* Hardware has to reinitialize queues and interrupts to
 	 * match packet buffer alignment. Unfortunately, the
@@ -575,7 +582,6 @@ int txgbe_setup_tc(struct net_device *dev, u8 tc)
 	else
 		txgbe_reset(wx);
 
-	txgbe_free_misc_irq(txgbe);
 	wx_clear_interrupt_scheme(wx);
 
 	if (tc)
@@ -584,7 +590,6 @@ int txgbe_setup_tc(struct net_device *dev, u8 tc)
 		netdev_reset_tc(dev);
 
 	wx_init_interrupt_scheme(wx);
-	txgbe_setup_misc_irq(txgbe);
 
 	if (netif_running(dev))
 		txgbe_open(dev);
@@ -882,13 +887,9 @@ static int txgbe_probe(struct pci_dev *pdev,
 
 	txgbe_init_fdir(txgbe);
 
-	err = txgbe_setup_misc_irq(txgbe);
-	if (err)
-		goto err_release_hw;
-
 	err = txgbe_init_phy(txgbe);
 	if (err)
-		goto err_free_misc_irq;
+		goto err_release_hw;
 
 	err = register_netdev(netdev);
 	if (err)
@@ -916,8 +917,6 @@ static int txgbe_probe(struct pci_dev *pdev,
 
 err_remove_phy:
 	txgbe_remove_phy(txgbe);
-err_free_misc_irq:
-	txgbe_free_misc_irq(txgbe);
 err_release_hw:
 	wx_clear_interrupt_scheme(wx);
 	wx_control_hw(wx, false);
@@ -957,7 +956,6 @@ static void txgbe_remove(struct pci_dev *pdev)
 	unregister_netdev(netdev);
 
 	txgbe_remove_phy(txgbe);
-	txgbe_free_misc_irq(txgbe);
 	wx_free_isb_resources(wx);
 
 	pci_release_selected_regions(pdev,
-- 
2.48.1

[PATCH net v2 2/3] net: wangxun: revert the adjustment of the IRQ vector sequence

[Jiawen Wu]

Due to hardware limitations of NGBE, queue IRQs can only be requested
on vector 0 to 7. When the number of queues is set to the maximum 8,
the PCI IRQ vectors are allocated from 0 to 8. The vector 0 is used by
MISC interrupt, and althrough the vector 8 is used by queue interrupt,
it is unable to receive packets. This will cause some packets to be
dropped when RSS is enabled and they are assigned to queue 8.

So revert the adjustment of the MISC IRQ location, to make it be the
last one in IRQ vectors.

Fixes: 937d46ecc5f9 ("net: wangxun: add ethtool_ops for channel number")
Cc: stable@vger.kernel.org
Signed-off-by: Jiawen Wu <jiawenwu@trustnetic.com>
---
 drivers/net/ethernet/wangxun/libwx/wx_lib.c     | 17 ++++++++---------
 drivers/net/ethernet/wangxun/libwx/wx_type.h    |  2 +-
 drivers/net/ethernet/wangxun/ngbe/ngbe_main.c   |  2 +-
 drivers/net/ethernet/wangxun/ngbe/ngbe_type.h   |  2 +-
 drivers/net/ethernet/wangxun/txgbe/txgbe_irq.c  |  6 +++---
 drivers/net/ethernet/wangxun/txgbe/txgbe_type.h |  4 ++--
 6 files changed, 16 insertions(+), 17 deletions(-)

diff --git a/drivers/net/ethernet/wangxun/libwx/wx_lib.c b/drivers/net/ethernet/wangxun/libwx/wx_lib.c
index 7f2e6cddfeb1..66eaf5446115 100644
--- a/drivers/net/ethernet/wangxun/libwx/wx_lib.c
+++ b/drivers/net/ethernet/wangxun/libwx/wx_lib.c
@@ -1746,7 +1746,7 @@ static void wx_set_num_queues(struct wx *wx)
  */
 static int wx_acquire_msix_vectors(struct wx *wx)
 {
-	struct irq_affinity affd = { .pre_vectors = 1 };
+	struct irq_affinity affd = { .post_vectors = 1 };
 	int nvecs, i;
 
 	/* We start by asking for one vector per queue pair */
@@ -1783,16 +1783,17 @@ static int wx_acquire_msix_vectors(struct wx *wx)
 		return nvecs;
 	}
 
-	wx->msix_entry->entry = 0;
-	wx->msix_entry->vector = pci_irq_vector(wx->pdev, 0);
 	nvecs -= 1;
 	for (i = 0; i < nvecs; i++) {
 		wx->msix_q_entries[i].entry = i;
-		wx->msix_q_entries[i].vector = pci_irq_vector(wx->pdev, i + 1);
+		wx->msix_q_entries[i].vector = pci_irq_vector(wx->pdev, i);
 	}
 
 	wx->num_q_vectors = nvecs;
 
+	wx->msix_entry->entry = nvecs;
+	wx->msix_entry->vector = pci_irq_vector(wx->pdev, nvecs);
+
 	return 0;
 }
 
@@ -2299,8 +2300,6 @@ static void wx_set_ivar(struct wx *wx, s8 direction,
 		wr32(wx, WX_PX_MISC_IVAR, ivar);
 	} else {
 		/* tx or rx causes */
-		if (!(wx->mac.type == wx_mac_em && wx->num_vfs == 7))
-			msix_vector += 1; /* offset for queue vectors */
 		msix_vector |= WX_PX_IVAR_ALLOC_VAL;
 		index = ((16 * (queue & 1)) + (8 * direction));
 		ivar = rd32(wx, WX_PX_IVAR(queue >> 1));
@@ -2339,7 +2338,7 @@ void wx_write_eitr(struct wx_q_vector *q_vector)
 
 	itr_reg |= WX_PX_ITR_CNT_WDIS;
 
-	wr32(wx, WX_PX_ITR(v_idx + 1), itr_reg);
+	wr32(wx, WX_PX_ITR(v_idx), itr_reg);
 }
 
 /**
@@ -2392,9 +2391,9 @@ void wx_configure_vectors(struct wx *wx)
 		wx_write_eitr(q_vector);
 	}
 
-	wx_set_ivar(wx, -1, 0, 0);
+	wx_set_ivar(wx, -1, 0, v_idx);
 	if (pdev->msix_enabled)
-		wr32(wx, WX_PX_ITR(0), 1950);
+		wr32(wx, WX_PX_ITR(v_idx), 1950);
 }
 EXPORT_SYMBOL(wx_configure_vectors);
 
diff --git a/drivers/net/ethernet/wangxun/libwx/wx_type.h b/drivers/net/ethernet/wangxun/libwx/wx_type.h
index 7730c9fc3e02..d392394791b3 100644
--- a/drivers/net/ethernet/wangxun/libwx/wx_type.h
+++ b/drivers/net/ethernet/wangxun/libwx/wx_type.h
@@ -1343,7 +1343,7 @@ struct wx {
 };
 
 #define WX_INTR_ALL (~0ULL)
-#define WX_INTR_Q(i) BIT((i) + 1)
+#define WX_INTR_Q(i) BIT((i))
 
 /* register operations */
 #define wr32(a, reg, value)	writel((value), ((a)->hw_addr + (reg)))
diff --git a/drivers/net/ethernet/wangxun/ngbe/ngbe_main.c b/drivers/net/ethernet/wangxun/ngbe/ngbe_main.c
index b5022c49dc5e..68415a7ef12f 100644
--- a/drivers/net/ethernet/wangxun/ngbe/ngbe_main.c
+++ b/drivers/net/ethernet/wangxun/ngbe/ngbe_main.c
@@ -161,7 +161,7 @@ static void ngbe_irq_enable(struct wx *wx, bool queues)
 	if (queues)
 		wx_intr_enable(wx, NGBE_INTR_ALL);
 	else
-		wx_intr_enable(wx, NGBE_INTR_MISC);
+		wx_intr_enable(wx, NGBE_INTR_MISC(wx));
 }
 
 /**
diff --git a/drivers/net/ethernet/wangxun/ngbe/ngbe_type.h b/drivers/net/ethernet/wangxun/ngbe/ngbe_type.h
index bb74263f0498..6eca6de475f7 100644
--- a/drivers/net/ethernet/wangxun/ngbe/ngbe_type.h
+++ b/drivers/net/ethernet/wangxun/ngbe/ngbe_type.h
@@ -87,7 +87,7 @@
 #define NGBE_PX_MISC_IC_TIMESYNC		BIT(11) /* time sync */
 
 #define NGBE_INTR_ALL				0x1FF
-#define NGBE_INTR_MISC				BIT(0)
+#define NGBE_INTR_MISC(A)			BIT((A)->num_q_vectors)
 
 #define NGBE_PHY_CONFIG(reg_offset)		(0x14000 + ((reg_offset) * 4))
 #define NGBE_CFG_LAN_SPEED			0x14440
diff --git a/drivers/net/ethernet/wangxun/txgbe/txgbe_irq.c b/drivers/net/ethernet/wangxun/txgbe/txgbe_irq.c
index dc468053bdf8..3885283681ec 100644
--- a/drivers/net/ethernet/wangxun/txgbe/txgbe_irq.c
+++ b/drivers/net/ethernet/wangxun/txgbe/txgbe_irq.c
@@ -31,7 +31,7 @@ void txgbe_irq_enable(struct wx *wx, bool queues)
 	wr32(wx, WX_PX_MISC_IEN, misc_ien);
 
 	/* unmask interrupt */
-	wx_intr_enable(wx, TXGBE_INTR_MISC);
+	wx_intr_enable(wx, TXGBE_INTR_MISC(wx));
 	if (queues)
 		wx_intr_enable(wx, TXGBE_INTR_QALL(wx));
 }
@@ -131,7 +131,7 @@ static irqreturn_t txgbe_misc_irq_handle(int irq, void *data)
 		txgbe->eicr = eicr;
 		if (eicr & TXGBE_PX_MISC_IC_VF_MBOX) {
 			wx_msg_task(txgbe->wx);
-			wx_intr_enable(wx, TXGBE_INTR_MISC);
+			wx_intr_enable(wx, TXGBE_INTR_MISC(wx));
 		}
 		return IRQ_WAKE_THREAD;
 	}
@@ -183,7 +183,7 @@ static irqreturn_t txgbe_misc_irq_thread_fn(int irq, void *data)
 		nhandled++;
 	}
 
-	wx_intr_enable(wx, TXGBE_INTR_MISC);
+	wx_intr_enable(wx, TXGBE_INTR_MISC(wx));
 	return (nhandled > 0 ? IRQ_HANDLED : IRQ_NONE);
 }
 
diff --git a/drivers/net/ethernet/wangxun/txgbe/txgbe_type.h b/drivers/net/ethernet/wangxun/txgbe/txgbe_type.h
index 42ec815159e8..41915d7dd372 100644
--- a/drivers/net/ethernet/wangxun/txgbe/txgbe_type.h
+++ b/drivers/net/ethernet/wangxun/txgbe/txgbe_type.h
@@ -302,8 +302,8 @@ struct txgbe_fdir_filter {
 #define TXGBE_DEFAULT_RX_WORK           128
 #endif
 
-#define TXGBE_INTR_MISC       BIT(0)
-#define TXGBE_INTR_QALL(A)    GENMASK((A)->num_q_vectors, 1)
+#define TXGBE_INTR_MISC(A)    BIT((A)->num_q_vectors)
+#define TXGBE_INTR_QALL(A)    (TXGBE_INTR_MISC(A) - 1)
 
 #define TXGBE_MAX_EITR        GENMASK(11, 3)
 
-- 
2.48.1

[PATCH net v2 3/3] net: ngbe: specify IRQ vector when the number of VFs is 7

[Jiawen Wu]

For NGBE devices, the queue number is limited to be 1 when SRIOV is
enabled. In this case, IRQ vector[0] is used for MISC and vector[1] is
used for queue, based on the previous patches. But for the hardware
design, the IRQ vector[1] must be allocated for use by the VF[6] when
the number of VFs is 7. So the IRQ vector[0] should be shared for PF
MISC and QUEUE interrupts.

+-----------+----------------------+
| Vector    | Assigned To          |
+-----------+----------------------+
| Vector 0  | PF MISC and QUEUE    |
| Vector 1  | VF 6                 |
| Vector 2  | VF 5                 |
| Vector 3  | VF 4                 |
| Vector 4  | VF 3                 |
| Vector 5  | VF 2                 |
| Vector 6  | VF 1                 |
| Vector 7  | VF 0                 |
+-----------+----------------------+

Minimize code modifications, only adjust the IRQ vector number for this
case.

Fixes: 877253d2cbf2 ("net: ngbe: add sriov function support")
Signed-off-by: Jiawen Wu <jiawenwu@trustnetic.com>
---
 drivers/net/ethernet/wangxun/libwx/wx_lib.c   | 9 +++++++++
 drivers/net/ethernet/wangxun/libwx/wx_sriov.c | 4 ++++
 drivers/net/ethernet/wangxun/libwx/wx_type.h  | 1 +
 drivers/net/ethernet/wangxun/ngbe/ngbe_main.c | 2 +-
 drivers/net/ethernet/wangxun/ngbe/ngbe_type.h | 3 ++-
 5 files changed, 17 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/wangxun/libwx/wx_lib.c b/drivers/net/ethernet/wangxun/libwx/wx_lib.c
index 66eaf5446115..7b53169cd216 100644
--- a/drivers/net/ethernet/wangxun/libwx/wx_lib.c
+++ b/drivers/net/ethernet/wangxun/libwx/wx_lib.c
@@ -1794,6 +1794,13 @@ static int wx_acquire_msix_vectors(struct wx *wx)
 	wx->msix_entry->entry = nvecs;
 	wx->msix_entry->vector = pci_irq_vector(wx->pdev, nvecs);
 
+	if (test_bit(WX_FLAG_IRQ_VECTOR_SHARED, wx->flags)) {
+		wx->msix_entry->entry = 0;
+		wx->msix_entry->vector = pci_irq_vector(wx->pdev, 0);
+		wx->msix_q_entries[0].entry = 0;
+		wx->msix_q_entries[0].vector = pci_irq_vector(wx->pdev, 1);
+	}
+
 	return 0;
 }
 
@@ -2292,6 +2299,8 @@ static void wx_set_ivar(struct wx *wx, s8 direction,
 
 	if (direction == -1) {
 		/* other causes */
+		if (test_bit(WX_FLAG_IRQ_VECTOR_SHARED, wx->flags))
+			msix_vector = 0;
 		msix_vector |= WX_PX_IVAR_ALLOC_VAL;
 		index = 0;
 		ivar = rd32(wx, WX_PX_MISC_IVAR);
diff --git a/drivers/net/ethernet/wangxun/libwx/wx_sriov.c b/drivers/net/ethernet/wangxun/libwx/wx_sriov.c
index e8656d9d733b..c82ae137756c 100644
--- a/drivers/net/ethernet/wangxun/libwx/wx_sriov.c
+++ b/drivers/net/ethernet/wangxun/libwx/wx_sriov.c
@@ -64,6 +64,7 @@ static void wx_sriov_clear_data(struct wx *wx)
 	wr32m(wx, WX_PSR_VM_CTL, WX_PSR_VM_CTL_POOL_MASK, 0);
 	wx->ring_feature[RING_F_VMDQ].offset = 0;
 
+	clear_bit(WX_FLAG_IRQ_VECTOR_SHARED, wx->flags);
 	clear_bit(WX_FLAG_SRIOV_ENABLED, wx->flags);
 	/* Disable VMDq flag so device will be set in NM mode */
 	if (wx->ring_feature[RING_F_VMDQ].limit == 1)
@@ -78,6 +79,9 @@ static int __wx_enable_sriov(struct wx *wx, u8 num_vfs)
 	set_bit(WX_FLAG_SRIOV_ENABLED, wx->flags);
 	dev_info(&wx->pdev->dev, "SR-IOV enabled with %d VFs\n", num_vfs);
 
+	if (num_vfs == 7 && wx->mac.type == wx_mac_em)
+		set_bit(WX_FLAG_IRQ_VECTOR_SHARED, wx->flags);
+
 	/* Enable VMDq flag so device will be set in VM mode */
 	set_bit(WX_FLAG_VMDQ_ENABLED, wx->flags);
 	if (!wx->ring_feature[RING_F_VMDQ].limit)
diff --git a/drivers/net/ethernet/wangxun/libwx/wx_type.h b/drivers/net/ethernet/wangxun/libwx/wx_type.h
index d392394791b3..c363379126c0 100644
--- a/drivers/net/ethernet/wangxun/libwx/wx_type.h
+++ b/drivers/net/ethernet/wangxun/libwx/wx_type.h
@@ -1191,6 +1191,7 @@ enum wx_pf_flags {
 	WX_FLAG_VMDQ_ENABLED,
 	WX_FLAG_VLAN_PROMISC,
 	WX_FLAG_SRIOV_ENABLED,
+	WX_FLAG_IRQ_VECTOR_SHARED,
 	WX_FLAG_FDIR_CAPABLE,
 	WX_FLAG_FDIR_HASH,
 	WX_FLAG_FDIR_PERFECT,
diff --git a/drivers/net/ethernet/wangxun/ngbe/ngbe_main.c b/drivers/net/ethernet/wangxun/ngbe/ngbe_main.c
index 68415a7ef12f..e0fc897b0a58 100644
--- a/drivers/net/ethernet/wangxun/ngbe/ngbe_main.c
+++ b/drivers/net/ethernet/wangxun/ngbe/ngbe_main.c
@@ -286,7 +286,7 @@ static int ngbe_request_msix_irqs(struct wx *wx)
 	 * for queue. But when num_vfs == 7, vector[1] is assigned to vf6.
 	 * Misc and queue should reuse interrupt vector[0].
 	 */
-	if (wx->num_vfs == 7)
+	if (test_bit(WX_FLAG_IRQ_VECTOR_SHARED, wx->flags))
 		err = request_irq(wx->msix_entry->vector,
 				  ngbe_misc_and_queue, 0, netdev->name, wx);
 	else
diff --git a/drivers/net/ethernet/wangxun/ngbe/ngbe_type.h b/drivers/net/ethernet/wangxun/ngbe/ngbe_type.h
index 6eca6de475f7..b6252b272364 100644
--- a/drivers/net/ethernet/wangxun/ngbe/ngbe_type.h
+++ b/drivers/net/ethernet/wangxun/ngbe/ngbe_type.h
@@ -87,7 +87,8 @@
 #define NGBE_PX_MISC_IC_TIMESYNC		BIT(11) /* time sync */
 
 #define NGBE_INTR_ALL				0x1FF
-#define NGBE_INTR_MISC(A)			BIT((A)->num_q_vectors)
+#define NGBE_INTR_MISC(A)			((A)->num_vfs == 7 ? \
+						 BIT(0) : BIT((A)->num_q_vectors))
 
 #define NGBE_PHY_CONFIG(reg_offset)		(0x14000 + ((reg_offset) * 4))
 #define NGBE_CFG_LAN_SPEED			0x14440
-- 
2.48.1

Re: [PATCH net v2 1/3] net: txgbe: request MISC IRQ in ndo_open

[Michal Swiatkowski]

On Tue, Jun 24, 2025 at 04:56:32PM +0800, Jiawen Wu wrote:
> Move the creating of irq_domain for MISC IRQ from .probe to .ndo_open,
> and free it in .ndo_stop, to maintain consistency with the queue IRQs.
> This it for subsequent adjustments to the IRQ vectors.
> 
> Fixes: aefd013624a1 ("net: txgbe: use irq_domain for interrupt controller")
> Cc: stable@vger.kernel.org
> Signed-off-by: Jiawen Wu <jiawenwu@trustnetic.com>
> ---
>  .../net/ethernet/wangxun/txgbe/txgbe_irq.c    |  2 +-
>  .../net/ethernet/wangxun/txgbe/txgbe_main.c   | 22 +++++++++----------
>  2 files changed, 11 insertions(+), 13 deletions(-)
> 
> diff --git a/drivers/net/ethernet/wangxun/txgbe/txgbe_irq.c b/drivers/net/ethernet/wangxun/txgbe/txgbe_irq.c
> index 20b9a28bcb55..dc468053bdf8 100644
> --- a/drivers/net/ethernet/wangxun/txgbe/txgbe_irq.c
> +++ b/drivers/net/ethernet/wangxun/txgbe/txgbe_irq.c
> @@ -78,7 +78,6 @@ int txgbe_request_queue_irqs(struct wx *wx)
>  		free_irq(wx->msix_q_entries[vector].vector,
>  			 wx->q_vector[vector]);
>  	}
> -	wx_reset_interrupt_capability(wx);
>  	return err;
>  }
>  
> @@ -211,6 +210,7 @@ void txgbe_free_misc_irq(struct txgbe *txgbe)
>  	free_irq(txgbe->link_irq, txgbe);
>  	free_irq(txgbe->misc.irq, txgbe);
>  	txgbe_del_irq_domain(txgbe);
> +	txgbe->wx->misc_irq_domain = false;
>  }
>  
>  int txgbe_setup_misc_irq(struct txgbe *txgbe)
> diff --git a/drivers/net/ethernet/wangxun/txgbe/txgbe_main.c b/drivers/net/ethernet/wangxun/txgbe/txgbe_main.c
> index f3d2778b8e35..a5867f3c93fc 100644
> --- a/drivers/net/ethernet/wangxun/txgbe/txgbe_main.c
> +++ b/drivers/net/ethernet/wangxun/txgbe/txgbe_main.c
> @@ -458,10 +458,14 @@ static int txgbe_open(struct net_device *netdev)
>  
>  	wx_configure(wx);
>  
> -	err = txgbe_request_queue_irqs(wx);
> +	err = txgbe_setup_misc_irq(wx->priv);

Don't you need misc interrupt before ndo_open is called? I wonder if it
won't be simpler to always use last index (8) for misc irq. Is it
possible? I mean, use 8 event if the number of queues is lower than 8.
If yes you can drop this patch and hardcode misc interrupts on index 8.

[...]

BTW, assuming you can't always use last index, the patch looks fine.

Re: [PATCH net v2 3/3] net: ngbe: specify IRQ vector when the number of VFs is 7

[Michal Swiatkowski]

On Tue, Jun 24, 2025 at 04:56:34PM +0800, Jiawen Wu wrote:
> For NGBE devices, the queue number is limited to be 1 when SRIOV is
> enabled. In this case, IRQ vector[0] is used for MISC and vector[1] is
> used for queue, based on the previous patches. But for the hardware
> design, the IRQ vector[1] must be allocated for use by the VF[6] when
> the number of VFs is 7. So the IRQ vector[0] should be shared for PF
> MISC and QUEUE interrupts.
> 
> +-----------+----------------------+
> | Vector    | Assigned To          |
> +-----------+----------------------+
> | Vector 0  | PF MISC and QUEUE    |
> | Vector 1  | VF 6                 |
> | Vector 2  | VF 5                 |
> | Vector 3  | VF 4                 |
> | Vector 4  | VF 3                 |
> | Vector 5  | VF 2                 |
> | Vector 6  | VF 1                 |
> | Vector 7  | VF 0                 |
> +-----------+----------------------+
> 
> Minimize code modifications, only adjust the IRQ vector number for this
> case.
> 
> Fixes: 877253d2cbf2 ("net: ngbe: add sriov function support")
> Signed-off-by: Jiawen Wu <jiawenwu@trustnetic.com>
> ---
>  drivers/net/ethernet/wangxun/libwx/wx_lib.c   | 9 +++++++++
>  drivers/net/ethernet/wangxun/libwx/wx_sriov.c | 4 ++++
>  drivers/net/ethernet/wangxun/libwx/wx_type.h  | 1 +
>  drivers/net/ethernet/wangxun/ngbe/ngbe_main.c | 2 +-
>  drivers/net/ethernet/wangxun/ngbe/ngbe_type.h | 3 ++-
>  5 files changed, 17 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/net/ethernet/wangxun/libwx/wx_lib.c b/drivers/net/ethernet/wangxun/libwx/wx_lib.c
> index 66eaf5446115..7b53169cd216 100644
> --- a/drivers/net/ethernet/wangxun/libwx/wx_lib.c
> +++ b/drivers/net/ethernet/wangxun/libwx/wx_lib.c
> @@ -1794,6 +1794,13 @@ static int wx_acquire_msix_vectors(struct wx *wx)
>  	wx->msix_entry->entry = nvecs;
>  	wx->msix_entry->vector = pci_irq_vector(wx->pdev, nvecs);
>  
> +	if (test_bit(WX_FLAG_IRQ_VECTOR_SHARED, wx->flags)) {
> +		wx->msix_entry->entry = 0;
> +		wx->msix_entry->vector = pci_irq_vector(wx->pdev, 0);
> +		wx->msix_q_entries[0].entry = 0;
> +		wx->msix_q_entries[0].vector = pci_irq_vector(wx->pdev, 1);
> +	}
> +
>  	return 0;
>  }
>  
> @@ -2292,6 +2299,8 @@ static void wx_set_ivar(struct wx *wx, s8 direction,
>  
>  	if (direction == -1) {
>  		/* other causes */
> +		if (test_bit(WX_FLAG_IRQ_VECTOR_SHARED, wx->flags))
> +			msix_vector = 0;
>  		msix_vector |= WX_PX_IVAR_ALLOC_VAL;
>  		index = 0;
>  		ivar = rd32(wx, WX_PX_MISC_IVAR);
> diff --git a/drivers/net/ethernet/wangxun/libwx/wx_sriov.c b/drivers/net/ethernet/wangxun/libwx/wx_sriov.c
> index e8656d9d733b..c82ae137756c 100644
> --- a/drivers/net/ethernet/wangxun/libwx/wx_sriov.c
> +++ b/drivers/net/ethernet/wangxun/libwx/wx_sriov.c
> @@ -64,6 +64,7 @@ static void wx_sriov_clear_data(struct wx *wx)
>  	wr32m(wx, WX_PSR_VM_CTL, WX_PSR_VM_CTL_POOL_MASK, 0);
>  	wx->ring_feature[RING_F_VMDQ].offset = 0;
>  
> +	clear_bit(WX_FLAG_IRQ_VECTOR_SHARED, wx->flags);
>  	clear_bit(WX_FLAG_SRIOV_ENABLED, wx->flags);
>  	/* Disable VMDq flag so device will be set in NM mode */
>  	if (wx->ring_feature[RING_F_VMDQ].limit == 1)
> @@ -78,6 +79,9 @@ static int __wx_enable_sriov(struct wx *wx, u8 num_vfs)
>  	set_bit(WX_FLAG_SRIOV_ENABLED, wx->flags);
>  	dev_info(&wx->pdev->dev, "SR-IOV enabled with %d VFs\n", num_vfs);
>  
> +	if (num_vfs == 7 && wx->mac.type == wx_mac_em)
> +		set_bit(WX_FLAG_IRQ_VECTOR_SHARED, wx->flags);
> +
>  	/* Enable VMDq flag so device will be set in VM mode */
>  	set_bit(WX_FLAG_VMDQ_ENABLED, wx->flags);
>  	if (!wx->ring_feature[RING_F_VMDQ].limit)
> diff --git a/drivers/net/ethernet/wangxun/libwx/wx_type.h b/drivers/net/ethernet/wangxun/libwx/wx_type.h
> index d392394791b3..c363379126c0 100644
> --- a/drivers/net/ethernet/wangxun/libwx/wx_type.h
> +++ b/drivers/net/ethernet/wangxun/libwx/wx_type.h
> @@ -1191,6 +1191,7 @@ enum wx_pf_flags {
>  	WX_FLAG_VMDQ_ENABLED,
>  	WX_FLAG_VLAN_PROMISC,
>  	WX_FLAG_SRIOV_ENABLED,
> +	WX_FLAG_IRQ_VECTOR_SHARED,
>  	WX_FLAG_FDIR_CAPABLE,
>  	WX_FLAG_FDIR_HASH,
>  	WX_FLAG_FDIR_PERFECT,
> diff --git a/drivers/net/ethernet/wangxun/ngbe/ngbe_main.c b/drivers/net/ethernet/wangxun/ngbe/ngbe_main.c
> index 68415a7ef12f..e0fc897b0a58 100644
> --- a/drivers/net/ethernet/wangxun/ngbe/ngbe_main.c
> +++ b/drivers/net/ethernet/wangxun/ngbe/ngbe_main.c
> @@ -286,7 +286,7 @@ static int ngbe_request_msix_irqs(struct wx *wx)
>  	 * for queue. But when num_vfs == 7, vector[1] is assigned to vf6.
>  	 * Misc and queue should reuse interrupt vector[0].
>  	 */
> -	if (wx->num_vfs == 7)
> +	if (test_bit(WX_FLAG_IRQ_VECTOR_SHARED, wx->flags))
>  		err = request_irq(wx->msix_entry->vector,
>  				  ngbe_misc_and_queue, 0, netdev->name, wx);
>  	else
> diff --git a/drivers/net/ethernet/wangxun/ngbe/ngbe_type.h b/drivers/net/ethernet/wangxun/ngbe/ngbe_type.h
> index 6eca6de475f7..b6252b272364 100644
> --- a/drivers/net/ethernet/wangxun/ngbe/ngbe_type.h
> +++ b/drivers/net/ethernet/wangxun/ngbe/ngbe_type.h
> @@ -87,7 +87,8 @@
>  #define NGBE_PX_MISC_IC_TIMESYNC		BIT(11) /* time sync */
>  
>  #define NGBE_INTR_ALL				0x1FF
> -#define NGBE_INTR_MISC(A)			BIT((A)->num_q_vectors)
> +#define NGBE_INTR_MISC(A)			((A)->num_vfs == 7 ? \
> +						 BIT(0) : BIT((A)->num_q_vectors))

Isn't it problematic that configuring interrupts is done in
ndo_open/ndo_stop on PF, but it depends on numvfs set in otther context.
If you start with misc on index 8 and after that set numvfs to 7 isn't
it fail?

>  
>  #define NGBE_PHY_CONFIG(reg_offset)		(0x14000 + ((reg_offset) * 4))
>  #define NGBE_CFG_LAN_SPEED			0x14440
> -- 
> 2.48.1

RE: [PATCH net v2 1/3] net: txgbe: request MISC IRQ in ndo_open

[Jiawen Wu]

On Wed, Jun 25, 2025 4:53 PM, Michal Swiatkowski wrote:
> On Tue, Jun 24, 2025 at 04:56:32PM +0800, Jiawen Wu wrote:
> > Move the creating of irq_domain for MISC IRQ from .probe to .ndo_open,
> > and free it in .ndo_stop, to maintain consistency with the queue IRQs.
> > This it for subsequent adjustments to the IRQ vectors.
> >
> > Fixes: aefd013624a1 ("net: txgbe: use irq_domain for interrupt controller")
> > Cc: stable@vger.kernel.org
> > Signed-off-by: Jiawen Wu <jiawenwu@trustnetic.com>
> > ---
> >  .../net/ethernet/wangxun/txgbe/txgbe_irq.c    |  2 +-
> >  .../net/ethernet/wangxun/txgbe/txgbe_main.c   | 22 +++++++++----------
> >  2 files changed, 11 insertions(+), 13 deletions(-)
> >
> > diff --git a/drivers/net/ethernet/wangxun/txgbe/txgbe_irq.c b/drivers/net/ethernet/wangxun/txgbe/txgbe_irq.c
> > index 20b9a28bcb55..dc468053bdf8 100644
> > --- a/drivers/net/ethernet/wangxun/txgbe/txgbe_irq.c
> > +++ b/drivers/net/ethernet/wangxun/txgbe/txgbe_irq.c
> > @@ -78,7 +78,6 @@ int txgbe_request_queue_irqs(struct wx *wx)
> >  		free_irq(wx->msix_q_entries[vector].vector,
> >  			 wx->q_vector[vector]);
> >  	}
> > -	wx_reset_interrupt_capability(wx);
> >  	return err;
> >  }
> >
> > @@ -211,6 +210,7 @@ void txgbe_free_misc_irq(struct txgbe *txgbe)
> >  	free_irq(txgbe->link_irq, txgbe);
> >  	free_irq(txgbe->misc.irq, txgbe);
> >  	txgbe_del_irq_domain(txgbe);
> > +	txgbe->wx->misc_irq_domain = false;
> >  }
> >
> >  int txgbe_setup_misc_irq(struct txgbe *txgbe)
> > diff --git a/drivers/net/ethernet/wangxun/txgbe/txgbe_main.c b/drivers/net/ethernet/wangxun/txgbe/txgbe_main.c
> > index f3d2778b8e35..a5867f3c93fc 100644
> > --- a/drivers/net/ethernet/wangxun/txgbe/txgbe_main.c
> > +++ b/drivers/net/ethernet/wangxun/txgbe/txgbe_main.c
> > @@ -458,10 +458,14 @@ static int txgbe_open(struct net_device *netdev)
> >
> >  	wx_configure(wx);
> >
> > -	err = txgbe_request_queue_irqs(wx);
> > +	err = txgbe_setup_misc_irq(wx->priv);
> 
> Don't you need misc interrupt before ndo_open is called? I wonder if it
> won't be simpler to always use last index (8) for misc irq. Is it
> possible? I mean, use 8 event if the number of queues is lower than 8.
> If yes you can drop this patch and hardcode misc interrupts on index 8.
> 
> [...]
> 
> BTW, assuming you can't always use last index, the patch looks fine.

Cannot always use index (8). Because the max RSS index is 63 for TXGBE,
but 8 for NGBE. This hardware limitation is only for NGBE. And index (8)
cannot be used by PF when SRIOV is enabled on NGBE.

In fact, the misc interrupt is only used when the device is opened.

RE: [PATCH net v2 3/3] net: ngbe: specify IRQ vector when the number of VFs is 7

[Jiawen Wu]

> > diff --git a/drivers/net/ethernet/wangxun/ngbe/ngbe_type.h b/drivers/net/ethernet/wangxun/ngbe/ngbe_type.h
> > index 6eca6de475f7..b6252b272364 100644
> > --- a/drivers/net/ethernet/wangxun/ngbe/ngbe_type.h
> > +++ b/drivers/net/ethernet/wangxun/ngbe/ngbe_type.h
> > @@ -87,7 +87,8 @@
> >  #define NGBE_PX_MISC_IC_TIMESYNC		BIT(11) /* time sync */
> >
> >  #define NGBE_INTR_ALL				0x1FF
> > -#define NGBE_INTR_MISC(A)			BIT((A)->num_q_vectors)
> > +#define NGBE_INTR_MISC(A)			((A)->num_vfs == 7 ? \
> > +						 BIT(0) : BIT((A)->num_q_vectors))
> 
> Isn't it problematic that configuring interrupts is done in
> ndo_open/ndo_stop on PF, but it depends on numvfs set in otther context.
> If you start with misc on index 8 and after that set numvfs to 7 isn't
> it fail?

When setting num_vfs, wx->setup_tc() is called to re-init the interrupt scheme.

Re: [PATCH net v2 1/3] net: txgbe: request MISC IRQ in ndo_open

[Michal Swiatkowski]

On Wed, Jun 25, 2025 at 05:22:27PM +0800, Jiawen Wu wrote:
> On Wed, Jun 25, 2025 4:53 PM, Michal Swiatkowski wrote:
> > On Tue, Jun 24, 2025 at 04:56:32PM +0800, Jiawen Wu wrote:
> > > Move the creating of irq_domain for MISC IRQ from .probe to .ndo_open,
> > > and free it in .ndo_stop, to maintain consistency with the queue IRQs.
> > > This it for subsequent adjustments to the IRQ vectors.
> > >
> > > Fixes: aefd013624a1 ("net: txgbe: use irq_domain for interrupt controller")
> > > Cc: stable@vger.kernel.org
> > > Signed-off-by: Jiawen Wu <jiawenwu@trustnetic.com>
> > > ---
> > >  .../net/ethernet/wangxun/txgbe/txgbe_irq.c    |  2 +-
> > >  .../net/ethernet/wangxun/txgbe/txgbe_main.c   | 22 +++++++++----------
> > >  2 files changed, 11 insertions(+), 13 deletions(-)
> > >
> > > diff --git a/drivers/net/ethernet/wangxun/txgbe/txgbe_irq.c b/drivers/net/ethernet/wangxun/txgbe/txgbe_irq.c
> > > index 20b9a28bcb55..dc468053bdf8 100644
> > > --- a/drivers/net/ethernet/wangxun/txgbe/txgbe_irq.c
> > > +++ b/drivers/net/ethernet/wangxun/txgbe/txgbe_irq.c
> > > @@ -78,7 +78,6 @@ int txgbe_request_queue_irqs(struct wx *wx)
> > >  		free_irq(wx->msix_q_entries[vector].vector,
> > >  			 wx->q_vector[vector]);
> > >  	}
> > > -	wx_reset_interrupt_capability(wx);
> > >  	return err;
> > >  }
> > >
> > > @@ -211,6 +210,7 @@ void txgbe_free_misc_irq(struct txgbe *txgbe)
> > >  	free_irq(txgbe->link_irq, txgbe);
> > >  	free_irq(txgbe->misc.irq, txgbe);
> > >  	txgbe_del_irq_domain(txgbe);
> > > +	txgbe->wx->misc_irq_domain = false;
> > >  }
> > >
> > >  int txgbe_setup_misc_irq(struct txgbe *txgbe)
> > > diff --git a/drivers/net/ethernet/wangxun/txgbe/txgbe_main.c b/drivers/net/ethernet/wangxun/txgbe/txgbe_main.c
> > > index f3d2778b8e35..a5867f3c93fc 100644
> > > --- a/drivers/net/ethernet/wangxun/txgbe/txgbe_main.c
> > > +++ b/drivers/net/ethernet/wangxun/txgbe/txgbe_main.c
> > > @@ -458,10 +458,14 @@ static int txgbe_open(struct net_device *netdev)
> > >
> > >  	wx_configure(wx);
> > >
> > > -	err = txgbe_request_queue_irqs(wx);
> > > +	err = txgbe_setup_misc_irq(wx->priv);
> > 
> > Don't you need misc interrupt before ndo_open is called? I wonder if it
> > won't be simpler to always use last index (8) for misc irq. Is it
> > possible? I mean, use 8 event if the number of queues is lower than 8.
> > If yes you can drop this patch and hardcode misc interrupts on index 8.
> > 
> > [...]
> > 
> > BTW, assuming you can't always use last index, the patch looks fine.
> 
> Cannot always use index (8). Because the max RSS index is 63 for TXGBE,
> but 8 for NGBE. This hardware limitation is only for NGBE. And index (8)
> cannot be used by PF when SRIOV is enabled on NGBE.
> 
> In fact, the misc interrupt is only used when the device is opened.
> 
> 

Ok, thanks
Reviewed-by: Michal Swiatkowski <michal.swiatkowski@linux.intel.com>

Re: [PATCH net v2 3/3] net: ngbe: specify IRQ vector when the number of VFs is 7

[Michal Swiatkowski]

On Wed, Jun 25, 2025 at 05:27:05PM +0800, Jiawen Wu wrote:
> > > diff --git a/drivers/net/ethernet/wangxun/ngbe/ngbe_type.h b/drivers/net/ethernet/wangxun/ngbe/ngbe_type.h
> > > index 6eca6de475f7..b6252b272364 100644
> > > --- a/drivers/net/ethernet/wangxun/ngbe/ngbe_type.h
> > > +++ b/drivers/net/ethernet/wangxun/ngbe/ngbe_type.h
> > > @@ -87,7 +87,8 @@
> > >  #define NGBE_PX_MISC_IC_TIMESYNC		BIT(11) /* time sync */
> > >
> > >  #define NGBE_INTR_ALL				0x1FF
> > > -#define NGBE_INTR_MISC(A)			BIT((A)->num_q_vectors)
> > > +#define NGBE_INTR_MISC(A)			((A)->num_vfs == 7 ? \
> > > +						 BIT(0) : BIT((A)->num_q_vectors))
> > 
> > Isn't it problematic that configuring interrupts is done in
> > ndo_open/ndo_stop on PF, but it depends on numvfs set in otther context.
> > If you start with misc on index 8 and after that set numvfs to 7 isn't
> > it fail?
> 
> When setting num_vfs, wx->setup_tc() is called to re-init the interrupt scheme.
> 

Right, maybe to be more clear you should use wx->msix_entry->entry
wherever misc index is used.

RE: [PATCH net v2 3/3] net: ngbe: specify IRQ vector when the number of VFs is 7

[Jiawen Wu]

On Wed, Jun 25, 2025 7:14 PM, Michal Swiatkowski wrote:
> On Wed, Jun 25, 2025 at 05:27:05PM +0800, Jiawen Wu wrote:
> > > > diff --git a/drivers/net/ethernet/wangxun/ngbe/ngbe_type.h b/drivers/net/ethernet/wangxun/ngbe/ngbe_type.h
> > > > index 6eca6de475f7..b6252b272364 100644
> > > > --- a/drivers/net/ethernet/wangxun/ngbe/ngbe_type.h
> > > > +++ b/drivers/net/ethernet/wangxun/ngbe/ngbe_type.h
> > > > @@ -87,7 +87,8 @@
> > > >  #define NGBE_PX_MISC_IC_TIMESYNC		BIT(11) /* time sync */
> > > >
> > > >  #define NGBE_INTR_ALL				0x1FF
> > > > -#define NGBE_INTR_MISC(A)			BIT((A)->num_q_vectors)
> > > > +#define NGBE_INTR_MISC(A)			((A)->num_vfs == 7 ? \
> > > > +						 BIT(0) : BIT((A)->num_q_vectors))
> > >
> > > Isn't it problematic that configuring interrupts is done in
> > > ndo_open/ndo_stop on PF, but it depends on numvfs set in otther context.
> > > If you start with misc on index 8 and after that set numvfs to 7 isn't
> > > it fail?
> >
> > When setting num_vfs, wx->setup_tc() is called to re-init the interrupt scheme.
> >
> 
> Right, maybe to be more clear you should use wx->msix_entry->entry
> wherever misc index is used.

Sure, thanks for your suggestion.