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 A3564A0C47; Mon, 26 Jul 2021 10:31:38 +0200 (CEST) Received: from [217.70.189.124] (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 2C0D440F35; Mon, 26 Jul 2021 10:31:38 +0200 (CEST) Received: from mga09.intel.com (mga09.intel.com [134.134.136.24]) by mails.dpdk.org (Postfix) with ESMTP id E3D9040DDA for ; Mon, 26 Jul 2021 10:31:36 +0200 (CEST) X-IronPort-AV: E=McAfee;i="6200,9189,10056"; a="212179870" X-IronPort-AV: E=Sophos;i="5.84,270,1620716400"; d="scan'208";a="212179870" Received: from fmsmga007.fm.intel.com ([10.253.24.52]) by orsmga102.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 26 Jul 2021 01:31:33 -0700 X-IronPort-AV: E=Sophos;i="5.84,270,1620716400"; d="scan'208";a="436678083" Received: from bricha3-mobl.ger.corp.intel.com ([10.252.8.178]) by fmsmga007-auth.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-SHA; 26 Jul 2021 01:31:29 -0700 Date: Mon, 26 Jul 2021 09:31:25 +0100 From: Bruce Richardson To: fengchengwen Cc: thomas@monjalon.net, ferruh.yigit@intel.com, jerinj@marvell.com, jerinjacobk@gmail.com, andrew.rybchenko@oktetlabs.ru, dev@dpdk.org, mb@smartsharesystems.com, nipun.gupta@nxp.com, hemant.agrawal@nxp.com, maxime.coquelin@redhat.com, honnappa.nagarahalli@arm.com, david.marchand@redhat.com, sburla@marvell.com, pkapoor@marvell.com, konstantin.ananyev@intel.com Message-ID: References: <1625231891-2963-1-git-send-email-fengchengwen@huawei.com> <1626785182-2973-1-git-send-email-fengchengwen@huawei.com> <6d6bc86b-0bda-bd76-4a31-5d67c0299442@huawei.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <6d6bc86b-0bda-bd76-4a31-5d67c0299442@huawei.com> Subject: Re: [dpdk-dev] [PATCH v10] dmadev: introduce DMA device library 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" On Mon, Jul 26, 2021 at 02:53:16PM +0800, fengchengwen wrote: > Friendly ping. > > On 2021/7/20 20:46, Chengwen Feng wrote: > > This patch introduce 'dmadevice' which is a generic type of DMA > > device. > > > > The APIs of dmadev library exposes some generic operations which can > > enable configuration and I/O with the DMA devices. > > > > Signed-off-by: Chengwen Feng > > --- > > v10: > > * fix rte_dmadev_completed_status comment. > > [snip] I'm still working through porting over our driver(s) to the latest revisions, and digging into the details of the error handling and approaches, looking for any issues. For now, a couple of small suggestions and ideas: * for the STATUS_UNKNOWN value, I think it should be renamed to ERROR_UNKNOWN since this is an error value. Alternatively, add a new "UNKNOWN_ERROR" entry in the list to cover this possibility i.e. we know status is an error, just not exactly what the error is. * While we have errors for both invalid source or invalid destination addresses, I think we also should add a slightly more general error code for "INVALID_ADDR" to cover the case where one is bad but we are not sure which. * Not sure exactly how to handle this, but I suspect we may need some sort of capability flag to cover behaviour on hitting an error. For HW using our original ioat driver, the jobs are done strictly in order and the HW will halt on error, while with hardware using newer idxd driver, things are done potentially out-of-order and other jobs continue after the failed job. I will come back with a more concrete proposal on this later, once I get both drivers up and working. I suspect we will encounter more edge-cases like this as people work on drivers. Overall, functionally this patchset looks pretty good to me. One thing that will be needed for merge into mainline is a chapter on dmadev for the programmers guide document, plus any other necessary doc updates such as a good release-note update for this new lib. For what is here now though, Acked-by: Bruce Richardson