2020-08-17 13:34:37

by Richard Gong

[permalink] [raw]
Subject: [PATCHv2 0/2] add Intel SoCFPGA crypto service driver

From: Richard Gong <[email protected]>

This is the 2nd submission of Intel SoCFPGA crypto service driver.

I followed the process to register or request a valid IOCTL number/letter,
but I got the delivery failure status notification.

Cypto service driver and service layer driver patches have been reviewed
internally by colleagues at Intel.

Intel SoCFPGA is composed of a 64 bit quad-core ARM Cortex A53 hard
processor system (HPS) and Secure Device Manager (SDM). SDM is the
hardware which does the FPGA configuration, QSPI, remote system update,
crypto and warm reset.

To meet the whole system security needs and support virtual machine
requesting communication with SDM, only the secure world of software (EL3,
Exception Level 3) can interface with SDM. All software entities running
on other exception levels must channel through the EL3 software whenever
it needs service from SDM.

Intel Stratix10 service layer driver is added to provide the service for
FPGA configuration, Remote System Update and FPGA crypto service (FCS).
Running at privileged exception level (EL1, Exception Level 1), Intel
Stratix10 service layer driver interfaces with the service clients at EL1
and manages secure monitor call (SMC) to communicate with secure monitor
software at secure monitor exception level (EL3).

The crypto services include security certificate, image boot validation,
security key cancellation, get provision data, random number generation,
advance encryption standard (AES) encryption and decryption services.

To perform supporting crypto features on Intel SoCFPGA platforms, Linux
user-space application interacts with FPGA crypto service (FCS) driver via
structures defined in include/uapi/linux/intel_fcs-ioctl.h.

The application allocates spaces for IOCTL structure to hold the contents
or points to the data that FCS driver needs, uses IOCTL calls to passes
data to kernel FCS driver for processing at low level firmware and get
processed data or status back form the low level firmware via FCS driver.

The user-space application named as fcs_client is at
https://github.com/altera-opensource/fcs_apps/tree/fcs_client.

Richard Gong (2):
firmware: stratix10-svc: extend svc to support new crypto features
misc: add Intel SoCFPGA crypto service driver

drivers/firmware/stratix10-svc.c | 178 +++++-
drivers/misc/Kconfig | 12 +
drivers/misc/Makefile | 1 +
drivers/misc/intel-fcs.c | 709 +++++++++++++++++++++
include/linux/firmware/intel/stratix10-smc.h | 147 ++++-
.../linux/firmware/intel/stratix10-svc-client.h | 42 ++
include/uapi/linux/intel-fcs_ioctl.h | 222 +++++++
7 files changed, 1292 insertions(+), 19 deletions(-)
create mode 100644 drivers/misc/intel-fcs.c
create mode 100644 include/uapi/linux/intel-fcs_ioctl.h

--
2.7.4


2020-08-17 13:38:33

by Richard Gong

[permalink] [raw]
Subject: [PATCHv2 2/2] misc: add Intel SoCFPGA crypto service driver

From: Richard Gong <[email protected]>

Add Intel FPGA crypto service (FCS) driver to support new crypto services
on Intel SoCFPGA platforms.

The crypto services include security certificate, image boot validation,
security key cancellation, get provision data, random number generation,
advance encrtption standard (AES) encryption and decryption services.

To perform supporting crypto features on Intel SoCFPGA platforms, Linux
user-space application interacts with FPGA crypto service (FCS) driver via
structures defined in include/uapi/linux/intel_fcs-ioctl.h.

The application allocates spaces for IOCTL structure to hold the contents
or points to the data that FCS driver needs, uses IOCTL calls to passes
data to kernel FCS driver for processing at low level firmware and get
processed data or status back form the low level firmware via FCS driver.

The user-space application named as fcs_client is at
https://github.com/altera-opensource/fcs_apps/tree/fcs_client.

Signed-off-by: Richard Gong <[email protected]>
---
v2: move driver to driver/misc from driver/crypto
---
drivers/misc/Kconfig | 12 +
drivers/misc/Makefile | 1 +
drivers/misc/intel-fcs.c | 709 +++++++++++++++++++++++++++++++++++
include/uapi/linux/intel-fcs_ioctl.h | 222 +++++++++++
4 files changed, 944 insertions(+)
create mode 100644 drivers/misc/intel-fcs.c
create mode 100644 include/uapi/linux/intel-fcs_ioctl.h

diff --git a/drivers/misc/Kconfig b/drivers/misc/Kconfig
index ce136d6..9b659de 100644
--- a/drivers/misc/Kconfig
+++ b/drivers/misc/Kconfig
@@ -112,6 +112,18 @@ config PHANTOM
If you choose to build module, its name will be phantom. If unsure,
say N here.

+config INTEL_FCS
+ tristate "Intel FPGA Crypto Service support"
+ depends on INTEL_STRATIX10_SERVICE
+ help
+ Support crypto services on Intel SoCFPGA platforms. The crypto
+ services include security certificate, image boot validation,
+ security key cancellation, get provision data, random number
+ advance encrtption standard (AES) encryption and decryption
+ services.
+
+ Say Y here if you want Intel FCS support
+
config INTEL_MID_PTI
tristate "Parallel Trace Interface for MIPI P1149.7 cJTAG standard"
depends on PCI && TTY && (X86_INTEL_MID || COMPILE_TEST)
diff --git a/drivers/misc/Makefile b/drivers/misc/Makefile
index c7bd01a..3238100 100644
--- a/drivers/misc/Makefile
+++ b/drivers/misc/Makefile
@@ -41,6 +41,7 @@ obj-$(CONFIG_PCH_PHUB) += pch_phub.o
obj-y += ti-st/
obj-y += lis3lv02d/
obj-$(CONFIG_ALTERA_STAPL) +=altera-stapl/
+obj-$(CONFIG_INTEL_FCS) +=intel-fcs.o
obj-$(CONFIG_INTEL_MEI) += mei/
obj-$(CONFIG_VMWARE_VMCI) += vmw_vmci/
obj-$(CONFIG_LATTICE_ECP3_CONFIG) += lattice-ecp3-config.o
diff --git a/drivers/misc/intel-fcs.c b/drivers/misc/intel-fcs.c
new file mode 100644
index 00000000..9c84094
--- /dev/null
+++ b/drivers/misc/intel-fcs.c
@@ -0,0 +1,709 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (C) 2020, Intel Corporation
+ */
+
+#include <linux/arm-smccc.h>
+#include <linux/bitfield.h>
+#include <linux/completion.h>
+#include <linux/firmware.h>
+#include <linux/fs.h>
+#include <linux/kobject.h>
+#include <linux/miscdevice.h>
+#include <linux/module.h>
+#include <linux/mutex.h>
+#include <linux/of.h>
+#include <linux/of_platform.h>
+#include <linux/platform_device.h>
+#include <linux/firmware/intel/stratix10-svc-client.h>
+#include <linux/string.h>
+#include <linux/slab.h>
+#include <linux/sysfs.h>
+#include <linux/uaccess.h>
+
+#include <uapi/linux/intel-fcs_ioctl.h>
+
+#define RANDOM_NUMBER_SIZE 32
+#define FILE_NAME_SIZE 32
+#define PS_BUF_SIZE 64
+#define INVALID_STATUS 0xff
+
+#define MIN_SDOS_BUF_SZ 16
+#define MAX_SDOS_BUF_SZ 32768
+
+#define FCS_REQUEST_TIMEOUT (msecs_to_jiffies(SVC_FCS_REQUEST_TIMEOUT_MS))
+#define FCS_COMPLETED_TIMEOUT (msecs_to_jiffies(SVC_COMPLETED_TIMEOUT_MS))
+
+typedef void (*fcs_callback)(struct stratix10_svc_client *client,
+ struct stratix10_svc_cb_data *data);
+
+struct intel_fcs_priv {
+ struct stratix10_svc_chan *chan;
+ struct stratix10_svc_client client;
+ struct completion completion;
+ struct mutex lock;
+ struct miscdevice miscdev;
+ unsigned int status;
+ void *kbuf;
+ unsigned int size;
+};
+
+static void fcs_data_callback(struct stratix10_svc_client *client,
+ struct stratix10_svc_cb_data *data)
+{
+ struct intel_fcs_priv *priv = client->priv;
+
+ if ((data->status == BIT(SVC_STATUS_OK)) ||
+ (data->status == BIT(SVC_STATUS_COMPLETED))) {
+ priv->status = 0;
+ priv->kbuf = data->kaddr2;
+ priv->size = *((unsigned int *)data->kaddr3);
+ } else if (data->status == BIT(SVC_STATUS_ERROR)) {
+ priv->status = *((unsigned int *)data->kaddr1);
+ dev_err(client->dev, "error, mbox_error=0x%x\n", priv->status);
+ priv->kbuf = data->kaddr2;
+ priv->size = (data->kaddr3) ?
+ *((unsigned int *)data->kaddr3) : 0;
+ } else {
+ dev_err(client->dev, "rejected\n");
+ priv->status = -EINVAL;
+ priv->kbuf = NULL;
+ priv->size = 0;
+ }
+
+ complete(&priv->completion);
+}
+
+static void fcs_vab_callback(struct stratix10_svc_client *client,
+ struct stratix10_svc_cb_data *data)
+{
+ struct intel_fcs_priv *priv = client->priv;
+
+ priv->status = 0;
+
+ if (data->status == BIT(SVC_STATUS_INVALID_PARAM)) {
+ priv->status = -EINVAL;
+ dev_warn(client->dev, "rejected, invalid param\n");
+ } else if (data->status == BIT(SVC_STATUS_ERROR)) {
+ priv->status = *((unsigned int *)data->kaddr1);
+ dev_err(client->dev, "mbox_error=0x%x\n", priv->status);
+ } else if (data->status == BIT(SVC_STATUS_BUSY)) {
+ priv->status = -ETIMEDOUT;
+ dev_err(client->dev, "timeout to get completed status\n");
+ }
+
+ complete(&priv->completion);
+}
+
+static int fcs_request_service(struct intel_fcs_priv *priv,
+ void *msg, unsigned long timeout)
+{
+ struct stratix10_svc_client_msg *p_msg =
+ (struct stratix10_svc_client_msg *)msg;
+ int ret;
+
+ mutex_lock(&priv->lock);
+ reinit_completion(&priv->completion);
+
+ ret = stratix10_svc_send(priv->chan, p_msg);
+ if (ret) {
+ ret = -EINVAL;
+ goto unlock;
+ }
+
+ ret = wait_for_completion_timeout(&priv->completion,
+ timeout);
+ if (!ret) {
+ dev_err(priv->client.dev,
+ "timeout waiting for SMC call\n");
+ ret = -ETIMEDOUT;
+ } else
+ ret = 0;
+
+unlock:
+ mutex_unlock(&priv->lock);
+ return ret;
+}
+
+static void fcs_close_services(struct intel_fcs_priv *priv,
+ void *sbuf, void *dbuf)
+{
+ if (sbuf)
+ stratix10_svc_free_memory(priv->chan, sbuf);
+
+ if (dbuf)
+ stratix10_svc_free_memory(priv->chan, dbuf);
+
+ stratix10_svc_done(priv->chan);
+}
+
+static long fcs_ioctl(struct file *file, unsigned int cmd,
+ unsigned long arg)
+{
+ struct intel_fcs_dev_ioctl *data;
+ struct intel_fcs_priv *priv;
+ struct device *dev;
+ struct stratix10_svc_client_msg *msg;
+ const struct firmware *fw;
+ char filename[FILE_NAME_SIZE];
+ size_t tsz, datasz;
+ void *s_buf;
+ void *d_buf;
+ void *ps_buf;
+ unsigned int buf_sz;
+ int ret = 0;
+ int i;
+
+ priv = container_of(file->private_data, struct intel_fcs_priv, miscdev);
+ dev = priv->client.dev;
+
+ data = devm_kzalloc(dev, sizeof(*data), GFP_KERNEL);
+ if (!data)
+ return -ENOMEM;
+
+ msg = devm_kzalloc(dev, sizeof(*msg), GFP_KERNEL);
+ if (!msg)
+ return -ENOMEM;
+
+ switch (cmd) {
+ case INTEL_FCS_DEV_VALIDATION_REQUEST:
+ if (copy_from_user(data, (void __user *)arg, sizeof(*data))) {
+ dev_err(dev, "failure on copy_from_user\n");
+ return -EFAULT;
+ }
+
+ /* for bitstream */
+ dev_dbg(dev, "file_name=%s, status=%d\n",
+ (char *)data->com_paras.s_request.src, data->status);
+ scnprintf(filename, FILE_NAME_SIZE, "%s",
+ (char *)data->com_paras.s_request.src);
+ ret = request_firmware(&fw, filename, priv->client.dev);
+ if (ret) {
+ dev_err(dev, "error requesting firmware %s\n",
+ (char *)data->com_paras.s_request.src);
+ return -EFAULT;
+ }
+
+ dev_dbg(dev, "FW size=%ld\n", fw->size);
+ s_buf = stratix10_svc_allocate_memory(priv->chan, fw->size);
+ if (!s_buf) {
+ dev_err(dev, "failed to allocate VAB buffer\n");
+ release_firmware(fw);
+ return -ENOMEM;
+ }
+
+ memcpy(s_buf, fw->data, fw->size);
+
+ msg->payload_length = fw->size;
+ release_firmware(fw);
+
+ msg->command = COMMAND_FCS_REQUEST_SERVICE;
+ msg->payload = s_buf;
+ priv->client.receive_cb = fcs_vab_callback;
+
+ ret = fcs_request_service(priv, (void *)msg,
+ FCS_REQUEST_TIMEOUT);
+ dev_dbg(dev, "fcs_request_service ret=%d\n", ret);
+ if (!ret && !priv->status) {
+ /* to query the complete status */
+ msg->command = COMMAND_POLL_SERVICE_STATUS;
+ priv->client.receive_cb = fcs_data_callback;
+ ret = fcs_request_service(priv, (void *)msg,
+ FCS_COMPLETED_TIMEOUT);
+ dev_dbg(dev, "fcs_request_service ret=%d\n", ret);
+ if (!ret && !priv->status)
+ data->status = 0;
+ else
+ data->status = priv->status;
+ } else
+ data->status = priv->status;
+
+ if (copy_to_user((void __user *)arg, data, sizeof(*data))) {
+ dev_err(dev, "failure on copy_to_user\n");
+ fcs_close_services(priv, s_buf, NULL);
+ ret = -EFAULT;
+ }
+
+ fcs_close_services(priv, s_buf, NULL);
+ break;
+
+ case INTEL_FCS_DEV_SEND_CERTIFICATE:
+ if (copy_from_user(data, (void __user *)arg, sizeof(*data))) {
+ dev_err(dev, "failure on copy_from_user\n");
+ return -EFAULT;
+ }
+
+ dev_dbg(dev, "Test=%d, Size=%d; Address=0x%p\n",
+ data->com_paras.c_request.test.test_bit,
+ data->com_paras.c_request.size,
+ data->com_paras.c_request.addr);
+
+ /* Allocate memory for certificate + test word */
+ tsz = sizeof(struct intel_fcs_cert_test_word);
+ datasz = data->com_paras.s_request.size + tsz;
+
+ s_buf = stratix10_svc_allocate_memory(priv->chan, datasz);
+ if (!s_buf) {
+ dev_err(dev, "failed to allocate VAB buffer\n");
+ return -ENOMEM;
+ }
+
+ ps_buf = stratix10_svc_allocate_memory(priv->chan, PS_BUF_SIZE);
+ if (!ps_buf) {
+ dev_err(dev, "failed to allocate p-status buf\n");
+ stratix10_svc_free_memory(priv->chan, s_buf);
+ return -ENOMEM;
+ }
+
+ /* Copy the test word */
+ memcpy(s_buf, &data->com_paras.c_request.test, tsz);
+
+ /* Copy in the certificate data (skipping over the test word) */
+ ret = copy_from_user(s_buf + tsz,
+ data->com_paras.c_request.addr,
+ data->com_paras.s_request.size);
+ if (ret) {
+ dev_err(dev, "failed copy buf ret=%d\n", ret);
+ fcs_close_services(priv, s_buf, ps_buf);
+ return -EFAULT;
+ }
+
+ msg->payload_length = datasz;
+ msg->command = COMMAND_FCS_SEND_CERTIFICATE;
+ msg->payload = s_buf;
+ priv->client.receive_cb = fcs_vab_callback;
+
+ ret = fcs_request_service(priv, (void *)msg,
+ FCS_REQUEST_TIMEOUT);
+ dev_dbg(dev, "fcs_request_service ret=%d\n", ret);
+ if (!ret && !priv->status) {
+ /* to query the complete status */
+ msg->payload = ps_buf;
+ msg->payload_length = PS_BUF_SIZE;
+ msg->command = COMMAND_POLL_SERVICE_STATUS;
+ priv->client.receive_cb = fcs_data_callback;
+ ret = fcs_request_service(priv, (void *)msg,
+ FCS_COMPLETED_TIMEOUT);
+ dev_dbg(dev, "request service ret=%d\n", ret);
+ if (!ret && !priv->status)
+ data->status = 0;
+ else {
+ if (priv->kbuf)
+ data->com_paras.c_request.c_status =
+ (*(u32 *)priv->kbuf);
+ else
+ data->com_paras.c_request.c_status =
+ INVALID_STATUS;
+ }
+ } else
+ data->status = priv->status;
+
+ if (copy_to_user((void __user *)arg, data, sizeof(*data))) {
+ dev_err(dev, "failure on copy_to_user\n");
+ fcs_close_services(priv, s_buf, NULL);
+ ret = -EFAULT;
+ }
+
+ fcs_close_services(priv, s_buf, ps_buf);
+ break;
+
+ case INTEL_FCS_DEV_RANDOM_NUMBER_GEN:
+ if (copy_from_user(data, (void __user *)arg, sizeof(*data))) {
+ dev_err(dev, "failure on copy_from_user\n");
+ return -EFAULT;
+ }
+
+ s_buf = stratix10_svc_allocate_memory(priv->chan,
+ RANDOM_NUMBER_SIZE);
+ if (!s_buf) {
+ dev_err(dev, "failed to allocate RNG buffer\n");
+ return -ENOMEM;
+ }
+
+ msg->command = COMMAND_FCS_RANDOM_NUMBER_GEN;
+ msg->payload = s_buf;
+ msg->payload_length = RANDOM_NUMBER_SIZE;
+ priv->client.receive_cb = fcs_data_callback;
+
+ ret = fcs_request_service(priv, (void *)msg,
+ FCS_REQUEST_TIMEOUT);
+
+ if (!ret && !priv->status) {
+ if (!priv->kbuf) {
+ dev_err(dev, "failure on kbuf\n");
+ fcs_close_services(priv, s_buf, NULL);
+ return -EFAULT;
+ }
+
+ for (i = 0; i < 8; i++)
+ dev_dbg(dev, "output_data[%d]=%d\n", i,
+ *((int *)priv->kbuf + i));
+
+ for (i = 0; i < 8; i++)
+ data->com_paras.rn_gen.rndm[i] =
+ *((int *)priv->kbuf + i);
+ data->status = priv->status;
+
+ } else {
+ /* failed to get RNG */
+ data->status = priv->status;
+ }
+
+ if (copy_to_user((void __user *)arg, data, sizeof(*data))) {
+ dev_err(dev, "failure on copy_to_user\n");
+ fcs_close_services(priv, s_buf, NULL);
+ ret = -EFAULT;
+ }
+
+ fcs_close_services(priv, s_buf, NULL);
+ break;
+ case INTEL_FCS_DEV_GET_PROVISION_DATA:
+ if (copy_from_user(data, (void __user *)arg,
+ sizeof(*data))) {
+ dev_err(dev, "failure on copy_from_user\n");
+ return -EFAULT;
+ }
+
+ s_buf = stratix10_svc_allocate_memory(priv->chan,
+ data->com_paras.gp_data.size);
+ if (!s_buf) {
+ dev_err(dev, "failed allocate provision buffer\n");
+ return -ENOMEM;
+ }
+
+ msg->command = COMMAND_FCS_GET_PROVISION_DATA;
+ msg->payload = s_buf;
+ msg->payload_length = data->com_paras.gp_data.size;
+ priv->client.receive_cb = fcs_data_callback;
+
+ ret = fcs_request_service(priv, (void *)msg,
+ FCS_REQUEST_TIMEOUT);
+ if (!ret && !priv->status) {
+ if (!priv->kbuf) {
+ dev_err(dev, "failure on kbuf\n");
+ fcs_close_services(priv, s_buf, NULL);
+ return -EFAULT;
+ }
+ data->com_paras.gp_data.size = priv->size;
+ memcpy(data->com_paras.gp_data.addr, priv->kbuf,
+ priv->size);
+ data->status = 0;
+ } else {
+ data->com_paras.gp_data.addr = NULL;
+ data->com_paras.gp_data.size = 0;
+ data->status = priv->status;
+ }
+
+ if (copy_to_user((void __user *)arg, data, sizeof(*data))) {
+ dev_err(dev, "failure on copy_to_user\n");
+ fcs_close_services(priv, s_buf, NULL);
+ return -EFAULT;
+ }
+
+ fcs_close_services(priv, s_buf, NULL);
+ break;
+ case INTEL_FCS_DEV_DATA_ENCRYPTION:
+ if (copy_from_user(data, (void __user *)arg, sizeof(*data))) {
+ dev_err(dev, "failure on copy_from_user\n");
+ return -EFAULT;
+ }
+
+ if ((data->com_paras.d_encryption.src_size < MIN_SDOS_BUF_SZ) ||
+ (data->com_paras.d_encryption.src_size > MAX_SDOS_BUF_SZ)) {
+ dev_err(dev, "Invalid SDOS Buffer src size:%d\n",
+ data->com_paras.d_encryption.src_size);
+ return -EFAULT;
+ }
+
+ if ((data->com_paras.d_encryption.dst_size < MIN_SDOS_BUF_SZ) ||
+ (data->com_paras.d_encryption.dst_size > MAX_SDOS_BUF_SZ)) {
+ dev_err(dev, "Invalid SDOS Buffer dst size:%d\n",
+ data->com_paras.d_encryption.dst_size);
+ return -EFAULT;
+ }
+
+ /* allocate buffer for both source and destination */
+ s_buf = stratix10_svc_allocate_memory(priv->chan,
+ MAX_SDOS_BUF_SZ);
+ if (!s_buf) {
+ dev_err(dev, "failed allocate encrypt src buf\n");
+ return -ENOMEM;
+ }
+ d_buf = stratix10_svc_allocate_memory(priv->chan,
+ MAX_SDOS_BUF_SZ);
+ if (!d_buf) {
+ dev_err(dev, "failed allocate encrypt dst buf\n");
+ stratix10_svc_free_memory(priv->chan, s_buf);
+ return -ENOMEM;
+ }
+ ps_buf = stratix10_svc_allocate_memory(priv->chan, PS_BUF_SIZE);
+ if (!ps_buf) {
+ dev_err(dev, "failed allocate p-status buffer\n");
+ fcs_close_services(priv, s_buf, d_buf);
+ return -ENOMEM;
+ }
+ ret = copy_from_user(s_buf,
+ data->com_paras.d_encryption.src,
+ data->com_paras.d_encryption.src_size);
+ if (ret) {
+ dev_err(dev, "failure on copy_from_user\n");
+ fcs_close_services(priv, ps_buf, NULL);
+ fcs_close_services(priv, s_buf, d_buf);
+ return -ENOMEM;
+ }
+
+ msg->command = COMMAND_FCS_DATA_ENCRYPTION;
+ msg->payload = s_buf;
+ msg->payload_length =
+ data->com_paras.d_encryption.src_size;
+ msg->payload_output = d_buf;
+ msg->payload_length_output =
+ data->com_paras.d_encryption.dst_size;
+ priv->client.receive_cb = fcs_vab_callback;
+
+ ret = fcs_request_service(priv, (void *)msg,
+ FCS_REQUEST_TIMEOUT);
+ if (!ret && !priv->status) {
+ msg->payload = ps_buf;
+ msg->payload_length = PS_BUF_SIZE;
+ msg->command = COMMAND_POLL_SERVICE_STATUS;
+
+ priv->client.receive_cb = fcs_data_callback;
+ ret = fcs_request_service(priv, (void *)msg,
+ FCS_COMPLETED_TIMEOUT);
+ dev_dbg(dev, "request service ret=%d\n", ret);
+
+ if (!ret && !priv->status) {
+ if (!priv->kbuf) {
+ dev_err(dev, "failure on kbuf\n");
+ fcs_close_services(priv, ps_buf, NULL);
+ fcs_close_services(priv, s_buf, d_buf);
+ return -EFAULT;
+ }
+ buf_sz = *(unsigned int *)priv->kbuf;
+ data->com_paras.d_encryption.dst_size = buf_sz;
+ memcpy(data->com_paras.d_encryption.dst,
+ d_buf, buf_sz);
+ data->status = 0;
+ } else {
+ data->com_paras.d_encryption.dst = NULL;
+ data->com_paras.d_encryption.dst_size = 0;
+ data->status = priv->status;
+ }
+ } else {
+ data->com_paras.d_encryption.dst = NULL;
+ data->com_paras.d_encryption.dst_size = 0;
+ data->status = priv->status;
+ }
+
+ if (copy_to_user((void __user *)arg, data, sizeof(*data))) {
+ dev_err(dev, "failure on copy_to_user\n");
+ fcs_close_services(priv, ps_buf, NULL);
+ fcs_close_services(priv, s_buf, d_buf);
+ ret = -EFAULT;
+ }
+
+ fcs_close_services(priv, ps_buf, NULL);
+ fcs_close_services(priv, s_buf, d_buf);
+ break;
+ case INTEL_FCS_DEV_DATA_DECRYPTION:
+ if (copy_from_user(data, (void __user *)arg, sizeof(*data))) {
+ dev_err(dev, "failure on copy_from_user\n");
+ return -EFAULT;
+ }
+
+ if ((data->com_paras.d_encryption.src_size < MIN_SDOS_BUF_SZ) ||
+ (data->com_paras.d_encryption.src_size > MAX_SDOS_BUF_SZ)) {
+ dev_err(dev, "Invalid SDOS Buffer src size:%d\n",
+ data->com_paras.d_encryption.src_size);
+ return -EFAULT;
+ }
+
+ if ((data->com_paras.d_encryption.dst_size < MIN_SDOS_BUF_SZ) ||
+ (data->com_paras.d_encryption.dst_size > MAX_SDOS_BUF_SZ)) {
+ dev_err(dev, "Invalid SDOS Buffer dst size:%d\n",
+ data->com_paras.d_encryption.dst_size);
+ return -EFAULT;
+ }
+
+ /* allocate buffer for both source and destination */
+ s_buf = stratix10_svc_allocate_memory(priv->chan,
+ MAX_SDOS_BUF_SZ);
+ if (!s_buf) {
+ dev_err(dev, "failed allocate decrypt src buf\n");
+ return -ENOMEM;
+ }
+ d_buf = stratix10_svc_allocate_memory(priv->chan,
+ MAX_SDOS_BUF_SZ);
+ if (!d_buf) {
+ dev_err(dev, "failed allocate decrypt dst buf\n");
+ stratix10_svc_free_memory(priv->chan, s_buf);
+ return -ENOMEM;
+ }
+
+ ps_buf = stratix10_svc_allocate_memory(priv->chan,
+ PS_BUF_SIZE);
+ if (!ps_buf) {
+ dev_err(dev, "failed allocate p-status buffer\n");
+ fcs_close_services(priv, s_buf, d_buf);
+ return -ENOMEM;
+ }
+
+ ret = copy_from_user(s_buf,
+ data->com_paras.d_decryption.src,
+ data->com_paras.d_decryption.src_size);
+ if (ret) {
+ dev_err(dev, "failure on copy_from_user\n");
+ fcs_close_services(priv, ps_buf, NULL);
+ fcs_close_services(priv, s_buf, d_buf);
+ return -EFAULT;
+ }
+
+ msg->command = COMMAND_FCS_DATA_DECRYPTION;
+ msg->payload = s_buf;
+ msg->payload_length =
+ data->com_paras.d_decryption.src_size;
+ msg->payload_output = d_buf;
+ msg->payload_length_output =
+ data->com_paras.d_decryption.dst_size;
+ priv->client.receive_cb = fcs_vab_callback;
+
+ ret = fcs_request_service(priv, (void *)msg,
+ FCS_REQUEST_TIMEOUT);
+ if (!ret && !priv->status) {
+ msg->command = COMMAND_POLL_SERVICE_STATUS;
+ msg->payload = ps_buf;
+ msg->payload_length = PS_BUF_SIZE;
+ priv->client.receive_cb = fcs_data_callback;
+ ret = fcs_request_service(priv, (void *)msg,
+ FCS_COMPLETED_TIMEOUT);
+ dev_dbg(dev, "request service ret=%d\n", ret);
+ if (!ret && !priv->status) {
+ if (!priv->kbuf) {
+ dev_err(dev, "failure on kbuf\n");
+ fcs_close_services(priv, ps_buf, NULL);
+ fcs_close_services(priv, s_buf, d_buf);
+ return -EFAULT;
+ }
+ buf_sz = *((unsigned int *)priv->kbuf);
+ memcpy(data->com_paras.d_decryption.dst,
+ d_buf, buf_sz);
+ data->com_paras.d_decryption.dst_size = buf_sz;
+ data->status = 0;
+ } else {
+ data->com_paras.d_decryption.dst = NULL;
+ data->com_paras.d_decryption.dst_size = 0;
+ data->status = priv->status;
+ }
+ } else {
+ data->com_paras.d_decryption.dst = NULL;
+ data->com_paras.d_decryption.dst_size = 0;
+ data->status = priv->status;
+ }
+
+ if (copy_to_user((void __user *)arg, data, sizeof(*data))) {
+ dev_err(dev, "failure on copy_to_user\n");
+ fcs_close_services(priv, ps_buf, NULL);
+ fcs_close_services(priv, s_buf, d_buf);
+ ret = -EFAULT;
+ }
+
+ fcs_close_services(priv, ps_buf, NULL);
+ fcs_close_services(priv, s_buf, d_buf);
+ break;
+ default:
+ dev_warn(dev, "shouldn't be here [0x%x]\n", cmd);
+ break;
+ }
+
+ return ret;
+}
+
+static int fcs_open(struct inode *inode, struct file *file)
+{
+ pr_debug("%s\n", __func__);
+
+ return 0;
+}
+
+static int fcs_close(struct inode *inode, struct file *file)
+{
+
+ pr_debug("%s\n", __func__);
+
+ return 0;
+}
+
+static const struct file_operations fcs_fops = {
+ .owner = THIS_MODULE,
+ .unlocked_ioctl = fcs_ioctl,
+ .open = fcs_open,
+ .release = fcs_close,
+};
+
+static int fcs_driver_probe(struct platform_device *pdev)
+{
+ struct device *dev = &pdev->dev;
+ struct intel_fcs_priv *priv;
+ int ret;
+
+ priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
+ if (!priv)
+ return -ENOMEM;
+
+ priv->client.dev = dev;
+ priv->client.receive_cb = NULL;
+ priv->client.priv = priv;
+ priv->status = INVALID_STATUS;
+
+ mutex_init(&priv->lock);
+ priv->chan = stratix10_svc_request_channel_byname(&priv->client,
+ SVC_CLIENT_FCS);
+ if (IS_ERR(priv->chan)) {
+ dev_err(dev, "couldn't get service channel %s\n",
+ SVC_CLIENT_FCS);
+ return PTR_ERR(priv->chan);
+ }
+
+ priv->miscdev.minor = MISC_DYNAMIC_MINOR;
+ priv->miscdev.name = "fcs";
+ priv->miscdev.fops = &fcs_fops;
+
+ init_completion(&priv->completion);
+
+ platform_set_drvdata(pdev, priv);
+
+ ret = misc_register(&priv->miscdev);
+ if (ret) {
+ dev_err(dev, "can't register on minor=%d\n",
+ MISC_DYNAMIC_MINOR);
+ return ret;
+ }
+
+ return 0;
+}
+
+static int fcs_driver_remove(struct platform_device *pdev)
+{
+ struct intel_fcs_priv *priv = platform_get_drvdata(pdev);
+
+ misc_deregister(&priv->miscdev);
+ stratix10_svc_free_channel(priv->chan);
+
+ return 0;
+}
+
+static struct platform_driver fcs_driver = {
+ .probe = fcs_driver_probe,
+ .remove = fcs_driver_remove,
+ .driver = {
+ .name = "intel-fcs",
+ },
+};
+
+module_platform_driver(fcs_driver);
+
+MODULE_LICENSE("GPL v2");
+MODULE_DESCRIPTION("Intel FGPA Crypto Services Driver");
+MODULE_AUTHOR("Richard Gong <[email protected]>");
+
diff --git a/include/uapi/linux/intel-fcs_ioctl.h b/include/uapi/linux/intel-fcs_ioctl.h
new file mode 100644
index 00000000..4d530ec
--- /dev/null
+++ b/include/uapi/linux/intel-fcs_ioctl.h
@@ -0,0 +1,222 @@
+/* SPDX-License-Identifier: GPL-2.0 WITH Linux-syscall-note */
+/*
+ * Copyright (C) 2020, Intel Corporation
+ */
+
+#ifndef __INTEL_FCS_IOCTL_H
+#define __INTEL_FCS_IOCTL_H
+
+#include <linux/types.h>
+
+#define INTEL_FCS_IOCTL 0xA2
+
+/**
+ * enum fcs_vab_img_type - enumeration of image types
+ * @INTEL_FCS_IMAGE_HPS: Image to validate is HPS image
+ * @INTEL_FCS_IMAGE_BITSTREAM: Image to validate is bitstream
+ */
+enum fcs_vab_img_type {
+ INTEL_FCS_IMAGE_HPS = 0,
+ INTEL_FCS_IMAGE_BITSTREAM = 1
+};
+
+/**
+ * enum fcs_certificate_test - enumeration of certificate test
+ * @INTEL_FCS_NO_TEST: Write to eFuses
+ * @INTEL_FCS_TEST: Write to cache, do not write eFuses
+ */
+enum fcs_certificate_test {
+ INTEL_FCS_NO_TEST = 0,
+ INTEL_FCS_TEST = 1
+};
+
+/**
+ * struct intel_fcs_cert_test_word - certificate test word
+ * @test_bit: if set, do not write fuses, write to cache only.
+ * @rsvd: write as 0
+ */
+struct intel_fcs_cert_test_word {
+ __u32 test_bit:1;
+ __u32 rsvd:31;
+};
+
+/**
+ * struct fcs_validation_request - validate HPS or bitstream image
+ * @so_type: the type of signed object, 0 for HPS and 1 for bitstream
+ * @src: the source of signed object,
+ * for HPS, this is the virtual address of the signed source
+ * for Bitstream, this is path of the signed source, the default
+ * path is /lib/firmware
+ * @size: the size of the signed object
+ */
+struct fcs_validation_request {
+ enum fcs_vab_img_type so_type;
+ void *src;
+ __u32 size;
+};
+
+/**
+ * struct fcs_key_manage_request - Request key management from SDM
+ * @addr: the virtual address of the signed object,
+ * @size: the size of the signed object
+ */
+struct fcs_key_manage_request {
+ void *addr;
+ __u32 size;
+};
+
+/**
+ * struct fcs_certificate_request - Certificate request to SDM
+ * @test: test bit (1 if want to write to cache instead of fuses)
+ * @addr: the virtual address of the signed object,
+ * @size: the size of the signed object
+ * @c_status: returned certificate status
+ */
+struct fcs_certificate_request {
+ struct intel_fcs_cert_test_word test;
+ void *addr;
+ __u32 size;
+ __u32 c_status;
+};
+
+/**
+ * struct fcs_data_encryption - aes data encryption command layout
+ * @src: the virtual address of the input data
+ * @src_size: the size of the unencrypted source
+ * @dst: the virtual address of the output data
+ * @dst_size: the size of the encrypted result
+ */
+struct fcs_data_encryption {
+ void *src;
+ __u32 src_size;
+ void *dst;
+ __u32 dst_size;
+};
+
+/**
+ * struct fcs_data_decryption - aes data decryption command layout
+ * @src: the virtual address of the input data
+ * @src_size: the size of the encrypted source
+ * @dst: the virtual address of the output data
+ * @dst_size: the size of the decrypted result
+ */
+struct fcs_data_decryption {
+ void *src;
+ __u32 src_size;
+ void *dst;
+ __u32 dst_size;
+};
+
+/**
+ * struct fcs_random_number_gen
+ * @rndm: 8 words of random data.
+ */
+struct fcs_random_number_gen {
+ __u32 rndm[8];
+};
+
+/**
+ * struct fcs_version
+ * @version: version data.
+ * @flags: Reserved as 0
+ */
+struct fcs_version {
+ __u32 version;
+ __u32 flags;
+};
+
+/**
+ * struct intel_fcs_dev_ioct: common structure passed to Linux
+ * kernel driver for all commands.
+ * @status: Used for the return code.
+ * -1 -- operation is not started
+ * 0 -- operation is successfully completed
+ * non-zero -- operation failed
+ * @s_request: Validation of a bitstream.
+ * @c_request: Certificate request.
+ * hps_vab: validation of an HPS image
+ * counter set: burn fuses for new counter values
+ * @gp_data: view the eFuse provisioning state.
+ * @d_encryption: AES encryption (SDOS)
+ * @d_decryption: AES decryption (SDOS)
+ * @rn_gen: random number generator result
+ */
+struct intel_fcs_dev_ioctl {
+ /* used for return status code */
+ int status;
+
+ /* command parameters */
+ union {
+ struct fcs_validation_request s_request;
+ struct fcs_certificate_request c_request;
+ struct fcs_key_manage_request gp_data;
+ struct fcs_data_encryption d_encryption;
+ struct fcs_data_decryption d_decryption;
+ struct fcs_random_number_gen rn_gen;
+ struct fcs_version version;
+ } com_paras;
+};
+
+/**
+ * intel_fcs_command_code - support fpga crypto service commands
+ *
+ * Values are subject to change as a result of upstreaming.
+ *
+ * @INTEL_FCS_DEV_VERSION_CMD:
+ *
+ * @INTEL_FCS_DEV_CERTIFICATE_CMD:
+ *
+ * @INTEL_FCS_DEV_VALIDATE_REQUEST_CMD:
+ *
+ * @INTEL_FCS_DEV_COUNTER_SET_CMD:
+ *
+ * @INTEL_FCS_DEV_SVN_COMMIT_CMD:
+ *
+ * @INTEL_FCS_DEV_DATA_ENCRYPTION_CMD:
+ *
+ * @INTEL_FCS_DEV_DATA_DECRYPTION_CMD:
+ *
+ * @INTEL_FCS_DEV_RANDOM_NUMBER_GEN_CMD:
+ */
+enum intel_fcs_command_code {
+ INTEL_FCS_DEV_COMMAND_NONE = 0,
+ INTEL_FCS_DEV_VERSION_CMD = 1,
+ INTEL_FCS_DEV_CERTIFICATE_CMD = 0xB,
+ INTEL_FCS_DEV_VALIDATE_REQUEST_CMD = 0x78,
+ INTEL_FCS_DEV_COUNTER_SET_CMD,
+ INTEL_FCS_DEV_GET_PROVISION_DATA_CMD = 0x7B,
+ INTEL_FCS_DEV_DATA_ENCRYPTION_CMD = 0x7E,
+ INTEL_FCS_DEV_DATA_DECRYPTION_CMD,
+ INTEL_FCS_DEV_RANDOM_NUMBER_GEN_CMD
+};
+
+#define INTEL_FCS_DEV_VERSION_REQUEST \
+ _IOWR(INTEL_FCS_IOCTL, \
+ INTEL_FCS_DEV_VERSION_CMD, struct intel_fcs_dev_ioctl)
+
+#define INTEL_FCS_DEV_VALIDATION_REQUEST \
+ _IOWR(INTEL_FCS_IOCTL, \
+ INTEL_FCS_DEV_VALIDATE_REQUEST_CMD, struct intel_fcs_dev_ioctl)
+
+#define INTEL_FCS_DEV_SEND_CERTIFICATE \
+ _IOWR(INTEL_FCS_IOCTL, \
+ INTEL_FCS_DEV_CERTIFICATE_CMD, struct intel_fcs_dev_ioctl)
+
+#define INTEL_FCS_DEV_GET_PROVISION_DATA \
+ _IOWR(INTEL_FCS_IOCTL, \
+ INTEL_FCS_DEV_GET_PROVISION_DATA_CMD, struct intel_fcs_dev_ioctl)
+
+#define INTEL_FCS_DEV_DATA_ENCRYPTION \
+ _IOWR(INTEL_FCS_IOCTL, \
+ INTEL_FCS_DEV_DATA_ENCRYPTION_CMD, struct intel_fcs_dev_ioctl)
+
+#define INTEL_FCS_DEV_DATA_DECRYPTION \
+ _IOWR(INTEL_FCS_IOCTL, \
+ INTEL_FCS_DEV_DATA_DECRYPTION_CMD, struct intel_fcs_dev_ioctl)
+
+#define INTEL_FCS_DEV_RANDOM_NUMBER_GEN \
+ _IOWR(INTEL_FCS_IOCTL, \
+ INTEL_FCS_DEV_RANDOM_NUMBER_GEN_CMD, struct intel_fcs_dev_ioctl)
+
+#endif
+
--
2.7.4

2020-08-17 13:40:19

by Richard Gong

[permalink] [raw]
Subject: [PATCHv2 1/2] firmware: stratix10-svc: extend svc to support new crypto features

From: Richard Gong <[email protected]>

Extend Intel service layer driver to support new crypto services on
Intel SoCFPGA platforms.

The crypto services include security certificate, image boot validation,
security key cancellation, get provision data, random number generation,
advance encrtption standard (AES) encryption and decryption services.

Signed-off-by: Richard Gong <[email protected]>
---
v2: no change
---
drivers/firmware/stratix10-svc.c | 178 +++++++++++++++++++--
include/linux/firmware/intel/stratix10-smc.h | 147 ++++++++++++++++-
.../linux/firmware/intel/stratix10-svc-client.h | 42 +++++
3 files changed, 348 insertions(+), 19 deletions(-)

diff --git a/drivers/firmware/stratix10-svc.c b/drivers/firmware/stratix10-svc.c
index 3aa489d..994d0a1 100644
--- a/drivers/firmware/stratix10-svc.c
+++ b/drivers/firmware/stratix10-svc.c
@@ -34,12 +34,13 @@
* timeout is set to 30 seconds (30 * 1000) at Intel Stratix10 SoC.
*/
#define SVC_NUM_DATA_IN_FIFO 32
-#define SVC_NUM_CHANNEL 2
+#define SVC_NUM_CHANNEL 3
#define FPGA_CONFIG_DATA_CLAIM_TIMEOUT_MS 200
#define FPGA_CONFIG_STATUS_TIMEOUT_SEC 30

/* stratix10 service layer clients */
#define STRATIX10_RSU "stratix10-rsu"
+#define INTEL_FCS "intel-fcs"

typedef void (svc_invoke_fn)(unsigned long, unsigned long, unsigned long,
unsigned long, unsigned long, unsigned long,
@@ -53,6 +54,7 @@ struct stratix10_svc_chan;
*/
struct stratix10_svc {
struct platform_device *stratix10_svc_rsu;
+ struct platform_device *intel_svc_fcs;
};

/**
@@ -97,8 +99,10 @@ struct stratix10_svc_data_mem {
/**
* struct stratix10_svc_data - service data structure
* @chan: service channel
- * @paddr: playload physical address
- * @size: playload size
+ * @paddr: physical address of to be processed payload
+ * @size: to be processed playload size
+ * @paddr_output: physical address of the processed payload
+ * @size_output: the processed payload size
* @command: service command requested by client
* @flag: configuration type (full or partial)
* @arg: args to be passed via registers and not physically mapped buffers
@@ -109,6 +113,8 @@ struct stratix10_svc_data {
struct stratix10_svc_chan *chan;
phys_addr_t paddr;
size_t size;
+ phys_addr_t paddr_output;
+ size_t size_output;
u32 command;
u32 flag;
u64 arg[3];
@@ -246,32 +252,54 @@ static void svc_thread_cmd_config_status(struct stratix10_svc_controller *ctrl,
{
struct arm_smccc_res res;
int count_in_sec;
+ unsigned long a0, a1, a2;

cb_data->kaddr1 = NULL;
cb_data->kaddr2 = NULL;
cb_data->kaddr3 = NULL;
cb_data->status = BIT(SVC_STATUS_ERROR);

- pr_debug("%s: polling config status\n", __func__);
+ pr_debug("%s: polling completed status\n", __func__);
+
+ a0 = INTEL_SIP_SMC_FPGA_CONFIG_ISDONE;
+ a1 = (unsigned long)p_data->paddr;
+ a2 = (unsigned long)p_data->size;
+
+ if (p_data->command == COMMAND_POLL_SERVICE_STATUS)
+ a0 = INTEL_SIP_SMC_SERVICE_COMPLETED;

count_in_sec = FPGA_CONFIG_STATUS_TIMEOUT_SEC;
while (count_in_sec) {
- ctrl->invoke_fn(INTEL_SIP_SMC_FPGA_CONFIG_ISDONE,
- 0, 0, 0, 0, 0, 0, 0, &res);
+ ctrl->invoke_fn(a0, a1, a2, 0, 0, 0, 0, 0, &res);
if ((res.a0 == INTEL_SIP_SMC_STATUS_OK) ||
- (res.a0 == INTEL_SIP_SMC_STATUS_ERROR))
+ (res.a0 == INTEL_SIP_SMC_STATUS_ERROR) ||
+ (res.a0 == INTEL_SIP_SMC_STATUS_REJECTED))
break;

/*
- * configuration is still in progress, wait one second then
+ * request is still in progress, wait one second then
* poll again
*/
msleep(1000);
count_in_sec--;
- }
+ };

- if (res.a0 == INTEL_SIP_SMC_STATUS_OK && count_in_sec)
+ if (!count_in_sec) {
+ pr_err("%s: poll status timeout\n", __func__);
+ cb_data->status = BIT(SVC_STATUS_BUSY);
+ } else if (res.a0 == INTEL_SIP_SMC_STATUS_OK) {
cb_data->status = BIT(SVC_STATUS_COMPLETED);
+ cb_data->kaddr2 = (res.a2) ?
+ svc_pa_to_va(res.a2) : NULL;
+ cb_data->kaddr3 = (res.a3) ? &res.a3 : NULL;
+ } else {
+ pr_err("%s: poll status error\n", __func__);
+ cb_data->kaddr1 = &res.a1;
+ cb_data->kaddr2 = (res.a2) ?
+ svc_pa_to_va(res.a2) : NULL;
+ cb_data->kaddr3 = (res.a3) ? &res.a3 : NULL;
+ cb_data->status = BIT(SVC_STATUS_ERROR);
+ }

p_data->chan->scl->receive_cb(p_data->chan->scl, cb_data);
}
@@ -296,6 +324,10 @@ static void svc_thread_recv_status_ok(struct stratix10_svc_data *p_data,
case COMMAND_RECONFIG:
case COMMAND_RSU_UPDATE:
case COMMAND_RSU_NOTIFY:
+ case COMMAND_FCS_REQUEST_SERVICE:
+ case COMMAND_FCS_SEND_CERTIFICATE:
+ case COMMAND_FCS_DATA_ENCRYPTION:
+ case COMMAND_FCS_DATA_DECRYPTION:
cb_data->status = BIT(SVC_STATUS_OK);
break;
case COMMAND_RECONFIG_DATA_SUBMIT:
@@ -314,6 +346,14 @@ static void svc_thread_recv_status_ok(struct stratix10_svc_data *p_data,
cb_data->kaddr1 = &res.a1;
cb_data->kaddr2 = &res.a2;
break;
+ case COMMAND_FCS_RANDOM_NUMBER_GEN:
+ case COMMAND_FCS_GET_PROVISION_DATA:
+ case COMMAND_POLL_SERVICE_STATUS:
+ cb_data->status = BIT(SVC_STATUS_OK);
+ cb_data->kaddr1 = &res.a1;
+ cb_data->kaddr2 = svc_pa_to_va(res.a2);
+ cb_data->kaddr3 = &res.a3;
+ break;
default:
pr_warn("it shouldn't happen\n");
break;
@@ -340,7 +380,7 @@ static int svc_normal_to_secure_thread(void *data)
struct stratix10_svc_data *pdata;
struct stratix10_svc_cb_data *cbdata;
struct arm_smccc_res res;
- unsigned long a0, a1, a2;
+ unsigned long a0, a1, a2, a3, a4, a5;
int ret_fifo = 0;

pdata = kmalloc(sizeof(*pdata), GFP_KERNEL);
@@ -357,6 +397,9 @@ static int svc_normal_to_secure_thread(void *data)
a0 = INTEL_SIP_SMC_FPGA_CONFIG_LOOPBACK;
a1 = 0;
a2 = 0;
+ a3 = 0;
+ a4 = 0;
+ a5 = 0;

pr_debug("smc_hvc_shm_thread is running\n");

@@ -422,6 +465,52 @@ static int svc_normal_to_secure_thread(void *data)
a1 = 0;
a2 = 0;
break;
+
+ /* for FPGA crypto service */
+ case COMMAND_FCS_DATA_ENCRYPTION:
+ a0 = INTEL_SIP_SMC_FCS_CRYPTION;
+ a1 = 1;
+ a2 = (unsigned long)pdata->paddr;
+ a3 = (unsigned long)pdata->size;
+ a4 = (unsigned long)pdata->paddr_output;
+ a5 = (unsigned long)pdata->size_output;
+ break;
+ case COMMAND_FCS_DATA_DECRYPTION:
+ a0 = INTEL_SIP_SMC_FCS_CRYPTION;
+ a1 = 0;
+ a2 = (unsigned long)pdata->paddr;
+ a3 = (unsigned long)pdata->size;
+ a4 = (unsigned long)pdata->paddr_output;
+ a5 = (unsigned long)pdata->size_output;
+ break;
+ case COMMAND_FCS_RANDOM_NUMBER_GEN:
+ a0 = INTEL_SIP_SMC_FCS_RANDOM_NUMBER;
+ a1 = (unsigned long)pdata->paddr;
+ a2 = 0;
+ break;
+ case COMMAND_FCS_REQUEST_SERVICE:
+ a0 = INTEL_SIP_SMC_FCS_SERVICE_REQUEST;
+ a1 = (unsigned long)pdata->paddr;
+ a2 = (unsigned long)pdata->size;
+ break;
+ case COMMAND_FCS_SEND_CERTIFICATE:
+ a0 = INTEL_SIP_SMC_FCS_SEND_CERTIFICATE;
+ a1 = (unsigned long)pdata->paddr;
+ a2 = (unsigned long)pdata->size;
+ break;
+ case COMMAND_FCS_GET_PROVISION_DATA:
+ a0 = INTEL_SIP_SMC_FCS_GET_PROVISION_DATA;
+ a1 = (unsigned long)pdata->paddr;
+ a2 = 0;
+ break;
+
+ /* for general service status polling */
+ case COMMAND_POLL_SERVICE_STATUS:
+ a0 = INTEL_SIP_SMC_SERVICE_COMPLETED;
+ a1 = (unsigned long)pdata->paddr;
+ a2 = (unsigned long)pdata->size;
+ break;
+
default:
pr_warn("it shouldn't happen\n");
break;
@@ -430,7 +519,14 @@ static int svc_normal_to_secure_thread(void *data)
__func__, (unsigned int)a0, (unsigned int)a1);
pr_debug(" a2=0x%016x\n", (unsigned int)a2);

- ctrl->invoke_fn(a0, a1, a2, 0, 0, 0, 0, 0, &res);
+ if (a0 == INTEL_SIP_SMC_FCS_CRYPTION) {
+ pr_debug(" a3=0x%016x\n", (unsigned int)a3);
+ pr_debug(" a4=0x%016x\n", (unsigned int)a4);
+ pr_debug(" a5=0x%016x\n", (unsigned int)a5);
+ ctrl->invoke_fn(a0, a1, a2, a3, a4, a5, 0, 0, &res);
+ } else {
+ ctrl->invoke_fn(a0, a1, a2, 0, 0, 0, 0, 0, &res);
+ }

pr_debug("%s: after SMC call -- res.a0=0x%016x",
__func__, (unsigned int)res.a0);
@@ -462,6 +558,7 @@ static int svc_normal_to_secure_thread(void *data)
pdata, cbdata);
break;
case COMMAND_RECONFIG_STATUS:
+ case COMMAND_POLL_SERVICE_STATUS:
svc_thread_cmd_config_status(ctrl,
pdata, cbdata);
break;
@@ -472,14 +569,31 @@ static int svc_normal_to_secure_thread(void *data)
break;
case INTEL_SIP_SMC_STATUS_REJECTED:
pr_debug("%s: STATUS_REJECTED\n", __func__);
+ /* for FPGA crypto service */
+ switch (pdata->command) {
+ case COMMAND_FCS_REQUEST_SERVICE:
+ case COMMAND_FCS_SEND_CERTIFICATE:
+ case COMMAND_FCS_GET_PROVISION_DATA:
+ case COMMAND_FCS_DATA_ENCRYPTION:
+ case COMMAND_FCS_DATA_DECRYPTION:
+ case COMMAND_FCS_RANDOM_NUMBER_GEN:
+ cbdata->status = BIT(SVC_STATUS_INVALID_PARAM);
+ cbdata->kaddr1 = NULL;
+ cbdata->kaddr2 = NULL;
+ cbdata->kaddr3 = NULL;
+ pdata->chan->scl->receive_cb(pdata->chan->scl,
+ cbdata);
+ break;
+ }
break;
case INTEL_SIP_SMC_STATUS_ERROR:
case INTEL_SIP_SMC_RSU_ERROR:
pr_err("%s: STATUS_ERROR\n", __func__);
cbdata->status = BIT(SVC_STATUS_ERROR);
- cbdata->kaddr1 = NULL;
- cbdata->kaddr2 = NULL;
- cbdata->kaddr3 = NULL;
+ cbdata->kaddr1 = &res.a1;
+ cbdata->kaddr2 = (res.a2) ?
+ svc_pa_to_va(res.a2) : NULL;
+ cbdata->kaddr3 = (res.a3) ? &res.a3 : NULL;
pdata->chan->scl->receive_cb(pdata->chan->scl, cbdata);
break;
default:
@@ -503,7 +617,7 @@ static int svc_normal_to_secure_thread(void *data)
break;

}
- }
+ };

kfree(cbdata);
kfree(pdata);
@@ -845,15 +959,25 @@ int stratix10_svc_send(struct stratix10_svc_chan *chan, void *msg)
list_for_each_entry(p_mem, &svc_data_mem, node)
if (p_mem->vaddr == p_msg->payload) {
p_data->paddr = p_mem->paddr;
+ p_data->size = p_msg->payload_length;
break;
}
+ if (p_msg->payload_output) {
+ list_for_each_entry(p_mem, &svc_data_mem, node)
+ if (p_mem->vaddr == p_msg->payload_output) {
+ p_data->paddr_output =
+ p_mem->paddr;
+ p_data->size_output =
+ p_msg->payload_length_output;
+ break;
+ }
+ }
}

p_data->command = p_msg->command;
p_data->arg[0] = p_msg->arg[0];
p_data->arg[1] = p_msg->arg[1];
p_data->arg[2] = p_msg->arg[2];
- p_data->size = p_msg->payload_length;
p_data->chan = chan;
pr_debug("%s: put to FIFO pa=0x%016x, cmd=%x, size=%u\n", __func__,
(unsigned int)p_data->paddr, p_data->command,
@@ -949,6 +1073,7 @@ void stratix10_svc_free_memory(struct stratix10_svc_chan *chan, void *kaddr)
break;
}

+ memset(kaddr, 0, size);
gen_pool_free(chan->ctrl->genpool, (unsigned long)kaddr, size);
pmem->vaddr = NULL;
list_del(&pmem->node);
@@ -1029,6 +1154,11 @@ static int stratix10_svc_drv_probe(struct platform_device *pdev)
chans[1].name = SVC_CLIENT_RSU;
spin_lock_init(&chans[1].lock);

+ chans[2].scl = NULL;
+ chans[2].ctrl = controller;
+ chans[2].name = SVC_CLIENT_FCS;
+ spin_lock_init(&chans[2].lock);
+
list_add_tail(&controller->node, &svc_ctrl);
platform_set_drvdata(pdev, controller);

@@ -1048,6 +1178,19 @@ static int stratix10_svc_drv_probe(struct platform_device *pdev)
platform_device_put(svc->stratix10_svc_rsu);
return ret;
}
+
+ svc->intel_svc_fcs = platform_device_alloc(INTEL_FCS, 1);
+ if (!svc->intel_svc_fcs) {
+ dev_err(dev, "failed to allocate %s device\n", INTEL_FCS);
+ return -ENOMEM;
+ }
+
+ ret = platform_device_add(svc->intel_svc_fcs);
+ if (ret) {
+ platform_device_put(svc->intel_svc_fcs);
+ return ret;
+ }
+
dev_set_drvdata(dev, svc);

pr_info("Intel Service Layer Driver Initialized\n");
@@ -1060,6 +1203,7 @@ static int stratix10_svc_drv_remove(struct platform_device *pdev)
struct stratix10_svc *svc = dev_get_drvdata(&pdev->dev);
struct stratix10_svc_controller *ctrl = platform_get_drvdata(pdev);

+ platform_device_unregister(svc->intel_svc_fcs);
platform_device_unregister(svc->stratix10_svc_rsu);

kfifo_free(&ctrl->svc_fifo);
diff --git a/include/linux/firmware/intel/stratix10-smc.h b/include/linux/firmware/intel/stratix10-smc.h
index c3e5ab0..07d6345 100644
--- a/include/linux/firmware/intel/stratix10-smc.h
+++ b/include/linux/firmware/intel/stratix10-smc.h
@@ -321,8 +321,6 @@ INTEL_SIP_SMC_FAST_CALL_VAL(INTEL_SIP_SMC_FUNCID_FPGA_CONFIG_COMPLETED_WRITE)
#define INTEL_SIP_SMC_ECC_DBE \
INTEL_SIP_SMC_FAST_CALL_VAL(INTEL_SIP_SMC_FUNCID_ECC_DBE)

-#endif
-
/**
* Request INTEL_SIP_SMC_RSU_NOTIFY
*
@@ -404,3 +402,148 @@ INTEL_SIP_SMC_FAST_CALL_VAL(INTEL_SIP_SMC_FUNCID_FPGA_CONFIG_COMPLETED_WRITE)
#define INTEL_SIP_SMC_FUNCID_RSU_MAX_RETRY 18
#define INTEL_SIP_SMC_RSU_MAX_RETRY \
INTEL_SIP_SMC_FAST_CALL_VAL(INTEL_SIP_SMC_FUNCID_RSU_MAX_RETRY)
+
+/**
+ * Request INTEL_SIP_SMC_SERVICE_COMPLETED
+ * Sync call to check if the secure world have completed service request
+ * or not.
+ *
+ * Call register usage:
+ * a0: INTEL_SIP_SMC_SERVICE_COMPLETED
+ * a1: this register is optional. If used, it is the physical address for
+ * secure firmware to put output data
+ * a2: this register is optional. If used, it is the size of output data
+ * a3-a7: not used
+ *
+ * Return status:
+ * a0: INTEL_SIP_SMC_STATUS_OK, INTEL_SIP_SMC_STATUS_ERROR,
+ * INTEL_SIP_SMC_REJECTED or INTEL_SIP_SMC_STATUS_BUSY
+ * a1: mailbox error if a0 is INTEL_SIP_SMC_STATUS_ERROR
+ * a2: physical address containing the process info
+ * for FCS certificate -- the data contains the certificate status
+ * for FCS cryption -- the data contains the actual data size FW processes
+ * a3: output data size
+ */
+#define INTEL_SIP_SMC_FUNCID_SERVICE_COMPLETED 30
+#define INTEL_SIP_SMC_SERVICE_COMPLETED \
+ INTEL_SIP_SMC_FAST_CALL_VAL(INTEL_SIP_SMC_FUNCID_SERVICE_COMPLETED)
+
+/**
+ * SMC call protocol for Mailbox, starting FUNCID from 60
+ */
+#define INTEL_SIP_SMC_FUNCID_MBOX_SEND_CMD 60
+ #define INTEL_SIP_SMC_MBOX_SEND_CMD \
+ INTEL_SIP_SMC_FAST_CALL_VAL(INTEL_SIP_SMC_FUNCID_MBOX_SEND_CMD)
+
+/**
+ * SMC call protocol for FPGA Crypto Service (FCS)
+ * FUNCID starts from 90
+ */
+
+/**
+ * Request INTEL_SIP_SMC_FCS_RANDOM_NUMBER
+ *
+ * Sync call used to query the random number generated by the firmware
+ *
+ * Call register usage:
+ * a0 INTEL_SIP_SMC_FCS_RANDOM_NUMBER
+ * a1 the physical address for firmware to write generated random data
+ * a2-a7 not used
+ *
+ * Return status:
+ * a0 INTEL_SIP_SMC_STATUS_OK, INTEL_SIP_SMC_FCS_ERROR or
+ * INTEL_SIP_SMC_FCS_REJECTED
+ * a1 mailbox error
+ * a2 the physical address of generated random number
+ * a3 size
+ */
+#define INTEL_SIP_SMC_FUNCID_FCS_RANDOM_NUMBER 90
+#define INTEL_SIP_SMC_FCS_RANDOM_NUMBER \
+ INTEL_SIP_SMC_FAST_CALL_VAL(INTEL_SIP_SMC_FUNCID_FCS_RANDOM_NUMBER)
+
+/**
+ * Request INTEL_SIP_SMC_FCS_CRYPTION
+ * Async call for data encryption and HMAC signature generation, or for
+ * data decryption and HMAC verification.
+ *
+ * Call INTEL_SIP_SMC_SERVICE_COMPLETED to get the output encrypted or
+ * decrypted data
+ *
+ * Call register usage:
+ * a0 INTEL_SIP_SMC_FCS_CRYPTION
+ * a1 cryption mode (1 for encryption and 0 for decryption)
+ * a2 physical address which stores to be encrypted or decrypted data
+ * a3 input data size
+ * a4 physical address which will hold the encrypted or decrypted output data
+ * a5 output data size
+ * a6-a7 not used
+ *
+ * Return status:
+ * a0 INTEL_SIP_SMC_STATUS_OK, INTEL_SIP_SMC_STATUS_ERROR or
+ * INTEL_SIP_SMC_STATUS_REJECTED
+ * a1-3 not used
+ */
+#define INTEL_SIP_SMC_FUNCID_FCS_CRYPTION 91
+#define INTEL_SIP_SMC_FCS_CRYPTION \
+ INTEL_SIP_SMC_STD_CALL_VAL(INTEL_SIP_SMC_FUNCID_FCS_CRYPTION)
+
+/**
+ * Request INTEL_SIP_SMC_FCS_SERVICE_REQUEST
+ * Async call for authentication service of HPS software
+ *
+ * Call register usage:
+ * a0 INTEL_SIP_SMC_FCS_SERVICE_REQUEST
+ * a1 the physical address of data block
+ * a2 size of data block
+ * a3-a7 not used
+ *
+ * Return status:
+ * a0 INTEL_SIP_SMC_STATUS_OK, INTEL_SIP_SMC_ERROR or
+ * INTEL_SIP_SMC_REJECTED
+ * a1-a3 not used
+ */
+#define INTEL_SIP_SMC_FUNCID_FCS_SERVICE_REQUEST 92
+#define INTEL_SIP_SMC_FCS_SERVICE_REQUEST \
+ INTEL_SIP_SMC_STD_CALL_VAL(INTEL_SIP_SMC_FUNCID_FCS_SERVICE_REQUEST)
+
+/**
+ * Request INTEL_SIP_SMC_FUNCID_FCS_SEND_CERTIFICATE
+ * Sync call to send a signed certificate
+ *
+ * Call register usage:
+ * a0 INTEL_SIP_SMC_FCS_SEND_CERTIFICATE
+ * a1 the physical address of CERTIFICATE block
+ * a2 size of data block
+ * a3-a7 not used
+ *
+ * Return status:
+ * a0 INTEL_SIP_SMC_STATUS_OK or INTEL_SIP_SMC_FCS_REJECTED
+ * a1-a3 not used
+ */
+#define INTEL_SIP_SMC_FUNCID_FCS_SEND_CERTIFICATE 93
+#define INTEL_SIP_SMC_FCS_SEND_CERTIFICATE \
+ INTEL_SIP_SMC_STD_CALL_VAL(INTEL_SIP_SMC_FUNCID_FCS_SEND_CERTIFICATE)
+
+/**
+ * Request INTEL_SIP_SMC_FCS_GET_PROVISION_DATA
+ * Sync call to dump all the fuses and key hashes
+ *
+ * Call register usage:
+ * a0 INTEL_SIP_SMC_FCS_GET_PROVISION_DATA
+ * a1 the physical address for firmware to write structure of fuse and
+ * key hashes
+ * a2-a7 not used
+ *
+ * Return status:
+ * a0 INTEL_SIP_SMC_STATUS_OK, INTEL_SIP_SMC_FCS_ERROR or
+ * INTEL_SIP_SMC_FCS_REJECTED
+ * a1 mailbox error
+ * a2 physical address for the structure of fuse and key hashes
+ * a3 the size of structure
+ *
+ */
+#define INTEL_SIP_SMC_FUNCID_FCS_GET_PROVISION_DATA 94
+#define INTEL_SIP_SMC_FCS_GET_PROVISION_DATA \
+ INTEL_SIP_SMC_FAST_CALL_VAL(INTEL_SIP_SMC_FUNCID_FCS_GET_PROVISION_DATA)
+
+#endif
diff --git a/include/linux/firmware/intel/stratix10-svc-client.h b/include/linux/firmware/intel/stratix10-svc-client.h
index a93d859..d858cf0 100644
--- a/include/linux/firmware/intel/stratix10-svc-client.h
+++ b/include/linux/firmware/intel/stratix10-svc-client.h
@@ -11,9 +11,11 @@
*
* fpga: for FPGA configuration
* rsu: for remote status update
+ * fcs: for FPGA crypto service
*/
#define SVC_CLIENT_FPGA "fpga"
#define SVC_CLIENT_RSU "rsu"
+#define SVC_CLIENT_FCS "fcs"

/**
* Status of the sent command, in bit number
@@ -41,6 +43,9 @@
* SVC_STATUS_NO_SUPPORT:
* Secure firmware doesn't support requested features such as RSU retry
* or RSU notify.
+ *
+ * SVC_STATUS_INVALID_PARAM:
+ * Request received by secure firmware has invalid parameter(s).
*/
#define SVC_STATUS_OK 0
#define SVC_STATUS_BUFFER_SUBMITTED 1
@@ -49,6 +54,7 @@
#define SVC_STATUS_BUSY 4
#define SVC_STATUS_ERROR 5
#define SVC_STATUS_NO_SUPPORT 6
+#define SVC_STATUS_INVALID_PARAM 7

/**
* Flag bit for COMMAND_RECONFIG
@@ -66,6 +72,8 @@
#define SVC_RECONFIG_REQUEST_TIMEOUT_MS 300
#define SVC_RECONFIG_BUFFER_TIMEOUT_MS 720
#define SVC_RSU_REQUEST_TIMEOUT_MS 300
+#define SVC_FCS_REQUEST_TIMEOUT_MS 2000
+#define SVC_COMPLETED_TIMEOUT_MS 30000

struct stratix10_svc_chan;

@@ -104,6 +112,27 @@ struct stratix10_svc_chan;
*
* @COMMAND_RSU_DCMF_VERSION: query firmware for the DCMF version, return status
* is SVC_STATUS_OK or SVC_STATUS_ERROR
+ *
+ * @COMMAND_FCS_REQUEST_SERVICE: request validation of image from firmware,
+ * return status is SVC_STATUS_OK, SVC_STATUS_INVALID_PARAM
+ *
+ * @COMMAND_FCS_SEND_CERTIFICATE: send a certificate, return status is
+ * SVC_STATUS_OK, SVC_STATUS_INVALID_PARAM, SVC_STATUS_ERROR
+ *
+ * @COMMAND_FCS_GET_PROVISION_DATA: read the provisioning data, return status is
+ * SVC_STATUS_OK, SVC_STATUS_INVALID_PARAM, SVC_STATUS_ERROR
+ *
+ * @COMMAND_FCS_DATA_ENCRYPTION: encrypt the data, return status is
+ * SVC_STATUS_OK, SVC_STATUS_INVALID_PARAM, SVC_STATUS_ERROR
+ *
+ * @COMMAND_FCS_DATA_DECRYPTION: decrypt the data, return status is
+ * SVC_STATUS_OK, SVC_STATUS_INVALID_PARAM, SVC_STATUS_ERROR
+ *
+ * @COMMAND_FCS_RANDOM_NUMBER_GEN: generate a random number, return status
+ * is SVC_STATUS_OK, SVC_STATUS_ERROR
+ *
+ * @COMMAND_POLL_SERVICE_STATUS: poll if the service request is complete,
+ * return statis is SVC_STATUS_OK, SVC_STATUS_ERROR or SVC_STATUS_BUSY
*/
enum stratix10_svc_command_code {
COMMAND_NOOP = 0,
@@ -117,18 +146,31 @@ enum stratix10_svc_command_code {
COMMAND_RSU_RETRY,
COMMAND_RSU_MAX_RETRY,
COMMAND_RSU_DCMF_VERSION,
+ /* for FPGA crypto service */
+ COMMAND_FCS_REQUEST_SERVICE = 20,
+ COMMAND_FCS_SEND_CERTIFICATE,
+ COMMAND_FCS_GET_PROVISION_DATA,
+ COMMAND_FCS_DATA_ENCRYPTION,
+ COMMAND_FCS_DATA_DECRYPTION,
+ COMMAND_FCS_RANDOM_NUMBER_GEN,
+ /* for general service status poll */
+ COMMAND_POLL_SERVICE_STATUS = 40,
};

/**
* struct stratix10_svc_client_msg - message sent by client to service
* @payload: starting address of data need be processed
* @payload_length: data size in bytes
+ * @payload_output: starting address of the processed data
+ * payload_length_output: the processed data size in bytes
* @command: service command
* @arg: args to be passed via registers and not physically mapped buffers
*/
struct stratix10_svc_client_msg {
void *payload;
size_t payload_length;
+ void *payload_output;
+ size_t payload_length_output;
enum stratix10_svc_command_code command;
u64 arg[3];
};
--
2.7.4

2020-08-28 10:49:44

by Greg Kroah-Hartman

[permalink] [raw]
Subject: Re: [PATCHv2 2/2] misc: add Intel SoCFPGA crypto service driver

On Mon, Aug 17, 2020 at 08:47:30AM -0500, [email protected] wrote:
> From: Richard Gong <[email protected]>
>
> Add Intel FPGA crypto service (FCS) driver to support new crypto services
> on Intel SoCFPGA platforms.
>
> The crypto services include security certificate, image boot validation,
> security key cancellation, get provision data, random number generation,
> advance encrtption standard (AES) encryption and decryption services.
>
> To perform supporting crypto features on Intel SoCFPGA platforms, Linux
> user-space application interacts with FPGA crypto service (FCS) driver via
> structures defined in include/uapi/linux/intel_fcs-ioctl.h.
>
> The application allocates spaces for IOCTL structure to hold the contents
> or points to the data that FCS driver needs, uses IOCTL calls to passes
> data to kernel FCS driver for processing at low level firmware and get
> processed data or status back form the low level firmware via FCS driver.
>
> The user-space application named as fcs_client is at
> https://github.com/altera-opensource/fcs_apps/tree/fcs_client.

Ugh, a custom userspace api just for one crypto driver? Why can't this
just be a userspace driver? Why does it have to be in the kernel?

> @@ -0,0 +1,709 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright (C) 2020, Intel Corporation
> + */
> +
> +#include <linux/arm-smccc.h>
> +#include <linux/bitfield.h>
> +#include <linux/completion.h>
> +#include <linux/firmware.h>
> +#include <linux/fs.h>
> +#include <linux/kobject.h>

Why is kobject.h needed here?

> +#include <linux/miscdevice.h>
> +#include <linux/module.h>
> +#include <linux/mutex.h>
> +#include <linux/of.h>
> +#include <linux/of_platform.h>
> +#include <linux/platform_device.h>
> +#include <linux/firmware/intel/stratix10-svc-client.h>
> +#include <linux/string.h>
> +#include <linux/slab.h>
> +#include <linux/sysfs.h>
> +#include <linux/uaccess.h>
> +
> +#include <uapi/linux/intel-fcs_ioctl.h>
> +
> +#define RANDOM_NUMBER_SIZE 32
> +#define FILE_NAME_SIZE 32
> +#define PS_BUF_SIZE 64
> +#define INVALID_STATUS 0xff
> +
> +#define MIN_SDOS_BUF_SZ 16
> +#define MAX_SDOS_BUF_SZ 32768
> +
> +#define FCS_REQUEST_TIMEOUT (msecs_to_jiffies(SVC_FCS_REQUEST_TIMEOUT_MS))
> +#define FCS_COMPLETED_TIMEOUT (msecs_to_jiffies(SVC_COMPLETED_TIMEOUT_MS))
> +
> +typedef void (*fcs_callback)(struct stratix10_svc_client *client,
> + struct stratix10_svc_cb_data *data);
> +
> +struct intel_fcs_priv {
> + struct stratix10_svc_chan *chan;
> + struct stratix10_svc_client client;
> + struct completion completion;
> + struct mutex lock;
> + struct miscdevice miscdev;
> + unsigned int status;
> + void *kbuf;
> + unsigned int size;
> +};
> +
> +static void fcs_data_callback(struct stratix10_svc_client *client,
> + struct stratix10_svc_cb_data *data)
> +{
> + struct intel_fcs_priv *priv = client->priv;
> +
> + if ((data->status == BIT(SVC_STATUS_OK)) ||
> + (data->status == BIT(SVC_STATUS_COMPLETED))) {
> + priv->status = 0;
> + priv->kbuf = data->kaddr2;
> + priv->size = *((unsigned int *)data->kaddr3);
> + } else if (data->status == BIT(SVC_STATUS_ERROR)) {
> + priv->status = *((unsigned int *)data->kaddr1);
> + dev_err(client->dev, "error, mbox_error=0x%x\n", priv->status);
> + priv->kbuf = data->kaddr2;
> + priv->size = (data->kaddr3) ?
> + *((unsigned int *)data->kaddr3) : 0;
> + } else {
> + dev_err(client->dev, "rejected\n");
> + priv->status = -EINVAL;
> + priv->kbuf = NULL;
> + priv->size = 0;
> + }
> +
> + complete(&priv->completion);
> +}
> +
> +static void fcs_vab_callback(struct stratix10_svc_client *client,
> + struct stratix10_svc_cb_data *data)
> +{
> + struct intel_fcs_priv *priv = client->priv;
> +
> + priv->status = 0;
> +
> + if (data->status == BIT(SVC_STATUS_INVALID_PARAM)) {
> + priv->status = -EINVAL;
> + dev_warn(client->dev, "rejected, invalid param\n");
> + } else if (data->status == BIT(SVC_STATUS_ERROR)) {
> + priv->status = *((unsigned int *)data->kaddr1);
> + dev_err(client->dev, "mbox_error=0x%x\n", priv->status);
> + } else if (data->status == BIT(SVC_STATUS_BUSY)) {
> + priv->status = -ETIMEDOUT;
> + dev_err(client->dev, "timeout to get completed status\n");
> + }
> +
> + complete(&priv->completion);
> +}
> +
> +static int fcs_request_service(struct intel_fcs_priv *priv,
> + void *msg, unsigned long timeout)
> +{
> + struct stratix10_svc_client_msg *p_msg =
> + (struct stratix10_svc_client_msg *)msg;
> + int ret;
> +
> + mutex_lock(&priv->lock);
> + reinit_completion(&priv->completion);
> +
> + ret = stratix10_svc_send(priv->chan, p_msg);
> + if (ret) {
> + ret = -EINVAL;
> + goto unlock;
> + }
> +
> + ret = wait_for_completion_timeout(&priv->completion,
> + timeout);
> + if (!ret) {
> + dev_err(priv->client.dev,
> + "timeout waiting for SMC call\n");
> + ret = -ETIMEDOUT;
> + } else
> + ret = 0;
> +
> +unlock:
> + mutex_unlock(&priv->lock);
> + return ret;
> +}
> +
> +static void fcs_close_services(struct intel_fcs_priv *priv,
> + void *sbuf, void *dbuf)
> +{
> + if (sbuf)
> + stratix10_svc_free_memory(priv->chan, sbuf);
> +
> + if (dbuf)
> + stratix10_svc_free_memory(priv->chan, dbuf);
> +
> + stratix10_svc_done(priv->chan);
> +}
> +
> +static long fcs_ioctl(struct file *file, unsigned int cmd,
> + unsigned long arg)
> +{
> + struct intel_fcs_dev_ioctl *data;
> + struct intel_fcs_priv *priv;
> + struct device *dev;
> + struct stratix10_svc_client_msg *msg;
> + const struct firmware *fw;
> + char filename[FILE_NAME_SIZE];
> + size_t tsz, datasz;
> + void *s_buf;
> + void *d_buf;
> + void *ps_buf;
> + unsigned int buf_sz;
> + int ret = 0;
> + int i;

You seem to "trust" the structure data from userspace a lot in this
function. Please audit each one of these calls again, I think there are
some places where you can do "bad things"...

> +
> + priv = container_of(file->private_data, struct intel_fcs_priv, miscdev);
> + dev = priv->client.dev;
> +
> + data = devm_kzalloc(dev, sizeof(*data), GFP_KERNEL);
> + if (!data)
> + return -ENOMEM;
> +
> + msg = devm_kzalloc(dev, sizeof(*msg), GFP_KERNEL);
> + if (!msg)
> + return -ENOMEM;
> +
> + switch (cmd) {
> + case INTEL_FCS_DEV_VALIDATION_REQUEST:
> + if (copy_from_user(data, (void __user *)arg, sizeof(*data))) {
> + dev_err(dev, "failure on copy_from_user\n");
> + return -EFAULT;
> + }
> +
> + /* for bitstream */
> + dev_dbg(dev, "file_name=%s, status=%d\n",
> + (char *)data->com_paras.s_request.src, data->status);
> + scnprintf(filename, FILE_NAME_SIZE, "%s",
> + (char *)data->com_paras.s_request.src);
> + ret = request_firmware(&fw, filename, priv->client.dev);
> + if (ret) {
> + dev_err(dev, "error requesting firmware %s\n",
> + (char *)data->com_paras.s_request.src);
> + return -EFAULT;
> + }
> +
> + dev_dbg(dev, "FW size=%ld\n", fw->size);
> + s_buf = stratix10_svc_allocate_memory(priv->chan, fw->size);
> + if (!s_buf) {
> + dev_err(dev, "failed to allocate VAB buffer\n");
> + release_firmware(fw);
> + return -ENOMEM;
> + }
> +
> + memcpy(s_buf, fw->data, fw->size);
> +
> + msg->payload_length = fw->size;
> + release_firmware(fw);
> +
> + msg->command = COMMAND_FCS_REQUEST_SERVICE;
> + msg->payload = s_buf;
> + priv->client.receive_cb = fcs_vab_callback;
> +
> + ret = fcs_request_service(priv, (void *)msg,
> + FCS_REQUEST_TIMEOUT);
> + dev_dbg(dev, "fcs_request_service ret=%d\n", ret);
> + if (!ret && !priv->status) {
> + /* to query the complete status */
> + msg->command = COMMAND_POLL_SERVICE_STATUS;
> + priv->client.receive_cb = fcs_data_callback;
> + ret = fcs_request_service(priv, (void *)msg,
> + FCS_COMPLETED_TIMEOUT);
> + dev_dbg(dev, "fcs_request_service ret=%d\n", ret);
> + if (!ret && !priv->status)
> + data->status = 0;
> + else
> + data->status = priv->status;
> + } else
> + data->status = priv->status;
> +
> + if (copy_to_user((void __user *)arg, data, sizeof(*data))) {
> + dev_err(dev, "failure on copy_to_user\n");
> + fcs_close_services(priv, s_buf, NULL);
> + ret = -EFAULT;
> + }
> +
> + fcs_close_services(priv, s_buf, NULL);
> + break;
> +
> + case INTEL_FCS_DEV_SEND_CERTIFICATE:
> + if (copy_from_user(data, (void __user *)arg, sizeof(*data))) {
> + dev_err(dev, "failure on copy_from_user\n");
> + return -EFAULT;
> + }
> +
> + dev_dbg(dev, "Test=%d, Size=%d; Address=0x%p\n",
> + data->com_paras.c_request.test.test_bit,
> + data->com_paras.c_request.size,
> + data->com_paras.c_request.addr);
> +
> + /* Allocate memory for certificate + test word */
> + tsz = sizeof(struct intel_fcs_cert_test_word);
> + datasz = data->com_paras.s_request.size + tsz;
> +
> + s_buf = stratix10_svc_allocate_memory(priv->chan, datasz);
> + if (!s_buf) {
> + dev_err(dev, "failed to allocate VAB buffer\n");
> + return -ENOMEM;
> + }
> +
> + ps_buf = stratix10_svc_allocate_memory(priv->chan, PS_BUF_SIZE);
> + if (!ps_buf) {
> + dev_err(dev, "failed to allocate p-status buf\n");
> + stratix10_svc_free_memory(priv->chan, s_buf);
> + return -ENOMEM;
> + }
> +
> + /* Copy the test word */
> + memcpy(s_buf, &data->com_paras.c_request.test, tsz);
> +
> + /* Copy in the certificate data (skipping over the test word) */
> + ret = copy_from_user(s_buf + tsz,
> + data->com_paras.c_request.addr,
> + data->com_paras.s_request.size);
> + if (ret) {
> + dev_err(dev, "failed copy buf ret=%d\n", ret);
> + fcs_close_services(priv, s_buf, ps_buf);
> + return -EFAULT;
> + }
> +
> + msg->payload_length = datasz;
> + msg->command = COMMAND_FCS_SEND_CERTIFICATE;
> + msg->payload = s_buf;
> + priv->client.receive_cb = fcs_vab_callback;
> +
> + ret = fcs_request_service(priv, (void *)msg,
> + FCS_REQUEST_TIMEOUT);
> + dev_dbg(dev, "fcs_request_service ret=%d\n", ret);
> + if (!ret && !priv->status) {
> + /* to query the complete status */
> + msg->payload = ps_buf;
> + msg->payload_length = PS_BUF_SIZE;
> + msg->command = COMMAND_POLL_SERVICE_STATUS;
> + priv->client.receive_cb = fcs_data_callback;
> + ret = fcs_request_service(priv, (void *)msg,
> + FCS_COMPLETED_TIMEOUT);
> + dev_dbg(dev, "request service ret=%d\n", ret);
> + if (!ret && !priv->status)
> + data->status = 0;
> + else {
> + if (priv->kbuf)
> + data->com_paras.c_request.c_status =
> + (*(u32 *)priv->kbuf);
> + else
> + data->com_paras.c_request.c_status =
> + INVALID_STATUS;
> + }
> + } else
> + data->status = priv->status;
> +
> + if (copy_to_user((void __user *)arg, data, sizeof(*data))) {
> + dev_err(dev, "failure on copy_to_user\n");
> + fcs_close_services(priv, s_buf, NULL);
> + ret = -EFAULT;
> + }
> +
> + fcs_close_services(priv, s_buf, ps_buf);
> + break;
> +
> + case INTEL_FCS_DEV_RANDOM_NUMBER_GEN:
> + if (copy_from_user(data, (void __user *)arg, sizeof(*data))) {
> + dev_err(dev, "failure on copy_from_user\n");
> + return -EFAULT;
> + }
> +
> + s_buf = stratix10_svc_allocate_memory(priv->chan,
> + RANDOM_NUMBER_SIZE);
> + if (!s_buf) {
> + dev_err(dev, "failed to allocate RNG buffer\n");
> + return -ENOMEM;
> + }
> +
> + msg->command = COMMAND_FCS_RANDOM_NUMBER_GEN;
> + msg->payload = s_buf;
> + msg->payload_length = RANDOM_NUMBER_SIZE;
> + priv->client.receive_cb = fcs_data_callback;
> +
> + ret = fcs_request_service(priv, (void *)msg,
> + FCS_REQUEST_TIMEOUT);
> +
> + if (!ret && !priv->status) {
> + if (!priv->kbuf) {
> + dev_err(dev, "failure on kbuf\n");
> + fcs_close_services(priv, s_buf, NULL);
> + return -EFAULT;
> + }
> +
> + for (i = 0; i < 8; i++)
> + dev_dbg(dev, "output_data[%d]=%d\n", i,
> + *((int *)priv->kbuf + i));
> +
> + for (i = 0; i < 8; i++)
> + data->com_paras.rn_gen.rndm[i] =
> + *((int *)priv->kbuf + i);
> + data->status = priv->status;
> +
> + } else {
> + /* failed to get RNG */
> + data->status = priv->status;
> + }
> +
> + if (copy_to_user((void __user *)arg, data, sizeof(*data))) {
> + dev_err(dev, "failure on copy_to_user\n");
> + fcs_close_services(priv, s_buf, NULL);
> + ret = -EFAULT;
> + }
> +
> + fcs_close_services(priv, s_buf, NULL);
> + break;
> + case INTEL_FCS_DEV_GET_PROVISION_DATA:
> + if (copy_from_user(data, (void __user *)arg,
> + sizeof(*data))) {
> + dev_err(dev, "failure on copy_from_user\n");
> + return -EFAULT;
> + }
> +
> + s_buf = stratix10_svc_allocate_memory(priv->chan,
> + data->com_paras.gp_data.size);
> + if (!s_buf) {
> + dev_err(dev, "failed allocate provision buffer\n");
> + return -ENOMEM;
> + }
> +
> + msg->command = COMMAND_FCS_GET_PROVISION_DATA;
> + msg->payload = s_buf;
> + msg->payload_length = data->com_paras.gp_data.size;
> + priv->client.receive_cb = fcs_data_callback;
> +
> + ret = fcs_request_service(priv, (void *)msg,
> + FCS_REQUEST_TIMEOUT);
> + if (!ret && !priv->status) {
> + if (!priv->kbuf) {
> + dev_err(dev, "failure on kbuf\n");
> + fcs_close_services(priv, s_buf, NULL);
> + return -EFAULT;
> + }
> + data->com_paras.gp_data.size = priv->size;
> + memcpy(data->com_paras.gp_data.addr, priv->kbuf,
> + priv->size);
> + data->status = 0;
> + } else {
> + data->com_paras.gp_data.addr = NULL;
> + data->com_paras.gp_data.size = 0;
> + data->status = priv->status;
> + }
> +
> + if (copy_to_user((void __user *)arg, data, sizeof(*data))) {
> + dev_err(dev, "failure on copy_to_user\n");
> + fcs_close_services(priv, s_buf, NULL);
> + return -EFAULT;
> + }
> +
> + fcs_close_services(priv, s_buf, NULL);
> + break;
> + case INTEL_FCS_DEV_DATA_ENCRYPTION:
> + if (copy_from_user(data, (void __user *)arg, sizeof(*data))) {
> + dev_err(dev, "failure on copy_from_user\n");
> + return -EFAULT;
> + }
> +
> + if ((data->com_paras.d_encryption.src_size < MIN_SDOS_BUF_SZ) ||
> + (data->com_paras.d_encryption.src_size > MAX_SDOS_BUF_SZ)) {
> + dev_err(dev, "Invalid SDOS Buffer src size:%d\n",
> + data->com_paras.d_encryption.src_size);
> + return -EFAULT;
> + }
> +
> + if ((data->com_paras.d_encryption.dst_size < MIN_SDOS_BUF_SZ) ||
> + (data->com_paras.d_encryption.dst_size > MAX_SDOS_BUF_SZ)) {
> + dev_err(dev, "Invalid SDOS Buffer dst size:%d\n",
> + data->com_paras.d_encryption.dst_size);
> + return -EFAULT;
> + }
> +
> + /* allocate buffer for both source and destination */
> + s_buf = stratix10_svc_allocate_memory(priv->chan,
> + MAX_SDOS_BUF_SZ);
> + if (!s_buf) {
> + dev_err(dev, "failed allocate encrypt src buf\n");
> + return -ENOMEM;
> + }
> + d_buf = stratix10_svc_allocate_memory(priv->chan,
> + MAX_SDOS_BUF_SZ);
> + if (!d_buf) {
> + dev_err(dev, "failed allocate encrypt dst buf\n");
> + stratix10_svc_free_memory(priv->chan, s_buf);
> + return -ENOMEM;
> + }
> + ps_buf = stratix10_svc_allocate_memory(priv->chan, PS_BUF_SIZE);
> + if (!ps_buf) {
> + dev_err(dev, "failed allocate p-status buffer\n");
> + fcs_close_services(priv, s_buf, d_buf);
> + return -ENOMEM;
> + }
> + ret = copy_from_user(s_buf,
> + data->com_paras.d_encryption.src,
> + data->com_paras.d_encryption.src_size);
> + if (ret) {
> + dev_err(dev, "failure on copy_from_user\n");
> + fcs_close_services(priv, ps_buf, NULL);
> + fcs_close_services(priv, s_buf, d_buf);
> + return -ENOMEM;
> + }
> +
> + msg->command = COMMAND_FCS_DATA_ENCRYPTION;
> + msg->payload = s_buf;
> + msg->payload_length =
> + data->com_paras.d_encryption.src_size;
> + msg->payload_output = d_buf;
> + msg->payload_length_output =
> + data->com_paras.d_encryption.dst_size;
> + priv->client.receive_cb = fcs_vab_callback;
> +
> + ret = fcs_request_service(priv, (void *)msg,
> + FCS_REQUEST_TIMEOUT);
> + if (!ret && !priv->status) {
> + msg->payload = ps_buf;
> + msg->payload_length = PS_BUF_SIZE;
> + msg->command = COMMAND_POLL_SERVICE_STATUS;
> +
> + priv->client.receive_cb = fcs_data_callback;
> + ret = fcs_request_service(priv, (void *)msg,
> + FCS_COMPLETED_TIMEOUT);
> + dev_dbg(dev, "request service ret=%d\n", ret);
> +
> + if (!ret && !priv->status) {
> + if (!priv->kbuf) {
> + dev_err(dev, "failure on kbuf\n");
> + fcs_close_services(priv, ps_buf, NULL);
> + fcs_close_services(priv, s_buf, d_buf);
> + return -EFAULT;
> + }
> + buf_sz = *(unsigned int *)priv->kbuf;
> + data->com_paras.d_encryption.dst_size = buf_sz;
> + memcpy(data->com_paras.d_encryption.dst,
> + d_buf, buf_sz);
> + data->status = 0;
> + } else {
> + data->com_paras.d_encryption.dst = NULL;
> + data->com_paras.d_encryption.dst_size = 0;
> + data->status = priv->status;
> + }
> + } else {
> + data->com_paras.d_encryption.dst = NULL;
> + data->com_paras.d_encryption.dst_size = 0;
> + data->status = priv->status;
> + }
> +
> + if (copy_to_user((void __user *)arg, data, sizeof(*data))) {
> + dev_err(dev, "failure on copy_to_user\n");
> + fcs_close_services(priv, ps_buf, NULL);
> + fcs_close_services(priv, s_buf, d_buf);
> + ret = -EFAULT;
> + }
> +
> + fcs_close_services(priv, ps_buf, NULL);
> + fcs_close_services(priv, s_buf, d_buf);
> + break;
> + case INTEL_FCS_DEV_DATA_DECRYPTION:
> + if (copy_from_user(data, (void __user *)arg, sizeof(*data))) {
> + dev_err(dev, "failure on copy_from_user\n");
> + return -EFAULT;
> + }
> +
> + if ((data->com_paras.d_encryption.src_size < MIN_SDOS_BUF_SZ) ||
> + (data->com_paras.d_encryption.src_size > MAX_SDOS_BUF_SZ)) {
> + dev_err(dev, "Invalid SDOS Buffer src size:%d\n",
> + data->com_paras.d_encryption.src_size);
> + return -EFAULT;
> + }
> +
> + if ((data->com_paras.d_encryption.dst_size < MIN_SDOS_BUF_SZ) ||
> + (data->com_paras.d_encryption.dst_size > MAX_SDOS_BUF_SZ)) {
> + dev_err(dev, "Invalid SDOS Buffer dst size:%d\n",
> + data->com_paras.d_encryption.dst_size);
> + return -EFAULT;
> + }
> +
> + /* allocate buffer for both source and destination */
> + s_buf = stratix10_svc_allocate_memory(priv->chan,
> + MAX_SDOS_BUF_SZ);
> + if (!s_buf) {
> + dev_err(dev, "failed allocate decrypt src buf\n");
> + return -ENOMEM;
> + }
> + d_buf = stratix10_svc_allocate_memory(priv->chan,
> + MAX_SDOS_BUF_SZ);
> + if (!d_buf) {
> + dev_err(dev, "failed allocate decrypt dst buf\n");
> + stratix10_svc_free_memory(priv->chan, s_buf);
> + return -ENOMEM;
> + }
> +
> + ps_buf = stratix10_svc_allocate_memory(priv->chan,
> + PS_BUF_SIZE);
> + if (!ps_buf) {
> + dev_err(dev, "failed allocate p-status buffer\n");
> + fcs_close_services(priv, s_buf, d_buf);
> + return -ENOMEM;
> + }
> +
> + ret = copy_from_user(s_buf,
> + data->com_paras.d_decryption.src,
> + data->com_paras.d_decryption.src_size);
> + if (ret) {
> + dev_err(dev, "failure on copy_from_user\n");
> + fcs_close_services(priv, ps_buf, NULL);
> + fcs_close_services(priv, s_buf, d_buf);
> + return -EFAULT;
> + }
> +
> + msg->command = COMMAND_FCS_DATA_DECRYPTION;
> + msg->payload = s_buf;
> + msg->payload_length =
> + data->com_paras.d_decryption.src_size;
> + msg->payload_output = d_buf;
> + msg->payload_length_output =
> + data->com_paras.d_decryption.dst_size;
> + priv->client.receive_cb = fcs_vab_callback;
> +
> + ret = fcs_request_service(priv, (void *)msg,
> + FCS_REQUEST_TIMEOUT);
> + if (!ret && !priv->status) {
> + msg->command = COMMAND_POLL_SERVICE_STATUS;
> + msg->payload = ps_buf;
> + msg->payload_length = PS_BUF_SIZE;
> + priv->client.receive_cb = fcs_data_callback;
> + ret = fcs_request_service(priv, (void *)msg,
> + FCS_COMPLETED_TIMEOUT);
> + dev_dbg(dev, "request service ret=%d\n", ret);
> + if (!ret && !priv->status) {
> + if (!priv->kbuf) {
> + dev_err(dev, "failure on kbuf\n");
> + fcs_close_services(priv, ps_buf, NULL);
> + fcs_close_services(priv, s_buf, d_buf);
> + return -EFAULT;
> + }
> + buf_sz = *((unsigned int *)priv->kbuf);
> + memcpy(data->com_paras.d_decryption.dst,
> + d_buf, buf_sz);
> + data->com_paras.d_decryption.dst_size = buf_sz;
> + data->status = 0;
> + } else {
> + data->com_paras.d_decryption.dst = NULL;
> + data->com_paras.d_decryption.dst_size = 0;
> + data->status = priv->status;
> + }
> + } else {
> + data->com_paras.d_decryption.dst = NULL;
> + data->com_paras.d_decryption.dst_size = 0;
> + data->status = priv->status;
> + }
> +
> + if (copy_to_user((void __user *)arg, data, sizeof(*data))) {
> + dev_err(dev, "failure on copy_to_user\n");
> + fcs_close_services(priv, ps_buf, NULL);
> + fcs_close_services(priv, s_buf, d_buf);
> + ret = -EFAULT;
> + }
> +
> + fcs_close_services(priv, ps_buf, NULL);
> + fcs_close_services(priv, s_buf, d_buf);
> + break;
> + default:
> + dev_warn(dev, "shouldn't be here [0x%x]\n", cmd);
> + break;
> + }
> +
> + return ret;
> +}
> +
> +static int fcs_open(struct inode *inode, struct file *file)
> +{
> + pr_debug("%s\n", __func__);
> +
> + return 0;
> +}
> +
> +static int fcs_close(struct inode *inode, struct file *file)
> +{
> +
> + pr_debug("%s\n", __func__);
> +
> + return 0;
> +}

If you do nothing in an open/close function, do not include them, they
are not needed.

> +
> +static const struct file_operations fcs_fops = {
> + .owner = THIS_MODULE,
> + .unlocked_ioctl = fcs_ioctl,
> + .open = fcs_open,
> + .release = fcs_close,
> +};
> +
> +static int fcs_driver_probe(struct platform_device *pdev)
> +{
> + struct device *dev = &pdev->dev;
> + struct intel_fcs_priv *priv;
> + int ret;
> +
> + priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
> + if (!priv)
> + return -ENOMEM;
> +
> + priv->client.dev = dev;
> + priv->client.receive_cb = NULL;
> + priv->client.priv = priv;
> + priv->status = INVALID_STATUS;
> +
> + mutex_init(&priv->lock);
> + priv->chan = stratix10_svc_request_channel_byname(&priv->client,
> + SVC_CLIENT_FCS);
> + if (IS_ERR(priv->chan)) {
> + dev_err(dev, "couldn't get service channel %s\n",
> + SVC_CLIENT_FCS);
> + return PTR_ERR(priv->chan);
> + }
> +
> + priv->miscdev.minor = MISC_DYNAMIC_MINOR;
> + priv->miscdev.name = "fcs";
> + priv->miscdev.fops = &fcs_fops;
> +
> + init_completion(&priv->completion);
> +
> + platform_set_drvdata(pdev, priv);
> +
> + ret = misc_register(&priv->miscdev);
> + if (ret) {
> + dev_err(dev, "can't register on minor=%d\n",
> + MISC_DYNAMIC_MINOR);
> + return ret;
> + }
> +
> + return 0;
> +}
> +
> +static int fcs_driver_remove(struct platform_device *pdev)
> +{
> + struct intel_fcs_priv *priv = platform_get_drvdata(pdev);
> +
> + misc_deregister(&priv->miscdev);
> + stratix10_svc_free_channel(priv->chan);
> +
> + return 0;
> +}
> +
> +static struct platform_driver fcs_driver = {
> + .probe = fcs_driver_probe,
> + .remove = fcs_driver_remove,
> + .driver = {
> + .name = "intel-fcs",
> + },
> +};
> +
> +module_platform_driver(fcs_driver);
> +
> +MODULE_LICENSE("GPL v2");
> +MODULE_DESCRIPTION("Intel FGPA Crypto Services Driver");
> +MODULE_AUTHOR("Richard Gong <[email protected]>");
> +
> diff --git a/include/uapi/linux/intel-fcs_ioctl.h b/include/uapi/linux/intel-fcs_ioctl.h
> new file mode 100644
> index 00000000..4d530ec
> --- /dev/null
> +++ b/include/uapi/linux/intel-fcs_ioctl.h
> @@ -0,0 +1,222 @@
> +/* SPDX-License-Identifier: GPL-2.0 WITH Linux-syscall-note */
> +/*
> + * Copyright (C) 2020, Intel Corporation
> + */
> +
> +#ifndef __INTEL_FCS_IOCTL_H
> +#define __INTEL_FCS_IOCTL_H
> +
> +#include <linux/types.h>
> +
> +#define INTEL_FCS_IOCTL 0xA2
> +
> +/**
> + * enum fcs_vab_img_type - enumeration of image types
> + * @INTEL_FCS_IMAGE_HPS: Image to validate is HPS image
> + * @INTEL_FCS_IMAGE_BITSTREAM: Image to validate is bitstream
> + */
> +enum fcs_vab_img_type {
> + INTEL_FCS_IMAGE_HPS = 0,
> + INTEL_FCS_IMAGE_BITSTREAM = 1
> +};
> +
> +/**
> + * enum fcs_certificate_test - enumeration of certificate test
> + * @INTEL_FCS_NO_TEST: Write to eFuses
> + * @INTEL_FCS_TEST: Write to cache, do not write eFuses
> + */
> +enum fcs_certificate_test {
> + INTEL_FCS_NO_TEST = 0,
> + INTEL_FCS_TEST = 1
> +};
> +
> +/**
> + * struct intel_fcs_cert_test_word - certificate test word
> + * @test_bit: if set, do not write fuses, write to cache only.
> + * @rsvd: write as 0
> + */
> +struct intel_fcs_cert_test_word {
> + __u32 test_bit:1;
> + __u32 rsvd:31;
> +};

What endian is this? Bit fields for ioctls is almost always a very bad
idea when done like this. Pick it apart in the kernel please.


> +
> +/**
> + * struct fcs_validation_request - validate HPS or bitstream image
> + * @so_type: the type of signed object, 0 for HPS and 1 for bitstream
> + * @src: the source of signed object,
> + * for HPS, this is the virtual address of the signed source
> + * for Bitstream, this is path of the signed source, the default
> + * path is /lib/firmware
> + * @size: the size of the signed object
> + */
> +struct fcs_validation_request {
> + enum fcs_vab_img_type so_type;
> + void *src;

void *????

Shouldn't you be using a portable type, otherwise this, and all of your
other pointers are going to break on 32/64 split user/kernel systems,
right?



> + __u32 size;
> +};
> +
> +/**
> + * struct fcs_key_manage_request - Request key management from SDM
> + * @addr: the virtual address of the signed object,
> + * @size: the size of the signed object
> + */
> +struct fcs_key_manage_request {
> + void *addr;
> + __u32 size;
> +};
> +
> +/**
> + * struct fcs_certificate_request - Certificate request to SDM
> + * @test: test bit (1 if want to write to cache instead of fuses)
> + * @addr: the virtual address of the signed object,
> + * @size: the size of the signed object
> + * @c_status: returned certificate status
> + */
> +struct fcs_certificate_request {
> + struct intel_fcs_cert_test_word test;
> + void *addr;
> + __u32 size;
> + __u32 c_status;
> +};
> +
> +/**
> + * struct fcs_data_encryption - aes data encryption command layout
> + * @src: the virtual address of the input data
> + * @src_size: the size of the unencrypted source
> + * @dst: the virtual address of the output data
> + * @dst_size: the size of the encrypted result
> + */
> +struct fcs_data_encryption {
> + void *src;
> + __u32 src_size;
> + void *dst;
> + __u32 dst_size;
> +};

Why put holes in your structures when you do not need them?

Please get all of these structures reviewed by someone within Intel who
understands how user/kernel apis work. :(

greg k-h

2020-09-02 02:30:06

by Richard Gong

[permalink] [raw]
Subject: Re: [PATCHv2 2/2] misc: add Intel SoCFPGA crypto service driver


Hi Greg,

Many thanks for your review comments!

On 8/28/20 5:47 AM, Greg KH wrote:
> On Mon, Aug 17, 2020 at 08:47:30AM -0500, [email protected] wrote:
>> From: Richard Gong <[email protected]>
>>
>> Add Intel FPGA crypto service (FCS) driver to support new crypto services
>> on Intel SoCFPGA platforms.
>>
>> The crypto services include security certificate, image boot validation,
>> security key cancellation, get provision data, random number generation,
>> advance encrtption standard (AES) encryption and decryption services.
>>
>> To perform supporting crypto features on Intel SoCFPGA platforms, Linux
>> user-space application interacts with FPGA crypto service (FCS) driver via
>> structures defined in include/uapi/linux/intel_fcs-ioctl.h.
>>
>> The application allocates spaces for IOCTL structure to hold the contents
>> or points to the data that FCS driver needs, uses IOCTL calls to passes
>> data to kernel FCS driver for processing at low level firmware and get
>> processed data or status back form the low level firmware via FCS driver.
>>
>> The user-space application named as fcs_client is at
>> https://github.com/altera-opensource/fcs_apps/tree/fcs_client.
>
> Ugh, a custom userspace api just for one crypto driver? Why can't this
> just be a userspace driver? Why does it have to be in the kernel?

We provide an example of a user space application that performs FPGA
crypto functions.

FPGA crypto firmware is running at exception level 3 (EL3).

We have added a new FPGA crypto service (FCS) driver to the kernel so
that FCS driver acts as one of clients of Intel Stratix10 service layer
driver and uses service layer driver to perform crypto functions.

>
>> @@ -0,0 +1,709 @@
>> +// SPDX-License-Identifier: GPL-2.0
>> +/*
>> + * Copyright (C) 2020, Intel Corporation
>> + */
>> +
>> +#include <linux/arm-smccc.h>
>> +#include <linux/bitfield.h>
>> +#include <linux/completion.h>
>> +#include <linux/firmware.h>
>> +#include <linux/fs.h>
>> +#include <linux/kobject.h>
>
> Why is kobject.h needed here?

My fault, I will remove that in next submission.
>
>> +#include <linux/miscdevice.h>
>> +#include <linux/module.h>
>> +#include <linux/mutex.h>
>> +#include <linux/of.h>
>> +#include <linux/of_platform.h>
>> +#include <linux/platform_device.h>
>> +#include <linux/firmware/intel/stratix10-svc-client.h>
>> +#include <linux/string.h>
>> +#include <linux/slab.h>
>> +#include <linux/sysfs.h>
>> +#include <linux/uaccess.h>
>> +
>> +#include <uapi/linux/intel-fcs_ioctl.h>
>> +
>> +#define RANDOM_NUMBER_SIZE 32
>> +#define FILE_NAME_SIZE 32
>> +#define PS_BUF_SIZE 64
>> +#define INVALID_STATUS 0xff
>> +
>> +#define MIN_SDOS_BUF_SZ 16
>> +#define MAX_SDOS_BUF_SZ 32768
>> +
>> +#define FCS_REQUEST_TIMEOUT (msecs_to_jiffies(SVC_FCS_REQUEST_TIMEOUT_MS))
>> +#define FCS_COMPLETED_TIMEOUT (msecs_to_jiffies(SVC_COMPLETED_TIMEOUT_MS))
>> +
>> +typedef void (*fcs_callback)(struct stratix10_svc_client *client,
>> + struct stratix10_svc_cb_data *data);
>> +
>> +struct intel_fcs_priv {
>> + struct stratix10_svc_chan *chan;
>> + struct stratix10_svc_client client;
>> + struct completion completion;
>> + struct mutex lock;
>> + struct miscdevice miscdev;
>> + unsigned int status;
>> + void *kbuf;
>> + unsigned int size;
>> +};
>> +
>> +static void fcs_data_callback(struct stratix10_svc_client *client,
>> + struct stratix10_svc_cb_data *data)
>> +{
>> + struct intel_fcs_priv *priv = client->priv;
>> +
>> + if ((data->status == BIT(SVC_STATUS_OK)) ||
>> + (data->status == BIT(SVC_STATUS_COMPLETED))) {
>> + priv->status = 0;
>> + priv->kbuf = data->kaddr2;
>> + priv->size = *((unsigned int *)data->kaddr3);
>> + } else if (data->status == BIT(SVC_STATUS_ERROR)) {
>> + priv->status = *((unsigned int *)data->kaddr1);
>> + dev_err(client->dev, "error, mbox_error=0x%x\n", priv->status);
>> + priv->kbuf = data->kaddr2;
>> + priv->size = (data->kaddr3) ?
>> + *((unsigned int *)data->kaddr3) : 0;
>> + } else {
>> + dev_err(client->dev, "rejected\n");
>> + priv->status = -EINVAL;
>> + priv->kbuf = NULL;
>> + priv->size = 0;
>> + }
>> +
>> + complete(&priv->completion);
>> +}
>> +
>> +static void fcs_vab_callback(struct stratix10_svc_client *client,
>> + struct stratix10_svc_cb_data *data)
>> +{
>> + struct intel_fcs_priv *priv = client->priv;
>> +
>> + priv->status = 0;
>> +
>> + if (data->status == BIT(SVC_STATUS_INVALID_PARAM)) {
>> + priv->status = -EINVAL;
>> + dev_warn(client->dev, "rejected, invalid param\n");
>> + } else if (data->status == BIT(SVC_STATUS_ERROR)) {
>> + priv->status = *((unsigned int *)data->kaddr1);
>> + dev_err(client->dev, "mbox_error=0x%x\n", priv->status);
>> + } else if (data->status == BIT(SVC_STATUS_BUSY)) {
>> + priv->status = -ETIMEDOUT;
>> + dev_err(client->dev, "timeout to get completed status\n");
>> + }
>> +
>> + complete(&priv->completion);
>> +}
>> +
>> +static int fcs_request_service(struct intel_fcs_priv *priv,
>> + void *msg, unsigned long timeout)
>> +{
>> + struct stratix10_svc_client_msg *p_msg =
>> + (struct stratix10_svc_client_msg *)msg;
>> + int ret;
>> +
>> + mutex_lock(&priv->lock);
>> + reinit_completion(&priv->completion);
>> +
>> + ret = stratix10_svc_send(priv->chan, p_msg);
>> + if (ret) {
>> + ret = -EINVAL;
>> + goto unlock;
>> + }
>> +
>> + ret = wait_for_completion_timeout(&priv->completion,
>> + timeout);
>> + if (!ret) {
>> + dev_err(priv->client.dev,
>> + "timeout waiting for SMC call\n");
>> + ret = -ETIMEDOUT;
>> + } else
>> + ret = 0;
>> +
>> +unlock:
>> + mutex_unlock(&priv->lock);
>> + return ret;
>> +}
>> +
>> +static void fcs_close_services(struct intel_fcs_priv *priv,
>> + void *sbuf, void *dbuf)
>> +{
>> + if (sbuf)
>> + stratix10_svc_free_memory(priv->chan, sbuf);
>> +
>> + if (dbuf)
>> + stratix10_svc_free_memory(priv->chan, dbuf);
>> +
>> + stratix10_svc_done(priv->chan);
>> +}
>> +
>> +static long fcs_ioctl(struct file *file, unsigned int cmd,
>> + unsigned long arg)
>> +{
>> + struct intel_fcs_dev_ioctl *data;
>> + struct intel_fcs_priv *priv;
>> + struct device *dev;
>> + struct stratix10_svc_client_msg *msg;
>> + const struct firmware *fw;
>> + char filename[FILE_NAME_SIZE];
>> + size_t tsz, datasz;
>> + void *s_buf;
>> + void *d_buf;
>> + void *ps_buf;
>> + unsigned int buf_sz;
>> + int ret = 0;
>> + int i;
>
> You seem to "trust" the structure data from userspace a lot in this
> function. Please audit each one of these calls again, I think there are
> some places where you can do "bad things"...

Thanks, I will take a closer look again.

>
>> +
>> + priv = container_of(file->private_data, struct intel_fcs_priv, miscdev);
>> + dev = priv->client.dev;
>> +
>> + data = devm_kzalloc(dev, sizeof(*data), GFP_KERNEL);
>> + if (!data)
>> + return -ENOMEM;
>> +
>> + msg = devm_kzalloc(dev, sizeof(*msg), GFP_KERNEL);
>> + if (!msg)
>> + return -ENOMEM;
>> +
>> + switch (cmd) {
>> + case INTEL_FCS_DEV_VALIDATION_REQUEST:
>> + if (copy_from_user(data, (void __user *)arg, sizeof(*data))) {
>> + dev_err(dev, "failure on copy_from_user\n");
>> + return -EFAULT;
>> + }
>> +
>> + /* for bitstream */
>> + dev_dbg(dev, "file_name=%s, status=%d\n",
>> + (char *)data->com_paras.s_request.src, data->status);
>> + scnprintf(filename, FILE_NAME_SIZE, "%s",
>> + (char *)data->com_paras.s_request.src);
>> + ret = request_firmware(&fw, filename, priv->client.dev);
>> + if (ret) {
>> + dev_err(dev, "error requesting firmware %s\n",
>> + (char *)data->com_paras.s_request.src);
>> + return -EFAULT;
>> + }
>> +
>> + dev_dbg(dev, "FW size=%ld\n", fw->size);
>> + s_buf = stratix10_svc_allocate_memory(priv->chan, fw->size);
>> + if (!s_buf) {
>> + dev_err(dev, "failed to allocate VAB buffer\n");
>> + release_firmware(fw);
>> + return -ENOMEM;
>> + }
>> +
>> + memcpy(s_buf, fw->data, fw->size);
>> +
>> + msg->payload_length = fw->size;
>> + release_firmware(fw);
>> +
>> + msg->command = COMMAND_FCS_REQUEST_SERVICE;
>> + msg->payload = s_buf;
>> + priv->client.receive_cb = fcs_vab_callback;
>> +
>> + ret = fcs_request_service(priv, (void *)msg,
>> + FCS_REQUEST_TIMEOUT);
>> + dev_dbg(dev, "fcs_request_service ret=%d\n", ret);
>> + if (!ret && !priv->status) {
>> + /* to query the complete status */
>> + msg->command = COMMAND_POLL_SERVICE_STATUS;
>> + priv->client.receive_cb = fcs_data_callback;
>> + ret = fcs_request_service(priv, (void *)msg,
>> + FCS_COMPLETED_TIMEOUT);
>> + dev_dbg(dev, "fcs_request_service ret=%d\n", ret);
>> + if (!ret && !priv->status)
>> + data->status = 0;
>> + else
>> + data->status = priv->status;
>> + } else
>> + data->status = priv->status;
>> +
>> + if (copy_to_user((void __user *)arg, data, sizeof(*data))) {
>> + dev_err(dev, "failure on copy_to_user\n");
>> + fcs_close_services(priv, s_buf, NULL);
>> + ret = -EFAULT;
>> + }
>> +
>> + fcs_close_services(priv, s_buf, NULL);
>> + break;
>> +
>> + case INTEL_FCS_DEV_SEND_CERTIFICATE:
>> + if (copy_from_user(data, (void __user *)arg, sizeof(*data))) {
>> + dev_err(dev, "failure on copy_from_user\n");
>> + return -EFAULT;
>> + }
>> +
>> + dev_dbg(dev, "Test=%d, Size=%d; Address=0x%p\n",
>> + data->com_paras.c_request.test.test_bit,
>> + data->com_paras.c_request.size,
>> + data->com_paras.c_request.addr);
>> +
>> + /* Allocate memory for certificate + test word */
>> + tsz = sizeof(struct intel_fcs_cert_test_word);
>> + datasz = data->com_paras.s_request.size + tsz;
>> +
>> + s_buf = stratix10_svc_allocate_memory(priv->chan, datasz);
>> + if (!s_buf) {
>> + dev_err(dev, "failed to allocate VAB buffer\n");
>> + return -ENOMEM;
>> + }
>> +
>> + ps_buf = stratix10_svc_allocate_memory(priv->chan, PS_BUF_SIZE);
>> + if (!ps_buf) {
>> + dev_err(dev, "failed to allocate p-status buf\n");
>> + stratix10_svc_free_memory(priv->chan, s_buf);
>> + return -ENOMEM;
>> + }
>> +
>> + /* Copy the test word */
>> + memcpy(s_buf, &data->com_paras.c_request.test, tsz);
>> +
>> + /* Copy in the certificate data (skipping over the test word) */
>> + ret = copy_from_user(s_buf + tsz,
>> + data->com_paras.c_request.addr,
>> + data->com_paras.s_request.size);
>> + if (ret) {
>> + dev_err(dev, "failed copy buf ret=%d\n", ret);
>> + fcs_close_services(priv, s_buf, ps_buf);
>> + return -EFAULT;
>> + }
>> +
>> + msg->payload_length = datasz;
>> + msg->command = COMMAND_FCS_SEND_CERTIFICATE;
>> + msg->payload = s_buf;
>> + priv->client.receive_cb = fcs_vab_callback;
>> +
>> + ret = fcs_request_service(priv, (void *)msg,
>> + FCS_REQUEST_TIMEOUT);
>> + dev_dbg(dev, "fcs_request_service ret=%d\n", ret);
>> + if (!ret && !priv->status) {
>> + /* to query the complete status */
>> + msg->payload = ps_buf;
>> + msg->payload_length = PS_BUF_SIZE;
>> + msg->command = COMMAND_POLL_SERVICE_STATUS;
>> + priv->client.receive_cb = fcs_data_callback;
>> + ret = fcs_request_service(priv, (void *)msg,
>> + FCS_COMPLETED_TIMEOUT);
>> + dev_dbg(dev, "request service ret=%d\n", ret);
>> + if (!ret && !priv->status)
>> + data->status = 0;
>> + else {
>> + if (priv->kbuf)
>> + data->com_paras.c_request.c_status =
>> + (*(u32 *)priv->kbuf);
>> + else
>> + data->com_paras.c_request.c_status =
>> + INVALID_STATUS;
>> + }
>> + } else
>> + data->status = priv->status;
>> +
>> + if (copy_to_user((void __user *)arg, data, sizeof(*data))) {
>> + dev_err(dev, "failure on copy_to_user\n");
>> + fcs_close_services(priv, s_buf, NULL);
>> + ret = -EFAULT;
>> + }
>> +
>> + fcs_close_services(priv, s_buf, ps_buf);
>> + break;
>> +
>> + case INTEL_FCS_DEV_RANDOM_NUMBER_GEN:
>> + if (copy_from_user(data, (void __user *)arg, sizeof(*data))) {
>> + dev_err(dev, "failure on copy_from_user\n");
>> + return -EFAULT;
>> + }
>> +
>> + s_buf = stratix10_svc_allocate_memory(priv->chan,
>> + RANDOM_NUMBER_SIZE);
>> + if (!s_buf) {
>> + dev_err(dev, "failed to allocate RNG buffer\n");
>> + return -ENOMEM;
>> + }
>> +
>> + msg->command = COMMAND_FCS_RANDOM_NUMBER_GEN;
>> + msg->payload = s_buf;
>> + msg->payload_length = RANDOM_NUMBER_SIZE;
>> + priv->client.receive_cb = fcs_data_callback;
>> +
>> + ret = fcs_request_service(priv, (void *)msg,
>> + FCS_REQUEST_TIMEOUT);
>> +
>> + if (!ret && !priv->status) {
>> + if (!priv->kbuf) {
>> + dev_err(dev, "failure on kbuf\n");
>> + fcs_close_services(priv, s_buf, NULL);
>> + return -EFAULT;
>> + }
>> +
>> + for (i = 0; i < 8; i++)
>> + dev_dbg(dev, "output_data[%d]=%d\n", i,
>> + *((int *)priv->kbuf + i));
>> +
>> + for (i = 0; i < 8; i++)
>> + data->com_paras.rn_gen.rndm[i] =
>> + *((int *)priv->kbuf + i);
>> + data->status = priv->status;
>> +
>> + } else {
>> + /* failed to get RNG */
>> + data->status = priv->status;
>> + }
>> +
>> + if (copy_to_user((void __user *)arg, data, sizeof(*data))) {
>> + dev_err(dev, "failure on copy_to_user\n");
>> + fcs_close_services(priv, s_buf, NULL);
>> + ret = -EFAULT;
>> + }
>> +
>> + fcs_close_services(priv, s_buf, NULL);
>> + break;
>> + case INTEL_FCS_DEV_GET_PROVISION_DATA:
>> + if (copy_from_user(data, (void __user *)arg,
>> + sizeof(*data))) {
>> + dev_err(dev, "failure on copy_from_user\n");
>> + return -EFAULT;
>> + }
>> +
>> + s_buf = stratix10_svc_allocate_memory(priv->chan,
>> + data->com_paras.gp_data.size);
>> + if (!s_buf) {
>> + dev_err(dev, "failed allocate provision buffer\n");
>> + return -ENOMEM;
>> + }
>> +
>> + msg->command = COMMAND_FCS_GET_PROVISION_DATA;
>> + msg->payload = s_buf;
>> + msg->payload_length = data->com_paras.gp_data.size;
>> + priv->client.receive_cb = fcs_data_callback;
>> +
>> + ret = fcs_request_service(priv, (void *)msg,
>> + FCS_REQUEST_TIMEOUT);
>> + if (!ret && !priv->status) {
>> + if (!priv->kbuf) {
>> + dev_err(dev, "failure on kbuf\n");
>> + fcs_close_services(priv, s_buf, NULL);
>> + return -EFAULT;
>> + }
>> + data->com_paras.gp_data.size = priv->size;
>> + memcpy(data->com_paras.gp_data.addr, priv->kbuf,
>> + priv->size);
>> + data->status = 0;
>> + } else {
>> + data->com_paras.gp_data.addr = NULL;
>> + data->com_paras.gp_data.size = 0;
>> + data->status = priv->status;
>> + }
>> +
>> + if (copy_to_user((void __user *)arg, data, sizeof(*data))) {
>> + dev_err(dev, "failure on copy_to_user\n");
>> + fcs_close_services(priv, s_buf, NULL);
>> + return -EFAULT;
>> + }
>> +
>> + fcs_close_services(priv, s_buf, NULL);
>> + break;
>> + case INTEL_FCS_DEV_DATA_ENCRYPTION:
>> + if (copy_from_user(data, (void __user *)arg, sizeof(*data))) {
>> + dev_err(dev, "failure on copy_from_user\n");
>> + return -EFAULT;
>> + }
>> +
>> + if ((data->com_paras.d_encryption.src_size < MIN_SDOS_BUF_SZ) ||
>> + (data->com_paras.d_encryption.src_size > MAX_SDOS_BUF_SZ)) {
>> + dev_err(dev, "Invalid SDOS Buffer src size:%d\n",
>> + data->com_paras.d_encryption.src_size);
>> + return -EFAULT;
>> + }
>> +
>> + if ((data->com_paras.d_encryption.dst_size < MIN_SDOS_BUF_SZ) ||
>> + (data->com_paras.d_encryption.dst_size > MAX_SDOS_BUF_SZ)) {
>> + dev_err(dev, "Invalid SDOS Buffer dst size:%d\n",
>> + data->com_paras.d_encryption.dst_size);
>> + return -EFAULT;
>> + }
>> +
>> + /* allocate buffer for both source and destination */
>> + s_buf = stratix10_svc_allocate_memory(priv->chan,
>> + MAX_SDOS_BUF_SZ);
>> + if (!s_buf) {
>> + dev_err(dev, "failed allocate encrypt src buf\n");
>> + return -ENOMEM;
>> + }
>> + d_buf = stratix10_svc_allocate_memory(priv->chan,
>> + MAX_SDOS_BUF_SZ);
>> + if (!d_buf) {
>> + dev_err(dev, "failed allocate encrypt dst buf\n");
>> + stratix10_svc_free_memory(priv->chan, s_buf);
>> + return -ENOMEM;
>> + }
>> + ps_buf = stratix10_svc_allocate_memory(priv->chan, PS_BUF_SIZE);
>> + if (!ps_buf) {
>> + dev_err(dev, "failed allocate p-status buffer\n");
>> + fcs_close_services(priv, s_buf, d_buf);
>> + return -ENOMEM;
>> + }
>> + ret = copy_from_user(s_buf,
>> + data->com_paras.d_encryption.src,
>> + data->com_paras.d_encryption.src_size);
>> + if (ret) {
>> + dev_err(dev, "failure on copy_from_user\n");
>> + fcs_close_services(priv, ps_buf, NULL);
>> + fcs_close_services(priv, s_buf, d_buf);
>> + return -ENOMEM;
>> + }
>> +
>> + msg->command = COMMAND_FCS_DATA_ENCRYPTION;
>> + msg->payload = s_buf;
>> + msg->payload_length =
>> + data->com_paras.d_encryption.src_size;
>> + msg->payload_output = d_buf;
>> + msg->payload_length_output =
>> + data->com_paras.d_encryption.dst_size;
>> + priv->client.receive_cb = fcs_vab_callback;
>> +
>> + ret = fcs_request_service(priv, (void *)msg,
>> + FCS_REQUEST_TIMEOUT);
>> + if (!ret && !priv->status) {
>> + msg->payload = ps_buf;
>> + msg->payload_length = PS_BUF_SIZE;
>> + msg->command = COMMAND_POLL_SERVICE_STATUS;
>> +
>> + priv->client.receive_cb = fcs_data_callback;
>> + ret = fcs_request_service(priv, (void *)msg,
>> + FCS_COMPLETED_TIMEOUT);
>> + dev_dbg(dev, "request service ret=%d\n", ret);
>> +
>> + if (!ret && !priv->status) {
>> + if (!priv->kbuf) {
>> + dev_err(dev, "failure on kbuf\n");
>> + fcs_close_services(priv, ps_buf, NULL);
>> + fcs_close_services(priv, s_buf, d_buf);
>> + return -EFAULT;
>> + }
>> + buf_sz = *(unsigned int *)priv->kbuf;
>> + data->com_paras.d_encryption.dst_size = buf_sz;
>> + memcpy(data->com_paras.d_encryption.dst,
>> + d_buf, buf_sz);
>> + data->status = 0;
>> + } else {
>> + data->com_paras.d_encryption.dst = NULL;
>> + data->com_paras.d_encryption.dst_size = 0;
>> + data->status = priv->status;
>> + }
>> + } else {
>> + data->com_paras.d_encryption.dst = NULL;
>> + data->com_paras.d_encryption.dst_size = 0;
>> + data->status = priv->status;
>> + }
>> +
>> + if (copy_to_user((void __user *)arg, data, sizeof(*data))) {
>> + dev_err(dev, "failure on copy_to_user\n");
>> + fcs_close_services(priv, ps_buf, NULL);
>> + fcs_close_services(priv, s_buf, d_buf);
>> + ret = -EFAULT;
>> + }
>> +
>> + fcs_close_services(priv, ps_buf, NULL);
>> + fcs_close_services(priv, s_buf, d_buf);
>> + break;
>> + case INTEL_FCS_DEV_DATA_DECRYPTION:
>> + if (copy_from_user(data, (void __user *)arg, sizeof(*data))) {
>> + dev_err(dev, "failure on copy_from_user\n");
>> + return -EFAULT;
>> + }
>> +
>> + if ((data->com_paras.d_encryption.src_size < MIN_SDOS_BUF_SZ) ||
>> + (data->com_paras.d_encryption.src_size > MAX_SDOS_BUF_SZ)) {
>> + dev_err(dev, "Invalid SDOS Buffer src size:%d\n",
>> + data->com_paras.d_encryption.src_size);
>> + return -EFAULT;
>> + }
>> +
>> + if ((data->com_paras.d_encryption.dst_size < MIN_SDOS_BUF_SZ) ||
>> + (data->com_paras.d_encryption.dst_size > MAX_SDOS_BUF_SZ)) {
>> + dev_err(dev, "Invalid SDOS Buffer dst size:%d\n",
>> + data->com_paras.d_encryption.dst_size);
>> + return -EFAULT;
>> + }
>> +
>> + /* allocate buffer for both source and destination */
>> + s_buf = stratix10_svc_allocate_memory(priv->chan,
>> + MAX_SDOS_BUF_SZ);
>> + if (!s_buf) {
>> + dev_err(dev, "failed allocate decrypt src buf\n");
>> + return -ENOMEM;
>> + }
>> + d_buf = stratix10_svc_allocate_memory(priv->chan,
>> + MAX_SDOS_BUF_SZ);
>> + if (!d_buf) {
>> + dev_err(dev, "failed allocate decrypt dst buf\n");
>> + stratix10_svc_free_memory(priv->chan, s_buf);
>> + return -ENOMEM;
>> + }
>> +
>> + ps_buf = stratix10_svc_allocate_memory(priv->chan,
>> + PS_BUF_SIZE);
>> + if (!ps_buf) {
>> + dev_err(dev, "failed allocate p-status buffer\n");
>> + fcs_close_services(priv, s_buf, d_buf);
>> + return -ENOMEM;
>> + }
>> +
>> + ret = copy_from_user(s_buf,
>> + data->com_paras.d_decryption.src,
>> + data->com_paras.d_decryption.src_size);
>> + if (ret) {
>> + dev_err(dev, "failure on copy_from_user\n");
>> + fcs_close_services(priv, ps_buf, NULL);
>> + fcs_close_services(priv, s_buf, d_buf);
>> + return -EFAULT;
>> + }
>> +
>> + msg->command = COMMAND_FCS_DATA_DECRYPTION;
>> + msg->payload = s_buf;
>> + msg->payload_length =
>> + data->com_paras.d_decryption.src_size;
>> + msg->payload_output = d_buf;
>> + msg->payload_length_output =
>> + data->com_paras.d_decryption.dst_size;
>> + priv->client.receive_cb = fcs_vab_callback;
>> +
>> + ret = fcs_request_service(priv, (void *)msg,
>> + FCS_REQUEST_TIMEOUT);
>> + if (!ret && !priv->status) {
>> + msg->command = COMMAND_POLL_SERVICE_STATUS;
>> + msg->payload = ps_buf;
>> + msg->payload_length = PS_BUF_SIZE;
>> + priv->client.receive_cb = fcs_data_callback;
>> + ret = fcs_request_service(priv, (void *)msg,
>> + FCS_COMPLETED_TIMEOUT);
>> + dev_dbg(dev, "request service ret=%d\n", ret);
>> + if (!ret && !priv->status) {
>> + if (!priv->kbuf) {
>> + dev_err(dev, "failure on kbuf\n");
>> + fcs_close_services(priv, ps_buf, NULL);
>> + fcs_close_services(priv, s_buf, d_buf);
>> + return -EFAULT;
>> + }
>> + buf_sz = *((unsigned int *)priv->kbuf);
>> + memcpy(data->com_paras.d_decryption.dst,
>> + d_buf, buf_sz);
>> + data->com_paras.d_decryption.dst_size = buf_sz;
>> + data->status = 0;
>> + } else {
>> + data->com_paras.d_decryption.dst = NULL;
>> + data->com_paras.d_decryption.dst_size = 0;
>> + data->status = priv->status;
>> + }
>> + } else {
>> + data->com_paras.d_decryption.dst = NULL;
>> + data->com_paras.d_decryption.dst_size = 0;
>> + data->status = priv->status;
>> + }
>> +
>> + if (copy_to_user((void __user *)arg, data, sizeof(*data))) {
>> + dev_err(dev, "failure on copy_to_user\n");
>> + fcs_close_services(priv, ps_buf, NULL);
>> + fcs_close_services(priv, s_buf, d_buf);
>> + ret = -EFAULT;
>> + }
>> +
>> + fcs_close_services(priv, ps_buf, NULL);
>> + fcs_close_services(priv, s_buf, d_buf);
>> + break;
>> + default:
>> + dev_warn(dev, "shouldn't be here [0x%x]\n", cmd);
>> + break;
>> + }
>> +
>> + return ret;
>> +}
>> +
>> +static int fcs_open(struct inode *inode, struct file *file)
>> +{
>> + pr_debug("%s\n", __func__);
>> +
>> + return 0;
>> +}
>> +
>> +static int fcs_close(struct inode *inode, struct file *file)
>> +{
>> +
>> + pr_debug("%s\n", __func__);
>> +
>> + return 0;
>> +}
>
> If you do nothing in an open/close function, do not include them, they
> are not needed.

Sure, I will remove them in the next submission.

>
>> +
>> +static const struct file_operations fcs_fops = {
>> + .owner = THIS_MODULE,
>> + .unlocked_ioctl = fcs_ioctl,
>> + .open = fcs_open,
>> + .release = fcs_close,
>> +};
>> +
>> +static int fcs_driver_probe(struct platform_device *pdev)
>> +{
>> + struct device *dev = &pdev->dev;
>> + struct intel_fcs_priv *priv;
>> + int ret;
>> +
>> + priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
>> + if (!priv)
>> + return -ENOMEM;
>> +
>> + priv->client.dev = dev;
>> + priv->client.receive_cb = NULL;
>> + priv->client.priv = priv;
>> + priv->status = INVALID_STATUS;
>> +
>> + mutex_init(&priv->lock);
>> + priv->chan = stratix10_svc_request_channel_byname(&priv->client,
>> + SVC_CLIENT_FCS);
>> + if (IS_ERR(priv->chan)) {
>> + dev_err(dev, "couldn't get service channel %s\n",
>> + SVC_CLIENT_FCS);
>> + return PTR_ERR(priv->chan);
>> + }
>> +
>> + priv->miscdev.minor = MISC_DYNAMIC_MINOR;
>> + priv->miscdev.name = "fcs";
>> + priv->miscdev.fops = &fcs_fops;
>> +
>> + init_completion(&priv->completion);
>> +
>> + platform_set_drvdata(pdev, priv);
>> +
>> + ret = misc_register(&priv->miscdev);
>> + if (ret) {
>> + dev_err(dev, "can't register on minor=%d\n",
>> + MISC_DYNAMIC_MINOR);
>> + return ret;
>> + }
>> +
>> + return 0;
>> +}
>> +
>> +static int fcs_driver_remove(struct platform_device *pdev)
>> +{
>> + struct intel_fcs_priv *priv = platform_get_drvdata(pdev);
>> +
>> + misc_deregister(&priv->miscdev);
>> + stratix10_svc_free_channel(priv->chan);
>> +
>> + return 0;
>> +}
>> +
>> +static struct platform_driver fcs_driver = {
>> + .probe = fcs_driver_probe,
>> + .remove = fcs_driver_remove,
>> + .driver = {
>> + .name = "intel-fcs",
>> + },
>> +};
>> +
>> +module_platform_driver(fcs_driver);
>> +
>> +MODULE_LICENSE("GPL v2");
>> +MODULE_DESCRIPTION("Intel FGPA Crypto Services Driver");
>> +MODULE_AUTHOR("Richard Gong <[email protected]>");
>> +
>> diff --git a/include/uapi/linux/intel-fcs_ioctl.h b/include/uapi/linux/intel-fcs_ioctl.h
>> new file mode 100644
>> index 00000000..4d530ec
>> --- /dev/null
>> +++ b/include/uapi/linux/intel-fcs_ioctl.h
>> @@ -0,0 +1,222 @@
>> +/* SPDX-License-Identifier: GPL-2.0 WITH Linux-syscall-note */
>> +/*
>> + * Copyright (C) 2020, Intel Corporation
>> + */
>> +
>> +#ifndef __INTEL_FCS_IOCTL_H
>> +#define __INTEL_FCS_IOCTL_H
>> +
>> +#include <linux/types.h>
>> +
>> +#define INTEL_FCS_IOCTL 0xA2
>> +
>> +/**
>> + * enum fcs_vab_img_type - enumeration of image types
>> + * @INTEL_FCS_IMAGE_HPS: Image to validate is HPS image
>> + * @INTEL_FCS_IMAGE_BITSTREAM: Image to validate is bitstream
>> + */
>> +enum fcs_vab_img_type {
>> + INTEL_FCS_IMAGE_HPS = 0,
>> + INTEL_FCS_IMAGE_BITSTREAM = 1
>> +};
>> +
>> +/**
>> + * enum fcs_certificate_test - enumeration of certificate test
>> + * @INTEL_FCS_NO_TEST: Write to eFuses
>> + * @INTEL_FCS_TEST: Write to cache, do not write eFuses
>> + */
>> +enum fcs_certificate_test {
>> + INTEL_FCS_NO_TEST = 0,
>> + INTEL_FCS_TEST = 1
>> +};
>> +
>> +/**
>> + * struct intel_fcs_cert_test_word - certificate test word
>> + * @test_bit: if set, do not write fuses, write to cache only.
>> + * @rsvd: write as 0
>> + */
>> +struct intel_fcs_cert_test_word {
>> + __u32 test_bit:1;
>> + __u32 rsvd:31;
>> +};
>
> What endian is this? Bit fields for ioctls is almost always a very bad
> idea when done like this. Pick it apart in the kernel please.
>
Sure, will do
> >> +
>> +/**
>> + * struct fcs_validation_request - validate HPS or bitstream image
>> + * @so_type: the type of signed object, 0 for HPS and 1 for bitstream
>> + * @src: the source of signed object,
>> + * for HPS, this is the virtual address of the signed source
>> + * for Bitstream, this is path of the signed source, the default
>> + * path is /lib/firmware
>> + * @size: the size of the signed object
>> + */
>> +struct fcs_validation_request {
>> + enum fcs_vab_img_type so_type;
>> + void *src;
>
> void *????
>
> Shouldn't you be using a portable type, otherwise this, and all of your
> other pointers are going to break on 32/64 split user/kernel systems,
> right?
>
You are correct. I will take a closer look and update accordingly.

>
>
>> + __u32 size;
>> +};
>> +
>> +/**
>> + * struct fcs_key_manage_request - Request key management from SDM
>> + * @addr: the virtual address of the signed object,
>> + * @size: the size of the signed object
>> + */
>> +struct fcs_key_manage_request {
>> + void *addr;
>> + __u32 size;
>> +};
>> +
>> +/**
>> + * struct fcs_certificate_request - Certificate request to SDM
>> + * @test: test bit (1 if want to write to cache instead of fuses)
>> + * @addr: the virtual address of the signed object,
>> + * @size: the size of the signed object
>> + * @c_status: returned certificate status
>> + */
>> +struct fcs_certificate_request {
>> + struct intel_fcs_cert_test_word test;
>> + void *addr;
>> + __u32 size;
>> + __u32 c_status;
>> +};
>> +
>> +/**
>> + * struct fcs_data_encryption - aes data encryption command layout
>> + * @src: the virtual address of the input data
>> + * @src_size: the size of the unencrypted source
>> + * @dst: the virtual address of the output data
>> + * @dst_size: the size of the encrypted result
>> + */
>> +struct fcs_data_encryption {
>> + void *src;
>> + __u32 src_size;
>> + void *dst;
>> + __u32 dst_size;
>> +};
>
> Why put holes in your structures when you do not need them?
>
> Please get all of these structures reviewed by someone within Intel who
> understands how user/kernel apis work. :(
>
> greg k-h
>

Sure, I will do that.

Regards,
Richard