[PATCH 0/3] Add Broadcom SPU Crypto Driver

[PATCH 0/3] Add Broadcom SPU Crypto Driver

[Rob Rice]

The Broadcom SPU crypto driver provides access to SPU hardware
for symmetric crypto offload. The driver supports ablkcipher,
ahash, and aead operations. The driver supports several
Broadcom SoCs with different revisions of the SPU hardware.
The driver supports SPU-M and SPU2 hardware revisions, and
a couple versions of each hw revision, each version with minor
differences.

The device tree entries for the SPU depends on device tree entries
for the Broadcom PDC driver. The PDC DT entries have been accepted
upstream, but are not yet included in the crypto repo. See commit
e79249143f468f8d3365dbbd1642c045bdcc98c5.

Rob Rice (3):
  crypto: brcm: DT documentation for Broadcom SPU driver
  crypto: brcm: Add Broadcom SPU driver
  crypto: brcm: Add Broadcom SPU driver DT entry.

 .../devicetree/bindings/crypto/brcm,spu-crypto.txt |   25 +
 arch/arm64/boot/dts/broadcom/ns2.dtsi              |   12 +
 drivers/crypto/Kconfig                             |   10 +
 drivers/crypto/Makefile                            |    1 +
 drivers/crypto/bcm/Makefile                        |   15 +
 drivers/crypto/bcm/cipher.c                        | 4943 ++++++++++++++++++++
 drivers/crypto/bcm/cipher.h                        |  472 ++
 drivers/crypto/bcm/spu.c                           | 1252 +++++
 drivers/crypto/bcm/spu.h                           |  288 ++
 drivers/crypto/bcm/spu2.c                          | 1402 ++++++
 drivers/crypto/bcm/spu2.h                          |  228 +
 drivers/crypto/bcm/spum.h                          |  174 +
 drivers/crypto/bcm/util.c                          |  584 +++
 drivers/crypto/bcm/util.h                          |  117 +
 14 files changed, 9523 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/crypto/brcm,spu-crypto.txt
 create mode 100644 drivers/crypto/bcm/Makefile
 create mode 100644 drivers/crypto/bcm/cipher.c
 create mode 100644 drivers/crypto/bcm/cipher.h
 create mode 100644 drivers/crypto/bcm/spu.c
 create mode 100644 drivers/crypto/bcm/spu.h
 create mode 100644 drivers/crypto/bcm/spu2.c
 create mode 100644 drivers/crypto/bcm/spu2.h
 create mode 100644 drivers/crypto/bcm/spum.h
 create mode 100644 drivers/crypto/bcm/util.c
 create mode 100644 drivers/crypto/bcm/util.h

--
2.1.0

[PATCH 1/3] crypto: brcm: DT documentation for Broadcom SPU driver

[Rob Rice]

Device tree documentation for Broadcom Secure Processing Unit
(SPU) crypto driver.

Signed-off-by: Steve Lin <steven.lin1@broadcom.com>
Signed-off-by: Rob Rice <rob.rice@broadcom.com>
---
 .../devicetree/bindings/crypto/brcm,spu-crypto.txt | 25 ++++++++++++++++++++++
 1 file changed, 25 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/crypto/brcm,spu-crypto.txt

diff --git a/Documentation/devicetree/bindings/crypto/brcm,spu-crypto.txt b/Documentation/devicetree/bindings/crypto/brcm,spu-crypto.txt
new file mode 100644
index 0000000..e5fe942
--- /dev/null
+++ b/Documentation/devicetree/bindings/crypto/brcm,spu-crypto.txt
@@ -0,0 +1,25 @@
+The Broadcom Secure Processing Unit (SPU) driver supports symmetric
+cryptographic offload for Broadcom SoCs with SPU hardware. A SoC may have
+multiple SPU hardware blocks.
+
+Required properties:
+- compatible : Should be "brcm,spum-crypto" for devices with SPU-M hardware
+  (e.g., Northstar2) or "brcm,spum-nsp-crypto" for the Northstar Plus variant
+  of the SPU-M hardware.
+
+- reg: Should contain SPU registers location and length.
+- mboxes: A list of mailbox channels to be used by the kernel driver. Mailbox
+channels correspond to DMA rings on the device.
+
+Example:
+	spu-crypto at 612d0000 {
+		compatible = "brcm,spum-crypto";
+		reg = <0 0x612d0000 0 0x900>,    /* SPU 0 control regs */
+			<0 0x612f0000 0 0x900>,  /* SPU 1 control regs */
+			<0 0x61310000 0 0x900>,  /* SPU 2 control regs */
+			<0 0x61330000 0 0x900>;  /* SPU 3 control regs */
+		mboxes = <&pdc0 0>,
+			<&pdc1 0>,
+			<&pdc2 0>,
+			<&pdc3 0>;
+	};
--
2.1.0

[PATCH 3/3] crypto: brcm: Add Broadcom SPU driver DT entry.

[Rob Rice]

Add Northstar2 device tree entry for Broadcom Secure Processing
Unit (SPU) crypto driver.

Signed-off-by: Steve Lin <steven.lin1@broadcom.com>
Signed-off-by: Rob Rice <rob.rice@broadcom.com>
---
 arch/arm64/boot/dts/broadcom/ns2.dtsi | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/arch/arm64/boot/dts/broadcom/ns2.dtsi b/arch/arm64/boot/dts/broadcom/ns2.dtsi
index d95dc40..8b81b0d 100644
--- a/arch/arm64/boot/dts/broadcom/ns2.dtsi
+++ b/arch/arm64/boot/dts/broadcom/ns2.dtsi
@@ -191,6 +191,18 @@

 		#include "ns2-clock.dtsi"

+		spu-crypto at 612d0000 {
+			compatible = "brcm,spum-crypto";
+			reg = <0x612d0000 0x900>,    /* SPU 0 control regs */
+				<0x612f0000 0x900>,  /* SPU 1 control regs */
+				<0x61310000 0x900>,  /* SPU 2 control regs */
+				<0x61330000 0x900>;  /* SPU 3 control regs */
+			mboxes = <&pdc0 0>,
+				<&pdc1 0>,
+				<&pdc2 0>,
+				<&pdc3 0>;
+		};
+
 		dma0: dma at 61360000 {
 			compatible = "arm,pl330", "arm,primecell";
 			reg = <0x61360000 0x1000>;
--
2.1.0

[PATCH 2/3] crypto: brcm: Add Broadcom SPU driver

[kbuild test robot]

Hi Rob,

[auto build test ERROR on cryptodev/master]
[also build test ERROR on v4.9-rc7 next-20161201]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Rob-Rice/crypto-brcm-DT-documentation-for-Broadcom-SPU-driver/20161202-010038
base:   https://git.kernel.org/pub/scm/linux/kernel/git/herbert/cryptodev-2.6.git master
config: arm64-defconfig (attached as .config)
compiler: aarch64-linux-gnu-gcc (Debian 6.1.1-9) 6.1.1 20160705
reproduce:
        wget https://git.kernel.org/cgit/linux/kernel/git/wfg/lkp-tests.git/plain/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # save the attached .config to linux build tree
        make.cross ARCH=arm64 

All errors (new ones prefixed by >>):

>> ERROR: "des_ekey" [drivers/crypto/bcm/bcm_crypto_spu.ko] undefined!

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation
-------------- next part --------------
A non-text attachment was scrubbed...
Name: .config.gz
Type: application/gzip
Size: 31957 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20161202/f5e756e2/attachment-0001.gz>

[PATCH 1/3] crypto: brcm: DT documentation for Broadcom SPU driver

[Mark Rutland]

On Wed, Nov 30, 2016 at 03:07:31PM -0500, Rob Rice wrote:
> Device tree documentation for Broadcom Secure Processing Unit
> (SPU) crypto driver.
> 
> Signed-off-by: Steve Lin <steven.lin1@broadcom.com>
> Signed-off-by: Rob Rice <rob.rice@broadcom.com>
> ---
>  .../devicetree/bindings/crypto/brcm,spu-crypto.txt | 25 ++++++++++++++++++++++
>  1 file changed, 25 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/crypto/brcm,spu-crypto.txt
> 
> diff --git a/Documentation/devicetree/bindings/crypto/brcm,spu-crypto.txt b/Documentation/devicetree/bindings/crypto/brcm,spu-crypto.txt
> new file mode 100644
> index 0000000..e5fe942
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/crypto/brcm,spu-crypto.txt
> @@ -0,0 +1,25 @@
> +The Broadcom Secure Processing Unit (SPU) driver supports symmetric
> +cryptographic offload for Broadcom SoCs with SPU hardware. A SoC may have
> +multiple SPU hardware blocks.

Bindings shound describe *hardware*, not *drivers*. Please drop mention
of the driver, and just decribe the hardware.

> +Required properties:
> +- compatible : Should be "brcm,spum-crypto" for devices with SPU-M hardware
> +  (e.g., Northstar2) or "brcm,spum-nsp-crypto" for the Northstar Plus variant
> +  of the SPU-M hardware.
> +
> +- reg: Should contain SPU registers location and length.
> +- mboxes: A list of mailbox channels to be used by the kernel driver. Mailbox
> +channels correspond to DMA rings on the device.
> +
> +Example:
> +	spu-crypto at 612d0000 {
> +		compatible = "brcm,spum-crypto";
> +		reg = <0 0x612d0000 0 0x900>,    /* SPU 0 control regs */
> +			<0 0x612f0000 0 0x900>,  /* SPU 1 control regs */
> +			<0 0x61310000 0 0x900>,  /* SPU 2 control regs */
> +			<0 0x61330000 0 0x900>;  /* SPU 3 control regs */

The above didn't mention there were several register sets, and the
comment beside each makes them sound like they're separate SPU
instances, so I don't think it makes sense to group them as one node.

What's going on here?

> +		mboxes = <&pdc0 0>,
> +			<&pdc1 0>,
> +			<&pdc2 0>,
> +			<&pdc3 0>;

Does each mbox correspond to one of the SPUs above? Or is there a shared
pool?

Thanks,
Mark.

[PATCH 2/3] crypto: brcm: Add Broadcom SPU driver

[Mark Rutland]

On Wed, Nov 30, 2016 at 03:07:32PM -0500, Rob Rice wrote:
> +static const struct of_device_id bcm_spu_dt_ids[] = {
> +	{
> +		.compatible = "brcm,spum-crypto",
> +		.data = &spum_ns2_types,
> +	},
> +	{
> +		.compatible = "brcm,spum-nsp-crypto",
> +		.data = &spum_nsp_types,
> +	},
> +	{
> +		.compatible = "brcm,spu2-crypto",
> +		.data = &spu2_types,
> +	},
> +	{
> +		.compatible = "brcm,spu2-v2-crypto",
> +		.data = &spu2_v2_types,
> +	},

These last two weren't in the binding document.

> +	{ /* sentinel */ }
> +};
> +
> +MODULE_DEVICE_TABLE(of, bcm_spu_dt_ids);
> +
> +static int spu_dt_read(struct platform_device *pdev)
> +{
> +	struct device *dev = &pdev->dev;
> +	struct spu_hw *spu = &iproc_priv.spu;
> +	struct device_node *dn = pdev->dev.of_node;
> +	struct resource *spu_ctrl_regs;
> +	const struct of_device_id *match;
> +	struct spu_type_subtype *matched_spu_type;
> +	void __iomem *spu_reg_vbase[MAX_SPUS];
> +	int i;
> +	int err;
> +
> +	if (!of_device_is_available(dn)) {
> +		dev_crit(dev, "SPU device not available");
> +		return -ENODEV;
> +	}

How can this happen?

> +	/* Count number of mailbox channels */
> +	spu->num_chan = of_count_phandle_with_args(dn, "mboxes", "#mbox-cells");
> +	dev_dbg(dev, "Device has %d SPU channels", spu->num_chan);
> +
> +	match = of_match_device(of_match_ptr(bcm_spu_dt_ids), dev);
> +	matched_spu_type = (struct spu_type_subtype *)match->data;

This cast usn't necessary.

> +	spu->spu_type = matched_spu_type->type;
> +	spu->spu_subtype = matched_spu_type->subtype;
> +
> +	/* Read registers and count number of SPUs */
> +	i = 0;
> +	while ((i < MAX_SPUS) && ((spu_ctrl_regs =
> +		platform_get_resource(pdev, IORESOURCE_MEM, i)) != NULL)) {
> +		dev_dbg(dev,
> +			"SPU %d control register region res.start = %#x, res.end = %#x",
> +			i,
> +			(unsigned int)spu_ctrl_regs->start,
> +			(unsigned int)spu_ctrl_regs->end);
> +
> +		spu_reg_vbase[i] = devm_ioremap_resource(dev, spu_ctrl_regs);
> +		if (IS_ERR(spu_reg_vbase[i])) {
> +			err = PTR_ERR(spu_reg_vbase[i]);
> +			dev_err(&pdev->dev, "Failed to map registers: %d\n",
> +				err);
> +			spu_reg_vbase[i] = NULL;
> +			return err;
> +		}
> +		i++;
> +	}

These *really* sound like independent devices. There are no shared
registers, and each has its own mbox.

Why do we group them like this?

Thanks,
Mark.

[PATCH 1/3] crypto: brcm: DT documentation for Broadcom SPU driver

[Rob (William) Rice]

Mark,

Thanks for your comments. Replies below.

Rob


On 12/6/2016 9:06 AM, Mark Rutland wrote:
> On Wed, Nov 30, 2016 at 03:07:31PM -0500, Rob Rice wrote:
>> Device tree documentation for Broadcom Secure Processing Unit
>> (SPU) crypto driver.
>>
>> Signed-off-by: Steve Lin <steven.lin1@broadcom.com>
>> Signed-off-by: Rob Rice <rob.rice@broadcom.com>
>> ---
>>   .../devicetree/bindings/crypto/brcm,spu-crypto.txt | 25 ++++++++++++++++++++++
>>   1 file changed, 25 insertions(+)
>>   create mode 100644 Documentation/devicetree/bindings/crypto/brcm,spu-crypto.txt
>>
>> diff --git a/Documentation/devicetree/bindings/crypto/brcm,spu-crypto.txt b/Documentation/devicetree/bindings/crypto/brcm,spu-crypto.txt
>> new file mode 100644
>> index 0000000..e5fe942
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/crypto/brcm,spu-crypto.txt
>> @@ -0,0 +1,25 @@
>> +The Broadcom Secure Processing Unit (SPU) driver supports symmetric
>> +cryptographic offload for Broadcom SoCs with SPU hardware. A SoC may have
>> +multiple SPU hardware blocks.
> Bindings shound describe *hardware*, not *drivers*. Please drop mention
> of the driver, and just decribe the hardware.
Makes sense. I'll change it.
>
>> +Required properties:
>> +- compatible : Should be "brcm,spum-crypto" for devices with SPU-M hardware
>> +  (e.g., Northstar2) or "brcm,spum-nsp-crypto" for the Northstar Plus variant
>> +  of the SPU-M hardware.
>> +
>> +- reg: Should contain SPU registers location and length.
>> +- mboxes: A list of mailbox channels to be used by the kernel driver. Mailbox
>> +channels correspond to DMA rings on the device.
>> +
>> +Example:
>> +	spu-crypto at 612d0000 {
>> +		compatible = "brcm,spum-crypto";
>> +		reg = <0 0x612d0000 0 0x900>,    /* SPU 0 control regs */
>> +			<0 0x612f0000 0 0x900>,  /* SPU 1 control regs */
>> +			<0 0x61310000 0 0x900>,  /* SPU 2 control regs */
>> +			<0 0x61330000 0 0x900>;  /* SPU 3 control regs */
> The above didn't mention there were several register sets, and the
> comment beside each makes them sound like they're separate SPU
> instances, so I don't think it makes sense to group them as one node.
>
> What's going on here?
That's right. For the SoC I used as the example, there are four SPU 
hardware blocks. The driver round robins crypto requests to the hardware 
blocks to handle requests in parallel and increase throughput. I want 
one instance of the SPU driver that registers algos once with the crypto 
API and manages all the mailbox channels. Maybe I can achieve that with 
separate device tree entries for each SPU block, I'm not sure. I'll look 
into it.
>
>> +		mboxes = <&pdc0 0>,
>> +			<&pdc1 0>,
>> +			<&pdc2 0>,
>> +			<&pdc3 0>;
> Does each mbox correspond to one of the SPUs above? Or is there a shared
> pool?
Yes, each of these mailbox channels corresponds to a different SPU. PDC 
is a DMA ring manager for DMAs to the SPUs.
>
> Thanks,
> Mark.

[PATCH 2/3] crypto: brcm: Add Broadcom SPU driver

[Rob (William) Rice]

Mark,

Thanks for the comments. Replies below.

Rob


On 12/6/2016 9:18 AM, Mark Rutland wrote:
> On Wed, Nov 30, 2016 at 03:07:32PM -0500, Rob Rice wrote:
>> +static const struct of_device_id bcm_spu_dt_ids[] = {
>> +	{
>> +		.compatible = "brcm,spum-crypto",
>> +		.data = &spum_ns2_types,
>> +	},
>> +	{
>> +		.compatible = "brcm,spum-nsp-crypto",
>> +		.data = &spum_nsp_types,
>> +	},
>> +	{
>> +		.compatible = "brcm,spu2-crypto",
>> +		.data = &spu2_types,
>> +	},
>> +	{
>> +		.compatible = "brcm,spu2-v2-crypto",
>> +		.data = &spu2_v2_types,
>> +	},
> These last two weren't in the binding document.
yes, I'll add them.
>
>> +	{ /* sentinel */ }
>> +};
>> +
>> +MODULE_DEVICE_TABLE(of, bcm_spu_dt_ids);
>> +
>> +static int spu_dt_read(struct platform_device *pdev)
>> +{
>> +	struct device *dev = &pdev->dev;
>> +	struct spu_hw *spu = &iproc_priv.spu;
>> +	struct device_node *dn = pdev->dev.of_node;
>> +	struct resource *spu_ctrl_regs;
>> +	const struct of_device_id *match;
>> +	struct spu_type_subtype *matched_spu_type;
>> +	void __iomem *spu_reg_vbase[MAX_SPUS];
>> +	int i;
>> +	int err;
>> +
>> +	if (!of_device_is_available(dn)) {
>> +		dev_crit(dev, "SPU device not available");
>> +		return -ENODEV;
>> +	}
> How can this happen?
You are correct. This is unnecessary. I will remove.
>
>> +	/* Count number of mailbox channels */
>> +	spu->num_chan = of_count_phandle_with_args(dn, "mboxes", "#mbox-cells");
>> +	dev_dbg(dev, "Device has %d SPU channels", spu->num_chan);
>> +
>> +	match = of_match_device(of_match_ptr(bcm_spu_dt_ids), dev);
>> +	matched_spu_type = (struct spu_type_subtype *)match->data;
> This cast usn't necessary.
Ok, will remove.
>
>> +	spu->spu_type = matched_spu_type->type;
>> +	spu->spu_subtype = matched_spu_type->subtype;
>> +
>> +	/* Read registers and count number of SPUs */
>> +	i = 0;
>> +	while ((i < MAX_SPUS) && ((spu_ctrl_regs =
>> +		platform_get_resource(pdev, IORESOURCE_MEM, i)) != NULL)) {
>> +		dev_dbg(dev,
>> +			"SPU %d control register region res.start = %#x, res.end = %#x",
>> +			i,
>> +			(unsigned int)spu_ctrl_regs->start,
>> +			(unsigned int)spu_ctrl_regs->end);
>> +
>> +		spu_reg_vbase[i] = devm_ioremap_resource(dev, spu_ctrl_regs);
>> +		if (IS_ERR(spu_reg_vbase[i])) {
>> +			err = PTR_ERR(spu_reg_vbase[i]);
>> +			dev_err(&pdev->dev, "Failed to map registers: %d\n",
>> +				err);
>> +			spu_reg_vbase[i] = NULL;
>> +			return err;
>> +		}
>> +		i++;
>> +	}
> These *really* sound like independent devices. There are no shared
> registers, and each has its own mbox.
>
> Why do we group them like this?
As I said in the previous email, I want one instance of the driver to 
register crypto algos once with the crypto API and to distribute crypto 
requests among all available SPU hw blocks.
>
> Thanks,
> Mark.

[PATCH 3/3] crypto: brcm: Add Broadcom SPU driver DT entry.

[kbuild test robot]

Hi Rob,

[auto build test ERROR on cryptodev/master]
[also build test ERROR on v4.9-rc8]
[cannot apply to next-20161209]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Rob-Rice/crypto-brcm-DT-documentation-for-Broadcom-SPU-driver/20161202-010038
base:   https://git.kernel.org/pub/scm/linux/kernel/git/herbert/cryptodev-2.6.git master
config: arm64-defconfig (attached as .config)
compiler: aarch64-linux-gnu-gcc (Debian 6.1.1-9) 6.1.1 20160705
reproduce:
        wget https://git.kernel.org/cgit/linux/kernel/git/wfg/lkp-tests.git/plain/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # save the attached .config to linux build tree
        make.cross ARCH=arm64 

All errors (new ones prefixed by >>):

>> ERROR: Input tree has errors, aborting (use -f to force output)

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation
-------------- next part --------------
A non-text attachment was scrubbed...
Name: .config.gz
Type: application/gzip
Size: 31957 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20161211/bead5da1/attachment-0001.gz>

[PATCH 3/3] crypto: brcm: Add Broadcom SPU driver DT entry.

[Rob (William) Rice]

I will rebase to Herbert's latest when I submit v3 to address Mark 
Rutland's DT comments (and any others that come in).

Rob


On 12/10/2016 7:14 PM, kbuild test robot wrote:
> Hi Rob,
>
> [auto build test ERROR on cryptodev/master]
> [also build test ERROR on v4.9-rc8]
> [cannot apply to next-20161209]
> [if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
>
> url:    https://github.com/0day-ci/linux/commits/Rob-Rice/crypto-brcm-DT-documentation-for-Broadcom-SPU-driver/20161202-010038
> base:   https://git.kernel.org/pub/scm/linux/kernel/git/herbert/cryptodev-2.6.git master
> config: arm64-defconfig (attached as .config)
> compiler: aarch64-linux-gnu-gcc (Debian 6.1.1-9) 6.1.1 20160705
> reproduce:
>          wget https://git.kernel.org/cgit/linux/kernel/git/wfg/lkp-tests.git/plain/sbin/make.cross -O ~/bin/make.cross
>          chmod +x ~/bin/make.cross
>          # save the attached .config to linux build tree
>          make.cross ARCH=arm64
>
> All errors (new ones prefixed by >>):
>
>>> ERROR: Input tree has errors, aborting (use -f to force output)
> ---
> 0-DAY kernel test infrastructure                Open Source Technology Center
> https://lists.01.org/pipermail/kbuild-all                   Intel Corporation