Hi Akhil,
> -----Original Message-----
> From: Akhil Goyal
> Sent: Tuesday, September 18, 2018 7:07 PM
> To: Gagandeep Singh <g.si...@nxp.com>; dev@dpdk.org
> Cc: Hemant Agrawal <hemant.agra...@nxp.com>
> Subject: Re: [dpdk-dev] [PATCH 03/10] crypto/caam_jr: add HW config for job
> rings
>
> Hi Gagan,
>
> On 9/13/2018 11:38 AM, Gagandeep Singh wrote:
> > From: Hemant Agrawal <hemant.agra...@nxp.com>
> >
> > Signed-off-by: Gagandeep Singh <g.si...@nxp.com>
> > Signed-off-by: Hemant Agrawal <hemant.agra...@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
ok
> >
> > +/* 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.
I will handled all of them in next version.
> > +{
> > + 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.
ok
> > +
> > + 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
Message will be updated in next version.
> >
> > + /* 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
Ok, will be removed.
> > +
> .. [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.
ok
> > + return caam_jr_mem_vtop(ptr);
> > +}
> > +
> >
