Re: [dpdk-dev] [RFC PATCH] dmadev: introduce DMA device library

[Bruce Richardson <bruce.richardson@intel.com>]

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 <fengchengwen@huawei.com>
> ---
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);