net/e1000: enable MSI-x for linkup and other events in 82574

PATCH Applied Luca Boccassi 3 participants 7 messages Jul 27, 2018 – Oct 10, 2018 Network drivers

Share & tools

Luca Boccassi

author

2018-07-27 18:13 UTC #0 lore raw

PATCH 1/1 net/e1000: enable MSI-x for linkup and other events in 82574

Linux supports 82574 MSI-x interrupt for linkup and other link conditions. Enable the same feature in the e1000 PMD in order to allow this card to work properly, as it requires interrupt handling for link negotiations among other things.

Signed-off-by Luca Boccassi <[email protected]>

---

Hi Wenzhuo - just to make it clear, this patch and the previous are not targeted for 18.08, just clearing my queue.

 drivers/net/e1000/base/e1000_82571.c | 5 +++++
 drivers/net/e1000/em_ethdev.c        | 5 +++--
 2 files changed, 8 insertions(+), 2 deletions(-)

drivers/net/e1000/base/e1000_82571.c +5 -0

index 7c279dbb3..397dd9482 100644
@@ -1257,6 +1257,11 @@ STATIC s32 e1000_init_hw_82571(struct e1000_hw *hw)
 	 */
 	e1000_clear_hw_cntrs_82571(hw);
 
+	/* MSI-X configure for 82574 */
+	if (mac->type == e1000_82574)
+		E1000_WRITE_REG(hw, E1000_IVAR,
+				(E1000_IVAR_INT_ALLOC_VALID << 16));
+
 	return ret_val;
 }

drivers/net/e1000/em_ethdev.c +3 -2

index 7039dc100..bd05e4de0 100644
@@ -1444,7 +1444,8 @@ eth_em_interrupt_setup(struct rte_eth_dev *dev)
 	/* clear interrupt */
 	E1000_READ_REG(hw, E1000_ICR);
 	regval = E1000_READ_REG(hw, E1000_IMS);
-	E1000_WRITE_REG(hw, E1000_IMS, regval | E1000_ICR_LSC);
+	E1000_WRITE_REG(hw, E1000_IMS,
+			regval | E1000_ICR_LSC | E1000_ICR_OTHER);
 	return 0;
 }

@@ -1494,7 +1495,7 @@ em_rxq_intr_enable(struct e1000_hw *hw)
 static void
 em_lsc_intr_disable(struct e1000_hw *hw)
 {
-	E1000_WRITE_REG(hw, E1000_IMC, E1000_IMS_LSC);
+	E1000_WRITE_REG(hw, E1000_IMC, E1000_IMS_LSC | E1000_IMS_OTHER);
 	E1000_WRITE_FLUSH(hw);
 }

Signature · 1 line

2.18.0

Luca Boccassi

author

2018-09-26 10:23 UTC #1 lore raw

Quoted 6 lines · Luca Boccassi

Linux supports 82574 MSI-x interrupt for linkup and other link conditions. Enable the same feature in the e1000 PMD in order to allow this card to work properly, as it requires interrupt handling for link negotiations among other things.
Signed-off-by Luca Boccassi <[email protected]>

Quoted 8 lines

---
Hi Wenzhuo - just to make it clear, this patch and the previous are not targeted for 18.08, just clearing my queue.
drivers/net/e1000/base/e1000_82571.c | 5 +++++
drivers/net/e1000/em_ethdev.c | 5 +++--
 2 files changed, 8 insertions(+), 2 deletions(-)

Ping. Any chance this can get looked at? Thanks!

Signature · 2 lines

Kind regards,
Luca Boccassi

Zhao1, Wei

2018-10-09 10:16 UTC #2 lore raw

In reply to Luca Boccassi

Hi, Luca Boccassi

A question for this feature, in em code, function eth_em_interrupt_handler() we have no process for handle other cause interrupt.
So, why do we enable this OTHER cause interrupt in DPDK?
You mean some application may take advance of this interrupt for some unknown purpose when using DPDK?

Quoted 66 lines

-----Original Message-----
From: dev [mailto:[email protected]] On Behalf Of Luca Boccassi
Sent: Saturday, July 28, 2018 2:13 AM
To: [email protected]
Cc: Lu, Wenzhuo <[email protected]>; Luca Boccassi <[email protected]>
Subject: [dpdk-dev] [PATCH] net/e1000: enable MSI-x for linkup and other events in 82574
Linux supports 82574 MSI-x interrupt for linkup and other link conditions. Enable the same feature in the e1000 PMD in order to allow this card to work properly, as it requires interrupt handling for link negotiations among other things.
Signed-off-by Luca Boccassi <[email protected]>
---
Hi Wenzhuo - just to make it clear, this patch and the previous are not targeted for 18.08, just clearing my queue.
 drivers/net/e1000/base/e1000_82571.c | 5 +++++
 drivers/net/e1000/em_ethdev.c        | 5 +++--
 2 files changed, 8 insertions(+), 2 deletions(-)
drivers/net/e1000/base/e1000_82571.c +5 -0
index 7c279dbb3..397dd9482 100644
@@ -1257,6 +1257,11 @@ STATIC s32 e1000_init_hw_82571(struct e1000_hw
 *hw)
 	 */
 	e1000_clear_hw_cntrs_82571(hw);
 
+	/* MSI-X configure for 82574 */
+	if (mac->type == e1000_82574)
+		E1000_WRITE_REG(hw, E1000_IVAR,
+				(E1000_IVAR_INT_ALLOC_VALID << 16));
+
 	return ret_val;
 }
(quoted diff) +0 -0
b/drivers/net/e1000/em_ethdev.c index 7039dc100..bd05e4de0 100644
drivers/net/e1000/em_ethdev.c +3 -3
@@ -1444,7 +1444,8 @@ eth_em_interrupt_setup(struct rte_eth_dev *dev)
 	/* clear interrupt */
 	E1000_READ_REG(hw, E1000_ICR);
 	regval = E1000_READ_REG(hw, E1000_IMS);
-	E1000_WRITE_REG(hw, E1000_IMS, regval | E1000_ICR_LSC);
+	E1000_WRITE_REG(hw, E1000_IMS,
+			regval | E1000_ICR_LSC | E1000_ICR_OTHER);
 	return 0;
 }
 
@@ -1494,7 +1495,7 @@ em_rxq_intr_enable(struct e1000_hw *hw)  static
 void  em_lsc_intr_disable(struct e1000_hw *hw)  {
-	E1000_WRITE_REG(hw, E1000_IMC, E1000_IMS_LSC);
+	E1000_WRITE_REG(hw, E1000_IMC, E1000_IMS_LSC |
 E1000_IMS_OTHER);
 	E1000_WRITE_FLUSH(hw);
 }
 
--
2.18.0

Luca Boccassi

author

2018-10-09 10:32 UTC #3 lore raw

Quoted 8 lines · Wei

Hi, Luca Boccassi
A question for this feature, in em code, function eth_em_interrupt_handler() we have no process for handle other cause interrupt.
So, why do we enable this OTHER cause interrupt in DPDK? You mean some application may take advance of this interrupt for some unknown purpose when using DPDK?

Hi,

This MSI-x interrupt is for link state change, which is already handled by the driver.
As mentioned, without this link state change for the 82574 NIC does not work.

Quoted 79 lines

Quoted 75 lines
-----Original Message-----
From: dev [mailto:[email protected]] On Behalf Of Luca Boccassi
Sent: Saturday, July 28, 2018 2:13 AM
To: [email protected]
Cc: Lu, Wenzhuo <[email protected]>; Luca Boccassi <[email protected]>
Subject: [dpdk-dev] [PATCH] net/e1000: enable MSI-x for linkup and other
events in 82574
Linux supports 82574 MSI-x interrupt for linkup and other link conditions.
Enable the same feature in the e1000 PMD in order to allow this card to work
properly, as it requires interrupt handling for link negotiations among other
things.
Signed-off-by Luca Boccassi <[email protected]>
---
Hi Wenzhuo - just to make it clear, this patch and the previous are not
targeted for 18.08, just clearing my queue.
drivers/net/e1000/base/e1000_82571.c | 5 +++++
drivers/net/e1000/em_ethdev.c | 5 +++--
 2 files changed, 8 insertions(+), 2 deletions(-)
drivers/net/e1000/base/e1000_82571.c +0 -0
index 7c279dbb3..397dd9482 100644
@@ -1257,6 +1257,11 @@ STATIC s32 e1000_init_hw_82571(struct
e1000_hw
*hw)
*/
e1000_clear_hw_cntrs_82571(hw);
drivers/net/e1000/base/e1000_82571.c +4 -0
+	/* MSI-X configure for 82574 */
+	if (mac->type == e1000_82574)
+		E1000_WRITE_REG(hw, E1000_IVAR,
+				(E1000_IVAR_INT_ALLOC_VALID <<
16));
+
return ret_val;
}
(quoted diff) +0 -0
b/drivers/net/e1000/em_ethdev.c index 7039dc100..bd05e4de0 100644
drivers/net/e1000/em_ethdev.c +3 -3
@@ -1444,7 +1444,8 @@ eth_em_interrupt_setup(struct rte_eth_dev
 *dev)
  	/* clear interrupt */
  	E1000_READ_REG(hw, E1000_ICR);
  	regval = E1000_READ_REG(hw, E1000_IMS);
-	E1000_WRITE_REG(hw, E1000_IMS, regval | E1000_ICR_LSC);
+	E1000_WRITE_REG(hw, E1000_IMS,
+			regval | E1000_ICR_LSC | E1000_ICR_OTHER);
  	return 0;
  }
@@ -1494,7 +1495,7 @@ em_rxq_intr_enable(struct e1000_hw
 *hw)  static
 void  em_lsc_intr_disable(struct e1000_hw *hw)  {
-	E1000_WRITE_REG(hw, E1000_IMC, E1000_IMS_LSC);
+	E1000_WRITE_REG(hw, E1000_IMC, E1000_IMS_LSC |
 E1000_IMS_OTHER);
  	E1000_WRITE_FLUSH(hw);
  }
 
--
2.18.0

Signature · 2 lines

Kind regards,
Luca Boccassi

Zhao1, Wei

Acked-by

2018-10-10 07:13 UTC #4 lore raw

Acked-by Wei Zhao <[email protected]>

Quoted 104 lines

-----Original Message-----
From: Luca Boccassi [mailto:[email protected]]
Sent: Tuesday, October 9, 2018 6:33 PM
To: Zhao1, Wei <[email protected]>; [email protected]
Cc: Lu, Wenzhuo <[email protected]>; Zhang, Qi Z <[email protected]>
Subject: Re: [dpdk-dev] [PATCH] net/e1000: enable MSI-x for linkup and other events in 82574
Quoted 8 lines · Wei
Hi, Luca Boccassi
A question for this feature, in em code, function eth_em_interrupt_handler() we have no process for handle other cause interrupt.
So, why do we enable this OTHER cause interrupt in DPDK? You mean some application may take advance of this interrupt for some unknown purpose when using DPDK?
Hi,
This MSI-x interrupt is for link state change, which is already handled by the driver.
As mentioned, without this link state change for the 82574 NIC does not work.
Quoted 71 lines
Quoted 71 lines
-----Original Message-----
From: dev [mailto:[email protected]] On Behalf Of Luca Boccassi
Sent: Saturday, July 28, 2018 2:13 AM
To: [email protected]
Cc: Lu, Wenzhuo <[email protected]>; Luca Boccassi <[email protected]>
Subject: [dpdk-dev] [PATCH] net/e1000: enable MSI-x for linkup and other events in 82574
Linux supports 82574 MSI-x interrupt for linkup and other link conditions.
Enable the same feature in the e1000 PMD in order to allow this card to work properly, as it requires interrupt handling for link negotiations among other things.
Signed-off-by Luca Boccassi <[email protected]>
---
Hi Wenzhuo - just to make it clear, this patch and the previous are not targeted for 18.08, just clearing my queue.
drivers/net/e1000/base/e1000_82571.c | 5 +++++
drivers/net/e1000/em_ethdev.c | 5 +++--
 2 files changed, 8 insertions(+), 2 deletions(-)
drivers/net/e1000/base/e1000_82571.c +0 -0
index 7c279dbb3..397dd9482 100644
@@ -1257,6 +1257,11 @@ STATIC s32 e1000_init_hw_82571(struct
e1000_hw
*hw)
*/
e1000_clear_hw_cntrs_82571(hw);
drivers/net/e1000/base/e1000_82571.c +4 -0
+	/* MSI-X configure for 82574 */
+	if (mac->type == e1000_82574)
+		E1000_WRITE_REG(hw, E1000_IVAR,
+				(E1000_IVAR_INT_ALLOC_VALID <<
16));
+
return ret_val;
}
(quoted diff) +0 -0
b/drivers/net/e1000/em_ethdev.c index 7039dc100..bd05e4de0 100644
drivers/net/e1000/em_ethdev.c +3 -2
@@ -1444,7 +1444,8 @@ eth_em_interrupt_setup(struct rte_eth_dev
 *dev)
  	/* clear interrupt */
  	E1000_READ_REG(hw, E1000_ICR);
  	regval = E1000_READ_REG(hw, E1000_IMS);
-	E1000_WRITE_REG(hw, E1000_IMS, regval | E1000_ICR_LSC);
+	E1000_WRITE_REG(hw, E1000_IMS,
+			regval | E1000_ICR_LSC | E1000_ICR_OTHER);
  	return 0;
  }
@@ -1494,7 +1495,7 @@ em_rxq_intr_enable(struct e1000_hw
 *hw)  static
 void  em_lsc_intr_disable(struct e1000_hw *hw)  {
-	E1000_WRITE_REG(hw, E1000_IMC, E1000_IMS_LSC);
+	E1000_WRITE_REG(hw, E1000_IMC, E1000_IMS_LSC |
 E1000_IMS_OTHER);
  	E1000_WRITE_FLUSH(hw);
  }
 
Quoted 4 lines
Quoted 2 lines
--
2.18.0
--
Kind regards,
Luca Boccassi

Zhang, Qi Z

Applied

2018-10-10 18:01 UTC #5 lore raw

Quoted 11 lines

-----Original Message-----
From: Zhao1, Wei
Sent: Wednesday, October 10, 2018 12:14 AM
To: Luca Boccassi <[email protected]>; [email protected]
Cc: Lu, Wenzhuo <[email protected]>; Zhang, Qi Z <[email protected]>
Subject: RE: [dpdk-dev] [PATCH] net/e1000: enable MSI-x for linkup and other events in 82574

Quoted 1 line

Acked-by Wei Zhao <[email protected]>

Applied to dpdk-next-net-intel.

Thanks
Qi

Quoted 107 lines

Quoted 20 lines
-----Original Message-----
From: Luca Boccassi [mailto:[email protected]]
Sent: Tuesday, October 9, 2018 6:33 PM
To: Zhao1, Wei <[email protected]>; [email protected]
Cc: Lu, Wenzhuo <[email protected]>; Zhang, Qi Z <[email protected]>
Subject: Re: [dpdk-dev] [PATCH] net/e1000: enable MSI-x for linkup and other events in 82574
Quoted 8 lines · Wei
Hi, Luca Boccassi
A question for this feature, in em code, function eth_em_interrupt_handler() we have no process for handle other cause interrupt.
So, why do we enable this OTHER cause interrupt in DPDK? You mean some application may take advance of this interrupt for some unknown purpose when using DPDK?
Quoted 5 lines
Hi,
This MSI-x interrupt is for link state change, which is already handled by the driver.
As mentioned, without this link state change for the 82574 NIC does not
work.
Quoted 48 lines
Quoted 48 lines
Quoted 48 lines
-----Original Message-----
From: dev [mailto:[email protected]] On Behalf Of Luca Boccassi
Sent: Saturday, July 28, 2018 2:13 AM
To: [email protected]
Cc: Lu, Wenzhuo <[email protected]>; Luca Boccassi <[email protected]>
Subject: [dpdk-dev] [PATCH] net/e1000: enable MSI-x for linkup and other events in 82574
Linux supports 82574 MSI-x interrupt for linkup and other link conditions.
Enable the same feature in the e1000 PMD in order to allow this card to work properly, as it requires interrupt handling for link negotiations among other things.
Signed-off-by Luca Boccassi <[email protected]>
---
Hi Wenzhuo - just to make it clear, this patch and the previous are not targeted for 18.08, just clearing my queue.
drivers/net/e1000/base/e1000_82571.c | 5 +++++
drivers/net/e1000/em_ethdev.c | 5 +++--
 2 files changed, 8 insertions(+), 2 deletions(-)
drivers/net/e1000/base/e1000_82571.c +0 -0
index 7c279dbb3..397dd9482 100644
@@ -1257,6 +1257,11 @@ STATIC s32 e1000_init_hw_82571(struct
e1000_hw
*hw)
*/
e1000_clear_hw_cntrs_82571(hw);
drivers/net/e1000/base/e1000_82571.c +4 -0
+	/* MSI-X configure for 82574 */
+	if (mac->type == e1000_82574)
+		E1000_WRITE_REG(hw, E1000_IVAR,
+				(E1000_IVAR_INT_ALLOC_VALID <<
16));
+
return ret_val;
}
Quoted 2 lines
Quoted 2 lines
Quoted 2 lines
(quoted diff) +0 -0
b/drivers/net/e1000/em_ethdev.c index 7039dc100..bd05e4de0
100644
Quoted 30 lines
Quoted 22 lines
Quoted 22 lines
drivers/net/e1000/em_ethdev.c +3 -2
@@ -1444,7 +1444,8 @@ eth_em_interrupt_setup(struct rte_eth_dev
 *dev)
  	/* clear interrupt */
  	E1000_READ_REG(hw, E1000_ICR);
  	regval = E1000_READ_REG(hw, E1000_IMS);
-	E1000_WRITE_REG(hw, E1000_IMS, regval | E1000_ICR_LSC);
+	E1000_WRITE_REG(hw, E1000_IMS,
+			regval | E1000_ICR_LSC | E1000_ICR_OTHER);
  	return 0;
  }
@@ -1494,7 +1495,7 @@ em_rxq_intr_enable(struct e1000_hw
 *hw)  static
 void  em_lsc_intr_disable(struct e1000_hw *hw)  {
-	E1000_WRITE_REG(hw, E1000_IMC, E1000_IMS_LSC);
+	E1000_WRITE_REG(hw, E1000_IMC, E1000_IMS_LSC |
 E1000_IMS_OTHER);
  	E1000_WRITE_FLUSH(hw);
  }
 
Quoted 4 lines
Quoted 2 lines
--
2.18.0
--
Kind regards,
Luca Boccassi

Zhao1, Wei

2018-10-09 10:29 UTC #6 lore raw

In reply to Luca Boccassi

As datasheet list:

Other Interrupt. Indicates one of the following interrupts was set:
* Link Status Change.
* Receiver Overrun.
* MDIO Access Complete.
* Small Receive Packet Detected.
* Receive ACK Frame Detected.
* Manageability Event Detected.

Which is need when using DPDK?

Quoted 89 lines

-----Original Message-----
From: Zhao1, Wei
Sent: Tuesday, October 9, 2018 6:17 PM
To: 'Luca Boccassi' <[email protected]>; [email protected]
Cc: Lu, Wenzhuo <[email protected]>; Zhang, Qi Z <[email protected]>
Subject: RE: [dpdk-dev] [PATCH] net/e1000: enable MSI-x for linkup and other events in 82574
Hi, Luca Boccassi
A question for this feature, in em code, function
eth_em_interrupt_handler() we have no process for handle other cause interrupt.
So, why do we enable this OTHER cause interrupt in DPDK? You mean some application may take advance of this interrupt for some unknown purpose when using DPDK?
drivers/net/e1000/base/e1000_82571.c STATIC s32 e1000_init_hw_82571(struct
Quoted 30 lines
-----Original Message-----
From: dev [mailto:[email protected]] On Behalf Of Luca Boccassi
Sent: Saturday, July 28, 2018 2:13 AM
To: [email protected]
Cc: Lu, Wenzhuo <[email protected]>; Luca Boccassi <[email protected]>
Subject: [dpdk-dev] [PATCH] net/e1000: enable MSI-x for linkup and other events in 82574
Linux supports 82574 MSI-x interrupt for linkup and other link conditions. Enable the same feature in the e1000 PMD in order to allow this card to work properly, as it requires interrupt handling for link negotiations among other things.
Signed-off-by Luca Boccassi <[email protected]>
---
Hi Wenzhuo - just to make it clear, this patch and the previous are not targeted for 18.08, just clearing my queue.
 drivers/net/e1000/base/e1000_82571.c | 5 +++++
 drivers/net/e1000/em_ethdev.c        | 5 +++--
 2 files changed, 8 insertions(+), 2 deletions(-)
drivers/net/e1000/base/e1000_82571.c +0 -0
index 7c279dbb3..397dd9482 100644
@@ -1257,6 +1257,11 @@ STATIC s32 e1000_init_hw_82571(struct
e1000_hw
drivers/net/e1000/em_ethdev.c eth_em_interrupt_setup(struct rte_eth_dev
Quoted 17 lines
drivers/net/e1000/base/e1000_82571.c +5 -0
 *hw)
 	 */
 	e1000_clear_hw_cntrs_82571(hw);
 
+	/* MSI-X configure for 82574 */
+	if (mac->type == e1000_82574)
+		E1000_WRITE_REG(hw, E1000_IVAR,
+				(E1000_IVAR_INT_ALLOC_VALID << 16));
+
 	return ret_val;
 }
(quoted diff) +0 -0
b/drivers/net/e1000/em_ethdev.c index 7039dc100..bd05e4de0 100644
drivers/net/e1000/em_ethdev.c +0 -0
@@ -1444,7 +1444,8 @@ eth_em_interrupt_setup(struct rte_eth_dev
*dev)
drivers/net/e1000/em_ethdev.c em_rxq_intr_enable(struct e1000_hw *hw)
Quoted 10 lines
drivers/net/e1000/em_ethdev.c +2 -1
 	/* clear interrupt */
 	E1000_READ_REG(hw, E1000_ICR);
 	regval = E1000_READ_REG(hw, E1000_IMS);
-	E1000_WRITE_REG(hw, E1000_IMS, regval | E1000_ICR_LSC);
+	E1000_WRITE_REG(hw, E1000_IMS,
+			regval | E1000_ICR_LSC | E1000_ICR_OTHER);
 	return 0;
 }
 
@@ -1494,7 +1495,7 @@ em_rxq_intr_enable(struct e1000_hw *hw)
static
Quoted 9 lines
drivers/net/e1000/em_ethdev.c +1 -2
 void  em_lsc_intr_disable(struct e1000_hw *hw)  {
-	E1000_WRITE_REG(hw, E1000_IMC, E1000_IMS_LSC);
+	E1000_WRITE_REG(hw, E1000_IMC, E1000_IMS_LSC |
 E1000_IMS_OTHER);
 	E1000_WRITE_FLUSH(hw);
 }
 
--
2.18.0