| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <6ec0ca8d-85c7-53d6-acf2-22c4ac13e805@codeaurora.org>
Date: Thu, 1 Apr 2021 18:12:14 -0700
From: Elliot Berman <eberman@...eaurora.org>
To: Stephen Boyd <swboyd@...omium.org>, Andy Gross <agross@...nel.org>,
Bjorn Andersson <bjorn.andersson@...aro.org>
Cc: linux-kernel@...r.kernel.org, linux-arm-msm@...r.kernel.org,
Brian Masney <masneyb@...tation.org>,
Stephan Gerhold <stephan@...hold.net>,
Jeffrey Hugo <jhugo@...eaurora.org>,
Douglas Anderson <dianders@...omium.org>
Subject: Re: [PATCH v2] firmware: qcom_scm: Only compile legacy calls on ARM
On 3/23/2021 3:43 PM, Stephen Boyd wrote:
> These scm calls are never used outside of legacy ARMv7 based platforms.
> That's because PSCI, mandated on arm64, implements them for modern SoCs
> via the PSCI spec. Let's move them to the legacy file and only compile
> the legacy file into the kernel when CONFIG_ARM=y. Otherwise provide
> stubs and fail the calls. This saves a little bit of space in an
> arm64 allmodconfig.
>
> $ ./scripts/bloat-o-meter vmlinux.before vmlinux.after
> add/remove: 0/8 grow/shrink: 5/6 up/down: 509/-4401 (-3892)
> Function old new delta
> __qcom_scm_set_dload_mode.constprop 312 452 +140
> qcom_scm_qsmmu500_wait_safe_toggle 288 416 +128
> qcom_scm_io_writel 288 408 +120
> qcom_scm_io_readl 376 492 +116
> __param_str_download_mode 23 28 +5
> __warned 4327 4326 -1
> e843419@...f_00010432_324 8 - -8
> qcom_scm_call 228 208 -20
> CSWTCH 5925 5877 -48
> _sub_I_65535_1 163100 163040 -60
> _sub_D_65535_0 163100 163040 -60
> qcom_scm_wb 64 - -64
> qcom_scm_lock 320 160 -160
> qcom_scm_call_atomic 212 - -212
> qcom_scm_cpu_power_down 308 - -308
> scm_legacy_call_atomic 520 - -520
> qcom_scm_set_warm_boot_addr 720 - -720
> qcom_scm_set_cold_boot_addr 728 - -728
> scm_legacy_call 1492 - -1492
> Total: Before=66737606, After=66733714, chg -0.01%
>
> Commit 9a434cee773a ("firmware: qcom_scm: Dynamically support SMCCC and
> legacy conventions") didn't mention any motivating factors for keeping
> the legacy code around on arm64 kernels, i.e. presumably that commit
> wasn't trying to support these legacy APIs on arm64 kernels.
>
> Cc: Elliot Berman <eberman@...eaurora.org>
> Cc: Brian Masney <masneyb@...tation.org>
> Cc: Stephan Gerhold <stephan@...hold.net>
> Cc: Jeffrey Hugo <jhugo@...eaurora.org>
> Cc: Douglas Anderson <dianders@...omium.org>
> Signed-off-by: Stephen Boyd <swboyd@...omium.org>
> ---
It might be a good idea to wrap these lines from qcom_scm_call with #if
IS_ENABLED(CONFIG_ARM), and the corresponding ones in qcom_scm_call_atomic:
case SMC_CONVENTION_LEGACY:
return scm_legacy_call(dev, desc, res);
If something is wrong with loaded firmware and LEGACY convention is
incorrectly selected, you would get a better hint about the problem:
"Unknown current SCM calling convention." You would still get the hint
earlier from __get_convention, but that may not be obvious to someone
unfamiliar with the SCM driver.
I'll defer to your/Bjorn's preference:
Acked-by: Elliot Berman <eberman@...eaurora.org>
with or without modifying qcom_scm_call{_atomic}.
>
> Followup to v1 (https://lore.kernel.org/r/20210223214539.1336155-7-swboyd@chromium.org):
> * Don't change the legacy file to use legacy calls only
> * Wrap more things in CONFIG_ARM checks
>
> drivers/firmware/Makefile | 4 +++-
> drivers/firmware/qcom_scm.c | 47 ++++++++++++++++++++-----------------
> drivers/firmware/qcom_scm.h | 15 ++++++++++++
> include/linux/qcom_scm.h | 21 ++++++++++-------
> 4 files changed, 56 insertions(+), 31 deletions(-)
>
> diff --git a/drivers/firmware/Makefile b/drivers/firmware/Makefile
> index 5e013b6a3692..0b7b35555a6c 100644
> --- a/drivers/firmware/Makefile
> +++ b/drivers/firmware/Makefile
> @@ -17,7 +17,9 @@ obj-$(CONFIG_ISCSI_IBFT) += iscsi_ibft.o
> obj-$(CONFIG_FIRMWARE_MEMMAP) += memmap.o
> obj-$(CONFIG_RASPBERRYPI_FIRMWARE) += raspberrypi.o
> obj-$(CONFIG_FW_CFG_SYSFS) += qemu_fw_cfg.o
> -obj-$(CONFIG_QCOM_SCM) += qcom_scm.o qcom_scm-smc.o qcom_scm-legacy.o
> +obj-$(CONFIG_QCOM_SCM) += qcom_scm_objs.o
> +qcom_scm_objs-$(CONFIG_ARM) += qcom_scm-legacy.o
> +qcom_scm_objs-$(CONFIG_QCOM_SCM) += qcom_scm.o qcom_scm-smc.o
> obj-$(CONFIG_TI_SCI_PROTOCOL) += ti_sci.o
> obj-$(CONFIG_TRUSTED_FOUNDATIONS) += trusted_foundations.o
> obj-$(CONFIG_TURRIS_MOX_RWTM) += turris-mox-rwtm.o
> diff --git a/drivers/firmware/qcom_scm.c b/drivers/firmware/qcom_scm.c
> index ee9cb545e73b..747808a8ddf4 100644
> --- a/drivers/firmware/qcom_scm.c
> +++ b/drivers/firmware/qcom_scm.c
> @@ -49,28 +49,6 @@ struct qcom_scm_mem_map_info {
> __le64 mem_size;
> };
>
> -#define QCOM_SCM_FLAG_COLDBOOT_CPU0 0x00
> -#define QCOM_SCM_FLAG_COLDBOOT_CPU1 0x01
> -#define QCOM_SCM_FLAG_COLDBOOT_CPU2 0x08
> -#define QCOM_SCM_FLAG_COLDBOOT_CPU3 0x20
> -
> -#define QCOM_SCM_FLAG_WARMBOOT_CPU0 0x04
> -#define QCOM_SCM_FLAG_WARMBOOT_CPU1 0x02
> -#define QCOM_SCM_FLAG_WARMBOOT_CPU2 0x10
> -#define QCOM_SCM_FLAG_WARMBOOT_CPU3 0x40
> -
> -struct qcom_scm_wb_entry {
> - int flag;
> - void *entry;
> -};
> -
> -static struct qcom_scm_wb_entry qcom_scm_wb[] = {
> - { .flag = QCOM_SCM_FLAG_WARMBOOT_CPU0 },
> - { .flag = QCOM_SCM_FLAG_WARMBOOT_CPU1 },
> - { .flag = QCOM_SCM_FLAG_WARMBOOT_CPU2 },
> - { .flag = QCOM_SCM_FLAG_WARMBOOT_CPU3 },
> -};
> -
> static const char *qcom_scm_convention_names[] = {
> [SMC_CONVENTION_UNKNOWN] = "unknown",
> [SMC_CONVENTION_ARM_32] = "smc arm 32",
> @@ -260,6 +238,30 @@ static bool __qcom_scm_is_call_available(struct device *dev, u32 svc_id,
> return ret ? false : !!res.result[0];
> }
>
> +#if IS_ENABLED(CONFIG_ARM)
> +
> +#define QCOM_SCM_FLAG_COLDBOOT_CPU0 0x00
> +#define QCOM_SCM_FLAG_COLDBOOT_CPU1 0x01
> +#define QCOM_SCM_FLAG_COLDBOOT_CPU2 0x08
> +#define QCOM_SCM_FLAG_COLDBOOT_CPU3 0x20
> +
> +#define QCOM_SCM_FLAG_WARMBOOT_CPU0 0x04
> +#define QCOM_SCM_FLAG_WARMBOOT_CPU1 0x02
> +#define QCOM_SCM_FLAG_WARMBOOT_CPU2 0x10
> +#define QCOM_SCM_FLAG_WARMBOOT_CPU3 0x40
> +
> +struct qcom_scm_wb_entry {
> + int flag;
> + void *entry;
> +};
> +
> +static struct qcom_scm_wb_entry qcom_scm_wb[] = {
> + { .flag = QCOM_SCM_FLAG_WARMBOOT_CPU0 },
> + { .flag = QCOM_SCM_FLAG_WARMBOOT_CPU1 },
> + { .flag = QCOM_SCM_FLAG_WARMBOOT_CPU2 },
> + { .flag = QCOM_SCM_FLAG_WARMBOOT_CPU3 },
> +};
> +
> /**
> * qcom_scm_set_warm_boot_addr() - Set the warm boot address for cpus
> * @entry: Entry point function for the cpus
> @@ -369,6 +371,7 @@ void qcom_scm_cpu_power_down(u32 flags)
> qcom_scm_call_atomic(__scm ? __scm->dev : NULL, &desc, NULL);
> }
> EXPORT_SYMBOL(qcom_scm_cpu_power_down);
> +#endif
>
> int qcom_scm_set_remote_state(u32 state, u32 id)
> {
> diff --git a/drivers/firmware/qcom_scm.h b/drivers/firmware/qcom_scm.h
> index 632fe3142462..735e975320e4 100644
> --- a/drivers/firmware/qcom_scm.h
> +++ b/drivers/firmware/qcom_scm.h
> @@ -68,11 +68,26 @@ extern int __scm_smc_call(struct device *dev, const struct qcom_scm_desc *desc,
> __scm_smc_call((dev), (desc), qcom_scm_convention, (res), (atomic))
>
> #define SCM_LEGACY_FNID(s, c) (((s) << 10) | ((c) & 0x3ff))
> +#if IS_ENABLED(CONFIG_ARM)
> extern int scm_legacy_call_atomic(struct device *dev,
> const struct qcom_scm_desc *desc,
> struct qcom_scm_res *res);
> extern int scm_legacy_call(struct device *dev, const struct qcom_scm_desc *desc,
> struct qcom_scm_res *res);
> +#else
> +static inline int scm_legacy_call_atomic(struct device *dev,
> + const struct qcom_scm_desc *desc,
> + struct qcom_scm_res *res)
> +{
> + return -EINVAL;
> +}
> +
> +static inline int scm_legacy_call(struct device *dev, const struct qcom_scm_desc *desc,
> + struct qcom_scm_res *res)
> +{
> + return -EINVAL;
> +}
> +#endif
>
> #define QCOM_SCM_SVC_BOOT 0x01
> #define QCOM_SCM_BOOT_SET_ADDR 0x01
> diff --git a/include/linux/qcom_scm.h b/include/linux/qcom_scm.h
> index 0165824c5128..0ec905d56e1a 100644
> --- a/include/linux/qcom_scm.h
> +++ b/include/linux/qcom_scm.h
> @@ -64,9 +64,6 @@ enum qcom_scm_ice_cipher {
> #if IS_ENABLED(CONFIG_QCOM_SCM)
> extern bool qcom_scm_is_available(void);
>
> -extern int qcom_scm_set_cold_boot_addr(void *entry, const cpumask_t *cpus);
> -extern int qcom_scm_set_warm_boot_addr(void *entry, const cpumask_t *cpus);
> -extern void qcom_scm_cpu_power_down(u32 flags);
> extern int qcom_scm_set_remote_state(u32 state, u32 id);
>
> extern int qcom_scm_pas_init_image(u32 peripheral, const void *metadata,
> @@ -115,11 +112,6 @@ extern int qcom_scm_qsmmu500_wait_safe_toggle(bool en);
>
> static inline bool qcom_scm_is_available(void) { return false; }
>
> -static inline int qcom_scm_set_cold_boot_addr(void *entry,
> - const cpumask_t *cpus) { return -ENODEV; }
> -static inline int qcom_scm_set_warm_boot_addr(void *entry,
> - const cpumask_t *cpus) { return -ENODEV; }
> -static inline void qcom_scm_cpu_power_down(u32 flags) {}
> static inline u32 qcom_scm_set_remote_state(u32 state,u32 id)
> { return -ENODEV; }
>
> @@ -171,4 +163,17 @@ static inline int qcom_scm_hdcp_req(struct qcom_scm_hdcp_req *req, u32 req_cnt,
> static inline int qcom_scm_qsmmu500_wait_safe_toggle(bool en)
> { return -ENODEV; }
> #endif
> +
> +#if IS_ENABLED(CONFIG_ARM) && IS_ENABLED(CONFIG_QCOM_SCM)
> +extern int qcom_scm_set_cold_boot_addr(void *entry, const cpumask_t *cpus);
> +extern int qcom_scm_set_warm_boot_addr(void *entry, const cpumask_t *cpus);
> +extern void qcom_scm_cpu_power_down(u32 flags);
> +#else
> +static inline int qcom_scm_set_cold_boot_addr(void *entry,
> + const cpumask_t *cpus) { return -ENODEV; }
> +static inline int qcom_scm_set_warm_boot_addr(void *entry,
> + const cpumask_t *cpus) { return -ENODEV; }
> +static inline void qcom_scm_cpu_power_down(u32 flags) {}
> +#endif
> +
> #endif
>
> base-commit: 3b9cdafb5358eb9f3790de2f728f765fef100731
> prerequisite-patch-id: 77da2cfd7591b1d7c35e879dca67d4f037f40e48
> prerequisite-patch-id: 021337034973fa8ce52fc8c84787f40dabb33df6
> prerequisite-patch-id: 5d374e97d8f0d384098a46e91006811ab89c84b0
> prerequisite-patch-id: 892de894cc937f7fe6ddb8f95ec9e2e3557830c7
> prerequisite-patch-id: 33b2442181aeb8adfa1c08d9a167d3bcbd1660fe
>
Powered by blists - more mailing lists