[v3,6/8] raw/ioat: add configure, start and stop functions
Checks
Commit Message
Allow initializing a driver instance. Include selftest to validate these
functions.
Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
---
V3: don't add a new descriptor format struct, reuse from rte_ioat_spec.h
V2: test cases placed in self-test routine
---
app/test/test_rawdev.c | 2 +-
doc/guides/rawdevs/ioat_rawdev.rst | 32 ++++++++++++
drivers/raw/ioat/Makefile | 1 +
drivers/raw/ioat/ioat_rawdev.c | 78 +++++++++++++++++++++++++++++
drivers/raw/ioat/ioat_rawdev_test.c | 41 +++++++++++++++
drivers/raw/ioat/meson.build | 3 +-
drivers/raw/ioat/rte_ioat_rawdev.h | 4 +-
7 files changed, 158 insertions(+), 3 deletions(-)
create mode 100644 drivers/raw/ioat/ioat_rawdev_test.c
Comments
On 27-Jun-19 11:40 AM, Bruce Richardson wrote:
> Allow initializing a driver instance. Include selftest to validate these
> functions.
>
> Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
>
> ---
>
> V3: don't add a new descriptor format struct, reuse from rte_ioat_spec.h
> V2: test cases placed in self-test routine
> ---
> app/test/test_rawdev.c | 2 +-
> doc/guides/rawdevs/ioat_rawdev.rst | 32 ++++++++++++
> drivers/raw/ioat/Makefile | 1 +
> drivers/raw/ioat/ioat_rawdev.c | 78 +++++++++++++++++++++++++++++
> drivers/raw/ioat/ioat_rawdev_test.c | 41 +++++++++++++++
> drivers/raw/ioat/meson.build | 3 +-
> drivers/raw/ioat/rte_ioat_rawdev.h | 4 +-
> 7 files changed, 158 insertions(+), 3 deletions(-)
> create mode 100644 drivers/raw/ioat/ioat_rawdev_test.c
>
> diff --git a/app/test/test_rawdev.c b/app/test/test_rawdev.c
> index 4db762b4c..731e51717 100644
> --- a/app/test/test_rawdev.c
> +++ b/app/test/test_rawdev.c
> @@ -36,7 +36,7 @@ test_rawdev_selftest_ioat(void)
> struct rte_rawdev_info info = { .dev_private = NULL };
> if (rte_rawdev_info_get(i, &info) == 0 &&
> strstr(info.driver_name, "ioat") != NULL)
> - return 0;
> + return rte_rawdev_selftest(i);
Even though it doesn't matter in practice, technically, we can't pass a
raw return value to the test caller. It should be TEST_SUCCESS or
TEST_FAILURE.
> }
>
> printf("No IOAT rawdev found, skipping tests\n");
> diff --git a/doc/guides/rawdevs/ioat_rawdev.rst b/doc/guides/rawdevs/ioat_rawdev.rst
> index 0ce984490..9ab97e2aa 100644
> --- a/doc/guides/rawdevs/ioat_rawdev.rst
> +++ b/doc/guides/rawdevs/ioat_rawdev.rst
> @@ -117,3 +117,35 @@ the ``dev_private`` field in the ``rte_rawdev_info`` struct should either
> be NULL, or else be set to point to a structure of type
> ``rte_ioat_rawdev_config``, in which case the size of the configured device
> input ring will be returned in that structure.
> +
> +Device Configuration
> +~~~~~~~~~~~~~~~~~~~~~
> +
> +Configuring an IOAT rawdev device is done using the
> +``rte_rawdev_configure()`` API, which takes the same structure parameters
> +as the, previously referenced, ``rte_rawdev_info_get()`` API. The main
> +difference is that, because the parameter is used as input rather than
> +output, the ``dev_private`` structure element cannot be NULL, and must
> +point to a valid ``rte_ioat_rawdev_config`` structure, containing the ring
> +size to be used by the device. The ring size must be a power of two,
> +between 64 and 4096.
<snip>
> + if (params->ring_size > 4096 || params->ring_size < 64 ||
> + !rte_is_power_of_2(params->ring_size))
> + return -EINVAL;
> +
> + ioat->ring_size = params->ring_size;
> + if (ioat->desc_ring != NULL) {
> + rte_free(ioat->desc_ring);
> + ioat->desc_ring = NULL;
> + }
> +
> + /* allocate one block of memory for both descriptors
> + * and completion handles.
> + */
> + ioat->desc_ring = rte_zmalloc_socket(NULL,
> + (DESC_SZ + COMPLETION_SZ) * ioat->ring_size,
> + 0, /* alignment, default to 64Byte */
> + dev->device->numa_node);
Using rte_zmalloc for hardware structures seems suspect. Do you not need
IOVA-contiguous memory here?
> + if (ioat->desc_ring == NULL)
> + return -ENOMEM;
> + ioat->hdls = (void *)&ioat->desc_ring[ioat->ring_size];
> +
> + ioat->ring_addr = rte_malloc_virt2iova(ioat->desc_ring);
> +
> + /* configure descriptor ring - each one points to next */
> + for (i = 0; i < ioat->ring_size; i++) {
> + ioat->desc_ring[i].next = ioat->ring_addr +
> + (((i + 1) % ioat->ring_size) * DESC_SZ);
> + }
OK, this *definitely* looks suspect :) with rte_zmalloc(), there's no
guarantee that the entire allocated block resides on the same physical
page, so you can't assume IOVA addresses will be contiguous either,
unless you only intend to operate in IOVA as VA mode (which i didn't
notice).
> -----Original Message-----
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Bruce Richardson
> Sent: Thursday, June 27, 2019 11:41 AM
> To: dev@dpdk.org
> Cc: thomas@monjalon.net; jerinj@marvell.com; Richardson, Bruce
> <bruce.richardson@intel.com>
> Subject: [dpdk-dev] [PATCH v3 6/8] raw/ioat: add configure, start and stop
> functions
<snip>
> +int ioat_rawdev_test(uint16_t dev_id);
> +
> static int
> ioat_rawdev_create(const char *name, struct rte_pci_device *dev) {
> static const struct rte_rawdev_ops ioat_rawdev_ops = {
> + .dev_configure = ioat_dev_configure,
> + .dev_start = ioat_dev_start,
> + .dev_stop = ioat_dev_stop,
> .dev_info_get = ioat_dev_info_get,
> + .dev_selftest = ioat_rawdev_test,
Build fail for ./devtools/test-build.sh x86_64-native-linux-gcc+next+shared
/drivers/raw/ioat/ioat_rawdev.c: In function 'ioat_rawdev_create':
drivers/raw/ioat/ioat_rawdev.c:163:20: error: initialization of 'int (*)(void)' from incompatible pointer type 'int (*)(uint16_t)' {aka 'int (*)(short unsigned int)'} [-Werror=incompatible-pointer-types]
163 | .dev_selftest = ioat_rawdev_test,
Thanks,
Reshma
On Thu, Jun 27, 2019 at 05:37:33PM +0100, Pattan, Reshma wrote:
>
>
> > -----Original Message-----
> > From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Bruce Richardson
> > Sent: Thursday, June 27, 2019 11:41 AM
> > To: dev@dpdk.org
> > Cc: thomas@monjalon.net; jerinj@marvell.com; Richardson, Bruce
> > <bruce.richardson@intel.com>
> > Subject: [dpdk-dev] [PATCH v3 6/8] raw/ioat: add configure, start and stop
> > functions
>
> <snip>
>
> > +int ioat_rawdev_test(uint16_t dev_id);
> > +
> > static int
> > ioat_rawdev_create(const char *name, struct rte_pci_device *dev) {
> > static const struct rte_rawdev_ops ioat_rawdev_ops = {
> > + .dev_configure = ioat_dev_configure,
> > + .dev_start = ioat_dev_start,
> > + .dev_stop = ioat_dev_stop,
> > .dev_info_get = ioat_dev_info_get,
> > + .dev_selftest = ioat_rawdev_test,
>
> Build fail for ./devtools/test-build.sh x86_64-native-linux-gcc+next+shared
> /drivers/raw/ioat/ioat_rawdev.c: In function 'ioat_rawdev_create':
> drivers/raw/ioat/ioat_rawdev.c:163:20: error: initialization of 'int (*)(void)' from incompatible pointer type 'int (*)(uint16_t)' {aka 'int (*)(short unsigned int)'} [-Werror=incompatible-pointer-types]
> 163 | .dev_selftest = ioat_rawdev_test,
>
Apologies, I forgot to put in the cover letter that this depends upon the
rawdev patchset I previously upstreamed:
http://patches.dpdk.org/project/dpdk/list/?series=5120
@@ -36,7 +36,7 @@ test_rawdev_selftest_ioat(void)
struct rte_rawdev_info info = { .dev_private = NULL };
if (rte_rawdev_info_get(i, &info) == 0 &&
strstr(info.driver_name, "ioat") != NULL)
- return 0;
+ return rte_rawdev_selftest(i);
}
printf("No IOAT rawdev found, skipping tests\n");
@@ -117,3 +117,35 @@ the ``dev_private`` field in the ``rte_rawdev_info`` struct should either
be NULL, or else be set to point to a structure of type
``rte_ioat_rawdev_config``, in which case the size of the configured device
input ring will be returned in that structure.
+
+Device Configuration
+~~~~~~~~~~~~~~~~~~~~~
+
+Configuring an IOAT rawdev device is done using the
+``rte_rawdev_configure()`` API, which takes the same structure parameters
+as the, previously referenced, ``rte_rawdev_info_get()`` API. The main
+difference is that, because the parameter is used as input rather than
+output, the ``dev_private`` structure element cannot be NULL, and must
+point to a valid ``rte_ioat_rawdev_config`` structure, containing the ring
+size to be used by the device. The ring size must be a power of two,
+between 64 and 4096.
+
+The following code shows how the device is configured in
+``test_ioat_rawdev.c``:
+
+.. code-block:: C
+
+ #define IOAT_TEST_RINGSIZE 512
+ struct rte_ioat_rawdev_config p = { .ring_size = -1 };
+ struct rte_rawdev_info info = { .dev_private = &p };
+
+ /* ... */
+
+ p.ring_size = IOAT_TEST_RINGSIZE;
+ if (rte_rawdev_configure(dev_id, &info) != 0) {
+ printf("Error with rte_rawdev_configure()\n");
+ return -1;
+ }
+
+Once configured, the device can then be made ready for use by calling the
+``rte_rawdev_start()`` API.
@@ -21,6 +21,7 @@ EXPORT_MAP := rte_pmd_ioat_version.map
# library source files
SRCS-$(CONFIG_RTE_LIBRTE_PMD_IOAT_RAWDEV) += ioat_rawdev.c
+SRCS-$(CONFIG_RTE_LIBRTE_PMD_IOAT_RAWDEV) += ioat_rawdev_test.c
# export include files
SYMLINK-y-include += rte_ioat_rawdev.h
@@ -24,6 +24,78 @@ static struct rte_pci_driver ioat_pmd_drv;
#define IOAT_PMD_ERR(fmt, args...) IOAT_PMD_LOG(ERR, fmt, ## args)
#define IOAT_PMD_WARN(fmt, args...) IOAT_PMD_LOG(WARNING, fmt, ## args)
+#define DESC_SZ sizeof(struct rte_ioat_generic_hw_desc)
+#define COMPLETION_SZ sizeof(__m128i)
+
+static int
+ioat_dev_configure(const struct rte_rawdev *dev, rte_rawdev_obj_t config)
+{
+ struct rte_ioat_rawdev_config *params = config;
+ struct rte_ioat_rawdev *ioat = dev->dev_private;
+ unsigned short i;
+
+ if (dev->started)
+ return -EBUSY;
+
+ if (params == NULL)
+ return -EINVAL;
+
+ if (params->ring_size > 4096 || params->ring_size < 64 ||
+ !rte_is_power_of_2(params->ring_size))
+ return -EINVAL;
+
+ ioat->ring_size = params->ring_size;
+ if (ioat->desc_ring != NULL) {
+ rte_free(ioat->desc_ring);
+ ioat->desc_ring = NULL;
+ }
+
+ /* allocate one block of memory for both descriptors
+ * and completion handles.
+ */
+ ioat->desc_ring = rte_zmalloc_socket(NULL,
+ (DESC_SZ + COMPLETION_SZ) * ioat->ring_size,
+ 0, /* alignment, default to 64Byte */
+ dev->device->numa_node);
+ if (ioat->desc_ring == NULL)
+ return -ENOMEM;
+ ioat->hdls = (void *)&ioat->desc_ring[ioat->ring_size];
+
+ ioat->ring_addr = rte_malloc_virt2iova(ioat->desc_ring);
+
+ /* configure descriptor ring - each one points to next */
+ for (i = 0; i < ioat->ring_size; i++) {
+ ioat->desc_ring[i].next = ioat->ring_addr +
+ (((i + 1) % ioat->ring_size) * DESC_SZ);
+ }
+
+ return 0;
+}
+
+static int
+ioat_dev_start(struct rte_rawdev *dev)
+{
+ struct rte_ioat_rawdev *ioat = dev->dev_private;
+
+ if (ioat->ring_size == 0 || ioat->desc_ring == NULL)
+ return -EBUSY;
+
+ /* inform hardware of where the descriptor ring is */
+ ioat->regs->chainaddr = ioat->ring_addr;
+ /* inform hardware of where to write the status/completions */
+ ioat->regs->chancmp = ioat->status_addr;
+
+ /* prime the status register to be set to the last element */
+ ioat->status = ioat->ring_addr + ((ioat->ring_size - 1) * DESC_SZ);
+ return 0;
+}
+
+static void
+ioat_dev_stop(struct rte_rawdev *dev)
+{
+ RTE_SET_USED(dev);
+}
+
static void
ioat_dev_info_get(struct rte_rawdev *dev, rte_rawdev_obj_t dev_info)
{
@@ -34,11 +106,17 @@ ioat_dev_info_get(struct rte_rawdev *dev, rte_rawdev_obj_t dev_info)
cfg->ring_size = ioat->ring_size;
}
+int ioat_rawdev_test(uint16_t dev_id);
+
static int
ioat_rawdev_create(const char *name, struct rte_pci_device *dev)
{
static const struct rte_rawdev_ops ioat_rawdev_ops = {
+ .dev_configure = ioat_dev_configure,
+ .dev_start = ioat_dev_start,
+ .dev_stop = ioat_dev_stop,
.dev_info_get = ioat_dev_info_get,
+ .dev_selftest = ioat_rawdev_test,
};
struct rte_rawdev *rawdev = NULL;
new file mode 100644
@@ -0,0 +1,41 @@
+/* SPDX-License-Identifier: BSD-3-Clause
+ * Copyright(c) 2019 Intel Corporation
+ */
+
+#include "rte_rawdev.h"
+#include "rte_ioat_rawdev.h"
+
+int ioat_rawdev_test(uint16_t dev_id); /* pre-define to keep compiler happy */
+
+int
+ioat_rawdev_test(uint16_t dev_id)
+{
+#define IOAT_TEST_RINGSIZE 512
+ struct rte_ioat_rawdev_config p = { .ring_size = -1 };
+ struct rte_rawdev_info info = { .dev_private = &p };
+
+ rte_rawdev_info_get(dev_id, &info);
+ if (p.ring_size != 0) {
+ printf("Error, initial ring size is non-zero (%d)\n",
+ (int)p.ring_size);
+ return -1;
+ }
+
+ p.ring_size = IOAT_TEST_RINGSIZE;
+ if (rte_rawdev_configure(dev_id, &info) != 0) {
+ printf("Error with rte_rawdev_configure()\n");
+ return -1;
+ }
+ rte_rawdev_info_get(dev_id, &info);
+ if (p.ring_size != IOAT_TEST_RINGSIZE) {
+ printf("Error, ring size is not %d (%d)\n",
+ IOAT_TEST_RINGSIZE, (int)p.ring_size);
+ return -1;
+ }
+
+ if (rte_rawdev_start(dev_id) != 0) {
+ printf("Error with rte_rawdev_start()\n");
+ return -1;
+ }
+ return 0;
+}
@@ -2,7 +2,8 @@
# Copyright 2019 Intel Corporation
build = dpdk_conf.has('RTE_ARCH_X86')
-sources = files('ioat_rawdev.c')
+sources = files('ioat_rawdev.c',
+ 'ioat_rawdev_test.c')
deps += ['rawdev', 'bus_pci']
install_headers('rte_ioat_rawdev.h',
@@ -14,6 +14,7 @@
* @b EXPERIMENTAL: these structures and APIs may change without prior notice
*/
+#include <x86intrin.h>
#include <rte_memory.h>
#include <rte_ioat_spec.h>
@@ -46,7 +47,8 @@ struct rte_ioat_rawdev {
phys_addr_t ring_addr;
unsigned short ring_size;
- struct rte_ioat_desc *desc_ring;
+ struct rte_ioat_generic_hw_desc *desc_ring;
+ __m128i *hdls; /* completion handles for returning to user */
/* to report completions, the device will write status back here */
volatile uint64_t status __rte_cache_aligned;