On 15/09/2021 15:34, Bruce Richardson wrote:
On Wed, Sep 15, 2021 at 02:51:55PM +0100, Kevin Laatz wrote:
On 07/09/2021 13:56, Chengwen Feng wrote:
This patch introduce DMA device library implementation which includes
configuration and I/O with the DMA devices.
Signed-off-by: Chengwen Feng <fengcheng...@huawei.com>
Acked-by: Bruce Richardson <bruce.richard...@intel.com>
Acked-by: Morten Brørup <m...@smartsharesystems.com>
Reviewed-by: Kevin Laatz <kevin.la...@intel.com>
Reviewed-by: Conor Walsh <conor.wa...@intel.com>
---
config/rte_config.h | 3 +
lib/dmadev/meson.build | 1 +
lib/dmadev/rte_dmadev.c | 607 +++++++++++++++++++++++++++++++++++
lib/dmadev/rte_dmadev.h | 118 ++++++-
lib/dmadev/rte_dmadev_core.h | 2 +
lib/dmadev/version.map | 1 +
6 files changed, 720 insertions(+), 12 deletions(-)
create mode 100644 lib/dmadev/rte_dmadev.c
[snip]
/**
* @warning
@@ -941,10 +1018,27 @@ rte_dmadev_completed(uint16_t dev_id, uint16_t vchan,
const uint16_t nb_cpls,
* status array are also set.
*/
__rte_experimental
-uint16_t
+static inline uint16_t
rte_dmadev_completed_status(uint16_t dev_id, uint16_t vchan,
const uint16_t nb_cpls, uint16_t *last_idx,
- enum rte_dma_status_code *status);
+ enum rte_dma_status_code *status)
+{
+ struct rte_dmadev *dev = &rte_dmadevices[dev_id];
+ uint16_t idx;
+
+#ifdef RTE_DMADEV_DEBUG
+ if (!rte_dmadev_is_valid_dev(dev_id) || !dev->data->dev_started ||
+ vchan >= dev->data->dev_conf.nb_vchans ||
+ nb_cpls == 0 || status == NULL)
+ return 0;
+ RTE_FUNC_PTR_OR_ERR_RET(*dev->completed_status, 0);
+#endif
+
+ if (last_idx == NULL)
+ last_idx = &idx;
Hi Chengwen,
An internal coverity scan on the IDXD dmadev driver patches flagged a
potential null pointer dereference when using completed_status().
IMO it is a false positive for the driver code since it should be checked at
the library API level, however the check is also not present in the library.
For the v22, can you add the NULL pointer check for status here, like you
have for last_idx, please?
I think the check would have to be different than that for last_idx, since
the status pointer is a pointer to an array, rather than a single value -
which procludes a simple replacement in the wrapper function that the
compiler can inline away if unnecessary.
It's probably best to add it as a check in the debug block, with an
error-return if status is NULL.
+1
/Kevin
