From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga14.intel.com (mga14.intel.com [192.55.52.115]) by dpdk.org (Postfix) with ESMTP id B87235A8F for ; Thu, 29 Jan 2015 06:39:07 +0100 (CET) Received: from fmsmga002.fm.intel.com ([10.253.24.26]) by fmsmga103.fm.intel.com with ESMTP; 28 Jan 2015 21:32:53 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.09,484,1418112000"; d="scan'208";a="669515341" Received: from kmsmsx151.gar.corp.intel.com ([172.21.73.86]) by fmsmga002.fm.intel.com with ESMTP; 28 Jan 2015 21:39:04 -0800 Received: from shsmsx103.ccr.corp.intel.com (10.239.4.69) by KMSMSX151.gar.corp.intel.com (172.21.73.86) with Microsoft SMTP Server (TLS) id 14.3.195.1; Thu, 29 Jan 2015 13:39:03 +0800 Received: from shsmsx104.ccr.corp.intel.com ([169.254.5.231]) by SHSMSX103.ccr.corp.intel.com ([169.254.4.192]) with mapi id 14.03.0195.001; Thu, 29 Jan 2015 13:39:02 +0800 From: "Zhou, Danny" To: "Qiu, Michael" , "dev@dpdk.org" Thread-Topic: [dpdk-dev] [PATCH v1 2/5] ixgbe: enable rx queue interrupts for both PF and VF Thread-Index: AQHQOuARpTG4aqzXM0q3ChPwghITAZzWkgDQ Date: Thu, 29 Jan 2015 05:39:00 +0000 Message-ID: References: <1422438631-7853-1-git-send-email-danny.zhou@intel.com> <1422438631-7853-3-git-send-email-danny.zhou@intel.com> <533710CFB86FA344BFBF2D6802E60286CCEACD@SHSMSX101.ccr.corp.intel.com> In-Reply-To: <533710CFB86FA344BFBF2D6802E60286CCEACD@SHSMSX101.ccr.corp.intel.com> Accept-Language: zh-CN, en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-originating-ip: [10.239.127.40] Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 Subject: Re: [dpdk-dev] [PATCH v1 2/5] ixgbe: enable rx queue interrupts for both PF and VF X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: patches and discussions about DPDK List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 29 Jan 2015 05:39:08 -0000 > -----Original Message----- > From: Qiu, Michael > Sent: Thursday, January 29, 2015 11:40 AM > To: Zhou, Danny; dev@dpdk.org > Subject: Re: [dpdk-dev] [PATCH v1 2/5] ixgbe: enable rx queue interrupts = for both PF and VF >=20 > On 1/28/2015 5:52 PM, Danny Zhou wrote: > > Signed-off-by: Danny Zhou > > --- > > lib/librte_pmd_ixgbe/ixgbe_ethdev.c | 371 ++++++++++++++++++++++++++++= ++++++++ > > lib/librte_pmd_ixgbe/ixgbe_ethdev.h | 9 + > > 2 files changed, 380 insertions(+) > > > > diff --git a/lib/librte_pmd_ixgbe/ixgbe_ethdev.c b/lib/librte_pmd_ixgbe= /ixgbe_ethdev.c > > index b341dd0..39f883a 100644 > > --- a/lib/librte_pmd_ixgbe/ixgbe_ethdev.c > > +++ b/lib/librte_pmd_ixgbe/ixgbe_ethdev.c > > @@ -60,6 +60,7 @@ > > #include > > #include > > #include > > +#include > > #include > > > > #include "ixgbe_logs.h" > > @@ -173,6 +174,7 @@ static int ixgbe_dev_rss_reta_query(struct rte_eth_= dev *dev, > > uint16_t reta_size); > > static void ixgbe_dev_link_status_print(struct rte_eth_dev *dev); > > static int ixgbe_dev_lsc_interrupt_setup(struct rte_eth_dev *dev); > > +static int ixgbe_dev_rxq_interrupt_setup(struct rte_eth_dev *dev); > > static int ixgbe_dev_interrupt_get_status(struct rte_eth_dev *dev); > > static int ixgbe_dev_interrupt_action(struct rte_eth_dev *dev); > > static void ixgbe_dev_interrupt_handler(struct rte_intr_handle *handle= , > > @@ -186,11 +188,14 @@ static void ixgbe_dcb_init(struct ixgbe_hw *hw,st= ruct ixgbe_dcb_config *dcb_conf > > /* For Virtual Function support */ > > static int eth_ixgbevf_dev_init(struct eth_driver *eth_drv, > > struct rte_eth_dev *eth_dev); > > +static int ixgbevf_dev_interrupt_get_status(struct rte_eth_dev *dev); > > +static int ixgbevf_dev_interrupt_action(struct rte_eth_dev *dev); > > static int ixgbevf_dev_configure(struct rte_eth_dev *dev); > > static int ixgbevf_dev_start(struct rte_eth_dev *dev); > > static void ixgbevf_dev_stop(struct rte_eth_dev *dev); > > static void ixgbevf_dev_close(struct rte_eth_dev *dev); > > static void ixgbevf_intr_disable(struct ixgbe_hw *hw); > > +static void ixgbevf_intr_enable(struct ixgbe_hw *hw); > > static void ixgbevf_dev_stats_get(struct rte_eth_dev *dev, > > struct rte_eth_stats *stats); > > static void ixgbevf_dev_stats_reset(struct rte_eth_dev *dev); > > @@ -198,8 +203,15 @@ static int ixgbevf_vlan_filter_set(struct rte_eth_= dev *dev, > > uint16_t vlan_id, int on); > > static void ixgbevf_vlan_strip_queue_set(struct rte_eth_dev *dev, > > uint16_t queue, int on); > > +static void ixgbevf_set_ivar(struct ixgbe_hw *hw, s8 direction, u8 que= ue, u8 msix_vector); >=20 > ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^= ^^^^^^^^^^^^^^ > > static void ixgbevf_vlan_offload_set(struct rte_eth_dev *dev, int mask= ); > > static void ixgbevf_set_vfta_all(struct rte_eth_dev *dev, bool on); > > +static void ixgbevf_dev_interrupt_handler(struct rte_intr_handle *hand= le, > > + void *param); > > +static int ixgbevf_dev_rx_queue_intr_enable(struct rte_eth_dev *dev, u= int16_t queue_id); > > +static int ixgbevf_dev_rx_queue_intr_disable(struct rte_eth_dev *dev, = uint16_t queue_id); > > +static void ixgbevf_set_ivar(struct ixgbe_hw *hw, s8 direction, u8 que= ue, u8 msix_vector); >=20 > Yes re-claim static void ixgbevf_set_ivar() for twice? Or are they > different? >=20 Good catch. > > +static void ixgbevf_configure_msix(struct ixgbe_hw *hw); > > > > /* For Eth VMDQ APIs support */ > > static int ixgbe_uc_hash_table_set(struct rte_eth_dev *dev, struct > > @@ -217,6 +229,11 @@ static int ixgbe_mirror_rule_set(struct rte_eth_de= v *dev, > > static int ixgbe_mirror_rule_reset(struct rte_eth_dev *dev, > > uint8_t rule_id); >=20 > [...] > > +static void > > +ixgbe_configure_msix(struct ixgbe_hw *hw) > > +{ > > + int queue_id; > > + u32 mask; > > + u32 gpie; > > + > > + /* set GPIE for in MSI-x mode */ > > + gpie =3D IXGBE_READ_REG(hw, IXGBE_GPIE); > > + gpie =3D IXGBE_GPIE_MSIX_MODE | IXGBE_GPIE_PBA_SUPPORT | > > + IXGBE_GPIE_OCD; > > + gpie |=3D IXGBE_GPIE_EIAME; >=20 > As you will override gpie with other flags why need to read the reg and > save to gpie first? >=20 > Maybe read the reg to reset? >=20 > I guess should be: >=20 > + gpie =3D IXGBE_READ_REG(hw, IXGBE_GPIE); > + gpie |=3D IXGBE_GPIE_MSIX_MODE | IXGBE_GPIE_PBA_SUPPORT | > + IXGBE_GPIE_OCD | IXGBE_GPIE_EIAME; >=20 > Maybe not correct as I not familiar with IXGBE. >=20 >=20 Accepted.=20 > > + /* > > + * use EIAM to auto-mask when MSI-X interrupt is asserted > > + * this saves a register write for every interrupt > > + */ > > + switch (hw->mac.type) { > > + case ixgbe_mac_82598EB: > > + IXGBE_WRITE_REG(hw, IXGBE_EIAM, IXGBE_EICS_RTX_QUEUE); > > + break; > > + case ixgbe_mac_82599EB: > > + case ixgbe_mac_X540: > > + default: > > + IXGBE_WRITE_REG(hw, IXGBE_EIAM_EX(0), 0xFFFFFFFF); > > + IXGBE_WRITE_REG(hw, IXGBE_EIAM_EX(1), 0xFFFFFFFF); > > + break; > > + } > > + IXGBE_WRITE_REG(hw, IXGBE_GPIE, gpie); > > + > > + /* > > + * Populate the IVAR table and set the ITR values to the > > + * corresponding register. > > + */ > > + for (queue_id =3D 0; queue_id < VFIO_MAX_QUEUE_ID; queue_id++) > > + ixgbe_set_ivar(hw, 0, queue_id, queue_id); > > + > > + switch (hw->mac.type) { > > + case ixgbe_mac_82598EB: > > + ixgbe_set_ivar(hw, -1, IXGBE_IVAR_OTHER_CAUSES_INDEX, > > + VFIO_MAX_QUEUE_ID); > > + break; > > + case ixgbe_mac_82599EB: > > + case ixgbe_mac_X540: > > + ixgbe_set_ivar(hw, -1, 1, 32); >=20 > May be better to make those values for a macro just as above. >=20 To be fixed in V2. > > + break; > > + default: > > + break; > > + } > > + IXGBE_WRITE_REG(hw, IXGBE_EITR(queue_id), 1950); >=20 > Also here, what's "1950" stands for? >=20 It is an experienced interrupt throttle value which is used for interrupt s= upport > > + > > + /* set up to autoclear timer, and the vectors */ > > + mask =3D IXGBE_EIMS_ENABLE_MASK; > > + mask &=3D ~(IXGBE_EIMS_OTHER | > > + IXGBE_EIMS_MAILBOX | > > + IXGBE_EIMS_LSC); > > + > > + IXGBE_WRITE_REG(hw, IXGBE_EIAC, mask); > > +} > > + > > static int ixgbe_set_queue_rate_limit(struct rte_eth_dev *dev, > > uint16_t queue_idx, uint16_t tx_rate) > > { > > diff --git a/lib/librte_pmd_ixgbe/ixgbe_ethdev.h b/lib/librte_pmd_ixgbe= /ixgbe_ethdev.h > > index 1383194..328c387 100644 > > --- a/lib/librte_pmd_ixgbe/ixgbe_ethdev.h > > +++ b/lib/librte_pmd_ixgbe/ixgbe_ethdev.h > > @@ -38,6 +38,8 @@ > > #include "ixgbe/ixgbe_dcb_82598.h" > > #include "ixgbe_bypass.h" > > > > +#include > > + > > /* need update link, bit flag */ > > #define IXGBE_FLAG_NEED_LINK_UPDATE (uint32_t)(1 << 0) > > #define IXGBE_FLAG_MAILBOX (uint32_t)(1 << 1) > > @@ -98,6 +100,11 @@ > > #define IXGBE_5TUPLE_MAX_PRI 7 > > #define IXGBE_5TUPLE_MIN_PRI 1 > > > > +#define IXGBE_VF_IRQ_ENABLE_MASK 3 /* vf interrupt ena= ble mask */ > > +#define IXGBE_VF_MAXMSIVECTOR 1 > > +/* maximum other interrupts besides rx&tx*/ > > +#define IXGBE_MAX_OTHER_INTR 1 > > +#define IXGBEVF_MAX_OTHER_INTR 1 > > /* > > * Information about the fdir mode. > > */ > > @@ -116,6 +123,7 @@ struct ixgbe_hw_fdir_info { > > struct ixgbe_interrupt { > > uint32_t flags; > > uint32_t mask; > > + rte_spinlock_t lock; > > }; > > > > struct ixgbe_stat_mapping_registers { > > @@ -260,6 +268,7 @@ uint32_t ixgbe_dev_rx_queue_count(struct rte_eth_de= v *dev, > > uint16_t rx_queue_id); > > > > int ixgbe_dev_rx_descriptor_done(void *rx_queue, uint16_t offset); > > +int ixgbevf_dev_rx_descriptor_done(void *rx_queue, uint16_t offset); > > > > int ixgbe_dev_rx_init(struct rte_eth_dev *dev); > >