Re: [dpdk-dev] [PATCH 02/10] crypto/caam_jr: introduce basic driver

[Gagandeep Singh <G.Singh@nxp.com>]

Hi

> -----Original Message-----
> From: Akhil Goyal
> Sent: Tuesday, September 18, 2018 5:44 PM
> To: Gagandeep Singh <G.Singh@nxp.com>; dev@dpdk.org
> Cc: Hemant Agrawal <hemant.agrawal@nxp.com>
> Subject: Re: [dpdk-dev] [PATCH 02/10] crypto/caam_jr: introduce basic driver
> 
> Hi Gagan,
> 
> On 9/13/2018 11:38 AM, Gagandeep Singh wrote:
> 
> > From: Hemant Agrawal <hemant.agrawal@nxp.com>
> >
> > This patch introduces basic support for caam_jr crypto driver.
> >
> > Signed-off-by: Gagandeep Singh <g.singh@nxp.com>
> > Signed-off-by: Hemant Agrawal <hemant.agrawal@nxp.com>
> > ---
> >   config/common_base                            |   8 +
> >   config/common_linuxapp                        |   1 +
> >   config/defconfig_arm64-dpaa-linuxapp-gcc      |   4 +
> >   drivers/crypto/Makefile                       |   1 +
> >   drivers/crypto/caam_jr/Makefile               |  40 +++++
> >   drivers/crypto/caam_jr/caam_jr.c              | 157 ++++++++++++++++++
> >   drivers/crypto/caam_jr/caam_jr_log.h          |  42 +++++
> >   drivers/crypto/caam_jr/meson.build            |  11 ++
> >   .../caam_jr/rte_pmd_caam_jr_version.map       |   4 +
> >   drivers/crypto/meson.build                    |   2 +-
> >   10 files changed, 269 insertions(+), 1 deletion(-)
> >   create mode 100644 drivers/crypto/caam_jr/Makefile
> >   create mode 100644 drivers/crypto/caam_jr/caam_jr.c
> >   create mode 100644 drivers/crypto/caam_jr/caam_jr_log.h
> >   create mode 100644 drivers/crypto/caam_jr/meson.build
> >   create mode 100644
> > drivers/crypto/caam_jr/rte_pmd_caam_jr_version.map
> >
> > diff --git a/config/common_base b/config/common_base index
> > 4bcbaf923..a73f063d1 100644
> > --- a/config/common_base
> > +++ b/config/common_base
> > @@ -479,6 +479,14 @@ CONFIG_RTE_CRYPTO_MAX_DEVS=64
> >   CONFIG_RTE_LIBRTE_PMD_ARMV8_CRYPTO=n
> >   CONFIG_RTE_LIBRTE_PMD_ARMV8_CRYPTO_DEBUG=n
> >
> > +#
> > +# Compile NXP CAAM JR crypto Driver
> > +#
> > +CONFIG_RTE_LIBRTE_PMD_CAAM_JR=n
> > +CONFIG_RTE_LIBRTE_PMD_CAAM_JR_BE=n
> > +CONFIG_RTE_LIBRTE_PMD_CAAM_JR_DEBUG=n
> 
> Do you really need DEBUG?
We are using DEBUG for additional development debugging like descriptor dump, packets dump. But yes this DEBUG can be removed from here and can be used as local flag. In next version, I will remove this config flag from here. 
> 
> > +CONFIG_RTE_CAAM_JR_PMD_MAX_NB_SESSIONS=2048
> 
> MAX_NB_SESSIONS is no more used in any of the drivers.
> 
> Do you have a limitation in your hardware?
No need of this, will be removed in next set.
> 
> > +
> >   #
> >   # Compile NXP DPAA2 crypto sec driver for CAAM HW
> >   #
> > diff --git a/config/common_linuxapp b/config/common_linuxapp index
> > 9c5ea9d89..c1c7c4287 100644
> > --- a/config/common_linuxapp
> > +++ b/config/common_linuxapp
> > @@ -35,6 +35,7 @@ CONFIG_RTE_LIBRTE_DPAA_MEMPOOL=y
> >   CONFIG_RTE_LIBRTE_DPAA_PMD=y
> >   CONFIG_RTE_LIBRTE_PMD_DPAA_EVENTDEV=y
> >   CONFIG_RTE_LIBRTE_PMD_DPAA_SEC=y
> > +CONFIG_RTE_LIBRTE_PMD_CAAM_JR=y
> >
> >   # NXP FSLMC BUS and DPAA2 drivers
> >   CONFIG_RTE_LIBRTE_FSLMC_BUS=y
> > diff --git a/config/defconfig_arm64-dpaa-linuxapp-gcc
> > b/config/defconfig_arm64-dpaa-linuxapp-gcc
> > index c47aec0a6..e5343f7a9 100644
> > --- a/config/defconfig_arm64-dpaa-linuxapp-gcc
> > +++ b/config/defconfig_arm64-dpaa-linuxapp-gcc
> > @@ -21,3 +21,7 @@ CONFIG_RTE_PKTMBUF_HEADROOM=128
> >   # NXP DPAA Bus
> >   CONFIG_RTE_LIBRTE_DPAA_DEBUG_DRIVER=n
> >   CONFIG_RTE_LIBRTE_DPAA_HWDEBUG=n
> > +
> > +# NXP CAAM_JR driver
> > +CONFIG_RTE_LIBRTE_PMD_CAAM_JR=y
> > +CONFIG_RTE_LIBRTE_PMD_CAAM_JR_BE=y
> > diff --git a/drivers/crypto/Makefile b/drivers/crypto/Makefile index
> > c480cbd37..e3711d703 100644
> > --- a/drivers/crypto/Makefile
> > +++ b/drivers/crypto/Makefile
> > @@ -6,6 +6,7 @@ include $(RTE_SDK)/mk/rte.vars.mk
> >   DIRS-$(CONFIG_RTE_LIBRTE_PMD_AESNI_GCM) += aesni_gcm
> >   DIRS-$(CONFIG_RTE_LIBRTE_PMD_AESNI_MB) += aesni_mb
> >   DIRS-$(CONFIG_RTE_LIBRTE_PMD_ARMV8_CRYPTO) += armv8
> > +DIRS-$(CONFIG_RTE_LIBRTE_PMD_CAAM_JR) += caam_jr
> >   DIRS-$(CONFIG_RTE_LIBRTE_PMD_CCP) += ccp
> >   DIRS-$(CONFIG_RTE_LIBRTE_PMD_OPENSSL) += openssl
> >   DIRS-$(CONFIG_RTE_LIBRTE_PMD_CRYPTO_SCHEDULER) += scheduler diff
> > --git a/drivers/crypto/caam_jr/Makefile
> > b/drivers/crypto/caam_jr/Makefile new file mode 100644 index
> > 000000000..46d752af7
> > --- /dev/null
> > +++ b/drivers/crypto/caam_jr/Makefile
> > @@ -0,0 +1,40 @@
> > +# SPDX-License-Identifier: BSD-3-Clause # Copyright 2017 NXP
> > +
> > +include $(RTE_SDK)/mk/rte.vars.mk
> > +
> > +#
> > +# library name
> > +#
> > +LIB = librte_pmd_caam_jr.a
> > +
> > +# build flags
> > +CFLAGS += -DALLOW_EXPERIMENTAL_API
> > +CFLAGS += -D _GNU_SOURCE
> > +ifeq ($(CONFIG_RTE_LIBRTE_CAAM_JR_DEBUG),y)
> > +CFLAGS += -O0 -g
> > +CFLAGS += "-Wno-error"
> > +else
> > +CFLAGS += -O3
> > +CFLAGS += $(WERROR_FLAGS)
> > +endif
> > +
> > +CFLAGS += -I$(RTE_SDK)/drivers/crypto/caam_jr
> > +CFLAGS += -I$(RTE_SDK)/lib/librte_eal/common/include
> > +CFLAGS += -I$(RTE_SDK)/lib/librte_eal/linuxapp/eal
> > +
> > +# versioning export map
> > +EXPORT_MAP := rte_pmd_caam_jr_version.map
> > +
> > +# library version
> > +LIBABIVER := 1
> > +
> > +# library source files
> > +SRCS-$(CONFIG_RTE_LIBRTE_PMD_CAAM_JR) += caam_jr.c # library
> > +dependencies
> > +
> > +LDLIBS += -lrte_eal -lrte_mbuf -lrte_mempool -lrte_ring LDLIBS +=
> > +-lrte_cryptodev LDLIBS += -lrte_bus_vdev
> > +
> > +include $(RTE_SDK)/mk/rte.lib.mk
> > diff --git a/drivers/crypto/caam_jr/caam_jr.c
> > b/drivers/crypto/caam_jr/caam_jr.c
> > new file mode 100644
> > index 000000000..68779cba5
> > --- /dev/null
> > +++ b/drivers/crypto/caam_jr/caam_jr.c
> > @@ -0,0 +1,157 @@
> > +/* SPDX-License-Identifier: BSD-3-Clause
> > + * Copyright 2017-2018 NXP
> > + */
> > +
> > +#include <fcntl.h>
> > +#include <unistd.h>
> > +#include <sched.h>
> > +#include <net/if.h>
> > +
> > +#include <rte_byteorder.h>
> > +#include <rte_common.h>
> > +#include <rte_cryptodev_pmd.h>
> > +#include <rte_crypto.h>
> > +#include <rte_cryptodev.h>
> > +#include <rte_bus_vdev.h>
> > +#include <rte_malloc.h>
> > +#include <rte_security_driver.h>
> > +#include <rte_hexdump.h>
> > +
> > +#include <caam_jr_log.h>
> > +
> > +#define CRYPTODEV_NAME_CAAM_JR_PMD	crypto_caam_jr
> > +static uint8_t cryptodev_driver_id;
> > +int caam_jr_logtype;
> > +
> > +
> > +/*
> > + * @brief Release the resources used by the SEC user space driver.
> > + *
> > + * Reset and release SEC's job rings indicated by the User
> > +Application at
> > + * init_job_ring() and free any memory allocated internally.
> > + * Call once during application tear down.
> > + *
> > + * @note In case there are any descriptors in-flight (descriptors
> > +received by
> > + * SEC driver for processing and for which no response was yet
> > +provided to UA),
> > + * the descriptors are discarded without any notifications to User Application.
> > + *
> > + * @retval ::0			is returned for a successful execution
> > + * @retval ::-1		is returned if SEC driver release is in progress
> > + */
> > +
> > +static int
> > +caam_jr_dev_uninit(struct rte_cryptodev *dev) {
> > +
> > +	if (dev == NULL)
> > +		return -ENODEV;
> > +
> > +
> > +	CAAM_JR_INFO("Closing DPAA_SEC device %s", dev->data->name);
> > +
> > +
> > +	return 0;
> > +}
> 
> remove extra spacing.
ok
>
> > +
> > +static int
> > +caam_jr_dev_init(const char *name,
> > +			 struct rte_vdev_device *vdev,
> > +			 struct rte_cryptodev_pmd_init_params *init_params) {
> > +	struct rte_cryptodev *dev;
> > +
> > +	PMD_INIT_FUNC_TRACE();
> > +
> > +	dev = rte_cryptodev_pmd_create(name, &vdev->device, init_params);
> > +	if (dev == NULL) {
> > +		CAAM_JR_ERR("failed to create cryptodev vdev");
> > +		goto cleanup;
> > +	}
> > +
> > +	dev->driver_id = cryptodev_driver_id;
> > +	dev->dev_ops = NULL;
> > +
> > +	/* For secondary processes, we don't initialise any further as primary
> > +	 * has already done this work. Only check we don't need a different
> > +	 * RX function
> > +	 */
> > +	if (rte_eal_process_type() != RTE_PROC_PRIMARY) {
> > +		CAAM_JR_WARN("Device already init by primary process");
> > +		return 0;
> > +	}
> > +
> > +	RTE_LOG(INFO, PMD, "%s cryptodev init\n", dev->data->name);
> > +
> > +	return 0;
> > +
> > +cleanup:
> > +	CAAM_JR_ERR("driver %s: cryptodev_caam_jr_create failed",
> > +			init_params->name);
> > +
> > +	return -ENXIO;
> > +}
> > +
> > +/** Initialise CAAM JR crypto device */ static int
> > +cryptodev_caam_jr_probe(struct rte_vdev_device *vdev) {
> > +	struct rte_cryptodev_pmd_init_params init_params = {
> > +		"",
> > +		128,
> > +		rte_socket_id(),
> > +		RTE_CRYPTODEV_PMD_DEFAULT_MAX_NB_QUEUE_PAIRS
> > +	};
> > +	const char *name;
> > +	const char *input_args;
> > +
> > +	name = rte_vdev_device_name(vdev);
> > +	if (name == NULL)
> > +		return -EINVAL;
> > +
> > +	input_args = rte_vdev_device_args(vdev);
> > +	rte_cryptodev_pmd_parse_input_args(&init_params, input_args);
> > +
> > +	return caam_jr_dev_init(name, vdev, &init_params); }
> > +
> > +/** Uninitialise CAAM JR crypto device */ static int
> > +cryptodev_caam_jr_remove(struct rte_vdev_device *vdev) {
> > +	struct rte_cryptodev *cryptodev;
> > +	const char *name;
> > +
> > +	name = rte_vdev_device_name(vdev);
> > +	if (name == NULL)
> > +		return -EINVAL;
> > +
> > +	cryptodev = rte_cryptodev_pmd_get_named_dev(name);
> > +	if (cryptodev == NULL)
> > +		return -ENODEV;
> > +
> > +	caam_jr_dev_uninit(cryptodev);
> > +
> > +	return rte_cryptodev_pmd_destroy(cryptodev);
> > +}
> > +
> > +static struct rte_vdev_driver cryptodev_caam_jr_drv = {
> > +	.probe = cryptodev_caam_jr_probe,
> > +	.remove = cryptodev_caam_jr_remove
> > +};
> > +
> > +static struct cryptodev_driver caam_jr_crypto_drv;
> > +
> > +RTE_PMD_REGISTER_VDEV(CRYPTODEV_NAME_CAAM_JR_PMD,
> > +cryptodev_caam_jr_drv);
> RTE_PMD_REGISTER_PARAM_STRING(CRYPTODEV_NAME_CAAM_JR_PMD,
> > +	"max_nb_queue_pairs=<int>"
> > +	"max_nb_sessions=<int>"
> 
> max_nb_sessions may not be needed
ok
> 
> > +	"socket_id=<int>");
> > +RTE_PMD_REGISTER_CRYPTO_DRIVER(caam_jr_crypto_drv,
> cryptodev_caam_jr_drv.driver,
> > +		cryptodev_driver_id);
> > +
> > +RTE_INIT(caam_jr_init_log)
> > +{
> > +	caam_jr_logtype = rte_log_register("pmd.crypto.caam");
> > +	if (caam_jr_logtype >= 0)
> > +		rte_log_set_level(caam_jr_logtype, RTE_LOG_NOTICE); }