Re: [PATCH 03/10] crypto/caam_jr: add HW config for job rings

[Akhil Goyal <akhil.goyal@nxp.com>]

Hi Gagan,

On 9/13/2018 11:38 AM, Gagandeep Singh wrote:
> From: Hemant Agrawal <hemant.agrawal@nxp.com>
>
> Signed-off-by: Gagandeep Singh <g.singh@nxp.com>
> Signed-off-by: Hemant Agrawal <hemant.agrawal@nxp.com>
> ---
>   drivers/crypto/caam_jr/Makefile              |   6 +
>   drivers/crypto/caam_jr/caam_jr.c             | 329 +++++++++++-
>   drivers/crypto/caam_jr/caam_jr_config.h      | 207 ++++++++
>   drivers/crypto/caam_jr/caam_jr_hw.c          | 365 ++++++++++++++
>   drivers/crypto/caam_jr/caam_jr_hw_specific.h | 503 +++++++++++++++++++
>   drivers/crypto/caam_jr/caam_jr_pvt.h         | 285 +++++++++++
>   drivers/crypto/caam_jr/caam_jr_uio.c         | 491 ++++++++++++++++++
>   drivers/crypto/caam_jr/meson.build           |   5 +-
>   8 files changed, 2188 insertions(+), 3 deletions(-)
>   create mode 100644 drivers/crypto/caam_jr/caam_jr_config.h
>   create mode 100644 drivers/crypto/caam_jr/caam_jr_hw.c
>   create mode 100644 drivers/crypto/caam_jr/caam_jr_hw_specific.h
>   create mode 100644 drivers/crypto/caam_jr/caam_jr_pvt.h
>   create mode 100644 drivers/crypto/caam_jr/caam_jr_uio.c
>
> diff --git a/drivers/crypto/caam_jr/Makefile b/drivers/crypto/caam_jr/Makefile
> index 46d752af7..8b863b4af 100644
> --- a/drivers/crypto/caam_jr/Makefile
> +++ b/drivers/crypto/caam_jr/Makefile
> @@ -19,7 +19,10 @@ CFLAGS += -O3
>   CFLAGS += $(WERROR_FLAGS)
>   endif
>   
> +CFLAGS += -I$(RTE_SDK)/drivers/bus/dpaa/include
>   CFLAGS += -I$(RTE_SDK)/drivers/crypto/caam_jr
> +#sharing the hw flib headers from dpaa2_sec pmd
> +CFLAGS += -I$(RTE_SDK)/drivers/crypto/dpaa2_sec/
>   CFLAGS += -I$(RTE_SDK)/lib/librte_eal/common/include
>   CFLAGS += -I$(RTE_SDK)/lib/librte_eal/linuxapp/eal
>   
> @@ -30,11 +33,14 @@ EXPORT_MAP := rte_pmd_caam_jr_version.map
>   LIBABIVER := 1
>   
>   # library source files
> +SRCS-$(CONFIG_RTE_LIBRTE_PMD_CAAM_JR) += caam_jr_hw.c
> +SRCS-$(CONFIG_RTE_LIBRTE_PMD_CAAM_JR) += caam_jr_uio.c
>   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_dpaa
>   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
> index 68779cba5..9d5f5b79b 100644
> --- a/drivers/crypto/caam_jr/caam_jr.c
> +++ b/drivers/crypto/caam_jr/caam_jr.c
> @@ -16,13 +16,146 @@
>   #include <rte_malloc.h>
>   #include <rte_security_driver.h>
>   #include <rte_hexdump.h>
> +#include <caam_jr_config.h>
Please give one line break between the rte_ includes and driver specific 
includes
>   
> +/* RTA header files */
> +#include <hw/desc/common.h>
> +#include <of.h>
> +#include <caam_jr_hw_specific.h>
> +#include <caam_jr_pvt.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;
>   
> +enum rta_sec_era rta_sec_era;
> +
> +/* Lists the states possible for the SEC user space driver. */
> +enum sec_driver_state_e {
> +	SEC_DRIVER_STATE_IDLE,		/* Driver not initialized */
> +	SEC_DRIVER_STATE_STARTED,	/* Driver initialized and can be used*/
> +	SEC_DRIVER_STATE_RELEASE,	/* Driver release is in progress */
> +};
> +
> +/* Job rings used for communication with SEC HW */
> +static struct sec_job_ring_t g_job_rings[MAX_SEC_JOB_RINGS];
> +
> +/* The current state of SEC user space driver */
> +static enum sec_driver_state_e g_driver_state = SEC_DRIVER_STATE_IDLE;
> +
> +/* The number of job rings used by SEC user space driver */
> +static int g_job_rings_no;
> +static int g_job_rings_max;
> +
> +/* @brief Poll the HW for already processed jobs in the JR
> + * and silently discard the available jobs or notify them to UA
> + * with indicated error code.
> + *
> + * @param [in,out]  job_ring        The job ring to poll.
> + * @param [in]  do_notify           Can be #TRUE or #FALSE. Indicates if
> + *				    descriptors are to be discarded
> + *                                  or notified to UA with given error_code.
> + * @param [out] notified_descs    Number of notified descriptors. Can be NULL
> + *					if do_notify is #FALSE
> + */
> +static void hw_flush_job_ring(struct sec_job_ring_t *job_ring,
> +			      uint32_t do_notify,
> +			      uint32_t *notified_descs)
static void should be in a separate line.. Please check in rest of the 
code as well.
> +{
> +	int32_t jobs_no_to_discard = 0;
> +	int32_t discarded_descs_no = 0;
> +
> +	CAAM_JR_DEBUG("Jr[%p] pi[%d] ci[%d].Flushing jr notify desc=[%d]",
> +		job_ring, job_ring->pidx, job_ring->cidx, do_notify);
> +
> +	jobs_no_to_discard = hw_get_no_finished_jobs(job_ring);
> +
> +	/* Discard all jobs */
> +	CAAM_JR_DEBUG("Jr[%p] pi[%d] ci[%d].Discarding %d descs",
> +		  job_ring, job_ring->pidx, job_ring->cidx,
> +		  jobs_no_to_discard);
> +
> +	while (jobs_no_to_discard > discarded_descs_no) {
> +		/* Get completed descriptor */
> +#if 0
> +		/*TODO if we want to do something with desc*/
> +		/* Since the memory is contigous, then P2V translation is a
> +		 * mere addition to the base descriptor physical address
> +		 */
> +		current_desc = job_ring->output_ring[job_ring->cidx].desc;
> +#endif
Please remove dead code.
> +
> +		discarded_descs_no++;
> +		/* Now increment the consumer index for the current job ring,
> +		 * AFTER saving job in temporary location!
> +		 * Increment the consumer index for the current job ring
> +		 */
> +		job_ring->cidx = SEC_CIRCULAR_COUNTER(job_ring->cidx,
> +					 SEC_JOB_RING_SIZE);
> +
> +		hw_remove_entries(job_ring, 1);
> +	}
> +
> +	if (do_notify == true) {
> +		ASSERT(notified_descs != NULL);
> +		*notified_descs = discarded_descs_no;
> +	}
> +}
> +
> +
> +/* @brief Flush job rings of any processed descs.
> + * The processed descs are silently dropped,
> + * WITHOUT being notified to UA.
> + */
> +static void close_job_ring(struct sec_job_ring_t *job_ring)
> +{
> +	if (job_ring->irq_fd) {
> +		/* Producer index is frozen. If consumer index is not equal
> +		 * with producer index, then we have descs to flush.
> +		 */
> +		while (job_ring->pidx != job_ring->cidx)
> +			hw_flush_job_ring(job_ring, false, NULL);
> +
> +		/* free the uio job ring */
> +		free_job_ring(job_ring->irq_fd);
> +		job_ring->irq_fd = 0;
> +		caam_jr_dma_free(job_ring->input_ring);
> +		caam_jr_dma_free(job_ring->output_ring);
> +
> +		g_job_rings_no--;
> +
> +	}
> +}
> +
> +/** @brief Release the software and hardware resources tied to a job ring.
> + * @param [in] job_ring The job ring
> + *
> + * @retval  0 for success
> + * @retval  -1 for error
> + */
> +static int shutdown_job_ring(struct sec_job_ring_t *job_ring)
> +{
> +	int ret = 0;
> +
> +	ASSERT(job_ring != NULL);
> +	ret = hw_shutdown_job_ring(job_ring);
> +	SEC_ASSERT(ret == 0, ret,
> +		"Failed to shutdown hardware job ring %p",
> +		job_ring);
> +
> +	if (job_ring->coalescing_en)
> +		hw_job_ring_disable_coalescing(job_ring);
> +
> +	if (job_ring->jr_mode != SEC_NOTIFICATION_TYPE_POLL) {
> +		ret = caam_jr_disable_irqs(job_ring->irq_fd);
> +		SEC_ASSERT(ret == 0, ret,
> +		"Failed to disable irqs for job ring %p",
> +		job_ring);
> +	}
> +
> +	return ret;
> +}
>   
>   /*
>    * @brief Release the resources used by the SEC user space driver.
> @@ -42,31 +175,195 @@ int caam_jr_logtype;
>   static int
>   caam_jr_dev_uninit(struct rte_cryptodev *dev)
>   {
> +	struct sec_job_ring_t *internals;
>   
>   	if (dev == NULL)
>   		return -ENODEV;
>   
> +	internals = dev->data->dev_private;
> +	rte_free(dev->security_ctx);
> +
> +
> +	/* If any descriptors in flight , poll and wait
> +	 * until all descriptors are received and silently discarded.
> +	 */
> +	if (internals) {
> +		shutdown_job_ring(internals);
> +		close_job_ring(internals);
> +		rte_mempool_free(internals->ctx_pool);
> +	}
>   
>   	CAAM_JR_INFO("Closing DPAA_SEC device %s", dev->data->name);
DPAA_SEC??? check rest of the code as well
>   
> +	/* last caam jr instance) */
> +	if (g_job_rings_no == 0)
> +		g_driver_state = SEC_DRIVER_STATE_IDLE;
>   
> -	return 0;
> +	return SEC_SUCCESS;
>   }
>   
> +/* @brief Initialize the software and hardware resources tied to a job ring.
.. [snip]
> diff --git a/drivers/crypto/caam_jr/caam_jr_hw_specific.h b/drivers/crypto/caam_jr/caam_jr_hw_specific.h
> new file mode 100644
> index 000000000..7c8909d2b
> --- /dev/null
> +++ b/drivers/crypto/caam_jr/caam_jr_hw_specific.h
> @@ -0,0 +1,503 @@
> +/* SPDX-License-Identifier: BSD-3-Clause
> + * Copyright 2017 NXP
> + */
> +
> +#ifndef CAAM_JR_HW_SPECIFIC_H
> +#define CAAM_JR_HW_SPECIFIC_H
> +
> +#include <caam_jr_config.h>
> +
> +/*
> + * Offset to the registers of a job ring.
> + * Is different for each job ring.
> + */
> +#define CHAN_BASE(jr)   ((size_t)(jr)->register_base_addr)
> +
> +#ifndef unlikely
> +#define unlikely(x)     __builtin_expect(!!(x), 0)
> +#endif
likely/unlikely are already defined in DPDK
> +
.. [snip]
> +
> +static inline rte_iova_t
> +caam_jr_mem_vtop(void *vaddr)
> +{
> +	const struct rte_memseg *ms;
> +
> +	ms = rte_mem_virt2memseg(vaddr, NULL);
> +	if (ms)
> +		return ms->iova + RTE_PTR_DIFF(vaddr, ms->addr);
> +	return (size_t)NULL;
> +}
> +
> +static inline void *
> +caam_jr_dma_ptov(rte_iova_t paddr)
> +{
> +	return rte_mem_iova2virt(paddr);
> +}
> +
> +/* Virtual to physical address conversion */
> +static inline rte_iova_t caam_jr_dma_vtop(void *ptr)
> +{
> +	//return rte_malloc_virt2iova(ptr);
remove this comment.
> +	return caam_jr_mem_vtop(ptr);
> +}
> +
>