From mboxrd@z Thu Jan 1 00:00:00 1970 From: Stephen Hemminger Subject: Re: [PATCH 3/3] igb_uio: remove sys files for setting pci config space Date: Mon, 21 Dec 2015 10:57:12 -0800 Message-ID: <20151221105712.46dfd6ce@xeon-e3> References: <1450665486-8335-1-git-send-email-helin.zhang@intel.com> <1450665486-8335-4-git-send-email-helin.zhang@intel.com> Mime-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Cc: dev@dpdk.org To: Helin Zhang Return-path: Received: from mail-pf0-f173.google.com (mail-pf0-f173.google.com [209.85.192.173]) by dpdk.org (Postfix) with ESMTP id E8AA358CF for ; Mon, 21 Dec 2015 19:57:05 +0100 (CET) Received: by mail-pf0-f173.google.com with SMTP id u7so46272951pfb.1 for ; Mon, 21 Dec 2015 10:57:05 -0800 (PST) In-Reply-To: <1450665486-8335-4-git-send-email-helin.zhang@intel.com> List-Id: patches and discussions about DPDK List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" On Mon, 21 Dec 2015 10:38:06 +0800 Helin Zhang 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 > --- > 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