From mboxrd@z Thu Jan 1 00:00:00 1970
Return-Path: <gaetan.rivet@6wind.com>
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 <dev@dpdk.org>; Mon, 18 Sep 2017 13:52:02 +0200 (CEST)
Received: by mail-wm0-f54.google.com with SMTP id 13so1897152wmq.2
for <dev@dpdk.org>; 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 <gaetan.rivet@6wind.com>
To: Shreyansh Jain <shreyansh.jain@nxp.com>
Cc: dev@dpdk.org
Message-ID: <20170918115151.GS21444@bidouze.vm.6wind.com>
References: <cover.1503651392.git.gaetan.rivet@6wind.com>
<cover.1505726803.git.gaetan.rivet@6wind.com>
<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 <dev.dpdk.org>
List-Unsubscribe: <http://dpdk.org/ml/options/dev>,
<mailto:dev-request@dpdk.org?subject=unsubscribe>
List-Archive: <http://dpdk.org/ml/archives/dev/>
List-Post: <mailto:dev@dpdk.org>
List-Help: <mailto:dev-request@dpdk.org?subject=help>
List-Subscribe: <http://dpdk.org/ml/listinfo/dev>,
<mailto:dev-request@dpdk.org?subject=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 <gaetan.rivet@6wind.com>
> >---
> > 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
> >
>
> <lot of snip here...>
>
> >+#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