On Mon, 21 Dec 2015 10:38:06 +0800
Helin Zhang <helin.zhang at 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 at 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 at networkplumber.org>
