Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]


Groups > linux.kernel > #1540369 > unrolled thread

[RFC PATCH 0/3] Cavium ThunderX ZIP driver

Started byJan Glauber <jglauber@cavium.com>
First post2016-12-12 16:10 +0100
Last post2016-12-28 10:10 +0100
Articles 7 — 5 participants

Back to article view | Back to linux.kernel


Contents

  [RFC PATCH 0/3] Cavium ThunderX ZIP driver Jan Glauber <jglauber@cavium.com> - 2016-12-12 16:10 +0100
    [RFC PATCH 2/3] crypto: zip - Wire-up Compression / decompression HW offload Jan Glauber <jglauber@cavium.com> - 2016-12-12 16:10 +0100
    Re: [RFC PATCH 1/3] crypto: zip - Add ThunderX ZIP driver core Corentin Labbe <clabbe.montjoie@gmail.com> - 2016-12-13 14:40 +0100
    Re: [RFC PATCH 1/3] crypto: zip - Add ThunderX ZIP driver core Sasha Levin <levinsasha928@gmail.com> - 2016-12-19 17:20 +0100
    Re: [RFC PATCH 0/3] Cavium ThunderX ZIP driver Herbert Xu <herbert@gondor.apana.org.au> - 2016-12-27 10:40 +0100
      Re: [RFC PATCH 0/3] Cavium ThunderX ZIP driver "Challa, Mahipal" <Mahipal.Challa@cavium.com> - 2016-12-27 14:20 +0100
        Re: [RFC PATCH 0/3] Cavium ThunderX ZIP driver Herbert Xu <herbert@gondor.apana.org.au> - 2016-12-28 10:10 +0100

#1540369 — [RFC PATCH 0/3] Cavium ThunderX ZIP driver

FromJan Glauber <jglauber@cavium.com>
Date2016-12-12 16:10 +0100
Subject[RFC PATCH 0/3] Cavium ThunderX ZIP driver
Message-ID<sNAoN-5UC-11@gated-at.bofh.it>
Hi Herbert,

this series adds support for hardware accelerated compression & decompression
as found on ThunderX (arm64) SOCs. I've been reviewing this driver internally
for some time and would like to get feedback on the RFC to see if this goes
into the right direction and to see if there are any concerns.

We've discussed switching to the new acomp algorithm but for the time being
decided against acomp because our test cases are not yet supported with it.

To test the ZIP driver we've used ZSWAP and IPComp.

Performance numbers from ZSWAP look promising.
The "average time" for compressing a 4KB page:

Compression Software	:  128 usec
Compression HW deflate	:   16 usec
Compression HW LZS	:   10 usec

Decompression Software	: 20 usec
Decompression HW deflate: 7 usec
Decompression HW LZS	: 5 usec

Patches are on top of 4.9.

Feedback welcome!
Jan 

---------------------

Mahipal Challa (3):
  crypto: zip - Add ThunderX ZIP driver core
  crypto: zip - Wire-up Compression / decompression HW offload
  crypto: zip - Add Compression/decompression statistics

 drivers/crypto/Kconfig                  |    7 +
 drivers/crypto/Makefile                 |    1 +
 drivers/crypto/cavium/Makefile          |    4 +
 drivers/crypto/cavium/zip/Makefile      |   11 +
 drivers/crypto/cavium/zip/common.h      |  258 ++++++
 drivers/crypto/cavium/zip/zip_crypto.c  |  243 ++++++
 drivers/crypto/cavium/zip/zip_crypto.h  |   67 ++
 drivers/crypto/cavium/zip/zip_deflate.c |  200 +++++
 drivers/crypto/cavium/zip/zip_deflate.h |   62 ++
 drivers/crypto/cavium/zip/zip_device.c  |  209 +++++
 drivers/crypto/cavium/zip/zip_device.h  |  138 ++++
 drivers/crypto/cavium/zip/zip_inflate.c |  223 ++++++
 drivers/crypto/cavium/zip/zip_inflate.h |   62 ++
 drivers/crypto/cavium/zip/zip_main.c    |  698 ++++++++++++++++
 drivers/crypto/cavium/zip/zip_main.h    |  141 ++++
 drivers/crypto/cavium/zip/zip_mem.c     |  120 +++
 drivers/crypto/cavium/zip/zip_mem.h     |   78 ++
 drivers/crypto/cavium/zip/zip_regs.h    | 1326 +++++++++++++++++++++++++++++++
 18 files changed, 3848 insertions(+)
 create mode 100644 drivers/crypto/cavium/Makefile
 create mode 100644 drivers/crypto/cavium/zip/Makefile
 create mode 100644 drivers/crypto/cavium/zip/common.h
 create mode 100644 drivers/crypto/cavium/zip/zip_crypto.c
 create mode 100644 drivers/crypto/cavium/zip/zip_crypto.h
 create mode 100644 drivers/crypto/cavium/zip/zip_deflate.c
 create mode 100644 drivers/crypto/cavium/zip/zip_deflate.h
 create mode 100644 drivers/crypto/cavium/zip/zip_device.c
 create mode 100644 drivers/crypto/cavium/zip/zip_device.h
 create mode 100644 drivers/crypto/cavium/zip/zip_inflate.c
 create mode 100644 drivers/crypto/cavium/zip/zip_inflate.h
 create mode 100644 drivers/crypto/cavium/zip/zip_main.c
 create mode 100644 drivers/crypto/cavium/zip/zip_main.h
 create mode 100644 drivers/crypto/cavium/zip/zip_mem.c
 create mode 100644 drivers/crypto/cavium/zip/zip_mem.h
 create mode 100644 drivers/crypto/cavium/zip/zip_regs.h

-- 
2.9.0.rc0.21.g7777322

[toc] | [next] | [standalone]


#1540372 — [RFC PATCH 2/3] crypto: zip - Wire-up Compression / decompression HW offload

FromJan Glauber <jglauber@cavium.com>
Date2016-12-12 16:10 +0100
Subject[RFC PATCH 2/3] crypto: zip - Wire-up Compression / decompression HW offload
Message-ID<sNAoN-5UC-19@gated-at.bofh.it>
In reply to#1540369
From: Mahipal Challa <Mahipal.Challa@cavium.com>

This contains changes for adding compression/decompression h/w offload
functionality for both DEFLATE and LZS.

Signed-off-by: Mahipal Challa <Mahipal.Challa@cavium.com>
Signed-off-by: Vishnu Nair <Vishnu.Nair@cavium.com>
Signed-off-by: Jan Glauber <jglauber@cavium.com>
---
 drivers/crypto/cavium/zip/Makefile      |   5 +-
 drivers/crypto/cavium/zip/zip_crypto.c  | 243 ++++++++++++++++++++++++++++++++
 drivers/crypto/cavium/zip/zip_crypto.h  |   6 +
 drivers/crypto/cavium/zip/zip_deflate.c | 190 +++++++++++++++++++++++++
 drivers/crypto/cavium/zip/zip_deflate.h |  62 ++++++++
 drivers/crypto/cavium/zip/zip_device.c  |   1 +
 drivers/crypto/cavium/zip/zip_inflate.c | 211 +++++++++++++++++++++++++++
 drivers/crypto/cavium/zip/zip_inflate.h |  62 ++++++++
 drivers/crypto/cavium/zip/zip_main.c    |  29 ----
 9 files changed, 779 insertions(+), 30 deletions(-)
 create mode 100644 drivers/crypto/cavium/zip/zip_crypto.c
 create mode 100644 drivers/crypto/cavium/zip/zip_deflate.c
 create mode 100644 drivers/crypto/cavium/zip/zip_deflate.h
 create mode 100644 drivers/crypto/cavium/zip/zip_inflate.c
 create mode 100644 drivers/crypto/cavium/zip/zip_inflate.h

diff --git a/drivers/crypto/cavium/zip/Makefile b/drivers/crypto/cavium/zip/Makefile
index 2c07508..b2f3baaf 100644
--- a/drivers/crypto/cavium/zip/Makefile
+++ b/drivers/crypto/cavium/zip/Makefile
@@ -5,4 +5,7 @@
 obj-$(CONFIG_CRYPTO_DEV_CAVIUM_ZIP) += thunderx_zip.o
 thunderx_zip-y := zip_main.o    \
                   zip_device.o  \
-                  zip_mem.o
+                  zip_crypto.o  \
+                  zip_mem.o     \
+                  zip_deflate.o \
+                  zip_inflate.o
diff --git a/drivers/crypto/cavium/zip/zip_crypto.c b/drivers/crypto/cavium/zip/zip_crypto.c
new file mode 100644
index 0000000..888e18b
--- /dev/null
+++ b/drivers/crypto/cavium/zip/zip_crypto.c
@@ -0,0 +1,243 @@
+/***********************license start************************************
+ * Copyright (c) 2003-2016 Cavium, Inc.
+ * All rights reserved.
+ *
+ * License: one of 'Cavium License' or 'GNU General Public License Version 2'
+ *
+ * This file is provided under the terms of the Cavium License (see below)
+ * or under the terms of GNU General Public License, Version 2, as
+ * published by the Free Software Foundation. When using or redistributing
+ * this file, you may do so under either license.
+ *
+ * Cavium License:  Redistribution and use in source and binary forms, with
+ * or without modification, are permitted provided that the following
+ * conditions are met:
+ *
+ *  * Redistributions of source code must retain the above copyright
+ *    notice, this list of conditions and the following disclaimer.
+ *
+ *  * Redistributions in binary form must reproduce the above
+ *    copyright notice, this list of conditions and the following
+ *    disclaimer in the documentation and/or other materials provided
+ *    with the distribution.
+ *
+ *  * Neither the name of Cavium Inc. nor the names of its contributors may be
+ *    used to endorse or promote products derived from this software without
+ *    specific prior written permission.
+ *
+ * This Software, including technical data, may be subject to U.S. export
+ * control laws, including the U.S. Export Administration Act and its
+ * associated regulations, and may be subject to export or import
+ * regulations in other countries.
+ *
+ * TO THE MAXIMUM EXTENT PERMITTED BY LAW, THE SOFTWARE IS PROVIDED "AS IS"
+ * AND WITH ALL FAULTS AND CAVIUM INC. MAKES NO PROMISES, REPRESENTATIONS
+ * OR WARRANTIES, EITHER EXPRESS, IMPLIED, STATUTORY, OR OTHERWISE, WITH
+ * RESPECT TO THE SOFTWARE, INCLUDING ITS CONDITION, ITS CONFORMITY TO ANY
+ * REPRESENTATION OR DESCRIPTION, OR THE EXISTENCE OF ANY LATENT OR PATENT
+ * DEFECTS, AND CAVIUM SPECIFICALLY DISCLAIMS ALL IMPLIED (IF ANY)
+ * WARRANTIES OF TITLE, MERCHANTABILITY, NONINFRINGEMENT, FITNESS FOR A
+ * PARTICULAR PURPOSE, LACK OF VIRUSES, ACCURACY OR COMPLETENESS, QUIET
+ * ENJOYMENT, QUIET POSSESSION OR CORRESPONDENCE TO DESCRIPTION. THE
+ * ENTIRE  RISK ARISING OUT OF USE OR PERFORMANCE OF THE SOFTWARE LIES
+ * WITH YOU.
+ ***********************license end**************************************/
+
+#include "zip_crypto.h"
+
+static void zip_static_init_zip_ops(struct zip_operation *zip_ops,
+				    int lzs_flag)
+{
+	zip_ops->flush        = ZIP_FLUSH_FINISH;
+
+	/* equivalent to level 6 of opensource zlib */
+	zip_ops->speed          = 1;
+
+	if (!lzs_flag) {
+		zip_ops->ccode		= 0; /* Auto Huffman */
+		zip_ops->lzs_flag	= 0;
+		zip_ops->format		= ZLIB_FORMAT;
+	} else {
+		zip_ops->ccode		= 3; /* LZS Encoding */
+		zip_ops->lzs_flag	= 1;
+		zip_ops->format		= LZS_FORMAT;
+	}
+	zip_ops->begin_file   = 1;
+	zip_ops->history_len  = 0;
+	zip_ops->end_file     = 1;
+	zip_ops->compcode     = 0;
+	zip_ops->csum	      = 1; /* Adler checksum desired */
+}
+
+/* Legacy Compress framework start */
+
+int zip_alloc_zip_ctx(struct crypto_tfm *tfm)
+{
+	struct zip_kernel_ctx *zip_ctx    = crypto_tfm_ctx(tfm);
+	struct zip_operation  *comp_ctx   = &zip_ctx->zip_comp;
+	struct zip_operation  *decomp_ctx = &zip_ctx->zip_decomp;
+
+	zip_static_init_zip_ops(comp_ctx, 0);
+	zip_static_init_zip_ops(decomp_ctx, 0);
+
+	comp_ctx->input  = zip_data_buf_alloc(MAX_INPUT_BUFFER_SIZE);
+	if (!comp_ctx->input)
+		return -ENOMEM;
+
+	comp_ctx->output = zip_data_buf_alloc(MAX_OUTPUT_BUFFER_SIZE);
+	if (!comp_ctx->output)
+		goto err_comp_input;
+
+	decomp_ctx->input  = zip_data_buf_alloc(MAX_INPUT_BUFFER_SIZE);
+	if (!decomp_ctx->input)
+		goto err_comp_output;
+
+	decomp_ctx->output = zip_data_buf_alloc(MAX_OUTPUT_BUFFER_SIZE);
+	if (!decomp_ctx->output)
+		goto err_decomp_input;
+
+	return 0;
+
+err_decomp_input:
+	zip_data_buf_free(decomp_ctx->input, MAX_INPUT_BUFFER_SIZE);
+
+err_comp_output:
+	zip_data_buf_free(comp_ctx->output, MAX_OUTPUT_BUFFER_SIZE);
+
+err_comp_input:
+	zip_data_buf_free(comp_ctx->input, MAX_INPUT_BUFFER_SIZE);
+
+	return -ENOMEM;
+}
+
+int zip_alloc_lzs_ctx(struct crypto_tfm *tfm)
+{
+	struct zip_kernel_ctx *zip_ctx    = crypto_tfm_ctx(tfm);
+	struct zip_operation  *comp_ctx   = &zip_ctx->zip_comp;
+	struct zip_operation  *decomp_ctx = &zip_ctx->zip_decomp;
+
+	zip_static_init_zip_ops(comp_ctx, 1);
+	zip_static_init_zip_ops(decomp_ctx, 1);
+
+	comp_ctx->input  = zip_data_buf_alloc(MAX_INPUT_BUFFER_SIZE);
+	if (!comp_ctx->input)
+		return -ENOMEM;
+
+	comp_ctx->output = zip_data_buf_alloc(MAX_OUTPUT_BUFFER_SIZE);
+	if (!comp_ctx->output)
+		goto err_comp_input;
+
+	decomp_ctx->input  = zip_data_buf_alloc(MAX_INPUT_BUFFER_SIZE);
+	if (!decomp_ctx->input)
+		goto err_comp_output;
+
+	decomp_ctx->output = zip_data_buf_alloc(MAX_OUTPUT_BUFFER_SIZE);
+	if (!decomp_ctx->output)
+		goto err_decomp_input;
+
+	return 0;
+
+err_decomp_input:
+	zip_data_buf_free(decomp_ctx->input, MAX_INPUT_BUFFER_SIZE);
+
+err_comp_output:
+	zip_data_buf_free(comp_ctx->output, MAX_OUTPUT_BUFFER_SIZE);
+
+err_comp_input:
+	zip_data_buf_free(comp_ctx->input, MAX_INPUT_BUFFER_SIZE);
+
+	return -ENOMEM;
+}
+
+void zip_free_zip_ctx(struct crypto_tfm *tfm)
+{
+	struct zip_kernel_ctx *zip_ctx    = crypto_tfm_ctx(tfm);
+	struct zip_operation  *comp_ctx   = &zip_ctx->zip_comp;
+	struct zip_operation  *dec_ctx = &zip_ctx->zip_decomp;
+
+	zip_data_buf_free(comp_ctx->input, MAX_INPUT_BUFFER_SIZE);
+	zip_data_buf_free(comp_ctx->output, MAX_OUTPUT_BUFFER_SIZE);
+
+	zip_data_buf_free(dec_ctx->input, MAX_INPUT_BUFFER_SIZE);
+	zip_data_buf_free(dec_ctx->output, MAX_OUTPUT_BUFFER_SIZE);
+}
+
+int  zip_deflate_comp(struct crypto_tfm *tfm,
+		      const u8 *src, unsigned int slen,
+		      u8 *dst, unsigned int *dlen)
+{
+	struct zip_kernel_ctx *zip_ctx  = NULL;
+	struct zip_operation  *zip_ops   = NULL;
+	struct zip_state      zip_state;
+	struct zip_device     *zip = NULL;
+	int ret;
+
+	if (!tfm || !src || !dst || !dlen)
+		return -ENOMEM;
+
+	zip = zip_get_device(zip_get_node_id());
+	if (!zip)
+		return -ENODEV;
+
+	memset(&zip_state, 0, sizeof(struct zip_state));
+
+	zip_ctx = crypto_tfm_ctx(tfm);
+	zip_ops = &zip_ctx->zip_comp;
+
+	zip_ops->input_len  = slen;
+	zip_ops->output_len = *dlen;
+
+	memcpy(zip_ops->input, src, slen);
+
+	ret = zip_deflate(zip_ops, &zip_state, zip);
+
+	if (!ret) {
+		*dlen = zip_ops->output_len;
+		memcpy(dst, zip_ops->output, *dlen);
+	}
+
+	return ret;
+}
+
+int  zip_inflate_comp(struct crypto_tfm *tfm,
+		      const u8 *src, unsigned int slen,
+		      u8 *dst, unsigned int *dlen)
+{
+	struct zip_kernel_ctx *zip_ctx  = NULL;
+	struct zip_operation  *zip_ops   = NULL;
+	struct zip_state      zip_state;
+	struct zip_device     *zip = NULL;
+	int ret;
+
+	if (!tfm || !src || !dst || !dlen)
+		return -ENOMEM;
+
+	zip = zip_get_device(zip_get_node_id());
+	if (!zip)
+		return -ENODEV;
+
+	memset(&zip_state, 0, sizeof(struct zip_state));
+
+	zip_ctx = crypto_tfm_ctx(tfm);
+	zip_ops = &zip_ctx->zip_decomp;
+
+	memcpy(zip_ops->input, src, slen);
+
+	/* Work around for a bug in zlib which needs an extra bytes sometimes */
+	if (zip_ops->ccode != 3) /* Not LZS Encoding */
+		zip_ops->input[slen++] = 0;
+
+	zip_ops->input_len  = slen;
+	zip_ops->output_len = *dlen;
+
+	ret = zip_inflate(zip_ops, &zip_state, zip);
+
+	if (!ret) {
+		*dlen = zip_ops->output_len;
+		memcpy(dst, zip_ops->output, *dlen);
+	}
+
+	return ret;
+}
+
+/* Legacy compress framework end */
diff --git a/drivers/crypto/cavium/zip/zip_crypto.h b/drivers/crypto/cavium/zip/zip_crypto.h
index 1215049..26792e9 100644
--- a/drivers/crypto/cavium/zip/zip_crypto.h
+++ b/drivers/crypto/cavium/zip/zip_crypto.h
@@ -48,6 +48,8 @@
 
 #include <linux/crypto.h>
 #include "common.h"
+#include "zip_deflate.h"
+#include "zip_inflate.h"
 
 struct zip_kernel_ctx {
 	struct zip_operation zip_comp;
@@ -57,5 +59,9 @@ struct zip_kernel_ctx {
 int  zip_alloc_zip_ctx(struct crypto_tfm *tfm);
 int  zip_alloc_lzs_ctx(struct crypto_tfm *tfm);
 void zip_free_zip_ctx(struct crypto_tfm *tfm);
+int  zip_deflate_comp(struct crypto_tfm *tfm, const u8 *src, unsigned int slen,
+		      u8 *dst, unsigned int *dlen);
+int  zip_inflate_comp(struct crypto_tfm *tfm, const u8 *src, unsigned int slen,
+		      u8 *dst, unsigned int *dlen);
 
 #endif
diff --git a/drivers/crypto/cavium/zip/zip_deflate.c b/drivers/crypto/cavium/zip/zip_deflate.c
new file mode 100644
index 0000000..913cc25
--- /dev/null
+++ b/drivers/crypto/cavium/zip/zip_deflate.c
@@ -0,0 +1,190 @@
+/***********************license start************************************
+ * Copyright (c) 2003-2016 Cavium, Inc.
+ * All rights reserved.
+ *
+ * License: one of 'Cavium License' or 'GNU General Public License Version 2'
+ *
+ * This file is provided under the terms of the Cavium License (see below)
+ * or under the terms of GNU General Public License, Version 2, as
+ * published by the Free Software Foundation. When using or redistributing
+ * this file, you may do so under either license.
+ *
+ * Cavium License:  Redistribution and use in source and binary forms, with
+ * or without modification, are permitted provided that the following
+ * conditions are met:
+ *
+ *  * Redistributions of source code must retain the above copyright
+ *    notice, this list of conditions and the following disclaimer.
+ *
+ *  * Redistributions in binary form must reproduce the above
+ *    copyright notice, this list of conditions and the following
+ *    disclaimer in the documentation and/or other materials provided
+ *    with the distribution.
+ *
+ *  * Neither the name of Cavium Inc. nor the names of its contributors may be
+ *    used to endorse or promote products derived from this software without
+ *    specific prior written permission.
+ *
+ * This Software, including technical data, may be subject to U.S. export
+ * control laws, including the U.S. Export Administration Act and its
+ * associated regulations, and may be subject to export or import
+ * regulations in other countries.
+ *
+ * TO THE MAXIMUM EXTENT PERMITTED BY LAW, THE SOFTWARE IS PROVIDED "AS IS"
+ * AND WITH ALL FAULTS AND CAVIUM INC. MAKES NO PROMISES, REPRESENTATIONS
+ * OR WARRANTIES, EITHER EXPRESS, IMPLIED, STATUTORY, OR OTHERWISE, WITH
+ * RESPECT TO THE SOFTWARE, INCLUDING ITS CONDITION, ITS CONFORMITY TO ANY
+ * REPRESENTATION OR DESCRIPTION, OR THE EXISTENCE OF ANY LATENT OR PATENT
+ * DEFECTS, AND CAVIUM SPECIFICALLY DISCLAIMS ALL IMPLIED (IF ANY)
+ * WARRANTIES OF TITLE, MERCHANTABILITY, NONINFRINGEMENT, FITNESS FOR A
+ * PARTICULAR PURPOSE, LACK OF VIRUSES, ACCURACY OR COMPLETENESS, QUIET
+ * ENJOYMENT, QUIET POSSESSION OR CORRESPONDENCE TO DESCRIPTION. THE
+ * ENTIRE  RISK ARISING OUT OF USE OR PERFORMANCE OF THE SOFTWARE LIES
+ * WITH YOU.
+ ***********************license end**************************************/
+
+#include <linux/delay.h>
+#include <linux/sched.h>
+
+#include "common.h"
+#include "zip_deflate.h"
+
+/* Prepares the deflate zip command */
+static int prepare_zip_command(struct zip_operation *zip_ops,
+			       struct zip_state *s, union zip_inst_s *zip_cmd)
+{
+	union zip_zres_s *result_ptr = &s->result;
+
+	memset(zip_cmd, 0, sizeof(s->zip_cmd));
+	memset(result_ptr, 0, sizeof(s->result));
+
+	/* IWORD #0 */
+	/* History gather */
+	zip_cmd->s.hg = 0;
+	/* compression enable = 1 for deflate */
+	zip_cmd->s.ce = 1;
+	/* sf (sync flush) */
+	zip_cmd->s.sf = 1;
+	/* ef (end of file) */
+	if (zip_ops->flush == ZIP_FLUSH_FINISH) {
+		zip_cmd->s.ef = 1;
+		zip_cmd->s.sf = 0;
+	}
+
+	zip_cmd->s.cc = zip_ops->ccode;
+	/* ss (compression speed/storage) */
+	zip_cmd->s.ss = zip_ops->speed;
+
+	/* IWORD #1 */
+	/* adler checksum */
+	zip_cmd->s.adlercrc32 = zip_ops->csum;
+	zip_cmd->s.historylength = zip_ops->history_len;
+	zip_cmd->s.dg = 0;
+
+	/* IWORD # 6 and 7 - compression input/history pointer */
+	zip_cmd->s.inp_ptr_addr.s.addr  = __pa(zip_ops->input);
+	zip_cmd->s.inp_ptr_ctl.s.length = (zip_ops->input_len +
+					   zip_ops->history_len);
+	zip_cmd->s.ds = 0;
+
+	/* IWORD # 8 and 9 - Output pointer */
+	zip_cmd->s.out_ptr_addr.s.addr  = __pa(zip_ops->output);
+	zip_cmd->s.out_ptr_ctl.s.length = zip_ops->output_len;
+	/* maximum number of output-stream bytes that can be written */
+	zip_cmd->s.totaloutputlength    = zip_ops->output_len;
+
+	/* IWORD # 10 and 11 - Result pointer */
+	zip_cmd->s.res_ptr_addr.s.addr = __pa(result_ptr);
+	/* Clearing completion code */
+	result_ptr->s.compcode = 0;
+
+	return 0;
+}
+
+/**
+ * zip_deflate - API to offload deflate operation to hardware
+ * @zip_ops: Pointer to zip operation structure
+ * @s:       Pointer to the structure representing zip state
+ * @zip_dev: Pointer to zip device structure
+ *
+ * This function prepares the zip deflate command and submits it to the zip
+ * engine for processing.
+ *
+ * Return: 0 if successful or error code
+ */
+int zip_deflate(struct zip_operation *zip_ops, struct zip_state *s,
+		struct zip_device *zip_dev)
+{
+	union zip_inst_s *zip_cmd = &s->zip_cmd;
+	union zip_zres_s *result_ptr = &s->result;
+	u32 queue;
+
+	/* Prepares zip command based on the input parameters */
+	prepare_zip_command(zip_ops, s, zip_cmd);
+
+	/* Loads zip command into command queues and rings door bell */
+	queue = zip_load_instr(zip_cmd, zip_dev);
+
+	while (!result_ptr->s.compcode)
+		continue;
+
+	zip_ops->compcode = result_ptr->s.compcode;
+	switch (zip_ops->compcode) {
+	case ZIP_NOTDONE:
+		zip_dbg("Zip instruction not yet completed");
+		return ZIP_ERROR;
+
+	case ZIP_SUCCESS:
+		zip_dbg("Zip instruction completed successfully");
+		zip_update_cmd_bufs(zip_dev, queue);
+		break;
+
+	case ZIP_DTRUNC:
+		zip_dbg("Output Truncate error");
+		/* Returning ZIP_ERROR to avoid copy to user */
+		return ZIP_ERROR;
+
+	default:
+		zip_err("Zip instruction failed. Code:%d", zip_ops->compcode);
+		return ZIP_ERROR;
+	}
+
+	/* Update the CRC depending on the format */
+	switch (zip_ops->format) {
+	case RAW_FORMAT:
+		zip_dbg("RAW Format: %d ", zip_ops->format);
+		/* Get checksum from engine, need to feed it again */
+		zip_ops->csum = result_ptr->s.adler32;
+		break;
+
+	case ZLIB_FORMAT:
+		zip_dbg("ZLIB Format: %d ", zip_ops->format);
+		zip_ops->csum = result_ptr->s.adler32;
+		break;
+
+	case GZIP_FORMAT:
+		zip_dbg("GZIP Format: %d ", zip_ops->format);
+		zip_ops->csum = result_ptr->s.crc32;
+		break;
+
+	case LZS_FORMAT:
+		zip_dbg("LZS Format: %d ", zip_ops->format);
+		break;
+
+	default:
+		zip_err("Unknown Format:%d\n", zip_ops->format);
+	}
+
+	/* Update output_len */
+	if (zip_ops->output_len < result_ptr->s.totalbyteswritten) {
+		/* Dynamic stop && strm->output_len < zipconstants[onfsize] */
+		zip_err("output_len (%d) < total bytes written(%d)\n",
+			zip_ops->output_len, result_ptr->s.totalbyteswritten);
+		zip_ops->output_len = 0;
+
+	} else {
+		zip_ops->output_len = result_ptr->s.totalbyteswritten;
+	}
+
+	return 0;
+}
diff --git a/drivers/crypto/cavium/zip/zip_deflate.h b/drivers/crypto/cavium/zip/zip_deflate.h
new file mode 100644
index 0000000..bdb5207
--- /dev/null
+++ b/drivers/crypto/cavium/zip/zip_deflate.h
@@ -0,0 +1,62 @@
+/***********************license start************************************
+ * Copyright (c) 2003-2016 Cavium, Inc.
+ * All rights reserved.
+ *
+ * License: one of 'Cavium License' or 'GNU General Public License Version 2'
+ *
+ * This file is provided under the terms of the Cavium License (see below)
+ * or under the terms of GNU General Public License, Version 2, as
+ * published by the Free Software Foundation. When using or redistributing
+ * this file, you may do so under either license.
+ *
+ * Cavium License:  Redistribution and use in source and binary forms, with
+ * or without modification, are permitted provided that the following
+ * conditions are met:
+ *
+ *  * Redistributions of source code must retain the above copyright
+ *    notice, this list of conditions and the following disclaimer.
+ *
+ *  * Redistributions in binary form must reproduce the above
+ *    copyright notice, this list of conditions and the following
+ *    disclaimer in the documentation and/or other materials provided
+ *    with the distribution.
+ *
+ *  * Neither the name of Cavium Inc. nor the names of its contributors may be
+ *    used to endorse or promote products derived from this software without
+ *    specific prior written permission.
+ *
+ * This Software, including technical data, may be subject to U.S. export
+ * control laws, including the U.S. Export Administration Act and its
+ * associated regulations, and may be subject to export or import
+ * regulations in other countries.
+ *
+ * TO THE MAXIMUM EXTENT PERMITTED BY LAW, THE SOFTWARE IS PROVIDED "AS IS"
+ * AND WITH ALL FAULTS AND CAVIUM INC. MAKES NO PROMISES, REPRESENTATIONS
+ * OR WARRANTIES, EITHER EXPRESS, IMPLIED, STATUTORY, OR OTHERWISE, WITH
+ * RESPECT TO THE SOFTWARE, INCLUDING ITS CONDITION, ITS CONFORMITY TO ANY
+ * REPRESENTATION OR DESCRIPTION, OR THE EXISTENCE OF ANY LATENT OR PATENT
+ * DEFECTS, AND CAVIUM SPECIFICALLY DISCLAIMS ALL IMPLIED (IF ANY)
+ * WARRANTIES OF TITLE, MERCHANTABILITY, NONINFRINGEMENT, FITNESS FOR A
+ * PARTICULAR PURPOSE, LACK OF VIRUSES, ACCURACY OR COMPLETENESS, QUIET
+ * ENJOYMENT, QUIET POSSESSION OR CORRESPONDENCE TO DESCRIPTION. THE
+ * ENTIRE  RISK ARISING OUT OF USE OR PERFORMANCE OF THE SOFTWARE LIES
+ * WITH YOU.
+ ***********************license end**************************************/
+
+#ifndef __ZIP_DEFLATE_H__
+#define __ZIP_DEFLATE_H__
+
+/**
+ * zip_deflate - API to offload deflate operation to hardware
+ * @zip_ops: Pointer to zip operation structure
+ * @s:       Pointer to the structure representing zip state
+ * @zip_dev: Pointer to the structure representing zip device
+ *
+ * This function prepares the zip deflate command and submits it to the zip
+ * engine by ringing the doorbell.
+ *
+ * Return: 0 if successful or error code
+ */
+int zip_deflate(struct zip_operation *zip_ops, struct zip_state *s,
+		struct zip_device *zip_dev);
+#endif
diff --git a/drivers/crypto/cavium/zip/zip_device.c b/drivers/crypto/cavium/zip/zip_device.c
index ed21c5a..a72cdcf0 100644
--- a/drivers/crypto/cavium/zip/zip_device.c
+++ b/drivers/crypto/cavium/zip/zip_device.c
@@ -44,6 +44,7 @@
  ***********************license end**************************************/
 
 #include "common.h"
+#include "zip_deflate.h"
 
 /**
  * zip_cmd_queue_consumed - Calculates the space consumed in the command queue.
diff --git a/drivers/crypto/cavium/zip/zip_inflate.c b/drivers/crypto/cavium/zip/zip_inflate.c
new file mode 100644
index 0000000..849c4c85
--- /dev/null
+++ b/drivers/crypto/cavium/zip/zip_inflate.c
@@ -0,0 +1,211 @@
+/***********************license start************************************
+ * Copyright (c) 2003-2016 Cavium, Inc.
+ * All rights reserved.
+ *
+ * License: one of 'Cavium License' or 'GNU General Public License Version 2'
+ *
+ * This file is provided under the terms of the Cavium License (see below)
+ * or under the terms of GNU General Public License, Version 2, as
+ * published by the Free Software Foundation. When using or redistributing
+ * this file, you may do so under either license.
+ *
+ * Cavium License:  Redistribution and use in source and binary forms, with
+ * or without modification, are permitted provided that the following
+ * conditions are met:
+ *
+ *  * Redistributions of source code must retain the above copyright
+ *    notice, this list of conditions and the following disclaimer.
+ *
+ *  * Redistributions in binary form must reproduce the above
+ *    copyright notice, this list of conditions and the following
+ *    disclaimer in the documentation and/or other materials provided
+ *    with the distribution.
+ *
+ *  * Neither the name of Cavium Inc. nor the names of its contributors may be
+ *    used to endorse or promote products derived from this software without
+ *    specific prior written permission.
+ *
+ * This Software, including technical data, may be subject to U.S. export
+ * control laws, including the U.S. Export Administration Act and its
+ * associated regulations, and may be subject to export or import
+ * regulations in other countries.
+ *
+ * TO THE MAXIMUM EXTENT PERMITTED BY LAW, THE SOFTWARE IS PROVIDED "AS IS"
+ * AND WITH ALL FAULTS AND CAVIUM INC. MAKES NO PROMISES, REPRESENTATIONS
+ * OR WARRANTIES, EITHER EXPRESS, IMPLIED, STATUTORY, OR OTHERWISE, WITH
+ * RESPECT TO THE SOFTWARE, INCLUDING ITS CONDITION, ITS CONFORMITY TO ANY
+ * REPRESENTATION OR DESCRIPTION, OR THE EXISTENCE OF ANY LATENT OR PATENT
+ * DEFECTS, AND CAVIUM SPECIFICALLY DISCLAIMS ALL IMPLIED (IF ANY)
+ * WARRANTIES OF TITLE, MERCHANTABILITY, NONINFRINGEMENT, FITNESS FOR A
+ * PARTICULAR PURPOSE, LACK OF VIRUSES, ACCURACY OR COMPLETENESS, QUIET
+ * ENJOYMENT, QUIET POSSESSION OR CORRESPONDENCE TO DESCRIPTION. THE
+ * ENTIRE  RISK ARISING OUT OF USE OR PERFORMANCE OF THE SOFTWARE LIES
+ * WITH YOU.
+ ***********************license end**************************************/
+
+#include <linux/delay.h>
+#include <linux/sched.h>
+
+#include "common.h"
+#include "zip_inflate.h"
+
+static int prepare_inflate_zcmd(struct zip_operation *zip_ops,
+				struct zip_state *s, union zip_inst_s *zip_cmd)
+{
+	union zip_zres_s *result_ptr = &s->result;
+
+	memset(zip_cmd, 0, sizeof(s->zip_cmd));
+	memset(result_ptr, 0, sizeof(s->result));
+
+	/* IWORD#0 */
+
+	/* Decompression History Gather list - no gather list */
+	zip_cmd->s.hg = 0;
+	/* For decompression, CE must be 0x0. */
+	zip_cmd->s.ce = 0;
+	/* For decompression, SS must be 0x0. */
+	zip_cmd->s.ss = 0;
+	/* For decompression, SF should always be set. */
+	zip_cmd->s.sf = 1;
+
+	/* Begin File */
+	if (zip_ops->begin_file == 0)
+		zip_cmd->s.bf = 0;
+	else
+		zip_cmd->s.bf = 1;
+
+	zip_cmd->s.ef = 1;
+	/* 0: for Deflate decompression, 3: for LZS decompression */
+	zip_cmd->s.cc = zip_ops->ccode;
+
+	/* IWORD #1*/
+
+	/* adler checksum */
+	zip_cmd->s.adlercrc32 = zip_ops->csum;
+
+	/*
+	 * HISTORYLENGTH must be 0x0 for any ZIP decompress operation.
+	 * History data is added to a decompression operation via IWORD3.
+	 */
+	zip_cmd->s.historylength = 0;
+	zip_cmd->s.ds = 0;
+
+	/* IWORD # 8 and 9 - Output pointer */
+	zip_cmd->s.out_ptr_addr.s.addr  = __pa(zip_ops->output);
+	zip_cmd->s.out_ptr_ctl.s.length = zip_ops->output_len;
+
+	/* Maximum number of output-stream bytes that can be written */
+	zip_cmd->s.totaloutputlength    = zip_ops->output_len;
+
+	zip_dbg("Data Direct Input case ");
+
+	/* IWORD # 6 and 7 - input pointer */
+	zip_cmd->s.dg = 0;
+	zip_cmd->s.inp_ptr_addr.s.addr  = __pa((u8 *)zip_ops->input);
+	zip_cmd->s.inp_ptr_ctl.s.length = zip_ops->input_len;
+
+	/* IWORD # 10 and 11 - Result pointer */
+	zip_cmd->s.res_ptr_addr.s.addr = __pa(result_ptr);
+
+	/* Clearing completion code */
+	result_ptr->s.compcode = 0;
+
+	/* Returning 0 for time being.*/
+	return 0;
+}
+
+/**
+ * zip_inflate - API to offload inflate operation to hardware
+ * @zip_ops: Pointer to zip operation structure
+ * @s:       Pointer to the structure representing zip state
+ * @zip_dev: Pointer to zip device structure
+ *
+ * This function prepares the zip inflate command and submits it to the zip
+ * engine for processing.
+ *
+ * Return: 0 if successful or error code
+ */
+int zip_inflate(struct zip_operation *zip_ops, struct zip_state *s,
+		struct zip_device *zip_dev)
+{
+	union zip_inst_s *zip_cmd    = &s->zip_cmd;
+	union zip_zres_s  *result_ptr = &s->result;
+	u32 queue;
+
+	/* Prepare inflate zip command */
+	prepare_inflate_zcmd(zip_ops, s, zip_cmd);
+
+	/* Load inflate command to zip queue and ring the doorbell */
+	queue = zip_load_instr(zip_cmd, zip_dev);
+
+	while (!result_ptr->s.compcode)
+		continue;
+
+	zip_ops->compcode = result_ptr->s.compcode;
+	switch (zip_ops->compcode) {
+	case ZIP_NOTDONE:
+		zip_dbg("Zip Instruction not yet completed\n");
+		return ZIP_ERROR;
+
+	case ZIP_SUCCESS:
+		zip_dbg("Zip Instruction completed successfully\n");
+		break;
+
+	case ZIP_DYNAMIC_STOP:
+		zip_dbg(" Dynamic stop Initiated\n");
+		break;
+
+	default:
+		zip_dbg("Instruction failed. Code = %d\n", zip_ops->compcode);
+		zip_update_cmd_bufs(zip_dev, queue);
+		return ZIP_ERROR;
+	}
+
+	zip_update_cmd_bufs(zip_dev, queue);
+
+	if ((zip_ops->ccode == 3) && (zip_ops->flush == 4) &&
+	    (zip_ops->compcode != ZIP_DYNAMIC_STOP))
+		result_ptr->s.ef = 1;
+
+	zip_ops->csum = result_ptr->s.adler32;
+
+	if (zip_ops->output_len < result_ptr->s.totalbyteswritten) {
+		zip_err("output_len (%d) < total bytes written (%d)\n",
+			zip_ops->output_len, result_ptr->s.totalbyteswritten);
+		zip_ops->output_len = 0;
+	} else {
+		zip_ops->output_len = result_ptr->s.totalbyteswritten;
+	}
+
+	zip_ops->bytes_read = result_ptr->s.totalbytesread;
+	zip_ops->bits_processed = result_ptr->s.totalbitsprocessed;
+	zip_ops->end_file = result_ptr->s.ef;
+	if (zip_ops->end_file) {
+		switch (zip_ops->format) {
+		case RAW_FORMAT:
+			zip_dbg("RAW Format: %d ", zip_ops->format);
+			/* Get checksum from engine */
+			zip_ops->csum = result_ptr->s.adler32;
+			break;
+
+		case ZLIB_FORMAT:
+			zip_dbg("ZLIB Format: %d ", zip_ops->format);
+			zip_ops->csum = result_ptr->s.adler32;
+			break;
+
+		case GZIP_FORMAT:
+			zip_dbg("GZIP Format: %d ", zip_ops->format);
+			zip_ops->csum = result_ptr->s.crc32;
+			break;
+
+		case LZS_FORMAT:
+			zip_dbg("LZS Format: %d ", zip_ops->format);
+			break;
+
+		default:
+			zip_err("Format error:%d\n", zip_ops->format);
+		}
+	}
+
+	return 0;
+}
diff --git a/drivers/crypto/cavium/zip/zip_inflate.h b/drivers/crypto/cavium/zip/zip_inflate.h
new file mode 100644
index 0000000..4cee4c9
--- /dev/null
+++ b/drivers/crypto/cavium/zip/zip_inflate.h
@@ -0,0 +1,62 @@
+/***********************license start************************************
+ * Copyright (c) 2003-2016 Cavium, Inc.
+ * All rights reserved.
+ *
+ * License: one of 'Cavium License' or 'GNU General Public License Version 2'
+ *
+ * This file is provided under the terms of the Cavium License (see below)
+ * or under the terms of GNU General Public License, Version 2, as
+ * published by the Free Software Foundation. When using or redistributing
+ * this file, you may do so under either license.
+ *
+ * Cavium License:  Redistribution and use in source and binary forms, with
+ * or without modification, are permitted provided that the following
+ * conditions are met:
+ *
+ *  * Redistributions of source code must retain the above copyright
+ *    notice, this list of conditions and the following disclaimer.
+ *
+ *  * Redistributions in binary form must reproduce the above
+ *    copyright notice, this list of conditions and the following
+ *    disclaimer in the documentation and/or other materials provided
+ *    with the distribution.
+ *
+ *  * Neither the name of Cavium Inc. nor the names of its contributors may be
+ *    used to endorse or promote products derived from this software without
+ *    specific prior written permission.
+ *
+ * This Software, including technical data, may be subject to U.S. export
+ * control laws, including the U.S. Export Administration Act and its
+ * associated regulations, and may be subject to export or import
+ * regulations in other countries.
+ *
+ * TO THE MAXIMUM EXTENT PERMITTED BY LAW, THE SOFTWARE IS PROVIDED "AS IS"
+ * AND WITH ALL FAULTS AND CAVIUM INC. MAKES NO PROMISES, REPRESENTATIONS
+ * OR WARRANTIES, EITHER EXPRESS, IMPLIED, STATUTORY, OR OTHERWISE, WITH
+ * RESPECT TO THE SOFTWARE, INCLUDING ITS CONDITION, ITS CONFORMITY TO ANY
+ * REPRESENTATION OR DESCRIPTION, OR THE EXISTENCE OF ANY LATENT OR PATENT
+ * DEFECTS, AND CAVIUM SPECIFICALLY DISCLAIMS ALL IMPLIED (IF ANY)
+ * WARRANTIES OF TITLE, MERCHANTABILITY, NONINFRINGEMENT, FITNESS FOR A
+ * PARTICULAR PURPOSE, LACK OF VIRUSES, ACCURACY OR COMPLETENESS, QUIET
+ * ENJOYMENT, QUIET POSSESSION OR CORRESPONDENCE TO DESCRIPTION. THE
+ * ENTIRE  RISK ARISING OUT OF USE OR PERFORMANCE OF THE SOFTWARE LIES
+ * WITH YOU.
+ ***********************license end**************************************/
+
+#ifndef __ZIP_INFLATE_H__
+#define __ZIP_INFLATE_H__
+
+/**
+ * zip_inflate - API to offload inflate operation to hardware
+ * @zip_ops: Pointer to zip operation structure
+ * @s:       Pointer to the structure representing zip state
+ * @zip_dev: Pointer to the structure representing zip device
+ *
+ * This function prepares the zip inflate command and submits it to the zip
+ * engine for processing.
+ *
+ * Return: 0 if successful or error code
+ */
+int zip_inflate(struct zip_operation *zip_ops, struct zip_state *s,
+		struct zip_device *zip_dev);
+#endif
diff --git a/drivers/crypto/cavium/zip/zip_main.c b/drivers/crypto/cavium/zip/zip_main.c
index 052c42d..ae3395f 100644
--- a/drivers/crypto/cavium/zip/zip_main.c
+++ b/drivers/crypto/cavium/zip/zip_main.c
@@ -364,35 +364,6 @@ static void zip_remove(struct pci_dev *pdev)
 	zip_dbg_exit();
 }
 
-/* Dummy Functions */
-int zip_alloc_lzs_ctx(struct crypto_tfm *tfm)
-{
-	return 0;
-}
-
-int zip_alloc_zip_ctx(struct crypto_tfm *tfm)
-{
-	return 0;
-}
-
-void zip_free_zip_ctx(struct crypto_tfm *tfm)
-{
-}
-
-int  zip_deflate_comp(struct crypto_tfm *tfm,
-		      const u8 *src, unsigned int slen,
-		      u8 *dst, unsigned int *dlen)
-{
-	return 0;
-}
-
-int  zip_inflate_comp(struct crypto_tfm *tfm,
-		      const u8 *src, unsigned int slen,
-		      u8 *dst, unsigned int *dlen)
-{
-	return 0;
-}
-
 /* PCI Sub-System Interface */
 static struct pci_driver zip_driver = {
 	.name	    =  DRV_NAME,
-- 
2.9.0.rc0.21.g7777322

[toc] | [prev] | [next] | [standalone]


#1541066 — Re: [RFC PATCH 1/3] crypto: zip - Add ThunderX ZIP driver core

FromCorentin Labbe <clabbe.montjoie@gmail.com>
Date2016-12-13 14:40 +0100
SubjectRe: [RFC PATCH 1/3] crypto: zip - Add ThunderX ZIP driver core
Message-ID<sNVtg-1Kh-9@gated-at.bofh.it>
In reply to#1540369
Hello

I have some comment below

On Mon, Dec 12, 2016 at 04:04:37PM +0100, Jan Glauber wrote:
> From: Mahipal Challa <Mahipal.Challa@cavium.com>
> 
[...]
> --- a/drivers/crypto/Makefile
> +++ b/drivers/crypto/Makefile
> @@ -27,6 +27,7 @@ obj-$(CONFIG_CRYPTO_DEV_MXC_SCC) += mxc-scc.o
>  obj-$(CONFIG_CRYPTO_DEV_TALITOS) += talitos.o
>  obj-$(CONFIG_CRYPTO_DEV_UX500) += ux500/
>  obj-$(CONFIG_CRYPTO_DEV_QAT) += qat/
> +obj-$(CONFIG_CRYPTO_DEV_CAVIUM_ZIP) += cavium/
>  obj-$(CONFIG_CRYPTO_DEV_QCE) += qce/
>  obj-$(CONFIG_CRYPTO_DEV_VMX) += vmx/
>  obj-$(CONFIG_CRYPTO_DEV_SUN4I_SS) += sunxi-ss/

Try to keep some alphabetical order

[...]
> +/* ZIP invocation result completion status codes */
> +#define ZIP_NOTDONE		0x0
> +
> +/* Successful completion. */
> +#define ZIP_SUCCESS		0x1
> +
> +/* Output truncated */
> +#define ZIP_DTRUNC		0x2
> +
> +/* Dynamic Stop */
> +#define ZIP_DYNAMIC_STOP	0x3
> +
> +/* Uncompress ran out of input data when IWORD0[EF] was set */
> +#define ZIP_ITRUNC		0x4
> +
> +/* Uncompress found the reserved block type 3 */
> +#define ZIP_RBLOCK		0x5
> +
> +/* Uncompress found LEN != ZIP_NLEN in an uncompressed block in the input */
> +#define ZIP_NLEN		0x6
> +
> +/* Uncompress found a bad code in the main Huffman codes. */
> +#define ZIP_BADCODE		0x7
> +
> +/* Uncompress found a bad code in the 19 Huffman codes encoding lengths. */
> +#define ZIP_BADCODE2	        0x8
> +
> +/* Compress found a zero-length input. */
> +#define ZIP_ZERO_LEN	        0x9
> +
> +/* The compress or decompress encountered an internal parity error. */
> +#define ZIP_PARITY		0xA

Perhaps all errors could begin with ZIP_ERR_xxx ?

[...]
> +static inline u64 zip_depth(void)
> +{
> +	struct zip_device *zip_dev = zip_get_device(zip_get_node_id());
> +
> +	if (!zip_dev)
> +		return -ENODEV;
> +
> +	return zip_dev->depth;
> +}

This function is not used.

> +
> +static inline u64 zip_onfsize(void)
> +{
> +	struct zip_device *zip_dev = zip_get_device(zip_get_node_id());
> +
> +	if (!zip_dev)
> +		return -ENODEV;
> +
> +	return zip_dev->onfsize;
> +}

Same for this

> +
> +static inline u64 zip_ctxsize(void)
> +{
> +	struct zip_device *zip_dev = zip_get_device(zip_get_node_id());
> +
> +	if (!zip_dev)
> +		return -ENODEV;
> +
> +	return zip_dev->ctxsize;
> +}

Again

[...]
> +
> +/*
> + * Allocates new ZIP device structure
> + * Returns zip_device pointer or NULL if cannot allocate memory for zip_device
> + */
> +static struct zip_device *zip_alloc_device(struct pci_dev *pdev)

pdev is not used, so you can remove it from arglist.
Or keep it and use devm_kzalloc for allocating zip

> +{
> +	struct zip_device *zip = NULL;
> +	int idx = 0;
> +
> +	for (idx = 0; idx < MAX_ZIP_DEVICES; idx++) {
> +		if (!zip_dev[idx])
> +			break;
> +	}
> +
> +	zip = kzalloc(sizeof(*zip), GFP_KERNEL);
> +
> +	if (!zip)
> +		return NULL;
> +
> +	zip_dev[idx] = zip;
> +	zip->index = idx;
> +	return zip;
> +}

[...]
> +static int zip_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
> +{
> +	struct device *dev = &pdev->dev;
> +	struct zip_device *zip = NULL;
> +	int    err;
> +
> +	zip_dbg_enter();
> +
> +	zip = zip_alloc_device(pdev);
> +
> +	if (!zip)
> +		return -ENOMEM;
> +
> +	pr_info("Found ZIP device %d %x:%x on Node %d\n", zip->index,
> +		pdev->vendor, pdev->device, dev_to_node(dev));

You use lots of pr_info, why not using more dev_info/dev_err ?

> +
> +	zip->pdev = pdev;
> +
> +	pci_set_drvdata(pdev, zip);
> +
> +	err = pci_enable_device(pdev);
> +	if (err) {
> +		zip_err("Failed to enable PCI device");
> +		goto err_free_device;
> +	}
> +
> +	err = pci_request_regions(pdev, DRV_NAME);
> +	if (err) {
> +		zip_err("PCI request regions failed 0x%x", err);
> +		goto err_disable_device;
> +	}
> +
> +	err = pci_set_dma_mask(pdev, DMA_BIT_MASK(48));
> +	if (err) {
> +		dev_err(dev, "Unable to get usable DMA configuration\n");
> +		goto err_release_regions;
> +	}
> +
> +	err = pci_set_consistent_dma_mask(pdev, DMA_BIT_MASK(48));
> +	if (err) {
> +		dev_err(dev, "Unable to get 48-bit DMA for allocations\n");
> +		goto err_release_regions;
> +	}
> +
> +	/* MAP configuration registers */
> +	zip->reg_base = pci_ioremap_bar(pdev, PCI_CFG_ZIP_PF_BAR0);
> +	if (!zip->reg_base) {
> +		zip_err("ZIP: Cannot map BAR0 CSR memory space, aborting");
> +		err = -ENOMEM;
> +		goto err_release_regions;
> +	}
> +
> +	/* Initialize ZIP Hardware */
> +	err = zip_init_hw(zip);
> +	if (err)
> +		goto err_release_regions;
> +
> +	return 0;
> +
> +err_release_regions:
> +	if (zip->reg_base)
> +		iounmap(zip->reg_base);
> +	pci_release_regions(pdev);
> +
> +err_disable_device:
> +	pci_disable_device(pdev);
> +
> +err_free_device:
> +	pci_set_drvdata(pdev, NULL);
> +
> +	/* remove zip_dev from zip_device list, free the zip_device memory */
> +	zip_dev[zip->index] = NULL;
> +	kfree(zip);
> +
> +	zip_dbg_exit();
> +	return err;
> +}

[...]
> +static int __init zip_init_module(void)
> +{
> +	int ret;
> +
> +	memset(&zip_dev, 0, sizeof(zip_dev));

A static variable is already zeroed

> +
> +	zip_msg("%s\n", DRV_NAME);
> +
> +	ret = pci_register_driver(&zip_driver);
> +	if (ret < 0) {
> +		zip_err("ZIP: pci_register_driver() returned %d\n", ret);
> +		return ret;
> +	}
> +
> +	/* Register with the Kernel Crypto Interface */
> +	ret = zip_register_compression_device();
> +	if (ret < 0) {
> +		zip_err("ZIP: Kernel Crypto Registration failed\n");
> +		return 1;

1 is not a good error code. And you quit without unregistering.

> +	}
> +
> +	return ret;
> +}
> +
> +static void __exit zip_cleanup_module(void)
> +{
> +	/* Unregister this driver for pci zip devices */
> +	pci_unregister_driver(&zip_driver);
> +
> +	/* Unregister from the kernel crypto interface */
> +	zip_unregister_compression_device();

I think you must do the opposite. (unregister crypto first)

> +
> +	pr_info("ThunderX-ZIP driver is removed successfully\n");
> +}
> +
> +module_init(zip_init_module);
> +module_exit(zip_cleanup_module);

Why not using module_pci_driver ?

[...]
> +/**
> + * enum zip_comp_e - ZIP Completion Enumeration, enumerates the values of
> + * ZIP_ZRES_S[COMPCODE].
> + */
> +enum zip_comp_e {
> +	ZIP_COMP_E_BADCODE = 0x7,
> +	ZIP_COMP_E_BADCODE2 = 0x8,
> +	ZIP_COMP_E_DTRUNC = 0x2,
> +	ZIP_COMP_E_FATAL = 0xb,
> +	ZIP_COMP_E_ITRUNC = 0x4,
> +	ZIP_COMP_E_NLEN = 0x6,
> +	ZIP_COMP_E_NOTDONE = 0x0,
> +	ZIP_COMP_E_PARITY = 0xa,
> +	ZIP_COMP_E_RBLOCK = 0x5,
> +	ZIP_COMP_E_STOP = 0x3,
> +	ZIP_COMP_E_SUCCESS = 0x1,
> +	ZIP_COMP_E_ZERO_LEN = 0x9,
> +	ZIP_COMP_E_ENUM_LAST = 0xc,

Why not using already declared define ? ZIP_COMP_E_BADCODE = ZIP_BADCODE 

Regards
Corentin Labbe

[toc] | [prev] | [next] | [standalone]


#1544636 — Re: [RFC PATCH 1/3] crypto: zip - Add ThunderX ZIP driver core

FromSasha Levin <levinsasha928@gmail.com>
Date2016-12-19 17:20 +0100
SubjectRe: [RFC PATCH 1/3] crypto: zip - Add ThunderX ZIP driver core
Message-ID<sQ8Pn-1KN-3@gated-at.bofh.it>
In reply to#1540369
On Mon, Dec 12, 2016 at 10:04 AM, Jan Glauber <jglauber@cavium.com> wrote:
> +/* error messages */
> +#define zip_err(fmt, args...) pr_err("ZIP ERR:%s():%d: " \
> +                             fmt "\n", __func__, __LINE__, ## args)
> +
> +#ifdef MSG_ENABLE
> +/* Enable all messages */
> +#define zip_msg(fmt, args...) pr_info("ZIP_MSG:" fmt "\n", ## args)
> +#else
> +#define zip_msg(fmt, args...)
> +#endif
> +
> +#if defined(ZIP_DEBUG_ENABLE) && defined(MSG_ENABLE)
> +
> +#ifdef DEBUG_LEVEL
> +
> +#define FILE_NAME (strrchr(__FILE__, '/') ? strrchr(__FILE__, '/') + 1 : \
> +       strrchr(__FILE__, '\\') ? strrchr(__FILE__, '\\') + 1 : __FILE__)
> +
> +#if DEBUG_LEVEL >= 4
> +
> +#define zip_dbg(fmt, args...) pr_info("ZIP DBG: %s: %s() : %d: " \
> +                             fmt "\n", FILE_NAME, __func__, __LINE__, ## args)
> +
> +#define zip_dbg_enter(fmt, args...) pr_info("ZIP_DBG: %s() in %s" \
> +                                   fmt "\n", __func__, FILE_NAME, ## args)
> +
> +#define zip_dbg_exit(fmt, args...) pr_info("ZIP_DBG:Exit %s() in %s" \
> +                                  fmt "\n", __func__, FILE_NAME, ## args)
> +
> +#elif DEBUG_LEVEL >= 3
> +
> +#define zip_dbg(fmt, args...) pr_info("ZIP DBG: %s: %s() : %d: " \
> +                             fmt "\n", FILE_NAME, __func__, __LINE__, ## args)
> +
> +#elif DEBUG_LEVEL >= 2
> +
> +#define zip_dbg(fmt, args...) pr_info("ZIP DBG: %s() : %d: " \
> +                             fmt "\n", __func__, __LINE__, ## args)
> +
> +#else
> +
> +#define zip_dbg(fmt, args...) pr_info("ZIP DBG:" fmt "\n", ## args)
> +
> +#endif /* DEBUG LEVEL >= */
> +
> +#if DEBUG_LEVEL <= 3
> +
> +#define zip_dbg_enter(fmt, args...)
> +#define zip_dbg_exit(fmt, args...)
> +
> +#endif /* DEBUG_LEVEL <= 3 */
> +#else
> +
> +#define zip_dbg(fmt, args...) pr_info("ZIP DBG:" fmt "\n", ## args)
> +
> +#define zip_dbg_enter(fmt, args...)
> +#define zip_dbg_exit(fmt, args...)
> +
> +#endif /* DEBUG_LEVEL */
> +#else
> +
> +#define zip_dbg(fmt, args...)
> +#define zip_dbg_enter(fmt, args...)
> +#define zip_dbg_exit(fmt, args...)
> +
> +#endif /* ZIP_DEBUG_ENABLE */

The whole zip_dbg_[enter,exit] thing can be just done with tracepoints
instead of reinventing the wheel. No reason to carry that code

> +u32 zip_load_instr(union zip_inst_s *instr,
> +                  struct zip_device *zip_dev)
> +{
> +       union zip_quex_doorbell dbell;
> +       u32 queue = 0;
> +       u32 consumed = 0;
> +       u64 *ncb_ptr = NULL;
> +       union zip_nptr_s ncp;
> +
> +       /*
> +        * Distribute the instructions between the enabled queues based on
> +        * the CPU id.
> +        */
> +       if (raw_smp_processor_id() % 2 == 0)
> +               queue = 0;
> +       else
> +               queue = 1;

Is this just simplistic load balancing scheme? There's no guarantee
that the cpu won't change later on.

> +
> +       /* Poll for the IQ cmd completion code */
> +       zip_dbg_exit();

The comment doesn't match with what actually happens?

> +static inline u64 zip_depth(void)
> +{
> +       struct zip_device *zip_dev = zip_get_device(zip_get_node_id());
> +
> +       if (!zip_dev)
> +               return -ENODEV;
> +
> +       return zip_dev->depth;
> +}
> +
> +static inline u64 zip_onfsize(void)
> +{
> +       struct zip_device *zip_dev = zip_get_device(zip_get_node_id());
> +
> +       if (!zip_dev)
> +               return -ENODEV;
> +
> +       return zip_dev->onfsize;
> +}
> +
> +static inline u64 zip_ctxsize(void)
> +{
> +       struct zip_device *zip_dev = zip_get_device(zip_get_node_id());
> +
> +       if (!zip_dev)
> +               return -ENODEV;
> +
> +       return zip_dev->ctxsize;
> +}

Where is it all used?

> +/*
> + * Allocates new ZIP device structure
> + * Returns zip_device pointer or NULL if cannot allocate memory for zip_device
> + */
> +static struct zip_device *zip_alloc_device(struct pci_dev *pdev)
> +{
> +       struct zip_device *zip = NULL;
> +       int idx = 0;
> +
> +       for (idx = 0; idx < MAX_ZIP_DEVICES; idx++) {
> +               if (!zip_dev[idx])
> +                       break;
> +       }
> +
> +       zip = kzalloc(sizeof(*zip), GFP_KERNEL);
> +
> +       if (!zip)
> +               return NULL;
> +
> +       zip_dev[idx] = zip;

If for some odd reason we try to allocate more than MAX_ZIP_DEVICES
this will deref an invalid pointer.

> +/**
> + * zip_get_device - Get ZIP device based on node id of cpu
> + *
> + * @node: Node id of the current cpu
> + * Return: Pointer to Zip device structure
> + */
> +struct zip_device *zip_get_device(int node)
> +{
> +       if ((node < MAX_ZIP_DEVICES) && (node >= 0))
> +               return zip_dev[node];
> +
> +       zip_err("ZIP device not found for node id %d\n", node);
> +       return NULL;
> +}
> +
> +/**
> + * zip_get_node_id - Get the node id of the current cpu
> + *
> + * Return: Node id of the current cpu
> + */
> +int zip_get_node_id(void)
> +{
> +       return cpu_to_node(raw_smp_processor_id());
> +}

This looks off: why are you calling raw_smp_processor_id() instead of
just smp_processor_id()?

I guess you saw warnings from smp_processor_id(), but they happen to
warn you that the cpu might change under you, making this node number
incorrect.


> +#ifndef __ZIP_MAIN_H__
> +#define __ZIP_MAIN_H__
> +
> +#include "zip_device.h"
> +#include "zip_regs.h"
> +
> +/* PCI device IDs */
> +#define PCI_DEVICE_ID_THUNDERX_ZIP   0xA01A
> +
> +/* ZIP device BARs */
> +#define PCI_CFG_ZIP_PF_BAR0   0  /* Base addr for normal regs */
> +#define PCI_CFG_ZIP_PF_BAR4   4  /* Base addr for MSI-X regs  */
> +
> +/* Maximum available zip queues */
> +#define ZIP_MAX_NUM_QUEUES    8
> +#define ZIP_MAXQ_PER_ZIPENG   4
> +#define ZIP_NUMENG_CN88XX     2
> +
> +#define ZIP_128B_ALIGN        7
> +
> +/* Buffer size and alignment */
> +#define ZIP_CMD_QBUF_SIZE     (8064 + 8)
> +#define ZIP_CMD_QBUF_ALIGN    128
> +#define ZIP_DATA_BUF_ALIGN    8
>
> +/*
> + * There will be max of 2^20 zip cmds in the zip instruction queue.
> + * So no of zip Chunk buffers = ((2^20) / ((2*1024)/64))
> + */
> +#define ZIP_MAX_CMD           (1024 * 1024)
> +#define ZIP_CMD_PER_BUF       (ZIP_CMD_QBUF_SIZE / 64)
> +#define ZIP_CMD_QBUF_MAX_CNT  (1 * 1024)
> +
> +/* Data buffer size 64K for time being */
> +#define ZIP_DATA_BUF_SIZE     (64 * 1024)
> +
> +/* Number of data buffers */
> +#define ZIP_DATA_BUF_CNT      (32 * 1024)

Bunch of these defines aren't used.

> +/**
> + * zip_cmd_qbuf_alloc - Allocates a cmd buffer for ZIP Instruction Queue
> + * @zip: Pointer to zip device structure
> + * @q:   Queue number to allocate bufffer to
> + * Return: 0 if successful, -ENOMEM otherwise
> + */
> +int zip_cmd_qbuf_alloc(struct zip_device *zip, int q)
> +{
> +       zip_dbg_enter();
> +
> +       zip->iq[q].sw_head = (u64 *)__get_free_pages((GFP_KERNEL | GFP_DMA),
> +                                               get_order(ZIP_CMD_QBUF_SIZE));
> +
> +       if (!zip->iq[q].sw_head)
> +               return -ENOMEM;
> +
> +       memset(zip->iq[q].sw_head, 0, ZIP_CMD_QBUF_SIZE);
> +
> +       zip_dbg("cmd_qbuf_alloc[%d] Success : %p\n", q, zip->iq[q].sw_head);
> +       zip_dbg_exit();
> +       return 0;
> +}
> +
> +/**
> + * zip_cmd_qbuf_free - Frees the cmd Queue buffer
> + * @zip: Pointer to zip device structure
> + * @q:   Queue number to free buffer of
> + */
> +void zip_cmd_qbuf_free(struct zip_device *zip, int q)
> +{
> +       zip_dbg("Freeing cmd_qbuf 0x%lx\n", zip->iq[q].sw_tail);
> +
> +       free_pages((u64)zip->iq[q].sw_tail, get_order(ZIP_CMD_QBUF_SIZE));
> +}

Why are you going through __get_free_pages() for constant size
allocations? kmemcache would be better here.

> +/**
> + * zip_data_buf_alloc - Allocates memory for a data bufffer
> + * @size:   Size of the buffer to allocate
> + * Returns: Pointer to the buffer allocated
> + */
> +u8 *zip_data_buf_alloc(u64 size)
> +{
> +       u8 *ptr;
> +
> +       zip_dbg_enter();
> +
> +       ptr = (u8 *)__get_free_pages((GFP_ATOMIC | GFP_DMA),
> +                                       get_order(size));
> +
> +       if (!ptr)
> +               return NULL;
> +
> +       memset(ptr, 0, size);
> +
> +       zip_dbg("Data buffer allocation success\n");
> +       zip_dbg_exit();
> +       return ptr;
> +}
> +
> +/**
> + * zip_data_buf_free - Frees the memory of a data buffer
> + * @ptr:  Pointer to the buffer
> + * @size: Buffer size
> + */
> +void zip_data_buf_free(u8 *ptr, u64 size)
> +{
> +       zip_dbg("Freeing data buffer 0x%lx\n", ptr);
> +
> +       free_pages((u64)ptr, get_order(size));
> +}

Same question here; you end up allocating constant sizes - why not kmemcache?

Also, this is quite a lot to allocate out of the atomic pool, is there
a reason for that?


Thanks,
Sasha

[toc] | [prev] | [next] | [standalone]


#1547531

FromHerbert Xu <herbert@gondor.apana.org.au>
Date2016-12-27 10:40 +0100
Message-ID<sSWoF-ha-1@gated-at.bofh.it>
In reply to#1540369
Hi Jan:

On Mon, Dec 12, 2016 at 04:04:36PM +0100, Jan Glauber wrote:
> 
> this series adds support for hardware accelerated compression & decompression
> as found on ThunderX (arm64) SOCs. I've been reviewing this driver internally
> for some time and would like to get feedback on the RFC to see if this goes
> into the right direction and to see if there are any concerns.
> 
> We've discussed switching to the new acomp algorithm but for the time being
> decided against acomp because our test cases are not yet supported with it.

OK.  Do you see any major problems in converting this over to acomp?

Thanks,
-- 
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

[toc] | [prev] | [next] | [standalone]


#1547590

From"Challa, Mahipal" <Mahipal.Challa@cavium.com>
Date2016-12-27 14:20 +0100
Message-ID<sSZPA-2xe-7@gated-at.bofh.it>
In reply to#1547531
Hi Herbert,

Thanks for your response

>Hi Jan:

On Mon, Dec 12, 2016 at 04:04:36PM +0100, Jan Glauber wrote:
> >
> >this series adds support for hardware accelerated compression & decompression
> >as found on ThunderX (arm64) SOCs. I've been reviewing this driver internally
> >for some time and would like to get feedback on the RFC to see if this goes
> >into the right direction and to see if there are any concerns.
> >
> >We've discussed switching to the new acomp algorithm but for the time being
> >decided against acomp because our test cases are not yet supported with it.

> OK.  Do you see any major problems in converting this over to acomp?
Mahipal: One major issue is, the kernel use cases to validate the Cavium ThunderX ZIP driver are "ZSWAP" and "IPComp" which are not yet supported with scomp-acomp framework. 

Regards,
-Mahipal



From: Herbert Xu <herbert@gondor.apana.org.au>
Sent: Tuesday, December 27, 2016 2:32 PM
To: Jan Glauber
Cc: linux-crypto@vger.kernel.org; linux-kernel@vger.kernel.org; David S . Miller; Challa, Mahipal; Nair, Vishnu
Subject: Re: [RFC PATCH 0/3] Cavium ThunderX ZIP driver
    
Hi Jan:

On Mon, Dec 12, 2016 at 04:04:36PM +0100, Jan Glauber wrote:
> 
> this series adds support for hardware accelerated compression & decompression
> as found on ThunderX (arm64) SOCs. I've been reviewing this driver internally
> for some time and would like to get feedback on the RFC to see if this goes
> into the right direction and to see if there are any concerns.
> 
> We've discussed switching to the new acomp algorithm but for the time being
> decided against acomp because our test cases are not yet supported with it.

OK.  Do you see any major problems in converting this over to acomp?

Thanks,
-- 
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
    

[toc] | [prev] | [next] | [standalone]


#1547890

FromHerbert Xu <herbert@gondor.apana.org.au>
Date2016-12-28 10:10 +0100
Message-ID<sTipc-67E-7@gated-at.bofh.it>
In reply to#1547590
On Tue, Dec 27, 2016 at 11:39:21AM +0000, Challa, Mahipal wrote:
>
> Mahipal: One major issue is, the kernel use cases to validate the Cavium ThunderX ZIP driver are "ZSWAP" and "IPComp" which are not yet supported with scomp-acomp framework. 

OK I'll look into converting IPcomp.

Thanks,
-- 
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

[toc] | [prev] | [standalone]


Back to top | Article view | linux.kernel


csiph-web