Re: [PATCH 1/3] drivers: crypto: Add Support for Octeon-tx CPT Engine
From: George Cherian
Date: Fri Nov 18 2016 - 15:03:28 EST
Hi David,
Thanks for the review.
On Saturday 19 November 2016 12:25 AM, David Daney wrote:
On 11/18/2016 07:00 AM, gcherianv@xxxxxxxxx wrote:
From: George Cherian <george.cherian@xxxxxxxxxx>
Enable the Physical Function diver for the Cavium Crypto Engine (CPT)
found in Octeon-tx series of SoC's. CPT is the Cryptographic
Acceleration
Unit. CPT includes microcoded GigaCypher symmetric engines (SEs) and
asymmetric engines (AEs).
Signed-off-by: George Cherian <george.cherian@xxxxxxxxxx>
How was this tested?
Using ecryptfs and dm-crypt.
---
drivers/crypto/cavium/cpt/Kconfig | 22 +
drivers/crypto/cavium/cpt/Makefile | 2 +
drivers/crypto/cavium/cpt/cpt.h | 90 +++
drivers/crypto/cavium/cpt/cpt_common.h | 377 +++++++++++++
drivers/crypto/cavium/cpt/cpt_hw_types.h | 940
+++++++++++++++++++++++++++++++
drivers/crypto/cavium/cpt/cpt_main.c | 891
+++++++++++++++++++++++++++++
drivers/crypto/cavium/cpt/cpt_pf_mbox.c | 174 ++++++
7 files changed, 2496 insertions(+)
create mode 100644 drivers/crypto/cavium/cpt/Kconfig
create mode 100644 drivers/crypto/cavium/cpt/Makefile
create mode 100644 drivers/crypto/cavium/cpt/cpt.h
create mode 100644 drivers/crypto/cavium/cpt/cpt_common.h
create mode 100644 drivers/crypto/cavium/cpt/cpt_hw_types.h
create mode 100644 drivers/crypto/cavium/cpt/cpt_main.c
create mode 100644 drivers/crypto/cavium/cpt/cpt_pf_mbox.c
diff --git a/drivers/crypto/cavium/cpt/Kconfig
b/drivers/crypto/cavium/cpt/Kconfig
new file mode 100644
index 0000000..8fe3f44
--- /dev/null
+++ b/drivers/crypto/cavium/cpt/Kconfig
@@ -0,0 +1,22 @@
+#
+# Cavium crypto device configuration
+#
+
+config CRYPTO_DEV_CPT
+ tristate
+ select HW_RANDOM_OCTEON
This makes no sense. HW_RANDOM_OCTEON is for a mips64 based SOC and
isn't present on devices that have this crypto block. Why select this?
Yeah true... I actually wanted to this one instead
|CONFIG_HW_RANDOM_CAVIUM|
+ select CRYPTO_AES
+ select CRYPTO_DES
+ select CRYPTO_BLKCIPHER
+ select FW_LOADER
+
+config OCTEONTX_CPT_PF
+ tristate "Octeon-tx CPT Physical function driver"
+ depends on ARCH_THUNDER
+ select CRYPTO_DEV_CPT
+ help
+ Support for Cavium CPT block found in octeon-tx series of
+ processors.
+
+ To compile this as a module, choose M here: the module will be
+ called cptpf.
diff --git a/drivers/crypto/cavium/cpt/Makefile
b/drivers/crypto/cavium/cpt/Makefile
new file mode 100644
index 0000000..bf758e2
--- /dev/null
+++ b/drivers/crypto/cavium/cpt/Makefile
@@ -0,0 +1,2 @@
+obj-$(CONFIG_OCTEONTX_CPT_PF) += cptpf.o
+cptpf-objs := cpt_main.o cpt_pf_mbox.o
diff --git a/drivers/crypto/cavium/cpt/cpt.h
b/drivers/crypto/cavium/cpt/cpt.h
new file mode 100644
index 0000000..63d12da
--- /dev/null
+++ b/drivers/crypto/cavium/cpt/cpt.h
@@ -0,0 +1,90 @@
+/*
+ * Copyright (C) 2016 Cavium, Inc.
+ *
+ * This program is free software; you can redistribute it and/or
modify it
+ * under the terms of version 2 of the GNU General Public License
+ * as published by the Free Software Foundation.
+ */
+
+#ifndef __CPT_H
+#define __CPT_H
+
+#include "cpt_common.h"
+
+#define BASE_PROC_DIR "cavium"
+
+#define PF 0
+#define VF 1
+
+struct cpt_device;
+
+struct microcode {
+ uint8_t is_mc_valid;
s/uint8_t/u8/ ??
That could be done everywhere.
will do
[...]
diff --git a/drivers/crypto/cavium/cpt/cpt_common.h
b/drivers/crypto/cavium/cpt/cpt_common.h
new file mode 100644
index 0000000..351ed4a
--- /dev/null
+++ b/drivers/crypto/cavium/cpt/cpt_common.h
@@ -0,0 +1,377 @@
+/*
+ * Copyright (C) 2016 Cavium, Inc.
+ *
+ * This program is free software; you can redistribute it and/or
modify it
+ * under the terms of version 2 of the GNU General Public License
+ * as published by the Free Software Foundation.
+ */
+
+#ifndef __CPT_COMMON_H
+#define __CPT_COMMON_H
+
+#include <asm/byteorder.h>
+#include <linux/uaccess.h>
+#include <linux/types.h>
+#include <linux/spinlock.h>
+#include <linux/pci.h>
+#include <linux/cpumask.h>
+#include <linux/string.h>
+#include <linux/pci_regs.h>
+#include <linux/delay.h>
+#include <linux/printk.h>
+#include <linux/sched.h>
+#include <linux/completion.h>
+#include <asm/arch_timer.h>
+#include <linux/types.h>
+
+#include "cpt_hw_types.h"
+
+/* configuration space offsets */
+#ifndef PCI_VENDOR_ID
+#define PCI_VENDOR_ID 0x00 /* 16 bits */
+#endif
+#ifndef PCI_DEVICE_ID
+#define PCI_DEVICE_ID 0x02 /* 16 bits */
+#endif
+#ifndef PCI_REVISION_ID
+#define PCI_REVISION_ID 0x08 /* Revision ID */
+#endif
+#ifndef PCI_CAPABILITY_LIST
+#define PCI_CAPABILITY_LIST 0x34 /* first capability list entry */
+#endif
+
Standard PCI core functions give you access to all that information,
use pdev->device, pdev->revision, etc. instead of reinventing the
wheel here with all these #defines.
+/* Device ID */
+#define PCI_VENDOR_ID_CAVIUM 0x177d
This is defined in pci_ids.h, use value from there instead of placing
a duplicate definition here.
okay will remove them
+#define CPT_81XX_PCI_PF_DEVICE_ID 0xa040
+#define CPT_81XX_PCI_VF_DEVICE_ID 0xa041
+
+#define PASS_1_0 0x0
+
+/* CPT Models ((Device ID<<16)|Revision ID) */
+/* CPT models */
+#define CPT_81XX_PASS1_0 ((CPT_81XX_PCI_PF_DEVICE_ID << 8) | PASS_1_0)
+#define CPTVF_81XX_PASS1_0 ((CPT_81XX_PCI_VF_DEVICE_ID << 8) |
PASS_1_0)
+
+#define PF 0
+#define VF 1
+
+#define DEFAULT_DEVICE_QUEUES CPT_NUM_QS_PER_VF
+
+#define SUCCESS (0)
+#define FAIL (1)
+
+#ifndef ROUNDUP4
+#define ROUNDUP4(val) (((val) + 3) & 0xfffffffc)
+#endif
+
+#ifndef ROUNDUP8
+#define ROUNDUP8(val) (((val) + 7) & 0xfffffff8)
+#endif
+
+#ifndef ROUNDUP16
+#define ROUNDUP16(val) (((val) + 15) & 0xfffffff0)
+#endif
+
kernel.h has round_up(), use that instead of defining all these.
+#define ERR_ADDR_LEN 8
+
What is that for? It looks unused.
[...]
+/*###### PCIE EP-Mode Configuration Registers #########*/
+#define PCIEEP0_CFG000 (0x0)
+#define PCIEEP0_CFG002 (0x8)
+#define PCIEEP0_CFG011 (0x2C)
+#define PCIEEP0_CFG020 (0x50)
+#define PCIEEP0_CFG025 (0x64)
+#define PCIEEP0_CFG030 (0x78)
+#define PCIEEP0_CFG044 (0xB0)
+#define PCIEEP0_CFG045 (0xB4)
+#define PCIEEP0_CFG082 (0x148)
+#define PCIEEP0_CFG095 (0x17C)
+#define PCIEEP0_CFG096 (0x180)
+#define PCIEEP0_CFG097 (0x184)
+#define PCIEEP0_CFG103 (0x19C)
+#define PCIEEP0_CFG460 (0x730)
+#define PCIEEP0_CFG461 (0x734)
+#define PCIEEP0_CFG462 (0x738)
+
+/*####### PCIe EP-Mode SR-IOV Configuration Registers #####*/
+#define PCIEEPVF0_CFG000 (0x0)
+#define PCIEEPVF0_CFG002 (0x8)
+#define PCIEEPVF0_CFG011 (0x2C)
+#define PCIEEPVF0_CFG030 (0x78)
+#define PCIEEPVF0_CFG044 (0xB0)
+
Where are all those defines used? What are they for?
That's all I can look at for now.
I will address your comments in next version.
David.