[PATCH v3 0/4] Add Amlogic secure monitor driver

[PATCH v3 0/4] Add Amlogic secure monitor driver

[Carlo Caione]

From: Carlo Caione <carlo-6IF/jdPJHihWk0Htik3J/w@public.gmane.org>

In the Amlogic SoCs the secure monitor calls are used for a lot of reasons:
interact with the NVMEM (efuses), check the reboot reason, set USB boot, ...

This driver defines a generic interface towards the secure monitor that can be
used by more specialized drivers to interact with the secure monitor itself
without worrying about bounce buffers managing.

Changelog:

v3:
  * Moved driver to drivers/firmware
  * Added EXPORT_SYMBOL macros
  * Changed '_' in '-'
  * Introduced 'struct meson_sm_firmware' and 'meson_sm_get_fw()' to be used by
    drivers using the secure-monitor to enforce probe ordering

v2:
  * All the SMC function identifiers are now in a SoC-specific header file to
    be included by the DTS files so the SMC commands are now defined in the DT
    instead to be hardcoded into the driver
  * Patchset is no longer an RFC
  * Better error management and boundary checking
  * s/unsigned int/u32/
  * SMC call not only on CPU 0
  * Fix memory leaking
  * s/amlogic/meson/ in the directory names


Carlo Caione (4):
  firmware: Amlogic: Add secure monitor driver
  firmware: dt-bindings: Add secure monitor header file for GXBB
  ARM64: dts: amlogic: gxbb: Enable secure monitor
  documentation: Add secure monitor binding documentation

 .../bindings/firmware/meson/meson_sm.txt           |  51 ++++++
 arch/arm64/boot/dts/amlogic/meson-gxbb.dtsi        |  11 ++
 drivers/firmware/Kconfig                           |   1 +
 drivers/firmware/Makefile                          |   1 +
 drivers/firmware/meson/Kconfig                     |   8 +
 drivers/firmware/meson/Makefile                    |   1 +
 drivers/firmware/meson/meson_sm.c                  | 179 +++++++++++++++++++++
 include/dt-bindings/firmware/meson-gxbb-sm.h       |  44 +++++
 include/linux/firmware/meson/meson_sm.h            |  38 +++++
 9 files changed, 334 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/firmware/meson/meson_sm.txt
 create mode 100644 drivers/firmware/meson/Kconfig
 create mode 100644 drivers/firmware/meson/Makefile
 create mode 100644 drivers/firmware/meson/meson_sm.c
 create mode 100644 include/dt-bindings/firmware/meson-gxbb-sm.h
 create mode 100644 include/linux/firmware/meson/meson_sm.h

-- 
2.7.4

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v3 1/4] firmware: Amlogic: Add secure monitor driver

[Carlo Caione]

From: Carlo Caione <carlo-6IF/jdPJHihWk0Htik3J/w@public.gmane.org>

Introduce a driver to provide calls into secure monitor mode.

In the Amlogic SoCs these calls are used for multiple reasons: access to
NVMEM, set USB boot, enable JTAG, etc...

Signed-off-by: Carlo Caione <carlo-6IF/jdPJHihWk0Htik3J/w@public.gmane.org>
---
 drivers/firmware/Kconfig                |   1 +
 drivers/firmware/Makefile               |   1 +
 drivers/firmware/meson/Kconfig          |   8 ++
 drivers/firmware/meson/Makefile         |   1 +
 drivers/firmware/meson/meson_sm.c       | 179 ++++++++++++++++++++++++++++++++
 include/linux/firmware/meson/meson_sm.h |  38 +++++++
 6 files changed, 228 insertions(+)
 create mode 100644 drivers/firmware/meson/Kconfig
 create mode 100644 drivers/firmware/meson/Makefile
 create mode 100644 drivers/firmware/meson/meson_sm.c
 create mode 100644 include/linux/firmware/meson/meson_sm.h

diff --git a/drivers/firmware/Kconfig b/drivers/firmware/Kconfig
index 6664f11..686e395 100644
--- a/drivers/firmware/Kconfig
+++ b/drivers/firmware/Kconfig
@@ -199,5 +199,6 @@ config HAVE_ARM_SMCCC
 source "drivers/firmware/broadcom/Kconfig"
 source "drivers/firmware/google/Kconfig"
 source "drivers/firmware/efi/Kconfig"
+source "drivers/firmware/meson/Kconfig"
 
 endmenu
diff --git a/drivers/firmware/Makefile b/drivers/firmware/Makefile
index 474bada..fc4bb09 100644
--- a/drivers/firmware/Makefile
+++ b/drivers/firmware/Makefile
@@ -21,6 +21,7 @@ obj-$(CONFIG_QCOM_SCM_32)	+= qcom_scm-32.o
 CFLAGS_qcom_scm-32.o :=$(call as-instr,.arch armv7-a\n.arch_extension sec,-DREQUIRES_SEC=1) -march=armv7-a
 
 obj-y				+= broadcom/
+obj-y				+= meson/
 obj-$(CONFIG_GOOGLE_FIRMWARE)	+= google/
 obj-$(CONFIG_EFI)		+= efi/
 obj-$(CONFIG_UEFI_CPER)		+= efi/
diff --git a/drivers/firmware/meson/Kconfig b/drivers/firmware/meson/Kconfig
new file mode 100644
index 0000000..fff11a3
--- /dev/null
+++ b/drivers/firmware/meson/Kconfig
@@ -0,0 +1,8 @@
+#
+# Amlogic Secure Monitor driver
+#
+config MESON_SM
+	bool
+	default ARCH_MESON
+	help
+	  Say y here to enable the Amlogic secure monitor driver
diff --git a/drivers/firmware/meson/Makefile b/drivers/firmware/meson/Makefile
new file mode 100644
index 0000000..9ab3884
--- /dev/null
+++ b/drivers/firmware/meson/Makefile
@@ -0,0 +1 @@
+obj-$(CONFIG_MESON_SM) +=	meson_sm.o
diff --git a/drivers/firmware/meson/meson_sm.c b/drivers/firmware/meson/meson_sm.c
new file mode 100644
index 0000000..445bc7f
--- /dev/null
+++ b/drivers/firmware/meson/meson_sm.c
@@ -0,0 +1,179 @@
+/*
+ * Amlogic Secure Monitor driver
+ *
+ * Copyright (C) 2016 Endless Mobile, Inc.
+ * Author: Carlo Caione <carlo-6IF/jdPJHihWk0Htik3J/w@public.gmane.org>
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * version 2 as published by the Free Software Foundation.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program. If not, see <http://www.gnu.org/licenses/>.
+ */
+
+#include <stdarg.h>
+#include <asm/cacheflush.h>
+#include <asm/compiler.h>
+#include <linux/arm-smccc.h>
+#include <linux/io.h>
+#include <linux/ioport.h>
+#include <linux/module.h>
+#include <linux/platform_device.h>
+#include <linux/of.h>
+#include <linux/smp.h>
+
+#include <linux/firmware/meson/meson_sm.h>
+
+#define SM_MEM_SIZE	0x1000
+
+struct meson_sm_data {
+	u32 cmd;
+	u32 arg0;
+	u32 arg1;
+	u32 arg2;
+	u32 arg3;
+	u32 arg4;
+	u32 ret;
+};
+
+static void __meson_sm_call(void *info)
+{
+	struct meson_sm_data *data = info;
+	struct arm_smccc_res res;
+
+	arm_smccc_smc(data->cmd,
+		      data->arg0, data->arg1, data->arg2,
+		      data->arg3, data->arg4, 0, 0, &res);
+	data->ret = res.a0;
+}
+
+u32 meson_sm_call(u32 cmd, u32 arg0, u32 arg1, u32 arg2,
+		  u32 arg3, u32 arg4)
+{
+	struct meson_sm_data data;
+
+	data.cmd = cmd;
+	data.arg0 = arg0;
+	data.arg1 = arg1;
+	data.arg2 = arg2;
+	data.arg3 = arg3;
+	data.arg4 = arg4;
+	data.ret = 0;
+
+	__meson_sm_call(&data);
+
+	return data.ret;
+}
+EXPORT_SYMBOL(meson_sm_call);
+
+u32 meson_sm_call_read(struct meson_sm_firmware *fw, void *buffer,
+		       u32 cmd, u32 arg0, u32 arg1, u32 arg2,
+		       u32 arg3, u32 arg4)
+{
+	u32 size;
+
+	if (!fw)
+		return -EINVAL;
+
+	size = meson_sm_call(cmd, arg0, arg1, arg2, arg3, arg4);
+
+	if (!size || size > SM_MEM_SIZE)
+		return -EINVAL;
+
+	memcpy(buffer, fw->sm_sharemem_out_base, size);
+	return size;
+}
+EXPORT_SYMBOL(meson_sm_call_read);
+
+u32 meson_sm_call_write(struct meson_sm_firmware *fw, void *buffer,
+			unsigned int b_size, u32 cmd, u32 arg0,
+			u32 arg1, u32 arg2, u32 arg3, u32 arg4)
+{
+	u32 size;
+
+	if (!fw)
+		return -EINVAL;
+
+	if (b_size > SM_MEM_SIZE)
+		return -EINVAL;
+
+	memcpy(fw->sm_sharemem_in_base, buffer, b_size);
+
+	size = meson_sm_call(cmd, arg0, arg1, arg2, arg3, arg4);
+
+	if (!size)
+		return -EINVAL;
+
+	return size;
+}
+EXPORT_SYMBOL(meson_sm_call_write);
+
+static int meson_sm_probe(struct platform_device *pdev)
+{
+	struct device_node *np = pdev->dev.of_node;
+	struct meson_sm_firmware *fw;
+	u32 sm_phy_in_base, sm_phy_out_base;
+	int cmd_in, cmd_out;
+
+	fw = devm_kzalloc(&pdev->dev, sizeof(*fw), GFP_KERNEL);
+	if (!fw)
+		return -ENOMEM;
+
+	fw->dev = &pdev->dev;
+
+	if (of_property_read_u32(np, "amlogic,sm-cmd-input-base", &cmd_in))
+		return -EINVAL;
+
+	if (of_property_read_u32(np, "amlogic,sm-cmd-output-base", &cmd_out))
+		return -EINVAL;
+
+	sm_phy_in_base = meson_sm_call(cmd_in, 0, 0, 0, 0, 0);
+	fw->sm_sharemem_in_base = ioremap_cache(sm_phy_in_base, SM_MEM_SIZE);
+	if (!fw->sm_sharemem_in_base)
+		return -EINVAL;
+
+	sm_phy_out_base = meson_sm_call(cmd_out, 0, 0, 0, 0, 0);
+	fw->sm_sharemem_out_base = ioremap_cache(sm_phy_out_base, SM_MEM_SIZE);
+	if (!fw->sm_sharemem_out_base) {
+		iounmap(fw->sm_sharemem_in_base);
+		return -EINVAL;
+	}
+
+	platform_set_drvdata(pdev, fw);
+
+	return 0;
+}
+
+struct meson_sm_firmware *meson_sm_get_fw(struct device_node *firmware_node)
+{
+	struct platform_device *pdev = of_find_device_by_node(firmware_node);
+
+	/*
+	 * Returns NULL is the firmware device is not ready.
+	 */
+	if (!pdev)
+		return NULL;
+
+	return platform_get_drvdata(pdev);
+}
+EXPORT_SYMBOL(meson_sm_get_fw);
+
+static const struct of_device_id meson_sm_ids[] = {
+	{ .compatible = "amlogic,meson-sm" },
+	{ /* sentinel */},
+};
+MODULE_DEVICE_TABLE(of, meson_sm_ids);
+
+static struct platform_driver meson_sm_platform_driver = {
+	.probe	= meson_sm_probe,
+	.driver	= {
+		.name		= "secmon",
+		.of_match_table	= meson_sm_ids,
+	},
+};
+module_platform_driver(meson_sm_platform_driver);
+
+MODULE_AUTHOR("Carlo Caione <carlo-6IF/jdPJHihWk0Htik3J/w@public.gmane.org>");
+MODULE_DESCRIPTION("Amlogic secure monitor driver");
+MODULE_LICENSE("GPL");
diff --git a/include/linux/firmware/meson/meson_sm.h b/include/linux/firmware/meson/meson_sm.h
new file mode 100644
index 0000000..7b6b524
--- /dev/null
+++ b/include/linux/firmware/meson/meson_sm.h
@@ -0,0 +1,38 @@
+/*
+ * Copyright (C) 2016 Endless Mobile, Inc.
+ * Author: Carlo Caione <carlo-6IF/jdPJHihWk0Htik3J/w@public.gmane.org>
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * version 2 as published by the Free Software Foundation.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program. If not, see <http://www.gnu.org/licenses/>.
+ */
+
+#ifndef _MESON_SM_H_
+#define _MESON_SM_H_
+
+#include <linux/types.h>
+#include <linux/of_device.h>
+
+struct meson_sm_firmware {
+	struct device *dev;
+	void __iomem *sm_sharemem_in_base;
+	void __iomem *sm_sharemem_out_base;
+};
+
+u32 meson_sm_call(u32 cmd, u32 arg0, u32 arg1, u32 arg2,
+		  u32 arg3, u32 arg4);
+
+u32 meson_sm_call_write(struct meson_sm_firmware *fw, void *buffer,
+			unsigned int b_size, u32 cmd, u32 arg0,
+			u32 arg1, u32 arg2, u32 arg3, u32 arg4);
+
+u32 meson_sm_call_read(struct meson_sm_firmware *fw, void *buffer,
+		       u32 cmd, u32 arg0, u32 arg1, u32 arg2,
+		       u32 arg3, u32 arg4);
+
+struct meson_sm_firmware *meson_sm_get_fw(struct device_node *firmware_node);
+
+#endif /* _MESON_SM_H_ */
-- 
2.7.4

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v3 2/4] firmware: dt-bindings: Add secure monitor header file for GXBB

[Carlo Caione]

From: Carlo Caione <carlo-6IF/jdPJHihWk0Htik3J/w@public.gmane.org>

This patch adds the header file describing all the SMC function
identifiers for the Amlogic Meson GXBB SoC.

Signed-off-by: Carlo Caione <carlo-6IF/jdPJHihWk0Htik3J/w@public.gmane.org>
---
 include/dt-bindings/firmware/meson-gxbb-sm.h | 44 ++++++++++++++++++++++++++++
 1 file changed, 44 insertions(+)
 create mode 100644 include/dt-bindings/firmware/meson-gxbb-sm.h

diff --git a/include/dt-bindings/firmware/meson-gxbb-sm.h b/include/dt-bindings/firmware/meson-gxbb-sm.h
new file mode 100644
index 0000000..b7d02df
--- /dev/null
+++ b/include/dt-bindings/firmware/meson-gxbb-sm.h
@@ -0,0 +1,44 @@
+/*
+ * Copyright (C) 2016 Endless Mobile, Inc.
+ * Author: Carlo Caione <carlo-6IF/jdPJHihWk0Htik3J/w@public.gmane.org>
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * version 2 as published by the Free Software Foundation.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program. If not, see <http://www.gnu.org/licenses/>.
+ */
+
+#ifndef _MESON_SM_GXBB_H_
+#define _MESON_SM_GXBB_H_
+
+#define SM_GET_SHARE_MEM_INPUT_BASE		0x82000020
+#define SM_GET_SHARE_MEM_OUTPUT_BASE		0x82000021
+#define SM_GET_REBOOT_REASON			0x82000022
+#define SM_GET_SHARE_STORAGE_IN_BASE		0x82000023
+#define SM_GET_SHARE_STORAGE_OUT_BASE		0x82000024
+#define SM_GET_SHARE_STORAGE_BLOCK_BASE		0x82000025
+#define SM_GET_SHARE_STORAGE_MESSAGE_BASE	0x82000026
+#define SM_GET_SHARE_STORAGE_BLOCK_SIZE		0x82000027
+#define SM_EFUSE_READ				0x82000030
+#define SM_EFUSE_WRITE				0x82000031
+#define SM_EFUSE_WRITE_PATTERN			0x82000032
+#define SM_EFUSE_USER_MAX			0x82000033
+#define SM_JTAG_ON				0x82000040
+#define SM_JTAG_OFF				0x82000041
+#define SM_SET_USB_BOOT_FUNC			0x82000043
+#define SM_SECURITY_KEY_QUERY			0x82000060
+#define SM_SECURITY_KEY_READ			0x82000061
+#define SM_SECURITY_KEY_WRITE			0x82000062
+#define SM_SECURITY_KEY_TELL			0x82000063
+#define SM_SECURITY_KEY_VERIFY			0x82000064
+#define SM_SECURITY_KEY_STATUS			0x82000065
+#define SM_SECURITY_KEY_NOTIFY			0x82000066
+#define SM_SECURITY_KEY_LIST			0x82000067
+#define SM_SECURITY_KEY_REMOVE			0x82000068
+#define SM_DEBUG_EFUSE_WRITE_PATTERN		0x820000F0
+#define SM_DEBUG_EFUSE_READ_PATTERN		0x820000F1
+#define SM_AML_DATA_PROCESS			0x820000FF
+
+#endif /* _MESON_SM_GXBB_H_ */
-- 
2.7.4

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v3 3/4] ARM64: dts: amlogic: gxbb: Enable secure monitor

[Carlo Caione]

From: Carlo Caione <carlo-6IF/jdPJHihWk0Htik3J/w@public.gmane.org>

Add the secure monitor node in the Amlogic Meson GXBB DTSI file to
enable it.

Signed-off-by: Carlo Caione <carlo-6IF/jdPJHihWk0Htik3J/w@public.gmane.org>
---
 arch/arm64/boot/dts/amlogic/meson-gxbb.dtsi | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/arch/arm64/boot/dts/amlogic/meson-gxbb.dtsi b/arch/arm64/boot/dts/amlogic/meson-gxbb.dtsi
index 76b3b6d..aeff919 100644
--- a/arch/arm64/boot/dts/amlogic/meson-gxbb.dtsi
+++ b/arch/arm64/boot/dts/amlogic/meson-gxbb.dtsi
@@ -44,6 +44,7 @@
 #include <dt-bindings/interrupt-controller/irq.h>
 #include <dt-bindings/interrupt-controller/arm-gic.h>
 #include <dt-bindings/gpio/meson-gxbb-gpio.h>
+#include <dt-bindings/firmware/meson-gxbb-sm.h>
 
 / {
 	compatible = "amlogic,meson-gxbb";
@@ -98,6 +99,16 @@
 		method = "smc";
 	};
 
+	firmware {
+		compatible = "simple-bus";
+
+		sm: secure-monitor {
+			compatible = "amlogic,meson-sm";
+			amlogic,sm-cmd-input-base = <SM_GET_SHARE_MEM_INPUT_BASE>;
+			amlogic,sm-cmd-output-base = <SM_GET_SHARE_MEM_OUTPUT_BASE>;
+		};
+	};
+
 	timer {
 		compatible = "arm,armv8-timer";
 		interrupts = <GIC_PPI 13
-- 
2.7.4

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v3 4/4] documentation: Add secure monitor binding documentation

[Carlo Caione]

From: Carlo Caione <carlo-6IF/jdPJHihWk0Htik3J/w@public.gmane.org>

Add the binding documentation for the Amlogic secure monitor driver.

Signed-off-by: Carlo Caione <carlo-6IF/jdPJHihWk0Htik3J/w@public.gmane.org>
---
 .../bindings/firmware/meson/meson_sm.txt           | 51 ++++++++++++++++++++++
 1 file changed, 51 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/firmware/meson/meson_sm.txt

diff --git a/Documentation/devicetree/bindings/firmware/meson/meson_sm.txt b/Documentation/devicetree/bindings/firmware/meson/meson_sm.txt
new file mode 100644
index 0000000..2b71716
--- /dev/null
+++ b/Documentation/devicetree/bindings/firmware/meson/meson_sm.txt
@@ -0,0 +1,51 @@
+* Amlogic Secure Monitor
+
+In the Amlogic SoCs the Secure Monitor code is used to provide access to the
+NVMEM, enable JTAG, set USB boot, etc...
+
+Required properties for the secure monitor node:
+- compatible: Should be "amlogic,meson-sm"
+- amlogic,sm-cmd-input-base: SMC32 function identifier to read the physical
+                             address of the input buffer
+- amlogic,sm-cmd-output-base: SMC32 function identifier to read the physical
+                              address of the output buffer
+
+Example:
+
+	#include <dt-bindings/firmware/meson-gxbb-sm.h>
+
+	...
+
+	firmware {
+		compatible = "simple-bus";
+
+		sm: secure-monitor {
+			compatible = "amlogic,meson-sm";
+			amlogic,sm-cmd-input-base = <SM_GET_SHARE_MEM_INPUT_BASE>;
+			amlogic,sm-cmd-output-base = <SM_GET_SHARE_MEM_OUTPUT_BASE>;
+		};
+	};
+
+Node of a device using the secure monitor must have a custom property to
+specify the SMC32 function the driver is going to use. The values of all the
+SMC function identifiers are listed in a SoC-specific header file:
+
+"include/dt-bindings/firmware/meson-gxbb-sm.h" - for the Meson GXBB secure monitor.
+
+Example of the node using the secure monitor:
+
+	#include <dt-bindings/firmware/meson-gxbb-sm.h>
+
+	...
+
+	efuse: efuse {
+		compatible = "amlogic,meson-gxbb-efuse";
+		secure-monitor = <&sm>;
+		amlogic,sm-cmd-read-efuse = <SM_EFUSE_READ>;
+		amlogic,sm-cmd-max-efuse = <SM_EFUSE_USER_MAX>;
+		#address-cells = <1>;
+		#size-cells = <1>;
+
+		...
+	};
+
-- 
2.7.4

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v3 4/4] documentation: Add secure monitor binding documentation

[Mark Rutland]

Hi,

Minor nit, but please put the binding earlier in the series than the
code implementing it, as per
Documentation/devicetree/bindings/submitting-patches.txt

This makes it possible to review a series in a linear fashion.

On Mon, May 23, 2016 at 06:30:24PM +0200, Carlo Caione wrote:
> From: Carlo Caione <carlo-6IF/jdPJHihWk0Htik3J/w@public.gmane.org>
> 
> Add the binding documentation for the Amlogic secure monitor driver.
> 
> Signed-off-by: Carlo Caione <carlo-6IF/jdPJHihWk0Htik3J/w@public.gmane.org>
> ---
>  .../bindings/firmware/meson/meson_sm.txt           | 51 ++++++++++++++++++++++
>  1 file changed, 51 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/firmware/meson/meson_sm.txt
> 
> diff --git a/Documentation/devicetree/bindings/firmware/meson/meson_sm.txt b/Documentation/devicetree/bindings/firmware/meson/meson_sm.txt
> new file mode 100644
> index 0000000..2b71716
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/firmware/meson/meson_sm.txt
> @@ -0,0 +1,51 @@
> +* Amlogic Secure Monitor
> +
> +In the Amlogic SoCs the Secure Monitor code is used to provide access to the
> +NVMEM, enable JTAG, set USB boot, etc...
> +
> +Required properties for the secure monitor node:
> +- compatible: Should be "amlogic,meson-sm"
> +- amlogic,sm-cmd-input-base: SMC32 function identifier to read the physical
> +                             address of the input buffer
> +- amlogic,sm-cmd-output-base: SMC32 function identifier to read the physical
> +                              address of the output buffer

Do the IDs for these actually differ per board?

Are some functions simply not implemented on some boards?

> +
> +Example:
> +
> +	#include <dt-bindings/firmware/meson-gxbb-sm.h>
> +
> +	...
> +
> +	firmware {
> +		compatible = "simple-bus";
> +
> +		sm: secure-monitor {
> +			compatible = "amlogic,meson-sm";
> +			amlogic,sm-cmd-input-base = <SM_GET_SHARE_MEM_INPUT_BASE>;
> +			amlogic,sm-cmd-output-base = <SM_GET_SHARE_MEM_OUTPUT_BASE>;
> +		};
> +	};
> +
> +Node of a device using the secure monitor must have a custom property to
> +specify the SMC32 function the driver is going to use. The values of all the
> +SMC function identifiers are listed in a SoC-specific header file:
> +
> +"include/dt-bindings/firmware/meson-gxbb-sm.h" - for the Meson GXBB secure monitor.
> +
> +Example of the node using the secure monitor:
> +
> +	#include <dt-bindings/firmware/meson-gxbb-sm.h>
> +
> +	...
> +
> +	efuse: efuse {
> +		compatible = "amlogic,meson-gxbb-efuse";
> +		secure-monitor = <&sm>;
> +		amlogic,sm-cmd-read-efuse = <SM_EFUSE_READ>;
> +		amlogic,sm-cmd-max-efuse = <SM_EFUSE_USER_MAX>;

Likewise for these?

Mark.

> +		#address-cells = <1>;
> +		#size-cells = <1>;
> +
> +		...
> +	};
> +
> -- 
> 2.7.4
> 
> --
> To unsubscribe from this list: send the line "unsubscribe devicetree" in
> the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v3 4/4] documentation: Add secure monitor binding documentation

[Carlo Caione]

On 23/05/16 17:38, Mark Rutland wrote:
> Hi,
> 
> Minor nit, but please put the binding earlier in the series than the
> code implementing it, as per
> Documentation/devicetree/bindings/submitting-patches.txt
> 
> This makes it possible to review a series in a linear fashion.

I'll do. Thanks for the pointer.

[...]
> > +
> > +Required properties for the secure monitor node:
> > +- compatible: Should be "amlogic,meson-sm"
> > +- amlogic,sm-cmd-input-base: SMC32 function identifier to read the physical
> > +                             address of the input buffer
> > +- amlogic,sm-cmd-output-base: SMC32 function identifier to read the physical
> > +                              address of the output buffer
> 
> Do the IDs for these actually differ per board?

I expect these to differ per SoC (GXBB in this case), not per board. The
driver is generic enough to be (hopefully) used for several SoCs just
changing the related header file that defines the SCM commands.

> Are some functions simply not implemented on some boards?

I don't think this is possible.

[...]
> > +
> > +Example of the node using the secure monitor:
> > +
> > +	#include <dt-bindings/firmware/meson-gxbb-sm.h>
> > +
> > +	...
> > +
> > +	efuse: efuse {
> > +		compatible = "amlogic,meson-gxbb-efuse";
> > +		secure-monitor = <&sm>;
> > +		amlogic,sm-cmd-read-efuse = <SM_EFUSE_READ>;
> > +		amlogic,sm-cmd-max-efuse = <SM_EFUSE_USER_MAX>;
> 
> Likewise for these?

AFAIK the secure monitor code and related commands are SoC specific.

Thanks,

-- 
Carlo Caione
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v3 0/4] Add Amlogic secure monitor driver

[Matthias Brugger]

On 23/05/16 18:30, Carlo Caione wrote:
> From: Carlo Caione <carlo-6IF/jdPJHihWk0Htik3J/w@public.gmane.org>
>
> In the Amlogic SoCs the secure monitor calls are used for a lot of reasons:
> interact with the NVMEM (efuses), check the reboot reason, set USB boot, ...
>
> This driver defines a generic interface towards the secure monitor that can be
> used by more specialized drivers to interact with the secure monitor itself
> without worrying about bounce buffers managing.
>

For the whole series:
Reviewed-by: Matthias Brugger <matthias.bgg-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>

> Changelog:
>
> v3:
>    * Moved driver to drivers/firmware
>    * Added EXPORT_SYMBOL macros
>    * Changed '_' in '-'
>    * Introduced 'struct meson_sm_firmware' and 'meson_sm_get_fw()' to be used by
>      drivers using the secure-monitor to enforce probe ordering
>
> v2:
>    * All the SMC function identifiers are now in a SoC-specific header file to
>      be included by the DTS files so the SMC commands are now defined in the DT
>      instead to be hardcoded into the driver
>    * Patchset is no longer an RFC
>    * Better error management and boundary checking
>    * s/unsigned int/u32/
>    * SMC call not only on CPU 0
>    * Fix memory leaking
>    * s/amlogic/meson/ in the directory names
>
>
> Carlo Caione (4):
>    firmware: Amlogic: Add secure monitor driver
>    firmware: dt-bindings: Add secure monitor header file for GXBB
>    ARM64: dts: amlogic: gxbb: Enable secure monitor
>    documentation: Add secure monitor binding documentation
>
>   .../bindings/firmware/meson/meson_sm.txt           |  51 ++++++
>   arch/arm64/boot/dts/amlogic/meson-gxbb.dtsi        |  11 ++
>   drivers/firmware/Kconfig                           |   1 +
>   drivers/firmware/Makefile                          |   1 +
>   drivers/firmware/meson/Kconfig                     |   8 +
>   drivers/firmware/meson/Makefile                    |   1 +
>   drivers/firmware/meson/meson_sm.c                  | 179 +++++++++++++++++++++
>   include/dt-bindings/firmware/meson-gxbb-sm.h       |  44 +++++
>   include/linux/firmware/meson/meson_sm.h            |  38 +++++
>   9 files changed, 334 insertions(+)
>   create mode 100644 Documentation/devicetree/bindings/firmware/meson/meson_sm.txt
>   create mode 100644 drivers/firmware/meson/Kconfig
>   create mode 100644 drivers/firmware/meson/Makefile
>   create mode 100644 drivers/firmware/meson/meson_sm.c
>   create mode 100644 include/dt-bindings/firmware/meson-gxbb-sm.h
>   create mode 100644 include/linux/firmware/meson/meson_sm.h
>
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v3 4/4] documentation: Add secure monitor binding documentation

[Mark Rutland]

On Mon, May 23, 2016 at 06:59:31PM +0200, Carlo Caione wrote:
> On 23/05/16 17:38, Mark Rutland wrote:
> > > +Required properties for the secure monitor node:
> > > +- compatible: Should be "amlogic,meson-sm"
> > > +- amlogic,sm-cmd-input-base: SMC32 function identifier to read the physical
> > > +                             address of the input buffer
> > > +- amlogic,sm-cmd-output-base: SMC32 function identifier to read the physical
> > > +                              address of the output buffer
> > 
> > Do the IDs for these actually differ per board?
> 
> I expect these to differ per SoC (GXBB in this case), not per board. The
> driver is generic enough to be (hopefully) used for several SoCs just
> changing the related header file that defines the SCM commands.
> 
> > Are some functions simply not implemented on some boards?
> 
> I don't think this is possible.

Given that, I think it may be better to just have a
"amlogic,meson-gxbb-sm" compatible string, and derive the set of
functions and associated IDs from that.

That is, unless you know that future revisions have functions with the
exact same semantics but differing IDs.

Regardless, it would make sense to have a GXBB-specific compatible
string prepended to the list. That way in the future we can handle
anything specific to the GXBB variant of the secure monitor if
necessary.

Thanks,
Mark.
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v3 1/4] firmware: Amlogic: Add secure monitor driver

[Kevin Hilman]

Carlo Caione <carlo-KA+7E9HrN00dnm+yROfE0A@public.gmane.org> writes:

> From: Carlo Caione <carlo-6IF/jdPJHihWk0Htik3J/w@public.gmane.org>
>
> Introduce a driver to provide calls into secure monitor mode.
>
> In the Amlogic SoCs these calls are used for multiple reasons: access to
> NVMEM, set USB boot, enable JTAG, etc...
>
> Signed-off-by: Carlo Caione <carlo-6IF/jdPJHihWk0Htik3J/w@public.gmane.org>

[...]

> +static int meson_sm_probe(struct platform_device *pdev)
> +{
> +	struct device_node *np = pdev->dev.of_node;
> +	struct meson_sm_firmware *fw;
> +	u32 sm_phy_in_base, sm_phy_out_base;
> +	int cmd_in, cmd_out;
> +
> +	fw = devm_kzalloc(&pdev->dev, sizeof(*fw), GFP_KERNEL);
> +	if (!fw)
> +		return -ENOMEM;
> +
> +	fw->dev = &pdev->dev;
> +
> +	if (of_property_read_u32(np, "amlogic,sm-cmd-input-base", &cmd_in))
> +		return -EINVAL;
> +
> +	if (of_property_read_u32(np, "amlogic,sm-cmd-output-base", &cmd_out))
> +		return -EINVAL;
> +
> +	sm_phy_in_base = meson_sm_call(cmd_in, 0, 0, 0, 0, 0);

Should there be any error checking here?  Do we have any info on the
return values here in case of error, or in case of missing firmware,
etc.

Kevin
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v3 4/4] documentation: Add secure monitor binding documentation

[Carlo Caione]

On 23/05/16 18:11, Mark Rutland wrote:
> On Mon, May 23, 2016 at 06:59:31PM +0200, Carlo Caione wrote:
> > On 23/05/16 17:38, Mark Rutland wrote:
> > > > +Required properties for the secure monitor node:
> > > > +- compatible: Should be "amlogic,meson-sm"
> > > > +- amlogic,sm-cmd-input-base: SMC32 function identifier to read the physical
> > > > +                             address of the input buffer
> > > > +- amlogic,sm-cmd-output-base: SMC32 function identifier to read the physical
> > > > +                              address of the output buffer
> > > 
> > > Do the IDs for these actually differ per board?
> > 
> > I expect these to differ per SoC (GXBB in this case), not per board. The
> > driver is generic enough to be (hopefully) used for several SoCs just
> > changing the related header file that defines the SCM commands.
> > 
> > > Are some functions simply not implemented on some boards?
> > 
> > I don't think this is possible.
> 
> Given that, I think it may be better to just have a
> "amlogic,meson-gxbb-sm" compatible string, and derive the set of
> functions and associated IDs from that.

I can reference a specific SMC function using an index in the DT, the
same index for all the SoCs. The index is then associated to the actual
SoC-specific command ID in the driver according to the compatible string
used for the secure-monitor node.

Something like:

  // Not SoC-specific
  #include <dt-bindings/firmware/meson.h>

  sm: sm {
  	compatible = "amlogic,meson-gxbb-sm";
  };
  
  efuse {
  	compatible = "amlogic,meson-gxbb-efuse";
  	secure-monitor = <&sm>;
  	amlogic,cmd-read-efuse = <READ_EFUSE>;
  	...
  };

Is this any better?

At this point I wonder if it makes sense having the driver-specific
function IDs (like 'amlogic,cmd-read-efuse' above) defined in the DT.

> That is, unless you know that future revisions have functions with the
> exact same semantics but differing IDs.

This is most probably the case.

Also the driver exports really generic functions to access the
secure-monitor on purpose, so that the driver using it can define the
semantic of the SMC call. I really would like to avoid fixing the
semantic in the SM driver itself since we will end up with: different
semantics for each SMC call and for each SoC.

> Regardless, it would make sense to have a GXBB-specific compatible
> string prepended to the list. That way in the future we can handle
> anything specific to the GXBB variant of the secure monitor if
> necessary.

Agree on this.

Cheers,

-- 
Carlo Caione
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v3 1/4] firmware: Amlogic: Add secure monitor driver

[Carlo Caione]

On 23/05/16 13:58, Kevin Hilman wrote:

[...]
> > +	if (of_property_read_u32(np, "amlogic,sm-cmd-input-base", &cmd_in))
> > +		return -EINVAL;
> > +
> > +	if (of_property_read_u32(np, "amlogic,sm-cmd-output-base", &cmd_out))
> > +		return -EINVAL;
> > +
> > +	sm_phy_in_base = meson_sm_call(cmd_in, 0, 0, 0, 0, 0);
> 
> Should there be any error checking here?  Do we have any info on the
> return values here in case of error, or in case of missing firmware,
> etc.

We do not have any info on that but I can assume 0 is considered an
error as for other SMC calls. Fix in v4.

-- 
Carlo Caione
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html