Re: [dpdk-dev] [PATCH 03/16] crypto/cpt/base: add hardware initialization API for CPT

[Jerin Jacob <jerin.jacob@caviumnetworks.com>]

-----Original Message-----
> Date: Fri,  8 Jun 2018 22:15:12 +0530
> From: Anoob Joseph <anoob.joseph@caviumnetworks.com>
> To: Akhil Goyal <akhil.goyal@nxp.com>, Pablo de Lara
>  <pablo.de.lara.guarch@intel.com>, Thomas Monjalon <thomas@monjalon.net>
> Cc: Nithin Dabilpuram <nithin.dabilpuram@cavium.com>, Ankur Dwivedi
>  <ankur.dwivedi@cavium.com>, Jerin Jacob <jerin.jacob@caviumnetworks.com>,
>  Murthy NSSR <Nidadavolu.Murthy@cavium.com>, Narayana Prasad
>  <narayanaprasad.athreya@caviumnetworks.com>, Ragothaman Jayaraman
>  <Ragothaman.Jayaraman@cavium.com>, Srisivasubramanian Srinivasan
>  <Srisivasubramanian.Srinivasan@cavium.com>, dev@dpdk.org
> Subject: [PATCH 03/16] crypto/cpt/base: add hardware initialization API for
>  CPT
> X-Mailer: git-send-email 2.7.4
> 
> From: Nithin Dabilpuram <nithin.dabilpuram@cavium.com>
> 
> Adds hardware device initialization specific api for Cavium CPT device.
> 
> Signed-off-by: Ankur Dwivedi <ankur.dwivedi@cavium.com>
> Signed-off-by: Murthy NSSR <Nidadavolu.Murthy@cavium.com>
> Signed-off-by: Nithin Dabilpuram <nithin.dabilpuram@cavium.com>
> Signed-off-by: Ragothaman Jayaraman <Ragothaman.Jayaraman@cavium.com>
> Signed-off-by: Srisivasubramanian Srinivasan <Srisivasubramanian.Srinivasan@cavium.com>
> ---
>  drivers/crypto/cpt/base/cpt8xxx_device.c | 200 ++++++++++++++++
>  drivers/crypto/cpt/base/cpt8xxx_device.h |  85 +++++++
>  drivers/crypto/cpt/base/cpt_debug.h      | 231 +++++++++++++++++++
>  drivers/crypto/cpt/base/cpt_device.c     | 383 +++++++++++++++++++++++++++++++
>  drivers/crypto/cpt/base/cpt_device.h     | 162 +++++++++++++
>  drivers/crypto/cpt/base/cpt_vf_mbox.c    | 176 ++++++++++++++
>  drivers/crypto/cpt/base/cpt_vf_mbox.h    |  60 +++++
>  7 files changed, 1297 insertions(+)
>  create mode 100644 drivers/crypto/cpt/base/cpt8xxx_device.c
>  create mode 100644 drivers/crypto/cpt/base/cpt8xxx_device.h
>  create mode 100644 drivers/crypto/cpt/base/cpt_debug.h
>  create mode 100644 drivers/crypto/cpt/base/cpt_device.c
>  create mode 100644 drivers/crypto/cpt/base/cpt_device.h
>  create mode 100644 drivers/crypto/cpt/base/cpt_vf_mbox.c
>  create mode 100644 drivers/crypto/cpt/base/cpt_vf_mbox.h
> 
> +#include <assert.h>
> +#include "cpt_request_mgr.h"
> +#include <rte_eal_memconfig.h>
> +
> +#ifdef CPT_DEBUG

Remove CPT_DEBUG. No harming in compiling these definitions either case.
It will avoid the chance of build breakage.

> +static inline void *
> +os_iova2va(phys_addr_t physaddr)
> +{
> +	return rte_mem_iova2virt(physaddr);
> +}
> +
> +static inline void __cpt_dump_buffer(const char *prefix_str,
> +				     void *buf, size_t len, int rowsize)
> +{
> +	size_t i = 0;
> +	unsigned char *ptr = (unsigned char *)buf;
> +
> +	PRINT("\n%s[%p]", prefix_str, buf);


Something cpt_log() makes more sense than PRINT

> +	PRINT("\n%.8lx: ", i);
> +
> +	if (buf == NULL) {
> +		PRINT("\n!!!NULL ptr\n");
> +		abort();
> +	}
> +
> +	for (i = 0; i < len; i++) {
> +		if (i && !(i % rowsize))
> +			PRINT("\n%.8lx: ", i);
> +		PRINT("%02x ", ptr[i]);
> +	}
> +	PRINT("\n\n");
> +}
> +
> +static inline void cpt_dump_buffer(const char *prefix_str,
> +				   void *buf, size_t len)
> +{
> +	__cpt_dump_buffer(prefix_str, buf, len, 8);
> +}
> +
> +#define cpt_fn_trace(fmt, ...)				\
> +	do {						\
> +		if (msg_req_trace(debug))		\
> +			cpt_info(fmt, ##__VA_ARGS__);	\
> +	} while (0)
> +
> +static inline void dump_cpt_request_info(struct cpt_request_info *req,
> +					 cpt_inst_s_t *inst)
> +{
> +	vq_cmd_word0_t vq_cmd_w0;
> +	vq_cmd_word3_t vq_cmd_w3;
> +	uint16_t opcode, param1, param2, dlen;
> +
> +	vq_cmd_w0.u64 = be64toh(inst->s.ei0);
> +	opcode = be16toh(vq_cmd_w0.s.opcode);
> +	param1 = be16toh(vq_cmd_w0.s.param1);
> +	param2 = be16toh(vq_cmd_w0.s.param2);
> +	dlen = be16toh(vq_cmd_w0.s.dlen);
> +	vq_cmd_w3.u64 = inst->s.ei3;
> +
> +	PRINT("\ncpt Request Info...\n");
> +	PRINT("\tdma_mode: %u\n", req->dma_mode);
> +	PRINT("\tis_se   : %u\n", req->se_req);
> +	PRINT("\tgrp     : 0\n");
> +
> +	PRINT("\nRequest Info...\n");
> +	PRINT("\topcode: 0x%0x\n", opcode);
> +	PRINT("\tparam1: 0x%0x\n", param1);
> +	PRINT("\tparam2: 0x%0x\n", param2);
> +	PRINT("\tdlen: %u\n", dlen);
> +	PRINT("\tctx_handle vaddr %p, dma 0x%lx\n",
> +		     os_iova2va((uint64_t)vq_cmd_w3.s.cptr),
> +		     (uint64_t)vq_cmd_w3.s.cptr);
> +}
> +
> +			list_ptr[i*4+0].dma_addr = be64toh(sg_ptr->ptr[0]);
> +			list_ptr[i*4+1].dma_addr = be64toh(sg_ptr->ptr[1]);
> +			list_ptr[i*4+2].dma_addr = be64toh(sg_ptr->ptr[2]);
> +			list_ptr[i*4+3].dma_addr = be64toh(sg_ptr->ptr[3]);

use dpdk primitives for be64toh if possible.

> +
> +			list_ptr[i*4+0].vaddr =
> +				os_iova2va(list_ptr[i*4+0].dma_addr);
> +			list_ptr[i*4+1].vaddr =
> +				os_iova2va(list_ptr[i*4+1].dma_addr);
> +			list_ptr[i*4+2].vaddr =
> +				os_iova2va(list_ptr[i*4+2].dma_addr);
> +			list_ptr[i*4+3].vaddr =
> +				os_iova2va(list_ptr[i*4+3].dma_addr);
> +			sg_ptr++;
> +		}
> +		components = list_cnt % 4;
> +
> +		switch (components) {
> +		case 3:
> +			list_ptr[i*4+2].size = be16toh(sg_ptr->u.s.len[2]);
> +			list_ptr[i*4+2].dma_addr = be64toh(sg_ptr->ptr[2]);
> +			list_ptr[i*4+2].vaddr =
> +				os_iova2va(list_ptr[i*4+2].dma_addr);
> +			/* fall through */
> +		case 2:
> +			list_ptr[i*4+1].size = be16toh(sg_ptr->u.s.len[1]);
> +			list_ptr[i*4+1].dma_addr = be64toh(sg_ptr->ptr[1]);
> +			list_ptr[i*4+1].vaddr =
> +				os_iova2va(list_ptr[i*4+1].dma_addr);
> +			/* fall through */
> +		case 1:
> +			list_ptr[i*4+0].size = be16toh(sg_ptr->u.s.len[0]);
> +			list_ptr[i*4+0].dma_addr = be64toh(sg_ptr->ptr[0]);
> +			list_ptr[i*4+0].vaddr =
> +				os_iova2va(list_ptr[i*4+0].dma_addr);
> +			break;
> +		default:
> +			break;
> +		}
> +
> +		for (i = 0; i < list_cnt; i++) {
> +			snprintf(suffix, sizeof(suffix),
> +				 "%s[%d]: vaddr %p, dma 0x%lx len %u: ",
> +				 list, i, list_ptr[i].vaddr,
> +				 list_ptr[i].dma_addr,
> +				 list_ptr[i].size);
> +			if (data)
> +				cpt_dump_buffer(suffix,
> +						list_ptr[i].vaddr,
> +						list_ptr[i].size);
> +			else
> +				PRINT("%s\n", suffix);
> +		}
> +	} else {
> +		PRINT("%s: Direct Mode\n", header);
> +
> +		if (glist) {
> +			snprintf(suffix, sizeof(suffix),
> +				 "DPTR: vaddr %p, dma 0x%lx len %u: ",
> +				 os_iova2va(inst->s.ei1),
> +				 inst->s.ei1, dlen);
> +			if (data)
> +				cpt_dump_buffer(suffix,
> +						os_iova2va(inst->s.ei1),
> +						dlen);
> +			else
> +				PRINT("%s\n", suffix);
> +		} else {
> +			snprintf(suffix, sizeof(suffix),
> +				 "RPTR: vaddr %p, dma 0x%lx len %u+..: ",
> +				 os_iova2va(inst->s.ei2),
> +				 inst->s.ei2, dlen);
> +			/*
> +			 * In direct mode, we don't have rlen
> +			 * to dump exactly, so dump dlen + 32
> +			 */
> +			if (data)
> +				cpt_dump_buffer(suffix,
> +						os_iova2va(inst->s.ei2),
> +						dlen + 32);
> +			else
> +				PRINT("%s\n", suffix);
> +		}
> +	}
> +}
> +
> +
> +#else
> +
> +
> +int cptvf_deinit_device(struct cpt_vf *dev)
> +{
> +	struct cpt_vf *cptvf = (struct cpt_vf *)dev;
> +
> +	/* Do misc work one last time */
> +	cptvf_poll_misc(cptvf);
> +
> +	/* TODO anything else ?? */

Remove unclear TODOs

> +
> +	return 0;
> +}
> +
> +int cptvf_init_device(struct cpt_vf *cptvf,
> +		      void *pdev,
> +		      void *reg_base,
> +		      char *name,
> +		      uint32_t flags)
> +{
> +	(void) flags;


RTE_SET_USED

> +
> +	memset(cptvf, 0, sizeof(struct cpt_vf));
> +
> +	/* Bar0 base address */
> +	cptvf->reg_base = reg_base;
> +	strncpy(cptvf->dev_name, name, 32);
> +
> +	cptvf->nr_queues = 1;
> +	cptvf->max_queues = 1;
> +	cptvf->pdev = pdev;
> +
> +	/* To clear if there are any pending mbox msgs */
> +	cptvf_poll_misc(cptvf);
> +
> +	if (cpt_vf_init(cptvf)) {
> +		PMD_DRV_LOG(ERR, "Failed to initialize CPT VF device\n");
> +		return -1;
> +	}
> +
> +	return 0;
> +}
> +
> +
> +	uint16_t flags;
> +	/**< Flags to hold device status bits */
> +	uint8_t  vfid;
> +	/**< Device Index (0...CPT_MAX_VQ_NUM */
> +	uint8_t  vftype;
> +	/**< VF type of cpt_vf_type_t (SE_TYPE(2) or AE_TYPE(1) */
> +	uint8_t  vfgrp;
> +	/**< VF group (0 - 8) */
> +	uint8_t  node;
> +	/**< Operating node: Bits (46:44) in BAR0 address */
> +
> +	/* VF-PF mailbox communication */
> +	bool pf_acked;
> +	bool pf_nacked;
> +	char dev_name[32];
> +} ____cacheline_aligned_in_smp;


Use DPDK primitive if possible.

> +
> +#define CPT_CSR_REG_BASE(cpt)       ((cpt)->reg_base)
> +