Hi, Player, Timmons > -----Original Message----- > From: dev [mailto:dev-boun...@dpdk.org] On Behalf Of Player, Timmons > Sent: Thursday, November 8, 2018 2:31 AM > To: Lu, Wenzhuo <wenzhuo...@intel.com> > Cc: dev@dpdk.org; Player, Timmons <timmons.pla...@spirent.com> > Subject: [dpdk-dev] [PATCH] net/igb: fix LSC interrupt when using MSI-X > > Take the 'other interrupt' into account when setting up MSI-X interrupts and > use the proper mask when enabling it. Also rearm the MSI-X vector after the > LSC interrupt fires. > > This change allows both LSC and RXQ interrupts to work at the same time. > > Signed-off-by: Timmons C. Player <timmons.pla...@spirent.com> > --- > drivers/net/e1000/igb_ethdev.c | 19 +++++++++++++++++-- > 1 file changed, 17 insertions(+), 2 deletions(-) > > diff --git a/drivers/net/e1000/igb_ethdev.c > b/drivers/net/e1000/igb_ethdev.c index d9d29d22f..62c63a623 100644 > --- a/drivers/net/e1000/igb_ethdev.c > +++ b/drivers/net/e1000/igb_ethdev.c > @@ -68,6 +68,9 @@ > #define E1000_VET_VET_EXT 0xFFFF0000 > #define E1000_VET_VET_EXT_SHIFT 16 > > +/* MSI-X other interrupt vector */ > +#define IGB_MSIX_OTHER_INTR_VEC 0 > + > static int eth_igb_configure(struct rte_eth_dev *dev); static int > eth_igb_start(struct rte_eth_dev *dev); static void eth_igb_stop(struct > rte_eth_dev *dev); @@ -540,6 +543,7 @@ igb_intr_enable(struct > rte_eth_dev *dev) > E1000_DEV_PRIVATE_TO_HW(dev->data->dev_private); > > E1000_WRITE_REG(hw, E1000_IMS, intr->mask); > + E1000_WRITE_REG(hw, E1000_EIMS, 1 << > IGB_MSIX_OTHER_INTR_VEC);
Enable of other reason interrupt is controlled by dev->data->dev_conf.intr_conf.lsc, So why not add some "if(dev->data->dev_conf.intr_conf.lsc)" judgment statement before enable it? ALL you other code has this judgement. And also, I am afraid that, this enable code to be add here is not suitable, because it is related to MSI-X mode interrupt, most of that is in function eth_igb_configure_msix_intr(), So, I suggest we can do that there with "if ()" before it. > E1000_WRITE_FLUSH(hw); > } > > @@ -2768,12 +2772,15 @@ static int eth_igb_rxq_interrupt_setup(struct > rte_eth_dev *dev) > uint32_t mask, regval; > struct e1000_hw *hw = > E1000_DEV_PRIVATE_TO_HW(dev->data->dev_private); > + struct rte_pci_device *pci_dev = RTE_ETH_DEV_TO_PCI(dev); > + struct rte_intr_handle *intr_handle = &pci_dev->intr_handle; > + int misc_shift = rte_intr_allow_others(intr_handle) ? 1 : 0; > struct rte_eth_dev_info dev_info; > > memset(&dev_info, 0, sizeof(dev_info)); > eth_igb_infos_get(dev, &dev_info); > > - mask = 0xFFFFFFFF >> (32 - dev_info.max_rx_queues); > + mask = (0xFFFFFFFF >> (32 - dev_info.max_rx_queues)) << > misc_shift; > regval = E1000_READ_REG(hw, E1000_EIMS); > E1000_WRITE_REG(hw, E1000_EIMS, regval | mask); > > @@ -5583,13 +5590,17 @@ eth_igb_configure_msix_intr(struct rte_eth_dev > *dev) > E1000_GPIE_NSICR); > intr_mask = RTE_LEN2MASK(intr_handle->nb_efd, uint32_t) > << > misc_shift; > + > + if (dev->data->dev_conf.intr_conf.lsc != 0) > + intr_mask |= (1 << IGB_MSIX_OTHER_INTR_VEC); > + > regval = E1000_READ_REG(hw, E1000_EIAC); > E1000_WRITE_REG(hw, E1000_EIAC, regval | intr_mask); > > /* enable msix_other interrupt */ > regval = E1000_READ_REG(hw, E1000_EIMS); > E1000_WRITE_REG(hw, E1000_EIMS, regval | intr_mask); > - tmpval = (dev->data->nb_rx_queues | E1000_IVAR_VALID) > << 8; > + tmpval = (IGB_MSIX_OTHER_INTR_VEC | E1000_IVAR_VALID) > << 8; > E1000_WRITE_REG(hw, E1000_IVAR_MISC, tmpval); > } > > @@ -5598,6 +5609,10 @@ eth_igb_configure_msix_intr(struct rte_eth_dev > *dev) > */ > intr_mask = RTE_LEN2MASK(intr_handle->nb_efd, uint32_t) << > misc_shift; > + > + if (dev->data->dev_conf.intr_conf.lsc != 0) > + intr_mask |= (1 << IGB_MSIX_OTHER_INTR_VEC); > + > regval = E1000_READ_REG(hw, E1000_EIAM); > E1000_WRITE_REG(hw, E1000_EIAM, regval | intr_mask); > > -- > 2.17.1
