From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from dpdk.org (dpdk.org [92.243.14.124]) by inbox.dpdk.org (Postfix) with ESMTP id 47738A046B for ; Thu, 27 Jun 2019 14:30:04 +0200 (CEST) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id B0083559A; Thu, 27 Jun 2019 14:30:03 +0200 (CEST) Received: from mga01.intel.com (mga01.intel.com [192.55.52.88]) by dpdk.org (Postfix) with ESMTP id ACE20559A for ; Thu, 27 Jun 2019 14:30:01 +0200 (CEST) X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from orsmga004.jf.intel.com ([10.7.209.38]) by fmsmga101.fm.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 27 Jun 2019 05:30:00 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.63,423,1557212400"; d="scan'208";a="313778533" Received: from aburakov-mobl1.ger.corp.intel.com (HELO [10.252.3.102]) ([10.252.3.102]) by orsmga004.jf.intel.com with ESMTP; 27 Jun 2019 05:29:59 -0700 To: Bruce Richardson , dev@dpdk.org Cc: thomas@monjalon.net, jerinj@marvell.com References: <20190530212525.40370-1-bruce.richardson@intel.com> <20190627104055.8244-1-bruce.richardson@intel.com> <20190627104055.8244-7-bruce.richardson@intel.com> From: "Burakov, Anatoly" Message-ID: <4bd6561f-0f0b-2486-1fd0-b0b931d50fbf@intel.com> Date: Thu, 27 Jun 2019 13:29:58 +0100 User-Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:60.0) Gecko/20100101 Thunderbird/60.7.2 MIME-Version: 1.0 In-Reply-To: <20190627104055.8244-7-bruce.richardson@intel.com> Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-US Content-Transfer-Encoding: 7bit Subject: Re: [dpdk-dev] [PATCH v3 6/8] raw/ioat: add configure, start and stop functions X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" 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 > > --- > > 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. > + 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). -- Thanks, Anatoly