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 44318A0C44; Tue, 15 Jun 2021 18:39:39 +0200 (CEST) Received: from [217.70.189.124] (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id BBE344067A; Tue, 15 Jun 2021 18:39:38 +0200 (CEST) Received: from mga17.intel.com (mga17.intel.com [192.55.52.151]) by mails.dpdk.org (Postfix) with ESMTP id BBC9640140 for ; Tue, 15 Jun 2021 18:39:36 +0200 (CEST) IronPort-SDR: lhmtfayK/SOnb4JQ0PenP9eMDG2nFsrCfTYNYFU0tRh2S/59S/mCO6Wc1Ko8wTT5pItNHtPgRb HMzCTVhbmyaw== X-IronPort-AV: E=McAfee;i="6200,9189,10016"; a="186404309" X-IronPort-AV: E=Sophos;i="5.83,275,1616482800"; d="scan'208";a="186404309" Received: from orsmga001.jf.intel.com ([10.7.209.18]) by fmsmga107.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 15 Jun 2021 09:38:41 -0700 IronPort-SDR: 6lv3ajY4ML1xzr8Eom87eVRHUZpTDQeKXy3jIG1nGfgrcdmVk7mRg9NRRQlMGB4ilLz6gO9U0x l+OiRsvkK2YA== X-IronPort-AV: E=Sophos;i="5.83,275,1616482800"; d="scan'208";a="484531810" Received: from bricha3-mobl.ger.corp.intel.com ([10.252.31.246]) by orsmga001-auth.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-SHA; 15 Jun 2021 09:38:39 -0700 Date: Tue, 15 Jun 2021 17:38:34 +0100 From: Bruce Richardson To: Chengwen Feng Cc: thomas@monjalon.net, ferruh.yigit@intel.com, dev@dpdk.org, nipun.gupta@nxp.com, hemant.agrawal@nxp.com, maxime.coquelin@redhat.com, honnappa.nagarahalli@arm.com, jerinj@marvell.com, david.marchand@redhat.com, jerinjacobk@gmail.com Message-ID: References: <1623763327-30987-1-git-send-email-fengchengwen@huawei.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1623763327-30987-1-git-send-email-fengchengwen@huawei.com> Subject: Re: [dpdk-dev] [RFC PATCH] 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 Tue, Jun 15, 2021 at 09:22:07PM +0800, Chengwen Feng wrote: > This patch introduces '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 > --- Thanks for sending this. Of most interest to me right now are the key data-plane APIs. While we are still in the prototyping phase, below is a draft of what we are thinking for the key enqueue/perform_ops/completed_ops APIs. Some key differences I note in below vs your original RFC: * Use of void pointers rather than iova addresses. While using iova's makes sense in the general case when using hardware, in that it can work with both physical addresses and virtual addresses, if we change the APIs to use void pointers instead it will still work for DPDK in VA mode, while at the same time allow use of software fallbacks in error cases, and also a stub driver than uses memcpy in the background. Finally, using iova's makes the APIs a lot more awkward to use with anything but mbufs or similar buffers where we already have a pre-computed physical address. * Use of id values rather than user-provided handles. Allowing the user/app to manage the amount of data stored per operation is a better solution, I feel than proscribing a certain about of in-driver tracking. Some apps may not care about anything other than a job being completed, while other apps may have significant metadata to be tracked. Taking the user-context handles out of the API also makes the driver code simpler. * I've kept a single combined API for completions, which differs from the separate error handling completion API you propose. I need to give the two function approach a bit of thought, but likely both could work. If we (likely) never expect failed ops, then the specifics of error handling should not matter that much. For the rest, the control / setup APIs are likely to be rather uncontroversial, I suspect. However, I think that rather than xstats APIs, the library should first provide a set of standardized stats like ethdev does. If driver-specific stats are needed, we can add xstats later to the API. Appreciate your further thoughts on this, thanks. Regards, /Bruce /** * @warning * @b EXPERIMENTAL: this API may change without prior notice. * * Enqueue a copy operation onto the DMA device * * This queues up a copy operation to be performed by hardware, but does not * trigger hardware to begin that operation. * * @param dev_id * The dmadev device id of the DMA instance * @param src * The source buffer * @param dst * The destination buffer * @param length * The length of the data to be copied * @return * - On success, id (uint16_t) of job enqueued * - On failure, negative error code */ static inline int __rte_experimental rte_dmadev_enqueue_copy(uint16_t dev_id, void * src, void * dst, unsigned int length); /** * @warning * @b EXPERIMENTAL: this API may change without prior notice. * * Trigger hardware to begin performing enqueued operations * * This API is used to write the "doorbell" to the hardware to trigger it * to begin the operations previously enqueued by e.g. rte_dmadev_enqueue_copy() * * @param dev_id * The dmadev device id of the DMA instance * @return * 0 on success, negative errno on error */ static inline int __rte_experimental rte_dmadev_perform_ops(uint16_t dev_id); /** * @warning * @b EXPERIMENTAL: this API may change without prior notice. * * Returns details of operations that have been completed * * In the normal case of no failures in hardware performing the requested jobs, * the return value is the ID of the last completed operation, and * "num_reported_status" is 0. * * If errors have occured the status of "num_reported_status" (<= "max_status") * operations are reported in the "status" array, with the return value being * the ID of the last operation reported in that array. * * @param dev_id * The dmadev device id of the DMA instance * @param max_status * The number of entries which can fit in the status arrays, i.e. max number * of completed operations to report. * @param[out] status * Array to hold the status of each completed operation. * A value of RTE_DMA_OP_SKIPPED implies an operation was not attempted, * and any other non-zero value indicates operation failure. * @param[out] num_reported_status * Returns the number of elements in status. If this value is returned as * zero (the expected case), the status array will not have been modified * by the function and need not be checked by software * @return * On success, ID of the last completed/reported operation. * Negative errno on error, with all parameters unmodified. */ static inline int __rte_experimental rte_dmadev_completed_ops(uint16_t dev_id, uint8_t max_status, uint32_t *status, uint8_t *num_reported_status);