diff mbox

[dpdk-dev,3/3] igb_uio: remove sys files for setting pci config space

Message ID 1450665486-8335-4-git-send-email-helin.zhang@intel.com (mailing list archive)
State Superseded, archived
Delegated to: Thomas Monjalon
Headers show

Commit Message

Helin Zhang Dec. 21, 2015, 2:38 a.m. UTC
Sys files of 'extended_tag' and 'max_read_request_size' are
useless, as nobody will use them for setting pci config space.

Signed-off-by: Helin Zhang <helin.zhang@intel.com>
---
 doc/guides/linux_gsg/enable_func.rst      |  22 ------
 doc/guides/rel_notes/deprecation.rst      |   3 +
 doc/guides/rel_notes/release_2_3.rst      |   6 ++
 lib/librte_eal/linuxapp/igb_uio/igb_uio.c | 108 ------------------------------
 4 files changed, 9 insertions(+), 130 deletions(-)

Comments

Stephen Hemminger Dec. 21, 2015, 6:57 p.m. UTC | #1
On Mon, 21 Dec 2015 10:38:06 +0800
Helin Zhang <helin.zhang@intel.com> wrote:

> Sys files of 'extended_tag' and 'max_read_request_size' are
> useless, as nobody will use them for setting pci config space.
> 
> Signed-off-by: Helin Zhang <helin.zhang@intel.com>
> ---
>  doc/guides/linux_gsg/enable_func.rst      |  22 ------
>  doc/guides/rel_notes/deprecation.rst      |   3 +
>  doc/guides/rel_notes/release_2_3.rst      |   6 ++
>  lib/librte_eal/linuxapp/igb_uio/igb_uio.c | 108 ------------------------------
>  4 files changed, 9 insertions(+), 130 deletions(-)
> 
> diff --git a/doc/guides/linux_gsg/enable_func.rst b/doc/guides/linux_gsg/enable_func.rst
> index c3fa6d3..ec0e04d 100644
> --- a/doc/guides/linux_gsg/enable_func.rst
> +++ b/doc/guides/linux_gsg/enable_func.rst
> @@ -186,28 +186,6 @@ Check with the local Intel's Network Division application engineers for firmware
>  The base driver to support firmware version of FVL3E will be integrated in the next
>  DPDK release, so currently the validated firmware version is 4.2.6.
>  
> -Enabling Extended Tag and Setting Max Read Request Size
> -~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> -
> -PCI configurations of ``extended_tag`` and max _read_requ st_size have big impacts on performance of small packets on 40G NIC.
> -Enabling extended_tag and setting ``max_read_request_size`` to small size such as 128 bytes provide great helps to high performance of small packets.
> -
> -*   These can be done in some BIOS implementations.
> -
> -*   For other BIOS implementations, PCI configurations can be changed by using command of ``setpci``, or special configurations in DPDK config file of ``common_linux``.
> -
> -    *   Bits 7:5 at address of 0xA8 of each PCI device is used for setting the max_read_request_size,
> -        and bit 8 of 0xA8 of each PCI device is used for enabling/disabling the extended_tag.
> -        lspci and setpci can be used to read the values of 0xA8 and then write it back after being changed.
> -
> -    *   In config file of common_linux, below three configurations can be changed for the same purpose.
> -
> -        ``CONFIG_RTE_PCI_CONFIG``
> -
> -        ``CONFIG_RTE_PCI_EXTENDED_TAG``
> -
> -        ``CONFIG_RTE_PCI_MAX_READ_REQUEST_SIZE``
> -
>  Use 16 Bytes RX Descriptor Size
>  ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>  
> diff --git a/doc/guides/rel_notes/deprecation.rst b/doc/guides/rel_notes/deprecation.rst
> index e94d4a2..7438f80 100644
> --- a/doc/guides/rel_notes/deprecation.rst
> +++ b/doc/guides/rel_notes/deprecation.rst
> @@ -49,3 +49,6 @@ Deprecation Notices
>    commands (such as RETA update in testpmd).  This should impact
>    CMDLINE_PARSE_RESULT_BUFSIZE, STR_TOKEN_SIZE and RDLINE_BUF_SIZE.
>    It should be integrated in release 2.3.
> +
> +* The eal function of pci_config_space_set is deprecated in release 2.3, and
> +  will be removed from 2.4.
> diff --git a/doc/guides/rel_notes/release_2_3.rst b/doc/guides/rel_notes/release_2_3.rst
> index efd258b..ed10d94 100644
> --- a/doc/guides/rel_notes/release_2_3.rst
> +++ b/doc/guides/rel_notes/release_2_3.rst
> @@ -16,6 +16,12 @@ Resolved Issues
>  EAL
>  ~~~
>  
> +* **eal/linux: removed sys files for pci config space.**
> +
> +  Removed sys files of 'extended_tag' and 'max_read_request_size' and
> +  their relavant operations, as they shouldn't be done in eal for all
> +  possible devices.
> +
>  
>  Drivers
>  ~~~~~~~
> diff --git a/lib/librte_eal/linuxapp/igb_uio/igb_uio.c b/lib/librte_eal/linuxapp/igb_uio/igb_uio.c
> index f5617d2..054d053 100644
> --- a/lib/librte_eal/linuxapp/igb_uio/igb_uio.c
> +++ b/lib/librte_eal/linuxapp/igb_uio/igb_uio.c
> @@ -40,15 +40,6 @@
>  
>  #include "compat.h"
>  
> -#ifdef RTE_PCI_CONFIG
> -#define PCI_SYS_FILE_BUF_SIZE      10
> -#define PCI_DEV_CAP_REG            0xA4
> -#define PCI_DEV_CTRL_REG           0xA8
> -#define PCI_DEV_CAP_EXT_TAG_MASK   0x20
> -#define PCI_DEV_CTRL_EXT_TAG_SHIFT 8
> -#define PCI_DEV_CTRL_EXT_TAG_MASK  (1 << PCI_DEV_CTRL_EXT_TAG_SHIFT)
> -#endif
> -
>  /**
>   * A structure describing the private information for a uio device.
>   */
> @@ -90,109 +81,10 @@ store_max_vfs(struct device *dev, struct device_attribute *attr,
>  	return err ? err : count;
>  }
>  
> -#ifdef RTE_PCI_CONFIG
> -static ssize_t
> -show_extended_tag(struct device *dev, struct device_attribute *attr, char *buf)
> -{
> -	struct pci_dev *pci_dev = to_pci_dev(dev);
> -	uint32_t val = 0;
> -
> -	pci_read_config_dword(pci_dev, PCI_DEV_CAP_REG, &val);
> -	if (!(val & PCI_DEV_CAP_EXT_TAG_MASK)) /* Not supported */
> -		return snprintf(buf, PCI_SYS_FILE_BUF_SIZE, "%s\n", "invalid");
> -
> -	val = 0;
> -	pci_bus_read_config_dword(pci_dev->bus, pci_dev->devfn,
> -					PCI_DEV_CTRL_REG, &val);
> -
> -	return snprintf(buf, PCI_SYS_FILE_BUF_SIZE, "%s\n",
> -		(val & PCI_DEV_CTRL_EXT_TAG_MASK) ? "on" : "off");
> -}
> -
> -static ssize_t
> -store_extended_tag(struct device *dev,
> -		   struct device_attribute *attr,
> -		   const char *buf,
> -		   size_t count)
> -{
> -	struct pci_dev *pci_dev = to_pci_dev(dev);
> -	uint32_t val = 0, enable;
> -
> -	if (strncmp(buf, "on", 2) == 0)
> -		enable = 1;
> -	else if (strncmp(buf, "off", 3) == 0)
> -		enable = 0;
> -	else
> -		return -EINVAL;
> -
> -	pci_cfg_access_lock(pci_dev);
> -	pci_bus_read_config_dword(pci_dev->bus, pci_dev->devfn,
> -					PCI_DEV_CAP_REG, &val);
> -	if (!(val & PCI_DEV_CAP_EXT_TAG_MASK)) { /* Not supported */
> -		pci_cfg_access_unlock(pci_dev);
> -		return -EPERM;
> -	}
> -
> -	val = 0;
> -	pci_bus_read_config_dword(pci_dev->bus, pci_dev->devfn,
> -					PCI_DEV_CTRL_REG, &val);
> -	if (enable)
> -		val |= PCI_DEV_CTRL_EXT_TAG_MASK;
> -	else
> -		val &= ~PCI_DEV_CTRL_EXT_TAG_MASK;
> -	pci_bus_write_config_dword(pci_dev->bus, pci_dev->devfn,
> -					PCI_DEV_CTRL_REG, val);
> -	pci_cfg_access_unlock(pci_dev);
> -
> -	return count;
> -}
> -
> -static ssize_t
> -show_max_read_request_size(struct device *dev,
> -			   struct device_attribute *attr,
> -			   char *buf)
> -{
> -	struct pci_dev *pci_dev = to_pci_dev(dev);
> -	int val = pcie_get_readrq(pci_dev);
> -
> -	return snprintf(buf, PCI_SYS_FILE_BUF_SIZE, "%d\n", val);
> -}
> -
> -static ssize_t
> -store_max_read_request_size(struct device *dev,
> -			    struct device_attribute *attr,
> -			    const char *buf,
> -			    size_t count)
> -{
> -	struct pci_dev *pci_dev = to_pci_dev(dev);
> -	unsigned long size = 0;
> -	int ret;
> -
> -	if (0 != kstrtoul(buf, 0, &size))
> -		return -EINVAL;
> -
> -	ret = pcie_set_readrq(pci_dev, (int)size);
> -	if (ret < 0)
> -		return ret;
> -
> -	return count;
> -}
> -#endif
> -
>  static DEVICE_ATTR(max_vfs, S_IRUGO | S_IWUSR, show_max_vfs, store_max_vfs);
> -#ifdef RTE_PCI_CONFIG
> -static DEVICE_ATTR(extended_tag, S_IRUGO | S_IWUSR, show_extended_tag,
> -	store_extended_tag);
> -static DEVICE_ATTR(max_read_request_size, S_IRUGO | S_IWUSR,
> -	show_max_read_request_size, store_max_read_request_size);
> -#endif
>  
>  static struct attribute *dev_attrs[] = {
>  	&dev_attr_max_vfs.attr,
> -#ifdef RTE_PCI_CONFIG
> -	&dev_attr_extended_tag.attr,
> -	&dev_attr_max_read_request_size.attr,
> -#endif
>  	NULL,
>  };
>  

Agreed, the current way was a mess and it is always possible to change
pci settings easier with setpci anyway.

Acked-by: Stephen Hemminger <stephen.hemminger@networkplumber.org>
diff mbox

Patch

diff --git a/doc/guides/linux_gsg/enable_func.rst b/doc/guides/linux_gsg/enable_func.rst
index c3fa6d3..ec0e04d 100644
--- a/doc/guides/linux_gsg/enable_func.rst
+++ b/doc/guides/linux_gsg/enable_func.rst
@@ -186,28 +186,6 @@  Check with the local Intel's Network Division application engineers for firmware
 The base driver to support firmware version of FVL3E will be integrated in the next
 DPDK release, so currently the validated firmware version is 4.2.6.
 
-Enabling Extended Tag and Setting Max Read Request Size
-~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
-
-PCI configurations of ``extended_tag`` and max _read_requ st_size have big impacts on performance of small packets on 40G NIC.
-Enabling extended_tag and setting ``max_read_request_size`` to small size such as 128 bytes provide great helps to high performance of small packets.
-
-*   These can be done in some BIOS implementations.
-
-*   For other BIOS implementations, PCI configurations can be changed by using command of ``setpci``, or special configurations in DPDK config file of ``common_linux``.
-
-    *   Bits 7:5 at address of 0xA8 of each PCI device is used for setting the max_read_request_size,
-        and bit 8 of 0xA8 of each PCI device is used for enabling/disabling the extended_tag.
-        lspci and setpci can be used to read the values of 0xA8 and then write it back after being changed.
-
-    *   In config file of common_linux, below three configurations can be changed for the same purpose.
-
-        ``CONFIG_RTE_PCI_CONFIG``
-
-        ``CONFIG_RTE_PCI_EXTENDED_TAG``
-
-        ``CONFIG_RTE_PCI_MAX_READ_REQUEST_SIZE``
-
 Use 16 Bytes RX Descriptor Size
 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
 
diff --git a/doc/guides/rel_notes/deprecation.rst b/doc/guides/rel_notes/deprecation.rst
index e94d4a2..7438f80 100644
--- a/doc/guides/rel_notes/deprecation.rst
+++ b/doc/guides/rel_notes/deprecation.rst
@@ -49,3 +49,6 @@  Deprecation Notices
   commands (such as RETA update in testpmd).  This should impact
   CMDLINE_PARSE_RESULT_BUFSIZE, STR_TOKEN_SIZE and RDLINE_BUF_SIZE.
   It should be integrated in release 2.3.
+
+* The eal function of pci_config_space_set is deprecated in release 2.3, and
+  will be removed from 2.4.
diff --git a/doc/guides/rel_notes/release_2_3.rst b/doc/guides/rel_notes/release_2_3.rst
index efd258b..ed10d94 100644
--- a/doc/guides/rel_notes/release_2_3.rst
+++ b/doc/guides/rel_notes/release_2_3.rst
@@ -16,6 +16,12 @@  Resolved Issues
 EAL
 ~~~
 
+* **eal/linux: removed sys files for pci config space.**
+
+  Removed sys files of 'extended_tag' and 'max_read_request_size' and
+  their relavant operations, as they shouldn't be done in eal for all
+  possible devices.
+
 
 Drivers
 ~~~~~~~
diff --git a/lib/librte_eal/linuxapp/igb_uio/igb_uio.c b/lib/librte_eal/linuxapp/igb_uio/igb_uio.c
index f5617d2..054d053 100644
--- a/lib/librte_eal/linuxapp/igb_uio/igb_uio.c
+++ b/lib/librte_eal/linuxapp/igb_uio/igb_uio.c
@@ -40,15 +40,6 @@ 
 
 #include "compat.h"
 
-#ifdef RTE_PCI_CONFIG
-#define PCI_SYS_FILE_BUF_SIZE      10
-#define PCI_DEV_CAP_REG            0xA4
-#define PCI_DEV_CTRL_REG           0xA8
-#define PCI_DEV_CAP_EXT_TAG_MASK   0x20
-#define PCI_DEV_CTRL_EXT_TAG_SHIFT 8
-#define PCI_DEV_CTRL_EXT_TAG_MASK  (1 << PCI_DEV_CTRL_EXT_TAG_SHIFT)
-#endif
-
 /**
  * A structure describing the private information for a uio device.
  */
@@ -90,109 +81,10 @@  store_max_vfs(struct device *dev, struct device_attribute *attr,
 	return err ? err : count;
 }
 
-#ifdef RTE_PCI_CONFIG
-static ssize_t
-show_extended_tag(struct device *dev, struct device_attribute *attr, char *buf)
-{
-	struct pci_dev *pci_dev = to_pci_dev(dev);
-	uint32_t val = 0;
-
-	pci_read_config_dword(pci_dev, PCI_DEV_CAP_REG, &val);
-	if (!(val & PCI_DEV_CAP_EXT_TAG_MASK)) /* Not supported */
-		return snprintf(buf, PCI_SYS_FILE_BUF_SIZE, "%s\n", "invalid");
-
-	val = 0;
-	pci_bus_read_config_dword(pci_dev->bus, pci_dev->devfn,
-					PCI_DEV_CTRL_REG, &val);
-
-	return snprintf(buf, PCI_SYS_FILE_BUF_SIZE, "%s\n",
-		(val & PCI_DEV_CTRL_EXT_TAG_MASK) ? "on" : "off");
-}
-
-static ssize_t
-store_extended_tag(struct device *dev,
-		   struct device_attribute *attr,
-		   const char *buf,
-		   size_t count)
-{
-	struct pci_dev *pci_dev = to_pci_dev(dev);
-	uint32_t val = 0, enable;
-
-	if (strncmp(buf, "on", 2) == 0)
-		enable = 1;
-	else if (strncmp(buf, "off", 3) == 0)
-		enable = 0;
-	else
-		return -EINVAL;
-
-	pci_cfg_access_lock(pci_dev);
-	pci_bus_read_config_dword(pci_dev->bus, pci_dev->devfn,
-					PCI_DEV_CAP_REG, &val);
-	if (!(val & PCI_DEV_CAP_EXT_TAG_MASK)) { /* Not supported */
-		pci_cfg_access_unlock(pci_dev);
-		return -EPERM;
-	}
-
-	val = 0;
-	pci_bus_read_config_dword(pci_dev->bus, pci_dev->devfn,
-					PCI_DEV_CTRL_REG, &val);
-	if (enable)
-		val |= PCI_DEV_CTRL_EXT_TAG_MASK;
-	else
-		val &= ~PCI_DEV_CTRL_EXT_TAG_MASK;
-	pci_bus_write_config_dword(pci_dev->bus, pci_dev->devfn,
-					PCI_DEV_CTRL_REG, val);
-	pci_cfg_access_unlock(pci_dev);
-
-	return count;
-}
-
-static ssize_t
-show_max_read_request_size(struct device *dev,
-			   struct device_attribute *attr,
-			   char *buf)
-{
-	struct pci_dev *pci_dev = to_pci_dev(dev);
-	int val = pcie_get_readrq(pci_dev);
-
-	return snprintf(buf, PCI_SYS_FILE_BUF_SIZE, "%d\n", val);
-}
-
-static ssize_t
-store_max_read_request_size(struct device *dev,
-			    struct device_attribute *attr,
-			    const char *buf,
-			    size_t count)
-{
-	struct pci_dev *pci_dev = to_pci_dev(dev);
-	unsigned long size = 0;
-	int ret;
-
-	if (0 != kstrtoul(buf, 0, &size))
-		return -EINVAL;
-
-	ret = pcie_set_readrq(pci_dev, (int)size);
-	if (ret < 0)
-		return ret;
-
-	return count;
-}
-#endif
-
 static DEVICE_ATTR(max_vfs, S_IRUGO | S_IWUSR, show_max_vfs, store_max_vfs);
-#ifdef RTE_PCI_CONFIG
-static DEVICE_ATTR(extended_tag, S_IRUGO | S_IWUSR, show_extended_tag,
-	store_extended_tag);
-static DEVICE_ATTR(max_read_request_size, S_IRUGO | S_IWUSR,
-	show_max_read_request_size, store_max_read_request_size);
-#endif
 
 static struct attribute *dev_attrs[] = {
 	&dev_attr_max_vfs.attr,
-#ifdef RTE_PCI_CONFIG
-	&dev_attr_extended_tag.attr,
-	&dev_attr_max_read_request_size.attr,
-#endif
 	NULL,
 };