From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mails.dpdk.org (mails.dpdk.org [217.70.189.124]) by inbox.dpdk.org (Postfix) with ESMTP id C04DAA0547; Fri, 25 Jun 2021 05:26:52 +0200 (CEST) Received: from [217.70.189.124] (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 429E940141; Fri, 25 Jun 2021 05:26:52 +0200 (CEST) Received: from NAM12-BN8-obe.outbound.protection.outlook.com (mail-bn8nam12on2071.outbound.protection.outlook.com [40.107.237.71]) by mails.dpdk.org (Postfix) with ESMTP id DF74540040 for ; Fri, 25 Jun 2021 05:26:50 +0200 (CEST) ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=AJTf9WlVNvUk+ahr/UG0tMAltPu0dYyTY859+VoXaQIywhZXA2hmWTzI2DI7HEhlhkMDzornI3Qg0Y+Ex+hbffA97P8S7+lcQ2JYHft8qwLxV07J2dnjNUWVCI4FCPo9CEI432OBEnUJfrN90ZjzVqf1xmo+QlvdIMc5owmep02gUCcCzmhrjZr5E0YGtVH2wRXloGo4Xk62YQeeQoKPyfUxb0BD1RAmQGUVKJHjccEhmUStZni7uHrFEAGY/eFhGChqFSB+nJL6NCmSkaYgpPXw5YekvDEQ4XHnOtG3eRRQW+JctNJSC4A9T/0MNtz+kLY7FDUAZ5IHO3GqOlb+6A== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=arcselector9901; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=CUH6KO1OVhlrdL8sG5jpmokQ41MBeW/Lj7QlnmLhZlk=; b=BoWNqMmFvyMMjW0cz4mRGqHpHpvzr7U32KsDB7dHNOKW+If25xjmPpznosZzbqcuF0MMX+SyrEVxGzqOamR6s5RQoPR0F3Jb04q1rPdiSyj4vWY7rzPetcwP1aN5J2Bk0HYa9xxWLABbt670/MTv/tWMQ4Nk2eqCKUWPTaGa1ye9JV9ZVAKNbzvEJIZIigygSqSAEZWuasmIMoToZxifjl6vksiDQr23pO1KsBiNiNVsHDw6pFidyvxXJ3QDP+54n43sNzspaQrtEA+bQA5c0PnIOa/D3TXE7MjwAr2sOLVXtKyacjoOAx2Ncog5WDGMRFnhecPiEFnqG+kJgsJo1g== ARC-Authentication-Results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=nvidia.com; dmarc=pass action=none header.from=nvidia.com; dkim=pass header.d=nvidia.com; arc=none DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=Nvidia.com; s=selector2; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=CUH6KO1OVhlrdL8sG5jpmokQ41MBeW/Lj7QlnmLhZlk=; b=riwJ18wXGgLshWeKgwtdMouNW7RGAkNv2jbPkdsrlwyc35HAkSh5pQWfN6yh5zG5SXGJWjb1QumuLPF21qsNBK0PNvKvOjKfAM053ayp4AGL+IpWr5+cM06sEOavpJVxmtOdsWRCOFx1L5LtzcnN6M9rk8iPrBYtyafXEEeK0IsHj39bnGV+eqocbzl7cjGTgJaRy5wFA/My+zvcSiaPMgjKouY/eLCwvWWpH/Q6CZ7PwghLk3x+1Edr9YB/YdRv0+CSihSYXAv+eVyO7p6Xv/2xLbZBny/Jy7C0gbboN8oKKFt542DvTJgzGnin/JgIKimxG1ea0U29+XDlfP50xg== Received: from DM4PR12MB5373.namprd12.prod.outlook.com (2603:10b6:5:39a::17) by DM4PR12MB5039.namprd12.prod.outlook.com (2603:10b6:5:38a::18) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.4264.18; Fri, 25 Jun 2021 03:26:48 +0000 Received: from DM4PR12MB5373.namprd12.prod.outlook.com ([fe80::ac75:9b9a:a03f:1676]) by DM4PR12MB5373.namprd12.prod.outlook.com ([fe80::ac75:9b9a:a03f:1676%3]) with mapi id 15.20.4264.023; Fri, 25 Jun 2021 03:26:48 +0000 From: "Xueming(Steven) Li" To: NBU-Contact-Thomas Monjalon CC: "dev@dpdk.org" , Wang Haiyue , Kinsella Ray Thread-Topic: [dpdk-dev] [PATCH v5 2/2] bus/auxiliary: introduce auxiliary bus Thread-Index: AQHXZ8N2ArsEUTckik+THYbEFs3bcKsjWayAgACs6wA= Date: Fri, 25 Jun 2021 03:26:48 +0000 Message-ID: References: <20210613125846.19852-1-xuemingl@nvidia.com> <20210623000349.631468-2-xuemingl@nvidia.com> <2020318.PlHIg8rldJ@thomas> In-Reply-To: <2020318.PlHIg8rldJ@thomas> Accept-Language: en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: authentication-results: monjalon.net; dkim=none (message not signed) header.d=none;monjalon.net; dmarc=none action=none header.from=nvidia.com; x-originating-ip: [115.236.163.138] x-ms-publictraffictype: Email x-ms-office365-filtering-correlation-id: 3fe2ac90-0f0f-4b43-229b-08d9378910c6 x-ms-traffictypediagnostic: DM4PR12MB5039: x-ld-processed: 43083d15-7273-40c1-b7db-39efd9ccc17a,ExtAddr x-microsoft-antispam-prvs: x-ms-oob-tlc-oobclassifiers: OLM:1079; x-ms-exchange-senderadcheck: 1 x-microsoft-antispam: BCL:0; x-microsoft-antispam-message-info: Z45LoIap8qTtucuA/PurBDcALrRtwkcqBwgP+SUou1n1teDO2F3QT75tntUOO2V4yISH45eKtlQM8y6Gol/UQCtHTHlXjXm8UyIUN7F2ekAfZraoNdAP9y76JzU4wzhubKezqTtrjrHgYNZzJirF5eyQKT6NgK8pMi5OdAT4rybp/5XPxdJ89vlqV2jxIu7WneL2EsdeEQ6mpyDRlN9BSpVG6XmZEYZHVbOFEhIDi85KT+d2+hTvKt7CYPjanGsgyNAau1s3vSdgHVQ2UOYpwu9zPmxnxa9Vctr35bnAN0b6ecC0ju1ET23GfXmyXY0U1R3h5Kt7ZJxbM8sZFIojPp101Gkjj67wo/RmT19oyfMCyhrVnGciTO5aWQOoyjhna2fpSTBWC24oSe6erkU/0rqamCUpkBbu6wCpnx+Y7Wp8jri36otVfoYrtUwU5MNNofh9luzwKXx6SykGr49MK/utdmKk7/l7Dh7qqypvOcCk3hJrIzLQO25Fxtsf9k/TC88wr89arpG8vhj4QJZ9hbH804irNTGeYko+6CNKcZsQ4uRmF3HOffgW76yxprSdy5G8mv325PgkD0x2kIHNaZA8bPTc+pLKBEGmUHPNm+lPVqteEtsLO7twCV0PWrBGyFT9MTaZJ37D/oLq4Cw/CA== x-forefront-antispam-report: CIP:255.255.255.255; CTRY:; LANG:en; SCL:1; SRV:; IPV:NLI; SFV:NSPM; H:DM4PR12MB5373.namprd12.prod.outlook.com; PTR:; CAT:NONE; SFS:(4636009)(39860400002)(376002)(366004)(396003)(136003)(346002)(478600001)(66946007)(8936002)(186003)(6506007)(6916009)(26005)(66446008)(64756008)(66556008)(54906003)(53546011)(316002)(66476007)(7696005)(8676002)(5660300002)(71200400001)(52536014)(86362001)(38100700002)(122000001)(83380400001)(76116006)(55016002)(9686003)(33656002)(2906002)(4326008); DIR:OUT; SFP:1101; x-ms-exchange-antispam-messagedata-chunkcount: 1 x-ms-exchange-antispam-messagedata-0: =?us-ascii?Q?1LkCQoZB8mRRR/GryqiMe5mtfrKSgyqdZPrfdeHNVid/vvYAXiCBioGI17Pc?= =?us-ascii?Q?vKEI1sSC43qbJQHkYnXTGWKCg6PPmRnutkKIq2EV36KYw7cgct90Y01rH/Jc?= =?us-ascii?Q?AblKuW5YlGbHsU3vUAY9zu6XeMjeWHM+xSPv1NvKhRueumBQfHtHaOsfDxh2?= =?us-ascii?Q?YfvBlZavpXK91PU6re6P5BR1IwHVRTzRBMLJoLQDn2MnenJSDjRVQZmqYyUS?= =?us-ascii?Q?n0SSXbS/SzOTU68DywgC4hx8lVDSnK5sbqnqLp6vjEAMo+KHkAaRLxpZprvR?= =?us-ascii?Q?Hknp+DT1eMELc8qtG/2JttK8psRJ1zIvROqEuV4BnnSedOosXu6KvEfmpU5o?= =?us-ascii?Q?j15WJlz92xqDqFNFBlUPdTFAiDDUOUIONOEk+Q8O/EXmg/szI34lGfOfXQzU?= =?us-ascii?Q?1oPj1oiFAVqVs3CgdqA1zn92NG1naYMmOUUUPSdnvCkGDHk0wCwkRFN2zhhL?= =?us-ascii?Q?mtPzsbNRmC823EIGql71EtgOweSPqRZfspj1sTdTiIoqBoD1eEebVQpvAknr?= =?us-ascii?Q?EcTWE3BrQwLElCEh7Z88yVqVx2jPWRt19FfdEFuXkBqzms0fcGgG7g3p2wM0?= =?us-ascii?Q?f/LYiheABst8Q2mdpD08enJBpwCrq3Wq8gnK2/P0xAraT3NAnjwmA26ZFHT/?= =?us-ascii?Q?dx7kV2OjiJ29Hl+Ym1myZe66IjF86nGmZJKcU4w10C7//r4pmLlCME9l4of+?= =?us-ascii?Q?PRp9IVtFheHAAZtu/uWUq6Bab8jnJM9ompw9L2hVF9CzLowRfkV5mYNIHaEL?= =?us-ascii?Q?NfbDtVfX21kvBzMK3lJaqhLYBnWEVrSgt+uZBOTZH6f3qmjhm4KCkaV2OeOH?= =?us-ascii?Q?b4A092J8MgkD2Fx6vsC39u9SgcwoLFa7SfHThF7XBZ5tLAL28bdKrZkg7+iB?= =?us-ascii?Q?+IZtisMm6Zd7BLIMc4rgQ56xj5x07YIr/OdPmlRqRyvNlY3hUprD6oop+0Sx?= =?us-ascii?Q?kbU8/s3R/WTCDKZJaQEflCPgWne8ihOlKpAjjvkDcbLOjpGtZXytC3+zAha5?= =?us-ascii?Q?gvk4Mod7Nm/J3KTwqwn1v8mrPDQeWehIZuFktSeJAc9XZXaHcjuJTVJzpzVw?= =?us-ascii?Q?TN9LJFSKaIsA1S0KEMvVZfG0ahAriRkN4Yn0prta6Ui17HmJ+1svCxJqzVY0?= =?us-ascii?Q?5/xPbvkIZ9MFb7S0DsGK8OnQWNEZ6wNJBoCDRIYeoO70buwvVF6p7i4m9CJL?= =?us-ascii?Q?JDpQfR1V/Ze9JqB6C165jA1y2nUDhILW+zPfANXz1G5GjV8xRBENBNR56cb2?= =?us-ascii?Q?YUJnx+YJiu4yWJkRCvcWrlhPcNijiNgYShLsJT4PyRp50p3rxuP4BUQKnh7e?= =?us-ascii?Q?1aZEkSX8x3ATeGvM/ASXKgBu?= x-ms-exchange-transport-forked: True Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 X-OriginatorOrg: Nvidia.com X-MS-Exchange-CrossTenant-AuthAs: Internal X-MS-Exchange-CrossTenant-AuthSource: DM4PR12MB5373.namprd12.prod.outlook.com X-MS-Exchange-CrossTenant-Network-Message-Id: 3fe2ac90-0f0f-4b43-229b-08d9378910c6 X-MS-Exchange-CrossTenant-originalarrivaltime: 25 Jun 2021 03:26:48.0356 (UTC) X-MS-Exchange-CrossTenant-fromentityheader: Hosted X-MS-Exchange-CrossTenant-id: 43083d15-7273-40c1-b7db-39efd9ccc17a X-MS-Exchange-CrossTenant-mailboxtype: HOSTED X-MS-Exchange-CrossTenant-userprincipalname: WBCxF/pSP6rK14AdsI9B9Q/ccGBe5yaJDJJcI/WkwvrRPy7yIz3l34OBlN1wgduC2LTce/xG4z71h0CL7c8XDA== X-MS-Exchange-Transport-CrossTenantHeadersStamped: DM4PR12MB5039 Subject: Re: [dpdk-dev] [PATCH v5 2/2] bus/auxiliary: introduce auxiliary bus X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" > -----Original Message----- > From: Thomas Monjalon > Sent: Friday, June 25, 2021 12:19 AM > To: Xueming(Steven) Li > Cc: dev@dpdk.org; Wang Haiyue ; Kinsella Ray > Subject: Re: [dpdk-dev] [PATCH v5 2/2] bus/auxiliary: introduce auxiliary= bus >=20 > 23/06/2021 02:03, Xueming Li: > > +static int > > +rte_auxiliary_probe_one_driver(struct rte_auxiliary_driver *drv, > > + struct rte_auxiliary_device *dev) > [...] > > + AUXILIARY_LOG(DEBUG, "Probe driver: %s", drv->driver.name); >=20 > I think the debug log above is useless given the ones below. >=20 > > + > > + iova_mode =3D rte_eal_iova_mode(); > > + if ((drv->drv_flags & RTE_AUXILIARY_DRV_NEED_IOVA_AS_VA) > 0 && > > + iova_mode !=3D RTE_IOVA_VA) { > > + AUXILIARY_LOG(ERR, "Expecting VA IOVA mode but current mode is PA, > > +not initializing"); >=20 > The error log should mention which driver is requesting VA and not initia= lizing. >=20 > > + return -EINVAL; > > + } > > + > > + dev->driver =3D drv; > > + > > + AUXILIARY_LOG(INFO, "Probe auxiliary driver: %s device: %s (socket %i= )", > > + drv->driver.name, dev->name, dev->device.numa_node); > > + ret =3D drv->probe(drv, dev); > > + if (ret !=3D 0) > > + dev->driver =3D NULL; > > + else > > + dev->device.driver =3D &drv->driver; > > + > > + return ret; > > +} > > + > > +/* > > + * Call the remove() function of the driver. > > + */ > > +static int > > +rte_auxiliary_driver_remove_dev(struct rte_auxiliary_device *dev) { > > + struct rte_auxiliary_driver *drv; > > + int ret =3D 0; > > + > > + if (dev =3D=3D NULL) > > + return -EINVAL; > > + > > + drv =3D dev->driver; > > + > > + AUXILIARY_LOG(DEBUG, "Auxiliary device %s on NUMA socket %i", > > + dev->name, dev->device.numa_node); > > + > > + AUXILIARY_LOG(DEBUG, "remove driver: %s %s", > > + dev->name, drv->driver.name); >=20 > Better to merge both logs together. >=20 > > + > > + if (drv->remove !=3D NULL) { > > + ret =3D drv->remove(dev); > > + if (ret < 0) > > + return ret; > > + } > > + > > + /* clear driver structure */ > > + dev->driver =3D NULL; > > + dev->device.driver =3D NULL; > > + > > + return 0; > > +} >=20 > [...] > > +static int > > +auxiliary_parse(const char *name, void *addr) { > > + struct rte_auxiliary_driver *drv =3D NULL; > > + const char **out =3D addr; > > + > > + /* Allow dummy name to prevent bus scan. */ > > + if (strlen(name) =3D=3D 0) > > + return 0; >=20 > Which syntax is it? Allow empty device name "auxiliary:" to bypass entire auxiliary bus scan. >=20 > [...] > > + kvargs =3D rte_kvargs_parse(str, auxiliary_params_keys); > > + if (kvargs =3D=3D NULL) { > > + RTE_LOG(ERR, EAL, "cannot parse argument list\n"); >=20 > Should use AUXILIARY_LOG. >=20 > [...] > > +++ b/drivers/bus/auxiliary/linux/auxiliary.c > > + /* Get numa node, default to 0 if not present */ >=20 > s/numa/NUMA/ >=20 > [...] > > +/* > > + * Scan the content of the auxiliary bus, and the devices in the > > +devices > > + * list > > + */ >=20 > Simpler: Scan the devices in the auxiliary bus. >=20 > > +int > > +auxiliary_scan(void) > > +{ >=20 > [...] > > +++ b/drivers/bus/auxiliary/private.h > > @@ -0,0 +1,75 @@ > > +/* SPDX-License-Identifier: BSD-3-Clause > > + * Copyright (c) 2021 NVIDIA Corporation & Affiliates */ > > + > > +#ifndef _AUXILIARY_PRIVATE_H_ > > +#define _AUXILIARY_PRIVATE_H_ >=20 > nit: we can avoid the first and final underscores >=20 > [...] > > +/* > > + * Validate whether a device with given auxiliary device should be > > +ignored > > + * or not. > > + */ > > +bool auxiliary_ignore_device(const char *name); >=20 > Suggestion: auxiliary_is_ignored_device() ? >=20 > > +/* > > + * Add an auxiliary device to the auxiliary bus (append to auxiliary > > +Device >=20 > s/Device/device/ >=20 > > + * list). This function also updates the bus references of the > > + auxiliary > > + * Device (and the generic device object embedded within. >=20 > s/(and/and/ >=20 > > + */ > > +void auxiliary_add_device(struct rte_auxiliary_device *aux_dev); > > + > > +/* > > + * Insert an auxiliary device in the auxiliary bus at a particular > > +location > > + * in the device list. It also updates the auxiliary bus reference of > > +the > > + * new devices to be inserted. > > + */ > > +void auxiliary_insert_device(struct rte_auxiliary_device *exist_aux_de= v, > > + struct rte_auxiliary_device *new_aux_dev); > > + > > +/* > > + * Match the auxiliary Driver and Device by driver function >=20 > no need of capital letters in driver and device. >=20 > > + */ > > +bool auxiliary_match(const struct rte_auxiliary_driver *aux_drv, > > + const struct rte_auxiliary_device *aux_dev); > > + > > +/* > > + * Iterate over internal devices, matching any device against the > > +provided >=20 > What do you mean by "internal"? >=20 > > + * string. > > + */ > > +void *auxiliary_dev_iterate(const void *start, const char *str, > > + const struct rte_dev_iterator *it); > > + >=20 > [...] > > +++ b/drivers/bus/auxiliary/rte_bus_auxiliary.h > > +struct rte_auxiliary_driver { > > + TAILQ_ENTRY(rte_auxiliary_driver) next; /**< Next in list. */ > > + struct rte_driver driver; /**< Inherit core driver. */ > > + struct rte_auxiliary_bus *bus; /**< Auxiliary bus reference. */ > > + rte_auxiliary_match_t *match; /**< Device match function. */ > > + rte_auxiliary_probe_t *probe; /**< Device Probe function. */ > > + rte_auxiliary_remove_t *remove; /**< Device Remove function. */ >=20 > No capital in Probe and Remove >=20 > > + rte_auxiliary_dma_map_t *dma_map; /**< Device dma map function. *= / > > + rte_auxiliary_dma_unmap_t *dma_unmap; /**< Device dma unmap > > +function. */ >=20 > s/dma/DMA/ > There may be other occurences to check. >=20 > > + uint32_t drv_flags; /**< Flags RTE_AUXILIARY_DRV_*. = */ > > +}; >=20 > The alignment of some comments above can be improved by one space. >=20 > [...] > > +/** Helper for auxiliary device registration from driver instance */ > > +#define RTE_PMD_REGISTER_AUXILIARY(nm, auxiliary_drv) \ > > + RTE_INIT(auxiliaryinitfn_ ##nm) \ > > + { \ > > + (auxiliary_drv).driver.name =3D RTE_STR(nm); \ > > + rte_auxiliary_register(&(auxiliary_drv)); \ > > + } \ > > + RTE_PMD_EXPORT_NAME(nm, __COUNTER__) >=20 > I think it is better not trying to align backslashes in such macro, but l= eave only a single space. >=20 > [...] > > +++ b/drivers/bus/auxiliary/version.map > > @@ -0,0 +1,3 @@ > > +EXPERIMENTAL { > > + # added in 21.08 > > +}; >=20 > Looks like you need to bring the symbols back :) >=20 > Looks good overrall, thanks. >=20