[dpdk-dev,v2,1/3] i40e: enable extended tag

Message ID 1456113585-15259-2-git-send-email-helin.zhang@intel.com (mailing list archive)
State Superseded, archived
Headers

Commit Message

Zhang, Helin Feb. 22, 2016, 3:59 a.m. UTC
  PCIe feature of 'Extended Tag' is important for 40G performance.
It adds its enabling during each port initialization, to ensure
the high performance.

Signed-off-by: Helin Zhang <helin.zhang@intel.com>
---
 doc/guides/rel_notes/release_16_04.rst |  6 ++++
 drivers/net/i40e/i40e_ethdev.c         | 65 ++++++++++++++++++++++++++++++++--
 2 files changed, 68 insertions(+), 3 deletions(-)

v2:
 - Changed the type of return value of i40e_enable_extended_tag() to 'void'.
  

Comments

Bruce Richardson Feb. 23, 2016, 10:44 a.m. UTC | #1
On Mon, Feb 22, 2016 at 11:59:43AM +0800, Helin Zhang wrote:
> PCIe feature of 'Extended Tag' is important for 40G performance.
> It adds its enabling during each port initialization, to ensure
> the high performance.
> 
> Signed-off-by: Helin Zhang <helin.zhang@intel.com>
> ---
>  doc/guides/rel_notes/release_16_04.rst |  6 ++++
>  drivers/net/i40e/i40e_ethdev.c         | 65 ++++++++++++++++++++++++++++++++--
>  2 files changed, 68 insertions(+), 3 deletions(-)
> 
> v2:
>  - Changed the type of return value of i40e_enable_extended_tag() to 'void'.
> 
> diff --git a/doc/guides/rel_notes/release_16_04.rst b/doc/guides/rel_notes/release_16_04.rst
> index 5786f74..bed5779 100644
> --- a/doc/guides/rel_notes/release_16_04.rst
> +++ b/doc/guides/rel_notes/release_16_04.rst
> @@ -46,6 +46,12 @@ This section should contain new features added in this release. Sample format:
>  
>  * **Added vhost-user live migration support.**
>  
> +* **i40e: Enabled extended tag.**
> +
> +  It enabled extended tag by checking and writing corresponding PCI config
> +  space bytes, to boost the performance. In the meanwhile, it deprecated the
> +  legacy way via reading/writing sysfile supported by kernel module of igb_uio.
> +

Hi Helin,

does this really need to go into the release notes? Is it a user-visible change
that affects the user experience in any way?

/Bruce
  
Zhang, Helin Feb. 24, 2016, 12:39 a.m. UTC | #2
> -----Original Message-----
> From: Richardson, Bruce
> Sent: Tuesday, February 23, 2016 6:45 PM
> To: Zhang, Helin <helin.zhang@intel.com>
> Cc: dev@dpdk.org; zhe.tag@intel.com
> Subject: Re: [dpdk-dev] [PATCH v2 1/3] i40e: enable extended tag
> 
> On Mon, Feb 22, 2016 at 11:59:43AM +0800, Helin Zhang wrote:
> > PCIe feature of 'Extended Tag' is important for 40G performance.
> > It adds its enabling during each port initialization, to ensure the
> > high performance.
> >
> > Signed-off-by: Helin Zhang <helin.zhang@intel.com>
> > ---
> >  doc/guides/rel_notes/release_16_04.rst |  6 ++++
> >  drivers/net/i40e/i40e_ethdev.c         | 65
> ++++++++++++++++++++++++++++++++--
> >  2 files changed, 68 insertions(+), 3 deletions(-)
> >
> > v2:
> >  - Changed the type of return value of i40e_enable_extended_tag() to 'void'.
> >
> > diff --git a/doc/guides/rel_notes/release_16_04.rst
> > b/doc/guides/rel_notes/release_16_04.rst
> > index 5786f74..bed5779 100644
> > --- a/doc/guides/rel_notes/release_16_04.rst
> > +++ b/doc/guides/rel_notes/release_16_04.rst
> > @@ -46,6 +46,12 @@ This section should contain new features added in this
> release. Sample format:
> >
> >  * **Added vhost-user live migration support.**
> >
> > +* **i40e: Enabled extended tag.**
> > +
> > +  It enabled extended tag by checking and writing corresponding PCI
> > + config  space bytes, to boost the performance. In the meanwhile, it
> > + deprecated the  legacy way via reading/writing sysfile supported by kernel
> module of igb_uio.
> > +
> 
> Hi Helin,
> 
> does this really need to go into the release notes? Is it a user-visible change that
> affects the user experience in any way?
Previously we enable it in eal via igb_uio sys file (by default, it is disabled), which was deprecated now, and will be removed from next release.
All now added into i40e PMD init only.
Yes, user might see the performance boost without doing anything, which is different from previous version of DPDK.

I think it should be mentioned somewhere. Any better idea?
Thanks,
Helin

> 
> /Bruce
  

Patch

diff --git a/doc/guides/rel_notes/release_16_04.rst b/doc/guides/rel_notes/release_16_04.rst
index 5786f74..bed5779 100644
--- a/doc/guides/rel_notes/release_16_04.rst
+++ b/doc/guides/rel_notes/release_16_04.rst
@@ -46,6 +46,12 @@  This section should contain new features added in this release. Sample format:
 
 * **Added vhost-user live migration support.**
 
+* **i40e: Enabled extended tag.**
+
+  It enabled extended tag by checking and writing corresponding PCI config
+  space bytes, to boost the performance. In the meanwhile, it deprecated the
+  legacy way via reading/writing sysfile supported by kernel module of igb_uio.
+
 
 Resolved Issues
 ---------------
diff --git a/drivers/net/i40e/i40e_ethdev.c b/drivers/net/i40e/i40e_ethdev.c
index ef24122..7e68c61 100644
--- a/drivers/net/i40e/i40e_ethdev.c
+++ b/drivers/net/i40e/i40e_ethdev.c
@@ -273,6 +273,17 @@ 
 #define I40E_INSET_IPV6_TC_MASK       0x0009F00FUL
 #define I40E_INSET_IPV6_NEXT_HDR_MASK 0x000C00FFUL
 
+/* PCI offset for querying capability */
+#define PCI_DEV_CAP_REG            0xA4
+/* PCI offset for enabling/disabling Extended Tag */
+#define PCI_DEV_CTRL_REG           0xA8
+/* Bit mask of Extended Tag capability */
+#define PCI_DEV_CAP_EXT_TAG_MASK   0x20
+/* Bit shift of Extended Tag enable/disable */
+#define PCI_DEV_CTRL_EXT_TAG_SHIFT 8
+/* Bit mask of Extended Tag enable/disable */
+#define PCI_DEV_CTRL_EXT_TAG_MASK  (1 << PCI_DEV_CTRL_EXT_TAG_SHIFT)
+
 static int eth_i40e_dev_init(struct rte_eth_dev *eth_dev);
 static int eth_i40e_dev_uninit(struct rte_eth_dev *eth_dev);
 static int i40e_dev_configure(struct rte_eth_dev *dev);
@@ -386,7 +397,7 @@  static int i40e_dev_filter_ctrl(struct rte_eth_dev *dev,
 static int i40e_dev_get_dcb_info(struct rte_eth_dev *dev,
 				  struct rte_eth_dcb_info *dcb_info);
 static void i40e_configure_registers(struct i40e_hw *hw);
-static void i40e_hw_init(struct i40e_hw *hw);
+static void i40e_hw_init(struct rte_eth_dev *dev);
 static int i40e_config_qinq(struct i40e_hw *hw, struct i40e_vsi *vsi);
 static int i40e_mirror_rule_set(struct rte_eth_dev *dev,
 			struct rte_eth_mirror_conf *mirror_conf,
@@ -765,7 +776,7 @@  eth_i40e_dev_init(struct rte_eth_dev *dev)
 	i40e_clear_hw(hw);
 
 	/* Initialize the hardware */
-	i40e_hw_init(hw);
+	i40e_hw_init(dev);
 
 	/* Reset here to make sure all is clean for each PF */
 	ret = i40e_pf_reset(hw);
@@ -7262,13 +7273,61 @@  i40e_dev_filter_ctrl(struct rte_eth_dev *dev,
 }
 
 /*
+ * Check and enable Extended Tag.
+ * Enabling Extended Tag is important for 40G performance.
+ */
+static void
+i40e_enable_extended_tag(struct rte_eth_dev *dev)
+{
+	uint32_t buf = 0;
+	int ret;
+
+	ret = rte_eal_pci_read_config(dev->pci_dev, &buf, sizeof(buf),
+				      PCI_DEV_CAP_REG);
+	if (ret < 0) {
+		PMD_DRV_LOG(ERR, "Failed to read PCI offset 0x%x",
+			    PCI_DEV_CAP_REG);
+		return;
+	}
+	if (!(buf & PCI_DEV_CAP_EXT_TAG_MASK)) {
+		PMD_DRV_LOG(ERR, "Does not support Extended Tag");
+		return;
+	}
+
+	buf = 0;
+	ret = rte_eal_pci_read_config(dev->pci_dev, &buf, sizeof(buf),
+				      PCI_DEV_CTRL_REG);
+	if (ret < 0) {
+		PMD_DRV_LOG(ERR, "Failed to read PCI offset 0x%x",
+			    PCI_DEV_CTRL_REG);
+		return;
+	}
+	if (buf & PCI_DEV_CTRL_EXT_TAG_MASK) {
+		PMD_DRV_LOG(DEBUG, "Extended Tag has already been enabled");
+		return;
+	}
+	buf |= PCI_DEV_CTRL_EXT_TAG_MASK;
+	ret = rte_eal_pci_write_config(dev->pci_dev, &buf, sizeof(buf),
+				       PCI_DEV_CTRL_REG);
+	if (ret < 0) {
+		PMD_DRV_LOG(ERR, "Failed to write PCI offset 0x%x",
+			    PCI_DEV_CTRL_REG);
+		return;
+	}
+}
+
+/*
  * As some registers wouldn't be reset unless a global hardware reset,
  * hardware initialization is needed to put those registers into an
  * expected initial state.
  */
 static void
-i40e_hw_init(struct i40e_hw *hw)
+i40e_hw_init(struct rte_eth_dev *dev)
 {
+	struct i40e_hw *hw = I40E_DEV_PRIVATE_TO_HW(dev->data->dev_private);
+
+	i40e_enable_extended_tag(dev);
+
 	/* clear the PF Queue Filter control register */
 	I40E_WRITE_REG(hw, I40E_PFQF_CTL_0, 0);