On 21/08/2020 17:29, Bruce Richardson wrote:
Rather than having a single long complicated header file for general use we
can split things so that there is one header with all the publically needed
information - data structs and function prototypes - while the rest of the
internal details are put separately. This makes it easier to read,
understand and use the APIs.

Signed-off-by: Bruce Richardson <bruce.richard...@intel.com>
---

There are a couple of checkpatch errors about spacing in this patch,
however, it appears that these are false positives.
---
  drivers/raw/ioat/meson.build           |   1 +
  drivers/raw/ioat/rte_ioat_rawdev.h     | 144 +---------------------
  drivers/raw/ioat/rte_ioat_rawdev_fns.h | 164 +++++++++++++++++++++++++
  3 files changed, 171 insertions(+), 138 deletions(-)
  create mode 100644 drivers/raw/ioat/rte_ioat_rawdev_fns.h

<snip>
diff --git a/drivers/raw/ioat/rte_ioat_rawdev.h 
b/drivers/raw/ioat/rte_ioat_rawdev.h
index 4bc6491d91..7ace5c085a 100644
--- a/drivers/raw/ioat/rte_ioat_rawdev.h
+++ b/drivers/raw/ioat/rte_ioat_rawdev.h
@@ -14,12 +14,7 @@
   * @b EXPERIMENTAL: these structures and APIs may change without prior notice
   */
-#include <x86intrin.h>
-#include <rte_atomic.h>
-#include <rte_memory.h>
-#include <rte_memzone.h>
-#include <rte_prefetch.h>
-#include "rte_ioat_spec.h"
+#include <rte_common.h>
/** Name of the device driver */
  #define IOAT_PMD_RAWDEV_NAME rawdev_ioat
@@ -38,38 +33,6 @@ struct rte_ioat_rawdev_config {
        bool hdls_disable;    /**< if set, ignore user-supplied handle params */
  };
-/**
- * @internal
- * Structure representing a device instance
- */
-struct rte_ioat_rawdev {
-       struct rte_rawdev *rawdev;
-       const struct rte_memzone *mz;
-       const struct rte_memzone *desc_mz;
-
-       volatile struct rte_ioat_registers *regs;
-       phys_addr_t status_addr;
-       phys_addr_t ring_addr;
-
-       unsigned short ring_size;
-       struct rte_ioat_generic_hw_desc *desc_ring;
-       bool hdls_disable;
-       __m128i *hdls; /* completion handles for returning to user */
-
-
-       unsigned short next_read;
-       unsigned short next_write;
-
-       /* some statistics for tracking, if added/changed update xstats fns*/
-       uint64_t enqueue_failed __rte_cache_aligned;
-       uint64_t enqueued;
-       uint64_t started;
-       uint64_t completed;
-
-       /* to report completions, the device will write status back here */
-       volatile uint64_t status __rte_cache_aligned;
-};
-
  /**
   * Enqueue a copy operation onto the ioat device
   *
@@ -104,38 +67,7 @@ struct rte_ioat_rawdev {
  static inline int
  rte_ioat_enqueue_copy(int dev_id, phys_addr_t src, phys_addr_t dst,
                unsigned int length, uintptr_t src_hdl, uintptr_t dst_hdl,
-               int fence)
-{
-       struct rte_ioat_rawdev *ioat = rte_rawdevs[dev_id].dev_private;


This assignment needs to be type cast to "struct rte_ioat_rawdev *" for C++ compilation compatibility.

There are a number of occurrences of this in this patch.


-       unsigned short read = ioat->next_read;
-       unsigned short write = ioat->next_write;
-       unsigned short mask = ioat->ring_size - 1;
-       unsigned short space = mask + read - write;
-       struct rte_ioat_generic_hw_desc *desc;
-
-       if (space == 0) {
-               ioat->enqueue_failed++;
-               return 0;
-       }
-
-       ioat->next_write = write + 1;
-       write &= mask;
-
-       desc = &ioat->desc_ring[write];
-       desc->size = length;
-       /* set descriptor write-back every 16th descriptor */
-       desc->u.control_raw = (uint32_t)((!!fence << 4) | (!(write & 0xF)) << 
3);
-       desc->src_addr = src;
-       desc->dest_addr = dst;
-       if (!ioat->hdls_disable)
-               ioat->hdls[write] = _mm_set_epi64x((int64_t)dst_hdl,
-                                       (int64_t)src_hdl);
-
-       rte_prefetch0(&ioat->desc_ring[ioat->next_write & mask]);
-
-       ioat->enqueued++;
-       return 1;
-}
+               int fence);
<snip>
+/**
+ * Returns details of copy operations that have been completed
+ */
+static inline int
+rte_ioat_completed_copies(int dev_id, uint8_t max_copies,
+               uintptr_t *src_hdls, uintptr_t *dst_hdls)
+{
+       struct rte_ioat_rawdev *ioat = rte_rawdevs[dev_id].dev_private;
+       unsigned short mask = (ioat->ring_size - 1);
+       unsigned short read = ioat->next_read;
+       unsigned short end_read, count;
+       int error;
+       int i = 0;
+
+       end_read = (rte_ioat_get_last_completed(ioat, &error) + 1) & mask;
+       count = (end_read - (read & mask)) & mask;
+
+       if (error) {
+               rte_errno = EIO;
+               return -1;
+       }
+
+       if (ioat->hdls_disable) {
+               read += count;
+               goto end;
+       }
+
+       if (count > max_copies)
+               count = max_copies;
+
+       for (; i < count - 1; i += 2, read += 2) {
+               __m128i hdls0 = _mm_load_si128(&ioat->hdls[read & mask]);
+               __m128i hdls1 = _mm_load_si128(&ioat->hdls[(read + 1) & mask]);
+
+               _mm_storeu_si128((void *)&src_hdls[i],
+                               _mm_unpacklo_epi64(hdls0, hdls1));
+               _mm_storeu_si128((void *)&dst_hdls[i],
+                               _mm_unpackhi_epi64(hdls0, hdls1));

"src_hdls" and "dst_hdls" need to be type cast to "__m128i *" here for C++ compatibility.


+       }
+       for (; i < count; i++, read++) {
+               uintptr_t *hdls = (void *)&ioat->hdls[read & mask];

Type cast for "ioat->hdls" to "__m128i *" needed here for C++ compatibility.


+               src_hdls[i] = hdls[0];
+               dst_hdls[i] = hdls[1];
+       }
+
+end:
+       ioat->next_read = read;
+       ioat->completed += count;
+       return count;
+}
+
+#endif /* _RTE_IOAT_RAWDEV_FNS_H_ */

Thanks,
Kevin

Reply via email to