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

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

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 <fengchengwen@huawei.com>
> > ---
> > 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 <bruce.richardson@intel.com>