From mboxrd@z Thu Jan 1 00:00:00 1970 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) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=networkplumber-org.20150623.gappssmtp.com; s=20150623; h=date:from:to:cc:subject:message-id:in-reply-to:references :mime-version:content-type:content-transfer-encoding; bh=8zzNI1LGW1SMEyzFli6FMWTs8/jMYXdoMlRxI0cfc1w=; b=aVJNJUotWX469Ynyo0N1JY9l316TYU+ZjCpSnhqvRxdoVQXEFdWb9koE+BfoSk1Lso jvQ14zl0XIUVZLqn13y+kqrIljcmJ8vrQNhd5D571qrD6R2X2ok0/yvzQGAbXPpmvH2E I7V7+WX6Wv5Wp0tp5n0YFDFubPAuHBwJzk0LNjNQhK6UHMOCZYF5ReCy4HLVx3J6iiiC pn/200/+AEWpidBItF6BQMmyLjScp5YlYsvEm5vdGq7I2tfzvAv7fRZYxmlzKQO+GvTm zOtMsgDM0mCt5raLIH8+XZQAXsV0V4eCcr832o+dpcpDC+KUCp3QaGUa8i9TVDjDxuGh wMHQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:date:from:to:cc:subject:message-id:in-reply-to :references:mime-version:content-type:content-transfer-encoding; bh=8zzNI1LGW1SMEyzFli6FMWTs8/jMYXdoMlRxI0cfc1w=; b=O4osv3xjz0Ts+yvak4YmA3Vu4UrCw2EtVOm/Vk+Rm0fwnD+8tWVJvVEFKJ18Ar4zyQ RI/kZBX0Y9nd8qtviMvLfbnjLeHR6olSdFo3jQ89XV8n50gBOpYgyaxjZlUb2mcYpTYe mIOUIjZKs8JR9EhYEnU1wB38zzGWBhNOcuy/FSH5PoUqRPPFnx4f7PieyVufsmxdVI/M 1qRl1m67LHkFH5JaJm5xWYG7XushvRjaPu+w77gWovBU+4LriSFw46Q9HSFJYEB0lyXR B4aNpJ1CXbSTvyarRXHEAtZYtslPybz3EisBV+pqC2ZYRMgGEUQDyHdaNW5cBUGLNUgm 0Vuw== X-Gm-Message-State: ALoCoQmzk1KwQmfqvWIZ4vUlh2dxBR8E1txh/p60V2GGtT7FIf2Vdfu8pHJBYi9gsZyDJ9osYIpz/2MaK2DrjCEBGsQaBcTosA== X-Received: by 10.98.75.10 with SMTP id y10mr29582168pfa.86.1450724225084; Mon, 21 Dec 2015 10:57:05 -0800 (PST) Received: from xeon-e3 (static-50-53-82-155.bvtn.or.frontiernet.net. [50.53.82.155]) by smtp.gmail.com with ESMTPSA id ry1sm40866556pab.30.2015.12.21.10.57.04 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Mon, 21 Dec 2015 10:57:04 -0800 (PST) Date: Mon, 21 Dec 2015 10:57:12 -0800 From: Stephen Hemminger To: Helin Zhang Message-ID: <20151221105712.46dfd6ce@xeon-e3> In-Reply-To: <1450665486-8335-4-git-send-email-helin.zhang@intel.com> 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 Subject: Re: [dpdk-dev] [PATCH 3/3] igb_uio: remove sys files for setting pci config space X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: patches and discussions about DPDK List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 21 Dec 2015 18:57:06 -0000 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