From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-wm0-f54.google.com (mail-wm0-f54.google.com [74.125.82.54]) by dpdk.org (Postfix) with ESMTP id D0281592B for ; Mon, 18 Sep 2017 13:52:02 +0200 (CEST) Received: by mail-wm0-f54.google.com with SMTP id 13so1897152wmq.2 for ; Mon, 18 Sep 2017 04:52:02 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=6wind-com.20150623.gappssmtp.com; s=20150623; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:content-transfer-encoding:in-reply-to :user-agent; bh=Smlmdh7URj1ULLjRtLy2ad3ZP5pwm0bPObOqNDol7Qs=; b=k4g25qVzUSg5S2Hu4aWr7BvOGTV9xkDVZT0ZABwBF3KyU0fjJJB/lwfST1oCkauARY 2KEeKJhImYd6yHUOwiUeWZrIAu6NlRDU08OOANlYD4XtY1mjJcitj/VRwxeCzkQPgKT7 MMZKjf+YNZWy5wg2P32fYJA5fA6CXAp2rg3U+SFOHd2rF6vSObApIEUochZnGlPdQkTX Dfcmc83CzvlF5c7ryN2ZdsBii5RrukZ2d00VsPp3jeZn1X3V4REzzRNxcu8zySSTMwjL lapOxsBBXJoUz7UJg9q5FntnKEB0QhQiQY1RG45eIHww+PGSCikmWZUcxmwrEohlGk2z cSRA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:content-transfer-encoding :in-reply-to:user-agent; bh=Smlmdh7URj1ULLjRtLy2ad3ZP5pwm0bPObOqNDol7Qs=; b=IangqibBSOtklP/U9gEoq+TfxkHjlnpWSizo1szY+eTbI8HA7FxmP0m3mWiEV4eD1s 3Y5jQoD+qRumCg6D+wRMcybCqfMQjq4z/PydO/9dfNVHlTd+FQubTMZGE9Sf00J7gGCC 3pyyWTEQiaPc6H2k2iv/umrhdktiZhxKpwhyhu4GshsLi5g1Eqy4/YVZZ/DivNu1I1bP xz8IpIHigqpXZikJ3DB2Z9QM/gQJ1iL/8UqkfjJP9ulNGoMiJaPblwHjfVv3eAtrpW3S 3MExHXrSnD1ZxsEMc49G4/q9nCNkPaXgdgYJeOsMZSioH8UtEI8RjeWaBjSFSrQ1Lq3C 9pHQ== X-Gm-Message-State: AHPjjUg1gvkG53zwNbvENE0H6DiD9DsCUtEYu/Ze13kqHdYBxETdKGiW KEWUq3q8ABnE/WdwA/M= X-Google-Smtp-Source: AOwi7QDArOQDAKGea5B08zNet7Nx8jYFPKRNzeLZYJ3K+dHh6tC+TYgYbT/Aex5AgxZeql4pnAnYgg== X-Received: by 10.28.197.133 with SMTP id v127mr9443504wmf.52.1505735522377; Mon, 18 Sep 2017 04:52:02 -0700 (PDT) Received: from bidouze.vm.6wind.com (host.78.145.23.62.rev.coltfrance.com. [62.23.145.78]) by smtp.gmail.com with ESMTPSA id f17sm6015021wmf.47.2017.09.18.04.52.01 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Mon, 18 Sep 2017 04:52:01 -0700 (PDT) Date: Mon, 18 Sep 2017 13:51:51 +0200 From: =?iso-8859-1?Q?Ga=EBtan?= Rivet To: Shreyansh Jain Cc: dev@dpdk.org Message-ID: <20170918115151.GS21444@bidouze.vm.6wind.com> References: <618e47fbcfbc9ead4c009f0456faab563f22a426.1505726803.git.gaetan.rivet@6wind.com> <88d7466f-8c93-b97c-48d9-fb7bab7e7f77@nxp.com> MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <88d7466f-8c93-b97c-48d9-fb7bab7e7f77@nxp.com> User-Agent: Mutt/1.5.23 (2014-03-12) Subject: Re: [dpdk-dev] [PATCH v2 05/14] pci: introduce PCI lib and bus X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 18 Sep 2017 11:52:03 -0000 Hey, On Mon, Sep 18, 2017 at 05:23:23PM +0530, Shreyansh Jain wrote: > Hello Gaetan, > > On Monday 18 September 2017 03:01 PM, Gaetan Rivet wrote: > >The PCI lib defines the types and methods allowing to use PCI elements. > > > >The PCI bus implements a bus driver for PCI devices by constructing > >rte_bus elements using the PCI lib. > > > >Move the relevant code out of the EAL to their expected place. > > > >Signed-off-by: Gaetan Rivet > >--- > > config/common_base | 10 + > > drivers/bus/Makefile | 2 + > > drivers/bus/pci/Makefile | 59 ++ > > drivers/bus/pci/bsd/Makefile | 32 ++ > > drivers/bus/pci/bsd/rte_pci.c | 670 ++++++++++++++++++++++ > > drivers/bus/pci/include/rte_bus_pci.h | 387 +++++++++++++ > > drivers/bus/pci/linux/Makefile | 37 ++ > > drivers/bus/pci/linux/rte_pci.c | 722 ++++++++++++++++++++++++ > > drivers/bus/pci/linux/rte_pci_init.h | 97 ++++ > > drivers/bus/pci/linux/rte_pci_uio.c | 567 +++++++++++++++++++ > > drivers/bus/pci/linux/rte_pci_vfio.c | 674 ++++++++++++++++++++++ > > drivers/bus/pci/linux/rte_vfio_mp_sync.c | 424 ++++++++++++++ > > drivers/bus/pci/private.h | 173 ++++++ > > drivers/bus/pci/rte_bus_pci_version.map | 21 + > > drivers/bus/pci/rte_pci_common.c | 542 ++++++++++++++++++ > > drivers/bus/pci/rte_pci_common_uio.c | 234 ++++++++ > > lib/Makefile | 2 + > > lib/librte_eal/bsdapp/eal/Makefile | 3 - > > lib/librte_eal/bsdapp/eal/eal_pci.c | 670 ---------------------- > > lib/librte_eal/bsdapp/eal/rte_eal_version.map | 15 - > > lib/librte_eal/common/Makefile | 2 +- > > lib/librte_eal/common/eal_common_pci.c | 580 ------------------- > > lib/librte_eal/common/eal_common_pci_uio.c | 233 -------- > > lib/librte_eal/common/include/rte_pci.h | 598 -------------------- > > lib/librte_eal/linuxapp/eal/Makefile | 10 - > > lib/librte_eal/linuxapp/eal/eal_pci.c | 722 ------------------------ > > lib/librte_eal/linuxapp/eal/eal_pci_init.h | 97 ---- > > lib/librte_eal/linuxapp/eal/eal_pci_uio.c | 567 ------------------- > > lib/librte_eal/linuxapp/eal/eal_pci_vfio.c | 674 ---------------------- > > lib/librte_eal/linuxapp/eal/eal_vfio_mp_sync.c | 424 -------------- > > lib/librte_eal/linuxapp/eal/rte_eal_version.map | 15 - > > lib/librte_ether/rte_ethdev.h | 2 - > > lib/librte_pci/Makefile | 48 ++ > > lib/librte_pci/include/rte_pci.h | 279 +++++++++ > > lib/librte_pci/rte_pci.c | 92 +++ > > lib/librte_pci/rte_pci_version.map | 8 + > > mk/rte.app.mk | 3 + > > 37 files changed, 5084 insertions(+), 4611 deletions(-) > > create mode 100644 drivers/bus/pci/Makefile > > create mode 100644 drivers/bus/pci/bsd/Makefile > > create mode 100644 drivers/bus/pci/bsd/rte_pci.c > > create mode 100644 drivers/bus/pci/include/rte_bus_pci.h > > create mode 100644 drivers/bus/pci/linux/Makefile > > create mode 100644 drivers/bus/pci/linux/rte_pci.c > > create mode 100644 drivers/bus/pci/linux/rte_pci_init.h > > create mode 100644 drivers/bus/pci/linux/rte_pci_uio.c > > create mode 100644 drivers/bus/pci/linux/rte_pci_vfio.c > > create mode 100644 drivers/bus/pci/linux/rte_vfio_mp_sync.c > > create mode 100644 drivers/bus/pci/private.h > > create mode 100644 drivers/bus/pci/rte_bus_pci_version.map > > create mode 100644 drivers/bus/pci/rte_pci_common.c > > create mode 100644 drivers/bus/pci/rte_pci_common_uio.c > > delete mode 100644 lib/librte_eal/bsdapp/eal/eal_pci.c > > delete mode 100644 lib/librte_eal/common/eal_common_pci.c > > delete mode 100644 lib/librte_eal/common/eal_common_pci_uio.c > > delete mode 100644 lib/librte_eal/common/include/rte_pci.h > > delete mode 100644 lib/librte_eal/linuxapp/eal/eal_pci.c > > delete mode 100644 lib/librte_eal/linuxapp/eal/eal_pci_init.h > > delete mode 100644 lib/librte_eal/linuxapp/eal/eal_pci_uio.c > > delete mode 100644 lib/librte_eal/linuxapp/eal/eal_pci_vfio.c > > delete mode 100644 lib/librte_eal/linuxapp/eal/eal_vfio_mp_sync.c > > create mode 100644 lib/librte_pci/Makefile > > create mode 100644 lib/librte_pci/include/rte_pci.h > > create mode 100644 lib/librte_pci/rte_pci.c > > create mode 100644 lib/librte_pci/rte_pci_version.map > > > > > > >+#endif /* _PCI_PRIVATE_H_ */ > >diff --git a/drivers/bus/pci/rte_bus_pci_version.map b/drivers/bus/pci/rte_bus_pci_version.map > >new file mode 100644 > >index 0000000..eca49e9 > >--- /dev/null > >+++ b/drivers/bus/pci/rte_bus_pci_version.map > >@@ -0,0 +1,21 @@ > >+DPDK_17.08 { > > You might want to bump this to 17.11. > Thanks, fixing this. > >+ global: > >+ > >+ rte_pci_detach; > >+ rte_pci_dump; > >+ rte_pci_ioport_map; > >+ rte_pci_ioport_read; > >+ rte_pci_ioport_unmap; > >+ rte_pci_ioport_write; > >+ rte_pci_map_device; > >+ rte_pci_probe; > >+ rte_pci_probe_one; > >+ rte_pci_read_config; > >+ rte_pci_register; > >+ rte_pci_scan; > >+ rte_pci_unmap_device; > >+ rte_pci_unregister; > >+ rte_pci_write_config; > >+ > >+ local: *; > >+}; > > This is huuuge patch :( and I am not yet through it (most of it is movement > so I doubt anything major would be problem here). > Just the above comment in case you are spinning a new series. > > Thanks for reading the patch. Yes, most of it is moving the code as-is to a new location. I tried to reduce it, but at some point it does not really make sense anymore. I think the important thing to look for here is the build system, dependency graph and the division of the PCI API between the lib and the bus driver. I divided it along the lines of the rte_pci_device being defined or not. Anything using an rte_pci_device going to the bus and everything else going to the lib. I'm mostly worried about this divide. Having the rte_pci_device defined seems mostly of the responsibility of the bus driver, in my opinion. I'd like to hear others'. -- Gaëtan Rivet 6WIND