[PATCH v3 0/6] crypto: Add Qcom PRNG support

[PATCH v3 0/6] crypto: Add Qcom PRNG support

[Vinod Koul]

This series removes the hwrng qcom driver and replaces it with crypto qcom
driver and then adds support for Execution Environment (EE) found in v2
version of the hardware and ACPI support for these

Timur Tabi (1):
  crypto: qcom: Add ACPI support

Vinod Koul (5):
  hwrng: remove msm hw_random driver
  dt-bindings: crypto: Move prng binding to crypto
  crypto: Add Qcom prng driver
  dt-bindings: crypto: Add new compatible qcom,prng-ee
  crypto: qcom: Add support for prng-ee

 .../bindings/{rng => crypto}/qcom,prng.txt         |   4 +-
 drivers/char/hw_random/Kconfig                     |  13 --
 drivers/char/hw_random/Makefile                    |   1 -
 drivers/char/hw_random/msm-rng.c                   | 183 ----------------
 drivers/crypto/Kconfig                             |  11 +
 drivers/crypto/Makefile                            |   1 +
 drivers/crypto/qcom-rng.c                          | 244 +++++++++++++++++++++
 7 files changed, 259 insertions(+), 198 deletions(-)
 rename Documentation/devicetree/bindings/{rng => crypto}/qcom,prng.txt (73%)
 delete mode 100644 drivers/char/hw_random/msm-rng.c
 create mode 100644 drivers/crypto/qcom-rng.c

-- 
2.14.4

[PATCH v3 1/6] hwrng: remove msm hw_random driver

[Vinod Koul]

This driver is for a psedo-rng so should not be added in hwrng.
Remove it so that it's replacement can be added.

Signed-off-by: Vinod Koul <vkoul@kernel.org>
---
 drivers/char/hw_random/Kconfig   |  13 ---
 drivers/char/hw_random/Makefile  |   1 -
 drivers/char/hw_random/msm-rng.c | 183 ---------------------------------------
 3 files changed, 197 deletions(-)
 delete mode 100644 drivers/char/hw_random/msm-rng.c

diff --git a/drivers/char/hw_random/Kconfig b/drivers/char/hw_random/Kconfig
index c34b257d852d..dac895dc01b9 100644
--- a/drivers/char/hw_random/Kconfig
+++ b/drivers/char/hw_random/Kconfig
@@ -307,19 +307,6 @@ config HW_RANDOM_HISI
 
 	  If unsure, say Y.
 
-config HW_RANDOM_MSM
-	tristate "Qualcomm SoCs Random Number Generator support"
-	depends on HW_RANDOM && ARCH_QCOM
-	default HW_RANDOM
-	---help---
-	  This driver provides kernel-side support for the Random Number
-	  Generator hardware found on Qualcomm SoCs.
-
-	  To compile this driver as a module, choose M here. the
-	  module will be called msm-rng.
-
-	  If unsure, say Y.
-
 config HW_RANDOM_ST
 	tristate "ST Microelectronics HW Random Number Generator support"
 	depends on HW_RANDOM && ARCH_STI
diff --git a/drivers/char/hw_random/Makefile b/drivers/char/hw_random/Makefile
index 533e913c93d1..e35ec3ce3a20 100644
--- a/drivers/char/hw_random/Makefile
+++ b/drivers/char/hw_random/Makefile
@@ -29,7 +29,6 @@ obj-$(CONFIG_HW_RANDOM_POWERNV) += powernv-rng.o
 obj-$(CONFIG_HW_RANDOM_HISI)	+= hisi-rng.o
 obj-$(CONFIG_HW_RANDOM_BCM2835) += bcm2835-rng.o
 obj-$(CONFIG_HW_RANDOM_IPROC_RNG200) += iproc-rng200.o
-obj-$(CONFIG_HW_RANDOM_MSM) += msm-rng.o
 obj-$(CONFIG_HW_RANDOM_ST) += st-rng.o
 obj-$(CONFIG_HW_RANDOM_XGENE) += xgene-rng.o
 obj-$(CONFIG_HW_RANDOM_STM32) += stm32-rng.o
diff --git a/drivers/char/hw_random/msm-rng.c b/drivers/char/hw_random/msm-rng.c
deleted file mode 100644
index 841fee845ec9..000000000000
--- a/drivers/char/hw_random/msm-rng.c
+++ /dev/null
@@ -1,183 +0,0 @@
-/*
- * Copyright (c) 2011-2013, The Linux Foundation. All rights reserved.
- *
- * This program is free software; you can redistribute it and/or modify
- * it under the terms of the GNU General Public License version 2 and
- * only version 2 as published by the Free Software Foundation.
- *
- * This program is distributed in the hope that it will be useful,
- * but WITHOUT ANY WARRANTY; without even the implied warranty of
- * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
- * GNU General Public License for more details.
- *
- */
-#include <linux/clk.h>
-#include <linux/err.h>
-#include <linux/hw_random.h>
-#include <linux/io.h>
-#include <linux/module.h>
-#include <linux/of.h>
-#include <linux/platform_device.h>
-
-/* Device specific register offsets */
-#define PRNG_DATA_OUT		0x0000
-#define PRNG_STATUS		0x0004
-#define PRNG_LFSR_CFG		0x0100
-#define PRNG_CONFIG		0x0104
-
-/* Device specific register masks and config values */
-#define PRNG_LFSR_CFG_MASK	0x0000ffff
-#define PRNG_LFSR_CFG_CLOCKS	0x0000dddd
-#define PRNG_CONFIG_HW_ENABLE	BIT(1)
-#define PRNG_STATUS_DATA_AVAIL	BIT(0)
-
-#define MAX_HW_FIFO_DEPTH	16
-#define MAX_HW_FIFO_SIZE	(MAX_HW_FIFO_DEPTH * 4)
-#define WORD_SZ			4
-
-struct msm_rng {
-	void __iomem *base;
-	struct clk *clk;
-	struct hwrng hwrng;
-};
-
-#define to_msm_rng(p)	container_of(p, struct msm_rng, hwrng)
-
-static int msm_rng_enable(struct hwrng *hwrng, int enable)
-{
-	struct msm_rng *rng = to_msm_rng(hwrng);
-	u32 val;
-	int ret;
-
-	ret = clk_prepare_enable(rng->clk);
-	if (ret)
-		return ret;
-
-	if (enable) {
-		/* Enable PRNG only if it is not already enabled */
-		val = readl_relaxed(rng->base + PRNG_CONFIG);
-		if (val & PRNG_CONFIG_HW_ENABLE)
-			goto already_enabled;
-
-		val = readl_relaxed(rng->base + PRNG_LFSR_CFG);
-		val &= ~PRNG_LFSR_CFG_MASK;
-		val |= PRNG_LFSR_CFG_CLOCKS;
-		writel(val, rng->base + PRNG_LFSR_CFG);
-
-		val = readl_relaxed(rng->base + PRNG_CONFIG);
-		val |= PRNG_CONFIG_HW_ENABLE;
-		writel(val, rng->base + PRNG_CONFIG);
-	} else {
-		val = readl_relaxed(rng->base + PRNG_CONFIG);
-		val &= ~PRNG_CONFIG_HW_ENABLE;
-		writel(val, rng->base + PRNG_CONFIG);
-	}
-
-already_enabled:
-	clk_disable_unprepare(rng->clk);
-	return 0;
-}
-
-static int msm_rng_read(struct hwrng *hwrng, void *data, size_t max, bool wait)
-{
-	struct msm_rng *rng = to_msm_rng(hwrng);
-	size_t currsize = 0;
-	u32 *retdata = data;
-	size_t maxsize;
-	int ret;
-	u32 val;
-
-	/* calculate max size bytes to transfer back to caller */
-	maxsize = min_t(size_t, MAX_HW_FIFO_SIZE, max);
-
-	ret = clk_prepare_enable(rng->clk);
-	if (ret)
-		return ret;
-
-	/* read random data from hardware */
-	do {
-		val = readl_relaxed(rng->base + PRNG_STATUS);
-		if (!(val & PRNG_STATUS_DATA_AVAIL))
-			break;
-
-		val = readl_relaxed(rng->base + PRNG_DATA_OUT);
-		if (!val)
-			break;
-
-		*retdata++ = val;
-		currsize += WORD_SZ;
-
-		/* make sure we stay on 32bit boundary */
-		if ((maxsize - currsize) < WORD_SZ)
-			break;
-	} while (currsize < maxsize);
-
-	clk_disable_unprepare(rng->clk);
-
-	return currsize;
-}
-
-static int msm_rng_init(struct hwrng *hwrng)
-{
-	return msm_rng_enable(hwrng, 1);
-}
-
-static void msm_rng_cleanup(struct hwrng *hwrng)
-{
-	msm_rng_enable(hwrng, 0);
-}
-
-static int msm_rng_probe(struct platform_device *pdev)
-{
-	struct resource *res;
-	struct msm_rng *rng;
-	int ret;
-
-	rng = devm_kzalloc(&pdev->dev, sizeof(*rng), GFP_KERNEL);
-	if (!rng)
-		return -ENOMEM;
-
-	platform_set_drvdata(pdev, rng);
-
-	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
-	rng->base = devm_ioremap_resource(&pdev->dev, res);
-	if (IS_ERR(rng->base))
-		return PTR_ERR(rng->base);
-
-	rng->clk = devm_clk_get(&pdev->dev, "core");
-	if (IS_ERR(rng->clk))
-		return PTR_ERR(rng->clk);
-
-	rng->hwrng.name = KBUILD_MODNAME,
-	rng->hwrng.init = msm_rng_init,
-	rng->hwrng.cleanup = msm_rng_cleanup,
-	rng->hwrng.read = msm_rng_read,
-
-	ret = devm_hwrng_register(&pdev->dev, &rng->hwrng);
-	if (ret) {
-		dev_err(&pdev->dev, "failed to register hwrng\n");
-		return ret;
-	}
-
-	return 0;
-}
-
-static const struct of_device_id msm_rng_of_match[] = {
-	{ .compatible = "qcom,prng", },
-	{}
-};
-MODULE_DEVICE_TABLE(of, msm_rng_of_match);
-
-static struct platform_driver msm_rng_driver = {
-	.probe = msm_rng_probe,
-	.driver = {
-		.name = KBUILD_MODNAME,
-		.of_match_table = of_match_ptr(msm_rng_of_match),
-	}
-};
-module_platform_driver(msm_rng_driver);
-
-MODULE_ALIAS("platform:" KBUILD_MODNAME);
-MODULE_AUTHOR("The Linux Foundation");
-MODULE_DESCRIPTION("Qualcomm MSM random number generator driver");
-MODULE_LICENSE("GPL v2");
-- 
2.14.4

[PATCH v3 2/6] dt-bindings: crypto: Move prng binding to crypto

[Vinod Koul]

Now that we are adding new driver for prng in crypto, move the
binding as well.

Signed-off-by: Vinod Koul <vkoul@kernel.org>
---
 Documentation/devicetree/bindings/{rng => crypto}/qcom,prng.txt | 0
 1 file changed, 0 insertions(+), 0 deletions(-)
 rename Documentation/devicetree/bindings/{rng => crypto}/qcom,prng.txt (100%)

diff --git a/Documentation/devicetree/bindings/rng/qcom,prng.txt b/Documentation/devicetree/bindings/crypto/qcom,prng.txt
similarity index 100%
rename from Documentation/devicetree/bindings/rng/qcom,prng.txt
rename to Documentation/devicetree/bindings/crypto/qcom,prng.txt
-- 
2.14.4

[PATCH v3 3/6] crypto: Add Qcom prng driver

[Vinod Koul]

This ports the Qcom prng from older hw_random driver.

No change of functionality and move from hw_random to crypto
APIs is done.

Signed-off-by: Vinod Koul <vkoul@kernel.org>
---
 drivers/crypto/Kconfig    |  11 +++
 drivers/crypto/Makefile   |   1 +
 drivers/crypto/qcom-rng.c | 208 ++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 220 insertions(+)
 create mode 100644 drivers/crypto/qcom-rng.c

diff --git a/drivers/crypto/Kconfig b/drivers/crypto/Kconfig
index 43cccf6aff61..b8d9e71e550a 100644
--- a/drivers/crypto/Kconfig
+++ b/drivers/crypto/Kconfig
@@ -585,6 +585,17 @@ config CRYPTO_DEV_QCE
 	  hardware. To compile this driver as a module, choose M here. The
 	  module will be called qcrypto.
 
+config CRYPTO_DEV_QCOM_RNG
+	tristate "Qualcomm Randon Number Generator Driver"
+	depends on ARCH_QCOM || COMPILE_TEST
+	select CRYPTO_RNG
+	help
+	  This driver provides support for the Random Number
+	  Generator hardware found on Qualcomm SoCs.
+
+	  To compile this driver as a module, choose M here. the
+          module will be called qcom-rng. If unsure, say N.
+
 config CRYPTO_DEV_VMX
 	bool "Support for VMX cryptographic acceleration instructions"
 	depends on PPC64 && VSX
diff --git a/drivers/crypto/Makefile b/drivers/crypto/Makefile
index 7ae87b4f6c8d..3602875c4f80 100644
--- a/drivers/crypto/Makefile
+++ b/drivers/crypto/Makefile
@@ -33,6 +33,7 @@ obj-$(CONFIG_CRYPTO_DEV_PICOXCELL) += picoxcell_crypto.o
 obj-$(CONFIG_CRYPTO_DEV_PPC4XX) += amcc/
 obj-$(CONFIG_CRYPTO_DEV_QAT) += qat/
 obj-$(CONFIG_CRYPTO_DEV_QCE) += qce/
+obj-$(CONFIG_CRYPTO_DEV_QCOM_RNG) += qcom-rng.o
 obj-$(CONFIG_CRYPTO_DEV_ROCKCHIP) += rockchip/
 obj-$(CONFIG_CRYPTO_DEV_S5P) += s5p-sss.o
 obj-$(CONFIG_CRYPTO_DEV_SAHARA) += sahara.o
diff --git a/drivers/crypto/qcom-rng.c b/drivers/crypto/qcom-rng.c
new file mode 100644
index 000000000000..e235a35359d3
--- /dev/null
+++ b/drivers/crypto/qcom-rng.c
@@ -0,0 +1,208 @@
+// SPDX-License-Identifier: GPL-2.0
+// Copyright (c) 2017-18 Linaro Limited
+//
+// Based on msm-rng.c and downstream driver
+
+#include <crypto/internal/rng.h>
+#include <linux/clk.h>
+#include <linux/crypto.h>
+#include <linux/module.h>
+#include <linux/of_device.h>
+#include <linux/platform_device.h>
+
+/* Device specific register offsets */
+#define PRNG_DATA_OUT		0x0000
+#define PRNG_STATUS		0x0004
+#define PRNG_LFSR_CFG		0x0100
+#define PRNG_CONFIG		0x0104
+
+/* Device specific register masks and config values */
+#define PRNG_LFSR_CFG_MASK	0x0000ffff
+#define PRNG_LFSR_CFG_CLOCKS	0x0000dddd
+#define PRNG_CONFIG_HW_ENABLE	BIT(1)
+#define PRNG_STATUS_DATA_AVAIL	BIT(0)
+
+#define MAX_HW_FIFO_DEPTH	16
+#define MAX_HW_FIFO_SIZE	(MAX_HW_FIFO_DEPTH * 4)
+#define WORD_SZ			4
+
+struct qcom_rng {
+	struct mutex lock;
+	void __iomem *base;
+	struct clk *clk;
+};
+
+struct qcom_rng_ctx {
+	struct qcom_rng *rng;
+};
+
+static struct qcom_rng *qcom_rng_dev;
+
+static int qcom_rng_read(struct qcom_rng *rng, void *data, size_t max)
+{
+	size_t currsize = 0;
+	u32 *retdata = data;
+	u32 val;
+
+	/* read random data from hardware */
+	do {
+		val = readl_relaxed(rng->base + PRNG_STATUS);
+		if (!(val & PRNG_STATUS_DATA_AVAIL))
+			break;
+
+		val = readl_relaxed(rng->base + PRNG_DATA_OUT);
+		if (!val)
+			break;
+
+		*retdata++ = val;
+		currsize += WORD_SZ;
+
+		/* make sure we stay on 32bit boundary */
+		if ((max - currsize) < WORD_SZ)
+			break;
+	} while (currsize < max);
+
+	return currsize;
+}
+
+static int qcom_rng_generate(struct crypto_rng *tfm,
+			     const u8 *src, unsigned int slen,
+			     u8 *dstn, unsigned int dlen)
+{
+	struct qcom_rng_ctx *ctx = crypto_rng_ctx(tfm);
+	struct qcom_rng *rng = ctx->rng;
+	int ret;
+
+	ret = clk_prepare_enable(rng->clk);
+	if (ret)
+		return ret;
+
+	mutex_lock(&rng->lock);
+
+	ret = qcom_rng_read(rng, dstn, dlen);
+
+	mutex_unlock(&rng->lock);
+	clk_disable_unprepare(rng->clk);
+
+	return 0;
+}
+
+static int qcom_rng_seed(struct crypto_rng *tfm, const u8 *seed,
+			 unsigned int slen)
+{
+	return 0;
+}
+
+static int qcom_rng_enable(struct qcom_rng *rng)
+{
+	u32 val;
+	int ret;
+
+	ret = clk_prepare_enable(rng->clk);
+	if (ret)
+		return ret;
+
+	/* Enable PRNG only if it is not already enabled */
+	val = readl_relaxed(rng->base + PRNG_CONFIG);
+	if (val & PRNG_CONFIG_HW_ENABLE)
+		goto already_enabled;
+
+	val = readl_relaxed(rng->base + PRNG_LFSR_CFG);
+	val &= ~PRNG_LFSR_CFG_MASK;
+	val |= PRNG_LFSR_CFG_CLOCKS;
+	writel(val, rng->base + PRNG_LFSR_CFG);
+
+	val = readl_relaxed(rng->base + PRNG_CONFIG);
+	val |= PRNG_CONFIG_HW_ENABLE;
+	writel(val, rng->base + PRNG_CONFIG);
+
+already_enabled:
+	clk_disable_unprepare(rng->clk);
+
+	return 0;
+}
+
+static int qcom_rng_init(struct crypto_tfm *tfm)
+{
+	struct qcom_rng_ctx *ctx = crypto_tfm_ctx(tfm);
+
+	ctx->rng = qcom_rng_dev;
+
+	return qcom_rng_enable(ctx->rng);
+}
+
+static struct rng_alg qcom_rng_alg = {
+	.generate	= qcom_rng_generate,
+	.seed		= qcom_rng_seed,
+	.seedsize	= 0,
+	.base		= {
+		.cra_name		= "stdrng",
+		.cra_driver_name	= "qcom-rng",
+		.cra_flags		= CRYPTO_ALG_TYPE_RNG,
+		.cra_priority		= 300,
+		.cra_ctxsize		= sizeof(struct qcom_rng_ctx),
+		.cra_module		= THIS_MODULE,
+		.cra_init		= qcom_rng_init,
+	}
+};
+
+static int qcom_rng_probe(struct platform_device *pdev)
+{
+	struct resource *res;
+	struct qcom_rng *rng;
+	int ret;
+
+	rng = devm_kzalloc(&pdev->dev, sizeof(*rng), GFP_KERNEL);
+	if (!rng)
+		return -ENOMEM;
+
+	platform_set_drvdata(pdev, rng);
+	mutex_init(&rng->lock);
+
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	rng->base = devm_ioremap_resource(&pdev->dev, res);
+	if (IS_ERR(rng->base))
+		return PTR_ERR(rng->base);
+
+	rng->clk = devm_clk_get(&pdev->dev, "core");
+	if (IS_ERR(rng->clk))
+		return PTR_ERR(rng->clk);
+
+	qcom_rng_dev = rng;
+	ret = crypto_register_rng(&qcom_rng_alg);
+	if (ret) {
+		dev_err(&pdev->dev, "Register crypto rng failed: %d\n", ret);
+		qcom_rng_dev = NULL;
+	}
+
+	return ret;
+}
+
+static int qcom_rng_remove(struct platform_device *pdev)
+{
+	crypto_unregister_rng(&qcom_rng_alg);
+
+	qcom_rng_dev = NULL;
+
+	return 0;
+}
+
+static const struct of_device_id qcom_rng_of_match[] = {
+	{ .compatible = "qcom,prng" },
+	{}
+};
+MODULE_DEVICE_TABLE(of, qcom_rng_of_match);
+
+static struct platform_driver qcom_rng_driver = {
+	.probe = qcom_rng_probe,
+	.remove =  qcom_rng_remove,
+	.driver = {
+		.name = KBUILD_MODNAME,
+		.of_match_table = of_match_ptr(qcom_rng_of_match),
+	}
+};
+module_platform_driver(qcom_rng_driver);
+
+MODULE_ALIAS("platform:" KBUILD_MODNAME);
+MODULE_DESCRIPTION("Qualcomm random number generator driver");
+MODULE_LICENSE("GPL v2");
-- 
2.14.4

[PATCH v3 4/6] dt-bindings: crypto: Add new compatible qcom,prng-ee

[Vinod Koul]

Later qcom chips support v2 of the prng, which exposes an EE
(Execution Enviornment) for OS to use so add new compatible
qcom,prng-ee for this.

Signed-off-by: Vinod Koul <vkoul@kernel.org>
---
 Documentation/devicetree/bindings/crypto/qcom,prng.txt | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/crypto/qcom,prng.txt b/Documentation/devicetree/bindings/crypto/qcom,prng.txt
index 8e5853c2879b..7ee0e9eac973 100644
--- a/Documentation/devicetree/bindings/crypto/qcom,prng.txt
+++ b/Documentation/devicetree/bindings/crypto/qcom,prng.txt
@@ -2,7 +2,9 @@ Qualcomm MSM pseudo random number generator.
 
 Required properties:
 
-- compatible  : should be "qcom,prng"
+- compatible  : should be "qcom,prng" for 8916 etc
+              : should be "qcom,prng-ee" for 8996 and later using EE
+		(Execution Environment) slice of prng
 - reg         : specifies base physical address and size of the registers map
 - clocks      : phandle to clock-controller plus clock-specifier pair
 - clock-names : "core" clocks all registers, FIFO and circuits in PRNG IP block
-- 
2.14.4

[PATCH v3 5/6] crypto: qcom: Add support for prng-ee

[Vinod Koul]

Qcom 8996 and later chips features multiple Execution Environments
(EE) and secure world is typically responsible for configuring the
prng.

Add driver data for qcom,prng as 0 and qcom,prng-ee as 1 and use
that to skip initialization routine.

Signed-off-by: Vinod Koul <vkoul@kernel.org>
---
 drivers/crypto/qcom-rng.c | 11 +++++++++--
 1 file changed, 9 insertions(+), 2 deletions(-)

diff --git a/drivers/crypto/qcom-rng.c b/drivers/crypto/qcom-rng.c
index e235a35359d3..fdbbcac7bcb8 100644
--- a/drivers/crypto/qcom-rng.c
+++ b/drivers/crypto/qcom-rng.c
@@ -30,6 +30,7 @@ struct qcom_rng {
 	struct mutex lock;
 	void __iomem *base;
 	struct clk *clk;
+	unsigned int skip_init;
 };
 
 struct qcom_rng_ctx {
@@ -128,7 +129,10 @@ static int qcom_rng_init(struct crypto_tfm *tfm)
 
 	ctx->rng = qcom_rng_dev;
 
-	return qcom_rng_enable(ctx->rng);
+	if (!ctx->rng->skip_init)
+		return qcom_rng_enable(ctx->rng);
+
+	return 0;
 }
 
 static struct rng_alg qcom_rng_alg = {
@@ -168,6 +172,8 @@ static int qcom_rng_probe(struct platform_device *pdev)
 	if (IS_ERR(rng->clk))
 		return PTR_ERR(rng->clk);
 
+	rng->skip_init = (unsigned long)of_device_get_match_data(&pdev->dev);
+
 	qcom_rng_dev = rng;
 	ret = crypto_register_rng(&qcom_rng_alg);
 	if (ret) {
@@ -188,7 +194,8 @@ static int qcom_rng_remove(struct platform_device *pdev)
 }
 
 static const struct of_device_id qcom_rng_of_match[] = {
-	{ .compatible = "qcom,prng" },
+	{ .compatible = "qcom,prng", .data = (void *)0},
+	{ .compatible = "qcom,prng-ee", .data = (void *)1},
 	{}
 };
 MODULE_DEVICE_TABLE(of, qcom_rng_of_match);
-- 
2.14.4

[PATCH v3 6/6] crypto: qcom: Add ACPI support

[Vinod Koul]

From: Timur Tabi <timur@codeaurora.org>

Add support for probing on ACPI systems, with ACPI HID QCOM8160.

On ACPI systems, clocks are always enabled, the PRNG should
already be enabled, and the register region is read-only.
The driver only verifies that the hardware is already
enabled never tries to disable or configure it.

Signed-off-by: Timur Tabi <timur@codeaurora.org>
[port to crypto API]
Signed-off-by: Vinod Koul <vkoul@kernel.org>
---
 drivers/crypto/qcom-rng.c | 37 +++++++++++++++++++++++++++++++++----
 1 file changed, 33 insertions(+), 4 deletions(-)

diff --git a/drivers/crypto/qcom-rng.c b/drivers/crypto/qcom-rng.c
index fdbbcac7bcb8..bc0131d130d6 100644
--- a/drivers/crypto/qcom-rng.c
+++ b/drivers/crypto/qcom-rng.c
@@ -4,6 +4,7 @@
 // Based on msm-rng.c and downstream driver
 
 #include <crypto/internal/rng.h>
+#include <linux/acpi.h>
 #include <linux/clk.h>
 #include <linux/crypto.h>
 #include <linux/module.h>
@@ -154,6 +155,7 @@ static int qcom_rng_probe(struct platform_device *pdev)
 {
 	struct resource *res;
 	struct qcom_rng *rng;
+	u32 val;
 	int ret;
 
 	rng = devm_kzalloc(&pdev->dev, sizeof(*rng), GFP_KERNEL);
@@ -168,11 +170,27 @@ static int qcom_rng_probe(struct platform_device *pdev)
 	if (IS_ERR(rng->base))
 		return PTR_ERR(rng->base);
 
-	rng->clk = devm_clk_get(&pdev->dev, "core");
-	if (IS_ERR(rng->clk))
-		return PTR_ERR(rng->clk);
 
-	rng->skip_init = (unsigned long)of_device_get_match_data(&pdev->dev);
+	/*
+	 * ACPI systems have v2 hardware. The clocks are always enabled,
+	 * the PRNG register space is read-only and the PRNG should
+	 * already be enabled.
+	 */
+	if (has_acpi_companion(&pdev->dev)) {
+		val = readl(rng->base + PRNG_CONFIG);
+		if (!(val & PRNG_CONFIG_HW_ENABLE)) {
+			dev_err(&pdev->dev, "device is not enabled\n");
+			return -ENODEV;
+		}
+		rng->skip_init = 1;
+	} else {
+		rng->clk = devm_clk_get(&pdev->dev, "core");
+		if (IS_ERR(rng->clk))
+			return PTR_ERR(rng->clk);
+
+		rng->skip_init =
+			(unsigned long)of_device_get_match_data(&pdev->dev);
+	}
 
 	qcom_rng_dev = rng;
 	ret = crypto_register_rng(&qcom_rng_alg);
@@ -193,6 +211,16 @@ static int qcom_rng_remove(struct platform_device *pdev)
 	return 0;
 }
 
+#if IS_ENABLED(CONFIG_ACPI)
+static const struct acpi_device_id qcom_rng_acpi_match[] = {
+	{
+		.id = "QCOM8160",
+	},
+	{}
+};
+MODULE_DEVICE_TABLE(acpi, msm_rng_acpi_match);
+#endif
+
 static const struct of_device_id qcom_rng_of_match[] = {
 	{ .compatible = "qcom,prng", .data = (void *)0},
 	{ .compatible = "qcom,prng-ee", .data = (void *)1},
@@ -206,6 +234,7 @@ static struct platform_driver qcom_rng_driver = {
 	.driver = {
 		.name = KBUILD_MODNAME,
 		.of_match_table = of_match_ptr(qcom_rng_of_match),
+		.acpi_match_table = ACPI_PTR(qcom_rng_acpi_match),
 	}
 };
 module_platform_driver(qcom_rng_driver);
-- 
2.14.4

Re: [PATCH v3 3/6] crypto: Add Qcom prng driver

[Stephan Mueller]

Am Dienstag, 3. Juli 2018, 08:04:31 CEST schrieb Vinod Koul:

Hi Vinod,

> +static int qcom_rng_read(struct qcom_rng *rng, void *data, size_t max)
> +{
> +	size_t currsize = 0;
> +	u32 *retdata = data;

How can you be sure that this cast is appropriate? I.e. how is it guaranteed 
that data is 4-byte aligned?

Also, the data variable in qcom_rng_generate is a u8 -- shouldn't this type be 
used instead of a void?


Ciao
Stephan

Re: [PATCH v3 6/6] crypto: qcom: Add ACPI support

[Timur Tabi]

On 7/3/18 1:04 AM, Vinod Koul wrote:
> Add support for probing on ACPI systems, with ACPI HID QCOM8160.
> 
> On ACPI systems, clocks are always enabled, the PRNG should
> already be enabled, and the register region is read-only.
> The driver only verifies that the hardware is already
> enabled never tries to disable or configure it.
> 
> Signed-off-by: Timur Tabi<timur@codeaurora.org>
> [port to crypto API]
> Signed-off-by: Vinod Koul<vkoul@kernel.org>

I've asked a colleague who still works at Qualcomm to test this code on 
silicon.  It looks okay, but I just want to be sure.

> +	/*
> +	 * ACPI systems have v2 hardware. The clocks are always enabled,
> +	 * the PRNG register space is read-only and the PRNG should
> +	 * already be enabled.
> +	 */
> +	if (has_acpi_companion(&pdev->dev)) {
> +		val = readl(rng->base + PRNG_CONFIG);
> +		if (!(val & PRNG_CONFIG_HW_ENABLE)) {
> +			dev_err(&pdev->dev, "device is not enabled\n");
> +			return -ENODEV;
> +		}

I'm having second thoughts about this PRNG_CONFIG_HW_ENABLE check.  The 
PRNG on the QDF2400 is the same as the one on the 8996, so it should 
have the same register interface.  Currently, the ACPI table points to a 
full PRNG register block, but I'm beginning to believe that it should 
instead point to a "reduced" block that doesn't have a PRNG_CONFIG register.

Re: [PATCH v3 6/6] crypto: qcom: Add ACPI support

[Jeffrey Hugo]

On 7/3/2018 12:04 AM, Vinod Koul wrote:
> From: Timur Tabi <timur@codeaurora.org>
> 
> Add support for probing on ACPI systems, with ACPI HID QCOM8160.
> 
> On ACPI systems, clocks are always enabled, the PRNG should
> already be enabled, and the register region is read-only.
> The driver only verifies that the hardware is already
> enabled never tries to disable or configure it.
> 
> Signed-off-by: Timur Tabi <timur@codeaurora.org>
> [port to crypto API]
> Signed-off-by: Vinod Koul <vkoul@kernel.org>
> ---
>   drivers/crypto/qcom-rng.c | 37 +++++++++++++++++++++++++++++++++----
>   1 file changed, 33 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/crypto/qcom-rng.c b/drivers/crypto/qcom-rng.c
> index fdbbcac7bcb8..bc0131d130d6 100644
> --- a/drivers/crypto/qcom-rng.c
> +++ b/drivers/crypto/qcom-rng.c
> @@ -4,6 +4,7 @@
>   // Based on msm-rng.c and downstream driver
>   
>   #include <crypto/internal/rng.h>
> +#include <linux/acpi.h>
>   #include <linux/clk.h>
>   #include <linux/crypto.h>
>   #include <linux/module.h>
> @@ -154,6 +155,7 @@ static int qcom_rng_probe(struct platform_device *pdev)
>   {
>   	struct resource *res;
>   	struct qcom_rng *rng;
> +	u32 val;
>   	int ret;
>   
>   	rng = devm_kzalloc(&pdev->dev, sizeof(*rng), GFP_KERNEL);
> @@ -168,11 +170,27 @@ static int qcom_rng_probe(struct platform_device *pdev)
>   	if (IS_ERR(rng->base))
>   		return PTR_ERR(rng->base);
>   
> -	rng->clk = devm_clk_get(&pdev->dev, "core");
> -	if (IS_ERR(rng->clk))
> -		return PTR_ERR(rng->clk);
>   
> -	rng->skip_init = (unsigned long)of_device_get_match_data(&pdev->dev);
> +	/*
> +	 * ACPI systems have v2 hardware. The clocks are always enabled,
> +	 * the PRNG register space is read-only and the PRNG should
> +	 * already be enabled.
> +	 */
> +	if (has_acpi_companion(&pdev->dev)) {
> +		val = readl(rng->base + PRNG_CONFIG);
> +		if (!(val & PRNG_CONFIG_HW_ENABLE)) {
> +			dev_err(&pdev->dev, "device is not enabled\n");
> +			return -ENODEV;
> +		}
> +		rng->skip_init = 1;
> +	} else {
> +		rng->clk = devm_clk_get(&pdev->dev, "core");
> +		if (IS_ERR(rng->clk))
> +			return PTR_ERR(rng->clk);
> +
> +		rng->skip_init =
> +			(unsigned long)of_device_get_match_data(&pdev->dev);
> +	}
>   
>   	qcom_rng_dev = rng;
>   	ret = crypto_register_rng(&qcom_rng_alg);
> @@ -193,6 +211,16 @@ static int qcom_rng_remove(struct platform_device *pdev)
>   	return 0;
>   }
>   
> +#if IS_ENABLED(CONFIG_ACPI)
> +static const struct acpi_device_id qcom_rng_acpi_match[] = {
> +	{
> +		.id = "QCOM8160",
> +	},
> +	{}
> +};
> +MODULE_DEVICE_TABLE(acpi, msm_rng_acpi_match);

qcom_rng_acpi_match?
Looks like a copy/paste issue.  This causes a build failure for me.
I'm trying to see if it works otherwise...

> +#endif
> +
>   static const struct of_device_id qcom_rng_of_match[] = {
>   	{ .compatible = "qcom,prng", .data = (void *)0},
>   	{ .compatible = "qcom,prng-ee", .data = (void *)1},
> @@ -206,6 +234,7 @@ static struct platform_driver qcom_rng_driver = {
>   	.driver = {
>   		.name = KBUILD_MODNAME,
>   		.of_match_table = of_match_ptr(qcom_rng_of_match),
> +		.acpi_match_table = ACPI_PTR(qcom_rng_acpi_match),
>   	}
>   };
>   module_platform_driver(qcom_rng_driver);
> 


-- 
Jeffrey Hugo
Qualcomm Datacenter Technologies as an affiliate of Qualcomm 
Technologies, Inc.
Qualcomm Technologies, Inc. is a member of the
Code Aurora Forum, a Linux Foundation Collaborative Project.

Re: [PATCH v3 3/6] crypto: Add Qcom prng driver

[Vinod]

Hi Stephan,

On 03-07-18, 15:28, Stephan Mueller wrote:
> Am Dienstag, 3. Juli 2018, 08:04:31 CEST schrieb Vinod Koul:
> > +static int qcom_rng_read(struct qcom_rng *rng, void *data, size_t max)
> > +{
> > +	size_t currsize = 0;
> > +	u32 *retdata = data;
> 
> How can you be sure that this cast is appropriate? I.e. how is it guaranteed 
> that data is 4-byte aligned?

While reading we check the alignment:

               /* make sure we stay on 32bit boundary */
               if ((max - currsize) < WORD_SZ)
                       break;

> Also, the data variable in qcom_rng_generate is a u8 -- shouldn't this type be 
> used instead of a void?

That does make sense to me. IIRC the read is for a byte. I will check
this and update it

Thanks for the quick review
-- 
~Vinod

Re: [PATCH v3 6/6] crypto: qcom: Add ACPI support

[Vinod]

On 03-07-18, 09:10, Timur Tabi wrote:
> On 7/3/18 1:04 AM, Vinod Koul wrote:
> > Add support for probing on ACPI systems, with ACPI HID QCOM8160.
> > 
> > On ACPI systems, clocks are always enabled, the PRNG should
> > already be enabled, and the register region is read-only.
> > The driver only verifies that the hardware is already
> > enabled never tries to disable or configure it.
> > 
> > Signed-off-by: Timur Tabi<timur@codeaurora.org>
> > [port to crypto API]
> > Signed-off-by: Vinod Koul<vkoul@kernel.org>
> 
> I've asked a colleague who still works at Qualcomm to test this code on
> silicon.  It looks okay, but I just want to be sure.
> 
> > +	/*
> > +	 * ACPI systems have v2 hardware. The clocks are always enabled,
> > +	 * the PRNG register space is read-only and the PRNG should
> > +	 * already be enabled.
> > +	 */
> > +	if (has_acpi_companion(&pdev->dev)) {
> > +		val = readl(rng->base + PRNG_CONFIG);
> > +		if (!(val & PRNG_CONFIG_HW_ENABLE)) {
> > +			dev_err(&pdev->dev, "device is not enabled\n");
> > +			return -ENODEV;
> > +		}
> 
> I'm having second thoughts about this PRNG_CONFIG_HW_ENABLE check.  The PRNG
> on the QDF2400 is the same as the one on the 8996, so it should have the
> same register interface.  Currently, the ACPI table points to a full PRNG
> register block, but I'm beginning to believe that it should instead point to
> a "reduced" block that doesn't have a PRNG_CONFIG register.

That was my doubt too. I will go ahead and make it skip this then...

-- 
~Vinod

Re: [PATCH v3 6/6] crypto: qcom: Add ACPI support

[Vinod]

On 03-07-18, 11:08, Jeffrey Hugo wrote:
> On 7/3/2018 12:04 AM, Vinod Koul wrote:

> > +#if IS_ENABLED(CONFIG_ACPI)
> > +static const struct acpi_device_id qcom_rng_acpi_match[] = {
> > +	{
> > +		.id = "QCOM8160",
> > +	},
> > +	{}
> > +};
> > +MODULE_DEVICE_TABLE(acpi, msm_rng_acpi_match);
> 
> qcom_rng_acpi_match?
> Looks like a copy/paste issue.  This causes a build failure for me.
> I'm trying to see if it works otherwise...

Ah sorry about that, I though I had fixed it, looks like I missed to
fold the fix.

Did it work for you?

-- 
~Vinod

Re: [PATCH v3 3/6] crypto: Add Qcom prng driver

[Vinod]

On 04-07-18, 09:40, Vinod wrote:
> Hi Stephan,
> 
> On 03-07-18, 15:28, Stephan Mueller wrote:
> > Am Dienstag, 3. Juli 2018, 08:04:31 CEST schrieb Vinod Koul:
> > > +static int qcom_rng_read(struct qcom_rng *rng, void *data, size_t max)
> > > +{
> > > +	size_t currsize = 0;
> > > +	u32 *retdata = data;
> > 
> > How can you be sure that this cast is appropriate? I.e. how is it guaranteed 
> > that data is 4-byte aligned?
> 
> While reading we check the alignment:
> 
>                /* make sure we stay on 32bit boundary */
>                if ((max - currsize) < WORD_SZ)
>                        break;
> 
> > Also, the data variable in qcom_rng_generate is a u8 -- shouldn't this type be 
> > used instead of a void?
> 
> That does make sense to me. IIRC the read is for a byte. I will check
> this and update it

Okay so I rechecked this, the hardware gives 32 bits of random data. I
am thinking of splitting the word and updating by each byte. That way
trailing zero can also be avoided which is the case now

-- 
~Vinod

Re: [PATCH v3 3/6] crypto: Add Qcom prng driver

[Stephan Mueller]

Am Mittwoch, 4. Juli 2018, 08:10:35 CEST schrieb Vinod:

Hi Vinod,

> On 04-07-18, 09:40, Vinod wrote:
> > Hi Stephan,
> > 
> > On 03-07-18, 15:28, Stephan Mueller wrote:
> > > Am Dienstag, 3. Juli 2018, 08:04:31 CEST schrieb Vinod Koul:
> > > > +static int qcom_rng_read(struct qcom_rng *rng, void *data, size_t
> > > > max)
> > > > +{
> > > > +	size_t currsize = 0;
> > > > +	u32 *retdata = data;
> > > 
> > > How can you be sure that this cast is appropriate? I.e. how is it
> > > guaranteed that data is 4-byte aligned?
> > 
> > While reading we check the alignment:
> >                /* make sure we stay on 32bit boundary */
> >                if ((max - currsize) < WORD_SZ)
> >                
> >                        break;

I am not sure I follow your argument.

You cast a void (or u8 pointer into u32:

+       u32 *retdata = data;

You use it:

+       *retdata++ = val;

Followed by your check.

What I mean is that the initial cast and then the subsequent write operation 
is only guaranteed to work if the initial pointer is alighed on a 4 byte 
boundary. However, since it is an u8 pointer, it is not guaranteed to be 
aligned.

So, I guess you want to use memcpy (at least if it is not aligned).

Ciao
Stephan

Re: [PATCH v3 3/6] crypto: Add Qcom prng driver

[Timur Tabi]

On 7/4/18 1:10 AM, Vinod wrote:
> Okay so I rechecked this, the hardware gives 32 bits of random data. I
> am thinking of splitting the word and updating by each byte. That way
> trailing zero can also be avoided which is the case now

The current driver only returns data in multiples of 4 bytes.  Why can't 
you keep that?

Re: [PATCH v3 3/6] crypto: Add Qcom prng driver

[Stephan Mueller]

Am Dienstag, 3. Juli 2018, 08:04:31 CEST schrieb Vinod Koul:

Hi Vinod,

> +static int qcom_rng_seed(struct crypto_rng *tfm, const u8 *seed,
> +			 unsigned int slen)
> +{
> +	return 0;
> +}

One more question: is it not possible to mix in data into the DRNG? I thought 
it would be possible with the Qualcomm DRBG.

Note, I am asking because of my /dev/random drop-in-replacement 
implementation, any RNG from the kernel crypto API can be configured to be 
used as an output DRNG. Though, this will only work if the DRNG also accepts 
seed from the software noise sources.

Ciao
Stephan

Re: [PATCH v3 3/6] crypto: Add Qcom prng driver

[Vinod]

Hi Stephan,

On 04-07-18, 18:02, Stephan Mueller wrote:
> Am Dienstag, 3. Juli 2018, 08:04:31 CEST schrieb Vinod Koul:
> > +static int qcom_rng_seed(struct crypto_rng *tfm, const u8 *seed,
> > +			 unsigned int slen)
> > +{
> > +	return 0;
> > +}
> 
> One more question: is it not possible to mix in data into the DRNG? I thought 
> it would be possible with the Qualcomm DRBG.
> 
> Note, I am asking because of my /dev/random drop-in-replacement 
> implementation, any RNG from the kernel crypto API can be configured to be 
> used as an output DRNG. Though, this will only work if the DRNG also accepts 
> seed from the software noise sources.

The v1 hardware supports seeding but the register is Read Only for SW and
only trusted zone (firmware) can write.

v2 hardware slice does not have seeding. v2 seeding is not accessible to
SW.

So in short, it is not available for us to use :(

-- 
~Vinod

Re: [PATCH v3 6/6] crypto: qcom: Add ACPI support

[Jeffrey Hugo]

On 7/3/2018 10:13 PM, Vinod wrote:
> On 03-07-18, 11:08, Jeffrey Hugo wrote:
>> On 7/3/2018 12:04 AM, Vinod Koul wrote:
> 
>>> +#if IS_ENABLED(CONFIG_ACPI)
>>> +static const struct acpi_device_id qcom_rng_acpi_match[] = {
>>> +	{
>>> +		.id = "QCOM8160",
>>> +	},
>>> +	{}
>>> +};
>>> +MODULE_DEVICE_TABLE(acpi, msm_rng_acpi_match);
>>
>> qcom_rng_acpi_match?
>> Looks like a copy/paste issue.  This causes a build failure for me.
>> I'm trying to see if it works otherwise...
> 
> Ah sorry about that, I though I had fixed it, looks like I missed to
> fold the fix.
> 
> Did it work for you?
> 
It seems to work.  Driver compiles, and loads.  Still working on the 
verification.

-- 
Jeffrey Hugo
Qualcomm Datacenter Technologies as an affiliate of Qualcomm 
Technologies, Inc.
Qualcomm Technologies, Inc. is a member of the
Code Aurora Forum, a Linux Foundation Collaborative Project.