Re: crash in ppc4xx-rng on canyonland

2016-04-20 Thread Herbert Xu 
On Mon, Apr 18, 2016 at 01:42:49PM +0200, Christian Lamparter wrote:
>
> what else I fixed in v1->v2: 
>  - added a check to test trng device's status state with
>of_device_is_available.
>  - if the hwrng device registration failed, the flag which
>enables the trng was left enabled (note: the v1 code
>disabled the hwrng device as part of crypto4xx_remove.
>so it wasn't enabled when the crypto4xx driver was
>unloaded)

Patch applied.  Thanks!
-- 
Email: Herbert Xu 
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
--
To unsubscribe from this list: send the line "unsubscribe linux-crypto" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: crash in ppc4xx-rng on canyonland

2016-04-18 Thread Christian Lamparter 
On Monday, April 18, 2016 05:59:39 PM Herbert Xu wrote:
> Christian Lamparter  wrote:
> > 
> > I tried to move ppc4xx-rng into crypto4xx (see attachment - patch #1).
> > The driver works as is. But I can't come up with a way to attach the
> > crypto4xx driver to the ppc4xx-rng OF node cleanly. Basically,
> > I'm looking for a way to have one driver (with one context) be 
> > in charge of two different OF nodes (ppc4xx-rng and ppc4xx-crypto).
> > Is there any solution to this? Because otherwise, I would add a
> 
> Is it possible to have an RNG unit without the crypto unit?
No. In AMCC's product brief the TRNG (true random number generator) is
part of the security engine. The security engine also provides more 
features like a "public key authentication" core which has it's own
address range.


> If not then your first patch should be OK, provided that you add
> some error handling when the RNG probe fails.  For example, if 
> the RNG probe fails we should probably not call remove on it later.
I checked the error handling code again and verified it works on the
device. The original code resets the dev->trng_base = NULL and
core_dev->trng = NULL; in the err_out case. the "dev and core_dev"
are coming from the main crypto driver, they will always be
valid. Hence ppc4xx_trng_remove can safely be executed, even if 
ppc4xx_trng_probe fails as devm_hwrng_unregister, iounmap and kfree
can deal with the NULL properly. Nevertheless, I added an early
bailout if core_dev->trng == NULL.

what else I fixed in v1->v2: 
 - added a check to test trng device's status state with
   of_device_is_available.
 - if the hwrng device registration failed, the flag which
   enables the trng was left enabled (note: the v1 code
   disabled the hwrng device as part of crypto4xx_remove.
   so it wasn't enabled when the crypto4xx driver was
   unloaded)

---
>From c0b580a50bdade97f0d06c98fc7dccbf64d25eb2 Mon Sep 17 00:00:00 2001
From: Christian Lamparter 
Date: Mon, 18 Apr 2016 12:57:41 +0200
Subject: [PATCH v2] crypto4xx: integrate ppc4xx-rng into crypto4xx

This patch integrates the ppc4xx-rng driver into the existing
crypto4xx. This is because the true random number generator
is controlled and part of the security core.

Signed-off-by: Christian Lamparter 
---
 drivers/char/hw_random/Kconfig  |  13 ---
 drivers/char/hw_random/Makefile |   1 -
 drivers/char/hw_random/ppc4xx-rng.c | 147 
 drivers/crypto/Kconfig  |   8 ++
 drivers/crypto/amcc/Makefile|   1 +
 drivers/crypto/amcc/crypto4xx_core.c|   7 +-
 drivers/crypto/amcc/crypto4xx_core.h|   4 +
 drivers/crypto/amcc/crypto4xx_reg_def.h |   1 +
 drivers/crypto/amcc/crypto4xx_trng.c| 131 
 drivers/crypto/amcc/crypto4xx_trng.h|  34 
 10 files changed, 184 insertions(+), 163 deletions(-)
 delete mode 100644 drivers/char/hw_random/ppc4xx-rng.c
 create mode 100644 drivers/crypto/amcc/crypto4xx_trng.c
 create mode 100644 drivers/crypto/amcc/crypto4xx_trng.h

diff --git a/drivers/char/hw_random/Kconfig b/drivers/char/hw_random/Kconfig
index 3c5be60..abc8720 100644
--- a/drivers/char/hw_random/Kconfig
+++ b/drivers/char/hw_random/Kconfig
@@ -268,19 +268,6 @@ config HW_RANDOM_NOMADIK
 
  If unsure, say Y.
 
-config HW_RANDOM_PPC4XX
-   tristate "PowerPC 4xx generic true random number generator support"
-   depends on PPC && 4xx
-   default HW_RANDOM
-   ---help---
-This driver provides the kernel-side support for the TRNG hardware
-found in the security function of some PowerPC 4xx SoCs.
-
-To compile this driver as a module, choose M here: the
-module will be called ppc4xx-rng.
-
-If unsure, say N.
-
 config HW_RANDOM_PSERIES
tristate "pSeries HW Random Number Generator support"
depends on PPC64 && IBMVIO
diff --git a/drivers/char/hw_random/Makefile b/drivers/char/hw_random/Makefile
index f5a6fa7..079745f 100644
--- a/drivers/char/hw_random/Makefile
+++ b/drivers/char/hw_random/Makefile
@@ -22,7 +22,6 @@ obj-$(CONFIG_HW_RANDOM_TX4939) += tx4939-rng.o
 obj-$(CONFIG_HW_RANDOM_MXC_RNGA) += mxc-rnga.o
 obj-$(CONFIG_HW_RANDOM_OCTEON) += octeon-rng.o
 obj-$(CONFIG_HW_RANDOM_NOMADIK) += nomadik-rng.o
-obj-$(CONFIG_HW_RANDOM_PPC4XX) += ppc4xx-rng.o
 obj-$(CONFIG_HW_RANDOM_PSERIES) += pseries-rng.o
 obj-$(CONFIG_HW_RANDOM_POWERNV) += powernv-rng.o
 obj-$(CONFIG_HW_RANDOM_EXYNOS) += exynos-rng.o
diff --git a/drivers/char/hw_random/ppc4xx-rng.c 
b/drivers/char/hw_random/ppc4xx-rng.c
deleted file mode 100644
index c0db438..000
--- a/drivers/char/hw_random/ppc4xx-rng.c
+++ /dev/null
@@ -1,147 +0,0 @@
-/*
- * Generic PowerPC 44x RNG driver
- *
- * Copyright 2011 IBM Corporation
- *
- * This program is free software; you can redistribute it

Re: crash in ppc4xx-rng on canyonland

2016-04-18 Thread Herbert Xu 
Christian Lamparter  wrote:
> 
> I tried to move ppc4xx-rng into crypto4xx (see attachment - patch #1).
> The driver works as is. But I can't come up with a way to attach the
> crypto4xx driver to the ppc4xx-rng OF node cleanly. Basically,
> I'm looking for a way to have one driver (with one context) be 
> in charge of two different OF nodes (ppc4xx-rng and ppc4xx-crypto).
> Is there any solution to this? Because otherwise, I would add a

Is it possible to have an RNG unit without the crypto unit? If not
then your first patch should be OK, provided that you add some error
handling when the RNG probe fails.  For example, if the RNG probe
fails we should probably not call remove on it later.

Cheers,
-- 
Email: Herbert Xu 
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
--
To unsubscribe from this list: send the line "unsubscribe linux-crypto" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: crash in ppc4xx-rng on canyonland

2016-04-08 Thread Christian Lamparter 
On Tuesday, April 05, 2016 08:11:19 PM Herbert Xu wrote:
> Christian Lamparter  wrote:
> >
> > The crash is caused by a bad read in ppc4xx_rng_enable [0]. From what I
> > can tell, the driver is mapping the crypto control registers. The
> > problem is that they are claimed by the main crypto driver: crypto4xx [1].
> > 
> > I'm not sure what to do in this case. In my opinion the crypto4xx driver
> > should just initialize the trng [see patch]. I would like to move the
> > trng into the crypto-ppc4xx, but given that this breaks some of the DTS
> > out there I'm not sure.
> 
> I think you'll also need to ensure that the crypto module is
> successfully loaded before the RNG is accessed.
> 
> It's probably easier to just merge the two drivers but still
> maintaining the original driver names.

I tried to move ppc4xx-rng into crypto4xx (see attachment - patch #1).
The driver works as is. But I can't come up with a way to attach the
crypto4xx driver to the ppc4xx-rng OF node cleanly. Basically,
I'm looking for a way to have one driver (with one context) be 
in charge of two different OF nodes (ppc4xx-rng and ppc4xx-crypto).
Is there any solution to this? Because otherwise, I would add a
bool ppc4xx_trng variable in crypto4xx_core.c and use EXPORT_SYMBOL...
[see patch #2]


---
Patch #1

>From 156bcb0eb06361c1668237ed2d0c3227b0837ff3 Mon Sep 17 00:00:00 2001
From: Christian Lamparter 
Date: Fri, 25 Mar 2016 19:00:05 +0100
Subject: [PATCH] crypto4xx: integrate ppc4xx-rng into crypto4xx

This patch integrates the ppc4xx-rng driver into the existing
crypto4xx. This is because the true random number generator
is controlled and part of the security core.

Signed-off-by: Christian Lamparter 
---
 drivers/char/hw_random/Kconfig  |  13 ---
 drivers/char/hw_random/Makefile |   1 -
 drivers/char/hw_random/ppc4xx-rng.c | 147 
 drivers/crypto/Kconfig  |   8 ++
 drivers/crypto/amcc/Makefile|   1 +
 drivers/crypto/amcc/crypto4xx_core.c|   7 +-
 drivers/crypto/amcc/crypto4xx_core.h|   4 +
 drivers/crypto/amcc/crypto4xx_reg_def.h |   1 +
 drivers/crypto/amcc/crypto4xx_trng.c| 128 +++
 drivers/crypto/amcc/crypto4xx_trng.h|  34 
 10 files changed, 181 insertions(+), 163 deletions(-)
 delete mode 100644 drivers/char/hw_random/ppc4xx-rng.c
 create mode 100644 drivers/crypto/amcc/crypto4xx_trng.c
 create mode 100644 drivers/crypto/amcc/crypto4xx_trng.h

diff --git a/drivers/char/hw_random/Kconfig b/drivers/char/hw_random/Kconfig
index 67ee8b0..a6d7b9b 100644
--- a/drivers/char/hw_random/Kconfig
+++ b/drivers/char/hw_random/Kconfig
@@ -268,19 +268,6 @@ config HW_RANDOM_NOMADIK
 
  If unsure, say Y.
 
-config HW_RANDOM_PPC4XX
-   tristate "PowerPC 4xx generic true random number generator support"
-   depends on PPC && 4xx
-   default HW_RANDOM
-   ---help---
-This driver provides the kernel-side support for the TRNG hardware
-found in the security function of some PowerPC 4xx SoCs.
-
-To compile this driver as a module, choose M here: the
-module will be called ppc4xx-rng.
-
-If unsure, say N.
-
 config HW_RANDOM_PSERIES
tristate "pSeries HW Random Number Generator support"
depends on PPC64 && IBMVIO
diff --git a/drivers/char/hw_random/Makefile b/drivers/char/hw_random/Makefile
index f5a6fa7..079745f 100644
--- a/drivers/char/hw_random/Makefile
+++ b/drivers/char/hw_random/Makefile
@@ -22,7 +22,6 @@ obj-$(CONFIG_HW_RANDOM_TX4939) += tx4939-rng.o
 obj-$(CONFIG_HW_RANDOM_MXC_RNGA) += mxc-rnga.o
 obj-$(CONFIG_HW_RANDOM_OCTEON) += octeon-rng.o
 obj-$(CONFIG_HW_RANDOM_NOMADIK) += nomadik-rng.o
-obj-$(CONFIG_HW_RANDOM_PPC4XX) += ppc4xx-rng.o
 obj-$(CONFIG_HW_RANDOM_PSERIES) += pseries-rng.o
 obj-$(CONFIG_HW_RANDOM_POWERNV) += powernv-rng.o
 obj-$(CONFIG_HW_RANDOM_EXYNOS) += exynos-rng.o
diff --git a/drivers/char/hw_random/ppc4xx-rng.c 
b/drivers/char/hw_random/ppc4xx-rng.c
deleted file mode 100644
index c0db438..000
--- a/drivers/char/hw_random/ppc4xx-rng.c
+++ /dev/null
@@ -1,147 +0,0 @@
-/*
- * Generic PowerPC 44x RNG driver
- *
- * Copyright 2011 IBM Corporation
- *
- * This program is free software; you can redistribute it and/or modify it
- * under the terms of the GNU General Public License as published by the
- * Free Software Foundation; version 2 of the License.
- */
-
-#include 
-#include 
-#include 
-#include 
-#include 
-#include 
-#include 
-#include 
-
-#define PPC4XX_TRNG_DEV_CTRL 0x60080
-
-#define PPC4XX_TRNGE 0x0002
-#define PPC4XX_TRNG_CTRL 0x0008
-#define PPC4XX_TRNG_CTRL_DALM 0x20
-#define PPC4XX_TRNG_STAT 0x0004
-#define PPC4XX_TRNG_STAT_B 0x1
-#define PPC4XX_TRNG_DATA 0x
-
-#define MODULE_NAME "ppc4xx_rng"
-
-static int ppc4xx_rng_data_present(struct hwrng *rng, int wait)
-{
-   void __iomem *rng_regs = (void __iomem *)

Re: crash in ppc4xx-rng on canyonland

2016-04-05 Thread Herbert Xu 
Christian Lamparter  wrote:
>
> The crash is caused by a bad read in ppc4xx_rng_enable [0]. From what I
> can tell, the driver is mapping the crypto control registers. The
> problem is that they are claimed by the main crypto driver: crypto4xx [1].
> 
> I'm not sure what to do in this case. In my opinion the crypto4xx driver
> should just initialize the trng [see patch]. I would like to move the
> trng into the crypto-ppc4xx, but given that this breaks some of the DTS
> out there I'm not sure.

I think you'll also need to ensure that the crypto module is
successfully loaded before the RNG is accessed.

It's probably easier to just merge the two drivers but still
maintaining the original driver names.

Cheers,
-- 
Email: Herbert Xu 
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
--
To unsubscribe from this list: send the line "unsubscribe linux-crypto" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

crash in ppc4xx-rng on canyonland

2016-03-25 Thread Christian Lamparter 
I'm currently trying to port a Western Digital MyBook Live to
a 4.4.6 kernel. The device has a APM82181 and the board is called
Apollo-3G, which is a derivative of the "amcc,canyonland".

Almost everything is working, except when I try to enable the
ppc4xx-rng in the dts. Then the machine dies with the following:

[0.801839] Machine check in kernel mode.
[0.805830] Data Read PLB Error
[0.808962] Oops: Machine check, sig: 7 [#1]
[0.813208] Canyonlands
[0.815646] Modules linked in:
[0.818710] CPU: 0 PID: 1 Comm: swapper Not tainted 4.4.6 #39
[0.824436] task: cf83 ti: cfff2000 task.ti: cf834000
[0.829808] NIP: c01b9a00 LR: c01b99e8 CTR: 
[0.834751] REGS: cfff3f10 TRAP: 0214   Not tainted  (4.4.6)
[0.840382] MSR: 00029000   CR: 820c3222  XER: 
[0.846515] 
GPR00: c01b99e8 cf835d50 cf83 d108 d110 cf857800 0020 0004 
GPR08:  d10e d10e0080 0020051b 820c3428  c0001c54  
GPR16:       c042 c03d7290 
GPR24: 0043  c044e1cc c03a500f 0001  cfffb9b8 0005 
[0.876429] NIP [c01b9a00] ppc4xx_rng_enable+0xec/0x12c
[0.881634] LR [c01b99e8] ppc4xx_rng_enable+0xd4/0x12c
[0.886752] Call Trace:
[0.889194] [cf835d50] [c01b99e8] ppc4xx_rng_enable+0xd4/0x12c (unreliable)
[0.896141] [cf835db0] [c01b9ab8] ppc4xx_rng_probe+0x2c/0x80
[0.901791] [cf835dc0] [c01c0614] platform_drv_probe+0x34/0x70
[0.907606] [cf835dd0] [c01beda8] driver_probe_device+0x110/0x264
[0.913679] [cf835e00] [c01bef74] __driver_attach+0x78/0xac
[0.919243] [cf835e20] [c01bd300] bus_for_each_dev+0x9c/0xac
[0.924885] [cf835e50] [c01be49c] bus_add_driver+0xe0/0x1fc
[0.930441] [cf835e70] [c01bf688] driver_register+0xb4/0x104
[0.936086] [cf835e90] [c0001704] do_one_initcall+0x11c/0x1ac
[0.941825] [cf835f00] [c03d7af0] kernel_init_freeable+0x128/0x1c4
[0.947989] [cf835f30] [c0001c68] kernel_init+0x14/0x114
[0.953288] [cf835f40] [c000a748] ret_from_kernel_thread+0x5c/0x64
[0.959449] Instruction dump:
[0.962415] 2f9f0004 3bff0001 409effa8 3880 7fc3f378 4806f6c5 2c03 
41a2ff68 
[0.970197] 3d230006 39490080 7c0004ac 7d00542c <0c08> 4c00012c 2f9c 
550a03da 
[0.978180] ---[ end trace cff1212128646932 ]---

The crash is caused by a bad read in ppc4xx_rng_enable [0]. From what I
can tell, the driver is mapping the crypto control registers. The
problem is that they are claimed by the main crypto driver: crypto4xx [1].

I'm not sure what to do in this case. In my opinion the crypto4xx driver
should just initialize the trng [see patch]. I would like to move the
trng into the crypto-ppc4xx, but given that this breaks some of the DTS
out there I'm not sure.

Regards,
Christian
---
>From 2159e200fcb68f88a94b1d5570d6000c100133a8 Mon Sep 17 00:00:00 2001
From: Christian Lamparter 
Date: Fri, 25 Mar 2016 19:00:05 +0100
Subject: [PATCH] ppc4xx-rng: fix crash during init

This patch fixes a crash that happens in the ppc4xx-rng driver
when accessing the main crypto core to enable the true random
number generator.

Signed-off-by: Christian Lamparter 
---
 drivers/char/hw_random/ppc4xx-rng.c | 42 -
 drivers/crypto/amcc/crypto4xx_core.c|  1 +
 drivers/crypto/amcc/crypto4xx_reg_def.h |  1 +
 3 files changed, 2 insertions(+), 42 deletions(-)

diff --git a/drivers/char/hw_random/ppc4xx-rng.c 
b/drivers/char/hw_random/ppc4xx-rng.c
index c0db438..a9a8a29 100644
--- a/drivers/char/hw_random/ppc4xx-rng.c
+++ b/drivers/char/hw_random/ppc4xx-rng.c
@@ -17,9 +17,6 @@
 #include 
 #include 
 
-#define PPC4XX_TRNG_DEV_CTRL 0x60080
-
-#define PPC4XX_TRNGE 0x0002
 #define PPC4XX_TRNG_CTRL 0x0008
 #define PPC4XX_TRNG_CTRL_DALM 0x20
 #define PPC4XX_TRNG_STAT 0x0004
@@ -51,40 +48,6 @@ static int ppc4xx_rng_data_read(struct hwrng *rng, u32 *data)
return 4;
 }
 
-static int ppc4xx_rng_enable(int enable)
-{
-   struct device_node *ctrl;
-   void __iomem *ctrl_reg;
-   int err = 0;
-   u32 val;
-
-   /* Find the main crypto device node and map it to turn the TRNG on */
-   ctrl = of_find_compatible_node(NULL, NULL, "amcc,ppc4xx-crypto");
-   if (!ctrl)
-   return -ENODEV;
-
-   ctrl_reg = of_iomap(ctrl, 0);
-   if (!ctrl_reg) {
-   err = -ENODEV;
-   goto out;
-   }
-
-   val = in_le32(ctrl_reg + PPC4XX_TRNG_DEV_CTRL);
-
-   if (enable)
-   val |= PPC4XX_TRNGE;
-   else
-   val = val & ~PPC4XX_TRNGE;
-
-   out_le32(ctrl_reg + PPC4XX_TRNG_DEV_CTRL, val);
-   iounmap(ctrl_reg);
-
-out:
-   of_node_put(ctrl);
-
-   return err;
-}
-
 static struct hwrng ppc4xx_rng = {
.name = MODULE_NAME,
.data_present = ppc4xx_rng_data_present,
@@ -100,10 +63,6 @@ static int ppc4xx_rng_probe(struct