soc: Amlogic: Add secure monitor driver
Subsystems (2)
RFC PATCH 1/1 soc: Amlogic: Add secure monitor driver
From: Carlo Caione <[email protected]>
Introduce a driver to provide calls into secure monitor mode.
In the Amlogic SoCs these calls are used for multiple reasons: access to NVMEM, reboot, set USB boot, enable JTAG, etc...
--- arch/arm64/boot/dts/amlogic/meson-gxbb.dtsi | 4 + drivers/soc/Kconfig | 1 + drivers/soc/Makefile | 1 + drivers/soc/amlogic/Kconfig | 8 ++ drivers/soc/amlogic/Makefile | 1 + drivers/soc/amlogic/meson_sm.c | 141 ++++++++++++++++++++++++++++ include/linux/soc/amlogic/meson_sm.h | 57 +++++++++++ 7 files changed, 213 insertions(+) create mode 100644 drivers/soc/amlogic/Kconfig create mode 100644 drivers/soc/amlogic/Makefile create mode 100644 drivers/soc/amlogic/meson_sm.c create mode 100644 include/linux/soc/amlogic/meson_sm.h
arch/arm64/boot/dts/amlogic/meson-gxbb.dtsi +4 -0
@@ -98,6 +98,10 @@ method = "smc"; }; + sm { + compatible = "amlogic,meson-sm"; + }; + timer { compatible = "arm,armv8-timer"; interrupts = <GIC_PPI 13
drivers/soc/Kconfig +1 -0
@@ -1,5 +1,6 @@ menu "SOC (System On Chip) specific Drivers" +source "drivers/soc/amlogic/Kconfig" source "drivers/soc/bcm/Kconfig" source "drivers/soc/brcmstb/Kconfig" source "drivers/soc/fsl/qe/Kconfig"
drivers/soc/Makefile +1 -0
@@ -8,6 +8,7 @@ obj-$(CONFIG_ARCH_DOVE) += dove/ obj-$(CONFIG_MACH_DOVE) += dove/ obj-y += fsl/ obj-$(CONFIG_ARCH_MEDIATEK) += mediatek/ +obj-$(CONFIG_ARCH_MESON) += amlogic/ obj-$(CONFIG_ARCH_QCOM) += qcom/ obj-$(CONFIG_ARCH_ROCKCHIP) += rockchip/ obj-$(CONFIG_SOC_SAMSUNG) += samsung/
drivers/soc/amlogic/Kconfig new file +8 -0
@@ -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
drivers/soc/amlogic/Makefile new file +1 -0
@@ -0,0 +1 @@ +obj-$(CONFIG_MESON_SM) += meson_sm.o
drivers/soc/amlogic/meson_sm.c new file +141 -0
141 lines not on this page · raw message
include/linux/soc/amlogic/meson_sm.h new file +57 -0
57 lines not on this page · raw message
Signature · 1 line
2.7.4
RFC PATCH 1/1 soc: Amlogic: Add secure monitor driver
Quoted 8 lines · Carlo Caione
From: Carlo Caione <[email protected]>
Introduce a driver to provide calls into secure monitor mode.
In the Amlogic SoCs these calls are used for multiple reasons: access to NVMEM, reboot, set USB boot, enable JTAG, etc...
Signed-off-by Carlo Caione <[email protected]>
Quoted 9 lines
--- arch/arm64/boot/dts/amlogic/meson-gxbb.dtsi | 4 + drivers/soc/Kconfig | 1 + drivers/soc/Makefile | 1 + drivers/soc/amlogic/Kconfig | 8 ++ drivers/soc/amlogic/Makefile | 1 + drivers/soc/amlogic/meson_sm.c | 141 ++++++++++++++++++++++++++++ include/linux/soc/amlogic/meson_sm.h | 57 +++++++++++ 7 files changed, 213 insertions(+)
The binding is missing documentation.
Is there any documentation of the actual interface?
Are there any restrictions on how it's used? It looks like calls are only expected to happen from the boot CPU, per the code below.
There are no users in this patch. Are there any example uses (i.e. specific code rather than the high-level examples above)?
Quoted 10 lines
create mode 100644 drivers/soc/amlogic/Kconfig create mode 100644 drivers/soc/amlogic/Makefile create mode 100644 drivers/soc/amlogic/meson_sm.c create mode 100644 include/linux/soc/amlogic/meson_sm.harch/arm64/boot/dts/amlogic/meson-gxbb.dtsi +0
-0
arch/arm64/boot/dts/amlogic/meson-gxbb.dtsi line 103
Quoted 6 lines
arch/arm64/boot/dts/amlogic/meson-gxbb.dtsi +3
-0@@ -98,6 +98,10 @@ method = "smc"; }; + sm { + compatible = "amlogic,meson-sm"; + };
We should have more specific strings, at least in addition to this.
Quoted 94 lines
arch/arm64/boot/dts/amlogic/meson-gxbb.dtsi +1
-0+ timer { compatible = "arm,armv8-timer"; interrupts = <GIC_PPI 13drivers/soc/Kconfig +1
-0@@ -1,5 +1,6 @@ menu "SOC (System On Chip) specific Drivers" +source "drivers/soc/amlogic/Kconfig" source "drivers/soc/bcm/Kconfig" source "drivers/soc/brcmstb/Kconfig" source "drivers/soc/fsl/qe/Kconfig"drivers/soc/Makefile +1
-0@@ -8,6 +8,7 @@ obj-$(CONFIG_ARCH_DOVE) += dove/ obj-$(CONFIG_MACH_DOVE) += dove/ obj-y += fsl/ obj-$(CONFIG_ARCH_MEDIATEK) += mediatek/ +obj-$(CONFIG_ARCH_MESON) += amlogic/ obj-$(CONFIG_ARCH_QCOM) += qcom/ obj-$(CONFIG_ARCH_ROCKCHIP) += rockchip/ obj-$(CONFIG_SOC_SAMSUNG) += samsung/drivers/soc/amlogic/Kconfig new file +8
-0@@ -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 driverdrivers/soc/amlogic/Makefile new file +1
-0@@ -0,0 +1 @@ +obj-$(CONFIG_MESON_SM) += meson_sm.odrivers/soc/amlogic/meson_sm.c new file +40
-0@@ -0,0 +1,141 @@ +/* +* Amlogic Secure Monitor driver +* +* Copyright (C) 2016 Endless Mobile, Inc. +* Author: Carlo Caione <[email protected]> +* +* 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/soc/amlogic/meson_sm.h> + +#define SM_MEM_SIZE 0x1000 + +/* +* To read from / write to the secure monitor we use two bounce buffers. The +* physical addresses of the two buffers are obtained by querying the secure +* monitor itself. +*/ + +static u32 sm_phy_in_base; +static u32 sm_phy_out_base; + +static void __iomem *sm_sharemem_in_base; +static void __iomem *sm_sharemem_out_base;
drivers/soc/amlogic/meson_sm.c line 50
Quoted 10 lines
drivers/soc/amlogic/meson_sm.c new file +10
-0+ +struct meson_sm_data { +unsigned int cmd; +unsigned int arg0; +unsigned int arg1; +unsigned int arg2; +unsigned int arg3; +unsigned int arg4; +unsigned int ret; +};
Please be explicit and use u32 rather than unsigned int.
It looks like all the calls you care about are SMC32 SiP calls, per the IDs in the header.
Quoted 17 lines
drivers/soc/amlogic/meson_sm.c new file +17
-0+ +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; +} + +unsigned int meson_sm_call (unsigned int cmd,unsigned int arg0, +unsigned int arg1,unsigned int arg2, +unsigned int arg3,unsigned int arg4) +{ +struct meson_sm_data data;
drivers/soc/amlogic/meson_sm.c line 77
Quoted 10 lines
drivers/soc/amlogic/meson_sm.c new file +10
-0+ + data.cmd = cmd; + data.arg0 = arg0; + data.arg1 = arg1; + data.arg2 = arg2; + data.arg3 = arg3; + data.arg4 = arg4; + data.ret =0 ; + +smp_call_function_single (0 , __meson_sm_call, &data,1 );
Why must this occur on a particular CPU?
With kexec and so on logical CPU 0 is not necessarily the CPU that the FW brought out of reset.
Can the core with the secure OS be powered down, or is that prevented with the usual PSCI machinery (e.g. MIGRATE, MIGRATE_INFO_TYPE, MIGRATE_INFO_UP_CPU)
I would recommend that any CPU restriction were described explicitly in the DT, or detected dynamically based on the MIGRATE_INFO_UP_CPU value.
Quoted 4 lines
drivers/soc/amlogic/meson_sm.c new file +4
-0+ +return data.ret; +} +
drivers/soc/amlogic/meson_sm.c line 91
Quoted 10 lines
drivers/soc/amlogic/meson_sm.c new file +10
-0+unsigned int meson_sm_call_read (void *buffer,unsigned int cmd, +unsigned int arg0,unsigned int arg1, +unsigned int arg2,unsigned int arg3, +unsigned int arg4) +{ +unsigned int size; + + size =meson_sm_call (cmd, arg0, arg1, arg2, arg3, arg4); + +if (size !=0 )
Is the size guaranteed to never be > SM_MEM_SIZE? Can we sanity-check that?
The call can't return an error code?
drivers/soc/amlogic/meson_sm.c line 92
Quoted 1 line
drivers/soc/amlogic/meson_sm.c new file +1
-0+memcpy (buffer, sm_sharemem_out_base, size);
Is the buffer completely opaque to this layer?
Are there any endianness concerns, for example?
Quoted 11 lines
drivers/soc/amlogic/meson_sm.c new file +11
-0+ +return size; +} + +unsigned int meson_sm_call_write (void *buffer,unsigned int size, +unsigned int cmd,unsigned int arg0, +unsigned int arg1,unsigned int arg2, +unsigned int arg3,unsigned int arg4) +{ +memcpy (sm_sharemem_in_base, buffer, size); +
drivers/soc/amlogic/meson_sm.c line 113
Quoted 10 lines
drivers/soc/amlogic/meson_sm.c new file +10
-0+return meson_sm_call (cmd, arg0, arg1, arg2, arg3, arg4); +} + +static int meson_sm_probe (struct platform_device *pdev) +{ + sm_phy_in_base =meson_sm_call (SM_GET_SHARE_MEM_INPUT_BASE ,0 ,0 ,0 ,0 ,0 ); + + sm_sharemem_in_base =ioremap_cache (sm_phy_in_base,SM_MEM_SIZE ); +if (!sm_sharemem_in_base) +return -EINVAL ;
Are the attributes used for ioremap_cache guaranteed to match and not cause a mismatched alias against the firmware's mapping?
Which precise memory attributes does the secure firmware use for the shared memory?
Quoted 66 lines
drivers/soc/amlogic/meson_sm.c new file +28
-0+ + sm_phy_out_base =meson_sm_call (SM_GET_SHARE_MEM_OUTPUT_BASE ,0 ,0 ,0 ,0 ,0 ); + + sm_sharemem_out_base =ioremap_cache (sm_phy_out_base,SM_MEM_SIZE ); +if (!sm_sharemem_out_base) +return -EINVAL ; + +return 0 ; +} + +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 <[email protected]>" ); +MODULE_DESCRIPTION ("Amlogic secure monitor driver" ); +MODULE_LICENSE ("GPL" );include/linux/soc/amlogic/meson_sm.h new file +32
-0@@ -0,0 +1,57 @@ +/* +* Copyright (C) 2016 Endless Mobile, Inc. +* Author: Carlo Caione <[email protected]> +* +* 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_ + +#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
include/linux/soc/amlogic/meson_sm.h line 42
Quoted 10 lines
include/linux/soc/amlogic/meson_sm.h new file +10
-0+#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
Is there any documentation for these functions?
include/linux/soc/amlogic/meson_sm.h line 43
Quoted 1 line
include/linux/soc/amlogic/meson_sm.h new file +1
-0+#define SM_PSCI_SYS_REBOOT 0x84000009
The existing PCSI code should be the only caller of this. This should not be used here.
Thanks,
Mark.
RFC PATCH 1/1 soc: Amlogic: Add secure monitor driver
Quoted 19 lines · Mark Rutland
Quoted 8 lines · Carlo Caione
From: Carlo Caione <[email protected]>
Introduce a driver to provide calls into secure monitor mode.
In the Amlogic SoCs these calls are used for multiple reasons: access to NVMEM, reboot, set USB boot, enable JTAG, etc...
Signed-off-by Carlo Caione <[email protected]>Quoted 9 lines
--- arch/arm64/boot/dts/amlogic/meson-gxbb.dtsi | 4 + drivers/soc/Kconfig | 1 + drivers/soc/Makefile | 1 + drivers/soc/amlogic/Kconfig | 8 ++ drivers/soc/amlogic/Makefile | 1 + drivers/soc/amlogic/meson_sm.c | 141 ++++++++++++++++++++++++++++ include/linux/soc/amlogic/meson_sm.h | 57 +++++++++++ 7 files changed, 213 insertions(+)
Quoted 1 line
The binding is missing documentation.
Yes, not much to document for this RFC.
Quoted 1 line
Is there any documentation of the actual interface?
There isn't or at least I do not have it. This driver has been written by reverse engineering the Amlogic driver shipped in their SDK [1]
Quoted 2 lines
Are there any restrictions on how it's used? It looks like calls are only expected to happen from the boot CPU, per the code below.
Yes, that's what I could infer from the Amlogic code here [2] but then I checked this better and it seems that's not the case (see below).
Quoted 2 lines
There are no users in this patch. Are there any example uses (i.e. specific code rather than the high-level examples above)?
I use it already to read the NVMEM. It works fine.
I'll submit the NVMEM driver as soon as I have this merged (if ever)
Quoted 13 lines
Quoted 13 lines
create mode 100644 drivers/soc/amlogic/Kconfig create mode 100644 drivers/soc/amlogic/Makefile create mode 100644 drivers/soc/amlogic/meson_sm.c create mode 100644 include/linux/soc/amlogic/meson_sm.harch/arm64/boot/dts/amlogic/meson-gxbb.dtsi +0
-0@@ -98,6 +98,10 @@ method = "smc"; };
Quoted 5 lines
arch/arm64/boot/dts/amlogic/meson-gxbb.dtsi line 103
Quoted 3 lines
arch/arm64/boot/dts/amlogic/meson-gxbb.dtsi +3
-0+ sm { + compatible = "amlogic,meson-sm"; + };We should have more specific strings, at least in addition to this.
Agree, not sure what I can add though.
[...]
Quoted 11 lines
arch/arm64/boot/dts/amlogic/meson-gxbb.dtsi
Quoted 9 lines
arch/arm64/boot/dts/amlogic/meson-gxbb.dtsi +9
-0+struct meson_sm_data { + unsigned int cmd; + unsigned int arg0; + unsigned int arg1; + unsigned int arg2; + unsigned int arg3; + unsigned int arg4; + unsigned int ret; +};Please be explicit and use u32 rather than unsigned int.
Ok
Quoted 2 lines
It looks like all the calls you care about are SMC32 SiP calls, per the IDs in the header.
Yes, all the SMC function identifiers I gathered from the Amlogic sources are SMC32 SiP calls
Quoted 30 lines
Quoted 17 lines
arch/arm64/boot/dts/amlogic/meson-gxbb.dtsi +17
-0+ +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; +} + +unsigned int meson_sm_call(unsigned int cmd, unsigned int arg0, + unsigned int arg1, unsigned int arg2, + unsigned int arg3, unsigned int arg4) +{ + struct meson_sm_data data;arch/arm64/boot/dts/amlogic/meson-gxbb.dtsi
Quoted 10 lines
arch/arm64/boot/dts/amlogic/meson-gxbb.dtsi +10
-0+ + data.cmd = cmd; + data.arg0 = arg0; + data.arg1 = arg1; + data.arg2 = arg2; + data.arg3 = arg3; + data.arg4 = arg4; + data.ret = 0; + + smp_call_function_single(0, __meson_sm_call, &data, 1);Why must this occur on a particular CPU?
Quoted 2 lines
With kexec and so on logical CPU 0 is not necessarily the CPU that the FW brought out of reset.
You are right. I tried to move this to a different CPU and it keeps working fine. Still I wonder why Amlogic has something like [2]
Quoted 6 lines
Can the core with the secure OS be powered down, or is that prevented with the usual PSCI machinery (e.g. MIGRATE,
MIGRATE_INFO_TYPE,MIGRATE_INFO_UP_CPU)I would recommend that any CPU restriction were described explicitly in the DT, or detected dynamically based on the
MIGRATE_INFO_UP_CPUvalue.
MIGRATE_INFO_TYPE returns PSCI_0_2_TOS_MP. Does this confirm that the SMC calls can happen not only from the CPU 0?
Quoted 9 lines
Quoted 9 lines
arch/arm64/boot/dts/amlogic/meson-gxbb.dtsi +9
-0+ + return data.ret; +} + +unsigned int meson_sm_call_read(void *buffer, unsigned int cmd, + unsigned int arg0, unsigned int arg1, + unsigned int arg2, unsigned int arg3, + unsigned int arg4) +{
Quoted 8 lines
arch/arm64/boot/dts/amlogic/meson-gxbb.dtsi
Quoted 5 lines
arch/arm64/boot/dts/amlogic/meson-gxbb.dtsi +5
-0+ unsigned int size; + + size = meson_sm_call(cmd, arg0, arg1, arg2, arg3, arg4); + + if (size != 0)Is the size guaranteed to never be >
SM_MEM_SIZE? Can we sanity-check that?
Fix in v2
Quoted 1 line
The call can't return an error code?
0 in R0 is considered error that we return back.
Quoted 5 lines
Quoted 1 line
+
memcpy(buffer, sm_sharemem_out_base, size);Is the buffer completely opaque to this layer?
Are there any endianness concerns, for example?
The best answer I can give you here is: not that I know of. The driver works fine when reading from the NVMEM, so I assume there isn't any endiness issue.
Quoted 25 lines
Quoted 11 lines
arch/arm64/boot/dts/amlogic/meson-gxbb.dtsi +11
-0+ + return size; +} + +unsigned int meson_sm_call_write(void *buffer, unsigned int size, + unsigned int cmd, unsigned int arg0, + unsigned int arg1, unsigned int arg2, + unsigned int arg3, unsigned int arg4) +{ + memcpy(sm_sharemem_in_base, buffer, size); +arch/arm64/boot/dts/amlogic/meson-gxbb.dtsi
Quoted 10 lines
arch/arm64/boot/dts/amlogic/meson-gxbb.dtsi +10
-0+ return meson_sm_call(cmd, arg0, arg1, arg2, arg3, arg4); +} + +static int meson_sm_probe(struct platform_device *pdev) +{ + sm_phy_in_base = meson_sm_call(SM_GET_SHARE_MEM_INPUT_BASE, 0, 0, 0, 0, 0); + + sm_sharemem_in_base = ioremap_cache(sm_phy_in_base, SM_MEM_SIZE); + if (!sm_sharemem_in_base) + return -EINVAL;Are the attributes used for
ioremap_cacheguaranteed to match and not cause a mismatched alias against the firmware's mapping?
Quoted 2 lines
Which precise memory attributes does the secure firmware use for the shared memory?
Again I wish I had more information. Is there any way I can check this? The only reasonable answer I can give you right now is that this is how it is done in the original Amlogic driver in [1]
[...]
Quoted 21 lines
Quoted 21 lines
arch/arm64/boot/dts/amlogic/meson-gxbb.dtsi +21
-0+#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
Quoted 8 lines
arch/arm64/boot/dts/amlogic/meson-gxbb.dtsi
Quoted 6 lines
arch/arm64/boot/dts/amlogic/meson-gxbb.dtsi +6
-0+#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 0x820000FFIs there any documentation for these functions?
Unfortunately there isn't.
I could restrict the list to those functions I know how to use (*_EFUSE) maybe adding the remaining ones when we actually use them.
Quoted 4 lines
Quoted 1 line
+#define
SM_PSCI_SYS_REBOOT0x84000009The existing PCSI code should be the only caller of this. This should not be used here.
Ups, that slipped through when pasting the list.
Thank you for reviewing this RFC, I'll submit a v2 soon.
Cheers,
[1] https://github.com/hardkernel/linux/blob/odroidc2-3.14.y/drivers/amlogic/secmon/secmon.c
[2] https://github.com/hardkernel/linux/blob/odroidc2-3.14.y/drivers/amlogic/efuse/efuse_hw64.c#L94
Signature · 1 line
Carlo Caione
RFC PATCH 1/1 soc: Amlogic: Add secure monitor driver
Quoted 54 lines · Carlo Caione
Quoted 19 lines · Mark Rutland
Quoted 8 lines · Carlo Caione
From: Carlo Caione <[email protected]>
Introduce a driver to provide calls into secure monitor mode.
In the Amlogic SoCs these calls are used for multiple reasons: access to NVMEM, reboot, set USB boot, enable JTAG, etc...
Signed-off-by Carlo Caione <[email protected]>Quoted 9 lines
--- arch/arm64/boot/dts/amlogic/meson-gxbb.dtsi | 4 + drivers/soc/Kconfig | 1 + drivers/soc/Makefile | 1 + drivers/soc/amlogic/Kconfig | 8 ++ drivers/soc/amlogic/Makefile | 1 + drivers/soc/amlogic/meson_sm.c | 141 ++++++++++++++++++++++++++++ include/linux/soc/amlogic/meson_sm.h | 57 +++++++++++ 7 files changed, 213 insertions(+)Quoted 1 line
The binding is missing documentation.
Yes, not much to document for this RFC.
Quoted 1 line
Is there any documentation of the actual interface?
There isn't or at least I do not have it. This driver has been written by reverse engineering the Amlogic driver shipped in their SDK [1]
Quoted 2 lines
Are there any restrictions on how it's used? It looks like calls are only expected to happen from the boot CPU, per the code below.
Yes, that's what I could infer from the Amlogic code here [2] but then I checked this better and it seems that's not the case (see below).
Quoted 2 lines
There are no users in this patch. Are there any example uses (i.e. specific code rather than the high-level examples above)?
I use it already to read the NVMEM. It works fine.
I'll submit the NVMEM driver as soon as I have this merged (if ever)Quoted 13 lines
Quoted 13 lines
create mode 100644
drivers/soc/amlogic/Kconfigcreate mode 100644drivers/soc/amlogic/Makefilecreate mode 100644drivers/soc/amlogic/meson_sm.ccreate mode 100644include/linux/soc/amlogic/meson_sm.harch/arm64/boot/dts/amlogic/meson-gxbb.dtsi +0
-0@@ -98,6 +98,10 @@ method = "smc"; };
Quoted 7 lines
Quoted 5 lines
arch/arm64/boot/dts/amlogic/meson-gxbb.dtsi line 103
Quoted 3 lines
arch/arm64/boot/dts/amlogic/meson-gxbb.dtsi +3
-0+ sm { + compatible = "amlogic,meson-sm"; + };We should have more specific strings, at least in addition to this.
Agree, not sure what I can add though.
I would say someting linke "amlogic,gxbb-sm" to specify the SoC.
Quoted 183 lines
[...]
Quoted 11 lines
arch/arm64/boot/dts/amlogic/meson-gxbb.dtsi
Quoted 9 lines
arch/arm64/boot/dts/amlogic/meson-gxbb.dtsi +9
-0+struct meson_sm_data { + unsigned int cmd; + unsigned int arg0; + unsigned int arg1; + unsigned int arg2; + unsigned int arg3; + unsigned int arg4; + unsigned int ret; +};Please be explicit and use u32 rather than unsigned int.
Ok
Quoted 2 lines
It looks like all the calls you care about are SMC32 SiP calls, per the IDs in the header.
Yes, all the SMC function identifiers I gathered from the Amlogic sources are SMC32 SiP calls
Quoted 30 lines
Quoted 17 lines
arch/arm64/boot/dts/amlogic/meson-gxbb.dtsi +17
-0+ +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; +} + +unsigned int meson_sm_call(unsigned int cmd, unsigned int arg0, + unsigned int arg1, unsigned int arg2, + unsigned int arg3, unsigned int arg4) +{ + struct meson_sm_data data;arch/arm64/boot/dts/amlogic/meson-gxbb.dtsi
Quoted 10 lines
arch/arm64/boot/dts/amlogic/meson-gxbb.dtsi +10
-0+ + data.cmd = cmd; + data.arg0 = arg0; + data.arg1 = arg1; + data.arg2 = arg2; + data.arg3 = arg3; + data.arg4 = arg4; + data.ret = 0; + + smp_call_function_single(0, __meson_sm_call, &data, 1);Why must this occur on a particular CPU?
Quoted 2 lines
With kexec and so on logical CPU 0 is not necessarily the CPU that the FW brought out of reset.
You are right. I tried to move this to a different CPU and it keeps working fine. Still I wonder why Amlogic has something like [2]
Quoted 6 lines
Can the core with the secure OS be powered down, or is that prevented with the usual PSCI machinery (e.g. MIGRATE,
MIGRATE_INFO_TYPE,MIGRATE_INFO_UP_CPU)I would recommend that any CPU restriction were described explicitly in the DT, or detected dynamically based on the
MIGRATE_INFO_UP_CPUvalue.
MIGRATE_INFO_TYPEreturnsPSCI_0_2_TOS_MP. Does this confirm that the SMC calls can happen not only from the CPU 0?Quoted 9 lines
Quoted 9 lines
arch/arm64/boot/dts/amlogic/meson-gxbb.dtsi +9
-0+ + return data.ret; +} + +unsigned int meson_sm_call_read(void *buffer, unsigned int cmd, + unsigned int arg0, unsigned int arg1, + unsigned int arg2, unsigned int arg3, + unsigned int arg4) +{Quoted 8 lines
arch/arm64/boot/dts/amlogic/meson-gxbb.dtsi
Quoted 5 lines
arch/arm64/boot/dts/amlogic/meson-gxbb.dtsi +5
-0+ unsigned int size; + + size = meson_sm_call(cmd, arg0, arg1, arg2, arg3, arg4); + + if (size != 0)Is the size guaranteed to never be >
SM_MEM_SIZE? Can we sanity-check that?Fix in v2
Quoted 1 line
The call can't return an error code?
0 in R0 is considered error that we return back.
Quoted 5 lines
Quoted 1 line
+
memcpy(buffer, sm_sharemem_out_base, size);Is the buffer completely opaque to this layer?
Are there any endianness concerns, for example?
The best answer I can give you here is: not that I know of. The driver works fine when reading from the NVMEM, so I assume there isn't any endiness issue.
Quoted 25 lines
Quoted 11 lines
arch/arm64/boot/dts/amlogic/meson-gxbb.dtsi +11
-0+ + return size; +} + +unsigned int meson_sm_call_write(void *buffer, unsigned int size, + unsigned int cmd, unsigned int arg0, + unsigned int arg1, unsigned int arg2, + unsigned int arg3, unsigned int arg4) +{ + memcpy(sm_sharemem_in_base, buffer, size); +arch/arm64/boot/dts/amlogic/meson-gxbb.dtsi
Quoted 10 lines
arch/arm64/boot/dts/amlogic/meson-gxbb.dtsi +10
-0+ return meson_sm_call(cmd, arg0, arg1, arg2, arg3, arg4); +} + +static int meson_sm_probe(struct platform_device *pdev) +{ + sm_phy_in_base = meson_sm_call(SM_GET_SHARE_MEM_INPUT_BASE, 0, 0, 0, 0, 0); + + sm_sharemem_in_base = ioremap_cache(sm_phy_in_base, SM_MEM_SIZE); + if (!sm_sharemem_in_base) + return -EINVAL;Are the attributes used for
ioremap_cacheguaranteed to match and not cause a mismatched alias against the firmware's mapping?Quoted 2 lines
Which precise memory attributes does the secure firmware use for the shared memory?
Again I wish I had more information. Is there any way I can check this? The only reasonable answer I can give you right now is that this is how it is done in the original Amlogic driver in [1]
[...]
Quoted 21 lines
Quoted 21 lines
arch/arm64/boot/dts/amlogic/meson-gxbb.dtsi +21
-0+#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 0x82000065Quoted 8 lines
arch/arm64/boot/dts/amlogic/meson-gxbb.dtsi
Quoted 6 lines
arch/arm64/boot/dts/amlogic/meson-gxbb.dtsi +6
-0+#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 0x820000FFIs there any documentation for these functions?
Unfortunately there isn't.
I could restrict the list to those functions I know how to use (*_EFUSE) maybe adding the remaining ones when we actually use them.Quoted 4 lines
Quoted 1 line
+#define
SM_PSCI_SYS_REBOOT0x84000009The existing PCSI code should be the only caller of this. This should not be used here.
Ups, that slipped through when pasting the list.
Thank you for reviewing this RFC, I'll submit a v2 soon.
Cheers,
[1] https://github.com/hardkernel/linux/blob/odroidc2-3.14.y/drivers/amlogic/secmon/secmon.c
[2] https://github.com/hardkernel/linux/blob/odroidc2-3.14.y/drivers/amlogic/efuse/efuse_hw64.c#L94
RFC PATCH 1/1 soc: Amlogic: Add secure monitor driver
On 23/05/16 13:09, Matthias Brugger wrote:
[...]
Quoted 14 lines
Quoted 14 lines
Quoted 13 lines
arch/arm64/boot/dts/amlogic/meson-gxbb.dtsi line 103
Quoted 11 lines
arch/arm64/boot/dts/amlogic/meson-gxbb.dtsi +3
-0@@ -98,6 +98,10 @@ method = "smc"; }; + sm { + compatible = "amlogic,meson-sm"; + };We should have more specific strings, at least in addition to this.
Quoted 3 lines
Quoted 1 line
Agree, not sure what I can add though.
I would say someting linke "amlogic,gxbb-sm" to specify the SoC.
If you look at v2 of this patchset [1] I separated the SoC specific SMC32 function definitions in a separate header file so I think the main driver is generic enough to be called meson-sm.
Cheers,
[1] http://www.spinics.net/lists/arm-kernel/msg505087.html
Signature · 1 line
Carlo Caione