* [PATCH 2/4] firmware: Samsung: Add secure monitor driver [not found] <CGME20220713045516epcms1p86b3f6a8795d767faac65eb947405f911@epcms1p8> @ 2022-07-13 4:55 ` Dongjin Yang 2022-07-13 4:58 ` Randy Dunlap ` (3 more replies) 0 siblings, 4 replies; 6+ messages in thread From: Dongjin Yang @ 2022-07-13 4:55 UTC (permalink / raw) To: jesper.nilsson@axis.com, lars.persson@axis.com, robh+dt@kernel.org, krzk+dt@kernel.org, lee.jones@linaro.org Cc: javierm@redhat.com, bjorn.andersson@linaro.org, linux-arm-kernel@axis.com, devicetree@vger.kernel.org, linux-kernel@vger.kernel.org, Moon-Ki Jun, Sang Min Kim, Wangseok Lee, Dongjin Yang Introduce a driver to provide calls into secure monitor mode. This driver is used for SoCs produced by Samsung Foundry to provide SMC call. This patch supports register read/write request to secure monitor. Also, SMC call request which uses shared memory to exchange the data between kernel and secure monitor. Signed-off-by: Dongjin Yang <dj76.yang@samsung.com> --- MAINTAINERS | 2 + drivers/firmware/Kconfig | 11 +++ drivers/firmware/Makefile | 1 + drivers/firmware/samsung-smc-svc.c | 154 +++++++++++++++++++++++++++++++ include/linux/firmware/samsung-smc-svc.h | 59 ++++++++++++ 5 files changed, 227 insertions(+) create mode 100644 drivers/firmware/samsung-smc-svc.c create mode 100644 include/linux/firmware/samsung-smc-svc.h diff --git a/MAINTAINERS b/MAINTAINERS index 6763746c349f..d173043ffb46 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -1868,8 +1868,10 @@ F: arch/arm/boot/dts/artpec6* F: arch/arm/mach-artpec F: drivers/clk/axis F: drivers/crypto/axis +F: drivers/firmware/samsung-smc-svc.c F: drivers/mmc/host/usdhi6rol0.c F: drivers/pinctrl/pinctrl-artpec* +F: include/linux/firmware/samsung-smc-svc.h ARM/ASPEED I2C DRIVER M: Brendan Higgins <brendanhiggins@google.com> diff --git a/drivers/firmware/Kconfig b/drivers/firmware/Kconfig index e5cfb01353d8..4b0f2d033f58 100644 --- a/drivers/firmware/Kconfig +++ b/drivers/firmware/Kconfig @@ -217,6 +217,17 @@ config QCOM_SCM_DOWNLOAD_MODE_DEFAULT Say Y here to enable "download mode" by default. +config SAMSUNG_SECURE_SERVICE + bool "Samsung Foundry Secure Service Layer" + depends on HAVE_ARM_SMCCC + default n + help + Support secure service layer for SoCs which is manufactured by + Samsung Foundry. + + This option provide support of secure monitor service call using + Trusted Foundations. + config SYSFB bool default y diff --git a/drivers/firmware/Makefile b/drivers/firmware/Makefile index 4e58cb474a68..985e30a9665f 100644 --- a/drivers/firmware/Makefile +++ b/drivers/firmware/Makefile @@ -19,6 +19,7 @@ obj-$(CONFIG_RASPBERRYPI_FIRMWARE) += raspberrypi.o obj-$(CONFIG_FW_CFG_SYSFS) += qemu_fw_cfg.o obj-$(CONFIG_QCOM_SCM) += qcom-scm.o qcom-scm-objs += qcom_scm.o qcom_scm-smc.o qcom_scm-legacy.o +obj-$(CONFIG_SAMSUNG_SECURE_SERVICE) += samsung-smc-svc.o obj-$(CONFIG_SYSFB) += sysfb.o obj-$(CONFIG_SYSFB_SIMPLEFB) += sysfb_simplefb.o obj-$(CONFIG_TI_SCI_PROTOCOL) += ti_sci.o diff --git a/drivers/firmware/samsung-smc-svc.c b/drivers/firmware/samsung-smc-svc.c new file mode 100644 index 000000000000..eb3a5285cf2b --- /dev/null +++ b/drivers/firmware/samsung-smc-svc.c @@ -0,0 +1,154 @@ +// SPDX-License-Identifier: GPL-2.0 +/* + * Copyright (c) 2019 Samsung Electronics Co., Ltd. + * Author: Kihyun Yoon<kihyun.yoon@samsung.com> + * Author: Dongjin Yang<dj76.yang@samsung.com> + * + * 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. + * + */ + +#include <linux/types.h> +#include <linux/errno.h> +#include <linux/module.h> +#include <linux/io.h> +#include <linux/of.h> +#include <linux/of_device.h> +#include <linux/platform_device.h> +#include <linux/firmware/samsung-smc-svc.h> + +struct samsung_smc_version { + u32 major; + u32 minor; +}; + +struct samsung_smc_data { + struct samsung_smc_version version; + u32 svc_cmd_list[SAMSUNG_SIP_NR_SIP]; +}; + +struct samsung_smc_data *svc_data; + +static int samsung_smc_svc_init(void); + +static int __samsung_get_svc_cmd(u32 cmd_idx) +{ + if (cmd_idx >= SAMSUNG_SIP_NR_SIP) + return SAMSUNG_SIP_NOT_SUPPORTED; + + /* Initialize this driver if it is not ready */ + if (!svc_data && samsung_smc_svc_init()) { + pr_err("samsung,smccc_service initialization is failed\n"); + return SAMSUNG_SIP_NOT_SUPPORTED; + } + + return svc_data->svc_cmd_list[cmd_idx]; +} + +int samsung_smc_call(u32 cmd_idx, + unsigned long a0, unsigned long a1, + unsigned long a2, unsigned long a3, + u32 *val) + +{ + struct arm_smccc_res res; + u32 cmd = __samsung_get_svc_cmd(cmd_idx); + + if (cmd == SAMSUNG_SIP_NOT_SUPPORTED) + return -EOPNOTSUPP; + + arm_smccc_smc(cmd, a0, a1, a2, a3, 0, 0, 0, &res); + if (res.a0 == SMCCC_RET_NOT_SUPPORTED) + return -EINVAL; + + if (val) + *val = (u32)res.a1; + + return 0; +} +EXPORT_SYMBOL(samsung_smc_call); + +int samsung_smc_reg_read(void *base, u32 reg, u32 *val) +{ + return samsung_smc_call(SAMSUNG_SIP_READ_REG, + (unsigned long)base, reg, 0, 0, val); +} +EXPORT_SYMBOL(samsung_smc_reg_read); + +int samsung_smc_reg_write(void *base, u32 reg, u32 val) +{ + return samsung_smc_call(SAMSUNG_SIP_WRITE_REG, + (unsigned long)base, reg, val, 0, NULL); +} +EXPORT_SYMBOL(samsung_smc_reg_write); + +static int samsung_smc_check_version(struct samsung_smc_version *ver) +{ + struct arm_smccc_res res; + + arm_smccc_smc(SAMSUNG_SIP_SVC_VERSION, 0, 0, 0, 0, 0, 0, 0, &res); + + if ((u32)res.a0 != ver->major || (u32)res.a1 != ver->minor) + return -EINVAL; + + pr_info("Samsung SVC version [%d.%d] is detected\n", + ver->major, ver->minor); + + return 0; +} + +static const struct samsung_smc_data svc_0_1 = { + .version = { + .major = 0, + .minor = 1 + }, + .svc_cmd_list = { + [SAMSUNG_SIP_READ_REG] = 0x82000020, + [SAMSUNG_SIP_WRITE_REG] = 0x82000021, + /* TODO: SMC call for clock driver should be added */ + } +}; + +static const struct of_device_id samsung_smc_of_match[] = { + { .compatible = "samsung,smccc-svc", .data = &svc_0_1 }, + { /* sentinel */ } +}; + +static int samsung_smc_svc_init(void) +{ + struct device_node *fw_np; + struct device_node *np; + const struct of_device_id *match; + int ret; + + if (svc_data) { + pr_info("samsung,smccc_service is already initialized\n"); + return 0; + } + + fw_np = of_find_node_by_name(NULL, "firmware"); + if (!fw_np) + return -ENODEV; + + np = of_find_matching_node_and_match(fw_np, samsung_smc_of_match, + &match); + if (!np) + return -ENODEV; + + svc_data = (struct samsung_smc_data *)match->data; + if (!svc_data) + return -ENODEV; + + ret = samsung_smc_check_version(&svc_data->version); + if (ret) { + pr_err("samsung svc version is not matched\n"); + return ret; + } + + pr_info("samsung smc svc is initialized\n"); + return 0; +} + +early_initcall(samsung_smc_svc_init); diff --git a/include/linux/firmware/samsung-smc-svc.h b/include/linux/firmware/samsung-smc-svc.h new file mode 100644 index 000000000000..9c94fd3e10a2 --- /dev/null +++ b/include/linux/firmware/samsung-smc-svc.h @@ -0,0 +1,59 @@ +/* SPDX-License-Identifier: GPL-2.0-only */ + +#ifndef __SAMSUNG_SMC_H +#define __SAMSUNG_SMC_H + +#include <linux/arm-smccc.h> + +#ifdef CONFIG_SAMSUNG_SECURE_SERVICE + +#define SAMSUNG_SIP_CALL_VAL(x) \ + ARM_SMCCC_CALL_VAL(ARM_SMCCC_FAST_CALL, \ + ARM_SMCCC_SMC_32, ARM_SMCCC_OWNER_SIP, x) + +/* Common SIP SVC number */ +#define SAMSUING_SIP_SVC_CALL_COUNT SAMSUNG_SIP_CALL_VAL(0xff00) +#define SAMSUING_SIP_SVC_UID SAMSUNG_SIP_CALL_VAL(0xff01) +#define SAMSUNG_SIP_SVC_VERSION SAMSUNG_SIP_CALL_VAL(0xff03) +#define SAMSUNG_SIP_NOT_SUPPORTED 0x0 + +enum { + /* SIP enumeration for V0.1 */ + SAMSUNG_SIP_READ_REG, + SAMSUNG_SIP_WRITE_REG, + /* Add here for later version */ + SAMSUNG_SIP_NR_SIP +}; + +int samsung_smc_call(u32 cmd_idx, + unsigned long a0, unsigned long a1, + unsigned long a2, unsigned long a3, + u32 *val); +int samsung_smc_reg_read(void *addr, unsigned int reg, u32 *val); +int samsung_smc_reg_write(void *addr, unsigned int reg, u32 val); + +#else + +static inline int samsung_smc_call(u32 cmd_idx, + unsigned long a0, unsigned long a1, + unsigned long a2, unsigned long a3, + u32 *val) +{ + return -EOPNOTSUPP; +} + +static inline int samsung_smc_reg_read(void *base, + unsigned int reg, u32 *val) +{ + return -EOPNOTSUPP; +} + +static inline int samsung_smc_reg_write(void *base, + unsigned int reg, u32 val) +{ + return -EOPNOTSUPP; +} + +#endif + +#endif /* __SAMSUNG_SMC_H */ -- 2.9.5 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH 2/4] firmware: Samsung: Add secure monitor driver 2022-07-13 4:55 ` [PATCH 2/4] firmware: Samsung: Add secure monitor driver Dongjin Yang @ 2022-07-13 4:58 ` Randy Dunlap 2022-07-13 7:11 ` Krzysztof Kozlowski ` (2 subsequent siblings) 3 siblings, 0 replies; 6+ messages in thread From: Randy Dunlap @ 2022-07-13 4:58 UTC (permalink / raw) To: dj76.yang, jesper.nilsson@axis.com, lars.persson@axis.com, robh+dt@kernel.org, krzk+dt@kernel.org, lee.jones@linaro.org Cc: javierm@redhat.com, bjorn.andersson@linaro.org, linux-arm-kernel@axis.com, devicetree@vger.kernel.org, linux-kernel@vger.kernel.org, Moon-Ki Jun, Sang Min Kim, Wangseok Lee Hi-- On 7/12/22 21:55, Dongjin Yang wrote: > diff --git a/drivers/firmware/Kconfig b/drivers/firmware/Kconfig > index e5cfb01353d8..4b0f2d033f58 100644 > --- a/drivers/firmware/Kconfig > +++ b/drivers/firmware/Kconfig > @@ -217,6 +217,17 @@ config QCOM_SCM_DOWNLOAD_MODE_DEFAULT > > Say Y here to enable "download mode" by default. > > +config SAMSUNG_SECURE_SERVICE > + bool "Samsung Foundry Secure Service Layer" > + depends on HAVE_ARM_SMCCC > + default n Drop that line, it's the default anyway. > + help > + Support secure service layer for SoCs which is manufactured by which are > + Samsung Foundry. > + > + This option provide support of secure monitor service call using > + Trusted Foundations. -- ~Randy ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 2/4] firmware: Samsung: Add secure monitor driver 2022-07-13 4:55 ` [PATCH 2/4] firmware: Samsung: Add secure monitor driver Dongjin Yang 2022-07-13 4:58 ` Randy Dunlap @ 2022-07-13 7:11 ` Krzysztof Kozlowski [not found] ` <CGME20220713045516epcms1p86b3f6a8795d767faac65eb947405f911@epcms1p4> [not found] ` <CGME20220713045516epcms1p86b3f6a8795d767faac65eb947405f911@epcms1p6> 3 siblings, 0 replies; 6+ messages in thread From: Krzysztof Kozlowski @ 2022-07-13 7:11 UTC (permalink / raw) To: dj76.yang, jesper.nilsson@axis.com, lars.persson@axis.com, robh+dt@kernel.org, krzk+dt@kernel.org, lee.jones@linaro.org Cc: javierm@redhat.com, bjorn.andersson@linaro.org, linux-arm-kernel@axis.com, devicetree@vger.kernel.org, linux-kernel@vger.kernel.org, Moon-Ki Jun, Sang Min Kim, Wangseok Lee On 13/07/2022 06:55, Dongjin Yang wrote: > Introduce a driver to provide calls into secure monitor mode. > This driver is used for SoCs produced by Samsung Foundry to provide > SMC call. This patch supports register read/write request to secure > monitor. Also, SMC call request which uses shared memory to exchange > the data between kernel and secure monitor. > > Signed-off-by: Dongjin Yang <dj76.yang@samsung.com> > --- > MAINTAINERS | 2 + > drivers/firmware/Kconfig | 11 +++ > drivers/firmware/Makefile | 1 + > drivers/firmware/samsung-smc-svc.c | 154 +++++++++++++++++++++++++++++++ > include/linux/firmware/samsung-smc-svc.h | 59 ++++++++++++ > 5 files changed, 227 insertions(+) > create mode 100644 drivers/firmware/samsung-smc-svc.c > create mode 100644 include/linux/firmware/samsung-smc-svc.h > > diff --git a/MAINTAINERS b/MAINTAINERS > index 6763746c349f..d173043ffb46 100644 > --- a/MAINTAINERS > +++ b/MAINTAINERS > @@ -1868,8 +1868,10 @@ F: arch/arm/boot/dts/artpec6* > F: arch/arm/mach-artpec > F: drivers/clk/axis > F: drivers/crypto/axis > +F: drivers/firmware/samsung-smc-svc.c > F: drivers/mmc/host/usdhi6rol0.c > F: drivers/pinctrl/pinctrl-artpec* > +F: include/linux/firmware/samsung-smc-svc.h Same as for bindings - not related to Artpec platforms. Either add dedicated entry or put it under Samsung SoC. > > ARM/ASPEED I2C DRIVER > M: Brendan Higgins <brendanhiggins@google.com> > diff --git a/drivers/firmware/Kconfig b/drivers/firmware/Kconfig > index e5cfb01353d8..4b0f2d033f58 100644 > --- a/drivers/firmware/Kconfig > +++ b/drivers/firmware/Kconfig > @@ -217,6 +217,17 @@ config QCOM_SCM_DOWNLOAD_MODE_DEFAULT > > Say Y here to enable "download mode" by default. > > +config SAMSUNG_SECURE_SERVICE > + bool "Samsung Foundry Secure Service Layer" > + depends on HAVE_ARM_SMCCC > + default n > + help > + Support secure service layer for SoCs which is manufactured by > + Samsung Foundry. > + > + This option provide support of secure monitor service call using > + Trusted Foundations. > + > config SYSFB > bool > default y > diff --git a/drivers/firmware/Makefile b/drivers/firmware/Makefile > index 4e58cb474a68..985e30a9665f 100644 > --- a/drivers/firmware/Makefile > +++ b/drivers/firmware/Makefile > @@ -19,6 +19,7 @@ obj-$(CONFIG_RASPBERRYPI_FIRMWARE) += raspberrypi.o > obj-$(CONFIG_FW_CFG_SYSFS) += qemu_fw_cfg.o > obj-$(CONFIG_QCOM_SCM) += qcom-scm.o > qcom-scm-objs += qcom_scm.o qcom_scm-smc.o qcom_scm-legacy.o > +obj-$(CONFIG_SAMSUNG_SECURE_SERVICE) += samsung-smc-svc.o > obj-$(CONFIG_SYSFB) += sysfb.o > obj-$(CONFIG_SYSFB_SIMPLEFB) += sysfb_simplefb.o > obj-$(CONFIG_TI_SCI_PROTOCOL) += ti_sci.o > diff --git a/drivers/firmware/samsung-smc-svc.c b/drivers/firmware/samsung-smc-svc.c > new file mode 100644 > index 000000000000..eb3a5285cf2b > --- /dev/null > +++ b/drivers/firmware/samsung-smc-svc.c > @@ -0,0 +1,154 @@ > +// SPDX-License-Identifier: GPL-2.0 > +/* > + * Copyright (c) 2019 Samsung Electronics Co., Ltd. > + * Author: Kihyun Yoon<kihyun.yoon@samsung.com> > + * Author: Dongjin Yang<dj76.yang@samsung.com> > + * > + * 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. > + * Drop boiler plate with trailing empty '*' comment. > + */ > + > +#include <linux/types.h> > +#include <linux/errno.h> > +#include <linux/module.h> > +#include <linux/io.h> > +#include <linux/of.h> > +#include <linux/of_device.h> > +#include <linux/platform_device.h> > +#include <linux/firmware/samsung-smc-svc.h> Order the headers by name. > + > +struct samsung_smc_version { > + u32 major; > + u32 minor; > +}; > + > +struct samsung_smc_data { > + struct samsung_smc_version version; > + u32 svc_cmd_list[SAMSUNG_SIP_NR_SIP]; > +}; > + > +struct samsung_smc_data *svc_data; No, no globals. Even no static variables. > + > +static int samsung_smc_svc_init(void); > + > +static int __samsung_get_svc_cmd(u32 cmd_idx) > +{ > + if (cmd_idx >= SAMSUNG_SIP_NR_SIP) > + return SAMSUNG_SIP_NOT_SUPPORTED; > + > + /* Initialize this driver if it is not ready */ > + if (!svc_data && samsung_smc_svc_init()) { > + pr_err("samsung,smccc_service initialization is failed\n"); > + return SAMSUNG_SIP_NOT_SUPPORTED; > + } > + > + return svc_data->svc_cmd_list[cmd_idx]; > +} > + > +int samsung_smc_call(u32 cmd_idx, > + unsigned long a0, unsigned long a1, > + unsigned long a2, unsigned long a3, > + u32 *val) > + > +{ > + struct arm_smccc_res res; > + u32 cmd = __samsung_get_svc_cmd(cmd_idx); > + > + if (cmd == SAMSUNG_SIP_NOT_SUPPORTED) > + return -EOPNOTSUPP; > + > + arm_smccc_smc(cmd, a0, a1, a2, a3, 0, 0, 0, &res); > + if (res.a0 == SMCCC_RET_NOT_SUPPORTED) > + return -EINVAL; > + > + if (val) > + *val = (u32)res.a1; > + > + return 0; > +} > +EXPORT_SYMBOL(samsung_smc_call); EXPORT_SYMBOL_GPL You need kerneldoc for all these. > + > +int samsung_smc_reg_read(void *base, u32 reg, u32 *val) > +{ > + return samsung_smc_call(SAMSUNG_SIP_READ_REG, > + (unsigned long)base, reg, 0, 0, val); > +} > +EXPORT_SYMBOL(samsung_smc_reg_read); > + > +int samsung_smc_reg_write(void *base, u32 reg, u32 val) > +{ > + return samsung_smc_call(SAMSUNG_SIP_WRITE_REG, > + (unsigned long)base, reg, val, 0, NULL); > +} > +EXPORT_SYMBOL(samsung_smc_reg_write); > + > +static int samsung_smc_check_version(struct samsung_smc_version *ver) > +{ > + struct arm_smccc_res res; > + > + arm_smccc_smc(SAMSUNG_SIP_SVC_VERSION, 0, 0, 0, 0, 0, 0, 0, &res); > + > + if ((u32)res.a0 != ver->major || (u32)res.a1 != ver->minor) > + return -EINVAL; > + > + pr_info("Samsung SVC version [%d.%d] is detected\n", > + ver->major, ver->minor); > + > + return 0; > +} > + > +static const struct samsung_smc_data svc_0_1 = { > + .version = { > + .major = 0, > + .minor = 1 > + }, > + .svc_cmd_list = { > + [SAMSUNG_SIP_READ_REG] = 0x82000020, > + [SAMSUNG_SIP_WRITE_REG] = 0x82000021, > + /* TODO: SMC call for clock driver should be added */ > + } > +}; > + > +static const struct of_device_id samsung_smc_of_match[] = { > + { .compatible = "samsung,smccc-svc", .data = &svc_0_1 }, > + { /* sentinel */ } > +}; > + > +static int samsung_smc_svc_init(void) > +{ > + struct device_node *fw_np; > + struct device_node *np; > + const struct of_device_id *match; > + int ret; > + > + if (svc_data) { > + pr_info("samsung,smccc_service is already initialized\n"); > + return 0; > + } > + > + fw_np = of_find_node_by_name(NULL, "firmware"); > + if (!fw_np) > + return -ENODEV; > + > + np = of_find_matching_node_and_match(fw_np, samsung_smc_of_match, > + &match); > + if (!np) > + return -ENODEV; > + > + svc_data = (struct samsung_smc_data *)match->data; > + if (!svc_data) > + return -ENODEV; > + > + ret = samsung_smc_check_version(&svc_data->version); > + if (ret) { > + pr_err("samsung svc version is not matched\n"); > + return ret; > + } > + > + pr_info("samsung smc svc is initialized\n"); No useless success printks. We have tracing and sysfs for that. > + return 0; > +} > + > +early_initcall(samsung_smc_svc_init); No. Make it a platform driver. > diff --git a/include/linux/firmware/samsung-smc-svc.h b/include/linux/firmware/samsung-smc-svc.h > new file mode 100644 > index 000000000000..9c94fd3e10a2 > --- /dev/null > +++ b/include/linux/firmware/samsung-smc-svc.h > @@ -0,0 +1,59 @@ > +/* SPDX-License-Identifier: GPL-2.0-only */ > + > +#ifndef __SAMSUNG_SMC_H > +#define __SAMSUNG_SMC_H Extend the header gaurd to match path. > + > +#include <linux/arm-smccc.h> > + > +#ifdef CONFIG_SAMSUNG_SECURE_SERVICE No ifdefs in headers. This is not a subsystem. > + > +#define SAMSUNG_SIP_CALL_VAL(x) \ > + ARM_SMCCC_CALL_VAL(ARM_SMCCC_FAST_CALL, \ > + ARM_SMCCC_SMC_32, ARM_SMCCC_OWNER_SIP, x) Need kerneldoc. > + > +/* Common SIP SVC number */ > +#define SAMSUING_SIP_SVC_CALL_COUNT SAMSUNG_SIP_CALL_VAL(0xff00) > +#define SAMSUING_SIP_SVC_UID SAMSUNG_SIP_CALL_VAL(0xff01) > +#define SAMSUNG_SIP_SVC_VERSION SAMSUNG_SIP_CALL_VAL(0xff03) > +#define SAMSUNG_SIP_NOT_SUPPORTED 0x0 Need kerneldoc. > + > +enum { > + /* SIP enumeration for V0.1 */ > + SAMSUNG_SIP_READ_REG, > + SAMSUNG_SIP_WRITE_REG, > + /* Add here for later version */ > + SAMSUNG_SIP_NR_SIP > +}; Need kerneldoc. > + > +int samsung_smc_call(u32 cmd_idx, > + unsigned long a0, unsigned long a1, > + unsigned long a2, unsigned long a3, > + u32 *val); > +int samsung_smc_reg_read(void *addr, unsigned int reg, u32 *val); > +int samsung_smc_reg_write(void *addr, unsigned int reg, u32 val); > + Need kerneldoc in the driver. > +#else > + No stubs, no ifdefs. It's not a subsystem. Best regards, Krzysztof ^ permalink raw reply [flat|nested] 6+ messages in thread
[parent not found: <CGME20220713045516epcms1p86b3f6a8795d767faac65eb947405f911@epcms1p4>]
* Re: [PATCH 2/4] firmware: Samsung: Add secure monitor driver [not found] ` <CGME20220713045516epcms1p86b3f6a8795d767faac65eb947405f911@epcms1p4> @ 2022-07-15 2:02 ` Dongjin Yang 0 siblings, 0 replies; 6+ messages in thread From: Dongjin Yang @ 2022-07-15 2:02 UTC (permalink / raw) To: Randy Dunlap, jesper.nilsson@axis.com, lars.persson@axis.com, robh+dt@kernel.org, krzk+dt@kernel.org, lee.jones@linaro.org Cc: javierm@redhat.com, bjorn.andersson@linaro.org, linux-arm-kernel@axis.com, devicetree@vger.kernel.org, linux-kernel@vger.kernel.org, Moon-Ki Jun, Sang Min Kim, Wangseok Lee On 7/13/22 04:58, Randy Dunlap wrote: > Hi-- > > On 7/12/22 21:55, Dongjin Yang wrote: > > diff --git a/drivers/firmware/Kconfig b/drivers/firmware/Kconfig > > index e5cfb01353d8..4b0f2d033f58 100644 > > --- a/drivers/firmware/Kconfig > > +++ b/drivers/firmware/Kconfig > > @@ -217,6 +217,17 @@ config QCOM_SCM_DOWNLOAD_MODE_DEFAULT > > > > Say Y here to enable "download mode" by default. > > > > +config SAMSUNG_SECURE_SERVICE > > + bool "Samsung Foundry Secure Service Layer" > > + depends on HAVE_ARM_SMCCC > > + default n > > Drop that line, it's the default anyway. Sure thanks. > > > + help > > + Support secure service layer for SoCs which is manufactured by > > which are Thanks for correcting. > > > + Samsung Foundry. > > + > > + This option provide support of secure monitor service call using > > + Trusted Foundations. > > -- > ~Randy ^ permalink raw reply [flat|nested] 6+ messages in thread
[parent not found: <CGME20220713045516epcms1p86b3f6a8795d767faac65eb947405f911@epcms1p6>]
* Re: [PATCH 2/4] firmware: Samsung: Add secure monitor driver [not found] ` <CGME20220713045516epcms1p86b3f6a8795d767faac65eb947405f911@epcms1p6> @ 2022-07-26 0:04 ` Dongjin Yang 2022-07-26 6:47 ` Krzysztof Kozlowski 0 siblings, 1 reply; 6+ messages in thread From: Dongjin Yang @ 2022-07-26 0:04 UTC (permalink / raw) To: Krzysztof Kozlowski, jesper.nilsson@axis.com, lars.persson@axis.com, robh+dt@kernel.org, krzk+dt@kernel.org, lee.jones@linaro.org Cc: javierm@redhat.com, bjorn.andersson@linaro.org, linux-arm-kernel@axis.com, devicetree@vger.kernel.org, linux-kernel@vger.kernel.org, Moon-Ki Jun, Sang Min Kim, Wangseok Lee On 14/07/2022 04:28, Krzysztof Kozlowski wrote: > On 13/07/2022 06:55, Dongjin Yang wrote: > > Introduce a driver to provide calls into secure monitor mode. > > This driver is used for SoCs produced by Samsung Foundry to provide > > SMC call. This patch supports register read/write request to secure > > monitor. Also, SMC call request which uses shared memory to exchange > > the data between kernel and secure monitor. > > > > Signed-off-by: Dongjin Yang <dj76.yang@samsung.com> > > --- > > MAINTAINERS | 2 + > > drivers/firmware/Kconfig | 11 +++ > > drivers/firmware/Makefile | 1 + > > drivers/firmware/samsung-smc-svc.c | 154 +++++++++++++++++++++++++++++++ > > include/linux/firmware/samsung-smc-svc.h | 59 ++++++++++++ > > 5 files changed, 227 insertions(+) > > create mode 100644 drivers/firmware/samsung-smc-svc.c > > create mode 100644 include/linux/firmware/samsung-smc-svc.h > > > > diff --git a/MAINTAINERS b/MAINTAINERS > > index 6763746c349f..d173043ffb46 100644 > > --- a/MAINTAINERS > > +++ b/MAINTAINERS > > @@ -1868,8 +1868,10 @@ F: arch/arm/boot/dts/artpec6* > > F: arch/arm/mach-artpec > > F: drivers/clk/axis > > F: drivers/crypto/axis > > +F: drivers/firmware/samsung-smc-svc.c > > F: drivers/mmc/host/usdhi6rol0.c > > F: drivers/pinctrl/pinctrl-artpec* > > +F: include/linux/firmware/samsung-smc-svc.h > > Same as for bindings - not related to Artpec platforms. Either add > dedicated entry or put it under Samsung SoC. > This patch is for Artpec8 SoC. > > > > ARM/ASPEED I2C DRIVER > > M: Brendan Higgins <brendanhiggins@google.com> > > diff --git a/drivers/firmware/Kconfig b/drivers/firmware/Kconfig > > index e5cfb01353d8..4b0f2d033f58 100644 > > --- a/drivers/firmware/Kconfig > > +++ b/drivers/firmware/Kconfig > > @@ -217,6 +217,17 @@ config QCOM_SCM_DOWNLOAD_MODE_DEFAULT > > > > Say Y here to enable "download mode" by default. > > > > +config SAMSUNG_SECURE_SERVICE > > + bool "Samsung Foundry Secure Service Layer" > > + depends on HAVE_ARM_SMCCC > > + default n > > + help > > + Support secure service layer for SoCs which is manufactured by > > + Samsung Foundry. > > + > > + This option provide support of secure monitor service call using > > + Trusted Foundations. > > + > > config SYSFB > > bool > > default y > > diff --git a/drivers/firmware/Makefile b/drivers/firmware/Makefile > > index 4e58cb474a68..985e30a9665f 100644 > > --- a/drivers/firmware/Makefile > > +++ b/drivers/firmware/Makefile > > @@ -19,6 +19,7 @@ obj-$(CONFIG_RASPBERRYPI_FIRMWARE) += raspberrypi.o > > obj-$(CONFIG_FW_CFG_SYSFS) += qemu_fw_cfg.o > > obj-$(CONFIG_QCOM_SCM) += qcom-scm.o > > qcom-scm-objs += qcom_scm.o qcom_scm-smc.o qcom_scm-legacy.o > > +obj-$(CONFIG_SAMSUNG_SECURE_SERVICE) += samsung-smc-svc.o > > obj-$(CONFIG_SYSFB) += sysfb.o > > obj-$(CONFIG_SYSFB_SIMPLEFB) += sysfb_simplefb.o > > obj-$(CONFIG_TI_SCI_PROTOCOL) += ti_sci.o > > diff --git a/drivers/firmware/samsung-smc-svc.c b/drivers/firmware/samsung-smc-svc.c > > new file mode 100644 > > index 000000000000..eb3a5285cf2b > > --- /dev/null > > +++ b/drivers/firmware/samsung-smc-svc.c > > @@ -0,0 +1,154 @@ > > +// SPDX-License-Identifier: GPL-2.0 > > +/* > > + * Copyright (c) 2019 Samsung Electronics Co., Ltd. > > + * Author: Kihyun Yoon<kihyun.yoon@samsung.com> > > + * Author: Dongjin Yang<dj76.yang@samsung.com> > > + * > > + * 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. > > + * > > Drop boiler plate with trailing empty '*' comment. > Yes, > > + */ > > + > > +#include <linux/types.h> > > +#include <linux/errno.h> > > +#include <linux/module.h> > > +#include <linux/io.h> > > +#include <linux/of.h> > > +#include <linux/of_device.h> > > +#include <linux/platform_device.h> > > +#include <linux/firmware/samsung-smc-svc.h> > > Order the headers by name. > Yes I will, > > + > > +struct samsung_smc_version { > > + u32 major; > > + u32 minor; > > +}; > > + > > +struct samsung_smc_data { > > + struct samsung_smc_version version; > > + u32 svc_cmd_list[SAMSUNG_SIP_NR_SIP]; > > +}; > > + > > +struct samsung_smc_data *svc_data; > > No, no globals. Even no static variables. > Yes, let me change the driver > > + > > +static int samsung_smc_svc_init(void); > > + > > +static int __samsung_get_svc_cmd(u32 cmd_idx) > > +{ > > + if (cmd_idx >= SAMSUNG_SIP_NR_SIP) > > + return SAMSUNG_SIP_NOT_SUPPORTED; > > + > > + /* Initialize this driver if it is not ready */ > > + if (!svc_data && samsung_smc_svc_init()) { > > + pr_err("samsung,smccc_service initialization is failed\n"); > > + return SAMSUNG_SIP_NOT_SUPPORTED; > > + } > > + > > + return svc_data->svc_cmd_list[cmd_idx]; > > +} > > + > > +int samsung_smc_call(u32 cmd_idx, > > + unsigned long a0, unsigned long a1, > > + unsigned long a2, unsigned long a3, > > + u32 *val) > > + > > +{ > > + struct arm_smccc_res res; > > + u32 cmd = __samsung_get_svc_cmd(cmd_idx); > > + > > + if (cmd == SAMSUNG_SIP_NOT_SUPPORTED) > > + return -EOPNOTSUPP; > > + > > + arm_smccc_smc(cmd, a0, a1, a2, a3, 0, 0, 0, &res); > > + if (res.a0 == SMCCC_RET_NOT_SUPPORTED) > > + return -EINVAL; > > + > > + if (val) > > + *val = (u32)res.a1; > > + > > + return 0; > > +} > > +EXPORT_SYMBOL(samsung_smc_call); > > EXPORT_SYMBOL_GPL > > You need kerneldoc for all these. > Yes, > > + > > +int samsung_smc_reg_read(void *base, u32 reg, u32 *val) > > +{ > > + return samsung_smc_call(SAMSUNG_SIP_READ_REG, > > + (unsigned long)base, reg, 0, 0, val); > > +} > > +EXPORT_SYMBOL(samsung_smc_reg_read); > > + > > +int samsung_smc_reg_write(void *base, u32 reg, u32 val) > > +{ > > + return samsung_smc_call(SAMSUNG_SIP_WRITE_REG, > > + (unsigned long)base, reg, val, 0, NULL); > > +} > > +EXPORT_SYMBOL(samsung_smc_reg_write); > > + > > +static int samsung_smc_check_version(struct samsung_smc_version *ver) > > +{ > > + struct arm_smccc_res res; > > + > > + arm_smccc_smc(SAMSUNG_SIP_SVC_VERSION, 0, 0, 0, 0, 0, 0, 0, &res); > > + > > + if ((u32)res.a0 != ver->major || (u32)res.a1 != ver->minor) > > + return -EINVAL; > > + > > + pr_info("Samsung SVC version [%d.%d] is detected\n", > > + ver->major, ver->minor); > > + > > + return 0; > > +} > > + > > +static const struct samsung_smc_data svc_0_1 = { > > + .version = { > > + .major = 0, > > + .minor = 1 > > + }, > > + .svc_cmd_list = { > > + [SAMSUNG_SIP_READ_REG] = 0x82000020, > > + [SAMSUNG_SIP_WRITE_REG] = 0x82000021, > > + /* TODO: SMC call for clock driver should be added */ > > + } > > +}; > > + > > +static const struct of_device_id samsung_smc_of_match[] = { > > + { .compatible = "samsung,smccc-svc", .data = &svc_0_1 }, > > + { /* sentinel */ } > > +}; > > + > > +static int samsung_smc_svc_init(void) > > +{ > > + struct device_node *fw_np; > > + struct device_node *np; > > + const struct of_device_id *match; > > + int ret; > > + > > + if (svc_data) { > > + pr_info("samsung,smccc_service is already initialized\n"); > > + return 0; > > + } > > + > > + fw_np = of_find_node_by_name(NULL, "firmware"); > > + if (!fw_np) > > + return -ENODEV; > > + > > + np = of_find_matching_node_and_match(fw_np, samsung_smc_of_match, > > + &match); > > + if (!np) > > + return -ENODEV; > > + > > + svc_data = (struct samsung_smc_data *)match->data; > > + if (!svc_data) > > + return -ENODEV; > > + > > + ret = samsung_smc_check_version(&svc_data->version); > > + if (ret) { > > + pr_err("samsung svc version is not matched\n"); > > + return ret; > > + } > > + > > + pr_info("samsung smc svc is initialized\n"); > > No useless success printks. We have tracing and sysfs for that. > Ok, > > + return 0; > > +} > > + > > +early_initcall(samsung_smc_svc_init); > > No. Make it a platform driver. > Ok, I will change, > > diff --git a/include/linux/firmware/samsung-smc-svc.h b/include/linux/firmware/samsung-smc-svc.h > > new file mode 100644 > > index 000000000000..9c94fd3e10a2 > > --- /dev/null > > +++ b/include/linux/firmware/samsung-smc-svc.h > > @@ -0,0 +1,59 @@ > > +/* SPDX-License-Identifier: GPL-2.0-only */ > > + > > +#ifndef __SAMSUNG_SMC_H > > +#define __SAMSUNG_SMC_H > > Extend the header gaurd to match path. > Yes, > > + > > +#include <linux/arm-smccc.h> > > + > > +#ifdef CONFIG_SAMSUNG_SECURE_SERVICE > > No ifdefs in headers. This is not a subsystem. > Yes, I will remove, > > + > > +#define SAMSUNG_SIP_CALL_VAL(x) \ > > + ARM_SMCCC_CALL_VAL(ARM_SMCCC_FAST_CALL, \ > > + ARM_SMCCC_SMC_32, ARM_SMCCC_OWNER_SIP, x) > > Need kerneldoc. > Yes, I will add kerneldoc. > > + > > +/* Common SIP SVC number */ > > +#define SAMSUING_SIP_SVC_CALL_COUNT SAMSUNG_SIP_CALL_VAL(0xff00) > > +#define SAMSUING_SIP_SVC_UID SAMSUNG_SIP_CALL_VAL(0xff01) > > +#define SAMSUNG_SIP_SVC_VERSION SAMSUNG_SIP_CALL_VAL(0xff03) > > +#define SAMSUNG_SIP_NOT_SUPPORTED 0x0 > > Need kerneldoc. > > > > + > > +enum { > > + /* SIP enumeration for V0.1 */ > > + SAMSUNG_SIP_READ_REG, > > + SAMSUNG_SIP_WRITE_REG, > > + /* Add here for later version */ > > + SAMSUNG_SIP_NR_SIP > > +}; > > Need kerneldoc. > > > > + > > +int samsung_smc_call(u32 cmd_idx, > > + unsigned long a0, unsigned long a1, > > + unsigned long a2, unsigned long a3, > > + u32 *val); > > +int samsung_smc_reg_read(void *addr, unsigned int reg, u32 *val); > > +int samsung_smc_reg_write(void *addr, unsigned int reg, u32 val); > > + > > Need kerneldoc in the driver. > > > > +#else > > + > > No stubs, no ifdefs. It's not a subsystem. > > > > Best regards, > Krzysztof ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 2/4] firmware: Samsung: Add secure monitor driver 2022-07-26 0:04 ` Dongjin Yang @ 2022-07-26 6:47 ` Krzysztof Kozlowski 0 siblings, 0 replies; 6+ messages in thread From: Krzysztof Kozlowski @ 2022-07-26 6:47 UTC (permalink / raw) To: dj76.yang, jesper.nilsson@axis.com, lars.persson@axis.com, robh+dt@kernel.org, krzk+dt@kernel.org, lee.jones@linaro.org Cc: javierm@redhat.com, bjorn.andersson@linaro.org, linux-arm-kernel@axis.com, devicetree@vger.kernel.org, linux-kernel@vger.kernel.org, Moon-Ki Jun, Sang Min Kim, Wangseok Lee On 26/07/2022 02:04, Dongjin Yang wrote: > > On 14/07/2022 04:28, Krzysztof Kozlowski wrote: >> On 13/07/2022 06:55, Dongjin Yang wrote: >>> Introduce a driver to provide calls into secure monitor mode. >>> This driver is used for SoCs produced by Samsung Foundry to provide >>> SMC call. This patch supports register read/write request to secure >>> monitor. Also, SMC call request which uses shared memory to exchange >>> the data between kernel and secure monitor. >>> >>> Signed-off-by: Dongjin Yang <dj76.yang@samsung.com> >>> --- >>> MAINTAINERS | 2 + >>> drivers/firmware/Kconfig | 11 +++ >>> drivers/firmware/Makefile | 1 + >>> drivers/firmware/samsung-smc-svc.c | 154 +++++++++++++++++++++++++++++++ >>> include/linux/firmware/samsung-smc-svc.h | 59 ++++++++++++ >>> 5 files changed, 227 insertions(+) >>> create mode 100644 drivers/firmware/samsung-smc-svc.c >>> create mode 100644 include/linux/firmware/samsung-smc-svc.h >>> >>> diff --git a/MAINTAINERS b/MAINTAINERS >>> index 6763746c349f..d173043ffb46 100644 >>> --- a/MAINTAINERS >>> +++ b/MAINTAINERS >>> @@ -1868,8 +1868,10 @@ F: arch/arm/boot/dts/artpec6* >>> F: arch/arm/mach-artpec >>> F: drivers/clk/axis >>> F: drivers/crypto/axis >>> +F: drivers/firmware/samsung-smc-svc.c >>> F: drivers/mmc/host/usdhi6rol0.c >>> F: drivers/pinctrl/pinctrl-artpec* >>> +F: include/linux/firmware/samsung-smc-svc.h >> >> Same as for bindings - not related to Artpec platforms. Either add >> dedicated entry or put it under Samsung SoC. >> > > This patch is for Artpec8 SoC. This does not answer my comment at all. > Best regards, Krzysztof ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2022-07-26 6:47 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <CGME20220713045516epcms1p86b3f6a8795d767faac65eb947405f911@epcms1p8>
2022-07-13 4:55 ` [PATCH 2/4] firmware: Samsung: Add secure monitor driver Dongjin Yang
2022-07-13 4:58 ` Randy Dunlap
2022-07-13 7:11 ` Krzysztof Kozlowski
[not found] ` <CGME20220713045516epcms1p86b3f6a8795d767faac65eb947405f911@epcms1p4>
2022-07-15 2:02 ` Dongjin Yang
[not found] ` <CGME20220713045516epcms1p86b3f6a8795d767faac65eb947405f911@epcms1p6>
2022-07-26 0:04 ` Dongjin Yang
2022-07-26 6:47 ` Krzysztof Kozlowski
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).