From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <stephen@networkplumber.org>
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 <dev@dpdk.org>; Mon, 16 Jan 2017 19:27:19 +0100 (CET)
Received: by mail-pg0-f42.google.com with SMTP id 194so17381832pgd.2
 for <dev@dpdk.org>; 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 <stephen@networkplumber.org>
To: Shreyansh Jain <shreyansh.jain@nxp.com>
Cc: <david.marchand@6wind.com>, <dev@dpdk.org>, <thomas.monjalon@6wind.com>
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 <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, 16 Jan 2017 18:27:20 -0000

On Mon, 16 Jan 2017 21:08:19 +0530
Shreyansh Jain <shreyansh.jain@nxp.com> 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'