From: Stephen Hemminger <stephen@networkplumber.org>
To: Helin Zhang <helin.zhang@intel.com>
Cc: dev@dpdk.org
Subject: Re: [dpdk-dev] [PATCH 3/3] igb_uio: remove sys files for setting pci config space
Date: Mon, 21 Dec 2015 10:57:12 -0800 [thread overview]
Message-ID: <20151221105712.46dfd6ce@xeon-e3> (raw)
In-Reply-To: <1450665486-8335-4-git-send-email-helin.zhang@intel.com>
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>
next prev parent reply other threads:[~2015-12-21 18:57 UTC|newest]
Thread overview: 32+ messages / expand[flat|nested] mbox.gz Atom feed top
2015-12-21 2:38 [dpdk-dev] [PATCH 0/3] i40e: enable extended tag Helin Zhang
2015-12-21 2:38 ` [dpdk-dev] [PATCH 1/3] " Helin Zhang
2016-01-22 1:34 ` Wu, Jingjing
2016-01-22 10:26 ` Thomas Monjalon
2016-01-24 3:25 ` Zhang, Helin
2016-01-25 9:16 ` Thomas Monjalon
2016-01-26 0:29 ` Zhang, Helin
2015-12-21 2:38 ` [dpdk-dev] [PATCH 2/3] eal: remove pci config of " Helin Zhang
2016-02-22 3:59 ` [dpdk-dev] [PATCH v2 0/3] enable extended tag for i40e Helin Zhang
2016-02-22 3:59 ` [dpdk-dev] [PATCH v2 1/3] i40e: enable extended tag Helin Zhang
2016-02-23 10:44 ` Bruce Richardson
2016-02-24 0:39 ` Zhang, Helin
2016-02-22 3:59 ` [dpdk-dev] [PATCH v2 2/3] eal: remove pci config of " Helin Zhang
2016-03-08 17:05 ` Thomas Monjalon
2016-02-22 3:59 ` [dpdk-dev] [PATCH v2 3/3] igb_uio: deprecate sys files Helin Zhang
2016-03-08 17:48 ` Thomas Monjalon
2016-03-08 18:02 ` Thomas Monjalon
2016-02-22 5:52 ` [dpdk-dev] [PATCH v2 0/3] enable extended tag for i40e Wu, Jingjing
2016-03-08 18:38 ` [dpdk-dev] [PATCH v3 " Thomas Monjalon
2016-03-08 18:38 ` [dpdk-dev] [PATCH v3 1/3] i40e: enable extended tag Thomas Monjalon
2016-03-08 18:38 ` [dpdk-dev] [PATCH v3 2/3] pci: remove config of " Thomas Monjalon
2016-03-08 18:38 ` [dpdk-dev] [PATCH v3 3/3] igb_uio: deprecate " Thomas Monjalon
2016-03-08 18:41 ` [dpdk-dev] [PATCH v3 0/3] enable extended tag for i40e Thomas Monjalon
2016-03-09 0:48 ` Zhang, Helin
2016-03-09 0:50 ` Thomas Monjalon
2016-03-09 0:52 ` Thomas Monjalon
2015-12-21 2:38 ` [dpdk-dev] [PATCH 3/3] igb_uio: remove sys files for setting pci config space Helin Zhang
2015-12-21 18:57 ` Stephen Hemminger [this message]
2016-02-22 6:31 ` [dpdk-dev] [PATCH v2 0/3] enable extended tag for i40e Helin Zhang
2016-02-22 6:31 ` [dpdk-dev] [PATCH v2 1/3] i40e: enable extended tag Helin Zhang
2016-02-22 6:31 ` [dpdk-dev] [PATCH v2 2/3] eal: remove pci config of " Helin Zhang
2016-02-22 6:31 ` [dpdk-dev] [PATCH v2 3/3] igb_uio: deprecate sys files Helin Zhang
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20151221105712.46dfd6ce@xeon-e3 \
--to=stephen@networkplumber.org \
--cc=dev@dpdk.org \
--cc=helin.zhang@intel.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).