From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-pg0-f42.google.com (mail-pg0-f42.google.com [74.125.83.42]) by dpdk.org (Postfix) with ESMTP id AB4BCDE5 for ; Mon, 16 Jan 2017 19:27:19 +0100 (CET) Received: by mail-pg0-f42.google.com with SMTP id 194so17381832pgd.2 for ; Mon, 16 Jan 2017 10:27:19 -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-transfer-encoding; bh=EDbZkcIdmMbMpQnLskqTvLEeuXcscPxrfRlVm+SQY6c=; b=i5P6MrCdT5QxDUfL6AIpFHaxuSVWlX5YEIEnoY/CCwM7vY0zOP4mdufe5M5gqWG9r6 DF61/gaBvZyAoz0dccXmuG2QA1HtWMMcaMtgxsPoOA8igpaA+a1SAmbB34uXtRw1eh/a rxrt0wuVBLSVwH83FqeeZdFJTe5THTTSdRmI+p31b0Zs3HP++woS5qzrGHu+RfEr6BTX UqKz+i+l47dU6N5+aBycMhLcgNSOX1cMNrTjEec+UUkGi+p7KU+mbzefjLuf2Bi44pnS sQcuB6j3V7bhm3XBRvP7AVZGLBKJsrAsbZmKs6nmX53DatMccD+xhURyya+ZGgvfva2C MlXg== 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:in-reply-to :references:mime-version:content-transfer-encoding; bh=EDbZkcIdmMbMpQnLskqTvLEeuXcscPxrfRlVm+SQY6c=; b=Tj+ToJzCh9O9WHXmBZWTttctPhGMZuCn33rcMmZy++pOfn2AP3bnfc5IjXACDV750L KgopfjHUXSHX1YfaP7ZrRHBCcFnVD3KQJEnQ8U6tGkjqAZ7hFsDERB7wdSNX6dNLYzD1 D1DkzSMtodh1SkCjYQxPIJM4gDV97kbHRG5KuRewPQb+ebquS594ebBhgY0/F7wZhSIx ZaF/6yS2WdSiibWzbWvQS3eYlGwRGAfMM9HVwOrbQG0Zpha/hp236plwHMn1n1uUp5wG Xo2e5d82QOzo91Xq2F5zDvPiOUXN3Io/qzp8xU1/eyXQyjxt1kTcjzHSUEXnMTklpi4A E36A== X-Gm-Message-State: AIkVDXIgyrWGALfx/YrTE03nyflvLCm7T6a0T0gdctBmirfL37WWPrsfJsFBgBz1+UnBJA== X-Received: by 10.99.237.17 with SMTP id d17mr41962995pgi.82.1484591238612; Mon, 16 Jan 2017 10:27:18 -0800 (PST) Received: from xeon-e3 (204-195-18-65.wavecable.com. [204.195.18.65]) by smtp.gmail.com with ESMTPSA id u78sm49380882pfa.53.2017.01.16.10.27.18 (version=TLS1_2 cipher=ECDHE-RSA-CHACHA20-POLY1305 bits=256/256); Mon, 16 Jan 2017 10:27:18 -0800 (PST) Date: Mon, 16 Jan 2017 10:27:06 -0800 From: Stephen Hemminger To: Shreyansh Jain Cc: , , Message-ID: <20170116102706.58d1738a@xeon-e3> In-Reply-To: <1484581107-2025-1-git-send-email-shreyansh.jain@nxp.com> References: <1484581107-2025-1-git-send-email-shreyansh.jain@nxp.com> MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Subject: Re: [dpdk-dev] [PATCH v6 0/8] Introducing EAL Bus-Device-Driver Model 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, 16 Jan 2017 18:27:20 -0000 On Mon, 16 Jan 2017 21:08:19 +0530 Shreyansh Jain wrote: > Link to v5: [15] > > :: Introduction :: > > DPDK has been inherently a PCI inclined framework. Because of this, the > design of device tree (or list) within DPDK is also PCI inclined. A > non-PCI device doesn't have a way of being expressed without using hooks > started from EAL to PMD. > > (Check 'Version Changes' section for changes) > > :: Overview of the Proposed Changes :: > > Assuming the below graph for a computing node: > > device A1 > | > +==.===='==============.============+ Bus A. > | `--> driver A11 \ > device A2 `-> driver A12 \______ > |CPU | > /````` > device B1 / > | / > +==.===='==============.============+ Bus B` > | `--> driver B11 > device B2 `-> driver B12 > > > - One or more buses are connected to a CPU (or core) > - One or more devices are conneted to a Bus > - Drivers are running instances which manage one or more devices > - Bus is responsible for identifying devices (and interrupt propogation) > - Driver is responsible for initializing the device > > In [15], model assumes that rte_bus would be the base class using which > all the bus implementations would instantiate the objects. This patches > takes a much more basic approach, on the same lines of rte_device/ > rte_driver and rte_pci_device/rte_pci_driver. > This is based on various review comments as well as offline (IRC) > discussions. > > - rte_bus is an abstract class which includes basic methods supported by > all buses. > - specific implementation, for example for PCI rte_pci_bus, would extend > this class to form their own buses, with their own bus specific device > and driver list. > - > > +-----------------+ > |rte_pci_bus | > |+---------------+| > ||rte_bus || > |+---------------+| > +-----------------+ > > And example implementation would look like: > > .--------------->+-------------------+ > | |rte_pci_bus | > | | +----------------+| > | | |rte_bus <------. > | | | name || | > | | | scan || | > | | | probe || | > | | | attach || | > | | | detach || | > | | +----------------+| | > | | pci_driver_list | | > | .-------------->pci_device_list | | > | | | ... | | > | | +-------------------+ | > | | | > | +-------------------+ | > | |rte_pci_device | | > '----bus | | > | +----------------+| | > | |rte_device || | > | | bus --------------------------------' > | | ... || > | +----------------+| > | ... | > +-------------------+ > > > :: Brief about Patch Layout :: > > 0001~0002: Introducing the basic Bus model and associated test case > 0003 : Split the PCI match into a separate function > 0004~0005: Introduce bus->scan APIs and integrate with EAL > 0006 : Update the Bus and PCI test cases for scanning/probing > 0007 : Enable PCI bus and remove code for direct PCI scan/probe from > EAL > 0008 : Add device hotplugging over the bus and introduce PCI helpers > > > :: Pending Changes/Caveats :: > > 1. This patchset only moves the PCI into a bus. And, that movement is also > currently part of the EAL (lib/librte_eal/linux) > Eventual aim is the PCI bus reside in driver/bus/pci > > 2. Though the implementation for bus is common for Linux and BSD, the PCI > bus implementation has been done/tested only for Linux. > > 3. RTE_REGISTER_BUS has been declared with contructor priority of 101 > It is important that Bus is registered *before* drivers are registered. > Only way I could find to assure that was via > __attribute(contructor(priority)) of GCC. I am not sure how it would > behave on other compilers. Any suggestions? > - One suggestion from David Marchand was to use global bus object > handles, which I have not implemented for now. If that is common > choice, I will change in v3. > > :: ToDo list :: > > - Bump to librte_eal version > - Documentation continues to have references to some _old_ PCI symbols > > :: References :: > > [1] http://dpdk.org/ml/archives/dev/2016-November/050186.html > [2] http://dpdk.org/ml/archives/dev/2016-November/050622.html > [3] http://dpdk.org/ml/archives/dev/2016-November/050416.html > [4] http://dpdk.org/ml/archives/dev/2016-November/050567.html > [5] http://dpdk.org/ml/archives/dev/2016-November/050628.html > [6] http://dpdk.org/ml/archives/dev/2016-November/050415.html > [7] http://dpdk.org/ml/archives/dev/2016-November/050443.html > [8] http://dpdk.org/ml/archives/dev/2016-November/050624.html > [9] http://dpdk.org/ml/archives/dev/2016-November/050296.html > [10] http://dpdk.org/ml/archives/dev/2016-December/051349.html > [12] http://dpdk.org/ml/archives/dev/2016-December/052092.html > [13] http://dpdk.org/ml/archives/dev/2016-December/052381.html > [14] http://dpdk.org/ml/archives/dev/2016-December/053302.html > [15] http://dpdk.org/ml/archives/dev/2016-December/053315.html > > :: Version Changes :: > v6: > - Rearchitecture to bring bus object parallel to rte_device/driver > This majorly includes: > -- rte_pci_bus class and pci_bus as its object for PCI > -- bus->attach/detach (hotplugging) > -- removing bus->match as that is local to an implementation > - rename symbols rte_eal_bus_* to rte_bus_* > - restructuring patches (order) for simplicity > - update to test_pci > > v5: > - Fix checkpatch error in Patch 0003 > > v4: > - rebase over master (eac901ce) > - Fix a bug in test_bus while setup and cleanup > - rename rte_eal_get_bus to rte_eal_bus_get > - Add helper (iterator) macros for easy bus,device,driver traversal > - removed 0001 patch as it is already merged in master > - fix missing rte_eal_bus_insert_device symbol in map file > > v3: > - rebase over master (c431384c8f) > - revert patch 0001 changes for checkpatch (container_of macro) > - qat/rte_qat_cryptodev update for rte_driver->probe > - test_pci update for using a test_pci_bus for verification > - some bug fixes based on internal testing. > -- rte_eal_dev_attach not handling devargs > -- blacklisting not working > > v2: > - No more bus->probe() > Now, rte_eal_bus_probe() calls rte_driver->probe based on match output > - new functions, rte_eal_pci_probe and rte_eal_pci_remove have been > added as glue code between PCI PMDs and PCI Bus > `-> PMDs are updated to use these new functions as callbacks for > rte_driver > - 'default' keyword has been removed from match and scan > - Fix for incorrect changes in mlx* and nicvf* > - Checkpatch fixes > - Some variable checks have been removed from internal functions; > functions which are externally visible continue to have such checks > - Some rearrangement of patches: > -- changes to drivers have been separated from EAL changes (but this > does make PCI PMDs non-working for a particular patch) > > Shreyansh Jain (8): > eal/bus: introduce bus abstraction > test: add basic bus infrastructure tests > pci: split match and probe function > eal/bus: support for scanning of bus > eal: introduce bus scan and probe in EAL > test: update bus and pci unit test cases > eal: enable PCI bus > eal: enable hotplugging of devices on bus > > app/test/Makefile | 2 +- > app/test/test.h | 2 + > app/test/test_bus.c | 686 ++++++++++++++++++++++++ > app/test/test_pci.c | 164 ++++-- > lib/librte_eal/bsdapp/eal/Makefile | 1 + > lib/librte_eal/bsdapp/eal/eal.c | 13 +- > lib/librte_eal/bsdapp/eal/eal_pci.c | 13 + > lib/librte_eal/bsdapp/eal/rte_eal_version.map | 15 +- > lib/librte_eal/common/Makefile | 2 +- > lib/librte_eal/common/eal_common_bus.c | 140 +++++ > lib/librte_eal/common/eal_common_dev.c | 56 +- > lib/librte_eal/common/eal_common_pci.c | 330 ++++++++---- > lib/librte_eal/common/eal_private.h | 10 - > lib/librte_eal/common/include/rte_bus.h | 206 +++++++ > lib/librte_eal/common/include/rte_dev.h | 1 + > lib/librte_eal/common/include/rte_pci.h | 161 +++++- > lib/librte_eal/linuxapp/eal/Makefile | 1 + > lib/librte_eal/linuxapp/eal/eal.c | 13 +- > lib/librte_eal/linuxapp/eal/eal_pci.c | 53 +- > lib/librte_eal/linuxapp/eal/rte_eal_version.map | 15 +- > 20 files changed, 1646 insertions(+), 238 deletions(-) > create mode 100644 app/test/test_bus.c > create mode 100644 lib/librte_eal/common/eal_common_bus.c > create mode 100644 lib/librte_eal/common/include/rte_bus.h > All this is excellent, but doesn't go far enough to break the assumptions of 'struct eth_driver'