[PATCH mvebu + mvebu/dt64 4/4] arm64: dts: marvell: armada-37xx: move firmware node to generic dtsi file
From: Pali Rohár pali@kernel.org
Move the turris-mox-rwtm firmware node from Turris MOX' device tree into the generic armada-37xx.dtsi file.
The Turris MOX rWTM firmware can be used on any Armada 37xx device, giving them access to the rWTM hardware random number generator, which is otherwise unavailable.
This change allows Linux to load the turris-mox-rwtm.ko module on these boards.
Tested on ESPRESSObin v5 with both default Marvell WTMI firmware and CZ.NIC's firmware. With default WTMI firmware the turris-mox-rwtm fails to probe, while with CZ.NIC's firmware it registers the HW random number generator.
Signed-off-by: Pali Rohár pali@kernel.org Signed-off-by: Marek Behún kabel@kernel.org Cc: stable@vger.kernel.org # 5.4+: 46d2f6d0c99f ("arm64: dts: armada-3720-turris-mox: add firmware node") --- arch/arm64/boot/dts/marvell/armada-3720-turris-mox.dts | 8 -------- arch/arm64/boot/dts/marvell/armada-37xx.dtsi | 8 ++++++++ 2 files changed, 8 insertions(+), 8 deletions(-)
diff --git a/arch/arm64/boot/dts/marvell/armada-3720-turris-mox.dts b/arch/arm64/boot/dts/marvell/armada-3720-turris-mox.dts index d239ab70ed99..8447f303a294 100644 --- a/arch/arm64/boot/dts/marvell/armada-3720-turris-mox.dts +++ b/arch/arm64/boot/dts/marvell/armada-3720-turris-mox.dts @@ -107,14 +107,6 @@ sfp: sfp { /* enabled by U-Boot if SFP module is present */ status = "disabled"; }; - - firmware { - turris-mox-rwtm { - compatible = "cznic,turris-mox-rwtm"; - mboxes = <&rwtm 0>; - status = "okay"; - }; - }; };
&i2c0 { diff --git a/arch/arm64/boot/dts/marvell/armada-37xx.dtsi b/arch/arm64/boot/dts/marvell/armada-37xx.dtsi index 7a2df148c6a3..31126f1ffe5b 100644 --- a/arch/arm64/boot/dts/marvell/armada-37xx.dtsi +++ b/arch/arm64/boot/dts/marvell/armada-37xx.dtsi @@ -503,4 +503,12 @@ pcie_intc: interrupt-controller { }; }; }; + + firmware { + turris-mox-rwtm { + compatible = "cznic,turris-mox-rwtm"; + mboxes = <&rwtm 0>; + status = "okay"; + }; + }; };
Hello Marek,
From: Pali Rohár pali@kernel.org
Move the turris-mox-rwtm firmware node from Turris MOX' device tree into the generic armada-37xx.dtsi file.
I disagree with this patch. This firmware is specific to Turris MOX so it is not something that should be exposed to all the Armada 3700 based boards.
If you want you still can create an dtsi for this and include it when needed.
Gregory
The Turris MOX rWTM firmware can be used on any Armada 37xx device, giving them access to the rWTM hardware random number generator, which is otherwise unavailable.
This change allows Linux to load the turris-mox-rwtm.ko module on these boards.
Tested on ESPRESSObin v5 with both default Marvell WTMI firmware and CZ.NIC's firmware. With default WTMI firmware the turris-mox-rwtm fails to probe, while with CZ.NIC's firmware it registers the HW random number generator.
Signed-off-by: Pali Rohár pali@kernel.org Signed-off-by: Marek Behún kabel@kernel.org Cc: stable@vger.kernel.org # 5.4+: 46d2f6d0c99f ("arm64: dts: armada-3720-turris-mox: add firmware node")
arch/arm64/boot/dts/marvell/armada-3720-turris-mox.dts | 8 -------- arch/arm64/boot/dts/marvell/armada-37xx.dtsi | 8 ++++++++ 2 files changed, 8 insertions(+), 8 deletions(-)
diff --git a/arch/arm64/boot/dts/marvell/armada-3720-turris-mox.dts b/arch/arm64/boot/dts/marvell/armada-3720-turris-mox.dts index d239ab70ed99..8447f303a294 100644 --- a/arch/arm64/boot/dts/marvell/armada-3720-turris-mox.dts +++ b/arch/arm64/boot/dts/marvell/armada-3720-turris-mox.dts @@ -107,14 +107,6 @@ sfp: sfp { /* enabled by U-Boot if SFP module is present */ status = "disabled"; };
- firmware {
turris-mox-rwtm {compatible = "cznic,turris-mox-rwtm";mboxes = <&rwtm 0>;status = "okay";};- };
}; &i2c0 { diff --git a/arch/arm64/boot/dts/marvell/armada-37xx.dtsi b/arch/arm64/boot/dts/marvell/armada-37xx.dtsi index 7a2df148c6a3..31126f1ffe5b 100644 --- a/arch/arm64/boot/dts/marvell/armada-37xx.dtsi +++ b/arch/arm64/boot/dts/marvell/armada-37xx.dtsi @@ -503,4 +503,12 @@ pcie_intc: interrupt-controller { }; }; };
- firmware {
turris-mox-rwtm {compatible = "cznic,turris-mox-rwtm";mboxes = <&rwtm 0>;status = "okay";};- };
};
2.26.2
On Fri, 12 Mar 2021 09:58:34 +0100 Gregory CLEMENT gregory.clement@bootlin.com wrote:
Hello Marek,
From: Pali Rohár pali@kernel.org
Move the turris-mox-rwtm firmware node from Turris MOX' device tree into the generic armada-37xx.dtsi file.
I disagree with this patch. This firmware is specific to Turris MOX so it is not something that should be exposed to all the Armada 3700 based boards.
If you want you still can create an dtsi for this and include it when needed.
Gregory
Gregory, we are planning to send pull-request for TF-A documentation, adding information that people can compile the firmware with CZ.NIC's firmware.
Since this firmware exposes HW random number generator, it is possible that people will start using it for espressobin.
In that case this won't be specific for Turris MOX anymore.
Maybe we should add another compatible to the turris-mox-rwtm driver?
Marek
On Fri, Mar 12, 2021 at 10:10:27AM +0100, Marek Behún wrote:
On Fri, 12 Mar 2021 09:58:34 +0100 Gregory CLEMENT gregory.clement@bootlin.com wrote:
Hello Marek,
From: Pali Rohár pali@kernel.org
Move the turris-mox-rwtm firmware node from Turris MOX' device tree into the generic armada-37xx.dtsi file.
I disagree with this patch. This firmware is specific to Turris MOX so it is not something that should be exposed to all the Armada 3700 based boards.
If you want you still can create an dtsi for this and include it when needed.
Gregory
Gregory, we are planning to send pull-request for TF-A documentation, adding information that people can compile the firmware with CZ.NIC's firmware.
Since this firmware exposes HW random number generator, it is possible that people will start using it for espressobin.
In that case this won't be specific for Turris MOX anymore.
Part of the problem is that it looks specific to the Turris MOX.
But please help me understand the big picture first. How is the firmware distributed? Is the binary part of linux-firmware? How does it get loaded? Does the firmware contain anything which is specific to the Turris MOX? Could the hardware number generator part be split out into a more generic sounding name blob?
Andrew
On Fri, 12 Mar 2021 15:48:14 +0100 Andrew Lunn andrew@lunn.ch wrote:
On Fri, Mar 12, 2021 at 10:10:27AM +0100, Marek Behún wrote:
On Fri, 12 Mar 2021 09:58:34 +0100 Gregory CLEMENT gregory.clement@bootlin.com wrote:
Hello Marek,
From: Pali Rohár pali@kernel.org
Move the turris-mox-rwtm firmware node from Turris MOX' device tree into the generic armada-37xx.dtsi file.
I disagree with this patch. This firmware is specific to Turris MOX so it is not something that should be exposed to all the Armada 3700 based boards.
If you want you still can create an dtsi for this and include it when needed.
Gregory
Gregory, we are planning to send pull-request for TF-A documentation, adding information that people can compile the firmware with CZ.NIC's firmware.
Since this firmware exposes HW random number generator, it is possible that people will start using it for espressobin.
In that case this won't be specific for Turris MOX anymore.
Part of the problem is that it looks specific to the Turris MOX.
But please help me understand the big picture first. How is the firmware distributed? Is the binary part of linux-firmware? How does it get loaded? Does the firmware contain anything which is specific to the Turris MOX? Could the hardware number generator part be split out into a more generic sounding name blob?
Andrew
Hello Andrew,
The WTMI firmware is loaded before kernel. This firmware is loaded by BootROM, and it is this firmware that does DDR training before loading TF-A + U-Boot.
For example for ESPRESSObin you have several repositories you need to create a final flash-image.bin containing this WTMI firmware + TF-A + U-Boot. These repositories are: trusted-firmware-a (contains documentation how to build all this) A3700-utils-marvell u-boot mv-ddr-marvell From these sources you are able to create a final flash-image.bin that you can flash onto the SPI-NOR (or eMMC or other devices which A3720 can boot from).
The A3700-utils-marvell repository contains the code of the WTMI firmware.
On Turris MOX this is a little bit different, because - we have implemented the WTMI firmware differently (more mailbox commands, HW crypto, ...) - it supports retrieving MOX board information (MAC addresses, serial number) stored in eFuses (this information is stored in a specific way that in only true for MOX) - the firmware binary must be signed by our private key in order to boot on MOX.
This is because the secure firmware has access to an ECDSA engine with a private key storage in eFuses (each MOX has its own private key generated and stored into the eFuses when manufactured) In order to disallow hackers to somehow extract the private key, the firmware must be signed so that they cannot load arbitrary firmware into the secure processor.
BUT - since this firmware is able to provide HWRNG, we wanted to make it available for other Armada 3720 boards - we updated the code so that users can build it for non-MOX devices - it does not have to be signed for other devices - it does not contain MOX specific stuff for non-MOX devices
So currently when users build the flash-image.bin binary containing WTMI firmware, they are using code from A3700-utils-marvell. This code is split into 2 parts: - sys_init - does HW and DDR initialization and execution of an "app" - efuse - default "app" which is loaded by sys_init The way it is written is that user can select a different "app" when building, and we have updated Turris MOX firmware code to be loadable as this "app" for sys_init. (And we have renamed it from "Turris MOX secure firmware" to "CZ.NIC's Armada 3720 secure firmware").
Could the hardware number generator part be split out into a more generic sounding name blob?
It basically is. As I have written above, when users build the flash-image.bin with CZ.NIC's firmware, the prompt does not say anything about Turris MOX. Instead it says something like CZ.NIC's Armada 3720 Secure Firmware version build date Running on ESPRESSObin and currently provides only the random number generator command.
So theoretically the turris-mox-rwtm driver can be renamed into something else and we can add a different compatible in order not to sound so turris-mox specific.
Marek
So theoretically the turris-mox-rwtm driver can be renamed into something else and we can add a different compatible in order not to sound so turris-mox specific.
That would be a good idea. And if possible, try to push the hardware random number code upstream in the firmware repos, so everybody gets it by default, not just those using your build. Who is responsible for upstream? Marvell?
Andrew
On Friday 12 March 2021 16:53:32 Andrew Lunn wrote:
So theoretically the turris-mox-rwtm driver can be renamed into something else and we can add a different compatible in order not to sound so turris-mox specific.
That would be a good idea. And if possible, try to push the hardware random number code upstream in the firmware repos, so everybody gets it by default, not just those using your build. Who is responsible for upstream? Marvell?
Hello Andrew! The issue is that upstream Marvell repository contains only 'fuse.bin' application which is suitable only for fuse programming. I think it is not correct if this Marvell fuse application start providing other functionality not relevant to fuse programming. And Marvell does not provide any other application (publicly). So it would be needed to send it as another application, not part of 'fuse.bin'. And then it complicates build system and compile options, which is already too complicated (you need to set tons of TF-A options and prepare two sets of cross compile toolchains).
But because application / firmware for MOX / Armada 3720 is actively developed on different place, I do not think that it make sense to send every change to two different locations (and wait for Marvell until review every change and include it into their repository). Such thing just increase maintenance cost at both sides.
For me it looks like a better solution to provide 'wtmi_app.bin' application with HW number generator from separate repository, where it is currently developed and where it is available for a longer time.
We are planning to send documentation update to Trusted-Firmware project to specify how to build Armada 3720 firmware image with our application. So people who are building Armada 3720 firmware would be able to switch from Marvell's 'fuse.bin' application to our 'wtmi_app.bin'.
On Fri, 12 Mar 2021 17:18:57 +0100 Pali Rohár pali@kernel.org wrote:
On Friday 12 March 2021 16:53:32 Andrew Lunn wrote:
So theoretically the turris-mox-rwtm driver can be renamed into something else and we can add a different compatible in order not to sound so turris-mox specific.
That would be a good idea. And if possible, try to push the hardware random number code upstream in the firmware repos, so everybody gets it by default, not just those using your build. Who is responsible for upstream? Marvell?
Hello Andrew! The issue is that upstream Marvell repository contains only 'fuse.bin' application which is suitable only for fuse programming. I think it is not correct if this Marvell fuse application start providing other functionality not relevant to fuse programming.
Why not? We can rename it to fuse+hwrng and implement hwrng there. Maybe Konstantin will agree with such a change :)
And Marvell does not provide any other application (publicly). So it would be needed to send it as another application, not part of 'fuse.bin'. And then it complicates build system and compile options, which is already too complicated (you need to set tons of TF-A options and prepare two sets of cross compile toolchains).
But because application / firmware for MOX / Armada 3720 is actively developed on different place, I do not think that it make sense to send every change to two different locations (and wait for Marvell until review every change and include it into their repository). Such thing just increase maintenance cost at both sides.
This is a little bit better argument than the previous one. But I think Andrew may be right in that for end-users it just complicates things if they have more options. Better to give them one option.
For me it looks like a better solution to provide 'wtmi_app.bin' application with HW number generator from separate repository, where it is currently developed and where it is available for a longer time.
We are planning to send documentation update to Trusted-Firmware project to specify how to build Armada 3720 firmware image with our application. So people who are building Armada 3720 firmware would be able to switch from Marvell's 'fuse.bin' application to our 'wtmi_app.bin'.
On Friday 12 March 2021 16:53:32 Andrew Lunn wrote:
So theoretically the turris-mox-rwtm driver can be renamed into something else and we can add a different compatible in order not to sound so turris-mox specific.
That would be a good idea. And if possible, try to push the hardware random number code upstream in the firmware repos, so everybody gets it by default, not just those using your build. Who is responsible for upstream? Marvell?
Andrew
Hello Andrew!
I do not think that renaming driver is the best option. For future it would complicate backporting patches to stable kernel and also it would make usage of 'gitk' harder as this tool cannot automatically track file renames.
I saw that kernel drivers lot of times got support for a new HW but driver name was not changed (maybe for backward compatibility, maybe to simplify things).
Andrew, now when you got a full picture of this problem, would you be willing to accept these patches? Or do you require some changes?
On Mon, Mar 15, 2021 at 11:14:54AM +0100, Pali Rohár wrote:
On Friday 12 March 2021 16:53:32 Andrew Lunn wrote:
So theoretically the turris-mox-rwtm driver can be renamed into something else and we can add a different compatible in order not to sound so turris-mox specific.
That would be a good idea. And if possible, try to push the hardware random number code upstream in the firmware repos, so everybody gets it by default, not just those using your build. Who is responsible for upstream? Marvell?
AndrewHello Andrew!
I do not think that renaming driver is the best option. For future it would complicate backporting patches to stable kernel and also it would make usage of 'gitk' harder as this tool cannot automatically track file renames.
Hi Pali
I'm not suggesting renaming the .c file.
What would be good is to add additional compatible strings. Add a more generic compatible. What goes into the .dtsi should use the generic name. Also, the node names should also be generic, since the node name is probably not used anywhere, just the compatible string. Keep the current compatible in the driver, for backwards compatibility with older DT blobs.
Andrew
On Monday 15 March 2021 13:08:16 Andrew Lunn wrote:
On Mon, Mar 15, 2021 at 11:14:54AM +0100, Pali Rohár wrote:
On Friday 12 March 2021 16:53:32 Andrew Lunn wrote:
So theoretically the turris-mox-rwtm driver can be renamed into something else and we can add a different compatible in order not to sound so turris-mox specific.
That would be a good idea. And if possible, try to push the hardware random number code upstream in the firmware repos, so everybody gets it by default, not just those using your build. Who is responsible for upstream? Marvell?
AndrewHello Andrew!
I do not think that renaming driver is the best option. For future it would complicate backporting patches to stable kernel and also it would make usage of 'gitk' harder as this tool cannot automatically track file renames.
Hi Pali
I'm not suggesting renaming the .c file.
What would be good is to add additional compatible strings. Add a more generic compatible. What goes into the .dtsi should use the generic name. Also, the node names should also be generic, since the node name is probably not used anywhere, just the compatible string. Keep the current compatible in the driver, for backwards compatibility with older DT blobs.
Ok! What about compatible string "marvell,armada-3700-rwtm-firmware"?
Mailbox layer which is used by this driver has compatible string "marvell,armada-3700-rwtm-mailbox", so name is similar.
linux-stable-mirror@lists.linaro.org
-
Andrew Lunn -
Gregory CLEMENT -
Marek Behún -
Pali Rohár