From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga03.intel.com (mga03.intel.com [134.134.136.65]) by dpdk.org (Postfix) with ESMTP id 6C0DC1F5 for ; Thu, 29 Jan 2015 11:13:18 +0100 (CET) Received: from orsmga003.jf.intel.com ([10.7.209.27]) by orsmga103.jf.intel.com with ESMTP; 29 Jan 2015 02:08:54 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.09,485,1418112000"; d="scan'208";a="519553963" Received: from pgsmsx104.gar.corp.intel.com ([10.221.44.91]) by orsmga003.jf.intel.com with ESMTP; 29 Jan 2015 02:06:01 -0800 Received: from shsmsx152.ccr.corp.intel.com (10.239.6.52) by PGSMSX104.gar.corp.intel.com (10.221.44.91) with Microsoft SMTP Server (TLS) id 14.3.195.1; Thu, 29 Jan 2015 18:13:13 +0800 Received: from shsmsx101.ccr.corp.intel.com ([169.254.1.253]) by SHSMSX152.ccr.corp.intel.com ([169.254.6.129]) with mapi id 14.03.0195.001; Thu, 29 Jan 2015 18:13:12 +0800 From: "Qiu, Michael" To: "Zhou, Danny" , "dev@dpdk.org" Thread-Topic: [dpdk-dev] [PATCH v1 2/5] ixgbe: enable rx queue interrupts for both PF and VF Thread-Index: AQHQOuAR7rjsOEuA7UeVj6BS0t03Xg== Date: Thu, 29 Jan 2015 10:13:12 +0000 Message-ID: <533710CFB86FA344BFBF2D6802E60286CCFDD2@SHSMSX101.ccr.corp.intel.com> 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> Accept-Language: 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 10:13:19 -0000 On 1/29/2015 1:39 PM, Zhou, Danny wrote:=0A= >=0A= >> -----Original Message-----=0A= >> From: Qiu, Michael=0A= >> Sent: Thursday, January 29, 2015 11:40 AM=0A= >> To: Zhou, Danny; dev@dpdk.org=0A= >> Subject: Re: [dpdk-dev] [PATCH v1 2/5] ixgbe: enable rx queue interrupts= for both PF and VF=0A= >>=0A= >> On 1/28/2015 5:52 PM, Danny Zhou wrote:=0A= >>> Signed-off-by: Danny Zhou =0A= >>> ---=0A= >>> lib/librte_pmd_ixgbe/ixgbe_ethdev.c | 371 ++++++++++++++++++++++++++++= ++++++++=0A= >>> lib/librte_pmd_ixgbe/ixgbe_ethdev.h | 9 +=0A= >>> 2 files changed, 380 insertions(+)=0A= >>>=0A= >>> diff --git a/lib/librte_pmd_ixgbe/ixgbe_ethdev.c b/lib/librte_pmd_ixgbe= /ixgbe_ethdev.c=0A= >>> index b341dd0..39f883a 100644=0A= >>> --- a/lib/librte_pmd_ixgbe/ixgbe_ethdev.c=0A= >>> +++ b/lib/librte_pmd_ixgbe/ixgbe_ethdev.c=0A= >>> @@ -60,6 +60,7 @@=0A= >>> #include =0A= >>> #include =0A= >>> #include =0A= >>> +#include =0A= >>> #include =0A= >>>=0A= >>> #include "ixgbe_logs.h"=0A= >>> @@ -173,6 +174,7 @@ static int ixgbe_dev_rss_reta_query(struct rte_eth_= dev *dev,=0A= >>> uint16_t reta_size);=0A= >>> static void ixgbe_dev_link_status_print(struct rte_eth_dev *dev);=0A= >>> static int ixgbe_dev_lsc_interrupt_setup(struct rte_eth_dev *dev);=0A= >>> +static int ixgbe_dev_rxq_interrupt_setup(struct rte_eth_dev *dev);=0A= >>> static int ixgbe_dev_interrupt_get_status(struct rte_eth_dev *dev);=0A= >>> static int ixgbe_dev_interrupt_action(struct rte_eth_dev *dev);=0A= >>> static void ixgbe_dev_interrupt_handler(struct rte_intr_handle *handle= ,=0A= >>> @@ -186,11 +188,14 @@ static void ixgbe_dcb_init(struct ixgbe_hw *hw,st= ruct ixgbe_dcb_config *dcb_conf=0A= >>> /* For Virtual Function support */=0A= >>> static int eth_ixgbevf_dev_init(struct eth_driver *eth_drv,=0A= >>> struct rte_eth_dev *eth_dev);=0A= >>> +static int ixgbevf_dev_interrupt_get_status(struct rte_eth_dev *dev);= =0A= >>> +static int ixgbevf_dev_interrupt_action(struct rte_eth_dev *dev);=0A= >>> static int ixgbevf_dev_configure(struct rte_eth_dev *dev);=0A= >>> static int ixgbevf_dev_start(struct rte_eth_dev *dev);=0A= >>> static void ixgbevf_dev_stop(struct rte_eth_dev *dev);=0A= >>> static void ixgbevf_dev_close(struct rte_eth_dev *dev);=0A= >>> static void ixgbevf_intr_disable(struct ixgbe_hw *hw);=0A= >>> +static void ixgbevf_intr_enable(struct ixgbe_hw *hw);=0A= >>> static void ixgbevf_dev_stats_get(struct rte_eth_dev *dev,=0A= >>> struct rte_eth_stats *stats);=0A= >>> static void ixgbevf_dev_stats_reset(struct rte_eth_dev *dev);=0A= >>> @@ -198,8 +203,15 @@ static int ixgbevf_vlan_filter_set(struct rte_eth_= dev *dev,=0A= >>> uint16_t vlan_id, int on);=0A= >>> static void ixgbevf_vlan_strip_queue_set(struct rte_eth_dev *dev,=0A= >>> uint16_t queue, int on);=0A= >>> +static void ixgbevf_set_ivar(struct ixgbe_hw *hw, s8 direction, u8 que= ue, u8 msix_vector);=0A= >> ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^= ^^^^^^^^^^^^^^^=0A= >>> static void ixgbevf_vlan_offload_set(struct rte_eth_dev *dev, int mask= );=0A= >>> static void ixgbevf_set_vfta_all(struct rte_eth_dev *dev, bool on);=0A= >>> +static void ixgbevf_dev_interrupt_handler(struct rte_intr_handle *hand= le,=0A= >>> + void *param);=0A= >>> +static int ixgbevf_dev_rx_queue_intr_enable(struct rte_eth_dev *dev, u= int16_t queue_id);=0A= >>> +static int ixgbevf_dev_rx_queue_intr_disable(struct rte_eth_dev *dev, = uint16_t queue_id);=0A= >>> +static void ixgbevf_set_ivar(struct ixgbe_hw *hw, s8 direction, u8 que= ue, u8 msix_vector);=0A= >> Yes re-claim static void ixgbevf_set_ivar() for twice? Or are they=0A= >> different?=0A= >>=0A= > Good catch.=0A= >=0A= >>> +static void ixgbevf_configure_msix(struct ixgbe_hw *hw);=0A= >>>=0A= >>> /* For Eth VMDQ APIs support */=0A= >>> static int ixgbe_uc_hash_table_set(struct rte_eth_dev *dev, struct=0A= >>> @@ -217,6 +229,11 @@ static int ixgbe_mirror_rule_set(struct rte_eth_de= v *dev,=0A= >>> static int ixgbe_mirror_rule_reset(struct rte_eth_dev *dev,=0A= >>> uint8_t rule_id);=0A= >> [...]=0A= >>> +static void=0A= >>> +ixgbe_configure_msix(struct ixgbe_hw *hw)=0A= >>> +{=0A= >>> + int queue_id;=0A= >>> + u32 mask;=0A= >>> + u32 gpie;=0A= >>> +=0A= >>> + /* set GPIE for in MSI-x mode */=0A= >>> + gpie =3D IXGBE_READ_REG(hw, IXGBE_GPIE);=0A= >>> + gpie =3D IXGBE_GPIE_MSIX_MODE | IXGBE_GPIE_PBA_SUPPORT |=0A= >>> + IXGBE_GPIE_OCD;=0A= >>> + gpie |=3D IXGBE_GPIE_EIAME;=0A= >> As you will override gpie with other flags why need to read the reg and= =0A= >> save to gpie first?=0A= >>=0A= >> Maybe read the reg to reset?=0A= >>=0A= >> I guess should be:=0A= >>=0A= >> + gpie =3D IXGBE_READ_REG(hw, IXGBE_GPIE);=0A= >> + gpie |=3D IXGBE_GPIE_MSIX_MODE | IXGBE_GPIE_PBA_SUPPORT |=0A= >> + IXGBE_GPIE_OCD | IXGBE_GPIE_EIAME;=0A= >>=0A= >> Maybe not correct as I not familiar with IXGBE.=0A= >>=0A= >>=0A= > Accepted. =0A= >=0A= >>> + /*=0A= >>> + * use EIAM to auto-mask when MSI-X interrupt is asserted=0A= >>> + * this saves a register write for every interrupt=0A= >>> + */=0A= >>> + switch (hw->mac.type) {=0A= >>> + case ixgbe_mac_82598EB:=0A= >>> + IXGBE_WRITE_REG(hw, IXGBE_EIAM, IXGBE_EICS_RTX_QUEUE);=0A= >>> + break;=0A= >>> + case ixgbe_mac_82599EB:=0A= >>> + case ixgbe_mac_X540:=0A= >>> + default:=0A= >>> + IXGBE_WRITE_REG(hw, IXGBE_EIAM_EX(0), 0xFFFFFFFF);=0A= >>> + IXGBE_WRITE_REG(hw, IXGBE_EIAM_EX(1), 0xFFFFFFFF);=0A= >>> + break;=0A= >>> + }=0A= >>> + IXGBE_WRITE_REG(hw, IXGBE_GPIE, gpie);=0A= >>> +=0A= >>> + /*=0A= >>> + * Populate the IVAR table and set the ITR values to the=0A= >>> + * corresponding register.=0A= >>> + */=0A= >>> + for (queue_id =3D 0; queue_id < VFIO_MAX_QUEUE_ID; queue_id++)=0A= >>> + ixgbe_set_ivar(hw, 0, queue_id, queue_id);=0A= >>> +=0A= >>> + switch (hw->mac.type) {=0A= >>> + case ixgbe_mac_82598EB:=0A= >>> + ixgbe_set_ivar(hw, -1, IXGBE_IVAR_OTHER_CAUSES_INDEX,=0A= >>> + VFIO_MAX_QUEUE_ID);=0A= >>> + break;=0A= >>> + case ixgbe_mac_82599EB:=0A= >>> + case ixgbe_mac_X540:=0A= >>> + ixgbe_set_ivar(hw, -1, 1, 32);=0A= >> May be better to make those values for a macro just as above.=0A= >>=0A= > To be fixed in V2.=0A= >=0A= >>> + break;=0A= >>> + default:=0A= >>> + break;=0A= >>> + }=0A= >>> + IXGBE_WRITE_REG(hw, IXGBE_EITR(queue_id), 1950);=0A= >> Also here, what's "1950" stands for?=0A= >>=0A= > It is an experienced interrupt throttle value which is used for interrupt= support=0A= =0A= Then, I think it is better to define it in a macro and do some brief=0A= introduction on this value.=0A= =0A= Thanks,=0A= Michael=0A= >>> +=0A= >>> + /* set up to autoclear timer, and the vectors */=0A= >>> + mask =3D IXGBE_EIMS_ENABLE_MASK;=0A= >>> + mask &=3D ~(IXGBE_EIMS_OTHER |=0A= >>> + IXGBE_EIMS_MAILBOX |=0A= >>> + IXGBE_EIMS_LSC);=0A= >>> +=0A= >>> + IXGBE_WRITE_REG(hw, IXGBE_EIAC, mask);=0A= >>> +}=0A= >>> +=0A= >>> static int ixgbe_set_queue_rate_limit(struct rte_eth_dev *dev,=0A= >>> uint16_t queue_idx, uint16_t tx_rate)=0A= >>> {=0A= >>> diff --git a/lib/librte_pmd_ixgbe/ixgbe_ethdev.h b/lib/librte_pmd_ixgbe= /ixgbe_ethdev.h=0A= >>> index 1383194..328c387 100644=0A= >>> --- a/lib/librte_pmd_ixgbe/ixgbe_ethdev.h=0A= >>> +++ b/lib/librte_pmd_ixgbe/ixgbe_ethdev.h=0A= >>> @@ -38,6 +38,8 @@=0A= >>> #include "ixgbe/ixgbe_dcb_82598.h"=0A= >>> #include "ixgbe_bypass.h"=0A= >>>=0A= >>> +#include =0A= >>> +=0A= >>> /* need update link, bit flag */=0A= >>> #define IXGBE_FLAG_NEED_LINK_UPDATE (uint32_t)(1 << 0)=0A= >>> #define IXGBE_FLAG_MAILBOX (uint32_t)(1 << 1)=0A= >>> @@ -98,6 +100,11 @@=0A= >>> #define IXGBE_5TUPLE_MAX_PRI 7=0A= >>> #define IXGBE_5TUPLE_MIN_PRI 1=0A= >>>=0A= >>> +#define IXGBE_VF_IRQ_ENABLE_MASK 3 /* vf interrupt ena= ble mask */=0A= >>> +#define IXGBE_VF_MAXMSIVECTOR 1=0A= >>> +/* maximum other interrupts besides rx&tx*/=0A= >>> +#define IXGBE_MAX_OTHER_INTR 1=0A= >>> +#define IXGBEVF_MAX_OTHER_INTR 1=0A= >>> /*=0A= >>> * Information about the fdir mode.=0A= >>> */=0A= >>> @@ -116,6 +123,7 @@ struct ixgbe_hw_fdir_info {=0A= >>> struct ixgbe_interrupt {=0A= >>> uint32_t flags;=0A= >>> uint32_t mask;=0A= >>> + rte_spinlock_t lock;=0A= >>> };=0A= >>>=0A= >>> struct ixgbe_stat_mapping_registers {=0A= >>> @@ -260,6 +268,7 @@ uint32_t ixgbe_dev_rx_queue_count(struct rte_eth_de= v *dev,=0A= >>> uint16_t rx_queue_id);=0A= >>>=0A= >>> int ixgbe_dev_rx_descriptor_done(void *rx_queue, uint16_t offset);=0A= >>> +int ixgbevf_dev_rx_descriptor_done(void *rx_queue, uint16_t offset);= =0A= >>>=0A= >>> int ixgbe_dev_rx_init(struct rte_eth_dev *dev);=0A= >>>=0A= >=0A= =0A=