From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga09.intel.com (mga09.intel.com [134.134.136.24]) by dpdk.org (Postfix) with ESMTP id 60DE63237 for ; Mon, 1 Feb 2016 14:47:34 +0100 (CET) Received: from fmsmga004.fm.intel.com ([10.253.24.48]) by orsmga102.jf.intel.com with ESMTP; 01 Feb 2016 05:47:33 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.22,380,1449561600"; d="scan'208";a="40218265" Received: from yliu-dev.sh.intel.com (HELO yliu-dev) ([10.239.66.49]) by fmsmga004.fm.intel.com with ESMTP; 01 Feb 2016 05:47:32 -0800 Date: Mon, 1 Feb 2016 21:48:54 +0800 From: Yuanhan Liu To: Santosh Shukla , david.marchand@6wind.com Message-ID: <20160201134854.GE4257@yliu-dev.sh.intel.com> References: <0000-cover-letter.patch> <1454091717-32251-1-git-send-email-sshukla@mvista.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1454091717-32251-1-git-send-email-sshukla@mvista.com> User-Agent: Mutt/1.5.23 (2014-03-12) Cc: dev@dpdk.org Subject: Re: [dpdk-dev] [PATCH v6 1/8] eal: pci: add api to rd/wr pci bar region 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, 01 Feb 2016 13:47:36 -0000 On Fri, Jan 29, 2016 at 11:51:50PM +0530, Santosh Shukla wrote: > Introducing below api for pci bar region rd/wr. > Api's are: > - rte_eal_pci_read_bar > - rte_eal_pci_write_bar > > Signed-off-by: Santosh Shukla > --- > v5-->v6: > - update api infor in rte_eal_version.map file > suggested by david manchand. > > lib/librte_eal/bsdapp/eal/eal_pci.c | 19 ++++++++++++ > lib/librte_eal/bsdapp/eal/rte_eal_version.map | 3 ++ > lib/librte_eal/common/include/rte_pci.h | 38 +++++++++++++++++++++++ > lib/librte_eal/linuxapp/eal/eal_pci.c | 34 ++++++++++++++++++++ > lib/librte_eal/linuxapp/eal/eal_pci_init.h | 6 ++++ > lib/librte_eal/linuxapp/eal/eal_pci_vfio.c | 28 +++++++++++++++++ > lib/librte_eal/linuxapp/eal/rte_eal_version.map | 3 ++ > 7 files changed, 131 insertions(+) > > diff --git a/lib/librte_eal/bsdapp/eal/eal_pci.c b/lib/librte_eal/bsdapp/eal/eal_pci.c > index 95c32c1..2e535ea 100644 > --- a/lib/librte_eal/bsdapp/eal/eal_pci.c > +++ b/lib/librte_eal/bsdapp/eal/eal_pci.c ... > +int rte_eal_pci_read_bar(const struct rte_pci_device *device, > + void *buf, size_t len, off_t offset, > + int bar_idx) > + > +{ > + const struct rte_intr_handle *intr_handle = &device->intr_handle; I'd suggest to reference this var inside pci_vfio_read/write_bar(), and pass device as the parmater instead. > + > + switch (device->kdrv) { > + case RTE_KDRV_VFIO: > + return pci_vfio_read_bar(intr_handle, buf, len, > + offset, bar_idx); > + default: > + RTE_LOG(ERR, EAL, "write bar not supported by driver\n"); ^^^^^ typo. BTW, I have a question about this API. Obviously, reading/writing bar space is supported with UIO (when memory resource is mmapped). And I know why you introduced such 2 APIs, for reading IO bar. So, here is the question: what are the 2 APIs for, for being gerneric APIs to read/write bar spaces, or just to read IO bar spaces? If it's former, the message is wrong; if it's later, you may better rename it to rte_eal_pci_read/write_io_bar()? David, what do you think of that? > + return -1; > + } > +} > + ... > +int > +pci_vfio_read_bar(const struct rte_intr_handle *intr_handle, > + void *buf, size_t len, off_t offs, int bar_idx) > +{ > + if (bar_idx < VFIO_PCI_BAR0_REGION_INDEX > + || bar_idx > VFIO_PCI_BAR5_REGION_INDEX) { A minor nit: it's more nature to put the '||' at the end of expression, instead of at the front: if (bar_idx < VFIO_PCI_BAR0_REGION_INDEX || bar_idx > VFIO_PCI_BAR5_REGION_INDEX) { --yliu