by Pali Rohár

[permalink] [raw]

Subject: Re: [PATCH 00/10] omap3 crypto fixes

On Thursday 26 February 2015 23:46:05 Aaro Koskinen wrote:
> Hi,
>
> On Thu, Feb 26, 2015 at 02:49:50PM +0100, Pali Rohár wrote:
> > This patch series fix crypto support for omap3 devices which
> > use DT.
> >
> > It enables AES and SHAM on N9/N950 and SHAM on N900. AES is
> > still disabled for N900.
>
> (Please format your lines somewhere near < 76 chars,
> especially in commit logs, otherwise "git log" looks ugly on
> 80 char terminals.)
>
> I tested these with stock bootloaders, and devices boot
> normally and I can now modprobe omap-aes (on N9 and N950) and
> omap-sham (on all three):
>
> omap-aes 480c5000.aes: OMAP AES hw accel rev: 2.6
> omap-sham 480c3000.sham: hw accel on OMAP rev 0.9
>
> However I get these when CONFIG_CRYPTO_MANAGER_DISABLE_TESTS
> is not set:
>
> alg: hash: Chunking test 1 failed for omap-sha1
> alg: hash: Chunking test 1 failed for omap-md5
> alg: hash: Chunking test 1 failed for omap-hmac-sha1
> alg: hash: Chunking test 1 failed for omap-hmac-md5
>
> But that's probably unrelated to this series. For patches 1-8,
> feel free to add:
>
> Tested-by: Aaro Koskinen <[email protected]>
>
> A.

Hi, thanks for testing!

Now I'm happy that I can load omap-aes and omap-sham drivers on
real n900 device, qemu n900 and after your tests also on real n9
and n950 without any kernel crash or oops. Of course omap-aes is
disabled on n900, but loading omap-aes.ko (accidentally) does not
hurt.

It looks like there are other parts in md5 and sha broken, but
first step is load & register all drivers without kernel crash.
And this is working now.

--
Pali Rohár
[email protected]

Attachments:

signature.asc (198.00 B)
This is a digitally signed message part.

2015-02-27 16:01:55

On Sunday 08 March 2015 17:35:13 Paul Walmsley wrote:
> On Sun, 8 Mar 2015, Pali Rohár wrote:
> > On Friday 06 March 2015 23:23:06 Aaro Koskinen wrote:
> > > On Fri, Mar 06, 2015 at 10:36:32AM -0800, Tony Lindgren
wrote:
> > > > Are there any fixes in this series that should go into
> > > > v4.0-rc series, or can it all wait for v4.1?
> > >
> > > I think these all should wait for v4.1.
> > >
> > > A.
> >
> > I would suggest to include at least patches 01, 04, 06.
> > Probably those could go to -stable tree... but this
> > decision is up to you.
>
> I'm not sure patch 1 is a fix. As far as I know we haven't
> run into any issues with it on real hardware - only on QEMU -
> unless you know otherwise, Pali? Are we sure that the QEMU
> model behavior matches the hardware?
>
>
> - Paul

Patch 1 check for return value of more functions. If real HW or
software emulated HW (in qemu) does not support e.g. aes then
kernel show oops message, because kernel does not check return
values and try to touch non-existent HW. So I think patch 1 is
really fix. In my opinion if something can fail, then kernel
should check if it failed. And not expect that function call
always pass.

--
Pali Rohár
[email protected]

Attachments:

signature.asc (198.00 B)
This is a digitally signed message part.

2015-03-16 21:49:33

by Tony Lindgren

[permalink] [raw]

Subject: Re: [PATCH 06/10] ARM: dts: omap3: Add missing dmas for crypto

* Pavel Machek <[email protected]> [150228 08:49]:
> On Thu 2015-02-26 14:49:56, Pali Rohár wrote:
> > This patch adds missing dma DTS definitions for omap aes and sham drivers.
> > Without it kernel drivers do not work.
> >
> > Signed-off-by: Pali Rohár <[email protected]>
>
> Acked-by: Pavel Machek <[email protected]>

Applying this into omap-for-v4.0/fixes to remove the regression
for legacy vs DT based booting for GP omap3 boards.

Tony

2015-03-19 16:43:32

by Tony Lindgren

[permalink] [raw]

Subject: Re: [PATCH 07/10] ARM: dts: n9/n950: Enable omap crypto support

* Pavel Machek <[email protected]> [150228 08:45]:
> On Thu 2015-02-26 14:49:57, Pali Rohár wrote:
> > Harmattan system on Nokia N9 and N950 devices uses omap crypto support.
> > Bootloader on those devices is known that it enables HW crypto support.
> > This patch just include omap36xx.dtsi directly, so aes and sham is enabled.
> >
> > Signed-off-by: Pali Rohár <[email protected]>
>
> Acked-by: Pavel Machek <[email protected]>

I'm picking patches 7 - 10 into omap-for-v4.1/dt branch thanks.

Tony

2015-05-14 21:17:41

On Thursday 14 May 2015 23:34:19 Tony Lindgren wrote:
> * Pali Rohár <[email protected]> [150514 14:19]:
> > On Friday 06 March 2015 23:23:06 Aaro Koskinen wrote:
> > > Hi,
> > >
> > > On Fri, Mar 06, 2015 at 10:36:32AM -0800, Tony Lindgren wrote:
> > > > Are there any fixes in this series that should go into
> > > > v4.0-rc series, or can it all wait for v4.1?
> > >
> > > I think these all should wait for v4.1.
> > >
> > > A.
> >
> > 4.1-rc3 was released and this patch series is still not included...
>
> The dts changes should be merged?
>

Looks like yes, but first two arch/arm/mach-omap2 patches not.

> I suggest you repost the driver fixes so Herbert can queue them.
>

Ok.

> Regards,
>
> Tony

--
Pali Rohár
[email protected]

Attachments:

signature.asc (198.00 B)
This is a digitally signed message part.

2015-05-14 21:40:37

On Tue, 26 May 2015, Pali Rohár wrote:

> Hi Paul,
>
> this patch is also for omap2... Can you review it too?
>
> On Saturday 28 February 2015 17:24:36 Pavel Machek wrote:
> > On Thu 2015-02-26 14:49:52, Pali Rohár wrote:
> > > Register crypto hwmod links only if they are not disabled in DT.
> > > If DT information is missing, enable them only for GP devices.
> > >
> > > Before this patch crypto hwmod links were always disabled for all HS devices
> > > and it was not possible to use omap-aes and omap-sham linux drivers.
> > >
> > > Signed-off-by: Pali Rohár <[email protected]>
> >
> > Acked-by: Pavel Machek <[email protected]>
> >

Sent to Tony for v4.2, after tweaking it a bit (below)

- Paul

Subject: [PATCH] ARM: OMAP3: Fix crypto support for HS devices

Register crypto hwmod links only if they are not disabled in DT.
If DT information is missing, enable them only for GP devices.

Before this patch crypto hwmod links were always disabled for all HS
devices and it was not possible to use omap-aes and omap-sham linux
drivers.

Signed-off-by: Pali Rohár <[email protected]>
Acked-by: Pavel Machek <[email protected]>
[[email protected]: move the complex IP-block presence heuristics into their
own function to simplify the code; fix some checkpatch warnings]
Signed-off-by: Paul Walmsley <[email protected]>
---
arch/arm/mach-omap2/omap_hwmod_3xxx_data.c | 107 +++++++++++++++++++++++++----
1 file changed, 94 insertions(+), 13 deletions(-)

diff --git a/arch/arm/mach-omap2/omap_hwmod_3xxx_data.c b/arch/arm/mach-omap2/omap_hwmod_3xxx_data.c
index 0ca4d3fb7df6..dc55f8dedf2c 100644
--- a/arch/arm/mach-omap2/omap_hwmod_3xxx_data.c
+++ b/arch/arm/mach-omap2/omap_hwmod_3xxx_data.c
@@ -3736,29 +3736,54 @@ static struct omap_hwmod_ocp_if *omap3xxx_hwmod_ocp_ifs[] __initdata = {
/* GP-only hwmod links */
static struct omap_hwmod_ocp_if *omap34xx_gp_hwmod_ocp_ifs[] __initdata = {
&omap3xxx_l4_sec__timer12,
- &omap3xxx_l4_core__sham,
- &omap3xxx_l4_core__aes,
NULL
};

static struct omap_hwmod_ocp_if *omap36xx_gp_hwmod_ocp_ifs[] __initdata = {
&omap3xxx_l4_sec__timer12,
- &omap3xxx_l4_core__sham,
- &omap3xxx_l4_core__aes,
NULL
};

static struct omap_hwmod_ocp_if *am35xx_gp_hwmod_ocp_ifs[] __initdata = {
&omap3xxx_l4_sec__timer12,
- /*
- * Apparently the SHA/MD5 and AES accelerator IP blocks are
- * only present on some AM35xx chips, and no one knows which
- * ones. See
- * http://www.spinics.net/lists/arm-kernel/msg215466.html So
- * if you need these IP blocks on an AM35xx, try uncommenting
- * the following lines.
- */
+ NULL
+};
+
+/* crypto hwmod links */
+static struct omap_hwmod_ocp_if *omap34xx_sham_hwmod_ocp_ifs[] __initdata = {
+ &omap3xxx_l4_core__sham,
+ NULL
+};
+
+static struct omap_hwmod_ocp_if *omap34xx_aes_hwmod_ocp_ifs[] __initdata = {
+ &omap3xxx_l4_core__aes,
+ NULL
+};
+
+static struct omap_hwmod_ocp_if *omap36xx_sham_hwmod_ocp_ifs[] __initdata = {
+ &omap3xxx_l4_core__sham,
+ NULL
+};
+
+static struct omap_hwmod_ocp_if *omap36xx_aes_hwmod_ocp_ifs[] __initdata = {
+ &omap3xxx_l4_core__aes,
+ NULL
+};
+
+/*
+ * Apparently the SHA/MD5 and AES accelerator IP blocks are
+ * only present on some AM35xx chips, and no one knows which
+ * ones. See
+ * http://www.spinics.net/lists/arm-kernel/msg215466.html So
+ * if you need these IP blocks on an AM35xx, try uncommenting
+ * the following lines.
+ */
+static struct omap_hwmod_ocp_if *am35xx_sham_hwmod_ocp_ifs[] __initdata = {
/* &omap3xxx_l4_core__sham, */
+ NULL
+};
+
+static struct omap_hwmod_ocp_if *am35xx_aes_hwmod_ocp_ifs[] __initdata = {
/* &omap3xxx_l4_core__aes, */
NULL
};
@@ -3860,10 +3885,41 @@ static struct omap_hwmod_ocp_if *omap3xxx_dss_hwmod_ocp_ifs[] __initdata = {
NULL
};

+/**
+ * omap3xxx_hwmod_is_hs_ip_block_usable - is a security IP block accessible?
+ * @bus: struct device_node * for the top-level OMAP DT data
+ * @dev_name: device name used in the DT file
+ *
+ * Determine whether a "secure" IP block @dev_name is usable by Linux.
+ * There doesn't appear to be a 100% reliable way to determine this,
+ * so we rely on heuristics. If @bus is null, meaning there's no DT
+ * data, then we only assume the IP block is accessible if the OMAP is
+ * fused as a 'general-purpose' SoC. If however DT data is present,
+ * test to see if the IP block is described in the DT data and set to
+ * 'status = "okay"'. If so then we assume the ODM has configured the
+ * OMAP firewalls to allow access to the IP block.
+ *
+ * Return: 0 if device named @dev_name is not likely to be accessible,
+ * or 1 if it is likely to be accessible.
+ */
+static int __init omap3xxx_hwmod_is_hs_ip_block_usable(struct device_node *bus,
+ const char *dev_name)
+{
+ if (!bus)
+ return (omap_type() == OMAP2_DEVICE_TYPE_GP) ? 1 : 0;
+
+ if (of_device_is_available(of_find_node_by_name(bus, dev_name)))
+ return 1;
+
+ return 0;
+}
+
int __init omap3xxx_hwmod_init(void)
{
int r;
- struct omap_hwmod_ocp_if **h = NULL, **h_gp = NULL;
+ struct omap_hwmod_ocp_if **h = NULL, **h_gp = NULL, **h_sham = NULL;
+ struct omap_hwmod_ocp_if **h_aes = NULL;
+ struct device_node *bus = NULL;
unsigned int rev;

omap_hwmod_init();
@@ -3885,13 +3941,19 @@ int __init omap3xxx_hwmod_init(void)
rev == OMAP3430_REV_ES3_1 || rev == OMAP3430_REV_ES3_1_2) {
h = omap34xx_hwmod_ocp_ifs;
h_gp = omap34xx_gp_hwmod_ocp_ifs;
+ h_sham = omap34xx_sham_hwmod_ocp_ifs;
+ h_aes = omap34xx_aes_hwmod_ocp_ifs;
} else if (rev == AM35XX_REV_ES1_0 || rev == AM35XX_REV_ES1_1) {
h = am35xx_hwmod_ocp_ifs;
h_gp = am35xx_gp_hwmod_ocp_ifs;
+ h_sham = am35xx_sham_hwmod_ocp_ifs;
+ h_aes = am35xx_aes_hwmod_ocp_ifs;
} else if (rev == OMAP3630_REV_ES1_0 || rev == OMAP3630_REV_ES1_1 ||
rev == OMAP3630_REV_ES1_2) {
h = omap36xx_hwmod_ocp_ifs;
h_gp = omap36xx_gp_hwmod_ocp_ifs;
+ h_sham = omap36xx_sham_hwmod_ocp_ifs;
+ h_aes = omap36xx_aes_hwmod_ocp_ifs;
} else {
WARN(1, "OMAP3 hwmod family init: unknown chip type\n");
return -EINVAL;
@@ -3908,6 +3970,25 @@ int __init omap3xxx_hwmod_init(void)
return r;
}

+ /*
+ * Register crypto hwmod links only if they are not disabled in DT.
+ * If DT information is missing, enable them only for GP devices.
+ */
+
+ if (of_have_populated_dt())
+ bus = of_find_node_by_name(NULL, "ocp");
+
+ if (h_sham && omap3xxx_hwmod_is_hs_ip_block_usable(bus, "sham")) {
+ r = omap_hwmod_register_links(h_sham);
+ if (r < 0)
+ return r;
+ }
+
+ if (h_aes && omap3xxx_hwmod_is_hs_ip_block_usable(bus, "aes")) {
+ r = omap_hwmod_register_links(h_aes);
+ if (r < 0)
+ return r;
+ }

/*
* Register hwmod links specific to certain ES levels of a
--
2.1.4