From mboxrd@z Thu Jan 1 00:00:00 1970 From: Ferruh Yigit Subject: Re: [PATCH v4 1/4] igb_uio: add wc option Date: Fri, 29 Jun 2018 14:40:56 +0100 Message-ID: <5353cd63-b0c5-36fe-6902-80fec2212550@intel.com> References: <1530267873-7236-2-git-send-email-rk@semihalf.com> <1530274637-10156-1-git-send-email-rk@semihalf.com> <1530274637-10156-2-git-send-email-rk@semihalf.com> Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit Cc: mw@semihalf.com, mk@semihalf.com, gtzalik@amazon.com, evgenys@amazon.com, matua@amazon.com, igorch@amazon.com, thomas@monjalon.net To: Rafal Kozik , dev@dpdk.org Return-path: Received: from mga09.intel.com (mga09.intel.com [134.134.136.24]) by dpdk.org (Postfix) with ESMTP id E204A1B3AF for ; Fri, 29 Jun 2018 15:41:09 +0200 (CEST) In-Reply-To: <1530274637-10156-2-git-send-email-rk@semihalf.com> Content-Language: en-US List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" On 6/29/2018 1:17 PM, Rafal Kozik wrote: > From: Kozik > > Write combining (WC) increases NIC performance by making better > utilization of PCI bus, but cannot be use by all PMD. > > To get internal_addr memory need to be mapped. But as memory could not be > mapped twice: with and without WC it should be skipped for WC. [1] > > To do not spoil other drivers that potentially could use internal_addr, > parameter wc_activate adds possibility to skip it for those PMDs, > that do not use it. > > [1] https://www.kernel.org/doc/ols/2008/ols2008v2-pages-135-144.pdf > section 5.3 and 5.4 > > Signed-off-by: Rafal Kozik > Acked-by: Bruce Richardson > --- > kernel/linux/igb_uio/igb_uio.c | 18 +++++++++++++++--- > 1 file changed, 15 insertions(+), 3 deletions(-) > > diff --git a/kernel/linux/igb_uio/igb_uio.c b/kernel/linux/igb_uio/igb_uio.c > index b3233f1..e16e760 100644 > --- a/kernel/linux/igb_uio/igb_uio.c > +++ b/kernel/linux/igb_uio/igb_uio.c > @@ -30,6 +30,7 @@ struct rte_uio_pci_dev { > int refcnt; > }; > > +static int wc_activate; > static char *intr_mode; > static enum rte_intr_mode igbuio_intr_mode_preferred = RTE_INTR_MODE_MSIX; > /* sriov sysfs */ > @@ -375,9 +376,14 @@ igbuio_pci_setup_iomem(struct pci_dev *dev, struct uio_info *info, > len = pci_resource_len(dev, pci_bar); > if (addr == 0 || len == 0) > return -1; > - internal_addr = ioremap(addr, len); > - if (internal_addr == NULL) > - return -1; > + if (wc_activate == 0) { > + internal_addr = ioremap(addr, len); > + if (internal_addr == NULL) > + return -1; > + } else { > + internal_addr = NULL; > + pr_info("wc_activate is set\n"); This location has been reached during device probe, so this log has been printed for each BAR of each device, which is unnecessary. My intention was print the log once when wc_activate is set, similar to "intr_mode", may bad that I point to wrong place, Would you mind making one more version to fix this, you can keep the ack. Thanks, ferruh > + } > info->mem[n].name = name; > info->mem[n].addr = addr; > info->mem[n].internal_addr = internal_addr; > @@ -650,6 +656,12 @@ MODULE_PARM_DESC(intr_mode, > " " RTE_INTR_MODE_LEGACY_NAME " Use Legacy interrupt\n" > "\n"); > > +module_param(wc_activate, int, 0); > +MODULE_PARM_DESC(wc_activate, > +"Activate support for write combining (WC) (default=0)\n" > +" 0 - disable\n" > +" other - enable\n"); > + > MODULE_DESCRIPTION("UIO driver for Intel IGB PCI cards"); > MODULE_LICENSE("GPL"); > MODULE_AUTHOR("Intel Corporation"); >